From b506501126dc1ad09defb6d339c2936de5defc11 Mon Sep 17 00:00:00 2001 From: mateo Date: Sun, 16 Aug 2026 20:59:40 +0000 Subject: [PATCH] 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> --- .github/scripts/agent_shin_shared.py | 13 ++++++++----- .github/scripts/close_low_quality_prs.py | 19 +++++++++++-------- .github/scripts/triage_rollout_heads_up.py | 5 +++-- .github/scripts/triage_with_llm.py | 19 ++++++++++++------- .../test_github_close_low_quality_prs.py | 8 ++++---- .../test_github_triage_with_llm.py | 10 +++++----- .../test_triage_rollout_heads_up.py | 4 +++- 7 files changed, 46 insertions(+), 32 deletions(-) diff --git a/.github/scripts/agent_shin_shared.py b/.github/scripts/agent_shin_shared.py index 8f3dc3c2322..75e86daef51 100644 --- a/.github/scripts/agent_shin_shared.py +++ b/.github/scripts/agent_shin_shared.py @@ -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 = "" # rejects the contributor. AGENT_SHIN_CLOSE_MARKER = "" -# 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]" diff --git a/.github/scripts/close_low_quality_prs.py b/.github/scripts/close_low_quality_prs.py index 7b9bbb579e3..cdb0fe670e0 100644 --- a/.github/scripts/close_low_quality_prs.py +++ b/.github/scripts/close_low_quality_prs.py @@ -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 ( diff --git a/.github/scripts/triage_rollout_heads_up.py b/.github/scripts/triage_rollout_heads_up.py index a5dedb1c9e7..44f69ed82ce 100644 --- a/.github/scripts/triage_rollout_heads_up.py +++ b/.github/scripts/triage_rollout_heads_up.py @@ -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" diff --git a/.github/scripts/triage_with_llm.py b/.github/scripts/triage_with_llm.py index e23a012425a..dad7eab6b69 100644 --- a/.github/scripts/triage_with_llm.py +++ b/.github/scripts/triage_with_llm.py @@ -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" diff --git a/tests/test_litellm/test_github_close_low_quality_prs.py b/tests/test_litellm/test_github_close_low_quality_prs.py index e3b653dde64..43e93debcf3 100644 --- a/tests/test_litellm/test_github_close_low_quality_prs.py +++ b/tests/test_litellm/test_github_close_low_quality_prs.py @@ -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) diff --git a/tests/test_litellm/test_github_triage_with_llm.py b/tests/test_litellm/test_github_triage_with_llm.py index 96b77e80457..90231f1bfc7 100644 --- a/tests/test_litellm/test_github_triage_with_llm.py +++ b/tests/test_litellm/test_github_triage_with_llm.py @@ -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 diff --git a/tests/test_litellm/test_triage_rollout_heads_up.py b/tests/test_litellm/test_triage_rollout_heads_up.py index 535590fd2d6..303115fc61d 100644 --- a/tests/test_litellm/test_triage_rollout_heads_up.py +++ b/tests/test_litellm/test_triage_rollout_heads_up.py @@ -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