From e871c49a3173a637172f501f21f6a2753ea5a39f Mon Sep 17 00:00:00 2001 From: Colby Mchenry Date: Sun, 12 Jul 2026 20:09:03 -0500 Subject: [PATCH] fix(resolution): clean up processed refs by row id so batch boundaries can't drop sibling call sites (#1269) (#1270) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Post-batch cleanup deleted resolved refs (and parked failed ones) by (from_node_id, reference_name, reference_kind) — no line/col. When one caller had several call sites to the same callee and a batch boundary split them, the first batch's cleanup removed every row with that key, including later-batch siblings that were never attempted — their edges were silently never created. On nlohmann/json this ate 422 real call edges (write_cbor's 38 to_char_type calls indexed as 11). Refs loaded from unresolved_refs now carry their row id through resolution, and all three persist paths (sync resolveAndPersist, the yielding retry pass, the batched drain loop) delete / mark-failed by exactly that id. The key-tuple methods remain only as the fallback for hand-built refs from the public API. Failed-parking gains the same precision: outcome can differ per call site (receiver inference reads the ref's line), so a sibling must not inherit another row's failure. Also untracks the zz-scratch local test files that slipped into #1268 and gitignores the pattern. Validation: red-green regression test (5 sites, batch size 2 — old code kept 2 edges, fix keeps 5); nlohmann/json re-index is a strict superset of the previous edge set (0 lost, 422 recovered, spot-checked against source). Co-authored-by: Claude Fable 5 --- .gitignore | 3 + CHANGELOG.md | 1 + __tests__/batched-ref-cleanup.test.ts | 121 +++++++++++++++++++++++ __tests__/zz-scratch.test.ts | 43 --------- __tests__/zz-scratch2.test.ts | 41 -------- __tests__/zz-scratch3.test.ts | 14 --- src/db/queries.ts | 43 +++++++++ src/resolution/index.ts | 133 +++++++++++++++++--------- src/resolution/name-matcher.ts | 9 +- src/resolution/types.ts | 3 + src/types.ts | 11 +++ 11 files changed, 277 insertions(+), 145 deletions(-) create mode 100644 __tests__/batched-ref-cleanup.test.ts delete mode 100644 __tests__/zz-scratch.test.ts delete mode 100644 __tests__/zz-scratch2.test.ts delete mode 100644 __tests__/zz-scratch3.test.ts diff --git a/.gitignore b/.gitignore index 39f990e..4efcff6 100644 --- a/.gitignore +++ b/.gitignore @@ -70,3 +70,6 @@ assets/__pycache__/ assets/generate-waitlist.py .kommandr/ + +# Local scratch tests (never commit) +__tests__/zz-scratch* diff --git a/CHANGELOG.md b/CHANGELOG.md index a0551c7..22fb4c5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ### Fixes +- Callers and impact analysis no longer silently under-count a function that calls the same callee many times. When one caller contained several call sites to the same callee and an internal resolution batch boundary happened to split them, cleanup after the first batch removed the later sites' pending rows before they were ever attempted — their edges were never created, deterministically, and which edges went missing shifted with unrelated changes to the project's total reference count. Post-pass cleanup now targets the exact database row each processed reference came from. Found while validating the operator-call fix on nlohmann/json, where `write_cbor`'s 11 calls to `to_char_type` indexed as 10. (#1269) - C++ explicit operator calls — `a.operator+(b)`, `p->operator+(b)`, `a.operator[](3)`, and the other symbolic forms — now produce a `calls` edge to the operator method, so an operator invoked only through the explicit syntax no longer looks uncalled in callers and impact analysis. tree-sitter parses these call sites with the operator name stranded in an error node (never as a normal member access), so the call's target was silently read as just the receiver variable; the operator name is now recovered from the error node and resolved through receiver-type inference like any other member call — a same-named operator on an unrelated class can never capture the edge. Infix uses (`a + b`, `a[i]`) need real type inference and are tracked separately. (#1247) ## [1.4.1] - 2026-07-10 diff --git a/__tests__/batched-ref-cleanup.test.ts b/__tests__/batched-ref-cleanup.test.ts new file mode 100644 index 0000000..f831826 --- /dev/null +++ b/__tests__/batched-ref-cleanup.test.ts @@ -0,0 +1,121 @@ +/** + * Batched resolution cleanup precision (#1269) + * + * Post-batch cleanup used to delete resolved refs (and park failed ones) by + * (from_node_id, reference_name, reference_kind) — no line/col. When one + * caller contains several call sites to the SAME callee and a batch boundary + * splits them, resolving the first batch's sites deleted every row with that + * key, including sibling rows in later batches that were never attempted — + * their edges were silently never created. Observed on nlohmann/json: + * `write_cbor` calls `to_char_type` at 11 lines; the batch boundary + * deterministically dropped the last site's edge. + * + * Cleanup now targets the exact `unresolved_refs.id` for DB-loaded refs, with + * the key-tuple delete kept only as the fallback for hand-built refs (public + * resolveAndPersist API) that carry no row id. + */ + +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import * as fs from 'fs'; +import * as path from 'path'; +import * as os from 'os'; +import { DatabaseConnection } from '../src/db'; +import { QueryBuilder } from '../src/db/queries'; +import { createResolver } from '../src/resolution'; +import { Node, UnresolvedReference } from '../src/types'; + +function makeNode(id: string, name: string, kind: Node['kind'], filePath: string, startLine: number): Node { + return { + id, + kind, + name, + qualifiedName: name, + filePath, + language: 'typescript', + startLine, + endLine: startLine + 2, + startColumn: 0, + endColumn: 0, + updatedAt: Date.now(), + }; +} + +function makeRef(fromNodeId: string, name: string, line: number): UnresolvedReference { + return { + fromNodeId, + referenceName: name, + referenceKind: 'calls', + line, + column: 2, + filePath: 'caller.ts', + language: 'typescript', + }; +} + +describe('Batched ref cleanup precision (#1269)', () => { + let dir: string; + let db: DatabaseConnection; + let q: QueryBuilder; + + beforeEach(() => { + dir = fs.mkdtempSync(path.join(os.tmpdir(), 'codegraph-refcleanup-')); + db = DatabaseConnection.initialize(path.join(dir, 'test.db')); + q = new QueryBuilder(db.getDb()); + // The files the refs/nodes point at must exist for resolution context. + fs.writeFileSync(path.join(dir, 'caller.ts'), 'callee();\ncallee();\ncallee();\ncallee();\ncallee();\n'); + fs.writeFileSync(path.join(dir, 'callee.ts'), 'export function callee() {}\n'); + q.insertNode(makeNode('fn:caller', 'caller', 'function', 'caller.ts', 1)); + q.insertNode(makeNode('fn:callee', 'callee', 'function', 'callee.ts', 1)); + }); + + afterEach(() => { + db.close(); + fs.rmSync(dir, { recursive: true, force: true }); + }); + + it('creates an edge for EVERY same-named call site when the sites straddle a batch boundary', async () => { + // 5 call sites, batch size 2 → boundaries after sites 2 and 4. With the + // key-tuple delete, batch 1's cleanup removed ALL five rows and only 2 + // edges ever existed. + const lines = [1, 2, 3, 4, 5]; + q.insertUnresolvedRefsBatch(lines.map((line) => makeRef('fn:caller', 'callee', line))); + expect(q.getUnresolvedReferencesCount()).toBe(5); + + const resolver = createResolver(dir, q); + await resolver.resolveAndPersistBatched(undefined, 2); + + const edges = q + .getOutgoingEdges('fn:caller') + .filter((e) => e.kind === 'calls' && e.target === 'fn:callee'); + expect(edges.map((e) => e.line).sort()).toEqual(lines); + // Every processed row left the pending set (drain terminated normally). + expect(q.getUnresolvedReferencesCount()).toBe(0); + }); + + it('parks EVERY unresolvable same-named site as failed only after its own attempt', async () => { + // 4 sites calling a name with no definition, batch size 2. Both halves + // must drain to status='failed' (previously batch 1's key-tuple update + // also flipped batch 2's rows before they were attempted — same outcome + // here, but the loop must still terminate and leave nothing pending). + q.insertUnresolvedRefsBatch([1, 2, 3, 4].map((line) => makeRef('fn:caller', 'missingCallee', line))); + + const resolver = createResolver(dir, q); + await resolver.resolveAndPersistBatched(undefined, 2); + + expect(q.getUnresolvedReferencesCount()).toBe(0); // nothing pending + const failed = q.getUnresolvedReferences().filter((r) => r.referenceName === 'missingCallee'); + expect(failed).toHaveLength(4); // all parked, none deleted + }); + + it('hand-built refs without a rowId still clean up through the key fallback', () => { + // Public resolveAndPersist API: refs built in memory (no rowId) that also + // exist as DB rows — the legacy key-tuple delete must still clear them. + q.insertUnresolvedRefsBatch([makeRef('fn:caller', 'callee', 1)]); + const resolver = createResolver(dir, q); + const inMemory = makeRef('fn:caller', 'callee', 1); // no rowId + const result = resolver.resolveAndPersist([inMemory]); + + expect(result.resolved).toHaveLength(1); + expect(q.getUnresolvedReferencesCount()).toBe(0); + }); +}); diff --git a/__tests__/zz-scratch.test.ts b/__tests__/zz-scratch.test.ts deleted file mode 100644 index 2ec9686..0000000 --- a/__tests__/zz-scratch.test.ts +++ /dev/null @@ -1,43 +0,0 @@ -import { describe, it, beforeAll } from 'vitest'; -import { extractFromSource } from '../src/extraction'; -import { initGrammars, loadAllGrammars, getParser } from '../src/extraction/grammars'; - -beforeAll(async () => { - await initGrammars(); - await loadAllGrammars(); -}); - -const CODE = `struct V { - int x; - V operator+(const V& o) const { return V{x + o.x}; } - V operator[](int i) const { return V{x + i}; } - int get() const { return x; } -}; - -int plainCaller(const V& a) { return a.get(); } -V explicitCaller(const V& a, const V& b) { return a.operator+(b); } -`; - -describe('scratch', () => { - it('dumps', async () => { - const parser: any = getParser('cpp' as any); - const tree = parser.parse(CODE); - const dump = (n: any, d = 0) => { - let out = `${' '.repeat(d)}${n.type} [${JSON.stringify(n.text.slice(0, 40))}]\n`; - for (let i = 0; i < n.childCount; i++) { - const c = n.child(i); - const f = n.fieldNameForChild ? n.fieldNameForChild(i) : null; - out += `${' '.repeat(d + 1)}${f ? f + ': ' : ''}`.trimEnd() ? '' : ''; - out += dump(c, d + 1); - } - return out; - }; - // just dump the explicitCaller subtree - console.log(dump(tree.rootNode)); - - const result = extractFromSource('optest.cpp', CODE); - console.log('NODES', result.nodes.map((n: any) => `${n.kind}:${n.name}`)); - console.log('KEYS', Object.keys(result)); - console.log('UREFS', JSON.stringify((result as any).unresolvedReferences, null, 1)); - }); -}); diff --git a/__tests__/zz-scratch2.test.ts b/__tests__/zz-scratch2.test.ts deleted file mode 100644 index fbd4fee..0000000 --- a/__tests__/zz-scratch2.test.ts +++ /dev/null @@ -1,41 +0,0 @@ -import { describe, it, beforeAll } from 'vitest'; -import { extractFromSource } from '../src/extraction'; -import { initGrammars, loadAllGrammars, getParser } from '../src/extraction/grammars'; - -beforeAll(async () => { await initGrammars(); await loadAllGrammars(); }); - -const CASES: Record = { - dot_plus: 'V f(V a, V b) { return a.operator+(b); }', - arrow_plus: 'V f(V* a, V b) { return a->operator+(b); }', - dot_sub: 'V f(V a) { return a.operator[](3); }', - dot_call: 'V f(V a) { return a.operator()(3); }', - dot_eq: 'bool f(V a, V b) { return a.operator==(b); }', - dot_bool: 'bool f(V a) { return a.operator bool(); }', - qualified: 'V f(V a, V b) { return V::operator+(a, b); }', - free_op: 'V f(V a, V b) { return operator+(a, b); }', - this_op: 'struct V { V g(V b) { return this->operator+(b); } };', - member_op: 'struct V { V x; V g(V b) { return x.operator+(b); } };', - arrow_deref: 'V f(V a) { return a.operator->(); }', - dot_notop: 'bool f(V a) { return a.operator!(); }', -}; - -describe('dump', () => { - it('all', () => { - const p: any = getParser('cpp' as any); - for (const [k, code] of Object.entries(CASES)) { - const tree = p.parse(code); - const dump = (n: any, d = 0): string => { - let out = `${' '.repeat(d)}${n.type}${n.childCount === 0 ? ' ' + JSON.stringify(n.text) : ''}\n`; - for (let i = 0; i < n.childCount; i++) out += dump(n.child(i), d + 1); - return out; - }; - const call = (function find(n: any): any { - if (n.type === 'call_expression') return n; - for (let i = 0; i < n.childCount; i++) { const r = find(n.child(i)); if (r) return r; } - return null; - })(tree.rootNode); - const refs = extractFromSource('t.cpp', code).unresolvedReferences.filter((r: any) => r.referenceKind === 'calls'); - console.log(`\n=== ${k}: ${code}\n${call ? dump(call) : '(no call_expression)'}refs: ${JSON.stringify(refs.map((r: any) => r.referenceName))} hasError=${tree.rootNode.hasError}`); - } - }); -}); diff --git a/__tests__/zz-scratch3.test.ts b/__tests__/zz-scratch3.test.ts deleted file mode 100644 index 3ac0785..0000000 --- a/__tests__/zz-scratch3.test.ts +++ /dev/null @@ -1,14 +0,0 @@ -import { describe, it, beforeAll } from 'vitest'; -import { extractFromSource } from '../src/extraction'; -import { initGrammars, loadAllGrammars } from '../src/extraction/grammars'; -beforeAll(async () => { await initGrammars(); await loadAllGrammars(); }); -describe('d', () => { it('n', () => { - const code = `struct V { - int x; - operator bool() const { return x != 0; } - V operator+(const V& o) const { return V{x+o.x}; } - V& operator=(const V& o) { x = o.x; return *this; } -};`; - const r = extractFromSource('t.cpp', code); - console.log('NODES', r.nodes.map((n: any) => `${n.kind}:${JSON.stringify(n.name)}`)); -}); }); diff --git a/src/db/queries.ts b/src/db/queries.ts index 522df29..15f2611 100644 --- a/src/db/queries.ts +++ b/src/db/queries.ts @@ -1827,6 +1827,7 @@ export class QueryBuilder { candidates: row.candidates ? safeJsonParse(row.candidates, undefined) : undefined, filePath: row.file_path, language: row.language as Language, + rowId: row.id, })); } @@ -1844,6 +1845,7 @@ export class QueryBuilder { candidates: row.candidates ? safeJsonParse(row.candidates, undefined) : undefined, filePath: row.file_path, language: row.language as Language, + rowId: row.id, })); } @@ -1886,6 +1888,7 @@ export class QueryBuilder { candidates: row.candidates ? safeJsonParse(row.candidates, undefined) : undefined, filePath: row.file_path, language: row.language as Language, + rowId: row.id, })); } @@ -1954,6 +1957,7 @@ export class QueryBuilder { candidates: row.candidates ? safeJsonParse(row.candidates, undefined) : undefined, filePath: row.file_path, language: row.language as Language, + rowId: row.id, })); } @@ -1998,6 +2002,23 @@ export class QueryBuilder { deleteMany(refs); } + /** + * Delete unresolved-ref rows by row id — the precise cleanup for refs a + * resolution pass actually processed. The key-tuple variant above also + * deletes SIBLING rows (same caller calling the same callee at other lines) + * that a later batch hasn't attempted yet, so when a batch boundary split a + * caller's same-named call sites, the later sites' edges were silently never + * created (#1269). + */ + deleteReferencesByRowIds(rowIds: number[]): void { + if (rowIds.length === 0) return; + for (let i = 0; i < rowIds.length; i += SQLITE_PARAM_CHUNK_SIZE) { + const chunk = rowIds.slice(i, i + SQLITE_PARAM_CHUNK_SIZE); + const placeholders = chunk.map(() => '?').join(','); + this.db.prepare(`DELETE FROM unresolved_refs WHERE id IN (${placeholders})`).run(...chunk); + } + } + /** * Mark refs a completed resolution pass could not resolve as status='failed' * instead of deleting them (#1240). Failed rows are invisible to the pending @@ -2020,6 +2041,27 @@ export class QueryBuilder { markMany(refs); } + /** + * Park refs as status='failed' by row id — the precise counterpart of + * markReferencesFailed, for the same reason as deleteReferencesByRowIds: + * the key-tuple variant also flips same-key sibling rows in later batches + * to 'failed' before they were ever attempted (#1269). Resolution outcome + * can differ per call site (receiver-type inference reads the ref's line), + * so a sibling must not inherit this row's failure. + */ + markReferencesFailedByRowIds(refs: Array<{ rowId: number; referenceName: string }>): void { + if (refs.length === 0) return; + const stmt = this.db.prepare( + "UPDATE unresolved_refs SET status = 'failed', name_tail = ? WHERE id = ?" + ); + const markMany = this.db.transaction((items: typeof refs) => { + for (const ref of items) { + stmt.run(referenceNameTail(ref.referenceName), ref.rowId); + } + }); + markMany(refs); + } + /** * Failed refs whose name tail matches one of the given symbol names — the * candidates a sync should retry after files carrying those names changed @@ -2068,6 +2110,7 @@ export class QueryBuilder { candidates: row.candidates ? safeJsonParse(row.candidates, undefined) : undefined, filePath: row.file_path, language: row.language as Language, + rowId: row.id, })); } diff --git a/src/resolution/index.ts b/src/resolution/index.ts index 9f94c72..e956794 100644 --- a/src/resolution/index.ts +++ b/src/resolution/index.ts @@ -625,6 +625,7 @@ export class ReferenceResolver { column: ref.column, filePath: ref.filePath || this.getFilePathFromNodeId(ref.fromNodeId), language: ref.language || this.getLanguageFromNodeId(ref.fromNodeId), + rowId: ref.rowId, })); const total = refs.length; @@ -1008,6 +1009,56 @@ export class ReferenceResolver { }); } + /** + * Split resolved refs into rows deletable by id and hand-built refs that + * must fall back to the key-tuple delete. Rows loaded from the database + * carry their row id and are deleted by exactly that id; the key tuple + * omits line/col, so it also removes SIBLING rows — the same caller calling + * the same callee at other lines — that a later batch hadn't attempted yet: + * when a batch boundary split a caller's same-named call sites, the later + * sites' edges were silently never created (#1269). + */ + private static partitionResolvedCleanup(resolved: ResolvedRef[]): { + rowIds: number[]; + legacyKeys: Array<{ fromNodeId: string; referenceName: string; referenceKind: string }>; + } { + const rowIds: number[] = []; + const legacyKeys: Array<{ fromNodeId: string; referenceName: string; referenceKind: string }> = []; + for (const r of resolved) { + if (r.original.rowId != null) rowIds.push(r.original.rowId); + else legacyKeys.push({ + fromNodeId: r.original.fromNodeId, + referenceName: r.original.referenceName, + referenceKind: r.original.referenceKind, + }); + } + return { rowIds, legacyKeys }; + } + + /** + * Same row-id precision for parking unresolvable refs as status='failed' + * (#1240): the key-tuple fallback would flip same-key sibling rows in later + * batches to 'failed' before they were ever attempted, and resolution + * outcome can differ per call site (receiver-type inference reads the + * ref's line), so a sibling must not inherit this row's failure (#1269). + */ + private static partitionFailedCleanup(unresolved: UnresolvedRef[]): { + byRowId: Array<{ rowId: number; referenceName: string }>; + legacyKeys: Array<{ fromNodeId: string; referenceName: string; referenceKind: string }>; + } { + const byRowId: Array<{ rowId: number; referenceName: string }> = []; + const legacyKeys: Array<{ fromNodeId: string; referenceName: string; referenceKind: string }> = []; + for (const r of unresolved) { + if (r.rowId != null) byRowId.push({ rowId: r.rowId, referenceName: r.referenceName }); + else legacyKeys.push({ + fromNodeId: r.fromNodeId, + referenceName: r.referenceName, + referenceKind: r.referenceKind, + }); + } + return { byRowId, legacyKeys }; + } + /** * Resolve and persist edges to database */ @@ -1027,13 +1078,9 @@ export class ReferenceResolver { // Clean up resolved refs from unresolved_refs table so metrics are accurate if (result.resolved.length > 0) { - this.queries.deleteSpecificResolvedReferences( - result.resolved.map((r) => ({ - fromNodeId: r.original.fromNodeId, - referenceName: r.original.referenceName, - referenceKind: r.original.referenceKind, - })) - ); + const { rowIds, legacyKeys } = ReferenceResolver.partitionResolvedCleanup(result.resolved); + this.queries.deleteReferencesByRowIds(rowIds); + this.queries.deleteSpecificResolvedReferences(legacyKeys); } // Park unresolvable refs as status='failed' — parity with @@ -1046,13 +1093,9 @@ export class ReferenceResolver { // is still 'pending', so any pending row at rest belongs to an // interrupted run and the sweep can key off the pending count. if (result.unresolved.length > 0) { - this.queries.markReferencesFailed( - result.unresolved.map((r) => ({ - fromNodeId: r.fromNodeId, - referenceName: r.referenceName, - referenceKind: r.referenceKind, - })) - ); + const { byRowId, legacyKeys } = ReferenceResolver.partitionFailedCleanup(result.unresolved); + this.queries.markReferencesFailedByRowIds(byRowId); + this.queries.markReferencesFailed(legacyKeys); } return result; @@ -1078,23 +1121,23 @@ export class ReferenceResolver { await maybeYield(); } - const resolvedKeys = result.resolved.map((r) => ({ - fromNodeId: r.original.fromNodeId, - referenceName: r.original.referenceName, - referenceKind: r.original.referenceKind, - })); - for (let i = 0; i < resolvedKeys.length; i += PERSIST_CHUNK) { - this.queries.deleteSpecificResolvedReferences(resolvedKeys.slice(i, i + PERSIST_CHUNK)); + const resolvedCleanup = ReferenceResolver.partitionResolvedCleanup(result.resolved); + for (let i = 0; i < resolvedCleanup.rowIds.length; i += PERSIST_CHUNK) { + this.queries.deleteReferencesByRowIds(resolvedCleanup.rowIds.slice(i, i + PERSIST_CHUNK)); + await maybeYield(); + } + for (let i = 0; i < resolvedCleanup.legacyKeys.length; i += PERSIST_CHUNK) { + this.queries.deleteSpecificResolvedReferences(resolvedCleanup.legacyKeys.slice(i, i + PERSIST_CHUNK)); await maybeYield(); } - const unresolvedKeys = result.unresolved.map((r) => ({ - fromNodeId: r.fromNodeId, - referenceName: r.referenceName, - referenceKind: r.referenceKind, - })); - for (let i = 0; i < unresolvedKeys.length; i += PERSIST_CHUNK) { - this.queries.markReferencesFailed(unresolvedKeys.slice(i, i + PERSIST_CHUNK)); + const failedCleanup = ReferenceResolver.partitionFailedCleanup(result.unresolved); + for (let i = 0; i < failedCleanup.byRowId.length; i += PERSIST_CHUNK) { + this.queries.markReferencesFailedByRowIds(failedCleanup.byRowId.slice(i, i + PERSIST_CHUNK)); + await maybeYield(); + } + for (let i = 0; i < failedCleanup.legacyKeys.length; i += PERSIST_CHUNK) { + this.queries.markReferencesFailed(failedCleanup.legacyKeys.slice(i, i + PERSIST_CHUNK)); await maybeYield(); } @@ -1184,6 +1227,7 @@ export class ReferenceResolver { column: raw.column, filePath: raw.filePath || this.getFilePathFromNodeId(raw.fromNodeId), language: raw.language || this.getLanguageFromNodeId(raw.fromNodeId), + rowId: raw.rowId, }; const result = this.resolveOne(ref); if (result) { @@ -1262,14 +1306,17 @@ export class ReferenceResolver { await maybeYield(); } - // Clean up resolved refs so they don't appear in the next batch - const resolvedKeys = result.resolved.map((r) => ({ - fromNodeId: r.original.fromNodeId, - referenceName: r.original.referenceName, - referenceKind: r.original.referenceKind, - })); - for (let i = 0; i < resolvedKeys.length; i += PERSIST_CHUNK) { - this.queries.deleteSpecificResolvedReferences(resolvedKeys.slice(i, i + PERSIST_CHUNK)); + // Clean up resolved refs so they don't appear in the next batch — + // by row id, so a same-key sibling ref in a LATER batch (same caller + // calling the same callee at another line) is left pending for its own + // attempt instead of being swept out with this batch's rows (#1269). + const resolvedCleanup = ReferenceResolver.partitionResolvedCleanup(result.resolved); + for (let i = 0; i < resolvedCleanup.rowIds.length; i += PERSIST_CHUNK) { + this.queries.deleteReferencesByRowIds(resolvedCleanup.rowIds.slice(i, i + PERSIST_CHUNK)); + await maybeYield(); + } + for (let i = 0; i < resolvedCleanup.legacyKeys.length; i += PERSIST_CHUNK) { + this.queries.deleteSpecificResolvedReferences(resolvedCleanup.legacyKeys.slice(i, i + PERSIST_CHUNK)); await maybeYield(); } @@ -1277,13 +1324,13 @@ export class ReferenceResolver { // leave the pending set (the batch reader and non-progress guard below // only see pending rows) but stay retryable when a later sync adds a // symbol that could satisfy them (#1240). - const unresolvedKeys = result.unresolved.map((r) => ({ - fromNodeId: r.fromNodeId, - referenceName: r.referenceName, - referenceKind: r.referenceKind, - })); - for (let i = 0; i < unresolvedKeys.length; i += PERSIST_CHUNK) { - this.queries.markReferencesFailed(unresolvedKeys.slice(i, i + PERSIST_CHUNK)); + const failedCleanup = ReferenceResolver.partitionFailedCleanup(result.unresolved); + for (let i = 0; i < failedCleanup.byRowId.length; i += PERSIST_CHUNK) { + this.queries.markReferencesFailedByRowIds(failedCleanup.byRowId.slice(i, i + PERSIST_CHUNK)); + await maybeYield(); + } + for (let i = 0; i < failedCleanup.legacyKeys.length; i += PERSIST_CHUNK) { + this.queries.markReferencesFailed(failedCleanup.legacyKeys.slice(i, i + PERSIST_CHUNK)); await maybeYield(); } diff --git a/src/resolution/name-matcher.ts b/src/resolution/name-matcher.ts index 91ddd1a..9e77d63 100644 --- a/src/resolution/name-matcher.ts +++ b/src/resolution/name-matcher.ts @@ -918,10 +918,11 @@ export function matchDottedCallChain( // CRITICAL: resolve the TARGET via a synthetic bare-name ref, but return the // match tied to the ORIGINAL `ref` (referenceName `inner().method`). The // batched resolver (resolveAndPersistBatched) reads unresolved rows from - // offset 0 every pass and relies on deleteSpecificResolvedReferences — - // keyed on referenceName — to clear each resolved row so the batch empties. - // If we propagated the synthetic ref's bare `method` as `.original`, the - // delete would never match the stored `inner().method` row, the batch would + // offset 0 every pass and relies on the post-batch cleanup (row-id delete + // for DB-loaded refs, referenceName-keyed delete otherwise, #1269) to + // clear each resolved row so the batch empties. If we propagated the + // synthetic ref's bare `method` as `.original`, a key-based delete + // would never match the stored `inner().method` row, the batch would // never drain, and the loop would re-resolve + re-insert forever (a runaway // that grew gin's graph to 5M edges / 1.4 GB before this fix). const bareRef = { ...ref, referenceName: method }; diff --git a/src/resolution/types.ts b/src/resolution/types.ts index c853916..bc80e2f 100644 --- a/src/resolution/types.ts +++ b/src/resolution/types.ts @@ -26,6 +26,9 @@ export interface UnresolvedRef { language: Language; /** Possible qualified names it might resolve to */ candidates?: string[]; + /** `unresolved_refs.id` when loaded from the database — post-pass cleanup + * targets exactly this row instead of every same-key sibling (#1269). */ + rowId?: number; } /** diff --git a/src/types.ts b/src/types.ts index d160d04..6f3de63 100644 --- a/src/types.ts +++ b/src/types.ts @@ -323,6 +323,17 @@ export interface UnresolvedReference { /** Possible qualified names it might resolve to */ candidates?: string[]; + + /** + * `unresolved_refs.id` when this ref was loaded from the database. Post-pass + * cleanup (delete-on-resolve / park-as-failed) targets exactly this row. + * Without it, cleanup falls back to deleting by (fromNodeId, referenceName, + * referenceKind) — which also removes SIBLING rows (same caller calling the + * same callee at other lines) that a later batch hasn't attempted yet, so + * their edges were silently never created when a batch boundary split the + * call sites (#1269). + */ + rowId?: number; } // =============================================================================