diff options
| author | Adam Malczewski <[email protected]> | 2026-10-21 03:23:35 +0900 |
|---|---|---|
| committer | Adam Malczewski <[email protected]> | 2026-10-21 03:23:35 +0900 |
| commit | 550ade33285d1f39a909733e02a6b27211464df2 (patch) | |
| tree | 365672bb62798b2b64d44a18b237f1f34394073f | |
| parent | 0c0f366b3820136c9877f80005939011c30b2de4 (diff) | |
| download | dispatch-web-550ade33285d1f39a909733e02a6b27211464df2.tar.gz dispatch-web-550ade33285d1f39a909733e02a6b27211464df2.zip | |
fix(review-4.7): preserve images through composer queue submissions
| -rw-r--r-- | src/app/App.test.ts | 47 | ||||
| -rw-r--r-- | src/app/store.svelte.ts | 6 | ||||
| -rw-r--r-- | src/app/views/ComposerPane.svelte | 2 | ||||
| -rw-r--r-- | src/features/chat/store.svelte.ts | 10 | ||||
| -rw-r--r-- | src/features/chat/store.test.ts | 34 | ||||
| -rw-r--r-- | src/features/chat/ui.test.ts | 79 | ||||
| -rw-r--r-- | src/features/chat/ui/Composer.svelte | 29 |
7 files changed, 178 insertions, 29 deletions
diff --git a/src/app/App.test.ts b/src/app/App.test.ts index 183b8b4..14dac3a 100644 --- a/src/app/App.test.ts +++ b/src/app/App.test.ts @@ -434,6 +434,53 @@ describe("App component interaction tests", () => { store.dispose(); }); + it.each([ + "look at this", + "", + ])("queues staged images through the full UI with text %j", async (text) => { + const ws = fakeSocket(); + const store = createAppStore({ + socketFactory: () => ws, + fetchImpl: fakeFetchImpl(), + localStorage: createFakeStorage(), + chatFlushScheduler: null, + }); + ws.resolveOpen(); + const { container } = render(App, { props: { store, onNavigate: vi.fn() } }); + const user = userEvent.setup(); + const textarea = screen.getByRole("textbox", { name: "Message input" }); + await user.type(textarea, "start"); + await user.click(screen.getByRole("button", { name: "Send" })); + const conversationId = activeConversationId(store); + ws.feedServerMessage({ + type: "chat.delta", + event: { type: "turn-start", conversationId, turnId: "t1" }, + }); + if (text) await user.type(textarea, text); + const input = container.querySelector('input[type="file"]') as HTMLInputElement; + Object.defineProperty(input, "files", { + value: [new File(["GIF"], "shot.gif", { type: "image/gif" })], + }); + input.dispatchEvent(new Event("change", { bubbles: true })); + await screen.findByRole("button", { name: "Remove image" }); + ws.sent.length = 0; + + await user.click(screen.getByRole("button", { name: "Queue" })); + + expect(sentMessages(ws).filter((m: { type: string }) => m.type.startsWith("chat."))).toEqual([ + { + type: "chat.queue", + conversationId, + workspaceId: "default", + text, + images: [{ url: "data:image/gif;base64,R0lG", mimeType: "image/gif" }], + }, + ]); + expect(textarea).toHaveValue(""); + expect(screen.queryByRole("button", { name: "Remove image" })).not.toBeInTheDocument(); + store.dispose(); + }); + it("incoming chat.delta renders text in the chat transcript", async () => { const ws = fakeSocket(); const store = createAppStore({ diff --git a/src/app/store.svelte.ts b/src/app/store.svelte.ts index 2eb13ef..f3d0c3b 100644 --- a/src/app/store.svelte.ts +++ b/src/app/store.svelte.ts @@ -257,7 +257,7 @@ export interface AppStore { * auto-starts a turn (equivalent to `send`). Safe to offer whenever the user * wants to add input — the server owns the idle-vs-generating decision. */ - queueMessage(text: string): void; + queueMessage(text: string, images?: readonly ImageInput[]): void; /** * Cancel (remove) a single queued steering message by id so it never runs * (`chat.queue.cancel` WS op). Fire-and-forget + idempotent: the @@ -1983,13 +1983,13 @@ export function createAppStore(opts?: CreateAppStoreOptions): AppStore { } }, - queueMessage(text: string): void { + queueMessage(text: string, images?: readonly ImageInput[]): void { // Only offered while generating (Composer switches to `chat.queue` // when `status === "running"`), so a draft (never generating) never // reaches here. `chat.queue` auto-starts a turn if idle, so even a race // (turn sealed between the status read and the send) is safe — the // server starts a fresh turn with the message as its opening prompt. - activeChat.queueMessage(text); + activeChat.queueMessage(text, images); }, cancelQueuedMessage(messageId: string): void { diff --git a/src/app/views/ComposerPane.svelte b/src/app/views/ComposerPane.svelte index dcd5b47..ac95598 100644 --- a/src/app/views/ComposerPane.svelte +++ b/src/app/views/ComposerPane.svelte @@ -27,7 +27,7 @@ <Composer onSend={(text, images) => store.send(text, images)} - onQueue={(text) => store.queueMessage(text)} + onQueue={(text, images) => store.queueMessage(text, images)} onStop={() => store.stopGeneration()} contextSize={store.activeChat.currentContextSize} contextWindow={store.modelInfo[store.activeModel]?.contextWindow} diff --git a/src/features/chat/store.svelte.ts b/src/features/chat/store.svelte.ts index dab1ba2..6cfa484 100644 --- a/src/features/chat/store.svelte.ts +++ b/src/features/chat/store.svelte.ts @@ -159,9 +159,10 @@ export interface ChatStore { * auto-starts a turn with the message as its opening prompt (equivalent to * `chat.send`). No optimistic transcript echo — the queue SURFACE carries the * pending message until drain; the `steering` event places it in the - * transcript. `text` must be non-empty (the server 400/errors otherwise). + * transcript. Empty `text` is allowed when images are supplied. WS delivery + * is fire-and-forget; transport send is not a persistence acknowledgement. */ - queueMessage(text: string): void; + queueMessage(text: string, images?: readonly ImageInput[]): void; /** * Cancel (remove) a single queued steering message by id so it never runs * (`chat.queue.cancel` WS op). Fire-and-forget + idempotent: success is @@ -539,13 +540,14 @@ export function createChatStore(deps: ChatStoreDependencies): ChatStore { deps.transport.send(msg); }, - queueMessage(text: string): void { + queueMessage(text: string, images?: readonly ImageInput[]): void { const trimmed = text.trim(); - if (trimmed.length === 0) return; + if (trimmed.length === 0 && !images?.length) return; const msg: ChatQueueMessage = { type: "chat.queue", conversationId: deps.conversationId, text: trimmed, + ...(images !== undefined && images.length > 0 ? { images: [...images] } : {}), ...(deps.workspaceId !== undefined ? { workspaceId: deps.workspaceId } : {}), }; deps.transport.send(msg); diff --git a/src/features/chat/store.test.ts b/src/features/chat/store.test.ts index bb1e304..8c94316 100644 --- a/src/features/chat/store.test.ts +++ b/src/features/chat/store.test.ts @@ -255,10 +255,43 @@ describe("createChatStore", () => { expect(transport.sentQueue[0]?.type).toBe("chat.queue"); expect(transport.sentQueue[0]?.conversationId).toBe(CONV_ID); expect(transport.sentQueue[0]?.text).toBe("steer left"); + expect(transport.sentQueue[0]).not.toHaveProperty("images"); store.dispose(); }); + it.each([" describe this ", "", " "])("queues images with text %j", (text) => { + const transport = createFakeTransport(); + const store = createChatStore({ + conversationId: CONV_ID, + workspaceId: "images-workspace", + transport: transport.impl, + historySync: createFakeHistorySync().impl, + metricsSync: createFakeMetricsSync().impl, + cache: createFakeCache().impl, + }); + const images: readonly ImageInput[] = [ + { url: "data:image/png;base64,AAAA", mimeType: "image/png" }, + { url: "https://example.com/photo.jpg" }, + ]; + + store.queueMessage(text, images); + + expect(transport.sentQueue).toEqual([ + { + type: "chat.queue", + conversationId: CONV_ID, + workspaceId: "images-workspace", + text: text.trim(), + images, + }, + ]); + expect(transport.sentQueue[0]?.images).not.toBe(images); + expect(transport.sent).toHaveLength(0); + expect(store.chunks).toHaveLength(0); + store.dispose(); + }); + it("trims whitespace before sending", () => { const transport = createFakeTransport(); const historySync = createFakeHistorySync(); @@ -294,6 +327,7 @@ describe("createChatStore", () => { store.queueMessage(" "); store.queueMessage(""); + store.queueMessage("", []); expect(transport.sentQueue).toHaveLength(0); diff --git a/src/features/chat/ui.test.ts b/src/features/chat/ui.test.ts index c557e4f..2367e64 100644 --- a/src/features/chat/ui.test.ts +++ b/src/features/chat/ui.test.ts @@ -1029,17 +1029,28 @@ describe("Composer", () => { expect(screen.queryByRole("button", { name: "Remove image" })).not.toBeInTheDocument(); }); - it("queues (steers) text-only and never forwards images", async () => { - // While running, the Send button becomes "Queue"; steering is text-only. + it("still queues text-only with images omitted", async () => { const onQueue = vi.fn(); const onSend = vi.fn(); const user = userEvent.setup(); - const { container } = render(Composer, { props: { onSend, onQueue, status: "running" } }); + render(Composer, { props: { onSend, onQueue, status: "running" } }); + await user.type(screen.getByRole("textbox", { name: "Message input" }), "steer{Enter}"); + expect(onQueue).toHaveBeenCalledExactlyOnceWith("steer", undefined); + expect(onSend).not.toHaveBeenCalled(); + }); + + it("queues text and images while running and clears the composer", async () => { + const onQueue = vi.fn(); + const onSend = vi.fn(); + const user = userEvent.setup(); + const { container } = render(Composer, { + props: { onSend, onQueue, status: "running", encodeJpeg: async () => null }, + }); const textarea = screen.getByRole("textbox", { name: "Message input" }); await user.type(textarea, "steer here"); - // Also stage an image — it must NOT be forwarded on a queue. + // Stage an image along with the steering text. const file = new File(["PNG"], "shot.png", { type: "image/png" }); const input = container.querySelector('input[type="file"]') as HTMLInputElement; Object.defineProperty(input, "files", { value: [file], writable: false }); @@ -1049,8 +1060,66 @@ describe("Composer", () => { }); await user.click(screen.getByRole("button", { name: "Queue" })); - expect(onQueue).toHaveBeenCalledWith("steer here"); + expect(onQueue).toHaveBeenCalledExactlyOnceWith("steer here", [ + { url: "data:image/png;base64,UE5H", mimeType: "image/png" }, + ]); + expect(onSend).not.toHaveBeenCalled(); + expect(textarea).toHaveValue(""); + expect(screen.queryByRole("button", { name: "Remove image" })).not.toBeInTheDocument(); + }); + + it.each(["running", "queued"] as const)("queues images-only while %s", async (status) => { + const onQueue = vi.fn(); + const onSend = vi.fn(); + const user = userEvent.setup(); + const { container } = render(Composer, { props: { onSend, onQueue, status } }); + const input = container.querySelector('input[type="file"]') as HTMLInputElement; + Object.defineProperty(input, "files", { + value: [new File(["GIF"], "shot.gif", { type: "image/gif" })], + }); + input.dispatchEvent(new Event("change", { bubbles: true })); + await screen.findByRole("button", { name: "Remove image" }); + + await user.click(screen.getByRole("button", { name: "Queue" })); + + expect(onQueue).toHaveBeenCalledExactlyOnceWith("", [ + { url: "data:image/gif;base64,R0lG", mimeType: "image/gif" }, + ]); expect(onSend).not.toHaveBeenCalled(); + expect(screen.queryByRole("button", { name: "Remove image" })).not.toBeInTheDocument(); + }); + + it.each([ + "running", + "queued", + ] as const)("keeps images and text staged while %s without onQueue", async (status) => { + const onSend = vi.fn(); + const user = userEvent.setup(); + const { container, rerender } = render(Composer, { props: { onSend, status } }); + const input = container.querySelector('input[type="file"]') as HTMLInputElement; + Object.defineProperty(input, "files", { + value: [new File(["GIF"], "shot.gif", { type: "image/gif" })], + }); + input.dispatchEvent(new Event("change", { bubbles: true })); + await screen.findByRole("button", { name: "Remove image" }); + const textarea = screen.getByRole("textbox", { name: "Message input" }); + await user.type(textarea, "{Enter}"); // image-only: no queue callback + await user.type(textarea, "keep this{Enter}"); + container + .querySelector("form") + ?.dispatchEvent(new Event("submit", { bubbles: true, cancelable: true })); + + expect(onSend).not.toHaveBeenCalled(); + expect(textarea).toHaveValue("keep this"); + expect(screen.getByRole("button", { name: "Remove image" })).toBeInTheDocument(); + expect(screen.getByRole("button", { name: "Queue" })).toBeDisabled(); + // Once idle, the same staged input can actually be sent. + await rerender({ status: "idle" }); + await user.click(screen.getByRole("button", { name: "Send" })); + expect(onSend).toHaveBeenCalledExactlyOnceWith("keep this", [ + { url: "data:image/gif;base64,R0lG", mimeType: "image/gif" }, + ]); + expect(screen.queryByRole("button", { name: "Remove image" })).not.toBeInTheDocument(); }); }); diff --git a/src/features/chat/ui/Composer.svelte b/src/features/chat/ui/Composer.svelte index c7d9d02..3cc115b 100644 --- a/src/features/chat/ui/Composer.svelte +++ b/src/features/chat/ui/Composer.svelte @@ -70,10 +70,10 @@ /** * Enqueue a steering message (`chat.queue`). When provided AND the status * is `running`, the send button becomes a "Queue" button that steers the - * in-flight turn instead of starting a new one. Steering is text-only — - * it never carries images (a mid-turn injection has no image surface). + * in-flight turn instead of starting a new one. Carries staged images just + * like onSend; without this callback, in-flight input stays staged. */ - onQueue?: (text: string) => void; + onQueue?: (text: string, images?: ImageInput[]) => void; /** Stop the in-flight generation (`POST /conversations/:id/stop`). */ onStop?: () => void; /** @@ -107,23 +107,20 @@ const hasText = $derived(text.trim().length > 0); const hasImages = $derived(images.length > 0); - const canSend = $derived(hasText || hasImages); + const inFlight = $derived(status === "running" || status === "queued"); + const canSend = $derived((hasText || hasImages) && (!inFlight || onQueue !== undefined)); const effectiveMax = $derived(contextWindow ?? FALLBACK_CONTEXT_WINDOW); const usage = $derived(computeContextUsage(contextSize, effectiveMax)); // One button, three modes: // - idle → "Send" (starts a turn via chat.send) - // - running/queued + text → "Queue" (steers via chat.queue — text only) + // - running/queued + text/images → "Queue" (steers via chat.queue) // - running/queued + empty → "Stop" (aborts via POST /stop) // (`queued` behaves like `running` — the turn is in flight, just waiting for a // concurrency slot; the user can still steer or stop it.) - // Steering never carries images: when running with images staged but no text, - // the images stay staged (queue is text-only). Images-without-text while running - // is an unusual case that still sends (the server auto-starts/resolves). - const inFlight = $derived(status === "running" || status === "queued"); const buttonMode = $derived.by<"send" | "queue" | "stop">(() => { if (inFlight && !hasText && !hasImages && onStop !== undefined) return "stop"; - if (inFlight && hasText && onQueue !== undefined) return "queue"; + if (inFlight) return "queue"; return "send"; }); const placeholder = $derived( @@ -283,15 +280,15 @@ const trimmed = text.trim(); // Allow a send with images even when text is empty (an image-only turn). if (trimmed.length === 0 && !hasImages) return; - if (buttonMode === "queue") { - // Steering is text-only — never forward images. - onQueue?.(trimmed); + const toSend = hasImages ? images.map((img) => imageInput(img)) : undefined; + if (inFlight) { + if (onQueue === undefined) return; // Keep staged input if no queue port is wired. + onQueue(trimmed, toSend); } else { - const toSend: ImageInput[] | undefined = hasImages - ? images.map((img) => imageInput(img)) - : undefined; onSend(trimmed, toSend); } + // Clear only after handing the complete input to a callback. WS is still + // fire-and-forget: this does not imply backend storage has completed. text = ""; images = []; } |
