From 0ff164701dde2b4e86016e754ede8edacf86eeae Mon Sep 17 00:00:00 2001 From: brick30llc-ctrl Date: Tue, 1 Sep 2026 09:02:05 -0500 Subject: [PATCH] fix(sse): trust finish_reason over reasoning-ratio heuristic in response quality validation (#12262) * fix(sse): trust finish_reason:length/max_tokens over the reasoning-ratio heuristic in response quality validation A truncated response with empty content and reasoning_content present was only rejected by validateResponseQuality() when reasoning consumed >=90% of completion_tokens. A response truncated at a lower ratio (e.g. 63%) passed through as "valid" even though the caller received no usable content and finish_reason was explicitly "length" (or the alternate "max_tokens" naming some providers use) -- an unambiguous truncation signal the validator wasn't reading. Reproduced live against nvidia/nemotron-3-super-120b-a12b: content:null, finish_reason:length, reasoning_tokens 645/1024 (63%). Trust finish_reason directly when it's reported, falling back to the existing token-ratio heuristic only when it isn't. Does not affect the deliberate-tiny-probe case (e.g. max_tokens:1 connectivity pings) -- those never produce reasoning_content, so the branch this change is in doesn't run for them. * docs(changelog): add fragment for #12262 --------- Co-authored-by: brick30llc-ctrl --- ...2262-reasoning-truncation-quality-guard.md | 1 + open-sse/services/combo/validateQuality.ts | 18 +++++ .../combo-quality-validator-reasoning.test.ts | 65 +++++++++++++++++++ 3 files changed, 84 insertions(+) create mode 100644 changelog.d/fixes/12262-reasoning-truncation-quality-guard.md diff --git a/changelog.d/fixes/12262-reasoning-truncation-quality-guard.md b/changelog.d/fixes/12262-reasoning-truncation-quality-guard.md new file mode 100644 index 0000000000..8b3d5bfc43 --- /dev/null +++ b/changelog.d/fixes/12262-reasoning-truncation-quality-guard.md @@ -0,0 +1 @@ +- **fix(sse):** trust `finish_reason: "length"`/`"max_tokens"` over the reasoning-consumed-token ratio in response quality validation, so a reasoning model truncated below the old 90% threshold correctly fails and retries instead of returning empty content as a silent "success" ([#12262](https://github.com/diegosouzapw/OmniRoute/pull/12262)) diff --git a/open-sse/services/combo/validateQuality.ts b/open-sse/services/combo/validateQuality.ts index 76ef5546ab..be70038994 100644 --- a/open-sse/services/combo/validateQuality.ts +++ b/open-sse/services/combo/validateQuality.ts @@ -745,6 +745,24 @@ export async function validateResponseQuality( // tokens or falls back to a non-reasoning model. const contentIsEmpty = content === null || content === undefined || content === ""; if (contentIsEmpty && hasReasoningContent && !hasToolCalls) { + // The 90%-of-completion-tokens ratio below is a proxy for "the request was + // truncated mid-reasoning" for providers that don't report finish_reason + // reliably. When finish_reason IS reported as "length" (or the Anthropic-shape + // "max_tokens"), that's a direct, unambiguous signal of truncation — trust it + // over the ratio instead of requiring reasoning to also clear 90%. A response + // truncated at, say, 60% reasoning still has zero usable content for the + // caller. This does not affect the deliberate-tiny-probe case (e.g. + // `max_tokens: 1` connectivity pings, see errorClassifier.ts's + // LEGIT_EMPTY_OPENAI_FINISH): those produce no reasoning_content at all, so + // hasReasoningContent is already false and this branch never runs for them. + const finishReason = + typeof firstChoice.finish_reason === "string" ? firstChoice.finish_reason : ""; + if (finishReason === "length" || finishReason === "max_tokens") { + return { + valid: false, + reason: `reasoning truncated at token limit (finish_reason: ${finishReason}) — no content output`, + }; + } const usage = json?.usage as Record | undefined; if (usage) { const completionTokens = Number(usage.completion_tokens) || 0; diff --git a/tests/unit/combo-quality-validator-reasoning.test.ts b/tests/unit/combo-quality-validator-reasoning.test.ts index 0650acd1d8..64609d3ebf 100644 --- a/tests/unit/combo-quality-validator-reasoning.test.ts +++ b/tests/unit/combo-quality-validator-reasoning.test.ts @@ -199,6 +199,71 @@ test("#3587 reasoning via completion_tokens_details.reasoning_tokens → invalid assert.match(out.reason ?? "", /reasoning consumed/i); }); +test("client-audit-2026-09-01: finish_reason:length + reasoning <90% of tokens → invalid (direct truncation signal beats the ratio heuristic)", async () => { + // Reproduces a live CT124 response: nvidia/nemotron truncated by max_tokens + // with content:null, finish_reason:"length", and reasoning at only 63% of + // completion_tokens (645/1024) — below the old 90% threshold, so it used to + // pass the validator as "valid" even though the caller got nothing usable. + const res = makeResponse({ + choices: [ + { + message: { + content: null, + reasoning: "Step-by-step analysis that never reached a final answer...", + }, + finish_reason: "length", + }, + ], + usage: { + completion_tokens: 1024, + completion_tokens_details: { reasoning_tokens: 645 }, + }, + }); + const out = await validateResponseQuality(res, false, silentLog); + assert.equal(out.valid, false, "should be invalid: finish_reason:length with empty content"); + assert.match(out.reason ?? "", /truncated at token limit/i); +}); + +test("client-audit-2026-09-01: finish_reason:max_tokens (Anthropic-shape naming) + empty content → invalid", async () => { + const res = makeResponse({ + choices: [ + { + message: { content: null, reasoning_content: "Partial reasoning trace" }, + finish_reason: "max_tokens", + }, + ], + usage: { completion_tokens: 512, reasoning_tokens: 100 }, + }); + const out = await validateResponseQuality(res, false, silentLog); + assert.equal(out.valid, false, "should be invalid: max_tokens finish_reason with empty content"); +}); + +test("client-audit-2026-09-01: no finish_reason + reasoning <90% of tokens → still valid (regression guard, #3587 behavior preserved)", async () => { + // Same low ratio as the case above, but no finish_reason reported at all — + // the direct-truncation-signal branch must not fire, only the ratio heuristic. + const res = makeResponse({ + choices: [{ message: { content: null, reasoning_content: "Some reasoning" } }], + usage: { completion_tokens: 1024, reasoning_tokens: 645 }, + }); + const out = await validateResponseQuality(res, false, silentLog); + assert.equal(out.valid, true, "should stay valid: no finish_reason signal, ratio under 90%"); +}); + +test("client-audit-2026-09-01: finish_reason:stop + empty content + reasoning → ratio heuristic still applies unchanged", async () => { + const res = makeResponse({ + choices: [ + { + message: { content: null, reasoning_content: "Deep reasoning" }, + finish_reason: "stop", + }, + ], + usage: { completion_tokens: 4096, reasoning_tokens: 3800 }, + }); + const out = await validateResponseQuality(res, false, silentLog); + assert.equal(out.valid, false, "finish_reason:stop doesn't short-circuit — ratio (>90%) still applies"); + assert.match(out.reason ?? "", /reasoning consumed/i); +}); + test("#3587 edge: completion_tokens=0 → safe (no division by zero)", async () => { const res = makeResponse({ choices: [