From 9ef54e4a89e10114b40e18330df96b00b0f69c77 Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Sat, 27 Jun 2026 15:45:24 -0700 Subject: [PATCH] fix(observability): surface RAG index-population failures to Sentry MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit fetchRepoTree and listStoredChunkPaths logged their catch failures via console.log with no level, so the central Sentry forwarder (level:error/fatal only) never saw them — a broken RAG index population (GitHub tree fetch down, D1 read error) silently degraded retrieval quality with no signal. Promote both to level:error (event rag_index_tree_error / rag_list_paths_error, keeping the ev tag for log continuity), mirroring the rag.ts pattern from #1619. Both stay fail-safe; only visibility changes. Covered by the existing tree-throws and stored-paths-read-error tests, now asserting the level:error log. --- src/review/rag-index.ts | 4 ++-- test/unit/rag-index.test.ts | 12 ++++++++++-- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/src/review/rag-index.ts b/src/review/rag-index.ts index 6662bffe6e..8a42eee5cb 100644 --- a/src/review/rag-index.ts +++ b/src/review/rag-index.ts @@ -83,7 +83,7 @@ async function fetchRepoTree(env: Env, repoFullName: string, ref: string, token: } return entries; } catch (error) { - console.log(JSON.stringify({ ev: "rag_index_tree_error", repo: repoFullName, message: String(error).slice(0, 200) })); + console.error(JSON.stringify({ level: "error", event: "rag_index_tree_error", ev: "rag_index_tree_error", repo: repoFullName, message: String(error).slice(0, 200) })); return null; } } @@ -178,7 +178,7 @@ async function listStoredChunkPaths(infra: ReturnType(); return (rows.results ?? []).map((row) => row.path).filter((path) => typeof path === "string" && path.length > 0); } catch (error) { - console.log(JSON.stringify({ ev: "rag_list_paths_error", project, repo, message: String(error).slice(0, 200) })); + console.error(JSON.stringify({ level: "error", event: "rag_list_paths_error", ev: "rag_list_paths_error", project, repo, message: String(error).slice(0, 200) })); return []; } } diff --git a/test/unit/rag-index.test.ts b/test/unit/rag-index.test.ts index 153eef8598..80a9d16ae9 100644 --- a/test/unit/rag-index.test.ts +++ b/test/unit/rag-index.test.ts @@ -173,18 +173,23 @@ describe("indexRepo: full repo index (tree → chunk → embed → upsert)", () expect(vec.upserted.length).toBe(0); }); - it("a tree fetch that THROWS degrades to nothing indexed (fetchRepoTree catch arm)", async () => { + it("a tree fetch that THROWS degrades to nothing indexed (fetchRepoTree catch arm) + surfaces it at ERROR for Sentry (#5)", async () => { const { env, vec } = indexEnv(); + const errSpy = vi.spyOn(console, "error").mockImplementation(() => {}); vi.stubGlobal("fetch", async (input: RequestInfo | URL) => { if (input.toString().includes("/git/trees/")) throw new Error("network down"); return new Response("missing", { status: 404 }); }); await expect(indexRepo(env, PROJECT, REPO)).resolves.toEqual({ indexed: 0, files: 0, capped: false }); expect(vec.upserted.length).toBe(0); + // A broken RAG index-population (tree fetch) now surfaces at level:error → captured by the central Sentry forwarder. + expect(errSpy.mock.calls.some((c) => String(c[0]).includes("rag_index_tree_error") && String(c[0]).includes('"level":"error"'))).toBe(true); + errSpy.mockRestore(); }); - it("a storage error while listing stored paths is fail-safe (prunes nothing, still indexes)", async () => { + it("a storage error while listing stored paths is fail-safe (prunes nothing, still indexes) + surfaces it at ERROR for Sentry (#5)", async () => { const { env } = indexEnv(); + const errSpy = vi.spyOn(console, "error").mockImplementation(() => {}); // Make ONLY the listStoredChunkPaths SELECT throw; everything else uses the real test D1. const realPrepare = env.DB.prepare.bind(env.DB); env.DB.prepare = ((query: string) => @@ -198,6 +203,9 @@ describe("indexRepo: full repo index (tree → chunk → embed → upsert)", () // The list failed → [] → nothing pruned, but the current file still indexes (fail-safe). expect(result.files).toBe(1); expect(await pathsFor(env, PROJECT, "gittensory")).toContain("src/current.ts"); + // A broken stored-paths read now surfaces at level:error → captured by the central Sentry forwarder. + expect(errSpy.mock.calls.some((c) => String(c[0]).includes("rag_list_paths_error") && String(c[0]).includes('"level":"error"'))).toBe(true); + errSpy.mockRestore(); }); it("listStoredChunkPaths drops blank paths and tolerates an absent result set (defensive branches)", async () => {