refactor(pre-commit): move added rationale out of source comments

Greptile flagged the explanatory comments this branch added as a violation of
the repo convention against new source comments, and it is right.

The Python rationale moves into docstrings on the three memoized helpers, which
is what the rest of these gates already use, and the thread-width constant is
described in the module docstring that documents every other measurement
parameter. The Makefile and shell comments go entirely.

The one constraint a comment was carrying that nothing else did, that the
dashboard is provisioned once rather than once per node block, is now a test
instead: it fails if the provisioning moves inside dashboard_checks and
genapi_checks, where two npm installs would race in the same directory.

The header line listing what a dashboard-staged commit runs is updated rather
than removed, so it does not go stale.

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

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

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)

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:

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()

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("")

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"}

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)
 

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")