fix(sync): a call left with one candidate links after a delete or a rename

x.go() with go in two classes stays unlinked while both exist. A sync that
deleted one of them, or renamed it away, never retried that call: the
failed-reference retry looks refs up by the names the changed files carry
now, which a deleted file or a renamed-away name no longer has, and a sync
that only deletes files skips the retry. Failed refs under the names a
sync's definitionDelta lost or gained now go back to pending for the
sweep, next to the CG-33 rebind that does the same for resolved edges,
under the same per-name ceiling. CODEGRAPH_NO_REBIND=1 turns both off.

Review changes: the CHANGELOG entry beside the sync entries for calls,
with credit.

Validation: 6 tests; on main's source the 4 delete and rename tests fail
and the 2 still-ambiguous controls pass. 10 fixture edits (Python,
TypeScript, JavaScript, Go, Java), each compared with a fresh index of the
same tree, in a later process and in the same process: main diverged in 6
of each, this in none. excalidraw with excalidraw-app/data/LocalData.ts
deleted (its `save` was one of two): main was 38 edges short of a fresh
index, identical here over nodes, edges, refs and files; renaming
BinaryHeap's `pop` away re-opens 28 refs that fail again, as in a fresh
index.

Original PR #3004 by @inth3shadows.
Fixes #3006

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Eric Minish
2026-10-30 12:00:19 +00:00
committed by Colby Mchenry
co-authored by Claude Opus 5.5
parent e431491972
commit 2d98c1b69a
4 changed files with 158 additions and 3 deletions
+1
View File
@@ -182,6 +182,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
- `codegraph sync` and the file watcher now link a file to a module it imports under a name of its own when the module is added after the file was indexed, comes back after being deleted, or gets the export in a later edit: a default import like `import tagsController from './tag/tag.controller'`, a namespace import like `import * as api from './api'`, or an aliased one like `import { Component as GroupsPageWrapper } from './groups_page'`, in JavaScript, TypeScript, Vue, Svelte and Astro. Before, the import and every use of the name, such as a call, `new`, a base class, a JSX or template tag or an Express `app.use` mount, stayed unlinked until the importing file changed or the project was re-indexed, so the module looked unused and `codegraph affected` missed the files that use it. Re-index after upgrading to pick up links an earlier version missed.
- `codegraph sync` and the file watcher now also link a call or read on a name a file imports, like `NODE_PATH_EXCLUDELIST.includes(path)` after `import { NODE_PATH_EXCLUDELIST } from './constants'`, `store.get(key)` through a default import or `api.get()` through a namespace import, when the module gets the export in a later edit, is added after the importing file was indexed, or comes back after being deleted, in JavaScript, TypeScript, Vue, Svelte and Astro. Before, such a call stayed unlinked until the importing file changed or the project was re-indexed, because sync looked it up only by its method's name, which the module seldom declares, and skipped a name as common as `includes`, `get` or `map` altogether. Re-index after upgrading to pick up links an earlier version missed.
- `codegraph sync` and the file watcher now also link a call on a variable or parameter declared with a class a file imports, like `cache.get(key)` after `const cache = new LRUCache(10)` or in a function that takes `cache: LRUCache<string, number>`, when the class's module gets the class in a later edit, is added after the file was indexed, or comes back after being deleted, in JavaScript and TypeScript. Before, such a call stayed unlinked until its own file changed or the project was re-indexed whenever its method's name, like `get`, was too common for sync to look up. Re-index after upgrading to pick up links an earlier version missed.
- `codegraph sync` and the file watcher now link a call that had two candidates once a sync deletes the file holding one of them or renames it away, like `x.go()` with `go` defined in two classes, as a full index does. Before, the call stayed unlinked until its own file changed or the project was re-indexed. Thanks @inth3shadows for the report and the pull request. (#3006)
- `codegraph sync` and the file watcher now also link a name a file imports from a barrel module, like `import { useInvocation } from './context'` where `context/index.ts` re-exports it from the file that declares it, when an edit gives the barrel that re-export after the importing file was indexed, in JavaScript, TypeScript, Vue, Svelte and Astro. Before, the import and every use of the name, such as a call or a JSX or template tag, stayed unlinked until the importing file changed or the project was re-indexed, because a barrel declares none of the names it passes on. Re-index after upgrading to pick up links an earlier version missed.
- `codegraph sync` and the file watcher now also update the files that import through a barrel module when an edit points one of its re-exports at another module, like `export { foo } from './a'` changed to `'./b'` or an `export *` moved the same way, takes one away, or drops a name from it, in JavaScript and TypeScript. Before, those files kept the names linked to the old module until they changed or the project was re-indexed. Thanks @inth3shadows for the report and @danusha2345. (#2978)
- In TypeScript and JavaScript, a call of an array method, like `items.find(…)` or `items.push(…)`, on a variable or parameter annotated as an array of a class (`items: Item[]`) no longer links to the class's method of the same name. Before, the annotation was read as the class itself: in vscode, `distinctElements.find(…)` on an `ExplorerItem[]` linked to `ExplorerItem.find`, and `ranges.map(…)` on an `OffsetRange[]` to `OffsetRange.map`, so those methods showed callers they don't have. A call on an element that shares the array's name, as in `d.forEach(d => d.dispose())`, still links to the class. Re-index TypeScript and JavaScript projects after upgrading.
+140
View File
@@ -0,0 +1,140 @@
/**
* A call that only failed because a second candidate shared its name resolves
* once a sync removes that candidate.
*
* `x.go()` on a value of unknown type, with `go` defined in two classes, is
* ambiguous: a full index leaves it unresolved. Take one of the two away and a
* full index links the call to the other. Sync didn't: its failed-reference
* retry looked failed refs up by the names in the files it changed, so a
* deleted file (no names left) or an edit that renames the competitor away
* never brought the call up again, and a sync that only deleted files skipped
* the retry altogether.
*/
import { describe, it, expect, beforeEach, afterEach } from 'vitest';
import * as fs from 'fs';
import * as path from 'path';
import * as os from 'os';
import CodeGraph from '../src/index';
/** Every node and edge by natural key, so two indexes of the same files compare directly. */
function graphOf(cg: CodeGraph): { nodes: string[]; edges: string[] } {
const keys = new Map(
cg.getFiles()
.flatMap((file) => cg.getNodesInFile(file.path))
.map((n) => [n.id, `${n.kind} ${n.filePath}:${n.qualifiedName}:${n.startLine}`] as const)
);
const edges = cg.getOutgoingEdgesFrom([...keys.keys()])
.map((e) => `${e.kind} ${keys.get(e.source)} -> ${keys.get(e.target) ?? e.target}`);
return { nodes: [...keys.values()].sort(), edges: edges.sort() };
}
async function reindexFromScratch(cg: CodeGraph): Promise<CodeGraph> {
const root = cg.getProjectRoot();
cg.close();
const rebuilt = await CodeGraph.recreate(root);
await rebuilt.indexAll();
return rebuilt;
}
const REINDEX_TIMEOUT = 60_000;
describe('sync re-resolves a call whose competing candidate went away', () => {
let dir: string;
let cg: CodeGraph;
const write = (rel: string, content: string) => fs.writeFileSync(path.join(dir, rel), content);
function callTargets(caller: string): string[] {
const node = cg.searchNodes(caller).map((r) => r.node).find((n) => n.name === caller && n.kind === 'function');
return node ? cg.getCallees(node.id).map((c) => c.node.qualifiedName).sort() : [];
}
afterEach(() => {
cg?.destroy();
fs.rmSync(dir, { recursive: true, force: true });
});
describe('Python', () => {
beforeEach(async () => {
dir = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-competitor-py-'));
write('a.py', 'class A:\n def go(self):\n return 1\n');
write('b.py', 'class B:\n def go(self):\n return 2\n');
write('t.py', 'from a import A\n\ndef make():\n return A()\n\ndef use():\n x = make()\n x.go()\n');
cg = CodeGraph.initSync(dir);
await cg.indexAll();
// Two candidates: the call stays unresolved.
expect(callTargets('use')).toEqual(['make']);
});
it('after the file holding the other candidate is deleted', async () => {
fs.unlinkSync(path.join(dir, 'b.py'));
const result = await cg.sync();
expect(result.filesRemoved).toBe(1);
expect(callTargets('use')).toEqual(['A::go', 'make']);
const synced = graphOf(cg);
cg = await reindexFromScratch(cg);
expect(synced).toEqual(graphOf(cg));
}, REINDEX_TIMEOUT);
it('after an edit renames the other candidate away', async () => {
write('b.py', 'class B:\n def stop(self):\n return 2\n');
await cg.sync();
expect(callTargets('use')).toEqual(['A::go', 'make']);
const synced = graphOf(cg);
cg = await reindexFromScratch(cg);
expect(synced).toEqual(graphOf(cg));
}, REINDEX_TIMEOUT);
it('a deletion that leaves the call ambiguous changes nothing', async () => {
write('c.py', 'class C:\n def go(self):\n return 3\n');
await cg.sync();
fs.unlinkSync(path.join(dir, 'b.py'));
await cg.sync();
expect(callTargets('use')).toEqual(['make']);
const synced = graphOf(cg);
cg = await reindexFromScratch(cg);
expect(synced).toEqual(graphOf(cg));
}, REINDEX_TIMEOUT);
});
describe('TypeScript', () => {
beforeEach(async () => {
dir = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-competitor-ts-'));
write('a.ts', 'export class A {\n go(): number { return 1; }\n}\n');
write('b.ts', 'export class B {\n go(): number { return 2; }\n}\n');
write('t.ts', 'export function use(x: any): void {\n x.go();\n}\n');
cg = CodeGraph.initSync(dir);
await cg.indexAll();
expect(callTargets('use')).toEqual([]);
});
it('after the file holding the other candidate is deleted', async () => {
fs.unlinkSync(path.join(dir, 'b.ts'));
await cg.sync();
expect(callTargets('use')).toEqual(['A::go']);
const synced = graphOf(cg);
cg = await reindexFromScratch(cg);
expect(synced).toEqual(graphOf(cg));
}, REINDEX_TIMEOUT);
it('after an edit renames the other candidate away', async () => {
write('b.ts', 'export class B {\n stop(): number { return 2; }\n}\n');
await cg.sync();
expect(callTargets('use')).toEqual(['A::go']);
const synced = graphOf(cg);
cg = await reindexFromScratch(cg);
expect(synced).toEqual(graphOf(cg));
}, REINDEX_TIMEOUT);
it('a deletion that leaves the call ambiguous changes nothing', async () => {
write('c.ts', 'export class C {\n go(): number { return 3; }\n}\n');
await cg.sync();
fs.unlinkSync(path.join(dir, 'b.ts'));
await cg.sync();
expect(callTargets('use')).toEqual([]);
const synced = graphOf(cg);
cg = await reindexFromScratch(cg);
expect(synced).toEqual(graphOf(cg));
}, REINDEX_TIMEOUT);
});
});
+4 -1
View File
@@ -78,7 +78,10 @@ identical graph still picked a different candidate.
edges targeting a symbol of that name whose source is in an *unchanged* file,
and re-inserts each as the reference that created it (the `metadata.refName`
stamp). The existing orphan sweep then resolves them against the post-sync
graph — the same input a rebuild resolves from. Kill switch:
graph — the same input a rebuild resolves from.
- Failed references parked under a delta name go back to pending for the same
sweep: a call that failed only because a second definition shared its name
links once that definition is deleted or renamed away. Kill switch for both:
`CODEGRAPH_NO_REBIND=1`.
The delta is compared **per file**, not as one name set over the whole batch: a
+13 -2
View File
@@ -1526,7 +1526,8 @@ export class CodeGraph {
// as a full index resolves.
//
// `definitionDelta` is empty for a body-only edit, so the overwhelmingly
// common sync pays one branch. CODEGRAPH_NO_REBIND=1 disables it.
// common sync pays one branch. CODEGRAPH_NO_REBIND=1 disables it, and
// the failed-ref re-open that follows the rebind with it.
if (result.definitionDelta && process.env.CODEGRAPH_NO_REBIND !== '1') {
const tRebind = Date.now();
const rebound = this.orchestrator.resurrectStaleResolutionEdges(
@@ -1534,9 +1535,19 @@ export class CodeGraph {
result.changedFilePaths ?? [],
edgesBeforeRetry
);
// And the failed refs under those names: a call that failed only
// because two definitions shared its name (`x.go()` with `go` in two
// classes) resolves once one of them is gone. The retry above looks
// failed refs up by the names the changed files carry now, which a
// deleted file or a name renamed away no longer has, and a sync that
// only deletes files skips it. The sweep below resolves what this
// re-opens.
const reopenedFailed = this.queries.reopenFailedReferences(
this.queries.getRetryableFailedReferences(result.definitionDelta).flatMap((ref) => (ref.rowId === undefined ? [] : [ref.rowId]))
);
if (process.env.CODEGRAPH_SYNTH_TIMINGS) {
console.error(
`[phase-timing] sync-rebind: ${Date.now() - tRebind}ms (${result.definitionDelta.length} changed names, ${rebound} edges re-opened)`
`[phase-timing] sync-rebind: ${Date.now() - tRebind}ms (${result.definitionDelta.length} changed names, ${rebound} edges and ${reopenedFailed} failed refs re-opened)`
);
}
}