Skip to content

Conversation

@hannesrudolph
Copy link
Collaborator

@hannesrudolph hannesrudolph commented Feb 10, 2026

Summary

Fixes scroll-anchoring race conditions in the chat view by consolidating scroll tracking into Virtuoso's own callbacks, eliminating stale-closure bugs, and removing direct DOM scroll manipulation from child components.


Related GitHub Issue

Closes: #

Description

isAtBottom state → ref

isAtBottom was React state captured in closures by handleRowHeightChange and the input area's onHeightChange. Because React batches state updates asynchronously, these callbacks frequently read stale values — the scroll position had changed but the closure still held the old boolean. Converting to a ref (isAtBottomRef) ensures every read gets the current value synchronously. This also removes isAtBottom from handleRowHeightChange's dependency array, preventing unnecessary callback re-creation that cascaded into Virtuoso row re-renders.

Consolidated scroll listeners

A useEffect attached a raw scroll event listener to the scroll container DOM element to detect when the user scrolled away from the bottom and clear stickyFollowRef. This duplicated logic already present in Virtuoso's atBottomStateChange callback, creating a race between two sources of truth for the same state. The raw listener is removed. atBottomStateChange now handles all three responsibilities: updating isAtBottomRef, toggling the scroll-to-bottom button, and clearing sticky follow.

New-task scroll anchoring

Starting a new task relied solely on Virtuoso's initialTopMostItemIndex to position the view at the bottom. When message data arrived asynchronously after mount, the view could be left stranded mid-scroll. A new block in the task?.ts effect now also sets stickyFollowRef = true and issues a requestAnimationFramescrollTo({ top: MAX_SAFE_INTEGER }) to cover the async race.

Removed CodeBlock outer-scroll manipulation

CodeBlock was independently tracking the outer Virtuoso scroller's position via document.querySelector and a dedicated scroll listener, then manually setting scrollTop after syntax highlighting completed. This fought Virtuoso's own scroll management. All of this is removed — outer container scrolling is now fully delegated to Virtuoso's followOutput and ChatView's handleRowHeightChange.

What changed, concretely

Aspect Before After
isAtBottom React state (stale in closures) Ref (always current)
Sticky-follow disable Separate scroll listener + Virtuoso callback (duplicate/racing) Single atBottomStateChange callback
New task scroll anchor initialTopMostItemIndex only + sticky follow + explicit scrollTo via rAF
CodeBlock outer scroll Direct DOM query + manual scrollTop Delegated to Virtuoso/ChatView

Test Procedure

  1. Start a new task — view should anchor to bottom immediately, even if messages load asynchronously
  2. During streaming, scroll up — sticky follow should disengage and the "scroll to bottom" button should appear
  3. Click "scroll to bottom" — should snap to bottom and re-engage follow
  4. While at bottom, let a code block with syntax highlighting stream in — outer view should stay anchored without jitter
  5. Expand/collapse a code block row while at bottom — view should smooth-scroll to maintain position
  6. Expand/collapse a code block row while scrolled up — view should NOT jump

…ions

The chat view was losing bottom anchoring due to several interacting
issues in the scroll tracking system:

- A manual scroll listener was attached to the Virtuoso wrapper div
  which doesn't scroll (Virtuoso's internal scroller does), causing
  conflicting state updates with Virtuoso's atBottomStateChange.
  Removed the dead listener and consolidated scroll tracking into
  Virtuoso's callback.

- handleRowHeightChange captured isAtBottom as React state in its
  closure. Since it was in the dependency chain of itemContent, every
  bottom-state change forced Virtuoso to re-render all rows, creating
  timing windows for stale callbacks. Replaced with a ref to break
  the cascade.

- Entering an existing task didn't anchor to bottom because
  stickyFollowRef was never reset on task switch, and
  initialTopMostItemIndex could race with async message loading.
  Now resets stickyFollowRef and explicitly scrolls after a frame.

- CodeBlock directly manipulated the Virtuoso scroller's scrollTop
  via document.querySelector, fighting Virtuoso's own scroll
  management. Removed the direct DOM manipulation in favor of
  Virtuoso's followOutput and handleRowHeightChange mechanisms.
@dosubot dosubot bot added size:M This PR changes 30-99 lines, ignoring generated files. bug Something isn't working labels Feb 10, 2026
@roomote
Copy link
Contributor

roomote bot commented Feb 10, 2026

Rooviewer Clock   See task

All previously flagged issues have been addressed. The cancelAnimationFrame cleanup is now properly wired into the effect's teardown.

  • Cancel requestAnimationFrame in the task-switch effect cleanup to avoid stale callbacks during rapid switches
Previous reviews

Mention @roomote in a comment to request specific changes to this pull request or fix all unresolved issues.

Cancel the requestAnimationFrame in the useEffect cleanup to prevent
stale callbacks from accumulating during rapid task switches.
@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Feb 10, 2026
@hannesrudolph hannesrudolph merged commit 097f648 into RooCodeInc:main Feb 10, 2026
13 checks passed
@hannesrudolph hannesrudolph deleted the fix/scroll-anchoring-race-conditions branch February 10, 2026 23:40
@github-project-automation github-project-automation bot moved this from New to Done in Roo Code Roadmap Feb 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working lgtm This PR has been approved by a maintainer size:M This PR changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants