mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-09-19 13:23:50 +03:00
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.
This commit is contained in:
committed by
GitHub
parent
f3ab24b8c7
commit
1b82b2f982
@@ -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)
|
||||
@@ -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;
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -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.
|
||||
*
|
||||
|
||||
136
tests/unit/combo-13870-chars4-overdrops-override-primary.test.ts
Normal file
136
tests/unit/combo-13870-chars4-overdrops-override-primary.test.ts
Normal file
@@ -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");
|
||||
}
|
||||
});
|
||||
Reference in New Issue
Block a user