From 66c36b6423cbd9e2df7a1be9f2e9a60232cfab43 Mon Sep 17 00:00:00 2001 From: Clayton Date: Mon, 13 Jul 2026 06:43:52 -0500 Subject: [PATCH 1/2] feat(ui): add network-vs-API error distinction and loading skeletons to StateBoundary Several routes had minimal loading/error handling: no retry, no way to tell a connectivity failure apart from a server-side error, and a loading state that rendered identically to a genuinely-empty one. useApiResource now surfaces the apiFetch failure kind and status; StateBoundary/ErrorState use it to show distinct copy and iconography for "can't reach the server" vs "the server returned an error", and accept an optional content-shaped loadingSkeleton in place of the generic spinner. Both are additive and backward-compatible -- every existing StateBoundary/ErrorState/useApiResource consumer keeps its current default copy unless it opts in. Wires the new primitives into app.runs.tsx, the most minimal of the routes named in the issue: a proper StateBoundary (retry, network-aware error, loading skeleton) replaces the old ad-hoc warning banner and the misleading loading state that showed "no runs match filters" before the first fetch even completed, plus a RefreshMeta "last refresh" label in the header. --- .../src/components/site/state-views.test.tsx | 138 +++++++++ .../src/components/site/state-views.tsx | 73 ++++- .../src/lib/api/use-api-resource.test.ts | 40 +++ .../src/lib/api/use-api-resource.ts | 21 +- apps/gittensory-ui/src/routes/app.runs.tsx | 275 ++++++++++-------- 5 files changed, 406 insertions(+), 141 deletions(-) create mode 100644 apps/gittensory-ui/src/components/site/state-views.test.tsx diff --git a/apps/gittensory-ui/src/components/site/state-views.test.tsx b/apps/gittensory-ui/src/components/site/state-views.test.tsx new file mode 100644 index 0000000000..9e8fe3c561 --- /dev/null +++ b/apps/gittensory-ui/src/components/site/state-views.test.tsx @@ -0,0 +1,138 @@ +import { fireEvent, render, screen } from "@testing-library/react"; +import { describe, expect, it, vi } from "vitest"; + +const { notifyApiFailure } = vi.hoisted(() => ({ notifyApiFailure: vi.fn() })); +vi.mock("@/lib/api/request", () => ({ + notifyApiFailure: (...args: unknown[]) => notifyApiFailure(...args), +})); +vi.mock("sonner", () => ({ toast: Object.assign(vi.fn(), { success: vi.fn(), error: vi.fn() }) })); + +import { ErrorState, StateBoundary } from "@/components/site/state-views"; + +describe("ErrorState network-vs-API distinction (#793)", () => { + it("uses the generic 'couldn't load' copy when no errorKind is given (unchanged default)", () => { + render(); + expect(screen.getByText("Couldn't load this")).toBeTruthy(); + }); + + it("uses connectivity-specific copy for a network errorKind", () => { + render(); + expect(screen.getByText("Can't reach the server")).toBeTruthy(); + }); + + it("uses connectivity-specific copy for a timeout errorKind too", () => { + render(); + expect(screen.getByText("Can't reach the server")).toBeTruthy(); + }); + + it("falls back to the generic copy for an http errorKind", () => { + render(); + expect(screen.getByText("Couldn't load this")).toBeTruthy(); + }); + + it("lets an explicit title/description override the errorKind-derived copy", () => { + render(); + expect(screen.getByText("Custom title")).toBeTruthy(); + expect(screen.getByText("Custom description")).toBeTruthy(); + expect(screen.queryByText("Can't reach the server")).toBeNull(); + }); +}); + +describe("StateBoundary loadingSkeleton (#793)", () => { + it("renders the default spinner LoadingState when no skeleton is given", () => { + render( + +
content
+ , + ); + expect(screen.getByRole("status")).toBeTruthy(); + expect(screen.queryByText("content")).toBeNull(); + }); + + it("renders the provided skeleton instead of the spinner when loading", () => { + render( + placeholder}> +
content
+ , + ); + expect(screen.getByTestId("skeleton")).toBeTruthy(); + expect(screen.queryByRole("status")).toBeNull(); + expect(screen.queryByText("content")).toBeNull(); + }); +}); + +describe("StateBoundary errorKind passthrough (#793)", () => { + it("keeps the pre-#793 default error copy when no errorKind is given", () => { + render( + +
content
+ , + ); + expect(screen.getByText("Couldn't load data")).toBeTruthy(); + }); + + it("falls through to ErrorState's network-aware copy when errorKind is network and no override is given", () => { + render( + +
content
+ , + ); + expect(screen.getByText("Can't reach the server")).toBeTruthy(); + expect(screen.queryByText("Couldn't load data")).toBeNull(); + }); + + it("lets an explicit errorTitle/errorDescription win even with a network errorKind", () => { + render( + +
content
+ , + ); + expect(screen.getByText("Custom title")).toBeTruthy(); + expect(screen.getByText("Custom description")).toBeTruthy(); + }); + + it("passes the real errorKind (not a hardcoded 'network') to the error-failure notifier", () => { + render( + +
content
+ , + ); + expect(notifyApiFailure).toHaveBeenCalledWith(expect.objectContaining({ kind: "http" })); + }); + + it("defaults the notifier kind to 'network' when no errorKind is given, matching pre-#793 behavior", () => { + render( + +
content
+ , + ); + expect(notifyApiFailure).toHaveBeenCalledWith(expect.objectContaining({ kind: "network" })); + }); +}); + +describe("StateBoundary retry/refresh actions (#793 regression guard)", () => { + it("still invokes onRetry from the error state's retry button", () => { + const onRetry = vi.fn(); + render( + +
content
+ , + ); + fireEvent.click(screen.getByRole("button", { name: /try again/i })); + expect(onRetry).toHaveBeenCalledTimes(1); + }); + + it("renders children unchanged when neither loading, error, nor empty", () => { + render( + +
content
+ , + ); + expect(screen.getByText("content")).toBeTruthy(); + }); +}); diff --git a/apps/gittensory-ui/src/components/site/state-views.tsx b/apps/gittensory-ui/src/components/site/state-views.tsx index f9eafcf88e..c860be86ff 100644 --- a/apps/gittensory-ui/src/components/site/state-views.tsx +++ b/apps/gittensory-ui/src/components/site/state-views.tsx @@ -1,9 +1,13 @@ -import { Loader2, Inbox, AlertTriangle, RefreshCw } from "lucide-react"; +import { Loader2, Inbox, AlertTriangle, RefreshCw, WifiOff } from "lucide-react"; import { useCallback, useEffect, useState, type ReactNode } from "react"; import { toast } from "sonner"; import { cn } from "@/lib/utils"; -import { notifyApiFailure } from "@/lib/api/request"; +import { notifyApiFailure, type ApiFailureKind } from "@/lib/api/request"; + +/** `errorKind`s that mean "we never reached the server" as opposed to "the server answered with an + * error" — StateBoundary/ErrorState use this to show connectivity-specific copy and iconography (#793). */ +const NETWORK_ERROR_KINDS: ReadonlySet = new Set(["network", "timeout"]); /** * Shared state primitives so every panel/route renders a consistent @@ -140,8 +144,9 @@ export function StateActionButton({ } export function ErrorState({ - title = "Couldn't load this", - description = "Something went wrong fetching this data. You can retry, or come back in a moment. If this keeps happening, check status or try again shortly.", + title, + description, + errorKind, onRetry, retryLabel = "Try again", secondaryAction, @@ -150,18 +155,34 @@ export function ErrorState({ }: { title?: string; description?: ReactNode; + /** Distinguishes "couldn't reach the server at all" from "the server answered with an error" (#793). + * Only changes the default `title`/`description` — an explicit `title`/`description` always wins. */ + errorKind?: ApiFailureKind; onRetry?: () => void; retryLabel?: string; secondaryAction?: ReactNode; toastOnRetry?: boolean; className?: string; }) { + const isNetworkIssue = errorKind !== undefined && NETWORK_ERROR_KINDS.has(errorKind); + const resolvedTitle = title ?? (isNetworkIssue ? "Can't reach the server" : "Couldn't load this"); + const resolvedDescription = + description ?? + (isNetworkIssue + ? "We couldn't reach the API — check your connection and retry. This is a connectivity issue, not a problem with the data itself." + : "Something went wrong fetching this data. You can retry, or come back in a moment. If this keeps happening, check status or try again shortly."); return ( } - title={title} - description={description} + icon={ + isNetworkIssue ? ( + + ) : ( + + ) + } + title={resolvedTitle} + description={resolvedDescription} action={ onRetry && ( void; onRefresh?: () => void; /** When provided, surfaces a global API-failure toast with a Retry action. */ errorLabel?: string; children: ReactNode; }) { + const isNetworkIssue = errorKind !== undefined && NETWORK_ERROR_KINDS.has(errorKind); + // Preserve the pre-#793 defaults exactly when no errorKind is given; when one is given and the caller + // didn't override the copy, fall through to ErrorState's own network-aware defaults instead. + const resolvedErrorTitle = errorTitle ?? (isNetworkIssue ? undefined : "Couldn't load data"); + const resolvedErrorDescription = + errorDescription ?? + (isNetworkIssue + ? undefined + : "The data source did not respond. Retry the request, or check back once the service has recovered."); + // When this boundary flips into the error state, surface a toast with Retry. useEffect(() => { if (isError && errorLabel) { notifyApiFailure({ label: errorLabel, - kind: "network", + kind: errorKind ?? "network", message: - typeof errorDescription === "string" ? errorDescription : "Data source did not respond.", + typeof resolvedErrorDescription === "string" + ? resolvedErrorDescription + : "Data source did not respond.", retry: onRetry, }); } - }, [isError, errorLabel, errorDescription, onRetry]); + }, [isError, errorLabel, errorKind, resolvedErrorDescription, onRetry]); if (isLoading) { - return ; + return ( + loadingSkeleton ?? + ); } if (isError) { return ( { expect(result.current.loadedAt).toBeNull(); }); }); + +describe("useApiResource errorKind/errorStatus (#793)", () => { + it("carries the apiFetch failure kind and status through to the error state", async () => { + apiFetch.mockResolvedValue({ + ok: false, + kind: "http", + message: "500 Internal Server Error", + status: 500, + durationMs: 5, + }); + const { result } = renderHook(() => useApiResource("/v1/thing", "Thing")); + await waitFor(() => expect(result.current.status).toBe("error")); + expect(result.current).toMatchObject({ errorKind: "http", errorStatus: 500 }); + }); + + it("carries a network failure kind with no status", async () => { + apiFetch.mockResolvedValue({ ok: false, kind: "network", message: "fetch failed", durationMs: 5 }); + const { result } = renderHook(() => useApiResource("/v1/thing", "Thing")); + await waitFor(() => expect(result.current.status).toBe("error")); + expect(result.current).toMatchObject({ errorKind: "network", errorStatus: undefined }); + }); + + it("carries a timeout failure kind", async () => { + apiFetch.mockResolvedValue({ ok: false, kind: "timeout", message: "Request timed out", durationMs: 5 }); + const { result } = renderHook(() => useApiResource("/v1/thing", "Thing")); + await waitFor(() => expect(result.current.status).toBe("error")); + expect(result.current).toMatchObject({ errorKind: "timeout" }); + }); + + it("leaves errorKind undefined for the synthetic disabled sentinel", async () => { + const { result } = renderHook(() => + useApiResource("/v1/thing", "Thing", undefined, { enabled: false }), + ); + await waitFor(() => expect(result.current.status).toBe("error")); + const state = result.current; + if (state.status !== "error") throw new Error("expected error status"); + expect(state.error).toBe("disabled"); + expect(state.errorKind).toBeUndefined(); + }); +}); diff --git a/apps/gittensory-ui/src/lib/api/use-api-resource.ts b/apps/gittensory-ui/src/lib/api/use-api-resource.ts index fc59b3cdf7..14bf95cfb5 100644 --- a/apps/gittensory-ui/src/lib/api/use-api-resource.ts +++ b/apps/gittensory-ui/src/lib/api/use-api-resource.ts @@ -1,12 +1,20 @@ import { useCallback, useEffect, useState } from "react"; import { getApiOrigin } from "./origin"; -import { apiFetch } from "./request"; +import { apiFetch, type ApiFailureKind } from "./request"; type ResourceState = | { status: "loading"; data: null; error: null; loadedAt: null } | { status: "ready"; data: T; error: null; loadedAt: number } - | { status: "error"; data: null; error: string; loadedAt: null }; + | { + status: "error"; + data: null; + error: string; + /** Absent for the synthetic "disabled" sentinel below — only real `apiFetch` failures carry one (#793). */ + errorKind?: ApiFailureKind; + errorStatus?: number; + loadedAt: null; + }; type UseApiResourceOptions = { enabled?: boolean; @@ -42,7 +50,14 @@ export function useApiResource( if (result.ok) { setState({ status: "ready", data: result.data, error: null, loadedAt: Date.now() }); } else { - setState({ status: "error", data: null, error: result.message, loadedAt: null }); + setState({ + status: "error", + data: null, + error: result.message, + errorKind: result.kind, + errorStatus: result.status, + loadedAt: null, + }); } }, [enabled, label, path, token]); diff --git a/apps/gittensory-ui/src/routes/app.runs.tsx b/apps/gittensory-ui/src/routes/app.runs.tsx index 9c1e524d52..154e054b3f 100644 --- a/apps/gittensory-ui/src/routes/app.runs.tsx +++ b/apps/gittensory-ui/src/routes/app.runs.tsx @@ -24,7 +24,9 @@ import { } from "@/components/site/control-primitives"; import { useApiResource } from "@/lib/api/use-api-resource"; import { useSession } from "@/lib/api/session"; -import { EmptyState } from "@/components/site/state-views"; +import { EmptyState, StateBoundary } from "@/components/site/state-views"; +import { RefreshMeta } from "@/components/site/refresh-meta"; +import { Skeleton } from "@/components/ui/skeleton"; import { useLocalStorage } from "@/lib/use-local-storage"; import { cn } from "@/lib/utils"; import { SnapshotReplayCard } from "@/components/site/snapshot-replay"; @@ -232,139 +234,149 @@ function AgentRuns() { and a public/private boundary.

- {sourceLabel} - - - {canUseLiveRuns && liveRuns.status === "error" && liveRuns.error !== "disabled" && ( -
- Live runs are unavailable right now ({liveRuns.error}). +
+ {sourceLabel} +
- )} + -
-
-
- - Status -
-
- {STATUS_FILTERS.map((s) => ( - setStatus(s)}> - {s} - - ))} -
- -
- Kind -
-
- {KIND_FILTERS.map((k) => ( - setKind(k)}> - {k} - - ))} -
-
- - setQ(e.target.value)} - placeholder="Search runs…" - className="w-40 border-0 bg-transparent py-1 text-token-sm outline-none placeholder:text-muted-foreground" - /> + } + > +
+
+
+
+ + Status +
+
+ {STATUS_FILTERS.map((s) => ( + setStatus(s)}> + {s} + + ))} +
+ +
+ Kind +
+
+ {KIND_FILTERS.map((k) => ( + setKind(k)}> + {k} + + ))} +
+
+ + setQ(e.target.value)} + placeholder="Search runs…" + className="w-40 border-0 bg-transparent py-1 text-token-sm outline-none placeholder:text-muted-foreground" + /> +
+
-
-
- - navigate({ - search: () => ({ - status: v.status === "all" ? undefined : v.status, - kind: v.kind === "all" ? undefined : v.kind, - q: v.q ? v.q : undefined, - }), - replace: true, - }) - } - /> + + navigate({ + search: () => ({ + status: v.status === "all" ? undefined : v.status, + kind: v.kind === "all" ? undefined : v.kind, + q: v.q ? v.q : undefined, + }), + replace: true, + }) + } + /> -
- Showing {filtered.length} of {runs.length} -
+
+ Showing {filtered.length} of {runs.length} +
- {filtered.length === 0 ? ( -
    -
  • - { - setStatus("all"); - setKind("all"); - setQ(""); - toast("Filters cleared", { - description: "Showing all available agent runs again.", - }); - }} - className="inline-flex min-w-0 items-center justify-center rounded-token border border-border bg-transparent px-3 py-1.5 text-center text-token-xs font-medium text-foreground transition-all duration-150 hover:bg-accent focus-ring motion-reduce:transition-none motion-reduce:active:scale-100 active:scale-[0.98]" - > - Clear filters - - } - /> -
    • -
- ) : ( -
- {grouped.map((bucket) => ( -
-

- {bucket.label} · {bucket.runs.length} -

-
    - {bucket.runs.map((r) => ( -
  • + {filtered.length === 0 ? ( +
      +
    • + setSelected(r.id)} - aria-current={selectedId === r.id ? "true" : undefined} - className={cn( - "grid w-full grid-cols-[auto_minmax(0,1fr)_auto] items-center gap-3 rounded-token border bg-transparent p-3 text-left transition-all duration-150 focus-ring motion-reduce:transition-none motion-reduce:active:scale-100 active:scale-[0.99]", - selectedId === r.id - ? "border-mint/40 bg-mint/[0.04]" - : "border-border hover:border-foreground/30", - )} + onClick={() => { + setStatus("all"); + setKind("all"); + setQ(""); + toast("Filters cleared", { + description: "Showing all available agent runs again.", + }); + }} + className="inline-flex min-w-0 items-center justify-center rounded-token border border-border bg-transparent px-3 py-1.5 text-center text-token-xs font-medium text-foreground transition-all duration-150 hover:bg-accent focus-ring motion-reduce:transition-none motion-reduce:active:scale-100 active:scale-[0.98]" > - - {r.signal_fidelity} - -
      -
      {r.kind}
      -
      - {r.id} - · - {r.source} - · - {r.repo} -
      -
      -
      - {new Date(r.created_at).toUTCString().slice(5, 22)} -
      + Clear filters -
      • - ))} -
    -
- ))} + } + /> + + + ) : ( +
+ {grouped.map((bucket) => ( +
+

+ {bucket.label} · {bucket.runs.length} +

+
    + {bucket.runs.map((r) => ( +
  • + +
    • + ))} +
+
+ ))} +
+ )}
- )} + + +
    + {Array.from({ length: 5 }, (_, index) => ( +
  • + +
    • + ))} +
