Skip to content

Commit 7d6d3a6

Browse files
committed
fix(lab): keep the two exact-own override maps exact
Review feedback on #2077: routing every override map through `modelRecordValue` was too broad, and for two of them it inverted the very contract the PR is enforcing. `modelPreferHostedTools` and `modelOpenRouterRouting` are deliberately exact at runtime. The adapter reads the first through `hasOwnProperty` (src/adapters/openai-responses.ts:1001) and `resolveOpenRouterRouting` reads the second through `Object.hasOwn` (src/providers/openrouter-routing.ts:89); the type documents the first as "Exact-model hosted tools" (src/types.ts:1584). Family-resolving them would make the report say a `gpt-oss` entry applies to `gpt-oss:120b` when the adapter never applies it -- the same divergence this PR removes, pointed the other way. A bare index is not the answer for those two either: it walks the prototype chain, which is the defect `modelValue` was changed to fix. `modelOpenRouterRouting` was still a bare read at behavior.ts:87 and therefore still carried that bug. Neither existing primitive fits, so this adds a third: `exactOwnValue`. Split is now nine family-aware maps through `modelValue`, two exact-own maps through `exactOwnValue`. Five tests. The two "report agrees" cases are red if either map is sent back through `modelRecordValue`; the ground-truth and control cases pass either way by design. Ground truth for the routing half is executable -- `resolveOpenRouterRouting` is exported, and it returns the entry for the exact key and undefined for the tagged sibling. Also corrects the docblock's control flow, which the review flagged: the throw is caught at src/routing/compatibility/subject.ts:125, which returns no route. `resolvePassiveRouteSubjectId`'s catch is a second backstop and does not see it.
1 parent 0ce4935 commit 7d6d3a6

2 files changed

Lines changed: 89 additions & 6 deletions

File tree

‎src/routing/compatibility/behavior.ts‎

Lines changed: 29 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ function includesModel(list: string[] | undefined, modelId: string): boolean {
5353
}
5454

5555
/**
56-
* Per-model override lookup for the report rows.
56+
* Per-model override lookup for the nine family-aware report rows.
5757
*
5858
* Delegates to modelRecordValue so the report reads these maps the way the
5959
* runtime does -- own properties only, then the pre-colon family, then a
@@ -62,14 +62,37 @@ function includesModel(list: string[] | undefined, modelId: string): boolean {
6262
* differently-cased key, and walked the prototype chain, so a routed model id
6363
* of `constructor`/`toString` yielded an Object.prototype function. That last
6464
* one made buildBehaviorFingerprintV1 throw ("unsupported value type
65-
* function"), which resolvePassiveRouteSubjectId swallows -- the subject is
66-
* silently dropped, and the linker contract says implementations do not throw.
67-
* openai-responses.ts guards modelPreferHostedTools for the same reason.
65+
* function"); the caller catches it (`src/routing/compatibility/subject.ts:125`)
66+
* and returns no route, so the subject is silently dropped -- and the linker
67+
* contract says implementations do not throw.
68+
*
69+
* Not every override map belongs here. `modelPreferHostedTools` and
70+
* `modelOpenRouterRouting` are exact-own at runtime and go through
71+
* `exactOwnValue` below; widening those to the family would be this same bug
72+
* with the sign flipped.
6873
*/
6974
function modelValue<T>(map: Record<string, T> | undefined, modelId: string): T | undefined {
7075
return modelRecordValue(map, modelId);
7176
}
7277

78+
/**
79+
* Exact, own-property lookup for the two maps the runtime resolves that way.
80+
*
81+
* `modelPreferHostedTools` and `modelOpenRouterRouting` are deliberately exact: the
82+
* adapter reads the first through `hasOwnProperty`
83+
* (`src/adapters/openai-responses.ts:1001`) and the second through `Object.hasOwn`
84+
* (`src/providers/openrouter-routing.ts:89`), and the type documents the first as
85+
* "Exact-model hosted tools" (`src/types.ts:1584`). Sending them through
86+
* `modelRecordValue` would make the report say a `gpt-oss` entry applies to
87+
* `gpt-oss:120b` when the adapter will never apply it -- the same divergence this
88+
* file exists to remove, pointed the other way. A bare index is not the answer
89+
* either: it walks the prototype chain, which is the bug `modelValue` just fixed.
90+
* Neither existing primitive is right for these two, so this is the third one.
91+
*/
92+
function exactOwnValue<T>(map: Record<string, T> | undefined, modelId: string): T | undefined {
93+
return map !== undefined && Object.hasOwn(map, modelId) ? map[modelId] : undefined;
94+
}
95+
7396
const CREDENTIAL_HEADER = /(authorization|api[-_]?key|token|secret|credential|cookie)/i;
7497

