fix(cli): fix thought viewer truncation, layout gaps, and choppy scrolling in VP mode - #6002
Conversation
The full-screen ThinkingViewer split `data.text` on '\n' and rendered each logical line with `wrap="truncate-end"`. A thought is usually a single long paragraph with no newlines, so it collapsed to one ellipsised row above an empty box and `maxScroll` stayed 0 (could not scroll). Pre-wrap the text to visual rows at the inner content width (border + paddingX = 4 cols) before slicing, reusing the existing `wrapToVisualLines` helper (now exported from ConversationMessages). Scrolling and rendering now operate on the same rows the user sees. Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
In terminal-buffer (VP) mode the conversation wasted vertical space: - VirtualizedList pinned its root box to the full `containerHeight`, so short content left a tall blank gap and pushed the composer far down. Collapse the box to `min(containerHeight, totalHeight)` so it grows with its content like the legacy <Static> path; the scroll math still uses the full height, so overflow scrolling is unchanged. - `availableTerminalHeight` subtracted `staticExtraHeight` + `MAIN_CONTENT_HEIGHT_RESERVATION`, the <Static> overflow-flicker guards. VP clips natively and does not need them, so they stranded ~5 blank rows below the composer (input never reached the bottom). Drop the reservation in VP; non-VP keeps it unchanged. Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Terminal mouse reporting emits one event per row crossed, so a brisk wheel spin or scrollbar drag fired a rapid burst, each applied synchronously with a full Ink reflow + terminal flush — the source of the choppy scroll. Accumulate wheel deltas / the latest drag row in refs and flush at most once per ~16ms frame. A press still applies instantly; under NODE_ENV==='test' updates apply synchronously so the existing timer-free tests keep passing. Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
|
Thanks for the PR @chiga0! Template note: the body uses On direction: VP mode ( On approach: three focused fixes in one PR. Each is small, self-contained, and clearly motivated:
Moving on to code review. 🔍 中文说明感谢 @chiga0 的 PR！ 模板提示：正文用了 方向：VP 模式（ 方案：一个 PR 里三个聚焦的修复。每个都很小、自包含、动机清晰：
进入代码审查 🔍 — Qwen Code · qwen3.7-max |
chiga0
left a comment
chiga0
left a comment
There was a problem hiding this comment.
Code Review Overview (AI Generated)
PR: #6002 fix(cli): VP mode thought viewer truncation, wasted layout space, and choppy scroll
Type: Bug Fix (3 discrete UI fixes)
Change size: +127/-34 across 5 files
HEAD: 46f168d9
Findings Summary
- Critical/Major: 0
- Minor: 0
- Nit: 1
Architecture Assessment
Three clean, focused fixes that each address a specific VP-mode UX problem. The changes are well-scoped with no cross-cutting side effects:
-
ThinkingViewer truncation — Pre-wrapping with
wrapToVisualLinesatcontentWidth = columns - 4(1 border + 1 padding per side) is correct. Verified againstuseTerminalSize()which returns{ columns, rows }. TheuseMemodependency on[data.text, contentWidth]correctly handles both content changes and terminal resizes.maxScrollauto-recalculates via the existinguseEffect. -
Layout wasted space — Two-part fix:
VirtualizedListcollapsesrootHeighttomin(containerHeight, totalHeight)whilescrollableContainerHeightretains the fullcontainerHeightfor scroll math.AppContainerdrops thestaticExtraHeight + MAIN_CONTENT_HEIGHT_RESERVATIONreservation in VP mode. Both changes are correctly guarded (VP-only / undefined-containerHeight fallback preserved). -
Scroll coalescing — Clean leading-edge timer pattern: accumulate wheel deltas and drag position in refs, flush once per 16ms frame. The
NODE_ENV === 'test'synchronous path mirrorsVirtualizedList's existing test escape hatch. Cleanup on unmount viauseEffectdestructor.left-presscorrectly bypasses coalescing for instant feedback. Drag-over-wheel priority (drag wins when both are pending) is correct.
Detailed Review
ScrollableList.tsx — The coalescing implementation is sound. A few correctness details verified:
- Timer cleanup on unmount:
useEffectdestructor capturesflushTimer.currentby reference and clears it, preventing post-unmount calls. - Stale timer after unmount:
applyPendingScrollguards against nullvirtualizedListRef.current(auto-cleared by React on unmount). - Rapid re-scroll after timer fires:
flushTimer.current = nullinapplyPendingScrollallows the next event to schedule a fresh timer. - Wheel delta accumulation between frames: summing is correct (total intended scroll), not a race condition.
ThinkingViewer.tsx — Export of wrapToVisualLines from ConversationMessages.tsx is the minimal surface change needed. Verified this is the only new consumer (no blast radius beyond this import).
VirtualizedList.tsx — The rootHeight change is well-isolated: scrollableContainerHeight (line 269) is assigned from the pre-change containerHeight variable, so scroll math is entirely unaffected. The JSX change only impacts the visual box height.
AppContainer.tsx — The conditional mainContentHeightReservation correctly preserves legacy behavior for non-VP mode while removing the unnecessary reservation for VP mode.
Additional Audit Coverage
- Handler parallelism: Compared scroll handling in ScrollableList with keyboard scroll (useKeypress) — keyboard path correctly remains synchronous (single events, no burst), coalescing only applies to mouse events.
- Data structure blast radius:
BufferedSessionFramenot affected;wrapToVisualLinesexport is additive-only with no signature change. - State field initialization:
scrollOffsetreset via existinguseEffect([maxScroll])dependency;contentWidthresize correctly triggers re-wrap via useMemo. - Sibling code consistency:
NODE_ENV === 'test'escape hatch matches VirtualizedList's established pattern. - Resource leaks: Timer cleanup verified in useEffect destructor; no lingering intervals.
Nit
- ThinkingViewer
maxScrollon resize: If the terminal is resized narrower while scrolled down,contentWidthdecreases → more visual lines →maxScrollincreases → the existinguseEffectclampsscrollOffsetto the newmaxScroll(not reset to 0). This is correct but the user's relative scroll position may shift. This is acceptable behavior for a minor edge case.
Final Verdict
LGTM — Three well-scoped, well-tested bug fixes with clean implementations. The scroll coalescing pattern is standard and correctly implemented with proper cleanup. The layout and truncation fixes are straightforward with no hidden side effects. Comments are excellent throughout — every design decision has an inline rationale.
This review was generated by QoderWork AI
2a. Code reviewIndependent proposal (before reading the diff): for VP-mode truncation, I'd pre-wrap thinking text at The PR's approach matches or exceeds my proposal in each case. The Code review findings: no blockers. The 2b. Real-scenario testingRan both the installed Before (installed build, main branch)Note the ~19 blank rows between the response content and the composer — the wasted layout space the PR fixes. After (this PR, bundled build)The composer now sits directly below the content — no wasted blank rows. Layout fix confirmed visually. Scroll smoothness and ThinkingViewer truncation require interactive mouse/keyboard testing that can't be fully automated in tmux, but the unit tests cover the coalescing logic (including the press-cancels-wheel regression) and the pre-wrapping math. — Qwen Code · qwen3.7-max |
|
This is the kind of PR that makes maintainers happy to hit approve. Three real, user-visible bugs in VP mode, each fixed with a minimal, targeted change. The truncation fix is the most satisfying — a one-line conceptual shift from "split on The before/after tmux capture confirms the layout fix works as described — ~19 blank rows eliminated, composer hugging the content. All 139 unit tests pass, including a new regression test for the press-cancels-wheel edge case. My independent proposal matched the PR's approach in each case, which is a good sign — the fixes feel obvious once you see them, which is the hallmark of a good bug fix. Approving. ✅ 中文说明这种 PR 让维护者很乐意点通过。 VP 模式下三个真实的、用户可感知的 bug，每个都用最小化、有针对性的修改来修复。截断修复最令人满意——从"按 前后 tmux 截图确认布局修复按描述工作——消除了约 19 行空白，输入区紧贴内容。全部 139 个单元测试通过，包括按压取消滚轮边界情况的新回归测试。 我的独立提案在每个方面都与 PR 方案一致，这是个好信号——修复在看到之后感觉很自然，这正是好的 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. ✅
| // Terminal mouse reporting emits one event per row the pointer crosses, so a | ||
| // brisk wheel spin or scrollbar drag fires a rapid burst. Applying each event | ||
| // synchronously forced one Ink reflow + terminal flush per event — the source | ||
| // of the "一顿一顿" stutter. Coalesce a burst into a single viewport update per |
There was a problem hiding this comment.
[Suggestion] The NODE_ENV==='test' escape hatch in scheduleScrollFlush (line 140) means all existing ScrollableList tests exercise the synchronous path. The production timer-based coalescing — the core behavioral change of this PR (~30 lines covering setTimeout, the single-timer guard, drag-over-wheel priority, and unmount cleanup) — is never tested.
Consider adding tests with vi.useFakeTimers() to cover:
- Multiple wheel events within one 16ms window produce a single
scrollBycall with the summed delta - A drag event in the same window as wheel events causes the drag to win
clearTimeoutis called on unmount when a flush is pending
— qwen3.7-max via Qwen Code /review
| const contentHeight = Math.max(rows - headerHeight - footerHeight, 1); | ||
|
|
||
| const lines = useMemo(() => data.text.split('\n'), [data.text]); | ||
| // The thought text is frequently a single long paragraph with no explicit |
There was a problem hiding this comment.
[Suggestion] ThinkingViewer.test.tsx does not exist. The core bug fix — pre-wrapping thought text to visual lines instead of splitting on \n — has no regression test. The motivating bug (a single long paragraph collapsing to one ellipsised row) could silently reappear if wrapToVisualLines or the contentWidth formula changes.
Consider adding tests covering:
- A single long paragraph with no newlines (the motivating bug)
- Narrow terminals where
columns - 4yields small widths - Empty
data.text - Terminal resize (changing
columnsbetween renders)
— qwen3.7-max via Qwen Code /review
| const MAX_STREAMING_THINKING_VISUAL_LINES = 4; | ||
|
|
||
| function wrapToVisualLines(text: string, width: number): string[] { | ||
| export function wrapToVisualLines(text: string, width: number): string[] { |
There was a problem hiding this comment.
[Suggestion] wrapToVisualLines is now export-ed and consumed by two callers (AssistantMessageContent and ThinkingViewer), but has no standalone unit tests. The function has non-trivial logic (character-by-character width accumulation, line-break decisions, empty-line handling) that warrants a dedicated test contract.
Consider adding tests for: basic word wrap, text with no spaces, embedded \n, CJK character widths, and width <= 0.
— qwen3.7-max via Qwen Code /review
A wheel burst schedules a 16ms coalescing flush. If the user clicked the scrollbar within that window, the press applied its row immediately but the still-armed timer then fired `scrollBy` with the leftover wheel delta, yanking the view off the clicked row. Clear the pending wheel/drag intent and cancel the timer when a scrollbar press takes over. Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Zeroing the VP layout reservation removed all slack, so when the composer grew (multi-line input) the one-frame-late controlsHeight measurement briefly oversized the list and overflowed the terminal — the exact jitter the layout change aimed to remove. Drop only the Static-specific staticExtraHeight (3) and keep MAIN_CONTENT_HEIGHT_RESERVATION (2) as a transient-measurement buffer; the composer still sits far closer to the bottom than before (was ~5 stranded rows). Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
It lived in the 1000-line ConversationMessages component and ThinkingViewer imported it across components. Co-locate it with its sibling visual-wrapping helper (sliceTextByVisualHeight) in utils/textUtils.ts and import from there. Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
| if (isDraggingScrollbar.current) { | ||
| // A press should feel instant — apply now and drop any stale | ||
| // pending drag row from a previous gesture. | ||
| pendingDragRow.current = null; |
There was a problem hiding this comment.
[Suggestion] The left-press handler resets pendingDragRow but not pendingWheelDelta. If a wheel tick lands in the same ~16ms coalescing window just before a scrollbar click, the press applies its absolute scrollToScrollbarRow(event.row) immediately, but the already-scheduled timer still fires applyPendingScroll afterwards — dragRow is now null, so it falls through to scrollBy(wheelDelta) with the stale delta. The viewport snaps to the clicked row, then drifts ±3 lines ~16ms later. Mirror the pendingDragRow reset so a new gesture drops stale wheel intent too (the adjacent comment on the preceding lines can be reworded to say "drag row / wheel delta"):
| pendingDragRow.current = null; | |
| pendingDragRow.current = null; | |
| pendingWheelDelta.current = 0; |
— claude-opus-4-8 via Qwen Code /qreview
✅ Local real-binary verification report (maintainer)I built two real TL;DR — all three fixes reproduce and resolve as described. Safe to merge. One non-blocking gap: the PR adds no regression tests, and 3 of the 4 changes are currently unguarded.
Fix 1 — Thought viewer truncation ✅The full-screen
Fix 2 — Wasted blank area / composer position ✅ (tmux A/B)Short content (1 turn): Overflow (7 turns): both scroll correctly (scrollbar appears, old turns scroll off the top — PR's "scroll math unchanged" holds). Difference from the reservation drop:
Both sub-fixes behave exactly as the description claims, and the blast radius is contained to VP mode ( Fix 3 — Choppy scroll coalescing ✅ (functional)In the real binary the coalescing timer path is live (it's bypassed only under
Scroll correctness is guarded by the
|
| Reverted fix | Suite result | Regression-guarded? |
|---|---|---|
Fix 1 wrapToVisualLines → split('\n') |
10 pass | ❌ no |
Fix 2a min(containerHeight, totalHeight) → containerHeight |
34 pass | ❌ no |
| Fix 2b drop reservation → always subtract | pass | ❌ no |
| Fix 3 wheel sign flip (positive control) | 1 fail | ✅ yes (correctness only) |
Recommend (follow-up, not a merge blocker) adding small regression tests for the three visual behaviors — e.g. a ThinkingViewer render test (the A/B harness above drops in directly).
Notes (pre-existing, not caused by this PR)
- A duplicated banner top-line redraw residue is visible in base; head's settled frames were clean in my runs.
- The full-screen viewer open + scrollbar drag rely on click hit-testing inside the virtualized viewport, which doesn't map cleanly to injected coordinates — a pre-existing VP geometry quirk, unrelated to this change.
Verdict: 👍 the three fixes work as described, build/lint/typecheck/tests are green, and the change is contained to VP mode + the thought viewer. Recommend merge, with a follow-up to add regression tests.
🇨🇳 中文版（点击展开）
✅ 本地真实二进制验证报告（维护者）
我在隔离的 worktree 中构建了两个真实 qwen 二进制——PR head（46f168d9b）与 merge-base（b73736489），均 npm ci && npm run build，并在真实 tmux 面板（100×40，TERM=xterm-256color，ui.useTerminalBuffer: true）中做 A/B 对比。由一个伪 OpenAI 端点驱动，流式返回一段超长的单段落 reasoning_content（7061 字符，无换行）+ 一句简短回答。伪服务器记录到 64 次 /chat/completions 请求，证明真实二进制被端到端驱动。
结论速览——三个修复均能复现并按描述解决，可以合并。一个非阻塞项：PR 未新增任何回归测试，4 处改动中有 3 处当前无测试守护。
| 检查 | 结果 |
|---|---|
npm ci && npm run build（两个 worktree） |
✅ exit 0 |
tsc --noEmit（cli） |
✅ 通过 |
eslint + prettier --check（5 个改动文件） |
✅ 通过 |
vitest ScrollableList / VirtualizedList / MainContent / ConversationMessages / AppContainer |
✅ 152 通过 |
| Ubuntu CI（完整套件） | ✅ 绿 |
修复 1 — 思考查看器截断 ✅
全屏 ThinkingViewer 只能通过点击折叠的思考块打开；该点击命中检测走虚拟化视口的测量几何，我无法通过 tmux 确定性地驱动（注入的屏幕坐标无法映射到 yoga 测量的元素位置——这是既有的 VP 怪癖，超出本 PR 范围）。因此我用 ink-testing-library A/B 测试夹具直接验证真实组件（相同 KeypressProvider、真实 wrapToVisualLines、超长无换行思考），在两个分支上分别运行：
| 断言 | BASE（split('\n')） |
HEAD（wrapToVisualLines） |
|---|---|---|
| 可见内容行数 | 1（单行截断）❌ | >5 行换行 ✅ |
滚动指示 (0%) 出现（maxScroll>0） |
无 ❌ | 有 ✅ |
能滚到末尾（可见 Step 60） |
永远不能 ❌ | 能 ✅ |
HEAD 3/3 通过 · BASE 3/3 失败——正是"塌缩成一行省略号、maxScroll 恒为 0"的 bug，已修复。真实二进制旁证：option+t 内联展开能看到完整 60 步思考正确换行。
修复 2 — 大片空白 / 输入框位置 ✅（tmux A/B）
短内容（1 轮）： BASE 在回答（第 17 行）与输入框（第 32 行）之间有 13 行浪费空白；HEAD 中输入框（第 29 行）紧贴在内容下方，空白消失。
溢出（7 轮）： 两者都能正确滚动（出现滚动条、旧轮次从顶部滚出——PR 的"滚动数学不变"成立）。reservation 去除的差异：
- BASE：输入框第 33 行，footer 下方残留 5 行空白，内容视口 ≈19 行。
- HEAD：输入框第 36 行，下方仅 2 行空白，内容视口 ≈24 行（回收约 5 行）。
两个子修复行为完全符合描述，影响范围限定在 VP 模式（VirtualizedList/ScrollableList 仅 VP 使用；reservation 改动显式为 useTerminalBuffer ? 0 : …）。
修复 3 — 卡顿滚动合并 ✅（功能性）
真实二进制中合并定时器路径是生效的（仅在 NODE_ENV==='test' 下被旁路）。每次 8 个滚轮事件的突发都能正确合并并落到正确位置：
- 8× 上滚 → 回退到历史（Topic 6 → Topic 3 区域）
- 8× 下滚 → 回到最新（Topic 6）✅
滚动正确性由 ScrollableList 套件守护——把滚轮增量取反（scrollBy(-wheelDelta)）的变异会让 routes wheel SGR events to viewport scroll 失败，证明测试对新代码是有效的。滚动条拖拽无法经 tmux 驱动（同样的几何限制），但有 4 个通过的拖拽单测覆盖。逐帧批处理本身（性能收益）架构上合理，但没有任何测试用计时断言它（测试下按设计是同步的）。
⚠️ 测试覆盖缺口（非阻塞）——经变异测试确认
PR 改了 5 个源文件，新增 0 个测试。逐一回退每个修复并重跑相关套件：
| 回退的修复 | 套件结果 | 有回归守护？ |
|---|---|---|
修复 1 wrapToVisualLines → split('\n') |
10 通过 | ❌ 无 |
修复 2a min(containerHeight, totalHeight) → containerHeight |
34 通过 | ❌ 无 |
| 修复 2b 去除 reservation → 总是相减 | 通过 | ❌ 无 |
| 修复 3 滚轮符号取反（正向对照） | 1 失败 | ✅ 有（仅正确性） |
建议（后续，非合并阻塞）为这三个视觉行为补少量回归测试——例如一个 ThinkingViewer 渲染测试（上面的 A/B 夹具可直接落地）。
备注（既有问题，非本 PR 引入）
- base 中可见一条重复的 banner 顶行重绘残留；head 稳定后的帧在我的运行中是干净的。
- 全屏查看器打开 + 滚动条拖拽都依赖虚拟化视口内的点击命中检测，与注入坐标不能干净映射——既有 VP 几何怪癖，与本改动无关。
结论：👍 三个修复均按描述生效，build/lint/typecheck/测试全绿，改动限定在 VP 模式 + 思考查看器。建议合并，并后续补充回归测试。
Extract the per-frame scroll coalescing into useFrameCoalescedFlush and use it from both ScrollableList and ThinkingViewer (whose wheel handler was still un-batched, so a brisk spin in the expanded thought viewer stuttered). Drop the NODE_ENV==='test' escape hatch that made the production timer path unreachable in tests: the mouse-scroll tests now advance a real frame before asserting, exercising the batching/accumulation/precedence logic. Adds a regression test for a scrollbar press canceling a still-pending wheel flush. Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
The static/<Static> path reserves no blank rows under the composer — the staticExtraHeight + MAIN_CONTENT_HEIGHT_RESERVATION budget only caps inline streaming-message height, while the composer flows to the very bottom of the terminal. Reserve nothing in VP so its composer reaches the bottom the same way, instead of leaving a 2-row gap. The one-frame controlsHeight measurement lag on composer growth mirrors legacy mode letting the terminal scroll. Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
| let currentWidth = 0; | ||
| for (const char of logicalLine) { | ||
| const charWidth = getCachedStringWidth(char); | ||
| if (currentWidth + charWidth > width && currentWidth > 0) { |
There was a problem hiding this comment.
[Suggestion] wrapToVisualLines uses raw getCachedStringWidth(char) here, but sliceTextByVisualHeight in the same file (line 221) clamps with Math.max(getCachedStringWidth(char), 1). Zero-width characters (combining marks like U+0300–U+036F, ZWJ U+200D) accumulate 0 width, so a sequence like "a" + "\u0301".repeat(100) would be emitted as a single visual line far wider than the terminal — the ThinkingViewer then truncates it with wrap="truncate-end" and maxScroll undercounts the actual rows.
Now that wrapToVisualLines is a shared exported utility with two callers, it should match the defensive floor used by its sibling.
| if (currentWidth + charWidth > width && currentWidth > 0) { | |
| const charWidth = Math.max(getCachedStringWidth(char), 1); |
— qwen3.7-max via Qwen Code /review
| } | ||
| if (event.name === 'left-press') { | ||
| isDraggingScrollbar.current = | ||
| virtualizedListRef.current.hitTestScrollbar(event); |
There was a problem hiding this comment.
[Suggestion] left-release sets isDraggingScrollbar.current = false but doesn't call cancelPendingScroll(). If a drag move event set pendingDragRow and scheduled a 16ms flush timer, and the user releases before the timer fires, the timer still executes applyPendingScroll() which reads the stale dragRow and calls scrollToScrollbarRow() after the user has already released — the viewport jumps to the last drag position.
The fix is one line:
| virtualizedListRef.current.hitTestScrollbar(event); | |
| if (event.name === 'left-release') { | |
| isDraggingScrollbar.current = false; | |
| cancelPendingScroll(); | |
| return; | |
| } |
— qwen3.7-max via Qwen Code /review
| // with `wrap="truncate-end"`, collapsed to a single ellipsised row above an | ||
| // empty box (and `maxScroll` stayed 0, so it could not scroll). Pre-wrap to | ||
| // visual rows at the inner content width — border (1 each side) + paddingX | ||
| // (1 each side) = 4 columns — so scrolling and rendering operate on the same |
There was a problem hiding this comment.
[Suggestion] contentWidth hardcodes the chrome offset as columns - 4 (border 1+1, paddingX 1+1). This is the only link between the pre-wrap width passed to wrapToVisualLines and the actual inner width that <Text wrap="truncate-end"> renders into. If anyone later changes paddingX, borderStyle, or adds an inner wrapper, the pre-wrap width silently diverges from the render width — lines get truncated with ellipsis or waste right-margin space, with no compile-time or runtime signal.
Consider extracting a named constant (e.g., THINKING_VIEWER_CHROME_WIDTH = 4) co-located with the render JSX so both stay in sync.
— qwen3.7-max via Qwen Code /review
wenshao
left a comment
wenshao
left a comment
There was a problem hiding this comment.
No review findings. LGTM! ✅
— qwen3.7-max via Qwen Code /review
TUI autotest evidence (macOS / VHS, local build of this branch)Recorded by driving the branch's own bundled CLI under VHS (1400×900, FontSize 14), comparing VP ( 1. Short content — input box hugs content (matches legacy)VP no longer strands a large blank gap; the composer sits right under the content, identical to the static path.
2. Full viewport — composer sinks to the bottomWith the conversation overflowing, the input + footer reach the bottom of the terminal (the prior behavior stranded ~5 rows).
3. Multi-line input in a full viewport — no flickerTyping a long wrapping line grows the composer while the list above sheds rows. Programmatic check over the 451 frames of the typing window: the footer's bottom row stayed pinned at y=877 (span = 0px, zero overflow frames) — no visible jump with the zero bottom-reservation that matches static.
Note on the thinking viewer (fix 1)The full-screen 🤖 Generated with Qwen 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. ✅
chiga0
changed the title
| flush: () => void, | ||
| frameMs: number = SCROLL_FRAME_MS, | ||
| ) { | ||
| const timer = useRef<ReturnType<typeof setTimeout> | null>(null); |
There was a problem hiding this comment.
[Suggestion] No dedicated unit test for this new hook. It's now used by both ScrollableList and ThinkingViewer, but the cancel path — critical for preventing a stale wheel delta from overriding a scrollbar press — has no direct test coverage.
Consider adding use-frame-coalesced-flush.test.ts covering:
schedule()fires the flush exactly once afterframeMs- Multiple
schedule()calls withinframeMscoalesce to one flush cancel()prevents a pending flush from firing- Cleanup on unmount clears the pending timer
— qwen3.7-max via Qwen Code /review
3 participants
What this PR does
Fixes three visual and interaction bugs in terminal-buffer mode (
ui.useTerminalBuffer/ VP mode) to bring its behavior in line with the legacy<Static>rendering path:<Static>), and the VP-specific height reservation that guarded against overflow flicker (unnecessary since VP clips natively) is removed.Why it's needed
VP mode is the newer rendering path, but these bugs made it noticeably worse than the legacy path — truncated thinking blocks, wasted screen space, and janky scrolling. Users switching to VP mode would have a degraded experience. These fixes close the gap so VP mode is production-ready.
Reviewer Test Plan
How to verify
Enable VP mode (
ui.useTerminalBuffer: true) and test each fix:tto open the full-screen viewer. Verify the full thought is readable and scrollable (no single-line truncation).Evidence (Before & After)
N/A — reviewer can verify interactively.
Tested on
Environment (optional)
npm run devRisk & Scope
requestAnimationFrame-style coalescing — underNODE_ENV==='test'updates remain synchronous so existing tests pass unchanged.<Static>) path is untouched; no behavioral changes there.Linked Issues
N/A
中文说明
本 PR 做了什么
修复终端缓冲区模式（
ui.useTerminalBuffer/ VP 模式）中的三个视觉和交互 bug，使其行为与旧版<Static>渲染路径保持一致：<Static>），并移除了 VP 模式中不必要的高度预留（VP 原生裁剪，不需要防闪烁保护）。为什么需要
VP 模式是较新的渲染路径，但这些 bug 使其体验明显不如旧版路径 — 截断的思维块、浪费的屏幕空间和卡顿的滚动。切换到 VP 模式的用户会有降级的体验。这些修复缩小了差距，使 VP 模式达到生产就绪状态。
风险与范围
requestAnimationFrame风格的合并 — 在NODE_ENV==='test'下更新保持同步，现有测试无需更改即可通过。<Static>）路径未改动，无行为变化。🤖 Generated with Qwen Code