From f1ef49e82befb1782a98ab5d375dfc82c28c7db7 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Tue, 4 Aug 2026 01:48:00 -0700 Subject: [PATCH] fix(native-chat): keep the Claude model picker rendered MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Claude picker rendered nothing until the per-host `list_models` probe returned, so it popped in ~1s after mount and never appeared at all when the probe failed โ€” an old CLI without `list_models`, no `claude` on PATH, or an older remote runtime whose response omits `catalogOrigin`. Restore the version-neutral family seed as the starting list; discovery still replaces it wholesale on success, so a host with a real catalog never shows an obsolete hardcoded row. Separately, the tracked model could fall outside the active list: the terminal header scrape yields family ids (`opus`) while a current CLI lists `opus[1m]` and no plain `opus`. That blanked the picker trigger and dropped the model's effort and fast-mode controls. Reconcile the tracked id into the active list once, so the snapshot, the appliers, and typed command recording all see a labelled, operable row for it. --- .../native-chat/NativeChatComposer.test.tsx | 55 +++++++++++++ .../native-chat-pty-session-options.test.ts | 80 ++++++++++++++++++- .../native-chat-pty-session-options.ts | 37 ++++++--- .../native-chat-session-option-snapshot.ts | 8 +- .../use-native-chat-session-options.ts | 15 ++-- 5 files changed, 172 insertions(+), 23 deletions(-) diff --git a/src/renderer/src/components/native-chat/NativeChatComposer.test.tsx b/src/renderer/src/components/native-chat/NativeChatComposer.test.tsx index bbe19223b0c..4eac2b5739c 100644 --- a/src/renderer/src/components/native-chat/NativeChatComposer.test.tsx +++ b/src/renderer/src/components/native-chat/NativeChatComposer.test.tsx @@ -8,6 +8,7 @@ import type { } from '../../../../shared/native-chat-session-options' import type * as nativeChatAgentProfiles from '../../../../shared/native-chat-agent-profiles' import { clearNativeChatSessionOptionCacheForTests } from './native-chat-session-option-cache' +import { clearNativeChatModelEnrichmentForTests } from './native-chat-session-option-enrichment' const mocks = vi.hoisted(() => ({ cancelPendingSends: vi.fn(), @@ -138,6 +139,7 @@ describe('NativeChatComposer', () => { beforeEach(() => { vi.clearAllMocks() clearNativeChatSessionOptionCacheForTests() + clearNativeChatModelEnrichmentForTests() mocks.fieldProps = null mocks.modelSwitchOutcome = 'applied' mocks.draftScopeKeys.length = 0 @@ -318,6 +320,59 @@ describe('NativeChatComposer', () => { expect(mocks.setDraft).not.toHaveBeenCalled() }) + it('renders the Claude model picker while host discovery is still pending', () => { + mocks.discoverCommitMessageModels.mockReturnValue(new Promise(() => {})) + render( + null} + /> + ) + + expect(mocks.fieldProps?.sessionOptionsSnapshot?.[0]).toMatchObject({ + id: 'model', + kind: { + choices: expect.arrayContaining([ + expect.objectContaining({ value: 'opus', label: 'Opus' }), + expect.objectContaining({ value: 'sonnet', label: 'Sonnet' }) + ]) + } + }) + }) + + it('keeps the Claude model picker when an older remote runtime omits the catalog origin', async () => { + mocks.discoverCommitMessageModels.mockResolvedValue({ + success: true, + defaultModelId: 'sonnet', + models: [{ id: 'sonnet', label: 'Sonnet' }] + }) + render( + null} + /> + ) + + await waitFor(() => expect(mocks.discoverCommitMessageModels).toHaveBeenCalled()) + await act(async () => undefined) + + expect(mocks.fieldProps?.sessionOptionsSnapshot?.[0]).toMatchObject({ + id: 'model', + kind: { + choices: expect.arrayContaining([ + expect.objectContaining({ value: 'fable', label: 'Fable' }), + expect.objectContaining({ value: 'haiku', label: 'Haiku' }) + ]) + } + }) + }) + it('shows the model already selected in the Claude TUI when chat opens', async () => { mocks.getMainBufferSnapshot.mockResolvedValue({ data: 'Claude Code v2.1.211\r\nOpus 4.8 with medium effort ยท API Usage Billing', diff --git a/src/renderer/src/components/native-chat/native-chat-pty-session-options.test.ts b/src/renderer/src/components/native-chat/native-chat-pty-session-options.test.ts index 135ad862449..c4de028b244 100644 --- a/src/renderer/src/components/native-chat/native-chat-pty-session-options.test.ts +++ b/src/renderer/src/components/native-chat/native-chat-pty-session-options.test.ts @@ -9,7 +9,7 @@ import { createNativeChatPtySessionOptions } from './native-chat-pty-session-opt describe('native chat PTY session options', () => { beforeEach(() => clearNativeChatSessionOptionCacheForTests()) - it('hides the model picker until the host catalog is known', () => { + it('renders nothing when no model list exists at all', () => { const surface = createNativeChatPtySessionOptions({ agent: 'claude', scopeKey: 'pty-1', @@ -20,6 +20,28 @@ describe('native chat PTY session options', () => { expect(surface.getSnapshot()).toEqual([]) }) + it('renders the version-neutral seed picker before any host catalog arrives', () => { + const surface = createNativeChatPtySessionOptions({ + agent: 'claude', + scopeKey: 'pty-1', + mode: 'live', + dispatchCommand: vi.fn() + })! + + expect(surface.getSnapshot()[0]).toMatchObject({ + id: 'model', + valueSource: 'unknown', + kind: { + choices: [ + expect.objectContaining({ value: 'fable', label: 'Fable' }), + expect.objectContaining({ value: 'opus', label: 'Opus' }), + expect.objectContaining({ value: 'sonnet', label: 'Sonnet' }), + expect.objectContaining({ value: 'haiku', label: 'Haiku' }) + ] + } + }) + }) + it('uses model and effort reported by the live Claude terminal', () => { const surface = createNativeChatPtySessionOptions({ agent: 'claude', @@ -583,7 +605,7 @@ describe('native chat PTY session options', () => { expect(surface.getSnapshot()[0]).toMatchObject({ valueSource: 'unknown' }) }) - it('tracks an unavailable Claude model without advertising it as a choice', () => { + it('passes an unknown persisted model through as a literal choice', () => { seedNativeChatAppliedSessionOptions('pty-1', 'claude', { model: 'future-model' }) @@ -596,7 +618,59 @@ describe('native chat PTY session options', () => { const model = surface.getSnapshot()[0] expect(model.kind).toMatchObject({ currentValue: 'future-model', - choices: expect.not.arrayContaining([{ value: 'future-model', label: 'future-model' }]) + choices: expect.arrayContaining([{ value: 'future-model', label: 'future-model' }]) + }) + }) + + it('keeps a tracked alias selectable when the host catalog omits it', async () => { + seedNativeChatAppliedSessionOptions('pty-1', 'claude', { + model: 'opus', + effort: 'xhigh' + }) + const dispatch = vi.fn() + const surface = createNativeChatPtySessionOptions({ + agent: 'claude', + scopeKey: 'pty-1', + // Why: current CLIs list `opus[1m]` and no plain `opus`. + initialModels: [ + { id: 'opus[1m]', label: 'Opus (1M context)', options: [] }, + { id: 'sonnet', label: 'Sonnet', options: [] } + ], + mode: 'live', + dispatchCommand: dispatch + })! + + expect(surface.getSnapshot()[0].kind).toMatchObject({ + currentValue: 'opus', + choices: expect.arrayContaining([ + { value: 'opus', label: 'Opus', description: expect.any(String) } + ]) + }) + expect(surface.getSnapshot().find(({ id }) => id === 'effort')).toMatchObject({ + settable: true, + kind: { currentValue: 'xhigh' } + }) + + await surface.setOption('effort', 'high') + + expect(dispatch).toHaveBeenCalledWith('/effort high') + }) + + it('drops the reconciled row once the tracked model moves onto the host catalog', async () => { + seedNativeChatAppliedSessionOptions('pty-1', 'claude', { model: 'opus' }) + const surface = createNativeChatPtySessionOptions({ + agent: 'claude', + scopeKey: 'pty-1', + initialModels: [{ id: 'opus[1m]', label: 'Opus (1M context)', options: [] }], + mode: 'live', + dispatchCommand: vi.fn() + })! + + await surface.setOption('model', 'opus[1m]') + + expect(surface.getSnapshot()[0].kind).toMatchObject({ + currentValue: 'opus[1m]', + choices: [{ value: 'opus[1m]', label: 'Opus (1M context)' }] }) }) diff --git a/src/renderer/src/components/native-chat/native-chat-pty-session-options.ts b/src/renderer/src/components/native-chat/native-chat-pty-session-options.ts index 8a424bf6984..a48a3611a22 100644 --- a/src/renderer/src/components/native-chat/native-chat-pty-session-options.ts +++ b/src/renderer/src/components/native-chat/native-chat-pty-session-options.ts @@ -1,5 +1,6 @@ import { getAgentSessionOptionCatalog, + type AgentSessionOptionCatalog, type CatalogModel } from '../../../../shared/agent-session-option-catalog' import type { AgentType } from '../../../../shared/agent-status-types' @@ -11,7 +12,8 @@ import type { import { createNativeChatSessionOptionRecord, readNativeChatSessionOptionCache, - writeNativeChatSessionOptionCache + writeNativeChatSessionOptionCache, + type NativeChatSessionOptionRecord } from './native-chat-session-option-cache' import { createSessionOptionAppliers } from './native-chat-session-option-apply' import { @@ -47,6 +49,24 @@ export type CreateNativeChatPtySessionOptionsArgs = { onDraftValuesChanged?: (values: Record) => void } +/** + * Why: the tracked model can sit outside the active list โ€” a persisted default, + * or an alias this host's CLI no longer lists. Keeping a row for it preserves + * the labelled selection and the model's own options instead of blanking both. + */ +function withTrackedModel( + catalog: AgentSessionOptionCatalog, + models: readonly CatalogModel[], + record: NativeChatSessionOptionRecord +): CatalogModel[] { + const trackedId = typeof record.model?.value === 'string' ? record.model.value : null + if (!trackedId || models.some((model) => model.id === trackedId)) { + return [...models] + } + const seeded = catalog.models.find((model) => model.id === trackedId) + return [...models, seeded ?? { id: trackedId, label: trackedId, options: [] }] +} + export function createNativeChatPtySessionOptions( args: CreateNativeChatPtySessionOptionsArgs ): NativeChatPtySessionOptionsSurface | null { @@ -65,12 +85,12 @@ export function createNativeChatPtySessionOptions( if (args.reportedValues && applyNativeChatReportedSessionOptions(record, args.reportedValues)) { writeNativeChatSessionOptionCache(args.scopeKey, record) } + const activeModels = (): CatalogModel[] => withTrackedModel(catalog, models, record) let snapshot = buildNativeChatSessionOptionSnapshot({ catalog, - models, + models: activeModels(), record, - mode: args.mode, - includeTrackedModelChoice: args.agent !== 'claude' + mode: args.mode }) const listeners = new Set<(value: SessionOptionDescriptor[]) => void>() @@ -78,10 +98,9 @@ export function createNativeChatPtySessionOptions( writeNativeChatSessionOptionCache(args.scopeKey, record) snapshot = buildNativeChatSessionOptionSnapshot({ catalog, - models, + models: activeModels(), record, - mode: args.mode, - includeTrackedModelChoice: args.agent !== 'claude' + mode: args.mode }) for (const listener of listeners) { listener(snapshot) @@ -126,7 +145,7 @@ export function createNativeChatPtySessionOptions( const appliers = createSessionOptionAppliers({ mode: args.mode, catalog, - getModels: () => models, + getModels: activeModels, getRecord: () => record, dispatchCommand: args.dispatchCommand, onAgentPicker: args.onAgentPicker, @@ -148,7 +167,7 @@ export function createNativeChatPtySessionOptions( recordOutgoingCommand: (command) => { const result = recordNativeChatSessionOptionCommand({ catalog, - models, + models: activeModels(), record, command, persist diff --git a/src/renderer/src/components/native-chat/native-chat-session-option-snapshot.ts b/src/renderer/src/components/native-chat/native-chat-session-option-snapshot.ts index 33e00c8f811..273267f3690 100644 --- a/src/renderer/src/components/native-chat/native-chat-session-option-snapshot.ts +++ b/src/renderer/src/components/native-chat/native-chat-session-option-snapshot.ts @@ -118,21 +118,19 @@ export function buildNativeChatSessionOptionSnapshot(args: { models: readonly CatalogModel[] record: NativeChatSessionOptionRecord mode: NativeChatSessionOptionMode - includeTrackedModelChoice: boolean }): SessionOptionDescriptor[] { const { catalog, models, record, mode } = args if (models.length === 0) { return [] } const modelTracked = record.model - const listedModelChoices = models.map(({ id, label, description }) => ({ + // Why: callers reconcile the tracked model into `models`, so every listed row + // is a real choice and the trigger never shows a value without one. + const modelChoices = models.map(({ id, label, description }) => ({ value: id, label, ...(description ? { description } : {}) })) - const modelChoices = args.includeTrackedModelChoice - ? choiceWithCurrent(listedModelChoices, modelTracked) - : listedModelChoices const modelSettable = settableState({ mode, apply: catalog.modelApply }) const modelAction = actionForApply(catalog.modelApply, modelTracked, mode) const snapshot: SessionOptionDescriptor[] = [ diff --git a/src/renderer/src/components/native-chat/use-native-chat-session-options.ts b/src/renderer/src/components/native-chat/use-native-chat-session-options.ts index 9769b580763..b0dfe6503fe 100644 --- a/src/renderer/src/components/native-chat/use-native-chat-session-options.ts +++ b/src/renderer/src/components/native-chat/use-native-chat-session-options.ts @@ -54,12 +54,15 @@ export function useNativeChatSessionOptions(args: { agent, scopeKey, ...(targetPtyId ? { fallbackScopeKey: terminalTabId } : {}), - initialModels: discoveryContext - ? (readNativeChatEnrichedModels(agent, discoveryContext.hostKey) ?? - (agent === 'claude' ? [] : undefined)) - : agent === 'claude' - ? [] - : undefined, + // Why: the catalog seed carries version-neutral family labels, so it is + // safe on every host while the once-per-host probe runs or after it fails + // โ€” without it the whole picker would pop in late or never appear. + ...(discoveryContext + ? { + initialModels: + readNativeChatEnrichedModels(agent, discoveryContext.hostKey) ?? undefined + } + : {}), mode: targetPtyId ? 'live' : 'draft', reportedValues, dispatchCommand,