build: move basedpyright onto a dedicated make typecheck target

basedpyright dominates the wall clock of `make lint`, and so of
`make pre-commit`: the ruff, budget, circular-import and import-safety
checks finish in seconds while the two basedpyright passes take minutes,
so every commit paid for a type check even when nothing about types
changed.

`make typecheck` now owns both passes, the litellm/ budget gate and the
zero-error tests/e2e pass, and reuses the same setup `lint` does (proxy-dev
sync, generated Prisma client, base fetch) so import resolution still
matches CI. `make lint` and `make pre-commit` keep everything else and both
point at `make typecheck` on the way out, so a clean run is not mistaken
for a green CI lint. CI is untouched: its workflows call the gate scripts
directly, so `make lint && make typecheck` is still the parity pair.

While splitting the pre-commit script, its scratch files moved into a
single mktemp -d created before any job starts. Cleanup on Ctrl-C was a
per-variable `rm -f`, which leaked whichever temp file was created but not
yet assigned when the signal landed, plus the dashboard eslint report,
which nothing cleaned on interrupt at all.
This commit is contained in:
mateo-berri 2026-08-06 09:26:45 +00:00
parent a0e627f99d
commit ec3507b57d
No known key found for this signature in database
8 changed files with 195 additions and 41 deletions

View file

@ -43,6 +43,8 @@ When you fix violations gated by `ruff-strict-budget.json`, `type-discipline-bud
`make pre-commit` always saves its complete output to a per-worktree log file and prints that path as its first and last output lines. To inspect a run, read or grep that log instead of re-running the multi-minute checks just to see a different slice, and re-run only after the working tree actually changed
`make pre-commit` runs every gating CI check except basedpyright, which lives on `make typecheck` (both the `litellm/` budget gate and the zero-error `tests/e2e` pass). Run `make typecheck` too whenever you touch Python, otherwise CI can still come back red on types
If you're trying to create a new function that relies on untyped stuff, instead of adding more Any's and pushing `reportAny` / `reportExplicitAny` closer to their basedpyright ceilings, just validate it in the caller with Pydantic (a model or `TypeAdapter` that returns the typed thing or raises will do) and then pass the now typed variable in
If you get an LIT001 or LIT002 fail, refactor the code to follow functional programming best practices rather than introducing mutable data structures. For example, build values in one shot with comprehensions or generators wrapped in `tuple()` / `frozenset()` instead of seeding an empty `list`/`dict`/`set` and mutating it over time. Ideally, `# mutable-ok` is never used; reach for it only as a genuine last resort when an immutable rewrite is truly impossible, and always pair it with a real reason

View file

