mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-14 23:21:35 +00:00
feat(triage): add "ready for review" label lifecycle to Agent Shin
Adds review_gate(), a state machine that keeps a `ready for review` label in sync with whether an external PR clears BOTH gates — the LLM rubric and Greptile's most recent confidence score: - pass (untagged) -> add label + "ready for review" / "all clear" comment - pass (already tagged) -> no-op (idempotent across re-runs) - regress (Greptile < 4/5 or QA proof removed) -> remove label + "what's missing" comment, PR stays open - recover after a regression -> "all clear again" comment + re-add the label - fail & untagged, < 24h old -> one-time "what's missing" notice (grace window) - fail & untagged, > 24h old -> close + comment (reopen via @agent-shin reconsider) The label itself is the persisted state, so comments fire only on transitions (never on every scheduled run). All side effects are gated behind --close, so the dry-run contract matches the existing triage flow. Lifecycle comments use hidden HTML markers and deliberately avoid the auto-close marker so they never trip the reconsider provenance check. Relocates the shared Greptile helpers (extract_greptile_score, SCORE_PATTERN, GREPTILE_BOT_LOGINS, parse_iso8601) into triage_with_llm.py so the daily sweep and the review gate read the score through one implementation, and adds the review_gate.yml workflow (dry-run unless AGENT_SHIN_ENABLED=true) plus 18 unit tests covering every branch and a full pass->regress->recover cycle. https://claude.ai/code/session_01XyyWa8t2VYmoGd6mKMEqkZ
This commit is contained in:
Claude
parent
0ad6836e31
commit
b4d970371e
5 changed files with 967 additions and 65 deletions
57
.github/scripts/close_low_quality_prs.py
vendored
57
.github/scripts/close_low_quality_prs.py
vendored
|
|
@ -38,11 +38,9 @@ from __future__ import annotations
|
|||
import argparse
|
||||
import datetime as dt
|
||||
import json
|
||||
import re
|
||||
import subprocess
|
||||
import sys
|
||||
from pathlib import Path
|
||||
from typing import Iterable
|
||||
|
||||
# Share constants with the sibling Agent Shin script instead of duplicating
|
||||
# them. `AGENT_SHIN_AUTO_CLOSE_MARKER` is the literal phrase the reconsider
|
||||
|
|
@ -53,23 +51,21 @@ from typing import Iterable
|
|||
_SCRIPTS_DIR = Path(__file__).resolve().parent
|
||||
if str(_SCRIPTS_DIR) not in sys.path:
|
||||
sys.path.insert(0, str(_SCRIPTS_DIR))
|
||||
# `extract_greptile_score`, `GREPTILE_BOT_LOGINS`, `SCORE_PATTERN`, and
|
||||
# `parse_iso8601` now live in triage_with_llm so the daily sweep and the
|
||||
# review gate read the Greptile score through one implementation (drift would
|
||||
# silently let one path act on a PR the other would spare).
|
||||
from triage_with_llm import ( # noqa: E402
|
||||
AGENT_SHIN_AUTO_CLOSE_MARKER,
|
||||
GREPTILE_BOT_LOGINS,
|
||||
INTERNAL_ASSOCIATIONS,
|
||||
SCORE_PATTERN,
|
||||
extract_greptile_score,
|
||||
parse_iso8601,
|
||||
)
|
||||
|
||||
# Greptile's GitHub App appears as `greptile-apps[bot]` in REST API comments
|
||||
# and `greptile-apps` in `gh pr view --json` output. Accept either form.
|
||||
GREPTILE_BOT_LOGINS = frozenset({"greptile-apps", "greptile-apps[bot]"})
|
||||
|
||||
# Matches lines like:
|
||||
# <h3>Confidence Score: 3/5</h3>
|
||||
# **Confidence Score: 4/5**
|
||||
# Confidence Score: 5 / 5
|
||||
SCORE_PATTERN = re.compile(
|
||||
r"confidence\s*score\s*[:\-]?\s*(\d+)\s*/\s*5",
|
||||
re.IGNORECASE,
|
||||
)
|
||||
# Re-exported above for any caller that imports them from this module.
|
||||
__all__ = ["GREPTILE_BOT_LOGINS", "SCORE_PATTERN", "extract_greptile_score"]
|
||||
|
||||
# Default labels that exempt a PR from auto-close. Defined at module scope (not
|
||||
# as a mutable argparse default) so that `--optout-label foo` REPLACES the
|
||||
|
|
@ -197,39 +193,6 @@ def fetch_pr_comments(pr_number: int, repo: str | None) -> list[dict]:
|
|||
return comments
|
||||
|
||||
|
||||
def extract_greptile_score(comments: Iterable[dict]) -> tuple[int, dict] | None:
|
||||
"""Return (score, comment) for the most recent Greptile-authored comment
|
||||
that contains a "Confidence Score: X/5". Returns None if no such comment.
|
||||
|
||||
"Most recent" is determined by the comment's `updated_at` (falling back to
|
||||
`created_at`), so re-reviews override earlier passes.
|
||||
"""
|
||||
candidates: list[tuple[str, int, dict]] = []
|
||||
for comment in comments:
|
||||
user = (comment.get("user") or {}).get("login", "")
|
||||
if user not in GREPTILE_BOT_LOGINS:
|
||||
continue
|
||||
body = comment.get("body") or ""
|
||||
match = SCORE_PATTERN.search(body)
|
||||
if not match:
|
||||
continue
|
||||
score = int(match.group(1))
|
||||
timestamp = comment.get("updated_at") or comment.get("created_at") or ""
|
||||
candidates.append((timestamp, score, comment))
|
||||
|
||||
if not candidates:
|
||||
return None
|
||||
|
||||
candidates.sort(key=lambda triple: triple[0])
|
||||
_, score, comment = candidates[-1]
|
||||
return score, comment
|
||||
|
||||
|
||||
def parse_iso8601(value: str) -> dt.datetime:
|
||||
"""Parse a GitHub ISO-8601 timestamp into a timezone-aware datetime."""
|
||||
return dt.datetime.fromisoformat(value.replace("Z", "+00:00"))
|
||||
|
||||
|
||||
def has_optout_label(pr: dict, optout_labels: set[str]) -> bool:
|
||||
labels = {label.get("name", "").lower() for label in pr.get("labels", [])}
|
||||
return bool(labels & {lbl.lower() for lbl in optout_labels})
|
||||
|
|
|
|||
461
.github/scripts/triage_with_llm.py
vendored
461
.github/scripts/triage_with_llm.py
vendored
|
|
@ -30,18 +30,55 @@ Environment:
|
|||
from __future__ import annotations
|
||||
|
||||
import argparse
|
||||
import datetime as dt
|
||||
import json
|
||||
import os
|
||||
import re
|
||||
import subprocess
|
||||
import sys
|
||||
import textwrap
|
||||
from typing import Any
|
||||
import urllib.parse
|
||||
from typing import Any, Iterable
|
||||
|
||||
DEFAULT_MODEL = "gpt-5.4-mini"
|
||||
|
||||
INTERNAL_ASSOCIATIONS = frozenset({"OWNER", "MEMBER", "COLLABORATOR"})
|
||||
|
||||
# --- Review-gate ("ready for review" label lifecycle) configuration ----------
|
||||
# The review gate keeps a single label in sync with whether a PR currently
|
||||
# clears BOTH quality bars: the LLM rubric (clear problem + expected/actual +
|
||||
# QA proof, or a linked issue) AND Greptile's most recent confidence score.
|
||||
READY_FOR_REVIEW_LABEL = "ready for review"
|
||||
DEFAULT_GRACE_DAYS = 1 # 24h before an un-passing, un-tagged PR is auto-closed
|
||||
DEFAULT_MIN_GREPTILE_SCORE = 4 # Greptile < 4/5 counts as "not passing"
|
||||
|
||||
# Hidden HTML-comment markers stamped into review-gate comments. They never
|
||||
# render in the GitHub UI but let the gate detect its own prior actions so it
|
||||
# (a) posts the within-grace "what's missing" notice at most once and (b) can
|
||||
# tell a first-time pass ("ready for review") from a recovery after a
|
||||
# regression ("all clear again"). They deliberately do NOT contain
|
||||
# AGENT_SHIN_AUTO_CLOSE_MARKER, so review-gate chatter on an open PR never
|
||||
# trips the reconsider provenance check (which keys off the close marker).
|
||||
READY_MARKER = "<!-- agent-shin:ready -->"
|
||||
REGRESSED_MARKER = "<!-- agent-shin:regressed -->"
|
||||
WITHIN_GRACE_MARKER = "<!-- agent-shin:within-grace -->"
|
||||
|
||||
# Greptile's GitHub App appears as `greptile-apps[bot]` in REST API comments
|
||||
# and `greptile-apps` in `gh pr view --json` output. Accept either form. These
|
||||
# live here (rather than in close_low_quality_prs.py) so both the daily sweep
|
||||
# and the review gate read the score through one implementation — drift would
|
||||
# silently let one path close/label a PR the other would spare.
|
||||
GREPTILE_BOT_LOGINS = frozenset({"greptile-apps", "greptile-apps[bot]"})
|
||||
|
||||
# Matches lines like:
|
||||
# <h3>Confidence Score: 3/5</h3>
|
||||
# **Confidence Score: 4/5**
|
||||
# Confidence Score: 5 / 5
|
||||
SCORE_PATTERN = re.compile(
|
||||
r"confidence\s*score\s*[:\-]?\s*(\d+)\s*/\s*5",
|
||||
re.IGNORECASE,
|
||||
)
|
||||
|
||||
# Marker phrase Agent Shin always includes in its auto-close comments
|
||||
# (see `format_pr_close_comment` / `format_issue_close_comment`). The
|
||||
# provenance check for reconsider matches this marker against a comment
|
||||
|
|
@ -169,6 +206,35 @@ def reopen_issue(repo: str, number: int) -> None:
|
|||
)
|
||||
|
||||
|
||||
def add_label(repo: str, number: int, label: str) -> None:
|
||||
"""Add a label to a PR/issue (GitHub creates the label if it's missing)."""
|
||||
gh(
|
||||
"api",
|
||||
f"repos/{repo}/issues/{number}/labels",
|
||||
"-X",
|
||||
"POST",
|
||||
"-f",
|
||||
f"labels[]={label}",
|
||||
)
|
||||
|
||||
|
||||
def remove_label(repo: str, number: int, label: str) -> None:
|
||||
"""Remove a label from a PR/issue. A missing label (404) is not an error."""
|
||||
encoded = urllib.parse.quote(label, safe="")
|
||||
try:
|
||||
gh(
|
||||
"api",
|
||||
f"repos/{repo}/issues/{number}/labels/{encoded}",
|
||||
"-X",
|
||||
"DELETE",
|
||||
)
|
||||
except subprocess.CalledProcessError as exc:
|
||||
stderr = (exc.stderr or "").lower()
|
||||
if "404" in stderr or "not found" in stderr:
|
||||
return
|
||||
raise
|
||||
|
||||
|
||||
def fetch_issue_comments(repo: str, number: int) -> list[dict]:
|
||||
"""Fetch all issue-style comments on a PR/issue (paginated).
|
||||
|
||||
|
|
@ -309,6 +375,43 @@ def is_internal_contributor(item: dict) -> bool:
|
|||
return False
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Greptile score + age helpers (shared with close_low_quality_prs.py)
|
||||
|
||||
|
||||
def extract_greptile_score(comments: Iterable[dict]) -> tuple[int, dict] | None:
|
||||
"""Return (score, comment) for the most recent Greptile-authored comment
|
||||
that contains a "Confidence Score: X/5". Returns None if no such comment.
|
||||
|
||||
"Most recent" is determined by the comment's `updated_at` (falling back to
|
||||
`created_at`), so re-reviews override earlier passes.
|
||||
"""
|
||||
candidates: list[tuple[str, int, dict]] = []
|
||||
for comment in comments:
|
||||
user = (comment.get("user") or {}).get("login", "")
|
||||
if user not in GREPTILE_BOT_LOGINS:
|
||||
continue
|
||||
body = comment.get("body") or ""
|
||||
match = SCORE_PATTERN.search(body)
|
||||
if not match:
|
||||
continue
|
||||
score = int(match.group(1))
|
||||
timestamp = comment.get("updated_at") or comment.get("created_at") or ""
|
||||
candidates.append((timestamp, score, comment))
|
||||
|
||||
if not candidates:
|
||||
return None
|
||||
|
||||
candidates.sort(key=lambda triple: triple[0])
|
||||
_, score, comment = candidates[-1]
|
||||
return score, comment
|
||||
|
||||
|
||||
def parse_iso8601(value: str) -> dt.datetime:
|
||||
"""Parse a GitHub ISO-8601 timestamp into a timezone-aware datetime."""
|
||||
return dt.datetime.fromisoformat(value.replace("Z", "+00:00"))
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Prompt construction
|
||||
|
||||
|
|
@ -328,8 +431,7 @@ def build_pr_prompt(*, title: str, body: str) -> str:
|
|||
# Dedent the static template *before* interpolating dynamic fields so that
|
||||
# multi-line bodies (whose 2nd+ lines start at column 0) don't defeat the
|
||||
# common-indent computation in textwrap.dedent.
|
||||
template = textwrap.dedent(
|
||||
"""
|
||||
template = textwrap.dedent("""
|
||||
You are "Agent Shin", the OSS triage bot for the LiteLLM open-source
|
||||
repository (BerriAI/litellm). Decide whether this external pull request
|
||||
meets the project's contribution standards.
|
||||
|
|
@ -375,8 +477,7 @@ def build_pr_prompt(*, title: str, body: str) -> str:
|
|||
---
|
||||
{cleaned_body}
|
||||
---
|
||||
"""
|
||||
).strip()
|
||||
""").strip()
|
||||
return template.format(title=title, cleaned_body=cleaned_body)
|
||||
|
||||
|
||||
|
|
@ -385,8 +486,7 @@ def build_issue_prompt(*, title: str, body: str) -> str:
|
|||
# Dedent the static template *before* interpolating dynamic fields so that
|
||||
# multi-line bodies (whose 2nd+ lines start at column 0) don't defeat the
|
||||
# common-indent computation in textwrap.dedent.
|
||||
template = textwrap.dedent(
|
||||
"""
|
||||
template = textwrap.dedent("""
|
||||
You are "Agent Shin", the OSS triage bot for the LiteLLM open-source
|
||||
repository (BerriAI/litellm). Decide whether this GitHub issue meets
|
||||
the project's reporting standards.
|
||||
|
|
@ -429,8 +529,7 @@ def build_issue_prompt(*, title: str, body: str) -> str:
|
|||
---
|
||||
{cleaned_body}
|
||||
---
|
||||
"""
|
||||
).strip()
|
||||
""").strip()
|
||||
return template.format(title=title, cleaned_body=cleaned_body)
|
||||
|
||||
|
||||
|
|
@ -621,6 +720,282 @@ def format_reconsider_still_failing_comment(kind: str, verdict: dict) -> str:
|
|||
)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Review gate — "ready for review" label lifecycle
|
||||
|
||||
_UNSET = object()
|
||||
|
||||
|
||||
def _combine_missing(
|
||||
verdict: dict, greptile_score: int | None, min_score: int
|
||||
) -> list[str]:
|
||||
"""Merge the LLM rubric's `missing` list with a Greptile-score shortfall."""
|
||||
missing = list(verdict.get("missing") or [])
|
||||
if greptile_score is not None and greptile_score < min_score:
|
||||
missing.insert(
|
||||
0,
|
||||
f"Greptile's most recent review scored this PR {greptile_score}/5 "
|
||||
f"(below the {min_score}/5 bar)",
|
||||
)
|
||||
return missing or ["(see explanation below)"]
|
||||
|
||||
|
||||
def _has_marker(comments: Iterable[dict], marker: str) -> bool:
|
||||
return any(marker in (comment.get("body") or "") for comment in comments)
|
||||
|
||||
|
||||
def format_ready_for_review_comment(verdict: dict, greptile_score: int | None) -> str:
|
||||
"""Posted the first time a PR clears the bar (label added)."""
|
||||
score_line = (
|
||||
f" Greptile scored it **{greptile_score}/5**."
|
||||
if greptile_score is not None
|
||||
else ""
|
||||
)
|
||||
explanation = verdict.get("explanation") or ""
|
||||
return (
|
||||
"✅ **Triage passed — tagging `ready for review`.**\n"
|
||||
"\n"
|
||||
"Agent Shin checked this PR against the "
|
||||
"[contribution rubric](https://github.com/BerriAI/litellm/blob/main/.github/pull_request_template.md) "
|
||||
"and it clears the bar (a linked issue, or a clear problem description "
|
||||
f"+ expected vs. actual + QA proof).{score_line}\n"
|
||||
"\n"
|
||||
f"> {explanation}\n"
|
||||
"\n"
|
||||
"A maintainer will take it from here. If a later re-check finds the PR "
|
||||
f"has regressed (Greptile drops below {DEFAULT_MIN_GREPTILE_SCORE}/5, "
|
||||
"the QA proof is removed, etc.) I'll pull the tag and comment with "
|
||||
"what's missing — fix it and the tag comes back automatically.\n"
|
||||
f"{READY_MARKER}"
|
||||
)
|
||||
|
||||
|
||||
def format_all_clear_comment(verdict: dict, greptile_score: int | None) -> str:
|
||||
"""Posted when a PR recovers after a regression (label re-added)."""
|
||||
score_line = (
|
||||
f" Greptile is back to **{greptile_score}/5**."
|
||||
if greptile_score is not None
|
||||
else ""
|
||||
)
|
||||
explanation = verdict.get("explanation") or ""
|
||||
return (
|
||||
"✅ **All clear again — re-adding `ready for review`.**\n"
|
||||
"\n"
|
||||
"Thanks for addressing the earlier feedback. On re-check this PR meets "
|
||||
f"the contribution bar once more.{score_line}\n"
|
||||
"\n"
|
||||
f"> {explanation}\n"
|
||||
"\n"
|
||||
"A maintainer will take another look.\n"
|
||||
f"{READY_MARKER}"
|
||||
)
|
||||
|
||||
|
||||
def format_regression_comment(missing: list[str], explanation: str) -> str:
|
||||
"""Posted when a previously-tagged PR regresses (label removed, PR stays open)."""
|
||||
return (
|
||||
"⚠️ **Removing the `ready for review` tag.**\n"
|
||||
"\n"
|
||||
"On a re-check this PR no longer meets the contribution bar. What's "
|
||||
"missing now:\n"
|
||||
"\n"
|
||||
f"{_format_missing(missing)}\n"
|
||||
"\n"
|
||||
f"> {explanation}\n"
|
||||
"\n"
|
||||
"The PR stays open — address the points above and Agent Shin will post "
|
||||
'an "all clear" comment and re-add the tag automatically.\n'
|
||||
f"{REGRESSED_MARKER}"
|
||||
)
|
||||
|
||||
|
||||
def format_within_grace_comment(
|
||||
missing: list[str], explanation: str, grace_days: int
|
||||
) -> str:
|
||||
"""Posted once while a failing PR is still inside its grace window."""
|
||||
window = "24 hours" if grace_days == 1 else f"{grace_days} days"
|
||||
return (
|
||||
"👋 Hi, thanks for the PR! This is **Agent Shin**, the automated triage "
|
||||
"bot. This PR doesn't meet the contribution bar yet:\n"
|
||||
"\n"
|
||||
f"{_format_missing(missing)}\n"
|
||||
"\n"
|
||||
f"> {explanation}\n"
|
||||
"\n"
|
||||
f"You have ~{window} from when this PR was opened to add the missing "
|
||||
"pieces. Once it passes I'll tag it `ready for review`; otherwise I'll "
|
||||
"auto-close it (you can always re-open the conversation with "
|
||||
"`@agent-shin reconsider`).\n"
|
||||
f"{WITHIN_GRACE_MARKER}"
|
||||
)
|
||||
|
||||
|
||||
def review_gate(
|
||||
*,
|
||||
repo: str,
|
||||
number: int,
|
||||
close: bool,
|
||||
model: str,
|
||||
judge: Any = None,
|
||||
greptile_score: Any = _UNSET,
|
||||
comments: Any = _UNSET,
|
||||
now: dt.datetime | None = None,
|
||||
grace_days: int = DEFAULT_GRACE_DAYS,
|
||||
min_greptile_score: int = DEFAULT_MIN_GREPTILE_SCORE,
|
||||
label: str = READY_FOR_REVIEW_LABEL,
|
||||
) -> dict:
|
||||
"""Reconcile the `ready for review` label with a PR's current quality.
|
||||
|
||||
A PR is *passing* when it clears BOTH gates: the LLM rubric (linked issue,
|
||||
or problem description + expected/actual + QA proof) AND Greptile's most
|
||||
recent confidence score (>= ``min_greptile_score``; absence of a score is
|
||||
not held against the PR). The gate then drives a small state machine, using
|
||||
the label itself as the persisted state so comments fire only on
|
||||
transitions (never on every scheduled run):
|
||||
|
||||
passing, untagged -> add label + "ready for review" / "all clear"
|
||||
passing, tagged -> noop-passing
|
||||
not passing, tagged -> remove label + regression comment (stays open)
|
||||
not passing, untagged, old -> close + comment (past the grace window)
|
||||
not passing, untagged, new -> one-time "what's missing" notice (within grace)
|
||||
|
||||
``close`` gates every destructive side effect: with ``close=False`` the
|
||||
function returns a ``would-*`` preview and touches nothing, mirroring the
|
||||
dry-run contract of :func:`triage`. ``judge``/``greptile_score``/
|
||||
``comments``/``now`` are injectable for tests; in production they are
|
||||
resolved from the OpenAI judge, the PR's Greptile comment, the live comment
|
||||
list, and the wall clock respectively.
|
||||
"""
|
||||
item = fetch_pr(repo, number)
|
||||
|
||||
title = item.get("title") or ""
|
||||
body = item.get("body") or ""
|
||||
login = (item.get("user") or {}).get("login") or ""
|
||||
association = item.get("author_association") or ""
|
||||
state = item.get("state") or ""
|
||||
labels_now = {(lbl.get("name") or "") for lbl in (item.get("labels") or [])}
|
||||
created_raw = item.get("created_at") or ""
|
||||
|
||||
base_result = {
|
||||
"kind": "pr",
|
||||
"number": number,
|
||||
"title": title,
|
||||
"author": login,
|
||||
"author_association": association,
|
||||
"state": state,
|
||||
"labeled": label in labels_now,
|
||||
"review_gate": True,
|
||||
}
|
||||
|
||||
if state != "open":
|
||||
return {**base_result, "action": "skip-not-open"}
|
||||
|
||||
if is_internal_contributor(item):
|
||||
return {**base_result, "action": "skip-internal-author"}
|
||||
|
||||
# Resolve the comment list once — used for both the Greptile score and the
|
||||
# marker-based dedup below.
|
||||
if comments is _UNSET:
|
||||
comments = fetch_issue_comments(repo, number)
|
||||
|
||||
# --- rubric verdict: linked-issue short-circuit, else the LLM judge -------
|
||||
if has_linked_issue(body):
|
||||
verdict = {
|
||||
"verdict": "pass",
|
||||
"linked_issue": True,
|
||||
"missing": [],
|
||||
"explanation": "Linked-issue regex matched; LLM was not called.",
|
||||
}
|
||||
rubric_pass = True
|
||||
else:
|
||||
prompt = build_pr_prompt(title=title, body=body)
|
||||
if judge is None:
|
||||
api_key = os.environ.get("OPENAI_API_KEY")
|
||||
if not api_key:
|
||||
return {**base_result, "action": "skip-no-llm-key"}
|
||||
base_url = os.environ.get("OPENAI_BASE_URL") or None
|
||||
|
||||
def judge(p: str) -> str:
|
||||
return call_llm_judge(
|
||||
p, model=model, api_key=api_key, base_url=base_url
|
||||
)
|
||||
|
||||
try:
|
||||
verdict = parse_verdict(judge(prompt))
|
||||
except Exception as exc: # noqa: BLE001 - judge errors must never act
|
||||
return {**base_result, "action": "skip-llm-error", "error": str(exc)}
|
||||
rubric_pass = (verdict.get("verdict") or "").lower() == "pass"
|
||||
|
||||
# --- Greptile score -------------------------------------------------------
|
||||
if greptile_score is _UNSET:
|
||||
extraction = extract_greptile_score(comments)
|
||||
greptile_score = extraction[0] if extraction else None
|
||||
greptile_ok = greptile_score is None or greptile_score >= min_greptile_score
|
||||
passing = rubric_pass and greptile_ok
|
||||
|
||||
# --- age ------------------------------------------------------------------
|
||||
age_days = None
|
||||
if created_raw:
|
||||
reference = now or dt.datetime.now(dt.timezone.utc)
|
||||
age_days = (reference - parse_iso8601(created_raw)).days
|
||||
|
||||
label_present = label in labels_now
|
||||
explanation = verdict.get("explanation") or ""
|
||||
base_result = {
|
||||
**base_result,
|
||||
"verdict": verdict,
|
||||
"greptile_score": greptile_score,
|
||||
"passing": passing,
|
||||
"age_days": age_days,
|
||||
}
|
||||
|
||||
if passing:
|
||||
if label_present:
|
||||
return {**base_result, "action": "noop-passing"}
|
||||
recovered = _has_marker(comments, REGRESSED_MARKER)
|
||||
comment = (
|
||||
format_all_clear_comment(verdict, greptile_score)
|
||||
if recovered
|
||||
else format_ready_for_review_comment(verdict, greptile_score)
|
||||
)
|
||||
if not close:
|
||||
return {**base_result, "action": "would-label-ready", "comment": comment}
|
||||
post_comment(repo, number, comment)
|
||||
add_label(repo, number, label)
|
||||
return {**base_result, "action": "labeled-ready", "comment": comment}
|
||||
|
||||
missing = _combine_missing(verdict, greptile_score, min_greptile_score)
|
||||
|
||||
if label_present:
|
||||
comment = format_regression_comment(missing, explanation)
|
||||
if not close:
|
||||
return {**base_result, "action": "would-remove-label", "comment": comment}
|
||||
remove_label(repo, number, label)
|
||||
post_comment(repo, number, comment)
|
||||
return {**base_result, "action": "label-removed-regressed", "comment": comment}
|
||||
|
||||
# Not passing and not tagged: close if past the grace window, else notify once.
|
||||
if age_days is not None and age_days >= grace_days:
|
||||
comment = format_pr_close_comment({**verdict, "missing": missing})
|
||||
if not close:
|
||||
return {**base_result, "action": "would-close", "comment": comment}
|
||||
post_comment(repo, number, comment)
|
||||
close_pr(repo, number)
|
||||
return {**base_result, "action": "closed", "comment": comment}
|
||||
|
||||
if _has_marker(comments, WITHIN_GRACE_MARKER):
|
||||
return {**base_result, "action": "within-grace-already-notified"}
|
||||
comment = format_within_grace_comment(missing, explanation, grace_days)
|
||||
if not close:
|
||||
return {
|
||||
**base_result,
|
||||
"action": "would-notify-within-grace",
|
||||
"comment": comment,
|
||||
}
|
||||
post_comment(repo, number, comment)
|
||||
return {**base_result, "action": "within-grace-notified", "comment": comment}
|
||||
|
||||
|
||||
def triage(
|
||||
*,
|
||||
repo: str,
|
||||
|
|
@ -842,6 +1217,16 @@ def render_summary(result: dict) -> str:
|
|||
f"- **Author**: `{result.get('author', '')}` ({result.get('author_association', '')})"
|
||||
)
|
||||
lines.append(f"- **State**: {result.get('state', '')}")
|
||||
if result.get("review_gate"):
|
||||
score = result.get("greptile_score")
|
||||
lines.append(
|
||||
f"- **Greptile**: {score}/5"
|
||||
if score is not None
|
||||
else "- **Greptile**: (no score yet)"
|
||||
)
|
||||
lines.append(f"- **`ready for review` label present**: {result.get('labeled')}")
|
||||
if result.get("age_days") is not None:
|
||||
lines.append(f"- **Age**: {result['age_days']}d")
|
||||
lines.append(f"- **Action**: `{result['action']}`")
|
||||
verdict = result.get("verdict")
|
||||
if verdict:
|
||||
|
|
@ -898,20 +1283,60 @@ def main() -> int:
|
|||
"PR/issue author or an internal collaborator."
|
||||
),
|
||||
)
|
||||
parser.add_argument(
|
||||
"--review-gate",
|
||||
action="store_true",
|
||||
help=(
|
||||
"Reconcile the `ready for review` label for an OPEN PR: tag on "
|
||||
"pass, remove the tag + comment on regression, close after the "
|
||||
"grace window if it never passed. PR-only."
|
||||
),
|
||||
)
|
||||
parser.add_argument(
|
||||
"--grace-days",
|
||||
type=int,
|
||||
default=DEFAULT_GRACE_DAYS,
|
||||
help=(
|
||||
"Review-gate only: hours/24 a failing, un-tagged PR may stay open "
|
||||
f"before auto-close (default: {DEFAULT_GRACE_DAYS} = 24h)."
|
||||
),
|
||||
)
|
||||
parser.add_argument(
|
||||
"--min-greptile-score",
|
||||
type=int,
|
||||
default=DEFAULT_MIN_GREPTILE_SCORE,
|
||||
choices=range(1, 6),
|
||||
help=(
|
||||
"Review-gate only: Greptile score below which a PR counts as not "
|
||||
f"passing (default: {DEFAULT_MIN_GREPTILE_SCORE} -> <4/5 regresses)."
|
||||
),
|
||||
)
|
||||
args = parser.parse_args()
|
||||
|
||||
kind = "pr" if args.pr is not None else "issue"
|
||||
number = args.pr if args.pr is not None else args.issue
|
||||
|
||||
result = triage(
|
||||
repo=args.repo,
|
||||
kind=kind,
|
||||
number=number,
|
||||
close=args.close,
|
||||
model=args.model,
|
||||
print_prompt=args.print_prompt,
|
||||
reconsider=args.reconsider,
|
||||
)
|
||||
if args.review_gate:
|
||||
if kind != "pr":
|
||||
parser.error("--review-gate applies to pull requests only (use --pr).")
|
||||
result = review_gate(
|
||||
repo=args.repo,
|
||||
number=number,
|
||||
close=args.close,
|
||||
model=args.model,
|
||||
grace_days=args.grace_days,
|
||||
min_greptile_score=args.min_greptile_score,
|
||||
)
|
||||
else:
|
||||
result = triage(
|
||||
repo=args.repo,
|
||||
kind=kind,
|
||||
number=number,
|
||||
close=args.close,
|
||||
model=args.model,
|
||||
print_prompt=args.print_prompt,
|
||||
reconsider=args.reconsider,
|
||||
)
|
||||
|
||||
if result.get("action") == "print-prompt":
|
||||
print(result["prompt"])
|
||||
|
|
|
|||
128
.github/workflows/review_gate.yml
vendored
Normal file
128
.github/workflows/review_gate.yml
vendored
Normal file
|
|
@ -0,0 +1,128 @@
|
|||
name: Agent Shin — review gate
|
||||
|
||||
# Keeps the `ready for review` label in sync with whether an external PR
|
||||
# currently clears BOTH the LLM rubric AND Greptile's confidence score.
|
||||
#
|
||||
# pass -> add `ready for review` + a "passed / all clear" comment
|
||||
# regress -> remove the label + a "what's missing" comment (PR stays open)
|
||||
# fail, <24h old -> a one-time "what's missing" notice (grace window)
|
||||
# fail, >24h old -> close + a comment (reopen via `@agent-shin reconsider`)
|
||||
#
|
||||
# DRY-RUN BY DEFAULT. Every side effect (label add/remove, comment, close) is
|
||||
# gated behind `--close`, which is only added when the repo variable
|
||||
# `AGENT_SHIN_ENABLED == "true"`. Until then runs only write the verdict to the
|
||||
# workflow step summary.
|
||||
#
|
||||
# Manual single PR: gh workflow run "Agent Shin — review gate" -f pr_number=NNN
|
||||
# Manual dry-run: gh workflow run "Agent Shin — review gate" -f close=false
|
||||
#
|
||||
# We use `pull_request_target` so the workflow can read repo secrets and run
|
||||
# against fork PRs. Fork code is never checked out — only PR metadata is read
|
||||
# via `gh api`.
|
||||
|
||||
on:
|
||||
pull_request_target:
|
||||
types: [opened, reopened, synchronize, ready_for_review]
|
||||
schedule:
|
||||
# Daily at 09:30 UTC — re-reconciles labels as Greptile re-reviews land.
|
||||
- cron: "30 9 * * *"
|
||||
workflow_dispatch:
|
||||
inputs:
|
||||
pr_number:
|
||||
description: "Single PR to reconcile (omit to sweep all open PRs)."
|
||||
required: false
|
||||
close:
|
||||
description: "If AGENT_SHIN_ENABLED=true, actually act (false = dry run)."
|
||||
required: false
|
||||
default: "false"
|
||||
type: choice
|
||||
options:
|
||||
- "true"
|
||||
- "false"
|
||||
grace_days:
|
||||
description: "Hours/24 a failing, un-tagged PR may stay open before close."
|
||||
required: false
|
||||
default: "1"
|
||||
min_greptile_score:
|
||||
description: "Greptile score below which a PR counts as not passing (1-5)."
|
||||
required: false
|
||||
default: "4"
|
||||
|
||||
permissions:
|
||||
contents: read
|
||||
issues: write
|
||||
pull-requests: write
|
||||
|
||||
jobs:
|
||||
review-gate:
|
||||
if: github.repository == 'BerriAI/litellm'
|
||||
runs-on: ubuntu-latest
|
||||
steps:
|
||||
- name: Checkout triage script
|
||||
uses: actions/checkout@08eba0b27e820071cde6df949e0beb9ba4906955 # v4.3.0
|
||||
with:
|
||||
sparse-checkout: .github/scripts
|
||||
persist-credentials: false
|
||||
|
||||
- name: Set up Python
|
||||
uses: actions/setup-python@a26af69be951a213d495a4c3e4e4022e16d87065 # v5.6.0
|
||||
with:
|
||||
python-version: "3.12"
|
||||
|
||||
- name: Install LLM client
|
||||
run: pip install --no-cache-dir "openai>=1.40.0"
|
||||
|
||||
- name: Run review gate
|
||||
env:
|
||||
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
|
||||
# Mirror the triage workflow: only expose the LLM key when the bot is
|
||||
# enabled or a collaborator triggers it manually, so an external user
|
||||
# can't force paid LLM calls by churning a fork PR while the bot is
|
||||
# still in dry-run.
|
||||
OPENAI_API_KEY: ${{ (vars.AGENT_SHIN_ENABLED == 'true' || github.event_name == 'workflow_dispatch') && secrets.OPENAI_API_KEY || '' }}
|
||||
OPENAI_BASE_URL: ${{ vars.OPENAI_BASE_URL }}
|
||||
TRIAGE_MODEL: ${{ vars.TRIAGE_MODEL }}
|
||||
AGENT_SHIN_ENABLED: ${{ vars.AGENT_SHIN_ENABLED }}
|
||||
CLOSE_FLAG: ${{ github.event.inputs.close || 'false' }}
|
||||
GRACE_DAYS: ${{ github.event.inputs.grace_days || '1' }}
|
||||
MIN_GREPTILE_SCORE: ${{ github.event.inputs.min_greptile_score || '4' }}
|
||||
EVENT_PR: ${{ github.event.pull_request.number }}
|
||||
INPUT_PR: ${{ github.event.inputs.pr_number }}
|
||||
run: |
|
||||
set -euo pipefail
|
||||
COMMON=(--review-gate --grace-days "${GRACE_DAYS}" --min-greptile-score "${MIN_GREPTILE_SCORE}")
|
||||
|
||||
# Fail-safe gating, identical philosophy to the Greptile closer:
|
||||
# - AGENT_SHIN_ENABLED must be the EXACT string "true" to act at all.
|
||||
# - A manual dispatch can still preview with close=false.
|
||||
# - Automatic triggers (PR events, schedule) act once enabled — that
|
||||
# is the whole point of the gate (re-tag / un-tag automatically).
|
||||
DO_CLOSE="false"
|
||||
if [ "${AGENT_SHIN_ENABLED:-false}" != "true" ]; then
|
||||
echo "::notice::AGENT_SHIN_ENABLED is not 'true' -> dry-run (no labels/comments/closes)."
|
||||
elif [ "${GITHUB_EVENT_NAME:-}" = "workflow_dispatch" ] && [ "${CLOSE_FLAG:-false}" = "true" ]; then
|
||||
DO_CLOSE="true"
|
||||
echo "::notice::Manual run -> acting for real."
|
||||
elif [ "${GITHUB_EVENT_NAME:-}" != "workflow_dispatch" ]; then
|
||||
DO_CLOSE="true"
|
||||
echo "::notice::Enabled automatic trigger (${GITHUB_EVENT_NAME:-}) -> acting for real."
|
||||
else
|
||||
echo "::notice::Manual dispatch with close=false -> dry-run."
|
||||
fi
|
||||
if [ "${DO_CLOSE}" = "true" ]; then
|
||||
COMMON+=(--close)
|
||||
fi
|
||||
|
||||
# Single PR (PR event or explicit input) vs. sweep over all open PRs.
|
||||
TARGET_PR="${EVENT_PR:-${INPUT_PR:-}}"
|
||||
if [ -n "${TARGET_PR}" ]; then
|
||||
python3 .github/scripts/triage_with_llm.py --repo "${{ github.repository }}" --pr "${TARGET_PR}" "${COMMON[@]}"
|
||||
else
|
||||
echo "::notice::Sweeping all open PRs."
|
||||
mapfile -t NUMBERS < <(gh pr list --repo "${{ github.repository }}" --state open --limit 1000 --json number --jq '.[].number')
|
||||
for n in "${NUMBERS[@]}"; do
|
||||
echo "::group::PR #${n}"
|
||||
python3 .github/scripts/triage_with_llm.py --repo "${{ github.repository }}" --pr "${n}" "${COMMON[@]}" || echo "::warning::review gate errored on #${n}"
|
||||
echo "::endgroup::"
|
||||
done
|
||||
fi
|
||||
385
tests/test_litellm/test_github_review_gate.py
Normal file
385
tests/test_litellm/test_github_review_gate.py
Normal file
|
|
@ -0,0 +1,385 @@
|
|||
"""Unit tests for the `ready for review` label lifecycle (Agent Shin review gate).
|
||||
|
||||
Exercises `triage_with_llm.review_gate`, the state machine that keeps the
|
||||
`ready for review` label in sync with whether a PR clears both the LLM rubric
|
||||
and Greptile's confidence score:
|
||||
|
||||
* pass (untagged) -> add label + "ready for review" comment
|
||||
* pass (untagged, recovered) -> add label + "all clear again" comment
|
||||
* pass (already tagged) -> noop
|
||||
* regress (tagged) -> remove label + "what's missing" comment, stays open
|
||||
* fail (untagged, within 24h)-> one-time "what's missing" notice
|
||||
* fail (untagged, >24h) -> close + comment
|
||||
* dry run (close=False) -> would-* previews, no side effects
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import datetime as dt
|
||||
import importlib.util
|
||||
import sys
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
|
||||
SCRIPT_PATH = (
|
||||
Path(__file__).resolve().parents[2] / ".github" / "scripts" / "triage_with_llm.py"
|
||||
)
|
||||
|
||||
NOW = dt.datetime(2026, 5, 24, 12, 0, 0, tzinfo=dt.timezone.utc)
|
||||
JUST_NOW = "2026-05-24T11:00:00Z" # 1h old -> within 24h grace
|
||||
TWO_DAYS_AGO = "2026-05-22T11:00:00Z" # >24h old -> past grace
|
||||
|
||||
|
||||
@pytest.fixture(scope="module")
|
||||
def triage_module():
|
||||
spec = importlib.util.spec_from_file_location("triage_with_llm", SCRIPT_PATH)
|
||||
assert spec and spec.loader
|
||||
module = importlib.util.module_from_spec(spec)
|
||||
sys.modules["triage_with_llm"] = module
|
||||
spec.loader.exec_module(module)
|
||||
return module
|
||||
|
||||
|
||||
class _Recorder:
|
||||
"""Captures every gh mutation review_gate could fire, and fails loudly
|
||||
on the ones a given scenario forbids."""
|
||||
|
||||
def __init__(self, triage_module, monkeypatch):
|
||||
self.comments: list[str] = []
|
||||
self.added: list[str] = []
|
||||
self.removed: list[str] = []
|
||||
self.closed: list[int] = []
|
||||
monkeypatch.setattr(
|
||||
triage_module,
|
||||
"post_comment",
|
||||
lambda repo, n, body: self.comments.append(body),
|
||||
)
|
||||
monkeypatch.setattr(
|
||||
triage_module,
|
||||
"add_label",
|
||||
lambda repo, n, label: self.added.append(label),
|
||||
)
|
||||
monkeypatch.setattr(
|
||||
triage_module,
|
||||
"remove_label",
|
||||
lambda repo, n, label: self.removed.append(label),
|
||||
)
|
||||
monkeypatch.setattr(
|
||||
triage_module,
|
||||
"close_pr",
|
||||
lambda repo, n: self.closed.append(n),
|
||||
)
|
||||
|
||||
|
||||
def _make_pr(**overrides):
|
||||
base = {
|
||||
"number": 7,
|
||||
"title": "feat: do a thing",
|
||||
"body": "some body without a linked issue or QA proof",
|
||||
"state": "open",
|
||||
"author_association": "NONE",
|
||||
"user": {"login": "outside-dev"},
|
||||
"labels": [],
|
||||
"created_at": JUST_NOW,
|
||||
}
|
||||
base.update(overrides)
|
||||
return base
|
||||
|
||||
|
||||
def _pass(prompt):
|
||||
return '{"verdict": "pass", "missing": [], "explanation": "looks good"}'
|
||||
|
||||
|
||||
def _fail(prompt):
|
||||
return (
|
||||
'{"verdict": "fail", "missing": ["QA proof", "expected vs. actual"],'
|
||||
' "explanation": "thin description"}'
|
||||
)
|
||||
|
||||
|
||||
def _gate(triage_module, **kwargs):
|
||||
"""Call review_gate with safe defaults for the injectable hooks."""
|
||||
params = dict(
|
||||
repo="o/r",
|
||||
number=7,
|
||||
close=True,
|
||||
model="m",
|
||||
judge=_pass,
|
||||
greptile_score=None,
|
||||
comments=[],
|
||||
now=NOW,
|
||||
)
|
||||
params.update(kwargs)
|
||||
return triage_module.review_gate(**params)
|
||||
|
||||
|
||||
class TestReviewGatePass:
|
||||
def test_pass_untagged_adds_label_and_ready_comment(
|
||||
self, triage_module, monkeypatch
|
||||
):
|
||||
monkeypatch.setattr(triage_module, "fetch_pr", lambda repo, n: _make_pr())
|
||||
rec = _Recorder(triage_module, monkeypatch)
|
||||
|
||||
result = _gate(triage_module, judge=_pass, greptile_score=5)
|
||||
|
||||
assert result["action"] == "labeled-ready"
|
||||
assert rec.added == [triage_module.READY_FOR_REVIEW_LABEL]
|
||||
assert rec.removed == [] and rec.closed == []
|
||||
assert len(rec.comments) == 1
|
||||
assert "ready for review" in rec.comments[0].lower()
|
||||
assert triage_module.READY_MARKER in rec.comments[0]
|
||||
assert "5/5" in rec.comments[0]
|
||||
|
||||
def test_pass_already_tagged_is_noop(self, triage_module, monkeypatch):
|
||||
pr = _make_pr(labels=[{"name": "ready for review"}])
|
||||
monkeypatch.setattr(triage_module, "fetch_pr", lambda repo, n: pr)
|
||||
rec = _Recorder(triage_module, monkeypatch)
|
||||
|
||||
result = _gate(triage_module, judge=_pass, greptile_score=5)
|
||||
|
||||
assert result["action"] == "noop-passing"
|
||||
assert rec.added == [] and rec.removed == [] and rec.comments == []
|
||||
|
||||
def test_pass_after_prior_regression_uses_all_clear_wording(
|
||||
self, triage_module, monkeypatch
|
||||
):
|
||||
# A regression marker in history -> this is a recovery, not a first pass.
|
||||
monkeypatch.setattr(triage_module, "fetch_pr", lambda repo, n: _make_pr())
|
||||
rec = _Recorder(triage_module, monkeypatch)
|
||||
prior = [{"user": {"login": "x"}, "body": triage_module.REGRESSED_MARKER}]
|
||||
|
||||
result = _gate(triage_module, judge=_pass, greptile_score=5, comments=prior)
|
||||
|
||||
assert result["action"] == "labeled-ready"
|
||||
assert "all clear" in rec.comments[0].lower()
|
||||
|
||||
def test_linked_issue_passes_without_calling_judge(
|
||||
self, triage_module, monkeypatch
|
||||
):
|
||||
pr = _make_pr(body="Fixes #4321\n\nbody")
|
||||
monkeypatch.setattr(triage_module, "fetch_pr", lambda repo, n: pr)
|
||||
rec = _Recorder(triage_module, monkeypatch)
|
||||
|
||||
result = _gate(
|
||||
triage_module,
|
||||
judge=lambda p: pytest.fail("LLM must not be called for linked issue"),
|
||||
greptile_score=5,
|
||||
)
|
||||
assert result["action"] == "labeled-ready"
|
||||
assert rec.added == [triage_module.READY_FOR_REVIEW_LABEL]
|
||||
|
||||
|
||||
class TestReviewGateRegression:
|
||||
def test_regression_removes_label_and_keeps_pr_open(
|
||||
self, triage_module, monkeypatch
|
||||
):
|
||||
pr = _make_pr(labels=[{"name": "ready for review"}])
|
||||
monkeypatch.setattr(triage_module, "fetch_pr", lambda repo, n: pr)
|
||||
rec = _Recorder(triage_module, monkeypatch)
|
||||
|
||||
result = _gate(triage_module, judge=_fail, greptile_score=5)
|
||||
|
||||
assert result["action"] == "label-removed-regressed"
|
||||
assert rec.removed == [triage_module.READY_FOR_REVIEW_LABEL]
|
||||
assert rec.closed == [] # regression NEVER closes the PR
|
||||
assert triage_module.REGRESSED_MARKER in rec.comments[0]
|
||||
assert "QA proof" in rec.comments[0]
|
||||
|
||||
def test_greptile_drop_alone_triggers_regression(self, triage_module, monkeypatch):
|
||||
# Rubric still passes, but Greptile fell to 2/5 -> not passing.
|
||||
pr = _make_pr(labels=[{"name": "ready for review"}])
|
||||
monkeypatch.setattr(triage_module, "fetch_pr", lambda repo, n: pr)
|
||||
rec = _Recorder(triage_module, monkeypatch)
|
||||
|
||||
result = _gate(triage_module, judge=_pass, greptile_score=2)
|
||||
|
||||
assert result["action"] == "label-removed-regressed"
|
||||
assert rec.removed == [triage_module.READY_FOR_REVIEW_LABEL]
|
||||
assert "2/5" in rec.comments[0]
|
||||
|
||||
def test_greptile_score_read_from_comments_when_not_injected(
|
||||
self, triage_module, monkeypatch
|
||||
):
|
||||
pr = _make_pr(labels=[{"name": "ready for review"}])
|
||||
monkeypatch.setattr(triage_module, "fetch_pr", lambda repo, n: pr)
|
||||
rec = _Recorder(triage_module, monkeypatch)
|
||||
greptile = [
|
||||
{
|
||||
"user": {"login": "greptile-apps[bot]"},
|
||||
"body": "Confidence Score: 2/5",
|
||||
"created_at": "2026-05-24T10:00:00Z",
|
||||
}
|
||||
]
|
||||
|
||||
result = _gate(
|
||||
triage_module,
|
||||
judge=_pass,
|
||||
greptile_score=triage_module._UNSET,
|
||||
comments=greptile,
|
||||
)
|
||||
assert result["action"] == "label-removed-regressed"
|
||||
assert "2/5" in rec.comments[0]
|
||||
|
||||
|
||||
class TestReviewGateGraceAndClose:
|
||||
def test_within_grace_posts_one_time_notice(self, triage_module, monkeypatch):
|
||||
monkeypatch.setattr(
|
||||
triage_module, "fetch_pr", lambda repo, n: _make_pr(created_at=JUST_NOW)
|
||||
)
|
||||
rec = _Recorder(triage_module, monkeypatch)
|
||||
|
||||
result = _gate(triage_module, judge=_fail, greptile_score=None)
|
||||
|
||||
assert result["action"] == "within-grace-notified"
|
||||
assert rec.closed == [] and rec.added == [] and rec.removed == []
|
||||
assert triage_module.WITHIN_GRACE_MARKER in rec.comments[0]
|
||||
assert "QA proof" in rec.comments[0]
|
||||
|
||||
def test_within_grace_does_not_double_notify(self, triage_module, monkeypatch):
|
||||
monkeypatch.setattr(
|
||||
triage_module, "fetch_pr", lambda repo, n: _make_pr(created_at=JUST_NOW)
|
||||
)
|
||||
rec = _Recorder(triage_module, monkeypatch)
|
||||
prior = [{"user": {"login": "b"}, "body": triage_module.WITHIN_GRACE_MARKER}]
|
||||
|
||||
result = _gate(triage_module, judge=_fail, greptile_score=None, comments=prior)
|
||||
|
||||
assert result["action"] == "within-grace-already-notified"
|
||||
assert rec.comments == []
|
||||
|
||||
def test_past_grace_closes_with_comment(self, triage_module, monkeypatch):
|
||||
monkeypatch.setattr(
|
||||
triage_module,
|
||||
"fetch_pr",
|
||||
lambda repo, n: _make_pr(created_at=TWO_DAYS_AGO),
|
||||
)
|
||||
rec = _Recorder(triage_module, monkeypatch)
|
||||
|
||||
result = _gate(triage_module, judge=_fail, greptile_score=None)
|
||||
|
||||
assert result["action"] == "closed"
|
||||
assert rec.closed == [7]
|
||||
assert len(rec.comments) == 1
|
||||
# The close comment must carry the reconsider provenance marker.
|
||||
assert triage_module.AGENT_SHIN_AUTO_CLOSE_MARKER in rec.comments[0]
|
||||
|
||||
|
||||
class TestReviewGateDryRun:
|
||||
@pytest.mark.parametrize(
|
||||
"scenario,labels,judge,score,created,expected",
|
||||
[
|
||||
("pass", [], _pass, 5, JUST_NOW, "would-label-ready"),
|
||||
(
|
||||
"regress",
|
||||
[{"name": "ready for review"}],
|
||||
_fail,
|
||||
5,
|
||||
JUST_NOW,
|
||||
"would-remove-label",
|
||||
),
|
||||
("within-grace", [], _fail, None, JUST_NOW, "would-notify-within-grace"),
|
||||
("past-grace", [], _fail, None, TWO_DAYS_AGO, "would-close"),
|
||||
],
|
||||
)
|
||||
def test_dry_run_previews_without_side_effects(
|
||||
self,
|
||||
triage_module,
|
||||
monkeypatch,
|
||||
scenario,
|
||||
labels,
|
||||
judge,
|
||||
score,
|
||||
created,
|
||||
expected,
|
||||
):
|
||||
pr = _make_pr(labels=labels, created_at=created)
|
||||
monkeypatch.setattr(triage_module, "fetch_pr", lambda repo, n: pr)
|
||||
rec = _Recorder(triage_module, monkeypatch)
|
||||
|
||||
result = _gate(triage_module, close=False, judge=judge, greptile_score=score)
|
||||
|
||||
assert result["action"] == expected
|
||||
# Dry run touches nothing.
|
||||
assert rec.added == [] and rec.removed == [] and rec.closed == []
|
||||
assert rec.comments == []
|
||||
assert "comment" in result # preview body still surfaced
|
||||
|
||||
|
||||
class TestReviewGateGuards:
|
||||
def test_skips_internal_author(self, triage_module, monkeypatch):
|
||||
pr = _make_pr(author_association="MEMBER", user={"login": "krrish"})
|
||||
monkeypatch.setattr(triage_module, "fetch_pr", lambda repo, n: pr)
|
||||
result = _gate(
|
||||
triage_module, judge=lambda p: pytest.fail("no LLM for internal")
|
||||
)
|
||||
assert result["action"] == "skip-internal-author"
|
||||
|
||||
def test_skips_closed_pr(self, triage_module, monkeypatch):
|
||||
pr = _make_pr(state="closed")
|
||||
monkeypatch.setattr(triage_module, "fetch_pr", lambda repo, n: pr)
|
||||
result = _gate(triage_module, judge=lambda p: pytest.fail("no LLM for closed"))
|
||||
assert result["action"] == "skip-not-open"
|
||||
|
||||
def test_llm_error_is_non_destructive(self, triage_module, monkeypatch):
|
||||
monkeypatch.setattr(triage_module, "fetch_pr", lambda repo, n: _make_pr())
|
||||
rec = _Recorder(triage_module, monkeypatch)
|
||||
|
||||
def boom(prompt):
|
||||
raise RuntimeError("api down")
|
||||
|
||||
result = _gate(triage_module, judge=boom, greptile_score=None)
|
||||
|
||||
assert result["action"] == "skip-llm-error"
|
||||
assert rec.closed == [] and rec.added == [] and rec.removed == []
|
||||
|
||||
def test_full_recovery_cycle(self, triage_module, monkeypatch):
|
||||
"""pass -> regress -> recover, threading labels/comments like GitHub would."""
|
||||
state = {"labels": [], "comments": []}
|
||||
|
||||
def fake_fetch(repo, n):
|
||||
return _make_pr(labels=list(state["labels"]), created_at=JUST_NOW)
|
||||
|
||||
monkeypatch.setattr(triage_module, "fetch_pr", fake_fetch)
|
||||
monkeypatch.setattr(
|
||||
triage_module,
|
||||
"post_comment",
|
||||
lambda repo, n, body: state["comments"].append(
|
||||
{"user": {"login": "agent-shin[bot]"}, "body": body}
|
||||
),
|
||||
)
|
||||
monkeypatch.setattr(
|
||||
triage_module,
|
||||
"add_label",
|
||||
lambda repo, n, label: state["labels"].append({"name": label}),
|
||||
)
|
||||
monkeypatch.setattr(
|
||||
triage_module,
|
||||
"remove_label",
|
||||
lambda repo, n, label: state["labels"].clear(),
|
||||
)
|
||||
monkeypatch.setattr(
|
||||
triage_module, "close_pr", lambda repo, n: pytest.fail("must not close")
|
||||
)
|
||||
|
||||
# 1) passes -> tagged
|
||||
r1 = _gate(
|
||||
triage_module, judge=_pass, greptile_score=5, comments=state["comments"]
|
||||
)
|
||||
assert r1["action"] == "labeled-ready"
|
||||
assert any(lbl["name"] == "ready for review" for lbl in state["labels"])
|
||||
|
||||
# 2) regresses -> tag removed, comment posted, PR still open
|
||||
r2 = _gate(
|
||||
triage_module, judge=_fail, greptile_score=2, comments=state["comments"]
|
||||
)
|
||||
assert r2["action"] == "label-removed-regressed"
|
||||
assert state["labels"] == []
|
||||
|
||||
# 3) fixed again -> "all clear" + tag back
|
||||
r3 = _gate(
|
||||
triage_module, judge=_pass, greptile_score=5, comments=state["comments"]
|
||||
)
|
||||
assert r3["action"] == "labeled-ready"
|
||||
assert any(lbl["name"] == "ready for review" for lbl in state["labels"])
|
||||
assert "all clear" in state["comments"][-1]["body"].lower()
|
||||
|
|
@ -43,6 +43,7 @@ DESTRUCTIVE_GATE_ENV: dict[str, str] = {
|
|||
"triage_pr_with_llm.yml": "DISPATCH_CLOSE",
|
||||
"triage_issue_with_llm.yml": "DISPATCH_CLOSE",
|
||||
"close_low_quality_prs.yml": "CLOSE_FLAG",
|
||||
"review_gate.yml": "CLOSE_FLAG",
|
||||
# The reconsider workflow has no per-run "really do it?" knob — its
|
||||
# only kill switch is `AGENT_SHIN_ENABLED`, which already serves as
|
||||
# both the destructive gate and the global enablement gate.
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue