mirror of
https://github.com/BerriAI/litellm.git
synced 2026-10-06 02:48:13 +00:00
fix(lint): measure the basedpyright budget gate in a gate-owned venv
The gate previously measured whatever environment the caller happened to have. Locally that is the fat bootstrap venv (--extra proxy pulls in fastapi-sso, whose type info flips a reportUnnecessaryIsInstance diagnostic in ui_sso.py), while CI's publisher venv only has the proxy-dev and e2e-dev groups, so identical trees measured 866 locally vs 865 in CI and every local gate run breached by a phantom +1 scripts/type_check_gate.py now provisions .venv-typecheck itself: a frozen uv sync of the canonical proxy-dev and e2e-dev groups, the interpreter pinned to pyrightconfig.json's pythonVersion, plus the generated Prisma client. Every measurement pass is pinned to that env with --pythonpath, because basedpyright auto-detects a .venv in the project root and that auto-detection beats both PATH order and VIRTUAL_ENV, so the CLI flag is the only pin that actually works. The dependency-group set is folded into the environment fingerprint, so artifacts or caches recorded under a different group set never match and the gate falls back to computing base counts locally instead of comparing mismatched environments The publisher workflow drops its own install and prisma steps and lets the script build the measurement env, and the node heap for the full-tree pass drops from 12GB to 8GB (peak RSS measured at 5.4GB)
This commit is contained in:
parent
ba91768146
commit
fa47c47020
5 changed files with 203 additions and 41 deletions
|
|
@ -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"
|
||||
|
||||
|
|
|
|||
1
.gitignore
vendored
1
.gitignore
vendored
|
|
@ -1,5 +1,6 @@
|
|||
.python-version
|
||||
.venv
|
||||
.venv-typecheck
|
||||
.venv_policy_test
|
||||
.env
|
||||
.claude
|
||||
|
|
|
|||
8
Makefile
8
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
|
||||
|
|
|
|||
|
|
@ -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 = "<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(
|
||||
|
|
|
|||
|
|
@ -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})
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue