From f3a125666be1f57ece69cdf7272e1d34b47ae67f Mon Sep 17 00:00:00 2001 From: claude Date: Sat, 22 Aug 2026 00:23:24 +0000 Subject: [PATCH] feat(ai_review): per-PR merge confidence 1-5 in header --- pilot/ai_review.py | 62 ++++++++++++++++++++++++++-- tests/pilot/test_ai_review.py | 78 +++++++++++++++++++++++++++++++++++ 2 files changed, 137 insertions(+), 3 deletions(-) diff --git a/pilot/ai_review.py b/pilot/ai_review.py index b03d135..55a2a23 100644 --- a/pilot/ai_review.py +++ b/pilot/ai_review.py @@ -58,7 +58,7 @@ import urllib.error import urllib.parse import urllib.request -REVIEW_HEADER = "🤖 **AI Review** · pragent pilot · {model} · `{sha}`" +REVIEW_HEADER = "🤖 **AI Review** · pragent pilot · {model} · `{sha}` · Merge confidence: {confidence}" # Hidden marker the dedupe pass scans for. Full sha so a re-push (new sha) is # never mistaken for an already-reviewed commit, and a label-toggle (same sha) # is correctly skipped. @@ -225,6 +225,44 @@ def _int_env(name: str, default: int) -> int: return default +# 1-5 merge-verdict score (higher = safer). Buckets: +# 5 = clean (or low/info/trivial only — nothing worth blocking on) +# 4 = medium present +# 3 = high present (operator should at least look) +# 1 = critical present (block the merge by default) +# Cross-lens agreement on any finding takes one more off, floored at 1. +_CONFIDENCE_BADGE = {5: "🟢", 4: "🟢", 3: "🟡", 2: "🟠", 1: "🔴"} + + +def merge_confidence(findings: list[dict]) -> int: + """1-5 merge verdict: higher = safer. + + Tier drops driven by the most severe finding present: + - critical → 1 + - high → 3 + - medium → 4 + - else → 5 (low / trivial / info / unknown → no drop) + + An extra -1 when ANY finding carries ``_multi_lens`` (cross-lens agreement + bumps confidence in the signal — penalise harder). The final score is + clamped to [1, 5] so a critical + multi_lens combo doesn't go negative. + """ + if not findings: + return 5 + max_rank = max(SEVERITY_RANK.get(f.get("severity", "low"), 0) for f in findings) + if max_rank >= SEVERITY_RANK["critical"]: + score = 1 + elif max_rank >= SEVERITY_RANK["high"]: + score = 3 + elif max_rank >= SEVERITY_RANK["medium"]: + score = 4 + else: + score = 5 + if any(f.get("_multi_lens") for f in findings): + score -= 1 + return max(1, min(5, score)) + + def format_review_body( findings: str, model: str, @@ -236,12 +274,13 @@ def format_review_body( risks: list[str] | None = None, findings_for_table: list[dict] | None = None, inline_count: int = 0, + confidence: int = 5, ) -> str: """Format the posted review summary body. Layout (per the operator's format guide): - * Header line (``🤖 AI Review …``). + * Header line (``🤖 AI Review …``) including the merge-confidence badge. * **Summary of Changes** — 2–4 bullets of what the PR introduces (`summary_changes`); falls back to the opencode prose `summary` if the agent didn't emit the list. @@ -255,11 +294,22 @@ def format_review_body( the body stays scannable; cost lines stay inside it. * Hidden SHA marker — for the dedupe pass. + `confidence` is a 1-5 merge verdict rendered as `/5 ` in the + header. Clamped to [1, 5] so a stray value (e.g. 0 from a missing + finding list) doesn't print a broken badge. + Empty `summary_changes` + empty `risks` + empty `summary` collapse into a single "Summary of Changes: _no summary provided._" line so the body never looks half-rendered. """ - header = REVIEW_HEADER.format(model=model, sha=sha[:8] if sha else "unknown") + score = max(1, min(5, confidence)) + badge = _CONFIDENCE_BADGE.get(score, "🟢") + confidence_str = f"{score}/5 {badge}" + header = REVIEW_HEADER.format( + model=model, + sha=sha[:8] if sha else "unknown", + confidence=confidence_str, + ) parts: list[str] = [header] # --- Summary of Changes --- @@ -2057,6 +2107,11 @@ def review_pr( summary_parts = [] if bullets: summary_parts.append("### Unanchored Notes\n\n" + bullets) + # 1-5 merge verdict for the header badge. Computed AFTER filtering + + # anchoring so the verdict reflects what the operator sees (a critical + # finding that fails to anchor is still a critical finding). The + # default 5 keeps any failure path (e.g. empty findings) green. + confidence = merge_confidence(findings) summary_body = format_review_body( "\n\n".join(summary_parts), model, sha, summary=review_summary, @@ -2065,6 +2120,7 @@ def review_pr( risks=risks, findings_for_table=findings, inline_count=len(anchored), + confidence=confidence, ) post_inline_review(api, repo, index, token, summary_body, anchored) diff --git a/tests/pilot/test_ai_review.py b/tests/pilot/test_ai_review.py index 98230ce..e3b58a5 100644 --- a/tests/pilot/test_ai_review.py +++ b/tests/pilot/test_ai_review.py @@ -11,6 +11,7 @@ sys.path.insert(0, os.path.join(ROOT, "pilot")) import ai_review # noqa: E402 from ai_review import ( # noqa: E402 + _CONFIDENCE_BADGE, _SEVERITY_EMOJI, _balanced_json_substring, _extract_first_json_object, @@ -24,6 +25,7 @@ from ai_review import ( # noqa: E402 fmt_tokens, format_review_body, inline_comment_body, + merge_confidence, parse_diff_anchors, parse_findings, parse_repo_config, @@ -31,6 +33,7 @@ from ai_review import ( # noqa: E402 parse_text_blocks, prior_review_bodies, reviewed_shas, + REVIEW_HEADER, SEVERITIES, SEVERITY_RANK, split_findings, @@ -1821,3 +1824,78 @@ def test_parse_repo_config_compare_against_caps_at_12(monkeypatch): assert "fake-model-13" not in cfg["compare_against"] assert "bogus-extra" not in cfg["compare_against"] + +# --------------------------------------------------------------------------- +# Task 6 — merge_confidence + REVIEW_HEADER confidence badge +# --------------------------------------------------------------------------- + + +def test_merge_confidence_clean_is_five(): + assert merge_confidence([]) == 5 + + +def test_merge_confidence_only_low_is_five(): + f = {"severity": "low"} + assert merge_confidence([f, f, f]) == 5 + + +def test_merge_confidence_medium_drops_one(): + f = {"severity": "medium"} + assert merge_confidence([f]) == 4 + + +def test_merge_confidence_high_drops_two(): + f = {"severity": "high"} + assert merge_confidence([f]) == 3 + + +def test_merge_confidence_critical_drops_to_one(): + f = {"severity": "critical"} + assert merge_confidence([f]) == 1 + + +def test_merge_confidence_multi_lens_drops_extra(): + f = {"severity": "low", "_multi_lens": True} + assert merge_confidence([f]) == 4 + + +def test_merge_confidence_clamped(): + # Three critical findings must NOT take the score below 1. + f = {"severity": "critical"} + assert merge_confidence([f, f, f]) == 1 + + +def test_review_header_includes_confidence(): + # REVIEW_HEADER gains a {confidence} placeholder; verify the format works. + h = REVIEW_HEADER.format(model="glm-5.2:cloud", sha="abc1234567", confidence="3/5 🟡") + assert "Merge confidence: 3/5 🟡" in h + + +def test_confidence_badge_table_complete(): + # Sanity-check the badge table the render layer reads from. + assert _CONFIDENCE_BADGE == {5: "🟢", 4: "🟢", 3: "🟡", 2: "🟠", 1: "🔴"} + + +def test_format_review_body_default_confidence_is_green(): + # Default confidence kwarg should produce a green 5/5 badge in the header, + # matching the pre-existing "clean PR" semantics. + body = format_review_body("- [high] x:1 — bug", "glm-5.2:cloud", "abcdef1234567890") + assert "Merge confidence: 5/5 🟢" in body + + +def test_format_review_body_low_confidence_shows_red_badge(): + body = format_review_body( + "- [critical] x:1 — bug", "glm-5.2:cloud", "abcdef1234567890", + confidence=1, + ) + assert "Merge confidence: 1/5 🔴" in body + + +def test_format_review_body_confidence_clamps_out_of_range(): + # Out-of-range confidence is clamped to [1, 5] in the badge string. + body_hi = format_review_body("- x", "glm-5.2:cloud", "abcdef1234567890", confidence=99) + assert "Merge confidence: 5/5 🟢" in body_hi + body_lo = format_review_body("- x", "glm-5.2:cloud", "abcdef1234567890", confidence=0) + assert "Merge confidence: 1/5 🔴" in body_lo + +