fix(621): make an unparseable decision record loud instead of silently invisible #641

Merged

timothy merged 5 commits from fix/621-record-parse-guard into main 2026-07-26 13:23:13 +02:00

Conversation 1 Commits 5 Files Changed 5

+458 -1

timothy commented 2026-07-26 01:10:38 +02:00

Owner

Copy Link

Copy Source

fixes #621

The defect, reproduced end-to-end

A file under the record wings that the dependency-free frontmatter reader cannot parse returns [] and simply vanishes from the corpus. Reproduced on main with a YAML block scalar (rule: >-):

parse_file            -> []
in all_active_records -> False
decisions_validate.py -> decisions-validate: OK        (exit 0)
build_catalog --check -> catalog up to date            (exit 0)

No error, no warning, record absent. This 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. #609's marker printing OK while doing nothing, #603's stale-after silently never firing).

Not a pre-existing-only hole: under PyYAML that same file parsed fine, so #610's dependency-free reader widened it.

What was already safe

An existing record disappearing stays loud — the no-vanish diff check catches it. The gap is a newly added record, which that check structurally cannot see because there is no base state to diff against, so the author's own PR looks clean. That asymmetry is why the fix is path-driven rather than diff-driven.

Fix

record_wing_files / record_wing_faults in decisions_validate.py: every *.md under docs/decisions/records/** and docs/decisions/archive/<area>/ must parse to exactly one keyed record. Surfaced through validate() as errors (a file in the record wings that is not a record is a mistake by definition), reported first — a file that failed to parse is absent from records, so every downstream check is otherwise silently evaluating an incomplete corpus.

Per-path rather than per-construct on purpose: enumerating the YAML the reader rejects would need redoing every time it meets something new. Asserting "this path yields exactly one keyed record" turns any present or future reader limitation from silent to loud in one move.

Exempt: the five top-level docs/decisions/archive/*.md stripped legacy files. Each parses to a single keyless pseudo-record from its generated "Records formerly in this file" index — indexes, not records, and what keeps older date-based pointers resolvable.

Block scalars: decided NO (Done-when box 5)

Rationale in docs.record-wing-parse-guard. Correct folding is a real subset of YAML (> vs |, strip/clip/keep chomping, indentation-relative continuation); a subtly wrong folder would silently alter rule text — the very failure class this closes — while test_frontmatter_reader_matches_pyyaml kept passing on the flat records that make up the whole corpus. With the structural check in place the cost of not supporting them is a loud error naming the fix, and all 167 records already use single quoted lines.

Verification

• 8 tests added, mutation-verified: with record_wing_faults neutered, 5 go red; restored, 119 pass.
• Controls run against the real corpus: block scalar → red, unterminated frontmatter → red, stray note file → red, keyless record → red, clean corpus → OK, stripped legacy archive files → still exempt.
• test_real_repo_record_wings_are_all_parseable asserts the wings are non-empty (>100 files) so a clean result can never be vacuous.

Independent-review note (process.independent-review-rubric): Python + docs, no C#/locks/auth/API-write-path/migrations — below every trigger; reviewed cold against the diff in-session.

fixes #621 ## The defect, reproduced end-to-end A file under the record wings that the dependency-free frontmatter reader cannot parse returns `[]` and simply **vanishes** from the corpus. Reproduced on `main` with a YAML block scalar (`rule: >-`): ``` parse_file -> [] in all_active_records -> False decisions_validate.py -> decisions-validate: OK (exit 0) build_catalog --check -> catalog up to date (exit 0) ``` No error, no warning, record absent. This 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. #609's marker printing `OK` while doing nothing, #603's `stale-after` silently never firing). **Not a pre-existing-only hole:** under PyYAML that same file parsed fine, so #610's dependency-free reader *widened* it. ## What was already safe An *existing* record disappearing stays loud — the no-vanish diff check catches it. The gap is a **newly added** record, which that check structurally cannot see because there is no base state to diff against, so the author's own PR looks clean. That asymmetry is why the fix is path-driven rather than diff-driven. ## Fix `record_wing_files` / `record_wing_faults` in `decisions_validate.py`: every `*.md` under `docs/decisions/records/**` and `docs/decisions/archive/<area>/` must parse to **exactly one keyed record**. Surfaced through `validate()` as **errors** (a file in the record wings that is not a record is a mistake by definition), reported first — a file that failed to parse is absent from `records`, so every downstream check is otherwise silently evaluating an incomplete corpus. Per-path rather than per-construct on purpose: enumerating the YAML the reader rejects would need redoing every time it meets something new. Asserting "this path yields exactly one keyed record" turns **any** present or future reader limitation from silent to loud in one move. **Exempt:** the five top-level `docs/decisions/archive/*.md` stripped legacy files. Each parses to a single *keyless* pseudo-record from its generated "Records formerly in this file" index — indexes, not records, and what keeps older date-based pointers resolvable. ## Block scalars: decided NO (Done-when box 5) Rationale in `docs.record-wing-parse-guard`. Correct folding is a real subset of YAML (`>` vs `|`, strip/clip/keep chomping, indentation-relative continuation); a subtly wrong folder would silently *alter rule text* — the very failure class this closes — while `test_frontmatter_reader_matches_pyyaml` kept passing on the flat records that make up the whole corpus. With the structural check in place the cost of not supporting them is a loud error naming the fix, and all 167 records already use single quoted lines. ## Verification - 8 tests added, **mutation-verified**: with `record_wing_faults` neutered, 5 go red; restored, 119 pass. - Controls run against the real corpus: block scalar → red, unterminated frontmatter → red, stray note file → red, keyless record → red, clean corpus → OK, stripped legacy archive files → still exempt. - `test_real_repo_record_wings_are_all_parseable` asserts the wings are non-empty (>100 files) so a clean result can never be vacuous. Independent-review note (`process.independent-review-rubric`): Python + docs, no C#/locks/auth/API-write-path/migrations — below every trigger; reviewed cold against the diff in-session.

timothy force-pushed fix/621-record-parse-guard from ebc4e51edd to 3d720a6bc1 2026-07-26 12:04:41 +02:00 Compare

timothy added 1 commit 2026-07-26 12:05:14 +02:00

docs(621): state the mutation count precisely — it depends on which mutation ... 9949703585

The record said "10 tests go red"; on the rebased base a full short-circuit reddens 11.
Both numbers are real and measure different mutations: neutering only the scan loop
leaves the empty-wing check live (10), while returning [] from the whole function
disables that too (11). The reviewer flagged the phrasing as ambiguous before the count
also drifted.

Now states the mutation alongside the number, since a bare count is exactly the kind of
frozen figure this session has watched rot four times.

Refs #621

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

timothy commented 2026-07-26 13:21:41 +02:00

Author

Owner

Copy Link

Copy Source

Review-verdict: MERGEABLE @ 9949703

Round-3 independent cold review returned MERGEABLE: defect reproduced on main, fix mutation-verified end-to-end, all factual claims in the record reproduce exactly (168 record files, 117 unquoted / 51 quoted rule values), basename hole closed and pinned. Round-2 findings (untested main() wiring, basename escape, archive exemption by location, parse-to-WRONG junk keys, empty wing reporting clean) all fixed and mutation-verified. One review finding deliberately DECLINED with reason: empty-wing symmetry across both wings would fail a correct young corpus, since git tracks no empty archive dir; asymmetry documented in code and pinned by a test. 194 tests green; verified under both jq 1.8.2 and jq 1.6.

Review-verdict: MERGEABLE @ 9949703 Round-3 independent cold review returned MERGEABLE: defect reproduced on main, fix mutation-verified end-to-end, all factual claims in the record reproduce exactly (168 record files, 117 unquoted / 51 quoted rule values), basename hole closed and pinned. Round-2 findings (untested main() wiring, basename escape, archive exemption by location, parse-to-WRONG junk keys, empty wing reporting clean) all fixed and mutation-verified. One review finding deliberately DECLINED with reason: empty-wing symmetry across both wings would fail a correct young corpus, since git tracks no empty archive dir; asymmetry documented in code and pinned by a test. 194 tests green; verified under both jq 1.8.2 and jq 1.6.

timothy scheduled this pull request to auto merge when all checks succeed 2026-07-26 13:23:11 +02:00

timothy merged commit eb339084f9 into main 2026-07-26 13:23:13 +02:00

timothy deleted branch fix/621-record-parse-guard 2026-07-26 13:23:14 +02:00

timothy referenced this issue from a commit 2026-07-26 13:23:15 +02:00

Merge pull request 'fix(621): make an unparseable decision record loud instead of silently invisible' (#641) from fix/621-record-parse-guard into main

timothy referenced this pull request 2026-07-26 13:23:58 +02:00

decisions: a record file that fails to parse is silently invisible — no structural "one keyed record per file" check #621

Sign in to join this conversation.

Reviewers

No Reviewers

Labels

Clear labels

ad-hoc

One-off / ad-hoc work not tracked by a dedicated issue

api

REST API / HTTP endpoints

bug

Something isn't working

ci-cd

Build, test, deploy pipeline

content

Channel content / schedules / playlists

dependencies

Dependency updates (Renovate)

enhancement

New feature or improvement

frontend

ChicoryTV React SPA frontend

in-progress

Claimed by an active session — do not pick up

jellyfin

Jellyfin tuner / IPTV integration

parked

Excluded from automatic queue pickup; work only when explicitly selected

priority: high

priority: low

priority: medium

review

Adversarial review finding

security

Security / vulnerability fix

No labels

Milestone

No items

No Milestone

Projects

Clear projects

No projects

Assignees

Clear assignees

renovate timothy (Timothy)

No Assignees

1 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: timothy/ersatztv#641