Merge pull request 'fix(674,688): cross-check decision frontmatter against PyYAML; split the ceiling calibration claim' (#725) from fix/674-688-decisions-validator into main
Build ErsatzTV Image / API docs in sync (OpenAPI + endpoint index) (push) Has been skipped
Build ErsatzTV Image / Formatting (changed .cs conform to .editorconfig) (push) Has been skipped
Build ErsatzTV Image / Functional E2E (curl + UI contracts) (push) Successful in 16m55s
Build ErsatzTV Image / Build & test (.NET) (push) Successful in 22m18s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (push) Successful in 22m23s
Build ErsatzTV Image / Build & push image (amd64) (push) Successful in 11m16s

This commit was merged in pull request #725.
This commit is contained in:
2026-08-04 21:43:10 +00:00
7 changed files with 847 additions and 55 deletions
+24 -5
View File
@@ -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,24 @@ step is now continuous. The release boundary is instead where you:
- a **per-record prose ceiling** (`decisions_validate.py --record-ceiling <n>`, 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 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 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
live corpus; as a fraction the accepted range is 43..180.
- **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
`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
+2 -1
View File
@@ -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 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/<area>/<topic>.md` (archived ones at `docs/decisions/archive/<area>/<topic>.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) |
@@ -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 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"'
---
@@ -46,19 +46,49 @@ 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 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 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 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
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 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
`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 **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.
**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 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.
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
@@ -68,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
@@ -0,0 +1,72 @@
---
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` 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. 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. 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, 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
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 on the definition.
**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
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
`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.
+211 -2
View File
@@ -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,18 @@ _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 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:
"""`stale-after` as a date, or None if absent, empty, or malformed.
@@ -541,6 +554,102 @@ 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 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 ({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:
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()}
# `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 — "
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:
@@ -629,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
@@ -642,6 +751,79 @@ 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: MINORITY_MIN <= fraction_over <= 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: the lengths climb to the ceiling
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.
`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. 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`.
The THIRD arm is the tightest and is stated here because it is the easy one to forget:
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.
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
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=CEILING_MINORITY_MIN <= frac <= CEILING_MINORITY_MAX,
)
def _catalog_ok() -> bool:
try:
import scripts.build_decisions_catalog as bc # pyright: ignore[reportMissingImports]
@@ -684,6 +866,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 +887,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 +918,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)
+11 -14
View File
@@ -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)} frontmatter fault(s) vs PyYAML:\n" + "\n".join(faults[:5])
+488 -24
View File
@@ -1002,39 +1002,423 @@ 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. The lengths climb to the ceiling and then
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
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 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
`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"
# 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
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"
# 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"
assert dv.ceiling_calibration(recs, 60).flags_minority
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 — 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
# 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(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(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.
# 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) > 20, "corpus looks empty — this check would be vacuous"
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)
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
# 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.
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.
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 + [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.
`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.
"""
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
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)]
)
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():
"""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 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 = (
[_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 ------------------------------
_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_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_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")
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_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 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"
)
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.
@@ -1046,24 +1430,104 @@ 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): 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 — 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
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_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]
# 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
# 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"
)
expectations.append(expected)
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):
"""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"
# 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. 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 -1 every record is over it — the warning branch must fire"
)
def test_trend_notice_reports_record_prose_and_scaffolding_separately(capsys):