fix(review): drop whitespace-only RAG chunks and surface real embed errors

src/review/rag.ts

@@ -194,11 +194,15 @@ export function chunkFile(path: string, text: string, namespace = "", opts?: Chu
   // chunk is one coherent unit rather than "this function + 19 unrelated ones". Small files still collapse to
   // one chunk (the packer combines units up to chunkChars), so the per-repo vector budget is unaffected; a
   // single oversized unit falls back to newline splitting. (#282) Non-JS/TS keeps the newline chunker.
-  if (JS_TS_RE.test(path)) {
-    const logical = chunkJsTs(path, text, kind, namespace, chunkChars, chunkOverlap);
-    if (logical) return logical;
-  }
-  return newlineChunks(path, text, kind, namespace, chunkChars, chunkOverlap);
+  const chunks = JS_TS_RE.test(path) ? (chunkJsTs(path, text, kind, namespace, chunkChars, chunkOverlap) ?? newlineChunks(path, text, kind, namespace, chunkChars, chunkOverlap)) : newlineChunks(path, text, kind, namespace, chunkChars, chunkOverlap);
+  // #4996: the whole-FILE non-empty check above (`!text.trim()`) doesn't guarantee every individual SLICE is
+  // non-empty -- a newline-boundary split can land a chunk entirely inside a run of blank/whitespace-only
+  // lines (e.g. a large trailing gap between logical units). An empty/whitespace-only chunk reaching the embed
+  // API was the most plausible cause of the ai_embed_http_400 failures observed in production (Ollama's
+  // OpenAI-compatible /embeddings endpoint rejects it); filtering here is the single, centralized guard, since
+  // both chunkJsTs and newlineChunks can produce one. chunkIndex intentionally keeps its ORIGINAL (pre-filter)
+  // value -- it only needs to be a stable id component, not a gap-free sequence.
+  return chunks.filter((c) => c.text.trim().length > 0);
 }
 
 const JS_TS_RE = /\.(ts|tsx|js|jsx|mjs|cjs)$/i;

src/selfhost/ai.ts

@@ -286,7 +286,15 @@ export function createOpenAiCompatibleAi(opts: {
           body: JSON.stringify({ model: opts.embedModel ?? "bge-m3", input: options.text }),
           signal: AbortSignal.timeout(120_000),
         });
-        if (!res.ok) throw new Error(`ai_embed_http_${res.status}`);
+        // #4996: the error previously carried only the status code, with no detail from the response body --
+        // diagnosing a real production ai_embed_http_400 required SSHing into the box and reasoning about
+        // likely causes from first principles, since the actual rejection reason (e.g. Ollama's own "input
+        // length exceeds..." message) was thrown away. Bounded read, best-effort (a body-read failure must
+        // never mask the original status in the thrown error).
+        if (!res.ok) {
+          const detail = await res.text().then((t) => t.slice(0, 300)).catch(() => "");
+          throw new Error(detail ? `ai_embed_http_${res.status}: ${detail}` : `ai_embed_http_${res.status}`);
+        }
         const json = (await res.json()) as { data?: Array<{ embedding: number[] }> };
         return { data: (json.data ?? []).map((d) => d.embedding) };
       }

test/unit/rag.test.ts

@@ -195,6 +195,20 @@ describe("rag: per-file chunking", () => {
     expect(chunkFile("src/a.ts", "   ")).toEqual([]);
   });
 
+  it("REGRESSION (#4996): filters out an individual whitespace-only chunk, even though the whole file is non-empty (the most plausible cause of production ai_embed_http_400s)", () => {
+    // chunkChars=10, overlap=0: chunk0 = 10 X's, chunk1 = 10 SPACES (whitespace-only -- must be dropped),
+    // chunk2 = 10 Y's. Non-JS path (.py) so this exercises newlineChunks, the chunker actually used in prod
+    // for the file types most commonly indexed by size (large generated/data-adjacent code files).
+    const text = "X".repeat(10) + " ".repeat(10) + "Y".repeat(10);
+    const chunks = chunkFile("src/big.py", text, "", { chunkChars: 10, chunkOverlap: 0 });
+    expect(chunks.every((c) => c.text.trim().length > 0)).toBe(true);
+    expect(chunks.map((c) => c.text)).toEqual(["X".repeat(10), "Y".repeat(10)]);
+    // chunkIndex/id keep their ORIGINAL (pre-filter) position -- the surviving Y chunk is still index 2, not
+    // renumbered to 1, since it's only ever used as a stable id component, not required to be gap-free.
+    expect(chunks[1]?.chunkIndex).toBe(2);
+    expect(chunks[1]?.id).toBe("src/big.py::2");
+  });
+
   it("splits a JS/TS file at FUNCTION boundaries, never mid-function, tagging the boundary kind (#282)", () => {
     const fn = (n: number) => `export function f${n}() {\n${Array.from({ length: 120 }, (_, i) => `  const v${i} = ${i}; // padding aaaaaaaaaaaaaaaaaaaaaaaa`).join("\n")}\n}\n`;
     const chunks = chunkFile("src/multi.ts", fn(1) + fn(2) + fn(3)); // 3 functions, each ~6k; total > CHUNK_CHARS

test/unit/selfhost-ai.test.ts

@@ -184,9 +184,30 @@ describe("createOpenAiCompatibleAi (#979)", () => {
     expect(out).toEqual({ data: [[0.1, 0.2], [0.3, 0.4]] });
   });
 
-  it("throws on a non-OK embeddings response", async () => {
-    vi.stubGlobal("fetch", vi.fn(async () => new Response("e", { status: 502 })));
-    await expect(createOpenAiCompatibleAi({ baseUrl: "http://x/v1" }).run("m", { text: ["a"] })).rejects.toThrow(/ai_embed_http_502/);
+  it("throws on a non-OK embeddings response, including the response body detail (#4996: previously thrown away)", async () => {
+    vi.stubGlobal("fetch", vi.fn(async () => new Response("bad request: input exceeds max length", { status: 400 })));
+    await expect(createOpenAiCompatibleAi({ baseUrl: "http://x/v1" }).run("m", { text: ["a"] })).rejects.toThrow(
+      "ai_embed_http_400: bad request: input exceeds max length",
+    );
+  });
+
+  it("falls back to the bare status code when the error response body is empty", async () => {
+    vi.stubGlobal("fetch", vi.fn(async () => new Response("", { status: 502 })));
+    await expect(createOpenAiCompatibleAi({ baseUrl: "http://x/v1" }).run("m", { text: ["a"] })).rejects.toThrow(/^ai_embed_http_502$/);
+  });
+
+  it("still throws the bare status code (never masked) when reading the error response body itself fails", async () => {
+    vi.stubGlobal(
+      "fetch",
+      vi.fn(async () => ({ ok: false, status: 503, text: () => Promise.reject(new Error("stream error")) }) as unknown as Response),
+    );
+    await expect(createOpenAiCompatibleAi({ baseUrl: "http://x/v1" }).run("m", { text: ["a"] })).rejects.toThrow(/^ai_embed_http_503$/);
+  });
+
+  it("bounds the captured error detail to 300 chars", async () => {
+    vi.stubGlobal("fetch", vi.fn(async () => new Response("x".repeat(1000), { status: 400 })));
+    const error = await createOpenAiCompatibleAi({ baseUrl: "http://x/v1" }).run("m", { text: ["a"] }).catch((e: Error) => e.message);
+    expect(error).toBe(`ai_embed_http_400: ${"x".repeat(300)}`);
   });
 
   it("empty text array returns { data: [] } without a fetch", async () => {