fix(openaimodel): preserve refusals and completed stream content

[ktsoator]

Link to Issue or Description of Change

Closes #1466. Closes #1476.

The OpenAI stream translator ignores response.refusal.delta. A refusal-only
response loses incremental delivery, and a refusal following reasoning can be
omitted entirely because the non-empty aggregator suppresses terminal-response
fallback. The resulting thought-only turn may cause up to 10 model calls without
surfacing an answer.

Translate non-empty refusal deltas into ordinary, non-thought text, matching the
blocking representation. Explicitly classify response.refusal.done as an
informational event; this documents its existing behavior and avoids emitting
the complete refusal again after its deltas.

When response.completed contains non-thought text or a function call, use
its content for the final response only if it retains the visible text and
function calls already collected from the stream. Otherwise, keep the aggregate.
When replacement is allowed, final reasoning follows the completed response;
thoughts already delivered as partials are not added back.

This does not merge content from both sources. For example, if visible text
streams first and the completed response contains only a tool call, the text
is retained and the call is not recovered. Calls whose streamed ID fell back
to item_id can also prevent recovery when the completed response uses a
different call_id. Both remain limitations of the current approach.

Preserve aggregated content when the completed payload is empty or cannot be
converted, and when the stream ends without a terminal event or its first
terminal event is response.incomplete. With no aggregated response, the
existing terminal fallback and conversion errors are unchanged. Final metadata
continues through the existing finalization path.

The partial-delta case was reproduced with synthetic SSE events. This validates
the adapter's behavior for that input; it does not establish how often a live
provider emits such a stream.

Testing Plan

• Direct tests cover reasoning filtering, usable output, text containment,
  argument and ID differences, and one-to-one call matching. Each of the five
  mutations identified in the review makes a test fail.
• Full-stream tests cover reasoning that contains the visible answer, streamed
  thoughts omitted by the completed response, and recovery of completed text
  after a function call with null or {} arguments. The null case failed
  before argument normalization and passes afterward.
• Translator tests cover non-empty and empty refusal deltas.
• Full-stream tests compare final refusal content directly with blocking output,
  including partial deltas with and without refusal.done, and partial refusal
  after reasoning. They also verify that incremental text contains only the
  deltas actually received.
• Regression tests cover text completion, mixed output order, empty and
  unsupported terminal content, incomplete responses, and first-terminal-event
  precedence.
• TestModel_GenerateStream_TerminalOnlyMessageContent preserves the original
  model/openaimodel: terminal-only message content is dropped after streamed deltas #1476 reproduction: reasoning is streamed, while the visible answer appears
  only in response.completed. It fails against the pre-fix implementation and
  passes with the completed-content fix, comparing final content with blocking.
• Tests cover reasoning-only and shorter completed responses, completed
  responses that omit a streamed function call, and function calls that appear
  only in response.completed.
• Updated the existing function-call fixture: response.output_item.added
  previously omitted its type, and response.completed contained a message
  instead of the function call from the stream. Both events now describe the
  same call.

Before the terminal-content fix, all three new partial-refusal cases failed:

expected final text: "I cannot help with that."
actual final text:   "I cannot "

They pass after the fix. Local validation on Go 1.26.6:

go build -mod=readonly work
passed

go test -race -mod=readonly -count=1 -shuffle=on work
all packages passed, including plugin/agentanalytics

golangci-lint run
0 issues in both modules

go mod tidy -diff
no differences in either module

Tests use a local HTTP server and synthetic SSE; no live model calls are made.

Checklist

• Read CONTRIBUTING.md and performed self-review.
• Added regression tests and verified they fail before the fix.
• Build, race tests, lint, and tidy checks pass locally.
• No public API or dependency changes.
• No dependent downstream changes.

[karolpiotrowicz]

The diagnosis holds up and the fix is the right shape. I checked out the branch at 2cdaa26, ran the full suite sandboxed against both this branch and its merge base, and confirmed the bug end to end: your tests fail on 56d628d and pass here. The two subtests fail differently on base, and that difference is worth putting on the record because it is the strongest argument for this change — refusal only still rebuilds correct final content through the terminal-response fallback and merely loses incremental delivery, while refusal after reasoning drops the refusal outright, since the reasoning delta makes aggregator.Close() non-nil and the fallback at openai.go:168-177 never fires.

