feat(chat): virtualize message list with @tanstack/react-virtual#166
Conversation
Use @tanstack/react-virtual against the existing use-stick-to-bottom scroll container so both systems observe the same viewport. Rows are measured with the virtualizer's ResizeObserver-backed measureElement callback, with date dividers kept inside each measured row so markdown and divider height contribute to the row size. The stick-to-bottom contentRef now points at the virtualizer inner sizing div, preserving the total-height content measurement and overflow-anchor:none placement.
…ss harness profile
|
Your free trial PR review limit of 300 PRs has been reached. Please upgrade your plan to continue using CodeAnt AI. |
|
Warning Review limit reached
More reviews will be available in 15 minutes and 33 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (3)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Code Review
This pull request introduces message virtualization to the ChatView component using @tanstack/react-virtual to improve performance in large chat channels, along with corresponding DOM tests. Feedback on the changes suggests adding a unique key prop to the VirtualizedMessageList to force a clean remount when switching channels or DMs, and applying the flow-root class to the virtual row container to prevent margin collapsing from date dividers, which can lead to incorrect height measurements.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| <VirtualizedMessageList | ||
| messages={messages} | ||
| authUser={authUser} | ||
| activeChannelName={activeChannelName} | ||
| directMessageParticipants={directMessageParticipants} | ||
| activeThreadMessageId={activeThreadMessageId} | ||
| canInteractWithMessages={canInteractWithMessages} | ||
| scrollRef={scrollRef} | ||
| contentRef={contentRef} | ||
| scrollToBottom={scrollToBottom} | ||
| onReply={handleReplyToMessage} | ||
| onReact={handleReactToMessage} | ||
| /> |
There was a problem hiding this comment.
When switching between channels or DMs, the messages array changes, but the VirtualizedMessageList component is reused because it lacks a unique key prop. This causes the underlying @tanstack/react-virtual instance to persist its internal scroll offset, total size, and measurement cache across different rooms. This can lead to severe layout glitches (such as rendering empty space or overlapping messages) until a scroll event occurs.\n\nAdding a unique key based on the active room (channel or DM participants) forces React to unmount and remount the list, cleanly resetting the virtualizer's state and measurement cache for the new room.
<VirtualizedMessageList\n key={directMessageParticipants ? `dm:${directMessageParticipants.join('\\u0000')}` : `channel:${activeChannelName || 'all'}`}\n messages={messages}\n authUser={authUser}\n activeChannelName={activeChannelName}\n directMessageParticipants={directMessageParticipants}\n activeThreadMessageId={activeThreadMessageId}\n canInteractWithMessages={canInteractWithMessages}\n scrollRef={scrollRef}\n contentRef={contentRef}\n scrollToBottom={scrollToBottom}\n onReply={handleReplyToMessage}\n onReact={handleReactToMessage}\n />
| <div | ||
| key={virtualRow.key} | ||
| ref={messageVirtualizer.measureElement} | ||
| data-index={index} | ||
| data-testid="chat-virtual-row" | ||
| className="absolute left-0 top-0 w-full overflow-visible" | ||
| style={{ transform: `translateY(${virtualRow.start}px)` }} | ||
| > |
There was a problem hiding this comment.
The DateDivider component rendered inside the virtual row container uses vertical margins (my-4). Because the row container is absolutely positioned and has overflow-visible (to keep the reactions picker visible), these margins can collapse or protrude outside the row container's block boundaries. As a result, messageVirtualizer.measureElement may measure an incorrect height (excluding the margins), causing adjacent virtual rows to overlap or jump during scrolling.\n\nAdding the flow-root class establishes a new block formatting context (BFC) on the row container, which prevents margin collapsing and ensures that child margins are accurately included in the element's measured height.
<div\n key={virtualRow.key}\n ref={messageVirtualizer.measureElement}\n data-index={index}\n data-testid=\
There was a problem hiding this comment.
1 issue found across 4 files
You’re at about 95% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/renderer/src/components/chat/ChatView.tsx">
<violation number="1" location="src/renderer/src/components/chat/ChatView.tsx:179">
P2: The `DateDivider` component rendered inside these virtual rows uses `my-4` (vertical margins). The default `measureElement` implementation in `@tanstack/react-virtual` uses `getBoundingClientRect()` which does not include margins. This means rows containing a date divider will be measured shorter than their actual visual extent, causing adjacent virtual rows to overlap. Adding `flow-root` to this container's className (establishing a new block formatting context) will ensure child margins are included in the measured height, or alternatively convert `my-4` to `py-4` on the DateDivider.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| ref={messageVirtualizer.measureElement} | ||
| data-index={index} | ||
| data-testid="chat-virtual-row" | ||
| className="absolute left-0 top-0 w-full overflow-visible" |
There was a problem hiding this comment.
P2: The DateDivider component rendered inside these virtual rows uses my-4 (vertical margins). The default measureElement implementation in @tanstack/react-virtual uses getBoundingClientRect() which does not include margins. This means rows containing a date divider will be measured shorter than their actual visual extent, causing adjacent virtual rows to overlap. Adding flow-root to this container's className (establishing a new block formatting context) will ensure child margins are included in the measured height, or alternatively convert my-4 to py-4 on the DateDivider.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/renderer/src/components/chat/ChatView.tsx, line 179:
<comment>The `DateDivider` component rendered inside these virtual rows uses `my-4` (vertical margins). The default `measureElement` implementation in `@tanstack/react-virtual` uses `getBoundingClientRect()` which does not include margins. This means rows containing a date divider will be measured shorter than their actual visual extent, causing adjacent virtual rows to overlap. Adding `flow-root` to this container's className (establishing a new block formatting context) will ensure child margins are included in the measured height, or alternatively convert `my-4` to `py-4` on the DateDivider.</comment>
<file context>
@@ -102,6 +103,100 @@ function CountPill({
+ ref={messageVirtualizer.measureElement}
+ data-index={index}
+ data-testid="chat-virtual-row"
+ className="absolute left-0 top-0 w-full overflow-visible"
+ style={{ transform: `translateY(${virtualRow.start}px)` }}
+ >
</file context>
Resolves the 817ms longest-frame perf issue at 5k visible chat under 1000-agent load that PR #158's stress harness phase 3 surfaced. Original Superset survey called out chat-list virtualization as the deferred Tier 3 follow-up; this PR ships it.
What changed
729a7da— virtualizedChatViewmessage list with@tanstack/react-virtual.useStickToBottom'sscrollReffeeds the virtualizer'sgetScrollElement;contentRefis the sized inner div with[overflow-anchor:none].measureElement+data-indexon the row wrapper handles variable-height markdown content. Date dividers stay inline so their height contributes to row measurement.5e3cd0e— interaction preservation under virtualization: per-room-key scroll-to-bottom effect (won't fight new-message arrivals), ThreadPanel layout preserved, reactions picker stays visible without portal (rows keptoverflow-visible, pickerz-20clears next row in shared stacking context), active-thread highlight remeasure, empty-state bypass keeps virtualizer unmounted atmessages.length === 0.4ec28ba— regression test atsrc/renderer/src/components/chat/ChatView.dom.test.tsasserts>0 && <=15mounted rows after seeding 1000 messages (vs the previous 1000), plus a scroll-shifts-window test. Bonus: replaced the duplicate parent scrollToBottom effect with the ref-gated per-room-key one.Verdict
virt-review (claude) GO with no REWORK at any phase. Detailed per-phase audit captured in the team review thread.
Gates locally:
npx tsc --noEmit -p tsconfig.web.json— cleannpx vitest run— 218/218 pass (up from 214, the new dom test adds 4)npm run build— cleanNon-blocking follow-ups (per review)
space-y-0.5) is gone since rows are absolutely positioned. If the gap is desired, it should be added to the row's own padding.ChatView.tsx:134-135— theSTRESS_PROFILE=chat-heavyprofile in test-harness-web's Playwright harness isn't on main yet. Once that ships, the TODO marker should be replaced with the actual assertion that the FPS invariant now passes (it was previously gated asexpected-to-fail-until-virtualization).🤖 Generated with Claude Code