diff --git a/graphify/cli.py b/graphify/cli.py index fb4d04ae..e6b3ee5e 100644 --- a/graphify/cli.py +++ b/graphify/cli.py @@ -1088,8 +1088,13 @@ def dispatch_command(cmd: str) -> 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"]) - # Force directed so the renderer can recover stored caller→callee direction. - _raw = {**_raw, "directed": True} + # Force directed so the renderer can recover stored caller→callee + # direction, and multigraph so exact-pair parallel links (e.g. a + # `references` and a `calls` edge between the same two nodes) survive load + # instead of being silently collapsed last-writer-wins — otherwise the + # printed relation could be one the traversed pair doesn't actually + # carry (#2074). Local to this read; serve's shared graph is untouched. + _raw = {**_raw, "directed": True, "multigraph": True} try: G = json_graph.node_link_graph(_raw, edges="links") except TypeError: @@ -1129,26 +1134,38 @@ def dispatch_command(cmd: str) -> None: f"(top score {_top:g}, runner-up {_runner:g})", file=sys.stderr, ) + # Deterministic shortest path (#2074): to_undirected(as_view=True) + # iterates neighbors via a hash-seeded set union, so among equal-length + # paths BFS returned an arbitrary route that varied per process. Build a + # sorted, materialized undirected graph so neighbor order — and thus the + # chosen path — is canonical for a given graph.json. + _und = _nx.Graph() + _und.add_nodes_from(sorted(G.nodes)) + _und.add_edges_from(sorted((min(u, v), max(u, v)) for u, v in G.edges())) try: - path_nodes = _nx.shortest_path(G.to_undirected(as_view=True), src_nid, tgt_nid) + path_nodes = _nx.shortest_path(_und, src_nid, tgt_nid) except (_nx.NetworkXNoPath, _nx.NodeNotFound): print(f"No path found between '{source_label}' and '{target_label}'.") sys.exit(0) hops = len(path_nodes) - 1 segments = [] - from graphify.build import edge_data + from graphify.build import edge_datas for i in range(len(path_nodes) - 1): u, v = path_nodes[i], path_nodes[i + 1] - # Check which direction the stored edge points. + # Report the ACTUAL stored relation(s) of the traversed pair and + # direction — never a fabricated `calls` (#2074). A pair may carry + # several parallel relations; show all, and fall back to an honest + # "related" when the stored edge has no relation. if G.has_edge(u, v): - edata = edge_data(G, u, v) + datas = edge_datas(G, u, v) forward = True else: - edata = edge_data(G, v, u) + datas = edge_datas(G, v, u) forward = False - rel = edata.get("relation", "") - conf = edata.get("confidence", "") - conf_str = f" [{conf}]" if conf else "" + rels = sorted({d.get("relation") for d in datas if d.get("relation")}) + rel = "/".join(rels) if rels else "related" + confs = sorted({d.get("confidence") for d in datas if d.get("confidence")}) + conf_str = f" [{'/'.join(confs)}]" if confs else "" if i == 0: segments.append(G.nodes[u].get("label", u)) if forward: diff --git a/graphify/serve.py b/graphify/serve.py index eab34f39..5802ecfb 100644 --- a/graphify/serve.py +++ b/graphify/serve.py @@ -10,7 +10,7 @@ from typing import NamedTuple import networkx as nx from networkx.readwrite import json_graph from graphify.security import sanitize_label, check_graph_file_size_cap -from graphify.build import edge_data +from graphify.build import edge_data, edge_datas from graphify.paths import default_graph_json as _default_graph_json try: @@ -1376,8 +1376,14 @@ def _build_server(graph_path: str): ) max_hops = int(arguments.get("max_hops", 8)) try: - # Use undirected view for path-finding (works regardless of query src/tgt order) - path_nodes = nx.shortest_path(G.to_undirected(as_view=True), src_nid, tgt_nid) + # Deterministic path (#2074): the hash-seeded undirected view picked an + # arbitrary route among equal-length paths. Build a sorted, materialized + # undirected graph so the chosen path is canonical. Serve's shared G is + # left untouched (its degree feeds query-seed tie-breaks). + _und = nx.Graph() + _und.add_nodes_from(sorted(G.nodes)) + _und.add_edges_from(sorted((min(u, v), max(u, v)) for u, v in G.edges())) + path_nodes = nx.shortest_path(_und, src_nid, tgt_nid) except (nx.NetworkXNoPath, nx.NodeNotFound): return f"No path found between '{G.nodes[src_nid].get('label', src_nid)}' and '{G.nodes[tgt_nid].get('label', tgt_nid)}'." hops = len(path_nodes) - 1 @@ -1386,15 +1392,18 @@ def _build_server(graph_path: str): segments = [] for i in range(len(path_nodes) - 1): u, v = path_nodes[i], path_nodes[i + 1] + # Report the actual stored relation(s), never a fabricated `calls`; + # fall back to an honest "related" when the edge has no relation (#2074). if G.has_edge(u, v): - edata = edge_data(G, u, v) + datas = edge_datas(G, u, v) forward = True else: - edata = edge_data(G, v, u) + datas = edge_datas(G, v, u) forward = False - rel = edata.get("relation", "") - conf = edata.get("confidence", "") - conf_str = f" [{conf}]" if conf else "" + rels = sorted({d.get("relation") for d in datas if d.get("relation")}) + rel = "/".join(rels) if rels else "related" + confs = sorted({d.get("confidence") for d in datas if d.get("confidence")}) + conf_str = f" [{'/'.join(confs)}]" if confs else "" if i == 0: segments.append(G.nodes[u].get("label", u)) if forward: diff --git a/tests/test_path_cli.py b/tests/test_path_cli.py index 8b979e3a..2584ce4b 100644 --- a/tests/test_path_cli.py +++ b/tests/test_path_cli.py @@ -1,6 +1,10 @@ -"""Regression tests for `graphify path` arrow direction (#849).""" +"""Regression tests for `graphify path` arrow direction (#849) and determinism + +honest edge labels (#2074).""" from __future__ import annotations import json +import os +import subprocess +import sys import networkx as nx import pytest from networkx.readwrite import json_graph @@ -103,3 +107,88 @@ def test_endpoint_falls_back_to_score_head(monkeypatch, tmp_path, capsys): mainmod.main() assert exc_info.value.code == 0 assert "No path found" in capsys.readouterr().out + + +# ── #2074: deterministic route + honest edge relation ──────────────────────── + +def _diamond_graph(tmp_path): + """Two equal-length routes A->P->B and A->Q->B — a tie the traversal must + resolve deterministically.""" + data = { + "directed": False, "multigraph": False, "graph": {}, + "nodes": [ + {"id": "a", "label": "Alpha", "source_file": "a.py"}, + {"id": "p", "label": "Pmid", "source_file": "p.py"}, + {"id": "q", "label": "Qmid", "source_file": "q.py"}, + {"id": "b", "label": "Beta", "source_file": "b.py"}, + ], + "links": [ + {"source": "a", "target": "p", "relation": "calls", "confidence": "EXTRACTED"}, + {"source": "p", "target": "b", "relation": "calls", "confidence": "EXTRACTED"}, + {"source": "a", "target": "q", "relation": "calls", "confidence": "EXTRACTED"}, + {"source": "q", "target": "b", "relation": "calls", "confidence": "EXTRACTED"}, + ], + } + p = tmp_path / "graph.json" + p.write_text(json.dumps(data)) + return p + + +def _arrow_line(stdout: str) -> str: + return next((l.strip() for l in stdout.splitlines() if "-->" in l or "<--" in l), "") + + +def test_path_deterministic_across_hash_seeds(tmp_path): + """#2074: the same graph must yield the same route regardless of + PYTHONHASHSEED. pytest fixes the seed per process, so run out-of-process.""" + gp = _diamond_graph(tmp_path) + routes = set() + for seed in ("0", "1", "2", "3", "4", "5", "6", "7"): + env = {**os.environ, "PYTHONHASHSEED": seed} + r = subprocess.run( + [sys.executable, "-m", "graphify", "path", "Alpha", "Beta", "--graph", str(gp)], + capture_output=True, text=True, env=env, cwd=str(tmp_path), + ) + assert r.returncode == 0, r.stderr + routes.add(_arrow_line(r.stdout)) + assert len(routes) == 1, f"non-deterministic path across hash seeds: {routes}" + # Canonical tie-break picks the lexicographically-smaller mid node (Pmid). + assert "Pmid" in next(iter(routes)) + + +def test_path_relation_matches_stored_edge_not_fabricated(monkeypatch, tmp_path, capsys): + """#2074: the printed relation must be the edge's ACTUAL stored relation, + never a hardcoded/fabricated `calls`.""" + data = { + "directed": False, "multigraph": False, "graph": {}, + "nodes": [ + {"id": "a", "label": "Alpha", "source_file": "a.py"}, + {"id": "b", "label": "Beta", "source_file": "b.py"}, + ], + "links": [ + {"source": "a", "target": "b", "relation": "references", "confidence": "INFERRED"}, + ], + } + gp = tmp_path / "graph.json" + gp.write_text(json.dumps(data)) + out = _run(monkeypatch, gp, "Alpha", "Beta", capsys) + assert "--references [INFERRED]-->" in out + assert "calls" not in out + + +def test_path_relation_fallback_related_when_missing(monkeypatch, tmp_path, capsys): + """#2074: an edge with no stored relation prints an honest 'related', not an + empty '---->' arrow and not a fabricated relation.""" + data = { + "directed": False, "multigraph": False, "graph": {}, + "nodes": [ + {"id": "a", "label": "Alpha", "source_file": "a.py"}, + {"id": "b", "label": "Beta", "source_file": "b.py"}, + ], + "links": [{"source": "a", "target": "b"}], + } + gp = tmp_path / "graph.json" + gp.write_text(json.dumps(data)) + out = _run(monkeypatch, gp, "Alpha", "Beta", capsys) + assert "--related-->" in out + assert "---->" not in out.replace("--related-->", "")