refactor(types): cut 653 implicit and explicit Any diagnostics across 11 modules - #36054
Conversation
Types the values that were flowing through as Any in the highest-density modules, using shapes the code already assumes: TypedDicts for the JSON payloads read by literal key, Protocols for the prisma rows, existing litellm types where they were already modeled, and `object` where a value is only stored and forwarded. Annotation-level only, no runtime behavior change. New annotations use read-only views (Mapping / Sequence / tuple) rather than dict / list, so LIT001 drops alongside the Any counts instead of trading one budget for another. No suppressions, casts, or type guards were added. basedpyright across the touched files: 1547 -> 856 errors, with reportAny down 399 and reportExplicitAny down 134, and no rule increasing.
…alues An adversarial review of the previous commit found annotations that described what the code wished were true rather than what flows through. A false annotation is worse than the Any it replaced, since it launders a wrong assumption past the type checker. - purview: `_resolve_user_id` claimed every request-body value was a Mapping, contradicting `_resolve_trusted_user_id` one method over, which types the same argument `Mapping[str, object]`. `_should_block` claimed every Graph response value was a sequence of str->str mappings and was not assignable from its own producer's return type. - cato: `_CatoAnalyzeResponse.required_action` was required and non-nullable while the API returns null, as seven fixtures in the guardrail's own suite assert. `analysis_result` had the same problem. The streaming hook narrowed an override parameter below what `ProxyLogging` actually passes it. - marketplace: `_PluginRecord.manifest_json` was `str` against a nullable column. Making it honest surfaced a latent crash, covered below. - ownership: two functions took an attribute Protocol while their own bodies branch on `isinstance(response, dict)`, which no Protocol can satisfy. - openapi generator: `paths` claimed every path-item value was an operation, though path items also carry `parameters`, `summary` and `$ref`. - custom openapi spec: a TypedDict asserted a shape that the function returns raw Pydantic sub-schemas out of. Reverted to Any, which is imprecise but not false. `get_marketplace` did an unguarded `json.loads` on the nullable `manifest_json` inside an `except json.JSONDecodeError`, which cannot catch the TypeError a NULL raises, so one NULL row 500s the endpoint. It now skips the plugin like the file's other two read sites already do, with a regression test that fails without the guard. Where honesty cost precision, precision lost. `_should_block` went back to its original signature entirely: the narrowing needed to type it turned a fail-closed DLP control fail-open, because the TypeError it used to raise on a malformed response reached `except Exception` and became a 400.
Lowers the committed ceilings so the headroom shrinks by exactly what was cleared instead of leaving stale slack for the next change to spend. basedpyright -653 errors across 48 rules, with reportAny 29204 -> 28842 and reportExplicitAny 9227 -> 9105. Strict ruff -80 violations, led by ANN401 -59. LIT rules -85, led by LIT001 -76.
Greptile SummaryThe PR tightens annotations across caching, integrations, proxy utilities, guardrails, and policy modules while reducing static-analysis budgets. It also prevents nullable plugin manifests from breaking marketplace generation and adds regression coverage.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| litellm/proxy/_experimental/mcp_server/openapi_to_mcp_generator.py | Refines OpenAPI payload annotations without changing the generator’s runtime control flow. |
| litellm/proxy/anthropic_endpoints/claude_code_endpoints/claude_code_marketplace.py | Models nullable plugin manifests accurately and skips unusable marketplace rows instead of failing the endpoint. |
| tests/test_litellm/proxy/anthropic_endpoints/test_claude_code_marketplace.py | Adds focused regression coverage for marketplace rows with null manifests. |
| litellm/caching/redis_semantic_cache.py | Narrows cache interfaces and internal values using callable, mapping, and object annotations. |
| litellm/integrations/galileo.py | Adds typed logging payload fields and narrows callback, record, and serialization interfaces. |
| litellm/proxy/container_endpoints/ownership.py | Introduces protocols for managed-object rows and tables while preserving ownership behavior. |
Reviews (2): Last reviewed commit: "chore(lint): ratchet lint budgets down b..." | Re-trigger Greptile
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
…itellm_reduce_any_types # Conflicts: # basedpyright-code-budget.json # type-discipline-budget.json
The base branch ratcheted the same limits in 28a277e, so the conflicting files were reset to base and the ratchet re-run against the new merge-base rather than resolved by hand. Each limit is now the base value minus this branch's own delta, so both ratchets survive: basedpyright -653 across 48 rules, strict ruff -80, LIT -85.
|
I ran the shard's exact file list with the same settings CI uses (
Nothing in this PR touches |
TLDR
Problem this solves:
Anyin basedpyright, ceilings never moveget_marketplace500s on a NULLmanifest_jsonrowHow it solves it:
Relevant issues
Linear ticket
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
@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
The one runtime change in this PR is the
manifest_jsonnull guard, so that is what the proof covers. Before is the base commit388943ac, after is this branch at4ab7a33d. Both runs hit the same live proxy onlocalhost:4000backed by a real Postgres, against the same two database rows, with no mocksSetup, run once against either commit:
That second row is what the schema already permits, since
schema.prisma:1340declaresmanifest_json String?Before, on
388943ac, one such row takes down the whole endpoint:with this in the proxy log:
After, on
4ab7a33d, same rows, same request:The bad row is still there and is now skipped instead of crashing the response, which is what the file's two other read sites already did
Everything else in this PR is annotation-only, so the proof that matters is that behavior did not move.
custom_openapi_spec.pyis the file whose public signatures changed most, and it feeds/openapi.json, so I ran all four of its entry points at both commits over the live 1.25 MB spec this proxy serves and diffed the results:Type
🧹 Refactoring
🐛 Bug Fix
Changes
Eleven modules were picked for high
Anydensity and low importer count, so the blast radius stays small. The types come from what the code already assumes: TypedDicts for JSON payloads read by literal key, Protocols for prisma rows, existinglitellm/types/models where the shape was already declared, andobjectwhere a value is only stored and forwarded. New annotations use read-only views rather thandict/list, so LIT001 falls alongside theAnycounts instead of trading one budget for another. No suppressions, casts, or type guards were added anywhereTree-wide basedpyright goes 148012 to 147359, with
reportAnydown 362,reportExplicitAnydown 122, and the fourreportUnknown*rules down 145 between them. No rule increases anywhere in the tree, including in files this PR does not touch. Strict ruff drops 80, led by ANN401 down 59, and the LIT rules drop 85, led by LIT001 down 76. All three budget files are ratcheted by exactly those amountsSome annotations from the first pass turned out to describe what the code wished were true rather than what flows through, and those are corrected here.
_resolve_user_idclaimed every request-body value was aMappingwhile_resolve_trusted_user_idone method over types the same argumentMapping[str, object]._CatoAnalyzeResponse.required_actionwas required and non-nullable though the API returns null, which seven fixtures in the guardrail's own suite already asserted._PluginRecord.manifest_jsonwasstragainst a nullable column, and making it honest is what surfaced the crash above. Two functions inownership.pytook an attribute Protocol while their own bodies branch onisinstance(response, dict), which no Protocol can satisfyWhere honesty cost precision, precision lost.
_should_blockwent back to its original signature entirely: the narrowing needed to type it turned a fail-closed DLP control fail-open, because theTypeErrorit used to raise on a malformed Graph response reachedexcept Exceptioninpurview_dlp.pyand became a 400. A slightly lowerAnycount is not worth letting traffic through a guardrail that used to reject it._extract_field_schemawent back toAnyfor the same reason, since the TypedDict proposed for it excluded$ref,itemsandenumkeys that the function returns straight out of a Pydantic sub-schemaFinal Attestation