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) <[email protected]>
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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,
|
||||
}
|
||||
@@ -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`.
|
||||
|
||||
@@ -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
|
||||
|
||||
|
||||
@@ -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()
|
||||
Reference in New Issue
Block a user