Skip to content

Commit 76cbf81

Browse files
authored
Merge pull request #3734 from docker/feat/effort-completion
feat(tui): argument auto-completion for /effort
2 parents 5339608 + f40621a commit 76cbf81

8 files changed

Lines changed: 420 additions & 10 deletions

File tree

docs/features/tui/index.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ Type `/` during a session to see available commands, or press <kbd>Ctrl</kbd>+<k
7878
| `/export` | Export the session as HTML |
7979
| `/sessions` | Browse and load past sessions |
8080
| `/model` | Change the model for the current agent |
81-
| `/effort` | Set the current model's reasoning-effort level (`/effort <none\|minimal\|low\|medium\|high\|xhigh\|max>`, or `/effort` alone to pick from the supported levels; reasoning models only) |
81+
| `/effort` | Set the current model's reasoning-effort level (`/effort <none\|minimal\|low\|medium\|high\|xhigh\|max>`, or `/effort` alone to pick from the supported levels; reasoning models only). Press <kbd>Tab</kbd> after `/effort` and a space to complete a level the current model supports |
8282
| `/settings` | Manage appearance, behavior, and notification preferences |
8383
| `/yolo` | Toggle automatic tool call approval |
8484
| `/title` | Set or regenerate session title |

docs/guides/thinking/index.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -334,6 +334,7 @@ While running in the TUI, press **Shift+Tab** to cycle the thinking effort level
334334
- This applies as a session override — it is **not** saved to the config file. The next session starts from the level defined in your YAML.
335335
- For models that don't support reasoning, and for remote runtimes, Shift+Tab is a no-op and an informational message is displayed.
336336
- `/effort` only accepts levels the current model supports; requesting an unsupported level shows the model's supported list. Like Shift+Tab, it is unavailable for non-reasoning models and remote runtimes.
337+
- Press <kbd>Tab</kbd> after `/effort` and a space to complete a level from the current model's supported range; it lists the same levels the picker shows (and, like the picker, offers no candidates for non-reasoning models or remote runtimes).
337338

338339
## Sharing Thinking Config Across Models
339340

pkg/app/app.go

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -296,6 +296,27 @@ func (a *App) RestartToolset(ctx context.Context, name string) error {
296296
return a.runtime.RestartToolset(ctx, name)
297297
}
298298

