perf(mothership): virtualize chat transcript and isolate input from stream re-renders#5019
Conversation
…tream re-renders Long chats rendered every message into the DOM with no windowing — a custom rAF "progressive list" only smeared the mount cost across frames without capping it. At ~1000 messages this was 52k DOM nodes and a 21s main-thread block on open, and the input toolbar re-rendered on every streamed token. - Virtualize the message list with @tanstack/react-virtual using dynamic measureElement, stable per-row keys, and a tuned size estimate. Only the visible window mounts, so load cost is now flat regardless of transcript length. Remove the now-redundant useProgressiveList hook. - Memoize UserInput and stabilize its callbacks (useCallback in MothershipChat and home) so streaming ticks no longer re-render the entire input toolbar. - Keep the existing useAutoScroll for streaming stick-to-bottom (it reads the virtualizer's real scrollHeight) and add a per-chat scrollToIndex for initial positioning before paint. Measured on a cloned 1032-message chat: time-to-rendered 26.3s -> 1.7s, main-thread blocked 21.4s -> 0.8s, DOM nodes 52k -> 1.4k, typing-while- streaming p-max 104ms -> 26ms. Adds scripts/perf/ harness used to validate.
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
PR SummaryMedium Risk Overview Initial scroll is reworked: per-chat
Reviewed by Cursor Bugbot for commit 73bea30. Configure here. |
|
@greptile review |
|
@cursor review |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Greptile SummaryThis PR replaces rAF-batched progressive rendering with
Confidence Score: 5/5Safe to merge. The scroll guards, virtualizer key scheme, and memo stabilisation are all correct; streaming tail-follow relies on the unchanged useAutoScroll, which correctly reads the virtualizer-driven scrollHeight. All changed paths — virtualizer setup, initial-scroll sentinel logic, pending→persisted chat id transition, rangeExtractor last-row pin, and UserInput memo — were traced through their edge cases and behave as intended. The useAutoScroll MutationObserver continues to observe the correct scroll element and detects both characterData updates and childList mutations. No incorrect state transitions, stale closures, or missing guards were found. No files require special attention. Live-stream follow-through on staging is the remaining validation gap, but it rests on the unchanged useAutoScroll logic. Important Files Changed
Sequence DiagramsequenceDiagram
participant Home as home.tsx
participant MC as MothershipChat
participant VZ as useVirtualizer
participant AS as useAutoScroll
participant DOM as ScrollElement
Home->>MC: messages[], chatId, isSending
MC->>AS: useAutoScroll(isStreamActive)
AS-->>MC: autoScrollRef
MC->>VZ: useVirtualizer(count, estimateSize, rangeExtractor)
VZ-->>MC: virtualizer instance
MC->>DOM: attach via setScrollElement callback ref
DOM-->>AS: "containerRef.current = el"
DOM-->>VZ: getScrollElement returns el
MC->>VZ: useLayoutEffect scrollToIndex(lastIndex, end)
VZ->>DOM: "scrollTop = estimated offset of last item"
loop Streaming token arrives
AS->>DOM: MutationObserver fires on characterData
AS->>DOM: "if sticky then scrollTop = scrollHeight"
end
loop User scrolls up
AS->>AS: detach sticky
VZ->>DOM: unmount out-of-view rows
VZ->>DOM: keep lastIndex mounted via rangeExtractor
end
Reviews (7): Last reviewed commit: "fix(mothership): don't re-scroll when a ..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Greptile SummaryThis PR replaces the rAF-based progressive rendering hook with
Confidence Score: 4/5The core virtualization and memoization changes are well-structured and the streaming auto-scroll path is preserved through the existing useAutoScroll hook. The main outstanding risk is streaming scroll follow, which the author explicitly flags as needing staging confirmation since the backend was unreachable locally. The virtualization implementation follows TanStack Virtual's recommended chat pattern, key stability is carefully handled for streaming placeholders, and callback stabilization in home.tsx is correct (getCurrentRequestId and stopGeneration are both stable useCallbacks). The dev scripts have a SQL injection pattern and a hardcoded email that would break the tool for other developers, but these don't affect the production runtime. The one runtime concern — empty AssistantMessageRow still occupying rowGap height in the virtual wrapper — is an edge case cosmetic issue. Streaming scroll follow is the highest-risk untested path. The streaming scroll interaction between useAutoScroll's MutationObserver and the virtualizer's ResizeObserver-driven sizer height updates in mothership-chat.tsx should get close attention on the staging deploy. The perf scripts in scripts/perf/ need the hardcoded email fixed before other team members can use them.
|
| Filename | Overview |
|---|---|
| apps/sim/app/workspace/[workspaceId]/home/components/mothership-chat/mothership-chat.tsx | Core change: replaces progressive-list rendering with TanStack useVirtualizer (dynamic measureElement, overscan: 6, stable getItemKey). Scroll setup is split between a callback ref (for useAutoScroll) and scrollElementRef (for the virtualizer's getScrollElement). Initial positioning is via virtualizer.scrollToIndex in useLayoutEffect, keyed on chatId. One minor issue: AssistantMessageRow returning null still occupies the pb-6 wrapper height inside each virtual item. |
| apps/sim/app/workspace/[workspaceId]/home/components/user-input/user-input.tsx | Wrapped with memo() to prevent re-renders on every streaming tick. The internal implementation is unchanged; the named export now points to the memoized wrapper. |
| apps/sim/app/workspace/[workspaceId]/home/home.tsx | Converted handleStopGeneration and handleSubmit to stable useCallback references. Both getCurrentRequestId and stopGeneration are stable (the former is a useCallback([], []) in useChat), so handleStopGeneration is genuinely stable across re-renders. |
| apps/sim/hooks/use-progressive-list.ts | File deleted. The rAF-based progressive rendering hook is superseded by the virtualizer which caps DOM nodes at the visible window rather than just spreading mount cost across frames. |
| scripts/perf/chat-load-perf.mjs | New headless-Chromium performance harness. Hardcodes the author's email as the default (waleed@sim.ai), making it non-functional for other developers without --email. Both the email and chatId parameters are directly interpolated into SQL strings passed to psql, creating an injection pattern. |
| scripts/perf/seed-chat-scale.mjs | Seeds synthetic chats by cycling messages from a source chat. SOURCE chatId is interpolated directly into the SQL. The SQL logic itself is correct; cleanup instruction is documented. |
| scripts/perf/stream-validate.mjs | Streaming correctness probe that asserts monotonic growth and scroll-pinning. Same hardcoded email default and SQL injection pattern as chat-load-perf.mjs. |
Sequence Diagram
sequenceDiagram
participant App as MothershipChat
participant VR as useVirtualizer
participant AS as useAutoScroll
participant DOM as ScrollContainer (DOM)
App->>DOM: "setScrollElement(el) — sets both scrollElementRef & autoScrollRef"
App->>VR: getScrollElement() → scrollElementRef.current
Note over App,VR: useLayoutEffect fires once per chatId
App->>VR: "scrollToIndex(last, {align:'end'})"
VR->>DOM: "el.scrollTop = estimatedBottom"
loop Streaming tokens arrive
App->>DOM: React renders updated AssistantMessageRow text
DOM-->>AS: MutationObserver (characterData) fires
AS->>DOM: "el.scrollTop = el.scrollHeight (sticky scroll)"
DOM-->>VR: ResizeObserver (item height changed)
VR->>DOM: updates sizer style.height (getTotalSize)
end
Note over App,AS: stream ends → useEffect cleanup
AS->>DOM: final scrollToBottom() at true scrollHeight
Comments Outside Diff (3)
-
scripts/perf/chat-load-perf.mjs, line 631-637 (link)SQL injection in psql query
The
EMAILvalue (from--emailCLI arg) is directly interpolated into the SQL string passed topsql. A value like' OR '1'='1would alter the query logic.seed-chat-scale.mjshas the same pattern with theSOURCEchatId. Even for a local dev tool that talks to a local database, parameterized queries prevent accidental breakage and keep the pattern safe:psqlsupports positional params via the-vflag, or you can use a separate prepare/execute approach. The same issue exists instream-validate.mjsaround the equivalentmintSessionCookiequery. -
scripts/perf/chat-load-perf.mjs, line 611 (link)Hardcoded author email bakes in a single-user default
waleed@sim.aiis the PR author's personal email. Any other team member running this script without passing--emailwill get "No live session for waleed@sim.ai" and the script will exit with an error. Consider removing the default entirely (let it throw a clear usage error), or document that--emailis required. The same default appears instream-validate.mjs.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
-
apps/sim/app/workspace/[workspaceId]/home/components/mothership-chat/mothership-chat.tsx, line 340-375 (link)AssistantMessageRownow occupiespb-6/pb-4vertical spaceAssistantMessageRowreturnsnullwhen an assistant message has no renderable content and is not streaming. Previously that collapsed to zero height. Now every virtual item, including those returningnull, lives inside a wrapper div withstyles.rowGap(pb-6= 24 px in mothership-view), so such messages leave a visible blank gap in the virtual list. This is an edge case (a finalised assistant turn with no content), but any such messages in existing chats will produce unexpected whitespace. A small guard — checking renderable content before rendering — or moving thenull-return logic up to the virtual item's own render and conditionally omitting the wrapper would fix it.
Reviews (2): Last reviewed commit: "perf(mothership): virtualize chat transc..." | Re-trigger Greptile
… gap, per-role estimate - Pending-chat initial scroll (Cursor, high): seed the scrolled-chat guard with a unique sentinel so a not-yet-persisted chat (undefined chatId) with messages still scrolls to bottom instead of being treated as already-scrolled. - Streaming-row flash (review of virtualization): pin the last row in the rendered window via rangeExtractor so scrolling it out of the overscan window and back mid-stream can't unmount/remount it and re-fire the reveal fade. - Empty assistant row gap (Greptile): move the row gap from the virtual-item wrapper into the row content so a null-rendering (finalised, empty) assistant turn collapses to zero height instead of leaving a pb-6 blank slot. - Per-role row-height estimate instead of a single blended constant, so the scrollbar drifts less as off-screen rows resolve. - Drop the scripts/perf harness from the PR.
|
Addressed all review findings in 6d94ee8:
Perf is unchanged by these fixes (n=1032: ~1.7s to rendered, ~0.8s main-thread blocked, 1.4k DOM nodes). Typecheck, Biome, and @greptile review |
Restoring pt-3/pt-2 on the user row keeps the exact inter-row rhythm from the old space-y-6 layout: assistant→user gaps stay 24+12px and user→assistant stay 24px, instead of becoming uniform when the gap moved to per-row pb.
|
@greptile |
|
@cursor review |
The per-chat initial-scroll guard treated undefined→persisted-id as a chat switch and scrolled to the bottom again, yanking the viewport down if the user had scrolled up mid-stream. Treat that transition as the same conversation: adopt the id without re-scrolling. Genuine chat switches (id→different id) still re-scroll.
|
Latest commit is 73bea30 (pending-chat scroll-guard fix). All prior findings from both bots are fixed and their threads resolved; PR is MERGEABLE/CLEAN with 0 open threads. Requesting a fresh pass on the current HEAD: @greptile review |
|
@cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 73bea30. Configure here.
1 participant
Problem
Long Mothership chats got progressively slower to open and to type in. The transcript rendered every message into the DOM with no windowing — a custom rAF "progressive list" hook only smeared the mount cost across frames without capping it. The input toolbar also re-rendered on every streamed token because the parent transcript re-renders each chunk.
Measured on a cloned production chat scaled to several sizes (harness in
scripts/perf/):Load cost is now flat regardless of transcript length (n=129 ≈ n=1032).
Changes
@tanstack/react-virtual(useVirtualizer) using dynamicmeasureElement, stable per-row keys (getItemKey),overscan: 6, and a tuned size estimate. Only the visible window mounts. This is the approach in TanStack's official chat guidance.useProgressiveList— windowing supersedes rAF-batched incremental mounting (it never capped the DOM-node ceiling). Dead hook deleted.UserInput+ stabilize its callbacks withuseCallback(inMothershipChatandhome.tsx) so streaming ticks no longer re-render the entire input toolbar. Verified every prop is stable.useAutoScrollfor streaming stick-to-bottom (it reads the virtualizer's realscrollHeight), plus a per-chatscrollToIndex(last, {align:'end'})for initial positioning before paint.Validation
check:api-validation✅.MothershipChatre-renders ~11× (was 28+) and the markdown renderer dropped out of the hot path entirely (was 247 renders / 792ms).useSmoothText/Streamdown) is untouched.Note on streaming validation
The streaming render path is unchanged and append-follow is verified under virtualization. A live LLM stream could not be exercised locally (the copilot agent backend is unreachable from the dev machine) — continuous tail-follow rests on the unchanged
useAutoScrollreading the virtualizer's correctscrollHeight, and should be confirmed on the staging deploy.Harness
scripts/perf/adds a headless-Chromium load tester (chat-load-perf.mjs, with optional react-scan + send/stream phases), a scale seeder (seed-chat-scale.mjs), and a streaming-follow validator (stream-validate.mjs). Seescripts/perf/README.md.🤖 Generated with Claude Code