From 1ef8866aae5c6d57dd8eba42dc37d3be08b36647 Mon Sep 17 00:00:00 2001 From: Colby Mchenry Date: Thu, 22 Oct 2026 01:38:45 +0000 Subject: [PATCH] test(explore): the cross-call dedup test passes with LF and CRLF checkouts alike (#2772) explore-cross-call-dedup's "never re-sends a line it already sent" case has failed on every LF checkout (Linux, macOS, CI) since PR #2655. It passed on Windows only because core.autocrlf writes the fixture with CRLF. Root cause. Since PR #2655 the payroll fixture's RunResult struct (cycle.go lines 31-37) references Payslip through a field, so call 1 renders it. Call 2 renders the file's head, and that 7-line held run comes back inside it: dedupeRange keeps a covered run shorter than MIN_COVERED_LINES (8) in the emit set on purpose, rule 3 of the range algebra, which its own unit test pins. The handler case counted any repeated line as a duplicate. On a CRLF checkout explore's per-file character budget also counts each line's carriage return, so cycle.go rendered fewer lines and the case never met the short run. Fix, test only: - The case groups the repeated lines into consecutive runs and fails on a run of MIN_COVERED_LINES lines or more, read from EXPLORE_DEDUP. A shorter run is the algebra's deliberate re-send. What agents receive is unchanged. - The suite gives its copy of the fixture LF line endings, so every checkout indexes and renders the same bytes. Validation on Windows, with the kernel and with CODEGRAPH_KERNEL=0: - main's test fails with an LF fixture ("call 2 re-sent internal/usecase/payroll/cycle.go lines 31,32,33,34,35") and passes with the CRLF one. The new test passes 26/26 with both. - With the normalisation and main's assertion, a CRLF checkout fails the way an LF one does (lines 31-37): both checkouts now test one scenario. - Teeth: an algebra mutated to re-send held runs of 8-23 lines fails the new case ("call 2 re-sent internal/usecase/payroll/cycle.go lines 42-51 (10 lines)"). - tsc on the test file is clean. Contributor PR #2758 by @danusha2345 reached the same diagnosis. This change was written separately and also normalises the fixture. Fixes #2756 Co-Authored-By: Claude Opus 5.5 --- __tests__/explore-cross-call-dedup.test.ts | 46 ++++++++++++++++++++-- 1 file changed, 42 insertions(+), 4 deletions(-) diff --git a/__tests__/explore-cross-call-dedup.test.ts b/__tests__/explore-cross-call-dedup.test.ts index ce208dc95..e18cc4371 100644 --- a/__tests__/explore-cross-call-dedup.test.ts +++ b/__tests__/explore-cross-call-dedup.test.ts @@ -42,6 +42,37 @@ const FIXTURE_SRC = path.join(__dirname, 'fixtures', 'payroll-go'); const QUERY = 'how does payroll cycle create and calculate payslips?'; const POINTER = 'Already sent earlier in this conversation'; +/** + * Give every file of the copied fixture LF line endings. A Windows checkout + * with `core.autocrlf` writes the fixture with CRLF, and explore's per-file + * character budget counts each line's `\r`, so there a file renders fewer + * lines and the second call meets different held spans than on Linux or macOS + * (#2756). The suite tests one scenario on every checkout. + */ +function normalizeLineEndings(dir: string): void { + for (const entry of fs.readdirSync(dir, { withFileTypes: true })) { + const file = path.join(dir, entry.name); + if (entry.isDirectory()) { + normalizeLineEndings(file); + continue; + } + // latin1 maps every byte to one char, so the rewrite changes nothing but the `\r`s. + const text = fs.readFileSync(file, 'latin1'); + if (text.includes('\r\n')) fs.writeFileSync(file, text.replace(/\r\n/g, '\n'), 'latin1'); + } +} + +/** Consecutive line numbers grouped into runs: [3, 4, 5, 9] → [[3, 4, 5], [9]]. */ +function consecutiveRuns(lines: Iterable): number[][] { + const runs: number[][] = []; + for (const n of [...lines].sort((a, b) => a - b)) { + const run = runs[runs.length - 1]; + if (run && n === run[run.length - 1]! + 1) run.push(n); + else runs.push([n]); + } + return runs; +} + describe('dedup configuration', () => { it('defaults off and requires an explicit truthy opt-in', () => { const previous = process.env.CODEGRAPH_EXPLORE_DEDUP; @@ -213,6 +244,7 @@ describe('a second call against a real index', () => { testDir = fs.mkdtempSync(path.join(os.tmpdir(), 'codegraph-cg18-')); fs.cpSync(FIXTURE_SRC, testDir, { recursive: true }); fs.rmSync(path.join(testDir, '.codegraph'), { recursive: true, force: true }); + normalizeLineEndings(testDir); cg = CodeGraph.initSync(testDir); await cg.indexAll(); handler = new ToolHandler(cg); @@ -266,10 +298,16 @@ describe('a second call against a real index', () => { expect(second).toContain(POINTER); for (const [file, lines] of before) { const repeated = [...(after.get(file) ?? [])].filter((n) => lines.has(n)); - if (repeated.length === 0) continue; - // The only sanctioned repeat is the anti-abandonment restore, which fires - // ONLY when the call found nothing new to say — and this one did. - throw new Error(`call 2 re-sent ${file} lines ${repeated.slice(0, 5).join(',')}`); + // Two repeats are sanctioned. The anti-abandonment restore fires ONLY + // when the call found nothing new to say, and this one did. And rule 3 + // of the range algebra: a held run shorter than MIN_COVERED_LINES stays + // inside the span around it rather than becoming a pointer (cycle.go's + // 7-line `RunResult` struct, held by call 1, comes back inside call 2's + // render of the file's head, #2756). A longer run is a real duplicate. + for (const run of consecutiveRuns(repeated)) { + if (run.length < EXPLORE_DEDUP.MIN_COVERED_LINES) continue; + throw new Error(`call 2 re-sent ${file} lines ${run[0]}-${run[run.length - 1]} (${run.length} lines)`); + } } }, 120_000);