From 975d8a1d634967f6315e2abeefda5bc98c455b8f Mon Sep 17 00:00:00 2001 From: abhay-codes07 Date: Mon, 17 Aug 2026 00:40:39 +0530 Subject: [PATCH] fix(build): keep the specific relation when an edge pair collapses 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 (#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) --- graphify/build.py | 29 ++++ tests/test_relation_collapse_precedence.py | 159 +++++++++++++++++++++ 2 files changed, 188 insertions(+) create mode 100644 tests/test_relation_collapse_precedence.py diff --git a/graphify/build.py b/graphify/build.py index 7f195fe2f..809f78bc8 100644 --- a/graphify/build.py +++ b/graphify/build.py @@ -52,6 +52,16 @@ def _is_ast_tier(item: dict) -> bool: return isinstance(loc, str) and bool(_AST_LOC_RE.match(loc)) +# Relations that say only "these two symbols appear together", with no claim about +# HOW. An extractor that finds a specific fact for a pair — a call, an import, an +# inheritance — routinely emits one of these for the same pair as well, so when the +# simple graph collapses the pair to one edge, the generic one must never be the +# survivor. Deliberately a small denylist rather than a full precedence order over +# every relation: ranking `contains` against `calls` would be inventing a +# cross-axis judgement, whereas "specific beats generic" is the only comparison +# this collapse actually needs. +_GENERIC_RELATIONS: frozenset[str] = frozenset({"references", "uses", "mentions"}) + # Language interop families, keyed by extension, for the cross-language phantom-edge # guard in the edge loop below. Families group by REAL interop (JS/TS share a module # graph; C/C++/ObjC share a compilation unit via headers; JVM langs share bytecode), @@ -1230,6 +1240,25 @@ def build_from_json(extraction: dict, *, directed: bool = False, root: str | Pat existing.get("_src") == tgt and existing.get("_tgt") == src ): continue + # A pair that already carries a SPECIFIC relation must not be downgraded + # to a generic one. Only one edge survives per pair here, and the sort + # above orders same-pair edges by relation name, so "last write wins" + # resolved the winner alphabetically — which put `references` after + # `calls` and `uses` after everything. On graphify's own corpus that + # rewrote all 144 pairs where the extraction found both `calls` and + # `references` into plain `references`, and callflow's relation filter + # does not include `references`, so those call sites left the call graph + # entirely. Alphabetical order carries no meaning; keeping the specific + # fact does. The reverse (specific arriving after generic) still + # overwrites, so the outcome no longer depends on edge order at all. + if G.has_edge(src, tgt): + existing_rel = edge_data(G, src, tgt).get("relation") + if ( + attrs.get("relation") in _GENERIC_RELATIONS + and existing_rel is not None + and existing_rel not in _GENERIC_RELATIONS + ): + continue G.add_edge(src, tgt, **attrs) hyperedges = extraction.get("hyperedges", []) if hyperedges: diff --git a/tests/test_relation_collapse_precedence.py b/tests/test_relation_collapse_precedence.py new file mode 100644 index 000000000..28c214468 --- /dev/null +++ b/tests/test_relation_collapse_precedence.py @@ -0,0 +1,159 @@ +"""A collapsed edge must keep the specific relation, not the alphabetical one. + +`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 (#1061 — an +unstable order flipped `_src`/`_tgt` run to run), 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. On +graphify's own corpus that rewrote all 144 pairs where the extraction found both +`calls` and `references` into plain `references` — 144 out of 144, not a +sampling — and callflow's relation filter +(`calls, imports, imports_from, uses, method, indirect_call`) does not include +`references`, so those call sites dropped out of the call graph entirely. + +Alphabetical order carries no meaning. These tests pin that a generic relation +never overwrites a specific one, in either arrival order, and that nothing else +about the collapse changed. +""" +import pytest + +from graphify.build import build_from_json, edge_data + +SPECIFIC = ["calls", "imports", "imports_from", "inherits", "implements", + "method", "indirect_call", "re_exports", "contains"] +GENERIC = ["references", "uses", "mentions"] + + +def _extraction(edges): + return { + "nodes": [ + {"id": "a", "label": "a()", "file_type": "code", "source_file": "a.py"}, + {"id": "b", "label": "b()", "file_type": "code", "source_file": "b.py"}, + ], + "edges": edges, + "hyperedges": [], + } + + +def _edge(rel, src="a", tgt="b", **kw): + return {"source": src, "target": tgt, "relation": rel, + "confidence": "EXTRACTED", **kw} + + +def _relation(G): + return edge_data(G, "a", "b").get("relation") + + +# --------------------------------------------------------------------------- +# The bug +# --------------------------------------------------------------------------- + +@pytest.mark.parametrize("generic", GENERIC) +@pytest.mark.parametrize("specific", SPECIFIC) +@pytest.mark.parametrize("order", ["specific_first", "generic_first"]) +def test_generic_never_overwrites_specific(specific, generic, order): + pair = [_edge(specific), _edge(generic)] + if order == "generic_first": + pair.reverse() + G = build_from_json(_extraction(pair)) + assert _relation(G) == specific, ( + f"{generic!r} overwrote {specific!r} when added {order.replace('_', ' ')}") + + +def test_the_reported_case_keeps_calls(): + """The exact shape seen 144 times on graphify's own graph.""" + G = build_from_json(_extraction([ + _edge("calls", source_location="L10"), + _edge("references", source_location="L10"), + ])) + assert _relation(G) == "calls" + + +def test_calls_survives_for_callflow(monkeypatch): + """The consequence that made this worth fixing: callflow filters on relation + and does not list `references`, so a downgraded pair leaves the call graph.""" + callflow_relations = ("calls", "imports", "imports_from", "uses", "method", + "indirect_call") + G = build_from_json(_extraction([_edge("calls"), _edge("references")])) + assert _relation(G) in callflow_relations + + +# --------------------------------------------------------------------------- +# Everything else about the collapse is unchanged +# --------------------------------------------------------------------------- + +def test_specific_still_overwrites_generic(): + """The fix is one-directional: a specific relation arriving later still wins, + so the outcome no longer depends on arrival order in either direction.""" + G = build_from_json(_extraction([_edge("references"), _edge("calls")])) + assert _relation(G) == "calls" + + +def test_two_generic_relations_keep_previous_behaviour(): + G = build_from_json(_extraction([_edge("references"), _edge("uses")])) + assert _relation(G) in {"references", "uses"} + + +def test_two_specific_relations_keep_previous_behaviour(): + """Deliberately NOT ranked against each other — `contains` vs `calls` is a + cross-axis judgement this collapse does not need to make.""" + G = build_from_json(_extraction([_edge("calls"), _edge("contains")])) + assert _relation(G) in {"calls", "contains"} + + +def test_collapse_still_yields_exactly_one_edge(): + G = build_from_json(_extraction([ + _edge("calls"), _edge("references"), _edge("uses"), _edge("calls"), + ])) + assert G.number_of_edges() == 1 + assert G.number_of_nodes() == 2 + + +def test_edge_count_is_unchanged_by_the_fix(): + """The fix chooses WHICH edge survives; it must not add or drop any.""" + edges = [_edge("calls"), _edge("references"), + _edge("imports", src="b", tgt="a"), _edge("uses", src="b", tgt="a")] + G = build_from_json(_extraction(edges)) + assert G.number_of_edges() == 1 + + +def test_reverse_direction_guard_still_holds(): + """#1061: same relation, opposite directions — first-seen direction wins.""" + G = build_from_json(_extraction([ + _edge("calls", src="a", tgt="b"), + _edge("calls", src="b", tgt="a"), + ])) + d = edge_data(G, "a", "b") + assert (d.get("_src"), d.get("_tgt")) == ("a", "b") + + +def test_a_lone_generic_edge_is_kept(): + """Generic relations are only ever demoted against a specific one on the SAME + pair — a pair that has nothing else must keep its edge.""" + G = build_from_json(_extraction([_edge("references")])) + assert _relation(G) == "references" + assert G.number_of_edges() == 1 + + +def test_direction_metadata_survives_the_demotion(): + """The surviving edge must still carry the specific edge's own direction, not + the demoted one's.""" + G = build_from_json(_extraction([ + _edge("calls", src="a", tgt="b", source_location="L1"), + _edge("references", src="b", tgt="a", source_location="L2"), + ])) + d = edge_data(G, "a", "b") + assert d.get("relation") == "calls" + assert (d.get("_src"), d.get("_tgt")) == ("a", "b") + assert d.get("source_location") == "L1" + + +def test_directed_graphs_get_the_same_protection(): + G = build_from_json(_extraction([_edge("calls"), _edge("references")]), + directed=True) + assert G.is_directed() + assert edge_data(G, "a", "b").get("relation") == "calls"