ci: make the basedpyright budget gate delta-vs-base (#31106)

* ci: re-run absolute basedpyright budget gate on push to long-lived branches

The basedpyright budget gate counts codebase-wide errors per rule against a
committed ceiling, but it only ran on pull_request against each PR's own head.
Two PRs that each pass in isolation can together push a per-rule count over its
ceiling once both merge, and nothing re-evaluated the budget on the merge
commit, so the breach only surfaced on the next PR that happened to be checked
out after the count crossed the line.

Add a push trigger on the long-lived branches and a post-merge-budget job that
re-runs the absolute gate on the merged tree, catching the accumulation on the
merge commit itself. The existing pull_request jobs are guarded so their
delta-vs-base gates don't misfire on push, where no PR base SHA exists.

* ci: shallow-fetch the post-merge-budget checkout

The post-merge-budget job only runs basedpyright over the working tree and
the committed budget file; it never inspects git history, unlike the lint
job whose delta-vs-base gates need full history. Drop its checkout from
fetch-depth: 0 to fetch-depth: 1 to avoid cloning the whole repo history.

* ci: scope post-merge-budget push trigger to long-lived branches

On a push event the branches filter matches the branch being pushed to,
not the PR target. The litellm_** glob, correct for the pull_request
filter where it matches the target branch, therefore fired the
post-merge-budget basedpyright job on every short-lived feature branch
carrying the litellm_ prefix (litellm_dev_*, litellm_add_*, and so on),
duplicating the PR lint job and burning ~10 minutes of CI per push.

Restrict the push trigger to the long-lived branches PRs actually merge
into (main, litellm_internal_staging, litellm_oss_branch), where budget
accumulation happens. The pull_request filter keeps litellm_** so PRs
targeting any long-lived branch are still linted.

* ci: make the basedpyright budget gate delta-vs-base

The basedpyright gate counted absolute codebase-wide errors per rule against a
committed ceiling and ran only on each PR's own head. Two PRs that each pass in
isolation could together push a rule past its ceiling once both merged, and
because the gate had no comparison against the base, the next unrelated PR
branched off the now-over-ceiling tree inherited a red it did nothing to cause.

Give it the same shape as the ruff strict gate: a rule fails only when its total
is both over the ceiling and higher than the count on the merge-base it merges
into. Drift already in the base is never blamed on a bystander, while any change
that actually grows a rule past the cap still fails. Head counts come from the
existing stdin pipe; the base count is a second basedpyright pass over a detached
worktree at the merge-base, reusing the head environment so import resolution
matches and no second uv sync is needed.

This obsoletes the push-triggered post-merge-budget job (and its event guards),
which only detected accumulation after the fact; the delta check blocks it on the
PR instead. Slack for reportReturnType and reportUnnecessaryComparison is raised
to give real headroom under the cap.

* refactor(ci): give the base ref its own name in type_check_gate cmd_check

cmd_check took a parameter named base that held a git ref string, then
rebound the same name to the dict of base-tree error counts returned by
base_counts. Rename the parameter to base_ref so the ref and the counts
each keep a single name and type, matching the no-reassignment style used
elsewhere; behavior is unchanged.

---------

Co-authored-by: Claude <noreply@anthropic.com>

.github/workflows/test-linting.yml

@@ -14,7 +14,7 @@ permissions:
 jobs:
   lint:
     runs-on: ubuntu-latest
-    timeout-minutes: 10
+    timeout-minutes: 15
 
     steps:
       - uses: actions/checkout@08eba0b27e820071cde6df949e0beb9ba4906955 # v4.3.0
@@ -87,9 +87,11 @@ jobs:
         run: |
           uv run --no-sync python -c "import openai; print(f'OpenAI version: {openai.__version__}')"
 
-      - name: Run basedpyright type checking
+      - name: Check basedpyright budget (delta vs base)
+        env:
+          BASE_SHA: ${{ github.event.pull_request.base.sha }}
         run: |
-          (uv run --no-sync basedpyright --outputjson || true) | uv run --no-sync python scripts/type_check_gate.py
+          (uv run --no-sync basedpyright --outputjson || true) | uv run --no-sync python scripts/type_check_gate.py --base "$BASE_SHA"
 
       - name: Check for circular imports
         run: |

Makefile

@@ -125,7 +125,8 @@ lint-ruff-FULL-dev: install-dev
 	else echo "No changed .py files to check."; fi
 
 lint-basedpyright: install-dev
-	($(UV_RUN) basedpyright --outputjson || true) | $(UV_RUN) python scripts/type_check_gate.py
+	git fetch origin litellm_internal_staging
+	($(UV_RUN) basedpyright --outputjson || true) | $(UV_RUN) python scripts/type_check_gate.py --base origin/litellm_internal_staging
 
 lint-basedpyright-budget-update: install-dev
 	($(UV_RUN) basedpyright --outputjson || true) | $(UV_RUN) python scripts/type_check_gate.py --update

basedpyright-code-budget.json

@@ -121,7 +121,7 @@
   },
   "reportReturnType": {
     "baseline": 126,
-    "slack": 13
+    "slack": 100
   },
   "reportTypedDictNotRequiredAccess": {
     "baseline": 20,
@@ -157,7 +157,7 @@
   },
   "reportUnnecessaryComparison": {
     "baseline": 683,
-    "slack": 10
+    "slack": 100
   },
   "reportUnnecessaryContains": {
     "baseline": 4,

scripts/type_check_gate.py

@@ -1,21 +1,22 @@
 #!/usr/bin/env python3
-"""Per-rule count gate for basedpyright.
+"""Delta-vs-base per-rule gate for basedpyright.
 
 basedpyright's ``--outputjson`` is reduced to a count of errors per *rule*
 (``reportAny``, ``reportArgumentType``, ...) and checked against a committed
 budget of the form ``{rule: {baseline, slack}}``, the same shape as
-``ruff-strict-budget.json``. A rule fails when its codebase-wide total exceeds
-``baseline + slack``. Counts ignore file, line, and column, so a violation
-moving anywhere in the tree is invisible; only the per-rule total moves the
-needle.
+``ruff-strict-budget.json``. A rule fails only when its codebase-wide total is
+both over its ceiling (``baseline + slack``) *and* higher than the count on the
+base it merges into, so a change is blamed for the errors it adds, never for
+drift that already sits in the base. That ``> base`` guard is what stops an
+unrelated PR from inheriting a red once two PRs each land near the ceiling and
+their sum crosses it: the bystander's count equals its base, so it is spared,
+while any PR that actually grows the rule past the cap still fails.
 
-Unlike ``ruff_strict_gate.py`` this does *not* re-run the tool on the merge base
-to compute a delta: a second basedpyright pass is minutes and gigabytes, whereas
-ruff is milliseconds. The committed budget is the baseline instead -- exactly
-how the previous per-file gate worked -- so keep it fresh with ``--update``
-(ratchet), which re-captures every rule's count from the current tree while
-preserving each rule's slack. Tool output is read from stdin, so the caller
-decides how to invoke basedpyright (and from which cwd).
+Head counts are read from stdin (the caller runs basedpyright once and pipes
+``--outputjson`` in); the base count is a second basedpyright pass over a
+detached worktree at the merge-base, run under the same environment so import
+resolution matches. ``--update`` re-captures the absolute per-rule baselines for
+the ratchet, preserving each rule's slack.
 
 ``--outputjson`` is used rather than text diagnostics because the latter wrap
 across lines, leaving the ``(reportRule)`` on a continuation line away from the
@@ -24,13 +25,21 @@ carries an unambiguous ``rule`` field.
 """
 
 import argparse
+import contextlib
 import json
+import shutil
+import subprocess
 import sys
+import tempfile
 from collections import Counter
+from collections.abc import Iterator, Mapping
 from pathlib import Path
-from typing import Mapping, NamedTuple
+from typing import NamedTuple
 
 REPO_ROOT = Path(__file__).resolve().parent.parent
+BUDGET_PATH = REPO_ROOT / "basedpyright-code-budget.json"
+PYRIGHT_CONFIG = REPO_ROOT / "pyrightconfig.json"
+DEFAULT_BASE = "origin/litellm_internal_staging"
 
 # Bucket for a basedpyright diagnostic with no `rule`. Counted so it's gated.
 UNCODED = "<uncoded>"
@@ -45,6 +54,7 @@ class Breach(NamedTuple):
     code: str
     total: int
     cap: int
+    added: int
 
 
 def _seed_slack(baseline: int) -> int:
@@ -54,18 +64,19 @@ def _seed_slack(baseline: int) -> int:
     return 10 if baseline >= 50 else 3
 
 
-def _to_repo_relative(raw: str) -> str | None:
+def _to_relative(raw: str, root: Path) -> str | None:
     path = Path(raw)
-    absolute = path if path.is_absolute() else Path.cwd() / path
+    absolute = path if path.is_absolute() else root / path
     try:
-        return absolute.resolve().relative_to(REPO_ROOT).as_posix()
+        return absolute.resolve().relative_to(root).as_posix()
     except ValueError:
         return None
 
 
-def count_basedpyright(payload: str) -> dict[str, int]:
-    """Count in-repo basedpyright errors per rule from `--outputjson`. Warnings
-    and information are ignored; only `severity == "error"` is gated."""
+def count_basedpyright(payload: str, root: Path = REPO_ROOT) -> dict[str, int]:
+    """Count in-tree basedpyright errors per rule from `--outputjson`. Warnings
+    and information are ignored; only `severity == "error"` is gated. Files
+    outside `root` (the venv's site-packages, say) are dropped."""
     try:
         data = json.loads(payload or "{}")
     except json.JSONDecodeError as exc:
@@ -79,21 +90,62 @@ def count_basedpyright(payload: str) -> dict[str, int]:
     for diag in data.get("generalDiagnostics", []):
         if diag.get("severity") != "error":
             continue
-        if _to_repo_relative(diag.get("file", "")) is None:
+        if _to_relative(diag.get("file", ""), root) is None:
             continue
         counts[diag.get("rule") or UNCODED] += 1
     return dict(counts)
 
 
+def _run(cmd: list[str], cwd: Path = REPO_ROOT) -> str:
+    proc = subprocess.run(cmd, cwd=cwd, capture_output=True, text=True)
+    if proc.returncode not in (0, 1):
+        sys.stderr.write(proc.stderr)
+        raise SystemExit(f"{cmd[0]} exited {proc.returncode}")
+    return proc.stdout
+
+
+@contextlib.contextmanager
+def _temp_worktree(ref: str) -> Iterator[Path]:
+    parent = Path(tempfile.mkdtemp(prefix="bpr_base_"))
+    worktree = parent / "wt"
+    try:
+        _run(["git", "worktree", "add", "--detach", str(worktree), ref])
+        yield worktree
+    finally:
+        subprocess.run(
+            ["git", "worktree", "remove", "--force", str(worktree)],
+            cwd=REPO_ROOT,
+            capture_output=True,
+            text=True,
+        )
+        shutil.rmtree(parent, ignore_errors=True)
+
+
+def base_counts(ref: str) -> dict[str, int]:
+    """basedpyright error counts per rule for the merge-base tree. The head
+    config is copied in so the base is judged by today's rules, and the run uses
+    the head environment's basedpyright (on PATH) so imports resolve the same."""
+    exe = shutil.which("basedpyright") or "basedpyright"
+    with _temp_worktree(ref) as worktree:
+        shutil.copy(PYRIGHT_CONFIG, worktree / "pyrightconfig.json")
+        proc = subprocess.run(
+            [exe, "--outputjson"], cwd=worktree, capture_output=True, text=True
+        )
+        return count_basedpyright(proc.stdout, root=worktree)
+
+
 def evaluate(
-    counts: Mapping[str, int], budget: Mapping[str, Mapping[str, int]]
+    head: Mapping[str, int],
+    base: Mapping[str, int],
+    budget: Mapping[str, Mapping[str, int]],
 ) -> list[Breach]:
     breaches = []
-    for code, total in counts.items():
+    for code, total in head.items():
         spec = budget.get(code)
         cap = spec["baseline"] + spec["slack"] if spec else DEFAULT_SLACK
-        if total > cap:
-            breaches.append(Breach(code, total, cap))
+        prior = base.get(code, 0)
+        if total > cap and total > prior:
+            breaches.append(Breach(code, total, cap, total - prior))
     return sorted(breaches)
 
 
@@ -107,9 +159,6 @@ def is_vacuous_run(
     return not counts and any(spec["baseline"] for spec in budget.values())
 
 
-BUDGET_PATH = REPO_ROOT / "basedpyright-code-budget.json"
-
-
 def cmd_update(counts: Mapping[str, int]) -> None:
     existing = json.loads(BUDGET_PATH.read_text()) if BUDGET_PATH.exists() else {}
     budget = {
@@ -127,9 +176,10 @@ def cmd_update(counts: Mapping[str, int]) -> None:
     )
 
 
-def cmd_check(counts: Mapping[str, int]) -> None:
+def cmd_check(base_ref: str) -> None:
     budget = json.loads(BUDGET_PATH.read_text())
-    if is_vacuous_run(counts, budget):
+    head = count_basedpyright(sys.stdin.read())
+    if is_vacuous_run(head, budget):
         expected = sum(spec["baseline"] for spec in budget.values())
         print(
             f"FAIL: basedpyright produced no errors, but {BUDGET_PATH.name} expects "
@@ -137,27 +187,44 @@ def cmd_check(counts: Mapping[str, int]) -> None:
             f"nothing; refusing to certify a vacuous run."
         )
         raise SystemExit(1)
-    breaches = evaluate(counts, budget)
+    base_point = _run(["git", "merge-base", base_ref, "HEAD"]).strip() or base_ref
+    base = base_counts(base_point)
+    if is_vacuous_run(base, budget):
+        print(
+            f"FAIL: basedpyright produced no errors for the base tree at "
+            f"{base_point[:12]}, so every rule would look freshly added. The base "
+            f"pass almost certainly crashed; refusing to blame this change for it."
+        )
+        raise SystemExit(1)
+    breaches = evaluate(head, base, budget)
     if not breaches:
         print(
-            f"OK: every rule is within its basedpyright ceiling ({sum(counts.values())} errors total)"
+            f"OK: every rule is within its basedpyright ceiling or no higher than base ({sum(head.values())} errors total)"
         )
         return
     print("FAIL: basedpyright errors exceed the per-rule ceiling:")
     for breach in breaches:
-        print(f"  {breach.code}: {breach.total} errors over cap {breach.cap}")
+        print(
+            f"  {breach.code}: total {breach.total} over cap {breach.cap} (this change added {breach.added})"
+        )
     print(
-        "Resolve the new errors, or run 'make lint-basedpyright-budget-update' if the ceiling should move."
+        "Reduce the new errors or remove an equal number elsewhere; the ceiling is "
+        "baseline + slack in basedpyright-code-budget.json."
     )
+    summary = "; ".join(f"{b.code} {b.total}/{b.cap} (+{b.added})" for b in breaches)
+    print(f"BREACHED RULES: {summary}")
     raise SystemExit(1)
 
 
 def main() -> None:
     parser = argparse.ArgumentParser(description=__doc__)
+    parser.add_argument("--base", default=DEFAULT_BASE)
     parser.add_argument("--update", action="store_true")
     args = parser.parse_args()
-    counts = count_basedpyright(sys.stdin.read())
-    cmd_update(counts) if args.update else cmd_check(counts)
+    if args.update:
+        cmd_update(count_basedpyright(sys.stdin.read()))
+    else:
+        cmd_check(args.base)
 
 
 if __name__ == "__main__":

tests/test_litellm/test_type_check_gate.py

@@ -56,29 +56,58 @@ def test_paths_outside_repo_are_skipped():
 
 def test_at_or_under_ceiling_passes():
     budget = {"no-any-return": {"baseline": 5, "slack": 0}}
-    assert gate.evaluate({"no-any-return": 5}, budget) == []
+    assert gate.evaluate({"no-any-return": 5}, {}, budget) == []
 
 
 def test_one_more_error_than_ceiling_fails():
     budget = {"no-any-return": {"baseline": 5, "slack": 0}}
-    assert gate.evaluate({"no-any-return": 6}, budget) == [
-        gate.Breach("no-any-return", 6, 5)
+    assert gate.evaluate({"no-any-return": 6}, {}, budget) == [
+        gate.Breach("no-any-return", 6, 5, 6)
     ]
 
 
 def test_slack_absorbs_small_increase_then_fails_past_it():
     budget = {"arg-type": {"baseline": 5, "slack": 5}}
-    assert gate.evaluate({"arg-type": 10}, budget) == []
-    assert gate.evaluate({"arg-type": 11}, budget) == [gate.Breach("arg-type", 11, 10)]
+    assert gate.evaluate({"arg-type": 10}, {}, budget) == []
+    assert gate.evaluate({"arg-type": 11}, {}, budget) == [
+        gate.Breach("arg-type", 11, 10, 11)
+    ]
 
 
 def test_unbudgeted_new_code_uses_default_slack():
-    assert gate.evaluate({"brand-new": gate.DEFAULT_SLACK}, {}) == []
-    assert gate.evaluate({"brand-new": gate.DEFAULT_SLACK + 1}, {}) == [
-        gate.Breach("brand-new", gate.DEFAULT_SLACK + 1, gate.DEFAULT_SLACK)
+    assert gate.evaluate({"brand-new": gate.DEFAULT_SLACK}, {}, {}) == []
+    assert gate.evaluate({"brand-new": gate.DEFAULT_SLACK + 1}, {}, {}) == [
+        gate.Breach(
+            "brand-new",
+            gate.DEFAULT_SLACK + 1,
+            gate.DEFAULT_SLACK,
+            gate.DEFAULT_SLACK + 1,
+        )
     ]
 
 
+def test_drift_already_over_cap_in_base_is_not_blamed_on_a_flat_change():
+    # The bystander case: a rule sits over its ceiling because two earlier PRs
+    # summed past it. A PR that branches off that base and adds nothing must pass
+    # -- total > cap but total == base, so the `> base` guard spares it.
+    budget = {"arg-type": {"baseline": 5, "slack": 5}}
+    assert gate.evaluate({"arg-type": 12}, {"arg-type": 12}, budget) == []
+
+
+def test_change_that_grows_an_over_cap_rule_is_blamed_for_only_what_it_added():
+    # Over cap AND above base: blamed, and `added` is the delta vs base, not the
+    # whole overage, so the message points at this change's contribution.
+    budget = {"arg-type": {"baseline": 5, "slack": 5}}
+    assert gate.evaluate({"arg-type": 14}, {"arg-type": 12}, budget) == [
+        gate.Breach("arg-type", 14, 10, 2)
+    ]
+
+
+def test_reducing_an_over_cap_rule_below_base_passes():
+    budget = {"arg-type": {"baseline": 5, "slack": 5}}
+    assert gate.evaluate({"arg-type": 11}, {"arg-type": 12}, budget) == []
+
+
 def test_no_output_against_a_nonempty_budget_is_a_vacuous_run():
     # A crashed type checker emits nothing; the gate must not certify it as clean.
     budget = {"no-untyped-def": {"baseline": 4888, "slack": 10}}