The user-visible cost on base is larger than the issue describes. A refusal after reasoning leaves a turn whose parts are all Thought: true, so isThoughtOnlyTurn sends the flow round again at base_flow.go:144-152 — 10 model calls in total before it gives up and still surfaces no answer. Mapping the delta to a non-thought part at stream.go:53-59 ends the turn on the first response. Choosing Thought: false is load-bearing rather than incidental, since the aggregator flushes its text buffer exactly when that flag flips, and marking a refusal as a thought would have concatenated it into the preceding reasoning part and hidden it from the user.

Nothing here blocks merging. Build, vet, gofmt, golangci-lint, staticcheck and govulncheck are all clean, apidiff reports no exported API change, coverage moves 86.3% to 86.8%, and the only two failing packages fail identically on the merge base for reasons unrelated to this change. This also merges cleanly with #1373, the other open PR touching the package.

Four things worth your attention, in order of consequence.

Refusal deltas now suppress the terminal-response fallback

Because any delta reaching the aggregator makes Close() non-nil, a stream that carries a refusal delta and content present only on the terminal event now loses that content. Concretely: one response.refusal.delta plus a response.completed whose message also holds an output_text that never streamed yields "Here is I cannot help." on the merge base and "I cannot help." here. I want to be clear that I do not think you should change anything for it — the mechanism predates your change (a single reasoning delta already suppresses the same fallback and already loses batched text on base), and it is not separable from the feature, since streaming refusals incrementally is routing them through the aggregator. The naive repair of merging the terminal response back in would double-emit every ordinary refusal. It is worth a follow-up issue against the all-or-nothing condition at openai.go:169, which affects every delta type equally.

The new end-to-end test does not actually pin parity with the blocking path

TestModel_GenerateStream_Refusal asserts against hand-written wantParts literals rather than running the blocking path, the way TestModel_GenerateStream_MatchesBlocking does through runBlocking. The literals are correct today. The gap is drift: if the refusal mapping at response.go:79-80 ever changed, the two paths would diverge and this test would keep passing. Since matching the blocking representation is the stated goal, comparing against it directly would pin the thing the PR is actually claiming.

The empty-delta guard has no test

stream.go:55-57 is a live branch rather than defensive padding — without it an empty part reaches the aggregator and suppresses the fallback described above. No test in the package sends "delta":"". In fairness the three sibling guards in the same file are equally untested, so this is the house pattern and not something you introduced. One extra table row would cover it.

One correction to the PR description

Adding responseRefusalDone at stream.go:110 does not prevent duplicate content, because the pre-existing default: branch already returned (nil, nil) for that event — the base branch never double-emitted either. I checked what would happen if it did emit: the .done event repeats the full refusal text the deltas already carried, so emitting from it produces "I cannot help with that.I cannot help with that." and fails your own tests. The line is still worth keeping, since it moves the event from "unknown, ignored" to "known, deliberately informational", but it is documentation rather than a behavior change.

One thing I could not settle: a refusal delivered only through response.refusal.done with no deltas at all is still dropped when another delta has already reached the aggregator. That behaves identically on the merge base, so it is not something this PR introduces, and closing it would need per-item tracking of whether any delta arrived rather than a change to the .done branch. Mentioning it only so it is written down somewhere.

[ktsoator]

Thank you for the detailed review and for tracing the different failure modes
on the merge base.

I've addressed the test and documentation follow-ups in 8b685f8:

• TestModel_GenerateStream_Refusal now compares the final streamed content
  directly with the blocking response for both refusal-only and mixed
  reasoning/refusal outputs.
• Added coverage for empty refusal deltas.
• Updated the PR description to clarify that explicitly handling
  response.refusal.done documents its existing informational behavior rather
  than preventing previously duplicated output.
• Documented the impact of a thought-only turn potentially causing up to 10
  model calls.

I also merged the latest main.

I agree that the all-or-nothing terminal-response fallback is a pre-existing,
broader issue. I have kept it out of this PR and plan to track it separately.

All CI checks are now passing. Thank you again for the thorough feedback. This
should be ready for another look.

[karolpiotrowicz]

The three follow-ups from the last round are done, and I checked each rather than taking the description's word for it. TestModel_GenerateStream_Refusal now really does run the blocking path — runBlocking drives it through the same local server the streaming case uses — and cmp.Diff(blocking[0].Content, final.Content) at openai_test.go:426 compares whole *genai.Content values. That is a stricter comparison than TestModel_GenerateStream_MatchesBlocking makes, since that one flattens every part into one string with allText and so cannot see part boundaries or the Thought flag at all. The empty-delta guard is now covered, and the coverage is real: I deleted the three lines if delta.Delta == "" { return nil, nil } from stream.go:55-57 and nothing else, and TestStreamTranslator_EmptyRefusalDelta failed. The description now describes response.refusal.done correctly.

Nothing blocks merging. Build, vet, gofmt, staticcheck, golangci-lint and govulncheck are clean, apidiff reports no exported API change, and coverage of the package moves 86.3% to 87.0%. Three packages fail the full suite, and all three fail identically on the merge base — telemetry because it dials the metadata server, and the two agent/remoteagent packages because they are timing-sensitive and flaked under load. I re-ran those two in isolation twice on each tree and all four runs passed, so neither is attributable here. The branch is behind main but merges into it cleanly with the change intact. Worth noting that #1359 also touches stream.go and stream_test.go, so that is the one pairing to check before landing both.

Two things worth your attention, neither blocking.

An empty refusal delta is never driven through the full stream. The unit test pins the guard, and I confirmed that by mutation. What no test exercises is the guard's actual consequence — that an empty part reaching the aggregator would make Close() non-nil and suppress the terminal-response rebuild at openai.go:168-170. A stream case sending "delta":"" and then putting the answer only on response.completed would pin the thing the guard exists for, rather than the guard's return value. Optional, and worth saying that yours is already the best-covered of the four: stream.go has the same delta.Delta == "" guard on output_text, reasoning_text and reasoning_summary_text, and no test in the package sends an empty delta for any of those three.

One more shape for #1476, if you want it there. Since a refusal delta now reaches the aggregator, a refusal that follows ordinary output_text in the same turn gets concatenated into the preceding part, because the aggregator flushes its text buffer only when the Thought flag flips. Streaming yields one part where the blocking path yields two. This is the same all-or-nothing behavior you already filed, seen from a different angle, and it is not new: I ran the same shape on the merge base with no refusal event anywhere — two plain output_text items from two output items merge into a single part there too. On the refusal shape specifically the change is a clear improvement, since the base loses the refusal text outright and this keeps it. I mention it only because it is the part of the picture #1476's reproduction does not currently show.

Two small notes that are genuinely optional. TestModel_GenerateStream_Refusal has no doc comment while six of the seven other streaming tests in the file open with one naming the invariant they pin. And separately, response.go:79-80 appends a refusal part with no != "" guard while the output_text case just above it has one — that is pre-existing and outside this change, so leave it unless you want to pick it up.

One thing you may not know, which cuts in your favour on scope. adk-python has this same bug, in the same place: its Responses translator at src/google/adk/labs/openai/_openai_responses_llm.py handles refusal in the blocking converter but its stream dispatch has no response.refusal.delta or response.refusal.done branch either. So this lands ahead of Python rather than diverging from it. The one real difference is representation — adk-python prefixes refusal text with 'OpenAI refusal: ' and adk-java with "[[REFUSAL]]: ", while adk-go emits it unmarked. That difference already exists in adk-go's blocking path, so following it here is the right call for this change, but it is probably worth a maintainer deciding whether adk-go wants a marker at all.

[ktsoator]

Thanks for the follow-up. I addressed both optional notes.

The refusal stream table now includes a full-stream case with an empty response.refusal.delta, followed by refusal content available only in response.completed. The case verifies that no incremental refusal text is emitted and that the final content still matches the blocking response.

I also added a comment documenting the invariant covered by TestModel_GenerateStream_Refusal.

