test(diagnostics): close the rule-4 acceptance gap and three ledger nitpicks

rust/crates/own-diagnostics/tests/ledger_replay.rs

@@ -8,12 +8,16 @@
 //! [`own_diagnostics::TITLES`] must have a case, so a family cannot be added
 //! without a fixture.
 //!
-//! The three failure modes, each with its own test so a red build names the
-//! actual problem:
+//! The failure modes, each with its own test so a red build names the actual
+//! problem:
 //!
 //! * **missing** — a `TITLES` code with no ledger case;
 //! * **orphan** — a ledger case naming a code absent from `TITLES`;
-//! * **divergence** — the rendered text disagrees with the reference.
+//! * **divergence** — the rendered text disagrees with the reference;
+//! * **collapsed shape** — the generator still covers every code, but stopped
+//! producing one of the optional-field arms it exists to pin. Invisible to
+//! the divergence check, which only asks whether each case matches its *own*
+//! recorded text: drop the two-evidence arm and that test still passes 47/47.
 //!
 //! What this does NOT claim: that the analyzer emits a given code. That is step
 //! 4's contract, pinned over the real `.own` corpus by `diag_parity.json`. The
@@ -25,7 +29,7 @@
 
 use std::collections::BTreeSet;
 
-use own_diagnostics::{title, Diagnostic, TITLES};
+use own_diagnostics::{title, Diagnostic, Severity, TITLES};
 use serde_json::Value;
 
 const FIXTURE: &str = concat!(
@@ -183,6 +187,29 @@ fn rendered_text_matches_the_reference_for_every_family() {
  }
  }
 
+ // `changed` and `unexplained` move together **by construction**: the loop
+ // above increments both on the same branch, because this replay has no
+ // mechanism for marking a difference as *explained*. That is deliberate —
+ // an explained divergence would need a reviewed entry in the fixture, and
+ // no such entry exists. Asserting the identity records the fact instead of
+ // leaving two counters that merely look independent: if an explanation
+ // channel is ever added, this line fails and forces the two to be split on
+ // purpose rather than letting `unexplained` quietly over-report.
+ //
+ // Honest about its own reach: while the tree is green this is `0 == 0` and
+ // proves nothing — the divergence branch never runs. It is a tripwire that
+ // arms only once something actually diverges, which is exactly when a
+ // silently over-reporting `unexplained` would mislead. Verified by a
+ // compound mutation (corrupt one case's recorded text so a divergence
+ // exists, and drop the `unexplained` increment): this fires first, naming
+ // the counter split rather than the divergence.
+ assert_eq!(
+ counters.changed, counters.unexplained,
+ "changed and unexplained are equal by construction; they diverged, so an \
+ explanation mechanism was introduced without separating the counters: \
+ {counters:?}"
+ );
+
  assert_eq!(
  counters.unexplained,
  0,
@@ -195,6 +222,66 @@ fn rendered_text_matches_the_reference_for_every_family() {
  assert_eq!(counters.changed, 0, "counters: {counters:?}");
 }
 
+/// Every optional-field arm the generator can produce must still appear.
+///
+/// The ledger's value is that it renders *shapes*, not 47 copies of one shape:
+/// `_shape_for` rotates subject, resource kind, severity and evidence length by
+/// each code's own seed. Nothing so far would notice that rotation collapsing.
+/// Changing the vocabulary changes every seed-derived arm at once, so a future
+/// edit could quietly leave, say, no two-evidence case anywhere — and the
+/// rendering replay would still pass 47/47, because it only checks that each
+/// case matches its own recorded text.
+///
+/// This asserts presence, not counts: the exact split is a property of the hash
+/// and would make the test a second golden with no extra signal.
+#[test]
+fn every_optional_field_shape_is_still_exercised() {
+ let root = load();
+ let diagnostics: Vec<Diagnostic> = cases(&root)
+ .iter()
+ .map(|case| {
+ let code = code_of(case);
+ serde_json::from_value(case.get("diagnostic").expect("'diagnostic'").clone())
+ .unwrap_or_else(|e| panic!("code {code:?}: diagnostic does not load: {e}"))
+ })
+ .collect();
+
+ let count = |f: &dyn Fn(&Diagnostic) -> bool| diagnostics.iter().filter(|d| f(d)).count();
+ let arms: [(&str, usize); 9] = [
+ ("subject: present", count(&|d| d.subject.is_some())),
+ ("subject: absent", count(&|d| d.subject.is_none())),
+ (
+ "resource_kind: present",
+ count(&|d| d.resource_kind.is_some()),
+ ),
+ (
+ "resource_kind: absent",
+ count(&|d| d.resource_kind.is_none()),
+ ),
+ (
+ "severity: warning",
+ count(&|d| d.severity == Severity::Warning),
+ ),
+ ("severity: error", count(&|d| d.severity == Severity::Error)),
+ ("evidence: none", count(&|d| d.evidence.is_empty())),
+ ("evidence: one", count(&|d| d.evidence.len() == 1)),
+ ("evidence: two", count(&|d| d.evidence.len() == 2)),
+ ];
+
+ let unexercised: Vec<&str> = arms
+ .iter()
+ .filter(|(_, n)| *n == 0)
+ .map(|(name, _)| *name)
+ .collect();
+ assert!(
+ unexercised.is_empty(),
+ "{} optional-field shape arm(s) are no longer exercised by any ledger case: \
+ {unexercised:?}. The ledger stopped covering a shape it is supposed to pin — \
+ restore the rotation rather than deleting the arm. Full census: {arms:?}",
+ unexercised.len()
+ );
+}
+
 #[test]
 fn analyzer_corpus_coverage_is_recorded_not_assumed() {
  // Rendering coverage is total; analyzer-corpus coverage is not, and the
@@ -212,7 +299,11 @@ fn analyzer_corpus_coverage_is_recorded_not_assumed() {
  .filter(|c| {
  c.get("analyzer_corpus")
  .and_then(Value::as_bool)
- .unwrap_or(false)
+ // `expect`, not `unwrap_or(false)`: every other accessor in this
+ // file treats a missing key as a broken fixture, and defaulting
+ // here would silently under-count coverage if the generator ever
+ // stopped emitting the flag.
+ .expect("case 'analyzer_corpus'")
  })
  .count();
  assert_eq!(

tests/test_diag_ledger_fixtures.py

@@ -26,14 +26,18 @@
 reach. Silently rendering all 47 and calling it "full parity" would overstate
 the evidence, so the coverage flag is carried per code and summarised.
 
-## The three failure modes it closes
+## The failure modes it closes
 
 * **missing** — a code in `TITLES` with no case. The replay fails naming it, so
  adding a diagnostic without a fixture is a red build.
 * **orphan** — a case whose code is not in `TITLES`. Catches a removed or
  renamed code leaving a fixture behind.
 * **stale** — the generated file differs from what the current reference
  produces (the standing `--write` discipline, as in the other two slices).
+* **unstable under insertion** — adding one vocabulary member must rewrite no
+ existing record, add exactly one, and remove none. This is P-022 rule 4's
+ normative acceptance, enforced by [`_insertion_effect`]; the three numbers fail
+ in different directions and none implies the others.
 
 Run: python tests/test_diag_ledger_fixtures.py (verify)
  python tests/test_diag_ledger_fixtures.py --write (regenerate)
@@ -47,6 +51,7 @@
 import os
 import re
 import sys
+from typing import NamedTuple
 
 sys.path.insert(0, os.path.join(os.path.dirname(__file__), ".."))
 
@@ -218,44 +223,109 @@ def run() -> int:
  f"{orphans}")
  return 1
 
- churn = _insertion_churn()
- if churn:
- print(f"FAIL: inserting one code rewrote {churn} existing ledger record(s). "
- f"Shapes must derive from the code itself, never from its position in "
- f"the sorted vocabulary — adding a family has to be a ONE-record diff, "
- f"or a real renderer change hides in the churn")
+ effect = _insertion_effect()
+ if effect.churn:
+ print(f"FAIL: inserting one code rewrote {effect.churn} existing ledger "
+ f"record(s); rule 4 requires 0. Shapes must derive from the code itself, "
+ f"never from its position in the sorted vocabulary — adding a family has "
+ f"to be a ONE-record diff, or a real renderer change hides in the churn")
+ return 1
+ if effect.added != 1:
+ print(f"FAIL: inserting one code produced {effect.added} new ledger record(s); "
+ f"rule 4 requires exactly 1. Zero means the generator does not cover the "
+ f"new family at all — the ledger would go quietly incomplete. More than "
+ f"one means a record count is derived from the vocabulary's SIZE, which "
+ f"is the churn defect wearing a different hat")
+ return 1
+ if effect.removed:
+ print(f"FAIL: inserting one code REMOVED {effect.removed} existing ledger "
+ f"record(s); rule 4 requires 0. A delta is a gain: a generator that adds "
+ f"the new record while dropping another has not grown, and the dropped "
+ f"record leaves the churn comparison entirely, so neither of the other "
+ f"two numbers can see it")
  return 1
 
  totals = data["totals"]
  print(f"diagnostic ledger OK: {totals['codes']}/{len(TITLES)} codes covered "
  f"({totals['by_family']}), "
  f"{totals['analyzer_corpus']} of them also produced by the .own corpus sweep; "
- f"a new code rewrites {churn} existing record(s)")
+ f"inserting a code rewrites {effect.churn} existing record(s), "
+ f"adds {effect.added} and removes {effect.removed}")
  return 0
 
 
-def _insertion_churn() -> int:
- """How many EXISTING records change when one new code joins the vocabulary.
+class _InsertionEffect(NamedTuple):
+ """What inserting one vocabulary member does to the generated ledger.
+
+ P-022 rule 4 states the acceptance as:
 
- Must be zero. This is the property that makes the ledger readable: the whole
- point is that adding a diagnostic family shows up as a single new record a
- reviewer can check, not as a wall of unrelated edits. An index-derived shape
- scored 42 of 47 here before the seed was made position-independent.
+ insert one synthetic vocabulary member
+ existing-record churn == 0
+ new-record delta == 1
 
- A probe code is inserted mid-vocabulary (so it shifts sorted positions) and
- removed again in a `finally`, so the live TITLES the rest of the suite sees
- is untouched."""
+ `added` and `removed` are both halves of what that second line *means*: a
+ delta is a gain, and a run that adds one record while quietly dropping
+ another has gained nothing. Counting only additions would accept exactly
+ that, so the measurement is split rather than the norm widened.
+ """
+
+ churn: int
+ """EXISTING records whose content changed. Must be 0."""
+ added: int
+ """Records that did not exist before. Must be exactly 1."""
+ removed: int
+ """Records that existed before and are now gone. Must be 0."""
+
+
+def _insertion_effect() -> _InsertionEffect:
+ """Measure rule 4's acceptance against the live generator.
+
+ `churn` is the half that was paid for: an index-derived shape scored 42 of
+ 47 here before the seed was made position-independent. Adding a family has
+ to read as a single new record, or a real renderer change hides in the noise.
+
+ `added` is the other half, and it fails in the opposite direction. Zero means
+ the generator did not emit the new family at all — the ledger goes quietly
+ incomplete, which is precisely the gap `churn` alone cannot see, because a
+ generator that skips the new code has perfect churn. Above one means some
+ record count is derived from the vocabulary's *size* rather than its members,
+ which is the churn defect in different clothing.
+
+ `removed` closes the hole the other two leave between them. A generator
+ refactored to cap the ledger at its previous size emits the probe *and*
+ drops an existing code: the dropped record is absent from `after`, so it
+ never enters the churn comparison, and the addition still counts one. Both
+ original numbers read clean while the ledger did not grow at all — measured,
+ not hypothesised (probe added, `OWN052` silently gone).
+
+ None of the three is the mechanism. A content hash satisfies them today; any
+ stable, cross-process-reproducible mapping that holds the acceptance conforms,
+ and swapping it is not a violation.
+
+ The probe is inserted **mid-vocabulary**, not appended: `DI999` sorts at
+ position 5 of 48, between `DI005` and `EFF001`, shifting the sorted index of
+ the remaining 42 codes — which is what makes an index-derived shape score
+ churn 42 here. It is removed again in a `finally`, so the live TITLES the
+ rest of the suite sees is untouched."""
  probe = "DI999"
  if probe in TITLES: # pragma: no cover - defensive
- return 0
+ raise AssertionError(
+ f"{probe} is a real diagnostic code now, so it cannot act as the "
+ f"insertion probe; pick an unused code rather than letting this "
+ f"check silently pass"
+ )
  before = {case["code"]: case for case in build()["cases"]}
  TITLES[probe] = "ledger insertion probe (not a real diagnostic)"
  try:
  after = {case["code"]: case for case in build()["cases"]}
  finally:
  del TITLES[probe]
- shared = set(before) & set(after) - {probe}
- return sum(1 for code in shared if before[code] != after[code])
+ shared = (set(before) & set(after)) - {probe}
+ return _InsertionEffect(
+ churn=sum(1 for code in shared if before[code] != after[code]),
+ added=len(set(after) - set(before)),
+ removed=len(set(before) - set(after)),
+ )
 
 
 if __name__ == "__main__":