fix(graph): impact reads a node's edges in one unlimited query again (#3248)

__tests__/graph.test.ts

@@ -361,6 +361,20 @@ export { main };
       // sibling method and explode impact for a leaf symbol. (#536)
       expect(impact.nodes.has(derived!.id)).toBe(false);
     });
+
+    it('reads incoming edges without contains, in the same order as getIncomingEdges (#3248)', () => {
+      const queries = (cg as unknown as { queries: import('../src/db/queries').QueryBuilder }).queries;
+      const all = queries.getAllNodes();
+      let withContains = 0;
+      for (const node of all) {
+        const incoming = queries.getIncomingEdges(node.id);
+        const expected = incoming.filter((e) => e.kind !== 'contains');
+        if (expected.length !== incoming.length) withContains++;
+        expect(queries.getIncomingEdgesExceptContains(node.id)).toEqual(expected);
+      }
+      // The fixture exercises the filter: members have an incoming contains edge.
+      expect(withContains).toBeGreaterThan(0);
+    });
   });
 
   describe('findPath()', () => {
@@ -549,6 +563,8 @@ function tGraph(nodes: Node[], edges: Edge[]): GraphTraverser {
       edges.filter((e) => e.source === source && (!kinds || kinds.includes(e.kind))),
     getIncomingEdges: (target: string, kinds?: string[]) =>
       edges.filter((e) => e.target === target && (!kinds || kinds.includes(e.kind))),
+    getIncomingEdgesExceptContains: (target: string) =>
+      edges.filter((e) => e.target === target && e.kind !== 'contains'),
   };
   return new GraphTraverser(q as never);
 }
@@ -741,6 +757,63 @@ describe('Traversal edge-completeness & limits (#1086–#1090)', () => {
     expect(sub.edges.every((edge) => sub.nodes.has(edge.source) && sub.nodes.has(edge.target))).toBe(true);
   });
 
+  it('getImpactRadius reads a node\'s edges without a SQL LIMIT, and budgets after dropping contains (#3248)', () => {
+    // K is a class that contains A. B calls A, C calls B.
+    const nodes = [tNode('A'), tNode('B'), tNode('C'), tNode('K', 'class')];
+    const edges: Edge[] = [
+      { source: 'K', target: 'A', kind: 'contains', line: 1 },
+      { source: 'B', target: 'A', kind: 'calls', line: 2 },
+      { source: 'C', target: 'B', kind: 'calls', line: 3 },
+    ];
+    const byId = new Map(nodes.map((n) => [n.id, n]));
+    const limits: unknown[] = [];
+    const q = {
+      getNodeById: (id: string) => byId.get(id) ?? null,
+      getNodesByIds: (ids: readonly string[]) =>
+        new Map(ids.flatMap((id) => (byId.has(id) ? [[id, byId.get(id)!] as const] : []))),
+      // Every read behaves like its SQL: a kind list filters, a limit caps.
+      getOutgoingEdges: (source: string, kinds?: string[], provenance?: string, limit?: number) => {
+        limits.push(limit);
+        const rows = edges.filter((e) => e.source === source && (!kinds || kinds.includes(e.kind)));
+        return limit === undefined ? rows : rows.slice(0, limit);
+      },
+      getIncomingEdges: (target: string, kinds?: string[], limit?: number) => {
+        limits.push(limit);
+        const rows = edges.filter((e) => e.target === target && (!kinds || kinds.includes(e.kind)));
+        return limit === undefined ? rows : rows.slice(0, limit);
+      },
+      getIncomingEdgesExceptContains: (...args: unknown[]) => {
+        // Any argument past the target would be a new bound on the read.
+        limits.push(args.length > 1 ? args[1] : undefined);
+        return edges.filter((e) => e.target === args[0] && e.kind !== 'contains');
+      },
+    };
+    const traverser = new GraphTraverser(q as never);
+
+    const sub = traverser.getImpactRadius('A', 3);
+    expect([...sub.nodes.keys()].sort()).toEqual(['A', 'B', 'C']);
+    expect(sub.edges.some((e) => e.kind === 'contains')).toBe(false);
+    expect(sub.truncated).toBeUndefined();
+    // The walk reads once per visited node, and a LIMIT under the reads'
+    // ORDER BY made each one markedly slower, so the budget is applied after
+    // the read instead. K is a container, so impact on it reads its members.
+    traverser.getImpactRadius('K', 2);
+    traverser.getImpactRadius('K', 2, { maxEdges: 1 });
+    expect(limits.length).toBeGreaterThan(0);
+    expect(limits.every((limit) => limit === undefined)).toBe(true);
+
+    // The contains edge is dropped before the budget applies, so it neither
+    // takes A's one edge slot nor marks the result truncated.
+    const tight = traverser.getImpactRadius('A', 1, { maxEdges: 1 });
+    expect(tight.edges.map((e) => `${e.source}>${e.target}`)).toEqual(['B>A']);
+    expect(tight.truncated).toBeUndefined();
+    // A second dependent of A is over that budget, and says so.
+    edges.push({ source: 'C', target: 'A', kind: 'calls', line: 4 });
+    const over = traverser.getImpactRadius('A', 1, { maxEdges: 1 });
+    expect(over.edges).toHaveLength(1);
+    expect(over.truncated).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')]]),

src/db/queries.ts

@@ -483,6 +483,7 @@ export class QueryBuilder {
     deleteEdgesByTarget?: SqliteStatement;
     getEdgesBySource?: SqliteStatement;
     getEdgesByTarget?: SqliteStatement;
+    getDependentEdgesByTarget?: SqliteStatement;
     getUnresolvedFromNode?: SqliteStatement;
     getUnresolvedInFile?: SqliteStatement;
     insertFile?: SqliteStatement;
@@ -2768,6 +2769,24 @@ export class QueryBuilder {
     return rows.map(rowToEdge);
   }
 
+  /**
+   * Incoming edges to a node except `contains`: the edges that make their
+   * source depend on the target, in {@link getIncomingEdges} order.
+   *
+   * Impact traversal runs this once per visited node, so it stays one index
+   * search with no LIMIT. A kind list searches the index once per kind, and a
+   * LIMIT under this ORDER BY made each read markedly slower (#3248).
+   */
+  getIncomingEdgesExceptContains(targetId: string): Edge[] {
+    if (!this.stmts.getDependentEdgesByTarget) {
+      this.stmts.getDependentEdgesByTarget = this.db.prepare(
+        "SELECT * FROM edges WHERE target = ? AND kind != 'contains' ORDER BY kind, source, line, col"
+      );
+    }
+    const rows = this.stmts.getDependentEdgesByTarget.all(targetId) as EdgeRow[];
+    return rows.map(rowToEdge);
+  }
+
   /**
    * Get incoming edges to a node. Kind must precede opaque source IDs:
    * file IDs sort before function IDs, so source-first ordering lets imports

src/graph/traversal.ts

@@ -4,7 +4,7 @@
  * BFS and DFS traversal for the code knowledge graph.
  */
 
-import { Node, Edge, Subgraph, TraversalOptions, EdgeKind, ImpactOptions, EDGE_KINDS } from '../types';
+import { Node, Edge, Subgraph, TraversalOptions, EdgeKind, ImpactOptions } from '../types';
 import { QueryBuilder } from '../db/queries';
 
 /**
@@ -21,7 +21,6 @@ const DEFAULT_OPTIONS: Required<TraversalOptions> = {
 
 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
@@ -663,7 +662,8 @@ export class GraphTraverser {
       const containerKinds = new Set(['class', 'interface', 'struct', 'union', 'trait', 'protocol', 'module', 'enum']);
       if (containerKinds.has(focalNode.kind)) {
         const remaining = budget.maxEdges - edges.length;
-        const fetched = this.queries.getOutgoingEdges(nodeId, ['contains'], undefined, remaining + 1);
+        // No SQL LIMIT here either: the slice applies the budget (#3248).
+        const fetched = this.queries.getOutgoingEdges(nodeId, ['contains']);
         if (fetched.length > remaining) budget.truncated = true;
         const containsEdges = fetched.slice(0, remaining);
         if (containsEdges.length > 0) {
@@ -692,9 +692,9 @@ export class GraphTraverser {
     // 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 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);
+    // The whole list, then the slice applies the budget: a SQL LIMIT made this
+    // per-node read markedly slower (#3248).
+    const fetched = this.queries.getIncomingEdgesExceptContains(nodeId);
     if (fetched.length > remaining) budget.truncated = true;
     const incomingEdges = fetched.slice(0, remaining);
     if (incomingEdges.length === 0) return;