From f4a9af297b9b26eaab233b8fab57c83fbcf1f747 Mon Sep 17 00:00:00 2001 From: Colby Mchenry Date: Sun, 27 Sep 2026 13:09:25 +0000 Subject: [PATCH] fix(graph): enqueue path-search targets once (#1359) (#2016) Path search marked nodes visited only on dequeue, allowing pending targets to be queued repeatedly with copied paths. Track enqueued nodes from the starting node and exclude them from target lookups and subsequent pushes. Add real-SQLite operation-count and path-correctness regression coverage. Co-authored-by: Claude Opus 5.5 --- __tests__/graph.test.ts | 79 +++++++++++++++++++++++++++++++++++++++++ src/graph/traversal.ts | 8 +++-- 2 files changed, 84 insertions(+), 3 deletions(-) diff --git a/__tests__/graph.test.ts b/__tests__/graph.test.ts index c4ac708a4..875c9ed2a 100644 --- a/__tests__/graph.test.ts +++ b/__tests__/graph.test.ts @@ -651,3 +651,82 @@ describe('Traversal edge-completeness & limits (#1086–#1090)', () => { expect(callees.map((c) => c.node.id).sort()).toEqual(['a', 'b', 'c']); }); }); + +describe('findPath enqueue-once (#1359)', () => { + let testDir: string; + let cg: CodeGraph; + + beforeEach(() => { + testDir = fs.mkdtempSync(path.join(os.tmpdir(), 'codegraph-findpath-')); + fs.writeFileSync(path.join(testDir, 'graph.ts'), 'export function start() {}\n'); + cg = CodeGraph.initSync(testDir); + }); + + afterEach(() => { + cg?.close(); + fs.rmSync(testDir, { recursive: true, force: true }); + }); + + function seed(ids: string[], edges: Edge[]) { + for (const id of ids) { + cg['queries'].insertNode({ ...tNode(id), filePath: 'graph.ts' }); + } + edges.forEach((edge, i) => cg['queries'].insertEdge({ ...edge, line: i + 1 })); + } + + it('enqueues each dense fan-in target once while preserving the shortest path', () => { + const layerA = Array.from({ length: 16 }, (_, i) => `a${i}`); + const layerB = Array.from({ length: 16 }, (_, i) => `b${i}`); + const ids = ['start', ...layerA, ...layerB, 'end']; + const edges: Edge[] = []; + for (const a of layerA) { + edges.push({ source: 'start', target: a, kind: 'calls' }); + for (const b of layerB) edges.push({ source: a, target: b, kind: 'calls' }); + } + for (const b of layerB) edges.push({ source: b, target: 'end', kind: 'calls' }); + seed(ids, edges); + + // Count actual queue insertions, without timing thresholds or replacing SQLite. + const counts = new Map(); + const push = Array.prototype.push; + let result: ReturnType; + try { + Array.prototype.push = function (...items) { + for (const item of items) { + if (item && typeof item.nodeId === 'string' && Array.isArray(item.path)) { + counts.set(item.nodeId, (counts.get(item.nodeId) ?? 0) + 1); + } + } + return Reflect.apply(push, this, items); + }; + result = cg.findPath('start', 'end', ['calls']); + } finally { + Array.prototype.push = push; + } + expect(result?.map((step) => step.node.id)).toEqual(['start', 'a0', 'b0', 'end']); + expect(result?.slice(1).every((step) => step.edge?.kind === 'calls')).toBe(true); + expect(counts).toEqual(new Map(ids.slice(1).map((id) => [id, 1]))); + }); + + it('preserves shortest paths and edge filters with cycles and parallel edges', () => { + seed(['start', 'a', 'b', 'end', 'isolated'], [ + { source: 'start', target: 'start', kind: 'calls' }, + { source: 'start', target: 'a', kind: 'calls' }, + { source: 'start', target: 'a', kind: 'calls' }, + { source: 'a', target: 'start', kind: 'calls' }, + { source: 'a', target: 'b', kind: 'calls' }, + { source: 'b', target: 'end', kind: 'calls' }, + { source: 'start', target: 'end', kind: 'references' }, + ]); + const result = cg.findPath('start', 'end', ['calls']); + expect(result?.map((step) => step.node.id)).toEqual(['start', 'a', 'b', 'end']); + expect(result?.[1].edge?.line).toBe(2); + expect(cg.findPath('start', 'end')?.map((step) => step.node.id)).toEqual(['start', 'end']); + expect(cg.findPath('start', 'start')?.map((step) => step.node.id)).toEqual(['start']); + expect(cg.findPath('start', 'isolated')).toBeNull(); + expect(cg.findPath('missing', 'end')).toBeNull(); + expect(cg.findPath('start', 'missing')).toBeNull(); + expect(cg.findPath('missing', 'missing')).toBeNull(); + expect(cg.findPath('start', 'end', ['imports'])).toBeNull(); + }); +}); diff --git a/src/graph/traversal.ts b/src/graph/traversal.ts index 481d30d7b..47368ac6a 100644 --- a/src/graph/traversal.ts +++ b/src/graph/traversal.ts @@ -650,6 +650,7 @@ export class GraphTraverser { // BFS to find shortest path const visited = new Set(); + const enqueued = new Set([fromId]); const queue: Array<{ nodeId: string; path: Array<{ node: Node; edge: Edge | null }> }> = [ { nodeId: fromId, path: [{ node: fromNode, edge: null }] }, ]; @@ -673,16 +674,17 @@ export class GraphTraverser { ); if (outgoingEdges.length === 0) continue; - // Batch-fetch only the unvisited targets (was N+1 per BFS frontier). + // Batch-fetch only targets not yet visited or queued. const wantIds = outgoingEdges .map((e) => e.target) - .filter((id) => !visited.has(id)); + .filter((id) => !visited.has(id) && !enqueued.has(id)); const nextNodes = wantIds.length > 0 ? this.queries.getNodesByIds(wantIds) : new Map(); for (const edge of outgoingEdges) { - if (!visited.has(edge.target)) { + if (!visited.has(edge.target) && !enqueued.has(edge.target)) { const nextNode = nextNodes.get(edge.target); if (nextNode) { + enqueued.add(edge.target); queue.push({ nodeId: edge.target, path: [...path, { node: nextNode, edge }],