getImpactRecursive: inconsistent visited vs nodes gate causes incoming edges to be silently dropped

[inth3shadows]

File: src/graph/traversal.ts
Lines: 506–509 (depth guard), 522 (container-children gate), 543 (incoming-edges gate)

Defect: getImpactRecursive uses two different sets to gate whether a node is re-processed:

• Container-children loop (L522): if (childNode && !visited.has(childNode.id))
• Incoming-edges loop (L543): if (sourceNode && !nodes.has(sourceNode.id))

The depth guard at L506–508 (if (currentDepth >= maxDepth || visited.has(nodeId)) return;) returns before visited.add(nodeId) at L509 when the depth limit fires. This means a node can be added to nodes (by its parent's loop) but absent from visited. When that node later appears as an incoming-edge source for a different node, the !nodes.has() gate at L543 skips it — and the corresponding edge is silently dropped from the result.

Failure trace:

• Graph: container P has child method M. Both M and P have an incoming calls edge from Q.
• getImpactRadius("Z", 2) where Z → P → M in the dependency chain; currentDepth=1 when reaching P.
• getImpactRecursive(P, 2, 1, …): visited.add(P). Container loop: !visited.has(M) = true → nodes.set(M); recurse (M, 2, 1, …).
  • Inside M's recursion: visited.add(M). Incoming from Q: !nodes.has(Q) = true → nodes.set(Q); recurse (Q, 2, 2, …).
    • currentDepth=2 ≥ maxDepth=2 → return without visited.add(Q). Q is in nodes, not in visited.
• Back in P's incoming-edges loop (L541): P has incoming from Q (Q→P).
  • !nodes.has(Q) = false (Q was added via M's processing). Gate fails → Q→P edge silently dropped.
• Impact result is missing edge Q→P; Q's path to P via its direct edge is invisible.

Expected fix:
Unify the two gates to use the same set. Since visited tracks "fully processed" nodes while nodes tracks "collected" nodes, the correct guard for both loops should be !nodes.has() (skip if already in result) OR !visited.has() (re-process if not yet traversed). Given the depth-guard asymmetry, the simplest fix is to ensure visited.add(nodeId) always runs before any early-return from the depth arm:

if (visited.has(nodeId)) return;
visited.add(nodeId);                   // mark before depth check
if (currentDepth >= maxDepth) return;

Then both loops can safely use !visited.has() for consistency, or keep the nodes check (since a visited node will also be in nodes).

[inth3shadows]

---

Verification & metadata (added after filing)

Anchor: Introduced in cc6e7a5c (2026-01-18, the original graph-traversal implementation); present in every release through v1.1.6 and still on main @ 712a406. Not a recent regression.

These 5 issues are really 3 root defects:

• Depth guard returns before visited.add → getCallersRecursive/getCalleesRecursive: depth guard returns before visited.add, causing duplicate results #1086, getImpactRecursive: inconsistent visited vs nodes gate causes incoming edges to be silently dropped #1089
• Dedup gate also gates edge collection (an edge is dropped when its endpoint was already collected) → getImpactRecursive: inconsistent visited vs nodes gate causes incoming edges to be silently dropped #1089, traverseBFS: same target enqueued via two edges; second dequeue skips edge, producing incomplete edge set #1090
• limit checked once per stack frame, not per node added → traverseBFS: inner neighbor loop adds nodes without limit check, overshooting opts.limit #1087, dfsRecursive: nodes.set called before limit check in inner loop, overshooting opts.limit for all siblings #1088

Regression coverage belongs in __tests__/graph.test.ts.

Reproduced against the compiled dist/ build with the real GraphTraverser and a mock QueryBuilder (only getNodeById/getNodesByIds/get{In,Out}goingEdges).

Container P contains method M; Q calls both M and P:

getImpactRadius('P', 3)
// Observed: Q→M edge present, Q→P edge DROPPED, Q in node set

Correction to the fix proposed in the issue body. The suggested remedy (run visited.add before the depth-return, then switch both loops to !visited.has()) does not fix the drop: once Q is marked visited, the incoming-edges loop's !visited.has(Q) is false and Q→P is skipped again — verified in the repro above. The real root cause is narrower and shared with #1090: the recursion-dedup gate also gates edge collection. Correct fix — record the edge unconditionally, gate only node-collection + recursion:

for (const edge of incomingEdges) {
  const sourceNode = sources.get(edge.source);
  if (!sourceNode) continue;
  edges.push(edge);                     // always record the relationship
  if (!nodes.has(sourceNode.id)) {      // gate only node-collection + recursion
    nodes.set(sourceNode.id, sourceNode);
    this.getImpactRecursive(sourceNode.id, maxDepth, currentDepth + 1, nodes, edges, visited);
  }
}

[colbymchenry]

Fixed in 43a6fa6 (#1106) — good catch on the two-gate asymmetry.

getImpactRecursive now (a) marks visited before the depth check, so a node collected at the depth boundary always lands in visited, and (b) records the incoming dependency edge unconditionally instead of gating edge collection on !nodes.has(...) — with both loops unified on visited, as you recommended. So a direct edge into a node already collected via another path is no longer dropped.

In practice this only ever dropped a redundant edge (the source node still appeared in the impact set via the other path), but the edge set is now complete. Regression test added in graph.test.ts. Ships in the next release.