Verify that pr-ledger.md matches pr-ledger.json (539 stale rows, attribution intact)

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())

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()