From 5133c9284fe023436f4251fac2a6e8fb00a883b4 Mon Sep 17 00:00:00 2001 From: Cristina Poncela Cubeiro <140309543+cristinaponcela@users.noreply.github.com> Date: Thu, 20 Aug 2026 13:30:47 +0200 Subject: [PATCH] chore(settings-selector): get rid of --default and global model ctrl+s is enough, there goes my joy --- .../coding-agent/src/core/agent-session.ts | 17 +-- .../coding-agent/src/core/slash-commands.ts | 4 +- .../components/settings-selector.ts | 101 +------------- .../src/modes/interactive/interactive-mode.ts | 125 +++--------------- .../test/interactive-mode-status.test.ts | 64 +-------- .../agent-session-model-extension.test.ts | 19 ++- 6 files changed, 50 insertions(+), 280 deletions(-) diff --git a/packages/coding-agent/src/core/agent-session.ts b/packages/coding-agent/src/core/agent-session.ts index 1be95d1d1..17b5b5909 100644 --- a/packages/coding-agent/src/core/agent-session.ts +++ b/packages/coding-agent/src/core/agent-session.ts @@ -1608,8 +1608,8 @@ export class AgentSession { this.settingsManager.setDefaultModelAndProvider(model.provider, model.id); } - // Re-clamp session thinking level for new model's capabilities. - // Per-model thinking level overrides take priority over session carry-over. + // Apply thinking level for the new model. + // Per-model thinking level overrides take priority over the global default. // Model persistence does not implicitly rewrite the global thinking default. this.setThinkingLevel(thinkingLevel); @@ -1660,9 +1660,9 @@ export class AgentSession { this.settingsManager.setDefaultModelAndProvider(next.model.provider, next.model.id); } - // Apply session thinking level. - // - Explicit scoped model thinking level overrides current session level - // - Undefined scoped model thinking level inherits the current session preference + // Apply thinking level for the new model. + // - Explicit scoped model thinking level overrides defaults + // - Per-model thinking level overrides take priority over the global default // setThinkingLevel clamps to model capabilities. // Model persistence does not implicitly rewrite the global thinking default. this.setThinkingLevel(thinkingLevel); @@ -1694,7 +1694,7 @@ export class AgentSession { this.settingsManager.setDefaultModelAndProvider(nextModel.provider, nextModel.id); } - // Re-clamp session thinking level for new model's capabilities. + // Apply thinking level for the new model. // Model persistence does not implicitly rewrite the global thinking default. this.setThinkingLevel(thinkingLevel); @@ -1781,10 +1781,7 @@ export class AgentSession { return perModel; } } - if (!this.supportsThinking()) { - return this.settingsManager.getDefaultThinkingLevel() ?? DEFAULT_THINKING_LEVEL; - } - return this.thinkingLevel; + return this.settingsManager.getDefaultThinkingLevel() ?? this.thinkingLevel ?? DEFAULT_THINKING_LEVEL; } private _clampThinkingLevel(level: ThinkingLevel, _availableLevels: ThinkingLevel[]): ThinkingLevel { diff --git a/packages/coding-agent/src/core/slash-commands.ts b/packages/coding-agent/src/core/slash-commands.ts index d149074c2..57c3e23d6 100644 --- a/packages/coding-agent/src/core/slash-commands.ts +++ b/packages/coding-agent/src/core/slash-commands.ts @@ -18,8 +18,8 @@ export interface BuiltinSlashCommand { export const BUILTIN_SLASH_COMMANDS: ReadonlyArray = [ { name: "settings", description: "Open settings menu" }, - { name: "model", description: "Select model (opens selector UI)", argumentHint: "[--default] " }, - { name: "thinking", description: "Set thinking level", argumentHint: "[--default] " }, + { name: "model", description: "Select model (opens selector UI)", argumentHint: "" }, + { name: "thinking", description: "Set thinking level", argumentHint: "" }, { name: "scoped-models", description: "Enable/disable models for Ctrl+P cycling" }, { name: "export", description: "Export session (HTML default, or specify path: .html/.jsonl)" }, { name: "import", description: "Import and resume a session from a JSONL file" }, diff --git a/packages/coding-agent/src/modes/interactive/components/settings-selector.ts b/packages/coding-agent/src/modes/interactive/components/settings-selector.ts index 0cfb64923..c8683d290 100644 --- a/packages/coding-agent/src/modes/interactive/components/settings-selector.ts +++ b/packages/coding-agent/src/modes/interactive/components/settings-selector.ts @@ -24,9 +24,6 @@ import { DynamicBorder } from "./dynamic-border.ts"; import { keyDisplayText } from "./keybinding-hints.ts"; import { SelectSubmenu, SteppedSubmenu, type SteppedSubmenuStep } from "./settings-submenu.ts"; -const NO_DEFAULT_MODEL_VALUE = "__none__"; -const NO_DEFAULT_MODEL_LABEL = "not set"; - const MODEL_PICKER_LAYOUT = { minPrimaryColumnWidth: 12, maxPrimaryColumnWidth: 46 }; const THINKING_DESCRIPTIONS: Record = { @@ -91,7 +88,6 @@ export interface SettingsConfig { export interface SettingsCallbacks { onAutoCompactChange: (enabled: boolean) => void; - onDefaultModelChange: (model: Model) => Promise; onShowImagesChange: (enabled: boolean) => void; onImageWidthCellsChange: (width: number) => void; onAutoResizeImagesChange: (enabled: boolean) => void; @@ -183,16 +179,6 @@ function modelDisplayLabel(model: Model): string { return `${model.id} [${model.provider}]`; } -function defaultModelDisplayValue( - key: string, - model: Model | undefined, - overrides: Record, -): string { - const label = model ? modelDisplayLabel(model) : key; - const override = overrides[key]; - return override ? `${label} \u00b7 ${override}` : label; -} - function modelThinkingOverridesSummary(overrides: Record): string { const count = Object.keys(overrides).length; if (count === 0) return "none"; @@ -203,24 +189,6 @@ function modelItemLabel(model: Model): string { return `${model.id} ${theme.fg("muted", `[${model.provider}]`)}`; } -function defaultModelItems(models: readonly Model[], overrides?: Record): SelectItem[] { - return [...models] - .sort((a, b) => { - const providerCompare = a.provider.localeCompare(b.provider); - if (providerCompare !== 0) return providerCompare; - return (a.name || a.id).localeCompare(b.name || b.id); - }) - .map((model) => { - const key = modelSettingKey(model); - const override = overrides?.[key]; - return { - value: key, - label: modelItemLabel(model), - description: override ?? undefined, - }; - }); -} - function themeItems(availableThemes: string[]): SelectItem[] { return availableThemes.map((name) => ({ value: name, label: name })); } @@ -478,13 +446,10 @@ export class SettingsSelectorComponent extends Container { const cycleThinkingKey = keyDisplayText("app.thinking.cycle"); let currentWarnings = { ...config.warnings }; const currentModelThinkingLevels = { ...config.modelThinkingLevels }; - let lastSelectedDefaultModel: Model | undefined; const defaultModelByValue = new Map( config.availableDefaultModels.map((model) => [modelSettingKey(model), model]), ); - let currentDefaultModelKey: string | undefined = defaultModelByValue.has(config.defaultModel) - ? config.defaultModel - : undefined; + const currentDefaultModelKey = defaultModelByValue.has(config.defaultModel) ? config.defaultModel : undefined; const items: SettingItem[] = [ { @@ -516,55 +481,6 @@ export class SettingsSelectorComponent extends Container { currentValue: config.transport, values: ["sse", "websocket", "websocket-cached", "auto"], }, - { - id: "default-model", - label: "Default model", - description: "Startup model for new sessions", - currentValue: defaultModelDisplayValue( - config.defaultModel, - defaultModelByValue.get(config.defaultModel), - currentModelThinkingLevels, - ), - submenu: (currentValue, done) => { - const fresh = defaultModelItems(config.availableDefaultModels, currentModelThinkingLevels); - const options = - fresh.length > 0 - ? fresh - : [ - { - value: NO_DEFAULT_MODEL_VALUE, - label: "No models available", - description: "Log in to a provider or configure an API key first", - }, - ]; - return new SelectSubmenu( - "Default Model", - "Select the model to use when starting new sessions", - options, - currentValue === NO_DEFAULT_MODEL_LABEL ? NO_DEFAULT_MODEL_VALUE : currentValue, - (value) => { - const model = defaultModelByValue.get(value); - if (!model) { - done(); - return; - } - void callbacks.onDefaultModelChange(model).then( - () => { - lastSelectedDefaultModel = model; - currentDefaultModelKey = value; - done(defaultModelDisplayValue(value, model, currentModelThinkingLevels), { - navigateTo: "model-thinking", - }); - }, - () => done(), - ); - }, - () => done(), - undefined, - { searchable: true, layout: MODEL_PICKER_LAYOUT }, - ); - }, - }, { id: "http-idle-timeout", label: "HTTP idle timeout", @@ -679,9 +595,6 @@ export class SettingsSelectorComponent extends Container { description: `Override the default thinking level for specific models. ${cycleThinkingKey} cycles in-session.`, currentValue: modelThinkingOverridesSummary(currentModelThinkingLevels), submenu: (_currentValue, done) => { - const preselected = lastSelectedDefaultModel; - lastSelectedDefaultModel = undefined; - const steps: SteppedSubmenuStep[] = [ { key: "model", @@ -770,19 +683,9 @@ export class SettingsSelectorComponent extends Container { } }, () => { - this.settingsList.updateValue( - "default-model", - defaultModelDisplayValue( - currentDefaultModelKey ?? config.defaultModel, - defaultModelByValue.get(currentDefaultModelKey ?? config.defaultModel), - currentModelThinkingLevels, - ), - ); done(summary()); }, - preselected - ? { startAtStep: 1, initialContext: { model: modelSettingKey(preselected) } } - : { loop: true }, + { loop: true }, ); }, }, diff --git a/packages/coding-agent/src/modes/interactive/interactive-mode.ts b/packages/coding-agent/src/modes/interactive/interactive-mode.ts index 1a1edef95..b20b7ef5e 100644 --- a/packages/coding-agent/src/modes/interactive/interactive-mode.ts +++ b/packages/coding-agent/src/modes/interactive/interactive-mode.ts @@ -233,44 +233,6 @@ function isDeadTerminalError(error: unknown): boolean { return code !== undefined && DEAD_TERMINAL_ERROR_CODES.has(code); } -export interface DefaultFlagArgs { - searchTerm?: string; - persist: boolean; - error?: string; -} - -export function parseDefaultFlagArgs(commandName: string, args: string | undefined): DefaultFlagArgs { - if (!args?.trim()) return { persist: false }; - - let persist = false; - const searchTerms: string[] = []; - for (const token of args.trim().split(/\s+/u)) { - if (token === "--default") { - persist = true; - continue; - } - - if (token.startsWith("--default=")) { - persist = true; - const value = token.slice("--default=".length); - if (value) searchTerms.push(value); - continue; - } - - if (token.startsWith("--")) { - return { - persist, - searchTerm: searchTerms.join(" ") || undefined, - error: `Unknown /${commandName} option "${token}". Supported option: --default.`, - }; - } - - searchTerms.push(token); - } - - return { persist, searchTerm: searchTerms.join(" ") || undefined }; -} - const ANTHROPIC_SUBSCRIPTION_AUTH_WARNING = "Anthropic subscription auth is active. Third-party harness usage draws from extra usage and is billed per token, not your Claude plan limits. Manage extra usage at https://claude.ai/settings/usage. Disable this warning in /settings."; @@ -721,16 +683,6 @@ export class InteractiveMode { const modelCommand = slashCommands.find((command) => command.name === "model"); if (modelCommand) { modelCommand.getArgumentCompletions = (prefix: string): AutocompleteItem[] | null => { - const trimmedPrefix = prefix.trimStart(); - if (trimmedPrefix.startsWith("--") && !trimmedPrefix.includes(" ")) { - return "--default".startsWith(trimmedPrefix) - ? [{ value: "--default ", label: "--default", description: "Set as startup default" }] - : null; - } - - const parsed = parseDefaultFlagArgs("model", prefix); - if (parsed.error) return null; - const models = this.session.scopedModels.length > 0 ? this.session.scopedModels.map((s) => s.model) @@ -746,11 +698,10 @@ export class InteractiveMode { label: `${m.provider}/${m.id}`, })); - const searchPrefix = parsed.persist ? (parsed.searchTerm ?? "") : prefix; - return createFuzzyAutocompleteItems(items, searchPrefix, getModelSearchText, (item) => ({ - value: parsed.persist ? `--default ${item.label}` : item.label, + return createFuzzyAutocompleteItems(items, prefix, getModelSearchText, (item) => ({ + value: item.label, label: item.id, - description: parsed.persist ? `${item.provider} · set as startup default` : item.provider, + description: item.provider, })); }; } @@ -758,24 +709,13 @@ export class InteractiveMode { const thinkingCommand = slashCommands.find((command) => command.name === "thinking"); if (thinkingCommand) { thinkingCommand.getArgumentCompletions = (prefix: string): AutocompleteItem[] | null => { - const trimmedPrefix = prefix.trimStart(); - if (trimmedPrefix.startsWith("--") && !trimmedPrefix.includes(" ")) { - return "--default".startsWith(trimmedPrefix) - ? [{ value: "--default ", label: "--default", description: "Set as startup default" }] - : null; - } - - const parsed = parseDefaultFlagArgs("thinking", prefix); - if (parsed.error) return null; - const searchPrefix = parsed.persist ? (parsed.searchTerm ?? "") : prefix; return createFuzzyAutocompleteItems( this.session.getAvailableThinkingLevels(), - searchPrefix, + prefix, (level) => level, (level) => ({ - value: parsed.persist ? `--default ${level}` : level, + value: level, label: level, - description: parsed.persist ? "set as startup default" : undefined, }), ); }; @@ -3027,26 +2967,15 @@ export class InteractiveMode { return; } if (text === "/model" || text.startsWith("/model ")) { - const args = parseDefaultFlagArgs("model", text.startsWith("/model ") ? text.slice(7).trim() : undefined); + const searchTerm = text.startsWith("/model ") ? text.slice(7).trim() : undefined; this.editor.setText(""); - if (args.error) { - this.showError(args.error); - return; - } - await this.handleModelCommand(args.searchTerm, { persist: args.persist }); + await this.handleModelCommand(searchTerm); return; } if (text === "/thinking" || text.startsWith("/thinking ")) { - const args = parseDefaultFlagArgs( - "thinking", - text.startsWith("/thinking ") ? text.slice(10).trim() : undefined, - ); + const searchTerm = text.startsWith("/thinking ") ? text.slice(10).trim() : undefined; this.editor.setText(""); - if (args.error) { - this.showError(args.error); - return; - } - this.handleThinkingCommand(args.searchTerm, { persist: args.persist }); + this.handleThinkingCommand(searchTerm); return; } if (text === "/export" || text.startsWith("/export ")) { @@ -4635,18 +4564,6 @@ export class InteractiveMode { this.session.setAutoCompactionEnabled(enabled); this.footer.setAutoCompactEnabled(enabled); }, - onDefaultModelChange: async (model) => { - try { - await this.session.setModel(model, { persist: true }); - this.footer.invalidate(); - this.updateEditorBorderColor(); - this.showStatus(`Default model: ${model.provider}/${model.id}`); - void this.maybeWarnAboutAnthropicSubscriptionAuth(model); - this.checkDaxnutsEasterEgg(model); - } catch (error) { - this.showError(error instanceof Error ? error.message : String(error)); - } - }, onShowImagesChange: (enabled) => { this.settingsManager.setShowImages(enabled); for (const child of this.chatContainer.children) { @@ -4836,10 +4753,10 @@ export class InteractiveMode { }); } - private handleThinkingCommand(searchTerm?: string, options: { persist?: boolean } = {}): void { + private handleThinkingCommand(searchTerm?: string): void { const availableLevels = this.session.getAvailableThinkingLevels(); if (!searchTerm) { - this.showThinkingSelector(options); + this.showThinkingSelector(); return; } @@ -4850,7 +4767,7 @@ export class InteractiveMode { return; } - this.selectThinkingLevel(level, options.persist === true); + this.selectThinkingLevel(level, false); } private selectThinkingLevel(level: ThinkingLevel, persist: boolean): void { @@ -4864,7 +4781,7 @@ export class InteractiveMode { } } - private showThinkingSelector(options: { persist?: boolean } = {}): void { + private showThinkingSelector(): void { this.showSelector((done) => { const selectLevel = (level: ThinkingLevel, persist: boolean) => { this.selectThinkingLevel(level, persist); @@ -4873,7 +4790,7 @@ export class InteractiveMode { const selector = new ThinkingSelectorComponent( this.session.thinkingLevel ?? DEFAULT_THINKING_LEVEL, this.session.getAvailableThinkingLevels(), - (level) => selectLevel(level, options.persist === true), + (level) => selectLevel(level, false), () => { done(); this.ui.requestRender(); @@ -4884,19 +4801,19 @@ export class InteractiveMode { }); } - private async handleModelCommand(searchTerm?: string, options: { persist?: boolean } = {}): Promise { + private async handleModelCommand(searchTerm?: string): Promise { if (!searchTerm) { - this.showModelSelector(undefined, options); + this.showModelSelector(); return; } const model = await this.findExactModelMatch(searchTerm); if (model) { try { - await this.session.setModel(model, { persist: options.persist === true }); + await this.session.setModel(model, { persist: false }); this.footer.invalidate(); this.updateEditorBorderColor(); - this.showStatus(options.persist ? `Default model: ${model.provider}/${model.id}` : `Model: ${model.id}`); + this.showStatus(`Model: ${model.id}`); void this.maybeWarnAboutAnthropicSubscriptionAuth(model); this.checkDaxnutsEasterEgg(model); } catch (error) { @@ -4905,7 +4822,7 @@ export class InteractiveMode { return; } - this.showModelSelector(searchTerm, options); + this.showModelSelector(searchTerm); } private async findExactModelMatch(searchTerm: string): Promise | undefined> { @@ -5033,7 +4950,7 @@ export class InteractiveMode { }); } - private showModelSelector(initialSearchInput?: string, options: { persist?: boolean } = {}): void { + private showModelSelector(initialSearchInput?: string): void { this.showSelector((done) => { const selectModel = async (model: Model, persist: boolean) => { try { @@ -5054,7 +4971,7 @@ export class InteractiveMode { this.session.model, this.session.modelRuntime, this.session.scopedModels, - (model) => selectModel(model, options.persist === true), + (model) => selectModel(model, false), () => { done(); this.ui.requestRender(); diff --git a/packages/coding-agent/test/interactive-mode-status.test.ts b/packages/coding-agent/test/interactive-mode-status.test.ts index 52729e85d..a4aaa48c0 100644 --- a/packages/coding-agent/test/interactive-mode-status.test.ts +++ b/packages/coding-agent/test/interactive-mode-status.test.ts @@ -8,7 +8,7 @@ import { VirtualTerminal } from "../../tui/test/virtual-terminal.ts"; import type { AutocompleteProviderFactory } from "../src/core/extensions/types.ts"; import type { SourceInfo } from "../src/core/source-info.ts"; import type { AuthSelectorProvider } from "../src/modes/interactive/components/oauth-selector.ts"; -import { InteractiveMode, parseDefaultFlagArgs } from "../src/modes/interactive/interactive-mode.ts"; +import { InteractiveMode } from "../src/modes/interactive/interactive-mode.ts"; import { initTheme } from "../src/modes/interactive/theme/theme.ts"; function renderLastLine(container: Container, width = 120): string { @@ -401,22 +401,6 @@ describe("InteractiveMode.setupAutocompleteProvider", () => { }); describe("InteractiveMode.createBaseAutocompleteProvider", () => { - describe("parseDefaultFlagArgs", () => { - test("parses --default as a persistent model selection", () => { - expect(parseDefaultFlagArgs("model", "--default openai/gpt-5")).toEqual({ - persist: true, - searchTerm: "openai/gpt-5", - }); - }); - - test("rejects unknown /model flags", () => { - expect(parseDefaultFlagArgs("model", "--global openai/gpt-5")).toMatchObject({ - persist: false, - error: 'Unknown /model option "--global". Supported option: --default.', - }); - }); - }); - test("matches model command arguments across provider/model order", async () => { type TestModel = { id: string; provider: string; name: string }; type FakeInteractiveMode = { @@ -468,52 +452,6 @@ describe("InteractiveMode.createBaseAutocompleteProvider", () => { ]); }); - test("preserves --default when completing model command arguments", async () => { - type TestModel = { id: string; provider: string; name: string }; - type FakeInteractiveMode = { - session: { - scopedModels: Array<{ model: TestModel }>; - modelRuntime: { getAvailableSnapshot: () => TestModel[] }; - promptTemplates: []; - extensionRunner: { getRegisteredCommands: () => [] }; - resourceLoader: { getSkills: () => { skills: [] } }; - }; - settingsManager: { getEnableSkillCommands: () => boolean }; - skillCommands: Map; - sessionManager: { getCwd: () => string }; - fdPath: null; - }; - - const createBaseAutocompleteProvider = ( - InteractiveMode as unknown as { - prototype: { createBaseAutocompleteProvider(this: FakeInteractiveMode): AutocompleteProvider }; - } - ).prototype.createBaseAutocompleteProvider; - const models = [{ id: "gpt-5.5", provider: "openai-codex", name: "GPT-5.5" }]; - const fakeThis: FakeInteractiveMode = { - session: { - scopedModels: [], - modelRuntime: { getAvailableSnapshot: () => models }, - promptTemplates: [], - extensionRunner: { getRegisteredCommands: () => [] }, - resourceLoader: { getSkills: () => ({ skills: [] }) }, - }, - settingsManager: { getEnableSkillCommands: () => false }, - skillCommands: new Map(), - sessionManager: { getCwd: () => "/tmp" }, - fdPath: null, - }; - - const provider = createBaseAutocompleteProvider.call(fakeThis); - const line = "/model --default codex"; - const suggestions = await provider.getSuggestions([line], 0, line.length, { - signal: new AbortController().signal, - }); - - expect(suggestions?.prefix).toBe("--default codex"); - expect(suggestions?.items[0]?.value).toBe("--default openai-codex/gpt-5.5"); - }); - test("matches login command arguments by provider id and name", async () => { type FakeInteractiveMode = { session: { diff --git a/packages/coding-agent/test/suite/agent-session-model-extension.test.ts b/packages/coding-agent/test/suite/agent-session-model-extension.test.ts index 487f55e0c..f024a67d1 100644 --- a/packages/coding-agent/test/suite/agent-session-model-extension.test.ts +++ b/packages/coding-agent/test/suite/agent-session-model-extension.test.ts @@ -110,6 +110,7 @@ describe("AgentSession model and extension characterization", () => { { id: "faux-1", name: "One", reasoning: true }, { id: "faux-2", name: "Two", reasoning: true }, ], + settings: { defaultThinkingLevel: "medium" }, }); harnesses.push(harness); @@ -125,10 +126,24 @@ describe("AgentSession model and extension characterization", () => { await harness.session.setModel(model2); expect(harness.session.thinkingLevel).toBe("low"); - // Switch back to faux-1 → no per-model override, carries session level + // Switch back to faux-1 → no per-model override, uses global default const model1 = harness.getModel("faux-1")!; await harness.session.setModel(model1); - expect(harness.session.thinkingLevel).toBe("low"); + expect(harness.session.thinkingLevel).toBe("medium"); + }); + + it("falls back to current session thinking level when no per-model or global default is configured", async () => { + const harness = await createHarness({ + models: [ + { id: "faux-1", name: "One", reasoning: true }, + { id: "faux-2", name: "Two", reasoning: true }, + ], + }); + harnesses.push(harness); + + harness.session.setThinkingLevel("high"); + await harness.session.setModel(harness.getModel("faux-2")!); + expect(harness.session.thinkingLevel).toBe("high"); }); it("per-model override takes priority over global default during model switch", async () => {