diff --git a/frontend/src/components/workspace/model-load-error-banner.tsx b/frontend/src/components/workspace/model-load-error-banner.tsx index 129294a04..1638e988d 100644 --- a/frontend/src/components/workspace/model-load-error-banner.tsx +++ b/frontend/src/components/workspace/model-load-error-banner.tsx @@ -1,6 +1,6 @@ "use client"; -import { useState } from "react"; +import { useRef, useState } from "react"; import { Alert, AlertDescription } from "@/components/ui/alert"; import { Button } from "@/components/ui/button"; @@ -23,7 +23,17 @@ export function ModelLoadErrorBanner({ const [isRetrying, setIsRetrying] = useState(false); // Observe the shared query without starting it. Model consumers remain in // charge of loading; this single observer only centralizes their feedback. - const { error, refetch } = useModels({ enabled: false }); + const { error, isFetching, refetch } = useModels({ enabled: false }); + const visibleErrorRef = useRef(null); + + // TanStack clears an empty query's error while another observer refetches. + // Keep the banner stable until that shared request either succeeds or fails. + if (error) { + visibleErrorRef.current = error; + } else if (!isFetching) { + visibleErrorRef.current = null; + } + const visibleError = error ?? visibleErrorRef.current; const retry = async () => { setIsRetrying(true); @@ -38,8 +48,8 @@ export function ModelLoadErrorBanner({ // Rendering a model-specific warning during navigation would be duplicate // and misleading feedback. if ( - (!error && !isRetrying) || - error instanceof UnauthorizedError || + (!visibleError && !isRetrying) || + visibleError instanceof UnauthorizedError || shouldShowOfflineBanner(user, gatewayUnavailable) ) { return null; diff --git a/frontend/src/core/models/hooks.ts b/frontend/src/core/models/hooks.ts index 636316464..eb4b4f7de 100644 --- a/frontend/src/core/models/hooks.ts +++ b/frontend/src/core/models/hooks.ts @@ -1,14 +1,20 @@ import { useQuery } from "@tanstack/react-query"; +import { UnauthorizedError } from "@/core/api/errors"; + import { loadModels } from "./api"; export const MODELS_QUERY_KEY = ["models"] as const; export function useModels({ enabled = true }: { enabled?: boolean } = {}) { - const { data, isLoading, error, refetch } = useQuery({ + const { data, isLoading, isFetching, error, refetch } = useQuery({ queryKey: MODELS_QUERY_KEY, queryFn: () => loadModels(), enabled, + // Surface persistent gateway failures promptly while retaining one retry + // for transient startup or network errors. + retry: (failureCount, queryError) => + !(queryError instanceof UnauthorizedError) && failureCount < 1, refetchOnWindowFocus: false, // Model config changes rarely and every subtask card mounts its own // observer of this query; without a staleTime each newly-mounted card would @@ -21,6 +27,7 @@ export function useModels({ enabled = true }: { enabled?: boolean } = {}) { models: data?.models ?? [], tokenUsageEnabled: data?.token_usage.enabled ?? false, isLoading, + isFetching, error, refetch, }; diff --git a/frontend/tests/unit/components/workspace/model-load-error-banner.dom.test.tsx b/frontend/tests/unit/components/workspace/model-load-error-banner.dom.test.tsx index c6c9bb898..2f9cd7c87 100644 --- a/frontend/tests/unit/components/workspace/model-load-error-banner.dom.test.tsx +++ b/frontend/tests/unit/components/workspace/model-load-error-banner.dom.test.tsx @@ -76,7 +76,7 @@ afterEach(() => { function createWrapper() { const queryClient = new QueryClient({ defaultOptions: { - queries: { retry: false }, + queries: { retry: false, retryDelay: 0 }, }, }); @@ -105,9 +105,38 @@ describe("ModelLoadErrorBanner", () => { expect(screen.queryByRole("alert")).toBeNull(); }); + it("limits automatic model loading retries before surfacing the error", async () => { + mockedLoadModels.mockRejectedValue(new Error("Gateway returned 503")); + const queryClient = new QueryClient({ + defaultOptions: { + queries: { retryDelay: 0 }, + }, + }); + + function RetryPolicyWrapper({ children }: PropsWithChildren) { + return ( + + {children} + + ); + } + + render( + <> + + + , + { wrapper: RetryPolicyWrapper }, + ); + + expect(await screen.findByRole("alert")).not.toBeNull(); + expect(mockedLoadModels).toHaveBeenCalledTimes(2); + }); + it("shows one actionable error for all model consumers and clears after retry", async () => { const retryResult = createDeferred(); mockedLoadModels + .mockRejectedValueOnce(new Error("Gateway returned 503")) .mockRejectedValueOnce(new Error("Gateway returned 503")) .mockImplementationOnce(() => retryResult.promise); const { QueryWrapper } = createWrapper(); @@ -125,7 +154,7 @@ describe("ModelLoadErrorBanner", () => { expect(alert.textContent).toContain("Models couldn't be loaded"); expect(alert.textContent).not.toContain("Gateway returned 503"); expect(screen.getAllByRole("alert")).toHaveLength(1); - expect(mockedLoadModels).toHaveBeenCalledTimes(1); + expect(mockedLoadModels).toHaveBeenCalledTimes(2); fireEvent.click(screen.getByRole("button", { name: "Retry" })); @@ -142,7 +171,7 @@ describe("ModelLoadErrorBanner", () => { await waitFor(() => { expect(screen.queryByRole("alert")).toBeNull(); }); - expect(mockedLoadModels).toHaveBeenCalledTimes(2); + expect(mockedLoadModels).toHaveBeenCalledTimes(3); }); it("does not duplicate the login redirect with a model warning", async () => { @@ -160,12 +189,13 @@ describe("ModelLoadErrorBanner", () => { await waitFor(() => { expect(queryClient.getQueryState(MODELS_QUERY_KEY)?.status).toBe("error"); }); + expect(mockedLoadModels).toHaveBeenCalledTimes(1); expect(screen.queryByRole("alert")).toBeNull(); }); it("suppresses a model symptom only while the gateway banner is visible", async () => { mockedUseAuth.mockReturnValue(createAuthState(null)); - mockedLoadModels.mockRejectedValueOnce(new Error("Gateway returned 503")); + mockedLoadModels.mockRejectedValue(new Error("Gateway returned 503")); const { queryClient, QueryWrapper } = createWrapper(); const renderView = () => ( @@ -187,9 +217,10 @@ describe("ModelLoadErrorBanner", () => { expect(await screen.findByRole("alert")).not.toBeNull(); }); - it("does not show manual retry progress for a shared background refetch", async () => { + it("keeps the error visible without showing manual retry progress during a shared refetch", async () => { const backgroundResult = createDeferred(); mockedLoadModels + .mockRejectedValueOnce(new Error("Gateway returned 503")) .mockRejectedValueOnce(new Error("Gateway returned 503")) .mockImplementationOnce(() => backgroundResult.promise); const { queryClient, QueryWrapper } = createWrapper(); @@ -207,9 +238,10 @@ describe("ModelLoadErrorBanner", () => { queryKey: MODELS_QUERY_KEY, }); await waitFor(() => { - expect(mockedLoadModels).toHaveBeenCalledTimes(2); + expect(mockedLoadModels).toHaveBeenCalledTimes(3); }); + expect(screen.getByRole("alert")).not.toBeNull(); expect(screen.queryByRole("button", { name: "Retrying…" })).toBeNull(); backgroundResult.resolve({