From 4be0f7b3bfe9ac32f6a8caca5bbd6cac632d20c8 Mon Sep 17 00:00:00 2001 From: safishamsi Date: Wed, 8 Jul 2026 12:45:08 +0100 Subject: [PATCH] fix(merge-graphs): give each input a distinct repo tag so same-stem nodes don't collapse (#1729) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit merge-graphs prefixed each graph's node ids with `::` where repo was `gp.parent.parent.name` — the graphify-out parent dir name. That tag is not unique across inputs: `src/graphify-out` and `frontend/src/graphify-out` both yield `src`, so a bare `app` node from a backend `src/app.js` and a frontend `App.jsx` both became `src::app` and nx.compose silently merged them into one node (one graph's label/source_file, both graphs' edges) — inventing false cross-runtime `path` results with no warning. New `distinct_repo_tags` guarantees a unique prefix per graph: colliding tags are widened with their own parent dir (`frontend_src`), then an index suffix backstops any residual duplicate. The handler prints a note when it disambiguates. Verified end to end: the two `app` nodes now survive as `..._src::app` and `frontend_src::app` instead of collapsing. Regression tests cover the merge case and the tag uniquifier (pass-through, widening, and triple-collision fallback). Co-Authored-By: Claude Opus 4.8 (1M context) --- graphify/__main__.py | 13 +++++++--- graphify/build.py | 28 +++++++++++++++++++++ tests/test_merge_graphs_cli.py | 46 ++++++++++++++++++++++++++++++++++ 3 files changed, 84 insertions(+), 3 deletions(-) diff --git a/graphify/__main__.py b/graphify/__main__.py index 285d19c..b869e86 100644 --- a/graphify/__main__.py +++ b/graphify/__main__.py @@ -3993,7 +3993,7 @@ def main() -> None: sys.exit(1) import networkx as _nx from networkx.readwrite import json_graph as _jg - from graphify.build import prefix_graph_for_global as _prefix + from graphify.build import prefix_graph_for_global as _prefix, distinct_repo_tags as _repo_tags graphs = [] for gp in graph_paths: if not gp.exists(): @@ -4025,9 +4025,16 @@ def main() -> None: if type(g) is not _nx.Graph: return _nx.Graph(g) return g + # Unique repo tag per graph. The bare `graphify-out/..` dir name is not + # unique across inputs (src/graphify-out and frontend/src/graphify-out both + # → "src"), which collides same-stem node ids and silently merges unrelated + # entities (#1729). distinct_repo_tags guarantees a distinct prefix per graph. + repo_tags = _repo_tags(graph_paths) + naive_tags = [gp.parent.parent.name for gp in graph_paths] + if len(set(naive_tags)) != len(naive_tags): + print(f" note: repo dir names collide; using distinct tags: {', '.join(repo_tags)}") merged = _nx.Graph() - for G, gp in zip(graphs, graph_paths): - repo_tag = gp.parent.parent.name # graphify-out/../ → repo dir name + for G, repo_tag in zip(graphs, repo_tags): prefixed = _to_simple(_prefix(G, repo_tag)) merged = _nx.compose(merged, prefixed) try: diff --git a/graphify/build.py b/graphify/build.py index b26ddcb..f0676a8 100644 --- a/graphify/build.py +++ b/graphify/build.py @@ -956,6 +956,34 @@ def prefix_graph_for_global(G: nx.Graph, repo_tag: str) -> nx.Graph: return H +def distinct_repo_tags(graph_paths: "list[Path]") -> "list[str]": + """Return a unique, human-meaningful repo tag per input graph for merge-graphs. + + The naive tag (the ``graphify-out`` parent dir name) is NOT unique across + inputs: ``src/graphify-out`` and ``frontend/src/graphify-out`` both yield + ``src``. Prefixing both node sets with ``src::`` then makes same-stem nodes + (a backend ``src/app.js`` and a frontend ``App.jsx``, both bare ``app``) + collide, so ``nx.compose`` silently merges two unrelated entities and invents + cross-runtime edges (#1729). Colliding tags are widened with their own parent + dir (``frontend_src``), then an index suffix guarantees uniqueness so no two + graphs ever share a prefix. + """ + repo_dirs = [p.parent.parent for p in graph_paths] # graphify-out/.. → repo dir + tags = [d.name or "repo" for d in repo_dirs] + if len(set(tags)) != len(tags): + widened: list[str] = [] + for d in repo_dirs: + parent = d.parent.name + widened.append(f"{parent}_{d.name}" if parent and d.name else (d.name or "repo")) + tags = widened + seen: dict[str, int] = {} + unique: list[str] = [] + for t in tags: + seen[t] = seen.get(t, 0) + 1 + unique.append(t if seen[t] == 1 else f"{t}-{seen[t]}") + return unique + + def prune_repo_from_graph(G: nx.Graph, repo_tag: str) -> int: """Remove all nodes tagged with repo_tag from G in-place. Returns count removed.""" to_remove = [n for n, d in G.nodes(data=True) if d.get("repo") == repo_tag] diff --git a/tests/test_merge_graphs_cli.py b/tests/test_merge_graphs_cli.py index f3e0e05..5ff06b3 100644 --- a/tests/test_merge_graphs_cli.py +++ b/tests/test_merge_graphs_cli.py @@ -46,3 +46,49 @@ def test_merge_graphs_mixed_directed_and_multigraph(tmp_path): assert {"r1::x", "r2::y", "r3::z"} <= ids or len(ids) == 3 assert data.get("directed") is False assert data.get("multigraph") is False + + +def test_merge_graphs_same_named_repo_dirs_do_not_collapse(tmp_path): + # #1729: two graphs under a same-named repo dir (src/graphify-out and + # frontend/src/graphify-out both → tag "src") share the `src::` prefix, so a + # bare `app` node from each collapsed into one — silently merging unrelated + # entities and inventing cross-runtime edges. Distinct tags must keep them apart. + a = tmp_path / "src" / "graphify-out" / "graph.json" + b = tmp_path / "frontend" / "src" / "graphify-out" / "graph.json" + a.parent.mkdir(parents=True, exist_ok=True) + b.parent.mkdir(parents=True, exist_ok=True) + a.write_text(json.dumps({"directed": False, "multigraph": False, "nodes": [ + {"id": "app", "label": "app.js", "source_file": "app.js"}], "links": []})) + b.write_text(json.dumps({"directed": False, "multigraph": False, "nodes": [ + {"id": "app", "label": "App.jsx", "source_file": "App.jsx"}], "links": []})) + out = tmp_path / "merged.json" + + r = _run(["merge-graphs", str(a), str(b), "--out", str(out)], tmp_path) + assert r.returncode == 0, r.stderr + data = json.loads(out.read_text()) + app_nodes = [n for n in data["nodes"] if n["id"].endswith("::app")] + assert len(app_nodes) == 2, f"both app nodes must survive; got {[n['id'] for n in app_nodes]}" + labels = {n.get("label") for n in app_nodes} + assert labels == {"app.js", "App.jsx"}, f"both entities preserved; got {labels}" + + +def test_distinct_repo_tags_unit(tmp_path): + from graphify.build import distinct_repo_tags + # distinct repo dirs pass through unchanged + assert distinct_repo_tags([ + Path("backend/graphify-out/graph.json"), + Path("web/graphify-out/graph.json"), + ]) == ["backend", "web"] + # same-named repo dirs are widened to stay distinct + tags = distinct_repo_tags([ + Path("proj/src/graphify-out/graph.json"), + Path("proj/frontend/src/graphify-out/graph.json"), + ]) + assert len(set(tags)) == 2, tags + # a repeated dir name triple still yields all-distinct tags (index fallback) + tags3 = distinct_repo_tags([ + Path("a/src/graphify-out/graph.json"), + Path("b/src/graphify-out/graph.json"), + Path("c/src/graphify-out/graph.json"), + ]) + assert len(set(tags3)) == 3, tags3