Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 6 additions & 10 deletions ruff-strict-budget.json
Original file line number Diff line number Diff line change
@@ -1,27 +1,27 @@
{
"ANN001": {
"baseline": 2865,
"slack": 50
"slack": 287
},
"ANN002": {
"baseline": 64,
"slack": 5
},
"ANN003": {
"baseline": 759,
"slack": 30
"slack": 76
},
"ANN201": {
"baseline": 1944,
"slack": 50
"slack": 194
},
"ANN202": {
"baseline": 858,
"slack": 30
"slack": 86
},
"ANN204": {
"baseline": 658,
"slack": 20
"slack": 66
},
"ANN205": {
"baseline": 117,
Expand All @@ -33,7 +33,7 @@
},
"ANN401": {
"baseline": 1886,
"slack": 50
"slack": 189
},
"ASYNC230": {
"baseline": 11,
Expand Down Expand Up @@ -231,10 +231,6 @@
"baseline": 6,
"slack": 3
},
"PLR0913": {
"baseline": 1813,
"slack": 50
},
"PLR1704": {
"baseline": 3,
"slack": 3
Expand Down
2 changes: 1 addition & 1 deletion ruff-strict.toml
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ extend = "ruff.toml"

[lint]
preview = true
select = ["ANN", "ASYNC230", "B004", "B006", "B008", "B009", "B010", "B018", "B019", "B021", "B026", "B033", "BLE", "C401", "C404", "C405", "C408", "C414", "C419", "C901", "D419", "DTZ001", "DTZ003", "DTZ005", "DTZ006", "DTZ007", "DTZ011", "EXE001", "EXE002", "F401", "FURB136", "FURB168", "FURB188", "I001", "LOG015", "N999", "PERF102", "PERF401", "PERF402", "PERF403", "PIE790", "PIE800", "PIE804", "PIE810", "PLC0206", "PLC0208", "PLC0414", "PLR0124", "PLR0206", "PLR0402", "PLR0913", "PLR1704", "PLR1711", "PLR1714", "PLR1730", "PLR2044", "PLW0127", "PLW0133", "PLW0602", "PLW0603", "PLW1508", "PLW1510", "PYI030", "PYI036", "PYI041", "PYI064", "RET501", "RET504", "RUF010", "RUF012", "RUF015", "RUF019", "RUF022", "RUF023", "RUF046", "RUF051", "RUF059", "RUF100", "S110", "S112", "SIM101", "SIM102", "SIM103", "SIM113", "SIM114", "SIM115", "SIM117", "SIM118", "SIM201", "SIM210", "SIM211", "SIM222", "SIM401", "TC004", "TC005", "TID251", "TRY002", "TRY004", "TRY201", "TRY203", "TRY300", "UP006", "UP007", "UP008", "UP012", "UP018", "UP024", "UP028", "UP031", "UP032", "UP034", "UP035", "UP036", "UP037", "UP045"]
select = ["ANN", "ASYNC230", "B004", "B006", "B008", "B009", "B010", "B018", "B019", "B021", "B026", "B033", "BLE", "C401", "C404", "C405", "C408", "C414", "C419", "C901", "D419", "DTZ001", "DTZ003", "DTZ005", "DTZ006", "DTZ007", "DTZ011", "EXE001", "EXE002", "F401", "FURB136", "FURB168", "FURB188", "I001", "LOG015", "N999", "PERF102", "PERF401", "PERF402", "PERF403", "PIE790", "PIE800", "PIE804", "PIE810", "PLC0206", "PLC0208", "PLC0414", "PLR0124", "PLR0206", "PLR0402", "PLR1704", "PLR1711", "PLR1714", "PLR1730", "PLR2044", "PLW0127", "PLW0133", "PLW0602", "PLW0603", "PLW1508", "PLW1510", "PYI030", "PYI036", "PYI041", "PYI064", "RET501", "RET504", "RUF010", "RUF012", "RUF015", "RUF019", "RUF022", "RUF023", "RUF046", "RUF051", "RUF059", "RUF100", "S110", "S112", "SIM101", "SIM102", "SIM103", "SIM113", "SIM114", "SIM115", "SIM117", "SIM118", "SIM201", "SIM210", "SIM211", "SIM222", "SIM401", "TC004", "TC005", "TID251", "TRY002", "TRY004", "TRY201", "TRY203", "TRY300", "UP006", "UP007", "UP008", "UP012", "UP018", "UP024", "UP028", "UP031", "UP032", "UP034", "UP035", "UP036", "UP037", "UP045"]
extend-select = []

[lint.mccabe]
Expand Down
99 changes: 75 additions & 24 deletions scripts/budget_ratchet_check.py
Original file line number Diff line number Diff line change
@@ -1,16 +1,19 @@
#!/usr/bin/env python3
"""Non-gating ratchet guard: budget ceilings may only fall, never rise.
"""Non-gating ratchet guard: budget baselines and ceilings may only fall, never rise.

Every `*-budget.json` file (ruff-strict, type-discipline, basedpyright-code) is a
one-way ratchet: each rule's ceiling is `baseline + slack`, and the whole point is
to drive that number DOWN over time. This check compares every budget file against
its own content at the merge-base with the target branch and fails (exits 1, red) if:

* a rule's ceiling went up,
one-way ratchet: each rule's ceiling is `baseline + slack`, and both the recorded
`baseline` (the live violation count) and that ceiling are meant to be driven DOWN
over time. This check compares every budget file against its own content at the
merge-base with the target branch and fails (exits 1, red) if:

* a rule's ceiling (`baseline + slack`) went up,
* a rule's `baseline` went up, even if `slack` was lowered to keep the ceiling
flat (a higher baseline bakes in more accepted debt and must be acknowledged),
* a rule was dropped from a budget (its ceiling effectively became infinite), or
* an entire budget file was deleted.

New rules and lowered/equal ceilings are fine.
New rules and lowered/equal baselines and ceilings are fine.

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
Expand Down Expand Up @@ -66,7 +69,12 @@ def _load_head(rel: str) -> dict | None:


def _ref_is_commit(ref: str) -> bool:
return _run(["git", "rev-parse", "--verify", "--quiet", f"{ref}^{{commit}}"]).returncode == 0
return (
_run(
["git", "rev-parse", "--verify", "--quiet", f"{ref}^{{commit}}"]
).returncode
== 0
)


def _load_base(rel: str, ref: str) -> dict | None:
Expand All @@ -81,13 +89,55 @@ def _load_base(rel: str, ref: str) -> dict | None:
return json.loads(proc.stdout)


def _baselines(budget: dict) -> dict[str, int]:
"""Map each rule to its recorded baseline; skip malformed specs."""
return {
rule: int(spec.get("baseline", 0))
for rule, spec in budget.items()
if isinstance(spec, dict)
}


def _caps(budget: dict) -> dict[str, int]:
"""Map each rule to its ceiling (baseline + slack); skip malformed specs."""
caps: dict[str, int] = {}
for rule, spec in budget.items():
if isinstance(spec, dict):
caps[rule] = int(spec.get("baseline", 0)) + int(spec.get("slack", 0))
return caps
return {
rule: int(spec.get("baseline", 0)) + int(spec.get("slack", 0))
for rule, spec in budget.items()
if isinstance(spec, dict)
}


def _regression_detail(
rule: str,
base_caps: dict[str, int],
head_caps: dict[str, int],
base_baselines: dict[str, int],
head_baselines: dict[str, int],
) -> str | None:
"""Why `rule` regressed vs base, or None when it held flat or fell.

A dropped rule is terminal; otherwise a raised ceiling and a raised baseline are
independent loosenings (the latter catches a baseline bump masked by a slack cut),
so both reasons are reported when both apply.
"""
base_cap = base_caps[rule]
if rule not in head_caps:
return f"rule dropped (ceiling {base_cap} -> removed)"
reasons = tuple(
message
for raised, message in (
(
head_caps[rule] > base_cap,
f"ceiling raised {base_cap} -> {head_caps[rule]}",
),
(
head_baselines[rule] > base_baselines[rule],
f"baseline raised {base_baselines[rule]} -> {head_baselines[rule]}",
),
)
if raised
)
return "; ".join(reasons) or None


def regressions_for(rel: str, base: dict | None, head: dict | None) -> list[Regression]:
Expand All @@ -96,18 +146,17 @@ def regressions_for(rel: str, base: dict | None, head: dict | None) -> list[Regr
if head is None:
return [Regression(rel, "*", "budget file was deleted (every ceiling removed)")]

base_caps = _caps(base)
head_caps = _caps(head)
base_caps, head_caps = _caps(base), _caps(head)
base_baselines, head_baselines = _baselines(base), _baselines(head)
return [
Regression(
rel,
rule,
f"rule dropped (ceiling {base_cap} -> removed)"
if rule not in head_caps
else f"ceiling raised {base_cap} -> {head_caps[rule]}",
Regression(rel, rule, detail)
for rule in sorted(base_caps)
if (
detail := _regression_detail(
rule, base_caps, head_caps, base_baselines, head_baselines
)
)
for rule, base_cap in sorted(base_caps.items())
if rule not in head_caps or head_caps[rule] > base_cap
is not None
]


Expand Down Expand Up @@ -141,7 +190,9 @@ def main() -> int:
regressions.extend(regressions_for(rel, base, head))

if regressions:
print(f"FAIL: budget ceiling(s) loosened vs base {args.base} (merge-base {ref[:12]}):")
print(
f"FAIL: budget baseline(s)/ceiling(s) loosened vs base {args.base} (merge-base {ref[:12]}):"
)
for reg in regressions:
print(f" {reg.budget} {reg.rule}: {reg.detail}")
print(
Expand Down
33 changes: 28 additions & 5 deletions tests/test_litellm/test_budget_ratchet_check.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
"""Tests for scripts/budget_ratchet_check.py.

The guard's whole contract is "ceilings may only fall": a raised ceiling, a dropped
rule, or a deleted file is a regression, while a lowered/equal ceiling, a brand-new
The guard's contract is "baselines and ceilings may only fall": a raised ceiling, a
raised baseline (even when slack is cut to keep the ceiling flat), a dropped rule, or
a deleted file is a regression, while a lowered/equal baseline and ceiling, a brand-new
rule, or a brand-new budget file is fine. Each branch is pinned here.
"""

Expand All @@ -10,7 +11,9 @@
import sys
from pathlib import Path

_MODULE_PATH = Path(__file__).resolve().parents[2] / "scripts" / "budget_ratchet_check.py"
_MODULE_PATH = (
Path(__file__).resolve().parents[2] / "scripts" / "budget_ratchet_check.py"
)
_spec = importlib.util.spec_from_file_location("budget_ratchet_check", _MODULE_PATH)
ratchet = importlib.util.module_from_spec(_spec)
_spec.loader.exec_module(ratchet)
Expand All @@ -35,10 +38,30 @@ def test_raised_ceiling_is_a_regression():

def test_lowered_or_equal_ceiling_is_clean():
base = {"LIT006": _spec_of(1013, 10)}
# baseline drops, slack flat -> ceiling falls
assert ratchet.regressions_for("b.json", base, {"LIT006": _spec_of(1000, 10)}) == []
# nothing changes
assert ratchet.regressions_for("b.json", base, {"LIT006": _spec_of(1013, 10)}) == []
# slack traded for baseline at the same ceiling is fine
assert ratchet.regressions_for("b.json", base, {"LIT006": _spec_of(1023, 0)}) == []
# slack cut while baseline holds -> ceiling falls, baseline flat
assert ratchet.regressions_for("b.json", base, {"LIT006": _spec_of(1013, 0)}) == []


def test_raised_baseline_is_a_regression_even_when_ceiling_held_flat():
# baseline 1013 -> 1023 with slack cut 10 -> 0 keeps the ceiling at 1023, but a
# higher baseline bakes in more accepted debt and must still surface as a regression
base = {"LIT006": _spec_of(1013, 10)}
regs = ratchet.regressions_for("b.json", base, {"LIT006": _spec_of(1023, 0)})
assert [r.rule for r in regs] == ["LIT006"]
assert "baseline raised 1013 -> 1023" in regs[0].detail
assert "ceiling raised" not in regs[0].detail


def test_raised_baseline_and_ceiling_report_both_reasons():
base = {"LIT006": _spec_of(1013, 10)}
regs = ratchet.regressions_for("b.json", base, {"LIT006": _spec_of(1100, 10)})
assert [r.rule for r in regs] == ["LIT006"]
assert "ceiling raised 1023 -> 1110" in regs[0].detail
assert "baseline raised 1013 -> 1100" in regs[0].detail


def test_dropped_rule_is_a_regression():
Expand Down
Loading