From e1e76f042032eadd852862d32ce3d4b6b627e728 Mon Sep 17 00:00:00 2001 From: Jason Walker <913443@dadeschools.net> Date: Tue, 7 Jul 2026 10:30:32 -0400 Subject: [PATCH] feat: require proof-backed PR review handoff claims (Closes #395) Reviewer final reports must not claim proof-sensitive workflow steps unless each claim carries explicit command/tool evidence or structured MCP result metadata: - New reviewer_proof_backed_claims.py verifier covering the five claim families from the issue: inventory completeness (explicit pagination metadata required; page-size assumptions rejected), earlier-PR skips (per-PR command/tool or structured mergeability/blocker evidence), baseline validation (worktree path, target SHA, dirty-before, exact command, exact result, dirty-after), master integration (exact merge/rebase/simulation command), and cleanup (final git worktree list proof). - PROOF_PROVENANCE_CLASSES constant distinguishes command actually run, MCP metadata, inherited from previous blocker state, and not checked. - Re-export from review_proofs and wire into build_final_report as a downgrade check with proof_backed_claims_proven/violations fields. - New workflow section 30A documents the rule; contract test locks the marker and an export test locks the re-export. - 13 tests in tests/test_reviewer_proof_backed_claims.py covering the required scenarios: baseline claim without command, skipped-PR claim without evidence, inventory-complete from page-size assumption only, cleanup without final worktree proof, and a fully proof-backed report passing. Co-Authored-By: Claude Opus 4.8 (1M context) --- review_proofs.py | 21 ++ reviewer_proof_backed_claims.py | 132 +++++++++++++ .../workflows/review-merge-pr.md | 31 +++ tests/test_llm_workflow_split.py | 7 + tests/test_reviewer_proof_backed_claims.py | 185 ++++++++++++++++++ 5 files changed, 376 insertions(+) create mode 100644 reviewer_proof_backed_claims.py create mode 100644 tests/test_reviewer_proof_backed_claims.py diff --git a/review_proofs.py b/review_proofs.py index 332f866..34f0b2d 100644 --- a/review_proofs.py +++ b/review_proofs.py @@ -1725,6 +1725,11 @@ def build_final_report(checkout_proof, inventory, validation, contamination, if report_text else {"proven": True, "block": False, "reasons": []} ) + proof_backed_claims = ( + assess_proof_backed_handoff_report(report_text) + if report_text + else {"proven": True, "block": False, "reasons": []} + ) if baseline_validation is None and report_text: baseline_validation = assess_reviewer_baseline_validation_proof( report_text=report_text, @@ -1906,6 +1911,11 @@ def build_final_report(checkout_proof, inventory, validation, contamination, "inventory pagination or worktree fields inconsistent (#293)" ) downgrade_reasons.extend(inventory_worktree.get("reasons", [])) + if not proof_backed_claims.get("proven"): + downgrade_reasons.append( + "proof-sensitive handoff claims lack explicit evidence (#395)" + ) + downgrade_reasons.extend(proof_backed_claims.get("reasons", [])) if not baseline_validation.get("proven"): downgrade_reasons.append( "reviewer baseline validation proof missing or failed (#325)" @@ -2017,6 +2027,10 @@ def build_final_report(checkout_proof, inventory, validation, contamination, "inventory_worktree_violations": list( inventory_worktree.get("reasons") or [] ), + "proof_backed_claims_proven": bool(proof_backed_claims.get("proven")), + "proof_backed_claims_violations": list( + proof_backed_claims.get("reasons") or [] + ), "baseline_validation_proven": bool(baseline_validation.get("proven")), "baseline_validation_violations": list( baseline_validation.get("violations") or [] @@ -5355,6 +5369,13 @@ def assess_inventory_worktree_report(report_text, **kwargs): return _assess(report_text, **kwargs) +def assess_proof_backed_handoff_report(report_text, **kwargs): + """#395: proof-sensitive handoff claims require explicit evidence.""" + from reviewer_proof_backed_claims import assess_proof_backed_handoff_report as _assess + + return _assess(report_text, **kwargs) + + def assess_infra_stop_handoff_report(report_text, **kwargs): """#289: repair handoff purity when infra_stop blocks reviewer capability.""" from reviewer_infra_stop_handoff import assess_infra_stop_handoff_report as _assess diff --git a/reviewer_proof_backed_claims.py b/reviewer_proof_backed_claims.py new file mode 100644 index 0000000..9609d67 --- /dev/null +++ b/reviewer_proof_backed_claims.py @@ -0,0 +1,132 @@ +"""Proof-backed reviewer handoff claim verifier (#395). + +Reviewer final reports must not claim proof-sensitive workflow steps unless +each claim is backed by explicit command/tool evidence in the transcript or +structured MCP result metadata. This verifier scans report text for the five +proof-sensitive claim families from issue #395 (inventory completeness, +earlier-PR skips, baseline validation, master integration, cleanup) and fails +closed when a claim appears without its required evidence. +""" + +from __future__ import annotations + +import re +from typing import Any + +# Issue #395 criterion 6: reports must distinguish these proof provenances. +PROOF_PROVENANCE_CLASSES = ( + "command actually run", + "MCP metadata", + "inherited from previous blocker state", + "not checked", +) + +_INVENTORY_CLAIM_RE = re.compile( + r"inventory\s+(?:complete|completeness\s*:\s*complete)", re.IGNORECASE +) +_PAGINATION_METADATA_RE = re.compile( + r"(?:has_more\s*=?\s*false|is_final_page\s*=?\s*true|" + r"inventory_complete\s*=?\s*true|total[_ ]count\s*[=:]\s*\d+|" + r"no next page|final page|pagination complete)", + re.IGNORECASE, +) +_PAGE_SIZE_ASSUMPTION_RE = re.compile( + r"(?:less|fewer)\s+than\s+(?:the\s+)?(?:default\s+)?page\s*size|" + r"under\s+the\s+(?:default\s+)?page\s*size|" + r"below\s+(?:the\s+)?(?:default\s+)?page\s*size", + re.IGNORECASE, +) + +_SKIPPED_PR_RE = re.compile(r"skipped\s+PR\s*#?(\d+)", re.IGNORECASE) +_SKIP_EVIDENCE_RE = re.compile( + r"(?:mergeable\s*:\s*false|gitea_view_pr|gitea_get_pr_review_feedback|" + r"gitea_check_pr_eligibility|git\s+merge-tree|merge simulation|" + r"conflicting files?\s*:|REQUEST_CHANGES\s+at\s+(?:current\s+)?head|" + r"prior blocker reused|blocker revalidated live)", + re.IGNORECASE, +) + +_BASELINE_CLAIM_RE = re.compile(r"\bbaseline\b", re.IGNORECASE) +_BASELINE_REQUIRED = ( + ("baseline worktree path", re.compile(r"branches/baseline-[\w./-]+", re.IGNORECASE)), + ("baseline target SHA", re.compile(r"target\s+SHA\s*:?\s*[0-9a-f]{7,40}", re.IGNORECASE)), + ("baseline dirty-before status", re.compile(r"dirty\s+before\s*:?\s*\w+", re.IGNORECASE)), + ("exact baseline validation command", re.compile(r"(?:command\s*:|pytest|python\s+-m)", re.IGNORECASE)), + ("exact baseline result", re.compile(r"(?:result\s*:|passed|failed)", re.IGNORECASE)), + ("baseline dirty-after status", re.compile(r"dirty\s+after\s*:?\s*\w+", re.IGNORECASE)), +) + +_INTEGRATION_CLAIM_RE = re.compile( + r"(?:master\s+(?:was\s+)?(?:merged|rebased|integrated)|" + r"merged?\s+master\s+into|master[- ]integration|rebased\s+onto\s+master)", + re.IGNORECASE, +) +_INTEGRATION_EVIDENCE_RE = re.compile( + r"(?:git\s+merge(?:\s+--no-commit|\s+--no-ff)?|git\s+rebase|git\s+merge-tree)", + re.IGNORECASE, +) + +_CLEANUP_CLAIM_RE = re.compile( + r"worktrees?\s+(?:were\s+)?(?:removed|cleaned|deleted)|" + r"cleanup\s*:\s*(?!none)\S", + re.IGNORECASE, +) +_CLEANUP_EVIDENCE_RE = re.compile(r"git\s+worktree\s+list", re.IGNORECASE) + + +def assess_proof_backed_handoff_report(report_text: str) -> dict[str, Any]: + """Fail closed on proof-sensitive claims that lack explicit evidence (#395).""" + text = report_text or "" + reasons: list[str] = [] + + if _INVENTORY_CLAIM_RE.search(text): + if _PAGE_SIZE_ASSUMPTION_RE.search(text): + reasons.append( + "inventory completeness claimed from a page size assumption; " + "explicit pagination metadata (has_more=false / is_final_page=true / " + "inventory_complete=true / total count) is required" + ) + elif not _PAGINATION_METADATA_RE.search(text): + reasons.append( + "inventory completeness claimed without explicit pagination " + "metadata proof (has_more=false, is_final_page=true, " + "inventory_complete=true, or total count)" + ) + + for match in _SKIPPED_PR_RE.finditer(text): + pr_number = match.group(1) + window = text[match.start():match.start() + 400] + if not _SKIP_EVIDENCE_RE.search(window): + reasons.append( + f"skipped PR #{pr_number} claim has no explicit command/tool " + "proof or structured mergeability/blocker evidence" + ) + + if _BASELINE_CLAIM_RE.search(text): + missing = [ + name for name, pattern in _BASELINE_REQUIRED if not pattern.search(text) + ] + if missing: + reasons.append( + "baseline validation claim missing required proof fields: " + + ", ".join(missing) + ) + + if _INTEGRATION_CLAIM_RE.search(text) and not _INTEGRATION_EVIDENCE_RE.search(text): + reasons.append( + "master-integration claim lacks the exact merge/rebase/simulation " + "command and result" + ) + + if _CLEANUP_CLAIM_RE.search(text) and not _CLEANUP_EVIDENCE_RE.search(text): + reasons.append( + "cleanup claim lacks final `git worktree list` (or equivalent) proof " + "that session-owned worktrees were removed" + ) + + return { + "proven": not reasons, + "block": bool(reasons), + "reasons": reasons, + "provenance_classes": PROOF_PROVENANCE_CLASSES, + } diff --git a/skills/llm-project-workflow/workflows/review-merge-pr.md b/skills/llm-project-workflow/workflows/review-merge-pr.md index 86eb359..11d0162 100644 --- a/skills/llm-project-workflow/workflows/review-merge-pr.md +++ b/skills/llm-project-workflow/workflows/review-merge-pr.md @@ -836,6 +836,37 @@ Include: If the report and actual tool/command log disagree, fix the report before final output. +## 30A. Proof-backed claims rule (#395) + +Do not claim proof-sensitive workflow steps unless each claim is backed by +explicit command/tool evidence in the transcript or structured MCP result +metadata. + +Inventory completeness claims must include explicit pagination metadata: +`has_more=false`, `is_final_page=true`, `inventory_complete=true`, or the +total count. Do not claim inventory completeness from a page size assumption. + +Earlier-PR skip claims must include explicit command/tool proof for each +skipped PR, or structured inventory fields proving non-mergeable or +prior-blocked state. + +Baseline validation claims must include the baseline worktree path, baseline +target SHA, baseline dirty-before status, exact baseline validation command, +exact baseline result, and baseline dirty-after status. + +Review-worktree master-integration claims must include the exact +merge/rebase/simulation command and result. + +Cleanup claims must include final `git worktree list` or equivalent proof +that session-owned worktrees were removed. + +Every proof-sensitive claim must distinguish its provenance: + +* command actually run +* MCP metadata +* inherited from previous blocker state +* not checked + ## 31. Final report must distinguish mutation types Do not use the legacy field `Workspace mutations`. diff --git a/tests/test_llm_workflow_split.py b/tests/test_llm_workflow_split.py index 255b8f0..b72b116 100644 --- a/tests/test_llm_workflow_split.py +++ b/tests/test_llm_workflow_split.py @@ -61,6 +61,7 @@ def test_review_merge_workflow_contract(): assert "## 0. Load the canonical workflow first" in text assert "## 26A. Terminal review mutation hard-stop" in text assert "## 11A. Skipped PRs are read-only" in text + assert "## 30A. Proof-backed claims rule (#395)" in text assert "## 35. Duplicate request-changes prevention" in text assert "## 37. Controller handoff schema" in text assert "INVENTORY_PAGINATION_UNPROVEN" in text @@ -219,6 +220,12 @@ def test_inventory_worktree_verifier_exported(): assert callable(assess_inventory_worktree_report) +def test_proof_backed_claims_verifier_exported(): + from review_proofs import assess_proof_backed_handoff_report + + assert callable(assess_proof_backed_handoff_report) + + def test_infra_stop_handoff_verifier_exported(): from review_proofs import assess_infra_stop_handoff_report diff --git a/tests/test_reviewer_proof_backed_claims.py b/tests/test_reviewer_proof_backed_claims.py new file mode 100644 index 0000000..4f80b60 --- /dev/null +++ b/tests/test_reviewer_proof_backed_claims.py @@ -0,0 +1,185 @@ +"""Tests for proof-backed reviewer handoff claim verification (#395).""" +import sys +import unittest +from pathlib import Path + +sys.path.insert(0, str(Path(__file__).resolve().parent.parent)) + +from reviewer_proof_backed_claims import ( # noqa: E402 + PROOF_PROVENANCE_CLASSES, + assess_proof_backed_handoff_report, +) + + +def _proof_backed_report(): + """A review report where every proof-sensitive claim carries evidence.""" + return "\n".join( + [ + "## Final Report", + "- Inventory complete: true (has_more=false, is_final_page=true,", + " inventory_complete=true, total_count=11) [proof: MCP metadata]", + "- Skipped PR #372: non-mergeable, mergeable: false via gitea_view_pr", + " re-fetch this session, head 202be15764c173b43e0a571a1ba04d8dbed5f646", + " [proof: command actually run]", + "- Baseline validation: baseline worktree branches/baseline-master-pr374,", + " baseline target SHA 7db8298cddbc18e885fb70c6415f3431333af4aa,", + " baseline dirty before: clean, command: venv/bin/pytest tests/ -q in", + " branches/baseline-master-pr374, result: 1290 passed 1 failed,", + " baseline dirty after: clean [proof: command actually run]", + "- Master integration: git merge --no-commit prgs/master in", + " branches/review-pr374, result: clean merge, aborted with", + " git merge --abort [proof: command actually run]", + "- Cleanup: session-owned worktrees removed; final git worktree list", + " shows no session-owned entries [proof: command actually run]", + ] + ) + + +class TestProofBackedReportPasses(unittest.TestCase): + def test_fully_proof_backed_report_passes(self): + result = assess_proof_backed_handoff_report(_proof_backed_report()) + self.assertTrue(result["proven"], result["reasons"]) + self.assertEqual(result["reasons"], []) + + def test_report_without_proof_sensitive_claims_passes(self): + result = assess_proof_backed_handoff_report( + "## Final Report\n- Review decision: REQUEST_CHANGES\n- Blockers: tests fail" + ) + self.assertTrue(result["proven"], result["reasons"]) + + +class TestInventoryClaims(unittest.TestCase): + def test_inventory_complete_from_page_size_assumption_fails(self): + report = ( + "## Final Report\n" + "- Inventory complete: 13 results were less than page size 50, so no" + " further pages were assumed.\n" + ) + result = assess_proof_backed_handoff_report(report) + self.assertFalse(result["proven"]) + self.assertTrue( + any("page size" in r.lower() for r in result["reasons"]), + result["reasons"], + ) + + def test_inventory_complete_without_pagination_metadata_fails(self): + report = "## Final Report\n- Inventory complete: all open PRs listed.\n" + result = assess_proof_backed_handoff_report(report) + self.assertFalse(result["proven"]) + self.assertTrue( + any("pagination" in r.lower() for r in result["reasons"]), + result["reasons"], + ) + + def test_inventory_complete_with_metadata_passes(self): + report = ( + "## Final Report\n" + "- Inventory complete: true (has_more=false, is_final_page=true," + " inventory_complete=true, total_count=11)\n" + ) + result = assess_proof_backed_handoff_report(report) + self.assertTrue(result["proven"], result["reasons"]) + + +class TestSkippedPrClaims(unittest.TestCase): + def test_skipped_pr_conflict_claim_without_evidence_fails(self): + report = ( + "## Final Report\n" + "- Skipped PR #372 because it has merge conflicts.\n" + ) + result = assess_proof_backed_handoff_report(report) + self.assertFalse(result["proven"]) + self.assertTrue( + any("372" in r for r in result["reasons"]), + result["reasons"], + ) + + def test_skipped_pr_with_structured_mergeability_evidence_passes(self): + report = ( + "## Final Report\n" + "- Skipped PR #372: mergeable: false via gitea_view_pr re-fetch" + " this session.\n" + ) + result = assess_proof_backed_handoff_report(report) + self.assertTrue(result["proven"], result["reasons"]) + + +class TestBaselineClaims(unittest.TestCase): + def test_baseline_claim_without_command_fails(self): + report = ( + "## Final Report\n" + "- Baseline validation confirmed the failures are pre-existing" + " on master.\n" + ) + result = assess_proof_backed_handoff_report(report) + self.assertFalse(result["proven"]) + self.assertTrue( + any("baseline" in r.lower() for r in result["reasons"]), + result["reasons"], + ) + + def test_baseline_claim_missing_dirty_status_fails(self): + report = ( + "## Final Report\n" + "- Baseline validation: baseline worktree branches/baseline-master-pr374," + " baseline target SHA 7db8298cddbc18e885fb70c6415f3431333af4aa," + " command: venv/bin/pytest tests/ -q, result: 1290 passed.\n" + ) + result = assess_proof_backed_handoff_report(report) + self.assertFalse(result["proven"]) + self.assertTrue( + any("dirty" in r.lower() for r in result["reasons"]), + result["reasons"], + ) + + +class TestIntegrationAndCleanupClaims(unittest.TestCase): + def test_master_integration_claim_without_exact_command_fails(self): + report = ( + "## Final Report\n" + "- Master was merged into the review worktree before validation.\n" + ) + result = assess_proof_backed_handoff_report(report) + self.assertFalse(result["proven"]) + self.assertTrue( + any("integration" in r.lower() or "merge" in r.lower() for r in result["reasons"]), + result["reasons"], + ) + + def test_cleanup_claim_without_final_worktree_proof_fails(self): + report = ( + "## Final Report\n" + "- Cleanup: baseline and review worktrees were removed.\n" + ) + result = assess_proof_backed_handoff_report(report) + self.assertFalse(result["proven"]) + self.assertTrue( + any("worktree list" in r.lower() for r in result["reasons"]), + result["reasons"], + ) + + +class TestProvenanceClasses(unittest.TestCase): + def test_provenance_classes_cover_required_distinctions(self): + self.assertEqual( + PROOF_PROVENANCE_CLASSES, + ( + "command actually run", + "MCP metadata", + "inherited from previous blocker state", + "not checked", + ), + ) + + +class TestExports(unittest.TestCase): + def test_review_proofs_reexport(self): + from review_proofs import ( + assess_proof_backed_handoff_report as exported, + ) + + self.assertTrue(callable(exported)) + + +if __name__ == "__main__": + unittest.main()