Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 21 additions & 12 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -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 format \
info lint lint-dev lint-checks format \
lint-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 \
Expand Down Expand Up @@ -53,6 +53,11 @@ help:
UV := uv
UV_RUN := $(UV) run --no-sync

LINT_DEP_INSTALL ?= install-dev
LINT_DEP_BASE ?= lint-fetch-base
LINT_JOBS := $(shell sysctl -n hw.ncpu 2>/dev/null || nproc 2>/dev/null || echo 4)
LINT_OUTPUT_SYNC := $(if $(filter output-sync,$(.FEATURES)),--output-sync=target,)

# Show info
info:
@echo "UV: $(UV)"
Expand Down Expand Up @@ -107,12 +112,12 @@ lint-fetch-base:
# running proxy need.
lint-install:
$(UV) sync --inexact --frozen --group proxy-dev
$(UV_RUN) prisma generate --schema litellm/proxy/schema.prisma
$(UV_RUN) python scripts/prisma_generate_if_needed.py

# 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
lint-format-check-changed: $(LINT_DEP_INSTALL) $(LINT_DEP_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."; \
Expand All @@ -121,7 +126,7 @@ lint-format-check-changed: install-dev lint-fetch-base
fi

# Linting targets
lint-ruff: install-dev
lint-ruff: $(LINT_DEP_INSTALL)
cd litellm && $(UV_RUN) ruff check . && cd ..

# faster linter for developing ...
Expand Down Expand Up @@ -156,12 +161,12 @@ 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 lint-fetch-base
lint-basedpyright: $(LINT_DEP_INSTALL) $(LINT_DEP_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
lint-type-discipline: $(LINT_DEP_INSTALL) $(LINT_DEP_BASE)
$(UV_RUN) python scripts/type_discipline_gate.py --base origin/litellm_internal_staging

# --update lowers each limit by what this branch fixed since its branch point, so
Expand All @@ -176,7 +181,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 lint-fetch-base
lint-gate: $(LINT_DEP_INSTALL) $(LINT_DEP_BASE)
$(UV_RUN) python scripts/ruff_strict_gate.py --base origin/litellm_internal_staging

lint-ruff-budget-update: install-dev lint-fetch-base
Expand All @@ -188,20 +193,24 @@ lint-type-discipline-budget-update: install-dev lint-fetch-base
# Ratchet all budgets in one shot (ruff strict + type-discipline + basedpyright)
lint-budget-update: lint-ruff-budget-update lint-type-discipline-budget-update lint-basedpyright-budget-update

check-circular-imports: install-dev
check-circular-imports: $(LINT_DEP_INSTALL)
cd litellm && $(UV_RUN) python ../tests/documentation_tests/test_circular_imports.py && cd ..

check-import-safety: install-dev
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
# 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
# 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.
lint: lint-install lint-fetch-base
$(MAKE) -j $(LINT_JOBS) $(LINT_OUTPUT_SYNC) LINT_DEP_INSTALL= LINT_DEP_BASE= lint-checks

lint-checks: 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
Expand Down
2 changes: 1 addition & 1 deletion scripts/pre_commit_lint.sh
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ if [ -n "$spec_files" ]; then
# 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
if ! uv run --no-sync python scripts/prisma_generate_if_needed.py; 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
Expand Down
69 changes: 69 additions & 0 deletions scripts/prisma_generate_if_needed.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
#!/usr/bin/env python3
"""Run ``prisma generate`` only when its inputs changed since the last run.

The generated client is a pure function of ``litellm/proxy/schema.prisma`` and
the installed prisma package version, so a stamp of those two written next to
the venv is enough to prove the client is current. The stamp lives under
``sys.prefix`` so recreating the venv discards it, and a missing generated
client (a fresh or reinstalled prisma package) forces a regenerate even when
the stamp matches. The prisma package itself is never imported here: once
generated it re-exports the whole client on import, which costs more than the
generate this script exists to skip.
"""

import hashlib
import importlib.metadata
import importlib.util
import subprocess
import sys
from pathlib import Path

REPO_ROOT = Path(__file__).resolve().parent.parent
SCHEMA = REPO_ROOT / "litellm" / "proxy" / "schema.prisma"
STAMP = Path(sys.prefix) / "litellm-prisma-schema.stamp"


def stamp_value(schema_bytes: bytes, prisma_version: str) -> str:
return f"{hashlib.sha256(schema_bytes).hexdigest()}:{prisma_version}"


def should_skip(stamp: Path, expected: str, client_generated: bool) -> bool:
if not client_generated:
return False
try:
return stamp.read_text() == expected
except OSError:
return False


def client_is_generated() -> bool:
spec = importlib.util.find_spec("prisma")
if spec is None or not spec.submodule_search_locations:
return False
return any(
(Path(location) / "client.py").exists()
for location in spec.submodule_search_locations
)


def main() -> int:
version = importlib.metadata.version("prisma")
expected = stamp_value(SCHEMA.read_bytes(), version)
if should_skip(STAMP, expected, client_is_generated()):
print(
f"Prisma client already generated for {SCHEMA.relative_to(REPO_ROOT)} "
f"(prisma {version}); skipping prisma generate"
)
return 0
result = subprocess.run(
[sys.executable, "-m", "prisma", "generate", "--schema", str(SCHEMA)],
cwd=REPO_ROOT,
)
if result.returncode != 0:
return result.returncode
STAMP.write_text(expected)
return 0


if __name__ == "__main__":
raise SystemExit(main())
18 changes: 17 additions & 1 deletion scripts/ruff_strict_gate.py
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,18 @@ def base_counts(ref: str) -> dict:
shutil.rmtree(parent, ignore_errors=True)


def over_ceiling(head: dict, budget: dict) -> frozenset:
"""Rules whose head count already exceeds their limit.

A rule can only breach when it is over its limit, so when none are the base
comparison cannot change the verdict and the base worktree scan can be skipped.
"""
return frozenset(
rule for rule, spec in budget.items()
if head.get(rule, 0) > spec["limit"]
)


def evaluate(head: dict, base: dict, budget: dict) -> list:
breaches = []
for rule, spec in budget.items():
Expand Down Expand Up @@ -119,8 +131,12 @@ def introduced(violations: list, changed: dict) -> list:
def cmd_check(base: str) -> None:
budget = json.loads(BUDGET_PATH.read_text())
head = head_violations()
head_counts = count_by_rule(head)
if not over_ceiling(head_counts, budget):
print(f"OK: every strict rule is within its codebase ceiling (base {base})")
return
base_point = _run(["git", "merge-base", base, "HEAD"]).strip() or base
breaches = evaluate(count_by_rule(head), base_counts(base_point), budget)
breaches = evaluate(head_counts, base_counts(base_point), budget)
if not breaches:
print(f"OK: every strict rule is within its codebase ceiling (base {base})")
return
Expand Down
128 changes: 122 additions & 6 deletions scripts/type_check_gate.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,13 @@
grows the rule past its limit still fails.

Head counts are read from stdin (the caller runs basedpyright once and pipes
``--outputjson`` in); the base count is a second basedpyright pass over a
detached worktree at the merge-base, run under the same environment so import
resolution matches. ``--update`` ratchets each rule's ``limit`` down by the
``--outputjson`` in). The base count only matters once some rule is over its
limit, so when none is the base pass is skipped outright. When it is needed, it
is a second basedpyright pass over a detached worktree at the merge-base, run
under the same environment so import resolution matches, and its per-rule
counts are cached under the repo's git common dir keyed by merge-base commit,
``pyrightconfig.json``, and ``uv.lock``, so re-runs against the same branch
point pay for it once. ``--update`` ratchets each rule's ``limit`` down by the
number of errors this branch fixed relative to its branch point (the merge-base),
so the headroom you were granted shrinks by exactly what you cleared and never
grows.
Expand All @@ -28,20 +32,24 @@

import argparse
import contextlib
import hashlib
import json
import os
import shutil
import subprocess
import sys
import tempfile
from collections import Counter
from collections.abc import Iterator, Mapping
from collections.abc import Callable, Iterator, Mapping
from pathlib import Path
from typing import NamedTuple

REPO_ROOT = Path(__file__).resolve().parent.parent
BUDGET_PATH = REPO_ROOT / "basedpyright-code-budget.json"
PYRIGHT_CONFIG = REPO_ROOT / "pyrightconfig.json"
UV_LOCK = REPO_ROOT / "uv.lock"
DEFAULT_BASE = "origin/litellm_internal_staging"
CACHE_FILE_PREFIX = "basedpyright-base-"

# Bucket for a basedpyright diagnostic with no `rule`. Counted so it's gated.
UNCODED = "<uncoded>"
Expand Down Expand Up @@ -129,6 +137,109 @@ def base_counts(ref: str) -> dict[str, int]:
return count_basedpyright(proc.stdout, root=worktree)


def over_ceiling(
head: Mapping[str, int], budget: Mapping[str, Mapping[str, int]]
) -> frozenset[str]:
"""Rules whose head count already exceeds their limit.

A rule can only breach when it is over its limit, so when none are the base
comparison cannot change the verdict and the base worktree pass can be skipped.
"""
return frozenset(
code
for code, total in head.items()
if total > (budget[code]["limit"] if code in budget else DEFAULT_LIMIT)
)


def environment_fingerprints() -> tuple[str, ...]:
return tuple(
hashlib.sha256(path.read_bytes()).hexdigest()
for path in (PYRIGHT_CONFIG, UV_LOCK)
if path.exists()
)


def cache_key(base_point: str, fingerprints: tuple[str, ...]) -> str:
return hashlib.sha256("|".join((base_point, *fingerprints)).encode()).hexdigest()[
:16
]


def cache_path(
directory: Path, base_point: str, fingerprints: tuple[str, ...]
) -> Path:
return directory / f"{CACHE_FILE_PREFIX}{cache_key(base_point, fingerprints)}.json"


def default_cache_dir() -> Path:
common = Path(_run(["git", "rev-parse", "--git-common-dir"]).strip())
resolved = common if common.is_absolute() else REPO_ROOT / common
return resolved / "litellm-lint-cache"


def load_cached_counts(path: Path) -> dict[str, int] | None:
try:
data = json.loads(path.read_text())
except (OSError, json.JSONDecodeError):
return None
counts = data.get("counts") if isinstance(data, dict) else None
if not isinstance(counts, dict):
return None
if not all(
isinstance(code, str) and isinstance(total, int) and not isinstance(total, bool)
for code, total in counts.items()
):
return None
return counts


def scratch_path(path: Path) -> Path:
"""In-flight scratch for the tmp+rename write. Dot-prefixed so the prune
glob in `store_counts` can never match it (a concurrent run would otherwise
unlink it between write and rename), and pid-suffixed so two concurrent
writers of the same entry never share a scratch."""
return path.with_name(f".{path.name}.{os.getpid()}.tmp")


def store_counts(
directory: Path, path: Path, base_point: str, counts: Mapping[str, int]
) -> None:
directory.mkdir(parents=True, exist_ok=True)
for stale in directory.glob(f"{CACHE_FILE_PREFIX}*.json"):
if stale != path:
stale.unlink(missing_ok=True)
scratch = scratch_path(path)
scratch.write_text(
json.dumps(
{"base_point": base_point, "counts": dict(sorted(counts.items()))},
indent=2,
)
+ "\n"
)
scratch.replace(path)
Comment thread
greptile-apps[bot] marked this conversation as resolved.

Comment thread
greptile-apps[bot] marked this conversation as resolved.

def base_counts_cached(
base_point: str,
cache_dir: Path | None = None,
compute: Callable[[str], dict[str, int]] = base_counts,
) -> dict[str, int]:
"""`base_counts` memoized on disk. The base tree at a given commit is
immutable, so its counts are a pure function of the merge-base plus the
environment fingerprints in the cache key; an empty result is never stored
because it is the signature of a crashed pass, not a clean tree."""
directory = default_cache_dir() if cache_dir is None else cache_dir
path = cache_path(directory, base_point, environment_fingerprints())
cached = load_cached_counts(path)
if cached is not None:
return cached
counts = compute(base_point)
if counts:
store_counts(directory, path, base_point, counts)
return counts


def evaluate(
head: Mapping[str, int],
base: Mapping[str, int],
Expand Down Expand Up @@ -185,7 +296,7 @@ def cmd_update(current: Mapping[str, int], base_ref: str = DEFAULT_BASE) -> None
"""
budget = json.loads(BUDGET_PATH.read_text()) if BUDGET_PATH.exists() else {}
base_point = _run(["git", "merge-base", base_ref, "HEAD"]).strip() or base_ref
updated = ratcheted_budget(budget, current, base_counts(base_point))
updated = ratcheted_budget(budget, current, base_counts_cached(base_point))
BUDGET_PATH.write_text(json.dumps(updated, indent=2, sort_keys=True) + "\n")
cleared = sum(budget[code]["limit"] - updated[code]["limit"] for code in updated)
print(
Expand All @@ -205,8 +316,13 @@ def cmd_check(base_ref: str) -> None:
f"nothing; refusing to certify a vacuous run."
)
raise SystemExit(1)
if not over_ceiling(head, budget):
print(
f"OK: every rule is within its basedpyright limit ({sum(head.values())} errors total)"
)
return
base_point = _run(["git", "merge-base", base_ref, "HEAD"]).strip() or base_ref
base = base_counts(base_point)
base = base_counts_cached(base_point)
if is_vacuous_run(base, budget):
print(
f"FAIL: basedpyright produced no errors for the base tree at "
Expand Down
Loading
Loading