From 495a93c930012279121db66d68d8803dd299ae70 Mon Sep 17 00:00:00 2001 From: mateo-berri <277851410+mateo-berri@users.noreply.github.com> Date: Thu, 2 Jul 2026 17:00:51 -0700 Subject: [PATCH 1/2] perf(lint): skip and cache base gate passes, parallelize make lint, skip redundant prisma generate make pre-commit paid for a full second basedpyright pass over a merge-base worktree on every run even when no rule was over its ceiling, re-generated an unchanged Prisma client, and ran seven independent checks sequentially. The basedpyright and ruff strict gates now skip the base pass when head is within every limit (the same early-out type_discipline_gate already had), the basedpyright base counts are cached under the git common dir keyed by merge-base commit, pyrightconfig.json, and uv.lock, prisma generate only runs when the schema or prisma version changed, and make lint fans its checks out through a parallel sub-make after a single setup phase --- Makefile | 33 +++-- scripts/pre_commit_lint.sh | 2 +- scripts/prisma_generate_if_needed.py | 69 ++++++++++ scripts/ruff_strict_gate.py | 18 ++- scripts/type_check_gate.py | 119 +++++++++++++++++- .../test_prisma_generate_if_needed.py | 35 ++++++ tests/test_litellm/test_ruff_strict_gate.py | 16 +++ tests/test_litellm/test_type_check_gate.py | 97 ++++++++++++++ 8 files changed, 369 insertions(+), 20 deletions(-) create mode 100644 scripts/prisma_generate_if_needed.py create mode 100644 tests/test_litellm/test_prisma_generate_if_needed.py diff --git a/Makefile b/Makefile index 2cc4ec3e45a..f8d10de2917 100644 --- a/Makefile +++ b/Makefile @@ -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 \ @@ -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)" @@ -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."; \ @@ -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 ... @@ -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 @@ -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 @@ -188,10 +193,10 @@ 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 @@ -199,9 +204,13 @@ check-import-safety: install-dev # 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 diff --git a/scripts/pre_commit_lint.sh b/scripts/pre_commit_lint.sh index d667d6758e1..04a2af56ce2 100755 --- a/scripts/pre_commit_lint.sh +++ b/scripts/pre_commit_lint.sh @@ -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 diff --git a/scripts/prisma_generate_if_needed.py b/scripts/prisma_generate_if_needed.py new file mode 100644 index 00000000000..d2c40adf820 --- /dev/null +++ b/scripts/prisma_generate_if_needed.py @@ -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()) diff --git a/scripts/ruff_strict_gate.py b/scripts/ruff_strict_gate.py index 5273e4805f6..25f6c4d29ba 100644 --- a/scripts/ruff_strict_gate.py +++ b/scripts/ruff_strict_gate.py @@ -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(): @@ -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 diff --git a/scripts/type_check_gate.py b/scripts/type_check_gate.py index d3837cc2c0d..e2ab12dfb63 100644 --- a/scripts/type_check_gate.py +++ b/scripts/type_check_gate.py @@ -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. @@ -28,20 +32,23 @@ import argparse import contextlib +import hashlib import json 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 = "" @@ -129,6 +136,101 @@ 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 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}*"): + if stale != path: + stale.unlink(missing_ok=True) + scratch = path.with_name(path.name + ".tmp") + scratch.write_text( + json.dumps( + {"base_point": base_point, "counts": dict(sorted(counts.items()))}, + indent=2, + ) + + "\n" + ) + scratch.replace(path) + + +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], @@ -185,7 +287,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( @@ -205,8 +307,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 " diff --git a/tests/test_litellm/test_prisma_generate_if_needed.py b/tests/test_litellm/test_prisma_generate_if_needed.py new file mode 100644 index 00000000000..39b9fcc4202 --- /dev/null +++ b/tests/test_litellm/test_prisma_generate_if_needed.py @@ -0,0 +1,35 @@ +import importlib.util +from pathlib import Path + +_MODULE_PATH = ( + Path(__file__).resolve().parents[2] / "scripts" / "prisma_generate_if_needed.py" +) +_spec = importlib.util.spec_from_file_location("prisma_generate_if_needed", _MODULE_PATH) +mod = importlib.util.module_from_spec(_spec) +_spec.loader.exec_module(mod) + + +def test_stamp_changes_with_schema_and_with_prisma_version(): + stamp = mod.stamp_value(b"model A {}", "0.11.0") + assert mod.stamp_value(b"model A {}", "0.11.0") == stamp + assert mod.stamp_value(b"model B {}", "0.11.0") != stamp + assert mod.stamp_value(b"model A {}", "0.12.0") != stamp + + +def test_skip_requires_a_matching_stamp(tmp_path): + stamp = tmp_path / "stamp" + expected = mod.stamp_value(b"schema", "0.11.0") + assert mod.should_skip(stamp, expected, client_generated=True) is False + stamp.write_text(expected) + assert mod.should_skip(stamp, expected, client_generated=True) is True + assert ( + mod.should_skip(stamp, mod.stamp_value(b"other", "0.11.0"), client_generated=True) + is False + ) + + +def test_skip_requires_a_generated_client_even_with_a_matching_stamp(tmp_path): + stamp = tmp_path / "stamp" + expected = mod.stamp_value(b"schema", "0.11.0") + stamp.write_text(expected) + assert mod.should_skip(stamp, expected, client_generated=False) is False diff --git a/tests/test_litellm/test_ruff_strict_gate.py b/tests/test_litellm/test_ruff_strict_gate.py index ec8f49730dd..aad0e1bc9f9 100644 --- a/tests/test_litellm/test_ruff_strict_gate.py +++ b/tests/test_litellm/test_ruff_strict_gate.py @@ -94,3 +94,19 @@ def test_introduced_keeps_only_violations_on_changed_lines(): @pytest.mark.parametrize("hunk", ["@@ -1 +1 @@", "@@ -1,0 +1,2 @@"]) def test_parse_changed_lines_handles_single_and_ranged_hunks(hunk): assert gate.parse_changed_lines(f"+++ b/litellm/a.py\n{hunk}\n")["litellm/a.py"] + + +def test_over_ceiling_flags_only_counts_above_the_limit(): + budget = rule("C901", 10) + assert gate.over_ceiling({"C901": 10}, budget) == frozenset() + assert gate.over_ceiling({"C901": 11}, budget) == frozenset({"C901"}) + assert gate.over_ceiling({}, budget) == frozenset() + + +def test_over_ceiling_ignores_rules_missing_from_the_budget(): + assert gate.over_ceiling({"NEW99": 100}, rule("C901", 10)) == frozenset() + + +def test_over_ceiling_is_independent_across_rules(): + budget = {**rule("ANN001", 150), **rule("C901", 10)} + assert gate.over_ceiling({"ANN001": 130, "C901": 11}, budget) == frozenset({"C901"}) diff --git a/tests/test_litellm/test_type_check_gate.py b/tests/test_litellm/test_type_check_gate.py index e602bf6e66f..1be9fffe1bc 100644 --- a/tests/test_litellm/test_type_check_gate.py +++ b/tests/test_litellm/test_type_check_gate.py @@ -173,3 +173,100 @@ def test_empty_basedpyright_payload_counts_zero(): # Empty (not malformed) output parses to zero; the vacuous-run guard, not the # parser, is what rejects an empty run. assert gate.count_basedpyright("") == {} + + +def test_over_ceiling_flags_only_rules_above_their_limit(): + budget = {"reportAny": {"limit": 10}} + assert gate.over_ceiling({"reportAny": 10}, budget) == frozenset() + assert gate.over_ceiling({"reportAny": 11}, budget) == frozenset({"reportAny"}) + assert gate.over_ceiling({}, budget) == frozenset() + + +def test_over_ceiling_holds_unbudgeted_rules_to_the_default_limit(): + assert gate.over_ceiling({"brand-new": gate.DEFAULT_LIMIT}, {}) == frozenset() + assert gate.over_ceiling({"brand-new": gate.DEFAULT_LIMIT + 1}, {}) == frozenset( + {"brand-new"} + ) + + +def test_over_ceiling_is_independent_across_rules(): + budget = {"reportAny": {"limit": 10}, "reportArgumentType": {"limit": 5}} + assert gate.over_ceiling( + {"reportAny": 9, "reportArgumentType": 6}, budget + ) == frozenset({"reportArgumentType"}) + + +def test_cache_key_changes_with_base_point_and_each_fingerprint(): + key = gate.cache_key("abc", ("cfg", "lock")) + assert gate.cache_key("abc", ("cfg", "lock")) == key + assert gate.cache_key("def", ("cfg", "lock")) != key + assert gate.cache_key("abc", ("cfg2", "lock")) != key + assert gate.cache_key("abc", ("cfg", "lock2")) != key + + +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}) + assert gate.load_cached_counts(path) == {"reportAny": 3, "reportCall": 1} + + +def test_missing_corrupt_or_misshapen_cache_reads_as_none(tmp_path): + path = tmp_path / "cache.json" + assert gate.load_cached_counts(path) is None + path.write_text("{not json") + assert gate.load_cached_counts(path) is None + path.write_text(json.dumps(["counts"])) + assert gate.load_cached_counts(path) is None + path.write_text(json.dumps({"base_point": "abc"})) + assert gate.load_cached_counts(path) is None + path.write_text(json.dumps({"counts": {"reportAny": "three"}})) + assert gate.load_cached_counts(path) is None + path.write_text(json.dumps({"counts": {"reportAny": True}})) + assert gate.load_cached_counts(path) is None + + +def test_store_prunes_entries_for_other_branch_points(tmp_path): + old = gate.cache_path(tmp_path, "old", ("f",)) + gate.store_counts(tmp_path, old, "old", {"reportAny": 1}) + new = gate.cache_path(tmp_path, "new", ("f",)) + gate.store_counts(tmp_path, new, "new", {"reportAny": 2}) + assert not old.exists() + assert gate.load_cached_counts(new) == {"reportAny": 2} + + +def test_base_counts_cached_returns_the_hit_without_recomputing(tmp_path): + path = gate.cache_path(tmp_path, "abc123", gate.environment_fingerprints()) + gate.store_counts(tmp_path, path, "abc123", {"reportAny": 7}) + + def explode(ref): + raise AssertionError("a cache hit must not re-run the base pass") + + assert gate.base_counts_cached("abc123", cache_dir=tmp_path, compute=explode) == { + "reportAny": 7 + } + + +def test_base_counts_cached_computes_once_then_hits(tmp_path): + calls = [] + + def fake(ref): + calls.append(ref) + return {"reportAny": 4} + + first = gate.base_counts_cached("abc123", cache_dir=tmp_path, compute=fake) + second = gate.base_counts_cached("abc123", cache_dir=tmp_path, compute=fake) + assert first == second == {"reportAny": 4} + assert calls == ["abc123"] + + +def test_an_empty_base_pass_is_never_cached(tmp_path): + calls = [] + + def crashed(ref): + calls.append(ref) + return {} + + assert gate.base_counts_cached("abc123", cache_dir=tmp_path, compute=crashed) == {} + assert gate.base_counts_cached("abc123", cache_dir=tmp_path, compute=crashed) == {} + assert calls == ["abc123", "abc123"] + assert list(tmp_path.iterdir()) == [] From 9689595e4a462b7b6e006fb4bb84c91018166941 Mon Sep 17 00:00:00 2001 From: mateo-berri <277851410+mateo-berri@users.noreply.github.com> Date: Fri, 3 Jul 2026 01:14:15 +0000 Subject: [PATCH 2/2] fix(lint): keep the base-cache scratch file out of the prune glob The tmp+rename scratch in store_counts was named basedpyright-base-.json.tmp, which the stale-entry prune glob (basedpyright-base-*) also matches, so a concurrent lint run from another worktree sharing the same git common dir could unlink it between write_text and replace and crash the gate with FileNotFoundError. The scratch is now dot-prefixed so the glob can never see it, pid-suffixed so concurrent writers of the same entry never share a scratch, and the prune glob is restricted to committed *.json entries --- scripts/type_check_gate.py | 13 +++++++++++-- tests/test_litellm/test_type_check_gate.py | 17 +++++++++++++++++ 2 files changed, 28 insertions(+), 2 deletions(-) diff --git a/scripts/type_check_gate.py b/scripts/type_check_gate.py index e2ab12dfb63..2c5306cec7d 100644 --- a/scripts/type_check_gate.py +++ b/scripts/type_check_gate.py @@ -34,6 +34,7 @@ import contextlib import hashlib import json +import os import shutil import subprocess import sys @@ -193,14 +194,22 @@ def load_cached_counts(path: Path) -> dict[str, int] | 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}*"): + for stale in directory.glob(f"{CACHE_FILE_PREFIX}*.json"): if stale != path: stale.unlink(missing_ok=True) - scratch = path.with_name(path.name + ".tmp") + scratch = scratch_path(path) scratch.write_text( json.dumps( {"base_point": base_point, "counts": dict(sorted(counts.items()))}, diff --git a/tests/test_litellm/test_type_check_gate.py b/tests/test_litellm/test_type_check_gate.py index 1be9fffe1bc..66a28360af9 100644 --- a/tests/test_litellm/test_type_check_gate.py +++ b/tests/test_litellm/test_type_check_gate.py @@ -225,6 +225,23 @@ def test_missing_corrupt_or_misshapen_cache_reads_as_none(tmp_path): assert gate.load_cached_counts(path) is None +def test_scratch_is_invisible_to_the_prune_glob(): + import fnmatch + + scratch = gate.scratch_path(gate.cache_path(Path("/c"), "abc", ("f",))) + assert not fnmatch.fnmatch(scratch.name, f"{gate.CACHE_FILE_PREFIX}*") + + +def test_store_prune_spares_a_concurrent_runs_in_flight_scratch(tmp_path): + foreign = gate.scratch_path(gate.cache_path(tmp_path, "other", ("f",))) + foreign.parent.mkdir(parents=True, exist_ok=True) + foreign.write_text("{}") + mine = gate.cache_path(tmp_path, "mine", ("f",)) + gate.store_counts(tmp_path, mine, "mine", {"reportAny": 1}) + assert foreign.exists() + assert gate.load_cached_counts(mine) == {"reportAny": 1} + + def test_store_prunes_entries_for_other_branch_points(tmp_path): old = gate.cache_path(tmp_path, "old", ("f",)) gate.store_counts(tmp_path, old, "old", {"reportAny": 1})