I mutation-checked the new case by temporarily removing the empty-delta guard. The test failed with Parts: nil instead of the terminal refusal content, then passed again after restoring the guard.

The branch has also been synchronized with the latest main.

Validation completed successfully:

• go build -mod=readonly work
• go test -race -mod=readonly -count=1 -shuffle=on work
• go vet ./... in both modules
• golangci-lint run in both modules
• go mod tidy -diff in both modules

[karolpiotrowicz]

The empty-refusal follow-up from the last round is done, and it holds up under the check I care about rather than just reading correctly. openai_test.go:395-400 drives "delta":"" through the full stream and compares the caller's final *genai.Content against the blocking path, which is the thing the guard exists for rather than the guard's return value. I checked it three ways: deleting the delta.Delta == "" guard at stream.go:55-57 fails it, making the terminal-response rebuild at openai.go:168-170 never fire fails it, and making that rebuild produce empty content fails it. The doc comment at openai_test.go:363-364 is there too.

One measured regression against main should land here rather than in a follow-up, and nothing else stands in the way. Build, vet, gofmt, go mod tidy -diff, golangci-lint, staticcheck, govulncheck and apidiff are clean, coverage of the package moves 86.6% to 87.2%, and the two packages that fail the full suite fail identically on the merge base, on current main, and on the merge result, so neither is attributable here. I also ran the second module, plugin/agentanalytics, separately, because go test ./... does not cross a nested go.mod — it passes on both sides.

One shape is worse than main, and it is the sharper half of the issue you already filed

I said last round that a refusal delta now suppresses the terminal-response rebuild, and that the naive repair would double-emit. That is still my view. What I did not have then is the refusal-specific version, and it is worth adding to #1476, because #1476's reproduction currently shows content being lost around a refusal rather than the refusal itself being lost.

A single refusal delta that carries only part of the refusal, with the whole refusal on response.completed, streams as "I cannot " with TurnComplete: true and FinishReason: STOP — a truncated refusal presented as a cleanly finished answer. On main (a6114727) the same events produce "I cannot help with that.", matching blocking exactly, because the unhandled event type left the aggregator empty and the rebuild ran.

I want to be clear about what this does and does not mean. The mechanism is the all-or-nothing condition at openai.go:169, which you do not touch, and the same loss already happens on main for an ordinary output_text delta and for a reasoning delta. Your change stops refusal being accidentally exempt from a rule every other delta type already obeys, which is the right direction. I could not establish whether the Responses API ever emits a partial refusal delta alongside a complete terminal payload, and that is what would decide how much it matters. A captured stream showing it would settle it.

Smaller things

The interleaved shape I mentioned last round is not yours to fix, and I can now show that rather than assert it. One message carrying two ordinary output_text content blocks and no refusal anywhere already yields one streamed part against blocking's two, identically on your branch, on the merge base and on current main — the merge happens at stream_aggregator.go:109-115 and predates you. On the refusal shape specifically your branch is better than main, which drops the refusal text outright.

On landing: this merges cleanly into current main and pairs cleanly with #1359, #1392 and #1395 individually, with go build ./model/... and go test ./model/openaimodel/... green in each pair, including the three-way with #1359 and #1392. #1359 is the one that also edits stream.go. Worth knowing that #1392 and #1395 conflict with each other in doc.go, so whoever sequences those two has a merge to resolve — it does not involve this branch.

[ktsoator]

I reproduced the partial-refusal case against the previous PR head and addressed
it here. A non-empty delta containing I cannot followed by the full refusal in
response.completed now retains the full final text, both with and without a
response.refusal.done event, and after reasoning. The incremental responses
still contain only the deltas that were received.

The fix uses usable content from the first response.completed for the final
response instead of appending it to the aggregate. Empty or unconvertible
completed payloads preserve the aggregate, as do response.incomplete and
streams without a terminal event. Only final Content is replaced; final
metadata keeps its existing path. This also covers the completed-response case
in #1476, which remains open for the broader reconciliation discussion.

