From 6e085564ab10cd89f439c1c706fbef893eb0f9e4 Mon Sep 17 00:00:00 2001 From: igerber Date: Sun, 30 Aug 2026 10:06:30 -0400 Subject: [PATCH 1/9] chore(infra): tracking-file contract guard, LWDiD real-data canary, /push-pr-update range scan Closes three TODO.md infra rows: - tests/test_tracking_files.py: CI guard for the TODO.md/DEFERRED.md split contract - no TODO table row points work at DEFERRED.md, rows cross-linking an M-xxx ledger id do not restate lifecycle state, and the three documented table shapes (TODO Actionable, DEFERRED blocker, DEFERRED Decision record) are enforced with per-row column counts. The parser handles escaped pipes, unspaced rows, and GFM no-leading-pipe tables, and fails loudly on malformed (non-rendering) tables. Wired into docs-tests.yml (path filters + step); excluded from rust-test.yml triggers. CLAUDE.md documents the Decision-record shape. - .github/workflows/lwdid-data-canary.yml: weekly cron + dispatch lane that fails loudly if load_prop99/load_walmart fall back to synthetic data (source != 'lwdid_ssc_ancillary'), then runs the LWDiD replication tests the canary de-gates. Test docstrings now point at the lane instead of the deleted TODO row. - .claude/commands/push-pr-update.md section 3b: restored the committed-range pattern scan via premerge_scan.py --range with the command's own COMPARISON_REF resolver; findings informational, scan-integrity failures (exit 3/4) stop-and-report. pre-merge-check.md splits premerge_scan and changelog_compile check into separate Bash calls so the first exit status is not masked. Also hardens the freshly merged changelog fragments compiler (#806), found during local review of this branch: - Malformed/duplicate release headers and link definitions refused on both fresh and exit-4 paths; exit-4 predecessor derived only from a canonical compare/tag link; --previous-version cross-check (bump-version.md now passes it) with interrupted-bump recovery kept; prev-anchor date validation with a documented legacy exemption. - Existing-target sections are validated with the full fragment grammar on the exit-4 path (orphan bullets / unknown-category bullets refuse). - Indented ATX headings (CommonMark accepts 1-3 leading spaces) are rejected by the fragment grammar - previously ' ## [9.9.9]' compiled into the changelog as a real H2 invisible to the header scanners. - Dirty guard catches deleted-but-committed fragments; compile rolls back changelog + fragments on a mid-flight unlink failure; strict kebab fragment-name grammar. Release note in changelog.d/20260830-infra-process-sweep.md (Internal). --- .claude/commands/bump-version.md | 2 +- .claude/commands/pre-merge-check.md | 15 +- .claude/commands/push-pr-update.md | 46 +- .claude/scripts/changelog_compile.py | 218 +++++++++- .github/workflows/docs-tests.yml | 20 + .github/workflows/lwdid-data-canary.yml | 69 +++ .github/workflows/rust-test.yml | 2 + CLAUDE.md | 9 + DEFERRED.md | 6 +- TODO.md | 3 - changelog.d/20260830-infra-process-sweep.md | 13 + tests/test_changelog_fragments.py | 223 ++++++++++ tests/test_methodology_lwdid.py | 12 +- tests/test_tracking_files.py | 450 ++++++++++++++++++++ 14 files changed, 1043 insertions(+), 45 deletions(-) create mode 100644 .github/workflows/lwdid-data-canary.yml create mode 100644 changelog.d/20260830-infra-process-sweep.md create mode 100644 tests/test_tracking_files.py diff --git a/.claude/commands/bump-version.md b/.claude/commands/bump-version.md index 511815971..f4a0cfd8d 100644 --- a/.claude/commands/bump-version.md +++ b/.claude/commands/bump-version.md @@ -43,7 +43,7 @@ Files that need updating: is removed): ```bash - python3 .claude/scripts/changelog_compile.py compile --version NEW_VERSION --date "$(date +%F)" + python3 .claude/scripts/changelog_compile.py compile --version NEW_VERSION --date "$(date +%F)" --previous-version "$OLD_VERSION" ``` Key off the exit code: diff --git a/.claude/commands/pre-merge-check.md b/.claude/commands/pre-merge-check.md index 7eba49d40..1ddd572b6 100644 --- a/.claude/commands/pre-merge-check.md +++ b/.claude/commands/pre-merge-check.md @@ -53,17 +53,26 @@ every path** — emitting only validated-safe run-lists. It is unit-tested (`tests/test_premerge_scan.py`) with staged *and* untracked `$(touch sentinel)` filenames asserting nothing executes. -Run it: +Run it — as TWO separate Bash calls, so each command's exit status is +inspected on its own (a single block reports only the LAST status, and a +scan exit 3/4 must never be masked by a passing changelog check): ```bash SCRATCH="$(git rev-parse --git-path premerge-scan)"; mkdir -p "$SCRATCH" python3 .claude/scripts/premerge_scan.py --scratch "$SCRATCH" +``` + +Handle the scan's exit per the rules below BEFORE moving on. Then, +separately: + +```bash python3 .claude/scripts/changelog_compile.py check ``` -The second command is the changelog-fragment guard (pointer-only +This second call is the changelog-fragment guard (pointer-only `## [Unreleased]` + fragment grammar) - it catches a direct Unreleased edit -here instead of first failing in the label-gated docs-tests CI lane. +here instead of first failing in the label-gated docs-tests CI lane; a +non-zero exit is its own finding to report, independent of the scan. - If it **exits 4**, a git or file-read operation failed — the scan is **incomplete** and its run-lists were truncated to empty. **Stop and report the error;** do NOT diff --git a/.claude/commands/push-pr-update.md b/.claude/commands/push-pr-update.md index 750665632..ae372d519 100644 --- a/.claude/commands/push-pr-update.md +++ b/.claude/commands/push-pr-update.md @@ -146,15 +146,41 @@ When the working tree is clean but commits are ahead, scan for secrets in the co When the working tree is clean but commits are ahead, check for methodology issues before pushing: -1. **Methodology review of already-committed changes is deferred to `/pre-merge-check`.** - The changes here are already committed, so this pattern check is non-blocking, and - the pre-merge gate (run before committing) is the right place for it. Do **not** - interpolate a comparison ref into a scan command here — `/pre-merge-check` covers the - working-tree case safely via `premerge_scan.py`, and re-deriving a committed range in - prose is where injection creeps back in. If you want the committed range checked, run - `/pre-merge-check` on the branch before it was committed, or review the diff by eye. - -3. **Documentation impact check**: Check which source files in `diff_diff/` are in the committed changes. +1. **Methodology pattern scan of the committed range** — via the tested argv-safe + helper's `--range` mode (`premerge_scan.py`; the range is passed as DATA in a + quoted variable, never a raw placeholder — the injection shape that got the old + prose-grep version removed). Re-derive the comparison ref inside this one Bash + call using the same fallback chain as Section 2 (shell variables do not persist + across tool calls): + + ```bash + SCRATCH="$(git rev-parse --git-path premerge-scan)"; mkdir -p "$SCRATCH" + DEFAULT_BRANCH="$(gh repo view --json defaultBranchRef --jq '.defaultBranchRef.name' 2>/dev/null || echo main)" + if UP="$(git rev-parse --abbrev-ref @{u} 2>/dev/null)"; then + COMPARISON_REF="$UP" + elif git rev-parse --verify "$DEFAULT_BRANCH" >/dev/null 2>&1; then + COMPARISON_REF="$DEFAULT_BRANCH" + elif git rev-parse --verify "origin/$DEFAULT_BRANCH" >/dev/null 2>&1; then + COMPARISON_REF="origin/$DEFAULT_BRANCH" + else + git fetch origin "$DEFAULT_BRANCH" --depth=1 2>/dev/null || true + COMPARISON_REF="origin/$DEFAULT_BRANCH" + fi + python3 .claude/scripts/premerge_scan.py --scratch "$SCRATCH" --range "$COMPARISON_REF..HEAD" + ``` + + (Upstream-first, exactly Section 2.4's resolver: on an existing PR the scan + covers only the UNPUSHED commits — comparing against the default branch would + rescan previously pushed, already-reviewed changes.) + + Pattern FINDINGS are informational (report file:line; the changes are already + committed). Scan-INTEGRITY failures are not: **exit 3** means a changed path + carries shell metacharacters (excluded from the scan — surface it for manual + review) and **exit 4** means a git/read failure truncated the run-lists — the + scan is incomplete, so report the error rather than describing the range as + clean. + +2. **Documentation impact check**: Check which source files in `diff_diff/` are in the committed changes. If source files are present, read `docs/doc-deps.yaml` and check which dependent documentation files are NOT also in the committed changes. Warn about: - ALL docs with `type: methodology` (regardless of `drift_risk`) @@ -169,7 +195,7 @@ When the working tree is clean but commits are ahead, check for methodology issu `changelog.d/` fragment (see CONTRIBUTING.md "Changelog fragments"). This is a WARNING, not a blocker. -Note: Section 3b checks are informational warnings only — no AskUserQuestion prompt, since changes are already committed and cannot be unstaged. This differs from the staged-changes path (Section 3) which offers a "fix vs continue" choice. +Note: Section 3b FINDINGS are informational warnings only — no AskUserQuestion prompt, since changes are already committed and cannot be unstaged (unlike the staged-changes path, Section 3, which offers a "fix vs continue" choice). The one exception is scan INTEGRITY: `premerge_scan.py` exit 3/4 means the scan itself is incomplete — report that rather than proceeding as if the range were clean. ### 3. Stage and Commit Changes diff --git a/.claude/scripts/changelog_compile.py b/.claude/scripts/changelog_compile.py index 48a3d4700..6658aadb9 100644 --- a/.claude/scripts/changelog_compile.py +++ b/.claude/scripts/changelog_compile.py @@ -55,7 +55,7 @@ "compiled at release by .claude/scripts/changelog_compile.py -->" ) -FRAGMENT_NAME_RE = re.compile(r"^(\d{8})-[a-z0-9][a-z0-9-]*\.md$") +FRAGMENT_NAME_RE = re.compile(r"^(\d{8})-[a-z0-9]+(?:-[a-z0-9]+)*\.md$") VERSION_RE = re.compile(r"^(0|[1-9]\d*)\.(0|[1-9]\d*)\.(0|[1-9]\d*)$") EXIT_OK = 0 @@ -89,6 +89,12 @@ def _parse_fragment(text): current = (cat, []) blocks.append(current) continue + if re.match(r" {1,3}#{1,6}(?:[ \t]|$)", line): + # CommonMark recognizes ATX headings indented up to 3 spaces, so + # an indented '## ...' would slip past the column-zero checks + # above as a "continuation" yet render as a real heading. + errors.append(f"line {lineno}: indented Markdown heading not allowed in a fragment") + continue if not line.strip(): if current is not None: current[1].append(line) @@ -236,19 +242,26 @@ def _existing_headers(changelog_text): return out -def _section_nonempty(changelog_text, header_start): +def _section_body(changelog_text, header_start): nl = changelog_text.find("\n", header_start) if nl == -1: - # Header is the last line with no trailing newline: empty section. - return False + return "" body_start = nl + 1 nxt = re.compile(r"^## ", flags=re.MULTILINE).search(changelog_text, body_start) - body = changelog_text[body_start : nxt.start() if nxt else len(changelog_text)] - has_cat = any( - ln.startswith("### ") and ln[4:].strip() in CATEGORIES for ln in body.splitlines() - ) - has_bullet = any(ln.startswith("- ") for ln in body.splitlines()) - return has_cat and has_bullet + return changelog_text[body_start : nxt.start() if nxt else len(changelog_text)] + + +def _section_grammar_errors(changelog_text, header_start): + """Errors from validating a release section's body with the fragment + block grammar. A compiled section is exactly a concatenation of + fragment category blocks, so compiler OUTPUT always passes; an orphan + bullet, an empty category, or an unknown-category bullet is a state + compile could not have produced.""" + body = _section_body(changelog_text, header_start) + if not body.strip(): + return ["section is empty"] + _, errors = _parse_fragment(body) + return errors def _canonical_date(s): @@ -261,7 +274,7 @@ def _canonical_date(s): return parsed.isoformat() == s -def run_compile(root, version, date_s, allow_dirty): +def run_compile(root, version, date_s, allow_dirty, previous_version=None): if not VERSION_RE.match(version): print( f"error: --version {version!r} is not a canonical SemVer " "X.Y.Z (no leading zeros)", @@ -283,6 +296,21 @@ def run_compile(root, version, date_s, allow_dirty): changelog_path = root / "CHANGELOG.md" text = changelog_path.read_text() + # Reject release-LIKE headers the canonical grammar does not match + # (e.g. '## [1.3.0] - 2026-08-30 draft'): _existing_headers sees only + # canonical shapes, so a malformed section would otherwise be invisible + # to the duplicate/date safeguards and compile could add a SECOND + # section for the same version. + canonical_starts = {m_start for _, _, m_start in _existing_headers(text)} + for m in re.finditer(r"^## \[(?!Unreleased\])[^\]]*\][^\n]*$", text, flags=re.MULTILINE): + if m.start() not in canonical_starts: + print( + "error: malformed release header in CHANGELOG.md: " + f"{m.group(0)!r} does not match '## [X.Y.Z] - YYYY-MM-DD' — " + "fix the file before compiling", + file=sys.stderr, + ) + return EXIT_FINDINGS headers = _existing_headers(text) versions = [h[0] for h in headers] duplicated = sorted({v for v in versions if versions.count(v) > 1}) @@ -298,7 +326,52 @@ def run_compile(root, version, date_s, allow_dirty): file=sys.stderr, ) return EXIT_FINDINGS + # Link-definition duplicate check runs BEFORE the existing-target path: + # exit 4 must never certify a changelog carrying two definitions for + # any version (one correct + one wrong would otherwise pass). + link_versions = re.findall(r"^\[(\d+\.\d+\.\d+)\]:\s+\S+\s*$", text, flags=re.MULTILINE) + dup_links = sorted({v for v in link_versions if link_versions.count(v) > 1}) + if dup_links: + print( + "error: duplicated comparison-link definition(s) in " + "CHANGELOG.md: " + ", ".join(f"'[{v}]:'" for v in dup_links), + file=sys.stderr, + ) + return EXIT_FINDINGS prev = max(versions, key=_semver_tuple) if versions else None + if prev is not None: + # The anchor header is the compiler's own output from the previous + # cycle, so its date must be canonical (a fresh compile atop + # '## [X.Y.Z]' with no/impossible date would silently extend a + # corrupt tip). OLDER legacy headers are deliberately exempt - the + # real CHANGELOG carries dateless pre-convention releases + # (## [0.6.0] and earlier) that the compiler never touches. + prev_date = next(d for v, d, _ in headers if v == prev) + if prev_date is None or not _canonical_date(prev_date): + print( + f"error: the latest release header '## [{prev}]' has a " + f"missing or non-canonical date ({prev_date!r}) — fix it " + "before compiling a new release on top", + file=sys.stderr, + ) + return EXIT_FINDINGS + if previous_version is not None and prev is not None and previous_version != prev: + # Cross-check against the caller's package-metadata version + # (bump-version's OLD_VERSION): a drifted CHANGELOG would otherwise + # silently anchor the comparison link to the wrong ancestor and + # delete the fragments before anyone noticed. EXCEPTION: when the + # latest release IS the compile target, this is the interrupted-bump + # recovery shape (compile finished, package metadata not yet + # updated) — fall through to the existing-target path, which itself + # validates previous_version against the target's real predecessor. + if prev != version: + print( + f"error: --previous-version {previous_version!r} does not match " + f"the latest CHANGELOG.md release {prev!r} — package metadata " + "and changelog have drifted; reconcile before compiling", + file=sys.stderr, + ) + return EXIT_FINDINGS # Existing-header detection FIRST, so the idempotent re-run path is # reachable before any monotonicity check. @@ -311,10 +384,12 @@ def run_compile(root, version, date_s, allow_dirty): file=sys.stderr, ) return EXIT_FINDINGS - if not _section_nonempty(text, hstart): + section_errors = _section_grammar_errors(text, hstart) + if section_errors: print( - f"error: '## [{version}]' exists but its section is " - "empty — not a completed compile; fix the header", + f"error: '## [{version}]' exists but its section is not " + "a state compile could have produced — not a completed " + "compile; fix the section:\n " + "\n ".join(section_errors), file=sys.stderr, ) return EXIT_FINDINGS @@ -335,6 +410,21 @@ def run_compile(root, version, date_s, allow_dirty): return EXIT_FINDINGS below = [v for v in versions if _semver_tuple(v) < _semver_tuple(version)] predecessor = max(below, key=_semver_tuple) if below else None + if previous_version is not None and previous_version not in ( + version, + predecessor, + ): + # Recovery accepts package metadata at the target (bump + # completed) or its immediate predecessor (interrupted + # before the metadata update); anything else is drift. + print( + f"error: --previous-version {previous_version!r} matches " + f"neither '## [{version}]' nor its predecessor " + f"{predecessor!r} — package metadata and changelog have " + "drifted; reconcile before re-running", + file=sys.stderr, + ) + return EXIT_FINDINGS if predecessor is None: # compile always anchors its comparison link to an existing # release, so a sole-header state cannot be its output. @@ -346,10 +436,35 @@ def run_compile(root, version, date_s, allow_dirty): ) return EXIT_FINDINGS if predecessor is not None: + # The target link must use the SAME base URL as the + # predecessor's link (the base the compiler itself would + # have written) - a correct-version link pointing at an + # unrelated repository is not a completed compile. + pred_m = re.search( + r"^\[" + re.escape(predecessor) + r"\]:\s+(\S+?)/(?:compare|releases)/\S+$", + text, + flags=re.MULTILINE, + ) + if pred_m is None: + # The compiler's own output always links the + # predecessor (compare/, or releases/tag/ for the very + # first release) - no link means this is not a state + # compile produced; never fall back to accepting any + # base. + print( + f"error: no link definition found for the " + f"predecessor '[{predecessor}]' — not a completed " + "compile; fix the link block", + file=sys.stderr, + ) + return EXIT_FINDINGS + pred_base = re.escape(pred_m.group(1)) link_re = re.compile( r"^\[" + re.escape(version) - + r"\]: \S+/compare/v" + + r"\]: " + + pred_base + + r"/compare/v" + re.escape(predecessor) + r"\.\.\.v" + re.escape(version) @@ -434,6 +549,28 @@ def run_compile(root, version, date_s, allow_dirty): ).stdout if blob != p.read_bytes(): problems.append(f"{rel}: worktree bytes differ from the HEAD blob") + # Set equality with HEAD: a committed fragment DELETED from the + # worktree would otherwise be silently dropped from the release + # (the compile removes every fragment file, so the git diff shows + # both deletions while only the surviving one was compiled). + head_res = subprocess.run( + ["git", "-C", str(root), "ls-tree", "-r", "--name-only", "HEAD", "--", "changelog.d"], + capture_output=True, + text=True, + ) + if head_res.returncode == 0: + head_frags = { + Path(line).name + for line in head_res.stdout.splitlines() + if FRAGMENT_NAME_RE.match(Path(line).name) + } + worktree_frags = {p.name for p in fragments} + for missing in sorted(head_frags - worktree_frags): + problems.append( + f"changelog.d/{missing}: committed in HEAD but absent " + "from the worktree - its release note would be " + "silently lost" + ) if problems: print( "error: changelog.d/ fragments must be committed unchanged " @@ -465,14 +602,30 @@ def run_compile(root, version, date_s, allow_dirty): insert_at = sl[1] text = text[:insert_at] + new_section + text[insert_at:] + # A definition for the TARGET version must not already exist on the + # fresh-compile path (compiling would emit a duplicate '[X.Y.Z]:' line; + # duplicates generally were rejected before the existing-target path). + if version in link_versions: + print( + f"error: a comparison-link definition '[{version}]:' already " + "exists in CHANGELOG.md but the release header does not — " + "inconsistent state; fix the link block before compiling", + file=sys.stderr, + ) + return EXIT_FINDINGS + # Same predecessor-link form the exit-4 path accepts: compare/ links, + # or releases/tag/ for a sole first release (its second release could + # not compile otherwise); whitespace-tolerant after ':'. prev_link_re = re.compile( - r"^\[" + re.escape(prev) + r"\]: (\S+?)/compare/\S+$", flags=re.MULTILINE + r"^\[" + re.escape(prev) + r"\]:\s+(\S+?)/(?:compare|releases)/\S+$", + flags=re.MULTILINE, ) m = prev_link_re.search(text) if not m: print( - f"error: comparison link '[{prev}]: .../compare/...' not found " - "at the bottom of CHANGELOG.md", + f"error: comparison link '[{prev}]: .../compare/...' (or the " + "first release's .../releases/tag/... form) not found at the " + "bottom of CHANGELOG.md", file=sys.stderr, ) return EXIT_FINDINGS @@ -480,9 +633,26 @@ def run_compile(root, version, date_s, allow_dirty): new_link = f"[{version}]: {base}/compare/v{prev}...v{version}\n" text = text[: m.start()] + new_link + text[m.start() :] + original_text = changelog_path.read_text() + fragment_bytes = {p: p.read_bytes() for p in fragments} changelog_path.write_text(text) - for p in fragments: - p.unlink() + try: + for p in fragments: + p.unlink() + except OSError as exc: + # Roll back to the pre-compile state: a half-deleted fragment set + # beside an already-written release section would need manual + # recovery (the next run deliberately refuses that shape). + changelog_path.write_text(original_text) + for p, data in fragment_bytes.items(): + if not p.exists(): + p.write_bytes(data) + print( + f"error: fragment deletion failed ({exc}); CHANGELOG.md and " + "all fragments restored - nothing was compiled", + file=sys.stderr, + ) + return EXIT_FINDINGS print(f"compiled: version={version} date={date_s} fragments={len(fragments)}") return EXIT_OK @@ -496,6 +666,12 @@ def main(argv=None): comp.add_argument("--version", required=True) comp.add_argument("--date", required=True) comp.add_argument("--allow-dirty", action="store_true") + comp.add_argument( + "--previous-version", + default=None, + help="cross-check: must equal the latest CHANGELOG.md release " + "(bump-version passes its package-metadata OLD_VERSION)", + ) args = parser.parse_args(argv) root = (args.root or default_root()).resolve() @@ -507,7 +683,7 @@ def main(argv=None): return EXIT_FINDINGS print("check: OK") return EXIT_OK - return run_compile(root, args.version, args.date, args.allow_dirty) + return run_compile(root, args.version, args.date, args.allow_dirty, args.previous_version) if __name__ == "__main__": diff --git a/.github/workflows/docs-tests.yml b/.github/workflows/docs-tests.yml index 9e736a102..6e6e068e0 100644 --- a/.github/workflows/docs-tests.yml +++ b/.github/workflows/docs-tests.yml @@ -18,6 +18,13 @@ on: - 'changelog.d/**' - 'CHANGELOG.md' - '.claude/scripts/changelog_compile.py' + # Tracking-file contract guard: TODO/DEFERRED edits trigger here + # (their ONLY test lane - a tracking-file-only diff otherwise runs + # no suite that executes the guard). + - 'tests/test_tracking_files.py' + - 'TODO.md' + - 'DEFERRED.md' + - '.github/workflows/lwdid-data-canary.yml' # tests/conftest.py is auto-loaded by pytest for the snippet # test run and mutates sys.path + MPLBACKEND (conftest.py:14, 18); # changes there can break snippet exec without touching the test @@ -45,6 +52,13 @@ on: - 'changelog.d/**' - 'CHANGELOG.md' - '.claude/scripts/changelog_compile.py' + # Tracking-file contract guard: TODO/DEFERRED edits trigger here + # (their ONLY test lane - a tracking-file-only diff otherwise runs + # no suite that executes the guard). + - 'tests/test_tracking_files.py' + - 'TODO.md' + - 'DEFERRED.md' + - '.github/workflows/lwdid-data-canary.yml' - 'tests/conftest.py' - 'pyproject.toml' # sphinx-build job mirrors RTD setup; trigger when RTD config drifts @@ -129,6 +143,12 @@ jobs: # tests/test_changelog_fragments.py::TestWorkflowPins). run: PYTHONPATH=. DIFF_DIFF_BACKEND=python pytest tests/test_changelog_fragments.py -v + - name: Run tracking-file contract guard + # TODO/DEFERRED conventions (no deferred-work pointers in TODO rows, + # no ledger-lifecycle restatements beside M-xxx links, documented + # table shapes + per-row column counts). + run: PYTHONPATH=. DIFF_DIFF_BACKEND=python pytest tests/test_tracking_files.py -v + sphinx-build: name: Sphinx HTML build (-W warnings as errors) # Skip unrelated label churn: a non-ready-for-ci label add/remove won't run this job. diff --git a/.github/workflows/lwdid-data-canary.yml b/.github/workflows/lwdid-data-canary.yml new file mode 100644 index 000000000..eb98ab089 --- /dev/null +++ b/.github/workflows/lwdid-data-canary.yml @@ -0,0 +1,69 @@ +# Real-data canary for the LWDiD replication tests (TODO row: "Real-data CI +# canary for dataset-backed replication tests"). +# +# tests/test_methodology_lwdid.py's Prop 99 / Walmart goldens VISIBLY skip +# when the loaders fall back to synthetic data (df.attrs["source"] != +# "lwdid_ssc_ancillary"), so a network/URL/sha regression at the SSC mirror +# can stay green indefinitely. This lane makes that regression loud: the +# canary step fails the job if either loader is on its synthetic fallback, +# then runs the replication tests the canary de-gates (their committed-golden +# skips remain legal). +# +# Cron-only + manual (no PR trigger): the download is network-dependent by +# design, exactly what the label-gated PR lanes deliberately avoid. +name: LWDiD Data Canary + +on: + workflow_dispatch: + schedule: + # Weekly Sunday 8am UTC (docs-tests/notebooks run 6am, mmm-interop 7am — + # offset to avoid stacking the Sunday lanes). + - cron: '0 8 * * 0' + +permissions: + contents: read + +jobs: + lwdid-real-data: + name: Prop 99 / Walmart real-data canary + runs-on: ubuntu-latest + + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7 + + - name: Set up Python + uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v7.0.0 + with: + python-version: '3.12' + + - name: Install dependencies + run: | + pip install numpy pandas scipy pytest + # Add repo root to Python path so tests can import diff_diff + # (pip install -e . requires the Rust/maturin toolchain; .pth avoids that) + python -c "import site; print(site.getsitepackages()[0])" | xargs -I{} sh -c 'echo "$PWD" > {}/diff_diff_dev.pth' + + - name: Data canary + # Its own step so a download/sha/provenance failure FAILS the job + # loudly instead of degrading into the tests' visible-but-green + # synthetic-fallback skips. + env: + DIFF_DIFF_BACKEND: python + run: | + python -c " + import warnings + from diff_diff.datasets import load_prop99, load_walmart + with warnings.catch_warnings(): + warnings.simplefilter('error') # the fallback warns - make it fatal + for name, loader in (('prop99', load_prop99), ('walmart', load_walmart)): + df = loader() + src = df.attrs.get('source') + assert src == 'lwdid_ssc_ancillary', f'{name}: source={src!r}' + print(f'{name}: real data OK ({len(df)} rows)') + " + + - name: Run LWDiD replication tests + env: + DIFF_DIFF_BACKEND: python + run: | + pytest tests/test_methodology_lwdid.py -q --tb=short diff --git a/.github/workflows/rust-test.yml b/.github/workflows/rust-test.yml index 00cdd3006..dd6720c22 100644 --- a/.github/workflows/rust-test.yml +++ b/.github/workflows/rust-test.yml @@ -12,6 +12,7 @@ on: - '!tests/test_doc_snippets.py' # Same ownership split: the changelog-fragment guard runs in docs-tests.yml. - '!tests/test_changelog_fragments.py' + - '!tests/test_tracking_files.py' - 'tools/**' - 'pyproject.toml' - '.github/workflows/rust-test.yml' @@ -47,6 +48,7 @@ on: - '!tests/test_doc_snippets.py' # Same ownership split: the changelog-fragment guard runs in docs-tests.yml. - '!tests/test_changelog_fragments.py' + - '!tests/test_tracking_files.py' - 'tools/**' - 'pyproject.toml' - '.github/workflows/rust-test.yml' diff --git a/CLAUDE.md b/CLAUDE.md index 9fc94d64d..3dc0410ce 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -154,6 +154,15 @@ DEFERRED.md (blocker sections): |-------|----------|----|----------| | Description of deferred item | `file.py` | #NNN | Medium/Low | +DEFERRED.md (Decision record — won't-fix / waived): + +| Decision | Location | Verified | +|----------|----------|----------| +| The recorded decision | `file.py` | #NNN | + +These three shapes (and the no-deferred-pointers / no-ledger-restatement +rules above) are CI-enforced by `tests/test_tracking_files.py`. + ## README discipline `README.md` is a **landing page**, not the documentation. Target ~190 lines. The 3,119-line README that existed before the 2026-04 docs refresh grew because workflow conventions told contributors to add to README on every change. diff --git a/DEFERRED.md b/DEFERRED.md index cab4d793a..605120d0d 100644 --- a/DEFERRED.md +++ b/DEFERRED.md @@ -57,7 +57,7 @@ exists but parity can't be verified without a local toolchain. | `StaggeredTripleDifference` R cross-validation (the engine is shared with `TripleDifference`'s staggered mode since 3(b), so this covers both surfaces): CSV fixtures not committed (gitignored); tests skip without local R + `triplediff`. Commit fixtures or generate deterministically. | `tests/test_methodology_staggered_triple_diff.py` | #245 | Medium | | Staggered DDD R parity (both surfaces - one shared engine since 3(b)): benchmark only tests the no-covariate path (`xformla=~1`). Add covariate-adjusted scenarios + aggregation-SE parity assertions. | `benchmarks/R/benchmark_staggered_triplediff.R` | #245 | Medium | | Staggered DDD per-cohort group-effect SEs include WIF (both surfaces - one shared engine since 3(b)) (conservative vs R's `wif=NULL`); documented in REGISTRY. Could override the mixin for an exact R match (verification needs R `triplediff`). | `_staggered_triple_diff_engine.py` | #245 | Low | -| **WooldridgeDiD follow-up cluster** (PR-B Stage D/E fail-closed surfaces; re-enable after R/Stata validation):
• QMLE sandwich uses `aweight` cluster adjustment `(G/(G-1))·(n-1)/(n-k)` vs Stata's `G/(G-1)` (conservative); add a `qmle` weight type if Stata goldens confirm a material difference (`wooldridge.py`, `linalg.py`).
• response-scale APE / log-link coefficient bridge for R `etwfe(family=poisson|logit)` cell-level parity — needs `emfx()` APE extraction or link-inversion with baseline-mean adjustment (`generate_wooldridge_golden.R`, `test_methodology_wooldridge.py`).
• `aggregate(weights="cohort_share")` on survey-weighted fits: `_n_g_per_cohort` uses raw `unit.nunique()`; implement design-weighted unit totals per cohort (paper W2025 §7) and lift the `ValueError` gate (`wooldridge.py`, `wooldridge_results.py`).
• unconditional inference for `cohort_share` accounting for ω̂_g sampling uncertainty (W2025 §7.5); currently NaN-closed (`wooldridge_results.py`).
• `cohort_trends=True × survey_design` and `× control_group="never_treated"` raise `NotImplementedError` (unvalidated TSL variance / trend columns spanned jointly by the placebo cells and the unit FE, which absorb the cohort indicator and recover the omitted reference) (`wooldridge.py`).
• ~~Stata `jwdid` golden-value `TestReferenceValues`~~ RESOLVED: the golden ships four arms pinned by `tests/test_etwfe_cs_stata_parity.py` (no `TestReferenceValues` symbol was ever added). The QMLE bullet above REMAINS OPEN -- every golden arm is linear `jwdid`, so no QMLE cluster-SE reference exists, and SEs are pinned only as a ratio. | `wooldridge.py`, `wooldridge_results.py`, `linalg.py`, benchmarks | #216 · PR-B | Med-Low | +| **WooldridgeDiD follow-up cluster** (PR-B Stage D/E fail-closed surfaces; re-enable after R/Stata validation):
• QMLE sandwich uses `aweight` cluster adjustment `(G/(G-1))·(n-1)/(n-k)` vs Stata's `G/(G-1)` (conservative); add a `qmle` weight type if Stata goldens confirm a material difference (`wooldridge.py`, `linalg.py`).
• response-scale APE / log-link coefficient bridge for R `etwfe(family=poisson\|logit)` cell-level parity — needs `emfx()` APE extraction or link-inversion with baseline-mean adjustment (`generate_wooldridge_golden.R`, `test_methodology_wooldridge.py`).
• `aggregate(weights="cohort_share")` on survey-weighted fits: `_n_g_per_cohort` uses raw `unit.nunique()`; implement design-weighted unit totals per cohort (paper W2025 §7) and lift the `ValueError` gate (`wooldridge.py`, `wooldridge_results.py`).
• unconditional inference for `cohort_share` accounting for ω̂_g sampling uncertainty (W2025 §7.5); currently NaN-closed (`wooldridge_results.py`).
• `cohort_trends=True × survey_design` and `× control_group="never_treated"` raise `NotImplementedError` (unvalidated TSL variance / trend columns spanned jointly by the placebo cells and the unit FE, which absorb the cohort indicator and recover the omitted reference) (`wooldridge.py`).
• ~~Stata `jwdid` golden-value `TestReferenceValues`~~ RESOLVED: the golden ships four arms pinned by `tests/test_etwfe_cs_stata_parity.py` (no `TestReferenceValues` symbol was ever added). The QMLE bullet above REMAINS OPEN -- every golden arm is linear `jwdid`, so no QMLE cluster-SE reference exists, and SEs are pinned only as a ratio. | `wooldridge.py`, `wooldridge_results.py`, `linalg.py`, benchmarks | #216 · PR-B | Med-Low | | Extend `WooldridgeDiD` `method ∈ {logit, poisson}` with `vcov_type ∈ {classical, hc2, hc2_bm}`: composing HC2 leverage + Bell-McCaffrey DOF with the QMLE pseudo-residual sandwich needs derivation + R parity vs `clubSandwich::vcovCR(glm, type="CR2")`. Rejected at `__init__`. | `wooldridge.py` | follow-up | Medium | | Multi-constraint CR2 parallel-trends test (AHT/HTZ) for `hc2_bm` fits: DiagnosticReport's PT check routes `vcov_type="hc2_bm"` sources to Bonferroni over the BM-adjusted per-row p-values because the generic chi-square joint Wald would discard the CR2 small-sample correction (see REPORTING.md "hc2_bm parallel-trends policy"). The proper joint test is the AHT/HTZ Wald with a Satterthwaite-style denominator df over the pre-period contrast block; needs derivation for the stacked/pooled WLS-CR2 layout + parity vs `clubSandwich::Wald_test(..., test="HTZ")`. | `diagnostic_report.py`, `linalg.py` | vcov/df round-trip PR | Low | | `PreTrendsPower` CS/SA `anticipation=1` R-parity fixture: R `pretrends` has no anticipation parameter, so the Python `_extract_pre_period_params` anticipation filter isn't R-parity-locked. Build a synthetic CS/SA result with `anticipation=1` and assert γ_p matches R's `slope_for_power()`. (Mechanism already covered by MC + full-VCV tests.) | `tests/test_methodology_pretrends.py`, `generate_pretrends_golden.R` | PR-C | Low | @@ -88,7 +88,7 @@ For survey-specific limitations (`NotImplementedError` paths), see the | MMM interop, StackedDiD totals (remainder of the estimator-owned-totals delivery - `aggregate('total')` shipped for CS/DMLDiD/EfficientDiD/ImputationDiD/TwoStageDiD on panel non-survey fits): StackedDiD is STAGED OUT because the total estimand is ill-defined under `weighting="population"`/`"sample_share"` (a weighted estimand for which raw treated exposure is not the right mass), and its results class is view-only (a total would be its first computed level). Needs an estimand decision per weighting variant before any implementation. | `diff_diff/mmm.py`, `diff_diff/stacked.py` | mmm-interop | Low | | `SyntheticControl` fit-snapshot residency (`_SyntheticControlFitSnapshot`) — **investigated 2026-07-07, parked**: the snapshot ALIASES the fit's own working pivots (zero extra construction cost); the retained residency implements the documented freeze contract (post-fit mutation of estimator inputs must not change `in_space_placebo()` / `leave_one_out()` / conformal output on an already-returned results object, and `__getstate__` already excludes it from pickles). A compact array representation saves only pandas overhead (the float panel dominates); releasing residency needs new API surface (`release`/opt-out flag) or a freeze-contract change. Revisit on user demand for very large donor panels. | `synthetic_control.py`, `synthetic_control_results.py` | follow-up | Low | | Stratified survey-PSU multiplier-weight draw-tiling — **investigated 2026-07-07, parked**: the stratified generator (`generate_survey_multiplier_weights_batch`) consumes ONE sequential rng stream stratum-major (`rng.choice(size=(n_bootstrap, n_h))` per stratum, then lonely-PSU pooling), so draw-chunked assembly CANNOT reproduce the stream bit-identically (contra the old row's parenthetical) — it would need per-stratum generator state skipping (PCG64.advance + per-weight-type variate accounting; fragile) or a stream-layout change (MC-level SE changes → baseline/golden recapture + REGISTRY note). Stratified designs have few PSUs, so the full `(n_bootstrap × n_psu)` matrix rarely matters; unstratified (the large-`n_units` case) is already tiled. Revisit only if a large-PSU stratified design hits memory, as a documented stream change. | `diff_diff/bootstrap_chunking.py::iter_survey_multiplier_weight_blocks` | follow-up | Low | -| ChangesInChanges FULL Melly-Santangelo covariate estimator (monotonized integrated-indicator conditional CDFs, treated-post `F_{X|11}` integration, exchangeable bootstrap with variance-weighted KS bands, tail trimming, pre-period specification test). The simplified qte-`xformla` form of the MS pipeline is already supported (`covariates=` on both estimators, parity-tested vs qte 1.3.1); the FULL estimator has no reference implementation (the MS Stata code is the only one; distinct from Kranker's `cic`) and would need simulation-based validation. Reviewed: `docs/methodology/papers/melly-santangelo-2015-review.md`. | `diff_diff/changes_in_changes.py` | #682 | Low | +| ChangesInChanges FULL Melly-Santangelo covariate estimator (monotonized integrated-indicator conditional CDFs, treated-post `F_{X\|11}` integration, exchangeable bootstrap with variance-weighted KS bands, tail trimming, pre-period specification test). The simplified qte-`xformla` form of the MS pipeline is already supported (`covariates=` on both estimators, parity-tested vs qte 1.3.1); the FULL estimator has no reference implementation (the MS Stata code is the only one; distinct from Kranker's `cic`) and would need simulation-based validation. Reviewed: `docs/methodology/papers/melly-santangelo-2015-review.md`. | `diff_diff/changes_in_changes.py` | #682 | Low | | ChangesInChanges discrete-outcome bounds + DCIC point identification (Athey-Imbens Sections 4/5.2 incl. Imbens-Manski intervals; Kranker's Stata `cic` is the reference). The shipped ties warning marks the boundary of the continuous scope. | `diff_diff/changes_in_changes.py` | #682 | Low | | ChangesInChanges analytical SEs (Athey-Imbens Theorems 5.1-5.3 influence functions, panel 5.5-5.7, Appendix B covariances; needs the footnote-31 boundary density estimator - note the review's suspected half-range/midpoint typo). Bootstrap is the shipped inference. | `diff_diff/changes_in_changes.py` | #682 | Low | | Staggered/multi-period distributional DiD (Athey-Imbens Section 6 / Ciaccio arXiv:2408.01208v2; `ecic` is the staggered event-study CiC lineage - a distinct method from Ciaccio's copula approach, do not conflate). Reviewed: `docs/methodology/papers/ciaccio-2024-review.md`; ROADMAP row is reviewed-deferred pending demand. | `diff_diff/changes_in_changes.py` | #682 | Low | @@ -144,7 +144,7 @@ decisions (refactor waivers, perf trade-offs, test-infrastructure calls) are rec | **Fixed-B Stata SE-golden comparisons in the LWDiD suite run unmarked in default CI** (no `ci_params.bootstrap()` scaling, no `slow` marker): the SE tolerance derives from BOTH fixed rep counts (ours B=999, Stata's R from the golden meta) so scaling either side would invalidate the committed-golden comparison — the ci_params convention governs convergence-style tests, not fixed-B golden comparisons — and these tests are the PR #588 acceptance bar, which must run in default CI; module-scoped fit memoization bounds the cost (~1-2 s per fit locally) | `tests/test_methodology_lwdid.py` | 2026-08-16 | | **The 4.0 migration guide's code blocks are not snippet-executed.** `tests/test_doc_snippets.py` discovers a hardcoded list of `.rst` files and only `.. code-block:: python` / RST `::` bodies, so `docs/migration-4.0.md` gets no coverage. Deliberate: the guide is a MIXED document - most "after" examples (the renames, `results.att`) run on the current release, but the `field-flip` and `df-convention-flip` examples describe 4.0 behaviour that cannot run until 4.0, so a blanket execution lane would fail by construction. Closing the gap fully means a markdown-fence extractor plus a skip-marker convention for the future-API blocks - a harness change, out of scope for a docs PR. What IS gated: the appendix's ledger parity (`test_migration_guide_*`), which pins the row set and every mechanically checkable cell; and, since the first local review found all three merge examples carrying invalid keywords, `test_migration_guide_examples_bind_to_real_signatures`, which ast-parses the guide's python blocks and asserts every constructor/`fit()` keyword exists on the target signature. That is signature binding, NOT execution - it deliberately skips calls whose owner it cannot resolve (e.g. `results.aggregate(...)`), and it cannot catch a wrong VALUE or a wrong sequence of calls. The hand-written `Fix` prose remains unverifiable by any available means. | `docs/migration-4.0.md`, `tests/test_doc_snippets.py` | Phase 4 / 2026-08-09 | | **MMM launch carousel carries scoped marketing claims, not exporter-contract documentation.** The deck (a LinkedIn marketing artifact, not a docs surface) states capabilities in scoped-but-punchy form: guardrail copy says "the easy mistakes fail loudly" / "you own the design, it owns the math" (the caller-owned estimand/population/window/outcome-scale alignment lives in REGISTRY.md's MMM section and the exporter docstrings, not on slides), and slide 8 shows tutorial 29's worked staggered-boost lift row WITHOUT an on-slide linearity qualifier - the compression's linear-channel scoping is the tutorial's job, which the CTA points to. Absolutes ("no silent mis-calibration", "any estimate exports", "anything with an estimate + SE") are ban-listed by `tests/test_mmm_carousel_claims.py`; local review R1/R2 pressed for compliance-style scoping language on the slides and the marketing-appropriate scoped copy was chosen instead. | `carousel/generate_mmm_carousel.py`, `tests/test_mmm_carousel_claims.py` | mmm-carousel / 2026-08-20 | -| **MultiPeriodDiD deprecation shim loses static constructor-arg checking (3.9 window).** The M-010 shim is `__init__(*args, **kwargs)` + an import-time `__signature__` mirror of DiD's constructor: runtime introspection (get_params/set_params, `inspect.signature`) and eager validation are fully preserved, but static type checkers / IDEs cannot check constructor arguments for the deprecated class until its 4.0 removal. Accepted: the alternative (hand-mirroring ~20 parameters) is a drift magnet on a class with one minor version of remaining life. | `diff_diff/estimators.py` | 3(a) / 2026-08-07 | +| **MultiPeriodDiD deprecation shim loses static constructor-arg checking (3.9 window).** The M-010 shim is `__init__(*args, **kwargs)` + an import-time `__signature__` mirror of DiD's constructor: runtime introspection (get_params/set_params, `inspect.signature`) and eager validation are fully preserved, but static type checkers / IDEs cannot check constructor arguments for the deprecated class until the ledger-tracked M-010 removal. Accepted: the alternative (hand-mirroring ~20 parameters) is a drift magnet on a class with one minor version of remaining life. | `diff_diff/estimators.py` | 3(a) / 2026-08-07 | | **DCDH `sklearn.base.clone` param-identity failure won't-fix.** `ChaisemartinDHaultfoeuille._validate_paths_of_interest` unconditionally canonicalizes `paths_of_interest` into a fresh `List[Tuple[int, ...]]`, so sklearn `clone()`'s post-construction `param1 is param2` identity check fails for configured instances - a pre-existing normalization the BaseEstimator mixin PR documented rather than changed (get_params/set_params signatures are clone-compatible; the dependency-free `cls(**est.get_params())` config-equality contract is the enforced one, `tests/test_base_estimator.py`). Fixing would mean returning the caller's raw object from a validator whose job is canonicalization. | `chaisemartin_dhaultfoeuille.py` | mixin PR / 2026-08-01 | | **scikit-learn stays out of dev deps; clone-identity tests remain importorskip-only.** The sklearn-`clone()` round-trip tests (`test_base_estimator.py`, had/rdd/cic suites) run only where scikit-learn happens to be installed - deliberate, matching the numpy/pandas/scipy-only dependency posture; the always-running contract is the dependency-free re-instantiation config-equality test. | `tests/test_base_estimator.py` | mixin PR / 2026-08-01 | | **Plan-review hash gate threat model: accident prevention, NOT adversarial defense.** The ExitPlanMode content-hash gate (hook + `plan_snapshot.py`) exists to stop accidents — stale approvals, concurrent-worktree cross-talk, plans edited mid-review — all of which it closes by construction (snapshot identity + invocation-unique state tokens, 30+ behavioral tests). It does NOT and cannot defend against a malicious local process: nothing verifies review AUTHORSHIP, and such an actor can simply write a matching review file directly — no userland hook can prevent that short of signed reviews, which is out of scope for a personal workflow aid. Review findings that presuppose a hostile local actor against this gate are waived by this decision (2026-07-20, after 7 local AI-review rounds converged on ever-deeper adversarial-model refinements with no reachable fixpoint). Genuine accident vectors remain in scope and are fixed as found. | `.claude/hooks/check-plan-review.py`, `.claude/scripts/plan_snapshot.py` | 2026-07-20 | diff --git a/TODO.md b/TODO.md index b8152d2db..867c33744 100644 --- a/TODO.md +++ b/TODO.md @@ -83,14 +83,11 @@ generic sparse-FE, QR+SVD rank-detection redundancy, `check_finite` bypass — m | Committed `fixest::feols` event-study golden for TWFE `event_study=True` (within + pooled specs, unbalanced + covariate panels, matched CR1 cluster convention, per-period effects + vcov block) - the in-suite gates are shared-core cross-checks (TWFE-within == MPD-absorb, pooled == MPD bit-exact), so a defect common to the shared core would pass; the live-R harness (`benchmarks/R/benchmark_multiperiod.R`, `feols(y ~ treated * time_f \| unit)`) validated the within design in `docs/benchmarks.rst` but is not a committed regression test - follow the `fixest_did_twfe_golden.json` committed-golden pattern (pytest.skip when absent) | `tests/test_fixest_did_twfe_parity.py`, `benchmarks/R/` | 3(a) R2 | Mid | Medium | | Evaluate adding the `BaseEstimator` param surface (get_params/set_params) to the exported classes that never had it - `PowerAnalysis`, `LinearRegression`, `BusinessReport`, `DiagnosticReport`, `TWFEWeightsResult` (a NEW public surface, deliberately out of the 2(c)-i pure-refactor scope; `LinearRegression` is the one `fit`-bearing class excluded from the contract suite's roster-completeness test). | `diff_diff/linalg.py`, `diff_diff/power.py` | mixin PR | Mid | Low | | Tighten the mypy suppressions that back the enforced-zero posture: burn down `prep_dgp`'s per-module `[index]` override (needs a None-vs-array restructure that preserves the seeded RNG stream), and evaluate re-enabling the globally disabled codes (`arg-type`, `return-value`, `var-annotated`, `assignment`) one at a time — `assignment` alone hid several real annotation drifts found during the 2026-07 triage. | `pyproject.toml` `[tool.mypy]`, `diff_diff/prep_dgp.py` | lint-CI | Mid | Low | -| Tracking-file contract guard test: reject NEW active deferred-work pointers at `TODO.md` (deferred rows live in `DEFERRED.md`; allowlist for historical/past-tense prose and actionable-row pointers) and assert rows cross-linking a `docs/v4-deprecations.yaml` `M-xxx` id don't restate ledger status. Origin: tracking-split local review R2. | `tests/`, `TODO.md`, `DEFERRED.md` | tracking-split | Quick | Low | -| Real-data CI canary for dataset-backed replication tests: `test_methodology_lwdid.py`'s Prop 99 / Walmart goldens skip (visibly) when loaders fall back to synthetic; add a lane or canary asserting `df.attrs["source"] == "lwdid_ssc_ancillary"` in CI so network regressions cannot silently de-gate the replication tests. Follow-on from the loader-fallback repair (#723), which made provenance explicit but deliberately did not add a network-dependent CI lane. | `tests/test_methodology_lwdid.py`, `.github/workflows/` | LWDiD validation suite | Quick | Low | | Route LWDiD into the practitioner code surfaces: `practitioner_next_steps` (`diff_diff/practitioner.py`), `agent_workflow.py`, `DiagnosticReport`, and `BusinessReport` never dispatch to or mention LWDiD — the AI guides now cover it, but the code surfaces need contract decisions (when to recommend it, what diagnostics to run) plus tests | `diff_diff/practitioner.py`, `diff_diff/agent_workflow.py`, `diff_diff/diagnostic_report.py`, `diff_diff/business_report.py` | LWDiD release audit | Mid | Low | | df-provenance completion follow-up (PR C descoping): a Wooldridge public per-row df channel (a per-key dict that can also carry the hc2_bm BM contrast dofs the scalar `_df_analytic_fallback` cannot represent — the unified surface's Wooldridge df column stays NaN until then); ImputationDiD NON-survey per-lead df (the survey-fit scalar `event_study_df` + its bootstrap clearing shipped with the M-092 completion; what remains is surfacing the `resolve_tail_df`-derived per-lead dfs on non-survey fits, which needs the `_compute_lead_coefficients` return-path change); and the `inference_df` (DiD/MPD/SA/StackedDiD) vs `df_inference` (CS/dCDH) adapter-naming unification. NOTE (M-024): StackedDiD's `df_survey=None` on analytical fits is DELIBERATE - the container resolver's `df_inference` fallback not matching `inference_df` is load-bearing (normal-theory FLCI; see the REGISTRY StackedDiD M-024 Note and the pin in `tests/test_event_study_consumers.py`) - the unification must preserve that outcome or renegotiate it explicitly. | `diff_diff/wooldridge_results.py`, `diff_diff/imputation.py`, `diff_diff/results_base.py` | #variance-PR-C | Mid | Low | | `worktree-rm` safety via a tested argv helper: the prose rewrite (ask-before-remove confirmation gate, detached-HEAD reachability/rescue, tip-identity force-delete guard) was reverted to the main version because editing the prose repeatedly reintroduced shell-injection (last: sourcing a state file built from a git-derived branch name). Restore those guards in a `worktree_rm.py` that takes the name via file ingress, invokes git through argv arrays, and has a metacharacter-branch/path injection regression test — the pattern that worked for `pr_prepare.py`/`premerge_scan.py`. | `.claude/commands/worktree-rm.md`, `.claude/scripts/` | skill-audit | Heavy | Medium | | `premerge_scan.py` should scan the staged blob (`git show :path`) for staged methodology files, not the working-tree copy — a stage-then-revert-working-copy edit currently reads the safe working version and misses the staged violation. Union staged-index findings with unstaged/untracked filesystem findings. | `.claude/scripts/premerge_scan.py` | skill-audit | Mid | Low | | Carousel PDF drift guard: the committed deck PDFs are build artifacts of their `generate_*.py` scripts (README rule: regenerate in the same commit), but nothing verifies a committed PDF matches its generator — a source edit without regeneration, or a stale raster chart, passes every claims test. Applies to ALL decks, not just the MMM one. fpdf embeds creation-date metadata, so a byte-compare needs fixed metadata or a per-page rendered-text manifest. Origin: mmm-carousel local review R1. | `carousel/`, `tests/` | mmm-carousel | Mid | Low | -| Re-add committed-range methodology scanning to `/push-pr-update` §3b (clean tree, commits ahead) using `premerge_scan.py --range`, with the comparison ref passed as **data** (resolved into a quoted variable in one Bash call, never a raw ``). It was removed to avoid ref interpolation; the helper already implements and tests `--range`. | `.claude/commands/push-pr-update.md` | skill-audit | Quick | Low | | Codex reviewer isolation (repo-wide): `codex --sandbox read-only` blocks writes but does NOT confine READS to the worktree/repo (verified: a probe read `/etc/hosts`), and prompt guards are not a security boundary. Affects `/ai-review-local` (which already documents and accepts this surface) and plan-review-eval dual arms equally. Evaluate OS-level isolation for codex invocations (container / `sandbox-exec` profile / dedicated low-privilege account exposing only the worktree). | `.claude/scripts/openai_review.py`, `tools/plan-review-eval/` | plan-review-eval local review R6 | Heavy | Medium | | plan-review-eval dual arms: terminate the surviving peer subprocess when the first reviewer of a dual pair fails (today an early Claude failure still waits out the codex peer — up to its 3600s ceiling — before the run becomes INFRA_ERROR; needs process handles threaded through `_call_claude`/`call_codex` or a shared cancellation event). | `tools/plan-review-eval/plan_adapters/plan_reviewer.py` | plan-review-eval local review R3 | Mid | Low | | Reimplement the retired `/review-plan` `--updated` (delta re-review, `## Delta Assessment` section) and `--pr ` (PR-comment feedback-coverage) modes in the plan-review skill — descoped from the initial skill ship (the advertised-but-unimplemented flags were removed). `--updated` needs prior-review load + a delta diff; `--pr` needs a `gh` comment fetch + a new reviewer-prompt token. | `.claude/skills/plan-review/SKILL.md` | plan-review skill local review | Mid | Low | diff --git a/changelog.d/20260830-infra-process-sweep.md b/changelog.d/20260830-infra-process-sweep.md new file mode 100644 index 000000000..619b72764 --- /dev/null +++ b/changelog.d/20260830-infra-process-sweep.md @@ -0,0 +1,13 @@ +### Internal +- **Repo-process tooling** (infra sweep, part 2): a weekly `LWDiD Data + Canary` CI lane that fails loudly when the SHA-pinned Prop 99 / Walmart + loaders fall back to synthetic data (previously a visible-but-green test + skip), then runs the replication tests it de-gates; a + `tests/test_tracking_files.py` contract guard for TODO.md/DEFERRED.md + (no deferred-work pointers in TODO rows, no ledger-lifecycle + restatements beside `M-xxx` cross-links, documented table shapes with + per-row column counts — which also surfaced and fixed two DEFERRED.md + rows whose unescaped pipes broke the rendered tables and one row + restating M-010's version target); and the `/push-pr-update` + committed-range methodology scan restored via `premerge_scan.py --range` + with the comparison ref passed as quoted data. diff --git a/tests/test_changelog_fragments.py b/tests/test_changelog_fragments.py index c611ff533..e840cd5d5 100644 --- a/tests/test_changelog_fragments.py +++ b/tests/test_changelog_fragments.py @@ -21,6 +21,7 @@ import pathlib import shutil import subprocess +from pathlib import Path import pytest @@ -82,6 +83,7 @@ def mod(): def make_repo(tmp_path, changelog=MINIMAL_CHANGELOG, fragments=None, readme=True): + tmp_path.mkdir(parents=True, exist_ok=True) d = tmp_path / "changelog.d" d.mkdir() if readme: @@ -177,6 +179,9 @@ class TestCheck: "20269999-impossible-date.md", "20260830-UPPER.md", "20260830-slug.txt", + "20260830-topic-.md", + "20260830-topic--detail.md", + "20260830--topic.md", ], ) def test_bad_fragment_names(self, mod, tmp_path, name): @@ -194,6 +199,13 @@ def test_bad_fragment_names(self, mod, tmp_path, name): ("### Fixed\n- ok\n## [9.9.9]\n", "'## ' header"), ("- floating bullet\n### Fixed\n- ok\n", "outside any"), ("### Fixed\nnot a bullet\n", "not a top-level bullet"), + # CommonMark accepts ATX headings indented 1-3 spaces; they must + # not slip through as "continuation" lines (they render as real + # headings the compiler's column-zero scanners never see). + ("### Fixed\n ## [9.9.9] - 2099-01-01\n- entry\n", "indented Markdown heading"), + ("### Fixed\n ## smuggled\n- entry\n", "indented Markdown heading"), + ("### Fixed\n ## smuggled\n- entry\n", "indented Markdown heading"), + ("### Fixed\n- ok\n ### Unknown\n", "indented Markdown heading"), ], ) def test_bad_fragment_bodies(self, mod, tmp_path, body, needle): @@ -369,6 +381,41 @@ def test_exit4_requires_nonempty_section(self, mod, tmp_path): root = make_repo(tmp_path, changelog=changelog) assert run_compile(mod, root, version="1.2.0") == 1 + def test_exit4_rejects_orphan_bullet_before_category(self, mod, tmp_path): + # Any-header + any-bullet independently is not enough: an orphan + # bullet above a (now empty) valid category is a state compile + # could not have produced - the section grammar must reject it. + changelog = MINIMAL_CHANGELOG.replace( + "## [1.2.0] - 2026-01-15\n\n### Added\n- old entry\n", + "## [1.2.0] - 2026-01-15\n\n- orphan bullet\n\n### Added\n", + ) + root = make_repo(tmp_path, changelog=changelog) + assert run_compile(mod, root, version="1.2.0") == 1 + + def test_exit4_rejects_bullet_only_under_unknown_category(self, mod, tmp_path): + changelog = MINIMAL_CHANGELOG.replace( + "## [1.2.0] - 2026-01-15\n\n### Added\n- old entry\n", + "## [1.2.0] - 2026-01-15\n\n### Added\n\n### Unknown\n- smuggled\n", + ) + root = make_repo(tmp_path, changelog=changelog) + assert run_compile(mod, root, version="1.2.0") == 1 + + def test_indented_heading_fragment_cannot_compile(self, mod, tmp_path): + frag = "### Fixed\n ## [9.9.9] - 2099-01-01\n- entry\n" + root = make_repo(tmp_path, fragments={"20260830-x.md": frag}) + assert run_compile(mod, root) == 1 + # Nothing consumed, nothing written. + assert (root / "changelog.d" / "20260830-x.md").exists() + assert "9.9.9" not in (root / "CHANGELOG.md").read_text() + + def test_exit4_rejects_indented_heading_in_section(self, mod, tmp_path): + changelog = MINIMAL_CHANGELOG.replace( + "## [1.2.0] - 2026-01-15\n\n### Added\n- old entry\n", + "## [1.2.0] - 2026-01-15\n\n### Added\n- old entry\n ## smuggled\n", + ) + root = make_repo(tmp_path, changelog=changelog) + assert run_compile(mod, root, version="1.2.0") == 1 + def test_exit4_requires_dated_header(self, mod, tmp_path): changelog = MINIMAL_CHANGELOG.replace("## [1.2.0] - 2026-01-15", "## [1.2.0]") root = make_repo(tmp_path, changelog=changelog) @@ -393,6 +440,129 @@ def test_compile_refuses_duplicate_unreleased_headers(self, mod, tmp_path): assert run_compile(mod, root) == 1 assert (root / "changelog.d" / "20260830-x.md").exists() + def test_previous_version_mismatch_leaves_state_untouched(self, mod, tmp_path): + # bump-version passes its package-metadata OLD_VERSION; a drifted + # changelog (latest release != OLD_VERSION) must refuse before any + # write or fragment deletion. + root = make_repo(tmp_path, fragments={"20260830-x.md": GOOD_FRAGMENT}) + before = (root / "CHANGELOG.md").read_text() + rc = mod.run_compile(root, "1.3.0", "2026-08-30", True, previous_version="9.9.9") + assert rc == 1 + assert (root / "CHANGELOG.md").read_text() == before + assert (root / "changelog.d" / "20260830-x.md").exists() + # Matching previous-version compiles normally. + assert mod.run_compile(root, "1.3.0", "2026-08-30", True, previous_version="1.2.0") == 0 + # Interrupted-bump recovery: re-running with the SAME (now stale) + # previous-version must reach the idempotent exit-4 path, not the + # drift rejection (compile finished; package metadata not yet + # bumped). + assert mod.run_compile(root, "1.3.0", "2026-08-30", True, previous_version="1.2.0") == 4 + # A completed bump re-run (metadata already at the target) also + # certifies; an unrelated value is drift. + assert mod.run_compile(root, "1.3.0", "2026-08-30", True, previous_version="1.3.0") == 4 + assert mod.run_compile(root, "1.3.0", "2026-08-30", True, previous_version="9.9.9") == 1 + + def test_dateless_prev_anchor_rejected_legacy_exempt(self, mod, tmp_path): + # The ANCHOR (latest release) must carry a canonical date; older + # legacy headers without dates stay legal (the real CHANGELOG has + # pre-convention dateless releases). + frag = {"20260830-x.md": GOOD_FRAGMENT} + changelog = MINIMAL_CHANGELOG.replace("## [1.2.0] - 2026-01-15", "## [1.2.0]") + root = make_repo(tmp_path / "dateless", changelog=changelog, fragments=frag) + assert run_compile(mod, root, version="1.3.0") == 1 + # Impossible date on the anchor is equally rejected. + changelog = MINIMAL_CHANGELOG.replace("2026-01-15", "2026-99-99") + root = make_repo(tmp_path / "impossible", changelog=changelog, fragments=frag) + assert run_compile(mod, root, version="1.3.0") == 1 + # A dateless OLDER header does not block (legacy exemption). + changelog = MINIMAL_CHANGELOG.replace("## [1.1.0] - 2026-01-01", "## [1.1.0]") + root = make_repo(tmp_path / "legacy", changelog=changelog, fragments=frag) + assert run_compile(mod, root, version="1.3.0") == 0 + + def test_alternate_whitespace_target_link_detected(self, mod, tmp_path): + # A pre-existing target link with extra whitespace after ':' must + # still trip the duplicate-definition refusal. + changelog = MINIMAL_CHANGELOG + "[1.3.0]: https://example.com/compare/v1.2.0...v1.3.0\n" + root = make_repo(tmp_path, changelog=changelog, fragments={"20260830-x.md": GOOD_FRAGMENT}) + assert run_compile(mod, root, version="1.3.0") == 1 + + def test_preexisting_target_link_definition_rejected(self, mod, tmp_path): + # A '[X.Y.Z]:' link definition without its release header is an + # inconsistent state; compiling would emit a DUPLICATE definition. + changelog = MINIMAL_CHANGELOG + "[1.3.0]: https://example.com/compare/v1.2.0...v1.3.0\n" + root = make_repo(tmp_path, changelog=changelog, fragments={"20260830-x.md": GOOD_FRAGMENT}) + assert run_compile(mod, root, version="1.3.0") == 1 + assert (root / "changelog.d" / "20260830-x.md").exists() + + def test_exit4_rejects_duplicate_link_definitions(self, mod, tmp_path): + # One correct + one wrong definition for the target: the duplicate + # check now runs BEFORE the existing-target path, so exit 4 cannot + # certify the ambiguous state. + changelog = MINIMAL_CHANGELOG.replace( + "## [1.2.0] - 2026-01-15", + "## [1.3.0] - 2026-01-16\n\n### Fixed\n- prior.\n\n## [1.2.0] - 2026-01-15", + ) + changelog = changelog.replace( + "[1.2.0]:", + "[1.3.0]: https://example.com/compare/v1.2.0...v1.3.0\n" + "[1.3.0]: https://evil.example/compare/v1.2.0...v1.3.0\n[1.2.0]:", + 1, + ) + root = make_repo(tmp_path, changelog=changelog) + assert run_compile(mod, root, version="1.3.0") == 1 + + def test_exit4_requires_predecessor_link(self, mod, tmp_path): + # No link definition for the predecessor: never fall back to + # accepting any base - refuse (not a compiler-produced state). + changelog = MINIMAL_CHANGELOG.replace( + "## [1.2.0] - 2026-01-15", + "## [1.3.0] - 2026-01-16\n\n### Fixed\n- prior.\n\n## [1.2.0] - 2026-01-15", + ) + changelog = changelog.replace( + "[1.2.0]:", "[1.3.0]: https://example.com/compare/v1.2.0...v1.3.0\n[oops-1.2.0]:", 1 + ) + root = make_repo(tmp_path, changelog=changelog) + assert run_compile(mod, root, version="1.3.0") == 1 + + def test_exit4_rejects_wrong_base_target_link(self, mod, tmp_path): + # Correct predecessor/target versions but an UNRELATED repository + # base: exit 4 must not certify this as already-compiled. + changelog = MINIMAL_CHANGELOG.replace( + "## [1.2.0] - 2026-01-15", + "## [1.3.0] - 2026-01-16\n\n### Fixed\n- prior.\n\n## [1.2.0] - 2026-01-15", + ) + changelog = changelog.replace( + "[1.2.0]:", + "[1.3.0]: https://evil.example/compare/v1.2.0...v1.3.0\n[1.2.0]:", + 1, + ) + root = make_repo(tmp_path, changelog=changelog) + assert run_compile(mod, root, version="1.3.0") == 1 + + def test_malformed_release_header_rejected(self, mod, tmp_path): + # A release-LIKE header the canonical grammar does not match (extra + # suffix) is invisible to _existing_headers, so compiling that + # version would otherwise add a SECOND canonical section beside the + # malformed one; the compiler must refuse before touching anything. + changelog = MINIMAL_CHANGELOG.replace( + "## [1.2.0] - 2026-01-15", "## [1.2.0] - 2026-01-15 draft" + ) + root = make_repo(tmp_path, changelog=changelog, fragments={"20260830-x.md": GOOD_FRAGMENT}) + before = (root / "CHANGELOG.md").read_text() + assert run_compile(mod, root, version="1.2.0") == 1 + assert (root / "changelog.d" / "20260830-x.md").exists() + assert (root / "CHANGELOG.md").read_text() == before # untouched + + def test_mixed_canonical_and_malformed_duplicate_rejected(self, mod, tmp_path): + # Canonical 1.2.0 + a malformed 1.2.0 twin: the malformed-header + # rejection fires (the duplicate check alone cannot see the twin). + changelog = MINIMAL_CHANGELOG.replace( + "## [1.1.0] - 2026-01-01\n", + "## [1.2.0] - 2026-01-15 rc1\n\n### Fixed\n- twin\n\n" "## [1.1.0] - 2026-01-01\n", + ) + root = make_repo(tmp_path, changelog=changelog) + assert run_compile(mod, root, version="1.3.0") == 1 + def test_duplicate_release_headers_rejected_not_certified(self, mod, tmp_path): # Two '## [1.2.0]' sections with a nonempty first section and a valid # comparison link: exit 4 must NOT certify this (the loop inspects @@ -466,6 +636,59 @@ def test_dirty_guard(self, mod, tmp_path, git_commit_all): assert run_compile(mod, root, allow_dirty=False) == 1 assert run_compile(mod, root, allow_dirty=True) == 0 + def test_dirty_guard_catches_deleted_tracked_fragment(self, mod, tmp_path, git_commit_all): + # A committed fragment DELETED from the worktree must refuse: the + # compile would otherwise ship only the surviving fragment while + # the git diff deletes both - silently losing a release note. + root = make_repo( + tmp_path, + fragments={"20260830-a.md": GOOD_FRAGMENT, "20260830-b.md": GOOD_FRAGMENT}, + ) + git_commit_all(root) + (root / "changelog.d" / "20260830-a.md").unlink() + before = (root / "CHANGELOG.md").read_text() + assert run_compile(mod, root, allow_dirty=False) == 1 + assert (root / "CHANGELOG.md").read_text() == before + assert (root / "changelog.d" / "20260830-b.md").exists() + + def test_compile_rolls_back_on_unlink_failure(self, mod, tmp_path, monkeypatch): + # Transactionality: if fragment deletion fails after CHANGELOG.md + # was written, both the changelog and every fragment are restored. + root = make_repo( + tmp_path, + fragments={"20260830-a.md": GOOD_FRAGMENT, "20260830-b.md": GOOD_FRAGMENT}, + ) + before = (root / "CHANGELOG.md").read_text() + real_unlink = Path.unlink + calls = {"n": 0} + + def failing_unlink(self, *a, **k): + if self.suffix == ".md" and self.parent.name == "changelog.d": + calls["n"] += 1 + if calls["n"] == 2: + raise OSError("simulated unlink failure") + return real_unlink(self, *a, **k) + + monkeypatch.setattr(Path, "unlink", failing_unlink) + assert run_compile(mod, root, allow_dirty=True) == 1 + assert (root / "CHANGELOG.md").read_text() == before + assert (root / "changelog.d" / "20260830-a.md").exists() + assert (root / "changelog.d" / "20260830-b.md").exists() + + def test_second_release_over_tag_link_anchor(self, mod, tmp_path): + # A repo whose sole prior release carries the conventional + # releases/tag link must be able to compile its second release. + changelog = ( + "# Changelog\n\n## [Unreleased]\n\n" + + POINTER + + "\n\n## [1.0.0] - 2026-01-01\n\n### Added\n- first.\n\n" + "[1.0.0]: https://example.com/releases/tag/v1.0.0\n" + ) + root = make_repo(tmp_path, changelog=changelog, fragments={"20260830-x.md": GOOD_FRAGMENT}) + assert run_compile(mod, root, version="1.1.0") == 0 + out = (root / "CHANGELOG.md").read_text() + assert "[1.1.0]: https://example.com/compare/v1.0.0...v1.1.0" in out + def test_dirty_guard_catches_gitignored_fragment(self, mod, tmp_path, git_commit_all): # `git status` never sees ignored untracked files; the guard is a # committed-content check instead, so a fragment hidden by diff --git a/tests/test_methodology_lwdid.py b/tests/test_methodology_lwdid.py index 8bdc7bf3e..1b959e959 100644 --- a/tests/test_methodology_lwdid.py +++ b/tests/test_methodology_lwdid.py @@ -187,9 +187,10 @@ def _real_prop99(): """Load the real Prop 99 panel or skip (offline / synthetic fallback). Skips are VISIBLE (not silent): CI runners have network and the loaders - are SHA-256-pinned, so these tests run in practice; a dedicated - real-data canary lane is tracked in TODO.md alongside the legacy-loader - fallback repair. + are SHA-256-pinned, so these tests run in practice; the dedicated + real-data canary lane (`.github/workflows/lwdid-data-canary.yml`, + weekly cron) fails LOUDLY when either loader is on its synthetic + fallback, so a network/URL/sha regression cannot stay green. """ df = load_prop99() if df.attrs.get("source") != "lwdid_ssc_ancillary": @@ -198,7 +199,10 @@ def _real_prop99(): def _real_walmart(): - """Load the real Walmart panel or skip (offline / synthetic fallback).""" + """Load the real Walmart panel or skip (offline / synthetic fallback). + + Same skip contract and canary coverage as ``_real_prop99``. + """ df = load_walmart() if df.attrs.get("source") != "lwdid_ssc_ancillary": pytest.skip("real Walmart data unavailable (synthetic fallback in use)") diff --git a/tests/test_tracking_files.py b/tests/test_tracking_files.py new file mode 100644 index 000000000..b262315d5 --- /dev/null +++ b/tests/test_tracking_files.py @@ -0,0 +1,450 @@ +"""Tracking-file contract guards for TODO.md / DEFERRED.md. + +Origin: tracking-split local review R2 (TODO row). The split's contract: + +- TODO.md is the ACTIONABLE backlog; deferred/blocked work lives in + DEFERRED.md. A TODO table row that points its work at DEFERRED.md is a + misfiled deferral (prose links — the header's "Related tracking surfaces" + block and past-tense "moved to" notes — are legal and are not table rows). +- Lifecycle state (deprecated_in / removed_in / status) for `M-xxx` ledger + ids lives ONLY in the CI-enforced ``docs/v4-deprecations.yaml``; tracking + rows cross-link ids but must not restate ledger fields (which would go + stale silently — the ledger tests probe the yaml, not markdown prose). +- The table shapes are documented contract (CLAUDE.md "Tracking-file map"): + TODO Actionable rows carry an Effort column, DEFERRED blocker rows a PR + column, and the DEFERRED Decision record has its own three-column shape. +""" + +import re +from pathlib import Path + +import pytest + +REPO_ROOT = Path(__file__).resolve().parents[1] +TODO = REPO_ROOT / "TODO.md" +DEFERRED = REPO_ROOT / "DEFERRED.md" + +# Isolated-install CI runs from a wheel with no tracking files; these guards +# only make sense against a full checkout (the test_docs_ia.py pattern). +pytestmark = pytest.mark.skipif( + not TODO.exists() or not DEFERRED.exists(), + reason="tracking files not present (isolated-install CI)", +) + +TODO_HEADER = "| Issue | Location | Origin | Effort | Priority |" +DEFERRED_BLOCKER_HEADER = "| Issue | Location | PR | Priority |" +DEFERRED_DECISION_HEADER = "| Decision | Location | Verified |" + +_HEADER_SET = {TODO_HEADER, DEFERRED_BLOCKER_HEADER, DEFERRED_DECISION_HEADER} + +# Ledger-lifecycle restatements forbidden in rows that cross-link an M-id. +# Field-name tokens (the yaml spellings: status/deprecated_in/removed_in) +# plus " in " prose forms. Deliberately NARROW: +# "the M-011 removal" (naming a transition, no version) and "Lifecycle +# tracked in docs/v4-deprecations.yaml (M-008)" (a pointer) are legal; +# DEFERRED's Version-gated section may say "at v4" (a section-level frame, +# not a per-row field restatement). +# ``status:`` keeps the colon OUTSIDE a trailing word boundary (":" is not a +# word character, so ``status:\b`` would never match "status: planned"); +# the version form accepts an optional ``v`` and Markdown delimiters +# (``removed in v4``, ``deprecated in **3.9**``). +# Field-name forms: bare, backticked/emphasized, and colon/equals +# assignment spellings (``status: planned``, `` `status` = planned``). +_LEDGER_TOKENS = re.compile( + # ``status`` needs an assignment marker (the bare word is common prose); + # the underscore field names are ledger vocabulary on their own. + r"[`*_]*\bstatus\b[`*_]*\s*[:=]|\b(?:deprecated_in|removed_in)\b", + re.I, +) +# " " prose (in/at/for/by/with), the +# version-before-noun form ("its 4.0 removal", "the v4 deprecation"), and +# the noun--version form ("removal scheduled for v4"). The (? bool: + """True when a row restates ledger lifecycle state (forbidden beside + an M-xxx cross-link).""" + return bool(_LEDGER_TOKENS.search(line) or _LIFECYCLE_VERSION.search(line)) + + +_CELL_SPLIT = re.compile(r"(? str: + """Canonical pipe-row form: single spaces around cell text. + + Markdown accepts ``|Issue|Location|`` as readily as ``| Issue | Location |``; + normalizing keeps the guard from being bypassed by spacing. + """ + return "| " + " | ".join(_cells(row)) + " |" + + +def _is_divider(row: str) -> bool: + return bool(row) and set(row.strip()) <= {"|", "-", ":", " "} and "-" in row + + +def _tables(path: Path): + """Yield (header_lineno, normalized_header, [(lineno, normalized_row)]) + for every contiguous table block. + + A block is any run of ``|``-prefixed lines, OR a run of pipe-containing + lines whose second line is a divider (GFM also accepts tables WITHOUT + leading pipes - ``Issue | Location`` over ``---|---`` - and those must + not bypass the guard; the divider requirement keeps prose that merely + contains a pipe from being misread as a table). + """ + block: list = [] + for i, line in enumerate(path.read_text().split("\n") + [""], 1): + stripped = line.strip() + if stripped.startswith("|") or ("|" in stripped and (block or _looks_tabular(path, i))): + block.append((i, stripped)) + continue + if block: + yield from _flush_block(block) + block = [] + if block: + yield from _flush_block(block) + + +def _looks_tabular(path: Path, lineno: int) -> bool: + """True when the line AFTER ``lineno`` is a divider row (the GFM + no-leading-pipe table shape's signature).""" + lines = path.read_text().split("\n") + return lineno < len(lines) and _is_divider(lines[lineno]) + + +def _flush_block(block): + header_i, header = block[0] + # A no-leading-pipe block only counts as a table when its second line + # is a divider; a leading-pipe block always does (current convention). + if not header.startswith("|") and not (len(block) >= 2 and _is_divider(block[1][1])): + return + # A recognized table must be WELL-FORMED: GFM only renders a table when + # line 2 is a divider whose cell count equals the header's. A pipe-shaped + # block failing that renders as prose, so its rows would silently escape + # every row check below - fail loudly instead of skipping. + if len(block) < 2 or not _is_divider(block[1][1]): + raise AssertionError( + f"line {header_i}: pipe-shaped block has no divider on its second " + f"line - not a rendering table, rows would escape the guard: " + f"{header[:80]!r}" + ) + if len(_cells(block[1][1])) != len(_cells(header)): + raise AssertionError( + f"line {header_i + 1}: table divider has {len(_cells(block[1][1]))} " + f"cells but the header has {len(_cells(header))} - GFM will not " + f"render this as a table: {header[:80]!r}" + ) + rows = [(j, _normalize(ln)) for j, ln in block[1:] if not _is_divider(ln)] + yield header_i, _normalize(header), rows + + +def _table_rows(path: Path): + """Yield (lineno, normalized line) for table DATA rows of every block.""" + for _, _, rows in _tables(path): + yield from rows + + +class TestTodoNoDeferredPointers: + def test_no_table_row_points_work_at_deferred(self): + offenders = [(i, ln[:100]) for i, ln in _table_rows(TODO) if "DEFERRED.md" in ln] + assert not offenders, ( + "TODO.md table rows must not point work at DEFERRED.md - a " + "deferred/blocked item belongs in DEFERRED.md itself (prose " + f"links outside table rows are fine): {offenders}" + ) + + +class TestNoLedgerRestatement: + @pytest.mark.parametrize("path", [TODO, DEFERRED], ids=lambda p: p.name) + def test_m_id_rows_do_not_restate_lifecycle(self, path): + offenders = [] + for i, ln in _table_rows(path): + if not _M_ID.search(ln): + continue + if _restates_lifecycle(ln): + offenders.append((i, ln[:100])) + assert not offenders, ( + f"{path.name} rows cross-linking an M-xxx id must not restate " + "ledger lifecycle fields (status/deprecated_in/removed_in or " + "' in ') - the CI-enforced " + f"docs/v4-deprecations.yaml is the only authority: {offenders}" + ) + + +class TestTableShapes: + def test_todo_headers_are_the_actionable_shape(self): + headers = [(i, h) for i, h, _ in _tables(TODO)] + assert headers, "TODO.md has no tables" + bad = [(i, h) for i, h in headers if h != TODO_HEADER] + assert not bad, f"undocumented TODO.md table header shape(s): {bad}" + + def test_deferred_headers_are_the_documented_shapes(self): + headers = [(i, h) for i, h, _ in _tables(DEFERRED)] + assert headers, "DEFERRED.md has no tables" + bad = [ + (i, h) + for i, h in headers + if h not in (DEFERRED_BLOCKER_HEADER, DEFERRED_DECISION_HEADER) + ] + assert not bad, f"undocumented DEFERRED.md table header shape(s): {bad}" + + +class TestLifecycleRegexCalibration: + """The guard's own detector fixtures: prohibited spellings are caught, + the legitimate current-row shapes are not.""" + + @pytest.mark.parametrize( + "text", + [ + "| M-001 status: planned | x | y |", + "| M-001 removed in v4 | x | y |", + "| M-001 deprecated in **3.9** | x | y |", + "| M-001 removed in 4.0 | x | y |", + "| M-001 deprecated_in restated | x | y |", + "| M-001 shimmed in '3.9' | x | y |", + "| M-001 until its 4.0 removal | x | y |", + "| M-001 `status`: planned | x | y |", + "| M-001 status = planned | x | y |", + "| M-001 the v4 deprecation | x | y |", + "| M-010 will be removed with v4 | x | y |", + "| M-010 removal scheduled for v4 | x | y |", + "| M-010 planned for 4.0 | x | y |", + ], + ) + def test_prohibited_spellings_are_caught(self, text): + assert _restates_lifecycle(text) + + @pytest.mark.parametrize( + "text", + [ + "| interplay with the M-011 removal | x | y |", + "| Lifecycle tracked in docs/v4-deprecations.yaml (M-008) | x | y |", + "| default flip at v4 (M-004) | x | y |", + "| the M-024 renamed key | x | y |", + "| the M-011 removal interplay | x | y |", + ], + ) + def test_legitimate_shapes_pass(self, text): + assert not _restates_lifecycle(text) + + +class TestTableParserCalibration: + """The parser recognizes unspaced pipe rows and unknown headers.""" + + def test_unspaced_rows_are_normalized_and_seen(self, tmp_path): + f = tmp_path / "t.md" + f.write_text( + "|Issue|Location|Origin|Effort|Priority|\n|---|---|---|---|---|\n|work DEFERRED.md|x|y|z|w|\n" + ) + tables = list(_tables(f)) + assert tables[0][1] == TODO_HEADER # unspaced header normalizes + rows = list(_table_rows(f)) + assert rows and "DEFERRED.md" in rows[0][1] + + def test_unknown_header_is_a_distinct_shape(self, tmp_path): + f = tmp_path / "t.md" + f.write_text("| Task | Who |\n|------|-----|\n| a | b |\n") + ((_, header, _),) = _tables(f) + assert header == "| Task | Who |" + assert header not in _HEADER_SET + + +class TestRowShapes: + r"""Every data row carries exactly its table header's column count + (escaped ``\|`` inside cells honored).""" + + @pytest.mark.parametrize("path", [TODO, DEFERRED], ids=lambda p: p.name) + def test_data_rows_match_header_column_count(self, path): + offenders = [] + for _, header, rows in _tables(path): + n = len(_cells(header)) + for i, ln in rows: + if len(_cells(ln)) != n: + offenders.append((i, len(_cells(ln)), n, ln[:80])) + assert not offenders, ( + f"{path.name} rows with a cell count differing from their " + f"header (line, got, want, row): {offenders}" + ) + + def test_missing_and_extra_cells_are_detected(self, tmp_path): + f = tmp_path / "t.md" + f.write_text( + "| Issue | Location | Origin | Effort | Priority |\n" + "|---|---|---|---|---|\n" + "| short row | only two |\n" + "| a | b | c | d | e | extra |\n" + "| ok `a \\| b` cell | x | y | z | w |\n" + ) + ((_, header, rows),) = _tables(f) + n = len(_cells(header)) + counts = [len(_cells(ln)) for _, ln in rows] + assert counts == [2, 6, n] # short, extra, escaped-pipe row intact + + +class TestLifecycleCaseInsensitivity: + @pytest.mark.parametrize( + "text", + [ + "| M-001 Status: planned | x |", + "| M-001 REMOVED_IN: 4.0 | x |", + "| M-001 Deprecated In 3.9 | x |", + ], + ) + def test_mixed_case_restatements_are_caught(self, text): + assert _restates_lifecycle(text) + + +class TestNoLeadingPipeTables: + """GFM tables written without leading pipes are recognized too.""" + + def test_no_leading_pipe_table_is_parsed(self, tmp_path): + f = tmp_path / "t.md" + f.write_text( + "Issue | Location | Origin | Effort | Priority\n" + "------|----------|--------|--------|----------\n" + "work DEFERRED.md | x | y | z | w\n" + ) + tables = list(_tables(f)) + assert tables and tables[0][1] == TODO_HEADER + rows = list(_table_rows(f)) + assert rows and "DEFERRED.md" in rows[0][1] + + def test_prose_with_a_pipe_is_not_a_table(self, tmp_path): + f = tmp_path / "t.md" + f.write_text("some prose with a | pipe in it\nand a second line\n") + assert list(_tables(f)) == [] + + +class TestMalformedTablesFailLoudly: + """A recognized (pipe-shaped) table missing a well-formed divider does + not render in GFM, so its rows would escape every row check - the + parser fails loudly instead of silently skipping the block.""" + + def test_missing_divider_fails(self, tmp_path): + f = tmp_path / "t.md" + f.write_text( + "| Issue | Location | Origin | Effort | Priority |\n" + "| a row with no divider above it | x | y | z | w |\n" + ) + with pytest.raises(AssertionError, match="no divider"): + list(_tables(f)) + + def test_short_divider_fails(self, tmp_path): + f = tmp_path / "t.md" + f.write_text( + "| Issue | Location | Origin | Effort | Priority |\n" + "|---|---|---|\n" + "| a | b | c | d | e |\n" + ) + with pytest.raises(AssertionError, match="3 cells but the header has 5"): + list(_tables(f)) + + def test_no_leading_pipe_short_divider_fails(self, tmp_path): + f = tmp_path / "t.md" + f.write_text("Issue | Location | Origin\n" "------|----------\n" "a | b | c\n") + with pytest.raises(AssertionError, match="2 cells but the header has 3"): + list(_tables(f)) + + @pytest.mark.parametrize("path", [TODO, DEFERRED], ids=lambda p: p.name) + def test_real_tracking_files_are_well_formed(self, path): + # The strict parser raises on a malformed table; consuming every + # block is itself the assertion. + assert list(_tables(path)) + + +class TestWorkflowPins: + """The docs-tests lane is this guard's ONLY CI lane: pin its path + filters and step so a workflow edit cannot silently de-wire it (the + changelog-fragment guard's TestWorkflowPins precedent).""" + + _ENTRIES = [ + "tests/test_tracking_files.py", + "TODO.md", + "DEFERRED.md", + ".github/workflows/lwdid-data-canary.yml", + ] + + @pytest.fixture(scope="class") + def docs_tests_text(self): + path = REPO_ROOT / ".github" / "workflows" / "docs-tests.yml" + if not path.exists(): + pytest.skip("docs-tests.yml not present") + return path.read_text() + + @pytest.mark.parametrize("trigger", ["push:", "pull_request:"]) + def test_path_filters_cover_tracking_surfaces(self, docs_tests_text, trigger): + on_block = docs_tests_text[: docs_tests_text.index("\njobs:")] + tstart = on_block.index(trigger) + others = [ + on_block.index(t) + for t in ("push:", "pull_request:", "schedule:", "workflow_dispatch:") + if t != trigger and t in on_block and on_block.index(t) > tstart + ] + tblock = on_block[tstart : min(others)] if others else on_block[tstart:] + for entry in self._ENTRIES: + assert f"'{entry}'" in tblock, ( + f"docs-tests.yml {trigger} paths filter is missing {entry!r} - " + "a tracking-file-only PR would run no CI guard" + ) + + def test_step_invokes_guard(self, docs_tests_text): + jobs = docs_tests_text[docs_tests_text.index("\njobs:") :] + assert ( + "pytest tests/test_tracking_files.py" in jobs + ), "docs-tests.yml no longer runs the tracking-file guard" + + +class TestCanaryWorkflowPins: + """Pin the LWDiD data-canary lane's load-bearing pieces: it has no PR + trigger, so without these pins a canary-only edit could merge with the + loaders/provenance/replication wiring silently broken and the first + signal would be the next weekly run.""" + + @pytest.fixture(scope="class") + def canary_text(self): + path = REPO_ROOT / ".github" / "workflows" / "lwdid-data-canary.yml" + if not path.exists(): + pytest.skip("lwdid-data-canary.yml not present") + return path.read_text() + + def test_triggers(self, canary_text): + assert "workflow_dispatch:" in canary_text + assert "schedule:" in canary_text and "cron:" in canary_text + + def test_canary_step_wiring(self, canary_text): + for needle in ( + "load_prop99", + "load_walmart", + "lwdid_ssc_ancillary", + "simplefilter('error')", + ): + assert needle in canary_text, f"canary step lost {needle!r}" + + def test_replication_step_and_backend(self, canary_text): + assert "pytest tests/test_methodology_lwdid.py" in canary_text + assert "DIFF_DIFF_BACKEND: python" in canary_text From 79026febbc421a99e2cff9230e89d3ef14666011 Mon Sep 17 00:00:00 2001 From: igerber Date: Sun, 30 Aug 2026 10:16:51 -0400 Subject: [PATCH 2/9] Address CI review: indented CHANGELOG constructs, fail-closed ls-tree, backslash parity - changelog_compile.py: refuse CommonMark-indented (1-3 space) ATX headings and version-link definitions anywhere in CHANGELOG.md - the duplicate/date scans are column-zero anchored, so ' ## [1.3.0]' or ' [1.3.0]: ...' rendered as real constructs while staying invisible to every safeguard (reproduced: compile succeeded leaving two rendered 1.3.0 headings + two link definitions). Regressions on both the fresh and exit-4 paths. - changelog_compile.py: the deleted-fragment guard now fails CLOSED when 'git ls-tree -r HEAD' fails - previously a nonzero return silently skipped the set-equality check and could compile an incomplete release. Regression forces the enumeration to fail and asserts nothing is written or consumed. - test_tracking_files.py: pipe escaping is parity-based ('\|' literal, '\\|' backslash + separator); the fixed-width (? 1}) @@ -571,6 +591,16 @@ def run_compile(root, version, date_s, allow_dirty, previous_version=None): "from the worktree - its release note would be " "silently lost" ) + else: + # Fail CLOSED: an unenumerable HEAD means the deleted-fragment + # guard cannot run, so the dirty state is unverifiable. (An + # unborn HEAD is already refused above — every present fragment + # was flagged "not committed in HEAD".) + problems.append( + "changelog.d: cannot enumerate HEAD's fragments " + f"(git ls-tree failed: {head_res.stderr.strip() or 'unknown error'}) " + "- deleted-fragment check is unverifiable" + ) if problems: print( "error: changelog.d/ fragments must be committed unchanged " diff --git a/.github/workflows/lwdid-data-canary.yml b/.github/workflows/lwdid-data-canary.yml index eb98ab089..2f56bf641 100644 --- a/.github/workflows/lwdid-data-canary.yml +++ b/.github/workflows/lwdid-data-canary.yml @@ -1,5 +1,5 @@ -# Real-data canary for the LWDiD replication tests (TODO row: "Real-data CI -# canary for dataset-backed replication tests"). +# Real-data canary for the LWDiD replication tests (closed the former TODO +# row "Real-data CI canary for dataset-backed replication tests"). # # tests/test_methodology_lwdid.py's Prop 99 / Walmart goldens VISIBLY skip # when the loaders fall back to synthetic data (df.attrs["source"] != diff --git a/tests/test_changelog_fragments.py b/tests/test_changelog_fragments.py index e840cd5d5..c3b7edc85 100644 --- a/tests/test_changelog_fragments.py +++ b/tests/test_changelog_fragments.py @@ -400,6 +400,55 @@ def test_exit4_rejects_bullet_only_under_unknown_category(self, mod, tmp_path): root = make_repo(tmp_path, changelog=changelog) assert run_compile(mod, root, version="1.2.0") == 1 + @pytest.mark.parametrize( + "smuggled", + [ + " ## [9.9.9] - 2099-01-01", + " ## [9.9.9] - 2099-01-01", + " [9.9.9]: https://github.com/x/y/compare/v1.2.0...v9.9.9", + ], + ids=["indented-header-1sp", "indented-header-3sp", "indented-link-def"], + ) + def test_indented_changelog_constructs_refused(self, mod, tmp_path, smuggled): + # CommonMark renders headings/link definitions indented 1-3 spaces, + # but the compiler's duplicate scans are column-zero anchored - an + # indented construct must refuse, on BOTH the fresh and exit-4 paths. + changelog = MINIMAL_CHANGELOG.replace( + "### Added\n- old entry\n", + f"### Added\n- old entry\n{smuggled}\n", + ) + fresh = make_repo( + tmp_path / "fresh", changelog=changelog, fragments={"20260830-x.md": GOOD_FRAGMENT} + ) + assert run_compile(mod, fresh) == 1 + # Refused before writing: no new section, fragment untouched. + assert "## [1.3.0]" not in (fresh / "CHANGELOG.md").read_text() + assert (fresh / "changelog.d" / "20260830-x.md").exists() + exit4 = make_repo(tmp_path / "exit4", changelog=changelog) + assert run_compile(mod, exit4, version="1.2.0") == 1 + + def test_ls_tree_enumeration_failure_fails_closed( + self, mod, tmp_path, monkeypatch, git_commit_all + ): + # A failed HEAD enumeration means the deleted-fragment guard cannot + # run; that must refuse the compile, never silently skip the check. + root = make_repo(tmp_path, fragments={"20260830-x.md": GOOD_FRAGMENT}) + git_commit_all(root) + before = (root / "CHANGELOG.md").read_text() + real_run = mod.subprocess.run + + def failing_run(cmd, *a, **k): + if "ls-tree" in cmd and "-r" in cmd: + # A genuine nonzero-returncode CompletedProcess. + return real_run(["git", "--no-such-flag"], capture_output=True, text=True) + return real_run(cmd, *a, **k) + + monkeypatch.setattr(mod.subprocess, "run", failing_run) + assert run_compile(mod, root, allow_dirty=False) == 1 + # Nothing written, nothing consumed. + assert (root / "CHANGELOG.md").read_text() == before + assert (root / "changelog.d" / "20260830-x.md").exists() + def test_indented_heading_fragment_cannot_compile(self, mod, tmp_path): frag = "### Fixed\n ## [9.9.9] - 2099-01-01\n- entry\n" root = make_repo(tmp_path, fragments={"20260830-x.md": frag}) diff --git a/tests/test_tracking_files.py b/tests/test_tracking_files.py index b262315d5..104683c27 100644 --- a/tests/test_tracking_files.py +++ b/tests/test_tracking_files.py @@ -81,17 +81,40 @@ def _restates_lifecycle(line: str) -> bool: return bool(_LEDGER_TOKENS.search(line) or _LIFECYCLE_VERSION.search(line)) -_CELL_SPLIT = re.compile(r"(? str: @@ -306,6 +329,30 @@ def test_missing_and_extra_cells_are_detected(self, tmp_path): assert counts == [2, 6, n] # short, extra, escaped-pipe row intact +class TestBackslashParity: + r"""Markdown pipe escaping is parity-based: ``\|`` literal pipe, + ``\\|`` literal backslash + SEPARATOR, ``\\\|`` backslash + literal + pipe. A fixed-width lookbehind gets the even runs wrong.""" + + @pytest.mark.parametrize( + "row,expected", + [ + (r"| a \| b | x |", 2), # odd: escaped pipe stays in-cell + (r"| a \\| b | x |", 3), # even: separator after literal backslash + (r"| a \\\| b | x |", 2), # odd again + (r"| a \\\\| b | x |", 3), # even again + ], + ) + def test_backslash_run_parity(self, row, expected): + assert len(_cells(row)) == expected + + def test_trailing_backslash_pipe_parity(self): + # '\\|' at row end: the pipe is a real (trailing) delimiter. + assert _cells(r"| a | b \\|") == ["a", r"b \\"] + # '\|' at row end: escaped pipe belongs to the cell. + assert _cells(r"| a | b \|") == ["a", r"b \|"] + + class TestLifecycleCaseInsensitivity: @pytest.mark.parametrize( "text", From 3cd3845aced36173f3566534bc422ed60a3322ad Mon Sep 17 00:00:00 2001 From: igerber Date: Sun, 30 Aug 2026 10:24:06 -0400 Subject: [PATCH 3/9] Address CI review round 2: heading-spacing bypass, GFM divider grammar, bump-version variable - changelog_compile.py: the malformed-header scan now accepts ANY CommonMark heading spacing and depth ('## [x]', a tab after the hashes, '### [x]') instead of the single-space '## [' shape, so a rendering-but-noncanonical release-like heading can no longer hide from the duplicate/date safeguards; exact-canonical release and Unreleased headers are the only exemptions. Regressions for double-space, tab, and H3 shapes on both fresh and exit-4 paths (real CHANGELOG audited: all 85 bracketed headings canonical). - test_tracking_files.py: _is_divider now enforces the GFM delimiter grammar per cell (':?-+:?') - an empty or colon-only delimiter cell stops the whole table from rendering, so such a row no longer counts as a divider and the well-formedness check raises. Tests for empty, colon-only, and space delimiter cells. - bump-version.md: OLD_VERSION is resolved inside the SAME Bash block that passes --previous-version (variables do not persist across Bash calls; the prose-only 'store as OLD_VERSION' step would have expanded empty and failed every normal release compile). --- .claude/commands/bump-version.md | 14 +++++++++++--- .claude/scripts/changelog_compile.py | 7 ++++++- tests/test_changelog_fragments.py | 22 +++++++++++++++++----- tests/test_tracking_files.py | 24 +++++++++++++++++++++++- 4 files changed, 57 insertions(+), 10 deletions(-) diff --git a/.claude/commands/bump-version.md b/.claude/commands/bump-version.md index f4a0cfd8d..3cd2613d4 100644 --- a/.claude/commands/bump-version.md +++ b/.claude/commands/bump-version.md @@ -35,17 +35,25 @@ Files that need updating: - If invalid, ask user to provide a valid version 2. **Get current version**: - - Read `diff_diff/__init__.py` and extract the current `__version__` value - - Store as `OLD_VERSION` for comparison link generation + - Read `diff_diff/__init__.py` and note the current `__version__` value — + the "old version" used for comparison-link generation and sanity checks + in the steps below 3. **Compile the changelog and resolve `RELEASE_DATE`** (release notes come exclusively from `changelog.d/` fragments; the old git-log generation step - is removed): + is removed). Shell variables do NOT persist across Bash calls, so + `OLD_VERSION` is resolved inside the SAME block that consumes it (never + substitute it as prose — an empty expansion would fail the compiler's + `--previous-version` cross-check on every normal release): ```bash + OLD_VERSION="$(sed -n 's/^__version__ = "\(.*\)"$/\1/p' diff_diff/__init__.py)" python3 .claude/scripts/changelog_compile.py compile --version NEW_VERSION --date "$(date +%F)" --previous-version "$OLD_VERSION" ``` + (Substitute the literal target version for `NEW_VERSION`; it was validated + against the semver pattern in step 1.) + Key off the exit code: - **Exit 0** (compiled): the `## [NEW_VERSION]` section and comparison link were written and the fragments deleted. `RELEASE_DATE` = today (the diff --git a/.claude/scripts/changelog_compile.py b/.claude/scripts/changelog_compile.py index 5969cb421..632ac1c80 100644 --- a/.claude/scripts/changelog_compile.py +++ b/.claude/scripts/changelog_compile.py @@ -301,8 +301,13 @@ def run_compile(root, version, date_s, allow_dirty, previous_version=None): # canonical shapes, so a malformed section would otherwise be invisible # to the duplicate/date safeguards and compile could add a SECOND # section for the same version. + # The scan accepts ANY CommonMark heading spacing ('## [x]', a tab, + # any hash depth) so a rendering-but-noncanonical heading cannot hide + # from the duplicate/date safeguards; only exact-canonical release and + # Unreleased headers are exempt. canonical_starts = {m_start for _, _, m_start in _existing_headers(text)} - for m in re.finditer(r"^## \[(?!Unreleased\])[^\]]*\][^\n]*$", text, flags=re.MULTILINE): + canonical_starts |= {m.start() for m in _unreleased_headers(text)} + for m in re.finditer(r"^#{1,6}[ \t]+\[[^\]]*\][^\n]*$", text, flags=re.MULTILINE): if m.start() not in canonical_starts: print( "error: malformed release header in CHANGELOG.md: " diff --git a/tests/test_changelog_fragments.py b/tests/test_changelog_fragments.py index c3b7edc85..54e539efc 100644 --- a/tests/test_changelog_fragments.py +++ b/tests/test_changelog_fragments.py @@ -406,13 +406,25 @@ def test_exit4_rejects_bullet_only_under_unknown_category(self, mod, tmp_path): " ## [9.9.9] - 2099-01-01", " ## [9.9.9] - 2099-01-01", " [9.9.9]: https://github.com/x/y/compare/v1.2.0...v9.9.9", + "## [9.9.9] - 2099-01-01", + "##\t[9.9.9] - 2099-01-01", + "### [9.9.9] - 2099-01-01", + ], + ids=[ + "indented-header-1sp", + "indented-header-3sp", + "indented-link-def", + "double-space-after-hashes", + "tab-after-hashes", + "h3-release-like", ], - ids=["indented-header-1sp", "indented-header-3sp", "indented-link-def"], ) - def test_indented_changelog_constructs_refused(self, mod, tmp_path, smuggled): - # CommonMark renders headings/link definitions indented 1-3 spaces, - # but the compiler's duplicate scans are column-zero anchored - an - # indented construct must refuse, on BOTH the fresh and exit-4 paths. + def test_noncanonical_changelog_constructs_refused(self, mod, tmp_path, smuggled): + # CommonMark renders headings indented 1-3 spaces, with a tab or + # multiple spaces after the hashes, and at any hash depth - all + # invisible to column-zero/single-space-anchored duplicate scans. + # Every rendering-but-noncanonical shape must refuse, on BOTH the + # fresh and exit-4 paths. changelog = MINIMAL_CHANGELOG.replace( "### Added\n- old entry\n", f"### Added\n- old entry\n{smuggled}\n", diff --git a/tests/test_tracking_files.py b/tests/test_tracking_files.py index 104683c27..bc1537c57 100644 --- a/tests/test_tracking_files.py +++ b/tests/test_tracking_files.py @@ -126,8 +126,16 @@ def _normalize(row: str) -> str: return "| " + " | ".join(_cells(row)) + " |" +_DIVIDER_CELL = re.compile(r"^:?-+:?$") + + def _is_divider(row: str) -> bool: - return bool(row) and set(row.strip()) <= {"|", "-", ":", " "} and "-" in row + """GFM delimiter-row grammar: every cell is ``:?-+:?`` (an empty or + colon-only cell makes the whole table not render).""" + if not row or "-" not in row: + return False + cells = _cells(row) + return bool(cells) and all(_DIVIDER_CELL.match(c) for c in cells) def _tables(path: Path): @@ -417,6 +425,20 @@ def test_no_leading_pipe_short_divider_fails(self, tmp_path): with pytest.raises(AssertionError, match="2 cells but the header has 3"): list(_tables(f)) + @pytest.mark.parametrize( + "divider", + ["|---||---|", "|:|---|", "| |---|"], + ids=["empty-cell", "colon-only-cell", "space-cell"], + ) + def test_invalid_divider_cell_fails(self, tmp_path, divider): + # GFM requires every delimiter cell to be ':?-+:?'; an empty or + # colon-only cell stops the whole table from rendering. + assert not _is_divider(divider) + f = tmp_path / "t.md" + f.write_text(f"| a | b |\n{divider}\n| x | y |\n") + with pytest.raises(AssertionError, match="no divider"): + list(_tables(f)) + @pytest.mark.parametrize("path", [TODO, DEFERRED], ids=lambda p: p.name) def test_real_tracking_files_are_well_formed(self, path): # The strict parser raises on a malformed table; consuming every From c8c1a9e82a6cc7980514f7205daa92027c3933c1 Mon Sep 17 00:00:00 2001 From: igerber Date: Sun, 30 Aug 2026 10:33:30 -0400 Subject: [PATCH 4/9] Address CI review round 3: no-whitespace link definitions changelog_compile.py: CommonMark permits '[1.3.0]:https://...' with no whitespace after the colon (and resolves a label to its FIRST definition), so the '\s+' link-definition scan missed such a definition - a fresh compile could append the canonical definition while the hidden earlier one kept controlling the rendered link, and the exit-4 path could certify the pair. The duplicate/already-defined-target scan now matches on the label alone ('^[X.Y.Z]:'). The predecessor-link lookups keep their canonical '\]:\s+' form deliberately: a no-space predecessor definition refuses as not-found (fail closed) since compiler output always writes the space. Regressions: hidden no-space target definition blocks the fresh compile (nothing written or consumed); no-space duplicate blocks exit-4. --- .claude/scripts/changelog_compile.py | 8 +++++++- tests/test_changelog_fragments.py | 22 ++++++++++++++++++++++ 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/.claude/scripts/changelog_compile.py b/.claude/scripts/changelog_compile.py index 632ac1c80..9d6695bba 100644 --- a/.claude/scripts/changelog_compile.py +++ b/.claude/scripts/changelog_compile.py @@ -354,7 +354,13 @@ def run_compile(root, version, date_s, allow_dirty, previous_version=None): # Link-definition duplicate check runs BEFORE the existing-target path: # exit 4 must never certify a changelog carrying two definitions for # any version (one correct + one wrong would otherwise pass). - link_versions = re.findall(r"^\[(\d+\.\d+\.\d+)\]:\s+\S+\s*$", text, flags=re.MULTILINE) + # Label-only match: CommonMark permits '[1.3.0]:https://...' with NO + # whitespace after the colon (and even a next-line destination), so + # requiring '\s+' here would let such a definition hide from the + # duplicate and already-defined-target checks while still controlling + # the rendered link (CommonMark resolves a label to its FIRST + # definition). + link_versions = re.findall(r"^\[(\d+\.\d+\.\d+)\]:", text, flags=re.MULTILINE) dup_links = sorted({v for v in link_versions if link_versions.count(v) > 1}) if dup_links: print( diff --git a/tests/test_changelog_fragments.py b/tests/test_changelog_fragments.py index 54e539efc..9c123c86c 100644 --- a/tests/test_changelog_fragments.py +++ b/tests/test_changelog_fragments.py @@ -439,6 +439,28 @@ def test_noncanonical_changelog_constructs_refused(self, mod, tmp_path, smuggled exit4 = make_repo(tmp_path / "exit4", changelog=changelog) assert run_compile(mod, exit4, version="1.2.0") == 1 + def test_nospace_link_definition_blocks_fresh_target(self, mod, tmp_path): + # CommonMark allows '[x]:dest' with no whitespace after the colon, + # and resolves a label to its FIRST definition - a hidden no-space + # definition for the target must refuse the fresh compile, or the + # appended canonical definition would be silently outranked. + changelog = MINIMAL_CHANGELOG.replace( + "[1.2.0]:", + "[1.3.0]:https://github.com/x/y/compare/vWRONG...vWRONG\n[1.2.0]:", + ) + root = make_repo(tmp_path, changelog=changelog, fragments={"20260830-x.md": GOOD_FRAGMENT}) + assert run_compile(mod, root) == 1 + assert "## [1.3.0]" not in (root / "CHANGELOG.md").read_text() + assert (root / "changelog.d" / "20260830-x.md").exists() + + def test_nospace_duplicate_link_definition_blocks_exit4(self, mod, tmp_path): + changelog = MINIMAL_CHANGELOG.replace( + "[1.2.0]:", + "[1.2.0]:https://github.com/x/y/compare/vWRONG...vWRONG\n[1.2.0]:", + ) + root = make_repo(tmp_path, changelog=changelog) + assert run_compile(mod, root, version="1.2.0") == 1 + def test_ls_tree_enumeration_failure_fails_closed( self, mod, tmp_path, monkeypatch, git_commit_all ): From 9ced8f415223e87283fde386ac54d5a1700160ed Mon Sep 17 00:00:00 2001 From: igerber Date: Sun, 30 Aug 2026 10:42:07 -0400 Subject: [PATCH 5/9] Address CI review round 4: holistic version-link-shape rejection Third same-class CommonMark-smuggling finding (indented defs, no-space colons, now container/whitespace-normalized labels) - stop chasing shapes and enforce the invariant directly: a version-label-plus-colon ('[X.Y.Z]:', any label spacing, any container) may exist ONLY as a canonical column-zero definition, because CommonMark registers definitions inside list items/blockquotes, whitespace-normalizes labels ('[ 1.3.0 ]' resolves as '1.3.0'), and resolves each label to its FIRST definition - any smuggled form outranks what the compiler writes. _noncanonical_version_link() enforces this in three places: - the pre-branch CHANGELOG scan (covers fresh and exit-4 paths), - the fragment grammar (a '- [1.3.0]:url' bullet refuses at check time), - a backstop on the fully assembled text before any write (multi-line labels can span the line-based fragment check). Real CHANGELOG and changelog.d audited: no non-canonical shapes exist. Regressions: whitespace-normalized label, list-item and nested-list definitions on both compile paths; three fragment-body forms. --- .claude/scripts/changelog_compile.py | 59 ++++++++++++++++++++++++++++ tests/test_changelog_fragments.py | 12 ++++++ 2 files changed, 71 insertions(+) diff --git a/.claude/scripts/changelog_compile.py b/.claude/scripts/changelog_compile.py index 9d6695bba..726eb8bb3 100644 --- a/.claude/scripts/changelog_compile.py +++ b/.claude/scripts/changelog_compile.py @@ -95,6 +95,15 @@ def _parse_fragment(text): # above as a "continuation" yet render as a real heading. errors.append(f"line {lineno}: indented Markdown heading not allowed in a fragment") continue + if _VERSION_LINK_SHAPE.search(line): + # A '- [1.3.0]:url' bullet would register a CommonMark link + # definition ONCE COMPILED — and the first definition wins, + # outranking the canonical one the compiler appends. + errors.append( + f"line {lineno}: version-link-like construct ('[X.Y.Z]:') " + "not allowed in a fragment" + ) + continue if not line.strip(): if current is not None: current[1].append(line) @@ -242,6 +251,29 @@ def _existing_headers(changelog_text): return out +_VERSION_LINK_SHAPE = re.compile(r"\[\s*\d+\.\d+\.\d+\s*\]:") +_CANONICAL_LINK_SHAPE = re.compile(r"\[\d+\.\d+\.\d+\]:") + + +def _noncanonical_version_link(text): + """(lineno, snippet) of the first version-link-LIKE construct that is + not a canonical column-zero '[X.Y.Z]:' definition, else None. + + CommonMark registers reference definitions inside containers (list + items, blockquotes) and whitespace-normalizes labels ('[ 1.3.0 ]' + resolves as '1.3.0'), and the FIRST definition wins — so any + version-label-plus-colon anywhere outside the canonical link block + could silently outrank the definitions this compiler writes and + scans. Rather than chase every container form, refuse them all.""" + for m in _VERSION_LINK_SHAPE.finditer(text): + at_col0 = m.start() == 0 or text[m.start() - 1] == "\n" + if at_col0 and _CANONICAL_LINK_SHAPE.match(text, m.start()): + continue # canonical: exactly what link_versions collects + lineno = text.count("\n", 0, m.start()) + 1 + return lineno, m.group(0).replace("\n", "\\n") + return None + + def _section_body(changelog_text, header_start): nl = changelog_text.find("\n", header_start) if nl == -1: @@ -336,6 +368,20 @@ def run_compile(root, version, date_s, allow_dirty, previous_version=None): file=sys.stderr, ) return EXIT_FINDINGS + # Version-link-like constructs (any container form, any label spacing) + # must not exist outside the canonical column-zero definitions — see + # _noncanonical_version_link. + bad_link = _noncanonical_version_link(text) + if bad_link: + print( + f"error: CHANGELOG.md line {bad_link[0]}: version-link-like " + f"construct {bad_link[1]!r} outside the canonical column-zero " + "link block (CommonMark would register it as a definition, " + "invisible to the duplicate/target scans) — fix the file " + "before compiling", + file=sys.stderr, + ) + return EXIT_FINDINGS headers = _existing_headers(text) versions = [h[0] for h in headers] duplicated = sorted({v for v in versions if versions.count(v) > 1}) @@ -674,6 +720,19 @@ def run_compile(root, version, date_s, allow_dirty, previous_version=None): new_link = f"[{version}]: {base}/compare/v{prev}...v{version}\n" text = text[: m.start()] + new_link + text[m.start() :] + # Backstop on the fully assembled text (multi-line labels can span the + # fragment-line checks): the only version-link definitions in the + # output must be the canonical column-zero ones. + bad_link = _noncanonical_version_link(text) + if bad_link: + print( + f"error: assembled CHANGELOG line {bad_link[0]}: version-link-" + f"like construct {bad_link[1]!r} would be written outside the " + "canonical link block — refusing to compile", + file=sys.stderr, + ) + return EXIT_FINDINGS + original_text = changelog_path.read_text() fragment_bytes = {p: p.read_bytes() for p in fragments} changelog_path.write_text(text) diff --git a/tests/test_changelog_fragments.py b/tests/test_changelog_fragments.py index 9c123c86c..a27e20cd9 100644 --- a/tests/test_changelog_fragments.py +++ b/tests/test_changelog_fragments.py @@ -206,6 +206,12 @@ def test_bad_fragment_names(self, mod, tmp_path, name): ("### Fixed\n ## smuggled\n- entry\n", "indented Markdown heading"), ("### Fixed\n ## smuggled\n- entry\n", "indented Markdown heading"), ("### Fixed\n- ok\n ### Unknown\n", "indented Markdown heading"), + # CommonMark registers link definitions inside list items, and + # the FIRST definition wins once compiled - a version-link-like + # construct has no place in a release-note fragment. + ("### Fixed\n- [1.3.0]:https://evil.example/wrong\n", "version-link-like"), + ("### Fixed\n- ok\n - [1.3.0]: https://evil.example\n", "version-link-like"), + ("### Fixed\n- see [ 1.3.0 ]: for details\n", "version-link-like"), ], ) def test_bad_fragment_bodies(self, mod, tmp_path, body, needle): @@ -409,6 +415,9 @@ def test_exit4_rejects_bullet_only_under_unknown_category(self, mod, tmp_path): "## [9.9.9] - 2099-01-01", "##\t[9.9.9] - 2099-01-01", "### [9.9.9] - 2099-01-01", + "[ 9.9.9 ]: https://github.com/x/y/compare/vA...vB", + "- [9.9.9]:https://evil.example/wrong", + " - [9.9.9]: https://evil.example/wrong", ], ids=[ "indented-header-1sp", @@ -417,6 +426,9 @@ def test_exit4_rejects_bullet_only_under_unknown_category(self, mod, tmp_path): "double-space-after-hashes", "tab-after-hashes", "h3-release-like", + "whitespace-normalized-label", + "list-item-link-def", + "nested-list-link-def", ], ) def test_noncanonical_changelog_constructs_refused(self, mod, tmp_path, smuggled): From 19fdb0c95eaefa79c2d7a037267f45804f893513 Mon Sep 17 00:00:00 2001 From: igerber Date: Sun, 30 Aug 2026 10:48:07 -0400 Subject: [PATCH 6/9] Address CI review round 5: terminal-link-block invariant Inverse of round 4's hazard: a column-zero '[1.2.0]: url' inside a fenced code or raw-HTML block LOOKS canonical to the line-based scans but does not render as a definition - the predecessor-anchor search could then insert the new comparison link inside that block, and exit-4 could certify a release whose header link never resolves. Rather than teach the scans Markdown block context, enforce the terminal invariant the file already satisfies (all 84 real definitions form one contiguous run at EOF): _link_block_violation() refuses any canonical-looking definition outside the terminal contiguous link block, where no fence/HTML context can exist without breaking the run. Runs on the pre-branch text (fresh + exit-4) and as a backstop on the assembled text before any write. Regressions: predecessor definition existing only inside a fenced block and only inside an HTML block, on both paths, asserting nothing is written or consumed. --- .claude/scripts/changelog_compile.py | 47 ++++++++++++++++++++++++++++ tests/test_changelog_fragments.py | 29 +++++++++++++++++ 2 files changed, 76 insertions(+) diff --git a/.claude/scripts/changelog_compile.py b/.claude/scripts/changelog_compile.py index 726eb8bb3..43fdd770c 100644 --- a/.claude/scripts/changelog_compile.py +++ b/.claude/scripts/changelog_compile.py @@ -274,6 +274,31 @@ def _noncanonical_version_link(text): return None +def _link_block_violation(text): + """(lineno, snippet) of the first canonical-LOOKING definition that is + not part of the terminal contiguous link block, else None. + + The inverse hazard of _noncanonical_version_link: a column-zero + '[X.Y.Z]: url' inside a fenced code or raw-HTML block LOOKS canonical + to the line-based scans but does not render as a definition — the + anchor search could then insert the new link inside that block. + Requiring every definition to live in ONE contiguous run at the end + of the file (where no fence/HTML context can exist without breaking + the run) removes Markdown block context from the problem entirely.""" + lines = text.split("\n") + end = len(lines) + while end > 0 and not lines[end - 1].strip(): + end -= 1 + start = end + while start > 0 and _CANONICAL_LINK_SHAPE.match(lines[start - 1]): + start -= 1 + terminal = set(range(start, end)) + for i, ln in enumerate(lines): + if _CANONICAL_LINK_SHAPE.match(ln) and i not in terminal: + return i + 1, ln[:60] + return None + + def _section_body(changelog_text, header_start): nl = changelog_text.find("\n", header_start) if nl == -1: @@ -382,6 +407,19 @@ def run_compile(root, version, date_s, allow_dirty, previous_version=None): file=sys.stderr, ) return EXIT_FINDINGS + # And the inverse: canonical-LOOKING definitions must all live in the + # terminal contiguous link block — see _link_block_violation. + stray = _link_block_violation(text) + if stray: + print( + f"error: CHANGELOG.md line {stray[0]}: definition-like line " + f"{stray[1]!r} outside the terminal link block (it may sit in a " + "fenced/HTML block and not render, yet the scans would treat it " + "as canonical) — move it into the contiguous block at the end " + "of the file", + file=sys.stderr, + ) + return EXIT_FINDINGS headers = _existing_headers(text) versions = [h[0] for h in headers] duplicated = sorted({v for v in versions if versions.count(v) > 1}) @@ -732,6 +770,15 @@ def run_compile(root, version, date_s, allow_dirty, previous_version=None): file=sys.stderr, ) return EXIT_FINDINGS + stray = _link_block_violation(text) + if stray: + print( + f"error: assembled CHANGELOG line {stray[0]}: definition-like " + f"line {stray[1]!r} would sit outside the terminal link block — " + "refusing to compile", + file=sys.stderr, + ) + return EXIT_FINDINGS original_text = changelog_path.read_text() fragment_bytes = {p: p.read_bytes() for p in fragments} diff --git a/tests/test_changelog_fragments.py b/tests/test_changelog_fragments.py index a27e20cd9..617165ce2 100644 --- a/tests/test_changelog_fragments.py +++ b/tests/test_changelog_fragments.py @@ -451,6 +451,35 @@ def test_noncanonical_changelog_constructs_refused(self, mod, tmp_path, smuggled exit4 = make_repo(tmp_path / "exit4", changelog=changelog) assert run_compile(mod, exit4, version="1.2.0") == 1 + @pytest.mark.parametrize( + "wrapper", + [ + "```\n{d}\n```", + "
\n{d}\n
", + ], + ids=["fenced-code", "html-block"], + ) + def test_definition_inside_block_context_refused(self, mod, tmp_path, wrapper): + # A column-zero '[1.2.0]: url' inside a fenced/HTML block LOOKS + # canonical to line-based scans but does not render - the anchor + # search could insert the new link inside that block. Every + # definition must live in the terminal contiguous link block. + d = "[1.2.0]: https://github.com/x/y/compare/v1.1.0...v1.2.0" + changelog = MINIMAL_CHANGELOG.replace( + "### Added\n- old entry\n", + "### Added\n- old entry\n\n" + wrapper.format(d=d) + "\n", + ).replace( + d + "\n", "" + ) # the def now exists ONLY inside the block + fresh = make_repo( + tmp_path / "fresh", changelog=changelog, fragments={"20260830-x.md": GOOD_FRAGMENT} + ) + assert run_compile(mod, fresh) == 1 + assert "## [1.3.0]" not in (fresh / "CHANGELOG.md").read_text() + assert (fresh / "changelog.d" / "20260830-x.md").exists() + exit4 = make_repo(tmp_path / "exit4", changelog=changelog) + assert run_compile(mod, exit4, version="1.2.0") == 1 + def test_nospace_link_definition_blocks_fresh_target(self, mod, tmp_path): # CommonMark allows '[x]:dest' with no whitespace after the colon, # and resolves a label to its FIRST definition - a hidden no-space From 58f75b1766ca0ea135360ccb6a1195cb6768b3fd Mon Sep 17 00:00:00 2001 From: igerber Date: Sun, 30 Aug 2026 10:54:56 -0400 Subject: [PATCH 7/9] Address CI review round 6: refuse persistent Markdown block contexts entirely Round 5's terminal-link-block invariant assumed a block context cannot reach EOF; CommonMark lets an unterminated fence or 
 stay open
