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