Skip to content

Preserve OpenAI message phase and close out partial streams cleanly across providers - #271

Merged
myzie merged 7 commits into
mainfrom
fix/openai-responses-preserve-phase
Aug 22, 2026
Merged

Preserve OpenAI message phase and close out partial streams cleanly across providers#271
myzie merged 7 commits into
mainfrom
fix/openai-responses-preserve-phase

Conversation

@myzie

@myzie myzie commented Aug 22, 2026

Copy link
Copy Markdown
Collaborator

This started as a fix for one reported bug — OpenAI's phase label on assistant messages was being dropped — and grew to cover a few closely related streaming problems the fix exposed. Everything here is about making sure what a model sends us survives the round trip intact: into memory, into storage, and back out on the next request or to whoever is watching the stream.

1. OpenAI's message "phase" is no longer lost

When a model on OpenAI's Responses API replies, it can label each message as commentary ("checking that now…") or final_answer. OpenAI's SDK docs say to send those labels back on follow-up requests and warn that dropping them can make the model perform worse. We were dropping them.

I reproduced that first — the report's non-streaming test failed on main exactly as described. The fix keeps the label on each text block as a small piece of provider metadata (openai.phase), using the same channel that already carries OpenAI reasoning state and Google thought signatures. It's read when we decode a response, written back when we replay history, and tracked while streaming.

Two design notes:

  • The label lives on each text block rather than as a new Phase field on the shared llm.Message type. That's enough because we already send one output message per text block when replaying, so a commentary message, a tool call, and a final answer go back out as three separate messages, each with its own label. It also keeps an OpenAI-specific idea out of the provider-neutral core.
  • Refusal content and output-message IDs/status are left alone on purpose. Refusals have no metadata field and we don't replay them anyway, so adding one would be dead weight. Echoing provider-issued message IDs back is a behavior change with its own risk and isn't needed for the label to survive — worth a separate look.

2. Streaming delivers the phase while the block is still open

OpenAI sometimes only tells us the phase at the end of an output item, after the text has finished streaming. Previously we had already closed the text block by then, so the label arrived too late for anyone listening to the stream. Now a text block stays open until its item is done, the phase is announced inside the block's lifetime, and we don't announce it twice if it was already known up front.

3. Streams that end early now close out cleanly — every provider

Pulling on that thread showed that a stream which ends abruptly — OpenAI sending response.incomplete, or a connection simply stopping — could leave blocks "open" with no closing event, or close them in the wrong order. Anyone consuming the stream then sees an unbalanced sequence. I audited every provider:

  • OpenAI Responses (also used by Grok): any text, reasoning, or tool-call block still open when the response ends is now closed, and closed before the final message_delta, which is the order the other providers already use.
  • OpenAI-compatible chat completions (used by Mistral and OpenRouter): a stream that hit [DONE] or end-of-input without a finish_reason used to emit nothing at all to finish the message — no closing events, no message_delta, no message_stop. It now closes open text and tool-call blocks and ends the message properly, exactly once.
  • Google / Gemini: already correct. It finalizes when the stream ends rather than waiting for a finish reason, and tool calls arrive whole, so nothing can dangle. I added a test so that stays true.
  • Anthropic (also used by Ollama): passes the API's events straight through, so there's nothing for us to close.

4. Compaction keeps provider metadata

When compaction shrinks an oversized text or tool_use block, it used to rebuild the block without its metadata — silently throwing away things like the phase label above or a Google thought signature. It now keeps them.

Testing

  • providers/openai/phase_test.go — 11 cases covering decode → persist → encode for both phases, streaming, a mixed commentary/tool-call/final-answer response, Copy, unlabeled messages, and making sure nothing leaks into other providers. Confirmed these fail without the fix rather than assuming.
  • TestStreamIteratorClosesDanglingBlocks in the OpenAI, chat-completions, and Google stream iterator tests — one block start, one block stop, the stop before message_delta, message_stop last, and the partial content still arriving as a well-formed message. The OpenAI and chat-completions versions fail without their fixes; the Google one passes unchanged.
  • A compaction test for the metadata fix and a provider-neutral persistence test in llm/message_test.go.
  • provider_integration_test.go gains a live multi-turn check against GPT-5.6 (-tags integration, needs OPENAI_API_KEY) that persists a real turn, reloads it, and confirms OpenAI accepts the replayed history. I have not run this one against the live API — it needs a key and a billed call — so please run it before relying on it.

