fix(web-shell): constrain virtual scroll rows - #6362
Conversation
|
Thanks for the PR! (re-triage after test addition) Template looks good ✓ Problem: Structural CSS bug — when the virtualizer kicks in for long web-shell sessions, the Direction: Aligned. Web-shell layout correctness is in scope, and this is a straightforward CSS fix. No product-direction concerns. Approach: The scope is tight — 62 additions, 6 deletions across 3 files (CSS, TSX, and now a focused DOM test). The approach mirrors what I'd do independently: a Moving on to code review. 🔍 中文说明感谢贡献！（添加测试后重新审查） 模板完整 ✓ 问题：结构性 CSS bug——当 web-shell 长会话触发虚拟滚动后， 方向：对齐。Web-shell 布局正确性在项目范围内，这是一个直接的 CSS 修复。没有产品方向方面的顾虑。 方案：范围紧凑——3 个文件，62 行增加、6 行删除（CSS、TSX 以及新增的 DOM 测试）。方案与独立分析的结果一致： 进入代码审查 🔍 — Qwen Code · qwen3.7-max |
Code ReviewThe diff is clean and minimal. Two CSS classes do the work:
In the TSX, inline No correctness bugs, no security concerns, no regressions spotted. The CSS correctly preserves the measurement container's full width while constraining visible rows. New TestThe author added a focused DOM test ( Test ResultsRan the MessageList DOM test suite on the PR code: All 34 tests pass (33 existing + 1 new virtual scroll test). No regressions. Real-Scenario Testing (tmux)N/A — this is a CSS layout fix for the web-shell browser UI. The tmux workflows ( Browser verification by a reviewer (open a long web-shell session, confirm centered width is maintained across the virtual-scroll threshold) is the appropriate validation step here. 中文说明代码审查Diff 干净且最小化。两个 CSS 类完成了工作：
在 TSX 中，容器的内联 没有发现正确性 bug、安全问题或回归。CSS 正确保留了测量容器的全宽，同时约束了可见行。 新增测试作者新增了针对性的 DOM 测试（ 测试结果在 PR 代码上运行了 MessageList DOM 测试套件： 全部 34 个测试通过（33 个已有 + 1 个新增虚拟滚动测试）。没有回归。 真实场景测试 (tmux)不适用——这是 web-shell 浏览器 UI 的 CSS 布局修复。 tmux 工作流 ( 审查者的浏览器验证（打开一个长 web-shell 会话，确认在虚拟滚动阈值前后居中宽度保持一致）是此处合适的验证步骤。 — Qwen Code · qwen3.7-max |
|
This is a clean, well-scoped CSS fix. The structural bug is real: the The author has now added a focused DOM test that mocks the virtualizer and verifies the correct CSS classes are applied, closing the test-coverage gap from the initial submission. 34/34 tests pass. 62 lines across 3 files, no drive-by changes, no new abstractions. If I had to maintain this in six months, I'd thank the author for keeping it simple. The one gap remains browser screenshots — the structural analysis is convincing from the code, but a visual confirmation from a reviewer would close the loop. That's a reviewer task, not a blocker. Approving. ✅ 中文说明这是一个干净、范围合理的 CSS 修复。结构性 bug 是真实存在的： 作者现已新增了一个针对性的 DOM 测试，mock 虚拟滚动器并验证正确的 CSS 类是否被应用，弥补了初次提交时的测试覆盖缺口。34/34 测试通过。 3 个文件 62 行改动，没有夹带无关改动，没有新增抽象。如果六个月后需要维护这段代码，我会感谢作者保持了简洁。 唯一的缺憾是缺少浏览器截图——从代码层面看结构性分析是有说服力的，但审查者的视觉确认可以闭环验证。这是审查者的任务，不是阻塞项。 批准 ✅ — 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. ✅
wenshao
left a comment
wenshao
left a comment
There was a problem hiding this comment.
No review findings. Downgraded from Approve to Comment: CI still running.
— qwen3.7-max via Qwen Code /review
| /> | ||
| {useVirtualScroll ? ( | ||
| <div | ||
| className={styles.virtualSizer} |
There was a problem hiding this comment.
[Suggestion] No existing DOM test exercises the virtual scroll rendering path. The VIRTUAL_SCROLL_THRESHOLD is 200 messages, but all tests in MessageList.dom.test.tsx use fewer than 20 messages. Since this PR changes CSS classes and inline styles on the virtual path, a regression in this branch (e.g., a CSS specificity conflict with .list > *) would go undetected.
Consider adding a test that triggers the virtual scroll branch:
it('renders virtual scroll rows with sizer and row classes', () => {
const messages = simpleTurns(110); // 220 messages, above threshold
const c = mount(messages);
expect(c.querySelector(`.${styles.virtualSizer}`)).not.toBeNull();
expect(c.querySelectorAll(`.${styles.virtualRow}`).length).toBeGreaterThan(0);
});— qwen3.7-max via Qwen Code /review
|
Addressed review comment #6362 (comment). Added a focused DOM test for the virtual branch in Verified locally: cd packages/web-shell && npx vitest run client/components/MessageList.dom.test.tsx |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
No review findings. Downgraded from Approve to Comment: CI still running.
Clean, minimal fix. The .virtualSizer / .virtualRow class split correctly separates the measurement layer (full width) from the rendered rows (content-width constrained), mirroring the existing .list > * centering behavior. The new DOM test covers the virtual scroll rendering path. LGTM ✅
— qwen3.7-max via Qwen Code /review
|
@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. ✅
✅ Verification report — real-browser A/B (merge reference)I reproduced the bug and verified the fix by rendering the real A/B: identical harness + identical vite server, swapping only the 2 source files — base
Measured DOM geometry (pane = 1440px, cap
|
| metric | ✗ BEFORE (main) |
✓ AFTER (this PR) |
|---|---|---|
virtualized (real virtualizer active) |
true |
true |
| row width | 1388px — full pane content | 1000px — exactly the cap |
row left / margin |
50px / 0 — flush-left |
244px / 194px each side — centered |
| row center vs pane center (744) | overflows cap by ~194px per side | 744 = exact pane center ✅ |
| sizer / measurement layer | 1388px (full) | 1388px (full) — unchanged ✅ |
| row DOM | inline width:100%, no class |
.virtualRow class + right:0 |
The BEFORE rows blow straight through the green guide (the expected centered cap); the AFTER rows hug it. Crucially, the sizer stays full width in both — the fix only re-applies the centered content cap to each rendered row (.virtualRow) while keeping the virtualizer's full-width measurement container (.virtualSizer). This matches the non-virtualized path, where .list > * already caps direct children; virtual rows are grandchildren of .list, so they escaped that rule — which is the root cause this PR addresses.
Unit test
MessageList.dom.test.tsxon this PR: 34/34 pass (incl. the newrenders virtual scroll rows with sizer and row width classes).- Non-vacuity check — reverting only the source to
mainwhile keeping the PR's test makes the new test fail:expected null not to be nullatquerySelector('.'+styles.virtualSizer)(main emits no.virtualSizer). So the added test genuinely guards the fix.
Verdict
LGTM — safe to merge. The reported symptom (virtualized rows expanding to full pane width) is reproduced on main and fully resolved by this PR; centering and the content-width cap now match the non-virtualized transcript, and the virtualizer's measurement layer is untouched. Scope is limited to the web-shell virtualized transcript layout; no behavioral change to the non-virtual path.
🇨🇳 中文版（点击展开）
✅ 验证报告 —— 真实浏览器 A/B（供 merge 参考）
我通过在真实的无头 Chromium（Playwright）中渲染真实的 MessageList 组件 + 真实的 MessageList.module.css 来复现问题并验证修复，喂入 420 条消息——超过 VIRTUAL_SCROLL_THRESHOLD = 200，因此本 PR 改动的虚拟化渲染分支被真实执行（真实的 @tanstack/react-virtual 生效：virtualized: true，420 行中窗口化渲染 34 行）。只有叶子行内容被打桩替换——与本 PR 自带 DOM 测试所 mock 的模块完全一致（MessageItem、ParallelAgentsGroup、CompactModeContext），这些都不影响行宽。
A/B： 相同的测试壳 + 相同的 vite 服务，仅交换 2 个源文件——基线 main @edc0555e vs PR head @01f7546。环境：🐧 Linux（PR 作者仅在 macOS 测过），因此本次也补上了 PR 中标注"未测试"的 Linux 覆盖。
实测 DOM 几何（面板 = 1440px，上限 --chat-content-width: 1000px）
| 指标 | ✗ 修复前（main） |
✓ 修复后（本 PR） |
|---|---|---|
virtualized（真实虚拟化生效） |
true |
true |
| 行宽 | 1388px —— 撑满面板内容区 | 1000px —— 恰好等于上限 |
行 left / margin |
50px / 0 —— 左对齐 |
244px / 两侧各 194px —— 居中 |
| 行中心 vs 面板中心 (744) | 每侧超出上限约 194px | 744 = 面板正中心 ✅ |
| sizer / 测量层 | 1388px（全宽） | 1388px（全宽）—— 未变 ✅ |
| 行 DOM | 内联 width:100%，无 class |
.virtualRow class + right:0 |
修复前的行直接穿过绿色参考线（期望的居中上限）；修复后的行紧贴参考线。关键点：两种情况下 sizer 都保持全宽——修复只是给每个实际渲染的行（.virtualRow）重新套上居中内容上限，同时保留虚拟化器的全宽测量容器（.virtualSizer）。这与非虚拟化路径一致：.list > * 本就会给 .list 的直接子元素加上限，而虚拟行是 .list 的孙子元素，因此逃逸了该规则——这正是本 PR 修复的根因。
单元测试
- 本 PR 上
MessageList.dom.test.tsx：34/34 通过（含新增的renders virtual scroll rows with sizer and row width classes）。 - 非空验证 —— 仅把源码回退到
main、保留 PR 的测试，新测试即失败：在querySelector('.'+styles.virtualSizer)处报expected null not to be null（main 不产生.virtualSizer）。说明新增测试确实能守护该修复。
结论
LGTM —— 可以安全合并。 所报告的现象（虚拟化行撑满面板宽度）在 main 上可复现，本 PR 完全修复；居中与内容宽度上限现已与非虚拟化 transcript 一致，且虚拟化器测量层未受影响。改动范围仅限 web-shell 虚拟化 transcript 布局，未改变非虚拟路径行为。
Methodology: real component + real CSS module in headless Chromium; leaf children stubbed identically to the PR's own vitest DOM test (row width is governed purely by the CSS/DOM structure this PR changes, not by row content). Screenshots hosted on my fork's release.
3 participants
What this PR does
Constrain web-shell virtualized transcript rows to the same centered chat content width used by the non-virtualized transcript. The virtualizer's full-width measurement layer remains full width, while each rendered row now carries the chat content width cap.
Why it's needed
When long web-shell sessions crossed the virtual scrolling threshold, the transcript rows expanded across the whole pane instead of respecting the configured chat maximum width. This made the page look full-width only after virtualization kicked in.
Reviewer Test Plan
How to verify
Open a web-shell session with enough transcript rows to enable virtual scrolling. Confirm messages remain centered and capped to the normal chat content width before and after virtual scrolling is active. Locally, I verified the existing MessageList DOM behavior with
cd packages/web-shell && npx vitest run client/components/MessageList.dom.test.tsx.Evidence (Before & After)
Before: virtualized transcript rows filled the available page width once the virtual scroller activated. After: virtualized transcript rows stay centered and constrained to the normal chat content width while the virtualizer measurement layer remains full width.
Tested on
Environment (optional)
Local vitest run in
packages/web-shell.Risk & Scope
Linked Issues
N/A
中文说明
What this PR does
让 web-shell 虚拟化后的 transcript 行继续使用非虚拟列表相同的居中聊天内容宽度。虚拟滚动器的测量层仍保持全宽，每个实际渲染的行重新套用聊天内容最大宽度。
Why it's needed
长会话超过虚拟滚动阈值后，transcript 行会展开到整个面板宽度，不再遵守配置的聊天最大宽度。这样页面会在虚拟滚动触发后才突然变成满宽。
Reviewer Test Plan
How to verify
打开一个足够长、能够触发虚拟滚动的 web-shell 会话。确认虚拟滚动启用前后，消息仍保持居中并限制在正常聊天内容宽度内。本地已通过
cd packages/web-shell && npx vitest run client/components/MessageList.dom.test.tsx验证现有 MessageList DOM 行为。Evidence (Before & After)
Before: 虚拟滚动启用后，transcript 行会填满可用页面宽度。After: 虚拟滚动启用后，transcript 行仍保持居中并限制在正常聊天内容宽度内，同时虚拟滚动器测量层继续保持全宽。
Tested on
Environment (optional)
在
packages/web-shell中运行本地 vitest。Risk & Scope
Linked Issues
N/A