diff --git a/.github/workflows/publish-basedpyright-base-counts.yml b/.github/workflows/publish-basedpyright-base-counts.yml index d9b034684a4..c85d30df0ce 100644 --- a/.github/workflows/publish-basedpyright-base-counts.yml +++ b/.github/workflows/publish-basedpyright-base-counts.yml @@ -43,22 +43,14 @@ jobs: with: version: "0.10.9" - - name: Install dependencies - run: | - uv sync --frozen --group proxy-dev --group e2e-dev - - # Mirrors test-linting.yml's lint job: basedpyright resolves Prisma's - # generated client only after `prisma generate`, and the published counts - # must match what that job would measure for the same tree. - - name: Generate Prisma client + # The gate provisions its own measurement env (.venv-typecheck: a frozen + # uv sync of its canonical dependency groups plus a generated Prisma + # client), so no install step here can drift from what local runs measure. + - name: Emit basedpyright counts for HEAD env: PRISMA_BINARY_CACHE_DIR: ${{ runner.temp }}/prisma-cache run: | - uv run --no-sync prisma generate --schema litellm/proxy/schema.prisma - - - name: Emit basedpyright counts for HEAD - run: | - uv run --no-sync python scripts/type_check_gate.py --emit-counts-dir "$RUNNER_TEMP/basedpyright-counts" + python scripts/type_check_gate.py --emit-counts-dir "$RUNNER_TEMP/basedpyright-counts" counts_file=$(ls "$RUNNER_TEMP"/basedpyright-counts/basedpyright-counts-*.json) echo "COUNTS_ARTIFACT_NAME=$(basename "$counts_file" .json)" >> "$GITHUB_ENV" diff --git a/.gitignore b/.gitignore index 13f2202305d..3329f39ca10 100644 --- a/.gitignore +++ b/.gitignore @@ -1,5 +1,6 @@ .python-version .venv +.venv-typecheck .venv_policy_test .env .claude diff --git a/Makefile b/Makefile index 3e82e141c77..493828571b7 100644 --- a/Makefile +++ b/Makefile @@ -124,10 +124,10 @@ lint-fetch-base: git fetch origin litellm_internal_staging # Mirror test-linting.yml's lint job environment: the proxy-dev group plus a generated -# Prisma client, so basedpyright resolves the same modules CI does (without the generated -# client the DB wrappers typed against it degrade to Unknown, drifting the budget from -# CI's). --inexact tops up the venv instead of pruning the proxy extras gen:api and the -# running proxy need. +# Prisma client, so `basedpyright tests/e2e` resolves the same modules CI does. The +# budget gate itself no longer measures here (scripts/type_check_gate.py provisions its +# own .venv-typecheck). --inexact tops up the venv instead of pruning the proxy extras +# gen:api and the running proxy need. lint-install: $(UV) sync --inexact --frozen --group proxy-dev --group e2e-dev $(UV_RUN) python scripts/prisma_generate_if_needed.py diff --git a/scripts/type_check_gate.py b/scripts/type_check_gate.py index b8c6cb29a4e..f9d7f2912fd 100644 --- a/scripts/type_check_gate.py +++ b/scripts/type_check_gate.py @@ -12,6 +12,18 @@ a red once two PRs each land near the limit and their sum crosses it: the bystander's count equals its base, so it is spared, while any PR that actually grows the rule past its limit still fails. +Installed packages are part of the measurement: a typed dependency that is +present changes what basedpyright can prove (and therefore which diagnostics +fire) versus when it is absent, so counts from two differently provisioned +venvs are not comparable and their comparison produces phantom breaches no +diff hunk explains. The gate therefore provisions its own environment at +``.venv-typecheck`` (a frozen ``uv sync`` of one canonical dependency-group +set, plus a generated Prisma client) and runs every basedpyright pass from it, +so pre-commit, the CI lint job, and the artifact publisher measure one package +set by construction; re-syncs of an up-to-date env are a near-instant no-op. +The group set is folded into the cache and artifact fingerprint, so counts +recorded under a different set are 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 process OOMs at the ~4 GB default, and when callers had to remember the flag, @@ -21,8 +33,8 @@ 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 +common dir keyed by merge-base commit, ``pyrightconfig.json``, ``uv.lock``, +and the dependency-group set, so re-runs against the same branch point pay for it once. A CI workflow publishes every staging commit's counts as an artifact (``--emit-counts-dir`` is its entry point), and on a disk-cache miss the gate first tries to download the merge-base's artifact through the ``gh`` @@ -64,10 +76,18 @@ CACHE_FILE_PREFIX = "basedpyright-base-" ARTIFACT_NAME_PREFIX = "basedpyright-counts-" GH_TIMEOUT_SECONDS = 10 +# The one environment every basedpyright pass measures in. The group set is +# the slim one the CI publisher has always installed (not bootstrap's fatter +# --extra proxy env), so the committed budgets stay valid; changing it re-keys +# every cache and artifact fingerprint, so stale counts can never be matched. +TYPECHECK_ENV_DIR = REPO_ROOT / ".venv-typecheck" +TYPECHECK_DEP_GROUPS = ("proxy-dev", "e2e-dev") +PRISMA_GENERATE_SCRIPT = REPO_ROOT / "scripts" / "prisma_generate_if_needed.py" + # basedpyright's node process needs more than the ~4 GB default heap on this # repo; appended last so it wins node's last-flag-wins resolution over any # caller-set value while preserving the caller's other NODE_OPTIONS flags. -NODE_HEAP_OPTION = "--max-old-space-size=12288" +NODE_HEAP_OPTION = "--max-old-space-size=8192" # Bucket for a basedpyright diagnostic with no `rule`. Counted so it's gated. UNCODED = "" @@ -129,14 +149,78 @@ def node_options_with_heap(base_env: Mapping[str, str]) -> str: return f"{base_env.get('NODE_OPTIONS', '')} {NODE_HEAP_OPTION}".strip() -def run_basedpyright(cwd: Path = REPO_ROOT) -> str: - """One basedpyright pass over `cwd` with the raised node heap exported. +def typecheck_python_version() -> str | None: + """The interpreter version to build the owned env with, read from + pyrightconfig's `pythonVersion` so the packages installed for basedpyright + to see always come from the same version it type-checks against.""" + try: + config = json.loads(PYRIGHT_CONFIG.read_text()) + except (OSError, ValueError): + return None + version: Final = config.get("pythonVersion") if isinstance(config, dict) else None + return version if isinstance(version, str) else None - Exit 0 (clean) and 1 (errors found) are both output-bearing runs; anything - else is a crash and fails loudly instead of reading as zero errors.""" - exe = shutil.which("basedpyright") or "basedpyright" + +def typecheck_env_commands(env_dir: Path = TYPECHECK_ENV_DIR) -> tuple[tuple[str, ...], ...]: + python_pin: Final = typecheck_python_version() + sync: Final = ( + "uv", + "sync", + "--frozen", + *(("--python", python_pin) if python_pin else ()), + *(flag for group in TYPECHECK_DEP_GROUPS for flag in ("--group", group)), + ) + generate: Final = (str(env_dir / "bin" / "python"), str(PRISMA_GENERATE_SCRIPT)) + return (sync, generate) + + +def _run_provision_step(cmd: tuple[str, ...], env: Mapping[str, str]) -> int: proc = subprocess.run( - [exe, "--outputjson"], + list(cmd), cwd=REPO_ROOT, env=dict(env), capture_output=True, text=True + ) + if proc.returncode != 0: + sys.stderr.write(proc.stdout) + sys.stderr.write(proc.stderr) + return proc.returncode + + +def ensure_typecheck_env( + env_dir: Path = TYPECHECK_ENV_DIR, + run: Callable[[tuple[str, ...], Mapping[str, str]], int] = _run_provision_step, +) -> Path: + """Sync the gate-owned venv (and its generated Prisma client) before a + measurement pass. Unconditional on purpose: an up-to-date env makes both + steps near-instant no-ops, and skipping them on a heuristic is how the + measured environment and the fingerprinted one drift apart.""" + env: Final = {**os.environ, "UV_PROJECT_ENVIRONMENT": str(env_dir)} + for cmd in typecheck_env_commands(env_dir): + if run(cmd, env) != 0: + raise SystemExit( + f"could not provision the type-check environment at {env_dir}: " + f"`{' '.join(cmd)}` failed" + ) + return env_dir + + +def run_basedpyright(cwd: Path = REPO_ROOT, env_dir: Path = TYPECHECK_ENV_DIR) -> str: + """One basedpyright pass over `cwd` from the gate-owned venv, with the + raised node heap exported. + + `--pythonpath` pins import resolution to the owned env's interpreter; it is + the only pin that works, because basedpyright auto-detects a `.venv` in the + project root and that beats both PATH order and VIRTUAL_ENV, silently + measuring the caller's fatter venv (whose extra typed packages flip + diagnostics) whenever the repo has one. Exit 0 (clean) and 1 (errors + found) are both output-bearing runs; anything else is a crash and fails + loudly instead of reading as zero errors.""" + bin_dir: Final = env_dir / "bin" + proc = subprocess.run( + [ + str(bin_dir / "basedpyright"), + "--outputjson", + "--pythonpath", + str(bin_dir / "python"), + ], cwd=cwd, capture_output=True, text=True, @@ -208,11 +292,16 @@ def over_ceiling( ) -def environment_fingerprints() -> tuple[str, ...]: - return tuple( - hashlib.sha256(path.read_bytes()).hexdigest() - for path in (PYRIGHT_CONFIG, UV_LOCK) - if path.exists() +def environment_fingerprints( + dep_groups: tuple[str, ...] = TYPECHECK_DEP_GROUPS, +) -> tuple[str, ...]: + return ( + *( + hashlib.sha256(path.read_bytes()).hexdigest() + for path in (PYRIGHT_CONFIG, UV_LOCK) + if path.exists() + ), + "groups:" + ",".join(dep_groups), ) @@ -560,6 +649,7 @@ def main() -> None: parser.add_argument("--update", action="store_true") parser.add_argument("--emit-counts-dir", type=Path) args = parser.parse_args() + ensure_typecheck_env() head = count_basedpyright(run_basedpyright()) if args.emit_counts_dir is not None: cmd_emit_counts( diff --git a/tests/test_litellm/test_type_check_gate.py b/tests/test_litellm/test_type_check_gate.py index e381f787d78..2d9731db53f 100644 --- a/tests/test_litellm/test_type_check_gate.py +++ b/tests/test_litellm/test_type_check_gate.py @@ -1,6 +1,5 @@ import importlib.util import json -import os import subprocess from pathlib import Path @@ -84,33 +83,51 @@ def test_node_options_with_heap_appends_after_caller_flags_so_it_wins(): assert merged == f"--max-old-space-size=4096 --no-warnings {gate.NODE_HEAP_OPTION}" -def _stub_basedpyright(tmp_path, monkeypatch, script_body): - stub = tmp_path / "basedpyright" +def _stub_env(tmp_path, script_body): + bin_dir = tmp_path / "bin" + bin_dir.mkdir(exist_ok=True) + stub = bin_dir / "basedpyright" stub.write_text(f"#!/bin/sh\n{script_body}\n") stub.chmod(0o755) - monkeypatch.setenv("PATH", str(tmp_path), prepend=os.pathsep) + return tmp_path def test_run_basedpyright_exports_the_raised_heap_to_the_child(tmp_path, monkeypatch): captured = tmp_path / "node_options.txt" - _stub_basedpyright( + env_dir = _stub_env( tmp_path, - monkeypatch, f'echo "$NODE_OPTIONS" > "{captured}"\necho \'{{"generalDiagnostics": []}}\'', ) monkeypatch.delenv("NODE_OPTIONS", raising=False) - assert json.loads(gate.run_basedpyright(cwd=tmp_path)) == {"generalDiagnostics": []} + assert json.loads(gate.run_basedpyright(cwd=tmp_path, env_dir=env_dir)) == { + "generalDiagnostics": [] + } assert captured.read_text().strip() == gate.NODE_HEAP_OPTION -def test_run_basedpyright_fails_loudly_on_a_crash_exit_code(tmp_path, monkeypatch): +def test_run_basedpyright_pins_import_resolution_to_the_owned_env(tmp_path): + # basedpyright auto-detects a `.venv` in the project root, and that beats + # PATH order and VIRTUAL_ENV; only an explicit --pythonpath keeps the + # caller's fatter venv (whose extra typed packages flip diagnostics vs CI) + # out of the measurement. + captured = tmp_path / "argv.txt" + env_dir = _stub_env( + tmp_path, + f'echo "$@" > "{captured}"\necho \'{{"generalDiagnostics": []}}\'', + ) + gate.run_basedpyright(cwd=tmp_path, env_dir=env_dir) + argv = captured.read_text().split() + assert argv[argv.index("--pythonpath") + 1] == str(env_dir / "bin" / "python") + + +def test_run_basedpyright_fails_loudly_on_a_crash_exit_code(tmp_path): import pytest # 134 is SIGABRT, what node dies with on a heap OOM; it must never read as a # clean zero-error run. - _stub_basedpyright(tmp_path, monkeypatch, "exit 134") + env_dir = _stub_env(tmp_path, "exit 134") with pytest.raises(SystemExit): - gate.run_basedpyright(cwd=tmp_path) + gate.run_basedpyright(cwd=tmp_path, env_dir=env_dir) def test_at_or_under_ceiling_passes(): @@ -248,6 +265,68 @@ def test_cache_key_changes_with_base_point_and_each_fingerprint(): assert gate.cache_key("abc", ("cfg", "lock2")) != key +def test_fingerprints_carry_the_dependency_group_set(): + # Counts measured under one group set must never be compared against + # another's: the fingerprint difference re-keys every cache entry and + # artifact name, so a changed canonical set falls back to recompute. + assert gate.environment_fingerprints() == gate.environment_fingerprints() + assert gate.environment_fingerprints( + dep_groups=("proxy-dev",) + ) != gate.environment_fingerprints(dep_groups=("proxy-dev", "e2e-dev")) + assert gate.environment_fingerprints()[-1] == "groups:" + ",".join( + gate.TYPECHECK_DEP_GROUPS + ) + + +def test_env_commands_sync_the_canonical_groups_then_generate_prisma(): + sync, generate = gate.typecheck_env_commands(Path("/envdir")) + assert sync[:3] == ("uv", "sync", "--frozen") + adjacent = list(zip(sync, sync[1:])) + for group in gate.TYPECHECK_DEP_GROUPS: + assert ("--group", group) in adjacent + assert generate == ( + str(Path("/envdir") / "bin" / "python"), + str(gate.PRISMA_GENERATE_SCRIPT), + ) + + +def test_env_interpreter_pin_tracks_pyrightconfigs_python_version(): + configured = json.loads((ROOT / "pyrightconfig.json").read_text())[ + "pythonVersion" + ] + assert gate.typecheck_python_version() == configured + sync = gate.typecheck_env_commands()[0] + assert sync[sync.index("--python") + 1] == configured + + +def test_ensure_env_targets_the_owned_dir_and_runs_sync_then_generate(tmp_path): + calls = [] + + def runner(cmd, env): + calls.append((cmd[:2], env["UV_PROJECT_ENVIRONMENT"])) + return 0 + + assert gate.ensure_typecheck_env(env_dir=tmp_path, run=runner) == tmp_path + assert calls == [ + (("uv", "sync"), str(tmp_path)), + ((str(tmp_path / "bin" / "python"), str(gate.PRISMA_GENERATE_SCRIPT)), str(tmp_path)), + ] + + +def test_ensure_env_fails_loudly_and_stops_at_the_first_failed_step(tmp_path): + import pytest + + calls = [] + + def failing(cmd, env): + calls.append(cmd) + return 2 + + with pytest.raises(SystemExit): + gate.ensure_typecheck_env(env_dir=tmp_path, run=failing) + assert len(calls) == 1 + + def test_cached_counts_round_trip(tmp_path): path = gate.cache_path(tmp_path, "abc123", ("f1", "f2")) gate.store_counts(tmp_path, path, "abc123", {"reportAny": 3, "reportCall": 1})