refactor(#3471): one enforcement point for the empty case, and reports that match the disk (#3519)

* refactor(#3471): one enforcement point for the empty case, and reports that match the disk

Implements ADR-3408 section 8.5 and section 8.4's residue (folded in when Phase
3 closed as subsumed). Four items, and two findings the design did not predict.

FINDING 1 — the guards could not simply be deleted, as the design instructed.
state sync and REGENERATE_STATE never run applyStatePreservation at all, so
those six conditions were their ONLY empty-field fallback. A baseline probe on
the unedited tree confirmed unconditional deletion drops current_phase,
current_phase_name, current_plan, stopped_at and paused_at from a blank-body
STATE.md on state sync — breaking the byte-identical requirement section 8.3
grants those two sanctioned-permanent exceptions. They are now GATED, not
deleted: on for the exceptions, off for the write seam, where an empty derived
value finally reaches the executor unmolested.

FINDING 2, the more serious one — there was a FOURTH encoding of this policy.
The pre-existing #2202 unknown-key carry-forward loop independently restored
the same six fields whenever derivedFm lacked the key, completely neutralizing
the fix. It is named nowhere in the ADR, the design, or three prior phases. It
was found only because a probe that should have passed did not: the first
attempt reported divergedFields: [] and silently restored both fields,
reproducing the exact bug this phase exists to close.

That is worth stating plainly. This epic's thesis is 'policy declared in one
table, enforcement hand-rolled per call site.' The final phase found one more
call site than anyone had counted — which is the fourth consecutive time a copy
count in this epic proved to be a lower bound.

Also: divergedFields could only observe fields the executor actively RESTORED,
by diffing postFm. A discard-to-empty is absent both before and after, so it
was invisible. A second pass now reports it, which is what makes section 8.5's
'preservation is visible' true for the delete-the-body-line case rather than
aspirational.

cmdPhaseComplete now reports what it preserved — #3374 was filed against that
command and its complaint was warnings: [], silence.

cmdStateJson's private third copy of the guards is routed onto the executor's
preserve-when-unchanged rule. A read is definitionally not a write, so the
#1230 delta is 'unchanged' and curated wins over a stale annotation.
shouldPreserveExistingProgress is a different rule and is untouched.

Report reconciliation is ONE shared helper across seven commands, not five
copies of fix(#3351)'s block. Five copies of a reconciliation is precisely the
shape this epic removes, and introducing it in the final phase would have been
a poor joke. Both untraced commands were traced rather than assumed:
cmdStatePlannedPhase matched cmdStateBeginPhase exactly; cmdStateCompletePhase
turned out to be a different legacy hand-rolled path reporting a mix of field
names AND a section name, where the naive helper would have dropped 'Current
Position' as a false negative every time.

* test(#3471): characterization coverage for one enforcement point and reconciled reports

Matrix sections A-E, asserted at the consumer's output per ADR-3180 Decision
4(b)/(c) — this phase owes Decision 5's outcome metric, the one the drift
guard's zero may never be reported without.

Three walls matter more than the new coverage:

  A2 is SIX separately named tests, one per gated guard, not one parameterised
  assertion over a list. A list is trivially shortened later; six named tests
  are not, and six guards is exactly where a field gets silently dropped.

  A6 pins what Phases 1-3 already fixed — non-empty stale body, delta
  unchanged, losing to fresher curated frontmatter, with the divergence
  reported. If A6 reddens, this phase broke the thing the epic was for.

  D1/D2 pin state sync byte-identical. The implementation had to GATE the six
  guards rather than delete them precisely because state sync has no executor,
  and a baseline probe showed unconditional deletion drops five fields.
  Nothing else in the suite would notice that regression.

E6 covers #3345's direction — a field preservation restored that the intent
never named IS reported. Nothing has ever tested that direction.

Assertions were empirically verified against the compiled lib and the real CLI
before being written, since the suite cannot be executed locally. That caught
two type bugs in the draft: fm.current_phase after a quoted-YAML round-trip is
the string '5', not the number 5.

E5 is recorded as structurally unreachable rather than weakened or faked. Those
four commands report body Title-Case labels, which cannot string-collide with a
frontmatter snake_case key the way cmdStatePatch's arbitrary field names can —
which is why fix(#3351) targeted only cmdStatePatch. Testing it directly would
need reconcileReportedFields exported from private scope; the helper is
exercised through E6 and all seven commands instead.

* docs(#3471): amend ADR-3408 section 8.5 — a fourth enforcement point, and guards that could not be deleted

Amendment 3. The contract held; two of section 8.5's own statements did not.

It said the six empty-only guards are DELETED. They cannot be. writeStateMd is
the sole path for both section 8.3 sanctioned-permanent exceptions and never
runs applyStatePreservation, so those guards were their only empty-field
fallback. A baseline probe on the unedited tree confirmed unconditional
deletion drops five fields from a blank-body STATE.md on state sync, breaking
the byte-identical guarantee section 8.3 grants it. They are gated instead.

It also mis-located cmdStateJson's guards, describing them as living in
syncStateFrontmatter. They were a separate private copy on the read path with
no delta check at all, so a stale body annotation always beat fresher curated
frontmatter in state.json — #3395's shape entirely outside the write seam.

THE FINDING: a fourth enforcement point nobody had counted. The pre-existing
#2202 unknown-key carry-forward loop independently restored the same six
fields, silently neutralizing the fix. It is named nowhere in this ADR, in the
phase design, or in three prior phases, and was found only because a probe that
should have passed did not.

Fourth consecutive time a copy count in this epic proved a lower bound: 2
write-seam bypasses became 4, three preservation encodings became four, and the
estimate was wrong every time. ADR-3180's standing rule has earned itself in
every phase — read the code, not the write-up.

Records the Row 2 decision (a discard-to-empty wins per the delta rule and is
reported, not silent — the sharpest Hyrum exposure in the epic), section 8.4's
residue landing as ONE shared reconcileReportedFields across seven commands
rather than five copies, and the parity assertion added because
FRONTMATTER_KEY_TO_BODY_LABEL was itself a second table that failed silently —
this epic's shape in miniature, in its final phase.

* fix(#3471): repair four regressions the checkpoint caught

Checkpoint returned 16 failures of 34389: six real regressions in pre-existing
tests, plus seven of my own test bugs.

My hypothesis was wrong and is recorded as such. I predicted the #2202
carry-forward skip was the cause, reasoning it had removed a load-bearing
fallback the way the six guards nearly were. It was not implicated in any of
the six. Three unrelated causes:

#2111 — current_phase came back undefined from milestone complete, which is
the epic's own defect class reintroduced by its final phase. Root cause is
Row 2 working exactly as designed: milestoneCompleteCore rewrites the body
Phase: line to a closure message, so current_phase's #1230 delta reads
CHANGED and the new rule correctly discards the curated value. The transition
never declared any intent to touch that field. Fixed by re-asserting
current_phase and current_phase_name through authoritativeFm — the existing
#2736 mechanism beginPhaseCore and completePhaseCore already use — rather
than by weakening Row 2, which A5 pins.

That interaction is worth naming: a rule that keys on 'did this write change
the body source' will fire on a transition that moves the body line for an
entirely unrelated reason. The design did not anticipate it.

#1264 / #3242 / the state.patch progress report — reconcileReportedFields
folded EVERY divergedFields entry into updated, including preserve-always
progress restores no caller asked about. Now scoped to preserve-when-unchanged
rows only.

#1162 / case-insensitive table fields — valueOf checked frontmatter before
body, so a lowercase table field name exact-matched the lowercase frontmatter
key sync always derives, comparing stale pre-sync body text against a
post-sync frontmatter enum. Flipped to body-first.

That last one is the SAME lesson as Phase 2's patchCore, recurring in a
different function two phases later: in this model the body is authoritative
and frontmatter is the projection, so a name that could mean either resolves
body-first. Twice now.

Test bugs: a stray unused parameter shifted every argument at six call sites,
so body arrived undefined; and A4 compared nested progress scalars against
numbers when extractFrontmatter returns raw YAML strings. The string-vs-number
YAML round-trip has now been caught three times in this phase alone.

* test(#3471): one helper for the progress coercion that bit four times

A2f failed on the string-vs-number YAML round-trip: extractFrontmatter returns
nested progress scalars as raw YAML strings, so a comparison against numeric
literals can never pass.

This is the FOURTH time this exact class has been caught in this phase — twice
during test authoring, once as A4 in the previous checkpoint, now as A2f.
Patching it a fourth time by hand would guarantee a fifth.

Added numericProgress() with a comment saying why it exists, and routed every
progress-reading assertion in the #3471 block through it. Swept the block:
C3 needed no change, because cmdStateJson's output already runs through
normalizeProgressNumbers.

Deliberately NOT shared with frontmatter.test.cjs's readPersistedProgress:
that one is path-based and re-reads from disk, while these assert on an
in-memory string that is never written. Sharing would have meant either a
disk round-trip these tests do not do, or duplicating half the helper — so
the coercion pattern is mirrored locally and the reason recorded, rather
than manufacturing a dependency to satisfy the letter of consolidation.

* chore(#3471): backfill pr number in changeset fragment

---------

Co-authored-by: sim <sim@local>

docs/adr/3408-state-write-path-preservation.md

@@ -214,7 +214,13 @@ Both are **permanent entries in the ratchet with `owner: sanctioned-permanent`**
 
 **Question.** The body source disagrees with frontmatter and the derived value is non-empty. Who wins?
 
-**Rule.** The declared policy decides, on the same terms as an empty derived value. `preserve-when-unchanged` restores the curated frontmatter value when that field's body source did not change in **this** write. The empty-only "#905" guards in `syncStateFrontmatter` are **deleted**, not kept in sync — one enforcement point on every path, including `writeStateMd` and `cmdStateJson`.
+**Rule.** The declared policy decides, on the same terms as an empty derived value. `preserve-when-unchanged` restores the curated frontmatter value when that field's body source did not change in **this** write.
+
+**One enforcement point on the write seam — with the §8.3 exceptions carved out explicitly.** The empty-only "#905" guards in `syncStateFrontmatter` are **gated, not deleted** (corrected by Amendment 3): they stay active for the two sanctioned-permanent exceptions, which never run `applyStatePreservation` and for which those guards are the *only* empty-field fallback, and are off on the write seam where the executor owns the empty case.
+
+**Rule — a discard is as visible as a restore.** When the body source *did* change this write and the derived value is empty, the derived value wins per the delta rule and the curated value is dropped. That drop is reported through the same channel as a restore. Silence would make "preservation is visible" true only for the half of the rule that adds a value back.
+
+**Rule — `cmdStateJson` is governed too.** It is a read path, and it carried its **own private copy** of the empty-only guards with no delta check at all. It routes through the executor's `preserve-when-unchanged` rule instead. *(This section previously described those guards as living "in `syncStateFrontmatter`". They did not — they were a separate third encoding, corrected by Amendment 3.)*
 
 **Rule.** **Preservation is visible.** When policy restores a curated value over a disagreeing derived one, the command emits a divergence warning. Silence is the defect #3374 reported (`warnings: []`), not the fix.
 
@@ -333,4 +339,24 @@ This was not a theoretical over-reach. #3469's own scope line, inherited from th
 
 **Criterion 6 context.** All five of the epic's named instances were closed by point fixes while Phase 1 was in flight, so Phase 2 and Phase 4 are driven by **characterization tests at the consumer's output** (Decision 4(b)/(c)) rather than fail-first tests. Weaker, and stated as such.
 
-*(Phase 3 records the §8.4 bucket decision here — though see the epic: `fix(#3351)` (#3487) appears to have subsumed most of Phase 3.)*
+### Amendment 3 — Phase 4 (#3471): there was a FOURTH enforcement point, and the guards could not be deleted
+
+The contract held. Two of this section's own statements did not.
+
+**§8.5 said the guards are "deleted". They cannot be.** `writeStateMd` — the sole path for both §8.3 sanctioned-permanent exceptions — never runs `applyStatePreservation`, so those six conditions were their **only** empty-field fallback. A baseline probe on the unedited tree confirmed unconditional deletion drops `current_phase`, `current_phase_name`, `current_plan`, `stopped_at` and `paused_at` from a blank-body STATE.md on `state sync`, breaking the byte-identical guarantee §8.3 grants it. They are **gated**: on for the exceptions, off for the write seam. §8.5 is corrected above.
+
+**§8.5 mis-located `cmdStateJson`'s guards.** It described them as living "in `syncStateFrontmatter`". They were a **separate private copy** on the read path, with no delta check at all — so a stale body annotation always beat fresher curated frontmatter in `state.json`, reproducing #3395's shape entirely outside the write seam. Now routed through `applyPreserveWhenUnchanged`. `shouldPreserveExistingProgress`'s cross-milestone logic is a different rule and is untouched.
+
+**THE FINDING: a fourth enforcement point nobody had counted.** The pre-existing #2202 unknown-key carry-forward loop independently restored the same six fields whenever `derivedFm` lacked the key — silently neutralizing the gating fix. It is named nowhere in this ADR, in Phase 4's design, or in three prior phases. It was found only because a probe that should have passed did not: the first attempt reported `divergedFields: []` and restored the stale values, reproducing the exact bug the phase existed to close.
+
+That is the **fourth consecutive time** a copy count in this epic proved a lower bound — 2 write-seam bypasses became 4, three preservation encodings became four, and the estimate was wrong every single time. ADR-3180 Amendment 3's standing rule has now earned itself in every phase of this epic: **read the code, not the write-up.**
+
+**`divergedFields` could not see a discard.** It diffed `postFm` before and after preservation, so it could only observe fields actively *restored*. A discard-to-empty is absent on both sides and was therefore invisible. A second pass reports it, which is what makes the new "a discard is as visible as a restore" rule real rather than aspirational.
+
+**Decided — Row 2, the delete-the-body-line case.** When a transform deliberately removes a body line, the derived (empty) value wins per the delta rule and the curated value is dropped. Consistent with "same terms as empty", and it discards a value that previously survived on that path — so it is reported rather than silent. This is the sharpest Hyrum exposure in the epic and ships with its own call-out.
+
+**§8.4's residue, folded in when Phase 3 (#3470) closed as subsumed.** `fix(#3351)` reconciled only `cmdStatePatch`. Seven commands now share **one** `reconcileReportedFields` helper — not five copies of that block, which would have been this epic's own defect class introduced in its final phase. Both previously-untraced commands were traced rather than assumed: `cmdStatePlannedPhase` matched `cmdStateBeginPhase` exactly; `cmdStateCompletePhase` turned out to be a different legacy path reporting field names **and** a section name, where the naive helper would have dropped `Current Position` as a false negative every time.
+
+**A parity assertion, because the fix needed one.** `FRONTMATTER_KEY_TO_BODY_LABEL` is a second table beside `FIELD_CLASSIFICATION`, and a missing entry originally fell back silently to the raw key. That is this epic's shape in miniature. A missing `preserve-when-unchanged` label now throws with a structured error mirroring §8.2's `throwUnwiredRow`, and a test asserts every such row has an entry — the parity assertion `CLAUDE.md`'s *Generative Fix Divergence* entry requires whenever two surfaces share a constant. The throw is deliberately scoped to that policy: `divergedFields` legitimately carries `progress`, `milestone` and `milestone_name`, which have no body-line label, and an unconditional throw would have broken a live case.
+
+*(Phase 3's §8.4 bucket decision was folded here; `fix(#3351)` (#3487) subsumed most of Phase 3, which closed as subsumed.)*