UI: harden chat scroll interrupts#72957
Conversation
Greptile SummaryThis PR hardens chat scroll auto-follow in the Control UI by adding an explicit Confidence Score: 4/5Safe to merge — well-scoped UI-only change with comprehensive test coverage and no regressions to existing behavior. No P0 or P1 issues found. Logic is correctly implemented: chatFollowLocked, chatSmoothAutoScrolling, and chatLastScrollTop form a sound state machine, all key transitions are covered by deterministic unit tests, and the nested-scrollable ancestor walk correctly guards against false releases. Score reflects P2-only quality with no blocking concerns. No files require special attention. Reviews (1): Last reviewed commit: "UI: refine chat wheel follow handling" | Re-trigger Greptile |
|
Codex review: needs changes before merge. Reviewed June 3, 2026, 10:44 AM ET / 14:44 UTC. Summary PR surface: Source +121, Tests +141, Docs +1. Total +263 across 9 files. Reproducibility: yes. from source inspection: current Review metrics: 1 noteworthy metric.
Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Risk before merge
Maintainer options:
Next step before merge
Security Review findings
Review detailsBest possible solution: Keep the scroll follow-lock implementation and tests, remove the direct Do we have a high-confidence way to reproduce the issue? Yes, from source inspection: current Is this the best way to solve the issue? Yes for the functional fix: the PR changes the existing Control UI scroll owner and adds focused tests while preserving the persisted Full review comments:
Overall correctness: patch is correct AGENTS.md: found and applied where relevant. Codex review notes: model gpt-5.5, reasoning high; reviewed against 8f6f2617ec06. Label changesLabel changes:
Label justifications:
Evidence reviewedPR surface: Source +121, Tests +141, Docs +1. Total +263 across 9 files. View PR surface stats
Acceptance criteria:
What I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
|
a21f3a5 to
88360ac
Compare
|
Thanks for the review. I addressed the missing changelog item in Local validation is green for I also tried to run the requested Testbox changed gate, but Blacksmith auth reports: "You don't have any GitHub organizations with Blacksmith installed. Install the Blacksmith GitHub App first." My GitHub account therefore cannot start an OpenClaw Testbox from here. Since the remaining required gate is specifically One note: because this PR now touches |
|
Related maintainer note: #81629 has now landed a separate persisted WebChat auto-scroll mode selector on This PR is still a distinct bug-fix lane for manual-scroll follow-lock / sticky-bottom behavior, not a duplicate of the setting work. Before any further review or merge attempt, please rebase against latest Thanks for the work on the follow-lock edge cases; this remains useful, just separate from the preference that shipped today. |
|
Thanks, done. I rebased this PR onto latest This PR now keeps #81629's auto-scroll mode selector/settings path intact and only carries the separate manual-scroll follow-lock / sticky-bottom fix:
Verification:
I also updated the PR body with a |
|
@clawsweeper hatch |
|
🦞👀 I queued a comment sync for this PR. If the egg is hatchable, ClawSweeper will generate the image once and update the existing review comment. |
Summary
Describe the problem and fix in 2-5 bullets:
If this PR fixes a plugin beta-release blocker, title it
fix(<plugin-id>): beta blocker - <summary>and link the matchingBeta blocker: <plugin-name> - <summary>issue labeledbeta-blocker. Contributors cannot label PRs, so the title is the PR-side signal for maintainers and automation.ui/src/ui/app-scroll.test.ts.mainafter feat(ui): add WebChat auto-scroll mode selector #81629 landed the separate persisted WebChatchatAutoScrollmode selector. This PR remains the manual-scroll follow-lock / sticky-bottom bug-fix lane, not a duplicate of the setting work.Change Type (select all)
Scope (select all touched areas)
Linked Issue/PR
Root Cause (if applicable)
For bug fixes or regressions, explain why this happened, not just what changed. Otherwise write
N/A. If the cause is unclear, writeUnknown.Regression Test Plan (if applicable)
For bug fixes or regressions, name the smallest reliable test coverage that should catch this. Otherwise write
N/A.ui/src/ui/app-scroll.test.tsUser-visible / Behavior Changes
List user-visible changes (including defaults/config).
If none, write
None.chatAutoScrollsetting from feat(ui): add WebChat auto-scroll mode selector #81629 is preserved and respected; the follow-lock only tightens the default/manual sticky-bottom state machine around that setting.Diagram (if applicable)
For UI changes or non-trivial logic flows, include a small ASCII diagram reviewers can scan quickly. Otherwise write
N/A.Security Impact (required)
Yes/No) NoYes/No) NoYes/No) NoYes/No) NoYes/No) NoYes, explain risk + mitigation: N/ARepro + Verification
Environment
ws://127.0.0.1:18789Steps
Expected
Actual
Evidence
Attach at least one:
Real behavior proof
mainwith feat(ui): add WebChat auto-scroll mode selector #81629's persistedchatAutoScrollsetting present. The proof command imports the actualui/src/ui/app-scroll.tsimplementation and exercises the WebChat scroll state used by the Control UI.pnpm exec tsxproof command againstui/src/ui/app-scroll.tswithchatAutoScroll: "near-bottom", simulating a user scroll from bottom toscrollTop=1540, a streamingscheduleChatScroll()tick, then returning to bottom atscrollTop=1576.{ "proof": "Control UI WebChat manual-scroll follow-lock after patch", "afterManual": { "scrollTop": 1540, "chatUserNearBottom": false, "chatFollowLocked": true, "chatNewMessagesBelow": false, "chatAutoScroll": "near-bottom" }, "afterStreamTick": { "scrollTop": 1540, "chatUserNearBottom": false, "chatFollowLocked": true, "chatNewMessagesBelow": true }, "afterReturnBottom": { "scrollTop": 1576, "chatUserNearBottom": true, "chatFollowLocked": false, "chatNewMessagesBelow": false } }chatFollowLockedistrueand the streaming tick leavesscrollTopat1540instead of snapping to2000;chatNewMessagesBelowbecomestrue. After returning to bottom,chatFollowLockedclears andchatNewMessagesBelowclears. The persistedchatAutoScrollmode remainsnear-bottom.Human Verification (required)
What you personally verified (not just CI), and how:
mainkept feat(ui): add WebChat auto-scroll mode selector #81629'schatAutoScrollselector/settings path intact, then ranpnpm test ui/src/ui/app-scroll.test.ts ui/src/ui/app-chat.test.ts.Review Conversations
If a bot review conversation is addressed by this PR, resolve that conversation yourself. Do not leave bot review conversation cleanup for maintainers.
Compatibility / Migration
Yes/No) YesYes/No) NoYes/No) NoRisks and Mitigations
List only real risks for this PR. Add/remove entries as needed. If none, write
None.