proposal 0002: validate warning for dead renames overlay entries (#19)

proposals/0002-validate-warning-dead-renames.md

@@ -0,0 +1,146 @@
+# 0002 — `validate` warning for dead `renames` overlay entries
+
+| | |
+|---|---|
+| **Status** | Accepted |
+| **Tracking issue** | #19 |
+| **Principles touched** | P4 |
+
+## Problem
+
+A manifest `renames:` entry whose `from` names no rule the active set actually renamed is
+**silently ignored** at materialization. The overlay maps a pack's published rule id to a local
+id (`{pack, kind: rule, from, to}`, decision 0007, D38 at `archive/pre-github`); the mapping is
+consulted only through `materialize.local_rule` while walking selected methods
+(`src/ea_core/materialize.py`). An entry that no selected method reaches is never applied and
+never reported — the closure just doesn't call `local_rule` with that key.
+
+So a user who mistypes `from` (or `pack`), or whose target rule left the active set after a
+`method remove`/`migrate`, gets **no signal**. The overlay entry is dead weight, indistinguishable
+in the manifest from a live one. The Phase-26 note flagged "a `validate` warning surfacing dead
+rename entries" as a later nicety; this is that nicety.
+
+The gap is exactly the one `policy.dangling` already fills for the sibling overlay: a
+method-deprecation policy naming an inactive method is warned, not enforced
+(`validate._policy_checks`, D23/D25). `renames` is the only org overlay without such a mirror —
+`policies` has `policy.dangling`, `relationships` has `relationship.overlay_dangling`
+(`model.py`), `renames` has nothing.
+
+## Design
+
+Core, read-only, non-blocking. `validate` emits one **`warning`** diagnostic, code
+`renames.dangling`, `path` = the manifest, for each `renames` entry that resolves to nothing
+against the active materialized set. No exit-3 path, no schema change, no new write.
+
+### Placement
+
+A new `_renames_checks(model, diags)` in `src/ea_core/validate.py`, called from
+`validate_semantic` beside `_policy_checks` and `_shadow_checks`. It reads
+`model.manifest.raw["renames"]` (the overlay is carried on the loaded manifest) and the active
+rule provenance, and appends warnings sorted deterministically by `(pack, from)` (P1).
+
+Message, mirroring `policy.dangling`'s register:
+
+```
+rename overlay entry {pack}:{from} → {to} applies to no active rule
+```
+
+### What "resolves to nothing" means — provenance, not id presence
+
+A **live** `kind: rule` rename produces exactly one materialized rule: the file is written under
+its local `to` id, and materialization already stamps `source.original_id = from` /
+`source.pack = pack` on it (`materialize.py` `_claim`; the `original_id` breadcrumb is written
+*only* when `local != rid`). So the set of applied renames is recoverable from the store:
+
+```
+applied = { (rule.source.pack, rule.source.original_id)
+            for each active rule whose source carries original_id }
+```
+
+An entry `{pack, kind: rule, from, to}` is **dead** iff `(pack, from) ∉ applied`. This is the
+precise reading of the issue's "`from` matches no active rule": post-rename the active rule is
+keyed by `to`, so presence of the *`to`* id is not the test — the provenance breadcrumb is,
+and it already exists on disk for exactly this purpose.
+
+Only `kind: rule` entries are checked (the whole enum today; extending the enum is #8, which
+would extend this check in lockstep).
+
+### One additive model touch
+
+`load_definitions` currently discards the store's `source` block — `_ruledef_from_doc` keeps
+only `id`/`kind`/`applies_to`/`params`, and `defs.methods` keeps only the inner method doc. So
+the provenance the check needs is on disk but not in the loaded `Model`. The implementation
+threads it in minimally: give `RuleDef` an additive `source: dict = {}` field populated from the
+rule doc's `source`. Additive, provenance-only, and independently useful (the model today throws
+away lineage the materializer deliberately recorded). `validate` reads
+`model.definitions.rules[*].source`; nothing else changes.
+
+### Forward-looking entries are warned too — deliberately
+
+`manifest rename add` accepts a `from` for a **not-yet-active** pack as *forward-looking*
+(manifest.md; essential because the clashing `method add` hasn't run yet). A forward-looking
+entry is, by construction, dead *right now* — and it is **indistinguishable from a typo'd
+`pack`/`from` from repo state alone**. This proposal warns on it anyway, for parity with
+`policy.dangling`, which already warns on a forward-looking policy naming a not-yet-active
+method. The register makes this safe: it is a non-blocking heads-up ("this entry isn't doing
+anything yet"), never a red repo, so a green forward-looking manifest stays green. The noise cost
+is real but bounded and matches an accepted precedent; the alternative of suppressing it silently
+loses the most common real bug (a mistyped pack name looks exactly like forward-looking). See
+forks for the tiered variant.
+
+## Forks considered
+
+- **Do nothing** — dead entries are harmless (ignored, never *mis*-applied); the only cost is a
+  silent typo. But the same argument was rejected for `policy.dangling`: a silently-ineffective
+  governance/tailoring entry is a latent surprise, and the overlay-consistency mirror is cheap.
+- **Refuse at `manifest rename add`** (usage error) instead of warning at `validate` — breaks the
+  forward-looking case manifest.md explicitly supports (a rename staged before its clashing
+  `method add`); a hard refuse there would make legitimate staging impossible. A warning is the
+  right register — it surfaces the dead entry without forbidding the forward-looking one.
+- **`to`-id-presence heuristic** (dead iff `to ∉ active rules`) — needs no model change at all,
+  but a *native* rule coincidentally named `to` masks a genuinely-dead entry (false negative),
+  and the collision guard runs on post-rename ids so a live entry always leaves `to` present
+  anyway. Provenance is only one additive field more and is exact; the heuristic trades
+  correctness for that field.
+- **Suppress forward-looking (pack-not-active) entries; warn only when the pack is active but
+  `from` is unmatched** — quieter, and the pack-active case is the unambiguous-typo tier. But
+  "pack active" is itself only knowable from source breadcrumbs, a mistyped `pack` name is then
+  never caught (it reads as forward-looking), and it diverges from the `policy.dangling`
+  register for no schema saving. Recorded as the tiered alternative; the flat parity design is
+  simpler and catches the common typo. Revisit if the forward-looking noise proves annoying in
+  practice.
+- **Fold into the rename-kind-extension issue (#8)** — kept separate: this applies to today's
+  `kind: rule` renaming and is independent of adding new kinds. #8, when it lands, extends the
+  same check to the new kinds rather than replacing it.
+
+## Compatibility & migration
+
+Additive. A new advisory diagnostic **code** (`renames.dangling`) joins the explicitly
+non-exhaustive dotted-code set cli.md documents ("e.g. …"); consumers match codes, so a new
+warning code is additive by the contract's own design. No exit-code change (warnings never move
+exit status), no `schema_version` bump, no manifest-shape change. The `RuleDef.source` field is
+an additive dataclass member with an empty default. Existing repos are unaffected: a repo with no
+dead entries sees no new diagnostics; a repo with a dead entry that was silently green stays
+green, now with a warning. This does not move a binding decision — it is the advisory consequence
+of decision 0007's overlay, mirroring how `policy.dangling` rode alongside decision 0008 without a
+decision-record change.
+
+## Proof
+
+**Objective half** (implementation PR):
+
+- A scenario e2e case (extending the manifest-overlay validate suite): a manifest with a live
+  rename (materialized, no warning) plus a dead entry whose `from` matches no applied rename —
+  `validate` stays exit 0 and emits one `renames.dangling` warning naming `pack`/`from`/`to`;
+  a second dead entry produces a second warning, and the two are ordered by `(pack, from)`
+  (byte-stability). A forward-looking entry (pack not yet active) warns identically — asserting
+  the deliberate parity choice. A repo with only live renames emits none.
+- Unit coverage for the `applied`-set derivation from rule provenance (renamed vs. un-renamed
+  rules) and the additive `RuleDef.source` population.
+- Docs in the same PR: `docs/reference/cli.md` (add `renames.dangling` to the diagnostic-code
+  examples) and `docs/reference/manifest.md` (a line under the `renames` section noting a dead
+  entry is warned, not enforced, and pointing at `validate`).
+
+**Agent-facing half:** none required — this is a core diagnostic with no surface/skill change and
+no new judgment. The plugins already teach `validate` as the repo-health verb; the new warning
+rides that verb with no prompt edit.