diff --git a/graphify/build.py b/graphify/build.py index 4ea30a45e..53f4a5f21 100644 --- a/graphify/build.py +++ b/graphify/build.py @@ -1124,9 +1124,14 @@ def deduplicate_by_label(nodes: list[dict], edges: list[dict]) -> tuple[list[dic return deduped_nodes, deduped_edges -def _load_existing_graph(graph_path: Path) -> "tuple[list, list, list] | None": - """Load (nodes, edges, hyperedges) from an existing graph.json for an - incremental merge, accepting both the ``links`` and ``edges`` spellings. +def _load_existing_graph(graph_path: Path) -> "tuple[list, list, list, bool] | None": + """Load (nodes, edges, hyperedges, directed) from an existing graph.json for + an incremental merge, accepting both the ``links`` and ``edges`` spellings. + + ``directed`` is the persisted graph type, so an incremental merge can rebuild + the same kind of graph instead of silently downgrading a directed graph to an + undirected one (#2342). Missing key reads as False, matching the + ``cluster-only`` round-trip in cli.py. Reads the JSON directly instead of going through node_link_graph(). The latter rebuilds an undirected nx.Graph and then enumerating @@ -1156,6 +1161,7 @@ def _load_existing_graph(graph_path: Path) -> "tuple[list, list, list] | None": list(data.get("nodes", [])), list(data.get(links_key, [])), list(data.get("hyperedges", [])), + bool(data.get("directed", False)), ) @@ -1193,7 +1199,7 @@ def merge_raw_extraction( loaded = _load_existing_graph(graph_path) if loaded is None: return new - existing_nodes, existing_edges, existing_hyperedges = loaded + existing_nodes, existing_edges, existing_hyperedges, _existing_directed = loaded _eff_root = ( str(Path(root).resolve()) if root is not None @@ -1260,7 +1266,7 @@ def build_merge( graph_path: str | Path | None = None, prune_sources: list[str] | None = None, *, - directed: bool = False, + directed: bool | None = None, dedup: bool = True, dedup_llm_backend: str | None = None, root: str | Path | None = None, @@ -1273,17 +1279,23 @@ def build_merge( preserved unchanged; deleted files are removed via prune_sources. Safe to call repeatedly. root: if given, absolute source_file paths in new_chunks are made relative (#932). + directed: None (default) INHERITS the loaded graph's type, so a routine + incremental refresh can't silently rebuild a directed graph as undirected and + turn betweenness into a ubiquity score (#2342). Pass True/False to force it. """ graph_path = Path(graph_path if graph_path is not None else _default_graph_json()) _loaded = _load_existing_graph(graph_path) if _loaded is not None: - existing_nodes, existing_edges, existing_hyperedges = _loaded + existing_nodes, existing_edges, existing_hyperedges, existing_directed = _loaded had_graph = True else: existing_nodes = [] existing_edges = [] existing_hyperedges = [] + existing_directed = False had_graph = False + if directed is None: + directed = existing_directed # Effective root for relativizing absolute source_file / prune paths back to the # stored relative source_file keys. When the caller passes root we use it; diff --git a/graphify/watch.py b/graphify/watch.py index c87ec8e6f..5c847b1cb 100644 --- a/graphify/watch.py +++ b/graphify/watch.py @@ -1258,7 +1258,11 @@ def _add_deleted_source(path: Path) -> None: "total_words": detected.get("total_words", 0), } - G = build_from_json(result) + # Inherit the persisted graph type. Rebuilding a graph that was written + # with --directed as an undirected one silently breaks betweenness — a + # pure sink every module imports scores as the top god node — and nothing + # in the report says the graph changed type (#2342). + G = build_from_json(result, directed=bool(existing_graph_data.get("directed", False))) candidate_topology = _topology_from_graph(G) if existing_graph_data: try: diff --git a/tests/test_build.py b/tests/test_build.py index 0851ba75e..695268d41 100644 --- a/tests/test_build.py +++ b/tests/test_build.py @@ -498,6 +498,81 @@ def test_ghost_merge_non_ast_same_file_still_merges(): assert G.number_of_nodes() == 1 +def _directed_graph_json(tmp_path): + """Write a small DIRECTED graph.json (caller -> callee -> leaf) and return its path.""" + from graphify.export import to_json + + extraction = { + "nodes": [ + {"id": "a_caller", "label": "caller()", "file_type": "code", "source_file": "a.py"}, + {"id": "a_callee", "label": "callee()", "file_type": "code", "source_file": "a.py"}, + {"id": "a_leaf", "label": "leaf()", "file_type": "code", "source_file": "a.py"}, + ], + "edges": [ + {"source": "a_caller", "target": "a_callee", "relation": "calls", + "confidence": "EXTRACTED", "confidence_score": 1.0, "source_file": "a.py"}, + {"source": "a_callee", "target": "a_leaf", "relation": "calls", + "confidence": "EXTRACTED", "confidence_score": 1.0, "source_file": "a.py"}, + ], + "hyperedges": [], + } + graph_path = tmp_path / "graph.json" + G = build_from_json(extraction, directed=True) + assert to_json(G, {}, str(graph_path), force=True) + assert json.loads(graph_path.read_text())["directed"] is True + return graph_path + + +def test_build_merge_inherits_persisted_directed_flag(tmp_path): + """Regression for #2342. + + build_merge defaulted to directed=False, so an incremental refresh rebuilt a + graph saved with --directed as an undirected one and wrote it back. Nothing + reported the change, but betweenness silently stops meaning anything: an + undirected graph turns a pure sink (in_degree=N, out_degree=0) into a fake + bridge between every pair of its importers, so god-node rankings report + ubiquity instead of architecture. + + The saved flag already round-trips on the cluster-only path + (cli.py: build_from_json(_raw, directed=_directed)); the merge path must + honour it too. + """ + from graphify.export import to_json + + graph_path = _directed_graph_json(tmp_path) + + # What `graphify update` does: merge an empty chunk set and save back. + G2 = build_merge([{"nodes": [], "edges": [], "hyperedges": []}], graph_path, dedup=False) + assert G2.is_directed(), ( + "build_merge downgraded a directed graph to undirected (#2342)" + ) + assert to_json(G2, {}, str(graph_path), force=True) + assert json.loads(graph_path.read_text())["directed"] is True + + # A sink must not score as a bridge once direction survives the merge. + assert nx.betweenness_centrality(G2)["a_leaf"] == 0.0 + + +def test_build_merge_directed_flag_still_forceable(tmp_path): + """directed=None inherits, but an explicit value still wins in both + directions — callers that deliberately pick a graph type keep doing so.""" + graph_path = _directed_graph_json(tmp_path) + + assert not build_merge([], graph_path, dedup=False, directed=False).is_directed() + assert build_merge([], graph_path, dedup=False, directed=True).is_directed() + + +def test_build_merge_defaults_undirected_without_existing_graph(tmp_path): + """No graph on disk means nothing to inherit — keep the historical default.""" + chunk = { + "nodes": [{"id": "n1", "label": "n1()", "file_type": "code", "source_file": "a.py"}], + "edges": [], + "hyperedges": [], + } + G = build_merge([chunk], tmp_path / "missing.json", dedup=False) + assert not G.is_directed() + + def test_build_merge_preserves_call_edge_direction(tmp_path): """Regression for #760. diff --git a/tests/test_watch.py b/tests/test_watch.py index 1b2efce33..afad11a78 100644 --- a/tests/test_watch.py +++ b/tests/test_watch.py @@ -160,6 +160,41 @@ def test_graphify_root_preserves_relative_when_invoked_with_relative_path(tmp_pa ) +def test_rebuild_code_preserves_directed_graph(tmp_path): + """#2342: `graphify update` must rebuild a directed graph as directed. + + _rebuild_code called build_from_json(result) with no `directed=`, so every + routine refresh rewrote a graph saved with --directed as an undirected one, + silently and with nothing in the output saying the graph changed type. The + damage is in the report: betweenness is only meaningful on a directed graph, + so undirected a pure sink every module imports becomes the top god node.""" + from graphify.watch import _rebuild_code + + corpus = tmp_path / "corpus" + corpus.mkdir() + (corpus / "a.py").write_text( + "def alpha():\n return beta()\n\ndef beta():\n return 1\n", encoding="utf-8" + ) + assert _rebuild_code(corpus, acquire_lock=False) is True + + # Stand in for a graph built with --directed: flip the persisted graph type. + graph_file = corpus / "graphify-out" / "graph.json" + graph = json.loads(graph_file.read_text(encoding="utf-8")) + graph["directed"] = True + graph_file.write_text(json.dumps(graph), encoding="utf-8") + + # Grow the corpus so the next rebuild has real work and actually writes. + (corpus / "b.py").write_text( + "import a\n\ndef gamma():\n return a.alpha()\n", encoding="utf-8" + ) + assert _rebuild_code(corpus, acquire_lock=False) is True + + rebuilt = json.loads(graph_file.read_text(encoding="utf-8")) + assert rebuilt["directed"] is True, ( + "incremental rebuild downgraded a directed graph to undirected (#2342)" + ) + + def test_rebuild_code_writes_community_name(tmp_path): """#1808: `graphify update` / _rebuild_code must forward community_labels to to_json, so graph.json nodes carry a human-readable community_name (hub-derived