diff --git a/graphify/cli.py b/graphify/cli.py index d2c02db61..8ded44b91 100644 --- a/graphify/cli.py +++ b/graphify/cli.py @@ -168,6 +168,25 @@ def _resolve(value: str) -> Path: } +def _shrink_is_self_explained(unverified_shrink, existing_n, new_n) -> bool: + """Does the graph's total net shrink amount to no more than what the + flagged files' own (prior -> fresh) counts already account for? + + The #3203 guard exists because an unverified per-file drop could be + masking a loss somewhere else in the graph that the guard cannot see. + When the flagged files' own numbers already explain the entire net + shrink, there is nothing left outside them for the guard to be worried + about, so a write that would otherwise be refused can proceed (#3412). + Deliberately conservative in the other direction: any shortfall those + numbers do not cover (an unrelated failure, a second shrunk file that + was never flagged) leaves the guard armed exactly as before. + """ + if not unverified_shrink or not isinstance(existing_n, int): + return False + explained = sum(max(0, prior - fresh) for prior, fresh in unverified_shrink.values()) + return existing_n - new_n <= explained + + def _handle_unverified_semantic_shrink( unverified_shrink, *, @@ -4207,6 +4226,7 @@ def _invalidate_file_manifest_for_db_graph() -> None: stages.total() sys.exit(0) + _unverified_shrink_detail = None if merge_existing_graph: # #2169: this raw path used to write ONLY this run's extraction # over graph.json — on an incremental run that is just the @@ -4234,8 +4254,9 @@ def _invalidate_file_manifest_for_db_graph() -> None: # raw-dump this run's partial extraction over it. print(f"error: {exc}", file=sys.stderr) sys.exit(1) + _unverified_shrink_detail = merged.get("_unverified_semantic_shrink") _shrink = _handle_unverified_semantic_shrink( - merged.get("_unverified_semantic_shrink"), + _unverified_shrink_detail, cli_allow_partial=cli_allow_partial, files_by_type=files_by_type, sem_result=sem_result, @@ -4276,6 +4297,14 @@ def _invalidate_file_manifest_for_db_graph() -> None: _existing_n = _existing_graph_node_count(graph_json_path) _malformed = _existing_n is _MALFORMED_GRAPH _shrinks = isinstance(_existing_n, int) and len(merged["nodes"]) < _existing_n + if _shrinks and _shrink_is_self_explained( + _unverified_shrink_detail, _existing_n, len(merged["nodes"]) + ): + # #3412: the flagged files' own reported counts already + # account for the entire net shrink, so nothing outside + # them went missing -- the guard has nothing left to + # verify. + _shrinks = False if _malformed or _shrinks: _detail = ( f"the existing {graph_json_path} is present but unparseable " @@ -4349,9 +4378,13 @@ def _invalidate_file_manifest_for_db_graph() -> None: build_merge as _build_merge, ) from graphify.cluster import cluster as _cluster, score_all as _score_all - from graphify.export import to_json as _to_json + from graphify.export import ( + to_json as _to_json, + existing_graph_node_count as _existing_graph_node_count, + ) from graphify.analyze import god_nodes as _god_nodes, surprising_connections as _surprising dedup_backend = backend if dedup_llm else None + _unverified_shrink_detail = None if merge_existing_graph: # Prune everything the current scan no longer covers: genuinely # deleted manifest rows, excluded-but-alive manifest rows (#1908), @@ -4370,8 +4403,11 @@ def _invalidate_file_manifest_for_db_graph() -> None: dedup_llm_backend=dedup_backend, root=target, ) + _unverified_shrink_detail = ( + G.graph.get("_unverified_semantic_shrink") if hasattr(G, "graph") else None + ) _shrink = _handle_unverified_semantic_shrink( - G.graph.get("_unverified_semantic_shrink") if hasattr(G, "graph") else None, + _unverified_shrink_detail, cli_allow_partial=cli_allow_partial, files_by_type=files_by_type, sem_result=sem_result, @@ -4442,6 +4478,15 @@ def _invalidate_file_manifest_for_db_graph() -> None: # passing --allow-partial (the good graph is preserved and the manifest # is not stamped, so the retry re-extracts). _force_write = cli_allow_partial or not _extraction_incomplete + if not _force_write and _shrink_is_self_explained( + _unverified_shrink_detail, + _existing_graph_node_count(existing_graph_path), + G.number_of_nodes(), + ): + # #3412: the flagged files' own reported counts already account + # for the entire net shrink, so nothing outside them went + # missing -- the guard has nothing left to verify. + _force_write = True # 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_unverified_semantic_shrink.py b/tests/test_unverified_semantic_shrink.py index 9baf65a87..20e40d560 100644 --- a/tests/test_unverified_semantic_shrink.py +++ b/tests/test_unverified_semantic_shrink.py @@ -173,9 +173,14 @@ def _run_cli(): return 0 -def test_3203_e2e_repro_protected_and_manifest_unstamped(tmp_path, monkeypatch, capsys): - """Reproduce #3203: Initial 3 nodes for README.md -> re-extracted with 1 node. - The shrink guard refuses the overwrite, exits 1, and README.md is not stamped. +def test_3203_e2e_repro_self_explained_shrink_proceeds(tmp_path, monkeypatch, capsys): + """#3203 then #3412: Initial 3 nodes for README.md -> re-extracted with 1 + node, nothing else in the corpus touched. The total graph shrink (5 -> 3) + exactly matches README.md's own reported loss (3 -> 1), so nothing outside + the flagged file went missing -- #3412 lets a write like this proceed + without needing --allow-partial, since the guard has nothing left to + verify. The write still logs the shrink it waved through, and the + manifest is stamped for the newly accepted content. """ corpus = tmp_path / "repo" corpus.mkdir() @@ -239,22 +244,187 @@ def _mock_llm_run2(files, **kwargs): monkeypatch.setattr(mainmod.sys, "argv", ["graphify", "extract", str(corpus), "--backend", "claude"]) code2 = _run_cli() - # The shrink guard arms and refuses overwrite because graph shrinks 5 -> 3 - assert code2 == 1 + # #3412: the guard still flags and logs the shrink, but the total loss + # (5 -> 3) is fully explained by README.md's own reported loss (3 -> 1), + # so nothing else in the corpus went missing and the write proceeds. + assert code2 == 0 err = capsys.readouterr().err assert "unverified semantic shrink detected for 'README.md' (3 -> 1 nodes)" in err + + graph2 = json.loads((out_dir / "graph.json").read_text(encoding="utf-8")) + assert len(graph2["nodes"]) == 3 + + # Manifest IS stamped: the write went through, so a retry must not + # re-dispatch README.md again. + manifest2 = json.loads((out_dir / "manifest.json").read_text(encoding="utf-8")) + assert manifest2["README.md"]["semantic_hash"] != initial_hash + + +def test_3203_shrink_beyond_the_flagged_file_still_refuses(tmp_path, monkeypatch, capsys): + """#3412's relaxation must stay narrow: when the corpus loses MORE than + what the flagged file's own reported counts explain -- something else + also went missing, unrelated to the flagged shrink -- the guard still + refuses exactly as #3203 intended.""" + corpus = tmp_path / "repo" + corpus.mkdir() + readme = corpus / "README.md" + guide = corpus / "GUIDE.md" + readme.write_text("# Readme\nInitial content\n", encoding="utf-8") + guide.write_text("# Guide\nGuide content\n", encoding="utf-8") + + out_dir = corpus / "graphify-out" + + def _mock_llm_run1(files, **kwargs): + on_chunk = kwargs.get("on_chunk_done") + res = { + "nodes": [ + {"id": "readme_doc", "label": "Readme", "file_type": "document", "source_file": "README.md"}, + {"id": "readme_sec1", "label": "Section 1", "file_type": "document", "source_file": "README.md"}, + {"id": "readme_sec2", "label": "Section 2", "file_type": "document", "source_file": "README.md"}, + {"id": "guide_doc", "label": "Guide", "file_type": "document", "source_file": "GUIDE.md"}, + {"id": "guide_sec1", "label": "Guide Sec", "file_type": "document", "source_file": "GUIDE.md"}, + ], + "edges": [], "hyperedges": [], + "input_tokens": 10, "output_tokens": 10, "uncovered_files": [], + } + if on_chunk: + on_chunk(0, 1, res) + return res + + monkeypatch.setattr(llmmod, "extract_corpus_parallel", _mock_llm_run1) + monkeypatch.setattr(mainmod, "_check_skill_version", lambda _: None) + monkeypatch.setenv("ANTHROPIC_API_KEY", "sk-fake") + + monkeypatch.setattr(mainmod.sys, "argv", ["graphify", "extract", str(corpus), "--backend", "claude"]) + assert _run_cli() == 0 + + graph1 = json.loads((out_dir / "graph.json").read_text(encoding="utf-8")) + assert len(graph1["nodes"]) == 5 + manifest1 = json.loads((out_dir / "manifest.json").read_text(encoding="utf-8")) + initial_hash = manifest1["README.md"]["semantic_hash"] + + # Edit only README.md, but have the mocked run ALSO silently drop + # GUIDE.md's carried-forward nodes from the merged result -- simulating a + # loss unrelated to the flagged shrink (e.g. a chunking/merge bug). + readme.write_text("# Readme\nEdited content\n", encoding="utf-8") + + def _mock_llm_run2(files, **kwargs): + on_chunk = kwargs.get("on_chunk_done") + res = { + "nodes": [ + {"id": "readme_doc_renamed", "label": "Readme Renamed", "file_type": "document", "source_file": "README.md"}, + ], + "edges": [], "hyperedges": [], + "input_tokens": 10, "output_tokens": 5, "uncovered_files": [], + } + if on_chunk: + on_chunk(0, 1, res) + return res + + monkeypatch.setattr(llmmod, "extract_corpus_parallel", _mock_llm_run2) + + import graphify.build as buildmod + real_build_merge = buildmod.build_merge + + def _build_merge_and_drop_guide(*args, **kwargs): + G = real_build_merge(*args, **kwargs) + for nid in [n for n, d in G.nodes(data=True) if d.get("source_file") == "GUIDE.md"]: + G.remove_node(nid) + return G + + # `dispatch_command` imports build_merge locally at call time (`from + # graphify.build import build_merge as _build_merge`), so patching the + # module attribute it reads from -- not a graphify.cli name -- is what + # actually takes effect. + monkeypatch.setattr(buildmod, "build_merge", _build_merge_and_drop_guide) + monkeypatch.setattr(mainmod.sys, "argv", ["graphify", "extract", str(corpus), "--backend", "claude"]) + + code2 = _run_cli() + # Total loss (5 -> 1) exceeds README.md's own reported loss (3 -> 1 = 2), + # since GUIDE.md's 2 nodes also vanished unexplained -- still refused. + assert code2 == 1 + + err = capsys.readouterr().err assert "Refusing to overwrite" in err - # The existing graph on disk is still the healthy 5-node graph graph2 = json.loads((out_dir / "graph.json").read_text(encoding="utf-8")) assert len(graph2["nodes"]) == 5 - # Manifest is NOT stamped with the new hash for README.md manifest2 = json.loads((out_dir / "manifest.json").read_text(encoding="utf-8")) assert manifest2["README.md"]["semantic_hash"] == initial_hash +def test_3203_e2e_no_cluster_self_explained_shrink_proceeds(tmp_path, monkeypatch, capsys): + """RT-parity: the --no-cluster raw write path has its own inline copy of + this guard (it never calls to_json), so #3412's relaxation must be + applied there too, not just on the clustered path.""" + corpus = tmp_path / "repo" + corpus.mkdir() + readme = corpus / "README.md" + guide = corpus / "GUIDE.md" + readme.write_text("# Readme\nInitial content\n", encoding="utf-8") + guide.write_text("# Guide\nGuide content\n", encoding="utf-8") + + out_dir = corpus / "graphify-out" + + def _mock_llm_run1(files, **kwargs): + on_chunk = kwargs.get("on_chunk_done") + res = { + "nodes": [ + {"id": "readme_doc", "label": "Readme", "file_type": "document", "source_file": "README.md"}, + {"id": "readme_sec1", "label": "Section 1", "file_type": "document", "source_file": "README.md"}, + {"id": "readme_sec2", "label": "Section 2", "file_type": "document", "source_file": "README.md"}, + {"id": "guide_doc", "label": "Guide", "file_type": "document", "source_file": "GUIDE.md"}, + {"id": "guide_sec1", "label": "Guide Sec", "file_type": "document", "source_file": "GUIDE.md"}, + ], + "edges": [], "hyperedges": [], + "input_tokens": 10, "output_tokens": 10, "uncovered_files": [], + } + if on_chunk: + on_chunk(0, 1, res) + return res + + monkeypatch.setattr(llmmod, "extract_corpus_parallel", _mock_llm_run1) + monkeypatch.setattr(mainmod, "_check_skill_version", lambda _: None) + monkeypatch.setenv("ANTHROPIC_API_KEY", "sk-fake") + + monkeypatch.setattr(mainmod.sys, "argv", + ["graphify", "extract", str(corpus), "--backend", "claude", "--no-cluster"]) + assert _run_cli() == 0 + + graph1 = json.loads((out_dir / "graph.json").read_text(encoding="utf-8")) + assert len(graph1["nodes"]) == 5 + + readme.write_text("# Readme\nEdited content\n", encoding="utf-8") + + def _mock_llm_run2(files, **kwargs): + on_chunk = kwargs.get("on_chunk_done") + res = { + "nodes": [ + {"id": "readme_doc_renamed", "label": "Readme Renamed", "file_type": "document", "source_file": "README.md"}, + ], + "edges": [], "hyperedges": [], + "input_tokens": 10, "output_tokens": 5, "uncovered_files": [], + } + if on_chunk: + on_chunk(0, 1, res) + return res + + monkeypatch.setattr(llmmod, "extract_corpus_parallel", _mock_llm_run2) + monkeypatch.setattr(mainmod.sys, "argv", + ["graphify", "extract", str(corpus), "--backend", "claude", "--no-cluster"]) + + code2 = _run_cli() + assert code2 == 0 + + err = capsys.readouterr().err + assert "unverified semantic shrink detected for 'README.md' (3 -> 1 nodes)" in err + + graph2 = json.loads((out_dir / "graph.json").read_text(encoding="utf-8")) + assert len(graph2["nodes"]) == 3 + + def test_3203_allow_partial_override_permits_intentional_reduction(tmp_path, monkeypatch, capsys): """Passing --allow-partial permits an intentional semantic reduction and stamps manifest.""" corpus = tmp_path / "repo"