diff options
Diffstat (limited to 'packages')
| -rw-r--r-- | packages/conversation-store/src/reconcile.test.ts | 14 | ||||
| -rw-r--r-- | packages/conversation-store/src/reconcile.ts | 2 | ||||
| -rw-r--r-- | packages/conversation-store/src/store.test.ts | 62 | ||||
| -rw-r--r-- | packages/conversation-store/src/store.ts | 10 |
4 files changed, 86 insertions, 2 deletions
diff --git a/packages/conversation-store/src/reconcile.test.ts b/packages/conversation-store/src/reconcile.test.ts index b1926e9..25b47d5 100644 --- a/packages/conversation-store/src/reconcile.test.ts +++ b/packages/conversation-store/src/reconcile.test.ts @@ -417,4 +417,18 @@ describe("reconcile", () => { expect(report.repairedCount).toBe(1); expect(report.repairedToolCallIds).toEqual(["call_orph"]); }); + + it("reconcile preserves a thinking-only assistant message", () => { + // Regression: an assistant message with only a thinking chunk (no text, + // no tool-call) was being dropped by the hasContent check. Thinking IS + // valid content — the model's reasoning must survive a load/reconcile + // cycle so it appears in the conversation history. + const messages: ChatMessage[] = [ + { role: "user", chunks: [{ type: "text", text: "hello" }] }, + { role: "assistant", chunks: [{ type: "thinking", text: "just thinking..." }] }, + ]; + const { messages: result, report } = reconcileWithReport(messages); + expect(result).toEqual(messages); + expect(report.droppedEmptyMessages).toBe(0); + }); }); diff --git a/packages/conversation-store/src/reconcile.ts b/packages/conversation-store/src/reconcile.ts index 9023415..2d2b68d 100644 --- a/packages/conversation-store/src/reconcile.ts +++ b/packages/conversation-store/src/reconcile.ts @@ -46,7 +46,7 @@ export function reconcileWithReport(messages: readonly ChatMessage[]): Reconcile for (const msg of stripped) { if (msg.role === "assistant") { const hasContent = msg.chunks.some( - (chunk) => chunk.type === "text" || chunk.type === "tool-call", + (chunk) => chunk.type === "text" || chunk.type === "tool-call" || chunk.type === "thinking", ); if (!hasContent) { droppedEmptyMessages++; diff --git a/packages/conversation-store/src/store.test.ts b/packages/conversation-store/src/store.test.ts index 6b12d0a..d336e0e 100644 --- a/packages/conversation-store/src/store.test.ts +++ b/packages/conversation-store/src/store.test.ts @@ -143,6 +143,68 @@ describe("ConversationStore", () => { expect(result).toEqual([...turn1, ...turn2]); }); + it("preserves message boundaries across single-message appends (orchestrator pattern)", async () => { + // Regression: the orchestrator persists messages one at a time — + // append([user]) at turn start, then append([assistant]) or + // append([assistant, ...toolResults]) via onStepComplete. Since each + // append() call assigns msgIdx starting at 0, single-message appends all + // share msgIdx=0. load() must split on role changes too, not just msgIdx, + // or messages from different turns collapse into one. + const store = createConversationStore(storage); + const user1: ChatMessage = { role: "user", chunks: [{ type: "text", text: "hello" }] }; + const asst1: ChatMessage = { + role: "assistant", + chunks: [ + { type: "thinking", text: "greeting" }, + { type: "text", text: "Hi!" }, + ], + }; + const user2: ChatMessage = { role: "user", chunks: [{ type: "text", text: "read a file" }] }; + const asst2: ChatMessage = { + role: "assistant", + chunks: [{ type: "text", text: "Sure." }], + }; + const user3: ChatMessage = { role: "user", chunks: [{ type: "text", text: "thanks" }] }; + + // Each message appended individually — the real orchestrator pattern. + await store.append("conv1", [user1]); + await store.append("conv1", [asst1]); + await store.append("conv1", [user2]); + await store.append("conv1", [asst2]); + await store.append("conv1", [user3]); + + const result = await store.load("conv1"); + expect(result).toEqual([user1, asst1, user2, asst2, user3]); + }); + + it("preserves message boundaries with single-message appends + multi-message step (tool calls)", async () => { + // Regression: the full orchestrator pattern including tool calls. + // Turn: append([user]) → step: append([assistant{thinking,text,tool-call}, toolResult]) + // All single-message appends get msgIdx=0; the multi-message step gets 0,1. + const store = createConversationStore(storage); + const user: ChatMessage = { role: "user", chunks: [{ type: "text", text: "do it" }] }; + const asst: ChatMessage = { + role: "assistant", + chunks: [ + { type: "thinking", text: "calling a tool" }, + { type: "text", text: "ok" }, + { type: "tool-call", toolCallId: "c1", toolName: "t", input: {} }, + ], + }; + const toolResult: ChatMessage = { + role: "tool", + chunks: [ + { type: "tool-result", toolCallId: "c1", toolName: "t", content: "done", isError: false }, + ], + }; + + await store.append("conv1", [user]); + await store.append("conv1", [asst, toolResult]); + + const result = await store.load("conv1"); + expect(result).toEqual([user, asst, toolResult]); + }); + it("preserves message ordering", async () => { const store = createConversationStore(storage); const messages: ChatMessage[] = []; diff --git a/packages/conversation-store/src/store.ts b/packages/conversation-store/src/store.ts index ed39d8e..41df92f 100644 --- a/packages/conversation-store/src/store.ts +++ b/packages/conversation-store/src/store.ts @@ -681,7 +681,15 @@ export function createConversationStore( continue; } - if (entry.msgIdx !== currentMsgIdx) { + // A message boundary is detected when EITHER the msgIdx changes OR the + // role changes. The msgIdx alone is insufficient because append() assigns + // it as a LOCAL index (reset to 0 for each call) — so consecutive + // single-message appends (e.g. the orchestrator's per-step persistence: + // append([user]) then append([assistant]) then append([user])...) all + // share msgIdx=0 and would collapse into one message without the role + // check. The role check restores correct boundaries for the common + // alternating user/assistant/tool pattern. + if (entry.msgIdx !== currentMsgIdx || entry.role !== currentRole) { if (currentMsgIdx >= 0 && currentRole !== undefined) { messages.push({ role: currentRole, chunks: currentChunks }); } |