@ -14,7 +14,7 @@ Here are the core requirements for any PR submitted to LiteLLM:
- [ ] **Add testing** - Adding at least 1 test is a hard requirement - [see details](#adding-testing)
- [ ] **Ensure your PR passes all checks**:
- [ ] [Unit Tests](#running-unit-tests) - `make test-unit`
- [ ] [Linting / Formatting](#running-linting-and-formatting-checks) - `make lint`
- [ ] [Linting / Formatting](#running-linting-and-formatting-checks) - `make lint` and `make typecheck`
#### UI PRs
@ -70,6 +70,7 @@ make format
# Run all linting checks (matches CI exactly)
make lint
make typecheck
# Run unit tests to ensure nothing is broken
make test-unit
@ -144,17 +145,19 @@ uv run pytest tests/test_litellm/test_your_file.py -v
### Running Linting and Formatting Checks
Run all linting checks (matches CI exactly):
Run all linting checks (matches CI exactly). `make lint` covers everything except type checking, which is slower and lives on its own target:
```bash
make lint
make typecheck
```
Individual linting commands:
```bash
make format-check # Check Black formatting
make lint-ruff # Run Ruff linting
make lint-basedpyright # Run basedpyright type checking
make lint-basedpyright # Run basedpyright over litellm/
make lint-e2e-basedpyright # Run basedpyright over tests/e2e
make check-circular-imports # Check for circular imports
make check-import-safety # Check import safety
```
@ -184,6 +187,7 @@ To ensure your changes will pass CI, run the exact same checks locally:
```bash
# This runs the same checks as the GitHub workflows
make lint
make typecheck
make test-unit
```
@ -203,7 +207,8 @@ make install-proxy-dev # Install proxy development dependencies
make install-test-deps # Install the full local test environment
make format # Apply Black code formatting
make format-check # Check Black formatting (matches CI)
make lint # Run all linting checks
make lint # Run all linting checks except type checking
make typecheck # Run basedpyright (litellm/ budget + tests/e2e)
make test-unit # Run unit tests
make test-integration # Run integration tests
make test-unit-helm # Run Helm unit tests
@ -226,7 +231,7 @@ All checks must pass before your PR can be merged.
### 1. Linting Failures
If `make lint` fails:
If `make lint` or `make typecheck` fails:
1. **Formatting issues**: Run `make format` to auto-fix
2. **Ruff issues**: Check the output and fix manually

View file

@ -4,7 +4,7 @@
.PHONY: help test test-unit test-unit-llms test-unit-proxy-guardrails test-unit-proxy-core test-unit-proxy-misc \
test-unit-integrations test-unit-core-utils test-unit-other test-unit-root \
test-proxy-unit-a test-proxy-unit-b test-integration test-unit-helm \
info lint lint-dev lint-checks format \
info lint lint-dev lint-checks format typecheck typecheck-checks \
lint-basedpyright lint-e2e-basedpyright lint-basedpyright-budget-update lint-type-discipline lint-type-discipline-budget-update \
lint-ruff-budget lint-ruff-budget-update lint-budget-update lint-gate \
install-dev install-proxy-dev install-test-deps install-hooks \
@ -23,9 +23,10 @@ help:
@echo " make install-helm-unittest - Install helm unittest plugin"
@echo " make install-hooks - Install git hooks (Conventional Commits + Branches)"
@echo " make pre-commit - Run CI-equivalent lint on staged files (run before committing)"
@echo " make typecheck - Run every basedpyright pass (litellm budget + tests/e2e)"
@echo " make format - Apply ruff format code formatting"
@echo " make format-check - Check ruff format code formatting (matches CI)"
@echo " make lint - Run all linting (Ruff, basedpyright, format check, circular imports, import safety)"
@echo " make lint - Run all non-basedpyright linting (Ruff, format check, budgets, circular imports, import safety)"
@echo " make lint-ruff - Run Ruff linting only"
@echo " make lint-basedpyright - Run basedpyright strict, gated by per-rule error counts"
@echo " make lint-e2e-basedpyright - Run basedpyright over tests/e2e (zero errors allowed)"
@ -220,18 +221,30 @@ check-circular-imports: $(LINT_DEP_INSTALL)
check-import-safety: $(LINT_DEP_INSTALL)
@$(UV_RUN) python -c "from litellm import *; print('[from litellm import *] OK! no issues!');" || (echo '🚨 import failed, this means you introduced unprotected imports! 🚨'; exit 1)
# Combined linting, isomorphic to test-linting.yml's lint job so a local pass means a
# green CI lint: it installs the same env (proxy-dev + generated Prisma client) and then
# runs the diff-scoped ruff format check, whole-tree ruff check, the strict-rule /
# type-discipline / basedpyright budgets as a delta vs the base, then the circular-import
# and import-safety checks. Steps that compare against the base resolve it the same way CI
# Every basedpyright pass test-linting.yml runs, split out of `lint` (and so out of
# `make pre-commit`) because it dominates the wall clock: the rest of the lint job
# finishes in seconds while basedpyright takes minutes, and the two answer different
# questions, so a dev fixing a ruff nit shouldn't have to sit through a type check.
# `make lint && make typecheck` is the CI-parity pair. Setup runs once here, then a
# sub-make fans the two passes out with the dep vars emptied, same as `lint`.
typecheck: lint-install lint-fetch-base
$(MAKE) -j $(LINT_JOBS) $(LINT_OUTPUT_SYNC) LINT_DEP_INSTALL= LINT_E2E_DEP_INSTALL= LINT_DEP_BASE= typecheck-checks
typecheck-checks: lint-basedpyright lint-e2e-basedpyright
# Combined linting: test-linting.yml's lint job minus its basedpyright steps (those are
# `make typecheck`). It installs the same env (proxy-dev + generated Prisma client) and
# then runs the diff-scoped ruff format check, whole-tree ruff check, the strict-rule and
# type-discipline budgets as a delta vs the base, then the circular-import and
# import-safety checks. Steps that compare against the base resolve it the same way CI
# does (merge-base with origin/litellm_internal_staging). Setup (env sync, Prisma client,
# base fetch) runs once up front; the checks themselves are independent, so a sub-make
# fans them out with -j and the fast ones finish under basedpyright's shadow.
# fans them out with -j.
lint: lint-install lint-fetch-base
$(MAKE) -j $(LINT_JOBS) $(LINT_OUTPUT_SYNC) LINT_DEP_INSTALL= LINT_E2E_DEP_INSTALL= LINT_DEP_BASE= lint-checks
@echo "lint: clean. basedpyright is not part of this target; run 'make typecheck' for it."
lint-checks: lint-format-check-changed lint-ruff lint-gate lint-type-discipline lint-basedpyright lint-e2e-basedpyright check-circular-imports check-import-safety
lint-checks: lint-format-check-changed lint-ruff lint-gate lint-type-discipline check-circular-imports check-import-safety
# Faster linting for local development (only checks changed code)
lint-dev: lint-format-changed check-circular-imports check-import-safety
@ -239,6 +252,7 @@ lint-dev: lint-format-changed check-circular-imports check-import-safety
# Run the gating CI checks against your staged files right before committing. Mirrors
# test-linting.yml (Python), test-litellm-ui-build.yml's frontend-lint (dashboard), and
# check-ui-api-types.yml (API-type drift), skipping any whose files you didn't stage.
# basedpyright is the one CI check it leaves out; run `make typecheck` for that.
# Not auto-installed as a git hook so it never slows an unrelated human commit.
pre-commit: bootstrap
./scripts/pre_commit_lint.sh

View file

@ -614,7 +614,8 @@ git clone https://github.com/BerriAI/litellm.git
cd litellm
make install-dev # Install development dependencies
make format # Format your code
make lint # Run all linting checks
make lint # Run all linting checks except type checking
make typecheck # Run basedpyright type checking
make test-unit # Run unit tests
make format-check # Check formatting only
```

View file

@ -2,17 +2,19 @@
#
# pre_commit_lint.sh — shift CI lint left. Run it (via `make pre-commit`) right
# before `git commit`; it inspects your staged files and runs only the matching
# gating CI checks, so a clean run means a green CI lint:
# - litellm/ Python staged -> `make lint` (test-linting.yml's lint job)
# - tests/e2e Python staged -> `make lint-e2e-basedpyright` (test-linting.yml's e2e type-check step)
# + raw HTTP client ban (test-code-quality.yml's check_e2e_no_raw_requests)
# gating CI checks, so a clean run plus a clean `make typecheck` means a green CI lint:
# - litellm/ Python staged -> `make lint` (test-linting.yml's lint job, minus basedpyright)
# - tests/e2e Python staged -> raw HTTP client ban (test-code-quality.yml's check_e2e_no_raw_requests)
# - dashboard staged -> prettier + eslint + lint budgets (test-litellm-ui-build.yml's frontend-lint)
# - proxy/types staged -> regenerate dashboard API types and fail on drift (check-ui-api-types.yml)
#
# Each block is skipped when no matching files are staged, so unrelated commits stay
# fast. This is intentionally not auto-installed as a git hook (see scripts/install_git_hooks.sh):
# the dashboard and basedpyright passes can take minutes, so it's run on demand rather
# than firing on every human commit. It is hook-compatible if you want that anyway:
# CI's two basedpyright steps are deliberately not here: they take minutes while
# everything above takes seconds, so they live in `make typecheck`, which this script
# reminds you to run when you stage Python. Each block is skipped when no matching files
# are staged, so unrelated commits stay fast. This is intentionally not auto-installed as
# a git hook (see scripts/install_git_hooks.sh): the dashboard pass can still take a
# while, so it's run on demand rather than firing on every human commit. It is
# hook-compatible if you want that anyway:
# `ln -s ../../scripts/pre_commit_lint.sh .git/hooks/pre-commit`.
set -eu
@ -97,10 +99,9 @@ EOF
# Whole-folder lint budgets, exactly as the frontend-lint job runs them: the
# counts are not diff-scoped, so a local pass here means the budget step will
# pass in CI too.
report=$(mktemp)
report="$work_dir/eslint-report.json"
npx eslint . -f json -o "$report" || true
node scripts/check-lint-budgets.mjs "$report" eslint-budgets.json || rc=1
rm -f "$report"
exit $rc
)
}
@ -127,29 +128,28 @@ python_checks() {
return $rc
}
# Every scratch file lives in one directory created before any job starts, so an
# interrupt can't leak one: cleanup is a single rm -rf that doesn't depend on which
# per-file variable happened to be assigned when the signal landed.
on_interrupt() {
trap - INT TERM
rm -f "${python_log:-}" "${dash_log:-}" "${gen_log:-}"
for job_pid in ${python_pid:-} ${dash_pid:-} ${gen_pid:-}; do
kill -- "-$job_pid" 2>/dev/null || true
done
rm -rf "${work_dir:-}"
exit 130
}
trap on_interrupt INT TERM
work_dir=$(mktemp -d)
if [ -n "$litellm_py_files" ]; then
python_log=$(mktemp)
python_log="$work_dir/python.log"
set -m
python_checks > "$python_log" 2>&1 &
python_pid=$!
set +m
fi
if [ -n "$e2e_py_files" ] && [ -z "$litellm_py_files" ]; then
echo "pre-commit: type-checking tests/e2e (make lint-e2e-basedpyright)"
make lint-e2e-basedpyright || { echo "✗ tests/e2e basedpyright failed. Fix the errors above, then re-run make pre-commit." >&2; status=1; }
fi
if [ -n "$e2e_py_files" ]; then
echo "pre-commit: checking tests/e2e raw HTTP client ban (check_e2e_no_raw_requests)"
uv run --no-sync python tests/code_coverage_tests/check_e2e_no_raw_requests.py \
@ -167,7 +167,7 @@ dashboard_checks() {
}
if [ -n "$ui_prettier_files" ] || [ -n "$ui_eslint_files" ]; then
dash_log=$(mktemp)
dash_log="$work_dir/dashboard.log"
set -m
dashboard_checks > "$dash_log" 2>&1 &
dash_pid=$!
@ -205,7 +205,7 @@ genapi_checks() {
}
if [ -n "$spec_files" ]; then
gen_log=$(mktemp)
gen_log="$work_dir/genapi.log"
set -m
genapi_checks > "$gen_log" 2>&1 &
gen_pid=$!
@ -214,15 +214,20 @@ fi
if [ -n "${python_pid:-}" ]; then
wait "$python_pid" || status=1
cat "$python_log"; rm -f "$python_log"
cat "$python_log"
fi
if [ -n "${dash_pid:-}" ]; then
wait "$dash_pid" || status=1
cat "$dash_log"; rm -f "$dash_log"
cat "$dash_log"
fi
if [ -n "${gen_pid:-}" ]; then
wait "$gen_pid" || status=1
cat "$gen_log"; rm -f "$gen_log"
cat "$gen_log"
fi
rm -rf "$work_dir"
if [ -n "$litellm_py_files" ] || [ -n "$e2e_py_files" ]; then
echo "pre-commit: basedpyright is not part of this run; CI still type-checks. Run: make typecheck"
fi
exit $status

View file

@ -138,7 +138,7 @@ Mark live tests with `@pytest.mark.e2e` (on the class or the module). Pure cover
Before you push
1. Run `make lint-e2e-basedpyright` (or `make pre-commit` with your changes staged); the harness is fully typed and the gate allows zero basedpyright errors, enforced in CI on any PR touching `tests/e2e/**/*.py`
1. Run `make typecheck` (or `make lint-e2e-basedpyright` for just this tree); the harness is fully typed and the gate allows zero basedpyright errors, enforced in CI on any PR touching `tests/e2e/**/*.py`. `make pre-commit` covers the rest of the CI lint, but not basedpyright
2. Add the models your test needs to the config your local proxy loads

View file

@ -0,0 +1,55 @@
import subprocess
from pathlib import Path
import pytest
ROOT = Path(__file__).resolve().parents[2]
BASEDPYRIGHT_COMMANDS = (
"uv run --no-sync python scripts/type_check_gate.py --base origin/litellm_internal_staging",
"uv run --no-sync basedpyright tests/e2e",
)
def _dry_run(target: str) -> str:
proc = subprocess.run(
["make", "--dry-run", target],
cwd=ROOT,
capture_output=True,
text=True,
timeout=120,
)
assert proc.returncode == 0, proc.stdout + proc.stderr
return proc.stdout
@pytest.mark.parametrize("command", BASEDPYRIGHT_COMMANDS)
def test_typecheck_runs_every_basedpyright_pass(command: str) -> None:
assert command in _dry_run("typecheck")
def test_typecheck_provisions_the_environment_basedpyright_resolves_against() -> None:
output = _dry_run("typecheck")
assert "uv sync --inexact --frozen --group proxy-dev --group e2e-dev" in output
assert "scripts/prisma_generate_if_needed.py" in output
assert "git fetch origin litellm_internal_staging" in output
@pytest.mark.parametrize("command", BASEDPYRIGHT_COMMANDS)
def test_lint_leaves_basedpyright_to_typecheck(command: str) -> None:
assert command not in _dry_run("lint")
@pytest.mark.parametrize(
"command",
[
"ruff format --check",
"ruff check .",
"scripts/ruff_strict_gate.py --base origin/litellm_internal_staging",
"scripts/type_discipline_gate.py --base origin/litellm_internal_staging",
"tests/documentation_tests/test_circular_imports.py",
"from litellm import *",
],
)
def test_lint_still_runs_the_other_gating_ci_checks(command: str) -> None:
assert command in _dry_run("lint")

View file

@ -29,6 +29,7 @@ BARRIER_HELPER = """barrier_sync() {
MAKE_STUB = """#!/bin/sh
. "$STUB_BIN/barrier.sh"
echo "$*" >> "$STUB_CALLS"
case "$*" in
lint)
[ "${STUB_FAIL:-}" = "make-lint" ] && exit 1
@ -52,6 +53,12 @@ case "$*" in
"eslint --no-warn-ignored"*)
[ "${STUB_FAIL:-}" = "eslint" ] && exit 1
;;
"eslint . -f json"*)
if [ -n "${STUB_HANG_ESLINT_DIR:-}" ]; then
touch "$STUB_HANG_ESLINT_DIR/eslint.started"
sleep 60
fi
;;
esac
exit 0
"""
@ -85,11 +92,15 @@ def _write_executable(path: Path, body: str) -> None:
path.chmod(0o755)
def _sandbox(tmp_path: Path) -> tuple[Path, Path]:
def _sandbox(tmp_path: Path, *, litellm_python: bool = True, e2e_python: bool = False) -> tuple[Path, Path]:
repo = tmp_path / "repo"
(repo / "litellm" / "proxy").mkdir(parents=True)
(repo / "litellm" / "foo.py").write_text("x = 1\n")
(repo / "litellm" / "proxy" / "spec.py").write_text("y = 2\n")
if litellm_python:
(repo / "litellm" / "foo.py").write_text("x = 1\n")
(repo / "litellm" / "proxy" / "spec.py").write_text("y = 2\n")
if e2e_python:
(repo / "tests" / "e2e").mkdir(parents=True)
(repo / "tests" / "e2e" / "test_flow.py").write_text("z = 3\n")
dashboard = repo / "ui" / "litellm-dashboard"
(dashboard / "src").mkdir(parents=True)
(dashboard / "node_modules").mkdir()
@ -113,10 +124,22 @@ def _env(repo: Path, bin_dir: Path, extra_env: dict[str, str]) -> dict[str, str]
"PATH": os.pathsep.join([str(bin_dir), "/usr/bin", "/bin"]),
"HOME": str(repo.parent),
"STUB_BIN": str(bin_dir),
"STUB_CALLS": str(_calls_file(bin_dir)),
**extra_env,
}
def _calls_file(bin_dir: Path) -> Path:
return bin_dir.parent / "make_calls.log"
def _make_calls(bin_dir: Path) -> list[str]:
calls = _calls_file(bin_dir)
if not calls.exists():
return []
return [line for line in calls.read_text().splitlines() if line]
def _run(repo: Path, bin_dir: Path, extra_env: dict[str, str]) -> subprocess.CompletedProcess[str]:
env = _env(repo, bin_dir, extra_env)
return subprocess.run(
@ -142,6 +165,30 @@ def test_python_dashboard_and_gen_api_blocks_run_concurrently_with_grouped_outpu
assert python_at < dashboard_at < gen_api_at
def test_staged_litellm_python_lints_without_type_checking_and_points_at_typecheck(tmp_path: Path) -> None:
repo, bin_dir = _sandbox(tmp_path)
proc = _run(repo, bin_dir, {})
assert proc.returncode == 0, proc.stdout + proc.stderr
assert _make_calls(bin_dir) == ["lint"]
assert "make typecheck" in proc.stdout
def test_staged_e2e_python_skips_basedpyright_but_keeps_the_raw_http_ban(tmp_path: Path) -> None:
repo, bin_dir = _sandbox(tmp_path, litellm_python=False, e2e_python=True)
proc = _run(repo, bin_dir, {})
assert proc.returncode == 0, proc.stdout + proc.stderr
assert _make_calls(bin_dir) == []
assert "raw HTTP client ban" in proc.stdout
assert "make typecheck" in proc.stdout
def test_commit_without_python_gets_no_typecheck_reminder(tmp_path: Path) -> None:
repo, bin_dir = _sandbox(tmp_path, litellm_python=False)
proc = _run(repo, bin_dir, {})
assert proc.returncode == 0, proc.stdout + proc.stderr
assert "typecheck" not in proc.stdout
def test_all_blocks_passing_exits_zero(tmp_path: Path) -> None:
repo, bin_dir = _sandbox(tmp_path)
proc = _run(repo, bin_dir, {})
@ -227,6 +274,31 @@ def test_interrupt_kills_background_jobs_and_removes_logs(tmp_path: Path) -> Non
os.killpg(proc.pid, signal.SIGTERM)
def test_interrupt_mid_dashboard_leaves_no_scratch_files_behind(tmp_path: Path) -> None:
repo, bin_dir = _sandbox(tmp_path, litellm_python=False)
hang_dir = tmp_path / "hang"
hang_dir.mkdir()
tmp_dir = tmp_path / "tmpdir"
tmp_dir.mkdir()
extra = {"STUB_HANG_ESLINT_DIR": str(hang_dir), "TMPDIR": str(tmp_dir)}
proc = subprocess.Popen(
[str(SCRIPT)],
cwd=repo,
env=_env(repo, bin_dir, extra),
stdout=subprocess.DEVNULL,
stderr=subprocess.DEVNULL,
start_new_session=True,
)
try:
assert _wait_until((hang_dir / "eslint.started").exists, 10)
os.killpg(proc.pid, signal.SIGINT)
assert proc.wait(timeout=10) != 0
assert _wait_until(lambda: list(tmp_dir.iterdir()) == [], 5), list(tmp_dir.iterdir())
finally:
with suppress(ProcessLookupError, PermissionError):
os.killpg(proc.pid, signal.SIGTERM)
def test_interrupt_spares_the_invoking_process(tmp_path: Path) -> None:
repo, bin_dir = _sandbox(tmp_path)
hang_dir = tmp_path / "hang"