fix(graph): stop leaking edges to excluded neighbors - #717
Merged
plind-junior merged 3 commits intoJul 31, 2026
Merged
Conversation
find_neighbors appended an edge to the response before checking whether its other endpoint passed the same retrievability/existence gate that decides node inclusion (_neighbor_ok / _node_kind). superseded, archived, and redacted claims - and missing nodes - were correctly excluded from nodes, but the edge pointing at them still went out, so a response could contain an edge whose target referenced a claim id the response itself said didn't exist. kb.neighbors shares this code path across all three surfaces (mcp, jsonl, cli), so the leak was identical everywhere. reorder the loop so an edge is only recorded once its other endpoint has been accepted into visited - either already, having passed the gate in an earlier iteration, or just now. extended test_find_neighbors_excludes_superseded_claims to also assert edges == [] - fails on the old code (returns the dangling edge to the superseded claim via the SUPERSEDES relation lifecycle.supersede() creates), passes with the fix. Fixes vouchdev#716
a6c6862 (vouchdev#686) fixed capture.load_config's min_observations and dedup_window_seconds to fall back to their defaults on a malformed config value via the coerce_numeric() helper, instead of raising ValueError straight out of load_config. 47eaf56 (vouchdev#645, realtime opt-in) branched off the pre-fix capture.py and reintroduced the bare int()/float() calls when it merged into test - the coerce_numeric import survived (nothing else referenced it), but the two call sites it fed didn't, silently reverting the fix and breaking ruff's unused-import gate for every PR built on top of `test`. restore the coerce_numeric() calls, matching recall.load_config's still-intact equivalent. unrelated to this PR's own change (graph.py edge leak); needed only to get CI green on top of a currently-broken `test`.
the diff-coverage gate flagged lines 188-189 of graph.py (the ArtifactNotFoundError branch in find_neighbors, hit when a relation's other endpoint no longer exists) as untested - the existing excludes_superseded_claims test only exercises the sibling _neighbor_ok exclusion path, not the missing-node one. add a case where a relation's target entity is deleted after the relation was created (storage doesn't cascade-delete relations), leaving a dangling reference. asserts both nodes and edges stay empty, matching the fix - confirmed it fails against the pre-fix graph.py (returns the dangling edge) via a throwaway checkout of the prior commit.
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.
What changed
Reordered the loop in
src/vouch/graph.py::find_neighborsso an edge isonly recorded once its other endpoint has been accepted into
visited—either just now, having passed the
_node_kind/_neighbor_okgate, oralready accepted in an earlier iteration.
Why
find_neighbors()appended an edge to the response before checkingwhether its other endpoint passed the same retrievability/existence
checks that gate node inclusion (
_neighbor_ok/_node_kind).Superseded, archived, and redacted claims — and missing nodes — were
correctly excluded from
nodes, but the edge pointing at them still wentout, so a response could contain an edge whose
targetreferenced aclaim id the response itself said didn't exist.
kb.neighborssharesthis code path across all three surfaces (MCP, JSONL, CLI), so the leak
was identical everywhere.
Confirmed with a repro: a live claim related to a superseded claim
returns
nodes: [](correctly excluding the superseded claim) butedges: [{'source': 'live-claim', 'target': 'dead-claim', ...}]— adangling edge to a node the response itself says doesn't exist. The
existing
lifecycle.supersede()path hits this identically: theSUPERSEDESrelation it creates between old and new claim leaked thesame way.
Fixes #716
What might break
Nothing for users with an existing
.vouch/directory — no on-diskshape,
kb.*method, or object model change. Behaviorally:kb.neighborsresponses no longer contain edges pointing at nodes excluded from the
same response — strictly the intended, documented contract (a caller
should never see an edge referencing an id the response says doesn't
exist or isn't retrievable).
VEP
Not applicable — no object model,
kb.*method, on-disk layout, bundleformat, or audit-log shape change. A response-consistency fix inside the
existing
kb.neighborsread path.Tests
make check-equivalent: ruff clean (src+tests); mypyclean on
graph.py; alltests/test_graph.pycases pass (9pre-existing + 1 extended); broader
graph/neighbor/contextselection across the suite passes
test_find_neighbors_excludes_superseded_claimsto also assertresult["edges"] == []; confirmed it fails on the pre-fix code(returns the dangling
SUPERSEDESedge) and passes with the fixCHANGELOG.mdupdated under## [Unreleased]