feat: require true pre-merge baseline proof for non-zero validation exits (Closes #533)
Add premerge_baseline_proof.py distinguishing four validation outcomes (clean pass / current-master failure reproduced / pre-merge baseline-proven failure / unresolved regression risk). A non-zero exit may only be labeled "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. Current-master (post-merge) reproduction is explicitly rejected as sufficient baseline proof. - premerge_baseline_proof.py: assess_premerge_baseline_proof + labels - final_report_validator.py: reviewer.premerge_baseline_proof rule on review_pr and reconcile_already_landed tasks - skills/llm-project-workflow/templates/review-pr.md: baseline proof section requiring base commit, tested commit, command, exit status, failure signature - tests/test_premerge_baseline_proof.py: reject current-master-only proof, accept pre-merge base proof and documented known-failure records Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
This commit is contained in:
@@ -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(
|
def _rule_reviewer_validation_status_vocabulary(
|
||||||
report_text: str,
|
report_text: str,
|
||||||
*,
|
*,
|
||||||
@@ -1294,6 +1313,7 @@ _RULES_BY_TASK: dict[str, list[Callable[..., list[dict[str, str]]]]] = {
|
|||||||
_rule_reviewer_validation_structured,
|
_rule_reviewer_validation_structured,
|
||||||
_rule_reviewer_linked_issue,
|
_rule_reviewer_linked_issue,
|
||||||
_rule_reviewer_baseline_on_failure,
|
_rule_reviewer_baseline_on_failure,
|
||||||
|
_rule_reviewer_premerge_baseline_proof,
|
||||||
_rule_reviewer_validation_status_vocabulary,
|
_rule_reviewer_validation_status_vocabulary,
|
||||||
_rule_reviewer_main_checkout_baseline,
|
_rule_reviewer_main_checkout_baseline,
|
||||||
_rule_reviewer_main_checkout_path,
|
_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_linked_issue_live,
|
||||||
_rule_reconcile_close_proof,
|
_rule_reconcile_close_proof,
|
||||||
_rule_reconcile_pagination_proof,
|
_rule_reconcile_pagination_proof,
|
||||||
|
_rule_reviewer_premerge_baseline_proof,
|
||||||
_rule_reviewer_git_fetch_readonly,
|
_rule_reviewer_git_fetch_readonly,
|
||||||
_rule_reviewer_legacy_workspace_mutations,
|
_rule_reviewer_legacy_workspace_mutations,
|
||||||
_rule_reviewer_vague_mutations_none,
|
_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,
|
Pinned reviewed head, Review decision, Merge result, Linked issue status,
|
||||||
Cleanup status. If you could not merge, name the exact gate. Reports missing
|
Cleanup status. If you could not merge, name the exact gate. Reports missing
|
||||||
the handoff are downgraded (review_proofs.assess_controller_handoff).
|
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