From b9a9e0f4bd3ae68389b78e1b1210184ebd15c3d2 Mon Sep 17 00:00:00 2001 From: Matthew Robert Wesney <157447210+dovvnloading@users.noreply.github.com> Date: Mon, 7 Sep 2026 12:17:28 -0400 Subject: [PATCH] fix(frontend): clear the composer when a retry succeeds A failed send deliberately leaves the text in the composer -- "Your message is still here" -- so the user does not lose it. Retry then sent that text without clearing it, so the message ended up in the transcript *and* back in the box, one Enter away from being sent a second time. Only `submitDraft` cleared the composer. `retryLastPrompt` called `startGeneration` directly and skipped all of it, including the migration of draft and attachments from the "new chat" placeholder scope to the real thread when a retry is what creates that thread. That settling logic is now a helper both paths use, so they cannot drift. Extracting it changed no behaviour: the existing 33 ChatPage tests passed against the extraction alone, before retry was wired to it. Retry passes the live draft only when it is still the message being retried (`draft.trim() === lastPrompt`, since lastPrompt is the trimmed input). A draft the user has started rewriting since the failure is left alone -- the helper's existing "clear only if unchanged" guard then does the rest. Co-Authored-By: Claude Opus 5 --- frontend/src/features/chat/ChatPage.test.tsx | 67 ++++++++++++++++++++ frontend/src/features/chat/ChatPage.tsx | 63 +++++++++++++++--- 2 files changed, 121 insertions(+), 9 deletions(-) diff --git a/frontend/src/features/chat/ChatPage.test.tsx b/frontend/src/features/chat/ChatPage.test.tsx index 10d8600..0b9c367 100644 --- a/frontend/src/features/chat/ChatPage.test.tsx +++ b/frontend/src/features/chat/ChatPage.test.tsx @@ -1265,6 +1265,73 @@ describe("ChatPage composer integration", () => { expect(generate).toHaveBeenCalledTimes(1); }); + it("clears the composer when a retry succeeds", async () => { + // A failed send deliberately keeps the text in the box ("Your message is + // still here"). Retry then sent it without clearing, so the message was + // in the transcript and back in the composer, one Enter from going twice. + const user = userEvent.setup(); + const generate = vi.fn() + .mockRejectedValueOnce(new ApiError(503, "The response could not be started.")) + .mockResolvedValueOnce({ + job_id: "job-retry", + kind: "generation", + status: "queued", + thread_id: "thread-a", + user_message_id: "message-user-1", + }); + const api = chatApi({ + generate, + chat: vi.fn(async (id: string) => emptyChat(id)), + streamGeneration: vi.fn(() => new Promise(() => undefined)), + }); + renderChat(api); + + const composer = await screen.findByLabelText("Message Cortex"); + await user.type(composer, "Send me once"); + await user.click(screen.getByRole("button", { name: "Send message" })); + + // The failed send keeps the text, which is the behaviour being built on. + await screen.findByRole("button", { name: "Retry last message" }); + expect(composer).toHaveValue("Send me once"); + + await user.click(screen.getByRole("button", { name: "Retry last message" })); + + await waitFor(() => expect(generate).toHaveBeenCalledTimes(2)); + await waitFor(() => expect(screen.getByLabelText("Message Cortex")).toHaveValue("")); + }); + + it("leaves a draft the user has rewritten since the failure alone", async () => { + // Only the draft that is still the message being retried is cleared. + const user = userEvent.setup(); + const generate = vi.fn() + .mockRejectedValueOnce(new ApiError(503, "The response could not be started.")) + .mockResolvedValueOnce({ + job_id: "job-retry-2", + kind: "generation", + status: "queued", + thread_id: "thread-a", + user_message_id: "message-user-2", + }); + const api = chatApi({ + generate, + chat: vi.fn(async (id: string) => emptyChat(id)), + streamGeneration: vi.fn(() => new Promise(() => undefined)), + }); + renderChat(api); + + const composer = await screen.findByLabelText("Message Cortex"); + await user.type(composer, "Send me once"); + await user.click(screen.getByRole("button", { name: "Send message" })); + await screen.findByRole("button", { name: "Retry last message" }); + + await user.clear(composer); + await user.type(composer, "Actually, something else"); + await user.click(screen.getByRole("button", { name: "Retry last message" })); + + await waitFor(() => expect(generate).toHaveBeenCalledTimes(2)); + expect(screen.getByLabelText("Message Cortex")).toHaveValue("Actually, something else"); + }); + it("explains the image capability mismatch before a generation request is made", async () => { const user = userEvent.setup(); const attachment: ChatAttachment = { diff --git a/frontend/src/features/chat/ChatPage.tsx b/frontend/src/features/chat/ChatPage.tsx index fb1cfb1..554e828 100644 --- a/frontend/src/features/chat/ChatPage.tsx +++ b/frontend/src/features/chat/ChatPage.tsx @@ -481,15 +481,19 @@ export function ChatPage({ } }; - const submitDraft = async (): Promise => { - const submittedDraft = draft; - const submittedAttachments = attachments; - const submittedScope = draftScope; - const submittedAttachmentScope = attachmentScope; - const submittedThreadId = threadId; - const started = await startGeneration(submittedDraft, undefined, submittedAttachments); - if (!started) return false; - + // Everything the composer has to let go of once a message is genuinely on + // its way: the draft text, its attachments, and -- for a first message -- + // the migration of both from the "new chat" placeholder scope to the real + // thread. Retry sends a message too, and used to skip all of it, leaving + // the text it had just sent sitting in the box ready to be sent twice. + const settleComposerAfterSend = ( + started: StartedGeneration, + submittedDraft: string, + submittedAttachments: readonly ChatAttachment[], + submittedScope: string, + submittedAttachmentScope: string, + submittedThreadId: string | null, + ) => { const destinationThreadId = submittedThreadId ?? started.threadId; const destinationDraftScope = composerDraftKey(destinationThreadId); const destinationAttachmentScope = composerAttachmentKey(destinationThreadId); @@ -554,6 +558,25 @@ export function ChatPage({ } writeComposerAttachments(destinationThreadId, retainedAttachments); } + }; + + const submitDraft = async (): Promise => { + const submittedDraft = draft; + const submittedAttachments = attachments; + const submittedScope = draftScope; + const submittedAttachmentScope = attachmentScope; + const submittedThreadId = threadId; + const started = await startGeneration(submittedDraft, undefined, submittedAttachments); + if (!started) return false; + + settleComposerAfterSend( + started, + submittedDraft, + submittedAttachments, + submittedScope, + submittedAttachmentScope, + submittedThreadId, + ); if (!submittedThreadId) onThreadCreated(started.threadId); return true; }; @@ -593,6 +616,26 @@ export function ChatPage({ const retryLastPrompt = async (): Promise => { if (!lastPrompt) return false; + // A failed send deliberately leaves the text in the composer ("Your + // message is still here"), so a retry that succeeds has to clear it the + // way a submit does -- otherwise the message is both in the transcript + // and back in the box, one Enter away from being sent twice. + // + // `lastPrompt` is the trimmed input, so compare on that: only the draft + // that is still the message being retried is cleared, and a draft the + // user has since started rewriting is left alone. + const submittedDraft = draft.trim() === lastPrompt ? draft : ""; + const submittedScope = draftScope; + const submittedAttachmentScope = attachmentScope; + const submittedThreadId = threadId; + const settle = (started: StartedGeneration) => settleComposerAfterSend( + started, + submittedDraft, + lastAttachments, + submittedScope, + submittedAttachmentScope, + submittedThreadId, + ); // A stream-level failure after the user's message was already durably // admitted leaves that message as the thread's last one with no reply. // Retrying must regenerate a reply for it, not resubmit the same text @@ -604,6 +647,7 @@ export function ChatPage({ const danglingUserMessageId = lastMessage?.role === "user" ? lastMessage.id ?? undefined : undefined; if (danglingUserMessageId) { const started = await startGeneration(lastPrompt, danglingUserMessageId, lastAttachments); + if (started) settle(started); if (started && !threadId) onThreadCreated(started.threadId); return Boolean(started); } @@ -618,6 +662,7 @@ export function ChatPage({ pendingAdmission?.requestId, pendingAdmission ?? undefined, ); + if (started) settle(started); if (started && !threadId) onThreadCreated(started.threadId); return Boolean(started); };