From 9e41a22fdb386e865657ca0fd9da2d4ac4e4f765 Mon Sep 17 00:00:00 2001 From: mateo-berri <277851410+mateo-berri@users.noreply.github.com> Date: Mon, 25 May 2026 20:27:41 -0700 Subject: [PATCH] feat(triage): add one-shot 7-day heads-up sweep for Agent Shin rollout MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds a rollout-day workflow that comments on every open external PR/issue that the new triage bot WOULD auto-close, giving contributors 7 days to fix their description before any destructive action runs. Why now: merging this PR enables Agent Shin in dry-run. The follow-up "enact" PR (next Monday) flips the destructive paths on. Without this heads-up, contributors would get a close-comment on day 8 with no prior warning. The heads-up names the cutoff date, lists the rubric, calls out each PR/issue's specific missing pieces, and explains the recovery paths (@agent-shin reconsider for PRs, edit + reopen for issues). Files - .github/scripts/_agent_shin_actions.py — thin maybe_post_comment / maybe_close_* / maybe_add_label / etc. wrappers. Each is a single `if dry_run: log; return; else: call_through()` so a dry-run preview differs from the real run in exactly one call site per mutation. The call-through goes via `triage_with_llm.` (module-qualified) so monkeypatching the underlying function in tests is reflected here. - .github/scripts/triage_rollout_heads_up.py — the sweep. Iterates every open PR + issue via `gh pr list` / `gh issue list`, runs the future rubric (review_gate for PRs, triage(kind="issue") for issues), and posts the heads-up on any item that would be auto-closed. Idempotent via a `` marker. Defaults to dry- run; --close opts in to real posts. --close-on overrides the cutoff date (defaults to today + 7 days). - .github/workflows/triage_rollout_heads_up.yml — one-shot workflow. Triggers on push to litellm_internal_staging filtered to the script path (fires on rollout merge) plus workflow_dispatch with a dry_run input that defaults to "true" for safe manual re-runs. - tests/test_litellm/test_triage_rollout_heads_up.py — 28 unit tests covering: the dry-run wrappers (each maybe_* gates correctly), the _would_be_closed predicate for PR vs. issue results, the comment formatter (cutoff/rubric/marker/recovery wording), per-item dispatch (skip-not-open, skip-internal-author, skip-already-notified, skip-passing, would-post/posted), and the sweep loop end-to-end. Local preview (no GitHub mutations): python3 .github/scripts/triage_rollout_heads_up.py --repo BerriAI/litellm Real run (what the workflow does): python3 .github/scripts/triage_rollout_heads_up.py --repo BerriAI/litellm --close TODO: replace the placeholder ROLLOUT_BLOG_URL with the canonical docs URL once the litellm-docs PR ships. Co-Authored-By: Claude Opus 4.7 --- .github/scripts/_agent_shin_actions.py | 105 +++ .github/scripts/triage_rollout_heads_up.py | 518 +++++++++++++++ .github/workflows/triage_rollout_heads_up.yml | 85 +++ .../test_triage_rollout_heads_up.py | 628 ++++++++++++++++++ 4 files changed, 1336 insertions(+) create mode 100644 .github/scripts/_agent_shin_actions.py create mode 100644 .github/scripts/triage_rollout_heads_up.py create mode 100644 .github/workflows/triage_rollout_heads_up.yml create mode 100644 tests/test_litellm/test_triage_rollout_heads_up.py diff --git a/.github/scripts/_agent_shin_actions.py b/.github/scripts/_agent_shin_actions.py new file mode 100644 index 00000000000..dabc0ac8dff --- /dev/null +++ b/.github/scripts/_agent_shin_actions.py @@ -0,0 +1,105 @@ +"""Thin dry-run wrappers around every Agent Shin GitHub mutation. + +Every destructive operation the rollout scripts perform (post a comment, close +a PR/issue, reopen, add/remove a label) is funneled through a `maybe_*` helper +here. The helpers take a single ``dry_run: bool`` keyword argument and the +body is intentionally trivial: + + if dry_run: + print(...) # log what we would do, return + return + real_mutation(...) # otherwise, actually do it + +That shape means a dry-run preview differs from the real run in exactly one +line per side effect: the call site. So when you `python3 script.py` locally +without ``--close``, you can be confident the actions printed are the ones the +GitHub Action would have performed (modulo ordering on retry/error paths, +which are deliberately simple). + +Importing from this module pulls in the real mutations from +``triage_with_llm`` — call sites in the rollout scripts should NEVER import +``post_comment`` / ``close_pr`` / etc. directly; that would skip the dry-run +gate and is the bug class this module exists to prevent. +""" + +from __future__ import annotations + +import sys +import textwrap + +# Import the module itself rather than the bare names so monkeypatching +# `triage_with_llm.post_comment` (or any of the other mutations) in tests is +# reflected here — `from triage_with_llm import post_comment` would bind the +# original function to a local name and bypass the patch, defeating the whole +# point of these wrappers. +import triage_with_llm + + +def _log(line: str) -> None: + """Print a single dry-run line to stdout (one log statement per side effect).""" + print(line, file=sys.stdout, flush=True) + + +def maybe_post_comment(repo: str, number: int, body: str, *, dry_run: bool) -> None: + """Post a comment on ``repo#number`` — or, in dry-run, log what we would post.""" + if dry_run: + _log(f"[DRY RUN] comment {repo}#{number}:") + _log(textwrap.indent(body, " ")) + return + triage_with_llm.post_comment(repo, number, body) + + +def maybe_close_pr(repo: str, number: int, *, dry_run: bool) -> None: + """Close ``repo#number`` (PR) — or log it.""" + if dry_run: + _log(f"[DRY RUN] close PR {repo}#{number}") + return + triage_with_llm.close_pr(repo, number) + + +def maybe_close_issue( + repo: str, number: int, *, dry_run: bool, not_planned: bool = True +) -> None: + """Close ``repo#number`` (issue) — or log it. ``not_planned=True`` sets the + standard ``state_reason`` for triage closures so they don't look like + 'completed'.""" + if dry_run: + _log( + f"[DRY RUN] close issue {repo}#{number}" + f" (state_reason={'not_planned' if not_planned else 'completed'})" + ) + return + triage_with_llm.close_issue(repo, number, not_planned=not_planned) + + +def maybe_reopen_pr(repo: str, number: int, *, dry_run: bool) -> None: + """Reopen a previously-closed PR — or log it.""" + if dry_run: + _log(f"[DRY RUN] reopen PR {repo}#{number}") + return + triage_with_llm.reopen_pr(repo, number) + + +def maybe_reopen_issue(repo: str, number: int, *, dry_run: bool) -> None: + """Reopen a previously-closed issue — or log it.""" + if dry_run: + _log(f"[DRY RUN] reopen issue {repo}#{number}") + return + triage_with_llm.reopen_issue(repo, number) + + +def maybe_add_label(repo: str, number: int, label: str, *, dry_run: bool) -> None: + """Add a label to ``repo#number`` — or log it.""" + if dry_run: + _log(f"[DRY RUN] add label {label!r} to {repo}#{number}") + return + triage_with_llm.add_label(repo, number, label) + + +def maybe_remove_label(repo: str, number: int, label: str, *, dry_run: bool) -> None: + """Remove a label from ``repo#number`` — or log it. A missing label is not + an error in the real path either.""" + if dry_run: + _log(f"[DRY RUN] remove label {label!r} from {repo}#{number}") + return + triage_with_llm.remove_label(repo, number, label) diff --git a/.github/scripts/triage_rollout_heads_up.py b/.github/scripts/triage_rollout_heads_up.py new file mode 100644 index 00000000000..0e36c21f9c1 --- /dev/null +++ b/.github/scripts/triage_rollout_heads_up.py @@ -0,0 +1,518 @@ +#!/usr/bin/env python3 +"""One-shot 7-day heads-up sweep for the Agent Shin rollout. + +Posts a friendly "the OSS triage bot kicks in next Monday" comment on every +open external PR/issue that currently *would* fail the new rubric — i.e., +every PR/issue Agent Shin would close once the rollout completes. The point +is to give contributors a full week to fix their description before the bot +ever takes a destructive action, so nobody is surprised by an auto-close. + +The script is designed to run **exactly once**, on the merge commit of the +rollout PR. Re-runs are safe: every comment is stamped with the hidden +``HEADS_UP_MARKER`` and PRs/issues that already carry the marker are skipped. + +Dry-run vs. real run +-------------------- +Defaults to dry-run. The matching GitHub Action passes ``--close`` to flip +into real mode. Every GitHub mutation goes through ``_agent_shin_actions``, +which has a one-line ``if dry_run: log else: do_it`` per call — so the only +difference between a dry-run preview and the real run is the call site that +actually hits the GitHub API. + +Local preview:: + + python3 .github/scripts/triage_rollout_heads_up.py --repo BerriAI/litellm + +Real run (the workflow uses this):: + + python3 .github/scripts/triage_rollout_heads_up.py --repo BerriAI/litellm --close +""" + +from __future__ import annotations + +import argparse +import datetime as dt +import json +import os +import sys +from pathlib import Path +from typing import Any + +# Make the sibling triage_with_llm + _agent_shin_actions importable when this +# script is invoked directly (the GitHub workflow does `python3 .github/scripts/...`). +_SCRIPTS_DIR = Path(__file__).resolve().parent +if str(_SCRIPTS_DIR) not in sys.path: + sys.path.insert(0, str(_SCRIPTS_DIR)) + +from _agent_shin_actions import maybe_post_comment # noqa: E402 +from triage_with_llm import ( # noqa: E402 + DEFAULT_MODEL, + call_llm_judge, + fetch_issue, + fetch_pr, + gh, + is_internal_contributor, + review_gate, + triage, +) + +# Hidden marker so re-runs skip PRs/issues we've already notified. Distinct from +# the within-grace / ready / regressed markers so it can't be confused with the +# steady-state lifecycle comments. +HEADS_UP_MARKER = "" + +# Placeholder until the litellm-docs PR ships. The rollout blog post explains +# the new rubric, the 7-day grace, and how to recover after an auto-close. +# TODO(docs): replace with the canonical URL once the litellm-docs PR merges. +ROLLOUT_BLOG_URL = "https://docs.litellm.ai/docs/agent_shin_triage_rollout" + +# Default cutoff is one week from "now". Computed at runtime so the wording +# stays correct even if the rollout is merged later than planned. The user can +# override with --close-on YYYY-MM-DD when running the script manually. +DEFAULT_GRACE_DAYS = 7 + + +def _rubric_section_pr() -> str: + return ( + "**Going forward, every external PR needs ONE of:**\n" + "\n" + "- A linked GitHub issue using a closing keyword: " + "`Fixes #1234`, `Closes #1234`, or `Resolves #1234`, OR\n" + "- All three of: a clear **problem description**, **expected vs. " + "actual behavior**, and **visual QA proof** " + "(before/after screenshots, a short screen recording, or terminal/" + "log output).\n" + "\n" + "PRs also need a **Greptile confidence score of 4/5 or higher** before " + "the bot will tag them `ready for review`. You can `@greptileai` to " + "request a fresh review at any time, including after the PR is closed." + ) + + +def _rubric_section_issue() -> str: + return ( + "**Going forward, every external issue needs:**\n" + "\n" + "- For **bug reports**: a runnable reproduction (code/curl/config), " + "expected vs. actual behavior, and a screenshot, traceback, or log " + "showing the bug.\n" + "- For **feature requests**: a clear description of the proposed " + "feature plus a use case + concrete example (config, API call, UI " + "flow, or scenario showing what's blocked today)." + ) + + +def _description_only_note(kind: str) -> str: + noun = "PR" if kind == "pr" else "issue" + return ( + f"⚠️ **The requirements must live in the {noun} *description*, not in " + "comments.** Some PRs/issues collect 100+ comments from humans and " + "bots; reading the entire thread on every triage run would balloon " + "GitHub API usage (we'd start getting 429'd) and blow out the LLM " + "judge's context. The bot only reads the description, so anything " + "you add as a comment will be invisible to it." + ) + + +def _missing_section(verdict: dict, greptile_score: int | None) -> str: + """Bullet list of what's currently missing on this PR/issue. + + Combines the LLM judge's `missing` list (rubric items) with a Greptile + shortfall (for PRs) so the contributor sees one list of things to fix. + """ + missing = list(verdict.get("missing") or []) + if greptile_score is not None and greptile_score < 4: + missing.insert( + 0, + f"Greptile's most recent review scored this PR {greptile_score}/5 " + "(below the 4/5 bar Agent Shin will require).", + ) + if not missing: + return ( + "_The bot couldn't articulate a specific missing piece — see the " + "rubric link above and double-check the description includes all " + "of it before the rollout._" + ) + bullets = "\n".join(f"- {m}" for m in missing) + return f"**What this one is currently missing:**\n\n{bullets}" + + +def _recovery_section(kind: str) -> str: + if kind == "pr": + return ( + "**If the bot closes this PR after the rollout:** update the " + "description with the missing pieces, then either open a fresh " + "PR or comment `@agent-shin reconsider` on the closed PR. If " + "Greptile re-scores you at 4/5 or higher I'll reopen and tag " + "the PR `ready for review`. (`@greptileai` works on closed PRs " + "too — a fresh review is one of the signals that lifts you back " + "into the queue.) This is **not** us losing interest in your " + "change — far from it. We just need open PRs to be a list of " + "things a maintainer can act on, so we can get to yours faster." + ) + return ( + "**If the bot closes this issue after the rollout:** edit the issue " + "description to add the missing pieces and reopen it (GitHub lets " + "external authors reopen their own issues). The bot will re-evaluate " + "and, if the rubric is met, leave it open. This is **not** us saying " + "the bug isn't real or the request isn't useful — it's so the " + "remaining open issues are a list of things a maintainer can act on." + ) + + +def format_heads_up_comment( + *, kind: str, verdict: dict, greptile_score: int | None, cutoff: dt.date +) -> str: + """Compose the friendly 7-day heads-up comment posted on a failing PR/issue.""" + noun = "PR" if kind == "pr" else "issue" + rubric = _rubric_section_pr() if kind == "pr" else _rubric_section_issue() + explanation = (verdict.get("explanation") or "").strip() + explanation_block = ( + f"> _(The judge's note for this one: {explanation})_\n\n" if explanation else "" + ) + + return ( + "👋 **Heads-up: new OSS triage bot landing on " + f"{cutoff.strftime('%A, %B %-d, %Y')}.**\n" + "\n" + "We're rolling out **Agent Shin**, an LLM-as-judge triage bot for " + f"external {noun}s. After the rollout, the bot will read each open " + f"{noun}'s description, score it against a small rubric, and " + f"auto-close any {noun} that's missing the basics — with a single " + f"comment explaining what's missing and how to recover. Full " + f"context: [Agent Shin rollout blog post]({ROLLOUT_BLOG_URL}).\n" + "\n" + f"{rubric}\n" + "\n" + f"{_description_only_note(kind)}\n" + "\n" + f"{_missing_section(verdict, greptile_score)}\n" + "\n" + f"{explanation_block}" + "**Timeline (you have a week):**\n" + "\n" + f"- You have **until {cutoff.strftime('%A, %B %-d')}** " + f"({DEFAULT_GRACE_DAYS} days from this comment) to update the " + f"{noun} description with the missing pieces. Nothing happens to " + f"this {noun} during that window.\n" + f"- After the rollout, the bot runs daily. From then on, any new " + f"{noun} that fails the rubric gets a **24-hour grace period** " + "(one warning comment, then auto-close).\n" + "\n" + f"{_recovery_section(kind)}\n" + "\n" + f"{HEADS_UP_MARKER}" + ) + + +def _list_open_numbers(repo: str, kind: str) -> list[int]: + """Return all open PR or issue numbers in ``repo`` (paginated via gh).""" + # `gh issue list` includes PRs unless we filter; use the dedicated commands. + cmd = "pr" if kind == "pr" else "issue" + raw = gh( + cmd, + "list", + "--repo", + repo, + "--state", + "open", + "--limit", + "1000", + "--json", + "number", + ) + return [item["number"] for item in json.loads(raw)] + + +def _has_heads_up_marker(item: dict) -> bool: + """Cheap fast-path: check the PR/issue body itself for the marker. + + The marker is appended to the *comment* we post, not the body, so this + will only fire if the body literally contains the marker text. We still + do the comment-marker check separately below; this body check just lets + us short-circuit for PRs/issues that quote the marker for any reason. + """ + body = item.get("body") or "" + return HEADS_UP_MARKER in body + + +def _comments_have_marker(repo: str, kind: str, number: int) -> bool: + """True if any existing issue-style comment already carries the marker. + + Used for idempotency: a re-run skips items the previous run notified. + The marker is unique to this rollout and never appears in steady-state + Agent Shin comments, so a substring match is sufficient. + """ + raw = gh( + "api", + "--paginate", + f"repos/{repo}/issues/{number}/comments?per_page=100", + ) + for line in raw.splitlines(): + line = line.strip() + if not line: + continue + try: + payload = json.loads(line) + except json.JSONDecodeError: + continue + comments = payload if isinstance(payload, list) else [payload] + for comment in comments: + if HEADS_UP_MARKER in (comment.get("body") or ""): + return True + return False + + +def _evaluate_pr(*, repo: str, number: int, model: str, judge: Any = None) -> dict: + """Run the future PR rubric (review_gate) in dry-run and return the result.""" + return review_gate( + repo=repo, + number=number, + close=False, # we only want the verdict, never act here + model=model, + judge=judge, + ) + + +def _evaluate_issue(*, repo: str, number: int, model: str, judge: Any = None) -> dict: + """Run the future issue rubric (triage kind='issue') in dry-run.""" + return triage( + repo=repo, + kind="issue", + number=number, + close=False, + model=model, + judge=judge, + ) + + +def _would_be_closed(kind: str, result: dict) -> bool: + """True if the future triage would auto-close this PR/issue based on the + rubric (regardless of grace-period gating). + + For PRs we trust ``review_gate``'s ``passing`` field — it combines the LLM + verdict and the Greptile score. For issues we read the LLM verdict + directly. Both fields are ``None``/missing on skip paths + (skip-internal-author, skip-llm-error, etc.) where the future bot would + NOT close the item — those return False. + """ + if kind == "pr": + passing = result.get("passing") + if passing is None: + return False # skipped — nothing for the heads-up to warn about + return passing is False + verdict = result.get("verdict") or {} + return (verdict.get("verdict") or "").lower() == "fail" + + +def _process_one( + *, + repo: str, + kind: str, + number: int, + model: str, + cutoff: dt.date, + dry_run: bool, + judge: Any = None, + skip_marker_check: bool = False, +) -> dict: + """Evaluate one PR/issue and post a heads-up if it would be auto-closed. + + Returns a per-item dict for the summary table. + """ + base = {"kind": kind, "number": number} + fetcher = fetch_pr if kind == "pr" else fetch_issue + item = fetcher(repo, number) + + if (item.get("state") or "") != "open": + return {**base, "action": "skip-not-open"} + if is_internal_contributor(item): + return {**base, "action": "skip-internal-author"} + if not skip_marker_check and _has_heads_up_marker(item): + return {**base, "action": "skip-already-marked-in-body"} + if not skip_marker_check and _comments_have_marker(repo, kind, number): + return {**base, "action": "skip-already-notified"} + + if kind == "pr": + result = _evaluate_pr(repo=repo, number=number, model=model, judge=judge) + else: + result = _evaluate_issue(repo=repo, number=number, model=model, judge=judge) + + if not _would_be_closed(kind, result): + return {**base, "action": "skip-passing", "evaluator": result.get("action")} + + verdict = result.get("verdict") or {} + greptile_score = result.get("greptile_score") if kind == "pr" else None + comment = format_heads_up_comment( + kind=kind, verdict=verdict, greptile_score=greptile_score, cutoff=cutoff + ) + maybe_post_comment(repo, number, comment, dry_run=dry_run) + return { + **base, + "action": "heads-up-posted" if not dry_run else "would-post-heads-up", + "verdict": (verdict.get("verdict") or "").lower(), + "greptile_score": greptile_score, + } + + +def _print_summary(results: list[dict]) -> None: + """Tally per-action counts so a dry-run preview tells you at a glance how + many comments the real run would post.""" + counts: dict[str, int] = {} + for r in results: + counts[r["action"]] = counts.get(r["action"], 0) + 1 + print("\n=== rollout heads-up summary ===") + for action in sorted(counts): + print(f" {action:35s} {counts[action]}") + print(f" total {len(results)}") + + +def run( + *, + repo: str, + close: bool, + cutoff: dt.date, + model: str, + kinds: tuple[str, ...] = ("pr", "issue"), + judge: Any = None, + only_numbers: dict[str, list[int]] | None = None, + skip_marker_check: bool = False, +) -> list[dict]: + """Sweep ``repo`` and post heads-up comments. Returns the per-item results.""" + dry_run = not close + if dry_run: + print( + f"[DRY RUN] sweeping {repo}; --close not passed, no comments will be posted." + ) + else: + print(f"[REAL RUN] sweeping {repo}; comments WILL be posted.") + print(f"Cutoff date in comment body: {cutoff.isoformat()}") + + results: list[dict] = [] + for kind in kinds: + if only_numbers and kind in only_numbers: + numbers = list(only_numbers[kind]) + else: + numbers = _list_open_numbers(repo, kind) + print(f"\n--- {kind}s: {len(numbers)} open ---") + for n in numbers: + try: + result = _process_one( + repo=repo, + kind=kind, + number=n, + model=model, + cutoff=cutoff, + dry_run=dry_run, + judge=judge, + skip_marker_check=skip_marker_check, + ) + except ( + Exception + ) as exc: # noqa: BLE001 - per-item errors don't abort the sweep + result = { + "kind": kind, + "number": n, + "action": "error", + "error": str(exc), + } + print(f"!! {kind}#{n}: {exc}", file=sys.stderr) + print(f" {kind}#{n}: {result['action']}") + results.append(result) + _print_summary(results) + return results + + +def main() -> int: + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("--repo", required=True, help="owner/repo") + parser.add_argument( + "--close", + action="store_true", + help=( + "Actually post comments. Without this flag the script is in " + "dry-run mode and only logs what it would do." + ), + ) + parser.add_argument( + "--close-on", + type=dt.date.fromisoformat, + default=None, + help=( + "Cutoff date shown in the heads-up comment as the rollout date " + f"(default: today + {DEFAULT_GRACE_DAYS} days)." + ), + ) + parser.add_argument( + "--model", + default=os.environ.get("TRIAGE_MODEL") or DEFAULT_MODEL, + help=f"Model for the rubric LLM judge (default: {DEFAULT_MODEL}).", + ) + parser.add_argument( + "--kind", + choices=("pr", "issue", "both"), + default="both", + help="Restrict the sweep to PRs or issues only (default: both).", + ) + parser.add_argument( + "--only-pr", + type=int, + action="append", + default=[], + help="Limit the PR sweep to these PR numbers (repeat for several).", + ) + parser.add_argument( + "--only-issue", + type=int, + action="append", + default=[], + help="Limit the issue sweep to these issue numbers (repeat for several).", + ) + parser.add_argument( + "--ignore-existing-marker", + action="store_true", + help=( + "Re-post on PRs/issues that already carry the heads-up marker. " + "Useful for testing the comment wording on a known PR." + ), + ) + args = parser.parse_args() + + cutoff = args.close_on or ( + dt.datetime.now(dt.timezone.utc).date() + dt.timedelta(days=DEFAULT_GRACE_DAYS) + ) + + kinds: tuple[str, ...] + if args.kind == "pr": + kinds = ("pr",) + elif args.kind == "issue": + kinds = ("issue",) + else: + kinds = ("pr", "issue") + + only: dict[str, list[int]] = {} + if args.only_pr: + only["pr"] = args.only_pr + if args.only_issue: + only["issue"] = args.only_issue + + # The script must NOT hit the LLM in dry-run if no key is set — we still + # want a useful preview that says "skip-no-llm-key" for items that would + # have been judged. Production runs require OPENAI_API_KEY. + if args.close and not os.environ.get("OPENAI_API_KEY"): + parser.error("OPENAI_API_KEY must be set for --close (real-run) mode.") + + run( + repo=args.repo, + close=args.close, + cutoff=cutoff, + model=args.model, + kinds=kinds, + only_numbers=only or None, + skip_marker_check=args.ignore_existing_marker, + ) + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/.github/workflows/triage_rollout_heads_up.yml b/.github/workflows/triage_rollout_heads_up.yml new file mode 100644 index 00000000000..82a404b49c5 --- /dev/null +++ b/.github/workflows/triage_rollout_heads_up.yml @@ -0,0 +1,85 @@ +name: Agent Shin — rollout heads-up (one-shot) + +# Fires the 7-day heads-up comment on every open external PR/issue that the +# new triage bot would auto-close. Designed to run **once**, on the merge +# commit of the rollout PR — but the script is idempotent (skips items that +# already carry the `` marker), so an +# accidental re-run is harmless. +# +# The workflow is a thin shell over `.github/scripts/triage_rollout_heads_up.py`. +# Dry-run vs. real run differ in EXACTLY one CLI flag (`--close`) — the +# auto-trigger uses `--close`, the manual `workflow_dispatch` trigger defaults +# to dry-run so you can preview before the real fire. + +on: + push: + branches: + - litellm_internal_staging + paths: + # The presence of this script on staging IS the rollout merge marker. + # Editing the file later would re-fire the workflow; that's safe because + # the script skips PRs/issues that already have the heads-up marker. + - ".github/scripts/triage_rollout_heads_up.py" + workflow_dispatch: + inputs: + dry_run: + description: "Dry run (true = preview only, false = actually post comments)." + required: false + default: "true" + type: choice + options: + - "true" + - "false" + +permissions: + contents: read + issues: write + pull-requests: write + +jobs: + heads-up: + if: github.repository == 'BerriAI/litellm' + runs-on: ubuntu-latest + steps: + - name: Checkout triage scripts + 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 heads-up sweep + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + OPENAI_API_KEY: ${{ secrets.OPENAI_API_KEY }} + OPENAI_BASE_URL: ${{ vars.OPENAI_BASE_URL }} + TRIAGE_MODEL: ${{ vars.TRIAGE_MODEL }} + # Auto-trigger from push -> always real (`--close`). For + # workflow_dispatch we honor the dry_run input. Use the EXACT + # string "true"/"false" comparison so unexpected input values + # fail-closed to dry-run (mirrors the AGENT_SHIN_ENABLED pattern + # in the sibling workflows). + DRY_RUN_INPUT: ${{ github.event.inputs.dry_run }} + run: | + set -euo pipefail + ARGS=(--repo "${{ github.repository }}") + if [ "${GITHUB_EVENT_NAME:-}" = "workflow_dispatch" ]; then + if [ "${DRY_RUN_INPUT:-true}" = "false" ]; then + ARGS+=(--close) + echo "::notice::Manual dispatch with dry_run=false -> comments WILL be posted." + else + echo "::notice::Manual dispatch in dry-run mode -> no comments will be posted." + fi + else + # push to litellm_internal_staging = the rollout merge -> real run + ARGS+=(--close) + echo "::notice::Rollout-merge trigger -> heads-up comments WILL be posted." + fi + python3 .github/scripts/triage_rollout_heads_up.py "${ARGS[@]}" diff --git a/tests/test_litellm/test_triage_rollout_heads_up.py b/tests/test_litellm/test_triage_rollout_heads_up.py new file mode 100644 index 00000000000..469d05fee03 --- /dev/null +++ b/tests/test_litellm/test_triage_rollout_heads_up.py @@ -0,0 +1,628 @@ +"""Unit tests for the one-shot 7-day heads-up sweep. + +Exercises: + + * The ``_agent_shin_actions`` dry-run wrappers — each ``maybe_*`` helper + must call the real underlying mutation iff ``dry_run=False``, and log to + stdout otherwise. + * ``triage_rollout_heads_up._would_be_closed`` — the predicate that + decides "would the future bot close this?" for both PRs and issues. + * ``triage_rollout_heads_up._process_one`` — the per-item processor: + skip when state != open, skip internal authors, skip already-notified + items, post heads-up on failing items, leave passing items alone. + * ``triage_rollout_heads_up.run`` — the sweep loop end-to-end, in both + dry-run and real modes, with the comment-posting injected so we never + talk to GitHub. + +Every test stubs out ``gh()`` and the GitHub mutations; nothing in this file +ever shells out. +""" + +from __future__ import annotations + +import datetime as dt +import importlib.util +import sys +from pathlib import Path + +import pytest + +_SCRIPTS_DIR = Path(__file__).resolve().parents[2] / ".github" / "scripts" + + +@pytest.fixture(scope="module") +def triage_module(): + """Load triage_with_llm under its canonical name so the sibling modules + can `from triage_with_llm import ...`.""" + spec = importlib.util.spec_from_file_location( + "triage_with_llm", _SCRIPTS_DIR / "triage_with_llm.py" + ) + 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 + + +@pytest.fixture(scope="module") +def actions_module(triage_module): + spec = importlib.util.spec_from_file_location( + "_agent_shin_actions", _SCRIPTS_DIR / "_agent_shin_actions.py" + ) + assert spec and spec.loader + module = importlib.util.module_from_spec(spec) + sys.modules["_agent_shin_actions"] = module + spec.loader.exec_module(module) + return module + + +@pytest.fixture(scope="module") +def heads_up_module(triage_module, actions_module): + spec = importlib.util.spec_from_file_location( + "triage_rollout_heads_up", _SCRIPTS_DIR / "triage_rollout_heads_up.py" + ) + assert spec and spec.loader + module = importlib.util.module_from_spec(spec) + sys.modules["triage_rollout_heads_up"] = module + spec.loader.exec_module(module) + return module + + +# --------------------------------------------------------------------------- +# _agent_shin_actions: the dry-run wrappers + + +class TestActionsDryRun: + """Each maybe_* helper must NOT hit GitHub in dry-run, and MUST hit it + in real mode. The whole rollout's safety story rests on this.""" + + def test_maybe_post_comment_dry_run_logs_only( + self, actions_module, triage_module, monkeypatch, capsys + ): + called = [] + monkeypatch.setattr( + triage_module, + "post_comment", + lambda *a, **k: called.append((a, k)), + ) + actions_module.maybe_post_comment("o/r", 7, "hello", dry_run=True) + assert called == [] + assert "[DRY RUN] comment o/r#7" in capsys.readouterr().out + + def test_maybe_post_comment_real_run_calls_through( + self, actions_module, triage_module, monkeypatch + ): + called = [] + monkeypatch.setattr( + triage_module, + "post_comment", + lambda repo, n, body: called.append((repo, n, body)), + ) + actions_module.maybe_post_comment("o/r", 7, "hello", dry_run=False) + assert called == [("o/r", 7, "hello")] + + @pytest.mark.parametrize( + "fn,target_name,extra_args", + [ + ("maybe_close_pr", "close_pr", ()), + ("maybe_reopen_pr", "reopen_pr", ()), + ("maybe_reopen_issue", "reopen_issue", ()), + ], + ) + def test_simple_mutators_gate_on_dry_run( + self, actions_module, triage_module, monkeypatch, fn, target_name, extra_args + ): + called = [] + monkeypatch.setattr( + triage_module, target_name, lambda *a, **k: called.append((a, k)) + ) + getattr(actions_module, fn)("o/r", 7, *extra_args, dry_run=True) + assert called == [] + getattr(actions_module, fn)("o/r", 7, *extra_args, dry_run=False) + assert len(called) == 1 + + def test_maybe_close_issue_passes_not_planned( + self, actions_module, triage_module, monkeypatch + ): + called = [] + monkeypatch.setattr( + triage_module, + "close_issue", + lambda repo, n, *, not_planned=True: called.append(not_planned), + ) + actions_module.maybe_close_issue("o/r", 7, dry_run=False) + actions_module.maybe_close_issue("o/r", 7, dry_run=False, not_planned=False) + assert called == [True, False] + + @pytest.mark.parametrize( + "fn,target", + [ + ("maybe_add_label", "add_label"), + ("maybe_remove_label", "remove_label"), + ], + ) + def test_label_mutators_gate_on_dry_run( + self, actions_module, triage_module, monkeypatch, fn, target, capsys + ): + called = [] + monkeypatch.setattr(triage_module, target, lambda *a, **k: called.append(a)) + getattr(actions_module, fn)("o/r", 7, "ready for review", dry_run=True) + assert called == [] + assert "ready for review" in capsys.readouterr().out + getattr(actions_module, fn)("o/r", 7, "ready for review", dry_run=False) + assert called == [("o/r", 7, "ready for review")] + + +# --------------------------------------------------------------------------- +# _would_be_closed predicate + + +class TestWouldBeClosed: + def test_pr_passing_returns_false(self, heads_up_module): + assert ( + heads_up_module._would_be_closed( + "pr", {"passing": True, "action": "noop-passing"} + ) + is False + ) + + def test_pr_failing_returns_true(self, heads_up_module): + assert ( + heads_up_module._would_be_closed( + "pr", + { + "passing": False, + "action": "would-close", + "verdict": {"verdict": "fail"}, + }, + ) + is True + ) + + def test_pr_skipped_returns_false(self, heads_up_module): + # passing is None for skip paths (internal-author, llm-error, etc.) + assert ( + heads_up_module._would_be_closed("pr", {"action": "skip-internal-author"}) + is False + ) + + def test_issue_pass_returns_false(self, heads_up_module): + assert ( + heads_up_module._would_be_closed( + "issue", {"action": "pass-llm", "verdict": {"verdict": "pass"}} + ) + is False + ) + + def test_issue_fail_returns_true(self, heads_up_module): + assert ( + heads_up_module._would_be_closed( + "issue", {"action": "would-close", "verdict": {"verdict": "fail"}} + ) + is True + ) + + def test_issue_missing_verdict_returns_false(self, heads_up_module): + # Skip paths don't surface a verdict; treat as "won't close". + assert ( + heads_up_module._would_be_closed("issue", {"action": "skip-not-open"}) + is False + ) + + +# --------------------------------------------------------------------------- +# Comment formatter — wording sanity checks + + +class TestHeadsUpCommentBody: + def test_pr_comment_contains_cutoff_rubric_marker(self, heads_up_module): + body = heads_up_module.format_heads_up_comment( + kind="pr", + verdict={"verdict": "fail", "missing": ["QA proof"], "explanation": "thin"}, + greptile_score=3, + cutoff=dt.date(2026, 6, 1), + ) + assert "Monday, June 1, 2026" in body # cutoff readable + 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 + assert heads_up_module.HEADS_UP_MARKER in body # idempotency marker + + def test_issue_comment_uses_reopen_recovery_path(self, heads_up_module): + body = heads_up_module.format_heads_up_comment( + kind="issue", + verdict={"verdict": "fail", "missing": ["repro"], "explanation": ""}, + greptile_score=None, + cutoff=dt.date(2026, 6, 1), + ) + assert "reopen" in body.lower() # issues recover by reopening + assert "@agent-shin reconsider" not in body # PR-only recovery path + assert heads_up_module.HEADS_UP_MARKER in body + + def test_empty_missing_uses_fallback_copy(self, heads_up_module): + body = heads_up_module.format_heads_up_comment( + kind="pr", + verdict={"verdict": "fail", "missing": [], "explanation": ""}, + greptile_score=None, + cutoff=dt.date(2026, 6, 1), + ) + assert "couldn't articulate" in body + # Make sure the fallback didn't leave us with a broken sentence. + assert "specific missing piece" in body + + +# --------------------------------------------------------------------------- +# _process_one — per-item dispatch + + +def _stub_fetchers(heads_up_module, triage_module, *, item): + """Monkeypatch fetch_pr and fetch_issue (both in triage_with_llm and the + re-imported names in heads_up_module) to return `item`.""" + return [ + (triage_module, "fetch_pr", lambda repo, n: item), + (triage_module, "fetch_issue", lambda repo, n: item), + (heads_up_module, "fetch_pr", lambda repo, n: item), + (heads_up_module, "fetch_issue", lambda repo, n: item), + ] + + +class TestProcessOne: + """Per-item processing: the right skip reason fires for each scenario, + and the heads-up only goes out when the rubric is genuinely failing.""" + + @pytest.fixture + def patch_env(self, heads_up_module, triage_module, monkeypatch): + """Helper that returns a callable to install a PR/issue body, suppress + marker checks, and stub the comment poster.""" + posts = [] + monkeypatch.setattr( + heads_up_module, + "maybe_post_comment", + lambda repo, n, body, *, dry_run: posts.append((repo, n, body, dry_run)), + ) + monkeypatch.setattr(heads_up_module, "_has_heads_up_marker", lambda item: False) + monkeypatch.setattr( + heads_up_module, "_comments_have_marker", lambda repo, kind, n: False + ) + + def _install(item): + for mod, name, fn in _stub_fetchers( + heads_up_module, triage_module, item=item + ): + monkeypatch.setattr(mod, name, fn) + + return _install, posts + + def test_skip_closed_pr(self, heads_up_module, patch_env): + install, posts = patch_env + install( + {"state": "closed", "user": {"login": "ext"}, "author_association": "NONE"} + ) + r = heads_up_module._process_one( + repo="o/r", + kind="pr", + number=7, + model="m", + cutoff=dt.date(2026, 6, 1), + dry_run=True, + ) + assert r["action"] == "skip-not-open" + assert posts == [] + + def test_skip_internal_pr(self, heads_up_module, patch_env): + install, posts = patch_env + install( + { + "state": "open", + "user": {"login": "mateo-berri"}, + "author_association": "MEMBER", + "body": "", + "labels": [], + "created_at": "2026-05-25T00:00:00Z", + } + ) + r = heads_up_module._process_one( + repo="o/r", + kind="pr", + number=7, + model="m", + cutoff=dt.date(2026, 6, 1), + dry_run=True, + ) + assert r["action"] == "skip-internal-author" + assert posts == [] + + def test_skip_passing_pr(self, heads_up_module, patch_env, monkeypatch): + install, posts = patch_env + install( + { + "state": "open", + "user": {"login": "outside-dev"}, + "author_association": "NONE", + "body": "Fixes #123 — clean fix with a passing rubric.", + "labels": [], + "created_at": "2026-05-25T00:00:00Z", + } + ) + monkeypatch.setattr( + heads_up_module, + "_evaluate_pr", + lambda **kwargs: { + "action": "noop-passing", + "passing": True, + "verdict": {"verdict": "pass"}, + "greptile_score": 5, + }, + ) + r = heads_up_module._process_one( + repo="o/r", + kind="pr", + number=7, + model="m", + cutoff=dt.date(2026, 6, 1), + dry_run=True, + ) + assert r["action"] == "skip-passing" + assert posts == [] + + def test_failing_pr_posts_heads_up_dry_run( + self, heads_up_module, patch_env, monkeypatch, capsys + ): + install, posts = patch_env + install( + { + "state": "open", + "user": {"login": "outside-dev"}, + "author_association": "NONE", + "body": "thin", + "labels": [], + "created_at": "2026-05-25T00:00:00Z", + } + ) + monkeypatch.setattr( + heads_up_module, + "_evaluate_pr", + lambda **kwargs: { + "action": "would-close", + "passing": False, + "verdict": { + "verdict": "fail", + "missing": ["QA proof"], + "explanation": "PR body is one line.", + }, + "greptile_score": 3, + }, + ) + r = heads_up_module._process_one( + repo="o/r", + kind="pr", + number=7, + model="m", + cutoff=dt.date(2026, 6, 1), + dry_run=True, + ) + assert r["action"] == "would-post-heads-up" + assert posts == [("o/r", 7, posts[0][2], True)] # tuple shape preserved + assert "QA proof" in posts[0][2] + assert heads_up_module.HEADS_UP_MARKER in posts[0][2] + + def test_failing_issue_posts_heads_up_real_run( + self, heads_up_module, patch_env, monkeypatch + ): + install, posts = patch_env + install( + { + "state": "open", + "user": {"login": "outside-dev"}, + "author_association": "NONE", + "body": "X is broken", + "labels": [], + "created_at": "2026-05-25T00:00:00Z", + } + ) + monkeypatch.setattr( + heads_up_module, + "_evaluate_issue", + lambda **kwargs: { + "action": "would-close", + "verdict": { + "verdict": "fail", + "missing": ["reproduction"], + "explanation": "too thin", + }, + }, + ) + r = heads_up_module._process_one( + repo="o/r", + kind="issue", + number=42, + model="m", + cutoff=dt.date(2026, 6, 1), + dry_run=False, + ) + assert r["action"] == "heads-up-posted" + assert len(posts) == 1 + _, n, _, dry = posts[0] + assert n == 42 and dry is False + + def test_already_notified_is_skipped(self, heads_up_module, patch_env, monkeypatch): + install, posts = patch_env + install( + { + "state": "open", + "user": {"login": "outside-dev"}, + "author_association": "NONE", + "body": "thin", + "labels": [], + "created_at": "2026-05-25T00:00:00Z", + } + ) + # Override the marker check for this scenario only. + monkeypatch.setattr( + heads_up_module, "_comments_have_marker", lambda repo, kind, n: True + ) + r = heads_up_module._process_one( + repo="o/r", + kind="pr", + number=7, + model="m", + cutoff=dt.date(2026, 6, 1), + dry_run=True, + ) + assert r["action"] == "skip-already-notified" + assert posts == [] + + def test_ignore_existing_marker_forces_post( + self, heads_up_module, patch_env, monkeypatch + ): + install, posts = patch_env + install( + { + "state": "open", + "user": {"login": "outside-dev"}, + "author_association": "NONE", + "body": "thin", + "labels": [], + "created_at": "2026-05-25T00:00:00Z", + } + ) + monkeypatch.setattr( + heads_up_module, "_comments_have_marker", lambda repo, kind, n: True + ) + monkeypatch.setattr( + heads_up_module, + "_evaluate_pr", + lambda **kwargs: { + "action": "would-close", + "passing": False, + "verdict": {"verdict": "fail", "missing": ["X"], "explanation": ""}, + "greptile_score": None, + }, + ) + r = heads_up_module._process_one( + repo="o/r", + kind="pr", + number=7, + model="m", + cutoff=dt.date(2026, 6, 1), + dry_run=True, + skip_marker_check=True, + ) + assert r["action"] == "would-post-heads-up" + + +# --------------------------------------------------------------------------- +# run() — sweep loop + + +class TestRun: + """End-to-end the sweep loop with a tiny fake repo: 1 passing PR, 1 + failing PR, 1 passing issue, 1 failing issue.""" + + @pytest.fixture + def configured(self, heads_up_module, triage_module, monkeypatch): + posts = [] + monkeypatch.setattr( + heads_up_module, + "maybe_post_comment", + lambda repo, n, body, *, dry_run: posts.append((n, dry_run, body)), + ) + monkeypatch.setattr(heads_up_module, "_has_heads_up_marker", lambda item: False) + monkeypatch.setattr( + heads_up_module, "_comments_have_marker", lambda repo, kind, n: False + ) + + def fake_list(repo, kind): + return [1, 2] if kind == "pr" else [101, 102] + + monkeypatch.setattr(heads_up_module, "_list_open_numbers", fake_list) + + def make_item(login="outside-dev"): + return { + "state": "open", + "user": {"login": login}, + "author_association": "NONE", + "body": "thin", + "labels": [], + "created_at": "2026-05-25T00:00:00Z", + } + + monkeypatch.setattr(heads_up_module, "fetch_pr", lambda repo, n: make_item()) + monkeypatch.setattr(heads_up_module, "fetch_issue", lambda repo, n: make_item()) + monkeypatch.setattr(triage_module, "fetch_pr", lambda repo, n: make_item()) + monkeypatch.setattr(triage_module, "fetch_issue", lambda repo, n: make_item()) + + def pr_eval(*, number, **kwargs): + if number == 1: + return { + "action": "noop-passing", + "passing": True, + "verdict": {"verdict": "pass"}, + } + return { + "action": "would-close", + "passing": False, + "verdict": {"verdict": "fail", "missing": ["m"], "explanation": ""}, + "greptile_score": 2, + } + + def issue_eval(*, number, **kwargs): + if number == 101: + return {"action": "pass-llm", "verdict": {"verdict": "pass"}} + return { + "action": "would-close", + "verdict": {"verdict": "fail", "missing": ["repro"], "explanation": ""}, + } + + monkeypatch.setattr(heads_up_module, "_evaluate_pr", pr_eval) + monkeypatch.setattr(heads_up_module, "_evaluate_issue", issue_eval) + return posts + + def test_dry_run_posts_nothing_but_logs_both_would_posts( + self, heads_up_module, configured, capsys + ): + results = heads_up_module.run( + repo="o/r", + close=False, + cutoff=dt.date(2026, 6, 1), + model="m", + ) + actions = [r["action"] for r in results] + assert actions.count("would-post-heads-up") == 2 + assert actions.count("skip-passing") == 2 + assert all(dry for _, dry, _ in configured) # every post was dry-run + + def test_real_run_posts_two_comments(self, heads_up_module, configured): + results = heads_up_module.run( + repo="o/r", + close=True, + cutoff=dt.date(2026, 6, 1), + model="m", + ) + assert sum(1 for r in results if r["action"] == "heads-up-posted") == 2 + # Two real-run posts: one failing PR (#2), one failing issue (#102). + real_posts = [n for n, dry, _ in configured if dry is False] + assert sorted(real_posts) == [2, 102] + + def test_kinds_filter_skips_issues(self, heads_up_module, configured): + results = heads_up_module.run( + repo="o/r", + close=False, + cutoff=dt.date(2026, 6, 1), + model="m", + kinds=("pr",), + ) + assert {r["kind"] for r in results} == {"pr"} + + def test_only_numbers_restricts_sweep(self, heads_up_module, configured): + results = heads_up_module.run( + repo="o/r", + close=False, + cutoff=dt.date(2026, 6, 1), + model="m", + only_numbers={"pr": [2], "issue": [101]}, + ) + assert sorted((r["kind"], r["number"]) for r in results) == [ + ("issue", 101), + ("pr", 2), + ]