litellm/scripts/budget_ratchet_check.py
yuneng-jiang ffab5a39d0
feat(ci): ratchet the test suite's zero-assert, mock-echo and global-state debt (#37588)
* feat(ci): ratchet the test suite's zero-assert, mock-echo and global-state debt

The suite's dominant failure mode is tests that cannot fail for the reason anyone
would want them to. The testing-strategy audit measured five shapes of it, and
nothing mechanical stops any of them from reproducing, so they keep reproducing.

`scripts/check_test_quality.py` is an AST checker for those five, emitting the
same `path:line: CODE message` contract as `scripts/check_type_discipline.py`:

  TQ001  a collectible test with no assertion of any kind
  TQ002  mock-echo, where every assertion only inspects the mock that was patched
  TQ003  sys.path.insert inside the test tree
  TQ004  raw `os.environ[...] =`, which leaks into whatever runs next
  TQ005  `litellm.<attr> =`, the process-wide leak the 491-line conftest undoes

`scripts/test_quality_gate.py` caps each rule against test-quality-budget.json,
seeded at exactly today's count, and fails only when a rule is both over its
limit and higher than the base being merged into, so a change is blamed for what
it adds and never for drift already in the base. `--update` lowers a limit by
what a branch cleared, so the ceilings only ever fall. It runs in the existing
required lint job, which means it enforces without a ruleset change.

TQ001 follows assertions into helpers defined in the same module, transitively.
Without that it flagged 111 tests in tests/e2e, the harness this program holds up
as the reference, because that suite factors its assertions into shared helpers
(`assert_auth_denied(result, ...)`). Following them leaves 25, all of which reach
their assertions across a module boundary; those are grandfathered and documented
rather than papered over.

The seeded counts land within about 10% of the audit's independent numbers for
every rule measured on the same subtree, which is the cross-check that the
definitions here match the ones the audit pinned.

* fix(ci): resolve test helpers per scope, not by bare name

The helper walk keyed every function in a module by its bare name, so two
same-named helpers in different classes collided and the last one parsed won.
A test calling `self._check()` could be cleared by a `_check` belonging to a
different class, or flagged because of one.

Resolution is now scoped: a bare name looks up the module-level functions, and
`self.<name>` looks up the enclosing class's own methods and no other class's.
Recursion is tracked by function identity rather than by name, so the cycle
guard cannot be confused by the same collision.

This surfaced one real zero-assert test that a same-named helper elsewhere had
been clearing, so TQ001 seeds at 750 rather than 749.

The test module has to register itself in sys.modules before exec_module:
`@dataclass(slots=True)` rebuilds its class through `sys.modules[__module__]`,
and Scope fails to construct without it. Recorded at the call site, since it
reads like avoidable global mutation otherwise.

* fix: register test-quality-budget.json with the ratchet alarm

The repo keeps one census over its budget files: every *-budget.json on disk
must appear in DEFAULT_BUDGETS, or its ceilings can be raised with no signal.
tests/test_litellm/test_budget_ratchet_check.py asserts that set equality and
caught the new budget on the way in.

Registering it also turns the alarm on for TQ001-TQ005, so a later PR cannot
quietly raise a test-quality ceiling. The file already uses the {limit: N}
schema the ratchet reads, so no other change was needed.
2026-08-20 10:08:49 -07:00

228 lines
7.5 KiB
Python

