mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-13 23:11:40 +00:00
fix(agent-shin): give contributors 24 hours of grace before auto-close
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
This commit is contained in:
parent
973329e986
commit
b506501126
7 changed files with 46 additions and 32 deletions
13
.github/scripts/agent_shin_shared.py
vendored
13
.github/scripts/agent_shin_shared.py
vendored
|
|
@ -7,7 +7,8 @@ agree on the same notions of:
|
|||
* What counts as a Greptile-authored review comment
|
||||
(``GREPTILE_BOT_LOGINS``) and how to extract a confidence score from
|
||||
its body (``SCORE_PATTERN`` / :func:`extract_greptile_score`).
|
||||
* How long the 2-hour grace window is (``GRACE_PERIOD_SECONDS``) and
|
||||
* How long the grace window is (``GRACE_PERIOD_SECONDS``, rendered for
|
||||
contributor-facing comments as ``GRACE_PERIOD_LABEL``) and
|
||||
the HTML marker stamped into a grace-warning comment so the *other*
|
||||
script can see "Agent Shin already warned" and behave accordingly
|
||||
(``GRACE_COMMENT_MARKER``).
|
||||
|
|
@ -49,10 +50,12 @@ GRACE_COMMENT_MARKER = "<!-- agent-shin:grace-warning -->"
|
|||
# rejects the contributor.
|
||||
AGENT_SHIN_CLOSE_MARKER = "<!-- agent-shin:closed -->"
|
||||
|
||||
# 2 hours between the grace warning and the auto-close. Short enough to
|
||||
# dogfood the "fix it before it closes" loop in one sitting; bump back up
|
||||
# (e.g. 86400 for a day) for the public rollout.
|
||||
GRACE_PERIOD_SECONDS = 7200
|
||||
GRACE_PERIOD_SECONDS = 86400
|
||||
|
||||
# Every contributor-facing mention of the grace window renders this instead of
|
||||
# a hardcoded duration, so changing GRACE_PERIOD_SECONDS can't leave the warning
|
||||
# and close comments promising a window the code no longer honors.
|
||||
GRACE_PERIOD_LABEL = f"{GRACE_PERIOD_SECONDS // 3600} hours"
|
||||
|
||||
AGENT_SHIN_DEFAULT_BOT_LOGIN = "github-actions[bot]"
|
||||
|
||||
|
|
|
|||
19
.github/scripts/close_low_quality_prs.py
vendored
19
.github/scripts/close_low_quality_prs.py
vendored
|
|
@ -52,6 +52,7 @@ from agent_shin_shared import ( # noqa: E402 -- sys.path adjusted above
|
|||
AGENT_SHIN_CLOSE_MARKER,
|
||||
ALLOWLIST_LOGINS,
|
||||
GRACE_COMMENT_MARKER,
|
||||
GRACE_PERIOD_LABEL,
|
||||
GRACE_PERIOD_SECONDS,
|
||||
GREPTILE_BOT_LOGINS,
|
||||
SCORE_PATTERN,
|
||||
|
|
@ -81,7 +82,7 @@ DEFAULT_OPTOUT_LABELS = ("do not close", "keep open", "wip")
|
|||
# `GRACE_COMMENT_MARKER` (HTML marker appended to grace-period warning
|
||||
# comments — used by either script to recognize that a warning was
|
||||
# already posted) and `GRACE_PERIOD_SECONDS` (length of the grace
|
||||
# period between the warning and the actual auto-close, 2 hours) are
|
||||
# period between the warning and the actual auto-close) are
|
||||
# imported from `agent_shin_shared` so the Agent Shin LLM judge and
|
||||
# this daily Greptile sweep agree on the same marker and duration.
|
||||
|
||||
|
|
@ -190,11 +191,11 @@ def seconds_since_last_grace_warning(
|
|||
|
||||
def format_grace_warning_comment(score: int, threshold: int) -> str:
|
||||
"""Comment posted on the FIRST low-Greptile-score detection — gives
|
||||
the contributor a 2-hour grace window before the auto-close fires on
|
||||
the next daily cron run.
|
||||
the contributor a `GRACE_PERIOD_SECONDS` window before the auto-close
|
||||
fires on the next daily cron run.
|
||||
|
||||
Mirrors `format_grace_warning_pr_comment` in
|
||||
`triage_with_llm.py` in spirit (2-hour grace + escape hatches), but
|
||||
`triage_with_llm.py` in spirit (same grace window + escape hatches), but
|
||||
framed around Greptile's confidence score instead of the LLM judge's
|
||||
rubric since the close trigger here is the Greptile signal.
|
||||
"""
|
||||
|
|
@ -205,7 +206,8 @@ def format_grace_warning_comment(score: int, threshold: int) -> str:
|
|||
"Heads up: Greptile's most recent review scored this PR "
|
||||
f"**{score}/5**, below our merge bar of **{threshold}/5**.\n"
|
||||
"\n"
|
||||
"If the score isn't lifted in the next **2 hours**, I'll auto-close this PR. That's "
|
||||
f"If the score isn't lifted in the next **{GRACE_PERIOD_LABEL}**, I'll auto-close this "
|
||||
"PR. That's "
|
||||
"**not** us saying the change isn't worthwhile. We want the open-PR list to mirror "
|
||||
"what a maintainer can act on *right now*, so contributors like you don't get lost in "
|
||||
"a backlog. Take your time; everything below still works after the close.\n"
|
||||
|
|
@ -215,7 +217,8 @@ def format_grace_warning_comment(score: int, threshold: int) -> str:
|
|||
f"the new score is **{threshold}/5 or higher**, the PR stays open and no further "
|
||||
"action is needed on your side.\n"
|
||||
"\n"
|
||||
"**If the PR does get auto-closed in 2 hours, you still have an easy recovery path:**\n"
|
||||
f"**If the PR does get auto-closed in {GRACE_PERIOD_LABEL}, you still have an easy "
|
||||
"recovery path:**\n"
|
||||
"\n"
|
||||
"- Comment `@greptileai` to request a fresh review. **This still works even after "
|
||||
f"the PR is closed**, and a score of {threshold}/5 or higher is one of the signals "
|
||||
|
|
@ -234,7 +237,7 @@ def post_grace_warning(
|
|||
repo: str | None,
|
||||
dry_run: bool,
|
||||
) -> None:
|
||||
"""Post the 2-hour grace-period warning comment on `pr`.
|
||||
"""Post the grace-period warning comment on `pr`.
|
||||
|
||||
The warning carries `GRACE_COMMENT_MARKER` so subsequent runs can
|
||||
detect that the contributor has already been told about the
|
||||
|
|
@ -268,7 +271,7 @@ def format_close_comment(score: int, threshold: int) -> str:
|
|||
"""
|
||||
score_sentence = (
|
||||
f"Greptile's most recent review scored this PR **{score}/5**, below "
|
||||
f"our merge bar of **{threshold}/5**, and the 2-hour grace period since "
|
||||
f"our merge bar of **{threshold}/5**, and the {GRACE_PERIOD_LABEL} grace period since "
|
||||
"the warning has elapsed.\n\n"
|
||||
)
|
||||
return (
|
||||
|
|
|
|||
5
.github/scripts/triage_rollout_heads_up.py
vendored
5
.github/scripts/triage_rollout_heads_up.py
vendored
|
|
@ -49,6 +49,7 @@ from _agent_shin_actions import maybe_post_comment # noqa: E402
|
|||
from agent_shin_shared import ( # noqa: E402
|
||||
AGENT_SHIN_DEFAULT_BOT_LOGIN,
|
||||
ALLOWLIST_LOGINS,
|
||||
GRACE_PERIOD_LABEL,
|
||||
list_open_items,
|
||||
)
|
||||
from triage_with_llm import ( # noqa: E402
|
||||
|
|
@ -223,8 +224,8 @@ def format_heads_up_comment(
|
|||
f"- If this {noun} still fails the rubric at **{cutoff_str}**, "
|
||||
"we'll close it.\n"
|
||||
f"- From then on the bot runs daily, and every {noun} that fails "
|
||||
"the rubric gets a **2-hour lifetime**: one warning comment, then "
|
||||
"auto-close 2 hours later.\n"
|
||||
f"the rubric gets a **{GRACE_PERIOD_LABEL} lifetime**: one warning "
|
||||
f"comment, then auto-close {GRACE_PERIOD_LABEL} later.\n"
|
||||
"\n"
|
||||
f"{_recovery_section(kind)}\n"
|
||||
"\n"
|
||||
|
|
|
|||
19
.github/scripts/triage_with_llm.py
vendored
19
.github/scripts/triage_with_llm.py
vendored
|
|
@ -52,6 +52,7 @@ from agent_shin_shared import ( # noqa: E402 -- sys.path adjusted above
|
|||
AGENT_SHIN_DEFAULT_BOT_LOGIN,
|
||||
ALLOWLIST_LOGINS,
|
||||
GRACE_COMMENT_MARKER,
|
||||
GRACE_PERIOD_LABEL,
|
||||
GRACE_PERIOD_SECONDS,
|
||||
GREPTILE_BOT_LOGINS,
|
||||
SCORE_PATTERN,
|
||||
|
|
@ -88,7 +89,7 @@ RECONSIDER_RATE_LIMIT_SECONDS = 600
|
|||
# posted on the first low-quality detection — used on subsequent triage
|
||||
# runs to detect that a warning was already posted and measure how long
|
||||
# ago it was posted) and `GRACE_PERIOD_SECONDS` (length of the grace
|
||||
# period between the warning and the actual auto-close, 2 hours) are
|
||||
# period between the warning and the actual auto-close) are
|
||||
# imported from `agent_shin_shared` so the daily Greptile sweep and the
|
||||
# LLM judge agree on the same marker and duration.
|
||||
|
||||
|
|
@ -869,8 +870,8 @@ def format_issue_close_comment(verdict: dict) -> str:
|
|||
|
||||
def format_grace_warning_pr_comment(verdict: dict) -> str:
|
||||
"""Comment posted on the FIRST low-quality detection — gives the
|
||||
contributor a 2-hour grace window to fix the PR before the next
|
||||
triage run actually closes it.
|
||||
contributor a `GRACE_PERIOD_SECONDS` window to fix the PR before the
|
||||
next triage run actually closes it.
|
||||
|
||||
This is the "before-close" warning. On the second triage run, if the
|
||||
grace marker is older than `GRACE_PERIOD_SECONDS` AND the PR still
|
||||
|
|
@ -896,7 +897,8 @@ def format_grace_warning_pr_comment(verdict: dict) -> str:
|
|||
"\n"
|
||||
f"> {explanation}\n"
|
||||
"\n"
|
||||
"If the description isn't updated in the next **2 hours**, I'll auto-close this PR. "
|
||||
f"If the description isn't updated in the next **{GRACE_PERIOD_LABEL}**, I'll auto-close "
|
||||
"this PR. "
|
||||
"That's **not** us saying we don't care about the change; we want the open-PR list to "
|
||||
"mirror what a maintainer can act on *right now*, so contributors don't get lost in a "
|
||||
'backlog. A closed PR is a soft "park this for later," not a rejection. Take your '
|
||||
|
|
@ -908,7 +910,8 @@ def format_grace_warning_pr_comment(verdict: dict) -> str:
|
|||
"[what counts as QA proof](https://docs.litellm.ai/blog/agent-shin-triage#the-rubric-for-pull-requests) "
|
||||
"for the full rubric (a linked issue alone isn't enough; it covers context, not proof).\n"
|
||||
"\n"
|
||||
"**If the PR does get auto-closed in 2 hours, you still have easy recovery paths:**\n"
|
||||
f"**If the PR does get auto-closed in {GRACE_PERIOD_LABEL}, you still have easy recovery "
|
||||
"paths:**\n"
|
||||
"\n"
|
||||
"- Comment `@agent-shin reconsider` after updating the description. I'll re-evaluate "
|
||||
"and reopen the PR if it now passes.\n"
|
||||
|
|
@ -944,7 +947,8 @@ def format_grace_warning_issue_comment(verdict: dict) -> str:
|
|||
"\n"
|
||||
f"> {explanation}\n"
|
||||
"\n"
|
||||
"If the issue isn't updated in the next **2 hours**, I'll auto-close it. That's **not** us "
|
||||
f"If the issue isn't updated in the next **{GRACE_PERIOD_LABEL}**, I'll auto-close it. "
|
||||
"That's **not** us "
|
||||
"saying the bug isn't real or the request isn't useful; we want the open-issue list "
|
||||
"to mirror what a maintainer can act on *right now*, so reports like yours don't get "
|
||||
'buried in a backlog. A closed issue is a soft "park this for later," not a '
|
||||
|
|
@ -965,7 +969,8 @@ def format_grace_warning_issue_comment(verdict: dict) -> str:
|
|||
"dead-end (a video, a screenshot, or the exact commands you ran with their real "
|
||||
"output showing where the flow stops today). Mocked or stubbed runs don't count.\n"
|
||||
"\n"
|
||||
"**If the issue does get auto-closed in 2 hours**, comment `@agent-shin reconsider` "
|
||||
f"**If the issue does get auto-closed in {GRACE_PERIOD_LABEL}**, comment "
|
||||
"`@agent-shin reconsider` "
|
||||
"and I'll re-evaluate. If it now meets the bar, I'll reopen the issue.\n"
|
||||
"\n"
|
||||
"Internal BerriAI contributors: this rubric doesn't apply to you; ping a maintainer.\n"
|
||||
|
|
|
|||
|
|
@ -176,7 +176,7 @@ class TestEvaluatePr:
|
|||
# `min_age_days=0` means no age filter — a freshly-opened PR is
|
||||
# eligible the moment Greptile scores it below threshold. The
|
||||
# first detection still goes through the warn-grace step rather
|
||||
# than closing immediately, giving the contributor 2 hours to
|
||||
# than closing immediately, giving the contributor a grace window to
|
||||
# respond before the next run actually closes the PR.
|
||||
monkeypatch.setattr(
|
||||
closer_module,
|
||||
|
|
@ -321,7 +321,7 @@ class TestEvaluatePr:
|
|||
old_warning = {
|
||||
"user": {"login": "github-actions[bot]"},
|
||||
"body": (
|
||||
"you have 2 hours to fix this\n\n" + closer_module.GRACE_COMMENT_MARKER
|
||||
"you have a grace window to fix this\n\n" + closer_module.GRACE_COMMENT_MARKER
|
||||
),
|
||||
"created_at": (
|
||||
_now - dt.timedelta(seconds=closer_module.GRACE_PERIOD_SECONDS + 60)
|
||||
|
|
@ -355,7 +355,7 @@ class TestEvaluatePr:
|
|||
def test_should_skip_when_grace_warning_within_window(
|
||||
self, closer_module, _now, monkeypatch
|
||||
):
|
||||
# Within the 2-hour grace window the closer must NOT close the
|
||||
# Within the grace window the closer must NOT close the
|
||||
# PR even if the score is still low. The warning is only an hour
|
||||
# old; give the contributor time to push fixes before destruction.
|
||||
recent_warning = {
|
||||
|
|
@ -573,7 +573,7 @@ class TestGraceWarningCommentText:
|
|||
body = closer_module.format_grace_warning_comment(score=2, threshold=4)
|
||||
# The user's PR explicitly said "specify in the comment" — pin
|
||||
# that the grace window appears in the comment.
|
||||
assert "2 hours" in body
|
||||
assert closer_module.GRACE_PERIOD_LABEL in body
|
||||
|
||||
def test_should_mention_agent_shin_reconsider(self, closer_module):
|
||||
body = closer_module.format_grace_warning_comment(score=2, threshold=4)
|
||||
|
|
|
|||
|
|
@ -1729,7 +1729,7 @@ class TestTriageOrchestration:
|
|||
assert result["action"] == "warned-grace"
|
||||
assert posted["n"] == 42
|
||||
# Pin the user-facing language pieces the user explicitly asked for.
|
||||
assert "2 hours" in posted["body"]
|
||||
assert triage_module.GRACE_PERIOD_LABEL in posted["body"]
|
||||
assert "@agent-shin reconsider" in posted["body"]
|
||||
assert "@greptileai" in posted["body"]
|
||||
assert "even after the PR is closed" in posted["body"]
|
||||
|
|
@ -1804,7 +1804,7 @@ class TestTriageOrchestration:
|
|||
judge=lambda p: json.dumps(verdict),
|
||||
)
|
||||
assert result["action"] == "would-warn-grace"
|
||||
assert "2 hours" in result["comment"]
|
||||
assert triage_module.GRACE_PERIOD_LABEL in result["comment"]
|
||||
|
||||
def test_should_warn_grace_for_swiftwinds_not_close_instantly(
|
||||
self, triage_module, monkeypatch
|
||||
|
|
@ -1846,7 +1846,7 @@ class TestTriageOrchestration:
|
|||
judge=lambda p: json.dumps(verdict),
|
||||
)
|
||||
assert result["action"] == "warned-grace"
|
||||
assert "2 hours" in posted["body"]
|
||||
assert triage_module.GRACE_PERIOD_LABEL in posted["body"]
|
||||
|
||||
|
||||
class TestGraceWarningCommentText:
|
||||
|
|
@ -1858,7 +1858,7 @@ class TestGraceWarningCommentText:
|
|||
{"verdict": "fail", "missing": ["QA proof"], "explanation": "thin"}
|
||||
)
|
||||
# The user explicitly asked: "specify in the comment" the grace window.
|
||||
assert "2 hours" in body
|
||||
assert triage_module.GRACE_PERIOD_LABEL in body
|
||||
|
||||
def test_pr_grace_warning_should_mention_reconsider_during_grace(
|
||||
self, triage_module
|
||||
|
|
@ -1892,7 +1892,7 @@ class TestGraceWarningCommentText:
|
|||
{"verdict": "fail", "missing": [], "explanation": ""}
|
||||
)
|
||||
assert triage_module.GRACE_COMMENT_MARKER in body
|
||||
assert "2 hours" in body
|
||||
assert triage_module.GRACE_PERIOD_LABEL in body
|
||||
# OSS authors can't reopen a bot-closed issue, so recovery is
|
||||
# `@agent-shin reconsider` (the bot reopens), like the PR path.
|
||||
assert "@agent-shin reconsider" in body
|
||||
|
|
|
|||
|
|
@ -174,7 +174,9 @@ class TestHeadsUpCommentBody:
|
|||
assert "Monday, June 1, 2026" in body # cutoff readable
|
||||
assert "09:00 UTC" in body # deadline is timezone-explicit
|
||||
assert "we'll close it" in body # hard deadline, not a passive notice
|
||||
assert "2-hour lifetime" in body # post-rollout steady state
|
||||
assert (
|
||||
f"{heads_up_module.GRACE_PERIOD_LABEL} lifetime" in body
|
||||
) # post-rollout steady state
|
||||
assert "Greptile" in body and "3/5" in body # specific shortfall
|
||||
assert "QA proof" in body # missing piece surfaced
|
||||
assert "PR *description*" in body # description-only note
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue