fix(sse): exempt tiny-budget reasoning probes and trace persisted-cooldown skips (#12659)

changelog.d/fixes/12659-combo-skip-reasons-tiny-budget-probe.md

@@ -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)

open-sse/services/combo/comboAttemptLoop.ts

@@ -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;

open-sse/services/combo/decisionTrace.ts

@@ -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 }

open-sse/services/combo/executeTargetGates.ts

@@ -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 };

open-sse/services/combo/validateQuality.ts

@@ -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`,

open-sse/utils/error.ts

@@ -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;
 }
 

tests/unit/combo-quality-tiny-budget-probe.test.ts

@@ -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})`);
+});

tests/unit/combo/combo-skipped-targets-summary.test.ts

@@ -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");
+});