Repository navigation
fix(responses): drop client_metadata and merge system messages for Databricks chat-only models - #42390
Conversation
Codex CLI sends client_metadata on every /v1/responses call. For a provider with no native Responses config the chat-completions bridge forwarded the raw kwargs, so client_metadata reached the provider as a chat body field and Databricks rejected the request with an unknown field 400. The bridge now drops the Responses-only request fields before calling completion while still passing every other kwarg through, so deployment-level params such as chat_template_kwargs keep reaching providers without a native config.
|
I'll fix CI failures and address comments from users with write access. I'll skip comments containing "(aside)".
|
|
This comment has been minimized.
This comment has been minimized.
Merging this PR will not alter performance
Comparing |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
…models Codex sends instructions plus a leading developer item, which the Responses bridge and the developer-to-system translation turn into two consecutive system messages that Databricks chat-template models reject with "System message must be at the beginning". Each run of consecutive system messages is now merged into one before the request is built for non-Claude models. Also keep client_metadata out of the bridged chat request even when allowed_openai_params names it, so both bridge branches drop the same set.
| import json | ||
| from typing import Final | ||
|
|
||
| import httpx | ||
| import respx | ||
|
|
||
| import litellm | ||
|
|
||
|
|
||
| def test_completion_merges_leading_system_and_developer_messages_for_chat_template_models( | ||
| respx_mock: respx.MockRouter, | ||
| ): | ||
| upstream: Final = respx_mock.post("https://example.databricks.test/serving-endpoints/chat/completions").mock( | ||
| return_value=httpx.Response( | ||
| status_code=200, | ||
| json={ | ||
| "id": "chatcmpl-123", | ||
| "object": "chat.completion", | ||
| "created": 1677652288, | ||
| "model": "my-custom-model", | ||
| "choices": [{"index": 0, "message": {"role": "assistant", "content": "Answer"}, "finish_reason": "stop"}], | ||
| "usage": {"prompt_tokens": 9, "completion_tokens": 1, "total_tokens": 10}, | ||
| }, | ||
| ) | ||
| ) | ||
|
|
||
| response: Final = litellm.completion( | ||
| model="databricks/my-custom-model", | ||
| messages=[ | ||
| {"role": "system", "content": "You are terse."}, | ||
| {"role": "developer", "content": "Skills: none."}, | ||
| {"role": "user", "content": "Hello"}, | ||
| ], | ||
| api_base="https://example.databricks.test/serving-endpoints", | ||
| api_key="fake-databricks-api-key", | ||
| num_retries=0, | ||
| ) | ||
|
|
||
| assert upstream.call_count == 1 | ||
| request_body: Final = json.loads(upstream.calls[0].request.read()) | ||
| assert request_body["messages"] == [ | ||
| {"role": "system", "content": "You are terse.\n\nSkills: none."}, | ||
| {"role": "user", "content": "Hello"}, | ||
| ] | ||
| assert response.choices[0].message.content == "Answer" |
There was a problem hiding this comment.
This bug fix creates a new Databricks test file instead of extending the existing mapped transformation test. Repository instructions require bug-fix regressions to extend that file, so this must be moved 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.
The mirror path tests/test_litellm/llms/databricks/chat/ had no test file for this module; tests/unit is the legacy tree, so this is the mapped file
There was a problem hiding this comment.
You’re right. The tests/test_litellm/llms/databricks/chat/ tree had no pre-existing transformation test, while tests/unit/llms/databricks/chat/ is the legacy suite. Given the repository’s mapped test layout, adding test_databricks_chat_transformation.py at the mirror path is appropriate; there is no existing mapped file to extend. Please disregard my earlier placement comment.
…ones Databricks drops empty content before the merge, so a system message in a run could carry no content key and the merge iterated None. Those messages are now skipped; a run with no content at all keeps its first message.
|
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 3d4dc9c. Configure here.
TLDR
Problem this solves:
client_metadataleaks into the chat completions body Databricks rejectsinstructionsplus its leadingdeveloperskills message become two consecutive system messages, which Databricks chat-template models reject withSystem message must be at the beginningHow it solves it:
client_metadata) for every provider, on both of its brancheschat_template_kwargsstill pass throughUser Flow
Before: a Codex CLI user pointed at the proxy with a Databricks model that has no native Responses config gets a 400 on every turn
model_provider = "litellm"withbase_url = "https://litellm-domain/v1",wire_api = "responses", andmodel = "databricks-qwen35-122b-a10b"in~/.codex/config.toml, runcodex, and type a promptmodel,input(led by adeveloperskills message),instructions,tools,reasoning,stream: true, and its telemetryclient_metadataobject (turn_id,thread_id,session_id, and so on)litellm.BadRequestError: DatabricksException - {"error_code":"BAD_REQUEST","message":"Bad request: json: unknown field \"client_metadata\"\n"}and Codex shows that error in its pane instead of a reply; withclient_metadatagone the same turn still 400s withSystem message must be at the beginning, because theinstructionsand thedevelopermessage reach the model as two consecutive system messagesAfter: the same turn comes back with the model's reply
model_provider = "litellm"withbase_url = "https://litellm-domain/v1",wire_api = "responses", andmodel = "databricks-qwen35-122b-a10b"in~/.codex/config.toml, runcodex, and type a promptmodel,input(led by adeveloperskills message),instructions,tools,reasoning,stream: true, and its telemetryclient_metadataobject (turn_id,thread_id,session_id, and so on)response.created,response.output_text.delta,response.completed), and Codex shows the model's reply in its paneDesign decision
Stated as what the value has to mean rather than the field name: a Responses request served through the chat completions bridge has to reach a chat-only provider as a chat request that provider accepts, while deployment-level provider-specific params keep passing through.
client_metadatais a Responses-surface field with no chat completions equivalent. No provider accepts it in a chat body, and LiteLLM's native Responses dispatch never forwards it either. So the bridge drops it for every provider before calling completion. For a provider with a native Responses config the bridge already filtered it out through the allowlist #41144 added. For a provider without one the bridge returned the raw kwargs, and that is the branch this PR changes: it now returns the kwargs minus a named set of Responses-only request fields, today justclient_metadata, and still forwards everything else. Both branches drop that same set: the native-config branch subtracts it from its allowlist as well, so a deployment whoseallowed_openai_paramsnamesclient_metadatacannot re-add itThree alternatives were considered and rejected for the
client_metadatadrop. Applying the native-config allowlist to providers without a config would also drop deployment-level params such aschat_template_kwargs, which #41144 kept on purpose and which theno-native-config-keeps-passthroughtest pins. Poppingclient_metadatainside the Databricks chat transformation would copy the per-provider pop that the anthropic chat, bedrock converse, and mistral chat transformations already carry for this same leak, and every other provider without a native config would keep leaking, so those three existing pops stay and no fourth one is added. Makingclient_metadataa litellm-level param stripped on every path, chat completions included, would change chat completions behavior nobody asked for, since no chat client sends the field and Codex no longer has a chat wire modeThe second failure has the same shape. Codex sends
instructionsand leadsinputwith adeveloperskills message. The bridge turnsinstructionsinto a system message and LiteLLM's developer-to-system translation turns the developer message into another one, so Databricks chat-template models such as Qwen receive[system, system, user]and reject it, since their chat template accepts one system message at the front. What the messages have to mean is one system prompt in the same order the client wrote it, so Databricks now merges each run of consecutive system messages into one before building the request, next to its other message rewrites for non-Claude models (_split_parallel_tool_calls). All-string runs are joined with a blank line; a run holding list content becomes one list of text parts with the original parts kept as they are, so acache_controlmarker survives. A system message whose content Databricks already stripped as empty is skipped in the merge. Claude models on Databricks go through the Anthropic path, which already folds system messages into thesystemfield, so they are left aloneTwo alternatives were considered and rejected for the merge. Merging in the Responses bridge would fix Codex only: the same
[system, developer, user]history reaches Databricks through/chat/completionsdirectly from any OpenAI SDK client that sends a developer message, and the bridge has no way to know which providers accept a single system message, so the rewrite belongs with the provider. Overridingtranslate_developer_role_to_system_roleonDatabricksConfigto fold the developer message into the preceding system message would only cover the developer-derived case and miss two literal system messages, which the same clients produce, and it would put a message rewrite in the translation hook instead of next to Databricks' other message rewritesRelevant issues
Fixes #42347
The issue reports two failure modes on the same route. Its headline
reasoning_effortsymptom no longer reproduces onmain(the bridge already sends the effort as a plain string), so theclient_metadataleak from the issue's second comment is the failure mode left, and this PR fixes itAffected release
Linear ticket
Resolves LIT-8304
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
Live before and after against the real Databricks workspace (real spend), each leg a proxy booted with
--num_workers 2from the QA config below, driven first with curl and then with the real OpenAI Codex CLI 0.155.1 TUI under tmux (wire_api = "responses",model = "databricks-qwen35-122b-a10b"). Before: the merge based22a6806df(PYTHONPATH=<base worktree>,python -c 'import litellm; print(litellm.__file__)'printed the base worktree'slitellm/__init__.py). After: this PR's tip3d4dc9cf7aBefore (
d22a6806df){"error":{"message":"litellm.BadRequestError: DatabricksException - {\"error_code\":\"BAD_REQUEST\",\"message\":\"Bad request: json: unknown field \\\"client_metadata\\\"\\n\"}. Received Model Group=databricks-qwen35-122b-a10b\nAvailable Model Group Fallbacks=None","type":"invalid_request_error","param":null,"code":"400"}}Codex TUI, same proxy, prompt
Reply with the single word pong and nothing else:With only the first commit of this PR (
064506c9d9, theclient_metadatadrop), the curl above already returned 200, but the same Codex turn still failed, which is why the second commit exists:Direct curls to the Databricks endpoint (no LiteLLM) pin the model behavior:
[system, user]200,[system, system, user]400System message must be at the beginning,[system, developer, user]400Unexpected message role, whiledatabricks-llama-4-maverickaccepts all threeAfter (
3d4dc9cf7a)Same curl, port of the after proxy:
HTTP 200, output trimmed to the fields that matter:
{"status": "completed", "model": "databricks-qwen35-122b-a10b", "output": [{"type": "reasoning"}, {"type": "message", "content": [{"type": "output_text", "text": "pong"}]}], "error": null}The Codex wire shape on record too,
instructionsplus a leadingdeveloperitem withinput_textparts andclient_metadata, the exact combination that 400'd twice before:{"status": "completed", "output_text": ["pong"], "error": null}Codex TUI, same proxy, same prompt:
Observations from the run:
/responsesis not offered for these endpointsType
🐛 Bug Fix
Caveats (if any)
Risk summary from a live base-vs-tip A/B of the changed symbols' dependents:
_bridge_kwargsis private with one caller inlitellm/responses/main.py,merge_consecutive_system_messagesis new with one caller, and no public signature or config key changesclient_metadatato any chat provider, even when a deployment'sallowed_openai_paramsnames it (no chat provider accepts the field). Databricks non-Claude chat requests carrying consecutive system messages now arrive as one system message with the same text in the same ordercache_controlsurvives. Claude models on Databricks take the Anthropic path and are untouchedDatabricksConfig._transform_messagesserves both the sync and async Databricks chat paths and is covered by the new transformation test plus the existingtests/unit/llms/databricks/chatsuite (224 pass). The bridge flag tests pinno-native-config-keeps-passthroughnext to the two new drop tests/v1/chat/completionsrequest with[system, developer, user]to Databricks through the proxy (unit test only), Llama through the proxy after the merge (direct curls only), a deployment withallowed_openai_params: [client_metadata](unit test only), and Claude on Databricks (code path untouched, not run)unit,llm_translation_testing,local_testing_part1,integration-cost,integration-providers,e2e_ui_testing) on exactly the tests that fail on main pipelines 90021 and 90023 (secret-detection timing, Fireworks inline images, Bedrock model-info drift, cost breakdown persistence, fal video wire, the MCP tools Playwright spec), none in a file this PR touches.mcp-integration / Run testspassed 422 tests and failed only on the coverage-artifact upload (403), so it was rerun. Every required check is greenFinal Attestation
Note
Medium Risk
Changes request shaping on the Responses-to-chat bridge and Databricks message transforms; behavior is intentional but alters forwarded kwargs and how multi-system histories are sent to non-Claude Databricks models.
Overview
Fixes Codex-style Responses → chat completions bridging for Databricks chat-template models by stripping Responses-only payload and normalizing system prompts before the upstream call.
The Responses bridge now always removes
client_metadatawhen forwarding to chat completions—on both the allowlisted native-config path and the passthrough path for providers without a Responses config—so it cannot leak even ifallowed_openai_paramsnames it. Provider-specific fields such aschat_template_kwargsstill pass through.For Databricks non-Claude chat, consecutive
systemmessages are merged into one (strings joined with blank lines; mixed or list content flattened to text parts while preserving markers likecache_control) immediately before existing parallel tool-call splitting, so patterns likeinstructionsplus a leadingdevelopermessage no longer trigger “system message must be at the beginning.”Reviewed by Cursor Bugbot for commit 3d4dc9c. Bugbot is set up for automated code reviews on this repo. Configure here.