Skip to content

Commit 552134c

Browse files
fix(review): guarantee newlineChunks forward progress under high overlap
Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent c18b123 commit 552134c

2 files changed

Lines changed: 47 additions & 4 deletions

File tree

src/review/rag.ts

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -195,9 +195,9 @@ export function chunkFile(path: string, text: string, namespace = "", opts?: Chu
195195
// file re-failed the whole upsert batch (up to 49 otherwise-good chunks rolled back with it) on every
196196
// push and cron retry (GITTENSORY-D).
197197
text = text.replace(/(?![\t\n\r])\p{Cc}/gu, " ");
198-
// Clamp to safe ranges: chunkChars must be >= 1 (a 0/negative budget makes newlineChunks loop forever on an
199-
// oversized file — a misconfig DOS) and overlap must be < chunkChars (else `end - overlap` can't advance).
200-
// (#rag-verify infinite-loop guard)
198+
// Clamp to safe ranges: chunkChars must be >= 1 (a 0/negative budget never advances the window) and
199+
// overlap must be < chunkChars. Overlap alone is NOT enough to prevent stalls — newline snap can shrink
200+
// `end` so `end - overlap <= start`; newlineChunks floors the next start at `start + 1` (#7447 / #rag-verify).
201201
const chunkChars = Math.max(1, Math.floor(opts?.chunkChars ?? CHUNK_CHARS));
202202
const chunkOverlap = Math.min(Math.max(0, Math.floor(opts?.chunkOverlap ?? CHUNK_OVERLAP)), chunkChars - 1);
203203
// JS/TS: cut on LOGICAL boundaries (function/class/export) instead of arbitrary newlines, so a retrieved
@@ -291,7 +291,10 @@ function newlineChunks(path: string, text: string, kind: RagKind, namespace: str
291291
chunks.push({ id: chunkId(namespace, path, idx), path, chunkIndex: idx, kind, text: text.slice(start, end), boundary: "file" });
292292
idx += 1;
293293
if (end >= text.length) break;
294-
start = Math.max(0, end - chunkOverlap);
294+
// #7447: newline snap can shrink `end` well below `start + chunkChars`, so `end - chunkOverlap` may not
295+
// advance past the previous `start` even when overlap < chunkChars. Floor at start + 1 so every
296+
// iteration makes forward progress (Math.max(0, …) is redundant once start >= 0, which the loop maintains).
297+
start = Math.max(start + 1, end - chunkOverlap);
295298
}
296299
return chunks;
297300
}

test/unit/rag.test.ts

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -240,6 +240,46 @@ describe("rag: per-file chunking", () => {
240240
expect(chunks.every((c) => c.text.length > 0)).toBe(true);
241241
});
242242

243+
it("REGRESSION (#7447): does NOT hang when newline snap + near-budget overlap would stall `end - overlap <= start`", () => {
244+
// Exact repro from the issue: early qualifying newline (~index 60 > chunkChars/2) shrinks `end`, and
245+
// chunkOverlap=99 (allowed by the old clamp of chunkChars-1) makes `end - overlap` land at/before the
246+
// previous start. Without the start+1 floor this never returns.
247+
const text = `${"x".repeat(60)}\n${"y".repeat(5000)}`;
248+
const chunks = chunkFile("f.py", text, "", { chunkChars: 100, chunkOverlap: 99 });
249+
expect(chunks.length).toBeGreaterThan(1);
250+
// Worst case every iteration advances exactly 1 char → at most text.length chunks.
251+
expect(chunks.length).toBeLessThanOrEqual(text.length);
252+
expect(chunks.every((c) => c.text.length > 0)).toBe(true);
253+
});
254+
255+
it.each([
256+
{ chunkChars: 100, chunkOverlap: 99 },
257+
{ chunkChars: 100, chunkOverlap: 50 },
258+
{ chunkChars: 50, chunkOverlap: 49 },
259+
{ chunkChars: 20, chunkOverlap: 19 },
260+
{ chunkChars: 1, chunkOverlap: 0 },
261+
{ chunkChars: 1, chunkOverlap: 999 }, // clamped to 0
262+
])(
263+
"REGRESSION (#7447): chunkFile returns finitely for chunkChars=$chunkChars chunkOverlap=$chunkOverlap (forward progress)",
264+
({ chunkChars, chunkOverlap }) => {
265+
const text = `${"x".repeat(60)}\n${"y".repeat(5000)}`;
266+
const chunks = chunkFile("f.py", text, "", { chunkChars, chunkOverlap });
267+
expect(chunks.length).toBeGreaterThan(0);
268+
expect(chunks.length).toBeLessThanOrEqual(text.length);
269+
},
270+
);
271+
272+
it("REGRESSION (#7447): chunkJsTs oversized-unit fallback through newlineChunks also cannot stall on high overlap", () => {
273+
// Two logical units so chunkJsTs runs; the first exceeds chunkChars and falls back to newlineChunks with
274+
// the caller-supplied (near-budget) overlap — the other production path that must stay hang-free.
275+
const huge = `export function big() {\n${"x".repeat(60)}\n${"y".repeat(5000)}\n}\n`;
276+
const small = "export function small() { return 1; }\n";
277+
const text = huge + small;
278+
const chunks = chunkFile("src/huge.ts", text, "", { chunkChars: 100, chunkOverlap: 99 });
279+
expect(chunks.length).toBeGreaterThan(0);
280+
expect(chunks.length).toBeLessThanOrEqual(text.length);
281+
});
282+
243283
it("PACKS a small multi-function JS file into one chunk (free-tier vector budget unaffected) (#282)", () => {
244284
const chunks = chunkFile("src/small.ts", "export function a(){return 1;}\nexport function b(){return 2;}\nexport function c(){return 3;}\n");
245285
expect(chunks).toHaveLength(1);

0 commit comments

Comments
 (0)