From bfd0a9d6a1b24d587cbabf9c3d77b194fe2f25c7 Mon Sep 17 00:00:00 2001 From: keyarr Date: Fri, 31 Jul 2026 20:38:20 -0300 Subject: [PATCH 1/9] =?UTF-8?q?fix(model-picker):=20eliminate=20O(n=C2=B2)?= =?UTF-8?q?=20catalog=20rebuild=20lag=20in=20/model?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit getModelOptions() ran an O(n²) optionMatchesModel loop (catalog scan per option) plus an O(n²) duplicate-apiName filter, costing ~43ms per call on catalogs with hundreds of models (e.g. Fireworks' ~280 entries). The picker also rebuilt the full options list on every keystroke via isGenuineSwitchProfileValue, so arrow-key navigation lagged badly. - hoist catalog lookup out of the per-option loop (hasOptionValue) - precompute duplicate apiNames into a Set - short-circuit isGenuineSwitchProfileValue for non-switch values getModelOptions(): 43.5ms -> 2.0ms on a 277-entry catalog --- src/components/ModelPicker.tsx | 12 ++++++- src/utils/model/modelOptions.ts | 60 +++++++++++++++++++++++++-------- 2 files changed, 57 insertions(+), 15 deletions(-) diff --git a/src/components/ModelPicker.tsx b/src/components/ModelPicker.tsx index fe8d99b775..f6c42f5bdf 100644 --- a/src/components/ModelPicker.tsx +++ b/src/components/ModelPicker.tsx @@ -11,7 +11,7 @@ import { useAppState, useSetAppState } from '../state/AppState.js'; import { convertEffortValueToLevel, type EffortLevel, getAvailableEffortLevels, getDefaultEffortForModel, modelSupportsEffort, modelSupportsMaxEffort, resolvePickerEffortPersistence, toPersistableEffort } from '../utils/effort.js'; import { isModelAllowed } from '../utils/model/modelAllowlist.js'; import { getDefaultMainLoopModel, type ModelSetting, modelDisplayString, parseUserSpecifiedModel } from '../utils/model/model.js'; -import { getModelOptions, type ModelOption, parseSwitchProfileValue, resolveSelectedSwitchProfileId } from '../utils/model/modelOptions.js'; +import { getModelOptions, SWITCH_PROFILE_VALUE_PREFIX, type ModelOption, parseSwitchProfileValue, resolveSelectedSwitchProfileId } from '../utils/model/modelOptions.js'; import { getSettingsForSource, updateSettingsForSource } from '../utils/settings/settings.js'; import { ConfigurableShortcutHint } from './ConfigurableShortcutHint.js'; import { Select } from './CustomSelect/index.js'; @@ -496,7 +496,17 @@ function _temp(s) { // options share the value (a literal id colliding with an encoded switch // value), the match is ambiguous, so require exactly one option and treat that // lone option's marker as authoritative. +// +// Values that don't start with the switch prefix can never match a switch +// option (switch values are always prefix-encoded), so short-circuit before +// rebuilding the full getModelOptions() list — this runs on every render and +// every focus change while the picker is open, and the rebuild is the dominant +// cost with large model catalogs (e.g. hundreds of discovered/catalogued +// models). function isGenuineSwitchProfileValue(value: string): boolean { + if (!value.startsWith(SWITCH_PROFILE_VALUE_PREFIX)) { + return false + } return resolveSelectedSwitchProfileId(getModelOptions(), value) !== undefined; } function resolveOptionModel(value?: string): string | undefined { diff --git a/src/utils/model/modelOptions.ts b/src/utils/model/modelOptions.ts index b40f243717..7a4cfe6048 100644 --- a/src/utils/model/modelOptions.ts +++ b/src/utils/model/modelOptions.ts @@ -938,11 +938,32 @@ function mergeModelOptionsByNormalizedValue( return merged } -function getCatalogOptionValue(entry: { id: string; apiName: string }, entries: readonly { apiName: string }[]): string { +/** + * ApiNames that appear more than once in the catalog (case-insensitive, after + * trimming). Computed once up front so `getCatalogOptionValue` is O(1) per + * entry instead of re-scanning the whole catalog for every entry — catalogs + * with hundreds of models (e.g. Fireworks) would otherwise make this O(n²). + */ +function getDuplicateCatalogApiNames( + entries: readonly { apiName: string }[], +): Set { + const counts = new Map() + for (const entry of entries) { + const key = entry.apiName.trim().toLowerCase() + counts.set(key, (counts.get(key) ?? 0) + 1) + } + const duplicates = new Set() + for (const [key, count] of counts) { + if (count > 1) { + duplicates.add(key) + } + } + return duplicates +} + +function getCatalogOptionValue(entry: { id: string; apiName: string }, duplicateApiNames: Set): string { const apiName = entry.apiName.trim() - const duplicateApiName = entries.filter(candidate => - candidate.apiName.trim().toLowerCase() === apiName.toLowerCase(), - ).length > 1 + const duplicateApiName = duplicateApiNames.has(apiName.toLowerCase()) return duplicateApiName ? entry.id.trim() : apiName } @@ -953,8 +974,9 @@ function getActiveOpenAIRouteCatalogOptions(): ModelOption[] { } const entries = getCatalogEntriesForRoute(routeId) + const duplicateApiNames = getDuplicateCatalogApiNames(entries) return entries.flatMap(entry => { - const value = getCatalogOptionValue(entry, entries) + const value = getCatalogOptionValue(entry, duplicateApiNames) if (!value) { return [] } @@ -995,19 +1017,29 @@ function getRouteCatalogModelOption(value: ModelSetting): ModelOption | null { } return { - value: getCatalogOptionValue(catalogEntry, entries), + value: getCatalogOptionValue(catalogEntry, getDuplicateCatalogApiNames(entries)), label: catalogEntry.label ?? catalogEntry.apiName, description: catalogEntry.apiName, } } -function optionMatchesModel(option: ModelOption, model: ModelSetting): boolean { - if (option.value === model) { +/** + * True when `value` (or its canonical route-catalog entry) already appears in + * `options`. The catalog lookup is hoisted out of the per-option loop — with + * large static catalogs (e.g. Fireworks' ~280 entries) the previous + * `options.some(optionMatchesModel)` re-ran the catalog scan for every option, + * making each `getModelOptions()` call O(n²) and the `/model` picker lag on + * every keystroke. + */ +function hasOptionValue(options: ModelOption[], value: ModelSetting): boolean { + if (options.some(option => option.value === value)) { return true } - - const catalogOption = getRouteCatalogModelOption(model) - return catalogOption !== null && option.value === catalogOption.value + const catalogOption = getRouteCatalogModelOption(value) + return ( + catalogOption !== null && + options.some(option => option.value === catalogOption.value) + ) } export function getModelOptions(fastMode = false): ModelOption[] { @@ -1021,7 +1053,7 @@ export function getModelOptions(fastMode = false): ModelOption[] { const envCustomModel = process.env.ANTHROPIC_CUSTOM_MODEL_OPTION if ( envCustomModel && - !options.some(existing => optionMatchesModel(existing, envCustomModel)) + !hasOptionValue(options, envCustomModel) ) { options.push({ value: envCustomModel, @@ -1037,7 +1069,7 @@ export function getModelOptions(fastMode = false): ModelOption[] { const catalogOption = getRouteCatalogModelOption(opt.value) const nextOption = catalogOption ? { ...opt, ...catalogOption } : opt if ( - !options.some(existing => optionMatchesModel(existing, nextOption.value)) + !hasOptionValue(options, nextOption.value) ) { options.push(nextOption) } @@ -1055,7 +1087,7 @@ export function getModelOptions(fastMode = false): ModelOption[] { } if ( customModel === null || - options.some(opt => optionMatchesModel(opt, customModel)) + hasOptionValue(options, customModel) ) { return filterModelOptionsByAllowlist(options) } else if (customModel === 'opusplan') { From 724bea5cd727cc1bbc43b6b0445012bdaae216dd Mon Sep 17 00:00:00 2001 From: keyarr Date: Fri, 31 Jul 2026 22:24:12 -0300 Subject: [PATCH 2/9] perf(model-options): share route-catalog context across getModelOptions checks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit getRouteCatalogModelOption re-resolved the active route, fetched the catalog entries, and rebuilt the duplicate-apiName set on every call — getModelOptions() invoked it up to 3x per build (env custom model, each scoped additional option, active custom model + fallback), so large catalogs (Fireworks ~280 entries) paid O(n) context rebuilds repeatedly. Build the RouteCatalogContext lazily once per options build and pass it through findRouteCatalogOption/hasOptionValue. Behavior unchanged; the catalog-miss path measures 6.1ms -> 4.1ms on a 277-entry catalog. --- src/utils/model/modelOptions.ts | 74 ++++++++++++++++++++++++++------- 1 file changed, 60 insertions(+), 14 deletions(-) diff --git a/src/utils/model/modelOptions.ts b/src/utils/model/modelOptions.ts index 7a4cfe6048..e5efc0d0a4 100644 --- a/src/utils/model/modelOptions.ts +++ b/src/utils/model/modelOptions.ts @@ -989,23 +989,45 @@ function getActiveOpenAIRouteCatalogOptions(): ModelOption[] { }) } -function getRouteCatalogModelOption(value: ModelSetting): ModelOption | null { - if (typeof value !== 'string') { - return null - } +/** + * Route-catalog lookup state for one `getModelOptions()` build. Resolving the + * route id, fetching its catalog entries, and computing the duplicate-apiName + * set are all O(catalog size) — rebuilding them for every check would make + * builds with several catalog lookups (env custom model, each scoped + * additional option, the active custom model) repeatedly rescan the whole + * catalog. Built once per options build and shared across all lookups. + */ +type RouteCatalogContext = { + entries: ReturnType + duplicateApiNames: Set +} +function getRouteCatalogContext(): RouteCatalogContext | null { const routeId = getActiveOpenAIRouteId() if (!routeId) { return null } + const entries = getCatalogEntriesForRoute(routeId) + return { + entries, + duplicateApiNames: getDuplicateCatalogApiNames(entries), + } +} + +function findRouteCatalogOption( + context: RouteCatalogContext | null, + value: ModelSetting, +): ModelOption | null { + if (!context || typeof value !== 'string') { + return null + } const normalizedValue = normalizeRouteModelOptionKey(value) if (!normalizedValue) { return null } - const entries = getCatalogEntriesForRoute(routeId) - const catalogEntry = entries.find(entry => + const catalogEntry = context.entries.find(entry => normalizeRouteModelOptionKey(entry.apiName) === normalizedValue || normalizeRouteModelOptionKey(entry.id) === normalizedValue || (entry.aliases ?? []).some( @@ -1017,7 +1039,7 @@ function getRouteCatalogModelOption(value: ModelSetting): ModelOption | null { } return { - value: getCatalogOptionValue(catalogEntry, getDuplicateCatalogApiNames(entries)), + value: getCatalogOptionValue(catalogEntry, context.duplicateApiNames), label: catalogEntry.label ?? catalogEntry.apiName, description: catalogEntry.apiName, } @@ -1031,11 +1053,15 @@ function getRouteCatalogModelOption(value: ModelSetting): ModelOption | null { * making each `getModelOptions()` call O(n²) and the `/model` picker lag on * every keystroke. */ -function hasOptionValue(options: ModelOption[], value: ModelSetting): boolean { +function hasOptionValue( + options: ModelOption[], + value: ModelSetting, + getRouteCatalogContext: () => RouteCatalogContext | null, +): boolean { if (options.some(option => option.value === value)) { return true } - const catalogOption = getRouteCatalogModelOption(value) + const catalogOption = findRouteCatalogOption(getRouteCatalogContext(), value) return ( catalogOption !== null && options.some(option => option.value === catalogOption.value) @@ -1049,11 +1075,25 @@ export function getModelOptions(fastMode = false): ModelOption[] { const options = getModelOptionsBase(fastMode) + // Route-catalog lookup state is built once per options build (on first use) + // and shared by every catalog check below, so repeated lookups — the env + // custom model, each scoped additional option, and the active custom model — + // don't each rescan the full catalog and rebuild the duplicate-apiName set. + let routeCatalogContextBuilt = false + let routeCatalogContext: RouteCatalogContext | null = null + const getSharedRouteCatalogContext = (): RouteCatalogContext | null => { + if (!routeCatalogContextBuilt) { + routeCatalogContextBuilt = true + routeCatalogContext = getRouteCatalogContext() + } + return routeCatalogContext + } + // Add the custom model from the ANTHROPIC_CUSTOM_MODEL_OPTION env var const envCustomModel = process.env.ANTHROPIC_CUSTOM_MODEL_OPTION if ( envCustomModel && - !hasOptionValue(options, envCustomModel) + !hasOptionValue(options, envCustomModel, getSharedRouteCatalogContext) ) { options.push({ value: envCustomModel, @@ -1066,10 +1106,13 @@ export function getModelOptions(fastMode = false): ModelOption[] { // Append additional model options fetched during bootstrap for (const opt of getScopedAdditionalModelOptions()) { - const catalogOption = getRouteCatalogModelOption(opt.value) + const catalogOption = findRouteCatalogOption( + getSharedRouteCatalogContext(), + opt.value, + ) const nextOption = catalogOption ? { ...opt, ...catalogOption } : opt if ( - !hasOptionValue(options, nextOption.value) + !hasOptionValue(options, nextOption.value, getSharedRouteCatalogContext) ) { options.push(nextOption) } @@ -1087,7 +1130,7 @@ export function getModelOptions(fastMode = false): ModelOption[] { } if ( customModel === null || - hasOptionValue(options, customModel) + hasOptionValue(options, customModel, getSharedRouteCatalogContext) ) { return filterModelOptionsByAllowlist(options) } else if (customModel === 'opusplan') { @@ -1127,7 +1170,10 @@ export function getModelOptions(fastMode = false): ModelOption[] { getMergedOpus1MOption(fastMode), ]) } else { - const catalogOption = getRouteCatalogModelOption(customModel) + const catalogOption = findRouteCatalogOption( + getSharedRouteCatalogContext(), + customModel, + ) if (catalogOption) { options.push(catalogOption) return filterModelOptionsByAllowlist(options) From 0f976ca451069d145fbb26a78186a335484d41b0 Mon Sep 17 00:00:00 2001 From: keyarr Date: Fri, 31 Jul 2026 22:42:39 -0300 Subject: [PATCH 3/9] test(model-picker): add regression tests for catalog dedup and switch-profile guard --- .../ModelPicker.switchProfile.test.ts | 246 ++++++++++++++ src/components/ModelPicker.tsx | 2 +- .../model/modelOptions.catalogDedup.test.ts | 308 ++++++++++++++++++ 3 files changed, 555 insertions(+), 1 deletion(-) create mode 100644 src/components/ModelPicker.switchProfile.test.ts create mode 100644 src/utils/model/modelOptions.catalogDedup.test.ts diff --git a/src/components/ModelPicker.switchProfile.test.ts b/src/components/ModelPicker.switchProfile.test.ts new file mode 100644 index 0000000000..12feaad11d --- /dev/null +++ b/src/components/ModelPicker.switchProfile.test.ts @@ -0,0 +1,246 @@ +import { afterEach, beforeEach, expect, mock, test } from 'bun:test' + +import { resetModelStringsForTestingOnly } from '../bootstrap/state.js' +import { acquireEnvMutex, releaseEnvMutex } from '../entrypoints/sdk/shared.js' +import { saveGlobalConfig } from '../utils/config.js' +import { + resetSettingsCache, + setSessionSettingsCache, +} from '../utils/settings/settingsCache.js' +import * as actualAuth from '../utils/auth.js' +import * as actualProviderProfiles from '../utils/providerProfiles.js' +import * as actualProviders from '../utils/model/providers.js' +import type { ProviderProfile } from '../utils/config.js' + +// Snapshot the real modules before any mock.module runs. bun live-repoints the +// `actual*` namespaces to the active mock, so these plain-object copies are the +// stable handle on the genuine implementations (2026-04-30 mock-leak lesson). +const realAuth = { ...actualAuth } +const realProviderProfiles = { ...actualProviderProfiles } +const realProviders = { ...actualProviders } + +// bun's mock.module is process-wide and mock.restore() does NOT undo it, so +// each mock is gated and falls through to the real implementation whenever +// activeProfilesOverride is null — a transparent passthrough for every other +// suite in the same `bun test` run. +let activeProfilesOverride: Partial | null = null + +mock.module('../utils/model/providers.js', () => ({ + ...realProviders, + getAPIProvider: () => + activeProfilesOverride ? 'openai' : realProviders.getAPIProvider(), + getAPIProviderForStatsig: () => + activeProfilesOverride + ? 'openai' + : realProviders.getAPIProviderForStatsig(), + isFirstPartyAnthropicBaseUrl: (...args: Parameters) => + activeProfilesOverride + ? false + : realProviders.isFirstPartyAnthropicBaseUrl(...args), + isGithubNativeAnthropicMode: (...args: Parameters) => + activeProfilesOverride + ? false + : realProviders.isGithubNativeAnthropicMode(...args), + usesAnthropicAccountFlow: (...args: Parameters) => + activeProfilesOverride + ? false + : realProviders.usesAnthropicAccountFlow(...args), +})) + +mock.module('../utils/auth.js', () => ({ + ...realAuth, + isClaudeAISubscriber: (...args: Parameters) => + activeProfilesOverride ? false : realAuth.isClaudeAISubscriber(...args), + isMaxSubscriber: (...args: Parameters) => + activeProfilesOverride ? false : realAuth.isMaxSubscriber(...args), + isTeamPremiumSubscriber: (...args: Parameters) => + activeProfilesOverride + ? false + : realAuth.isTeamPremiumSubscriber(...args), +})) + +mock.module('../utils/providerProfiles.js', () => ({ + ...realProviderProfiles, + getProviderProfiles: (...args: Parameters) => + (activeProfilesOverride?.getProviderProfiles ?? + realProviderProfiles.getProviderProfiles)(...args), + getActiveProviderProfile: (...args: Parameters) => + (activeProfilesOverride?.getActiveProviderProfile ?? + realProviderProfiles.getActiveProviderProfile)(...args), + getProfileModelOptions: (...args: Parameters) => + (activeProfilesOverride?.getProfileModelOptions ?? + realProviderProfiles.getProfileModelOptions)(...args), +})) + +// ModelPicker imports getModelOptions from modelOptions.js, whose providers / +// auth / providerProfiles bindings are resolved at ITS load time. Fresh-import +// modelOptions (with nonce) so those imports hit the gated mocks above, then +// mock modelOptions for the fresh ModelPicker import so its isGenuineSwitch- +// ProfileValue is bound to the fresh instance. +async function importFreshModelPicker( + profilesMock: Partial, +) { + activeProfilesOverride = profilesMock + const nonce = `${Date.now()}-${Math.random()}` + const modelOptionsModule = await import( + `../utils/model/modelOptions.js?switchProfile=${nonce}` + ) + mock.module('../utils/model/modelOptions.js', () => modelOptionsModule) + return import(`./ModelPicker.js?switchProfile=${nonce}`) +} + +function buildProfileFixture( + overrides: Partial = {}, +): ProviderProfile { + return { + id: 'profile_default', + name: 'Default Profile', + provider: 'openai', + baseUrl: 'https://api.example.com/v1', + model: 'example-model', + apiKey: 'sk-example', + ...overrides, + } +} + +const originalEnv = { + CLAUDE_CODE_PROVIDER_PROFILE_ENV_APPLIED: + process.env.CLAUDE_CODE_PROVIDER_PROFILE_ENV_APPLIED, + CLAUDE_CODE_USE_OPENAI: process.env.CLAUDE_CODE_USE_OPENAI, + OPENAI_BASE_URL: process.env.OPENAI_BASE_URL, + OPENAI_MODEL: process.env.OPENAI_MODEL, + OPENAI_API_KEY: process.env.OPENAI_API_KEY, +} + +function restoreEnvValue(key: keyof typeof originalEnv): void { + const value = originalEnv[key] + if (value === undefined) { + delete process.env[key] + } else { + process.env[key] = value + } +} + +beforeEach(async () => { + await acquireEnvMutex() + mock.restore() + activeProfilesOverride = null + setSessionSettingsCache({ settings: {}, errors: [] }) + for (const key of Object.keys(originalEnv) as (keyof typeof originalEnv)[]) { + delete process.env[key] + } + resetModelStringsForTestingOnly() + saveGlobalConfig(current => ({ + ...current, + providerProfiles: [], + activeProviderProfileId: undefined, + })) +}) + +afterEach(() => { + try { + mock.restore() + activeProfilesOverride = null + resetSettingsCache() + for (const key of Object.keys(originalEnv) as (keyof typeof originalEnv)[]) { + restoreEnvValue(key) + } + saveGlobalConfig(current => ({ + ...current, + providerProfiles: [], + activeProviderProfileId: undefined, + })) + resetModelStringsForTestingOnly() + } finally { + releaseEnvMutex() + } +}) + +test('ordinary model ids are never switch values (prefix short-circuit)', async () => { + const { isGenuineSwitchProfileValue } = await importFreshModelPicker({}) + + expect(isGenuineSwitchProfileValue('gpt-5.5')).toBe(false) + expect(isGenuineSwitchProfileValue('deepseek-chat')).toBe(false) + expect(isGenuineSwitchProfileValue('claude-sonnet-4-6')).toBe(false) +}) + +test('malformed prefixed values are not genuine switches', async () => { + const { isGenuineSwitchProfileValue } = await importFreshModelPicker({}) + + expect(isGenuineSwitchProfileValue('__switch_profile__')).toBe(false) + expect(isGenuineSwitchProfileValue('__switch_profile__:')).toBe(false) + expect(isGenuineSwitchProfileValue('__switch_profile__:only-id:')).toBe(false) +}) + +test('a switch option from the base list is a genuine switch', async () => { + const active = buildProfileFixture({ + id: 'profile_active', + name: 'Active', + model: 'kimi-k2.6', + }) + const inactive = buildProfileFixture({ + id: 'profile_inactive', + name: 'GLM', + model: 'glm-5.1', + }) + + process.env.CLAUDE_CODE_PROVIDER_PROFILE_ENV_APPLIED = '1' + try { + const { isGenuineSwitchProfileValue } = await importFreshModelPicker({ + getProviderProfiles: () => [active, inactive], + getActiveProviderProfile: () => active, + getProfileModelOptions: profile => [ + { value: profile.model, label: profile.model, description: profile.name }, + ], + }) + const { getModelOptions } = await import( + `../utils/model/modelOptions.js?switchProfile-${Date.now()}-${Math.random()}` + ) + const switchOption = getModelOptions().find( + option => option.switchToProfileId !== undefined, + ) + + expect(switchOption).toBeDefined() + expect(isGenuineSwitchProfileValue(switchOption!.value)).toBe(true) + } finally { + delete process.env.CLAUDE_CODE_PROVIDER_PROFILE_ENV_APPLIED + } +}) + +test('a prefixed value is not genuine without the marker-backed option (profile env not applied)', async () => { + const active = buildProfileFixture({ + id: 'profile_active', + name: 'Active', + model: 'kimi-k2.6', + }) + const inactive = buildProfileFixture({ + id: 'profile_inactive', + name: 'GLM', + model: 'glm-5.1', + }) + + const { isGenuineSwitchProfileValue } = await importFreshModelPicker({ + getProviderProfiles: () => [active, inactive], + getActiveProviderProfile: () => active, + getProfileModelOptions: profile => [ + { value: profile.model, label: profile.model, description: profile.name }, + ], + }) + + expect( + isGenuineSwitchProfileValue('__switch_profile__:profile_inactive:glm-5.1'), + ).toBe(false) +}) + +test('a literal custom model id that merely starts with the prefix is not decoded', async () => { + process.env.CLAUDE_CODE_USE_OPENAI = '1' + process.env.OPENAI_BASE_URL = 'https://openrouter.ai/api/v1' + process.env.OPENAI_API_KEY = 'sk-test' + process.env.OPENAI_MODEL = '__switch_profile__:ghost:model' + + const { isGenuineSwitchProfileValue } = await importFreshModelPicker({}) + + expect(isGenuineSwitchProfileValue('__switch_profile__:ghost:model')).toBe( + false, + ) +}) diff --git a/src/components/ModelPicker.tsx b/src/components/ModelPicker.tsx index f6c42f5bdf..514a9fee9d 100644 --- a/src/components/ModelPicker.tsx +++ b/src/components/ModelPicker.tsx @@ -503,7 +503,7 @@ function _temp(s) { // every focus change while the picker is open, and the rebuild is the dominant // cost with large model catalogs (e.g. hundreds of discovered/catalogued // models). -function isGenuineSwitchProfileValue(value: string): boolean { +export function isGenuineSwitchProfileValue(value: string): boolean { if (!value.startsWith(SWITCH_PROFILE_VALUE_PREFIX)) { return false } diff --git a/src/utils/model/modelOptions.catalogDedup.test.ts b/src/utils/model/modelOptions.catalogDedup.test.ts new file mode 100644 index 0000000000..07f1f3eaf7 --- /dev/null +++ b/src/utils/model/modelOptions.catalogDedup.test.ts @@ -0,0 +1,308 @@ +import { afterEach, beforeEach, expect, mock, test } from 'bun:test' + +import { resetModelStringsForTestingOnly } from '../../bootstrap/state.js' +import { acquireEnvMutex, releaseEnvMutex } from '../../entrypoints/sdk/shared.js' +import type { ModelCatalogEntry } from '../../integrations/descriptors.js' +import { saveGlobalConfig } from '../config.js' +import { + resetSettingsCache, + setSessionSettingsCache, +} from '../settings/settingsCache.js' +import * as actualIndex from '../../integrations/index.js' +import * as actualProviderConfig from '../../services/api/providerConfig.js' +import * as actualProviderProfiles from '../providerProfiles.js' +import type { ModelOption } from './modelOptions.js' + +// Snapshot the real modules before any mock.module runs (same lesson as the +// 2026-04-30 mock-leak note in lessons_learned.md — bun live-repoints the +// `actual*` namespaces to the active mock, so these copies are the stable +// handle on the genuine implementations). +const realIndex = { ...actualIndex } +const realProviderConfig = { ...actualProviderConfig } +const realProviderProfiles = { ...actualProviderProfiles } + +// bun's mock.module is process-wide and mock.restore() does NOT undo it, so +// each mock installed here is gated and falls through to the real +// implementation whenever its gate is null — a transparent passthrough for +// every other suite in the same `bun test` run. +let activeCatalogEntries: ModelCatalogEntry[] | null = null +let activeCacheScopeOverride: string | null = null +let activeProfileOverride: Partial | null = null + +mock.module('../../integrations/index.js', () => ({ + ...realIndex, + getCatalogEntriesForRoute: (...args: Parameters) => + activeCatalogEntries ?? realIndex.getCatalogEntriesForRoute(...args), +})) + +mock.module('../../services/api/providerConfig.js', () => ({ + ...realProviderConfig, + getAdditionalModelOptionsCacheScope: () => + activeCacheScopeOverride ?? + realProviderConfig.getAdditionalModelOptionsCacheScope(), +})) + +mock.module('../providerProfiles.js', () => ({ + ...realProviderProfiles, + getActiveProviderProfile: (...args: Parameters) => + (activeProfileOverride?.getActiveProviderProfile ?? + realProviderProfiles.getActiveProviderProfile)(...args), + getActiveOpenAIModelOptionsCache: () => + activeProfileOverride + ? [{ value: 'cache-model', label: 'Cached', description: 'Cached' }] + : realProviderProfiles.getActiveOpenAIModelOptionsCache(), +})) + +async function importFreshModelOptionsModule() { + mock.module('./providers.js', () => ({ + getAPIProvider: () => 'openai', + getAPIProviderForStatsig: () => 'openai', + isFirstPartyAnthropicBaseUrl: () => false, + isFirstPartyAnthropicProvider: () => false, + isCustomAnthropicProvider: () => false, + isGithubNativeAnthropicMode: () => false, + usesAnthropicAccountFlow: () => false, + })) + const nonce = `${Date.now()}-${Math.random()}` + const modelModule = await import(`./model.js?catalogDedup=${nonce}`) + mock.module('./model.js', () => modelModule) + return import(`./modelOptions.js?ts=${nonce}`) +} + +async function getRouteCatalogModelOptions( + entries: ModelCatalogEntry[], + model: string, +): Promise { + activeCatalogEntries = entries + process.env.CLAUDE_CODE_USE_OPENAI = '1' + process.env.OPENAI_BASE_URL = 'https://openrouter.ai/api/v1' + process.env.OPENAI_API_KEY = 'sk-test' + process.env.OPENAI_MODEL = model + const { getModelOptions } = await importFreshModelOptionsModule() + return getModelOptions() +} + +const originalEnv = { + CLAUDE_CODE_USE_OPENAI: process.env.CLAUDE_CODE_USE_OPENAI, + OPENAI_BASE_URL: process.env.OPENAI_BASE_URL, + OPENAI_MODEL: process.env.OPENAI_MODEL, + OPENAI_API_KEY: process.env.OPENAI_API_KEY, + ANTHROPIC_CUSTOM_MODEL_OPTION: process.env.ANTHROPIC_CUSTOM_MODEL_OPTION, + ANTHROPIC_CUSTOM_MODEL_OPTION_NAME: process.env.ANTHROPIC_CUSTOM_MODEL_OPTION_NAME, + ANTHROPIC_CUSTOM_MODEL_OPTION_DESCRIPTION: process.env.ANTHROPIC_CUSTOM_MODEL_OPTION_DESCRIPTION, +} + +function restoreEnvValue(key: keyof typeof originalEnv): void { + const value = originalEnv[key] + if (value === undefined) { + delete process.env[key] + } else { + process.env[key] = value + } +} + +function resetGlobalConfig(): void { + saveGlobalConfig(current => ({ + ...current, + additionalModelOptionsCache: [], + additionalModelOptionsCacheScope: undefined, + openaiAdditionalModelOptionsCache: [], + openaiAdditionalModelOptionsCacheByProfile: {}, + providerProfiles: [], + activeProviderProfileId: undefined, + })) +} + +beforeEach(async () => { + await acquireEnvMutex() + mock.restore() + activeCatalogEntries = null + activeCacheScopeOverride = null + activeProfileOverride = null + setSessionSettingsCache({ settings: {}, errors: [] }) + for (const key of Object.keys(originalEnv) as (keyof typeof originalEnv)[]) { + delete process.env[key] + } + resetModelStringsForTestingOnly() + resetGlobalConfig() +}) + +afterEach(() => { + try { + mock.restore() + activeCatalogEntries = null + activeCacheScopeOverride = null + activeProfileOverride = null + resetSettingsCache() + for (const key of Object.keys(originalEnv) as (keyof typeof originalEnv)[]) { + restoreEnvValue(key) + } + resetGlobalConfig() + resetModelStringsForTestingOnly() + } finally { + releaseEnvMutex() + } +}) + +test('unique apiNames stay literal: ordinary model ids keep their apiName value', async () => { + const options = await getRouteCatalogModelOptions( + [ + { id: 'a', apiName: 'foo/bar', label: 'Foo Bar' }, + { id: 'b', apiName: 'x/y' }, + ], + 'foo/bar', + ) + + expect(options.map(option => option.value)).toEqual([null, 'foo/bar', 'x/y']) + expect(options.find(option => option.value === 'foo/bar')).toMatchObject({ + label: 'Foo Bar', + description: 'foo/bar', + }) +}) + +test('duplicate apiNames resolve to catalog ids so both models stay selectable', async () => { + const options = await getRouteCatalogModelOptions( + [ + { id: 'a', apiName: 'dup/api' }, + { id: 'b', apiName: 'dup/api' }, + ], + 'a', + ) + + expect(options.map(option => option.value)).toEqual([null, 'a', 'b']) +}) + +test('case and whitespace variants of an apiName count as duplicates', async () => { + const options = await getRouteCatalogModelOptions( + [ + { id: 'a', apiName: 'Dup/Api ' }, + { id: 'b', apiName: ' dup/api' }, + ], + 'a', + ) + + expect(options.map(option => option.value)).toEqual([null, 'a', 'b']) +}) + +test('trailing whitespace alone makes an apiName duplicate', async () => { + const options = await getRouteCatalogModelOptions( + [ + { id: 'a', apiName: 'foo/bar' }, + { id: 'b', apiName: 'foo/bar ' }, + ], + 'a', + ) + + expect(options.map(option => option.value)).toEqual([null, 'a', 'b']) +}) + +test('custom model matching a catalog alias resolves to the canonical entry without duplication', async () => { + const options = await getRouteCatalogModelOptions( + [{ id: 'a', apiName: 'real/name', aliases: ['alias-name'] }], + 'alias-name', + ) + + expect(options.map(option => option.value)).toEqual([null, 'real/name']) +}) + +test('env custom model outside the catalog is appended with its env label', async () => { + process.env.ANTHROPIC_CUSTOM_MODEL_OPTION = 'my-custom' + process.env.ANTHROPIC_CUSTOM_MODEL_OPTION_NAME = 'My Custom' + process.env.ANTHROPIC_CUSTOM_MODEL_OPTION_DESCRIPTION = 'A custom endpoint' + + const options = await getRouteCatalogModelOptions( + [{ id: 'a', apiName: 'real/name' }], + 'real/name', + ) + + expect(options.find(option => option.value === 'my-custom')).toMatchObject({ + label: 'My Custom', + description: 'A custom endpoint', + }) +}) + +test('env custom model matching a catalog alias is not duplicated', async () => { + process.env.ANTHROPIC_CUSTOM_MODEL_OPTION = 'alias-name' + + const options = await getRouteCatalogModelOptions( + [{ id: 'a', apiName: 'real/name', aliases: ['alias-name'] }], + 'real/name', + ) + + expect(options.map(option => option.value)).toEqual([null, 'real/name']) +}) + +test('scoped additional options are canonicalized against the catalog and deduplicated', async () => { + activeCacheScopeOverride = 'openai:http://localhost:1234/v1:catalog-dedup' + activeProfileOverride = { + getActiveProviderProfile: () => ({ + id: 'profile_scoped', + name: 'Scoped', + provider: 'openai', + baseUrl: 'http://localhost:1234/v1', + model: 'cache-model', + apiKey: 'sk-test', + }), + } + saveGlobalConfig(current => ({ + ...current, + additionalModelOptionsCacheScope: 'openai:http://localhost:1234/v1:catalog-dedup', + additionalModelOptionsCache: [ + { value: 'dup/api', label: 'Scoped Dup', description: 'Scoped Dup' }, + { value: 'standalone', label: 'Standalone', description: 'Standalone' }, + ], + })) + process.env.CLAUDE_CODE_USE_OPENAI = '1' + process.env.OPENAI_BASE_URL = 'http://localhost:1234/v1' + process.env.OPENAI_API_KEY = 'sk-test' + process.env.OPENAI_MODEL = 'a' + + const options = await getRouteCatalogModelOptions( + [ + { id: 'a', apiName: 'dup/api' }, + { id: 'b', apiName: 'dup/api' }, + ], + 'a', + ) + + expect(options.map(option => option.value)).toEqual([ + null, + 'cache-model', + 'a', + 'b', + 'standalone', + ]) + expect(options.find(option => option.value === 'standalone')?.label).toBe( + 'Standalone', + ) +}) + +test('large catalogs with mixed duplicates resolve to ids without dropping unique models', async () => { + const entries: ModelCatalogEntry[] = [] + for (let i = 0; i < 400; i++) { + entries.push({ id: `u-${i}`, apiName: `uniq/model-${i}` }) + } + for (let i = 0; i < 50; i++) { + const apiName = `dup/model-${i}` + const variant = + i % 3 === 0 + ? ` DUP/MODEL-${i}` + : i % 3 === 1 + ? `dup/model-${i} ` + : apiName + entries.push({ id: `d-${i}-1`, apiName }) + entries.push({ id: `d-${i}-2`, apiName: variant }) + } + + const options = await getRouteCatalogModelOptions(entries, 'uniq/model-0') + + const values = options.map(option => option.value) + expect(values.length).toBe(501) + for (let i = 0; i < 50; i++) { + expect(values).toContain(`d-${i}-1`) + expect(values).toContain(`d-${i}-2`) + expect(values).not.toContain(`dup/model-${i}`) + } + expect(values).toContain('uniq/model-399') + expect(values).not.toContain('u-0') +}) From 1a270d377db818d14424b932584c180ae73cea79 Mon Sep 17 00:00:00 2001 From: keyarr Date: Fri, 31 Jul 2026 22:55:38 -0300 Subject: [PATCH 4/9] test(model-picker): gate process-wide mocks in regression tests --- .../ModelPicker.switchProfile.test.ts | 9 +++- .../model/modelOptions.catalogDedup.test.ts | 45 +++++++++++++++---- 2 files changed, 44 insertions(+), 10 deletions(-) diff --git a/src/components/ModelPicker.switchProfile.test.ts b/src/components/ModelPicker.switchProfile.test.ts index 12feaad11d..6c8b2cc466 100644 --- a/src/components/ModelPicker.switchProfile.test.ts +++ b/src/components/ModelPicker.switchProfile.test.ts @@ -8,6 +8,7 @@ import { setSessionSettingsCache, } from '../utils/settings/settingsCache.js' import * as actualAuth from '../utils/auth.js' +import * as actualModelOptions from '../utils/model/modelOptions.js' import * as actualProviderProfiles from '../utils/providerProfiles.js' import * as actualProviders from '../utils/model/providers.js' import type { ProviderProfile } from '../utils/config.js' @@ -16,6 +17,7 @@ import type { ProviderProfile } from '../utils/config.js' // `actual*` namespaces to the active mock, so these plain-object copies are the // stable handle on the genuine implementations (2026-04-30 mock-leak lesson). const realAuth = { ...actualAuth } +const realModelOptions = { ...actualModelOptions } const realProviderProfiles = { ...actualProviderProfiles } const realProviders = { ...actualProviders } @@ -85,7 +87,12 @@ async function importFreshModelPicker( const modelOptionsModule = await import( `../utils/model/modelOptions.js?switchProfile=${nonce}` ) - mock.module('../utils/model/modelOptions.js', () => modelOptionsModule) + // Gated on the profile override: when no test is actively driving the + // profiles, later suites importing modelOptions.js keep resolving to the + // real canonical module instead of a stale fresh instance. + mock.module('../utils/model/modelOptions.js', () => + activeProfilesOverride ? modelOptionsModule : realModelOptions, + ) return import(`./ModelPicker.js?switchProfile=${nonce}`) } diff --git a/src/utils/model/modelOptions.catalogDedup.test.ts b/src/utils/model/modelOptions.catalogDedup.test.ts index 07f1f3eaf7..34076d0832 100644 --- a/src/utils/model/modelOptions.catalogDedup.test.ts +++ b/src/utils/model/modelOptions.catalogDedup.test.ts @@ -11,6 +11,7 @@ import { import * as actualIndex from '../../integrations/index.js' import * as actualProviderConfig from '../../services/api/providerConfig.js' import * as actualProviderProfiles from '../providerProfiles.js' +import * as actualProviders from './providers.js' import type { ModelOption } from './modelOptions.js' // Snapshot the real modules before any mock.module runs (same lesson as the @@ -20,6 +21,7 @@ import type { ModelOption } from './modelOptions.js' const realIndex = { ...actualIndex } const realProviderConfig = { ...actualProviderConfig } const realProviderProfiles = { ...actualProviderProfiles } +const realProviders = { ...actualProviders } // bun's mock.module is process-wide and mock.restore() does NOT undo it, so // each mock installed here is gated and falls through to the real @@ -28,6 +30,37 @@ const realProviderProfiles = { ...actualProviderProfiles } let activeCatalogEntries: ModelCatalogEntry[] | null = null let activeCacheScopeOverride: string | null = null let activeProfileOverride: Partial | null = null +let activeOpenAIProvider = false + +mock.module('./providers.js', () => ({ + ...realProviders, + getAPIProvider: () => + activeOpenAIProvider ? 'openai' : realProviders.getAPIProvider(), + getAPIProviderForStatsig: () => + activeOpenAIProvider + ? 'openai' + : realProviders.getAPIProviderForStatsig(), + isFirstPartyAnthropicBaseUrl: (...args: Parameters) => + activeOpenAIProvider + ? false + : realProviders.isFirstPartyAnthropicBaseUrl(...args), + isFirstPartyAnthropicProvider: (...args: Parameters) => + activeOpenAIProvider + ? false + : realProviders.isFirstPartyAnthropicProvider(...args), + isCustomAnthropicProvider: (...args: Parameters) => + activeOpenAIProvider + ? false + : realProviders.isCustomAnthropicProvider(...args), + isGithubNativeAnthropicMode: (...args: Parameters) => + activeOpenAIProvider + ? false + : realProviders.isGithubNativeAnthropicMode(...args), + usesAnthropicAccountFlow: (...args: Parameters) => + activeOpenAIProvider + ? false + : realProviders.usesAnthropicAccountFlow(...args), +})) mock.module('../../integrations/index.js', () => ({ ...realIndex, @@ -54,15 +87,6 @@ mock.module('../providerProfiles.js', () => ({ })) async function importFreshModelOptionsModule() { - mock.module('./providers.js', () => ({ - getAPIProvider: () => 'openai', - getAPIProviderForStatsig: () => 'openai', - isFirstPartyAnthropicBaseUrl: () => false, - isFirstPartyAnthropicProvider: () => false, - isCustomAnthropicProvider: () => false, - isGithubNativeAnthropicMode: () => false, - usesAnthropicAccountFlow: () => false, - })) const nonce = `${Date.now()}-${Math.random()}` const modelModule = await import(`./model.js?catalogDedup=${nonce}`) mock.module('./model.js', () => modelModule) @@ -74,6 +98,7 @@ async function getRouteCatalogModelOptions( model: string, ): Promise { activeCatalogEntries = entries + activeOpenAIProvider = true process.env.CLAUDE_CODE_USE_OPENAI = '1' process.env.OPENAI_BASE_URL = 'https://openrouter.ai/api/v1' process.env.OPENAI_API_KEY = 'sk-test' @@ -119,6 +144,7 @@ beforeEach(async () => { activeCatalogEntries = null activeCacheScopeOverride = null activeProfileOverride = null + activeOpenAIProvider = false setSessionSettingsCache({ settings: {}, errors: [] }) for (const key of Object.keys(originalEnv) as (keyof typeof originalEnv)[]) { delete process.env[key] @@ -133,6 +159,7 @@ afterEach(() => { activeCatalogEntries = null activeCacheScopeOverride = null activeProfileOverride = null + activeOpenAIProvider = false resetSettingsCache() for (const key of Object.keys(originalEnv) as (keyof typeof originalEnv)[]) { restoreEnvValue(key) From 88cf0ae5468e7d51b58e801eda5a9c0d66aca7a4 Mon Sep 17 00:00:00 2001 From: keyarr Date: Fri, 31 Jul 2026 23:22:47 -0300 Subject: [PATCH 5/9] test(model-picker): reuse mocked modelOptions instance in switch-profile test --- .../ModelPicker.switchProfile.test.ts | 23 +++++++++---------- 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/src/components/ModelPicker.switchProfile.test.ts b/src/components/ModelPicker.switchProfile.test.ts index 6c8b2cc466..2ee0784bc7 100644 --- a/src/components/ModelPicker.switchProfile.test.ts +++ b/src/components/ModelPicker.switchProfile.test.ts @@ -93,7 +93,8 @@ async function importFreshModelPicker( mock.module('../utils/model/modelOptions.js', () => activeProfilesOverride ? modelOptionsModule : realModelOptions, ) - return import(`./ModelPicker.js?switchProfile=${nonce}`) + const pickerModule = await import(`./ModelPicker.js?switchProfile=${nonce}`) + return { ...pickerModule, modelOptionsModule } } function buildProfileFixture( @@ -193,17 +194,15 @@ test('a switch option from the base list is a genuine switch', async () => { process.env.CLAUDE_CODE_PROVIDER_PROFILE_ENV_APPLIED = '1' try { - const { isGenuineSwitchProfileValue } = await importFreshModelPicker({ - getProviderProfiles: () => [active, inactive], - getActiveProviderProfile: () => active, - getProfileModelOptions: profile => [ - { value: profile.model, label: profile.model, description: profile.name }, - ], - }) - const { getModelOptions } = await import( - `../utils/model/modelOptions.js?switchProfile-${Date.now()}-${Math.random()}` - ) - const switchOption = getModelOptions().find( + const { isGenuineSwitchProfileValue, modelOptionsModule } = + await importFreshModelPicker({ + getProviderProfiles: () => [active, inactive], + getActiveProviderProfile: () => active, + getProfileModelOptions: profile => [ + { value: profile.model, label: profile.model, description: profile.name }, + ], + }) + const switchOption = modelOptionsModule.getModelOptions().find( option => option.switchToProfileId !== undefined, ) From 0262777fde7c2b4d4bd288fb79830a1ce36f6cce Mon Sep 17 00:00:00 2001 From: keyarr Date: Sun, 2 Aug 2026 07:59:48 -0300 Subject: [PATCH 6/9] test(model-picker): prevent mock leakage --- src/utils/model/modelOptions.catalogDedup.test.ts | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/utils/model/modelOptions.catalogDedup.test.ts b/src/utils/model/modelOptions.catalogDedup.test.ts index 34076d0832..ec671026d9 100644 --- a/src/utils/model/modelOptions.catalogDedup.test.ts +++ b/src/utils/model/modelOptions.catalogDedup.test.ts @@ -12,6 +12,7 @@ import * as actualIndex from '../../integrations/index.js' import * as actualProviderConfig from '../../services/api/providerConfig.js' import * as actualProviderProfiles from '../providerProfiles.js' import * as actualProviders from './providers.js' +import * as actualModel from './model.js' import type { ModelOption } from './modelOptions.js' // Snapshot the real modules before any mock.module runs (same lesson as the @@ -22,6 +23,7 @@ const realIndex = { ...actualIndex } const realProviderConfig = { ...actualProviderConfig } const realProviderProfiles = { ...actualProviderProfiles } const realProviders = { ...actualProviders } +const realModel = { ...actualModel } // bun's mock.module is process-wide and mock.restore() does NOT undo it, so // each mock installed here is gated and falls through to the real @@ -31,6 +33,8 @@ let activeCatalogEntries: ModelCatalogEntry[] | null = null let activeCacheScopeOverride: string | null = null let activeProfileOverride: Partial | null = null let activeOpenAIProvider = false +let activeModelOverride: typeof realModel | null = null +let modelMockRegistered = false mock.module('./providers.js', () => ({ ...realProviders, @@ -89,7 +93,11 @@ mock.module('../providerProfiles.js', () => ({ async function importFreshModelOptionsModule() { const nonce = `${Date.now()}-${Math.random()}` const modelModule = await import(`./model.js?catalogDedup=${nonce}`) - mock.module('./model.js', () => modelModule) + activeModelOverride = modelModule + if (!modelMockRegistered) { + mock.module('./model.js', () => activeModelOverride ?? realModel) + modelMockRegistered = true + } return import(`./modelOptions.js?ts=${nonce}`) } @@ -145,6 +153,7 @@ beforeEach(async () => { activeCacheScopeOverride = null activeProfileOverride = null activeOpenAIProvider = false + activeModelOverride = null setSessionSettingsCache({ settings: {}, errors: [] }) for (const key of Object.keys(originalEnv) as (keyof typeof originalEnv)[]) { delete process.env[key] @@ -160,6 +169,7 @@ afterEach(() => { activeCacheScopeOverride = null activeProfileOverride = null activeOpenAIProvider = false + activeModelOverride = null resetSettingsCache() for (const key of Object.keys(originalEnv) as (keyof typeof originalEnv)[]) { restoreEnvValue(key) From 3cddbdc89f824c313f71819f44adaae2826d43c6 Mon Sep 17 00:00:00 2001 From: keyarr Date: Sun, 2 Aug 2026 10:18:53 -0300 Subject: [PATCH 7/9] test(model-picker): drop dead env overrides overwritten by catalog dedup helper OPENAI_BASE_URL and OPENAI_MODEL assigned in the scoped-cache test were immediately overwritten by getRouteCatalogModelOptions, so they never affected the exercised path. Remove the dead assignments; keep OPENAI_API_KEY to preserve the helper's auth path. --- src/utils/model/modelOptions.catalogDedup.test.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/utils/model/modelOptions.catalogDedup.test.ts b/src/utils/model/modelOptions.catalogDedup.test.ts index ec671026d9..20987e213f 100644 --- a/src/utils/model/modelOptions.catalogDedup.test.ts +++ b/src/utils/model/modelOptions.catalogDedup.test.ts @@ -290,9 +290,7 @@ test('scoped additional options are canonicalized against the catalog and dedupl ], })) process.env.CLAUDE_CODE_USE_OPENAI = '1' - process.env.OPENAI_BASE_URL = 'http://localhost:1234/v1' process.env.OPENAI_API_KEY = 'sk-test' - process.env.OPENAI_MODEL = 'a' const options = await getRouteCatalogModelOptions( [ From a69458c2b5017561db31d39fb9eb2b9c293711aa Mon Sep 17 00:00:00 2001 From: keyarr Date: Sun, 2 Aug 2026 11:00:13 -0300 Subject: [PATCH 8/9] test(model-picker): assert getModelOptions skipped for ordinary ids isGenuineSwitchProfileValue short-circuits on the switch-profile prefix, skipping the getModelOptions() rebuild for ordinary model ids. Track the gated getModelOptions binding ModelPicker captures (opt-in call-through mock in importFreshModelPicker) and assert it is never invoked for non-prefixed ids, alongside the existing false-result assertions. --- .../ModelPicker.switchProfile.test.ts | 30 +++++++++++++++++-- 1 file changed, 27 insertions(+), 3 deletions(-) diff --git a/src/components/ModelPicker.switchProfile.test.ts b/src/components/ModelPicker.switchProfile.test.ts index 2ee0784bc7..4fce2f213b 100644 --- a/src/components/ModelPicker.switchProfile.test.ts +++ b/src/components/ModelPicker.switchProfile.test.ts @@ -81,20 +81,39 @@ mock.module('../utils/providerProfiles.js', () => ({ // ProfileValue is bound to the fresh instance. async function importFreshModelPicker( profilesMock: Partial, + options: { + trackGetModelOptions?: boolean + } = {}, ) { activeProfilesOverride = profilesMock const nonce = `${Date.now()}-${Math.random()}` const modelOptionsModule = await import( `../utils/model/modelOptions.js?switchProfile=${nonce}` ) + // Wrap getModelOptions in a call-through spy when a test opts in, so the + // binding ModelPicker captures is itself tracked. Install before + // mock.module / the picker import so the live export slot is the spy. + const getModelOptionsSpy = options.trackGetModelOptions + ? mock( + (...args: Parameters) => + realModelOptions.getModelOptions(...args), + ) + : null + const gatedModelOptionsModule = getModelOptionsSpy + ? { ...modelOptionsModule, getModelOptions: getModelOptionsSpy } + : modelOptionsModule // Gated on the profile override: when no test is actively driving the // profiles, later suites importing modelOptions.js keep resolving to the // real canonical module instead of a stale fresh instance. mock.module('../utils/model/modelOptions.js', () => - activeProfilesOverride ? modelOptionsModule : realModelOptions, + activeProfilesOverride ? gatedModelOptionsModule : realModelOptions, ) const pickerModule = await import(`./ModelPicker.js?switchProfile=${nonce}`) - return { ...pickerModule, modelOptionsModule } + return { + ...pickerModule, + modelOptionsModule, + getModelOptionsSpy, + } } function buildProfileFixture( @@ -165,11 +184,16 @@ afterEach(() => { }) test('ordinary model ids are never switch values (prefix short-circuit)', async () => { - const { isGenuineSwitchProfileValue } = await importFreshModelPicker({}) + const { isGenuineSwitchProfileValue, getModelOptionsSpy } = + await importFreshModelPicker({}, { trackGetModelOptions: true }) expect(isGenuineSwitchProfileValue('gpt-5.5')).toBe(false) expect(isGenuineSwitchProfileValue('deepseek-chat')).toBe(false) expect(isGenuineSwitchProfileValue('claude-sonnet-4-6')).toBe(false) + + // Ordinary ids don't start with the switch prefix, so the short-circuit + // must skip the getModelOptions() rebuild entirely. + expect(getModelOptionsSpy).toHaveBeenCalledTimes(0) }) test('malformed prefixed values are not genuine switches', async () => { From f3ad04c8a3dc1d079b88886100ec083224205ddb Mon Sep 17 00:00:00 2001 From: keyarr Date: Mon, 3 Aug 2026 16:40:58 -0300 Subject: [PATCH 9/9] test(model-picker): address review feedback on spies, fetch bounds, and alias dedup --- .../ModelPicker.switchProfile.test.ts | 4 +- .../model/modelOptions.catalogDedup.test.ts | 64 ++++++++++++++++++- 2 files changed, 63 insertions(+), 5 deletions(-) diff --git a/src/components/ModelPicker.switchProfile.test.ts b/src/components/ModelPicker.switchProfile.test.ts index 4fce2f213b..3634601470 100644 --- a/src/components/ModelPicker.switchProfile.test.ts +++ b/src/components/ModelPicker.switchProfile.test.ts @@ -95,8 +95,8 @@ async function importFreshModelPicker( // mock.module / the picker import so the live export slot is the spy. const getModelOptionsSpy = options.trackGetModelOptions ? mock( - (...args: Parameters) => - realModelOptions.getModelOptions(...args), + (...args: Parameters) => + modelOptionsModule.getModelOptions(...args), ) : null const gatedModelOptionsModule = getModelOptionsSpy diff --git a/src/utils/model/modelOptions.catalogDedup.test.ts b/src/utils/model/modelOptions.catalogDedup.test.ts index 20987e213f..0913ab2ece 100644 --- a/src/utils/model/modelOptions.catalogDedup.test.ts +++ b/src/utils/model/modelOptions.catalogDedup.test.ts @@ -35,6 +35,7 @@ let activeProfileOverride: Partial | null = null let activeOpenAIProvider = false let activeModelOverride: typeof realModel | null = null let modelMockRegistered = false +let catalogFetchCount = 0 mock.module('./providers.js', () => ({ ...realProviders, @@ -68,8 +69,12 @@ mock.module('./providers.js', () => ({ mock.module('../../integrations/index.js', () => ({ ...realIndex, - getCatalogEntriesForRoute: (...args: Parameters) => - activeCatalogEntries ?? realIndex.getCatalogEntriesForRoute(...args), + getCatalogEntriesForRoute: (...args: Parameters) => { + if (activeCatalogEntries !== null) { + catalogFetchCount++ + } + return activeCatalogEntries ?? realIndex.getCatalogEntriesForRoute(...args) + }, })) mock.module('../../services/api/providerConfig.js', () => ({ @@ -154,6 +159,7 @@ beforeEach(async () => { activeProfileOverride = null activeOpenAIProvider = false activeModelOverride = null + catalogFetchCount = 0 setSessionSettingsCache({ settings: {}, errors: [] }) for (const key of Object.keys(originalEnv) as (keyof typeof originalEnv)[]) { delete process.env[key] @@ -170,6 +176,7 @@ afterEach(() => { activeProfileOverride = null activeOpenAIProvider = false activeModelOverride = null + catalogFetchCount = 0 resetSettingsCache() for (const key of Object.keys(originalEnv) as (keyof typeof originalEnv)[]) { restoreEnvValue(key) @@ -312,6 +319,42 @@ test('scoped additional options are canonicalized against the catalog and dedupl ) }) +test('scoped additional option aliased to a catalog entry canonicalizes to the entry without duplicating it', async () => { + activeCacheScopeOverride = 'openai:http://localhost:1234/v1:catalog-dedup' + activeProfileOverride = { + getActiveProviderProfile: () => ({ + id: 'profile_scoped', + name: 'Scoped', + provider: 'openai', + baseUrl: 'http://localhost:1234/v1', + model: 'cache-model', + apiKey: 'sk-test', + }), + } + saveGlobalConfig(current => ({ + ...current, + additionalModelOptionsCacheScope: 'openai:http://localhost:1234/v1:catalog-dedup', + additionalModelOptionsCache: [ + { value: 'alias-name', label: 'Alias Cached', description: 'Alias Cached' }, + ], + })) + process.env.CLAUDE_CODE_USE_OPENAI = '1' + process.env.OPENAI_API_KEY = 'sk-test' + + const options = await getRouteCatalogModelOptions( + [{ id: 'a', apiName: 'real/name', aliases: ['alias-name'] }], + 'real/name', + ) + + expect(options.map(option => option.value)).toEqual([ + null, + 'cache-model', + 'real/name', + ]) + expect(options.filter(option => option.value === 'real/name')).toHaveLength(1) + expect(options.some(option => option.value === 'alias-name')).toBe(false) +}) + test('large catalogs with mixed duplicates resolve to ids without dropping unique models', async () => { const entries: ModelCatalogEntry[] = [] for (let i = 0; i < 400; i++) { @@ -329,7 +372,22 @@ test('large catalogs with mixed duplicates resolve to ids without dropping uniqu entries.push({ id: `d-${i}-2`, apiName: variant }) } - const options = await getRouteCatalogModelOptions(entries, 'uniq/model-0') + activeOpenAIProvider = true + process.env.CLAUDE_CODE_USE_OPENAI = '1' + process.env.OPENAI_BASE_URL = 'https://openrouter.ai/api/v1' + process.env.OPENAI_API_KEY = 'sk-test' + process.env.OPENAI_MODEL = 'uniq/model-0' + const { getModelOptions } = await importFreshModelOptionsModule() + // The shared RouteCatalogContext eliminates per-lookup rescans: the whole + // build performs a small fixed number of catalog fetches (base options plus + // a handful of metadata/alias lookups) that never scales with catalog size — + // a per-option rescan of this 500-entry fixture would fetch 500+ times. + // Snapshot + delta are computed synchronously around the build so unrelated + // suites sharing this process can't inflate the bound. + const fetchesBefore = catalogFetchCount + activeCatalogEntries = entries + const options = getModelOptions() + expect(catalogFetchCount - fetchesBefore).toBeLessThanOrEqual(20) const values = options.map(option => option.value) expect(values.length).toBe(501)