fix(621): exempt by exact path, not basename — my "no basename exemption" claim was false
review-verdict/h10 Awaiting review verdict for 3d720a6
Review verdict / Set review-verdict status (pull_request) Successful in 2s
PR Gates / CI image pin matches docker/ci (pull_request) Successful in 14s
PR Gates / Docs update reminder (pull_request) Successful in 14s
PR Gates / decisions lifecycle (pull_request) Successful in 28s
Build ErsatzTV Image / Build & test (.NET) (pull_request) Successful in 8m48s
Build ErsatzTV Image / API docs in sync (OpenAPI + endpoint index) (pull_request) Successful in 9s
Build ErsatzTV Image / Formatting (changed .cs conform to .editorconfig) (pull_request) Successful in 7s
Build ErsatzTV Image / Functional E2E (curl + UI contracts) (pull_request) Successful in 17m57s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (pull_request) Successful in 22m0s
Build ErsatzTV Image / Build & push image (amd64) (pull_request) Has been skipped
review-verdict/h10 Awaiting review verdict for 3d720a6
Review verdict / Set review-verdict status (pull_request) Successful in 2s
PR Gates / CI image pin matches docker/ci (pull_request) Successful in 14s
PR Gates / Docs update reminder (pull_request) Successful in 14s
PR Gates / decisions lifecycle (pull_request) Successful in 28s
Build ErsatzTV Image / Build & test (.NET) (pull_request) Successful in 8m48s
Build ErsatzTV Image / API docs in sync (OpenAPI + endpoint index) (pull_request) Successful in 9s
Build ErsatzTV Image / Formatting (changed .cs conform to .editorconfig) (pull_request) Successful in 7s
Build ErsatzTV Image / Functional E2E (curl + UI contracts) (pull_request) Successful in 17m57s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (pull_request) Successful in 22m0s
Build ErsatzTV Image / Build & push image (amd64) (pull_request) Has been skipped
Re-review found the basename exemption was not actually removed: the code still dropped
any wing-root README.md, and the docstring defended that with "no such file exists
today; the catalog README lives in TOPIC_DIR". That is false — docs/decisions/archive/
README.md exists and parses to []. So the rule text in the record, the catalog row and
ci-cd.md all asserted something the shipped code contradicted.
It was also a live hole, not just wrong prose: dl.active_files() globs RECORDS_DIR.rglob
with NO filter, so a future docs/decisions/records/README.md would be a corpus source
while exempt from the guard — verbatim the hole this PR cites as the reason to stop
using _NON_DECISION_FILES, reintroduced one directory over.
Now exempt by exact RELATIVE PATH ({archive/README.md}), with a test pinning that the
same basename in the ACTIVE wing is still faulted. Mutation-verified.
DISAGREEING with one review finding, deliberately. It asked for the empty-wing
anti-vacuity check to be symmetric across both wings. The semantics differ: zero ACTIVE
records means the scan measured nothing and any clean result is vacuous, while zero
ARCHIVED records is a normal state — nothing superseded yet, true of any young repo and
every fresh clone before the first supersession. Making it symmetric fails a correct
corpus (it reddened 7 tests). The asymmetry stays, with the reason in the code and a
test pinning it so nobody "fixes" it back.
127 passed.
Refs #621
This commit is contained in:
+3
-2
@@ -584,8 +584,9 @@ in the range (ersatztv#609), a structural per-path check that every `*.md` under
|
||||
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; a file directly in `archive/` is exempt only when it really is a stripped index — one keyless
|
||||
record with a known generated heading — never merely by its location, and nothing is exempt by
|
||||
basename),
|
||||
record with a known generated heading — never merely by its location; the single further exemption,
|
||||
`archive/README.md`, is by exact relative path, never by basename, which would otherwise exempt the
|
||||
same filename in the active wing),
|
||||
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,7 +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/**` 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; nothing is exempt by BASENAME. `_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.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) |
|
||||
| `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) |
|
||||
|
||||
@@ -5,7 +5,7 @@ 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/**` 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; nothing is exempt by BASENAME. `_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.'
|
||||
rule: '`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.'
|
||||
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"'
|
||||
---
|
||||
|
||||
@@ -419,22 +419,27 @@ def record_wing_files(records_dir: Path | None = None, archive_dir: Path | None
|
||||
`archive/`, which are scanned but may qualify for the stripped-index exemption applied in
|
||||
`record_wing_faults` (see there). Nothing is excluded by BASENAME.
|
||||
|
||||
Excluding by basename was a real hole: `_NON_DECISION_FILES` is `{README.md, migration-map.md,
|
||||
Excluding by BASENAME was a real hole: `_NON_DECISION_FILES` is `{README.md, migration-map.md,
|
||||
retrieval-eval.md}` — three TOPIC-dir names — and applying it to the wings meant a genuine
|
||||
record at `records/docs/retrieval-eval.md` was silently skipped. That path is not hypothetical:
|
||||
the path<->key rule forces key `docs.retrieval-eval` to live at exactly that filename, and
|
||||
`docs/decisions/retrieval-eval.md` is a real unmigrated file, i.e. a plausible migration target.
|
||||
Worse, `dl.active_files()` applies that filter only to the TOPIC_DIR glob, not to
|
||||
`RECORDS_DIR.rglob` — so such a file IS a corpus source while being exempt from the guard.
|
||||
Only a literal `README.md` sitting directly in a wing root is dropped here (no such file exists
|
||||
today; the catalog README lives in TOPIC_DIR, one level up).
|
||||
|
||||
So the exemption is by exact RELATIVE PATH, never by basename. The only entry is
|
||||
`archive/README.md`, a hand-written directory README that really does exist — an earlier
|
||||
version of this function excluded any wing-root `README.md` "since no such file exists today",
|
||||
which was simply false and would additionally have exempted a future `records/README.md`, i.e.
|
||||
reintroduced the very hole one directory over.
|
||||
"""
|
||||
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
|
||||
exempt_paths = {archive_dir / "README.md"}
|
||||
files: list[Path] = []
|
||||
for wing in (records_dir, archive_dir):
|
||||
if wing.exists():
|
||||
files += sorted(p for p in wing.rglob("*.md") if not (p.parent == wing and p.name == "README.md"))
|
||||
files += sorted(p for p in wing.rglob("*.md") if p not in exempt_paths)
|
||||
return files
|
||||
|
||||
|
||||
@@ -490,6 +495,12 @@ def record_wing_faults(records_dir: Path | None = None, archive_dir: Path | None
|
||||
# A wing that is absent or empty must be LOUD, not silently clean. Otherwise a partial checkout,
|
||||
# a renamed directory, or a bad monkeypatch turns the whole guard into a no-op that reports
|
||||
# success — which is the defect this function exists to close, applied to itself.
|
||||
# DELIBERATELY asymmetric: only the ACTIVE wing must be non-empty. A corpus with zero active
|
||||
# records is definitionally broken (and means the scan is measuring nothing, so a clean result
|
||||
# would be vacuous). An empty ARCHIVE is a perfectly normal state — it just means nothing has
|
||||
# been superseded or retired yet, which is true of any young repo and of a fresh clone before
|
||||
# the first supersession. Faulting on it would fail a correct corpus. Review asked for symmetry
|
||||
# here; the semantics genuinely differ, so this stays asymmetric with the reason stated.
|
||||
if not records_dir.exists() or not any(records_dir.rglob("*.md")):
|
||||
faults.append(
|
||||
f"{records_dir}: the active record wing is missing or contains no *.md files — refusing "
|
||||
|
||||
@@ -907,3 +907,34 @@ def test_an_empty_or_missing_record_wing_is_LOUD(tmp_path):
|
||||
records, archive = _wing(tmp_path) # exists but holds no *.md
|
||||
empty = dv.record_wing_faults(records, archive)
|
||||
assert empty and "missing or contains no" in empty[0], empty
|
||||
|
||||
|
||||
def test_a_wing_root_README_is_exempt_by_PATH_not_by_basename(tmp_path):
|
||||
"""`docs/decisions/archive/README.md` really exists (a hand-written directory README), so it
|
||||
must be exempt — but by exact relative path, not by basename.
|
||||
|
||||
An earlier version excluded ANY wing-root `README.md` on the stated grounds that no such file
|
||||
existed. That was false, and it would additionally have exempted a future
|
||||
`records/README.md` — reintroducing the basename hole one directory over, in the wing that
|
||||
matters most."""
|
||||
records, archive = _wing(tmp_path)
|
||||
(records / "ci" / "good.md").write_text(_GOOD)
|
||||
(archive / "README.md").write_text("# archive\n\nHand-written directory README.\n")
|
||||
assert dv.record_wing_faults(records, archive) == [], "archive/README.md must stay exempt"
|
||||
|
||||
(records / "README.md").write_text("# not a record\n")
|
||||
faults = dv.record_wing_faults(records, archive)
|
||||
assert len(faults) == 1 and "README.md" in faults[0], (
|
||||
f"a README in the ACTIVE wing must NOT inherit the archive exemption: {faults}"
|
||||
)
|
||||
|
||||
|
||||
def test_an_empty_ARCHIVE_wing_is_legitimate(tmp_path):
|
||||
"""Deliberate asymmetry with the active-wing check, pinned so nobody 'fixes' it into symmetry.
|
||||
|
||||
Zero active records means the scan measured nothing and any clean result is vacuous. Zero
|
||||
ARCHIVED records just means nothing has been superseded yet — normal for a young repo and for
|
||||
every fresh clone before the first supersession."""
|
||||
records, archive = _wing(tmp_path)
|
||||
(records / "ci" / "good.md").write_text(_GOOD)
|
||||
assert dv.record_wing_faults(records, archive) == [], "an empty archive must not fault"
|
||||
|
||||
Reference in New Issue
Block a user