Skip to content

perf(cli): reduce virtualized history scroll latency - #10043

Open
DragonnZhang wants to merge 5 commits into
QwenLM:mainfrom
DragonnZhang:dragon/virtualized-history-scroll-performance
Open

perf(cli): reduce virtualized history scroll latency#10043
DragonnZhang wants to merge 5 commits into
QwenLM:mainfrom
DragonnZhang:dragon/virtualized-history-scroll-performance

Conversation

@DragonnZhang

Copy link
Copy Markdown
Collaborator

What this PR does

This change makes Virtualized History scroll scheduling leading-edge and deadline-aware. The first wheel or drag update now applies immediately, later updates within the same 16 ms window remain coalesced, and an update arriving after an over-budget render no longer waits through another full frame interval. Accumulated wheel deltas and scrollbar cancellation semantics remain unchanged.

It also adds focused coverage for immediate leading updates, remaining-deadline scheduling, over-budget scheduling, burst coalescing, and cancellation, and records the profiling evidence and non-goals behind the change.

Why it's needed

Real PTY profiling showed that the input parser was inexpensive, while each emitted Ink frame spent about 35 ms in layout, composition, ANSI diffing, and terminal output. The previous trailing-only scheduler added a fresh 16 ms delay on top of that work, causing sluggish first response and low visible frame rate even though wheel deltas were preserved.

Removing the avoidable scheduling delay improves responsiveness without introducing a larger rendering or indexing rewrite.

Reviewer Test Plan

How to verify

Enable Virtualized History, open a session with enough history to scroll, and send a sustained burst of SGR wheel events. Confirm that the first wheel event updates the viewport immediately, subsequent events are coalesced without losing their accumulated row delta, and clicking the scrollbar cancels any pending trailing wheel update.

Run the focused CLI tests for the frame coalescer, scrollable list, and virtualized list. Confirm all tests pass, then run the repository build and typecheck.

Evidence (Before & After)

Metric Before (global 0.22.0) After (local candidate)
First visible output after wheel input 29.01 ms 11.11 ms
Sustained synchronized output 18.71 FPS 31.75 FPS
Synchronized frames in the same capture 16 25
Wheel distance per SGR tick 3 rows 3 rows

The same 100x30 real-PTY workload and 80-event oscillating wheel sequence were used for both captures. The change reduced first-output latency by about 61.7% and increased visible frame rate by about 69.7% while retaining the terminal row delta.

Tested on

OS Status
🍏 macOS
🪟 Windows ⚠️
🐧 Linux ⚠️

Environment (optional)

macOS real PTY at 100x30, Node.js 22-compatible workspace runtime, global Qwen Code 0.22.0 baseline, and the locally built candidate. Validation also included 52 focused Vitest tests, focused ESLint, the full repository build, and the full repository typecheck.

Risk & Scope

  • Main risk or tradeoff: the first update in an idle burst now renders immediately; continued input is still limited to one application flush per frame window.
  • Not validated / out of scope: Windows and Linux terminals, pixel-smooth touchpad scrolling, host terminal scrollback while alternate-screen Virtualized History is enabled, Ink incremental rendering, and the separate O(item-count) offsets rebuild seen only in very large resumed histories.
  • Breaking changes / migration notes: none. Users who prefer native terminal scrollback can continue to disable ui.useTerminalBuffer and restart.

Linked Issues

N/A

中文说明

本 PR 做了什么

此更改让 Virtualized History 的滚动调度同时支持前沿立即执行和基于帧截止时间的调度。第一个滚轮或拖动更新现在会立即生效,同一个 16 ms 窗口内的后续更新仍会合并;当一次渲染已经超过帧预算时,后续更新也不再额外等待完整的一帧。滚轮增量累积和滚动条取消语义保持不变。

同时新增了针对首次立即更新、剩余截止时间调度、超预算调度、突发事件合并和取消行为的集中测试,并记录了本次修改所依据的性能分析证据和非目标。

为什么需要它

真实 PTY 性能分析表明,输入解析开销很小,而 Ink 每个可见输出帧会在布局、组合、ANSI 差异计算和终端输出上花费约 35 ms。之前仅尾沿执行的调度器还会在这些工作之外新增完整的 16 ms 等待,因此即使滚轮增量没有丢失,首次响应仍然迟缓,可见帧率也较低。

移除这段可避免的调度等待可以提升响应速度,同时不需要引入更大范围的渲染或索引结构重写。

Reviewer Test Plan

如何验证

启用 Virtualized History,打开一段足够长、可以滚动的会话历史,然后持续发送一组 SGR 滚轮事件。确认第一个滚轮事件会立即更新视口,后续事件会被合并但不会丢失累计行数,并且点击滚动条会取消仍在等待的尾沿滚轮更新。

运行帧合并器、可滚动列表和虚拟列表相关的 CLI 集中测试,确认全部通过,然后运行仓库全量构建和类型检查。

证据(修改前后)

指标 修改前(全局 0.22.0) 修改后(本地候选版本)
滚轮输入后的首次可见输出 29.01 ms 11.11 ms
持续同步输出 18.71 FPS 31.75 FPS
同一次采集中的同步帧数 16 25
每个 SGR 滚轮事件的移动距离 3 行 3 行

两次采集使用相同的 100x30 真实 PTY 工作负载和包含 80 个事件的往返滚轮序列。此更改将首次输出延迟降低约 61.7%,将可见帧率提升约 69.7%,同时完整保留终端行增量。

测试平台

操作系统 状态
🍏 macOS
🪟 Windows ⚠️
🐧 Linux ⚠️

环境(可选)

macOS 真实 PTY(100x30)、兼容 Node.js 22 的工作区运行时、全局 Qwen Code 0.22.0 基线版本和本地构建的候选版本。验证还包括 52 个集中 Vitest 测试、集中 ESLint、仓库全量构建和仓库全量类型检查。

风险与范围

  • 主要风险或取舍:空闲事件序列中的第一个更新现在会立即触发渲染;持续输入仍限制为每个帧窗口最多应用一次刷新。
  • 未验证或不在范围内:Windows 和 Linux 终端、触控板像素级平滑滚动、启用 alternate-screen Virtualized History 时的宿主终端滚动历史、Ink 增量渲染,以及仅在超大型恢复历史中观察到的独立 O(item-count) 偏移数组重建。
  • 破坏性变更或迁移说明:无。希望使用终端原生滚动历史的用户仍可关闭 ui.useTerminalBuffer 并重启。

关联 Issue

@DragonnZhang

Copy link
Copy Markdown
Collaborator Author

E2E / PTY test report

Tested on macOS with a real 100x30 PTY using the same history workload and 80-event oscillating SGR wheel sequence for the global 0.22.0 baseline and the locally built candidate.

Check Result
First visible output 29.01 ms → 11.11 ms
Sustained synchronized output 18.71 FPS → 31.75 FPS
Synchronized frames 16 → 25
Wheel delta Preserved at 3 terminal rows per SGR tick
Alternate-screen protocol Unchanged
Button-motion mouse reporting Unchanged

Additional validation:

  • Focused Vitest: 52/52 passed across the frame coalescer, scrollable list, and virtualized list.
  • Focused ESLint: passed.
  • Full repository build: passed.
  • Full repository typecheck, including integration types: passed.

The Inspector capture attributed roughly 35 ms per emitted frame to Ink/Yoga layout, composition, ANSI diffing, and terminal output. Mouse parsing was negligible. A separate O(item-count) offsets rebuild was observed only in very large synthetic resumed histories and is intentionally outside this PR.

@qwen-code-ci-bot

qwen-code-ci-bot commented Aug 25, 2026

Copy link
Copy Markdown
Collaborator

Qwen Triage finishedview run. See the stage comments in this thread for the result.

