Skip to content

fix(ci): excuse retired test-quality rules in the budget ratchet - #42116

Merged
yuneng-berri merged 3 commits into
mainfrom
litellm_ratchet_retired_rules
Sep 20, 2026
Merged

yuneng-berri merged 3 commits into
mainfrom
litellm_ratchet_retired_rules

Conversation

@devin-ai-integration

@devin-ai-integration devin-ai-integration Bot commented Sep 20, 2026 •

Copy link
Copy Markdown
Contributor

TLDR

Problem this solves:

How it solves it:

  • check_test_quality.py exposes RULE_CODES, the codes it can emit
  • the ratchet excuses a dropped rule the paired checker no longer emits
  • a raised limit is never excused, same as graduation

User Flow

Before: a contributor opens a PR from a branch cut before #42103 and the ratchet job fails for a rule nobody touched

  1. They push a branch based on 776f8bc and open a PR
  2. The budget-ratchet job in test-linting goes red with test-quality-budget.json TQ008: rule dropped (limit 10993 -> removed)
  3. They have to rebase or explain a failure unrelated to their change

After: the same PR gets a green ratchet

  1. They push a branch based on 776f8bc and open a PR
  2. The budget-ratchet job prints OK: no budget limit increased vs base ...
  3. Nothing else to do

Relevant issues

Follow-up to #42103

Affected release

Linear ticket

Pre-Submission checklist

Please complete all items before asking a LiteLLM maintainer to review your PR

  • I have added meaningful tests
  • The handful of test files covering my change pass locally, e.g. uv run pytest tests/test_litellm/<your_test_file>.py -v. Leave the suites (make test-unit-*, make test-unit) to CI: it finishes in ~15 minutes where a laptop takes an hour or more
  • My PR passes all required CI/CD checks (e.g., lint, schema.d.ts sync check, etc.)
  • My PR's scope is as isolated as possible; it only solves 1 specific problem
  • I have received a Greptile Confidence Score of at least 4/5 before requesting a maintainer review (Greptile reviews automatically once the PR is opened; only comment @greptileai to re-request a review after pushing changes)

Delays in PR merge?

If you're seeing a delay in your PR being merged, ping the LiteLLM Team on Slack (#pr-review).

Screenshots / Proof of Fix

Base sha from the failing job: https://github.com/BerriAI/litellm/actions/runs/35498026484/job/106044581382

Before (6ef7b86)

  1. python scripts/budget_ratchet_check.py --base 776f8bc3090cc13ca0d34ac38858c9d3c095436c
  2. Output, exit 1:
FAIL: budget limit(s) loosened vs base 776f8bc3090cc13ca0d34ac38858c9d3c095436c (merge-base 776f8bc3090c):
  test-quality-budget.json  TQ008: rule dropped (limit 10993 -> removed)
Budgets are one-way ratchets and may only go down or stay flat. This check is non-gating: if the increase is justified (e.g. a newly banned API), a human can merge over the red after acknowledging it.

After (bbfa853)

  1. python scripts/budget_ratchet_check.py --base 776f8bc3090cc13ca0d34ac38858c9d3c095436c
  2. Output, exit 0:
OK: no budget limit increased vs base 776f8bc3090cc13ca0d34ac38858c9d3c095436c (ruff-strict-budget.json, type-discipline-budget.json, basedpyright-code-budget.json, test-quality-budget.json)
  1. uv run --no-sync pytest tests/test_litellm/test_budget_ratchet_check.py tests/test_litellm/test_check_test_quality.py -q: 94 passed. Dropping TQ009 from RULE_CODES, or adding TQ008, makes test_rule_codes_match_every_code_the_checker_emits fail

Type

🐛 Bug Fix
🚄 Infrastructure

Caveats (if any)

Low

  • the ratchet loads check_test_quality.py by path so it works under pytest too

Final Attestation

  • The tests check the right things, including the edge cases, and regressions in the respective real-world customer use-cases are not possible after this PR

Link to Devin session: https://app.devin.ai/sessions/f042d1c827b8430ca4685e83b5bd86bc
Open in Devin Desktop: https://app.devin.ai/desktop/session/f042d1c827b8430ca4685e83b5bd86bc?variant=devin

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
@devin-ai-integration

Copy link
Copy Markdown
Contributor Author

I'll fix CI failures and address comments from users with write access. I'll skip comments containing "(aside)".

  • Disable automatic comment, CI, and merge conflict monitoring

@devin-ai-integration
devin-ai-integration Bot requested a review from a team September 20, 2026 08:13
@greptile-apps

greptile-apps Bot commented Sep 20, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 5/5

The PR appears safe to merge; no actionable new defects or outstanding review findings remain.

Summary

This PR updates the budget ratchet to allow removal of test-quality budget entries when the paired checker no longer emits those rule codes.

  • Adds an immutable registry of test-quality rule codes.
  • Distinguishes retired rules from raised limits and still-emitted dropped rules.
  • Adds behavioral coverage for retirement handling and rule-code registry completeness.
  • Adds Final annotations requested during the previous review.

Reviews (4) · Last reviewed commit: "test(ci): annotate new test locals as Fi..."

Comment thread tests/test_litellm/test_check_test_quality.py Outdated
Comment thread scripts/budget_ratchet_check.py Outdated
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
@devin-ai-integration

Copy link
Copy Markdown
Contributor Author

@greptileai the registry test now runs every rule through the checker and retired_rules takes dict[str, object], please re-review

@codecov

codecov Bot commented Sep 20, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@devin-ai-integration

Copy link
Copy Markdown
Contributor Author

@greptileai

Comment thread scripts/budget_ratchet_check.py
Comment thread tests/test_litellm/test_check_test_quality.py Outdated
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
@devin-ai-integration

Copy link
Copy Markdown
Contributor Author

@greptileai

@yuneng-berri
yuneng-berri merged commit 0b2de7d into main Sep 20, 2026
90 checks passed
@yuneng-berri
yuneng-berri deleted the litellm_ratchet_retired_rules branch September 20, 2026 10:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant