cc-os/plugins/os-doc-hygiene/openspec/changes/lifecycle-aware-doc-hygiene/design.md

392 lines
24 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters

This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.

# Design: lifecycle-aware-doc-hygiene
## Context
os-doc-hygiene today only monitors stale/bloated docs (`doc-check`,
`doc-clean`, `report-schema` specs, already shipped). The locked lifecycle
design (`plugins/os-doc-hygiene/lifecycle-spec.md`, wayfinder map #31, ADRs
00380041) extends it to *lifecycle management*: every managed file gets a
lifetime (`keep` / `temporary` / `delete-once-served`, plus an `extract`
modifier), rules live in a rulebook, and a new `:calibrate` skill learns rules
per project. This design translates the locked spec into the concrete seams
of the existing pipeline (`scanner.py` → subagent classification →
`report_builder.py``validate_report.py``patch_applier.py`) without
reopening any decision already recorded in the spec or its ADRs.
## Goals / Non-Goals
**Goals**
- Add a stdlib-only `rulebook.py` loader (global + per-project override,
add-only merge, `glob.translate` dialect) consumed by the scanner.
- Give the scanner a lifecycle signal class where a directory-rule match
prunes the walk — this is simultaneously the IGNORE-surface implementation
(no separate ignore mechanism).
- Extend `report_builder.py` / `validate_report.py` / `patch_applier.py` to
carry `delete` and `extract-then-delete` op types end-to-end under the
ADR-0039 autonomy tier matrix, with all tracked/dirty state verified at
application time via git, never trusted from the rule or the report cache.
- Implement temporary-tier age computation (git commit time, mtime fallback
for untracked files, directory-inode mtime for untracked directory entries)
and retain-recent-N semantics.
- Add `conventions.json` and wire promotion-candidate nudging into `:check`.
- Add `:calibrate` as a skill orchestrating haiku nomination + strong-model
judge subagents, per the 6-step protocol in spec §8.
- Resolve the state-dir wrinkle (below) so the shipped IGNORE surface
actually excludes the real state directory.
**Non-Goals**
- No ignore-surface propagation into `.graphifyignore`, `.dochygiene-ignore`,
or any other tool's config (ADR-0040 — closed, not revisited here).
- No bulk-clean of any project other than cc-os (calibration pass #1 target
per spec §9); no other project's rulebook is touched by this change.
- No recurring cross-project categorize-and-learn skill — that is charted as
fog on map #31 and explicitly out of scope for this design (spec, "Out of
scope" section).
- No `propagate_ignore` field, in any form (removed entirely per ADR-0040,
not left as a documented-but-unused slot).
- No new deletion destinations beyond the existing knowledge-routing targets
(ADR/CLAUDE.md/docs for repo-durable extraction, SecondBrain vault via
`/os-vault:write` for cross-repo extraction).
## Decisions
### 1. `rulebook.py` as a new stdlib-only module
A new `scripts/rulebook.py`, following the existing scripts' conventions
(small single-responsibility classes, structured JSON-serializable output, no
model, injectable for testing). Responsibilities:
- Load and parse the global `plugins/os-doc-hygiene/rulebook.json` and, if
present, the project's committed `.dochygiene-rules.json`, both under the
envelope `{"schema_version": 1, "rules": [...]}`.
- Compile every rule's `glob` once at load time via stdlib
`glob.translate(pattern, recursive=True, include_hidden=True)` (Python ≥
3.13, per spec §2) into a matchable regex; this is a hard runtime
requirement on the Python version the plugin scripts run under — no
fallback dialect is provided (matches the spec's explicit dialect pin).
- Merge add-only: project rules are appended to global rules, never replacing
or deleting a global entry. A project "neutralizes" a global rule only by
adding a shadowing rule with `lifetime: "keep"` at equal-or-higher
precedence (per the two-axis precedence below) — there is no
rule-removal mechanism, by design (ADR-0038).
- Resolve precedence per path: project file-rule > project directory-rule >
global file-rule > global directory-rule; ties within the same tier broken
by longest `glob` string, then by last-defined (definition order within the
rules array, project-before-global order does not matter once the four
buckets above are checked first).
- Validate: skip-and-warn per invalid or unconfirmed rule (a rule missing
`confirmed_by` is loaded but flagged `inactive`, never contributes a
signal, and never blocks the rest of the rulebook from loading). Hard-fail
(raise / non-zero exit) only on unparseable JSON or an unrecognized
`schema_version`.
- Expose a single query surface consumed by the scanner: given a
project-root-relative path, return either `None` (unmatched — flows
through existing signals unchanged) or the winning rule (with its
precedence-resolved fields) plus whether the match was file-level or
directory-level.
`rulebook.py` has no knowledge of git, classification, or the report schema —
it only resolves "which rule, if any, governs this path," keeping it testable
in complete isolation with fixture rulebook JSON and injected path lists, in
line with every other script in `scripts/`.
### 2. Where the lifecycle signal plugs into `scanner.py`
A new signal class, `LifecycleSignal` (or equivalent), is attached during the
existing scanner walk, alongside the current objective signals (broken refs,
version skew, etc.). Concretely:
- Before recursing into a directory, the scanner asks the loaded rulebook
whether a directory-rule matches that directory's path. If so, the walk is
**pruned** — the scanner never opens any file beneath it — and the scanner
emits **one aggregate shortlist/signal entry** for the directory path
itself (not one entry per file inside it), carrying the lifecycle signal
(rule ref, lifetime, `served_when`/`served_when_path` if present).
- This directory-rule-prune mechanism **is** the IGNORE surface described in
spec §1 and §2 — there is no separate ignore-list data structure in the
scanner. The IGNORE surface's seed members (`graphify-out/**`,
`.dochygiene/**`) are shipped as ordinary directory rules with
`lifetime: "keep"`... except `keep` implies "scanned and reported," which
contradicts "never walked." Resolution: IGNORE-surface entries are a
rulebook rule with no `lifetime` field at all (or a reserved sentinel the
scanner recognizes before even considering lifetime) whose sole effect is
walk-pruning with **no** shortlist/signal emission at all — distinct from
`keep`, which is walked and reported. See "Ambiguities resolved" below;
this is the one place the spec's wording ("directory-rule match prunes the
walk and emits one aggregate entry — this IS the ignore surface") needed
disambiguating against the separate statement that IGNORE-surface members
are "never walked" with no entry at all. This design follows the latter
(spec §1, §3 table: "IGNORE surface … never walked", no aggregate-entry
language attached to it) and reserves the "prunes the walk and emits one
aggregate entry" behavior for ordinary `temporary`/`delete-once-served`
directory rules (e.g., `autoresearch/<run-id>/`), which need a signal to
drive deletion decisions, whereas pure IGNORE members need none.
- For files matched by a file-rule (not caught by a directory prune), the
scanner attaches the lifecycle signal to that file's existing entry in the
shortlist, alongside any pre-existing objective signals — a file can be
both lifecycle-tagged and, say, stale-by-broken-ref.
- The lifecycle signal is consumed downstream by the classification subagent
as a new signal class (per doc-check's existing "signals are passed
through verbatim" contract) and ultimately drives `op`/`op_type` selection
(`delete` / `extract-then-delete`) in place of, or alongside, the existing
stale/bloat vocabulary.
### 3. Delete / extract-then-delete op flow
`report_builder.py``validate_report.py``patch_applier.py`, following
the existing division of labor (model proposes judgment fields only;
deterministic finalize pass authors the guardrail fields; validator enforces
the schema; applier mutates at clean time with runtime re-verification).
- **`report_builder.py`** gains two new `exact_edit.kind` values in its
`KIND_TABLE`: `delete` and `extract-then-delete`. Both carry an `anchor`
covering the full file (or, for a directory-rule aggregate entry, the
directory path with no anchor) so the biconditional with `op_type` holds.
`extract-then-delete` additionally carries the extraction destination
classification (`repo-durable` vs `cross-repo`) as a proposal field the
model supplies (it is a judgment about content, not a derived guardrail
field) plus, for `repo-durable`, a target doc reference; `cross-repo`
routes through `/os-vault:write` at clean time and carries no fixed
destination path (spec §1: "no new destinations").
- **`validate_report.py`**'s `derive_safety_tier` remains the single source
of tier derivation (invariant #10) and is extended, not replaced, with the
ADR-0039 matrix as additional input dimensions beyond `(op_type,
is_destructive, is_reversible)`:
- a `lifecycle` object on the entry (`rule_ref`, `lifetime`,
`served_when_path` XOR `served_when`, `git_state` placeholder recomputed
at runtime — see below)
- `derive_safety_tier` gains a lifecycle-aware branch: for `op_type` in
(`delete`, `extract-then-delete`), tier is `auto` **only** when the
entry's lifecycle evidence is `scanner-proven` (a `served_when_path` hit,
or a temporary-tier age/retain-recent computation) **and** the file's
git state is tracked+clean; every other combination (dirty, untracked,
or `served_when` classifier-judged) forces `confirm`. This is additive:
the existing non-lifecycle branches (stale/bloat ops) are unchanged, and
`derive_safety_tier` is still the one function both `report_builder.py`
and `validate_report.py` call — no second tier-deriving code path is
introduced.
- Because tracked/dirty state can only be known against the live worktree,
`report_builder.py` calls a runtime `git ls-files` + dirty check
**at report-build time** to populate the `git_state` input for tier
derivation — but per ADR-0039 this is explicitly re-verified again at
apply time by `patch_applier.py`, because time passes between check and
clean. The report's tier is advisory-fresh, not authoritative-forever.
- **`patch_applier.py`** gains handling for `delete` and
`extract-then-delete`:
- Before applying either, it re-runs `git ls-files <path>` and a dirty
check against that specific path (never trusting the report's cached
tier or git_state) — if the path is no longer tracked+clean when the
rule required that for `auto`, the applier downgrades that entry to
skip-and-report (`git-state-changed-since-check`), the same family of
guard as the existing content-hash guard, not a silent auto-apply.
- `delete` performs a true `git rm <path>` (or `git rm -r` for a
directory-rule aggregate entry) staged into the same single hygiene
commit the rest of the run produces — no archive directory, no move.
This is staged precisely like every other op (no `git add -A`).
- `extract-then-delete` first invokes the generative extraction (repo-
durable → written via the same live-read Sonnet-subagent path
`doc-clean` already uses for generative ops, target = ADR/CLAUDE.md/docs
per the model's proposal; cross-repo → `/os-vault:write` invoked by the
clean skill, not the applier itself, since vault writes are not a
filesystem op the applier owns) and only after that step succeeds does
it `git rm` the source path, both landing in the one hygiene commit.
If extraction fails, the delete does not proceed for that entry (fails
closed, consistent with the existing "hard failure → rollback,
partial-guard-skip → no rollback" split: an extraction failure is a
per-entry skip, not a run-level hard failure, unless it is the kind of
error `doc-clean`'s existing rollback rules already treat as hard).
### 4. Temporary-tier age computation
- **Age source:** git commit time of the path's most recent commit touching
it, obtained via `git log -1 --format=%cI -- <path>`; falls back to
filesystem `mtime` only when the path is untracked (no commit history to
read). No per-rule `age_source` override field exists (ADR-0039/#48).
- **Untracked directory entries** (a directory-rule match on an untracked
directory) use **one `stat()` on the directory inode itself**, not a
recursive max-mtime walk over its contents — cheap and self-healing per
spec §4/§48 (a spuriously bumped directory mtime only delays deletion by
one round, it never causes incorrect *early* deletion).
- **Retention unit** is the rule's own match granularity: a file-rule's
matches are individual files; a directory-rule's matches are whole
directories (e.g. `autoresearch/<run-id>/` — "3 most recent runs," not "3
most recent files inside the newest run"). `rulebook.py` and the scanner
must agree on this: the directory-rule aggregate shortlist entry (§2
above) is the unit `retain_recent`/`max_age_days` operate over, computed
by grouping sibling directory-rule matches under their common parent glob
and ranking by age (newest first), keeping the top `retain_recent`
regardless of age and deleting the rest once they exceed `max_age_days`.
- `retain_recent` defaults to 3, `max_age_days` defaults to 3, both per-rule
overridable in the rulebook JSON.
### 5. `conventions.json` and promotion candidates
- `plugins/os-doc-hygiene/conventions.json` is a global-only, plugin-shipped,
machine-readable file (envelope TBD-but-simple: an array of convention
objects — name, "what it proves," `served_when_path`/frontmatter template
a rule graduates to, one-line human pitch). v1 ships exactly the two
entries named in the spec (`archive-bucket`, `status-frontmatter`); no
per-project override file exists for it (ADR-0041: "the catalog only
recommends; adoption lands in the project's own rulebook").
`report_builder.py` (deterministic, no model) reads this file directly —
no subagent involvement — and, for every entry whose lifecycle signal is
classifier-judged (`served_when` present, no `served_when_path`), checks
whether any catalog convention's `served_when_path` pattern is *not yet*
satisfied by that project's current structure, and if so emits a
`promotion_candidates` entry naming the convention and the one-line pitch.
This is a deterministic string/structure check, not a judgment call, so it
belongs in `report_builder.py` alongside the other guardrail fields the
model must not author.
- The `:check` report gains a `promotion_candidates` array section at the
top level (sibling to `entries`), populated on every run where at least
one classifier-judged lifecycle signal exists and a matching catalog
convention applies. `:calibrate` (out of this change's core report path,
but sharing the same catalog) may go further and draft the adoption
(graduated rule + the file moves that convention implies) for human
approval, never applying unasked.
### 6. `:calibrate` as an orchestrating skill
New `skills/calibrate/SKILL.md` (the only new skill this change adds),
following the existing skill pattern (`check`/`clean`/`sweep`
SKILL.md + a `workflows/*.md` LOOP-GUARD subagent pointer where a subagent
must not recurse into the top-level skill file). It orchestrates, per spec
§8:
1. A deterministic clustering pass (script, no model) over the current
scanner's unmatched shortlist (unmatched = unmanaged = candidate pool),
grouping by path-shape similarity and sampling representatives per
cluster — this is a new small deterministic helper, not a new pipeline
stage in `check`/`clean`, since calibration is a separate on-demand skill.
2. A **haiku** subagent per cluster, constrained by prompt contract to
nominate a bare glob + candidate lifetime, never an exact-instance path
(the rule-quality "class, never path" test is enforced by the strong-
model judge in the next step, not trusted from haiku).
3. One batched **strong-model** (Opus/Fable) judge subagent that gathers its
own evidence (re-reads matched paths, checks near-miss boundaries) and
returns verdicts (`confirm` / `reject` / `amend` / `consult`), with
`consult` mandatory whenever an artifact's purpose is unclear.
4. A deterministic report-assembly step (script) that renders the required
5-element rule report (glob verbatim, matches with capped sample + total
count, near-miss boundary, tier, plain-language why) to the human **before
any persistence call is made** — persistence is a separate, explicit step
gated on human review of this report, not something the judge subagent
triggers directly.
5. Persistence: project-rule writes to `.dochygiene-rules.json` happen on
judge confirmation (post human rule-report review); global-rulebook
writes are additionally human-gated (a distinct confirmation, since it is
a cross-repo write into cc-os); rule removals are HITL-only in all cases.
6. A retest loop (re-run clustering against the shrunk unmatched pool) that
stops when a round nominates fewer than 2 new rules or shrinks the
unmatched pool by less than 10%, hard-capped at 3 rounds regardless.
### 7. The state-dir wrinkle
`lifecycle-spec.md`'s IGNORE seed lists `.dochygiene/**`. Since ADR-0027
(pre-dating this change), the plugin's actual state directory is
`.cc-os/dochygiene/`, with `.dochygiene/` retained only as a legacy read-
fallback that `state_store.py` migrates away from on first write; `scanner.py`
already self-excludes both `.cc-os/` and `.dochygiene/` by directory name at
any depth (per `scripts/CONTEXT.md`'s documented default excludes). This
change's rulebook-driven IGNORE surface must therefore ship **both** seed
entries — `.dochygiene/**` (matching the spec text verbatim, covering the
legacy dir on projects that haven't migrated) and an equivalent rule (or
reliance on the scanner's pre-existing hardcoded self-exclusion, which
already covers `.cc-os/**`) — so that on a project already migrated to
`.cc-os/dochygiene/`, the IGNORE surface still holds. This design does not
change the scanner's existing hardcoded self-exclusion; the rulebook's
IGNORE rule list is additive to it, not a replacement, so this is satisfied
without a code change beyond confirming the shipped `rulebook.json`'s IGNORE
entries include `.dochygiene/**` and documenting that `.cc-os/**` is already
covered by the pre-existing self-exclusion.
## Risks / Trade-offs
- **Two overlapping walk-pruning mechanisms** (the scanner's pre-existing
hardcoded `excluded_dirs` and the new rulebook-driven directory-rule
prune) risk confusing future maintainers about which one is authoritative
for a given path. Mitigated by treating the hardcoded excludes as a
distinct, lower-level safety net (self-exclusion of the tool's own state
and pre-existing fixture excludes) and the rulebook as the
project-configurable lifecycle layer; `scripts/CONTEXT.md` should document
both once implemented.
- **`git ls-files` / dirty check runs twice** (once at report-build time,
once at apply time) for lifecycle deletes — a deliberate, spec-mandated
redundancy (ADR-0039: "never trusting the rule … at runtime") rather than
an oversight; the cost is one extra git subprocess call per lifecycle
entry per run, acceptable given the destructive nature of the operation.
- **`extract-then-delete` spans two subsystems** (a generative rewrite via
the existing Sonnet-distillation path, then a `git rm`) inside a single
op type, and for cross-repo extraction additionally calls into
`/os-vault:write`, a different plugin's skill. This is more moving parts
in one applied entry than any existing op; a partial failure (extraction
succeeds, delete fails, or vice versa) needs care to keep the "one hygiene
commit" invariant intact — this design treats extraction-then-delete as
atomic per entry (both steps land in the same commit, or neither does),
which may need revisiting once implemented against real vault-write
latency/failure modes.
- **Directory-rule aggregate entries change the meaning of "one entry per
file"** that today's `doc-check`/`report-schema` specs assume throughout
(e.g. "every `entries[].path` SHALL be a member of `shortlist`" — a
directory aggregate entry's path is a directory, not a file, which is a
new shape for `path` that existing consumers may not expect). The delta
specs below extend rather than replace this requirement, but any code
outside this change's touched files that assumes `path` is always a
regular file should be audited during implementation.
- **`rulebook.py`'s hard requirement on Python ≥3.13** (`glob.translate`) is
new relative to the rest of the scripts directory (whose stdlib-only
policy has not previously pinned a Python floor this high); if the
environment running `os-doc-hygiene` scripts is older, the whole rulebook
layer hard-fails to import. This is accepted as a locked design decision
(spec §2), not reopened here, but is called out as an operational risk
worth a version check with a clear error message at load time.
## Ambiguities resolved while writing this design
(Reported per instructions — none silently decided against the spec; these
are places the locked documents left an implementation-level gap.)
1. **IGNORE-surface entries vs. ordinary directory-rule "prune + aggregate
entry" behavior.** Spec §1/§2 phrasing could be read as saying the
IGNORE surface *itself* produces an aggregate shortlist entry ("a
directory-rule match prunes the walk and emits one aggregate entry —
this IS the ignore surface"), but spec §1 and the §3 tier table also say
IGNORE-surface members are simply "never walked," with no entry
language. Resolved by treating "prune + aggregate entry" as the general
mechanism for lifecycle directory rules with a real lifetime (temporary /
delete-once-served), and reserving true zero-signal, zero-entry pruning
for the specific IGNORE seed list (`graphify-out/**`, `.dochygiene/**`).
Flagging this because it affects whether IGNORE members ever appear
anywhere in a report — this design says they never do.
2. **`conventions.json` file shape.** The spec names the file and its v1
contents (archive-bucket, status-frontmatter) and per-entry conceptual
fields (name, what it proves, template, pitch) but does not give a JSON
schema. This design treats it as a flat array of objects with those four
conceptual fields, no envelope/schema_version wrapper (unlike
`rulebook.json`, which the spec explicitly gives an envelope). Flagging
because a schema_version envelope could be added instead if implementation
prefers consistency with rulebook.json; nothing in the spec forecloses
either choice.
3. **Where the `git ls-files`/dirty runtime check physically lives** (spec
says "verified at runtime via git ls-files + dirty check, never trusting
the rule" but doesn't say whether that's a report-build-time check, an
apply-time check, or both). This design does both — advisory at
report-build time (so the report's tier reflects current reality when
shown to the human), authoritative at apply time (so a stale report
never causes an unsafe auto-delete) — matching the existing
content-hash-guard pattern (`expected_sha256` is likewise computed at
build time and re-verified at apply time). Flagging because the spec's
single sentence doesn't explicitly mandate the double-check; it was
inferred from ADR-0039's "never trusting the rule" plus the existing
applier's re-verification precedent (`patch_applier.py`'s content-hash
guard).
4. **Extraction-failure handling within `extract-then-delete`** (does a
failed extraction hard-fail the whole clean run, or just skip that
entry?) is not addressed in the spec. This design treats it as a
per-entry skip (consistent with `doc-clean`'s existing partial-success
semantics), not a run-level hard failure, unless the failure mode matches
one of the existing hard-failure triggers (applier exit 2, write error).
Flagging since this determines commit granularity under failure.