Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 7 additions & 4 deletions src/review/rag.ts
Original file line number Diff line number Diff line change
Expand Up @@ -195,9 +195,9 @@ export function chunkFile(path: string, text: string, namespace = "", opts?: Chu
// file re-failed the whole upsert batch (up to 49 otherwise-good chunks rolled back with it) on every
// push and cron retry (GITTENSORY-D).
text = text.replace(/(?![\t\n\r])\p{Cc}/gu, " ");
// Clamp to safe ranges: chunkChars must be >= 1 (a 0/negative budget makes newlineChunks loop forever on an
// oversized file — a misconfig DOS) and overlap must be < chunkChars (else `end - overlap` can't advance).
// (#rag-verify infinite-loop guard)
// Clamp to safe ranges: chunkChars must be >= 1 (a 0/negative budget never advances the window) and
// overlap must be < chunkChars. Overlap alone is NOT enough to prevent stalls — newline snap can shrink
// `end` so `end - overlap <= start`; newlineChunks floors the next start at `start + 1` (#7447 / #rag-verify).
const chunkChars = Math.max(1, Math.floor(opts?.chunkChars ?? CHUNK_CHARS));
const chunkOverlap = Math.min(Math.max(0, Math.floor(opts?.chunkOverlap ?? CHUNK_OVERLAP)), chunkChars - 1);
// JS/TS: cut on LOGICAL boundaries (function/class/export) instead of arbitrary newlines, so a retrieved
Expand Down Expand Up @@ -291,7 +291,10 @@ function newlineChunks(path: string, text: string, kind: RagKind, namespace: str
chunks.push({ id: chunkId(namespace, path, idx), path, chunkIndex: idx, kind, text: text.slice(start, end), boundary: "file" });
idx += 1;
if (end >= text.length) break;
start = Math.max(0, end - chunkOverlap);
// #7447: newline snap can shrink `end` well below `start + chunkChars`, so `end - chunkOverlap` may not
// advance past the previous `start` even when overlap < chunkChars. Floor at start + 1 so every
// iteration makes forward progress (Math.max(0, …) is redundant once start >= 0, which the loop maintains).
start = Math.max(start + 1, end - chunkOverlap);
}
return chunks;
}
Expand Down
40 changes: 40 additions & 0 deletions test/unit/rag.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -240,6 +240,46 @@ describe("rag: per-file chunking", () => {
expect(chunks.every((c) => c.text.length > 0)).toBe(true);
});

it("REGRESSION (#7447): does NOT hang when newline snap + near-budget overlap would stall `end - overlap <= start`", () => {
// Exact repro from the issue: early qualifying newline (~index 60 > chunkChars/2) shrinks `end`, and
// chunkOverlap=99 (allowed by the old clamp of chunkChars-1) makes `end - overlap` land at/before the
// previous start. Without the start+1 floor this never returns.
const text = `${"x".repeat(60)}\n${"y".repeat(5000)}`;
const chunks = chunkFile("f.py", text, "", { chunkChars: 100, chunkOverlap: 99 });
expect(chunks.length).toBeGreaterThan(1);
// Worst case every iteration advances exactly 1 char → at most text.length chunks.
expect(chunks.length).toBeLessThanOrEqual(text.length);
expect(chunks.every((c) => c.text.length > 0)).toBe(true);
});

it.each([
{ chunkChars: 100, chunkOverlap: 99 },
{ chunkChars: 100, chunkOverlap: 50 },
{ chunkChars: 50, chunkOverlap: 49 },
{ chunkChars: 20, chunkOverlap: 19 },
{ chunkChars: 1, chunkOverlap: 0 },
{ chunkChars: 1, chunkOverlap: 999 }, // clamped to 0
])(
"REGRESSION (#7447): chunkFile returns finitely for chunkChars=$chunkChars chunkOverlap=$chunkOverlap (forward progress)",
({ chunkChars, chunkOverlap }) => {
const text = `${"x".repeat(60)}\n${"y".repeat(5000)}`;
const chunks = chunkFile("f.py", text, "", { chunkChars, chunkOverlap });
expect(chunks.length).toBeGreaterThan(0);
expect(chunks.length).toBeLessThanOrEqual(text.length);
},
);

it("REGRESSION (#7447): chunkJsTs oversized-unit fallback through newlineChunks also cannot stall on high overlap", () => {
// Two logical units so chunkJsTs runs; the first exceeds chunkChars and falls back to newlineChunks with
// the caller-supplied (near-budget) overlap — the other production path that must stay hang-free.
const huge = `export function big() {\n${"x".repeat(60)}\n${"y".repeat(5000)}\n}\n`;
const small = "export function small() { return 1; }\n";
const text = huge + small;
const chunks = chunkFile("src/huge.ts", text, "", { chunkChars: 100, chunkOverlap: 99 });
expect(chunks.length).toBeGreaterThan(0);
expect(chunks.length).toBeLessThanOrEqual(text.length);
});

it("PACKS a small multi-function JS file into one chunk (free-tier vector budget unaffected) (#282)", () => {
const chunks = chunkFile("src/small.ts", "export function a(){return 1;}\nexport function b(){return 2;}\nexport function c(){return 3;}\n");
expect(chunks).toHaveLength(1);
Expand Down