From 33bbf24a625009413c44c7682f28993a5c2f35d8 Mon Sep 17 00:00:00 2001 From: Brooklyn Nicholson Date: Wed, 29 Jul 2026 21:26:22 -0500 Subject: [PATCH] fix(desktop): make the composer model dropdown's search commit honestly MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two fixes to the pill dropdown's filter, following the consensus across VS Code, Zed, Open WebUI, and Cherry Studio: - The pinned current model no longer rides along on a query it doesn't match. It sat above the real matches looking like the top result, so typing 'grok' and committing landed you back on the current model. - Enter in the search field commits the first visible match (VS Code's 'so Enter works without pressing DownArrow first'). Radix highlights nothing until an arrow key, so Enter used to dead-end; now open → type → Enter is the whole switch. Matched letters also render through HighlightMatches like the other searchable pickers. --- .../src/app/shell/model-menu-panel.test.tsx | 69 ++++++++++++++++++- .../src/app/shell/model-menu-panel.tsx | 44 ++++++++++-- 2 files changed, 106 insertions(+), 7 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 7da7c4af3d3..8d4512925b8 100644 --- a/apps/desktop/src/app/shell/model-menu-panel.test.tsx +++ b/apps/desktop/src/app/shell/model-menu-panel.test.tsx @@ -144,6 +144,65 @@ describe('ModelMenuPanel current selection', () => { }) }) +describe('ModelMenuPanel search', () => { + // The pinned current model must NOT ride along on a query it doesn't match: + // it reads like the top result, so Enter/click picks the wrong model (the + // "type grok, get fable" bug). Every surveyed picker (VS Code, Zed, Open + // WebUI, Cherry Studio) drops the pin while filtering. + // Highlighted labels are split across nodes, so single-text-node + // queries miss them — match on the row span's composed textContent. + const rowWithText = (content: ReturnType['content'], pattern: RegExp) => + content.queryByText((_, element) => element?.tagName === 'SPAN' && pattern.test(element.textContent ?? '')) + + it('hides the non-matching current model while a query is active', async () => { + $currentProvider.set('deepseek') + $currentModel.set('deepseek-v4-pro') + const { content } = renderPanel() + + await content.findByText(/Deepseek V4 Pro/i) + + const input = screen.getByRole('textbox', { name: 'Search models' }) + fireEvent.change(input, { target: { value: 'gemini' } }) + + await vi.waitFor(() => { + expect(rowWithText(content, /Gemini 3\.1 Pro/i)).not.toBeNull() + }) + expect(rowWithText(content, /Deepseek V4 Pro/i)).toBeNull() + }) + + it('Enter in the search field commits the first match', async () => { + const { content, onSelectModel } = renderPanel() + + await content.findByText('DeepSeek') + + const input = screen.getByRole('textbox', { name: 'Search models' }) + fireEvent.change(input, { target: { value: 'gemini' } }) + + await vi.waitFor(() => { + expect(rowWithText(content, /Gemini 3\.1 Pro/i)).not.toBeNull() + }) + + fireEvent.keyDown(input, { key: 'Enter' }) + + // First matching family of the first (alphabetical) matching provider. + await vi.waitFor(() => { + expect(onSelectModel).toHaveBeenCalledWith({ model: 'gemini-3.1-pro', provider: 'google', sessionId: 'runtime-1' }) + }) + }) + + it('Enter with no matches is a no-op (menu stays put, nothing selected)', async () => { + const { content, onSelectModel } = renderPanel() + + await content.findByText('DeepSeek') + + const input = screen.getByRole('textbox', { name: 'Search models' }) + fireEvent.change(input, { target: { value: 'zzz-no-such-model' } }) + fireEvent.keyDown(input, { key: 'Enter' }) + + expect(onSelectModel).not.toHaveBeenCalled() + }) +}) + describe('ModelMenuPanel provider collapse', () => { it('shows all provider models by default (none collapsed)', async () => { const { content } = renderPanel() @@ -205,9 +264,15 @@ describe('ModelMenuPanel provider collapse', () => { expect(input).not.toBeNull() fireEvent.change(input, { target: { value: 'deepseek' } }) - // Should show models — search bypasses collapse + // Should show models — search bypasses collapse. The matched letters render + // inside a , splitting the label across nodes, so match on the row + // span's composed textContent instead of a single text node. await vi.waitFor(() => { - expect(content.queryByText('Deepseek V4 Pro')).not.toBeNull() + expect( + content.queryByText( + (_, element) => element?.tagName === 'SPAN' && (element.textContent ?? '').startsWith('Deepseek V4 Pro') + ) + ).not.toBeNull() }) }) diff --git a/apps/desktop/src/app/shell/model-menu-panel.tsx b/apps/desktop/src/app/shell/model-menu-panel.tsx index 8ca63f5afd6..5ec34d61be4 100644 --- a/apps/desktop/src/app/shell/model-menu-panel.tsx +++ b/apps/desktop/src/app/shell/model-menu-panel.tsx @@ -16,6 +16,7 @@ import { DropdownMenuSub, DropdownMenuSubTrigger } from '@/components/ui/dropdown-menu' +import { HighlightMatches } from '@/components/ui/highlight-matches' import { Skeleton } from '@/components/ui/skeleton' import type { HermesGateway } from '@/hermes' import { useI18n } from '@/i18n' @@ -212,9 +213,38 @@ export function ModelMenuPanel({ gateway, onSelectModel, profile = 'default', re [pickerProviders, search, optionsModel, optionsProvider, effectiveVisibleModels] ) + // Enter in the search field commits the FIRST match — the VS Code pattern + // ("so Enter works without pressing DownArrow first"): ⌘⇧M → "grok" → Enter + // is the whole switch. Radix highlights nothing until an arrow key, so + // without this Enter would dead-end. Arrow-selected rows keep their own + // Enter (focus has left the input by then). + const commitFirstMatch = () => { + const group = groups[0] + const family = group?.families[0] + + if (!family) { + return + } + + void selectFamily(family, group.provider) + closeMenu() + } + return ( <> - + { + if (event.key === 'Enter' && normalize(search)) { + event.preventDefault() + event.stopPropagation() + commitFirstMatch() + } + }} + onValueChange={setSearch} + placeholder={copy.search} + value={search} + /> @@ -258,7 +288,9 @@ export function ModelMenuPanel({ gateway, onSelectModel, profile = 'default', re }} textValue="" > - {group.provider.name} + + + - {name} + {meta ? {meta} : null} {isCurrent ? ( @@ -452,9 +484,11 @@ function groupModels( // Always include the active model — but keep every row in the provider's // stable curated order (filter `allFamilies`, never reorder), so selecting - // a model can't shuffle the list. + // a model can't shuffle the list. While SEARCHING, the pin is skipped: a + // query means "show me matches", and a pinned non-match sitting above them + // reads like the top result (type "grok", see the current Fable first). const activeId = - provider.slug === current.provider && current.model + !q && provider.slug === current.provider && current.model ? allFamilies.find(family => family.id === current.model || family.fastId === current.model)?.id : undefined