From 5be072990c7ba4573219c5da25fb2d5c2c8fcd5e Mon Sep 17 00:00:00 2001 From: Matthew Robert Wesney <157447210+dovvnloading@users.noreply.github.com> Date: Mon, 7 Sep 2026 11:46:42 -0400 Subject: [PATCH] fix(frontend): only offer Retry for failures resending the prompt can fix The composer banner shows one "Retry last message" button for every failure it can display, and most of those are not generation failures. Clicking it after an unrelated error resent a prompt the thread had already answered, adding a duplicate turn. Three of the six sites that raise this banner cannot be fixed by resending: - a failed fork -- not a generation at all; the thread is answered and unchanged; - a failed stop -- the response is still running, so a resend starts a second one alongside it; - a reload that failed after generation finished -- the answer exists and was saved, only the refetch failed. The other three are genuine: a generation that failed, a runtime that was unavailable, and an admission that could not be started. Those keep Retry, and the replay logic behind it is untouched. `ScopedError` now records whether resending is the remedy, and the button is rendered only for an error that says so. The regression test sends and completes a turn before forking, on purpose: `lastPrompt` is only set once something has been sent this session, so a test that forks in a freshly loaded thread never renders a Retry button and passes against the unfixed code. My first version did exactly that. Co-Authored-By: Claude Opus 5 --- frontend/src/features/chat/ChatPage.test.tsx | 50 ++++++++++++++++++++ frontend/src/features/chat/ChatPage.tsx | 26 ++++++++-- 2 files changed, 71 insertions(+), 5 deletions(-) diff --git a/frontend/src/features/chat/ChatPage.test.tsx b/frontend/src/features/chat/ChatPage.test.tsx index 63ee285..10d8600 100644 --- a/frontend/src/features/chat/ChatPage.test.tsx +++ b/frontend/src/features/chat/ChatPage.test.tsx @@ -1215,6 +1215,56 @@ describe("ChatPage composer integration", () => { expect(await screen.findByRole("alert")).toHaveTextContent("Only 8 of 9 files were attached"); }); + it("does not offer to resend the prompt when forking fails", async () => { + // The banner shows one Retry for every failure it can display, and a fork + // is not a generation. With a prompt already sent and answered this + // session, the fork failure offered Retry, which resent a turn the model + // had already replied to. + const user = userEvent.setup(); + let emit: ((event: unknown) => void) | null = null; + let resolveStream: (() => void) | undefined; + const generate = vi.fn().mockResolvedValue({ + job_id: "job-fork", + kind: "generation", + status: "queued", + thread_id: "thread-a", + user_message_id: "message-user-1", + }); + const api = chatApi({ + generate, + forkChat: vi.fn().mockRejectedValue(new Error("fork exploded")), + chat: vi.fn(async (id: string) => ({ + ...emptyChat(id), + messages: [ + { id: "message-user-1", role: "user" as const, content: "Answered already" }, + { id: "message-assistant-1", role: "assistant" as const, content: "Here you go." }, + ], + })), + streamGeneration: vi.fn((_jobId, onEvent) => new Promise((resolve) => { + resolveStream = resolve; + emit = (event) => (onEvent as (event: unknown) => void)(event); + })), + }); + renderChat(api); + + // Send a turn so `lastPrompt` is set -- without one the banner offers no + // Retry at all and the check below would pass vacuously. + await user.type(await screen.findByLabelText("Message Cortex"), "Answered already"); + await user.click(screen.getByRole("button", { name: "Send message" })); + await waitFor(() => expect(emit).not.toBeNull()); + await act(async () => { + emit!({ event_id: 1, event: "generation.completed", job_id: "job-fork", thread_id: "thread-a", data: {} }); + resolveStream?.(); + }); + await screen.findByText("Here you go."); + + await user.click((await screen.findAllByRole("button", { name: "Fork chat from this message" }))[0]); + + expect(await screen.findByText("Could not fork this chat.")).toBeInTheDocument(); + expect(screen.queryByRole("button", { name: "Retry last message" })).not.toBeInTheDocument(); + expect(generate).toHaveBeenCalledTimes(1); + }); + 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 fa01ddd..fb1cfb1 100644 --- a/frontend/src/features/chat/ChatPage.tsx +++ b/frontend/src/features/chat/ChatPage.tsx @@ -44,6 +44,12 @@ type Props = { type ScopedError = { message: string; threadId: string | null; + // Whether resending the last prompt is the remedy for this error. The + // banner offers one Retry button for every failure it can show, and most + // of them are not generation failures at all -- a failed fork, a failed + // stop, a reload that failed after the answer was already saved. Retrying + // the prompt there resubmits a turn the thread has already answered. + retryable: boolean; }; type ChatLoadState = { @@ -140,7 +146,7 @@ export function ChatPage({ const handledClearRequestsRef = useRef(new Set()); const reportGenerationFailure = useCallback((failedThreadId: string, message: string) => { - setGenerationError({ threadId: failedThreadId, message }); + setGenerationError({ threadId: failedThreadId, message, retryable: true }); }, []); const loadChat = useCallback(async ({ preserveCurrent = false }: { preserveCurrent?: boolean } = {}) => { @@ -214,9 +220,10 @@ export function ChatPage({ const displayedThreadId = threadId ?? resolvedThreadId; const activeJobForCurrentThread = Boolean(generation.jobId && generation.threadId === displayedThreadId); const generationElsewhere = Boolean(generation.jobId && !activeJobForCurrentThread); - const visibleGenerationError = generationError && generationError.threadId === displayedThreadId - ? generationError.message + const visibleError = generationError && generationError.threadId === displayedThreadId + ? generationError : null; + const visibleGenerationError = visibleError?.message ?? null; const composerPhase: ComposerPhase = !runtimeReady ? "unavailable" : generation.phase === "stopping" @@ -245,7 +252,9 @@ export function ChatPage({ } } catch { if (!isLatestRequest()) return; - setGenerationError({ threadId: id, message: "Generation finished, but the saved chat could not be reloaded." }); + // The generation succeeded; only the reload failed. Resending would + // ask for a second answer to a question already answered. + setGenerationError({ threadId: id, message: "Generation finished, but the saved chat could not be reloaded.", retryable: false }); if (viewThreadIdRef.current !== id) return; // This call bumped the shared request version, so any route load still // in flight for this thread has already returned early as stale. If we @@ -368,6 +377,7 @@ export function ChatPage({ setGenerationError({ threadId, message: runtimeMessage ?? "The local runtime is unavailable. Rescan local models after it is running.", + retryable: true, }); return null; } @@ -462,6 +472,7 @@ export function ChatPage({ setGenerationError({ threadId, message: requestError instanceof ApiError ? requestError.detail : "The response could not be started. Your message is still here.", + retryable: true, }); return null; } finally { @@ -568,9 +579,12 @@ export function ChatPage({ } } catch (requestError) { useChatStore.getState().revertStopping(jobId); + // Stop failed, so the response is still running. Sending the prompt + // again would start a second one alongside it. setGenerationError({ threadId: jobThreadId, message: requestError instanceof ApiError ? requestError.detail : "Could not stop the response.", + retryable: false, }); } finally { stoppingRef.current = false; @@ -615,9 +629,11 @@ export function ChatPage({ const forked = await api.forkChat(threadId, message.id); onForked(forked); } catch (requestError) { + // Forking is not a generation. The thread is answered and unchanged. setGenerationError({ threadId, message: requestError instanceof ApiError ? requestError.detail : "Could not fork this chat.", + retryable: false, }); } finally { setForkingMessage(null); @@ -788,7 +804,7 @@ export function ChatPage({ onStop={cancel} onSelectModel={onSelectModel} onRescanModels={onRescanModels} - onRetry={lastPrompt ? retryLastPrompt : undefined} + onRetry={lastPrompt && visibleError?.retryable ? retryLastPrompt : undefined} onDismissError={() => setGenerationError(null)} generationOptions={threadOptions} generationDefaults={generationDefaults}