fix(build): keep the specific relation when an edge pair collapses (#2803) - #2804
fix(build): keep the specific relation when an edge pair collapses (#2803)#2804abhay-codes07 wants to merge 1 commit into
Conversation
build_from_json puts one edge per node pair into a simple graph and resolved same-pair collisions by "last write wins" over a sort keyed on (source, target, relation). That sort exists for determinism -- an unstable order flipped _src/_tgt run to run (Graphify-Labs#1061) -- but it also decided which RELATION survived, and it decided it alphabetically: calls < contains < imports < ... < references < uses So `references` always overwrote `calls`, and `uses` overwrote everything. This is not a tie-break, it is a systematic downgrade: on graphify's own corpus all 144 pairs where the extraction found both `calls` and `references` came out as plain `references` -- 144 of 144, not a sample. It is not cosmetic either. callflow filters on relation and its list is (calls, imports, imports_from, uses, method, indirect_call), which excludes `references`, so every one of those call sites left the call graph. Anything else asking "what calls X" -- query, path, the audit trail -- read the weaker fact too. A generic relation now never overwrites a specific one on the same pair. The reverse still overwrites, so the surviving relation no longer depends on arrival order in either direction, and the graph is unchanged in every other respect: same node count, same edge count, same _src/_tgt direction handling. _GENERIC_RELATIONS is deliberately a small denylist rather than a full precedence order over every relation. Ranking `contains` against `calls` would be inventing a cross-axis judgement; "specific beats generic" is the only comparison this collapse actually needs, and two specifics keep their previous behaviour. Measured on graphify's own 2287-node graph: calls edges 1914 -> 2060 references edges 548 -> 405 edges visible to callflow 2788 -> 2931 pairs reporting a weaker relation 146 -> 0 total edges 5396 -> 5396 (unchanged)
There was a problem hiding this comment.
Graphify reviewed this change.
Looks safe to merge — no coupling regressions and no blocking issues, checked against the code graph (not a self-assessment).
Formal verification. No changes could be formally verified in this run.
Graphify review — findings
Fixes edge collapse in build_from_json so a generic relation (references, uses, mentions) never overwrites a specific one on the same node pair — previously the deterministic (source, target, relation) sort picked the survivor alphabetically, silently rewriting calls to references and dropping those pairs out of callflow's relation filter. Adds a _GENERIC_RELATIONS denylist and a same-pair guard that skips a generic edge when a specific one already exists (specific-arrives-later still overwrites, so order no longer matters). Adds tests/test_relation_collapse_precedence.py pinning the demotion behavior in both arrival orders, the one-edge-per-pair invariant, direction metadata, and the directed-graph case.
No blocking issues surfaced. 1 lower-confidence candidate did not survive cross-model review.
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 1014 functions depend on the 106 functions this change touches.
Health — this change adds coupling hotspots:
- new:
_rebuild_code()— 95 callers, 51 callees - new:
build_from_json()— 164 callers, 18 callees - new:
build_merge()— 41 callers, 14 callees - new:
to_obsidian()— 29 callers, 12 callees - new:
extract_files_direct()— 17 callers, 20 callees - new:
to_wiki()— 41 callers, 7 callees - new:
_call_claude_cli()— 31 callers, 9 callees - new:
extract_corpus_parallel()— 26 callers, 10 callees - …and 37 more — each is listed as a finding
Verification — 1014 functions in the blast radius were not formally verified this run (proofs are advisory here).
Gate & verification
graphify gate
PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.
Advisory (not blocking):
- verification_scope: 658 function(s) in the blast radius were not formally verified this run
Formal verification
Could not verify: Could not verify build\_from\_json.
The verifier did not have enough to check build\_from\_json, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: not verifiable: all 6 sampled inputs raised on both versions — the function never executed, so 'no divergence' would be vacuous (mostly NameError — names the real obstacle, not a sampling gap)
· 45 more finding(s) on lines outside this diff (see the check run).
Fixes #2803.
The bug
build_from_jsonkeeps one edge per node pair and resolves collisions by "last write wins" over a sort keyed on(source, target, relation). That sort is there for determinism — an unstable order flipped_src/_tgtbetween runs (#1061) — but the third key element also decides which relation survives, and it decides it alphabetically:So
referencesalways overwritescalls, andusesoverwrites everything. That is not a tie-break, it is a systematic downgrade. On graphify's own package every single pair where the extraction found both came out as the weaker fact:144 of 144 — not a sample. Swapping the two edges in the input changes nothing, because the sort reorders them either way.
Why it is not cosmetic
callflow_html.pyselects edges by relation:referencesis not in that set, so every downgraded pair drops out of the call graph entirely. The audit trail also states the weaker fact for an edge the extractor taggedEXTRACTED/calls, andquery/path/explainall read the surviving relation when answering "what calls X".Every extractor that records a call tends to record a reference for the same pair, so this fires on ordinary code rather than an unusual shape. The 144/144 rate is the tell.
The change
A generic relation never overwrites a specific one on the same pair. The reverse still overwrites, so the surviving relation no longer depends on arrival order in either direction.
I deliberately did not introduce a full precedence order over every relation. Ranking
containsagainstcallswould be inventing a cross-axis judgement that this collapse does not need — the only comparison required is "specific beats generic". Two specific relations keep their existing behaviour, and there is a test pinning that so the denylist cannot quietly grow into an ordering.This is also not a fix for #2791. That issue is about edges being dropped by the simple-graph model, which is a much bigger change; this only corrects which of the colliding edges survives. Node and edge counts are byte-identical before and after.
Measured on graphify's own graph
81 files, 2287 nodes, 6176 extracted edges:
callsedgesreferencesedgesTests
tests/test_relation_collapse_precedence.py(65 tests). The core is a matrix over every specific relation × every generic relation × both arrival orders, since the point of the fix is that order stops mattering.Beyond that it pins what must not change: two generics keep previous behaviour, two specifics keep previous behaviour, the #1061 reverse-direction guard still holds, a lone generic edge is still kept, exactly one edge still survives per pair, and the surviving edge keeps the specific edge's own
_src/_tgtandsource_locationrather than inheriting the demoted one's.Reverting
build.pyand keeping the tests fails 55 of the 65. These are pure graph-shape assertions with no filesystem or platform dependency, so they have the same teeth on your Ubuntu CI as locally.Validation
Windows 11, Python 3.12, branched off
4fca621(0.9.44).20 failed, 4469 passed->20 failed, 4534 passed. Identical failure set — no regressions; the +65 are the new tests. The 20 are pre-existing Windows failures (symlink privileges, FIFO/socket fixtures, and similar), unrelated to this change.