The three new partial-refusal tests failed before the fix and pass afterward.
Additional coverage checks terminal precedence, empty and unsupported content,
mixed output order, and function calls. I corrected the existing tool-call
fixture so its terminal payload contains the same tool call, and independently
asserted that the tool call is still emitted exactly once incrementally.

Full workspace build and race tests passed, including plugin/agentanalytics;
lint reports 0 issues in both modules, and both tidy checks are clean. The
partial-delta evidence is from synthetic SSE, so I am not claiming a measured
frequency on live OpenAI streams. This is ready for another review.

[ktsoator]

Added the original #1476 reproduction as a dedicated regression test in 1c6293a. It fails against the pre-fix implementation and passes with the completed-content fix.

All local checks and CI pass. This PR now includes Closes #1476 and is ready for another review.

[karolpiotrowicz]

The terminal-response regression from the last round is fixed. I re-ran the three shapes that gated it — a refusal streaming with the reasoning item carried only by response.completed, one partial refusal delta with the whole refusal on the terminal event, and one refusal delta with batched output_text only on the terminal event — and all three now match the blocking path exactly. On these your branch is better than main rather than merely level with it: main reaches the same final content by discarding the refusal stream altogether, so the caller receives nothing at all mid-turn, where yours delivers "I cannot help with that." as it arrives and still agrees with blocking at the end. TestModel_GenerateStream_TerminalOnlyMessageContent earns its place too — with the new branch deleted it goes red on the missing The final answer., which is exactly what #1476 was about.

What I want to raise is the shape of the fix rather than the bug it closes. Three things need changing before this merges: the usability guard must stop counting a reasoning part as recoverable content, the branch needs a decision on whether a snapshot narrower than the stream should be allowed to override it at all, and the || part.FunctionCall != nil arm needs a test that fails without it. The fourth item, the rewritten fixture, needs a line in the PR description whichever way the second one goes.

The fallback replaces the whole aggregate rather than filling the gap

openai.go:186-199 assigns final.Content = content outright. So for every stream that ends in response.completed, the final response becomes whatever the terminal snapshot converts to, and anything the stream produced that the snapshot does not repeat is gone. That is the correct answer when the snapshot is a superset of the deltas, which is the case the change targets. It is the wrong answer when the snapshot is narrower, and the guard above it does not distinguish the two.

A reasoning-only snapshot destroys the answer, and the turn then loops

This is the one I would fix first. The guard at openai.go:193 tests part.Text != "" without consulting part.Thought, and convertOutputItems puts reasoning text in Part.Text with Thought: true. A snapshot whose only surviving part is reasoning therefore passes the guard and replaces the answer the caller already saw.

It does not take a snapshot that contradicts its own stream to get there. convertOutputItems already drops an output_text whose text is "", so a snapshot that simply declines to repeat streamed text converts to reasoning only. That snapshot shape is one you already treat as real — it is the empty text row at openai_test.go:550, which correctly keeps the aggregate. Add one reasoning item to that same body and the outcome flips:

// head:  final Content.Parts == [THOUGHT("Weighing options")]        -- "hello" is gone
// main:  final Content.Parts == [THOUGHT("Weighing options"), TEXT("hello")]
const body = `{"id":"resp_1","model":"stream-model","status":"completed","output":[` +
    `{"id":"rs_1","type":"reasoning","summary":[{"type":"summary_text","text":"Weighing options"}],"content":[]},` +
    `{"id":"msg_1","type":"message","role":"assistant","content":[{"type":"output_text","text":""}]}]}`
got, _ := runStream(t, evCreated,
    `{"type":"response.reasoning_summary_text.delta","item_id":"rs_1","delta":"Weighing options"}`,
    evDelta1, evDelta2,
    `{"type":"response.completed","response":`+body+`}`)
// got[len(got)-1].Content holds one part, and it is the thought

The cost is not only the lost answer. A final whose parts are all Thought satisfies isThoughtOnlyTurn, so Flow.Run declines to stop and calls the model again, up to ten times, with a growing history. The user gets no answer and pays for ten more calls.

(part.Text != "" && !part.Thought) || part.FunctionCall != nil fixes it. I applied that one change and ran the package: the suite stays green with no failing case, and the shape above goes back to [THOUGHT("Weighing options"), TEXT("hello")], which is what main returns. It costs nothing you are trying to recover, because every snapshot in your new tests carries a non-thought part.

A tool call that streamed can be dropped from the final response

The clearest way to see this is your own test. TestModel_GenerateStream_FunctionCall at the merge base (openai_test.go:412-417) drove runStream(t, evCreated, evItemAdded, evArgsDone, evCompleted), where the shared evCompleted body is a message and no function call. Run that body unchanged against this branch and it fails — calls comes back nil and the final text is hello. Against main (a6114727) it passes.

That matters more than a content difference, because the final response is the only one that reaches tool dispatch: base_flow.go:678 does if resp.Partial { continue } immediately before handleFunctionCalls. A call the stream emitted and the snapshot omits is a tool that silently never runs.

The arm of the guard that governs the opposite case is also untested. Delete || part.FunctionCall != nil from openai.go:193 and the whole package still passes, yet it is not dead code: with a reasoning delta streaming and the tool call arriving only on response.completed, that arm is the only reason the final response carries the call at all. TestModel_GenerateStream_FunctionCall cannot see it, because there the aggregate already holds the identical call, so keeping the aggregate and adopting the snapshot produce the same result. A case where the aggregate has no call and the snapshot does would pin it.

I can argue both sides of whether that snapshot is realistic, and I want to be straight that I could not settle it: nothing offline tells me whether a real Responses endpoint, or one of the OpenAI-compatible gateways ClientConfig.BaseURL exists for, ever sends a response.completed that omits an item it just streamed. A captured stream either way would settle it. What I can say is that the change assumes a provider may under-populate the deltas — which is the bug you are fixing — while assuming the same provider can never under-populate the snapshot, and nothing here justifies treating those differently.

The fixture change belongs in the description

Related to the above, and worth separating from it: the fixture at openai_test.go:625-629 was rewritten in the same commit that makes the old one fail, with no note anywhere saying so. The new fixture is a better wire model — the merge-base one had an output_item.added with no type field and a terminal body that contradicted its own stream — and the added streamed-call and blocking-parity assertions are a real improvement. But a pre-existing test that goes red against new production code, edited alongside it, reads as a laundered regression to anyone who does not re-run the old one. Say in the PR description that the fixture changed and why, whichever way the point above lands.

On landing

Open PR #1373 touches openai.go and openai_test.go on the same terminal-event handling and conflicts with this branch textually, so whoever sequences the two has a merge to resolve, and it is worth a look for a semantic clash as well. Otherwise this merges cleanly into current main with the change intact, and build, vet, gofmt, golangci-lint, staticcheck and apidiff are all clean on both sides.

[ktsoator]

Thanks for the detailed review. I addressed the requested changes in 66d2c4c.

The completed response now replaces the aggregate only when it contains usable non-thought text or a function call and still includes the visible text and function calls collected from the stream. Otherwise, the aggregate is kept.

I added cases for reasoning-only and shorter completed responses, a completed response that omits a streamed function call, and a function call that appears only in response.completed. I also confirmed that the last case fails when the function-call condition is removed.

The PR description now explains why the old function-call fixture was changed. All local checks and CI are passing.

Superseded: re-verified at d9c0d84. The findings behind this review are closed; what remains is carried by a fresh review.

[karolpiotrowicz]

The four things I raised last round are fixed, and I checked each one by running it rather than by reading the diff. A reasoning-only terminal snapshot no longer destroys a streamed answer, a streamed function call is no longer erased by a message-only snapshot, the FunctionCall arm now has a test that discriminates it, and the merge base's own TestModel_GenerateStream_FunctionCall, lifted verbatim, passes against this head and fails against 1c6293a4. The refusal shapes the PR exists to fix all still reach parity with the blocking path, and they now deliver the refusal mid-stream where main delivers nothing at all.

What I am asking for before this merges is tests. Five decisions inside completedContentSupersedes can each be changed to the wrong behavior with the package suite still green, and the predicate has no direct test anywhere in the tree. The one at openai.go:232 is the one I would not merge without, for the reason below. The rest are on their lines, and a single table test of the predicate would cover all five at once.

