Skip to content

perf(frontend): memoize chat items to stop re-parsing markdown on every token - #150

Closed
lucastononro wants to merge 5 commits into
fix/100-dedupe-eventsourcefrom
fix/98-memoize-chat
Closed

perf(frontend): memoize chat items to stop re-parsing markdown on every token#150
lucastononro wants to merge 5 commits into
fix/100-dedupe-eventsourcefrom
fix/98-memoize-chat

Conversation

@lucastononro

@lucastononro lucastononro commented Jul 19, 2026

Copy link
Copy Markdown
Owner

Summary

  • renderGroupedChatItems/renderChatItem were plain functions, and every assistant bubble ran ReactMarkdown. Each agent_token SSE event calls setChatItems, which re-ran the whole list and re-parsed markdown for every prior message — O(messages) parses per streamed chunk, causing visible jank in long sessions.

Changes

  • src/app/page.tsx — extracted renderChatItem into a memo-wrapped ChatItemView component, rendered with key={item.id} from renderGroupedChatItems. Since only the currently-streaming assistant item's object reference changes between renders (all other ChatItem objects in chatItems are untouched), memo's shallow-prop comparison now lets React bail out and skip re-render (and re-parse) for every bubble except the one actually changing.
  • Hoisted the assistant bubble's remarkPlugins={[remarkGfm]} array to a module-level CHAT_MARKDOWN_PLUGINS constant — a fresh array literal on every render would otherwise look like a "new" plugin list to ReactMarkdown and undermine the memoization, mirroring the existing ReportMarkdown/components memoization pattern (page.tsx ReportMarkdown).
  • Purely a refactor of the render path — no changes to ChatItem, the SSE handler, or any card component's own logic (CollapsibleToolCard, SubAgentCard, ClarificationCard, AgentToolCard are called with identical props, just without the now-redundant inline key).

Test plan

Existing (must still work)

  • Streaming chat: user bubbles, assistant bubbles (with per-agent color/avatar), tool cards, subagent cards, clarification cards, error/status/stage-complete rows all still render identically.
  • Live metrics tab and file/image/PDF viewing in the workspace canvas — untouched by this change, but worth a smoke check since the same page component owns the whole chat panel.
  • Notebook updates — untouched by this PR; verify still working after the PR Stop opening two EventSource connections to the same stream #100 stream-dedupe base.

New

  • Long session with many chat messages: while an assistant reply streams token-by-token, scroll up to earlier messages and confirm the UI stays smooth (no visible re-render jank across the whole list per token) — this is the perf regression the issue reports.
  • React DevTools profiler (optional but recommended): confirm only the streaming ChatItemView instance re-renders per agent_token event, not the full list.

Closes #98

🤖 Generated with Claude Code

Greptile Summary

This PR memoizes the chat message list to eliminate O(messages) ReactMarkdown re-parses per streamed token, by converting renderChatItem into a memo-wrapped ChatItemView component with a module-level CHAT_MARKDOWN_PLUGINS constant. The streamingItemId string prop is replaced with a per-item isStreaming boolean, so only the actively-streaming bubble receives a prop change at stream start/end boundaries.

  • ChatItemView (formerly renderChatItem) is wrapped in React.memo and keyed by item.id; the SSE handler's [...prev] spread preserves object references for all non-streaming items, so memo's shallow comparison correctly bails out for every prior bubble during a stream.
  • CHAT_MARKDOWN_PLUGINS = [remarkGfm] is hoisted to module scope, preventing a fresh array reference on each render from defeating memo for ReactMarkdown.
  • Both issues flagged in the previous Greptile review (the streamingItemId-string prop and the type signature) are resolved in this revision.

Confidence Score: 5/5

Safe to merge; the change is a pure render-path refactor with no logic, state, or SSE handler changes.

The SSE handler already preserves object references for non-streaming items ([...prev] with only the streaming slot replaced), so memo's shallow prop comparison works as intended. Both issues from the prior review round are addressed: the shared streamingItemId string is replaced with a per-item isStreaming boolean, and the ChatItemView type signature matches. CHAT_MARKDOWN_PLUGINS is correctly hoisted. The only outstanding item is ToolGroupCard re-rendering on every token (pre-existing, no markdown parsing cost).

