From 74a25f587747f827de0287e0bd8381e2eaa646fd Mon Sep 17 00:00:00 2001 From: danusha2345 Date: Fri, 30 Oct 2026 17:53:44 -0500 Subject: [PATCH] fix(graph): impact stops at a safety budget and says it was truncated The impact walk had no limit, so one call on a heavily referenced symbol could load every transitive dependent into the query worker or the MCP process, and a large enough answer could run the process out of memory. getImpactRadius now takes a node and edge budget, 10,000 nodes and 50,000 edges by default, merged into the depth-aware walk from PR #2000. Re-expanding a node from a nearer depth is still allowed; only new nodes and new edges spend the budget. Edge reads are limited in SQL with one row over the budget, so hitting it can be told from an exact fit. codegraph_impact says when the answer was truncated and suggests `file` or a smaller `depth`. Changed on top of the PR: - codegraph_impact uses the same budget as the library. The PR gave MCP 1,000 nodes and 5,000 edges, but that truncated 25 of 75 busy-symbol queries on excalidraw, a mid-sized repo, and dropped direct dependents an answer shows today (the walk is depth-first, so a capped walk favours its first branches). - `codegraph impact` says so too, in its text and as `truncated` in --json. Without this the CLI would have stopped at the limit silently. Validation: - Red on main: 6 of 6 dependents past a budget of 3, 10,051 nodes from the CLI, and no truncation note. - Same answers: codegraph_impact on the 25 busiest symbols of excalidraw, gin and flask at depth 2, 3 and 5 gave byte-identical text on both arms (225 of 225). - A synthetic graph with 60 callers per node, depth 3: 219,661 nodes, 174 ms, 76 MB of heap and 155 MB RSS on main; 10,000 nodes, 8 ms, 4 MB and 59 MB here. Original PR #1583 by @danusha2345. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 1 + __tests__/cli-definition-grouping.test.ts | 27 +++++++++++++ __tests__/graph.test.ts | 24 ++++++++++++ src/bin/codegraph.ts | 10 ++++- src/db/queries.ts | 26 ++++++++++--- src/graph/traversal.ts | 47 +++++++++++++++++++---- src/index.ts | 5 ++- src/mcp/tools.ts | 23 +++++++++-- src/types.ts | 11 ++++++ 9 files changed, 153 insertions(+), 21 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c864a0aad4..bb0ab73990 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -135,6 +135,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). - Opening a project whose index needs a schema update no longer fails when several CodeGraph processes open it at the same moment, like agent sessions starting right after an upgrade, or the MCP server and a git hook: one of them updates the index while the others wait for it and then carry on. Before, the ones that lost the race applied the same update again, and failed with "UNIQUE constraint failed" or "database is locked". Thanks @inth3shadows for the report and @johnhldiver-beep for the first fix. (#2980) - A qualified name given to `codegraph callers`, `callees` or `impact`, or to `codegraph_node`, like `Format.Price`, now names only the symbol whose scope is spelled that way, so its answer no longer merges in `LegacyFormat.Price` or any other class whose name only ends with `Format`, nor a class that just sits in a file named `Format.cs`. In C#, `BasketService.AddItemToBasket` no longer also selects the `IBasketService` interface's method; ask for `IBasketService.AddItemToBasket` for that one. A file or folder still names the module in Rust, Python and Go, and a PHP namespace can be written with its own backslashes, like `Billing\Format::price`. No re-index needed. Thanks @Gobakabana for the report and @yang0228. (#2908) - Finding the path between two symbols no longer slows down sharply, or grows its memory use, when the path is long or the graph is densely connected. Thanks @danusha2345. +- `codegraph impact` and `codegraph_impact` on a very heavily referenced symbol now stop at a safety limit and say the answer was truncated, instead of exhausting the memory of the background server. Narrow it with `--file` / `file` or a smaller depth. Thanks @danusha2345. - `codegraph callers` and the `codegraph_callers` tool now list each caller at the lines where it calls the symbol, every one of them, like `src/report.ts:9, 10` for a function that calls it twice, and a function that calls itself among its own callers, marked as calling itself. Before, a caller showed only the line its own declaration starts on, a second call in the same caller didn't show at all, and a recursive call was left out, though each is a line to edit when the symbol changes. `codegraph_node` lists its callers the same way, `codegraph callees` and `codegraph_callees` include a function that calls itself, and `--json` gives every caller its call-site lines. A caller with many calls shows the first few lines and how many more there are. Thanks @Gobakabana for the report and @johnhldiver-beep for the first fix. (#2909) - `codegraph affected --filter` now matches its glob against whole names: `tests/**/*.py` also finds the tests directly inside `tests/`, including a changed test file itself, `tests/*.py` no longer counts a changed `tests/x.py.bak` as a test, and `e2e/*.spec.ts` no longer matches a folder that only ends in `e2e`, like `xe2e/`. A pattern with a folder in it still matches that folder at any depth, so `e2e/*.spec.ts` keeps finding `apps/web/e2e/login.spec.ts`; start it with `/` to match from the project root only. Thanks @Jseca666. - `codegraph init` and `codegraph index` no longer report a file you deleted as one that "could not be read" while git still lists it, which it does until the deletion is staged: such a file is now skipped, and so is a symlink that points at nothing. A file that is there but can't be read is still reported. Thanks @xianzhi-xzw for the report and the first fix. (#2904) diff --git a/__tests__/cli-definition-grouping.test.ts b/__tests__/cli-definition-grouping.test.ts index 12f2f6b2a2..a65ca44c48 100644 --- a/__tests__/cli-definition-grouping.test.ts +++ b/__tests__/cli-definition-grouping.test.ts @@ -235,6 +235,33 @@ describe('CLI definition boundaries and limits', () => { } }); + it('says so when impact stops at its safety limit, and only then', async () => { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-impact-limit-')); + try { + const callers = Array.from({ length: 10_050 }, (_, i) => `export function caller${i}() { hub(); }`); + fs.writeFileSync(path.join(root, 'hub.ts'), ['export function hub() {}', ...callers].join('\n')); + (await CodeGraph.init(root, { index: true })).close(); + const impact = (...args: string[]) => spawnSync(process.execPath, [BIN, 'impact', '-p', root, ...args, '--', 'hub'], { + encoding: 'utf-8', + env: { ...process.env, CODEGRAPH_NO_DAEMON: '1', CODEGRAPH_WASM_RELAUNCHED: '1', NO_COLOR: '1' }, + maxBuffer: 64 * 1024 * 1024, + timeout: 60_000, + }); + + const out = JSON.parse(impact('--json').stdout); + expect(out.nodeCount).toBe(10_000); + expect(out.truncated).toBe(true); + expect(out.definitions[0].truncated).toBe(true); + const text = impact(); + expect(text.status, text.stderr).toBe(0); + expect(text.stdout + text.stderr).toMatch(/truncated at the safety limit/i); + + expect(json('impact').truncated).toBeUndefined(); + } finally { + fs.rmSync(root, { recursive: true, force: true }); + } + }, 120_000); + it('applies impact depth within each definition and reports its own graph counts', () => { for (const depth of [1, 2]) { const out = json('impact', 'handle', ['--depth', String(depth)]); diff --git a/__tests__/graph.test.ts b/__tests__/graph.test.ts index 105268952b..7edd0570fb 100644 --- a/__tests__/graph.test.ts +++ b/__tests__/graph.test.ts @@ -11,6 +11,7 @@ import * as os from 'os'; import CodeGraph from '../src/index'; import { Node, Edge } from '../src/types'; import { GraphTraverser } from '../src/graph/traversal'; +import { ToolHandler } from '../src/mcp/tools'; describe('Graph Queries', () => { let testDir: string; @@ -727,6 +728,29 @@ describe('Traversal edge-completeness & limits (#1086–#1090)', () => { const callees = tGraph(depthNodes, edges).getCallees('t', 2); expect(callees.map((c) => c.node.id).sort()).toEqual(['a', 'b', 'c']); }); + + it('getImpactRadius stops at node/edge budgets and marks truncation', () => { + const dependents = ['B', 'C', 'D', 'E', 'F']; + const nodes = [tNode('A'), ...dependents.map((id) => tNode(id))]; + const edges: Edge[] = dependents.map((source) => ({ source, target: 'A', kind: 'calls' })); + const sub = tGraph(nodes, edges).getImpactRadius('A', 2, { maxNodes: 3, maxEdges: 2 }); + + expect(sub.nodes.size).toBe(3); + expect(sub.edges).toHaveLength(2); + expect(sub.truncated).toBe(true); + expect(sub.edges.every((edge) => sub.nodes.has(edge.source) && sub.nodes.has(edge.target))).toBe(true); + }); + + it('surfaces impact truncation explicitly in MCP output', () => { + const formatted = (new ToolHandler(null) as any).formatImpact('A', { + nodes: new Map([['A', tNode('A')]]), + edges: [], + roots: ['A'], + truncated: true, + }); + expect(formatted).toMatch(/truncated at safety limit/i); + expect(formatted).toMatch(/reduce `depth`/i); + }); }); describe('findPath enqueue-once (#1359)', () => { diff --git a/src/bin/codegraph.ts b/src/bin/codegraph.ts index 1cc271e114..6c0b40f4f8 100644 --- a/src/bin/codegraph.ts +++ b/src/bin/codegraph.ts @@ -2680,24 +2680,28 @@ program const collected = groups.map((group) => { const nodes = new Map(); const edges = new Map(); + let truncated = false; for (const target of group) { const impact = cg.getImpactRadius(target.id, depth); + truncated ||= impact.truncated === true; for (const [id, node] of impact.nodes) nodes.set(id, node); for (const edge of impact.edges) edges.set(`${edge.source}->${edge.target}:${edge.kind}`, edge); } - return { group, nodes, edges }; + return { group, nodes, edges, truncated }; }); + const truncated = collected.some((c) => c.truncated); if (options.json) { const unionNodes = new Map(); const unionEdges = new Map(); - const definitions = collected.map(({ group, nodes, edges }) => { + const definitions = collected.map(({ group, nodes, edges, truncated }) => { for (const [id, node] of nodes) unionNodes.set(id, node); for (const [key, edge] of edges) unionEdges.set(key, edge); return { ...cliDefinition(group), nodeCount: nodes.size, edgeCount: edges.size, + ...(truncated ? { truncated } : {}), affected: [...nodes.values()].map((node) => ({ id: node.id, ...cliNode(node) })), edges: [...edges.values()], }; @@ -2714,6 +2718,7 @@ program definitions, nodeCount: unionNodes.size, edgeCount: unionEdges.size, + ...(truncated ? { truncated } : {}), affected: [...unionNodes.values()].map(cliNode), }, null, 2)); } else { @@ -2743,6 +2748,7 @@ program console.log(); } } + if (truncated) warn('Result truncated at the safety limit. Narrow it with --file or a smaller --depth.'); } } finally { cg.destroy(); diff --git a/src/db/queries.ts b/src/db/queries.ts index 5435cf9984..c91effda7a 100644 --- a/src/db/queries.ts +++ b/src/db/queries.ts @@ -2639,8 +2639,8 @@ export class QueryBuilder { * Get outgoing edges from a node. Preserve the source/kind index order * (calls before imports/references), then break ties deterministically. */ - getOutgoingEdges(sourceId: string, kinds?: EdgeKind[], provenance?: string): Edge[] { - if ((kinds && kinds.length > 0) || provenance) { + getOutgoingEdges(sourceId: string, kinds?: EdgeKind[], provenance?: string, limit?: number): Edge[] { + if ((kinds && kinds.length > 0) || provenance || limit !== undefined) { let sql = 'SELECT * FROM edges WHERE source = ?'; const params: (string | number)[] = [sourceId]; @@ -2655,6 +2655,10 @@ export class QueryBuilder { } sql += ' ORDER BY kind, target, line, col'; + if (limit !== undefined) { + sql += ' LIMIT ?'; + params.push(Math.max(0, Math.floor(limit))); + } const rows = this.edgeKindStmt(sql).all(...params) as EdgeRow[]; return rows.map(rowToEdge); } @@ -2672,10 +2676,20 @@ export class QueryBuilder { * displace actual calls in capped caller lists. Keep deterministic ties * without changing the target/kind index's established kind precedence. */ - getIncomingEdges(targetId: string, kinds?: EdgeKind[]): Edge[] { - if (kinds && kinds.length > 0) { - const sql = `SELECT * FROM edges WHERE target = ? AND kind IN (${kinds.map(() => '?').join(',')}) ORDER BY kind, source, line, col`; - const rows = this.edgeKindStmt(sql).all(targetId, ...kinds) as EdgeRow[]; + getIncomingEdges(targetId: string, kinds?: EdgeKind[], limit?: number): Edge[] { + if ((kinds && kinds.length > 0) || limit !== undefined) { + let sql = 'SELECT * FROM edges WHERE target = ?'; + const params: (string | number)[] = [targetId]; + if (kinds && kinds.length > 0) { + sql += ` AND kind IN (${kinds.map(() => '?').join(',')})`; + params.push(...kinds); + } + sql += ' ORDER BY kind, source, line, col'; + if (limit !== undefined) { + sql += ' LIMIT ?'; + params.push(Math.max(0, Math.floor(limit))); + } + const rows = this.edgeKindStmt(sql).all(...params) as EdgeRow[]; return rows.map(rowToEdge); } diff --git a/src/graph/traversal.ts b/src/graph/traversal.ts index 3630ed3df4..280ee59c83 100644 --- a/src/graph/traversal.ts +++ b/src/graph/traversal.ts @@ -4,7 +4,7 @@ * BFS and DFS traversal for the code knowledge graph. */ -import { Node, Edge, Subgraph, TraversalOptions, EdgeKind } from '../types'; +import { Node, Edge, Subgraph, TraversalOptions, EdgeKind, ImpactOptions, EDGE_KINDS } from '../types'; import { QueryBuilder } from '../db/queries'; /** @@ -19,6 +19,10 @@ const DEFAULT_OPTIONS: Required = { includeStart: true, }; +export const DEFAULT_IMPACT_MAX_NODES = 10_000; +export const DEFAULT_IMPACT_MAX_EDGES = 50_000; +const IMPACT_INCOMING_KINDS = EDGE_KINDS.filter((kind) => kind !== 'contains'); + /** * Result of a single traversal step */ @@ -600,7 +604,7 @@ export class GraphTraverser { * @param maxDepth - Maximum depth to traverse (default: 3) * @returns Subgraph containing potentially impacted nodes */ - getImpactRadius(nodeId: string, maxDepth: number = 3): Subgraph { + getImpactRadius(nodeId: string, maxDepth: number = 3, options: ImpactOptions = {}): Subgraph { const focalNode = this.queries.getNodeById(nodeId); if (!focalNode) { return { nodes: new Map(), edges: [], roots: [] }; @@ -609,17 +613,23 @@ export class GraphTraverser { const nodes = new Map(); const edges: Edge[] = []; const visited = new Map(); + const budget = { + maxNodes: Math.max(1, Math.floor(options.maxNodes ?? DEFAULT_IMPACT_MAX_NODES)), + maxEdges: Math.max(0, Math.floor(options.maxEdges ?? DEFAULT_IMPACT_MAX_EDGES)), + truncated: false, + }; // Add focal node nodes.set(focalNode.id, focalNode); // Traverse incoming edges to find all dependents - this.getImpactRecursive(nodeId, maxDepth, 0, nodes, edges, visited, new Set()); + this.getImpactRecursive(nodeId, maxDepth, 0, nodes, edges, visited, new Set(), budget); return { nodes, edges, roots: [nodeId], + truncated: budget.truncated || undefined, }; } @@ -630,7 +640,8 @@ export class GraphTraverser { nodes: Map, edges: Edge[], visited: Map, - expanded: Set + expanded: Set, + budget: { maxNodes: number; maxEdges: number; truncated: boolean } ): void { // Mark visited before the depth check so a node collected at the depth // boundary still lands in `visited`. Otherwise it could sit in `nodes` but @@ -651,18 +662,25 @@ export class GraphTraverser { if (focalNode) { const containerKinds = new Set(['class', 'interface', 'struct', 'union', 'trait', 'protocol', 'module', 'enum']); if (containerKinds.has(focalNode.kind)) { - const containsEdges = this.queries.getOutgoingEdges(nodeId, ['contains']); + const remaining = budget.maxEdges - edges.length; + const fetched = this.queries.getOutgoingEdges(nodeId, ['contains'], undefined, remaining + 1); + if (fetched.length > remaining) budget.truncated = true; + const containsEdges = fetched.slice(0, remaining); if (containsEdges.length > 0) { const children = this.queries.getNodesByIds(containsEdges.map((e) => e.target)); for (const edge of containsEdges) { const childNode = children.get(edge.target); if (childNode && this.nearerThanBefore(visited, childNode.id, currentDepth)) { if (!nodes.has(childNode.id)) { + if (nodes.size >= budget.maxNodes || edges.length >= budget.maxEdges) { + budget.truncated = true; + continue; + } nodes.set(childNode.id, childNode); edges.push(edge); } // Recurse into children at the same depth (they're part of the same symbol) - this.getImpactRecursive(childNode.id, maxDepth, currentDepth, nodes, edges, visited, expanded); + this.getImpactRecursive(childNode.id, maxDepth, currentDepth, nodes, edges, visited, expanded, budget); } } } @@ -673,7 +691,12 @@ export class GraphTraverser { // `contains`: a container "contains" its members but does not *depend* on // them, so following it upward would climb to the parent class and then // re-expand every sibling member — exploding impact for a leaf symbol. (#536) - const incomingEdges = this.queries.getIncomingEdges(nodeId).filter((e) => e.kind !== 'contains'); + const remaining = budget.maxEdges - edges.length; + // Fetch one extra row so hitting the SQL LIMIT is distinguishable from an + // exact-size result and can be surfaced as explicit truncation. + const fetched = this.queries.getIncomingEdges(nodeId, IMPACT_INCOMING_KINDS, remaining + 1); + if (fetched.length > remaining) budget.truncated = true; + const incomingEdges = fetched.slice(0, remaining); if (incomingEdges.length === 0) return; const sources = this.queries.getNodesByIds(incomingEdges.map((e) => e.source)); @@ -685,10 +708,18 @@ export class GraphTraverser { // node already collected via another path was silently dropped from // `edges` even though it's a real dependency (#1089). Each node's incoming // edges are recorded on its first expansion only, so no edge repeats. + if (!nodes.has(sourceNode.id) && nodes.size >= budget.maxNodes) { + budget.truncated = true; + continue; + } + if (edges.length >= budget.maxEdges) { + budget.truncated = true; + continue; + } if (firstExpansion) edges.push(edge); if (this.nearerThanBefore(visited, sourceNode.id, currentDepth + 1)) { nodes.set(sourceNode.id, sourceNode); - this.getImpactRecursive(sourceNode.id, maxDepth, currentDepth + 1, nodes, edges, visited, expanded); + this.getImpactRecursive(sourceNode.id, maxDepth, currentDepth + 1, nodes, edges, visited, expanded, budget); } } } diff --git a/src/index.ts b/src/index.ts index 76a5b3c51a..56ea79d9ed 100644 --- a/src/index.ts +++ b/src/index.ts @@ -27,6 +27,7 @@ import { UnresolvedReference, IndexHealth, LanguageCoverage, + ImpactOptions, Language, } from './types'; import { DatabaseConnection, getDatabasePath, removeDatabaseFiles } from './db'; @@ -3046,8 +3047,8 @@ export class CodeGraph { * @param maxDepth - Maximum depth to traverse (default: 3) * @returns Subgraph containing potentially impacted nodes */ - getImpactRadius(nodeId: string, maxDepth: number = 3): Subgraph { - return this.traverser.getImpactRadius(nodeId, maxDepth); + getImpactRadius(nodeId: string, maxDepth: number = 3, options: ImpactOptions = {}): Subgraph { + return this.traverser.getImpactRadius(nodeId, maxDepth, options); } /** diff --git a/src/mcp/tools.ts b/src/mcp/tools.ts index b8e7dcbf19..f5961d3012 100644 --- a/src/mcp/tools.ts +++ b/src/mcp/tools.ts @@ -58,7 +58,7 @@ import { findDynamicBoundaries, type BoundarySite } from '../graph/dynamic-bound import { countImplementers } from '../graph/type-hierarchy'; import { describeSynthesizedHop } from '../graph/synthesized-hop'; import { callSiteLines, formatCallSiteLines } from '../graph/call-sites'; -import type { CallSiteRow } from '../graph/traversal'; +import { DEFAULT_IMPACT_MAX_EDGES, DEFAULT_IMPACT_MAX_NODES, type CallSiteRow } from '../graph/traversal'; import { isCopybookInclude, MAX_COPYBOOK_INCLUDES, type NamedCopybook } from '../graph/cobol-copybooks'; import { groupFilesByPackage, packageGroupTitle, packageListingTitle } from '../graph/owning-package'; import { @@ -3582,21 +3582,35 @@ export class ToolHandler { const mergedNodes = new Map(); const mergedEdges: Edge[] = []; const seenEdges = new Set(); + let truncated = false; for (const node of defNodes) { const impact = cg.getImpactRadius(node.id, depth); + truncated ||= impact.truncated === true; for (const [id, n] of impact.nodes) { + if (!mergedNodes.has(id) && mergedNodes.size >= DEFAULT_IMPACT_MAX_NODES) { + truncated = true; + continue; + } mergedNodes.set(id, n); answerPaths.add(n.filePath); } for (const e of impact.edges) { + if (!mergedNodes.has(e.source) || !mergedNodes.has(e.target)) { + truncated = true; + continue; + } const key = `${e.source}->${e.target}:${e.kind}`; if (!seenEdges.has(key)) { + if (mergedEdges.length >= DEFAULT_IMPACT_MAX_EDGES) { + truncated = true; + continue; + } seenEdges.add(key); mergedEdges.push(e); } } } - return { nodes: mergedNodes, edges: mergedEdges, roots: defNodes.map((n) => n.id) }; + return { nodes: mergedNodes, edges: mergedEdges, roots: defNodes.map((n) => n.id), truncated }; }; // Single definition (or same-file overloads): the familiar merged report. @@ -9043,9 +9057,12 @@ export class ToolHandler { // Compact format: just list affected symbols grouped by file const lines: string[] = [ - `**Impact: "${symbol}" affects ${nodeCount} symbols**`, + `**Impact: "${symbol}" affects ${nodeCount} symbols${impact.truncated ? ' (truncated at safety limit)' : ''}**`, '', ]; + if (impact.truncated) { + lines.push('> Result truncated to protect the MCP process on a high-fanout graph. Narrow with `file` or reduce `depth`.', ''); + } // Group by file const byFile = new Map(); diff --git a/src/types.ts b/src/types.ts index 05313675f0..b16f55c9ed 100644 --- a/src/types.ts +++ b/src/types.ts @@ -440,6 +440,17 @@ export interface Subgraph { * for graph traversals that don't run the search-ranking path. */ confidence?: 'high' | 'low'; + + /** True when a traversal stopped at its node/edge safety budget. */ + truncated?: boolean; +} + +/** Safety budgets for impact traversal on high-fanout graphs. */ +export interface ImpactOptions { + /** Maximum nodes retained, including the focal node. Default: 10,000. */ + maxNodes?: number; + /** Maximum edges retained. Default: 50,000. */ + maxEdges?: number; } /**