From a32e81c8e823156f39d6ee5af89ee5f68ebe5e39 Mon Sep 17 00:00:00 2001 From: licheer-zte Date: Sat, 8 Aug 2026 22:48:17 +0800 Subject: [PATCH] fix(model-fallback): treat empty non-GPT completions as failed candidates (#120132) (#120148) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(model-fallback): treat empty non-GPT completions as failed candidates (#120132) Empty and whitespace-only completions from non-GPT models were counted as candidate_succeeded, silently dropping the turn on visible channels. Apply the empty/reasoning-only classification to every model; deliberate silent replies and committed outbound deliveries remain successful. * fix(model-fallback): classify mixed reasoning-plus-blank completions as failed (#120148) A completion like [{ isReasoning: true, text: "thinking" }, { text: " " }] carries no user-visible reply: reasoning text is invisible to the shared visibility test (includeReasoningPayloads: false), so counting it as visible made the run look successful and silently ended visible-channel turns. Filter reasoning payloads out of the empty/whitespace predicate so mixed reasoning-plus-blank results classify as empty_result (fallback-worthy), while mixed reasoning-plus-visible-text results stay successful. Regression tests: mixed reasoning+blank -> empty_result; mixed reasoning+visible -> success. * fix(model-fallback): require deliverable assistant results Use one owner-boundary deliverability predicate for fallback classification, preserve intentional terminal outcomes, and add a mock-channel Gateway scenario for mixed reasoning-plus-blank recovery.\n\nCo-authored-by: 李琪0668001400 * chore: preserve contributor credit Co-authored-by: 李琪0668001400 * test(qa): cover default model fallback scenario Make the mixed reasoning-plus-blank fixture recover through both the catalog default alternate and the explicit proof model. Co-authored-by: 李琪0668001400 --------- Co-authored-by: licheer-zte Co-authored-by: Peter Steinberger --- .../mock-openai/mock-openai-contracts.ts | 2 + .../src/providers/mock-openai/server.test.ts | 26 ++++ .../src/providers/mock-openai/server.ts | 17 ++- .../mixed-reasoning-blank-model-fallback.yaml | 115 ++++++++++++++++++ .../result-fallback-classifier.test.ts | 83 +++++++++++++ .../result-fallback-classifier.ts | 82 +++++++------ .../outcome-fallback-runtime-contract.test.ts | 17 ++- 7 files changed, 294 insertions(+), 48 deletions(-) create mode 100644 qa/scenarios/runtime/mixed-reasoning-blank-model-fallback.yaml diff --git a/extensions/qa-lab/src/providers/mock-openai/mock-openai-contracts.ts b/extensions/qa-lab/src/providers/mock-openai/mock-openai-contracts.ts index 29414bedce28..3299819d3d8b 100644 --- a/extensions/qa-lab/src/providers/mock-openai/mock-openai-contracts.ts +++ b/extensions/qa-lab/src/providers/mock-openai/mock-openai-contracts.ts @@ -222,6 +222,8 @@ export const TINY_PNG_BASE64 = "iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mP8/x8AAwMCAO7Z0nQAAAAASUVORK5CYII="; export const QA_REASONING_ONLY_RECOVERY_PROMPT_RE = /reasoning-only continuation qa check/i; export const QA_REASONING_ONLY_SIDE_EFFECT_PROMPT_RE = /reasoning-only after write safety check/i; +export const QA_MIXED_REASONING_BLANK_FALLBACK_PROMPT_RE = + /mixed reasoning blank fallback qa check/i; export const QA_ANTHROPIC_THINKING_ERROR_RECOVERY_PROMPT_RE = /anthropic thinking error qa check/i; export const QA_THINKING_VISIBILITY_OFF_PROMPT_RE = /qa thinking visibility check off/i; export const QA_THINKING_VISIBILITY_MAX_PROMPT_RE = /qa thinking visibility check max/i; diff --git a/extensions/qa-lab/src/providers/mock-openai/server.test.ts b/extensions/qa-lab/src/providers/mock-openai/server.test.ts index a0396391d1b8..8f72fd0ffe57 100644 --- a/extensions/qa-lab/src/providers/mock-openai/server.test.ts +++ b/extensions/qa-lab/src/providers/mock-openai/server.test.ts @@ -18,6 +18,8 @@ const QA_REASONING_ONLY_RECOVERY_PROMPT = "Reasoning-only continuation QA check: read QA_KICKOFF_TASK.md, then answer with exactly REASONING-RECOVERED-OK."; const QA_REASONING_ONLY_SIDE_EFFECT_PROMPT = "Reasoning-only after write safety check: write reasoning-only-side-effect.txt, then answer with exactly SIDE-EFFECT-GUARD-OK."; +const QA_MIXED_REASONING_BLANK_FALLBACK_PROMPT = + "Mixed reasoning blank fallback QA check: recover through the alternate model."; const QA_THINKING_VISIBILITY_OFF_PROMPT = "QA thinking visibility check off: answer exactly THINKING-OFF-OK."; const QA_THINKING_VISIBILITY_MAX_PROMPT = @@ -7920,6 +7922,30 @@ Update and merge these partial structured summaries.`, ); }); + it.each([ + { name: "default", primaryModel: "gpt-5.6-luna", fallbackModel: "gpt-5.6-luna-alt" }, + { + name: "explicit", + primaryModel: "mock-empty-primary", + fallbackModel: "mock-visible-fallback", + }, + ])("scripts mixed reasoning-plus-blank output for the $name model pair", async (models) => { + const server = await startMockServer(); + + const primary = await expectOpenAiNonStreamingResponsesJson(server, { + model: models.primaryModel, + input: [makeUserInput(QA_MIXED_REASONING_BLANK_FALLBACK_PROMPT)], + }); + expect(outputItems(primary).map((item) => item.type)).toEqual(["reasoning", "message"]); + expect(outputText(primary, 1)).toBe(" "); + + const fallback = await expectOpenAiNonStreamingResponsesJson(server, { + model: models.fallbackModel, + input: [makeUserInput(QA_MIXED_REASONING_BLANK_FALLBACK_PROMPT)], + }); + expect(outputText(fallback)).toBe("MODEL-FALLBACK-VISIBLE-OK"); + }); + it("scripts the GPT-5.6 Luna thinking visibility switch prompts", async () => { const server = await startMockServer(); diff --git a/extensions/qa-lab/src/providers/mock-openai/server.ts b/extensions/qa-lab/src/providers/mock-openai/server.ts index d78c3157eccb..c406f86a7e01 100644 --- a/extensions/qa-lab/src/providers/mock-openai/server.ts +++ b/extensions/qa-lab/src/providers/mock-openai/server.ts @@ -32,6 +32,7 @@ import { TINY_PNG_BASE64, QA_REASONING_ONLY_RECOVERY_PROMPT_RE, QA_REASONING_ONLY_SIDE_EFFECT_PROMPT_RE, + QA_MIXED_REASONING_BLANK_FALLBACK_PROMPT_RE, QA_ANTHROPIC_THINKING_ERROR_RECOVERY_PROMPT_RE, QA_THINKING_VISIBILITY_OFF_PROMPT_RE, QA_THINKING_VISIBILITY_MAX_PROMPT_RE, @@ -789,9 +790,8 @@ async function buildResponsesPayload( compactionSummaryFaultMode?: MockCompactionSummaryFaultMode; } = {}, ) { - const providerVariant = resolveProviderVariant( - typeof body.model === "string" ? body.model : undefined, - ); + const model = typeof body.model === "string" ? body.model : ""; + const providerVariant = resolveProviderVariant(model); const input = normalizeResponsesInput(body.input); const toolDeclarationBody = resolveCurrentToolDeclarationSurface(body, input); const prompt = extractLastUserText(input); @@ -1352,6 +1352,17 @@ async function buildResponsesPayload( } return buildAssistantEvents("BUG-SHOULD-NOT-AUTO-RETRY"); } + if (QA_MIXED_REASONING_BLANK_FALLBACK_PROMPT_RE.test(allInputText)) { + // The catalog's default mock alternate and the explicit proof model both + // recover, so the scenario exercises the same fallback path with or without flags. + if (model === "gpt-5.6-luna-alt" || model === "mock-visible-fallback") { + return buildAssistantEvents("MODEL-FALLBACK-VISIBLE-OK"); + } + return buildReasoningAndAssistantEvents({ + reasoningId: `rs_mock_mixed_blank_${model.replaceAll(/[^a-z0-9]+/gi, "_")}`, + answerText: " ", + }); + } if (QA_THINKING_VISIBILITY_MAX_PROMPT_RE.test(prompt)) { return buildReasoningAndAssistantEvents({ reasoningId: "rs_mock_thinking_visibility_max", diff --git a/qa/scenarios/runtime/mixed-reasoning-blank-model-fallback.yaml b/qa/scenarios/runtime/mixed-reasoning-blank-model-fallback.yaml new file mode 100644 index 000000000000..e835db3afdd0 --- /dev/null +++ b/qa/scenarios/runtime/mixed-reasoning-blank-model-fallback.yaml @@ -0,0 +1,115 @@ +title: Mixed reasoning-plus-blank model fallback + +scenario: + id: mixed-reasoning-blank-model-fallback + surface: runtime + coverage: + primary: + - agent-runtime.failure-recovery-empty-response-recovery + - channels.qa-channel-final-reply + secondary: + - agent-runtime.failure-recovery-retry-policy + objective: Verify a mixed reasoning-plus-whitespace completion advances non-GPT model fallback to one visible answer. + successCriteria: + - The scenario runs through qa-channel, the scenario-aware mock provider, and an ephemeral Gateway child. + - The primary non-GPT model returns one reasoning item plus one whitespace-only final-answer item. + - The fallback runner rejects that invisible completion and attempts the configured alternate model. + - The alternate model's exact marker reaches the channel exactly once. + docsRefs: + - docs/help/testing.md + - docs/channels/qa-channel.md + codeRefs: + - extensions/qa-lab/src/providers/mock-openai/server.ts + - src/agents/embedded-agent-runner/result-fallback-classifier.ts + - src/agents/model-fallback-runner.ts + execution: + kind: flow + providerMode: mock-openai + retryCount: 0 + summary: Exercise mixed invisible output through mock provider, model fallback, ephemeral Gateway, and qa-channel delivery. + config: + requiredProviderMode: mock-openai + promptSnippet: Mixed reasoning blank fallback QA check + prompt: "Mixed reasoning blank fallback QA check: recover through the alternate model." + expectedReply: MODEL-FALLBACK-VISIBLE-OK + +flow: + steps: + - name: advances the invisible primary candidate and delivers the fallback + actions: + - assert: + expr: "env.providerMode === config.requiredProviderMode" + message: + expr: "`expected provider mode ${config.requiredProviderMode}, got ${env.providerMode}`" + - call: waitForGatewayHealthy + args: + - ref: env + - 60000 + - call: reset + - set: outboundStartIndex + value: + expr: "state.getSnapshot().messages.filter((message) => message.direction === 'outbound').length" + - set: requestCursorBefore + value: + expr: "(await fetchJson(`${env.mock.baseUrl}/debug/request-cursor`)).cursor" + - set: sessionKey + value: + expr: "`agent:qa:mixed-reasoning-blank:${randomUUID().slice(0, 8)}`" + - call: startAgentRun + saveAs: started + args: + - ref: env + - sessionKey: + ref: sessionKey + message: + expr: config.prompt + timeoutMs: + expr: liveTurnTimeoutMs(env, 45000) + - set: waited + value: + expr: "await env.gateway.call('agent.wait', { runId: started.runId, timeoutMs: liveTurnTimeoutMs(env, 45000) }, { timeoutMs: liveTurnTimeoutMs(env, 50000) })" + - assert: + expr: "['ok', 'completed', 'succeeded'].includes(String(waited?.status)) || (waited?.status === 'error' && String(waited?.error ?? '').trim().toLowerCase() === 'completed')" + message: + expr: "`agent.wait returned ${String(waited?.status ?? 'unknown')}: ${String(waited?.error ?? '')}`" + - call: waitForCondition + saveAs: outbound + args: + - lambda: + expr: "state.getSnapshot().messages.filter((message) => message.direction === 'outbound').slice(outboundStartIndex).find((message) => message.conversation.id === 'qa-operator')" + - expr: liveTurnTimeoutMs(env, 30000) + - 100 + - call: sleep + args: + - 300 + - set: scenarioOutbound + value: + expr: "state.getSnapshot().messages.filter((message) => message.direction === 'outbound').slice(outboundStartIndex).filter((message) => message.conversation.id === 'qa-operator')" + - set: scenarioRequests + value: + expr: "(await fetchJson(`${env.mock.baseUrl}/debug/requests?after=${requestCursorBefore}`)).filter((request) => String(request.allInputText ?? '').includes(config.promptSnippet))" + - set: expectedModels + value: + expr: "[splitModelRef(env.primaryModel)?.model, splitModelRef(env.alternateModel)?.model]" + - set: primaryRequests + value: + expr: "scenarioRequests.filter((request) => request.model === expectedModels[0])" + - set: fallbackRequests + value: + expr: "scenarioRequests.filter((request) => request.model === expectedModels[1])" + - assert: + expr: "expectedModels.every((model) => typeof model === 'string' && model.length > 0) && primaryRequests.length > 0 && fallbackRequests.length === 1 && scenarioRequests.at(-1) === fallbackRequests[0] && scenarioRequests.every((request) => !request.plannedToolName)" + message: + expr: "`expected ordered mixed-output primary retries then one visible fallback ${JSON.stringify(expectedModels)}, saw ${JSON.stringify(scenarioRequests.map((request) => ({ model: request.model, tool: request.plannedToolName ?? null })))}`" + - assert: + expr: "scenarioOutbound.length === 1 && String(outbound.text ?? '') === config.expectedReply" + message: + expr: "`expected one visible fallback reply, saw ${JSON.stringify(scenarioOutbound.map((message) => String(message.text ?? '')))}`" + - set: verdict + value: + expr: "({ verdict: 'PASS', harness: 'qa-channel + qa-lab bus + ephemeral Gateway child + mock-openai', primaryOutput: 'reasoning item plus whitespace-only final answer', primaryModel: expectedModels[0], fallbackModel: expectedModels[1], primaryRequestCount: primaryRequests.length, fallbackRequestCount: fallbackRequests.length, attemptedModels: scenarioRequests.map((request) => request.model), visibleTerminalPayloads: scenarioOutbound.map((message) => String(message.text ?? '')), outboundCount: scenarioOutbound.length, silentDropPrevented: scenarioOutbound.length === 1, pass: primaryRequests.length > 0 && fallbackRequests.length === 1 && scenarioOutbound.length === 1 && String(outbound.text ?? '') === config.expectedReply })" + - assert: + expr: verdict.pass === true + message: + expr: "`mixed reasoning fallback verdict failed: ${JSON.stringify(verdict)}`" + detailsExpr: "JSON.stringify(verdict, null, 2)" diff --git a/src/agents/embedded-agent-runner/result-fallback-classifier.test.ts b/src/agents/embedded-agent-runner/result-fallback-classifier.test.ts index 685bd74aba38..30f02c692393 100644 --- a/src/agents/embedded-agent-runner/result-fallback-classifier.test.ts +++ b/src/agents/embedded-agent-runner/result-fallback-classifier.test.ts @@ -437,6 +437,89 @@ describe("classifyEmbeddedAgentRunResultForModelFallback", () => { expect(result).toBeNull(); }); + it.each([ + { + name: "empty", + payloads: [], + code: "empty_result", + suffix: "without a visible assistant reply", + }, + { + name: "whitespace-only", + payloads: [{ text: " " }], + code: "empty_result", + suffix: "without a visible assistant reply", + }, + { + name: "reasoning-only", + payloads: [{ isReasoning: true, text: "thinking about the answer" }], + code: "reasoning_only_result", + suffix: "with reasoning only", + }, + { + name: "mixed reasoning-plus-blank", + payloads: [{ isReasoning: true, text: "thinking about the answer" }, { text: " " }], + code: "empty_result", + suffix: "without a visible assistant reply", + }, + { + name: "commentary-only", + payloads: [{ isCommentary: true, text: "progress only" }], + code: "empty_result", + suffix: "without a visible assistant reply", + }, + { + name: "explicitly hidden", + payloads: [{ visible: false, text: "internal" }], + code: "empty_result", + suffix: "without a visible assistant reply", + }, + { + name: "blank error", + payloads: [{ isError: true, text: " " }], + code: "empty_result", + suffix: "without a visible assistant reply", + }, + ])("classifies $name non-GPT completions as fallback-worthy", ({ payloads, code, suffix }) => { + const result = classifyEmbeddedAgentRunResultForModelFallback({ + provider: "zai", + model: "glm-5.2", + result: { + payloads, + meta: { durationMs: 42 }, + }, + }); + + expect(result).toEqual({ + message: `zai/glm-5.2 ended ${suffix}`, + reason: "format", + code, + }); + }); + + it.each([ + { + name: "mixed reasoning-plus-visible text", + payloads: [{ isReasoning: true, text: "thinking" }, { text: "Here is the answer" }], + }, + { name: "media-only", payloads: [{ mediaUrl: "https://example.test/result.png" }] }, + { + name: "rich error", + payloads: [{ isError: true, mediaUrl: "https://example.test/error.png" }], + }, + ])("keeps $name completions successful", ({ payloads }) => { + const result = classifyEmbeddedAgentRunResultForModelFallback({ + provider: "zai", + model: "glm-5.2", + result: { + payloads, + meta: { durationMs: 42 }, + }, + }); + + expect(result).toBeNull(); + }); + it("keeps side-effecting incomplete tool turns out of fallback before harness classification", () => { const result = classifyEmbeddedAgentRunResultForModelFallback({ provider: "openai", diff --git a/src/agents/embedded-agent-runner/result-fallback-classifier.ts b/src/agents/embedded-agent-runner/result-fallback-classifier.ts index 34b009ebaf24..44aeab7d7a4e 100644 --- a/src/agents/embedded-agent-runner/result-fallback-classifier.ts +++ b/src/agents/embedded-agent-runner/result-fallback-classifier.ts @@ -5,7 +5,6 @@ import { GENERIC_EXTERNAL_RUN_FAILURE_TEXT } from "../../auto-reply/reply/agent- import { isSilentReplyPayloadText } from "../../auto-reply/tokens.js"; import { classifyFailoverReason } from "../embedded-agent-helpers/errors.js"; import type { FailoverReason } from "../embedded-agent-helpers/types.js"; -import { isGpt5ModelId } from "../gpt5-prompt-overlay.js"; import type { ModelFallbackResultClassification } from "../model-fallback-attempt.js"; import { hasCommittedOutboundDeliveryEvidence, @@ -81,17 +80,36 @@ export function hasDeliberateSilentTerminalReply(result: EmbeddedAgentRunResult) ); } +function hasDeliverableAssistantPayload(result: { + payloads?: unknown; + meta?: { finalAssistantVisibleText?: unknown }; +}): boolean { + const finalVisibleText = result.meta?.finalAssistantVisibleText; + const payloads = Array.isArray(result.payloads) + ? result.payloads.filter((payload) => { + if (!payload || typeof payload !== "object" || Array.isArray(payload)) { + return true; + } + const record = payload as { isCommentary?: unknown; visible?: unknown }; + return record.isCommentary !== true && record.visible !== false; + }) + : []; + return ( + (typeof finalVisibleText === "string" && + finalVisibleText.trim().length > 0 && + !isSilentReplyPayloadText(finalVisibleText)) || + hasVisibleAgentPayload( + { payloads }, + { includeErrorPayloads: false, includeReasoningPayloads: false }, + ) + ); +} + function hasNonTextVisiblePayloadContent( payload: NonNullable[number], ): boolean { - const { text: _text, ...payloadWithoutText } = payload; - return hasVisibleAgentPayload( - { payloads: [payloadWithoutText] }, - { - includeErrorPayloads: false, - includeReasoningPayloads: false, - }, - ); + const { isError: _isError, text: _text, ...payloadWithoutText } = payload; + return hasDeliverableAssistantPayload({ payloads: [payloadWithoutText] }); } function classifyGenericExternalRunFailurePayload(params: { @@ -216,19 +234,7 @@ export function classifyEmbeddedAgentRunResultForModelFallback(params: { if (genericExternalFailureClassification) { return genericExternalFailureClassification; } - if ( - typeof params.result.meta.finalAssistantVisibleText === "string" && - params.result.meta.finalAssistantVisibleText.trim().length > 0 && - !isSilentReplyPayloadText(params.result.meta.finalAssistantVisibleText) - ) { - return null; - } - if ( - hasVisibleAgentPayload(params.result, { - includeErrorPayloads: false, - includeReasoningPayloads: false, - }) - ) { + if (hasDeliverableAssistantPayload(params.result)) { return null; } if (fallbackSafeIncompleteTurn) { @@ -270,29 +276,33 @@ export function classifyEmbeddedAgentRunResultForModelFallback(params: { }; } - if (!isGpt5ModelId(params.model)) { + // Once the shared visibility owner finds no deliverable assistant payload, + // empty and reasoning-only output must advance fallback for every model. + if (hasDeliberateSilentTerminalReply(params.result)) { return null; } - - // Legacy GPT-5 handling treats empty/reasoning-only payloads as fallback - // candidates, while deliberate silent replies remain successful terminal work. - if (payloads.length === 0 && hasDeliberateSilentTerminalReply(params.result)) { + if (errorText.trim()) { return null; } - if (payloads.length === 0) { - return { - message: `${params.provider}/${params.model} ended without a visible assistant reply`, - reason: "format", - code: "empty_result", - }; + if ( + payloads.some((payload) => payload.isError === true && hasNonTextVisiblePayloadContent(payload)) + ) { + return null; } - if (payloads.every((payload) => payload.isReasoning === true)) { + const assistantPayloads = payloads.filter((payload) => payload.isError !== true); + if ( + assistantPayloads.length > 0 && + assistantPayloads.every((payload) => payload.isReasoning === true) + ) { return { message: `${params.provider}/${params.model} ended with reasoning only`, reason: "format", code: "reasoning_only_result", }; } - - return null; + return { + message: `${params.provider}/${params.model} ended without a visible assistant reply`, + reason: "format", + code: "empty_result", + }; } diff --git a/src/agents/outcome-fallback-runtime-contract.test.ts b/src/agents/outcome-fallback-runtime-contract.test.ts index 401a4a171629..202dfd528724 100644 --- a/src/agents/outcome-fallback-runtime-contract.test.ts +++ b/src/agents/outcome-fallback-runtime-contract.test.ts @@ -60,12 +60,11 @@ describe("Outcome/fallback runtime contract - embedded runtime fallback classifi }, ); - it("advances to the configured fallback after a classified GPT-5 terminal result", async () => { + it("advances to the configured fallback after an invisible non-GPT terminal result", async () => { + const primaryProvider = "zai"; + const primaryModel = "glm-5.2"; const primary = createContractRunResult({ - meta: { - durationMs: 1, - agentHarnessResultClassification: "empty", - }, + payloads: [{ isReasoning: true, text: "thinking" }, { text: " " }], }); const fallback = createContractRunResult({ payloads: [{ text: "fallback ok" }], @@ -75,8 +74,8 @@ describe("Outcome/fallback runtime contract - embedded runtime fallback classifi const result = await runWithModelFallback>({ cfg: undefined, - provider: OUTCOME_FALLBACK_RUNTIME_CONTRACT.primaryProvider, - model: OUTCOME_FALLBACK_RUNTIME_CONTRACT.primaryModel, + provider: primaryProvider, + model: primaryModel, fallbacksOverride: contractFallbackOverride, run, classifyResult: ({ provider, model, result: resultValue }) => @@ -97,8 +96,8 @@ describe("Outcome/fallback runtime contract - embedded runtime fallback classifi OUTCOME_FALLBACK_RUNTIME_CONTRACT.fallbackModel, { isFinalFallbackAttempt: true }, ]); - expect(result.attempts[0]?.provider).toBe(OUTCOME_FALLBACK_RUNTIME_CONTRACT.primaryProvider); - expect(result.attempts[0]?.model).toBe(OUTCOME_FALLBACK_RUNTIME_CONTRACT.primaryModel); + expect(result.attempts[0]?.provider).toBe(primaryProvider); + expect(result.attempts[0]?.model).toBe(primaryModel); expect(result.attempts[0]?.reason).toBe("format"); expect(result.attempts[0]?.code).toBe("empty_result"); });