test(e2e): cover /v1/responses openai basic nonstream and stream - #33830
Conversation
Co-Authored-By: Ishaan Jaffer <155045088+ishaan-berri@users.noreply.github.com>
🤖 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 SummaryThis PR adds e2e test coverage for the
Confidence Score: 4/5All changes are confined to the The termination assertion in the streaming test checks that the last recognized (delta or completed) event is tests/e2e/llm_translation/test_responses_e2e.py — specifically the
|
| Filename | Overview |
|---|---|
| tests/e2e/e2e_http.py | Adds stream_events: list[str] to StreamingResponse and populates it with raw data: payloads during SSE streaming; logic is correct and non-breaking for non-streaming callers. |
| tests/e2e/llm_translation/endpoints_client.py | Adds stream parameter to ResponsesRequest and responses(), adds typed streaming event models with Literal type constraints; no issues found. |
| tests/e2e/llm_translation/test_responses_e2e.py | Adds streaming e2e test with SSE event parsing; termination assertion checks last recognized event rather than the absolute last SSE line in the stream. |
Reviews (1): Last reviewed commit: "test(e2e): cover /v1/responses openai ba..." | Re-trigger Greptile
| assert completed_events and isinstance(parsed_events[-1], ResponsesCompletedEvent), ( | ||
| "responses stream did not terminate with response.completed" | ||
| ) |
There was a problem hiding this comment.
Termination check operates on filtered events only
parsed_events contains only response.output_text.delta and response.completed events — all other SSE event types (e.g. response.output_text.done, response.output_item.done, response.done) are silently dropped to None by _parse_stream_event. So isinstance(parsed_events[-1], ResponsesCompletedEvent) confirms that no recognized event arrived after response.completed, not that response.completed was the absolute final line in the stream. If OpenAI or LiteLLM ever appends a trailing event of an unrecognized type, the assertion continues to pass even though the stream did not literally end on response.completed. Consider also capturing the raw final data: line and asserting its content, or extending _parse_stream_event to return a sentinel for any non-None parsed event.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Co-Authored-By: Ishaan Jaffer <155045088+ishaan-berri@users.noreply.github.com>
Co-Authored-By: Ishaan Jaffer <155045088+ishaan-berri@users.noreply.github.com>
Co-Authored-By: Ishaan Jaffer <155045088+ishaan-berri@users.noreply.github.com>
Relevant issues
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
Video proof of the live run (docker compose stack, liveliness check, pytest run of both tests, and a live curl against /v1/responses showing a
response.output_text.deltaevent and the finalresponse.completed) is embedded under the QA runbook below. Captured at commit fb20177; the follow-up commits only tightened the termination assertion and moved the event-type model, and the same two tests were re-run green against the live stack at 8a62280Type
✅ Test
Changes
PR 1 of a 4-PR series adding e2e coverage for the /v1/responses endpoint. It closes exactly two registry cells that already exist in
tests/e2e/coverage_registry/llm_conversational.yaml:llm.responses.openai.basic.nonstream.worksandllm.responses.openai.basic.stream.worksThe existing smoke test
TestResponses::test_responses_returns_completionnow carries the nonstream marker; its assertion is unchanged (parses the body asResponsesResultand asserts non-empty output text). A newtest_responses_streaming_returns_completionregisters anopenai/gpt-4o-minideployment at runtime, calls /v1/responses withstream: truethrough the shared transport, and asserts the stream carried at least oneresponse.output_text.deltaevent with non-empty text and that the final raw SSE event (excluding the[DONE]sentinel) has typeresponse.completed; a stream with no text deltas failsTo support this,
e2e_http.pynow preserves the raw SSEdata:payloads onStreamingResponse.stream_events, andendpoints_client.pygainsstream: bool = Falseon the responses call plus typedResponsesOutputTextDeltaEvent/ResponsesStreamEventTypemodels. No raw dicts orAnycoverage_registry.collector --strictpasses with both markers resolving;make lint-e2e-basedpyrightreports zero errorsQA runbook
cd tests/e2eand create.envwithOPENAI_API_KEY(plus the other provider keys the compose file references)docker compose up -dand wait forcurl -fs http://localhost:4000/health/livelinessto return"I'm alive!"curl -sN http://localhost:4000/v1/responses -H "Authorization: Bearer sk-1234" -H "Content-Type: application/json" -d '{"model": "gpt-5.5", "input": "reply with one word", "stream": true}'and watch aresponse.output_text.deltaevent followed byresponse.completeduv run pytest llm_translation/test_responses_e2e.py -vand expect both tests to passVideo of the full live run: https://app.devin.ai/attachments/70280648-11bb-4993-8c25-09124b02aadc/rec-5424436e-bc2b-4bc2-9c8b-a1160cc4d08b-edited.mp4
openai/gpt-4o-minioutput[].content[].text"stream": trueand consume the SSE streamresponse.output_text.deltaevent with non-emptydeltaand a finalresponse.completedeventFinal Attestation
Link to Devin session: https://app.devin.ai/sessions/b4e79c562b4a4187ba6cefec54f92d5a
Requested by: @ishaan-berri