fix(core): render a thought part's reasoning instead of the boolean flag - #7866
Conversation
`Part.thought` is a boolean flag and the reasoning lives in `part.text`.
`partToString` declared it as `thought?: string` in a local cast and
interpolated the flag, so every thought part rendered verbose as the
literal `[Thought: true]` and the reasoning was dropped.
The local cast was the only thing making that compile -- the SDK types it
as `thought?: boolean`, `createOpenAIReasoningThoughtPart` builds
`{ text, thought: true }`, and every other consumer reads the text and
tests the flag for truthiness. Drop the cast and follow the same shape.
Testing `!== undefined` also caught a part carrying `thought: false`,
which is an ordinary part: it rendered as `[Thought: false]` rather than
as its own text. Use truthiness, matching the rest of the codebase.
Two existing tests asserted the old output and are corrected. Both were
built from shapes the SDK never emits: `{ thought: 'thinking' }`, which
only type-checks through `as unknown as Part`, and a bare `{ thought:
true }` expected to print its own flag.
|
✅ Qwen Triage finished — CI landed green on ✅ Qwen Triage 已完成 —— |
|
Thanks for the PR! Template looks good ✓ Problem: observed bug, clearly demonstrated. Direction: aligned. This is a rendering-correctness fix in a core utility; well within scope. CHANGELOG has no direct reference, but the area (thought/reasoning rendering) is actively used. Size: core paths touched ( Approach: the scope feels right — one function fix, tests updated to use realistic Moving on to code review. 🔍 中文说明感谢贡献! 模板完整 ✓ 问题:已观测到的 bug,有清晰论证。 方向:对齐。这是核心工具函数中的渲染正确性修复,完全在范围内。CHANGELOG 无直接引用,但该领域(思维/推理渲染)正在被积极使用。 规模:触及核心路径( 方案:范围合理——一个函数修复,测试更新为使用真实的 进入代码审查 🔍 — Qwen Code · qwen3.8-max-preview Reviewed at |
Code ReviewIndependent proposal: given "verbose thought rendering shows the boolean flag instead of the reasoning text", I would: (1) remove the Comparison with the diff: the PR does exactly this, and adds one thing I would have considered optional but is genuinely valuable — a test that goes through the repo's own No critical blockers or convention violations found. The code comment explaining why Downstream consumers verified: Test EvidenceThis is an unattended CI run — PR code is not executed locally. Evidence below is from the PR's own CI checks on the reviewed commit, fetched via the API. Final CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行(取最新一次运行),省略 skipped,失败项排在最前。 The ubuntu unit/integration suite is still running. macOS and Windows tests were skipped (likely gated on ubuntu passing first). No failures so far. Not verified: full unit suite result (pending), TUI behavior (not user-visible per the PR — the verbose string is used in prompt expansion and token counting, not rendered in the terminal). 中文说明代码审查独立方案: 针对"verbose 模式下 thought 渲染显示布尔标志而非推理文本"的问题,我的方案是:(1) 移除局部类型断言中的 与 diff 的对比: PR 完全采用了上述方案,并额外增加了一个我认为可选但确实有价值的测试——通过仓库自身的 未发现关键阻塞项或规范违反。解释 已验证下游使用方: 测试证据本次为无人值守 CI 运行——不在本地执行 PR 代码。以下证据来自 PR 自身在被审查提交上的 CI 检查,通过 API 获取。 ubuntu 单元/集成测试套件仍在运行中。macOS 和 Windows 测试被跳过(可能以 ubuntu 先通过为前置条件)。目前无失败。未验证:完整单元测试套件结果(待定)、TUI 行为(据 PR 说明非用户可见——verbose 字符串用于 prompt 展开和 token 计数,不在终端渲染)。 — Qwen Code · qwen3.8-max-preview Reviewed at |
|
Confidence: 5/5 — clean, minimal bug fix with a real problem, a correct solution, and thorough tests; would merge without hesitation. The bug is unambiguous: a local cast overrode the SDK's The PR is disciplined about scope. If I had to maintain this in six months, I'd thank the author: the code comment explains the non-obvious why (the old cast made the wrong mental model compile), the tests document the expected shapes, and the diff is small enough to revert trivially if anything surfaces. Approval deferred until CI lands green on 中文说明置信度:5/5 —— 干净、最小化的 bug 修复,问题真实、方案正确、测试充分;毫不犹豫即可合并。 bug 明确无误:局部类型断言将 SDK 的 PR 在范围控制上很自律。 如果六个月后需要维护这段代码,我会感谢作者:代码注释解释了不直观的 为什么(旧断言让错误的心智模型得以编译),测试记录了预期形状,diff 足够小,万一出现问题可以轻松回退。 审批推迟至 CI 在 — Qwen Code · qwen3.8-max-preview Reviewed at |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship — CI landed green after the review. ✅
wenshao
left a comment
There was a problem hiding this comment.
No issues found. LGTM! ✅
中文说明
未发现问题。LGTM!✅
— qwen3.8-max-preview via Qwen Code /review
Maintainer verification — real local build + runtime A/BI built this locally and exercised it against real code paths rather than re-reading the diff. Verdict: the fix is correct, the tests are bound to it, and the blast radius is exactly the thought-flagged verbose branch — nothing else moves. Safe to merge. Setup — PR head 1 · The tests are bound to the fix
Two things worth noting. First, the PR body says the revert produces 3 assertion failures — it actually produces 6; the two 2 · Wire-level: real SSE stream → real generator → real renderThe most useful question for a rendering fix is whether the shape it assumes is the shape that actually arrives. So: a fake OpenAI-compatible server streams The part that comes off the wire is 3 · Cross-package: the CLI's
|
| shape | merge-base | this PR | |
|---|---|---|---|
{ thought: false, inlineData } |
[Thought: false] |
<image/png> |
misrouting fixed for non-text parts too |
{ thought: false } (no text) |
[Thought: false] |
"" |
falls through, consistent with the non-verbose path |
{ thought: true, functionCall } |
[Thought: true] |
[Thought] |
equally lossy either way — no regression |
5 · Gates
| check | result |
|---|---|
| PR's own tests | 60/60 pass |
full packages/core suite |
17,931 passed, 13 skipped, 2 failed — both unrelated (see below) |
packages/cli userPromptExpansionHook.test.ts |
12/12 pass |
tsc --noEmit -p packages/core |
clean |
eslint --max-warnings 0 on the 3 changed files |
clean |
The 2 failures are in src/services/session-writer-lease.test.ts ("classifies an unreadable owned lock as unavailable" and the retried-cleanup case). They chmod 0o000 a lock file and expect a read to reject — which cannot happen when the suite runs as uid 0, as it does on my box. That file has no reference to partUtils and is untouched by this PR; CI runs as non-root and is green on 65b063a.
6 · Blast radius today — worth stating plainly
There are exactly two verbose call sites, and I traced both:
partListUnionToString(core/geminiRequest.ts) — zero in-repo callers; it exists only as public API (packages/core/src/index.tsre-exports both it andpartToString).serializeUserPromptExpansionPrompt(cli/src/utils/userPromptExpansionHook.ts) — feeds theUserPromptExpansionhook. I walked everysubmit_promptproducer that reaches it (custom TOML commands via the prompt-pipeline processors, skills, bundled skills, MCP prompts — which stringify to JSON text — and the plan/goal/dream/remember/init/statusline/model commands). All of them carry user-prompt material: text and@-file / media parts. None emits a thought part.
So no in-repo path feeds a thought part into a verbose serializer today, and this PR changes nothing a user currently sees — which matches the PR body's own "N/A — not user-visible in the TUI", and is why there is no TUI before/after to screenshot. What it does fix is the exported API: any SDK consumer or extension that renders model-response parts verbose is silently losing reasoning right now, and any future in-repo caller would inherit that. Cheap, well-tested, and it removes a local cast that made the wrong mental model type-check. Worth taking on those grounds.
7 · One correction for the follow-up PR
The deferral of loggingContentGenerator.toPart is the right call — text is preserved there, only a useless [Thought: true] marker is appended — but the stated reason is off. That marker does not reach the CountToken API (countTokens just delegates to the wrapped generator); toPart is called from toContents → logApiRequest, so it lands in the ApiRequestEvent telemetry payload. The in-code comment above it says "CountToken API compatibility" and is stale, which is presumably where the description came from. Worth correcting when that follow-up lands.
中文说明
维护者验证 —— 本地真实构建 + 运行时 A/B
我在本地做了真实构建,并让改动跑在真实代码路径上,而不是只读 diff。结论:修复正确,测试确实绑定在这个修复上,影响面精确地局限于 thought 标志位的 verbose 分支,其余全部不变。 可以合并。
环境 —— PR head 65b063a45d15953c7db0b03682ba7ab016d5f8d3,与 main 的 merge-base 为 3209b89f,Linux,Node v22.22.2,vitest 3.2.4。下面每组 A/B 的 "before" 一侧都是对 packages/core 的第二次真实构建,其中仅把 src/utils/partUtils.ts 回退到 merge-base;测试、验证脚本以及其他所有文件在两次运行中逐字节相同。
1 · 测试确实绑定在修复上
npx vitest run --root packages/core src/utils/partUtils.test.ts src/core/geminiRequest.test.ts —— PR head 上 60/60 通过。仅回退 partUtils.ts、保留测试:6 个失败,54 个仍然通过。(截图见英文第 1 节)
有两点值得说明。其一,PR 描述称回退后产生 3 个断言失败,实际是 6 个——geminiRequest.test.ts 的两个用例同样失败,所以 A/B 比描述中更有力。其二,非 verbose 的保护用例位于仍然通过的那 54 个之中,这正是它被写出来的目的:测试锁定了新行为,又没有过度断言。
2 · 链路级:真实 SSE 流 → 真实 generator → 真实渲染
对一个渲染修复来说,最值得回答的问题是:它所假设的数据形态,是否就是实际到达的形态。因此:一个仿真的 OpenAI 兼容服务通过 HTTP 流式下发 reasoning_content,进入真实构建的 OpenAIContentGenerator,其产出的 Part 对象再交给构建后的 partListUnionToString。整个 harness 中没有任何一个 Part 是手工拼装的。
从链路上下来的 part 是 { text: '…', thought: true },typeof thought === 'boolean',且 isOpenAIReasoningThoughtPart() 对其返回 true——它就是仓库自身 createOpenAIReasoningThoughtPart 的产物,经由流式路径抵达,而非测试夹具。在 merge-base 构建下,推理文本在渲染结果中消失;在本 PR 下则得以保留。这就坐实了前提:旧的局部断言 thought?: string 与真实流过的值相矛盾。
3 · 跨包:CLI 的 UserPromptExpansion 序列化器
packages/cli/src/utils/userPromptExpansionHook.ts 原封不动地被引入,只切换它所解析到的构建后 core。对照项(纯文本 prompt、图片 part)在两次运行中逐字节一致。
4 · 完整差分——到底改变了什么
partToString 会分支处理的每一种 Part 形态,在两种模式下、分别跑过两个真实构建:
40 组 形态/模式 组合:9 组改变,31 组逐字节相同。非 verbose 行改变数为 0;所有发生改变的行都同时满足 verbose: true 且带 thought 标志。 这精确地界定了回归面——仓库内两个非 verbose 调用方(client.ts、memory/extract.ts)不可能受影响。
有三行是 PR 的表格未列出的,且均为中性或更优:
| 形态 | merge-base | 本 PR | |
|---|---|---|---|
{ thought: false, inlineData } |
[Thought: false] |
<image/png> |
非文本 part 的误路由同样被修正 |
{ thought: false }(无 text) |
[Thought: false] |
"" |
落到后续分支,与非 verbose 路径一致 |
{ thought: true, functionCall } |
[Thought: true] |
[Thought] |
两侧同样有损——不构成回归 |
5 · 各项检查
| 检查项 | 结果 |
|---|---|
| PR 自带测试 | 60/60 通过 |
packages/core 全量测试 |
17,931 通过,13 跳过,2 失败——均无关(见下) |
packages/cli 的 userPromptExpansionHook.test.ts |
12/12 通过 |
tsc --noEmit -p packages/core |
干净 |
对 3 个改动文件执行 eslint --max-warnings 0 |
干净 |
那 2 个失败位于 src/services/session-writer-lease.test.ts("classifies an unreadable owned lock as unavailable" 以及重试清理那一例)。它们把锁文件 chmod 0o000 后期望读取被拒绝——而当测试以 uid 0(我这台机器上正是如此)运行时,这不可能发生。该文件完全没有引用 partUtils,本 PR 也未触及;CI 以非 root 运行,在 65b063a 上是绿的。
6 · 当前的实际影响面——有必要说清楚
verbose 调用点恰好有两个,我都做了追踪:
partListUnionToString(core/geminiRequest.ts)——仓库内零调用方;它仅作为公开 API 存在(packages/core/src/index.ts同时导出了它和partToString)。serializeUserPromptExpansionPrompt(cli/src/utils/userPromptExpansionHook.ts)——为UserPromptExpansion钩子提供输入。我走查了所有能到达它的submit_prompt产出方(经 prompt-pipeline 处理器的自定义 TOML 命令、skills、内置 skills、MCP prompts——其结果被序列化为 JSON 文本——以及 plan/goal/dream/remember/init/statusline/model 等命令)。它们承载的都是用户 prompt 素材:文本与@文件/媒体 part,没有任何一个会产出 thought part。
因此当前仓库内没有任何路径会把 thought part 送入 verbose 序列化器,本 PR 也不会改变用户当下所见——这与 PR 描述自己写的 "N/A——在 TUI 中不可见" 一致,也正是没有 TUI 前后对比截图可拍的原因。它真正修好的是导出的 API:任何以 verbose 方式渲染模型响应 part 的 SDK 使用方或扩展,现在都在静默丢失推理内容,未来仓库内新增的调用方也会继承这个问题。成本低、测试充分,并且移除了一个让错误心智模型得以通过类型检查的局部断言。基于这些理由,值得合入。
7 · 给后续 PR 的一处更正
推迟处理 loggingContentGenerator.toPart 是正确的判断——那里文本并未丢失,只是多追加了一个无用的 [Thought: true] 标记——但陈述的理由有偏差。该标记并不会进入 CountToken API(countTokens 只是转发给被包装的 generator);toPart 是由 toContents → logApiRequest 调用的,因此它落在 ApiRequestEvent 的遥测负载中。其上方的代码注释写着 "CountToken API compatibility",已经过时,PR 描述大概率源自该注释。后续 PR 落地时值得一并更正。
|
Recovered after restart; restarting task |
|
Released in v0.21.1. |




What this PR does
Makes the verbose rendering of a thought part show the reasoning it carries, rather than the literal string
true.Why it's needed
Part.thoughtis a boolean flag; the reasoning itself lives inpart.text.partToStringdeclared it asthought?: stringin a local cast and then interpolated the flag directly, so every thought part rendered verbose as[Thought: true]and the reasoning was thrown away.The local cast was the only thing letting that compile. The SDK declares
thought?: boolean(@google/genaigenai.d.ts),createOpenAIReasoningThoughtPartinthoughtUtils.tsbuilds exactly{ text, thought: true }, and every other consumer in the repo reads the text and tests the flag for truthiness —part.thought === true,if (!part.thought),filter((part) => part.text && !part.thought).partToStringwas the one place treating the flag as the payload.Testing
!== undefinedinstead of truthiness had a second consequence: a part carryingthought: falseis an ordinary part, but it took the thought branch and rendered as[Thought: false]instead of its own text.Both live verbose callers are affected —
partListUnionToStringingeminiRequest.tsand the prompt-expansion hook inpackages/cli.Reviewer Test Plan
How to verify
{ thought: true, text: 'thinking' }[Thought: true][Thought: thinking]{ thought: true }[Thought: true][Thought]{ thought: false, text: 'ordinary' }[Thought: false]ordinarycreateOpenAIReasoningThoughtPart('step one')[Thought: true][Thought: step one]{ thought: true, text: 'thinking' }, non-verbosethinkingthinking(unchanged)The fourth row goes through the repo's own constructor rather than a hand-built object, so it pins the fix to the shape the code actually produces. The last row is a guard against over-correcting and passes both before and after.
Evidence (Before & After)
N/A — not user-visible in the TUI; the rendering table above is the before/after.
Tested on
Environment (optional)
Unit tests only.
Risk & Scope
[Thought: true]would need updating — I searched and found only the two tests corrected here.loggingContentGenerator.toParthas the same interpolation and appends[Thought: true]to the text it sends to the CountToken API. It is a different function with different semantics — there the text is not lost, only a useless marker is added — so it belongs in its own PR rather than being folded in here.Linked Issues
None.
中文说明
本 PR 的作用
让 thought part 的 verbose 渲染显示其承载的推理内容,而不是字面量字符串
true。为什么需要
Part.thought是一个布尔标志,推理内容本身位于part.text中。partToString在局部类型断言里把它声明为thought?: string,然后直接对该标志做字符串插值,于是每个 thought part 在 verbose 模式下都渲染成[Thought: true],推理内容被丢弃。正是这个局部断言让上述代码得以通过类型检查。SDK 将其声明为
thought?: boolean(@google/genai的genai.d.ts),thoughtUtils.ts中的createOpenAIReasoningThoughtPart构造的正是{ text, thought: true },而仓库中其他所有使用方都是读取 text、并对该标志做真值判断——part.thought === true、if (!part.thought)、filter((part) => part.text && !part.thought)。partToString是唯一把标志当作内容本身的地方。使用
!== undefined而非真值判断还带来第二个后果:带有thought: false的 part 属于普通 part,却进入了 thought 分支,渲染成[Thought: false]而不是它自己的文本。两个实际使用 verbose 的调用方都受影响——
geminiRequest.ts中的partListUnionToString,以及packages/cli中的 prompt 展开钩子。审阅者测试计划
如何验证
{ thought: true, text: 'thinking' }[Thought: true][Thought: thinking]{ thought: true }[Thought: true][Thought]{ thought: false, text: 'ordinary' }[Thought: false]ordinarycreateOpenAIReasoningThoughtPart('step one')[Thought: true][Thought: step one]{ thought: true, text: 'thinking' },非 verbosethinkingthinking(不变)第四行走的是仓库自身的构造函数,而非手工拼装的对象,因此能把修复锚定在代码实际产出的形态上。最后一行是防过度修正的保护用例,修复前后均通过。
证据(修复前后对比)
N/A——在 TUI 中不可见;上方的渲染对照表即为修复前后对比。
测试环境
运行环境(可选)
仅单元测试。
风险与影响范围
[Thought: true]做匹配的代码都需要相应调整——我已检索过,只有本 PR 中修正的那两个测试。loggingContentGenerator.toPart存在同样的插值问题,会把[Thought: true]追加到发往 CountToken API 的文本中。那是一个语义不同的函数——在那里文本并未丢失,只是多了一个无用的标记——因此应当另开 PR,而不是并入本 PR。关联 Issue
无。