Beyond that there are two questions about the design that I think you are better placed to answer than I am, and neither needs code from you if the answer is that the current behavior is intended.

The property to hold a fix against

completedContentSupersedes answers a binary question — replace the whole aggregate, or keep the whole aggregate. So on any input where the snapshot and the stream each hold something the other lacks, one of them is discarded. The contract you are working to wants the union of the two, which means merging rather than choosing. That single property explains every point below, and it is more useful to check a broader fix against than any individual case:

• Retention is partial. Both sums at openai.go:221 and :232 filter on !part.Thought, so visible text and function calls are protected and reasoning is protected by nothing.
• Recovery is partial. When the guard returns false at openai.go:195, every snapshot-only item goes with it, including a tool call that never streamed.

The test I would want before merge

Delete && !part.Thought at openai.go:232 and go test ./model/openaimodel/ is still green, but a turn that streamed a visible answer now ends as reasoning only — which is exactly the failure the last round blocked on, and it is also the shape that makes Flow.Run re-call the model up to ten times. That line is doing the most important work in the change and nothing is holding it in place.

Your existing reasoning-only snapshot does not replace an answer case cannot catch it. Its reasoning text is "Weighing options", which does not contain the streamed answer, so the substring check at line 241 fails whether the filter is there or not.

A fourth case in TestModel_GenerateStream_NarrowerCompletedContentPreservesAggregate does catch it. I ran this one — it passes as written, it fails with the filter removed, and the three existing cases in that table pass either way.

The case

{
    name: "reasoning-only snapshot containing the answer does not replace it",
    events: []string{
        evCreated,
        evDelta1,
        evDelta2,
        `{"type":"response.completed","response":{"id":"resp_1","model":"stream-model","status":"completed","output":[{"id":"rs_1","type":"reasoning","status":"completed","summary":[],"content":[{"type":"reasoning_text","text":"hello is the answer"}]}]}}`,
    },
    want: genai.NewContentFromText("hello", genai.RoleModel),
},

Does a streamed reasoning part have to survive the turn?

This is a genuine question about the contract rather than a defect I can prove, and I could not settle it from the code.

When the snapshot carries no reasoning item, the guard replaces the aggregate and the streamed thought is gone. Measured against main, with a stream of one reasoning delta plus "hello" and a terminal snapshot holding only the message:

	final content
main	[THOUGHT("Checking the request"), TEXT("hello")]
this head	[TEXT("hello")]
blocking path, same body	[TEXT("hello")]

So head matches the blocking path here and main does not, which is the parity the PR is built for. TestModel_GenerateStream_CompletedFunctionCallReplacesThoughtOnlyAggregate already asserts a final identical to blocking after a reasoning delta, so the choice looks deliberate — I would just like it stated, because the retention rule as written does not carve reasoning out and the next person to read completedContentSupersedes will not know which way it is meant to go. A comment on the function saying parity wins for reasoning parts would settle it.

Worth knowing before you decide: the thought does still reach the caller as a partial, and dropping it cannot trigger the thought-only re-call loop, because superseding requires a non-thought part to be present.

Adding refusal deltas changed which path a refusal stream takes

This one is new, and it comes from the two halves of the PR interacting rather than from either alone.

Before this change there was no response.refusal.delta case in the translator, so refusal deltas fell through to the default and nothing reached the aggregator. A refusal-only stream therefore always took the final == nil rebuild and got the terminal snapshot unconditionally. stream.go:53 now puts those deltas into the aggregate, so the same stream goes through the new predicate instead:

stream, then terminal snapshot	main	this head
refusal deltas "I cannot help.", snapshot's refusal is different text	snapshot's wording	"I cannot help."
refusal deltas "I cannot help.", snapshot is reasoning only	[THOUGHT("Policy check failed")]	[TEXT("I cannot help.")]

I am not claiming head is worse for the user on either row — keeping the refusal the reader actually saw looks better than replacing it with a bare thought, and main only reaches parity there by throwing the whole stream away. The point is narrower: this is the one shape class where "the guard returned false" no longer means "behaves like main", nothing covers it, and it is easy to miss because the two changes look independent.

