fix(build): keep the specific relation when an edge pair collapses (#2803)

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:

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"