7598
function nonCredentialHeaderDigest(
@@ -84,7 +107,7 @@ function nonCredentialHeaderDigest(
84107
}
85108

86109
function effectiveOpenRouterRouting(effective: OcxProviderConfig, modelId: string) {
87-
return effective.modelOpenRouterRouting?.[modelId] ?? effective.openRouterRouting;
110+
return exactOwnValue(effective.modelOpenRouterRouting, modelId) ?? effective.openRouterRouting;
88111
}
89112

90113
/**
@@ -199,7 +222,7 @@ export function resolveProductionBehaviorValues(
199222
effective.parallelToolCalls ?? (upstreamProtocol === "openai-chat"),
200223
),
201224
"tools.hostedPreference": behaviorRow("provider_config", {
202-
tools: modelValue(effective.modelPreferHostedTools, modelId) ?? [],
225+
tools: exactOwnValue(effective.modelPreferHostedTools, modelId) ?? [],
203226
}),
204227
"tools.builtinNameEscaping": behaviorRow("provider_config", effective.escapeBuiltinToolNames === true),
205228
"cache.forwarding": behaviorRow("provider_config", effective.promptCacheKey === true),

‎tests/routing-compatibility-model-matching.test.ts‎

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@ import { describe, expect, test } from "bun:test";
88
import { resolveProductionBehaviorValues } from "../src/routing/compatibility/behavior";
99
import { createOpenAIChatAdapter } from "../src/adapters/openai-chat";
1010
import { buildBehaviorFingerprintV1 } from "../src/lab/subject/behavior-fingerprint";
11+
import { resolveOpenRouterRouting } from "../src/providers/openrouter-routing";
12+
import { modelRecordValue } from "../src/reasoning-effort";
1113
import type { OcxConfig, OcxParsedRequest, OcxProviderConfig } from "../src/types";
1214

1315
// ollama-cloud ships `gpt-oss:120b` verbatim (src/providers/registry.ts) and the same
@@ -138,3 +140,61 @@ describe("a prototype-shaped model id resolves to no override", () => {
138140
.toBe(buildBehaviorFingerprintV1(overrideValues("toString")));
139141
});
140142
});
143+
144+
// Not every override map is family-aware, and the two that are not must stay that way.
145+
// The adapter reads `modelPreferHostedTools` through `hasOwnProperty`
146+
// (`src/adapters/openai-responses.ts:1001`) and `resolveOpenRouterRouting` reads
147+
// `modelOpenRouterRouting` through `Object.hasOwn` (`src/providers/openrouter-routing.ts:89`);
148+
// the type calls the first "Exact-model hosted tools" (`src/types.ts:1584`). Sending
149+
// these through modelRecordValue would be the divergence above with the sign flipped:
150+
// the report would claim an override applies that the adapter will never apply.
151+
const EXACT_ONLY: OcxProviderConfig = {
152+
adapter: "openai-responses",
153+
baseUrl: "https://openrouter.ai/api/v1",
154+
apiKey: "sk-test",
155+
authMode: "key",
156+
modelPreferHostedTools: { "gpt-oss": ["image_generation"] },
157+
modelOpenRouterRouting: { "gpt-oss": { order: ["fireworks"] } },
158+
} as unknown as OcxProviderConfig;
159+
160+
const exactConfig = { providers: { "ollama-cloud": EXACT_ONLY } } as unknown as OcxConfig;
161+
162+
const exactValues = (modelId: string) =>
163+
resolveProductionBehaviorValues(exactConfig, "ollama-cloud", modelId, EXACT_ONLY, "salt")!;
164+
165+
describe("exact-own override maps must not spread to the family", () => {
166+
test("the runtime really does not apply the bare-family entry to the :tag model (ground truth)", () => {
167+
// openRouter routing is resolvable directly, so this half is executable rather than cited.
168+
expect(resolveOpenRouterRouting(EXACT_ONLY, "gpt-oss")).toEqual({ order: ["fireworks"] });
169+
expect(resolveOpenRouterRouting(EXACT_ONLY, MODEL)).toBeUndefined();
170+
171+
// And the divergence is real rather than theoretical: the family-aware primitive
172+
// resolves the entry that the adapter's own-property guard does not see.
173+
expect(modelRecordValue(EXACT_ONLY.modelPreferHostedTools, MODEL)).toEqual(["image_generation"]);
174+
expect(Object.hasOwn(EXACT_ONLY.modelPreferHostedTools!, MODEL)).toBe(false);
175+
});
176+
177+
test("report agrees: the :tag model gets no hosted-tool preference", () => {
178+
expect(exactValues(MODEL)["tools.hostedPreference"]!.value).toEqual({ tools: [] });
179+
});
180+
181+
test("report agrees: the :tag model gets no openrouter routing", () => {
182+
expect(exactValues(MODEL)["openrouter.order"]!.value).toEqual([]);
183+
});
184+
185+
test("an exact key still resolves on both maps (control)", () => {
186+
const v = exactValues("gpt-oss");
187+
expect(v["tools.hostedPreference"]!.value).toEqual({ tools: ["image_generation"] });
188+
expect(v["openrouter.order"]!.value).toEqual(["fireworks"]);
189+
});
190+
191+
test("a prototype-shaped id resolves neither map", () => {
192+
// The bare index this replaced walked the prototype chain here too, so these two
193+
// maps had the original bug and must not simply inherit the family-aware fix.
194+
for (const modelId of ["constructor", "toString", "valueOf"]) {
195+
const v = exactValues(modelId);
196+
expect(v["tools.hostedPreference"]!.value).toEqual({ tools: [] });
197+
expect(v["openrouter.order"]!.value).toEqual([]);
198+
}
199+
});
200+
});

0 commit comments

Comments
 (0)