refactor(ai): resolve responses item ids once at the stream boundary - #46885
Merged
Merged
Conversation
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.
Why
The Responses parser had grown a second identity path for
function_callitems that arrive without anid:item.id ?? call_idcoalescing,Object.values(state.tools).some((tool) => tool?.id === call_id)scans on dedup and terminal reconciliation, conditionalitemIdmetadata, and per-event "message is missing id" guards. #46084 extended it further. The parser body should be able to assumeitem.idexists; the missing-id policy belongs in one place.What the wire actually does
Probed every Responses API we have credentials for, with a plain-text turn and a forced tool-call turn each:
output_item.addedoutput_item.doneresponse.completed.outputgpt-5-mini)gpt-5.4-nano)grok-4-1-fast*)api.meta.ai,muse-spark-1.1)perplexity/sonar,openai/gpt-5-mini)openai.gpt-oss-120b/20b)id— renamed toitem_id, strayoutput_indexinside the itemid— sameSo the Open Responses schema is right that
idis required, and five of six providers honor it, but Mantle's gateway drops it from terminalfunction_callitems. That is the real case the old fallbacks were serving, and a strict schema would regress it.What changes
OpenResponses.normalize(state, event)runs beforestepand does two things: the existingoutput_index→item_idremap for delta events, and item-id resolution mirroring Codex'sassign_missing_streamed_response_item_id: an item without anidadopts the id already registered in its output slot (state.outputItems[output_index]; forresponse.completed.output, the array position), otherwise it gets a locally mintedfc_/msg_/rs_/cmp_+ UUID.step,onOutputItemAdded,onOutputItemDone, and the hosted-tool helpers takeOutputItem = StreamItem & { id: string }and never look at a missing id again.call_idscans,item.id ?? call_idcoalescing, conditionalitemIdmetadata,isReasoningItemnarrowing, and themessage is missing idguards. Pending tools,completedTools, and terminal reconciliation are keyed byitem.idonly.steps (openai-responses,xai-responses) callnormalizefirst;outputItemIDis no longer exported.item_idpresent, strayoutput_index) and a done-only no-id case asserting a mintedfc_id. Removed tests whose only purpose was id-less items; fixed fixtures that omitted ids incidentally.Live verification
Ran a real two-turn tool-call conversation through the patched client, switching provider between the tool call and the tool result:
fc_…recovered fromoutput_item.added(the Mantle case)fc_…replayed to Mantlefc_<uuid>→ OpenAI / Azure / xAI / MantlecompileRequest)