fix(frontend): stabilize model load error feedback (#5021)

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<Error | null>(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;

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,
   };

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 (
+        <QueryClientProvider client={queryClient}>
+          {children}
+        </QueryClientProvider>
+      );
+    }
+
+    render(
+      <>
+        <ModelLoadErrorBanner />
+        <ModelConsumer />
+      </>,
+      { 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<ModelsResponse>();
     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<ModelsResponse>();
     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({