Compare commits

...

1 Commits

Author SHA1 Message Date
diegosouzapw
e29123c359 fix(sse): exempt tiny-budget reasoning probes and trace persisted-cooldown skips (#12659)
- validateResponseQuality (combo quality gate) now exempts a reasoning
  truncation whose completion_tokens is below the #10281 tiny-budget
  threshold (256), passing the original 200 through instead of failing
  quality and poisoning the model lockout for a deliberate capability
  probe.
- executeTargetGates.ts records a persisted_cooldown decision on the
  decision trace (new allowlisted ComboSkipReason); comboAttemptLoop's
  buildComboDiag surfaces it via a new skippedTargets[] field on the
  ALL_TARGETS_SKIPPED diagnostics body, sanitized through
  sanitizeComboDiagnostics like every other diagnostics field.
2026-09-10 15:31:28 -03:00
8 changed files with 249 additions and 3 deletions

View File

@@ -0,0 +1 @@
- fix(sse): exempt tiny-budget reasoning probes from combo quality failure and surface persisted-cooldown skips in ALL_TARGETS_SKIPPED diagnostics (#12659)

View File

@@ -33,7 +33,12 @@ import {
waitForCooldownAwareRetry,
} from "../../../src/sse/services/cooldownAwareRetry.ts";
import { toRetryAfterDisplayValue } from "./validateQuality.ts";
import { finalizeComboTrace, finishComboTrace } from "./decisionTrace.ts";
import {
finalizeComboTrace,
finishComboTrace,
getComboTrace,
summarizeSkippedTargets,
} from "./decisionTrace.ts";
import { isRetryAfterEligibleStatus } from "./unavailableRetryGate.ts";
import { withQuotaExhaustionClassification } from "./quotaExhaustion.ts";
import {
@@ -133,6 +138,16 @@ export async function dispatchWithCooldownRetry(opts: {
attemptOrder: state.comboAttemptOrder,
terminalReason,
recovery: buildRecoveryHint(terminalReason, retryAfterSeconds),
// #12659: surface per-target skip reasons (e.g. persisted_cooldown)
// that `excluded` above never captures — only worth the trace lookup
// on the diagnostic-heavy terminal reason.
skippedTargets:
terminalReason === "all_targets_skipped"
? summarizeSkippedTargets(getComboTrace(deps.traceInvocationId)).map((g) => ({
reason: g.reason,
targets: g.targets,
}))
: undefined,
});
let globalResolve: ((res: Response) => void) | null = null;

View File

@@ -21,6 +21,7 @@ import { randomUUID } from "node:crypto";
export const COMBO_SKIP_REASONS = [
"circuit_open",
"provider_cooldown",
"persisted_cooldown",
"request_exhaustion",
"model_lockout",
"quota_cutoff",
@@ -43,6 +44,12 @@ export interface ComboTraceEntry {
decision: ComboDecision;
reason?: ComboSkipReason;
ts: number;
/**
* Safe, non-secret elaboration on `reason` (e.g. a cooldown reset ISO
* timestamp). SAFETY CONTRACT above still applies: never a credential
* fragment, header, or raw upstream error string.
*/
detail?: string;
}
export interface ComboTrace {
@@ -121,9 +128,43 @@ export function recordComboDecision(
decision: entry.decision,
reason: entry.reason as ComboSkipReason | undefined,
ts: Date.now(),
detail: entry.detail,
});
}
/** One skip reason's targets, for the ALL_TARGETS_SKIPPED diagnostics body. */
export interface SkippedTargetGroup {
reason: ComboSkipReason;
targets: string[];
detail?: string;
}
/**
* #12659: group a trace's skipped-before-dispatch decisions by reason so an
* ALL_TARGETS_SKIPPED 503 body can report WHY every target was skipped
* instead of an opaque `excluded: []`. Pure — takes a trace, returns groups;
* does not read or mutate the in-memory store.
*/
export function summarizeSkippedTargets(trace: ComboTrace | null): SkippedTargetGroup[] {
if (!trace) return [];
const byReason = new Map<ComboSkipReason, SkippedTargetGroup>();
for (const entry of trace.decisions) {
if (entry.decision !== "skipped_before_dispatch" || !entry.reason) continue;
const group = byReason.get(entry.reason);
if (group) {
group.targets.push(entry.target);
if (!group.detail && entry.detail) group.detail = entry.detail;
} else {
byReason.set(entry.reason, {
reason: entry.reason,
targets: [entry.target],
detail: entry.detail,
});
}
}
return Array.from(byReason.values());
}
export function finishComboTrace(
invocationId: string,
terminal: { status: number | null; errorClass?: string | null }

View File

@@ -141,6 +141,15 @@ export async function evaluateExecuteTargetGates(opts: {
if (persistedSkip) {
// Lift-as-is: combo.ts skips without observeFailure / stopProtectedPriorityTarget.
deps.log.info("COMBO", persistedSkip);
// #12659: this branch used to be untraced, so an ALL_TARGETS_SKIPPED
// caused purely by persisted cooldowns surfaced as an opaque
// `attempted=0, excluded=[]` diagnostics body.
recordComboDecision(deps.traceInvocationId, {
step: target.executionKey,
target: modelStr,
decision: "skipped_before_dispatch",
reason: "persisted_cooldown",
});
deps.clearStaleLKGP(deps.combo.name, target.executionKey, deps.combo.id, deps.log, "COMBO");
bumpFallback();
return { kind: "skip", result: null };

View File

@@ -14,8 +14,24 @@ import {
} from "../../utils/streamHelpers.ts";
import { evaluateResponseValidation, type ResponseValidationConfig } from "./responseValidation.ts";
import { getReasoningTokens } from "../../../src/lib/usage/tokenAccounting.ts";
import { REASONING_BUFFER_MIN_TRIGGER } from "../reasoningTokenBuffer.ts";
import type { ComboRetryAfter } from "./types.ts";
/**
* #12659: below this actual `completion_tokens` count, a reasoning-truncated
* response is a deliberate tiny-budget capability probe (#10281, e.g. Claude
* Code's `/model` check sending `max_tokens: 1`) rather than a genuine
* exhaustion of a real reasoning budget -- `completion_tokens` cannot exceed
* the caller's `max_tokens`, so a tiny count here proves a tiny budget was
* requested without needing to thread the request body through the combo
* dispatch call sites. Reuses #10281's own threshold constant instead of
* duplicating the magic number; every existing #3587 exhaustion regression
* case (512/1024/4096 completion_tokens) sits well above it.
*/
function isTinyBudgetTruncation(completionTokens: number): boolean {
return completionTokens > 0 && completionTokens < REASONING_BUFFER_MIN_TRIGGER;
}
/**
* Detects tool_calls entries within one assistant message that repeat the
* exact same function name + arguments verbatim -- always a bug (no
@@ -801,19 +817,26 @@ export async function validateResponseQuality(
// hasReasoningContent is already false and this branch never runs for them.
const finishReason =
typeof firstChoice.finish_reason === "string" ? firstChoice.finish_reason : "";
const usage = json?.usage as Record<string, unknown> | undefined;
const completionTokens = usage ? Number(usage.completion_tokens) || 0 : 0;
if (finishReason === "length" || finishReason === "max_tokens") {
// #12659: a tiny deliberate capability probe (e.g. `max_tokens: 1`
// connectivity/`/model` pings) hits this exact shape on a reasoning
// model -- exempt it into the #10281 truncated-200 treatment (pass the
// original 200 through unmodified) instead of a genuine quality
// failure, so the caller never records a model-lockout for a probe.
if (isTinyBudgetTruncation(completionTokens)) return { valid: true };
return {
valid: false,
reason: `reasoning truncated at token limit (finish_reason: ${finishReason}) — no content output`,
};
}
const usage = json?.usage as Record<string, unknown> | undefined;
if (usage) {
const completionTokens = Number(usage.completion_tokens) || 0;
const reasoningTokens = getReasoningTokens(usage);
// If reasoning consumed 90%+ of completion tokens, the model ran out of
// budget before producing any content output.
if (completionTokens > 0 && reasoningTokens >= completionTokens * 0.9) {
if (isTinyBudgetTruncation(completionTokens)) return { valid: true };
return {
valid: false,
reason: `reasoning consumed ${reasoningTokens}/${completionTokens} tokens — no content output`,

View File

@@ -387,6 +387,11 @@ export interface ComboExclusion {
model?: string;
reason: string;
}
/** #12659: one skip reason's targets, surfaced on an ALL_TARGETS_SKIPPED body. */
export interface ComboSkippedTargetGroup {
reason: string;
targets: string[];
}
export interface ComboDiagnostics {
poolSize: number;
attempted: number;
@@ -395,6 +400,13 @@ export interface ComboDiagnostics {
terminalReason: string;
/** Optional next-step hint — populated when the dispatcher can recommend a recovery action. */
recovery?: ComboRecoveryHint;
/**
* #12659: per-target skip reasons (e.g. `persisted_cooldown`) recorded on the
* decision trace but not captured by `excluded` (which only sources from
* exhaustedProviders/exhaustedConnections). Optional — populated only when
* the caller has a decision trace to summarize.
*/
skippedTargets?: ComboSkippedTargetGroup[];
}
function clampDiagStr(v: unknown, max = 128): string {
@@ -482,6 +494,12 @@ export function sanitizeComboDiagnostics(d: ComboDiagnostics): ComboDiagnostics
terminalReason: clampDiagStr(d?.terminalReason, 200),
};
if (recovery) out.recovery = recovery;
if (Array.isArray(d?.skippedTargets) && d.skippedTargets.length > 0) {
out.skippedTargets = d.skippedTargets.slice(0, 32).map((g) => ({
reason: clampDiagStr(g?.reason, 64),
targets: (g?.targets ?? []).slice(0, 32).map((t) => clampDiagStr(t, 96)),
}));
}
return out;
}

View File

@@ -0,0 +1,19 @@
import test from "node:test";
import assert from "node:assert/strict";
const { validateResponseQuality } = await import("../../open-sse/services/combo.ts");
const silentLog = { warn: () => {} };
function makeTinyProbeResponse(): Response {
// Mirrors the issue's reported shape verbatim: "reasoning consumed 10/10 tokens — no content output"
return new Response(
JSON.stringify({
choices: [{ message: { content: null, reasoning_content: "Ok" }, finish_reason: "length" }],
usage: { completion_tokens: 10, reasoning_tokens: 10 },
}),
{ status: 200, headers: { "content-type": "application/json" } }
);
}
test("#12659 EXPECTED: combo validator exempts a tiny-budget reasoning probe (finish_reason:length, tiny completion_tokens) instead of a genuine quality failure", async () => {
const res = makeTinyProbeResponse();
const out = await validateResponseQuality(res, false, silentLog);
assert.equal(out.valid, true, `reproduces #12659: ... (reason: ${out.reason})`);
});

View File

@@ -0,0 +1,120 @@
/**
* #12659 — ALL_TARGETS_SKIPPED must carry per-target skip reasons.
*
* Before this fix, `executeTargetGates.ts`'s persisted-connection-cooldown
* skip branch never called `recordComboDecision`, `persisted_cooldown` was
* not even an allowlisted `ComboSkipReason`, and the 503 diagnostics body's
* `excluded[]` only ever sourced from exhaustedProviders/exhaustedConnections
* — so a persisted-cooldown-only failure surfaced as an opaque
* `attempted=0, excluded=[]`.
*/
import { test, beforeEach } from "node:test";
import assert from "node:assert/strict";
import fs from "node:fs";
import os from "node:os";
import path from "node:path";
const TEST_DATA_DIR = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-combo-skipped-"));
process.env.DATA_DIR = TEST_DATA_DIR;
process.env.API_KEY_SECRET = process.env.API_KEY_SECRET || "combo-skipped-targets-secret";
const {
COMBO_SKIP_REASONS,
recordComboDecision,
resetComboTraceStore,
startComboTrace,
summarizeSkippedTargets,
getComboTrace,
} = await import("../../../open-sse/services/combo/decisionTrace.ts");
beforeEach(() => resetComboTraceStore());
test("#12659: persisted_cooldown is an allowlisted skip reason", () => {
assert.ok(
(COMBO_SKIP_REASONS as readonly string[]).includes("persisted_cooldown"),
"persisted_cooldown must be recordable — it used to have no allowlist entry at all"
);
});
test("#12659: a persisted-cooldown skip is grouped into skippedTargets[] by reason", () => {
startComboTrace("combo-skip-1", { strategy: "priority", comboName: "my-combo" });
recordComboDecision("combo-skip-1", {
step: "step-1",
target: "zai/glm-5.3",
decision: "skipped_before_dispatch",
reason: "persisted_cooldown",
});
recordComboDecision("combo-skip-1", {
step: "step-2",
target: "openai/gpt-x",
decision: "skipped_before_dispatch",
reason: "persisted_cooldown",
});
recordComboDecision("combo-skip-1", {
step: "step-3",
target: "anthropic/claude-y",
decision: "skipped_before_dispatch",
reason: "circuit_open",
});
const groups = summarizeSkippedTargets(getComboTrace("combo-skip-1"));
const persisted = groups.find((g) => g.reason === "persisted_cooldown");
assert.ok(persisted, "expected a persisted_cooldown group in the summary");
assert.deepEqual(persisted!.targets.sort(), ["openai/gpt-x", "zai/glm-5.3"]);
const circuit = groups.find((g) => g.reason === "circuit_open");
assert.ok(circuit);
assert.deepEqual(circuit!.targets, ["anthropic/claude-y"]);
});
test("#12659: summarizeSkippedTargets ignores dispatched/not_reached decisions", () => {
startComboTrace("combo-skip-2", { strategy: "priority", comboName: "my-combo" });
recordComboDecision("combo-skip-2", {
step: "step-1",
target: "zai/glm-5.3",
decision: "dispatched",
});
recordComboDecision("combo-skip-2", {
step: "step-2",
target: "openai/gpt-x",
decision: "not_reached",
});
const groups = summarizeSkippedTargets(getComboTrace("combo-skip-2"));
assert.deepEqual(groups, []);
});
test("#12659: summarizeSkippedTargets is safe on a null/missing trace", () => {
assert.deepEqual(summarizeSkippedTargets(null), []);
assert.deepEqual(summarizeSkippedTargets(getComboTrace("does-not-exist")), []);
});
test("#12659: diagnostics body groups persisted-cooldown skips WITHOUT leaking a connection id or a stack trace", async () => {
const { errorResponseWithComboDiagnostics } = await import("../../../open-sse/utils/error.ts");
const res = errorResponseWithComboDiagnostics(
503,
"Service temporarily unavailable: all targets were skipped by pre-dispatch filters",
{
poolSize: 2,
attempted: 0,
excluded: [],
attemptOrder: [],
terminalReason: "all_targets_skipped",
skippedTargets: [{ reason: "persisted_cooldown", targets: ["zai/glm-5.3", "openai/gpt-x"] }],
},
{ code: "ALL_TARGETS_SKIPPED", type: "service_unavailable" }
);
const body = (await res.json()) as {
diagnostics: { skippedTargets?: Array<{ reason: string; targets: string[] }> };
};
assert.ok(body.diagnostics.skippedTargets, "diagnostics.skippedTargets must be present");
assert.deepEqual(body.diagnostics.skippedTargets![0], {
reason: "persisted_cooldown",
targets: ["zai/glm-5.3", "openai/gpt-x"],
});
const serialized = JSON.stringify(body);
// Task notes (#12659): a skip reason must never leak an upstream stack
// trace or a credential/account-id fragment — this body was built through
// buildErrorBody()/sanitizeComboDiagnostics(), never raw err.stack.
assert.ok(!/\bat\s+\/[\w./-]+:\d+:\d+/.test(serialized), "no stack-trace frame in the body");
assert.ok(!serialized.includes("0217fa47"), "no connection/account id leaked into the body");
});