fix/restore thread scroll position - #5552
Conversation
- Preserve each thread's timeline offset across route changes - Restore the saved position when returning to a thread
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| onClick={() => { | ||
| beginTimelineManualNavigation(); | ||
| scrollToEnd(true); | ||
| }} |
There was a problem hiding this comment.
Scroll pill saves partial offset
Medium Severity
The “Scroll to end” control calls beginTimelineManualNavigation before the animated scrollToEnd. That arms manual-scroll tracking, so the first onTimelineScrollOffsetChange where the offset moves more than 0.5px from the starting value persists a manual position at an in-between offset. Leaving the thread before the animation finishes restores the wrong scroll position on return.
Reviewed by Cursor Bugbot for commit 6705a75. Configure here.
| timelineScrollPositionByThreadKey.set( | ||
| routeThreadKey, | ||
| existing?.kind === "manual" ? { ...existing, offset } : { kind: "automatic", offset }, | ||
| ); |
There was a problem hiding this comment.
Early scroll overwrites saved offset
High Severity
onTimelineScrollOffsetChange writes every scroll offset into timelineScrollPositionByThreadKey with no entry/restore guard. MessagesTimeline also invokes the same callback from a requestAnimationFrame handleScroll when rows mount or change, which can run before initialScrollOffset is applied. A transient offset (often near zero) can overwrite the saved position so the next visit restores the wrong place.
Additional Locations (2)
Reviewed by Cursor Bugbot for commit 6705a75. Configure here.
ApprovabilityVerdict: Needs human review This PR introduces new scroll position restoration logic with module-level state management. There are 3 unresolved review comments identifying potential bugs, including a high-severity race condition where transient scroll offsets may overwrite saved positions. You can customize Macroscope's approvability policy. Learn more. |
|
+1 on this finding it very annoying that scroll is not preserved when navigating across threads as it is in Codex app |
github-actions
Bot
added
vouch:unvouched
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 2 potential issues.
There are 4 total unresolved issues (including 2 from previous reviews).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit a7cd406. Configure here.
| return { kind: "restore-position", offset: input.savedPosition.offset }; | ||
| } | ||
| return input.latestTurnId | ||
| ? { kind: "anchor-response", turnId: input.latestTurnId } |
There was a problem hiding this comment.
Automatic offset blocks response anchor
Medium Severity
The new anchor-response path for latestTurnId is skipped whenever any saved timeline position exists, including kind: "automatic" offsets written during initial list layout. After a brief first visit that only auto-settled at the bottom, returning to the same thread restores that offset instead of anchoring at the latest response top described in the PR.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit a7cd406. Configure here.
2 participants
What Changed
Small changes to scroll behavior -
Why
I work with multiple threads quite a bit and it's annoying to scroll up every single time
UI Changes
t3codepr.mp4
(sorry in advance for the crappy video)
Checklist
AI Description -
Restores each web thread’s timeline scroll position when navigating away and back. Active turns reopen at the current response, while manual scrolling is preserved and opts out of auto-follow so streaming updates don’t pull the user away from their chosen position.
Note
Medium Risk
Touches core chat scrolling, LegendList lifecycle, and auto-follow during streaming; regressions could mis-position threads or fight user scroll, but no auth or data changes.
Overview
Per-thread scroll memory — The chat timeline now keeps each thread’s scroll offset in module-level state (keyed by scoped thread key) so switching threads and returning restores where you left off; a full page reload still resets positions.
Smarter initial placement — On open,
resolveTimelineEntryScrollModechooses follow-end, restore saved offset, or anchor to the user message for the active/latest turn. First-time opens of completed threads land at the top of the agent response instead of the bottom; threads with a running turn anchor to that response unless you had manually scrolled that same turn.Manual scroll vs auto-follow — User navigation (wheel, touch, pointer on the scroll surface, and arrow/page/home/end/space keys) records a manual position and opts out of live-follow.
MessagesTimelineacceptsinitialScrollOffsetand reportsonScrollOffsetChange; the list remount key isrouteThreadKeyso restoration applies on thread switches. Sending a message or using “Scroll to end” still jumps to the live edge and clears the preserved-response mode.Reviewed by Cursor Bugbot for commit a7cd406. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Restore thread scroll position and detect manual navigation in chat timeline
timelineScrollPositionByThreadKeymap in ChatView.tsx to persist each thread's scroll offset across route changes and restore it on return.resolveTimelineEntryScrollModeto compute whether the timeline should follow the end, restore a saved offset, or anchor to a turn response, based on running/latest turn state.initialScrollOffsetandonScrollOffsetChangeprops so the parent can seed and track scroll position.Macroscope summarized a7cd406.