Repository navigation
chore(lint): remove the LIT002 mutable-construction rule - #43971
Conversation
Drop LIT002 from scripts/check_type_discipline.py along with its helpers, its budget entry, its unit tests, and the AGENTS.md and gate docstring mentions. `# mutable-ok` now only suppresses LIT001, so the markers that only existed to silence LIT002 became LIT013 stale suppressions and are removed. The files whose layout depended on those trailing comments are reformatted with ruff format. Every other LIT rule count is unchanged and the ASTs of all touched litellm/ files match main apart from one docstring.
Restore the ~1.4k `# mutable-ok` markers stripped in the previous commit so this PR only touches the checker, its tests, the budget, and docs. Those markers no longer suppress anything, so `# mutable-ok` is exempt from LIT013 until a follow-up strips them.
|
| @@ -2,9 +2,6 @@ | |||
| "LIT001": { | |||
| "limit": 22174 | |||
| }, | |||
| "LIT002": { | |||
| "limit": 26715 | |||
| }, | |||
| "LIT003": { | |||
There was a problem hiding this comment.
Budget edited on PR branch The repository’s contributor instructions say never to edit
type-discipline-budget.json on a PR branch and to drop any such edit before opening the PR. This PR removes the LIT002 entry from that file. The checker and budget still need to agree, but this repository requirement must be satisfied before merging.
Context Used: AGENTS.md (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
There was a problem hiding this comment.
wait we have to remove this entry to you know remove the LIT lint check
There was a problem hiding this comment.
You’re right: removing LIT002 means its budget entry must also be removed. The checker and budget schema need to stay synchronized, so my earlier comment was incorrect.
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
…w-up" This reverts commit c35bc0b.
|
bugbot run |
Comments Outside DiffThese findings could not be posted inline.
|
…ines Main renamed litellm/proxy/engine to litellm/proxy/lens and added 22 mutable-ok markers that only suppressed LIT002, across 10 files. Conflicted files take main's version with those markers removed, so the merged tree differs from main by comments only outside the checker, its test and the gate docstring.
1 similar comment
|
bugbot run |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit bb5dfe3. Configure here.
After rebasing onto main (which removed the LIT002 mutable-construction rule in BerriAI#43971), the '# mutable-ok' annotation on this line suppressed nothing and tripped the LIT013 stale-suppression gate. The copy is a deliberate rebinding, so annotate it as one.
…aned BerriAI#43971 removed the LIT002 mutable-construction rule, so the `# mutable-ok` comments on the request headers and the request body no longer suppress anything, and type_discipline_gate reports each as an unused suppression (LIT013). The one on the tier_map dict still applies and stays. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Main already retired LIT002 (#43971). This narrows LIT001 to mutable sequences and sets, so dict, Dict, DefaultDict, OrderedDict, Counter, ChainMap, defaultdict and MutableMapping annotations are allowed, nested list/set inside a mapping still trips, and the 945 mutable-ok suppressions that only covered mapping annotations are deleted (LIT013 now flags them). AGENTS.md guidance updated to match Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
TLDR
Problem this solves:
# mutable-okmarkers to silence itHow it solves it:
# mutable-oknow only suppresses LIT001User Flow
Before: a contributor writing an ordinary list literal in
litellm/gets a lint violationacc: Final = []and runpython scripts/check_type_discipline.py <file># mutable-ok: <reason>markerAfter: the same line passes, and LIT001 still catches mutable types in annotations
acc: Final = []and run the same command# mutable-okmarker on that line is now reported as LIT013 (suppresses nothing)Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
uv run pytest tests/unit/<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@greptileaito 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
Shared snippet:
Before (main at a76b59d)
python3 scripts/check_type_discipline.py snippet.pyAfter (bb5dfe3)
python3 scripts/check_type_discipline.py snippet.pypython scripts/type_discipline_gate.py --base origin/mainlitellm/except LIT002 and LIT013 is identical on main and on this branch (LIT001 22133, LIT010 15974, LIT011 5460, LIT014 370, and so on). LIT013 goes from 9 to 6 because three of the removed markers were already flagged on mainteam_callback_endpoints.py, so the marker removal and reformat change no runtime behaviormake checkpasses on the merged treelitellm/proxy/enginetolitellm/proxy/lensrename take main's version, and the markers main added on construction lines since then are removed the same wayType
Refactoring
Infrastructure
Caveats (if any)
Low
type-discipline-budget.json, which normally only the ratchet automation touchestest_budget_covers_exactly_the_checker_rulesasserts budget keys match checker rulesLIT002entry is deleted, no other limit changesbudget-ratchetcheck is red for this reason, since it flags any dropped entry# mutable-okon a construction line will fail the LIT013 gate once they merge main# mutable-okmarkers on construction lines inlitellm/proxy/management_endpoints/model_insights_endpoints.pyafter this branch last merged mainlitellm/proxy/lens/inference.py) is about code main already haslitellm/proxy/lens/is comment removal only, with identical ASTsschema-migrationfails on main too (main run at a76b59d)osv-scanfails on every open PR right nowFinal Attestation
Note
Low Risk
Lint-only policy change with AST-equivalent edits; risk is contributor friction from LIT013 on branches that still carry obsolete
# mutable-okcomments.Overview
Removes the LIT002 type-discipline rule that flagged mutable
list/dict/setliterals acrosslitellm/, along with its budget entry and related checker/tests (per PR scope).# mutable-oknow only suppresses LIT001 (mutable annotations/accumulators); stale markers that suppressed nothing are reported as LIT013.The bulk of the diff is mechanical cleanup: delete thousands of
# mutable-oktrailing comments (and minor reformatting where ruff depended on them) in caching, integrations, providers, proxy-adjacent core utils, andAGENTS.mdguidance narrowed from “LIT001 or LIT002” to LIT001 only. A few sites (e.g.litellm/litellm_core_utils/tokenizer.py) keep# mutable-ok: [LIT001]where the rule still applies.No intended runtime or API behavior change—only lint policy and comment hygiene.
Reviewed by Cursor Bugbot for commit bb5dfe3. Bugbot is set up for automated code reviews on this repo. Configure here.