Skip to content

Commit ccfff2d

Browse files
marthakellyclaude
andcommitted
refactor(react-core): extract deduplicateMessages, add useMemo, expand test coverage
- Extract dedup logic into exported deduplicateMessages() pure function so it can be unit-tested directly - Wrap with useMemo([messages]) to avoid allocating a new Map on every frame - Collapse three-way branch to two-way (the two arms were identical) - Use { ...existing, ...message, content } for a true merge so fields present only in an earlier occurrence are not silently dropped - Expand comment to explain why || treats empty string as falsy - Add deduplicateMessages unit tests: toolCalls assertion, reverse scenario, non-assistant keep-last behavior - Rename "keeping the last occurrence" test — no longer accurate for assistant messages which now merge rather than keep-last Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 6e444ec commit ccfff2d

2 files changed

Lines changed: 91 additions & 22 deletions

File tree

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

Lines changed: 39 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import React, { useEffect, useReducer, useState } from "react";
1+
import React, { useEffect, useMemo, useReducer, useState } from "react";
22
import { WithSlots, renderSlot, isReactComponentType } from "../../lib/slots";
33
import CopilotChatAssistantMessage from "./CopilotChatAssistantMessage";
44
import CopilotChatUserMessage from "./CopilotChatUserMessage";
@@ -273,6 +273,40 @@ const MemoizedCustomMessage = React.memo(
273273
},
274274
);
275275

