fix(chat): redesign rehydration scroll lifecycle#11491
fix(chat): redesign rehydration scroll lifecycle#11491roomote[bot] wants to merge 1 commit intomainfrom
Conversation
Redesigns chat rehydration scroll lifecycle so existing chats consistently converge to the newest message on open while preserving explicit user control. - Introduce an explicit phase model in ChatView for hydration, anchored following, and user-history browsing states - Replace ad-hoc settle loops with a bounded initial settle window using frame and time caps - Re-anchoring now uses deterministic phase transitions instead of race-prone sticky toggles - Upward user intent (wheel, keyboard navigation, or pointer/manual upward scroll) immediately disengages automatic follow - Scroll-to-bottom CTA re-anchors in one interaction - Settle loop respects user scroll intent via stickyFollowRef gating - Explicit escape hatches cover keyboard nav, pointer drag, wheel, and row expansion scroll-away methods Squashed from scroll-fix branch (PR #11483).
roomote
bot
requested review from
cte,
daniel-lxs,
hannesrudolph,
jr and
mrubens
as code owners
dosubot
bot
added
the
size:XL
The scroll lifecycle redesign is well-structured overall. The explicit phase model and bounded settle window are solid improvements over the previous ad-hoc approach. One race condition found in the settle completion path.
Mention @roomote in a comment to request specific changes to this pull request or fix all unresolved issues. |
![roomote[bot]](https://avatars.githubusercontent.com/in/1546624?s=60&v=4){kind=small}
| const completeInitialSettle = useCallback(() => { | ||
| cancelInitialSettleFrame() | ||
| isSettlingRef.current = false | ||
| if (isAtBottomRef.current && settleBottomConfirmedRef.current) { | ||
| enterAnchoredFollowing() | ||
| return | ||
| } | ||
|
|
||
| transitionScrollPhase("USER_BROWSING_HISTORY") | ||
| setShowScrollToBottom(true) | ||
| }, [cancelInitialSettleFrame, enterAnchoredFollowing, transitionScrollPhase]) |
There was a problem hiding this comment.
completeInitialSettle can override a user-initiated phase transition. If the user triggers an escape hatch (wheel-up, PageUp, pointer drag) during the settle window, enterUserBrowsingHistory sets the phase to USER_BROWSING_HISTORY. But the settle loop's next rAF detects that isSettleWindowOpen is now false (phase changed) and calls completeInitialSettle. If isAtBottomRef.current and settleBottomConfirmedRef.current are both still true at that instant (Virtuoso's atBottomStateChange(false) hasn't fired yet), enterAnchoredFollowing() re-engages follow mode, overriding the user's explicit disengage. Adding a phase guard at the top of this function would make it safe:
| const completeInitialSettle = useCallback(() => { | |
| cancelInitialSettleFrame() | |
| isSettlingRef.current = false | |
| if (isAtBottomRef.current && settleBottomConfirmedRef.current) { | |
| enterAnchoredFollowing() | |
| return | |
| } | |
| transitionScrollPhase("USER_BROWSING_HISTORY") | |
| setShowScrollToBottom(true) | |
| }, [cancelInitialSettleFrame, enterAnchoredFollowing, transitionScrollPhase]) | |
| const completeInitialSettle = useCallback(() => { | |
| cancelInitialSettleFrame() | |
| isSettlingRef.current = false | |
| if (scrollPhaseRef.current !== "HYDRATING_PINNED_TO_BOTTOM") { | |
| return | |
| } | |
| if (isAtBottomRef.current && settleBottomConfirmedRef.current) { | |
| enterAnchoredFollowing() | |
| return | |
| } | |
| transitionScrollPhase("USER_BROWSING_HISTORY") | |
| setShowScrollToBottom(true) | |
| }, [cancelInitialSettleFrame, enterAnchoredFollowing, transitionScrollPhase]) |
Fix it with Roo Code or mention @roomote and request a fix.
2 participants
Related GitHub Issue
Supersedes: #11483
Description
Clean, squashed version of the scroll lifecycle redesign from PR #11483.
Redesigns chat rehydration scroll lifecycle so existing chats consistently converge to the newest message on open while preserving explicit user control.
Key implementation details:
stickyFollowRefgatingstickyFollowRefis only cleared by explicit user actions (wheel-up, keyboard nav up, pointer drag up, row expansion), not by transientatBottomStateChangeflicker during layout reflowsfollowOutputreturns"auto"instead oftruefor consistent Virtuoso behaviorscrollToIndexused instead ofscrollTo({ top: MAX_SAFE_INTEGER })for reliable convergenceTest Procedure
ChatView.scroll-debug-repro.spec.tsxcovering:followOutputcallback returns correct values based on statecd webview-ui && npx vitest run src/components/chat/__tests__/ChatView.scroll-debug-repro.spec.tsxcd webview-ui && npx tsc --noEmitPre-Submission Checklist
Documentation Updates
Additional Notes
This is a clean squash of the 4-commit
scroll-fixbranch from PR #11483. The debug plumbing (chatScrollDebug.ts) that was added and removed during iteration is not present in this PR. All reviewer feedback from PR #11483 has been addressed.