From 2f36625e7f53e508c94a72a5acaa96051bf2a1a7 Mon Sep 17 00:00:00 2001 From: mateo-berri <277851410+mateo-berri@users.noreply.github.com> Date: Tue, 4 Aug 2026 21:18:00 -0700 Subject: [PATCH 1/3] perf(pre-commit): run python, dashboard, and gen-api checks concurrently --- scripts/pre_commit_lint.sh | 50 +++++-- tests/test_litellm/test_pre_commit_lint.py | 150 +++++++++++++++++++++ 2 files changed, 192 insertions(+), 8 deletions(-) create mode 100644 tests/test_litellm/test_pre_commit_lint.py diff --git a/scripts/pre_commit_lint.sh b/scripts/pre_commit_lint.sh index 8e148be399f..bc3cdec61dc 100755 --- a/scripts/pre_commit_lint.sh +++ b/scripts/pre_commit_lint.sh @@ -95,17 +95,25 @@ bootstrap_hint() { echo " Fix: make bootstrap" >&2 } -if [ -n "$litellm_py_files" ]; then +python_checks() { + local rc=0 echo "pre-commit: linting Python (make lint)" - make lint || { echo "✗ Python lint failed. Fix the reds above, then re-run make pre-commit." >&2; status=1; } + make lint || { echo "✗ Python lint failed. Fix the reds above, then re-run make pre-commit." >&2; rc=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; } + || { echo "✗ Unformatted staged files. Fix with: make format, then re-stage." >&2; rc=1; } fi + return $rc +} + +if [ -n "$litellm_py_files" ]; then + python_log=$(mktemp) + python_checks > "$python_log" 2>&1 & + python_pid=$! fi if [ -n "$e2e_py_files" ] && [ -z "$litellm_py_files" ]; then @@ -119,18 +127,24 @@ if [ -n "$e2e_py_files" ]; then || { echo "✗ Raw HTTP client import in tests/e2e. Route the call through tests/e2e/e2e_http.py, then re-run make pre-commit." >&2; status=1; } fi -if [ -n "$ui_prettier_files" ] || [ -n "$ui_eslint_files" ]; then +dashboard_checks() { echo "pre-commit: linting dashboard (prettier + eslint + lint budgets)" if [ ! -d ui/litellm-dashboard/node_modules ]; then echo "✗ ui/litellm-dashboard/node_modules is missing; dashboard lint cannot run." >&2 bootstrap_hint - status=1 - else - lint_dashboard || { echo "✗ Dashboard lint failed. See above; format with: (cd ui/litellm-dashboard && npm run format)." >&2; status=1; } + return 1 fi + lint_dashboard || { echo "✗ Dashboard lint failed. See above; format with: (cd ui/litellm-dashboard && npm run format)." >&2; return 1; } +} + +if [ -n "$ui_prettier_files" ] || [ -n "$ui_eslint_files" ]; then + dash_log=$(mktemp) + dashboard_checks > "$dash_log" 2>&1 & + dash_pid=$! fi -if [ -n "$spec_files" ]; then +genapi_checks() { + local status=0 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 @@ -156,6 +170,26 @@ if [ -n "$spec_files" ]; then echo "✗ Could not regenerate API types (npm run gen:api failed)." >&2 status=1 fi + return $status +} + +if [ -n "$spec_files" ]; then + gen_log=$(mktemp) + genapi_checks > "$gen_log" 2>&1 & + gen_pid=$! +fi + +if [ -n "${python_pid:-}" ]; then + wait "$python_pid" || status=1 + cat "$python_log"; rm -f "$python_log" +fi +if [ -n "${dash_pid:-}" ]; then + wait "$dash_pid" || status=1 + cat "$dash_log"; rm -f "$dash_log" +fi +if [ -n "${gen_pid:-}" ]; then + wait "$gen_pid" || status=1 + cat "$gen_log"; rm -f "$gen_log" fi exit $status diff --git a/tests/test_litellm/test_pre_commit_lint.py b/tests/test_litellm/test_pre_commit_lint.py new file mode 100644 index 00000000000..f77b3b25062 --- /dev/null +++ b/tests/test_litellm/test_pre_commit_lint.py @@ -0,0 +1,150 @@ +import os +import subprocess +from pathlib import Path + +import pytest + +ROOT = Path(__file__).resolve().parents[2] +SCRIPT = ROOT / "scripts" / "pre_commit_lint.sh" + +BARRIER_HELPER = """barrier_sync() { + touch "$STUB_BARRIER_DIR/$1.started" + for other in $2; do + tries=0 + while [ ! -f "$STUB_BARRIER_DIR/$other.started" ]; do + tries=$((tries + 1)) + if [ "$tries" -gt 100 ]; then + echo "barrier timeout: $1 never saw $other start" >&2 + exit 1 + fi + sleep 0.1 + done + done +} +""" + +MAKE_STUB = """#!/bin/sh +. "$STUB_BIN/barrier.sh" +case "$*" in + lint) + [ "${STUB_FAIL:-}" = "make-lint" ] && exit 1 + [ -n "${STUB_BARRIER_DIR:-}" ] && barrier_sync python "dashboard genapi" + ;; +esac +exit 0 +""" + +NPX_STUB = """#!/bin/sh +. "$STUB_BIN/barrier.sh" +case "$*" in + prettier*) + [ -n "${STUB_BARRIER_DIR:-}" ] && barrier_sync dashboard "python genapi" + ;; + "eslint --no-warn-ignored"*) + [ "${STUB_FAIL:-}" = "eslint" ] && exit 1 + ;; +esac +exit 0 +""" + +UV_STUB = """#!/bin/sh +. "$STUB_BIN/barrier.sh" +case "$*" in + *orjson*) + [ -n "${STUB_BARRIER_DIR:-}" ] && barrier_sync genapi "python dashboard" + ;; +esac +exit 0 +""" + +NPM_STUB = """#!/bin/sh +case "$*" in + "run gen:api") + [ "${STUB_FAIL:-}" = "gen-api" ] && exit 1 + ;; +esac +exit 0 +""" + +NODE_STUB = """#!/bin/sh +exit 0 +""" + + +def _write_executable(path: Path, body: str) -> None: + path.write_text(body) + path.chmod(0o755) + + +def _sandbox(tmp_path: Path) -> 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") + dashboard = repo / "ui" / "litellm-dashboard" + (dashboard / "src").mkdir(parents=True) + (dashboard / "node_modules").mkdir() + (dashboard / "src" / "app.ts").write_text("export {}\n") + subprocess.run(["git", "init", "-q"], cwd=repo, check=True) + subprocess.run(["git", "add", "."], cwd=repo, check=True) + + bin_dir = tmp_path / "bin" + bin_dir.mkdir() + (bin_dir / "barrier.sh").write_text(BARRIER_HELPER) + _write_executable(bin_dir / "make", MAKE_STUB) + _write_executable(bin_dir / "npx", NPX_STUB) + _write_executable(bin_dir / "uv", UV_STUB) + _write_executable(bin_dir / "npm", NPM_STUB) + _write_executable(bin_dir / "node", NODE_STUB) + return repo, bin_dir + + +def _run(repo: Path, bin_dir: Path, extra_env: dict[str, str]) -> subprocess.CompletedProcess[str]: + env = { + "PATH": os.pathsep.join([str(bin_dir), "/usr/bin", "/bin"]), + "HOME": str(repo.parent), + "STUB_BIN": str(bin_dir), + **extra_env, + } + return subprocess.run( + [str(SCRIPT)], + cwd=repo, + capture_output=True, + text=True, + env=env, + timeout=120, + ) + + +def test_python_dashboard_and_gen_api_blocks_run_concurrently_with_grouped_output(tmp_path: Path) -> None: + repo, bin_dir = _sandbox(tmp_path) + barrier_dir = tmp_path / "barrier" + barrier_dir.mkdir() + proc = _run(repo, bin_dir, {"STUB_BARRIER_DIR": str(barrier_dir)}) + assert proc.returncode == 0, proc.stdout + proc.stderr + assert "barrier timeout" not in proc.stdout + proc.stderr + python_at = proc.stdout.index("linting Python") + dashboard_at = proc.stdout.index("linting dashboard") + gen_api_at = proc.stdout.index("API types") + assert python_at < dashboard_at < gen_api_at + + +def test_all_blocks_passing_exits_zero(tmp_path: Path) -> None: + repo, bin_dir = _sandbox(tmp_path) + proc = _run(repo, bin_dir, {}) + assert proc.returncode == 0, proc.stdout + proc.stderr + + +@pytest.mark.parametrize( + ("fail", "message"), + [ + ("make-lint", "Python lint failed"), + ("eslint", "Dashboard lint failed"), + ("gen-api", "npm run gen:api failed"), + ], +) +def test_a_failing_block_fails_the_whole_run(tmp_path: Path, fail: str, message: str) -> None: + repo, bin_dir = _sandbox(tmp_path) + proc = _run(repo, bin_dir, {"STUB_FAIL": fail}) + assert proc.returncode == 1 + assert message in proc.stdout + proc.stderr From a1f497c7c087f3ea8bd90bc9340bdc034a717b8e Mon Sep 17 00:00:00 2001 From: mateo-berri <277851410+mateo-berri@users.noreply.github.com> Date: Tue, 4 Aug 2026 21:28:46 -0700 Subject: [PATCH 2/3] fix(pre-commit): kill background jobs and remove their logs on interrupt --- scripts/pre_commit_lint.sh | 8 +++ tests/test_litellm/test_pre_commit_lint.py | 61 +++++++++++++++++++++- 2 files changed, 67 insertions(+), 2 deletions(-) diff --git a/scripts/pre_commit_lint.sh b/scripts/pre_commit_lint.sh index bc3cdec61dc..02fd78dfbb3 100755 --- a/scripts/pre_commit_lint.sh +++ b/scripts/pre_commit_lint.sh @@ -110,6 +110,14 @@ python_checks() { return $rc } +on_interrupt() { + trap - INT TERM + rm -f "${python_log:-}" "${dash_log:-}" "${gen_log:-}" + kill 0 2>/dev/null + exit 130 +} +trap on_interrupt INT TERM + if [ -n "$litellm_py_files" ]; then python_log=$(mktemp) python_checks > "$python_log" 2>&1 & diff --git a/tests/test_litellm/test_pre_commit_lint.py b/tests/test_litellm/test_pre_commit_lint.py index f77b3b25062..f9dbf17b8b2 100644 --- a/tests/test_litellm/test_pre_commit_lint.py +++ b/tests/test_litellm/test_pre_commit_lint.py @@ -1,5 +1,9 @@ import os +import signal import subprocess +import time +from collections.abc import Callable +from contextlib import suppress from pathlib import Path import pytest @@ -29,6 +33,11 @@ case "$*" in lint) [ "${STUB_FAIL:-}" = "make-lint" ] && exit 1 [ -n "${STUB_BARRIER_DIR:-}" ] && barrier_sync python "dashboard genapi" + if [ -n "${STUB_HANG_DIR:-}" ]; then + echo "$$" > "$STUB_HANG_DIR/make.pid" + touch "$STUB_HANG_DIR/make.started" + sleep 60 + fi ;; esac exit 0 @@ -99,13 +108,17 @@ def _sandbox(tmp_path: Path) -> tuple[Path, Path]: return repo, bin_dir -def _run(repo: Path, bin_dir: Path, extra_env: dict[str, str]) -> subprocess.CompletedProcess[str]: - env = { +def _env(repo: Path, bin_dir: Path, extra_env: dict[str, str]) -> dict[str, str]: + return { "PATH": os.pathsep.join([str(bin_dir), "/usr/bin", "/bin"]), "HOME": str(repo.parent), "STUB_BIN": str(bin_dir), **extra_env, } + + +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( [str(SCRIPT)], cwd=repo, @@ -135,6 +148,50 @@ def test_all_blocks_passing_exits_zero(tmp_path: Path) -> None: assert proc.returncode == 0, proc.stdout + proc.stderr +def _wait_until(predicate: Callable[[], bool], timeout_seconds: float) -> bool: + deadline = time.monotonic() + timeout_seconds + while time.monotonic() < deadline: + if predicate(): + return True + time.sleep(0.05) + return predicate() + + +def _pid_gone(pid: int) -> bool: + try: + os.kill(pid, 0) + except ProcessLookupError: + return True + return False + + +def test_interrupt_kills_background_jobs_and_removes_logs(tmp_path: Path) -> None: + repo, bin_dir = _sandbox(tmp_path) + hang_dir = tmp_path / "hang" + hang_dir.mkdir() + tmp_dir = tmp_path / "tmpdir" + tmp_dir.mkdir() + extra = {"STUB_HANG_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 / "make.started").exists, 10) + os.killpg(proc.pid, signal.SIGINT) + assert proc.wait(timeout=10) != 0 + make_pid = int((hang_dir / "make.pid").read_text()) + assert _wait_until(lambda: _pid_gone(make_pid), 5) + assert list(tmp_dir.iterdir()) == [] + finally: + with suppress(ProcessLookupError, PermissionError): + os.killpg(proc.pid, signal.SIGTERM) + + @pytest.mark.parametrize( ("fail", "message"), [ From d7dbb28b3239d629680ea81ef41675accca2e9c9 Mon Sep 17 00:00:00 2001 From: mateo-berri <277851410+mateo-berri@users.noreply.github.com> Date: Tue, 4 Aug 2026 21:37:01 -0700 Subject: [PATCH 3/3] fix(pre-commit): scope interrupt cleanup to the job process groups --- scripts/pre_commit_lint.sh | 10 ++++++++- tests/test_litellm/test_pre_commit_lint.py | 24 ++++++++++++++++++++++ 2 files changed, 33 insertions(+), 1 deletion(-) diff --git a/scripts/pre_commit_lint.sh b/scripts/pre_commit_lint.sh index 02fd78dfbb3..d2cf7cd307f 100755 --- a/scripts/pre_commit_lint.sh +++ b/scripts/pre_commit_lint.sh @@ -113,15 +113,19 @@ python_checks() { on_interrupt() { trap - INT TERM rm -f "${python_log:-}" "${dash_log:-}" "${gen_log:-}" - kill 0 2>/dev/null + for job_pid in ${python_pid:-} ${dash_pid:-} ${gen_pid:-}; do + kill -- "-$job_pid" 2>/dev/null || true + done exit 130 } trap on_interrupt INT TERM if [ -n "$litellm_py_files" ]; then python_log=$(mktemp) + set -m python_checks > "$python_log" 2>&1 & python_pid=$! + set +m fi if [ -n "$e2e_py_files" ] && [ -z "$litellm_py_files" ]; then @@ -147,8 +151,10 @@ dashboard_checks() { if [ -n "$ui_prettier_files" ] || [ -n "$ui_eslint_files" ]; then dash_log=$(mktemp) + set -m dashboard_checks > "$dash_log" 2>&1 & dash_pid=$! + set +m fi genapi_checks() { @@ -183,8 +189,10 @@ genapi_checks() { if [ -n "$spec_files" ]; then gen_log=$(mktemp) + set -m genapi_checks > "$gen_log" 2>&1 & gen_pid=$! + set +m fi if [ -n "${python_pid:-}" ]; then diff --git a/tests/test_litellm/test_pre_commit_lint.py b/tests/test_litellm/test_pre_commit_lint.py index f9dbf17b8b2..92de89a3cd5 100644 --- a/tests/test_litellm/test_pre_commit_lint.py +++ b/tests/test_litellm/test_pre_commit_lint.py @@ -192,6 +192,30 @@ def test_interrupt_kills_background_jobs_and_removes_logs(tmp_path: Path) -> Non 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" + hang_dir.mkdir() + marker = tmp_path / "invoker_survived" + proc = subprocess.Popen( + ["bash", "-c", 'trap : INT; "$1"; echo "$?" > "$2"', "bash", str(SCRIPT), str(marker)], + cwd=repo, + env=_env(repo, bin_dir, {"STUB_HANG_DIR": str(hang_dir)}), + stdout=subprocess.DEVNULL, + stderr=subprocess.DEVNULL, + start_new_session=True, + ) + try: + assert _wait_until((hang_dir / "make.started").exists, 10) + os.killpg(proc.pid, signal.SIGINT) + assert proc.wait(timeout=10) == 0 + assert _wait_until(marker.exists, 5) + assert marker.read_text().strip() == "130" + finally: + with suppress(ProcessLookupError, PermissionError): + os.killpg(proc.pid, signal.SIGTERM) + + @pytest.mark.parametrize( ("fail", "message"), [