fix(ui): collapse duplicate in-flight tool_group rendered from history + pending - #9421
fix(ui): collapse duplicate in-flight tool_group rendered from history + pending#9421qwen-code-dev-bot wants to merge 5 commits into
Conversation
…y + pending (#9420) The latest tool call rendered twice: the same in-flight tool_group was present both in committed history and the live pending list (or twice within pending), so MainContent's combined virtualized list showed it twice until the next tool call finalized. Collapse consecutive tool_group rows with an identical callId signature, keeping the later (live pending) copy so status keeps updating. Adds a regression test asserting a duplicated tool_group renders once. Fixes #9420
github-actions
Bot
added
the
review/self-reported
|
✅ Qwen Triage finished — CI landed green on ✅ Qwen Triage 已完成 —— |
|
Thanks for the PR! Template looks good ✓ Problem: observed bug with solid evidence — linked issue #9420 carries concrete reproduction steps and a root-cause analysis of the history/pending double-include. Self-reported by the same author, but the repro is specific (any sequential tool calls in the default TUI) and the mechanism is plausible. Direction: aligned — a visible per-tool-call rendering defect on the default TUI path. No direct match in the reference CHANGELOG, but duplicate-render fixes are a recurring defect class there too; the area is clearly relevant. Size: no core paths touched ( Approach: scope feels right. One question worth sitting with: #9420 suggested either filtering the pending copy that is already in history, or deferring the history commit until the group finalizes — this PR instead de-dupes at the render seam, keeping the later (live) item. That is a defensible, minimal choice, but it leaves the underlying double-include in place; the code review should confirm the signature match is exact enough that a genuinely distinct batch can never be collapsed by accident. Risk: no elevated risk signals. Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ 问题：已观测到的 bug，证据充分 —— 关联 issue #9420 提供了具体复现步骤和 history/pending 双重包含的根因分析。虽为同一作者自报，但复现明确（默认 TUI 下任意连续工具调用），机制可信。 方向：对齐 —— 这是默认 TUI 路径上每次工具调用都可见的渲染缺陷。参考 CHANGELOG 无直接对应条目，但重复渲染修复在那里也是常见缺陷类别；该领域显然相关。 规模：未触及核心路径（packages/cli/src/ui/components）；42 行生产代码（MainContent.tsx）+ 32 行测试。小而聚焦。 方案：范围合理。一个值得考虑的问题：#9420 建议过滤已在 history 中的 pending 副本，或推迟到 group 结束再写入 history —— 本 PR 选择在渲染接缝处去重，保留后者（实时项）。这是可辩护的最小改动，但底层的双重包含依然存在；代码审查需确认签名匹配足够精确，确保真正独立的批次不会被误折叠。 风险：无升级风险信号。 进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
Code reviewMy independent take on this bug before reading the diff: de-duplicate at the seam where Details verified while reading:
No blockers; style, typing ( Test evidence (PR's own CI via API — no PR code executed in this review)The main Linux unit-test job is still running at the time of writing; the macOS/Windows test jobs were skipped for this run. No failures so far: Final CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行（取最新一次运行），省略 skipped，失败项排在最前。 The finalize job will update this table once CI settles. The author reports 24 unit tests (including the new seam test) passing locally — that is the author's claim; the CI run above is what substantiates it. Sandboxed verification would settle what CI cannot: 中文说明代码审查读 diff 之前我对这个 bug 的独立想法是：在 MainContent 拼接 history 与 pending 列表的接缝处、按 tool_group 的 callId 集合去重，保留持续更新的 pending 副本。PR 的实现与此完全一致——在已有 useMemo 中做一次 O(n) 遍历，不改动 streaming/history 生命周期。相比 issue 中提到的另外两个方案（过滤已在 history 中的 pending 项、或推迟到结束再写入 history），这是最保守的选项，对渲染层症状来说范围也合适。 阅读中核实的细节：
无阻塞问题；风格、类型（unknown 收窄、无 any）、同目录测试约定均干净。PR 诚实地说明只在渲染接缝去重，未触及更深层的双重包含根因（同一 group 为何同时进 history 与 pending）——根因值得后续开 issue 跟进，但就用户可见症状而言，这个修复的体量是合适的。 测试证据（PR 自身 CI，经 API 获取——本审查未执行任何 PR 代码）撰写时 Linux 主单测 job 仍在运行；本次 macOS/Windows 测试 job 被跳过。目前无失败。上方表格由 finalize 任务在 CI 结束后更新。作者自述本地 24 个单测（含新增接缝测试）通过——此为作者声明，以上 CI 才是验证依据。 沙箱验证可补足 CI 无法覆盖的部分： — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 4/5 — clean, minimal, tested fix that matches what I would have written; the two nits below are follow-ups, not defects. Stepping back: this is the fix I'd have proposed for #9420 — I independently landed on seam de-duplication before reading the diff, and the PR executes it with the right conservatism: one pass at the render seam, no streaming/history lifecycle changes, the live pending copy wins so status updates keep flowing, and a unit test that pins the seam. Every change in the diff is necessary; there are no drive-by edits. The problem is real and observed (concrete repro in the linked issue), the mechanism was confirmed by reading the code, and the scope matches the reported symptom. Two non-blocking nits for the record:
Verdict: approve. Approval is deferred until CI lands green on 中文说明置信度：4/5 —— 干净、最小、有测试的修复，与我独立给出的方案一致；以下两点是后续事项，不是缺陷。 整体来看：这正是我为 #9420 会提出的修复——读 diff 之前我独立想到的就是接缝去重，而 PR 的执行足够保守：只在渲染接缝做一次遍历，不改 streaming/history 生命周期，保留实时 pending 副本以维持状态更新，并有单测钉住接缝行为。diff 中每一处改动都是必要的，没有夹带无关修改。问题真实且已被观测（关联 issue 有具体复现），机制经代码阅读确认，范围与所报症状匹配。 两条不阻塞的记录项：
结论：通过。批准推迟到该提交的 CI 全绿——撰写时 Qwen Code CI 单测仍在运行。若该提交的所有检查通过，finalize 任务将代发钉在该提交上的批准；若有检查变红或 head 移动，则会标记而不批准。 — Qwen Code · qwen3.8-max Reviewed at |
This comment has been minimized.
This comment has been minimized.
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
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. ✅
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Not explored to full depth (tool budget reached): "agent 6c": run packages/cli vitest for MainContent.test.tsx to confirm the new test and adjacent tests pass (worktree needs npm ci + full npm run build first; not ….
Test Plan (not a blocker): 24 tests pass — this review observed 21756 passed.
中文说明
未探索到全部深度（达到工具调用预算）："agent 6c"：run packages/cli vitest for MainContent.test.tsx to confirm the new test and adjacent tests pass (worktree needs npm ci + full npm run build first; not …。
Test Plan（非阻断）：24 tests pass — this review observed 21756 passed。
— qwen3.8-max via Qwen Code /review (v0.21.13)
| const deduped: VpItem[] = []; | ||
| for (const item of combined) { | ||
| const prev = deduped[deduped.length - 1]; |
There was a problem hiding this comment.
[Critical] The collapse only fires for adjacent duplicate tool_group rows, but in the real lifecycle behind issue 9420 the two copies are separated by whatever pending items the continuation turn streams, so the reported double-render persists in the common case. On batch completion, onComplete commits the group via addItem and then awaits handleCompletedTools → submitQuery(ToolResult), which consumes the entire continuation stream; the core scheduler clears the display list only afterwards (in the finally after onAllToolCallsComplete). Since pendingToolCallGroupDisplay is always the LAST entry of the pending array, any continuation-streamed thought/text lands between the committed copy (history tail) and the stale pending copy — Failure scenario: any sequential-tool task where the model emits thought/text between calls or a final answer after the last batch (the issue's own repro: a couple of read_file calls) → the combined list is [..., batchN(history), thought/text(pending), batchN(pending)]; the copies are not adjacent, the loop never collapses them, and the latest tool row still renders twice until the next batch arrives. The new test only replays the adjacent shape, so its green run does not demonstrate the issue is fixed.
Witness (probe):
history=[tool_group(callId 'dup-call')]
pending=[gemini_thought, tool_group(callId 'dup-call')]
deduped dataIds: [-9007199254740991, 1, -1, -2]
Both copies (history id=1, pending id=-2) render, separated by the thought (id=-1); the collapse did not fire.
Suggested fix: dedup on identity regardless of adjacency (drop the earlier committed copy when the same signature appears anywhere later in the combined list), or fix the lifecycle seam — clear/replace the scheduler display state at commit time (in onComplete, right after addItem, before await handleCompletedTools), so the stale pending copy never overlaps committed history. Then add a test with a pending item between the two copies.
中文说明
[Critical] 折叠仅对相邻的重复 tool_group 行生效，但在 issue 9420 背后的真实生命周期中，两份副本之间会被续接回合流式产生的 pending 项隔开，因此所报告的双重渲染在常见场景下依然存在。批次完成时，onComplete 通过 addItem 将该组提交到 history，随后 await handleCompletedTools → submitQuery(ToolResult) 消费整个续接流；核心调度器直到其后（onAllToolCallsComplete 之后的 finally 中）才清空展示列表。由于 pendingToolCallGroupDisplay 始终是 pending 数组的最后一项，任何续接流式产生的思考/文本都会落在已提交副本（history 末尾）与过期的 pending 副本之间 —— 失败场景：任何模型在工具调用之间输出思考/文本、或在最后一批之后输出最终回答的顺序工具任务（issue 本身的复现：几次 read_file 调用）→ 合并列表为 [..., batchN(history), thought/text(pending), batchN(pending)]；两份副本不相邻，循环永远不会折叠它们，最新的工具行仍然会渲染两次，直到下一批到来。新增测试只复现了相邻形态，因此它的通过并不能证明问题已修复。
证据（探针）：
history=[tool_group(callId 'dup-call')]
pending=[gemini_thought, tool_group(callId 'dup-call')]
deduped dataIds: [-9007199254740991, 1, -1, -2]
两份副本（history id=1，pending id=-2）均被渲染，中间隔着思考项（id=-1）；折叠未触发。
建议修复：按身份去重而不限于相邻（当相同签名在合并列表后续任意位置出现时丢弃较早的已提交副本），或修复生命周期接缝——在提交时清空/替换调度器展示状态（onComplete 中 addItem 之后、await handleCompletedTools 之前），使过期的 pending 副本不再与已提交 history 重叠。然后补充一个两份副本之间夹有 pending 项的测试。
— qwen3.8-max via Qwen Code /review (v0.21.13)
| // Collapse consecutive duplicate tool_group rows (#9420): the same | ||
| // in-flight tool batch can appear both in committed history and the live | ||
| // pending list (or twice within pending), rendering the latest tool call | ||
| // twice until the next call finalizes. Keep the LATER item so the live | ||
| // pending copy (which keeps updating) wins over the static history copy. |
There was a problem hiding this comment.
[Suggestion] The collapse lives only in the virtual-viewport (allVirtualItems) path; the legacy <Static> path renders the same duplicated state uncollapsed, so the double row persists when ui.useTerminalBuffer: false — Concrete cost: the duplication is produced by render-path-agnostic machinery (useGeminiStream commits via addItem while pendingToolCallGroupDisplay still reflects the batch; it has zero references to useTerminalBuffer, and AppContainer feeds one shared pendingHistoryItems memo to both paths). With the supported setting ui.useTerminalBuffer: false (also screen-reader mode and non-interactive terminals via shouldUseVirtualViewport), the committed copy renders in <Static> and the live copy in the pending region at the same time — the exact double row this PR fixes for the virtual viewport. Rated Suggestion rather than Critical because useTerminalBuffer defaults to true and issue 9420 is explicitly VP-mode-scoped, so the fix matches the report's scope.
Suggested fix: apply the same toolGroupSignature-based collapse to the legacy path, or lift the fix to the producer (useGeminiStream) by suppressing pendingToolCallGroupDisplay once its batch has been committed via addItem — covering both paths at once.
中文说明
[Suggestion] 折叠仅存在于虚拟视口（allVirtualItems）路径；旧版 <Static> 路径对同样的重复状态不做折叠，因此在 ui.useTerminalBuffer: false 时双重行依然存在 —— 具体代价：重复状态由与渲染路径无关的机制产生（useGeminiStream 在 pendingToolCallGroupDisplay 仍反映该批次时通过 addItem 提交；它完全不引用 useTerminalBuffer，且 AppContainer 向两条路径提供同一个共享的 pendingHistoryItems memo）。在受支持的设置 ui.useTerminalBuffer: false 下（以及屏幕阅读器模式、通过 shouldUseVirtualViewport 的非交互终端），已提交副本渲染在 <Static> 中、实时副本同时渲染在下方 pending 区域 —— 正是本 PR 为虚拟视口修复的双重行。评为 Suggestion 而非 Critical，因为 useTerminalBuffer 默认为 true，且 issue 9420 明确限定在 VP 模式，修复与报告范围一致。
建议修复：对旧版路径应用同样基于 toolGroupSignature 的折叠，或将修复上移到生产者（useGeminiStream）：在批次经 addItem 提交后抑制 pendingToolCallGroupDisplay —— 一次覆盖两条路径。
— qwen3.8-max via Qwen Code /review (v0.21.13)
There was a problem hiding this comment.
Deferred to the follow-up queue. The mechanism is verified: the duplication is produced by producer-side machinery shared by both render paths (useGeminiStream commits the batch via addItem while pendingToolCallGroupDisplay still reflects it, and AppContainer feeds one shared pendingHistoryItems memo to both paths), so ui.useTerminalBuffer: false / screen-reader mode shows the same double row. The fix belongs in the producer (suppress or replace the pending display at commit time): Ink's <Static> is append-only, so a render-side filter on the legacy path cannot unrender a committed copy that was already flushed. That change is outside this PR's footprint — issue #9420 is explicitly virtual-viewport-scoped and this fix matches the report's scope, as the finding itself notes.
中文说明
已推迟到后续队列。机制已核实：重复由两条渲染路径共享的生产者侧机制产生（useGeminiStream 通过 addItem 提交批次时 pendingToolCallGroupDisplay 仍反映该批次，且 AppContainer 向两条路径提供同一个共享的 pendingHistoryItems memo），因此 ui.useTerminalBuffer: false / 屏幕阅读器模式下会出现同样的双重行。修复应位于生产者侧（在提交时抑制或替换 pending 展示）：Ink 的 <Static> 是只追加的，旧版路径上的渲染侧过滤器无法撤掉已经刷出的已提交副本。该改动超出本 PR 的足迹 —— issue #9420 明确限定在虚拟视口，本修复与报告范围一致，该发现本身也指出了这一点。
| return (tools as Array<{ callId?: unknown }>) | ||
| .map((tool) => String(tool?.callId ?? '')) | ||
| .sort() | ||
| .join(','); |
There was a problem hiding this comment.
[Suggestion] Sorted-callId signatures are not a universally unique batch identity: the accepted-speculation render path in AppContainer.tsx synthesizes spec-${name}-${i} callIds where i restarts at 0 per tool-only model message, so two legitimately different adjacent batches can share a signature and the collapse deletes a real committed transcript row — Concrete cost: with experimental ui.enableSpeculation: true (default false), accepting a speculated suggestion whose run performs two sequential tool-only turns of the same tool (e.g. read_file(a.ts) then read_file(b.ts)) commits two adjacent tool_groups both signed spec-read_file-0; the loop collapses them, keeping only the second, and the first tool call's row is permanently missing from the display (before this diff both rendered). The comma-join is additionally ambiguous (['a,b'] vs ['a','b']-shaped ids). Rated Suggestion rather than Critical because speculation is experimental and default-off; this would be Critical if it were default-on.
Suggested fix: scope the collapse to the shape it was designed for — only merge when one of the pair is a pending copy (negative id) — and/or make synthetic spec callIds globally unique (e.g. spec-${messageIndex}-${name}-${i}); optionally make the join delimiter-safe (e.g. JSON.stringify each id).
中文说明
[Suggestion] 排序后的 callId 签名并非普遍唯一的批次身份：AppContainer.tsx 中「采纳 speculation」的渲染路径会合成 spec-${name}-${i} 形式的 callId，其中 i 在每个纯工具 model message 处从 0 重新开始，因此两个合法不同的相邻批次可能共享同一签名，折叠便会从渲染的会话记录中删除真实的一行 —— 具体代价：在实验性 ui.enableSpeculation: true（默认 false）下，采纳一条推测建议且其运行对同一工具连续执行两个纯工具回合（如先 read_file(a.ts) 再 read_file(b.ts)）时，会提交两个相邻的 tool_group，签名均为 spec-read_file-0；循环将二者折叠、仅保留后者，第一个工具调用的行从此在显示中永久消失（本 diff 之前两行都会渲染）。此外逗号连接存在歧义（形如 ['a,b'] 与 ['a','b'] 的 id）。评为 Suggestion 而非 Critical，因为 speculation 是实验性且默认关闭的；若默认开启则为 Critical。
建议修复：将折叠限定为其设计的形态 —— 仅当两者之一是 pending 副本（负 id）时才合并 —— 并/或让合成的 spec callId 全局唯一（如 spec-${messageIndex}-${name}-${i}）；可选地让连接符无歧义（如对每个 id 做 JSON.stringify）。
— qwen3.8-max via Qwen Code /review (v0.21.13)
| const frame = lastFrame() ?? ''; | ||
| expect(frame).toContain('VP_ITEM:-1'); | ||
| expect(frame).not.toContain('VP_ITEM:1'); |
There was a problem hiding this comment.
[Suggestion] The substring assertions don't pin the surviving item count — Concrete cost: a one-line mutation deleting continue; in the dedup loop makes the duplicate item both replace prev AND get pushed (two pending copies render — the exact double row this PR fixes), and this test still passes (probe-confirmed: the mutant survives).
| const frame = lastFrame() ?? ''; | |
| expect(frame).toContain('VP_ITEM:-1'); | |
| expect(frame).not.toContain('VP_ITEM:1'); | |
| const frame = lastFrame() ?? ''; | |
| expect(frame.match(/VP_ITEM:-1/g)).toHaveLength(1); | |
| expect(frame).not.toContain('VP_ITEM:1'); |
Alternatively assert on the list data via the existing spy: expect(scrollableListPropsSpy.mock.calls.at(-1)?.[0].data.map((i) => i.id)).toEqual([Number.MIN_SAFE_INTEGER, -1]).
中文说明
[Suggestion] 子串断言无法固定存活条目的数量 —— 具体代价：在去重循环中删除 continue; 的单行变异会使重复项既替换 prev 又被 push（渲染出两个 pending 副本 —— 正是本 PR 要修复的双重行），而本测试仍然通过（已用探针确认：该变异存活）。
或者通过现有 spy 断言列表数据：expect(scrollableListPropsSpy.mock.calls.at(-1)?.[0].data.map((i) => i.id)).toEqual([Number.MIN_SAFE_INTEGER, -1])。
— qwen3.8-max via Qwen Code /review (v0.21.13)
| const sig = toolGroupSignature(item); | ||
| if (prev && sig !== null && sig === toolGroupSignature(prev)) { |
There was a problem hiding this comment.
[Suggestion] No VP-path test renders two adjacent tool_groups with different signatures, so an over-collapse regression survives the entire suite — Concrete cost: probe-confirmed: relaxing the condition to collapse ANY adjacent tool_group pair leaves all 24 tests green, yet a probe with two adjacent distinct groups (call-A id=1, call-B id=2) drops group 1 under the mutant (dataIds: [-9007199254740991, 2] vs [-9007199254740991, 1, 2] under correct code). Such a regression merges two consecutive distinct batches into one row, silently dropping the completed batch from the transcript. The only pre-existing adjacent-group guard lives in the useTerminalBuffer=false Static-path describe block and never exercises this code.
Suggested fix: add a VP-path test with two adjacent tool_groups carrying different callIds (one in history, one in pendingHistoryItems) asserting both ids render.
中文说明
[Suggestion] 没有任何 VP 路径测试渲染两个签名不同的相邻 tool_group，因此「过度折叠」回归可以在整套测试中存活 —— 具体代价：已用探针确认：把条件放宽为折叠任意相邻 tool_group 对后，全部 24 个测试依然为绿；而对两个相邻的不同批次（call-A id=1、call-B id=2）的探针显示，变异体下 group 1 被丢弃（dataIds: [-9007199254740991, 2]，正确代码为 [-9007199254740991, 1, 2]）。这类回归会把两个连续的不同批次合并为一行，悄无声息地从记录中丢掉已完成的批次。唯一既有的相邻批次守卫位于 useTerminalBuffer=false 的 Static 路径 describe 块中，根本不会执行到这段代码。
建议修复：新增一个 VP 路径测试，构造两个携带不同 callId 的相邻 tool_group（一个在 history、一个在 pendingHistoryItems），断言两个 id 都被渲染。
— qwen3.8-max via Qwen Code /review (v0.21.13)
| expect(lastFrame()).toMatch(/VP_ITEM:1[\s\S]*VP_ITEM:2/); | ||
| }); | ||
|
|
||
| it('collapses a tool_group duplicated across history and pending (#9420)', () => { |
There was a problem hiding this comment.
[Suggestion] The implementation comment claims the loop also handles the same batch appearing "twice within pending", but no test covers that placement — Concrete cost: probe-confirmed: a mutant adding && prev.id > 0 (collapse only when prev is a history item; pending items have negative ids) leaves all 24 tests green; under it, two adjacent pending copies of the same in-flight batch both render, re-introducing the double row in the pending-only case the comment names.
Suggested fix: add a case with pendingHistoryItems: [dupToolGroup, dupToolGroup] asserting only the later copy renders.
中文说明
[Suggestion] 实现注释声称该循环也处理同一批次「在 pending 内出现两次」的情况，但没有任何测试覆盖该形态 —— 具体代价：已用探针确认：加入 && prev.id > 0（仅当 prev 是 history 项时才折叠；pending 项 id 为负）的变异体让全部 24 个测试保持为绿；在该变异体下，同一在途批次的两个相邻 pending 副本都会被渲染，使注释所述「仅 pending」场景下的双重行复现。
建议修复：新增一个 pendingHistoryItems: [dupToolGroup, dupToolGroup] 的用例，断言仅渲染较后的副本。
— qwen3.8-max via Qwen Code /review (v0.21.13)
| function toolGroupSignature(item: unknown): string | null { | ||
| const it = item as { type?: unknown; tools?: unknown } | null; |
There was a problem hiding this comment.
[Suggestion] Convention drift from this file's sibling helpers: virtualKeyExtractor and virtualIsStaticItem take the typed VpItem union and narrow on the type discriminant, while this new helper takes unknown and double-casts — Concrete cost: the casts silence the compiler on exactly the accesses (.type, .tools, .callId) that define the dedup identity; if tools or callId is ever renamed or reshaped, this function still compiles and silently degrades — a missing tools field makes every signature '' (via the Array.isArray fallback), collapsing ALL adjacent tool_groups into one row with no type error to catch it.
function toolGroupSignature(item: VpItem): string | null {
if (item.type !== 'tool_group') return null;
return item.tools
.map((tool) => tool.callId)
.sort()
.join(',');
}(The fix spans the whole function body, so no one-click suggestion block.)
中文说明
[Suggestion] 与本文件同类辅助函数的约定不一致：virtualKeyExtractor 与 virtualIsStaticItem 接收类型化的 VpItem 联合并按 type 判别式收窄，而新辅助函数却接收 unknown 并双重强转 —— 具体代价：强转恰好让编译器在定义去重身份的访问（.type、.tools、.callId）上失声；一旦 tools 或 callId 被改名或重构，本函数仍能编译并静默退化 —— 缺失 tools 字段会使每个签名变为 ''（经 Array.isArray 兜底），把所有相邻 tool_group 折叠为一行，且没有任何类型错误可以捕获。
（修复覆盖整个函数体，故不使用一键 suggestion 代码块。）
— qwen3.8-max via Qwen Code /review (v0.21.13)
| if (prev && sig !== null && sig === toolGroupSignature(prev)) { | ||
| deduped[deduped.length - 1] = item; | ||
| continue; |
There was a problem hiding this comment.
[Suggestion] The collapse is completely silent — when a row is dropped (correctly or by mistake) there is no debug-log breadcrumb, and this file has no debug logger at all — Concrete cost: a user reporting "a tool call row disappeared from my transcript" gets zero signal under --debug that a collapse ever fired; the investigator must reconstruct the exact history/pending interleaving and callId sets by hand. The neighboring VirtualizedList deliberately logs its analogous silent edge cases with debugLogger.debug(...) (lines 583-587) precisely so --debug users can self-diagnose.
Suggested fix: add a debugLogger.debug(...) call inside the collapse branch logging the signature and the ids being replaced.
中文说明
[Suggestion] 折叠完全静默 —— 无论正确还是误删，丢弃一行时没有任何调试日志痕迹，且本文件根本没有调试日志器 —— 具体代价：当用户报告「我的会话记录里少了一行工具调用」时，即使开启 --debug 也看不到任何折叠曾触发的信号；排查者只能手工重建 history/pending 的精确交错与 callId 集合。相邻的 VirtualizedList 特意用 debugLogger.debug(...)（583-587 行）记录其类似的静默边界情况，正是为了让 --debug 用户可以自查。
建议修复：在折叠分支内加一处 debugLogger.debug(...)，记录签名与被替换的 id。
— qwen3.8-max via Qwen Code /review (v0.21.13)
There was a problem hiding this comment.
Declined. MainContent.tsx has no debug logger today; adding an import plus a log call for this intended, deterministic collapse is not worth the diff growth (AGENTS.md Simplicity First). The behavior is now pinned by five focused regression tests, so an over- or under-collapse is caught at build time rather than diagnosed after the fact. If a field report of a missing tool row still needs a runtime breadcrumb, the log can ride along with that diagnosis.
中文说明
拒绝。MainContent.tsx 目前没有调试日志器；为这个有意为之且确定性的折叠新增一个 import 加一处日志调用不值得增加 diff（AGENTS.md 简洁优先）。该行为现在由五个聚焦的回归测试固定，过度折叠或折叠不足会在构建期被捕获，而不是事后诊断。如果现场仍有「工具行消失」的报告需要运行时线索，日志可以随那次诊断一起加入。
doudouOUC
left a comment
doudouOUC
left a comment
There was a problem hiding this comment.
Not reviewed: the entire diff — no agent reported covering it; nobody read it.
Not reviewed: the linked-issue fidelity pass, the line-by-line correctness pass, the security pass, the reuse and duplication pass, the altitude and abstraction pass, the consistency and clarity pass, the performance pass, the test-coverage pass, the open-ended audit (attacker mindset), the open-ended audit (oncall mindset), the open-ended audit (maintainer mindset), the removed-behavior audit, the cross-file consistency pass, the build-and-test check — its prompt was built, but no agent on record was launched with it.
Not reviewed: verification and reverse audit — both prompts were built, but no agent was launched with either — the posted findings cannot be counted as verified, and the pass that hunts what the rest of the review missed cannot be certified.
中文说明
未审查：整个 diff——没有 agent 报告覆盖过这部分，也没有人读过它。
未审查：关联 issue 一致性检查、逐行正确性检查、安全检查、复用与重复代码检查、修复层次与抽象合理性检查、一致性与可读性检查、性能检查、测试覆盖检查、开放式审计（攻击者视角）、开放式审计（值班排障视角）、开放式审计（后续维护者视角）、删除行为审计、跨文件一致性检查、构建与测试验证——它的 prompt 已构建，但没有任何 agent 有记录用它启动过。
未审查：验证与反向审计——两份 prompt 都已构建，但都没有 agent 用它们启动——发布的发现不能算作已验证，搜寻评审遗漏问题的工序也无法作证。
— deepseek-v4 via Qwen Code /review (v0.21.13)
| // twice until the next call finalizes. Keep the LATER item so the live | ||
| // pending copy (which keeps updating) wins over the static history copy. | ||
| const deduped: VpItem[] = []; | ||
| for (const item of combined) { |
There was a problem hiding this comment.
[Suggestion] Suggestion: The dedup only checks the immediately preceding item. If a non-tool_group item sits between two identical tool_groups, they are not collapsed. Consider scanning the entire deduped array or documenting the race window.
| */ | ||
| function toolGroupSignature(item: unknown): string | null { | ||
| const it = item as { type?: unknown; tools?: unknown } | null; | ||
| if (!it || it.type !== 'tool_group') return null; |
There was a problem hiding this comment.
[Suggestion] Suggestion: The signature is built from callId only. Two different tool batches with the same callIds would produce the same signature. Consider including tool name for defense-in-depth.
There was a problem hiding this comment.
Declined. After this round's fix, a collapse only fires when a live pending copy shares the signature, and callIds are unique per tool call — identical sorted-callId sets mean the same logical batch, so the tool name adds no discriminating power. The one real collision case (two committed speculation batches whose synthetic spec-${name}-${i} ids repeat) involves identical tool names as well, and those pairs are no longer collapsed at all: committed rows without a live pending counterpart are preserved unconditionally, pinned by the new "keeps committed tool_groups sharing a signature when no pending copy is live" test.
中文说明
拒绝。本轮修复后，折叠仅在存在共享同一签名的实时 pending 副本时触发，而 callId 对每个工具调用是唯一的 —— 排序后的 callId 集合相同即表示同一逻辑批次，因此工具名不提供任何额外区分力。唯一真实的冲突场景（两个已提交的 speculation 批次，其合成的 spec-${name}-${i} id 重复）中工具名同样相同，且这类配对现在完全不再被折叠：没有实时 pending 副本的已提交行无条件保留，并由新测试「无实时 pending 副本时保留同签名的已提交 tool_group」固定。
|
|
||
| const dupToolGroup = { | ||
| type: 'tool_group' as const, | ||
| tools: [ |
There was a problem hiding this comment.
[Suggestion] Suggestion: The code comment mentions "or twice within pending" as a scenario, but the test only covers history+pending duplication. Add a test for the pending-only duplicate case.
|
|
||
| /** | ||
| * Stable identity of a tool_group for consecutive-duplicate collapsing | ||
| * (#9420): the sorted callIds of its tool calls. Two adjacent tool_group rows |
There was a problem hiding this comment.
[Suggestion] Suggestion: When tools is an empty array, the signature is an empty string, which would collapse two consecutive empty tool_groups. Consider returning null when tools.length === 0.
There was a problem hiding this comment.
Declined. After this round's fix, a collapse requires a live pending counterpart with the same signature, and an empty-tools pending tool_group cannot exist: pendingToolCallGroupDisplay is undefined — not an empty group — when toolCalls.length === 0 (useGeminiStream.ts, the memo guarding it). With no pending empty group there is nothing for an empty committed group to collapse against, and committed-only pairs are never merged regardless of signature. The tools.length === 0 guard would defend against a state the producer cannot reach.
中文说明
拒绝。本轮修复后，折叠需要存在同签名的实时 pending 副本，而空 tools 的 pending tool_group 不可能存在：当 toolCalls.length === 0 时 pendingToolCallGroupDisplay 为 undefined —— 而不是空组（见 useGeminiStream.ts 中守护它的 memo）。不存在 pending 空组，空的已提交组就没有可折叠的对象，而纯已提交配对无论签名如何都不会被合并。tools.length === 0 守卫防御的是生产者无法到达的状态。
|
|
||
| const dupToolGroup = { | ||
| type: 'tool_group' as const, | ||
| tools: [ |
There was a problem hiding this comment.
[Suggestion] Suggestion: Add a test for the non-consecutive duplicates boundary (a non-tool_group item between two identical tool_groups should preserve both).
|
|
||
| const dupToolGroup = { | ||
| type: 'tool_group' as const, | ||
| tools: [ |
There was a problem hiding this comment.
[Suggestion] Suggestion: Add a test with multiple tool_groups with different signatures to verify all survive the dedup loop.
|
🕐 Review received — an automatic review of the current head is still running, so this round is held until it lands (a push now would cancel it and discard its work, #8888). Your feedback stays queued for the next eligible round. 中文说明🕐 已收到评审 —— 当前 head 上仍有一轮自动 review 在运行，本轮暂缓（现在推送会取消该 review 并丢弃其工作，#8888）。反馈保持排队，等待下一次可运行的轮次处理。 |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
🤖 Addressed the latest review feedback (round 1/10). What changed, and what I pushed back on: · 已处理最新评审反馈（第 1/10 轮）。改动内容与我反驳保留之处如下： Autofix round summary — PR #9421 (issue #9420)Outcome: made changes — commit The Critical, reproduced first
Feedback dispositions
Changes
Verification
中文说明Autofix 本轮总结 — PR #9421（issue #9420）结果： 已修改 — 在 Critical 的复现
反馈处理
变更内容
验证
Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 10 this bot stops and leaves the PR for a human. · 有空请复审；第 10 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
Test Plan (not a blocker): 24 tests pass — this review observed 21757 passed.
中文说明
已审查——无阻断问题。 建议见行内评论。
Test Plan（非阻断）：24 tests pass — this review observed 21757 passed。
— qwen3.8-max via Qwen Code /review (v0.21.13)
| const deduped: VpItem[] = []; | ||
| for (const item of combined) { | ||
| const sig = toolGroupSignature(item); |
There was a problem hiding this comment.
[Suggestion] The #9420 dedup recomputes a callId signature (map + sort + join, fresh allocations) for every committed tool_group in history on every memo recompute — i.e. on every streaming tick, since each 60 ms throttled flush gives pendingHistoryItems a fresh array identity — even when no pending tool_group exists and neither dedup branch can fire (both provably require a non-empty livePendingSigs).
Failure scenario: in a long session with thousands of committed tool_groups, the second pass walks the entire combined list and rebuilds signatures on every streamed chunk — including during pure text streaming — adding history-sized allocation churn per tick on a render hot path this file otherwise fights (see the issue 3899 comments above).
Witness (probe): rendering 100 committed tool_groups plus one pending content item and re-rendering with fresh pending identity, the current code performs 100 sort invocations per simulated chunk; with the early return below, 0 — row counts identical in both arms, and all 28 tests in this file pass with the change applied.
| const deduped: VpItem[] = []; | |
| for (const item of combined) { | |
| const sig = toolGroupSignature(item); | |
| if (livePendingSigs.size === 0) return combined; | |
| const deduped: VpItem[] = []; | |
| for (const item of combined) { | |
| const sig = toolGroupSignature(item); |
中文说明
[Suggestion] #9420 去重在每次 memo 重算时都会为 history 中每一个已提交的 tool_group 重新计算 callId 签名（map + sort + join，全新分配）——也就是每个流式 tick 都会发生（每 60ms 的节流刷新都会使 pendingHistoryItems 获得新的数组身份）——即使当前不存在任何 pending tool_group、两个去重分支都不可能触发（二者都以 livePendingSigs 非空为前提）。
失败场景：在拥有数千个已提交 tool_group 的长会话中，第二个循环会在每个流式块（包括纯文本流式阶段）遍历整个合并列表并重建签名，在这条本文件其他位置（见上方 issue 3899 的注释）一直刻意优化的渲染热路径上，每个 tick 增加与 history 规模成正比的分配开销。
证据（探针）：渲染 100 个已提交 tool_group 加 1 个 pending 内容项，并以新的 pending 身份重渲染，当前代码每个模拟块产生 100 次 sort 调用；采用下面的提前返回后为 0 —— 两种情况下行列表完全一致，且应用该修改后本文件全部 28 个测试通过。
— qwen3.8-max via Qwen Code /review (v0.21.13)
|
🤖 Addressed the latest review feedback (round 2/10). What changed, and what I pushed back on: · 已处理最新评审反馈（第 2/10 轮）。改动内容与我反驳保留之处如下： Autofix review round summary — PR #9421Feedback points and decisions[rv:4965136323] Review body — "Reviewed — no blockers. Suggestions are inline."Decision: no action needed. The review body carries no findings; the single suggestion is handled inline below. [rc:3807397565] Suggestion — skip the dedup pass when no pending
|
|
Qwen Code review did not complete successfully. The review pipeline failed before a review could be posted. A transient error is retried automatically; if you are seeing this, retry with |
|
🔀 Base updated: red check(s) [Test (ubuntu-latest, Node 22.x), review-pr] pass on current main — merged current main via update-branch; CI will re-run. 中文说明🔀 已更新 base：红色检查 [Test (ubuntu-latest, Node 22.x), review-pr] 在当前 main 上通过 —— 已通过 update-branch 合入当前 main，CI 将重新运行。 |
doudouOUC
left a comment
doudouOUC
left a comment
There was a problem hiding this comment.
Not reviewed: the executable-script lint — could not read the plan to check the gate.
Not reviewed: coverage — the plan could not be used (ENOENT: no such file or directory, open 'C:\Users\jinye.djy.qoderwork\workspace\mspqz3u5etjh72hs\qwen-code\Usersjinye.djy.qoderworkworkspacemspqz3u5etjh72hsqwen-code.qwen mpqwen-review-pr-9421-fetch.json'), so this run cannot show that any of the diff was read.
Not reviewed: verification — could not check that Step 4 and Step 5 ran (ENOENT: no such file or directory, open 'C:\Users\jinye.djy.qoderwork\workspace\mspqz3u5etjh72hs\qwen-code\Usersjinye.djy.qoderworkworkspacemspqz3u5etjh72hsqwen-code.qwen mpqwen-review-pr-9421-fetch.json').
— qwen3.8-max via Qwen Code /review (v0.21.13)
| if (sig !== null) { | ||
| if (item.id > 0 && livePendingSigs.has(sig)) continue; | ||
| // Same batch twice within the pending list: keep the later copy. | ||
| const prev = deduped[deduped.length - 1]; |
There was a problem hiding this comment.
[Suggestion] Pending-list dedup only checks adjacent items; non-adjacent duplicates within the pending list are not collapsed — Failure scenario: If two identical tool_group copies appear in pendingHistoryItems separated by a non-tool_group item, the second copy is not collapsed because the dedup only checks deduped[deduped.length - 1]. The combined list would render the same tool_group twice.
— qwen3.8-max via Qwen Code /review (v0.21.13)
中文说明
[Suggestion] pending 列表内的去重仅检查相邻项，非相邻的重复项不会被折叠 —— 失败场景：如果两个相同的 tool_group 副本之间被非 tool_group 项隔开，第二个副本不会被折叠。
— qwen3.8-max via Qwen Code /review (v0.21.13)
| return item.tools | ||
| .map((tool) => tool.callId) | ||
| .sort() | ||
| .join(','); |
There was a problem hiding this comment.
[Suggestion] toolGroupSignature uses join(',') which could produce ambiguous signatures if callId values contain commas — Failure scenario: If a callId contained a comma, two tool_groups with different callId sets could produce the same signature string. This would incorrectly collapse two distinct tool batches. Currently callIds are system-generated and never contain commas, so this is a theoretical concern.
— qwen3.8-max via Qwen Code /review (v0.21.13)
中文说明
[Suggestion] toolGroupSignature 使用 join(',') 在 callId 值包含逗号时可能产生歧义签名。
— qwen3.8-max via Qwen Code /review (v0.21.13)
| }); | ||
|
|
||
| // Shared fixtures for the #9420 collapse tests. | ||
| const toolGroupFixture = (callId: string) => ({ |
There was a problem hiding this comment.
[Suggestion] Multi-tool toolGroupSignature sorting is untested — the fixture always creates single-tool groups — Failure scenario: toolGroupFixture always creates a single-tool group, so the .sort() call in toolGroupSignature is never exercised in a meaningful way. If a future refactor accidentally removed the .sort(), two tool_groups with the same tools in different orders would not be collapsed.
read
— qwen3.8-max via Qwen Code /review (v0.21.13)
中文说明
[Suggestion] 多工具 toolGroupSignature 排序未被测试。
— qwen3.8-max via Qwen Code /review (v0.21.13)
|
🕐 Review received — an automatic review of the current head is still running, so this round is held until it lands (a push now would cancel it and discard its work, #8888). Your feedback stays queued for the next eligible round. 中文说明🕐 已收到评审 —— 当前 head 上仍有一轮自动 review 在运行，本轮暂缓（现在推送会取消该 review 并丢弃其工作，#8888）。反馈保持排队，等待下一次可运行的轮次处理。 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Deferred under the convergence posture (round 3, not a blocker) — recorded, not requested in this round:
packages/cli/src/ui/components/MainContent.tsx:339 — [probe] committed-collision guard in the dedup loop has zero loop-path test coverage (probe-confirmed surviving mutant)
中文说明
收敛姿态下延后（第 3 轮，非阻断）——已记录，本轮不要求修改：共 1 条（原文未翻译，列表见上方英文部分）。
— qwen3.8-max via Qwen Code /review (v0.21.13)
| for (const item of combined) { | ||
| const sig = toolGroupSignature(item); | ||
| if (sig !== null) { | ||
| if (item.id > 0 && livePendingSigs.has(sig)) continue; |
There was a problem hiding this comment.
[Critical] R3-1: callId-signature collision hides committed tool_group rows — this branch drops EVERY committed tool_group whose signature matches a live pending batch, but callIds are not globally unique, so a committed row from a different batch is silently hidden for the whole duplicate window. Reachable without provider misbehavior: full compaction replaces core history while the TUI display history stays intact (handleChatCompressionEvent only adds an info item), and normalizeModelToolCallIds seeds usedToolCallIds per-send from the current core history (geminiChat.ts ~4456) — so the deterministic generated ids (call_qwen_1, ...) are re-minted after compaction and collide with pre-compaction display rows. Wire ids from OpenAI-compatible endpoints also pass through unfiltered (converter.ts ~1250; handledProviderToolCallIdsRef is cleared at every top-level submit), so a naive or malicious endpoint reusing ids across turns collides too; join(',') is additionally ambiguous for callIds containing a comma. — Failure scenario: a committed row with callId X exists in display history; after compaction (or in the next turn) an unrelated batch streams with the same callId X as the live pending tool_group → this branch drops the committed row, and an executed tool batch vanishes from the rendered list until the pending copy clears — for a malicious provider, a way to conceal executed tool calls. Pre-diff both rows rendered (duplicated but visible); the diff turns the collision into silent disappearance (window-length — the row returns when the stale pending clears, but is invisible during the window).
Witness (probe against the real component):
committed [tool_group call_qwen_1] + pending [read_file call_qwen_1]
observed: [ -9007199254740991, 2, -1 ] <- committed id 1 dropped
flip (branch disabled): [ -9007199254740991, 1, 2, -1 ]
Suggested fix: match committed<->pending copies by a batch identifier the scheduler itself assigns (a batchId carried from the display-state copy into the committed item), or restrict this drop to only the LATEST committed tool_group matching a live pending signature (the just-committed stale copy is always the most recent such row); if a content signature must stay, make the join unambiguous (encodeURIComponent + \u0000 separator).
中文说明
[Critical] callId 签名冲突会隐藏已提交的 tool_group 行——该分支会丢弃所有签名与实时 pending 批次匹配的已提交 tool_group，但 callId 并非全局唯一，因此来自另一个批次的已提交行会在整个重复窗口内被静默隐藏。无需 provider 异常即可触发：完全压缩（full compaction）会替换核心历史，但 TUI 展示历史保持不变（handleChatCompressionEvent 仅添加一条 info 项），而 normalizeModelToolCallIds 每次发送时以当前核心历史为种子初始化 usedToolCallIds（geminiChat.ts ~4456）——因此确定性生成的 id（call_qwen_1 等）在压缩后会被重新铸造，与压缩前已存在的展示行冲突。OpenAI 兼容端点的 wire id 也会原样透传（converter.ts ~1250；handledProviderToolCallIdsRef 在每次顶层提交时清空），因此跨回合复用 id 的 naive/恶意端点同样会冲突；join(',') 对含逗号的 callId 还存在歧义。— 失败场景：展示历史中已存在 callId 为 X 的已提交行；压缩后（或下一回合）一个不相关的新批次以相同的 callId X 作为实时 pending tool_group 流式出现 → 该分支丢弃已提交行，一个已执行的工具批次从渲染列表中消失，直到 pending 副本被清除——对恶意 provider 而言，这是隐藏已执行工具调用的一种手段。diff 之前两行都会渲染（重复但可见）；该 diff 使冲突变为静默消失（窗口期长度——过期 pending 清除后该行会恢复，但在窗口期内不可见）。
证据（针对真实组件的探针）：
committed [tool_group call_qwen_1] + pending [read_file call_qwen_1]
observed: [ -9007199254740991, 2, -1 ] <- 已提交 id 1 被丢弃
flip（禁用该分支）: [ -9007199254740991, 1, 2, -1 ]
建议修复：用调度器自身分配的批次标识（将 batchId 从展示状态副本携带到已提交项）来匹配已提交与 pending 副本，或将该丢弃限制为与实时 pending 签名匹配的最新一个已提交 tool_group（刚提交的过期副本总是其中最新的一行）；若必须保留内容签名，则使拼接无歧义（encodeURIComponent + \u0000 分隔符）。
— qwen3.8-max via Qwen Code /review (v0.21.13)
| for (const item of combined) { | ||
| const sig = toolGroupSignature(item); |
There was a problem hiding this comment.
[Suggestion] R2-1: still stands (partially addressed by 4584d05dcf) — the early return covers only the no-pending-tool_group case; while any tool_group IS pending (the steady state of an agentic turn), every streaming tick still (a) walks the entire combined list in pass 1 checking item.id < 0, and (b) recomputes map+sort+join signatures (fresh allocations) for every committed tool_group in pass 2 — even though committed items are stable references whose signatures never change across ticks. — Concrete cost: a long agentic session with hundreds of committed tool_groups streaming a batch: pendingHistoryItems gets a fresh array identity on each throttled flush, so the memo recomputes every tick, the early return does not fire, and the session pays a full-history walk + G signature allocations per tick on the default render path this file is explicitly tuned for (see the #3899 work). (Magnitude is an inspection estimate, not a benchmark.)
Suggested fix: cache signatures keyed by item reference in a module-level WeakMap<VpItem, string | null> — committed items are passed by reference and never mutated (this fix itself relies on that), so their signatures compute once per session; pending items get fresh objects per tick, so the WeakMap naturally misses and entries are GC'd with them — and/or build livePendingSigs by iterating pendingHistoryItems directly instead of walking the combined list.
中文说明
[Suggestion] R2-1 仍然存在（4584d05dcf 仅部分解决）——提前返回只覆盖了没有 pending tool_group 的情况；只要存在 pending 的 tool_group（agent 回合的常态），每个流式 tick 依然会：(a) 在第一遍遍历整个合并列表检查 item.id < 0；(b) 在第二遍为每个已提交的 tool_group 重新计算 map+sort+join 签名（每次都是新分配）——尽管已提交项是稳定的引用，其签名跨 tick 从不变。— 具体代价：拥有数百个已提交 tool_group 的长会话在流式执行批次时：pendingHistoryItems 每次节流刷新都获得新的数组身份，memo 每 tick 重算，提前返回不会触发，会话在这个文件专门为长历史调优（见 #3899 工作）的默认渲染路径上，每 tick 付出整段历史遍历 + G 次签名分配的开销。（量级为检查估算，非基准测试。）
建议修复：用模块级 WeakMap<VpItem, string | null> 以项引用为键缓存签名——已提交项按引用传递且从不被修改（本修复本身就依赖这一点），其签名每会话只计算一次；pending 项每 tick 是新对象，WeakMap 自然未命中，条目随对象一起被 GC——和/或直接遍历 pendingHistoryItems 来构建 livePendingSigs，而不是遍历整个合并列表。
— qwen3.8-max via Qwen Code /review (v0.21.13)
|
🤖 Addressed the latest review feedback (round 3/10). What changed, and what I pushed back on: · 已处理最新评审反馈（第 3/10 轮）。改动内容与我反驳保留之处如下： Autofix review round — PR #9421Commit: Feedback points and dispositions[rc:3808214007] [Critical] R3-1: callId-signature collision hides committed [rc:3808149681] [Suggestion] pending-list dedup only checks adjacent items — RESOLVED. [rc:3808149689] [Suggestion] [rc:3808149693] [Suggestion] multi-tool signature sorting untested — RESOLVED. [rc:3808214023] [Suggestion] R2-1: per-tick signature recomputation / full-list walk — RESOLVED. [rv:4966112501] review body (CHANGES_REQUESTED) — no separate action required. [rv:4966034307] review body (COMMENTED) — no action. States "nothing further to act on here" (three unanchorable suggestions were dropped by that run). [ic:5333680838] issue-level fallback comment — no action. Informational notice that an earlier review run crashed; transient, retried automatically. Notes
VerificationCommands actually run (from the repository root unless noted; all exit 0 unless stated):
中文说明Autofix 审查轮次 — PR #9421提交： 反馈点及处置[rc:3808214007] [Critical] R3-1：callId 签名冲突导致已提交的 [rc:3808149681] [Suggestion] pending 列表去重仅检查相邻项 — 已解决。 [rc:3808149689] [Suggestion] [rc:3808149693] [Suggestion] 多工具签名排序未被测试 — 已解决。 [rc:3808214023] [Suggestion] R2-1：每 tick 重算签名 / 全列表遍历 — 已解决。 [rv:4966112501] 审查主体（CHANGES_REQUESTED）— 无需单独处理。 [rv:4966034307] 审查主体（COMMENTED）— 无需处理。 其声明 "nothing further to act on here"（该次运行丢弃了 3 条无法锚定的建议）。 [ic:5333680838] issue 级兜底评论 — 无需处理。 关于早前一次审查运行崩溃的告知性说明；属瞬时错误，会自动重试。 备注
验证实际执行的命令（未注明时均在仓库根目录执行；除特别说明外退出码均为 0）：
Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 10 this bot stops and leaves the PR for a human. · 有空请复审；第 10 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
中文说明
仅完成部分审查，审查缺口已披露。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
— qwen3.8-max via Qwen Code /review (v0.21.13)
| keptPendingBySig.set(sig, item); | ||
| } | ||
| } | ||
| for (const item of latestCommittedBySig.values()) dropped.add(item); |
There was a problem hiding this comment.
[Critical] R3-1: callId-signature collision still hides committed tool_group rows — the round-3 fix ("drop only the LATEST committed match for a live pending signature") holds only AFTER the live batch commits. Before onComplete commits it, the batch has no stale committed copy yet, so the unrelated earlier committed row whose callIds merely collide IS the latest (often only) match, and this line drops it — an executed tool batch disappears from the rendered transcript for its whole execution window. — Failure scenario: full core-history compaction replaces core history while the TUI display history stays intact (handleChatCompressionEvent only adds an info item); usedToolCallIds is re-seeded per-send from the post-compaction core history (geminiChat.ts:4456), so the next turn's first call re-mints call_qwen_1. From scheduling until commit of that new batch — its whole execution, minutes for a slow shell/build tool — the pre-compaction committed row holding call_qwen_1 is filtered out of the rendered list, silently reappearing only after the new batch commits. Provider wire-id reuse across turns reaches the same shape. Pre-diff both rows rendered (duplicated but visible); the diff turns the collision into silent disappearance, violating issue #9420's expectation that each tool call renders exactly once.
Witness (probe against the real component):
history=[tool_group(id 1, 'dup-call')], pending=[tool_group('dup-call')] (no committed stale copy)
expected [ -9007199254740991, 1, -1 ] — received [ -9007199254740991, -1 ] <- committed id 1 dropped
flip (drop loop disabled): [ -9007199254740991, 1, -1 ] passes
Suggested fix: give the collapse a real batch identity instead of inferring it from callIds — mint a batchId when the batch is scheduled, stamp it onto both the pending display group and the item committed by onComplete, and collapse only rows sharing that batchId (R3-1 option (a)); or fix the lifecycle seam (clear/replace the scheduler display state at commit time) so the two copies never coexist. If the signature heuristic stays, first add a regression test for the pre-commit shape (single committed colliding row + live pending batch, no stale copy) — it currently fails.
中文说明
[Critical] R3-1：callId 签名冲突仍会隐藏已提交的 tool_group 行 —— 第 3 轮修复（"只丢弃与实时 pending 签名匹配的最新一条已提交行"）仅在该批次提交之后成立。onComplete 提交之前，实时批次在 history 中还没有过期副本，此时 callId 恰好冲突的无关已提交行就是最新（往往也是唯一）的匹配项，被这一行丢弃 —— 一个已执行的工具批次在整个执行窗口内从渲染结果中消失。— 失败场景：完全压缩替换核心历史而 TUI 展示历史保持不变（handleChatCompressionEvent 仅追加 info 项）；usedToolCallIds 每次发送都以压缩后的核心历史重新播种（geminiChat.ts:4456），于是下一回合的第一个调用重新铸出 call_qwen_1。从新批次被调度到提交之间（整个执行期，慢速 shell/构建工具可达数分钟），压缩前那条持有 call_qwen_1 的已提交行被过滤出渲染列表，直到新批次提交后才悄悄恢复。provider 跨回合复用 wire id 会触发同样形态。diff 之前两行都会渲染（重复但可见）；本改动把冲突变成了静默消失，违背 issue #9420 "每个工具调用恰好渲染一次" 的预期。
证据（针对真实组件的探针）：
history=[tool_group(id 1, 'dup-call')], pending=[tool_group('dup-call')]（无已提交过期副本）
expected [ -9007199254740991, 1, -1 ] — received [ -9007199254740991, -1 ] <- 已提交 id 1 被丢弃
flip（禁用丢弃循环）: [ -9007199254740991, 1, -1 ] 通过
建议修复：给折叠一个真实的批次身份，而不是从 callId 推断 —— 调度批次时铸造 batchId，同时盖在 pending 展示组与 onComplete 提交的项上，仅折叠共享该 batchId 的行（R3-1 方案 (a)）；或修复生命周期接缝（提交时清空/替换调度器展示状态），使两份副本永不共存。若暂留签名启发式，先补一个提交前形态的回归测试（单个冲突已提交行 + 实时 pending 批次、无过期副本）—— 它当前会失败。
— qwen3.8-max via Qwen Code /review (v0.21.13)
| // history. Earlier committed rows sharing the signature are distinct | ||
| // executions whose callIds merely collide (ids re-minted after core | ||
| // history compaction, provider wire-id reuse) and must keep rendering. | ||
| const latestCommittedBySig = new Map<string, VpItem>(); |
There was a problem hiding this comment.
[Suggestion] No test exercises two DISTINCT live pending tool_group signatures at once, so the per-signature keying of latestCommittedBySig / keptPendingBySig is unpinned — Concrete cost: probe-confirmed: replacing the sig-keyed Map with a single "latest committed match" slot survives all nine #9420 tests, yet a real input discriminates it: history=[batchA_committed, batchB_committed], pending=[batchA, batchB] should render [banner, -1, -2], but the mutant renders [banner, 1, -1, -2], resurrecting the #9420 duplicate for the older batch.
Witness (probe):
single-slot mutant vs suite: 9 passed | 25 skipped (34) <- mutant survives all nine #9420 tests
pinning test vs mutant: expected [ -9007199254740991, -1, -2 ] — received [ -9007199254740991, 1, -1, -2 ]
Suggested fix — add one test with two live batches each having a committed counterpart:
renderMainContent(
createUIState({
history: [
{ id: 1, ...toolGroupFixture('call-A') },
{ id: 2, ...toolGroupFixture('call-B') },
],
pendingHistoryItems: [toolGroupFixture('call-A'), toolGroupFixture('call-B')],
}),
);
expect(lastVpDataIds()).toEqual([Number.MIN_SAFE_INTEGER, -1, -2]);中文说明
[Suggestion] 没有任何测试同时验证两个不同的实时 pending tool_group 签名，因此 latestCommittedBySig / keptPendingBySig 的按签名键控未被固化 —— 具体代价：探针确认：把按签名的 Map 替换为单个"最新已提交匹配"槽位后，全部九个 #9420 测试依然通过，但真实输入可以区分二者：history=[batchA_committed, batchB_committed]、pending=[batchA, batchB] 应渲染 [banner, -1, -2]，而变异体渲染 [banner, 1, -1, -2]，让较早批次的 #9420 重复渲染复活。
证据（探针）：
单槽位变异体 vs 测试套件: 9 passed | 25 skipped (34) <- 变异体在全部九个 #9420 测试下存活
固化测试 vs 变异体: expected [ -9007199254740991, -1, -2 ] — received [ -9007199254740991, 1, -1, -2 ]
建议修复 —— 新增一个两个实时批次各有已提交对应项的测试：
renderMainContent(
createUIState({
history: [
{ id: 1, ...toolGroupFixture('call-A') },
{ id: 2, ...toolGroupFixture('call-B') },
],
pendingHistoryItems: [toolGroupFixture('call-A'), toolGroupFixture('call-B')],
}),
);
expect(lastVpDataIds()).toEqual([Number.MIN_SAFE_INTEGER, -1, -2]);— qwen3.8-max via Qwen Code /review (v0.21.13)
4 participants
What this PR does
Fixes the TUI bug where the most recently invoked tool call row renders twice while executing, then collapses to one when the next tool call arrives (which then shows twice). The same in-flight
tool_groupwas present both in committed history and the live pending list (or twice within pending), soMainContent's combined virtualized list rendered it twice.Fix: in
MainContent, collapse consecutivetool_grouprows that share an identical callId signature (toolGroupSignature), keeping the later item so the live pending copy (which keeps updating) wins over the static history copy. Only true consecutive duplicates are collapsed; distinct batches are untouched.Why it's needed
#9420: every sequential tool call showed the latest row twice until the next arrived — a visible, per-tool-call rendering defect in the default TUI.
Reviewer Test Plan
How to verify
npx vitest run packages/cli/src/ui/components/MainContent.test.tsx— 24 tests pass, including the new "collapses a tool_group duplicated across history and pending (bug(ui): in-flight tool call rendered twice until the next tool call arrives (history + pending double-include) #9420)" test (asserts the duplicated batch renders once as the pending item, not twice).read_filecalls; each tool row should render exactly once while executing.Evidence (Before & After)
Before: latest tool call row twice; previous collapses to one when next arrives. After: each tool call renders once. (Unit test pins the combined-list dedup.)
Tested on
Risk & Scope
Linked Issues
Fixes #9420
中文说明
这个 PR 做了什么
修复 TUI 中"最新一条工具调用显示两次、下一条到来才恢复"的 bug（#9420）。根因是同一 in-flight tool_group 同时存在于已提交 history 与实时 pending 列表（或 pending 内两次），MainContent 拼接的虚拟列表渲染了两份。修复：对相邻且 callId 签名相同的 tool_group 折叠为一份，保留后者（实时 pending 副本，状态持续更新）。仅折叠真正的相邻重复，不同批次不受影响。
风险与范围