Skip to content

build(makefile): run bootstrap before pre-commit lint - #35547

Merged
mateo-berri merged 1 commit into
litellm_internal_stagingfrom
litellm_precommit_runs_bootstrap
Aug 2, 2026
Merged

build(makefile): run bootstrap before pre-commit lint#35547
mateo-berri merged 1 commit into
litellm_internal_stagingfrom
litellm_precommit_runs_bootstrap

Conversation

@mateo-berri

Copy link
Copy Markdown
Contributor

TLDR

Problem this solves:

  • make pre-commit assumes deps are already provisioned
  • stale venv, prisma client, or dashboard deps cause confusing lint failures

How it solves it:

Relevant issues

Linear ticket

Pre-Submission checklist

Please complete all items before asking a LiteLLM maintainer to review your PR

  • I have added meaningful tests
  • My PR passes all CI/CD checks (e.g., lint, format, unit tests)
  • My PR's scope is as isolated as possible; it only solves 1 specific problem
  • I have received a Greptile Confidence Score of at least 4/5 before requesting a maintainer review (Greptile reviews automatically once the PR is opened; only comment @greptileai to re-request a review after pushing changes)

Delays in PR merge?

If you're seeing a delay in your PR being merged, ping the LiteLLM Team on Slack (#pr-review).

Screenshots / Proof of Fix

Before, at base b1fd20f: make pre-commit goes straight to the lint script, so it inherits whatever state the venv and node_modules happen to be in

$ time make pre-commit
./scripts/pre_commit_lint.sh
make pre-commit  0.12s user 0.38s system 72% cpu 0.684 total

After, at 28d56ef: bootstrap runs first and a warm run adds about 1.5 seconds before the lint script starts

$ time make pre-commit
uv sync --inexact --frozen --extra proxy --group proxy-dev --group e2e-dev
Audited 228 packages in 51ms
uv run --no-sync python scripts/prisma_generate_if_needed.py
Prisma client already generated for litellm/proxy/schema.prisma (prisma 0.11.0); skipping prisma generate
cd ui/litellm-dashboard && npm install --no-audit --no-fund

up to date in 519ms
bootstrap: .env left untouched
bootstrap: done
./scripts/pre_commit_lint.sh
make pre-commit  0.78s user 0.60s system 88% cpu 1.561 total

Type

🚄 Infrastructure

Changes

pre-commit gains bootstrap as a make prerequisite (both are already .PHONY), so the environment is synced right before the gating lint checks run. Since #35509 made the dashboard install incremental, the fully warm case costs about 1.5 seconds, dominated by npm confirming node_modules is up to date; when deps actually drifted, that is exactly the case where you want bootstrap to run before linting

@greptile-apps

greptile-apps Bot commented Aug 2, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR makes bootstrap a prerequisite of pre-commit, ensuring Python, Prisma, and dashboard dependencies are provisioned before staged-file checks run.

  • Runs the existing bootstrap workflow before scripts/pre_commit_lint.sh.
  • Leaves the lint script and bootstrap implementation unchanged.

Confidence Score: 5/5

The PR appears safe to merge, with the added provisioning behavior matching the documented purpose of the pre-commit workflow.

The change reuses the existing bootstrap target before linting, and the resulting setup work and warm-run overhead are intentional and documented without exposing a concrete broken caller.

Important Files Changed

Filename Overview
Makefile Adds the existing bootstrap target as a prerequisite of pre-commit; no actionable regression was identified.

Reviews (1): Last reviewed commit: "build(makefile): run bootstrap before pr..." | Re-trigger Greptile

@codecov

codecov Bot commented Aug 2, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@mateo-berri
mateo-berri merged commit 35ec1df into litellm_internal_staging Aug 2, 2026
76 checks passed
@mateo-berri
mateo-berri deleted the litellm_precommit_runs_bootstrap branch August 2, 2026 00:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants