Repository navigation
fix(databricks): translate reasoning_effort to thinking for Gemini 2.5 - #40909
mateo-berri merged 15 commits into
Conversation
…aging, resolving conflicts Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…on schema Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…ture Addresses Greptile P2 feedback: reject an adaptive thinking payload for non-Claude Databricks models instead of silently emitting a shape the Gemini endpoint can't parse, and scope the local-cost-map fixture to the tests that need it rather than autouse. Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…i-reasoning-effort Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…us/sonnet 4-6 Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…i-reasoning-effort Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…i-reasoning-effort
…hinking payload Gate the reasoning_effort translation on the cost-map flag or the model name containing claude, so unmapped Claude serving endpoints keep translating. Flag the newer Claude entries that were missing it. Expose supports_anthropic_thinking_payload as a public helper next to the other supports_* wrappers instead of importing the private factory. Drop the adaptive-only guard, since the adaptive flags only ever match Claude ids, and add regression tests for an unmapped Claude endpoint and an adaptive Claude model
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
Greptile SummaryThe PR translates
Confidence Score: 5/5The PR appears safe to merge with no outstanding actionable findings. The metadata-driven translation is consistently wired through the cost maps, schema, model information, request transformation, and focused tests. The previous hardcoded-capability finding was withdrawn after mateo-berri explained that the Claude name check preserves compatibility for user-named endpoints absent from the cost map, and the subsequent Greptile reply conceded the point.
|
| Filename | Overview |
|---|---|
| litellm/llms/databricks/chat/transformation.py | Uses model metadata plus the established Claude compatibility fallback to select Anthropic-style reasoning translation. |
| model_prices_and_context_window.json | Marks Databricks models that require the Anthropic thinking payload. |
| litellm/utils.py | Exposes and propagates the new model capability through existing model-info infrastructure. |
| model_prices_and_context_window.schema.json | Adds schema validation for the new boolean capability. |
| tests/test_litellm/llms/databricks/chat/test_databricks_chat_transformation.py | Covers translated and pass-through reasoning behavior across relevant Databricks model families. |
| .circleci/config.yml | Removes a stale OCR test path whose referenced file no longer exists. |
Reviews (3): Last reviewed commit: "ci: drop the removed rust bridge test pa..." | Re-trigger Greptile
| from litellm.utils import supports_anthropic_thinking_payload | ||
|
|
||
| normalized: Final = model.lower().replace(".", "-") | ||
| return "claude" in normalized or supports_anthropic_thinking_payload( |
There was a problem hiding this comment.
The literal Claude check hardcodes model capability detection. Repository rules require model-specific capabilities to come from model metadata before merging
Rule Used: What: Do not hardcode model-specific flags in the codebase. Instead, put them in model_prices_and_context_window.json and then read them in via get_model_info Why: Prevents need for users to upgrade litellm each time a new model supports this featu... (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.
The name check predates this PR and covers user-named Databricks Claude endpoints that have no cost-map entry. Removing it would 400 requests that work today
There was a problem hiding this comment.
You're right — removing the name check would be a backwards-incompatible change for user-named Databricks Claude endpoints that are absent from the cost map. The metadata flag is the preferred path for mapped models, while "claude" in normalized is a necessary compatibility fallback for unmapped endpoints. I withdraw this comment.
|
bugbot run |
|
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 a7ebc12. Configure here.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
9d98437
into
litellm_internal_staging
Replaces #34001 at head c7b607c, moved onto a
litellm_branch with the same commits and authors so CircleCI runs the legacy Databricks suitesTLDR
Problem this solves:
reasoning_efforton a Databricks Gemini 2.5 model came back as a 400 from Databricksthinkingwhen the model name containedclaudedrop_paramsdid not help, since Databricks reportsreasoning_effortas supported everywhereHow it solves it:
supports_anthropic_thinking_payload, marks which Databricks models take the Anthropicthinkingshapereasoning_effortthrough untouched, as Databricks documentsAlso in this PR, one CI line: the
ocr_testingCircleCI job listedtests/test_litellm/ocr/test_rust_bridge.pynext to its glob, and #40734 removed that file from the base branch without touching the job, so pytest crashed at collection on every worker ("4 workers [0 items]", exit 123) for any PR that reached the job. This PR was the first to run it since; dropping the stale path lets the job run the fourtests/ocr_testssuites for real againUser Flow
Before: a developer calling a Databricks-hosted Gemini 2.5 model through the gateway with
reasoning_effortgets a 400 back, while the same request against a Databricks Claude model works"model": "databricks-gemini-2-5-flash", a user message, and"reasoning_effort": "low"Invalid JSON payload received. Unknown name "reasoning_effort" at 'generation_config': Cannot find field.reasoning_effortand get a 200, so they cannot control thinking on that model through the gatewayAfter: the same request returns 200 and the model reasons at the requested effort
"model": "databricks-gemini-2-5-flash", a user message, and"reasoning_effort": "low"Relevant issues
Replacement for #34001, which was the reopened copy of #32786 with the merge conflicts against
litellm_internal_stagingresolved. The fix and its tests come from #32786 (head 2bff743); its author could not carry it further. #34001 sat on adevin/head that CircleCI's branch filter skips, so this PR carries the same commits on alitellm_branchLinear ticket
Resolves LIT-7665
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
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@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
Both legs below ran at c7b607c, the last commit that touches proxy code. The commit after it (a7ebc12) changes only
.circleci/config.yml, which cannot change what the proxy sends, so the proof stands at that hashShared setup for both legs. One proxy per leg runs from a worktree checked out at that leg's commit, with the config below, a random free port,
--detailed_debugso the payload the proxy sends to Databricks shows up in the log as a-d '{...}'line, andLITELLM_LOCAL_MODEL_COST_MAP=Trueso the cost map on the checkout is the one loaded. The Databricks workspace used here serves Claude and GPT-OSS but has no Gemini serving endpoint, so the Gemini 2.5 cases show the payload the proxy sends upstream (the part Databricks rejects with the 400) and the workspace answers 404 on both legs; the Claude and GPT-OSS cases are full round trips against the real endpointsThe same request loop runs against each leg, followed by the grep that pulls the upstream payloads out of the log:
Before (559247f)
Gemini 2.5 Flash, /v1/chat/completions, reasoning_effort low
"model": "databricks-gemini-2-5-flash"and"reasoning_effort": "low"HTTP 404from this workspace,litellm.NotFoundError: DatabricksException - {"error_code":"ENDPOINT_NOT_FOUND", ...}reasoning_effortsent as is, which is what a workspace that serves the model rejects withUnknown name "reasoning_effort" at 'generation_config':Claude Sonnet 4.5, /v1/chat/completions, reasoning_effort low
"model": "databricks-claude-sonnet-4-5"and"reasoning_effort": "low"HTTP 200,"content": "The sky is blue because molecules in Earth's atmosphere scatter short-wavelength blue light ...", with"reasoning_content"filled inClaude Opus 4.7, /v1/chat/completions, reasoning_effort high
"model": "databricks-claude-opus-4-7"and"reasoning_effort": "high"HTTP 200,"content": "The sky appears blue because molecules in Earth's atmosphere scatter shorter wavelengths of sunlight (blue) ..."GPT-OSS 120B, /v1/chat/completions, reasoning_effort low
"model": "databricks-gpt-oss-120b"and"reasoning_effort": "low"HTTP 200,"content": "The sky appears blue because molecules in Earth's atmosphere scatter shorter (blue) wavelengths ...","reasoning_content": "Provide concise answer."Gemini 2.5 Flash, /v1/responses, reasoning.effort low
"model": "databricks-gemini-2-5-flash"and"reasoning": {"effort": "low"}HTTP 404from this workspace, sameENDPOINT_NOT_FOUNDbodyreasoning_effort:Claude Sonnet 4.5, /v1/responses, reasoning.effort low
"model": "databricks-claude-sonnet-4-5"and"reasoning": {"effort": "low"}HTTP 200, aresp_...object with the answer'thinking': {'type': 'enabled', 'budget_tokens': 1024}, 'max_tokens': 5120After (c7b607c)
Gemini 2.5 Flash, /v1/chat/completions, reasoning_effort low
"model": "databricks-gemini-2-5-flash"and"reasoning_effort": "low"HTTP 404from this workspace, sameENDPOINT_NOT_FOUNDbody, since it has no Gemini endpointthinkingand noreasoning_effort, the shape Databricks accepts for Gemini 2.5:Claude Sonnet 4.5, /v1/chat/completions, reasoning_effort low
"model": "databricks-claude-sonnet-4-5"and"reasoning_effort": "low"HTTP 200,"content": "The sky appears blue because molecules in Earth's atmosphere scatter shorter blue wavelengths ...", with"reasoning_content"filled inClaude Opus 4.7, /v1/chat/completions, reasoning_effort high
"model": "databricks-claude-opus-4-7"and"reasoning_effort": "high"HTTP 200,"content": "The sky appears blue because molecules in Earth's atmosphere scatter shorter blue wavelengths of sunlight more strongly ..."GPT-OSS 120B, /v1/chat/completions, reasoning_effort low
"model": "databricks-gpt-oss-120b"and"reasoning_effort": "low"HTTP 200,"content": "The sky appears blue because molecules in Earth's atmosphere scatter shorter (blue) wavelengths ...","reasoning_content": "Provide concise answer."reasoning_effortstill passes through for the models that take it natively:Gemini 2.5 Flash, /v1/responses, reasoning.effort low
"model": "databricks-gemini-2-5-flash"and"reasoning": {"effort": "low"}HTTP 404from this workspace, sameENDPOINT_NOT_FOUNDbodythinkinginstead ofreasoning_effort, so the Responses API route gets the same fix:Claude Sonnet 4.5, /v1/responses, reasoning.effort low
"model": "databricks-claude-sonnet-4-5"and"reasoning": {"effort": "low"}HTTP 200, aresp_...object with the answer'thinking': {'type': 'enabled', 'budget_tokens': 1024}, 'max_tokens': 5120Type
🐛 Bug Fix
Caveats (if any)
Medium
GET /api/2.0/serving-endpoints), so both legs get a 404 there; the Claude and GPT-OSS cases are full round trips through the same gate and mapping. A workspace whose region servesdatabricks-gemini-2-5-flashis what closes itreasoning_effort: "none"on a flagged non-Claude model follows the shared Anthropic mapping: the proxy sends nothinkingkey and the model's own default applies, which for Gemini 2.5 means thinking stays on. The alternative, sendingthinking: {"type": "disabled"}for those models, was not taken because the QA workspace cannot confirm Databricks accepts it for Gemini 2.5 and a wrong guess turnsnoneback into the 400 this PR removes. A caller who needs thinking off passes"thinking": {"type": "disabled"}explicitly, which the proxy forwards unchangedLow
max_tokensgetsmax_tokensset to the thinking budget plus 4096 (5120 forlow), the same default the Anthropic, Bedrock and Vertex Claude paths apply and Databricks Claude already had; Databricks requiresbudget_tokensbelowmax_tokensfor Claude and documents nothing for Gemini 2.5. Passingmax_tokensoverrides itclaudestill passesreasoning_effortthrough unchanged. That is the documented behavior for Gemini 3, GPT-5 and GPT-OSS. A custom-named endpoint that needs the translation getsmodel_info: {supports_anthropic_thinking_payload: true}on its proxy config deployment, which registers the flag through the routerreasoning_effortnatively, so the Gemini part of this fix has a short window; the flag keeps working for Claude endpointsthinkingtokens guard from fix(databricks): translate reasoning_effort to thinking for Gemini 2.5 #32786 already landed on the base branch on its own, so it is not part of this diffFinal Attestation
Link to Devin session: https://app.devin.ai/sessions/aacc049712434d50888cf457809a50b1
Note
Medium Risk
Changes upstream request shaping for Databricks chat completions; wrong flags could mis-translate reasoning params for some endpoints, though behavior for unflagged models is unchanged.
Overview
Fixes Databricks chat requests that sent
reasoning_efforton models that only accept the Anthropic-stylethinkingpayload (notably Gemini 2.5), which caused upstream 400s.Databricks mapping now decides translation via
_uses_anthropic_thinking_param(): Claude names still match, and any model withsupports_anthropic_thinking_payloadin the cost map (or deploymodel_info) getsreasoning_effortmapped throughAnthropicConfig._map_reasoning_effortand stripped from the outbound request. Gemini 3, GPT-5, and GPT-OSS stay on passthroughreasoning_effort.Adds the new capability flag end-to-end (
supports_anthropic_thinking_payloadhelper, types, schema, and entries on Databricks Claude + Gemini 2.5 models in the price/context JSON), plus unit tests for Claude, adaptive Claude, Gemini 2.5/2.0/3, and GPT paths.CI: the
ocr_testingjob drops a hard-coded reference to removedtests/test_litellm/ocr/test_rust_bridge.pyso pytest collection succeeds again.Reviewed by Cursor Bugbot for commit a7ebc12. Bugbot is set up for automated code reviews on this repo. Configure here.