feat(cli): VP mode — inline thought expand on click + auto-hiding scrollbar - #6079
Conversation
…ollbar Two VP-mode (ui.useTerminalBuffer) UX improvements: 1. Thinking: clicking a thought now expands it inline, in place, instead of opening a full-screen modal. The expanded thought becomes part of the conversation and scrolls with it, matching the lighter inline pattern. A thought spans the `gemini_thought` head plus its trailing `gemini_thought_content` continuations, so expansion is keyed by the head id (buildThoughtHeadIdMap) and one click expands/collapses the whole group. Alt+T still toggles all thoughts at once. The full-screen ThinkingViewer modal (and its context) is removed: it only ever opened in VP, where an inline-expanded thought is already scrollable via the viewport, so it was redundant. Drops ThinkingViewer.tsx, ThinkingViewerContext.tsx, and the now-dead thinkingFullText plumbing. 2. Scrollbar: the VP scrollbar now auto-hides — it renders as blank cells while idle (keeping width 1 so the viewport never reflows) and pops in only while scrolling, then fades out. Adds `ui.showScrollbar` (default true) to hide it entirely. 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.
Thanks for the PR, @chiga0!
Unfortunately the PR body doesn't follow our pull request template. The template requires these sections:
## What this PR does## Why it's needed## Reviewer Test Plan(with### How to verify,### Evidence (Before & After),### Tested on)## Risk & Scope## Linked Issues
Your PR uses ## Summary and ## Testing instead, and is missing the reviewer test plan, evidence, risk assessment, and linked issues sections. These help reviewers understand the change and verify it quickly — could you update the body to match the template?
The code changes themselves look interesting and I'll come back for a full review once the template is filled in. 🙏
中文说明
感谢贡献 @chiga0！
PR 描述没有按照模板填写。模板要求以下章节：
## What this PR does## Why it's needed## Reviewer Test Plan（含### How to verify、### Evidence (Before & After)、### Tested on）## Risk & Scope## Linked Issues
目前用的是 ## Summary 和 ## Testing，缺少 reviewer test plan、证据截图、风险评估和关联 issue。请按模板更新描述，之后我会回来做完整 review。🙏
— 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.
[Suggestion] packages/cli/src/ui/components/AlternateScreen.tsx is now dead code — its sole consumer (ThinkingViewer.tsx) is deleted in this PR. A grep for from.*AlternateScreen across the entire packages/ tree returns zero imports. Consider deleting the file in this changeset to keep the cleanup complete.
— qwen3.7-max via Qwen Code /review
| @@ -91,32 +94,28 @@ interface HistoryItemDisplayProps { | |||
| */ | |||
| const ClickableThinkMessage: React.FC<{ | |||
There was a problem hiding this comment.
[Suggestion] Test coverage gap: ClickableThinkMessage is a new component introduced in this PR, but the click hit-testing logic is not tested. The existing test ('subscribes the click handler without bypassVpGate') only verifies useMouseEvents was called with { isActive: true } — it never simulates a mouse event or asserts that (a) an in-bounds left-press fires onToggle, (b) an out-of-bounds click is ignored, or (c) isPending=true disables the handler via isActive=false.
Since this is the core new interaction replacing the deleted ThinkingViewer, consider adding a test that mocks measureElementPosition to return known bounds, feeds a synthetic { name: 'left-press', col, row } event to the captured useMouseEvents callback, and asserts onToggle fires only for in-bounds coordinates.
— qwen3.7-max via Qwen Code /review
| // `thoughtHeadId`; the head itself falls back to its own id. | ||
| const thoughtGroupHeadId = thoughtHeadId ?? item.id; | ||
| const resolvedThoughtExpanded = | ||
| thoughtExpanded ?? (allExpanded || expandedHeadIds.has(thoughtGroupHeadId)); |
There was a problem hiding this comment.
[Suggestion] Test coverage gap: the new resolvedThoughtExpanded resolution has three untested code paths:
expandedHeadIds.has(thoughtGroupHeadId)— the per-thought expansion via clickthoughtHeadIdprop overridingitem.idfor continuationsthoughtExpandedprop forcing expansion regardless of context
The existing test ('renders committed thinking expanded when ThoughtExpandedProvider is true') only exercises allExpanded: true with an empty expandedHeadIds set. The expandedHeadIds path is the primary replacement for the deleted ThinkingViewer — a regression in the set-membership check or the ?? item.id fallback would silently break per-thought expansion.
Consider adding tests that exercise the expandedHeadIds path with a set containing the target head id, and one that passes thoughtHeadId for a continuation item to verify the grouping.
— qwen3.7-max via Qwen Code /review
| @@ -430,7 +432,7 @@ export const MainContent = () => { | |||
| isPending={false} | |||
There was a problem hiding this comment.
[Suggestion] thoughtHeadId is correctly passed to committed items here and in the virtual path above (line 363), but the pending items rendering below (lines ~447-463) does not pass thoughtHeadId. The same gap exists in AgentChatContent.tsx (lines ~251-265). Pending items also have their id overridden to 0 ({ ...item, id: 0 }), so thoughtGroupHeadId falls back to 0 for all of them.
If the user expands a thought group by clicking the head, pending continuation items that stream in won't match (expandedHeadIds.has(0) is false) and will render collapsed while the head is expanded — a brief visual desync during streaming. The desync self-heals when the item commits and the correct thoughtHeadId is passed.
Consider passing thoughtHeadId from the map to pending items as well, or using the item's original id instead of overriding it to 0.
— qwen3.7-max via Qwen Code /review
| // gutter competing with the conversation. On scroll it pops in — | ||
| // bright `█` thumb over a dim `│` track — then fades back to blank | ||
| // after the idle window. The column keeps width 1 in all states, so | ||
| // the viewport never reflows (which would force a per-item |
There was a problem hiding this comment.
[Suggestion] Test coverage gap: the auto-hide scrollbar rendering is the central visual change in this file, but no test exercises it with showScrollbar={true}. Every existing VirtualizedList.test.tsx and ScrollableList.test.tsx passes showScrollbar={false}, which early-returns from scrollbarContent before the new blank-cell / active-thumb branches are reached.
The three rendering states (idle-blank <Text> </Text>, active track+thumb │/█, and the transition between them) are entirely uncovered. Consider adding at least one test that renders with showScrollbar={true} and verifies the blank-cell idle state and the active thumb/track glyphs.
— qwen3.7-max via Qwen Code /review
Effect (macOS / VHS, local build of this branch)Auto-hiding scrollbar (Fix 2)Same scroll position — the bar shows while scrolling and fades to blank cells after ~1.5s idle (column width stays 1, so no reflow).
Inline thought expand (Fix 1)The collapsed, clickable thought line (click, or
The expanded state isn't screenshotted here: VHS can't inject SGR mouse clicks or |
The collapsed thinking line only hinted "option+t to expand", so the new click-to-expand affordance was undiscoverable. Show "(click or option+t to expand)" when the click handler is actually active — i.e. VP mode (ui.useTerminalBuffer) — and keep the plain "(option+t to expand)" in non-VP, where clicking does nothing (native scrollback is preserved). Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
| durationMs != null | ||
| ? `${t('Thought for')} ${formatDuration(durationMs)}` | ||
| : t('Thinking'); | ||
| const hint = clickable |
There was a problem hiding this comment.
[Suggestion] The collapsed-state hint correctly advertises "click or Alt+T to expand" in VP mode, but the expanded-state hint (around line 354) hardcodes t('({{keyHint}} to collapse)', ...) without checking the clickable prop. Since isActive = !isPending in ClickableThinkMessage, clicking an expanded thought does collapse it — but the hint only says "Alt+T to collapse", creating a UX asymmetry.
| const hint = clickable | |
| const expandHint = clickable | |
| ? t('(click or {{keyHint}} to expand)', { keyHint: toggleKeyHint }) | |
| : t('({{keyHint}} to expand)', { keyHint: toggleKeyHint }); | |
| const collapseHint = clickable | |
| ? t('(click or {{keyHint}} to collapse)', { keyHint: toggleKeyHint }) | |
| : t('({{keyHint}} to collapse)', { keyHint: toggleKeyHint }); |
Then use collapseHint in the expanded branch and expandHint in the collapsed branch.
— qwen3.7-max via Qwen Code /review
| const isActive = !isPending; | ||
|
|
||
| useMouseEvents( | ||
| useCallback( |
There was a problem hiding this comment.
[Suggestion] isActive changed from !isPending && !expanded to !isPending, so every non-pending thought now has an active click handler — including expanded ones. The <Box ref={isActive ? ref : undefined}> wraps the full ThinkMessage, so clicking anywhere on the expanded text body (not just the header line) collapses the block. There's no visual affordance that the expanded text is interactive (the "click to collapse" hint is only shown in collapsed state).
Users reading expanded thought text who click to focus or scroll within it will accidentally collapse the block they're reading. Consider either:
- Restricting the hit-test to the header line only (wrap only the icon + label
<Text>in theref-bearing<Box>), so clicking the expanded body doesn't trigger collapse. - Or restoring
!expandedtoisActiveand relying on Alt+T for collapse (as before).
— qwen3.7-max via Qwen Code /review
| contentWidth={contentWidth} | ||
| durationMs={itemForDisplay.durationMs} | ||
| onToggle={() => toggleThought(thoughtGroupHeadId)} | ||
| /> |
There was a problem hiding this comment.
[Suggestion] When the thoughtExpanded prop overrides the context read (e.g., SessionPreview passes true), the resolved expansion state ignores expandedHeadIds. However, onToggle still calls toggleThought(thoughtGroupHeadId), which writes to the shared context's expandedHeadIds. This creates an invisible mutation: clicking a thought in SessionPreview adds head IDs to the global set. When the user navigates back to the main conversation view (prop removed), those IDs cause phantom expansions — thoughts appear expanded without user action in that view.
Guard the toggle when the prop overrides:
| /> | |
| onToggle={thoughtExpanded != null ? () => {} : () => toggleThought(thoughtGroupHeadId)} |
— qwen3.7-max via Qwen Code /review
✅ Local real-build verification (maintainer, tmux + component harness)Verified PR head Verdict: functionally sound and ready to merge. Both features work as designed; the open 1. Auto-hiding scrollbar (Fix 2) — A/B vs
|
| # | Location | Nature | Verified at head | Severity |
|---|---|---|---|---|
| 1 | HistoryItemDisplay.tsx:95 |
click hit-test untested | gap real — filled by my §4 harness; feature works | low (coverage) |
| 2 | HistoryItemDisplay.tsx:220 |
resolvedThoughtExpanded paths untested |
gap real — §4 exercises expandedHeadIds + continuation |
low (coverage) |
| 3 | MainContent.tsx:432 |
pending items lack thoughtHeadId (rendered with id:0) |
confirmed | low — brief streaming desync, self-heals on commit |
| 4 | VirtualizedList.tsx:841 |
scrollbar render untested (showScrollbar={true}) |
gap real — covered by §1 tmux A/B | low (coverage) |
| 5 | ConversationMessages.tsx expanded hint |
expanded hint doesn't advertise "click" | confirmed — (option+t to collapse) only |
low (UX polish) |
| 6 | HistoryItemDisplay.tsx:124 |
isActive = !isPending → clicking anywhere on the expanded body collapses it (ref wraps the whole block) |
confirmed | low–med (UX choice, no affordance) |
| 7 | HistoryItemDisplay.tsx:293 |
SessionPreview passes thoughtExpanded={true} but onToggle is unguarded → click writes the head id into the shared expandedHeadIds |
logic gap confirmed (§4 harness case d: toggle(42) fires while force-expanded) |
low–med (latent; 1-line guard fixes) |
Cheapest worthwhile follow-ups: #7 (guard onToggle when thoughtExpanded != null) and #3 (pass thoughtHeadId to pending items / stop overriding id to 0). #5/#6 are UX judgment calls. For #7 the real-terminal trigger additionally needs a click to land inside SessionPreview's <Static> in VP mode, which I couldn't inject-test — so the logic gap is proven, end-to-end reachability is plausible-but-unconfirmed; the one-line guard is cheap insurance either way.
Merge state
mergeable = MERGEABLE, Test (ubuntu-latest, Node 22.x) = success on 67d3373a (mac/win/integration skipped per the named-job pattern). The only gate is reviewDecision = CHANGES_REQUESTED from qwen-code-ci-bot, which does not auto-clear on thread resolution — a maintainer dismiss (or re-review) is required.
🇨🇳 中文版（完整对应）
✅ 本地真实构建验证（维护者，tmux + 组件测试台）
在全新 worktree 上验证 PR head 67d3373a（npm ci + npm run build，退出码 0，tsc --build 干净）。方法：真实 qwen 二进制在 tmux 中以 VP 模式（ui.useTerminalBuffer: true）运行，配一个伪 OpenAI 端点流式返回「一段思考 + 60 行溢出答案」；点击路径用合成鼠标事件的组件测试台验证（点击无法经终端注入——见说明）。
结论：功能正确，可以合并。 两个特性都按设计工作；qwen-code-ci-bot 未关闭的线程全是 [Suggestion] 级别（测试覆盖 + 交互打磨 + 一处廉价的潜在防护），都不影响功能。
1. 自动隐藏滚动条（Fix 2）—— 与 origin/main A/B
同一滚动位置，110×30，用 SGR 注入滚轮，立即抓取一次、空闲 2.5 秒后再抓一次。内容区最右列：
origin/main (基线) PR head (67d3373a)
滚动后： █ 滑块 + │ 轨道 (16) █ 滑块 + │ 轨道 (16) ← 两者都弹出
空闲 2.5 秒后： │ │ │ │ │ │ … (16) (空白) ( 4) ← 基线保留一列暗
（常驻占位列） （滚动条彻底消失） │；PR 淡出
（PR 残留的 4 行是 banner 边框，第 3–6 行，不是滚动条。）任何状态下该列都保持宽度 1，视口从不 reflow。✔️ 承重性：只把 VirtualizedList.tsx 回退到 main 再重建，常驻的 │ 列就回来了。
2. ui.showScrollbar: false —— 新设置
置为 false 后重启：即使刚滚完轮，内容区也零滚动条字形。默认 true 保留上面的自动隐藏行为。✔️
3. 思考内联展开（Fix 1）+ 移除模态框
- 折叠态：
∴ Thought for 0s (click or option+t to expand)——新的点击提示（commit67d3373a）在 VP 下已生效。 - Alt+T 把推理就地内联展开（完整文本嵌在对话流里，答案接在下面），提示切换为
(option+t to collapse)；再按一次 Alt+T 折叠回去。 - 没有全屏覆盖 / alt-screen 接管——旧的
ThinkingViewer模态框、ThinkingViewerContext、thinkingFullText管线都删掉了。✔️
4. 点击路径（组件测试台——5/5 通过）
向真实的 ClickableThinkMessage 命中测试注入合成 left-press（useMouseEvents mock 成捕获回调，measureElementPosition mock 成已知边界）：
- ✔️ 界内点击就地展开折叠的思考；界外点击忽略；滚轮/非
left-press忽略。 - ✔️ head-id 分组：点击头部一次，头部及其
gemini_thought_content续块一起展开（二者经buildThoughtHeadIdMap共享 head id）。
5. 单元测试 + 变异
- PR 套件 115/115 通过：
historyUtils、HistoryItemDisplay、ConversationMessages、VirtualizedList、ScrollableList、settingsSchema。 - 变异（证明新测试非空过）：把
buildThoughtHeadIdMap里续块映射改坏（headId → headId+999），2 个测试失败（expected 1000 to be 1）；还原后 13 个全过。
点击注入说明： 点击展开无法经 tmux/VHS 驱动——SGR 点击对
measureElementPosition做命中测试，其坐标不映射到虚拟化视口的屏幕行（VP 既有的几何限制；这也是作者截图缺少展开态的原因）。滚轮可以注入，上面滚动条 A/B 即由此驱动。因此点击路径用 §4 组件测试台验证，而非 tmux——这恰是 bot 指出的覆盖缺口，且通过。
对 qwen-code-ci-bot 未关闭线程的评估（全部 [Suggestion]）
逐条对照 head 代码——都真实存在，但都不阻塞：
| # | 位置 | 性质 | head 处核实 | 严重度 |
|---|---|---|---|---|
| 1 | HistoryItemDisplay.tsx:95 |
点击命中测试无测试 | 缺口真实——已由 §4 测试台补上；功能正常 | 低（覆盖） |
| 2 | HistoryItemDisplay.tsx:220 |
resolvedThoughtExpanded 分支无测试 |
缺口真实——§4 覆盖了 expandedHeadIds + 续块 |
低（覆盖） |
| 3 | MainContent.tsx:432 |
pending 项没传 thoughtHeadId（以 id:0 渲染） |
已确认 | 低——流式期间短暂错位，提交后自愈 |
| 4 | VirtualizedList.tsx:841 |
滚动条渲染无测试（showScrollbar={true}） |
缺口真实——已由 §1 tmux A/B 覆盖 | 低（覆盖） |
| 5 | ConversationMessages.tsx 展开态提示 |
展开态提示未标「click」 | 已确认——只有 (option+t to collapse) |
低（交互打磨） |
| 6 | HistoryItemDisplay.tsx:124 |
isActive = !isPending → 点击展开正文任意处都会折叠（ref 包住整块） |
已确认 | 低–中（交互取舍，无提示） |
| 7 | HistoryItemDisplay.tsx:293 |
SessionPreview 传 thoughtExpanded={true} 但 onToggle 未加守卫 → 点击会把 head id 写入共享的 expandedHeadIds |
逻辑缺口已确认（§4 测试台用例 d：强制展开态下 toggle(42) 仍触发） |
低–中（潜在；一行守卫即可） |
最值得做的低成本跟进：#7（当 thoughtExpanded != null 时守卫 onToggle）和 #3（给 pending 项传 thoughtHeadId / 别把 id 覆盖成 0）。#5/#6 属交互取舍。#7 的真实终端触发还需要点击落进 VP 模式下 SessionPreview 的 <Static>，我无法注入测试——所以逻辑缺口已证实，端到端可达性是「合理但未确认」；无论如何那一行守卫都是廉价保险。
合并状态
mergeable = MERGEABLE，Test (ubuntu-latest, Node 22.x) = success（就在 67d3373a 上；mac/win/integration 按 named-job 模式 skip）。唯一卡点是 qwen-code-ci-bot 的 reviewDecision = CHANGES_REQUESTED，它不会随线程 resolve 自动清除——需要维护者 dismiss（或重新 review）。
Method: worktree build of 67d3373a · real qwen binary in tmux (VP mode, fake OpenAI) · synthetic-mouse component harness · 115 PR unit tests + mutation. Fresh isolated HOME, no live model.
|
@qwen-code /triage |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Hi @chiga0 — thanks for this PR! The inline thought expansion and auto-hiding scrollbar look like meaningful UX improvements for VP mode.
However, the PR body doesn't follow our PR template. A few required sections are missing:
- What this PR does / Why it's needed — the
## Summarysection covers some of this, but the template asks for these as separate headings so reviewers can quickly understand scope and motivation. - Reviewer Test Plan — this is the most important missing piece. We need:
- How to verify — steps a reviewer can follow to confirm the behavior
- Evidence (Before & After) — screenshots, tmux captures, or a short recording showing the before/after
- Tested on — which OSes you verified (macOS / Windows / Linux)
- Risk & Scope — main risks, what's out of scope, any breaking changes
- Linked Issues — related issue references
Could you update the PR body to match the template? It helps reviewers (and the automated triage) evaluate the change efficiently.
中文说明
感谢 PR！内联思考展开和自动隐藏滚动条对 VP 模式来说是很好的 UX 改进。
但 PR 正文没有遵循 PR 模板，缺少以下必填部分：
- What this PR does / Why it's needed —
## Summary涵盖了部分内容，但模板要求这两个作为独立标题 - Reviewer Test Plan — 最重要的缺失部分，需要包含验证步骤、前后对比证据、测试平台
- Risk & Scope — 主要风险、不在范围内的内容、破坏性变更
- Linked Issues — 关联的 issue
请按模板更新 PR 正文，方便审查。
— Qwen Code · qwen3.7-max
|
@qwen-code /triage |
|
Thanks for the PR, @chiga0! The body doesn't use the PR template headings (no "What this PR does", "Why it's needed", "Reviewer Test Plan", "Risk & Scope", "Linked Issues", or 中文说明). The content is thorough — the Summary covers direction and scope well, and the Testing section is solid — but the missing Reviewer Test Plan section (especially the "Tested on" OS matrix and "How to verify" steps) means reviewers have to piece together how to confirm the changes. Worth reformatting to the template for the next revision. On direction: both changes are clearly user-facing VP-mode improvements. The full-screen ThinkingViewer modal is genuinely disruptive — replacing it with inline expand is the right call, and it's the same direction other TUI tools have taken. The auto-hiding scrollbar is a natural polish follow-up. CHANGELOG has scroll-related fixes in recent releases (scroll position jumping, subagent scroll bleeding), confirming the VP scrolling area is actively maintained. On approach: the scope is tight and well-motivated. The One design question worth thinking about: when Alt+T sets Moving on to code review and testing. 🔍 中文说明感谢贡献，@chiga0！ PR 正文没有使用PR 模板标题（缺少 "What this PR does"、"Why it's needed"、"Reviewer Test Plan"、"Risk & Scope"、"Linked Issues" 和中文说明）。内容本身很充实——Summary 涵盖了方向和范围，Testing 部分也很完整——但缺少 Reviewer Test Plan（特别是 "Tested on" 操作系统矩阵和 "How to verify" 步骤）意味着审查者需要自己拼凑验证方法。建议下次修订时使用模板格式。 方向：两项改动都是明确的 VP 模式用户体验改善。全屏 ThinkingViewer 模态框确实具有干扰性——用内联展开替代是正确选择，也与其他 TUI 工具的方向一致。自动隐藏滚动条是自然的打磨改进。CHANGELOG 中近期版本有多项滚动相关修复（滚动位置跳动、子代理滚动泄漏），证实 VP 滚动区域是活跃维护的方向。 方案：范围紧凑且有明确动机。 一个值得思考的设计问题：当 Alt+T 设置 进入代码审查和测试。🔍 — Qwen Code · qwen3.7-max |
Code ReviewIndependent proposal (before reading the diff): To replace the full-screen ThinkingViewer modal, I'd add per-thought inline expansion state keyed by some thought identifier, wire a click handler on the collapsed thinking line, and remove the modal + its context. For the scrollbar, I'd make the existing Comparison with PR approach: the PR's solution matches this almost exactly. Reuse check: the PR reuses Correctness review — no blockers found: The The maintainer (@wenshao) flagged item #7 in their review — Convention compliance: follows project patterns — ESM, strict TypeScript, collocated tests, proper context/provider patterns, i18n via Test ResultsAll relevant suites pass on PR head: Build ( Real-Scenario Testing (tmux)CLI starts without import errors or crashes from the changed context shapes ( The maintainer's tmux + component harness testing (posted as a prior comment) provides thorough coverage:
中文说明代码审查独立方案（读 diff 前）： 要替换全屏 ThinkingViewer 模态框，我会为每个思考添加内联展开状态，以某种标识符为键，在折叠的思考行上绑定点击处理，然后移除模态框及其上下文。对于滚动条，我会让现有的 与 PR 方案对比： PR 的方案与此几乎完全一致。 复用检查： PR 复用了 正确性审查——未发现阻塞问题：
维护者指出的 #7 项—— 规范合规性： 遵循项目模式——ESM、严格 TypeScript、测试同置、正确的 context/provider 模式、i18n。 测试结果所有相关套件在 PR head 上通过（共 224 个测试，0 失败）。构建（ 真实场景测试（tmux）CLI 启动无 import 错误或上下文形状变更导致的崩溃。由于 CI 环境无 API 凭证，无法端到端测试 VP 模式特性。 维护者的 tmux + 组件测试台测试提供了全面的覆盖：滚动条自动隐藏 A/B 对比确认、内联思考展开验证、点击路径合成鼠标测试 5/5 通过、变异测试确认新测试非空。 — Qwen Code · qwen3.7-max |
|
Stepping back: this PR does two small things and does them well. The full-screen thinking modal was the kind of UX that looks fine in a demo but grates in daily use — it hijacks the screen, blocks input, and forces a modal context that doesn't belong in a conversation flow. Inline expand is the obvious fix, and the implementation is clean: a head-id grouping keyed off the existing The scrollbar change is six lines that turn a persistent gutter column into an overlay-style auto-hide — it pops in while scrolling and fades to blank cells when idle, keeping the column width at 1 so nothing reflows. The My independent proposal would have been essentially identical. The PR doesn't miss a simpler path. Every change in the diff is needed for the stated goal — there's no drive-by refactoring, no speculative features, no scope creep. The net result is -160 lines and two genuine UX improvements. The 224 unit tests all pass. The maintainer's tmux + component harness testing (A/B scrollbar comparison, click injection, mutation testing) validates the features end-to-end. I couldn't reproduce the VP-mode features in this CI (no API credentials), but the code loads cleanly and the component tests cover the logic paths. Two minor items worth noting but not blocking on:
Approving. ✅ 中文说明退一步看：这个 PR 做了两件小事，都做得很好。全屏思考模态框是那种在演示中看起来没问题，但日常使用中很烦人的 UX——它劫持屏幕、阻塞输入、强制一个不属于对话流的模态上下文。内联展开是显而易见的修复，实现很干净：以现有 滚动条改动是六行代码，把持久的边栏列变成了覆盖式自动隐藏——滚动时弹出，空闲时淡出为空白单元格，保持列宽为 1 所以不会回流。 我的独立方案本质上会是一样的。PR 没有遗漏更简单的路径。diff 中的每个改动都是目标所需的——没有顺手重构、没有投机性功能、没有范围蔓延。最终结果是净减 160 行和两个真实的 UX 改善。 224 个单元测试全部通过。维护者的 tmux + 组件测试台（A/B 滚动条对比、点击注入、变异测试）端到端验证了功能。我无法在此 CI 中复现 VP 模式特性（无 API 凭证），但代码加载干净，组件测试覆盖了逻辑路径。 两个值得注意但不阻塞的小项：
批准。✅ — 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. ✅
DragonnZhang
left a comment
DragonnZhang
left a comment
There was a problem hiding this comment.
APPROVE
Replaces the full-screen ThinkingViewer modal with inline per-thought expansion that scrolls with the conversation. The refactor is clean: ThoughtExpandedContext gains per-thought head-id tracking alongside the Alt+T global toggle, buildThoughtHeadIdMap simplifies the old text-aggregation helper, and the VirtualizedList scrollbar gets an auto-hide overlay style. Tests are updated for the new context shape and click-affordance logic. CI passes.
— qwen3-coder via Qwen Code /review
wenshao
left a comment
wenshao
left a comment
There was a problem hiding this comment.
Reviewed at medium effort: 8 finder angles (line-by-line, removed-behavior, cross-file tracing, reuse/simplification/efficiency/altitude, conventions) over the full diff, each candidate then independently verified against the PR head. 6 findings survived and are posted inline (2 in the agent view / Alt+T interplay, 1 scrollbar hit-testing, 3 polish). Two candidates were refuted during verification and are not filed, for the record: (a) expandedThoughtHeadIds never being reset is harmless — ids are timestamp-based and resume mints fresh ids, so stale entries are inert; (b) the per-click context broadcast re-render is bounded to the mounted VP window and the click forces a relayout anyway, so no actionable perf cost.
One repo-level cleanup that has no diff line to anchor on: packages/cli/src/ui/components/AlternateScreen.tsx is now dead code — its only importer was the deleted ThinkingViewer.tsx, and after this PR nothing in the package references it (grep confirms). Since alt-screen escape handling is delicate around Ink exit teardown, better to delete it in this PR than leave an orphan that looks load-bearing.
Also verified clean: sanitization is preserved (escapeAnsiCtrlCodes(item) covers the inline path), continuations do expand with their head via the head-id map, non-VP behavior is not a regression (the old click was VP-gated too), the vscode settings-schema mirror was updated correctly, and no orphaned ThinkingViewer/thinkingFullText references remain.
中文
以中等强度审查:8 个查找视角(逐行、删除行为审计、跨文件追踪、复用/简化/效率/层次、约定)覆盖全部 diff,每个候选再独立对照 PR head 验证。6 条发现存活,已作为行内评论发出(2 条涉及 agent 视图 / Alt+T 交互,1 条滚动条命中测试,3 条打磨项)。两条候选在验证中被驳回,特此说明:(a) expandedThoughtHeadIds 从不重置是无害的——id 基于时间戳,resume 会铸造新 id,陈旧条目不生效;(b) 每次点击的 context 广播重渲染仅限已挂载的 VP 窗口,且点击本身就强制重排,无可行动的性能代价。
一条无法锚定到 diff 行的仓库级清理:packages/cli/src/ui/components/AlternateScreen.tsx 现在是死代码——它唯一的引用方是被删除的 ThinkingViewer.tsx,本 PR 之后包内无任何引用(grep 确认)。鉴于 alt-screen 转义处理在 Ink 退出拆解附近很微妙,建议在本 PR 中一并删除,而不是留下一个看似承重的孤儿组件。
另验证无问题:净化逻辑保留(escapeAnsiCtrlCodes(item) 覆盖内联路径);continuation 通过 head-id 映射随头部一起展开;非 VP 行为无回归(旧的点击本来也被 VP 门控);vscode settings-schema 镜像已正确更新;无 ThinkingViewer/thinkingFullText 残留引用。
| terminalWidth={terminalWidth} | ||
| mainAreaWidth={contentWidth} | ||
| thinkingFullText={thinkingFullTextByItem.get(item)} | ||
| thoughtHeadId={thoughtHeadIdByItem.get(item)} |
There was a problem hiding this comment.
[Confirmed] Click-to-expand is a guaranteed no-op for committed thoughts in the agent view, yet the hint advertises it.
Unlike MainContent (which switches to ScrollableList in VP mode), AgentChatContent always renders committed items inside a real Ink <Static> (line 224). Static's useLayoutEffect advances its index past flushed items, so the ClickableThinkMessage for a committed thought is unmounted one commit after its output is written, tearing down its useMouseEvents subscription — the click can never fire. But clickable = !!settings.merged.ui?.useTerminalBuffer (HistoryItemDisplay.tsx:120) doesn't know about <Static>, so in VP mode every committed thought here renders "(click or alt+t to expand)" as a dead affordance. Alt+T still works only because its handler calls refreshStatic() (AppContainer.tsx:3286-3289), which the click path doesn't.
A second hazard sits behind this one: expandedThoughtHeadIds is a single app-global set keyed by raw item.id, while agentMessagesToHistoryItems numbers every agent transcript from 0 (let nextId = 0, agentHistoryAdapter.ts:47). If agent-view clicks are ever made to work, expanding thought id 3 in agent A marks id 3 expanded in every other agent's view. Suggest suppressing the click hint for this surface for now (e.g. a clickable={false} / prop override like SessionPreview's thoughtExpanded), and namespacing the expansion state per surface if/when agent-view expansion is wired up.
中文
[已确认] agent 视图中已提交的 thought 点击展开必然无效,但提示仍宣传可点击。
与 MainContent(VP 模式切换到 ScrollableList)不同,AgentChatContent 始终把已提交条目渲染在真正的 Ink <Static> 里(第 224 行)。Static 的 useLayoutEffect 会把 index 推进到已刷出条目之后,所以已提交 thought 的 ClickableThinkMessage 在输出写出后的下一次提交即被卸载,其 useMouseEvents 订阅随之拆除——点击永远不会触发。但 clickable = !!settings.merged.ui?.useTerminalBuffer(HistoryItemDisplay.tsx:120)不感知 <Static>,因此 VP 模式下这里每条已提交 thought 都渲染 "(click or alt+t to expand)" 这一死亡提示。Alt+T 仍然可用只是因为其处理器调用了 refreshStatic()(AppContainer.tsx:3286-3289),而点击路径没有。
背后还有第二个隐患:expandedThoughtHeadIds 是按原始 item.id 键控的应用级全局集合,而 agentMessagesToHistoryItems 对每个 agent 转录都从 0 编号(let nextId = 0,agentHistoryAdapter.ts:47)。若将来让 agent 视图的点击生效,在 agent A 中展开 id 3 会使其他所有 agent 视图中的 id 3 也标记为展开。建议目前先对该 surface 屏蔽点击提示(例如类似 SessionPreview 的 thoughtExpanded 强制 prop 传 clickable={false}),将来接通 agent 视图展开时再对展开状态按 surface 命名空间化。
| // `thoughtHeadId`; the head itself falls back to its own id. | ||
| const thoughtGroupHeadId = thoughtHeadId ?? item.id; | ||
| const resolvedThoughtExpanded = | ||
| thoughtExpanded ?? (allExpanded || expandedHeadIds.has(thoughtGroupHeadId)); |
There was a problem hiding this comment.
[Confirmed] Alt+T and per-thought click interact badly: clicking while allExpanded is on silently poisons the set.
resolvedThoughtExpanded is a pure OR, and the mouse handler stays armed on expanded blocks (isActive = !isPending, line 121). Sequence: Alt+T on → user clicks a long thought intending to collapse it → nothing changes visually (OR still true) but toggleThoughtExpanded adds its head id to expandedThoughtHeadIds → user turns Alt+T off → that one thought stays expanded and needs a second click to clear the stale entry. Nothing ever resets the set on Alt+T (the Alt+T handler only does setThoughtExpanded + refreshStatic(), AppContainer.tsx:3285-3289).
Two easy fixes: have the click handler treat allExpanded as the effective state (delete-only, never add, while allExpanded is true), or clear expandedThoughtHeadIds whenever Alt+T flips (Alt+T then behaves as an absolute set/clear, which matches its "toggle all" label).
中文
[已确认] Alt+T 与单条点击交互不良:allExpanded 开启时点击会静默污染集合。
resolvedThoughtExpanded 是纯 OR,且展开块上鼠标处理器仍然激活(isActive = !isPending,第 121 行)。序列:开 Alt+T → 用户点击长 thought 想收起 → 视觉无变化(OR 仍为 true)但 toggleThoughtExpanded 把 head id 加入 expandedThoughtHeadIds → 用户关 Alt+T → 该 thought 仍然展开,需再点一次才能清掉脏条目。Alt+T 从不重置集合(其处理器只做 setThoughtExpanded + refreshStatic(),AppContainer.tsx:3285-3289)。
两个简单修法:点击处理器把 allExpanded 视为有效状态(allExpanded 为 true 时只删不加);或 Alt+T 翻转时清空 expandedThoughtHeadIds(Alt+T 变成绝对的全开/全关,与其"全部切换"的语义一致)。
| // after the idle window. The column keeps width 1 in all states, so | ||
| // the viewport never reflows (which would force a per-item | ||
| // re-measure + visible jitter). | ||
| if (!scrollbarThumbActive) { |
There was a problem hiding this comment.
[Confirmed] The auto-hidden scrollbar is invisible but still clickable/draggable — hit-testing never consults scrollbarThumbActive.
Auto-hide is implemented only in the paint path; getScrollbarGeometry / hitTestScrollbar / scrollToScrollbarRow (lines 587-657) and ScrollableList's left-press/drag handler (ScrollableList.tsx:155-163, 127) are unchanged. Pre-PR the track was always visibly drawn, so the click target matched a visible affordance; now, while the bar is idle-hidden, the rightmost viewport column is an invisible full-height jump-scroll target: a left-click there (e.g. aiming near the right edge, or clicking to focus the window) yanks the conversation to a position proportional to the click row — a top-right click warps to the top of history — with no visible scrollbar explaining the jump. A press-and-move also starts an invisible drag.
Suggest gating the press hit-test on thumb visibility (scrollbarThumbActive), or having the first press only flashScrollbar() (reveal) and require the bar to be visible before a press jumps/drags — the overlay-scrollbar behavior users know from editors.
中文
[已确认] 自动隐藏后的滚动条不可见但仍可点击/拖拽——命中测试从不检查 scrollbarThumbActive。
自动隐藏只实现在绘制路径;getScrollbarGeometry / hitTestScrollbar / scrollToScrollbarRow(587-657 行)以及 ScrollableList 的左键按下/拖拽处理(ScrollableList.tsx:155-163、127)均未改。PR 之前轨道始终可见,点击目标与可见示能一致;现在滚动条闲置隐藏时,视口最右一列变成一个不可见的全高跳转目标:在那里左键点击(比如瞄准右缘附近内容、或点击聚焦窗口)会把会话拽到与点击行成比例的位置——右上角一点直接跳回历史顶部——且屏幕上没有任何滚动条解释这次跳动。按住移动还会开始一次不可见拖拽。
建议按下命中测试以 thumb 可见性(scrollbarThumbActive)为门;或首次按下只 flashScrollbar()(先显形),滚动条可见后按下才允许跳转/拖拽——即编辑器里用户熟悉的 overlay 滚动条行为。
| // via Alt+T. Advertise "click" in the collapsed hint only in VP, where the | ||
| // click actually does something. | ||
| const settings = useSettings(); | ||
| const clickable = !!settings.merged.ui?.useTerminalBuffer; |
There was a problem hiding this comment.
clickable copies one of the three gate terms useMouseEvents actually uses — the hint can advertise a dead click today, and the copies will drift.
The hook's effective gate is enabled = isActive && isRawModeSupported && vpGateOpen (useMouseEvents.ts:119); this line copies only the VP-setting term. Concretely reachable today: VP mode on a non-raw-mode stdin → the collapsed line says "(click or … to expand)" while the handler never subscribes. And any future gate condition (a ui.disableMouse setting, per-platform opt-out) silently desyncs the hint. Three finder angles independently flagged this. Suggest a single source: export a small useIsMouseClickAvailable() next to useMouseEvents (VP gate + isRawModeSupported), or have useMouseEvents return its enabled state, and drive both the subscription and the hint from it. (Note useUIState isn't usable here — HistoryItemDisplay also renders under SessionPreview without that provider.)
中文
clickable 只复制了 useMouseEvents 实际门控三项中的一项——今天就存在提示宣传死点击的场景,且两份拷贝会漂移。
hook 的有效门控是 enabled = isActive && isRawModeSupported && vpGateOpen(useMouseEvents.ts:119);本行只复制了 VP 设置这一项。今天即可触达:VP 模式 + 不支持 raw mode 的 stdin → 折叠行显示 "(click or … to expand)" 而处理器根本不会订阅。将来门控加任何新条件(如 ui.disableMouse 设置、按平台关闭)都会让提示静默失同步。三个查找视角独立标记了此问题。建议单一来源:在 useMouseEvents 旁导出一个小的 useIsMouseClickAvailable()(VP 门 + isRawModeSupported),或让 useMouseEvents 返回其 enabled 状态,订阅与提示都从它驱动。(注意这里不能用 useUIState——HistoryItemDisplay 也在无该 provider 的 SessionPreview 下渲染。)
| durationMs != null | ||
| ? `${t('Thought for')} ${formatDuration(durationMs)}` | ||
| : t('Thinking'); | ||
| const hint = clickable |
There was a problem hiding this comment.
Minor: the expanded state never advertises click-to-collapse, though the whole block is a click hotspot.
clickable is only consulted in the collapsed branch; the expanded header still renders "({{keyHint}} to collapse)" (line ~354) with the prop unused. Meanwhile, dropping !expanded from isActive (HistoryItemDisplay.tsx:121) makes the entire expanded block — potentially hundreds of rows — collapse on any left-click inside it, so a click meant to focus the window mid-read collapses the thought and loses the reading position, with nothing indicating clicks there are destructive. Cheap symmetry fix: mirror the collapsed hint ("(click or {{keyHint}} to collapse)" when clickable); if you want to remove the trap too, restrict the collapse hit-test to the header line instead of the whole block.
中文
次要:展开态从不宣传点击可收起,尽管整块都是点击热区。
clickable 只在折叠分支被消费;展开头部仍渲染 "({{keyHint}} to collapse)"(约 354 行),prop 传入但未用。同时 isActive 去掉 !expanded(HistoryItemDisplay.tsx:121)使整个展开块——可能几百行——在内部任意左键点击时收起:阅读中途想点击聚焦窗口,thought 立即塌缩、丢失阅读位置,且没有任何提示表明那里的点击是破坏性的。廉价的对称修法:镜像折叠提示(clickable 时显示 "(click or {{keyHint}} to collapse)");若还想消除误触陷阱,可把收起的命中区域限制在头部行而非整块。
| export const MainContent = () => { | ||
| const { version } = useAppContext(); | ||
| const uiState = useUIState(); | ||
| const showScrollbar = uiState.showScrollbar ?? true; |
There was a problem hiding this comment.
Minor: the showScrollbar default of true is now encoded in four independent places.
settingsSchema default, AppContainer (settings.merged.ui?.showScrollbar ?? true), this line, and VirtualizedList (props.showScrollbar ?? true, twice at lines 587/800). AppContainer already resolves the default before publishing to UIState, so this layer exists only because UIState.showScrollbar is declared optional — unlike its sibling useTerminalBuffer: boolean, which is required and consumed with no fallback (line 117). Declaring showScrollbar: boolean required removes this re-default; flipping the default later then can't silently disagree across layers.
中文
次要:showScrollbar 的默认值 true 现在编码在四个独立位置。
settingsSchema 默认值、AppContainer(settings.merged.ui?.showScrollbar ?? true)、本行、以及 VirtualizedList(props.showScrollbar ?? true,587/800 两处)。AppContainer 发布到 UIState 前已解析过默认值,这一层的存在只因 UIState.showScrollbar 声明为可选——不像其兄弟 useTerminalBuffer: boolean 是必填且消费时无回退(第 117 行)。把 showScrollbar: boolean 声明为必填即可去掉本处再默认;将来翻转默认值也不会在各层间静默不一致。
Resolve conflicts around the thinking-block interaction model. main QwenLM#6079 ("VP mode — inline thought expand on click") deleted the full-screen ThinkingViewer modal and replaced click-to-open with per-thought inline expansion keyed by head id (`ThoughtExpandedContext` now exposes `{ allExpanded, expandedHeadIds, toggle }` instead of a bare boolean). - HistoryItemDisplay.tsx: adopt main's per-group inline toggle; keep this branch's `fullDetail` (Ctrl+O forces every thought expanded, layered on top of main's toggle set). Drop the now-obsolete `thinkingFullText` prop (it fed the retired modal viewer; inline expansion shows the full text in place). - AppContainer.tsx: keep the Ctrl+O transcript machinery (transcriptFreeze, the transcript input-owns-input branch, TranscriptView render) and main's new per-thought expansion state; remove the retired ThinkingViewer modal wiring (state, open/close callbacks, provider, render branch, imports). Do not reintroduce main's CompactModeProvider — this branch removes compact mode. ThinkingViewer.tsx / .test / context deletions from main are kept. tsc clean; HistoryItemDisplay, AppContainer, MainContent tests green (154). Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
4 participants
Summary
Two VP-mode (
ui.useTerminalBuffer) UX improvements, motivated by comparison with other TUI agents (lighter inline thinking, more natural scrolling).1. Thinking: click → inline expand (replaces the full-screen modal)
Clicking a thought now expands it inline, in place — it becomes part of the conversation and scrolls with it — instead of opening a full-screen overlay that takes over input.
gemini_thoughthead + its trailinggemini_thought_contentcontinuations, so expansion is keyed by the head id (buildThoughtHeadIdMap); one click expands/collapses the whole group.Alt+Tstill toggles all thoughts at once.ThinkingViewermodal + its context are removed: it only ever opened in VP, where an inline-expanded thought is already scrollable via the viewport, so it was redundant. Also drops the now-deadthinkingFullTextplumbing.2. Scrollbar: auto-hide (overlay style) + toggle
The VP scrollbar now auto-hides — it renders as blank cells while idle (keeping column width 1 so the viewport never reflows) and pops in only while scrolling, then fades out. Adds
ui.showScrollbar(defaulttrue) to hide it entirely.Not in this PR (evaluated)
main—scrollByclamps via theMAX_SAFE_INTEGERsticky anchor and re-sticks at>= maxScroll; verified both fits/overflow paths./pressed): environment-specific (terminals that reportrows=0at startup); couldn't reproduce locally, so deferred rather than guess.?1002hhijacks drag): a real P0, but the terminal-mode fix (?1000h) conflicts with scrollbar-drag and needs per-terminal wheel verification — deferred to a focused change.?1003h) tracking, which has session-wide perf + text-selection costs.Testing
tsc --noEmit(cli) clean; eslint + prettier clean.vitest runfor MainContent, HistoryItemDisplay, historyUtils, AppContainer, VirtualizedList, ScrollableList, settingsSchema/settings — all passing (updatedbuildThoughtHeadIdMap+ThoughtExpandedProvidershape tests).🤖 Generated with Qwen Code