Merge pull request 'feat: require true pre-merge baseline proof for non-zero validation exits (Closes #533)' (#574) from feat/issue-533-premerge-baseline-proof into master

2026-07-08 22:21:30 -05:00
parent f29df56a99 7fb813446f
commit 066eec8477
4 changed files with 319 additions and 0 deletions
@@ -748,6 +748,25 @@ def _rule_reviewer_main_checkout_baseline(report_text: str) -> list[dict[str, st
    ]


+def _rule_reviewer_premerge_baseline_proof(report_text: str) -> list[dict[str, str]]:
+    from premerge_baseline_proof import assess_premerge_baseline_proof
+
+    result = assess_premerge_baseline_proof(report_text)
+    if not result.get("block"):
+        return []
+    return _findings_from_reasons(
+        "reviewer.premerge_baseline_proof",
+        result.get("reasons") or [],
+        field="Pre-merge baseline proof",
+        severity="block",
+        safe_next_action=result.get("safe_next_action")
+        or (
+            "prove the failure on the PR pre-merge base commit or a known-failure "
+            "record predating the PR; current-master reproduction is not baseline proof"
+        ),
+    )
+
+
 def _rule_reviewer_validation_status_vocabulary(
    report_text: str,
    *,
@@ -1328,6 +1347,7 @@ _RULES_BY_TASK: dict[str, list[Callable[..., list[dict[str, str]]]]] = {
        _rule_reviewer_validation_structured,
        _rule_reviewer_linked_issue,
        _rule_reviewer_baseline_on_failure,
+        _rule_reviewer_premerge_baseline_proof,
        _rule_reviewer_validation_status_vocabulary,
        _rule_reviewer_main_checkout_baseline,
        _rule_reviewer_main_checkout_path,
@@ -1353,6 +1373,7 @@ _RULES_BY_TASK: dict[str, list[Callable[..., list[dict[str, str]]]]] = {
        _rule_reconcile_linked_issue_live,
        _rule_reconcile_close_proof,
        _rule_reconcile_pagination_proof,
+        _rule_reviewer_premerge_baseline_proof,
        _rule_reviewer_git_fetch_readonly,
        _rule_reviewer_legacy_workspace_mutations,
        _rule_reviewer_vague_mutations_none,
@@ -0,0 +1,176 @@
+"""Pre-merge baseline proof verifier for non-zero validation exits (#533).
+
+A controller/reviewer may reproduce a failing test on *current* master and
+describe it as proof of a "known baseline failure". Reproducing a failure on
+post-merge master only proves the failure exists on current master — it does
+NOT prove the failure pre-existed the PR under review. A PR can introduce or
+preserve a regression, then once merged the failure appears on master and gets
+mislabeled "baseline", weakening validation gates.
+
+This module distinguishes four canonical validation outcomes:
+
+- ``CLEAN_PASS`` — the validation command exited zero.
+- ``CURRENT_MASTER_FAILURE_REPRODUCED`` — the same failure reproduces on
+  current (post-merge) master. Not sufficient as baseline proof.
+- ``PREMERGE_BASELINE_PROVEN_FAILURE`` — the failure is proven on the PR's
+  pre-merge base commit, or by a documented known-failure record that predates
+  the PR.
+- ``UNRESOLVED_REGRESSION_RISK`` — a non-zero exit that is neither a clean pass
+  nor proven pre-existing.
+
+A report may only call a non-zero validation exit a "baseline"/"pre-existing"
+failure when it carries pre-merge proof.
+"""
+
+from __future__ import annotations
+
+import re
+from typing import Any
+
+CLEAN_PASS = "clean pass"
+CURRENT_MASTER_FAILURE_REPRODUCED = "current-master failure reproduced"
+PREMERGE_BASELINE_PROVEN_FAILURE = "pre-merge baseline-proven failure"
+UNRESOLVED_REGRESSION_RISK = "unresolved regression risk"
+
+# A non-zero validation exit is claimed somewhere in the report.
+_NONZERO_EXIT_RE = re.compile(
+    r"(?:exit(?:\s*(?:status|code))?\s*[:=]?\s*(?:[1-9]\d*)"
+    r"|\b\d+\s+failed\b"
+    r"|\bnon[- ]zero\s+(?:exit|validation))",
+    re.IGNORECASE,
+)
+# The report claims the failure is pre-existing / a baseline failure.
+_BASELINE_CLAIM_RE = re.compile(
+    r"(?:pre[- ]?existing(?:\s+(?:baseline|failure))?"
+    r"|baseline(?:[- ]proven)?\s+failure"
+    r"|known[- ]baseline"
+    r"|failure\s+is\s+baseline"
+    r"|already\s+fail(?:s|ing)\s+on\s+master)",
+    re.IGNORECASE,
+)
+# Only-current-master reproduction language (insufficient on its own).
+_CURRENT_MASTER_RE = re.compile(
+    r"(?:current[- ]master\s+(?:reproduc|failure)"
+    r"|reproduc\w*\s+on\s+(?:current\s+)?master"
+    r"|post[- ]merge\s+master"
+    r"|fails\s+on\s+(?:current\s+)?master\s+(?:now|too|as\s+well))",
+    re.IGNORECASE,
+)
+# Pre-merge base proof: a base commit tested before the PR landed.
+_PREMERGE_BASE_RE = re.compile(
+    r"(?:pre[- ]merge\s+base(?:\s+commit)?"
+    r"|pr\s+base\s+commit"
+    r"|merge[- ]base(?:\s+commit)?"
+    r"|base\s+commit\s+before\s+(?:the\s+)?pr)",
+    re.IGNORECASE,
+)
+# Documented known-failure record predating the PR.
+_KNOWN_FAILURE_RECORD_RE = re.compile(
+    r"known[- ]failure\s+(?:record|reference|ledger)\b.{0,80}?"
+    r"(?:predat|before\s+(?:the\s+)?pr|pre[- ]existing|prior\s+to\s+(?:the\s+)?pr)",
+    re.IGNORECASE | re.DOTALL,
+)
+
+# Required proof fields for a pre-merge baseline claim.
+_FIELD_PATTERNS: dict[str, re.Pattern[str]] = {
+    "base commit": re.compile(
+        r"(?:pre[- ]merge\s+)?base\s+commit\s*[:=]\s*([0-9a-f]{7,40})", re.IGNORECASE
+    ),
+    "tested commit": re.compile(
+        r"tested\s+commit\s*[:=]\s*([0-9a-f]{7,40})", re.IGNORECASE
+    ),
+    "command": re.compile(r"command\s*[:=]\s*\S", re.IGNORECASE),
+    "exit status": re.compile(r"exit\s*(?:status|code)?\s*[:=]\s*\d+", re.IGNORECASE),
+    "failure signature": re.compile(
+        r"failure\s+signature\s*[:=]\s*\S", re.IGNORECASE
+    ),
+}
+
+
+def assess_premerge_baseline_proof(report_text: str) -> dict[str, Any]:
+    """Assess whether a claimed baseline failure carries pre-merge proof.
+
+    Returns a dict with ``proven``, ``block``, ``label``, ``reasons``,
+    ``skipped`` and ``safe_next_action``. Only blocks when the report claims a
+    non-zero validation exit is baseline/pre-existing without valid pre-merge
+    proof.
+    """
+    text = report_text or ""
+
+    has_failure = bool(_NONZERO_EXIT_RE.search(text))
+    claims_baseline = bool(_BASELINE_CLAIM_RE.search(text))
+
+    # No failure claimed, or no baseline assertion — nothing to enforce here.
+    if not has_failure and not claims_baseline:
+        return {
+            "proven": True,
+            "block": False,
+            "label": CLEAN_PASS,
+            "reasons": [],
+            "skipped": True,
+            "safe_next_action": "",
+        }
+
+    if not claims_baseline:
+        # A non-zero exit not asserted as baseline — surface as regression risk,
+        # but do not block (the report never claimed a clean/baseline pass).
+        return {
+            "proven": True,
+            "block": False,
+            "label": UNRESOLVED_REGRESSION_RISK,
+            "reasons": [],
+            "skipped": False,
+            "safe_next_action": "",
+        }
+
+    has_premerge_base = bool(_PREMERGE_BASE_RE.search(text))
+    has_known_record = bool(_KNOWN_FAILURE_RECORD_RE.search(text))
+    only_current_master = bool(_CURRENT_MASTER_RE.search(text))
+
+    reasons: list[str] = []
+
+    if not (has_premerge_base or has_known_record):
+        if only_current_master:
+            reasons.append(
+                "baseline failure claimed from current-master reproduction only; "
+                "that proves the failure on current master, not that it pre-existed "
+                "the PR — provide pre-merge base proof or a known-failure record"
+            )
+        else:
+            reasons.append(
+                "baseline/pre-existing failure claimed without pre-merge proof "
+                "(no pre-merge base commit and no documented known-failure record "
+                "predating the PR)"
+            )
+
+    # Require the concrete proof fields regardless of proof source.
+    missing = [name for name, pat in _FIELD_PATTERNS.items() if not pat.search(text)]
+    if missing:
+        reasons.append(
+            "pre-merge baseline claim missing required proof field(s): "
+            + ", ".join(missing)
+        )
+
+    if reasons:
+        return {
+            "proven": False,
+            "block": True,
+            "label": UNRESOLVED_REGRESSION_RISK,
+            "reasons": reasons,
+            "skipped": False,
+            "safe_next_action": (
+                "prove the failure on the PR pre-merge base commit (or cite a "
+                "known-failure record predating the PR) and state base commit, "
+                "tested commit, command, exit status, and failure signature; "
+                "current-master reproduction alone is not baseline proof"
+            ),
+        }
+
+    return {
+        "proven": True,
+        "block": False,
+        "label": PREMERGE_BASELINE_PROVEN_FAILURE,
+        "reasons": [],
+        "skipped": False,
+        "safe_next_action": "",
+    }
@@ -127,4 +127,14 @@ including the review/merge role fields: Selected PR, Reviewer eligibility,
 Pinned reviewed head, Review decision, Merge result, Linked issue status,
 Cleanup status. If you could not merge, name the exact gate. Reports missing
 the handoff are downgraded (review_proofs.assess_controller_handoff).
+
+Baseline failure proof (#533): if a validation command exits non-zero, do NOT
+call it a clean pass. Only label a failure "baseline"/"pre-existing" with
+pre-merge proof — the failure reproduced on the PR's pre-merge base commit, or a
+documented known-failure record predating the PR. Reproducing on current
+(post-merge) master is "current-master failure reproduced", NOT baseline proof.
+State: base commit, tested commit, command, exit status, failure signature.
+Use one label: clean pass / current-master failure reproduced / pre-merge
+baseline-proven failure / unresolved regression risk
+(final_report_validator: reviewer.premerge_baseline_proof).
 ```
@@ -0,0 +1,112 @@
+import unittest
+
+from premerge_baseline_proof import (
+    CLEAN_PASS,
+    PREMERGE_BASELINE_PROVEN_FAILURE,
+    UNRESOLVED_REGRESSION_RISK,
+    assess_premerge_baseline_proof,
+)
+from final_report_validator import assess_final_report_validator
+
+_FIELDS = (
+    "Pre-merge base commit: 1a2b3c4d5e6f7a8b9c0d1e2f3a4b5c6d7e8f9a0b\n"
+    "Tested commit: 1a2b3c4d5e6f7a8b9c0d1e2f3a4b5c6d7e8f9a0b\n"
+    "Command: venv/bin/python -m pytest tests/test_foo.py -q\n"
+    "Exit status: 1\n"
+    "Failure signature: AssertionError: None is not true\n"
+)
+
+# Current-master-only reproduction carries no pre-merge base proof (that is the
+# whole defect #533 guards against): a command/exit/signature but no base commit.
+_CURRENT_ONLY_FIELDS = (
+    "Command: venv/bin/python -m pytest tests/test_foo.py -q\n"
+    "Exit status: 1\n"
+    "Failure signature: AssertionError: None is not true\n"
+)
+
+
+class TestPremergeBaselineProof(unittest.TestCase):
+    def test_clean_pass_not_blocked(self):
+        result = assess_premerge_baseline_proof(
+            "Validation: full suite passed, exit status 0. Clean pass."
+        )
+        self.assertFalse(result["block"])
+        self.assertTrue(result["skipped"])
+        self.assertEqual(result["label"], CLEAN_PASS)
+
+    def test_nonzero_exit_not_claimed_baseline_not_blocked(self):
+        result = assess_premerge_baseline_proof(
+            "Full suite: 1 failed. Investigating the regression; not yet classified."
+        )
+        self.assertFalse(result["block"])
+        self.assertEqual(result["label"], UNRESOLVED_REGRESSION_RISK)
+
+    def test_current_master_only_reproduction_rejected(self):
+        report = (
+            "Full suite: 1 failed. This is a pre-existing baseline failure — "
+            "reproduced on current master after the PR merged.\n" + _CURRENT_ONLY_FIELDS
+        )
+        result = assess_premerge_baseline_proof(report)
+        self.assertTrue(result["block"])
+        self.assertTrue(
+            any("current-master reproduction only" in r for r in result["reasons"])
+        )
+
+    def test_baseline_claim_missing_fields_blocked(self):
+        report = (
+            "Full suite: 1 failed. This failure is baseline / pre-existing. "
+            "Verified against the pre-merge base commit."
+        )
+        result = assess_premerge_baseline_proof(report)
+        self.assertTrue(result["block"])
+        self.assertTrue(
+            any("missing required proof field" in r for r in result["reasons"])
+        )
+
+    def test_premerge_base_proof_accepted(self):
+        report = (
+            "Full suite: 1 failed. This is a pre-existing baseline failure, proven "
+            "on the PR pre-merge base commit.\n" + _FIELDS
+        )
+        result = assess_premerge_baseline_proof(report)
+        self.assertFalse(result["block"])
+        self.assertTrue(result["proven"])
+        self.assertEqual(result["label"], PREMERGE_BASELINE_PROVEN_FAILURE)
+
+    def test_documented_known_failure_record_accepted(self):
+        report = (
+            "Full suite: 1 failed. Baseline failure: cited known-failure record #529 "
+            "which predates the PR.\n" + _FIELDS
+        )
+        result = assess_premerge_baseline_proof(report)
+        self.assertFalse(result["block"])
+        self.assertEqual(result["label"], PREMERGE_BASELINE_PROVEN_FAILURE)
+
+
+class TestPremergeBaselineProofValidatorIntegration(unittest.TestCase):
+    def _has_rule(self, report):
+        result = assess_final_report_validator(report, "review_pr")
+        return any(
+            f["rule_id"] == "reviewer.premerge_baseline_proof"
+            for f in result["findings"]
+        )
+
+    def test_review_report_current_master_only_flagged(self):
+        report = (
+            "## Reviewer final report\n"
+            "Full suite: 1 failed. Pre-existing baseline failure — reproduced on "
+            "post-merge master.\n" + _CURRENT_ONLY_FIELDS
+        )
+        self.assertTrue(self._has_rule(report))
+
+    def test_review_report_premerge_proof_clean(self):
+        report = (
+            "## Reviewer final report\n"
+            "Full suite: 1 failed. Pre-existing baseline failure proven on the "
+            "pre-merge base commit.\n" + _FIELDS
+        )
+        self.assertFalse(self._has_rule(report))
+
+
+if __name__ == "__main__":
+    unittest.main()