diff --git a/src/lib/modelMetadataRegistry.ts b/src/lib/modelMetadataRegistry.ts index 64887ae10c..bc0dc39ee6 100644 --- a/src/lib/modelMetadataRegistry.ts +++ b/src/lib/modelMetadataRegistry.ts @@ -303,6 +303,9 @@ export function getCanonicalModelMetadata(input: { // a rebuild instead of rebuilt per lookup. const lowercaseIndexCache = new WeakMap>(); +/** Colliding keys named in the aggregated collision warning before it truncates. */ +const COLLISION_SAMPLE_SIZE = 5; + /** Test hook (#13601): exercised directly by the collision-naming unit test. */ export function findInsensitive( obj: Record | null | undefined, @@ -313,24 +316,36 @@ export function findInsensitive( let index = lowercaseIndexCache.get(obj); if (!index) { index = new Map(); + const collisions: string[] = []; const firstKeyByLower = 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). - // Names both keys so the operator can tell which entries clash; resolution - // stays deterministic first-seen-wins (#13601). + // Collisions are a real data-quality signal from an upstream sync (e.g. + // models.dev returning both "OpenAI" and "openai" as distinct provider + // keys), so they are surfaced rather than swallowed — first-seen-wins, + // matching the pre-#8697 scan's behavior. Each collision is recorded with + // BOTH spellings (#13601) so the operator can tell which entries clash; + // they are reported together once the index finishes building. const firstKey = firstKeyByLower.get(lowerKey); if (firstKey !== undefined) { - console.warn( - `[modelMetadataRegistry] findInsensitive: case-insensitive key collision on "${lowerKey}" ("${firstKey}" vs "${k}") — keeping first-seen value, later one discarded` - ); + collisions.push(`"${lowerKey}" ("${firstKey}" vs "${k}")`); continue; } firstKeyByLower.set(lowerKey, k); index.set(lowerKey, v); } + // Aggregate into ONE line per index build. Warning per colliding key made + // the signal unreadable and expensive: a real catalog collides on hundreds + // of keys, and a production log carried 27,296 of these lines (40% of the + // file, ~500/sec bursts) driving 52 MB rotations. The count plus a bounded + // sample keeps the diagnostic without the flood. + if (collisions.length > 0) { + const sample = collisions.slice(0, COLLISION_SAMPLE_SIZE).join(", "); + const more = collisions.length > COLLISION_SAMPLE_SIZE ? ", …" : ""; + console.warn( + `[modelMetadataRegistry] findInsensitive: ${collisions.length} case-insensitive key collision(s) — keeping first-seen value, later ones discarded. Keys: ${sample}${more}` + ); + } lowercaseIndexCache.set(obj, index); } return index.get(key.toLowerCase()) as T | undefined; diff --git a/tests/unit/model-metadata-registry-collision-log.test.ts b/tests/unit/model-metadata-registry-collision-log.test.ts new file mode 100644 index 0000000000..58e9f9b9b5 --- /dev/null +++ b/tests/unit/model-metadata-registry-collision-log.test.ts @@ -0,0 +1,113 @@ +/** + * `findInsensitive()` in modelMetadataRegistry builds a lowercase-key index and + * warns when two keys collide case-insensitively (e.g. models.dev returning both + * "OpenAI" and "openai"). The warning is a genuine upstream data-quality signal + * and must be kept. + * + * The problem is volume, not the signal: it logged once PER COLLIDING KEY per + * index build. On a real catalog that is hundreds of lines per rebuild — a + * production log captured 27,296 of these in a single file, 40% of all lines, + * in bursts of ~500/sec, driving 52 MB log rotations. + * + * This test pins the aggregate shape: one warning per index build, carrying the + * collision count, no matter how many keys collide. It fails against the + * per-key implementation (3 warnings for 3 collisions). + */ + +import assert from "node:assert/strict"; +import test from "node:test"; + +import { enrichCatalogModelEntry } from "@/lib/modelMetadataRegistry"; + +type Warn = (...args: unknown[]) => void; + +/** Runs `fn` with console.warn captured; returns the warning lines. */ +function captureWarnings(fn: () => void): string[] { + const lines: string[] = []; + const original: Warn = console.warn; + console.warn = (...args: unknown[]) => { + lines.push(args.map(String).join(" ")); + }; + try { + fn(); + } finally { + console.warn = original; + } + return lines.filter((l) => l.includes("findInsensitive")); +} + +/** + * Fresh object per call: the lowercase index is cached in a WeakMap keyed by + * object identity, so reusing one would skip the rebuild (and the warning). + * + * The provider key is deliberately spelled "OpenAI", not "openai". findInsensitive + * short-circuits on `if (key in obj) return obj[key]` — an exact hit returns before + * the index is ever built, so a fixture containing the literal lookup key produces + * zero warnings and proves nothing. + */ +function pricingWithCollisions(count: number): Record { + const pricing: Record = { + OpenAI: { "gpt-4o": { input: 1, output: 2 } }, + }; + for (let i = 0; i < count; i++) { + // Same key differing only in case -> collides with a previously inserted one. + pricing[`Dup${i}Provider`] = { m: { input: 1, output: 2 } }; + pricing[`dup${i}provider`] = { m: { input: 9, output: 9 } }; + } + return pricing; +} + +test("findInsensitive emits ONE aggregated warning per index build, not one per collision", () => { + const warnings = captureWarnings(() => { + enrichCatalogModelEntry( + { id: "gpt-4o", owned_by: "openai" }, + { provider: "openai", model: "gpt-4o" }, + { modelsDevPricing: pricingWithCollisions(3) } as never + ); + }); + + assert.equal( + warnings.length, + 1, + `expected a single aggregated warning, got ${warnings.length}:\n${warnings.join("\n")}` + ); + assert.match(warnings[0], /3 case-insensitive key collision/); +}); + +test("the aggregated warning still names colliding keys so the signal survives", () => { + const warnings = captureWarnings(() => { + enrichCatalogModelEntry( + { id: "gpt-4o", owned_by: "openai" }, + { provider: "openai", model: "gpt-4o" }, + { modelsDevPricing: pricingWithCollisions(2) } as never + ); + }); + + assert.equal(warnings.length, 1); + assert.match(warnings[0], /dup0provider/); +}); + +test("warning count stays at one as collisions scale", () => { + const warnings = captureWarnings(() => { + enrichCatalogModelEntry( + { id: "gpt-4o", owned_by: "openai" }, + { provider: "openai", model: "gpt-4o" }, + { modelsDevPricing: pricingWithCollisions(50) } as never + ); + }); + + assert.equal(warnings.length, 1, "50 collisions must still produce exactly one line"); + assert.match(warnings[0], /50 case-insensitive key collision/); +}); + +test("no collisions means no warning at all", () => { + const warnings = captureWarnings(() => { + enrichCatalogModelEntry( + { id: "gpt-4o", owned_by: "openai" }, + { provider: "openai", model: "gpt-4o" }, + { modelsDevPricing: { OpenAI: { "gpt-4o": { input: 1, output: 2 } } } } as never + ); + }); + + assert.deepEqual(warnings, []); +});