mirror of
https://github.com/safishamsi/graphify.git
synced 2026-08-27 00:36:39 +00:00
fix(build): keep the specific relation when an edge pair collapses (#2803)
When two edges connect the same (source, target) pair with different relations, the simple-graph collapse was last-write-wins by alphabetical relation, so a specific `calls` was systematically overwritten by a generic `references`/ `uses` — and `references` is not in the callflow filter, so those call sites dropped out of the call graph. A generic relation can no longer clobber a stored non-generic one; the outcome is now order-independent and the edge count is unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
committed by
safishamsi
co-authored by
Claude Opus 4.8
parent
0738af373a
commit
a359b7f39b
@@ -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:
|
||||
|
||||
@@ -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"
|
||||
Reference in New Issue
Block a user