-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Optimize chat message rendering and message-tree updates with memoization #2706
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
…ing and structured content display.
…ies for nested message management.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
2 issues found across 5 files
Prompt for AI agents (all 2 issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="frontend/src/components/chat/Messages/Message/index.tsx">
<violation number="1" location="frontend/src/components/chat/Messages/Message/index.tsx:92">
P1: `useMemo` is called inside the conditional user-message branch, violating React’s hook rules because hooks must run unconditionally and in the same order every render. Move the memoized JSX computation to the top level (e.g., assign it to a variable with useMemo before the return) and reference that variable inside the conditional instead of invoking the hook there.</violation>
</file>
<file name="frontend/src/components/chat/Messages/Message/Content/index.tsx">
<violation number="1" location="frontend/src/components/chat/Messages/Message/Content/index.tsx:130">
P2: `sections` prop changes are ignored by the new memo comparator, so toggling which sections render no longer updates the UI.</violation>
</file>
Since this is your first cubic review, here's how it works:
- cubic automatically reviews your code and comments on bugs and improvements
- Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
- Ask questions if you need clarification on any suggestion
Reply to cubic to teach it or ask questions. Re-run a review with @cubic-dev-ai review this PR
| allowHtml={allowHtml} | ||
| latex={latex} | ||
| /> | ||
| {useMemo( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P1: useMemo is called inside the conditional user-message branch, violating React’s hook rules because hooks must run unconditionally and in the same order every render. Move the memoized JSX computation to the top level (e.g., assign it to a variable with useMemo before the return) and reference that variable inside the conditional instead of invoking the hook there.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At frontend/src/components/chat/Messages/Message/index.tsx, line 92:
<comment>`useMemo` is called inside the conditional user-message branch, violating React’s hook rules because hooks must run unconditionally and in the same order every render. Move the memoized JSX computation to the top level (e.g., assign it to a variable with useMemo before the return) and reference that variable inside the conditional instead of invoking the hook there.</comment>
<file context>
@@ -87,12 +89,17 @@ const Message = memo(
- allowHtml={allowHtml}
- latex={latex}
- />
+ {useMemo(
+ () => (
+ <MessageContent
</file context>
✅ Addressed in 78bbcb1
| return ( | ||
| prevProps.allowHtml === nextProps.allowHtml && | ||
| prevProps.latex === nextProps.latex && | ||
| prevProps.elements === nextProps.elements && |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P2: sections prop changes are ignored by the new memo comparator, so toggling which sections render no longer updates the UI.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At frontend/src/components/chat/Messages/Message/Content/index.tsx, line 130:
<comment>`sections` prop changes are ignored by the new memo comparator, so toggling which sections render no longer updates the UI.</comment>
<file context>
@@ -111,7 +122,18 @@ const MessageContent = memo(
+ return (
+ prevProps.allowHtml === nextProps.allowHtml &&
+ prevProps.latex === nextProps.latex &&
+ prevProps.elements === nextProps.elements &&
+ isEqual(
+ getMessageRenderProps(prevProps.message),
</file context>
| prevProps.elements === nextProps.elements && | |
| prevProps.elements === nextProps.elements && | |
| isEqual( | |
| prevProps.sections ?? ['input', 'output'], | |
| nextProps.sections ?? ['input', 'output'] | |
| ) && |
✅ Addressed in 78bbcb1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No issues found across 5 files
Summary
This PR improves the performance and clarity of the chat message rendering flow by adding proper memoization, refining render-relevant props, and optimizing message-tree update utilities.
Changes
memocomparison forMessageContentto prevent unnecessary re-renders based on only the props that affect rendering.UserMessageandMessageinmemoand introduceduseMemowhere appropriate to avoid repeated child rendering.getMessageRenderPropsto ensure stable message-dependent render comparisons.useMemofor consistent scorable-message detection.Motivation
Summary by cubic
Optimized chat message rendering with targeted memoization and faster message-tree updates to reduce re-renders and smooth streaming.
Written for commit 78bbcb1. Summary will update automatically on new commits.