From 0998e2bd55716e62123ac2c535f826f6cdd1ea97 Mon Sep 17 00:00:00 2001 From: L4XB Date: Tue, 15 Sep 2026 21:56:35 +0200 Subject: [PATCH] fix(watch): let the shrink guard refuse on a full rebuild On a full rebuild `rebuilt_sources` is every file in the corpus, so `_accounted` returned True for every lost node and `_check_shrink` could never refuse. The protection was structurally absent on exactly the path its own refusal message recommends as the remedy. Membership in `rebuilt_sources` says a file was scanned, not that losing its nodes was intended. A re-extracted source now accounts for its own losses 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 this guard exists for. Files the caller declared deleted stay exempt, since contributing nothing is the correct outcome for them. An emptied-but-present file keeps its file node, so a legitimate "removed every symbol" edit still passes without --force. --- graphify/watch.py | 32 ++++++++++++++-- tests/test_watch.py | 92 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 120 insertions(+), 4 deletions(-) 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