From 16f40de70a59a07068e0cbbc615f8fe5e7cfebbd Mon Sep 17 00:00:00 2001 From: Zero Liu Date: Tue, 26 May 2026 10:34:09 -0700 Subject: [PATCH] Render model descriptions --- designdocs/todo/AGENT_MODE_TODOS.md | 3 + src/agentMode/agentModelDiscovery.test.ts | 28 ++++ src/agentMode/agentModelDiscovery.ts | 64 +++++++-- src/agentMode/backends/claude/descriptor.ts | 1 + src/agentMode/backends/codex/descriptor.ts | 11 ++ .../sdk/ClaudeSdkBackendProcess.test.ts | 10 +- src/agentMode/sdk/ClaudeSdkBackendProcess.ts | 6 +- src/agentMode/session/descriptor.ts | 21 +++ .../session/translateBackendState.test.ts | 46 ++++++- .../session/translateBackendState.ts | 16 ++- src/agentMode/ui/ModelEnableList.tsx | 4 +- .../ui/agentModelPickerHelpers.test.ts | 58 ++++++++ src/agentMode/ui/agentModelPickerHelpers.ts | 13 +- src/components/ui/ModelEffortPicker.tsx | 9 +- src/components/ui/ModelSelector.tsx | 7 + .../setup/AgentSetupApi.test.ts | 70 ++++++++++ src/modelManagement/setup/AgentSetupApi.ts | 125 ++++++++++++++---- src/modelManagement/types/catalog.ts | 7 + .../configuredModelGrouping.test.ts | 46 ++++++- .../v2/components/configuredModelGrouping.ts | 17 ++- 20 files changed, 499 insertions(+), 63 deletions(-) diff --git a/designdocs/todo/AGENT_MODE_TODOS.md b/designdocs/todo/AGENT_MODE_TODOS.md index 5c2e2cb4..95cb6a05 100644 --- a/designdocs/todo/AGENT_MODE_TODOS.md +++ b/designdocs/todo/AGENT_MODE_TODOS.md @@ -18,10 +18,13 @@ - [x] Redesign the model settings - discussed on May 7 group meeting - [x] support self host model - [x] remove built-in models + - [ ] migrate existing models to the new format - [ ] P0: Test opencode works well with local models +- [ ] P0: Test migration (skill, model) to make sure no unrecoverable migration is introduced in pre-release - [ ] P0: 非migrate的skill最好能直接引用,同名的skill如果内容一样migrate可以做得更好 - [x] P0: Auto-save chat history controls - [x] P0: Support image context +- [ ] P1: Better binary detection - [ ] P1: MCP - Basic functionality is ready - [ ] P1: Surface externally-managed MCP servers (claude.ai remote, plugin-provided) — see [MCP_EXTERNALLY_MANAGED_SERVERS.md](./MCP_EXTERNALLY_MANAGED_SERVERS.md) diff --git a/src/agentMode/agentModelDiscovery.test.ts b/src/agentMode/agentModelDiscovery.test.ts index 504958bd..efc9ef31 100644 --- a/src/agentMode/agentModelDiscovery.test.ts +++ b/src/agentMode/agentModelDiscovery.test.ts @@ -223,6 +223,32 @@ describe("wireAgentModelDiscovery", () => { unsub(); }); + it("omits a reported empty name from fallbackDisplayNames (never overwrites with '')", async () => { + mockDescriptors = [makeDescriptor({ id: "codex" })]; + const m = makeManagerFake(); + const a = makeApiFake(); + m.setState("codex", { + model: { + current: { baseModelId: "gpt-5", effort: null }, + availableModels: [ + { baseModelId: "gpt-5", name: "GPT-5", provider: null, effortOptions: [] }, + { baseModelId: "blank", name: "", provider: null, effortOptions: [] }, + ], + }, + mode: null, + }); + + const unsub = wireAgentModelDiscovery(makePlugin(a.api), m.manager); + await flush(); + + // "blank" is still enrolled (it's a real wire id) but contributes no + // display-name fallback, so resolution falls back to catalog/id instead. + expect(a.registerAgentProvider.mock.calls[0][0].fallbackDisplayNames).toEqual({ + "gpt-5": "GPT-5", + }); + unsub(); + }); + it("seeds enabledModels to the agent's current model on first enrollment", async () => { mockDescriptors = [makeDescriptor({ id: "codex" })]; const m = makeManagerFake(); @@ -297,6 +323,8 @@ describe("wireAgentModelDiscovery", () => { expect(a.syncAgentModels.mock.calls[0][0]).toEqual({ agentType: "codex", wireModelIds: ["gpt-5", "gpt-5.5"], + fallbackDisplayNames: { "gpt-5": "gpt-5", "gpt-5.5": "gpt-5.5" }, + fallbackDescriptions: {}, }); // Seeding never re-runs on the sync branch. expect(a.setEnabledModels).not.toHaveBeenCalled(); diff --git a/src/agentMode/agentModelDiscovery.ts b/src/agentMode/agentModelDiscovery.ts index 313bc198..61a88ec4 100644 --- a/src/agentMode/agentModelDiscovery.ts +++ b/src/agentMode/agentModelDiscovery.ts @@ -59,10 +59,14 @@ export function wireAgentModelDiscovery( const runForBackend = (descriptor: BackendDescriptor): void => { const state = manager.getCachedBackendState(descriptor.id); - const reported = reportedWireIds(state); + const reported = reportedModels(state); if (reported === null) return; // No model state yet — agent hasn't settled. - const signature = reported.join("\n"); + // Include the display strings in the signature so a CLI upgrade that + // renames a model (or rewrites its blurb) re-syncs the persisted info. + const signature = reported + .map((r) => `${r.wireId}\t${r.name}\t${r.description ?? ""}`) + .join("\n"); if (lastEnrolled.get(descriptor.id) === signature) return; // Unchanged — no-op. // The model the agent currently has selected — the one first enrollment @@ -117,13 +121,27 @@ export function wireAgentModelDiscovery( async function enrollBackend( api: ModelManagementApi, descriptor: BackendDescriptor, - reported: readonly string[], + reported: readonly ReportedModel[], currentWireId: string | undefined ): Promise { // A backend's id doubles as its model-management AgentType. const agentType = descriptor.id as AgentType; + const reportedWireIds = reported.map((r) => r.wireId); const wireModelIds = - descriptor.id === "opencode" ? suppressManagedOpencode(api, reported) : reported; + descriptor.id === "opencode" ? suppressManagedOpencode(api, reportedWireIds) : reportedWireIds; + + // Agent-reported display strings, keyed by wire id. Passed as fallbacks so + // `ConfiguredModel.info.displayName`/`.description` match the chat picker's + // `ModelEntry` exactly — the agent owns these for agent-origin models. + const fallbackDisplayNames: Record = {}; + const fallbackDescriptions: Record = {}; + for (const r of reported) { + // Both guarded by truthiness so a backend that reports an empty name never + // overwrites a good catalog/existing displayName with "" (which would strand + // the row at its raw configuredModelId in the enable list). + if (r.name) fallbackDisplayNames[r.wireId] = r.name; + if (r.description) fallbackDescriptions[r.wireId] = r.description; + } // An empty list means a transient/degraded probe (zero models settled, or — // for opencode — every model was suppressed as Copilot-managed), NOT "the @@ -142,7 +160,12 @@ async function enrollBackend( .find((p) => p.origin.kind === "agent" && p.origin.agentType === agentType); if (existing) { - await api.setup.agent.syncAgentModels({ agentType, wireModelIds }); + await api.setup.agent.syncAgentModels({ + agentType, + wireModelIds, + fallbackDisplayNames, + fallbackDescriptions, + }); return; } @@ -156,6 +179,8 @@ async function enrollBackend( // own models, so the keychain id stays null. apiKey: null, wireModelIds, + fallbackDisplayNames, + fallbackDescriptions, }); // `configuredModelIds` come back in `wireModelIds` order, so zip them to @@ -210,12 +235,25 @@ export function buildManagedOpencodeProviderIds( return managed; } -/** - * The reported `baseModelId`s from a cached `BackendState`, or `null` when the - * backend hasn't reported a model state yet — distinct from an empty array (a - * settled state with zero models), which callers treat differently. - */ -function reportedWireIds(state: BackendState | null): string[] | null { - if (!state?.model) return null; - return state.model.availableModels.map((m) => m.baseModelId); +/** One agent-reported model: its wire id plus the display strings to persist. */ +interface ReportedModel { + wireId: string; + name: string; + description?: string; +} + +/** + * The reported models from a cached `BackendState`, or `null` when the backend + * hasn't reported a model state yet — distinct from an empty array (a settled + * state with zero models), which callers treat differently. Carries each + * model's translated `name`/`description` so enrollment persists the same + * strings the chat picker shows. + */ +function reportedModels(state: BackendState | null): ReportedModel[] | null { + if (!state?.model) return null; + return state.model.availableModels.map((m) => ({ + wireId: m.baseModelId, + name: m.name, + description: m.description, + })); } diff --git a/src/agentMode/backends/claude/descriptor.ts b/src/agentMode/backends/claude/descriptor.ts index 5034be35..bac8ba74 100644 --- a/src/agentMode/backends/claude/descriptor.ts +++ b/src/agentMode/backends/claude/descriptor.ts @@ -133,6 +133,7 @@ export const ClaudeBackendDescriptor: BackendDescriptor = { crossDiscoveredAgents: [], restartOnManagedSkillsChange: false, wire: claudeWire, + showModelDescriptions: true, getEnabledBaseModelIds(settings: CopilotSettings): ReadonlySet { // All Claude Code models are agent-origin. diff --git a/src/agentMode/backends/codex/descriptor.ts b/src/agentMode/backends/codex/descriptor.ts index 6ce506bb..66060c8f 100644 --- a/src/agentMode/backends/codex/descriptor.ts +++ b/src/agentMode/backends/codex/descriptor.ts @@ -75,12 +75,23 @@ export const CodexBackendDescriptor: BackendDescriptor = { crossDiscoveredAgents: [], restartOnManagedSkillsChange: false, wire: codexWire, + showModelDescriptions: true, getEnabledBaseModelIds(settings: CopilotSettings): ReadonlySet { // All Codex models are agent-origin. return agentOriginEnabledWireIds(settings, "codex", (wireId) => codexWire.decode(wireId)); }, + /** + * codex-acp reports inconsistently-cased names (`GPT-5.5` but also + * `gpt-5.4`, `gpt-5.3-codex`). Uppercase only the anchored `gpt` prefix so + * the column reads consistently — no family/token guessing, so the wire + * ids and any mid-string tokens are left untouched. + */ + normalizeModelName(name: string): string { + return name.replace(/^gpt/i, "GPT"); + }, + getInstallState(settings: CopilotSettings): InstallState { return binaryPathInstallState(settings.agentMode?.backends?.codex?.binaryPath); }, diff --git a/src/agentMode/sdk/ClaudeSdkBackendProcess.test.ts b/src/agentMode/sdk/ClaudeSdkBackendProcess.test.ts index a040726d..03eb3f5a 100644 --- a/src/agentMode/sdk/ClaudeSdkBackendProcess.test.ts +++ b/src/agentMode/sdk/ClaudeSdkBackendProcess.test.ts @@ -62,6 +62,7 @@ function fakeDescriptor(): BackendDescriptor { return { id: "claude", displayName: "Claude", + showModelDescriptions: true, wire: { encode: (sel: { baseModelId: string; effort: string | null }) => sel.baseModelId, decode: (id: string) => ({ @@ -413,10 +414,11 @@ describe("ClaudeSdkBackendProcess.newSession dynamic catalog", () => { const ids = resp.state.model?.availableModels.map((m) => m.baseModelId); expect(ids).toContain("claude-fake-pro"); expect(ids).toContain("claude-fake-mini"); - const proEffort = resp.state.model?.availableModels - .find((m) => m.baseModelId === "claude-fake-pro") - ?.effortOptions.map((o) => o.value); - expect(proEffort).toEqual(["low", "medium", "high"]); + const pro = resp.state.model?.availableModels.find((m) => m.baseModelId === "claude-fake-pro"); + expect(pro?.effortOptions.map((o) => o.value)).toEqual(["low", "medium", "high"]); + // The SDK's per-model `description` is carried into the entry (used as the + // capability second line in the picker + settings). + expect(pro?.description).toBe("test"); }); it("honors persisted default model when it appears in the catalog", async () => { diff --git a/src/agentMode/sdk/ClaudeSdkBackendProcess.ts b/src/agentMode/sdk/ClaudeSdkBackendProcess.ts index 1e8d55d7..86cd464c 100644 --- a/src/agentMode/sdk/ClaudeSdkBackendProcess.ts +++ b/src/agentMode/sdk/ClaudeSdkBackendProcess.ts @@ -571,7 +571,11 @@ export class ClaudeSdkBackendProcess implements BackendProcess { catalog.length > 0 && seedModel ? { currentModelId: seedModel, - availableModels: catalog.map((m) => ({ modelId: m.value, name: m.displayName })), + availableModels: catalog.map((m) => ({ + modelId: m.value, + name: m.displayName, + description: m.description, + })), } : null; const modes: RawModeState = { diff --git a/src/agentMode/session/descriptor.ts b/src/agentMode/session/descriptor.ts index f3d079cc..197f5f3e 100644 --- a/src/agentMode/session/descriptor.ts +++ b/src/agentMode/session/descriptor.ts @@ -114,6 +114,27 @@ export interface BackendDescriptor { */ readonly wire: ModelWireCodec; + /** + * Optional: normalize a backend-reported model display name before it + * becomes the canonical `ModelEntry.name`. Applied by the translator at + * the single point that builds the name, so every downstream consumer + * (chat picker and settings enrollment alike) inherits the same string. + * + * Keep transforms robust and anchored — no free-text parsing. Codex uses + * it to uppercase the inconsistently-cased `gpt` prefix that codex-acp + * reports (`gpt-5.4` → `GPT-5.4`); most backends omit it. + */ + normalizeModelName?(name: string): string; + + /** + * Opt in to surfacing this backend's per-model `description` as the row + * subtitle in the chat picker and the settings enable list. Set for backends + * whose catalog is small and curated with meaningful blurbs (claude, codex); + * left off for flooding catalogs (opencode) where the line is just noise. + * BYOK/Plus models have no description, so they never show one regardless. + */ + readonly showModelDescriptions?: boolean; + /** * Apply a (baseModelId, effort) selection to a live session. The descriptor * decides whether effort travels in the wire model id (suffix-style diff --git a/src/agentMode/session/translateBackendState.test.ts b/src/agentMode/session/translateBackendState.test.ts index 272beeab..73c434d2 100644 --- a/src/agentMode/session/translateBackendState.test.ts +++ b/src/agentMode/session/translateBackendState.test.ts @@ -86,6 +86,47 @@ describe("translateBackendState — model: null cases", () => { }); }); +describe("translateBackendState — name normalization + description", () => { + it("passes the backend-reported description through when the backend opts in", () => { + const models: RawModelState = { + currentModelId: "m", + availableModels: [{ modelId: "m", name: "M", description: "Opus 4.7 with 1M context" }], + }; + const desc = descriptor({ showModelDescriptions: true }); + const state = translateBackendState({ models, modes: null, configOptions: null }, desc); + expect(findModelEntry(state.model, "m")?.description).toBe("Opus 4.7 with 1M context"); + }); + + it("drops the description when the backend does not opt in (e.g. opencode)", () => { + const models: RawModelState = { + currentModelId: "m", + availableModels: [{ modelId: "m", name: "M", description: "noisy blurb" }], + }; + const state = translateBackendState({ models, modes: null, configOptions: null }, descriptor()); + expect(findModelEntry(state.model, "m")?.description).toBeUndefined(); + }); + + it("applies descriptor.normalizeModelName to the entry name", () => { + const models: RawModelState = { + currentModelId: "gpt-5.4", + availableModels: [{ modelId: "gpt-5.4", name: "gpt-5.4" }], + }; + const desc = descriptor({ normalizeModelName: (n: string) => n.replace(/^gpt/i, "GPT") }); + const state = translateBackendState({ models, modes: null, configOptions: null }, desc); + expect(findModelEntry(state.model, "gpt-5.4")?.name).toBe("GPT-5.4"); + }); + + it("normalizes the synthesized current entry's name (current not in catalog)", () => { + const models: RawModelState = { + currentModelId: "gpt-x", + availableModels: [{ modelId: "other", name: "Other" }], + }; + const desc = descriptor({ normalizeModelName: (n: string) => n.replace(/^gpt/i, "GPT") }); + const state = translateBackendState({ models, modes: null, configOptions: null }, desc); + expect(findModelEntry(state.model, "gpt-x")?.name).toBe("GPT-x"); + }); +}); + describe("translateBackendState — suffix-style backends", () => { it("collapses gpt-5 + variants into one entry with effort options (case 2)", () => { const models: RawModelState = { @@ -364,7 +405,7 @@ describe("translateBackendState — provider/parsing edge cases", () => { expect(state.model!.current.effort).toBeNull(); }); - it("description present / absent round-trips (case 14)", () => { + it("description present / absent round-trips for opted-in backends (case 14)", () => { const models: RawModelState = { currentModelId: "claude-sonnet", availableModels: [ @@ -372,7 +413,8 @@ describe("translateBackendState — provider/parsing edge cases", () => { { modelId: "claude-haiku", name: "Haiku" }, ], }; - const state = translateBackendState({ models, modes: null, configOptions: null }, descriptor()); + const desc = descriptor({ showModelDescriptions: true }); + const state = translateBackendState({ models, modes: null, configOptions: null }, desc); expect(state.model!.availableModels[0].description).toBe("Smart and balanced"); expect(state.model!.availableModels[1].description).toBeUndefined(); }); diff --git a/src/agentMode/session/translateBackendState.ts b/src/agentMode/session/translateBackendState.ts index 71558206..04938d6d 100644 --- a/src/agentMode/session/translateBackendState.ts +++ b/src/agentMode/session/translateBackendState.ts @@ -111,8 +111,13 @@ function translateModel( // becomes redundant. The recognized vocabulary comes from the // variants themselves (decoded by the descriptor's wire codec), // so backends own their effort tokens — we don't duplicate them. - name: g.variants.length >= 2 ? stripEffortSuffix(g.name, g.variants) : g.name, - description: g.description, + name: normalizeName( + g.variants.length >= 2 ? stripEffortSuffix(g.name, g.variants) : g.name, + descriptor + ), + // Only backends that opt in surface their per-model blurb; others (opencode) + // would just add noisy/duplicative lines, so the field is dropped here. + description: descriptor.showModelDescriptions ? g.description : undefined, provider: g.provider, effortOptions: deriveEffortOptions(g, descriptor), })); @@ -130,7 +135,7 @@ function translateModel( : []; currentEntry = { baseModelId: currentBaseId, - name: currentBaseId, + name: normalizeName(currentBaseId, descriptor), provider: decodedCurrent.provider, effortOptions: synthEffortOptions, }; @@ -296,6 +301,11 @@ function reverseProjectMode( return null; } +/** Apply the descriptor's optional display-name normalization, if any. */ +function normalizeName(name: string, descriptor: BackendDescriptor): string { + return descriptor.normalizeModelName?.(name) ?? name; +} + function stripEffortSuffix(name: string, variants: { effort: string | null }[]): string { const m = name.match(/^(.*?)\s*\(([^()]+)\)\s*$/); if (!m) return name; diff --git a/src/agentMode/ui/ModelEnableList.tsx b/src/agentMode/ui/ModelEnableList.tsx index b42b0a06..8742b33e 100644 --- a/src/agentMode/ui/ModelEnableList.tsx +++ b/src/agentMode/ui/ModelEnableList.tsx @@ -9,8 +9,10 @@ export interface ModelEnableRow { id: string; /** Primary label shown to the user. */ label: string; - /** Optional secondary line (e.g. wire id / description). */ + /** Optional secondary line — the model's capability blurb. */ description?: string; + /** Wire id, matched by search but never rendered (it duplicates the label). */ + wireId?: string; /** Whether the model is currently enabled. */ enabled: boolean; } diff --git a/src/agentMode/ui/agentModelPickerHelpers.test.ts b/src/agentMode/ui/agentModelPickerHelpers.test.ts index 6b3de81e..d1b3acc2 100644 --- a/src/agentMode/ui/agentModelPickerHelpers.test.ts +++ b/src/agentMode/ui/agentModelPickerHelpers.test.ts @@ -283,6 +283,64 @@ describe("buildPickerEntries", () => { expect(entries.map((e) => e.name)).toEqual(["kept-model"]); }); + it("carries the model description onto the picker entry as _subtitle", () => { + const entry: ModelEntry = { + baseModelId: "gpt-5", + name: "GPT-5", + description: "Frontier model for complex coding", + provider: null, + effortOptions: [], + }; + const codex = { + ...makeDescriptor("codex"), + getEnabledBaseModelIds: () => new Set(["gpt-5"]), + } as unknown as BackendDescriptor; + const manager = makeManager({ + cachedStateById: { + codex: { model: makeModelState("gpt-5", [entry]), mode: null }, + }, + }); + const ctx: ModelActiveContext = { + activeSession: { backendId: "codex" } as unknown as AgentSession, + activeChatUIState: null, + activeBackendId: "codex", + activeDescriptor: codex, + activeSessionHasHistory: false, + activeModelState: makeModelState("gpt-5", [entry]), + activeCurrentEntry: entry, + }; + const { entries } = buildPickerEntries(manager, [codex], ctx, emptySettings); + expect(entries[0]._subtitle).toBe("Frontier model for complex coding"); + }); + + it("carries the description onto a synthesized stranded entry", () => { + const stranded: ModelEntry = { + baseModelId: "ghost", + name: "Ghost", + description: "Opus 4.7 with 1M context", + provider: null, + effortOptions: [], + }; + const visible = makeModelEntry("real-model"); + const codex = makeDescriptor("codex"); + const manager = makeManager({ + cachedStateById: { + codex: { model: makeModelState("real-model", [visible]), mode: null }, + }, + }); + const ctx: ModelActiveContext = { + activeSession: { backendId: "codex" } as unknown as AgentSession, + activeChatUIState: null, + activeBackendId: "codex", + activeDescriptor: codex, + activeSessionHasHistory: false, + activeModelState: makeModelState("ghost", [stranded]), + activeCurrentEntry: stranded, + }; + const { entries } = buildPickerEntries(manager, [codex], ctx, emptySettings); + expect(entries[0]._subtitle).toBe("Opus 4.7 with 1M context"); + }); + it("keeps the sticky active model even when getEnabledBaseModelIds excludes it", () => { // The active (sticky) model is no longer in the enabled set, but // keepBaseModelId must preserve it so curation never strands the diff --git a/src/agentMode/ui/agentModelPickerHelpers.ts b/src/agentMode/ui/agentModelPickerHelpers.ts index 254a4825..e980a7dc 100644 --- a/src/agentMode/ui/agentModelPickerHelpers.ts +++ b/src/agentMode/ui/agentModelPickerHelpers.ts @@ -77,14 +77,15 @@ export function appendBackendSection( enabledSet?.has(entry.baseModelId) === true || entry.baseModelId === ctx.keepBaseModelId ); for (const m of filtered) { - entries.push(synthesizeAgentEntry(m.baseModelId, m.name, descriptor)); + entries.push(synthesizeAgentEntry(m.baseModelId, m.name, descriptor, m.description)); } } export function synthesizeAgentEntry( baseModelId: string, humanName: string, - descriptor: BackendDescriptor + descriptor: BackendDescriptor, + subtitle?: string ): ModelSelectorEntry { return { name: baseModelId, @@ -94,6 +95,7 @@ export function synthesizeAgentEntry( displayName: humanName || baseModelId, _group: descriptor.displayName, _backendId: descriptor.id, + _subtitle: subtitle, }; } @@ -217,7 +219,12 @@ export function buildPickerEntries( if (match) { valueKey = getModelKeyFromModel(match); } else { - const synth = synthesizeAgentEntry(baseId, ctx.activeCurrentEntry.name, ctx.activeDescriptor); + const synth = synthesizeAgentEntry( + baseId, + ctx.activeCurrentEntry.name, + ctx.activeDescriptor, + ctx.activeCurrentEntry.description + ); entries.unshift(synth); valueKey = getModelKeyFromModel(synth); } diff --git a/src/components/ui/ModelEffortPicker.tsx b/src/components/ui/ModelEffortPicker.tsx index 1885598a..e953bec4 100644 --- a/src/components/ui/ModelEffortPicker.tsx +++ b/src/components/ui/ModelEffortPicker.tsx @@ -268,7 +268,14 @@ export function ModelEffortPicker({ override, className }: ModelEffortPickerProp {isActive ? "✓" : isHighlight ? "›" : ""} - +
+ + {entry._subtitle && ( +
+ {entry._subtitle} +
+ )} +
{rightLabel && ( {rightLabel} diff --git a/src/components/ui/ModelSelector.tsx b/src/components/ui/ModelSelector.tsx index 8d372765..d4a00ec3 100644 --- a/src/components/ui/ModelSelector.tsx +++ b/src/components/ui/ModelSelector.tsx @@ -38,6 +38,13 @@ export type ModelSelectorEntry = CustomModel & { _disabledReason?: string; _group?: string; _backendId?: string; + /** + * Optional second line rendered beneath the title in the dropdown row + * (agent backends only). Carries the model's capability blurb so the + * picker row matches the settings list row. The collapsed trigger pill + * ignores it and stays single-line. + */ + _subtitle?: string; }; interface ModelSelectorProps { diff --git a/src/modelManagement/setup/AgentSetupApi.test.ts b/src/modelManagement/setup/AgentSetupApi.test.ts index 4df1b719..65e5a8d2 100644 --- a/src/modelManagement/setup/AgentSetupApi.test.ts +++ b/src/modelManagement/setup/AgentSetupApi.test.ts @@ -88,6 +88,12 @@ class FakeConfiguredModelRegistry { return configuredModelId; }); + update = jest.fn(async (configuredModelId: string, patch: { info?: Partial }) => { + const row = this.rows.find((m) => m.configuredModelId === configuredModelId); + if (!row) throw new Error(`unknown ${configuredModelId}`); + if (patch.info) row.info = { ...row.info, ...patch.info }; + }); + remove = jest.fn(async (configuredModelId: string) => { this.rows = this.rows.filter((m) => m.configuredModelId !== configuredModelId); }); @@ -251,6 +257,70 @@ describe("AgentSetupApi.registerAgentProvider", () => { expect(h.backends.enabledFor("codex")).toEqual([]); }); + it("lets the agent-reported name + description win over catalog metadata", async () => { + const h = makeHarness(); + // Catalog knows "claude-sonnet-4-5" as "Claude Sonnet 4.5"; the agent's + // own name/description must override so settings match the chat picker. + const result = await h.api.registerAgentProvider({ + agentType: "claude", + providerType: "anthropic", + displayName: "Claude Code", + apiKey: null, + wireModelIds: ["claude-sonnet-4-5"], + fallbackDisplayNames: { "claude-sonnet-4-5": "Sonnet" }, + fallbackDescriptions: { "claude-sonnet-4-5": "Sonnet 4.6 · Best for everyday tasks" }, + }); + const info = h.models.listByProvider(result.providerId)[0].info; + expect(info.displayName).toBe("Sonnet"); + expect(info.description).toBe("Sonnet 4.6 · Best for everyday tasks"); + }); + + it("uses the fallback name + description for wire ids the catalog doesn't know", async () => { + const h = makeHarness(); + const result = await h.api.registerAgentProvider({ + agentType: "claude", + providerType: "anthropic", + displayName: "Claude Code", + apiKey: null, + wireModelIds: ["default"], + fallbackDisplayNames: { default: "Default (recommended)" }, + fallbackDescriptions: { default: "Opus 4.7 with 1M context · Most capable for complex work" }, + }); + const info = h.models.listByProvider(result.providerId)[0].info; + expect(info.id).toBe("default"); + expect(info.displayName).toBe("Default (recommended)"); + expect(info.description).toBe("Opus 4.7 with 1M context · Most capable for complex work"); + }); + + it("refreshes an existing model's display strings on re-register, preserving its id", async () => { + const h = makeHarness(); + const first = await h.api.registerAgentProvider({ + agentType: "claude", + providerType: "anthropic", + displayName: "Claude Code", + apiKey: null, + wireModelIds: ["default"], + fallbackDisplayNames: { default: "default" }, // stale, pre-feature label + }); + const idBefore = h.models.listByProvider(first.providerId)[0].configuredModelId; + + await h.api.registerAgentProvider({ + agentType: "claude", + providerType: "anthropic", + displayName: "Claude Code", + apiKey: null, + wireModelIds: ["default"], + fallbackDisplayNames: { default: "Default (recommended)" }, + fallbackDescriptions: { default: "Opus 4.7 with 1M context · Most capable for complex work" }, + }); + + const row = h.models.listByProvider(first.providerId)[0]; + // Same row (enabled-set refs don't churn), refreshed strings. + expect(row.configuredModelId).toBe(idBefore); + expect(row.info.displayName).toBe("Default (recommended)"); + expect(row.info.description).toBe("Opus 4.7 with 1M context · Most capable for complex work"); + }); + it("is idempotent on (agentType, providerType): re-running updates in place, no duplicate provider", async () => { const h = makeHarness(); const first = await h.api.registerAgentProvider({ diff --git a/src/modelManagement/setup/AgentSetupApi.ts b/src/modelManagement/setup/AgentSetupApi.ts index 52a4aec0..522210be 100644 --- a/src/modelManagement/setup/AgentSetupApi.ts +++ b/src/modelManagement/setup/AgentSetupApi.ts @@ -33,13 +33,20 @@ export interface RegisterAgentProviderInput { /** Full set of wire ids the agent reports; the existing model set is diffed * against this list. */ wireModelIds: readonly string[]; - /** Display names for wire ids the catalog doesn't know yet. Keyed by wire id. */ + /** Agent-reported display names, keyed by wire id. For agent-origin + * providers these win over catalog metadata (the agent owns the name). */ fallbackDisplayNames?: Record; + /** Agent-reported capability blurbs, keyed by wire id. Win over catalog. */ + fallbackDescriptions?: Record; } export interface SyncAgentModelsInput { agentType: AgentType; wireModelIds: readonly string[]; + /** See `RegisterAgentProviderInput.fallbackDisplayNames`. */ + fallbackDisplayNames?: Record; + /** See `RegisterAgentProviderInput.fallbackDescriptions`. */ + fallbackDescriptions?: Record; } export interface AgentSetupResult { @@ -116,7 +123,8 @@ export class AgentSetupApi { const infos = await this.#resolveModelInfos( input.providerType, input.wireModelIds, - input.fallbackDisplayNames + input.fallbackDisplayNames, + input.fallbackDescriptions ); const { added, removed } = await this.#reconcileModels(input.agentType, providerId, infos); @@ -159,7 +167,12 @@ export class AgentSetupApi { if (providers.length === 1) { const provider = providers[0]; // Every reported wire id belongs to this one provider. - const infos = await this.#resolveModelInfosForProvider(provider, input.wireModelIds); + const infos = await this.#resolveModelInfosForProvider( + provider, + input.wireModelIds, + input.fallbackDisplayNames, + input.fallbackDescriptions + ); const { added, removed } = await this.#reconcileModels( input.agentType, provider.providerId, @@ -187,7 +200,12 @@ export class AgentSetupApi { // Reconcile only owned ids: a dropped id is one this provider owned and // the agent no longer reports. Unowned ids aren't added here (no // providerType to resolve their catalog metadata). - const infos = await this.#resolveModelInfosForProvider(provider, ownedWireIds); + const infos = await this.#resolveModelInfosForProvider( + provider, + ownedWireIds, + input.fallbackDisplayNames, + input.fallbackDescriptions + ); const { added, removed } = await this.#reconcileModels( input.agentType, provider.providerId, @@ -236,9 +254,11 @@ export class AgentSetupApi { } /** - * Catalog-enrich each wire id under one `providerType`, falling back to - * `fallbackDisplayNames[id] ?? id` on a miss (so correctness never depends on - * the catalog being reachable). + * Catalog-enrich each wire id under one `providerType`, then overlay the + * agent-reported `fallbackDisplayNames`/`fallbackDescriptions` so the agent + * owns the display strings (catalog `limits`/`cost` survive). On a catalog + * miss the id itself is the base, so correctness never depends on the catalog + * being reachable. * * The input carries only `providerType`, which maps to many catalog * providers, so the lookup scans them and takes the first match. A wire id @@ -248,28 +268,38 @@ export class AgentSetupApi { async #resolveModelInfos( providerType: ProviderType, wireModelIds: readonly string[], - fallbackDisplayNames?: Record + fallbackDisplayNames?: Record, + fallbackDescriptions?: Record ): Promise { const wireToInfo = await this.#buildCatalogLookup(providerType); - return this.#snapshotInfos(wireModelIds, wireToInfo, fallbackDisplayNames); + return this.#snapshotInfos( + wireModelIds, + wireToInfo, + fallbackDisplayNames, + fallbackDescriptions + ); } /** * Like `#resolveModelInfos` but scoped to one provider: on a catalog miss it * reuses the existing `ConfiguredModel.info`, so a re-sync never downgrades an - * already-enriched row to a bare fallback. + * already-enriched row to a bare fallback. Agent-reported display strings + * still override the resolved name/description (see `#applyAgentDisplay`). */ async #resolveModelInfosForProvider( provider: Provider, - wireModelIds: readonly string[] + wireModelIds: readonly string[], + fallbackDisplayNames?: Record, + fallbackDescriptions?: Record ): Promise { const wireToInfo = await this.#buildCatalogLookup(provider.providerType); return wireModelIds.map((wireId) => { - const fromCatalog = wireToInfo.get(wireId); - if (fromCatalog) return fromCatalog; - const existing = this.#models.getByWireId(provider.providerId, wireId); - if (existing) return existing.info; - return { id: wireId, displayName: wireId }; + const base = wireToInfo.get(wireId) ?? + this.#models.getByWireId(provider.providerId, wireId)?.info ?? { + id: wireId, + displayName: wireId, + }; + return this.#applyAgentDisplay(base, wireId, fallbackDisplayNames, fallbackDescriptions); }); } @@ -277,15 +307,38 @@ export class AgentSetupApi { #snapshotInfos( wireModelIds: readonly string[], wireToInfo: ReadonlyMap, - fallbackDisplayNames?: Record + fallbackDisplayNames?: Record, + fallbackDescriptions?: Record ): ModelInfo[] { return wireModelIds.map((wireId) => { - const fromCatalog = wireToInfo.get(wireId); - if (fromCatalog) return fromCatalog; - return { id: wireId, displayName: fallbackDisplayNames?.[wireId] ?? wireId }; + const base = wireToInfo.get(wireId) ?? { id: wireId, displayName: wireId }; + return this.#applyAgentDisplay(base, wireId, fallbackDisplayNames, fallbackDescriptions); }); } + /** + * Overlay the agent's reported name/description onto a resolved `ModelInfo`. + * For agent-origin models the agent owns these strings, so they win over any + * catalog match — this is what keeps `ConfiguredModel.info` byte-identical to + * the chat picker's `ModelEntry`. Catalog-supplied `limits`/`cost`/etc. are + * preserved. A missing fallback leaves the resolved value untouched. + */ + #applyAgentDisplay( + base: ModelInfo, + wireId: string, + fallbackDisplayNames?: Record, + fallbackDescriptions?: Record + ): ModelInfo { + const displayName = fallbackDisplayNames?.[wireId]; + const description = fallbackDescriptions?.[wireId]; + if (displayName === undefined && description === undefined) return base; + return { + ...base, + displayName: displayName ?? base.displayName, + description: description ?? base.description, + }; + } + /** * Build a `wireId → ModelInfo` map across every catalog provider whose * `providerType` matches. Best-effort: `ensureLoaded` failure is @@ -313,9 +366,10 @@ export class AgentSetupApi { /** * Diff-reconcile one provider's ConfiguredModel set against `infos`: add new * wire ids (auto-enrolling each into `backends[agentType]` only), - * cascade-remove vanished ones, leave unchanged ids untouched. Only real - * deltas write, so re-syncing an unchanged list is a no-op that never resets - * user curation. + * refresh the display strings of existing ids whose name/description changed + * (so a CLI upgrade or this feature's rollout updates already-enrolled rows), + * and cascade-remove vanished ones. Only real deltas write, so re-syncing an + * unchanged list is a no-op that never resets user curation. */ async #reconcileModels( agentType: AgentType, @@ -331,12 +385,25 @@ export class AgentSetupApi { const added: Array<{ wireId: string; configuredModelId: string }> = []; for (const info of infos) { - if (existingByWireId.has(info.id)) continue; - const configuredModelId = await this.#models.add({ providerId, info }); - // Enroll into this agent's backend only — agent models never leak into - // chat or another agent's picker. - await this.#backends.enableModel(agentType, configuredModelId); - added.push({ wireId: info.id, configuredModelId }); + const current = existingByWireId.get(info.id); + if (!current) { + const configuredModelId = await this.#models.add({ providerId, info }); + // Enroll into this agent's backend only — agent models never leak into + // chat or another agent's picker. + await this.#backends.enableModel(agentType, configuredModelId); + added.push({ wireId: info.id, configuredModelId }); + continue; + } + // Refresh display strings in place when they drifted, without touching the + // configuredModelId (so `BackendConfig.enabledModels` refs don't churn). + if ( + current.info.displayName !== info.displayName || + current.info.description !== info.description + ) { + await this.#models.update(current.configuredModelId, { + info: { displayName: info.displayName, description: info.description }, + }); + } } const removed: Array<{ wireId: string; configuredModelId: string }> = []; diff --git a/src/modelManagement/types/catalog.ts b/src/modelManagement/types/catalog.ts index 15570f3d..40322896 100644 --- a/src/modelManagement/types/catalog.ts +++ b/src/modelManagement/types/catalog.ts @@ -40,6 +40,13 @@ export interface ModelInfo { /** Wire-form id passed to the SDK ("claude-sonnet-4-5", "gpt-5", …). */ id: string; displayName: string; + /** + * Optional one-line capability blurb. `models.dev` never sends this + * (see `modelsDevWire.ts`), so it stays empty for catalog-derived models; + * agent backends populate it from their own reported metadata (e.g. the + * Claude SDK's "Opus 4.7 with 1M context · …" or codex-acp's model blurb). + */ + description?: string; /** * Every sub-field is optional so partial catalog coverage (e.g. a * model that publishes only an `input` limit, or `output`-only cost) diff --git a/src/settings/v2/components/configuredModelGrouping.test.ts b/src/settings/v2/components/configuredModelGrouping.test.ts index aefb492a..bed09790 100644 --- a/src/settings/v2/components/configuredModelGrouping.test.ts +++ b/src/settings/v2/components/configuredModelGrouping.test.ts @@ -2,6 +2,7 @@ import { buildModelEnableGroups, opencodeOnlySubGroupLabel, partitionCandidates, + rowMatches, toRow, type Candidate, } from "./configuredModelGrouping"; @@ -177,7 +178,7 @@ describe("opencodeOnlySubGroupLabel", () => { }); describe("toRow", () => { - it("surfaces the wire id as a description only when it differs from the label", () => { + it("never surfaces the wire id as the description (it duplicates the label)", () => { const provider = byokProvider("p", "Anthropic"); const withName: Candidate = { configuredModel: { @@ -191,7 +192,9 @@ describe("toRow", () => { }; const row = toRow(withName); expect(row.label).toBe("Claude Sonnet 4.5"); - expect(row.description).toBe("claude-sonnet-4-5"); + expect(row.description).toBeUndefined(); + // ...but the wire id is still carried for search (just not rendered). + expect(row.wireId).toBe("claude-sonnet-4-5"); const sameAsId: Candidate = { configuredModel: model("cm2", "p", "raw-id"), @@ -200,6 +203,45 @@ describe("toRow", () => { }; expect(toRow(sameAsId).description).toBeUndefined(); }); + + it("prefers the capability blurb over the wire id for the description line", () => { + const provider = agentProvider("claude", "claude", "Claude"); + const candidate: Candidate = { + configuredModel: { + configuredModelId: "cm", + providerId: "claude", + info: { + id: "default", + displayName: "Default (recommended)", + description: "Opus 4.7 with 1M context · Most capable for complex work", + }, + configuredAt: 0, + }, + provider, + enabled: true, + }; + const row = toRow(candidate); + expect(row.label).toBe("Default (recommended)"); + expect(row.description).toBe("Opus 4.7 with 1M context · Most capable for complex work"); + }); +}); + +describe("rowMatches", () => { + it("matches a model by its wire id even when the label differs", () => { + const row = toRow({ + configuredModel: { + configuredModelId: "cm", + providerId: "p", + info: { id: "claude-sonnet-4-5", displayName: "Claude Sonnet 4.5" }, + configuredAt: 0, + }, + provider: byokProvider("p", "Anthropic"), + enabled: true, + }); + expect(rowMatches(row, "claude-sonnet-4-5")).toBe(true); + expect(rowMatches(row, "sonnet")).toBe(true); // label still matches + expect(rowMatches(row, "gpt")).toBe(false); + }); }); describe("buildModelEnableGroups", () => { diff --git a/src/settings/v2/components/configuredModelGrouping.ts b/src/settings/v2/components/configuredModelGrouping.ts index e9d5d465..c22cdb7b 100644 --- a/src/settings/v2/components/configuredModelGrouping.ts +++ b/src/settings/v2/components/configuredModelGrouping.ts @@ -73,14 +73,22 @@ export function opencodeOnlySubGroupLabel(model: ConfiguredModel, provider: Prov return provider.displayName; } -/** A `ModelEnableRow` from a candidate, surfacing the wire id as a secondary line when it differs from the label. */ +/** + * A `ModelEnableRow` from a candidate. The secondary line is the model's + * capability blurb (`info.description`), which only the curated agent backends + * persist (claude, codex); BYOK/Plus and opencode carry none and so render a + * single line. We deliberately don't fall back to the wire id for display — it + * duplicates the label — but we still carry it in `wireId` so search keeps + * matching it. This keeps the row identical to the chat picker. + */ export function toRow(candidate: Candidate): ModelEnableRow { const { configuredModel, enabled } = candidate; - const label = configuredModel.info.displayName || configuredModel.info.id; + const { displayName, id, description } = configuredModel.info; return { id: configuredModel.configuredModelId, - label, - description: label === configuredModel.info.id ? undefined : configuredModel.info.id, + label: displayName || id, + description: description || undefined, + wireId: id, enabled, }; } @@ -91,6 +99,7 @@ export function rowMatches(row: ModelEnableRow, q: string): boolean { return ( row.label.toLowerCase().includes(q) || row.id.toLowerCase().includes(q) || + (row.wireId?.toLowerCase().includes(q) ?? false) || (row.description?.toLowerCase().includes(q) ?? false) ); }