From 1b82b2f982e4214e80dd9c92848ec7b104c2cfbe Mon Sep 17 00:00:00 2001 From: Diego Rodrigues de Sa e Souza Date: Fri, 18 Sep 2026 11:57:40 -0300 Subject: [PATCH] fix(combo): stop chars/4 overestimate demoting a verified context override (#13870) (#14046) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit filterTargetsByRequestCompatibility ranked combo targets solely on the chars/4 estimateTokens() heuristic. On a repetitive agent-session body the estimate overstates real usage several-fold, so a manually-overridden primary sized correctly for the real request got marked context-incompatible and was reordered behind an unconfirmed catalog "emergency" member with a large but unverified limit_context. Fix: when the reorder branch promotes known-context-compatible targets, split them by whether their pass came from an operator-set model_context_override (trusted) or bare catalog metadata (advisory), and also trust a near-boundary override rejection (required tokens within 5x the override — covering the ~3.7x overestimate the issue measured) over a catalog-only pass. An override target keeps or regains priority over an unconfirmed catalog-only "known compatible" target; two override targets or two catalog-only targets keep resolving purely on their own fit as before. Regression test: tests/unit/combo-13870-chars4-overdrops-override-primary.test.ts (RED before the fix — emergency member promoted to position 0 ahead of the override primary; GREEN after). ⚠️ base-red inherited: #14004 — docs env/docs contract (fixed separately in #14022), chatHelpers file-size drift. Not touched by this branch. --- ...870-combo-context-fit-override-priority.md | 1 + open-sse/services/combo/comboStructure.ts | 78 +++++++++- .../services/combo/contextOverrideGate.ts | 16 +++ ...-chars4-overdrops-override-primary.test.ts | 136 ++++++++++++++++++ 4 files changed, 224 insertions(+), 7 deletions(-) create mode 100644 changelog.d/fixes/13870-combo-context-fit-override-priority.md create mode 100644 tests/unit/combo-13870-chars4-overdrops-override-primary.test.ts diff --git a/changelog.d/fixes/13870-combo-context-fit-override-priority.md b/changelog.d/fixes/13870-combo-context-fit-override-priority.md new file mode 100644 index 0000000000..83fa712c17 --- /dev/null +++ b/changelog.d/fixes/13870-combo-context-fit-override-priority.md @@ -0,0 +1 @@ +- fix(combo): stop the chars/4 context estimate from demoting an operator-verified `model_context_override` behind an unconfirmed catalog "emergency" fallback in combo priority ordering (#13870) diff --git a/open-sse/services/combo/comboStructure.ts b/open-sse/services/combo/comboStructure.ts index a12aeb1528..080608b8e9 100644 --- a/open-sse/services/combo/comboStructure.ts +++ b/open-sse/services/combo/comboStructure.ts @@ -29,7 +29,7 @@ import { dedupeTargetsByExecutionKey, isRecord } from "./comboData.ts"; import { resolveComboTargetModelStr } from "./opencodeTargetAlias.ts"; import { isComboModelVisible } from "./comboVisibility.ts"; import { getTargetProvider, MAX_COMBO_DEPTH } from "./comboPredicates.ts"; -import { evaluateContextLimit } from "./contextOverrideGate.ts"; +import { evaluateContextLimit, getModelContextOverrideValue } from "./contextOverrideGate.ts"; import { normalizeModelEntry, orderTargetsForWeightedFallback, @@ -146,9 +146,7 @@ function normalizeRuntimeStep( // to a subset of the provider's connections. This is the second writer of // `allowedConnectionIds` (tag routing is the first); both feed the existing // credential-selection filter in auth.ts. - ...(allowedConnectionIds && allowedConnectionIds.length > 0 - ? { allowedConnectionIds } - : {}), + ...(allowedConnectionIds && allowedConnectionIds.length > 0 ? { allowedConnectionIds } : {}), weight, label, // `prompt` is a per-step pipeline input and only exists on a model step — @@ -549,6 +547,36 @@ function hasKnownCompatibleContextLimit( return evaluateContextLimit(capabilities, requirements, target.modelStr) === true; } +/** + * #13870: how far the required-token estimate exceeds an operator-set + * `model_context_override` before that override is no longer trusted as + * "probably a chars/4 overestimate, not a real overflow." The issue's own + * reproduction measured chars/4 overstating real tokenizer usage ~3.7x on a + * repetitive agent-session body; this margin covers that plus headroom while + * still bounding how far off an override-rejected target can be trusted. + */ +const OVERRIDE_REJECT_TRUST_MARGIN = 5; + +/** + * A target with an explicit `model_context_override` that fails the context + * check, but only because the required-token estimate is within + * `OVERRIDE_REJECT_TRUST_MARGIN`x of the override. This is the #13870 case: + * the override is operator-verified real capacity, while the chars/4 estimate + * that rejected it is a heuristic known to overstate repetitive content by a + * similar multiple — so a near-boundary override rejection is more likely + * estimate noise than a genuine overflow. + */ +function isNearBoundaryOverrideReject( + target: ResolvedComboTarget, + requirements: RequestCompatibilityRequirements +): boolean { + if (requirements.requiredContextTokens <= 0) return false; + const override = getModelContextOverrideValue(target.modelStr); + if (override == null || override <= 0) return false; + if (override >= requirements.requiredContextTokens) return false; + return requirements.requiredContextTokens <= override * OVERRIDE_REJECT_TRUST_MARGIN; +} + const HARD_COMPAT_REASONS = new Set(["tools", "vision", "structured_output", "output_tokens"]); /** @@ -761,9 +789,45 @@ export function filterTargetsByRequestCompatibility( const knownContextCompatible = compatible.filter((target) => hasKnownCompatibleContextLimit(target, requirements) ); - if (knownContextCompatible.length > 0 && knownContextCompatible.length < compatible.length) { - const knownSet = new Set(knownContextCompatible); - return [...knownContextCompatible, ...compatible.filter((target) => !knownSet.has(target))]; + // #13870: an operator-verified override must not be outranked by a + // catalog-only "known compatible" target (e.g. a large but unconfirmed + // emergency fallback) purely because the chars/4 estimate that rejected + // the override is itself known to overstate repetitive content several + // -fold. Split the known-compatible tier by override vs. catalog-only — + // only the catalog-only subset can be leapfrogged by a near-boundary + // override rejection; a target that is ALREADY known-compatible via its + // own override (evaluateContextLimit resolves overrides first) keeps its + // earned place ahead of every override-rejected target. + const overrideVerifiedCompatible = knownContextCompatible.filter( + (target) => getModelContextOverrideValue(target.modelStr) != null + ); + const catalogOnlyCompatible = knownContextCompatible.filter( + (target) => getModelContextOverrideValue(target.modelStr) == null + ); + const overrideTrustedRejects = compatible.filter( + (target) => + !knownContextCompatible.includes(target) && + (targetReasons.get(target) || []).includes("context_window") && + isNearBoundaryOverrideReject(target, requirements) + ); + const preferredTier = [ + ...overrideVerifiedCompatible, + ...overrideTrustedRejects, + ...catalogOnlyCompatible, + ]; + if (preferredTier.length > 0) { + const preferredSet = new Set(preferredTier); + const reordered = [ + ...preferredTier, + ...compatible.filter((target) => !preferredSet.has(target)), + ]; + // Only return when this tiering actually changes the order — e.g. when + // every compatible target already lands in `preferredTier` in the same + // relative order it started in, `compatible` (built by a single stable + // filter over `targets`) is already correct and re-wrapping it would be + // a no-op. + const changedOrder = reordered.some((target, index) => target !== compatible[index]); + if (changedOrder) return reordered; } } diff --git a/open-sse/services/combo/contextOverrideGate.ts b/open-sse/services/combo/contextOverrideGate.ts index 478fbd99be..cae52d3814 100644 --- a/open-sse/services/combo/contextOverrideGate.ts +++ b/open-sse/services/combo/contextOverrideGate.ts @@ -90,6 +90,22 @@ function resolveContextOverrideVerdict( return override >= requiredContextTokens; } +/** + * Resolve a target's raw persisted `model_context_override` value (effort-suffix + * inheritance included), or `null` when none is set. + * + * #13870: exposed so the combo compat-filter reorder step (comboStructure.ts) + * can tell an operator-verified override apart from a catalog-advisory limit — + * an override is a stronger trust signal than an unconfirmed catalog number, + * so it must not be unconditionally outranked by one when the chars/4 estimate + * that rejected it is itself known to overstate real usage (issue #13870 + * measured a ~3.7x overestimate on a repetitive agent-session body). + */ +export function getModelContextOverrideValue(modelStr: string | undefined): number | null { + if (!modelStr) return null; + return lookupOverrideWithEffortInheritance(modelStr); +} + /** * Decide whether a target's known context limit accommodates the request. * diff --git a/tests/unit/combo-13870-chars4-overdrops-override-primary.test.ts b/tests/unit/combo-13870-chars4-overdrops-override-primary.test.ts new file mode 100644 index 0000000000..b9fb6f23d1 --- /dev/null +++ b/tests/unit/combo-13870-chars4-overdrops-override-primary.test.ts @@ -0,0 +1,136 @@ +import test from "node:test"; +import assert from "node:assert/strict"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; + +// Repro for #13870: the combo context-fit compat filter (filterTargetsByRequestCompatibility) +// uses the chars/4 estimate (estimateTokens) as the sole ground truth for ranking targets. +// On a repetitive agent-session body, chars/4 overstates the real tokenizer count by ~3-4x +// (issue measured 276,792 estimated vs 74,679 real for the same body). A user-defined combo +// whose primary/fallback members carry manual model_context_overrides sized for the REAL +// window (250k) gets those overrides evaluated against the inflated estimate, fail the +// context check, and are demoted behind the emergency (899k catalog) member -- even though +// the real request would have fit the primary with 3x headroom. + +const TEST_DATA_DIR = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-13870-")); +const ORIGINAL_DATA_DIR = process.env.DATA_DIR; +process.env.DATA_DIR = TEST_DATA_DIR; + +const core = await import("../../src/lib/db/core.ts"); +const { saveModelsDevCapabilities, clearModelsDevCapabilities } = + await import("../../src/lib/modelsDevSync.ts"); +const { filterTargetsByRequestCompatibility } = await import("../../open-sse/services/combo.ts"); +const { estimateTokens } = await import("../../open-sse/services/contextManager.ts"); +const { setModelContextOverride, removeModelContextOverride } = + await import("../../src/lib/db/modelContextOverrides.ts"); + +test.after(() => { + core.resetDbInstance(); + if (ORIGINAL_DATA_DIR === undefined) { + delete process.env.DATA_DIR; + } else { + process.env.DATA_DIR = ORIGINAL_DATA_DIR; + } + fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 }); +}); + +test.beforeEach(() => { + clearModelsDevCapabilities(); +}); + +const noopLog = { info() {}, warn() {}, error() {}, debug() {} }; + +function target(modelStr: string) { + return { + kind: "model" as const, + stepId: modelStr, + executionKey: modelStr, + modelStr, + provider: modelStr.split("/")[0], + providerId: null, + connectionId: null, + weight: 1, + label: null, + }; +} + +function capabilityEntry(limitContext: number | null) { + return { + tool_call: true, + reasoning: false, + attachment: false, + structured_output: true, + temperature: true, + modalities_input: JSON.stringify(["text"]), + modalities_output: JSON.stringify(["text"]), + knowledge_cutoff: null, + release_date: null, + last_updated: null, + status: null, + family: null, + open_weights: false, + limit_context: limitContext, + limit_input: limitContext, + limit_output: 65_536, + interleaved_field: null, + }; +} + +function repetitiveAgentBody() { + const toolResultChunk = + '{"tool_call_id":"call_abc123","role":"tool","content":"' + + "line of repeated structured tool output ".repeat(160) + + '"}'; + const messages = []; + for (let i = 0; i < 400; i++) { + messages.push({ role: i % 2 === 0 ? "assistant" : "tool", content: toolResultChunk }); + } + const tools = Array.from({ length: 25 }, (_, i) => ({ + type: "function", + function: { name: `tool_${i}`, description: "d".repeat(200), parameters: { type: "object" } }, + })); + return { messages, tools, max_tokens: 32000 }; +} + +test("#13870: chars/4 overestimate demotes a real-fitting override primary behind the catalog emergency member", () => { + saveModelsDevCapabilities({ + "unit-13870-primary": { model: capabilityEntry(null) }, + "unit-13870-fallback": { model: capabilityEntry(null) }, + "unit-13870-emergency": { model: capabilityEntry(899_153) }, + "unit-13870-small": { model: capabilityEntry(128_450) }, + }); + + setModelContextOverride("unit-13870-primary", "model", 250_000); + setModelContextOverride("unit-13870-fallback", "model", 250_000); + + try { + const body = repetitiveAgentBody(); + + const estimated = estimateTokens({ messages: body.messages, tools: body.tools }); + assert.ok( + estimated > 250_000 - 32_000, + `expected the chars/4 estimate to exceed the override window; got ${estimated}` + ); + + const targets = [ + target("unit-13870-primary/model"), + target("unit-13870-fallback/model"), + target("unit-13870-emergency/model"), + target("unit-13870-small/model"), + ]; + + const out = filterTargetsByRequestCompatibility(targets, body, noopLog); + + assert.equal( + out[0].modelStr, + "unit-13870-primary/model", + `expected the real-fitting override primary to stay first, but the inflated ` + + `chars/4 estimate promoted "${out[0].modelStr}" ahead of it -- this is the ` + + `"collapse to the emergency member" reported in #13870` + ); + } finally { + removeModelContextOverride("unit-13870-primary", "model"); + removeModelContextOverride("unit-13870-fallback", "model"); + } +});