diff --git a/CHANGELOG.md b/CHANGELOG.md index c864a0aad..bb0ab7399 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 12f2f6b2a..a65ca44c4 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 105268952..7edd0570f 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 1cc271e11..6c0b40f4f 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 5435cf998..c91effda7 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 3630ed3df..280ee59c8 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 76a5b3c51..56ea79d9e 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 b8e7dcbf1..f5961d301 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 05313675f..b16f55c9e 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; } /**