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 /src/features/chat | |
| parent | 0c0f366b3820136c9877f80005939011c30b2de4 (diff) | |
| download | dispatch-web-550ade33285d1f39a909733e02a6b27211464df2.tar.gz dispatch-web-550ade33285d1f39a909733e02a6b27211464df2.zip | |
fix(review-4.7): preserve images through composer queue submissions
Diffstat (limited to 'src/features/chat')
| -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 |
4 files changed, 127 insertions, 25 deletions
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 = []; } |