Qwen Triage 已完成 —— 查看运行。结果见本线程中的各阶段评论。

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Thanks for the PR!

  • Template: complete ✓ — all sections present, including the bilingual summary.
  • Problem: observed, not theoretical. The PR is grounded in real PTY profiling: ~35 ms per emitted Ink frame plus a fixed 16 ms trailing wait added by the scheduler, with before/after captures (29.01 ms → 11.11 ms first output; 18.71 → 31.75 FPS sustained). The mechanism is clearly identified — trailing-only scheduling delays the first update of every burst by a full frame, and an over-budget render pays another full wait.
  • Direction: aligned. Virtualized History is an actively developed feature (the author shipped related work in fix(cli): enable footer text selection in VP mode #8329), and scroll responsiveness inside the alternate-screen viewport is core TUI UX. CHANGELOG: no direct "scroll latency" entry in the reference product, but scroll UX fixes (jumping content, scroll position, fullscreen scrollback) recur there — the area is clearly relevant.
  • Size: no core paths touched — all changes are in packages/cli/src/ui/** plus a design doc. Production logic is 25 lines (one hook), tests 85 lines, docs 37 lines. Core-module gate not applicable.
  • Approach: minimal and focused — it modifies the existing useFrameCoalescedFlush hook rather than adding a parallel mechanism: leading-edge flush for the first update of a burst, deadline-aware trailing flush for the rest, immediate flush when the previous render already overran the frame budget. Accumulated wheel deltas, drag precedence, and scrollbar-cancellation semantics are stated as unchanged. This is the smallest change that addresses both identified latency sources. The design doc under docs/design/ records the evidence and explicit non-goals, which matches repo convention.
  • Risk: no elevated risk signals — none of the changed files match the revert-correlated high-risk paths.

Moving on to code review. 🔍

中文说明

感谢贡献!

  • 模板:完整 ✓ —— 所有章节齐全,包含双语说明。
  • 问题:已观测到,不是理论问题。PR 基于真实 PTY 性能分析:每个 Ink 可见输出帧约 35 ms,而调度器还会额外叠加固定的 16 ms 尾沿等待;并提供了 before/after 数据(首次输出 29.01 ms → 11.11 ms;持续帧率 18.71 → 31.75 FPS)。机理定位清晰——仅尾沿的调度让每个突发序列的第一个更新都要等满一帧,而一次超预算渲染之后还要再等一个完整窗口。
  • 方向:对齐。Virtualized History 是活跃开发中的功能(作者此前贡献过相关的 fix(cli): enable footer text selection in VP mode #8329),alternate-screen 视口内的滚动响应速度是核心 TUI 体验。CHANGELOG:参考产品没有直接的"滚动延迟"条目,但滚动体验类修复(内容跳动、滚动位置、全屏滚动历史)反复出现——这个方向明显相关。
  • 规模:未触及核心路径——所有改动都在 packages/cli/src/ui/** 加一篇设计文档。生产逻辑 25 行(单个 hook),测试 85 行,文档 37 行。核心模块门禁不适用。
  • 方案:最小且聚焦——直接修改现有的 useFrameCoalescedFlush hook,而不是新增一套并行机制:突发序列的第一个更新前沿立即执行,同窗口内的后续更新按剩余截止时间尾沿合并,上一次渲染已超出帧预算时下一个更新立即执行。滚轮增量累积、拖动优先和滚动条取消语义声明保持不变。这是解决两个已定位延迟来源的最小改动。docs/design/ 下的设计文档记录了证据和明确的非目标,符合仓库惯例。
  • 风险:无升级风险信号——改动文件均未命中与 revert 相关的高风险路径。

进入代码审查。🔍

Qwen Code · qwen3.8-max

Reviewed at 6bde498b571ae4838a083e7f2b9f805027298412 · re-run with @qwen-code /triage

@qwen-code-ci-bot

qwen-code-ci-bot commented Aug 25, 2026

Copy link
Copy Markdown
Collaborator

Code review

Read the diff against the current main implementation. The change does exactly what the description promises, and nothing else: a lastFlushAt ref stamps when the last flush started, schedule() flushes immediately when no flush has happened yet or the previous frame already overran its deadline, and otherwise arms the trailing timer at the remaining deadline instead of a fresh 16 ms. No blockers found. Details I verified:

  • The hook has a single consumer (ScrollableList), and schedule() is only called from mouse event handlers — so the new synchronous leading flush can't fire during render, and React batches the resulting state update inside the handler. No new failure mode: applyPendingScroll already guards a missing list ref.
  • The once-per-frame cap is preserved: a pending timer still short-circuits schedule(), and sustained input still averages one flush per 16 ms window — the burst just renders its first frame immediately instead of one window late. No extra render load per burst.
  • Cancellation semantics are intact (cancel() clears the pending timer; lastFlushAt correctly survives it), so the scrollbar-press path that drops a queued wheel burst still works — and the updated ScrollableList.test.tsx scenario remains valid: the leading wheel now scrolls +3 immediately, but the press re-pins the top and the canceled trailing delta still must not fire.
  • The deadline tests assume performance.now() advances with vitest's fake timers — that matches existing repo precedent (useTimer.test.ts asserts sub-second performance.now() deltas via advanceTimersByTime), and the CLI vitest config doesn't override the default timer mocks.
  • Style follows conventions: license header, kebab-case file, .js import extensions, no any, and the hook's doc comment was updated to match the new behavior rather than left stale.

No Critical issues, no AGENTS.md violations.

Testing evidence — this PR's CI (fetched via API; per gate rules I do not run PR code)

At the time of this pass, the three platform unit-test lanes are still running on the reviewed commit; security, precheck, and desktop-shell checks are green, and the conditional jobs (integration, tmux, verify) are skipped pending their triggers. Not verified: the live latency/FPS improvement — the unit suite pins the scheduling semantics, not the end-to-end timing win, and the before/after numbers in the PR description are the author's own macOS PTY measurements. I'm not treating them as independent evidence.

Final CI results for 6bde498 (auto-updated by the triage finalize job after CI completed):

Check Conclusion
Test (macos-latest, Node 22.x) 🚫 cancelled
Test (windows-latest, Node 22.x) 🚫 cancelled
Classify platform sensitivity ✅ success
Classify PR ✅ success
Dependency CVE audit ✅ success
Desktop Shell (ubuntu-22.04) ✅ success
Desktop Shell (windows-2022) ✅ success
Secret scan (TruffleHog) ✅ success
Test (ubuntu-latest, Node 22.x) ✅ success
web-shell E2E Smoke (ubuntu-latest, Node 22.x) ✅ success

One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行(取最新一次运行),省略 skipped,失败项排在最前。

Sandboxed verification would settle the remaining claim: the live responsiveness win (29 ms → 11 ms first output, 18.7 → 31.7 FPS) is currently the author's measurement on macOS only, and a green suite can't substantiate it — @qwen-code /tmux would drive real wheel bursts in the TUI and show viewport updates and preserved row deltas, and @qwen-code /verify would A/B the scheduling behavior against the base build. The author has write access, so either lane can be triggered directly on this PR.

中文说明

代码审查

对照当前 main 上的实现读了 diff。改动与描述完全一致,没有夹带其他内容:新增 lastFlushAt ref 记录上一次 flush 的开始时间;schedule() 在尚未发生过 flush、或上一帧已超出截止时间时立即执行 flush,否则按剩余截止时间设置尾沿定时器,而不是重新等满 16 ms。未发现阻塞问题。核实过的细节:

  • 该 hook 只有一个使用方(ScrollableList),且 schedule() 只在鼠标事件处理器中调用——因此新的同步前沿 flush 不会在渲染期间触发,React 会在处理器内批处理相应的状态更新。没有新的失败路径:applyPendingScroll 已对 list ref 缺失做了保护。
  • 每帧一次的合并上限保持不变:存在待执行定时器时 schedule() 仍会直接返回,持续输入仍然平均每个 16 ms 窗口只应用一次——突发序列只是立即渲染第一帧,而不是晚一个窗口。每个突发序列的渲染负担没有增加。
  • 取消语义完好(cancel() 清除待执行定时器;lastFlushAt 正确地不受其影响),因此滚动条按下时丢弃排队滚轮突发的路径仍然有效——更新后的 ScrollableList.test.tsx 场景也依然成立:前沿滚轮现在会立即滚动 +3,但按下会重新钉住顶部,被取消的尾沿增量仍然不能生效。
  • 截止时间相关测试假设 performance.now() 随 vitest 假定时器推进——这与仓库既有先例一致(useTimer.test.ts 通过 advanceTimersByTime 断言基于 performance.now() 的亚秒级差值),且 CLI 的 vitest 配置没有覆盖默认定时器 mock。
  • 风格符合约定:license 头、kebab-case 文件名、.js 导入扩展名、无 any,且 hook 的文档注释已随新行为更新而非留着过时描述。

无 Critical 问题,无 AGENTS.md 违规。

测试证据 —— 本 PR 自身的 CI(通过 API 获取;按门禁规则不运行 PR 代码)

截至本次审查,三个平台的单元测试通道仍在评审中的提交上运行;安全扫描、预检和桌面 shell 检查已通过,条件触发的任务(集成、tmux、verify)处于跳过状态等待触发。未验证:真实的延迟/帧率提升——单元测试固定的是调度语义,不是端到端的时间收益;PR 描述中的 before/after 数字是作者本人在 macOS 上的 PTY 测量结果,不作为独立证据引用。

CI 明细见上方表格(终审工作流会在 CI 结束后原地更新该表)。

沙箱验证可以了结剩余的声明:真实的响应速度提升(首次输出 29 ms → 11 ms、帧率 18.7 → 31.7)目前仅是作者在 macOS 上的测量,绿色测试套件无法证实——@qwen-code /tmux 可以在真实 TUI 中驱动滚轮突发序列,展示视口更新和保留的行增量;@qwen-code /verify 可以对照基线构建做 A/B 调度行为验证。作者拥有 write 权限,可以直接在本 PR 上触发任一通道。

Qwen Code · qwen3.8-max

Reviewed at 6bde498b571ae4838a083e7f2b9f805027298412 · re-run with @qwen-code /triage

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Confidence: 4/5 — clean, minimal, evidence-grounded change; the only reservation is that the size of the latency/FPS win rests on the author's single-platform measurement, and CI was still running at review time.

Honest read: this is a well-constructed PR. Before reading the diff, the obvious fix for "trailing-only scheduler adds a fixed 16 ms on top of a 35 ms frame" was leading-edge flush plus remaining-deadline scheduling — which is exactly what landed, in 25 production lines on the existing hook, with tests that pin each new behavior and a design doc that records the profiling evidence and explicit non-goals. No simpler path was missed, nothing unrelated rode along, and the invariants that matter (once-per-frame cap, drag precedence, scrollbar cancellation, preserved wheel deltas) are all kept and tested.

The problem is real and measured, not theoretical: a fixed scheduling delay stacked on an already-expensive frame, with before/after captures whose numbers are internally consistent with the mechanism (removing one 16 ms wait from a ~51 ms per-frame path lands close to the reported 29 ms → 11 ms first output and ~2× frame-rate region). The one thing I can't independently confirm from here is the magnitude of the win on real terminals — that's the author's macOS PTY data, and Windows/Linux were declared out of scope; the CI unit suite settles the scheduling semantics on all three platforms but not the end-to-end timing. That gap is named in the Stage 2 comment together with the sandboxed lanes that would close it, and it's a reservation, not a blocker: the code change is safe and correct on its own terms.

Verdict: approve. CI is still running on the reviewed commit, so approval is deferred until CI lands green on 6bde498b571ae4838a083e7f2b9f805027298412.

中文说明

信心度:4/5 —— 干净、最小、有证据支撑的改动;唯一的保留意见是延迟/帧率提升的幅度目前只有作者单平台的测量数据,且审查时 CI 仍在运行。

直接说:这是一个结构良好的 PR。在读 diff 之前,针对"仅尾沿调度器在 35 ms 帧之上再叠加固定 16 ms"的显而易见修法就是前沿立即执行加剩余截止时间调度——实际落地的正是这个方案:25 行生产代码改在现有 hook 上,测试固定了每一种新行为,设计文档记录了性能分析证据和明确的非目标。没有错过更简路径,没有夹带无关改动,关键不变量(每帧一次上限、拖动优先、滚动条取消、滚轮增量保留)全部保留且有测试覆盖。

问题真实且经过测量,不是理论问题:在已经很昂贵的帧之上叠加了固定调度延迟,before/after 数据与机理自洽(从约 51 ms 的每帧路径中移除一次 16 ms 等待,与报告的 29 ms → 11 ms 首次输出和约 2 倍帧率区间吻合)。我无法从这里独立确认的只有真实终端上收益的幅度——那是作者在 macOS 上的 PTY 数据,Windows/Linux 声明为范围外;CI 单元测试在三个平台上固定了调度语义,但不覆盖端到端耗时。这个缺口已在 Stage 2 评论中与可以关闭它的沙箱通道一并指出;它是保留意见而非阻塞项:就代码改动本身而言是安全且正确的。

结论:批准。CI 仍在评审中的提交上运行,因此批准推迟到 CI 在该提交上全部变绿之后。

Qwen Code · qwen3.8-max

Reviewed at 6bde498b571ae4838a083e7f2b9f805027298412 · re-run with @qwen-code /triage

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Qwen Code review did not complete successfully. The review pipeline failed before a review could be posted. A transient error is retried automatically; if you are seeing this, retry with @qwen-code /review. See workflow logs.

@qqqys

qqqys commented Aug 26, 2026

Copy link
Copy Markdown
Collaborator

tmux E2E test report (head 6bde498b)

Review of the diff found no blocking (Critical) issues; per review policy, here is a functional e2e pass instead. Scope: verify the changed scroll scheduling does not regress virtualized-history scrolling behavior in a real TUI.

Setup. Checked out the PR head into a throwaway worktree, built packages/core, and launched the CLI from source (tsx packages/cli/index.ts) inside a tmux pane (200x50) with ui.useTerminalBuffer and ui.mouseTracking at their defaults (both on). Populated history with local-only commands (/about, /help, /stats — no model calls), then injected raw SGR wheel events (ESC[<64;x;yM wheel-up, ESC[<65;x;yM wheel-down) via tmux send-keys and captured the pane between bursts.

Results.

  • TUI boots cleanly on the PR build; virtualized viewport renders history; no crash or render corruption across all scroll bursts.
  • Wheel-down scrolls the viewport toward newer content; wheel-up scrolls back toward the banner/top. Both directions work.
  • Accumulation works: repeated wheel events move the viewport monotonically (a burst of 25 wheel-down events scrolled from mid-history to the composer; wheel-up returned to the first history item). No lost-direction or rubber-banding artifacts.
  • Boundary behavior sane: at the top, further wheel-up is a no-op (no crash); at the bottom, the composer/status line remains intact.

Not verified by this e2e (honest limits). tmux capture-pane granularity is too coarse to measure the sub-frame latency improvement this PR targets (leading-edge first apply, 29ms→11ms first-output, higher sustained FPS). In my capture a lone wheel event's movement was not distinctly separable from the following burst, so I can neither confirm nor refute the latency claim from tmux — that remains the author's real-PTY profiling territory. This e2e confirms functional correctness (no scroll regression), not the perf delta.

Supplementary (non-e2e). Focused unit tests pass on this head: use-frame-coalesced-flush.test.ts 5/5, ScrollableList.test.tsx + VirtualizedList.test.tsx 47/47.

CI note. Ubuntu lane is green on this head. The Windows lane failure's log was unavailable (expired) and could not be attributed to this PR — noting it for the author, given the repo's ongoing Windows-lane repair effort.


tmux E2E 测试报告(head 6bde498b

对 diff 的审查未发现阻塞性(Critical)问题;按评审规则,这里给出一次功能性 e2e 验证。目标:确认真实 TUI 下,改动后的滚动调度没有让虚拟历史滚动行为回归。

方法。 将 PR head 检出到临时 worktree,构建 packages/core,在 tmux(200x50)中以源码方式启动 CLI(ui.useTerminalBufferui.mouseTracking 均为默认开启)。用纯本地命令(/about/help/stats,不触发模型调用)填充历史,然后通过 tmux send-keys 注入原始 SGR 滚轮事件,并在每次突发之间抓取面板。

结果。

  • PR 构建正常启动;虚拟视口正常渲染历史;全部滚动突发过程中无崩溃、无渲染错乱。
  • 向下滚动移向更新内容,向上滚动回到顶部/横幅,两个方向均正常。
  • 累积位移正常:连续滚轮事件使视口单调移动(25 次向下滚动从历史中部滚到输入区;向上可回到首条历史项),无丢方向、无回弹异常。
  • 边界行为正常:顶部继续向上为空操作(不崩溃),底部输入区/状态行保持完整。

本 e2e 未能验证的部分(如实说明)。 tmux 抓取面板的粒度不足以度量本 PR 目标的亚帧延迟改进(首事件即时生效、首帧输出 29ms→11ms、更高的持续帧率)。在我的抓取中,单个滚轮事件的位移无法与随后的突发区分开,因此无法用 tmux 证实或证伪延迟收益——那仍属于作者真实 PTY 剖分的范畴。本 e2e 确认的是功能正确性(滚动无回归),而非性能增量。

补充(非 e2e)。 本 head 上焦点单测通过:use-frame-coalesced-flush.test.ts 5/5,ScrollableList.test.tsx + VirtualizedList.test.tsx 47/47。

CI 说明。 本 head 的 Ubuntu 通道为绿。Windows 通道的失败日志已过期、无法归因到本 PR——考虑到仓库正在修复 Windows 通道,此处向作者提示该情况。

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed — no blockers. Suggestions are inline.

中文说明

已审查——无阻断问题。 建议见行内评论。

— qwen3.8-max via Qwen Code /review (v0.22.0)

Comment thread docs/design/virtualized-history-scroll-scheduling.md Outdated
Comment thread docs/design/virtualized-history-scroll-scheduling.md
Comment thread packages/cli/src/ui/components/shared/ScrollableList.test.tsx
Comment thread packages/cli/src/ui/hooks/use-frame-coalesced-flush.ts
Address review feedback on QwenLM#10043:

- Add a ScrollableList mouse test that writes a single SGR wheel event
  to stdin and asserts the viewport moved exactly WHEEL_LINES_PER_TICK
  before any frame timer fires. This pins the PR's headline immediate
  first-wheel behavior through the real stdin -> handler -> schedule()
  pipeline; previously immediacy was only covered at the hook unit
  level, so a regression re-introducing pre-flush delay in the mouse
  path would have passed the whole component suite.

- Add a useFrameCoalescedFlush test that schedules twice (leading
  applies, trailing pending), unmounts, advances timers past the
  deadline, and expects exactly one flush. Guards the documented
  'timer is always cleared on unmount' contract; deleting the cleanup
  effect was previously a surviving mutant.
…acing coalescer decision

Address review feedback on QwenLM#10043:

- Label all performance figures in the new scheduling doc as
  single-machine observations (one macOS host, 100x30 real PTY,
  Node.js 22 runtime) that have not been independently reproduced, and
  note the capture scripts are not part of the tree.

- Record the before/after PTY comparison the Verification section
  previously described without results: capture environment plus the
  measured table (first-output 29.01 -> 11.11 ms, sustained 18.71 ->
  31.75 FPS, synchronized frames 16 -> 25, wheel distance 3 rows).

- Add a 'Relationship to the frame pacing doc' section that cites
  docs/design/2026-08-21-vp-scroll-frame-pacing.md and explains why
  its leading-and-trailing rejection no longer holds: the scheduler
  changes when the first application happens, not how far a burst
  scrolls — accumulated delta preservation is guarded by the
  burst-preservation unit test (exact 90-row delta) and the real-PTY
  row-delta capture (239/240 rows).

- Mark the pacing doc's trailing-coalescer bullet as superseded and
  point to the new doc so only one authoritative statement on VP
  scroll input coalescing remains.
@DragonnZhang

Copy link
Copy Markdown
Collaborator Author

@qwen-code /triage — CI on the current head is fully green now (the earlier deferred-approval hold cited a run that has since landed green); re-running triage for the deferred approval.

@qwen-code-ci-bot

qwen-code-ci-bot commented Aug 27, 2026

Copy link
Copy Markdown
Collaborator

Sandboxed verification: ✅ passed — merge-ready (agent verdict) - workflow run

Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check.

Scripted assertions: 212 passed · 0 failed · 212 total

Flakiness gate: ✅ 2 changed test file(s) x 5 identical rounds, no divergence

中文 — 判定:✅ 通过 · 可合入(agent 判定)

沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码(与 base 构建 A/B 对照、无 mock harness 断言、定向门禁)。仅作为评审证据,不构成评审、批准或 CI 检查

脚本断言:212 通过 · 0 失败 · 212 总计

抖动门:✅ 2 changed test file(s) x 5 identical rounds, no divergence

Verification report

PR #10043 — perf(cli): reduce virtualized history scroll latency

Verdict: merge-ready — 212/212 scripted assertions passed, 0 unexpected failures.
Verified head: 7c0aabcc79b2f63eff8da5de11d7cdf67fd2d56e (git rev-parse HEAD^2); merge base tip exercised: 5563a6c58035586327276fd35ae72dc4621b7c33 (HEAD^1).

中文摘要
  • 结论merge-ready。212 条脚本化断言全部通过,0 个意外失败;无阻塞性发现。
  • A/B 结论(head vs base,同一 harness 在两棵树各跑一遍):中央声明成立且 load-bearing。base 的首个滚轮事件要等满 16 ms 帧窗口(同 tick scrollTop 保持 0,实测延迟 16 ms);head 的首个滚轮事件在同一 tick 同步生效(scrollTop 立即为 3,延迟 0 ms)。两侧最终状态一致(burst 30 tick 均保留 90 行、滚动条点击均取消挂起的尾沿刷新),证明 PR 只移除调度延迟、不改变滚动语义。把 hook 还原为 base 实现后,PR 新增的 6 个 hook 测试与 1 个 ScrollableList 测试按预期失败(7 failed | 15 passed),证明测试不空转。
  • Findings:无阻塞性问题。两条非阻塞观察:组件层套件无法区分"每帧一次尾沿"与"总是立即"两种调度(该语义仅由 hook 层测试钉住);既有的 30 ms 真实定时器等待与 SCROLL_FRAME_MS=16 常量耦合(PR 未引入,未恶化)。
  • 未覆盖范围:真实 PTY 的 FPS/首帧数字(采集脚本不在仓库内,且为单机 macOS 观察值);Windows/Linux 终端、触控板平滑滚动、宿主 scrollback、O(item-count) 偏移重建(PR 自身声明的范围外);逐 commit 归因(depth-2 shallow,快照 4 个 commit 本地仅可达 1 个);全仓测试套件与全仓 build(环境已在 HEAD 完成 build,PR CI 覆盖全量;本轮跑定向滚动面 64/64 + packages/cli typecheck + ESLint)。

Scope selection

  • Central claim: the first wheel/drag update in an idle burst applies to the viewport immediately (leading edge) instead of after a fresh full 16 ms trailing delay.
  • Secondary 1: later updates in the same frame flush at the remaining deadline (and immediately once the deadline has passed), while a burst still coalesces to at most one application flush per frame window and loses no accumulated row delta.
  • Secondary 2: cancellation (scrollbar press) and unmount cleanup semantics are unchanged.

Changed surface is one 25-line hook (use-frame-coalesced-flush.ts, sole caller ScrollableList), two test files, and two docs. No package.json/lockfile changes, so reusing the root node_modules for the base control is dependency-neutral.

Central claim — A/B load-bearing proof

Control: git worktree add tmp/base-tree HEAD^1 (trailing-only scheduler confirmed in the base source). Both arms ran the identical harness (verify-ab-hook.test.ts, verify-ab-component.test.tsx in the artifact dir) through vitest with real timers, driving the real stdin → SGR parser → handleMouseEvent → schedule() pipeline; expectations parameterized by VERIFY_ARM. Internal workspace links: the cli vitest config aliases @qwen-code/qwen-code-core to each tree's own ../core/index.ts (base arm therefore compiles base core source; the PR touches no core file anyway), and third-party deps resolve through nested node_modules symlinked into the base worktree — readlink -f on both links confirmed they point at the head checkout's identical-version packages (lockfile unchanged). Witnesses: 01-ab-head-immediate-first-wheel.png, 02-ab-base-trailing-16ms-delay.png.

Cell (real timers) Oracle head base
First schedule() applies synchronously flush count at return 1 0
First SGR wheel byte → viewport moves same tick getScrollState().scrollTop 3 0
Latency to first viewport move poll until scrollTop==3 0 ms 16 ms
Trailing flush time after a 4-schedule burst flush timestamps 2 flushes @​ ~13/27 ms 1 flush @​ ~29 ms
schedule() after deadline passed immediate flush count 1 0 (fresh full wait)
30-tick burst delta preserved (195→105) scrollTop after flush 105 105
Scrollbar press cancels pending wheel flush view pinned to clicked row true true
Convergence: identical end state scrollTop 3 3

The head/base flip on every leading-edge cell (0 vs 16 ms at the component level; synchronous vs deferred flush at the hook level) while the semantics cells (delta preservation, cancellation, convergence) agree on both arms — the change is load-bearing and behavior-preserving. This is the mechanism-level analog of the PR's real-PTY "first visible output 29.01 → 11.11 ms" row; the scheduling delay removed is exactly one 16 ms frame wait.

Secondary claims verified at the hook level with the PR's own fake-timer tests (6/6 green) plus the real-timer cells above: the deadline test's pass also proves vi.advanceTimersByTime advances performance.now() in vitest 3.2.7 (otherwise the trailing timer would fire at fake t=21, not t=16, and the test would fail) — so the deadline-awareness measurement is sound.

Reviewer Test Plan walkthrough

Plan step Result
First wheel event updates viewport immediately ✅ A/B: head 0 ms / base 16 ms (component level, real stdin)
Subsequent events coalesced without losing accumulated delta ✅ head: 2 flushes per burst (leading + one trailing), 90/90 rows preserved on both arms
Scrollbar click cancels pending trailing wheel update ✅ pinned-to-clicked-row on both arms
Focused CLI tests pass; repo build and typecheck ✅ 64/64 scroll-surface tests (04-gate-scroll-surface-64-pass.png); tsc --noEmit exit 0 on packages/cli; ESLint exit 0 on changed files; full build was completed at HEAD by the environment before this round
Real-PTY burst capture (100x30, SGR oscillation) ⚠️ not executable here — capture scripts are not in the tree (the PR doc says so); mechanism-level equivalent measured instead

Mutation matrix (PR's new tests vs mutants of the changed file)

Each mutant applied to the head tree, focused suites (use-frame-coalesced-flush.test.ts + ScrollableList.test.tsx, 22 tests) run, file restored. Witness: 03-m1-revert-kills-new-tests.png.

Mutant Result Killed by
M0 control (unmutated) 22/22 green — (green control)
M1 revert hook to base (trailing-only) 7 failed | 15 passed all 6 new hook tests + new ScrollableList immediacy test, each with the intended assertion (expected "spy" to be called 1 times, but got 0; expected +0 to be 3 at ScrollableList.test.tsx:174)
M2 never record lastFlushAt (always-immediate) 4 failed | 18 passed deadline, coalescing, cancel, unmount tests
M3 disable over-budget immediate path 1 failed | 21 passed exactly the over-budget test
M4 delete unmount cleanup effect 1 failed | 21 passed exactly the unmount test — confirms the PR's claim that this was a previously surviving mutant
M5 positive control (frameMs * 4) 4 failed | 18 passed deadline, over-budget, coalescing + ScrollableList burst test

Zero survivors. Every guard the PR introduces is pinned by a test that fails with the intended behavioral assertion when the guard is removed. M1 doubles as the vacuity check for the whole PR.

Findings

None blocking. Non-blocking observations (completeness reporting, not merge conditions):

  1. Coalescing is pinned only at hook level. Under M2 (every schedule applies immediately), the entire 16-test ScrollableList suite still passes — at the component level, delta preservation and final state are identical for both schedulings, so only the hook-level call-count tests distinguish them. This is correct layering (the hook is the unit that owns flush cadence), just worth knowing.
  2. Pre-existing real-timer coupling. flushScrollFrame() waits a fixed 30 ms against the 16 ms frame constant; M5 showed doubling the constant would break the burst test. Pre-existing design, unchanged by this PR, and the PR's new component test deliberately avoids it (same-tick oracle, no timer advance).

Not covered

  • Real-PTY performance figures (29.01→11.11 ms first output, 18.71→31.75 FPS): single-machine macOS captures; scripts not in the tree. I verified the mechanism (one 16 ms wait removed, 0 ms leading latency) but not the end-to-end FPS numbers. This reproduces the mechanism, not the author's terminal-side measurement.
  • Windows/Linux terminals, pixel-smooth touchpad scrolling, host scrollback under alternate screen, Ink incremental rendering, the O(item-count) offsets rebuild — all declared out of scope by the PR itself.
  • Per-commit attribution: depth-2 shallow checkout; the snapshot lists 4 commits but only HEAD^2 is locally reachable (git rev-list HEAD^1..HEAD^2 = 1). The aggregate HEAD^1..HEAD diff was verified; per-commit behavior attribution was out of reach.
  • Base drift: the snapshot's baseRefOid (0756be0c…) is not present locally; the merge commit was built against the current base tip 5563a6c580, which is what the A/B exercised. The merge applied cleanly (worktree diff clean).
  • Repo-wide test suite: PR CI covers it; I ran the targeted scroll surface (5 files, 64 tests), packages/cli typecheck, and ESLint on changed files. npm run build at HEAD is given by the environment.

Methodology

Environment: node:22-bookworm container (node v22.23.2), depth-2 merge checkout (HEAD=723973a778, HEAD^1=5563a6c580, HEAD^2=7c0aabcc79), npm ci + npm run build pre-completed at HEAD. Harnesses (verify-ab-hook.test.ts, verify-ab-component.test.tsx, copied into each tree for the run, then removed) run via the package's own vitest config (jsdom, real timers unless stated) against TypeScript source; the base worktree reused the head checkout's third-party node_modules via symlinks whose realpaths were asserted (@testing-library/react nested-only; ajv v8 nested vs v6 at root — root alone resolves the wrong major). Mutation matrix via git show/sed on a scratch copy of the hook, focused vitest per mutant, restore via git checkout. Gates: vitest (64/64), tsc --noEmit (exit 0), ESLint (exit 0 on changed files; liveness proven by a planted unused-variable violation reported with exit 1). Assertion convention: one vitest test case = one scripted assertion (each contains fail-capable expects); mutant-run "failures" count as passes because each kill matched the predicted kill set (expected-failures-are-passes rule); plus standalone scripted checks (typecheck, lint, liveness probe, realpath checks, commit-reachability check). Tally: 5 (head harness) + 5 (base harness) + 132 (6×22 mutation outcomes, all as predicted) + 64 (gate) + 1 (tsc) + 1 (eslint) + 1 (lint liveness) + 2 (realpath) + 1 (reachability) = 212 pass, 0 fail. Raw logs: ab-hook.jsonl, ab-component.jsonl, mutation-*.log, gate-scroll-surface.log, typecheck-cli.log, eslint-changed.log; captures produced with scripts/verify-capture.mjs.

Flakiness gate log

rounds=5 files=2 skipped=0
file packages/cli/src/ui/components/shared/ScrollableList.test.tsx: (cd packages/cli) npx --no-install vitest run ./src/ui/components/shared/ScrollableList.test.tsx
file packages/cli/src/ui/hooks/use-frame-coalesced-flush.test.ts: (cd packages/cli) npx --no-install vitest run ./src/ui/hooks/use-frame-coalesced-flush.test.ts


per-file results (P=pass F=fail I=infra-exit, one letter per run):
  packages/cli/src/ui/components/shared/ScrollableList.test.tsx: PPPPP
  packages/cli/src/ui/hooks/use-frame-coalesced-flush.test.ts: PPPPP

verdict: pass
summary: 2 changed test file(s) x 5 identical rounds, no divergence

--- per-invocation detail (full copy in the artifact) ---
round 1 · packages/cli/src/ui/components/shared/ScrollableList.test.tsx: P (exit 0)
round 1 · packages/cli/src/ui/hooks/use-frame-coalesced-flush.test.ts: P (exit 0)
round 2 · packages/cli/src/ui/components/shared/ScrollableList.test.tsx: P (exit 0)
round 2 · packages/cli/src/ui/hooks/use-frame-coalesced-flush.test.ts: P (exit 0)
round 3 · packages/cli/src/ui/components/shared/ScrollableList.test.tsx: P (exit 0)
round 3 · packages/cli/src/ui/hooks/use-frame-coalesced-flush.test.ts: P (exit 0)
round 4 · packages/cli/src/ui/components/shared/ScrollableList.test.tsx: P (exit 0)
round 4 · packages/cli/src/ui/hooks/use-frame-coalesced-flush.test.ts: P (exit 0)
round 5 · packages/cli/src/ui/components/shared/ScrollableList.test.tsx: P (exit 0)
round 5 · packages/cli/src/ui/hooks/use-frame-coalesced-flush.test.ts: P (exit 0)

Evidence images

01-ab-head-immediate-first-wheel

02-ab-base-trailing-16ms-delay

03-m1-revert-kills-new-tests

04-gate-scroll-surface-64-pass

Harness scripts and raw logs are in the workflow run artifacts (7-day retention).

Qwen Code · sandboxed verification

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Triage re-run completed without a new review.

⚠️ The bot has neither a verdict nor a deferral on 7c0aabcc79b2f63eff8da5de11d7cdf67fd2d56e — no APPROVED, CHANGES_REQUESTED, or COMMENTED review of its own. A DISMISSED one does not count: dismiss_stale_reviews voids the bot's approval on every push, which is exactly when a fresh one is needed. If this re-run was meant to review or approve, it did not, and an approval left by another account is a separate vote that does not count as the bot's own.

⚠️ 机器人在 7c0aabcc79b2f63eff8da5de11d7cdf67fd2d56e既没有裁决也没有 defer —— 没有属于它自己的 APPROVEDCHANGES_REQUESTEDCOMMENTED 评审。DISMISSED 不算:dismiss_stale_reviews 会在每次推送时作废机器人的批准,而那恰恰是需要一次新批准的时刻。如果这次重跑本应评审或批准,那么它没有做到;而其他账号留下的批准是另一张票,不能算作机器人自己的。

The stage comments above were updated with the latest result. View workflow run.

上方各阶段评论已更新为最新结果。查看工作流运行

@DragonnZhang

Copy link
Copy Markdown
Collaborator Author

CI note: the only failing check is the same pre-existing main-side failure as #9682/#8332Test (ubuntu-latest) → Run .github/scripts helper tests → release note classification → "updates labels after a lookup failure and exits non-zero" (classify-release-notes.test.mjs:224). Neither the script nor the test is touched by this branch (byte-identical to main), and main's own ci.yml runs have been red at this same subtest for days. Everything else on this head is green; the merge with current main was conflict-free.

中文说明:唯一失败为主干预存的 classify-release-notes 辅助测试问题(本分支未改动相关文件),其余检查全绿,与最新 main 的合并无冲突。

@DragonnZhang

Copy link
Copy Markdown
Collaborator Author

@qwen-code /review

@DragonnZhang

Copy link
Copy Markdown
Collaborator Author

@qwen-code /review

@github-actions

github-actions Bot commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Qwen Code review request accepted. Review is running in workflow run. A command-triggered review is not listed under the checks of this PR; the result is posted here as a review when it finishes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants