fix(ui): bottom-align short VP content so blank space is at top - #9305
fix(ui): bottom-align short VP content so blank space is at top#9305qwen-code-dev-bot wants to merge 28 commits into
Conversation
In VP mode, when the conversation fits within the viewport, content was top-aligned, leaving a blank gap between the last message and the composer. Bottom-align short bottom-stuck content (blank space at the top, latest message directly above the composer), matching standard chat TUIs. Only applies while sticking to the bottom with content that fits; overflow and scrolled-away cases are unchanged. Updates the three tests that pinned the previous top-aligned/collapsed layout to the new bottom-aligned expectation. Fixes #9300
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
⏸️ Holding at the direction gate — this is not a code-quality rejection, and no code review was performed.
- The problem is real. #9300 is a well-formed observed report (screenshots, repro steps, default-on VP mode), so the "does the problem exist" gate passes.
- The direction is a pending maintainer product decision. Bottom-aligning short content reverses a documented, intentional, test-locked layout choice:
- The design comment above
rootHeightinVirtualizedList.tsxrecords why the root collapses to content height: pinning it to full height "left a tall empty gap below short content and pushed the composer far down the screen"; collapsing makes "the composer sit right beneath the conversation". - #7652 pinned that contract with regression tests and explicitly removed an earlier production attempt at bottom-alignment.
- This PR rewrites three of those pinned tests to the opposite expectation.
- The design comment above
- The decision is still open. #9300 was triaged with
need-discussionabout an hour before this PR opened — "maintainer product decision needed — the requested behavior reverses an intentional, test-locked layout choice" — and no maintainer has weighed in on either thread since.
What unblocks this: a maintainer direction call on #9300. If bottom-alignment is accepted as the intended layout, the follow-up implementation should also rewrite the design comment above rootHeight (it would then describe the reversed behavior) and say why the #7652 removal doesn't apply this time. Until then, the gate stops here at Stage 1c.
中文说明
⏸️ 在方向关卡暂停 —— 这不是代码质量层面的拒绝，也未进行代码审查。
- 问题是真实的。 #9300 是有完整观测的报告（截图、复现步骤、默认开启的 VP 模式），"问题是否存在"这一关通过。
- 方向上有一个待定的维护者产品决策。 将短内容底部对齐会推翻一个有文档记载、有意为之、且有回归测试锁定的布局设计：
VirtualizedList.tsx中rootHeight上方的设计注释记录了根盒子收缩到内容高度的原因：固定全高会"在短内容下方留下大片空白、把输入框推到屏幕底部"，收缩是为了"让输入框紧跟在对话下方"。- #7652 用回归测试锁定了该约定，并明确移除了一次更早的底部对齐生产代码尝试。
- 本 PR 将其中三个锁定测试改写为相反的期望。
- 决策仍未做出。 本 PR 提交前约一小时，#9300 被打上
need-discussion标签，triage 结论为"需要维护者做产品决策——报告者要求的行为会推翻一个有意的、有回归测试锁定的布局设计"，此后两个 thread 均无维护者表态。
解锁条件：维护者在 #9300 上做出方向决策。若底部对齐被确认为期望布局，后续实现还需同步重写 rootHeight 上方的设计注释（届时它将描述被推翻的旧行为），并说明为何 #7652 当时的移除结论在本次不适用。在此之前，按关卡规则在 Stage 1c 停止。
— Qwen Code · qwen3.8-max
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed. Suggestions are inline.
Unresolved, please confirm:
- [Critical] Triage stage-1c direction gate (review 4948451627, @qwen-code-ci-bot): this PR reverses the intentional, test-locked collapse layout from #7652 and is held pending a maintainer product-direction decision on #9300 — not decidable from the co…
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
中文说明
仅完成部分审查，审查缺口已披露。 建议见行内评论。
未决，请确认：共 1 条（原文未翻译，列表见上方英文部分）。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
— qwen3.8-max via Qwen Code /review (v0.21.13)
| const rootHeight = | ||
| props.containerHeight !== undefined | ||
| ? fullHeightMeasurementPending | ||
| ? fullHeightMeasurementPending || bottomAlignGap > 0 |
There was a problem hiding this comment.
[Suggestion] The design-comment block above rootHeight (lines 953-964, untouched by this diff) now states the opposite of the shipped behavior — Failure scenario: it still says "Collapse to totalHeight whenever the content fits so the composer sits right beneath the conversation" and "The root collapses again after the measurement so short content does not leave a gap above the composer", but the new || bottomAlignGap > 0 clause deliberately keeps the root pinned to full containerHeight so the blank gap renders ABOVE the content (#9300). A future maintainer debugging frame-height/composer placement reads this authoritative comment (the only prose explaining rootHeight), concludes the full-height pin for short content is a regression against documented policy, and restores the collapse — silently re-introducing #9300. Suggested fix: update the block to state the new contract — the root collapses only while not bottom-stuck; while bottomAlignGap > 0 it stays at containerHeight and the gap renders above the content (#9300).
中文说明
rootHeight 上方的设计注释块（第 953-964 行，本 diff 未改动）现在描述的行为与实际行为相反 —— 失败场景：注释仍然写着"内容放得下时收缩到 totalHeight，让输入框紧跟在对话下方"、"测量完成后根盒子会再次收缩，避免短内容在输入框上方留空隙"，但新增的 || bottomAlignGap > 0 分支恰恰让根盒子保持满 containerHeight，把空白推到内容上方（#9300）。未来排查帧高度/输入框位置问题的维护者读到这段权威注释（这是唯一解释 rootHeight 的文字），会认为短内容撑满全高是违反设计文档的回归，从而"修复"回收缩逻辑 —— 悄悄重新引入 #9300。建议：将该注释块更新为新约定 —— 仅在非贴底状态下收缩；bottomAlignGap > 0 时保持 containerHeight，空白渲染在内容上方（#9300）。
— qwen3.8-max via Qwen Code /review (v0.21.13)
| // Short bottom-stuck content is bottom-aligned (#9300): blank rows at the | ||
| // top, the five items pinned to the bottom of the 20-row container. |
There was a problem hiding this comment.
[Suggestion] This test's title 'collapses a short bottom-stuck list below the container height' now states the opposite of what the rewritten assertions verify — Failure scenario: the body now asserts lines.length === 20 (the frame is exactly the 20-row container height, bottom-aligned — NOT collapsed below it). An engineer bisecting a #9300 regression, or grepping test names for collapse coverage, finds a test whose name asserts collapse while its body asserts bottom-alignment; a plausible wrong "fix" is to reconcile the body with the name (restore the collapse assertions), reintroducing the #9300 bug under cover of a green suite. Suggested fix: rename to match the new contract, e.g. it('bottom-aligns a short bottom-stuck list within the container height', ...).
中文说明
该测试的标题 'collapses a short bottom-stuck list below the container height' 现在与改写后的断言含义相反 —— 失败场景：测试体现在断言 lines.length === 20（帧恰好等于 20 行容器高度、底部对齐，并没有收缩到容器高度以下）。将来有人在二分排查 #9300 回归、或按测试名搜索"收缩"相关覆盖时，会看到一个名字声称收缩、断言却是底部对齐的测试；一个看似合理的错误"修复"是把测试体改回与名字一致（恢复收缩断言），从而在测试全绿的掩护下重新引入 #9300。建议：重命名以匹配新约定，例如 it('bottom-aligns a short bottom-stuck list within the container height', ...)。
— qwen3.8-max via Qwen Code /review (v0.21.13)
| expect(lastFrame()?.split('\n')).toEqual(['confirm']); | ||
| // Bottom-aligned (#9300): the single confirmation sits at the container | ||
| // bottom with blank rows above. | ||
| expect(lastFrame()?.split('\n')).toEqual(['', '', '', '', 'confirm']); |
There was a problem hiding this comment.
[Suggestion] This test's title 'collapses after measuring changed content at full viewport height' no longer describes the tested behavior — Failure scenario: after this change the root never collapses back down in this scenario (the final frames are full-height 5-row bottom-aligned arrays, asserted just below). A reader or a future CI failure report citing this title would believe the collapse-after-measurement invariant is still covered when the assertions now verify the opposite shape. Suggested fix: rename, e.g. it('bottom-aligns changed content after measuring it at full viewport height', ...).
中文说明
该测试的标题 'collapses after measuring changed content at full viewport height' 已不再描述实际测试的行为 —— 失败场景：本改动之后，此场景中根盒子不会再收缩回去（最终帧是满 5 行容器高度、底部对齐的数组，见下方断言）。读者或未来引用此标题的 CI 失败报告，会误以为"测量后收缩"这一不变量仍有覆盖，而断言实际验证的是相反的形状。建议：重命名，例如 it('bottom-aligns changed content after measuring it at full viewport height', ...)。
— qwen3.8-max via Qwen Code /review (v0.21.13)
| // Bottom-aligned (#9300): the released height becomes blank space at the | ||
| // TOP, and the collapsed summary sits at the bottom (last row is content, | ||
| // not a gap between content and the composer). | ||
| expect(lines[lines.length - 1]!.trim()).not.toBe(''); |
There was a problem hiding this comment.
[Suggestion] The replacement assertion cannot pin the new bottom-aligned behavior on this dynamic transition — Failure scenario: the removed expect(lines.length).toBeLessThanOrEqual(8) was replaced with a last-line-non-blank check, but the shrunken top-aligned frame produced when bottom-alignment is disabled also ends in content, so this test passes either way. Mutation probe (applied, run, reverted): forcing bottomAlignGap to 0 leaves this test GREEN, while adding expect(lines.length).toBe(40) fails under the mutant (AssertionError: expected 6 to be 40) and passes on the correct code (32/32). A regression of bottom-alignment that manifests only on the thought-collapse transition — the exact scenario this test is named for — would ship green.
| expect(lines[lines.length - 1]!.trim()).not.toBe(''); | |
| expect(lines.length).toBe(40); | |
| expect(lines[lines.length - 1]!.trim()).not.toBe(''); |
中文说明
替换后的断言无法在这个动态折叠过渡场景上锁定新的底部对齐行为 —— 失败场景：被移除的 expect(lines.length).toBeLessThanOrEqual(8) 换成了"最后一行非空"的检查，但当底部对齐被禁用时，收缩后的顶部对齐小帧同样以内容行结尾，因此该测试在两种情况下都能通过。变异探针（应用、运行、已还原）：将 bottomAlignGap 强制为 0 后此测试仍为绿色；而加上 expect(lines.length).toBe(40) 后，在变异体下失败（AssertionError: expected 6 to be 40），在正确代码下通过（32/32）。只在这个思考块折叠过渡场景上显现的底部对齐回归 —— 恰是该测试名字所声称覆盖的场景 —— 会在测试全绿的情况下溜走。
— qwen3.8-max via Qwen Code /review (v0.21.13)
…#9305) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
|
🤖 Addressed the latest review feedback (round 1/10). What changed, and what I pushed back on: · 已处理最新评审反馈（第 1/10 轮）。改动内容与我反驳保留之处如下： Autofix review round — PR #9305All four inline Suggestions are resolved in code (commit
|
doudouOUC
left a comment
doudouOUC
left a comment
There was a problem hiding this comment.
权限测试：token 已可正常提交 PR review。
|
🕐 Review received — an automatic review of the current head is still running, so this round is held until it lands (a push now would cancel it and discard its work, #8888). Your feedback stays queued for the next eligible round. 中文说明🕐 已收到评审 —— 当前 head 上仍有一轮自动 review 在运行，本轮暂缓（现在推送会取消该 review 并丢弃其工作，#8888）。反馈保持排队，等待下一次可运行的轮次处理。 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Test Plan (not a blocker): 32 tests pass — this review observed 20933 passed.
Deferred under the convergence posture (round 2, not a blocker) — recorded, not requested in this round:
packages/cli/src/ui/components/shared/VirtualizedList.tsx:552 — [probe] Mutation survivor: the isStickingToBottom half of the bottomAlignGap guard is pinned by no test
[Critical] Triage stage-1c direction gate (review 4948451627, @qwen-code-ci-bot): this PR reverses the intentional, test-locked collapse layout from #7652 and remains held pending a maintainer product-direction decision on #9300 — bottom-aligning short VP content (blank space above the conversation, latest message directly above the composer) as the intended layout. Re-checked this round against the PR and the linked issue's full thread: no maintainer direction call has been recorded since (the issue retains need-discussion, and the latest thread comment keeps the gate open). Still stands — this is a product decision, not a code defect settleable from code. All four round-1 code Suggestions are verifiably fixed at this round's commit; the direction gate is the only remaining blocker.
中文说明
仅完成部分审查，审查缺口已披露。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
Test Plan（非阻断）：32 tests pass — this review observed 20933 passed。
收敛姿态下延后（第 2 轮，非阻断）——已记录，本轮不要求修改：共 1 条（原文未翻译，列表见上方英文部分）。
[Critical] Triage stage-1c direction gate (review 4948451627, @qwen-code-ci-bot): this PR reverses the intentional, test-locked collapse layout from #7652 and remains held pending a maintainer product-direction decision on #9300 — bottom-aligning short VP content (blank space above the conversation, latest message directly above the composer) as the intended layout. Re-checked this round against the PR and the linked issue's full thread: no maintainer direction call has been recorded since (the issue retains need-discussion, and the latest thread comment keeps the gate open). Still stands — this is a product decision, not a code defect settleable from code. All four round-1 code Suggestions are verifiably fixed at this round's commit; the direction gate is the only remaining blocker.
— qwen3.8-max via Qwen Code /review (v0.21.13)
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下： Autofix round summary — no code changes this roundThis round triaged one automated review (
|
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Not explored to full depth (tool budget reached): "agent 5": executing packages/cli/src/ui/components/shared/VirtualizedList.test.tsx under vitest — after a successful npm ci (2062 packages), a sibling pipeline proces….
Test Plan (not a blocker): 32 tests pass — this review observed 21296 passed.
[Critical] R2-1 [Critical] Triage stage-1c direction gate (review 4948451627, @qwen-code-ci-bot): this PR reverses the intentional, test-locked collapse layout from #7652 and remains held pending a maintainer product-direction decision on #9300 — whether bottom-aligning short VP content (blank space above the conversation, latest message directly above the composer) is the intended layout. Re-checked this round against the live issue thread: issue #9300 is OPEN with need-discussion, and the last thread comment (@doudouOUC, 2026-08-17T05:43:02Z) keeps the gate open — no maintainer direction call has been recorded since. Still stands — this is a product decision, not a code defect settleable from code. The PR description also still does not say why #7652's removal of the earlier bottom-align attempt doesn't apply this time (the gate's named follow-up). Failure scenario: merging before the direction call ships a contested layout and re-locks the opposite contract in three rewritten tests; if the maintainer keeps the #7652 collapse layout, the change must be reverted with its inverted test pins. Witness: gh issue view 9300 (2026-08-18): state OPEN, labels include need-discussion; @doudouOUC 2026-08-17T05:43:02Z: "the remaining blocker is purely the product-direction call, which needs maintainer sign-off — keeping need-discussion in place"; no approval exists on the PR.
中文说明
仅完成部分审查，审查缺口已披露。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未探索到全部深度（达到工具调用预算）："agent 5"：executing packages/cli/src/ui/components/shared/VirtualizedList.test.tsx under vitest — after a successful npm ci (2062 packages), a sibling pipeline proces…。
Test Plan（非阻断）：32 tests pass — this review observed 21296 passed。
[Critical] R2-1 [Critical] Triage stage-1c direction gate (review 4948451627, @qwen-code-ci-bot): this PR reverses the intentional, test-locked collapse layout from #7652 and remains held pending a maintainer product-direction decision on #9300 — whether bottom-aligning short VP content (blank space above the conversation, latest message directly above the composer) is the intended layout. Re-checked this round against the live issue thread: issue #9300 is OPEN with need-discussion, and the last thread comment (@doudouOUC, 2026-08-17T05:43:02Z) keeps the gate open — no maintainer direction call has been recorded since. Still stands — this is a product decision, not a code defect settleable from code. The PR description also still does not say why #7652's removal of the earlier bottom-align attempt doesn't apply this time (the gate's named follow-up). Failure scenario: merging before the direction call ships a contested layout and re-locks the opposite contract in three rewritten tests; if the maintainer keeps the #7652 collapse layout, the change must be reverted with its inverted test pins. Witness: gh issue view 9300 (2026-08-18): state OPEN, labels include need-discussion; @doudouOUC 2026-08-17T05:43:02Z: "the remaining blocker is purely the product-direction call, which needs maintainer sign-off — keeping need-discussion in place"; no approval exists on the PR.
— qwen3.8-max via Qwen Code /review (v0.21.13)
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下： 🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下： Autofix round summary — no code changes this roundThis round triaged one automated review (
|
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Not reviewed: test-efficacy — probe runner tripped the repo's fresh-checkout prerequisite guard; 0 mutants and 0/3 hunk probes executed (harnessValidated: null — the control never ran).
Not reviewed: reverse audit — stopped before round 4 by the review time budget.
Test Plan (not a blocker): 32 tests pass — this review observed 21928 passed.
[Critical] R2-1 [Critical] Triage stage-1c direction gate (review 4948451627, @qwen-code-ci-bot): this PR reverses the intentional, test-locked collapse layout from #7652 and remains held pending a maintainer product-direction decision on #9300 — whether bottom-aligning short VP content (blank space above the conversation, latest message directly above the composer) is the intended layout. Re-checked this round against the live issue: issue #9300 is OPEN with need-discussion, and the last thread comment (@doudouOUC, 2026-08-17T05:43:02Z) keeps the gate open — no maintainer direction call has been recorded since. Still stands — this is a product decision, not a code defect settleable from code. The PR description also still does not say why #7652's removal of the earlier bottom-align attempt doesn't apply this time (the gate's named follow-up). Failure scenario: merging before the direction call ships a contested layout and re-locks the opposite contract in three rewritten tests; if the maintainer keeps the #7652 collapse layout, the change must be reverted with its inverted test pins. Witness: gh issue view 9300 (2026-08-19): state OPEN, labels include need-discussion; no approval exists on the PR.
中文说明
仅完成部分审查，审查缺口已披露。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未审查：test-efficacy — probe runner tripped the repo's fresh-checkout prerequisite guard; 0 mutants and 0/3 hunk probes executed (harnessValidated: null — the control never ran)。
未审查：反向审计——评审时间预算不足，未能开始第 4 轮。
Test Plan（非阻断）：32 tests pass — this review observed 21928 passed。
[Critical] R2-1 [Critical] Triage stage-1c direction gate (review 4948451627, @qwen-code-ci-bot): this PR reverses the intentional, test-locked collapse layout from #7652 and remains held pending a maintainer product-direction decision on #9300 — whether bottom-aligning short VP content (blank space above the conversation, latest message directly above the composer) is the intended layout. Re-checked this round against the live issue: issue #9300 is OPEN with need-discussion, and the last thread comment (@doudouOUC, 2026-08-17T05:43:02Z) keeps the gate open — no maintainer direction call has been recorded since. Still stands — this is a product decision, not a code defect settleable from code. The PR description also still does not say why #7652's removal of the earlier bottom-align attempt doesn't apply this time (the gate's named follow-up). Failure scenario: merging before the direction call ships a contested layout and re-locks the opposite contract in three rewritten tests; if the maintainer keeps the #7652 collapse layout, the change must be reverted with its inverted test pins. Witness: gh issue view 9300 (2026-08-19): state OPEN, labels include need-discussion; no approval exists on the PR.
— qwen3.8-max via Qwen Code /review (v0.21.14)
| const bottomAlignGap = | ||
| isStickingToBottom && maxScroll === 0 |
There was a problem hiding this comment.
[Critical] R4-1: The new bottomAlignGap makes isStickingToBottom layout-visible when content fits, but the pre-existing mount-time re-stick effect (VirtualizedList.tsx:409-421) flips that flag to true on the first commit of ANY fitting list — silently overriding the host's explicit initialScrollIndex={0} top-anchored start for the banner-only list. Every fresh VP session (default ui.useTerminalBuffer) and every /clear mounts the list with only VP_BANNER_ITEM and initialScrollIndex={0} (MainContent.tsx:465-467), yet the banner renders bottom-aligned directly above the composer with the entire rest of the viewport blank ABOVE it, instead of top-anchored as pre-diff. The state is un-stickable while maxScroll === 0 because every scroll path re-sticks. The diff's documented intent (#9300) argues for bottom-aligning a bottom-stuck conversation — not the banner-only top-anchored mount; the host's deliberate 0-vs-SCROLL_TO_ITEM_END distinction for exactly this state is evidence it should start top-anchored, and no test covers a fitting mount with initialScrollIndex={0}. The concrete failure: open a session (or /clear) and instead of the banner at the top of the viewport it sits above the composer with all remaining rows blank above it, on every session start, until the first real message arrives.
Witness (probe, fitting 10-row item, initialScrollIndex={0}, 20-row container — flips between arms):
PR (bottomAlignGap active): numLines=20, firstContentLine=10
["","",...,"banner-0",...,"banner-9"] <- bottom-aligned, 10 blank rows above
REVERTED (bottomAlignGap=0): numLines=10, firstContentLine=0
["banner-0",...,"banner-9"] <- top-aligned
Suggested fix: gate the mount-time re-stick so it doesn't fire before the initial scroll is applied (mirror the isInitialScrollSet.current guard the initial-scroll effect uses), or exclude non-end-anchored/banner-only mounts from bottomAlignGap; plus a regression test: fitting content, initialScrollIndex={0}, assert the frame is top-aligned.
中文说明
[Critical] R4-1：当内容放得下视口时，新增的 bottomAlignGap 使 isStickingToBottom 在布局中可见，但既有的挂载期 re-stick effect（VirtualizedList.tsx:409-421）会在任何"放得下"的列表首次 commit 时把该标志翻为 true —— 悄悄覆盖了宿主对 banner-only 列表显式传入的 initialScrollIndex={0} 顶部锚定。每个全新 VP 会话（默认开启 ui.useTerminalBuffer）和每次 /clear 都只带 VP_BANNER_ITEM 以 initialScrollIndex={0} 挂载列表（MainContent.tsx:465-467），但 banner 却渲染为底部对齐、紧贴输入框上方，其上方是整片空白的视口 —— 与改动前的顶部锚定相反。由于 maxScroll === 0 时所有滚动路径都会重新贴底，该状态无法解除。本 diff 的文档意图（#9300）支持的是对贴底会话做底部对齐 —— 而非 banner-only 的顶部锚定挂载；宿主对这一状态刻意区分 0 与 SCROLL_TO_ITEM_END 正是其应保持顶部锚定的证据，且没有任何测试覆盖 initialScrollIndex={0} 的"放得下"挂载。具体故障：打开会话（或 /clear）时，banner 不再位于视口顶部，而是悬在输入框上方、上方全部留白，每次会话开始都如此，直到第一条真实消息到来。
证据（探针：放得下的 10 行条目、initialScrollIndex={0}、20 行容器 —— 两侧可翻转复现）：
PR（bottomAlignGap 生效）：numLines=20, firstContentLine=10
["","",...,"banner-0",...,"banner-9"] <- 底部对齐，上方 10 行空白
还原（bottomAlignGap=0）：numLines=10, firstContentLine=0
["banner-0",...,"banner-9"] <- 顶部对齐
建议修复：为挂载期 re-stick 加门控，使其在初始滚动应用前不触发（仿照初始滚动 effect 使用的 isInitialScrollSet.current 守卫），或将非末尾锚定/banner-only 挂载排除在 bottomAlignGap 之外；并补充回归测试：放得下的内容、initialScrollIndex={0}，断言帧为顶部对齐。
— qwen3.8-max via Qwen Code /review (v0.21.14)
| // gap between the last message and the composer. Zero whenever content | ||
| // overflows (maxScroll > 0) or the user has scrolled away from the bottom. | ||
| const bottomAlignGap = | ||
| isStickingToBottom && maxScroll === 0 |
There was a problem hiding this comment.
[Suggestion] R4-2: The isStickingToBottom gate of this new condition has zero discriminating test coverage — every fitting-content test is bottom-stuck from mount (SCROLL_TO_ITEM_END), and every scrolled-away test uses overflowing content where the gap is 0 regardless. The mutation isStickingToBottom && maxScroll === 0 → maxScroll === 0 survives the entire suite (verified: all 32 tests pass under the mutant), yet it is reachable: a user scrolls up in an overflowing conversation, then content shrinks in-place below the viewport so maxScroll becomes 0 — and the re-stick effect does not fire because wasAtBottom is false for a previously-overflowing, scrolled-away list. Under the mutant the root grows to full container height with blank rows above the content the user is reading — a visible layout jump that ships green.
Witness:
Mutant (gate removed): Test Files 1 passed (1) / Tests 32 passed (32)
Discriminating probe vs mutant:
AssertionError: expected [ '', '', '', '', '', '', '', …(3) ] to deeply equal [ 'item-0', 'item-1', 'item-2' ]
The same probe passes on the real code.
Suggested test: render an overflowing bottom-stuck list (e.g. 20 items, containerHeight 10), scrollTo(0), then rerender with the SAME item keys short enough to fit (e.g. 3 items) and assert the frame collapses to the content height with no leading blank rows (lines.length equals the content height, lines[0] is item-0). Note: a shrink that changes/prunes item keys does not discriminate — the heights-prune layout effect re-sticks via contentPreviouslyFit; the discriminating shape is same-key in-place shrink, asserted right after it settles.
中文说明
[Suggestion] R4-2：该新条件中的 isStickingToBottom 门控没有任何区分性测试覆盖 —— 所有"放得下"的测试都从挂载起即贴底（SCROLL_TO_ITEM_END），而所有已上滚的测试都使用溢出内容（此时 gap 恒为 0）。变异 isStickingToBottom && maxScroll === 0 → maxScroll === 0 在整个测试套件中存活（已验证：该变异下 32 个测试全部通过），但它是可达的：用户在溢出的会话中向上滚动，随后内容原地收缩到视口以下使 maxScroll 变为 0 —— 此时 re-stick effect 不会触发，因为对曾经溢出且已上滚的列表 wasAtBottom 为 false。在该变异下，根盒子会膨胀到整个容器高度，用户正在阅读的内容上方出现空白行 —— 一个能带着绿色测试上线的可见布局跳变。
证据：
变异（移除门控）：Test Files 1 passed (1) / Tests 32 passed (32)
区分性探针在变异下：
AssertionError: expected [ '', '', '', '', '', '', '', …(3) ] to deeply equal [ 'item-0', 'item-1', 'item-2' ]
同一探针在真实代码上通过。
建议测试：渲染一个溢出的贴底列表（如 20 项、containerHeight 10），scrollTo(0)，随后用相同 key、数量收缩到放得下的数据（如 3 项）rerender，断言帧收缩到内容高度且无前导空白行（lines.length 等于内容高度、lines[0] 为 item-0）。注意：改变/裁剪 item key 的收缩没有区分性 —— heights-prune layout effect 会通过 contentPreviouslyFit 重新贴底；有区分性的形态是相同 key 的原地收缩，并在其稳定后立即断言。
— qwen3.8-max via Qwen Code /review (v0.21.14)
) The re-stick effect treated "content fits" as "was at the bottom" and flipped isStickingToBottom to true on every commit of a fitting list. While fitting that flag used to be layout-invisible, but #9300's bottomAlignGap now keys on it, so the flip silently overrode the host's explicit top anchor: every banner-only VP session (MainContent mounts initialScrollIndex={0} for it) rendered the banner bottom-aligned above the composer with a blank viewport above, and a scrolled-away list whose content shrank to fit was yanked to the bottom on the follow-up heights-prune commit. Re-stick only from a real bottom position: previously overflowing with the viewport at the bottom pixels. The collapse cascade still re-engages sticking (its clamped scrollTop sits at the bottom pixels while the content is still overflowing), and growth auto-scroll keeps the broader wasAtBottom, so banner-only → first-message still snaps to the bottom-stuck layout. Adds the regression test for a fitting initialScrollIndex={0} mount and a discriminating test for the isStickingToBottom gate that kills the previously surviving maxScroll-only mutant.
|
🤖 Addressed the latest review feedback (round 2/10). What changed, and what I pushed back on: · 已处理最新评审反馈（第 2/10 轮）。改动内容与我反驳保留之处如下： Autofix round summary — PR #9305Commit: Feedback dispositionsR4-1 — [Critical] Mount-time re-stick bottom-aligns top-anchored mounts (
|
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Not reviewed: build-and-test — test-efficacy probe could not be validated in this repo (fresh-checkout prerequisite guard; harnessValidated null, 0 mutants and 0/4 hunk probes executed).
Not explored to full depth (tool budget reached): "agent 4": none — no check was cut short.; "agent 6c": stress-quantifying the flake rate of the two new tests (only 10 full-file runs sampled; root-cause mechanism not isolated).
Test Plan (not a blocker): 32 tests pass — this review observed 21936 passed.
Deferred under the convergence posture (round 5, not a blocker) — recorded, not requested in this round:
packages/cli/src/ui/components/shared/VirtualizedList.tsx:562 — [probe] bottomAlignGap has no content-presence guard: an empty list mounted bottom-stuck inflates the root to full container heightpackages/cli/src/ui/components/shared/VirtualizedList.tsx:562 — [review] undocumented load-bearing invariant: the render window does not model the bottomAlignGap shift
[Critical] R2-1 Triage stage-1c direction gate (review 4948451627, @qwen-code-ci-bot): this PR reverses the intentional, test-locked collapse layout from #7652 and remains held pending a maintainer product-direction decision on #9300 — whether bottom-aligning short VP content (blank space above the conversation, latest message directly above the composer) is the intended layout. Re-checked this round against the live issue: issue #9300 is OPEN with need-discussion, and the last thread comment (@doudouOUC, 2026-08-17T05:43:02Z) keeps the gate open — no maintainer direction call has been recorded since, and no approval exists on the PR. The gate's named follow-up is also still unmet: the PR description does not say why #7652's removal of the earlier bottom-align attempt doesn't apply this time. Failure scenario: merging before the direction call ships a contested layout and re-locks the opposite contract in the three rewritten test pins; if the maintainer keeps the #7652 collapse layout, the change must be reverted together with its inverted test pins. Witness: gh issue view 9300 (this round): state OPEN, labels include need-discussion; @doudouOUC 2026-08-17T05:43:02Z: "the remaining blocker is purely the product-direction call, which needs maintainer sign-off — keeping need-discussion in place".
中文说明
仅完成部分审查，审查缺口已披露。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未审查：build-and-test — test-efficacy probe could not be validated in this repo (fresh-checkout prerequisite guard; harnessValidated null, 0 mutants and 0/4 hunk probes executed)。
未探索到全部深度（达到工具调用预算）："agent 4"：none — no check was cut short.；"agent 6c"：stress-quantifying the flake rate of the two new tests (only 10 full-file runs sampled; root-cause mechanism not isolated)。
Test Plan（非阻断）：32 tests pass — this review observed 21936 passed。
收敛姿态下延后（第 5 轮，非阻断）——已记录，本轮不要求修改：共 2 条（原文未翻译，列表见上方英文部分）。
[Critical] R2-1 Triage stage-1c direction gate (review 4948451627, @qwen-code-ci-bot): this PR reverses the intentional, test-locked collapse layout from #7652 and remains held pending a maintainer product-direction decision on #9300 — whether bottom-aligning short VP content (blank space above the conversation, latest message directly above the composer) is the intended layout. Re-checked this round against the live issue: issue #9300 is OPEN with need-discussion, and the last thread comment (@doudouOUC, 2026-08-17T05:43:02Z) keeps the gate open — no maintainer direction call has been recorded since, and no approval exists on the PR. The gate's named follow-up is also still unmet: the PR description does not say why #7652's removal of the earlier bottom-align attempt doesn't apply this time. Failure scenario: merging before the direction call ships a contested layout and re-locks the opposite contract in the three rewritten test pins; if the maintainer keeps the #7652 collapse layout, the change must be reverted together with its inverted test pins. Witness: gh issue view 9300 (this round): state OPEN, labels include need-discussion; @doudouOUC 2026-08-17T05:43:02Z: "the remaining blocker is purely the product-direction call, which needs maintainer sign-off — keeping need-discussion in place".
— qwen3.8-max via Qwen Code /review (v0.21.14)
| !contentPreviouslyFit && | ||
| wasScrolledToBottomPixels && | ||
| actualScrollTop >= prevScrollTop.current |
There was a problem hiding this comment.
[Critical] R5-1: The /clear half of the R4-1 blocker is still open. This gate only stops the false→true re-stick flip at MOUNT. On /clear the list is not remounted — ScrollableList has no key in MainContent.tsx, and historyRemountKey keys only the legacy <Static> branch — so isStickingToBottom, still true from the bottom-stuck rest state, is carried into the banner-only state. Nothing resets sticking on shrink: the scrollAnchor.index >= data.length re-anchor branch fixes only the anchor, every un-stick site is user-scroll-only, and the component's only /clear handling is heights-cache pruning. With sticking carried true and maxScroll === 0, bottomAlignGap bottom-aligns the banner directly above the composer with the whole viewport blank ABOVE it — the exact outcome R4-1 quoted — on every /clear, while a genuinely fresh session (identical visible state, initialScrollIndex={0}) renders top-aligned per the new test. Pre-diff the same state collapsed top-aligned, so this diff introduces the regression.
Witness (probe, unmodified PR): mount 20 items end-anchored in a 10-row container, shrink to 1 item without remount:
after clear (banner only): 10 lines ["","","","","","","","","","banner"] <- bottom-aligned, 9 blank rows above
with a one-line sticking reset in the re-anchor branch: 1 line ["banner"]
Suggested fix: clear sticking when data collapses to the host's non-end-anchored state — e.g. setIsStickingToBottom(false) in the re-anchor branch when data.length <= 1 (or gate bottomAlignGap on an end-anchored scrollAnchor); plus a regression test: stuck mount with overflowing content, shrink data to one item without remount, assert the frame is top-aligned.
中文说明
[Critical] R5-1：R4-1 阻塞项的 /clear 一半仍然未封闭。此门控只阻止了挂载期 false→true 的 re-stick 翻转；而 /clear 时列表并不会重新挂载 —— MainContent.tsx 中的 ScrollableList 没有 key，historyRemountKey 只作用于旧版 <Static> 分支 —— 因此 isStickingToBottom 仍保持着贴底静息状态的 true，被原样带入 banner-only 状态。收缩时没有任何路径重置 sticking：scrollAnchor.index >= data.length 重锚定分支只修正锚点，所有解除贴底的位置都只在用户滚动时触发，组件内唯一的 /clear 处理只是裁剪高度缓存。sticking 保持 true 且 maxScroll === 0 时，bottomAlignGap 会把 banner 底部对齐到输入框正上方、其上方整片视口留白 —— 正是 R4-1 描述的故障现象 —— 每次 /clear 都发生；而一个真正的新会话（可见状态完全相同，initialScrollIndex={0}）按新测试渲染为顶部对齐。改动前同一状态是顶部对齐折叠的，因此该回归由本 diff 引入。
证据（探针，未改动的 PR 代码）：以末尾锚定挂载 20 项、容器 10 行，不重挂载直接收缩到 1 项：
clear 后（仅 banner）：10 行 ["","","","","","","","","","banner"] <- 底部对齐，上方 9 行空白
在重锚定分支加一行 sticking 重置后：1 行 ["banner"]
建议修复：当数据坍缩到宿主的非末尾锚定状态时清除 sticking —— 例如在重锚定分支中当 data.length <= 1 时 setIsStickingToBottom(false)（或将 bottomAlignGap 门控在末尾锚定的 scrollAnchor 上）；并补充回归测试：贴底挂载溢出内容、不重挂载收缩到单项，断言帧为顶部对齐。
— qwen3.8-max via Qwen Code /review (v0.21.14)
| // overflows (maxScroll > 0) or the user has scrolled away from the bottom. | ||
| const bottomAlignGap = | ||
| isStickingToBottom && maxScroll === 0 | ||
| ? Math.max(0, scrollableContainerHeight - totalHeight) |
There was a problem hiding this comment.
[Critical] R5-2: When content fits (maxScroll === 0), ANY scroll input re-engages isStickingToBottom through the unchanged imperative handlers, and this gap then bottom-aligns the content — destroying exactly the top-aligned states this PR creates and its new tests pin. In scrollBy, newScrollTop = Math.max(0, 0 ± δ) = 0, so newScrollTop >= maxScroll (0 >= 0) is always true → setIsStickingToBottom(true); for an upward scroll that overwrites the setIsStickingToBottom(false) two lines earlier within the same call. scrollTo(0) behaves the same. Concretely: one scroll keypress — Up/Down/PgUp/PgDn/Home/End or one mouse-wheel tick — on the new test's scrolled-away-then-shrunk state, or on the fresh-session landing screen, snaps the content to the viewport bottom with blank rows above, contradicting this diff's own comment that "bottom-alignment is reserved for bottom-stuck conversations". Pre-PR the flip was invisible on a fitting list (the root collapsed), so the visible defect is created by this diff.
Witness (probe, unmodified PR):
after shrink (the new test's state): ["item-0","item-1","item-2"]
after one scrollBy(1): ["","","","","","","","item-0","item-1","item-2"]
(scrollBy(-1) flips identically; scrollTo(0) on a fresh fitting mount flips a 3-line frame to 20-line bottom-aligned. Flip-check: requiring maxScroll > 0 in the bottom branch keeps all three probes top-aligned with all 34 tests still green.)
Suggested fix: don't engage sticking from a scroll attempt when there is nothing to scroll — in scrollBy/scrollTo's bottom branch require maxScroll > 0 before setIsStickingToBottom(true); add a regression test extending the R4-2 scenario with one scrollBy(1) after the shrink, asserting the frame stays top-aligned.
中文说明
[Critical] R5-2：当内容放得下视口（maxScroll === 0）时，任何滚动输入都会经由未改动的命令式滚动方法重新置位 isStickingToBottom，随后该 gap 把内容底部对齐 —— 恰恰摧毁了本 PR 新建并由新测试锁定的顶部对齐状态。scrollBy 中 newScrollTop = Math.max(0, 0 ± δ) = 0，于是 newScrollTop >= maxScroll（0 >= 0）恒为真 → setIsStickingToBottom(true)；向上滚动时，这会在同一次调用内覆盖两行之前的 setIsStickingToBottom(false)。scrollTo(0) 同理。具体表现：在新测试的"上滚后收缩至放得下"状态、或新会话首屏上，按任意滚动键 —— 上/下/PgUp/PgDn/Home/End 或一格滚轮 —— 内容会瞬间贴到视口底部、上方留白，与本 diff 自己注释中"底部对齐仅保留给贴底会话"相矛盾。改动前该翻转在放得下的列表上不可见（根盒子折叠），因此这一可见缺陷由本 diff 引入。
证据（探针，未改动的 PR 代码）：
收缩后（新测试锁定的状态）：["item-0","item-1","item-2"]
一次 scrollBy(1) 之后： ["","","","","","","","item-0","item-1","item-2"]
（scrollBy(-1) 翻转结果相同；在放得下的新挂载上 scrollTo(0) 会把 3 行帧翻成 20 行底部对齐。翻转验证：在贴底分支加上 maxScroll > 0 条件后，三个探针全部保持顶部对齐，且 34 个测试仍然全绿。）
建议修复：没有可滚动内容时不要让滚动尝试置位 sticking —— 在 scrollBy/scrollTo 的贴底分支中要求 maxScroll > 0 才 setIsStickingToBottom(true)；补充回归测试：在 R4-2 场景的收缩之后追加一次 scrollBy(1)，断言帧仍为顶部对齐。
— qwen3.8-max via Qwen Code /review (v0.21.14)
| // TOP (standard chat-TUI behavior), instead of top-aligning and leaving a | ||
| // gap between the last message and the composer. Zero whenever content | ||
| // overflows (maxScroll > 0) or the user has scrolled away from the bottom. | ||
| const bottomAlignGap = |
There was a problem hiding this comment.
[Critical] R5-3: The new gate can be armed by the content shrink itself: a scrolled-away list that shrinks to fit in two steps gets bottom-aligned and re-latched. The re-anchor branch clamps an overshooting viewport to exactly the new bottom (Math.max(0, totalHeight - scrollableContainerHeight)) without touching sticking. On the NEXT shrink that brings content under the container height, this gate then sees !contentPreviouslyFit (the intermediate state still overflowed), wasScrolledToBottomPixels true (prevScrollTop sits exactly at prevMaxScroll, inside the -1 tolerance), and actualScrollTop >= prevScrollTop whenever the collapse is at/below the anchored item — and flips sticking false→true. With maxScroll === 0, bottomAlignGap renders blank rows above content the user had explicitly scrolled away from, and sticking stays latched for subsequent growth (auto-follow resumes). The new test misses this because it scrolls all the way to top (scrollTo(0)), where the gate correctly stays shut.
Witness (probe, unmodified PR — 20 items, 10-row container, stuck mount, scrollBy(-5), collapse 20→12, collapse 12→8):
PR: final frame ['','','item-0','item-1',...] <- 2 blank rows above content, bottom-aligned
(probe asserting a top-aligned frame FAILED)
FIX (bottomAlignGap additionally gated on an end-anchored scrollAnchor):
final frame ['item-0','item-1',...] <- top-aligned, probe PASSED
Suggested fix: don't let content-driven clamps arm the gate — e.g. mark the re-anchor branch's clamp in a ref that the next gate run consumes. Note: gating bottomAlignGap on an end-anchored scrollAnchor also flips this probe, but would suppress bottom-alignment in a genuinely bottom-stuck collapse cascade whose re-anchor parks away from the end; the ref-mark variant avoids that trade-off. Regression test: overflowing list, scrollBy(-1), shrink-to-fit via two collapses with the collapse below the anchor, assert a top-aligned frame.
中文说明
[Critical] R5-3：新门控可以被内容收缩本身"武装"：一个已上滚的列表分两步收缩到放得下时，会被底部对齐并重新锁住贴底。重锚定分支会把越界的视口钳制到恰好新的底部（Math.max(0, totalHeight - scrollableContainerHeight)），但不触碰 sticking。下一次把内容收缩到容器高度以内时，本门控看到 !contentPreviouslyFit（中间状态仍在溢出）、wasScrolledToBottomPixels 为真（prevScrollTop 恰好停在 prevMaxScroll，处于 -1 容差内）、且坍缩发生在锚定项之下/之下时 actualScrollTop >= prevScrollTop —— 于是把 sticking 从 false 翻为 true。maxScroll === 0 时，bottomAlignGap 会在用户明确上滚离开的内容上方渲染空白行，且 sticking 保持锁住，后续内容增长时重新自动跟随。新测试没有覆盖到这一点，因为它直接滚到最顶部（scrollTo(0)），那里门控确实保持关闭。
证据（探针，未改动的 PR 代码 —— 20 项、10 行容器、贴底挂载、scrollBy(-5)、坍缩 20→12、再坍缩 12→8）：
PR：最终帧 ['','','item-0','item-1',...] <- 内容上方 2 行空白，底部对齐
（断言顶部对齐帧的探针 失败）
修复（bottomAlignGap 额外门控在末尾锚定的 scrollAnchor 上）：
最终帧 ['item-0','item-1',...] <- 顶部对齐，探针 通过
建议修复：不要让内容驱动的钳制武装门控 —— 例如在重锚定分支的钳制发生时在一个 ref 中打标记，供下一次门控运行时消费。注意：把 bottomAlignGap 门控在末尾锚定的 scrollAnchor 上也能翻转本探针，但会在真正的贴底坍缩级联（重锚定停在非末尾处）中抑制底部对齐；ref 打标记的方案可避免该权衡。回归测试：溢出列表、scrollBy(-1)、经两次坍缩（第二次低于锚定项）收缩到放得下，断言顶部对齐帧。
— qwen3.8-max via Qwen Code /review (v0.21.14)
|
🤖 Addressed the latest review feedback (round 3/10). What changed, and what I pushed back on: · 已处理最新评审反馈（第 3/10 轮）。改动内容与我反驳保留之处如下： Review round summary — PR #9305All three inline Critical findings were reproduced on the pre-round commit with focused regression tests (each failed with exactly the frame the finding's witness describes), then fixed at root cause and pinned by those tests. The review-body finding (R2-1 direction gate) is a maintainer product-direction decision and stays open — details below. Findings and dispositions[rc:3818187238] R5-1 — Reproduced: a stuck 20-item list in a 10-row container shrunk in place to one item renders 10 lines with the banner bottom-aligned and 9 blank rows above, while the identical fresh-session state renders top-aligned per this PR's own test. Fix (as suggested by the finding): the re-anchor branch now clears [rc:3818187239] R5-2 — any scroll input re-engages sticking when content fits → FIXED Reproduced: after the R4-2 shrink-to-fit state, one Fix (finding's suggested direction): when [rc:3818187242] R5-3 — the re-anchor clamp arms the re-stick gate on a two-step shrink → FIXED Reproduced: stuck mount, Fix: the ref-mark variant the finding suggests, with one refinement found necessary during implementation. A consume-once mark is defeated in practice by React re-running the effect on identity-only churn (height re-measurement produces a new [rv:4978543596] R2-1 — Triage stage-1c direction gate (issue #9300 product decision) → ESCALATED / OPEN This is a product-direction call, not an implementation defect: whether bottom-aligning short VP content is the intended layout, given #7652 intentionally locked the opposite collapse layout. Issue #9300 remains OPEN with Also from the review body: the partial-review disclosures (integration suite skipped in CI / not run locally, test-efficacy probe not validated, stress-quantification not completed) are acknowledged — no actionable request. The two items deferred under the convergence posture (empty-list Conflict notes
VerificationReproduction (pre-round code, new tests only):
Post-fix:
Commit: 中文说明审查轮次总结 — PR #9305三条行内 Critical 发现均已在本轮开始前的提交上用聚焦回归测试复现（每条的失败帧与发现中证据描述完全一致），随后按根因修复并由这些测试锁定。审查主体中的发现（R2-1 方向门控）属于维护者的产品方向决策，保持开放 —— 详见下文。 发现与处置[rc:3818187238] R5-1 — R4-1 阻塞项的 复现：贴底挂载的 20 项列表（10 行容器）原地收缩到 1 项时，渲染出 10 行、banner 被底部对齐、上方 9 行空白；而可见状态完全相同的新会话却按本 PR 自己的测试渲染为顶部对齐。 修复（按发现建议的方向）：重锚定分支在数据坍缩到 [rc:3818187239] R5-2 — 内容放得下时任何滚动输入都会重新置位贴底 → 已修复 复现：在 R4-2 的"收缩至放得下"状态之后，一次 修复（按发现建议的方向）：当 [rc:3818187242] R5-3 — 重锚定钳制在两步收缩中武装了 re-stick 门控 → 已修复 复现：贴底挂载、 修复：采用发现建议的 ref 打标记方案，并在实现中发现一处必要改进。"消费一次"式标记在实践中会被 React 因恒等性扰动重跑 effect 所破坏（高度重测产生数值相同但恒等不同的新 [rv:4978543596] R2-1 — Triage stage-1c 方向门控（issue #9300 产品决策）→ 已上报 / 开放 这是产品方向决策，不是实现缺陷：短 VP 内容底部对齐是否是预期布局 —— #7652 曾有意锁定了相反的折叠布局。issue #9300 仍为 OPEN 且带 审查主体中的其他内容：部分审查的缺口披露（集成套件在 CI 中被跳过且本地未运行、测试有效性探针未能验证、压力量化未完成）已知悉 —— 无可执行的修改请求。收敛姿态下延后的两项（空列表 冲突说明
验证复现（本轮前代码，仅新测试）：
修复后：
提交： Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 10 this bot stops and leaves the PR for a human. · 有空请复审；第 10 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Not reviewed: test-efficacy — probe harness could not be validated in this repo (fresh-checkout prerequisite guard trips on untracked generated files; harnessValidated null, 0 mutants and 0 hunk probes executed by the harness — round-6 agents ran manual mutation checks instead).
Test Plan (not a blocker): 32 tests pass — this review observed 21938 passed.
Deferred under the convergence posture (round 6, not a blocker) — recorded, not requested in this round:
packages/cli/src/ui/components/shared/VirtualizedList.tsx:825 — [probe] scrollToIndex/scrollToItem still release sticking unconditionally on fitting content — the new no-op rule's sibling methods were not guardedpackages/cli/src/ui/components/shared/VirtualizedList.tsx:447 — [probe] the rewritten re-stick gate's positive path has zero test coverage — deleting the whole gate block leaves all 36 tests green (mutation survives)packages/cli/src/ui/components/shared/VirtualizedList.tsx:483 — [review] the host's banner-only predicate is duplicated inside the shared component as a magic data.length <= 1 — coupled to MainContent's initialScrollIndex rule by nothing
[Critical] R2-1 Triage stage-1c direction gate (review 4948451627, @qwen-code-ci-bot): this PR reverses the intentional, test-locked collapse layout from #7652 and remains held pending a maintainer product-direction decision on #9300 — whether bottom-aligning short VP content (blank space above the conversation, latest message directly above the composer) is the intended layout. Re-checked this round against the live issue via a fresh issue-context fetch: issue #9300 is OPEN with need-discussion, and the last thread comment (@doudouOUC, 2026-08-17T05:43:02Z) keeps the gate open — no maintainer direction call has been recorded since, and no approval exists on the PR. The gate's named follow-up is also still unmet: the PR description does not say why #7652's removal of the earlier bottom-align attempt doesn't apply this time. Still stands — this is a product decision, not a code defect settleable from code; this round's three code Criticals are direction-neutral implementation fixes and do not supersede it. Failure scenario: merging before the direction call ships a contested layout and re-locks the opposite contract in the three rewritten test pins; if the maintainer keeps the #7652 collapse layout, the change must be reverted together with its inverted test pins. Witness: issue-context fetch (this round): state OPEN, labels include need-discussion; @doudouOUC 2026-08-17T05:43:02Z: "the remaining blocker is purely the product-direction call, which needs maintainer sign-off — keeping need-discussion in place".
中文说明
仅完成部分审查，审查缺口已披露。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未审查：test-efficacy — probe harness could not be validated in this repo (fresh-checkout prerequisite guard trips on untracked generated files; harnessValidated null, 0 mutants and 0 hunk probes executed by the harness — round-6 agents ran manual mutation checks instead)。
Test Plan（非阻断）：32 tests pass — this review observed 21938 passed。
收敛姿态下延后（第 6 轮，非阻断）——已记录，本轮不要求修改：共 3 条（原文未翻译，列表见上方英文部分）。
[Critical] R2-1 Triage stage-1c direction gate (review 4948451627, @qwen-code-ci-bot): this PR reverses the intentional, test-locked collapse layout from #7652 and remains held pending a maintainer product-direction decision on #9300 — whether bottom-aligning short VP content (blank space above the conversation, latest message directly above the composer) is the intended layout. Re-checked this round against the live issue via a fresh issue-context fetch: issue #9300 is OPEN with need-discussion, and the last thread comment (@doudouOUC, 2026-08-17T05:43:02Z) keeps the gate open — no maintainer direction call has been recorded since, and no approval exists on the PR. The gate's named follow-up is also still unmet: the PR description does not say why #7652's removal of the earlier bottom-align attempt doesn't apply this time. Still stands — this is a product decision, not a code defect settleable from code; this round's three code Criticals are direction-neutral implementation fixes and do not supersede it. Failure scenario: merging before the direction call ships a contested layout and re-locks the opposite contract in the three rewritten test pins; if the maintainer keeps the #7652 collapse layout, the change must be reverted together with its inverted test pins. Witness: issue-context fetch (this round): state OPEN, labels include need-discussion; @doudouOUC 2026-08-17T05:43:02Z: "the remaining blocker is purely the product-direction call, which needs maintainer sign-off — keeping need-discussion in place".
— qwen3.8-max via Qwen Code /review (v0.21.14)
| if (maxScroll === 0) { | ||
| // Same no-op rule as scrollBy: a scroll attempt on fitting content | ||
| // must not flip sticking either way. | ||
| return; | ||
| } |
There was a problem hiding this comment.
[Critical] R5-2: This fix is partial — the maxScroll === 0 no-op guard was added to scrollBy and scrollTo, but not to scrollToEnd, and the End key maps to scrollToEnd (ScrollableList SCROLL_END handler). End was explicitly one of the keys R5-2 named. On any fitting list (maxScroll === 0) — the fresh-session banner-only state (top-anchored per the R4-1 fix) or this round's scrolled-away-then-shrunk state the new tests pin as top-aligned — one End keypress calls scrollToEnd(), which unconditionally runs setIsStickingToBottom(true); with sticking engaged, bottomAlignGap bottom-aligns the content under a blank viewport — the exact outcome R5-2 described, contradicting this diff's own comment that bottom-alignment is reserved for bottom-stuck conversations. Worse, the state is now latched: the new guards make Home/scrollTo(0), Up/Down/PgUp/PgDn and wheel (all scrollBy) early-return no-ops, so nothing short of content overflowing can un-stick it.
Witness (probe at the reviewed commit, flip-checked): fitting top-anchored 3-item mount in a 20-row container — rows=3 ["item-0","item-1","item-2"]; after scrollToEnd() → rows=20 with 17 blank rows above the items; scrollTo(0) and scrollBy(-1) afterwards leave it unchanged (latched). The scrolled-away-then-shrunk variant shows 7 blank rows + the items. Adding the same guard to scrollToEnd keeps both scenarios top-aligned; fix reverted afterward.
Suggested fix: mirror the guard in scrollToEnd — compute maxScroll = Math.max(0, totalHeight - scrollableContainerHeight) and early-return when it is 0 before setIsStickingToBottom(true); extend the R5-2 regression loop with a scrollToEnd() call. If End is deliberately an explicit enter-follow-mode command, state that in a comment and make scrollTo(SCROLL_TO_ITEM_END) consistent with it.
中文说明
R5-2：本次修复不完整 —— maxScroll === 0 空操作守卫加到了 scrollBy 和 scrollTo，但没有加到 scrollToEnd，而 End 键映射到 scrollToEnd（ScrollableList 的 SCROLL_END 处理）。End 正是 R5-2 明确点名的按键之一。在任何放得下的列表（maxScroll === 0）上 —— 新会话的 banner-only 状态（按 R4-1 修复为顶部锚定），或本轮新测试锁定为顶部对齐的"上滚后收缩至放得下"状态 —— 按一次 End 会调用 scrollToEnd()，其中无条件执行 setIsStickingToBottom(true)；贴底一旦置位，bottomAlignGap 就把内容底部对齐到一片空白的视口之下 —— 正是 R5-2 描述的结果，与本 diff 自己"底部对齐仅保留给贴底会话"的注释相矛盾。更糟的是该状态现在被锁住：新守卫使 Home/scrollTo(0)、上/下/PgUp/PgDn 与滚轮（均为 scrollBy）全部早退为空操作，除非内容溢出，否则无法解除贴底。
证据（探针，在受审提交上运行，可翻转验证）：放得下的 3 项顶部锚定挂载、20 行容器 —— rows=3 ["item-0","item-1","item-2"]；scrollToEnd() 之后 → rows=20，内容上方 17 行空白；随后的 scrollTo(0) 与 scrollBy(-1) 均无法改变（已锁住）。"上滚后收缩"变体显示 7 行空白 + 内容。给 scrollToEnd 加上同样的守卫后两个场景均保持顶部对齐；探针修复随后已还原。
建议修复：在 scrollToEnd 中镜像该守卫 —— 计算 maxScroll = Math.max(0, totalHeight - scrollableContainerHeight)，为 0 时在 setIsStickingToBottom(true) 之前早退；并在 R5-2 回归循环中追加一次 scrollToEnd() 调用。若 End 被有意设计为显式的"进入跟随模式"命令，请用注释说明，并让 scrollTo(SCROLL_TO_ITEM_END) 与之一致。
— qwen3.8-max via Qwen Code /review (v0.21.14)
| if (!isStickingToBottom) { | ||
| reAnchorClampMark.current = newAnchor; | ||
| } |
There was a problem hiding this comment.
[Critical] R6-1: The R5-1 drop branch above queues setIsStickingToBottom(false), but this mark-install reads the stale render-time flag (still true), so reAnchorClampMark is NOT installed on the exact render the drop fires. Reachability is settled: the banner (AppHeader's 6-row logo + info panel + Tips — the component's own height estimate for index 0 is 10 rows — plus notifications) renders at natural height with no clamp anywhere, while containerHeight = terminalHeight − controlsHeight − tabBarHeight; small terminals and split panes overflow it. With the mark null, any later effect trigger (terminal resize, height re-measure) re-engages sticking through the re-stick gate: !clampParked, !contentPreviouslyFit (banner overflows), wasScrolledToBottomPixels (parked at the banner's bottom) and actualScrollTop >= prevScrollTop all hold — and nothing re-drops it. The banner-only list the host mounts top-anchored (initialScrollIndex 0) renders bottom-pinned and clipped to the banner's own bottom, with auto-follow latched for the next conversation. The new R5-1 regression test only covers the banner-fits case, so it passes.
Witness (probe at the reviewed commit modeling the real /clear lifecycle, flip-checked): after /clear with a cached 15-row banner in a 10-row container — rows=10 ["b5".."b14"], bottom-pinned with the top clipped, while the host state is top-anchored; after resize 10→8 — rows=8 ["b7".."b14"]; after growth 1→2 the frame yanks to scrollIndex=1, proving sticking latched true on the resize (wasAtBottom was provably false at that run, so only the gate with a null mark explains it). Installing the mark when the drop fires keeps the frame at scrollIndex=0 with no re-engagement; fix reverted afterward.
Suggested fix: compute the drop once — const droppingSticking = data.length <= 1 && isStickingToBottom; — and install the mark when (!isStickingToBottom || droppingSticking). Add a regression test: stuck list, collapse to one item taller than the container, resize, assert sticking stays released and the frame is not bottom-pinned.
中文说明
R6-1：上方 R5-1 的 drop 分支排队执行 setIsStickingToBottom(false)，但此处安装标记的代码读取的是过期的渲染期标志（仍为 true），因此 drop 触发的那次渲染上 reAnchorClampMark 并未被安装。可达性已确认：banner（AppHeader 的 6 行 logo + 信息面板 + Tips —— 组件自身对 index 0 的高度估计即为 10 行 —— 外加通知条）按自然高度渲染、全程无任何钳制，而 containerHeight = terminalHeight − controlsHeight − tabBarHeight；小终端与分屏会使其溢出。标记为 null 时，任何后续 effect 触发（终端改尺寸、高度重测）都会经由 re-stick 门控重新置位贴底：!clampParked、!contentPreviouslyFit（banner 溢出）、wasScrolledToBottomPixels（停在 banner 底部）、actualScrollTop >= prevScrollTop 全部成立 —— 且没有任何路径再次清除贴底。宿主以顶部锚定（initialScrollIndex 0）挂载的 banner-only 列表被渲染为钉在 banner 自身底部、首行被裁剪，且自动跟随被锁住留给下一段对话。新的 R5-1 回归测试只覆盖 banner 放得下的场景，因此仍然通过。
证据（探针，在受审提交上模拟真实 /clear 生命周期，可翻转验证）：缓存 15 行 banner、10 行容器下 /clear 之后 —— rows=10 ["b5".."b14"]，钉在底部且顶部被裁剪，而宿主状态是顶部锚定；resize 10→8 之后 —— rows=8 ["b7".."b14"]；增长 1→2 时帧被拽到 scrollIndex=1，证明贴底已在 resize 时锁为 true（该次运行中 wasAtBottom 可证为假，只有标记为 null 的门控能解释）。在 drop 触发时同步安装标记后，帧保持 scrollIndex=0、无重新置位；探针修复随后已还原。
建议修复：把 drop 决策计算一次 —— const droppingSticking = data.length <= 1 && isStickingToBottom; —— 并在 (!isStickingToBottom || droppingSticking) 时安装标记。补充回归测试：贴底列表、坍缩到单个高于容器的条目、改终端尺寸，断言贴底保持解除且帧不被钉在底部。
— qwen3.8-max via Qwen Code /review (v0.21.14)
| const clampParked = | ||
| reAnchorClampMark.current !== null && | ||
| scrollAnchor.index === reAnchorClampMark.current.index && |
There was a problem hiding this comment.
[Critical] R6-2: reAnchorClampMark suppresses only the re-stick gate; the growth auto-scroll branch in this same effect (listGrew && (isStickingToBottom || wasAtBottom)) still reads wasAtBottom from the clamp-parked position and re-engages sticking — the same content-driven-parked-position-read-as-user-at-bottom defect class R5-3 targets, surviving in a sibling branch. The mark this PR adds is what lets the parked state persist to a growth event, making this branch the surviving re-latch path.
Failure scenario (R5-3's exact numbers): 20 items, container 10, bottom-stuck; the user scrolls up (scrollBy(-5)), sticking off. Collapse 20→12 parks the viewport at the new bottom and sets the mark; collapse 12→8 fits, the gate is blocked by the mark, the frame is top-aligned as the new test pins. Then the next message/token arrives (8→9): listGrew true, wasAtBottom true (0 >= 0 − 1), and this branch snaps the anchor to the end and sets sticking true — the deliberately scrolled-away user is yanked back to the bottom, the frame bottom-aligns, and auto-follow is resurrected by a content-driven position. No added test covers park→growth.
Witness (probe at the reviewed commit, flip-checked): after the two-step shrink — rows=8 ["item-0".."item-7"] scrollIndex=0 (the R5-3 fix holds); after growth 8→9 — rows=10 ["","item-0".."item-8"], bottom-aligned with a blank top row, scrollIndex=8. Gating the wasAtBottom disjunct with !clampParked gives rows=9 top-aligned, and that variant passes all 36 existing tests; fix reverted afterward.
Suggested fix: exclude the parked position from the growth branch the same way the gate does — listGrew && (isStickingToBottom || (wasAtBottom && !clampParked)); clampParked is already in scope. Add a regression test extending the R5-3 scenario with one growth step, asserting the frame stays top-aligned.
中文说明
R6-2：reAnchorClampMark 只抑制了 re-stick 门控；同一 effect 中的增长自动滚动分支（listGrew && (isStickingToBottom || wasAtBottom)）仍会从钳制停驻位置读取 wasAtBottom 并重新置位贴底 —— 与 R5-3 针对的"内容驱动的停驻位置被当作用户在底部"缺陷同类，存活在一个兄弟分支里。本 PR 新增的标记让停驻状态得以存续到增长事件，使该分支成为残存的重新锁住路径。
故障场景（R5-3 的原始数字）：20 项、容器 10 行、贴底；用户上滚（scrollBy(-5)），贴底解除。坍缩 20→12 把视口停在新底部并设置标记；再坍缩 12→8 放得下，门控被标记阻断，帧按新测试所锁定为顶部对齐。随后下一条消息/token 到来（8→9）：listGrew 为真、wasAtBottom 为真（0 >= 0 − 1），该分支把锚点吸附到末尾并置位贴底 —— 已明确上滚离开的用户被拽回底部，帧变为底部对齐，自动跟随被内容驱动的位置复活。新增测试均未覆盖"停驻→增长"。
证据（探针，在受审提交上运行，可翻转验证）：两步收缩之后 —— rows=8 ["item-0".."item-7"] scrollIndex=0（R5-3 修复成立）；增长 8→9 之后 —— rows=10 ["","item-0".."item-8"]，底部对齐且顶部一行空白，scrollIndex=8。给 wasAtBottom 分句加上 !clampParked 门控后得到 rows=9 顶部对齐，且该变体通过全部 36 个既有测试；探针修复随后已还原。
建议修复：像门控一样把停驻位置排除在增长分支之外 —— listGrew && (isStickingToBottom || (wasAtBottom && !clampParked))；clampParked 已在作用域内。补充回归测试：在 R5-3 场景后追加一次增长，断言帧保持顶部对齐。
— qwen3.8-max via Qwen Code /review (v0.21.14)
|
🔀 Base updated: red check(s) [Test (ubuntu-latest, Node 22.x), Post Coverage Comment (ubuntu-latest, 22.x)] pass on current main — merged current main via update-branch; CI will re-run. 中文说明🔀 已更新 base：红色检查 [Test (ubuntu-latest, Node 22.x), Post Coverage Comment (ubuntu-latest, 22.x)] 在当前 main 上通过 —— 已通过 update-branch 合入当前 main，CI 将重新运行。 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Not reviewed: build-and-test — the build-test harness's test phase never ran: packages/cli's type-check build fails on a pre-existing main error (src/serve/server/session-archive.test.ts:1069, byte-identical at the merge base); review agents ran the targeted suite VirtualizedList.test.tsx 36/36 green instead.
Not reviewed: test-efficacy — probe harness could not be validated in this repo (fresh-checkout prerequisite guard trips on the repo's dist/generated-file guard; harnessValidated null, 0 mutants and 0 hunk probes executed by the harness); the verifier ran manual probes and mutants instead.
Deferred under the convergence posture (round 7, not a blocker) — recorded, not requested in this round:
packages/cli/src/ui/components/shared/VirtualizedList.tsx:439 — [probe] the reAnchorClampMark clearing branch has no test coverage — deleting it leaves all 36 tests green (surviving mutant)
[Critical] R2-1 Triage stage-1c direction gate (review 4948451627, @qwen-code-ci-bot): this PR reverses the intentional, test-locked collapse layout from #7652 and remains held pending a maintainer product-direction decision on #9300 — whether bottom-aligning short VP content (blank space above the conversation, latest message directly above the composer) is the intended layout. Re-checked this round against the live issue via a fresh issue-context fetch: issue #9300 is OPEN with need-discussion, and the last thread comment (@doudouOUC, 2026-08-17T05:43:02Z) keeps the gate open — no maintainer direction call has been recorded since, and no approval exists on the PR (the single human review is a token-permission test, not a direction call). The gate's named follow-up is also still unmet: the PR description does not say why #7652's removal of the earlier bottom-align attempt doesn't apply this time. Still stands — this is a product decision, not a code defect settleable from code; this round's three code Criticals are direction-neutral implementation fixes and do not supersede it. Failure scenario: merging before the direction call ships a contested layout and re-locks the opposite contract in the three rewritten test pins; if the maintainer keeps the #7652 collapse layout, the change must be reverted together with its inverted test pins. Witness: gh live fetch (this round): issue #9300 state OPEN, labels include need-discussion; @doudouOUC 2026-08-17T05:43:02Z: "the remaining blocker is purely the product-direction call, which needs maintainer sign-off — keeping need-discussion in place".
中文说明
仅完成部分审查，审查缺口已披露。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未审查：build-and-test — the build-test harness's test phase never ran: packages/cli's type-check build fails on a pre-existing main error (src/serve/server/session-archive.test.ts:1069, byte-identical at the merge base); review agents ran the targeted suite VirtualizedList.test.tsx 36/36 green instead。
未审查：test-efficacy — probe harness could not be validated in this repo (fresh-checkout prerequisite guard trips on the repo's dist/generated-file guard; harnessValidated null, 0 mutants and 0 hunk probes executed by the harness); the verifier ran manual probes and mutants instead。
收敛姿态下延后（第 7 轮，非阻断）——已记录，本轮不要求修改：共 1 条（原文未翻译，列表见上方英文部分）。
[Critical] R2-1 Triage stage-1c direction gate (review 4948451627, @qwen-code-ci-bot): this PR reverses the intentional, test-locked collapse layout from #7652 and remains held pending a maintainer product-direction decision on #9300 — whether bottom-aligning short VP content (blank space above the conversation, latest message directly above the composer) is the intended layout. Re-checked this round against the live issue via a fresh issue-context fetch: issue #9300 is OPEN with need-discussion, and the last thread comment (@doudouOUC, 2026-08-17T05:43:02Z) keeps the gate open — no maintainer direction call has been recorded since, and no approval exists on the PR (the single human review is a token-permission test, not a direction call). The gate's named follow-up is also still unmet: the PR description does not say why #7652's removal of the earlier bottom-align attempt doesn't apply this time. Still stands — this is a product decision, not a code defect settleable from code; this round's three code Criticals are direction-neutral implementation fixes and do not supersede it. Failure scenario: merging before the direction call ships a contested layout and re-locks the opposite contract in the three rewritten test pins; if the maintainer keeps the #7652 collapse layout, the change must be reverted together with its inverted test pins. Witness: gh live fetch (this round): issue #9300 state OPEN, labels include need-discussion; @doudouOUC 2026-08-17T05:43:02Z: "the remaining blocker is purely the product-direction call, which needs maintainer sign-off — keeping need-discussion in place".
— qwen3.8-max via Qwen Code /review (v0.21.14)
| const clampParked = | ||
| reAnchorClampMark.current !== null && | ||
| scrollAnchor.index === reAnchorClampMark.current.index && |
There was a problem hiding this comment.
[Critical] R6-2: reAnchorClampMark suppresses only the re-stick gate; the growth auto-scroll branch in this same effect (listGrew && (isStickingToBottom || wasAtBottom), tsx:459-463) still reads wasAtBottom from the clamp-parked position and re-engages sticking — the same content-driven-parked-position-read-as-user-at-bottom defect class R5-3 targets, surviving in a sibling branch. The mark this PR adds is what lets the parked state persist to a growth event, making this branch the surviving re-latch path.
Failure scenario (R5-3's exact numbers, replayed by the new two-step test): 20 items, 10-row container, bottom-stuck; the user scrolls up (scrollBy(-5)), sticking off. Collapse 20→12 parks the viewport at the new bottom and sets the mark; collapse 12→8 fits, the gate is blocked by the mark, the frame is top-aligned as the new test pins. Then the next message/token arrives (8→9): listGrew true, wasAtBottom true (via contentPreviouslyFit), and this branch snaps the anchor to the end and sets sticking true — the deliberately scrolled-away user is yanked back to the bottom, the frame bottom-aligns, and auto-follow is resurrected by a content-driven position. No added test covers park→growth.
Witness (probe at the reviewed commit, flip-checked): after the two-step shrink — rows=8 ["item-0".."item-7"] (the R5-3 fix holds); after growth 8→9 — rows=10 ["","item-0".."item-8"], bottom-aligned with a blank top row. Gating the wasAtBottom disjunct with !clampParked gives rows=9 top-aligned, and all 36 tests stay green with the fix.
Suggested fix: listGrew && (isStickingToBottom || (wasAtBottom && !clampParked)) — clampParked is already in scope. Add a regression test extending the two-step-shrink scenario with one growth step, asserting the frame stays top-aligned. If the R6-1 fix also lands, add one combined regression test: the two interact on the first post-/clear message in the overflowing-banner state.
中文说明
R6-2：reAnchorClampMark 只抑制了 re-stick 门控；同一 effect 中的增长自动滚动分支（listGrew && (isStickingToBottom || wasAtBottom)，tsx:459-463）仍会从钳制停驻位置读取 wasAtBottom 并重新置位贴底 —— 与 R5-3 针对的"内容驱动的停驻位置被当作用户在底部"缺陷同类，存活在一个兄弟分支里。本 PR 新增的标记让停驻状态得以存续到增长事件，使该分支成为残存的重新锁住路径。
故障场景（R5-3 的原始数字，新的两步测试已复现该过程）：20 项、容器 10 行、贴底；用户上滚（scrollBy(-5)），贴底解除。坍缩 20→12 把视口停在新底部并设置标记；再坍缩 12→8 放得下，门控被标记阻断，帧按新测试所锁定为顶部对齐。随后下一条消息/token 到来（8→9）：listGrew 为真、wasAtBottom 为真（经由 contentPreviouslyFit），该分支把锚点吸附到末尾并置位贴底 —— 已明确上滚离开的用户被拽回底部，帧变为底部对齐，自动跟随被内容驱动的位置复活。新增测试均未覆盖"停驻→增长"。
证据（探针，在受审提交上运行，可翻转验证）：两步收缩之后 —— rows=8 ["item-0".."item-7"]（R5-3 修复成立）；增长 8→9 之后 —— rows=10 ["","item-0".."item-8"]，底部对齐且顶部一行空白。给 wasAtBottom 分句加上 !clampParked 门控后得到 rows=9 顶部对齐，且该修复下全部 36 个测试保持绿色。
建议修复：listGrew && (isStickingToBottom || (wasAtBottom && !clampParked)) —— clampParked 已在作用域内。补充回归测试：在两步收缩场景后追加一次增长，断言帧保持顶部对齐。若 R6-1 的修复同时落地，请再补一个组合回归测试：两个修复在"banner 溢出 + /clear 后第一条消息"的窄场景下会相互作用。
— qwen3.8-max via Qwen Code /review (v0.21.14)
| if (!isStickingToBottom) { | ||
| reAnchorClampMark.current = newAnchor; | ||
| } |
There was a problem hiding this comment.
[Critical] R6-1: The data.length <= 1 drop branch above queues setIsStickingToBottom(false), but this mark-install reads the stale render-time flag (still true), so reAnchorClampMark is NOT installed on the exact render the drop fires, when the single remaining item overflows the container. Reachability is settled: the banner (AppHeader's 6-row logo + info panel + Tips — the component's own height estimate for index 0 is 10 rows — plus notifications) renders at natural height with no clamp anywhere, while containerHeight = terminalHeight − controlsHeight − tabBarHeight; small terminals and split panes overflow it. With the mark null, any later effect trigger (terminal resize, height re-measure) re-engages sticking through the re-stick gate: !clampParked, !contentPreviouslyFit (banner overflows), wasScrolledToBottomPixels (parked at the banner's bottom) and actualScrollTop >= prevScrollTop all hold — and nothing re-drops it. The banner-only list the host mounts top-anchored (initialScrollIndex 0) renders bottom-pinned and clipped to the banner's own bottom, with auto-follow latched for the next conversation. The R5-1 regression test only covers the banner-fits case, so it passes.
Witness (probe at the reviewed commit, host-shaped 12-row banner in an 8-row container, flip-checked): after the /clear-style collapse — rows=8 ["b4".."b11"], bottom-pinned with the top clipped; after resize 8→15 — rows=15 ["","","","b0".."b11"], bottom-aligned with 3 blank top rows. Installing the mark when the drop fires keeps the frame top-aligned after resize: rows=12 ["b0".."b11"].
Suggested fix: compute the drop once — const droppingSticking = data.length <= 1 && isStickingToBottom; — and install the mark when (!isStickingToBottom || droppingSticking). Add a regression test: stuck list → collapse to one item taller than the container → resize → assert sticking stays released and the frame is not bottom-pinned. Note: if the R6-2 fix also lands, add one combined regression test — the two interact on the first post-/clear message in the overflowing-banner state.
中文说明
R6-1：上方 data.length <= 1 的 drop 分支排队执行 setIsStickingToBottom(false)，但此处安装标记的代码读取的是过期的渲染期标志（仍为 true），因此当剩余单项高于容器时，drop 触发的那次渲染上 reAnchorClampMark 并未被安装。可达性已确认：banner（AppHeader 的 6 行 logo + 信息面板 + Tips —— 组件自身对 index 0 的高度估计即为 10 行 —— 外加通知条）按自然高度渲染、全程无任何钳制，而 containerHeight = terminalHeight − controlsHeight − tabBarHeight；小终端与分屏会使其溢出。标记为 null 时，任何后续 effect 触发（终端改尺寸、高度重测）都会经由 re-stick 门控重新置位贴底：!clampParked、!contentPreviouslyFit（banner 溢出）、wasScrolledToBottomPixels（停在 banner 底部）、actualScrollTop >= prevScrollTop 全部成立 —— 且没有任何路径再次清除贴底。宿主以顶部锚定（initialScrollIndex 0）挂载的 banner-only 列表被渲染为钉在 banner 自身底部、首行被裁剪，且自动跟随被锁住留给下一段对话。R5-1 回归测试只覆盖 banner 放得下的场景，因此仍然通过。
证据（探针，在受审提交上模拟宿主形态：12 行 banner、8 行容器，可翻转验证）：/clear 式坍缩之后 —— rows=8 ["b4".."b11"]，钉在底部且顶部被裁剪；resize 8→15 之后 —— rows=15 ["","","","b0".."b11"]，底部对齐且上方 3 行空白。在 drop 触发时同步安装标记后，resize 之后帧保持顶部对齐：rows=12 ["b0".."b11"]。
建议修复：把 drop 决策计算一次 —— const droppingSticking = data.length <= 1 && isStickingToBottom; —— 并在 (!isStickingToBottom || droppingSticking) 时安装标记。补充回归测试：贴底列表、坍缩到单个高于容器的条目、改终端尺寸，断言贴底保持解除且帧不被钉在底部。注意：若 R6-2 的修复同时落地，请再补一个组合回归测试 —— 两个修复在"banner 溢出 + /clear 后第一条消息"的窄场景下会相互作用。
— qwen3.8-max via Qwen Code /review (v0.21.14)
| if (maxScroll === 0) { | ||
| // Same no-op rule as scrollBy: a scroll attempt on fitting content | ||
| // must not flip sticking either way. | ||
| return; | ||
| } |
There was a problem hiding this comment.
[Critical] R5-2: This fix is still partial — the maxScroll === 0 no-op guard was added to scrollBy and scrollTo, but not to scrollToEnd (tsx:842-851), and the End key maps to scrollToEnd (ScrollableList SCROLL_END handler, default binding ctrl+end). One End keypress on any fitting list unconditionally runs setIsStickingToBottom(true), contradicting the rule this guard's own comment states: "a scroll attempt on fitting content must not flip sticking either way".
On any fitting list — the fresh-session banner-only state (top-anchored per the R4-1 fix) or the scrolled-away-then-shrunk state the new tests pin top-aligned — pressing End bottom-aligns the content under a blank viewport. The state then latches: the new guards make every other scroll path (Home/arrows/PgUp/PgDn/wheel via scrollBy, scrollTo(0)) early-return no-ops, so nothing short of content overflowing can un-stick it.
Witness (probe at the reviewed commit, flip-checked): fitting 3-item top-anchored mount, 20-row container — mount rows=3 ["item-0","item-1","item-2"]; after scrollToEnd() → rows=20 with 17 blank rows above the items; scrollBy(±1)/scrollTo(0) afterwards leave it unchanged (latched). Mirroring the guard into scrollToEnd keeps rows=3 top-aligned, and all 36 tests stay green with the fix.
Suggested fix: mirror the guard in scrollToEnd — compute maxScroll = Math.max(0, totalHeight - scrollableContainerHeight) and early-return when it is 0 before setIsStickingToBottom(true); extend the R5-2 regression loop with a scrollToEnd() call. If End is deliberately an explicit enter-follow-mode command, state that in a comment and make scrollTo(SCROLL_TO_ITEM_END) consistent with it.
中文说明
R5-2：本次修复仍不完整 —— maxScroll === 0 空操作守卫加到了 scrollBy 和 scrollTo，但没有加到 scrollToEnd（tsx:842-851），而 End 键映射到 scrollToEnd（ScrollableList 的 SCROLL_END 处理，默认绑定 ctrl+end）。在任何放得下的列表上按一次 End 会无条件执行 setIsStickingToBottom(true)，与本守卫自身注释写明的规则 —— "放得下的内容上的滚动尝试不得双向翻转贴底" —— 相矛盾。
在任何放得下的列表上 —— 新会话的 banner-only 状态（按 R4-1 修复为顶部锚定），或新测试锁定为顶部对齐的"上滚后收缩至放得下"状态 —— 按一次 End 会把内容底部对齐到一片空白的视口之下。该状态随后被锁住：新守卫使所有其他滚动路径（Home/方向键/PgUp/PgDn/滚轮，均经 scrollBy，以及 scrollTo(0)）早退为空操作，除非内容溢出，否则无法解除贴底。
证据（探针，在受审提交上运行，可翻转验证）：放得下的 3 项顶部锚定挂载、20 行容器 —— 挂载 rows=3 ["item-0","item-1","item-2"]；scrollToEnd() 之后 → rows=20，内容上方 17 行空白；随后的 scrollBy(±1)/scrollTo(0) 均无法改变（已锁住）。给 scrollToEnd 加上同样的守卫后保持 rows=3 顶部对齐，且该修复下全部 36 个测试保持绿色。
建议修复：在 scrollToEnd 中镜像该守卫 —— 计算 maxScroll = Math.max(0, totalHeight - scrollableContainerHeight)，为 0 时在 setIsStickingToBottom(true) 之前早退；并在 R5-2 回归循环中追加一次 scrollToEnd() 调用。若 End 被有意设计为显式的"进入跟随模式"命令，请用注释说明，并让 scrollTo(SCROLL_TO_ITEM_END) 与之一致。
— qwen3.8-max via Qwen Code /review (v0.21.14)
|
🔀 Base updated: red check(s) [Dependency CVE audit] pass on current main — merged current main via update-branch; CI will re-run. 中文说明🔀 已更新 base：红色检查 [Dependency CVE audit] 在当前 main 上通过 —— 已通过 update-branch 合入当前 main，CI 将重新运行。 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
4 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- the fitting-content no-op guard missing from scrollToIndex/scrollToItem while pasted into scrollBy/scrollTo/scrollToEnd (VirtualizedList.tsx:891) — already reported in the round-6 deferral list (review 4979659125), round-8 deferral D8-1 (re…
- the fitting-content no-op rule pasted into scrollBy/scrollTo/scrollToEnd, each recomputing maxScroll (VirtualizedList.tsx:819) — already recorded in the round-8 deferral list as D8-1 (review 4981837967)
- the host's banner-only predicate re-derived inside the shared component as data.length <= 1 (VirtualizedList.tsx:500) — already recorded in the round-6 deferral list (review 4979659125) and as R16-2 in the rounds 13-16 already-reported list…
- zero debugLogger coverage of sticking transitions in the clamp-mark state machine (VirtualizedList.tsx:409) — already recorded in the round-8 deferral list as D8-3 (review 4981837967) and as R16-3 in the round-16 already-reported list
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally (the macOS/Windows unit-test matrix jobs were also skipped; the diff is platform-independent TypeScript, and the ubuntu unit-test job plus this review's full packages/cli suite passed).
Not reviewed: test-efficacy — probe harness could not be validated in this repo (fresh-checkout prerequisite guard; harnessValidated null, 0 mutants and 0 hunk probes executed by the harness); verifiers ran manual flip-checked probes and mutants instead.
Deferred under the convergence posture (round 17, not a blocker) — recorded, not requested in this round:
packages/cli/src/ui/components/shared/VirtualizedList.tsx:427 — [probe] wasScrolledToBottomPixels has no positive-boundary test — the tolerance-removed and prevMaxScroll > 2 mutants survive the full suite while a parked one-row-above-bottom…packages/cli/src/ui/components/shared/VirtualizedList.tsx:511 — [probe] released-arm mark install nested inside the anchor-change check — a Home-parked {0,0} state installs no mark and the next fitting growth yanks the viewport to END
[Critical] R2-1 Triage stage-1c direction gate: this PR reverses the intentional, test-locked collapse layout from #7652 and remains held pending a maintainer product-direction decision on #9300 — whether bottom-aligning short VP content (blank space above the conversation, latest message directly above the composer) is the intended layout. Re-checked this round against the live issue via a fresh fetch: issue #9300 is OPEN with need-discussion, and the last thread comment (@doudouOUC, 2026-08-17T05:43:02Z) keeps the gate open — no maintainer direction call has been recorded since, and no approval exists on the PR (all PR comments are bot-authored; the single human review is a token-permission test, not a direction call). The gate's named follow-up is half-met: the design comment above rootHeight IS rewritten by this diff, but the PR description still does not say why #7652's removal of the earlier bottom-align attempt doesn't apply this time. Still stands — this is a product decision, not a code defect settleable from code; this round's three code Criticals (R17-1, R17-2, R17-3) are direction-neutral implementation defects and do not supersede it. Failure scenario: merging before the direction call ships a contested layout and re-locks the opposite contract in the three rewritten test pins; if the maintainer keeps the #7652 collapse layout, the change must be reverted together with its inverted test pins. Witness: gh live fetch this round — issue #9300 state OPEN, labels include need-discussion, updatedAt 2026-08-17T05:43:02Z unchanged since the gate-open comment; @doudouOUC 2026-08-17T05:43:02Z: 'the remaining blocker is purely the product-direction call, which needs maintainer sign-off — keeping need-discussion in place'; gh pr view 9305 → reviewDecision CHANGES_REQUESTED, 0 human approvals. On the code side this round: the twelve previously-reported implementation Criticals (R4-1, R5-1/2/3, R6-1/2, R8-1, R10-1, R11-1/4/6, R14-1) were re-checked against the reviewed commit and their mechanisms are verifiably closed by this diff (each pinned by a regression test in this diff; the full packages/cli suite is green apart from pre-existing environment-dependent settings.test.ts failures that fail identically at the merge base); three NEW implementation Criticals were found and probe-verified (R17-1, R17-2, R17-3).
中文说明
仅完成部分审查，审查缺口已披露。
本轮确认的 4 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally (the macOS/Windows unit-test matrix jobs were also skipped; the diff is platform-independent TypeScript, and the ubuntu unit-test job plus this review's full packages/cli suite passed)。
未审查：test-efficacy — probe harness could not be validated in this repo (fresh-checkout prerequisite guard; harnessValidated null, 0 mutants and 0 hunk probes executed by the harness); verifiers ran manual flip-checked probes and mutants instead。
收敛姿态下延后（第 17 轮，非阻断）——已记录，本轮不要求修改：共 2 条（原文未翻译，列表见上方英文部分）。
[Critical] R2-1 Triage stage-1c direction gate: this PR reverses the intentional, test-locked collapse layout from #7652 and remains held pending a maintainer product-direction decision on #9300 — whether bottom-aligning short VP content (blank space above the conversation, latest message directly above the composer) is the intended layout. Re-checked this round against the live issue via a fresh fetch: issue #9300 is OPEN with need-discussion, and the last thread comment (@doudouOUC, 2026-08-17T05:43:02Z) keeps the gate open — no maintainer direction call has been recorded since, and no approval exists on the PR (all PR comments are bot-authored; the single human review is a token-permission test, not a direction call). The gate's named follow-up is half-met: the design comment above rootHeight IS rewritten by this diff, but the PR description still does not say why #7652's removal of the earlier bottom-align attempt doesn't apply this time. Still stands — this is a product decision, not a code defect settleable from code; this round's three code Criticals (R17-1, R17-2, R17-3) are direction-neutral implementation defects and do not supersede it. Failure scenario: merging before the direction call ships a contested layout and re-locks the opposite contract in the three rewritten test pins; if the maintainer keeps the #7652 collapse layout, the change must be reverted together with its inverted test pins. Witness: gh live fetch this round — issue #9300 state OPEN, labels include need-discussion, updatedAt 2026-08-17T05:43:02Z unchanged since the gate-open comment; @doudouOUC 2026-08-17T05:43:02Z: 'the remaining blocker is purely the product-direction call, which needs maintainer sign-off — keeping need-discussion in place'; gh pr view 9305 → reviewDecision CHANGES_REQUESTED, 0 human approvals. On the code side this round: the twelve previously-reported implementation Criticals (R4-1, R5-1/2/3, R6-1/2, R8-1, R10-1, R11-1/4/6, R14-1) were re-checked against the reviewed commit and their mechanisms are verifiably closed by this diff (each pinned by a regression test in this diff; the full packages/cli suite is green apart from pre-existing environment-dependent settings.test.ts failures that fail identically at the merge base); three NEW implementation Criticals were found and probe-verified (R17-1, R17-2, R17-3).
— qwen3.8-max via Qwen Code /review (v0.21.15)
| (clampParked && | ||
| contentPreviouslyFit && | ||
| totalHeight > scrollableContainerHeight) || |
There was a problem hiding this comment.
[Critical] R17-1: The fit→overflow crossing arm of the auto-scroll gate has no data.length > 1 guard, so it can re-latch sticking on the banner-only remnant — undoing the droppingSticking = data.length <= 1 contract this same diff introduces. The mark installed at {0,0} by the released arm on a shrink-to-fit survives a shrink-to-banner: the collapse render parks at the same {0,0} anchor, so the anchor-mismatch guard skips the inner block and the mark is neither reinstalled nor cleared. A later terminal shrink (or a banner reflow on width change) crossing the banner into overflow then fires this arm — clampParked, contentPreviouslyFit and totalHeight > scrollableContainerHeight all hold — queuing setIsStickingToBottom(true) and the END anchor. The drop branch is unreachable afterwards (END anchor in range), so the banner-only state carries sticking again: a fitting banner bottom-aligns under a blank viewport — the exact frame the R5-1 test pins as top-aligned — and the first post-clear message yanks the viewport to the end and latches auto-follow, the R6-2 regression re-entered through the crossing arm. Each trigger step is an ordinary user action: scroll up → content or terminal change that fits → /clear → terminal shrink or banner reflow.
Witness (probe at the reviewed commit, unmodified PR code, flip-checked): after the shrink-to-overflow — ["b1","b2"], viewport yanked to the banner's bottom with sticking re-latched at data.length === 1; after grow-back-to-fit — ["","","","","","","","b0","b1","b2"], fitting banner bottom-aligned under a blank viewport; first post-clear message — ["","","","","","","b0","b1","b2","message"], yank to END with follow latched. With the guard below the probe flips to top-aligned frames and all 48 tests stay green.
| (clampParked && | |
| contentPreviouslyFit && | |
| totalHeight > scrollableContainerHeight) || | |
| (clampParked && | |
| contentPreviouslyFit && | |
| data.length > 1 && | |
| totalHeight > scrollableContainerHeight) || |
No pinned test needs this arm at data.length === 1: an overflow-installed mark (R6-1 shape) can only reach a contentPreviouslyFit render via a shrink that moves the anchor off the mark first, clearing it. Add a regression test: released shrink-to-fit → collapse to banner → shrink container past overflow → assert sticking stays released and the frame is not bottom-pinned.
中文说明
R17-1：自动滚动门控的 fit→overflow 跨越分支缺少 data.length > 1 守卫，因此可能在仅剩 banner 的残余上重新置位贴底 —— 推翻了同一 diff 引入的 droppingSticking = data.length <= 1 约定。released 分支在"收缩到放得下"时安装于 {0,0} 的标记，会存活过"收缩到仅剩 banner"：坍缩渲染停驻在相同的 {0,0} 锚点，锚点不匹配守卫跳过内层块，标记既未重装也未清除。随后终端收缩（或宽度变化导致 banner 重排变高）使 banner 跨过溢出时，本分支触发 —— clampParked、contentPreviouslyFit、totalHeight > scrollableContainerHeight 全部成立 —— 排队执行 setIsStickingToBottom(true) 与 END 锚点。此后 drop 分支不可达（END 锚点在范围内），于是 banner-only 状态重新携带贴底：放得下的 banner 被底部对齐到空白视口之下 —— 正是 R5-1 测试锁定为顶部对齐的帧 —— 且 /clear 后第一条消息把视口拽到末尾并锁住自动跟随，R6-2 回归经由跨越分支重新进入。每个触发步骤都是普通用户操作：上滚 → 内容或终端变化至放得下 → /clear → 终端收缩或 banner 重排。
证据（探针，在受审提交上运行，未改动的 PR 代码，可翻转验证）：收缩到溢出之后 —— ["b1","b2"]，视口被拽到 banner 底部，且在 data.length === 1 时贴底重新置位；重新增长到放得下之后 —— ["","","","","","","","b0","b1","b2"]，放得下的 banner 被底部对齐在空白视口之下；/clear 后第一条消息 —— ["","","","","","","b0","b1","b2","message"]，拽到 END 且跟随被锁住。加入下方守卫后探针翻转为顶部对齐帧，且全部 48 个测试保持绿色。
（修复见上方 suggestion 代码块。）
没有任何锁定测试需要本分支在 data.length === 1 时生效：溢出时安装的标记（R6-1 形态）只能经由一次"先把锚点移离标记、从而清除标记"的收缩到达 contentPreviouslyFit 渲染。补充回归测试：released 收缩到放得下 → 坍缩到仅剩 banner → 容器收缩至跨过溢出 → 断言贴底保持解除且帧不被钉在底部。
— qwen3.8-max via Qwen Code /review (v0.21.15)
| // growth auto-follow, defending nothing — the re-stick gate is | ||
| // already blocked by `!contentPreviouslyFit` (#9305 reviews R8-1, | ||
| // R10-1). | ||
| if (newScrollTop > 0 || (!isStickingToBottom && data.length > 1)) { |
There was a problem hiding this comment.
[Critical] R17-2: The overflow-park mark arm (newScrollTop > 0) installs the clamp mark on EVERY released drop-branch park — including a wholesale dataset swap into an overflowing session (/resume; ScrollableList carries no key, so list state survives the swap) — where the park lands at the live bottom of the new dataset. The mark then suppresses all three re-follow paths: the re-stick gate needs !clampParked, the growth arm needs wasAtBottom && !clampParked, and the crossing arm needs contentPreviouslyFit (false for an overflowing swap) — so auto-follow stays dead until the user manually scrolls.
Failure scenario: bottom-stuck in session A, the user scrolls up to read (sticking released); /resume swaps in session B whose height overflows; the carried anchor is out of range, so the drop branch parks the viewport at B's live bottom and this arm installs the mark; the first new messages in B then render below the fold and follow never re-engages until manual scroll.
Witness (A/B at the reviewed commit vs the merge base, identical probe on both trees): PR — park b-5..b-14, growth #1 and #2 leave the frame frozen at b-5..b-14 with b-15/b-16 below the fold; BASE (df768f091) — growth follows to b-6..b-15, then b-7..b-16. Pre-PR the bottom-pixels tolerance re-latched sticking on that growth, so this is a regression introduced by the mark mechanism. The existing "across a wholesale dataset replacement" test only covers a swap into a FITTING session (drop branch re-anchors to {0,0}); the swap-to-an-overflowing session is undefended.
Suggested fix: separate the states the shared arm serves rather than gating on remnant size — a remnant-size gate (newScrollTop > 0 && data.length <= 1) flips the probe but fails the R5-3/R6-2 two-step-shrink pin (47/48, verified), because the overflow arm also defends the scrolled-away clamp-park on a still-overflowing remnant. Distinguish the entries instead: the swap enters the drop branch through an out-of-range carried anchor (scrollAnchor.index >= data.length), the shrink case through an in-range anchor whose position overflowed — skip the mark on the swap entry. Add a regression test: released swap into an overflowing session → growth must follow.
中文说明
R17-2：溢出停驻安装分支（newScrollTop > 0）会在每一次 released 的 drop 分支停驻时安装钳制标记 —— 包括整体换入溢出会话的数据集交换（/resume；ScrollableList 不带 key，列表状态在交换后存活）—— 此时停驻落在新数据集的实时底部。标记随后封死全部三条重新跟随路径：re-stick 门控需要 !clampParked，增长分支需要 wasAtBottom && !clampParked，跨越分支需要 contentPreviouslyFit（溢出交换下为假）—— 于是自动跟随一直失效，直到用户手动滚动。
失败场景：在会话 A 贴底时上滚阅读（贴底解除）；/resume 换入高度溢出的会话 B；携带锚点越界，drop 分支把视口停驻在 B 的实时底部，本分支安装标记；此后 B 中的新消息渲染在折线之下，跟随不再重新置位，直到手动滚动。
证据（在受审提交与合并基线上的 A/B，同一探针跑两棵树）：PR —— 停驻 b-5..b-14，增长 1、增长 2 后帧冻结在 b-5..b-14，b-15/b-16 在折线之下；基线（df768f091）—— 增长跟随到 b-6..b-15、b-7..b-16。改动前 bottom-pixels 容差会在该增长上重新置位贴底，因此这是标记机制引入的回归。既有的"整体数据集替换"测试只覆盖换入"放得下"的会话（drop 分支重锚到 {0,0}）；换入溢出会话没有防御。
建议修复：把该分支服务的两种状态区分开，而不是按残余长度门控 —— 残余长度门控（newScrollTop > 0 && data.length <= 1）能翻转探针，但会使 R5-3/R6-2 两步收缩锁定测试失败（47/48，已验证），因为溢出分支同时还防御"已上滚用户停驻在仍溢出的残余底部"。应改为区分入口：交换经由越界的携带锚点（scrollAnchor.index >= data.length）进入 drop 分支，收缩场景经由位置溢出的范围内锚点进入 —— 在交换入口跳过标记安装。补充回归测试：released 状态换入溢出会话 → 增长必须跟随。
— qwen3.8-max via Qwen Code /review (v0.21.15)
| if (reAnchorClampMark.current !== null && !clampParked) { | ||
| reAnchorClampMark.current = null; | ||
| } |
There was a problem hiding this comment.
[Critical] R17-3: The clamp mark's only removal path is anchor movement — it is never invalidated on dataset-identity change, so a mark installed in one session survives a wholesale /resume dataset swap and suppresses auto-follow in the new session. This is distinct from R17-2: here the swap bypasses the drop branch entirely — the carried anchor stays in range in the new dataset and its position does not exceed the new maxScroll — so no arm fires, the anchor still equals the mark, clampParked stays true and the mark is kept; a fix scoped to separating drop-branch entry states (R17-2's territory) does not clear this stale mark.
Failure scenario: session A overflowing, the user scrolls away, content shrinks → the drop branch parks at the shrunken bottom and the overflow arm installs the mark with sticking released. /resume swaps session B in place. All three re-follow paths stay suppressed by session A's mark — the re-stick gate needs !clampParked, the growth arm needs !clampParked, the crossing arm needs contentPreviouslyFit (false, the carried geometry overflowed) — so session B's first streamed messages render below the fold and the frame stays frozen until the user manually scrolls the anchor off the mark.
Witness (probe at the reviewed commit): park in A item-5..14 → parked swap into B item-105..114 → growth leaves the frame frozen at item-105..114 (B's first streamed messages below the fold); scrollToEnd recovers follow (manual-scroll escape confirmed). A per-mark-key fix — store the keyExtractor key of the mark's item at install and null the mark when the key at that index no longer matches — flips the probe and keeps all 48 tests green. Note: a data.length + first/last-key fingerprint would NOT work — a new item changes the last key on every growth, clearing the mark during normal streaming and reintroducing the R6-2 yank (traced).
Suggested fix: give the mark dataset identity (the verified per-mark-key variant above) and add a regression test: released overflow park → swap into an in-range session → growth must follow.
中文说明
R17-3：钳制标记唯一的清除路径是锚点移动 —— 数据集身份变化时它从不失效，因此在一个会话中安装的标记会存活过整体 /resume 数据集交换，并在新会话中压制自动跟随。这与 R17-2 不同：此处交换完全绕过 drop 分支 —— 携带锚点在新数据集中仍在范围内，且位置不超过新的 maxScroll —— 于是没有任何分支触发，锚点仍等于标记，clampParked 保持为真，标记被保留；仅针对 drop 分支入口状态分离的修复（R17-2 的范畴）无法清除这个过期标记。
失败场景：会话 A 溢出，用户上滚离开底部，内容收缩 → drop 分支停驻在收缩后的底部，溢出分支在贴底解除时安装标记。/resume 就地换入会话 B。三条重新跟随路径全部被会话 A 的标记压制 —— re-stick 门控需要 !clampParked，增长分支需要 !clampParked，跨越分支需要 contentPreviouslyFit（携带几何溢出，为假）—— 于是会话 B 最初的流式消息渲染在折线之下，帧保持冻结，直到用户手动滚动把锚点移离标记。
证据（探针，在受审提交上运行）：A 中停驻 item-5..14 → 停驻状态换入 B item-105..114 → 增长后帧冻结在 item-105..114（B 的首条流式消息在折线之下）；scrollToEnd 恢复跟随（确认了手动滚动这一逃生路径）。按标记携带条目 key 的修复 —— 安装时记录标记条目的 keyExtractor key，当该索引处的 key 不再匹配时置空标记 —— 能翻转探针且全部 48 个测试保持绿色。注意：data.length + 首/末 key 指纹方案不可行 —— 每次增长新条目都会改变末 key，使标记在正常流式输出中被清除，重新引入 R6-2 的拽动（已推演）。
建议修复：给标记加上数据集身份（上述已验证的按标记 key 变体），并补充回归测试：released 溢出停驻 → 换入锚点仍在范围内的会话 → 增长必须跟随。
— qwen3.8-max via Qwen Code /review (v0.21.15)
|
🤖 Addressed the latest review feedback (round 9/10). What changed, and what I pushed back on: · 已处理最新评审反馈（第 9/10 轮）。改动内容与我反驳保留之处如下： Autofix round summary — PR #9305 (round 18)Critical-only mode is active; this round addressed the three new implementation Feedback points and dispositions[rc:3835091829] R17-1 — crossing arm missing [rc:3835091834] R17-2 — overflow-park mark installed on dataset-swap parks → Resolved. [rc:3835091835] R17-3 — mark never invalidated on dataset-identity change → Resolved. [rv:4998951719] R2-1 — triage stage-1c direction gate → Escalated for a maintainer decision. Not actionable this round (per Critical-only mode): the 4 Suggestion-level Changes
No conflict work ( VerificationAll commands run in this round, with results:
中文说明Autofix 轮次总结 — PR #9305（第 18 轮）当前处于仅处理 Critical 的模式；本轮处理了三条新的实现类 Critical（R17-1、R17-2、R17-3）。三条均先在受审提交上复现，再做根因修复，并各自补充了回归测试、通过变异探针验证。方向门控（R2-1）属于维护者的产品决策，下方以明确的开放问题形式上报。提交： 反馈条目与处置[rc:3835091829] R17-1 — 跨越分支缺少 [rc:3835091834] R17-2 — 溢出停驻标记被安装到数据集交换的停驻上 → 已解决。 [rc:3835091835] R17-3 — 标记在数据集身份变化时从不失效 → 已解决。 [rv:4998951719] R2-1 — Triage stage-1c 方向门控 → 上报维护者决策。 本轮不可执行（仅处理 Critical 模式）： 4 条已确认在 PR 上报告过的建议级发现（fitting-content no-op 守卫的对称性、maxScroll 重复计算、banner-only 谓词在共享组件内重推、贴底跃迁的 debugLogger 覆盖）以及 2 条收敛姿态下的延后项（ 变更内容
无冲突处理（ 验证本轮实际执行的命令及结果：
Deferred non-Critical feedbackCritical-only mode is active: 5 change-producing rounds are complete. The workflow excluded the non-Critical feedback below from this round's actionable sections; the items remain open for human follow-up. Maintainer feedback is deferred only after its author has used 2 regular feedback batches in this window's Critical-only tail; authors at that budget, if any, are named below. ( 中文说明已进入仅处理 Critical 的模式：已完成 5 个产生改动的轮次。本轮可执行区域已排除下方非 Critical 反馈；这些条目保持开放，留待人工跟进。维护者反馈仅在其本人于本窗口 Critical-only 阶段已使用 2 批常规反馈预算后才会延后；达到预算的作者（如有）在下方点名。（评论 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 10 this bot stops and leaves the PR for a human. · 有空请复审；第 10 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
5 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- the fitting-content no-op guard missing from scrollToIndex/scrollToItem while pasted into scrollBy/scrollTo/scrollToEnd (VirtualizedList.tsx) — already recorded in the round-6 deferral list (review 4979659125), round-8 deferral D8-1 (review…
- the fitting-content no-op rule pasted into scrollBy/scrollTo/scrollToEnd, each recomputing maxScroll (VirtualizedList.tsx) — already recorded in the round-8 deferral list as D8-1 (review 4981837967)
- the host's banner-only predicate re-derived inside the shared component as data.length <= 1 (VirtualizedList.tsx) — already recorded in the round-6 deferral list (review 4979659125) and the rounds 13-17 already-reported lists
- zero debugLogger coverage of sticking transitions in the clamp-mark state machine (VirtualizedList.tsx) — already recorded in the round-8 deferral list as D8-3 (review 4981837967) and the rounds 13-17 already-reported lists
- the re-stick gate's positive path has zero test coverage — deleting the gate block survives the suite (VirtualizedList.tsx) — already recorded in the round-6 deferral list (review 4979659125)
Not reviewed: test-efficacy — probe harness could not be validated in this repo (fresh-checkout prerequisite guard; harnessValidated null, 0 mutants and 0 hunk probes executed); verifiers ran manual flip-checked probes instead.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally (the macOS/Windows unit-test matrix jobs were also skipped; the diff is platform-independent TypeScript, and the ubuntu unit-test job plus this review's full packages/cli suite passed).
Test Plan (not a blocker): 32 tests pass — this review observed 22912 passed.
[Critical] R2-1 Triage stage-1c direction gate: this PR reverses the intentional, test-locked collapse layout from #7652 and remains held pending a maintainer product-direction decision on #9300 — whether bottom-aligning short VP content (blank space above the conversation, latest message directly above the composer) is the intended layout. Re-checked this round against the live issue: issue #9300 is OPEN with need-discussion, and the last thread comment (@doudouOUC, 2026-08-17T05:43:02Z) keeps the gate open — no maintainer direction call has been recorded since, and no approval exists on the PR (the sole human review is a token-permission test, not a direction call). The gate's named follow-up is half-met: the design comment above rootHeight IS rewritten by this diff, but the PR description still does not say why #7652's removal of the earlier bottom-align attempt doesn't apply this time. Still stands — this is a product decision, not a code defect settleable from code. Failure scenario: merging before the direction call ships a contested layout and re-locks the opposite contract in the three rewritten test pins; if the maintainer keeps the #7652 collapse layout, the change must be reverted together with its inverted test pins. Witness: gh live fetch this round — issue #9300 state OPEN, labels include need-discussion, updatedAt 2026-08-17T05:43:02Z unchanged since the gate-open comment; gh pr view 9305 → reviewDecision CHANGES_REQUESTED, 0 human approvals.
中文说明
仅完成部分审查，审查缺口已披露。
本轮确认的 5 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
未审查：test-efficacy — probe harness could not be validated in this repo (fresh-checkout prerequisite guard; harnessValidated null, 0 mutants and 0 hunk probes executed); verifiers ran manual flip-checked probes instead。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally (the macOS/Windows unit-test matrix jobs were also skipped; the diff is platform-independent TypeScript, and the ubuntu unit-test job plus this review's full packages/cli suite passed)。
Test Plan（非阻断）：32 tests pass — this review observed 22912 passed。
[Critical] R2-1 Triage stage-1c direction gate: this PR reverses the intentional, test-locked collapse layout from #7652 and remains held pending a maintainer product-direction decision on #9300 — whether bottom-aligning short VP content (blank space above the conversation, latest message directly above the composer) is the intended layout. Re-checked this round against the live issue: issue #9300 is OPEN with need-discussion, and the last thread comment (@doudouOUC, 2026-08-17T05:43:02Z) keeps the gate open — no maintainer direction call has been recorded since, and no approval exists on the PR (the sole human review is a token-permission test, not a direction call). The gate's named follow-up is half-met: the design comment above rootHeight IS rewritten by this diff, but the PR description still does not say why #7652's removal of the earlier bottom-align attempt doesn't apply this time. Still stands — this is a product decision, not a code defect settleable from code. Failure scenario: merging before the direction call ships a contested layout and re-locks the opposite contract in the three rewritten test pins; if the maintainer keeps the #7652 collapse layout, the change must be reverted together with its inverted test pins. Witness: gh live fetch this round — issue #9300 state OPEN, labels include need-discussion, updatedAt 2026-08-17T05:43:02Z unchanged since the gate-open comment; gh pr view 9305 → reviewDecision CHANGES_REQUESTED, 0 human approvals.
— qwen3.8-max via Qwen Code /review (v0.21.15)
| const clampParked = | ||
| clampMark !== null && | ||
| clampMarkItem !== undefined && | ||
| scrollAnchor.index === clampMark.index && | ||
| scrollAnchor.offset === clampMark.offset && | ||
| keyExtractor(clampMarkItem, clampMark.index) === clampMark.key; |
There was a problem hiding this comment.
[Critical] R18-1: The reAnchorClampMark follow-suppression mechanism is structurally unsound. It encodes "the user parked here — do not auto-follow" as a mark keyed on (item index, offset, item key), and none of those three reliably means what the mechanism needs: item keys are constant for the vp-banner sentinel and unstable for pending items (p-N→h-N), and index/offset survive or vanish across /resume dataset swaps and upstream removals without being tracked.
This round probe-verified eight distinct failures of this one mechanism; the round-17 fixes (key-based invalidation, the swapEntry out-of-range check, the data.length > 1 crossing guard) created several of them:
- A mark keyed on the constant
'vp-banner'key survives a/resumeswap into an overflowing session — auto-follow dead in the new session (flip-checked probe). - The same key comparison clears the mark on a pending→commit re-key while parked — the re-stick gate then yanks the scrolled-away user back to the bottom (flip-checked probe; narrow window).
- A
/clearbanner-overflow park installs a mark that suppresses every re-follow path — the entire next conversation streams below the fold until manual scroll (flip-checked probe). - A mark-less fitting banner-only park that later overflows has no re-follow path at all — the crossing arm requires
clampParked(flip-checked probe). - An in-range-but-past-
maxScroll/resumeswap is misclassified (swapEntryonly detects out-of-range anchors) and installs a mark on swap-parked content — the case the install comment forbids (flip-checked probe). - An R11-6 shrink-to-fit park mark inherits across
/clear(the drop branch never fires for a fitting remnant) and suppresses the next conversation (flip-checked probe). - An overflowing in-place shrink parks at the live bottom and installs a mark no growth path ever clears — follow dead permanently; ruled a defect, not intended R5-3 suppression (flip-checked probe).
- An upstream removal before the marked index shifts the index and clears the mark via key mismatch, re-entering the yank class (low confidence).
Witness (probe at the unmodified commit, banner 15 rows / container 10): after /clear the frame freezes at b5..b14 across four growth renders — every new message below the fold; a flip-checked candidate fix restores follow with all 51 tests green.
Eighteen rounds of per-entrance patches (R4→R17) have each produced new failures here; the surface cannot be closed entrance by entrance. Close it structurally: give the component an explicit dataset-identity and user-intent signal — key the host's ScrollableList by session so /clear//resume remount and reset carried state by construction, or pass a session-generation/dataset-id prop that resets the mark — and represent "the user deliberately parked" as a flag set only by a real user scroll, not by content-driven clamps.
中文说明
[Critical] R18-1：reAnchorClampMark 的跟随抑制机制在结构上不可靠。它把"用户停在此处——不要自动跟随"编码为以 (条目索引, 偏移, 条目 key) 为键的标记，但三者都不可靠地表达该机制所需的语义：条目 key 对 vp-banner 哨兵是常量、对 pending 条目不稳定（p-N→h-N），而索引/偏移会在 /resume 数据集交换与上游删除时存续或消失，却都不被追踪。
本轮经探针验证了该机制的八个不同失效；第 17 轮的修复（基于 key 的失效、swapEntry 越界检查、data.length > 1 跨越守卫）本身引入了其中数个：
- 以常量
'vp-banner'key 为键的标记会存活过换入溢出会话的/resume交换——新会话中自动跟随失效（翻转探针验证）。 - 同一 key 比较会在停驻期间因 pending→commit 换 key 而清除标记——re-stick 门控随后把已上滚的用户拽回底部（翻转探针验证；窗口较窄）。
/clear的 banner 溢出停驻安装的标记封死所有重新跟随路径——下一整段会话都在折线之下流式输出，直到手动滚动（翻转探针验证）。- 无标记的、放得下的 banner-only 停驻随后溢出时完全没有重新跟随路径——跨越分支需要
clampParked（翻转探针验证）。 - 范围内但越过
maxScroll的/resume交换被误分类（swapEntry只检测越界锚点），并在交换停驻的内容上安装标记——正是安装注释所禁止的情形（翻转探针验证）。 - R11-6 的收缩到放得下的停驻标记会跨过
/clear继承（放得下的残余不触发 drop 分支），压制下一段会话（翻转探针验证）。 - 溢出的原地收缩停驻在实时底部，安装的标记没有任何增长路径能清除——跟随永久失效；已裁定为缺陷而非预期的 R5-3 抑制（翻转探针验证）。
- 被标记索引之前的上游删除会移动索引并经由 key 不匹配清除标记，重新进入拽动类别（低置信度）。
证据（在未改动提交上的探针，banner 15 行/容器 10 行）：/clear 之后帧在四次增长渲染中冻结于 b5..b14——每条新消息都在折线之下；翻转验证的候选修复恢复跟随，且全部 51 个测试保持绿色。
18 轮逐入口修补（R4→R17）每次都在此产生新失效；该表面无法逐入口封闭。请从结构上封闭：给组件显式的数据集身份与用户意图信号——给宿主的 ScrollableList 加上按会话的 key，使 /clear//resume 重新挂载、从构造上重置携带状态，或传入会重置标记的会话代/数据集 id prop——并把"用户有意停驻"表示为仅由真实用户滚动置位的标志，而非内容驱动的钳制。
— qwen3.8-max via Qwen Code /review (v0.21.15)
|
🤖 Addressed the latest review feedback (round 10/10). What changed, and what I pushed back on: · 已处理最新评审反馈（第 10/10 轮）。改动内容与我反驳保留之处如下： Address-review summary — PR #9305 (round: Critical-only)Critical-only mode is active (5 change-producing rounds completed). The deferred Feedback dispositions[rv:4999795201] Critical R2-1 — Triage stage-1c direction gate → Escalated (needs a maintainer decision), unresolvedThe gate asks whether bottom-aligning short VP content (blank space above the (The same review body's 5 Suggestion-level already-reported findings and its [rc:3835659553] Critical R18-1 —
|
| Probe | Pre-round result |
|---|---|
| F2 shape: re-key the parked item, then cross fit→overflow | follow dead (frame clips at top instead of following) |
| F7 shape: in-place shrink parks at the live bottom, then growth | follow dead permanently (frame freezes below the fold) |
Host: session id swap (/clear//resume boundary) |
no remount — carried anchor/sticking/mark leak into the new dataset |
Fix (structural, per the finding's prescription).
- Dataset identity — host session key.
MainContentnow keys the VP
ScrollableListbysessionStats.sessionId. Both/clearand/resume
flow throughstartNewSession(verified inclearCommand.tsand
useResumeCommand.ts), so the list remounts on exactly those boundaries
and carried scroll state (anchor, sticking, park mark) resets by
construction. Same-session re-renders do NOT remount. This closes the
whole cross-dataset failure class — F1, F3, F5, F6 — structurally: a
mark can no longer survive/clear//resumebecause no state survives.
historyRemountKeywas deliberately NOT used (it also bumps on model
change, Ctrl+O, and terminal-shrink reflow, which would yank mid-session
scroll positions). - Positional park mark — fragile key identity removed. The mark no longer
stores/compares an item key. Keys are exactly the unreliable identity the
finding names: constant for thevp-bannersentinel, unstable across
pending→commit (p-N→h-N). Validation is now pure position (the anchor
still sits where the clamp parked it); any real scroll moves the anchor and
clears the mark; dataset swaps never reach it (point 1). This fixes F2
(re-key no longer clears the mark → no yank) and F8 (upstream removal
no longer clears it via key mismatch; low-confidence item, same deleted
code path). allowFollow— live-bottom parks get a follow path. The mark records
whether the park landed in a live multi-item remnant. Growth re-engages
follow from a multi-item live-bottom park (it IS the bottom of a live
conversation) — fixing F7's permanent suppression — while banner-only
remnants keep their R6-2/R17-1 suppression (no conversation to follow).
Mutation-proven in both directions (see Verification).
Sub-finding F4 declined with evidence. The mark-less fitting banner-only
state that overflows in place has no re-follow path — and that is the pinned,
intended behavior: R17-1 explicitly locks that banner-only remnants never
cross back (there is no conversation to follow), and the host mounts
banner-only sessions top-anchored (initialScrollIndex 0). The carried-park
variant of this shape is gone anyway (session remount). Adding a re-follow
path here would break the R17-1 pin the same review process installed.
Test changes.
- 3 new regression tests (MainContent remount pin; F2 probe; F7 probe) — all
fail on the pre-round branch, pass post-fix. - The former "invalidates the clamp mark when a dataset swap keeps the anchor
in range" test was renamed and its comment reframed: its user-visible
contract (follow stays alive in the replaced dataset) is preserved, but the
mechanism assertion changed — the old test's post-swap END frame came from
the gate latching after a key-mismatch clear (the yank class this finding
condemns); the new code reaches it through the live-bottom-park follow arm. - All 51 pre-existing component pins pass unchanged, including R5-1/R5-2/R5-3,
R6-1/R6-2, R8-1, R10-1, R11-1/R11-4/R11-6, R14-1, R17-1/R17-2.
Conflict notes
--conflict false; no merge performed, none needed.
Diff-growth note
The round is root-cause and partially subtractive: the mark's key field, the
key-comparison validation, and the swap-detection narratives built up over
R17-2/R17-3 were removed, replaced by one host attribute, one boolean on the
existing mark, and one growth arm.
Verification
- Reproduction probes on pre-round commit
839a87329b— 3/3 FAILED (as required for a defect fix) npx vitest run src/ui/components/shared/VirtualizedList.test.tsx src/ui/components/MainContent.test.tsx(packages/cli) — 87 passed (2 files)- Mutation probe M1: drop
key={sessionStats.sessionId}→ remount test FAILS; restored → green - Mutation probe M2: delete the live-bottom-park growth arm → F7 probe FAILS; restored → green
- Mutation probe M3: force
allowFollow: true→ R6-2 pin (does not yank the viewport to the first post-clear message) and R11-1 pin (does not preempt the sticking drop...) FAIL; restored → green - Mutation probe M4 (flip-check): the pre-round state IS the key-comparison configuration; the F2 probe fails there and passes with it removed
npm run build— passed (exit 0)npm run typecheck— passed (exit 0)npm run lint(full repo) — passed (exit 0)npx prettier --checkon the four touched files — passed- Full packages/cli suite (
npx vitest run) — 22895 passed, 33 failed in 10 files; ALL 33 are pre-existing environment failures in files the diff does not touch and does not share an import graph with (src/config/*sandbox-image/home-directory tests,src/serve/live/*, Footer snapshots, AuthDialog endpoint-metadata, command tests); this runner has no docker/podman, soconfig.getSandbox()is undefined there. None importMainContent/VirtualizedList/ScrollableList(grep-verified). - Integration tests — not applicable: the touched behavior is exercised by the unit suites (ink-testing-library harness), not only through the bundled CLI/integration harness.
npm run generate:settings-schema— not applicable (no settings source touched).
中文说明
处理审查总结 — PR #9305（本轮：仅 Critical）
当前处于仅处理 Critical 的模式（已完成 5 个产生改动的轮次）。被延后的非 Critical 部分未被触碰。本轮可处理两个 Critical 条目：一个行内 Critical 已在代码中解决，一个审查体 Critical 属于维护者决策，保持开放。
反馈处理
[rv:4999795201] Critical R2-1 — Triage stage-1c 方向门控 → 升级（需维护者决策），未解决
该门控询问：将较短的 VP 内容底部对齐（会话上方留白、最新消息紧贴输入框上方）是否是预期布局——这反转了 #7652 中经测试锁定的折叠布局。Issue #9300 处于 OPEN 状态、带 need-discussion 标签，且尚未记录任何维护者的方向性决定。这是产品决策，不是本检出能裁定的代码缺陷——实现或拒绝都不该由本轮决定。该条目保持未解决，等待维护者在 #9300 上给出答复；答复将在下一轮作为普通反馈到达。本轮未因此改动任何代码。
（同一审查体中 5 条建议级的已报告发现及其披露的未审查区域，本轮无需处理。）
[rc:3835659553] Critical R18-1 — reAnchorClampMark 结构性不可靠 → 已在代码中解决
先复现。 编写了三个聚焦探针，确认其在轮前提交（839a87329b）上失败，与发现的诊断一致：
| 探针 | 轮前结果 |
|---|---|
| F2 形态：对停驻条目换 key，然后跨越"放得下→溢出" | 跟随失效（帧停在顶部而非跟随） |
| F7 形态：原地收缩停驻在实时底部，随后增长 | 跟随永久失效（帧冻结在折线之下） |
宿主层：会话 id 交换（/clear//resume 边界） |
未重新挂载——携带的锚点/吸附/标记泄漏进新数据集 |
修复（结构性，按发现所给方案）。
- 数据集身份——宿主会话 key。
MainContent现在以sessionStats.sessionId作为 VPScrollableList的 key。/clear与/resume都经由startNewSession（已在clearCommand.ts与useResumeCommand.ts中核实），因此列表恰好在这两个边界上重新挂载，携带的滚动状态（锚点、吸附、停驻标记）从构造上被重置。同会话内的重新渲染不会重新挂载。这在结构上关闭了整个跨数据集失效类别——F1、F3、F5、F6：标记不再可能存活过/clear//resume，因为没有任何状态能存活。刻意未使用historyRemountKey（它在模型切换、Ctrl+O、终端收缩重排时也会递增，会把会话中途的滚动位置拽走）。 - 位置式停驻标记——移除脆弱的 key 身份。 标记不再存储/比较条目 key。key 正是发现所指出的不可靠身份：对
vp-banner哨兵是常量、对 pending→commit 不稳定（p-N→h-N）。校验现在是纯位置的（锚点仍停在钳制停放处）；任何真实滚动都会移动锚点并清除标记；数据集交换到不了它（见第 1 点）。这修复了 F2（换 key 不再清除标记 → 不再拽动）与 F8（上游删除不再经由 key 不匹配清除标记；低置信度条目，同一被删除代码路径）。 allowFollow——实时底部停驻获得跟随路径。 标记记录停驻是否落在多条目实时会话中。增长会从多条目实时底部停驻处重新启用跟随（它本就是一段活跃会话的底部）——修复 F7 的永久抑制——而仅剩 banner 的残余保持 R6-2/R17-1 的抑制（没有会话可跟随）。两个方向均经变异探针验证（见验证部分）。
子发现 F4 以证据拒绝。 无标记的、放得下的 banner-only 状态在原地溢出时没有重新跟随路径——而这正是被锁定的预期行为：R17-1 明确锁定仅剩 banner 的残余永不跨回（没有会话可跟随），且宿主以顶部锚定挂载 banner-only 会话（initialScrollIndex 0）。该形态的携带停驻变体也已消失（会话重挂载）。在此添加重新跟随路径会破坏同一审查流程安装的 R17-1 锁定。
测试改动。
- 3 个新回归测试（MainContent 重挂载锁定；F2 探针；F7 探针）——全部在轮前分支上失败、修复后通过。
- 原 "invalidates the clamp mark when a dataset swap keeps the anchor in range" 测试更名并重构注释：其用户可见契约（替换后的数据集中跟随保持存活）被保留，但机制断言已变——旧测试中交换后的 END 帧来自 key 不匹配清除后门控的锁存（正是本发现谴责的拽动类别）；新代码经由实时底部停驻的跟随分支达到同一帧。
- 全部 51 个既有组件锁定测试原样通过，包括 R5-1/R5-2/R5-3、R6-1/R6-2、R8-1、R10-1、R11-1/R11-4/R11-6、R14-1、R17-1/R17-2。
冲突说明
--conflict false；未执行合并，也无需合并。
差异增长说明
本轮为根因修复且部分是减法：移除了标记的 key 字段、key 比较校验、以及 R17-2/R17-3 累积的交换检测叙述，替换为一个宿主属性、既有标记上的一个布尔值、以及一个增长分支。
验证
- 轮前提交
839a87329b上的复现探针 — 3/3 失败（缺陷修复的必要前提） npx vitest run src/ui/components/shared/VirtualizedList.test.tsx src/ui/components/MainContent.test.tsx（packages/cli）— 87 通过（2 个文件）- 变异探针 M1：去掉
key={sessionStats.sessionId}→ 重挂载测试失败；恢复 → 绿 - 变异探针 M2：删除实时底部停驻跟随分支 → F7 探针失败；恢复 → 绿
- 变异探针 M3：强制
allowFollow: true→ R6-2 锁定（does not yank the viewport to the first post-clear message）与 R11-1 锁定（does not preempt the sticking drop...）失败；恢复 → 绿 - 变异探针 M4（翻转核对）：轮前状态本身就是带 key 比较的配置；F2 探针在该配置下失败、移除后通过
npm run build— 通过（exit 0）npm run typecheck— 通过（exit 0）npm run lint（全仓）— 通过（exit 0）npx prettier --check（四个改动文件）— 通过- packages/cli 全量套件（
npx vitest run）— 22895 通过，10 个文件中 33 个失败；全部 33 个均为既有环境性失败，位于 diff 未触碰且与其无共享导入图的文件（src/config/*的 sandbox 镜像/家目录测试、src/serve/live/*、Footer 快照、AuthDialog 端点元数据、命令测试）；本 runner 无 docker/podman，因此这些用例中config.getSandbox()为 undefined。它们均不导入MainContent/VirtualizedList/ScrollableList（已用 grep 核实）。 - 集成测试 — 不适用：所触行为由单元套件（ink-testing-library 测试台）覆盖，并非仅经打包 CLI/集成测试台运行。
npm run generate:settings-schema— 不适用（未触碰任何 settings 源）。
Deferred non-Critical feedback
Critical-only mode is active: 5 change-producing rounds are complete. The workflow excluded the non-Critical feedback below from this round's actionable sections; the items remain open for human follow-up. Maintainer feedback is deferred only after its author has used 2 regular feedback batches in this window's Critical-only tail; authors at that budget, if any, are named below. (@qwen-code /retry starts a fresh counting window.)
中文说明
已进入仅处理 Critical 的模式：已完成 5 个产生改动的轮次。本轮可执行区域已排除下方非 Critical 反馈；这些条目保持开放，留待人工跟进。维护者反馈仅在其本人于本窗口 Critical-only 阶段已使用 2 批常规反馈预算后才会延后；达到预算的作者（如有）在下方点名。（评论 @qwen-code /retry 可开启新的计数窗口。）
Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。
Re-review when you have a moment. After round 10 this bot stops and leaves the PR for a human. · 有空请复审；第 10 轮后本 bot 停止并将 PR 交给人工。
🧠 Handled by Qwen Code · model/模型 qwen3.8-max
qwen-code-dev-bot
added
the
autofix/needs-human
|
⏸️ AutoFix paused: this PR reached its automatic round cap (10/10) and the loop will not manage it further — new feedback and base conflicts stay unhandled. Comment 中文说明⏸️ AutoFix 已暂停：本 PR 达到自动轮次上限（10/10），循环不再管理——新反馈与 base 冲突将无人处理。评论 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
7 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- the fitting-content no-op guard missing from scrollToIndex/scrollToItem while pasted into scrollBy/scrollTo/scrollToEnd — already recorded in the round-6 deferral list (review 4979659125), round-8 deferral D8-1 (review 4981837967) and the r…
- the fitting-content no-op rule pasted into scrollBy/scrollTo/scrollToEnd, each recomputing maxScroll — already recorded in the round-8 deferral list as D8-1 (review 4981837967)
- zero debugLogger coverage of sticking transitions in the clamp-mark state machine — already recorded in the round-8 deferral list as D8-3 (review 4981837967) and the rounds 13-18 already-reported lists
- the R5-2 no-op loop pins only the engaging direction; the release direction survives mutation — already recorded in the round-8 deferral list as D8-2 (review 4981837967)
- the host's banner-only predicate re-derived inside the shared component as data.length <= 1 — already recorded in the round-6 deferral list (review 4979659125) and the rounds 13-18 already-reported lists
- bottomAlignGap has no content-presence guard for an empty stuck list — already recorded in the round-5 deferral list (review 4978543596)
- the re-stick gate's positive path has zero test coverage — already recorded in the round-6 deferral list (review 4979659125)
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Not explored to full depth (tool budget reached): "agent 8a": none — I completed the diff walk, the source walk of VirtualizedList.tsx / MainContent.tsx , and the host sessionId-transition verification within budget. I di….
Deferred under the convergence posture (round 19, not a blocker) — recorded, not requested in this round:
packages/cli/src/ui/components/shared/VirtualizedList.test.tsx:301 — [review] three new test comments assert ScrollableList carries no key / /clear does not remount — contradicted by this PR's own session key (also at lines 1033 and 1187)
[Critical] R2-1 Triage stage-1c direction gate: this PR reverses the intentional, test-locked collapse layout from #7652 and remains held pending a maintainer product-direction decision on #9300 — whether bottom-aligning short VP content (blank space above the conversation, latest message directly above the composer) is the intended layout. Re-checked this round against the live issue via a fresh fetch: issue #9300 is OPEN with need-discussion, and the last thread comment (@doudouOUC, 2026-08-17T05:43:02Z) keeps the gate open — no maintainer direction call has been recorded since, and no approval exists on the PR (all PR comments are bot-authored; the sole human review is a token-permission test, not a direction call). The gate's named follow-up is half-met: the design comment above rootHeight IS rewritten by this diff, but the PR description still does not say why #7652's removal of the earlier bottom-align attempt doesn't apply this time. Still stands — this is a product decision, not a code defect settleable from code; this round's carried code Critical (R18-1, re-posted with a narrowed demonstrated entrance) is a direction-neutral implementation defect and does not supersede it. Failure scenario: merging before the direction call ships a contested layout and re-locks the opposite contract in the three rewritten test pins; if the maintainer keeps the #7652 collapse layout, the change must be reverted together with its inverted test pins. Witness: gh issue view 9300 (this round): state OPEN, labels include need-discussion, updatedAt 2026-08-17T05:43:02Z unchanged since the gate-open comment; @doudouOUC 2026-08-17T05:43:02Z: 'the remaining blocker is purely the product-direction call, which needs maintainer sign-off — keeping need-discussion in place'; gh pr view 9305 → reviewDecision CHANGES_REQUESTED, 0 human approvals.
中文说明
仅完成部分审查，审查缺口已披露。
本轮确认的 7 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未探索到全部深度（达到工具调用预算）："agent 8a"：none — I completed the diff walk, the source walk of VirtualizedList.tsx / MainContent.tsx , and the host sessionId-transition verification within budget. I di…。
收敛姿态下延后（第 19 轮，非阻断）——已记录，本轮不要求修改：共 1 条（原文未翻译，列表见上方英文部分）。
[Critical] R2-1 Triage stage-1c direction gate: this PR reverses the intentional, test-locked collapse layout from #7652 and remains held pending a maintainer product-direction decision on #9300 — whether bottom-aligning short VP content (blank space above the conversation, latest message directly above the composer) is the intended layout. Re-checked this round against the live issue via a fresh fetch: issue #9300 is OPEN with need-discussion, and the last thread comment (@doudouOUC, 2026-08-17T05:43:02Z) keeps the gate open — no maintainer direction call has been recorded since, and no approval exists on the PR (all PR comments are bot-authored; the sole human review is a token-permission test, not a direction call). The gate's named follow-up is half-met: the design comment above rootHeight IS rewritten by this diff, but the PR description still does not say why #7652's removal of the earlier bottom-align attempt doesn't apply this time. Still stands — this is a product decision, not a code defect settleable from code; this round's carried code Critical (R18-1, re-posted with a narrowed demonstrated entrance) is a direction-neutral implementation defect and does not supersede it. Failure scenario: merging before the direction call ships a contested layout and re-locks the opposite contract in the three rewritten test pins; if the maintainer keeps the #7652 collapse layout, the change must be reverted together with its inverted test pins. Witness: gh issue view 9300 (this round): state OPEN, labels include need-discussion, updatedAt 2026-08-17T05:43:02Z unchanged since the gate-open comment; @doudouOUC 2026-08-17T05:43:02Z: 'the remaining blocker is purely the product-direction call, which needs maintainer sign-off — keeping need-discussion in place'; gh pr view 9305 → reviewDecision CHANGES_REQUESTED, 0 human approvals.
— qwen3.8-max via Qwen Code /review (v0.21.15)
| const wasScrolledToBottomPixels = | ||
| prevScrollTop.current >= | ||
| prevTotalHeight.current - prevContainerHeight.current - 1; | ||
| prevMaxScroll > 1 && prevScrollTop.current >= prevMaxScroll - 1; |
There was a problem hiding this comment.
[Critical] R18-1: The reAnchorClampMark follow-suppression class still has one live demonstrated entrance. This round's structural fix (session-key remount, positional mark) closed the key-instability and dataset-swap sub-families, but the rewritten bottom-pixels guard zeroes the bottom signal for the ENTIRE one-row scroll range — including the true bottom. A scrolled-away list that shrinks in place to a remnant overflowing by exactly one row parks at scrollTop 1 === maxScroll and installs the mark with allowFollow: true; on the next growth all four auto-scroll arms are dead: arm 1 is suppressed by clampParked, arm 2 needs contentPreviouslyFit (false), arm 3 needs wasScrolledToBottomPixels — which this guard admits only at prevMaxScroll === 2, a knife-edge window that any growth of ≥2 lines per render jumps straight over — and arm 4 needs sticking (released). The re-stick gate is blocked by the same signal, prevScrollTop stays 1 while prevMaxScroll grows, and no later render heals it: the entire next reply streams below the fold until a manual scroll to the bottom. This is the per-entrance patch → new entrance dynamic R18-1 named; pre-PR the unguarded tolerance (1 >= 0) re-engaged follow from this position, so the guard narrows a previously working recovery. R11-4 only needs the TOP of a one-row range rejected; there the top (scrollTop 0) and the bottom (scrollTop 1) are distinct positions.
Witness (probe at the unmodified commit; scrolled away, shrunk to an 11-row remnant in a 10-row container, parked at scrollTop 1):
growth#1 (+2 lines): ["item-1".."item-10"]
growth#2 (+2 lines): ["item-1".."item-10"]
growth#3 (+2 lines): ["item-1".."item-10"] ← reply grew 11→17 rows, all below the fold
1-line-per-render growth self-heals on the second chunk (arm 3 fires at prevMaxScroll === 2); the candidate fix below flip-checks — follow re-engages on growth #1 and all 53 VirtualizedList.test.tsx tests pass with it, including the R11-4 test. Add a regression test mirroring the R18-1 F7 shape with a one-row-overflow remnant and a ≥2-line growth step.
| prevMaxScroll > 1 && prevScrollTop.current >= prevMaxScroll - 1; | |
| prevMaxScroll > 1 | |
| ? prevScrollTop.current >= prevMaxScroll - 1 | |
| : prevMaxScroll === 1 && prevScrollTop.current >= prevMaxScroll; |
中文说明
[Critical] R18-1：reAnchorClampMark 跟随抑制类别仍有一个可演示的存活入口。本轮的结构性修复（按会话 key 重挂载、位置化标记）封闭了 key 不稳定与数据集交换两个子族，但重写后的底部像素门控把整个一行滚动范围的底部信号清零——包括真正的底部。一个已上滚的列表原地收缩到恰好溢出一行的残余时，会停驻在 scrollTop 1 === maxScroll 并以 allowFollow: true 安装标记；下一次增长时四个自动滚动分支全部失效：分支 1 被 clampParked 抑制，分支 2 需要 contentPreviouslyFit（为假），分支 3 需要 wasScrolledToBottomPixels——而该门控只在 prevMaxScroll === 2 时放行，任何单次渲染增长 ≥2 行都会直接跳过这个刀刃窗口——分支 4 需要贴底（已解除）。re-stick 门控被同一信号阻断，prevScrollTop 停在 1 而 prevMaxScroll 持续增长，后续任何渲染都无法自愈：整段下一条回复都在折线之下流式输出，直到手动滚动到底部。这正是 R18-1 所指出的"逐入口修补→新入口"动态；PR 之前的无门控容差（1 >= 0）在该位置会重新置位跟随，因此本门控收窄了一条原本可用的恢复路径。R11-4 只需要排除一行范围的顶部；在那里顶部（scrollTop 0）与底部（scrollTop 1）是两个不同的位置。
证据（探针，在未改动提交上运行：上滚后原地收缩到 10 行容器中的 11 行残余，停驻于 scrollTop 1）：三次连续的 +2 行增长中帧冻结在 ["item-1".."item-10"]，回复从 11 行增长到 17 行、全部位于折线之下；每次渲染增长 1 行的形态会在第二块自愈（分支 3 在 prevMaxScroll === 2 时触发）。下方候选修复经翻转验证——增长 #1 即恢复跟随，且全部 53 个 VirtualizedList.test.tsx 测试通过（含 R11-4 测试）。建议补充回归测试：仿照 R18-1 F7 形态，使用一行溢出残余 + 一次 ≥2 行的增长。
— qwen3.8-max via Qwen Code /review (v0.21.15)
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
1 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- D20-2 R5-2 no-op loop release direction untested (probe-verified mutant survives the suite) — already recorded in the round-8 deferral list as D8-2 (review 4981837967) and in the rounds 13-19 already-reported lists
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally (the diff is platform-independent TypeScript; the ubuntu unit-test job passed in CI and this review's full packages/cli suite is green).
Not reviewed: test-efficacy — probe harness could not be validated in this repo (fresh-checkout prerequisite guard; harnessValidated null).
Test Plan (not a blocker): 32 tests pass — this review observed 23155 passed.
Deferred under the convergence posture (round 20, not a blocker) — recorded, not requested in this round:
packages/cli/src/ui/components/shared/VirtualizedList.test.tsx:301 — [review] three new test comments claim ScrollableList carries no key — contradicted by this PR's own session key (also at lines 1034 and 1188)
[Critical] R2-1: Triage stage-1c direction gate: this PR reverses the intentional, test-locked collapse layout from #7652 and remains held pending a maintainer product-direction decision on #9300 — whether bottom-aligning short VP content (blank space above the conversation, latest message directly above the composer) is the intended layout. Re-checked live this round: issue #9300 is OPEN with need-discussion; the last thread comment (@doudouOUC, 2026-08-17T05:43:02Z) keeps the gate open — no maintainer direction call has been recorded since, and the PR has 0 approvals. The gate's named follow-up is half-met: the design comment above rootHeight IS rewritten by this diff, but the PR description contains no mention of #7652 and does not say why its removal of the earlier bottom-align attempt doesn't apply this time. Still stands — this is a product decision, not a code defect settleable from code. Failure scenario: merging before the direction call ships a contested layout and re-locks the opposite contract in the three rewritten test pins; if the maintainer keeps the #7652 collapse layout, the change must be reverted together with its inverted test pins. Witness: gh live fetch this round — issue #9300 state OPEN, labels include need-discussion, updatedAt 2026-08-17T05:43:02Z unchanged since the gate-open comment; PR #9305 reviews: 0 APPROVED; PR body grep '7652': no matches.
中文说明
仅完成部分审查，审查缺口已披露。
本轮确认的 1 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally (the diff is platform-independent TypeScript; the ubuntu unit-test job passed in CI and this review's full packages/cli suite is green)。
未审查：test-efficacy — probe harness could not be validated in this repo (fresh-checkout prerequisite guard; harnessValidated null)。
Test Plan（非阻断）：32 tests pass — this review observed 23155 passed。
收敛姿态下延后（第 20 轮，非阻断）——已记录，本轮不要求修改：共 1 条（原文未翻译，列表见上方英文部分）。
[Critical] R2-1: Triage stage-1c direction gate: this PR reverses the intentional, test-locked collapse layout from #7652 and remains held pending a maintainer product-direction decision on #9300 — whether bottom-aligning short VP content (blank space above the conversation, latest message directly above the composer) is the intended layout. Re-checked live this round: issue #9300 is OPEN with need-discussion; the last thread comment (@doudouOUC, 2026-08-17T05:43:02Z) keeps the gate open — no maintainer direction call has been recorded since, and the PR has 0 approvals. The gate's named follow-up is half-met: the design comment above rootHeight IS rewritten by this diff, but the PR description contains no mention of #7652 and does not say why its removal of the earlier bottom-align attempt doesn't apply this time. Still stands — this is a product decision, not a code defect settleable from code. Failure scenario: merging before the direction call ships a contested layout and re-locks the opposite contract in the three rewritten test pins; if the maintainer keeps the #7652 collapse layout, the change must be reverted together with its inverted test pins. Witness: gh live fetch this round — issue #9300 state OPEN, labels include need-discussion, updatedAt 2026-08-17T05:43:02Z unchanged since the gate-open comment; PR #9305 reviews: 0 APPROVED; PR body grep '7652': no matches.
— qwen3.8-max via Qwen Code /review (v0.22.0)
| const wasScrolledToBottomPixels = | ||
| prevScrollTop.current >= | ||
| prevTotalHeight.current - prevContainerHeight.current - 1; | ||
| prevMaxScroll > 1 && prevScrollTop.current >= prevMaxScroll - 1; |
There was a problem hiding this comment.
[Critical] R18-1: The one-row scroll-range guard still zeroes the true-bottom signal — this carried blocker still stands at the current head. wasScrolledToBottomPixels requires prevMaxScroll > 1, so when the previous render overflowed by exactly one row no position reads as "at the bottom" — not even scrollTop 1 === maxScroll, which IS the true bottom of that range. A scrolled-away list that shrinks in place to a multi-item remnant overflowing by exactly one row parks there with the clamp mark installed (allowFollow: true), and the next growth finds all four auto-follow arms dead: the wasAtBottom arm is suppressed by clampParked, the fit→overflow arm needs contentPreviouslyFit (false — the remnant overflowed), the allowFollow arm — the very arm this PR added for R18-1 F7 — needs the guard-killed wasScrolledToBottomPixels signal, and the sticking arm is released. The re-stick gate is blocked by the same signal, and prevScrollTop stays 1 while prevMaxScroll grows, so the window never reopens on its own: the entire next reply streams below the fold until a manual scroll. Pre-PR the unguarded tolerance re-engaged follow from this position, so the guard narrows a previously working recovery; R11-4 only needs the TOP of a one-row range (scrollTop 0) rejected — a distinct position.
Witness (probe at the reviewed commit, flip-checked): PR — grown frame ["s1".."s9","q0"], scrollTop stuck at 1 while scrollHeight reaches 13, the two new rows render below the fold and the park never moves; PR + the fix below — grown frame ["s3".."s9","q0","q1","q2"], follow re-engaged; with the fix all 53 VirtualizedList.test.tsx tests pass, including the R11-4 parked-TOP rejection.
| prevMaxScroll > 1 && prevScrollTop.current >= prevMaxScroll - 1; | |
| prevMaxScroll > 1 | |
| ? prevScrollTop.current >= prevMaxScroll - 1 | |
| : prevMaxScroll === 1 && prevScrollTop.current >= prevMaxScroll; |
Plus a regression test mirroring the R18-1 F7 shape with a one-row-overflow remnant and a ≥2-line growth step.
中文说明
[Critical] R18-1：一行滚动范围门控仍然把真底部信号清零 —— 该携带阻断在当前 head 上依然成立。wasScrolledToBottomPixels 要求 prevMaxScroll > 1，因此当上一次渲染恰好溢出一行时，任何位置都不会被读作"在底部"——连 scrollTop 1 === maxScroll（该范围的真底部）也不例外。一个已上滚的列表原地收缩到恰好溢出一行的多条目残余时，会停驻在该处并安装钳制标记（allowFollow: true）；下一次增长时四条自动跟随分支全部失效：wasAtBottom 分支被 clampParked 抑制，fit→overflow 分支需要 contentPreviouslyFit（为假——残余溢出了），allowFollow 分支——即本 PR 为 R18-1 F7 新增的那条——需要被门控清零的 wasScrolledToBottomPixels 信号，贴底分支已解除。re-stick 门控被同一信号阻断，且 prevScrollTop 停在 1 而 prevMaxScroll 持续增长，窗口不会自行重新打开：整段下一条回复都在折线之下流式输出，直到手动滚动。PR 之前无门控的容差会在该位置重新置位跟随，因此该门控收窄了一条原本可用的恢复路径；R11-4 只需要排除一行范围的顶部（scrollTop 0）——那是另一个不同的位置。
证据（探针，在受审提交上运行，可翻转验证）：PR —— 增长后帧 ["s1".."s9","q0"]，scrollHeight 达到 13 时 scrollTop 停在 1，两个新行渲染在折线之下、停驻永不移动；PR + 下方修复 —— 增长后帧 ["s3".."s9","q0","q1","q2"]，跟随恢复；该修复下全部 53 个 VirtualizedList.test.tsx 测试通过，包括 R11-4 的停驻顶部排除测试。
建议修复：采用上方 suggestion 代码块的等价改动，并补充回归测试：仿照 R18-1 F7 形态，使用一行溢出残余 + 一次 ≥2 行的增长。
— qwen3.8-max via Qwen Code /review (v0.22.0)
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
4 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- R21-2 the fitting-content no-op rule triplicated across scrollBy/scrollTo/scrollToEnd, each recomputing maxScroll (VirtualizedList.tsx:914) — already recorded in the round-8 deferral list as D8-1 (review 4981837967) and the rounds 13-20 alr…
- R21-3 three new test comments claim ScrollableList carries no key, contradicted by this PR's own session key (VirtualizedList.test.tsx:301, ~1033, ~1187) — already recorded in the round-19/20 deferral lists as D19-1/D20-1 (reviews 500016436…
- R21-4 scrollToIndex/scrollToItem lack the fitting-content no-op guard (VirtualizedList.tsx:928) — already recorded in the round-6 deferral list (review 4979659125) and round-8 deferral D8-1 (review 4981837967)
- R21-5 the no-op scroll loop pins only one sticking direction; the opposite mutant survives the suite (VirtualizedList.test.tsx:284) — already recorded in the round-8 deferral list as D8-2 (review 4981837967) and the round-20 already-reporte…
Not reviewed: build-and-test — test-efficacy probe could not be validated in this repo (fresh-checkout prerequisite guard; harnessValidated null, 0 mutants and 0 hunk probes executed).
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally (the macOS/Windows unit-test matrix jobs were also skipped; the diff is platform-independent TypeScript, and the ubuntu unit-test job plus this review's full packages/cli suite passed).
[Critical] R2-1: Triage stage-1c direction gate: this PR reverses the intentional, test-locked collapse layout from #7652 and remains held pending a maintainer product-direction decision on #9300 — whether bottom-aligning short VP content (blank space above the conversation, latest message directly above the composer) is the intended layout. Re-checked live this round via a fresh issue-context fetch (2026-08-23): issue #9300 is OPEN with need-discussion; the last thread comment (@doudouOUC, 2026-08-17T05:43:02Z) keeps the gate open — no maintainer direction call has been recorded since, and the PR has 0 approvals (the sole human review is a token-permission test, not a direction call). The gate's named follow-up is half-met: the design comment above rootHeight IS rewritten by this diff, but the PR description contains no mention of #7652 and does not say why its removal of the earlier bottom-align attempt doesn't apply this time. Still stands — this is a product decision, not a code defect settleable from code. Failure scenario: merging before the direction call ships a contested layout and re-locks the opposite contract in the three rewritten test pins; if the maintainer keeps the #7652 collapse layout, the change must be reverted together with its inverted test pins. Witness: fresh issue fetch this round — issue #9300 state OPEN, labels include need-discussion, updatedAt 2026-08-17T05:43:02Z unchanged since the gate-open comment; PR #9305 reviews: 0 APPROVED; PR body grep '7652': no matches. On the code side this round: the carried implementation Critical R18-1 (re-posted inline) is direction-neutral, and all twelve previously-fixed implementation Criticals (R4-1, R5-1/2/3, R6-1/2, R8-1, R10-1, R11-1/4/6, R14-1, R17-1/2/3) remain verifiably closed — the code is byte-identical to the round-20 head, each is pinned by a regression test in this diff, and the full packages/cli suite is green apart from failures measured to fail identically at the merge base.
中文说明
仅完成部分审查，审查缺口已披露。
本轮确认的 4 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
未审查：build-and-test — test-efficacy probe could not be validated in this repo (fresh-checkout prerequisite guard; harnessValidated null, 0 mutants and 0 hunk probes executed)。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally (the macOS/Windows unit-test matrix jobs were also skipped; the diff is platform-independent TypeScript, and the ubuntu unit-test job plus this review's full packages/cli suite passed)。
[Critical] R2-1: Triage stage-1c direction gate: this PR reverses the intentional, test-locked collapse layout from #7652 and remains held pending a maintainer product-direction decision on #9300 — whether bottom-aligning short VP content (blank space above the conversation, latest message directly above the composer) is the intended layout. Re-checked live this round via a fresh issue-context fetch (2026-08-23): issue #9300 is OPEN with need-discussion; the last thread comment (@doudouOUC, 2026-08-17T05:43:02Z) keeps the gate open — no maintainer direction call has been recorded since, and the PR has 0 approvals (the sole human review is a token-permission test, not a direction call). The gate's named follow-up is half-met: the design comment above rootHeight IS rewritten by this diff, but the PR description contains no mention of #7652 and does not say why its removal of the earlier bottom-align attempt doesn't apply this time. Still stands — this is a product decision, not a code defect settleable from code. Failure scenario: merging before the direction call ships a contested layout and re-locks the opposite contract in the three rewritten test pins; if the maintainer keeps the #7652 collapse layout, the change must be reverted together with its inverted test pins. Witness: fresh issue fetch this round — issue #9300 state OPEN, labels include need-discussion, updatedAt 2026-08-17T05:43:02Z unchanged since the gate-open comment; PR #9305 reviews: 0 APPROVED; PR body grep '7652': no matches. On the code side this round: the carried implementation Critical R18-1 (re-posted inline) is direction-neutral, and all twelve previously-fixed implementation Criticals (R4-1, R5-1/2/3, R6-1/2, R8-1, R10-1, R11-1/4/6, R14-1, R17-1/2/3) remain verifiably closed — the code is byte-identical to the round-20 head, each is pinned by a regression test in this diff, and the full packages/cli suite is green apart from failures measured to fail identically at the merge base.
— qwen3.8-max via Qwen Code /review (v0.22.0)
| const wasScrolledToBottomPixels = | ||
| prevScrollTop.current >= | ||
| prevTotalHeight.current - prevContainerHeight.current - 1; | ||
| prevMaxScroll > 1 && prevScrollTop.current >= prevMaxScroll - 1; |
There was a problem hiding this comment.
[Critical] R18-1: The one-row scroll-range guard still zeroes the true-bottom signal — this carried blocker still stands at the current head. wasScrolledToBottomPixels requires prevMaxScroll > 1, so when the previous render overflowed by exactly one row no position reads as "at the bottom" — not even scrollTop 1 === maxScroll, which IS the true bottom of that range. A scrolled-away list that shrinks in place to a multi-item remnant overflowing by exactly one row parks at the remnant's live bottom with the clamp mark installed (allowFollow: true), and the next growth finds all four auto-follow arms dead: the wasAtBottom arm is suppressed by clampParked, the fit→overflow arm needs contentPreviouslyFit (false — the remnant overflowed), the allowFollow arm — the very arm this PR added for the R18-1 live-bottom park — needs the guard-killed wasScrolledToBottomPixels signal, and the sticking arm is released. The re-stick gate is blocked by the same signal, and prevScrollTop stays 1 while prevMaxScroll grows, so the window never reopens on its own: the entire next reply streams below the fold until a manual scroll. The R11-4 defense only needs the TOP of a one-row range (scrollTop 0) rejected — a distinct position, so the blanket guard cuts more than it must.
Witness (probe at the reviewed commit cf04b3f, unmodified PR code, flip-checked): after the shrink to an 11-item remnant in a 10-row container — parked frame item-1..item-10; after the first append — STILL item-1..item-10 (item-11 renders below the fold); after the second append — yanked to item-3..item-12, follow latched one growth late. With the fix below the probe flips to item-2..item-11 on the first append (follow re-engaged), and all 53 VirtualizedList.test.tsx tests stay green — including the R11-4 parked-TOP rejection test.
| const wasScrolledToBottomPixels = | |
| prevScrollTop.current >= | |
| prevTotalHeight.current - prevContainerHeight.current - 1; | |
| prevMaxScroll > 1 && prevScrollTop.current >= prevMaxScroll - 1; | |
| const wasScrolledToBottomPixels = | |
| prevMaxScroll > 1 | |
| ? prevScrollTop.current >= prevMaxScroll - 1 | |
| : prevMaxScroll === 1 && prevScrollTop.current >= prevMaxScroll; |
Plus a regression test mirroring the R18-1 F7 shape with a one-row-overflow remnant and a ≥2-line growth step.
中文说明
[Critical] R18-1：一行滚动范围门控仍然把真底部信号清零 —— 该携带阻断在当前 head 上依然成立。wasScrolledToBottomPixels 要求 prevMaxScroll > 1，因此当上一次渲染恰好溢出一行时，任何位置都不会被读作"在底部"——连 scrollTop 1 === maxScroll（该范围的真底部）也不例外。一个已上滚的列表原地收缩到恰好溢出一行的多条目残余时，会停驻在残余的活底部并安装钳制标记（allowFollow: true）；下一次增长时四条自动跟随分支全部失效：wasAtBottom 分支被 clampParked 抑制，fit→overflow 分支需要 contentPreviouslyFit（为假——残余溢出了），allowFollow 分支——即本 PR 为 R18-1 活底部停驻新增的那条——需要被门控清零的 wasScrolledToBottomPixels 信号，贴底分支已解除。re-stick 门控被同一信号阻断，且 prevScrollTop 停在 1 而 prevMaxScroll 持续增长，窗口不会自行重新打开：整段下一条回复都在折线之下流式输出，直到手动滚动。R11-4 的防御只需要排除一行范围的顶部（scrollTop 0）——那是一个不同的位置，因此当前的一刀切门控切除得过多。
证据（探针，在受审提交 cf04b3f 的未修改 PR 代码上运行，可翻转验证）：10 行容器收缩到 11 条残余之后——停驻帧 item-1..item-10；第一次追加之后——仍为 item-1..item-10（item-11 渲染在折线之下）；第二次追加之后——被拽到 item-3..item-12，跟随晚一步才锁住。采用下方修复后，探针在第一次追加时翻转为 item-2..item-11（跟随恢复），且全部 53 个 VirtualizedList.test.tsx 测试保持绿色——包括 R11-4 的停驻顶部排除测试。
建议修复：采用上方英文部分 suggestion 代码块的等价改动，并补充回归测试：仿照 R18-1 F7 形态，使用一行溢出残余 + 一次 ≥2 行的增长。
— qwen3.8-max via Qwen Code /review (v0.22.0)
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
5 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- three new test comments assert ScrollableList carries no key / /clear does not remount (VirtualizedList.test.tsx:301, 1033, 1187) — already recorded in the round-19/20 deferral lists as D19-1/D20-1 (reviews 5000164364, 5000669914) and the r…
- the fitting-content no-op guard missing from scrollToIndex/scrollToItem while triplicated across scrollBy/scrollTo/scrollToEnd (VirtualizedList.tsx) — already recorded in the round-6 deferral list (review 4979659125), round-8 deferral D8-1 …
- mutation survivor: the released-arm data.length > 1 mark-install term is pinned by no test — a still-fitting growth step discriminates (VirtualizedList.tsx / VirtualizedList.test.tsx:583) — already recorded in the round-13 deferral list (re…
- zero debugLogger coverage of sticking transitions in the clamp-mark state machine (VirtualizedList.tsx) — already recorded in the round-8 deferral list as D8-3 (review 4981837967) and the rounds 13-21 already-reported lists
- the host's banner-only predicate re-derived inside the shared component as data.length <= 1 (VirtualizedList.tsx) — already recorded in the round-6 deferral list (review 4979659125) and the rounds 13-21 already-reported lists
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Not reviewed: test-efficacy — probe harness could not be validated in this repo (fresh-checkout prerequisite guard; harnessValidated null, 0 mutants and 0 hunk probes executed by the harness).
Deferred under the convergence posture (round 22, not a blocker) — recorded, not requested in this round:
packages/cli/src/ui/components/shared/VirtualizedList.test.tsx:1186 — [review] mutation survivor: the swap-park test passes with !swapEntry dropped from the overflow-park install condition — a shrink-to-fit after the swap park discriminatespackages/cli/src/ui/components/shared/VirtualizedList.tsx:418 — [review] the remount-invariant comment is false for same-id /resume, /restore, rewind, and /history expand-now — those swap the dataset in place without a remountpackages/cli/src/ui/components/MainContent.tsx:506 — [probe] the session-id remount key fires for /branch — the fork keeps the conversation content, but the remount yanks a scrolled-up user to the bottom and latches auto-follow
Convergence: round 22 posted 2 inline comment(s), 1 of them reported for the first time; the previous round posted 1 (0 new). Findings keep coming back to the same files: packages/cli/src/ui/components/shared/VirtualizedList.tsx (findings in round 18; 1 more now). A cluster that keeps producing siblings usually means the fixes are treating instances of a shared root cause — triaging that cause before the next round, or splitting an independent cluster into its own pull request, tends to end the loop faster than fixing them one at a time. (Observation only — nothing was withheld from this review because of this observation.)
[Critical] R2-1: Triage stage-1c direction gate: this PR reverses the intentional, test-locked collapse layout from #7652 and remains held pending a maintainer product-direction decision on #9300 — whether bottom-aligning short VP content (blank space above the conversation, latest message directly above the composer) is the intended layout. Re-checked live this round (2026-08-24): issue #9300 is OPEN with need-discussion; the last thread comment (@doudouOUC, 2026-08-17T05:43:02Z) keeps the gate open — no maintainer direction call has been recorded since, and the PR has 0 approvals (the sole human review is a token-permission test, not a direction call). The gate's named follow-up is half-met: the design comment above rootHeight IS rewritten by this diff, but the PR description contains no mention of #7652 and does not say why its removal of the earlier bottom-align attempt doesn't apply this time. Still stands — this is a product decision, not a code defect settleable from code. Failure scenario: merging before the direction call ships a contested layout and re-locks the opposite contract in the three rewritten test pins; if the maintainer keeps the #7652 collapse layout, the change must be reverted together with its inverted test pins. Witness: gh live fetch this round — issue #9300 state OPEN, labels include need-discussion, updatedAt 2026-08-17T05:43:02Z unchanged since the gate-open comment; PR #9305 reviewDecision CHANGES_REQUESTED, 0 approvals; PR body grep '7652': no matches. On the code side this round: the carried implementation Critical R18-1 still stands (re-posted inline, probe-verified); one NEW implementation Critical (R22-1) was found and probe-verified; and all twelve previously-fixed implementation Criticals (R4-1, R5-1/2/3, R6-1/2, R8-1, R10-1, R11-1/4/6, R14-1, R17-1/2/3) remain verifiably closed — each is pinned by a regression test in this diff, and the full packages/cli suite is green apart from failures measured to fail identically at the merge base.
中文说明
仅完成部分审查，审查缺口已披露。
本轮确认的 5 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未审查：test-efficacy — probe harness could not be validated in this repo (fresh-checkout prerequisite guard; harnessValidated null, 0 mutants and 0 hunk probes executed by the harness)。
收敛姿态下延后（第 22 轮，非阻断）——已记录，本轮不要求修改：共 3 条（原文未翻译，列表见上方英文部分）。
收敛情况：第 22 轮发布了 2 条行内评论，其中 1 条是首次提出；上一轮发布了 1 条（其中 0 条首次提出）。发现反复回到同一批文件：packages/cli/src/ui/components/shared/VirtualizedList.tsx（第 18 轮已出过发现，本轮又有 1 条）。一个不断再生兄弟发现的簇，通常意味着逐条修复只在处理同一根因的实例——先定位并处理该根因，或把独立的簇拆成单独的 PR，通常比逐条修复更快结束循环。（仅为观察——本轮评审未因此扣留任何内容。）
[Critical] R2-1: Triage stage-1c direction gate: this PR reverses the intentional, test-locked collapse layout from #7652 and remains held pending a maintainer product-direction decision on #9300 — whether bottom-aligning short VP content (blank space above the conversation, latest message directly above the composer) is the intended layout. Re-checked live this round (2026-08-24): issue #9300 is OPEN with need-discussion; the last thread comment (@doudouOUC, 2026-08-17T05:43:02Z) keeps the gate open — no maintainer direction call has been recorded since, and the PR has 0 approvals (the sole human review is a token-permission test, not a direction call). The gate's named follow-up is half-met: the design comment above rootHeight IS rewritten by this diff, but the PR description contains no mention of #7652 and does not say why its removal of the earlier bottom-align attempt doesn't apply this time. Still stands — this is a product decision, not a code defect settleable from code. Failure scenario: merging before the direction call ships a contested layout and re-locks the opposite contract in the three rewritten test pins; if the maintainer keeps the #7652 collapse layout, the change must be reverted together with its inverted test pins. Witness: gh live fetch this round — issue #9300 state OPEN, labels include need-discussion, updatedAt 2026-08-17T05:43:02Z unchanged since the gate-open comment; PR #9305 reviewDecision CHANGES_REQUESTED, 0 approvals; PR body grep '7652': no matches. On the code side this round: the carried implementation Critical R18-1 still stands (re-posted inline, probe-verified); one NEW implementation Critical (R22-1) was found and probe-verified; and all twelve previously-fixed implementation Criticals (R4-1, R5-1/2/3, R6-1/2, R8-1, R10-1, R11-1/4/6, R14-1, R17-1/2/3) remain verifiably closed — each is pinned by a regression test in this diff, and the full packages/cli suite is green apart from failures measured to fail identically at the merge base.
— qwen3.8-max via Qwen Code /review (v0.22.0)
| const wasScrolledToBottomPixels = | ||
| prevScrollTop.current >= | ||
| prevTotalHeight.current - prevContainerHeight.current - 1; | ||
| prevMaxScroll > 1 && prevScrollTop.current >= prevMaxScroll - 1; |
There was a problem hiding this comment.
[Critical] R18-1: The one-row scroll-range guard still zeroes the true-bottom signal — this carried blocker still stands at the current head, re-verified by an executed flip-checked probe at this commit. wasScrolledToBottomPixels requires prevMaxScroll > 1, so when the previous render overflowed by exactly one row no position reads as "at the bottom" — not even scrollTop 1 === maxScroll, which IS the true bottom of that range. A scrolled-away list that shrinks in place to a remnant overflowing by exactly one row parks at the live bottom (the drop branch installs the mark with allowFollow: true), and the next multi-row growth then fires no arm: arm 1 is blocked by clampParked, arm 2 by contentPreviouslyFit === false, arm 3 (the dedicated allowFollow re-follow) by wasScrolledToBottomPixels, arm 4 by sticking off — the new content renders below the fold and follow stays dead. One-row-at-a-time growth self-heals on the second tick, but a normal multi-row streamed chunk leaves follow dead until a manual scroll. The R11-4 hazard this guard defends against is only the TOP park of a one-row range (scrollTop 0); the bottom of that range is scrollTop 1, which the guard excludes too. No test covers this boundary (the R18-1 F7 test parks at maxScroll 4; the R11-4 test covers only the top park).
Witness (probe at the reviewed commit, flip-checked):
PR: PARK FRAME ["x1".."x10"]; GROWTH FRAME ["x1".."x10"] — FOLLOW RE-ENGAGED: false (g0-g2 below the fold)
FIXED: GROWTH FRAME ["x4".."x10","g0","g1","g2"] — FOLLOW RE-ENGAGED: true; VirtualizedList.test.tsx 53/53 green with the fix
Suggested fix: guard on position, not range size — prevScrollTop.current > 0 && prevScrollTop.current >= prevMaxScroll - 1 (equivalent for maxScroll >= 2; keeps the R11-4 top-park fix since 0 > 0 is false). Add a regression test: scrolled-away → shrink to a remnant overflowing by exactly 1 row → grow by >= 2 rows → follow must re-engage.
中文说明
R18-1：单行滚动范围门控仍然把真正的底部信号清零 —— 该遗留阻断在当前 head 上依然成立（已在受审提交上用可翻转探针实测复核）。wasScrolledToBottomPixels 要求 prevMaxScroll > 1，因此当上一次渲染恰好溢出一行时，任何位置都不会被读作"在底部" —— 连 scrollTop 1 === maxScroll（这正是该范围真正的底部）也不例外。一个已上滚的列表就地坍缩到恰好溢出一行的残余时，会停驻在活动底部（drop 分支安装 allowFollow: true 的标记），随后一次多行增长不会触发任何分支：分支 1 被 clampParked 阻断、分支 2 因 contentPreviouslyFit === false 不成立、分支 3（专门的 allowFollow 重新跟随）因 wasScrolledToBottomPixels 为假、分支 4 因贴底已解除 —— 新内容渲染在折叠线以下，跟随失效。逐行增长会在第二跳自愈，但一次正常的多行流式块会让跟随一直失效到手动滚动。此门控要防的 R11-4 风险只是单行范围的顶部停驻（scrollTop 0）；该范围的底部是 scrollTop 1，同样被门控排除。没有测试覆盖该边界（R18-1 F7 测试停驻在 maxScroll 4；R11-4 测试只覆盖顶部停驻）。
证据（探针，受审提交上运行，可翻转验证）：修复前增长帧不跟随（新内容在折叠线下）；采用建议修复后增长帧跟随恢复，且 VirtualizedList.test.tsx 53/53 全绿。
建议修复：按位置而非范围大小设门控 —— prevScrollTop.current > 0 && prevScrollTop.current >= prevMaxScroll - 1（maxScroll >= 2 时等价；因 0 > 0 为假，仍保留 R11-4 的顶部停防护）。补充回归测试：上滚 → 坍缩到恰好溢出 1 行的残余 → 增长 >= 2 行 → 跟随必须恢复。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| (clampMark !== null && | ||
| clampParked && | ||
| clampMark.allowFollow && | ||
| wasScrolledToBottomPixels && | ||
| totalHeight > prevTotalHeight.current) || |
There was a problem hiding this comment.
[Critical] R22-1: The allowFollow live-bottom-park re-follow arm re-derives "at the bottom" from stale prev-frame pixel math, so a container shrink of >= 2 rows between the park and the next growth permanently kills follow re-engagement while clampParked suppresses every other arm. Reproduced by an executed probe at this commit: user scrolls away (sticking released); an in-session truncation (/rewind) shrinks the list in place to a multi-item remnant that still overflows — the drop branch parks at the live bottom and installs the mark with allowFollow: true (the exact state pinned by the R18-1 F7 test). If the terminal is then resized >= 2 rows smaller with content still overflowing, on the resize render this arm fails on totalHeight > prevTotalHeight and nothing else fires; the prev refs then update so the park position — through no user action — is >= 2 rows from the bottom. When content grows, arm 1 is suppressed by clampParked, arm 2 needs contentPreviouslyFit, arm 3 needs the now-unattainable bottom-pixels predicate, the re-stick gate is suppressed by clampParked, and the drop branch never fires (maxScroll only grows). The anchor never moves, so the mark never clears: every subsequent message renders below the fold and auto-follow never comes back until a manual scroll — the exact "the mark would otherwise suppress follow forever" failure this arm's comment says it removes. The growth direction is NOT affected (the drop branch re-parks and recovers). The window is unbounded: /rewind keeps the same session (no session-key remount intervenes) and any >= 2-row terminal shrink before the next content sets the trap.
Witness (probe at the reviewed commit, flip-checked):
control (no resize): follow re-engages (baseline F7 shape reproduced)
SHRINK 10→8, then growth: frame frozen — growth below the fold, follow NOT re-engaged
2nd growth after the shrink: still frozen — the kill is permanent until a manual scroll
GROW 10→12 (control direction): follow re-engages (drop re-park recovers)
Suggested fix: while clampParked holds, the position is provably the content-driven park (any user scroll moves the anchor and clears the mark) — re-derive the bottom signal from the mark instead of stale pixels, or refresh/re-park the mark to the new bottom on a container resize while clampParked holds. Note: the literal one-line fix of deleting wasScrolledToBottomPixels && from this arm regresses three pinned behaviors — measured in the scratch tree: the suite goes 3 failed | 50 passed (R5-3/R6-2 two-step shrink, R17-1 banner-only overflow, R14-1 fitting-park growth) — because the same predicate also separates overflow-remnant parks (must re-follow) from fitting-remnant parks (must not). The fix must keep that distinction (e.g. an overflow guard on the re-park). Add a regression test: live-bottom park → resize >= 2 rows → growth must follow.
中文说明
R22-1：allowFollow 活动底部停驻的重新跟随分支用上一帧的过期像素计算重新推导"是否在底部"，因此从停驻到下一次增长之间只要发生 >= 2 行的容器收缩，重新跟随就被永久杀死 —— 期间 clampParked 抑制了所有其他分支。已在受审提交上用探针实测复现：用户上滚（贴底解除）；会话内截断（/rewind）把列表就地收缩为仍然溢出的多项残余 —— drop 分支把视口停驻在活动底部并安装 allowFollow: true 的标记（正是 R18-1 F7 测试锁定的状态）。随后在内容仍溢出时终端收缩 >= 2 行：该次渲染上本分支因 totalHeight > prevTotalHeight 不成立而不触发，其他分支也不触发；prev 引用随即更新，停驻位置在无用户操作的情况下距底部 >= 2 行。内容增长时：分支 1 被 clampParked 抑制、分支 2 需要 contentPreviouslyFit、分支 3 需要已不可能成立的底部像素谓词、re-stick 门控被 clampParked 抑制、drop 分支永不触发（maxScroll 只增）。锚点永不移动，标记永不清除：此后每条消息都渲染在折叠线以下，自动跟随直到手动滚动才恢复 —— 正是本分支注释声称要消除的"标记会永久压制跟随"失效。增长方向不受影响（drop 分支会重新停驻并恢复）。触发窗口无界：/rewind 保持同一会话（不会有会话键重挂载介入），下一条内容到来之前任何 >= 2 行的终端收缩都会布下陷阱。
证据（探针，受审提交上运行，可翻转验证）：无缩放对照下跟随恢复（复现 F7 基线）；收缩 10→8 后增长 —— 帧冻结、新内容在折叠线下、跟随未恢复；第二次增长仍然冻结（永久性失效，直到手动滚动）；增长方向对照 10→12 跟随恢复。
建议修复：clampParked 成立时，该位置可证明是内容驱动的停驻（任何用户滚动都会移动锚点并清除标记）—— 用标记而非过期像素重新推导底部信号，或在 clampParked 期间容器缩放时把标记刷新/重新停驻到新底部。注意：直接把 wasScrolledToBottomPixels && 从本分支删掉的单行修复会回归三个已锁定行为 —— 在 scratch 树中实测为 3 failed | 50 passed（R5-3/R6-2 两步收缩、R17-1 仅 banner 溢出、R14-1 适配停驻增长）—— 因为同一谓词还负责区分溢出残余停驻（必须重新跟随）与适配残余停驻（不可跟随）。修复必须保留该区分（例如给重新停驻加溢出守卫）。补充回归测试：活动底部停驻 → 缩放 >= 2 行 → 增长必须跟随。
— qwen3.8-max via Qwen Code /review (v0.22.0)
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
2 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- three stale test comments asserting ScrollableList carries no key / /clear does not remount (VirtualizedList.test.tsx:301, 1033, 1188) — already recorded in the round-19/20 deferral lists as D19-1/D20-1 (reviews 5000164364, 5000669914) and …
- same-id /resume, /restore, rewind and /history expand-now swap the dataset in place without the remount the remount-invariant comments claim (MainContent.tsx:506, VirtualizedList.tsx:418) — already recorded in the round-22 deferral list (re…
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally (the macOS/Windows unit-test matrix jobs were also skipped; the diff is platform-independent TypeScript, and the ubuntu unit-test job ran all 1711 tests green — the job's red conclusion is a vitest worker RPC teardown timeout, not a test failure).
Not reviewed: test-efficacy — probe harness could not be validated in this repo (fresh-checkout prerequisite guard; harnessValidated null, 0 mutants and 0 hunk probes executed by the harness); the verifier ran manual flip-checked probes instead.
Mechanism health: this round did not close cleanly, so it withholds the incremental anchor — and the round it recovered had none either, so the next review re-reads the whole diff and will keep doing so until a round's marker carries an anchor again. (Stated, not acted on — this changes nothing about what the round posts.)
[Critical] R2-1 Triage stage-1c direction gate: this PR reverses the intentional, test-locked collapse layout from #7652 and remains held pending a maintainer product-direction decision on #9300 — whether bottom-aligning short VP content (blank space above the conversation, latest message directly above the composer) is the intended layout. Re-checked live this round: issue #9300 is OPEN with need-discussion, the last thread comment (@doudouOUC, 2026-08-17T05:43:02Z) keeps the gate open — no maintainer direction call has been recorded since, and the PR has 0 approvals (the sole human review is a token-permission test, not a direction call). The gate's named follow-up remains half-met: the design comment above rootHeight IS rewritten by this diff, but the PR description contains no mention of #7652 and does not say why #7652's removal of the earlier bottom-align attempt doesn't apply this time. Still stands — this is a product decision, not a code defect settleable from code. Failure scenario: merging before the direction call ships a contested default-on layout and re-locks the opposite contract in the three rewritten test pins; if the maintainer keeps the #7652 collapse layout, the change must be reverted together with its inverted test pins, and #9300 will have been auto-closed while its own thread says the direction is undecided. Witness: gh live fetch this round — issue #9300 state open, labels include need-discussion, updatedAt 2026-08-17T05:43:02Z unchanged since the gate-open comment; @doudouOUC 2026-08-17T05:43:02Z: "the remaining blocker is purely the product-direction call, which needs maintainer sign-off — keeping need-discussion in place"; PR #9305 reviewDecision CHANGES_REQUESTED, 0 approvals; grep '7652' over the PR body: 0 matches.
中文说明
仅完成部分审查，审查缺口已披露。
本轮确认的 2 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally (the macOS/Windows unit-test matrix jobs were also skipped; the diff is platform-independent TypeScript, and the ubuntu unit-test job ran all 1711 tests green — the job's red conclusion is a vitest worker RPC teardown timeout, not a test failure)。
未审查：test-efficacy — probe harness could not be validated in this repo (fresh-checkout prerequisite guard; harnessValidated null, 0 mutants and 0 hunk probes executed by the harness); the verifier ran manual flip-checked probes instead。
机制健康：本轮未能干净收尾，因而扣留了增量锚点，而它恢复到的那一轮也没有锚点，因此下一次评审将重读整个 diff——并会一直如此，直到某一轮的标记重新带上锚点。（仅陈述，不据此行动——这不改变本轮发布的任何内容。）
[Critical] R2-1 Triage stage-1c direction gate: this PR reverses the intentional, test-locked collapse layout from #7652 and remains held pending a maintainer product-direction decision on #9300 — whether bottom-aligning short VP content (blank space above the conversation, latest message directly above the composer) is the intended layout. Re-checked live this round: issue #9300 is OPEN with need-discussion, the last thread comment (@doudouOUC, 2026-08-17T05:43:02Z) keeps the gate open — no maintainer direction call has been recorded since, and the PR has 0 approvals (the sole human review is a token-permission test, not a direction call). The gate's named follow-up remains half-met: the design comment above rootHeight IS rewritten by this diff, but the PR description contains no mention of #7652 and does not say why #7652's removal of the earlier bottom-align attempt doesn't apply this time. Still stands — this is a product decision, not a code defect settleable from code. Failure scenario: merging before the direction call ships a contested default-on layout and re-locks the opposite contract in the three rewritten test pins; if the maintainer keeps the #7652 collapse layout, the change must be reverted together with its inverted test pins, and #9300 will have been auto-closed while its own thread says the direction is undecided. Witness: gh live fetch this round — issue #9300 state open, labels include need-discussion, updatedAt 2026-08-17T05:43:02Z unchanged since the gate-open comment; @doudouOUC 2026-08-17T05:43:02Z: "the remaining blocker is purely the product-direction call, which needs maintainer sign-off — keeping need-discussion in place"; PR #9305 reviewDecision CHANGES_REQUESTED, 0 approvals; grep '7652' over the PR body: 0 matches.
— qwen3.8-max via Qwen Code /review (v0.22.0)
| const wasScrolledToBottomPixels = | ||
| prevScrollTop.current >= | ||
| prevTotalHeight.current - prevContainerHeight.current - 1; | ||
| prevMaxScroll > 1 && prevScrollTop.current >= prevMaxScroll - 1; |
There was a problem hiding this comment.
[Critical] R18-1: The one-row scroll-range guard zeroes the true-bottom signal — this carried blocker still stands at the current head (the anchored code is unchanged since it was filed; four finder lenses re-discovered it independently this round, and an executed flip-checked probe re-verified it). wasScrolledToBottomPixels requires prevMaxScroll > 1, so when the previous render overflowed by exactly one row no position reads as "at the bottom" — not even scrollTop 1 === maxScroll, which IS the true bottom of that range. A scrolled-away list that shrinks in place to a multi-item remnant overflowing by exactly one row parks at the live bottom (the drop branch installs the clamp mark with allowFollow: true); on the next growth the re-stick gate and the allowFollow re-follow arm both read wasScrolledToBottomPixels, so neither fires, arms 1/2/4 are dead as well, and the reply renders below the fold with auto-follow disengaged. The guard was added for R11-4 (a mark-less top-park at scrollTop 0 must not re-stick), but it also excludes the exact-bottom row — the two are distinguishable: the R11-4 park sits at scrollTop 0, this case at scrollTop 1 === maxScroll. Measured mitigation: the dead window is exactly one growth (once prevMaxScroll reaches 2 the parked position falls back inside the tolerance and the arm fires), but a reply that completes in one chunk leaves the user below the fold until a manual scroll.
Witness (probe at this commit, flip-checked): parked frame x1..x10 (scrollTop 1 of an 11-row remnant in a 10-row container); growth #1 → × probe R18-1 … expected 'x10' to be 'g0' (reply below the fold); growth #2 self-heals. Admitting the exact bottom at prevMaxScroll === 1 flips the probe to pass and keeps all 53 existing pins green.
Fix direction (verified in a scratch tree; closes R22-1 too): record the park kind at install time (allowFollow: data.length > 1 && newScrollTop > 0 — overflow live-bottom parks only) and drop the stale pixel re-check from the allowFollow arm; both this probe and R22-1's pass with all 53 pins green. Narrower alternative: admit the exact bottom of one-row ranges — prevScrollTop.current > 0 && prevScrollTop.current >= prevMaxScroll - 1 — keeping scrollTop 0 excluded. Caution, measured: the naive wasScrolledToBottomPixels → !contentPreviouslyFit swap flips the probe but breaks the R17-1 pin (a fitting park's allowFollow goes stale across the banner-only collapse). Add a regression test parking at maxScroll === 1.
中文说明
R18-1：单行滚动范围守卫把真正的底部信号清零 —— 该遗留阻断在当前 head 上依然成立（锚定代码自提出以来未变；本轮四个审查视角独立重新发现，并由一个可翻转验证的探针再次确认）。wasScrolledToBottomPixels 要求 prevMaxScroll > 1，因此当上一渲染恰好溢出一行时，任何位置都不会被读作"在底部" —— 即使 scrollTop 1 === maxScroll 正是该范围的真正底部。一个已上滚（贴底解除）的列表原地收缩为"恰好溢出一行"的多条目残余时，drop 分支把视口停在活底部（安装 allowFollow: true 的钳制标记）；下一次增长时，re-stick 门控与 allowFollow 重跟随分支都读取 wasScrolledToBottomPixels，两者都不触发，分支 1/2/4 同样全部失效，回复渲染在可视区域之下、自动跟随处于解除状态。该守卫是为 R11-4 添加的（无标记的顶部停驻 scrollTop 0 不得重新贴底），但它同时排除了真正的底部行 —— 两者可以区分：R11-4 停在 scrollTop 0，本场景停在 scrollTop 1 === maxScroll。实测缓解：死窗口恰好是一次增长（prevMaxScroll 到 2 后停驻位置重新落入 -1 容差、分支触发），但若回复一次性完成（单 chunk），用户将一直停在可视区域之下直到手动滚动。
证据（在本提交上运行的探针，可翻转验证）：停驻帧 x1..x10（10 行容器中 11 行残余的 scrollTop 1）；增长 #1 → × probe R18-1 … expected 'x10' to be 'g0'（回复在可视区域之下）；增长 #2 自愈。在 prevMaxScroll === 1 时接纳精确底部后探针翻转为通过，且全部 53 个既有测试保持绿色。
修复方向（已在临时树验证；同时关闭 R22-1）：在安装时记录停驻类型（allowFollow: data.length > 1 && newScrollTop > 0 —— 仅溢出态活底部停驻），并去掉 allowFollow 分支中的过期像素复检；本探针与 R22-1 的探针均通过，且 53 个既有测试全绿。更窄的替代方案：接纳单行范围的精确底部 —— prevScrollTop.current > 0 && prevScrollTop.current >= prevMaxScroll - 1 —— 继续排除 scrollTop 0。注意（实测）：朴素的 wasScrolledToBottomPixels → !contentPreviouslyFit 替换虽能翻转探针，但会破坏 R17-1 测试钉（fitting 停驻的 allowFollow 在 banner-only 坍缩后过期）。补充回归测试：停驻在 maxScroll === 1。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| clampMark.allowFollow && | ||
| wasScrolledToBottomPixels && | ||
| totalHeight > prevTotalHeight.current) || |
There was a problem hiding this comment.
[Critical] R22-1: The allowFollow live-bottom-park re-follow arm re-derives "at the bottom" from stale prev-frame pixel math, so a container shrink of ≥ 2 rows between the park and the next growth permanently kills follow re-engagement while clampParked suppresses every other arm — this carried blocker still stands at the current head (the anchored code is unchanged since it was filed; re-verified by an executed flip-checked probe this round). The parked scrollTop S freezes while the shrink raises maxScroll to S + k (k ≥ 2); the pixel check needs S ≥ S + k − 1, which is false forever. Arm 2 needs contentPreviouslyFit (forever false after an overflow park), arm 1's wasAtBottom disjunct is suppressed by clampParked, arm 4 needs sticking — dead until a manual scroll moves the anchor and clears the mark. Trigger reachability is ordinary: scrolled away + a thought-group collapse parking at the live bottom, then a terminal resize ≥ 2 rows smaller, then the next streamed reply.
Witness (probe at this commit, flip-checked): park frame x4..x13, post-resize frame x4..x11; growth #1 → × probe R22-1 … expected 'x11' to be 'g0'; growth #2 → × probe R22-1 … expected 'x11' to be 'g1' — dead on both growths (permanent; contrast R18-1's one-growth window).
Fix direction (same root as R18-1, verified in a scratch tree): record the park kind at install time (allowFollow: data.length > 1 && newScrollTop > 0 marks only overflow live-bottom parks) and drop wasScrolledToBottomPixels from this arm; both probes pass with all 53 existing pins green. Caution, measured: the naive !contentPreviouslyFit swap breaks the R17-1 pin. Add a regression test: live-bottom park → container shrink ≥ 2 rows → grow → follow re-engages.
中文说明
R22-1：allowFollow 活底部停驻的重跟随分支用过期的上一帧像素计算重新推导"是否在底部"，因此停驻与下一次增长之间只要容器缩小 ≥ 2 行，跟随的重新置位就被永久杀死，而 clampParked 同时抑制了所有其他分支 —— 该遗留阻断在当前 head 上依然成立（锚定代码自提出以来未变；本轮由可翻转验证的探针再次确认）。停驻的 scrollTop S 冻结不变，而收缩使 maxScroll 升到 S + k（k ≥ 2）；像素检查需要 S ≥ S + k − 1，永远为假。分支 2 需要 contentPreviouslyFit（溢出停驻后永远为假），分支 1 的 wasAtBottom 分句被 clampParked 抑制，分支 4 需要贴底 —— 在手动滚动移动锚点并清除标记之前全部失效。触发路径很平常：上滚 + 思考块折叠使列表停在活底部，随后终端缩小 ≥ 2 行，再随后下一条流式回复到来。
证据（在本提交上运行的探针，可翻转验证）：停驻帧 x4..x13，resize 后帧 x4..x11；增长 #1 → × probe R22-1 … expected 'x11' to be 'g0'；增长 #2 → × probe R22-1 … expected 'x11' to be 'g1' —— 两次增长均失效（永久；对比 R18-1 的一次增长窗口）。
修复方向（与 R18-1 同根因，已在临时树验证）：在安装时记录停驻类型（allowFollow: data.length > 1 && newScrollTop > 0，仅标记溢出态活底部停驻），并从本分支移除 wasScrolledToBottomPixels；两个探针均通过，且 53 个既有测试全绿。注意（实测）：朴素的 !contentPreviouslyFit 替换会破坏 R17-1 测试钉。补充回归测试：活底部停驻 → 容器缩小 ≥ 2 行 → 增长 → 跟随重新置位。
— qwen3.8-max via Qwen Code /review (v0.22.0)
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
2 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- stale test comments asserting ScrollableList carries no key / /clear does not remount (VirtualizedList.test.tsx:301, 1034, 1188) — already recorded in the round-19/20 deferral lists as D19-1/D20-1 (reviews 5000164364, 5000669914) and the ro…
- scrollToIndex/scrollToItem lack the fitting-content no-op guard (VirtualizedList.tsx:928) — already recorded in the round-6 deferral list (review 4979659125) and round-8 deferral D8-1 (review 4981837967)
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally (the macOS/Windows unit-test matrix jobs were also skipped; the diff is platform-independent TypeScript, and the ubuntu unit-test job plus this review's full packages/cli suite passed).
Test Plan (not a blocker): 32 tests pass — this review observed 23898 passed.
[Critical] R2-1: Triage stage-1c direction gate: this PR reverses the intentional, test-locked collapse layout from #7652 and remains held pending a maintainer product-direction decision on #9300 — whether bottom-aligning short VP content (blank space above the conversation, latest message directly above the composer) is the intended layout. Re-checked live this round: issue #9300 is OPEN with need-discussion; the last thread comment (@doudouOUC, 2026-08-17T05:43:02Z) keeps the gate open — no maintainer direction call has been recorded since, and the PR has 0 approvals (the sole human review is a token-permission test, not a direction call). The gate's named follow-up remains half-met: the design comment above rootHeight IS rewritten by this diff, but the PR description contains no mention of #7652 and does not say why #7652's removal of the earlier bottom-align attempt doesn't apply this time. Still stands — this is a product decision, not a code defect settleable from code. Failure scenario: merging before the direction call ships a contested default-on layout and re-locks the opposite contract in the three rewritten test pins; if the maintainer keeps the #7652 collapse layout, the change must be reverted together with its inverted test pins, and #9300 will have been auto-closed while its own thread says the direction is undecided. Witness: gh live fetch this round — issue #9300 state OPEN, labels include need-discussion, newest thread comment 2026-08-17T05:43:02Z "the remaining blocker is purely the product-direction call, which needs maintainer sign-off — keeping need-discussion in place"; PR #9305 reviewDecision CHANGES_REQUESTED, 0 approvals; grep '7652' over the PR body: 0 matches.
中文说明
仅完成部分审查，审查缺口已披露。
本轮确认的 2 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally (the macOS/Windows unit-test matrix jobs were also skipped; the diff is platform-independent TypeScript, and the ubuntu unit-test job plus this review's full packages/cli suite passed)。
Test Plan（非阻断）：32 tests pass — this review observed 23898 passed。
[Critical] R2-1: Triage stage-1c direction gate: this PR reverses the intentional, test-locked collapse layout from #7652 and remains held pending a maintainer product-direction decision on #9300 — whether bottom-aligning short VP content (blank space above the conversation, latest message directly above the composer) is the intended layout. Re-checked live this round: issue #9300 is OPEN with need-discussion; the last thread comment (@doudouOUC, 2026-08-17T05:43:02Z) keeps the gate open — no maintainer direction call has been recorded since, and the PR has 0 approvals (the sole human review is a token-permission test, not a direction call). The gate's named follow-up remains half-met: the design comment above rootHeight IS rewritten by this diff, but the PR description contains no mention of #7652 and does not say why #7652's removal of the earlier bottom-align attempt doesn't apply this time. Still stands — this is a product decision, not a code defect settleable from code. Failure scenario: merging before the direction call ships a contested default-on layout and re-locks the opposite contract in the three rewritten test pins; if the maintainer keeps the #7652 collapse layout, the change must be reverted together with its inverted test pins, and #9300 will have been auto-closed while its own thread says the direction is undecided. Witness: gh live fetch this round — issue #9300 state OPEN, labels include need-discussion, newest thread comment 2026-08-17T05:43:02Z "the remaining blocker is purely the product-direction call, which needs maintainer sign-off — keeping need-discussion in place"; PR #9305 reviewDecision CHANGES_REQUESTED, 0 approvals; grep '7652' over the PR body: 0 matches.
— qwen3.8-max via Qwen Code /review (v0.22.0)
| const wasScrolledToBottomPixels = | ||
| prevScrollTop.current >= | ||
| prevTotalHeight.current - prevContainerHeight.current - 1; | ||
| prevMaxScroll > 1 && prevScrollTop.current >= prevMaxScroll - 1; |
There was a problem hiding this comment.
[Critical] R18-1: The one-row scroll-range guard zeroes the true-bottom signal — this carried blocker still stands at the current head, re-verified by an executed flip-checked probe at this commit. wasScrolledToBottomPixels requires prevMaxScroll > 1, so when the previous render overflowed by exactly one row no position reads as "at the bottom" — not even scrollTop 1 === maxScroll, which IS the true bottom of that range. A clamp-park at the bottom of a one-row-overflow remnant (the drop branch installs the mark with allowFollow: true) then never re-engages follow on growth: arm 1 is blocked by clampParked, arm 2 by contentPreviouslyFit false, arm 3 by this zeroed signal, arm 4 by sticking off — and once the first growth is ≥ 2 rows the comparison never holds again, so auto-follow stays dead until a manual scroll.
Failure scenario: the user scrolled away (sticking released) in an overflowing conversation; an in-session shrink (/history collapse, VP thought-group collapse — same session, so the session-key remount does not fire) leaves a multi-item remnant overflowing by exactly one row; the drop branch parks at scrollTop 1 with an allowFollow mark; the next message (≥ 2 rows) renders below the fold and follow never re-engages.
Witness (probe at the reviewed commit, flip-checked): scenario = stuck 20-item list, scrollBy(-5), shrink to an 11-item remnant (exactly 1 row of overflow, park at scrollTop 1), append a 2-row growth item under the production estimator. PRISTINE: frame after growth = it-101..it-110 — expected it-103..g0, g1 (growth below the fold, follow dead). FLIP (stale bottom-ness read removed from the allowFollow arm): the same probe passed 2/2 (it-103..it-110, g0, g1); fix reverted afterward. Note: a naive deletion of the stale read breaks 3 pinned sibling tests (R5-3, R17-1, R14-1 shapes), and every repo test uses the () => 1 estimator that masks this case — the fix needs a live bottom test, not a deletion.
Suggested fix: exclude only the TOP of a one-row range from the bottom signal, keeping its true bottom — in the allowFollow arm, trust the mark's live position (compare the parked scrollTop against the current maxScroll, or validate the mark against dataset continuity) instead of the stale prev-frame pixels. Add a regression test: park at a one-row-overflow remnant, grow, expect follow to re-engage — using an estimator that does not mask the case.
中文说明
R18-1：单行滚动范围守卫把真正的底部信号清零 —— 该遗留阻断在当前 head 上依然成立，本轮已在受审提交上用可翻转验证的探针再次确认。wasScrolledToBottomPixels 要求 prevMaxScroll > 1，因此当上一帧恰好溢出一行时，任何位置都不会被读作"在底部"—— 包括 scrollTop 1 === maxScroll 这个该范围的真正底部。停驻在"溢出一行"残余底部的钳制停驻（drop 分支以 allowFollow: true 安装标记）在增长时永远无法重新置位跟随：臂 1 被 clampParked 阻断、臂 2 因 contentPreviouslyFit 为假被阻断、臂 3 被这个清零的信号阻断、臂 4 因贴底已解除被阻断 —— 且首次增长 ≥ 2 行后该比较永远不再成立，自动跟随自此失效，直到手动滚动。
故障场景：用户在溢出会话中上滚（贴底解除）；一次会话内收缩（/history 折叠、VP 思维组折叠 —— 同一 session，不会触发会话键重挂载）留下恰好溢出一行的多项残余；drop 分支以 allowFollow 标记停驻在 scrollTop 1；下一条消息（≥ 2 行）渲染在可视区域之下，跟随永不恢复。
证据（探针，在受审提交上运行，可翻转验证）：场景 = 贴底 20 项列表、scrollBy(-5)、坍缩到 11 项残余（恰好溢出 1 行，停驻 scrollTop 1）、按生产估计器追加一个 2 行增长项。原始代码：增长后帧 = it-101..it-110 —— 期望 it-103..g0, g1（增长落在可视区域之下，跟随失效）。翻转（从 allowFollow 臂移除过期的底部判定读取）：同一探针 2/2 通过（it-103..it-110, g0, g1）；探针修复随后已还原。注意：直接删除该过期读取会破坏 3 个被锁定的兄弟测试（R5-3、R17-1、R14-1 形态），且仓库内所有测试都使用会掩盖该场景的 () => 1 估计器 —— 修复需要一个"活底部"判定，而不是删除。
建议修复：只把单行范围的顶部排除出底部信号，保留其真正底部 —— 在 allowFollow 臂中信任标记自身的位置（把停驻的 scrollTop 与当前 maxScroll 比较，或对标记做数据集连续性校验），而不是读取过期的上一帧像素值。补充回归测试：停驻在溢出一行的残余上、增长、断言跟随恢复 —— 使用不会掩盖该场景的估计器。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| clampMark.allowFollow && | ||
| wasScrolledToBottomPixels && | ||
| totalHeight > prevTotalHeight.current) || |
There was a problem hiding this comment.
[Critical] R22-1: The allowFollow live-bottom-park re-follow arm re-derives "at the bottom" from stale prev-frame pixel math — this carried blocker still stands at the current head, re-verified by an executed flip-checked probe at this commit. The arm requires wasScrolledToBottomPixels computed from prevScrollTop/prevMaxScroll; after a container shrink of k ≥ 2 rows between the park and the next growth, the parked scrollTop S faces prevMaxScroll = S + k, so S >= S + k - 1 is false forever — while clampParked blocks arm 1, contentPreviouslyFit false blocks arm 2, and sticking off blocks arm 4. The anchor never moves, so the mark never clears: follow is dead until a manual scroll. Reachable without a remount — /rewind and same-id /resume keep the session id, so the session-key remount does not intervene.
Failure scenario: the user scrolls away (sticking released); an in-session shrink parks the viewport at the live bottom with allowFollow: true; the terminal then shrinks by ≥ 2 rows; the next message arrives and renders below the fold, with follow permanently dead until a manual scroll.
Witness (probe at the reviewed commit, flip-checked): scenario = stuck 20/10 list, scrollBy(-2), shrink to a 12-item remnant (park at the live bottom, frame it-202..it-211 asserted), container 10 → 8 (frame it-202..it-209 asserted), append g0. PRISTINE: frame after growth = it-202..it-209 — expected it-205..it-211, g0 (growth below the fold, follow dead). FLIP (stale bottom-ness read removed from the arm): the probe passed (it-205..it-211, g0); fix reverted afterward.
Suggested fix: record the park kind at install time instead of re-deriving bottom-ness from stale prev-frame pixels — when the mark carries allowFollow: true, treat a still-matched mark as the live-bottom signal for the growth arm, or re-derive bottom-ness against the CURRENT maxScroll rather than the previous frame's. Add a regression test extending the live-bottom-park scenario with a ≥ 2-row container shrink followed by growth.
中文说明
R22-1：allowFollow 活底部停驻的重新跟随臂从过期的上一帧像素计算重新推导"在底部"—— 该遗留阻断在当前 head 上依然成立，本轮已在受审提交上用可翻转验证的探针再次确认。该臂要求由 prevScrollTop/prevMaxScroll 计算出的 wasScrolledToBottomPixels；在停驻与下一次增长之间容器收缩 k ≥ 2 行后，停驻的 scrollTop S 面对 prevMaxScroll = S + k，于是 S >= S + k - 1 永远为假 —— 同时 clampParked 阻断臂 1、contentPreviouslyFit 为假阻断臂 2、贴底解除阻断臂 4。锚点从不移动，标记也永不清除：跟随自此失效，直到手动滚动。该场景无需重挂载即可到达 —— /rewind 与同 id 的 /resume 保持会话 id 不变，会话键重挂载不会介入。
故障场景：用户上滚（贴底解除）；一次会话内收缩把视口停驻在活底部并带 allowFollow: true；随后终端收缩 ≥ 2 行；下一条消息到达时渲染在可视区域之下，跟随永久失效，直到手动滚动。
证据（探针，在受审提交上运行，可翻转验证）：场景 = 贴底 20/10 列表、scrollBy(-2)、坍缩到 12 项残余（停驻活底部，帧 it-202..it-211 已断言）、容器 10 → 8（帧 it-202..it-209 已断言）、追加 g0。原始代码：增长后帧 = it-202..it-209 —— 期望 it-205..it-211, g0（增长落在可视区域之下，跟随失效）。翻转（从该臂移除过期的底部判定读取）：探针通过（it-205..it-211, g0）；探针修复随后已还原。
建议修复：在安装标记时记录停驻类型，而不是从过期的上一帧像素重新推导底部状态 —— 当标记携带 allowFollow: true 时，把仍然匹配的标记本身作为增长臂的活底部信号，或者针对当前帧的 maxScroll（而非上一帧）重新推导底部判定。补充回归测试：在活底部停驻场景后追加一次 ≥ 2 行的容器收缩，再增长，断言跟随恢复。
— qwen3.8-max via Qwen Code /review (v0.22.0)
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
5 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- the fitting-content no-op guard missing from scrollToIndex/scrollToItem while triplicated across scrollBy/scrollTo/scrollToEnd (VirtualizedList.tsx:928) — already recorded in the round-6 deferral list (review 4979659125), round-8 deferral D…
- stale test comments asserting ScrollableList carries no key / /clear does not remount the list (VirtualizedList.test.tsx:301, 1034, 1188) — already recorded in the round-19/20 deferral lists as D19-1/D20-1 (reviews 5000164364, 5000669914) a…
- the fitting-content no-op rule triplicated across scrollBy/scrollTo/scrollToEnd, each recomputing maxScroll (VirtualizedList.tsx:913) — already recorded in round-8 deferral D8-1 (review 4981837967) and the rounds 13-24 already-reported list…
- the no-op scroll loop pinning only the engaging sticking direction while the release direction survives mutation (VirtualizedList.test.tsx:284) — already recorded in round-8 deferral D8-2 (review 4981837967) and the rounds 13-24 already-rep…
- zero debugLogger coverage of sticking transitions in the clamp-mark state machine (VirtualizedList.tsx) — already recorded in round-8 deferral D8-3 (review 4981837967) and the rounds 13-24 already-reported lists
Not reviewed: build-and-test — the build-test harness's test phase never ran: packages/cli's type-check build fails on a pre-existing merge-base error (session-swap-telemetry.test.ts:197/215 TS1117, byte-identical at merge base 814b18d, merged into this branch from main after round 24); CI's ubuntu Test job fails on the same error inside npm-ci's prepare step before any test runs; targeted suites passed (VirtualizedList.test.tsx 53/53, MainContent.test.tsx 34/34).
Not reviewed: build-and-test — test-efficacy probe could not be validated in this repo (fresh-checkout prerequisite guard; harnessValidated null, 0 mutants and 0 hunk probes executed by the harness).
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally (the macOS/Windows unit-test lanes were also skipped; the diff is platform-independent TypeScript).
Convergence: round 25 posted 1 inline comment(s), 1 of them reported for the first time; the previous round posted 2 (0 new). Findings keep coming back to the same files: packages/cli/src/ui/components/shared/VirtualizedList.tsx (findings in rounds 18, 22; 1 more now). A cluster that keeps producing siblings usually means the fixes are treating instances of a shared root cause — triaging that cause before the next round, or splitting an independent cluster into its own pull request, tends to end the loop faster than fixing them one at a time. (Observation only — nothing was withheld from this review because of this observation.)
[Critical] R2-1: Triage stage-1c direction gate: this PR reverses the intentional, test-locked collapse layout from #7652 and remains held pending a maintainer product-direction decision on #9300 — whether bottom-aligning short VP content (blank space above the conversation, latest message directly above the composer) is the intended layout. Re-checked live this round: issue #9300 is OPEN with need-discussion; the newest thread comment (@doudouOUC, 2026-08-17T05:43:02Z) keeps the gate open; the PR has 0 approvals (the sole human review is a token-permission test, not a direction call). The gate's named follow-up remains half-met: the design comment above rootHeight IS rewritten by this diff, but the PR description still does not say why #7652's removal of the earlier bottom-align attempt doesn't apply this time. Still stands — this is a product decision, not a code defect settleable from code. Failure scenario: merging before the direction call ships a contested default-on layout, auto-closes #9300 while its own thread says the direction is undecided, and re-locks the opposite contract in the three rewritten test pins; if the maintainer keeps the #7652 collapse layout, the change must be reverted together with its inverted test pins. Witness: gh live fetch this round — issue #9300 state OPEN, labels include need-discussion, updatedAt 2026-08-17T05:43:02Z unchanged since the gate-open comment; @doudouOUC 2026-08-17T05:43:02Z: 'the remaining blocker is purely the product-direction call, which needs maintainer sign-off — keeping need-discussion in place'; PR #9305 reviewDecision CHANGES_REQUESTED, 0 approvals, no 7652 mention in the body.
中文说明
仅完成部分审查，审查缺口已披露。
本轮确认的 5 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
未审查：build-and-test — the build-test harness's test phase never ran: packages/cli's type-check build fails on a pre-existing merge-base error (session-swap-telemetry.test.ts:197/215 TS1117, byte-identical at merge base 814b18d, merged into this branch from main after round 24); CI's ubuntu Test job fails on the same error inside npm-ci's prepare step before any test runs; targeted suites passed (VirtualizedList.test.tsx 53/53, MainContent.test.tsx 34/34)。
未审查：build-and-test — test-efficacy probe could not be validated in this repo (fresh-checkout prerequisite guard; harnessValidated null, 0 mutants and 0 hunk probes executed by the harness)。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally (the macOS/Windows unit-test lanes were also skipped; the diff is platform-independent TypeScript)。
收敛情况：第 25 轮发布了 1 条行内评论，其中 1 条是首次提出；上一轮发布了 2 条（其中 0 条首次提出）。发现反复回到同一批文件：packages/cli/src/ui/components/shared/VirtualizedList.tsx（第 18、22 轮已出过发现，本轮又有 1 条）。一个不断再生兄弟发现的簇，通常意味着逐条修复只在处理同一根因的实例——先定位并处理该根因，或把独立的簇拆成单独的 PR，通常比逐条修复更快结束循环。（仅为观察——本轮评审未因此扣留任何内容。）
[Critical] R2-1: Triage stage-1c direction gate: this PR reverses the intentional, test-locked collapse layout from #7652 and remains held pending a maintainer product-direction decision on #9300 — whether bottom-aligning short VP content (blank space above the conversation, latest message directly above the composer) is the intended layout. Re-checked live this round: issue #9300 is OPEN with need-discussion; the newest thread comment (@doudouOUC, 2026-08-17T05:43:02Z) keeps the gate open; the PR has 0 approvals (the sole human review is a token-permission test, not a direction call). The gate's named follow-up remains half-met: the design comment above rootHeight IS rewritten by this diff, but the PR description still does not say why #7652's removal of the earlier bottom-align attempt doesn't apply this time. Still stands — this is a product decision, not a code defect settleable from code. Failure scenario: merging before the direction call ships a contested default-on layout, auto-closes #9300 while its own thread says the direction is undecided, and re-locks the opposite contract in the three rewritten test pins; if the maintainer keeps the #7652 collapse layout, the change must be reverted together with its inverted test pins. Witness: gh live fetch this round — issue #9300 state OPEN, labels include need-discussion, updatedAt 2026-08-17T05:43:02Z unchanged since the gate-open comment; @doudouOUC 2026-08-17T05:43:02Z: 'the remaining blocker is purely the product-direction call, which needs maintainer sign-off — keeping need-discussion in place'; PR #9305 reviewDecision CHANGES_REQUESTED, 0 approvals, no 7652 mention in the body.
— qwen3.8-max via Qwen Code /review (v0.22.0)
| const wasScrolledToBottomPixels = | ||
| prevScrollTop.current >= | ||
| prevTotalHeight.current - prevContainerHeight.current - 1; | ||
| prevMaxScroll > 1 && prevScrollTop.current >= prevMaxScroll - 1; |
There was a problem hiding this comment.
[Critical] R25-1: The clamp-park re-follow machinery re-derives "the user is at the bottom" from stale prev-frame pixel math (wasScrolledToBottomPixels computed from prevScrollTop/prevMaxScroll), and that inference cannot reliably carry park semantics. This finding supersedes the carried blockers R18-1 (tsx:437) and R22-1 (tsx:494-498), which are its two demonstrated entrances. The entrance space — every shrink/growth/resize geometry history — is unbounded: 24 rounds of this loop have minted a sibling defect in this same mechanism roughly every other round (R4-1, R5-1/2/3, R6-1/2, R8-1, R10-1, R11-1/4/6, R14-1, R17-1/2/3, R18-1, R22-1), each fixed point-wise, each fix minting the next corner. Two live entrances are demonstrated at this commit. (A) prevMaxScroll > 1 zeroes the true-bottom signal for the entire one-row scroll range, so a clamp-park at the bottom of a one-row-overflow remnant (the mark installed with allowFollow: true) never re-engages follow on growth: arm 1 is blocked by clampParked, arm 2 by contentPreviouslyFit false, arm 3 by this zeroed signal, arm 4 by sticking off. (B) The allowFollow arm at tsx:494-498 re-derives bottom-ness from stale prev-frame pixels: after a container shrink of k >= 2 rows between the park and the next growth, the parked scrollTop S faces prevMaxScroll = S + k, so S >= S + k - 1 is false forever while every other arm is blocked. In both shapes the outcome is the same: a user who scrolled away (sticking released) in an overflowing conversation, then an in-session shrink (/history collapse, VP thought-group collapse, /rewind, same-id /resume — none crosses the session-key remount) parks the viewport at the remnant's live bottom with an allowFollow: true mark; the next message renders below the fold and auto-follow stays dead until a manual scroll.
Witness (flip-checked probe at the reviewed commit 4960a7e, isolated scratch tree; all 53 pinned VirtualizedList tests stay green under the flip):
Entrance A: stuck 20 items -> scrollBy(-5) -> shrink to 11-item remnant
(exactly 1 row of overflow, park scrollTop 1) -> append 2-row growth.
PRISTINE: frame after growth = ['item-1'..'item-10'] — g0/g1 below the fold, follow dead.
Entrance B: park scrollTop 2 -> container 10 -> 8 -> growth.
PRISTINE: frame after growth = ['item-2'..'item-9'] — g0/g1 below the fold, follow dead.
Boundary: the same scenario with a 1-row container shrink (k=1) passes pristine
(['item-5'..'item-11','g0','g1']).
FLIP (trust the still-matched allowFollow mark as the live-bottom signal;
stale pixel read removed from the arm): both probes 2/2 pass.
Note: the repo's flat () => 1 height estimator masks entrance A —
regression tests need a true-height estimator.
Suggested fix: close the surface structurally instead of entrance-by-entrance — give the park marker live-bottom semantics (trust a still-matched allowFollow mark as the live-bottom signal for the growth arm, or compare the parked scrollTop against the CURRENT maxScroll instead of the previous frame's), or move sticking transitions to explicit user-intent signals (every production user scroll already flows through the imperative API, which sets sticking explicitly). Minimal entrance-A fix shape (not sufficient alone for entrance B):
const wasScrolledToBottomPixels =
(prevMaxScroll > 1 || prevScrollTop.current > 0) &&
prevScrollTop.current >= prevMaxScroll - 1;Add regression tests for both entrances with a non-masking height estimator.
中文说明
R25-1：钳制停驻（clamp-park）的重新跟随机制从过期的上一帧像素计算重新推导"用户在底部"（wasScrolledToBottomPixels 由 prevScrollTop/prevMaxScroll 计算），而该推导无法可靠地承载停驻语义。本发现收编遗留阻断 R18-1（tsx:437）与 R22-1（tsx:494-498）——它们是本缺陷的两个已演示入口。入口空间（所有收缩/增长/改尺寸的几何历史）是无界的：本循环 24 轮以来，同一机制大约每隔一轮就产生一个兄弟缺陷（R4-1、R5-1/2/3、R6-1/2、R8-1、R10-1、R11-1/4/6、R14-1、R17-1/2/3、R18-1、R22-1），每次逐点修复之后都会催生下一个角落。本提交上有两个存活入口。(A) prevMaxScroll > 1 把整个单行滚动范围的真正底部信号清零——停驻在"溢出一行"残余底部的钳制停驻（标记以 allowFollow: true 安装）在增长时永远无法重新置位跟随：臂 1 被 clampParked 阻断、臂 2 因 contentPreviouslyFit 为假被阻断、臂 3 被这个清零的信号阻断、臂 4 因贴底已解除被阻断。(B) tsx:494-498 的 allowFollow 臂从过期的上一帧像素重新推导底部状态：停驻与下一次增长之间容器收缩 k >= 2 行时，停驻的 scrollTop S 面对 prevMaxScroll = S + k，于是 S >= S + k - 1 永远为假，同时其余各臂均被阻断。两种形态的结果相同：用户在溢出会话中上滚（贴底解除）；一次会话内收缩（/history 折叠、VP 思维组折叠、/rewind、同 id /resume——均不跨越会话键重挂载）把视口停驻在残余的活底部并带 allowFollow: true 标记；下一条消息渲染在可视区域之下，自动跟随永久失效，直到手动滚动。
证据（受审提交 4960a7e 上的可翻转探针，隔离树运行；翻转状态下全部 53 个 VirtualizedList 锁定测试保持绿色）：
入口 A：贴底 20 项 -> scrollBy(-5) -> 坍缩到 11 项残余（恰好溢出 1 行，
停驻 scrollTop 1）-> 追加 2 行增长项。
原始代码：增长后帧 = ['item-1'..'item-10']——g0/g1 落在可视区域之下，跟随失效。
入口 B：停驻 scrollTop 2 -> 容器 10 -> 8 -> 增长。
原始代码：增长后帧 = ['item-2'..'item-9']——g0/g1 落在可视区域之下，跟随失效。
边界：同样场景在容器收缩 k=1 时原始代码即通过（['item-5'..'item-11','g0','g1']）。
翻转（信任仍然匹配的 allowFollow 标记作为活底部信号，从臂中移除过期像素读取）：
两个探针 2/2 通过。
注意：仓库内统一的 () => 1 高度估计器会掩盖入口 A——回归测试需要使用真实高度估计器。
建议修复：从结构上封闭该入口面，而不是逐个入口打补丁——为停驻标记赋予活底部语义（增长臂信任仍然匹配的 allowFollow 标记作为活底部信号，或把停驻的 scrollTop 与当前帧的 maxScroll 比较，而不是与上一帧比较），或者把贴底状态迁移到显式的用户意图信号（所有生产路径上的用户滚动本就经过命令式 API，该 API 已显式设置贴底状态）。入口 A 的最小修复形态（单独不足以覆盖入口 B）：
const wasScrolledToBottomPixels =
(prevMaxScroll > 1 || prevScrollTop.current > 0) &&
prevScrollTop.current >= prevMaxScroll - 1;为两个入口补充回归测试，使用不掩盖场景的高度估计器。
— qwen3.8-max via Qwen Code /review (v0.22.0)
doudouOUC
left a comment
doudouOUC
left a comment
There was a problem hiding this comment.
Review (human pass, round 26)
I reviewed the full diff at 4960a7e3 (25 commits, +1700/−23, 4 files). This is a well-tested PR with thorough edge-case coverage. The CI bot has done 25 rounds — I independently verified the standing findings and checked for new issues.
Standing Criticals (verified at HEAD)
R2-1 (direction gate) — still open. Issue #9300 remains OPEN with need-discussion; the newest thread (@doudouOUC, Aug 17) confirms the product-direction call is the sole remaining blocker. The PR description still does not address why #7652's removal of the earlier bottom-align attempt does not apply this time. Not a code defect — requires maintainer sign-off.
R25-1 (stale prev-frame pixel math) — still standing. The wasScrolledToBottomPixels computation at the top of the useLayoutEffect still derives "at the bottom" from prevScrollTop.current and prevMaxScroll — values from the previous render frame. The prevMaxScroll > 1 guard (R11-4) correctly prevents the one-row false positive, but the fundamental issue persists: a container shrink that changes prevMaxScroll between the prev frame and the current frame can produce a stale prevScrollTop reading. The !clampParked and actualScrollTop >= prevScrollTop.current conditions in the re-stick gate provide additional protection, but the allowFollow arm of the auto-scroll gate (clampParked && clampMark.allowFollow && wasScrolledToBottomPixels && totalHeight > prevTotalHeight.current) relies on wasScrolledToBottomPixels without the !clampParked guard — it is the only path that uses the stale prev-frame computation without the clamp-park gate.
R18-1 fixes (verified)
The structural fixes address the 8 sub-findings from round 18:
- Session key remount (
key={sessionStats.sessionId}) —/clearand/resumenow remount the list, resetting all scroll state by construction. This is the cleanest fix for the dataset-swap cases. - Positional mark (no item key) — avoids the p-N to h-N re-key and constant banner-key validation issues.
swapEntryguard — prevents marks on out-of-range carried anchors.allowFollowflag — distinguishes live-bottom parks from banner-only remnants.
No new Criticals found
The code is well-structured and the 1488+ lines of test coverage exercise the edge cases at high granularity. I found no new issues beyond the standing Criticals above.
Build & Test
I could not run the full test suite on this machine (Windows: getaddrinfo failure blocks git fetch, so the worktree-based build/test pipeline is unavailable). The CI bot reports 53/53 VirtualizedList tests and 34/34 MainContent tests passing, which is consistent with the diff being platform-independent TypeScript.
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
3 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- stale test comments asserting ScrollableList carries no key / /clear does not remount the list (VirtualizedList.test.tsx:301, 1034, 1188) — already recorded in the round-19/20 deferral lists as D19-1/D20-1 (reviews 5000164364, 5000669914) a…
- the fitting-content no-op rule triplicated across scrollBy/scrollTo/scrollToEnd, each recomputing maxScroll (VirtualizedList.tsx:912-918) — already recorded in round-8 deferral D8-1 (review 4981837967) and the rounds 13-25 already-reported …
- zero debugLogger coverage of sticking transitions in the clamp-mark state machine (VirtualizedList.tsx) — already recorded in round-8 deferral D8-3 (review 4981837967) and the rounds 13-25 already-reported lists
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Not reviewed: build-and-test — test-efficacy probe could not be validated in this repo (fresh-checkout prerequisite guard; harnessValidated null, 0 mutants and 0 hunk probes executed).
Test Plan (not a blocker): 32 tests pass — this review observed 24795 passed.
Deferred under the convergence posture (round 26, not a blocker) — recorded, not requested in this round:
packages/cli/src/ui/components/MainContent.test.tsx:927 — [probe] remount test missing the historyRemountKey-bump negative case that pins the chosen key
[Critical] R2-1 Triage stage-1c direction gate still stands: this PR reverses the intentional, test-locked collapse layout from #7652 and remains held pending a maintainer product-direction decision on #9300 — whether bottom-aligning short VP content (blank space above the conversation, latest message directly above the composer) is the intended layout. This is a product decision, not a code defect settleable from code. Re-checked live this round: issue #9300 is OPEN with need-discussion; the newest thread comment (@doudouOUC, 2026-08-17T05:43:02Z) keeps the gate open ("the remaining blocker is purely the product-direction call, which needs maintainer sign-off"); the PR has 0 approvals (reviewDecision CHANGES_REQUESTED). The gate's named follow-up remains half-met: the design comment above rootHeight IS rewritten by this diff, but the PR description still does not say why #7652's removal of the earlier bottom-align attempt doesn't apply this time. Failure scenario: merging before the direction call ships a contested default-on layout, auto-closes #9300 while its own thread says the direction is undecided, and re-locks the opposite contract in the three rewritten test pins; if the maintainer keeps the #7652 collapse layout, the change must be reverted together with its inverted test pins. Fix: hold the merge until a maintainer direction call is recorded on #9300 (or linked in the PR), and address in the PR description why #7652's removal of the earlier bottom-alignment attempt does not apply this time.
中文说明
仅完成部分审查，审查缺口已披露。
本轮确认的 3 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未审查：build-and-test — test-efficacy probe could not be validated in this repo (fresh-checkout prerequisite guard; harnessValidated null, 0 mutants and 0 hunk probes executed)。
Test Plan（非阻断）：32 tests pass — this review observed 24795 passed。
收敛姿态下延后（第 26 轮，非阻断）——已记录，本轮不要求修改：共 1 条（原文未翻译，列表见上方英文部分）。
[Critical] R2-1 Triage stage-1c direction gate still stands: this PR reverses the intentional, test-locked collapse layout from #7652 and remains held pending a maintainer product-direction decision on #9300 — whether bottom-aligning short VP content (blank space above the conversation, latest message directly above the composer) is the intended layout. This is a product decision, not a code defect settleable from code. Re-checked live this round: issue #9300 is OPEN with need-discussion; the newest thread comment (@doudouOUC, 2026-08-17T05:43:02Z) keeps the gate open ("the remaining blocker is purely the product-direction call, which needs maintainer sign-off"); the PR has 0 approvals (reviewDecision CHANGES_REQUESTED). The gate's named follow-up remains half-met: the design comment above rootHeight IS rewritten by this diff, but the PR description still does not say why #7652's removal of the earlier bottom-align attempt doesn't apply this time. Failure scenario: merging before the direction call ships a contested default-on layout, auto-closes #9300 while its own thread says the direction is undecided, and re-locks the opposite contract in the three rewritten test pins; if the maintainer keeps the #7652 collapse layout, the change must be reverted together with its inverted test pins. Fix: hold the merge until a maintainer direction call is recorded on #9300 (or linked in the PR), and address in the PR description why #7652's removal of the earlier bottom-alignment attempt does not apply this time.
— qwen3.8-max via Qwen Code /review (v0.22.0)
| const wasScrolledToBottomPixels = | ||
| prevScrollTop.current >= | ||
| prevTotalHeight.current - prevContainerHeight.current - 1; | ||
| prevMaxScroll > 1 && prevScrollTop.current >= prevMaxScroll - 1; |
There was a problem hiding this comment.
[Critical] R25-1: The clamp-park re-follow machinery re-derives "the user is at the bottom" from stale prev-frame pixel math (wasScrolledToBottomPixels computed from prevScrollTop/prevMaxScroll), and that inference cannot reliably carry park semantics. This carried blocker still stands at the current head, re-verified this round by an executed flip-checked probe at the reviewed commit: both demonstrated entrances reproduce on the unmodified PR code. (a) one-row scroll range — the prevMaxScroll > 1 guard zeroes the bottom signal for the ENTIRE range when the previous render overflowed by exactly one row, even the true bottom scrollTop 1 === maxScroll, so the allowFollow arm (tsx:492-499) and the re-stick gate never fire from a parked-at-live-bottom one-row remnant; (b) a container shrink of >= 2 rows between a live-bottom park and the next growth leaves prevScrollTop = parkTop vs prevMaxScroll = parkTop + k, failing prevScrollTop >= prevMaxScroll - 1 — the allowFollow arm dies, clampParked suppresses the other arms, and follow stays dead. Failure scenario: a user scrolls away in an overflowing conversation (sticking released); content then shrinks in place (thought-group or /history collapse) to a one-row-overflow remnant, or parks at the live bottom after a >=2-row shrink; the drop branch parks the viewport at the new live bottom and installs the mark with allowFollow: true. The next message arrives: every follow re-engagement arm evaluates false (arm 1 blocked by clampParked, arm 2 needs contentPreviouslyFit, arm 3 needs wasScrolledToBottomPixels, arm 4 needs sticking), the anchor never moves, and the reply plus every subsequent message renders below the fold until the user scrolls manually. Witness (probe at the reviewed commit d0ce0bb, flip-checked): ENTRANCE A — after the park at scrollTop 1 === maxScroll, growth at the production height estimate leaves the frame at ["item-1".."item-10"] where follow would show item-4..item-10,g0,g1,g2; ENTRANCE B — after a multi-row live-bottom park and a 2-row container shrink, growth leaves the frame at ["item-4".."item-11"] where follow would show item-7..item-14; a mutant that records the park's overflow in the mark instead of the stale pixel math makes the probe 4/4 pass with all 102 pinned tests green (VirtualizedList 53 + ScrollableList 15 + MainContent 34); fix reverted afterward. Suggested fix: carry the park semantics explicitly instead of re-deriving "at bottom" from prev-frame pixels — make the allowFollow arm and the re-stick gate read the park/live-bottom record the mark already holds; at minimum preserve bottom detection for a one-row range (e.g. prevScrollTop.current >= prevMaxScroll - 1 && (prevMaxScroll > 1 || prevScrollTop.current > 0), keeping the parked TOP at scrollTop 0 excluded) and rework entrance (b) so a >=2-row shrink between park and growth does not strand the allowFollow arm. Add regression tests: park -> one-row-overflow remnant -> one-shot growth re-engages follow; park -> >=2-row container shrink -> growth re-engages follow. This finding supersedes carried blockers R18-1 (tsx:437) and R22-1 (tsx:494-498), which are its two demonstrated entrances.
中文说明
R25-1（遗留阻断项，本轮经受审提交上执行的可翻转探针重新验证，仍然存在）：clamp-park 重新跟随机制仍通过过期的上一帧像素计算（由 prevScrollTop/prevMaxScroll 计算的 wasScrolledToBottomPixels）重新推断"用户在底部"，而该推断无法可靠承载停驻语义。两个已演示的入口在未改动的 PR 代码上均复现：(a) 单行滚动区间 —— 当上一帧恰好溢出 1 行时，prevMaxScroll > 1 守卫把整个区间的底部信号清零，即使真底部 scrollTop 1 === maxScroll 也不例外，因此 allowFollow 分支（tsx:492-499）与 re-stick 门控在"停驻于单行剩余内容底部"时永远不会触发；(b) 在活底部停驻与下一次增长之间容器收缩 >= 2 行时，prevScrollTop = parkTop 而 prevMaxScroll = parkTop + k，prevScrollTop >= prevMaxScroll - 1 不成立 —— allowFollow 分支失效，clampParked 抑制其余分支，跟随保持死亡。故障场景：用户在溢出会话中上滚（贴底解除）；内容随后就地收缩（思维组折叠或 /history 折叠）为恰好溢出 1 行的剩余内容，或在 >=2 行收缩后停驻于活底部；drop 分支把视口停在新活底部并以 allowFollow: true 安装标记。下一条消息到来时：所有重新跟随分支均为假（分支 1 被 clampParked 阻断，分支 2 需要 contentPreviouslyFit，分支 3 需要 wasScrolledToBottomPixels，分支 4 需要贴底），锚点永不移动，回复及其后每条消息都渲染在可视区域之外，直到用户手动滚动。证据（在受审提交 d0ce0bb 上执行的探针，可翻转验证）：入口 A —— 停驻于 scrollTop 1 === maxScroll 之后，按生产高度估计增长，帧停留在 ["item-1".."item-10"]，而跟随生效时应为 item-4..item-10,g0,g1,g2；入口 B —— 多行活底部停驻、容器收缩 2 行后增长，帧停留在 ["item-4".."item-11"]，而跟随生效时应为 item-7..item-14；将停驻时的溢出量记录进标记以替代过期像素计算的突变体使探针 4/4 通过，且全部 102 个固定测试保持绿色（VirtualizedList 53 + ScrollableList 15 + MainContent 34）；探针修复随后已还原。建议修复：显式承载停驻语义，而不是从上帧像素重新推断"在底部" —— 让 allowFollow 分支与 re-stick 门控读取标记已记录的停驻/活底部记录；至少保留单行区间的底部检测（例如 prevScrollTop.current >= prevMaxScroll - 1 && (prevMaxScroll > 1 || prevScrollTop.current > 0)，保持 scrollTop 0 的顶部停驻被排除），并重做入口 (b)，使停驻与增长之间 >=2 行的收缩不会使 allowFollow 分支失效。补充回归测试：停驻 -> 单行溢出剩余 -> 一次性增长重新跟随；停驻 -> 容器收缩 >=2 行 -> 增长重新跟随。本发现取代遗留阻断项 R18-1（tsx:437）与 R22-1（tsx:494-498），二者即它的两个已演示入口。
— qwen3.8-max via Qwen Code /review (v0.22.0)
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
2 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- stale test comments asserting ScrollableList carries no key / /clear does not remount the list (VirtualizedList.test.tsx:301, 1033-1034, 1187-1188) — already reported (deferrals D19-1/D20-1, reviews 5000164364, 5000669914)
- fitting-content no-op rule triplicated across scrollBy/scrollTo/scrollToEnd, each recomputing maxScroll (VirtualizedList.tsx:856-862, 890-895, 912-918) — already reported (deferral D8-1, review 4981837967)
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Not reviewed: build-and-test — test-efficacy probe could not be validated in this repo (fresh-checkout prerequisite guard; harnessValidated null, 0 mutants and 0 hunk probes executed).
Not explored to full depth (tool budget reached): "agent 1a": did not execute the test suites** ( VirtualizedList.test.tsx , MainContent.test.tsx ) — the review worktree has no node_modules installed (nor does the paren….
Test Plan (not a blocker): 32 tests pass — this review observed 25535 passed.
Deferred under the convergence posture (round 27, not a blocker) — recorded, not requested in this round:
packages/cli/src/ui/components/shared/VirtualizedList.tsx:937 (+2 locations) — [review] scrollToIndex/scrollToItem lack the fitting-content no-flip guard their three siblings carry (anchor sits in unchanged code outside the diff hunks, so i…
Residual risk: this loop is persistently critical — Criticals stood in the previous round's work-list and stand again this round (2 Critical(s)), the rate of first-time findings is not falling (this round 0, previous 0), and the standing Critical backlog is not shrinking. The severity floor will not converge it. Recommendation: land-with-residual-risk — the exit is a maintainer risk-acceptance decision (merge, carrying the residual risk), not another review round. Residual-risk inventory for that decision (maintainer to complete):
| standing Critical | attack surface | attacker-dependency | blast radius |
|---|---|---|---|
| (each standing Critical) | … | … | … |
Advisory only — it does not block this review.
[Critical] R2-1 Triage stage-1c direction gate still stands: this PR reverses the intentional, test-locked collapse layout from #7652 and remains held pending a maintainer product-direction decision on #9300 — whether bottom-aligning short VP content (blank space above the conversation, latest message directly above the composer) is the intended layout. This is a product decision, not a code defect settleable from code. Re-checked live this round (2026-08-28): issue #9300 OPEN with need-discussion, updatedAt 2026-08-17T05:43:02Z unchanged since @doudouOUC's gate-open comment ("the remaining blocker is purely the product-direction call, which needs maintainer sign-off — keeping need-discussion in place"); no newer issue comment; PR has 0 approvals (reviewDecision CHANGES_REQUESTED). The gate's named follow-up remains half-met: the rootHeight design comment IS rewritten by this diff, but the PR description still does not say why #7652's removal of the earlier bottom-align attempt doesn't apply this time (body grep '7652' = 0 matches). Failure scenario: merging before the direction call ships a contested default-on layout, auto-closes #9300 while its own thread says the direction is undecided, and re-locks the opposite contract in the three rewritten test pins; if the maintainer keeps the #7652 collapse layout, the change must be reverted together with its inverted test pins. Fix: hold the merge until a maintainer direction call is recorded on #9300 (or linked in the PR), and address in the PR description why #7652's removal of the earlier bottom-alignment attempt does not apply this time.
中文说明
仅完成部分审查，审查缺口已披露。
本轮确认的 2 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未审查：build-and-test — test-efficacy probe could not be validated in this repo (fresh-checkout prerequisite guard; harnessValidated null, 0 mutants and 0 hunk probes executed)。
未探索到全部深度（达到工具调用预算）："agent 1a"：did not execute the test suites** ( VirtualizedList.test.tsx , MainContent.test.tsx ) — the review worktree has no node_modules installed (nor does the paren…。
Test Plan（非阻断）：32 tests pass — this review observed 25535 passed。
收敛姿态下延后（第 27 轮，非阻断）——已记录，本轮不要求修改：共 1 条（原文未翻译，列表见上方英文部分）。
残余风险：本循环处于 persistently-critical 形态——上一轮工作清单中的 Critical 本轮依然存在（本轮 2 条 Critical），首次发现的速率没有下降（本轮 0，上一轮 0），且未决 Critical 积压没有减少。severity floor 无法使其收敛。建议：land-with-residual-risk——出口是 maintainer 的风险接受决定（合入并承担残余风险），而非再开一轮评审。供该决定使用的残余风险清单（maintainer 填写）：按每条未决 Critical 列出「攻击面 · 攻击者依赖性 · 影响范围」三栏。仅为建议——不阻断本次评审。
[Critical] R2-1 Triage stage-1c direction gate still stands: this PR reverses the intentional, test-locked collapse layout from #7652 and remains held pending a maintainer product-direction decision on #9300 — whether bottom-aligning short VP content (blank space above the conversation, latest message directly above the composer) is the intended layout. This is a product decision, not a code defect settleable from code. Re-checked live this round (2026-08-28): issue #9300 OPEN with need-discussion, updatedAt 2026-08-17T05:43:02Z unchanged since @doudouOUC's gate-open comment ("the remaining blocker is purely the product-direction call, which needs maintainer sign-off — keeping need-discussion in place"); no newer issue comment; PR has 0 approvals (reviewDecision CHANGES_REQUESTED). The gate's named follow-up remains half-met: the rootHeight design comment IS rewritten by this diff, but the PR description still does not say why #7652's removal of the earlier bottom-align attempt doesn't apply this time (body grep '7652' = 0 matches). Failure scenario: merging before the direction call ships a contested default-on layout, auto-closes #9300 while its own thread says the direction is undecided, and re-locks the opposite contract in the three rewritten test pins; if the maintainer keeps the #7652 collapse layout, the change must be reverted together with its inverted test pins. Fix: hold the merge until a maintainer direction call is recorded on #9300 (or linked in the PR), and address in the PR description why #7652's removal of the earlier bottom-alignment attempt does not apply this time.
— qwen3.8-max via Qwen Code /review (v0.22.2)
| const wasScrolledToBottomPixels = | ||
| prevScrollTop.current >= | ||
| prevTotalHeight.current - prevContainerHeight.current - 1; | ||
| prevMaxScroll > 1 && prevScrollTop.current >= prevMaxScroll - 1; |
There was a problem hiding this comment.
[Critical] R25-1: The clamp-park re-follow machinery re-derives "the user is at the bottom" from stale prev-frame pixel math (wasScrolledToBottomPixels computed from prevScrollTop/prevMaxScroll), and that inference cannot reliably carry park semantics. This carried blocker still stands at the current head, re-verified this round by an executed flip-checked probe at the reviewed commit. Two demonstrated entrances: (a) one-row scroll range — the prevMaxScroll > 1 guard zeroes the bottom signal for the ENTIRE range when the previous render overflowed by exactly one row, even the true bottom scrollTop 1 === maxScroll, so the allowFollow arm (tsx:492-499) and the re-stick gate never fire from a parked-at-live-bottom one-row remnant; (b) a container shrink of >= 2 rows between a live-bottom park and the next growth leaves prevScrollTop = parkTop vs prevMaxScroll = parkTop + k, failing prevScrollTop >= prevMaxScroll - 1 — the allowFollow arm dies (also on its totalHeight > prevTotalHeight.current growth-only conjunct at the shrink render itself), clampParked suppresses the other arms, and follow stays dead. Failure scenario: a user scrolls away in an overflowing conversation (sticking released); content then shrinks in place (thought-group or /history collapse) to a one-row-overflow remnant, or the terminal shrinks >= 2 rows after a live-bottom park; the drop branch parks the viewport at the new live bottom and installs the mark with allowFollow: true. The next message arrives: every follow re-engagement arm evaluates false (arm 1 blocked by clampParked, arm 2 needs contentPreviouslyFit, arm 3 needs wasScrolledToBottomPixels, arm 4 needs sticking), the anchor never moves, and the reply plus every subsequent message renders below the fold until the user scrolls manually. Four independent finder lenses this round re-discovered these entrances on the unmodified code.
Witness (probe at d21e8f5, flip-checked):
scroll-away -> shrink to 11 one-row items in a 10-row container -> grow by one item
HEAD: AssertionError: expected [ 'r2'..'r10','g0' ], received [ 'r1'..'r10' ]
fixed: probe passes; VirtualizedList.test.tsx 53 passed (53)
The round-26 probe re-verified entrance (b) at d0ce0bb; this file is byte-identical since. This finding supersedes carried blockers R18-1 (tsx:437) and R22-1 (tsx:494-498), which are its two demonstrated entrances.
Suggested fix: carry the park semantics explicitly instead of re-deriving "at bottom" from prev-frame pixels — make the allowFollow arm and the re-stick gate read the park/live-bottom record the mark already holds; at minimum preserve bottom detection for a one-row range and rework entrance (b) so a >=2-row shrink between park and growth does not strand the allowFollow arm:
const wasScrolledToBottomPixels =
prevScrollTop.current >= prevMaxScroll - 1 &&
(prevMaxScroll > 1 || prevScrollTop.current > 0);(The parked TOP at scrollTop 0 stays excluded.) Add regression tests: park -> one-row-overflow remnant -> one-shot growth re-engages follow; park -> >=2-row container shrink -> growth re-engages follow — reverting the fix must turn both red while the existing R11-4 top-of-range test stays green.
中文说明
R25-1（遗留阻断项，本轮经受审提交上执行的可翻转探针重新验证，仍然存在）：clamp-park 重新跟随机制仍通过过期的上一帧像素计算（由 prevScrollTop/prevMaxScroll 计算的 wasScrolledToBottomPixels）重新推断"用户在底部"，而该推断无法可靠承载停驻语义。两个已演示的入口：(a) 单行滚动区间 —— 当上一帧恰好溢出 1 行时，prevMaxScroll > 1 守卫把整个区间的底部信号清零，即使真底部 scrollTop 1 === maxScroll 也不例外，因此 allowFollow 分支（tsx:492-499）与 re-stick 门控在"停驻于单行剩余内容底部"时永远不会触发；(b) 在活底部停驻与下一次增长之间容器收缩 >= 2 行时，prevScrollTop = parkTop 而 prevMaxScroll = parkTop + k，prevScrollTop >= prevMaxScroll - 1 不成立 —— allowFollow 分支失效（在收缩渲染上也会因仅认内容增长的 totalHeight > prevTotalHeight.current 分句失效），clampParked 抑制其余分支，跟随保持死亡。故障场景：用户在溢出会话中上滚（贴底解除）；内容随后就地收缩（思维组折叠或 /history 折叠）为恰好溢出 1 行的剩余内容，或在活底部停驻后终端收缩 >=2 行；drop 分支把视口停在新活底部并以 allowFollow: true 安装标记。下一条消息到来时：所有重新跟随分支均为假（分支 1 被 clampParked 阻断，分支 2 需要 contentPreviouslyFit，分支 3 需要 wasScrolledToBottomPixels，分支 4 需要贴底），锚点永不移动，回复及其后每条消息都渲染在可视区域之外，直到用户手动滚动。本轮四个独立的审查视角在未改动的代码上重新发现了这些入口。
证据（在 d21e8f5 上执行的探针，可翻转验证）：上滚 → 在 10 行容器中收缩为 11 个单行条目 → 增长 1 项：HEAD 报错（expected [ 'r2'..'r10','g0' ], received [ 'r1'..'r10' ]）；修复后探针通过，且全部 53 个 VirtualizedList 测试保持绿色。第 26 轮的探针已在 d0ce0bb 上重新验证入口 (b)；该文件自那以后字节级未变。本发现取代遗留阻断项 R18-1（tsx:437）与 R22-1（tsx:494-498），二者即它的两个已演示入口。
建议修复：显式承载停驻语义，而不是从上帧像素重新推断"在底部" —— 让 allowFollow 分支与 re-stick 门控读取标记已记录的停驻/活底部记录；至少保留单行区间的底部检测（例如 prevScrollTop.current >= prevMaxScroll - 1 && (prevMaxScroll > 1 || prevScrollTop.current > 0)，保持 scrollTop 0 的顶部停驻被排除），并重做入口 (b)，使停驻与增长之间 >=2 行的收缩不会使 allowFollow 分支失效。补充回归测试：停驻 -> 单行溢出剩余 -> 一次性增长重新跟随；停驻 -> 容器收缩 >=2 行 -> 增长重新跟随 —— 还原修复后两个测试必须变红，且既有的 R11-4 区间顶部测试保持绿色。
— qwen3.8-max via Qwen Code /review (v0.22.2)
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
2 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- scrollToIndex/scrollToItem lack the fitting-content no-flip guard their three siblings carry (VirtualizedList.tsx:937 +2 locations) — already reported (round-27 deferral in the review body of review 5047911893)
- stale test comments asserting ScrollableList carries no key / /clear does not remount the list (VirtualizedList.test.tsx:301, 1033-1034, 1187-1188) — already reported (deferrals D19-1/D20-1, reviews 5000164364, 5000669914)
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Test Plan (not a blocker): 32 tests pass — this review observed 25888 passed.
Deferred under the convergence posture (round 28, not a blocker) — recorded, not requested in this round:
packages/cli/src/ui/components/MainContent.tsx:501 — [review] remount comment overstates the guarantee: same-id /resume and /restore swap the dataset without a remount (D28-1)
Residual risk: this loop is persistently critical — Criticals stood in the previous round's work-list and stand again this round (2 Critical(s)), the rate of first-time findings is not falling (this round 0, previous 0), and the standing Critical backlog is not shrinking. The severity floor will not converge it. Recommendation: land-with-residual-risk — the exit is a maintainer risk-acceptance decision (merge, carrying the residual risk), not another review round. Residual-risk inventory for that decision (maintainer to complete):
| standing Critical | attack surface | attacker-dependency | blast radius |
|---|---|---|---|
| (each standing Critical) | … | … | … |
Advisory only — it does not block this review.
[Critical] R2-1 Triage stage-1c direction gate still stands: this PR reverses the intentional, test-locked collapse layout from #7652 and remains held pending a maintainer product-direction decision on #9300 — whether bottom-aligning short VP content (blank space above the conversation, latest message directly above the composer) is the intended layout. This is a product decision, not a code defect settleable from code. Re-checked live this round (2026-08-29): issue #9300 OPEN with need-discussion; no maintainer direction statement newer than the gate-open comment ("the remaining blocker is purely the product-direction call, which needs maintainer sign-off — keeping need-discussion in place"); the PR has 0 approvals. The gate's named follow-up remains half-met: the rootHeight design comment IS rewritten by this diff, but the PR description still does not say why #7652's removal of the earlier bottom-align attempt doesn't apply this time. Failure scenario: merging before the direction call ships a contested default-on layout, auto-closes #9300 while its own thread says the direction is undecided, and re-locks the opposite contract in the three rewritten test pins; if the maintainer keeps the #7652 collapse layout, the change must be reverted together with its inverted test pins. Fix: hold the merge until a maintainer direction call is recorded on #9300 (or linked in the PR), and address in the PR description why #7652's removal of the earlier bottom-alignment attempt does not apply this time.
中文说明
仅完成部分审查，审查缺口已披露。
本轮确认的 2 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
Test Plan（非阻断）：32 tests pass — this review observed 25888 passed。
收敛姿态下延后（第 28 轮，非阻断）——已记录，本轮不要求修改：共 1 条（原文未翻译，列表见上方英文部分）。
残余风险：本循环处于 persistently-critical 形态——上一轮工作清单中的 Critical 本轮依然存在（本轮 2 条 Critical），首次发现的速率没有下降（本轮 0，上一轮 0），且未决 Critical 积压没有减少。severity floor 无法使其收敛。建议：land-with-residual-risk——出口是 maintainer 的风险接受决定（合入并承担残余风险），而非再开一轮评审。供该决定使用的残余风险清单（maintainer 填写）：按每条未决 Critical 列出「攻击面 · 攻击者依赖性 · 影响范围」三栏。仅为建议——不阻断本次评审。
[Critical] R2-1 Triage stage-1c direction gate still stands: this PR reverses the intentional, test-locked collapse layout from #7652 and remains held pending a maintainer product-direction decision on #9300 — whether bottom-aligning short VP content (blank space above the conversation, latest message directly above the composer) is the intended layout. This is a product decision, not a code defect settleable from code. Re-checked live this round (2026-08-29): issue #9300 OPEN with need-discussion; no maintainer direction statement newer than the gate-open comment ("the remaining blocker is purely the product-direction call, which needs maintainer sign-off — keeping need-discussion in place"); the PR has 0 approvals. The gate's named follow-up remains half-met: the rootHeight design comment IS rewritten by this diff, but the PR description still does not say why #7652's removal of the earlier bottom-align attempt doesn't apply this time. Failure scenario: merging before the direction call ships a contested default-on layout, auto-closes #9300 while its own thread says the direction is undecided, and re-locks the opposite contract in the three rewritten test pins; if the maintainer keeps the #7652 collapse layout, the change must be reverted together with its inverted test pins. Fix: hold the merge until a maintainer direction call is recorded on #9300 (or linked in the PR), and address in the PR description why #7652's removal of the earlier bottom-alignment attempt does not apply this time.
— qwen3.8-max via Qwen Code /review (v0.22.2)
| const wasScrolledToBottomPixels = | ||
| prevScrollTop.current >= | ||
| prevTotalHeight.current - prevContainerHeight.current - 1; | ||
| prevMaxScroll > 1 && prevScrollTop.current >= prevMaxScroll - 1; |
There was a problem hiding this comment.
[Critical] R25-1: (still stands — re-verified this round at a95525ef by an executed flip-checked probe; four independent finder lenses re-discovered entrance (a) on the unmodified code) The clamp-park re-follow machinery re-derives "the user is at the bottom" from stale prev-frame pixel math (wasScrolledToBottomPixels computed from prevScrollTop/prevMaxScroll), and that inference cannot reliably carry park semantics. Entrance (a): the prevMaxScroll > 1 guard zeroes the bottom signal for the ENTIRE one-row scroll range — even the true bottom scrollTop 1 === maxScroll — so after a park at the live bottom of a one-row-overflow remnant the first growth never re-engages follow. Entrance (b): a container shrink of >= 2 rows between a live-bottom park and the next growth leaves prevScrollTop = parkTop vs prevMaxScroll = parkTop + k, failing prevScrollTop >= prevMaxScroll - 1 permanently while clampParked suppresses the other arms — follow never re-engages at all. A user who scrolls away in an overflowing conversation and then sees content shrink in place (thought-group/history collapse) to a one-row-overflow remnant — or whose terminal shrinks >= 2 rows after a live-bottom park — gets the next message rendered below the fold: with a one-row remnant the first streamed item is invisible and the viewport yanks to the end one growth late; after a >=2-row shrink every subsequent message renders below the fold until the user scrolls manually. This is the exact regression class this PR removes for maxScroll >= 2.
Witness (probe at a95525ef, flip-checked):
entrance (a) — shrink to 11 one-row items in a 10-row container, park at scrollTop 1, grow:
HEAD first growth: ["r1".."r10"] <- g0 below the fold
HEAD second growth: ["r3".."r10","g0","g1"] <- yanked one growth late
FIXED first growth: ["r2".."r10","g0"] <- follows immediately
entrance (b) — live-bottom park at scrollTop 2, container 10->8, grow:
HEAD growths 1-3: ["r2".."r9"] unchanged <- follow dead indefinitely
FIXED first growth: ["r5".."r11","g0"] <- follow re-engages
All 53 existing VirtualizedList tests stay green with the fix; none of them covers either entrance. Suggested fix: carry the park semantics explicitly instead of re-deriving "at bottom" from prev-frame pixels — make the allowFollow arm and the re-stick gate read the park/live-bottom record the mark already holds; at minimum preserve bottom detection for a one-row range and rework entrance (b) so a >=2-row shrink between park and growth does not strand the allowFollow arm:
const wasScrolledToBottomPixels =
prevScrollTop.current >= prevMaxScroll - 1 &&
(prevMaxScroll > 1 || prevScrollTop.current > 0);(The parked TOP at scrollTop 0 stays excluded.) Add regression tests: park → one-row-overflow remnant → one-shot growth re-engages follow; park → >=2-row container shrink → growth re-engages follow — reverting the fix must turn both red while the existing R11-4 top-of-range test stays green.
中文说明
R25-1（仍然存在——本轮在 a95525ef 上经可翻转探针重新验证；四个独立审查视角在未改动的代码上重新发现了入口 (a)）：clamp-park 重新跟随机制仍通过过期的上一帧像素计算（由 prevScrollTop/prevMaxScroll 计算的 wasScrolledToBottomPixels）推断"用户在底部"，而该推断无法可靠承载停驻语义。入口 (a)：prevMaxScroll > 1 守卫把整个单行滚动区间的底部信号清零——即使真底部 scrollTop 1 === maxScroll 也不例外——因此停驻在单行溢出剩余内容的活底部后，第一次增长不会重新跟随。入口 (b)：在活底部停驻与下一次增长之间容器收缩 >=2 行时，prevScrollTop = parkTop 而 prevMaxScroll = parkTop + k，prevScrollTop >= prevMaxScroll - 1 永久不成立，且 clampParked 抑制其余分支——跟随彻底死亡。用户在溢出会话中上滚后，内容就地收缩（思维组/历史折叠）为恰好溢出 1 行的剩余内容，或活底部停驻后终端收缩 >=2 行时：下一条消息将渲染在可视区域之外——单行剩余时第一条流式内容不可见、视口晚一次增长才被拽到末尾；>=2 行收缩后每条后续消息都在可视区域之外，直到用户手动滚动。这正是本 PR 为 maxScroll >= 2 消除的那类回归。
证据（在 a95525ef 上执行的探针，可翻转验证）：见上方英文部分的输出对照——入口 (a)：HEAD 第一次增长 g0 在可视区域外、第二次增长才被拽到底部，修复后第一次增长即跟随；入口 (b)：HEAD 三次增长帧始终停在 ["r2".."r9"]（跟随永久死亡），修复后第一次增长即重新跟随。修复下全部 53 个既有 VirtualizedList 测试保持绿色；现有测试均未覆盖这两个入口。建议修复：显式承载停驻语义，而不是从上帧像素重新推断"在底部"——让 allowFollow 分支与 re-stick 门控读取标记已记录的停驻/活底部记录；至少保留单行区间的底部检测（例如上方代码，保持 scrollTop 0 的顶部停驻被排除），并重做入口 (b)，使停驻与增长之间 >=2 行的收缩不会使 allowFollow 分支失效。补充回归测试：停驻 → 单行溢出剩余 → 一次性增长重新跟随；停驻 → 容器收缩 >=2 行 → 增长重新跟随——还原修复后两个测试必须变红，且既有的 R11-4 区间顶部测试保持绿色。
— qwen3.8-max via Qwen Code /review (v0.22.2)
3 participants
What this PR does
In VP mode (
useTerminalBuffer, default on), when the conversation fits within the viewport the content was top-aligned, leaving a blank gap between the last message and the composer (reported in #9300, happens in every session). This bottom-aligns short bottom-stuck content: blank space moves to the TOP and the latest message sits directly above the composer, matching standard chat TUIs.Implementation: in
VirtualizedList, whenisStickingToBottomand content fits (maxScroll === 0), computebottomAlignGap = scrollableContainerHeight - totalHeight, render it as a leading spacer, and keep the root at fullcontainerHeightso there is room. Overflow (maxScroll > 0) and scrolled-away cases are unchanged.Why it's needed
#9300: default-on VP mode showed a large blank area between the last message and the composer in every session (short conversations and collapsed thinking blocks). High-frequency, default-on visual regression on the primary surface — worth a quick fix.
Reviewer Test Plan
How to verify
npx vitest run packages/cli/src/ui/components/shared/VirtualizedList.test.tsx— 32 tests pass, including the 3 updated to the new bottom-aligned layout.npx vitest run packages/cli/src/ui/components/MainContent.test.tsx packages/cli/src/ui/components/shared/ScrollableList.test.tsx— 36 pass (no layout regression).Evidence (Before & After)
Before: content top-aligned, blank gap between last message and composer. After: content bottom-aligned, blank at top. (Needs a live terminal to visually confirm; unit tests pin the new frame layout.)
Tested on
Risk & Scope
scrollableContainerHeightunchanged).totalHeight >= containerHeight(those already bottom-show viaclampedScrollTop = maxScroll).Linked Issues
Fixes #9300
中文说明
这个 PR 做了什么
VP 模式（默认开启）下，当会话内容高度小于视口时，内容顶部对齐，导致最后一条消息与输入框之间出现大片空白（#9300，每个 session 都出现）。本 PR 将短的、贴底的内容改为底部对齐：空白移到顶部，最新消息紧贴输入框上方，与主流 chat TUI 一致。
实现：
VirtualizedList中当isStickingToBottom且内容放得下（maxScroll === 0）时，计算bottomAlignGap = scrollableContainerHeight - totalHeight作为顶部占位，并保持 root 为满containerHeight。溢出或已上滚的场景不变。风险与范围
totalHeight >= containerHeight的高度高估场景（该场景本就通过clampedScrollTop = maxScroll底部显示）。