mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-09-19 13:23:50 +03:00
fix(sse): aggregate findInsensitive collision warning into one line per build (#12972)
modelMetadataRegistry's findInsensitive() warned once per colliding key while building its lowercase index. On a real catalog that is hundreds of lines per rebuild: a production log carried 27,296 of these in a single file — 40% of all lines, in ~500/sec bursts — driving 52 MB log rotations and ~466 MB of logs on disk. The warning itself is worth keeping: a case-insensitive collision is a genuine upstream data-quality signal (models.dev returning both "OpenAI" and "openai" as distinct provider keys), and first-match-wins silently discards the later value. Only the volume was wrong. Collisions are now collected during the index build and reported as a single line carrying the total count plus the first 5 keys, so the diagnostic survives at 1/N the volume. No behavior change: the index, the first-match-wins resolution, and the WeakMap identity cache are untouched. Validated by TDD (Hard Rule #18): tests/unit/model-metadata-registry-collision-log.test.ts fails on the old implementation (3 collisions -> 3 warnings, 50 -> 50) and passes after (always 1). Also covers the no-collision case emitting nothing, and asserts the aggregated line still names colliding keys. Note for reviewers: the test fixture deliberately spells the provider key "OpenAI" rather than "openai". findInsensitive short-circuits on `if (key in obj) return obj[key]` before the index is ever built, so a fixture containing the literal lookup key produces zero warnings and proves nothing. Gates: eslint clean on both changed files. typecheck:core reports 9 pre-existing errors in open-sse/services/compression/omniglyph* — unrelated to this change (those files are byte-identical to origin/release/v3.8.50) and caused by a local stale node_modules carrying omniglyph 1.3.1 against the required ^1.4.0. Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
This commit is contained in:
@@ -303,6 +303,9 @@ export function getCanonicalModelMetadata(input: {
|
||||
// a rebuild instead of rebuilt per lookup.
|
||||
const lowercaseIndexCache = new WeakMap<object, Map<string, unknown>>();
|
||||
|
||||
/** 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<T>(
|
||||
obj: Record<string, T> | null | undefined,
|
||||
@@ -313,24 +316,36 @@ export function findInsensitive<T>(
|
||||
let index = lowercaseIndexCache.get(obj);
|
||||
if (!index) {
|
||||
index = new Map();
|
||||
const collisions: string[] = [];
|
||||
const firstKeyByLower = new Map<string, string>();
|
||||
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;
|
||||
|
||||
113
tests/unit/model-metadata-registry-collision-log.test.ts
Normal file
113
tests/unit/model-metadata-registry-collision-log.test.ts
Normal file
@@ -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<string, unknown> {
|
||||
const pricing: Record<string, unknown> = {
|
||||
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, []);
|
||||
});
|
||||
Reference in New Issue
Block a user