fix(graph): complete edge sets & correct node limits in traversal (#1086–#1090)#1106
Merged
Merged
Conversation
, #1087, #1088, #1089, #1090) Five coordinated reports (thanks @inth3shadows) against src/graph/traversal.ts, three root defects: - Depth guard returned before visited.add. At the depth boundary a node reached from the same parent via two edges was pushed once per edge — duplicating callers/callees at the default maxDepth=1 (#1086), and leaving getImpactRecursive's two loops disagreeing about whether a node was already processed (#1089). - The dedup gate also gated edge collection. traverseBFS enqueued a target twice and dropped the second edge (#1090); getImpactRecursive dropped a real incoming dependency edge into an already-collected node (#1089). - limit was checked once per stack frame, not per node added, so one high-degree node overshot opts.limit in both traverseBFS (#1087) and dfsRecursive (#1088). traverseBFS now collects every distinct edge among kept nodes on the adjacency scan (deduped on edge identity), enqueues each node exactly once, and caps per-add. getCallers/getCallees/getImpactRecursive mark visited before the depth check; getImpactRecursive records the incoming dependency edge unconditionally and unifies its two loops on `visited`. 7 regression tests in graph.test.ts drive GraphTraverser against an in-memory graph; each fails on the pre-fix code. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This was referenced Jul 1, 2026
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes the coordinated batch of five traversal reports from @inth3shadows (#1086, #1087, #1088, #1089, #1090) — three root defects, all in
src/graph/traversal.ts. Each was mechanically real; most were masked at the integration layer (everygetCallers/getCalleesconsumer already dedups by node id; impact only ever dropped a redundant edge), so day-to-day query output was largely unaffected — but the underlying traversal was returning incomplete / over-budget sets, and the one unmasked leak was a small centrality-count skew in explore ranking. Cheap, low-risk, removes the latent landmines.Root defects → fixes
1. Depth guard returned before
visited.add(#1086, part of #1089)if (currentDepth >= maxDepth || visited.has(nodeId)) return;skipped thevisited.addwhen the depth arm fired. A caller/callee reached from the same parent via two edges (two call sites, orcalls+references) was then pushed once per edge — a duplicate at the defaultmaxDepth=1. Fixed ingetCallersRecursive,getCalleesRecursive,getImpactRecursiveby marking visited before the depth check.2. Dedup gate also gated edge collection (#1090, #1089)
traverseBFSguarded the enqueue onvisited(only set on dequeue), so a target reachable via two edges was enqueued twice and the second dequeue dropped its edge. It now collects every distinct edge among kept nodes on the adjacency scan (deduped on edge identity, so adirection:'both'scan doesn't double-count), and uses a separateenqueuedset so each node is queued exactly once.getImpactRecursivegated edge push on!nodes.has(source), silently dropping a direct incoming dependency edge into a node already collected via another path. It now records the dependency edge unconditionally and unifies both loops onvisited.3.
limitchecked per-frame, not per-add (#1087, #1088)A single high-degree node inserted its whole fan-out before the next frame's guard saw the size, overshooting
opts.limit. BothtraverseBFSanddfsRecursivenow check the cap per node added.Tests
7 regression tests in
__tests__/graph.test.tsdriveGraphTraverseragainst an in-memory graph (the reporter's own repro approach) — one per symptom. Each fails on the pre-fix code and passes with the fix (verified by stashing the fix and re-running).Full suite green (the lone
#662daemon timeout is the documented pre-existing flake under parallel load; passes in isolation).Closes #1086, closes #1087, closes #1088, closes #1089, closes #1090.
🤖 Generated with Claude Code