diff --git a/graphify/build.py b/graphify/build.py index dbbf3d2d1..c6491ae8f 100644 --- a/graphify/build.py +++ b/graphify/build.py @@ -1564,6 +1564,7 @@ def build( combined["input_tokens"] += ext.get("input_tokens", 0) combined["output_tokens"] += ext.get("output_tokens", 0) _root = str(Path(root).resolve()) if root else None + _dedup_collapsed = 0 if dedup and combined["nodes"]: # Numeric ids must be str before dedup, which keys on them and would # raise TypeError in _pick_winner's regex search (#2326). build_from_json @@ -1585,6 +1586,7 @@ def build( n["source_file"] = _norm_source_file(n["source_file"], _root) if "definition_file" in n: n["definition_file"] = _norm_source_file(n["definition_file"], _root) + _before_dedup = len(combined["nodes"]) combined["nodes"], combined["edges"] = deduplicate_entities( combined["nodes"], combined["edges"], communities={}, dedup_llm_backend=dedup_llm_backend, root=_root, @@ -1593,7 +1595,13 @@ def build( hyperedges=combined.get("hyperedges"), protected_ids=protected_ids, ) - return build_from_json(combined, directed=directed, root=_root) + _dedup_collapsed = _before_dedup - len(combined["nodes"]) + G = build_from_json(combined, directed=directed, root=_root) + # CLI reads this to tell a dedup shrink from a file deletion (#3774). + # Popped before to_json so it is not stored in graph.json. + if _dedup_collapsed: + G.graph["_dedup_collapsed"] = _dedup_collapsed + return G def _norm_label(label: str | None) -> str: @@ -2249,6 +2257,9 @@ def build_merge( if orphaned: G.remove_nodes_from(orphaned) n_nodes += len(orphaned) + # CLI uses this to excuse a smaller graph when the loss is a deletion (#3774). + # Popped before to_json so it is not stored in graph.json. + G.graph["_pruned_node_count"] = n_nodes # Report only the prune entries that ACTUALLY matched something — not # len(prune_sources), which counted every entry as pruned-from even diff --git a/graphify/cli.py b/graphify/cli.py index badf8bdea..ab779f4c4 100644 --- a/graphify/cli.py +++ b/graphify/cli.py @@ -3361,7 +3361,8 @@ def dispatch_command(cmd: str) -> None: "[--model M] [--mode deep] [--out DIR|--output DIR] [--google-workspace] [--no-cluster] " "[--no-gitignore] [--code-only] [--no-dedup] " "[--max-workers N] [--token-budget N] [--max-concurrency N] " - "[--api-timeout S] [--postgres DSN] [--cargo] [--allow-partial] [--timing]", + "[--api-timeout S] [--postgres DSN] [--cargo] [--allow-partial] " + "[--allow-dedup-shrink] [--timing]", file=sys.stderr, ) sys.exit(1) @@ -3383,6 +3384,9 @@ def dispatch_command(cmd: str) -> None: cli_postgres_dsn: str | None = None cli_cargo: bool = False cli_allow_partial: bool = False + # --allow-dedup-shrink: write a graph that is smaller because dedup + # merged nodes. A deleted file does not need this flag (#3774). + cli_allow_dedup_shrink: bool = False no_cluster = False dedup_llm = False # --no-dedup: skip entity deduplication entirely. On an incremental @@ -3511,6 +3515,8 @@ def dispatch_command(cmd: str) -> None: force = True; i += 1 elif a == "--allow-partial": cli_allow_partial = True; i += 1 + elif a == "--allow-dedup-shrink": + cli_allow_dedup_shrink = True; i += 1 elif a == "--timing": cli_timing = True; i += 1 else: @@ -4676,15 +4682,17 @@ def dispatch_command(cmd: str) -> None: _backup(graphify_out) _invalidate_file_manifest_for_db_graph() # force=True bypasses the #479 shrink guard entirely. A full build - # legitimately shrinks (fuzzy dedup collapse, deleted code) so it keeps - # force=True — EXCEPT when this run's extraction was incomplete (an - # extractor pass crashed or some semantic chunks failed). Then a partial - # 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. + # legitimately shrinks when files were deleted, so a complete run keeps + # force=True — EXCEPT: + # * extraction was incomplete (a pass crashed or a chunk came back + # empty). Fall back to the shrink guard unless --allow-partial. + # * dedup merged nodes and the saved graph would shrink by more than + # this run's deletions (#3774). Refuse the write before to_json unless + # --allow-dedup-shrink. --allow-partial does not accept that shrink. # - # 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). + # Both write paths are guarded for an incomplete extraction: 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. # # 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 @@ -4692,7 +4700,34 @@ def dispatch_command(cmd: str) -> None: # failure can therefore be refused here — recoverable by re-running or # passing --allow-partial (the good graph is preserved and the manifest # is not stamped, so the retry re-extracts). + _gattrs = G.graph if hasattr(G, "graph") else {} + _dedup_collapsed = int(_gattrs.pop("_dedup_collapsed", 0) or 0) + _pruned_nodes = int(_gattrs.pop("_pruned_node_count", 0) or 0) _force_write = cli_allow_partial or not _extraction_incomplete + _dedup_shrink_counts: tuple[int, int] | None = None + if ( + not _extraction_incomplete + and not no_dedup + and not cli_allow_dedup_shrink + and _dedup_collapsed > 0 + ): + from graphify.export import existing_graph_node_count as _existing_graph_node_count + _existing_n = _existing_graph_node_count(graph_json_path) + _new_n = G.number_of_nodes() + if isinstance(_existing_n, int) and _new_n < _existing_n - _pruned_nodes: + _dedup_shrink_counts = (_existing_n, _new_n) + if _dedup_shrink_counts is not None: + # Refuse before to_json. to_json's own refusal tells the caller to + # pass force=True, which is not this flag (#3774). + _old_n, _new_n = _dedup_shrink_counts + print( + "[graphify extract] error: dedup reduced the graph from " + f"{_old_n} nodes to {_new_n} nodes. Refusing to overwrite " + f"{graph_json_path}. Pass --allow-dedup-shrink to write " + "the smaller graph.", + file=sys.stderr, + ) + sys.exit(1) # Stamp provenance from the ANALYSED repo, not the shell's cwd: without # this, to_json's fallback asks `git rev-parse HEAD` in whatever repo the # command was invoked from, so `graphify extract ` run from diff --git a/tests/test_dedup_shrink_refuses_force_write.py b/tests/test_dedup_shrink_refuses_force_write.py new file mode 100644 index 000000000..620d01908 --- /dev/null +++ b/tests/test_dedup_shrink_refuses_force_write.py @@ -0,0 +1,161 @@ +"""A complete `graphify extract` must not force-write a shrink that dedup caused (#3774). + +A full build sets force=True, and to_json then skips the #479 node-count check. +Dedup can merge a same-file duplicate and replace graph.json with no message. +A deleted file is a different shrink: that write still happens. +`--allow-dedup-shrink` is the only flag that accepts the dedup shrink. +""" +from __future__ import annotations + +import json + +import pytest + +import graphify.__main__ as mainmod + + +def _notes(corpus): + readme = str(corpus / "README.md") + return [ + { + "id": "readme_decisions_a", + "label": "Decisions", + "file_type": "document", + "source_file": readme, + "source_location": "L1", + }, + { + "id": "readme_decisions_b", + "label": "Decisions", + "file_type": "document", + "source_file": readme, + "source_location": "L2", + }, + { + "id": "readme_overview", + "label": "Overview", + "file_type": "document", + "source_file": readme, + "source_location": "L3", + }, + ] + + +def _write_graph(path, nodes): + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text( + json.dumps({"nodes": nodes, "links": []}), + encoding="utf-8", + ) + + +def _node_ids(path): + data = json.loads(path.read_text(encoding="utf-8")) + return [n["id"] for n in data["nodes"]] + + +def _corpus(tmp_path): + (tmp_path / "README.md").write_text("# Notes\nThe entry point overview.\n", encoding="utf-8") + (tmp_path / "GUIDE.md").write_text("# Guide\nHow to use the thing.\n", encoding="utf-8") + return tmp_path + + +def _arm(monkeypatch, tmp_path, *, nodes, extra_argv=()): + corpus = _corpus(tmp_path) + out_dir = tmp_path / "out" + graph_path = out_dir / "graphify-out" / "graph.json" + _write_graph(graph_path, nodes) + 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, 1, {"nodes": [], "edges": [], "hyperedges": []}) + return { + "nodes": nodes, + "edges": [], + "hyperedges": [], + "input_tokens": 10, + "output_tokens": 5, + } + + 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", + "--out", str(out_dir), *extra_argv], + ) + return graph_path + + +def _run(): + try: + mainmod.main() + except SystemExit as exc: + return exc.code + return 0 + + +def test_complete_run_refuses_a_same_file_dedup_shrink(monkeypatch, tmp_path, capsys): + """Two same-file duplicates plus one keeper. Dedup merges the pair. + The file on disk must keep all three nodes.""" + graph_path = _arm(monkeypatch, tmp_path, nodes=_notes(tmp_path)) + code = _run() + err = capsys.readouterr().err + assert code not in (None, 0) + assert _node_ids(graph_path) == [ + "readme_decisions_a", + "readme_decisions_b", + "readme_overview", + ] + assert "3" in err and "2" in err + assert "--allow-dedup-shrink" in err + + +def test_allow_dedup_shrink_writes_the_smaller_graph(monkeypatch, tmp_path): + graph_path = _arm( + monkeypatch, tmp_path, nodes=_notes(tmp_path), + extra_argv=["--allow-dedup-shrink"], + ) + code = _run() + ids = _node_ids(graph_path) + assert code in (None, 0) + assert len(ids) == 2 + assert "readme_overview" in ids + assert not {"readme_decisions_a", "readme_decisions_b"} <= set(ids) + raw = graph_path.read_text(encoding="utf-8") + assert "_dedup_collapsed" not in raw + assert "_pruned_node_count" not in raw + + +def test_a_deleted_file_still_writes(monkeypatch, tmp_path): + """The shrink is one missing file. Dedup merges nothing. The write proceeds.""" + corpus = tmp_path + readme = str(corpus / "README.md") + gone = str(corpus / "gone.md") + nodes = [ + { + "id": "readme_overview", + "label": "Overview", + "file_type": "document", + "source_file": readme, + "source_location": "L1", + }, + { + "id": "gone_notes", + "label": "Gone notes", + "file_type": "document", + "source_file": gone, + "source_location": "L1", + }, + ] + fresh = [nodes[0]] + graph_path = _arm(monkeypatch, tmp_path, nodes=fresh) + # Replace the seeded graph with both files. The stub still returns only README. + _write_graph(graph_path, nodes) + code = _run() + ids = _node_ids(graph_path) + assert code in (None, 0) + assert "gone_notes" not in ids + assert "readme_overview" in ids diff --git a/tests/test_hollow_chunks_arm_shrink_guard.py b/tests/test_hollow_chunks_arm_shrink_guard.py index 01d6c2082..943a7f5d1 100644 --- a/tests/test_hollow_chunks_arm_shrink_guard.py +++ b/tests/test_hollow_chunks_arm_shrink_guard.py @@ -97,8 +97,8 @@ def test_allow_partial_still_overrides(monkeypatch, tmp_path): def test_a_run_with_every_file_covered_keeps_force_write(monkeypatch, tmp_path, capsys): - """The ordinary complete run is unchanged: a full build legitimately - shrinks (dedup, deleted code) and keeps bypassing the guard.""" + """A complete run that did not shrink the graph via dedup still force-writes. + A dedup shrink is refused unless --allow-dedup-shrink (#3774).""" rec = _record_force(monkeypatch) _arm(monkeypatch, tmp_path) _run() diff --git a/tests/test_incomplete_build_guard.py b/tests/test_incomplete_build_guard.py index ced50bc3f..af9bbb2cc 100644 --- a/tests/test_incomplete_build_guard.py +++ b/tests/test_incomplete_build_guard.py @@ -100,8 +100,9 @@ def test_allow_partial_forces_write_despite_incomplete(monkeypatch, tmp_path): def test_complete_extraction_keeps_force_write(monkeypatch, tmp_path): - # All chunks succeeded -> a complete build legitimately keeps force=True so a - # genuine dedup/deletion shrink still overwrites. + # All chunks succeeded and dedup did not shrink the saved graph. A complete + # run still force-writes in that case. A dedup shrink is refused unless + # --allow-dedup-shrink (#3774); see test_dedup_shrink_refuses_force_write. rec = _seed_to_json_recorder(monkeypatch, returns=True) _arm_extract(monkeypatch, tmp_path, chunk_total=1, chunk_succeeded=1)