diff --git a/apps/desktop/src/app/session/hooks/use-model-controls.test.tsx b/apps/desktop/src/app/session/hooks/use-model-controls.test.tsx index 42421589a1..01bbd96e90 100644 --- a/apps/desktop/src/app/session/hooks/use-model-controls.test.tsx +++ b/apps/desktop/src/app/session/hooks/use-model-controls.test.tsx @@ -532,29 +532,28 @@ describe('useModelControls', () => { expect($currentProvider.get()).toBe('custom:local') }) - it('reseeds a sticky manual pick that was removed from the catalog', async () => { - vi.mocked(getGlobalModelInfo).mockResolvedValue({ model: 'openai/gpt-5.5', provider: 'openai-codex' }) + it('keeps a sticky manual pick even when its provider row does not list the model', async () => { + // Rows are hints: a custom endpoint serves ids the picker row lacks. The + // pick is the user's selection and must not be reseeded to the default. + vi.mocked(getGlobalModelInfo).mockResolvedValue({ model: 'deepseek-v4-flash-0731', provider: 'custom:hyper' }) const queryClient = new QueryClient() - $activeGatewayProfile.set('compass') queryClient.setQueryData(modelOptionsQueryKey('default'), { - providers: [{ models: ['openrouter/owl-alpha'], name: 'OpenRouter', slug: 'openrouter' }] - }) - queryClient.setQueryData(modelOptionsQueryKey('compass'), { - providers: [{ models: ['openai/gpt-5.5'], name: 'OpenRouter', slug: 'openrouter' }] + providers: [ + { aliases: ['custom:hyper', 'hyper'], models: ['deepseek-v4-flash-0731'], name: 'Hyper', slug: 'hyper' } + ] }) - // A manual pick whose model no longer exists on its provider. - setCurrentModel('openrouter/owl-alpha') - setCurrentProvider('openrouter') + setCurrentModel('deepseek-v4.1-flash') + setCurrentProvider('custom:hyper') setCurrentModelSource('manual') const { result } = renderHook(() => useModelControls({ queryClient, requestGateway: vi.fn() })) await result.current.refreshCurrentModel() - expect($currentModel.get()).toBe('openai/gpt-5.5') - expect(getCurrentModelSource()).toBe('default') + expect($currentModel.get()).toBe('deepseek-v4.1-flash') + expect(getCurrentModelSource()).toBe('manual') }) it('keeps a sticky manual pick that is still in the catalog', async () => { diff --git a/apps/desktop/src/app/session/hooks/use-model-controls.ts b/apps/desktop/src/app/session/hooks/use-model-controls.ts index 8f28e1cf8e..e6ee91440e 100644 --- a/apps/desktop/src/app/session/hooks/use-model-controls.ts +++ b/apps/desktop/src/app/session/hooks/use-model-controls.ts @@ -6,7 +6,7 @@ import { getGlobalModelInfo } from '@/hermes' import { useI18n } from '@/i18n' import { isBusySessionModelSwitch } from '@/lib/gateway-rpc' import { surfaceModelSwitchConfirm } from '@/lib/guarded-model-switch' -import { manualPickRemoved, modelOptionsQueryKey } from '@/lib/model-options' +import { modelOptionsQueryKey } from '@/lib/model-options' import { notifyError } from '@/store/notifications' import { $activeGatewayProfile } from '@/store/profile' import { @@ -126,22 +126,10 @@ export function useModelControls({ return } - // A manual pick stays sticky UNLESS it was removed from the catalog (its - // model no longer exists on the provider), in which case keeping it would - // 404 every new chat — fall through to reseed from the profile default. - // Reads the model-options cache the composer already populated; an - // unknown/not-yet-loaded catalog conservatively preserves the pick. - const keepManualPick = () => { - if (force || !$currentModel.get() || getCurrentModelSource() !== 'manual') { - return false - } - - const options = queryClient.getQueryData( - modelOptionsQueryKey(cacheProfile || $activeGatewayProfile.get(), null, cacheOwnerConnectionId) - ) - - return !manualPickRemoved(options?.providers, $currentProvider.get(), $currentModel.get()) - } + // A manual pick is sticky. It is never diffed against the catalog: rows + // are hints, and a custom slug the row lacks is still the user's choice + // (the gateway validates it on switch). + const keepManualPick = () => !force && Boolean($currentModel.get()) && getCurrentModelSource() === 'manual' if (keepManualPick()) { return @@ -177,7 +165,7 @@ export function useModelControls({ // The delayed session.info event still updates this once the agent is ready. } }, - [cacheOwnerConnectionId, cacheProfile, queryClient] + [] ) // Returns whether the switch was applied so callers can await it before 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 dbe972dc05..695433ac09 100644 --- a/apps/desktop/src/app/shell/model-menu-panel.test.tsx +++ b/apps/desktop/src/app/shell/model-menu-panel.test.tsx @@ -1,25 +1,13 @@ import { QueryClient, QueryClientProvider } from '@tanstack/react-query' -import { act, cleanup, fireEvent, render, screen } from '@testing-library/react' -import { useState } from 'react' +import { cleanup, fireEvent, render, screen } from '@testing-library/react' import { afterEach, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest' -import { useModelControls } from '@/app/session/hooks/use-model-controls' import { DropdownMenu, DropdownMenuContent } from '@/components/ui/dropdown-menu' import { $collapsedProviders, toggleCollapsedProvider } from '@/store/provider-collapse' import { $activeSessionId, $currentModel, $currentProvider } from '@/store/session' import { ModelMenuPanel } from './model-menu-panel' -const notify = vi.fn((..._args: unknown[]) => 'confirm-toast-1') -const notifyError = vi.fn((..._args: unknown[]) => undefined) -const dismissNotification = vi.fn((..._args: unknown[]) => undefined) - -vi.mock('@/store/notifications', () => ({ - dismissNotification: (...args: unknown[]) => dismissNotification(...args), - notify: (...args: unknown[]) => notify(...args), - notifyError: (...args: unknown[]) => notifyError(...args) -})) - // Radix calls these on open; jsdom doesn't implement them. beforeAll(() => { Element.prototype.scrollIntoView = vi.fn() @@ -420,7 +408,9 @@ describe('ModelMenuPanel provider collapse', () => { expect($collapsedProviders.get()).toContain('deepseek') }) - it('switches the session model when Refresh Models drops the current pick', async () => { + it('keeps the current pick when Refresh Models no longer lists it', async () => { + // Rows are hints (discovered / curated / capped); a custom slug the row + // lacks is still what the user selected. Only the gateway may reject it. $currentProvider.set('zhipu') $currentModel.set('glm-4.5-air') getGlobalModelOptions @@ -442,12 +432,11 @@ describe('ModelMenuPanel provider collapse', () => { fireEvent.click(await content.findByText('Refresh models')) await vi.waitFor(() => { - expect(onSelectModel).toHaveBeenCalledWith({ - model: 'deepseek-v4-pro', - provider: 'deepseek', - sessionId: 'runtime-1' - }) + expect(getGlobalModelOptions).toHaveBeenCalledTimes(2) }) + expect(onSelectModel).not.toHaveBeenCalled() + expect($currentModel.get()).toBe('glm-4.5-air') + expect($currentProvider.get()).toBe('zhipu') }) it('does not switch when Refresh Models still lists the current pick', async () => { @@ -525,112 +514,3 @@ describe('ModelMenuPanel provider collapse', () => { expect(onSelectModel).not.toHaveBeenCalled() }) }) - -describe('ModelMenuPanel refresh reconcile × guarded-switch confirm handshake', () => { - // #95446 fix (reconcile after Refresh Models) composes with the - // confirm-handshake guard: when the reconcile target is itself a GUARDED - // model (contributor tier / expensive), the switch must surface the confirm - // flow — one config.set, a warning with a Confirm action, rollback until - // confirmed — never a silent retry loop and never a silently-painted pick. - function ConfirmHarness({ - requestGateway - }: { - requestGateway: (method: string, params?: Record) => Promise - }) { - const [client] = useState(() => new QueryClient({ defaultOptions: { queries: { retry: false } } })) - const controls = useModelControls({ queryClient: client, requestGateway }) - - return ( - - - - - - - - ) - } - - it('reconcile-triggered switch to a guarded model surfaces confirm, not a silent retry', async () => { - $activeSessionId.set('runtime-1') - $currentProvider.set('zhipu') - $currentModel.set('glm-4.5-air') - getGlobalModelOptions - .mockResolvedValueOnce({ - providers: [{ models: ['glm-4.5-air'], name: 'Zhipu', slug: 'zhipu' }, MOA_PROVIDER] - }) - // Refresh drops the current pick; the only remaining model is guarded. - .mockResolvedValueOnce({ - providers: [{ models: ['muse-spark-1.2-contributor'], name: 'OpenCode', slug: 'opencode-go' }, MOA_PROVIDER] - }) - - // Method-aware gateway: the panel's catalog reads (`model.options`) use - // the routed catalog mock; `config.set` runs the guarded handshake — - // confirm_required first, success on the confirmed resend. - let configSets = 0 - - const requestGateway = vi.fn(async (method: string, _params?: Record) => { - if (method === 'model.options') { - return getGlobalModelOptions() - } - - if (method !== 'config.set') { - throw new Error(`unexpected gateway method: ${method}`) - } - - configSets += 1 - - if (configSets === 1) { - return { - confirm_message: 'CONTRIBUTOR TIER: this model may train on your data.', - confirm_required: true, - key: 'model', - value: 'muse-spark-1.2-contributor' - } - } - - return { key: 'model', scope: 'global', value: 'muse-spark-1.2-contributor' } - }) - - const content = render() - - await content.findByText(/Glm 4\.5 Air/i) - fireEvent.click(await content.findByText('Refresh models')) - - // The reconcile fired exactly ONE switch attempt and it came back - // confirm_required → the confirm toast is up, nothing retried silently. - await vi.waitFor(() => { - expect(notify).toHaveBeenCalledWith( - expect.objectContaining({ - action: expect.objectContaining({ label: expect.any(String) }), - kind: 'warning', - message: 'CONTRIBUTOR TIER: this model may train on your data.' - }) - ) - }) - - const configSetCalls = requestGateway.mock.calls.filter(([method]) => method === 'config.set') - expect(configSetCalls).toHaveLength(1) - expect(configSetCalls[0][1]).not.toHaveProperty('confirm_expensive_model') - - // Pending confirmation = rolled back, not silently painted. - expect($currentModel.get()).toBe('glm-4.5-air') - expect($currentProvider.get()).toBe('zhipu') - - // User confirms → ONE resend carrying confirm_expensive_model: true. - const lastNotify = notify.mock.calls.at(-1)?.[0] as { action: { onClick: () => Promise } } - - await act(async () => { - await lastNotify.action.onClick() - }) - - await vi.waitFor(() => { - const resend = requestGateway.mock.calls.filter(([method]) => method === 'config.set') - expect(resend).toHaveLength(2) - expect(resend[1][1]).toMatchObject({ confirm_expensive_model: true, session_id: 'runtime-1' }) - }) - expect($currentModel.get()).toBe('muse-spark-1.2-contributor') - expect($currentProvider.get()).toBe('opencode-go') - expect(notifyError).not.toHaveBeenCalled() - }) -}) diff --git a/apps/desktop/src/app/shell/model-menu-panel.tsx b/apps/desktop/src/app/shell/model-menu-panel.tsx index 452c881143..58044d00fd 100644 --- a/apps/desktop/src/app/shell/model-menu-panel.tsx +++ b/apps/desktop/src/app/shell/model-menu-panel.tsx @@ -7,7 +7,7 @@ import { Codicon } from '@/components/ui/codicon' import { DropdownMenuItem, dropdownMenuRow } from '@/components/ui/dropdown-menu' import type { HermesGateway } from '@/hermes' import { useI18n } from '@/i18n' -import { modelOptionsQueryKey, reconcileSelectionAfterCatalogRefresh, requestModelOptions } from '@/lib/model-options' +import { modelOptionsQueryKey, requestModelOptions } from '@/lib/model-options' import { currentPickerSelection } from '@/lib/model-status-label' import { DEFAULT_REASONING_EFFORT } from '@/lib/reasoning-effort' import { cn } from '@/lib/utils' @@ -111,16 +111,9 @@ export function ModelMenuPanel({ sessionId: activeSessionId }) + // The refreshed catalog is a hint list, never a reason to move the pick: + // a custom slug the row lacks is still what the user selected. queryClient.setQueryData(queryKey, next) - - // Group / credential swaps can return a catalog that no longer contains - // the session's current model. The store + currentPickerSelection would - // otherwise keep painting the stale id (it is not in the new list). - const switchTo = reconcileSelectionAfterCatalogRefresh(optionsModel, next.providers, optionsProvider) - - if (switchTo) { - await onSelectModel({ ...switchTo, sessionId: activeSessionId || null }) - } } catch { // Network/backend hiccup — fall back to a plain invalidate so the next // open re-fetches (still cached, but no worse than before). diff --git a/apps/desktop/src/lib/model-options.test.ts b/apps/desktop/src/lib/model-options.test.ts index 2ab29bff5a..ee1953b519 100644 --- a/apps/desktop/src/lib/model-options.test.ts +++ b/apps/desktop/src/lib/model-options.test.ts @@ -3,15 +3,7 @@ import { afterEach, describe, expect, it, vi } from 'vitest' import { getGlobalModelOptions } from '@/hermes' -import { - catalogProviderMatches, - firstSelectableCatalogModel, - manualPickRemoved, - modelOptionsQueryKey, - reconcileSelectionAfterCatalogRefresh, - requestModelOptions, - selectionInCatalog -} from './model-options' +import { catalogProviderMatches, modelOptionsQueryKey, requestModelOptions } from './model-options' const globalOptions = { model: 'hermes-4', provider: 'nous', providers: [] } @@ -216,43 +208,6 @@ describe('modelOptionsQueryKey', () => { }) }) -describe('manualPickRemoved', () => { - const providers = [ - { name: 'OpenRouter', slug: 'openrouter', models: ['owl-alpha', 'gpt-5.5'] }, - { name: 'Nous', slug: 'nous', models: [] } // present but unconfigured / re-auth - ] - - it('flags a pick whose model was dropped from a populated provider', () => { - expect(manualPickRemoved(providers, 'openrouter', 'nemotron-removed')).toBe(true) - }) - - it('keeps a pick that is still in the catalog', () => { - expect(manualPickRemoved(providers, 'openrouter', 'gpt-5.5')).toBe(false) - }) - - it('matches the provider by name as well as slug', () => { - expect(manualPickRemoved(providers, 'OpenRouter', 'gpt-5.5')).toBe(false) - expect(manualPickRemoved(providers, 'OpenRouter', 'gone')).toBe(true) - }) - - it('never clobbers when the provider is absent (ambiguous / deauth)', () => { - expect(manualPickRemoved(providers, 'anthropic', 'claude-sonnet-4.6')).toBe(false) - }) - - it('never clobbers when the provider has an empty model list (re-auth)', () => { - expect(manualPickRemoved(providers, 'nous', 'hermes-4')).toBe(false) - }) - - it('never clobbers on a not-yet-loaded or empty catalog', () => { - expect(manualPickRemoved(undefined, 'openrouter', 'gpt-5.5')).toBe(false) - expect(manualPickRemoved([], 'openrouter', 'gpt-5.5')).toBe(false) - }) - - it('never clobbers when there is no pick', () => { - expect(manualPickRemoved(providers, '', '')).toBe(false) - }) -}) - describe('catalogProviderMatches', () => { const cloudflare = { aliases: ['custom:cloudflare', 'cloudflare'], @@ -268,84 +223,3 @@ describe('catalogProviderMatches', () => { expect(catalogProviderMatches(cloudflare, 'openrouter')).toBe(false) }) }) - -describe('reconcileSelectionAfterCatalogRefresh', () => { - const zhipu = { name: '智谱2', slug: 'zhipu', models: ['glm-4.5-air', 'glm-5-turbo'] } - - const bytea = { - name: '字节A', - slug: 'byteplus', - models: ['deepseek-v4-flash', 'doubao-seed-2.0-pro'] - } - - const moa = { name: 'Mixture of Agents', slug: 'moa', models: ['default'] } - - const openrouter = { - models: ['glm-4.5-air', 'gpt-5.5'], - name: 'OpenRouter', - slug: 'openrouter' - } - - it('switches to the first new-group model when the current pick is gone', () => { - expect(selectionInCatalog([bytea], 'glm-4.5-air', 'zhipu')).toBe(false) - expect(firstSelectableCatalogModel([moa, bytea])).toEqual({ - model: 'deepseek-v4-flash', - provider: 'byteplus' - }) - expect(reconcileSelectionAfterCatalogRefresh('glm-4.5-air', [moa, bytea], 'zhipu')).toEqual({ - model: 'deepseek-v4-flash', - provider: 'byteplus' - }) - }) - - it('keeps the current pick when it is still in the refreshed catalog', () => { - expect(reconcileSelectionAfterCatalogRefresh('glm-4.5-air', [zhipu, moa], 'zhipu')).toBeNull() - }) - - it('keeps the current provider when the same model id exists on another provider', () => { - expect(selectionInCatalog([openrouter, zhipu], 'glm-4.5-air', 'zhipu')).toBe(true) - expect(selectionInCatalog([openrouter, zhipu], 'glm-4.5-air', 'openrouter')).toBe(true) - expect(reconcileSelectionAfterCatalogRefresh('glm-4.5-air', [openrouter, zhipu], 'zhipu')).toBeNull() - }) - - it('keeps a custom provider pair when OpenRouter lists the same model id', () => { - const model = '@cf/meta/llama-3.3-70b-instruct-fp8-fast' - - const cloudflare = { - aliases: ['custom:cloudflare', 'cloudflare'], - models: [model], - name: 'Cloudflare', - slug: 'cloudflare' - } - - const openrouterCf = { models: [model, 'gpt-5.5'], name: 'OpenRouter', slug: 'openrouter' } - - expect(selectionInCatalog([openrouterCf, cloudflare], model, 'custom:cloudflare')).toBe(true) - expect(reconcileSelectionAfterCatalogRefresh(model, [openrouterCf, cloudflare], 'custom:cloudflare')).toBeNull() - }) - - it('does not jump to OpenRouter when the current provider is missing but still lists the same model id', () => { - expect(reconcileSelectionAfterCatalogRefresh('glm-4.5-air', [openrouter, moa], 'zhipu')).toBeNull() - }) - - it('keeps the pick when the current provider is present but unconfigured', () => { - const emptyZhipu = { models: [], name: '智谱2', slug: 'zhipu' } - - expect(reconcileSelectionAfterCatalogRefresh('glm-4.5-air', [emptyZhipu, openrouter], 'zhipu')).toBeNull() - }) - - it('falls back when the current provider is populated and dropped the model', () => { - const zhipuWithoutAir = { models: ['glm-5-turbo'], name: '智谱2', slug: 'zhipu' } - - expect(reconcileSelectionAfterCatalogRefresh('glm-4.5-air', [zhipuWithoutAir, openrouter], 'zhipu')).toEqual({ - model: 'glm-5-turbo', - provider: 'zhipu' - }) - }) - - it('does not wipe the pick when the refreshed catalog has no selectable models', () => { - expect(reconcileSelectionAfterCatalogRefresh('glm-4.5-air', [moa], 'zhipu')).toBeNull() - expect(reconcileSelectionAfterCatalogRefresh('glm-4.5-air', [], 'zhipu')).toBeNull() - expect(reconcileSelectionAfterCatalogRefresh('glm-4.5-air', undefined, 'zhipu')).toBeNull() - }) -}) diff --git a/apps/desktop/src/lib/model-options.ts b/apps/desktop/src/lib/model-options.ts index c858817768..65284e3b9f 100644 --- a/apps/desktop/src/lib/model-options.ts +++ b/apps/desktop/src/lib/model-options.ts @@ -17,157 +17,12 @@ export function catalogProviderMatches(provider: CatalogProviderIdentity, curren ) } -function findCatalogProvider( - providers: ModelOptionProvider[] | undefined, - provider: string -): ModelOptionProvider | undefined { - if (!providers?.length || !provider) { - return undefined - } - - return providers.find(row => catalogProviderMatches(row, provider)) -} - -function catalogHasModel(providers: ModelOptionProvider[] | undefined, model: string): boolean { - return Boolean(model) && (providers?.some(provider => (provider.models ?? []).includes(model)) ?? false) -} - -/** - * True only when a persisted **manual** composer pick has been removed from the - * catalog (its provider still ships models, but no longer this one) — so a new - * chat would keep 404'ing the dead model. Deliberately conservative to never - * clobber a still-valid pick: an unknown/absent provider, an empty model list - * (re-auth / unconfigured), or a not-yet-loaded catalog all return false. - */ -export function manualPickRemoved( - providers: ModelOptionProvider[] | undefined, - provider: string, - model: string -): boolean { - if (!providers?.length || !provider || !model) { - return false - } - - const row = findCatalogProvider(providers, provider) - - if (!row) { - return false - } - - const models = row.models ?? [] - - // Empty list means the provider is present but unconfigured / awaiting - // re-auth, not that the model was dropped — leave the pick alone. - if (models.length === 0) { - return false - } - - return !models.includes(model) -} - -const MOA_PROVIDER_SLUG = 'moa' - -/** True when THIS provider still lists `model`. Identity is the (provider, - * model) pair: the same id on OpenRouter does not count as the custom / - * first-party pick still being offered. */ -export function selectionInCatalog( - providers: ModelOptionProvider[] | undefined, - model: string, - provider?: string -): boolean { - if (!providers?.length || !model || !provider) { - return false - } - - const row = findCatalogProvider(providers, provider) - - return Boolean(row && (row.models ?? []).includes(model)) -} - -/** First real (non-MoA) catalog row that still has models. */ -export function firstSelectableCatalogModel( - providers: ModelOptionProvider[] | undefined -): { model: string; provider: string } | null { - if (!providers?.length) { - return null - } - - for (const provider of providers) { - if (provider.slug === MOA_PROVIDER_SLUG) { - continue - } - - const model = provider.models?.[0] - - if (model) { - return { model, provider: provider.slug } - } - } - - return null -} - -/** - * After Refresh Models replaces the catalog: keep the current **pair** when - * that provider still lists the model. Never rewrite the provider just because - * another catalog row (OpenRouter, …) exposes the same model id. - * - * Conservative like `manualPickRemoved` when the current provider is absent or - * unconfigured (empty models) — except a fully gone model (group / credential - * swap that dropped the id everywhere) still falls back to the first - * selectable row. Returns null when the catalog is empty/unloaded so we never - * wipe a selection on a failed or still-hydrating refresh. - */ -export function reconcileSelectionAfterCatalogRefresh( - currentModel: string, - providers: ModelOptionProvider[] | undefined, - currentProvider?: string -): { model: string; provider: string } | null { - const next = firstSelectableCatalogModel(providers) - - if (!next) { - return null - } - - if (!currentModel) { - return next - } - - if (currentProvider) { - if (selectionInCatalog(providers, currentModel, currentProvider)) { - return null - } - - const row = findCatalogProvider(providers, currentProvider) - - // Present but empty: re-auth / unconfigured, not "model dropped". - if (row && (row.models ?? []).length === 0) { - return null - } - - // This provider still ships models and dropped this one — fall back. - if (row) { - return next - } - - // Provider missing from the refreshed catalog. Another provider listing - // the same id is NOT a reason to switch (the OpenRouter collision). - if (catalogHasModel(providers, currentModel)) { - return null - } - - return next - } - - // No provider on the current pair: keep when the id is still offered - // anywhere, otherwise fall back. Callers that know the provider must pass it - // so a shared id cannot retarget the pick. - if (catalogHasModel(providers, currentModel)) { - return null - } - - return next -} +// A picked (provider, model) pair is never retargeted from catalog membership. +// Picker rows are hints (discovered / curated / capped lists); a custom endpoint +// or a newer release legitimately serves ids the row lacks, and the backend +// soft-accepts them. Diffing the pick against the catalog silently swapped +// `deepseek-v4.1-flash` for the row's `-0731` sibling. The only authority on a +// pick's validity is the gateway's switch result. interface ModelOptionsRequest { /** When false, include ambient/unconfigured providers (onboarding/setup diff --git a/hermes_cli/model_switch.py b/hermes_cli/model_switch.py index ca6d47af5d..21be566fe8 100644 --- a/hermes_cli/model_switch.py +++ b/hermes_cli/model_switch.py @@ -1391,9 +1391,18 @@ def _validate_switch(st: _Switch) -> Optional[ModelSwitchResult]: headers = st.validation_headers or ( _extra_headers_from_config(st.user_providers.get(st.target_provider)) if st.user_providers and st.target_provider in st.user_providers else None) + # A ``providers.`` endpoint is the user's own: validate it as a custom endpoint (an id its + # listing lacks is soft-accepted) whether the slug arrived as ``custom:`` or the bare key + # the picker rows carry — otherwise the bare spelling fell into the built-in live-listing + # branch and hard-rejected the very model the user selected. + validate_as = st.target_provider + if not validate_as.lower().startswith("custom"): + pdef = resolve_provider_full(validate_as, st.user_providers, st.custom_providers) + if pdef is not None and pdef.source == "user-config": + validate_as = f"custom:{validate_as}" try: validation = validate_requested_model( - st.new_model, st.target_provider, api_key=st.api_key, base_url=st.base_url, + st.new_model, validate_as, api_key=st.api_key, base_url=st.base_url, api_mode=st.api_mode or None, headers=headers) except Exception as e: validation = {"accepted": False, "persist": False, "recognized": False, @@ -1406,7 +1415,6 @@ def _validate_switch(st: _Switch) -> Optional[ModelSwitchResult]: validation.get("message", "Invalid model"), new_model=st.new_model, target_provider=st.target_provider, provider_label=st.provider_label) validation = {"accepted": True, "persist": True, "recognized": False, "message": validation.get("message", "")} - st.new_model = validation.get("corrected_model") or st.new_model st.validation = validation return None diff --git a/hermes_cli/models_validate.py b/hermes_cli/models_validate.py index ceae33a2e7..a62565a91e 100644 --- a/hermes_cli/models_validate.py +++ b/hermes_cli/models_validate.py @@ -21,13 +21,8 @@ from hermes_constants import openrouter_variant_base # ── Verdicts ───────────────────────────────────────────────────────────── -def _verdict(accepted: bool, persist: bool, recognized: bool, message: Optional[str], - corrected_model: Optional[str] = None) -> dict[str, Any]: - out: dict[str, Any] = {"accepted": accepted, "persist": persist, "recognized": recognized} - if corrected_model is not None: - out["corrected_model"] = corrected_model - out["message"] = message - return out +def _verdict(accepted: bool, persist: bool, recognized: bool, message: Optional[str]) -> dict[str, Any]: + return {"accepted": accepted, "persist": persist, "recognized": recognized, "message": message} def _accept() -> dict[str, Any]: @@ -47,28 +42,16 @@ def _soft_accept(message: Optional[str]) -> dict[str, Any]: return _verdict(True, True, False, message) -def _corrected(requested: str, corrected: str) -> dict[str, Any]: - return _verdict(True, True, True, f"Auto-corrected `{requested}` → `{corrected}`", - corrected_model=corrected) - - # ── Catalog matching ───────────────────────────────────────────────────── @dataclass class _Match: exact: bool = False - corrected: Optional[str] = None suggestion_text: str = "" - def verdict(self, req: "_Request", *, keep_suffix: bool = False) -> Optional[dict[str, Any]]: - """Accept on exact, auto-correct on a near-typo (re-attaching a preserved ``@preset/`` - suffix when *keep_suffix*), else None so the branch composes its own message.""" - if self.exact: - return _accept() - if self.corrected: - corrected = req.with_preset_suffix(self.corrected) if keep_suffix else self.corrected - return _corrected(req.requested, corrected) - return None + def verdict(self, req: "_Request") -> Optional[dict[str, Any]]: + """Accept on exact membership, else None so the branch composes its own message.""" + return _accept() if self.exact else None def _match_in_catalog( @@ -76,12 +59,15 @@ def _match_in_catalog( candidates, *, case_insensitive: bool = False, - auto_correct: bool = True, suggest_query: Optional[str] = None, suggest_cutoff: float = 0.5, suggest_label: str = "Similar models", ) -> _Match: - """Shared ladder: exact membership → typo auto-correct (cutoff .9) → suggestion text. + """Shared ladder: exact membership → suggestion text. Never rewrites the id: a requested model + that is merely CLOSE to a catalog entry is the user's selection (a newer release the listing + lacks, a dated snapshot, a qualifier) and goes to the wire verbatim — fuzzy "auto-correction" + swapped `deepseek-v4.1-flash` for `deepseek-v4-flash`, `gemini-3.8-flash` for `gemini-3.6-flash` + and `model:nitro` for `model` under the user's own label. The vendor's 400 names the valid ids. ``case_insensitive`` matches lower-cased ids and maps results back to the catalog's spelling (MiniMax ships mixed-case ids). ``suggest_query`` overrides the string the suggestion search uses (some branches search on the raw request, not the lookup form).""" @@ -99,9 +85,6 @@ def _match_in_catalog( if query in set(pool): return _Match(exact=True) - auto = get_close_matches(query, pool, n=1, cutoff=0.9) if auto_correct else [] - if auto: - return _Match(corrected=_show(auto[0])) suggestions = get_close_matches(suggest_query, pool, n=3, cutoff=suggest_cutoff) if not suggestions: return _Match() @@ -120,11 +103,6 @@ class _Request: base_url: Optional[str] api_mode: Optional[str] headers: Optional[dict[str, str]] - preset_suffix: str = "" - - def with_preset_suffix(self, model_id: str) -> str: - """Re-attach a preserved ``@preset/`` suffix after auto-correction.""" - return f"{model_id}{self.preset_suffix}" # ── Provider branches (None = not decided here) ───────────────────────── @@ -151,7 +129,7 @@ def _reject_whitespace(req: _Request) -> Optional[dict[str, Any]]: def _parse_openrouter_preset(req: _Request) -> Optional[dict[str, Any]]: """OpenRouter presets are account-scoped, so ``@preset/`` never appears in the public /v1/models listing. A bare preset is accepted unverified; ``@preset/`` validates - the base model and preserves the suffix through auto-correction. OpenRouter validates the slug + the base model; the full id (suffix included) goes to the wire. OpenRouter validates the slug at request time.""" marker = "@preset/" if marker not in req.requested: @@ -163,7 +141,6 @@ def _parse_openrouter_preset(req: _Request) -> Optional[dict[str, Any]]: if re.fullmatch(r"[A-Za-z0-9._~-]+", preset_slug) is None: return _reject("OpenRouter preset slugs must be non-empty URL-safe identifiers using only " "letters, digits, '.', '_', '~', or '-'.") - req.preset_suffix = f"{marker}{preset_slug}" if not preset_base: return _soft_accept(None) req.lookup = preset_base @@ -235,7 +212,7 @@ def _validate_ollama_native(req: _Request) -> Optional[dict[str, Any]]: f"Note: could not reach this Ollama endpoint's `/api/tags` model listing to validate `{req.requested}`. " "Hermes will save the model name, but local Ollama model discovery could not verify it." ) - match = _match_in_catalog(req.lookup, models, auto_correct=False, suggest_label="Similar local Ollama models") + match = _match_in_catalog(req.lookup, models, suggest_label="Similar local Ollama models") if match.exact: return _accept() empty_hint = " No models are currently listed by `/api/tags`." if not models else "" @@ -318,8 +295,7 @@ def _validate_static_catalog(req: _Request) -> Optional[dict[str, Any]]: # hidden provider slug — soft-accepting one silently runs at 272K on a different model. if req.lookup.strip().lower().endswith(CODEX_CONTEXT_VARIANT_SUFFIX) and req.lookup not in set(catalog): if is_codex_context_variant(req.lookup): - # Valid variant a stale catalog hasn't synthesized yet. Accept directly — the typo - # auto-corrector would otherwise "fix" it to the base slug and drop the opt-in. + # Valid variant a stale catalog hasn't synthesized yet. return _accept() base_guess = req.lookup[: -len(CODEX_CONTEXT_VARIANT_SUFFIX)] return _reject( @@ -433,17 +409,12 @@ def _validate_live_listing(req: _Request) -> Optional[dict[str, Any]]: if match.exact: return _accept() # OpenRouter routing variants (":nitro", ":floor", ...) are request-time modifiers, not - # catalog entries — validate the BASE but keep the suffixed id. Must run BEFORE fuzzy - # auto-correction, which would otherwise "correct" `model:nitro` → `model` and silently - # strip the routing opt-in. + # catalog entries — validate the BASE but keep the suffixed id. variant_base = openrouter_variant_base(req.lookup) if req.normalized == "openrouter" else None if variant_base is not None and variant_base in set(api_models): return _accept() # Listed but not found: the account may reach models absent from the public listing # (e.g. Z.AI Pro/Max plans use glm-5 on coding endpoints) — warn but allow where plausible. - verdict = match.verdict(req, keep_suffix=True) - if verdict is not None: - return verdict # Curated-catalog soft-accept: providers omit valid models from live listings (stale cache, # partial rollout, gated previews). EXCEPTION: official OpenAI hosts (canonical + data- # residency regional) — their listing is access-scoped and authoritative, so an absent model @@ -476,7 +447,7 @@ def _validate_bedrock(req: _Request) -> Optional[dict[str, Any]]: region = resolve_bedrock_runtime_region() discovered_ids = {m["id"] for m in discover_bedrock_models(region)} - match = _match_in_catalog(req.requested, list(discovered_ids), auto_correct=False, suggest_cutoff=0.4) + match = _match_in_catalog(req.requested, list(discovered_ids), suggest_cutoff=0.4) if match.exact: return _accept() # Still accept (custom inference profiles / cross-account access), but warn. @@ -508,7 +479,7 @@ def _validate_catalog_fallback(req: _Request) -> dict[str, Any]: variant_base = openrouter_variant_base(req.lookup) if variant_base is not None and variant_base.lower() in {m.lower() for m in catalog}: return _accept() - return match.verdict(req, keep_suffix=True) or _soft_accept( + return _soft_accept( f"Note: `{req.requested}` was not found in the {label} curated catalog " f"and the /models endpoint was unreachable.{match.suggestion_text}" f"\n The model may still work if it exists on the provider." @@ -558,7 +529,8 @@ def validate_requested_model( ) -> dict[str, Any]: """Validate a ``/model`` value for the active provider → dict with ``accepted`` (switch now), ``persist`` (safe to save to config), ``recognized`` (matched a known provider catalog), - ``message`` (optional warning / guidance) and ``corrected_model`` when a typo was fixed.""" + ``message`` (optional warning / guidance). The requested id is never rewritten: what the user + selected is what the wire sees.""" from hermes_cli import models as _m requested = (model_name or "").strip() diff --git a/tests/hermes_cli/test_model_switch_user_provider_slug_verbatim.py b/tests/hermes_cli/test_model_switch_user_provider_slug_verbatim.py new file mode 100644 index 0000000000..72dc418620 --- /dev/null +++ b/tests/hermes_cli/test_model_switch_user_provider_slug_verbatim.py @@ -0,0 +1,66 @@ +"""A model the user selected on their own ``providers.`` endpoint survives ``/model`` +verbatim — whether the slug arrives as ``custom:`` or as the bare config key the Desktop +picker rows carry. Its ``/v1/models`` listing is a hint: an id it lacks (a newer release, a dated +snapshot) is soft-accepted, never rejected or swapped for a listed sibling (the Desktop picker +kept "going back to deepseek 0731" because the bare-key spelling fell into the built-in +live-listing branch, whose near-miss auto-correct rewrote ``deepseek-v4.1-flash``). + +Loopback ``/v1/models`` server; no mocks on the validation chain. +""" + +import json +import os +import threading +from http.server import BaseHTTPRequestHandler, HTTPServer +from pathlib import Path + +import pytest + +from hermes_cli.model_switch import switch_model + +LISTING = ["deepseek-v4-flash-0731", "deepseek-v4-flash", "deepseek-v4-pro"] + + +class _Listing(BaseHTTPRequestHandler): + def do_GET(self): + body = json.dumps({"data": [{"id": m} for m in LISTING]}).encode() + self.send_response(200) + self.send_header("Content-Type", "application/json") + self.end_headers() + self.wfile.write(body) + + def log_message(self, format, *args): # noqa: A002 + pass + + +@pytest.fixture +def endpoint(monkeypatch): + """Loopback listing + the ``providers.hyper`` block on disk (credential resolution reads + config.yaml, exactly as the gateway does).""" + srv = HTTPServer(("127.0.0.1", 0), _Listing) + threading.Thread(target=srv.serve_forever, daemon=True).start() + base_url = f"http://127.0.0.1:{srv.server_port}/v1" + monkeypatch.setenv("HYPER_KEY", "test-key-12345") + (Path(os.environ["HERMES_HOME"]) / "config.yaml").write_text( + "model:\n provider: custom:hyper\n default: deepseek-v4-flash-0731\n" + f"providers:\n hyper:\n base_url: {base_url}\n api_key_env: HYPER_KEY\n") + try: + yield base_url + finally: + srv.shutdown() + + +@pytest.mark.parametrize("explicit_provider", ["hyper", "custom:hyper"]) +def test_unlisted_id_on_user_provider_is_kept_verbatim(endpoint, explicit_provider): + from hermes_cli.config import get_compatible_custom_providers, load_config + + cfg = load_config() + result = switch_model( + raw_input="deepseek-v4.1-flash", explicit_provider=explicit_provider, + current_provider="custom:hyper", current_model=LISTING[0], + current_base_url=endpoint, current_api_key="test-key-12345", + user_providers=cfg["providers"], custom_providers=get_compatible_custom_providers(cfg)) + assert result.success is True, result.error_message + assert result.new_model == "deepseek-v4.1-flash" + assert result.base_url == endpoint + assert "not found" in result.warning_message # warned, not rewritten diff --git a/tests/hermes_cli/test_model_validation.py b/tests/hermes_cli/test_model_validation.py index 88381f13e8..6f92a42c8d 100644 --- a/tests/hermes_cli/test_model_validation.py +++ b/tests/hermes_cli/test_model_validation.py @@ -400,11 +400,13 @@ class TestValidateFormatChecks: class TestValidateApiNotFound: - def test_warning_includes_suggestions(self): + def test_not_listed_rejects_with_suggestions(self): + """A near-miss on an aggregator listing is rejected with the listed sibling offered, never + silently swapped in (the user asked for 4.5, not 4.6).""" result = _validate("anthropic/claude-opus-4.5") - assert result["accepted"] is True - # Close match auto-corrects; less similar inputs show suggestions - assert "Auto-corrected" in result["message"] or "Similar models" in result["message"] + assert result["accepted"] is False + assert "corrected_model" not in result + assert "anthropic/claude-opus-4.6" in result["message"] # -- validate — API unreachable — soft-accept via catalog or warning -------- @@ -472,31 +474,32 @@ class TestValidateApiFallback: -# -- validate — Codex auto-correction ------------------------------------------ +# -- validate — the requested id is never rewritten ----------------------------- -class TestValidateCodexAutoCorrection: - """Auto-correction for typos on openai-codex provider.""" +class TestRequestedIdIsNeverRewritten: + """A selected id that is merely CLOSE to a catalog entry is the user's choice (a newer release, + a dated snapshot, a qualifier), never a typo to "fix": the verdict may warn or reject, but no + branch may return a different model under the user's label.""" - def test_missing_dash_auto_corrects(self): - """gpt5.3-codex (missing dash) auto-corrects to gpt-5.3-codex.""" - codex_models = ["gpt-5.4-mini", "gpt-5.4", "gpt-5.3-codex", - "gpt-5.2-codex", "gpt-5.1-codex-max"] - with patch("hermes_cli.models.provider_model_ids", return_value=codex_models): - result = validate_requested_model("gpt5.3-codex", "openai-codex") - assert result["accepted"] is True - assert result["recognized"] is True - assert result["corrected_model"] == "gpt-5.3-codex" - assert "Auto-corrected" in result["message"] + @pytest.mark.parametrize("requested, listing", [ + ("deepseek-v4.1-flash", ["deepseek-v4-flash-0731", "deepseek-v4-flash"]), # custom endpoint (#mao) + ("gemini-3.8-flash", ["gemini-3.6-flash", "gemini-3.6-pro"]), # version bump (#101975) + ("gpt5.3-codex", ["gpt-5.4", "gpt-5.3-codex"]), # genuine typo + ]) + def test_live_listing_near_miss_keeps_requested_id(self, requested, listing): + for provider, base_url in (("custom:hyper", "http://127.0.0.1:1/v1"), ("openrouter", None)): + result = _validate(requested, provider, api_models=listing, base_url=base_url) + assert "corrected_model" not in result + assert result["recognized"] is False + assert "Similar models" in (result["message"] or "") or listing[-1] in (result["message"] or "") - def test_exact_match_no_correction(self): - """Exact model name does not trigger auto-correction.""" + def test_static_catalog_near_miss_keeps_requested_id(self): codex_models = ["gpt-5.4-mini", "gpt-5.4", "gpt-5.3-codex"] with patch("hermes_cli.models.provider_model_ids", return_value=codex_models): - result = validate_requested_model("gpt-5.3-codex", "openai-codex") - assert result["accepted"] is True - assert result["recognized"] is True - assert result.get("corrected_model") is None - assert result["message"] is None + result = validate_requested_model("gpt5.3-codex", "openai-codex") + assert "corrected_model" not in result + assert result["recognized"] is False + assert "gpt-5.3-codex" in result["message"] # offered as a suggestion, not applied class TestValidateCodex900kVariants: diff --git a/tests/hermes_cli/test_openrouter_preset_validation.py b/tests/hermes_cli/test_openrouter_preset_validation.py index a7e280221b..a5a19b005b 100644 --- a/tests/hermes_cli/test_openrouter_preset_validation.py +++ b/tests/hermes_cli/test_openrouter_preset_validation.py @@ -75,7 +75,9 @@ def test_combined_openrouter_preset_reference_rejects_unknown_base_model(): assert "openai/gpt-5.4" in result["message"] -def test_combined_openrouter_preset_reference_preserves_suffix_on_autocorrect(): +def test_combined_preset_near_miss_base_is_not_rewritten(): + """A base model close to a listed id is the user's pick, not a typo — the verdict rejects with a + suggestion instead of swapping the model under the preset.""" with patch("hermes_cli.models.fetch_api_models", return_value=["openai/gpt-5.4"]): result = validate_requested_model( "openai/gpt-5.44@preset/email-copywriter", @@ -84,31 +86,9 @@ def test_combined_openrouter_preset_reference_preserves_suffix_on_autocorrect(): base_url="https://openrouter.ai/api/v1", ) - corrected = "openai/gpt-5.4@preset/email-copywriter" - assert result["accepted"] is True - assert result["corrected_model"] == corrected - assert corrected in result["message"] - - -def test_combined_preset_preserves_suffix_on_catalog_autocorrect(): - with ( - patch("hermes_cli.models.fetch_api_models", return_value=None), - patch( - "hermes_cli.models.provider_model_ids", - return_value=["openai/gpt-5.4"], - ), - ): - result = validate_requested_model( - "openai/gpt-5.44@preset/email-copywriter", - "openrouter", - api_key="key", - base_url="https://openrouter.ai/api/v1", - ) - - corrected = "openai/gpt-5.4@preset/email-copywriter" - assert result["accepted"] is True - assert result["corrected_model"] == corrected - assert corrected in result["message"] + assert result["accepted"] is False + assert "corrected_model" not in result + assert "openai/gpt-5.4" in result["message"] @pytest.mark.parametrize(