Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion open-sse/services/autoCombo/routingDecision.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,13 @@ export interface DecisionCandidateInput extends ProviderCandidate {
modelAvailable?: boolean;
/** Capabilities the model supports (tools, vision, ...). Undeclared means not checked. */
capabilities?: readonly string[];
/**
* True when `p95LatencyMs` is the per-model bootstrap guess rather than a measurement.
* The latency budget does not exclude on a guess, so neither may this reason — a decision
* that reports `latency_over_budget` for a candidate selection actually kept would make
* the recorded explanation disagree with what happened.
*/
latencyIsEstimated?: boolean;
}

export interface RoutingDecisionClock {
Expand Down Expand Up @@ -99,7 +106,10 @@ export function hardExclusionReasons(
if (maxCost !== undefined && estimateAutoRequestCostUsd(candidate.costPer1MTokens) > maxCost) {
reasons.push("cost_over_budget");
}
if (exceedsLatencyBudget(candidate.p95LatencyMs, request.budget?.maxLatencyMs)) {
if (
candidate.latencyIsEstimated !== true &&
exceedsLatencyBudget(candidate.p95LatencyMs, request.budget?.maxLatencyMs)
) {
reasons.push("latency_over_budget");
}
return reasons;
Expand Down
31 changes: 24 additions & 7 deletions open-sse/services/combo/autoCandidates.ts
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,15 @@ interface LatencyProfile {
p95LatencyMs: number;
latencyStdDev: number;
errorRate: number;
/**
* True when `p95LatencyMs` came from the per-model bootstrap table rather than from
* measurement — no 24h history above `MIN_HISTORY_SAMPLES` and no usable live metric.
*
* Scoring is happy to rank on a guess; a request's latency budget is not. Refusing a
* candidate on a hardcoded default would reject a model that might well be fast, and the
* traffic that would prove it can only happen if the candidate is allowed through.
*/
latencyIsEstimated: boolean;
/** TTFT / end-to-end / tokens-per-second telemetry, present only with enough history (#6875). */
speedTelemetry: ReturnType<typeof deriveSpeedTelemetry> | undefined;
}
Expand Down Expand Up @@ -236,12 +245,14 @@ function resolveLatencyProfile(
): LatencyProfile {
const historicalTotal = Number(historicalMetric?.totalRequests);
const hasHistory = Number.isFinite(historicalTotal) && historicalTotal >= MIN_HISTORY_SAMPLES;
const p95LatencyMs = resolveP95LatencyMs(
hasHistory,
Number(historicalMetric?.p95LatencyMs),
Number(liveMetric?.avgLatencyMs),
model
);
const historicalP95 = Number(historicalMetric?.p95LatencyMs);
const liveAvg = Number(liveMetric?.avgLatencyMs);
const p95LatencyMs = resolveP95LatencyMs(hasHistory, historicalP95, liveAvg, model);
// Mirrors resolveP95LatencyMs's own fallbacks: anything that does not come from the
// historical p95 or the live average is the bootstrap guess.
const latencyIsEstimated = hasHistory
? !(Number.isFinite(historicalP95) && historicalP95 > 0)
: !(Number.isFinite(liveAvg) && liveAvg > 0);
const historicalStdDev = Number(historicalMetric?.latencyStdDev);
const latencyStdDev =
hasHistory && Number.isFinite(historicalStdDev) && historicalStdDev > 0
Expand All @@ -250,6 +261,7 @@ function resolveLatencyProfile(
return {
p95LatencyMs,
latencyStdDev,
latencyIsEstimated,
errorRate: resolveErrorRate(
hasHistory,
Number(historicalMetric?.successRate),
Expand Down Expand Up @@ -390,7 +402,11 @@ interface CandidateContext {
* (the quota-share and session-availability paths read it), so the builder's own return type says
* so rather than dropping the field or widening the shared contract.
*/
type BuiltAutoCandidate = AutoProviderCandidate & { authType: string | null };
type BuiltAutoCandidate = AutoProviderCandidate & {
authType: string | null;
/** True when `p95LatencyMs` is the bootstrap guess, not a measurement (see LatencyProfile). */
latencyIsEstimated: boolean;
};

async function buildCandidate(
target: ResolvedComboTarget,
Expand Down Expand Up @@ -434,6 +450,7 @@ async function buildCandidate(
costPer1MTokens,
p95LatencyMs: latency.p95LatencyMs,
latencyStdDev: latency.latencyStdDev,
latencyIsEstimated: latency.latencyIsEstimated,
errorRate: latency.errorRate,
...latency.speedTelemetry,
accountTier: "standard" as const,
Expand Down
38 changes: 31 additions & 7 deletions open-sse/services/combo/latencyBudget.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,16 +20,36 @@ import { exceedsLatencyBudget } from "../routing/attemptPolicy.ts";
interface LatencyScoredCandidate {
executionKey: string;
p95LatencyMs: number;
latencyIsEstimated?: boolean;
}

/** Candidates whose estimated latency fits `maxLatencyMs`. Undefined budget keeps every one. */
export function candidatesWithinLatencyBudget<T extends { p95LatencyMs: number }>(
candidates: T[],
maxLatencyMs: number | undefined
): T[] {
/**
* Whether this candidate's latency is a measurement rather than the per-model bootstrap
* guess. `buildAutoCandidates` sets `latencyIsEstimated` when nothing was measured; a
* candidate that predates the flag, or one built by a caller that does not set it, is taken
* at face value so behaviour is unchanged for everything that was already measuring.
*/
function latencyIsMeasured(candidate: { latencyIsEstimated?: boolean }): boolean {
return candidate.latencyIsEstimated !== true;
}

/**
* Candidates whose estimated latency fits `maxLatencyMs`. Undefined budget keeps every one.
*
* A candidate whose latency was never measured is kept regardless of the budget. Every
* candidate carries a number — `resolveP95LatencyMs` falls back to a hardcoded per-model
* default — so without this distinction "unknown" is indistinguishable from "measured", and
* a fresh install would permanently refuse every model under its bootstrap value on no
* evidence at all. Worse, it is self-sealing: the traffic that would replace the guess with
* a measurement can only happen if the candidate is allowed through.
*/
export function candidatesWithinLatencyBudget<
T extends { p95LatencyMs: number; latencyIsEstimated?: boolean },
>(candidates: T[], maxLatencyMs: number | undefined): T[] {
if (maxLatencyMs === undefined) return candidates;
return candidates.filter(
(candidate) => !exceedsLatencyBudget(candidate.p95LatencyMs, maxLatencyMs)
(candidate) =>
!latencyIsMeasured(candidate) || !exceedsLatencyBudget(candidate.p95LatencyMs, maxLatencyMs)
);
}

Expand All @@ -44,7 +64,11 @@ export function dropTargetsOverLatencyBudget<T extends { executionKey: string }>
maxLatencyMs: number | undefined
): T[] {
if (maxLatencyMs === undefined) return targets;
const latencyByKey = new Map(candidates.map((c) => [c.executionKey, c.p95LatencyMs]));
// Only measured latencies are indexed: an unmeasured candidate is treated exactly like a
// target with no candidate row at all — no estimate, so no evidence of a breach.
const latencyByKey = new Map(
candidates.filter(latencyIsMeasured).map((c) => [c.executionKey, c.p95LatencyMs])
);
return targets.filter(
(target) => !exceedsLatencyBudget(latencyByKey.get(target.executionKey), maxLatencyMs)
);
Expand Down
94 changes: 94 additions & 0 deletions tests/unit/latency-budget-unmeasured-latency.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
/**
* The latency budget must not refuse a candidate on a guess.
*
* `latencyBudget.ts` promised that "candidates and targets whose latency is unknown are
* treated as within budget", but on the candidate path that state was unreachable:
* `resolveP95LatencyMs` always returns a number, falling back to `getBootstrapLatencyMs`,
* a hardcoded per-model table (`claude-opus-4.6: 6000`, everything unlisted: 1500). So an
* unmeasured model was indistinguishable from a measured one, and on a fresh install
* `X-OmniRoute-Latency-Budget: 1000` permanently excluded every model — on no evidence.
*
* Worse, it was self-sealing: the traffic that would replace the guess with a real p95 can
* only happen if the candidate is allowed through.
*
* `resolveLatencyProfile` now marks the bootstrap fallback with `latencyIsEstimated`, and
* both the selection filter and the failover filter honour it — as does the recorded
* decision, so an explanation never claims an exclusion that did not happen.
*
* The pre-existing suite could not catch this: it builds candidates with explicit
* `p95LatencyMs`, so it never exercises the bootstrap path at all.
*/
import test from "node:test";
import assert from "node:assert/strict";

const { candidatesWithinLatencyBudget, dropTargetsOverLatencyBudget } =
await import("../../open-sse/services/combo/latencyBudget.ts");

const measured = (executionKey: string, p95LatencyMs: number) => ({
executionKey,
p95LatencyMs,
latencyIsEstimated: false,
});
const guessed = (executionKey: string, p95LatencyMs: number) => ({
executionKey,
p95LatencyMs,
latencyIsEstimated: true,
});

test("a budget refuses a measured candidate but keeps an unmeasured one", () => {
const kept = candidatesWithinLatencyBudget(
[measured("fast", 400), measured("slow", 5000), guessed("unknown", 6000)],
1000
);

assert.deepEqual(
kept.map((c) => c.executionKey),
["fast", "unknown"],
"the 5000 ms measurement is evidence of a breach; the 6000 ms bootstrap default is not"
);
});

test("a candidate that does not declare the flag is still treated as measured", () => {
// Every caller that was already measuring keeps its old behaviour — the flag only ever
// widens what is allowed through, never narrows it.
const kept = candidatesWithinLatencyBudget(
[{ executionKey: "legacy", p95LatencyMs: 5000 }],
1000
);

assert.deepEqual(kept, []);
});

test("no budget keeps every candidate, measured or not", () => {
const candidates = [measured("slow", 9000), guessed("unknown", 9000)];

assert.deepEqual(candidatesWithinLatencyBudget(candidates, undefined), candidates);
});

test("the failover chain drops measured over-budget targets and keeps unmeasured ones", () => {
const targets = [
{ executionKey: "fast" },
{ executionKey: "slow" },
{ executionKey: "unknown" },
{ executionKey: "no-candidate-row" },
];
const candidates = [measured("fast", 400), measured("slow", 5000), guessed("unknown", 6000)];

const chain = dropTargetsOverLatencyBudget(targets, candidates, 1000);

assert.deepEqual(
chain.map((t) => t.executionKey),
["fast", "unknown", "no-candidate-row"],
"an unmeasured candidate must behave exactly like a target with no candidate row"
);
});

test("order is preserved — the filter never reshuffles the chain", () => {
const targets = [{ executionKey: "c" }, { executionKey: "a" }, { executionKey: "b" }];
const candidates = [measured("a", 100), measured("b", 200), measured("c", 300)];

assert.deepEqual(
dropTargetsOverLatencyBudget(targets, candidates, 5000).map((t) => t.executionKey),
["c", "a", "b"]
);
});
Loading