mirror of
https://github.com/BerriAI/litellm.git
synced 2026-08-28 05:25:59 +00:00
* ci(lint): enforce type-discipline budget for casts and type guards Add a ratcheted gate that blocks net-new typing.cast() usage and bans TypeGuard/TypeIs outright, layered on the existing ruff-strict budget setup. - ruff-strict.toml: ban cast/TypeGuard/TypeIs (typing + typing_extensions) via flake8-tidy-imports banned-api (TID251) for a coarse import-level freeze. - ruff-strict-budget.json: bump TID251 baseline 2404 -> 2662 to absorb the ~258 pre-existing usages now matched by the new banned-api entries. - scripts/check_type_discipline.py: AST checker adding LIT006 (cast call sites, suppress with `# cast-ok: <reason>`) and LIT007 (TypeGuard/TypeIs annotations, suppress with `# guard-ok: <reason>`) for per-call-site granularity. - scripts/type_discipline_gate.py: baseline+slack gate with delta-vs-base, mirroring ruff_strict_gate.py. - type-discipline-budget.json: LIT006 baseline 1013 (slack 10), LIT007 0/0. - test-linting.yml: run the gate in CI against the PR base SHA. * ci(lint): enforce suppression-reason budgets and guard budgets against loosening - wire the **kwargs ban (LIT008) into the vendored type-discipline checker so it matches the budget that already referenced it - freeze LIT003/LIT004 (noqa / type-ignore without codes or reason) and LIT005 (*-ok suppression without a reason) at slack 0 so any net-new unexplained suppression trips the type-discipline gate - add scripts/budget_ratchet_check.py and a separate, non-gating budget-ratchet CI job that turns red when any *-budget.json ceiling is raised, a rule is dropped, or a budget file is deleted * ci(lint): ban mutable collections in annotations and all mutable construction Expand LIT001 from coarse builtins at interfaces to any mutable collection in any annotation (builtins, typing aliases, collections concretes, mutable ABCs) across signatures, class attributes, locals, and globals. Add LIT009 to flag mutable-collection construction (literals, comprehensions, constructors) so the unannotated seed-then-mutate pattern is caught too. Enumerate any-ok in LIT005 so its reason requirement holds even when only the stdlib checker runs. Budget LIT001 (21452) and LIT009 (25222) with slack 10 to ratchet down. * ci(lint): recommend pydantic at boundaries and add functional-refactor guidance Drop the msgspec mention from the cast banned-api messages so the recommended validation path matches the codebase's primary pattern (pydantic). Add a note to CLAUDE.md that lint / type-discipline failures should be resolved by refactoring to functional, immutable patterns rather than reaching for mutable structures or `# mutable-ok`. * style: make CLAUDE.md more concise * chore: update CLAUDE.md guidelines * ci(lint): renumber mutable construction LIT009 -> LIT002 next to LIT001 Group the mutable-collection family together: LIT001 (mutable collection in any annotation) and the construction rule now sit adjacent at LIT001/LIT002. The freed LIT009 slot is taken by the sibling Any gate (check_any_discipline.py, #30379), which moves its Any-typed-value rule LIT002 -> LIT009 in lockstep so the shared LIT namespace stays contiguous with no holes. Budget, gate docstring, and the checker's own docstring/messages are updated to match. * fix: numbering in CLAUDE.md * test(lint): test type-discipline checker, scope LIT007 to return types Add regression tests for check_type_discipline.py (every LIT rule, its suppression, and the comment scanner) and for budget_ratchet_check.py. Confine LIT007 to function return annotations, the only place TypeGuard/TypeIs are valid, so a runtime name that merely reads those identifiers is no longer flagged. Switch scan_comments to io.StringIO(source).readline, the standard readline that returns '' at EOF, dropping the iter(...).__next__ idiom. * fix(lint): best-effort worktree teardown so cleanup can't mask the real error base_counts ran `git worktree remove` through the raising `_run` in its finally, so a failed `git worktree add` (or a failure in the body) was masked by a second SystemExit from the cleanup. Tear the worktree down best-effort, like the sibling rmtree, so the original error propagates. * fix(lint): ratchet fails loudly on an unresolvable base; drop dead checker state Verify the merge-base ref resolves to a commit before trusting a missing-file result from git show, so an invalid or empty BASE_SHA now turns the budget-ratchet guard red instead of skipping every budget and passing vacuously Also drop the unused Comments.by_line field and the phantom --changed-only usage line from check_type_discipline's docstring, and cover the ref handling with tests * fix(lint): degrade malformed source to LIT000 instead of crashing the checker tokenize.generate_tokens raises IndentationError (a SyntaxError subclass) on a dedent mismatch, which escaped scan_comments' tokenize.TokenError handler and crashed the whole checker run, zeroing the gate for that invocation. Catch SyntaxError too so the file falls through to ast.parse and is reported as LIT000, matching the checker's graceful-degradation contract. Also add the trailing newline ruff-strict.toml lacked * perf(lint): skip the base worktree scan when no rule is over its ceiling cmd_check created a git worktree and re-scanned the base tree on every run, but a rule can only breach when its head count is already over baseline + slack; when none are, the base comparison cannot change the verdict. Short-circuit to OK in that case, which is every green PR, roughly halving the gate's work. Extract over_ceiling and cover it (and evaluate's drift-safety) with tests * fix(lint): exempt .dict()/.list()/.set() method calls from LIT002 _construction_kind matched dict/list/set as constructors via func.attr too, flagging common method calls like pydantic's model.dict() as mutable construction; 200 such false positives existed in litellm. Recognize dict/list/set construction only when unqualified while keeping the collections concretes (deque/defaultdict/...) matchable as attributes, since those are rarely method names. Ratchet the LIT002 baseline down 25222 -> 25022 to reflect the removed false positives * chore(lint): bump basedpyright ceilings to absorb staging base drift The basedpyright gate added in #30379 is a total-count check against basedpyright-code-budget.json and the linting workflow runs only on pull_request, so pushes to litellm_internal_staging never re-baseline it. Merging staging into this branch surfaced that drift: seven reportAny/reportUnknown* rules sit 10-149 errors above their committed ceiling even though this PR changes no files under litellm/, the only path basedpyright scans (pyrightconfig include is litellm). The new baselines match the counts CI measured on the merge commit, with the existing per-rule slack preserved * fix(lint): ratchet guard watches every budget file, not just two DEFAULT_BUDGETS only listed ruff-strict-budget.json and type-discipline-budget.json, so mypy-code-budget.json and basedpyright-code-budget.json were unguarded and their ceilings could rise with no signal, which is exactly the failure mode this guard exists to prevent. The gap became concrete when this PR bumped basedpyright-code-budget.json to absorb staging drift. All four budgets are now watched, so the budget-ratchet job surfaces that basedpyright bump for human review the same way it surfaces the TID251 raise. A regression test pins that every *-budget.json on disk is in DEFAULT_BUDGETS, failing loudly if a future budget escapes the ratchet * fix: add a lot more slack * fix(lint): restore LIT003 frozen slack to 0 The blanket slack bump set LIT003 (bare # noqa without codes or a reason) to a slack of 50, which contradicts the documented zero-tolerance invariant: the gate docstring and the PR description table both freeze LIT003/LIT004/LIT005 at slack 0 so any net-new unexplained suppression trips the gate. Slack 50 would let 50 new bare noqas through silently. The actual LIT003 count is 397, well under the 516 baseline, so restoring slack to 0 keeps the gate green while putting the freeze back. LIT004/LIT005/LIT007 were already correct at 0 * fix(lint): restore documented slack 10 for the buffered LIT rules The slack bump left LIT001/LIT002/LIT006/LIT008 at 2000/2500/100/100, 10-250x the "/ 10" the PR description table and the gate docstring document. That buffer was never needed: the gate already blames a rule only when its count exceeds the ceiling and grew vs the merge-base, so the violations the staging merge added in litellm/ sit in both head and base and are never charged to this PR. With slack back at the documented 10 the gate stays green, and the ceiling is tight again (LIT006 no longer waves through 99 net-new cast() calls). Baselines are unchanged; only the slack returns to its documented value * fix(lint): ratchet LIT003 baseline down to its actual count The LIT003 baseline was 516 while the current bare-noqa count is 397, leaving ~119 units of headroom that undercut the documented zero-tolerance freeze: the gate docstring claims any net-new bare noqa trips the gate, but with cap 516 a PR could add over a hundred first. Drop the baseline to the measured 397 so the freeze is exact (cap = 397 + slack 0), the same hard-zero-at-the-boundary shape LIT005 and LIT007 already use and pass in CI. PR table row updated to 397 / 0 * fix: increase slack * fix: increase slack * docs(lint): align gate docstring with buffered LIT003/LIT004 slack The budget now gives LIT003/LIT004 nonzero slack, so the gate's prose no longer claims they are frozen at slack 0; LIT005 remains the reasonless- suppression freeze and LIT007 the hard zero.
157 lines
5.4 KiB
Python
157 lines
5.4 KiB
Python
#!/usr/bin/env python3
|
|
"""Non-gating ratchet guard: budget ceilings may only fall, never rise.
|
|
|
|
Every `*-budget.json` file (ruff-strict, type-discipline, mypy-code, basedpyright-code) is a
|
|
one-way ratchet: each rule's ceiling is `baseline + slack`, and the whole point is
|
|
to drive that number 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 ceiling 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 ceilings are fine.
|
|
|
|
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
|
|
from pathlib import Path
|
|
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",
|
|
"mypy-code-budget.json",
|
|
"basedpyright-code-budget.json",
|
|
)
|
|
|
|
|
|
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 _caps(budget: dict) -> dict[str, int]:
|
|
"""Map each rule to its ceiling (baseline + slack); skip malformed specs."""
|
|
caps: dict[str, int] = {}
|
|
for rule, spec in budget.items():
|
|
if isinstance(spec, dict):
|
|
caps[rule] = int(spec.get("baseline", 0)) + int(spec.get("slack", 0))
|
|
return caps
|
|
|
|
|
|
def regressions_for(rel: str, base: dict | None, head: dict | None) -> 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 ceiling removed)")]
|
|
|
|
base_caps = _caps(base)
|
|
head_caps = _caps(head)
|
|
out: list[Regression] = []
|
|
for rule, base_cap in sorted(base_caps.items()):
|
|
if rule not in head_caps:
|
|
out.append(Regression(rel, rule, f"rule dropped (ceiling {base_cap} -> removed)"))
|
|
elif head_caps[rule] > base_cap:
|
|
out.append(Regression(rel, rule, f"ceiling raised {base_cap} -> {head_caps[rule]}"))
|
|
return out
|
|
|
|
|
|
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))
|
|
|
|
if regressions:
|
|
print(f"FAIL: budget ceiling(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 ceiling increased vs base {args.base}{suffix}")
|
|
return 0
|
|
|
|
|
|
if __name__ == "__main__":
|
|
raise SystemExit(main())
|