#!/usr/bin/env python3
"""Non-gating ratchet guard: budget limits may only fall, never rise.
Every `*-budget.json` file (ruff-strict, type-discipline, basedpyright-code) is a
one-way ratchet: each rule's ceiling is its `limit`, and that limit is meant to be
driven DOWN over time. This check compares every budget file against its own
content at the merge-base with the target branch and fails (exits 1, red) if:
* a rule's `limit` went up,
* a rule was dropped from a budget (its ceiling effectively became infinite), or
* an entire budget file was deleted.
New rules and lowered/equal limits are fine. So is a rule that graduated: once a
paired config (ruff.toml for the ruff-strict budget) selects the rule outright it
hard-fails at the first violation, which is stricter than any ceiling the budget
could hold, so dropping its entry tightens the guard rather than removing it.
This is deliberately NOT a gating check. It should turn the run red so that a
loosening is impossible to miss in review, but it must stay OUT of the
branch-protection required-checks list: a justified bump (e.g. banning a new API,
which mechanically raises a baseline) can then still be merged by a human who has
seen the red and accepted it.
Usage:
python scripts/budget_ratchet_check.py [--base REF] [budget.json ...]
Stdlib only.
"""
from __future__ import annotations
import argparse
import json
import subprocess
import sys
import tomllib
from pathlib import Path
from types import MappingProxyType
from typing import NamedTuple
REPO_ROOT = Path(__file__).resolve().parent.parent
DEFAULT_BASE = "origin/litellm_internal_staging"
DEFAULT_BUDGETS: tuple[str, ...] = (
"ruff-strict-budget.json",
"type-discipline-budget.json",
"basedpyright-code-budget.json",
"test-quality-budget.json",
)
GRADUATION_CONFIGS = MappingProxyType({"ruff-strict-budget.json": "ruff.toml"})
class Regression(NamedTuple):
budget: str
rule: str
detail: str
def _run(cmd: list[str]) -> subprocess.CompletedProcess[str]:
return subprocess.run(cmd, cwd=REPO_ROOT, capture_output=True, text=True)
def _merge_base(base: str) -> str:
"""The common ancestor of `base` and HEAD, so unrelated base drift is ignored."""
proc = _run(["git", "merge-base", base, "HEAD"])
return proc.stdout.strip() or base
def _load_head(rel: str) -> dict | None:
path = REPO_ROOT / rel
if not path.exists():
return None
return json.loads(path.read_text())
def _ref_is_commit(ref: str) -> bool:
return (
_run(
["git", "rev-parse", "--verify", "--quiet", f"{ref}^{{commit}}"]
).returncode
== 0
)
def _load_base(rel: str, ref: str) -> dict | None:
"""Budget content at `ref`, or None when the file did not exist there.
`ref` is verified as a real commit by the caller, so a non-zero `git show` here means
the path was absent at that commit, not that the ref itself is unresolvable.
"""
proc = _run(["git", "show", f"{ref}:{rel}"])
if proc.returncode != 0:
return None
return json.loads(proc.stdout)
def _ceiling(spec: dict) -> int:
"""A rule's ceiling: its `limit`, or legacy `baseline + slack`.
The base side of the diff can predate the `limit` migration, so a spec is read
under either schema and the two are compared on the same footing.
"""
if "limit" in spec:
return int(spec["limit"])
return int(spec.get("baseline", 0)) + int(spec.get("slack", 0))
def _limits(budget: dict) -> dict[str, int]:
"""Map each rule to its ceiling; skip malformed specs."""
return {
rule: _ceiling(spec)
for rule, spec in budget.items()
if isinstance(spec, dict)
}
def selectors_hard_failed_by(lint: dict) -> tuple[str, ...]:
"""A ruff `[lint]` table's selected codes, minus anything `ignore` turns back off.
`lint.ignore` wins over `lint.extend-select` in ruff, so an ignored code is not
actually enforced and must not count as a graduation.
"""
ignored = tuple(lint.get("ignore", ()))
return tuple(
selector
for selector in lint.get("extend-select", ())
if not (ignored and selector.startswith(ignored))
)
def graduated_selectors(rel: str) -> tuple[str, ...]:
"""Selectors the budget's paired ruff config hard-fails, so its ceiling is moot."""
config = GRADUATION_CONFIGS.get(rel)
if config is None or not (REPO_ROOT / config).exists():
return ()
return selectors_hard_failed_by(
tomllib.loads((REPO_ROOT / config).read_text()).get("lint", {})
)
def _regression_detail(
rule: str,
base_limits: dict[str, int],
head_limits: dict[str, int],
graduated: tuple[str, ...],
) -> str | None:
"""Why `rule` regressed vs base, or None when it held flat, fell, or graduated.
A dropped rule is terminal unless it graduated; otherwise the only loosening
left is a raised limit.
"""
base_limit = base_limits[rule]
if rule not in head_limits:
if graduated and rule.startswith(graduated):
return None
return f"rule dropped (limit {base_limit} -> removed)"
if head_limits[rule] > base_limit:
return f"limit raised {base_limit} -> {head_limits[rule]}"
return None
def regressions_for(
rel: str,
base: dict | None,
head: dict | None,
graduated: tuple[str, ...] = (),
) -> list[Regression]:
if base is None:
return [] # new budget file: nothing to ratchet against yet
if head is None:
return [Regression(rel, "*", "budget file was deleted (every limit removed)")]
base_limits, head_limits = _limits(base), _limits(head)
return [
Regression(rel, rule, detail)
for rule in sorted(base_limits)
if (detail := _regression_detail(rule, base_limits, head_limits, graduated)) is not None
]
def main() -> int:
parser = argparse.ArgumentParser(description=__doc__)
parser.add_argument("--base", default=DEFAULT_BASE)
parser.add_argument("budgets", nargs="*", help="budget files to check")
args = parser.parse_args()
budgets = args.budgets or list(DEFAULT_BUDGETS)
ref = _merge_base(args.base)
if not _ref_is_commit(ref):
print(
f"FAIL: base ref {ref!r} does not resolve to a commit, so the ratchet has nothing "
f"to compare against; refusing to pass vacuously (check the --base / BASE_SHA value)",
file=sys.stderr,
)
return 1
regressions: list[Regression] = []
checked: list[str] = []
for rel in budgets:
base = _load_base(rel, ref)
head = _load_head(rel)
if base is None and head is None:
continue
if base is None:
print(f"skip {rel}: new file (no base at {args.base} to ratchet against)")
continue
checked.append(rel)
regressions.extend(regressions_for(rel, base, head, graduated_selectors(rel)))
if regressions:
print(
f"FAIL: budget limit(s) loosened vs base {args.base} (merge-base {ref[:12]}):"
)
for reg in regressions:
print(f" {reg.budget} {reg.rule}: {reg.detail}")
print(
"Budgets are one-way ratchets and may only go down or stay flat. This "
"check is non-gating: if the increase is justified (e.g. a newly banned "
"API), a human can merge over the red after acknowledging it."
)
return 1
suffix = f" ({', '.join(checked)})" if checked else ""
print(f"OK: no budget limit increased vs base {args.base}{suffix}")
return 0
if __name__ == "__main__":
raise SystemExit(main())