Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 50 additions & 0 deletions frontend/src/features/chat/ChatPage.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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<void>((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 = {
Expand Down
26 changes: 21 additions & 5 deletions frontend/src/features/chat/ChatPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand Down Expand Up @@ -140,7 +146,7 @@ export function ChatPage({
const handledClearRequestsRef = useRef(new Set<string>());

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 } = {}) => {
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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}
Expand Down