diff options
| author | Adam Malczewski <[email protected]> | 2026-10-21 14:52:42 +0900 |
|---|---|---|
| committer | Adam Malczewski <[email protected]> | 2026-10-21 14:52:42 +0900 |
| commit | e9786bf1dbf258b68f793870d02378c390939d3d (patch) | |
| tree | 4cecca2c569ce374aabf071cfd33976c936f850e /src | |
| parent | 0bdb04b0aa0c0500e68e2e985b1a2880730ad7e8 (diff) | |
| download | dispatch-web-e9786bf1dbf258b68f793870d02378c390939d3d.tar.gz dispatch-web-e9786bf1dbf258b68f793870d02378c390939d3d.zip | |
fix(review-followup-fed): reconcile compaction-queued send acknowledgements
Diffstat (limited to 'src')
| -rw-r--r-- | src/adapters/ws/index.test.ts | 23 | ||||
| -rw-r--r-- | src/adapters/ws/index.ts | 7 | ||||
| -rw-r--r-- | src/adapters/ws/logic.test.ts | 32 | ||||
| -rw-r--r-- | src/adapters/ws/logic.ts | 16 | ||||
| -rw-r--r-- | src/app/chat-registry.svelte.ts | 1 | ||||
| -rw-r--r-- | src/app/store.test.ts | 85 | ||||
| -rw-r--r-- | src/app/ws-handlers.ts | 5 | ||||
| -rw-r--r-- | src/core/chunks/index.ts | 1 | ||||
| -rw-r--r-- | src/core/chunks/local-echo.test.ts | 45 | ||||
| -rw-r--r-- | src/core/chunks/reducer.ts | 31 | ||||
| -rw-r--r-- | src/core/chunks/types.ts | 2 | ||||
| -rw-r--r-- | src/core/wire/conformance.test.ts | 7 | ||||
| -rw-r--r-- | src/core/wire/conformance.ts | 2 | ||||
| -rw-r--r-- | src/features/chat/queued.test.ts | 207 | ||||
| -rw-r--r-- | src/features/chat/store.svelte.ts | 20 | ||||
| -rw-r--r-- | src/features/chat/store.test.ts | 63 |
16 files changed, 536 insertions, 11 deletions
diff --git a/src/adapters/ws/index.test.ts b/src/adapters/ws/index.test.ts index 6f5e4ef..6461493 100644 --- a/src/adapters/ws/index.test.ts +++ b/src/adapters/ws/index.test.ts @@ -476,3 +476,26 @@ describe("reconnect backoff after open", () => { } }); }); + +it("routes chat.queued only through the typed chat callback, never surface/error handling", () => { + const ws = fakeSocket(); + const onChat = vi.fn(); + const onMessage = vi.fn(); + const handle = createSurfaceSocket({ + url: "ws://test", + socketFactory: () => ws, + onChat, + onMessage, + }); + const ack = { + type: "chat.queued", + conversationId: "c1", + reason: "compaction", + messageId: "q1", + requestId: "r1", + }; + ws.invokeMessage(JSON.stringify(ack)); + expect(onChat).toHaveBeenCalledExactlyOnceWith(ack); + expect(onMessage).not.toHaveBeenCalled(); + handle.close(); +}); diff --git a/src/adapters/ws/index.ts b/src/adapters/ws/index.ts index eeb7901..25e9911 100644 --- a/src/adapters/ws/index.ts +++ b/src/adapters/ws/index.ts @@ -1,6 +1,7 @@ import type { ChatDeltaMessage, ChatErrorMessage, + ChatQueuedMessage, ConversationCompactedMessage, ConversationOpenMessage, ConversationStatusChangedMessage, @@ -24,7 +25,7 @@ export interface WebSocketLike { export interface SurfaceSocketOptions { url: string; onMessage: (msg: SurfaceServerMessage) => void; - onChat?: (msg: ChatDeltaMessage | ChatErrorMessage) => void; + onChat?: (msg: ChatDeltaMessage | ChatErrorMessage | ChatQueuedMessage) => void; /** Broadcast when a conversation is "opened" (e.g. CLI `--open` flag). */ onConversationOpen?: (msg: ConversationOpenMessage) => void; /** Broadcast when a conversation's lifecycle status changes (active/idle/closed). */ @@ -100,8 +101,8 @@ export function createSurfaceSocket(opts: SurfaceSocketOptions): SurfaceSocketHa if (disposed) return; const msg = parseServerMessage(ev.data); if (msg !== null) { - if (msg.type === "chat.delta" || msg.type === "chat.error") { - opts.onChat?.(msg as ChatDeltaMessage | ChatErrorMessage); + if (msg.type === "chat.delta" || msg.type === "chat.error" || msg.type === "chat.queued") { + opts.onChat?.(msg as ChatDeltaMessage | ChatErrorMessage | ChatQueuedMessage); } else if (msg.type === "conversation.open") { opts.onConversationOpen?.(msg as ConversationOpenMessage); } else if (msg.type === "conversation.statusChanged") { diff --git a/src/adapters/ws/logic.test.ts b/src/adapters/ws/logic.test.ts index 4be2cd6..db6444a 100644 --- a/src/adapters/ws/logic.test.ts +++ b/src/adapters/ws/logic.test.ts @@ -577,3 +577,35 @@ it("preserves a compaction epoch and rejects absent or invalid epochs", () => { expect(parseServerMessage(JSON.stringify({ ...msg, historyEpoch }))).toBeNull(); } }); + +describe("chat.queued admission acknowledgement", () => { + const ack = { + type: "chat.queued", + conversationId: "c1", + reason: "compaction", + messageId: "q1", + }; + + it.each([undefined, "request-1"])("preserves optional requestId %s", (requestId) => { + const msg = { ...ack, ...(requestId !== undefined ? { requestId } : {}) }; + expect(parseServerMessage(JSON.stringify(msg))).toEqual(msg); + }); + + it.each([ + { conversationId: undefined }, + { conversationId: 12 }, + { messageId: undefined }, + { messageId: null }, + { reason: undefined }, + { reason: "other" }, + { requestId: null }, + { requestId: 12 }, + ])("rejects malformed acknowledgement %j", (bad) => { + expect(parseServerMessage(JSON.stringify({ ...ack, ...bad }))).toBeNull(); + }); + + it("serializes a correlated send", () => { + const msg = { type: "chat.send" as const, message: "hello", requestId: "r1" }; + expect(JSON.parse(serialize(msg))).toEqual(msg); + }); +}); diff --git a/src/adapters/ws/logic.ts b/src/adapters/ws/logic.ts index 858deaa..0efcd90 100644 --- a/src/adapters/ws/logic.ts +++ b/src/adapters/ws/logic.ts @@ -1,6 +1,7 @@ import type { ChatDeltaMessage, ChatErrorMessage, + ChatQueuedMessage, ConversationCompactedMessage, ConversationOpenMessage, ConversationStatusChangedMessage, @@ -25,6 +26,7 @@ const VALID_SERVER_TYPES = new Set([ "error", "chat.delta", "chat.error", + "chat.queued", "conversation.open", "conversation.statusChanged", "conversation.compacted", @@ -111,6 +113,20 @@ export function parseServerMessage(data: string): WsServerMessage | null { if (typeof event.type !== "string") return null; return { type: "chat.delta", event: event as unknown as ChatDeltaMessage["event"] }; } + case "chat.queued": { + if (typeof parsed.conversationId !== "string") return null; + if (typeof parsed.messageId !== "string") return null; + if (parsed.reason !== "compaction") return null; + if (parsed.requestId !== undefined && typeof parsed.requestId !== "string") return null; + const msg: ChatQueuedMessage = { + type: "chat.queued", + conversationId: parsed.conversationId, + messageId: parsed.messageId, + reason: "compaction", + ...(parsed.requestId !== undefined ? { requestId: parsed.requestId } : {}), + }; + return msg; + } case "chat.error": { if (typeof parsed.message !== "string") return null; const conversationId = parsed.conversationId; diff --git a/src/app/chat-registry.svelte.ts b/src/app/chat-registry.svelte.ts index 8e97af1..a9b28ff 100644 --- a/src/app/chat-registry.svelte.ts +++ b/src/app/chat-registry.svelte.ts @@ -78,6 +78,7 @@ export function createChatRegistry(deps: { function createChatFor(conversationId: string, model: string, workspaceId: string): ChatStore { return createChatStore({ conversationId, + createRequestId: randomId, model, workspaceId, transport: { diff --git a/src/app/store.test.ts b/src/app/store.test.ts index f257f3a..40a0fa7 100644 --- a/src/app/store.test.ts +++ b/src/app/store.test.ts @@ -2991,3 +2991,88 @@ it.each([ store.dispose(); } }); + +describe("compaction queued acknowledgement routing", () => { + it.each([ + "surface-first", + "ack-first", + "drained-before-ack", + ])("uses only authoritative queue snapshots: %s", (order) => { + const ws = fakeSocket(); + const store = createAppStore({ + socketFactory: () => ws, + fetchImpl: fakeFetchImpl(), + localStorage: createFakeStorage(), + chatFlushScheduler: null, + }); + ws.resolveOpen(); + store.send("queued text", [{ url: "data:image/png;base64,AAAA" }]); + const cid = activeConversationId(store); + const send = parseSent(ws).find((m) => (m as { type: string }).type === "chat.send") as { + requestId: string; + }; + expect(send.requestId).toEqual(expect.any(String)); + const sourceChat = store.activeChat; + const watch = store.watchConversation("other-conversation"); + watch.send("unrelated local echo"); + ws.feedSurfaceMessage({ + type: "catalog", + catalog: [{ id: "message-queue", region: "inline", title: "Queue", scope: "conversation" }], + }); + const spec = (messages: { id: string; text: string; timestamp: number }[]) => ({ + id: "message-queue", + region: "inline", + title: "Queue", + fields: [{ kind: "custom" as const, rendererId: "message-queue", payload: { messages } }], + }); + const queued = spec([{ id: "q1", text: "queued text", timestamp: 1 }]); + const drained = spec([]); + const ack = () => + ws.feedServerMessage({ + type: "chat.queued", + conversationId: cid, + reason: "compaction", + messageId: "q1", + requestId: send.requestId, + }); + if (order === "ack-first") { + ack(); + expect(store.surface("message-queue", cid)).toBeNull(); + } + ws.feedSurfaceMessage({ type: "surface", conversationId: cid, spec: queued }); + expect(store.surface("message-queue", cid)).toEqual(queued); + if (order === "drained-before-ack") { + ws.feedSurfaceMessage({ + type: "update", + update: { surfaceId: "message-queue", conversationId: cid, spec: drained }, + }); + ws.feedServerMessage({ + type: "chat.delta", + event: { type: "turn-start", conversationId: cid, turnId: "t1" }, + }); + ws.feedServerMessage({ + type: "chat.delta", + event: { type: "user-message", conversationId: cid, turnId: "t1", text: "queued text" }, + }); + } + ack(); + ack(); + expect(store.surface("message-queue", cid)).toEqual( + order === "drained-before-ack" ? drained : queued, + ); + expect(sourceChat.chunks.map((c) => c.chunk)).toEqual( + order === "drained-before-ack" ? [{ type: "text", text: "queued text" }] : [], + ); + expect(sourceChat.error).toBeNull(); + expect(watch.chunks.map((c) => c.chunk)).toEqual([ + { type: "text", text: "unrelated local echo" }, + ]); + expect(parseSent(ws).filter((m) => (m as { type: string }).type === "chat.send")).toHaveLength( + 2, + ); + expect(parseSent(ws).filter((m) => (m as { type: string }).type === "chat.queue")).toHaveLength( + 0, + ); + store.dispose(); + }); +}); diff --git a/src/app/ws-handlers.ts b/src/app/ws-handlers.ts index c76c9c1..aa1e03e 100644 --- a/src/app/ws-handlers.ts +++ b/src/app/ws-handlers.ts @@ -1,6 +1,7 @@ import type { ChatDeltaMessage, ChatErrorMessage, + ChatQueuedMessage, ConversationCompactedMessage, ConversationOpenMessage, ConversationStatusChangedMessage, @@ -52,7 +53,7 @@ export function createWsHandlers(deps: { layoutStore, tileLayoutStore, } = deps; - function handleChatMessage(msg: ChatDeltaMessage | ChatErrorMessage): void { + function handleChatMessage(msg: ChatDeltaMessage | ChatErrorMessage | ChatQueuedMessage): void { let targetId: string | undefined; if (msg.type === "chat.delta") { targetId = msg.event.conversationId; @@ -68,6 +69,8 @@ export function createWsHandlers(deps: { } } + if (msg.type === "chat.queued") return; + // fallback: try all stores (chat.error without conversationId) for (const store of registry.chatStores.values()) { store.handleDelta(msg); diff --git a/src/core/chunks/index.ts b/src/core/chunks/index.ts index 3822a79..49381d5 100644 --- a/src/core/chunks/index.ts +++ b/src/core/chunks/index.ts @@ -7,6 +7,7 @@ export { clearGenerating, foldEvent, initialState, + removeLocalEcho, } from "./reducer"; export type { ProviderRetryView } from "./retry-banner"; export { formatRetryDelay, viewProviderRetry } from "./retry-banner"; diff --git a/src/core/chunks/local-echo.test.ts b/src/core/chunks/local-echo.test.ts new file mode 100644 index 0000000..4b5eed4 --- /dev/null +++ b/src/core/chunks/local-echo.test.ts @@ -0,0 +1,45 @@ +import { describe, expect, it } from "vitest"; +import { appendUserMessage, applyHistory, foldEvent, initialState, removeLocalEcho } from "./index"; + +describe("request-local optimistic echo", () => { + it("retains identity through turn-start tags without conflating identical sends", () => { + let state = appendUserMessage(initialState(), "same", [{ url: "data:first" }], "r1"); + state = appendUserMessage(state, "same", [{ url: "data:second" }], "r2"); + state = foldEvent(state, { type: "turn-start", conversationId: "c1", turnId: "t1" }); + state = removeLocalEcho(state, "r1"); + expect(state.provisional.map((p) => p.localRequestId)).toEqual(["r2", "r2"]); + expect(state.provisional.map((p) => p.chunk)).toEqual([ + { type: "text", text: "same" }, + { type: "image", url: "data:second" }, + ]); + expect(state.generating).toBe(true); + expect(removeLocalEcho(state, "r1")).toBe(state); + }); + + it("a live user-message confirms only its deduplicated text, not every identical local send", () => { + let state = appendUserMessage(initialState(), "same", [{ url: "data:first" }], "r1"); + state = appendUserMessage(state, "same", [{ url: "data:second" }], "r2"); + state = foldEvent(state, { type: "turn-start", conversationId: "c1", turnId: "t1" }); + state = foldEvent(state, { + type: "user-message", + conversationId: "c1", + turnId: "t1", + text: "same", + }); + state = removeLocalEcho(removeLocalEcho(state, "r1"), "r2"); + expect(state.provisional).toEqual([ + { role: "user", turnId: "t1", chunk: { type: "text", text: "same" } }, + ]); + }); + + it("does not remove committed rows even if their content is identical", () => { + let state = appendUserMessage(initialState(), "same", undefined, "r1"); + state = applyHistory(state, [{ seq: 1, role: "user", chunk: { type: "text", text: "same" } }]); + const committed = state.committed; + state = removeLocalEcho(state, "r1"); + expect(state.committed).toBe(committed); + expect(state.provisional).toEqual([]); + expect(removeLocalEcho(state, "r1")).toBe(state); + expect(removeLocalEcho(initialState(), "r1")).toEqual(initialState()); + }); +}); diff --git a/src/core/chunks/reducer.ts b/src/core/chunks/reducer.ts index 0af2ddd..53af9c1 100644 --- a/src/core/chunks/reducer.ts +++ b/src/core/chunks/reducer.ts @@ -246,11 +246,17 @@ function reduceEvent(state: TranscriptState, event: AgentEvent): TranscriptState // text) emits no `user-message` and is not de-duped here. if (event.text.length === 0) return state; const run = trailingRun(state.provisional, "user"); - const alreadyEchoed = run.some((p) => p.chunk.type === "text" && p.chunk.text === event.text); - if (alreadyEchoed) { + const echoed = run.find((p) => p.chunk.type === "text" && p.chunk.text === event.text); + if (echoed !== undefined) { return { ...state, - provisional: tagTrailingUserEcho(state.provisional, event.turnId), + // Preserve the live text independently of the local send. A delayed + // queued ack may remove its images, but cannot erase this server event. + provisional: tagTrailingUserEcho(state.provisional, event.turnId).map((p) => { + if (p.chunk !== echoed.chunk || p.localRequestId === undefined) return p; + const { localRequestId: _local, ...confirmed } = p; + return confirmed; + }), generating: true, }; } @@ -463,6 +469,7 @@ export function appendUserMessage( state: TranscriptState, text: string, images?: readonly ImageInput[], + localRequestId?: string, ): TranscriptState { const provisional = flushAccumulating(state.provisional, state.accumulating); const userChunks: Chunk[] = []; @@ -484,7 +491,23 @@ export function appendUserMessage( } return { ...state, - provisional: [...provisional, ...userChunks.map((chunk) => ({ role: "user", chunk }) as const)], + provisional: [ + ...provisional, + ...userChunks.map( + (chunk) => + ({ + role: "user", + chunk, + ...(localRequestId !== undefined ? { localRequestId } : {}), + }) as const, + ), + ], accumulating: null, }; } + +/** Remove only still-local artifacts of one accepted queued send; never history/live text. */ +export function removeLocalEcho(state: TranscriptState, requestId: string): TranscriptState { + const provisional = state.provisional.filter((p) => p.localRequestId !== requestId); + return provisional.length === state.provisional.length ? state : { ...state, provisional }; +} diff --git a/src/core/chunks/types.ts b/src/core/chunks/types.ts index 949857a..f7bfe01 100644 --- a/src/core/chunks/types.ts +++ b/src/core/chunks/types.ts @@ -10,6 +10,8 @@ export interface AccumulatingChunk { /** A provisional chunk that has no authoritative seq yet. */ export interface ProvisionalChunk { + /** Local send identity only; turn-start tagging does NOT confirm this content. */ + readonly localRequestId?: string; readonly role: Role; readonly chunk: Chunk; /** diff --git a/src/core/wire/conformance.test.ts b/src/core/wire/conformance.test.ts index 1c5829b..1179fbc 100644 --- a/src/core/wire/conformance.test.ts +++ b/src/core/wire/conformance.test.ts @@ -174,6 +174,12 @@ describe("classifies every WsServerMessage type", () => { event: { type: "done" as const, conversationId: "c", turnId: "t", reason: "r" }, }, { type: "chat.error" as const, message: "e" }, + { + type: "chat.queued" as const, + conversationId: "conv-1", + reason: "compaction" as const, + messageId: "queued-1", + }, { type: "conversation.open" as const, conversationId: "c1", workspaceId: "w1" }, { type: "conversation.statusChanged" as const, @@ -198,6 +204,7 @@ describe("classifies every WsServerMessage type", () => { "error", "chat.delta", "chat.error", + "chat.queued", "conversation.open", "conversation.statusChanged", "conversation.compacted", diff --git a/src/core/wire/conformance.ts b/src/core/wire/conformance.ts index 9847081..78313f9 100644 --- a/src/core/wire/conformance.ts +++ b/src/core/wire/conformance.ts @@ -85,6 +85,8 @@ export function assertWsServerMessageExhaustive(msg: WsServerMessage): string { return "error"; case "chat.delta": return "chat.delta"; + case "chat.queued": + return "chat.queued"; case "chat.error": return "chat.error"; case "conversation.open": diff --git a/src/features/chat/queued.test.ts b/src/features/chat/queued.test.ts new file mode 100644 index 0000000..3b579c6 --- /dev/null +++ b/src/features/chat/queued.test.ts @@ -0,0 +1,207 @@ +import type { ChatQueuedMessage } from "@dispatch/transport-contract"; +import type { AgentEvent } from "@dispatch/wire"; +import { describe, expect, it, vi } from "vitest"; +import { createChatStore } from "./store.svelte"; +import { + createFakeCache, + createFakeHistorySync, + createFakeMetricsSync, + createFakeTransport, +} from "./test-helpers"; + +const images = [ + { url: "data:image/png;base64,AAAA", mimeType: "image/png" }, + { url: "data:image/png;base64,BBBB" }, +]; + +function fixture(batched = false) { + const transport = createFakeTransport(); + const history = createFakeHistorySync(); + let nextId = 0; + const scheduled: (() => void)[] = []; + const store = createChatStore({ + conversationId: "c1", + createRequestId: () => `r${++nextId}`, + transport: transport.impl, + historySync: history.impl, + metricsSync: createFakeMetricsSync().impl, + cache: createFakeCache().impl, + ...(batched ? { scheduleFlush: (flush: () => void) => scheduled.push(flush) } : {}), + }); + function ack(requestId: string | undefined = "r1", conversationId = "c1") { + const msg: ChatQueuedMessage = { + type: "chat.queued", + conversationId, + reason: "compaction", + messageId: "q1", + ...(requestId !== undefined ? { requestId } : {}), + }; + store.handleDelta(msg); + } + function event(event: AgentEvent) { + store.handleDelta({ type: "chat.delta", event }); + } + function start(text?: string, turnId = "t1") { + event({ type: "turn-start", conversationId: "c1", turnId }); + if (text !== undefined) event({ type: "user-message", conversationId: "c1", turnId, text }); + } + return { store, transport, history, scheduled, ack, event, start }; +} + +describe("compaction-deferred chat.send", () => { + it.each([ + "hello", + "", + ])("removes just the accepted local text/images for %j without error, retry or queue sends", (text) => { + const f = fixture(); + f.store.send(text, images); + expect(f.store.chunks).toHaveLength(text ? 3 : 2); + f.ack(); + f.ack(); + expect(f.store.chunks).toEqual([]); + expect(f.store.generating).toBe(false); + expect(f.store.error).toBeNull(); + expect(f.transport.sent).toHaveLength(1); + expect(f.transport.sent[0]).toMatchObject({ requestId: "r1", images, message: text }); + expect(f.transport.sentQueue).toEqual([]); + f.store.dispose(); + }); + + it("uses distinct request identities for identical sends and preserves unrelated content", () => { + const f = fixture(); + f.start("another client's prompt"); + f.event({ type: "text-delta", conversationId: "c1", turnId: "t1", delta: "working" }); + f.store.send("identical", images); + f.store.send("identical", images); + expect(f.transport.sent.map((m) => m.requestId)).toEqual(["r1", "r2"]); + f.ack("r1"); + f.ack("r1"); + expect(f.store.chunks.map((c) => c.chunk)).toEqual([ + { type: "text", text: "another client's prompt" }, + { type: "text", text: "working" }, + { type: "text", text: "identical" }, + ...images.map((i) => ({ type: "image", ...i })), + ]); + f.ack("r2"); + expect(f.store.chunks.map((c) => c.chunk)).toEqual([ + { type: "text", text: "another client's prompt" }, + { type: "text", text: "working" }, + ]); + expect(f.store.generating).toBe(true); + f.store.dispose(); + }); + + it.each([ + "missing", + "unknown", + "wrong-conversation", + ])("tolerates %s correlation without deleting any echo", (kind) => { + const f = fixture(); + f.store.send("hello", images); + f.store.handleDelta({ + type: "chat.queued", + conversationId: kind === "wrong-conversation" ? "c2" : "c1", + reason: "compaction", + messageId: "q1", + ...(kind === "missing" ? {} : { requestId: kind === "unknown" ? "r999" : "r1" }), + }); + expect(f.store.chunks).toHaveLength(3); + expect(f.store.error).toBeNull(); + f.store.dispose(); + }); + + it.each([ + false, + true, + ])("preserves a live confirmed prompt before delayed ack (batched=%s)", (batched) => { + const f = fixture(batched); + f.store.send("hello", images); + f.start("hello"); + f.event({ type: "text-delta", conversationId: "c1", turnId: "t1", delta: "answer" }); + f.ack(); + f.ack(); + for (const flush of f.scheduled) flush(); + expect(f.store.chunks.map((c) => c.chunk)).toEqual([ + { type: "text", text: "hello" }, + { type: "text", text: "answer" }, + ]); + expect(f.store.generating).toBe(true); + expect(f.store.error).toBeNull(); + f.store.dispose(); + }); + + it("turn-start tagging alone does not protect a still-local echo", () => { + const f = fixture(true); + f.store.send("hello", images); + f.start(); + f.ack(); + expect(f.store.chunks).toEqual([]); + expect(f.store.generating).toBe(true); + f.event({ type: "user-message", conversationId: "c1", turnId: "t1", text: "hello" }); + for (const flush of f.scheduled) flush(); + expect(f.store.chunks.map((c) => c.chunk)).toEqual([{ type: "text", text: "hello" }]); + f.store.dispose(); + }); + + it("keeps a combined queue opening and steering; removes only the original individual echoes", () => { + const f = fixture(true); + f.store.send("one", images); + f.store.send("two", images); + f.start("one\n\ntwo"); + f.event({ type: "steering", conversationId: "c1", turnId: "t1", text: "steer" }); + f.ack("r2"); + f.ack("r1"); + expect(f.store.chunks.map((c) => c.chunk)).toEqual([ + { type: "text", text: "one\n\ntwo" }, + { type: "text", text: "steer" }, + ]); + expect(f.store.generating).toBe(true); + f.store.dispose(); + }); + + it.each([ + "ack-first", + "reset-first", + "history-first", + ])("handles epoch replacement idempotently: %s", async (order) => { + const f = fixture(); + await f.store.load(); + f.store.send("hello", images); + if (order === "ack-first") f.ack(); + f.history.historyEpoch = 1; + f.history.returnChunks = [ + { seq: 1, role: "user", turnId: "t1", chunk: { type: "text", text: "hello" } }, + { seq: 2, role: "user", turnId: "t1", chunk: { type: "image", url: "/images/c1/saved.png" } }, + ]; + f.store.resync(1); + if (order === "reset-first") f.ack(); + await vi.waitFor(() => expect(f.store.pendingSync).toBe(false)); + f.start("hello"); + const before = f.store.chunks; + f.ack(); + f.ack(); + expect(f.store.chunks).toEqual(before); + expect(f.store.chunks.map((c) => c.chunk)).toEqual(f.history.returnChunks.map((c) => c.chunk)); + expect(f.store.generating).toBe(true); + expect(f.store.error).toBeNull(); + expect(f.transport.sent).toHaveLength(1); + f.store.dispose(); + }); + + it("keeps normal immediate-send text/image deduplication and authoritative history", async () => { + const f = fixture(); + f.store.send("hello", images); + f.start("hello"); + expect(f.store.chunks).toHaveLength(3); + f.history.returnChunks = [ + { seq: 1, role: "user", turnId: "t1", chunk: { type: "text", text: "hello" } }, + { seq: 2, role: "user", turnId: "t1", chunk: { type: "image", url: "/images/c1/saved.png" } }, + ]; + f.event({ type: "turn-sealed", conversationId: "c1", turnId: "t1" }); + await vi.waitFor(() => expect(f.store.pendingSync).toBe(false)); + expect(f.store.chunks.map((c) => c.chunk)).toEqual(f.history.returnChunks.map((c) => c.chunk)); + expect(f.store.chunks.every((c) => !c.provisional)).toBe(true); + expect(f.transport.sent).toHaveLength(1); + f.store.dispose(); + }); +}); diff --git a/src/features/chat/store.svelte.ts b/src/features/chat/store.svelte.ts index f48d441..66ee5ee 100644 --- a/src/features/chat/store.svelte.ts +++ b/src/features/chat/store.svelte.ts @@ -2,6 +2,7 @@ import type { ChatDeltaMessage, ChatErrorMessage, ChatQueueCancelMessage, + ChatQueuedMessage, ChatQueueMessage, ChatSendMessage, } from "@dispatch/transport-contract"; @@ -15,6 +16,7 @@ import { initialState, initialWindowSize, normalizeChatLimit, + removeLocalEcho, restoreEarlier, selectChunks, selectGenerating, @@ -43,6 +45,8 @@ import type { ChatTransport, HistorySync, MetricsSync } from "./ports"; export interface ChatStoreDependencies { readonly conversationId: string; + /** Injected unique client-local correlation token for each new send. */ + readonly createRequestId: () => string; readonly model?: string; readonly transport: ChatTransport; readonly historySync: HistorySync; @@ -142,7 +146,7 @@ export interface ChatStore { * Ignored while a turn is generating; a running turn gap keeps the later start. */ noteLastActivity(at: number): void; - handleDelta(msg: ChatDeltaMessage | ChatErrorMessage): void; + handleDelta(msg: ChatDeltaMessage | ChatErrorMessage | ChatQueuedMessage): void; /** * Send a user message (start a turn via `chat.send`). Optimistically echoes * the text + any `images` as provisional user chunks (`[text, image, …]` in @@ -540,7 +544,7 @@ export function createChatStore(deps: ChatStoreDependencies): ChatStore { cacheTimer = seedTurnGap(cacheTimer, at); }, - handleDelta(msg: ChatDeltaMessage | ChatErrorMessage): void { + handleDelta(msg: ChatDeltaMessage | ChatErrorMessage | ChatQueuedMessage): void { if (msg.type === "chat.error") { if (msg.conversationId !== undefined && msg.conversationId !== deps.conversationId) { return; @@ -548,6 +552,14 @@ export function createChatStore(deps: ChatStoreDependencies): ChatStore { _error = msg.message; return; } + if (msg.type === "chat.queued") { + if (disposed || msg.conversationId !== deps.conversationId) return; + // A queued live opening may already have confirmed the echoed text. + // Fold it first; turn-start alone must not protect still-local artifacts. + flushEvents(); + if (msg.requestId !== undefined) transcript = removeLocalEcho(transcript, msg.requestId); + return; + } if (msg.event.conversationId !== deps.conversationId) { return; } @@ -566,10 +578,12 @@ export function createChatStore(deps: ChatStoreDependencies): ChatStore { flushEvents(); // Sending ends the wait since the last turn: stamp it on that turn. cacheTimer = stopTurnGap(cacheTimer, now(), () => lastTurnIdOf(metrics)); - transcript = appendUserMessage(transcript, text, images); + const requestId = deps.createRequestId(); + transcript = appendUserMessage(transcript, text, images, requestId); maybeTrim(); const msg: ChatSendMessage = { type: "chat.send", + requestId, conversationId: deps.conversationId, message: text, ...(_model !== undefined ? { model: _model } : {}), diff --git a/src/features/chat/store.test.ts b/src/features/chat/store.test.ts index de84b8a..c24592d 100644 --- a/src/features/chat/store.test.ts +++ b/src/features/chat/store.test.ts @@ -32,6 +32,7 @@ describe("createChatStore", () => { const metricsSync = createFakeMetricsSync(); const cache = createFakeCache(); const store = createChatStore({ + createRequestId: () => crypto.randomUUID(), conversationId: CONV_ID, transport: transport.impl, historySync: historySync.impl, @@ -64,6 +65,7 @@ describe("createChatStore", () => { const metricsSync = createFakeMetricsSync(); const cache = createFakeCache(); const store = createChatStore({ + createRequestId: () => crypto.randomUUID(), conversationId: CONV_ID, transport: transport.impl, historySync: historySync.impl, @@ -107,6 +109,7 @@ describe("createChatStore", () => { const metricsSync = createFakeMetricsSync(); const cache = createFakeCache(); const store = createChatStore({ + createRequestId: () => crypto.randomUUID(), conversationId: CONV_ID, transport: transport.impl, historySync: historySync.impl, @@ -131,6 +134,7 @@ describe("createChatStore", () => { const metricsSync = createFakeMetricsSync(); const cache = createFakeCache(); const store = createChatStore({ + createRequestId: () => crypto.randomUUID(), conversationId: CONV_ID, model: "openai/gpt-4", transport: transport.impl, @@ -153,6 +157,7 @@ describe("createChatStore", () => { const metricsSync = createFakeMetricsSync(); const cache = createFakeCache(); const store = createChatStore({ + createRequestId: () => crypto.randomUUID(), conversationId: CONV_ID, transport: transport.impl, historySync: historySync.impl, @@ -185,6 +190,7 @@ describe("createChatStore", () => { it("send omits images on the wire when none are staged (backward compatible)", () => { const transport = createFakeTransport(); const store = createChatStore({ + createRequestId: () => crypto.randomUUID(), conversationId: CONV_ID, transport: transport.impl, historySync: createFakeHistorySync().impl, @@ -201,6 +207,7 @@ describe("createChatStore", () => { it("send omits images on the wire for an empty array", () => { const transport = createFakeTransport(); const store = createChatStore({ + createRequestId: () => crypto.randomUUID(), conversationId: CONV_ID, transport: transport.impl, historySync: createFakeHistorySync().impl, @@ -217,6 +224,7 @@ describe("createChatStore", () => { it("send allows an images-only message (empty text)", () => { const transport = createFakeTransport(); const store = createChatStore({ + createRequestId: () => crypto.randomUUID(), conversationId: CONV_ID, transport: transport.impl, historySync: createFakeHistorySync().impl, @@ -241,6 +249,7 @@ describe("createChatStore", () => { const metricsSync = createFakeMetricsSync(); const cache = createFakeCache(); const store = createChatStore({ + createRequestId: () => crypto.randomUUID(), conversationId: CONV_ID, transport: transport.impl, historySync: historySync.impl, @@ -263,6 +272,7 @@ describe("createChatStore", () => { it.each([" describe this ", "", " "])("queues images with text %j", (text) => { const transport = createFakeTransport(); const store = createChatStore({ + createRequestId: () => crypto.randomUUID(), conversationId: CONV_ID, workspaceId: "images-workspace", transport: transport.impl, @@ -298,6 +308,7 @@ describe("createChatStore", () => { const metricsSync = createFakeMetricsSync(); const cache = createFakeCache(); const store = createChatStore({ + createRequestId: () => crypto.randomUUID(), conversationId: CONV_ID, transport: transport.impl, historySync: historySync.impl, @@ -318,6 +329,7 @@ describe("createChatStore", () => { const metricsSync = createFakeMetricsSync(); const cache = createFakeCache(); const store = createChatStore({ + createRequestId: () => crypto.randomUUID(), conversationId: CONV_ID, transport: transport.impl, historySync: historySync.impl, @@ -340,6 +352,7 @@ describe("createChatStore", () => { const metricsSync = createFakeMetricsSync(); const cache = createFakeCache(); const store = createChatStore({ + createRequestId: () => crypto.randomUUID(), conversationId: CONV_ID, transport: transport.impl, historySync: historySync.impl, @@ -362,6 +375,7 @@ describe("createChatStore", () => { const metricsSync = createFakeMetricsSync(); const cache = createFakeCache(); const store = createChatStore({ + createRequestId: () => crypto.randomUUID(), conversationId: CONV_ID, transport: transport.impl, historySync: historySync.impl, @@ -387,6 +401,7 @@ describe("createChatStore", () => { const metricsSync = createFakeMetricsSync(); const cache = createFakeCache(); const store = createChatStore({ + createRequestId: () => crypto.randomUUID(), conversationId: CONV_ID, transport: transport.impl, historySync: historySync.impl, @@ -408,6 +423,7 @@ describe("createChatStore", () => { const metricsSync = createFakeMetricsSync(); const cache = createFakeCache(); const store = createChatStore({ + createRequestId: () => crypto.randomUUID(), conversationId: CONV_ID, transport: transport.impl, historySync: historySync.impl, @@ -433,6 +449,7 @@ describe("createChatStore", () => { const metricsSync = createFakeMetricsSync(); const cache = createFakeCache(); const store = createChatStore({ + createRequestId: () => crypto.randomUUID(), conversationId: CONV_ID, transport: transport.impl, historySync: historySync.impl, @@ -462,6 +479,7 @@ describe("createChatStore", () => { historySync.returnChunks = [makeStoredChunk(3, "assistant")]; const store = createChatStore({ + createRequestId: () => crypto.randomUUID(), conversationId: CONV_ID, transport: transport.impl, historySync: historySync.impl, @@ -490,6 +508,7 @@ describe("createChatStore", () => { historySync.returnChunks = [makeStoredChunk(1, "assistant")]; const store = createChatStore({ + createRequestId: () => crypto.randomUUID(), conversationId: CONV_ID, transport: transport.impl, historySync: historySync.impl, @@ -511,6 +530,7 @@ describe("createChatStore", () => { const metricsSync = createFakeMetricsSync(); const cache = createFakeCache(); const store = createChatStore({ + createRequestId: () => crypto.randomUUID(), conversationId: CONV_ID, transport: transport.impl, historySync: historySync.impl, @@ -543,6 +563,7 @@ describe("createChatStore", () => { const metricsSync = createFakeMetricsSync(); const cache = createFakeCache(); const store = createChatStore({ + createRequestId: () => crypto.randomUUID(), conversationId: CONV_ID, transport: transport.impl, historySync: historySync.impl, @@ -587,6 +608,7 @@ describe("createChatStore", () => { }; const store = createChatStore({ + createRequestId: () => crypto.randomUUID(), conversationId: CONV_ID, transport: transport.impl, historySync: slowHistorySync, @@ -621,6 +643,7 @@ describe("createChatStore", () => { const metricsSync = createFakeMetricsSync(); const cache = createFakeCache(); const store = createChatStore({ + createRequestId: () => crypto.randomUUID(), conversationId: CONV_ID, transport: transport.impl, historySync: historySync.impl, @@ -666,6 +689,7 @@ describe("createChatStore", () => { const metricsSync = createFakeMetricsSync(); const cache = createFakeCache(); const store = createChatStore({ + createRequestId: () => crypto.randomUUID(), conversationId: CONV_ID, model: "openai/gpt-4", transport: transport.impl, @@ -690,6 +714,7 @@ describe("createChatStore", () => { const metricsSync = createFakeMetricsSync(); const cache = createFakeCache(); const store = createChatStore({ + createRequestId: () => crypto.randomUUID(), conversationId: CONV_ID, transport: transport.impl, historySync: historySync.impl, @@ -713,6 +738,7 @@ describe("createChatStore", () => { const metricsSync = createFakeMetricsSync(); const cache = createFakeCache(); const store = createChatStore({ + createRequestId: () => crypto.randomUUID(), conversationId: CONV_ID, transport: transport.impl, historySync: historySync.impl, @@ -743,6 +769,7 @@ describe("createChatStore", () => { const metricsSync = createFakeMetricsSync(); const cache = createFakeCache(); const store = createChatStore({ + createRequestId: () => crypto.randomUUID(), conversationId: CONV_ID, transport: transport.impl, historySync: historySync.impl, @@ -763,6 +790,7 @@ describe("createChatStore", () => { const metricsSync = createFakeMetricsSync(); const cache = createFakeCache(); const store = createChatStore({ + createRequestId: () => crypto.randomUUID(), conversationId: CONV_ID, transport: transport.impl, historySync: historySync.impl, @@ -787,6 +815,7 @@ describe("createChatStore", () => { const metricsSync = createFakeMetricsSync(); const cache = createFakeCache(); const store = createChatStore({ + createRequestId: () => crypto.randomUUID(), conversationId: CONV_ID, transport: transport.impl, historySync: historySync.impl, @@ -825,6 +854,7 @@ describe("createChatStore", () => { const metricsSync = createFakeMetricsSync(); const cache = createFakeCache(); const store = createChatStore({ + createRequestId: () => crypto.randomUUID(), conversationId: CONV_ID, transport: transport.impl, historySync: historySync.impl, @@ -886,6 +916,7 @@ describe("createChatStore", () => { const metricsSync = createFakeMetricsSync(); const cache = createFakeCache(); const store = createChatStore({ + createRequestId: () => crypto.randomUUID(), conversationId: CONV_ID, transport: transport.impl, historySync: historySync.impl, @@ -930,6 +961,7 @@ describe("createChatStore", () => { const metricsSync = createFakeMetricsSync(); const cache = createFakeCache(); const store = createChatStore({ + createRequestId: () => crypto.randomUUID(), conversationId: CONV_ID, transport: transport.impl, historySync: historySync.impl, @@ -1002,6 +1034,7 @@ describe("createChatStore", () => { const metricsSync = createFakeMetricsSync(); const cache = createFakeCache(); const store = createChatStore({ + createRequestId: () => crypto.randomUUID(), conversationId: CONV_ID, transport: transport.impl, historySync: historySync.impl, @@ -1068,6 +1101,7 @@ describe("createChatStore", () => { ]; const store = createChatStore({ + createRequestId: () => crypto.randomUUID(), conversationId: CONV_ID, transport: transport.impl, historySync: historySync.impl, @@ -1092,6 +1126,7 @@ describe("createChatStore", () => { const metricsSync = createFakeMetricsSync(); const cache = createFakeCache(); const store = createChatStore({ + createRequestId: () => crypto.randomUUID(), conversationId: CONV_ID, transport: transport.impl, historySync: historySync.impl, @@ -1123,6 +1158,7 @@ describe("createChatStore", () => { const metricsSync = createFakeMetricsSync(); const cache = createFakeCache(); const store = createChatStore({ + createRequestId: () => crypto.randomUUID(), conversationId: CONV_ID, transport: transport.impl, historySync: historySync.impl, @@ -1147,6 +1183,7 @@ describe("createChatStore", () => { const metricsSync = createFakeMetricsSync(); const cache = createFakeCache(); const store = createChatStore({ + createRequestId: () => crypto.randomUUID(), conversationId: CONV_ID, transport: transport.impl, historySync: historySync.impl, @@ -1181,6 +1218,7 @@ describe("createChatStore", () => { const metricsSync = createFakeMetricsSync(); const cache = createFakeCache(); const store = createChatStore({ + createRequestId: () => crypto.randomUUID(), conversationId: CONV_ID, transport: transport.impl, historySync: historySync.impl, @@ -1227,6 +1265,7 @@ describe("createChatStore", () => { const cache = createFakeCache(); let atBottom = false; // reader scrolled up const store = createChatStore({ + createRequestId: () => crypto.randomUUID(), conversationId: CONV_ID, transport: transport.impl, historySync: historySync.impl, @@ -1281,6 +1320,7 @@ describe("createChatStore", () => { const cache = createFakeCache(); let atBottom = false; const store = createChatStore({ + createRequestId: () => crypto.randomUUID(), conversationId: CONV_ID, transport: transport.impl, historySync: historySync.impl, @@ -1319,6 +1359,7 @@ describe("createChatStore", () => { ); const store = createChatStore({ + createRequestId: () => crypto.randomUUID(), conversationId: CONV_ID, transport: transport.impl, historySync: historySync.impl, @@ -1348,6 +1389,7 @@ describe("createChatStore", () => { historySync.returnChunks = Array.from({ length: 500 }, (_, i) => makeStoredChunk(i + 1)); const store = createChatStore({ + createRequestId: () => crypto.randomUUID(), conversationId: CONV_ID, transport: transport.impl, historySync: historySync.impl, @@ -1383,6 +1425,7 @@ describe("createChatStore", () => { historySync.returnChunks = [makeStoredChunk(3)]; const store = createChatStore({ + createRequestId: () => crypto.randomUUID(), conversationId: CONV_ID, transport: transport.impl, historySync: historySync.impl, @@ -1407,6 +1450,7 @@ describe("createChatStore", () => { historySync.returnChunks = Array.from({ length: 500 }, (_, i) => makeStoredChunk(i + 1)); const store = createChatStore({ + createRequestId: () => crypto.randomUUID(), conversationId: CONV_ID, transport: transport.impl, historySync: historySync.impl, @@ -1444,6 +1488,7 @@ describe("createChatStore", () => { ); const store = createChatStore({ + createRequestId: () => crypto.randomUUID(), conversationId: CONV_ID, transport: transport.impl, historySync: historySync.impl, @@ -1476,6 +1521,7 @@ describe("createChatStore", () => { ); const store = createChatStore({ + createRequestId: () => crypto.randomUUID(), conversationId: CONV_ID, transport: transport.impl, historySync: historySync.impl, @@ -1507,6 +1553,7 @@ describe("createChatStore", () => { ); const store = createChatStore({ + createRequestId: () => crypto.randomUUID(), conversationId: CONV_ID, transport: transport.impl, historySync: historySync.impl, @@ -1539,6 +1586,7 @@ describe("createChatStore", () => { const metricsSync = createFakeMetricsSync(); const cache = createFakeCache(); const store = createChatStore({ + createRequestId: () => crypto.randomUUID(), conversationId: CONV_ID, transport: transport.impl, historySync: historySync.impl, @@ -1576,6 +1624,7 @@ describe("createChatStore", () => { Array.from({ length: 200 }, (_, i) => makeStoredChunk(i + 1)), ); const store = createChatStore({ + createRequestId: () => crypto.randomUUID(), conversationId: CONV_ID, transport: transport.impl, historySync: historySync.impl, @@ -1607,6 +1656,7 @@ describe("createChatStore", () => { // Server holds 200; cold-cache load at limit 100 → window 75 → seqs 126..200. historySync.returnChunks = Array.from({ length: 200 }, (_, i) => makeStoredChunk(i + 1)); const store = createChatStore({ + createRequestId: () => crypto.randomUUID(), conversationId: CONV_ID, transport: transport.impl, historySync: historySync.impl, @@ -1634,6 +1684,7 @@ describe("createChatStore", () => { const metricsSync = createFakeMetricsSync(); const cache = createFakeCache(); const store = createChatStore({ + createRequestId: () => crypto.randomUUID(), conversationId: CONV_ID, transport: transport.impl, historySync: historySync.impl, @@ -1673,6 +1724,7 @@ describe("createChatStore", () => { Array.from({ length: 50 }, (_, i) => makeStoredChunk(i + 1)), ); const store = createChatStore({ + createRequestId: () => crypto.randomUUID(), conversationId: CONV_ID, transport: transport.impl, historySync: historySync.impl, @@ -1700,6 +1752,7 @@ describe("createChatStore", () => { const metricsSync = createFakeMetricsSync(); const cache = createFakeCache(); const store = createChatStore({ + createRequestId: () => crypto.randomUUID(), conversationId: CONV_ID, transport: transport.impl, historySync: historySync.impl, @@ -1729,6 +1782,7 @@ describe("createChatStore", () => { const metricsSync = createFakeMetricsSync(); const cache = createFakeCache(); const store = createChatStore({ + createRequestId: () => crypto.randomUUID(), conversationId: CONV_ID, transport: transport.impl, historySync: historySync.impl, @@ -1750,6 +1804,7 @@ describe("createChatStore — batched delta application (scheduleFlush)", () => const scheduled: (() => void)[] = []; const historySync = createFakeHistorySync(); const store = createChatStore({ + createRequestId: () => crypto.randomUUID(), conversationId: CONV_ID, transport: createFakeTransport().impl, historySync: historySync.impl, @@ -1831,6 +1886,7 @@ describe("createChatStore — batched delta application (scheduleFlush)", () => describe("createChatStore — cacheTimer", () => { function makeStore(now: () => number) { return createChatStore({ + createRequestId: () => crypto.randomUUID(), conversationId: CONV_ID, transport: createFakeTransport().impl, historySync: createFakeHistorySync().impl, @@ -1940,6 +1996,7 @@ describe("history epochs", () => { const cache = createFakeCache(); history.returnChunks = Array.from({ length: 20 }, (_, i) => makeStoredChunk(i + 1)); const store = createChatStore({ + createRequestId: () => crypto.randomUUID(), conversationId: CONV_ID, transport: createFakeTransport().impl, historySync: history.impl, @@ -1980,6 +2037,7 @@ it("a compaction broadcast during an in-flight fetch discards the old response", .mockImplementation(history.impl); const cache = createFakeCache(); const store = createChatStore({ + createRequestId: () => crypto.randomUUID(), conversationId: CONV_ID, transport: createFakeTransport().impl, historySync: sync, @@ -2006,6 +2064,7 @@ it("reloads unknown legacy cache epochs rather than merging pre-upgrade rows", a const history = createFakeHistorySync(); history.returnChunks = [makeStoredChunk(101)]; const store = createChatStore({ + createRequestId: () => crypto.randomUUID(), conversationId: CONV_ID, transport: createFakeTransport().impl, historySync: history.impl, @@ -2037,6 +2096,7 @@ describe("server epoch rollback", () => { const history = createFakeHistorySync(); history.historyEpoch = 5; const store = createChatStore({ + createRequestId: () => crypto.randomUUID(), conversationId: CONV_ID, transport: createFakeTransport().impl, historySync: history.impl, @@ -2083,6 +2143,7 @@ describe("server epoch rollback", () => { return { chunks: [], latestSeq: 0, historyEpoch: calls - 1 }; }; store = createChatStore({ + createRequestId: () => crypto.randomUUID(), conversationId: CONV_ID, transport: createFakeTransport().impl, historySync: sync, @@ -2140,6 +2201,7 @@ it.each([ }; }); const store = createChatStore({ + createRequestId: () => crypto.randomUUID(), conversationId: CONV_ID, transport: createFakeTransport().impl, historySync: sync, @@ -2182,6 +2244,7 @@ it("replaces an empty high-epoch cache with the server's empty lower epoch in on await cache.reset(CONV_ID, 8); const history = createFakeHistorySync(); const store = createChatStore({ + createRequestId: () => crypto.randomUUID(), conversationId: CONV_ID, transport: createFakeTransport().impl, historySync: history.impl, |
