From bbc03dfdff92c31c028cb053120c2dd1fbc629e0 Mon Sep 17 00:00:00 2001 From: ProtocolWarden <32967198+ProtocolWarden@users.noreply.github.com> Date: Tue, 4 Aug 2026 16:08:13 -0400 Subject: [PATCH] fix(reconcile): make the prune plan describe what apply actually does MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #54. `prune --dry-run` printed N moves and `--apply` archived N plus every unclaimed section past `recent_n`. The extra sections were computed inside `_apply_plan_locked` and never reached `plan.moves`, which is all `format_plan` renders — so nothing reported them, before or after. The operator approved one thing and got another. What makes it bite rather than merely surprise: those sections are claimed by no item AND cleared by no gate. The DOC GAP gate only inspects *done* items, so anything still `partial` is never examined — and `owned_done` excludes it too, so its sections are never claimed and land in the age-swept bucket. Incomplete, undocumented work is therefore the MOST likely thing to be archived by age, which inverts what the gate is for. Destination is the private manifest, so on a public repo the content also changes repos on its way out. Three changes, no policy change: - `build_plan` records the age-swept sections as `PlannedTrim`, and `format_plan` prints them under their own heading, named for what they are. A separate type from `PlannedMove` on purpose: a move was released by the gate, a trim was swept by a number, and conflating them in the thing an operator approves is how this stayed invisible. - `PrunePlan` carries the `recent_n` it was built with, and `apply_plan` defaults to it. The two defaulted independently, so planning with `--recent 20` sweeps to 10 at apply time — a second, quieter divergence. An explicit `recent_n=` still overrides. - `is_noop` deliberately keeps ignoring `trims`: age-sweeping is a side effect of an otherwise-legitimate prune, never a reason for one. Counting trims there would start pruning repos that are today correctly left alone. There is a test pinning that. Not addressed here, because they are policy and belong to the spec owner: whether `recent_n` should sweep unclaimed sections at all, and whether the reconcile and trim operations should be separable. #54 lists both; this change only makes the current behaviour honest. 5 tests. Each of the three changes was reverted in turn to confirm a named test fails — the end-to-end one, which diffs the dry-run text against the headings that actually land in the archive, catches two of them on its own. Suite 493 -> 498 passed; the 17 failures are unchanged and identical. No narrative log entry, following #53 and #55: this repo's log is at 400/400, so adding one would trip RC1 and force a prune to land a commit about pruning. Co-Authored-By: Claude Opus 5 --- src/context_lifecycle/reconcile/prune.py | 56 +++++++++++++- tests/test_reconcile_prune.py | 96 ++++++++++++++++++++++++ 2 files changed, 149 insertions(+), 3 deletions(-) diff --git a/src/context_lifecycle/reconcile/prune.py b/src/context_lifecycle/reconcile/prune.py index 786a1ca..aa10987 100644 --- a/src/context_lifecycle/reconcile/prune.py +++ b/src/context_lifecycle/reconcile/prune.py @@ -58,6 +58,21 @@ class PlannedMove: matched_item: str # item id that claimed this section +@dataclass +class PlannedTrim: + """A log section archived for AGE, not because an item claimed it. + + Distinct from ``PlannedMove`` on purpose. A move is released by the DOC GAP + gate — a done item with a durable doc. A trim is swept by ``recent_n`` and + no gate has looked at it, so the two must not be conflated in the plan the + operator approves. + """ + + source: str # always "log.md" — backlog trims by section kind, not age + heading: str + reason: str + + @dataclass class PrunePlan: repo: str @@ -68,9 +83,18 @@ class PrunePlan: applied: bool = False noop: bool = False messages: list[str] = field(default_factory=list) + # Sections apply() will also archive by age. Recorded so the dry-run + # describes what apply actually does; see _retain_recent_log. + trims: list[PlannedTrim] = field(default_factory=list) + # The recent_n this plan was computed with, so apply() can honour it + # instead of silently re-deciding with its own default. + recent_n: int = DEFAULT_RECENT_N @property def is_noop(self) -> bool: + # Deliberately ignores `trims`: age-sweeping only ever happens as a side + # effect of an otherwise-legitimate prune. Counting trims here would + # start pruning repos that today are correctly left alone. return not self.moves @@ -138,7 +162,9 @@ def build_plan( cutoff = cutoff or date.today().isoformat() archive_dir = archive_dir_for(ws.repo, private_root=private_root) - plan = PrunePlan(repo=ws.repo, cutoff=cutoff, archive_dir=archive_dir) + plan = PrunePlan( + repo=ws.repo, cutoff=cutoff, archive_dir=archive_dir, recent_n=recent_n, + ) scrub_records = not _repo_is_private(ws.repo, vocabulary, repo_root) owned_done = [it for it in ws.items if it.is_done and not it.is_cross_repo(ws.repo)] @@ -165,6 +191,18 @@ def build_plan( _changelog_entry(item, cutoff, vocabulary, scrub=scrub_records) ) + # Apply also archives unclaimed sections past `recent_n`. Record them + # here or the dry-run under-reports: those sections are claimed by no + # item and cleared by no gate, so they were invisible until the file + # changed underneath the operator. + claimed_headings = {sec.heading for i, sec in enumerate(sections) if i in claimed} + _, would_archive = _retain_recent_log(sections, claimed_headings, recent_n) + plan.trims = [ + PlannedTrim("log.md", sec.heading, f"beyond --recent {recent_n}") + for sec in would_archive + if sec.heading not in claimed_headings + ] + # --- backlog.md: completed ("Done"/"Done (...)") sections are # archive-eligible (active In Progress / Up Next / Recent stay) ------ backlog_path = repo_root / BACKLOG_RELPATH @@ -206,7 +244,7 @@ def apply_plan( repo_root: Path, plan: PrunePlan, *, - recent_n: int = DEFAULT_RECENT_N, + recent_n: int | None = None, vocab: ScrubVocabulary | None = None, ) -> PrunePlan: """Execute ``plan``: append to archive, trim source, append CHANGELOG. Idempotent. @@ -220,10 +258,16 @@ def apply_plan( (raises :class:`~context_lifecycle.reconcile.lock.PruneLockHeld` if another apply holds it) — see ``reconcile.lock`` for the same-host vs cross-host rationale. + + ``recent_n`` defaults to the value the plan was built with. It used to + default independently, so applying a plan built with ``--recent 20`` swept + to 10 anyway and archived ten sections the preview never mentioned. Pass it + explicitly only to override the plan on purpose. """ repo_root = Path(repo_root) + effective_n = plan.recent_n if recent_n is None else recent_n with reconcile_lock(repo_root): - return _apply_plan_locked(repo_root, plan, recent_n=recent_n, vocab=vocab) + return _apply_plan_locked(repo_root, plan, recent_n=effective_n, vocab=vocab) def _apply_plan_locked( @@ -434,6 +478,12 @@ def format_plan(plan: PrunePlan, *, applied: bool) -> str: lines.append(" planned moves:") for m in plan.moves: lines.append(f" - {m.source}: '{m.heading}' (item {m.matched_item})") + if plan.trims: + # Listed apart from moves, and named for what they are: no item claimed + # these and no gate cleared them — they go purely because they are old. + lines.append(" also archived by age (claimed by no item):") + for t in plan.trims: + lines.append(f" - {t.source}: '{t.heading}' ({t.reason})") if plan.changelog_lines: lines.append(" CHANGELOG additions:") for c in plan.changelog_lines: diff --git a/tests/test_reconcile_prune.py b/tests/test_reconcile_prune.py index ec6e286..b603c18 100644 --- a/tests/test_reconcile_prune.py +++ b/tests/test_reconcile_prune.py @@ -13,6 +13,7 @@ PruneRefused, apply_plan, build_plan, + format_plan, ) from context_lifecycle.reconcile.scrub import load_scrub_vocabulary @@ -307,3 +308,98 @@ def test_private_manifest_root_prune_keeps_names(tmp_path, monkeypatch): assert SCRUB_NAME in changelog assert "a private downstream repo" not in retained + changelog + + +# --- #54: the dry-run must describe what apply actually does --------------- + + +def _archived_headings(plan) -> set[str]: + """H2 headings that actually landed in the archive file.""" + archive = plan.archive_dir / f"log-{plan.cutoff}.md" + if not archive.is_file(): + return set() + return { + line[3:].strip() + for line in archive.read_text(encoding="utf-8").splitlines() + if line.startswith("## ") + } + + +class TestPlanReportsAgeTrims: + """`_retain_recent_log` archives unclaimed sections past ``recent_n``. + + Those sections are claimed by no item and cleared by no gate — the DOC GAP + gate only inspects *done* items, so anything still `partial` is never + examined and lands here purely for being old. They were computed during + apply and never reached the plan, so the preview under-reported and the + operator learned about them when the file changed. + """ + + def _prep(self, tmp_path, monkeypatch): + monkeypatch.delenv("REPOGRAPH_BOUNDARY_ARTIFACT_FILE", raising=False) + monkeypatch.delenv("PRIVATE_MANIFEST_DIR", raising=False) + return _setup_repo(tmp_path), tmp_path / "Private" + + def test_plan_records_the_age_swept_sections(self, tmp_path, monkeypatch): + repo, private = self._prep(tmp_path, monkeypatch) + plan = build_plan(repo, recent_n=10, private_root=private) + # 12 unclaimed entries, keep 10 → 2 swept by age. + assert len(plan.trims) == 2 + assert all(t.source == "log.md" for t in plan.trims) + assert all("recent 10" in t.reason for t in plan.trims) + + def test_dry_run_names_every_section_apply_archives(self, tmp_path, monkeypatch): + """The regression: preview and apply must agree, section for section.""" + repo, private = self._prep(tmp_path, monkeypatch) + plan = build_plan(repo, recent_n=10, private_root=private) + preview = format_plan(plan, applied=False) + + apply_plan(repo, plan) + + for heading in _archived_headings(plan): + assert heading in preview, f"apply archived '{heading}', preview never mentioned it" + + def test_apply_honours_the_plans_recent_n(self, tmp_path, monkeypatch): + """Planning with --recent 20 must not sweep to 10 at apply time.""" + repo, private = self._prep(tmp_path, monkeypatch) + plan = build_plan(repo, recent_n=20, private_root=private) + assert plan.trims == [], "12 unclaimed entries are all within 20" + + apply_plan(repo, plan) # no recent_n → must use the plan's 20 + + remaining = (repo / ".console" / "log.md").read_text(encoding="utf-8") + assert remaining.count("misc entry") == 12, "an unclaimed entry was swept anyway" + + def test_explicit_recent_n_still_overrides_the_plan(self, tmp_path, monkeypatch): + """The override stays available — it just is not the silent default.""" + repo, private = self._prep(tmp_path, monkeypatch) + plan = build_plan(repo, recent_n=20, private_root=private) + + apply_plan(repo, plan, recent_n=5) + + remaining = (repo / ".console" / "log.md").read_text(encoding="utf-8") + assert remaining.count("misc entry") == 5 + + def test_trims_alone_do_not_make_a_noop_plan_act(self, tmp_path, monkeypatch): + """Age-sweeping is a side effect of a real prune, never a reason for one. + + A repo with nothing claimable must stay a no-op however old its log is, + or upgrading would start pruning repos that are currently left alone. + """ + monkeypatch.delenv("REPOGRAPH_BOUNDARY_ARTIFACT_FILE", raising=False) + monkeypatch.delenv("PRIVATE_MANIFEST_DIR", raising=False) + repo = tmp_path / "Quiet" + console = repo / ".console" + console.mkdir(parents=True) + (console / "reconcile.yaml").write_text("repo: Quiet\nitems: []\n", encoding="utf-8") + log = "# Log\n\n" + "".join( + f"## 2026-05-{10 + i:02d} — entry {i}\n\nBody {i}.\n\n" for i in range(12) + ) + (console / "log.md").write_text(log, encoding="utf-8") + + plan = build_plan(repo, recent_n=10, private_root=tmp_path / "Private") + assert plan.is_noop + + apply_plan(repo, plan) + + assert (console / "log.md").read_text(encoding="utf-8") == log