All test suites are green across every module (root, providers/openai, providers/google, providers/grok), plus go vet and gofmt. One heads-up: TestToolUse in openaicompletions is a live call against gpt-4o that occasionally answers with text instead of a tool call; it flaked once during my runs and passed 3/3 on retry. It doesn't touch any code in this PR.

🤖 Generated with Claude Code

https://claude.ai/code/session_01F1eYPGBnNesvpcJiSAouxm

Summary by CodeRabbit

  • New Features

    • Preserved OpenAI assistant message phases, including commentary and final_answer, across responses, streaming, persistence, and follow-up requests.
    • Maintained phase information for individual messages while leaving user messages and unlabeled assistant messages unchanged.
  • Bug Fixes

    • Ensured completed streaming phases take precedence over earlier partial phase updates.
  • Tests

    • Added coverage for phase replay, serialization, copying, streaming behavior, and provider metadata preservation.

OpenAI labels each assistant output message with a phase (`commentary` for
intermediate updates, `final_answer` for the answer) and asks that manually
replayed history resend it unchanged; dropping it degrades gpt-5.3-codex and
later. Dive discarded it in both the non-streaming and streaming decoders, so
persisted history replayed through encodeMessages produced a structurally valid
but lossy follow-up request, with no error identifying the missing field.
Consumers relying on `previous_response_id` were unaffected; durable runtimes
that persist and resend provider history were not.

Carry the phase as `openai.phase` provider metadata on each decoded text block,
reusing the existing ProviderMetadata replay channel rather than adding an
OpenAI-shaped field to the model-neutral llm.Message. The phase attaches per
text block rather than per turn because the encoder already emits one output
message per block, so a response mixing commentary, a tool call, and a final
answer replays each message under its own phase. Anthropic and Google strip or
ignore metadata they do not own, so the state stays provider-local.

Streaming treats `response.output_item.done` as authoritative, since OpenAI may
omit the phase when the item is added and only label it on completion; the
phase is also surfaced on content-block start when known early, so live
consumers can distinguish commentary from the final answer as it streams.

Dive never infers a phase: an unlabeled message stays unphased, and user
messages never carry one.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Aug 22, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 7e457577-e2dc-4040-ab6a-13d89c7c22f7

📥 Commits

Reviewing files that changed from the base of the PR and between 5d4fd0d and f2e684c.

📒 Files selected for processing (8)
  • CHANGELOG.md
  • llm/message_test.go
  • providers/openai/decode.go
  • providers/openai/encode.go
  • providers/openai/phase_test.go
  • providers/openai/provider_integration_test.go
  • providers/openai/stream_iterator.go
  • providers/openai/types.go

Included review availability: 0 reviews are currently available. Your included PR review attempts over the past 7 days set your current allowance at 1 review per hour.


📝 Walkthrough

Walkthrough

OpenAI assistant output phases are stored as openai.phase metadata on text content. Non-streaming and streaming paths preserve these phases through serialization, copying, and replay. Unphased messages and user messages remain without phase fields.

Changes

OpenAI phase preservation

Layer / File(s) Summary
Phase metadata contract and replay
providers/openai/types.go, providers/openai/decode.go, providers/openai/encode.go, llm/message_test.go
The provider stores non-empty phases in text metadata and restores them when encoding assistant messages. Message metadata survives JSON round trips and deep copies.
Streaming phase state
providers/openai/stream_iterator.go
Streaming output items track phases from added events. Completion events update phases authoritatively and emit metadata deltas for streamed text.
Phase preservation validation
providers/openai/phase_test.go, providers/openai/provider_integration_test.go, CHANGELOG.md
Tests cover persistence, copying, mixed phases, user-message exclusion, streaming precedence, and stateless replay. The changelog records the fix.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: ⚪ Minimal · up to f2e68

The PR preserves OpenAI assistant message phases across decoding, persistence, streaming, and replay without changing other providers’ behavior; no actionable merge-blocking risk remains beyond normal checks and review.

Sequence Diagram(s)

sequenceDiagram
  participant OpenAIResponses
  participant OpenAIDecoder
  participant Message
  participant OpenAIEncoder
  OpenAIResponses->>OpenAIDecoder: assistant output with phase
  OpenAIDecoder->>Message: text block with openai.phase metadata
  Message->>OpenAIEncoder: serialized assistant history
  OpenAIEncoder->>OpenAIResponses: replayed message with phase
Loading
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 47.83% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 23 functions across 7 files. (1 skipped: 1 unsupported.) Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the primary change: preserving OpenAI message phases, and it also references the streaming behavior covered by the changes.
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/openai-responses-preserve-phase

Comment @coderabbitai help to get the list of available commands.

