feat(ai_review): per-PR merge confidence 1-5 in header
This commit is contained in:
+59
-3
@@ -58,7 +58,7 @@ import urllib.error
|
|||||||
import urllib.parse
|
import urllib.parse
|
||||||
import urllib.request
|
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
|
# 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)
|
# never mistaken for an already-reviewed commit, and a label-toggle (same sha)
|
||||||
# is correctly skipped.
|
# is correctly skipped.
|
||||||
@@ -225,6 +225,44 @@ def _int_env(name: str, default: int) -> int:
|
|||||||
return default
|
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(
|
def format_review_body(
|
||||||
findings: str,
|
findings: str,
|
||||||
model: str,
|
model: str,
|
||||||
@@ -236,12 +274,13 @@ def format_review_body(
|
|||||||
risks: list[str] | None = None,
|
risks: list[str] | None = None,
|
||||||
findings_for_table: list[dict] | None = None,
|
findings_for_table: list[dict] | None = None,
|
||||||
inline_count: int = 0,
|
inline_count: int = 0,
|
||||||
|
confidence: int = 5,
|
||||||
) -> str:
|
) -> str:
|
||||||
"""Format the posted review summary body.
|
"""Format the posted review summary body.
|
||||||
|
|
||||||
Layout (per the operator's format guide):
|
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 of Changes** — 2–4 bullets of what the PR introduces
|
||||||
(`summary_changes`); falls back to the opencode prose `summary` if
|
(`summary_changes`); falls back to the opencode prose `summary` if
|
||||||
the agent didn't emit the list.
|
the agent didn't emit the list.
|
||||||
@@ -255,11 +294,22 @@ def format_review_body(
|
|||||||
the body stays scannable; cost lines stay inside it.
|
the body stays scannable; cost lines stay inside it.
|
||||||
* Hidden SHA marker — for the dedupe pass.
|
* Hidden SHA marker — for the dedupe pass.
|
||||||
|
|
||||||
|
`confidence` is a 1-5 merge verdict rendered as `<N>/5 <badge>` 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
|
Empty `summary_changes` + empty `risks` + empty `summary` collapse into
|
||||||
a single "Summary of Changes: _no summary provided._" line so the body
|
a single "Summary of Changes: _no summary provided._" line so the body
|
||||||
never looks half-rendered.
|
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]
|
parts: list[str] = [header]
|
||||||
|
|
||||||
# --- Summary of Changes ---
|
# --- Summary of Changes ---
|
||||||
@@ -2057,6 +2107,11 @@ def review_pr(
|
|||||||
summary_parts = []
|
summary_parts = []
|
||||||
if bullets:
|
if bullets:
|
||||||
summary_parts.append("### Unanchored Notes\n\n" + 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(
|
summary_body = format_review_body(
|
||||||
"\n\n".join(summary_parts), model, sha,
|
"\n\n".join(summary_parts), model, sha,
|
||||||
summary=review_summary,
|
summary=review_summary,
|
||||||
@@ -2065,6 +2120,7 @@ def review_pr(
|
|||||||
risks=risks,
|
risks=risks,
|
||||||
findings_for_table=findings,
|
findings_for_table=findings,
|
||||||
inline_count=len(anchored),
|
inline_count=len(anchored),
|
||||||
|
confidence=confidence,
|
||||||
)
|
)
|
||||||
|
|
||||||
post_inline_review(api, repo, index, token, summary_body, anchored)
|
post_inline_review(api, repo, index, token, summary_body, anchored)
|
||||||
|
|||||||
@@ -11,6 +11,7 @@ sys.path.insert(0, os.path.join(ROOT, "pilot"))
|
|||||||
|
|
||||||
import ai_review # noqa: E402
|
import ai_review # noqa: E402
|
||||||
from ai_review import ( # noqa: E402
|
from ai_review import ( # noqa: E402
|
||||||
|
_CONFIDENCE_BADGE,
|
||||||
_SEVERITY_EMOJI,
|
_SEVERITY_EMOJI,
|
||||||
_balanced_json_substring,
|
_balanced_json_substring,
|
||||||
_extract_first_json_object,
|
_extract_first_json_object,
|
||||||
@@ -24,6 +25,7 @@ from ai_review import ( # noqa: E402
|
|||||||
fmt_tokens,
|
fmt_tokens,
|
||||||
format_review_body,
|
format_review_body,
|
||||||
inline_comment_body,
|
inline_comment_body,
|
||||||
|
merge_confidence,
|
||||||
parse_diff_anchors,
|
parse_diff_anchors,
|
||||||
parse_findings,
|
parse_findings,
|
||||||
parse_repo_config,
|
parse_repo_config,
|
||||||
@@ -31,6 +33,7 @@ from ai_review import ( # noqa: E402
|
|||||||
parse_text_blocks,
|
parse_text_blocks,
|
||||||
prior_review_bodies,
|
prior_review_bodies,
|
||||||
reviewed_shas,
|
reviewed_shas,
|
||||||
|
REVIEW_HEADER,
|
||||||
SEVERITIES,
|
SEVERITIES,
|
||||||
SEVERITY_RANK,
|
SEVERITY_RANK,
|
||||||
split_findings,
|
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 "fake-model-13" not in cfg["compare_against"]
|
||||||
assert "bogus-extra" 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
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user