From d8f2eeaddffd8c993f727d57bb621c8ffbc52abc Mon Sep 17 00:00:00 2001 From: Colby Mchenry Date: Thu, 20 Aug 2026 12:36:22 -0500 Subject: [PATCH] fix(db): loop-append dense unresolved-ref result rows; make stripped-salvage visible (#1558) (#1576) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Real-world validation of #1575 on indexes damaged by the released v1.5.0 binary surfaced both of these. getUnresolvedReferencesByFiles chunked its INPUT under SQLite's parameter limit but appended each chunk's RESULT rows with a spread — every row becomes a call argument, so a dense recovery sync (the #1541 self-heal re-indexing 919 files produced 234,440 rows) exceeded V8's argument limit and killed resolution mid-sync with "Maximum call stack size exceeded", leaving the graph 226k edges short until another sync resumed the orphans (and that sweep resolves measurably worse than the batched path — see the follow-up issue). The failed-ref retry loader had the identical pattern on unbounded result rows. Both append with a loop now (#1558). The #1575 stripped-salvage warning also never rendered: init's summary prints only index_partial warnings and counts only hard errors, so a run with salvaged files still read as fully clean — and with no hard errors the detail wasn't written to errors.log either. Salvage entries now carry code 'salvaged_stripped', the summary prints a visible warning naming the files, and errors.log is written for salvage-only runs. Validated on real corpora with full-graph dumps: healthy-path inits stay byte-identical to the pre-#1575 baseline (cpython Lib, Alamofire, with a determinism control); a realistically-damaged index (41 wiped + 5 missing files, damage generated by the released binary) heals in one plain sync to identical per-file counts and an edge set within the normal incremental residual; pathological mass damage (52% of the repo) completes without crashing. New regression test reproduces the RangeError on the old code with 200k pending refs. Co-authored-by: Claude Fable 5 --- CHANGELOG.md | 1 + __tests__/refs-by-files-spread.test.ts | 74 ++++++++++++++++++++++++++ src/bin/codegraph.ts | 23 ++++++-- src/db/queries.ts | 12 ++++- src/extraction/index.ts | 1 + 5 files changed, 106 insertions(+), 5 deletions(-) create mode 100644 __tests__/refs-by-files-spread.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index aae12c4..32724a5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -49,6 +49,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). - The blast-radius section of `codegraph_explore` flagged "no covering tests found" whenever no test called a symbol directly — falsely branding helpers that tests exercise through their callers as untested (about 40% of flagged symbols in a measured sample). The check now follows caller chains up to 3 hops and reports indirect coverage as "tested via callers"; when nothing is found it states exactly what was checked instead of an unconditional warning. Thanks @inth3shadows for measuring the false-positive rate. (#1475) - Fixed a v1.5.0 regression where a perfectly valid file could be permanently recorded as having 0 symbols, with no error reported. When a file's first parse attempt was interrupted — a parsing worker crash or timeout, most likely on slow or heavily loaded machines — the automatic retry stored an empty result for any language on the native extraction path, so the file's functions and classes silently vanished from search, callers, and impact until the file was next edited. Retries now store the file's real symbols, and a file already recorded as symbol-free is detected and repaired automatically by the next sync or re-index after upgrading. Thanks @Baiae for the report. (#1541) - When indexing has to fall back to parsing a file with its comment lines stripped — a last-resort recovery after repeated parser crashes — the file is now flagged with a visible warning instead of being reported as cleanly indexed. The recovered result can be incomplete, and reporting success made a fresh index quietly disagree with a later re-parse of the same unchanged file. Thanks @jeremypetz for the precise init-versus-sync symbol accounting that exposed this. (#1565) +- Syncing a large batch of changed files no longer crashes with "Maximum call stack size exceeded" partway through. The crash aborted reference resolution after the files' symbols were already stored, leaving the graph with far fewer connections than a fresh index would have — and it hit exactly the scenarios that re-parse many files at once, including the automatic repair above. Thanks @netbrah for pinpointing the failure. (#1558) ## [1.5.0] - 2026-07-21 diff --git a/__tests__/refs-by-files-spread.test.ts b/__tests__/refs-by-files-spread.test.ts new file mode 100644 index 0000000..7d93429 --- /dev/null +++ b/__tests__/refs-by-files-spread.test.ts @@ -0,0 +1,74 @@ +/** + * getUnresolvedReferencesByFiles must survive dense result sets (#1558). + * + * The input file-path list is chunked under SQLite's parameter limit, but the + * ROWS a chunk returns are unbounded — and appending them with + * `rows.push(...chunkRows)` passes every row as a call argument, so a dense + * chunk (a recovery sync re-indexing many files at once, e.g. the #1541 + * self-heal) exceeded V8's argument limit and killed the whole sync with + * "Maximum call stack size exceeded" after the store phase, leaving every + * re-indexed file's references unresolved. Reproduced for real on a + * cpython-stdlib-sized heal (919 files, 234k refs). The append is now a loop; + * this pins it with a result set well past V8's argument ceiling (~124k). + */ +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import * as fs from 'node:fs'; +import * as path from 'node:path'; +import * as os from 'node:os'; +import { CodeGraph } from '../src'; +import type { UnresolvedReference } from '../src/types'; + +describe('unresolved-ref loads with dense result sets (#1558)', () => { + let dir: string; + let cg: CodeGraph; + + beforeEach(async () => { + dir = fs.mkdtempSync(path.join(os.tmpdir(), 'refs-spread-')); + fs.writeFileSync(path.join(dir, 'anchor.py'), 'def anchor():\n return 1\n'); + cg = await CodeGraph.init(dir); + await cg.indexAll(); + }); + + afterEach(() => { + cg.destroy(); + fs.rmSync(dir, { recursive: true, force: true }); + }); + + it('returns 200k pending refs from few files without exhausting the call stack', () => { + const queries = (cg as unknown as { + queries: { + insertUnresolvedRefsBatch(refs: UnresolvedReference[]): void; + getUnresolvedReferencesByFiles(paths: string[]): UnresolvedReference[]; + }; + }).queries; + + const FILES = 200; + const TOTAL = 200_000; + const paths: string[] = Array.from({ length: FILES }, (_, i) => `src/f${i}.py`); + // unresolved_refs.from_node_id is FK-constrained — anchor on a real node. + const anchorId = cg.getNodesInFile('anchor.py')[0]!.id; + + const batch: UnresolvedReference[] = []; + for (let i = 0; i < TOTAL; i++) { + batch.push({ + fromNodeId: anchorId, + referenceName: `ref_${i}`, + referenceKind: 'call', + line: (i % 1000) + 1, + column: 0, + filePath: paths[i % FILES]!, + language: 'python', + }); + if (batch.length === 20_000) { + queries.insertUnresolvedRefsBatch(batch); + batch.length = 0; + } + } + if (batch.length > 0) queries.insertUnresolvedRefsBatch(batch); + + // All 200 paths fit in ONE SQLite parameter chunk, so a single query + // returns all 200k rows — the exact shape that blew the argument limit. + const rows = queries.getUnresolvedReferencesByFiles(paths); + expect(rows.length).toBe(TOTAL); + }); +}); diff --git a/src/bin/codegraph.ts b/src/bin/codegraph.ts index 067cdd3..acec9c2 100644 --- a/src/bin/codegraph.ts +++ b/src/bin/codegraph.ts @@ -405,6 +405,16 @@ function printIndexResult(clack: typeof import('@clack/prompts'), result: IndexR for (const w of result.errors.filter((e) => e.code === 'index_partial')) { clack.log.warn(w.message); } + // Files salvaged from comment-stripped source after repeated parser + // failures are indexed but possibly incomplete — say so here, or the run + // reads as fully clean and the index quietly disagrees with a later + // re-parse of the same bytes (#1565). + const salvaged = result.errors.filter((e) => e.code === 'salvaged_stripped'); + if (salvaged.length > 0) { + const sample = salvaged.slice(0, 3).map((e) => e.filePath).filter(Boolean).join(', '); + const more = salvaged.length > 3 ? ', ...' : ''; + clack.log.warn(`${formatNumber(salvaged.length)} file(s) indexed from comment-stripped source after repeated parse failures ${getGlyphs().dash} symbols may be incomplete (${sample}${more})`); + } } else if (hasErrors) { clack.log.error(`Indexing failed ${getGlyphs().dash} all ${formatNumber(result.filesErrored)} files had errors`); } else { @@ -443,9 +453,16 @@ function printIndexResult(clack: typeof import('@clack/prompts'), result: IndexR clack.log.info(`The index is fully usable ${getGlyphs().dash} only the failed files are missing.`); } } else if (projectPath) { - const logPath = path.join(getCodeGraphDir(projectPath), 'errors.log'); - if (fs.existsSync(logPath)) { - fs.unlinkSync(logPath); + // No hard errors. Salvaged-file warnings still belong in the log — it + // carries the per-file detail behind the one-line summary above. + if (result.errors.some((e) => e.code === 'salvaged_stripped')) { + writeErrorLog(projectPath, result.errors); + clack.log.info('See .codegraph/errors.log for details'); + } else { + const logPath = path.join(getCodeGraphDir(projectPath), 'errors.log'); + if (fs.existsSync(logPath)) { + fs.unlinkSync(logPath); + } } } } diff --git a/src/db/queries.ts b/src/db/queries.ts index 451c6a9..2b8bc53 100644 --- a/src/db/queries.ts +++ b/src/db/queries.ts @@ -2359,7 +2359,12 @@ export class QueryBuilder { const chunkRows = this.db .prepare(`SELECT * FROM unresolved_refs WHERE status = 'pending' AND file_path IN (${placeholders})`) .all(...chunk) as UnresolvedRefRow[]; - rows.push(...chunkRows); + // Append with a loop, never a spread: the INPUT chunk is bounded, but + // the RESULT rows per chunk are not — a dense recovery sync (e.g. the + // #1541 self-heal re-indexing hundreds of files) returns more rows than + // V8 allows as arguments, and `push(...chunkRows)` dies with "Maximum + // call stack size exceeded", aborting resolution mid-sync (#1558). + for (const row of chunkRows) rows.push(row); } return rows.map((row) => ({ @@ -2541,7 +2546,10 @@ export class QueryBuilder { const chunkRows = this.db .prepare(`SELECT * FROM unresolved_refs WHERE status = 'failed' AND name_tail IN (${placeholders})`) .all(...chunk) as UnresolvedRefRow[]; - rows.push(...chunkRows); + // Loop, not spread — same V8 argument-limit hazard as + // getUnresolvedReferencesByFiles (#1558): a large definition delta can + // select an unbounded number of failed rows per chunk. + for (const row of chunkRows) rows.push(row); } return rows.map((row) => ({ diff --git a/src/extraction/index.ts b/src/extraction/index.ts index 9e915a2..3606d25 100644 --- a/src/extraction/index.ts +++ b/src/extraction/index.ts @@ -2102,6 +2102,7 @@ export class ExtractionOrchestrator { // on, and a silently "clean" file here is how an index quietly // disagrees with a later per-file sync of the same bytes (#1565). errEntry.severity = 'warning'; + errEntry.code = 'salvaged_stripped'; errEntry.message = `Indexed from comment-stripped source after repeated parse failures (symbols may be incomplete until the file is re-indexed): ${errEntry.message}`; filesErrored--; filesIndexed++;