From 9d2b30dc3b5c42bcf45dc3cf07868f6eb27d6cf6 Mon Sep 17 00:00:00 2001 From: Timothy Date: Tue, 4 Aug 2026 20:15:16 +0200 Subject: [PATCH 1/9] fix(674,688): cross-check frontmatter against PyYAML; split the ceiling calibration claim MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two defects in scripts/decisions_validate.py, fixed together because they share the validator and its pytest suite. #674 — the validator reported OK on frontmatter PyYAML rejects. The hand-rolled reader is deliberately dependency-free (decisions-guard and the Husky hooks install nothing), so it cannot see a bare apostrophe closing a single-quoted scalar. Hit twice in one session by two independent agents. `pyyaml_frontmatter_faults()` now cross-checks the parse against PyYAML whenever PyYAML is importable, and is SKIPPED with a ::notice:: when it is not — the read path stays dependency-free. The two known hazards fail differently and the fix covers both: the apostrophe makes PyYAML reject the document, while an unquoted ` #` parses fine and silently TRUNCATES the value. So the check compares parsed results key by key rather than try/except-ing the load, which is also what makes it generalize past the two known characters. PyYAML wrote these files, so on disagreement it is authoritative and the file is the defect. The comparison has one implementation, called by the validator and by the existing test_decisions_lib agreement test, so the tool and the suite cannot drift. #688 — test_real_corpus_ceiling_sits_at_the_TAIL_BOUNDARY asserted p90 <= 60 <= p95 in the BLOCKING script-tests job. p90 sat exactly on the ceiling and the distribution above it is sparse, so one ordinary record moved p90 by twenty lines and reddened CI for whoever wrote it; it reproduced twice live (#672, #706) and both times the only in-scope remedy was trimming the new record to fit the constant. v5 splits the claim by robustness instead of hunting for a better single assertion. The blocking test now asserts only the coarse, non-ratcheting property (the ceiling flags a nonempty proper minority, 0 < fraction_over < 1/3); the fine tail-boundary claim is measured every run and REPORTED as a ::notice::, on the same reasoning stale_records already uses — a constant going out of date is the passage of corpus growth, not a defect in the commit under test. The fine property is still asserted, against synthetic distributions the test owns. The ceiling stays 60. Verification: 424 scripts/tests pass; ruff at baseline parity (47 before and after); the cross-check is clean on all 183 real records; a positive control pins that record_wing_faults alone still reports both hazard files as clean, so the new red cannot pass for the wrong reason; and a test demonstrates that appending #672's 62-line and #687's 107-line records to the real corpus does not red the blocking property. Docs: new record docs.frontmatter-pyyaml-crosscheck, docs.corpus-size-signal updated for the v5 split, catalog regenerated, docs/ci-cd.md updated for both. fixes #674 fixes #688 Decisions-Edit: yes Co-Authored-By: Claude Opus 5 (1M context) --- docs/ci-cd.md | 22 +- docs/decisions/README.md | 3 +- .../records/docs/corpus-size-signal.md | 31 ++- .../docs/frontmatter-pyyaml-crosscheck.md | 66 +++++ scripts/decisions_validate.py | 177 +++++++++++++- scripts/tests/test_decisions_lib.py | 25 +- scripts/tests/test_decisions_validate.py | 228 ++++++++++++++++-- 7 files changed, 505 insertions(+), 47 deletions(-) create mode 100644 docs/decisions/records/docs/frontmatter-pyyaml-crosscheck.md diff --git a/docs/ci-cd.md b/docs/ci-cd.md index a5e9a01db..53c72644a 100644 --- a/docs/ci-cd.md +++ b/docs/ci-cd.md @@ -47,7 +47,12 @@ active record, relocate the predecessor to `docs/decisions/archive/` with recipr `supersedes`/`superseded-by` links), not a release-boundary batch job — most of the old "consolidate" step is now continuous. The release boundary is instead where you: 1. Run `PYTHONPATH=. python3 scripts/decisions_validate.py` — confirms lifecycle metadata is - well-formed and every `supersedes`/`superseded-by` link resolves both ways. + well-formed and every `supersedes`/`superseded-by` link resolves both ways. Since **ersatztv#674** + it also cross-checks its dependency-free frontmatter parse against **PyYAML when PyYAML is + importable**, failing on any file PyYAML rejects (a bare apostrophe in a single-quoted value) or + reads differently (an unquoted ` #`, which YAML truncates as a comment). Where PyYAML is absent — + the `decisions-guard` job, the Husky hooks — the cross-check is **skipped with a `::notice::`** + and every other check still runs; the read path stays dependency-free. 2. Confirm every record already classified `superseded`/`retired` actually lives under `docs/decisions/archive/` (the validator fails this, but eyeball it at the boundary too). 3. Regenerate the active catalog: `PYTHONPATH=. python3 scripts/build_decisions_catalog.py` and @@ -56,10 +61,17 @@ step is now continuous. The release boundary is instead where you: - a **per-record prose ceiling** (`decisions_validate.py --record-ceiling `, default **60**) — a **non-blocking `::warning::`** naming every record over it. This is the actionable signal: it points at a file. The 60 is derived from the distribution, not picked as a round number. - A test pins what that derivation MEANS rather than any particular numbers: the ceiling must sit - between the **90th and 95th percentile** of record lengths, i.e. at the tail boundary. Stated - as percentiles it is scale-free, so ordinary corpus growth cannot ratchet it — it fires only - when the ceiling genuinely stops marking the tail and should be re-derived. + Its **calibration is guarded in two pieces of different robustness** (ersatztv#688), because + four earlier single-assertion versions all ratcheted: + - **blocking** (`script-tests`) — only the coarse property that the ceiling flags a nonempty + **proper minority** of records (`0 < fraction_over < 1/3`). One record moves a fraction by at + most 1/N, so routine growth cannot cross it. + - **reported, never asserted** — the fine claim that the ceiling sits between the **90th and + 95th percentile**, i.e. at the tail boundary. `main()` prints a `::notice::` when it drifts. + It is an order statistic over a sparse distribution, so a single new record could move p90 by + twenty lines and red the blocking job for whoever wrote it; a ceiling going out of date is + the passage of corpus growth, not a defect in the commit under test, so it is treated like + `stale-after`. Re-derive the constant when the notice says so. - the **aggregate prose total**, printed every run as an unthresholded `::notice::` **trend**. It has no pass/fail. A total over a monotonically growing corpus can only ratchet: the old 4800→5600 budget went quiet at 5228 after #610 changed the metric and was back over at 5658 diff --git a/docs/decisions/README.md b/docs/decisions/README.md index 8e4dbbb1d..a690c4e60 100644 --- a/docs/decisions/README.md +++ b/docs/decisions/README.md @@ -72,10 +72,11 @@ the link for rationale. Superseded/retired history lives in `archive/`. Regenera | `concurrency.replace-all-contract` | Replace-all aggregate PUTs carry a uniform plain `int Version` concurrency token (EF `.IsConcurrencyToken()`), checked pre-save and enforced by the EF UPDATE guard, returning 412 (not 409) on a stale `If-Match`. | 2026-07-11 | [link](records/concurrency/replace-all-contract.md) | | `concurrency.schedule-item-child-identity` | `PUT /api/schedules/{id}/items` reconciles by an optional round-tripped child `Id` (null/absent/0 ⇒ new item), never by array position, so fill-group/shuffle state follows the logical item across reorders; an unknown or duplicate id is rejected 422 (checked after the §7a `CheckVersion`, so 412 precedes 422). | 2026-07-11 | [link](records/concurrency/schedule-item-child-identity.md) | | `docs.convention-docs-session-start` | Docs-first, not source-first: conventions (api-conventions, spa-conventions, e2e-local, blazor-route-parity, domain-model, decisions, README) are read from docs, not reverse-engineered from code, via `docs/README.md`'s task-signal map — only the sections it points to for the task at hand, not the whole set. Each doc is updated in the same PR that changes what it documents, replacing deferred/follow-up doc updates. | 2026-07-07 | [link](records/docs/convention-docs-session-start.md) | -| `docs.corpus-size-signal` | The corpus's size signal is a per-record prose ceiling (`decisions_validate.py --record-ceiling`, default 60, chosen at a natural gap in the distribution), reported as a NON-BLOCKING `::warning::` naming each record over it. The aggregate prose total is still printed every run but carries NO threshold — it is a `::notice::` trend only — because a total over a monotonically growing corpus can only ratchet, and the generated catalog (`docs/decisions/README.md`) is no longer counted at all since it gains one row per record and cannot be consolidated away. Being listed by the ceiling is an invitation to check for REDUNDANCY, never an instruction to cut: a long record that is all distinct findings is a legitimate decline, and should be recorded as one. | 2026-07-26 | [link](records/docs/corpus-size-signal.md) | +| `docs.corpus-size-signal` | The corpus's size signal is a per-record prose ceiling (`decisions_validate.py --record-ceiling`, default 60, chosen at a natural gap in the distribution), reported as a NON-BLOCKING `::warning::` naming each record over it. The aggregate prose total is still printed every run but carries NO threshold — it is a `::notice::` trend only — because a total over a monotonically growing corpus can only ratchet, and the generated catalog (`docs/decisions/README.md`) is no longer counted at all since it gains one row per record and cannot be consolidated away. Being listed by the ceiling is an invitation to check for REDUNDANCY, never an instruction to cut: a long record that is all distinct findings is a legitimate decline, and should be recorded as one. The ceiling's CALIBRATION is guarded in two pieces of different robustness (#688): the blocking test asserts only the coarse, non-ratcheting property that the ceiling flags a nonempty PROPER MINORITY of records (`0 < fraction_over < 1/3`), while the fine claim — that it sits between p90 and p95 — is REPORTED by `main()` as a `::notice::` and never asserted against the live corpus. A ceiling drifting out of date is the passage of corpus growth, not a defect in the commit under test, so it gets `stale_records`' treatment rather than a red in the blocking `script-tests` job. | 2026-07-26 | [link](records/docs/corpus-size-signal.md) | | `docs.decision-lifecycle` | every decision `##` record (active or archived) carries a 5-field metadata block (`key`, `status`, `since`, `supersedes`, `superseded-by`) checked by `scripts/decisions_validate.py`; a record is never deleted or line-edited to reverse a call — it is moved to `docs/decisions/archive/` with `status: superseded`/`retired` and a reciprocal `superseded-by`/`supersedes` key pair to its replacement. | 2026-07-21 | [link](records/docs/decision-lifecycle.md) | | `docs.decision-one-file-per-record` | Each decision record is its own file at `docs/decisions/records//.md` (archived ones at `docs/decisions/archive//.md`) with YAML frontmatter; the filename IS the key, so one-active-record-per-key is a filesystem property rather than a validator check, and supersession is a `git mv`. | 2026-07-25 | [link](records/docs/decision-one-file-per-record.md) | | `docs.decision-optional-provenance` | Decision records gain two OPTIONAL fields — `stale-after: YYYY-MM-DD` on the metadata line and a `**Sources:**` line in the metadata block; the Open Knowledge Format (OKF) itself is NOT adopted as the record format. | 2026-07-25 | [link](records/docs/decision-optional-provenance.md) | +| `docs.frontmatter-pyyaml-crosscheck` | `decisions_validate.py` runs `pyyaml_frontmatter_faults()` over every record-wing file: it loads the frontmatter with PyYAML and reports an ERROR when PyYAML rejects the document OR when any key's value differs from what the dependency-free `dl._read_frontmatter` read. PyYAML is the WRITER of these files (`migrate_decisions_split.render_record` emits them with `yaml.safe_dump`), so on any disagreement PyYAML is authoritative and the defect is in the FILE, not in either parser. The check is strictly additive: when PyYAML is not importable it is SKIPPED and `main()` says so with a `::notice::`, never silently — the read path stays dependency-free because `decisions-guard`, the Husky hooks and contributor machines install nothing. The comparison has exactly ONE implementation, called by both the validator and `test_frontmatter_reader_matches_pyyaml_on_every_real_record`, so the suite and the tool cannot drift on what "matches PyYAML" means. | 2026-08-04 | [link](records/docs/frontmatter-pyyaml-crosscheck.md) | | `docs.record-wing-parse-guard` | `decisions_validate.py` asserts, per PATH, that every `*.md` under `docs/decisions/records/**` and `docs/decisions/archive/**` parses to exactly one record carrying a `key` — an ERROR, not a warning, since a file in the record wings that is not a record is a mistake by definition. A file sitting DIRECTLY in `archive/` is exempt only when it actually looks like a #610 stripped index — exactly one keyless record with a known generated heading — never merely by living there. The one other exemption, `archive/README.md`, is by exact RELATIVE PATH; nothing is ever exempt by BASENAME, since that would exempt the same filename in the active wing too. `_read_frontmatter` is deliberately NOT extended to accept YAML block scalars: every record value goes on ONE line, and the structural check is what makes that limitation loud instead of silent. | 2026-07-26 | [link](records/docs/record-wing-parse-guard.md) | | `docs.tracker-comment-retrofit` | When the knowledge exporter flags an over-cap tracker issue and excludes it from ingestion, triage its comments instead of assuming a retrofit is owed — and for each decision-shaped item check the **worked issue first**, because a tracker session comment is by construction a précis of the fuller closing record posted on the issue it narrates. Applied to #237 (111 comments) this yielded **zero** records, so server-management#642's "a fact found only in a #237 comment" retrieval row has no valid subject and its interim target (an already-migrated record) is permanent. | 2026-07-21 | [link](records/docs/tracker-comment-retrofit.md) | | `ffmpeg.external-logo-graphics-engine` | External-URL channel logos pass through to the graphics engine like any other watermark source; `WatermarkSelector` must never gate them on `File.Exists` (always false for a URL) and never route them through the ffmpeg-native overlay shortcut. | 2026-07-20 | [link](records/ffmpeg/external-logo-graphics-engine.md) | diff --git a/docs/decisions/records/docs/corpus-size-signal.md b/docs/decisions/records/docs/corpus-size-signal.md index 73ae85459..be3c71868 100644 --- a/docs/decisions/records/docs/corpus-size-signal.md +++ b/docs/decisions/records/docs/corpus-size-signal.md @@ -5,8 +5,8 @@ status: active since: '2026-07-26' supersedes: none superseded-by: none -rule: 'The corpus''s size signal is a per-record prose ceiling (`decisions_validate.py --record-ceiling`, default 60, chosen at a natural gap in the distribution), reported as a NON-BLOCKING `::warning::` naming each record over it. The aggregate prose total is still printed every run but carries NO threshold — it is a `::notice::` trend only — because a total over a monotonically growing corpus can only ratchet, and the generated catalog (`docs/decisions/README.md`) is no longer counted at all since it gains one row per record and cannot be consolidated away. Being listed by the ceiling is an invitation to check for REDUNDANCY, never an instruction to cut: a long record that is all distinct findings is a legitimate decline, and should be recorded as one.' -signals: 'aggregate active-corpus budget, schedule a consolidation warning, permanently red ratchet, per-record ceiling, 60-line prose ceiling, generated catalog counted in budget, corpus consolidation has no owner, size is not redundancy · paths: `scripts/decisions_validate.py`, `scripts/tests/test_decisions_validate.py`, `docs/ci-cd.md` · issues: #620, #610, #603, #542, #520' +rule: 'The corpus''s size signal is a per-record prose ceiling (`decisions_validate.py --record-ceiling`, default 60, chosen at a natural gap in the distribution), reported as a NON-BLOCKING `::warning::` naming each record over it. The aggregate prose total is still printed every run but carries NO threshold — it is a `::notice::` trend only — because a total over a monotonically growing corpus can only ratchet, and the generated catalog (`docs/decisions/README.md`) is no longer counted at all since it gains one row per record and cannot be consolidated away. Being listed by the ceiling is an invitation to check for REDUNDANCY, never an instruction to cut: a long record that is all distinct findings is a legitimate decline, and should be recorded as one. The ceiling''s CALIBRATION is guarded in two pieces of different robustness (#688): the blocking test asserts only the coarse, non-ratcheting property that the ceiling flags a nonempty PROPER MINORITY of records (`0 < fraction_over < 1/3`), while the fine claim — that it sits between p90 and p95 — is REPORTED by `main()` as a `::notice::` and never asserted against the live corpus. A ceiling drifting out of date is the passage of corpus growth, not a defect in the commit under test, so it gets `stale_records`'' treatment rather than a red in the blocking `script-tests` job.' +signals: 'aggregate active-corpus budget, schedule a consolidation warning, permanently red ratchet, per-record ceiling, 60-line prose ceiling, generated catalog counted in budget, corpus consolidation has no owner, size is not redundancy, ceiling calibration reddens script-tests, adding a record fails CI on length, p90 sits on the ceiling, tail-boundary drift notice · paths: `scripts/decisions_validate.py`, `scripts/tests/test_decisions_validate.py`, `docs/ci-cd.md` · issues: #688, #620, #610, #603, #542, #520' mechanics: '`scripts/decisions_validate.py` -> `oversized_records` / `_budget_total`; `docs/ci-cd.md` -> "`decisions-guard` job"' --- @@ -46,19 +46,34 @@ meant to prevent exactly that could not see it: `max(under) <= 60 < min(over)` i construction** of the two lists it builds, and passes on a distribution with no gap at all. A rationale-guarding test that cannot fail is worse than none, because it launders the claim. -It is replaced by `test_real_corpus_ceiling_sits_at_the_TAIL_BOUNDARY_of_the_distribution`, which -states the property directly and scale-free: **the ceiling sits between the 90th and 95th percentile -of record lengths** — that is what "marks the start of the tail" means — and reads the value from -`RECORD_CEILING_DEFAULT` so test and CLI cannot drift. +It is replaced by `test_real_corpus_ceiling_flags_a_nonempty_proper_minority`, which reads the value +from `RECORD_CEILING_DEFAULT` so test and CLI cannot drift. -Getting there took four versions, and the failures are the useful part: +Getting there took five versions, and the failures are the useful part: | | assertion | why it failed | |---|---|---| | v1 | `max(under) <= 60 < min(over)` | true **by construction** of those two lists | | v2 | a minimum gap WIDTH | a ceiling of 200 also sits in a wide gap — it passed | | v3 | 2-12% fraction band + "clear air" above | **hostage to an unrelated record**: one ordinary 62-line addition reddened it with the ceiling correctly placed, and the only remedy was to RAISE the ceiling — this very treadmill, as a hard failure in what #631 makes a blocking job. The fraction band had the same coupling more slowly (12 more long records breached it), and `0 <= headroom` was vacuous. | -| v4 | `p90 <= ceiling <= p95` | percentiles move WITH the corpus, so routine growth cannot ratchet it; it fires only when the ceiling genuinely stops marking the tail | +| v4 | `p90 <= ceiling <= p95` | percentiles move with the corpus, but an order statistic over a SPARSE distribution is a STEP function. The lengths above the ceiling ran 60, 61, 62, 63 then jumped to 81, so ONE record moved p90 by twenty lines. It reddened the blocking job twice live (#672, #706), and both times the only in-scope remedy was to trim the new record to fit the constant — the v3 ratchet, pointed at record authors | +| v5 | coarse `0 < fraction_over < 1/3` asserted; fine `p90 <= ceiling <= p95` REPORTED | splits the claim by robustness instead of hunting for a better single assertion (#688) | + +**v5 is not a fifth attempt at the same shape — it stops trying.** Four versions failed because they +all asserted, in the blocking job, a property of a corpus the commit under test does not control. +The fine claim is genuinely useful and genuinely fragile, so it is now measured on every run and +printed as a `::notice::` — the same treatment `stale_records` gets, and for the same stated reason: +a constant going out of date is the passage of time, not a defect in this change. What stays +blocking is only what routine growth cannot break — each record moves a fraction by at most 1/N, so +from 18/182 over the ceiling it would take ~40 consecutive over-ceiling additions to reach a third, +against ~1 record to break v4. The coarse bound still catches real mis-calibration in both +directions: a ceiling of 200 flags nobody, a ceiling of 20 flags most of the corpus. + +The honest cost, stated rather than buried: **nothing now forces a re-derivation.** The ceiling can +drift while only a notice complains. That is accepted on the same reasoning this record already +applies to its two "keep listed" consolidation candidates — the warning names it on every run, which +tracks it better than a red that gets trimmed around, and a red an author can only clear by editing +an unrelated constant is not enforcement, it is a toll. Two rules came out of that sequence, and they outlive this metric: **a guard test must depend only on the thing it guards**, and diff --git a/docs/decisions/records/docs/frontmatter-pyyaml-crosscheck.md b/docs/decisions/records/docs/frontmatter-pyyaml-crosscheck.md new file mode 100644 index 000000000..b3d32bbcf --- /dev/null +++ b/docs/decisions/records/docs/frontmatter-pyyaml-crosscheck.md @@ -0,0 +1,66 @@ +--- +key: docs.frontmatter-pyyaml-crosscheck +title: '2026-08-04 — `decisions_validate.py` cross-checks its dependency-free frontmatter parse against PyYAML whenever PyYAML is importable (#674)' +status: active +since: '2026-08-04' +supersedes: none +superseded-by: none +rule: '`decisions_validate.py` runs `pyyaml_frontmatter_faults()` over every record-wing file: it loads the frontmatter with PyYAML and reports an ERROR when PyYAML rejects the document OR when any key''s value differs from what the dependency-free `dl._read_frontmatter` read. PyYAML is the WRITER of these files (`migrate_decisions_split.render_record` emits them with `yaml.safe_dump`), so on any disagreement PyYAML is authoritative and the defect is in the FILE, not in either parser. The check is strictly additive: when PyYAML is not importable it is SKIPPED and `main()` says so with a `::notice::`, never silently — the read path stays dependency-free because `decisions-guard`, the Husky hooks and contributor machines install nothing. The comparison has exactly ONE implementation, called by both the validator and `test_frontmatter_reader_matches_pyyaml_on_every_real_record`, so the suite and the tool cannot drift on what "matches PyYAML" means.' +signals: 'validator reports OK on a broken record, bare apostrophe in single-quoted frontmatter, unquoted hash truncates a value, hand-rolled frontmatter parser, PyYAML rejects the file but decisions-validate passes, dependency-free read path, frontmatter cross-check skipped · paths: `scripts/decisions_validate.py`, `scripts/decisions_lib.py`, `scripts/tests/test_decisions_validate.py`, `scripts/tests/test_decisions_lib.py` · issues: #674, #578, #651, #621' +mechanics: '`scripts/decisions_validate.py` -> `pyyaml_frontmatter_faults` / `_frontmatter_block`, wired into `main()` alongside `record_wing_faults`' +--- + +The validator read ordinary English prose in a `rule:` field and reported **OK** on a file PyYAML +refuses to parse. It was hit **twice in one session by two independent agents** on unrelated +branches (#578, #651), which is what makes it a guard rather than a note: it is not an exotic edge +case, it is what happens when anyone writes `SQLite's LOWER()` into a single-quoted scalar. + +**Why the hand parser exists, and why it stays.** `dl._read_frontmatter` is deliberately +dependency-free — it runs in `decisions-guard`, in the Husky hooks, and on every contributor +machine, none of which install anything. Requiring PyYAML there once made the validator crash with +`ModuleNotFoundError` on the very records the split had just written. So the fix could not be +"import yaml in the reader". It is a second, optional opinion layered on top. + +**The two known hazards fail DIFFERENTLY, and that shaped the fix.** + +| input | dependency-free reader | PyYAML | +|---|---|---| +| `rule: 'SQLite's LOWER()'` | `SQLite's LOWER()` | **`ParserError`** — the bare apostrophe closes the scalar early | +| `rule: use --flag #2` | `use --flag #2` | `use --flag` — ` #` starts a comment, **silently truncating** | + +A `try/except yaml.YAMLError` would have caught only the first row. The second produces no +exception at all: a valid record whose `rule` has quietly lost its tail — the `parse-to-WRONG` case +`docs.record-wing-parse-guard` explicitly names as the gap its structural check cannot see. So the +cross-check compares the parsed **result** key by key, and reports a rejection and a mismatch as two +distinct faults with different remedies. + +**That is also what makes it general.** #674 asked for a fix that catches the *next* character class +rather than enumerating hazards one at a time — the mistake the surrounding sessions kept making +elsewhere. Comparing against the writer's own library is that: any construct where the two parsers +disagree surfaces as a diff, with nobody having to predict it. Enumerating apostrophes and hashes +would have left the third character to be discovered the same way the first two were. + +**Direction is the load-bearing part.** PyYAML is not a second opinion of equal standing — it WROTE +these files. When the two disagree the on-disk bytes mean what PyYAML says they mean, so the record +is corrupt and the permissive reader is the one hiding it. Stating the direction is what turns an +ambiguous "parsers differ" report into an actionable "this record is silently wrong". + +**A skip is announced, not silent.** When PyYAML is absent the check does not run, which is correct +and expected on the dependency-free path — but `main()` prints a `::notice::` saying so. A check +that reports success while doing nothing is the defect this corpus keeps re-learning (#603's +`stale-after` that never fired, #609's marker that exempted everything while printing OK), and +adding a quiet skip while fixing a quiet pass would have reintroduced it one level up. + +**One implementation, two callers.** The comparison already existed — in the test suite only, which +is exactly why the validator could disagree with `scripts/tests` about the same file. Rather than +leaving a second copy, `test_frontmatter_reader_matches_pyyaml_on_every_real_record` now delegates +to `pyyaml_frontmatter_faults`, so the tool and the suite cannot drift apart on the definition. + +**What it still does not catch.** A mis-parse both parsers agree on, and anything in a file PyYAML +accepts as a mapping with the schema's own key names. The guard is strong, not total — the same +qualification `record_wing_faults` carries. + +**Tested with a positive control.** The suite pins that `record_wing_faults` alone reports both +hazard files as CLEAN. Without it the cross-check could be deleted and the tests would stay green +while the guard vanished; asserting the pre-#674 machinery's blindness is what makes the new red +mean something. diff --git a/scripts/decisions_validate.py b/scripts/decisions_validate.py index a52fda8e9..988f582a1 100644 --- a/scripts/decisions_validate.py +++ b/scripts/decisions_validate.py @@ -17,6 +17,7 @@ import subprocess import sys from datetime import date from pathlib import Path +from typing import NamedTuple import scripts.decisions_lib as dl # noqa: E402 (run with PYTHONPATH=. or as module) @@ -52,6 +53,11 @@ _STALE_AFTER_RE = re.compile(r"^\d{4}-\d{2}-\d{2}$") # guards the calibration claim asserts against the SAME value the CLI uses and the two cannot drift. RECORD_CEILING_DEFAULT = 60 +# The coarse, non-ratcheting calibration bound (#688): the ceiling must flag a nonempty PROPER +# MINORITY of records. Above a third it is cutting into the bulk rather than marking a tail. See +# `ceiling_calibration` for why the fine percentile claim is reported instead of asserted. +CEILING_MINORITY_MAX = 1 / 3 + def _parse_stale_after(value: str | None) -> date | None: """`stale-after` as a date, or None if absent, empty, or malformed. @@ -541,6 +547,92 @@ def record_wing_faults(records_dir: Path | None = None, archive_dir: Path | None return faults +def _frontmatter_block(text: str) -> str | None: + """The raw YAML between the opening `---` and the next `---`, or None if there isn't one.""" + if not dl.has_frontmatter(text): + return None + lines = text.splitlines() + end = next((i for i, ln in enumerate(lines[1:], start=1) if ln.rstrip() == "---"), None) + if end is None: + return None + return "\n".join(lines[1:end]) + + +def pyyaml_frontmatter_faults(files) -> tuple[list[str], bool]: + """Faults where PyYAML disagrees with the dependency-free reader. Returns (faults, ran). + + #674: the two known hazards are a bare apostrophe inside a single-quoted scalar + (`rule: 'SQLite's LOWER()'`) and an unquoted ` #` (`rule: use --flag #2`). Before this check the + validator reported OK on both, because `dl._read_frontmatter` is a hand parser that cannot see + either. It was hit TWICE in one session by two independent agents, which is what makes it worth + a guard rather than a note. + + The two hazards fail DIFFERENTLY, and catching only the first would have missed half of it: + + * the apostrophe makes PyYAML **reject** the document outright (`ParserError`); + * the unquoted ` #` parses fine and **silently truncates** the value — PyYAML reads + `use --flag`, the hand parser reads `use --flag #2`. No exception, a wrong value. + + So this compares the parsed RESULT and does not merely try/except the load. That is also why it + generalizes past the two known characters, which is the property #674 asked for: any future + construct where the writer's library and our reader disagree shows up as a diff, without anyone + enumerating it first. + + DIRECTION MATTERS: PyYAML is the WRITER (`migrate_decisions_split.render_record` emits these + files with `yaml.safe_dump`), so it is the authority on what the on-disk bytes mean. The hand + reader is the permissive one, and a disagreement is a defect in the FILE, not in either parser. + + `ran` is False when PyYAML is not importable. The read path is deliberately dependency-free — + `decisions-guard`, the Husky hooks and every contributor machine install nothing — so this check + is strictly additive: it must never be the reason the validator cannot run. main() announces the + skip rather than passing quietly, because a check that reports success while doing nothing is + the exact defect class this corpus keeps re-learning (#603's `stale-after`, #609's marker). + """ + try: + import yaml # pyright: ignore[reportMissingImports] + except Exception: + return [], False + + faults: list[str] = [] + for p in files: + try: + text = p.read_text(encoding="utf-8") + except Exception: # noqa: S112 (record_wing_faults already reports the unreadable file by name) + continue + block = _frontmatter_block(text) + if block is None: + continue # no/unterminated frontmatter: reported by record_wing_faults + try: + theirs_raw = yaml.safe_load(block) + except yaml.YAMLError as exc: + first = str(exc).splitlines()[0] if str(exc).strip() else exc.__class__.__name__ + faults.append( + f"{p}: PyYAML REJECTS this frontmatter, though the dependency-free reader accepted " + f"it ({first}). PyYAML is what WROTE these files, so its verdict is authoritative. " + f"Common cause: a bare apostrophe inside a single-quoted value (write '' for a " + f"literal apostrophe, as `yaml.safe_dump` does)." + ) + continue + if theirs_raw is None: + theirs_raw = {} + if not isinstance(theirs_raw, dict): + faults.append(f"{p}: frontmatter parses as {type(theirs_raw).__name__}, not a mapping.") + continue + mine = dl._read_frontmatter(block) + if mine is None: + continue # the reader bailed: reported by record_wing_faults as parse-to-0 + theirs = {k: ("" if v is None else str(v)) for k, v in theirs_raw.items()} + for k in sorted(set(mine) | set(theirs)): + if mine.get(k) != theirs.get(k): + faults.append( + f"{p}: frontmatter key {k!r} means different things to the two parsers — " + f"reader={mine.get(k)!r} but PyYAML={theirs.get(k)!r}. PyYAML wrote this file, " + f"so its reading is the real value and the record is silently corrupt. Common " + f"cause: an unquoted ` #`, which YAML treats as a comment and truncates there." + ) + return faults, True + + def _unknown_frontmatter_keys(path: Path) -> set[str]: """Frontmatter keys outside the known schema. Empty on any read/parse failure (reported elsewhere).""" try: @@ -642,6 +734,62 @@ def oversized_records(records, ceiling: int) -> list[tuple[str, int]]: return sorted([kv for kv in out if kv[1] > ceiling], key=lambda kv: -kv[1]) +class CeilingCalibration(NamedTuple): + n: int + p90: int + p95: int + n_over: int + fraction_over: float + marks_tail: bool # the FINE claim: p90 <= ceiling <= p95 + flags_minority: bool # the COARSE claim: 0 < fraction_over < CEILING_MINORITY_MAX + + +def ceiling_calibration(records, ceiling: int) -> CeilingCalibration: + """How well `ceiling` still marks the start of the corpus's tail (#688). + + Two claims of DIFFERENT robustness, deliberately separated, because conflating them is what + made the previous guard a ratchet: + + `marks_tail` — `p90 <= ceiling <= p95`. Correct as a definition of "start of the tail", but an + order statistic over a SPARSE distribution is a STEP function: at #688 the lengths just above + the ceiling ran 60, 61, 62, 63, then jumped to 81, so ONE new record could move p90 by twenty + lines and the only remedy the assertion admitted was to raise the ceiling. It is real signal, + but it is signal about the CONSTANT drifting, not a defect in the commit under test — the same + shape as `stale_records`, and it is reported the same way: a notice, never a failure. + + `flags_minority` — `0 < fraction_over < 1/3`. Deliberately coarse, and what the blocking test + asserts. Each added record moves a fraction by at most 1/N, so routine growth cannot cross it: + at 17/176 over the ceiling, reaching a third would take ~42 consecutive over-ceiling additions. + It still catches genuine mis-calibration in both directions — a ceiling of 200 flags nobody + (fraction 0, signals nothing), a ceiling of 20 flags most of the corpus (cutting into the bulk, + so every author learns to ignore it). + + Why not simply re-derive the constant instead: re-deriving fixes the instance and keeps the + mechanism. The v3 fraction band and the v4 percentile containment both failed the same way, one + faster than the other, and picking a new number would queue up the fifth version. + """ + lengths = sorted(record_prose_lines(r) for r in records if r.key) + n = len(lengths) + if n == 0: + return CeilingCalibration(0, 0, 0, 0, 0.0, False, False) + + def pct(q: float) -> int: + return lengths[min(int(n * q), n - 1)] + + n_over = sum(1 for v in lengths if v > ceiling) + frac = n_over / n + p90, p95 = pct(0.90), pct(0.95) + return CeilingCalibration( + n=n, + p90=p90, + p95=p95, + n_over=n_over, + fraction_over=frac, + marks_tail=p90 <= ceiling <= p95, + flags_minority=0 < frac < CEILING_MINORITY_MAX, + ) + + def _catalog_ok() -> bool: try: import scripts.build_decisions_catalog as bc # pyright: ignore[reportMissingImports] @@ -684,6 +832,19 @@ def main(argv=None) -> int: archive_records += dl.parse_file(f) removed, rewritten, demoted = _diff_findings(args.base, args.head) if args.base and args.head else ([], [], []) oversized = oversized_records(records, args.record_ceiling) + + # #674: cross-check the dependency-free reader against the library that WROTE these files. + # Strictly additive — absent PyYAML skips the check (and SAYS so) rather than failing the run. + yaml_faults, yaml_ran = pyyaml_frontmatter_faults(record_wing_files()) + if not yaml_ran: + print( + "::notice::decisions-validate: PyYAML is not importable, so the frontmatter cross-check " + "was SKIPPED — every other check ran. This is the expected state on the dependency-free " + "read path (decisions-guard, the Husky hooks); CI's script-tests job runs it with PyYAML " + "present.", + file=sys.stderr, + ) + errs = validate( records, archive_keys=_archive_keys(), @@ -692,7 +853,7 @@ def main(argv=None) -> int: rewritten=rewritten, archive_records=archive_records, demoted=demoted, - wing_faults=record_wing_faults(), + wing_faults=record_wing_faults() + yaml_faults, ) # Aggregate: an unthresholded TREND, not a gate (#620). Printed every run so the number stays @@ -723,6 +884,20 @@ def main(argv=None) -> int: file=sys.stderr, ) + # Ceiling calibration drift (#688): a NOTICE, never a failure. The ceiling drifting away from + # the tail boundary is the passage of corpus growth, not a defect in the commit under test — the + # same reasoning `stale_records` is built on. Asserting it in the blocking `script-tests` job + # made the next author of a substantial record pay for an unrelated constant going out of date. + cal = ceiling_calibration(records, args.record_ceiling) + if cal.n and not cal.marks_tail: + print( + f"::notice::decisions-validate: the {args.record_ceiling}-line ceiling has drifted from " + f"the tail boundary of the distribution (p90={cal.p90}, p95={cal.p95}, " + f"{cal.n_over}/{cal.n} records over it). Re-derive it when convenient — this is a " + f"maintenance signal about the constant, not a problem with this change.", + file=sys.stderr, + ) + stale = stale_records(records, date.today()) if stale: listed = "; ".join(f"{h} (stale-after {d})" for h, d in stale) diff --git a/scripts/tests/test_decisions_lib.py b/scripts/tests/test_decisions_lib.py index 6d34d2b04..bc2cf8a06 100644 --- a/scripts/tests/test_decisions_lib.py +++ b/scripts/tests/test_decisions_lib.py @@ -71,23 +71,20 @@ def test_frontmatter_reader_matches_pyyaml_on_every_real_record(): made the validator crash with ModuleNotFoundError once the corpus was migrated.) A hand parser is only safe if it provably matches the library that WROTE the files, so this compares the two across every record rather than on a sample. + + Since #674 the comparison itself lives in `decisions_validate.pyyaml_frontmatter_faults`, which + the VALIDATOR now runs too — before that it existed only here, so `decisions_validate.py` + happily reported OK on a record PyYAML rejects. This test delegates to that one implementation + rather than keeping a second copy of the comparison, so the suite and the validator cannot + drift apart and agree on what "matches PyYAML" means. """ - yaml = pytest.importorskip("yaml") + pytest.importorskip("yaml") + import scripts.decisions_validate as dv files = [p for p in dl.RECORDS_DIR.rglob("*.md")] + [p for p in dl.ARCHIVE_DIR.rglob("*.md")] files = [f for f in files if dl.has_frontmatter(f.read_text(encoding="utf-8"))] assert len(files) > 100, f"only {len(files)} frontmatter files found — test would be near-vacuous" - diffs = [] - for f in files: - lines = f.read_text(encoding="utf-8").splitlines() - end = next(i for i, ln in enumerate(lines[1:], start=1) if ln.rstrip() == "---") - block = "\n".join(lines[1:end]) - mine = dl._read_frontmatter(block) - theirs = yaml.safe_load(block) or {} - theirs = {k: ("" if v is None else str(v)) for k, v in theirs.items()} - if mine != theirs: - for k in set(mine or {}) | set(theirs): - if (mine or {}).get(k) != theirs.get(k): - diffs.append(f"{f.name}:{k}\n mine ={(mine or {}).get(k)!r}\n pyyaml={theirs.get(k)!r}") - assert not diffs, f"{len(diffs)} field(s) differ from PyYAML:\n" + "\n".join(diffs[:5]) + faults, ran = dv.pyyaml_frontmatter_faults(files) + assert ran, "PyYAML is importable here, so the comparison must have actually run" + assert not faults, f"{len(faults)} field(s) differ from PyYAML:\n" + "\n".join(faults[:5]) diff --git a/scripts/tests/test_decisions_validate.py b/scripts/tests/test_decisions_validate.py index 4d0683159..68d608441 100644 --- a/scripts/tests/test_decisions_validate.py +++ b/scripts/tests/test_decisions_validate.py @@ -1002,39 +1002,215 @@ def test_budget_total_excludes_the_generated_catalog(tmp_path, monkeypatch): assert total < 100, f"the 500-line generated catalog leaked into the total ({total})" -def test_real_corpus_ceiling_sits_at_the_TAIL_BOUNDARY_of_the_distribution(): - """Guards the calibration claim. This is the FOURTH version; the failures are the lesson. +def test_real_corpus_ceiling_flags_a_nonempty_proper_minority(): + """Guards the calibration claim. This is the FIFTH version; the failures are the lesson. v1 `max(under) <= 60 < min(over)` — true by construction of those two lists. v2 a minimum gap WIDTH — but a ceiling of 200 also sits in a wide gap, so it passed. v3 a 2-12% fraction band plus "clear air" measured against `min(over)` — the nearest record ABOVE the ceiling. That made the test a hostage to an unrelated record: one ordinary 62-line addition reddened it with the ceiling correctly placed, and the only - remedy the assertion admitted was to RAISE the ceiling. That is the ratchet this whole - change abolishes, reinstated as a hard failure in what #631 makes a blocking CI job. - The fraction band had the same coupling more slowly (12 more long records breached it), - and `0 <= headroom` was vacuous — `max(under)` is by construction <= ceiling. + remedy the assertion admitted was to RAISE the ceiling. + v4 `p90 <= ceiling <= p95`. Scale-free and correct AS A DEFINITION, but an order statistic + over a SPARSE distribution is a STEP function. At #688 the lengths above the ceiling ran + 60, 61, 62, 63 and then jumped to 81, so ONE new record moved p90 by twenty lines and + reddened the BLOCKING `script-tests` job for whoever happened to write it. It reproduced + twice live (#672, #706) and both times the only in-scope remedy was to trim the new + record to fit the constant — the ratchet pointed at record authors, which is precisely + what the v3 note says this whole design abolishes. - v4 states the property directly and scale-free: **the ceiling marks the start of the tail**, - i.e. it sits between the 90th and 95th percentile of record lengths. Percentiles move WITH the - corpus, so routine growth cannot ratchet this; it fires only when the ceiling genuinely stops - marking the tail boundary, which is exactly when it should be re-derived. + v5 SPLITS the claim by robustness instead of hunting for a better single assertion: + + * the COARSE property — the ceiling flags a nonempty proper minority — is asserted HERE, + blocking. One record moves a fraction by at most 1/N, so routine growth cannot cross it. + * the FINE property — `p90 <= ceiling <= p95` — is now REPORTED by `main()` as a notice. + It is real signal about the CONSTANT drifting out of date, which is the passage of corpus + growth rather than a defect in the commit under test. That is the same reasoning + `stale_records` is built on, and it gets the same treatment. + + Note what did NOT change: the ceiling is still 60, and the fine claim is still measured on + every run. v5 moves where each claim is enforced, it does not stop making them. """ recs = [r for r in dl.all_active_records() if r.key] assert len(recs) > 100, f"corpus looks empty ({len(recs)}) — this check would be vacuous" - ceiling = dv.RECORD_CEILING_DEFAULT # the value the CLI actually uses; cannot drift from here - lengths = sorted(dv.record_prose_lines(r) for r in recs) - p90 = lengths[int(len(lengths) * 0.90)] - p95 = lengths[int(len(lengths) * 0.95)] + ceiling = dv.RECORD_CEILING_DEFAULT # the value the CLI actually uses; cannot drift from here + cal = dv.ceiling_calibration(recs, ceiling) - assert p90 <= ceiling <= p95, ( - f"the ceiling ({ceiling}) no longer marks the tail boundary: p90={p90}, p95={p95}. " - f"Below p90 it cuts into the bulk and every author will learn to ignore it; above p95 it is " - f"parked among the outliers and signals nothing. Re-derive it from the distribution." + assert cal.flags_minority, ( + f"the ceiling ({ceiling}) no longer flags a nonempty proper minority of records: " + f"{cal.n_over}/{cal.n} = {cal.fraction_over:.1%} are over it. At 0% it names nobody and " + f"signals nothing; above {dv.CEILING_MINORITY_MAX:.0%} it is cutting into the bulk of the " + f"corpus rather than marking its tail. Re-derive it from the distribution." ) +def test_ceiling_calibration_detects_drift_in_BOTH_directions(): + """The fine claim is asserted here, on a distribution the test OWNS. + + This is the point of the v5 split: the property is still pinned, but against synthetic data + instead of the live corpus, so it cannot be reddened by someone else's record landing. + """ + # 100 records: 95 of 20 lines, 5 of 200. Index 90 lands in the short block and index 95 in the + # long one, so p90 == 20 and p95 == 200 — a wide, unambiguous tail boundary to aim at. + recs = [_rec_body(f"a.s{i}", 20) for i in range(95)] + [_rec_body(f"a.l{i}", 200) for i in range(5)] + assert [dv.ceiling_calibration(recs, 60).p90, dv.ceiling_calibration(recs, 60).p95] == [20, 200] + + assert dv.ceiling_calibration(recs, 60).marks_tail, "60 sits between p90=20 and p95=200" + assert not dv.ceiling_calibration(recs, 10).marks_tail, "below p90 it cuts into the bulk" + assert not dv.ceiling_calibration(recs, 999).marks_tail, "above p95 it is parked among outliers" + + # and the coarse property separates the same two failure modes + assert not dv.ceiling_calibration(recs, 999).flags_minority, "a ceiling nobody is over signals nothing" + assert not dv.ceiling_calibration(recs, 10).flags_minority, "90% over the ceiling is not a tail" + assert dv.ceiling_calibration(recs, 60).flags_minority + + +def test_ceiling_calibration_is_empty_safe(): + """A vacuous corpus must report both claims FALSE, never a passing default.""" + cal = dv.ceiling_calibration([], 60) + assert cal.n == 0 and not cal.marks_tail and not cal.flags_minority + + +def test_adding_ordinary_records_cannot_RED_the_blocking_property(): + """#688's Done-when, demonstrated rather than assumed. + + Takes the REAL corpus and appends the two record shapes that actually reddened v4 in the wild: + #672's 62-line record and #687's 107-line one. Under v4 the 107-line addition moved p90 into + the sparse gap above the ceiling and failed the blocking job. Under v5 the coarse property + still holds, and it holds with room to spare. + """ + real = [r for r in dl.all_active_records() if r.key] + ceiling = dv.RECORD_CEILING_DEFAULT + + grown = real + [_rec_body("new.typical", 62), _rec_body("new.substantial", 107)] + cal = dv.ceiling_calibration(grown, ceiling) + assert cal.flags_minority, f"two ordinary additions reddened the blocking property: {cal}" + + # Headroom, stated as a number so a future reader can see how much slack there is: how many + # consecutive over-ceiling records it would take to breach the bound. + n, over = cal.n, cal.n_over + need = 0 + while (over + need) / (n + need) < dv.CEILING_MINORITY_MAX: + need += 1 + assert need > 20, f"only {need} over-ceiling additions would breach the bound — too tight" + + +# --- #674: the validator cross-checks its own parse against PyYAML ------------------------------ + + +_HAZARDS = { + # PyYAML REJECTS: the bare apostrophe closes the single-quoted scalar early. + "apostrophe": "rule: 'SQLite's LOWER() folds ASCII only'", + # PyYAML ACCEPTS but reads a DIFFERENT value: ` #` starts a comment, truncating the rule. + "unquoted-hash": "rule: use --flag #2 for this", +} + + +def _wing_with(tmp_path: Path, frontmatter_line: str) -> tuple[Path, Path]: + """A record wing containing one file whose frontmatter carries `frontmatter_line`.""" + records = tmp_path / "records" / "ci" + records.mkdir(parents=True) + (records / "a.md").write_text( + "---\n" + "key: ci.a\n" + "title: 'T'\n" + "status: active\n" + "since: '2026-01-01'\n" + "supersedes: none\n" + "superseded-by: none\n" + f"{frontmatter_line}\n" + "signals: 's'\n" + "---\n\nprose.\n" + ) + archive = tmp_path / "archive" + archive.mkdir(parents=True) + return records, archive + + +@pytest.mark.parametrize("hazard", sorted(_HAZARDS)) +def test_pyyaml_crosscheck_catches_frontmatter_the_hand_reader_accepts(tmp_path, hazard): + """#674, both shapes. Hit twice in one session by two independent agents (#578, #651).""" + pytest.importorskip("yaml") + records, _ = _wing_with(tmp_path, _HAZARDS[hazard]) + + faults, ran = dv.pyyaml_frontmatter_faults(sorted(records.rglob("*.md"))) + assert ran, "PyYAML is installed here, so the cross-check must have run" + assert faults, f"the {hazard} hazard slipped through the cross-check" + + +@pytest.mark.parametrize("hazard", sorted(_HAZARDS)) +def test_the_hand_reader_really_IS_blind_to_these(tmp_path, hazard): + """The positive control: pin the MECHANISM, so this suite cannot pass for the wrong reason. + + If `record_wing_faults` ever started catching these on its own, the cross-check above could be + deleted and the tests would stay green while the guard vanished. Asserting that the pre-#674 + machinery reports these files as CLEAN is what makes the cross-check's red meaningful — and it + is the exact state #674 was filed about: `decisions_validate.py` printed OK on input the + writer's own library rejects. + """ + records, archive = _wing_with(tmp_path, _HAZARDS[hazard]) + assert dv.record_wing_faults(records, archive) == [], ( + "the structural guard now catches this by itself — re-derive whether the PyYAML " + "cross-check is still the thing closing this gap" + ) + + +def test_pyyaml_crosscheck_is_clean_on_the_REAL_corpus(): + """No false positives. A cross-check that flags correct records would be reverted within a day.""" + pytest.importorskip("yaml") + files = dv.record_wing_files() + assert len(files) > 100, f"only {len(files)} wing files found — this check would be near-vacuous" + + faults, ran = dv.pyyaml_frontmatter_faults(files) + assert ran + assert faults == [], "the cross-check disagrees with the live corpus:\n" + "\n".join(faults[:5]) + + +def test_crosscheck_skips_cleanly_when_pyyaml_is_absent(tmp_path, monkeypatch): + """The read path stays dependency-free (#674's second Done-when box). + + `decisions-guard`, the Husky hooks and every contributor machine install nothing, so an absent + PyYAML must SKIP the cross-check rather than fault or crash — while every other check runs. + """ + records, _ = _wing_with(tmp_path, _HAZARDS["apostrophe"]) + + import builtins + + real_import = builtins.__import__ + + def no_yaml(name, *a, **kw): + if name == "yaml": + raise ImportError("no yaml here") + return real_import(name, *a, **kw) + + monkeypatch.setattr(builtins, "__import__", no_yaml) + + faults, ran = dv.pyyaml_frontmatter_faults(sorted(records.rglob("*.md"))) + assert ran is False, "an absent PyYAML must report that the check did not run" + assert faults == [], "a skipped check must not manufacture faults" + + +def test_main_ANNOUNCES_a_skipped_crosscheck(capsys, monkeypatch): + """A skipped check that says nothing is the '#603 stale-after' defect: reports success, does + nothing. The skip is legitimate; staying quiet about it is not.""" + monkeypatch.setattr(dv, "pyyaml_frontmatter_faults", lambda files: ([], False)) + assert dv.main([]) == 0 + err = capsys.readouterr().err + assert "cross-check" in err and "SKIPPED" in err, err + + +def test_main_FAILS_when_the_crosscheck_reports_a_fault(capsys, monkeypatch): + """Wiring test: the faults must reach the exit code, not just be computed. + + Without this, `wing_faults=record_wing_faults() + yaml_faults` could drop the second term and + every other test here would still pass. + """ + monkeypatch.setattr(dv, "pyyaml_frontmatter_faults", lambda files: (["x.md: bogus fault"], True)) + assert dv.main([]) == 1 + assert "bogus fault" in capsys.readouterr().err + + def test_retired_budget_flag_says_it_is_ignored(capsys): """A retired flag must announce itself, not no-op silently. @@ -1051,6 +1227,22 @@ def test_no_budget_flag_means_no_retirement_warning(capsys): +def test_main_reports_ceiling_drift_as_a_NOTICE_and_still_exits_0(capsys): + """The fine claim's live wiring (#688). A ceiling of 999 is parked far above p95, so the drift + notice must fire — and must NOT turn the run red, which is the entire point of the v5 split.""" + assert dv.main(["--record-ceiling", "999"]) == 0 + err = capsys.readouterr().err + drift = [ln for ln in err.splitlines() if "drifted from the tail boundary" in ln] + assert len(drift) == 1, err + assert drift[0].startswith("::notice::"), f"drift must be a notice, not a warning: {drift[0]}" + + +def test_main_is_QUIET_about_drift_when_the_ceiling_is_calibrated(capsys): + """The complement — otherwise the notice above could be unconditional and the test vacuous.""" + dv.main([]) + assert "drifted from the tail boundary" not in capsys.readouterr().err + + def test_main_actually_REPORTS_the_ceiling_and_the_trend(capsys): """The new signal's live wiring was untested: `if oversized:` -> `if False:`, or bumping the default ceiling to 999999, left every test green while main() reported nothing. Only the pure From 80818aa2945982aab45c6366acd8c12fd5f8b13b Mon Sep 17 00:00:00 2001 From: Timothy Date: Tue, 4 Aug 2026 20:37:27 +0200 Subject: [PATCH 2/9] =?UTF-8?q?fix(674,688):=20address=20independent=20rev?= =?UTF-8?q?iew=20=E2=80=94=20restore=20the=20coarse=20bound's=20teeth?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two independent cold reviews (one cross-family) agreed on the top two findings. 1. The #688 fix was defeated by its own complement test. test_main_is_QUIET_about_ drift asserted the drift notice was ABSENT while running main() over the LIVE corpus — whose failure condition is bit-for-bit v4's assertion, in the same blocking job, three functions down. p90 sat exactly on 60, so one over-ceiling record would have reddened it. Replaced with an IFF test that uses ceiling_calibration as its oracle, so it asserts the WIRING rather than the corpus's current state, plus a guard that at least one branch fires. 2. The coarse bound was nearly unfalsifiable. `0 < fraction_over < 1/3` accepted EVERY ceiling from 39 to 229 on the real corpus — including the ceiling of 200 my own docstring offered as the case it catches, because one 230-line record keeps the count nonzero. That claim was simply false and is corrected. The floor is now a FRACTION (2%) and the cap 25%, which rejects 200/229/230 and 20, and accepts roughly 45..150. Headroom measured, not estimated: 37 consecutive over-ceiling additions, against ONE record to break v4. 3. yaml.safe_load raises a bare ValueError, not a YAMLError, on a well-shaped but impossible date (stale-after: 2026-06-31), which escaped as a traceback and killed the validator on any machine with PyYAML. The except is now deliberately broad, with a test. 4. PyYAML returns TYPED mapping keys, so a stray `1: x` made sorted(set|set) raise TypeError. Sorted with key=str. 5. The headroom prose was arithmetically wrong (~42/~40 where the real values are 63/64; each addition moves numerator AND denominator) and the record counts were stale. Corrected against measurement. 6. test_adding_ordinary_records passed identically with its two additions removed. It now asserts the additions were counted, and that they break the v4 property while leaving v5 satisfied — guarded by `if`, never asserted, since whether v4 currently holds is a fact about the live distribution and asserting it would rebuild the ratchet. Also recorded honestly in docs.frontmatter-pyyaml-crosscheck: decisions-guard installs no PyYAML, so in CI the cross-check always skips and script-tests already caught both hazards — the CI delta is close to zero and the real fix is the local loop plus the tool/suite agreement. And the new record was trimmed 64 -> 58 prose lines: at 64 it moved p90 to 64 by itself, i.e. this PR would have reddened the old blocking job. That is now cited in the test as the live demonstration. Verification: 426 scripts/tests pass; ruff at baseline parity (47); validator exits 0; corpus back to p90=60, 18/183 over the ceiling, marks_tail and flags_minority both true. Decisions-Edit: yes Co-Authored-By: Claude Opus 5 (1M context) --- docs/ci-cd.md | 8 +- docs/decisions/README.md | 2 +- .../records/docs/corpus-size-signal.md | 15 ++- .../docs/frontmatter-pyyaml-crosscheck.md | 48 +++++---- scripts/decisions_validate.py | 54 +++++++--- scripts/tests/test_decisions_validate.py | 101 ++++++++++++++++-- 6 files changed, 172 insertions(+), 56 deletions(-) diff --git a/docs/ci-cd.md b/docs/ci-cd.md index 53c72644a..52d8cc765 100644 --- a/docs/ci-cd.md +++ b/docs/ci-cd.md @@ -63,9 +63,11 @@ step is now continuous. The release boundary is instead where you: it points at a file. The 60 is derived from the distribution, not picked as a round number. Its **calibration is guarded in two pieces of different robustness** (ersatztv#688), because four earlier single-assertion versions all ratcheted: - - **blocking** (`script-tests`) — only the coarse property that the ceiling flags a nonempty - **proper minority** of records (`0 < fraction_over < 1/3`). One record moves a fraction by at - most 1/N, so routine growth cannot cross it. + - **blocking** (`script-tests`) — only the coarse property that the ceiling flags a + **meaningful minority** of records (`0.02 <= fraction_over <= 0.25`). One record moves a + fraction by at most 1/N, so routine growth cannot cross it: from today's 18/183 it takes 37 + consecutive over-ceiling additions. The floor is a fraction rather than "at least one + record", which would accept any ceiling up to 229 on the live corpus. - **reported, never asserted** — the fine claim that the ceiling sits between the **90th and 95th percentile**, i.e. at the tail boundary. `main()` prints a `::notice::` when it drifts. It is an order statistic over a sparse distribution, so a single new record could move p90 by diff --git a/docs/decisions/README.md b/docs/decisions/README.md index a690c4e60..4b1a9cd97 100644 --- a/docs/decisions/README.md +++ b/docs/decisions/README.md @@ -72,7 +72,7 @@ the link for rationale. Superseded/retired history lives in `archive/`. Regenera | `concurrency.replace-all-contract` | Replace-all aggregate PUTs carry a uniform plain `int Version` concurrency token (EF `.IsConcurrencyToken()`), checked pre-save and enforced by the EF UPDATE guard, returning 412 (not 409) on a stale `If-Match`. | 2026-07-11 | [link](records/concurrency/replace-all-contract.md) | | `concurrency.schedule-item-child-identity` | `PUT /api/schedules/{id}/items` reconciles by an optional round-tripped child `Id` (null/absent/0 ⇒ new item), never by array position, so fill-group/shuffle state follows the logical item across reorders; an unknown or duplicate id is rejected 422 (checked after the §7a `CheckVersion`, so 412 precedes 422). | 2026-07-11 | [link](records/concurrency/schedule-item-child-identity.md) | | `docs.convention-docs-session-start` | Docs-first, not source-first: conventions (api-conventions, spa-conventions, e2e-local, blazor-route-parity, domain-model, decisions, README) are read from docs, not reverse-engineered from code, via `docs/README.md`'s task-signal map — only the sections it points to for the task at hand, not the whole set. Each doc is updated in the same PR that changes what it documents, replacing deferred/follow-up doc updates. | 2026-07-07 | [link](records/docs/convention-docs-session-start.md) | -| `docs.corpus-size-signal` | The corpus's size signal is a per-record prose ceiling (`decisions_validate.py --record-ceiling`, default 60, chosen at a natural gap in the distribution), reported as a NON-BLOCKING `::warning::` naming each record over it. The aggregate prose total is still printed every run but carries NO threshold — it is a `::notice::` trend only — because a total over a monotonically growing corpus can only ratchet, and the generated catalog (`docs/decisions/README.md`) is no longer counted at all since it gains one row per record and cannot be consolidated away. Being listed by the ceiling is an invitation to check for REDUNDANCY, never an instruction to cut: a long record that is all distinct findings is a legitimate decline, and should be recorded as one. The ceiling's CALIBRATION is guarded in two pieces of different robustness (#688): the blocking test asserts only the coarse, non-ratcheting property that the ceiling flags a nonempty PROPER MINORITY of records (`0 < fraction_over < 1/3`), while the fine claim — that it sits between p90 and p95 — is REPORTED by `main()` as a `::notice::` and never asserted against the live corpus. A ceiling drifting out of date is the passage of corpus growth, not a defect in the commit under test, so it gets `stale_records`' treatment rather than a red in the blocking `script-tests` job. | 2026-07-26 | [link](records/docs/corpus-size-signal.md) | +| `docs.corpus-size-signal` | The corpus's size signal is a per-record prose ceiling (`decisions_validate.py --record-ceiling`, default 60, chosen at a natural gap in the distribution), reported as a NON-BLOCKING `::warning::` naming each record over it. The aggregate prose total is still printed every run but carries NO threshold — it is a `::notice::` trend only — because a total over a monotonically growing corpus can only ratchet, and the generated catalog (`docs/decisions/README.md`) is no longer counted at all since it gains one row per record and cannot be consolidated away. Being listed by the ceiling is an invitation to check for REDUNDANCY, never an instruction to cut: a long record that is all distinct findings is a legitimate decline, and should be recorded as one. The ceiling's CALIBRATION is guarded in two pieces of different robustness (#688): the blocking test asserts only the coarse, non-ratcheting property that the ceiling flags a MEANINGFUL MINORITY of records (`0.02 <= fraction_over <= 0.25`), while the fine claim — that it sits between p90 and p95 — is REPORTED by `main()` as a `::notice::` and never asserted against the live corpus. A ceiling drifting out of date is the passage of corpus growth, not a defect in the commit under test, so it gets `stale_records`' treatment rather than a red in the blocking `script-tests` job. | 2026-07-26 | [link](records/docs/corpus-size-signal.md) | | `docs.decision-lifecycle` | every decision `##` record (active or archived) carries a 5-field metadata block (`key`, `status`, `since`, `supersedes`, `superseded-by`) checked by `scripts/decisions_validate.py`; a record is never deleted or line-edited to reverse a call — it is moved to `docs/decisions/archive/` with `status: superseded`/`retired` and a reciprocal `superseded-by`/`supersedes` key pair to its replacement. | 2026-07-21 | [link](records/docs/decision-lifecycle.md) | | `docs.decision-one-file-per-record` | Each decision record is its own file at `docs/decisions/records//.md` (archived ones at `docs/decisions/archive//.md`) with YAML frontmatter; the filename IS the key, so one-active-record-per-key is a filesystem property rather than a validator check, and supersession is a `git mv`. | 2026-07-25 | [link](records/docs/decision-one-file-per-record.md) | | `docs.decision-optional-provenance` | Decision records gain two OPTIONAL fields — `stale-after: YYYY-MM-DD` on the metadata line and a `**Sources:**` line in the metadata block; the Open Knowledge Format (OKF) itself is NOT adopted as the record format. | 2026-07-25 | [link](records/docs/decision-optional-provenance.md) | diff --git a/docs/decisions/records/docs/corpus-size-signal.md b/docs/decisions/records/docs/corpus-size-signal.md index be3c71868..12183bbee 100644 --- a/docs/decisions/records/docs/corpus-size-signal.md +++ b/docs/decisions/records/docs/corpus-size-signal.md @@ -5,7 +5,7 @@ status: active since: '2026-07-26' supersedes: none superseded-by: none -rule: 'The corpus''s size signal is a per-record prose ceiling (`decisions_validate.py --record-ceiling`, default 60, chosen at a natural gap in the distribution), reported as a NON-BLOCKING `::warning::` naming each record over it. The aggregate prose total is still printed every run but carries NO threshold — it is a `::notice::` trend only — because a total over a monotonically growing corpus can only ratchet, and the generated catalog (`docs/decisions/README.md`) is no longer counted at all since it gains one row per record and cannot be consolidated away. Being listed by the ceiling is an invitation to check for REDUNDANCY, never an instruction to cut: a long record that is all distinct findings is a legitimate decline, and should be recorded as one. The ceiling''s CALIBRATION is guarded in two pieces of different robustness (#688): the blocking test asserts only the coarse, non-ratcheting property that the ceiling flags a nonempty PROPER MINORITY of records (`0 < fraction_over < 1/3`), while the fine claim — that it sits between p90 and p95 — is REPORTED by `main()` as a `::notice::` and never asserted against the live corpus. A ceiling drifting out of date is the passage of corpus growth, not a defect in the commit under test, so it gets `stale_records`'' treatment rather than a red in the blocking `script-tests` job.' +rule: 'The corpus''s size signal is a per-record prose ceiling (`decisions_validate.py --record-ceiling`, default 60, chosen at a natural gap in the distribution), reported as a NON-BLOCKING `::warning::` naming each record over it. The aggregate prose total is still printed every run but carries NO threshold — it is a `::notice::` trend only — because a total over a monotonically growing corpus can only ratchet, and the generated catalog (`docs/decisions/README.md`) is no longer counted at all since it gains one row per record and cannot be consolidated away. Being listed by the ceiling is an invitation to check for REDUNDANCY, never an instruction to cut: a long record that is all distinct findings is a legitimate decline, and should be recorded as one. The ceiling''s CALIBRATION is guarded in two pieces of different robustness (#688): the blocking test asserts only the coarse, non-ratcheting property that the ceiling flags a MEANINGFUL MINORITY of records (`0.02 <= fraction_over <= 0.25`), while the fine claim — that it sits between p90 and p95 — is REPORTED by `main()` as a `::notice::` and never asserted against the live corpus. A ceiling drifting out of date is the passage of corpus growth, not a defect in the commit under test, so it gets `stale_records`'' treatment rather than a red in the blocking `script-tests` job.' signals: 'aggregate active-corpus budget, schedule a consolidation warning, permanently red ratchet, per-record ceiling, 60-line prose ceiling, generated catalog counted in budget, corpus consolidation has no owner, size is not redundancy, ceiling calibration reddens script-tests, adding a record fails CI on length, p90 sits on the ceiling, tail-boundary drift notice · paths: `scripts/decisions_validate.py`, `scripts/tests/test_decisions_validate.py`, `docs/ci-cd.md` · issues: #688, #620, #610, #603, #542, #520' mechanics: '`scripts/decisions_validate.py` -> `oversized_records` / `_budget_total`; `docs/ci-cd.md` -> "`decisions-guard` job"' --- @@ -65,9 +65,16 @@ The fine claim is genuinely useful and genuinely fragile, so it is now measured printed as a `::notice::` — the same treatment `stale_records` gets, and for the same stated reason: a constant going out of date is the passage of time, not a defect in this change. What stays blocking is only what routine growth cannot break — each record moves a fraction by at most 1/N, so -from 18/182 over the ceiling it would take ~40 consecutive over-ceiling additions to reach a third, -against ~1 record to break v4. The coarse bound still catches real mis-calibration in both -directions: a ceiling of 200 flags nobody, a ceiling of 20 flags most of the corpus. +from **18/183** over the ceiling it takes **37** consecutive over-ceiling additions to reach the 25% +cap, against **one** record to break v4. + +**The floor is a fraction, not `> 0`, and review is why.** The first draft of v5 asserted only +`0 < fraction_over < 1/3`, which measured against the real corpus accepted **every ceiling from 39 +to 229** — including the ceiling of 200 the draft itself offered as the case it catches, because a +single 230-line record keeps the count nonzero. A bound that a deliberately absurd value satisfies +is not a guard. At a 2% floor and a 25% cap the accepted range is roughly 45..150: a ceiling of 200 +flags 0.5% of records and is rejected, a ceiling of 20 flags 60% and is rejected, and today's 9.8% +sits with about 5x headroom below the floor and 37 additions below the cap. The honest cost, stated rather than buried: **nothing now forces a re-derivation.** The ceiling can drift while only a notice complains. That is accepted on the same reasoning this record already diff --git a/docs/decisions/records/docs/frontmatter-pyyaml-crosscheck.md b/docs/decisions/records/docs/frontmatter-pyyaml-crosscheck.md index b3d32bbcf..163816b98 100644 --- a/docs/decisions/records/docs/frontmatter-pyyaml-crosscheck.md +++ b/docs/decisions/records/docs/frontmatter-pyyaml-crosscheck.md @@ -28,39 +28,43 @@ machine, none of which install anything. Requiring PyYAML there once made the va | `rule: 'SQLite's LOWER()'` | `SQLite's LOWER()` | **`ParserError`** — the bare apostrophe closes the scalar early | | `rule: use --flag #2` | `use --flag #2` | `use --flag` — ` #` starts a comment, **silently truncating** | -A `try/except yaml.YAMLError` would have caught only the first row. The second produces no -exception at all: a valid record whose `rule` has quietly lost its tail — the `parse-to-WRONG` case +A `try/except` would have caught only the first row. The second produces no exception at all: a +valid record whose `rule` has quietly lost its tail — the `parse-to-WRONG` case `docs.record-wing-parse-guard` explicitly names as the gap its structural check cannot see. So the cross-check compares the parsed **result** key by key, and reports a rejection and a mismatch as two -distinct faults with different remedies. +distinct faults with different remedies. The `except` around the load is deliberately broad, not +`yaml.YAMLError`: PyYAML's timestamp constructor raises a bare `ValueError` on an impossible date +(`stale-after: 2026-06-31`), and an additive check must never be the reason the validator can't run. **That is also what makes it general.** #674 asked for a fix that catches the *next* character class -rather than enumerating hazards one at a time — the mistake the surrounding sessions kept making -elsewhere. Comparing against the writer's own library is that: any construct where the two parsers -disagree surfaces as a diff, with nobody having to predict it. Enumerating apostrophes and hashes -would have left the third character to be discovered the same way the first two were. +rather than enumerating hazards one at a time. Comparing against the writer's own library is that: +any construct where the two parsers disagree surfaces as a diff, with nobody having to predict it. **Direction is the load-bearing part.** PyYAML is not a second opinion of equal standing — it WROTE -these files. When the two disagree the on-disk bytes mean what PyYAML says they mean, so the record -is corrupt and the permissive reader is the one hiding it. Stating the direction is what turns an -ambiguous "parsers differ" report into an actionable "this record is silently wrong". +these files, so when the two disagree the on-disk bytes mean what PyYAML says, the record is corrupt +and the permissive reader is the one hiding it. That is what turns an ambiguous "parsers differ" +report into an actionable "this record is silently wrong". **A skip is announced, not silent.** When PyYAML is absent the check does not run, which is correct -and expected on the dependency-free path — but `main()` prints a `::notice::` saying so. A check -that reports success while doing nothing is the defect this corpus keeps re-learning (#603's -`stale-after` that never fired, #609's marker that exempted everything while printing OK), and -adding a quiet skip while fixing a quiet pass would have reintroduced it one level up. +on the dependency-free path — but `main()` prints a `::notice::` saying so. A check that reports +success while doing nothing is the defect this corpus keeps re-learning (#603's `stale-after` that +never fired, #609's marker that exempted everything while printing OK), and adding a quiet skip +while fixing a quiet pass would have reintroduced it one level up. **One implementation, two callers.** The comparison already existed — in the test suite only, which is exactly why the validator could disagree with `scripts/tests` about the same file. Rather than leaving a second copy, `test_frontmatter_reader_matches_pyyaml_on_every_real_record` now delegates -to `pyyaml_frontmatter_faults`, so the tool and the suite cannot drift apart on the definition. +to `pyyaml_frontmatter_faults`, so the tool and the suite cannot drift on the definition. -**What it still does not catch.** A mis-parse both parsers agree on, and anything in a file PyYAML -accepts as a mapping with the schema's own key names. The guard is strong, not total — the same -qualification `record_wing_faults` carries. +**What this buys, stated precisely, because it is less than it looks.** In CI `decisions-guard` +installs nothing, so the validator there always takes the skip path, by design; and `script-tests` +already went red on both hazards before this change. So **`main` was never reachable by a broken +record, and still isn't — the CI delta is close to zero.** What this fixes is the case #674 actually +described: the LOCAL loop, where the validator is the tool a developer or agent reaches for directly +and it printed OK on a corrupt file — plus the standing disagreement between the tool and its own +suite, now impossible because they share one implementation. -**Tested with a positive control.** The suite pins that `record_wing_faults` alone reports both -hazard files as CLEAN. Without it the cross-check could be deleted and the tests would stay green -while the guard vanished; asserting the pre-#674 machinery's blindness is what makes the new red -mean something. +**Limits, and the positive control.** It does not catch a mis-parse both parsers agree on; the guard +is strong, not total, the same qualification `record_wing_faults` carries. The suite pins that +`record_wing_faults` ALONE still reports both hazard files as clean — without that, the cross-check +could be deleted and the tests would stay green while the guard vanished. diff --git a/scripts/decisions_validate.py b/scripts/decisions_validate.py index 988f582a1..fa367b293 100644 --- a/scripts/decisions_validate.py +++ b/scripts/decisions_validate.py @@ -53,10 +53,17 @@ _STALE_AFTER_RE = re.compile(r"^\d{4}-\d{2}-\d{2}$") # guards the calibration claim asserts against the SAME value the CLI uses and the two cannot drift. RECORD_CEILING_DEFAULT = 60 -# The coarse, non-ratcheting calibration bound (#688): the ceiling must flag a nonempty PROPER -# MINORITY of records. Above a third it is cutting into the bulk rather than marking a tail. See -# `ceiling_calibration` for why the fine percentile claim is reported instead of asserted. -CEILING_MINORITY_MAX = 1 / 3 +# The coarse, non-ratcheting calibration bound (#688): the ceiling must flag a MEANINGFUL MINORITY +# of records. Below the floor it is parked among the outliers and names almost nobody; above the cap +# it is cutting into the bulk rather than marking a tail. See `ceiling_calibration` for why the fine +# percentile claim is reported instead of asserted. +# +# The floor is NOT "at least one record" — that was the first draft and it was nearly unfalsifiable: +# measured on the live corpus it accepted every ceiling from 39 to 229, including the ceiling of 200 +# this module's own docstring offered as the case it catches (one 230-line record keeps the count +# nonzero). A 2% floor rejects 200/229/230 and still leaves ~5x headroom below today's 9.8%. +CEILING_MINORITY_MIN = 0.02 +CEILING_MINORITY_MAX = 0.25 def _parse_stale_after(value: str | None) -> date | None: @@ -604,13 +611,20 @@ def pyyaml_frontmatter_faults(files) -> tuple[list[str], bool]: continue # no/unterminated frontmatter: reported by record_wing_faults try: theirs_raw = yaml.safe_load(block) - except yaml.YAMLError as exc: + except Exception as exc: + # DELIBERATELY broad. `yaml.YAMLError` alone is too narrow: PyYAML's timestamp + # constructor raises a BARE ValueError for a well-shaped but impossible date + # (`stale-after: 2026-06-31` -> "day is out of range for month"), which would escape as + # a traceback. This check is meant to be strictly additive — it must never be the + # reason the validator cannot run, so every failure to load becomes a reported fault. first = str(exc).splitlines()[0] if str(exc).strip() else exc.__class__.__name__ faults.append( f"{p}: PyYAML REJECTS this frontmatter, though the dependency-free reader accepted " - f"it ({first}). PyYAML is what WROTE these files, so its verdict is authoritative. " - f"Common cause: a bare apostrophe inside a single-quoted value (write '' for a " - f"literal apostrophe, as `yaml.safe_dump` does)." + f"it ({exc.__class__.__name__}: {first}). PyYAML is what WROTE these files, so its " + f"verdict is authoritative. Quote the offending value and, inside single quotes, " + f"double any literal apostrophe — as `yaml.safe_dump` does. Usual causes: a bare " + f"apostrophe inside a single-quoted value, an unquoted `: ` or leading backtick, or " + f"an impossible date." ) continue if theirs_raw is None: @@ -622,7 +636,10 @@ def pyyaml_frontmatter_faults(files) -> tuple[list[str], bool]: if mine is None: continue # the reader bailed: reported by record_wing_faults as parse-to-0 theirs = {k: ("" if v is None else str(v)) for k, v in theirs_raw.items()} - for k in sorted(set(mine) | set(theirs)): + # `key=str`: PyYAML returns TYPED mapping keys, so a stray `1: x` yields an int key while the + # hand reader yields "1", and sorting that mixed set raises TypeError — an uncaught traceback + # replacing what `_unknown_frontmatter_keys` used to report as an actionable error. + for k in sorted(set(mine) | set(theirs), key=str): if mine.get(k) != theirs.get(k): faults.append( f"{p}: frontmatter key {k!r} means different things to the two parsers — " @@ -757,12 +774,17 @@ def ceiling_calibration(records, ceiling: int) -> CeilingCalibration: but it is signal about the CONSTANT drifting, not a defect in the commit under test — the same shape as `stale_records`, and it is reported the same way: a notice, never a failure. - `flags_minority` — `0 < fraction_over < 1/3`. Deliberately coarse, and what the blocking test - asserts. Each added record moves a fraction by at most 1/N, so routine growth cannot cross it: - at 17/176 over the ceiling, reaching a third would take ~42 consecutive over-ceiling additions. - It still catches genuine mis-calibration in both directions — a ceiling of 200 flags nobody - (fraction 0, signals nothing), a ceiling of 20 flags most of the corpus (cutting into the bulk, - so every author learns to ignore it). + `flags_minority` — `CEILING_MINORITY_MIN <= fraction_over <= CEILING_MINORITY_MAX`. Deliberately + coarse, and what the blocking test asserts. Each added record moves a fraction by at most 1/N, so + routine growth cannot cross it: measured live at 18/183 (9.8%), reaching the 25% cap takes 37 + consecutive over-ceiling additions, against ONE record to break `marks_tail`. + + It still catches genuine mis-calibration in both directions, measured on the real corpus: a + ceiling of 20 flags 60% of records (cutting into the bulk, so every author learns to ignore it), + and a ceiling of 200 flags 0.5% — one record — which is below the floor and rejected. Note that + "flags NOBODY" is the wrong way to state the upper failure: at a ceiling of 200 the count is + still nonzero because one 230-line record exists, which is exactly why the floor is a fraction + and not `> 0`. Why not simply re-derive the constant instead: re-deriving fixes the instance and keeps the mechanism. The v3 fraction band and the v4 percentile containment both failed the same way, one @@ -786,7 +808,7 @@ def ceiling_calibration(records, ceiling: int) -> CeilingCalibration: n_over=n_over, fraction_over=frac, marks_tail=p90 <= ceiling <= p95, - flags_minority=0 < frac < CEILING_MINORITY_MAX, + flags_minority=CEILING_MINORITY_MIN <= frac <= CEILING_MINORITY_MAX, ) diff --git a/scripts/tests/test_decisions_validate.py b/scripts/tests/test_decisions_validate.py index 68d608441..56146552f 100644 --- a/scripts/tests/test_decisions_validate.py +++ b/scripts/tests/test_decisions_validate.py @@ -1062,10 +1062,30 @@ def test_ceiling_calibration_detects_drift_in_BOTH_directions(): # and the coarse property separates the same two failure modes assert not dv.ceiling_calibration(recs, 999).flags_minority, "a ceiling nobody is over signals nothing" - assert not dv.ceiling_calibration(recs, 10).flags_minority, "90% over the ceiling is not a tail" + assert not dv.ceiling_calibration(recs, 10).flags_minority, "95% over the ceiling is not a tail" assert dv.ceiling_calibration(recs, 60).flags_minority +def test_the_coarse_bound_REJECTS_a_badly_placed_ceiling_on_the_REAL_corpus(): + """The blocking property must have teeth, demonstrated against the live distribution. + + Review's strongest finding on the first draft: a floor of `fraction_over > 0` was nearly + unfalsifiable — it accepted every ceiling from 39 to 229, including the ceiling of 200 that the + docstring itself offered as the case it catches, because one 230-line record keeps the count + nonzero. A FRACTION floor is what restores the teeth, so pin it on the real corpus. + """ + recs = [r for r in dl.all_active_records() if r.key] + assert len(recs) > 100, "corpus looks empty — this check would be vacuous" + + for bad in (200, 229, 230): + cal = dv.ceiling_calibration(recs, bad) + assert not cal.flags_minority, ( + f"a ceiling of {bad} flags only {cal.n_over}/{cal.n} records and must be rejected, got {cal}" + ) + assert not dv.ceiling_calibration(recs, 20).flags_minority, "a ceiling of 20 cuts into the bulk" + assert dv.ceiling_calibration(recs, dv.RECORD_CEILING_DEFAULT).flags_minority + + def test_ceiling_calibration_is_empty_safe(): """A vacuous corpus must report both claims FALSE, never a passing default.""" cal = dv.ceiling_calibration([], 60) @@ -1082,16 +1102,38 @@ def test_adding_ordinary_records_cannot_RED_the_blocking_property(): """ real = [r for r in dl.all_active_records() if r.key] ceiling = dv.RECORD_CEILING_DEFAULT - grown = real + [_rec_body("new.typical", 62), _rec_body("new.substantial", 107)] - cal = dv.ceiling_calibration(grown, ceiling) - assert cal.flags_minority, f"two ordinary additions reddened the blocking property: {cal}" + + before = dv.ceiling_calibration(real, ceiling) + after = dv.ceiling_calibration(grown, ceiling) + + # The additions must actually MOVE something, or this test proves nothing about them — review + # caught the first draft passing identically with the two records removed. + assert after.n == before.n + 2 and after.n_over == before.n_over + 2, ( + f"the appended records were not counted: {before} -> {after}" + ) + + # The v4 property is the one that reddened CI, and these additions break it — that is the whole + # demonstration: the SAME corpus change that would have failed the old blocking assertion leaves + # the v5 one satisfied. Guarded by `if`, never asserted: whether v4 currently holds is a fact + # about the live distribution, and asserting it here would rebuild the very ratchet this test + # exists to document. It is not hypothetical: an earlier draft of the record this branch adds + # was 64 prose lines, which by itself moved p90 from 60 to 64 and broke v4 before either + # synthetic addition was appended — i.e. this PR would have reddened the old blocking job. + if before.marks_tail: + assert not after.marks_tail, ( + f"these additions no longer break the old percentile property ({after}) — pick shapes " + f"that do, or this test has stopped demonstrating anything" + ) + + # ...and the v5 blocking property holds either way. + assert after.flags_minority, f"two ordinary additions reddened the blocking property: {after}" # Headroom, stated as a number so a future reader can see how much slack there is: how many # consecutive over-ceiling records it would take to breach the bound. - n, over = cal.n, cal.n_over + n, over = after.n, after.n_over need = 0 - while (over + need) / (n + need) < dv.CEILING_MINORITY_MAX: + while (over + need) / (n + need) <= dv.CEILING_MINORITY_MAX: need += 1 assert need > 20, f"only {need} over-ceiling additions would breach the bound — too tight" @@ -1156,6 +1198,23 @@ def test_the_hand_reader_really_IS_blind_to_these(tmp_path, hazard): ) +def test_crosscheck_REPORTS_an_impossible_date_instead_of_crashing(tmp_path): + """PyYAML raises a bare `ValueError`, not a `YAMLError`, for a well-shaped impossible date. + + `stale-after: 2026-06-31` (June has 30 days) escaped an `except yaml.YAMLError` as a traceback, + killing the validator on any machine with PyYAML — including the Husky pre-commit hook. A check + documented as "strictly additive, must never be the reason the validator cannot run" must + REPORT this, so the except is deliberately broad. + """ + pytest.importorskip("yaml") + records, _ = _wing_with(tmp_path, "stale-after: 2026-06-31") + + faults, ran = dv.pyyaml_frontmatter_faults(sorted(records.rglob("*.md"))) + assert ran + assert faults and "REJECTS" in faults[0], faults + assert "ValueError" in faults[0], f"the fault must name the real exception: {faults[0]}" + + def test_pyyaml_crosscheck_is_clean_on_the_REAL_corpus(): """No false positives. A cross-check that flags correct records would be reverted within a day.""" pytest.importorskip("yaml") @@ -1237,10 +1296,32 @@ def test_main_reports_ceiling_drift_as_a_NOTICE_and_still_exits_0(capsys): assert drift[0].startswith("::notice::"), f"drift must be a notice, not a warning: {drift[0]}" -def test_main_is_QUIET_about_drift_when_the_ceiling_is_calibrated(capsys): - """The complement — otherwise the notice above could be unconditional and the test vacuous.""" - dv.main([]) - assert "drifted from the tail boundary" not in capsys.readouterr().err +def test_main_reports_drift_IFF_the_ceiling_is_off_the_tail_boundary(capsys): + """The complement of the test above — asserting the WIRING, not the corpus's current state. + + The obvious way to write this is `dv.main([]); assert "drifted" not in err`, and that is a trap + review caught: `main()` emits the notice exactly when `p90 <= 60 <= p95` is false over the LIVE + corpus, so such a test fails under precisely the condition #688 exists to stop failing — it + would move v4's assertion three functions down and leave it in the same blocking job. Today p90 + sits exactly ON the ceiling, so ONE new over-ceiling record would have reddened it. + + So the oracle is `ceiling_calibration` itself: whatever the corpus currently looks like, the + notice must be present iff the fine claim is false. The 999 case pins that at least one branch + is genuinely exercised, so this cannot pass by never firing. + """ + recs = [r for r in dl.all_active_records() if r.key] + seen_drift = False + + for ceiling in (dv.RECORD_CEILING_DEFAULT, 999): + expected = not dv.ceiling_calibration(recs, ceiling).marks_tail + dv.main(["--record-ceiling", str(ceiling)]) + err = capsys.readouterr().err + assert ("drifted from the tail boundary" in err) is expected, ( + f"ceiling {ceiling}: expected drift notice={expected}, got the opposite" + ) + seen_drift |= expected + + assert seen_drift, "neither ceiling produced a drift notice — the test never exercised the notice" def test_main_actually_REPORTS_the_ceiling_and_the_trend(capsys): From b3a88262818fe8a3ae33b5655dd4dc6bbc555e6e Mon Sep 17 00:00:00 2001 From: Timothy Date: Tue, 4 Aug 2026 21:02:01 +0200 Subject: [PATCH 3/9] fix(688): stop asserting live-corpus order statistics anywhere in the suite MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review round 2 (both reviewers, independently) found the round-1 fix incomplete: the live-corpus coupling survived in two more tests. This is the THIRD instance of one defect class in this change, so the fix is to remove the coupling rather than patch another site. BLOCKER — test_adding_ordinary_records still asserted live order statistics. The `if before.marks_tail:` guard made the PRECONDITION conditional but left the CONCLUSION (`assert not after.marks_tail`) an assertion about the live p90. Verified: appending 16 ordinary 30-line records — nothing long, nothing unusual — makes both sides true and fires it, reddening the blocking job for an unrelated author. Exactly what #688 exists to abolish. The v4-vs-v5 contrast moved to test_v4_would_have_reddened_where_v5_holds, built on a distribution the test OWNS, reproducing the shape that matters (a sparse gap just above the ceiling). The real-corpus test now asserts only the robust claims: the additions were counted, v5 holds, and the measured headroom. Same treatment for the "bad ceiling" teeth test, which hard-coded that 200/229/230 stay rejected on the live corpus — three new 200+ line records flip it. Teeth now demonstrated synthetically; the only live-corpus assertion left is that today's ceiling is accepted, which needs 38 over-ceiling or 718 short additions to break. The IFF drift test could lose its quiet branch: one 61-line record makes BOTH the 60 and 999 ceilings drift, at which point an UNCONDITIONAL notice would pass. Both ceilings are now DERIVED — p90 itself (always calibrated, since p90 <= p90 <= p95) and max+1 (always off the tail) — so each branch is guaranteed by construction, and the test asserts it exercised both. Added the missing regression test for the typed-mapping-key TypeError: removing `key=str` now fails a test instead of only a manual probe. Corrected against measurement: the v5 row of the record's own version table still stated the REJECTED first-draft bound (`0 < f < 1/3`) — the canonical artefact contradicting both the code and its own next paragraph; accepted range is 43..180, not "roughly 45..150"; breaching the cap takes 38 additions, not 37 (37 lands exactly on 0.25, which passes under `<=`); "5x headroom below the floor" was inverted; ci-cd.md said four versions "all ratcheted" when v1 was vacuous and v2 accepted an absurd ceiling; and the crosscheck record overstated protection — script-tests is NOT a required check, so "no broken record has reached main" is procedural, not structural. Evidence the coupling is actually gone: mid-fix the corpus sat at marks_tail=False (an edit pushed this branch's own record to 61 lines, moving p90) and the suite stayed fully green. Under the old assertions that state reddened CI. The record is trimmed back to 60 so main ships calibrated and no drift notice nags. Verification: 428 scripts/tests pass; ruff at baseline parity (47); validator exits 0 with no drift notice; corpus at p90=60, 18/183 over the ceiling. Decisions-Edit: yes Co-Authored-By: Claude Opus 5 (1M context) --- docs/ci-cd.md | 10 +- .../records/docs/corpus-size-signal.md | 8 +- .../docs/frontmatter-pyyaml-crosscheck.md | 18 +-- scripts/decisions_validate.py | 7 +- scripts/tests/test_decisions_validate.py | 105 ++++++++++++++---- 5 files changed, 105 insertions(+), 43 deletions(-) diff --git a/docs/ci-cd.md b/docs/ci-cd.md index 52d8cc765..e574e6776 100644 --- a/docs/ci-cd.md +++ b/docs/ci-cd.md @@ -62,12 +62,14 @@ step is now continuous. The release boundary is instead where you: — a **non-blocking `::warning::`** naming every record over it. This is the actionable signal: it points at a file. The 60 is derived from the distribution, not picked as a round number. Its **calibration is guarded in two pieces of different robustness** (ersatztv#688), because - four earlier single-assertion versions all ratcheted: + four earlier single-assertion versions all failed — the first two by being vacuous or + accepting an absurd ceiling, the last two by ratcheting: - **blocking** (`script-tests`) — only the coarse property that the ceiling flags a **meaningful minority** of records (`0.02 <= fraction_over <= 0.25`). One record moves a - fraction by at most 1/N, so routine growth cannot cross it: from today's 18/183 it takes 37 - consecutive over-ceiling additions. The floor is a fraction rather than "at least one - record", which would accept any ceiling up to 229 on the live corpus. + fraction by at most 1/N, so routine growth cannot cross it: from today's 18/183 it takes 38 + consecutive over-ceiling additions, or 718 short ones to dilute below the floor. The floor is + a fraction rather than "at least one record", which would accept any ceiling up to 229 on the + live corpus; as a fraction the accepted range is 43..180. - **reported, never asserted** — the fine claim that the ceiling sits between the **90th and 95th percentile**, i.e. at the tail boundary. `main()` prints a `::notice::` when it drifts. It is an order statistic over a sparse distribution, so a single new record could move p90 by diff --git a/docs/decisions/records/docs/corpus-size-signal.md b/docs/decisions/records/docs/corpus-size-signal.md index 12183bbee..09b7f5157 100644 --- a/docs/decisions/records/docs/corpus-size-signal.md +++ b/docs/decisions/records/docs/corpus-size-signal.md @@ -57,7 +57,7 @@ Getting there took five versions, and the failures are the useful part: | v2 | a minimum gap WIDTH | a ceiling of 200 also sits in a wide gap — it passed | | v3 | 2-12% fraction band + "clear air" above | **hostage to an unrelated record**: one ordinary 62-line addition reddened it with the ceiling correctly placed, and the only remedy was to RAISE the ceiling — this very treadmill, as a hard failure in what #631 makes a blocking job. The fraction band had the same coupling more slowly (12 more long records breached it), and `0 <= headroom` was vacuous. | | v4 | `p90 <= ceiling <= p95` | percentiles move with the corpus, but an order statistic over a SPARSE distribution is a STEP function. The lengths above the ceiling ran 60, 61, 62, 63 then jumped to 81, so ONE record moved p90 by twenty lines. It reddened the blocking job twice live (#672, #706), and both times the only in-scope remedy was to trim the new record to fit the constant — the v3 ratchet, pointed at record authors | -| v5 | coarse `0 < fraction_over < 1/3` asserted; fine `p90 <= ceiling <= p95` REPORTED | splits the claim by robustness instead of hunting for a better single assertion (#688) | +| v5 | coarse `0.02 <= fraction_over <= 0.25` asserted; fine `p90 <= ceiling <= p95` REPORTED | splits the claim by robustness instead of hunting for a better single assertion (#688) | **v5 is not a fifth attempt at the same shape — it stops trying.** Four versions failed because they all asserted, in the blocking job, a property of a corpus the commit under test does not control. @@ -72,9 +72,9 @@ cap, against **one** record to break v4. `0 < fraction_over < 1/3`, which measured against the real corpus accepted **every ceiling from 39 to 229** — including the ceiling of 200 the draft itself offered as the case it catches, because a single 230-line record keeps the count nonzero. A bound that a deliberately absurd value satisfies -is not a guard. At a 2% floor and a 25% cap the accepted range is roughly 45..150: a ceiling of 200 -flags 0.5% of records and is rejected, a ceiling of 20 flags 60% and is rejected, and today's 9.8% -sits with about 5x headroom below the floor and 37 additions below the cap. +is not a guard. At a 2% floor and a 25% cap the accepted range is **43..180** (measured, contiguous): +a ceiling of 200 flags 0.5% of records and is rejected, a ceiling of 20 flags 60% and is rejected, +and today's 9.8% sits about 5x ABOVE the floor and 38 over-ceiling additions below the cap. The honest cost, stated rather than buried: **nothing now forces a re-derivation.** The ceiling can drift while only a notice complains. That is accepted on the same reasoning this record already diff --git a/docs/decisions/records/docs/frontmatter-pyyaml-crosscheck.md b/docs/decisions/records/docs/frontmatter-pyyaml-crosscheck.md index 163816b98..0a08e88b7 100644 --- a/docs/decisions/records/docs/frontmatter-pyyaml-crosscheck.md +++ b/docs/decisions/records/docs/frontmatter-pyyaml-crosscheck.md @@ -58,13 +58,15 @@ to `pyyaml_frontmatter_faults`, so the tool and the suite cannot drift on the de **What this buys, stated precisely, because it is less than it looks.** In CI `decisions-guard` installs nothing, so the validator there always takes the skip path, by design; and `script-tests` -already went red on both hazards before this change. So **`main` was never reachable by a broken -record, and still isn't — the CI delta is close to zero.** What this fixes is the case #674 actually -described: the LOCAL loop, where the validator is the tool a developer or agent reaches for directly -and it printed OK on a corrupt file — plus the standing disagreement between the tool and its own -suite, now impossible because they share one implementation. +already went red on both hazards before this change, and an advisory red still blocks the merge gate +(#598). So **no broken record has reached `main`, and the CI delta here is close to zero** — though +that protection is procedural rather than structural, since `script-tests` is not a REQUIRED check +and `decisions-guard`, which is, runs without PyYAML and so always skips this cross-check. What this +fixes is the case #674 actually +described: the LOCAL loop, where the validator is the tool an agent reaches for directly and it +printed OK on a corrupt file — plus the tool/suite disagreement, now impossible. -**Limits, and the positive control.** It does not catch a mis-parse both parsers agree on; the guard -is strong, not total, the same qualification `record_wing_faults` carries. The suite pins that -`record_wing_faults` ALONE still reports both hazard files as clean — without that, the cross-check +**Limits, and the positive control.** It does not catch a mis-parse both parsers agree on — strong, +not total, the same qualification `record_wing_faults` carries. The suite pins that +`record_wing_faults` ALONE still reports both hazard files as clean; without that, the cross-check could be deleted and the tests would stay green while the guard vanished. diff --git a/scripts/decisions_validate.py b/scripts/decisions_validate.py index fa367b293..c2fd318f0 100644 --- a/scripts/decisions_validate.py +++ b/scripts/decisions_validate.py @@ -758,7 +758,7 @@ class CeilingCalibration(NamedTuple): n_over: int fraction_over: float marks_tail: bool # the FINE claim: p90 <= ceiling <= p95 - flags_minority: bool # the COARSE claim: 0 < fraction_over < CEILING_MINORITY_MAX + flags_minority: bool # the COARSE claim: MINORITY_MIN <= fraction_over <= MINORITY_MAX def ceiling_calibration(records, ceiling: int) -> CeilingCalibration: @@ -776,8 +776,9 @@ def ceiling_calibration(records, ceiling: int) -> CeilingCalibration: `flags_minority` — `CEILING_MINORITY_MIN <= fraction_over <= CEILING_MINORITY_MAX`. Deliberately coarse, and what the blocking test asserts. Each added record moves a fraction by at most 1/N, so - routine growth cannot cross it: measured live at 18/183 (9.8%), reaching the 25% cap takes 37 - consecutive over-ceiling additions, against ONE record to break `marks_tail`. + routine growth cannot cross it: measured live at 18/183 (9.8%), BREACHING the 25% cap takes 38 + consecutive over-ceiling additions (37 lands exactly on 0.25, which still passes under `<=`), + or 718 short ones to dilute below the floor — against ONE record to break `marks_tail`. It still catches genuine mis-calibration in both directions, measured on the real corpus: a ceiling of 20 flags 60% of records (cutting into the bulk, so every author learns to ignore it), diff --git a/scripts/tests/test_decisions_validate.py b/scripts/tests/test_decisions_validate.py index 56146552f..bea4de3c4 100644 --- a/scripts/tests/test_decisions_validate.py +++ b/scripts/tests/test_decisions_validate.py @@ -1074,15 +1074,25 @@ def test_the_coarse_bound_REJECTS_a_badly_placed_ceiling_on_the_REAL_corpus(): docstring itself offered as the case it catches, because one 230-line record keeps the count nonzero. A FRACTION floor is what restores the teeth, so pin it on the real corpus. """ - recs = [r for r in dl.all_active_records() if r.key] - assert len(recs) > 100, "corpus looks empty — this check would be vacuous" + # The TEETH are demonstrated on an owned distribution, for the reason in + # `test_v4_would_have_reddened_where_v5_holds`: an assertion that a specific absurd ceiling is + # rejected by the LIVE corpus is itself growth-coupled (review found that three new 200+ line + # records would flip the 200 arm). 100 records of 30 lines and one of 230 — an outlier-only + # tail, which is precisely the shape a badly-placed ceiling fails to distinguish. + synthetic = [_rec_body(f"a.s{i}", 30) for i in range(100)] + [_rec_body("a.outlier", 230)] for bad in (200, 229, 230): - cal = dv.ceiling_calibration(recs, bad) + cal = dv.ceiling_calibration(synthetic, bad) assert not cal.flags_minority, ( f"a ceiling of {bad} flags only {cal.n_over}/{cal.n} records and must be rejected, got {cal}" ) - assert not dv.ceiling_calibration(recs, 20).flags_minority, "a ceiling of 20 cuts into the bulk" + assert not dv.ceiling_calibration(synthetic, 10).flags_minority, "a ceiling of 10 cuts into the bulk" + + # The only claim made against the LIVE corpus is the robust one: today's ceiling is accepted. + # Breaching it takes 38 consecutive over-ceiling additions (or 718 short ones by dilution), so + # ordinary growth cannot red this. + recs = [r for r in dl.all_active_records() if r.key] + assert len(recs) > 100, "corpus looks empty — this check would be vacuous" assert dv.ceiling_calibration(recs, dv.RECORD_CEILING_DEFAULT).flags_minority @@ -1113,24 +1123,11 @@ def test_adding_ordinary_records_cannot_RED_the_blocking_property(): f"the appended records were not counted: {before} -> {after}" ) - # The v4 property is the one that reddened CI, and these additions break it — that is the whole - # demonstration: the SAME corpus change that would have failed the old blocking assertion leaves - # the v5 one satisfied. Guarded by `if`, never asserted: whether v4 currently holds is a fact - # about the live distribution, and asserting it here would rebuild the very ratchet this test - # exists to document. It is not hypothetical: an earlier draft of the record this branch adds - # was 64 prose lines, which by itself moved p90 from 60 to 64 and broke v4 before either - # synthetic addition was appended — i.e. this PR would have reddened the old blocking job. - if before.marks_tail: - assert not after.marks_tail, ( - f"these additions no longer break the old percentile property ({after}) — pick shapes " - f"that do, or this test has stopped demonstrating anything" - ) - - # ...and the v5 blocking property holds either way. + # ...and the v5 blocking property holds. assert after.flags_minority, f"two ordinary additions reddened the blocking property: {after}" - # Headroom, stated as a number so a future reader can see how much slack there is: how many - # consecutive over-ceiling records it would take to breach the bound. + # Live headroom, as a number, so a future reader can see how much slack there actually is: how + # many consecutive over-ceiling records it would take to BREACH the cap from here. n, over = after.n, after.n_over need = 0 while (over + need) / (n + need) <= dv.CEILING_MINORITY_MAX: @@ -1138,6 +1135,38 @@ def test_adding_ordinary_records_cannot_RED_the_blocking_property(): assert need > 20, f"only {need} over-ceiling additions would breach the bound — too tight" +def test_v4_would_have_reddened_where_v5_holds(): + """The v4-vs-v5 contrast, on a distribution the test OWNS rather than the live corpus. + + THIRD TIME for this defect class in one change, which is why the fix is to remove the coupling + rather than patch the instance. Round 1 of review caught it in the drift test; round 2 caught it + here, in what looked like a safe `if before.marks_tail:` guard — the GUARD was conditional but + the CONCLUSION was still an assertion about live order statistics, and appending 16 ordinary + 30-line records (nothing long, nothing unusual) makes `after.marks_tail` true again and fires it: + + extra= 0 before(marks=True) after(marks=False) -> reds: False + extra=16 before(marks=True) after(marks=True) -> reds: True + + Nothing about this demonstration needs the real corpus. The synthetic base reproduces the shape + that matters — a sparse gap just above the ceiling, exactly what #688 measured on `main` (60, + 61, 62, 63, then a jump to 81) — so two over-ceiling additions advance the p90 index off the + ceiling and break v4, while v5 is untouched. + """ + base = ( + [_rec_body(f"a.s{i}", 30) for i in range(90)] # the bulk + + [_rec_body("a.edge", 60)] # sits exactly ON the ceiling, as main does today + + [_rec_body(f"a.l{i}", 112) for i in range(9)] # the tail, across a sparse gap + ) + before = dv.ceiling_calibration(base, 60) + assert (before.p90, before.p95) == (60, 112), before + assert before.marks_tail and before.flags_minority, before + + after = dv.ceiling_calibration(base + [_rec_body("new.a", 107), _rec_body("new.b", 107)], 60) + + assert not after.marks_tail, f"v4 must break on these additions, or the contrast is empty: {after}" + assert after.flags_minority, f"v5 must survive what broke v4: {after}" + + # --- #674: the validator cross-checks its own parse against PyYAML ------------------------------ @@ -1215,6 +1244,22 @@ def test_crosscheck_REPORTS_an_impossible_date_instead_of_crashing(tmp_path): assert "ValueError" in faults[0], f"the fault must name the real exception: {faults[0]}" +def test_crosscheck_survives_a_TYPED_mapping_key(tmp_path): + """PyYAML returns typed keys, so a stray `1: x` yields int 1 where the reader yields "1". + + Sorting that mixed set raised `TypeError` — an uncaught traceback replacing what + `_unknown_frontmatter_keys` used to report as an actionable error. Removing the `key=str` sort + key restores the crash, and without this test every other test here stays green. + """ + pytest.importorskip("yaml") + records, _ = _wing_with(tmp_path, "1: stray") + + faults, ran = dv.pyyaml_frontmatter_faults(sorted(records.rglob("*.md"))) + assert ran + assert faults, "a typed mapping key must be reported, not swallowed" + assert any("1" in f for f in faults), faults + + def test_pyyaml_crosscheck_is_clean_on_the_REAL_corpus(): """No false positives. A cross-check that flags correct records would be reverted within a day.""" pytest.importorskip("yaml") @@ -1310,18 +1355,30 @@ def test_main_reports_drift_IFF_the_ceiling_is_off_the_tail_boundary(capsys): is genuinely exercised, so this cannot pass by never firing. """ recs = [r for r in dl.all_active_records() if r.key] - seen_drift = False + lengths = sorted(dv.record_prose_lines(r) for r in recs) - for ceiling in (dv.RECORD_CEILING_DEFAULT, 999): + # Both ceilings are DERIVED so each branch is guaranteed by construction, not by luck. Review + # caught the earlier version relying on the live 60/999 pair: once one 61-line record lands, + # BOTH of those drift, and an UNCONDITIONAL notice would have passed the test. + # * p90 itself is always calibrated — `p90 <= p90 <= p95` holds for any distribution. + # * one line above the longest record is always off the tail — it exceeds p95 by definition. + quiet_ceiling = lengths[min(int(len(lengths) * 0.90), len(lengths) - 1)] + drift_ceiling = lengths[-1] + 1 + + expectations = [] + for ceiling in (quiet_ceiling, drift_ceiling): expected = not dv.ceiling_calibration(recs, ceiling).marks_tail dv.main(["--record-ceiling", str(ceiling)]) err = capsys.readouterr().err assert ("drifted from the tail boundary" in err) is expected, ( f"ceiling {ceiling}: expected drift notice={expected}, got the opposite" ) - seen_drift |= expected + expectations.append(expected) - assert seen_drift, "neither ceiling produced a drift notice — the test never exercised the notice" + assert expectations == [False, True], ( + f"the two derived ceilings must exercise BOTH branches, got {expectations} — otherwise an " + f"unconditional notice (or none at all) would pass this test" + ) def test_main_actually_REPORTS_the_ceiling_and_the_trend(capsys): From 0db56c3ebda0a7f682a672d75614177581545a0d Mon Sep 17 00:00:00 2001 From: Timothy Date: Tue, 4 Aug 2026 21:26:52 +0200 Subject: [PATCH 4/9] fix(688): remove the last live-corpus tripwires and three fabricated/wrong numbers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review round 3, both reviewers, NOT-MERGEABLE. Nothing needed rework — the code path was found sound and mutation-sensitive (a 17-mutation battery caught every mutation with the semantically correct test). What was left were tripwires and prose. TRIPWIRES - `assert need > 20` was the tightest live-corpus assertion left in the blocking job: it reds after 16 over-ceiling additions, while `flags_minority` — the property #688 exists to protect — survives to 36. An arbitrary threshold on a live order statistic is the ratchet wearing a different hat. Removed; the measured headroom lives in prose, where being out of date costs a doc fix rather than someone else's red build. This also removes an unbounded `while` loop that HUNG the suite rather than failing it when the ratio could not reach the cap. - test_main_reports_ceiling_drift hardcoded ceiling 999, which is not guaranteed above p95: ten valid 1000-line records make 999 calibrated and silently delete the test's only assertion. Now derived as max+1, off the tail by definition. - test_main_actually_REPORTS_the_ceiling_and_the_trend required >=1 over-ceiling record. The ceiling is ALLOWED to go green (test_oversized_records_can_go_green says so), so that would red the blocking job the day someone consolidates the last offender — punishing exactly the work the warning asks for. Restated as an IFF. - test_no_budget_flag_means_no_retirement_warning asserted no bare "RETIRED" in stderr; a legitimate stale record whose TITLE contains the word reds it. Matched precisely now. - Added the >100-record vacuity guard its siblings carry to the derived-ceiling test. NUMBERS — all three were mine, and two are the failure mode this repo calls worse than no note at all (a confident claim that was never measured): - "the lengths above the ceiling ran 60, 61, 62, 63 then jumped to 81" is FABRICATED. No record of 61, 62 or 63 lines exists at origin/main, at the #672 sha, or at the #706 sha. Measured, the sequence is 59, 59, 60, 60 then 81 — a 21-line jump, so the conclusion was if anything understated. Corrected in all three places it was repeated, including the canonical v4 row of docs.corpus-size-signal. - The crosscheck record called `decisions-guard` a REQUIRED check — introduced by the previous commit in the sentence rewritten to fix an overclaim. Verified against Gitea branch protection: `main` requires exactly `Build & test (.NET)`, `EF migration integrity` and `review-verdict/h10`. NEITHER script-tests NOR decisions-guard is required; the record now says so. - docs.corpus-size-signal said 37 additions "to reach" the cap two paragraphs above 38 "below the cap" — a same-document numeric inconsistency of exactly the class this change set out to remove. Both now state 38 to BREACH, noting 37 lands on 0.25 and passes. - Also: the old bound's accepted range is 39..229 (not 43..229 — 43 is the NEW bound's lower edge); "95% over the ceiling" was 100%; `oversized_records` said the #620 distribution began at 0 lines where the record itself says 2. Verification: 428 scripts/tests pass; ruff at baseline parity (47); validator exits 0 with no drift notice; corpus at p90=60, 18/183 over the ceiling, record trimmed to 60 lines so main ships calibrated. Decisions-Edit: yes Co-Authored-By: Claude Opus 5 (1M context) --- .../records/docs/corpus-size-signal.md | 6 +- .../docs/frontmatter-pyyaml-crosscheck.md | 12 ++-- scripts/decisions_validate.py | 4 +- scripts/tests/test_decisions_validate.py | 65 ++++++++++++------- 4 files changed, 53 insertions(+), 34 deletions(-) diff --git a/docs/decisions/records/docs/corpus-size-signal.md b/docs/decisions/records/docs/corpus-size-signal.md index 09b7f5157..44bee9434 100644 --- a/docs/decisions/records/docs/corpus-size-signal.md +++ b/docs/decisions/records/docs/corpus-size-signal.md @@ -56,7 +56,7 @@ Getting there took five versions, and the failures are the useful part: | v1 | `max(under) <= 60 < min(over)` | true **by construction** of those two lists | | v2 | a minimum gap WIDTH | a ceiling of 200 also sits in a wide gap — it passed | | v3 | 2-12% fraction band + "clear air" above | **hostage to an unrelated record**: one ordinary 62-line addition reddened it with the ceiling correctly placed, and the only remedy was to RAISE the ceiling — this very treadmill, as a hard failure in what #631 makes a blocking job. The fraction band had the same coupling more slowly (12 more long records breached it), and `0 <= headroom` was vacuous. | -| v4 | `p90 <= ceiling <= p95` | percentiles move with the corpus, but an order statistic over a SPARSE distribution is a STEP function. The lengths above the ceiling ran 60, 61, 62, 63 then jumped to 81, so ONE record moved p90 by twenty lines. It reddened the blocking job twice live (#672, #706), and both times the only in-scope remedy was to trim the new record to fit the constant — the v3 ratchet, pointed at record authors | +| v4 | `p90 <= ceiling <= p95` | percentiles move with the corpus, but an order statistic over a SPARSE distribution is a STEP function. The lengths around the ceiling run 59, 59, 60, 60 and then jump straight to 81 (measured), so ONE record moved p90 by twenty-one lines. It reddened the blocking job twice live (#672, #706), and both times the only in-scope remedy was to trim the new record to fit the constant — the v3 ratchet, pointed at record authors | | v5 | coarse `0.02 <= fraction_over <= 0.25` asserted; fine `p90 <= ceiling <= p95` REPORTED | splits the claim by robustness instead of hunting for a better single assertion (#688) | **v5 is not a fifth attempt at the same shape — it stops trying.** Four versions failed because they @@ -65,8 +65,8 @@ The fine claim is genuinely useful and genuinely fragile, so it is now measured printed as a `::notice::` — the same treatment `stale_records` gets, and for the same stated reason: a constant going out of date is the passage of time, not a defect in this change. What stays blocking is only what routine growth cannot break — each record moves a fraction by at most 1/N, so -from **18/183** over the ceiling it takes **37** consecutive over-ceiling additions to reach the 25% -cap, against **one** record to break v4. +from **18/183** over the ceiling it takes **38** consecutive over-ceiling additions to BREACH the +25% cap (37 lands exactly on 0.25, which still passes), against **one** record to break v4. **The floor is a fraction, not `> 0`, and review is why.** The first draft of v5 asserted only `0 < fraction_over < 1/3`, which measured against the real corpus accepted **every ceiling from 39 diff --git a/docs/decisions/records/docs/frontmatter-pyyaml-crosscheck.md b/docs/decisions/records/docs/frontmatter-pyyaml-crosscheck.md index 0a08e88b7..c76ce119a 100644 --- a/docs/decisions/records/docs/frontmatter-pyyaml-crosscheck.md +++ b/docs/decisions/records/docs/frontmatter-pyyaml-crosscheck.md @@ -59,12 +59,12 @@ to `pyyaml_frontmatter_faults`, so the tool and the suite cannot drift on the de **What this buys, stated precisely, because it is less than it looks.** In CI `decisions-guard` installs nothing, so the validator there always takes the skip path, by design; and `script-tests` already went red on both hazards before this change, and an advisory red still blocks the merge gate -(#598). So **no broken record has reached `main`, and the CI delta here is close to zero** — though -that protection is procedural rather than structural, since `script-tests` is not a REQUIRED check -and `decisions-guard`, which is, runs without PyYAML and so always skips this cross-check. What this -fixes is the case #674 actually -described: the LOCAL loop, where the validator is the tool an agent reaches for directly and it -printed OK on a corrupt file — plus the tool/suite disagreement, now impossible. +(#598). So **no broken record has reached `main` and the CI delta here is close to zero** — though +procedurally, not structurally: branch protection on `main` requires exactly three contexts (`Build +& test (.NET)`, `EF migration integrity`, `review-verdict/h10`), and NEITHER `script-tests` nor +`decisions-guard` is among them. What this fixes is the case #674 described: the LOCAL loop, where +the validator is the tool an agent reaches for directly and it printed OK on a corrupt file — plus +the tool/suite disagreement, now impossible. **Limits, and the positive control.** It does not catch a mis-parse both parsers agree on — strong, not total, the same qualification `record_wing_faults` carries. The suite pins that diff --git a/scripts/decisions_validate.py b/scripts/decisions_validate.py index c2fd318f0..2973c34a7 100644 --- a/scripts/decisions_validate.py +++ b/scripts/decisions_validate.py @@ -738,7 +738,7 @@ def oversized_records(records, ceiling: int) -> list[tuple[str, int]]: act on instead of asserting that "the corpus" is too big. The ceiling sits at a natural gap in the real distribution rather than a round number: at #620 - the records run 0..59 prose lines (median 26, p90 52) and then jump straight to 83, with + the records ran 2..59 prose lines (median 26, p90 52) and then jumped straight to 83, with nothing in between. 60 separates the bulk from the tail without splitting a cluster. IMPORTANT — a prompt for judgement, not a target. Length is a PROXY for "grown past what a @@ -769,7 +769,7 @@ def ceiling_calibration(records, ceiling: int) -> CeilingCalibration: `marks_tail` — `p90 <= ceiling <= p95`. Correct as a definition of "start of the tail", but an order statistic over a SPARSE distribution is a STEP function: at #688 the lengths just above - the ceiling ran 60, 61, 62, 63, then jumped to 81, so ONE new record could move p90 by twenty + the ceiling run 59, 59, 60, 60 and then jump straight to 81, so ONE new record could move p90 lines and the only remedy the assertion admitted was to raise the ceiling. It is real signal, but it is signal about the CONSTANT drifting, not a defect in the commit under test — the same shape as `stale_records`, and it is reported the same way: a notice, never a failure. diff --git a/scripts/tests/test_decisions_validate.py b/scripts/tests/test_decisions_validate.py index bea4de3c4..1c7013c88 100644 --- a/scripts/tests/test_decisions_validate.py +++ b/scripts/tests/test_decisions_validate.py @@ -1013,7 +1013,7 @@ def test_real_corpus_ceiling_flags_a_nonempty_proper_minority(): remedy the assertion admitted was to RAISE the ceiling. v4 `p90 <= ceiling <= p95`. Scale-free and correct AS A DEFINITION, but an order statistic over a SPARSE distribution is a STEP function. At #688 the lengths above the ceiling ran - 60, 61, 62, 63 and then jumped to 81, so ONE new record moved p90 by twenty lines and + 59, 59, 60, 60 and then jump straight to 81, so ONE new record moved p90 by 21 lines and reddened the BLOCKING `script-tests` job for whoever happened to write it. It reproduced twice live (#672, #706) and both times the only in-scope remedy was to trim the new record to fit the constant — the ratchet pointed at record authors, which is precisely @@ -1062,17 +1062,21 @@ def test_ceiling_calibration_detects_drift_in_BOTH_directions(): # and the coarse property separates the same two failure modes assert not dv.ceiling_calibration(recs, 999).flags_minority, "a ceiling nobody is over signals nothing" - assert not dv.ceiling_calibration(recs, 10).flags_minority, "95% over the ceiling is not a tail" + assert not dv.ceiling_calibration(recs, 10).flags_minority, "100% over the ceiling is not a tail" assert dv.ceiling_calibration(recs, 60).flags_minority -def test_the_coarse_bound_REJECTS_a_badly_placed_ceiling_on_the_REAL_corpus(): - """The blocking property must have teeth, demonstrated against the live distribution. +def test_the_coarse_bound_REJECTS_a_badly_placed_ceiling(): + """The blocking property must have teeth. Review's strongest finding on the first draft: a floor of `fraction_over > 0` was nearly - unfalsifiable — it accepted every ceiling from 39 to 229, including the ceiling of 200 that the - docstring itself offered as the case it catches, because one 230-line record keeps the count - nonzero. A FRACTION floor is what restores the teeth, so pin it on the real corpus. + unfalsifiable — measured on the live corpus it accepted every ceiling from 39 to 229, including + the ceiling of 200 the docstring itself offered as the case it catches, because one 230-line + record keeps the count nonzero. A FRACTION floor is what restores the teeth. + + The rejections are pinned on a SYNTHETIC distribution: asserting that a specific absurd ceiling + stays rejected by the live corpus is itself growth-coupled (three new 200+ line records flip the + 200 arm). Only the acceptance of today's ceiling is checked against live data. """ # The TEETH are demonstrated on an owned distribution, for the reason in # `test_v4_would_have_reddened_where_v5_holds`: an assertion that a specific absurd ceiling is @@ -1126,13 +1130,11 @@ def test_adding_ordinary_records_cannot_RED_the_blocking_property(): # ...and the v5 blocking property holds. assert after.flags_minority, f"two ordinary additions reddened the blocking property: {after}" - # Live headroom, as a number, so a future reader can see how much slack there actually is: how - # many consecutive over-ceiling records it would take to BREACH the cap from here. - n, over = after.n, after.n_over - need = 0 - while (over + need) / (n + need) <= dv.CEILING_MINORITY_MAX: - need += 1 - assert need > 20, f"only {need} over-ceiling additions would breach the bound — too tight" + # NO headroom assertion here. An earlier draft asserted `need > 20` over the live corpus, and + # review measured it failing after 16 over-ceiling additions — an arbitrary threshold on a live + # order statistic is the ratchet wearing a different hat. The measured headroom (38 additions, + # or 718 short ones by dilution) is stated in `ceiling_calibration` and `docs.corpus-size-signal` + # where being out of date costs a doc correction, not a red build for an unrelated author. def test_v4_would_have_reddened_where_v5_holds(): @@ -1149,7 +1151,7 @@ def test_v4_would_have_reddened_where_v5_holds(): Nothing about this demonstration needs the real corpus. The synthetic base reproduces the shape that matters — a sparse gap just above the ceiling, exactly what #688 measured on `main` (60, - 61, 62, 63, then a jump to 81) — so two over-ceiling additions advance the p90 index off the + 60, 60, then a jump to 81) — so two over-ceiling additions advance the p90 index off the ceiling and break v4, while v5 is untouched. """ base = ( @@ -1326,15 +1328,24 @@ def test_retired_budget_flag_says_it_is_ignored(capsys): def test_no_budget_flag_means_no_retirement_warning(capsys): + # Match the retirement notice specifically, not a bare "RETIRED": a legitimate record whose + # TITLE contains that word and whose `stale-after` has passed gets printed by the stale notice, + # which would red this on an unrelated corpus change. dv.main([]) - assert "RETIRED" not in capsys.readouterr().err + assert "is RETIRED and was IGNORED" not in capsys.readouterr().err def test_main_reports_ceiling_drift_as_a_NOTICE_and_still_exits_0(capsys): - """The fine claim's live wiring (#688). A ceiling of 999 is parked far above p95, so the drift - notice must fire — and must NOT turn the run red, which is the entire point of the v5 split.""" - assert dv.main(["--record-ceiling", "999"]) == 0 + """The fine claim's live wiring (#688): the drift notice must fire, and must NOT turn the run + red — the entire point of the v5 split. + + The ceiling is DERIVED as one line above the longest record, so it is off the tail boundary by + definition. A hardcoded 999 looked safe and was not: review showed ten valid 1000-line records + would put p95 at 1000, making 999 calibrated and silently deleting this test's only assertion. + """ + longest = max(dv.record_prose_lines(r) for r in dl.all_active_records() if r.key) + assert dv.main(["--record-ceiling", str(longest + 1)]) == 0 err = capsys.readouterr().err drift = [ln for ln in err.splitlines() if "drifted from the tail boundary" in ln] assert len(drift) == 1, err @@ -1355,6 +1366,7 @@ def test_main_reports_drift_IFF_the_ceiling_is_off_the_tail_boundary(capsys): is genuinely exercised, so this cannot pass by never firing. """ recs = [r for r in dl.all_active_records() if r.key] + assert len(recs) > 100, f"corpus looks empty ({len(recs)}) — the derived ceilings need records" lengths = sorted(dv.record_prose_lines(r) for r in recs) # Both ceilings are DERIVED so each branch is guaranteed by construction, not by luck. Review @@ -1385,15 +1397,22 @@ def test_main_actually_REPORTS_the_ceiling_and_the_trend(capsys): """The new signal's live wiring was untested: `if oversized:` -> `if False:`, or bumping the default ceiling to 999999, left every test green while main() reported nothing. Only the pure function `oversized_records()` was covered — so the replacement signal could silently do - nothing, which is the exact defect this change exists to retire.""" + nothing, which is the exact defect this change exists to retire. + + Stated as an IFF against the live offender list rather than `assert over` (#688): the ceiling + is ALLOWED to go green — `test_oversized_records_can_go_green` says so explicitly — so a bare + precondition that the corpus still has an offender would red the blocking job the day someone + consolidates the last one, punishing exactly the work the warning asks for.""" dv.main([]) err = capsys.readouterr().err assert "prose lines across" in err, "the aggregate trend notice must always print" - assert "exceed the" in err and "prose ceiling" in err, "the per-record ceiling warning must print" + over = [r.key for r in dl.all_active_records() if r.key and dv.record_prose_lines(r) > dv.RECORD_CEILING_DEFAULT] - assert over, "precondition: the live corpus has at least one over-ceiling record" - assert any(k in err for k in over), "the warning must NAME the offending records" + warned = "exceed the" in err and "prose ceiling" in err + assert warned is bool(over), f"ceiling warning printed={warned} but {len(over)} record(s) are over it" + if over: + assert any(k in err for k in over), "the warning must NAME the offending records" def test_trend_notice_reports_record_prose_and_scaffolding_separately(capsys): From c56dfdd539eda31ad51dd4f0760489e022d59155 Mon Sep 17 00:00:00 2001 From: Timothy Date: Tue, 4 Aug 2026 21:47:24 +0200 Subject: [PATCH 5/9] fix(688): delete the last over-tight test, pin the crosscheck's INPUT, fix 4 prose defects MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review round 4, both reviewers. Both report the code path SOUND and the #688 coupling class analytically gone (rows proved, not merely observed green); one caught 28 of 30 mutations. What blocked was one over-tight test, two unpinned mutations, and prose — including two defects the PREVIOUS commit introduced while claiming to fix numbers. TESTS - Deleted test_adding_ordinary_records_cannot_RED_the_blocking_property. It appended two long records to the LIVE corpus and asserted flags_minority on the RESULT, so it crossed the cap two records before production does (56/221 vs 54/219) — a test named "cannot RED the blocking property" being a tighter tripwire than the property. Fourth instance of the #688 defect in this change. Deleted rather than tuned: both its jobs are already covered off live data (the synthetic v4/v5 contrast, and the deliberate live guard at the production threshold). - test_main_FEEDS_the_crosscheck_the_REAL_wing_files closes a mutation hole found by review: replacing `pyyaml_frontmatter_faults(record_wing_files())` with `...([])` in main() left the ENTIRE suite green. Both existing wiring tests monkeypatch the function, so they pinned that its RETURN reaches errs, never that its ARGUMENT is the corpus — the '#609 marker that printed OK while doing nothing' defect one level up, which is the exact thing the new record indicts. Verified: the mutation now fails this test. - test_main_actually_REPORTS_... went vacuous whenever the ceiling legitimately goes green (`False is False` passes with the whole warning branch deleted). Added an arm at --record-ceiling 0, which no non-empty corpus can make vacuous. - Pinned two surviving mutations: ceiling_calibration's n_over boundary (it recomputes the count, so oversized_records' exclusivity test does not cover it — `>` vs `>=` differs by the 3 records sitting exactly on the ceiling) and p95's quantile (the 95/5 fixture cannot tell 0.95 from 0.99). PROSE — two of these were introduced by the previous commit, whose stated job was fixing numbers. That is the pattern worth naming, not the individual typos. - "so ONE new record could move p90 lines" — the previous commit deleted the magnitude and left the sentence ungrammatical. Now "by 21 lines". - It also introduced a THIRD variant of the sequence it was correcting ("60, 60, 60") and missed a FOURTH site in ci-cd.md still saying "twenty lines". All four sites now read the measured 59, 59, 60, 60 -> 81, and 21 lines. - 59- and 60-line records were described as "above the ceiling"; they are at or below it. - "routine growth cannot cross it" overstated the bound: it is deliberately less sensitive, not immune. Reworded, and the THIRD and tightest arm is now documented wherever the other two appear: consolidating 15 of the 18 offenders drops below the 2% floor (verified: 3/183 = 1.64%). That is in real tension with test_oversized_records_can_go_green and is stated as accepted — at 3/183 the constant genuinely is mis-calibrated — with the remedy named: a consolidation PR that large should re-derive the ceiling in the same change. - Corrected a docstring that called the 999-ceiling failure "silently deleting the assertion"; it would go red, not silent. Verification: 430 scripts/tests pass; ruff at baseline parity (47); validator exits 0 with no drift notice; corpus at p90=60, 18/183, calibrated. The two new claims were measured, not assumed: the empty-list mutation fails the new test, and 15 consolidations reaches 1.64%. Decisions-Edit: yes Co-Authored-By: Claude Opus 5 (1M context) --- docs/ci-cd.md | 8 +- .../records/docs/corpus-size-signal.md | 7 ++ scripts/decisions_validate.py | 11 ++- scripts/tests/test_decisions_validate.py | 99 +++++++++++++------ 4 files changed, 92 insertions(+), 33 deletions(-) diff --git a/docs/ci-cd.md b/docs/ci-cd.md index e574e6776..97707e995 100644 --- a/docs/ci-cd.md +++ b/docs/ci-cd.md @@ -66,14 +66,16 @@ step is now continuous. The release boundary is instead where you: accepting an absurd ceiling, the last two by ratcheting: - **blocking** (`script-tests`) — only the coarse property that the ceiling flags a **meaningful minority** of records (`0.02 <= fraction_over <= 0.25`). One record moves a - fraction by at most 1/N, so routine growth cannot cross it: from today's 18/183 it takes 38 - consecutive over-ceiling additions, or 718 short ones to dilute below the floor. The floor is + fraction by at most 1/N, so ordinary growth cannot cross it — it is deliberately less + sensitive, not immune. From today's 18/183 it takes 38 consecutive over-ceiling additions to + breach the cap, 718 short ones to dilute below the floor, or — the tightest arm — + consolidating 15 of the 18 offenders away. The floor is a fraction rather than "at least one record", which would accept any ceiling up to 229 on the live corpus; as a fraction the accepted range is 43..180. - **reported, never asserted** — the fine claim that the ceiling sits between the **90th and 95th percentile**, i.e. at the tail boundary. `main()` prints a `::notice::` when it drifts. It is an order statistic over a sparse distribution, so a single new record could move p90 by - twenty lines and red the blocking job for whoever wrote it; a ceiling going out of date is + 21 lines and red the blocking job for whoever wrote it; a ceiling going out of date is the passage of corpus growth, not a defect in the commit under test, so it is treated like `stale-after`. Re-derive the constant when the notice says so. - the **aggregate prose total**, printed every run as an unthresholded `::notice::` **trend**. diff --git a/docs/decisions/records/docs/corpus-size-signal.md b/docs/decisions/records/docs/corpus-size-signal.md index 44bee9434..ec6e627d1 100644 --- a/docs/decisions/records/docs/corpus-size-signal.md +++ b/docs/decisions/records/docs/corpus-size-signal.md @@ -76,6 +76,13 @@ is not a guard. At a 2% floor and a 25% cap the accepted range is **43..180** (m a ceiling of 200 flags 0.5% of records and is rejected, a ceiling of 20 flags 60% and is rejected, and today's 9.8% sits about 5x ABOVE the floor and 38 over-ceiling additions below the cap. +**Three arms, and the tightest is CONSOLIDATION** — stated because it is the easy one to forget. +Breaching the cap takes 38 over-ceiling additions; diluting below the floor takes 718 short ones; +but trimming or archiving **15** of today's 18 over-ceiling records also drops below the floor. That +is a real tension with `test_oversized_records_can_go_green`, and it is accepted rather than papered +over: at 3/183 the constant genuinely IS mis-calibrated, so the red is the signal working. A +consolidation PR big enough to reach it should re-derive the ceiling in the same change. + The honest cost, stated rather than buried: **nothing now forces a re-derivation.** The ceiling can drift while only a notice complains. That is accepted on the same reasoning this record already applies to its two "keep listed" consolidation candidates — the warning names it on every run, which diff --git a/scripts/decisions_validate.py b/scripts/decisions_validate.py index 2973c34a7..6f155788b 100644 --- a/scripts/decisions_validate.py +++ b/scripts/decisions_validate.py @@ -768,8 +768,8 @@ def ceiling_calibration(records, ceiling: int) -> CeilingCalibration: made the previous guard a ratchet: `marks_tail` — `p90 <= ceiling <= p95`. Correct as a definition of "start of the tail", but an - order statistic over a SPARSE distribution is a STEP function: at #688 the lengths just above - the ceiling run 59, 59, 60, 60 and then jump straight to 81, so ONE new record could move p90 + order statistic over a SPARSE distribution is a STEP function: at #688 the lengths AROUND the + ceiling run 59, 59, 60, 60 and then jump straight to 81, so ONE new record could move p90 by 21 lines and the only remedy the assertion admitted was to raise the ceiling. It is real signal, but it is signal about the CONSTANT drifting, not a defect in the commit under test — the same shape as `stale_records`, and it is reported the same way: a notice, never a failure. @@ -780,6 +780,13 @@ def ceiling_calibration(records, ceiling: int) -> CeilingCalibration: consecutive over-ceiling additions (37 lands exactly on 0.25, which still passes under `<=`), or 718 short ones to dilute below the floor — against ONE record to break `marks_tail`. + The THIRD arm is the tightest and is stated here because it is the easy one to forget: + CONSOLIDATION. Trimming or archiving 15 of today's 18 over-ceiling records drops the fraction + below the 2% floor. That is a real tension with `test_oversized_records_can_go_green` — the + ceiling is allowed to go green — and it is accepted rather than papered over: at 3/183 the + constant genuinely IS mis-calibrated and saying so is the signal working. A consolidation PR + large enough to hit it should re-derive the ceiling in the same change. + It still catches genuine mis-calibration in both directions, measured on the real corpus: a ceiling of 20 flags 60% of records (cutting into the bulk, so every author learns to ignore it), and a ceiling of 200 flags 0.5% — one record — which is below the floor and rejected. Note that diff --git a/scripts/tests/test_decisions_validate.py b/scripts/tests/test_decisions_validate.py index 1c7013c88..b231005fd 100644 --- a/scripts/tests/test_decisions_validate.py +++ b/scripts/tests/test_decisions_validate.py @@ -1012,7 +1012,7 @@ def test_real_corpus_ceiling_flags_a_nonempty_proper_minority(): ordinary 62-line addition reddened it with the ceiling correctly placed, and the only remedy the assertion admitted was to RAISE the ceiling. v4 `p90 <= ceiling <= p95`. Scale-free and correct AS A DEFINITION, but an order statistic - over a SPARSE distribution is a STEP function. At #688 the lengths above the ceiling ran + over a SPARSE distribution is a STEP function. At #688 the lengths AROUND the ceiling ran 59, 59, 60, 60 and then jump straight to 81, so ONE new record moved p90 by 21 lines and reddened the BLOCKING `script-tests` job for whoever happened to write it. It reproduced twice live (#672, #706) and both times the only in-scope remedy was to trim the new @@ -1106,35 +1106,41 @@ def test_ceiling_calibration_is_empty_safe(): assert cal.n == 0 and not cal.marks_tail and not cal.flags_minority -def test_adding_ordinary_records_cannot_RED_the_blocking_property(): - """#688's Done-when, demonstrated rather than assumed. +# `test_adding_ordinary_records_cannot_RED_the_blocking_property` used to live here. It appended two +# long synthetic records to the LIVE corpus and asserted `flags_minority` on the result — which +# crosses the 25% cap TWO records before the production bound does (56/221 vs 54/219), making the +# test named "cannot RED the blocking property" a tighter tripwire than the property it guarded. +# That is the #688 defect in miniature, and the fourth instance found in this change. +# +# Deleted rather than tuned, because both of its jobs are covered without touching live data: +# `test_v4_would_have_reddened_where_v5_holds` demonstrates the v4/v5 contrast on an owned +# distribution, and `test_real_corpus_ceiling_flags_a_nonempty_proper_minority` is the deliberate +# live guard — at the production threshold rather than two records inside it. - Takes the REAL corpus and appends the two record shapes that actually reddened v4 in the wild: - #672's 62-line record and #687's 107-line one. Under v4 the 107-line addition moved p90 into - the sparse gap above the ceiling and failed the blocking job. Under v5 the coarse property - still holds, and it holds with room to spare. + +def test_ceiling_calibration_counts_over_the_ceiling_EXCLUSIVELY(): + """`n_over` is recomputed inside `ceiling_calibration`, so its boundary needs its own pin. + + `oversized_records` has an exclusivity test; this counter does not share its code. Flipping + `>` to `>=` here would silently shift the fraction by the number of records sitting exactly ON + the ceiling (3 in the live corpus), and the mutation survived the whole suite. """ - real = [r for r in dl.all_active_records() if r.key] - ceiling = dv.RECORD_CEILING_DEFAULT - grown = real + [_rec_body("new.typical", 62), _rec_body("new.substantial", 107)] + recs = [_rec_body("a.under", 59), _rec_body("b.exact", 60), _rec_body("c.over", 61)] + assert dv.ceiling_calibration(recs, 60).n_over == 1 - before = dv.ceiling_calibration(real, ceiling) - after = dv.ceiling_calibration(grown, ceiling) - # The additions must actually MOVE something, or this test proves nothing about them — review - # caught the first draft passing identically with the two records removed. - assert after.n == before.n + 2 and after.n_over == before.n_over + 2, ( - f"the appended records were not counted: {before} -> {after}" +def test_ceiling_calibration_uses_the_95th_percentile_not_a_higher_one(): + """Pins p95's quantile. The synthetic 95/5 fixture cannot tell 0.95 from 0.99, so a mutation + widening the upper quantile survived the whole suite.""" + # 100 records: indices 0..89 = 10, 90..94 = 50, 95..98 = 90, 99 = 900. + recs = ( + [_rec_body(f"a.s{i}", 10) for i in range(90)] + + [_rec_body(f"b.m{i}", 50) for i in range(5)] + + [_rec_body(f"c.h{i}", 90) for i in range(4)] + + [_rec_body("d.max", 900)] ) - - # ...and the v5 blocking property holds. - assert after.flags_minority, f"two ordinary additions reddened the blocking property: {after}" - - # NO headroom assertion here. An earlier draft asserted `need > 20` over the live corpus, and - # review measured it failing after 16 over-ceiling additions — an arbitrary threshold on a live - # order statistic is the ratchet wearing a different hat. The measured headroom (38 additions, - # or 718 short ones by dilution) is stated in `ceiling_calibration` and `docs.corpus-size-signal` - # where being out of date costs a doc correction, not a red build for an unrelated author. + cal = dv.ceiling_calibration(recs, 60) + assert (cal.p90, cal.p95) == (50, 90), f"p95 must read index 95, not a higher quantile: {cal}" def test_v4_would_have_reddened_where_v5_holds(): @@ -1150,8 +1156,8 @@ def test_v4_would_have_reddened_where_v5_holds(): extra=16 before(marks=True) after(marks=True) -> reds: True Nothing about this demonstration needs the real corpus. The synthetic base reproduces the shape - that matters — a sparse gap just above the ceiling, exactly what #688 measured on `main` (60, - 60, 60, then a jump to 81) — so two over-ceiling additions advance the p90 index off the + that matters — a sparse gap just above the ceiling, exactly what #688 measured on `main` + (59, 59, 60, 60, then a jump to 81) — so two over-ceiling additions advance the p90 index off the ceiling and break v4, while v5 is untouched. """ base = ( @@ -1306,6 +1312,32 @@ def test_main_ANNOUNCES_a_skipped_crosscheck(capsys, monkeypatch): assert "cross-check" in err and "SKIPPED" in err, err +def test_main_FEEDS_the_crosscheck_the_REAL_wing_files(monkeypatch, capsys): + """Pins the cross-check's INPUT, not just that its output is consumed. + + Mutation testing found this hole: replacing `pyyaml_frontmatter_faults(record_wing_files())` + with `pyyaml_frontmatter_faults([])` in main() left the ENTIRE suite green — exit 0, no skip + notice, every other test passing. The two wiring tests monkeypatch the function itself, so they + prove the return value reaches `errs`; nothing proved the argument was the corpus. That is the + '#609 marker that printed OK while doing nothing' defect one level up, which is the exact thing + this record indicts — and the sibling of `test_main_actually_CALLS_the_wing_scan`. + """ + seen: list[list] = [] + + def spy(files): + seen.append(list(files)) + return [], True + + monkeypatch.setattr(dv, "pyyaml_frontmatter_faults", spy) + dv.main([]) + + assert seen, "main() never called the cross-check at all" + assert len(seen[0]) > 100, f"main() passed only {len(seen[0])} file(s) — not the real wings" + assert any(p.name == "corpus-size-signal.md" for p in seen[0]), ( + "the file list main() passed does not look like the live record wings" + ) + + def test_main_FAILS_when_the_crosscheck_reports_a_fault(capsys, monkeypatch): """Wiring test: the faults must reach the exit code, not just be computed. @@ -1342,7 +1374,8 @@ def test_main_reports_ceiling_drift_as_a_NOTICE_and_still_exits_0(capsys): The ceiling is DERIVED as one line above the longest record, so it is off the tail boundary by definition. A hardcoded 999 looked safe and was not: review showed ten valid 1000-line records - would put p95 at 1000, making 999 calibrated and silently deleting this test's only assertion. + would put p95 at 1000, making 999 calibrated — so the notice would stop firing and this test + would go RED, for a corpus change that is nobody's defect. """ longest = max(dv.record_prose_lines(r) for r in dl.all_active_records() if r.key) assert dv.main(["--record-ceiling", str(longest + 1)]) == 0 @@ -1414,6 +1447,16 @@ def test_main_actually_REPORTS_the_ceiling_and_the_trend(capsys): if over: assert any(k in err for k in over), "the warning must NAME the offending records" + # The IFF above is only non-vacuous while an offender exists: once the corpus is legitimately + # consolidated to zero, `False is False` passes even if main()'s whole `if oversized:` branch + # were deleted. So force the branch with a ceiling nothing can sit under. This arm cannot go + # vacuous for any non-empty corpus. + dv.main(["--record-ceiling", "0"]) + forced = capsys.readouterr().err + assert "exceed the" in forced and "prose ceiling" in forced, ( + "at a ceiling of 0 every record is over it — the warning branch must fire" + ) + def test_trend_notice_reports_record_prose_and_scaffolding_separately(capsys): """The notice used to print `_budget_total()` 'across N records', mixing two incompatible From b24c51ab51742fb65b6407b5e5689c988d318190 Mon Sep 17 00:00:00 2001 From: Timothy Date: Tue, 4 Aug 2026 22:11:25 +0200 Subject: [PATCH 6/9] fix(688): stop enumerating multiplicities, pin n and the keyless filter, de-couple the vacuity floors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review round 5. One reviewer returned MERGEABLE with prose findings; the other found four more, two of them real test gaps. Both are addressed here. THE MULTIPLICITIES WERE WRONG AGAIN — fourth commit running. The "measured" sequence 59, 59, 60, 60 -> 81 is measured nowhere: origin/main has 59, 59, 59, 60, 60 and HEAD has four 59s and three 60s. I had even tagged it `(measured)` in a canonical decision record. So this stops enumerating them. All four sites now state only the load-bearing, stable fact: the lengths climb to the ceiling and then jump STRAIGHT to 81 with nothing in between, so one record moves p90 by 21 lines. The multiplicities change with every record added; the gap is the point. This is the same "fix the boundary, not the site" move the tests got three rounds ago, applied to prose that had failed four times. TEST GAPS - Deleting the over-tight test removed the only pin on CeilingCalibration.n: a mutation returning n=1 passed all 19 relevant tests while printing a wrong denominator in the drift notice. Pinned. - The `if r.key` filter was load-bearing in production and unpinned: main() passes the UNFILTERED list (194 entries, 11 keyless, one a 106-line "Records formerly in this file" scaffolding block), while every test handed the function a pre-filtered list — oracle and production agreed only by accident. Pinned. - The --record-ceiling 0 arm's claim that it "cannot go vacuous for any non-empty corpus" was FALSE: an empty record body is validator-valid and record_prose_lines returns 0, so a corpus of empty-bodied records has no offender at 0. Now -1, which makes the claim true. - The three `len(recs) > 100` vacuity floors were themselves growth-coupled — 83 legitimate retirements would red them even with the ceiling still calibrated, which is the #688 class in the guard rather than the assertion. Lowered to >20 where a floor is meaningful, and to plain non-empty on the derived-ceiling test, whose derivations need nothing more. - test_main_FEEDS_the_crosscheck now compares against `set(record_wing_files())` instead of a hardcoded basename, killing the same mutation with zero corpus dependence. PROSE - "ordinary growth cannot cross it — NOT immune" contradicted itself in four places. Now: no SINGLE ordinary addition can cross it; this is measured headroom, not immunity. - "trimming or archiving 15" blurred two different denominators. Trimming leaves 3/183 = 1.64%; archiving leaves 3/168 = 1.79% because the denominator moves too. Both verified, both under the floor, now stated separately. Verification: 431 scripts/tests pass; ruff at baseline parity (47 — a 121-char docstring line briefly took it to 48 and is rewrapped); validator exits 0 with no drift notice; corpus at p90=60, 18/183, calibrated; record still 60 lines. Decisions-Edit: yes Co-Authored-By: Claude Opus 5 (1M context) --- docs/ci-cd.md | 4 +- .../records/docs/corpus-size-signal.md | 4 +- scripts/decisions_validate.py | 15 +++-- scripts/tests/test_decisions_validate.py | 60 +++++++++++++------ 4 files changed, 56 insertions(+), 27 deletions(-) diff --git a/docs/ci-cd.md b/docs/ci-cd.md index 97707e995..357f1af20 100644 --- a/docs/ci-cd.md +++ b/docs/ci-cd.md @@ -66,8 +66,8 @@ step is now continuous. The release boundary is instead where you: accepting an absurd ceiling, the last two by ratcheting: - **blocking** (`script-tests`) — only the coarse property that the ceiling flags a **meaningful minority** of records (`0.02 <= fraction_over <= 0.25`). One record moves a - fraction by at most 1/N, so ordinary growth cannot cross it — it is deliberately less - sensitive, not immune. From today's 18/183 it takes 38 consecutive over-ceiling additions to + fraction by at most 1/N, so no SINGLE ordinary addition can cross it. This is measured + headroom, not immunity: from today's 18/183 it takes 38 consecutive over-ceiling additions to breach the cap, 718 short ones to dilute below the floor, or — the tightest arm — consolidating 15 of the 18 offenders away. The floor is a fraction rather than "at least one record", which would accept any ceiling up to 229 on the diff --git a/docs/decisions/records/docs/corpus-size-signal.md b/docs/decisions/records/docs/corpus-size-signal.md index ec6e627d1..fe88ce668 100644 --- a/docs/decisions/records/docs/corpus-size-signal.md +++ b/docs/decisions/records/docs/corpus-size-signal.md @@ -56,7 +56,7 @@ Getting there took five versions, and the failures are the useful part: | v1 | `max(under) <= 60 < min(over)` | true **by construction** of those two lists | | v2 | a minimum gap WIDTH | a ceiling of 200 also sits in a wide gap — it passed | | v3 | 2-12% fraction band + "clear air" above | **hostage to an unrelated record**: one ordinary 62-line addition reddened it with the ceiling correctly placed, and the only remedy was to RAISE the ceiling — this very treadmill, as a hard failure in what #631 makes a blocking job. The fraction band had the same coupling more slowly (12 more long records breached it), and `0 <= headroom` was vacuous. | -| v4 | `p90 <= ceiling <= p95` | percentiles move with the corpus, but an order statistic over a SPARSE distribution is a STEP function. The lengths around the ceiling run 59, 59, 60, 60 and then jump straight to 81 (measured), so ONE record moved p90 by twenty-one lines. It reddened the blocking job twice live (#672, #706), and both times the only in-scope remedy was to trim the new record to fit the constant — the v3 ratchet, pointed at record authors | +| v4 | `p90 <= ceiling <= p95` | percentiles move with the corpus, but an order statistic over a SPARSE distribution is a STEP function. The lengths climb to the ceiling and then jump straight to 81 with NOTHING in between (measured; the multiplicities move with every record added, the gap is the point), so ONE record moved p90 by twenty-one lines. It reddened the blocking job twice live (#672, #706), and both times the only in-scope remedy was to trim the new record to fit the constant — the v3 ratchet, pointed at record authors | | v5 | coarse `0.02 <= fraction_over <= 0.25` asserted; fine `p90 <= ceiling <= p95` REPORTED | splits the claim by robustness instead of hunting for a better single assertion (#688) | **v5 is not a fifth attempt at the same shape — it stops trying.** Four versions failed because they @@ -78,7 +78,7 @@ and today's 9.8% sits about 5x ABOVE the floor and 38 over-ceiling additions bel **Three arms, and the tightest is CONSOLIDATION** — stated because it is the easy one to forget. Breaching the cap takes 38 over-ceiling additions; diluting below the floor takes 718 short ones; -but trimming or archiving **15** of today's 18 over-ceiling records also drops below the floor. That +but taking **15** of today's 18 over-ceiling records out of the over-set also drops below it — trimming them to <=60 leaves 3/183 = 1.64%, archiving them leaves 3/168 = 1.79%, since archiving moves the denominator too. That is a real tension with `test_oversized_records_can_go_green`, and it is accepted rather than papered over: at 3/183 the constant genuinely IS mis-calibrated, so the red is the signal working. A consolidation PR big enough to reach it should re-derive the ceiling in the same change. diff --git a/scripts/decisions_validate.py b/scripts/decisions_validate.py index 6f155788b..a7c11c148 100644 --- a/scripts/decisions_validate.py +++ b/scripts/decisions_validate.py @@ -768,22 +768,25 @@ def ceiling_calibration(records, ceiling: int) -> CeilingCalibration: made the previous guard a ratchet: `marks_tail` — `p90 <= ceiling <= p95`. Correct as a definition of "start of the tail", but an - order statistic over a SPARSE distribution is a STEP function: at #688 the lengths AROUND the - ceiling run 59, 59, 60, 60 and then jump straight to 81, so ONE new record could move p90 by 21 + order statistic over a SPARSE distribution is a STEP function: the lengths climb to the ceiling + and then jump STRAIGHT to 81 with nothing in between, so ONE new record could move p90 by 21 lines and the only remedy the assertion admitted was to raise the ceiling. It is real signal, but it is signal about the CONSTANT drifting, not a defect in the commit under test — the same shape as `stale_records`, and it is reported the same way: a notice, never a failure. `flags_minority` — `CEILING_MINORITY_MIN <= fraction_over <= CEILING_MINORITY_MAX`. Deliberately coarse, and what the blocking test asserts. Each added record moves a fraction by at most 1/N, so - routine growth cannot cross it: measured live at 18/183 (9.8%), BREACHING the 25% cap takes 38 + NO SINGLE ordinary addition can cross it — this is measured headroom, not immunity. Live at + 18/183 (9.8%), BREACHING the 25% cap takes 38 consecutive over-ceiling additions (37 lands exactly on 0.25, which still passes under `<=`), or 718 short ones to dilute below the floor — against ONE record to break `marks_tail`. The THIRD arm is the tightest and is stated here because it is the easy one to forget: - CONSOLIDATION. Trimming or archiving 15 of today's 18 over-ceiling records drops the fraction - below the 2% floor. That is a real tension with `test_oversized_records_can_go_green` — the - ceiling is allowed to go green — and it is accepted rather than papered over: at 3/183 the + CONSOLIDATION. Taking 15 of today's 18 over-ceiling records out of the over-set drops below the + 2% floor — trimming them to <=60 leaves 3/183 = 1.64%, archiving them outright leaves 3/168 = + 1.79% (the denominator moves too); either way, under the floor. That is a real tension with + `test_oversized_records_can_go_green` — the ceiling is allowed to go green — and it is accepted + rather than papered over: at 3/183 the constant genuinely IS mis-calibrated and saying so is the signal working. A consolidation PR large enough to hit it should re-derive the ceiling in the same change. diff --git a/scripts/tests/test_decisions_validate.py b/scripts/tests/test_decisions_validate.py index b231005fd..efbae2a5c 100644 --- a/scripts/tests/test_decisions_validate.py +++ b/scripts/tests/test_decisions_validate.py @@ -1012,8 +1012,8 @@ def test_real_corpus_ceiling_flags_a_nonempty_proper_minority(): ordinary 62-line addition reddened it with the ceiling correctly placed, and the only remedy the assertion admitted was to RAISE the ceiling. v4 `p90 <= ceiling <= p95`. Scale-free and correct AS A DEFINITION, but an order statistic - over a SPARSE distribution is a STEP function. At #688 the lengths AROUND the ceiling ran - 59, 59, 60, 60 and then jump straight to 81, so ONE new record moved p90 by 21 lines and + over a SPARSE distribution is a STEP function. The lengths climb to the ceiling and then + jump STRAIGHT to 81 with nothing between, so ONE new record moved p90 by 21 lines and reddened the BLOCKING `script-tests` job for whoever happened to write it. It reproduced twice live (#672, #706) and both times the only in-scope remedy was to trim the new record to fit the constant — the ratchet pointed at record authors, which is precisely @@ -1021,8 +1021,10 @@ def test_real_corpus_ceiling_flags_a_nonempty_proper_minority(): v5 SPLITS the claim by robustness instead of hunting for a better single assertion: - * the COARSE property — the ceiling flags a nonempty proper minority — is asserted HERE, - blocking. One record moves a fraction by at most 1/N, so routine growth cannot cross it. + * the COARSE property — the ceiling flags a meaningful minority — is asserted HERE, + blocking. One record moves a fraction by at most 1/N, so no SINGLE ordinary addition can + cross it — measured headroom, not immunity (38 over-ceiling additions, 718 short ones, or + consolidating 15 of the 18 offenders would each reach a bound). * the FINE property — `p90 <= ceiling <= p95` — is now REPORTED by `main()` as a notice. It is real signal about the CONSTANT drifting out of date, which is the passage of corpus growth rather than a defect in the commit under test. That is the same reasoning @@ -1032,7 +1034,9 @@ def test_real_corpus_ceiling_flags_a_nonempty_proper_minority(): every run. v5 moves where each claim is enforced, it does not stop making them. """ recs = [r for r in dl.all_active_records() if r.key] - assert len(recs) > 100, f"corpus looks empty ({len(recs)}) — this check would be vacuous" + # A low floor on purpose: this guards against a VACUOUS scan, not against corpus shrinkage. + # At >100 it would red after ~83 legitimate retirements even with the ceiling still calibrated. + assert len(recs) > 20, f"corpus looks empty ({len(recs)}) — this check would be vacuous" ceiling = dv.RECORD_CEILING_DEFAULT # the value the CLI actually uses; cannot drift from here cal = dv.ceiling_calibration(recs, ceiling) @@ -1093,10 +1097,10 @@ def test_the_coarse_bound_REJECTS_a_badly_placed_ceiling(): assert not dv.ceiling_calibration(synthetic, 10).flags_minority, "a ceiling of 10 cuts into the bulk" # The only claim made against the LIVE corpus is the robust one: today's ceiling is accepted. - # Breaching it takes 38 consecutive over-ceiling additions (or 718 short ones by dilution), so - # ordinary growth cannot red this. + # Reaching a bound takes 38 consecutive over-ceiling additions, 718 short ones by dilution, or + # consolidating 15 of the 18 offenders — the tightest arm, and the one worth remembering. recs = [r for r in dl.all_active_records() if r.key] - assert len(recs) > 100, "corpus looks empty — this check would be vacuous" + assert len(recs) > 20, "corpus looks empty — this check would be vacuous" assert dv.ceiling_calibration(recs, dv.RECORD_CEILING_DEFAULT).flags_minority @@ -1118,6 +1122,25 @@ def test_ceiling_calibration_is_empty_safe(): # live guard — at the production threshold rather than two records inside it. +def test_ceiling_calibration_IGNORES_keyless_records_and_counts_the_rest(): + """`n` and the `if r.key` filter, both of which review found unpinned. + + `main()` passes the UNFILTERED record list, so the filter is load-bearing in production while + every live-corpus test hands this function a pre-filtered list — the oracle and production's + input agreed only by accident. The corpus really does carry keyless entries (the generated + "Records formerly in this file" scaffolding, one of them 106 lines), and counting them would + drag p90/p95 around with content that is not a record. + + `n` itself lost its only pin when the over-tight live test was deleted: a mutation returning + `n=1` passed everything, which would print a wrong denominator in the drift notice. + """ + recs = [_rec_body(f"a.s{i}", 10) for i in range(9)] + [_rec_body("b.long", 500)] + keyless = _rec(key=None, heading="Records formerly in this file", body="\n".join("x" for _ in range(500))) + + assert dv.ceiling_calibration(recs, 60).n == 10, "n must count every keyed record" + assert dv.ceiling_calibration(recs + [keyless], 60) == dv.ceiling_calibration(recs, 60) + + def test_ceiling_calibration_counts_over_the_ceiling_EXCLUSIVELY(): """`n_over` is recomputed inside `ceiling_calibration`, so its boundary needs its own pin. @@ -1156,8 +1179,8 @@ def test_v4_would_have_reddened_where_v5_holds(): extra=16 before(marks=True) after(marks=True) -> reds: True Nothing about this demonstration needs the real corpus. The synthetic base reproduces the shape - that matters — a sparse gap just above the ceiling, exactly what #688 measured on `main` - (59, 59, 60, 60, then a jump to 81) — so two over-ceiling additions advance the p90 index off the + that matters — a sparse gap immediately above the ceiling, which is what #688 measured on + `main` (nothing at all between 60 and 81) — so two over-ceiling additions advance p90 off the ceiling and break v4, while v5 is untouched. """ base = ( @@ -1333,8 +1356,9 @@ def test_main_FEEDS_the_crosscheck_the_REAL_wing_files(monkeypatch, capsys): assert seen, "main() never called the cross-check at all" assert len(seen[0]) > 100, f"main() passed only {len(seen[0])} file(s) — not the real wings" - assert any(p.name == "corpus-size-signal.md" for p in seen[0]), ( - "the file list main() passed does not look like the live record wings" + assert set(seen[0]) == set(dv.record_wing_files()), ( + "main() passed a file list that is not record_wing_files() — the cross-check is not seeing " + "the corpus it is supposed to check" ) @@ -1399,7 +1423,8 @@ def test_main_reports_drift_IFF_the_ceiling_is_off_the_tail_boundary(capsys): is genuinely exercised, so this cannot pass by never firing. """ recs = [r for r in dl.all_active_records() if r.key] - assert len(recs) > 100, f"corpus looks empty ({len(recs)}) — the derived ceilings need records" + # Non-empty is all the derivations below need; a higher floor would itself be a growth tripwire. + assert recs, "corpus is empty — the derived ceilings need at least one record" lengths = sorted(dv.record_prose_lines(r) for r in recs) # Both ceilings are DERIVED so each branch is guaranteed by construction, not by luck. Review @@ -1449,12 +1474,13 @@ def test_main_actually_REPORTS_the_ceiling_and_the_trend(capsys): # The IFF above is only non-vacuous while an offender exists: once the corpus is legitimately # consolidated to zero, `False is False` passes even if main()'s whole `if oversized:` branch - # were deleted. So force the branch with a ceiling nothing can sit under. This arm cannot go - # vacuous for any non-empty corpus. - dv.main(["--record-ceiling", "0"]) + # were deleted. So force the branch with a ceiling nothing can sit under. It is -1, not 0: + # an empty record body is validator-valid and `record_prose_lines` returns 0, so a corpus of + # empty-bodied records has no offender at 0. Below zero the arm cannot go vacuous at all. + dv.main(["--record-ceiling", "-1"]) forced = capsys.readouterr().err assert "exceed the" in forced and "prose ceiling" in forced, ( - "at a ceiling of 0 every record is over it — the warning branch must fire" + "at a ceiling of -1 every record is over it — the warning branch must fire" ) From 2ff52d4236f8f4d0c15089107993a5f0555f41c8 Mon Sep 17 00:00:00 2001 From: Timothy Date: Tue, 4 Aug 2026 22:35:00 +0200 Subject: [PATCH 7/9] fix(688): make the n oracle dynamic; correct a tense that asserted false history MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review round 6: one MERGEABLE with non-blocking prose, one NOT-MERGEABLE with a real test defect. Both addressed. THE n PIN DID NOT PIN ANYTHING. `assert n == 10` was checked against a fixture holding exactly ten records, so a mutation returning a constant 10 for EVERY input satisfied it — while changing the live denominator from 183 to 10, which is precisely the production defect the test was added to close. A single hardcoded count cannot tell "counts the input" from "returns this number". Now a dynamic oracle at two distinct cardinalities; verified the constant-n mutation fails it. "MOVED p90 by 21 lines" asserted a history I had not measured. 21 is TODAY's gap (60 -> 81). The actual #672 event was smaller — at that tree p90 was 60 with the next value 83, so the 62-line record moved p90 to 62 and reddened CI with a 2-line move. The capability claim is what matters and is true at both refs; the past tense was not. Changed to "can move" in the two places that asserted it, which also makes all four sites agree with docs/ci-cd.md and the validator docstring, both of which already said "could". A REVIEW FINDING I REJECTED, having measured it. Round 6 called "p90 52" wrong for the #620-era distribution, measuring 57. That measurement is at `fefd11dff`; the record cites `f394d6ce`, and at THAT sha p90 is exactly 52 (n=167, min 2, median 26). The number is correct as written and is unchanged. Recording the disagreement rather than silently keeping it: the reviewer measured a different tree than the one the claim names. Also corrected in this branch's own commit message trail: `b24c51ab5` said origin/main has three 59-line records; it has four 59s and two 60s (HEAD: four and three). The claim that survives, and the only one the code and docs now make, is that NOTHING sits between 61 and 80 at either ref — verified independently at both. Cosmetics from the same round: a dangling modifier in ceiling_calibration's docstring, a test_decisions_lib assertion message that said "field(s) differ" when faults can now also be rejections, and a sentence in corpus-size-signal that named the replacement test without saying what it asserts. Verification: 431 scripts/tests pass; ruff at baseline parity (47); validator exits 0 with no drift notice; corpus at p90=60, 18/183, calibrated. Decisions-Edit: yes Co-Authored-By: Claude Opus 5 (1M context) --- docs/decisions/records/docs/corpus-size-signal.md | 7 ++++--- scripts/decisions_validate.py | 2 +- scripts/tests/test_decisions_lib.py | 2 +- scripts/tests/test_decisions_validate.py | 14 +++++++++++--- 4 files changed, 17 insertions(+), 8 deletions(-) diff --git a/docs/decisions/records/docs/corpus-size-signal.md b/docs/decisions/records/docs/corpus-size-signal.md index fe88ce668..da91b5123 100644 --- a/docs/decisions/records/docs/corpus-size-signal.md +++ b/docs/decisions/records/docs/corpus-size-signal.md @@ -46,8 +46,9 @@ meant to prevent exactly that could not see it: `max(under) <= 60 < min(over)` i construction** of the two lists it builds, and passes on a distribution with no gap at all. A rationale-guarding test that cannot fail is worse than none, because it launders the claim. -It is replaced by `test_real_corpus_ceiling_flags_a_nonempty_proper_minority`, which reads the value -from `RECORD_CEILING_DEFAULT` so test and CLI cannot drift. +It is replaced by `test_real_corpus_ceiling_flags_a_nonempty_proper_minority`, which asserts that the +ceiling flags a meaningful minority of records (`0.02 <= fraction_over <= 0.25`) and reads the value +from `RECORD_CEILING_DEFAULT`, so test and CLI cannot drift. Getting there took five versions, and the failures are the useful part: @@ -56,7 +57,7 @@ Getting there took five versions, and the failures are the useful part: | v1 | `max(under) <= 60 < min(over)` | true **by construction** of those two lists | | v2 | a minimum gap WIDTH | a ceiling of 200 also sits in a wide gap — it passed | | v3 | 2-12% fraction band + "clear air" above | **hostage to an unrelated record**: one ordinary 62-line addition reddened it with the ceiling correctly placed, and the only remedy was to RAISE the ceiling — this very treadmill, as a hard failure in what #631 makes a blocking job. The fraction band had the same coupling more slowly (12 more long records breached it), and `0 <= headroom` was vacuous. | -| v4 | `p90 <= ceiling <= p95` | percentiles move with the corpus, but an order statistic over a SPARSE distribution is a STEP function. The lengths climb to the ceiling and then jump straight to 81 with NOTHING in between (measured; the multiplicities move with every record added, the gap is the point), so ONE record moved p90 by twenty-one lines. It reddened the blocking job twice live (#672, #706), and both times the only in-scope remedy was to trim the new record to fit the constant — the v3 ratchet, pointed at record authors | +| v4 | `p90 <= ceiling <= p95` | percentiles move with the corpus, but an order statistic over a SPARSE distribution is a STEP function. The lengths climb to the ceiling and then jump straight to 81 with NOTHING in between (measured; the multiplicities move with every record added, the gap is the point), so ONE record can move p90 by twenty-one lines (that is today's gap; the #672 event moved it less and still reddened CI). It reddened the blocking job twice live (#672, #706), and both times the only in-scope remedy was to trim the new record to fit the constant — the v3 ratchet, pointed at record authors | | v5 | coarse `0.02 <= fraction_over <= 0.25` asserted; fine `p90 <= ceiling <= p95` REPORTED | splits the claim by robustness instead of hunting for a better single assertion (#688) | **v5 is not a fifth attempt at the same shape — it stops trying.** Four versions failed because they diff --git a/scripts/decisions_validate.py b/scripts/decisions_validate.py index a7c11c148..fb507618f 100644 --- a/scripts/decisions_validate.py +++ b/scripts/decisions_validate.py @@ -776,7 +776,7 @@ def ceiling_calibration(records, ceiling: int) -> CeilingCalibration: `flags_minority` — `CEILING_MINORITY_MIN <= fraction_over <= CEILING_MINORITY_MAX`. Deliberately coarse, and what the blocking test asserts. Each added record moves a fraction by at most 1/N, so - NO SINGLE ordinary addition can cross it — this is measured headroom, not immunity. Live at + NO SINGLE ordinary addition can cross it — this is measured headroom, not immunity. From a live 18/183 (9.8%), BREACHING the 25% cap takes 38 consecutive over-ceiling additions (37 lands exactly on 0.25, which still passes under `<=`), or 718 short ones to dilute below the floor — against ONE record to break `marks_tail`. diff --git a/scripts/tests/test_decisions_lib.py b/scripts/tests/test_decisions_lib.py index bc2cf8a06..7285d7202 100644 --- a/scripts/tests/test_decisions_lib.py +++ b/scripts/tests/test_decisions_lib.py @@ -87,4 +87,4 @@ def test_frontmatter_reader_matches_pyyaml_on_every_real_record(): faults, ran = dv.pyyaml_frontmatter_faults(files) assert ran, "PyYAML is importable here, so the comparison must have actually run" - assert not faults, f"{len(faults)} field(s) differ from PyYAML:\n" + "\n".join(faults[:5]) + assert not faults, f"{len(faults)} frontmatter fault(s) vs PyYAML:\n" + "\n".join(faults[:5]) diff --git a/scripts/tests/test_decisions_validate.py b/scripts/tests/test_decisions_validate.py index efbae2a5c..917e952a0 100644 --- a/scripts/tests/test_decisions_validate.py +++ b/scripts/tests/test_decisions_validate.py @@ -1013,7 +1013,7 @@ def test_real_corpus_ceiling_flags_a_nonempty_proper_minority(): remedy the assertion admitted was to RAISE the ceiling. v4 `p90 <= ceiling <= p95`. Scale-free and correct AS A DEFINITION, but an order statistic over a SPARSE distribution is a STEP function. The lengths climb to the ceiling and then - jump STRAIGHT to 81 with nothing between, so ONE new record moved p90 by 21 lines and + jump STRAIGHT to 81 with nothing between, so ONE new record can move p90 by 21 lines and reddened the BLOCKING `script-tests` job for whoever happened to write it. It reproduced twice live (#672, #706) and both times the only in-scope remedy was to trim the new record to fit the constant — the ratchet pointed at record authors, which is precisely @@ -1133,11 +1133,19 @@ def test_ceiling_calibration_IGNORES_keyless_records_and_counts_the_rest(): `n` itself lost its only pin when the over-tight live test was deleted: a mutation returning `n=1` passed everything, which would print a wrong denominator in the drift notice. + + The oracle is DYNAMIC and runs at two distinct cardinalities on purpose. The first attempt + asserted `n == 10` against a ten-record fixture, and review killed it: a mutation returning a + constant 10 for every input satisfied it while changing the live denominator from 183 to 10 — + preserving the exact production defect the test claims to close. A single hardcoded count + cannot distinguish "counts the input" from "returns this number". """ + for size in (7, 13): + recs = [_rec_body(f"a.s{i}", 10) for i in range(size)] + assert dv.ceiling_calibration(recs, 60).n == size, f"n must count the {size} keyed records given" + recs = [_rec_body(f"a.s{i}", 10) for i in range(9)] + [_rec_body("b.long", 500)] keyless = _rec(key=None, heading="Records formerly in this file", body="\n".join("x" for _ in range(500))) - - assert dv.ceiling_calibration(recs, 60).n == 10, "n must count every keyed record" assert dv.ceiling_calibration(recs + [keyless], 60) == dv.ceiling_calibration(recs, 60) From 0ff9671393eb185ab0bac8d08bcd6236c1433eed Mon Sep 17 00:00:00 2001 From: Timothy Date: Tue, 4 Aug 2026 22:52:03 +0200 Subject: [PATCH 8/9] fix(688): pin the minority band's constants and inclusivity; two prose corrections MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review round 7. Its adjudication of the round-6 dispute went the branch's way — measured at `f394d6ce`, the sha the record actually cites, the #620-era distribution is n=167, min 2, median 26, p90 52. Round 6 had measured `fefd11dff` (p90 57), a different tree. The number stays as written. BLOCKING FINDING: the 2%/25% constants and their inclusive boundaries were not pinned at all. Mutating 0.02 -> 0.03, 0.25 -> 0.30, or either `<=` to `<` passed all eight calibration tests. Those are not free parameters — they ARE the documented CI-red thresholds, so a silent shift would quietly falsify the 38/718 figures in docs.corpus-size-signal and docs/ci-cd.md (a strict cap reds after 37 long additions, a strict floor after 717 short ones). test_the_minority_band_BOUNDARIES_are_exactly_where_documented pins all four. It uses 100-record fixtures so k over the ceiling IS k%, and both 2/100 and 25/100 are exactly representable and compare equal to the constants — true boundary cases, not near-misses. Verified by mutation: all four now fail it. PROSE - corpus-size-signal said what stays blocking is "what routine growth cannot break", immediately before explaining that 38 routine additions break it. Now "what no SINGLE ordinary addition can break", which is what is actually true. - docs/ci-cd.md said the fine claim is "never asserted"; it is never asserted AGAINST THE LIVE CORPUS, and IS asserted on synthetic distributions the tests own. Corrected — the distinction is the whole design. Correction to an earlier commit message in this branch (9d2b30dc3, already pushed, so recorded here rather than rewritten): it said the cross-check was clean on "all 183 real records". 183 is the active keyed-record count; the cross-check scans the record WINGS — 195 files at that commit, 190 of them carrying frontmatter. The check was clean; the figure named the wrong population. Verification: 432 scripts/tests pass; ruff at baseline parity (47); validator exits 0 with no drift notice; corpus at p90=60, 18/183, calibrated; new record still 60 lines. Decisions-Edit: yes Co-Authored-By: Claude Opus 5 (1M context) --- docs/ci-cd.md | 5 +-- .../records/docs/corpus-size-signal.md | 5 +-- scripts/tests/test_decisions_validate.py | 31 +++++++++++++++++++ 3 files changed, 37 insertions(+), 4 deletions(-) diff --git a/docs/ci-cd.md b/docs/ci-cd.md index 357f1af20..f350d6c77 100644 --- a/docs/ci-cd.md +++ b/docs/ci-cd.md @@ -72,8 +72,9 @@ step is now continuous. The release boundary is instead where you: consolidating 15 of the 18 offenders away. The floor is a fraction rather than "at least one record", which would accept any ceiling up to 229 on the live corpus; as a fraction the accepted range is 43..180. - - **reported, never asserted** — the fine claim that the ceiling sits between the **90th and - 95th percentile**, i.e. at the tail boundary. `main()` prints a `::notice::` when it drifts. + - **reported, never asserted against the LIVE corpus** — the fine claim that the ceiling sits + between the **90th and 95th percentile**, i.e. at the tail boundary. `main()` prints a + `::notice::` when it drifts; the tests assert it only on distributions they own. It is an order statistic over a sparse distribution, so a single new record could move p90 by 21 lines and red the blocking job for whoever wrote it; a ceiling going out of date is the passage of corpus growth, not a defect in the commit under test, so it is treated like diff --git a/docs/decisions/records/docs/corpus-size-signal.md b/docs/decisions/records/docs/corpus-size-signal.md index da91b5123..650a7d0b2 100644 --- a/docs/decisions/records/docs/corpus-size-signal.md +++ b/docs/decisions/records/docs/corpus-size-signal.md @@ -65,8 +65,9 @@ all asserted, in the blocking job, a property of a corpus the commit under test The fine claim is genuinely useful and genuinely fragile, so it is now measured on every run and printed as a `::notice::` — the same treatment `stale_records` gets, and for the same stated reason: a constant going out of date is the passage of time, not a defect in this change. What stays -blocking is only what routine growth cannot break — each record moves a fraction by at most 1/N, so -from **18/183** over the ceiling it takes **38** consecutive over-ceiling additions to BREACH the +blocking is only what no SINGLE ordinary addition can break — each record moves a fraction by at +most 1/N, so from **18/183** over the ceiling it takes **38** consecutive over-ceiling additions to +BREACH the 25% cap (37 lands exactly on 0.25, which still passes), against **one** record to break v4. **The floor is a fraction, not `> 0`, and review is why.** The first draft of v5 asserted only diff --git a/scripts/tests/test_decisions_validate.py b/scripts/tests/test_decisions_validate.py index 917e952a0..6ce7d4833 100644 --- a/scripts/tests/test_decisions_validate.py +++ b/scripts/tests/test_decisions_validate.py @@ -1110,6 +1110,37 @@ def test_ceiling_calibration_is_empty_safe(): assert cal.n == 0 and not cal.marks_tail and not cal.flags_minority +def test_the_minority_band_BOUNDARIES_are_exactly_where_documented(): + """Pins both constants AND both inclusivities, which review found entirely unmutated. + + Mutating `0.02 -> 0.03`, `0.25 -> 0.30`, or either `<=` to `<` passed all eight calibration + tests. These are not free parameters — they ARE the documented CI-red thresholds, so a silent + shift changes them (a strict cap reds after 37 long additions instead of 38; a strict floor + after 717 short ones instead of 718), quietly falsifying the numbers in `docs.corpus-size-signal` + and `docs/ci-cd.md`. + + 100-record fixtures make the fraction exact and readable: k over the ceiling IS k%. Both + `2/100` and `25/100` are exactly representable and compare equal to the module constants, so + these are true boundary cases rather than near-misses. + """ + + def corpus(n_over: int, total: int = 100): + return [_rec_body(f"a.o{i}", 61) for i in range(n_over)] + [ + _rec_body(f"b.u{i}", 10) for i in range(total - n_over) + ] + + # The bounds are INCLUSIVE — exactly on either edge still passes. + assert dv.ceiling_calibration(corpus(2), 60).flags_minority, "the 2% floor must be inclusive" + assert dv.ceiling_calibration(corpus(25), 60).flags_minority, "the 25% cap must be inclusive" + + # ...and one record beyond either edge does not. + assert not dv.ceiling_calibration(corpus(1), 60).flags_minority, "1% is below the floor" + assert not dv.ceiling_calibration(corpus(26), 60).flags_minority, "26% is above the cap" + + # The constants themselves, so a change has to be deliberate and visible in the diff. + assert (dv.CEILING_MINORITY_MIN, dv.CEILING_MINORITY_MAX) == (0.02, 0.25) + + # `test_adding_ordinary_records_cannot_RED_the_blocking_property` used to live here. It appended two # long synthetic records to the LIVE corpus and asserted `flags_minority` on the result — which # crosses the 25% cap TWO records before the production bound does (56/221 vs 54/219), making the From efc34a3481454fec23d44c180f79eab7f076d38c Mon Sep 17 00:00:00 2001 From: Timothy Date: Tue, 4 Aug 2026 23:05:22 +0200 Subject: [PATCH 9/9] fix(688): pin p95's inclusivity; drop a stale ratio and hedge the gap width MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round 7's second reviewer returned MERGEABLE on the previous head after re-measuring every figure and running a 48-mutant battery — and reported ZERO wrong or unverified numbers, which ends this branch's five-commit streak of them. It also independently confirmed the round-6 adjudication: at `f394d6ce`, the sha the record cites, the #620-era distribution really is n=167, min 2, median 26, p90 52, next value 83. All five figures correct as written. This commit clears its four non-blocking items. - `marks_tail`'s UPPER inclusivity was the last meaningful surviving mutant: `ceiling <= p95` mutated to `<` survived the whole suite. Notice-only rather than blocking, but an unpinned boundary is how a documented claim quietly stops being true — the same defect the previous commit fixed for the coarse band. Both ends now pinned; verified the mutant fails. - "the largest by ~1.6x" was TRUE at `f394d6ce` (230/147 = 1.56) and is stale today (230/198 = 1.16). Unlike the consolidation table two paragraphs down, that sentence was never scoped to a sha — so rather than re-pin a number that will rot again, it now just says "the longest", which stays true however the tail moves. - The validator docstring asserted the 60->81 gap flatly; a 70-line record existed as recently as `8f6d4f443^`, so the gap's WIDTH is more volatile than that implied. Hedged to say it is the shape as measured today, not a constant. Nothing asserts it either way. - Rewrapped a mid-sentence line break left by the previous commit. Three surviving mutants are accepted and left: the crosscheck's not-a-mapping branch is unreachable from any fixture, the None -> "" normalisation only matters for an explicit YAML null no record has, and `_frontmatter_block` returning "" instead of None is a downstream no-op. Verification: 432 scripts/tests pass; ruff at baseline parity (47, and `ruff format --check` at parity 9/9); validator exits 0 with no drift notice; corpus at p90=60, 18/183, calibrated. Decisions-Edit: yes Co-Authored-By: Claude Opus 5 (1M context) --- docs/decisions/records/docs/corpus-size-signal.md | 5 ++--- scripts/decisions_validate.py | 5 +++-- scripts/tests/test_decisions_validate.py | 7 +++++++ 3 files changed, 12 insertions(+), 5 deletions(-) diff --git a/docs/decisions/records/docs/corpus-size-signal.md b/docs/decisions/records/docs/corpus-size-signal.md index 650a7d0b2..711fadb6b 100644 --- a/docs/decisions/records/docs/corpus-size-signal.md +++ b/docs/decisions/records/docs/corpus-size-signal.md @@ -67,8 +67,7 @@ printed as a `::notice::` — the same treatment `stale_records` gets, and for t a constant going out of date is the passage of time, not a defect in this change. What stays blocking is only what no SINGLE ordinary addition can break — each record moves a fraction by at most 1/N, so from **18/183** over the ceiling it takes **38** consecutive over-ceiling additions to -BREACH the -25% cap (37 lands exactly on 0.25, which still passes), against **one** record to break v4. +BREACH the 25% cap (37 lands exactly on 0.25, which still passes), against **one** record to break v4. **The floor is a fraction, not `> 0`, and review is why.** The first draft of v5 asserted only `0 < fraction_over < 1/3`, which measured against the real corpus accepted **every ceiling from 39 @@ -99,7 +98,7 @@ is restated as a self-referential fact** for the same reason: the warning report numbers on every run, and a number frozen in prose is one edit away from being a lie. **Size is a proxy for the thing we actually care about, and the proxy is demonstrably wrong.** Of -the records over the ceiling, the largest by ~1.6x — +the records over the ceiling, the longest — `scan.libraryfolder-unique-identity`, 230 lines — is a dozen-odd **distinct** hard-won traps (MySQL `utf8mb4_bin` PAD SPACE, create-the-composite-index-before-dropping-its-predecessor, clearing the connection pool per MySQL fixture, lazy hash healing that must never abort a scan…). Shortening it diff --git a/scripts/decisions_validate.py b/scripts/decisions_validate.py index fb507618f..8ed4b1bc5 100644 --- a/scripts/decisions_validate.py +++ b/scripts/decisions_validate.py @@ -769,8 +769,9 @@ def ceiling_calibration(records, ceiling: int) -> CeilingCalibration: `marks_tail` — `p90 <= ceiling <= p95`. Correct as a definition of "start of the tail", but an order statistic over a SPARSE distribution is a STEP function: the lengths climb to the ceiling - and then jump STRAIGHT to 81 with nothing in between, so ONE new record could move p90 by 21 - lines and the only remedy the assertion admitted was to raise the ceiling. It is real signal, + and then jump straight to 81 with nothing in between AS MEASURED TODAY (the gap's width moves + with the corpus — this is the shape, not a constant), so ONE new record could move p90 by 21 + lines, and the only remedy the assertion admitted was to raise the ceiling. It is real signal, but it is signal about the CONSTANT drifting, not a defect in the commit under test — the same shape as `stale_records`, and it is reported the same way: a notice, never a failure. diff --git a/scripts/tests/test_decisions_validate.py b/scripts/tests/test_decisions_validate.py index 6ce7d4833..8cf60d6f2 100644 --- a/scripts/tests/test_decisions_validate.py +++ b/scripts/tests/test_decisions_validate.py @@ -1064,6 +1064,13 @@ def test_ceiling_calibration_detects_drift_in_BOTH_directions(): assert not dv.ceiling_calibration(recs, 10).marks_tail, "below p90 it cuts into the bulk" assert not dv.ceiling_calibration(recs, 999).marks_tail, "above p95 it is parked among outliers" + # BOTH ends of `marks_tail` are inclusive. Review found the upper one unpinned — `ceiling <= p95` + # mutated to `<` survived the whole suite. It is notice-only rather than blocking, but an + # unpinned boundary is how a documented claim quietly stops being true. + assert dv.ceiling_calibration(recs, 20).marks_tail, "p90 itself must satisfy the lower bound" + assert dv.ceiling_calibration(recs, 200).marks_tail, "p95 itself must satisfy the upper bound" + assert not dv.ceiling_calibration(recs, 201).marks_tail, "one line above p95 must not" + # and the coarse property separates the same two failure modes assert not dv.ceiling_calibration(recs, 999).flags_minority, "a ceiling nobody is over signals nothing" assert not dv.ceiling_calibration(recs, 10).flags_minority, "100% over the ceiling is not a tail"