test(e2e): cover /chat/completions on Azure OpenAI, streaming and non-streaming - #33468
test(e2e): cover /chat/completions on Azure OpenAI, streaming and non-streaming#33468mateo-berri wants to merge 6 commits into
Conversation
Greptile SummaryAdds live e2e coverage for
Confidence Score: 4/5All changes are confined to the tests/e2e/ directory and add new coverage without touching production code; safe to merge once the minor config and test-diagnostic points are addressed. The Azure docker-compose entry omits an explicit api_version, which could cause silent failures on team members' machines whose LiteLLM default doesn't match the deployment's required version. The streaming test's unguarded model_validate_json will surface a raw Pydantic exception rather than a descriptive assertion message if an unexpected SSE payload arrives. Neither issue affects production code or masks a real regression, but they reduce reproducibility and diagnostic clarity of the new test suite. tests/e2e/docker-compose.yml (missing api_version for the Azure deployment) and tests/e2e/llm_translation/test_chat_completions_regression_e2e.py (unguarded model_validate_json on line 116)
|
| Filename | Overview |
|---|---|
| tests/e2e/docker-compose.yml | Adds azure-gpt-5.4-mini model config; missing explicit api_version may cause silent failures on environments where LiteLLM's default doesn't match the deployment's required version |
| tests/e2e/e2e_http.py | Refactors _streaming_outcome to eagerly buffer all SSE lines and adds events tuple capturing data: payloads; chunks/stream_error semantics preserved, but stream is now fully buffered in memory |
| tests/e2e/llm_translation/test_chat_completions_regression_e2e.py | Adds Azure OpenAI to parametrized non-streaming test and a new streaming test with thorough SSE assertions; unguarded model_validate_json may surface ValidationError instead of clean AssertionError on unexpected payloads |
| tests/e2e/models.py | Adds ChatStreamDelta/ChatStreamChoice/ChatStreamChunk Pydantic models for the streaming test; all fields are correctly optional to handle partial SSE chunks |
| tests/e2e/coverage_registry/llm_conversational.yaml | Adds missing llm.chat_completions.azure_openai.basic.stream.works registry entry; straightforward and correctly structured |
Reviews (1): Last reviewed commit: "test(e2e): cover /chat/completions on Az..." | Re-trigger Greptile
| - model_name: azure-gpt-5.4-mini | ||
| litellm_params: | ||
| model: azure/gpt-5.4-mini | ||
| api_base: os.environ/AZURE_API_BASE | ||
| api_key: os.environ/AZURE_API_KEY |
There was a problem hiding this comment.
The Azure deployment config is missing an explicit
api_version. LiteLLM's Azure provider falls back to an internal default when none is set; if the team's gpt-5.4-mini deployment requires a newer preview API version (e.g. 2025-01-01-preview), CI runs on other machines will silently get api_version mismatch errors that look like model-not-found failures rather than a config problem. Adding it explicitly here makes the requirement visible and matches the QA runbook's intent of a fully reproducible environment.
| - model_name: azure-gpt-5.4-mini | |
| litellm_params: | |
| model: azure/gpt-5.4-mini | |
| api_base: os.environ/AZURE_API_BASE | |
| api_key: os.environ/AZURE_API_KEY | |
| - model_name: azure-gpt-5.4-mini | |
| litellm_params: | |
| model: azure/gpt-5.4-mini | |
| api_base: os.environ/AZURE_API_BASE | |
| api_key: os.environ/AZURE_API_KEY | |
| api_version: os.environ/AZURE_API_VERSION |
| chunks = [ | ||
| ChatStreamChunk.model_validate_json(event) for event in result.events[:-1] |
There was a problem hiding this comment.
Unguarded
model_validate_json raises ValidationError instead of AssertionError
If any data: line before [DONE] is not valid JSON (e.g. an unexpected SSE keep-alive or a non-standard Azure error payload that evades the stream_error detector), model_validate_json raises a raw Pydantic ValidationError. This surfaces in pytest output as an exception traceback rather than the descriptive assertion messages used everywhere else in this test, making triage harder. Wrapping the list comprehension in a try/except ValidationError and re-raising as AssertionError with the raw event would match the diagnostic style of the rest of the file.
| raw_lines = tuple( | ||
| line.decode(errors="replace") | ||
| for line in cast("Iterator[bytes]", resp.iter_lines()) | ||
| if line | ||
| ) |
There was a problem hiding this comment.
Entire stream buffered eagerly into memory
The previous implementation iterated lazily; the new code materialises all decoded lines into a tuple before any processing happens. For the current tests this is harmless, but any future streaming test that generates long completions (e.g. a 128 k-token context replay) will silently buffer the full response in the test process before any assertion fires. The change is intentional (needed to reuse raw_lines for both stream_error and events), but a note-to-self comment would help future authors understand why the eager approach was chosen over a two-pass iterator or a collections.deque.
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!
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
…itellm_e2e_azure_openai_chat
… the dual token-param bug Adds two customer-regression rows to the Azure chat e2e suite. The first sends reasoning_effort=none to a custom-named deployment (gpt-5.6-sol-e2e, swappable via E2E_AZURE_CUSTOM_MODEL) whose capabilities resolve through base_model, asserting the request completes with zero reasoning tokens on a prompt that reasons at default effort, so both a gate 400 (GH #31243, SDK fix in PR #28490) and a silently dropped param fail the row. The second, a strict xfail until GH #31614 is fixed, sends a client max_completion_tokens to a gpt-4o deployment carrying a config-level max_tokens default; the proxy forwards both and Azure rejects the pair
Adapt the Azure OpenAI chat coverage onto the refactored e2e harness: drop tests/e2e/docker-compose.yml (removed upstream in #33837) and register the Azure deployments via /model/new inside the test with teardown, keep the upstream stream_events transport and add a stream_done terminator flag, port the ChatStreamChunk models, and extend LiteLLMParamsBody/ModelInfoBody with the max_tokens, drop_params, and base_model fields the Azure cases need Also drop the strict xfail from test_azure_config_token_cap_with_client_max_completion_tokens now that GH #31614 is fixed: AzureOpenAIConfig.map_openai_params skips max_tokens when the client also sends max_completion_tokens (PR #34214), so the row asserts the completion succeeds and guards against regression
Union the import block with the customer chat/messages coverage that landed in #34164; the Azure classes and the new provider classes coexist unchanged
TLDR
Problem this solves:
How it solves it:
Relevant issues
Guards #31243 (the SDK-level gate was fixed by #28490; this adds the e2e regression net). Guards #31614: the fix is #34214, which must merge before this PR, and the former strict-xfail row now asserts the deduped success outcome
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)Screenshots / Proof of Fix
All runs below hit the real
gpt-5.6-sol/gpt-5.6-terra/gpt-5.6-luna/gpt-5.6-sol-e2e/gpt-4odeployments on the shared Azure OpenAI e2e resource, costing real money; the tests register each deployment through/model/newand delete it on teardown. Earlier captures from the pre-rebase compose harness (commits 55f58cd and 187cab5) are preserved in this description's edit historyAfter: the full suite at merge commit 3156841, against a proxy running the #31614 fix (commit db30fa0 of #34214,
litellm --config e2e-verify-config.yml --port 52731with the example models prewired andstore_model_in_db: true). Every row passes, including the un-xfailed dedup row. Re-verified at head fdb3e1a (which merges the #34164 provider coverage into this file) with the identical 12-passed result(the deselected Cohere and hosted_vllm rows are pre-existing staging tests whose credentials are not available in this environment, as are the OpenAI/Bedrock provider classes #34164 added to this file after the first verification run)
Before: the same dedup row at the same merge commit 3156841, against a proxy built from this PR's own tree, which does not contain #34214 (port 52741, identical config). The row fails on the exact customer-visible Azure 400, proving it guards the regression
Type
✅ Test
Changes
Adds live e2e coverage proving
/chat/completionsreturns a well-formed 200 on Azure OpenAI in both modes, across all three GPT-5.6 capability tiers (sol, terra, luna).TestAzureOpenAIChatregisters each deployment through/model/newagainstAZURE_API_BASE/AZURE_API_KEYand deletes it on teardown, following the harness idiom the sibling Cohere/Gemini/hosted_vllm classes use. The nonstream rows assert a real model name and a non-empty assistant message per the #28991 standard. The streaming rows apply that same standard to the SSE path: the response must betext/event-stream, carry no in-stream error event, terminate with[DONE], everydata:event must parse as achat.completion.chunkwith a model name, and the deltas must reassemble into non-empty textThe deployment names are env-swappable:
e2e_configreadsE2E_AZURE_SOL_MODEL/E2E_AZURE_TERRA_MODEL/E2E_AZURE_LUNA_MODEL/E2E_AZURE_CUSTOM_MODEL(+E2E_AZURE_CUSTOM_BASE_MODEL) /E2E_AZURE_GPT4O_MODELwith the gpt-5.6-tier and gpt-4o defaults, so pointing the suite at differently named deployments needs no code changeBeyond the happy path, two rows guard specific customer-reported Azure chat regressions.
test_azure_custom_deployment_name_reasoning_effort_nonesendsreasoning_effort='none'to a custom-named deployment (gpt-5.6-sol-e2e, capabilities resolved viamodel_info.base_model) and asserts a real completion with zero reasoning tokens on a prompt that reasons at default effort, so both failure modes of #31243 fail the row: the capability-gate 400 that #28490 fixed at the SDK layer, and a silently dropped param, which globaldrop_params: truewould otherwise mask (the row setsdrop_params: falseon its deployment for that reason).test_azure_config_token_cap_with_client_max_completion_tokenssends a clientmax_completion_tokensto agpt-4odeployment carrying amax_tokensdefault in its litellm_params and asserts the call completes: #34214 fixed #31614 by havingAzureOpenAIConfig.map_openai_paramsskipmax_tokenswhenevermax_completion_tokensis also present, so the strict xfail this row originally shipped with is gone and the row now asserts success. That makes #34214 a hard dependency: this PR must land after it, since the row fails against any proxy without the fix (see the before run above)This revision also rebases the PR onto the e2e harness overhaul that landed on staging since it branched.
tests/e2e/docker-compose.ymlstays deleted (removed upstream in #33837) and the Azure model entries it carried became the/model/newregistrations above; the transport changes collapsed into what staging already ships (#33750'sProxyClientwithchat/chat_stream, andStreamingResponse.stream_eventsfor the SSE payloads), with one addition, astream_doneflag recording thedata: [DONE]terminator so the streaming rows can assert termination.models.pygains theChatStreamChunk/ChatStreamChoice/ChatStreamDeltachunk models,max_completion_tokensonChatBody,completion_tokens_details.reasoning_tokensonUsage,max_tokensanddrop_paramsonLiteLLMParamsBody, andbase_modelonModelInfoBody(exposed throughProxyClient.create_model). The registry gainsllm.chat_completions.azure_openai.basic.stream.works,...thinking.nonstream.works, and...basic.nonstream.token_param_dedup; the nonstream basic row already existed and is now coveredA note on kill power for the #31243 row: the SDK-level gate provably rejects this shape before #28490 (verified against the v1.86.2 source and live on litellm 1.86.7 from PyPI), but the proxy's own startup registration also resolves
base_modelcapabilities, so the v1.86.2 image passes this shape through a config-file proxy even though its own SDK path raises. The registry row is therefore markedfail_before_fix: unprovenat the proxy layer; SDK-layer unit coverage that does reproduce the pre-fix failure is added in #33615. The #31614 row isfail_before_fix: proven: the before run above reproduces the 400 liveQA runbook
Environment prerequisites:
AZURE_API_BASEandAZURE_API_KEYintests/e2e/.envpointing at an Azure OpenAI resource that hasgpt-5.6-sol,gpt-5.6-terra,gpt-5.6-luna,gpt-5.6-sol-e2e(a secondgpt-5.6-soldeployment under a custom name), andgpt-4odeployments (the shared e2e suite resource in the team dev subscription has all five); override theE2E_AZURE_*_MODELvars to use other deployment names. Start a proxy pertests/e2e/CONTRIBUTING.md(litellm --config <your-e2e-config>.yml --port 4000) with the example models prewired andstore_model_in_db: true, since every Azure row registers its deployment through/model/new. The dedup row needs a proxy that includes #34214. Prompts should include a random marker to dodge the shared response cachetests/e2e/llm_translation/test_chat_completions_regression_e2e.py::TestAzureOpenAIChat::test_azure_chat_returns_real_completion[gpt-5.6-sol] (and the terra/luna rows) - a non-streaming /chat/completions call to an Azure OpenAI deployment registered via /model/new returns a 200 whose body carries a model name and a non-empty assistant message
{"model_name":"qa-azure-sol","litellm_params":{"model":"azure/gpt-5.6-sol","api_base":"<AZURE_API_BASE>","api_key":"<AZURE_API_KEY>"},"model_info":{}}{"model":"qa-azure-sol","messages":[{"role":"user","content":"reply with one word <marker>"}],"max_tokens":512}model, a non-emptychoicesarray, andchoices[0].message.contentcontaining real textazure/gpt-5.6-terraandazure/gpt-5.6-luna, then POST /model/delete for each registered idtests/e2e/llm_translation/test_chat_completions_regression_e2e.py::TestAzureOpenAIChat::test_azure_stream_returns_real_completion[gpt-5.6-sol] (and the terra/luna rows) - a streaming call to the same deployments returns a well-formed SSE stream, not just a 200 with opaque bytes
"stream": trueand expect a 200 withContent-Type: text/event-streamdata:line except the last to parse as JSON with"object": "chat.completion.chunk", at least one carrying"model", and no error event anywhere in the streamdata: [DONE]and the concatenatedchoices[0].delta.contentfragments to form non-empty texttests/e2e/llm_translation/test_chat_completions_regression_e2e.py::TestAzureOpenAIChat::test_azure_custom_deployment_name_reasoning_effort_none - reasoning_effort='none' on a custom-named deployment resolves capabilities via base_model, reaches Azure, and actually disables reasoning (GH [Bug]: reasoning_effort='none' capability gate ignores base_model, returning 400 for Azure custom deployment names (Azure GPT-5) #31243)
{"model_name":"qa-azure-custom","litellm_params":{"model":"azure/gpt-5.6-sol-e2e","api_base":"<AZURE_API_BASE>","api_key":"<AZURE_API_KEY>","drop_params":false},"model_info":{"base_model":"azure/gpt-5.6-sol"}}{"model":"qa-azure-custom","messages":[{"role":"user","content":"A farmer has 17 sheep, all but 9 run away, then he buys twice as many as remain minus 3. How many sheep? Reply with just the number. <marker>"}],"max_completion_tokens":2000,"reasoning_effort":"none"}choices[0].message.contentandusage.completion_tokens_details.reasoning_tokensequal to 0reasoning_effortand expectreasoning_tokens> 0, proving the prompt reasons at default effort and the zero above means the param was honored, not droppedtests/e2e/llm_translation/test_chat_completions_regression_e2e.py::TestAzureOpenAIChat::test_azure_config_token_cap_with_client_max_completion_tokens - a client max_completion_tokens on a deployment carrying a max_tokens default completes because only max_completion_tokens is forwarded to Azure (GH [Bug]: Azure GPT-4.1 rejects both max_tokens and max_completion_tokens simultaneously #31614, fixed by fix(azure): send only max_completion_tokens when both token params are set #34214)
{"model_name":"qa-azure-4o","litellm_params":{"model":"azure/gpt-4o","api_base":"<AZURE_API_BASE>","api_key":"<AZURE_API_KEY>","max_tokens":512},"model_info":{}}{"model":"qa-azure-4o","messages":[{"role":"user","content":"reply with one word <marker>"}],"max_completion_tokens":256}choices[0].message.content; on a proxy without it, expect a 400 whose message contains "Setting 'max_tokens' and 'max_completion_tokens' at the same time is not supported", which is the regression this row guardsFinal Attestation