Skip to content

Commit 9b15d75

Browse files
committed
fix(client): preserve reasoning messages after MESSAGES_SNAPSHOT
Reasoning messages (role: "reasoning") were dropped when the MESSAGES_SNAPSHOT handler only exempted "activity" role from snapshot-based filtering. Added "reasoning" as a client-only role that is preserved across snapshot merges. Closes #1262 Supersedes #1370
1 parent 74fd5ba commit 9b15d75

2 files changed

Lines changed: 101 additions & 5 deletions

File tree

sdks/typescript/packages/client/src/apply/__tests__/default.activity.test.ts

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -567,6 +567,99 @@ describe("MESSAGES_SNAPSHOT preserves activity messages", () => {
567567
expect(msgs.map((m) => m.id)).toEqual(["m1", "act-1", "m2", "m3"]);
568568
});
569569

570+
it("preserves reasoning messages after MESSAGES_SNAPSHOT", async () => {
571+
const initial: Message[] = [
572+
{ id: "m1", role: "user", content: "hello" },
573+
{ id: "r1", role: "reasoning", content: "Let me think about this..." },
574+
{ id: "m2", role: "assistant", content: "hi there" },
575+
] as Message[];
576+
577+
const events$ = new Subject<BaseEvent>();
578+
const agent = createAgent(initial);
579+
const result$ = defaultApplyEvents(makeInput(initial), events$, agent, []);
580+
const updatesPromise = firstValueFrom(result$.pipe(toArray()));
581+
582+
// Snapshot does NOT contain the reasoning message (backends don't include them)
583+
events$.next({
584+
type: EventType.MESSAGES_SNAPSHOT,
585+
messages: [
586+
{ id: "m1", role: "user", content: "hello" },
587+
{ id: "m2", role: "assistant", content: "hi there" },
588+
],
589+
} as MessagesSnapshotEvent);
590+
591+
events$.complete();
592+
const updates = await updatesPromise;
593+
594+
const msgs = updates[0]?.messages!;
595+
expect(msgs.length).toBe(3);
596+
expect(msgs.map((m) => m.id)).toEqual(["m1", "r1", "m2"]);
597+
expect(msgs[1].role).toBe("reasoning");
598+
expect(msgs[1].content).toBe("Let me think about this...");
599+
});
600+
601+
it("preserves both activity and reasoning messages after MESSAGES_SNAPSHOT", async () => {
602+
const initial: Message[] = [
603+
{ id: "m1", role: "user", content: "explain this" },
604+
{ id: "act-1", role: "activity", activityType: "PLAN", content: { tasks: ["research"] } },
605+
{ id: "r1", role: "reasoning", content: "The user wants an explanation..." },
606+
{ id: "m2", role: "assistant", content: "Here is the explanation" },
607+
] as Message[];
608+
609+
const events$ = new Subject<BaseEvent>();
610+
const agent = createAgent(initial);
611+
const result$ = defaultApplyEvents(makeInput(initial), events$, agent, []);
612+
const updatesPromise = firstValueFrom(result$.pipe(toArray()));
613+
614+
events$.next({
615+
type: EventType.MESSAGES_SNAPSHOT,
616+
messages: [
617+
{ id: "m1", role: "user", content: "explain this" },
618+
{ id: "m2", role: "assistant", content: "Here is the explanation" },
619+
],
620+
} as MessagesSnapshotEvent);
621+
622+
events$.complete();
623+
const updates = await updatesPromise;
624+
625+
const msgs = updates[0]?.messages!;
626+
expect(msgs.length).toBe(4);
627+
expect(msgs.map((m) => m.id)).toEqual(["m1", "act-1", "r1", "m2"]);
628+
expect(msgs[1].role).toBe("activity");
629+
expect(msgs[2].role).toBe("reasoning");
630+
});
631+
632+
it("reasoning messages are not replaced by snapshot data", async () => {
633+
// Reasoning messages should be kept as-is, never overwritten by snapshot
634+
const initial: Message[] = [
635+
{ id: "m1", role: "user", content: "hello" },
636+
{ id: "r1", role: "reasoning", content: "original reasoning" },
637+
{ id: "m2", role: "assistant", content: "response" },
638+
] as Message[];
639+
640+
const events$ = new Subject<BaseEvent>();
641+
const agent = createAgent(initial);
642+
const result$ = defaultApplyEvents(makeInput(initial), events$, agent, []);
643+
const updatesPromise = firstValueFrom(result$.pipe(toArray()));
644+
645+
// Even if snapshot somehow had the same ID, reasoning should be preserved as-is
646+
events$.next({
647+
type: EventType.MESSAGES_SNAPSHOT,
648+
messages: [
649+
{ id: "m1", role: "user", content: "hello" },
650+
{ id: "m2", role: "assistant", content: "response" },
651+
],
652+
} as MessagesSnapshotEvent);
653+
654+
events$.complete();
655+
const updates = await updatesPromise;
656+
657+
const msgs = updates[0]?.messages!;
658+
const reasoning = msgs.find((m) => m.id === "r1")!;
659+
expect(reasoning.role).toBe("reasoning");
660+
expect(reasoning.content).toBe("original reasoning");
661+
});
662+
570663
it("preserves activity position when a message ID changes in snapshot", async () => {
571664
// Simulates the real-world scenario: streaming creates a tool message with ID "tool-stream",
572665
// but MESSAGES_SNAPSHOT has the same tool message with a different canonical ID "tool-canon".

sdks/typescript/packages/client/src/apply/default.ts

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -540,14 +540,17 @@ export const defaultApplyEvents = (
540540
const { messages: newMessages } = event as MessagesSnapshotEvent;
541541

542542
// Edit-based merge: update existing messages with snapshot data while
543-
// preserving activity messages (which the backend doesn't know about).
543+
// preserving client-only messages (activity and reasoning) which the
544+
// backend doesn't include in the snapshot.
545+
const isClientOnlyRole = (role: string) =>
546+
role === "activity" || role === "reasoning";
544547
const snapshotMap = new Map(newMessages.map((m) => [m.id, m]));
545548

546-
// Step 1 + 2: Keep activity messages as-is, keep messages present in
547-
// the snapshot (replaced with snapshot version), drop everything else.
549+
// Step 1 + 2: Keep client-only messages as-is, keep messages present
550+
// in the snapshot (replaced with snapshot version), drop everything else.
548551
messages = messages
549-
.filter((m) => m.role === "activity" || snapshotMap.has(m.id))
550-
.map((m) => (m.role === "activity" ? m : snapshotMap.get(m.id)!));
552+
.filter((m) => isClientOnlyRole(m.role) || snapshotMap.has(m.id))
553+
.map((m) => (isClientOnlyRole(m.role) ? m : snapshotMap.get(m.id)!));
551554

552555
// Step 3: Append messages from the snapshot that we don't have yet.
553556
const existingIds = new Set(messages.map((m) => m.id));

0 commit comments

Comments
 (0)