diff --git a/src/lib/modelCapabilities.ts b/src/lib/modelCapabilities.ts index 95330b45f8..3deb72832b 100644 --- a/src/lib/modelCapabilities.ts +++ b/src/lib/modelCapabilities.ts @@ -4,7 +4,7 @@ import { } from "@omniroute/open-sse/config/providerModels.ts"; import { parseModel, resolveCanonicalProviderModel } from "@omniroute/open-sse/services/model.ts"; import { - MODEL_SPECS, + findModelSpecIdByExactOrAlias, getAuthoritativeContextWindow, getAuthoritativeProviderContextWindow, getModelSpec, @@ -285,17 +285,18 @@ function getAuthoritativeStaticContextWindow( return null; } +// #8697-adjacent: this used to rescan Object.entries(MODEL_SPECS) per candidate per +// call — the top hotspot in a full catalog-rebuild profile once the pricing-path and +// getCanonicalModelSpecId() bottlenecks were fixed. Reuses the lazy index already built +// for getCanonicalModelSpecId() (@/shared/constants/modelSpecs) instead of duplicating a +// second cache over the same static table. function getStaticSpecCanonicalModelId(modelId: string | null, rawModel: string | null) { const candidates = [modelId, rawModel].filter( (candidate): candidate is string => typeof candidate === "string" && candidate.length > 0 ); for (const candidate of candidates) { - const lower = candidate.toLowerCase(); - for (const [canonical, spec] of Object.entries(MODEL_SPECS)) { - if (canonical === "__default__") continue; - if (canonical.toLowerCase() === lower) return canonical; - if (spec.aliases?.some((alias) => alias.toLowerCase() === lower)) return canonical; - } + const hit = findModelSpecIdByExactOrAlias(candidate); + if (hit) return hit; } return null; } @@ -311,7 +312,14 @@ function stripLatestAlias(modelId: string | null): string | null { return stripped && stripped !== modelId ? stripped : null; } -function reverseModelsDevProviders(provider: string): string[] { +// #8697-adjacent: MODELS_DEV_PROVIDER_MAP is a static module constant, so the result +// of reverseModelsDevProviders() never changes for a given provider — memoized by +// provider key instead of rescanning Object.entries(MODELS_DEV_PROVIDER_MAP) on every +// call (called once per model in a catalog rebuild). Never evicted — bounded by the +// number of distinct providers ever queried (~50-100 in practice), negligible memory. +const reverseModelsDevProvidersCache = new Map(); + +function reverseModelsDevProviders(provider: string): readonly string[] { // models.dev may store capabilities under a different OmniRoute provider id // that also maps from the same upstream models.dev provider. Build reverse // candidates from MODELS_DEV_PROVIDER_MAP (e.g. openai ↔ cx). @@ -321,6 +329,9 @@ function reverseModelsDevProviders(provider: string): string[] { // list their alias (cx/cc), never the canonical id. Also probe the // provider's alias so a canonical id like "codex"/"claude" still matches // the map entries keyed only by "cx"/"cc" (#8429). + const cached = reverseModelsDevProvidersCache.get(provider); + if (cached) return cached; + const out = new Set(); const providerAlias = PROVIDER_ID_TO_ALIAS[provider] || provider; for (const [modelsDevId, omniIds] of Object.entries(MODELS_DEV_PROVIDER_MAP)) { @@ -334,7 +345,12 @@ function reverseModelsDevProviders(provider: string): string[] { for (const id of omniIds) out.add(id); } } - return [...out]; + // Frozen: the result is now shared across every future call for this provider (via + // the cache above) instead of a fresh array per call — freeze prevents an accidental + // caller mutation (e.g. .push()) from corrupting the cache for everyone else. + const result = Object.freeze([...out]); + reverseModelsDevProvidersCache.set(provider, result); + return result; } function getSyncedCapabilityForResolved( @@ -694,8 +710,7 @@ export function capThinkingBudget(input: CapabilityInput, budget: number): numbe // default to "gemini". Without this a cap learned via the executor would be // invisible to bare-model callers. Provider-qualified inputs keep their own // provider, preserving per-provider independence. - const providerForLearned = - resolved.provider ?? (modelLower.includes("gemini") ? "gemini" : null); + const providerForLearned = resolved.provider ?? (modelLower.includes("gemini") ? "gemini" : null); const learned = getLearnedThinkingCap(providerForLearned, modelId); if (learned !== null) { diff --git a/src/lib/modelMetadataRegistry.ts b/src/lib/modelMetadataRegistry.ts index 2e03a981d3..9f4e93aa2a 100644 --- a/src/lib/modelMetadataRegistry.ts +++ b/src/lib/modelMetadataRegistry.ts @@ -258,25 +258,47 @@ export function getCanonicalModelMetadata(input: { }; } +// #8697 second bottleneck (after getModelsDevPricing memoization above): findInsensitive +// rebuilt a full Object.entries() scan on every miss, twice per model (provider lookup + +// model lookup) — ~6091 models × ~180-210 entries ≈ 1.2-1.3M allocations per catalog +// rebuild. Replaced with a lowercase-key index built once per distinct object and cached +// by identity (WeakMap) — getModelsDevPricing() returns the same object reference while +// its cache is warm, so the index is reused across every resolveCatalogPricing() call in +// a rebuild instead of rebuilt per lookup. +const lowercaseIndexCache = new WeakMap>(); + +function findInsensitive(obj: Record | null | undefined, key: string): T | undefined { + if (!obj || !key) return undefined; + if (key in obj) return obj[key]; + let index = lowercaseIndexCache.get(obj); + if (!index) { + index = new Map(); + for (const [k, v] of Object.entries(obj)) { + const lowerKey = k.toLowerCase(); + // Warn once at index-build time (not per-lookup) if two keys collide + // case-insensitively — a real data-quality signal from an upstream sync (e.g. + // models.dev returning both "OpenAI" and "openai" as distinct provider keys). + // Matches the pre-fix scan's silent first-match-wins behavior, just surfaced + // instead of swallowed. + if (index.has(lowerKey)) { + console.warn( + `[modelMetadataRegistry] findInsensitive: case-insensitive key collision on "${lowerKey}" — keeping first-seen value, later one discarded` + ); + continue; + } + index.set(lowerKey, v); + } + lowercaseIndexCache.set(obj, index); + } + return index.get(key.toLowerCase()) as T | undefined; +} + function resolveCatalogPricing( provider: string | null, model: string | null ): Record | null { if (!provider || !model) return null; - const findInsensitive = ( - obj: Record | null | undefined, - key: string - ): T | undefined => { - if (!obj || !key) return undefined; - if (key in obj) return obj[key]; - const lower = key.toLowerCase(); - for (const [k, v] of Object.entries(obj)) { - if (k.toLowerCase() === lower) return v; - } - return undefined; - }; - // Prefer models.dev synced pricing when present; fall back to hardcoded defaults. try { const modelsDev = getModelsDevPricing() as Record< diff --git a/src/lib/modelsDevSync.ts b/src/lib/modelsDevSync.ts index 8c34135feb..32a6e180f6 100644 --- a/src/lib/modelsDevSync.ts +++ b/src/lib/modelsDevSync.ts @@ -18,7 +18,7 @@ */ import { getDbInstance } from "./db/core"; -import { invalidateDbCache } from "./db/readCache"; +import { invalidateDbCache, getModelCatalogCacheVersion } from "./db/readCache"; import { backupDbFile } from "./db/backup"; import { @@ -193,10 +193,25 @@ function mapCapabilityRecord(record: Record): ModelCapabilityEn }; } +// #8697: getModelsDevPricing() re-ran the SELECT + JSON.parse of ~180 blobs on +// every call — called once per catalog model (up to ~6091x) instead of once per +// request, freezing the whole server 41-54s on a cold /v1/models rebuild. +// Memoized here, invalidated via the same modelCatalogCacheVersion signal +// save/clearModelsDevPricing already bump through invalidateDbCache("pricing") — +// reusing the existing pattern (getCachedRawProviderConnections et al. in +// db/readCache.ts) instead of introducing a new invalidation mechanism. +let pricingMemo: PricingByProvider | null = null; +let pricingMemoVersion = -1; // -1: never equals a real cacheVersion (starts at 0), guarantees a miss on the first call + /** * Read synced pricing from `models_dev_pricing` namespace. */ export function getModelsDevPricing(): PricingByProvider { + const currentVersion = getModelCatalogCacheVersion(); + if (pricingMemo !== null && pricingMemoVersion === currentVersion) { + return pricingMemo; + } + const db = getDbInstance(); const rows = db .prepare("SELECT key, value FROM key_value WHERE namespace = 'models_dev_pricing'") @@ -213,6 +228,8 @@ export function getModelsDevPricing(): PricingByProvider { console.warn(`[MODELS_DEV] Corrupted pricing data for provider "${key}", skipping`); } } + pricingMemo = synced; + pricingMemoVersion = currentVersion; return synced; } @@ -354,44 +371,26 @@ export function getSyncedCapability( ): ModelCapabilityEntry | null { if (!provider || !modelId) return null; - // Fast path: every provider is in the in-memory cache, skip SQLite entirely. - if (cachedCapabilitiesLoadedAll) { - const lookupCached = (p: string) => cachedCapabilities?.[p]?.[modelId] ?? null; - const directCached = lookupCached(provider); - if (directCached) return directCached; - const fallbacks = SYNCED_CAPABILITY_FALLBACK_ALIASES[provider]; - if (fallbacks) { - for (const alt of fallbacks) { - const found = lookupCached(alt); - if (found) return found; - } - } - return null; + // #8697-adjacent: this used to hit SQLite with a per-model SELECT on every cold + // call, relying on some other caller (getSyncedCapabilities() with no args) to have + // already warmed the whole-table cache first — no such caller sits in the /v1/models + // catalog build path, so a cold rebuild ran one SQLite round-trip per model per call + // site instead of one bulk read for the whole rebuild. Self-warm here instead of + // depending on an external caller. + if (!cachedCapabilitiesLoadedAll) { + getSyncedCapabilities(); } - // Cold path: hit SQLite. Prepare the statement once, reuse for every alias. - const db = getDbInstance(); - ensureCapabilitiesTable(); - const stmt = db.prepare( - "SELECT * FROM model_capabilities WHERE provider = ? AND model_id = ? LIMIT 1" - ); - const lookupDb = (p: string): ModelCapabilityEntry | null => { - const row = stmt.get(p, modelId); - if (!row) return null; - return mapCapabilityRecord(toRecord(row)); - }; - - const direct = lookupDb(provider); - if (direct) return direct; - + const lookupCached = (p: string) => cachedCapabilities?.[p]?.[modelId] ?? null; + const directCached = lookupCached(provider); + if (directCached) return directCached; const fallbacks = SYNCED_CAPABILITY_FALLBACK_ALIASES[provider]; if (fallbacks) { for (const alt of fallbacks) { - const found = lookupDb(alt); + const found = lookupCached(alt); if (found) return found; } } - return null; } diff --git a/src/shared/constants/modelSpecs.ts b/src/shared/constants/modelSpecs.ts index 3128877453..c9f46e67d4 100644 --- a/src/shared/constants/modelSpecs.ts +++ b/src/shared/constants/modelSpecs.ts @@ -608,26 +608,83 @@ export const MODEL_SPECS: Record = { __default__: {}, }; +// #8697-adjacent: getCanonicalModelSpecId() re-scanned Object.keys/entries(MODEL_SPECS) +// up to 3 times per call (exact ci, alias ci, prefix) — the top hotspot in a full +// catalog-rebuild profile once the pricing-path bottlenecks were fixed. MODEL_SPECS is +// a static module constant (never mutated at runtime), so the lowercase index below is +// built once, lazily, on first use and never invalidated. Iteration order for the +// prefix-match candidates is preserved exactly (same Object.keys() insertion order) so +// resolution outcomes for ambiguous prefixes are unchanged. +let modelSpecIndex: { + exactCi: Map; + aliasCi: Map; + aliasExact: Map; + prefixCandidates: Array<[lowerKey: string, canonical: string]>; +} | null = null; + +function getModelSpecIndex() { + if (modelSpecIndex) return modelSpecIndex; + const exactCi = new Map(); + const aliasCi = new Map(); + const aliasExact = new Map(); + const prefixCandidates: Array<[string, string]> = []; + for (const [canonical, spec] of Object.entries(MODEL_SPECS)) { + const lowerCanonical = canonical.toLowerCase(); + if (!exactCi.has(lowerCanonical)) exactCi.set(lowerCanonical, canonical); + for (const alias of spec.aliases || []) { + const lowerAlias = alias.toLowerCase(); + if (!aliasCi.has(lowerAlias)) aliasCi.set(lowerAlias, canonical); + if (!aliasExact.has(alias)) aliasExact.set(alias, canonical); + } + if (canonical !== "__default__") prefixCandidates.push([lowerCanonical, canonical]); + } + modelSpecIndex = { exactCi, aliasCi, aliasExact, prefixCandidates }; + return modelSpecIndex; +} + +/** + * Exact + alias case-insensitive lookup only (no prefix phase) — shared by + * modelCapabilities.ts's getStaticSpecCanonicalModelId(), which tries multiple id + * candidates and never wanted prefix matching. Reuses the same lazy index as + * getCanonicalModelSpecId() below instead of each caller maintaining its own cache + * over the same static MODEL_SPECS table. + * + * Contract: returns `null` for `__default__` (never a real canonical id), for an + * unrecognized `modelId`, or for an empty string. Matching is case-insensitive on + * both the canonical id and its aliases; there is no prefix-matching phase (unlike + * getCanonicalModelSpecId() below) — callers that need prefix matching should use + * that function instead. + */ +export function findModelSpecIdByExactOrAlias(modelId: string): string | null { + const lower = modelId.toLowerCase(); + const index = getModelSpecIndex(); + const exactHit = index.exactCi.get(lower); + if (exactHit && exactHit !== "__default__") return exactHit; + const aliasHit = index.aliasCi.get(lower); + if (aliasHit && aliasHit !== "__default__") return aliasHit; + return null; +} + export function getCanonicalModelSpecId(modelId: string): string | null { if (MODEL_SPECS[modelId]) return modelId; // Case-insensitive lookups: upstream model ids are often capitalized // (e.g. "MiniMax-M2.7") while specs/aliases use lowercase ids (#3141). const lower = modelId.toLowerCase(); + const index = getModelSpecIndex(); // Exact match (case-insensitive) - for (const canonical of Object.keys(MODEL_SPECS)) { - if (canonical.toLowerCase() === lower) return canonical; - } + const exactHit = index.exactCi.get(lower); + if (exactHit) return exactHit; // Buscas por alias (case-insensitive) - for (const [canonical, spec] of Object.entries(MODEL_SPECS)) { - if (spec.aliases?.some((alias) => alias.toLowerCase() === lower)) return canonical; - } + const aliasHit = index.aliasCi.get(lower); + if (aliasHit) return aliasHit; - // Prefix matching (case-insensitive) - for (const key of Object.keys(MODEL_SPECS)) { - if (key !== "__default__" && lower.startsWith(key.toLowerCase())) return key; + // Prefix matching (case-insensitive) — same insertion-order iteration as before, + // first match wins. + for (const [lowerKey, canonical] of index.prefixCandidates) { + if (lower.startsWith(lowerKey)) return canonical; } return null; @@ -721,9 +778,12 @@ export function capThinkingBudget(modelId: string, budget: number): number { return Math.min(budget, cap); } +// #8697-adjacent: rescanned Object.entries(MODEL_SPECS) on every call, unconditionally +// once per model in a catalog rebuild — verified 1:1 call ratio (no early +// short-circuit). Case-sensitive exact match (Array.includes(), no .toLowerCase()) — +// deliberately NOT reusing the case-insensitive aliasCi index above, which would +// silently broaden matches and change behavior. export function resolveModelAlias(modelId: string): string { - for (const [canonical, spec] of Object.entries(MODEL_SPECS)) { - if (spec.aliases?.includes(modelId)) return canonical; - } - return modelId; + const hit = getModelSpecIndex().aliasExact.get(modelId); + return hit ?? modelId; } diff --git a/tests/unit/catalog-pricing-lookup-index-8697.test.ts b/tests/unit/catalog-pricing-lookup-index-8697.test.ts new file mode 100644 index 0000000000..4afab30799 --- /dev/null +++ b/tests/unit/catalog-pricing-lookup-index-8697.test.ts @@ -0,0 +1,105 @@ +import assert from "node:assert/strict"; +import { describe, it, before, after } from "node:test"; +import { enrichCatalogModelEntry } from "../../src/lib/modelMetadataRegistry.ts"; +import { + saveModelsDevPricing, + clearModelsDevPricing, + type PricingByProvider, +} from "../../src/lib/modelsDevSync.ts"; + +const PROVIDER_COUNT = 180; +const MODELS_PER_PROVIDER = 34; +const ITERATIONS = 500; + +describe("catalog pricing lookup index (#8697 second bottleneck — findInsensitive)", () => { + before(() => { + // Mixed-case keys force the case-insensitive fallback scan in + // findInsensitive() — mirrors real models.dev data where provider/model + // casing does not always match the catalog's, and a large provider count + // mirrors the ~180 synced providers from the #8697 profiling run. + const pricing: PricingByProvider = {}; + for (let p = 0; p < PROVIDER_COUNT; p++) { + const providerKey = `Provider${p}`; + pricing[providerKey] = {}; + for (let m = 0; m < MODELS_PER_PROVIDER; m++) { + pricing[providerKey][`Model${m}`] = { input: p + m * 0.01, output: p + m * 0.02 }; + } + } + pricing.Openai = { "Gpt-4o": { input: 2.5, output: 10 } }; + saveModelsDevPricing(pricing); + }); + + after(() => { + try { + clearModelsDevPricing(); + } catch { + // ignore + } + }); + + it("resolves case-insensitive pricing correctly for every provider/model pair", () => { + const entry = enrichCatalogModelEntry({ + id: "provider42/model7", + owned_by: "provider42", + root: "model7", + }); + assert.ok(entry.pricing, "pricing should resolve via case-insensitive lookup"); + assert.equal((entry.pricing as { input: number }).input, 42.07); + }); + + it("does not rescan the pricing tables per lookup (regression guard for O(providers*models) scans)", () => { + // `provider`/`gpt-4o` always resolve through the same fast metadata path + // (real registered provider) so both scenarios below pay an identical + // getCanonicalModelMetadata cost — isolating the delta to pricing + // resolution alone, independent of unrelated catalog-metadata overhead. + const entryWithPricingPreset = () => + enrichCatalogModelEntry({ + id: "openai/gpt-4o", + owned_by: "openai", + root: "gpt-4o", + pricing: { input: 1, output: 1 }, // nextEntry.pricing != null → resolveCatalogPricing() never runs + }); + const entryNeedingPricingResolution = () => + enrichCatalogModelEntry({ + id: "openai/gpt-4o", + owned_by: "openai", + root: "gpt-4o", + }); + + // Warm up (index build, module init) outside the measured window. + entryWithPricingPreset(); + entryNeedingPricingResolution(); + + const originalEntries = Object.entries; + let calls = 0; + Object.entries = function patchedEntries(...args: Parameters) { + calls++; + return originalEntries.apply(this, args as never); + } as typeof Object.entries; + + let baselineCalls: number; + let withPricingCalls: number; + try { + calls = 0; + for (let i = 0; i < ITERATIONS; i++) entryWithPricingPreset(); + baselineCalls = calls; + + calls = 0; + for (let i = 0; i < ITERATIONS; i++) entryNeedingPricingResolution(); + withPricingCalls = calls; + } finally { + Object.entries = originalEntries; + } + + const delta = withPricingCalls - baselineCalls; + // Pre-fix: findInsensitive() called Object.entries() on every miss, twice per + // lookup (provider scan + model scan) → delta ≈ 2 * ITERATIONS. Indexed O(1) + // lookup: the index is built once per distinct object and reused, so delta + // stays a small constant regardless of ITERATIONS. + assert.ok( + delta < ITERATIONS, + `expected Object.entries() call delta to stay constant (not scale with ${ITERATIONS} ` + + `iterations), got delta=${delta} — findInsensitive() may have regressed to a linear scan per lookup` + ); + }); +}); diff --git a/tests/unit/model-spec-lookup-index-8697.test.ts b/tests/unit/model-spec-lookup-index-8697.test.ts new file mode 100644 index 0000000000..24618149f2 --- /dev/null +++ b/tests/unit/model-spec-lookup-index-8697.test.ts @@ -0,0 +1,53 @@ +import assert from "node:assert/strict"; +import { describe, it } from "node:test"; +import { getCanonicalModelSpecId, getModelSpec } from "../../src/shared/constants/modelSpecs.ts"; + +describe("model spec lookup index (#8697-adjacent — getCanonicalModelSpecId)", () => { + it("still resolves case-insensitive exact matches", () => { + // Real MODEL_SPECS entries — exercised via a mixed-case id, forcing the + // case-insensitive fallback the index covers. + const canonical = getCanonicalModelSpecId("GPT-5.6"); + assert.ok( + canonical, + "expected a canonical id to resolve for a known model, case-insensitively" + ); + assert.equal(getModelSpec("GPT-5.6"), getModelSpec(canonical!)); + }); + + it("returns null for a genuinely unknown model id", () => { + assert.equal(getCanonicalModelSpecId("definitely-not-a-real-model-xyz-123"), null); + }); + + it("does not rescan MODEL_SPECS per lookup (regression guard for O(n) scans)", () => { + // Warm the lazy index outside the measured window. + getCanonicalModelSpecId("gpt-5.6"); + + const originalEntries = Object.entries; + const originalKeys = Object.keys; + let entriesCalls = 0; + let keysCalls = 0; + Object.entries = function patchedEntries(...args: Parameters) { + entriesCalls++; + return originalEntries.apply(this, args as never); + } as typeof Object.entries; + Object.keys = function patchedKeys(...args: Parameters) { + keysCalls++; + return originalKeys.apply(this, args as never); + } as typeof Object.keys; + + try { + for (let i = 0; i < 500; i++) { + getCanonicalModelSpecId("gpt-5.6"); + } + } finally { + Object.entries = originalEntries; + Object.keys = originalKeys; + } + + // Pre-fix: every miss re-ran Object.keys()/Object.entries() up to 3x per call. + // Indexed: the lazy index is built once and reused, so no further + // Object.keys/entries calls should happen at all across 500 repeated lookups. + assert.equal(entriesCalls, 0, `expected 0 Object.entries() calls, got ${entriesCalls}`); + assert.equal(keysCalls, 0, `expected 0 Object.keys() calls, got ${keysCalls}`); + }); +}); diff --git a/tests/unit/models-dev-pricing-memoization-8697.test.ts b/tests/unit/models-dev-pricing-memoization-8697.test.ts new file mode 100644 index 0000000000..9ab1c12277 --- /dev/null +++ b/tests/unit/models-dev-pricing-memoization-8697.test.ts @@ -0,0 +1,59 @@ +import assert from "node:assert/strict"; +import { describe, it, before, after, mock } from "node:test"; +import { getDbInstance } from "../../src/lib/db/core.ts"; +import { + getModelsDevPricing, + saveModelsDevPricing, + clearModelsDevPricing, + type PricingByProvider, +} from "../../src/lib/modelsDevSync.ts"; + +describe("getModelsDevPricing memoization (#8697)", () => { + before(() => { + const pricing: PricingByProvider = { + openai: { + "gpt-4o": { input: 2.5, output: 10 }, + }, + }; + saveModelsDevPricing(pricing); + }); + + after(() => { + try { + clearModelsDevPricing(); + } catch { + // ignore + } + }); + + it("hits the DB once for repeated reads within the same cache version", () => { + const db = getDbInstance(); + const prepareSpy = mock.method(db, "prepare"); + const callsBefore = prepareSpy.mock.calls.length; + + getModelsDevPricing(); + getModelsDevPricing(); + getModelsDevPricing(); + + const callsAfter = prepareSpy.mock.calls.length; + prepareSpy.mock.restore(); + + // The N+1 bug re-runs the SELECT + JSON.parse on every call — memoized, + // 3 calls should cost at most 1 real DB round-trip (0 if a prior test + // already warmed the cache at the same version). + assert.ok( + callsAfter - callsBefore <= 1, + `expected at most 1 db.prepare() call across 3 reads, got ${callsAfter - callsBefore}` + ); + }); + + it("returns fresh data after a write invalidates the cache", () => { + getModelsDevPricing(); // warm the cache + saveModelsDevPricing({ + anthropic: { "claude-x": { input: 1, output: 2 } }, + }); + const pricing = getModelsDevPricing(); + assert.ok(pricing.anthropic, "cache should reflect the write, not a stale snapshot"); + assert.equal(pricing.anthropic["claude-x"].input, 1); + }); +}); diff --git a/tests/unit/resolve-model-alias-index-8697.test.ts b/tests/unit/resolve-model-alias-index-8697.test.ts new file mode 100644 index 0000000000..f521014b89 --- /dev/null +++ b/tests/unit/resolve-model-alias-index-8697.test.ts @@ -0,0 +1,51 @@ +import assert from "node:assert/strict"; +import { describe, it } from "node:test"; +import { resolveModelAlias } from "../../src/shared/constants/modelSpecs.ts"; + +describe("resolveModelAlias lookup index (#8697-adjacent)", () => { + it("still resolves a known exact alias", () => { + // Real MODEL_SPECS alias, case-sensitive exact match. + assert.equal(resolveModelAlias("openai/gpt-5.6"), "gpt-5.6"); + }); + + it("does not match a case-varied alias (case-sensitive semantics preserved)", () => { + // resolveModelAlias uses Array.includes(), never .toLowerCase() — a case-varied + // input must NOT resolve, unlike the case-insensitive getCanonicalModelSpecId(). + assert.equal(resolveModelAlias("OpenAI/GPT-5.6"), "OpenAI/GPT-5.6"); + }); + + it("returns the input unchanged for an unknown alias", () => { + assert.equal( + resolveModelAlias("definitely-not-a-real-alias-xyz"), + "definitely-not-a-real-alias-xyz" + ); + }); + + it("does not rescan MODEL_SPECS per call (regression guard for O(n) scans)", () => { + // Warm up outside the measured window. + resolveModelAlias("openai/gpt-5.6"); + + const originalEntries = Object.entries; + let calls = 0; + Object.entries = function patchedEntries(...args: Parameters) { + calls++; + return originalEntries.apply(this, args as never); + } as typeof Object.entries; + + try { + for (let i = 0; i < 500; i++) { + resolveModelAlias("openai/gpt-5.6"); + } + } finally { + Object.entries = originalEntries; + } + + // Pre-fix: every call re-ran Object.entries(MODEL_SPECS). Indexed: the lazy + // index is built once and reused, so no further Object.entries calls happen. + assert.equal( + calls, + 0, + `expected 0 Object.entries() calls across 500 repeated lookups, got ${calls}` + ); + }); +}); diff --git a/tests/unit/reverse-models-dev-providers-8697.test.ts b/tests/unit/reverse-models-dev-providers-8697.test.ts new file mode 100644 index 0000000000..1e3c762e1d --- /dev/null +++ b/tests/unit/reverse-models-dev-providers-8697.test.ts @@ -0,0 +1,47 @@ +import assert from "node:assert/strict"; +import { describe, it } from "node:test"; +import { getResolvedModelCapabilities } from "../../src/lib/modelCapabilities.ts"; +import { MODELS_DEV_PROVIDER_MAP } from "../../src/lib/modelsDevSync/transform.ts"; + +describe("reverseModelsDevProviders memoization (#8697-adjacent)", () => { + it("stays correct across repeated calls for the same provider", () => { + // codex/claude only list their alias (cx/cc) in MODELS_DEV_PROVIDER_MAP — exercises + // the reverse-lookup fallback this function builds (#8429). + const first = getResolvedModelCapabilities({ provider: "codex", model: "gpt-5.6" }); + const second = getResolvedModelCapabilities({ provider: "codex", model: "gpt-5.6" }); + assert.deepEqual(first, second, "memoized reverse-provider lookup must not change results"); + }); + + it("does not rescan MODELS_DEV_PROVIDER_MAP per call (regression guard for O(n) scans)", () => { + // Warm up outside the measured window. + getResolvedModelCapabilities({ provider: "codex", model: "gpt-5.6" }); + + // getResolvedModelCapabilities' wider call chain legitimately calls Object.entries() + // on unrelated objects (e.g. once per call, elsewhere in the chain) — count only calls + // targeting MODELS_DEV_PROVIDER_MAP specifically, the object reverseModelsDevProviders() + // scans, to isolate this fix's contribution precisely. + const originalEntries = Object.entries; + let mapScans = 0; + Object.entries = function patchedEntries(...args: Parameters) { + if (args[0] === MODELS_DEV_PROVIDER_MAP) mapScans++; + return originalEntries.apply(this, args as never); + } as typeof Object.entries; + + try { + for (let i = 0; i < 300; i++) { + getResolvedModelCapabilities({ provider: "codex", model: "gpt-5.6" }); + } + } finally { + Object.entries = originalEntries; + } + + // Pre-fix: reverseModelsDevProviders() rescanned Object.entries(MODELS_DEV_PROVIDER_MAP) + // on every call → mapScans would be ~300. Memoized by provider key: 0 scans once the + // "codex" entry is cached (the warm-up call above already populated it). + assert.equal( + mapScans, + 0, + `expected 0 Object.entries(MODELS_DEV_PROVIDER_MAP) scans across 300 repeated calls, got ${mapScans}` + ); + }); +}); diff --git a/tests/unit/synced-capability-warmup-8697.test.ts b/tests/unit/synced-capability-warmup-8697.test.ts new file mode 100644 index 0000000000..90045d8cdd --- /dev/null +++ b/tests/unit/synced-capability-warmup-8697.test.ts @@ -0,0 +1,32 @@ +import assert from "node:assert/strict"; +import { describe, it, mock } from "node:test"; +import { getDbInstance } from "../../src/lib/db/core.ts"; +import { getSyncedCapability } from "../../src/lib/modelsDevSync.ts"; + +describe("getSyncedCapability warm-up (#8697-adjacent)", () => { + it("does not run a DB round-trip per distinct model lookup (regression guard for the missing bulk warm-up)", () => { + const db = getDbInstance(); + const prepareSpy = mock.method(db, "prepare"); + const callsBefore = prepareSpy.mock.calls.length; + + // A catalog rebuild calls getSyncedCapability() once per distinct model — this + // used to run one SQLite SELECT per call on a cold cache (no warm-up caller sits + // in the /v1/models build path). Self-warmed, only the one-time bulk load (plus + // its CREATE TABLE IF NOT EXISTS guard) should touch the DB, regardless of how + // many distinct models are looked up afterward. + const N = 200; + for (let i = 0; i < N; i++) { + getSyncedCapability("openai", `synthetic-model-${i}`); + } + + const callsAfter = prepareSpy.mock.calls.length; + prepareSpy.mock.restore(); + + assert.ok( + callsAfter - callsBefore <= 2, + `expected at most 2 db.prepare() calls (bulk load + table guard) across ${N} distinct ` + + `model lookups, got ${callsAfter - callsBefore} — getSyncedCapability() may have regressed ` + + `to a per-model SQLite round-trip` + ); + }); +});