From cfc73f8d0b1fa41bbf7b53ac5d78ea48bd3ada68 Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Fri, 3 Jul 2026 20:28:27 +0200 Subject: [PATCH] fix(graph-ui): surface indexing-job failures instead of silently completing The backend /api/index-status reports status:"error" plus an error message for failed indexing jobs, but IndexProgress treated any non-"indexing" state as successful completion: the spinner vanished with no feedback (e.g. after an OOM-killed indexer subprocess), the project never appeared, and no error was shown. Render a visible error banner (path + error text) with a Dismiss button instead, and keep the success flow unchanged. Beyond the original PR: - Restore the empty-jobs guard the PR dropped: the backend keeps finished jobs listed as "done"/"error" (handle_index_status only skips idle slots), so an empty list mid-index only occurs on transient state loss and must not be treated as completion. - Route the new user-facing strings through the i18n system (projects.indexingFailed, common.dismiss; EN + zh entries). - Cover error banner + dismiss, success flow and the empty-jobs guard with vitest cases on the now-exported IndexProgress. Distilled from #549. Refs #524 Co-authored-by: sahil-mangla Signed-off-by: Martin Vogel --- .gitignore | 3 + graph-ui/src/components/StatsTab.test.tsx | 147 +++++++++++++++++++++- graph-ui/src/components/StatsTab.tsx | 50 +++++++- graph-ui/src/lib/i18n.ts | 4 + graph-ui/vite.config.ts | 5 + 5 files changed, 200 insertions(+), 9 deletions(-) diff --git a/.gitignore b/.gitignore index 5860fb1b6..2edd00383 100644 --- a/.gitignore +++ b/.gitignore @@ -59,3 +59,6 @@ soak-results/ # LSP originality-check reference cache (scripts/check-lsp-originality.sh) .lsp-refs/ + +# Local npm cache +graph-ui/.npm-cache-local/ diff --git a/graph-ui/src/components/StatsTab.test.tsx b/graph-ui/src/components/StatsTab.test.tsx index 1a3df5b6f..f0cc22d58 100644 --- a/graph-ui/src/components/StatsTab.test.tsx +++ b/graph-ui/src/components/StatsTab.test.tsx @@ -1,8 +1,9 @@ /* @vitest-environment jsdom */ import "@testing-library/jest-dom/vitest"; -import { fireEvent, render, screen, waitFor } from "@testing-library/react"; -import { afterEach, describe, expect, it, vi } from "vitest"; -import { StatsTab } from "./StatsTab"; +import { fireEvent, render, screen, waitFor, act } from "@testing-library/react"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { StatsTab, IndexProgress } from "./StatsTab"; +import { messages } from "../lib/i18n"; function mockProjectsFetch(extra?: (url: string, init?: RequestInit) => Response | undefined) { const fetchMock = vi.fn(async (input: RequestInfo | URL, init?: RequestInit) => { @@ -93,3 +94,143 @@ describe("StatsTab index modal", () => { expect(screen.getByRole("button", { name: "Browse D:/" })).toBeInTheDocument(); }); }); + +describe("IndexProgress", () => { + beforeEach(() => { + vi.useFakeTimers(); + }); + + afterEach(() => { + vi.useRealTimers(); + vi.unstubAllGlobals(); + vi.restoreAllMocks(); + }); + + it("polls and shows indexing in progress when active", async () => { + const fetchMock = vi.fn().mockImplementation(() => + Promise.resolve({ + json: () => Promise.resolve([ + { slot: 1, status: "indexing", path: "/path/to/project1" } + ]) + } as unknown as Response) + ); + vi.stubGlobal("fetch", fetchMock); + + const onDone = vi.fn(); + render(); + + // Fast-forward initial poll + await act(async () => { + await vi.advanceTimersByTimeAsync(2000); + }); + + expect(fetchMock).toHaveBeenCalledWith("/api/index-status"); + expect(screen.getByText(messages.en.projects.indexingInProgress)).toBeInTheDocument(); + expect(screen.getByText("/path/to/project1")).toBeInTheDocument(); + expect(onDone).not.toHaveBeenCalled(); + }); + + it("stops polling and calls onDone when indexing finishes successfully", async () => { + // Backend keeps finished jobs listed with status "done" (src/ui/http_server.c + // handle_index_status only skips idle slots) — success is a "done" entry, + // not an empty list. + let mockData = [ + { slot: 1, status: "indexing", path: "/path/to/project" } + ]; + const fetchMock = vi.fn().mockImplementation(() => + Promise.resolve({ + json: () => Promise.resolve(mockData) + } as unknown as Response) + ); + vi.stubGlobal("fetch", fetchMock); + + const onDone = vi.fn(); + render(); + + // First poll returns active + await act(async () => { + await vi.advanceTimersByTimeAsync(2000); + }); + expect(onDone).not.toHaveBeenCalled(); + + // Indexing finishes + mockData = [ + { slot: 1, status: "done", path: "/path/to/project" } + ]; + + await act(async () => { + await vi.advanceTimersByTimeAsync(2000); + }); + + expect(onDone).toHaveBeenCalled(); + }); + + it("keeps waiting and does NOT call onDone while the jobs list is empty", async () => { + // An empty list mid-index means the job is not visible (e.g. transient + // backend state loss) — it must NOT be treated as successful completion. + let mockData: { slot: number; status: string; path: string }[] = []; + const fetchMock = vi.fn().mockImplementation(() => + Promise.resolve({ + json: () => Promise.resolve(mockData) + } as unknown as Response) + ); + vi.stubGlobal("fetch", fetchMock); + + const onDone = vi.fn(); + render(); + + // Two empty polls: still waiting, no premature completion + await act(async () => { + await vi.advanceTimersByTimeAsync(2000); + }); + await act(async () => { + await vi.advanceTimersByTimeAsync(2000); + }); + expect(onDone).not.toHaveBeenCalled(); + expect(fetchMock).toHaveBeenCalledTimes(2); + + // Job becomes visible and finishes — now completion fires + mockData = [{ slot: 1, status: "done", path: "/path/to/project" }]; + await act(async () => { + await vi.advanceTimersByTimeAsync(2000); + }); + expect(onDone).toHaveBeenCalled(); + }); + + it("renders error banner and does NOT call onDone when indexing fails with error status", async () => { + const fetchMock = vi.fn().mockImplementation(() => + Promise.resolve({ + json: () => Promise.resolve([ + { slot: 1, status: "error", path: "/path/to/failed-project", error: "OOM Error" } + ]) + } as unknown as Response) + ); + vi.stubGlobal("fetch", fetchMock); + + const onDone = vi.fn(); + render(); + + await act(async () => { + await vi.advanceTimersByTimeAsync(2000); + }); + + // Error banner should show up + expect(screen.getByText(messages.en.projects.indexingFailed)).toBeInTheDocument(); + expect(screen.getByText("/path/to/failed-project")).toBeInTheDocument(); + expect(screen.getByText("OOM Error")).toBeInTheDocument(); + + // onDone should not be called automatically + expect(onDone).not.toHaveBeenCalled(); + + // Click Dismiss button + const dismissBtn = screen.getByRole("button", { name: messages.en.common.dismiss }); + expect(dismissBtn).toBeInTheDocument(); + + await act(async () => { + fireEvent.click(dismissBtn); + }); + + // onDone should be called after manual dismissal + expect(onDone).toHaveBeenCalled(); + }); +}); diff --git a/graph-ui/src/components/StatsTab.tsx b/graph-ui/src/components/StatsTab.tsx index 3c439dce4..07e15fd5a 100644 --- a/graph-ui/src/components/StatsTab.tsx +++ b/graph-ui/src/components/StatsTab.tsx @@ -378,21 +378,39 @@ function CreateIndexModal({ onClose, onCreated }: { onClose: () => void; onCreat /* ── Index Progress ─────────────────────────────────────── */ -function IndexProgress({ onDone }: { onDone: () => void }) { +export function IndexProgress({ onDone }: { onDone: () => void }) { const t = useUiMessages(); - const [jobs, setJobs] = useState<{ slot: number; status: string; path: string }[]>([]); + const [jobs, setJobs] = useState<{ slot: number; status: string; path: string; error?: string }[]>([]); + const [hasActive, setHasActive] = useState(true); useEffect(() => { + if (!hasActive) return; const poll = setInterval(async () => { try { const data = await (await fetch("/api/index-status")).json(); setJobs(data); - if (data.length > 0 && data.every((j: { status: string }) => j.status !== "indexing")) onDone(); - } catch { /* */ } + const stillIndexing = data.some((j: { status: string }) => j.status === "indexing"); + /* Empty list = job not visible: the backend keeps finished jobs listed + as "done"/"error", so [] mid-index only happens on transient state + loss (e.g. server restart) — keep polling, don't treat as done. */ + if (data.length > 0 && !stillIndexing) { + setHasActive(false); + const hasErrors = data.some((j: { status: string }) => j.status === "error"); + if (!hasErrors) { + onDone(); + } + } + } catch (error) { + console.error("[IndexProgress] Poll failed:", error); + } }, 2000); return () => clearInterval(poll); - }, [onDone]); + }, [onDone, hasActive]); + const active = jobs.filter((j) => j.status === "indexing"); - if (active.length === 0) return null; + const errors = jobs.filter((j) => j.status === "error"); + + if (active.length === 0 && errors.length === 0) return null; + return (
{active.map((j) => ( @@ -404,6 +422,26 @@ function IndexProgress({ onDone }: { onDone: () => void }) {
))} + {errors.map((j) => ( +
+ ⚠️ +
+

{t.projects.indexingFailed}

+

{j.path}

+ {j.error &&

{j.error}

} +
+
+ ))} + {errors.length > 0 && ( +
+ +
+ )} ); } diff --git a/graph-ui/src/lib/i18n.ts b/graph-ui/src/lib/i18n.ts index 10c68c748..942d67ade 100644 --- a/graph-ui/src/lib/i18n.ts +++ b/graph-ui/src/lib/i18n.ts @@ -17,6 +17,7 @@ export const messages = { saving: "Saving...", delete: "Delete", noMatches: "No matches", + dismiss: "Dismiss", }, graph: { selectedLabel: "Graph", @@ -37,6 +38,7 @@ export const messages = { healthCorrupt: "Database unhealthy", healthChecking: "Checking...", indexingInProgress: "Indexing in progress", + indexingFailed: "Indexing failed", }, index: { newIndex: "New Index", @@ -86,6 +88,7 @@ export const messages = { saving: "保存中...", delete: "删除", noMatches: "无匹配结果", + dismiss: "关闭", }, graph: { selectedLabel: "图谱", @@ -106,6 +109,7 @@ export const messages = { healthCorrupt: "数据库异常", healthChecking: "检查中...", indexingInProgress: "正在索引", + indexingFailed: "索引失败", }, index: { newIndex: "新建索引", diff --git a/graph-ui/vite.config.ts b/graph-ui/vite.config.ts index 0d02daf8f..d7a3a0b85 100644 --- a/graph-ui/vite.config.ts +++ b/graph-ui/vite.config.ts @@ -1,3 +1,4 @@ +/// import { defineConfig } from "vite"; import react from "@vitejs/plugin-react"; import tailwindcss from "@tailwindcss/vite"; @@ -10,6 +11,10 @@ export default defineConfig({ "@": path.resolve(__dirname, "./src"), }, }, + test: { + environment: "jsdom", + globals: true, + }, build: { outDir: "dist", assetsDir: "assets",