diff --git a/README.md b/README.md index bd1dff423..0f9b1cc4b 100644 --- a/README.md +++ b/README.md @@ -739,6 +739,7 @@ GRAPHIFY_TRIAGE_BACKEND=kimi graphify prs --triage # use a specific backend fo graphify clone https://github.com/karpathy/nanoGPT graphify merge-graphs a.json b.json --out merged.json +graphify merge-graphs a.json b.json --repo-tag api --repo-tag web --out merged.json graphify --version # print installed version graphify watch ./src graphify check-update ./src diff --git a/graphify/build.py b/graphify/build.py index 1686d196e..bc670c84b 100644 --- a/graphify/build.py +++ b/graphify/build.py @@ -21,6 +21,7 @@ # before any graph construction happens. # from __future__ import annotations +import copy import json import os import re @@ -29,7 +30,7 @@ from pathlib import Path import networkx as nx from .ids import make_id, normalize_id as _normalize_id -from .paths import default_graph_json as _default_graph_json +from .paths import GRAPHIFY_OUT_NAME, default_graph_json as _default_graph_json from .validate import validate_extraction @@ -999,29 +1000,121 @@ def prefix_graph_for_global(G: nx.Graph, repo_tag: str) -> nx.Graph: 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. + Canonical ``//graph.json`` inputs use the resolved + repository directory, while flat JSON inputs use their filename stem. + Colliding tags are widened with path context, then an index suffix guarantees + uniqueness so no two input graphs ever share a prefix (#1729). """ - 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 + tags: list[str] = [] + qualifiers: list[str] = [] + for path in graph_paths: + if path.name == "graph.json" and path.parent.name == GRAPHIFY_OUT_NAME: + repo_dir = path.parent.parent.resolve() + tags.append(repo_dir.name or "repo") + qualifiers.append(repo_dir.parent.name) + else: + parent = path.parent.resolve() + tags.append(path.stem or parent.name or "repo") + qualifiers.append(parent.name) + + duplicate_base_tags = {tag for tag in tags if tags.count(tag) > 1} + if duplicate_base_tags: + tags = [ + f"{qualifier}_{tag}" if ( + tag in duplicate_base_tags and qualifier and qualifier != tag + ) else tag + for tag, qualifier in zip(tags, qualifiers) + ] + + tag_buckets: dict[str, list[tuple[str, int]]] = {} + for index, (tag, path) in enumerate(zip(tags, graph_paths)): + tag_buckets.setdefault(tag, []).append((path.resolve().as_posix(), index)) + + unique_by_index: dict[int, str] = {} + for tag, bucket in tag_buckets.items(): + if len(bucket) == 1: + unique_by_index[bucket[0][1]] = tag + continue + for suffix, (_path_identity, index) in enumerate(sorted(bucket), start=1): + unique_by_index[index] = tag if suffix == 1 else f"{tag}-{suffix}" + return [unique_by_index[index] for index in range(len(tags))] + + +def _community_identity(value: object) -> str: + """Return a stable, sortable identity for a repository-local community id.""" + try: + return json.dumps(value, ensure_ascii=False, sort_keys=True) + except TypeError: + return repr(value) + + +def compose_repository_graphs(tagged_graphs: list[tuple[str, nx.Graph]]) -> nx.Graph: + """Compose repository graphs without conflating repository-local identity. + + Node and hyperedge ids are namespaced by repository tag. Community ids are + remapped deterministically into one non-overlapping integer range while + preserving each repository's existing partition. + """ + tags = [tag for tag, _graph in tagged_graphs] + invalid_tags = [tag for tag in tags if not tag or "::" in tag] + if invalid_tags: + raise ValueError("repository tags must be non-empty and cannot contain '::'") + duplicate_tags = sorted({tag for tag in tags if tags.count(tag) > 1}) + if duplicate_tags: + raise ValueError( + "duplicate repository tag(s): " + ", ".join(repr(tag) for tag in duplicate_tags) + ) + + community_keys = { + (tag, _community_identity(data["community"])) + for tag, graph in tagged_graphs + for _node, data in graph.nodes(data=True) + if data.get("community") is not None + } + community_ids = { + key: community_id for community_id, key in enumerate(sorted(community_keys)) + } + + merged = nx.Graph() + merged_hyperedges: list = [] + for tag, source_graph in tagged_graphs: + graph = source_graph if type(source_graph) is nx.Graph else nx.Graph(source_graph) + prefixed = prefix_graph_for_global(graph, tag) + + for _node, data in prefixed.nodes(data=True): + local_community = data.get("community") + if local_community is not None: + merged_community = community_ids[(tag, _community_identity(local_community))] + data["community"] = merged_community + if data.get("community_name") == f"Community {local_community}": + data["community_name"] = f"Community {merged_community}" + + raw_hyperedges = graph.graph.get("hyperedges", []) or [] + if not isinstance(raw_hyperedges, list): + raw_hyperedges = [] + for raw_hyperedge in raw_hyperedges: + if not isinstance(raw_hyperedge, dict): + continue + hyperedge = copy.deepcopy(raw_hyperedge) + _normalize_hyperedge_members(hyperedge) + hyperedge_nodes = hyperedge.get("nodes") + if not isinstance(hyperedge_nodes, list): + continue + filtered_nodes = [ + node for node in hyperedge_nodes + if isinstance(node, str) and node in graph + ] + if len(filtered_nodes) < 2: + continue + if hyperedge.get("id") is not None: + hyperedge["id"] = f"{tag}::{hyperedge['id']}" + hyperedge["nodes"] = [f"{tag}::{node}" for node in filtered_nodes] + merged_hyperedges.append(hyperedge) + + merged = nx.compose(merged, prefixed) + + merged.graph["hyperedges"] = merged_hyperedges + return merged def prune_repo_from_graph(G: nx.Graph, repo_tag: str) -> int: diff --git a/graphify/cli.py b/graphify/cli.py index 380bee50a..9e6ad40dd 100644 --- a/graphify/cli.py +++ b/graphify/cli.py @@ -1417,24 +1417,37 @@ def _load_graph(p: str): # graphify merge-graphs graph1.json graph2.json ... --out merged.json args = sys.argv[2:] graph_paths: list[Path] = [] + repo_tags: list[str] = [] out_path = Path(_GRAPHIFY_OUT) / "merged-graph.json" i = 0 while i < len(args): if args[i] == "--out" and i + 1 < len(args): out_path = Path(args[i + 1]) i += 2 + elif args[i] == "--repo-tag" and i + 1 < len(args): + repo_tags.append(args[i + 1]) + i += 2 else: graph_paths.append(Path(args[i])) i += 1 if len(graph_paths) < 2: print( - "Usage: graphify merge-graphs [...] [--out merged.json]", + "Usage: graphify merge-graphs [...] " + "[--repo-tag TAG ...] [--out merged.json]", + file=sys.stderr, + ) + sys.exit(1) + if repo_tags and len(repo_tags) != len(graph_paths): + print( + "error: --repo-tag must be repeated once for every input graph", file=sys.stderr, ) 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, distinct_repo_tags as _repo_tags + from graphify.build import ( + compose_repository_graphs as _compose_repository_graphs, + distinct_repo_tags as _repo_tags, + ) graphs = [] for gp in graph_paths: if not gp.exists(): @@ -1450,41 +1463,35 @@ def _load_graph(p: str): G = _jg.node_link_graph(data, edges="links") except TypeError: G = _jg.node_link_graph(data) + G.graph["hyperedges"] = data.get( + "hyperedges", G.graph.get("hyperedges", []) + ) graphs.append(G) - # nx.compose requires all graphs to be the same type. When input graphs - # come from different sources (e.g. an AST-only run vs a full LLM run) one - # may be a MultiGraph and another a Graph. Normalise everything to Graph - # (the graphify default) by converting MultiGraphs with nx.Graph(). - def _to_simple(g: "_nx.Graph") -> "_nx.Graph": - # nx.compose requires every graph to be the same type. Inputs may - # disagree on BOTH axes — directed vs undirected, and multi vs simple - # — because per-repo graph.json files are written by different extract - # paths at different times. Normalise everything to a plain undirected - # Graph (the merged cross-repo view is undirected anyway), which covers - # DiGraph / MultiGraph / MultiDiGraph. Without this a directed input - # crashed compose with "All graphs must be directed or undirected" (#1606). - 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, repo_tag in zip(graphs, repo_tags): - prefixed = _to_simple(_prefix(G, repo_tag)) - merged = _nx.compose(merged, prefixed) + if not repo_tags: + 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( + " note: repo dir names collide; using distinct tags: " + + ", ".join(repo_tags) + ) + try: + merged = _compose_repository_graphs(list(zip(repo_tags, graphs))) + except ValueError as exc: + print(f"error: {exc}", file=sys.stderr) + sys.exit(1) try: out_data = _jg.node_link_data(merged, edges="links") except TypeError: out_data = _jg.node_link_data(merged) + out_data["hyperedges"] = merged.graph.get("hyperedges", []) out_path.parent.mkdir(parents=True, exist_ok=True) out_path.write_text(json.dumps(out_data, indent=2), encoding="utf-8") - print(f"Merged {len(graphs)} graphs -> {merged.number_of_nodes()} nodes, {merged.number_of_edges()} edges") + print( + f"Merged {len(graphs)} graphs [{', '.join(repo_tags)}] -> " + f"{merged.number_of_nodes()} nodes, {merged.number_of_edges()} edges, " + f"{len(merged.graph.get('hyperedges', []))} hyperedges" + ) print(f"Written to: {out_path}") elif cmd == "clone": diff --git a/tests/test_merge_graphs_cli.py b/tests/test_merge_graphs_cli.py index 5ff06b391..42542a7fe 100644 --- a/tests/test_merge_graphs_cli.py +++ b/tests/test_merge_graphs_cli.py @@ -28,6 +28,35 @@ def _write(p: Path, directed: bool, multigraph: bool, node_id: str): })) +def _write_repository_graph(p: Path, label: str): + p.parent.mkdir(parents=True, exist_ok=True) + p.write_text(json.dumps({ + "directed": True, + "multigraph": False, + "graph": {}, + "nodes": [ + { + "id": "entry", + "label": f"{label} entry", + "community": 0, + "community_name": "Community 0", + }, + { + "id": "worker", + "label": f"{label} worker", + "community": 1, + "community_name": f"{label} workers", + }, + ], + "links": [ + {"source": "entry", "target": "worker", "relation": "calls"}, + ], + "hyperedges": [ + {"id": "request_flow", "nodes": ["entry", "worker"], "relation": "flow"}, + ], + })) + + def test_merge_graphs_mixed_directed_and_multigraph(tmp_path): a = tmp_path / "r1" / "graphify-out" / "graph.json" b = tmp_path / "r2" / "graphify-out" / "graph.json" @@ -43,7 +72,7 @@ def test_merge_graphs_mixed_directed_and_multigraph(tmp_path): data = json.loads(out.read_text()) ids = {n["id"] for n in data["nodes"]} # every input's node survives, normalized into one undirected simple graph - assert {"r1::x", "r2::y", "r3::z"} <= ids or len(ids) == 3 + assert ids == {"r1::x", "r2::y", "r3::z"} assert data.get("directed") is False assert data.get("multigraph") is False @@ -72,7 +101,8 @@ def test_merge_graphs_same_named_repo_dirs_do_not_collapse(tmp_path): assert labels == {"app.js", "App.jsx"}, f"both entities preserved; got {labels}" -def test_distinct_repo_tags_unit(tmp_path): +def test_distinct_repo_tags_unit(tmp_path, monkeypatch): + from graphify import build as build_mod from graphify.build import distinct_repo_tags # distinct repo dirs pass through unchanged assert distinct_repo_tags([ @@ -92,3 +122,204 @@ def test_distinct_repo_tags_unit(tmp_path): Path("c/src/graphify-out/graph.json"), ]) assert len(set(tags3)) == 3, tags3 + monkeypatch.setattr(build_mod, "GRAPHIFY_OUT_NAME", "custom-out") + assert distinct_repo_tags([ + Path("backend/custom-out/graph.json"), + Path("web/custom-out/graph.json"), + ]) == ["backend", "web"] + + +def test_merge_graphs_preserves_repository_local_identity(tmp_path): + a = tmp_path / "a.json" + b = tmp_path / "b.json" + _write_repository_graph(a, "A") + _write_repository_graph(b, "B") + 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()) + nodes = {node["id"]: node for node in data["nodes"]} + assert set(nodes) == {"a::entry", "a::worker", "b::entry", "b::worker"} + assert {nodes["a::entry"]["community"], nodes["a::worker"]["community"]}.isdisjoint( + {nodes["b::entry"]["community"], nodes["b::worker"]["community"]} + ) + assert nodes["b::entry"]["community_name"] == ( + f"Community {nodes['b::entry']['community']}" + ) + assert nodes["b::worker"]["community_name"] == "B workers" + assert { + (edge["source"], edge["target"], edge["relation"]) + for edge in data["links"] + } == { + ("a::entry", "a::worker", "calls"), + ("b::entry", "b::worker", "calls"), + } + assert { + hyperedge["id"]: hyperedge["nodes"] for hyperedge in data["hyperedges"] + } == { + "a::request_flow": ["a::entry", "a::worker"], + "b::request_flow": ["b::entry", "b::worker"], + } + assert "4 nodes, 2 edges, 2 hyperedges" in r.stdout + + +def test_merge_graphs_derives_tag_from_relative_canonical_path(tmp_path): + a = tmp_path / "repo-a" / "graphify-out" / "graph.json" + b = tmp_path / "repo-b" / "graphify-out" / "graph.json" + _write_repository_graph(a, "A") + _write_repository_graph(b, "B") + + r = _run([ + "merge-graphs", "graphify-out/graph.json", + "../repo-b/graphify-out/graph.json", "--out", "merged.json", + ], tmp_path / "repo-a") + + assert r.returncode == 0, r.stderr + ids = {node["id"] for node in json.loads((tmp_path / "repo-a/merged.json").read_text())["nodes"]} + assert {"repo-a::entry", "repo-b::entry"} <= ids + + +def test_merge_graphs_community_remap_is_input_order_independent(tmp_path): + a = tmp_path / "a.json" + b = tmp_path / "b.json" + _write_repository_graph(a, "A") + _write_repository_graph(b, "B") + out_ab = tmp_path / "merged-ab.json" + out_ba = tmp_path / "merged-ba.json" + + first = _run(["merge-graphs", str(a), str(b), "--out", str(out_ab)], tmp_path) + second = _run(["merge-graphs", str(b), str(a), "--out", str(out_ba)], tmp_path) + + assert first.returncode == second.returncode == 0 + communities_ab = { + node["id"]: node.get("community") + for node in json.loads(out_ab.read_text())["nodes"] + } + communities_ba = { + node["id"]: node.get("community") + for node in json.loads(out_ba.read_text())["nodes"] + } + assert communities_ab == communities_ba + + +def test_merge_graphs_disambiguates_duplicate_derived_tags(tmp_path): + backend = tmp_path / "backend" / "graphify-out" / "graph.json" + a = tmp_path / "owner-a" / "service" / "graphify-out" / "graph.json" + b = tmp_path / "owner-b" / "service" / "graphify-out" / "graph.json" + _write_repository_graph(backend, "Backend") + _write_repository_graph(a, "A") + _write_repository_graph(b, "B") + out = tmp_path / "merged.json" + + r = _run(["merge-graphs", str(backend), str(a), str(b), "--out", str(out)], tmp_path) + + assert r.returncode == 0, r.stderr + ids = {node["id"] for node in json.loads(out.read_text())["nodes"]} + assert "backend::entry" in ids + assert "owner-a_service::entry" in ids + assert "owner-b_service::entry" in ids + + +def test_merge_graphs_duplicate_tag_suffixes_are_input_order_independent(tmp_path): + a = tmp_path / "owner-a" / "common" / "service.json" + b = tmp_path / "owner-b" / "common" / "service.json" + _write_repository_graph(a, "A") + _write_repository_graph(b, "B") + out_ab = tmp_path / "merged-ab.json" + out_ba = tmp_path / "merged-ba.json" + + first = _run(["merge-graphs", str(a), str(b), "--out", str(out_ab)], tmp_path) + second = _run(["merge-graphs", str(b), str(a), "--out", str(out_ba)], tmp_path) + + assert first.returncode == second.returncode == 0 + + def entry_ids_by_label(path: Path) -> dict[str, str]: + data = json.loads(path.read_text()) + return { + node["label"]: node["id"] + for node in data["nodes"] + if node.get("local_id") == "entry" + } + + assert entry_ids_by_label(out_ab) == entry_ids_by_label(out_ba) == { + "A entry": "common_service::entry", + "B entry": "common_service-2::entry", + } + + +def test_merge_graphs_skips_malformed_hyperedge_entries(tmp_path): + a = tmp_path / "a.json" + b = tmp_path / "b.json" + _write_repository_graph(a, "A") + _write_repository_graph(b, "B") + data = json.loads(a.read_text()) + data["hyperedges"] = [ + None, + "invalid", + {"id": "scalar_nodes", "nodes": "entry"}, + {"id": "ghost_member", "nodes": ["entry", "ghost"]}, + {"id": "request_flow", "nodes": ["entry", "worker"], "relation": "flow"}, + ] + a.write_text(json.dumps(data)) + out = tmp_path / "merged.json" + + r = _run(["merge-graphs", str(a), str(b), "--out", str(out)], tmp_path) + + assert r.returncode == 0, r.stderr + hyperedges = json.loads(out.read_text())["hyperedges"] + assert all(isinstance(hyperedge, dict) for hyperedge in hyperedges) + assert {hyperedge["id"] for hyperedge in hyperedges} == { + "a::request_flow", + "b::request_flow", + } + + +def test_merge_graphs_accepts_explicit_repository_tags(tmp_path): + a = tmp_path / "owner-a" / "service" / "graphify-out" / "graph.json" + b = tmp_path / "owner-b" / "service" / "graphify-out" / "graph.json" + _write_repository_graph(a, "A") + _write_repository_graph(b, "B") + out = tmp_path / "merged.json" + + r = _run([ + "merge-graphs", str(a), str(b), + "--repo-tag", "owner-a-service", "--repo-tag", "owner-b-service", + "--out", str(out), + ], tmp_path) + + assert r.returncode == 0, r.stderr + ids = {node["id"] for node in json.loads(out.read_text())["nodes"]} + assert "owner-a-service::entry" in ids + assert "owner-b-service::entry" in ids + + +def test_merge_graphs_rejects_duplicate_explicit_tags(tmp_path): + a = tmp_path / "a.json" + b = tmp_path / "b.json" + _write_repository_graph(a, "A") + _write_repository_graph(b, "B") + out = tmp_path / "merged.json" + + r = _run([ + "merge-graphs", str(a), str(b), + "--repo-tag", "service", "--repo-tag", "service", + "--out", str(out), + ], tmp_path) + + assert r.returncode != 0 + assert "duplicate repository tag(s): 'service'" in r.stderr + assert not out.exists() + + +def test_merge_graphs_requires_one_explicit_tag_per_input(tmp_path): + a = tmp_path / "a.json" + b = tmp_path / "b.json" + _write_repository_graph(a, "A") + _write_repository_graph(b, "B") + + r = _run(["merge-graphs", str(a), str(b), "--repo-tag", "a"], tmp_path) + + assert r.returncode != 0 + assert "--repo-tag must be repeated once for every input graph" in r.stderr