Skip to content

Conversation

@roomote
Copy link
Contributor

@roomote roomote bot commented Aug 26, 2025

Summary

This PR fixes the excessive re-rendering issue in the ChatView component that was causing UI jittering when typing in the chat input.

Problem

The ChatView component was experiencing excessive re-renders whenever the user typed in the chat input field. This was causing visible jittering in the messages displayed within the chat view.

Root Cause Analysis

After systematic debugging, we identified two primary causes:

  1. itemContent callback recreation: The callback passed to the Virtuoso component was being recreated on every render, bypassing React.memo optimization in ChatRow
  2. lastModifiedMessage recalculation: The modifiedMessages.at(-1) was being recalculated on every render

Solution

Optimizations Applied:

  1. Properly memoized itemContent callback: Added correct dependencies to prevent unnecessary recreation
  2. Memoized lastModifiedMessage: Created a separate memoized value to prevent recalculation
  3. Cleaned up debug logging: Removed diagnostic logging after confirming the fix

Testing

  • ✅ All frontend tests pass (968 tests)
  • ✅ All backend tests pass (3630 tests)
  • ✅ Linting passes
  • ✅ Type checking passes

Impact

This fix eliminates the UI jittering issue when typing in the chat input, providing a smoother user experience without affecting any functionality.

Files Changed

  • webview-ui/src/components/chat/ChatView.tsx - Optimized memoization
  • webview-ui/src/components/chat/ChatRow.tsx - Removed debug logging

Important

Optimized ChatView to prevent excessive re-rendering by memoizing itemContent and lastModifiedMessage.

  • Behavior:
    • Optimized itemContent callback in ChatView.tsx to prevent unnecessary re-renders by adding stable dependencies.
    • Memoized lastModifiedMessage in ChatView.tsx to avoid recalculating on every render.
  • Misc:
    • Removed debug logging from ChatRow.tsx.

This description was created by Ellipsis for a9a0975. You can customize this summary. It will automatically update as commits are pushed.

- Properly memoized itemContent callback with stable dependencies
- Added memoization for lastModifiedMessage to prevent unnecessary recalculations
- Removed debug logging after confirming the fix works

This fixes the UI jittering issue that occurred when typing in the chat input
by preventing the groupedMessages and itemContent from being recalculated on
every keystroke.
@roomote roomote bot requested review from cte, jr and mrubens as code owners August 26, 2025 17:33
@dosubot dosubot bot added size:S This PR changes 10-29 lines, ignoring generated files. bug Something isn't working UI/UX UI/UX related or focused labels Aug 26, 2025
Copy link
Contributor Author

@roomote roomote bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewing my own code is like looking in a mirror after writing it blindfolded.

}, [])

// Memoize the last modified message to prevent unnecessary re-renders
const lastModifiedMessage = useMemo(() => modifiedMessages.at(-1), [modifiedMessages])
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While this memoization correctly prevents re-renders, I'm wondering if creating a new reference on every modifiedMessages change could lead to memory pressure in long-running sessions? Could we consider using a more granular dependency or perhaps a ref-based approach to track just the last message without triggering re-renders?

const lastModifiedMessage = useMemo(() => modifiedMessages.at(-1), [modifiedMessages])

// Properly memoized itemContent callback with stable dependencies
const itemContent = useCallback(
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this is a performance fix, would it be helpful to add a comment documenting the performance improvement achieved? For example: "Reduces re-renders from X to Y when typing". Future me would appreciate knowing why past me made this change.

expandedRows,
toggleRowExpansion,
modifiedMessages,
lastModifiedMessage, // Use the memoized value instead of recalculating
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we add a comment explaining why switching from modifiedMessages to lastModifiedMessage in the dependency array prevents the re-rendering issue? The fix works, but documenting the reasoning would help future maintainers understand the optimization.

@hannesrudolph hannesrudolph added the Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels. label Aug 26, 2025
@daniel-lxs daniel-lxs moved this from Triage to Issue [In Progress] in Roo Code Roadmap Aug 27, 2025
@daniel-lxs daniel-lxs moved this from Issue [In Progress] to PR [Needs Prelim Review] in Roo Code Roadmap Aug 27, 2025
@hannesrudolph hannesrudolph added PR - Needs Preliminary Review and removed Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels. labels Aug 27, 2025
@daniel-lxs daniel-lxs marked this pull request as draft August 30, 2025 19:49
@daniel-lxs daniel-lxs moved this from PR [Needs Prelim Review] to PR [Draft / In Progress] in Roo Code Roadmap Aug 30, 2025
@github-project-automation github-project-automation bot moved this from New to Done in Roo Code Roadmap Sep 23, 2025
@github-project-automation github-project-automation bot moved this from PR [Draft / In Progress] to Done in Roo Code Roadmap Sep 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working PR - Draft / In Progress size:S This PR changes 10-29 lines, ignoring generated files. UI/UX UI/UX related or focused

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

3 participants