From c943787bf6b67d064c7b6eaaff337bf47f426166 Mon Sep 17 00:00:00 2001 From: Gille <4317663+helix4u@users.noreply.github.com> Date: Sat, 25 Jul 2026 16:00:36 -0600 Subject: [PATCH] fix(desktop): keep model picker aligned with session state --- .../src/app/shell/model-menu-panel.test.tsx | 20 +++++++++++ .../src/app/shell/model-menu-panel.tsx | 1 - apps/desktop/src/components/model-picker.tsx | 1 - .../src/lib/model-status-label.test.ts | 14 +++++--- apps/desktop/src/lib/model-status-label.ts | 33 ++++++++++++++----- 5 files changed, 54 insertions(+), 15 deletions(-) diff --git a/apps/desktop/src/app/shell/model-menu-panel.test.tsx b/apps/desktop/src/app/shell/model-menu-panel.test.tsx index 5a8e1d649b02..e68d3b99a077 100644 --- a/apps/desktop/src/app/shell/model-menu-panel.test.tsx +++ b/apps/desktop/src/app/shell/model-menu-panel.test.tsx @@ -124,6 +124,26 @@ describe('ModelMenuPanel MoA presets', () => { }) }) +describe('ModelMenuPanel current selection', () => { + it('keeps the checkmark on the live SessionView model when a stale options response disagrees', async () => { + $currentProvider.set('google') + $currentModel.set('gemini-3.1-pro') + getGlobalModelOptions.mockResolvedValue({ + model: 'deepseek-chat', + provider: 'deepseek', + providers: MOCK_PROVIDERS + }) + + const { content } = renderPanel() + + const currentRow = (await content.findByText(/Gemini 3\.1 Pro/i)).closest('[role="menuitem"]') + const staleRow = content.getByText('Deepseek Chat').closest('[role="menuitem"]') + + expect(currentRow?.querySelector('.codicon-check')).not.toBeNull() + expect(staleRow?.querySelector('.codicon-check')).toBeNull() + }) +}) + describe('ModelMenuPanel provider collapse', () => { it('shows all provider models by default (none collapsed)', async () => { const { content } = renderPanel() diff --git a/apps/desktop/src/app/shell/model-menu-panel.tsx b/apps/desktop/src/app/shell/model-menu-panel.tsx index 9c3c60e5cdaf..fc8cd3e2f42b 100644 --- a/apps/desktop/src/app/shell/model-menu-panel.tsx +++ b/apps/desktop/src/app/shell/model-menu-panel.tsx @@ -96,7 +96,6 @@ export function ModelMenuPanel({ gateway, onSelectModel, profile = 'default', re }) const { model: optionsModel, provider: optionsProvider } = currentPickerSelection( - !!activeSessionId, { model: currentModel, provider: currentProvider }, modelOptions.data ) diff --git a/apps/desktop/src/components/model-picker.tsx b/apps/desktop/src/components/model-picker.tsx index 73e3401e078f..3395a0c3387a 100644 --- a/apps/desktop/src/components/model-picker.tsx +++ b/apps/desktop/src/components/model-picker.tsx @@ -64,7 +64,6 @@ export function ModelPickerDialog({ const providers = modelOptions.data?.providers ?? [] const { model: optionsModel, provider: optionsProvider } = currentPickerSelection( - !!sessionId, { model: currentModel, provider: currentProvider }, modelOptions.data ) diff --git a/apps/desktop/src/lib/model-status-label.test.ts b/apps/desktop/src/lib/model-status-label.test.ts index cc499c86d565..258e242dccf4 100644 --- a/apps/desktop/src/lib/model-status-label.test.ts +++ b/apps/desktop/src/lib/model-status-label.test.ts @@ -48,19 +48,23 @@ describe('model-status-label', () => { const options = { model: 'hermes-4', provider: 'nous' } it('prefers the sticky composer pick over the profile default pre-session', () => { - expect(currentPickerSelection(false, store, options)).toEqual(store) + expect(currentPickerSelection(store, options)).toEqual(store) }) - it('lets the live session model.options win when a session exists', () => { - expect(currentPickerSelection(true, store, options)).toEqual(options) + it('keeps the SessionView selection when a stale options response disagrees', () => { + expect(currentPickerSelection(store, options)).toEqual(store) }) it('falls back to options when the store is empty', () => { - expect(currentPickerSelection(false, { model: '', provider: '' }, options)).toEqual(options) + expect(currentPickerSelection({ model: '', provider: '' }, options)).toEqual(options) + }) + + it('uses the complete options pair instead of mixing a partial store selection', () => { + expect(currentPickerSelection({ model: 'opus', provider: '' }, options)).toEqual(options) }) it('falls back to the store while options are still loading', () => { - expect(currentPickerSelection(true, store, undefined)).toEqual(store) + expect(currentPickerSelection(store, undefined)).toEqual(store) }) }) }) diff --git a/apps/desktop/src/lib/model-status-label.ts b/apps/desktop/src/lib/model-status-label.ts index a1b28b8a4c58..6e2f284a91ed 100644 --- a/apps/desktop/src/lib/model-status-label.ts +++ b/apps/desktop/src/lib/model-status-label.ts @@ -21,19 +21,36 @@ export function reasoningEffortLabel(effort: string): string { return REASONING_LABELS[key] ?? effort } -/** Which model/provider a picker should mark "current". With a live session the - * gateway's `model.options` is authoritative; pre-session there is no server - * "current", so the sticky composer pick wins over the profile default the - * global options query returns — else the checkmark snaps back to the default - * and the pick looks ignored. */ +/** Which model/provider pair a picker should mark "current". SessionView state + * also drives the composer label, so a complete pair there wins over an older + * `model.options` response. During initial hydration (or pre-session startup), + * options remain the fallback. Pick one complete pair before mixing fields so + * a model is never shown under a different provider. */ export function currentPickerSelection( - hasSession: boolean, store: { model: string; provider: string }, options?: { model?: string; provider?: string } ): { model: string; provider: string } { + const storeSelection = { + model: String(store.model || ''), + provider: String(store.provider || '') + } + + const optionsSelection = { + model: String(options?.model || ''), + provider: String(options?.provider || '') + } + + if (storeSelection.model && storeSelection.provider) { + return storeSelection + } + + if (optionsSelection.model && optionsSelection.provider) { + return optionsSelection + } + return { - model: String((hasSession && options?.model) || store.model || options?.model || ''), - provider: String((hasSession && options?.provider) || store.provider || options?.provider || '') + model: storeSelection.model || optionsSelection.model, + provider: storeSelection.provider || optionsSelection.provider } }