through EOF, swallowing the terminal link block itself while every line
still matches the scans.

Close the whole question structurally: the changelog may not contain ANY
persistent block-context starter - fenced code (```/~~~ at 0-3 indent)
or raw-HTML block types 1-5 (
, processing
instructions, declarations, CDATA), plus HTML comments not closed on the
same line (the Unreleased pointer's closed form stays legal). Type-6/7
HTML blocks end at the first blank line and cannot reach the terminal
block. With no such starter anywhere, no scan ever needs Markdown block
context. Enforced pre-branch (fresh + exit-4) and on the assembled text
before any write (a 1-space-indented fence in a fragment continuation
would land at top level once compiled).

Real CHANGELOG audited: zero starters, all comments self-closing.
Regressions: unterminated fence and unterminated 
 on both paths.
---
 .claude/scripts/changelog_compile.py | 53 ++++++++++++++++++++++++++++
 tests/test_changelog_fragments.py    |  6 +++-
 2 files changed, 58 insertions(+), 1 deletion(-)

diff --git a/.claude/scripts/changelog_compile.py b/.claude/scripts/changelog_compile.py
index 43fdd770c..555dce561 100644
--- a/.claude/scripts/changelog_compile.py
+++ b/.claude/scripts/changelog_compile.py
@@ -274,6 +274,35 @@ def _noncanonical_version_link(text):
     return None
 
 
+# CommonMark block contexts that persist through blank lines (and through
+# EOF when unterminated): fenced code (```/~~~ at 0-3 indent) and raw-HTML
+# block types 1-5 (
, processing instructions,
+# declarations, CDATA). Any of these could swallow subsequent lines —
+# including the terminal link block — so headers/definitions inside them
+# would not render while still matching the line-based scans. Type-6/7
+# HTML blocks end at the first blank line and cannot reach the terminal
+# block. Comments (type 2) are allowed only when closed on the same line
+# (the Unreleased pointer's form).
+_BLOCK_CONTEXT_STARTER = re.compile(
+    r"^ {0,3}(?:```|~~~|<(?:pre|script|style|textarea)\b|<\?|" not in ln[ln.index("