Where the all-or-nothing choice costs the most

A stream that narrates before calling a tool loses the call, when the snapshot carries only the call:

response.created
response.output_text.delta   "Let me check the weather."
response.completed           output: [ function_call get_weather {"city":"SF"} ]

The visible text makes the aggregate's text non-empty, the snapshot has no text to contain it, the guard returns false, and the final response carries zero function calls. The final is the only response that reaches tool dispatch, so the agent says it is about to check the weather and then does nothing. With a reasoning delta in place of the visible text the same stream recovers the call correctly, which is what makes this specifically a consequence of the binary choice rather than a bug in the matching.

main behaves the same way, so nothing regresses here — but recovering terminal-only tool calls is one of the things this PR set out to do, and it currently only works when nothing visible streamed first.

Smaller things

The remaining points are on their lines. Beyond the tests asked for above, the only one worth acting on in this PR is the missing log line at openai.go:193, and that is your call rather than a requirement.

One note for sequencing rather than for you: the logprobs of a discarded snapshot are still applied to the retained aggregate, which is true on main too and is untouched by this change. #1373 is working on finish reason and logprobs in this same function and is the natural place for it, and it currently conflicts with this branch on openai.go.

[ktsoator]

Thanks for checking these cases. I added direct tests for the predicate and confirmed that all five mutations you listed now fail. The stream test also covers reasoning containing the visible answer.

I reproduced the null argument case and normalized it to an empty map in the stream decoder, matching blocking. The completed pong text is now retained.

For reasoning, the current choice is intentional: when replacement is allowed, the final content follows the completed response. Streamed thoughts still reach the caller as partials. I added a comment and tests for both reasoning delta types.

The ID fallback and the visible-text-then-terminal-only-call case remain unresolved. I have kept the binary replacement rule for this revision and documented those limits; I am not claiming general reconciliation here. Local build, race tests, lint and tidy checks pass in both modules.

[karolpiotrowicz]

The test I asked for is in, and it holds the line it needs to hold. Deleting && !part.Thought at openai.go:235 now fails three cases including the end-to-end one, and restoring the previous revision's test file makes that same deletion pass again — so it is the new tests doing the work rather than coverage that was already there. The other four decisions in the predicate are in the same position: each can still be flipped to the wrong behavior, and each now fails a case in TestCompletedContentSupersedes. Both reproductions from #1466 and #1476, run verbatim, fail on the merge base and pass here.

On the null arguments: you did not take the fix I suggested, and yours is the better of the two. I proposed relaxing the comparison inside completedContentSupersedes, which would have confined the change to the predicate and left the streamed call diverging from the blocking path everywhere else — including on the partial response the caller receives mid-stream, which the predicate never touches. Normalizing in emitFunctionCall instead makes the streamed call identical to the blocking one before anything compares them, and it leaves the strict reflect.DeepEqual in place, which matters because relaxing that to a name match lets a snapshot substitute different arguments for the streamed ones. I ran a stream that narrates, calls ping with "arguments":"null", and receives a snapshot carrying that call plus "pong": both the final response and the mid-stream partial now match blocking exactly, where on the previous revision the partial carried a nil map and "pong" was lost.

The doc comment at openai.go:209-211 answers the reasoning question I could not settle from the code, and the test beside it pins the answer. That was the last thing I had open.

Nothing here blocks. Three things are still worth doing and none of them belongs in another round on this PR — the ID fallback you flagged as overlapping #1373, the missing log line when terminal conversion fails, and the whitespace-sensitive text comparison. I am glad to see those become issues so this can land.

One sequencing note rather than something for you to fix: the branch is three commits behind main, and it conflicts with #1373 and #1392 on openai.go, so whichever lands second will need a rebase.

Superseded by the approving review. The items this review named are verified closed at the current head.

[ktsoator]

Thanks for the thorough review and for checking the reproductions. I’ll check for existing coverage and open follow-up issues for the remaining points, keeping the overlap with #1373 in mind. I’ll leave this PR focused and update the branch if needed before merging.