diff --git a/Makefile b/Makefile index 6813c875953..39713a9f724 100644 --- a/Makefile +++ b/Makefile @@ -77,9 +77,6 @@ install-dev: bootstrap: bootstrap-python bootstrap-dashboard @echo "bootstrap: done" -# The halves are separate targets so a caller can provision only what it needs: -# `make pre-commit` on a Python-only change takes bootstrap-python and never pays -# for (nor hard-fails on) the dashboard's node toolchain. bootstrap-python: $(UV) sync --inexact --frozen --extra proxy --group proxy-dev --group e2e-dev $(UV_RUN) python scripts/prisma_generate_if_needed.py diff --git a/scripts/pre_commit_lint.sh b/scripts/pre_commit_lint.sh index ad39a5fe550..f8ddb528729 100755 --- a/scripts/pre_commit_lint.sh +++ b/scripts/pre_commit_lint.sh @@ -6,7 +6,7 @@ # - 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) -# - dashboard staged -> prettier + eslint + lint budgets (test-litellm-ui-build.yml's frontend-lint) +# - dashboard staged -> `make bootstrap-dashboard` + 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 @@ -98,8 +98,6 @@ EOF # counts are not diff-scoped, so a local pass here means the budget step will # pass in CI too. report=$(mktemp) - # Ctrl-C reaches this job as a SIGTERM from on_interrupt; bash skips EXIT - # traps on an uncaught fatal signal, so catch it and exit through one. trap 'rm -f "$report"' EXIT trap 'exit 130' INT TERM npx eslint . -f json -o "$report" || true @@ -159,10 +157,6 @@ 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 -# `make pre-commit` provisions the Python env only, so top up the dashboard's -# node_modules for the commits that reach it. Placed after the Python block forked -# (so the install overlaps that lint) and before both node blocks fork (so two npm -# installs never race in the same directory). if [ -n "$ui_prettier_files" ] || [ -n "$ui_eslint_files" ] || [ -n "$spec_files" ]; then echo "pre-commit: provisioning the dashboard toolchain (make bootstrap-dashboard)" make bootstrap-dashboard || status=1 diff --git a/scripts/ruff_strict_gate.py b/scripts/ruff_strict_gate.py index 70adcce508d..ada845e8a1b 100644 --- a/scripts/ruff_strict_gate.py +++ b/scripts/ruff_strict_gate.py @@ -78,11 +78,13 @@ def _ruff_json(cwd: Path, config: Path) -> list: return json.loads(raw or "[]") -# Memoized because `resolve()` is a filesystem round trip and ruff reports many -# more violations than there are files: the cache makes it one realpath walk per -# file rather than one per violation. @functools.lru_cache(maxsize=None) def _relative_to_repo(filename: str) -> str: + """`filename` as a repo-relative path. + + Memoized because `resolve()` is a filesystem round trip and ruff reports far + more violations than there are files, so the cache makes it one realpath walk + per file rather than one per violation.""" name = Path(filename) return ( (name if name.is_absolute() else REPO_ROOT / name) diff --git a/scripts/type_check_gate.py b/scripts/type_check_gate.py index ccfb1d93d9e..8c5bf63c0e0 100644 --- a/scripts/type_check_gate.py +++ b/scripts/type_check_gate.py @@ -32,9 +32,11 @@ diagnostics differ between a serial pass, a two-thread pass and a four-thread pass; passes at the same width agree exactly, run after run. Width is therefore part of the measurement in exactly the way the installed package set is, and letting it follow ``nproc`` would make a 16-core laptop and a 4-core CI runner -report different totals for the same tree. It is a constant here and is folded -into the fingerprint alongside the group set, so a cache entry or CI artifact -recorded at another width is never matched, only recomputed. +report different totals for the same tree. It is the constant +``BASEDPYRIGHT_THREADS`` here, set to the GitHub-hosted runner's vCPU count so +CI gets full parallelism, and it is folded into the fingerprint alongside the +group set, so a cache entry or CI artifact recorded at another width is never +matched, only recomputed. The gate runs basedpyright itself, for both the head and the base pass, with ``NODE_OPTIONS`` raised to the heap this repo needs: basedpyright's node @@ -104,8 +106,6 @@ PRISMA_SCHEMA = REPO_ROOT / "litellm" / "proxy" / "schema.prisma" # caller-set value while preserving the caller's other NODE_OPTIONS flags. NODE_HEAP_OPTION = "--max-old-space-size=8192" -# Worker threads every pass is measured at. Four is the GitHub-hosted runner's -# vCPU count, so CI gets full parallelism and any dev box measures what CI does. BASEDPYRIGHT_THREADS: Final = 4 # Bucket for a basedpyright diagnostic with no `rule`. Counted so it's gated. @@ -124,11 +124,13 @@ class Breach(NamedTuple): added: int -# Memoized because it is called once per diagnostic, and `resolve()` is a -# filesystem round trip: this tree reports ~149k errors across ~2.2k files, so -# the cache turns ~149k realpath walks into one per file (~6s -> ~0.3s). @functools.lru_cache(maxsize=None) def _to_relative(raw: str, root: Path) -> str | None: + """`raw` as a path relative to `root`, or None when it falls outside. + + Memoized because it is called once per diagnostic while `resolve()` is a + filesystem round trip: this tree reports ~149k errors across ~2.2k files, so + the cache turns ~149k realpath walks into one per file (6.6s -> 0.9s).""" path = Path(raw) absolute = path if path.is_absolute() else root / path try: diff --git a/scripts/type_discipline_gate.py b/scripts/type_discipline_gate.py index b019dca1500..f39fa4b3b1a 100644 --- a/scripts/type_discipline_gate.py +++ b/scripts/type_discipline_gate.py @@ -89,11 +89,13 @@ def resolve_base_point(base_ref: str, cwd: Path = REPO_ROOT) -> str: return merge_point if older == head_point else head_point -# Memoized because `resolve()` is a filesystem round trip and the checker reports -# many more violations than there are files: the cache makes it one realpath walk -# per file rather than one per violation. @functools.lru_cache(maxsize=None) def _relative_to_root(filename: str, root: Path) -> str: + """`filename` as a path relative to `root`. + + Memoized because `resolve()` is a filesystem round trip and the checker + reports far more violations than there are files, so the cache makes it one + realpath walk per file rather than one per violation.""" name = Path(filename) full = name if name.is_absolute() else root / name return full.resolve().relative_to(root).as_posix() diff --git a/tests/test_litellm/test_pre_commit_lint.py b/tests/test_litellm/test_pre_commit_lint.py index 4230bfd110f..7efbc8c83be 100644 --- a/tests/test_litellm/test_pre_commit_lint.py +++ b/tests/test_litellm/test_pre_commit_lint.py @@ -157,9 +157,6 @@ def _make_invocations(tmp_path: Path, repo: Path, bin_dir: Path) -> list[str]: def test_a_python_only_commit_never_provisions_the_dashboard_toolchain(tmp_path: Path) -> None: - # npm install costs seconds this commit has no use for, and on a machine whose - # node predates the dashboard's engines floor it fails outright, which used to - # block `make pre-commit` for changes that never touch the dashboard. repo, bin_dir = _sandbox(tmp_path, stage=("litellm/foo.py",)) invocations = _make_invocations(tmp_path, repo, bin_dir) assert "lint" in invocations @@ -172,6 +169,12 @@ def test_a_commit_that_reaches_the_dashboard_provisions_it(tmp_path: Path, stage assert "bootstrap-dashboard" in _make_invocations(tmp_path, repo, bin_dir) +def test_the_dashboard_is_provisioned_once_when_both_node_blocks_run(tmp_path: Path) -> None: + repo, bin_dir = _sandbox(tmp_path) + invocations = _make_invocations(tmp_path, repo, bin_dir) + assert invocations.count("bootstrap-dashboard") == 1 + + def test_full_output_is_saved_to_a_log_file_in_the_git_dir(tmp_path: Path) -> None: repo, bin_dir = _sandbox(tmp_path) (repo / "scratch.txt").write_text("") diff --git a/tests/test_litellm/test_ruff_strict_gate.py b/tests/test_litellm/test_ruff_strict_gate.py index 45b1f710d55..5ca9613e650 100644 --- a/tests/test_litellm/test_ruff_strict_gate.py +++ b/tests/test_litellm/test_ruff_strict_gate.py @@ -17,8 +17,6 @@ def rule(name, limit): def test_paths_are_resolved_once_per_file_not_once_per_violation(): - # resolve() is a filesystem round trip and ruff reports ~20k strict violations - # over far fewer files, so resolving per violation is pure overhead. gate._relative_to_repo.cache_clear() same_file = str(gate.REPO_ROOT / "litellm" / "a.py") assert {gate._relative_to_repo(same_file) for _ in range(20)} == {"litellm/a.py"} diff --git a/tests/test_litellm/test_type_check_gate.py b/tests/test_litellm/test_type_check_gate.py index 282110a8f15..fc6d3f76ad4 100644 --- a/tests/test_litellm/test_type_check_gate.py +++ b/tests/test_litellm/test_type_check_gate.py @@ -47,8 +47,6 @@ def test_basedpyright_error_without_a_rule_is_bucketed(): def test_counting_resolves_each_file_once_not_once_per_diagnostic(): - # resolve() is a filesystem round trip and this tree reports ~149k errors over - # ~2.2k files, so a resolve per diagnostic costs ~6s of the gate for nothing. gate._to_relative.cache_clear() same_file = ROOT / "litellm" / "a.py" payload = json.dumps( @@ -135,9 +133,6 @@ def test_run_basedpyright_pins_import_resolution_to_the_owned_env(tmp_path): def test_run_basedpyright_pins_the_thread_width_instead_of_inheriting_the_hosts(tmp_path): - # Partitioning files across threads reorders a few order-dependent inferences, - # so counts are only comparable at equal width; letting it follow the core - # count would make a 16-core laptop and a 4-core runner disagree on one tree. captured = tmp_path / "argv.txt" env_dir = _stub_env( tmp_path, @@ -305,8 +300,6 @@ def test_fingerprints_carry_the_dependency_group_set(): def test_fingerprints_carry_the_thread_width(): - # Same reasoning as the group set: a count taken at another width is not - # comparable, so its cache entry and artifact name must not be reachable. assert f"threads:{gate.BASEDPYRIGHT_THREADS}" in gate.environment_fingerprints() assert gate.environment_fingerprints(threads=2) != gate.environment_fingerprints(threads=4) diff --git a/tests/test_litellm/test_type_discipline_gate.py b/tests/test_litellm/test_type_discipline_gate.py index ddab1e3e77d..75962b1223a 100644 --- a/tests/test_litellm/test_type_discipline_gate.py +++ b/tests/test_litellm/test_type_discipline_gate.py @@ -20,9 +20,6 @@ def _budget(limit): def test_paths_are_resolved_once_per_file_not_once_per_violation(): - # resolve() is a filesystem round trip and the checker reports tens of - # thousands of violations over ~2.2k files, so resolving per violation is - # pure overhead on the pre-commit and CI critical path. gate._relative_to_root.cache_clear() root = gate.REPO_ROOT.resolve() same_file = str(root / "litellm" / "a.py")