Add virtualization to chat thread for long conversations - #3781
Merged
Conversation
…shold Users report the app locking up in long threads with the experimental (ChatX) thread UI: unlike the legacy ConversationView (always-on VirtualizedList), it mounts every row and re-reconciles the whole transcript per streamed chunk, with content-visibility only bounding paint — not React or DOM cost. Following the upstream MessageScroller guidance (virtualization lives outside the primitive; add a virtualizer when the transcript needs it), the thread now ratchets once past 150 flat rows into a windowed body: the quill viewport stays the scroll element and @tanstack/react-virtual owns the rows inside ChatMessageScrollerContent. Below the threshold nothing changes, keeping the engine's anchor-scroll UX and native find/selection for typical sessions. - Agent turns flatten to one row per item so a single giant turn (autonomous sessions) can't defeat windowing; turn grouping survives via per-row flags, and the completed-turn timestamp rides the turn's last row. - Follow-bottom uses the proven VirtualizedList recipe (anchorTo end, followOnAppend, footer height as paddingEnd, totalSize re-pin, settle-at-end loop) plus the submit-from-anywhere recapture the non-virtualized ThreadAutoFollow provides. - Sticky header, jump picker, keyboard nav, and the scrollbar rail get windowed implementations (virtualizer measurements + scrollToIndex); the rail already interpolates unmounted rows. - Crossing the threshold mid-session resumes from the scroll state the non-virtualized body records, so the flip doesn't yank the viewport. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PZje6JV5QZEqmhFKrhLDZ7
|
😎 Merged directly without going through the merge queue, as the queue was empty and the PR was up to date with the target branch - details. |
|
React Doctor found 4 issues in 3 files · 4 warnings. 4 warnings
Reviewed by React Doctor for commit |
Replace the VirtualStickyHeader dismissal-clearing effect with a render-phase state adjustment, and collapse the footer measure effect to a single setState path. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PZje6JV5QZEqmhFKrhLDZ7
The virtualization PR grew ChatThread.tsx to ~1900 lines and react-doctor flagged the windowed body as a giant component pushing live state to its parent through a ref-filled effect. - Move the windowed body to VirtualThreadScrollBody.tsx, decomposed into useReservedFooterHeight / useSettleControls / useStickyAnchor / useFollowBottom. Row rendering comes in as a `renderRow` prop, so the row markup stays in ChatThread and there is no import cycle. - Move the sticky headers to ThreadStickyHeader.tsx and make the non-virtualized overlay clear its dismissal in render, like the windowed variant already did, instead of chaining through an effect. - Replace the jump-API ref + effect with a `renderNav` render prop: the nav layer is now composed by the parent but rendered beside the scroller, so the body hands it the jump implementation directly. - Fix the mid-session handoff. The two bodies do not share a scroll coordinate space (virtual offsets are 80px estimates until rows measure), so resuming from a recorded `scrollTop` landed anywhere, usually clamped to the bottom. Record the engine's anchored user message instead and resume with scrollToIndex. That also drops the recorder's scroll listener and DOM probe. - Add a dev-only Advanced setting overriding the virtualization threshold, so the windowed path and the mid-session flip can be tested without a 150-row conversation, plus a dev-only amber ring and "windowed mounted/total" badge on the windowed body — the two renderers are otherwise pixel-identical. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WejU6Qw29peK7Md1WH2JqE
…at-thread-virtualization-o5v13n # Conflicts: # packages/ui/src/features/sessions/components/chat-thread/ChatThread.tsx # packages/ui/src/features/sessions/components/scrollbar-rail/useMessageRailMarkers.ts
- Remove the dev-only windowed badge, amber ring, Advanced settings row and the persisted `chatThreadVirtualizationThreshold` store field. The threshold is a plain constant again. - Lower CHAT_THREAD_VIRTUALIZATION_THRESHOLD from 150 to 50 and document that it counts flat rows, not turns or messages — an exchange is several rows (~5 on a tool-heavy thread), so 50 rows is well under 50 turns. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WejU6Qw29peK7Md1WH2JqE
… data Prompts-per-task over 30 days (9,458 tasks): p50 3, p90 17, p99 72, max 2,007. At ~5 flat rows per exchange, 250 rows is ~50 prompts — the top 1.6% of threads, carrying 23% of all prompting. Lower thresholds windowed a large share of ordinary conversations for no measured gain: at 51 rows both renderers hold p50 8.3ms with one dropped frame in 300. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WejU6Qw29peK7Md1WH2JqE
k11kirky
marked this pull request as ready for review
July 26, 2026 09:34
Contributor
Prompt To Fix All With AIFix the following 1 code review issue. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 1
packages/ui/src/features/sessions/components/chat-thread/VirtualThreadScrollBody.tsx:200-209
**Sticky state misses remeasurement updates**
When asynchronous markdown, diff, or tool content changes a mounted row's height without a scroll or row-set update, the virtualizer's measurement cache changes but `useStickyAnchor` does not schedule a recomputation, causing the sticky pill to retain the wrong turn or visibility state until the user scrolls again.
Reviews (1): Last reviewed commit: "fix(chat-thread): set virtualization thr..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+200
to
+209
| // Re-derive on mount and whenever the row set changes (content growth moves every anchor below). | ||
| useEffect(() => { | ||
| schedule(); | ||
| return () => { | ||
| if (frameRef.current != null) { | ||
| cancelAnimationFrame(frameRef.current); | ||
| frameRef.current = null; | ||
| } | ||
| }; | ||
| }, [schedule]); |
Contributor
There was a problem hiding this comment.
When asynchronous markdown, diff, or tool content changes a mounted row's height without a scroll or row-set update, the virtualizer's measurement cache changes but useStickyAnchor does not schedule a recomputation, causing the sticky pill to retain the wrong turn or visibility state until the user scrolls again.
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/ui/src/features/sessions/components/chat-thread/VirtualThreadScrollBody.tsx
Line: 200-209
Comment:
**Sticky state misses remeasurement updates**
When asynchronous markdown, diff, or tool content changes a mounted row's height without a scroll or row-set update, the virtualizer's measurement cache changes but `useStickyAnchor` does not schedule a recomputation, causing the sticky pill to retain the wrong turn or visibility state until the user scrolls again.
How can I resolve this? If you propose a fix, please make it concise.Async markdown/diff/tool content can grow a mounted row with no scroll and no row-set change; the measurement cache moves but the sticky pill held its stale anchor until the next scroll. totalSize is the cheapest proxy for the cache moving, so use it as a recompute trigger. Flagged by Greptile on #3781. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PZje6JV5QZEqmhFKrhLDZ7
k11kirky
approved these changes
Jul 26, 2026
Contributor
Author
|
/trunk merge |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The experimental chat thread mounts every row. On a long thread that means an unbounded DOM plus a full-thread reconcile per streamed chunk.
content-visibility: autokeeps paint cheap but doesn't bound the mounted set.Changes
Windowed rendering past a row threshold, using
@tanstack/react-virtual— following the upstream MessageScroller guidance: the quill viewport stays the scroll element, virtualization lives outside the primitive.threadVirtualization.ts— pure helpers, unit-tested:flattenTurnRows,countFlatRows,completedTurnTimestamp,computeStickyAnchor.VirtualThreadScrollBody.tsx— the windowed body, split intouseReservedFooterHeight/useSettleControls/useStickyAnchor/useFollowBottom. Follow-bottom viaanchorTo: "end"+followOnAppend; jumps viascrollToIndexwith multi-frame settling; the footer is reserved aspaddingEndrather than a virtual row (a constant last key would permanently killfollowOnAppend).ThreadStickyHeader.tsx— both sticky-header variants. The windowed one can't use the engine's visibility state (its IntersectionObserver only sees mounted rows), so the anchor is derived from the virtualizer's measurement cache instead.scrollTop: the two bodies don't share a coordinate space, since virtual offsets are estimates until rows measure.Why 250
CHAT_THREAD_VIRTUALIZATION_THRESHOLDcounts flat rows, not turns or messages — a user message is one row, and an agent turn contributes one row per item (each prose block, each tool group, each git action). Measured at roughly 5 rows per exchange on a tool-heavy thread.Prompts per task, last 30 days, 9,458 tasks:
Long threads are rare but carry disproportionate use — tasks over 20 prompts are 7.5% of tasks and 47% of all prompting:
At ~5 rows per exchange, 250 rows ≈ 50 prompts — the top 1.6% of threads, carrying 23% of prompting. Lower thresholds windowed a large share of ordinary conversations for no measured gain (see below), and upstream documents the non-virtualized path as comfortable into the low thousands of turns. Windowing is for the tail that actually degrades, not the median thread.
How did you test this?
Unit tests for the pure helpers. Beyond that, driven against the real app over CDP on a 51-row thread with the threshold temporarily lowered:
computeStickyAnchorby hand at 0 / 900 / 2000 / 5000px; clicking the pill lands on that row's exact offset.Measured perf, 120px/frame over 300 frames — windowed vs full on the same thread:
Being explicit about the limits: at 51 rows virtualization buys nothing on scroll smoothness — the measured win is DOM node count and a mounted set that stays flat as the thread grows. The behaviour at 250+ rows, and the mid-session handoff, are not benchmarked; there is no thread-size or frame-timing instrumentation in the app to measure them with. Adding it (flat row count + long-frame count on an existing event) would turn the threshold from an inference into a measurement.
Blast radius is limited: below 250 rows nothing changes, and the ratchet means a bad flip costs a scroll position, not a broken thread.
Automatic notifications
https://claude.ai/code/session_01WejU6Qw29peK7Md1WH2JqE