fix(protocol): emit one complete response.completed per responses stream - #206
Merged
Merged
Conversation
openai_sse_to_openai_responses emitted `event: response.completed` with
`data: {}` on the upstream `[DONE]` line: a terminal event with neither
`type` nor `response`. Because the finish_reason branch also emits the
terminal, a normal upstream produced the terminal twice (counted by the
SSE `event:` name) — once well-formed, once payload-less; an upstream
that only sent `[DONE]` produced the payload-less one alone. A Responses
client dispatching on `event.response` therefore had no response object
at all, while the whole-body translator always returns a complete one.
The terminal object is now built once from the accumulated stream state
(text, tool calls, usage) and emitted at most once per stream, whichever
branch reaches the end first, and its shape is shared with the whole-body
path instead of being a second inline copy:
- new `protocol::openai_chat_message_to_responses_output` and
`protocol::openai_usage_to_responses_usage` are the single source for
the Responses `output` / `usage` shape; the whole-body translator
`openai_chat_to_openai_responses_resp` was refactored onto them with
unchanged behaviour (same message item id, same zeroed-usage default);
- `ResponsesStreamState` accumulates the upstream `id`/`model`, the text
deltas, the tool-call arguments (concatenated per call id) and the
upstream usage chunk; `terminal_response()` derives the object from it,
so the message item id stays `msg_…` and matches the `item_id` the
incremental events already sent;
- `completed_event()` guarantees a single emission; the payload-less
`data: {}` is gone.
Recorded divergences that stay untouched: the streamed `resp_`/`msg_`
id prefixes (a client only ever sees one path) and `status` — the
whole-body object has none, and the status/truncation semantics belong to
the host-adjudication family, so a stream that ends without `[DONE]`
still emits no terminal event.
Tests: 4 new (196 total, was 192). The terminal event is asserted once
per stream, counted by the SSE `event:` name; its `response` object is
checked field by field against what the stream carried (accumulated text,
concatenated tool-call arguments, mapped usage); a `[DONE]`-only stream
with and without a usage chunk is covered; and a cross-path parity test
compares the streamed terminal with the whole-body translator for four
upstream shapes, asserting the two recorded divergences as exact
relations rather than ignoring them. The comparator has its own positive
control (it must reject seven injected shape deviations).
A/B red-before / green-after, 3/3: reinstating the payload-less `[DONE]`
branch, dropping the once-only guard, and reverting the terminal object to
the empty shell each redden exactly the stream-driven tests (the duplicate
guard cannot redden the no-finish_reason case — that stream never takes
the finish_reason path), and all are green again after the revert.
No billing impact: streamed usage is recorded by the same slot as before.
Merged
12 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
openai_sse_to_openai_responsesemittedevent: response.completedwithdata: {}on the upstream[DONE]line — a terminal event with neithertypenorresponse. On a normal upstream the terminal therefore arrived twice (counted by the SSEevent:name): once well-formed, once payload-less; on an upstream that only sent[DONE], the payload-less one arrived alone. A Responses client that reads the documentedevent.responsegot nothing, while the whole-body translator for the same protocol pair always returns a complete object. The streamed terminalresponsewas also an empty shell (output: [], nousage).openai_sse_to_openai_responsesconverts every Responses-protocol client on a plan without aresponsesendpoint (11 of the 12 plans inconfig/config.toml), and serves theanthropic → responseschain too, so this is the path a Responses client actually gets.Related Issue
None — found by an in-repo sweep of the streamed vs whole-body shape fields of the five protocol pairs. (No issue exists to link;
needs:issuestays unpassable by design.)Changes
upstream_id,model, text deltas, tool calls with their concatenated arguments, upstream usage chunk) and emitted at most once per stream — whichever branch reaches the end first (finish_reasonor[DONE]). The payload-lessdata: {}emission is gone.protocol::openai_chat_message_to_responses_output(message, message_item_id)andprotocol::openai_usage_to_responses_usage(usage);openai_chat_to_openai_responses_respwas refactored onto them with unchanged behaviour (same message item id, same zeroed-usage default).msg_…in the terminal object, matching theitem_idthe incremental events already sent;usagefollows the whole-body rule (mapped from the upstream chunk, zeroed when absent, never omitted).resp_/msg_id prefixes (a client only ever sees one path) andstatus— the whole-body object has none, and status/truncation semantics are a separate decision, so a stream that ends without[DONE]still emits no terminal event.ui/, no i18n keys, no release.Tests
cargo testall pass — 196 passed (was 192),cargo fmt --checkclean,clippy --all-targets -- -D warningsclean.event:name, not the payloadtype— an empty payload carries notype); itsresponseobject is checked field by field against what the stream carried (accumulated text, concatenated tool-call arguments, mapped usage); a[DONE]-only stream with and without a usage chunk is covered; and a cross-path parity test compares the streamed terminal with the whole-body translator over four upstream shapes, asserting the two recorded divergences as exact relations (resp_{whole.id},msg_{whole.id}) rather than ignoring the fields.object/model/usage/status, wrongoutputlength, tampered content, broken id-prefix relation) and must accept the honest pair, so "consistent" is not a permanent exemption.[DONE]branch → the three stream-driven tests red; dropping the once-only guard → the duplicate case and the parity case red (the no-finish_reasoncase cannot go red — that stream never takes thefinish_reasonpath); reverting the terminal object to the empty shell → the three stream-driven tests red. All green again after each revert.Checklist
fix/)fix(protocol): …)protocol.rspart is a behaviour-preserving extraction of the shape the two paths now share