revert(pre-commit): drop basedpyright --threads from the type-check gate

Threading looked like the big win here and did not hold up. Three problems,
any one of which disqualifies it:

It changes the answer. Partitioning files across threads reorders
order-dependent inferences, so the tree totals move with the width (149330
serial, 149325 threaded). A gate whose whole job is counting violations must
not have its verdict depend on the host.

There is no portable width. Pinning a constant keeps counts comparable but
tunes the gate for one machine. Letting it follow nproc is correct per host,
yet the width is in the cache fingerprint, so every distinct core count gets
its own key and nobody can reuse CI's precomputed base artifact. Everyone
then pays the cold path.

The speed was noise. On four cores, --threads 4 ranged 84-114s against
137-151s serial, a spread that swallows the median I originally quoted, and
on a contributor's laptop width 8 ran 724s against 113s serial, roughly 6x
slower, while making the machine unusable.

Serial restores the original cache key, so the existing base entry and CI
artifact stay valid. The surviving win in this branch is the memoized path
resolution, which is measurable in isolation and host-independent.
This commit is contained in:
Claude 2026-08-08 10:55:40 +00:00
parent 23b4a674ca
commit 2ee8adf242
No known key found for this signature in database
2 changed files with 20 additions and 31 deletions

View file

@ -24,19 +24,17 @@ 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.
Checking is parallelized across a *pinned* number of worker threads rather than
the host's core count. Threading is what makes the pass roughly 1.6x faster
(137s -> 87s over this tree on four cores), but partitioning files across
threads also reorders a handful of order-dependent inferences, so a few
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 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.
Checking runs single-threaded on purpose. basedpyright's ``--threads`` looks
like free speed and is not: partitioning files across threads reorders
order-dependent inferences, so the tree's totals move with the width (149330
serial, 149325 threaded here), which makes a counting gate's verdict a function
of the host. Pinning a width to keep counts comparable then decouples it from
the machine, and letting it follow ``nproc`` instead gives every core count its
own fingerprint, so no one could reuse CI's precomputed base artifact. The
speed did not survive measurement either: on four cores ``--threads 4`` ranged
84-114s against 137-151s serial, and on one contributor's laptop it was ~6x
*slower* than serial (724s vs 113s at width 8). Serial is the only width that
is both portable and comparable.
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
@ -106,8 +104,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"
BASEDPYRIGHT_THREADS: Final = 4
# Bucket for a basedpyright diagnostic with no `rule`. Counted so it's gated.
UNCODED = "<uncoded>"
@ -240,10 +236,9 @@ def run_basedpyright(cwd: Path = REPO_ROOT, env_dir: Path = TYPECHECK_ENV_DIR) -
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. `--threads` is pinned for the same
reason: it is a measurement parameter, not a local tuning knob. 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."""
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(
[
@ -251,8 +246,6 @@ def run_basedpyright(cwd: Path = REPO_ROOT, env_dir: Path = TYPECHECK_ENV_DIR) -
"--outputjson",
"--pythonpath",
str(bin_dir / "python"),
"--threads",
str(BASEDPYRIGHT_THREADS),
],
cwd=cwd,
capture_output=True,
@ -327,7 +320,6 @@ def over_ceiling(
def environment_fingerprints(
dep_groups: tuple[str, ...] = TYPECHECK_DEP_GROUPS,
threads: int = BASEDPYRIGHT_THREADS,
) -> tuple[str, ...]:
return (
*(
@ -336,7 +328,6 @@ def environment_fingerprints(
if path.exists()
),
"groups:" + ",".join(dep_groups),
f"threads:{threads}",
)

View file

@ -132,15 +132,18 @@ def test_run_basedpyright_pins_import_resolution_to_the_owned_env(tmp_path):
assert argv[argv.index("--pythonpath") + 1] == str(env_dir / "bin" / "python")
def test_run_basedpyright_pins_the_thread_width_instead_of_inheriting_the_hosts(tmp_path):
def test_run_basedpyright_stays_single_threaded(tmp_path):
"""`--threads` shifts the tree's totals (149330 serial vs 149325 threaded), which
would make this counting gate's verdict a function of the host, and it measured
slower than serial on real hardware. Neither failure is visible from a passing
run, so pin the absence of the flag."""
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("--threads") + 1] == str(gate.BASEDPYRIGHT_THREADS)
assert "--threads" not in captured.read_text().split()
def test_run_basedpyright_fails_loudly_on_a_crash_exit_code(tmp_path):
@ -299,11 +302,6 @@ def test_fingerprints_carry_the_dependency_group_set():
assert "groups:" + ",".join(gate.TYPECHECK_DEP_GROUPS) in gate.environment_fingerprints()
def test_fingerprints_carry_the_thread_width():
assert f"threads:{gate.BASEDPYRIGHT_THREADS}" in gate.environment_fingerprints()
assert gate.environment_fingerprints(threads=2) != gate.environment_fingerprints(threads=4)
def test_fingerprints_cover_the_prisma_schema():
schema_hash = hashlib.sha256(gate.PRISMA_SCHEMA.read_bytes()).hexdigest()
assert schema_hash in gate.environment_fingerprints()