Skip to content

fix(build): preserve calls edges over references on undirected collap… - #2400

Open
abhiramArise wants to merge 1 commit into
Graphify-Labs:v8from
abhiramArise:fix-2391-relation-priority-collapse
Open

fix(build): preserve calls edges over references on undirected collap…#2400
abhiramArise wants to merge 1 commit into
Graphify-Labs:v8from
abhiramArise:fix-2391-relation-priority-collapse

Conversation

@abhiramArise

Copy link
Copy Markdown

Fixes #2391.

The sort by (source, target, relation) before G.add_edge was added for deterministic collapse order, but that also meant alphabetical order picked the survivor whenever two different relations landed on the same (src, tgt) pair — references always beat calls since "calls" < "references". G.add_edge just overwrites, no merge.

Added a small _RELATION_PRIORITY table so the higher-information relation wins the collapse instead of whichever sorts last. Unlisted relations default to priority 0, so anything not in the table keeps the old behavior — no regression there.

Left out the multigraph route and the also_relations merge idea from the issue to keep this focused on the actual data-loss bug; can follow up separately if maintainers want either.

Added a test covering both input orderings (since the bug is order-dependent) confirming calls survives a collapse with references either way.

Ran the full test_build.py suite before/after on a clean checkout — the 14 pre-existing failures (missing rapidfuzz/tree-sitter-javascript deps, one unrelated _semantic_id_remap bug) are identical with and without this change.

@graphify-labs graphify-labs Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks safe to merge — no coupling regressions and no blocking issues, checked against the code graph (not a self-assessment).

Graphify reviewed this change.

Looks safe to merge — no coupling regressions and no blocking issues, checked against the code graph (not a self-assessment).


Graphify review — findings

This PR modifies the edge collapse logic in build_from_json (graphify/build.py) for undirected graphs, where multiple edges landing on the same node pair get merged into one. It introduces a _RELATION_PRIORITY table and, when two edges on the same pair carry different relations, keeps the higher-priority relation instead of relying solely on the deterministic sort order. A new test with a helper (_same_pair_calls_references_extraction) verifies that a calls edge survives collapse against a references edge regardless of input order. The remaining changed symbols appear to be test-file rationale/comment blocks and unchanged surrounding tests included in the diff scope.

Worth a look

  • Priority tie between different relations lets last-write clobber unexpectedlygraphify/build.py:1074 · Escalate · medium
    • agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
Analysis details — impact, health, verification

Impact & health

Graphify review

Impact — 798 functions depend on the 196 functions this change touches.

Health — this change adds coupling hotspots:

  • worse: build_from_json() — 143 callers, 14 callees

Verification — 798 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: 597 function(s) in the blast radius were not formally verified this run

· 1 more finding(s) on lines outside this diff (see the check run).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Python: calls edges are deterministically deleted by references on the same node pair (alphabetical last-write-wins in build_from_json)

1 participant