From 1144a8027a3d0446e407f98c5cddc3a8c78831d5 Mon Sep 17 00:00:00 2001 From: Adam Malczewski Date: Sun, 7 Jun 2026 02:41:37 +0900 Subject: fix: optimistic user message echo + tabs persistence MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bug 1 (sent message didn't appear until turn end): the transcript only folded assistant AgentEvents, so the user's own message showed only after turn-sealed resync. Add core/chunks appendUserMessage() (provisional user chunk, superseded on history sync) and call it in chat send() — the message now renders instantly. Bug 2 (tabs didn't persist on refresh): the app passed { storage: undefined } to createLocalStore, which the adapter treats as a no-op store, so nothing was saved. Default to globalThis.localStorage. Regression test exercises the non-injected path. Also updated app store tests for the echo (assistant-vs-user chunk filtering). Verified: svelte-check 0/0, vitest 288 (stable x2), biome clean, build ok. --- src/app/store.svelte.ts | 2 +- src/app/store.test.ts | 66 ++++++++++++++++++++++++++++++++++----- src/core/chunks/index.ts | 2 +- src/core/chunks/reducer.test.ts | 63 ++++++++++++++++++++++++++++++++++++- src/core/chunks/reducer.ts | 16 ++++++++++ src/features/chat/store.svelte.ts | 2 ++ src/features/chat/store.test.ts | 58 ++++++++++++++++++++++++++++++++++ 7 files changed, 199 insertions(+), 10 deletions(-) (limited to 'src') diff --git a/src/app/store.svelte.ts b/src/app/store.svelte.ts index 07d850b..760c390 100644 --- a/src/app/store.svelte.ts +++ b/src/app/store.svelte.ts @@ -100,7 +100,7 @@ export function createAppStore(opts?: CreateAppStoreOptions): AppStore { const fetchImpl = opts?.fetchImpl ?? globalThis.fetch.bind(globalThis); const indexedDBFactory = opts?.indexedDB ?? globalThis.indexedDB; - const localStorageOpt = opts?.localStorage; + const localStorageOpt = opts?.localStorage ?? globalThis.localStorage; const storageAdapter = createLocalStore("dispatch.tabs", { storage: localStorageOpt, diff --git a/src/app/store.test.ts b/src/app/store.test.ts index dabc80d..86a21d6 100644 --- a/src/app/store.test.ts +++ b/src/app/store.test.ts @@ -388,9 +388,11 @@ describe("createAppStore", () => { }); expect(store.activeChat.chunks.length).toBeGreaterThan(0); - const textChunks = store.activeChat.chunks.filter((c) => c.chunk.type === "text"); - expect(textChunks).toHaveLength(1); - expect((textChunks[0]?.chunk as { type: "text"; text: string }).text).toBe("Hello world"); + const assistantChunks = store.activeChat.chunks.filter( + (c) => c.role === "assistant" && c.chunk.type === "text", + ); + expect(assistantChunks).toHaveLength(1); + expect((assistantChunks[0]?.chunk as { type: "text"; text: string }).text).toBe("Hello world"); store.dispose(); }); @@ -580,14 +582,19 @@ describe("createAppStore", () => { }); store.selectTab(convId1); - const textChunks1 = store.activeChat.chunks.filter((c) => c.chunk.type === "text"); - expect(textChunks1).toHaveLength(1); - expect((textChunks1[0]?.chunk as { type: "text"; text: string }).text).toBe( + const assistantChunks1 = store.activeChat.chunks.filter( + (c) => c.role === "assistant" && c.chunk.type === "text", + ); + expect(assistantChunks1).toHaveLength(1); + expect((assistantChunks1[0]?.chunk as { type: "text"; text: string }).text).toBe( "response to first", ); store.selectTab(convId2); - expect(store.activeChat.chunks).toEqual([]); + const assistantChunks2 = store.activeChat.chunks.filter( + (c) => c.role === "assistant" && c.chunk.type === "text", + ); + expect(assistantChunks2).toEqual([]); store.dispose(); }); @@ -654,6 +661,51 @@ describe("createAppStore", () => { store2.dispose(); }); + it("tabs persist to globalThis.localStorage when no storage is injected", () => { + const realLs = globalThis.localStorage; + const memLs = createFakeStorage(); + globalThis.localStorage = memLs; + try { + const ws1 = fakeSocket(); + const store = createAppStore({ + socketFactory: () => ws1, + fetchImpl: fakeFetchImpl(), + }); + ws1.resolveOpen(); + + store.send("persist via default"); + const convId = store.tabs[0]?.conversationId; + const title = store.tabs[0]?.title; + expect(convId).toBeDefined(); + expect(title).toBeDefined(); + + const raw = globalThis.localStorage.getItem("dispatch.tabs"); + expect(raw).not.toBeNull(); + const parsed = JSON.parse(raw as string); + expect(parsed.tabs).toHaveLength(1); + expect(parsed.tabs[0].conversationId).toBe(convId); + expect(parsed.tabs[0].title).toBe(title); + + store.dispose(); + + const ws2 = fakeSocket(); + const store2 = createAppStore({ + socketFactory: () => ws2, + fetchImpl: fakeFetchImpl(), + }); + ws2.resolveOpen(); + + expect(store2.tabs).toHaveLength(1); + expect(store2.tabs[0]?.conversationId).toBe(convId); + expect(store2.tabs[0]?.title).toBe(title); + expect(store2.activeConversationId).toBe(convId); + + store2.dispose(); + } finally { + globalThis.localStorage = realLs; + } + }); + it("newDraft resets to draft mode", () => { const ws = fakeSocket(); const store = createAppStore({ diff --git a/src/core/chunks/index.ts b/src/core/chunks/index.ts index 36ba7f4..67739bc 100644 --- a/src/core/chunks/index.ts +++ b/src/core/chunks/index.ts @@ -1,4 +1,4 @@ -export { applyHistory, foldEvent, initialState } from "./reducer"; +export { appendUserMessage, applyHistory, foldEvent, initialState } from "./reducer"; export { selectChunks, selectMessages } from "./selectors"; export type { AccumulatingChunk, diff --git a/src/core/chunks/reducer.test.ts b/src/core/chunks/reducer.test.ts index f83edb4..b7165e4 100644 --- a/src/core/chunks/reducer.test.ts +++ b/src/core/chunks/reducer.test.ts @@ -11,7 +11,7 @@ import type { TurnUsageEvent, } from "@dispatch/wire"; import { describe, expect, it } from "vitest"; -import { applyHistory, foldEvent, initialState } from "./reducer"; +import { appendUserMessage, applyHistory, foldEvent, initialState } from "./reducer"; import { selectChunks, selectMessages } from "./selectors"; const turnStart = (turnId: string): TurnStartEvent => ({ @@ -404,3 +404,64 @@ describe("selectMessages", () => { expect(msgs[1]?.chunks[0]).toEqual({ type: "text", text: "a1a2" }); }); }); + +describe("appendUserMessage", () => { + it("adds a provisional user text chunk", () => { + let s = initialState(); + s = appendUserMessage(s, "hello from user"); + const chunks = selectChunks(s); + expect(chunks).toHaveLength(1); + expect(chunks[0]?.seq).toBeNull(); + expect(chunks[0]?.role).toBe("user"); + expect(chunks[0]?.chunk).toEqual({ type: "text", text: "hello from user" }); + expect(chunks[0]?.provisional).toBe(true); + }); + + it("selectMessages includes the optimistic user message", () => { + let s = initialState(); + s = appendUserMessage(s, "what is 2+2?"); + const msgs = selectMessages(s); + expect(msgs).toHaveLength(1); + expect(msgs[0]?.role).toBe("user"); + expect(msgs[0]?.chunks).toHaveLength(1); + expect(msgs[0]?.chunks[0]).toEqual({ type: "text", text: "what is 2+2?" }); + }); + + it("user echo then turn-sealed + applyHistory supersedes the provisional user chunk", () => { + let s = initialState(); + s = appendUserMessage(s, "hi"); + expect(selectChunks(s)).toHaveLength(1); + + s = foldEvent(s, turnStart("t1")); + s = foldEvent(s, textDelta("t1", "hello back")); + s = foldEvent(s, turnSealed("t1")); + s = applyHistory(s, [ + storedChunk(1, "user", { type: "text", text: "hi" }), + storedChunk(2, "assistant", { type: "text", text: "hello back" }), + ]); + const chunks = selectChunks(s); + expect(chunks).toHaveLength(2); + expect(chunks[0]?.seq).toBe(1); + expect(chunks[0]?.role).toBe("user"); + expect(chunks[0]?.chunk).toEqual({ type: "text", text: "hi" }); + expect(chunks[0]?.provisional).toBe(false); + expect(chunks[1]?.seq).toBe(2); + expect(chunks[1]?.role).toBe("assistant"); + expect(chunks[1]?.provisional).toBe(false); + }); + + it("flushes accumulating chunk before appending user message", () => { + let s = initialState(); + s = foldEvent(s, turnStart("t1")); + s = foldEvent(s, textDelta("t1", "partial")); + expect(s.accumulating).toEqual({ kind: "text", text: "partial" }); + + s = appendUserMessage(s, "user msg"); + expect(s.accumulating).toBeNull(); + expect(s.provisional).toHaveLength(2); + expect(s.provisional[0]?.role).toBe("assistant"); + expect(s.provisional[0]?.chunk).toEqual({ type: "text", text: "partial" }); + expect(s.provisional[1]?.role).toBe("user"); + expect(s.provisional[1]?.chunk).toEqual({ type: "text", text: "user msg" }); + }); +}); diff --git a/src/core/chunks/reducer.ts b/src/core/chunks/reducer.ts index 0a8ea54..d3b999d 100644 --- a/src/core/chunks/reducer.ts +++ b/src/core/chunks/reducer.ts @@ -166,3 +166,19 @@ export function foldEvent(state: TranscriptState, event: AgentEvent): Transcript } } } + +/** + * Optimistically append a user message to the provisional list. + * Flushes any in-progress accumulating chunk first (defensively). + * The provisional user chunk is superseded when applyHistory receives + * the authoritative committed chunks after a turn seals. + */ +export function appendUserMessage(state: TranscriptState, text: string): TranscriptState { + const provisional = flushAccumulating(state.provisional, state.accumulating); + const userChunk: Chunk = { type: "text", text }; + return { + ...state, + provisional: [...provisional, { role: "user", chunk: userChunk }], + accumulating: null, + }; +} diff --git a/src/features/chat/store.svelte.ts b/src/features/chat/store.svelte.ts index e997f49..1d8ab17 100644 --- a/src/features/chat/store.svelte.ts +++ b/src/features/chat/store.svelte.ts @@ -6,6 +6,7 @@ import type { import type { ChatMessage } from "@dispatch/wire"; import type { RenderedChunk, TranscriptState } from "../../core/chunks"; import { + appendUserMessage, applyHistory, foldEvent, initialState, @@ -94,6 +95,7 @@ export function createChatStore(deps: ChatStoreDependencies): ChatStore { }, send(text: string): void { + transcript = appendUserMessage(transcript, text); const msg: ChatSendMessage = { type: "chat.send", conversationId: deps.conversationId, diff --git a/src/features/chat/store.test.ts b/src/features/chat/store.test.ts index 4ec40a9..de60b14 100644 --- a/src/features/chat/store.test.ts +++ b/src/features/chat/store.test.ts @@ -436,4 +436,62 @@ describe("createChatStore", () => { store.dispose(); }); + + it("send optimistically shows the user message immediately", () => { + const transport = createFakeTransport(); + const historySync = createFakeHistorySync(); + const cache = createFakeCache(); + const store = createChatStore({ + conversationId: CONV_ID, + transport: transport.impl, + historySync: historySync.impl, + cache: cache.impl, + }); + + store.send("hi"); + + expect(store.messages).toHaveLength(1); + expect(store.messages[0]?.role).toBe("user"); + expect(store.messages[0]?.chunks).toHaveLength(1); + expect(store.messages[0]?.chunks[0]?.type).toBe("text"); + expect((store.messages[0]?.chunks[0] as { type: "text"; text: string }).text).toBe("hi"); + + store.dispose(); + }); + + it("the optimistic user message is replaced after turn-sealed + history sync", async () => { + const transport = createFakeTransport(); + const historySync = createFakeHistorySync(); + const cache = createFakeCache(); + const store = createChatStore({ + conversationId: CONV_ID, + transport: transport.impl, + historySync: historySync.impl, + cache: cache.impl, + }); + + historySync.returnChunks = [ + { seq: 1, role: "user", chunk: { type: "text", text: "hi" } }, + { seq: 2, role: "assistant", chunk: { type: "text", text: "hello!" } }, + ]; + + store.send("hi"); + expect(store.messages).toHaveLength(1); + expect(store.messages[0]?.role).toBe("user"); + + store.handleDelta(deltaEvent({ type: "turn-start", conversationId: CONV_ID, turnId: "t1" })); + store.handleDelta( + deltaEvent({ type: "text-delta", conversationId: CONV_ID, turnId: "t1", delta: "hello!" }), + ); + store.handleDelta(deltaEvent({ type: "turn-sealed", conversationId: CONV_ID, turnId: "t1" })); + + await vi.waitFor(() => { + expect(store.messages.length).toBe(2); + }); + + expect(store.messages[0]?.role).toBe("user"); + expect(store.messages[1]?.role).toBe("assistant"); + + store.dispose(); + }); }); -- cgit v1.2.3