Repository navigation
test(e2e): add conversational matrix across chat, messages and responses - #42359
Conversation
Parameterizes one behavioral contract (reply, stream, cost log, tool call, tool round trip) across /v1/chat/completions, /v1/messages and /v1/responses, OpenAI and Anthropic models, and env-ref vs stored-credential auth, with record/replay fixtures. Adds general_settings.disable_model_info_refresh so the proxy fronting a replay fixture does not poll every OpenAI-compatible deployment's /v1/models in the background, which otherwise leaves unconsumed interactions in the recorded bundle. Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
|
I'll fix CI failures and address comments from users with write access. I'll skip comments containing "(aside)".
|
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
…er to Deployment Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
1 similar comment
| @dataclass(frozen=True, slots=True) | ||
| class Cell: | ||
| surface: SurfaceName | ||
| deployment: Deployment | ||
| auth: AuthMethod | ||
|
|
||
| @property | ||
| def id(self) -> str: | ||
| return f"{self.surface}-{self.deployment.label}-{self.auth}" | ||
|
|
||
| def registry_id(self, capability: Capability, streaming: Streaming, assertion: Assertion) -> str: | ||
| return f"llm.{self.surface}.{self.deployment.route}.{capability}.{streaming}.{assertion}" | ||
|
|
||
|
|
||
| CELLS: Final[tuple[Cell, ...]] = tuple( | ||
| Cell(surface=surface, deployment=deployment, auth=auth) | ||
| for surface in SURFACES | ||
| for deployment in DEPLOYMENTS | ||
| for auth in AUTH_METHODS | ||
| ) |
There was a problem hiding this comment.
beautifully done
|
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 bb4f670. Configure here.
TLDR
Problem this solves:
/v1/chat/completions,/v1/messages,/v1/responsesand get fixed one endpoint at a time/v1/modelsdiscovery makes strict record/replay of OpenAI deployments non-deterministicHow it solves it:
Deploymentrow inDEPLOYMENTS, no new test codereplayable: live, record, and replay all pass, replay spends nothinggeneral_settings.disable_model_info_refreshturns off the background poller, set in the record/replay configUser Flow
Before: a maintainer adding a shared helper across the three chat endpoints has no single test run that tells them whether all three still behave the same
/v1/chat/completions,/v1/messagesand/v1/responses/v1/messagespasses unnoticed because only the chat suite checks tool-call ids/v1/messagesregression, and the fix plus test gets added to the messages suite onlyAfter: the same maintainer runs one suite and sees the same 5 checks for every endpoint, provider, model and auth method
E2E_FIXTURE_MODE=replay pytest tests/e2e/llm_translation/test_conversational_matrix_e2e.pyagainst a local proxy in under 4 minutes with no provider spendtest_tool_call_is_returned_named_and_addressable[messages-claude-haiku-4-5-env_ref]fails with "messages tool call has no id, so the caller cannot answer it", naming exactly which endpoint, model and auth path regressedDeployment(...)row and re-record; all 30 per-model cases exist immediatelydisable_model_info_refresh: trueundergeneral_settings, restarts, and replay consumes every recorded interactionRelevant issues
Affected release
Linear ticket
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)Screenshots / Proof of Fix
Shared setup. Proxy booted from
tests/e2e/gateway/record_replay_ci_config.ymlwith Postgres,OPENAI_API_KEYandLITELLM_MASTER_KEYin the environment. A tiny HTTP sniffer listens on127.0.0.1:4999and logs every request it receives (auth header redacted). Two deployments exist in the DB before either boot:The chat request used in both runs:
Before (1a4e2b1)
Background /v1/models discovery hits the upstream on its own
Uvicorn runningat 22:56:37 UTC), send no requests, watch the snifferdisable_model_info_refresh: trueis not a recognized setting at this commit, so there is nothing an operator can set to stop itChat completion through the proxy
After (bb4f670)
Background /v1/models discovery hits the upstream on its own
disable_model_info_refresh: trueingeneral_settings(boot at 23:05:05 UTC), send no requests, watch the sniffer for 110 sGET /v1/modelson the proxy itself still lists both deployments, so nothing else about model registration changedChat completion through the proxy
Matrix runs against the same proxy at the tip, all three lanes, for the record (these are on top of the curl proof, not instead of it):
The tool tests force the call rather than hoping for it:
tool_choicenamesget_weatherand parallel calls are off on the first turn (tool_choice={"type":"function","function":{"name":"get_weather"}}, parallel_tool_calls=falseon chat,{"type":"tool","name":"get_weather","disable_parallel_tool_use":true}on Messages,{"type":"function","name":"get_weather"}, parallel_tool_calls=falseon Responses), so the assertion is exactly oneget_weathercall and a miss is a translation bug in litellm, not the model's mood. The second turn offers the tool without forcing it, so the model can answer in text. The recorded bundle shows the forced choice reaching both providers: 16 upstream bodies carry"parallel_tool_calls": falseand 12 carry"disable_parallel_tool_use": trueMutation check for the unit test: removing the
if model_info_scheduler is not Noneguard (always scheduling) makestest_proxy_startup_event_honors_disable_model_info_refresh[True-False]fail withdisable_model_info_refresh=True but refresh_model_info job is <Job ...>; restoring it goes greenType
🆕 New Feature
✅ Test
Caveats (if any)
Medium
disable_model_info_refreshalso hides discoveredmax_model_lenfrom/model/infofor hosted vLLM deployments; it is off by default and only set in the e2e record/replay configLow
test_lifecycle.pyhas pre-existing ruff findings (unused imports); untouched to keep the diff to the new testQA runbook
Prerequisites: a proxy on http://localhost:4000 booted from
tests/e2e/gateway/record_replay_ci_config.ymlwith Postgres,OPENAI_API_KEY,ANTHROPIC_API_KEYandLITELLM_MASTER_KEY($LITELLM_MASTER_KEYbelow). Each test runs once per cell, a cell being endpoint x model x auth: endpointschat_completions(/v1/chat/completions),messages(/v1/messages),responses(/v1/responses); modelsopenai/gpt-4o-mini,openai/gpt-5.4-mini,anthropic/claude-haiku-4-5; authenv_ref("api_key":"os.environ/OPENAI_API_KEY") orstored_credential(a/credentialsentry referenced bylitellm_credential_name). The steps below use thechat_completions-gpt-4o-mini-stored_credentialcell; swap the route and body for the other cellstests/e2e/llm_translation/test_conversational_matrix_e2e.py::TestConversationalMatrix::test_reply_carries_assistant_text_and_usage - a non-streaming reply on every cell carries an id, assistant text, positive usage, and an
x-litellm-call-id{"credential_name":"qa-cred","credential_values":{"api_key":"<real OPENAI key>"}}{"model_name":"qa-chat","litellm_params":{"model":"openai/gpt-4o-mini","litellm_credential_name":"qa-cred"}}; poll GET /v1/models untilqa-chatappears{}and use the returned key for the LLM call{"model":"qa-chat","messages":[{"role":"user","content":"Say hello."}]}id, non-emptychoices[0].message.content,usage.prompt_tokens > 0,usage.completion_tokens > 0, and anx-litellm-call-idresponse headertests/e2e/llm_translation/test_conversational_matrix_e2e.py::TestConversationalMatrix::test_stream_delivers_text_usage_and_a_terminal_event - a streamed reply on every cell arrives as more than one event, carries text, ends with a terminal event, and reports usage
"stream":true,"stream_options":{"include_usage":true}(for/v1/messagesand/v1/responsesplain"stream":truesuffices)delta.content, a chunk withfinish_reasonset, and a final chunk whoseusageis non-null (Messages: amessage_deltawithusage; Responses:response.completedwithresponse.usage)tests/e2e/llm_translation/test_conversational_matrix_e2e.py::TestConversationalMatrix::test_cost_header_matches_the_spend_log - on every cell
x-litellm-response-costis positive and equals the one priced/spend/logsrow written for a fresh key, which also carries token counts and the deployment's model{}for a brand new key so it has no prior spend rowsx-litellm-response-costfrom the headers and expect it > 0spend > 0appearsprompt_tokens > 0,completion_tokens > 0,modelequal togpt-4o-mini, andspendwithin 1% of the header valuetests/e2e/llm_translation/test_conversational_matrix_e2e.py::TestConversationalMatrix::test_tool_call_is_returned_named_and_addressable - on every cell a prompt that needs the
get_weathertool produces exactly that tool call, with a non-empty call id and JSON arguments that keep the requested location"tools":[{"type":"function","function":{"name":"get_weather","parameters":{"type":"object","properties":{"location":{"type":"string"}},"required":["location"]}}}],"tool_choice":{"type":"function","function":{"name":"get_weather"}},"parallel_tool_calls":falseand the user message "What is the weather in Paris right now? Use the get_weather tool." (Messages:"tool_choice":{"type":"tool","name":"get_weather","disable_parallel_tool_use":true}; Responses:"tool_choice":{"type":"function","name":"get_weather"},"parallel_tool_calls":false)choices[0].message.tool_callswith exactly one entry, namedget_weather, a non-emptyid, andargumentsthat parse to JSON withlocationcontaining "paris" (Messages: atool_useblock withid,name,input; Responses: afunction_callitem withcall_id,name,arguments)tests/e2e/llm_translation/test_conversational_matrix_e2e.py::TestConversationalMatrix::test_tool_result_round_trip_reaches_the_model - on every cell, feeding the tool result back under the model's own call id yields a final answer that repeats the returned 22 degrees
"Paris: 22 degrees Celsius, clear skies"addressed to that id (chat:role":"tool","tool_call_id"; Messages:tool_resultblock withtool_use_id; Responses:function_call_outputwithcall_id)Nuances a manual run will hit: the proxy must have the
qa-chatalias propagated before the first LLM call (poll/v1/models) and/v1/messagesneedsmax_tokens(the suite sends 512 everywhere)Final Attestation
Link to Devin session: https://app.devin.ai/sessions/3990b1604bb34b33aa75d198337d630a
Open in Devin Desktop: https://app.devin.ai/desktop/session/3990b1604bb34b33aa75d198337d630a?variant=devin
Requested by: @mateo-berri
Note
Low Risk
Opt-in startup flag skips background model polling; default unchanged. Most of the diff is new e2e coverage, not production path changes.
Overview
Adds a replayable e2e matrix that runs the same five conversation checks (basic reply/stream, cost vs spend log, forced tool call, tool-result round trip) on every combination of chat completions / messages / responses, OpenAI + Anthropic deployments, and env-ref vs stored credentials, via shared
Surfaceadapters inconversational_matrix.py. Coverage registry entries point at the new suite.Introduces
general_settings.disable_model_info_refresh: whentrue, the proxy does not schedule the periodicrefresh_model_infojob (background/v1/modelspolling). E2e record/replay config turns this on so fixtures stay deterministic; default behavior is unchanged. A lifecycle unit test pins the flag.Risk: Low for production (opt-in flag, tests dominate). Operators who disable refresh lose automatic discovered model metadata (noted in PR caveats).
Reviewed by Cursor Bugbot for commit bb4f670. Bugbot is set up for automated code reviews on this repo. Configure here.