From 4134e78f828606d8a36149f70f8f2f83df7dcb8e Mon Sep 17 00:00:00 2001 From: Adam Harris Date: Wed, 13 May 2026 10:04:11 -0700 Subject: [PATCH] fix(path): render arrows in original edge direction (#849) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `graphify path` previously printed every hop as `u --rel--> v` based on traversal order from the undirected `shortest_path` walk, ignoring the actual stored direction of the edge between u and v. When the user queried `path B A` and the stored edge was A→B, the output asserted a backwards call graph. The serialized link.source/link.target fields already encode the caller→callee direction regardless of the top-level "directed" flag the build wrote. Force directed interpretation on read so the renderer can recover and display the correct arrow. - Traverse on `G.to_undirected(as_view=True)` so a path is still found regardless of which direction the user supplied src/tgt in. - For each hop, check `G.has_edge(u, v)` against the directed graph; render `--rel-->` if the stored edge matches traversal direction, `<--rel--` if it points the other way. Distinct from #760 (extractor) — verified the extractor emits correct caller→callee directions on the reproducer graph; this is solely a build/render-layer bug. Tests in tests/test_path_cli.py cover both directions. --- graphify/__main__.py | 22 ++++++++-- tests/test_path_cli.py | 92 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 111 insertions(+), 3 deletions(-) create mode 100644 tests/test_path_cli.py diff --git a/graphify/__main__.py b/graphify/__main__.py index 584b02b5e..9b64f6a12 100644 --- a/graphify/__main__.py +++ b/graphify/__main__.py @@ -1514,6 +1514,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"]) + # #849: the serialized link.source/link.target fields encode the original + # caller→callee (or import→importee) direction regardless of the top-level + # "directed" flag the build wrote. Force directed interpretation on read so + # the renderer can recover and display the correct arrow direction below. + _raw = {**_raw, "directed": True} try: G = json_graph.node_link_graph(_raw, edges="links") except TypeError: @@ -1547,7 +1552,9 @@ def main() -> None: file=sys.stderr, ) try: - path_nodes = _nx.shortest_path(G, src_nid, tgt_nid) + # Traverse on the undirected view so a path can be found regardless of + # which direction the user supplied src/tgt in. + path_nodes = _nx.shortest_path(G.to_undirected(as_view=True), src_nid, tgt_nid) except (_nx.NetworkXNoPath, _nx.NodeNotFound): print(f"No path found between '{source_label}' and '{target_label}'.") sys.exit(0) @@ -1556,13 +1563,22 @@ def main() -> None: from graphify.build import edge_data for i in range(len(path_nodes) - 1): u, v = path_nodes[i], path_nodes[i + 1] - edata = edge_data(G, u, v) + # #849: render the arrow in the original stored edge direction. + if G.has_edge(u, v): + edata = edge_data(G, u, v) + forward = True + else: + edata = edge_data(G, v, u) + forward = False rel = edata.get("relation", "") conf = edata.get("confidence", "") conf_str = f" [{conf}]" if conf else "" if i == 0: segments.append(G.nodes[u].get("label", u)) - segments.append(f"--{rel}{conf_str}--> {G.nodes[v].get('label', v)}") + if forward: + segments.append(f"--{rel}{conf_str}--> {G.nodes[v].get('label', v)}") + else: + segments.append(f"<--{rel}{conf_str}-- {G.nodes[v].get('label', v)}") print(f"Shortest path ({hops} hops):\n " + " ".join(segments)) elif cmd == "explain": diff --git a/tests/test_path_cli.py b/tests/test_path_cli.py new file mode 100644 index 000000000..f59dee52a --- /dev/null +++ b/tests/test_path_cli.py @@ -0,0 +1,92 @@ +"""Tests for `graphify path` arrow-direction rendering (regression for #849).""" +from __future__ import annotations + +import json + +import networkx as nx +from networkx.readwrite import json_graph + +import graphify.__main__ as mainmod + + +def _write_undirected_graph(tmp_path): + """Write a graph.json with `directed: false` (the build's default) but link + source/target encoding caller→callee. This mirrors the on-disk shape that + ships from `graphify update`.""" + G = nx.Graph() + G.add_node( + "create_patch_handler", + label="createPatchHandler()", + source_file="server/create-patch-handler.ts", + source_location="L14", + community=0, + ) + G.add_node( + "validate_sanity_session", + label="validateSanitySession()", + source_file="server/sanity-validate-session.ts", + source_location="L9", + community=0, + ) + # The link is serialized as source=createPatchHandler, target=validateSanitySession + # because nx.Graph.add_edge(u, v, ...) places u in `source` and v in `target` + # when round-tripped through node_link_data. + G.add_edge( + "create_patch_handler", + "validate_sanity_session", + relation="calls", + confidence="EXTRACTED", + context="call", + ) + graph_path = tmp_path / "graph.json" + graph_path.write_text(json.dumps(json_graph.node_link_data(G, edges="links"))) + # Sanity-check the on-disk shape matches the production build (#849 reproducer). + on_disk = json.loads(graph_path.read_text()) + assert on_disk["directed"] is False + assert on_disk["links"][0]["source"] == "create_patch_handler" + assert on_disk["links"][0]["target"] == "validate_sanity_session" + return graph_path + + +def _run_path(monkeypatch, graph_path, source_label, target_label, capsys): + monkeypatch.setattr(mainmod, "_check_skill_version", lambda _: None) + monkeypatch.setattr( + mainmod.sys, + "argv", + ["graphify", "path", source_label, target_label, "--graph", str(graph_path)], + ) + mainmod.main() + return capsys.readouterr().out + + +def test_path_renders_forward_arrow_when_traversal_matches_edge_direction( + monkeypatch, tmp_path, capsys +): + """createPatchHandler --calls--> validateSanitySession is the stored direction; + querying in that order should print the arrow pointing right.""" + graph_path = _write_undirected_graph(tmp_path) + out = _run_path( + monkeypatch, graph_path, "createPatchHandler", "validateSanitySession", capsys + ) + assert "Shortest path (1 hops):" in out + assert "createPatchHandler() --calls [EXTRACTED]--> validateSanitySession()" in out + + +def test_path_renders_reverse_arrow_when_traversal_opposes_edge_direction( + monkeypatch, tmp_path, capsys +): + """When the user queries `path B A` but the stored edge is A→B, the arrow must + point back at A — not forward at A as if B called A. This is the #849 regression.""" + graph_path = _write_undirected_graph(tmp_path) + out = _run_path( + monkeypatch, graph_path, "validateSanitySession", "createPatchHandler", capsys + ) + assert "Shortest path (1 hops):" in out + # Reverse arrow: traversal goes validate→create but edge points create→validate. + assert ( + "validateSanitySession() <--calls [EXTRACTED]-- createPatchHandler()" in out + ) + # Critical guard: must NOT render forward in this direction (the #849 bug output). + assert ( + "validateSanitySession() --calls [EXTRACTED]--> createPatchHandler()" not in out + )