From 0965a4d1f4350e6991aa00ce77ef8eddc655d5fe Mon Sep 17 00:00:00 2001 From: Mateo Wang <277851410+mateo-berri@users.noreply.github.com> Date: Tue, 30 Jun 2026 09:37:12 -0700 Subject: [PATCH] chore: shift CI lint left with an opt-in `make pre-commit` and CLAUDE.md rule (#31544) * chore: shift CI lint left with a pre-commit hook and CLAUDE.md rule Add an opt-in pre-commit hook (.githooks/pre-commit, active after make install-hooks) that runs the CI-equivalent checks against staged files: make lint for Python, prettier plus eslint for the dashboard, and a gen:api drift check for the proxy OpenAPI types. Document the same expectation in CLAUDE.md so reds surface locally instead of in CI. * fix: make `make lint` isomorphic to the CI lint job `make lint` diverged from test-linting.yml in ways that produced both false reds and false greens: its format-check ran over the whole repo (CI scopes it to changed files vs the base), its ruff-strict budget ran in absolute mode (CI runs it as a delta vs base), and it omitted the type-discipline gate entirely. Recompose `lint` to replay CI's exact sequence: diff-scoped ruff format check, whole-tree ruff check, the strict / type-discipline / basedpyright budgets as a delta resolved the same way CI resolves it (merge-base with origin/litellm_internal_staging), then circular-import and import-safety. Factor the repeated base fetch into one shared prerequisite so the chain hits the network once. Align the pre-commit hook's eslint invocation with the CI frontend-lint job (`--pass-on-unpruned-suppressions`) and fix the CLAUDE.md guidance to point at the diff-scoped frontend commands instead of the whole-folder npm scripts, which are broader than CI. * fix(githooks): make pre-commit 1:1 with CI frontend-lint, lint, and type-gen The shift-left pre-commit hook diverged from the CI jobs it claims to mirror, so a clean commit did not actually mean a green CI lint. The dashboard block only ran prettier and eslint over js/jsx/ts/tsx/mjs/cjs, but CI's frontend-lint runs prettier over a wider set (also json, css, scss, md, mdx, yml, yaml, html) and additionally gates the whole-folder eslint lint budgets via scripts/check-lint-budgets.mjs. The hook now mirrors that split and runs the budget check, so a dashboard commit that passes locally passes the job. The API-types block ran npm run gen:api without LITELLM_PYTHON, so it shelled out to the system python3 which has no litellm installed and always failed with a false 'could not regenerate API types' red. It now passes LITELLM_PYTHON="uv run --no-sync python" the way check-ui-api-types.yml does. make lint format-checks the files in origin/base...HEAD, which at pre-commit time predates the staged change, so a brand-new commit's formatting went unchecked. The Python block now also runs ruff format --check over the staged litellm files directly to cover that case, and its trigger is scoped to staged litellm/ files (the only tree CI's lint job inspects) so a tests-only or scripts-only commit skips the slow make lint instead of wasting time on a run that could not catch anything. CLAUDE.md's shift-left rule was cut off mid-sentence and understated the frontend checks; it now describes all three gates accurately and points agents at make install-hooks to run them automatically before each commit. Co-authored-by: Mateo Wang * fix(githooks): scope the API-types check to all of check-ui-api-types.yml's triggers spec_files was filtered from the staged Python files, so the gen:api drift check only fired for .py changes under litellm/proxy or litellm/types. CI's check-ui-api-types.yml triggers on any file under those directories (Prisma schema, configs) plus the generator script and the dashboard package files, so a non-Python proxy/types change could pass the hook and still fail CI. Match the workflow's full trigger set instead. * fix(pre-commit): run prisma generate before gen:api to mirror CI * refactor(githooks): run shift-left lint via on-demand make pre-commit, not an auto-firing hook The pre-commit hook ran make lint plus the dashboard eslint budgets, which are minutes of work (basedpyright over litellm/, a whole-folder eslint . pass at ~40s). Wiring that into core.hooksPath via make install-hooks meant every human commit, not just an agent's, paid that cost, which is real friction for interactive committers. Move the staged-file checks out of .githooks/ into scripts/pre_commit_lint.sh and expose them as make pre-commit, and keep .githooks/ to only the fast Conventional Commits / Branches hooks so make install-hooks no longer makes commits slow. Agents run make pre-commit right before each commit (CLAUDE.md instructs this), so the slow gates fire only for the commits an agent is making and never auto-fire for a human typing git commit. The script stays hook-compatible for anyone who still wants it to fire automatically via a symlink. Preferred this over sniffing an agent env var to auto-fire only for agents: that is fragile (misses agents when the var is unset, fires on humans when it leaks into their shell, and silently no-ops a hook a human deliberately installed), whereas an on-demand command achieves the same humans-never, agents-per-commit outcome deterministically. Co-authored-by: Mateo Wang * fix(pre-commit): run make lint last so it can't prune the proxy deps gen:api needs make lint's install-dev prerequisite runs uv sync --frozen, which prunes the proxy extras (prisma, websockets, ...) from the venv. With the Python block running first, the subsequent API-types block then failed: gen:api imports litellm.proxy.proxy_server, which needs those deps, so every litellm/proxy change (the main trigger for the API-types check) hit a false 'could not regenerate API types' red. Run the dashboard and API-types blocks before the Python block so gen:api sees an intact env; CI is unaffected because there the lint and check-ui-api-types jobs run in separate environments. Co-authored-by: Mateo Wang * fix: make CLAUDE.md more concise * fix(makefile): give make lint the CI lint env and stop it pruning the venv make lint diverged from test-linting.yml's lint job in two ways: it never generated the Prisma client (so basedpyright resolved the DB wrappers as Unknown, drifting from CI's counts), and its bare uv sync --frozen pruned the proxy extras (prisma, websockets, ...) out of the venv on every run, which broke the gen:api step that imports litellm.proxy.proxy_server and left a dev unable to run the proxy until re-syncing. Add a lint-install target that mirrors the job's environment (the proxy-dev group plus prisma generate) and runs before the checks, and make both it and install-dev use uv sync --inexact so they top up the venv instead of tearing packages out. CI is unaffected since it installs its own env per job. Because make lint no longer prunes, the pre-commit reorder that ran it last (to dodge the prune) is no longer needed, so restore the original block order. Co-authored-by: Mateo Wang * fix(makefile): drop lint-install so make lint matches CI's slimmer env test-linting.yml's lint job installs deps with a bare uv sync --frozen (default dev group only, no proxy-dev, no prisma generate), but the lint-install target chained into make lint pulled in --group proxy-dev and ran prisma generate. Because the basedpyright budget step compares head and base counts against fixed thresholds, the extra symbols and Prisma client locally resolved can shift error counts away from CI's, producing false greens or false reds on the type-check gate. Remove the lint-install target and its slot in lint. The remaining sub-targets already chain install-dev, which now uses uv sync --inexact --frozen, so the venv still isn't pruned but the installed set stays aligned with what CI sees. * ci(linting): install proxy-dev and generate prisma in lint job, matching make lint make lint now installs the proxy-dev group and generates the Prisma client so basedpyright resolves the DB wrappers; the lint job here still installed only the base env, so a local pre-commit could pass while the required CI lint failed (or vice versa). Bring this job in line, which is the same environment litellm_internal_staging's lint job already uses. Co-authored-by: Mateo Wang * fix(makefile): keep make lint on the proxy-dev + prisma env to match CI A concurrent change dropped lint-install to match what looked like CI's slim env, but test-linting.yml's lint job (and the merge ref this PR's CI actually runs) installs --group proxy-dev and generates the Prisma client. With make lint slim and CI fat, basedpyright resolves fewer symbols locally than CI, so a prisma-typed error can stay Unknown locally (green) while CI catches it (red). Restore lint-install so make lint installs the same env CI does; the previous commit also brought this PR's test-linting.yml in line with that env, so the two now match. Co-authored-by: Mateo Wang --------- Co-authored-by: Cursor Agent Co-authored-by: Mateo Wang --- CLAUDE.md | 4 +- Makefile | 62 ++++++++++++++++--- scripts/install_git_hooks.sh | 3 + scripts/pre_commit_lint.sh | 112 +++++++++++++++++++++++++++++++++++ 4 files changed, 171 insertions(+), 10 deletions(-) create mode 100755 scripts/pre_commit_lint.sh diff --git a/CLAUDE.md b/CLAUDE.md index eb32c2cd6da..cea38b8527b 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -34,7 +34,7 @@ If you ever make public-facing PR descriptions, comments, issues, commit message Don't hesitate to use values in .env to get needed API keys and other secrets, as long as you never add them to conversation history, commit them, or include them in GitHub issues / PRs -Run tests, format your code, and lint your code before each commit +Run tests before you commit. Also, run `make pre-commit` right before each commit, which generates types (as needed) and formats/lints your code. Any errors found must be fixed When you fix violations gated by `ruff-strict-budget.json` or `basedpyright-code-budget.json`, run `make lint-budget-update` and commit the lowered baselines so the ceilings ratchet down instead of leaving stale headroom @@ -42,7 +42,7 @@ If you're trying to create a new function that relies on untyped stuff, instead 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 -Ask to commit and push your work when you're done (or if you're confident that your code is good and works, just do it) +Commit and push your work when you're done without asking When you must use real LLM models to, for example, write e2e tests, write a QA runbook, etc., make sure to use the latest models (doesn't have to be smartest, can also be a modern small, fast one. No strong preference for smart vs fast here, just use something modern) as of the year and month of the current date. Do a web search as necessary to figure that out diff --git a/Makefile b/Makefile index 7701f54e15c..fb927148c80 100644 --- a/Makefile +++ b/Makefile @@ -8,7 +8,8 @@ lint-basedpyright lint-basedpyright-budget-update \ lint-ruff-budget lint-ruff-budget-update lint-budget-update lint-gate \ install-dev install-proxy-dev install-test-deps install-hooks \ - install-helm-unittest check-circular-imports check-import-safety + install-helm-unittest check-circular-imports check-import-safety pre-commit \ + lint-install lint-fetch-base # Default target help: @@ -20,6 +21,7 @@ help: @echo " make install-test-deps - Install the full local test environment" @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 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)" @@ -56,8 +58,11 @@ info: @echo "UV: $(UV)" # Installation targets +# --inexact: sync the locked deps without pruning anything already installed, so running +# a lint/format target doesn't tear the proxy extras (prisma, websockets, ...) out from +# under a dev's venv (CI installs its own env per job, so it is unaffected by this). install-dev: - $(UV) sync --frozen + $(UV) sync --inexact --frozen install-proxy-dev: $(UV) sync --frozen --group proxy-dev --extra proxy @@ -90,6 +95,31 @@ format: install-dev format-check: install-dev cd litellm && $(UV_RUN) ruff format --check --exclude '/enterprise/' . && cd .. +# Single fetch of the PR base so the delta-based gates below share one network round +# trip instead of each re-fetching when chained from `lint`. +lint-fetch-base: + git fetch origin litellm_internal_staging + +# Mirror test-linting.yml's lint job environment: the proxy-dev group plus a generated +# Prisma client, so basedpyright resolves the same modules CI does (without the generated +# client the DB wrappers typed against it degrade to Unknown, drifting the budget from +# CI's). --inexact tops up the venv instead of pruning the proxy extras gen:api and the +# running proxy need. +lint-install: + $(UV) sync --inexact --frozen --group proxy-dev + $(UV_RUN) prisma generate --schema litellm/proxy/schema.prisma + +# Diff-scoped format check, identical to test-linting.yml's "Check ruff format" step: +# only the litellm Python files changed vs the base are checked, so a pre-existing +# format issue elsewhere doesn't block an unrelated commit. +lint-format-check-changed: install-dev lint-fetch-base + @files=$$(git diff --name-only origin/litellm_internal_staging...HEAD -- 'litellm/**/*.py' | grep -v '^litellm/enterprise/' || true); \ + if [ -z "$$files" ]; then \ + echo "No changed litellm Python files to format-check."; \ + else \ + echo "$$files" | xargs $(UV_RUN) ruff format --check --exclude '/enterprise/'; \ + fi + # Linting targets lint-ruff: install-dev cd litellm && $(UV_RUN) ruff check . && cd .. @@ -126,10 +156,14 @@ lint-ruff-FULL-dev: install-dev if [ -n "$$files" ]; then echo "$$files" | xargs $(UV_RUN) ruff check; \ else echo "No changed .py files to check."; fi -lint-basedpyright: install-dev - git fetch origin litellm_internal_staging +lint-basedpyright: install-dev lint-fetch-base ($(UV_RUN) basedpyright --outputjson || true) | $(UV_RUN) python scripts/type_check_gate.py --base origin/litellm_internal_staging +# Type-discipline budget (mutable collections / casts / type guards / kwargs / +# unexplained suppressions), the test-linting.yml step `make lint` used to omit. +lint-type-discipline: install-dev lint-fetch-base + $(UV_RUN) python scripts/type_discipline_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 @@ -140,8 +174,7 @@ lint-ruff-budget: install-dev # Strict gate, invoked the same way CI does in test-linting.yml so a local pass # means the CI check will pass too. -lint-gate: install-dev - git fetch origin litellm_internal_staging +lint-gate: install-dev lint-fetch-base $(UV_RUN) python scripts/ruff_strict_gate.py --base origin/litellm_internal_staging lint-ruff-budget-update: install-dev @@ -156,12 +189,25 @@ check-circular-imports: install-dev check-import-safety: install-dev @$(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 (matches test-linting.yml workflow) -lint: format-check lint-ruff lint-basedpyright check-circular-imports check-import-safety lint-ruff-budget +# 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 +# does (merge-base with origin/litellm_internal_staging). lint-install is first so the +# Prisma client exists before basedpyright runs. +lint: lint-install lint-format-check-changed lint-ruff lint-gate lint-type-discipline lint-basedpyright 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 +# 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. +# Not auto-installed as a git hook so it never slows an unrelated human commit. +pre-commit: + ./scripts/pre_commit_lint.sh + # Testing targets test: install-test-deps $(UV_RUN) pytest tests/ diff --git a/scripts/install_git_hooks.sh b/scripts/install_git_hooks.sh index 1e4e3c6de19..7ea8c3ff2e9 100755 --- a/scripts/install_git_hooks.sh +++ b/scripts/install_git_hooks.sh @@ -34,5 +34,8 @@ cat < `make lint` (test-linting.yml's lint job) +# - 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: +# `ln -s ../../scripts/pre_commit_lint.sh .git/hooks/pre-commit`. + +set -eu + +repo_root=$(git rev-parse --show-toplevel) +cd "$repo_root" + +staged=$(git diff --cached --name-only --diff-filter=ACMR) +staged_match() { printf '%s\n' "$staged" | grep -E "$1" || true; } + +# CI's lint job (test-linting.yml) only inspects litellm/, so a tests-only or +# scripts-only commit can't turn it red; scope the trigger there to skip the slow +# make lint when it couldn't catch anything. +litellm_py_files=$(staged_match '^litellm/.*\.py$') +# ruff format (and CI's format step) skip enterprise; the rest of make lint covers it. +fmt_files=$(printf '%s\n' "$litellm_py_files" | grep -v '^litellm/enterprise/' || true) +# check-ui-api-types.yml triggers on any file under litellm/proxy or litellm/types +# (Prisma schema and configs included, not just Python) plus the generator and its +# lockfiles, so match that whole trigger set rather than a Python subset. +spec_files=$(staged_match '^(litellm/(proxy|types)/.*|ui/litellm-dashboard/(scripts/gen-api-types\.mjs|package\.json|package-lock\.json|src/lib/http/schema\.d\.ts))$') +# CI's frontend-lint runs prettier over a wider extension set than eslint; keep that +# split so this flags exactly what the job would. +ui_prettier_files=$(staged_match '^ui/litellm-dashboard/.*\.(js|jsx|ts|tsx|mjs|cjs|json|css|scss|md|mdx|yml|yaml|html)$') +ui_eslint_files=$(staged_match '^ui/litellm-dashboard/.*\.(js|jsx|ts|tsx|mjs|cjs)$') + +lint_dashboard() { + ( + rc=0 + prettier_rel=() + eslint_rel=() + while IFS= read -r f; do + [ -n "$f" ] && prettier_rel+=("${f#ui/litellm-dashboard/}") + done <&2; status=1; } + # `make lint` format-checks files in origin/base...HEAD, which at pre-commit time + # predates the staged change, so format-check the staged litellm files directly to + # cover a brand-new commit before it lands. + if [ -n "$fmt_files" ]; then + echo "pre-commit: ruff format --check (staged litellm files)" + printf '%s\n' "$fmt_files" | xargs uv run --no-sync ruff format --check --exclude '/enterprise/' \ + || { echo "✗ Unformatted staged files. Fix with: make format, then re-stage." >&2; status=1; } + fi +fi + +if [ -n "$ui_prettier_files" ] || [ -n "$ui_eslint_files" ]; then + echo "pre-commit: linting dashboard (prettier + eslint + lint budgets)" + lint_dashboard || { echo "✗ Dashboard lint failed. See above; format with: (cd ui/litellm-dashboard && npm run format)." >&2; status=1; } +fi + +if [ -n "$spec_files" ]; then + echo "pre-commit: checking dashboard API types are in sync (npm run gen:api)" + # gen-api-types.mjs imports litellm.proxy.proxy_server, which needs the proxy deps + # and an up-to-date Prisma client; check-ui-api-types.yml installs those and runs + # prisma generate before gen:api, so mirror that here or a stale client can mask + # drift that CI will still flag. + if ! uv run --no-sync prisma generate --schema litellm/proxy/schema.prisma; then + echo "✗ Could not regenerate Prisma client (prisma generate failed)." >&2 + status=1 + elif ( cd ui/litellm-dashboard && LITELLM_PYTHON="uv run --no-sync python" npm run gen:api ); then + if ! git diff --quiet -- ui/litellm-dashboard/src/lib/http/schema.d.ts; then + echo "✗ Dashboard API types are stale; regenerated src/lib/http/schema.d.ts. Stage it and re-run make pre-commit." >&2 + status=1 + fi + else + echo "✗ Could not regenerate API types (npm run gen:api failed)." >&2 + status=1 + fi +fi + +exit $status