From 5316650a9b401a27b127f9b8b450b7b60199021d Mon Sep 17 00:00:00 2001 From: himanshupatro-334 Date: Wed, 29 Jul 2026 00:11:34 +0530 Subject: [PATCH] fix: preserve edge direction in merge-graphs --- graphify/build.py | 11 +++-- graphify/cli.py | 16 ++++++++ tests/test_global_graph.py | 16 ++++++++ tests/test_merge_graphs_cli.py | 75 ++++++++++++++++++++++++++++++++++ 4 files changed, 115 insertions(+), 3 deletions(-) diff --git a/graphify/build.py b/graphify/build.py index 1f840bbc0..4ea30a45e 100644 --- a/graphify/build.py +++ b/graphify/build.py @@ -1450,15 +1450,20 @@ def prefix_graph_for_global(G: nx.Graph, repo_tag: str) -> nx.Graph: """Return a copy of G with all node IDs prefixed with repo_tag::. Labels are preserved unchanged (for display). A 'local_id' attribute - is added to each node so the original ID can be recovered. Edges are - rewritten to match the new prefixed IDs. The 'repo' attribute is set - on every node. + is added to each node so the original ID can be recovered. Edges and + their directional attributes (_src/_tgt) are rewritten to match the new + prefixed IDs. The 'repo' attribute is set on every node. """ relabel = {n: f"{repo_tag}::{n}" for n in G.nodes} H = nx.relabel_nodes(G, relabel, copy=True) for node, data in H.nodes(data=True): data["repo"] = repo_tag data.setdefault("local_id", node.split("::", 1)[1]) + for u, v, data in H.edges(data=True): + if "_src" in data and data["_src"] in relabel: + data["_src"] = relabel[data["_src"]] + if "_tgt" in data and data["_tgt"] in relabel: + data["_tgt"] = relabel[data["_tgt"]] return H diff --git a/graphify/cli.py b/graphify/cli.py index bd8f12bca..dc9ed08cc 100644 --- a/graphify/cli.py +++ b/graphify/cli.py @@ -2093,6 +2093,15 @@ def _load_graph(p: str): # via node_link_data but older runs may have used "edges" (#738). if "links" not in data and "edges" in data: data = dict(data, links=data["edges"]) + # Preserve stored edge direction across undirected node_link_graph (#2261). + # Mirrors cli.py's query pattern and export.py's _src/_tgt restoration. + data = dict( + data, + links=[ + {**link, "_src": link.get("source"), "_tgt": link.get("target")} + for link in data.get("links", []) + ], + ) try: G = _jg.node_link_graph(data, edges="links") except TypeError: @@ -2129,6 +2138,13 @@ def _to_simple(g: "_nx.Graph") -> "_nx.Graph": out_data = _jg.node_link_data(merged, edges="links") except TypeError: out_data = _jg.node_link_data(merged) + # Restore original edge direction from _src/_tgt markers (same pattern as export.py #563/#2261) + for link in out_data.get("links", []): + tsrc = link.pop("_src", None) + ttgt = link.pop("_tgt", None) + if tsrc is not None and ttgt is not None: + link["source"] = tsrc + link["target"] = ttgt out_path.parent.mkdir(parents=True, exist_ok=True) from graphify.paths import write_json_atomic as _wja _wja(out_path, out_data, indent=2) diff --git a/tests/test_global_graph.py b/tests/test_global_graph.py index b91a1ebe7..389c88d26 100644 --- a/tests/test_global_graph.py +++ b/tests/test_global_graph.py @@ -66,6 +66,22 @@ def test_prefix_graph_rewrites_edges(): assert not H.has_edge("a", "b") +def test_prefix_graph_rewrites_edge_directional_attributes(): + """prefix_graph_for_global must update directional edge attributes (_src/_tgt) + so they stay aligned with the prefixed node IDs (#2261).""" + from graphify.build import prefix_graph_for_global + G = _make_graph( + [{"id": "rota", "label": "rota.js"}, {"id": "collections", "label": "collections.js"}], + [{"source": "rota", "target": "collections", "relation": "imports_from", "_src": "rota", "_tgt": "collections"}], + ) + H = prefix_graph_for_global(G, "repoA") + assert H.has_edge("repoA::rota", "repoA::collections") + data = H.get_edge_data("repoA::rota", "repoA::collections") + assert data["_src"] == "repoA::rota" + assert data["_tgt"] == "repoA::collections" + + + def test_prune_repo_removes_correct_nodes(): from graphify.build import prune_repo_from_graph G = nx.Graph() diff --git a/tests/test_merge_graphs_cli.py b/tests/test_merge_graphs_cli.py index 5ff06b391..491919f8e 100644 --- a/tests/test_merge_graphs_cli.py +++ b/tests/test_merge_graphs_cli.py @@ -92,3 +92,78 @@ def test_distinct_repo_tags_unit(tmp_path): Path("c/src/graphify-out/graph.json"), ]) assert len(set(tags3)) == 3, tags3 + + +def test_merge_graphs_preserves_import_edge_direction(tmp_path): + # #2261: merge-graphs loaded graph.json into an undirected nx.Graph without + # stashing _src/_tgt directional markers, causing node_link_data to re-serialize + # edges based on arbitrary node ordering and reversing import edge direction whenever + # target node index < source node index (turning imports into self-imports/reversed links). + a = tmp_path / "repo1" / "graphify-out" / "graph.json" + b = tmp_path / "repo2" / "graphify-out" / "graph.json" + a.parent.mkdir(parents=True, exist_ok=True) + b.parent.mkdir(parents=True, exist_ok=True) + + # Note: collections comes BEFORE rota in nodes list, so collections has a smaller index + a.write_text(json.dumps({ + "directed": False, + "multigraph": False, + "nodes": [ + {"id": "collections", "label": "collections.js"}, + {"id": "empresa", "label": "empresa.js"}, + {"id": "logger", "label": "logger.js"}, + {"id": "rota", "label": "rota.js"}, + ], + "links": [ + {"source": "rota", "target": "collections", "relation": "imports_from", "context": "import"}, + {"source": "rota", "target": "empresa", "relation": "imports_from", "context": "import"}, + {"source": "rota", "target": "logger", "relation": "imports_from", "context": "import"}, + ], + })) + + b.write_text(json.dumps({ + "directed": False, + "multigraph": False, + "nodes": [ + {"id": "main", "label": "main.js"}, + {"id": "utils", "label": "utils.js"}, + ], + "links": [ + {"source": "main", "target": "utils", "relation": "imports_from", "context": "import"}, + ], + })) + + out = tmp_path / "merged.json" + r = _run(["merge-graphs", str(a), str(b), "--out", str(out)], tmp_path) + assert r.returncode == 0, f"merge failed: {r.stderr}" + + data = json.loads(out.read_text(encoding="utf-8")) + + # 1. Node count & edge count correct + assert len(data["nodes"]) == 6 + assert len(data["links"]) == 4 + + # 2. Repo prefixes applied + node_ids = {n["id"] for n in data["nodes"]} + expected_ids = { + "repo1::collections", "repo1::empresa", "repo1::logger", "repo1::rota", + "repo2::main", "repo2::utils" + } + assert node_ids == expected_ids + + # 3. Import relationships preserve original direction (source=rota, target=collections/empresa/logger) + # and no import edge becomes a self-import or reversed. + repo1_links = [l for l in data["links"] if l["source"].startswith("repo1::") or l["target"].startswith("repo1::")] + assert len(repo1_links) == 3 + + for link in repo1_links: + src = link["source"] + tgt = link["target"] + assert src != tgt, f"import edge became a self-import: {link}" + assert src == "repo1::rota", f"expected source repo1::rota, got {src} in link {link}" + assert tgt in {"repo1::collections", "repo1::empresa", "repo1::logger"}, f"unexpected target {tgt} in link {link}" + + repo2_link = [l for l in data["links"] if l["source"].startswith("repo2::")][0] + assert repo2_link["source"] == "repo2::main" + assert repo2_link["target"] == "repo2::utils" +