fix(cli): allow clicking to expand/collapse thought while streaming - #8443
fix(cli): allow clicking to expand/collapse thought while streaming#8443chiga0 wants to merge 14 commits into
Conversation
|
@qwen-code /takeover |
|
✅ Qwen Triage finished — CI landed green on ✅ Qwen Triage 已完成 —— |
|
🤝 Takeover engaged: the autofix loop now manages this PR — it will address new review feedback and resolve base conflicts until the label is removed or the round cap is reached. This is a fork PR, so the first round comes from the next scheduled scan (usually within minutes). Remove the 中文说明🤝 已接管:autofix 循环现在管理此 PR —— 将持续处理新的评审反馈与 base 冲突,直到移除标签或达到轮次上限。本 PR 来自 fork,首轮处理将由下一次定时扫描执行(通常几分钟内)。移除 |
|
Thanks for the PR! Template looks good ✓ Problem: observed, not theoretical. The base code corroborates it directly: Direction: aligned — a TUI usability fix for the thinking block, squarely in the CLI's core experience. Claude Code's CHANGELOG has no exact match for click-to-watch-streaming-thinking, but that area is actively maintained there (thinking status-row rendering, elapsed-time updates), so the direction is consistent. Size: no core paths touched — everything is in Approach: the scope feels right. The change is the minimum possible — it removes the two Risk: no elevated risk signals — none of the changed files match the revert-correlated high-risk paths. Moving on to code review. 🔍 中文说明感谢贡献! 模板完整 ✓ 问题:已观测到的问题,不是理论性的。基础代码直接印证: 方向:对齐——思考块的 TUI 易用性修复,属于 CLI 核心体验。Claude Code 的 CHANGELOG 没有与"点击实时观看流式思考"完全对应的条目,但该领域在那里维护活跃(思考状态行渲染、耗时更新),方向一致。 规模:未触及核心路径——全部在 方案:范围合理。改动已是最小——移除两处 风险:无升级风险信号——改动文件均未命中与 revert 相关的高风险路径。 进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
Code reviewI reconstructed the minimal fix from the problem statement before reading the diff: the behavior is exactly two What I checked along the way:
No critical findings, no convention violations. CI evidence (PR's own checks on the reviewed commit, fetched via API)The main ubuntu unit suite is still running (it takes ~30 minutes — not polling). The macOS / Windows / integration checks are skipped by design: Final CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行(取最新一次运行),省略 skipped,失败项排在最前。 Sandboxed verification would settle the one claim static review can't: Not verified: live click behaviour in a real terminal (unattended CI run — no local tmux pass here); Windows/Linux terminals (author tested on macOS only — the author's claim, not independently re-run). 中文说明代码审查:先根据问题描述独立推导最小修复——该行为恰好由两处 核对过的点:终端模式门控未变(VP 门控、 无阻塞性问题,无规范违规。 CI 证据(通过 API 获取的被审提交上的 PR 自身检查):ubuntu 主测试套件仍在运行(约 30 分钟,不轮询等待);macOS / Windows / 集成测试按 沙箱验证可补齐静态审查看不到的唯一论断: 未验证:真实终端中的实际点击行为(本次为 CI 无人值守运行,不做本地 tmux 测试);Windows/Linux 终端(作者仅在 macOS 上测试——这是作者的自述,不是独立复测结果)。 — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 4/5 — clean, minimal, well-tested parity fix; the only reservation is that the live click path is exercised through mocked unit tests, so end-to-end behaviour rests on the Honest read: this is the kind of PR that's easy to approve for the right reasons. The problem is real and verified in the base code (two CI is still running (the ubuntu unit suite), so approval is deferred until CI lands green on 中文说明置信度:4/5 —— 干净、最小化、测试充分的交互对等修复;唯一保留是真实点击路径由 mock 的单元测试覆盖,端到端行为依赖上文指出的 直接说:这个 PR 值得以正确的理由被批准。问题真实存在且已在基础代码中核实(两处 CI 仍在运行(ubuntu 单元测试套件),因此批准推迟到 CI 在被审提交上变绿之后。 — Qwen Code · qwen3.8-max 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. ✅
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Test Plan (not a blocker): 57 tests pass — this review observed 17023 passed.
中文说明
Test Plan(非阻断):57 tests pass — this review observed 17023 passed。
— qwen3.8-max via Qwen Code /review (v0.21.4)
| // Active while pending too: clicking a streaming thought expands it so the | ||
| // reasoning can be watched live (and collapsed again). | ||
| { isActive: true }, |
There was a problem hiding this comment.
[Critical] Enabling the click handler while pending makes toggleThought(thoughtGroupHeadId) reachable for streaming thoughts, but MainContent renders every pending item as item={{ ...item, id: 0 }} without a thoughtHeadId (VP path MainContent.tsx:382, Static pending path :544), so thoughtGroupHeadId = thoughtHeadId ?? item.id resolves to the shared sentinel 0, and expandedThoughtHeadIds is session-lifetime state that is never cleared or migrated.
Failure scenario: the user clicks to expand a streaming thought → toggleThought(0). When the thought commits (commitPendingThought re-adds it under a real baseTimestamp + counter id, never 0), expandedHeadIds.has(realHeadId) is false → the thought the user expanded snaps collapsed the moment streaming finishes, defeating this PR's stated purpose. The stuck 0 also flips the initial state of every subsequent pending thought in the session. A third manifestation: reasoning exceeding STREAM_PENDING_ITEM_MAX_CHARS (16 384) commits its head chunk under a real id while the tail stays pending keyed off 0 — expanding the committed head leaves the live streaming tail invisible. The new tests render HistoryItemDisplay directly with thoughtItem.id = 1, bypassing the id: 0 remap, so they pass while production loses the state (probe-verified through the real MainContent render path, with a flip-check against the base).
Suggested fix: thread the thought-group head id through the pending render path (pass thoughtHeadId for pending items in MainContent) and migrate the pending expansion entry to the committed head id when the thought commits. A fix that only clears key 0 at commit still leaves split long thoughts broken.
中文说明
[Critical] 在 pending 时启用点击处理后,流式思考块会触发 toggleThought(thoughtGroupHeadId),但 MainContent 把每个 pending 项渲染为 item={{ ...item, id: 0 }} 且不传 thoughtHeadId(VP 路径 MainContent.tsx:382 与 Static pending 路径 :544 均如此),因此 thoughtGroupHeadId = thoughtHeadId ?? item.id 解析为共享哨兵值 0,而 expandedThoughtHeadIds 是会话级状态,从不清理或迁移。
失败场景:用户点击展开流式思考 → toggleThought(0)。思考提交时(commitPendingThought 以真实的 baseTimestamp + counter id 重新添加,永不为 0),expandedHeadIds.has(realHeadId) 为 false → 用户展开的思考块在流式结束的瞬间自动折叠,与本 PR 的目标直接冲突。残留的 0 还会翻转本会话中后续每个 pending 思考块的初始状态。第三种表现:推理文本超过 STREAM_PENDING_ITEM_MAX_CHARS(16 384)时,头部块以真实 id 提交,而尾部仍以 0 为键处于 pending——展开已提交的头部后,正在流式输出的尾部却不可见。新增测试直接以 thoughtItem.id = 1 渲染 HistoryItemDisplay,绕过了 id: 0 重映射,因此测试通过而生产环境丢失状态(已通过真实 MainContent 渲染路径的探针验证,并对 base 做了翻转对照)。
建议修复:将思考组的 head id 贯通到 pending 渲染路径(在 MainContent 中为 pending 项传入 thoughtHeadId),并在思考提交时把 pending 展开记录迁移到提交后的 head id——仅在提交时清理 0 键仍会导致被切分的长思考块失步。
— qwen3.8-max via Qwen Code /review (v0.21.4)
| const collapseHint = ` ${t('({{keyHint}} to collapse)', { | ||
| keyHint: toggleKeyHint, | ||
| })}`; |
There was a problem hiding this comment.
[Suggestion] The collapse hint now shown while a pending thought is expanded advertises ctrl+o, but ctrl+o toggles the app-wide full-detail switch (setThoughtExpanded in AppContainer), not the per-item expandedThoughtHeadIds set — it cannot collapse a thought that was expanded by clicking.
Failure scenario: the user clicks a streaming thought to expand it; the header reads "∵ Thinking… (ctrl+o to collapse)". Pressing ctrl+o force-expands every thought and tool group in the session (the opposite of the advertised effect); pressing it again returns to per-item state, which still contains the clicked key — so the block remains expanded. Only a second mouse click collapses it. This mismatch pre-exists for committed click-expanded thoughts, but this diff newly extends the hint to pending thoughts, where clicking is the primary expansion path.
Suggested fix: make the hint accurate for the click-expanded state — e.g. mention clicking again to collapse when clickable.
中文说明
[Suggestion] pending 思考块展开时显示的收起提示写着 ctrl+o,但 ctrl+o 切换的是应用级的 full-detail 开关(AppContainer 中的 setThoughtExpanded),而不是逐项的 expandedThoughtHeadIds 集合——它无法收起通过点击展开的思考块。
失败场景:用户点击流式思考块将其展开,头部显示 "∵ Thinking… (ctrl+o to collapse)"。按下 ctrl+o 会强制展开会话中的所有思考块和工具组(与提示效果相反);再按一次回到逐项状态,但其中仍包含被点击的键——该块依然保持展开。只有再次鼠标点击才能收起。这一错配在已提交的、通过点击展开的思考块上本就存在,但本 diff 首次将该提示扩展到 pending 思考块,而点击正是其主要展开途径。
建议修复:让提示在点击展开的状态下保持准确——例如在 clickable 时提示再次点击可收起。
— qwen3.8-max via Qwen Code /review (v0.21.4)
| const label = isPending | ||
| ? `${t('Thinking')}…${durationSuffix}` | ||
| : (completedLabel ?? t('Thinking')); |
There was a problem hiding this comment.
[Suggestion] The collapsed branch added by this diff duplicates the expanded branch's pending-label arm — `${t('Thinking')}…${durationSuffix}` appears at both ~line 332 and ~line 346 — and the pending-icon ternary {isPending ? THINKING_ICON_PENDING : THINKING_ICON} also appears twice (~line 339 and ~line 355). Before this PR a pending thought always fell through to the expanded branch, so the pending header was computed in exactly one place; now both branches are live during streaming.
Concrete cost: any change to the streaming header (wording, duration suffix, animated icon/spinner) must be made in two verbatim copies ~15 lines apart; missing one makes the collapsed and expanded states visibly disagree on every toggle during streaming.
Suggested fix: hoist the shared pieces above the if (!expanded) early return — e.g. const pendingLabel = ${t('Thinking')}…${durationSuffix}; and const headerIcon = isPending ? THINKING_ICON_PENDING : THINKING_ICON; — and use them in both branches (the committed fallbacks differ deliberately and should stay separate).
中文说明
[Suggestion] 本 diff 新增的 collapsed 分支复制了 expanded 分支的 pending 标签臂——`${t('Thinking')}…${durationSuffix}` 同时出现在约第 332 行与约第 346 行——pending 图标三元表达式 {isPending ? THINKING_ICON_PENDING : THINKING_ICON} 也出现两次(约第 339 行与约第 355 行)。在本 PR 之前,pending 思考块总是落入 expanded 分支,pending 头部只在一处计算;现在两个分支在流式输出期间都会被走到。
具体代价:对流式头部的任何改动(措辞、时长后缀、动画图标/spinner)都必须在相隔约 15 行的两份逐字拷贝中同步修改,漏改其一会导致收起与展开状态在用户流式切换时肉眼可见地不一致。
建议修复:把共享部分提升到 if (!expanded) 提前返回之前——例如 const pendingLabel = ${t('Thinking')}…${durationSuffix}; 与 const headerIcon = isPending ? THINKING_ICON_PENDING : THINKING_ICON;——并在两个分支中复用(committed 的兜底文案有意不同,应保持分开)。
— qwen3.8-max via Qwen Code /review (v0.21.4)
| // Active while pending too: clicking a streaming thought expands it so the | ||
| // reasoning can be watched live (and collapsed again). | ||
| { isActive: true }, |
There was a problem hiding this comment.
[Suggestion] onToggle calls toggleThought(thoughtGroupHeadId) unconditionally, with no full-detail guard: while fullDetail is active (resolvedThoughtExpanded = fullDetail || (...)), a click on a pending thought is a visual no-op that silently records a per-item toggle.
Failure scenario: with Ctrl+O full-detail on, the user clicks a streaming thought's header to collapse just that stream — nothing visibly changes, but the sentinel id is added to expandedThoughtHeadIds. When the user later exits full-detail via Ctrl+O (which the on-screen collapse hint advertises), everything else collapses but this thought stays expanded — the opposite of the two collapse attempts. The entry persists for the session. Pre-PR this was unreachable while pending (isActive = !isPending plus a detached ref); this diff newly enables it (the committed-thought variant predates the PR and is out of scope).
Suggested fix (probe-verified to flip the behavior): onToggle={fullDetail ? () => {} : () => toggleThought(thoughtGroupHeadId)}
中文说明
[Suggestion] onToggle 无条件调用 toggleThought(thoughtGroupHeadId),没有 full-detail 守卫:在 fullDetail 激活时(resolvedThoughtExpanded = fullDetail || (...)),点击 pending 思考块在界面上毫无变化,却会悄悄记录一次逐项切换。
失败场景:在 Ctrl+O full-detail 开启时,用户点击某个流式思考块的头部想只收起它——界面没有任何变化,但哨兵 id 已被加入 expandedThoughtHeadIds。当用户稍后再次按 Ctrl+O 退出 full-detail(屏幕上的收起提示正是这样引导的)时,其他内容都收起了,这个思考块却保持展开——与用户两次收起操作的意图相反。该记录在整个会话期间保留。PR 之前 pending 时无法触发(isActive = !isPending 且 ref 未挂载),本 diff 使其首次可达(committed 思考块的同类问题在本 PR 之前就存在,不在本次范围内)。
建议修复(已经探针验证可翻转该行为):onToggle={fullDetail ? () => {} : () => toggleThought(thoughtGroupHeadId)}
— qwen3.8-max via Qwen Code /review (v0.21.4)
In mapToDisplay's non-'error' branch every TrackedToolCall variant carries a required tool and invocation; the 'error' variant that lacks them is already excluded by the discriminant above. Assert non-null so tsc narrows cleanly. Pre-existing error that blocked npm run typecheck.
Address review feedback on PR QwenLM#8443: - Key a pending thought's click expansion off a dedicated sentinel and migrate it to the real committed head id when the thought commits, so a thought expanded while streaming stays expanded instead of snapping collapsed. A split (long) thought's pending tail keys off the already-committed head id. - Collapse hint now advertises clicking when the thought is clickable, since ctrl+o is the app-wide full-detail switch and cannot collapse a click-expanded thought. - Hoist the shared pending header label/icon so the collapsed and expanded branches no longer duplicate them. - Ignore clicks while ctrl+o fullDetail is active so they don't silently record a per-item toggle.
|
🤖 Addressed the latest review feedback (round 1/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 1/100 轮)。改动内容与我反驳保留之处如下: All four review findings are addressed and resolved in the code. No conflicts ( FindingsR1-1 — Critical: pending-thought expansion keyed off the shared
|
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline. 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.3)
| const settlePendingThoughtExpansion = useCallback( | ||
| (committedHeadId: number | null) => { | ||
| setExpandedThoughtHeadIds((prev) => { |
There was a problem hiding this comment.
[Suggestion] The pending→committed expansion migration — the mechanism that keeps a thought expanded after the user clicked it open during streaming — has zero test coverage: no test references this reducer or its three call sites in useGeminiStream.ts (settle(null) when a fresh thought starts, the split-commit, and commitPendingThought). The new MainContent tests pin only the render-side keying before commit, and the HistoryItemDisplay tests mock toggle — nothing crosses the pending→commit boundary.
Failure scenario: deleting next.add(committedHeadId) in this reducer, or the settlePendingThoughtExpansion?.(committedId) call in commitPendingThought, ships green — a thought the user clicked open during streaming silently snaps collapsed the instant it commits (the exact regression the round-1 Critical described), and nothing in the suite fails.
Suggested fix: add a useGeminiStream-level test passing a spy settlePendingThoughtExpansion — assert it is called with the committed head id on thought commit, with the split head id on a long-thought split, and with null when a fresh thought begins — and/or a unit test of this reducer (seed the set with PENDING_THOUGHT_HEAD_ID: settle(42) migrates to 42, settle(null) drops, settle without the sentinel no-ops).
中文说明
[Suggestion] pending→committed 展开迁移机制——让用户在流式期间点击展开的思考块在提交后保持展开——完全没有测试覆盖:没有任何测试引用此 reducer 或其在 useGeminiStream.ts 中的三个调用点(新思考开始时的 settle(null)、split 提交、commitPendingThought)。新增的 MainContent 测试只钉住提交前的渲染侧键控,HistoryItemDisplay 测试 mock 了 toggle——没有任何测试跨越 pending→commit 边界。
失败场景:删除此 reducer 中的 next.add(committedHeadId),或删除 commitPendingThought 中的 settlePendingThoughtExpansion?.(committedId) 调用,整个套件仍会全绿——用户在流式期间点击展开的思考块会在提交瞬间悄悄折叠(正是第一轮 Critical 描述的回归),没有任何测试能捕获。
建议修复:添加一个 useGeminiStream 层测试,传入 spy settlePendingThoughtExpansion——断言思考提交时以 committed head id 被调用、长思考 split 时以 split head id 被调用、新思考开始时以 null 被调用——和/或为此 reducer 补充单元测试(set 中预置 PENDING_THOUGHT_HEAD_ID:settle(42) 迁移到 42、settle(null) 丢弃、无哨兵时 no-op)。
— qwen3.8-max via Qwen Code /review (v0.21.4)
| const collapseHint = clickable | ||
| ? ` ${t('(click or {{keyHint}} to collapse)', { keyHint: toggleKeyHint })}` | ||
| : ` ${t('({{keyHint}} to collapse)', { keyHint: toggleKeyHint })}`; |
There was a problem hiding this comment.
[Suggestion] While fullDetail (Ctrl+O) is active, this hint advertises "click … to collapse", but the fullDetail click guard added in this same PR (HistoryItemDisplay.tsx: if (!fullDetail) toggleThought(...)) silently swallows clicks — an emergent contradiction between this round's two fixes (the click wording here and the fullDetail guard there). clickable is VP-gated only and fullDetail-unaware. Before this PR the collapse hint never mentioned clicking, so the dead affordance was not advertised.
Failure scenario: VP mode + mouse tracking on; the user presses Ctrl+O → every thought renders expanded showing "(click or ctrl+o to collapse)"; clicking a thought header does nothing (the guard swallows it), repeatedly, while the hint keeps promising the click works. Only Ctrl+O — which collapses everything — functions.
Suggested fix: thread fullDetail into ClickableThinkMessage and pass clickable={clickable && !fullDetail} to ThinkMessage (or otherwise suppress the click wording while fullDetail is on), so fullDetail mode shows the keyboard-only collapse hint.
中文说明
[Suggestion] 在 fullDetail(Ctrl+O)激活时,此提示宣传 "click … to collapse",但本 PR 同一轮新增的 fullDetail 点击守卫(HistoryItemDisplay.tsx:if (!fullDetail) toggleThought(...))会静默吞掉点击——这是本轮两个修复(此处的 click 措辞与那里的 fullDetail 守卫)之间的涌现矛盾。clickable 只受 VP 门控,不感知 fullDetail。本 PR 之前收起提示从不提及点击,因此这一死操作此前并未被宣传。
失败场景:VP 模式 + 鼠标追踪开启;用户按 Ctrl+O → 每个思考块都展开并显示 "(click or ctrl+o to collapse)";点击思考块头部毫无反应(守卫吞掉点击),反复点击依旧无效,而提示一直承诺点击可用。唯一有效的是 Ctrl+O——它会把所有内容全部收起。
建议修复:将 fullDetail 透传进 ClickableThinkMessage,向 ThinkMessage 传 clickable={clickable && !fullDetail}(或以其他方式在 fullDetail 开启时抑制 click 措辞),使 fullDetail 模式下只显示键盘收起提示。
— qwen3.8-max via Qwen Code /review (v0.21.4)
| displayName = trackedCall.tool!.displayName; | ||
| description = trackedCall.invocation!.getDescription(); | ||
| renderOutputAsMarkdown = trackedCall.tool!.isOutputMarkdown; |
There was a problem hiding this comment.
[Suggestion] These three non-null assertions (from the autofix commit cb344ee) are unnecessary at this commit — verified empirically: with the ! removed, npx tsc --noEmit in packages/cli still passes, because the status === 'error' discriminant already excludes ErroredToolCall, the only union member with optional tool / missing invocation. They are also unrelated to this PR's thought-expansion feature.
Failure scenario: if a future core change makes tool/invocation optional on a non-error ToolCall variant, these assertions keep the compiler silent and mapToDisplay throws a TypeError on every tool-call render, with no compile-time warning.
| displayName = trackedCall.tool!.displayName; | |
| description = trackedCall.invocation!.getDescription(); | |
| renderOutputAsMarkdown = trackedCall.tool!.isOutputMarkdown; | |
| displayName = trackedCall.tool.displayName; | |
| description = trackedCall.invocation.getDescription(); | |
| renderOutputAsMarkdown = trackedCall.tool.isOutputMarkdown; |
中文说明
[Suggestion] 这三个非空断言(来自 autofix 提交 cb344ee)在此提交下并不必要——已经实验验证:移除 ! 后,packages/cli 中的 npx tsc --noEmit 依然通过,因为 status === 'error' 判别式已排除 ErroredToolCall(唯一 tool 可选、无 invocation 的联合成员)。它们也与本 PR 的思考块展开功能无关。
失败场景:若未来 core 改动使某个非 error 状态的 ToolCall 变体的 tool/invocation 变为可选,这些断言会让编译器保持沉默,mapToDisplay 将在每次工具调用渲染时抛出 TypeError,且没有任何编译期警告。
— qwen3.8-max via Qwen Code /review (v0.21.4)
- Pin the pending->committed expansion settle calls with useGeminiStream tests: null on a fresh thought start, the committed head id on commit, the split head id on oversized-thought splits, and never for gemini_thought_content tails. - Suppress the click wording in the collapse hint while fullDetail (Ctrl+O) is active, since the fullDetail guard swallows clicks. - Drop the unnecessary non-null assertions in mapToDisplay; the status === 'error' discriminant already narrows them away, restoring the file to its pre-PR state.
|
🤖 Addressed the latest review feedback (round 2/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 2/100 轮)。改动内容与我反驳保留之处如下: Autofix review round — PR #8443All three inline suggestions from the automated reviewer (round 2) were addressed in one commit: Feedback points and dispositions
Conflict notes
Checks
VerificationCommands actually run and their results:
中文说明Autofix 审查轮次 — PR #8443自动审查者(第二轮)提出的三条行内建议已全部在一个提交中处理: 反馈点及处理结果
冲突说明
检查项
验证(Verification)实际执行的命令及结果:
Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审;第 100 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline. 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.3)
| // turned back off. Ignore it. | ||
| if (!fullDetail) toggleThought(thoughtGroupHeadId); |
There was a problem hiding this comment.
[Suggestion] R3-1: The thoughtHeadId consumption seam (thoughtGroupHeadId = thoughtHeadId ?? item.id, ~line 263) is exercised by no test — no test passes a thoughtHeadId prop to HistoryItemDisplay (its test file never sets it; MainContent.test.tsx mocks the component and asserts only on delivered props), so the prop-present branch never runs. — Failure scenario: verified by live mutation: replacing this consumption site with const thoughtGroupHeadId = item.id; leaves all 86 relevant tests passing, yet breaks the feature end-to-end in production — pending items render with item.id = 0, so a click records the toggle under 0 instead of the sentinel -1; settlePendingThoughtExpansion then finds no sentinel to migrate at commit, and the thought the user expanded while streaming collapses when it commits; split thought groups also un-link (tails no longer follow the head's expansion).
Suggested fix — add tests that exercise the seam:
// HistoryItemDisplay.test.tsx — render a pending thought with the sentinel
<HistoryItemDisplay
item={thoughtItem}
terminalWidth={100}
isPending={true}
thoughtHeadId={PENDING_THOUGHT_HEAD_ID}
/>
// assert a complete press+release click calls toggle with
// PENDING_THOUGHT_HEAD_ID (not item.id), and that a committed
// gemini_thought_content with thoughtHeadId={42} resolves
// expanded from expandedHeadIds.has(42)中文说明
thoughtHeadId 的消费接缝(thoughtGroupHeadId = thoughtHeadId ?? item.id,约第 263 行)没有任何测试覆盖——没有测试向 HistoryItemDisplay 传入 thoughtHeadId prop(其测试文件从未设置该 prop;MainContent.test.tsx 直接 mock 了该组件,只断言下发的 props),因此“prop 存在”的分支从未被执行。 — 失败场景(已通过活体突变验证):把该消费点替换为 const thoughtGroupHeadId = item.id;,全部 86 个相关测试仍然通过,但生产环境中功能端到端失效——pending 项以 item.id = 0 渲染,点击会把切换记录在 0 下而非哨兵值 -1;提交时 settlePendingThoughtExpansion 找不到可迁移的哨兵键,用户在流式期间展开的思考块会在提交瞬间折叠;被切分的思考组也会失联(尾部不再跟随头部展开)。
建议修复——新增覆盖该接缝的测试:以 thoughtHeadId={PENDING_THOUGHT_HEAD_ID} 渲染 pending 思考块,断言完整的按下+释放点击后 toggle 以 PENDING_THOUGHT_HEAD_ID(而非 item.id)被调用;再断言已提交的 gemini_thought_content(thoughtHeadId={42})的展开状态取自 expandedHeadIds.has(42)。
— qwen3.8-max via Qwen Code /review (v0.21.3)
| thoughtHeadId={pendingThoughtHeadId( | ||
| item.type, | ||
| lastCommittedThoughtHeadIdRef.current, | ||
| )} |
There was a problem hiding this comment.
[Suggestion] R3-2: Split-tail keying (pending gemini_thought_content → committed head id) is only tested through the Static render path; this VP path — the only mode where clicking works — has no tail test (its only sentinel test passes a gemini_thought head, which ignores the second argument of pendingThoughtHeadId). — Failure scenario: a regression that breaks this site's second argument — the ref read dropped during a refactor of renderVirtualItem, or the lastCommittedThoughtHeadIdRef.current = … assignment line deleted — survives every existing test. User-visible result in VP mode: while streaming an oversized split thought, the pending tail stops resolving against the head's expansion key, so clicking the head expands only the head chunk and the streaming tail stays collapsed — a fragmented live view, in the only mode where clicking exists.
Suggested fix — mirror the existing Static-path test through the VP path:
// MainContent.test.tsx — same scenario, VP mode
renderMainContent(
createUIState({
useTerminalBuffer: true,
history: [{ id: 42, type: 'gemini_thought', text: 'head chunk' }],
pendingHistoryItems: [
{ type: 'gemini_thought_content', text: 'streaming tail…' },
],
}),
);
// assert the pending tail's thoughtHeadId is 42中文说明
切分尾部的键控(pending gemini_thought_content → 已提交的 head id)只在 Static 渲染路径下有测试;VP 路径——唯一支持点击的模式——没有尾部测试(该路径唯一的哨兵测试传入的是 gemini_thought 头部,不会触及 pendingThoughtHeadId 的第二个参数)。 — 失败场景:任何破坏该调用点第二个参数的回归——重构 renderVirtualItem 时删掉 ref 读取,或删除 lastCommittedThoughtHeadIdRef.current = … 赋值行——都能在现有全部测试下存活。VP 模式下的用户可见后果:流式输出超长被切分的思考时,pending 尾部不再解析到头部的展开键,点击头部只会展开头部块,正在流式输出的尾部保持折叠——在唯一支持点击的模式下出现割裂的实时视图。
建议修复——把现有的 Static 路径测试镜像到 VP 路径:以 useTerminalBuffer: true 渲染“已提交 head(id 42)+ pending gemini_thought_content 尾部”的场景,断言尾部的 thoughtHeadId 为 42。
— qwen3.8-max via Qwen Code /review (v0.21.3)
| thoughtHeadId={pendingThoughtHeadId( | ||
| item.type, | ||
| lastCommittedThoughtHeadId, | ||
| )} |
There was a problem hiding this comment.
[Suggestion] R3-4: No test pins fullDetail propagation to pending items on either render path (this Static site and the VP twin at ~line 413). This diff makes pending thoughts clickable and adds the if (!fullDetail) toggleThought(…) guard in HistoryItemDisplay, whose effectiveness depends on MainContent forwarding fullDetail here. Probe-verified: deleting fullDetail={fullDetail} from both pending sites survives the whole existing suite (all 23 MainContent + 39 HistoryItemDisplay tests pass), while a test asserting the pending spy call received fullDetail: true fails on both paths under the mutant and passes on this PR. — Failure scenario: if a future refactor drops fullDetail={fullDetail} from one/both pending sites, every test still passes, and in production during Ctrl+O full-detail mode a streaming thought advertises “click … to expand/collapse” and records a per-head toggle on click; when fullDetail turns off, that thought's expansion silently flips — the deferred-flip regression the added guard's comment explicitly exists to prevent.
Suggested fix:
// MainContent.test.tsx — pending item under fullDetail
renderMainContent(
createUIState({
pendingHistoryItems: [{ type: 'gemini_thought', text: 'reasoning…' }],
}),
{ thoughtExpandedProviderValue: { allExpanded: true, expandedHeadIds: new Set(), toggle } },
);
// assert the pending HistoryItemDisplay spy call received fullDetail: true
// (mirror the existing committed tool_group assertion, VP and Static paths)中文说明
没有任何测试固定“fullDetail 向 pending 项的传播”(此 Static 调用点与约第 413 行的 VP 对应点均无)。本 PR 让 pending 思考块可点击,并在 HistoryItemDisplay 中新增了 if (!fullDetail) toggleThought(…) 守卫,而该守卫是否生效取决于 MainContent 在此处转发 fullDetail。探针验证:从两个 pending 调用点删除 fullDetail={fullDetail} 后,现有全部测试(MainContent 23 个 + HistoryItemDisplay 39 个)依然通过;而断言 pending spy 收到 fullDetail: true 的测试在该突变下两条路径均失败,在本 PR 代码下均通过。 — 失败场景:未来重构若从一个或两个 pending 调用点删除 fullDetail={fullDetail},所有测试仍然绿灯,而生产环境中 Ctrl+O 全详情模式下,流式思考块会显示“click … to expand/collapse”并在点击时记录按头切换;fullDetail 关闭后,该思考块的展开状态被静默翻转——正是新增守卫注释明确要防止的“延迟翻转”回归。
建议修复:在 ThoughtExpandedProvider(allExpanded: true)下渲染 pending gemini_thought,断言 HistoryItemDisplay 的 spy 调用收到 fullDetail: true(镜像现有已提交 tool_group 的断言,覆盖 VP 与 Static 两条路径)。
— qwen3.8-max via Qwen Code /review (v0.21.3)
| const lastCommittedThoughtHeadId = useMemo(() => { | ||
| for (let i = visibleHistory.length - 1; i >= 0; i--) { | ||
| const item = visibleHistory[i]; |
There was a problem hiding this comment.
[Suggestion] R3-5: This backward scan's “most recent committed thought head, not the first” property is pinned by no test — every existing keying test uses a history with exactly one committed thought (id 42). Probe-verified: a first-match mutant (iterating forward) survives the whole suite, while a two-head probe fails under it with expected 41 to be 42 on both render paths and passes on this PR. — Failure scenario: turn 1 commits a thought head A; in turn 2 a long thought splits (>16 384 chars), head B commits, and under the mutant the pending gemini_thought_content tail gets thoughtHeadId = A. Clicking collapsed head B in VP mode expands B but the actively-streaming tail stays hidden (the exact collapse regression this PR exists to fix), and conversely a previously click-expanded thought A makes turn 2's tail appear expanded under a collapsed head B.
Suggested fix:
// MainContent.test.tsx — two committed heads + pending tail
renderMainContent(
createUIState({
history: [
{ id: 41, type: 'gemini_thought', text: 'first head' },
{ id: 42, type: 'gemini_thought', text: 'second head' },
],
pendingHistoryItems: [
{ type: 'gemini_thought_content', text: 'streaming tail…' },
],
}),
);
// assert the pending tail's thoughtHeadId is 42, not 41
// (ideally on both the Static and VP paths)中文说明
该反向扫描“取最近一个已提交的思考头部(而非第一个)”的性质没有任何测试固定——现有键控测试使用的历史都只有一个已提交思考头(id 42)。探针验证:首匹配突变(改为正向遍历)能在整个测试套件下存活;而双头部探针在该突变下以 expected 41 to be 42 在两条渲染路径均失败,在本 PR 代码下通过。 — 失败场景:第 1 轮提交思考头 A;第 2 轮长思考被切分(>16 384 字符),头部 B 提交,突变下 pending gemini_thought_content 尾部拿到 thoughtHeadId = A。在 VP 模式点击折叠的头部 B 时,B 展开但正在流式输出的尾部仍被隐藏(正是本 PR 要修复的折叠回归);反过来,此前被点击展开的思考 A 会让第 2 轮的尾部在折叠的头部 B 下显示为展开。
建议修复:新增“两个已提交头部 + pending 尾部”的键控测试,断言尾部的 thoughtHeadId 是 42 而非 41(最好同时覆盖 Static 与 VP 路径)。
— qwen3.8-max via Qwen Code /review (v0.21.3)
| if (type === 'gemini_thought') return PENDING_THOUGHT_HEAD_ID; | ||
| if (type === 'gemini_thought_content') return lastCommittedThoughtHeadId; |
There was a problem hiding this comment.
[Suggestion] R3-7: The pending-head → sentinel keying is only pinned against an empty committed history: both head-sentinel tests (Static ~line 1075, VP ~line 1125 in MainContent.test.tsx) use the default history: [], and no test renders a pending gemini_thought head when committed thought heads already exist. Probe-verified: the mutant if (type === 'gemini_thought') return lastCommittedThoughtHeadId ?? PENDING_THOUGHT_HEAD_ID; leaves all 23 existing tests green, while a non-empty-history probe fails under it with expected 42 to be -1 on both paths and passes on this PR. — Failure scenario: from the second thought of any session onward (the common case), under the mutant clicking a streaming thought's head calls toggleThought(lastCommittedHeadId) — expanding/collapsing the previous thought group instead of recording the pending sentinel; settlePendingThoughtExpansion at commit then finds no PENDING_THOUGHT_HEAD_ID entry to migrate, so the thought the user clicked open while streaming collapses at commit — silently reintroducing the exact regression this PR fixes.
Suggested fix:
// MainContent.test.tsx — pending head against non-empty history
renderMainContent(
createUIState({
history: [{ id: 42, type: 'gemini_thought', text: 'earlier head' }],
pendingHistoryItems: [{ type: 'gemini_thought', text: 'reasoning…' }],
}),
);
// assert the pending head's thoughtHeadId is PENDING_THOUGHT_HEAD_ID, not 42
// (on both the Static and VP paths)中文说明
pending 头部 → 哨兵键控只在“已提交历史为空”时被固定:两个头部哨兵测试(MainContent.test.tsx Static 约第 1075 行、VP 约第 1125 行)都使用默认的 history: [],没有任何测试在已存在已提交思考头时渲染 pending gemini_thought 头部。探针验证:突变 if (type === 'gemini_thought') return lastCommittedThoughtHeadId ?? PENDING_THOUGHT_HEAD_ID; 下现有 23 个测试全部绿灯,而非空历史探针在该突变下以 expected 42 to be -1 在两条路径均失败,在本 PR 代码下通过。 — 失败场景:从会话的第二个思考块开始(常见情形),突变下点击流式思考头部会调用 toggleThought(lastCommittedHeadId)——展开/折叠的是上一个思考组,而不是记录 pending 哨兵;提交时 settlePendingThoughtExpansion 找不到 PENDING_THOUGHT_HEAD_ID 条目可迁移,用户在流式期间点击展开的思考块会在提交时折叠——静默地重新引入本 PR 要修复的回归。
建议修复:扩展键控测试,以 history: [{ id: 42, type: 'gemini_thought', … }] 加 pendingHistoryItems: [{ type: 'gemini_thought', … }] 渲染,断言 pending 头部的 thoughtHeadId 是 PENDING_THOUGHT_HEAD_ID(而非 42),覆盖 Static 与 VP 两条路径。
— qwen3.8-max via Qwen Code /review (v0.21.3)
|
🤖 Addressed the latest review feedback (round 3/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 3/100 轮)。改动内容与我反驳保留之处如下: Round-3 review feedback — addressedAll five findings were automated-reviewer [Suggestion] items, each mutation-verified by the reviewer (a named mutant survives the existing suite but breaks the feature in production). All five are test-only additions pinning seams this PR introduces; no production code changed. Every finding was implemented — nothing declined, deferred, or escalated.
Supporting tweak: the Mutation evidence (temporary source mutations, reverted after each run):
No conflict resolution was needed ( Verification
中文说明第 3 轮审查反馈 — 已处理五个发现全部来自自动审查器的 [Suggestion] 条目,且每条都由审查器做了突变验证(指定的突变体能在现有测试套件下存活,但会在生产中破坏功能)。五条均为纯测试补充,用于固定本 PR 引入的接缝;生产代码未改动。所有发现均已实现——没有拒绝、推迟或升级项。
配套微调: 突变证据(临时源码突变,每次运行后还原):
无需冲突处理( 验证
Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审;第 100 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline. 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.5)
| if (pendingThoughtType === 'gemini_thought') { | ||
| settlePendingThoughtExpansion?.(committedSplitId); | ||
| } |
There was a problem hiding this comment.
[Suggestion] R4-3: The migration rule "when a gemini_thought head commits, settle the pending expansion onto its committed id" is pasted into two callbacks in this same PR — here in the handleThoughtEvent split loop (guarded on pendingThoughtType === 'gemini_thought'), and again in commitPendingThought (~lines 1798-1800, guarded on item.type === 'gemini_thought'). The "heads settle, tails never settle" invariant — the seam the three new settlement tests pin — now lives in two places expressed in two terms.
Failure scenario: a future commit path or rule change (e.g. settling on drop) updated in one callback but not the other silently loses the provisional expansion entry — a thought the user clicked open while streaming collapses the moment it commits, with no error pointing at the missed call site.
Suggested fix — one hook-level wrapper used at both call sites (buildThoughtItem(pendingThoughtType, beforeText).type === pendingThoughtType, so the split site is covered by the same item.type check):
const addItemAndSettleThoughtHead = useCallback(
(item: HistoryItemWithoutId, ts: number) => {
const id = addItem(item, ts);
if (item.type === 'gemini_thought') settlePendingThoughtExpansion?.(id);
return id;
},
[addItem, settlePendingThoughtExpansion],
);中文说明
[Suggestion] R4-3:迁移规则"当 gemini_thought 头部提交时,把 pending 展开记录落到其提交后的 id 上"在本 PR 中被粘贴进了两个回调——此处 handleThoughtEvent 的拆分循环(以 pendingThoughtType === 'gemini_thought' 为条件),以及 commitPendingThought(约 1798-1800 行,以 item.type === 'gemini_thought' 为条件)。"头部 settle、尾部永不 settle"这一不变量——三个新 settlement 测试所钉住的接缝——现在以两种表述存在于两处。
失败场景:未来新增提交路径或修改规则(例如在丢弃时 settle)时,若只更新其中一个回调,会静默丢失临时展开记录——用户在流式输出期间点击展开的思考块会在提交瞬间折叠,且没有任何错误指向被遗漏的调用点。
建议修复——在两个调用点共用一个 hook 级包装函数(buildThoughtItem(pendingThoughtType, beforeText).type === pendingThoughtType,因此拆分点同样被 item.type 检查覆盖):
const addItemAndSettleThoughtHead = useCallback(
(item: HistoryItemWithoutId, ts: number) => {
const id = addItem(item, ts);
if (item.type === 'gemini_thought') settlePendingThoughtExpansion?.(id);
return id;
},
[addItem, settlePendingThoughtExpansion],
);— qwen3.8-max via Qwen Code /review (v0.21.5)
| it('hints click-to-collapse when expanded and clickable (VP mode)', () => { | ||
| const withoutClick = render( | ||
| <ThinkMessage {...defaultProps} isPending={true} expanded={true} />, |
There was a problem hiding this comment.
[Suggestion] R4-4: The new collapseHint = clickable ? '(click or …)' : '(…)' expression (ConversationMessages.tsx:348) now governs both pending AND committed expanded thoughts, but the clickable variant is only tested with isPending={true} — here and in the pending case below. The committed+expanded+clickable case is rendered nowhere (both to collapse assertions in HistoryItemDisplay.test.tsx have clickable=false). The mutant const collapseHint = clickable && isPending ? … : … passes every test in this diff, and the old code had exactly an !isPending && restriction in this spot, so a regression reintroducing it is plausible and currently untestable.
Failure scenario: a regression gating the click wording on isPending (as the pre-PR code did) silently drops the "click" wording from the collapse hint of a click-expanded committed thought in VP mode, and no test fails.
Suggested fix — add a sibling case for the committed branch:
it('hints click-to-collapse when committed, expanded and clickable', () => {
const withClick = render(
<ThinkMessage
{...defaultProps}
isPending={false}
expanded={true}
clickable={true}
/>,
).lastFrame();
expect(withClick).toContain('click');
expect(withClick).toContain('to collapse');
});中文说明
[Suggestion] R4-4:新的 collapseHint = clickable ? '(click or …)' : '(…)' 表达式(ConversationMessages.tsx:348)现在同时约束 pending 与已提交(committed)的展开思考块,但 clickable 变体只在 isPending={true} 下被测试——即此处与下方的 pending 用例。committed+expanded+clickable 的组合在任何测试中都未被渲染(HistoryItemDisplay.test.tsx 中两处 to collapse 断言的 clickable=false)。变异体 const collapseHint = clickable && isPending ? … : … 能通过本 diff 的全部测试,而旧代码此处恰好就有 !isPending && 限制,因此重新引入该限制的回归是可能发生的,且目前无法被测试捕获。
失败场景:若回归把 click 措辞重新限定在 isPending 上(与 PR 前的代码一致),VP 模式下已提交且通过点击展开的思考块,其折叠提示会悄悄丢掉 "click" 措辞,而没有任何测试失败。
建议修复——为 committed 分支补充一个平行用例(见上方代码块)。
— qwen3.8-max via Qwen Code /review (v0.21.5)
|
🤖 Addressed the latest review feedback (round 4/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 4/100 轮)。改动内容与我反驳保留之处如下: Round 4 review addressed (PR #8443)Commit: Feedback points and dispositions
ConflictNone ( Verification
中文说明已处理第 4 轮审查(PR #8443)提交: 反馈点及处理结果
冲突无( 验证
Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审;第 100 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
|
🔀 Base updated: red check(s) [Test (ubuntu-latest, Node 22.x)] pass on current main — merged current main via update-branch; CI will re-run. 中文说明🔀 已更新 base:红色检查 [Test (ubuntu-latest, Node 22.x)] 在当前 main 上通过 —— 已通过 update-branch 合入当前 main,CI 将重新运行。 |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline. Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally. Test Plan (not a blocker): 57 tests pass — this review observed 17127 passed.
中文说明
已审查。 建议见行内评论。 未审查:build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。 Test Plan(非阻断):57 tests pass — this review observed 17127 passed。
— qwen3.8-max via Qwen Code /review (v0.21.5)
| // Active while pending too: clicking a streaming thought expands it so the | ||
| // reasoning can be watched live (and collapsed again). |
There was a problem hiding this comment.
[Suggestion] R5-1: The fullDetail click-suppression rule is encoded twice — in clickable (mouseTrackingEnabled && !fullDetail, ~line 135) and again in the parent onToggle guard (if (!fullDetail) toggleThought(...), ~line 345) — because useMouseEvents here is given a hardcoded { isActive: true }. useMouseEvents already applies the VP / mouse-tracking / TTY gates independently (enabled = isActive && isRawModeSupported && vpGateOpen && mouseTrackingEnabled && Boolean(stdout.isTTY)), so { isActive: clickable } plus an unguarded onToggle is behaviorally identical with one !fullDetail check instead of two. (Distinct from the round-1 thread at line 183, which demanded the then-missing guard; this is about the shape of the resulting double encoding.) — Failure scenario: duplication with drift risk: a future change to the click-swallow policy must update both copies (~200 lines apart, two components); updating only clickable ships a hint advertising clicks that are silently ignored, and updating only the guard advertises nothing while clicks work. The new test 'keeps the click handler active while the thought is pending' pins isActive === true without a VP provider, locking the current encoding. Suggested fix: pass { isActive: clickable } here and keep exactly one of the two !fullDetail checks; update the isActive test to render under VirtualViewportContext.Provider value={true}.
中文说明
[Suggestion] fullDetail 的点击抑制规则被编码了两次——一次在 clickable(mouseTrackingEnabled && !fullDetail,约第 135 行),另一次在父级的 onToggle 守卫(if (!fullDetail) toggleThought(...),约第 345 行)——因为此处传给 useMouseEvents 的是硬编码的 { isActive: true }。useMouseEvents 本身已独立应用 VP / 鼠标跟踪 / TTY 门控(enabled = isActive && isRawModeSupported && vpGateOpen && mouseTrackingEnabled && Boolean(stdout.isTTY)),因此传 { isActive: clickable } 并去掉 onToggle 上的守卫在行为上完全等价,且只需一处 !fullDetail 检查而非两处。(与第 183 行第 1 轮的那个话题不同:那条评论要求补上当时缺失的守卫;本条针对的是由此产生的双重编码的形态。)——失败场景:重复编码带来漂移风险:未来修改点击吞没策略时必须同步更新两处副本(相距约 200 行、跨两个组件);只改 clickable 会发出一个宣传点击却被静默忽略的提示,只改守卫则点击可用却不做任何宣传。新测试 'keeps the click handler active while the thought is pending' 在没有 VP Provider 的情况下断言 isActive === true,把当前编码钉死了。建议修复:此处传 { isActive: clickable },两处 !fullDetail 检查只保留一处;并把 isActive 测试改为在 VirtualViewportContext.Provider value={true} 下渲染。
— qwen3.8-max via Qwen Code /review (v0.21.5)
| if (!expanded) { | ||
| const label = isPending ? pendingLabel : (completedLabel ?? t('Thinking')); |
There was a problem hiding this comment.
[Suggestion] R5-2: The ThinkMessageProps.expanded doc comment (line 60) still says "When committed (not pending), whether to show the full reasoning.", but this diff changed the collapsed branch from if (!isPending && !expanded) to if (!expanded), so expanded now gates pending (streaming) thoughts too. — Failure scenario: a maintainer touching ThinkMessage reads the prop doc and concludes pending rendering ignores expanded — e.g. "simplifies" this branch back to if (!isPending && !expanded), or omits expanded when wiring a new pending render surface. That silently re-expands every streaming thought, breaking this PR's click-to-collapse-while-streaming behavior and its tests ('should advertise expansion while pending (streaming)' asserts the pending body is hidden when expanded defaults to false). Suggested fix: update the expanded doc at line 60 to "Whether to show the full reasoning. Applies to both pending (streaming) and committed thoughts; a collapsed thought renders only the header hint line." (the neighbouring clickable doc is also mildly dated and worth touching up in the same pass).
中文说明
[Suggestion] ThinkMessageProps.expanded 的文档注释(第 60 行)仍写着 "When committed (not pending), whether to show the full reasoning.",但本 diff 已把收起分支从 if (!isPending && !expanded) 改为 if (!expanded),因此 expanded 现在同样约束 pending(流式输出中)的思考块。——失败场景:维护者改动 ThinkMessage 时读到该 prop 文档,会误以为 pending 渲染不受 expanded 控制——例如把这个分支"简化"回 if (!isPending && !expanded),或在接入新的 pending 渲染面时漏传 expanded。这会让每个流式思考块静默地重新展开,破坏本 PR 的"流式期间可点击收起"行为及其测试('should advertise expansion while pending (streaming)' 断言了 expanded 缺省为 false 时 pending 正文被隐藏)。建议修复:把第 60 行 expanded 的文档更新为 "Whether to show the full reasoning. Applies to both pending (streaming) and committed thoughts; a collapsed thought renders only the header hint line."(相邻的 clickable 文档也略有陈旧,可一并润色)。
— qwen3.8-max via Qwen Code /review (v0.21.5)
| ).toBe(true); | ||
| }); | ||
|
|
||
| describe('pending thought expansion keying', () => { |
There was a problem hiding this comment.
[Suggestion] R5-3: The read side of the new expansion keying is untested: no test asserts that a pending thought whose key is in expandedHeadIds actually renders expanded — every pending-state test either mocks toggle, asserts hint text, or passes expanded directly. Probe-verified: re-gating resolvedThoughtExpanded on !isPending (in HistoryItemDisplay.tsx) leaves all 94 tests in the three touched component suites green, while a probe asserting that a pending head (sentinel in the set) and a pending tail (head id in the set) render expanded fails under the mutation and passes when reverted. — Failure scenario: a regression making clicks during streaming record state that never renders ships with a green suite (MainContent.test.tsx mocks HistoryItemDisplay — props spy only — so it cannot see the read side). Distinct from R3-1 (the consumption seam, now covered) and R3-4 (fullDetail propagation). Suggested fix: in HistoryItemDisplay.test.tsx, render a pending thought with thoughtHeadId={PENDING_THOUGHT_HEAD_ID} under a ThoughtExpandedProvider whose expandedHeadIds is new Set([PENDING_THOUGHT_HEAD_ID]) and assert the thought text is visible and the collapse hint shows (and, with an empty set, that it is collapsed).
中文说明
[Suggestion] 新展开键控的"读取侧"没有测试覆盖:没有任何测试断言"键在 expandedHeadIds 中的 pending 思考块确实渲染为展开"——现有 pending 用例要么 mock 了 toggle、要么只断言提示文案、要么直接传 expanded。探针验证:在 HistoryItemDisplay.tsx 中给 resolvedThoughtExpanded 重新加上 !isPending 门控后,三个组件测试套件的全部 94 个测试依旧为绿;而一个断言 pending 头部(集合中有哨兵键)与 pending 尾部(集合中有 head id)渲染为展开的探针,在该变异下失败、还原后通过。——失败场景:若回归使"流式期间的点击只写入状态却永不渲染展开",整套测试仍为绿(MainContent.test.tsx mock 了 HistoryItemDisplay——只有 props 探针——看不到读取侧)。与 R3-1(消费接缝,现已覆盖)和 R3-4(fullDetail 传递)是不同的缺口。建议修复:在 HistoryItemDisplay.test.tsx 中,以 thoughtHeadId={PENDING_THOUGHT_HEAD_ID} 渲染一个 pending 思考块,并把 ThoughtExpandedProvider 的 expandedHeadIds 设为 new Set([PENDING_THOUGHT_HEAD_ID]),断言思考正文可见且显示收起提示(空集合时则断言其收起)。
— qwen3.8-max via Qwen Code /review (v0.21.5)
| it('keys a pending thought head off the sentinel in VP mode too', () => { | ||
| historyItemDisplayPropsSpy.mockClear(); |
There was a problem hiding this comment.
[Suggestion] R5-4: The new pending thought expansion keying describe holds five near-verbatim Static/VP test pairs that differ only by useTerminalBuffer: true (sentinel keying with empty history and against committed heads; split-tail keying off the head and off the most recent head; fullDetail forwarding), and the spy-extraction boilerplate (historyItemDisplayPropsSpy.mock.calls.map((c) => c[0]) + .find(...) + .toBeDefined()) is copied ~12 times. — Failure scenario: lockstep-maintenance cost: this PR is itself a prop-plumbing change through both render paths, and the next such change (or a rename of the spy/prop) must edit every copy or silently leaves one asserting a stale shape — review rounds R3-2/R3-5/R3-7 each demanded a missing twin that was answered with another copied block. Both paths genuinely need coverage; the duplication is in the encoding, not the scenarios. Suggested fix: extract a small findPendingProps(type) helper and run the scenario matrix with describe.each([{ vp: false }, { vp: true }]), threading useTerminalBuffer: vp into createUIState; keep the genuinely VP-specific ref-freshness test separate.
中文说明
[Suggestion] 新增的 pending thought expansion keying describe 里有五对几乎逐字相同的 Static/VP 测试,仅差 useTerminalBuffer: true(空历史与有已提交头部两种哨兵键控、尾部以 head 为键、尾部以最近 head 为键、fullDetail 传递),且探针提取样板(historyItemDisplayPropsSpy.mock.calls.map((c) => c[0]) + .find(...) + .toBeDefined())被复制了约 12 次。——失败场景:联动维护成本:本 PR 本身就是一次贯穿两条渲染路径的 prop 管线改动,下一次此类改动(或探针/prop 重命名)必须逐份修改,否则会静默留下某份断言着过期形状——评审第 R3-2/R3-5/R3-7 轮每次都因缺失孪生用例而追加一块复制代码。两条路径确实都需要覆盖;重复在于编码方式,而非场景本身。建议修复:抽出一个小工具函数 findPendingProps(type),用 describe.each([{ vp: false }, { vp: true }]) 跑场景矩阵,并把 useTerminalBuffer: vp 传入 createUIState;真正 VP 专属的 ref 新鲜度测试保持独立。
— qwen3.8-max via Qwen Code /review (v0.21.5)
|
🤖 Addressed the latest review feedback (round 5/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 5/100 轮)。改动内容与我反驳保留之处如下: Round-5 review feedback addressedCommit: Findings and dispositions
Review-level observations (no code action): the automated reviewer noted it could not review the Integration Tests (CLI, No Sandbox) job, and that the PR test plan says "57 tests pass" while it observed 17127. This round's changes are component/test-level and fully covered by the unit suites listed below; the touched behavior is not exercised only through the bundled CLI or integration harness, so no integration run was required. Files changed
Verification
中文说明已处理第 5 轮评审反馈提交: 各项反馈及处理
评审级别的两点观察(无需代码动作):自动评审器表示无法审查 Integration Tests (CLI, No Sandbox) 这个 job,并指出 PR 测试计划写的是 "57 tests pass" 而它观察到 17127。本轮改动都在组件/测试层面,由下面列出的单元测试套件完全覆盖;被改动的行为并非只能通过打包后的 CLI 或集成测试框架验证,因此不需要跑集成测试。 变更文件
验证
Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审;第 100 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline. Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally. Test Plan (not a blocker): 57 tests pass — this review observed 17128 passed.
中文说明
已审查。 建议见行内评论。 未审查:build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。 Test Plan(非阻断):57 tests pass — this review observed 17128 passed。
— qwen3.8-max via Qwen Code /review (v0.21.5)
| const lastCommittedThoughtHeadId = useMemo(() => { | ||
| for (let i = visibleHistory.length - 1; i >= 0; i--) { | ||
| const item = visibleHistory[i]; | ||
| if (item && item.type === 'gemini_thought') return item.id; | ||
| } | ||
| return undefined; | ||
| }, [visibleHistory]); |
There was a problem hiding this comment.
[Suggestion] The new lastCommittedThoughtHeadId backward scan re-derives what the existing buildThoughtHeadIdMap(visibleHistory) pass — memoized two lines above with the same visibleHistory dep — already produces: the last gemini_thought head its forward pass visits is exactly the id this backward scan recomputes. The "what counts as a thought head" rule (item.type === 'gemini_thought') now lives in two places (historyUtils.ts and this component), and while both existing shapes in historyUtils.ts (buildThoughtHeadIdMap and the backward findLastUserItemIndex) have unit tests, this inline scan in a 599-line component has none. — Failure scenario: every history change pays two passes deriving the same thought-group structure, and the head-classification rule must change in two places in lockstep when the thought item taxonomy evolves.
Suggested fix: fold the value into the existing pass — e.g. have buildThoughtHeadIdMap return { map, lastHeadId }, or add a findLastThoughtHeadId(items) sibling next to findLastUserItemIndex in historyUtils.ts (with unit tests) — and consume it here instead of the inline memo.
中文说明
[Suggestion] 新增的 lastCommittedThoughtHeadId 反向扫描重复了既有 buildThoughtHeadIdMap(visibleHistory) 遍历(上方两行以相同 visibleHistory 依赖 memo 化):正向遍历见到的最后一个 gemini_thought head 正是该反向扫描重新计算的值。“什么算 thought head”的规则(item.type === 'gemini_thought')因此同时存在于 historyUtils.ts 与本组件两处;historyUtils.ts 中两种形态(buildThoughtHeadIdMap 与反向的 findLastUserItemIndex)均有单元测试,而 599 行组件内的这段内联扫描没有任何测试。— 失败场景:每次 history 变更都要付出两次推导同一 thought-group 结构的遍历;当 thought 条目分类演进时,head 判定规则必须在两处同步修改。
建议修复:将该值并入既有遍历——让 buildThoughtHeadIdMap 返回 { map, lastHeadId },或在 historyUtils.ts 中 findLastUserItemIndex 旁新增 findLastThoughtHeadId(items)(并补单元测试),此处改为消费它。
— qwen3.8-max via Qwen Code /review (v0.21.5)
| const clickable = | ||
| useVirtualViewport(settings.merged.ui?.useTerminalBuffer) && | ||
| mouseTrackingEnabled; | ||
| const isActive = !isPending; | ||
| mouseTrackingEnabled && | ||
| !fullDetail; |
There was a problem hiding this comment.
[Suggestion] The forced-open click guard added here only covers fullDetail; its sibling forced-expansion input — the thoughtExpanded prop — is not covered. SessionPreview.tsx renders every thought with thoughtExpanded={true} and no fullDetail, inside AppContainer's real ThoughtExpandedProvider and VirtualViewportContext, so those thoughts stay clickable and the collapse hint this diff introduced still advertises "(click or ctrl+o to collapse)". — Failure scenario: probe-verified with a SessionPreview-exact render (VP mode + default-on mouse tracking): a full press+release fires toggle on the main session's shared expandedThoughtHeadIds set, but resolvedThoughtExpanded short-circuits on the forced prop, so the advertised click can never collapse — it only mutates state the preview never reflects. The pre-diff collapse hint was keyboard-only, so this dead-click advertisement is introduced by this diff; fullDetail got exactly this guard, its forced-open twin did not.
Suggested fix: treat prop-forced expansion like fullDetail — e.g. pass fullDetail={fullDetail || thoughtExpanded === true} to ClickableThinkMessage (or compute a forcedOpen value used for both the hint and the subscription), so SessionPreview gets the keyboard-only hint and a disarmed handler.
中文说明
[Suggestion] 此处新增的强制展开点击守卫只覆盖 fullDetail;其孪生强制展开输入——thoughtExpanded prop——未被覆盖。SessionPreview.tsx 以 thoughtExpanded={true} 且无 fullDetail 渲染每个思考块,并位于 AppContainer 真实的 ThoughtExpandedProvider 与 VirtualViewportContext 之内,因此这些思考块仍可点击,且本 diff 新引入的收起提示仍宣传 “(click or ctrl+o to collapse)”。— 失败场景:已用 SessionPreview 原样渲染探针验证(VP 模式 + 默认开启的鼠标跟踪):完整按下+释放会在主会话共享的 expandedThoughtHeadIds 集合上触发 toggle,但 resolvedThoughtExpanded 被强制 prop 短路,广告出来的点击永远无法真正收起——只会变更预览从不反映的状态。diff 之前收起提示仅提示键盘操作,因此“死点击”广告由本 diff 引入;fullDetail 恰好拿到了这个守卫,其强制展开孪生面没有。
建议修复:将 prop 强制展开与 fullDetail 同等对待——例如向 ClickableThinkMessage 传 fullDetail={fullDetail || thoughtExpanded === true}(或计算 forcedOpen 后同时用于提示与订阅),使 SessionPreview 得到仅键盘的提示并解除点击处理。
— qwen3.8-max via Qwen Code /review (v0.21.5)
| const lastCommittedThoughtHeadId = useMemo(() => { | ||
| for (let i = visibleHistory.length - 1; i >= 0; i--) { | ||
| const item = visibleHistory[i]; | ||
| if (item && item.type === 'gemini_thought') return item.id; |
There was a problem hiding this comment.
[Suggestion] No test covers the double-split state — a committed gemini_thought_content tail in history alongside a still-pending tail — so this head-only scan is unpinned there; every existing keying test populates history with gemini_thought heads only. — Failure scenario: mutation-verified: a one-line mutant accepting 'gemini_thought_content' in this scan survives all 31 existing MainContent.test.tsx tests, and a double-split probe then fails expected 43 to be 42 — the pending tail keys off the committed tail id instead of the head, so expanding the head (toggle 42) no longer expands the streaming tail. A thought exceeding 2×STREAM_PENDING_ITEM_MAX_CHARS reaches this state in production.
Suggested fix: add a case to the pending thought expansion keying describe (it runs for both vp variants automatically): history [{ id: 42, type: 'gemini_thought' }, { id: 43, type: 'gemini_thought_content' }] + pendingHistoryItems: [{ type: 'gemini_thought_content', … }] → expect(pendingTail.thoughtHeadId).toBe(42).
中文说明
[Suggestion] 没有测试覆盖“双切分”状态——history 中同时存在已提交的 gemini_thought_content 尾块与仍 pending 的尾块,因此这段只认 head 的扫描在该状态下无测试钉住;现有所有 keying 测试的 history 只包含 gemini_thought head。— 失败场景:已做变异验证:让该扫描接受 'gemini_thought_content' 的单行变异可通过全部 31 个既有 MainContent.test.tsx 测试;双切分状态探针随即报 expected 43 to be 42——pending 尾块被键到已提交尾块 id 而非 head id,展开 head(toggle 42)不再展开流式尾块。超过 2×STREAM_PENDING_ITEM_MAX_CHARS 的思考在生产环境即进入该状态。
建议修复:在 pending thought expansion keying describe 中新增用例(describe.each 会自动覆盖两个 vp 变体):history [{ id: 42, type: 'gemini_thought' }, { id: 43, type: 'gemini_thought_content' }] + pendingHistoryItems: [{ type: 'gemini_thought_content', … }] → expect(pendingTail.thoughtHeadId).toBe(42)。
— qwen3.8-max via Qwen Code /review (v0.21.5)
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline. 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.5)
| // Migrate a streamed thought's provisional pending expansion to its real | ||
| // committed head id when it commits (or drop it when passed `null`). See | ||
| // settlePendingThoughtExpansion in AppContainer. | ||
| settlePendingThoughtExpansion?: (committedHeadId: number | null) => void, |
There was a problem hiding this comment.
[Suggestion] R7-1: The new settlePendingThoughtExpansion callback is appended as the 26th positional parameter of useGeminiStream, perpetuating a parameter list whose cost is already visible in the test harness rather than being generalised into a trailing options object. — Failure scenario: no runtime defect today (the AppContainer call site and the hook signature agree), but the concrete cost is the one this diff itself pays: useGeminiStream.test.tsx must reference parameter types by numeric index (Parameters<typeof useGeminiStream>[15], [20], [24], and now [25]) and pad the call with positional undefined placeholders. The next callback threaded through this hook repeats that dance, and adjacent optional params with identical types (slots 22/23, both React.RefObject<number>) are exactly the shape where an inserted/misordered argument binds to the wrong slot without a type error. Suggested fix: fold the trailing optional refs/callbacks into a single trailing options object so additions are name-keyed (e.g. opts?: { settlePendingThoughtExpansion?: ...; goalQueueRef?: ...; ... }); at minimum, do not append a parameter 27 positionally. No change is required inside this PR.
中文说明
[建议] 新增的 settlePendingThoughtExpansion 回调作为第 26 个位置参数追加到 useGeminiStream,延续了这套参数列表——其代价在测试脚手架中已清晰可见——而不是收敛为一个尾置 options 对象。— 失败场景:当前没有运行时缺陷(AppContainer 调用点与 hook 签名一致),但具体代价是本 diff 自身正在支付的:useGeminiStream.test.tsx 必须按数字下标引用参数类型(Parameters<typeof useGeminiStream>[15]、[20]、[24],现在是 [25]),并用位置 undefined 占位符补齐调用。下一个接入该 hook 的回调会重复这套操作;而相邻的同类型可选参数(第 22/23 位,均为 React.RefObject<number>)正是那种插入/错位的参数会绑到错误槽位且不产生类型错误的形状。建议修复:将尾部的可选 ref/回调收敛进单个尾置 options 对象,使新增项按名称键控(如 opts?: { settlePendingThoughtExpansion?: ...; goalQueueRef?: ...; ... });至少不要再以位置参数形式追加第 27 个参数。本 PR 内无需改动。
— qwen3.8-max via Qwen Code /review (v0.21.5)
There was a problem hiding this comment.
Declined for this PR. The finding itself states no change is required here, and folding ~26 positional parameters into a trailing options object is a standalone refactor of the hook signature, the AppContainer call site, and the test harness's Parameters<typeof useGeminiStream>[N] references — out of scope for this late-round bugfix. The new parameter follows the existing positional pattern, and the call site and signature were updated together (typecheck passes), so no slot mismatch is introduced; the options-object migration belongs in a follow-up refactor.
中文说明
本 PR 内拒绝处理。该 finding 本身已声明本 PR 内无需改动;将约 26 个位置参数收敛为尾置 options 对象是一次独立重构,涉及 hook 签名、AppContainer 调用点以及测试脚手架中的 Parameters<typeof useGeminiStream>[N] 引用——超出本轮后期 bugfix 的范围。新增参数沿用既有位置参数模式,且调用点与签名同步更新(typecheck 通过),不会引入槽位错位;options 对象迁移应放到后续独立重构中完成。
| const next = new Set(prev); | ||
| next.delete(PENDING_THOUGHT_HEAD_ID); | ||
| if (committedHeadId != null) next.add(committedHeadId); |
There was a problem hiding this comment.
[Suggestion] R7-2: This reducer's set-update logic — the mechanism that keeps a click-expanded streaming thought expanded after it commits — has zero test coverage at any layer. A harness-validated test-efficacy probe found a surviving mutant: deleting next.delete(PENDING_THOUGHT_HEAD_ID); leaves every test green. Every test layer stubs the other side: the useGeminiStream tests pass a vi.fn() spy (pinning only invocation), the MainContent/HistoryItemDisplay tests inject static ThoughtExpandedProvider values, and AppContainer.test.tsx mocks useGeminiStream entirely. This is the still-uncovered half of the round-2 thread on this reducer: the call-site spy tests were added in later rounds, but the reducer unit test proposed in the same thread still does not exist. — Failure scenario: with the .delete() removed (e.g. a future refactor "simplifying" the migration), the -1 sentinel persists in expandedThoughtHeadIds after a thought commits, so the next streaming thought — keyed by the sentinel while pending — renders already expanded with no user action; deleting next.add(committedHeadId) ships the round-1 regression (a thought the user clicked open during streaming snaps collapsed the moment it commits) equally green. The harness confirmed the mutant survives with the full suite green, so either regression would ship silently. Suggested fix: extract the set-update into a small pure function and unit-test its three branches (no sentinel → returns prev unchanged; settle(id) → sentinel removed and id added; settle(null) → sentinel dropped):
export function settlePendingExpansion(
prev: ReadonlySet<number>,
committedHeadId: number | null,
): ReadonlySet<number> {
if (!prev.has(PENDING_THOUGHT_HEAD_ID)) return prev;
const next = new Set(prev);
next.delete(PENDING_THOUGHT_HEAD_ID);
if (committedHeadId != null) next.add(committedHeadId);
return next;
}中文说明
[建议] 该 reducer 的集合更新逻辑——让流式期间被点击展开的思考块在提交后保持展开的机制——在任何层级都没有测试覆盖。经 harness 校验的测试有效性探针发现存活变异体:删除 next.delete(PENDING_THOUGHT_HEAD_ID); 后所有测试仍全绿。每一层测试都把另一侧 stub 掉了:useGeminiStream 测试传入 vi.fn() 间谍(只钉住调用本身)、MainContent/HistoryItemDisplay 测试注入静态 ThoughtExpandedProvider 值、AppContainer.test.tsx 则整体 mock 掉 useGeminiStream。这是第 2 轮关于该 reducer 的评论中仍未覆盖的一半:调用点间谍测试已在后续轮次补上,但同一评论中提议的 reducer 单元测试至今不存在。— 失败场景:若删除该 .delete()(例如未来某次“简化”该迁移的重构),-1 哨兵会在思考提交后残留在 expandedThoughtHeadIds 中,于是下一个流式思考块——pending 期间以哨兵为键——会在用户未做任何操作时直接以展开状态渲染;而删除 next.add(committedHeadId) 则会以同样全绿的结果放行第 1 轮的回归(用户在流式期间点击展开的思考块在提交瞬间立刻折叠)。harness 已确认该变异体在整个套件全绿的情况下存活,因此两种回归都会悄无声息地进入发布。建议修复:将集合更新抽取为一个小型纯函数并单测其三个分支(无哨兵 → 原样返回 prev;settle(id) → 移除哨兵并加入 id;settle(null) → 丢弃哨兵),代码形状见上方英文部分。
— qwen3.8-max via Qwen Code /review (v0.21.5)
|
🤖 Addressed the latest review feedback (round 7/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 7/100 轮)。改动内容与我反驳保留之处如下: Round-7 review responseFeedback dispositionsR7-1 — R7-2 — reducer set-update logic has zero test coverage; surviving mutant (Suggestion) — Addressed.
The extraction also shrinks AppContainer: the inline updater and its comment block were removed, since the pure function's doc comment carries the same "why". Mutant check: removing Conflict notesNone — Verification
中文说明第 7 轮审查回应反馈处理结论R7-1 —— R7-2 —— reducer 集合更新逻辑零测试覆盖;存在存活变异体(建议) —— 已处理。
此次抽取还让 AppContainer 变小:内联 updater 及其注释块被移除,因为纯函数的文档注释已承载同样的"为什么"。 变异体检验:删除 冲突说明无 —— 验证
Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审;第 100 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline. 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.5)
| export function settlePendingExpansion( | ||
| prev: ReadonlySet<number>, | ||
| committedHeadId: number | null, | ||
| ): ReadonlySet<number> { |
There was a problem hiding this comment.
[Suggestion] R8-1: Click-vs-commit race — a press→release that straddles the thought's commit is a dead click (or a wrong-key toggle). On commit the thought moves between render sites with different keys: the VP key extractor yields p-${-item.id - 1} for pending items vs h-${item.id} for committed items (MainContent.tsx:109-114), and the non-VP path renders committed items in <Static> vs pending items in a separate live region. The pending ClickableThinkMessage therefore unmounts and a fresh committed instance mounts with pressRef.current === null. Any commit landing inside the ~50–150 ms press→release interval kills the click: the release either fires no toggle at all (the press record is lost in the remount), or — via the pre-commit subscriber — fires toggle(PENDING_THOUGHT_HEAD_ID) after settlePendingExpansion already migrated the sentinel: the collapse direction leaves {headId, sentinel}, so the thought stays expanded despite the click; the expand direction leaves a stray sentinel, so the committed thought renders collapsed despite the click.
Failure scenario: in VP mode with mouse tracking, the user presses on a streaming thought's line and the thought commits before release (the window sits exactly on the thinking→answer transition) → the click does nothing, or toggles the stale sentinel, and the expansion state ends up opposite to (or unchanged by) the user's intent. Probe-verified with flip checks: a commit-before-press control toggles the committed id and the frames flip correctly; reverting to pre-PR isActive: clickable && !isPending removes the armed surface. Introduced by this diff — pre-PR, pending thoughts never subscribed. The cost is an occasional dead/misfired click at the boundary; re-click or keyboard recovers, and a stray sentinel self-cleans at the next thought start's settle(null).
Suggested fix: allocate the thought head's history id when the thought starts and use it as the expansion key (and item key) through pending→committed — stable identity means no migration and the pressRef survives the transition; alternatively, document/accept the dead click.
中文说明
[建议] R8-1:点击与提交的竞态——跨越思考块提交时刻的"按下→释放"点击会成为死点击(或切换到错误的键)。提交时思考块在不同渲染位置之间迁移且键不同:VP 键提取器对 pending 项生成 p-${-item.id - 1}、对已提交项生成 h-${item.id}(MainContent.tsx:109-114);非 VP 路径中已提交项渲染于 <Static>、pending 项渲染于独立的实时区域。因此 pending 的 ClickableThinkMessage 会卸载,新挂载的已提交实例 pressRef.current === null。任何落在约 50–150 ms 按下→释放区间内的提交都会使点击失效:释放事件要么完全不触发 toggle(press 记录随重挂载丢失),要么经由提交前的订阅者在 settlePendingExpansion 已迁移哨兵之后触发 toggle(PENDING_THOUGHT_HEAD_ID):收起方向残留 {headId, sentinel},点击后思考块仍保持展开;展开方向残留孤立哨兵,已提交的思考块反而渲染为收起。
失败场景:VP 模式 + 鼠标跟踪下,用户在流式思考块上按下鼠标,思考块在释放前提交(窗口恰在思考→回答的转换点)→ 点击无效或切换了过期哨兵,展开状态与用户意图相反(或毫无变化)。已通过探针验证并做翻转对照:提交先于按下的对照组可正确切换已提交 id、画面正常翻转;回退到 PR 前的 isActive: clickable && !isPending 则解除点击武装。该竞态由本 diff 引入——PR 前 pending 思考块从不订阅鼠标事件。代价为边界处偶发的失效/误触发点击;重新点击或键盘操作可恢复,孤立哨兵会在下一思考块开始时的 settle(null) 中自动清理。
建议修复:在思考块开始时预分配其历史 id,并在 pending→committed 全程以该 id 作为展开键(与列表项键)——身份稳定后无需迁移,pressRef 也能跨越转换存活;或者在文档中说明/接受该失效点击。
— qwen3.8-max via Qwen Code /review (v0.21.5)
|
🤖 AutoFix ran out of time before finishing (timeout (1080000ms)) (attempt 8/100) — it will retry on the next scan. What I found before stopping: See the Qwen Autofix agent step logs for model/tool output. Run log: https://github.com/QwenLM/qwen-code/actions/runs/30978469269 🧠 Handled by Qwen Code · model/模型 |
|
🤖 Addressed the latest review feedback (round 9/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 9/100 轮)。改动内容与我反驳保留之处如下: Round 8 review response — PR #8443Feedback points and dispositionsR8-1 (rc:3718059310) — [Suggestion] Click-vs-commit race on pending thought expansion → accepted and documented (the reviewer's explicit fallback) The finding is real and was probe-verified: a press→release that straddles a thought's commit can dead-click or toggle the stale sentinel, because the pending Two code-level facts guided the disposition:
The reviewer explicitly offered "alternatively, document/accept the dead click". We take that option: the Conflict: none ( Verification
中文说明第 8 轮审查回应 — PR #8443反馈点与处置R8-1 (rc:3718059310) — [建议] pending 思考块展开的点击与提交竞态 → 接受并文档化(审查者明确给出的备选方案) 该发现属实且已经过探针验证:跨越思考块提交时刻的"按下→释放"点击会成为死点击或切换到过期哨兵,因为 pending 的 两个代码层面的事实决定了本次处置:
审查者明确给出了"或者在文档中说明/接受该失效点击"的备选方案。我们采纳该方案: 冲突:无( 验证
Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审;第 100 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline. Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally. Test Plan (not a blocker): 57 tests pass — this review observed 17138 passed.
中文说明
已审查。 建议见行内评论。 未审查:build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。 Test Plan(非阻断):57 tests pass — this review observed 17138 passed。
— qwen3.8-max via Qwen Code /review (v0.21.5)
| const settlePendingThoughtExpansion = useCallback( | ||
| (committedHeadId: number | null) => { | ||
| setExpandedThoughtHeadIds((prev) => |
There was a problem hiding this comment.
[Suggestion] R9-1: The reducer glue settlePendingThoughtExpansion — the mechanism that keeps a click-expanded streaming thought expanded after it commits — is exercised by no test at any layer. Each half is tested in isolation (the pure settlePendingExpansion in ThoughtExpandedContext.test.tsx; the hook's settle call timing in useGeminiStream.test.tsx via an injected spy), but nothing wires this real callback to the real state setter. — Failure scenario: probe-verified — the mutation settlePendingExpansion(prev, committedHeadId) → settlePendingExpansion(prev, null) compiles cleanly and passes all 453 tests across the 7 relevant suites; shipped, it would make every thought the user clicked open during streaming collapse at commit — the exact regression this migration exists to prevent. The sibling mutation settlePendingExpansion(new Set(), committedHeadId) also survives (407/407) and would wipe every other expanded thought on each commit.
Suggested fix: add one integration test over this wiring — with the real provider state, record an expansion under PENDING_THOUGHT_HEAD_ID, invoke this callback with a committed head id, and assert the committed thought renders expanded while other set entries survive.
中文说明
[建议] R9-1:reducer 胶水 settlePendingThoughtExpansion —— 让流式期间点击展开的思考块在提交后保持展开的机制 —— 没有任何层级的测试覆盖。两半被隔离测试(ThoughtExpandedContext.test.tsx 中的纯函数 settlePendingExpansion;useGeminiStream.test.tsx 中通过注入 spy 测试 hook 的 settle 调用时机),但没有任何测试把这个真实回调与真实状态 setter 连接起来。— 失败场景:已探针验证 —— 变异 settlePendingExpansion(prev, committedHeadId) → settlePendingExpansion(prev, null) 可编译并通过全部 7 个相关套件的 453 个测试;若带病上线,用户在流式期间点击展开的每个思考块都会在提交时折叠 —— 正是本迁移机制要防止的回归。兄弟变异 settlePendingExpansion(new Set(), committedHeadId) 同样存活(407/407),会在每次提交时清空所有其他已展开的思考块。
建议修复:为此接线添加一个集成测试 —— 使用真实 provider 状态,在 PENDING_THOUGHT_HEAD_ID 下记录一次展开,用已提交的 head id 调用此回调,并断言已提交的思考块渲染为展开状态且集合中其他条目保留。
— qwen3.8-max via Qwen Code /review (v0.21.5)
| // A fresh thought begins; drop any stale provisional expansion key | ||
| // left over from a pending thought that never committed. | ||
| settlePendingThoughtExpansion?.(null); |
There was a problem hiding this comment.
[Suggestion] R9-2: The if (startingNewThought) guard around this settle(null) call is pinned by no test — all three new settle tests stream exactly one Thought event per turn, so no test exercises a continuation Thought event with a settle spy attached. — Failure scenario: probe-verified — moving settlePendingThoughtExpansion?.(null) outside the guard passes all 186 useGeminiStream tests unchanged; a flip probe streaming two description-carrying Thought events showed settle calls [null, null, 1001] with the mutation vs [null, 1001] with the guard. Shipped without the guard: the model streams a continuation Thought chunk for the same thought while the user has it expanded → the sentinel is dropped mid-stream → the thought block collapses live while being read.
Suggested fix: add a test yielding two ServerGeminiEventType.Thought events for one thought and assert settle fires with null exactly once (first event only), then with the committed id at commit — e.g. expect(settle).toHaveBeenNthCalledWith(1, null) and expect(settle).toHaveBeenCalledTimes(2) after the commit.
中文说明
[建议] R9-2:此 settle(null) 调用外层的 if (startingNewThought) 守卫没有任何测试钉住 —— 三个新增 settle 测试均只流式输出一个 Thought 事件,因此没有测试在挂载 settle spy 的情况下执行续传 Thought 事件。— 失败场景:已探针验证 —— 把 settlePendingThoughtExpansion?.(null) 移出守卫后,全部 186 个 useGeminiStream 测试原样通过;翻转探针流式输出两个带 description 的 Thought 事件,变异下 settle 调用为 [null, null, 1001],守卫存在时为 [null, 1001]。若带病上线:模型为同一思考块续流一个 Thought 分块而用户已将其展开 → 哨兵在流式中途被丢弃 → 思考块在用户阅读时实时折叠。
建议修复:添加一个为一个思考块产出两个 ServerGeminiEventType.Thought 事件的测试,并断言 settle 仅以 null 触发一次(仅首个事件),随后在提交时以已提交 id 触发 —— 例如 expect(settle).toHaveBeenNthCalledWith(1, null),且提交后 expect(settle).toHaveBeenCalledTimes(2)。
— qwen3.8-max via Qwen Code /review (v0.21.5)
| // fullDetail and `thoughtExpanded === true` both pin the thought open, so | ||
| // a click could never collapse it — ClickableThinkMessage disarms itself. | ||
| const forcedOpen = fullDetail || thoughtExpanded === true; |
There was a problem hiding this comment.
[Suggestion] R9-4: forcedOpen misses the allExpanded (Ctrl+O) case on surfaces that don't forward it as fullDetail — specifically AgentChatContent, which passes neither fullDetail nor thoughtExpanded while agentHistoryAdapter.ts emits gemini_thought items. Ctrl+O force-expands those thoughts via context, but clickable stays true. — Failure scenario: probe-verified — in VP mode with mouse tracking, press Ctrl+O on an agent-view transcript: the handler stays armed and the header advertises "(click or ctrl+o to collapse)" (pre-PR the collapse hint was keyboard-only, so this false advertising is introduced by this diff). Clicks can never collapse the row while allExpanded dominates; they silently flip expandedHeadIds, and the accumulated invisible toggles surface when Ctrl+O turns off — state opposite to the user's repeated collapse clicks. This also breaks the invariant the added comment states ("a forced-open thought … can never collapse on click"). Probe flips with the fix below: disarmed handler, keyboard-only hint.
| // fullDetail and `thoughtExpanded === true` both pin the thought open, so | |
| // a click could never collapse it — ClickableThinkMessage disarms itself. | |
| const forcedOpen = fullDetail || thoughtExpanded === true; | |
| // fullDetail, Ctrl+O full-detail (`allExpanded`), and `thoughtExpanded === | |
| // true` all pin the thought open, so a click could never collapse it — | |
| // ClickableThinkMessage disarms itself. | |
| const forcedOpen = fullDetail || allExpanded || thoughtExpanded === true; |
(or have AgentChatContent forward fullDetail={allExpanded} the way MainContent does)
中文说明
[建议] R9-4:forcedOpen 遗漏了未把 allExpanded(Ctrl+O)以 fullDetail 形式转发的渲染面 —— 具体是 AgentChatContent:它既不传 fullDetail 也不传 thoughtExpanded,而 agentHistoryAdapter.ts 会产出 gemini_thought 项。Ctrl+O 通过 context 强制展开这些思考块,但 clickable 仍为 true。— 失败场景:已探针验证 —— VP 模式 + 鼠标跟踪下,在 agent 视图会话上按 Ctrl+O:处理器保持武装,头部显示 "(click or ctrl+o to collapse)"(PR 前收起提示仅为键盘措辞,因此这一虚假宣传由本 diff 引入)。在 allExpanded 主导期间点击永远无法收起该行;点击只会静默翻转 expandedHeadIds,累积的不可见切换会在 Ctrl+O 关闭时浮现 —— 状态与用户反复点击"收起"的意图相反。这同时违反了新增注释所声明的不变量("强制展开的思考块不可能被点击收起")。应用下方修复后探针翻转:处理器解除武装、提示变为纯键盘措辞。
(或者让 AgentChatContent 像 MainContent 一样转发 fullDetail={allExpanded})
— qwen3.8-max via Qwen Code /review (v0.21.5)
Review — PR #8443 (head
|
|
🤖 Addressed the latest review feedback (round 10/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 10/100 轮)。改动内容与我反驳保留之处如下: Round-9 review feedback — all three findings addressedCommit: R9-1 — settle wiring in AppContainer has no test (rc:3720331687) — AddressedAdded a Approach note: the suggested integration test was attempted first — real provider state, record an expansion under
Probe-verified: both reviewer mutations — R9-2 —
|
|
📊 Takeover milestone — round 10/100, in the current window. Census: 9 pushed fix(es), 0 no-change review(s), 1 timeout(s), 0 rejected attempt(s), 0 other round(s) (crash / model error / gate error / infra), 1 base update(s). This many rounds deserves a human look. Options: keep going (fine — nothing changes), split or reduce the PR if rounds keep accumulating, or release takeover (remove the 中文说明📊 接管里程碑 —— 第 10/100 轮(当前窗口)。统计:推送修复 9 次、审阅无需改动 0 次、超时 1 次、验证拒绝 0 次、其他轮次(崩溃/模型错误/门错误/infra)0 次、base 更新 1 次。 轮次到这个量值得人工看一眼。可选:继续(无需操作);若轮次持续累积,考虑拆分或缩减 PR;或释放接管(移除 |
|
🔀 Base updated: red check(s) [Test (ubuntu-latest, Node 22.x)] pass on current main — merged current main via update-branch; CI will re-run. 中文说明🔀 已更新 base:红色检查 [Test (ubuntu-latest, Node 22.x)] 在当前 main 上通过 —— 已通过 update-branch 合入当前 main,CI 将重新运行。 |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally. Test Plan (not a blocker): 57 tests pass — this review observed 17680 passed.
中文说明
已审查。 未审查:build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。 Test Plan(非阻断):57 tests pass — this review observed 17680 passed。
— qwen3.8-max via Qwen Code /review (v0.21.5)
|
🤖 Addressed the latest review feedback (round 11/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 11/100 轮)。改动内容与我反驳保留之处如下: Round summary — PR #8443 (addressing @wenshao's review of head
|
|
🤖 AutoFix ran out of time before finishing (timeout (1080000ms)) (attempt 12/100) — it will retry on the next scan. What I found before stopping: See the Qwen Autofix agent step logs for model/tool output. Run log: https://github.com/QwenLM/qwen-code/actions/runs/31029280757 🧠 Handled by Qwen Code · model/模型 |
|
🤖 AutoFix stopped: this counting window now contains 3 time-budget exhaustions (pushed rounds in between included; this round itself may have failed differently). That is 3 full agent runs that pushed nothing. A human should split or reduce the PR (or raise the agent time budget AND its step backstop together), then comment What I found before stopping: See the Qwen Autofix agent step logs for model/tool output. Run log: https://github.com/QwenLM/qwen-code/actions/runs/31037527888 🧠 Handled by Qwen Code · model/模型 |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Test Plan (not a blocker): 57 tests pass — this review observed 17685 passed.
中文说明
未审查:build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
Test Plan(非阻断):57 tests pass — this review observed 17685 passed。
— qwen3.8-max via Qwen Code /review (v0.21.6)
| if (item.type === 'gemini_thought') { | ||
| settlePendingThoughtExpansion?.(committedId); | ||
| } |
There was a problem hiding this comment.
[Critical] R11-1: The settle wiring assumes a single stream owns the hook's pending-thought slot, but the legacy concurrent ?btw path (allowConcurrentBtwDuringResponse — a supported, test-pinned path) runs a second stream through the same processGeminiStreamEvents/handleThoughtEvent/commitPendingThought handlers with the shared pendingThoughtItemRef and this settle callback, so a side-question stream can drop or mis-migrate the provisional expansion this PR introduces. (The sibling settle(null) call site at ~line 1720 is where the drop fires; both call sites are affected.)
Failure scenario: user in VP mouse mode click-expands a streaming thought (sentinel recorded). While it streams they submit ?btw .... Probe A — the btw answer starts with a Thought event: the btw stream's startingNewThought fires settlePendingThoughtExpansion?.(null) mid-flight, deleting the user's sentinel; when the main thought later commits, settle(committedId) finds nothing and the thought the user expanded commits collapsed — the exact regression this PR exists to prevent. Probe B — the btw answer starts with Content: the shared-slot commitPendingThought commits the main stream's partial head early and migrates the sentinel onto it; the remainder commits as a second head, stranding the expansion on the partial head. Probe-verified via settle-spy plus a walk through the real settlePendingExpansion reducer; a flip run with these two settle call sites reverted recorded zero settle calls, so the interference is introduced by this wiring.
Suggested fix: make settlement stream-owned — thread a per-stream ownership flag into processGeminiStreamEvents/handleThoughtEvent/addItemAndSettleThoughtHead and skip settlePendingThoughtExpansion on any stream that is not the primary turn.
中文说明
settle 接线假定只有一条流持有该 hook 的 pending-thought 槽位,但遗留的 ?btw 并发路径(allowConcurrentBtwDuringResponse —— 受支持且有测试固定的路径)会让第二条流复用相同的 processGeminiStreamEvents/handleThoughtEvent/commitPendingThought 处理器,共享 pendingThoughtItemRef 与该 settle 回调,因此旁路提问流可能丢弃或错误迁移本 PR 引入的临时展开状态。(另一个 settle(null) 调用点位于约 1720 行,是丢弃实际发生的位置;两个调用点都受影响。)
失败场景:用户在 VP 鼠标模式下点击展开正在流式输出的思考块(记录 sentinel)。流式进行中提交 ?btw ...。探针 A —— btw 回答以 Thought 事件开始:btw 流的 startingNewThought 在中途触发 settlePendingThoughtExpansion?.(null),删除用户的 sentinel;主思考随后提交时 settle(committedId) 找不到记录,用户展开的思考块在提交瞬间变为折叠 —— 正是本 PR 要防止的回归。探针 B —— btw 回答以 Content 开始:共享槽位的 commitPendingThought 提前提交主流的部分头部并把 sentinel 迁移到其上;剩余部分作为第二个头部提交,展开状态被搁浅在部分头部上。已通过 settle spy 加真实 settlePendingExpansion reducer 推演验证;将这两处 settle 调用点回退的对照运行未记录到任何 settle 调用,证明该干扰由本接线引入。
建议修复:让 settlement 按流隔离 —— 将每流所有权标志传入 processGeminiStreamEvents/handleThoughtEvent/addItemAndSettleThoughtHead,非主回合流跳过 settlePendingThoughtExpansion。
— qwen3.8-max via Qwen Code /review (v0.21.6)
| expect(settlePendingThoughtExpansion).toBeTypeOf('function'); | ||
| expect( | ||
| ( | ||
| settlePendingThoughtExpansion as (...args: unknown[]) => unknown | ||
| ).toString(), | ||
| ).toMatch( |
There was a problem hiding this comment.
[Suggestion] R11-2: This wiring test asserts on Function.prototype.toString() source text — the regex pins the exact identifier names (prev, committedHeadId, settlePendingExpansion) and the ESM-interop shape, and the callback is located via the last positional slot (mockedUseGeminiStream.mock.calls.at(-1)?.at(-1)) — so it pins the mechanism's source text, not its behavior.
Concrete cost: a behavior-neutral edit — renaming committedHeadId in AppContainer's settle callback, extracting the updater into a named helper, or appending a 27th positional parameter to useGeminiStream — fails this test with an opaque regex/position mismatch although production behavior is identical. The regex already carries a (?:\(0,\s*)?(?:[A-Za-z0-9_$]+\.)? carve-out for bundler ESM-interop forms, i.e. a transform difference nearly broke it once.
Suggested fix: pin behaviorally — e.g. export a factory from ThoughtExpandedContext (makeSettlePendingExpansion(setter)), unit-test it with a spy setter, and assert AppContainer passes an instance of it; or at minimum drop the identifier-name pins and assert only the load-bearing call structure.
中文说明
该接线测试对 Function.prototype.toString() 源码文本做断言 —— 正则固定了确切的标识符名(prev、committedHeadId、settlePendingExpansion)与 ESM interop 形态,且通过最后一个位置参数槽位(mockedUseGeminiStream.mock.calls.at(-1)?.at(-1))定位回调 —— 因此固定的是机制的源码文本,而非行为。
具体代价:行为无关的改动 —— 在 AppContainer 的 settle 回调中重命名 committedHeadId、把 updater 提取为具名辅助函数、或给 useGeminiStream 追加第 27 个位置参数 —— 都会让该测试以晦涩的正则/位置不匹配失败,而生产行为完全相同。正则中已有针对打包器 ESM interop 形态的 (?:\(0,\s*)?(?:[A-Za-z0-9_$]+\.)? 豁免项,说明转换差异曾险些使其失败。
建议修复:改为对行为断言 —— 例如从 ThoughtExpandedContext 导出工厂函数(makeSettlePendingExpansion(setter)),用 spy setter 对其单测,并断言 AppContainer 传入的是该工厂的实例;或至少去掉对标识符名的固定,只断言关键的调用结构。
— qwen3.8-max via Qwen Code /review (v0.21.6)
| // closed), so a click could never change it — ClickableThinkMessage | ||
| // disarms itself. `allExpanded` matters on surfaces that don't forward it | ||
| // as `fullDetail` (e.g. AgentChatContent). | ||
| const forcedOpen = fullDetail || allExpanded || thoughtExpanded !== undefined; |
There was a problem hiding this comment.
[Suggestion] R11-3: forcedOpen is a misleading name — it is also true when the thought is pinned closed (thoughtExpanded={false}), so it means "expansion state is pinned", not "forced open". The comment above correctly says the gate pins the state "(open or closed)", and this PR's own new test exercises forcedOpen === true with the thought rendered collapsed.
Concrete cost: today the value only gates clickable, so behavior is correct — but a future change that reuses forcedOpen where the name's meaning applies (e.g. expanded={forcedOpen || ...}, choosing the expanded header icon, or skipping collapse logic) would silently treat pinned-closed thoughts as open.
Suggested fix: rename to a state-neutral identifier such as expansionPinned in HistoryItemDisplayComponent and ClickableThinkMessage (prop declaration, destructure, and the clickable read site):
const expansionPinned = fullDetail || allExpanded || thoughtExpanded !== undefined;中文说明
forcedOpen 命名有误导性 —— 当思考块被固定为收起(thoughtExpanded={false})时它同样为 true,其含义是「展开状态被固定」,而非「强制展开」。上方注释正确地写明该门控将状态固定为「(展开或收起)」,且本 PR 新增的测试也在思考块渲染为收起时验证了 forcedOpen === true。
具体代价:目前该值只门控 clickable,行为正确 —— 但未来若在名称含义适用的地方复用 forcedOpen(例如 expanded={forcedOpen || ...}、选择展开态头部图标、或跳过收起逻辑),会把被固定收起的思考块静默当作展开处理。
建议修复:在 HistoryItemDisplayComponent 与 ClickableThinkMessage 中(prop 声明、解构及 clickable 读取处)重命名为状态中立的标识符,例如 expansionPinned:
const expansionPinned = fullDetail || allExpanded || thoughtExpanded !== undefined;— qwen3.8-max via Qwen Code /review (v0.21.6)
|
⏸️ Takeover paused: this PR reached its round cap (100/100). Comment 中文说明⏸️ 托管已暂停:本 PR 达到轮次上限(100/100)。评论 |
|
🔓 Takeover auto-released: the autofix loop paused on this PR 9 day(s) ago (🤖 AutoFix stopped: this counting window now contains 3 time-budget exhaustions (pushed rounds in between included; this ) and no re-arm followed, so the 中文说明🔓 已自动释放接管:autofix 循环在 9 天前暂停于此 PR(🤖 AutoFix stopped: this counting window now contains 3 time-budget exhaustions (pushed rounds in between included; this ),此后无人重新武装,现移除 |
What this PR does
Previously, the thought (thinking) block could only be expanded or collapsed by mouse click after the model finished thinking. While the response was still streaming (pending state), the click handler was disabled and the expand/collapse hint was hidden. This PR removes that restriction so that users can click to expand a streaming thought to watch the reasoning live, and collapse it again — the same toggle behavior that was already available via the keyboard shortcut.
The collapsed-state hint line is now also shown while pending (with the animated pending icon and elapsed duration), and the collapse hint is shown while pending and expanded. The "click or <key> to expand" wording only includes "click" when the terminal is in virtual-viewport mode where clicking is actually supported.
Why it's needed
When a model is reasoning for a long time, users often want to peek at the thinking content as it streams rather than waiting for completion. The keyboard shortcut already worked during streaming, but mouse users had no way to expand until the thought was committed. Making the block clickable while pending gives a consistent, live-watchable experience.
Reviewer Test Plan
How to verify
Automated coverage: unit tests assert that the click handler stays active while pending, that a full press+release toggles a pending thought, that the expand hint is shown while pending (with/without the "click" wording depending on virtual-viewport mode), and that the collapse hint appears while pending and expanded.
Test command:
cd packages/cli && npx vitest run src/ui/components/HistoryItemDisplay.test.tsx src/ui/components/messages/ConversationMessages.test.tsx— 57 tests pass.Evidence (Before & After)
Before: while streaming, the thought showed "Thinking…" with no expand hint and ignored mouse clicks.
After: while streaming, the thought shows "Thinking…(Ns) (click or A to expand)" and responds to clicks.
Tested on
Environment (optional)
npm run devon macOS; unit tests via vitest.Risk & Scope
Linked Issues
N/A
中文说明
这个 PR 做了什么
此前,思考(thinking)块只有在模型思考完成后才能通过鼠标点击展开/收起。在响应仍在流式输出(pending 状态)时,点击处理被禁用,展开/收起提示也被隐藏。本 PR 移除了该限制,用户可以在思考流式输出过程中点击展开以实时观看推理内容,也可以再次点击收起——与键盘快捷键已有的切换行为一致。
收起状态的提示行现在在 pending 时也会显示(带动画 pending 图标和已用时长),pending 且展开时会显示收起提示。"click or <key> to expand" 的措辞仅在终端处于支持点击的虚拟视口模式时才包含 "click"。
为什么需要
当模型长时间推理时,用户往往希望在流式输出过程中就查看思考内容,而不是等待完成。键盘快捷键在流式输出期间本就能用,但鼠标用户在思考提交前无法展开。让 pending 状态的块可点击,提供了一致的、可实时观看的体验。
审阅者测试计划
如何验证
自动化覆盖:单元测试断言了点击处理在 pending 时保持激活、完整的按下+释放可切换 pending 思考、展开提示在 pending 时显示(根据虚拟视口模式决定是否含 "click" 措辞)、以及 pending 且展开时显示收起提示。
测试命令:
cd packages/cli && npx vitest run src/ui/components/HistoryItemDisplay.test.tsx src/ui/components/messages/ConversationMessages.test.tsx—— 57 个测试通过。证据(前后对比)
之前:流式输出期间,思考块显示 "Thinking…",无展开提示且忽略鼠标点击。
之后:流式输出期间,思考块显示 "Thinking…(Ns) (click or A to expand)" 并响应点击。
测试环境
环境(可选)
macOS 上
npm run dev;单元测试用 vitest。风险与范围
关联 Issue
无