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}