feat: reject contradictory reviewer handoff ledgers (Closes #501)

Add reviewer_handoff_consistency validation for narrative vs mutation
ledger contradictions, wire it into review_pr final-report rules, and
document the blocked-review handoff template.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
This commit is contained in:
2026-07-08 03:33:31 -04:00
co-authored by Claude Opus 4.8
parent 48cf3a334b
commit 429faccd08
6 changed files with 385 additions and 0 deletions
+1
View File
@@ -821,6 +821,7 @@ scripts/release-tag v0.4.0 --notes-file /tmp/release-notes.md --push
## Related documents ## Related documents
- [`reviewer-handoff-consistency.md`](reviewer-handoff-consistency.md) — reject contradictory reviewer handoffs (#501).
- [`../skills/llm-project-workflow/SKILL.md`](../skills/llm-project-workflow/SKILL.md) — portable cross-project LLM workflow skill. - [`../skills/llm-project-workflow/SKILL.md`](../skills/llm-project-workflow/SKILL.md) — portable cross-project LLM workflow skill.
- [`gitea-execution-profiles.md`](gitea-execution-profiles.md) — the profile model. - [`gitea-execution-profiles.md`](gitea-execution-profiles.md) — the profile model.
- [`gitea-dual-namespace-deployment.md`](gitea-dual-namespace-deployment.md) — static author/reviewer namespace deployment (#139 decision). - [`gitea-dual-namespace-deployment.md`](gitea-dual-namespace-deployment.md) — static author/reviewer namespace deployment (#139 decision).
+35
View File
@@ -0,0 +1,35 @@
# Reviewer Handoff Consistency
Reviewer and final-review controller handoffs must not contradict themselves.
A narrative that says a merge happened, a review was blocked, or a terminal
mutation budget was consumed must match the mutation ledger fields in the same
handoff.
## What gets validated
`reviewer_handoff_consistency.assess_reviewer_handoff_consistency()` checks:
- merge claims appear under `Merge mutations` or `MCP/Gitea mutations`
- terminal-mutation-budget claims name the exact prior mutation in the ledger
- blocked review submission is not paired with "final decision marked"
- reviewer lease acquisition includes a `Review decision`
- blocked/rejected mutations include proof fields:
- tool called
- mutation attempted
- mutation rejected
- no server-side state changed
`final_report_validator` applies this as `reviewer.handoff_consistency` on
`review_pr` reports and fails closed.
## Blocked review template
When `gitea_submit_pr_review` fails closed, use
`reviewer_handoff_consistency.render_blocked_review_handoff_template()` or the
copy in
[`skills/llm-project-workflow/templates/blocked-review-handoff.md`](../skills/llm-project-workflow/templates/blocked-review-handoff.md).
## Related
- #331 — file-mutation ledger alignment
- #501 — contradictory narrative vs ledger detection
+21
View File
@@ -12,6 +12,7 @@ import re
from typing import Any, Callable from typing import Any, Callable
import issue_lock_provenance import issue_lock_provenance
import reviewer_handoff_consistency
from review_proofs import ( from review_proofs import (
HANDOFF_HEADING, HANDOFF_HEADING,
assess_controller_handoff, assess_controller_handoff,
@@ -956,6 +957,25 @@ def _rule_reviewer_validation_structured(
) )
def _rule_reviewer_handoff_consistency(report_text: str) -> list[dict[str, str]]:
result = reviewer_handoff_consistency.assess_reviewer_handoff_consistency(
report_text
)
if result.get("proven"):
return []
return _findings_from_reasons(
"reviewer.handoff_consistency",
result.get("reasons") or [],
field="Review mutations",
severity="block",
safe_next_action=(
"rewrite reviewer handoff so narrative claims match the mutation "
"ledger; use the blocked-review handoff template when submission "
"was rejected"
),
)
def _rule_reviewer_mutation_ledger( def _rule_reviewer_mutation_ledger(
report_text: str, report_text: str,
*, *,
@@ -1069,6 +1089,7 @@ _RULES_BY_TASK: dict[str, list[Callable[..., list[dict[str, str]]]]] = {
_rule_reviewer_already_landed_eligible, _rule_reviewer_already_landed_eligible,
_rule_reviewer_already_landed_state, _rule_reviewer_already_landed_state,
_rule_reviewer_target_branch_freshness, _rule_reviewer_target_branch_freshness,
_rule_reviewer_handoff_consistency,
_rule_reviewer_mutation_ledger, _rule_reviewer_mutation_ledger,
_rule_reviewer_review_mutation, _rule_reviewer_review_mutation,
_rule_reviewer_stale_head_proof, _rule_reviewer_stale_head_proof,
+186
View File
@@ -0,0 +1,186 @@
"""Reviewer handoff consistency validation (#501).
Detects contradictions between narrative claims and mutation ledger fields in
reviewer/final-review controller handoffs. Pure validation only — does not post
comments or mutate Gitea state.
"""
from __future__ import annotations
import re
_HANDOFF_FIELD_RE = re.compile(
r"^\s*[-*]\s*([^:]+):\s*(.*)$",
re.MULTILINE | re.IGNORECASE,
)
_MERGE_NARRATIVE_RE = re.compile(
r"\b(?:merged\s+pr\s*#|pr\s+#\d+\s+(?:was\s+)?merged|"
r"merge\s+result\s*:\s*merged|successfully\s+merged|"
r"terminal\s+mutation\s+budget.{0,80}\bmerge)\b",
re.IGNORECASE | re.DOTALL,
)
_REVIEW_SUBMIT_BLOCKED_RE = re.compile(
r"\b(?:review\s+submission\s+blocked|submit_pr_review\s+(?:failed|blocked)|"
r"gitea_submit_pr_review\s+(?:failed|blocked|not\s+(?:attempted|performed))|"
r"review\s+not\s+submitted|could\s+not\s+submit\s+review)\b",
re.IGNORECASE,
)
_FINAL_DECISION_MARKED_RE = re.compile(
r"\b(?:gitea_mark_final_review_decision|final\s+review\s+decision\s+marked|"
r"server-side\s+final\s+decision\s+marked|marked\s+final\s+review\s+decision)\b",
re.IGNORECASE,
)
_TERMINAL_BUDGET_CONSUMED_RE = re.compile(
r"\b(?:terminal\s+mutation\s+budget\s+(?:already\s+)?consumed|"
r"single[- ]terminal\s+mutation\s+(?:already\s+)?consumed|"
r"mutation\s+budget\s+exhausted)\b",
re.IGNORECASE,
)
_LEASE_NARRATIVE_RE = re.compile(
r"\b(?:reviewer\s+lease\s+acquired|acquired\s+reviewer\s+pr\s+lease|"
r"gitea_acquire_reviewer_pr_lease)\b",
re.IGNORECASE,
)
_REJECTED_MUTATION_RE = re.compile(
r"\b(?:mutation\s+rejected|tool\s+failed\s+closed|blocked\s+before\s+mutation|"
r"no\s+server-side\s+state\s+changed)\b",
re.IGNORECASE,
)
_MERGE_IN_LEDGER_RE = re.compile(r"\bmerge\b", re.IGNORECASE)
_REVIEW_SUBMITTED_IN_LEDGER_RE = re.compile(
r"\b(?:submitted|approve|request_changes|review\s+submitted)\b",
re.IGNORECASE,
)
_NONE_VALUE_RE = re.compile(r"^\s*(?:none|not\s+attempted|n/a)\s*$", re.IGNORECASE)
_BLOCKED_REVIEW_PROOF_FIELDS = (
"tool called",
"mutation attempted",
"mutation rejected",
"no server-side state changed",
)
def render_blocked_review_handoff_template() -> str:
"""Return a corrected handoff template for blocked review submissions."""
return """## Blocked Review Submission Handoff
- Tool called: gitea_submit_pr_review
- Mutation attempted: yes
- Mutation rejected: yes
- No server-side state changed: confirmed
- Proof/source: <paste exact tool error or gate reason>
- Prior terminal mutation that consumed budget: <name exact prior mutation or none>
- Review decision (local intent): <approve | request_changes | comment only>
- Server-side final decision marked: <yes with tool proof | no>
- Gitea review submitted: no
- Gitea review blocked because: <exact gate/error>
- Review mutations: none (submission blocked)
- MCP/Gitea mutations: <list only mutations that actually occurred>
- Safe next action: rewrite handoff with consistent ledger before posting
"""
def _handoff_fields(text: str | None) -> dict[str, str]:
fields: dict[str, str] = {}
for match in _HANDOFF_FIELD_RE.finditer(text or ""):
key = match.group(1).strip().lower()
value = match.group(2).strip()
fields[key] = value
return fields
def _is_none_value(value: str | None) -> bool:
return bool(_NONE_VALUE_RE.match(value or ""))
def _ledger_mentions_merge(*values: str | None) -> bool:
blob = " ".join(v for v in values if v)
return bool(blob) and bool(_MERGE_IN_LEDGER_RE.search(blob))
def _ledger_mentions_review_submission(*values: str | None) -> bool:
blob = " ".join(v for v in values if v)
return bool(blob) and bool(_REVIEW_SUBMITTED_IN_LEDGER_RE.search(blob))
def assess_reviewer_handoff_consistency(report_text: str | None) -> dict:
"""Validate reviewer handoff narrative against mutation ledger fields."""
text = report_text or ""
fields = _handoff_fields(text)
reasons: list[str] = []
review_mutations = fields.get("review mutations", "")
merge_mutations = fields.get("merge mutations", "")
mcp_mutations = fields.get("mcp/gitea mutations", "")
review_decision = fields.get("review decision", "")
if _MERGE_NARRATIVE_RE.search(text) and not _ledger_mentions_merge(
merge_mutations, mcp_mutations, review_mutations
):
reasons.append(
"narrative claims a merge occurred but mutation ledger omits merge"
)
if _TERMINAL_BUDGET_CONSUMED_RE.search(text):
if _ledger_mentions_merge(merge_mutations, mcp_mutations):
pass
elif _LEASE_NARRATIVE_RE.search(text) and not _ledger_mentions_merge(
merge_mutations, mcp_mutations
):
reasons.append(
"terminal mutation budget attributed to merge but ledger only "
"shows lease or non-merge activity"
)
elif not _ledger_mentions_merge(merge_mutations, mcp_mutations):
reasons.append(
"terminal mutation budget consumed claim must identify the "
"exact prior mutation in the ledger"
)
if _REVIEW_SUBMIT_BLOCKED_RE.search(text) and _FINAL_DECISION_MARKED_RE.search(text):
reasons.append(
"handoff claims review submission blocked but also claims "
"server-side final decision was marked"
)
lease_claimed = _LEASE_NARRATIVE_RE.search(text) or (
not _is_none_value(mcp_mutations)
and "lease" in mcp_mutations.lower()
)
if lease_claimed and _is_none_value(review_decision):
reasons.append(
"reviewer lease acquired but Review decision field is missing or none"
)
if _REVIEW_SUBMIT_BLOCKED_RE.search(text) or _REJECTED_MUTATION_RE.search(text):
lower = text.lower()
missing_proof = [
field
for field in _BLOCKED_REVIEW_PROOF_FIELDS
if field not in lower
]
if missing_proof:
reasons.append(
"blocked/rejected mutation claim missing proof fields: "
+ ", ".join(missing_proof)
)
if (
_FINAL_DECISION_MARKED_RE.search(text)
and _is_none_value(review_mutations)
and not _ledger_mentions_review_submission(mcp_mutations)
and not _REVIEW_SUBMIT_BLOCKED_RE.search(text)
):
reasons.append(
"final review decision marked but Review mutations ledger is empty"
)
proven = not reasons
return {
"proven": proven,
"block": not proven,
"reasons": reasons,
"fields": fields,
}
@@ -0,0 +1,22 @@
# Blocked review submission handoff
Use when `gitea_submit_pr_review` or the terminal mutation gate blocks review
submission.
```text
## Blocked Review Submission Handoff
- Tool called: gitea_submit_pr_review
- Mutation attempted: yes
- Mutation rejected: yes
- No server-side state changed: confirmed
- Proof/source: <paste exact tool error or gate reason>
- Prior terminal mutation that consumed budget: <exact prior mutation or none>
- Review decision (local intent): <approve | request_changes | comment only>
- Server-side final decision marked: <yes with tool proof | no>
- Gitea review submitted: no
- Gitea review blocked because: <exact gate/error>
- Review mutations: none (submission blocked)
- MCP/Gitea mutations: <only mutations that actually occurred>
- Safe next action: rewrite handoff with consistent ledger before posting
```
+120
View File
@@ -0,0 +1,120 @@
"""Tests for reviewer handoff consistency validation (#501)."""
import sys
import unittest
from pathlib import Path
sys.path.insert(0, str(Path(__file__).resolve().parent.parent))
import reviewer_handoff_consistency # noqa: E402
from final_report_validator import assess_final_report_validator # noqa: E402
def _base_handoff(**overrides):
lines = [
"## Controller Handoff",
"",
"- Task: review PR #472",
"- Review decision: request_changes",
"- Review mutations: submitted request_changes review",
"- Merge mutations: none",
"- MCP/Gitea mutations: reviewer lease acquired",
]
text = "\n".join(lines)
for key, value in overrides.items():
token = f"- {key}:"
if token in text:
before, _, after = text.partition(token)
rest = after.split("\n", 1)
suffix = rest[1] if len(rest) > 1 else ""
text = f"{before}{token} {value}" + (f"\n{suffix}" if suffix else "")
else:
text += f"\n- {key}: {value}"
return text
class TestReviewerHandoffConsistency(unittest.TestCase):
def test_consistent_handoff_passes(self):
result = reviewer_handoff_consistency.assess_reviewer_handoff_consistency(
_base_handoff()
)
self.assertTrue(result["proven"], result["reasons"])
def test_claimed_merge_missing_from_ledger(self):
report = _base_handoff(
**{
"Current status": "Merged PR #411 earlier; reviewing PR #472",
"Merge mutations": "none",
"MCP/Gitea mutations": "reviewer lease acquired",
}
)
result = reviewer_handoff_consistency.assess_reviewer_handoff_consistency(report)
self.assertFalse(result["proven"])
self.assertTrue(any("merge" in r.lower() for r in result["reasons"]))
def test_terminal_budget_consumed_without_merge_in_ledger(self):
report = _base_handoff(
**{
"Current status": "Terminal mutation budget already consumed by merge",
"Merge mutations": "none",
"MCP/Gitea mutations": "reviewer lease acquired",
}
)
result = reviewer_handoff_consistency.assess_reviewer_handoff_consistency(report)
self.assertFalse(result["proven"])
self.assertTrue(any("terminal mutation budget" in r for r in result["reasons"]))
def test_review_blocked_but_final_decision_marked(self):
report = _base_handoff(
**{
"Current status": "gitea_submit_pr_review blocked",
"Review mutations": "none",
}
)
report += "\nServer-side final decision marked via gitea_mark_final_review_decision."
result = reviewer_handoff_consistency.assess_reviewer_handoff_consistency(report)
self.assertFalse(result["proven"])
self.assertTrue(any("blocked" in r and "final decision" in r for r in result["reasons"]))
def test_lease_without_review_decision(self):
report = _base_handoff(**{"Review decision": "none"})
report += "\nReviewer lease acquired for PR #472."
result = reviewer_handoff_consistency.assess_reviewer_handoff_consistency(report)
self.assertFalse(result["proven"])
self.assertTrue(any("Review decision" in r for r in result["reasons"]))
def test_rejected_mutation_without_proof(self):
report = _base_handoff(
**{
"Review mutations": "none",
"MCP/Gitea mutations": "none",
}
)
report += "\nReview submission blocked before mutation."
result = reviewer_handoff_consistency.assess_reviewer_handoff_consistency(report)
self.assertFalse(result["proven"])
self.assertTrue(any("proof fields" in r for r in result["reasons"]))
def test_blocked_template_includes_required_fields(self):
template = reviewer_handoff_consistency.render_blocked_review_handoff_template()
for field in reviewer_handoff_consistency._BLOCKED_REVIEW_PROOF_FIELDS:
self.assertIn(field, template.lower())
def test_final_report_validator_integration(self):
report = _base_handoff(
**{
"Current status": "Merged PR #411; terminal mutation budget consumed",
"Merge mutations": "none",
}
)
result = assess_final_report_validator(report, "review_pr")
self.assertTrue(
any(
f["rule_id"] == "reviewer.handoff_consistency"
for f in result["findings"]
),
result["findings"],
)
if __name__ == "__main__":
unittest.main()