fix(cli): make the non-VP transcript scrollable during multi-agent runs - #6015
Conversation
|
Thanks for the PR, @chiga0! Template is complete ✓ — all required sections present with substantive content. On direction: this fixes a genuine usability regression. Non-VP scrolling breaking during On approach: the scope is tight and well-justified. Two independent root causes, two focused commits, each change traceable to a specific mechanism in ink's rendering pipeline. The Moving on to code review. 🔍 中文说明感谢贡献，@chiga0！ 模板完整 ✓ — 所有必需部分都有实质性内容。 方向：这修复了一个真实的可用性回退。在 方案：范围紧凑且理由充分。两个独立根因，两个聚焦的 commit，每处改动都能追溯到 ink 渲染管道中的具体机制。 进入代码审查 🔍 — Qwen Code · qwen3.7-max |
Code ReviewIndependent proposal (before reading the diff): given the two symptoms — scroll snap-back during parallel-agent runs and wheel hijacking in non-VP — I would have: (1) stopped the inline parallel panel from duplicating running agents that Findings: no blockers. The VP gate in The dedup in The height windowing in Tests are comprehensive: pure-parallel live-phase dedup (all-running → null, mixed → only terminal shown inline, committed → full roster), header tally honesty, height windowing with exact-fit boundary pinning ( TestingUnit tests: 104/104 passed across all 6 affected test files (verified on PR branch in worktree): Typecheck: all 7 packages clean (exit 0). Build: clean (exit 0). Only pre-existing lint warnings in vscode-ide-companion (unrelated). Tmux test (PR branch dev build): The PR branch starts cleanly without runtime errors and responds normally. The actual scroll snap-back symptom requires a live multi-agent run ( 中文说明代码审查独立提案（阅读 diff 前）：根据两个症状——并行 agent 运行时滚动弹回和非 VP 滚轮劫持——我会做两件事：(1) 阻止 inline 并行面板重复 发现： 无阻塞问题。 VP 门控逻辑正确，
测试全面：纯并行 live 阶段去重、header 计数、高度窗口化精确边界（ 测试单元测试： PR 分支 worktree 中 6 个受影响测试文件全部 104/104 通过。 Typecheck： 全部 7 个包干净（exit 0）。 构建： 干净（exit 0）。仅 vscode-ide-companion 中有预存的 lint 警告（无关）。 Tmux 测试（PR 分支 dev build）： PR 分支启动正常，无运行时错误，正常响应。实际滚动弹回症状需要带配置 API key 的真实多 agent 运行（ — Qwen Code · qwen3.7-max |
|
Looking at the whole picture: the PR identifies two distinct, well-understood root causes (ink The code is straightforward and follows project conventions. No over-abstraction, no speculative features, no drive-by refactors. Each edit in the diff is necessary for the stated goal. The Tests are thorough and well-reasoned. The exact-fit boundary test ( The live TUI symptom couldn't be reproduced on this CI (no API key), but the unit tests cover the mechanism thoroughly. A reviewer with a configured environment should do a quick Approving. This is a clean, focused bugfix that solves a real problem without adding complexity the codebase doesn't need. 中文说明纵观全貌：PR 识别了两个独立、理解透彻的根因（ink 代码简洁，遵循项目规范。无过度抽象，无投机性功能，无顺手重构。diff 中的每处编辑都是实现目标所必需的。 测试全面且推理清晰。精确边界测试（ Live TUI 症状无法在此 CI 复现（无 API key），但单元测试全面覆盖了机制。有配置环境的审查者应在短终端中快速运行 批准。这是一个干净、聚焦的 bug 修复，解决了真实问题，没有添加代码库不需要的复杂度。 — Qwen Code · qwen3.7-max |
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. ✅
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
No critical issues found. Two minor suggestions below.
— qwen3.7-max via Qwen Code /review
2a88a31 to
43aca0f
Compare
wenshao
left a comment
wenshao
left a comment
There was a problem hiding this comment.
No review findings. Downgraded from Approve to Comment: CI is still running (1 check pending).
— glm-5.2 via Qwen Code /review
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
[Suggestion] InlineParallelAgentsDisplay.tsx:16 — Module docstring states "Rendered in the committed phase only" but this PR changes ToolGroupMessage to render the component in both phases (live phase with filtered inlineToolCalls + totalAgentCount; committed phase with full list). Consider updating the docstring to reflect dual-phase usage.
— qwen3.7-max via Qwen Code /review
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
— qwen3.7-max via Qwen Code /review
doudouOUC
left a comment
doudouOUC
left a comment
There was a problem hiding this comment.
No issues found. LGTM! ✅
Two well-targeted fixes for non-VP scroll snap-back and wheel hijack. Root-cause analysis is thorough, the height windowing and VP gate logic handle all edge cases correctly, and test coverage is strong (100/100 tests pass across 5 suites). The 2 low-confidence observations (missing singular overflow text test, missing background agent status test) are trivial branches with minimal risk.
— bailian/glm-5.2 via Qwen Code /review
In non-VP mode ink clears the whole terminal (including scrollback) on every
repaint once the non-<Static> live frame exceeds the terminal height (see
ink's shouldClearTerminalForFrame). During a parallel-agent run the
pure-parallel inline panel rendered the UNFILTERED toolCalls, so running
subagents were shown inline AND in LiveAgentPanel below the composer — two
full rosters that push the live frame past the viewport. The per-second
elapsed/token ticks then fire that clear continuously, so scroll-up snaps
straight back to the bottom and flickers.
Route the pure-parallel branch through the same inlineToolCalls hand-off as
every other group: during the live phase render only the agents the panel is
not showing (terminal rows en route to <Static>), keeping the header total
honest via totalAgentCount. Add an availableTerminalHeight backstop to
InlineParallelAgentsDisplay that windows the rows ("+N more") so the panel can
never exceed its height budget.
Refs QwenLM#5798
Generated with AI
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
…crollback Enabling SGR mouse tracking (?1002h) makes the host terminal stop doing native scrollback on the wheel — it hands wheel events to the app (Terminal.app even diverts them to arrow keys, which then drive composer input history and the background-agent panel). That is only acceptable when the app itself owns the wheel: VP mode (ScrollableList) or an alternate-screen modal (ThinkingViewer), neither of which relies on main-screen scrollback. But a collapsed thinking block armed mouse tracking just for click-to-expand, unconditionally, so any non-VP session with thinking blocks could no longer scroll its transcript. Make useMouseEvents gate on VP mode by default: mouse tracking turns on only when `ui.useTerminalBuffer` is set. Surfaces that legitimately own the wheel pass the new `bypassVpGate` option (ScrollableList, ThinkingViewer). The thinking-block click handler does not, so in non-VP it stays dormant and native terminal scrollback is preserved (the block still expands via Alt+T). This is a single chokepoint, so future non-VP mouse subscribers can't reintroduce the regression. Refs QwenLM#5798 Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
…filters Address review feedback on the parallel-agent snap-back fix: - InlineParallelAgentsDisplay: the height backstop kept Math.max(1, budget-2) rows, so at availableTerminalHeight <= 2 it rendered header + indicator + 1 data row (3 lines) and overflowed the very budget meant to cap it, which can re-trigger the shouldClearTerminalForFrame snap-back. Reserve the header and (when overflowing) the indicator first, then window the remaining rows; at a budget of 1 keep only the header (its label still states the total). Total rendered height now never exceeds the budget. Adds budget=1/2 regression tests. - ToolGroupMessage: isPureParallelAgentGroup already guarantees every entry is a subagent, so the isSubagentToolEntry filters on inlineToolCalls and on totalAgentCount were no-ops; use inlineToolCalls and toolCalls.length directly. Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Address /review feedback: - ToolGroupMessage: only forward availableTerminalHeight to InlineParallelAgentsDisplay during the live phase (isPending). The height backstop guards the non-<Static> live frame; once committed the rows live in <Static> with no snap-back risk, and MainContent passes staticAreaMaxItemHeight (>=100) for committed items. Forwarding that let the cap fire on scrollback and could permanently hide completed agents behind a static "+N more". Pass undefined when committed, per the component's documented contract. - InlineParallelAgentsDisplay.test: add a "generous budget" case (10 agents, budget 20) pinning the `rows.length + 1 > budget` boundary so a regression to >= would be caught instead of silently truncating when there is room. - ThinkingViewer.test (new): pin that the modal wheel handler subscribes WITH bypassVpGate: true (mirror of the HistoryItemDisplay test that pins the opposite contract), so a future refactor dropping the flag can't silently break wheel scrolling in non-VP mode. Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Address /review test-coverage feedback:
- ToolGroupMessage.test: add a contrast case that renders the same many-agent
group with isPending={true} vs isPending={false} at the same tight
availableTerminalHeight, asserting the overflow indicator appears only in the
live phase and every agent renders in the committed phase. Pins the
load-bearing `isPending ? availableTerminalHeight : undefined` conditional.
- InlineParallelAgentsDisplay.test: add the budget=3 boundary case (the first
budget at which a single data row appears: rowsFit = budget - 2 = 1), so an
off-by-one in the windowing arithmetic is caught at the transition point
between budget=2 (zero rows) and budget=3 (one row).
Generated with AI
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
3b02006 to
0ee8cc2
Compare
…stop relies on Address /review feedback: the windowing math (rowsFit = availableTerminalHeight - 2) counts exactly one terminal line per AgentRow. That invariant — held today by the wrap="truncate-end" Texts in AgentRow — was undocumented, so a future change adding multi-line content (wrapped activity column, progress bar) would silently overflow the budget and re-trigger the shouldClearTerminalForFrame snap-back this PR fixes. Add the invariant at both the AgentRow definition and its render site. Comment-only; no behavior change. Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Address /review feedback: the "generous budget" case used budget=20 with 10 agents, but `11 > 20` and `11 >= 20` are both false, so a regression from `>` to `>=` in `rows.length + 1 > availableTerminalHeight` would go undetected. Use budget=11 (= rows.length + 1) instead: `11 > 11` is false (no windowing) while `11 >= 11` is true (windowing fires a spurious "+1 more"). The not.toContain( 'more agent') assertion catches the flip — the line count coincides at 11 either way, so it is the indicator, not the height, that pins the boundary. Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
|
Qwen Code review did not complete successfully. Qwen review aborted with an API error before posting comments. See workflow logs. |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
No review findings. Downgraded from Approve to Comment: CI still running.
— qwen3.7-max via Qwen Code /review
…e usage Address /review feedback: the module docstring claimed the panel is "Rendered in the committed phase only", but this PR routes it through ToolGroupMessage's inlineToolCalls hand-off so it renders in BOTH phases — live phase showing only terminal agents (running/background owned by LiveAgentPanel) under an availableTerminalHeight windowing cap, committed phase showing the full roster uncapped. Update the docstring to describe the dual-phase behaviour and the height backstop. Comment-only; no behaviour change. Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
|
已处理 review (qwen-code-ci-bot, 15:09) body 中关于 至此该轮自动评审的全部 findings（含此条 review-body 建议）均已落实，CI 实质检查（Test 等）全绿、无未解决 review thread。 |
Address /review [Critical]: the height backstop budgets the header, the optional "+N more" overflow indicator, and each AgentRow as exactly 1 terminal line, but the header Text, the indicator Text, and the AgentRow trailing (elapsed · tokens) Text lacked wrap="truncate-end". On a narrow terminal any of them could wrap to 2+ lines, pushing the rendered frame past availableTerminalHeight and re-firing the shouldClearTerminalForFrame snap-back this PR fixes — and it made the documented "all Text elements use truncate-end" invariant false. Add wrap="truncate-end" to every Text in the panel (header, indicator, glyph, trailing) so the 1-line invariant actually holds. No behaviour change at normal widths (the labels already fit); tightens the degenerate-width guarantee. Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
No review findings. Downgraded from Approve to Comment: CI still running.
— qwen3.7-max via Qwen Code /review
wenshao
left a comment
wenshao
left a comment
There was a problem hiding this comment.
No issues found. LGTM! ✅
Well-designed fix for two independent non-VP scroll regressions — the dedup of running agents through inlineToolCalls and the VP gate on SGR mouse tracking are both clean, focused solutions. Test coverage is thorough (103 tests passing), covering boundary conditions, phase transitions, and the VP-gate mechanism.
— qwen3.7-max via Qwen Code /review
DragonnZhang
left a comment
DragonnZhang
left a comment
There was a problem hiding this comment.
Automated code review — no high-confidence critical findings in the changed code at this commit.
Reviewed InlineParallelAgentsDisplay.tsx height windowing, useMouseEvents.ts VP gate, and ToolGroupMessage.tsx routing. The > strict boundary in rows.length + 1 > availableTerminalHeight correctly handles the exact-fit case. Forwarding availableTerminalHeight only during the live phase (not committed) is load-bearing to preserve full scrollback history. No critical issues found.
Generated by Claude Code
|
@qwen-code /triage |
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. ✅
✅ Maintainer local verification — real binary + real componentsBuilt the real 1. Automated suites — 104/104 greenThe 5 suites in the test plan, plus the new
2. Mutation tests — the regression tests are load-bearingReverting each source file to base while keeping the PR's tests (vitest, runtime):
3. Fix 2 (mouse-tracking VP gate) — real-binary A/B under a PTY 🎯Drove the real TUI under node-pty, made a fake OpenAI endpoint stream
This is exactly the regression and exactly the fix: the only non-VP main-screen mouse subscriber is the collapsed thinking block ( 4. Fix 1 (parallel-agent roster dedup + height backstop) — real-component A/BRendered the real 4a. Dedup — group of Base renders the running agents inline and in 4b. Height backstop — Base's live (non- Methodology notes / honesty
中文版（完整对应）✅ 维护者本地验证 — 真实二进制 + 真实组件从本 PR head（ 1. 自动化测试 — 104/104 全绿测试计划里的 5 个套件 + 新增的
2. 变异测试 — 回归测试确实承重把每个源文件还原到 base、保留 PR 的测试（vitest 运行时）：
3. Fix 2（鼠标追踪 VP 门）— PTY 下的真实二进制 A/B 🎯用 node-pty 驱动真实 TUI，用伪 OpenAI 端点流式返回
这正是回归、也正是修复：非 VP 主屏唯一的鼠标订阅者就是折叠的 thinking 块（ 4. Fix 1（并行 agent 名册去重 + 高度兜底）— 真实组件 A/B在 live 阶段（ 4a. 去重 —— base 把运行中的 agent 在 inline 和下方的 4b. 高度兜底 —— base 的 live（非 方法论说明 / 诚实交代
Verified locally on macOS (Darwin 25.5.0, node v22.22.2) in an isolated worktree at PR head. Raw PTY byte captures + per-suite logs retained. |
Resolve conflict in ToolGroupMessage.tsx: combine this branch's transcript full-detail guard (skip the dense parallel-agent panel so each agent renders its own full ToolMessage) with main's QwenLM#6015 change that routes the pure-parallel-agent group through the shared `inlineToolCalls` hand-off (avoids the non-VP scrollback snap-back, QwenLM#5798). Both now apply: `!fullDetail` gates entry, then the `inlineToolCalls.length === 0` early return from main runs inside. Also drop the stale `CompactModeProvider` wrapper that main's new "thinking-block mouse tracking is VP-gated" test added in HistoryItemDisplay.test.tsx — this branch retired the CompactMode context, so the block now renders via renderWithProviders directly. Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
| // Click-to-expand needs SGR mouse tracking. We do NOT pass `bypassVpGate`, so | ||
| // useMouseEvents enables it only in VP mode; in non-VP the click handler | ||
| // stays dormant and native terminal scrollback is preserved (the block still | ||
| // expands via Alt+T — the "option+t to expand" affordance it already shows). |
There was a problem hiding this comment.
[Suggestion] The comment says "Click-to-expand" and "the block still expands via Alt+T," but the click handler at line 134 calls openThinkingViewer(...) — it opens the modal viewer, not inline expansion. Alt+T (AppContainer.tsx:3290) calls setThoughtExpanded — it toggles inline expansion only. These are distinct features. After the VP gate, the ThinkingViewer modal becomes completely inaccessible in non-VP mode: no keyboard shortcut opens it, and the click handler is dormant. The comment's "the block still expands via Alt+T" is misleading because it implies the click's functionality (modal) is covered by Alt+T (inline), when they are different.
| // expands via Alt+T — the "option+t to expand" affordance it already shows). | |
| // Click-to-open-viewer needs SGR mouse tracking. We do NOT pass `bypassVpGate`, so | |
| // useMouseEvents enables it only in VP mode; in non-VP the click handler | |
| // stays dormant and native terminal scrollback is preserved. The block still | |
| // expands inline via Alt+T, but the full-screen modal viewer is only | |
| // accessible in VP mode (no keyboard shortcut opens it in non-VP). |
— glm-5.2 via Qwen Code /review
5 participants
What this PR does
Fixes two independent root causes that make the non-VP (default) transcript impossible to scroll up during rich runs (e.g.
/review's multi-agent fan-out). The view either snaps straight back to the bottom, or the wheel recalls composer history / drives the background-agent panel instead of scrolling.Two focused commits:
Stop scroll snap-back during parallel-agent runs. Ink clears the whole terminal (incl. scrollback) on every repaint once the non-
<Static>live frame exceeds the terminal height (shouldClearTerminalForFrame). The pure-parallel inline panel rendered the unfiltered tool calls, so running subagents were shown inline and inLiveAgentPanelbelow the composer — two full rosters that push the live frame past the viewport; the per-second elapsed/token ticks then fire the clear continuously. The branch now routes through the sameinlineToolCallshand-off as every other group (running subagents are owned by the panel), with anavailableTerminalHeightwindowing backstop onInlineParallelAgentsDisplay.Keep native scrollback usable in non-VP mode. Enabling SGR mouse tracking (
?1002h) makes the host terminal stop doing native scrollback on the wheel (Terminal.app even diverts the wheel to arrow keys, which then drive composer input history and the panel). A collapsed thinking block armed mouse tracking just for click-to-expand, unconditionally — so any non-VP session with thinking blocks could no longer scroll.useMouseEventsnow gates on VP mode by default (ui.useTerminalBuffer); surfaces that genuinely own the wheel opt out via a newbypassVpGate(ScrollableList, the alternate-screenThinkingViewer). The thinking-block click handler does not, so in non-VP it stays dormant and native scrollback is preserved (the block still expands via Alt+T). Single chokepoint → future non-VP mouse subscribers can't reintroduce the regression.Why it's needed
PR #5799 only corrected the footer-height accounting; it neither de-duplicated the parallel-agent rosters nor stopped mouse tracking from hijacking the wheel, so non-VP scrolling stayed broken during
/review-style runs. Root cause was confirmed against the real (forked) ink renderer: a non-<Static>frame taller than the terminal triggersclearTerminalon every render; below it, ink does an in-placelog-updateand native scrollback survives.Reviewer Test Plan
How to verify
Automated — from the repo root:
Added regression tests: pure-parallel live-phase dedup + header total honesty + windowing backstop;
useMouseEventsVP-gate (off in non-VP, on in VP,bypassVpGateoverrides); thinking-block subscribes withoutbypassVpGate.Manual (the live symptom), short terminal, non-VP: run
/reviewon a PR. Before: scroll-up snaps back / wheel recalls history. After: the transcript scrolls.Evidence (Before & After)
The before/after is encoded programmatically (the live TUI symptom needs a real TTY + API key, which CI lacks):
<Static>frame past the viewport and triggeringclearTerminalevery repaint. The windowing test asserts the inline frame height never exceeds itsavailableTerminalHeightbudget.useMouseEventsVP-gate test asserts mouse tracking is not enabled in non-VP withoutbypassVpGate(native scrollback preserved), is enabled in VP, and thatbypassVpGateoverrides. The thinking-block test pins that it subscribes without the bypass, i.e. stays gated.Tested on
Verified locally on macOS (the 5 suites above, plus tsc-diff = zero new type errors and eslint clean on the changed files). Windows/Linux not run locally — covered by CI.
Risk & Scope
done/total.Linked Issues
Refs #5798
中文说明
修复非 VP（默认）模式下 transcript 无法上滚的两个独立根因（
/review多 agent 场景必现）：上滚被弹回：Ink 在「非
<Static>动态帧 > 终端行数」时每帧clearTerminal（连 scrollback 一起清）。并行 agent 组的 inline 面板用了未过滤的 toolCalls，导致 running 子 agent 在顶部 inline 和底部LiveAgentPanel重复渲染两份名册，把动态帧撑过视口；每秒 tick 持续触发清屏。改为走与其它组相同的inlineToolCalls交接（running 由面板独占），并给InlineParallelAgentsDisplay加availableTerminalHeight窗口化兜底。滚轮被劫持：开启 SGR 鼠标追踪（
?1002h）会让终端不再对滚轮做原生 scrollback（Terminal.app 还把滚轮转成方向键 → 回放 composer 历史 / 操作面板）。collapsed thinking 块为了点击展开无条件开了鼠标追踪，于是非 VP 下只要有 thinking 块就滚不动。useMouseEvents改为默认仅在 VP（ui.useTerminalBuffer）下启用；真正需要滚轮的面板（ScrollableList、alt-screen 的ThinkingViewer）用新增的bypassVpGate显式豁免。thinking 块点击不豁免 → 非 VP 休眠、原生 scrollback 保住（仍可 Alt+T 展开）。单点收口，杜绝未来回归。PR #5799 只修了 footer 高度核算，未消除重复名册、也未阻止鼠标追踪劫持滚轮，故问题仍在。根因已对真实（fork 版）ink 渲染器实证。
🤖 Generated with Qwen Code