From ed054d34e21225cf3fc7a6207d7ce1146c62531e Mon Sep 17 00:00:00 2001 From: OpenCode Agent Date: Sun, 18 Oct 2026 16:39:55 +0300 Subject: [PATCH] Add pr-ledger.md/pr-ledger.json consistency verifier find_overlap.py reads docs/research/pr-ledger.md as its local-ledger overlap corpus, but verify_pr_reconciliation.py checks only pr-ledger.json and reconcile_pr_queue.py writes both files without verifying the committed .md. A stale or hand-edited .md therefore misreports disposition and recorded review coverage with no failing check. scripts/verify_pr_ledger_markdown.py re-derives the Markdown projection that reconcile_pr_queue.py emits (disposition counts plus one row per PR) and reports bounded differences. On current main it finds 539 of 3028 rows stale: contributor and disposition cells agree in every case, but the evidence-record column has lost the docs/research/pr-intake-decisions.json record for all 539. Source-bound attribution/consistency check only. It does not judge whether a recorded review was correct and does not contact GitHub. --- scripts/verify_pr_ledger_markdown.py | 203 ++++++++++++++++++++++++ tests/test_verify_pr_ledger_markdown.py | 158 ++++++++++++++++++ 2 files changed, 361 insertions(+) create mode 100644 scripts/verify_pr_ledger_markdown.py create mode 100644 tests/test_verify_pr_ledger_markdown.py diff --git a/scripts/verify_pr_ledger_markdown.py b/scripts/verify_pr_ledger_markdown.py new file mode 100644 index 0000000000..aff3eed19f --- /dev/null +++ b/scripts/verify_pr_ledger_markdown.py @@ -0,0 +1,203 @@ +#!/usr/bin/env python3 +"""Check docs/research/pr-ledger.md against docs/research/pr-ledger.json. + +`scripts/find_overlap.py` reads the Markdown ledger as its `local-ledger` +overlap corpus, so a stale or hand-edited `.md` silently misreports disposition, +attribution and recorded review coverage. `scripts/verify_pr_reconciliation.py` +checks the machine ledger only, and `scripts/reconcile_pr_queue.py` writes both +files but never verifies the committed `.md`. + +This script re-derives the Markdown projection that `reconcile_pr_queue.py` +emits (disposition counts, then one row per PR: number, contributor, disposition +and ordered evidence records) and compares it with the committed file. It +reports a bounded set of differences instead of a single boolean. + +Scope: this is an attribution/coverage binding check. It says nothing about +whether a recorded review was correct, and it does not touch GitHub. + + python3 scripts/verify_pr_ledger_markdown.py + python3 scripts/verify_pr_ledger_markdown.py --json + python3 scripts/verify_pr_ledger_markdown.py --root /tmp/fixture +""" +from __future__ import annotations + +import argparse +import json +import sys +from pathlib import Path + +ROOT = Path(__file__).resolve().parent.parent +MARKDOWN_RELATIVE = "docs/research/pr-ledger.md" +LEDGER_RELATIVE = "docs/research/pr-ledger.json" +REPORT_LIMIT = 20 +NO_REVIEW_DETAIL = "No exact-head review established; inspect contribution and dependencies." + + +class LedgerMismatch(Exception): + """Raised when the Markdown ledger cannot be projected from the JSON ledger.""" + + +def require(condition: object, message: str) -> None: + if not condition: + raise LedgerMismatch(message) + + +def expected_counts(ledger: dict) -> dict: + counts: dict[str, int] = {} + for pr in ledger["pull_requests"]: + counts[pr["disposition"]] = counts.get(pr["disposition"], 0) + 1 + return counts + + +def expected_detail(pr: dict) -> str: + """Reproduce the generator's evidence-record column for one PR.""" + refs = sorted({entry["evidence"] for entry in pr["evidence"]}) + if pr.get("reconciliation_evidence"): + refs.extend(pr["reconciliation_evidence"]) + refs = list(dict.fromkeys(refs)) + if not refs: + return NO_REVIEW_DETAIL + return " · ".join(f"[record {i + 1}](../../{name})" for i, name in enumerate(refs)) + + +def expected_row(pr: dict) -> str: + return "| [#{number}]({url}) | {login} | {disposition} | {detail} |".format( + number=pr["number"], + url=pr["url"], + login=pr["contributor"]["login"], + disposition=pr["disposition"].replace("_", " "), + detail=expected_detail(pr), + ) + + +def parse_markdown(text: str) -> tuple[dict, list, list]: + """Return (declared counts, per-PR row lines, structural problems).""" + lines = text.split("\n") + problems: list[str] = [] + + counts: dict[str, int] = {} + seen_header = False + for index, line in enumerate(lines): + if line.startswith("| Disposition | Count |"): + seen_header = True + cursor = index + 2 # skip the |---|---|---:| separator + while cursor < len(lines) and lines[cursor].startswith("|"): + cells = [cell.strip() for cell in lines[cursor].strip().strip("|").split("|")] + if len(cells) != 2: + problems.append(f"counts table line {cursor + 1}: expected 2 cells, found {len(cells)}") + else: + try: + counts[cells[0]] = int(cells[1]) + except ValueError: + problems.append(f"counts table line {cursor + 1}: non-integer count {cells[1]!r}") + cursor += 1 + break + if not seen_header: + problems.append("disposition counts table header '| Disposition | Count |' not found") + + rows = [(index + 1, line) for index, line in enumerate(lines) if line.startswith("| [#")] + if not rows: + problems.append("no '| [#N](...)' inventory rows found") + return counts, rows, problems + + +def display_counts(counts: dict) -> dict: + """The generator prints disposition keys with spaces; normalise for comparison.""" + return {name.replace("_", " "): value for name, value in counts.items()} + + +def check(markdown: str, ledger: dict) -> dict: + require(isinstance(ledger, dict) and "pull_requests" in ledger, "JSON ledger has no pull_requests") + counts, rows, problems = parse_markdown(markdown) + counts = display_counts(counts) + prs = ledger["pull_requests"] + + differences: list[str] = list(problems) + + want_counts = display_counts(expected_counts(ledger)) + declared = ledger.get("counts") + if declared is not None and display_counts(declared) != want_counts: + differences.append(f"JSON 'counts' field disagrees with its own pull_requests: {declared} != {expected_counts(ledger)}") + for name in sorted(set(want_counts) | set(counts)): + if counts.get(name) != want_counts.get(name): + differences.append(f"count {name!r}: markdown {counts.get(name)} != json {want_counts.get(name)}") + + if len(rows) != len(prs): + differences.append(f"inventory row count: markdown {len(rows)} != json {len(prs)}") + + row_text: dict[int, str] = {} + first_line: dict[int, int] = {} + for line_no, line in rows: + number = int(line.split("]", 1)[0].split("#", 1)[1]) + if number in row_text: + differences.append(f"line {line_no}: duplicate row for #{number} (first at line {first_line[number]})") + continue + row_text[number] = line.rstrip("\r") + first_line[number] = line_no + seen = first_line + missing = sorted({pr["number"] for pr in prs} - set(seen)) + extra = sorted(set(seen) - {pr["number"] for pr in prs}) + if missing: + differences.append(f"missing rows: {missing[:REPORT_LIMIT]}") + if extra: + differences.append(f"unexpected rows: {extra[:REPORT_LIMIT]}") + + by_number = {pr["number"]: pr for pr in prs} + field_mismatches = 0 + for number in sorted(seen): + pr = by_number.get(number) + if pr is None: + continue + want = expected_row(pr) + if row_text[number] != want: + field_mismatches += 1 + if field_mismatches <= REPORT_LIMIT: + differences.append(f"line {seen[number]}: #{number} row differs\n markdown: {row_text[number]!r}\n from json: {want!r}") + if field_mismatches > REPORT_LIMIT: + differences.append(f"... and {field_mismatches - REPORT_LIMIT} further row mismatches") + + return { + "markdown_rows": len(rows), + "json_prs": len(prs), + "counts": want_counts, + "markdown_counts": counts, + "row_mismatches": field_mismatches, + "missing_rows": missing, + "unexpected_rows": extra, + "differences": differences, + "ok": not differences, + } + + +def main(argv: list | None = None) -> int: + parser = argparse.ArgumentParser(description=__doc__, formatter_class=argparse.RawDescriptionHelpFormatter) + parser.add_argument("--root", type=Path, default=ROOT, help="repository root (default: this checkout)") + parser.add_argument("--markdown", type=Path, help="override Markdown ledger path") + parser.add_argument("--json-ledger", type=Path, help="override JSON ledger path") + parser.add_argument("--json", action="store_true", help="emit the full report as JSON") + args = parser.parse_args(argv) + + root = args.root.resolve() + markdown_path = args.markdown or root / MARKDOWN_RELATIVE + ledger_path = args.json_ledger or root / LEDGER_RELATIVE + try: + require(markdown_path.is_file(), f"missing Markdown ledger: {markdown_path}") + require(ledger_path.is_file(), f"missing JSON ledger: {ledger_path}") + ledger = json.loads(ledger_path.read_text(encoding="utf-8")) + report = check(markdown_path.read_text(encoding="utf-8"), ledger) + except LedgerMismatch as error: + report = {"ok": False, "differences": [str(error)]} + + if args.json: + print(json.dumps(report, indent=2, ensure_ascii=False)) + elif report["ok"]: + print(f"PASS_PR_LEDGER_MARKDOWN_CONSISTENT rows={report['markdown_rows']} counts={report['counts']}") + else: + print("FAIL_PR_LEDGER_MARKDOWN_MISMATCH", file=sys.stderr) + for difference in report["differences"]: + print(f" - {difference}", file=sys.stderr) + return 0 if report["ok"] else 1 + + +if __name__ == "__main__": + raise SystemExit(main()) \ No newline at end of file diff --git a/tests/test_verify_pr_ledger_markdown.py b/tests/test_verify_pr_ledger_markdown.py new file mode 100644 index 0000000000..6943acb67f --- /dev/null +++ b/tests/test_verify_pr_ledger_markdown.py @@ -0,0 +1,158 @@ +#!/usr/bin/env python3 +"""Tests for scripts/verify_pr_ledger_markdown.py. + +Positive control: a consistent Markdown ledger passes. Failure controls mutate +the fixture in each way the verifier is meant to catch, so a silently passing +checker is itself a failure. +""" +from __future__ import annotations + +import importlib.util +import json +import unittest +from pathlib import Path + +ROOT = Path(__file__).resolve().parent.parent + + +def _load(): + path = ROOT / "scripts/verify_pr_ledger_markdown.py" + spec = importlib.util.spec_from_file_location("verify_pr_ledger_markdown", path) + module = importlib.util.module_from_spec(spec) + spec.loader.exec_module(module) + return module + + +verify = _load() + +LEDGER = { + "schema_version": 1, + "counts": {"incorporated": 1, "partially_reviewed": 1}, + "pull_requests": [ + { + "number": 1, + "url": "https://github.com/CrocSwap/integer-mult-bounds/pull/1", + "contributor": {"login": "alice"}, + "disposition": "incorporated", + "evidence": [{"evidence": "research/a-audit/DISPOSITIONS.json"}, + {"evidence": "research/b-audit/DISPOSITIONS.json"}], + }, + { + "number": 7, + "url": "https://github.com/CrocSwap/integer-mult-bounds/pull/7", + "contributor": {"login": "bob"}, + "disposition": "partially_reviewed", + "evidence": [{"evidence": "research/a-audit/DISPOSITIONS.json"}, + {"evidence": "research/a-audit/DISPOSITIONS.json"}], + "reconciliation_evidence": ["research/c-audit/DISPOSITIONS.json"], + }, + ], +} + +ROWS = [ + "| [#1](https://github.com/CrocSwap/integer-mult-bounds/pull/1) | alice | incorporated | " + "[record 1](../../research/a-audit/DISPOSITIONS.json) · [record 2](../../research/b-audit/DISPOSITIONS.json) |", + "| [#7](https://github.com/CrocSwap/integer-mult-bounds/pull/7) | bob | partially reviewed | " + "[record 1](../../research/a-audit/DISPOSITIONS.json) · [record 2](../../research/c-audit/DISPOSITIONS.json) |", +] + +MARKDOWN = "\n".join([ + "# PR review and integration ledger", + "", + "| Disposition | Count |", + "|---|---:|", + "| incorporated | 1 |", + "| partially reviewed | 1 |", + "", + "| PR | Contributor | Disposition | Evidence or remaining question |", + "|---|---|---|---|", + *ROWS, +]) + "\n" + + +class VerifyLedgerMarkdown(unittest.TestCase): + def check(self, markdown=MARKDOWN, ledger=LEDGER): + return verify.check(markdown, ledger) + + def assertDifference(self, report, needle): + self.assertFalse(report["ok"]) + self.assertTrue(any(needle in d for d in report["differences"]), + f"expected a difference mentioning {needle!r}, got {report['differences']}") + + # Positive control. + def test_consistent_ledger_passes(self): + report = self.check() + self.assertTrue(report["ok"], report["differences"]) + self.assertEqual(report["row_mismatches"], 0) + self.assertEqual(report["markdown_rows"], 2) + + def test_deduplicates_evidence_and_appends_reconciliation(self): + # #7 lists a-audit twice in `evidence`; the projection keeps one copy + # and appends the reconciliation evidence in order. + self.assertIn("[record 1](../../research/a-audit/DISPOSITIONS.json) · " + "[record 2](../../research/c-audit/DISPOSITIONS.json)", ROWS[1]) + + def test_no_evidence_uses_declared_placeholder(self): + ledger = json.loads(json.dumps(LEDGER)) + ledger["pull_requests"][0]["evidence"] = [] + ledger["counts"] = {"partially_reviewed": 1, "incorporated": 1} + markdown = MARKDOWN.replace(ROWS[0], "| [#1](https://github.com/CrocSwap/integer-mult-bounds/pull/1) " + "| alice | incorporated | " + verify.NO_REVIEW_DETAIL + " |") + report = self.check(markdown, ledger) + self.assertTrue(report["ok"], report["differences"]) + + # Failure controls. + def test_detects_dropped_evidence_record(self): + """The failure actually observed on main: a stale row loses a record.""" + report = self.check(MARKDOWN.replace(" · [record 2](../../research/b-audit/DISPOSITIONS.json)", "")) + self.assertDifference(report, "#1 row differs") + + def test_detects_disposition_drift(self): + report = self.check(MARKDOWN.replace("| alice | incorporated |", "| alice | partially reviewed |")) + self.assertDifference(report, "#1 row differs") + + def test_detects_attribution_drift(self): + report = self.check(MARKDOWN.replace("| bob | partially reviewed |", "| mallory | partially reviewed |")) + self.assertDifference(report, "#7 row differs") + + def test_detects_missing_row(self): + report = self.check(MARKDOWN.replace(ROWS[1] + "\n", "")) + self.assertDifference(report, "missing rows: [7]") + + def test_detects_count_drift(self): + report = self.check(MARKDOWN.replace("| incorporated | 1 |", "| incorporated | 2 |")) + self.assertDifference(report, "count 'incorporated'") + + def test_detects_duplicate_row(self): + report = self.check(MARKDOWN + ROWS[0] + "\n") + self.assertDifference(report, "duplicate row for #1") + + def test_detects_absent_counts_table(self): + report = self.check("\n".join(line for line in MARKDOWN.split("\n") + if line != "| Disposition | Count |")) + self.assertDifference(report, "counts table header") + + def test_detects_missing_count_row(self): + report = self.check(MARKDOWN.replace("| incorporated | 1 |\n", "")) + self.assertDifference(report, "count 'incorporated'") + + def test_detects_json_counts_field_drift(self): + ledger = json.loads(json.dumps(LEDGER)) + ledger["counts"] = {"incorporated": 2, "partially_reviewed": 1} + self.assertDifference(self.check(ledger=ledger), "disagrees with its own pull_requests") + + +class VerifyLedgerMarkdownOnRepository(unittest.TestCase): + """The committed ledgers are the real fixture; report status without asserting it.""" + + def test_report_shape(self): + ledger = json.loads((ROOT / "docs/research/pr-ledger.json").read_text(encoding="utf-8")) + report = verify.check((ROOT / "docs/research/pr-ledger.md").read_text(encoding="utf-8"), ledger) + self.assertEqual(report["markdown_rows"], report["json_prs"]) + self.assertEqual(report["missing_rows"], []) + self.assertEqual(report["unexpected_rows"], []) + self.assertEqual(report["counts"], report["markdown_counts"]) + + +if __name__ == "__main__": + unittest.main() \ No newline at end of file