diff --git a/apps/desktop/src/app/shell/model-catalog-menu.test.tsx b/apps/desktop/src/app/shell/model-catalog-menu.test.tsx new file mode 100644 index 0000000000..27c6c18807 --- /dev/null +++ b/apps/desktop/src/app/shell/model-catalog-menu.test.tsx @@ -0,0 +1,108 @@ +import { QueryClient, QueryClientProvider } from '@tanstack/react-query' +import { cleanup, fireEvent, render, screen } from '@testing-library/react' +import { afterEach, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest' + +import { DropdownMenu, DropdownMenuContent } from '@/components/ui/dropdown-menu' +import { + $modelVisibilityOpen, + $visibleModels, + modelVisibilityKey, + setModelVisibilityOpen, + setVisibleModels +} from '@/store/model-visibility' + +import { ModelCatalogMenu, type ModelMenuController } from './model-catalog-menu' + +// Radix calls these on open; jsdom doesn't implement them. +beforeAll(() => { + Element.prototype.scrollIntoView = vi.fn() + Element.prototype.hasPointerCapture = vi.fn(() => false) + Element.prototype.releasePointerCapture = vi.fn() +}) + +const getGlobalModelOptions = vi.fn() + +vi.mock('@/hermes', () => ({ + getGlobalModelOptions: (...args: unknown[]) => getGlobalModelOptions(...args), + setApiRequestProfile: vi.fn() +})) + +beforeEach(() => { + $visibleModels.set(null) + setModelVisibilityOpen(false) + getGlobalModelOptions.mockResolvedValue({ + providers: [{ models: ['gemini-3.1-pro', 'gemini-2.5-flash'], name: 'Google', slug: 'google' }] + }) +}) + +afterEach(() => { + cleanup() + vi.clearAllMocks() +}) + +// A minimal controller — these tests are about the CATALOG's own behaviour +// (what it lists, what it offers), not about what any host does with a pick. +function renderMenu() { + const select = vi.fn() + + const controller: ModelMenuController = { + applyPreset: vi.fn(), + current: { effort: '', fast: false, model: '', provider: '' }, + presetFor: () => ({}), + select, + setOptions: vi.fn() + } + + const client = new QueryClient({ defaultOptions: { queries: { retry: false } } }) + + render( + + + + + + + + ) + + return select +} + +// Curation is ONE global preference, so it belongs to the catalog rather than +// to whichever surface mounted it. If a host had to opt in, the composer and +// the kanban board would end up disagreeing about what "my models" means — +// which is exactly the drift extracting this component was meant to prevent. +describe('the catalog owns model curation', () => { + it('honours the stored Edit Models shortlist', async () => { + setVisibleModels(new Set([modelVisibilityKey('google', 'gemini-2.5-flash')])) + + renderMenu() + + await screen.findByText(/Gemini 2\.5 Flash/i) + expect(screen.queryByText(/Gemini 3\.1 Pro/i)).toBeNull() + }) + + it('still finds a hidden model by search — curation narrows the default view, not the catalog', async () => { + setVisibleModels(new Set([modelVisibilityKey('google', 'gemini-2.5-flash')])) + + renderMenu() + await screen.findByText(/Gemini 2\.5 Flash/i) + + const input = screen.getByRole('textbox', { name: 'Search models' }) + + fireEvent.change(input, { target: { value: 'gemini-3.1' } }) + + await vi.waitFor(() => { + expect(screen.queryByText(/Gemini 3\.1 Pro/i)).not.toBeNull() + }) + }) + + it('offers Edit Models without the host wiring it up', async () => { + renderMenu() + await screen.findByText(/Gemini 3\.1 Pro/i) + + fireEvent.click(screen.getByText('Edit Models…')) + + expect($modelVisibilityOpen.get()).toBe(true) + }) +}) diff --git a/apps/desktop/src/app/shell/model-catalog-menu.tsx b/apps/desktop/src/app/shell/model-catalog-menu.tsx index 7e8aac12d3..92325f41a8 100644 --- a/apps/desktop/src/app/shell/model-catalog-menu.tsx +++ b/apps/desktop/src/app/shell/model-catalog-menu.tsx @@ -26,11 +26,13 @@ import { DEFAULT_REASONING_EFFORT, reasoningEffortLabel } from '@/lib/reasoning- import { normalize } from '@/lib/text' import { cn } from '@/lib/utils' import { + $visibleModels, collapseModelFamilies, DEFAULT_VISIBLE_PER_PROVIDER, effectiveVisibleKeys, type ModelFamily, - modelVisibilityKey + modelVisibilityKey, + setModelVisibilityOpen } from '@/store/model-visibility' import { $collapsedProviders, toggleCollapsedProvider } from '@/store/provider-collapse' import { $defaultReasoningEffort } from '@/store/session' @@ -87,11 +89,11 @@ interface ModelCatalogMenuProps { * Off for override surfaces, where a MoA preset isn't a worker model. */ includeMoa?: boolean profile?: string - /** The user's STORED visible-model keys (null = never customized). Resolved - * against the fetched catalog inside the menu — a caller can't resolve it - * early against an unpopulated cache without hiding every row. Pass - * `undefined` to skip visibility filtering entirely. */ - visibleModels?: Set | null + /** Session whose catalog to fetch. A live session's catalog can differ from + * the profile-global one, and the app invalidates the SESSION-scoped query + * key on model changes — a surface bound to a session must pass it or its + * menu goes stale. Detached surfaces (per-task overrides) omit it. */ + sessionId?: null | string } interface ProviderGroup { @@ -112,7 +114,7 @@ export function ModelCatalogMenu({ gateway, includeMoa = false, profile = 'default', - visibleModels + sessionId = null }: ModelCatalogMenuProps) { const { t } = useI18n() const copy = t.shell.modelMenu @@ -120,13 +122,18 @@ export function ModelCatalogMenu({ const [search, setSearch] = useState('') const collapsedProviders = useStoreCollapsed() const defaultEffort = useDefaultEffort() + // Which models the user curated in Edit Models. Read HERE rather than taken + // as a prop: it's one global preference, so every surface that shows a + // catalog must show the same shortlist. A per-caller opt-in is how the board + // and the composer would end up disagreeing about what "my models" means. + const visibleModels = useStore($visibleModels) const modelOptions = useQuery({ - queryKey: modelOptionsQueryKey(profile, null), + queryKey: modelOptionsQueryKey(profile, sessionId), // Gateway-first even with no session: a connected (possibly remote) // gateway owns the model catalog, including virtual providers the local // REST fallback can't know about (#53817). - queryFn: (): Promise => requestModelOptions({ gateway }) + queryFn: (): Promise => requestModelOptions({ gateway, sessionId }) }) const loading = modelOptions.isPending && !modelOptions.data @@ -157,7 +164,7 @@ export function ModelCatalogMenu({ // provider list would otherwise resolve to an empty key set that reads as // "user hid everything" and blanks the menu on first open. const shownKeys = useMemo( - () => (visibleModels === undefined ? null : effectiveVisibleKeys(visibleModels, pickerProviders)), + () => effectiveVisibleKeys(visibleModels, pickerProviders), [visibleModels, pickerProviders] ) @@ -496,6 +503,18 @@ export function ModelCatalogMenu({ {footer} ) : null} + + {/* Curation belongs to the catalog, not to one host: wherever you can + pick a model you can say which models you want, and the shortlist is + the same everywhere because it's one stored preference. */} + + setModelVisibilityOpen(true)} + > + + {copy.editModels} + ) } diff --git a/apps/desktop/src/app/shell/model-menu-panel.tsx b/apps/desktop/src/app/shell/model-menu-panel.tsx index 81bb369acb..25da3ae697 100644 --- a/apps/desktop/src/app/shell/model-menu-panel.tsx +++ b/apps/desktop/src/app/shell/model-menu-panel.tsx @@ -12,7 +12,7 @@ import { currentPickerSelection } from '@/lib/model-status-label' import { DEFAULT_REASONING_EFFORT } from '@/lib/reasoning-effort' import { cn } from '@/lib/utils' import { $modelPresets, applyModelPreset, modelPresetKey, setModelPreset } from '@/store/model-presets' -import { $visibleModels, setModelVisibilityOpen } from '@/store/model-visibility' +import { $visibleModels } from '@/store/model-visibility' import { notifyError } from '@/store/notifications' import { $defaultReasoningEffort, @@ -215,32 +215,22 @@ export function ModelMenuPanel({ gateway, onSelectModel, profile = 'default', re - { - event.preventDefault() - void refreshModels() - }} - > - - {copy.refreshModels} - - - setModelVisibilityOpen(true)} - > - - {copy.editModels} - - + { + event.preventDefault() + void refreshModels() + }} + > + + {copy.refreshModels} + } gateway={gateway} includeMoa profile={profile} - visibleModels={visibleModels} + sessionId={activeSessionId} /> ) }