diff --git a/graphify/watch.py b/graphify/watch.py index 9943b5caf0..6ffb13cf33 100644 --- a/graphify/watch.py +++ b/graphify/watch.py @@ -1130,6 +1130,7 @@ def _check_shrink( had_explicit_deletions: bool = False, rebuilt_sources: "set[str] | None" = None, failed_sources: "set[str] | None" = None, + deleted_sources: "set[str] | None" = None, ) -> bool: """Return True (ok to proceed) or False (shrink refused). @@ -1151,6 +1152,17 @@ def _check_shrink( ``--force`` (#1116 left stale nodes write-blocked even though build dropped them). Files in ``failed_sources`` never account for lost nodes: extraction did not complete, so their disappearance is the silent shrink this guard protects. + + Membership in ``rebuilt_sources`` alone is not evidence that a loss was + intended. On a full rebuild it is every file in the corpus, so the check + below degenerated to "always allow" on exactly the path the refusal + message recommends as the remedy (#3579). A re-extracted source therefore + accounts for its own lost nodes only while it still contributes something + to the new graph: a source that goes from N nodes to none while still on + disk is the silent shrink, not a refactor. Files named in + ``deleted_sources`` are exempt — they are gone, so contributing nothing is + the correct outcome. An emptied-but-present file keeps its file node, so + this does not refuse a legitimate "removed every symbol" edit. """ if force or not existing_data: return True @@ -1173,13 +1185,23 @@ def _check_shrink( new_ids = {n.get("id") for n in new_nodes} lost = [n for n in existing_nodes if n.get("id") not in new_ids] + surviving_sources = { + _norm_source_file(sf) + for n in new_nodes + if (sf := n.get("source_file")) + } + gone = {_norm_source_file(sf) for sf in (deleted_sources or set())} + def _accounted(n: dict) -> bool: sf = n.get("source_file") - if sf and failed_sources and _norm_source_file(sf) in failed_sources: + if not sf: + return True + norm = _norm_source_file(sf) + if failed_sources and norm in failed_sources: + return False + if sf not in rebuilt_sources and norm not in rebuilt_sources: return False - return (not sf - or sf in rebuilt_sources - or _norm_source_file(sf) in rebuilt_sources) + return norm in surviving_sources or norm in gone if all(_accounted(n) for n in lost): return True if tmp is not None: @@ -1888,6 +1910,7 @@ def _failed(f: str) -> bool: had_explicit_deletions=bool(deleted_paths), rebuilt_sources=rebuilt_sources, failed_sources=failed_sources, + deleted_sources=set(deleted_paths), ): return False from graphify.export import backup_if_protected as _backup @@ -2099,6 +2122,7 @@ def _failed(f: str) -> bool: had_explicit_deletions=bool(deleted_paths), rebuilt_sources=rebuilt_sources, failed_sources=failed_sources, + deleted_sources=set(deleted_paths), ): return False from graphify.exporters.html import _HTML_STALE_MARKER diff --git a/tests/test_watch.py b/tests/test_watch.py index a056538f44..31179e9b6e 100644 --- a/tests/test_watch.py +++ b/tests/test_watch.py @@ -4489,3 +4489,95 @@ def test_clustered_rebuild_survives_a_permission_error_on_replace(tmp_path, monk graph_path = corpus / "graphify-out" / "graph.json" labels = {n["label"] for n in json.loads(graph_path.read_text(encoding="utf-8"))["nodes"]} assert "added()" in labels, "the fallback must still land the new content" + + +# --- _check_shrink on a full rebuild (#3579) --- + + +def _sourced(*pairs: "tuple[str, int]") -> dict: + """Graph data whose nodes carry a source_file: (path, node count) per source.""" + nodes = [] + for source, count in pairs: + for i in range(count): + nodes.append({"id": f"{source}:{i}", "source_file": source}) + return {"nodes": nodes, "links": []} + + +def test_check_shrink_refuses_a_source_that_produced_nothing_on_a_full_rebuild(capsys): + """A full rebuild re-extracts everything, so membership proves nothing. + + `rebuilt_sources` is the whole corpus there, which made every lost node + "accounted" and the guard unable to refuse on exactly the path its own + refusal message recommends as the remedy. + """ + ok = _check_shrink( + force=False, + existing_data=_sourced(("a.py", 3), ("b.py", 2)), + new_data=_sourced(("b.py", 2)), + rebuilt_sources={"a.py", "b.py"}, + ) + assert ok is False + assert "Refusing to overwrite" in capsys.readouterr().err + + +def test_check_shrink_still_allows_a_symbol_removed_from_a_rebuilt_source(capsys): + """The #1116 case must keep working: the file is still there, with fewer nodes.""" + ok = _check_shrink( + force=False, + existing_data=_sourced(("a.py", 3), ("b.py", 2)), + new_data=_sourced(("a.py", 1), ("b.py", 2)), + rebuilt_sources={"a.py", "b.py"}, + ) + assert ok is True + assert "Refusing to overwrite" not in capsys.readouterr().err + + +def test_check_shrink_allows_a_deleted_source_to_contribute_nothing(): + """A file that is gone from disk is meant to contribute nothing.""" + ok = _check_shrink( + force=False, + existing_data=_sourced(("a.py", 3), ("b.py", 2)), + new_data=_sourced(("b.py", 2)), + rebuilt_sources={"a.py", "b.py"}, + deleted_sources={"a.py"}, + ) + assert ok is True + + +def test_check_shrink_refuses_a_loss_from_a_source_it_never_rebuilt(capsys): + """The incremental case the guard was written for. + + A file that was not re-extracted has no reason to lose nodes: it was + preserved from the previous graph verbatim. Losing some of them is the + silent shrink, and membership in `rebuilt_sources` is what separates it + from a symbol legitimately removed from a file that WAS re-extracted. + """ + ok = _check_shrink( + force=False, + existing_data=_sourced(("touched.py", 2), ("untouched.py", 5)), + new_data=_sourced(("touched.py", 2), ("untouched.py", 2)), + rebuilt_sources={"touched.py"}, + ) + assert ok is False + assert "Refusing to overwrite" in capsys.readouterr().err + + +def test_check_shrink_still_refuses_a_failed_source(): + """failed_sources keeps its precedence: extraction did not complete.""" + ok = _check_shrink( + force=False, + existing_data=_sourced(("a.py", 3), ("b.py", 2)), + new_data=_sourced(("a.py", 1), ("b.py", 2)), + rebuilt_sources={"a.py", "b.py"}, + failed_sources={"a.py"}, + ) + assert ok is False + + +def test_check_shrink_still_allows_a_loss_with_no_source_file(): + """Nodes without a source_file were always accounted and still are.""" + existing = {"nodes": [{"id": "x"}, {"id": "y"}, {"id": "z"}], "links": []} + new = {"nodes": [{"id": "x"}], "links": []} + assert _check_shrink( + force=False, existing_data=existing, new_data=new, rebuilt_sources={"a.py"} + ) is True