Migrate chat scrolling and branch lists to LegendList#1953
Migrate chat scrolling and branch lists to LegendList#1953juliusmarminge wants to merge 19 commits intomainfrom
Conversation
- Replace custom chat auto-scroll and branch virtualization with LegendList - Remove deprecated chat scroll helpers and related tests - Add LegendList dependency for list rendering
|
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 Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
- Reset the branch list correctly for virtualized and non-virtualized views - Preserve formatting cleanup in web package and timeline logic
ApprovabilityVerdict: Needs human review Major refactor that replaces the virtualization library and fundamentally changes chat scroll handling logic. The changes affect core chat UX behavior and there is an unresolved review comment identifying a potential bug in the structural sharing optimization where row reordering may not be detected correctly. You can customize Macroscope's approvability policy. Learn more. |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Autofix Details
Bugbot Autofix prepared a fix for 1 of the 2 issues found in the latest run.
- ✅ Fixed: Unused
scrollToEnddependency in callback, refs accessed directly- Replaced direct
legendListRef.current?.scrollToEnd?.({ animated: true })calls in bothonSendandonSubmitPlanFollowUpwithscrollToEnd(true)to use the wrapper consistently.
- Replaced direct
Or push these changes by commenting:
@cursor push b3c9d5931b
Preview (b3c9d5931b)
diff --git a/apps/web/src/components/ChatView.tsx b/apps/web/src/components/ChatView.tsx
--- a/apps/web/src/components/ChatView.tsx
+++ b/apps/web/src/components/ChatView.tsx
@@ -2435,7 +2435,7 @@
// Sending a message should always bring the latest user turn into view.
isAtEndRef.current = true;
requestAnimationFrame(() => {
- legendListRef.current?.scrollToEnd?.({ animated: true });
+ scrollToEnd(true);
setShowScrollToBottom(false);
});
@@ -2830,7 +2830,7 @@
]);
isAtEndRef.current = true;
requestAnimationFrame(() => {
- legendListRef.current?.scrollToEnd?.({ animated: true });
+ scrollToEnd(true);
setShowScrollToBottom(false);
});You can send follow-ups to the cloud agent here.
- Drop browser harness measurements for timeline height parity - Delete the unused timelineHeight helper and its unit tests
- Stop forcing LegendList to scroll to the end on thread mount - Rely on existing scroll state handling instead
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 3 total unresolved issues (including 2 from previous reviews).
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Uncancelled
requestAnimationFramein send message handlers- Added a
pendingSendScrollFrameRefto track RAF IDs from both send handlers, cancel any pending frame before scheduling a new one, and cancel on component unmount via a cleanup effect.
- Added a
Or push these changes by commenting:
@cursor push 546899ceba
Preview (546899ceba)
diff --git a/apps/web/src/components/ChatView.tsx b/apps/web/src/components/ChatView.tsx
--- a/apps/web/src/components/ChatView.tsx
+++ b/apps/web/src/components/ChatView.tsx
@@ -700,6 +700,7 @@
const attachmentPreviewHandoffByMessageIdRef = useRef<Record<string, string[]>>({});
const attachmentPreviewPromotionInFlightByMessageIdRef = useRef<Record<string, true>>({});
const sendInFlightRef = useRef(false);
+ const pendingSendScrollFrameRef = useRef<number | null>(null);
const terminalOpenByThreadRef = useRef<Record<string, boolean>>({});
const terminalState = useTerminalStateStore((state) =>
@@ -1176,6 +1177,13 @@
}
};
}, [clearAttachmentPreviewHandoffs]);
+ useEffect(() => {
+ return () => {
+ if (pendingSendScrollFrameRef.current != null) {
+ cancelAnimationFrame(pendingSendScrollFrameRef.current);
+ }
+ };
+ }, []);
const handoffAttachmentPreviews = useCallback((messageId: MessageId, previewUrls: string[]) => {
if (previewUrls.length === 0) return;
@@ -2423,7 +2431,11 @@
]);
// Sending a message should always bring the latest user turn into view.
isAtEndRef.current = true;
- requestAnimationFrame(() => {
+ if (pendingSendScrollFrameRef.current != null) {
+ cancelAnimationFrame(pendingSendScrollFrameRef.current);
+ }
+ pendingSendScrollFrameRef.current = requestAnimationFrame(() => {
+ pendingSendScrollFrameRef.current = null;
legendListRef.current?.scrollToEnd?.({ animated: true });
setShowScrollToBottom(false);
});
@@ -2818,7 +2830,11 @@
},
]);
isAtEndRef.current = true;
- requestAnimationFrame(() => {
+ if (pendingSendScrollFrameRef.current != null) {
+ cancelAnimationFrame(pendingSendScrollFrameRef.current);
+ }
+ pendingSendScrollFrameRef.current = requestAnimationFrame(() => {
+ pendingSendScrollFrameRef.current = null;
legendListRef.current?.scrollToEnd?.({ animated: true });
setShowScrollToBottom(false);
});You can send follow-ups to the cloud agent here.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 2 total unresolved issues (including 1 from previous review).
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Infinite scroll listener inactive for virtualized branch list
- Guarded the branches.length useEffect to skip when shouldVirtualizeBranchList is true, since branchListScrollElementRef is null in that path and the LegendList onEndReached callback already handles infinite loading for the virtualized case.
Or push these changes by commenting:
@cursor push 6f4824801a
Preview (6f4824801a)
diff --git a/apps/web/src/components/BranchToolbarBranchSelector.tsx b/apps/web/src/components/BranchToolbarBranchSelector.tsx
--- a/apps/web/src/components/BranchToolbarBranchSelector.tsx
+++ b/apps/web/src/components/BranchToolbarBranchSelector.tsx
@@ -460,8 +460,9 @@
}, [isBranchMenuOpen, maybeFetchNextBranchPage]);
useEffect(() => {
+ if (shouldVirtualizeBranchList) return;
maybeFetchNextBranchPage();
- }, [branches.length, maybeFetchNextBranchPage]);
+ }, [branches.length, maybeFetchNextBranchPage, shouldVirtualizeBranchList]);
const triggerLabel = getBranchTriggerLabel({
activeWorktreePath,You can send follow-ups to the cloud agent here.
When shouldVirtualizeBranchList is true, branchListScrollElementRef stays null because setBranchListRef is only attached to the non-virtualized ComboboxList. The branches.length effect would call maybeFetchNextBranchPage which bails out immediately due to the null scroll element. Guard the effect to skip when virtualized, since the LegendList onEndReached callback already handles infinite loading in that path, and with 40+ items the visible area is always filled. Applied via @cursor push command
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Scroll-to-bottom button not reset on thread switch
- Added
isAtEndRef.current = trueandsetShowScrollToBottom(false)to the existinguseEffectkeyed onactiveThread?.idso scroll state is properly reset when switching threads.
- Added
Or push these changes by commenting:
@cursor push 07fd60cc17
Preview (07fd60cc17)
diff --git a/apps/web/src/components/ChatView.tsx b/apps/web/src/components/ChatView.tsx
--- a/apps/web/src/components/ChatView.tsx
+++ b/apps/web/src/components/ChatView.tsx
@@ -1968,6 +1968,8 @@
useEffect(() => {
setExpandedWorkGroups({});
setPullRequestDialogState(null);
+ isAtEndRef.current = true;
+ setShowScrollToBottom(false);
if (planSidebarOpenOnNextThreadRef.current) {
planSidebarOpenOnNextThreadRef.current = false;
setPlanSidebarOpen(true);You can send follow-ups to the cloud agent here.
Reset isAtEndRef and showScrollToBottom when activeThread changes so stale scroll state from a previous thread does not persist into the new one. Applied via @cursor push command
- Scroll LegendList to the end without animation when sending messages - Avoid mid-flight layout changes from landing the view at the wrong position
- Move timeline row state into local/context-driven components to reduce list-wide rerenders - Fix scroll-to-end behavior when sending messages so LegendList stays pinned correctly - Add live ticking timers for working and streaming message metadata
- Drop the react-scan auto script from the web entry HTML - Keep the app shell leaner for production
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Stable-rows fallback returns previous iteration order
- Removed the stale fallback that returned Array.from(prev.values()) with old insertion order, and now always return the result array which has the correct current order and stable references.
Or push these changes by commenting:
@cursor push 5aa1d999df
Preview (5aa1d999df)
diff --git a/apps/web/src/components/chat/MessagesTimeline.tsx b/apps/web/src/components/chat/MessagesTimeline.tsx
--- a/apps/web/src/components/chat/MessagesTimeline.tsx
+++ b/apps/web/src/components/chat/MessagesTimeline.tsx
@@ -230,10 +230,7 @@
// from TimelineRowCtx, which propagates through LegendList's memo.
const renderItem = useCallback(
({ item }: { item: MessagesTimelineRow }) => (
- <div
- className="mx-auto w-full min-w-0 max-w-3xl overflow-x-hidden"
- data-timeline-root="true"
- >
+ <div className="mx-auto w-full min-w-0 max-w-3xl overflow-x-hidden" data-timeline-root="true">
<TimelineRowContent row={item} />
</div>
),
@@ -402,9 +399,7 @@
<div className="my-3 flex items-center gap-3">
<span className="h-px flex-1 bg-border" />
<span className="rounded-full border border-border bg-background px-2.5 py-1 text-[10px] uppercase tracking-[0.14em] text-muted-foreground/80">
- {ctx.completionSummary
- ? `Response • ${ctx.completionSummary}`
- : "Response"}
+ {ctx.completionSummary ? `Response • ${ctx.completionSummary}` : "Response"}
</span>
<span className="h-px flex-1 bg-border" />
</div>
@@ -794,7 +789,6 @@
return useMemo(() => {
const prev = prevById.current;
const next = new Map<string, MessagesTimelineRow>();
- let anyChanged = false;
const result = rows.map((row) => {
const prevRow = prev.get(row.id);
@@ -803,13 +797,11 @@
return prevRow;
}
next.set(row.id, row);
- anyChanged = true;
return row;
});
prevById.current = next;
- // If nothing changed and length matches, reuse the previous array reference
- return anyChanged || rows.length !== prev.size ? result : Array.from(prev.values());
+ return result;
}, [rows]);
}You can send follow-ups to the cloud agent here.
- Move LegendList scroll-to-end to commit-time layout effect - Debounce showing the scroll-to-bottom pill during thread switches
- Pin LegendList to the current end before appending optimistic turns - Simplify timeline row stabilization to always return the rebuilt array
- Keep MessagesTimeline row and response label JSX compact - No behavioral change
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Unused
anyChangedvariable defeats array stability optimization- Added a
prevResultref and used theanyChangedflag to return the previous array reference when no rows changed, preserving referential identity for LegendList's data prop.
- Added a
Or push these changes by commenting:
@cursor push cb46db8887
Preview (cb46db8887)
diff --git a/apps/web/src/components/chat/MessagesTimeline.tsx b/apps/web/src/components/chat/MessagesTimeline.tsx
--- a/apps/web/src/components/chat/MessagesTimeline.tsx
+++ b/apps/web/src/components/chat/MessagesTimeline.tsx
@@ -785,11 +785,12 @@
* hasn't changed since last call, the previous object reference is reused. */
function useStableRows(rows: MessagesTimelineRow[]): MessagesTimelineRow[] {
const prevById = useRef(new Map<string, MessagesTimelineRow>());
+ const prevResult = useRef<MessagesTimelineRow[]>([]);
return useMemo(() => {
const prev = prevById.current;
const next = new Map<string, MessagesTimelineRow>();
- let anyChanged = false;
+ let anyChanged = rows.length !== prev.size;
const result = rows.map((row) => {
const prevRow = prev.get(row.id);
@@ -803,6 +804,9 @@
});
prevById.current = next;
+
+ if (!anyChanged) return prevResult.current;
+ prevResult.current = result;
return result;
}, [rows]);
}You can send follow-ups to the cloud agent here.
…eStableRows The anyChanged variable was assigned but never read, causing a new array reference to always be returned from useMemo even when all rows were structurally identical. This partially defeated the purpose of the structural sharing hook for LegendList's data prop. Now we track the previous result and return it when no rows changed, preserving referential identity and avoiding unnecessary list reconciliation. Applied via @cursor push command
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Unused
groupIdprop inWorkGroupSection- Removed the unused
groupIdprop from theWorkGroupSectioncomponent interface, destructuring, and call site.
- Removed the unused
Or push these changes by commenting:
@cursor push 45227f2132
Preview (45227f2132)
diff --git a/apps/web/src/components/chat/MessagesTimeline.tsx b/apps/web/src/components/chat/MessagesTimeline.tsx
--- a/apps/web/src/components/chat/MessagesTimeline.tsx
+++ b/apps/web/src/components/chat/MessagesTimeline.tsx
@@ -296,9 +296,7 @@
data-message-id={row.kind === "message" ? row.message.id : undefined}
data-message-role={row.kind === "message" ? row.message.role : undefined}
>
- {row.kind === "work" && (
- <WorkGroupSection groupId={row.id} groupedEntries={row.groupedEntries} />
- )}
+ {row.kind === "work" && <WorkGroupSection groupedEntries={row.groupedEntries} />}
{row.kind === "message" &&
row.message.role === "user" &&
@@ -528,10 +526,8 @@
/** Owns its own expand/collapse state so toggling re-renders only this row.
* State resets on unmount which is fine — work groups start collapsed. */
const WorkGroupSection = memo(function WorkGroupSection({
- groupId,
groupedEntries,
}: {
- groupId: string;
groupedEntries: Extract<MessagesTimelineRow, { kind: "work" }>["groupedEntries"];
}) {
const [isExpanded, setIsExpanded] = useState(false);You can send follow-ups to the cloud agent here.
Applied via @cursor push command
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Ref-backed Maps won't trigger row re-renders for changed files
- Added a monotonic mapsRevision counter that increments when the turnDiffSummaryByAssistantMessageId or revertTurnCountByUserMessageId Map references change, and included it in the sharedState useMemo deps so the context identity updates and triggers re-renders through LegendList's memo boundaries.
Or push these changes by commenting:
@cursor push 05e58a0ce9
Preview (05e58a0ce9)
diff --git a/apps/web/src/components/chat/MessagesTimeline.tsx b/apps/web/src/components/chat/MessagesTimeline.tsx
--- a/apps/web/src/components/chat/MessagesTimeline.tsx
+++ b/apps/web/src/components/chat/MessagesTimeline.tsx
@@ -83,6 +83,9 @@
timestampFormat: TimestampFormat;
/** Stable getter — returns the current Maps from a ref. */
getMaps: () => TimelineRowMaps;
+ /** Monotonic counter — increments when the backing Maps change so
+ * context consumers re-render and read fresh values via getMaps(). */
+ mapsRevision: number;
routeThreadKey: string;
markdownCwd: string | undefined;
resolvedTheme: "light" | "dark";
@@ -175,8 +178,9 @@
}
}, [listRef, onIsAtEndChange]);
- // Volatile Maps go into a ref so they never bust the context identity.
- // Components read them via getMaps() during their own render pass.
+ // Maps go into a ref so getMaps() is a stable function reference.
+ // A revision counter is included in the context value so consumers
+ // re-render when the backing Maps change.
const mapsRef = useRef<TimelineRowMaps>({
turnDiffSummaryByAssistantMessageId,
revertTurnCountByUserMessageId,
@@ -186,6 +190,18 @@
revertTurnCountByUserMessageId,
};
const getMaps = useCallback(() => mapsRef.current, []);
+ const mapsRevisionRef = useRef(0);
+ const prevDiffMapRef = useRef(turnDiffSummaryByAssistantMessageId);
+ const prevRevertMapRef = useRef(revertTurnCountByUserMessageId);
+ if (
+ prevDiffMapRef.current !== turnDiffSummaryByAssistantMessageId ||
+ prevRevertMapRef.current !== revertTurnCountByUserMessageId
+ ) {
+ prevDiffMapRef.current = turnDiffSummaryByAssistantMessageId;
+ prevRevertMapRef.current = revertTurnCountByUserMessageId;
+ mapsRevisionRef.current += 1;
+ }
+ const mapsRevision = mapsRevisionRef.current;
// Memoised context value — only changes on state transitions, NOT on
// every streaming chunk. Callbacks from ChatView are useCallback-stable.
@@ -198,6 +214,7 @@
completionSummary,
timestampFormat,
getMaps,
+ mapsRevision,
routeThreadKey,
markdownCwd,
resolvedTheme,
@@ -215,6 +232,7 @@
completionSummary,
timestampFormat,
getMaps,
+ mapsRevision,
routeThreadKey,
markdownCwd,
resolvedTheme,You can send follow-ups to the cloud agent here.
- Replace the custom row context hook with React `use` - Keep timeline row and changed-files rendering on the same shared context
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 2 total unresolved issues (including 1 from previous review).
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Scroll-to-bottom pill may flash or misbehave on thread switch
- Added a useEffect cleanup that cancels the debouncer on component unmount, preventing stale setShowScrollToBottom(true) calls after the component is no longer mounted.
Or push these changes by commenting:
@cursor push 5082fcb421
Preview (5082fcb421)
diff --git a/apps/web/src/components/ChatView.tsx b/apps/web/src/components/ChatView.tsx
--- a/apps/web/src/components/ChatView.tsx
+++ b/apps/web/src/components/ChatView.tsx
@@ -1969,7 +1969,13 @@
planSidebarDismissedForTurnRef.current = null;
}, [activeThread?.id]);
+ // Cancel any pending debouncer timeout on unmount to avoid stale state updates.
useEffect(() => {
+ const debouncer = showScrollDebouncer.current;
+ return () => debouncer.cancel();
+ }, []);
+
+ useEffect(() => {
setIsRevertingCheckpoint(false);
}, [activeThread?.id]);You can send follow-ups to the cloud agent here.
- Pass assistant diff summaries and user revert counts into row data - Remove ref-backed map lookups from row rendering - Add coverage for summary and revert-count projection
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Structural sharing returns stale array on row reorder
- Added an element-wise identity check against the previous result array so that row reordering (same ids/content, different positions) is detected and returns the correctly ordered array.
Or push these changes by commenting:
@cursor push bfc761195e
Preview (bfc761195e)
diff --git a/apps/web/src/components/chat/MessagesTimeline.tsx b/apps/web/src/components/chat/MessagesTimeline.tsx
--- a/apps/web/src/components/chat/MessagesTimeline.tsx
+++ b/apps/web/src/components/chat/MessagesTimeline.tsx
@@ -779,6 +779,16 @@
prevById.current = next;
+ if (!anyChanged) {
+ const prevRes = prevResult.current;
+ for (let i = 0; i < result.length; i++) {
+ if (result[i] !== prevRes[i]) {
+ anyChanged = true;
+ break;
+ }
+ }
+ }
+
if (!anyChanged) return prevResult.current;
prevResult.current = result;
return result;You can send follow-ups to the cloud agent here.
Reviewed by Cursor Bugbot for commit eb77432. Configure here.
| if (!anyChanged) return prevResult.current; | ||
| prevResult.current = result; | ||
| return result; | ||
| }, [rows]); |
There was a problem hiding this comment.
Structural sharing returns stale array on row reorder
Low Severity
useStableRows returns the previous array reference (prevResult.current) when anyChanged is false, but anyChanged only tracks content mutations and length differences — not positional changes. If rows are reordered (same ids, same content, same count) the function returns the stale array in the old order instead of the newly built result array. The anyChanged flag at line 767 compares rows.length !== prev.size and is only set inside the loop when content differs, missing the reorder case entirely.
Reviewed by Cursor Bugbot for commit eb77432. Configure here.
- Ignore non-keyboard highlight events when the branch menu is open - Add a regression test for the preselected worktree branch picker



Summary
@legendapp/list.ChatViewscroll state by delegating stick-to-bottom behavior toLegendList.tanstack/react-virtualchat scroll helpers and related tests.Testing
bun fmt,bun lint,bun typecheck, andbun run test.Note
Medium Risk
Replaces core scrolling/virtualization behavior in the chat timeline and branch selector with a new (beta) list library, which can subtly change auto-scroll, anchoring, and rendering behavior. Also removes multiple regression/measurement tests, reducing coverage for scroll/virtualization edge cases.
Overview
Switches the chat timeline and branch picker dropdown from
@tanstack/react-virtualto@legendapp/list(LegendList), including new combobox plumbing (ComboboxListVirtualized) and updated keyboard scroll-into-view behavior.Chat scrolling is simplified by delegating stick-to-bottom and virtualization to
LegendList(maintainScrollAtEnd), replacing bespoke scroll listeners/threshold logic; the scroll-to-bottom pill is now driven byisAtEndwith a small debounce to avoid flashing during thread switches.Timeline row rendering is refactored to be list-friendly: shared props move into a context, work-group expand/collapse becomes per-row local state, changed-files expand state is read/written via the UI store keyed by
routeThreadKey, and live “working/elapsed” timestamps are handled by self-ticking row components. Obsolete scroll/height-estimation utilities and related tests/harnesses are removed, and a new browser regression test covers branch picker top-anchoring when opening with a preselected branch.Reviewed by Cursor Bugbot for commit ae4c4c7. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Migrate chat timeline and branch list scrolling to LegendList virtualization
@tanstack/react-virtualwith@legendapp/listin both MessagesTimeline.tsx and BranchToolbarBranchSelector.tsx, removing all manual virtualizer measurement and scroll adjustment logic.onIsAtEndChange; the scroll-to-bottom pill visibility is debounced 150ms when scrolling away from the end and hidden immediately when returning to it.WorkingTimerandLiveMessageMetaare self-ticking components that update every second, removing the need for the externalnowIsoprop.deriveMessagesTimelineRowsnow acceptsturnDiffSummaryByAssistantMessageIdandrevertTurnCountByUserMessageIdmaps and projects those values onto returned rows;estimateMessagesTimelineRowHeightis removed.estimateMessagesTimelineRowHeightis deleted — any remaining imports will break at build time.Macroscope summarized ae4c4c7.