Skip to content

Commit cdbc345

Browse files
marthakellyclaude
andcommitted
fix(react-core): stabilize messageView/labels props to prevent message re-renders on keystroke
Inline `messageView` and `labels` objects passed to CopilotChat caused all completed assistant messages to re-render on every keystroke because: 1. `ts-deepmerge.merge()` deep-clones its inputs, producing a new `messageView` reference on every render even when content is identical — defeating `MemoizedSlotWrapper`'s shallow equality check. 2. An inline `labels` object (new reference each render) invalidated the `mergedLabels` useMemo in `CopilotChatConfigurationProvider`, causing every `useCopilotChatConfiguration()` consumer to re-render on every keystroke. 3. The inline `onAddFile` arrow function was a new reference each render. Fixes: - Replace `merge()` with shallow spread; stabilize `messageView` via a ref + `shallowEqual` guard so the same object reference is returned when props are shallowly equal across renders. - Use `useCallback` for `handleAddFile`. - Add JSON.stringify-based dep keys in `CopilotChatConfigurationProvider` so `mergedLabels` is only recomputed when label values actually change. Adds deterministic render-count regression tests (FOR-75) that fail on the unfixed code and pass after the fix. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 99b0986 commit cdbc345

4 files changed

Lines changed: 303 additions & 29 deletions

File tree

.gitignore

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,3 +60,4 @@ lefthook-local.yml
6060
*.a
6161
*.lib
6262
*.wasm
63+
tasks/

packages/react-core/src/v2/components/chat/CopilotChat.tsx

Lines changed: 67 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -22,14 +22,13 @@ import React, {
2222
useRef,
2323
useState,
2424
} from "react";
25-
import { merge } from "ts-deepmerge";
2625
import {
2726
useCopilotKit,
2827
useLicenseContext,
2928
} from "../../providers/CopilotKitProvider";
3029
import { InlineFeatureWarning } from "../../components/license-warning-banner";
3130
import { AbstractAgent, HttpAgent } from "@ag-ui/client";
32-
import { renderSlot, SlotValue } from "../../lib/slots";
31+
import { renderSlot, shallowEqual, SlotValue } from "../../lib/slots";
3332
import {
3433
transcribeAudio,
3534
TranscriptionError,
@@ -406,22 +405,68 @@ export function CopilotChat({
406405
}
407406
}, [transcriptionError]);
408407

409-
const mergedProps = merge(
410-
{
411-
isRunning: agent.isRunning,
412-
suggestions: autoSuggestions,
413-
onSelectSuggestion: handleSelectSuggestion,
414-
suggestionView: providedSuggestionView,
415-
},
416-
{
417-
...restProps,
418-
...(typeof providedMessageView === "string"
419-
? { messageView: { className: providedMessageView } }
420-
: providedMessageView !== undefined
421-
? { messageView: providedMessageView }
422-
: {}),
423-
},
424-
);
408+
// Stabilize the messageView slot reference. Two problems without this:
409+
//
410+
// 1. String messageView: `{ className }` is created inline on every render,
411+
// producing a new object reference each time.
412+
// 2. Object messageView: ts-deepmerge (used below) deep-clones plain objects
413+
// even from a single source, so even a stable prop reference becomes a new
414+
// object after merge().
415+
//
416+
// Fix: use a ref + shallowEqual to keep the same object reference as long as
417+
// the slot's own properties haven't changed. This way MemoizedSlotWrapper's
418+
// shallow equality check passes on every keystroke, preventing unnecessary
419+
// re-renders of CopilotChatView and its message list.
420+
const processedMessageView =
421+
typeof providedMessageView === "string"
422+
? { className: providedMessageView }
423+
: providedMessageView;
424+
const messageViewRef = useRef(processedMessageView);
425+
if (messageViewRef.current !== processedMessageView) {
426+
// Both are plain objects — deep-compare to avoid spurious reference churn
427+
// (e.g. inline `messageView={{ assistantMessage: Cmp }}` creates a new object
428+
// on every parent render even though the values haven't changed).
429+
// For undefined / component / string values the reference check above is
430+
// sufficient; shallowEqual is only needed when both sides are plain objects.
431+
const prevMV = messageViewRef.current;
432+
const shouldUpdate =
433+
prevMV == null ||
434+
processedMessageView == null ||
435+
typeof prevMV !== "object" ||
436+
typeof processedMessageView !== "object" ||
437+
!shallowEqual(
438+
prevMV as Record<string, unknown>,
439+
processedMessageView as Record<string, unknown>,
440+
);
441+
if (shouldUpdate) {
442+
messageViewRef.current = processedMessageView;
443+
}
444+
}
445+
const stableMessageView = messageViewRef.current;
446+
447+
// Stabilize the `onAddFile` handler. Without useCallback, a new arrow
448+
// function is created inline on every render, causing CopilotChatView to
449+
// re-render on every keystroke even when nothing else changed.
450+
const handleAddFile = useCallback(() => {
451+
// Delay to let Radix dropdown menu close before triggering file input
452+
setTimeout(() => {
453+
fileInputRef.current?.click();
454+
}, 100);
455+
}, []);
456+
457+
// Use shallow spread instead of ts-deepmerge. ts-deepmerge deep-clones plain
458+
// objects even from a single source, which would defeat the reference
459+
// stability we just established for stableMessageView and other slot values.
460+
const mergedProps = {
461+
isRunning: agent.isRunning,
462+
suggestions: autoSuggestions,
463+
onSelectSuggestion: handleSelectSuggestion,
464+
suggestionView: providedSuggestionView,
465+
...restProps,
466+
...(stableMessageView !== undefined
467+
? { messageView: stableMessageView }
468+
: {}),
469+
};
425470

