From fe3ab7a8361a712568e8efeef02ebf5dee630457 Mon Sep 17 00:00:00 2001 From: diegosouzapw Date: Mon, 18 May 2026 09:10:35 -0300 Subject: [PATCH] fix(combo/validator): accept reasoning_content as valid output (#2341) `validateResponseQuality` flagged any response with `content: null` as empty and triggered a false-positive 502 combo fallback, even when the upstream returned the entire answer in `reasoning_content`. This affected every reasoning model that omits `content` by design: moonshotai/Kimi-K2.5-TEE, zai-org/GLM-5-TEE, zai-org/GLM-4.7-TEE, DeepSeek-R1 and Qwen-thinking variants via Chutes.ai, Nvidia, and other OpenAI-compatible gateways. Treat a non-empty `reasoning_content` (or its legacy `reasoning` alias) as valid content. Empty/whitespace-only strings still fall through to the existing empty-content rejection so we don't weaken the guard. Backward compat verified: regular content-only and tool_calls-only responses still validate without change. --- open-sse/services/combo.ts | 10 +- .../combo-quality-validator-reasoning.test.ts | 107 ++++++++++++++++++ 2 files changed, 116 insertions(+), 1 deletion(-) create mode 100644 tests/unit/combo-quality-validator-reasoning.test.ts diff --git a/open-sse/services/combo.ts b/open-sse/services/combo.ts index 9f8a661c72..40fd49c3a4 100644 --- a/open-sse/services/combo.ts +++ b/open-sse/services/combo.ts @@ -211,7 +211,15 @@ export async function validateResponseQuality( const content = message.content; const toolCalls = message.tool_calls; - const hasContent = content !== null && content !== undefined && content !== ""; + // Issue #2341: Reasoning models (Kimi-K2.5-TEE, GLM-5-TEE, etc.) emit their + // output in `reasoning_content` (or `reasoning`) with `content: null`. The + // validator used to flag those as empty and trigger a false-positive 502 + // fallback. Count a non-empty reasoning_content as valid output too. + const reasoningContent = message.reasoning_content ?? message.reasoning; + const hasReasoningContent = + typeof reasoningContent === "string" && reasoningContent.trim().length > 0; + const hasContent = + (content !== null && content !== undefined && content !== "") || hasReasoningContent; const hasToolCalls = Array.isArray(toolCalls) && toolCalls.length > 0; if (!hasContent && !hasToolCalls) { diff --git a/tests/unit/combo-quality-validator-reasoning.test.ts b/tests/unit/combo-quality-validator-reasoning.test.ts new file mode 100644 index 0000000000..f0948cde56 --- /dev/null +++ b/tests/unit/combo-quality-validator-reasoning.test.ts @@ -0,0 +1,107 @@ +/** + * Issue #2341 — `validateResponseQuality` must treat a response carrying + * `reasoning_content` (Kimi-K2.5-TEE, GLM-5-TEE, etc.) as valid even when + * `content` is null. The previous implementation only inspected `content` + * and `tool_calls`, so reasoning models triggered a false-positive + * "empty content" 502 and an unnecessary combo fallback. + */ +import test from "node:test"; +import assert from "node:assert/strict"; + +const { validateResponseQuality } = await import("../../open-sse/services/combo.ts"); + +function makeResponse(body: unknown, contentType = "application/json"): Response { + return new Response(JSON.stringify(body), { + status: 200, + headers: { "content-type": contentType }, + }); +} + +const silentLog = { warn: () => {} }; + +test("#2341 reasoning_content with null content is treated as valid", async () => { + const res = makeResponse({ + choices: [ + { + message: { + content: null, + reasoning_content: " The user simply said 'Say OK'. OK. ", + }, + }, + ], + }); + const out = await validateResponseQuality(res, false, silentLog); + assert.equal(out.valid, true, `expected valid, got reason: ${out.reason}`); +}); + +test("#2341 legacy `reasoning` field is also recognized", async () => { + // Some upstream variants use `reasoning` (no `_content` suffix). + const res = makeResponse({ + choices: [ + { + message: { + content: null, + reasoning: "Step-by-step deduction body here.", + }, + }, + ], + }); + const out = await validateResponseQuality(res, false, silentLog); + assert.equal(out.valid, true, `expected valid, got reason: ${out.reason}`); +}); + +test("#2341 empty reasoning_content + empty content + no tool_calls still rejected", async () => { + // Regression guard: the new branch must not weaken the empty-response check. + const res = makeResponse({ + choices: [ + { + message: { + content: null, + reasoning_content: " ", + }, + }, + ], + }); + const out = await validateResponseQuality(res, false, silentLog); + assert.equal(out.valid, false); + assert.match(out.reason ?? "", /empty content/i); +}); + +test("#2341 normal content-only response remains valid (backward compat)", async () => { + const res = makeResponse({ + choices: [{ message: { content: "Hello world." } }], + }); + const out = await validateResponseQuality(res, false, silentLog); + assert.equal(out.valid, true); +}); + +test("#2341 tool_calls-only response remains valid (backward compat)", async () => { + const res = makeResponse({ + choices: [ + { + message: { + content: null, + tool_calls: [{ id: "c1", type: "function", function: { name: "x", arguments: "{}" } }], + }, + }, + ], + }); + const out = await validateResponseQuality(res, false, silentLog); + assert.equal(out.valid, true); +}); + +test("#2341 reasoning_content as non-string is ignored (defensive)", async () => { + const res = makeResponse({ + choices: [ + { + message: { + content: null, + reasoning_content: { unexpected: "object" }, + }, + }, + ], + }); + const out = await validateResponseQuality(res, false, silentLog); + // Non-string reasoning_content shouldn't count as content; still rejected. + assert.equal(out.valid, false); +});