299+
// agentThinkingLevelsProvider is an optional runtime capability: resolving
300+
// the thinking-effort levels the current agent's active model supports.
301+
// Only the local runtime (which holds the agent and its model config)
302+
// implements it; remote runtimes don't, so /effort argument completion
303+
// simply offers no candidates for them.
304+
type agentThinkingLevelsProvider interface {
305+
CurrentAgentThinkingLevels(ctx context.Context) []effort.Level
306+
}
307+
308+
// CurrentAgentThinkingLevels returns the thinking-effort levels supported by
309+
// the current agent's active model, or nil when the runtime cannot resolve
310+
// this (remote runtime, unresolvable agent/model, or a model that does not
311+
// support thinking at all). Backs the /effort argument completer (#3731).
312+
func (a *App) CurrentAgentThinkingLevels(ctx context.Context) []effort.Level {
313+
p, ok := a.runtime.(agentThinkingLevelsProvider)
314+
if !ok {
315+
return nil
316+
}
317+
return p.CurrentAgentThinkingLevels(ctx)
318+
}
319+
299320
// CurrentAgentCommands returns the commands for the active agent
300321
func (a *App) CurrentAgentCommands(ctx context.Context) types.Commands {
301322
return a.runtime.CurrentAgentInfo(ctx).Commands

pkg/app/app_test.go

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -857,6 +857,37 @@ func (m *liveSessionsMockRuntime) CompactLiveSession(_ context.Context, sessionI
857857
return nil
858858
}
859859

860+
// thinkingLevelsMockRuntime is a minimal mockRuntime extension implementing
861+
// agentThinkingLevelsProvider, exercising the pass-through half of
862+
// App.CurrentAgentThinkingLevels (the type-assertion-miss half is covered by
863+
// plain *mockRuntime, which does not implement the interface).
864+
type thinkingLevelsMockRuntime struct {
865+
mockRuntime
866+
867+
levels []effort.Level
868+
}
869+
870+
func (m *thinkingLevelsMockRuntime) CurrentAgentThinkingLevels(context.Context) []effort.Level {
871+
return m.levels
872+
}
873+
874+
func TestApp_CurrentAgentThinkingLevels_UnsupportedRuntime(t *testing.T) {
875+
t.Parallel()
876+
877+
app := New(t.Context(), &mockRuntime{}, session.New())
878+
assert.Nil(t, app.CurrentAgentThinkingLevels(t.Context()),
879+
"runtimes without a thinking-levels resolution degrade to no /effort candidates")
880+
}
881+
882+
func TestApp_CurrentAgentThinkingLevels_PassesThroughRuntimeLevels(t *testing.T) {
883+
t.Parallel()
884+
885+
rt := &thinkingLevelsMockRuntime{levels: []effort.Level{effort.Low, effort.Medium, effort.High}}
886+
app := New(t.Context(), rt, session.New())
887+
888+
assert.Equal(t, []effort.Level{effort.Low, effort.Medium, effort.High}, app.CurrentAgentThinkingLevels(t.Context()))
889+
}
890+
860891
func TestApp_LiveSessions_UnsupportedRuntime(t *testing.T) {
861892
t.Parallel()
862893

pkg/runtime/model_switcher.go

Lines changed: 72 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -238,10 +238,78 @@ func levelNames(levels []effort.Level) string {
238238
return strings.Join(names, ", ")
239239
}
240240

241+
// resolveThinkingLevelsForModels returns the thinking-effort levels
242+
// supported by models[0] (the agent's primary effective model) plus its
243+
// currently active level. It is the pure resolution step shared by
244+
// resolveAgentThinkingLevels (the read-only getter path, which snapshots
245+
// EffectiveModels itself) and applyAgentThinkingLevel (the setter path,
246+
// which must reuse the exact same snapshot it later re-creates providers
247+
// from — see resolveAgentThinkingLevels for why that sharing matters).
248+
func (r *LocalRuntime) resolveThinkingLevelsForModels(ctx context.Context, models []provider.Provider) ([]effort.Level, effort.Level, error) {
249+
if len(models) == 0 {
250+
return nil, "", errors.New("agent has no model configured")
251+
}
252+
253+
baseCfg := models[0].BaseConfig().ModelConfig
254+
if !r.modelSupportsThinking(ctx, &baseCfg) {
255+
return nil, "", fmt.Errorf("model %q does not support thinking levels: %w", baseCfg.DisplayOrModel(), ErrUnsupported)
256+
}
257+
258+
return modelinfo.SupportedThinkingLevels(baseCfg.Provider, baseCfg.Model), currentThinkingLevel(&baseCfg), nil
259+
}
260+
261+
// resolveAgentThinkingLevels resolves agentName's current effective model
262+
// and returns the thinking-effort levels it supports plus its currently
263+
// active level. This is the single source of truth shared by
264+
// applyAgentThinkingLevel (which validates a pick against it) and
265+
// CurrentAgentThinkingLevels (which exposes it read-only for /effort
266+
// argument completion), so a level offered by one can never be rejected by
267+
// the other (#3731).
268+
//
269+
// It is read-only, so a single EffectiveModels snapshot is correct here.
270+
// applyAgentThinkingLevel must NOT call this: it needs the same snapshot
271+
// for both resolution and provider recreation (a second, later
272+
// EffectiveModels() call could race a concurrent /model change or scoped
273+
// override and validate against one model while applying to another).
274+
func (r *LocalRuntime) resolveAgentThinkingLevels(ctx context.Context, agentName string) ([]effort.Level, effort.Level, error) {
275+
if r.modelSwitcherCfg == nil {
276+
return nil, "", ErrUnsupported
277+
}
278+
279+
a, err := r.team.Agent(agentName)
280+
if err != nil {
281+
return nil, "", fmt.Errorf("agent not found: %w", err)
282+
}
283+
284+
return r.resolveThinkingLevelsForModels(ctx, a.EffectiveModels())
285+
}
286+
287+
// CurrentAgentThinkingLevels returns the thinking-effort levels the current
288+
// agent's active model supports, or nil when the runtime has no model
289+
// switcher configured, the agent/model can't be resolved, or the model does
290+
// not support thinking at all. Backs the /effort argument completer
291+
// (#3731): candidates must come from this resolution, not the static effort
292+
// vocabulary, because SetAgentThinkingLevel hard-rejects any level outside
293+
// it.
294+
func (r *LocalRuntime) CurrentAgentThinkingLevels(ctx context.Context) []effort.Level {
295+
supported, _, err := r.resolveAgentThinkingLevels(ctx, r.currentAgentName())
296+
if err != nil {
297+
return nil
298+
}
299+
return supported
300+
}
301+
241302
// applyAgentThinkingLevel resolves the agent's current model, asks pick to
242303
// choose the target level among the model's supported ones, re-creates the
243304
// effective provider(s) with that level, and installs them as a runtime
244305
// override.
306+
//
307+
// It snapshots EffectiveModels exactly once and reuses that same snapshot
308+
// for both capability resolution and provider recreation. Taking a second,
309+
// later snapshot would let a concurrent /model change or scoped override
310+
// land in between: the level would be validated against one model but
311+
// applied to another, and — if the second snapshot came back empty — the
312+
// override could be silently cleared while still reporting success.
245313
func (r *LocalRuntime) applyAgentThinkingLevel(ctx context.Context, agentName string, pick func(supported []effort.Level, current effort.Level) (effort.Level, error)) (effort.Level, error) {
246314
if r.modelSwitcherCfg == nil {
247315
return "", ErrUnsupported
@@ -253,17 +321,12 @@ func (r *LocalRuntime) applyAgentThinkingLevel(ctx context.Context, agentName st
253321
}
254322

255323
models := a.EffectiveModels()
256-
if len(models) == 0 {
257-
return "", errors.New("agent has no model configured")
258-
}
259-
260-
baseCfg := models[0].BaseConfig().ModelConfig
261-
if !r.modelSupportsThinking(ctx, &baseCfg) {
262-
return "", fmt.Errorf("model %q does not support thinking levels: %w", baseCfg.DisplayOrModel(), ErrUnsupported)
324+
supported, current, err := r.resolveThinkingLevelsForModels(ctx, models)
325+
if err != nil {
326+
return "", err
263327
}
264328

265-
supported := modelinfo.SupportedThinkingLevels(baseCfg.Provider, baseCfg.Model)
266-
next, err := pick(supported, currentThinkingLevel(&baseCfg))
329+
next, err := pick(supported, current)
267330
if err != nil {
268331
return "", err
269332
}

pkg/runtime/model_switcher_test.go

Lines changed: 153 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -300,6 +300,159 @@ func TestSetAgentThinkingLevel(t *testing.T) {
300300
})
301301
}
302302

303+
// TestSetAgentThinkingLevel_ConcurrentOverrideBetweenResolutionAndApply is
304+
// the regression test for the single-snapshot fix: applyAgentThinkingLevel
305+
// must resolve capabilities and re-create providers from the SAME
306+
// EffectiveModels snapshot. A second, later snapshot would let a
307+
// concurrent /model change or scoped override land between resolution and
308+
// application, validating the effort against one model but applying it to
309+
// another (see #3731 follow-up).
310+
func TestSetAgentThinkingLevel_ConcurrentOverrideBetweenResolutionAndApply(t *testing.T) {
311+
t.Parallel()
312+
313+
t.Run("override swapped mid-flight does not leak into the applied provider", func(t *testing.T) {
314+
t.Parallel()
315+
316+
// gpt-5 tops out at high; opus 4.7 accepts max. If applyAgentThinkingLevel
317+
// re-read EffectiveModels after pick ran, it would recreate the override
318+
// from opus instead of gpt-5, silently applying "high" to a model that was
319+
// never validated against it.
320+
modelA := newConfigProvider(latest.ModelConfig{Provider: "openai", Model: "gpt-5"})
321+
modelB := newConfigProvider(latest.ModelConfig{Provider: "anthropic", Model: "claude-opus-4-7"})
322+
323+
root := agent.New("root", "test", agent.WithModel(modelA))
324+
r := &LocalRuntime{
325+
team: team.New(team.WithAgents(root)),
326+
modelSwitcherCfg: &ModelSwitcherConfig{
327+
ProviderRegistry: testProviderRegistry(),
328+
EnvProvider: environment.NewMapEnvProvider(map[string]string{
329+
"OPENAI_API_KEY": "sk-test",
330+
"ANTHROPIC_API_KEY": "sk-test",
331+
}),
332+
},
333+
}
334+
335+
// pick runs after the snapshot is resolved but before providers are
336+
// re-created; a concurrent /model change landing here is exactly the
337+
// race the bug exposed.
338+
level, err := r.applyAgentThinkingLevel(t.Context(), "root", func(supported []effort.Level, _ effort.Level) (effort.Level, error) {
339+
require.NotContains(t, supported, effort.Max, "resolution must see model A (gpt-5), which tops out at high")
340+
root.SetModelOverride(modelB)
341+
return effort.High, nil
342+
})
343+
require.NoError(t, err)
344+
assert.Equal(t, effort.High, level)
345+
346+
override := root.Model(t.Context())
347+
require.NotNil(t, override)
348+
assert.Equal(t, "openai", override.BaseConfig().ModelConfig.Provider,
349+
"the applied provider must be re-created from the model validated against (A), not the one swapped in mid-flight (B)")
350+
budget := override.BaseConfig().ModelConfig.ThinkingBudget
351+
require.NotNil(t, budget)
352+
assert.Equal(t, "high", budget.Effort)
353+
})
354+
355+
t.Run("override cleared mid-flight does not install an empty override", func(t *testing.T) {
356+
t.Parallel()
357+
358+
// No configured models: the agent's only model comes from the runtime
359+
// override, so clearing it mid-flight reproduces the empty second-snapshot
360+
// case the bug allowed through.
361+
modelA := newConfigProvider(latest.ModelConfig{Provider: "openai", Model: "gpt-5"})
362+
root := agent.New("root", "test")
363+
root.SetModelOverride(modelA)
364+
365+
r := &LocalRuntime{
366+
team: team.New(team.WithAgents(root)),
367+
modelSwitcherCfg: &ModelSwitcherConfig{
368+
ProviderRegistry: testProviderRegistry(),
369+
EnvProvider: environment.NewMapEnvProvider(map[string]string{"OPENAI_API_KEY": "sk-test"}),
370+
},
371+
}
372+
373+
level, err := r.applyAgentThinkingLevel(t.Context(), "root", func(_ []effort.Level, _ effort.Level) (effort.Level, error) {
374+
// Simulate a concurrent override reset (e.g. /model default) landing
375+
// between resolution and provider recreation.
376+
root.SetModelOverride()
377+
return effort.High, nil
378+
})
379+
require.NoError(t, err)
380+
assert.Equal(t, effort.High, level)
381+
382+
override := root.Model(t.Context())
383+
require.NotNil(t, override, "must not silently install an empty override while reporting success")
384+
assert.Equal(t, "openai", override.BaseConfig().ModelConfig.Provider)
385+
})
386+
}
387+
388+
func TestCurrentAgentThinkingLevels(t *testing.T) {
389+
t.Parallel()
390+
391+
newThinkingRuntimeWithRouter := func(cfg latest.ModelConfig, env map[string]string) *LocalRuntime {
392+
root := agent.New("root", "test", agent.WithModel(newConfigProvider(cfg)))
393+
tm := team.New(team.WithAgents(root))
394+
return &LocalRuntime{
395+
team: tm,
396+
agents: newAgentRouter(tm, "root"),
397+
modelSwitcherCfg: &ModelSwitcherConfig{
398+
ProviderRegistry: testProviderRegistry(),
399+
EnvProvider: environment.NewMapEnvProvider(env),
400+
},
401+
}
402+
}
403+
404+
t.Run("reasoning model returns its supported levels", func(t *testing.T) {
405+
t.Parallel()
406+
r := newThinkingRuntimeWithRouter(
407+
latest.ModelConfig{Provider: "openai", Model: "gpt-5"},
408+
map[string]string{"OPENAI_API_KEY": "sk-test"},
409+
)
410+
411+
levels := r.CurrentAgentThinkingLevels(t.Context())
412+
assert.NotEmpty(t, levels)
413+
assert.Contains(t, levels, effort.High)
414+
assert.NotContains(t, levels, effort.Max, "gpt-5 tops out below max")
415+
})
416+
417+
t.Run("non-reasoning model is nil", func(t *testing.T) {
418+
t.Parallel()
419+
r := newThinkingRuntimeWithRouter(
420+
latest.ModelConfig{Provider: "openai", Model: "gpt-4o"},
421+
map[string]string{"OPENAI_API_KEY": "sk-test"},
422+
)
423+
424+
assert.Nil(t, r.CurrentAgentThinkingLevels(t.Context()))
425+
})
426+
427+
t.Run("nil modelSwitcherCfg is nil", func(t *testing.T) {
428+
t.Parallel()
429+
root := agent.New("root", "test")
430+
tm := team.New(team.WithAgents(root))
431+
r := &LocalRuntime{team: tm, agents: newAgentRouter(tm, "root")}
432+
433+
assert.Nil(t, r.CurrentAgentThinkingLevels(t.Context()))
434+
})
435+
436+
t.Run("matches the levels SetAgentThinkingLevel validates against", func(t *testing.T) {
437+
t.Parallel()
438+
r := newThinkingRuntimeWithRouter(
439+
latest.ModelConfig{Provider: "anthropic", Model: "claude-opus-4-7"},
440+
map[string]string{"ANTHROPIC_API_KEY": "sk-test"},
441+
)
442+
443+
levels := r.CurrentAgentThinkingLevels(t.Context())
444+
require.Contains(t, levels, effort.Max, "opus 4.7 top tier")
445+
446+
// Every level CurrentAgentThinkingLevels reports must be one
447+
// SetAgentThinkingLevel actually accepts -- the single-source-of-truth
448+
// guarantee #3731 depends on.
449+
for _, level := range levels {
450+
_, err := r.SetAgentThinkingLevel(t.Context(), "root", level)
451+
assert.NoError(t, err, "level %q reported as supported but rejected", level)
452+
}
453+
})
454+
}
455+
303456
// TestSetAgentThinkingLevel_NoneSurvivesOnGPT56 is the regression test for
304457
// the gpt-5.6+ "none" gating: cycling an agent's model to None on a model
305458
// with a real API-level none effort must produce a provider whose config

0 commit comments

Comments
 (0)