From e8436b411e9d8ba6ba003e6508b67ca9da9c8c65 Mon Sep 17 00:00:00 2001 From: Rishet Mehra Date: Sat, 1 Aug 2026 02:44:02 +0530 Subject: [PATCH] fix(build): inherit the on-disk directed flag in build_merge (#2342) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit build_merge took graph direction from its caller only, defaulting to directed=False, and rebuilt with that default at build.py:1327 — discarding the direction of the graph it had just loaded from graph_path. Since the only call site (cli.py:3547) passes no directed=, every incremental `graphify update` through the merge path silently returned an undirected graph for a directed one. _load_existing_graph now also returns the on-disk `directed` flag, parsed from the JSON it already reads, and build_merge's parameter becomes bool | None: None inherits that flag when a graph exists and falls back to False when there is none, while an explicit True/False still wins. --- graphify/build.py | 21 ++++++++--- tests/test_build.py | 91 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 106 insertions(+), 6 deletions(-) diff --git a/graphify/build.py b/graphify/build.py index 75af8abd..2a3b5bb8 100644 --- a/graphify/build.py +++ b/graphify/build.py @@ -1188,9 +1188,9 @@ 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. Reads the JSON directly instead of going through node_link_graph(). The latter rebuilds an undirected nx.Graph and then enumerating @@ -1220,6 +1220,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)), ) @@ -1257,7 +1258,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, _ = loaded _eff_root = ( str(Path(root).resolve()) if root is not None @@ -1324,7 +1325,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, @@ -1337,17 +1338,25 @@ 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: if None (default), honor the on-disk graph's own ``directed`` flag + when one exists, so an incremental merge can't silently flip a directed + graph undirected (#2342). Falls back to False when there is no existing + graph to inherit from. An explicit True/False always overrides the on-disk + flag. """ 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 if had_graph else False # 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/tests/test_build.py b/tests/test_build.py index 0851ba75..098a2b38 100644 --- a/tests/test_build.py +++ b/tests/test_build.py @@ -565,6 +565,97 @@ def test_build_merge_preserves_call_edge_direction(tmp_path): ) +def test_build_merge_directed_edge_direction_survives_round_trip(tmp_path): + """Regression for #2342: once build_merge correctly inherits the on-disk + `directed` flag, the resulting graph must actually be a DiGraph whose + edges are readable in the right direction (a -> b, not b -> a).""" + from graphify.extract import extract_js + from graphify.export import to_json + + src = "function b() {}\nfunction a() { b(); }\n" + src_file = tmp_path / "x.js" + src_file.write_text(src) + + extraction = extract_js(src_file) + call_edges = [e for e in extraction["edges"] if e["relation"] == "calls"] + assert len(call_edges) == 1 + truth_src = call_edges[0]["source"] + truth_tgt = call_edges[0]["target"] + + G1 = build([extraction], directed=True, dedup=False) + graph_path = tmp_path / "graph.json" + assert to_json(G1, {}, str(graph_path), force=True) + + G2 = build_merge([], graph_path, dedup=False) + assert G2.is_directed() is True + assert G2.has_edge(truth_src, truth_tgt) + assert not G2.has_edge(truth_tgt, truth_src) + + +def test_build_merge_inherits_directed_flag_from_disk(tmp_path): + """Regression for #2342. + + build_merge with no explicit `directed=` must honor the on-disk graph's + own `directed` flag instead of silently defaulting to False, or an + incremental --update on a directed graph downgrades it to undirected. + """ + ext = { + "nodes": [{"id": "a", "label": "a", "file_type": "concept", + "source_file": "x.md", "source_location": "L1"}], + "edges": [], + } + from graphify.export import to_json + + graph_path = tmp_path / "graph.json" + + # Directed graph on disk -> no directed= kwarg -> stays directed. + G1 = build([ext], directed=True, dedup=False) + assert to_json(G1, {}, str(graph_path), force=True) + G2 = build_merge([], graph_path, dedup=False) + assert G2.is_directed() is True + saved = json.loads(graph_path.read_text()) + assert saved.get("directed") is True + + # Undirected graph on disk -> no directed= kwarg -> stays undirected (no regression). + G3 = build([ext], directed=False, dedup=False) + assert to_json(G3, {}, str(graph_path), force=True) + G4 = build_merge([], graph_path, dedup=False) + assert G4.is_directed() is False + + +def test_build_merge_fresh_graph_defaults_undirected(tmp_path): + """No existing graph.json + no directed= kwarg -> falls back to the + current default (False), same as before #2342.""" + graph_path = tmp_path / "does_not_exist.json" + G = build_merge([], graph_path, dedup=False) + assert G.is_directed() is False + + +def test_build_merge_explicit_directed_overrides_disk_flag(tmp_path): + """An explicit directed=True/False from the caller must still win over + whatever is stored on disk (#2342).""" + ext = { + "nodes": [{"id": "a", "label": "a", "file_type": "concept", + "source_file": "x.md", "source_location": "L1"}], + "edges": [], + } + from graphify.export import to_json + + graph_path = tmp_path / "graph.json" + + # Directed on disk, explicit directed=False -> caller wins. + G1 = build([ext], directed=True, dedup=False) + assert to_json(G1, {}, str(graph_path), force=True) + G2 = build_merge([], graph_path, directed=False, dedup=False) + assert G2.is_directed() is False + + # Undirected on disk, explicit directed=True -> caller wins. + G3 = build([ext], directed=False, dedup=False) + assert to_json(G3, {}, str(graph_path), force=True) + G4 = build_merge([], graph_path, directed=True, dedup=False) + assert G4.is_directed() is True + + def test_build_from_json_preserves_first_direction_on_bidirectional_pair(tmp_path): """Regression for #1061.