fix: replay OpenAI Responses reasoning from encrypted content - #63
ibetitsmike wants to merge 4 commits into
Conversation
Capture encrypted_content, item ID and summary from the completed reasoning output item (stream output_item.done and Generate output) and mark that metadata Finalized. Replay finalized metadata with a blob as a full reasoning input item regardless of store, keeping its position before function, computer and web_search items. Unfinalized metadata (stream placeholders and rows persisted before this field) keeps the previous behavior: item_reference with store=true, skipped otherwise. Update the recorded follow-up request bodies of the summary-thinking cassettes to include the replayed reasoning items.
Empty summaries remain empty arrays rather than invented summary text entries.
…nses Record whether the source response was stored on finalized reasoning metadata and only replay a following web_search_call item reference when it was. Reasoning from an unstored response is still replayed inline from its encrypted content.
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Codex Review: Didn't find any major issues. What shall we delve into next? Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
mafredri
left a comment
mafredri
left a comment
There was a problem hiding this comment.
Capturing the reasoning item from output_item.done and replaying it inline regardless of store is the right shape, and gating inline replay on Finalized keeps the output_item.added placeholders already stored in Coder chats away from the API. One concern, one check, two suggestions.
Suggestions:
- The six summary-thinking cassettes (Azure gpt-5-mini, OpenAI gpt-5, OpenAI o4-mini) were edited offline, and the UAT through Coder ran gpt-5-mini and gpt-5.4. Could we re-record them, so each edited follow-up request has been accepted by the real API?
- charmbracelet#407 replays the same item fields inline, but only for
store=false, and has noFinalizedgate, so it would also replay the.addedplaceholder blobs in rows persisted before the fix. Could we proposeFinalizedthere, so the fork does not have to carry it?
Didn't review the tests in depth.
🤖 This review was automatically generated with Coder Agents.
| continue | ||
| } | ||
| // Item-reference replay predates source storage metadata. | ||
| canReferenceWebSearch = !meta.Finalized || meta.SourceStoreEnabled |
There was a problem hiding this comment.
Concern: a web search the model emitted without a reasoning item before it is still never replayed. This is unchanged by the PR, but since it rewrites this gate it seems the place to fix it. OpenAI does emit those: gpt-4.1-mini (search), and gpt-5.4-mini and gpt-5.5 at effort none (open_page), returned [web_search_call, message] with no reasoning item in every run I tried. With store=true, a bare item_reference to the ws_ id was accepted, and for open_page it brought the page back into the next turn (gpt-5.5: 11,485 input tokens with the reference, 4,574 without). For search it only restores the query.
From probes (not documented), the reasoning item is only required when one preceded the search: the 400 names that specific rs_ id. The previous prompt part can't decide this: Coder drops empty-summary legacy reasoning, and Generate drops reasoning with neither summary nor encrypted_content, which leaves a bare ws_ reference that returns 400. I think it needs, per search, whether its source response was stored and which reasoning id preceded it, then a reference only when that reasoning item is in the input or there was none. Searches without that record (rows written before it) would keep today's gate. validateResponsesItemReferences (line 948) also rejects a bare ws_ reference, so it needs the same rule.
🤖
| ID: done.Item.ID, | ||
| ProviderMetadata: fantasy.ProviderMetadata{ | ||
| Name: state.metadata, | ||
| Name: finalResponsesReasoningMetadata(done.Item, params.Store.Value), |
There was a problem hiding this comment.
Check: SourceStoreEnabled records the store the request asked for. For ZDR orgs OpenAI says store "will always be treated as false" (https://developers.openai.com/api/docs/guides/your-data), so a later ws_ reference to that response should 404. Responses echo store, readable via JSON.ExtraFields["store"] in Generate and on response.created in Stream, which arrives before the reasoning output_item.done. Not tested on a ZDR org. If ZDR responses echo false, could SourceStoreEnabled take the echoed value instead?
🤖
2 participants
OpenAI Responses reasoning was only replayed as an
item_reference, which needsstore=true. With the defaultstore=falseit was dropped from later requests, so reasoning models lost their chain of thought between tool steps and turns.This captures the completed reasoning item (ID,
encrypted_content, summary) fromresponse.output_item.doneand from the Generate output, marks that metadataFinalized, and replays it as a full reasoning input item regardless ofstore, before its function, computer, and web search items. Completed summaries are kept verbatim, so an empty summary stays[]. Unfinalized metadata (stream placeholders and rows persisted before this field) keeps the old behavior: an item reference withstore=true, skipped otherwise.Finalized metadata also records whether the source request was stored. A following
web_search_callitem reference is replayed only when both the source and the destination request store; otherwise it is omitted, because a reference to an unstored item returns 404, while the reasoning is still replayed inline.The follow-up request bodies of the six summary-thinking cassettes were updated offline to include the replayed reasoning items; they were not re-recorded live.
The base is
mike/mcp-tool-media-batch-order, the unmerged branch coder/coder currently pins, so this diff contains only these changes. Consumed by coder/coder#30298.Testing
go test ./...,go build ./...,go vet ./..., golangci-lint, and targeted race tests forproviders/openaipass.responses_reasoning_replay_test.gocovers Generate and Stream capture, replay understoretrue and false, unfinalized fallbacks, and web search reference eligibility from stored and unstored sources. Red-green verified by disabling each gate.Review record
govulncheckfails on GO-2026-6505 (otel v1.44.0) and GO-2026-6348 (grpc v1.83.0), which the open dependabot bumps fix; this PR does not changego.mod.