Cross-family review (High + Medium + Low), all three reproduced before fixing:
High -- on a pull_request event actions/checkout lands on a synthetic merge commit whose
body the forge composes from the PR description, so a description ending in an example
marker armed a guard no author armed. Excluded merge commits from the range; merging main
into a PR branch is separately forbidden, so no author-written commit is skipped.
Medium -- a folded value (`no` + continuation ` yes`) was split into independent lines and
the continuation armed on its own, inverting the value the author wrote. Read with `unfold`
so the value is judged whole.
Low -- the module docstring promised fail-open while marker resolution deliberately fails
closed. The posture is right; the docstring was wrong. Documented as the one exception.
Both new negative controls verified red against the unfixed matcher. Decision record
amended to state the residual honestly rather than overclaim: a quoted example that is the
FINAL paragraph of an ordinary commit is a trailer by git's own grammar and does arm. What
the change buys is that discussing the marker can no longer disable anything.
fixes#609
Decisions-Edit: yes
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The rationale-edit exemption was a substring test over the whole commit range, so any
message containing the literal marker armed it -- including prose explaining why no
marker was needed, which is how it fired live in PR #605: a green --base/--head run
that was vacuous on the body-diff dimension, in the one PR that hand-resolved a merge
conflict inside the corpus the guard exists to police.
Now read as an affirmative `Decisions-Edit:` git trailer. Git parses trailers only in
the final paragraph, so a quoted example message cannot arm it -- which matters because
this commit and its decision record both quote one. A non-affirmative value (`no`) does
not arm it either; the retired substring arms nothing and gets a ::warning:: nudge.
Tests: negative controls (prose mention, quoted mid-body trailer, `no` value, retired
substring) plus positive controls (trailer, uppercase, alongside Co-Authored-By). All
four negative controls verified red against the old matcher.
fixes#609
Decisions-Edit: yes
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1. `date.fromisoformat` is not a YYYY-MM-DD validator. On Python >= 3.11 it also
accepts ISO basic format ("20270101") and week dates ("2027-W01-1"), so a
malformed-looking value passed the blocking check — and which forms parse
depends on the interpreter, meaning the same corpus could validate differently
on a dev machine and on the runner (pr-checks.yml pins only python-version
'3.x'). Knock-on: the catalog's Review-due section sorts on the raw STRING, so
an accepted "20270101" sorted AFTER "2027-01-15" ('-' < '0'), contradicting the
section's own "sorted soonest-first" text. Gate on ^\d{4}-\d{2}-\d{2}$ first,
which fixes both — a fixed-width zero-padded form makes string sort == date sort.
2. A present-but-empty `stale-after:` was collapsed to None by `or None` in the
parser and then skipped by a truthiness guard in the validator, so it passed as
"absent" — a field that silently never fires, which is the exact failure mode
the blocking check exists to prevent. Keep "" distinct from None and test with
`is not None`.
3. `test_catalog_is_date_independent` was partly vacuous: with no date in either
render, both sides were trivially equal after the .replace(). It did still catch
an injected clock-derived marker, but it passed with the feature deleted. Assert
the dates are present.
4. The malformed-date check ran only over the active set, exempting archive
records. Staleness is moot there, but a typo is still a typo — check both wings.
Adds regression tests for each, plus a Review-due row for a topic-file record
(pinning the `../decisions.md` vs bare-filename link forms).
Evaluated the Open Knowledge Format (GoogleCloudPlatform/knowledge-catalog okf
v0.2, scaccogatto/okf-skills) as a replacement for our decision-record system and
rejected it: its conformance rules are deliberately permissive exactly where ours
are strict (broken links, unknown types and missing fields must all be tolerated;
`deprecated` points at no successor), and its stable identity is the file path,
which the breadcrumb rule tells agents not to trust.
Adopted two of its optional families instead, additively:
- `stale-after: YYYY-MM-DD` on the metadata line — marks a record asserting an
outside-world fact as due for re-confirmation. Absolute date, no TTL.
- `**Sources:**` in the metadata block — the evidence a record rests on, as
distinct from `Signals:` (recall keywords).
Neither is required; absence is never an error. A malformed `stale-after` is
blocking (it would silently never fire), but a past-due record is only a
non-blocking `::notice::` — going stale is the passage of time, not a defect in
whatever commit is under test. The catalog's new "Review due" section renders the
date only and never a clock-derived verdict, so it cannot drift `--check` red on a
calendar boundary with no commit touching the corpus.
No backfill: no existing record adopts either field here.
fixes#603
The decisions-guard validator enforced lifecycle metadata (key/status/since/
supersedes/superseded-by + reciprocal links) but not the **Signals:** line —
which is exactly what MemPalace's keyword recall matches on. A record without
it ingests with weak recall metadata and produces confident false-negatives
for the "MemPalace to find, file to confirm" retrieval workflow.
Add "signals" to REQUIRED_META so a migrated record with a missing or empty
**Signals:** line fails the same way a missing key does. All 114 active + 2
archive records already carry a Signals line, so this is non-breaking on the
current corpus. Archive records are intentionally out of scope (recall targets
the active corpus).
- scripts/decisions_validate.py: signals in REQUIRED_META (+ rationale comment)
- scripts/tests/test_decisions_validate.py: _rec() default + missing/empty/present cases
- docs/decisions.md: header + Enforcement note the requirement and why
fixes#545
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- Exclude docs/decisions/retrieval-eval.md from active decision parsing
(_NON_DECISION_FILES); its `## N.` eval-question headings were being
miscounted as 7 legacy-unmigrated records.
- Add decisions_lib.metadata_line_count() + a decisions_validate guard
that fails a record with more than one `key:` metadata line, so a
stacked-metadata-block migration bug (which the parser silently
tolerated by reading only the first block) can't recur unnoticed.
TDD: test_duplicate_metadata_block_fails / test_single_metadata_block_passes.
- De-duplicate the 6 docs/decisions.md records left with two stacked
metadata blocks (scan.getoraddfolder-db-lookup #488,
scan.musicvideo-reconciliation #494, scan.jellyfin-mixed-content-library
#489, iptv.logo-drives-bug-preset #67, ffmpeg.qsv-decode-encode-split
#498, ci.small-lane-git-only server-management#639), merging the union
of Signals/paths/issues/Mechanics from both blocks and keeping the
richer Rule wording; rationale prose untouched.
- Fill in the deferred Q6b row in docs/decisions/retrieval-eval.md now
that startup.parallel-orientation is active in docs/decisions.md,
scoring it as a real active-vs-superseded question against the
archived docs.queue-state-gitea-tracker.
- Regenerate docs/decisions/README.md via build_decisions_catalog.py.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
_anchor() mapped each space/hyphen to its own '-' without collapsing runs,
so the standard heading separator " — " (space, em-dash, space) produced a
double hyphen in every generated anchor. Since nearly every real decision
record heading uses that separator, this made the catalog emit a dead link
for essentially every row. Fix: after building the char list, collapse
consecutive '-' into one and strip leading/trailing '-' via re.sub, matching
how Goldmark/GitHub/Gitea sluggers behave.
Also fixed main() writing an extra trailing newline (want already ends in
"\n", then "+ \n" appended a second one) so docs/decisions/README.md now
ends with exactly one trailing newline; --check still compares via .strip().
Added test_anchor_collapses_em_dash_and_keeps_underscore to pin the anchor
behavior against the reported iptv.base_url case.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Adds scripts/build_decisions_catalog.py, which renders docs/decisions/README.md
as a compact table of only 'active' decision records (sorted by key), and its
test scripts/tests/test_build_catalog.py. Supports --check for CI drift
detection. No decision records are migrated yet, so the generated catalog is
currently empty (banner + header only) — expected at this stage.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Adds scripts/decisions_validate.py: lifecycle invariant checks (unique
active key, key format, reciprocal supersession, removed-without-archive,
rationale-rewrite-without-token, catalog staleness, corpus budget) plus
git-diff helpers for merge-base-based CI checks. Deviates from the task
brief in one spot: REQUIRED_META narrowed to (key, status) — the brief's
(key, status, since, supersedes, superseded_by) makes its own
test_clean_corpus_passes fail, since since/supersedes/superseded_by
default to None on bare Record() instances built without going through
the markdown parser.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Adds scripts/decisions_lib.py, the shared parser for ErsatzTV decision
records (docs/decisions.md + docs/decisions/*.md). Parses H2 sections
into Record dataclasses, distinguishing migrated records (visible
metadata block: key/status/since/supersedes/superseded-by + Rule/
Signals/Mechanics) from legacy-unmigrated ones with no metadata line.
scripts/ is now an importable package (scripts/__init__.py,
scripts/tests/__init__.py) so later tools can `import scripts.decisions_lib`.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
timothyandClaude Opus 5