From aee79ecc44ca083a61d233d14b37db2a2722c7e7 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 7 Aug 2026 13:12:00 +0000 Subject: [PATCH 1/2] test(diagnostics): close the rule-4 acceptance gap and three ledger nitpicks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit P-022 rule 4 states two lines as normative acceptance; the executable guard implemented only the first. This makes the guard match the norm already accepted in #322, and no more. _insertion_effect() now returns both halves. They fail in opposite directions and neither implies the other: * churn == 0 — an existing record changed. Paid for: an index-derived shape scored 42 of 47 before the seed was made position-independent. * delta == 1 — the new member produced exactly one record. Zero means the generator never covered the new family, and a generator that skips the new code has *perfect* churn, so the first half cannot see it. Above one means a record count derives from the vocabulary's size rather than its members. Both halves mutation-proved, each caught by exactly one half: * seed reverted to sorted index -> churn 42, delta 1; * generator iterating the committed golden's codes instead of TITLES -> churn 0, delta 0. The churn gate sits behind the staleness check, so it was additionally verified after regenerating under the mutation, where it fires with its own message. Three nitpicks from the #321 review, also mutation-checked: * analyzer_corpus read with expect, matching every other accessor in the file. unwrap_or(false) would silently under-count coverage if the generator stopped emitting the flag; dropping the key from one case now fails naming it. * changed == unexplained asserted explicitly. They move together by construction because no explanation mechanism exists, and the assertion forces that to be revisited deliberately if one is added. Recorded honestly: while the tree is green this is 0 == 0 and proves nothing. Verified by a compound mutation (corrupt one recorded text so a divergence exists, drop the unexplained increment) — it fires first, naming the counter split. * every_optional_field_shape_is_still_exercised guards the nine arms the generator rotates. Collapsing the two-evidence arm and regenerating fails only this test: the divergence check still passes 47/47, because it asks whether each case matches its own recorded text, never whether a shape disappeared. Refs #255, #250. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01CJF7MBi1ijU5m9cJVWgQsM --- .../own-diagnostics/tests/ledger_replay.rs | 101 +++++++++++++++++- tests/test_diag_ledger_fixtures.py | 80 +++++++++++--- 2 files changed, 159 insertions(+), 22 deletions(-) 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..58ea271d 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 and add exactly one. This is P-022 rule 4's normative + acceptance, enforced by [`_insertion_effect`]; the two halves fail in opposite + directions and neither implies the other. 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,85 @@ 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.delta != 1: + print(f"FAIL: inserting one code produced {effect.delta} 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 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"and adds {effect.delta}") 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. + + The two numbers P-022 rule 4 states as normative acceptance: + + insert one synthetic vocabulary member + existing-record churn == 0 + new-record delta == 1 + """ + + churn: int + """EXISTING records whose content changed. Must be 0.""" + delta: int + """Records that did not exist before. Must be exactly 1.""" + + +def _insertion_effect() -> _InsertionEffect: + """Measure both halves of 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. - 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. + `delta` 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. + + Neither number is the mechanism. A content hash satisfies both today; any + stable, cross-process-reproducible mapping that holds both lines conforms, + and swapping it is not a violation. 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.""" 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]), + delta=len(set(after) - set(before)), + ) if __name__ == "__main__": From 3aecec536739f7b59c5daa39864264b93d57a601 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 7 Aug 2026 13:18:48 +0000 Subject: [PATCH 2/2] test(diagnostics): reject records REMOVED by the insertion probe Codex found a real hole in the guard this PR adds, and it sits exactly between the two numbers already measured. 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; the addition still counts one. Both original numbers read clean while the ledger did not grow at all. Measured, not argued: probe added, OWN052 silently gone, guard ACCEPTS. `_insertion_effect()` now returns churn / added / removed. Each of the three mutations is caught by exactly one number, and the other two are structurally blind to it: seed = sorted index churn=42 added=1 removed=0 iterate the golden's codes churn=0 added=0 removed=0 cap ledger at previous size churn=0 added=1 removed=1 (baseline) churn=0 added=1 removed=0 This does not widen P-022 rule 4. The norm says "new-record delta == 1", and a delta is a gain: a run that adds one record while dropping another has gained nothing. Counting only additions was measuring the norm wrong, so the measurement is split, not the rule. Refs #255, #250. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01CJF7MBi1ijU5m9cJVWgQsM --- tests/test_diag_ledger_fixtures.py | 58 +++++++++++++++++++++--------- 1 file changed, 41 insertions(+), 17 deletions(-) diff --git a/tests/test_diag_ledger_fixtures.py b/tests/test_diag_ledger_fixtures.py index 58ea271d..6e05e4b2 100644 --- a/tests/test_diag_ledger_fixtures.py +++ b/tests/test_diag_ledger_fixtures.py @@ -35,9 +35,9 @@ * **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 and add exactly one. This is P-022 rule 4's normative - acceptance, enforced by [`_insertion_effect`]; the two halves fail in opposite - directions and neither implies the other. + 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) @@ -230,60 +230,83 @@ def run() -> int: 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.delta != 1: - print(f"FAIL: inserting one code produced {effect.delta} new ledger record(s); " + 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"inserting a code rewrites {effect.churn} existing record(s) " - f"and adds {effect.delta}") + f"inserting a code rewrites {effect.churn} existing record(s), " + f"adds {effect.added} and removes {effect.removed}") return 0 class _InsertionEffect(NamedTuple): """What inserting one vocabulary member does to the generated ledger. - The two numbers P-022 rule 4 states as normative acceptance: + P-022 rule 4 states the acceptance as: insert one synthetic vocabulary member existing-record churn == 0 new-record delta == 1 + + `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.""" - delta: int + 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 both halves of rule 4's acceptance against the live generator. + """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. - `delta` is the other half, and it fails in the opposite direction. Zero means + `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. - Neither number is the mechanism. A content hash satisfies both today; any - stable, cross-process-reproducible mapping that holds both lines conforms, + `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. - 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.""" + 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 raise AssertionError( @@ -300,7 +323,8 @@ def _insertion_effect() -> _InsertionEffect: shared = (set(before) & set(after)) - {probe} return _InsertionEffect( churn=sum(1 for code in shared if before[code] != after[code]), - delta=len(set(after) - set(before)), + added=len(set(after) - set(before)), + removed=len(set(before) - set(after)), )