mirror of
https://github.com/BerriAI/litellm.git
synced 2026-08-28 05:25:59 +00:00
fix(lint): let the ratchet guard recognise a graduated rule
A budget rule that graduates into a config's hard-fail select list rightly leaves the budget file, but the ratchet guard read any disappearance as a silently raised ceiling. Teach it the pairing between ruff-strict-budget.json and ruff.toml: a dropped rule is excused only when the paired config's lint.extend-select (minus lint.ignore) now hard-fails it, so deleting a rule without graduating it still trips the guard.
This commit is contained in:
parent
f304b7b19f
commit
5cd027cbbc
2 changed files with 89 additions and 6 deletions
|
|
@ -10,7 +10,10 @@ content at the merge-base with the target branch and fails (exits 1, red) if:
|
|||
* a rule was dropped from a budget (its ceiling effectively became infinite), or
|
||||
* an entire budget file was deleted.
|
||||
|
||||
New rules and lowered/equal limits are fine.
|
||||
New rules and lowered/equal limits are fine. So is a rule that graduated: once a
|
||||
paired config (ruff.toml for the ruff-strict budget) selects the rule outright it
|
||||
hard-fails at the first violation, which is stricter than any ceiling the budget
|
||||
could hold, so dropping its entry tightens the guard rather than removing it.
|
||||
|
||||
This is deliberately NOT a gating check. It should turn the run red so that a
|
||||
loosening is impossible to miss in review, but it must stay OUT of the
|
||||
|
|
@ -30,7 +33,9 @@ import argparse
|
|||
import json
|
||||
import subprocess
|
||||
import sys
|
||||
import tomllib
|
||||
from pathlib import Path
|
||||
from types import MappingProxyType
|
||||
from typing import NamedTuple
|
||||
|
||||
REPO_ROOT = Path(__file__).resolve().parent.parent
|
||||
|
|
@ -40,6 +45,7 @@ DEFAULT_BUDGETS: tuple[str, ...] = (
|
|||
"type-discipline-budget.json",
|
||||
"basedpyright-code-budget.json",
|
||||
)
|
||||
GRADUATION_CONFIGS = MappingProxyType({"ruff-strict-budget.json": "ruff.toml"})
|
||||
|
||||
|
||||
class Regression(NamedTuple):
|
||||
|
|
@ -106,24 +112,57 @@ def _limits(budget: dict) -> dict[str, int]:
|
|||
}
|
||||
|
||||
|
||||
def selectors_hard_failed_by(lint: dict) -> tuple[str, ...]:
|
||||
"""A ruff `[lint]` table's selected codes, minus anything `ignore` turns back off.
|
||||
|
||||
`lint.ignore` wins over `lint.extend-select` in ruff, so an ignored code is not
|
||||
actually enforced and must not count as a graduation.
|
||||
"""
|
||||
ignored = tuple(lint.get("ignore", ()))
|
||||
return tuple(
|
||||
selector
|
||||
for selector in lint.get("extend-select", ())
|
||||
if not (ignored and selector.startswith(ignored))
|
||||
)
|
||||
|
||||
|
||||
def graduated_selectors(rel: str) -> tuple[str, ...]:
|
||||
"""Selectors the budget's paired ruff config hard-fails, so its ceiling is moot."""
|
||||
config = GRADUATION_CONFIGS.get(rel)
|
||||
if config is None or not (REPO_ROOT / config).exists():
|
||||
return ()
|
||||
return selectors_hard_failed_by(
|
||||
tomllib.loads((REPO_ROOT / config).read_text()).get("lint", {})
|
||||
)
|
||||
|
||||
|
||||
def _regression_detail(
|
||||
rule: str,
|
||||
base_limits: dict[str, int],
|
||||
head_limits: dict[str, int],
|
||||
graduated: tuple[str, ...],
|
||||
) -> str | None:
|
||||
"""Why `rule` regressed vs base, or None when it held flat or fell.
|
||||
"""Why `rule` regressed vs base, or None when it held flat, fell, or graduated.
|
||||
|
||||
A dropped rule is terminal; otherwise the only loosening left is a raised limit.
|
||||
A dropped rule is terminal unless it graduated; otherwise the only loosening
|
||||
left is a raised limit.
|
||||
"""
|
||||
base_limit = base_limits[rule]
|
||||
if rule not in head_limits:
|
||||
if graduated and rule.startswith(graduated):
|
||||
return None
|
||||
return f"rule dropped (limit {base_limit} -> removed)"
|
||||
if head_limits[rule] > base_limit:
|
||||
return f"limit raised {base_limit} -> {head_limits[rule]}"
|
||||
return None
|
||||
|
||||
|
||||
def regressions_for(rel: str, base: dict | None, head: dict | None) -> list[Regression]:
|
||||
def regressions_for(
|
||||
rel: str,
|
||||
base: dict | None,
|
||||
head: dict | None,
|
||||
graduated: tuple[str, ...] = (),
|
||||
) -> list[Regression]:
|
||||
if base is None:
|
||||
return [] # new budget file: nothing to ratchet against yet
|
||||
if head is None:
|
||||
|
|
@ -133,7 +172,7 @@ def regressions_for(rel: str, base: dict | None, head: dict | None) -> list[Regr
|
|||
return [
|
||||
Regression(rel, rule, detail)
|
||||
for rule in sorted(base_limits)
|
||||
if (detail := _regression_detail(rule, base_limits, head_limits)) is not None
|
||||
if (detail := _regression_detail(rule, base_limits, head_limits, graduated)) is not None
|
||||
]
|
||||
|
||||
|
||||
|
|
@ -164,7 +203,7 @@ def main() -> int:
|
|||
print(f"skip {rel}: new file (no base at {args.base} to ratchet against)")
|
||||
continue
|
||||
checked.append(rel)
|
||||
regressions.extend(regressions_for(rel, base, head))
|
||||
regressions.extend(regressions_for(rel, base, head, graduated_selectors(rel)))
|
||||
|
||||
if regressions:
|
||||
print(
|
||||
|
|
|
|||
|
|
@ -68,6 +68,50 @@ def test_new_rule_in_head_is_clean():
|
|||
assert ratchet.regressions_for("b.json", {}, {"new-rule": _spec_of(5)}) == []
|
||||
|
||||
|
||||
def test_dropped_rule_that_graduated_to_a_hard_failing_config_is_clean():
|
||||
base = {"UP006": _spec_of(0)}
|
||||
assert ratchet.regressions_for("b.json", base, {}, graduated=("UP006",)) == []
|
||||
|
||||
|
||||
def test_graduation_matches_by_prefix_like_ruff_selectors_do():
|
||||
base = {"ANN202": _spec_of(865)}
|
||||
assert ratchet.regressions_for("b.json", base, {}, graduated=("ANN",)) == []
|
||||
|
||||
|
||||
def test_an_unrelated_graduation_does_not_excuse_a_dropped_rule():
|
||||
base = {"C901": _spec_of(3)}
|
||||
regs = ratchet.regressions_for("b.json", base, {}, graduated=("UP006", "SIM118"))
|
||||
assert [r.rule for r in regs] == ["C901"]
|
||||
assert "dropped" in regs[0].detail
|
||||
|
||||
|
||||
def test_graduation_never_excuses_a_raised_limit():
|
||||
base = {"UP006": _spec_of(0)}
|
||||
regs = ratchet.regressions_for("b.json", base, {"UP006": _spec_of(7)}, graduated=("UP006",))
|
||||
assert [r.rule for r in regs] == ["UP006"]
|
||||
assert "0 -> 7" in regs[0].detail
|
||||
|
||||
|
||||
def test_graduated_selectors_come_from_the_paired_ruff_config():
|
||||
selectors = ratchet.graduated_selectors("ruff-strict-budget.json")
|
||||
assert "UP006" in selectors
|
||||
assert "ANN" not in selectors
|
||||
|
||||
|
||||
def test_budgets_without_a_paired_config_can_never_graduate():
|
||||
assert ratchet.graduated_selectors("type-discipline-budget.json") == ()
|
||||
assert ratchet.graduated_selectors("basedpyright-code-budget.json") == ()
|
||||
|
||||
|
||||
def test_a_selector_the_config_also_ignores_does_not_count_as_graduated():
|
||||
lint = {"ignore": ["UP006"], "extend-select": ["UP006", "SIM118"]}
|
||||
assert ratchet.selectors_hard_failed_by(lint) == ("SIM118",)
|
||||
|
||||
|
||||
def test_selectors_hard_failed_by_reads_a_config_with_no_ignore_list():
|
||||
assert ratchet.selectors_hard_failed_by({"extend-select": ["UP006"]}) == ("UP006",)
|
||||
|
||||
|
||||
def test_deleted_budget_file_is_a_regression():
|
||||
regs = ratchet.regressions_for("b.json", {"LIT006": _spec_of(1)}, None)
|
||||
assert [r.rule for r in regs] == ["*"]
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue