# Design: Clean ## Context The check pipeline (build step #3) is in place: `scanner.py` + `report_builder.py` + `validate_report.py` + `state_store.py` together produce a schema-valid `.dochygiene/report.json` and `report.md`, then stamp `last_check`. The applier is also in place: `patch_applier.py` exists, has 263 passing tests, and knows how to apply every kind in the `KIND_TABLE` via per-file transactions with a content-hash guard. What is missing is the orchestration skill that loads that report, gates dangerous entries, invokes the applier, handles generative entries, and produces a single reviewable git commit. Constraints that shape this design: - **Invariant #4 (no accumulating artifacts):** keep only the latest report pair. The clean skill must not write new state files; `last_clean` is the only new state entry. - **Invariant #5 (git-safe single commit):** clean/committed tree required or an auto WIP checkpoint; the cleanup output is exactly one commit. NEVER two commits. - **Invariant #6 (deterministic-first):** apply, stage, and commit are scripts. Generative distillation is the only model pass during clean. - **Invariant #7 (safety-tier gating):** `auto` ops apply without prompt; `confirm` ops escalate. Gating does NOT relax under `sweep`. - **Invariant #8 (content guard):** the applier reads `expected_sha256` from the report; if a file changed since check, the entire file batch is skipped (not just the conflicting entry). The guard is a content hash, not mtime — but per the META-RULE, invariant #8's wording ("mtime guard") must not change without human approval. ## Goals / Non-Goals **Goals:** - A `hygiene-clean` skill that loads the report, gates confirms, runs the applier, handles generative entries via Sonnet, stages precisely, commits once, and stamps `last_clean`. - A `workflows/distill.md` Sonnet workflow for generative entries (LOOP-GUARD via subagent pointer). - Live routing in `commands/hygiene.md` for `clean` and `sweep` (replacing Phase 3 stubs). - Integration tests covering the invariant #5 and #7 GAPs identified in the design. **Non-Goals:** - `token_estimate` weighting (`injection_frequency`, `weighted_tokens`, rollup) — v2 bonus (PRD phase 5). - Inbound-link rewriting after `move-to-archive` — v1 move only; next check flags new orphans. - The live model-classification regression harness — separate, manually invoked. ## Decisions ### D1. Per-file transaction with descending-anchor application — the content-hash guard lives in `patch_applier.py` The core correctness invariant: applying edits one-at-a-time to a file is wrong because each write shifts line numbers, causing subsequent anchors to land on the wrong lines and potentially tripping the guard on stale content. The resolution: 1. **Read file bytes once.** 2. **Verify `sha256(bytes) == expected_sha256`** for every anchored entry on that file. On any mismatch, skip the **entire file batch** (reason: `content-changed-since-check`, recommendation: re-analysis) and continue other files. 3. **Apply anchored edits in memory, descending by `anchor.start_line`** — edits to later lines first so earlier-line anchors remain valid. 4. **Apply `insert-frontmatter` last** (it prepends, shifting all lines; doing it last means descending-anchor edits see the correct line numbers). 5. **Write once; stage once.** The guard is a content hash only — no mtime pre-filter. Per-entry `generated_at` in the report records the file's hash instant (distinct from the envelope/`last_check` stamp); the applier does not use `generated_at`, only `expected_sha256`. **Alternative considered:** apply edits sequentially with line-offset tracking — rejected: complex, error-prone across multiple edit kinds, and untestable in isolation. ### D2. `insert-frontmatter`: re-derive at apply time (resolves the deferred hole from `add-check`) The `add-check` design recorded a deferred hole: `insert-frontmatter` has `has_anchor = False` and therefore carries **no** `expected_sha256`, so invariant #8 cannot protect it at apply time via the normal content-hash path. Resolution (implemented in `patch_applier.py`): at apply time, re-read the file and parse its frontmatter fresh: - Key already present with the target value → **idempotent no-op** (success). - Key already present with a different value → **skip** with reason `frontmatter-key-conflict`; NEVER overwrite. - Key absent → insert `key: value` (creating a `---` block if needed) as the **last** in-memory step (after all descending-anchor edits). This re-derivation replaces the missing hash guard for frontmatter; it is idempotent and does not trust any cached state. This resolves the deferred hole noted in `add-check` and in `CLAUDE.md`. ### D3. Incompatible-ops detection — skip the file, never compose Per file, before any in-memory editing, the applier checks: - Does `move-to-archive` co-occur with any other op on the same file? - Do any anchor ranges overlap? If either is true, skip the entire file with reason `incompatible-ops-on-file` and record a recommendation for re-analysis. V1 never attempts to compose or sequence these — the interaction is undefined and the risk of silent corruption is too high. **Alternative considered:** attempt partial application (apply non-conflicting ops) — rejected: partial application of overlapping anchors produces unpredictable results and leaves the file in a state that cannot be re-analyzed cleanly. ### D4. Safety-tier gating — one batch-confirm before any mutation Invariant #7 requires `auto` ops to apply without prompt and `confirm` ops to escalate. The skill reads `safety_tier` from the report (derived by `report_builder` via `derive_safety_tier`) — never recomputes or trusts a model. Gating sequence: 1. Partition entries into `auto+deterministic` → mechanical; `confirm+deterministic` (i.e., `delete-range`) → approval; `generative` (always `confirm`) → approval. 2. **Gate confirm-tier FIRST**, before any mutation: present a single batch-confirm list (path · category · op · tokens · why) with per-entry opt-out. Visually distinguish `delete-range` (irreversible) from reversible `auto` ops. 3. Approved set = all `auto` entries + approved `confirm` entries. 4. Apply in that order. Under `sweep` (`/hygiene sweep` = check-then-clean), the confirm gate runs identically — the convenience path does NOT bypass invariant #7. Audit of approvals is folded into the commit body (no separate `.dochygiene/decisions` file — avoids new artifact, respects invariant #4). ### D5. Git safety — WIP checkpoint, single commit, precise staging, rollback **Pre-flight:** 1. Resolve `project_root` via `StateStore`. 2. `git status --porcelain`. Clean → `baseline = HEAD`. Dirty → auto-create a WIP checkpoint commit of user work FIRST (per invariant #5 "or auto WIP checkpoint"); `baseline = that checkpoint`. The cleanup commit is still exactly one. 3. Tracked-files-only: untracked candidate docs are skipped and reported (they have no git history to roll back to). **Staging (precise):** - Non-move ops: applier writes the file; skill calls `git add `. - `move-to-archive`: applier calls `git mv` (stages both source removal and dest add); skill must NOT `git add` the dest again (double-add corrupts the index). - NEVER `git add -A` or `git add .` (the report lives in gitignored `.dochygiene/` and must never enter the commit). **Single commit** via `git-context commit-apply --message-stdin` (gem 0.4.0). The commit message format: ``` docs: hygiene cleanup (N edits across M files) Auto: (move-to-archive ×a, replace-text ×b, dedupe ×c, freeze ×d) Confirmed: (delete-range ×e, distill ×f, split ×g) Skipped (re-analysis recommended): ``` No `Co-Authored-By`, no AI attribution, no emoji, no trailers (house rule confirmed by user). Do NOT delegate to the full commit skill (its grouping/secret-scan logic may split into multiple commits, violating invariant #5). **Failure handling:** - Hard failure (applier exit 2, `git mv` fail, write error) → `git restore`/`reset` to `baseline`; abort with a structured error. - Partial success (applier exit 1, some file batches guard-skipped) is NOT a rollback — commit what applied, report skipped files with `re-analysis recommended`. **Mid-run `last_clean` stamp:** stamped only after the commit succeeds, to the commit instant (not the run-start instant, so `last_clean` always points at an actual commit). ### D6. Generative entries — live-read freshness, Sonnet subagent, skill writes Generative entries (`distill`, `split`, and any unknown `op_type`) always require `confirm` gating. After approval, the skill (not the applier) handles them: 1. Confirm the file still exists (a `move-to-archive` for the same file earlier in the run would make it gone). 2. Read the **live** file contents now (not from the report cache — generative entries have no `expected_sha256`, so the only freshness guarantee is a live read). 3. Dispatch to a Sonnet subagent pointing at `workflows/distill.md` (LOOP-GUARD — the subagent reads that workflow doc, not SKILL.md, to avoid recursion). 4. Subagent returns new prose (or new-primary + archived-section for split). 5. Skill writes + stages the result. A file with BOTH a generative entry AND a deterministic entry is treated as having `incompatible-ops-on-file` (see D3) — v1 never composes; re-analysis recommended. ### D7. `sweep` is command-level sequencing, not a third skill `/hygiene sweep` → invoke `hygiene-check` skill, then invoke `hygiene-clean` skill, passing `--scope`/`--category` to both. This is wired in `commands/hygiene.md`, not in a third skill or by having `clean` call `check` internally. No double-commit: `check` writes only to the gitignored `.dochygiene/`; the single clean commit is the only git commit produced by the sweep. **Alternative considered:** a `hygiene-sweep` skill — rejected: `sweep` is purely `check ; clean` with shared flags; a dedicated skill adds ceremony with no new logic. ## Risks / Trade-offs - **[delete-range is irreversible]** → Confirm-gated (invariant #7); the batch-confirm display MUST visually distinguish deletes from reversible auto ops. Mitigated by the WIP checkpoint (user can always `git reset --hard `) but the content is gone from that commit. Documented. - **[move-to-archive creates link orphans]** → V1 move only; the next `/hygiene check` flags new orphans via the `broken_reference` signal. Documented in the clean summary as a follow-up action. - **[generative second-guard hole]** → Generative entries have no `expected_sha256`; freshness is guaranteed only via the live read at dispatch time. Do not cache the span between the report and the Sonnet call. If the file changes between skill start and Sonnet dispatch, the live read picks up the change (safe); do not add a pre-write hash check (would need a second read, adding complexity with minimal benefit in practice). - **[file with both generative + deterministic entries]** → Incompatible-ops-on-file; entire file is skipped, re-analysis recommended. V1 never composes. - **[`insert-frontmatter` key conflict]** → Preserved (skip + `frontmatter-key-conflict` reason), never overwritten. The idempotent re-derivation in D2 covers this. - **[tracked-files-only]** → Untracked candidate docs are skipped and reported. A user who adds a doc without `git add`-ing it won't see it cleaned. Documented. - **[invariant #8 naming vs. mechanism]** → Invariant #8 is titled "mtime guard" but the enforcement is a whole-file content sha256. The applier docstring notes this; the invariant wording is not changed without human approval (META-RULE). ## Migration Plan Additive only — no existing behavior changes. Deploy order: 1. `workflows/distill.md` (no dependencies beyond existing scripts). 2. `skills/hygiene-clean/SKILL.md` (depends on `distill.md` and the already-built `patch_applier.py`). 3. Integration tests (`tests/test_clean_integration.py`). 4. `commands/hygiene.md` update (stub replacement only — replaces the "Phase 4" stubs; no structural change to the command file). 5. CONTEXT.md updates (`scripts/CONTEXT.md`, `skills/hygiene-clean/CONTEXT.md`). Rollback: remove `skills/hygiene-clean/SKILL.md`, `workflows/distill.md`, and the integration tests; revert `commands/hygiene.md` to Phase 3 stubs. The deterministic core, report-builder, state-store, and `patch_applier.py` are untouched. ## Open Questions (resolved — recorded as decisions) 1. **Insert-frontmatter at apply time** → re-derive at apply time (D2); the deferred hole from `add-check` is now resolved. 2. **Guard mechanism** → content hash only (`expected_sha256`), no mtime pre-filter. Invariant #8 wording preserved (META-RULE). 3. **Commit attribution** → NONE (`git-context` house rule, user-confirmed). No `Co-Authored-By`, no AI attribution, no emoji. 4. **Generative freshness** → live read at dispatch time is the only guard; no cached span (D6). 5. **Sweep is command-level** → wired in `commands/hygiene.md`, not a third skill (D7). 6. **Partial success** → NOT a rollback; commit what applied, report skipped (D5). 7. **Audit trail** → folded into the commit body; no `.dochygiene/decisions` file (respects invariant #4). 8. **`git mv` double-staging** → the applier stages `git mv` both sides; skill must NOT `git add` the dest path again (D5).