fix(621): make an unparseable decision record loud instead of silently invisible
A file under the record wings that the dependency-free frontmatter reader cannot parse returned [] and simply vanished from the corpus: decisions_validate.py printed OK, build_decisions_catalog.py --check said "up to date", the record was absent, and nothing anywhere errored. Reproduced end-to-end with a YAML block scalar (`rule: >-`), which is the natural thing to reach for on this corpus's very long rule: values and which parsed fine under PyYAML before #610's dependency-free reader. An EXISTING record disappearing was already loud (the no-vanish diff check). The hole was a NEWLY ADDED record, which that check structurally cannot see — no base state to diff against — so the author's own PR looks clean. Hence a per-PATH check, not a per-construct or diff-driven one: asserting "this path yields exactly one keyed record" turns any present or future reader limitation from silent to loud in one move. Adds record_wing_files/record_wing_faults to decisions_validate.py, surfaced through validate() as ERRORS (a file in the record wings that is not a record is a mistake by definition) and reported first, since a file that failed to parse leaves every downstream check silently evaluating an incomplete corpus. The five top-level stripped legacy archive files are exempt — they are generated "Records formerly in this file" indexes, keyless by construction, and are what keeps older date-based pointers resolvable. _read_frontmatter is deliberately NOT extended to accept block scalars; rationale in the new docs.record-wing-parse-guard record. 8 tests, mutation-verified: with the check neutered 5 go red, restored all 119 pass. A live-corpus positive control asserts the wings are non-empty so a clean result can never be vacuous. Refs #621
This commit is contained in:
+5
-1
@@ -579,7 +579,11 @@ Enforces decision-record lifecycle invariants (ersatztv#521, supersedes the ersa
|
||||
append-only mechanic): well-formed 5-field metadata, exactly one `active` record per `key`,
|
||||
reciprocal `supersedes`/`superseded-by` links, no record vanishing from the active set without an
|
||||
archive copy, no rationale-prose rewrite without a `Decisions-Edit: yes` trailer on a non-merge commit
|
||||
in the range (ersatztv#609),
|
||||
in the range (ersatztv#609), a structural per-path check that every `*.md` under
|
||||
`docs/decisions/records/**` and `docs/decisions/archive/<area>/` parses to **exactly one keyed
|
||||
record** (ersatztv#621 — without it, a file the dependency-free frontmatter reader cannot parse,
|
||||
such as one using a YAML block scalar, yields `[]` and vanishes from the corpus with every check
|
||||
still reporting green; the top-level stripped legacy archive indexes are exempt),
|
||||
and the generated active catalog (`docs/decisions/README.md`) in sync with source. Two steps:
|
||||
`scripts/decisions_validate.py --base origin/<base> --head HEAD` (the merge-base diff checks, which
|
||||
need a base/head range — CI-only) and `scripts/build_decisions_catalog.py --check` (catalog drift).
|
||||
|
||||
@@ -66,6 +66,7 @@ the link for rationale. Superseded/retired history lives in `archive/`. Regenera
|
||||
| `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.record-wing-parse-guard` | `decisions_validate.py` asserts, per PATH, that every `*.md` under `docs/decisions/records/**` and `docs/decisions/archive/<area>/*.md` 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. The top-level `docs/decisions/archive/*.md` stripped legacy files and `_NON_DECISION_FILES` are exempt (they are generated indexes, not records). `_read_frontmatter` is deliberately NOT extended to accept YAML block scalars: every record value goes on ONE quoted 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) |
|
||||
| `ffmpeg.hls-cold-start-burst` | HLS cold-start latency is fixed with a bounded `-readrate_initial_burst` (gated on FFmpeg ≥6.1 capability detection), not by raising `work_ahead_limit`, which would remove the concurrency guarantee it exists for. | 2026-07-20 | [link](records/ffmpeg/hls-cold-start-burst.md) |
|
||||
|
||||
@@ -0,0 +1,50 @@
|
||||
---
|
||||
key: docs.record-wing-parse-guard
|
||||
title: '2026-07-26 — Every file in the record wings must parse to exactly one keyed record; the frontmatter reader stays flat-scalar-only (#621)'
|
||||
status: active
|
||||
since: '2026-07-26'
|
||||
supersedes: none
|
||||
superseded-by: none
|
||||
rule: '`decisions_validate.py` asserts, per PATH, that every `*.md` under `docs/decisions/records/**` and `docs/decisions/archive/<area>/*.md` 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. The top-level `docs/decisions/archive/*.md` stripped legacy files and `_NON_DECISION_FILES` are exempt (they are generated indexes, not records). `_read_frontmatter` is deliberately NOT extended to accept YAML block scalars: every record value goes on ONE quoted line, and the structural check is what makes that limitation loud instead of silent.'
|
||||
signals: 'record silently invisible, parsed to 0 records, block scalar rule >- , unterminated frontmatter, file vanishes from corpus, validator OK but record absent, catalog up to date but missing, one keyed record per file, stripped legacy archive index exempt · paths: `scripts/decisions_validate.py`, `scripts/decisions_lib.py`, `scripts/tests/test_decisions_validate.py` · issues: #621, #610, #609, #603'
|
||||
mechanics: '`scripts/decisions_validate.py` -> `record_wing_files` / `record_wing_faults`; `docs/ci-cd.md` -> "`decisions-guard` job"'
|
||||
---
|
||||
|
||||
Found in adversarial review of #610 / PR #619. Nothing enforced that a file under the record wings
|
||||
is actually a record. A file the dependency-free frontmatter reader cannot parse returns `[]` and
|
||||
simply **vanishes** — `decisions_validate.py` prints OK, `build_decisions_catalog.py --check` says
|
||||
up to date, the record is absent from the corpus, and there is no error anywhere.
|
||||
|
||||
**This is the corpus's own failure mode turned on itself.** The whole point of the decision system is
|
||||
retrievability; the one thing worse than a missing record is a missing record that reports success.
|
||||
Same family as the #609 marker that printed `OK` while doing nothing and the #603 `stale-after` field
|
||||
that silently never fired.
|
||||
|
||||
**What was already safe, and what wasn't.** An *existing* record disappearing was always loud — the
|
||||
no-vanish diff check catches it. The hole is a **newly added** record, which the diff check
|
||||
structurally cannot see because there is no base state to compare against, so the author's own PR
|
||||
looks clean. That asymmetry is why the fix had to be path-driven rather than diff-driven.
|
||||
|
||||
**Why per-path rather than per-construct.** Enumerating the YAML the reader rejects would have to be
|
||||
re-done every time the reader meets something new. Asserting *"this path must yield exactly one keyed
|
||||
record"* converts **any** present or future reader limitation from silent to loud in one move —
|
||||
block scalars, indented structure, unterminated frontmatter, a stray note file, an empty file.
|
||||
|
||||
**The block-scalar decision: NO.** `rule: >-` is the natural thing to reach for on this corpus's very
|
||||
long `rule:` values, and under PyYAML it parsed fine — so #610's dependency-free reader *widened* a
|
||||
hole rather than creating it. Teaching `_read_frontmatter` to fold block scalars was still declined:
|
||||
|
||||
- Correct folding is a real subset of YAML (`>` vs `|`, strip/clip/keep chomping, indentation-relative
|
||||
continuation). A subtly wrong folder is *worse* than a rejection, because it would silently alter
|
||||
rule text — the exact class of failure this record exists to close — while
|
||||
`test_frontmatter_reader_matches_pyyaml` (which compares against PyYAML on real records) would keep
|
||||
passing on the flat records that make up the entire corpus.
|
||||
- The reader must stay dependency-free: it runs in CI's `decisions lifecycle` job, in the Husky
|
||||
pre-commit hook, and on every contributor's machine, none of which install anything.
|
||||
- With the structural check in place the cost of not supporting block scalars is a **loud error
|
||||
naming the fix**, and all 167 existing records already use single quoted lines. There is no
|
||||
value a flat scalar cannot hold.
|
||||
|
||||
Verified by mutation, not by a green tick: with `record_wing_faults` neutered, five of the new tests
|
||||
go red; restored, all pass. `test_real_repo_record_wings_are_all_parseable` additionally asserts the
|
||||
wings are non-empty (>100 files) so a clean result can never be vacuous.
|
||||
@@ -96,9 +96,15 @@ def validate(
|
||||
rewritten,
|
||||
archive_records=None,
|
||||
demoted=(),
|
||||
wing_faults=(),
|
||||
) -> list[str]:
|
||||
errs: list[str] = []
|
||||
archive_records = archive_records or []
|
||||
# #621: structural "one keyed record per file". ERRORS, not warnings — a file under the record
|
||||
# wings that is not a record is a mistake by definition. Reported first because a file that
|
||||
# failed to parse is absent from `records`, so every downstream check below is silently
|
||||
# evaluating an incomplete corpus.
|
||||
errs += list(wing_faults)
|
||||
decision_recs = [r for r in records if r.heading not in SKIP_HEADINGS and r.status != "legacy-unmigrated"]
|
||||
|
||||
active_by_key: dict[str, int] = {}
|
||||
@@ -406,6 +412,68 @@ def _diff_findings(base: str, head: str) -> tuple[list[str], list[str], list[str
|
||||
return sorted(set(removed)), sorted(set(rewritten)), sorted(set(demoted))
|
||||
|
||||
|
||||
def record_wing_files(records_dir: Path | None = None, archive_dir: Path | None = None) -> list[Path]:
|
||||
"""Every file that MUST be exactly one keyed record.
|
||||
|
||||
Scope is the split-layout record wings only:
|
||||
* `docs/decisions/records/**/*.md` — the active wing
|
||||
* `docs/decisions/archive/<area>/*.md` — the archived wing, nested by area since #610
|
||||
|
||||
Deliberately NOT included: `docs/decisions/archive/*.md` at the top level. Those are the five
|
||||
stripped legacy topic files left behind by the #610 split (`api.md`, `scan.md`, `spa.md`,
|
||||
`startup.md`, `release-ci-governance.md`); each parses to a single KEYLESS pseudo-record from its
|
||||
generated "Records formerly in this file" index, which is exactly what keeps older date-based
|
||||
pointers resolvable. They are indexes, not records. `_NON_DECISION_FILES` (README.md and
|
||||
friends) is filtered on top of that.
|
||||
"""
|
||||
records_dir = dl.RECORDS_DIR if records_dir is None else records_dir
|
||||
archive_dir = dl.ARCHIVE_DIR if archive_dir is None else archive_dir
|
||||
files: list[Path] = []
|
||||
if records_dir.exists():
|
||||
files += sorted(records_dir.rglob("*.md"))
|
||||
if archive_dir.exists():
|
||||
files += sorted(archive_dir.glob("*/*.md")) # area-nested only — see docstring
|
||||
return [p for p in files if p.name not in dl._NON_DECISION_FILES]
|
||||
|
||||
|
||||
def record_wing_faults(records_dir: Path | None = None, archive_dir: Path | None = None) -> list[str]:
|
||||
"""Structural check (#621): every file in the record wings parses to exactly ONE keyed record.
|
||||
|
||||
Without this, a file the frontmatter reader cannot parse yields `[]` and simply VANISHES — no
|
||||
error, no warning, validator OK, catalog "up to date", record absent from the corpus. That is
|
||||
the corpus's own failure mode turned on itself: the one thing worse than a missing record is a
|
||||
missing record that reports success (cf. the #609 marker that printed OK while doing nothing and
|
||||
the #603 `stale-after` that silently never fired).
|
||||
|
||||
An EXISTING record disappearing was already loud — the no-vanish diff check catches it. This
|
||||
closes the case the diff check structurally cannot see: a NEWLY ADDED record, where the author's
|
||||
own PR looks clean because there is no prior state to diff against.
|
||||
|
||||
Making it path-driven rather than record-driven is the point: it converts ANY present or future
|
||||
reader limitation from silent to loud, instead of enumerating the constructs we happen to know
|
||||
about today.
|
||||
"""
|
||||
faults: list[str] = []
|
||||
for p in record_wing_files(records_dir, archive_dir):
|
||||
try:
|
||||
recs = dl.parse_file(p)
|
||||
except Exception as exc: # unreadable/undecodable file is a fault, not a crash
|
||||
faults.append(f"{p}: could not be read as a decision record ({exc})")
|
||||
continue
|
||||
if len(recs) != 1:
|
||||
faults.append(
|
||||
f"{p}: parsed to {len(recs)} records, expected exactly 1 — a file under the record "
|
||||
f"wings must be one record. Common cause: YAML the dependency-free frontmatter "
|
||||
f"reader does not accept (a block scalar such as `rule: >-` or `rule: |`, any "
|
||||
f"indented/nested structure, or unterminated `---` frontmatter). Put the value on "
|
||||
f"ONE quoted line, as all other records do."
|
||||
)
|
||||
continue
|
||||
if not recs[0].key:
|
||||
faults.append(f"{p}: parsed to a record with no `key` — every record in the wings must be keyed.")
|
||||
return faults
|
||||
|
||||
|
||||
def _archive_keys() -> set[str]:
|
||||
keys: set[str] = set()
|
||||
if dl.ARCHIVE_DIR.exists():
|
||||
@@ -491,6 +559,7 @@ def main(argv=None) -> int:
|
||||
rewritten=rewritten,
|
||||
archive_records=archive_records,
|
||||
demoted=demoted,
|
||||
wing_faults=record_wing_faults(),
|
||||
)
|
||||
|
||||
if not budget_ok:
|
||||
|
||||
@@ -700,3 +700,121 @@ def test_retitling_a_record_is_not_reported_as_a_removal(tmp_path):
|
||||
assert removed == [], f"a retitle was reported as a removal: {removed}"
|
||||
assert rewritten == [], f"a retitle was reported as a prose rewrite: {rewritten}"
|
||||
assert demoted == [], f"a retitle was reported as a demotion: {demoted}"
|
||||
|
||||
|
||||
# --- #621: structural "one keyed record per file" over the record wings -----------------------
|
||||
#
|
||||
# The defect these cover: a file the dependency-free frontmatter reader cannot parse yields `[]`
|
||||
# and vanishes from the corpus with NO error anywhere — validator OK, catalog "up to date", record
|
||||
# absent. Only a NEWLY ADDED record is affected; an existing one disappearing is already caught by
|
||||
# the no-vanish diff check, which structurally cannot see a record that never existed in the base.
|
||||
|
||||
|
||||
def _wing(tmp_path) -> tuple[Path, Path]:
|
||||
"""A tmp record-wing pair: (records_dir, archive_dir)."""
|
||||
records = tmp_path / "docs" / "decisions" / "records"
|
||||
archive = tmp_path / "docs" / "decisions" / "archive"
|
||||
(records / "ci").mkdir(parents=True)
|
||||
(archive / "ci").mkdir(parents=True)
|
||||
return records, archive
|
||||
|
||||
|
||||
_GOOD = (
|
||||
"---\n"
|
||||
"key: ci.good\n"
|
||||
"title: '2026-01-01 — Good (#1)'\n"
|
||||
"status: active\n"
|
||||
"since: '2026-01-01'\n"
|
||||
"supersedes: none\n"
|
||||
"superseded-by: none\n"
|
||||
"rule: 'a rule on one quoted line'\n"
|
||||
"signals: 'concept · paths: a/b.py · issues: #1'\n"
|
||||
"mechanics: 'x'\n"
|
||||
"---\n\nRationale prose.\n"
|
||||
)
|
||||
|
||||
|
||||
def test_wing_faults_clean_corpus_is_silent(tmp_path):
|
||||
records, archive = _wing(tmp_path)
|
||||
(records / "ci" / "good.md").write_text(_GOOD)
|
||||
assert dv.record_wing_faults(records, archive) == []
|
||||
|
||||
|
||||
def test_wing_faults_block_scalar_record_fails_loudly(tmp_path):
|
||||
"""The exact reproduction from #621: a YAML block scalar — the natural thing to reach for on
|
||||
this corpus's very long `rule:` values — makes the whole record silently invisible."""
|
||||
records, archive = _wing(tmp_path)
|
||||
bad = records / "ci" / "blockscalar.md"
|
||||
bad.write_text(_GOOD.replace("rule: 'a rule on one quoted line'\n", "rule: >-\n a long rule wrapped\n over two lines\n"))
|
||||
|
||||
# Precondition: this really is the silent-vanish case, not some other parse error.
|
||||
assert dl.parse_file(bad) == [], "expected the reader to drop the record entirely"
|
||||
|
||||
faults = dv.record_wing_faults(records, archive)
|
||||
assert len(faults) == 1, faults
|
||||
assert "blockscalar.md" in faults[0]
|
||||
assert "expected exactly 1" in faults[0]
|
||||
|
||||
|
||||
def test_wing_faults_unterminated_frontmatter_fails_loudly(tmp_path):
|
||||
records, archive = _wing(tmp_path)
|
||||
bad = records / "ci" / "unterminated.md"
|
||||
bad.write_text("---\nkey: ci.unterminated\nstatus: active\n\nNo closing delimiter.\n")
|
||||
assert dl.parse_file(bad) == []
|
||||
|
||||
faults = dv.record_wing_faults(records, archive)
|
||||
assert len(faults) == 1, faults
|
||||
assert "unterminated.md" in faults[0]
|
||||
|
||||
|
||||
def test_wing_faults_stray_note_file_fails_loudly(tmp_path):
|
||||
"""A file under records/ that is not a record at all — no frontmatter whatsoever."""
|
||||
records, archive = _wing(tmp_path)
|
||||
(records / "ci" / "notes.md").write_text("# Scratch notes\n\nNot a record.\n")
|
||||
faults = dv.record_wing_faults(records, archive)
|
||||
assert len(faults) == 1, faults
|
||||
assert "notes.md" in faults[0]
|
||||
|
||||
|
||||
def test_wing_faults_keyless_record_fails_loudly(tmp_path):
|
||||
"""Parses to exactly one record, but carries no `key` — still a fault."""
|
||||
records, archive = _wing(tmp_path)
|
||||
(records / "ci" / "keyless.md").write_text(_GOOD.replace("key: ci.good\n", ""))
|
||||
faults = dv.record_wing_faults(records, archive)
|
||||
assert len(faults) == 1, faults
|
||||
assert "no `key`" in faults[0]
|
||||
|
||||
|
||||
def test_wing_faults_exempts_stripped_legacy_archive_files(tmp_path):
|
||||
"""The #610 split left five KEYLESS stripped topic files at the archive TOP level (api.md,
|
||||
scan.md, spa.md, startup.md, release-ci-governance.md) whose only content is a generated
|
||||
"Records formerly in this file" index — that index is what keeps older date-based pointers
|
||||
resolvable. They are indexes, not records, and must stay exempt. Their area-NESTED siblings
|
||||
are real records and are checked."""
|
||||
records, archive = _wing(tmp_path)
|
||||
(records / "ci" / "good.md").write_text(_GOOD)
|
||||
# a stripped legacy file at the archive top level: parses to a single keyless pseudo-record
|
||||
(archive / "api.md").write_text("# api\n\n## Records formerly in this file\n\n- `api.thing`\n")
|
||||
(archive / "README.md").write_text("# archive\n")
|
||||
assert dv.record_wing_faults(records, archive) == []
|
||||
|
||||
# ...but a real archived record nested under an area IS checked
|
||||
(archive / "ci" / "broken.md").write_text("# not a record\n")
|
||||
faults = dv.record_wing_faults(records, archive)
|
||||
assert len(faults) == 1 and "broken.md" in faults[0], faults
|
||||
|
||||
|
||||
def test_validate_surfaces_wing_faults_as_errors(tmp_path):
|
||||
"""Faults must arrive as validator ERRORS (exit 1), not warnings."""
|
||||
errs = _v([_rec(key="ci.a", source=Path("docs/decisions/records/ci/a.md"), heading="A")],
|
||||
wing_faults=["docs/decisions/records/ci/x.md: parsed to 0 records, expected exactly 1"])
|
||||
assert any("x.md" in e for e in errs), errs
|
||||
|
||||
|
||||
def test_real_repo_record_wings_are_all_parseable():
|
||||
"""Positive control against the LIVE corpus: every one of the real record files parses to
|
||||
exactly one keyed record. This is what makes the check's clean result meaningful rather than
|
||||
vacuous — if the wings were empty or unreadable, the checks above would pass trivially."""
|
||||
files = dv.record_wing_files()
|
||||
assert len(files) > 100, f"record wings look empty ({len(files)} files) — check is vacuous"
|
||||
assert dv.record_wing_faults() == []
|
||||
|
||||
Reference in New Issue
Block a user