From 5b1a9563d60670e7044497e4be483dfdf2a2fc8b Mon Sep 17 00:00:00 2001 From: yuneng-jiang Date: Thu, 20 Aug 2026 10:07:14 -0700 Subject: [PATCH] chore(ci): close the test-census blind spots and move scripts out of workflows/ (#37586) The agent job's CircleCI glob collected `tests/agent_tests/**/test_*.py` and then piped it through `grep -v` to drop `local_only_agent_tests/`. `assert_ci_coverage.py` reads the glob but not the pipeline, so those two files looked covered and were invisible to the census. The glob now excludes them structurally and they carry an allowlist entry instead, which is a decision on the record rather than a hidden filter. The collected file set is unchanged: `tests/agent_tests/` holds exactly one CI-runnable test at the top level. `tests/scim_tests/` held a single JSON fixture and no tests, referenced from nowhere. `.github/workflows/` is for workflows. Both stray scripts move to `.github/scripts/` with their callers updated: the price-file updater is invoked by `auto_update_price_and_context_window.yml`, and the translation-report runner by `make test-llm-translation`. The audit listed the latter as orphaned, but Makefile line 317 still runs it, so it moves rather than being deleted. The rollout heads-up workflow was a deliberate one-shot for the agent-shin rollout. That rollout is done, the triage and auto-close workflows have been running daily since June, so the pre-flip warning window is long past. Its script and dedicated test go with it, and the sibling workflow-invariant test drops its entry. --- .circleci/config.yml | 2 +- .github/ci-coverage-allowlist.yml | 8 + ...to_update_price_and_context_window_file.py | 0 .../run_llm_translation_tests.py | 0 .github/scripts/triage_rollout_heads_up.py | 557 ------------- .../auto_update_price_and_context_window.yml | 2 +- .github/workflows/triage_rollout_heads_up.yml | 92 --- Makefile | 2 +- tests/scim_tests/scim_e2e_test.json | 750 ------------------ .../test_github_triage_workflows.py | 44 - .../test_triage_rollout_heads_up.py | 612 -------------- 11 files changed, 11 insertions(+), 2058 deletions(-) rename .github/{workflows => scripts}/auto_update_price_and_context_window_file.py (100%) rename .github/{workflows => scripts}/run_llm_translation_tests.py (100%) delete mode 100644 .github/scripts/triage_rollout_heads_up.py delete mode 100644 .github/workflows/triage_rollout_heads_up.yml delete mode 100644 tests/scim_tests/scim_e2e_test.json delete mode 100644 tests/test_litellm/test_triage_rollout_heads_up.py diff --git a/.circleci/config.yml b/.circleci/config.yml index e8a8483781b..5e77729df29 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -1025,7 +1025,7 @@ jobs: name: Run tests command: | mkdir -p test-results - TEST_FILES=$(circleci tests glob "tests/agent_tests/**/test_*.py" | grep -v "^tests/agent_tests/local_only_agent_tests/") + TEST_FILES=$(circleci tests glob "tests/agent_tests/test_*.py") echo "$TEST_FILES" | circleci tests run \ --verbose \ --command="tr ' ' '\\n' | awk '/\\.py/ {print; next} {sub(/\\.[A-Z][^.]*$/, \"\"); gsub(/\\./, \"/\"); print \$0 \".py\"}' | xargs uv run --no-sync python -m pytest \ diff --git a/.github/ci-coverage-allowlist.yml b/.github/ci-coverage-allowlist.yml index 1423228e725..ccc21512e79 100644 --- a/.github/ci-coverage-allowlist.yml +++ b/.github/ci-coverage-allowlist.yml @@ -116,6 +116,14 @@ test_paths: - tests/load_tests/test_otel_load_test.py - tests/load_tests/test_vertex_embeddings_load_test.py - tests/load_tests/test_vertex_load_tests.py + - reason: >- + A local-only agent rig: test_a2a_completion_bridge.py needs a LangGraph server on + localhost:2024 and test_a2a.py drives a live A2A endpoint, so neither can run in a + pull request job. Until 2026-08-20 the CircleCI agent job hid them behind a grep -v + that this census could not see; the glob now excludes them structurally and this entry + is the decision on the record. Revisit when the A2A bridge gets a recorded-wire fixture + paths: + - tests/agent_tests/local_only_agent_tests - reason: >- Third-party integration tests that skip themselves without OCI configuration or sandbox credentials, neither of which a pull request job holds diff --git a/.github/workflows/auto_update_price_and_context_window_file.py b/.github/scripts/auto_update_price_and_context_window_file.py similarity index 100% rename from .github/workflows/auto_update_price_and_context_window_file.py rename to .github/scripts/auto_update_price_and_context_window_file.py diff --git a/.github/workflows/run_llm_translation_tests.py b/.github/scripts/run_llm_translation_tests.py similarity index 100% rename from .github/workflows/run_llm_translation_tests.py rename to .github/scripts/run_llm_translation_tests.py diff --git a/.github/scripts/triage_rollout_heads_up.py b/.github/scripts/triage_rollout_heads_up.py deleted file mode 100644 index a5dedb1c9e7..00000000000 --- a/.github/scripts/triage_rollout_heads_up.py +++ /dev/null @@ -1,557 +0,0 @@ -#!/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** at rollout, fired by a manual -``workflow_dispatch`` (``dry_run=false``) on the heads-up workflow. 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. Passing ``--close`` flips 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 manual rollout dispatch 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 agent_shin_shared import ( # noqa: E402 - AGENT_SHIN_DEFAULT_BOT_LOGIN, - ALLOWLIST_LOGINS, - list_open_items, -) -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 - -# The daily auto-close sweeps (close_low_quality_prs.yml at 09:00 UTC and -# review_gate.yml at 09:30 UTC) are what actually close a still-failing item, -# so the deadline we promise contributors has to name that wall-clock moment. -ACTIVATION_TIME_UTC = "09:00 UTC" - - -def _format_cutoff(cutoff: dt.date) -> str: - """Human-readable, timezone-explicit cutoff, e.g. ``Monday, June 1, 2026 - (09:00 UTC)`` — the moment a still-failing PR/issue gets closed.""" - return ( - f"{cutoff.strftime('%A, %B')} {cutoff.day}, {cutoff.year} " - f"({ACTIVATION_TIME_UTC})" - ) - - -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 **end-to-end QA proof** (at least one of a " - "short screen recording / video, before/after screenshots, or the " - "exact commands you ran with their real output; mocked or stubbed " - "runs don't count).\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**: end-to-end evidence of the bug (at least one " - "of a screen recording / video, a screenshot, or the exact commands " - "you ran with their real output / traceback) plus expected vs. actual " - "behavior. Written steps with no run output don't count, and mocked " - "or stubbed runs don't count.\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, then comment `@agent-shin " - "reconsider` on the closed issue. I'll re-evaluate and, if the rubric " - "is met, reopen it. (GitHub doesn't let external authors reopen an " - "issue a maintainer or bot closed, so the comment is the reliable " - "path.) 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() - cutoff_str = _format_cutoff(cutoff) - 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: we're turning on the OSS triage bot in " - f"{DEFAULT_GRACE_DAYS} days, on {cutoff_str}.**\n" - "\n" - "We're rolling out **Agent Shin**, an LLM-as-judge triage bot for " - f"external {noun}s. Once it's live, the bot reads each open " - f"{noun}'s description, scores it against a small rubric, and " - f"auto-closes 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"- We turn the bot on in {DEFAULT_GRACE_DAYS} days, on " - f"**{cutoff_str}**. You have until then to update this {noun}'s " - "description with the missing pieces above.\n" - 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" - "\n" - f"{_recovery_section(kind)}\n" - "\n" - f"{HEADS_UP_MARKER}" - ) - - -def _list_open_numbers(repo: str, kind: str) -> list[int]: - """Return every open PR or issue number in ``repo``. - - Delegates to ``list_open_items`` so the full backlog is fetched (no cap) - and the `gh {pr,issue} list` invocation stays in one shared place. ``gh - issue list`` would include PRs, but ``list_open_items`` uses the dedicated - command per kind, so the two never mix. - """ - return [ - item["number"] for item in list_open_items(kind, repo=repo, fields="number") - ] - - -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, number: int) -> bool: - """True if the bot already posted a comment carrying the marker. - - Used for idempotency: a re-run skips items the previous run notified. - Filters by author (matching the sibling marker-checks in - ``triage_with_llm._has_marker`` and - ``agent_shin_shared.seconds_since_latest_marker_comment``) so a - contributor who quotes the heads-up via GitHub's "Quote reply" — which - preserves HTML comments in the raw markdown — can't trick the - idempotency check into silently skipping a real heads-up. - - Comments live on the unified issues endpoint regardless of whether the - item is a PR or an issue, so no ``kind`` argument is required here. - """ - expected_login = ( - os.environ.get("AGENT_SHIN_BOT_LOGIN") or AGENT_SHIN_DEFAULT_BOT_LOGIN - ).lower() - 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: - author = ((comment.get("user") or {}).get("login") or "").lower() - if author != expected_login: - continue - 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, - allowlist: frozenset[str] = ALLOWLIST_LOGINS, -) -> 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 allowlist: - login = (item.get("user") or {}).get("login") or "" - if login.lower() not in allowlist: - return {**base, "action": "skip-not-allowlisted"} - elif 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, 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/auto_update_price_and_context_window.yml b/.github/workflows/auto_update_price_and_context_window.yml index d391c0bd6ce..7e40a860ee9 100644 --- a/.github/workflows/auto_update_price_and_context_window.yml +++ b/.github/workflows/auto_update_price_and_context_window.yml @@ -23,7 +23,7 @@ jobs: version: "0.10.9" - name: Update JSON Data run: | - uv run --frozen --with 'aiohttp==3.13.3' python ".github/workflows/auto_update_price_and_context_window_file.py" + uv run --frozen --with 'aiohttp==3.13.3' python ".github/scripts/auto_update_price_and_context_window_file.py" - name: Regenerate JSON Schema run: | uv run --frozen python ci_cd/generate_model_prices_schema.py diff --git a/.github/workflows/triage_rollout_heads_up.yml b/.github/workflows/triage_rollout_heads_up.yml deleted file mode 100644 index 903960151e2..00000000000 --- a/.github/workflows/triage_rollout_heads_up.yml +++ /dev/null @@ -1,92 +0,0 @@ -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. The real sweep is a deliberate one-shot: -# trigger it at rollout via a manual `workflow_dispatch` with `dry_run=false`. -# The script is idempotent (skips items that already carry the -# `` marker), so a re-run is harmless. -# -# The automatic push trigger runs DRY-RUN only, so merging the script to -# `litellm_internal_staging` never posts a comment; it just confirms the -# workflow is wired up. Posting real comments requires the manual dispatch, -# which is also the only trigger that exposes `OPENAI_API_KEY`. The heads-up -# is intentionally NOT gated on `AGENT_SHIN_ENABLED`: it has to warn -# contributors while that flag is still off, ahead of the flip that turns on -# auto-closing. -# -# 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`), added only -# on a manual dispatch with `dry_run=false`. - -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 --require-hashes -r .github/scripts/triage-requirements.txt - - - name: Run heads-up sweep - env: - GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} - # Only the manual dispatch (the real-run trigger) needs the LLM key. - # The automatic push trigger runs dry-run and never posts, so it gets - # no key. Mirrors the sibling triage workflows, which expose the key - # only on an enabled/dispatched run rather than unconditionally. - OPENAI_API_KEY: ${{ github.event_name == 'workflow_dispatch' && secrets.OPENAI_API_KEY || '' }} - OPENAI_BASE_URL: ${{ vars.OPENAI_BASE_URL }} - TRIAGE_MODEL: ${{ vars.TRIAGE_MODEL }} - # The real run is a deliberate manual dispatch with dry_run=false. - # Use the EXACT "false" comparison so any unexpected input value - # fail-closes to dry-run (mirrors the AGENT_SHIN_ENABLED pattern in - # the sibling workflows). The automatic push trigger always stays - # dry-run, so merging the script never posts. - DRY_RUN_INPUT: ${{ github.event.inputs.dry_run }} - run: | - set -euo pipefail - ARGS=(--repo "${{ github.repository }}") - if [ "${GITHUB_EVENT_NAME:-}" = "workflow_dispatch" ] && [ "${DRY_RUN_INPUT:-true}" = "false" ]; then - ARGS+=(--close) - echo "::notice::Manual rollout dispatch with dry_run=false -> heads-up comments WILL be posted." - elif [ "${GITHUB_EVENT_NAME:-}" = "workflow_dispatch" ]; then - echo "::notice::Manual dispatch in dry-run mode -> previewing only, no comments will be posted." - else - echo "::notice::Automatic push trigger -> dry-run preview only. Fire the real rollout sweep with a manual workflow_dispatch (dry_run=false)." - fi - python3 .github/scripts/triage_rollout_heads_up.py "${ARGS[@]}" diff --git a/Makefile b/Makefile index 5e5f7c80027..6c125268678 100644 --- a/Makefile +++ b/Makefile @@ -314,7 +314,7 @@ test-unit-helm: install-helm-unittest # LLM Translation testing targets test-llm-translation: install-test-deps @echo "Running LLM translation tests..." - @python .github/workflows/run_llm_translation_tests.py + @python .github/scripts/run_llm_translation_tests.py test-llm-translation-single: install-test-deps @echo "Running single LLM translation test file..." diff --git a/tests/scim_tests/scim_e2e_test.json b/tests/scim_tests/scim_e2e_test.json deleted file mode 100644 index bc5810762da..00000000000 --- a/tests/scim_tests/scim_e2e_test.json +++ /dev/null @@ -1,750 +0,0 @@ -{ - "version": "1.0", - "exported_at": 1715608731, - "name": "Okta SCIM 2.0 SPEC Test", - "description": "Basic tests to see if your SCIM server will work with Okta", - "trigger_url": "https://api.runscope.com/radar/37d9f10e-e250-4071-9cec-1fa30e56b42b/trigger", - "steps": [ - { - "step_type": "request", - "skipped": false, - "note": "Required Test: Test Users endpoint", - "auth": {}, - "multipart_form": [], - "headers": { - "Accept-Charset": [ - "utf-8" - ], - "Content-Type": [ - "application/scim+json; charset=utf-8" - ], - "Accept": [ - "application/scim+json" - ], - "Authorization": [ - "{{auth}}" - ], - "User-Agent": [ - "OKTA SCIM Integration" - ] - }, - "method": "GET", - "url": "{{SCIMBaseURL}}/Users?count=1&startIndex=1", - "assertions": [ - { - "comparison": "equal_number", - "source": "response_status", - "value": "200" - }, - { - "comparison": "not_empty", - "source": "response_json", - "value": null, - "property": "Resources" - }, - { - "comparison": "has_value", - "source": "response_json", - "value": "urn:ietf:params:scim:api:messages:2.0:ListResponse", - "property": "schemas" - }, - { - "comparison": "is_a_number", - "source": "response_json", - "value": null, - "property": "itemsPerPage" - }, - { - "comparison": "is_a_number", - "source": "response_json", - "value": null, - "property": "startIndex" - }, - { - "comparison": "is_a_number", - "source": "response_json", - "value": null, - "property": "totalResults" - }, - { - "comparison": "not_empty", - "source": "response_json", - "value": null, - "property": "Resources[0].id" - }, - { - "comparison": "not_empty", - "source": "response_json", - "value": null, - "property": "Resources[0].name.familyName" - }, - { - "comparison": "not_empty", - "source": "response_json", - "value": null, - "property": "Resources[0].name.givenName" - }, - { - "comparison": "not_empty", - "source": "response_json", - "value": null, - "property": "Resources[0].userName" - }, - { - "comparison": "not_empty", - "source": "response_json", - "value": null, - "property": "Resources[0].active" - }, - { - "comparison": "not_empty", - "source": "response_json", - "value": null, - "property": "Resources[0].emails[0].value" - } - ], - "variables": [ - { - "source": "response_json", - "name": "ISVUserid", - "property": "Resources[0].id" - } - ], - "scripts": [], - "before_scripts": [] - }, - { - "step_type": "pause", - "skipped": false, - "duration": 5 - }, - { - "step_type": "request", - "skipped": false, - "note": "Required Test: Get Users/{{id}} ", - "auth": {}, - "multipart_form": [], - "headers": { - "Accept-Charset": [ - "utf-8" - ], - "Content-Type": [ - "application/scim+json; charset=utf-8" - ], - "Accept": [ - "application/scim+json" - ], - "Authorization": [ - "{{auth}}" - ], - "User-Agent": [ - "OKTA SCIM Integration" - ] - }, - "method": "GET", - "url": "{{SCIMBaseURL}}/Users/{{ISVUserid}}", - "assertions": [ - { - "comparison": "equal_number", - "source": "response_status", - "value": "200" - }, - { - "comparison": "not_empty", - "source": "response_json", - "value": null, - "property": "id" - }, - { - "comparison": "not_empty", - "source": "response_json", - "value": null, - "property": "name.familyName" - }, - { - "comparison": "not_empty", - "source": "response_json", - "value": null, - "property": "name.givenName" - }, - { - "comparison": "not_empty", - "source": "response_json", - "value": null, - "property": "userName" - }, - { - "comparison": "not_empty", - "source": "response_json", - "value": null, - "property": "active" - }, - { - "comparison": "not_empty", - "source": "response_json", - "value": null, - "property": "emails[0].value" - }, - { - "comparison": "equal", - "source": "response_json", - "value": "{{ISVUserid}}", - "property": "id" - } - ], - "variables": [], - "scripts": [], - "before_scripts": [] - }, - { - "step_type": "pause", - "skipped": false, - "duration": 5 - }, - { - "step_type": "request", - "skipped": false, - "note": "Required Test: Test invalid User by username", - "auth": {}, - "multipart_form": [], - "headers": { - "Accept-Charset": [ - "utf-8" - ], - "Content-Type": [ - "application/scim+json; charset=utf-8" - ], - "Accept": [ - "application/scim+json" - ], - "Authorization": [ - "{{auth}}" - ], - "User-Agent": [ - "OKTA SCIM Integration" - ] - }, - "method": "GET", - "url": "{{SCIMBaseURL}}/Users?filter=userName eq \"{{InvalidUserEmail}}\"", - "assertions": [ - { - "comparison": "equal_number", - "source": "response_status", - "value": "200" - }, - { - "comparison": "has_value", - "source": "response_json", - "value": "urn:ietf:params:scim:api:messages:2.0:ListResponse", - "property": "schemas" - }, - { - "comparison": "equal", - "source": "response_json", - "value": "0", - "property": "totalResults" - } - ], - "variables": [], - "scripts": [], - "before_scripts": [] - }, - { - "step_type": "pause", - "skipped": false, - "duration": 5 - }, - { - "step_type": "request", - "skipped": false, - "note": "Required Test: Test invalid User by ID", - "auth": {}, - "multipart_form": [], - "headers": { - "Accept-Charset": [ - "utf-8" - ], - "Content-Type": [ - "application/scim+json; charset=utf-8" - ], - "Authorization": [ - "{{auth}}" - ], - "Accept": [ - "application/scim+json" - ], - "User-Agent": [ - "OKTA SCIM Integration" - ] - }, - "method": "GET", - "url": "{{SCIMBaseURL}}/Users/{{UserIdThatDoesNotExist}}", - "assertions": [ - { - "comparison": "equal_number", - "source": "response_status", - "value": "404" - }, - { - "comparison": "not_empty", - "source": "response_json", - "value": null, - "property": "detail" - }, - { - "comparison": "has_value", - "source": "response_json", - "value": "urn:ietf:params:scim:api:messages:2.0:Error", - "property": "schemas" - } - ], - "variables": [], - "scripts": [], - "before_scripts": [] - }, - { - "step_type": "pause", - "skipped": false, - "duration": 5 - }, - { - "step_type": "request", - "skipped": false, - "note": "Required Test: Make sure random user doesn't exist", - "auth": {}, - "multipart_form": [], - "headers": { - "Accept-Charset": [ - "utf-8" - ], - "Content-Type": [ - "application/scim+json; charset=utf-8" - ], - "Authorization": [ - "{{auth}}" - ], - "Accept": [ - "application/scim+json" - ], - "User-Agent": [ - "OKTA SCIM Integration" - ] - }, - "method": "GET", - "url": "{{SCIMBaseURL}}/Users?filter=userName eq \"{{randomEmail}}\"", - "assertions": [ - { - "comparison": "equal_number", - "source": "response_status", - "value": "200" - }, - { - "comparison": "equal_number", - "source": "response_json", - "value": "0", - "property": "totalResults" - }, - { - "comparison": "has_value", - "source": "response_json", - "value": "urn:ietf:params:scim:api:messages:2.0:ListResponse", - "property": "schemas" - } - ], - "variables": [], - "scripts": [], - "before_scripts": [] - }, - { - "step_type": "pause", - "skipped": false, - "duration": 5 - }, - { - "step_type": "request", - "skipped": false, - "note": "Required Test: Create Okta user with realistic values", - "auth": {}, - "body": "{\"schemas\":[\"urn:ietf:params:scim:schemas:core:2.0:User\"],\"userName\":\"{{randomUsername}}\",\"name\":{\"givenName\":\"{{randomGivenName}}\",\"familyName\":\"{{randomFamilyName}}\"},\"emails\":[{\"primary\":true,\"value\":\"{{randomEmail}}\",\"type\":\"work\"}],\"displayName\":\"{{randomGivenName}} {{randomFamilyName}}\",\"active\":true}", - "form": {}, - "multipart_form": [], - "binary_body": null, - "headers": { - "Content-Type": [ - "application/json" - ], - "Authorization": [ - "{{auth}}" - ], - "Accept": [ - "application/scim+json; charset=utf-8" - ] - }, - "method": "POST", - "url": "{{SCIMBaseURL}}/Users", - "assertions": [ - { - "comparison": "equal_number", - "source": "response_status", - "value": "201" - }, - { - "comparison": "equal", - "source": "response_json", - "value": "true", - "property": "active" - }, - { - "comparison": "not_empty", - "source": "response_json", - "value": null, - "property": "id" - }, - { - "comparison": "equal", - "source": "response_json", - "value": "{{randomFamilyName}}", - "property": "name.familyName" - }, - { - "comparison": "equal", - "source": "response_json", - "value": "{{randomGivenName}}", - "property": "name.givenName" - }, - { - "comparison": "contains", - "source": "response_json", - "value": "urn:ietf:params:scim:schemas:core:2.0:User", - "property": "schemas" - }, - { - "comparison": "equal", - "source": "response_json", - "value": "{{randomUsername}}", - "property": "userName" - } - ], - "variables": [ - { - "source": "response_json", - "name": "idUserOne", - "property": "id" - }, - { - "source": "response_json", - "name": "randomUserEmail", - "property": "emails[0].value" - } - ], - "scripts": [ - "" - ], - "before_scripts": [] - }, - { - "step_type": "pause", - "skipped": false, - "duration": 5 - }, - { - "step_type": "request", - "skipped": false, - "note": "Required Test: Verify that user was created", - "auth": {}, - "multipart_form": [], - "headers": { - "Accept-Charset": [ - "utf-8" - ], - "Content-Type": [ - "application/scim+json; charset=utf-8" - ], - "Authorization": [ - "{{auth}}" - ], - "Accept": [ - "application/scim+json" - ], - "User-Agent": [ - "OKTA SCIM Integration" - ] - }, - "method": "GET", - "url": "{{SCIMBaseURL}}/Users/{{idUserOne}}", - "assertions": [ - { - "comparison": "equal_number", - "source": "response_status", - "value": "200" - }, - { - "comparison": "equal", - "source": "response_json", - "value": "{{randomUsername}}", - "property": "userName" - }, - { - "comparison": "equal", - "source": "response_json", - "value": "{{randomFamilyName}}", - "property": "name.familyName" - }, - { - "comparison": "equal", - "source": "response_json", - "value": "{{randomGivenName}}", - "property": "name.givenName" - } - ], - "variables": [], - "scripts": [], - "before_scripts": [] - }, - { - "step_type": "pause", - "skipped": false, - "duration": 10 - }, - { - "step_type": "request", - "skipped": false, - "note": "Required Test: Expect failure when recreating user with same values", - "auth": {}, - "body": "{\"schemas\":[\"urn:ietf:params:scim:schemas:core:2.0:User\"],\"userName\":\"{{randomUsername}}\",\"name\":{\"givenName\":\"{{randomGivenName}}\",\"familyName\":\"{{randomFamilyName}}\"},\"emails\":[{\"primary\":true,\"value\":\"{{randomUsername}}\",\"type\":\"work\"}],\"displayName\":\"{{randomGivenName}} {{randomFamilyName}}\",\"active\":true}", - "form": {}, - "multipart_form": [], - "binary_body": null, - "headers": { - "Content-Type": [ - "application/json" - ], - "Authorization": [ - "{{auth}}" - ], - "Accept": [ - "application/scim+json; charset=utf-8" - ] - }, - "method": "POST", - "url": "{{SCIMBaseURL}}/Users", - "assertions": [ - { - "comparison": "equal_number", - "source": "response_status", - "value": "409" - } - ], - "variables": [], - "scripts": [], - "before_scripts": [] - }, - { - "step_type": "pause", - "skipped": false, - "duration": 5 - }, - { - "step_type": "request", - "skipped": false, - "note": "Required Test: Username Case Sensitivity Check", - "auth": {}, - "multipart_form": [], - "headers": { - "Accept-Charset": [ - "utf-8" - ], - "Content-Type": [ - "application/scim+json; charset=utf-8" - ], - "Authorization": [ - "{{auth}}" - ], - "Accept": [ - "application/scim+json" - ], - "User-Agent": [ - "OKTA SCIM Integration" - ] - }, - "method": "GET", - "url": "{{SCIMBaseURL}}/Users?filter=userName eq \"{{randomUsernameCaps}}\"", - "assertions": [ - { - "comparison": "equal_number", - "source": "response_status", - "value": "200" - } - ], - "variables": [], - "scripts": [], - "before_scripts": [] - }, - { - "step_type": "pause", - "skipped": false, - "duration": 5 - }, - { - "step_type": "request", - "skipped": false, - "note": "Optional Test: Verify Groups endpoint", - "auth": {}, - "multipart_form": [], - "headers": { - "Accept-Charset": [ - "utf-8" - ], - "Content-Type": [ - "application/scim+json; charset=utf-8" - ], - "Accept": [ - "application/scim+json" - ], - "Authorization": [ - "{{auth}}" - ], - "User-Agent": [ - "OKTA SCIM Integration" - ] - }, - "method": "GET", - "url": "{{SCIMBaseURL}}/Groups", - "assertions": [ - { - "comparison": "equal_number", - "source": "response_status", - "value": "200" - }, - { - "comparison": "is_less_than", - "source": "response_time", - "value": "600" - } - ], - "variables": [], - "scripts": [ - "var data = JSON.parse(response.body);\nvar max = data.totalResults;\nvar res = data.Resources;\nvar exists = false;\n\nif (max === 0)\n\tassert(\"nogroups\", \"No Groups found in the endpoint\");\nelse if (max >= 1 && Array.isArray(res)) {\n exists = true;\n assert.ok(exists, \"Resources is of type Array\");\n\tlog(exists);\n}" - ], - "before_scripts": [] - }, - { - "step_type": "pause", - "skipped": false, - "duration": 5 - }, - { - "step_type": "request", - "skipped": false, - "note": "Required Test: Check status 401", - "multipart_form": [], - "headers": { - "Accept": [ - "application/scim+json" - ], - "Accept-Charset": [ - "utf-8" - ], - "Authorization": [ - "non-token" - ], - "Content-Type": [ - "application/scim+json; charset=utf-8" - ], - "User-Agent": [ - "OKTA SCIM Integration" - ] - }, - "auth": {}, - "method": "GET", - "url": "{{SCIMBaseURL}}/Users?filter=userName eq \"{{randomUsernameCaps}}\"", - "assertions": [ - { - "comparison": "equal_number", - "source": "response_status", - "value": "401" - }, - { - "comparison": "not_empty", - "source": "response_json", - "value": null, - "property": "detail" - }, - { - "comparison": "equal", - "source": "response_json", - "value": "401", - "property": "status" - }, - { - "comparison": "has_value", - "source": "response_json", - "value": "urn:ietf:params:scim:api:messages:2.0:Error", - "property": "schemas" - } - ], - "variables": [], - "scripts": [], - "before_scripts": [] - }, - { - "step_type": "pause", - "skipped": false, - "duration": 5 - }, - { - "step_type": "request", - "skipped": false, - "note": "Required Test: Check status 404", - "multipart_form": [], - "headers": { - "Accept": [ - "application/scim+json" - ], - "Accept-Charset": [ - "utf-8" - ], - "Authorization": [ - "{{auth}}" - ], - "Content-Type": [ - "application/scim+json; charset=utf-8" - ], - "User-Agent": [ - "OKTA SCIM Integration" - ] - }, - "auth": {}, - "method": "GET", - "url": "{{SCIMBaseURL}}/Users/00919288221112222", - "assertions": [ - { - "comparison": "equal_number", - "source": "response_status", - "value": "404" - }, - { - "comparison": "not_empty", - "source": "response_json", - "value": null, - "property": "detail" - }, - { - "comparison": "equal", - "source": "response_json", - "value": "404", - "property": "status" - }, - { - "comparison": "has_value", - "source": "response_json", - "value": "urn:ietf:params:scim:api:messages:2.0:Error", - "property": "schemas" - } - ], - "variables": [], - "scripts": [], - "before_scripts": [] - } - ] - } \ No newline at end of file diff --git a/tests/test_litellm/test_github_triage_workflows.py b/tests/test_litellm/test_github_triage_workflows.py index ec6e9fc2381..ef3ab8d25da 100644 --- a/tests/test_litellm/test_github_triage_workflows.py +++ b/tests/test_litellm/test_github_triage_workflows.py @@ -62,7 +62,6 @@ DESTRUCTIVE_GATE_ENV: dict[str, str] = { LLM_CLIENT_INSTALLER_WORKFLOWS = ( "triage_issue_with_llm.yml", "triage_reconsider.yml", - "triage_rollout_heads_up.yml", ) PINNED_INSTALL = "--require-hashes -r .github/scripts/triage-requirements.txt" @@ -206,49 +205,6 @@ def test_triage_requirements_are_fully_hash_pinned() -> None: ) -def _heads_up_run_step() -> dict: - workflow = _load_workflow("triage_rollout_heads_up.yml") - for step in workflow["jobs"]["heads-up"]["steps"]: - if isinstance(step.get("run"), str) and "triage_rollout_heads_up.py" in step["run"]: - return step - raise AssertionError("no run step invokes triage_rollout_heads_up.py") - - -def test_rollout_heads_up_push_trigger_never_posts() -> None: - """Merging the heads-up script to staging must stay inert: the automatic - push trigger only ever runs dry-run. The real one-shot sweep is a - deliberate manual `workflow_dispatch` with `dry_run=false`, the sole path - that adds `--close`. - - This guards the "inert by default" invariant for the one workflow that is - intentionally not gated on AGENT_SHIN_ENABLED (it has to warn contributors - before that flag flips on). A regression to auto-`--close`-on-push would - post real comments on every push that touches the script. - """ - run = _heads_up_run_step()["run"] - assert '"${GITHUB_EVENT_NAME:-}" = "workflow_dispatch"' in run, ( - "the real (--close) run must be a manual workflow_dispatch, not the automatic push trigger" - ) - assert '"${DRY_RUN_INPUT:-true}" = "false"' in run, ( - "the real run must require the dry_run input to be the exact string 'false' (fail-safe); any other value stays dry-run" - ) - assert run.count("ARGS+=(--close)") == 1, ( - "--close must appear once, inside the manual real-run branch; a second occurrence means the push path posts real comments on merge" - ) - - -def test_rollout_heads_up_key_is_dispatch_gated() -> None: - """OPENAI_API_KEY is exposed only on the manual dispatch (the real-run - trigger), never unconditionally. The sibling triage workflows gate the key - the same way; an unconditional `secrets.OPENAI_API_KEY` here would hand the - key to the automatic push run, which must stay a no-op dry-run preview. - """ - key_expr = (_heads_up_run_step().get("env") or {}).get("OPENAI_API_KEY", "") - assert "github.event_name == 'workflow_dispatch'" in key_expr, ( - f"OPENAI_API_KEY must be gated on workflow_dispatch so the automatic push trigger gets no key; found: {key_expr!r}" - ) - - def _reconsider_steps() -> list[dict]: workflow = _load_workflow("triage_reconsider.yml") return workflow["jobs"]["reconsider"]["steps"] diff --git a/tests/test_litellm/test_triage_rollout_heads_up.py b/tests/test_litellm/test_triage_rollout_heads_up.py deleted file mode 100644 index 535590fd2d6..00000000000 --- a/tests/test_litellm/test_triage_rollout_heads_up.py +++ /dev/null @@ -1,612 +0,0 @@ -"""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")] - - -# --------------------------------------------------------------------------- -# _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 "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 "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_reconsider_recovery_path(self, heads_up_module): - # OSS authors can't reopen an issue the bot closed (read access only - # lets them reopen issues they closed themselves), so the heads-up - # recovery path is `@agent-shin reconsider`, not self-reopen. - 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 "@agent-shin reconsider" in body - 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, 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": "krrishdholakia"}, - "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, - allowlist=frozenset(), - ) - 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": "mateo-berri"}, - "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": "mateo-berri"}, - "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": "mateo-berri"}, - "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": "mateo-berri"}, - "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, 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": "mateo-berri"}, - "author_association": "NONE", - "body": "thin", - "labels": [], - "created_at": "2026-05-25T00:00:00Z", - } - ) - monkeypatch.setattr( - heads_up_module, "_comments_have_marker", lambda repo, 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, 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="mateo-berri"): - 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), - ] - - -class TestListOpenNumbersNoCap: - """`_list_open_numbers` must sweep the WHOLE backlog, not a capped page. - - Regression guard: the rollout sweep is one-shot, so any item it misses - here never gets a heads-up before the bot starts auto-closing. - """ - - def test_delegates_to_list_open_items_with_no_cap( - self, heads_up_module, monkeypatch - ): - import agent_shin_shared - - captured: dict = {} - - def fake_gh(*args): - captured["args"] = args - return '[{"number": 5}, {"number": 9}]' - - monkeypatch.setattr(agent_shin_shared, "gh", fake_gh) - numbers = heads_up_module._list_open_numbers("o/r", "issue") - assert numbers == [5, 9] - args = captured["args"] - assert args[0] == "issue" - assert args[args.index("--limit") + 1] == str( - agent_shin_shared.GH_LIST_ALL_LIMIT - ) - assert "1000" not in args