+
+ ); +} + function Chip({ active, onClick, From 2398dc2ffef667715ef06a87d11b63260c28dda4 Mon Sep 17 00:00:00 2001 From: Clayton Date: Mon, 13 Jul 2026 09:19:10 -0500 Subject: [PATCH 2/2] style(ui): fix prettier formatting in #793 test files --- .../src/components/site/state-views.test.tsx | 4 +++- .../src/lib/api/use-api-resource.test.ts | 14 ++++++++++++-- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/apps/gittensory-ui/src/components/site/state-views.test.tsx b/apps/gittensory-ui/src/components/site/state-views.test.tsx index 9e8fe3c561..5a64223d78 100644 --- a/apps/gittensory-ui/src/components/site/state-views.test.tsx +++ b/apps/gittensory-ui/src/components/site/state-views.test.tsx @@ -31,7 +31,9 @@ describe("ErrorState network-vs-API distinction (#793)", () => { }); it("lets an explicit title/description override the errorKind-derived copy", () => { - render(); + render( + , + ); expect(screen.getByText("Custom title")).toBeTruthy(); expect(screen.getByText("Custom description")).toBeTruthy(); expect(screen.queryByText("Can't reach the server")).toBeNull(); diff --git a/apps/gittensory-ui/src/lib/api/use-api-resource.test.ts b/apps/gittensory-ui/src/lib/api/use-api-resource.test.ts index 36ad5ccf6d..db92addc1d 100644 --- a/apps/gittensory-ui/src/lib/api/use-api-resource.test.ts +++ b/apps/gittensory-ui/src/lib/api/use-api-resource.test.ts @@ -52,14 +52,24 @@ describe("useApiResource errorKind/errorStatus (#793)", () => { }); it("carries a network failure kind with no status", async () => { - apiFetch.mockResolvedValue({ ok: false, kind: "network", message: "fetch failed", durationMs: 5 }); + apiFetch.mockResolvedValue({ + ok: false, + kind: "network", + message: "fetch failed", + durationMs: 5, + }); const { result } = renderHook(() => useApiResource("/v1/thing", "Thing")); await waitFor(() => expect(result.current.status).toBe("error")); expect(result.current).toMatchObject({ errorKind: "network", errorStatus: undefined }); }); it("carries a timeout failure kind", async () => { - apiFetch.mockResolvedValue({ ok: false, kind: "timeout", message: "Request timed out", durationMs: 5 }); + apiFetch.mockResolvedValue({ + ok: false, + kind: "timeout", + message: "Request timed out", + durationMs: 5, + }); const { result } = renderHook(() => useApiResource("/v1/thing", "Thing")); await waitFor(() => expect(result.current.status).toBe("error")); expect(result.current).toMatchObject({ errorKind: "timeout" });