fix(core): filter thought parts from Stop hook last_assistant_message - #6009
Conversation
getLastModelMessageText() concatenated all text parts from the last model message without filtering out thought parts (part.thought === true). This caused the Stop hook's last_assistant_message to include the model's internal reasoning text, making structured output validation difficult. Added !part.thought to the filter condition in both GeminiChat and the client fallback implementation, consistent with existing thought filtering in copyCommand, partUtils, and stripThoughtPartsFromContent. Added two unit tests: - filters out thought parts from mixed model messages - returns undefined when all text parts are thoughts Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
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. |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
|
Thanks for the PR! Template looks good ✓ On direction: this is a clear-cut bug fix — thinking-model reasoning text leaking into Stop hook On approach: minimal and focused — 2 lines changed across 2 implementation files (adding Moving on to code review. 🔍 中文说明感谢贡献! 模板完整 ✓ 方向:这是一个明确的 bug 修复——thinking 模型的推理文本泄漏到 Stop hook 的 方案:极简且聚焦——2 个实现文件各改 1 行(在现有 filter 中添加 进入代码审查 🔍 — Qwen Code · qwen3.7-max |
Code ReviewIndependent proposal (before reading the diff): I'd add Diff comparison: the PR's approach matches my proposal exactly. Both implementation sites patched, edge cases covered (mixed parts, all-thoughts → undefined). Clean, correct. No blockers. No reuse gaps — the inline filter is simpler than calling Unit TestsAll 194 tests in
Build succeeds with 0 errors. Before (main branch, no fix)After (this PR)中文说明代码审查独立方案(读 diff 前):在 Diff 对比: PR 方案与我的独立方案完全一致。两处实现均已修复,边界情况已覆盖(混合 parts、纯 thought → undefined)。正确、无阻塞问题。 无复用缺口——内联 filter 比调用 单测
修复前(main 分支)Stop hook 收到 "The user is asking a simple arithmetic question." 等内部推理文本,多次 hook 调用均检测到 thought 污染。 修复后(本 PR)Stop hook 仅收到可见回复,无推理文本泄漏。THOUGHT_CONTAMINATION=clean。 — Qwen Code · qwen3.7-max |
|
This is a clean, well-scoped bug fix that does exactly what it says. The independent proposal I formed before reading the diff matched the PR's approach perfectly — add What I like: the change is surgical — no new abstractions, no refactoring of existing thought-filtering utilities, just the minimum fix in the two affected call sites. The PR body is thorough with clear motivation, reproduction steps, and risk analysis. No reservations. Ships it. ✅ 中文说明这是一个干净、范围合理的 bug 修复,完全符合描述。读 diff 前形成的独立方案与 PR 方案完全一致——在两处 filter 条件中添加 优点:改动精准——无新抽象,无对已有 thought 过滤工具的重构,仅在两个受影响的位置做最小修复。PR 描述详尽,动机清晰,复现步骤完整,风险分析到位。 无顾虑。通过。✅ — Qwen Code · qwen3.7-max |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
| @@ -2776,7 +2759,8 @@ export class GeminiChat { | |||
| const text = | |||
| message.parts | |||
| ?.filter( | |||
| (part): part is { text: string } => typeof part.text === 'string', | |||
| (part): part is { text: string } => | |||
| typeof part.text === 'string' && !part.thought, | |||
There was a problem hiding this comment.
[Suggestion] The inline filter here is a subset of the existing isPlainTextPart type guard (line 613) which delegates to isValidNonThoughtTextPart (line 859). The shared predicate already checks !part.thought && !part.thoughtSignature plus defensive guards for functionCall/functionResponse/inlineData/fileData — matching the codebase's stated principle: "Technically, the model should never generate parts that have text and any of these but we don't trust them so check anyways."
Reusing the file-local getPlainTextFromParts() helper (line 625) would cover the thoughtSignature gap, eliminate the duplication with client.ts, and centralize future filter changes:
| typeof part.text === 'string' && !part.thought, | |
| const text = getPlainTextFromParts(message.parts); |
For the client.ts fallback, export getPlainTextFromParts (or isValidNonThoughtTextPart) and import it there. Also consider adding a test case for thoughtSignature-only parts to protect the broader predicate.
— qwen3.7-max via Qwen Code /review
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
No review findings. Downgraded from Approve to Comment: CI failing (triage).
— qwen3.7-max via Qwen Code /review
What this PR does
Filters out thought parts (
part.thought === true) from the text returned bygetLastModelMessageText(). This method is used to populate the Stop hook'slast_assistant_messagefield. Previously it concatenated all text parts from the last model message — including the model's internal reasoning — which caused thinking-model reasoning text to leak into the hook input, breaking structured output validation.Why it's needed
When using thinking models (qwen3.7-max, deepseek-r1, etc.), the model response contains two kinds of text parts: reasoning/thought parts (
thought: true) and the visible response. The Stop hook'slast_assistant_messageis expected to contain only the visible response, butgetLastModelMessageText()included both. This caused:The fix adds
!part.thoughtto the filter condition in both theGeminiChat.getLastModelMessageText()implementation and theGeminiClientfallback, consistent with the existing thought-filtering pattern used elsewhere.Reviewer Test Plan
How to verify
last_assistant_messageto a file:~/.qwen/settings.jsonunderhooks.Stop:{ "hooks": [{ "type": "command", "command": "bash ~/.qwen/scripts/dump-stop-hook.sh", "timeout": 5000 }] }npm run dev -- --prompt "证明根号3是无理数,用数学归纳法"/tmp/stop-hook-last-message.txt— should contain only the visible response, no English reasoning text.Evidence (Before & After)
Before:

After:

Tested on
Environment
npm run devwith qwen3.7-max (thinking model via DashScope OpenAI-compatible API).Risk & Scope
roundTextaccumulation). StopFailure hook does not passlast_assistant_message.Linked Issues
N/A
中文说明
这个 PR 做了什么
在
getLastModelMessageText()中过滤掉 thought parts(part.thought === true)。该方法用于填充 Stop hook 的last_assistant_message字段。此前它将最后一条模型消息的所有 text parts 拼接在一起——包括模型的内部推理文本——导致 thinking 模型的推理内容泄漏到 hook 输入中,破坏了结构化输出校验。为什么需要
使用 thinking 模型(qwen3.7-max、deepseek-r1 等)时,模型响应包含两类 text parts:推理/思考(
thought: true)和可见回复。Stop hook 的last_assistant_message预期只包含可见回复,但getLastModelMessageText()将两者都包含了。这导致:修复方案:在
GeminiChat.getLastModelMessageText()和GeminiClientfallback 两处实现的 filter 条件中加入!part.thought,与代码库中已有的 thought 过滤模式保持一致。验证方法
last_assistant_messagedump 到文件并检测推理文本模式风险与范围
roundText累加时过滤 thought)last_assistant_message