The test previously asked a plain question, which only ever produces a
final_answer message, so the commentary half of the acceptance criteria went
unexercised. Give the model a tool and a prompt that invites a preamble: live
gpt-5.6-sol, -terra, and -luna all return a commentary message alongside their
tool calls.

The turn is now persisted, reloaded, replayed with tool results on a stateless
follow-up, and asserted to carry both commentary and final_answer. Logs report
the phases each turn actually returned so a qualification run shows its work.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@myzie

myzie commented Aug 22, 2026

Copy link
Copy Markdown
Collaborator Author

Live qualification run — done. The last acceptance criterion is now verified rather than just written.

I first ran the test as originally committed and it passed, but the log showed it had only exercised final_answer: a plain question never produces a commentary message, so half the criterion was silently going untested. A passing test that doesn't test the thing isn't qualification.

Reworked it to give the model a tool and a prompt that invites a preamble, which is where commentary actually appears:

turn 1 phases from gpt-5.6-sol: [commentary]
turn 2 phases from gpt-5.6-sol: [final_answer]
--- PASS: TestIntegration_PhaseReplayedAcrossStatelessTurns (6.71s)

The flow now qualifies the real path end to end: the model returns a commentary preamble alongside its tool calls → that turn is persisted and reloaded → replayed with tool results and no previous_response_id → OpenAI accepts the history → the concluding turn arrives as final_answer. Both phases are asserted, and each turn's phases are logged so a qualification run shows its work.

I also confirmed all three variants the downstream unblock covers emit commentary and decode it correctly:

gpt-5.6-sol   -> phases=[commentary]
gpt-5.6-terra -> phases=[commentary]
gpt-5.6-luna  -> phases=[commentary]

Every acceptance criterion is now verified, live included. Full suite green across all modules.

myzie and others added 5 commits August 21, 2026 23:40
Code review of the phase work found the metadata delta landing after the
block it belongs to was already closed: OpenAI sends output_text.done
before output_item.done, so the stop was emitted first and the late phase
label arrived against a finished block. It happened to survive only because
llm.ResponseAccumulator never closes blocks — a consumer that rebuilds
messages from the event stream and honors content_block_stop, exactly the
manual-replay runtime this branch targets, would finalize the text and drop
the phase again.

Close a message item's text blocks on its own done event instead, after any
metadata that event contributed, matching how reasoning and function_call
blocks already close. Emit the phase delta only when the block's start event
could not announce it, so a message labeled up front no longer re-sends its
phase. A response that ends without output_item.done closes its open text
blocks before message_stop, so the lifecycle stays balanced.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01F1eYPGBnNesvpcJiSAouxm
reduceBlock rebuilt a shrunken text block with its CacheControl and
Citations but not its Metadata, so an oversized block that compaction
truncated lost the replay state the provider needs on the next request —
a Google function-call thought signature today, and the OpenAI message
phase this branch adds. tool_use blocks lost theirs the same way when
their input was culled.

Carry the metadata across on both, cloned so the reduced block still owns
its own copy and the original stays untouched.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01F1eYPGBnNesvpcJiSAouxm
A response that ends without an item's done event (response.incomplete, or
response.completed arriving early) previously closed only its text blocks,
and did so after message_delta. Generalize the close to reasoning and
function-call blocks and emit the stops before message_delta so the event
order matches the Google and Anthropic iterators. Also repair the mangled
doc comment on the close helper.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01F1eYPGBnNesvpcJiSAouxm
The openaicompletions iterator (used by Mistral and OpenRouter) emitted no
content_block_stop, message_delta, or message_stop when a stream reached
[DONE] or EOF without a finish_reason chunk, leaving the accumulated
message open. Close open text and tool-call blocks and synthesize the
terminal events exactly once at either end marker.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01F1eYPGBnNesvpcJiSAouxm
Gemini streams terminate on iterator exhaustion rather than on a finish
reason, and queueFinalEvents already closes the open thinking or text
block before message_delta, so no behavior change is needed. Lock that
in with the same dangling-block test the OpenAI iterators now carry.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01F1eYPGBnNesvpcJiSAouxm
@myzie myzie changed the title Preserve OpenAI Responses output-message phase through decode and replay Preserve OpenAI message phase and close out partial streams cleanly across providers Aug 22, 2026
@myzie
myzie merged commit 24224d7 into main Aug 22, 2026
2 checks passed
@myzie
myzie deleted the fix/openai-responses-preserve-phase branch August 22, 2026 04:15
@myzie myzie mentioned this pull request Aug 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant