* ci(lint): enforce type-discipline budget for casts and type guards Add a ratcheted gate that blocks net-new typing.cast() usage and bans TypeGuard/TypeIs outright, layered on the existing ruff-strict budget setup. - ruff-strict.toml: ban cast/TypeGuard/TypeIs (typing + typing_extensions) via flake8-tidy-imports banned-api (TID251) for a coarse import-level freeze. - ruff-strict-budget.json: bump TID251 baseline 2404 -> 2662 to absorb the ~258 pre-existing usages now matched by the new banned-api entries. - scripts/check_type_discipline.py: AST checker adding LIT006 (cast call sites, suppress with `# cast-ok: <reason>`) and LIT007 (TypeGuard/TypeIs annotations, suppress with `# guard-ok: <reason>`) for per-call-site granularity. - scripts/type_discipline_gate.py: baseline+slack gate with delta-vs-base, mirroring ruff_strict_gate.py. - type-discipline-budget.json: LIT006 baseline 1013 (slack 10), LIT007 0/0. - test-linting.yml: run the gate in CI against the PR base SHA. * ci(lint): enforce suppression-reason budgets and guard budgets against loosening - wire the **kwargs ban (LIT008) into the vendored type-discipline checker so it matches the budget that already referenced it - freeze LIT003/LIT004 (noqa / type-ignore without codes or reason) and LIT005 (*-ok suppression without a reason) at slack 0 so any net-new unexplained suppression trips the type-discipline gate - add scripts/budget_ratchet_check.py and a separate, non-gating budget-ratchet CI job that turns red when any *-budget.json ceiling is raised, a rule is dropped, or a budget file is deleted * ci(lint): ban mutable collections in annotations and all mutable construction Expand LIT001 from coarse builtins at interfaces to any mutable collection in any annotation (builtins, typing aliases, collections concretes, mutable ABCs) across signatures, class attributes, locals, and globals. Add LIT009 to flag mutable-collection construction (literals, comprehensions, constructors) so the unannotated seed-then-mutate pattern is caught too. Enumerate any-ok in LIT005 so its reason requirement holds even when only the stdlib checker runs. Budget LIT001 (21452) and LIT009 (25222) with slack 10 to ratchet down. * ci(lint): recommend pydantic at boundaries and add functional-refactor guidance Drop the msgspec mention from the cast banned-api messages so the recommended validation path matches the codebase's primary pattern (pydantic). Add a note to CLAUDE.md that lint / type-discipline failures should be resolved by refactoring to functional, immutable patterns rather than reaching for mutable structures or `# mutable-ok`. * style: make CLAUDE.md more concise * chore: update CLAUDE.md guidelines * ci(lint): renumber mutable construction LIT009 -> LIT002 next to LIT001 Group the mutable-collection family together: LIT001 (mutable collection in any annotation) and the construction rule now sit adjacent at LIT001/LIT002. The freed LIT009 slot is taken by the sibling Any gate (check_any_discipline.py, #30379), which moves its Any-typed-value rule LIT002 -> LIT009 in lockstep so the shared LIT namespace stays contiguous with no holes. Budget, gate docstring, and the checker's own docstring/messages are updated to match. * fix: numbering in CLAUDE.md * test(lint): test type-discipline checker, scope LIT007 to return types Add regression tests for check_type_discipline.py (every LIT rule, its suppression, and the comment scanner) and for budget_ratchet_check.py. Confine LIT007 to function return annotations, the only place TypeGuard/TypeIs are valid, so a runtime name that merely reads those identifiers is no longer flagged. Switch scan_comments to io.StringIO(source).readline, the standard readline that returns '' at EOF, dropping the iter(...).__next__ idiom. * fix(lint): best-effort worktree teardown so cleanup can't mask the real error base_counts ran `git worktree remove` through the raising `_run` in its finally, so a failed `git worktree add` (or a failure in the body) was masked by a second SystemExit from the cleanup. Tear the worktree down best-effort, like the sibling rmtree, so the original error propagates. * fix(lint): ratchet fails loudly on an unresolvable base; drop dead checker state Verify the merge-base ref resolves to a commit before trusting a missing-file result from git show, so an invalid or empty BASE_SHA now turns the budget-ratchet guard red instead of skipping every budget and passing vacuously Also drop the unused Comments.by_line field and the phantom --changed-only usage line from check_type_discipline's docstring, and cover the ref handling with tests * fix(lint): degrade malformed source to LIT000 instead of crashing the checker tokenize.generate_tokens raises IndentationError (a SyntaxError subclass) on a dedent mismatch, which escaped scan_comments' tokenize.TokenError handler and crashed the whole checker run, zeroing the gate for that invocation. Catch SyntaxError too so the file falls through to ast.parse and is reported as LIT000, matching the checker's graceful-degradation contract. Also add the trailing newline ruff-strict.toml lacked * perf(lint): skip the base worktree scan when no rule is over its ceiling cmd_check created a git worktree and re-scanned the base tree on every run, but a rule can only breach when its head count is already over baseline + slack; when none are, the base comparison cannot change the verdict. Short-circuit to OK in that case, which is every green PR, roughly halving the gate's work. Extract over_ceiling and cover it (and evaluate's drift-safety) with tests * fix(lint): exempt .dict()/.list()/.set() method calls from LIT002 _construction_kind matched dict/list/set as constructors via func.attr too, flagging common method calls like pydantic's model.dict() as mutable construction; 200 such false positives existed in litellm. Recognize dict/list/set construction only when unqualified while keeping the collections concretes (deque/defaultdict/...) matchable as attributes, since those are rarely method names. Ratchet the LIT002 baseline down 25222 -> 25022 to reflect the removed false positives * chore(lint): bump basedpyright ceilings to absorb staging base drift The basedpyright gate added in #30379 is a total-count check against basedpyright-code-budget.json and the linting workflow runs only on pull_request, so pushes to litellm_internal_staging never re-baseline it. Merging staging into this branch surfaced that drift: seven reportAny/reportUnknown* rules sit 10-149 errors above their committed ceiling even though this PR changes no files under litellm/, the only path basedpyright scans (pyrightconfig include is litellm). The new baselines match the counts CI measured on the merge commit, with the existing per-rule slack preserved * fix(lint): ratchet guard watches every budget file, not just two DEFAULT_BUDGETS only listed ruff-strict-budget.json and type-discipline-budget.json, so mypy-code-budget.json and basedpyright-code-budget.json were unguarded and their ceilings could rise with no signal, which is exactly the failure mode this guard exists to prevent. The gap became concrete when this PR bumped basedpyright-code-budget.json to absorb staging drift. All four budgets are now watched, so the budget-ratchet job surfaces that basedpyright bump for human review the same way it surfaces the TID251 raise. A regression test pins that every *-budget.json on disk is in DEFAULT_BUDGETS, failing loudly if a future budget escapes the ratchet * fix: add a lot more slack * fix(lint): restore LIT003 frozen slack to 0 The blanket slack bump set LIT003 (bare # noqa without codes or a reason) to a slack of 50, which contradicts the documented zero-tolerance invariant: the gate docstring and the PR description table both freeze LIT003/LIT004/LIT005 at slack 0 so any net-new unexplained suppression trips the gate. Slack 50 would let 50 new bare noqas through silently. The actual LIT003 count is 397, well under the 516 baseline, so restoring slack to 0 keeps the gate green while putting the freeze back. LIT004/LIT005/LIT007 were already correct at 0 * fix(lint): restore documented slack 10 for the buffered LIT rules The slack bump left LIT001/LIT002/LIT006/LIT008 at 2000/2500/100/100, 10-250x the "/ 10" the PR description table and the gate docstring document. That buffer was never needed: the gate already blames a rule only when its count exceeds the ceiling and grew vs the merge-base, so the violations the staging merge added in litellm/ sit in both head and base and are never charged to this PR. With slack back at the documented 10 the gate stays green, and the ceiling is tight again (LIT006 no longer waves through 99 net-new cast() calls). Baselines are unchanged; only the slack returns to its documented value * fix(lint): ratchet LIT003 baseline down to its actual count The LIT003 baseline was 516 while the current bare-noqa count is 397, leaving ~119 units of headroom that undercut the documented zero-tolerance freeze: the gate docstring claims any net-new bare noqa trips the gate, but with cap 516 a PR could add over a hundred first. Drop the baseline to the measured 397 so the freeze is exact (cap = 397 + slack 0), the same hard-zero-at-the-boundary shape LIT005 and LIT007 already use and pass in CI. PR table row updated to 397 / 0 * fix: increase slack * fix: increase slack * docs(lint): align gate docstring with buffered LIT003/LIT004 slack The budget now gives LIT003/LIT004 nonzero slack, so the gate's prose no longer claims they are frozen at slack 0; LIT005 remains the reasonless- suppression freeze and LIT007 the hard zero.
9.3 KiB
Do not write comments unless they are absolutely necessary to explain some very complex business logic. Please clean up if there are comments that are not absolutely necessary. Do not remove comments that are unrelated to the addition of the code of this PR
Explanation: code comments are, in a way, a violation of DRY code. You must update logic in two locations to change the code and "hard to change" is literally the definition of tech debt. We should instead aim to write code that is intuitive to the reader, while being both easy to maintain and high performance
Don't assume that the existing code is correct or the right way of doing things / good coding patterns. In fact, there are a lot of bad coding practices, overly complex code, code smells, etc. If something doesn't look right, speak up. Feel free to break existing patterns or question weird existing code to make new code high quality, as in:
- correct
- secure
- performant
- readable
- easy to maintain/change
- modern
In that order of importance
When adding new features, add meaningful tests. Don't add tests that don't check anything substantial and is there just to make the code coverage pass. Yes, code coverage is important, but I'd rather have no signal whether the code is working than tests that don't fail when code is broken. The goal is to have tests that would fail before the feature was added/if the code was mutated in a way that breaks the feature and succeed only when the feature is fully working. I should run mutation testing and see > 90% kill rate
Same thing for bug fixes. The tests should make it so that this specific bug can never happen again without failing tests (i.e., regression)
tests/test_litellm/ mirrors litellm/ in a parallel path (see tests/test_litellm/readme.md). Name tests test_<filename>.py, but always match the existing test file in the directory you touch — many provider dirs use longer descriptive names (e.g. test_anthropic_chat_transformation.py) to avoid ambiguity across sibling folders. For bug fixes, extend the existing mapped test file rather than creating a new one. Only create a new test file for a new feature (provider, endpoint, or transformation module) that has no mapped test yet, following that directory's naming convention (or test_<filename>.py if you're the first test there). One focused regression test beats many shallow ones
When creating PRs, don't set base to main. litellm_internal_staging serves that purpose
Always use @.github/pull_request_template.md as a guide for your PR body
Never use pytest commands or the like as "Screenshots / Proof of Fix". We prefer curl'ing a live proxy instance running on localhost:4000 (I like to run it with python litellm/proxy/proxy_cli.py --config litellm/proxy/dev_config.yaml --detailed_debug --reload --use_v2_migration_resolver 2>&1 | tee litellm.log) and showing both the command run and the output. Also, it should hit real LLM provider APIs, not mocks, and cost real $ because that is the most realistic test. The proof of fix should be exactly what the end user / customer would see / do. The run logs in PR #27703 is a prime example of how to do it (not a huge fan of using a python test script that future me and the team will have no visibility into; I prefer just curl commands or a short list of bash commands (e.g., using for)). If it's a UI thing, just tell me which URLs to go to (e.g., http://localhost:4000/ui/?page=logs), where to click, what fields to fill out, etc. along with the other commands to run in an ordered list, and I'll do it myself and post the screenshots after you make the PR
If you ever make public-facing PR descriptions, comments, issues, commit messages, etc., always follow these guidelines to sound less AI-y:
- don't use emojis
- don't use "—". Instead, reach for ";", ".", etc.
- don't use the pattern "It's not X, it's Y", "You're not X, you're Y", etc.
- don't use bulleted or numbered lists unless it would be nonsensical not to. Instead, prefer prose
- don't add a trailing "." at the end of paragraphs (just like this file)
- don't use →. Instead, prefer not to use arrows, and if need be, use -> instead
Don't hesitate to use values in .env to get needed API keys and other secrets, as long as you never add them to conversation history, commit them, or include them in GitHub issues / PRs
Run tests, format your code, and lint your code before each commit
When you fix violations gated by ruff-strict-budget.json, mypy-code-budget.json, or basedpyright-code-budget.json, run make lint-budget-update and commit the lowered baselines so the ceilings ratchet down instead of leaving stale headroom
If you're trying to create a new function that relies on untyped stuff, instead of adding more Any's and bringing it closer to the max, just validate it in the caller with Pydantic (a model or TypeAdapter that returns the typed thing or raises will do) and then pass the now typed variable in
The Any-discipline gate (make lint-any, also a CI job) fails when a line you changed under litellm/ holds a value typed Any, including the X | Any. Ideally # any-ok: <reason> is never used; treat it as a last resort for a genuine typed/untyped boundary that Pydantic truly can't model
If you get an LIT001 or LIT002 fail, refactor the code to follow functional programming best practices rather than introducing mutable data structures. For example, build values in one shot with comprehensions or generators wrapped in tuple() / frozenset() instead of seeding an empty list/dict/set and mutating it over time. Ideally # mutable-ok is never used; reach for it only as a genuine last resort when an immutable rewrite is truly impossible, and always pair it with a real reason
Ask to commit and push your work when you're done (or if you're confident that your code is good and works, just do it)
When you must use real LLM models to, for example, write e2e tests, write a QA runbook, etc., make sure to use the latest models (doesn't have to be smartest, can also be a modern small, fast one. No strong preference for smart vs fast here, just use something modern) as of the year and month of the current date. Do a web search as necessary to figure that out
If you're an internal contributor, when creating a new PR, the typical flow is to branch off litellm_internal_staging and create a branch prefixed with litellm_. Do not create a branch prefixed with claude/ and generally do not have / in your branch names
Do not add Co-Authored-By: Claude or any Claude attribution to commit messages. Never use a claude/ prefix or put a / in a branch name. Do not add "Generated with Claude Code" (or any similar attribution) to PR descriptions or comments. Do not create a new PR/branch off the existing PR to fix/add something that is related and could've just been committed directly to the existing PR's branch
When working on a PR, keep the PR description in sync with new commits being made
Monkeypatching attributes of a class to do testing is an anti-pattern. Prefer dependency-injecting things into classes. That way, at unit test time, you can pass a mocked dependency in
Do not put names of customers or customer company names in code, PRs, and issues. The codebase is public
CI supply-chain safety: Never pipe a remote script into a shell (curl ... | bash, wget ... | sh); download the artifact to a file, verify its SHA-256 checksum, then install. Pin every external tool to a specific version with a full URL (not latest or stable). Verify checksums for all downloaded binaries, using the provider's official .sha256 / .sha256sum sidecar when available. These rules apply to every download in CI
Follow these coding conventions for new/updated code (a three-line fix in a legacy file shouldn't trigger huge drive-by refactors):
- Composition over inheritance
- Never-nester: early returns over deep nesting
- Don't throw; model failures as values (One function (e.g., raise_public) maps error union to existing public exception contracts via exhaustive match + assert_never)
- No mutation; don't reassign variables, global or local. Instead of mutable lists and dicts, prefer tuples, frozen dataclasses (with slots=True), etc.
- Use dependency injection
- Fully typed; no
Anyor coarse types likedict[str, Any]or justdict. Every function parameter must be strongly typed - Use tagged unions + match
- No monster files or god objects
- No file sprawl: deliberate file and folder structure
- Standard over hand-rolled: use the official SDK or a library where one exists; where none does, follow industry standards instead of inventing local conventions
Follow conventional commits for commit names and PR titles
Think Before Coding
Don't assume. Don't hide confusion. Surface tradeoffs
Before implementing:
- State your assumptions explicitly. If uncertain, ask
- If multiple interpretations exist, present them. Don't pick silently
- If a simpler approach exists, say so. Push back when warranted
- If something is unclear, stop. Name what's confusing. Ask
Simplicity First
Minimum code that solves the problem. Nothing speculative
- No features beyond what was asked
- No abstractions for single-use code
- No "flexibility" or "configurability" that wasn't requested
- No error handling for impossible scenarios
- If you write 200 lines and it could be 50, rewrite it
Ask yourself: "Would a senior engineer say this is overcomplicated?" If yes, simplify