Repository navigation
refactor(native): discover providers at shared dispatch boundaries - #39939
yujonglee-berri wants to merge 7 commits into
Conversation
9433cae to
c979a20
Compare
c979a20 to
7a101d4
Compare
LiteLLM Rust workflowWorkflow completed successfully for |
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
7a101d4 to
405deac
Compare
405deac to
660722e
Compare
660722e to
019080e
Compare
019080e to
27a51f0
Compare
27a51f0 to
9e0832c
Compare
9e0832c to
0ad7ae3
Compare
0ad7ae3 to
840963b
Compare
840963b to
152d041
Compare
d43fcc4 to
7899a93
Compare
7899a93 to
ae2e895
Compare
Greptile SummaryThis PR centralizes native-route admission and fallback dispatch for chat completions, messages, OCR, transcription, and Responses WebSockets
Confidence Score: 4/5The dispatch refactor appears behaviorally sound, but explicit repository requirements and accidentally uncollected tests must be fixed before merging No blocking runtime failure was established, but two tests silently stop running and changed provider handlers violate the repository’s source-comment and immutability requirements Files Needing Attention: tests/test_litellm/llms/custom_httpx/test_llm_http_handler.py, litellm/llms/anthropic/chat/handler.py, litellm/llms/bedrock/chat/converse_handler.py
|
| Filename | Overview |
|---|---|
| litellm/rust_bridge/runtime.py | Introduces shared endpoint selection, preflight, fallback, and upstream-error mapping |
| litellm/rust_bridge/chat_completions.py | Moves native chat discovery and request preparation to the shared completion boundary |
| litellm/rust_bridge/messages.py | Adds public Messages dispatch with provider-aware preflight and Python fallback |
| litellm/ocr/main.py | Moves OCR native preparation into the public dispatch path while supporting provider-native response preservation |
| litellm/rust_bridge/transcription.py | Adds shared transcription preparation and required-native handling for Bedrock |
| tests/test_litellm/llms/custom_httpx/test_llm_http_handler.py | Two renamed test functions no longer match pytest collection conventions |
| litellm/llms/anthropic/chat/handler.py | Removes local native dispatch but adds prohibited commentary and parameter rebinding |
| litellm/llms/bedrock/chat/converse_handler.py | Removes local native dispatch while introducing prohibited comments, parameter rebinding, and mutation |
Reviews (1): Last reviewed commit: 7899a93 | Re-trigger Greptile
|
|
||
|
|
||
| def test_has_agentic_completion_hook_detection(monkeypatch): | ||
| def testhas_agentic_completion_hook_detection(monkeypatch): |
There was a problem hiding this comment.
Both renamed functions miss pytest's test_ prefix, violating the requirement that modified tests preserve coverage and requiring correction before merge
Rule Used: What: Flag any modifications to existing tests and... (source)
| def build_request() -> tuple[dict, dict]: # mutable-ok: rewritten in place downstream | ||
| """Translate the request the Python way, returning `(headers, data)`. | ||
|
|
||
| The pair stays mutable because the streaming path rewrites it in | ||
| place (`data["stream"] = True`) before sending. | ||
| """ |
There was a problem hiding this comment.
The new request-building docstring and headings explain ordinary code, violating the repository's source-comment policy and requiring removal before merge
Context Used: CLAUDE.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!
| python_client: Final = ( | ||
| client if isinstance(client, HTTPHandler) else _get_httpx_client(params={"timeout": timeout}) | ||
| ) | ||
| if client is None or not isinstance(client, HTTPHandler): |
There was a problem hiding this comment.
The changed handlers reassign client and timeout and mutate _params, violating the repository's immutability requirement and requiring correction before merge
Context Used: CLAUDE.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!
ae2e895 to
e5060f8
Compare
e5060f8 to
792d8da
Compare
| else initial_headers | ||
| ) | ||
| log_details: Final[NativePreCallDetails] = { | ||
| "complete_input_dict": {"model": ctx.model, "messages": ctx.messages, **params}, |
There was a problem hiding this comment.
High: Bedrock credentials are copied into pre-call logs
For Bedrock deployments, params contains transport fields such as aws_access_key_id, aws_secret_access_key, aws_session_token, and aws_web_identity_token. Expanding it into complete_input_dict sends those values verbatim to pre-call callbacks, so an authenticated user can trigger disclosure of the deployment's AWS credentials into configured observability systems. Build the logging payload from the provider request body or redact all transport-only fields first; the same issue occurs in litellm/rust_bridge/transcription.py:253.
PR overviewThis PR refactors native request handling so providers are discovered at shared dispatch boundaries, including the Rust bridge paths for chat completions and transcription. A significant issue remains: authenticated Bedrock requests can cause AWS credentials and session tokens to be copied into pre-call callback payloads and configured observability systems. No reported issues have yet been addressed, so the credential-handling problem remains unresolved. Open issues (1)
Fixed/addressed: 0 · PR risk: 7/10 |
792d8da to
35f3894
Compare
|
Closing; stack is being cleaned up. |
TLDR
Problem this solves:
How it solves it:
User Flow
Before: the SDK request can violate this layer contract.
After: the same request follows one predictable contract.
Relevant issues
Linear ticket
Pre-Submission checklist
Stack
39928 -> 39913 -> 39923 -> 39941 -> 39939 -> 39484 -> 39986 -> 40054 -> 40055 -> 40056 -> 39765Layer 5 of 11.
Screenshots / Proof of Fix
After (
e5060f8838df1aa6ca2a54473e58cda191cebec2)UV_PYTHON=3.12 make check: passedNo paid-provider calls were used for deterministic validation.
Type
Bug Fix
Caveats (if any)
Medium
Final Attestation