From 2250f205b95f9bb366ab8726ad9b67022621e24a Mon Sep 17 00:00:00 2001 From: Mahaboob Pasha Date: Fri, 9 Oct 2026 12:20:51 +0530 Subject: [PATCH] fix(build): keep a method edge when a member constructs its own type The default graph keeps one undirected edge per node pair. A factory that returns new Report() also has a calls edge back to Report, and that call was the one that survived. Report then had no method edge to Fault, so anything that walks a type's members missed it. A method or contains edge now wins over a calls edge in the opposite direction. A directed graph still keeps both. Fixes #4248 Co-Authored-By: Grok Bot (cherry picked from commit 95546d678427cba2ecbcca494dbc3956c8115c75) --- graphify/build.py | 35 +++++++++++-- tests/test_csharp_object_creation.py | 33 +++++++++++++ tests/test_relation_collapse_precedence.py | 57 +++++++++++++++++++++- 3 files changed, 118 insertions(+), 7 deletions(-) diff --git a/graphify/build.py b/graphify/build.py index 4d8c76c59..bffcbb7b7 100644 --- a/graphify/build.py +++ b/graphify/build.py @@ -73,10 +73,21 @@ def _backfill_origin(item: dict) -> None: # 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. +# every relation: ranking `contains` against `calls` along the SAME direction would +# be inventing a cross-axis judgement, whereas "specific beats generic" is the only +# same-direction comparison this collapse needs. The one opposite-direction +# exception is _STRUCTURAL_MEMBER_RELATIONS. _GENERIC_RELATIONS: frozenset[str] = frozenset({"references", "uses", "mentions"}) +# Membership of a type or file. On an undirected graph these share a node pair +# with a `calls` edge in the opposite direction when a member calls or constructs +# its owner (`static Report Fault() { return new Report(); }`). Last-write kept +# `calls` whenever the member id sorted after the owner id, so the membership +# edge never reached graph.json and the member disappeared from type walks +# (member lookup, explain, call resolution into the type). The membership edge +# wins; the call is what this collapse drops. Directed graphs keep both, because +# the two directions are different arcs. Same-direction `contains`/`method` vs +# `calls` is not ranked here. +_STRUCTURAL_MEMBER_RELATIONS: frozenset[str] = frozenset({"method", "contains"}) _CONFIDENCE_RANK: dict[str, int] = {"EXTRACTED": 3, "INFERRED": 2, "AMBIGUOUS": 1} # Import-family relations whose target may legitimately be a module OUTSIDE the @@ -1557,8 +1568,22 @@ def build_from_json(extraction: dict, *, directed: bool = False, root: str | Pat # reverse-direction duplicate so the original direction is preserved (#1061). if not G.is_directed() and G.has_edge(src, tgt): existing = edge_data(G, src, tgt) - if existing.get("relation") == attrs.get("relation") and ( - existing.get("_src") == tgt and existing.get("_tgt") == src + existing_rel = existing.get("relation") + incoming_rel = attrs.get("relation") + reverse = existing.get("_src") == tgt and existing.get("_tgt") == src + if reverse and existing_rel == incoming_rel: + continue + # Different relations in opposite directions are two facts that only + # share a pair because the graph is undirected. A type's + # `method`/`contains` edge and its member's `calls` edge back are + # that case: last-write kept whichever source id sorted later, and + # when the call won the member vanished from every walk of the + # type. The membership edge wins. If the call is already stored, + # the membership edge falls through and replaces it below. + if ( + reverse + and existing_rel in _STRUCTURAL_MEMBER_RELATIONS + and incoming_rel == "calls" ): continue # A pair that already carries a SPECIFIC relation must not be downgraded diff --git a/tests/test_csharp_object_creation.py b/tests/test_csharp_object_creation.py index af33c6c34..dca55c9af 100644 --- a/tests/test_csharp_object_creation.py +++ b/tests/test_csharp_object_creation.py @@ -18,6 +18,7 @@ import os import tempfile from pathlib import Path +from graphify.build import build_from_json, edge_data from graphify.extract import extract @@ -238,6 +239,38 @@ def test_qualified_construction_resolves_when_the_method_also_declares_the_type( assert not any(target in sourceless for _, target in calls) +def test_factory_constructing_its_own_type_keeps_the_method_edge(tmp_path): + # `Fault` both belongs to `Report` and constructs it. On the default + # undirected graph those are one pair, and the `calls` edge used to be + # the one that survived, so `Report` had no `method` edge to `Fault`. + # The siblings that do not call the type were never affected. + _, r = _extract(tmp_path, {"Report.cs": ( + "namespace Demo\n" + "{\n" + " public sealed class Report\n" + " {\n" + " public string Verb;\n" + " public static Report Fault(string verb) { return new Report { Verb = verb }; }\n" + " public static string NotImplementedJson(string verb) { return verb; }\n" + " public string ToJson() { return Verb; }\n" + " }\n" + "}\n" + )}) + report = _find(r, "Report", "report") + fault = _find(r, ".Fault()", "fault") + G = build_from_json(r) + kept = edge_data(G, report, fault) + assert kept.get("relation") == "method" + assert (kept.get("_src"), kept.get("_tgt")) == (report, fault) + assert edge_data(G, report, _find(r, ".NotImplementedJson()", "notimplemented")).get( + "relation") == "method" + assert edge_data(G, report, _find(r, ".ToJson()", "tojson")).get("relation") == "method" + # Directed storage has room for both facts. + D = build_from_json(r, directed=True) + assert edge_data(D, report, fault).get("relation") == "method" + assert edge_data(D, fault, report).get("relation") == "calls" + + def test_qualified_construction_of_a_foreign_type_makes_no_stub_edge(tmp_path): # `new System.Text.StringBuilder()` has no declaration in the corpus. No edge # beats an edge into a sourceless placeholder that stands for nothing. diff --git a/tests/test_relation_collapse_precedence.py b/tests/test_relation_collapse_precedence.py index c6ff432fa..9ffa231ac 100644 --- a/tests/test_relation_collapse_precedence.py +++ b/tests/test_relation_collapse_precedence.py @@ -18,6 +18,11 @@ sampling — and callflow's relation filter 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. + +One opposite-direction exception: a `method` or `contains` edge and a `calls` +edge running back the other way share one undirected pair (a member that +constructs or calls its owner). The membership edge survives. Same-direction +`contains` vs `calls` stays unranked. A directed graph keeps both arcs. """ import pytest @@ -99,8 +104,10 @@ def test_two_generic_relations_keep_previous_behaviour(): 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.""" + """Same direction is deliberately NOT ranked — `contains` vs `calls` along + one direction is a cross-axis judgement this collapse does not make. + Opposite directions are a different pair of facts; see + test_structural_membership_beats_an_opposite_call.""" G = build_from_json(_extraction([_edge("calls"), _edge("contains")])) assert _relation(G) in {"calls", "contains"} @@ -184,6 +191,52 @@ def test_unknown_relation_is_treated_as_specific(): assert _relation(G) == "custom_rel", f"order {[e['relation'] for e in order]}" +@pytest.mark.parametrize("structural", ["method", "contains"]) +@pytest.mark.parametrize("structural_src, structural_tgt", [("a", "b"), ("b", "a")]) +def test_structural_membership_beats_an_opposite_call(structural, structural_src, structural_tgt): + """Type -method/contains-> member and member -calls-> type share one + undirected pair. Whichever endpoint id sorts later used to win, so a + factory's constructor call erased the membership edge and the member + dropped out of every type walk. The membership edge survives, with its + own direction and location, in either id order. Still exactly one edge.""" + call_src, call_tgt = structural_tgt, structural_src + G = build_from_json(_extraction([ + _edge(structural, src=structural_src, tgt=structural_tgt, source_location="L4"), + _edge("calls", src=call_src, tgt=call_tgt, source_location="L5"), + ])) + d = edge_data(G, "a", "b") + assert d.get("relation") == structural + assert (d.get("_src"), d.get("_tgt")) == (structural_src, structural_tgt) + assert d.get("source_location") == "L4" + assert G.number_of_edges() == 1 + + +def test_directed_graph_keeps_method_and_the_reverse_call(): + """Opposite directions are different arcs once the graph is directed, so + both facts survive. The undirected rule must not drop the call here.""" + G = build_from_json(_extraction([ + _edge("method", src="a", tgt="b", source_location="L4"), + _edge("calls", src="b", tgt="a", source_location="L5"), + ]), directed=True) + assert G.is_directed() + assert G.number_of_edges() == 2 + assert edge_data(G, "a", "b").get("relation") == "method" + assert (edge_data(G, "a", "b").get("_src"), edge_data(G, "a", "b").get("_tgt")) == ("a", "b") + assert edge_data(G, "b", "a").get("relation") == "calls" + assert edge_data(G, "b", "a").get("source_location") == "L5" + + +def test_same_direction_method_and_calls_stay_unranked(): + """The membership-vs-call rule applies only when the edges run in opposite + directions. Along one direction the pair is still a single unranked edge.""" + G = build_from_json(_extraction([ + _edge("method", src="a", tgt="b"), + _edge("calls", src="a", tgt="b"), + ])) + assert G.number_of_edges() == 1 + assert _relation(G) in {"method", "calls"} + + def test_same_relation_collision_preserves_extracted_over_inferred(): """An EXTRACTED AST edge must not be downgraded by a later INFERRED edge.""" for order in (