feat: require true pre-merge baseline proof for non-zero validation exits (Closes #533) #574
@@ -715,6 +715,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,
|
||||
*,
|
||||
@@ -1294,6 +1313,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,
|
||||
@@ -1318,6 +1338,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": "",
|
||||
}
|
||||
@@ -126,4 +126,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()
|
||||
Reference in New Issue
Block a user