426471
const hasMessages = agent.messages.length > 0;
427472
const shouldAllowStop = agent.isRunning && hasMessages;
@@ -468,7 +513,8 @@ export function CopilotChat({
468513
[messagesMemoKey],
469514
);
470515

471-
const finalProps = merge(mergedProps, {
516+
const finalProps: CopilotChatViewProps = {
517+
...mergedProps,
472518
messages,
473519
// Input behavior props
474520
onSubmitMessage: onSubmitInput,
@@ -486,19 +532,12 @@ export function CopilotChat({
486532
// Attachment props
487533
attachments: selectedAttachments,
488534
onRemoveAttachment: removeAttachment,
489-
onAddFile: attachmentsEnabled
490-
? () => {
491-
// Delay to let Radix dropdown menu close before triggering file input
492-
setTimeout(() => {
493-
fileInputRef.current?.click();
494-
}, 100);
495-
}
496-
: undefined,
535+
onAddFile: attachmentsEnabled ? handleAddFile : undefined,
497536
dragOver,
498537
onDragOver: handleDragOver,
499538
onDragLeave: handleDragLeave,
500539
onDrop: handleDrop,
501-
}) as CopilotChatViewProps;
540+
};
502541

503542
// Always create a provider with merged values
504543
// This ensures priority: props > existing config > defaults
Lines changed: 224 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,224 @@
1+
/**
2+
* Reproduction tests for FOR-75: messageView / labels props freeze
3+
*
4+
* These tests prove that passing `messageView` or `labels` as inline props
5+
* to CopilotChat causes completed assistant messages to re-render on every
6+
* keystroke — even though the messages haven't changed.
7+
*
8+
* Tests FAIL on main before the fix (reproducing the bug).
9+
* Tests PASS after the fix is applied.
10+
*
11+
* Render counts are deterministic regardless of hardware — the bug is about
12+
* reference instability, not wall-clock timing.
13+
*/
14+
import React from "react";
15+
import {
16+
render,
17+
screen,
18+
fireEvent,
19+
waitFor,
20+
act,
21+
} from "@testing-library/react";
22+
import {
23+
AbstractAgent,
24+
EventType,
25+
type BaseEvent,
26+
type RunAgentInput,
27+
} from "@ag-ui/client";
28+
import { Observable, Subject } from "rxjs";
29+
import { CopilotKitProvider } from "../../../providers/CopilotKitProvider";
30+
import { CopilotChat } from "../CopilotChat";
31+
32+
// ---------------------------------------------------------------------------
33+
// Shared mock agent (same pattern as CopilotChatToolRerenders.e2e.test.tsx)
34+
// ---------------------------------------------------------------------------
35+
class MockStepwiseAgent extends AbstractAgent {
36+
private subject = new Subject<BaseEvent>();
37+
38+
emit(event: BaseEvent) {
39+
if (event.type === EventType.RUN_STARTED) {
40+
this.isRunning = true;
41+
} else if (
42+
event.type === EventType.RUN_FINISHED ||
43+
event.type === EventType.RUN_ERROR
44+
) {
45+
this.isRunning = false;
46+
}
47+
act(() => {
48+
this.subject.next(event);
49+
});
50+
}
51+
52+
complete() {
53+
this.isRunning = false;
54+
this.subject.complete();
55+
}
56+
57+
clone(): MockStepwiseAgent {
58+
const cloned = new MockStepwiseAgent();
59+
cloned.agentId = this.agentId;
60+
(cloned as unknown as { subject: Subject<BaseEvent> }).subject =
61+
this.subject;
62+
return cloned;
63+
}
64+
65+
async detachActiveRun(): Promise<void> {}
66+
67+
run(_input: RunAgentInput): Observable<BaseEvent> {
68+
return this.subject.asObservable();
69+
}
70+
}
71+
72+
// ---------------------------------------------------------------------------
73+
// Helper: submit a user message (triggers agent.run()), then emit a
74+
// complete assistant response and wait for it to appear.
75+
// ---------------------------------------------------------------------------
76+
async function submitAndReceiveAssistantMessage(
77+
agent: MockStepwiseAgent,
78+
messageId: string,
79+
assistantText: string,
80+
) {
81+
// Submit a user message to trigger agent.run() — this subscribes to the subject
82+
const input = await screen.findByRole("textbox");
83+
fireEvent.change(input, { target: { value: "hello" } });
84+
fireEvent.keyDown(input, { key: "Enter", code: "Enter" });
85+
86+
await waitFor(() => {
87+
expect(screen.getByText("hello")).toBeDefined();
88+
});
89+
90+
// Now emit the assistant response through the (now-subscribed) subject
91+
agent.emit({ type: EventType.RUN_STARTED } as BaseEvent);
92+
agent.emit({
93+
type: EventType.TEXT_MESSAGE_CHUNK,
94+
messageId,
95+
delta: assistantText,
96+
} as BaseEvent);
97+
agent.emit({ type: EventType.RUN_FINISHED } as BaseEvent);
98+
99+
await waitFor(() => {
100+
expect(screen.getByText(assistantText)).toBeDefined();
101+
});
102+
103+
await act(async () => {
104+
agent.complete();
105+
});
106+
}
107+
108+
// ---------------------------------------------------------------------------
109+
// Counting component — defined OUTSIDE tests so its reference is stable.
110+
// The surrounding messageView object will be an inline object (new ref every
111+
// render), which is what triggers the bug.
112+
// ---------------------------------------------------------------------------
113+
let assistantRenderCount = 0;
114+
function CountingAssistantMessage(
115+
_props: React.HTMLAttributes<HTMLDivElement>,
116+
) {
117+
assistantRenderCount++;
118+
return <div data-testid="counting-assistant">hello from assistant</div>;
119+
}
120+
121+
// ---------------------------------------------------------------------------
122+
// Tests
123+
// ---------------------------------------------------------------------------
124+
125+
describe("FOR-75: messageView / labels props — no re-renders on input change", () => {
126+
beforeEach(() => {
127+
assistantRenderCount = 0;
128+
});
129+
130+
/**
131+
* Test A: messageView inline object
132+
*
133+
* When `messageView` is passed as an inline object prop (e.g.
134+
* `messageView={{ assistantMessage: Cmp }}`), a new object reference is
135+
* created on every parent render. ts-deepmerge deep-clones the value,
136+
* producing a new reference that defeats MemoizedSlotWrapper's shallow
137+
* equality check → CopilotChatView + CopilotChatMessageView re-render on
138+
* every keystroke → CountingAssistantMessage is called again.
139+
*
140+
* Fix: memoize the messageView transformation in CopilotChat.tsx so the
141+
* slot reference is stable across renders.
142+
*/
143+
it("messageView inline object: completed messages do not re-render on keystroke", async () => {
144+
const agent = new MockStepwiseAgent();
145+
146+
render(
147+
<CopilotKitProvider agents__unsafe_dev_only={{ default: agent }}>
148+
<div style={{ height: 400 }}>
149+
<CopilotChat
150+
// Inline object — new reference every render of the test component.
151+
// The assistantMessage value (CountingAssistantMessage) is stable,
152+
// but the outer object is cloned by ts-deepmerge on each render.
153+
messageView={{ assistantMessage: CountingAssistantMessage } as any}
154+
/>
155+
</div>
156+
</CopilotKitProvider>,
157+
);
158+
159+
await submitAndReceiveAssistantMessage(
160+
agent,
161+
"msg-1",
162+
"hello from assistant",
163+
);
164+
165+
const renderCountAfterMessage = assistantRenderCount;
166+
expect(renderCountAfterMessage).toBeGreaterThan(0);
167+
168+
// Type into the (now-cleared) input — only inputValue state changes; messages unchanged.
169+
// Completed messages must NOT re-render.
170+
const input = screen.getByRole("textbox");
171+
fireEvent.change(input, { target: { value: "a" } });
172+
fireEvent.change(input, { target: { value: "ab" } });
173+
fireEvent.change(input, { target: { value: "abc" } });
174+
175+
await act(async () => {});
176+
177+
// KEY ASSERTION: no additional renders caused by typing
178+
expect(assistantRenderCount).toBe(renderCountAfterMessage);
179+
});
180+
181+
/**
182+
* Test C: labels inline object
183+
*
184+
* When `labels` is passed as an inline object, it is a new reference every
185+
* render. This invalidates the mergedLabels useMemo in
186+
* CopilotChatConfigurationProvider → new context value → all context
187+
* consumers re-render on every keystroke.
188+
*
189+
* Fix: deep-compare labels dep in CopilotChatConfigurationProvider.
190+
*/
191+
it("labels inline object: completed messages do not re-render on keystroke", async () => {
192+
const agent = new MockStepwiseAgent();
193+
194+
render(
195+
<CopilotKitProvider agents__unsafe_dev_only={{ default: agent }}>
196+
<div style={{ height: 400 }}>
197+
<CopilotChat
198+
messageView={{ assistantMessage: CountingAssistantMessage } as any}
199+
// Inline labels object — new reference every render
200+
labels={{ chatInputPlaceholder: "Type here..." }}
201+
/>
202+
</div>
203+
</CopilotKitProvider>,
204+
);
205+
206+
await submitAndReceiveAssistantMessage(
207+
agent,
208+
"msg-labels-1",
209+
"hello from assistant",
210+
);
211+
212+
const renderCountAfterMessage = assistantRenderCount;
213+
expect(renderCountAfterMessage).toBeGreaterThan(0);
214+
215+
const input = screen.getByRole("textbox");
216+
fireEvent.change(input, { target: { value: "a" } });
217+
fireEvent.change(input, { target: { value: "ab" } });
218+
fireEvent.change(input, { target: { value: "abc" } });
219+
220+
await act(async () => {});
221+
222+
expect(assistantRenderCount).toBe(renderCountAfterMessage);
223+
});
224+
});

packages/react-core/src/v2/providers/CopilotChatConfigurationProvider.tsx

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,13 +65,23 @@ export const CopilotChatConfigurationProvider: React.FC<
6565
> = ({ children, labels, agentId, threadId, isModalDefaultOpen }) => {
6666
const parentConfig = useContext(CopilotChatConfiguration);
6767

68+
// Deep-compare labels by serializing to JSON. This prevents re-computing
69+
// mergedLabels (and changing the context value) when callers pass an inline
70+
// object like `labels={{ chatInputPlaceholder: "..." }}` — a new reference
71+
// on every render that would otherwise cause all context consumers to
72+
// re-render on every keystroke.
73+
// eslint-disable-next-line react-hooks/exhaustive-deps
74+
const labelsKey = JSON.stringify(labels);
75+
// eslint-disable-next-line react-hooks/exhaustive-deps
76+
const parentLabelsKey = JSON.stringify(parentConfig?.labels);
6877
const mergedLabels: CopilotChatLabels = useMemo(
6978
() => ({
7079
...CopilotChatDefaultLabels,
7180
...(parentConfig?.labels ?? {}),
7281
...(labels ?? {}),
7382
}),
74-
[labels, parentConfig?.labels],
83+
// eslint-disable-next-line react-hooks/exhaustive-deps
84+
[labelsKey, parentLabelsKey],
7585
);
7686

7787
const resolvedAgentId = agentId ?? parentConfig?.agentId ?? DEFAULT_AGENT_ID;

0 commit comments

Comments
 (0)