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"