fix(core): retry malformed streamed responses - #6754
Conversation
|
(Re-run after 10 commits addressing maintainer review feedback.) Template looks good ✓ Problem: observed production bug. Issue #6666 documents Qwen3 3.7 max returning raw Direction: clearly aligned — retry malformed streams instead of showing corrupted output to users. Standard resilience pattern for streaming APIs. Size: ~132 production logic lines, ~296 test lines. Well under the 500-line advisory. All changes in Approach: scope is tight. Two detection paths (nameless tool calls, thinking tag leaks) + unified retry budget + hold-and-release buffering. Each piece is needed for the stated goal. The buffering mechanism ( Moving on to code review. 🔍 中文说明(在 maintainer review 反馈后经过 10 次 commit 的重新运行。) 模板完整 ✓ 问题:已观测到的生产 bug。Issue #6666 记录了 Qwen3 3.7 max 在 方向:明确对齐——重试 malformed stream 而不是向用户展示损坏的输出。流式 API 的标准容错模式。 规模:约 132 行生产逻辑,约 296 行测试。远低于 500 行建议阈值。所有更改在 方案:范围紧凑。两条检测路径(nameless tool calls、thinking tag leaks)+ 统一 retry 预算 + hold-and-release 缓冲。每一部分都是实现目标所必需的。缓冲机制( 进入代码审查 🔍 — Qwen Code · qwen3.7-max |
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
|
(Re-run — head Code ReviewThe implementation handles two malformed stream shapes cleanly: Nameless tool calls: Thinking tag leak detection: Retry budget: unified to Reuse: No critical blockers. No AGENTS.md violations. Test ResultsAll relevant test suites pass: TypeScript typecheck: clean (no errors). Real-Scenario TestingMaintainer wenshao already verified with a local SSE mock replaying the exact #6666 malformed stream shapes and confirmed the fix works as described:
The author also provided a validation matrix covering 10 scenarios (plain response, reasoning + content, literal 中文说明代码审查实现干净地处理了两种 malformed stream 形态: Nameless tool calls: Thinking tag leak 检测: Retry 预算:统一为 无关键阻塞。无 AGENTS.md 违规。 测试结果所有相关测试套件通过:449/449。TypeScript typecheck 无错误。 真实场景测试Maintainer wenshao 已使用本地 SSE mock 重放 #6666 的 malformed stream 形态验证,确认修复按描述工作。 — Qwen Code · qwen3.7-max |
|
(Re-run — head This is a well-executed fix for a real production bug. The PR went through 10 iterations addressing maintainer feedback, and the result is tight: two focused detection paths, a clean buffering mechanism, and comprehensive test coverage (449 tests, including 12+ new test cases covering every edge case in the PR description). The author clearly understands the problem domain — the detection correctly distinguishes between "literal The unified retry budget simplifies the code without sacrificing correctness. The hold-and-release buffering prevents the worst UX outcome (flashing corrupted content in the TUI) while not delaying healthy responses. Maintainer wenshao reviewed in depth, requested changes, got them addressed, verified with real SSE mock testing, and approved. All unit tests and typecheck pass. Approving. ✅ 中文说明这是一个针对真实生产 bug 的高质量修复。PR 经过 10 次迭代处理 maintainer 反馈,最终结果紧凑:两条聚焦的检测路径、干净的缓冲机制和全面的测试覆盖(449 个测试,包括 12+ 个新测试用例覆盖 PR 描述中的每个边缘情况)。 作者清楚理解问题域——检测正确区分了"用户故意写的 literal 统一的 retry 预算简化了代码而不牺牲正确性。hold-and-release 缓冲防止了最差的 UX 结果(在 TUI 中闪现损坏内容),同时不延迟正常响应。 Maintainer wenshao 深入审查、请求更改、得到处理、通过真实 SSE mock 测试验证并批准。所有单元测试和 typecheck 通过。 批准 ✅ — Qwen Code · qwen3.7-max |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
|
本轮按最小范围处理 review:
验证:
|
Validation matrixI revalidated the latest head (
Tmux E2E evidenceAll tmux scenarios used the locally built bundle with a deterministic OpenAI-compatible SSE mock server.
The tmux assertions also confirmed that the test sessions and mock servers were cleaned up after execution. Verification summary
The resulting boundary is intentionally narrow:
|
ReviewOverviewThe PR makes What I verified against the PR head (
The two failure shapes from #6666 are real and the protocol-level framing (structured reasoning ⇒ visible thinking tags are corruption) is a defensible boundary. However, I found one correctness bug and one false-positive surface that I think need addressing before merge. Critical: duplicate finish chunks defeat the drop and emit the withheld tool call
// chunk 1: reasoning_content 'hidden reasoning' → thought part emitted
// chunk 2: content '</think> leaked …' + named tool call, finish 'tool_calls'
// → dropped, finishReason undefined (correct)
// chunk 3: empty delta, finish 'tool_calls' (duplicate finish)
// → parts: [{functionCall: {id: 'call_named', name: 'run_shell_command', args: {}}}]
// → finishReason: STOP ← malformed attempt accepted after allSo the exact attempt the PR means to discard gets accepted — worse, with its visible text stripped, and the tool call from the corrupted stream reaches execution ( Suggested fix: latch the decision on the context — e.g. set High: benign mentions of thinking tags become a hard turn failure on reasoning models
// reasoning_content: 'user asks about qwen tags'
// content: 'In Qwen output, the `<think>` tag wraps hidden reasoning text.'
// finish 'stop' → parts: [], finishReason: undefined → InvalidStreamError → retryBecause the model will regenerate substantially the same (correct) answer, all 4 transient retries burn (~20s of backoff plus 5 generations) and the turn fails outright. The word-boundary in The literal-tag preservation test only covers the no-structured-reasoning case, which excludes precisely the models most likely to be asked about thinking tags. Consider requiring a leak signature instead of any occurrence — the #6666 traces have distinctive shapes the benign case doesn't: an unmatched Medium: one tag mention disables streaming for the rest of the responseOnce anything is held, Low
Test coverageThe new tests are thorough on the intended drop/hold/flush shapes (split tags, same-chunk, clean-gap stickiness, tool-call suppression). Missing cases that map to the findings above: a duplicate-finish chunk after a dropped attempt (Critical repro), and a benign backticked mention under structured reasoning asserting whatever behavior is decided for it. |
wenshao
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
— qwen3.7-max via Qwen Code /review
|
Addressed in The duplicate-finish case is no longer reachable on the production path because the first malformed finish throws Focused tests now pass 476/476; lint, build, and typecheck also pass. |
Local build & real-run verification (head
|
| Check | Result |
|---|---|
npm run build (core + cli) |
✅ pass, CLI v0.19.9 boots |
tsc --noEmit (core) |
✅ pass |
eslint on the 5 changed source files |
✅ pass |
Focused vitest: converter.test.ts + geminiChat.test.ts |
✅ 379/379 |
Full core suite (vitest run, 15218 tests) |
✅ 15206 pass / 8 skipped / 4 fail — the 4 failures are all in src/memory/refresh.test.ts and fail identically on merge-base 5b4d0a358 (pre-existing local-env issue, unrelated to this diff) |
End-to-end: built CLI vs. a mock provider replaying the #6666 stream
Method: the built packages/cli/dist/index.js was driven in -p mode and interactively (pty + xterm), with OPENAI_BASE_URL pointed at a local SSE mock. The mock picks a stream script per scenario and counts HTTP requests server-side, so attempt counts are provider-observed, not client-claimed. The "before" build is the same worktree with packages/core/src overlaid from merge-base 5b4d0a358 and rebuilt.
Key results:
- Recorded cross-channel leak (structured reasoning +
</think>/<think>fragments in visible content): main accepts the malformed attempt — raw tags and the corrupted remainder are shown and recorded, exit 0. The PR build discards the attempt (PROTOCOL_TAG_LEAK), retries once, and only the clean second response reaches the UI. The mock observed exactly 2 requests. A same-chunk variant (reasoning_contentand<think>…</think>content in a single delta, test-plan item 5) was also discarded and retried cleanly — 2 provider-observed requests, zero leaked text. - Closing tag split across deltas (
</thi+nk>): same before/after contrast — main shows the reassembled tag; the PR retries to a clean response. - Nameless tool call (id + arguments, no function name): main silently drops the tool call and accepts the corrupted visible text as the final answer; the PR withholds the finish signal, the attempt is discarded through the invalid-stream path, and the retry recovers cleanly.
- Retry budget: with a mock that returns the malformed stream on every attempt, the PR build makes exactly 5 attempts (1 + 4 retries), exits non-zero, and never prints any leaked fragment — matching the unified
maxRetries: 4inINVALID_STREAM_RETRY_CONFIG. - No false positives: healthy
reasoning_content+ content, and literal<think>example</think>text without a reasoning channel, behave identically on both builds — single request, no spurious retry, literal tags preserved verbatim.
Interactive TUI, same mock stream and prompt on both builds:
Chat-record (JSONL) assertions
- PR-build leak session:
grep-verified zero occurrences of the leaked payload (</think>, "presubmit report", corrupted remainder) in the session JSONL; the recovered answer is present. - PR-build literal session: the two
</think>occurrences in the record are the preserved literal text (correct). - Main-build sessions recorded the leaked payload in the chat JSONL, confirming the before behavior extended to persistence.
Observations (non-blocking)
- After a discarded attempt, the TUI keeps one extra collapsed
∴ Thought for 0srow per retry (visible in the after-screenshot). No leaked content is shown or recorded — purely cosmetic, and arguably useful as a retry trace. - The 2s initial retry delay makes the worst case (budget exhaustion) take ~20s wall-clock; that matches the existing invalid-stream behavior and seems fine.
Verdict: behavior matches the PR description on all five claims of the reviewer test plan. LGTM from a runtime-verification standpoint.
中文版本(Chinese version)
本地构建与真实运行验证(head 1d7d497f9)
我在本地构建了此 PR,并用真实 CLI 二进制对着一个本地 OpenAI 兼容 SSE mock(回放 #6666 记录的畸形流形态)端到端地验证了所有重试路径。结论:修复行为与描述一致,healthy 流和字面 <think> 文本均未观察到回归——证据见上文截图。
环境: macOS (darwin 24.6)、Node v22.23.1、隔离 worktree(head 1d7d497f9)、隔离 $HOME。
构建与静态检查
| 检查项 | 结果 |
|---|---|
npm run build(core + cli) |
✅ 通过,CLI v0.19.9 可启动 |
tsc --noEmit(core) |
✅ 通过 |
对 5 个改动源文件运行 eslint |
✅ 通过 |
焦点测试:converter.test.ts + geminiChat.test.ts |
✅ 379/379 |
core 全量套件(vitest run,15218 个测试) |
✅ 15206 通过 / 8 跳过 / 4 失败——4 个失败全部位于 src/memory/refresh.test.ts,且在 merge-base 5b4d0a358 上以完全相同方式失败(本地环境存量问题,与本 diff 无关) |
端到端:构建后的 CLI 对抗回放 #6666 流的 mock provider
方法:用构建产物 packages/cli/dist/index.js 分别以 -p 模式和交互式(pty + xterm)运行,OPENAI_BASE_URL 指向本地 SSE mock。mock 按场景回放流脚本并在服务端统计 HTTP 请求数,因此attempt 次数是 provider 侧观测值,而非客户端自述。“before” 构建为同一 worktree 覆盖 merge-base 5b4d0a358 的 packages/core/src 后重新构建。
关键结果:
- 记录的跨通道泄漏(structured reasoning + 可见内容中的
</think>/<think>残片):main 接受该畸形 attempt——原始标签和损坏正文直接显示并被记录,exit 0;PR 构建丢弃该 attempt(PROTOCOL_TAG_LEAK)并重试一次,只有干净的第二次响应进入 UI。mock 服务端恰好观测到 2 次请求。same-chunk 变体(同一个 delta 里同时含reasoning_content和<think>…</think>内容,即测试计划第 5 条)同样被丢弃并干净重试——provider 侧观测 2 次请求,零泄漏文本。 - 闭合标签跨 delta 拆分(
</thi+nk>):对比相同——main 显示拼接后的标签;PR 重试后输出干净。 - 无名 tool call(有 id + arguments、无 function name):main 静默丢弃该 tool call 并把损坏的可见文本当作最终答案;PR 扣留 finish 信号,该 attempt 走 invalid-stream 路径被丢弃,重试后干净恢复。
- 重试预算:当 mock 每次 都返回畸形流时,PR 构建恰好发起 5 次 attempt(1 + 4 次重试),以非零退出码结束,且未打印任何泄漏残片——与
INVALID_STREAM_RETRY_CONFIG统一后的maxRetries: 4一致。 - 无误伤:healthy 的
reasoning_content+ content,以及没有 reasoning 通道的字面<think>example</think>文本,在两个构建上行为完全一致——单次请求、无多余重试、字面标签原样保留。
对话记录(JSONL)断言
- PR 构建泄漏场景会话:
grep验证会话 JSONL 中泄漏 payload(</think>、“presubmit report”、损坏正文)出现次数为零;恢复后的答案存在。 - PR 构建字面标签会话:记录中的两处
</think>是被正确保留的字面文本。 - main 构建的会话把泄漏 payload 一并写入了 chat JSONL,确认 before 行为会延伸到持久化层。
观察(不阻塞合并)
- attempt 被丢弃后,TUI 每次重试会多保留一行折叠的
∴ Thought for 0s(见 after 截图)。没有任何泄漏内容被显示或记录——纯外观问题,甚至可作为重试痕迹。 - 2 秒起始重试延迟使最坏情况(预算耗尽)约需 20 秒,与现有 invalid-stream 行为一致,可接受。
结论:Reviewer Test Plan 的全部五项主张均与实际运行行为吻合。从运行时验证角度 LGTM。
|
@qwen-code /triage |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
|
| Commit | PR | E2E Result |
|---|---|---|
ae9e7782d4 |
#6756 | ✅ 0 failures |
43d34f739d |
#6754 (this PR) | ❌ ~14 files failed (run cancelled) |
92b47a4e01 |
#6764 | ❌ ~14 files failed |
417d30584d |
#6772 | ❌ ~14 files failed |
Affected tests include permission-control, tool-control, sdk-mcp-server, subagents, multi-turn, edit, file-system, write_file, utf-bom-encoding, cron-tools, notebook-edit, tool-search, and more — all of which depend on tool calls being correctly emitted through the OpenAI→Gemini conversion pipeline.
Typical failure pattern: expected [] to include 'read_file' or expected toolCalls to contain 'mcp__sdk-math__sdk_multiply' — tool calls are simply never emitted.
Suggested Fix
Consider reverting this PR and re-approaching the malformed response retry logic with less aggressive detection heuristics. The shouldDropMalformedAttempt path in particular needs review — suppressing finish_reason causes all downstream tool call emission to be silently skipped.
⚠️ E2E 回归:本 PR 导致 main 分支大面积 E2E 测试失败
本 PR 合入(43d34f739d)后,E2E 测试从 0 失败 变为 ~14 个文件失败 / 31 个用例失败,Linux 和 macOS 均受影响。
根因
converter.ts 中新增的流式响应检测逻辑过于激进:
hasVisibleThinkingTagLeakSignature()— 可能把合法响应误判为 thinking 标签泄漏,触发InvalidStreamError丢弃整个响应。hasNamelessToolCall()—shouldDropMalformedAttempt路径将finish_reason设为undefined,导致工具调用完全不被发射,parts被清空。pendingUntrustedResponseParts缓冲 — 如果误判为不可信,有效的工具调用 parts 永远不会被释放。
建议
建议回滚本 PR,然后以更保守的检测策略重新实现畸形响应重试逻辑。特别是 shouldDropMalformedAttempt 路径需要重点审查——抑制 finish_reason 会导致所有下游工具调用发射被静默跳过。
…ction (QwenLM#6794) * Revert "Revert "fix(core): retry malformed streamed responses (QwenLM#6754)" (QwenLM#6783)" This reverts commit ff7d48a. * fix(core): narrow nameless tool-call detection to stop false-positive drops hasNamelessToolCall() flagged any buffer slot lacking a name, including phantom slots (no id, no argument content) created by trailing structural deltas. That tripped shouldDropMalformedAttempt on legitimate tool-call responses, clearing parts and suppressing finish_reason to undefined, which surfaced as 'Model stream ended without a finish reason' and broke E2E suites depending on tool-call emission. Only treat a nameless tool call as malformed when the provider actually started a real call (assigned id or streamed argument content). Genuine QwenLM#6666 nameless attempts (id or args but no name) still drop and retry; phantom slots are skipped by getCompletedToolCalls as before. * fix(core): preserve thinking tag stream boundaries * fix(core): address malformed stream review feedback * fix(core): tighten malformed stream retry signals * fix(core): address remaining stream review feedback * test(core): cover stream review edge cases * fix(core): address stream review regressions * refactor(core): remove duplicate thinking tag state * fix(core): close reverse-order stream validation gaps * fix(core): preserve cross-channel tag evidence * test(core): cover split cross-channel tags * fix(core): close remaining stream routing gaps * fix(core): handle late tool call names * fix(core): preserve active tool call routes * test(core): cover malformed stream edge cases * fix(core): preserve streaming tool call routes * refactor(core): simplify malformed stream validation * docs(core): clarify invalid stream retry budget * fix(core): preserve valid streamed metadata boundaries * fix(core): deduplicate streamed tool metadata * fix(core): preserve streamed protocol line boundaries



What this PR does
This PR retries malformed streamed responses before corrupted protocol text can be accepted as the assistant's final answer.
It covers two concrete failure shapes from #6666 and the follow-up traces:
reasoning_content/ thought), but visible content also contains raw<think>/</think>tag fragments. That means the reasoning and visible-content channels were mixed, so the attempt is discarded and retried instead of displaying those tags to the user.Both malformed response shapes use the existing invalid-stream retry budget: up to four retries after the initial call, for five total attempts. Protocol-tag leaks retain a distinct telemetry classification while using the same retry budget.
Literal
<think>...</think>text is still preserved and streamed immediately when there is no structured reasoning signal and no malformed nameless tool call. Provider-specific tagged-thinking parsing remains opt-in through provider response parsing options.Why it's needed
The original production case was not just "Qwen3 content may contain a complete
<think>...</think>block." The stronger evidence is that the model/provider stream mixed channels: it returned a separate reasoning/thought channel, then left raw thinking-tag fragments and reasoning text inside visible content. One recorded case also included a tool call with an ID and arguments but no function name, which the old converter silently dropped before accepting the remaining corrupted visible text as successful output.The safe boundary is therefore protocol-level, not model-name-level: when structured reasoning has appeared, a leading raw opening thinking tag or an unmatched raw closing thinking tag in visible content is treated as a malformed stream and retried. This avoids globally stripping user-visible literal tags and avoids enabling tagged-thinking parsing for every Qwen3-compatible endpoint.
Reviewer Test Plan
How to verify
<think>example</think>content without structured reasoning and without a malformed tool call. Confirm it remains visible.</think>/<think>fragments. Confirm the bad attempt is retried and the leaked tags never appear in the TUI or chat record.<think>...</think>content in the same chunk. Confirm the bad attempt is retried and the leaked tags never appear in the TUI or chat record.Evidence (Before & After)
Before: malformed streams could be accepted as final output, so raw
<think>/</think>fragments reached the UI and chat record.After: malformed attempts with nameless tool calls or structured-reasoning tag leaks produce no bad TUI/chat-record output, drop their content and tool calls, withhold the finish signal, and are retried through the existing invalid-stream retry path.
Focused core tests pass 476/476. A five-attempt integration test confirmed that four malformed attempts are discarded and only the clean fifth response reaches history. Repository
build,typecheck,lint, andbundlepass locally. Fresh interactive tmux validation covered healthy structured reasoning, literal tag content, the exact recorded cross-channel leak, and a terminal incomplete tag prefix. Strict assertions confirmed that leaked tag payloads did not appear in tmux output or chat JSONL.Tested on
Environment (optional)
Local OpenAI-compatible SSE mock, built CLI bundle, and interactive tmux.
Risk & Scope
<think>...</think>text without structured reasoning streams immediately and remains unchanged.Linked Issues
Resolves #6666
Related to #6751
中文说明
What this PR does
这个 PR 会在损坏的 streamed response 被当作最终成功答案之前,丢弃并重试这次 malformed attempt。
它覆盖 #6666 和后续 trace 里看到的两个具体形态:
reasoning_content/ thought),但 visible content 里又出现 raw<think>/</think>tag 残片。这说明 reasoning channel 和 visible content channel 混在了一起,所以这次 attempt 会被丢弃并重试,而不是把这些 tag 显示给用户。这两类 malformed response 都复用现有 invalid-stream retry budget:初次调用后最多重试 4 次,总计最多 5 次 attempt。protocol-tag leak 使用独立的 telemetry 分类,但 retry 次数保持一致。
如果没有 structured reasoning 信号,也没有 malformed nameless tool call,literal
<think>...</think>会立即流式透传并原样保留。provider-specific tagged-thinking parser 仍然只通过 provider response parsing options 显式启用。Why it's needed
原始生产 case 并不只是“Qwen3 content 里可能输出一段完整
<think>...</think>”。更强的证据是 model/provider stream 混了通道:它已经返回了独立 reasoning/thought channel,却又把 raw thinking-tag 残片和 reasoning 文本留在 visible content 里。另一个记录到的 case 还同时包含一个有 ID 和 arguments、但没有 function name 的 tool call;旧 converter 静默丢弃这个 tool call 后,把剩余损坏正文当作成功输出。所以安全边界应该是协议层信号,而不是模型名:当 structured reasoning 已经出现时,visible content 开头的 raw opening thinking tag 或没有对应 opening 的 raw closing thinking tag 会被视为 malformed stream 并触发 retry。这样不会全局剥离用户可见的 literal tag,也不会给所有 Qwen3-compatible endpoint 强行启用 tagged-thinking parser。
Reviewer Test Plan
How to verify
<think>example</think>,但没有 structured reasoning、也没有 malformed tool call,确认正文原样显示。</think>/<think>残片的 visible content,确认 bad attempt 被重试,泄漏 tag 不进入 TUI 或 chat record。<think>...</think>content,确认 bad attempt 被重试,泄漏 tag 不进入 TUI 或 chat record。Evidence (Before & After)
Before:malformed stream 可能被当作最终成功输出,所以 raw
<think>/</think>残片会进入 UI 和 chat record。After:包含 nameless tool call 或 structured-reasoning tag leak 的 malformed attempt 不会输出坏内容到 TUI/chat record,会丢弃本次 attempt 的 content 和 tool calls,withhold finish signal,并通过现有 invalid-stream retry path 重试。
core 相关 focused tests 476/476 通过。五次 attempt 的 integration test 确认前四次 malformed response 都被丢弃,只有第五次 clean response 进入 history。仓库
build、typecheck、lint、bundle本地通过。最新交互式 tmux 验证覆盖 healthy structured reasoning、literal tag content、实际记录的 cross-channel leak,以及 stream 结束时不完整的 tag prefix。严格断言确认泄漏 tag payload 没有进入 tmux output 或 chat JSONL。Tested on
Environment (optional)
本地 OpenAI-compatible SSE mock、构建后的 CLI bundle 和交互式 tmux。
Risk & Scope
<think>...</think>text 会立即流式透传并保持不变。Linked Issues
Resolves #6666
Related to #6751