Skip to content

Commit cfc73f8

Browse files
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 DeusData#549. Refs DeusData#524 Co-authored-by: sahil-mangla <manglasahil2017@gmail.com> Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
1 parent 09148ab commit cfc73f8

5 files changed

Lines changed: 200 additions & 9 deletions

File tree

.gitignore

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,3 +59,6 @@ soak-results/
5959

6060
# LSP originality-check reference cache (scripts/check-lsp-originality.sh)
6161
.lsp-refs/
62+
63+
# Local npm cache
64+
graph-ui/.npm-cache-local/

graph-ui/src/components/StatsTab.test.tsx

Lines changed: 144 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
11
/* @vitest-environment jsdom */
22
import "@testing-library/jest-dom/vitest";
3-
import { fireEvent, render, screen, waitFor } from "@testing-library/react";
4-
import { afterEach, describe, expect, it, vi } from "vitest";
5-
import { StatsTab } from "./StatsTab";
3+
import { fireEvent, render, screen, waitFor, act } from "@testing-library/react";
4+
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
5+
import { StatsTab, IndexProgress } from "./StatsTab";
6+
import { messages } from "../lib/i18n";
67

78
function mockProjectsFetch(extra?: (url: string, init?: RequestInit) => Response | undefined) {
89
const fetchMock = vi.fn(async (input: RequestInfo | URL, init?: RequestInit) => {
@@ -93,3 +94,143 @@ describe("StatsTab index modal", () => {
9394
expect(screen.getByRole("button", { name: "Browse D:/" })).toBeInTheDocument();
9495
});
9596
});
97+
98+
describe("IndexProgress", () => {
99+
beforeEach(() => {
100+
vi.useFakeTimers();
101+
});
102+
103+
afterEach(() => {
104+
vi.useRealTimers();
105+
vi.unstubAllGlobals();
106+
vi.restoreAllMocks();
107+
});
108+
109+
it("polls and shows indexing in progress when active", async () => {
110+
const fetchMock = vi.fn().mockImplementation(() =>
111+
Promise.resolve({
112+
json: () => Promise.resolve([
113+
{ slot: 1, status: "indexing", path: "/path/to/project1" }
114+
])
115+
} as unknown as Response)
116+
);
117+
vi.stubGlobal("fetch", fetchMock);
118+
119+
const onDone = vi.fn();
120+
render(<IndexProgress onDone={onDone} />);
121+
122+
// Fast-forward initial poll
123+
await act(async () => {
124+
await vi.advanceTimersByTimeAsync(2000);
125+
});
126+
127+
expect(fetchMock).toHaveBeenCalledWith("/api/index-status");
128+
expect(screen.getByText(messages.en.projects.indexingInProgress)).toBeInTheDocument();
129+
expect(screen.getByText("/path/to/project1")).toBeInTheDocument();
130+
expect(onDone).not.toHaveBeenCalled();
131+
});
132+
133+
it("stops polling and calls onDone when indexing finishes successfully", async () => {
134+
// Backend keeps finished jobs listed with status "done" (src/ui/http_server.c
135+
// handle_index_status only skips idle slots) — success is a "done" entry,
136+
// not an empty list.
137+
let mockData = [
138+
{ slot: 1, status: "indexing", path: "/path/to/project" }
139+
];
140+
const fetchMock = vi.fn().mockImplementation(() =>
141+
Promise.resolve({
142+
json: () => Promise.resolve(mockData)
143+
} as unknown as Response)
144+
);
145+
vi.stubGlobal("fetch", fetchMock);
146+
147+
const onDone = vi.fn();
148+
render(<IndexProgress onDone={onDone} />);
149+
150+
// First poll returns active
151+
await act(async () => {
152+
await vi.advanceTimersByTimeAsync(2000);
153+
});
154+
expect(onDone).not.toHaveBeenCalled();
155+
156+
// Indexing finishes
157+
mockData = [
158+
{ slot: 1, status: "done", path: "/path/to/project" }
159+
];
160+
161+
await act(async () => {
162+
await vi.advanceTimersByTimeAsync(2000);
163+
});
164+
165+
expect(onDone).toHaveBeenCalled();
166+
});
167+
168+
it("keeps waiting and does NOT call onDone while the jobs list is empty", async () => {
169+
// An empty list mid-index means the job is not visible (e.g. transient
170+
// backend state loss) — it must NOT be treated as successful completion.
171+
let mockData: { slot: number; status: string; path: string }[] = [];
172+
const fetchMock = vi.fn().mockImplementation(() =>
173+
Promise.resolve({
174+
json: () => Promise.resolve(mockData)
175+
} as unknown as Response)
176+
);
177+
vi.stubGlobal("fetch", fetchMock);
178+
179+
const onDone = vi.fn();
180+
render(<IndexProgress onDone={onDone} />);
181+
182+
// Two empty polls: still waiting, no premature completion
183+
await act(async () => {
184+
await vi.advanceTimersByTimeAsync(2000);
185+
});
186+
await act(async () => {
187+
await vi.advanceTimersByTimeAsync(2000);
188+
});
189+
expect(onDone).not.toHaveBeenCalled();
190+
expect(fetchMock).toHaveBeenCalledTimes(2);
191+
192+
// Job becomes visible and finishes — now completion fires
193+
mockData = [{ slot: 1, status: "done", path: "/path/to/project" }];
194+
await act(async () => {
195+
await vi.advanceTimersByTimeAsync(2000);
196+
});
197+
expect(onDone).toHaveBeenCalled();
198+
});
199+
200+
it("renders error banner and does NOT call onDone when indexing fails with error status", async () => {
201+
const fetchMock = vi.fn().mockImplementation(() =>
202+
Promise.resolve({
203+
json: () => Promise.resolve([
204+
{ slot: 1, status: "error", path: "/path/to/failed-project", error: "OOM Error" }
205+
])
206+
} as unknown as Response)
207+
);
208+
vi.stubGlobal("fetch", fetchMock);
209+
210+
const onDone = vi.fn();
211+
render(<IndexProgress onDone={onDone} />);
212+
213+
await act(async () => {
214+
await vi.advanceTimersByTimeAsync(2000);
215+
});
216+
217+
// Error banner should show up
218+
expect(screen.getByText(messages.en.projects.indexingFailed)).toBeInTheDocument();
219+
expect(screen.getByText("/path/to/failed-project")).toBeInTheDocument();
220+
expect(screen.getByText("OOM Error")).toBeInTheDocument();
221+
222+
// onDone should not be called automatically
223+
expect(onDone).not.toHaveBeenCalled();
224+
225+
// Click Dismiss button
226+
const dismissBtn = screen.getByRole("button", { name: messages.en.common.dismiss });
227+
expect(dismissBtn).toBeInTheDocument();
228+
229+
await act(async () => {
230+
fireEvent.click(dismissBtn);
231+
});
232+
233+
// onDone should be called after manual dismissal
234+
expect(onDone).toHaveBeenCalled();
235+
});
236+
});

graph-ui/src/components/StatsTab.tsx

Lines changed: 44 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -378,21 +378,39 @@ function CreateIndexModal({ onClose, onCreated }: { onClose: () => void; onCreat
378378

379379
/* ── Index Progress ─────────────────────────────────────── */
380380

381-
function IndexProgress({ onDone }: { onDone: () => void }) {
381+
export function IndexProgress({ onDone }: { onDone: () => void }) {
382382
const t = useUiMessages();
383-
const [jobs, setJobs] = useState<{ slot: number; status: string; path: string }[]>([]);
383+
const [jobs, setJobs] = useState<{ slot: number; status: string; path: string; error?: string }[]>([]);
384+
const [hasActive, setHasActive] = useState(true);
384385
useEffect(() => {
386+
if (!hasActive) return;
385387
const poll = setInterval(async () => {
386388
try {
387389
const data = await (await fetch("/api/index-status")).json();
388390
setJobs(data);
389-
if (data.length > 0 && data.every((j: { status: string }) => j.status !== "indexing")) onDone();
390-
} catch { /* */ }
391+
const stillIndexing = data.some((j: { status: string }) => j.status === "indexing");
392+
/* Empty list = job not visible: the backend keeps finished jobs listed
393+
as "done"/"error", so [] mid-index only happens on transient state
394+
loss (e.g. server restart) — keep polling, don't treat as done. */
395+
if (data.length > 0 && !stillIndexing) {
396+
setHasActive(false);
397+
const hasErrors = data.some((j: { status: string }) => j.status === "error");
398+
if (!hasErrors) {
399+
onDone();
400+
}
401+
}
402+
} catch (error) {
403+
console.error("[IndexProgress] Poll failed:", error);
404+
}
391405
}, 2000);
392406
return () => clearInterval(poll);
393-
}, [onDone]);
407+
}, [onDone, hasActive]);
408+
394409
const active = jobs.filter((j) => j.status === "indexing");
395-
if (active.length === 0) return null;
410+
const errors = jobs.filter((j) => j.status === "error");
411+
412+
if (active.length === 0 && errors.length === 0) return null;
413+
396414
return (
397415
<div className="rounded-xl border border-primary/20 bg-primary/5 p-4 mb-6">
398416
{active.map((j) => (
@@ -404,6 +422,26 @@ function IndexProgress({ onDone }: { onDone: () => void }) {
404422
</div>
405423
</div>
406424
))}
425+
{errors.map((j) => (
426+
<div key={j.slot} className="flex items-start gap-3 mt-3 first:mt-0 p-3 rounded-lg border border-destructive/20 bg-destructive/5 text-destructive">
427+
<span className="text-[14px]">⚠️</span>
428+
<div className="flex-1 min-w-0">
429+
<p className="text-[12px] font-semibold">{t.projects.indexingFailed}</p>
430+
<p className="text-[11px] font-mono truncate">{j.path}</p>
431+
{j.error && <p className="text-[10px] opacity-75 mt-1 font-mono">{j.error}</p>}
432+
</div>
433+
</div>
434+
))}
435+
{errors.length > 0 && (
436+
<div className="flex justify-end mt-3">
437+
<button
438+
onClick={onDone}
439+
className="px-3 py-1 rounded bg-destructive/10 hover:bg-destructive/20 text-destructive text-[11px] font-medium transition-all"
440+
>
441+
{t.common.dismiss}
442+
</button>
443+
</div>
444+
)}
407445
</div>
408446
);
409447
}

graph-ui/src/lib/i18n.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ export const messages = {
1717
saving: "Saving...",
1818
delete: "Delete",
1919
noMatches: "No matches",
20+
dismiss: "Dismiss",
2021
},
2122
graph: {
2223
selectedLabel: "Graph",
@@ -37,6 +38,7 @@ export const messages = {
3738
healthCorrupt: "Database unhealthy",
3839
healthChecking: "Checking...",
3940
indexingInProgress: "Indexing in progress",
41+
indexingFailed: "Indexing failed",
4042
},
4143
index: {
4244
newIndex: "New Index",
@@ -86,6 +88,7 @@ export const messages = {
8688
saving: "保存中...",
8789
delete: "删除",
8890
noMatches: "无匹配结果",
91+
dismiss: "关闭",
8992
},
9093
graph: {
9194
selectedLabel: "图谱",
@@ -106,6 +109,7 @@ export const messages = {
106109
healthCorrupt: "数据库异常",
107110
healthChecking: "检查中...",
108111
indexingInProgress: "正在索引",
112+
indexingFailed: "索引失败",
109113
},
110114
index: {
111115
newIndex: "新建索引",

graph-ui/vite.config.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
/// <reference types="vitest" />
12
import { defineConfig } from "vite";
23
import react from "@vitejs/plugin-react";
34
import tailwindcss from "@tailwindcss/vite";
@@ -10,6 +11,10 @@ export default defineConfig({
1011
"@": path.resolve(__dirname, "./src"),
1112
},
1213
},
14+
test: {
15+
environment: "jsdom",
16+
globals: true,
17+
},
1318
build: {
1419
outDir: "dist",
1520
assetsDir: "assets",

0 commit comments

Comments
 (0)