From 91ca8255305a6334ee1cb0e4942d01539bc2cff6 Mon Sep 17 00:00:00 2001 From: Adam Harris Date: Wed, 13 May 2026 12:51:41 -0700 Subject: [PATCH] fix(explain): render arrows in original edge direction MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Same structural bug as #849 (and the same fix shape). `graphify explain` previously iterated `G.neighbors(nid)` on the undirected loaded graph and printed every connection as `--> X [relation]`, ignoring whether the underlying edge actually pointed out of the queried node or into it. For a queried callee, its callers were rendered as outbound calls — asserting a backwards call graph. Confirmed in the wild on a real codebase: `explain validateSanitySession()` printed `--> createPatchHandler() [calls]` even though the stored edge is `createPatchHandler --calls--> validateSanitySession`. Fix mirrors #849: - Force directed interpretation on read (`_raw = {**_raw, "directed": True}`) so the loaded `G` is a `DiGraph` that preserves edge direction. - Iterate successors and predecessors separately, tagging each connection as "out" or "in". - Render `-->` for outbound, `<--` for inbound. A neighbor that has both an inbound AND outbound edge to the queried node (mutual references) is rendered twice — once per edge — which is the truthful directed representation. Tests in `tests/test_explain_cli.py` cover both directions. --- graphify/__main__.py | 32 +++++++++---- tests/test_explain_cli.py | 94 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 117 insertions(+), 9 deletions(-) create mode 100644 tests/test_explain_cli.py diff --git a/graphify/__main__.py b/graphify/__main__.py index 584b02b5e..dfbddd5d1 100644 --- a/graphify/__main__.py +++ b/graphify/__main__.py @@ -1584,6 +1584,11 @@ def main() -> None: _raw = json.loads(gp.read_text(encoding="utf-8")) if "links" not in _raw and "edges" in _raw: _raw = dict(_raw, links=_raw["edges"]) + # Same fix as #849 for `graphify path`: the serialized link.source/link.target + # fields encode caller→callee direction regardless of the top-level "directed" + # flag the build wrote. Force directed interpretation on read so the renderer + # can distinguish outbound (-->) from inbound (<--) connections. + _raw = {**_raw, "directed": True} try: G = json_graph.node_link_graph(_raw, edges="links") except TypeError: @@ -1600,17 +1605,26 @@ def main() -> None: print(f" Type: {d.get('file_type', '')}") print(f" Community: {d.get('community', '')}") print(f" Degree: {G.degree(nid)}") - neighbors = list(G.neighbors(nid)) - if neighbors: - from graphify.build import edge_data - print(f"\nConnections ({len(neighbors)}):") - for nb in sorted(neighbors, key=lambda n: G.degree(n), reverse=True)[:20]: - edata = edge_data(G, nid, nb) + # Collect outbound and inbound edges separately so we render each arrow + # in the actual stored direction. A neighbor that has both an inbound and + # an outbound edge (mutual references) is rendered twice — once per edge — + # which is the truthful representation in a directed graph. + from graphify.build import edge_data + connections: list[tuple[str, str, dict]] = [] # (direction, neighbor_id, edge_data) + for nb in G.successors(nid): + connections.append(("out", nb, edge_data(G, nid, nb))) + for nb in G.predecessors(nid): + connections.append(("in", nb, edge_data(G, nb, nid))) + if connections: + print(f"\nConnections ({len(connections)}):") + connections.sort(key=lambda c: G.degree(c[1]), reverse=True) + for direction, nb, edata in connections[:20]: rel = edata.get("relation", "") conf = edata.get("confidence", "") - print(f" --> {G.nodes[nb].get('label', nb)} [{rel}] [{conf}]") - if len(neighbors) > 20: - print(f" ... and {len(neighbors) - 20} more") + arrow = "-->" if direction == "out" else "<--" + print(f" {arrow} {G.nodes[nb].get('label', nb)} [{rel}] [{conf}]") + if len(connections) > 20: + print(f" ... and {len(connections) - 20} more") elif cmd == "add": if len(sys.argv) < 3: diff --git a/tests/test_explain_cli.py b/tests/test_explain_cli.py new file mode 100644 index 000000000..4408e24da --- /dev/null +++ b/tests/test_explain_cli.py @@ -0,0 +1,94 @@ +"""Tests for `graphify explain` arrow-direction rendering. + +Regression for the structural twin of #849 — the same undirected-graph blind spot +that bit `graphify path` also bit `graphify explain`. Previously `explain` printed +every neighbor as `--> X [relation]`, so for a queried node that is the *callee*, +its callers were rendered as outbound calls, asserting a backwards graph. +""" +from __future__ import annotations + +import json + +import graphify.__main__ as mainmod + + +def _write_undirected_graph(tmp_path): + """Write graph.json with `directed: false` but link.source/link.target + encoding caller→callee in *write order*, matching what `graphify update` + serializes. Writing the JSON directly (rather than via `nx.Graph` + + `node_link_data`) is necessary because the latter emits links in + node-insertion order, not edge-argument order, so a small fixture can + silently encode the wrong direction. + """ + graph_data = { + "directed": False, + "multigraph": False, + "graph": {}, + "nodes": [ + {"id": "validate", "label": "validateSanitySession()", + "source_file": "server/sanity-validate-session.ts", + "source_location": "L9", "community": 0}, + {"id": "create_patch", "label": "createPatchHandler()", + "source_file": "server/create-patch-handler.ts", + "source_location": "L14", "community": 0}, + {"id": "create_edit", "label": "createEditHandler()", + "source_file": "server/create-edit-handler.ts", + "source_location": "L18", "community": 0}, + {"id": "stable_stringify", "label": "stableStringify()", + "source_file": "shared/stringify.ts", + "source_location": "L4", "community": 0}, + ], + "links": [ + # Two callers of validate (edges point INTO validate). + {"source": "create_patch", "target": "validate", + "relation": "calls", "confidence": "EXTRACTED"}, + {"source": "create_edit", "target": "validate", + "relation": "calls", "confidence": "EXTRACTED"}, + # One callee of validate (edge points OUT of validate). + {"source": "validate", "target": "stable_stringify", + "relation": "calls", "confidence": "EXTRACTED"}, + ], + } + graph_path = tmp_path / "graph.json" + graph_path.write_text(json.dumps(graph_data)) + return graph_path + + +def _run_explain(monkeypatch, graph_path, label, capsys): + monkeypatch.setattr(mainmod, "_check_skill_version", lambda _: None) + monkeypatch.setattr( + mainmod.sys, + "argv", + ["graphify", "explain", label, "--graph", str(graph_path)], + ) + mainmod.main() + return capsys.readouterr().out + + +def test_explain_renders_callers_as_inbound_and_callees_as_outbound( + monkeypatch, tmp_path, capsys +): + """A queried callee should show its callers as `<--` and its own callees as `-->`. + Before the fix every neighbor printed as `-->`, regardless of stored direction.""" + graph_path = _write_undirected_graph(tmp_path) + out = _run_explain(monkeypatch, graph_path, "validateSanitySession", capsys) + + # validateSanitySession is called BY createPatchHandler and createEditHandler. + # Those must render as inbound (`<--`), not outbound. + assert "<-- createPatchHandler() [calls]" in out + assert "<-- createEditHandler() [calls]" in out + # validateSanitySession itself calls stableStringify — that is outbound. + assert "--> stableStringify() [calls]" in out + # Critical guard: must NOT render callers as outbound (the pre-fix buggy output). + assert "--> createPatchHandler() [calls]" not in out + assert "--> createEditHandler() [calls]" not in out + + +def test_explain_renders_pure_caller_as_outbound(monkeypatch, tmp_path, capsys): + """Sanity check the converse — querying a caller should show its callee outbound.""" + graph_path = _write_undirected_graph(tmp_path) + out = _run_explain(monkeypatch, graph_path, "createPatchHandler", capsys) + # createPatchHandler calls validateSanitySession. + assert "--> validateSanitySession() [calls]" in out + # And nothing calls createPatchHandler in this fixture, so no inbound rows. + assert "<-- " not in out