diff --git a/graphify/cli.py b/graphify/cli.py index 834ac71..d373454 100644 --- a/graphify/cli.py +++ b/graphify/cli.py @@ -2762,7 +2762,10 @@ def dispatch_command(cmd: str) -> None: # across modes (#1317; node dedup also collapses shared Swift module # anchors emitted per importing file, #1327). from graphify.build import dedupe_edges as _dedupe_edges, dedupe_nodes as _dedupe_nodes - from graphify.export import backup_if_protected as _backup + from graphify.export import ( + backup_if_protected as _backup, + existing_graph_node_count as _existing_graph_node_count, + ) if ( incremental_mode and not code_files @@ -2808,6 +2811,24 @@ def dispatch_command(cmd: str) -> None: _e["source_file"] = ( _node_sf.get(_e.get("source")) or _node_sf.get(_e.get("target")) or "" ) + # RT-parity for the raw path: an incomplete build must not force a + # partial graph over a larger complete one here either. The clustered + # path gets this from to_json's #479 guard; this path never calls + # to_json, so replicate the shrink check against the existing file and + # exit before the write/manifest unless --allow-partial is set. + if _extraction_incomplete and not cli_allow_partial: + _existing_n = _existing_graph_node_count(graph_json_path) + if _existing_n is not None and len(merged["nodes"]) < _existing_n: + print( + "[graphify extract] error: extraction was incomplete (an AST/" + "semantic pass failed) and the resulting --no-cluster graph is " + f"smaller than the existing {graph_json_path} " + f"({len(merged['nodes'])} < {_existing_n} nodes). Refusing to " + "overwrite a complete graph with a partial one. Re-run after " + "fixing the failures, or pass --allow-partial to overwrite anyway.", + file=sys.stderr, + ) + sys.exit(1) _backup(graphify_out) graph_json_path.write_text( json.dumps(merged, indent=2), encoding="utf-8" @@ -2908,10 +2929,9 @@ def dispatch_command(cmd: str) -> None: # graph could silently overwrite a good complete one, so fall back to the # shrink guard (force=False) unless the user opts in with --allow-partial. # - # Scope: this guards the clustered write path only. The `--no-cluster` - # raw-dump path (above) writes graph.json directly and has never had a - # shrink guard, so an incomplete --no-cluster run can still overwrite a - # complete graph — a pre-existing gap this change does not close. + # Both write paths are guarded: the clustered path here via to_json's + # #479 check, and the `--no-cluster` raw-dump path above via the same + # shrink check against the existing file (existing_graph_node_count). # # Trade-off: this reuses to_json's coarse node-count guard, not the # source-aware _check_shrink that watch/update use. On an incremental run diff --git a/graphify/export.py b/graphify/export.py index 2490249..39ee2ac 100644 --- a/graphify/export.py +++ b/graphify/export.py @@ -180,6 +180,38 @@ def _git_head() -> str | None: return None +def existing_graph_node_count(path: "str | Path") -> int | None: + """Node count of an existing graph.json, or None when it can't be compared + (absent, empty, unreadable, malformed, or over the size cap). + + The raw ``--no-cluster`` write path uses this to apply the same #479 shrink + guard that :func:`to_json` applies inline for the clustered path. None means + "can't verify — let the write proceed", matching how ``to_json`` treats an + empty/oversized/unreadable existing file. + """ + p = Path(path) + if not p.exists(): + return None + from graphify.security import check_graph_file_size_cap + try: + check_graph_file_size_cap(p) + except Exception: + # Oversized: reading it to compare would be the DoS the cap guards against. + return None + try: + raw = p.read_text(encoding="utf-8") + except Exception: + return None + if not raw.strip(): + return None + try: + data = json.loads(raw) + except Exception: + return None + nodes = data.get("nodes") if isinstance(data, dict) else None + return len(nodes) if isinstance(nodes, list) else None + + def to_json(G: nx.Graph, communities: dict[int, list[str]], output_path: str, *, force: bool = False, built_at_commit: str | None = None, community_labels: dict[int, str] | None = None) -> bool: # Safety check: refuse to silently shrink an existing graph (#479) existing_path = Path(output_path) diff --git a/tests/test_export.py b/tests/test_export.py index 05262b4..fa43dd7 100644 --- a/tests/test_export.py +++ b/tests/test_export.py @@ -775,3 +775,15 @@ def test_to_html_handles_null_source_file_and_label(tmp_path): out = tmp_path / "graph.html" to_html(G, {0: ["n1", "n2", "n3"]}, str(out)) assert out.exists() and out.stat().st_size > 0 + + +def test_existing_graph_node_count(tmp_path): + from graphify.export import existing_graph_node_count + p = tmp_path / "graph.json" + assert existing_graph_node_count(p) is None # absent + p.write_text("", encoding="utf-8") + assert existing_graph_node_count(p) is None # empty + p.write_text("{not json", encoding="utf-8") + assert existing_graph_node_count(p) is None # malformed + p.write_text('{"nodes": [{"id": "a"}, {"id": "b"}], "links": []}', encoding="utf-8") + assert existing_graph_node_count(p) == 2 # valid diff --git a/tests/test_incomplete_build_guard.py b/tests/test_incomplete_build_guard.py index 7c4d057..cec5c92 100644 --- a/tests/test_incomplete_build_guard.py +++ b/tests/test_incomplete_build_guard.py @@ -108,3 +108,62 @@ def test_complete_extraction_keeps_force_write(monkeypatch, tmp_path): mainmod.main() assert rec["called"] and rec["force"] is True + + +def _seed_existing_graph(gout, n): + import json + gout.mkdir(parents=True, exist_ok=True) + (gout / "graph.json").write_text( + json.dumps({"nodes": [{"id": f"keep{i}", "label": f"k{i}"} for i in range(n)], + "links": []}), + encoding="utf-8", + ) + + +def _arm_no_cluster(monkeypatch, tmp_path, *, extra_argv=()): + corpus = _make_docs_corpus(tmp_path) + out_dir = tmp_path / "out" + gout = out_dir / "graphify-out" + _seed_existing_graph(gout, 5) # existing complete graph + monkeypatch.setenv("ANTHROPIC_API_KEY", "sk-test-fake-key") + + def _stub_corpus(paths, **kwargs): + on_chunk = kwargs.get("on_chunk_done") + if on_chunk: + on_chunk(0, 3, {"nodes": [], "edges": [], "hyperedges": []}) # 1 of 3 -> partial + return {"nodes": [{"id": "s1", "source_file": str(corpus / "README.md"), + "file_type": "document", "label": "Notes"}], + "edges": [], "hyperedges": [], "input_tokens": 1, "output_tokens": 1} + + monkeypatch.setattr("graphify.llm.extract_corpus_parallel", _stub_corpus) + monkeypatch.setattr(mainmod, "_check_skill_version", lambda _: None) + monkeypatch.setattr( + mainmod.sys, "argv", + ["graphify", "extract", str(corpus), "--backend", "claude", "--no-cluster", + "--out", str(out_dir), *extra_argv], + ) + return gout / "graph.json" + + +def test_no_cluster_incomplete_build_refuses_to_shrink(tmp_path, monkeypatch, capsys): + import json + graph = _arm_no_cluster(monkeypatch, tmp_path) + + with pytest.raises(SystemExit) as exc: + mainmod.main() + + assert exc.value.code == 1 + assert "Refusing to overwrite" in capsys.readouterr().err + # The existing 5-node graph is untouched — the partial 1-node graph was refused. + assert len(json.loads(graph.read_text())["nodes"]) == 5 + + +def test_no_cluster_allow_partial_overwrites(tmp_path, monkeypatch): + import json + graph = _arm_no_cluster(monkeypatch, tmp_path, extra_argv=["--allow-partial"]) + + with pytest.raises(SystemExit) as exc: + mainmod.main() + + assert exc.value.code == 0 # the raw --no-cluster path exits 0 on success + assert len(json.loads(graph.read_text())["nodes"]) == 1