fix(merge-graphs): give each input a distinct repo tag so same-stem nodes don't collapse (#1729)
merge-graphs prefixed each graph's node ids with `<repo>::` 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) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
5c6f7272cd
commit
4be0f7b3bf
+10
-3
@@ -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:
|
||||
|
||||
@@ -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]
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user