Files Needing Attention: No files require special attention; the core change in page.tsx is straightforward and the SSE handler's reference-stability assumption is confirmed in the same file.

Important Files Changed

Filename Overview
frontend/src/app/page.tsx Converts renderChatItem into a memo-wrapped ChatItemView with isStreaming boolean prop and hoisted CHAT_MARKDOWN_PLUGINS; SSE handler preserves item references for non-streaming bubbles, so memoization is effective.
docs/manual-tests/chat-memoization.md New manual test guide covering memo bail-out verification, caret behavior, and visual regression checks for all chat item types.
STAGING-v0.0.5.md Adds row 31 to the staging merge log for PR #148 (dedupe-eventsource base); documentation-only change.

Sequence Diagram

sequenceDiagram
    participant SSE as SSE Handler
    participant State as chatItems state
    participant RGC as renderGroupedChatItems
    participant Memo as React.memo (ChatItemView)
    participant RM as ReactMarkdown

    SSE->>State: agent_token → setChatItems([...prev, updatedStreamingItem])
    Note over State: Non-streaming items keep same object refs
    State->>RGC: re-runs on parent render
    RGC->>Memo: "ChatItemView key=id item=stableRef isStreaming=false"
    Memo-->>Memo: shallow compare: item ref unchanged → bail out
    Note over RM: ReactMarkdown NOT called for prior bubbles
    RGC->>Memo: "ChatItemView key=streamId item=newRef isStreaming=true"
    Memo->>RM: props changed → render
    RM-->>Memo: parse + render markdown for streaming bubble only
Loading

Reviews (3): Last reviewed commit: "Merge branch 'staging-v0.0.5' into fix/9..." | Re-trigger Greptile

…wn on every token

renderChatItem was a plain function, so every agent_token/agent_message
setChatItems re-ran the whole list and re-parsed markdown for every prior
message. Extract a memo-wrapped ChatItemView keyed by item.id (only the
streaming bubble's item reference changes, so React bails out on the
rest) and hoist the assistant bubble's remark-plugins array to a module
constant so it doesn't defeat the memoization with a fresh array each
render.

Closes #98

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Wp66uSq9saSpRq9Bbmjxj4
Comment thread frontend/src/app/page.tsx
Comment thread frontend/src/app/page.tsx
- Pass a per-item `isStreaming` boolean to ChatItemView instead of the
  shared `streamingItemId` string, so stream start/end only re-renders
  the affected bubble instead of breaking memo bailout for all items
  (both P2 findings).
- Add a manual test tutorial for the memoization/streaming behavior
  (docs/manual-tests/chat-memoization.md) — no frontend unit-test
  infrastructure exists to host a vitest test.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Wp66uSq9saSpRq9Bbmjxj4
@lucastononro

Copy link
Copy Markdown
Owner Author

Greptile review findings addressed (b37907e)

Fixed (both P2 findings — they are two halves of the same valid issue):

  • page.tsx call site: ChatItemView now receives a per-item isStreaming={cur.id === streamingItemId} boolean instead of the shared streamingItemId string. Previously, when streaming started or ended, every item saw a changed prop and memo couldn't bail out — an O(messages) re-render at each stream boundary. Now only the one bubble whose flag actually flips re-renders.
  • ChatItemView props updated to match (isStreaming?: boolean), and the internal item.id === streamingItemId computation removed.

Dismissed: none — both findings were valid.

Tests: the frontend has no unit-test infrastructure (no vitest/jest, zero test files), and ChatItemView is a non-exported component inside the ~4500-line page.tsx, so introducing a whole test stack in this perf PR wasn't warranted. Added a manual test tutorial instead: docs/manual-tests/chat-memoization.md — React DevTools Profiler steps verifying (1) only the streaming bubble re-renders per token, (2) old markdown bubbles are not re-parsed, (3) the blinking caret appears/disappears only on the streaming bubble, (4) no rendering regressions across all chat item types incl. GFM markdown via the hoisted plugin array.

Validation: npx tsc --noEmit, npm run lint, npm run build — all pass.

@lucastononro

Copy link
Copy Markdown
Owner Author

Merged into integration branch staging-v0.0.5 (see the STAGING-v0.0.5.md ledger on that branch for merge order, Greptile follow-ups and test results). These changes will land on main via the staging merge — closing to clear the queue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant