diff --git a/rust/crates/own-diagnostics/tests/ledger_replay.rs b/rust/crates/own-diagnostics/tests/ledger_replay.rs index ecad8e16..0df39a37 100644 --- a/rust/crates/own-diagnostics/tests/ledger_replay.rs +++ b/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 = 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!( diff --git a/tests/test_diag_ledger_fixtures.py b/tests/test_diag_ledger_fixtures.py index aa941be0..6e05e4b2 100644 --- a/tests/test_diag_ledger_fixtures.py +++ b/tests/test_diag_ledger_fixtures.py @@ -26,7 +26,7 @@ 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. @@ -34,6 +34,10 @@ 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__":