From 5012d8734cf962e65e9462996dfe9d2cd6e95365 Mon Sep 17 00:00:00 2001 From: Nguyen Thanh Dat Date: Wed, 19 Aug 2026 11:24:01 +0700 Subject: [PATCH 1/3] fix(routing): resolve capability evidence the way the resolver resolves it candidateCapabilityEvidence read modelContextWindows, modelInputModalities and modelReasoningEfforts with bare lookups, while every runtime reader of those maps goes through modelRecordValue, which accepts a family entry for a tagged id. With contextWindow 8_000, modelContextWindows {"gpt-oss": 131_072}, modelInputModalities {"gpt-oss": ["text"]} and modelReasoningEfforts {"gpt-oss": ["low","high"]}, for gpt-oss:120b: runtime 131_072, text-only, [low, high] evidence {"contextWindow":8000,"tools":true,...} The window is the worst of the three. It did not degrade to unknown -- it fell through to the provider-wide contextWindow, so routing acted on a definite value belonging to a different model. That is exactly what this module's "unknown is not zero" contract exists to prevent. The other two dimensions simply went missing, which at least reads as unknown. A bare lookup also answered for prototype-shaped ids: a model named "constructor" resolved Object.prototype.constructor as its evidence. modelRecordValue uses hasOwnProperty, so that now resolves nothing. Five tests; three are red without the src change, two are guards against the fix over-reaching and pass either way. 68 tests green across the six routing/capability files. tsc --noEmit clean. --- src/routing/capability.ts | 18 ++-- .../routing-capability-model-matching.test.ts | 94 +++++++++++++++++++ 2 files changed, 106 insertions(+), 6 deletions(-) create mode 100644 tests/routing-capability-model-matching.test.ts diff --git a/src/routing/capability.ts b/src/routing/capability.ts index b49cac897e..023fe6d924 100644 --- a/src/routing/capability.ts +++ b/src/routing/capability.ts @@ -21,6 +21,7 @@ import { nativeReasoningEfforts, } from "../codex/catalog/metadata"; import { readCatalog, readCodexCatalogPath } from "../codex/catalog/parsing"; +import { modelRecordValue } from "../reasoning-effort"; import { statSync } from "node:fs"; import type { RouteCapabilityEvidence } from "./trace"; @@ -159,9 +160,14 @@ export function candidateCapabilityEvidence( const catalogRow = cachedCatalogModels().find(model => model.provider === providerName && model.id === modelId); const isNative = providerName === OPENAI_CODEX_PROVIDER_ID && !modelId.includes("/"); - const rawContextWindow = provider?.modelContextWindows?.[modelId] + // `modelRecordValue`, not a bare lookup: every runtime reader of these three maps + // resolves them that way, so a `gpt-oss` entry covers `gpt-oss:120b`. Reading raw + // made the evidence disagree with the resolver it claims to describe — and for the + // window it did not even degrade to unknown, it fell through to the provider-wide + // value, which is a definite wrong answer rather than an absent one. + const rawContextWindow = modelRecordValue(provider?.modelContextWindows, modelId) ?? provider?.contextWindow - ?? registryEntry?.modelContextWindows?.[modelId] + ?? modelRecordValue(registryEntry?.modelContextWindows, modelId) ?? catalogRow?.contextWindow ?? (isNative ? nativeOpenAiContextWindow(modelId, nativeContextLimits(config)) : undefined); // Native rows go through the accessor (raise-to-ceiling + opt-in). Routed rows keep @@ -170,8 +176,8 @@ export function candidateCapabilityEvidence( ? (nativeOpenAiContextWindow(modelId, nativeContextLimits(config)) ?? rawContextWindow) : rawContextWindow; - const modalities = provider?.modelInputModalities?.[modelId] - ?? registryEntry?.modelInputModalities?.[modelId] + const modalities = modelRecordValue(provider?.modelInputModalities, modelId) + ?? modelRecordValue(registryEntry?.modelInputModalities, modelId) ?? catalogRow?.inputModalities ?? (isNative ? nativeInputModalities(modelId) : undefined); const image = Array.isArray(modalities) @@ -196,8 +202,8 @@ export function candidateCapabilityEvidence( || provider?.parallelToolCalls === true || undefined; - const reasoningEfforts = provider?.modelReasoningEfforts?.[modelId] - ?? registryEntry?.modelReasoningEfforts?.[modelId] + const reasoningEfforts = modelRecordValue(provider?.modelReasoningEfforts, modelId) + ?? modelRecordValue(registryEntry?.modelReasoningEfforts, modelId) ?? (isNative ? nativeReasoningEfforts(modelId) : undefined); const tierSupport = provider diff --git a/tests/routing-capability-model-matching.test.ts b/tests/routing-capability-model-matching.test.ts new file mode 100644 index 0000000000..63fb0b3e18 --- /dev/null +++ b/tests/routing-capability-model-matching.test.ts @@ -0,0 +1,94 @@ +import { describe, expect, test } from "bun:test"; +import { candidateCapabilityEvidence } from "../src/routing/capability"; +import { modelRecordValue } from "../src/reasoning-effort"; +import { isModelTextOnly } from "../src/vision"; +import type { OcxConfig, OcxProviderConfig } from "../src/types"; + +/** + * `candidateCapabilityEvidence` describes what the resolver will do with a candidate, + * so it has to match the resolver. Every runtime reader of `modelContextWindows`, + * `modelInputModalities` and `modelReasoningEfforts` goes through `modelRecordValue` + * (`src/reasoning-effort.ts:108`, `src/server/effort-policy.ts:122`, + * `src/vision/index.ts:34`, `src/codex/catalog/provider-fetch.ts:612`), which accepts a + * family entry for a tagged id. This file pins the evidence to that same rule. + * + * The window matters most: a bare lookup did not degrade to unknown there, it fell + * through to the provider-wide `contextWindow` — a definite wrong answer, which the + * module's own "unknown is not zero" contract is written to avoid. + */ + +function providerWithFamilyEntries(): OcxProviderConfig { + return { + adapter: "openai-chat", + baseUrl: "https://example.test/v1", + contextWindow: 8_000, + models: ["gpt-oss:120b"], + modelContextWindows: { "gpt-oss": 131_072 }, + modelInputModalities: { "gpt-oss": ["text"] }, + modelReasoningEfforts: { "gpt-oss": ["low", "high"] }, + } as unknown as OcxProviderConfig; +} + +function configFor(provider: OcxProviderConfig): OcxConfig { + return { providers: { custom: provider } } as unknown as OcxConfig; +} + +describe("candidateCapabilityEvidence model matching", () => { + test("a family entry covers its tagged siblings, as the resolver does", () => { + const provider = providerWithFamilyEntries(); + + // Ground truth first: what the runtime itself resolves off this config. + expect(modelRecordValue(provider.modelContextWindows, "gpt-oss:120b")).toBe(131_072); + expect(isModelTextOnly(provider, "gpt-oss:120b")).toBe(true); + + const evidence = candidateCapabilityEvidence(configFor(provider), "custom", "gpt-oss:120b"); + expect(evidence.contextWindow).toBe(131_072); + expect(evidence.image).toBe(false); + expect(evidence.reasoningEfforts).toEqual(["low", "high"]); + }); + + test("the window does not fall through to the provider-wide value", () => { + // The specific regression: 8_000 here is not "unknown", it is a definite answer + // belonging to a different model, and routing would act on it. + const evidence = candidateCapabilityEvidence( + configFor(providerWithFamilyEntries()), + "custom", + "gpt-oss:120b", + ); + expect(evidence.contextWindow).not.toBe(8_000); + }); + + test("an exact entry still wins over the family entry", () => { + const provider = { + ...providerWithFamilyEntries(), + modelContextWindows: { "gpt-oss": 131_072, "gpt-oss:20b": 32_000 }, + } as unknown as OcxProviderConfig; + expect(candidateCapabilityEvidence(configFor(provider), "custom", "gpt-oss:20b").contextWindow) + .toBe(32_000); + }); + + test("an unrelated model still falls back to the provider-wide window", () => { + const evidence = candidateCapabilityEvidence( + configFor(providerWithFamilyEntries()), + "custom", + "some-other-model", + ); + expect(evidence.contextWindow).toBe(8_000); + expect(evidence.reasoningEfforts).toBeUndefined(); + }); + + test("a prototype-shaped model id resolves nothing", () => { + // modelRecordValue uses hasOwnProperty; a bare lookup would return Object.prototype + // members here and hand routing a function as evidence. + for (const modelId of ["constructor", "toString", "valueOf", "hasOwnProperty"]) { + const evidence = candidateCapabilityEvidence( + configFor(providerWithFamilyEntries()), + "custom", + modelId, + ); + expect(evidence.contextWindow).toBe(8_000); + expect(evidence.reasoningEfforts).toBeUndefined(); + expect(evidence.image).toBeUndefined(); + } + }); +}); From 9b88f49871db740ad08de3f1545e4b9767e015d0 Mon Sep 17 00:00:00 2001 From: Nguyen Thanh Dat Date: Wed, 19 Aug 2026 16:15:44 +0700 Subject: [PATCH 2/3] test(routing): cover the registry branch of the capability lookups MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The three registry lookups this PR changed are only reached when the provider is absent from the config, and every case in the file supplied one — so `registryEntry?.model*` went untested. Add a case that resolves `grok-4.6:latest` off the `xai` registry entry with no provider configured, asserting the window, the image modality and the reasoning efforts. It asserts the fixture's shape rather than its values, so registry churn does not turn into a false failure while real drift still does. Against origin/dev the new case fails with `Expected: 500000, Received: undefined`, which is the branch it is meant to hold. Thanks @coderabbitai for the catch. --- .../routing-capability-model-matching.test.ts | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/tests/routing-capability-model-matching.test.ts b/tests/routing-capability-model-matching.test.ts index 63fb0b3e18..382de8345b 100644 --- a/tests/routing-capability-model-matching.test.ts +++ b/tests/routing-capability-model-matching.test.ts @@ -1,5 +1,6 @@ import { describe, expect, test } from "bun:test"; import { candidateCapabilityEvidence } from "../src/routing/capability"; +import { PROVIDER_REGISTRY } from "../src/providers/registry"; import { modelRecordValue } from "../src/reasoning-effort"; import { isModelTextOnly } from "../src/vision"; import type { OcxConfig, OcxProviderConfig } from "../src/types"; @@ -77,6 +78,28 @@ describe("candidateCapabilityEvidence model matching", () => { expect(evidence.reasoningEfforts).toBeUndefined(); }); + test("a registry entry covers its tagged siblings with no provider configured", () => { + // The three registry lookups (capability.ts lines 170/180/206) are a separate branch + // from the configured-provider ones above: they are only reached when the provider is + // absent from the config, which every other case here supplies. + const registryEntry = PROVIDER_REGISTRY.find(entry => entry.id === "xai"); + if (!registryEntry) throw new Error("fixture drift: no `xai` entry in PROVIDER_REGISTRY"); + + // Pin the fixture's shape rather than its values, so registry churn does not turn + // into a false failure here while real drift still does. + const family = "grok-4.6"; + expect(registryEntry.modelContextWindows?.[family]).toBeNumber(); + expect(registryEntry.modelInputModalities?.[family]).toBeArray(); + expect(registryEntry.modelReasoningEfforts?.[family]).toBeArray(); + + const emptyConfig = { providers: {} } as unknown as OcxConfig; + const evidence = candidateCapabilityEvidence(emptyConfig, "xai", `${family}:latest`); + + expect(evidence.contextWindow).toBe(registryEntry.modelContextWindows![family]); + expect(evidence.image).toBe(registryEntry.modelInputModalities![family].includes("image")); + expect(evidence.reasoningEfforts).toEqual(registryEntry.modelReasoningEfforts![family]); + }); + test("a prototype-shaped model id resolves nothing", () => { // modelRecordValue uses hasOwnProperty; a bare lookup would return Object.prototype // members here and hand routing a function as evidence. From c90692c859aa72cd13bfc199bb1ab64cd96c00b7 Mon Sep 17 00:00:00 2001 From: Nguyen Thanh Dat Date: Wed, 19 Aug 2026 19:25:53 +0700 Subject: [PATCH 3/3] fix(routing): check noVisionModels before deriving the image modality Review feedback on #2100: the migration resolved `modelInputModalities` without first consulting `noVisionModels`, so the evidence still disagreed with the resolver it claims to describe. Given noVisionModels: ["gpt-oss"] modelInputModalities: { "gpt-oss:120b": ["text", "image"] } `isModelTextOnly` matches the no-vision list and returns true before it ever reads the modality map (src/vision/index.ts:32), so the runtime says text-only while `candidateCapabilityEvidence` reported `image: true`. This is worse here than on the CLI surface #2086 fixed: routing acts on this evidence, so it can select a candidate for image work that execution then refuses. Same ordering, same primitives as the merged #2086. Also strengthens the window oracle the review flagged: `not.toBe(8_000)` also passed for `undefined` and for any other wrong value, so it is now the exact expected number. Two new tests. The positive one is red without this change; the negative one (a model outside noVisionModels keeps its declared image modality) passes either way on purpose -- it guards the fix from over-reaching rather than demonstrating the defect. Against origin/dev the file is 5 fail / 3 pass; against this branch's previous commit, 1 fail / 7 pass. --- src/routing/capability.ts | 21 +++++++--- .../routing-capability-model-matching.test.ts | 41 +++++++++++++++++-- 2 files changed, 54 insertions(+), 8 deletions(-) diff --git a/src/routing/capability.ts b/src/routing/capability.ts index 023fe6d924..0681f64860 100644 --- a/src/routing/capability.ts +++ b/src/routing/capability.ts @@ -10,7 +10,7 @@ * how that affects eligibility. */ -import type { OcxConfig } from "../types"; +import { modelInList, type OcxConfig } from "../types"; import { isCanonicalOpenAiForwardProvider, OPENAI_CODEX_PROVIDER_ID } from "../providers/openai-tiers"; import { serviceTierSupportForModel } from "../providers/service-tier"; import { PROVIDER_REGISTRY } from "../providers/registry"; @@ -176,10 +176,21 @@ export function candidateCapabilityEvidence( ? (nativeOpenAiContextWindow(modelId, nativeContextLimits(config)) ?? rawContextWindow) : rawContextWindow; - const modalities = modelRecordValue(provider?.modelInputModalities, modelId) - ?? modelRecordValue(registryEntry?.modelInputModalities, modelId) - ?? catalogRow?.inputModalities - ?? (isNative ? nativeInputModalities(modelId) : undefined); + // `noVisionModels` is checked before the modality chain because that is the order + // `isModelTextOnly` uses: it matches the no-vision list and returns true before it + // ever reads `modelInputModalities` (`src/vision/index.ts:32`). So a `gpt-oss` + // no-vision entry beats an exact `gpt-oss:120b` entry that lists "image", and + // deriving `image` from the modality chain alone reported vision on a model the + // runtime refuses it for. That matters more here than on the CLI surface fixed in + // #2086: routing *acts* on this evidence, so it would select the candidate for image + // work that execution then rejects. + const noVision = modelInList(provider?.noVisionModels, modelId); + const modalities = noVision + ? ["text"] + : (modelRecordValue(provider?.modelInputModalities, modelId) + ?? modelRecordValue(registryEntry?.modelInputModalities, modelId) + ?? catalogRow?.inputModalities + ?? (isNative ? nativeInputModalities(modelId) : undefined)); const image = Array.isArray(modalities) ? modalities.includes("image") : undefined; diff --git a/tests/routing-capability-model-matching.test.ts b/tests/routing-capability-model-matching.test.ts index 382de8345b..d8839b1f98 100644 --- a/tests/routing-capability-model-matching.test.ts +++ b/tests/routing-capability-model-matching.test.ts @@ -49,14 +49,16 @@ describe("candidateCapabilityEvidence model matching", () => { }); test("the window does not fall through to the provider-wide value", () => { - // The specific regression: 8_000 here is not "unknown", it is a definite answer - // belonging to a different model, and routing would act on it. + // The specific regression: the provider-wide 8_000 is not "unknown", it is a + // definite answer belonging to a different model, and routing would act on it. + // Asserted as the exact expected number rather than `not.toBe(8_000)`, which + // would also pass for `undefined` or any other wrong value. const evidence = candidateCapabilityEvidence( configFor(providerWithFamilyEntries()), "custom", "gpt-oss:120b", ); - expect(evidence.contextWindow).not.toBe(8_000); + expect(evidence.contextWindow).toBe(131_072); }); test("an exact entry still wins over the family entry", () => { @@ -100,6 +102,39 @@ describe("candidateCapabilityEvidence model matching", () => { expect(evidence.reasoningEfforts).toEqual(registryEntry.modelReasoningEfforts![family]); }); + test("noVisionModels beats an exact modality entry, as isModelTextOnly does", () => { + // `isModelTextOnly` matches the no-vision list and returns true before it ever + // reads `modelInputModalities`, so the `gpt-oss` no-vision entry wins over an + // exact `gpt-oss:120b` entry listing "image". Evidence that disagrees here is + // worse than a wrong window: routing selects the candidate for image work and + // execution then refuses it. + const provider = { + ...providerWithFamilyEntries(), + noVisionModels: ["gpt-oss"], + modelInputModalities: { "gpt-oss:120b": ["text", "image"] }, + } as unknown as OcxProviderConfig; + + // Ground truth first: the resolver this evidence claims to describe says text-only. + expect(isModelTextOnly(provider, "gpt-oss:120b")).toBe(true); + + const evidence = candidateCapabilityEvidence(configFor(provider), "custom", "gpt-oss:120b"); + expect(evidence.image).toBe(false); + }); + + test("a model outside noVisionModels keeps its declared image modality", () => { + // The negative half: the no-vision check must not spread to models the list does + // not cover, or the fix would trade a false positive for a false negative. + const provider = { + ...providerWithFamilyEntries(), + models: ["gpt-oss:120b", "llava:13b"], + noVisionModels: ["gpt-oss"], + modelInputModalities: { "llava:13b": ["text", "image"] }, + } as unknown as OcxProviderConfig; + + expect(isModelTextOnly(provider, "llava:13b")).toBe(false); + expect(candidateCapabilityEvidence(configFor(provider), "custom", "llava:13b").image).toBe(true); + }); + test("a prototype-shaped model id resolves nothing", () => { // modelRecordValue uses hasOwnProperty; a bare lookup would return Object.prototype // members here and hand routing a function as evidence.