fix(674,688): cross-check frontmatter against PyYAML; split the ceiling calibration claim
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) <noreply@anthropic.com>
This commit is contained in:
Claude Opus 5
+17
-5
@@ -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 <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 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
|
||||
|
||||
@@ -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/<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 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
|
||||
|
||||
@@ -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.
|
||||
@@ -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)
|
||||
|
||||
@@ -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])
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user