276+
/**
277+
* Deduplicates messages by ID. For assistant messages, merges occurrences:
278+
* recovers non-empty content from any earlier occurrence if the latest wiped it
279+
* (empty string means the streaming update cleared the field, not blank text),
280+
* while keeping the latest toolCalls and all other fields. For all other roles,
281+
* keeps the last entry.
282+
*/
283+
export function deduplicateMessages(messages: Message[]): Message[] {
284+
const acc = new Map<string, Message>();
285+
for (const message of messages) {
286+
const existing = acc.get(message.id);
287+
if (
288+
existing &&
289+
message.role === "assistant" &&
290+
existing.role === "assistant"
291+
) {
292+
// Empty string means the streaming update cleared the field — fall back to
293+
// any non-empty content seen earlier. Use { ...existing, ...message } so
294+
// fields present only in an earlier occurrence are not silently dropped.
295+
const content =
296+
(message as AssistantMessage).content ||
297+
(existing as AssistantMessage).content;
298+
acc.set(message.id, {
299+
...existing,
300+
...message,
301+
content,
302+
} as AssistantMessage);
303+
} else {
304+
acc.set(message.id, message);
305+
}
306+
}
307+
return [...acc.values()];
308+
}
309+
276310
export type CopilotChatMessageViewProps = Omit<
277311
WithSlots<
278312
{
@@ -362,24 +396,10 @@ export function CopilotChatMessageView({
362396
);
363397
};
364398

365-
// Deduplicate messages by id. During streaming, the same message ID can appear
366-
// multiple times (e.g. a text chunk followed by tool-call updates that clear
367-
// the content field). For assistant messages we merge: keep the latest
368-
// toolCalls (they accumulate) but recover non-empty content from any earlier
369-
// occurrence if the latest wiped it. For all other roles, keep the last entry.
370-
const messageAccumulator = new Map<string, Message>();
371-
for (const message of messages) {
372-
const existing = messageAccumulator.get(message.id);
373-
if (!existing) {
374-
messageAccumulator.set(message.id, message);
375-
} else if (message.role === "assistant" && existing.role === "assistant") {
376-
const content = message.content || existing.content;
377-
messageAccumulator.set(message.id, { ...message, content });
378-
} else {
379-
messageAccumulator.set(message.id, message);
380-
}
381-
}
382-
const deduplicatedMessages = [...messageAccumulator.values()];
399+
const deduplicatedMessages = useMemo(
400+
() => deduplicateMessages(messages),
401+
[messages],
402+
);
383403

384404
if (
385405
process.env.NODE_ENV === "development" &&

packages/react-core/src/v2/components/chat/__tests__/CopilotChatMessageView.test.tsx

Lines changed: 52 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,9 @@ import { z } from "zod";
44
import { vi } from "vitest";
55
import { CopilotKitProvider } from "../../../providers/CopilotKitProvider";
66
import { CopilotChatConfigurationProvider } from "../../../providers/CopilotChatConfigurationProvider";
7-
import CopilotChatMessageView from "../CopilotChatMessageView";
7+
import CopilotChatMessageView, {
8+
deduplicateMessages,
9+
} from "../CopilotChatMessageView";
810
import type {
911
ActivityMessage,
1012
AssistantMessage,
@@ -131,7 +133,7 @@ describe("CopilotChatMessageView duplicate message deduplication", () => {
131133
expect(assistantMessages[0].textContent).toContain("Let me record that...");
132134
});
133135

134-
it("deduplicates messages with the same id, keeping the last occurrence", () => {
136+
it("uses latest content when all assistant duplicates have non-empty content", () => {
135137
const messages: Message[] = [
136138
userMsg("user-1", "Hello"),
137139
assistantMsg("assistant-1", "Partial response..."),
@@ -162,7 +164,7 @@ describe("CopilotChatMessageView duplicate message deduplication", () => {
162164
consoleSpy.mockRestore();
163165
});
164166

165-
it("preserves order of unique messages", () => {
167+
it("preserves order of unique messages (no duplicates)", () => {
166168
const messages: Message[] = [
167169
userMsg("user-1", "First question"),
168170
assistantMsg("assistant-1", "First answer"),
@@ -180,3 +182,50 @@ describe("CopilotChatMessageView duplicate message deduplication", () => {
180182
expect(assistantMessages).toHaveLength(2);
181183
});
182184
});
185+
186+
describe("deduplicateMessages", () => {
187+
it("recovers non-empty content and keeps latest toolCalls when later duplicate clears content", () => {
188+
const messages: Message[] = [
189+
assistantMsg("assistant-1", "Let me record that..."),
190+
assistantMsg("assistant-1", "", [toolCall("tc-1", "captureData")]),
191+
assistantMsg("assistant-1", "", [
192+
toolCall("tc-1", "captureData"),
193+
toolCall("tc-2", "updateMemory"),
194+
]),
195+
];
196+
197+
const result = deduplicateMessages(messages);
198+
199+
expect(result).toHaveLength(1);
200+
const merged = result[0] as AssistantMessage;
201+
// Content recovered from the first occurrence
202+
expect(merged.content).toBe("Let me record that...");
203+
// toolCalls from the latest occurrence (both tc-1 and tc-2)
204+
expect(merged.toolCalls).toHaveLength(2);
205+
expect(merged.toolCalls?.map((tc) => tc.id)).toEqual(["tc-1", "tc-2"]);
206+
});
207+
208+
it("uses content from a later occurrence when early occurrence has empty content", () => {
209+
const messages: Message[] = [
210+
assistantMsg("assistant-1", ""),
211+
assistantMsg("assistant-1", "Here is the result."),
212+
];
213+
214+
const result = deduplicateMessages(messages);
215+
216+
expect(result).toHaveLength(1);
217+
expect((result[0] as AssistantMessage).content).toBe("Here is the result.");
218+
});
219+
220+
it("keeps last entry for non-assistant roles", () => {
221+
const messages: Message[] = [
222+
userMsg("u-1", "Hello"),
223+
userMsg("u-1", "Hello (updated)"),
224+
];
225+
226+
const result = deduplicateMessages(messages);
227+
228+
expect(result).toHaveLength(1);
229+
expect((result[0] as UserMessage).content).toBe("Hello (updated)");
230+
});
231+
});

0 commit comments

Comments
 (0)