fix(ci): harden lint budget gates against crashed and vacuous base passes

The basedpyright budget gate discarded the base pass's exit code and stderr,
so an OOM-killed base pass surfaced only as an opaque "almost certainly
crashed" failure. The gate now checks the exit code, retries the base pass
once with the crash evidence written to stderr, and only then fails. The ruff
strict gate and the LIT type-discipline gate had no vacuous-run guard at all:
an empty head scan would certify silently and an empty base scan would blame
the branch for every pre-existing violation. Both now refuse vacuous runs the
same way the basedpyright gate does. cmd_check in all three gates takes its
collaborators as injectable parameters so the verdict wiring is unit-testable
without monkeypatching.
This commit is contained in:
mateo-berri 2026-07-22 20:05:02 -07:00
parent abf18f8760
commit db852f37ec
6 changed files with 391 additions and 23 deletions

View file

@ -17,6 +17,7 @@ import subprocess
import sys
import tempfile
from collections import Counter
from collections.abc import Callable, Mapping
from pathlib import Path
from typing import NamedTuple
@ -50,6 +51,10 @@ def _run(cmd: list, cwd: Path = REPO_ROOT) -> str:
return proc.stdout
def _merge_base(base: str) -> str:
return _run(["git", "merge-base", base, "HEAD"]).strip() or base
def _ruff_json(cwd: Path, config: Path) -> list:
raw = _run(
["ruff", "check", TARGET, "--config", str(config), "--output-format", "json"],
@ -101,6 +106,16 @@ def over_ceiling(head: dict, budget: dict) -> frozenset:
)
def is_vacuous_run(
counts: Mapping[str, int], budget: Mapping[str, Mapping[str, int]]
) -> bool:
"""True when nothing was counted but the budget expects violations -- the
signature of a scan that crashed or whose output failed to parse. Without
this guard an empty head scan would clear every limit and pass silently,
and an empty base scan would make every head violation look freshly added."""
return not counts and any(spec["limit"] for spec in budget.values())
def evaluate(head: dict, base: dict, budget: dict) -> list:
breaches = []
for rule, spec in budget.items():
@ -128,15 +143,37 @@ def introduced(violations: list, changed: dict) -> list:
return [v for v in violations if v.line in changed.get(v.file, set())]
def cmd_check(base: str) -> None:
budget = json.loads(BUDGET_PATH.read_text())
head = head_violations()
def cmd_check(
base: str,
violations: Callable[[], list] = head_violations,
base_counts_for: Callable[[str], dict] = base_counts,
merge_base: Callable[[str], str] = _merge_base,
budget_path: Path = BUDGET_PATH,
) -> None:
budget = json.loads(budget_path.read_text())
head = violations()
head_counts = count_by_rule(head)
if is_vacuous_run(head_counts, budget):
expected = sum(spec["limit"] for spec in budget.values())
print(
f"FAIL: ruff reported no strict-rule violations, but {budget_path.name} "
f"allows up to ~{expected}. The scan almost certainly crashed or emitted "
f"nothing; refusing to certify a vacuous run."
)
raise SystemExit(1)
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(head_counts, base_counts(base_point), budget)
base_point = merge_base(base)
base_totals = base_counts_for(base_point)
if is_vacuous_run(base_totals, budget):
print(
f"FAIL: ruff reported no strict-rule violations for the base tree at "
f"{base_point[:12]}, so every rule would look freshly added. The base scan "
f"almost certainly crashed; refusing to blame this change for it."
)
raise SystemExit(1)
breaches = evaluate(head_counts, base_totals, budget)
if not breaches:
print(f"OK: every strict rule is within its codebase ceiling (base {base})")
return
@ -182,7 +219,7 @@ def cmd_update(base_ref: str = DEFAULT_BASE) -> None:
fixes tighten its own ceilings by exactly what they cleared since it diverged.
"""
budget = json.loads(BUDGET_PATH.read_text())
base_point = _run(["git", "merge-base", base_ref, "HEAD"]).strip() or base_ref
base_point = _merge_base(base_ref)
updated = ratcheted_budget(
budget, count_by_rule(head_violations()), base_counts(base_point)
)

View file

@ -59,6 +59,8 @@ UNCODED = "<uncoded>"
# fails once it clears this many errors.
DEFAULT_LIMIT = 10
BASE_PASS_ATTEMPTS = 2
class Breach(NamedTuple):
code: str
@ -107,6 +109,10 @@ def _run(cmd: list[str], cwd: Path = REPO_ROOT) -> str:
return proc.stdout
def _merge_base(base_ref: str) -> str:
return _run(["git", "merge-base", base_ref, "HEAD"]).strip() or base_ref
@contextlib.contextmanager
def _temp_worktree(ref: str) -> Iterator[Path]:
parent = Path(tempfile.mkdtemp(prefix="bpr_base_"))
@ -124,6 +130,24 @@ def _temp_worktree(ref: str) -> Iterator[Path]:
shutil.rmtree(parent, ignore_errors=True)
def run_base_pass(
run: Callable[[], "subprocess.CompletedProcess[str]"],
attempts: int = BASE_PASS_ATTEMPTS,
) -> "subprocess.CompletedProcess[str]":
for attempt in range(1, attempts + 1):
proc = run()
if proc.returncode in (0, 1):
return proc
sys.stderr.write(
f"basedpyright base pass exited {proc.returncode} "
f"(attempt {attempt}/{attempts}); stderr tail:\n{proc.stderr[-2000:]}\n"
)
raise SystemExit(
f"basedpyright base pass crashed {attempts} times; its exit code and "
f"stderr are above"
)
def base_counts(ref: str) -> dict[str, int]:
"""basedpyright error counts per rule for the merge-base tree. The head
config is copied in so the base is judged by today's rules, and the run uses
@ -131,8 +155,10 @@ def base_counts(ref: str) -> dict[str, int]:
exe = shutil.which("basedpyright") or "basedpyright"
with _temp_worktree(ref) as worktree:
shutil.copy(PYRIGHT_CONFIG, worktree / "pyrightconfig.json")
proc = subprocess.run(
[exe, "--outputjson"], cwd=worktree, capture_output=True, text=True
proc = run_base_pass(
lambda: subprocess.run(
[exe, "--outputjson"], cwd=worktree, capture_output=True, text=True
)
)
return count_basedpyright(proc.stdout, root=worktree)
@ -295,7 +321,7 @@ def cmd_update(current: Mapping[str, int], base_ref: str = DEFAULT_BASE) -> None
by exactly what they cleared since it diverged, and limits never rise.
"""
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
base_point = _merge_base(base_ref)
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)
@ -305,13 +331,19 @@ def cmd_update(current: Mapping[str, int], base_ref: str = DEFAULT_BASE) -> None
)
def cmd_check(base_ref: str) -> None:
budget = json.loads(BUDGET_PATH.read_text())
head = count_basedpyright(sys.stdin.read())
def cmd_check(
base_ref: str,
head_payload: Callable[[], str] = sys.stdin.read,
base_counts_for: Callable[[str], dict[str, int]] = base_counts_cached,
merge_base: Callable[[str], str] = _merge_base,
budget_path: Path = BUDGET_PATH,
) -> None:
budget = json.loads(budget_path.read_text())
head = count_basedpyright(head_payload())
if is_vacuous_run(head, budget):
expected = sum(spec["limit"] for spec in budget.values())
print(
f"FAIL: basedpyright produced no errors, but {BUDGET_PATH.name} allows "
f"FAIL: basedpyright produced no errors, but {budget_path.name} allows "
f"up to ~{expected}. The type checker almost certainly crashed or emitted "
f"nothing; refusing to certify a vacuous run."
)
@ -321,8 +353,8 @@ def cmd_check(base_ref: str) -> None:
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_cached(base_point)
base_point = merge_base(base_ref)
base = base_counts_for(base_point)
if is_vacuous_run(base, budget):
print(
f"FAIL: basedpyright produced no errors for the base tree at "

View file

@ -27,6 +27,7 @@ import subprocess
import sys
import tempfile
from collections import Counter
from collections.abc import Callable, Mapping
from pathlib import Path
from typing import NamedTuple
@ -61,6 +62,10 @@ def _run(cmd: list, cwd: Path = REPO_ROOT) -> str:
return proc.stdout
def _merge_base(base: str) -> str:
return _run(["git", "merge-base", base, "HEAD"]).strip() or base
def _check(root: Path, checker: Path) -> list:
# Resolve root first: on macOS tempfile dirs (/var/...) resolve to /private/var/...,
# and the checker prints already-resolved absolute paths, so relative_to would fail.
@ -118,6 +123,17 @@ def over_ceiling(head: dict, budget: dict) -> frozenset:
)
def is_vacuous_run(
counts: Mapping[str, int], budget: Mapping[str, Mapping[str, int]]
) -> bool:
"""True when nothing was counted but the budget expects violations -- the
signature of a checker pass that crashed or whose output failed to parse
(say, after an output-format change). Without this guard an empty head pass
would clear every limit and pass silently, and an empty base pass would make
every head violation look freshly added."""
return not counts and any(spec["limit"] for spec in budget.values())
def evaluate(head: dict, base: dict, budget: dict) -> list:
breaches = []
for rule, spec in budget.items():
@ -145,15 +161,37 @@ def introduced(violations: list, changed: dict) -> list:
return [v for v in violations if v.line in changed.get(v.file, set())]
def cmd_check(base: str) -> None:
budget = json.loads(BUDGET_PATH.read_text())
head = head_violations()
def cmd_check(
base: str,
violations: Callable[[], list] = head_violations,
base_counts_for: Callable[[str], dict] = base_counts,
merge_base: Callable[[str], str] = _merge_base,
budget_path: Path = BUDGET_PATH,
) -> None:
budget = json.loads(budget_path.read_text())
head = violations()
head_counts = count_by_rule(head)
if is_vacuous_run(head_counts, budget):
expected = sum(spec["limit"] for spec in budget.values())
print(
f"FAIL: the LIT checker reported no violations, but {budget_path.name} "
f"allows up to ~{expected}. The pass almost certainly crashed or its "
f"output failed to parse; refusing to certify a vacuous run."
)
raise SystemExit(1)
if not over_ceiling(head_counts, budget):
print(f"OK: every LIT rule is within its codebase ceiling (base {base})")
return
base_point = _run(["git", "merge-base", base, "HEAD"]).strip() or base
breaches = evaluate(head_counts, base_counts(base_point), budget)
base_point = merge_base(base)
base_totals = base_counts_for(base_point)
if is_vacuous_run(base_totals, budget):
print(
f"FAIL: the LIT checker reported no violations for the base tree at "
f"{base_point[:12]}, so every rule would look freshly added. The base "
f"pass almost certainly crashed; refusing to blame this change for it."
)
raise SystemExit(1)
breaches = evaluate(head_counts, base_totals, budget)
if not breaches:
print(f"OK: every LIT rule is within its codebase ceiling (base {base})")
return
@ -203,7 +241,7 @@ def cmd_update(base_ref: str = DEFAULT_BASE) -> None:
fixes tighten its own ceilings by exactly what they cleared since it diverged.
"""
budget = json.loads(BUDGET_PATH.read_text())
base_point = _run(["git", "merge-base", base_ref, "HEAD"]).strip() or base_ref
base_point = _merge_base(base_ref)
updated = ratcheted_budget(
budget, count_by_rule(head_violations()), base_counts(base_point)
)

View file

@ -110,3 +110,73 @@ def test_over_ceiling_ignores_rules_missing_from_the_budget():
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"})
def test_no_violations_against_a_nonempty_budget_is_vacuous():
assert gate.is_vacuous_run({}, rule("ANN001", 110)) is True
def test_genuine_zero_counts_are_not_vacuous():
assert gate.is_vacuous_run({}, {}) is False
assert gate.is_vacuous_run({}, rule("ANN001", 0)) is False
assert gate.is_vacuous_run({"ANN001": 1}, rule("ANN001", 110)) is False
def _violations(code, count):
return [Violation("litellm/a.py", line, code) for line in range(1, count + 1)]
def _raise_if_called(*args):
raise AssertionError("must not be called")
def _budget_file(tmp_path, limit):
path = tmp_path / "budget.json"
path.write_text(f'{{"ANN001": {{"limit": {limit}}}}}')
return path
def test_check_rejects_a_vacuous_head_scan(tmp_path, capsys):
with pytest.raises(SystemExit):
gate.cmd_check(
"origin/main",
violations=lambda: [],
base_counts_for=_raise_if_called,
merge_base=_raise_if_called,
budget_path=_budget_file(tmp_path, 110),
)
assert "vacuous" in capsys.readouterr().out
def test_check_within_ceiling_skips_the_base_scan(tmp_path, capsys):
gate.cmd_check(
"origin/main",
violations=lambda: _violations("ANN001", 3),
base_counts_for=_raise_if_called,
merge_base=_raise_if_called,
budget_path=_budget_file(tmp_path, 110),
)
assert capsys.readouterr().out.startswith("OK")
def test_check_refuses_to_blame_the_change_for_a_vacuous_base_scan(tmp_path, capsys):
with pytest.raises(SystemExit):
gate.cmd_check(
"origin/main",
violations=lambda: _violations("ANN001", 3),
base_counts_for=lambda ref: {},
merge_base=lambda base: "a" * 40,
budget_path=_budget_file(tmp_path, 2),
)
assert "refusing to blame" in capsys.readouterr().out
def test_check_spares_a_bystander_whose_base_matches_head(tmp_path, capsys):
gate.cmd_check(
"origin/main",
violations=lambda: _violations("ANN001", 3),
base_counts_for=lambda ref: {"ANN001": 3},
merge_base=lambda base: "a" * 40,
budget_path=_budget_file(tmp_path, 2),
)
assert capsys.readouterr().out.startswith("OK")

View file

@ -1,7 +1,10 @@
import importlib.util
import json
import subprocess
from pathlib import Path
import pytest
_MODULE_PATH = Path(__file__).resolve().parents[2] / "scripts" / "type_check_gate.py"
_spec = importlib.util.spec_from_file_location("type_check_gate", _MODULE_PATH)
gate = importlib.util.module_from_spec(_spec)
@ -163,8 +166,6 @@ def test_update_clamps_a_limit_at_zero_never_negative():
def test_malformed_basedpyright_json_exits_loudly_not_as_zero_errors():
import pytest
with pytest.raises(SystemExit):
gate.count_basedpyright("startup warning\n{not json")
@ -287,3 +288,121 @@ def test_an_empty_base_pass_is_never_cached(tmp_path):
assert gate.base_counts_cached("abc123", cache_dir=tmp_path, compute=crashed) == {}
assert calls == ["abc123", "abc123"]
assert list(tmp_path.iterdir()) == []
def _proc(returncode, stdout="", stderr=""):
return subprocess.CompletedProcess(["basedpyright"], returncode, stdout, stderr)
@pytest.mark.parametrize("code", [0, 1])
def test_base_pass_accepts_clean_and_errorful_exit_codes_without_retry(code):
calls = []
def run():
calls.append(code)
return _proc(code, stdout="{}")
assert gate.run_base_pass(run).returncode == code
assert calls == [code]
@pytest.mark.parametrize("code", [2, 137, -9])
def test_base_pass_retries_after_a_crash_and_reports_the_evidence(code, capsys):
procs = iter([_proc(code, stderr="node blew up"), _proc(0, stdout="{}")])
assert gate.run_base_pass(lambda: next(procs)).returncode == 0
err = capsys.readouterr().err
assert f"exited {code}" in err
assert "node blew up" in err
def test_base_pass_that_keeps_crashing_exits_loudly_not_as_zero_counts(capsys):
attempts = []
def crash():
attempts.append(1)
return _proc(134, stderr="JavaScript heap out of memory")
with pytest.raises(SystemExit):
gate.run_base_pass(crash)
assert len(attempts) == gate.BASE_PASS_ATTEMPTS
err = capsys.readouterr().err
assert err.count("exited 134") == gate.BASE_PASS_ATTEMPTS
assert "JavaScript heap out of memory" in err
def _payload(rule, count):
return json.dumps(
{
"generalDiagnostics": [
_bpr(f"{ROOT}/litellm/x.py", "error", rule) for _ in range(count)
]
}
)
def _raise_if_called(*args):
raise AssertionError("must not be called")
def _budget_file(tmp_path, limit):
path = tmp_path / "budget.json"
path.write_text(json.dumps({"reportAny": {"limit": limit}}))
return path
def test_check_rejects_a_vacuous_head_run_before_touching_the_base(tmp_path, capsys):
with pytest.raises(SystemExit):
gate.cmd_check(
"origin/main",
head_payload=lambda: "",
base_counts_for=_raise_if_called,
merge_base=_raise_if_called,
budget_path=_budget_file(tmp_path, 5),
)
assert "vacuous" in capsys.readouterr().out
def test_check_within_limits_passes_without_a_base_pass(tmp_path, capsys):
gate.cmd_check(
"origin/main",
head_payload=lambda: _payload("reportAny", 3),
base_counts_for=_raise_if_called,
merge_base=_raise_if_called,
budget_path=_budget_file(tmp_path, 5),
)
assert capsys.readouterr().out.startswith("OK")
def test_check_refuses_to_blame_the_change_for_a_vacuous_base_pass(tmp_path, capsys):
with pytest.raises(SystemExit):
gate.cmd_check(
"origin/main",
head_payload=lambda: _payload("reportAny", 6),
base_counts_for=lambda ref: {},
merge_base=lambda base: "a" * 40,
budget_path=_budget_file(tmp_path, 5),
)
assert "refusing to blame" in capsys.readouterr().out
def test_check_spares_a_bystander_whose_base_matches_head(tmp_path, capsys):
gate.cmd_check(
"origin/main",
head_payload=lambda: _payload("reportAny", 6),
base_counts_for=lambda ref: {"reportAny": 6},
merge_base=lambda base: "a" * 40,
budget_path=_budget_file(tmp_path, 5),
)
assert capsys.readouterr().out.startswith("OK")
def test_check_fails_a_change_that_grew_a_rule_past_its_limit(tmp_path, capsys):
with pytest.raises(SystemExit):
gate.cmd_check(
"origin/main",
head_payload=lambda: _payload("reportAny", 6),
base_counts_for=lambda ref: {"reportAny": 4},
merge_base=lambda base: "a" * 40,
budget_path=_budget_file(tmp_path, 5),
)
assert "BREACHED RULES" in capsys.readouterr().out

View file

@ -8,6 +8,8 @@ drift-safe breach check). Both are pinned here.
import importlib.util
from pathlib import Path
import pytest
_MODULE_PATH = Path(__file__).resolve().parents[2] / "scripts" / "type_discipline_gate.py"
_spec = importlib.util.spec_from_file_location("type_discipline_gate", _MODULE_PATH)
gate = importlib.util.module_from_spec(_spec)
@ -50,3 +52,73 @@ def test_update_ratchets_limit_down_by_what_the_branch_fixed_never_up():
"LIT001": {"limit": 85},
"LIT006": {"limit": 10},
}
def test_no_violations_against_a_nonempty_budget_is_vacuous():
assert gate.is_vacuous_run({}, _budget(12)) is True
def test_genuine_zero_counts_are_not_vacuous():
assert gate.is_vacuous_run({}, {}) is False
assert gate.is_vacuous_run({}, _budget(0)) is False
assert gate.is_vacuous_run({"LIT006": 1}, _budget(12)) is False
def _violations(code, count):
return [gate.Violation("litellm/a.py", line, code) for line in range(1, count + 1)]
def _raise_if_called(*args):
raise AssertionError("must not be called")
def _budget_file(tmp_path, limit):
path = tmp_path / "budget.json"
path.write_text(f'{{"LIT006": {{"limit": {limit}}}}}')
return path
def test_check_rejects_a_vacuous_head_pass(tmp_path, capsys):
with pytest.raises(SystemExit):
gate.cmd_check(
"origin/main",
violations=lambda: [],
base_counts_for=_raise_if_called,
merge_base=_raise_if_called,
budget_path=_budget_file(tmp_path, 12),
)
assert "vacuous" in capsys.readouterr().out
def test_check_within_ceiling_skips_the_base_pass(tmp_path, capsys):
gate.cmd_check(
"origin/main",
violations=lambda: _violations("LIT006", 3),
base_counts_for=_raise_if_called,
merge_base=_raise_if_called,
budget_path=_budget_file(tmp_path, 12),
)
assert capsys.readouterr().out.startswith("OK")
def test_check_refuses_to_blame_the_change_for_a_vacuous_base_pass(tmp_path, capsys):
with pytest.raises(SystemExit):
gate.cmd_check(
"origin/main",
violations=lambda: _violations("LIT006", 3),
base_counts_for=lambda ref: {},
merge_base=lambda base: "a" * 40,
budget_path=_budget_file(tmp_path, 2),
)
assert "refusing to blame" in capsys.readouterr().out
def test_check_spares_a_bystander_whose_base_matches_head(tmp_path, capsys):
gate.cmd_check(
"origin/main",
violations=lambda: _violations("LIT006", 3),
base_counts_for=lambda ref: {"LIT006": 3},
merge_base=lambda base: "a" * 40,
budget_path=_budget_file(tmp_path, 2),
)
assert capsys.readouterr().out.startswith("OK")