Repository navigation
fix(ai): accept fully-identified tool calls after the finish reason #58843
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: dev
Are you sure you want to change the base?
Changes from 14 commits
646f90e
bee7f9b
008a8fa
3eff40a
e67695f
0bbae88
9a344c0
2711496
67942eb
9d5a159
4696286
0b73814
39ac1da
2ed3073
ce9f795
712971f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -93,7 +93,11 @@ const OpenRouterDetailFields = { | |
| } | ||
| const ReasoningDetail = Schema.Union([ | ||
| Schema.StructWithRest( | ||
| Schema.Struct({ type: Schema.Literal("reasoning.text"), text: Schema.optional(Schema.String), ...OpenRouterDetailFields }), | ||
| Schema.Struct({ | ||
| type: Schema.Literal("reasoning.text"), | ||
| text: Schema.optional(Schema.String), | ||
| ...OpenRouterDetailFields, | ||
| }), | ||
| [Schema.Record(Schema.String, Schema.Unknown)], | ||
| ), | ||
| Schema.StructWithRest( | ||
|
|
@@ -313,6 +317,8 @@ export interface ParserState { | |
| readonly providerMetadataKey: string | ||
| readonly tools: ToolStream.State<number> | ||
| readonly pendingTools: Partial<Record<number, PendingToolDelta>> | ||
| /** Tool call ids finalized in-step; late replay deltas for them are dropped. */ | ||
| readonly finishedToolIds: ReadonlyArray<string> | ||
| readonly toolCallEvents: ReadonlyArray<LLMEvent> | ||
| readonly usage?: Usage | ||
| readonly finishReason?: FinishReasonDetails | ||
|
|
@@ -420,10 +426,7 @@ const lowerReasoningDetail = (detail: ReasoningDetail) => { | |
|
|
||
| const isKimiDetail = (detail: { readonly type: string }) => detail.type === "summary" || detail.type === "encrypted" | ||
|
|
||
| const lowerUserMessage = Effect.fnUntraced(function* ( | ||
| message: OpenAIChatRequestMessage, | ||
| options: LoweringOptions, | ||
| ) { | ||
| const lowerUserMessage = Effect.fnUntraced(function* (message: OpenAIChatRequestMessage, options: LoweringOptions) { | ||
| const content: OpenAIChatUserContent[] = [] | ||
| for (const part of message.content) { | ||
| if (part.type === "text") { | ||
|
|
@@ -509,10 +512,7 @@ const lowerAssistantMessage = Effect.fnUntraced(function* ( | |
| return { ...result, [field]: reasoningText } | ||
| }) | ||
|
|
||
| const lowerToolMessages = Effect.fnUntraced(function* ( | ||
| message: OpenAIChatRequestMessage, | ||
| options: LoweringOptions, | ||
| ) { | ||
| const lowerToolMessages = Effect.fnUntraced(function* (message: OpenAIChatRequestMessage, options: LoweringOptions) { | ||
| const messages: OpenAIChatMessage[] = [] | ||
| const attachments: OpenAIChatUserContent[] = [] | ||
| for (const part of message.content) { | ||
|
|
@@ -620,10 +620,7 @@ const lowerMessages = Effect.fnUntraced(function* (request: LLMRequest, options: | |
| if (pendingAttachments.length > 0) { | ||
| messages.push({ | ||
| role: "user", | ||
| content: [ | ||
| ...pendingAttachments.splice(0), | ||
| { type: "text", text: part.text, cache_control: cacheControl }, | ||
| ], | ||
| content: [...pendingAttachments.splice(0), { type: "text", text: part.text, cache_control: cacheControl }], | ||
| }) | ||
| continue | ||
| } | ||
|
|
@@ -641,10 +638,7 @@ const lowerMessages = Effect.fnUntraced(function* (request: LLMRequest, options: | |
| else if (previous?.role === "user" && Array.isArray(previous.content)) | ||
| messages[messages.length - 1] = { | ||
| role: "user", | ||
| content: [ | ||
| ...previous.content, | ||
| { type: "text", text: part.text, cache_control: cacheControl }, | ||
| ], | ||
| content: [...previous.content, { type: "text", text: part.text, cache_control: cacheControl }], | ||
| } | ||
| else | ||
| messages.push( | ||
|
|
@@ -1059,19 +1053,21 @@ const step = (state: ParserState, event: OpenAIChatEvent) => | |
| let pendingTools = state.pendingTools | ||
| let latestToolIndex = state.latestToolIndex | ||
| let nextToolIndex = state.nextToolIndex | ||
| let { finishedToolIds } = state | ||
|
|
||
| let lifecycle = state.lifecycle | ||
|
|
||
| const reasoning = reasoningDelta(delta, state.reasoningField) | ||
| if ( | ||
| state.finishReason !== undefined && | ||
| toolDeltas.some((tool) => Boolean(tool.id) || Boolean(tool.function?.name) || Boolean(tool.function?.arguments)) | ||
| ) | ||
| return yield* ProviderShared.eventError( | ||
| ADAPTER, | ||
| "OpenAI Chat received content after the finish reason", | ||
| ProviderShared.encodeJson(event), | ||
| ) | ||
|
|
||
| // Deltas that arrive after a terminal chunk are absorbed into the open | ||
| // channels instead of failing the step. Several OpenAI-compatible hosts | ||
| // (radoxai.com, vLLM behind LiteLLM, DeepInfra via OpenRouter, SenseNova) | ||
| // emit real output — text, reasoning, or fully-identified tool calls — | ||
| // after `finish_reason`; failing the step threw away a complete, already | ||
| // billed answer. The finish event and its usage are published once, when | ||
| // the stream ends, so absorbing these deltas preserves spec-legal event | ||
| // ordering. Tool calls that open post-finish stay open in `tools` until | ||
| // the terminal's close finalizes them. See #43040 / #56455. | ||
|
|
||
| const reasoningField = state.reasoningField ?? reasoning?.field | ||
| const reasoningTextObserved = state.reasoningTextObserved || reasoning !== undefined | ||
|
|
@@ -1111,6 +1107,10 @@ const step = (state: ParserState, event: OpenAIChatEvent) => | |
| const current = tools[index] | ||
| const pending = pendingTools[index] | ||
| const id = current?.id ?? pending?.id ?? (tool.id || undefined) | ||
| // A host that replays an already finalized call after the finish would | ||
| // otherwise open a second identical call. Drop the replay instead; the | ||
| // settled tool keeps its own parsed input. | ||
| if (id && finishedToolIds.includes(id)) continue | ||
| const name = current?.name ?? pending?.name ?? (tool.function?.name || undefined) | ||
| const text = `${pending?.input ?? ""}${tool.function?.arguments ?? ""}` | ||
| const extraContent = pending?.extraContent ?? decodeExtraContent(tool.extra_content) | ||
|
|
@@ -1167,19 +1167,30 @@ const step = (state: ParserState, event: OpenAIChatEvent) => | |
| ) | ||
|
|
||
| // Filtering or truncation terminates the response without confirming pending tool calls. | ||
| const finished = | ||
| const finalizeTools = | ||
| finishReason !== undefined && | ||
| !incompleteTools && | ||
| state.finishReason === undefined && | ||
| Object.keys(tools).length > 0 | ||
| ? yield* ToolStream.finishAll(ADAPTER, tools) | ||
| : undefined | ||
| // The in-step terminal finalizes every open call; remember their ids so a | ||
| // later replay of the same call is dropped instead of re-opened. | ||
| const finalizedToolIds = finalizeTools | ||
| ? [ | ||
| ...state.finishedToolIds, | ||
| ...Object.values(tools) | ||
| .filter((tool) => tool !== undefined) | ||
| .map((tool) => tool.id) | ||
| .filter((id): id is string => id !== undefined), | ||
| ] | ||
| : state.finishedToolIds | ||
| const finished = finalizeTools ? yield* ToolStream.finishAll(ADAPTER, tools) : undefined | ||
|
|
||
| return [ | ||
| { | ||
| providerMetadataKey: state.providerMetadataKey, | ||
| tools: finished?.tools ?? tools, | ||
| pendingTools, | ||
| finishedToolIds: finalizedToolIds, | ||
| toolCallEvents: finished?.events ?? state.toolCallEvents, | ||
| usage, | ||
| finishReason: state.finishReason ?? finishReason, | ||
|
|
@@ -1206,11 +1217,35 @@ export const finishEvents = Effect.fnUntraced(function* (state: ParserState) { | |
| route: ADAPTER, | ||
| }), | ||
| }) | ||
| // Identity-less tool fragments cannot form a call: fail instead of settling | ||
| // the step with silently dropped data — the same rule the terminal chunk | ||
| // applies mid-stream. Empty no-op deltas (no id, no name, no argument text) | ||
| // do not reach `pendingTools` with anything visible, so they never count as | ||
| // fragments. Truncation finishes intentionally leave calls unconfirmed. | ||
| // Streams without a finish reason keep their existing leniency here. | ||
| const unresolvableFragments = Object.values(state.pendingTools).some( | ||
| (fragment) => | ||
| fragment !== undefined && (Boolean(fragment.id) || Boolean(fragment.name) || fragment.input.length > 0), | ||
| ) | ||
| const truncatedFinish = | ||
| state.finishReason?.normalized === "content-filter" || state.finishReason?.normalized === "length" | ||
| if (state.finishReason !== undefined && !truncatedFinish && unresolvableFragments) | ||
| return yield* new AIError({ | ||
| reason: new InvalidProviderOutputError({ | ||
| message: "OpenAI Chat tool call delta is missing id or name", | ||
| route: ADAPTER, | ||
| }), | ||
| }) | ||
| const events: LLMEvent[] = [] | ||
| const toolCallEvents = | ||
| state.finishReason === undefined && Object.keys(state.tools).length > 0 | ||
| // Tool calls that opened after the finish reason are still unconfirmed here | ||
| // and finalize with the terminal's tool-call close. A `length` or | ||
| // `content-filter` finish truncates on purpose, so their late calls stay | ||
| // unconfirmed — the same rule that applies when the finish arrives. | ||
| const lateToolEvents = | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Only
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Addressed in bee7f9b: |
||
| !truncatedFinish && Object.keys(state.tools).length > 0 | ||
| ? (yield* ToolStream.finishAll(ADAPTER, state.tools)).events | ||
| : state.toolCallEvents | ||
| : [] | ||
| const toolCallEvents = [...state.toolCallEvents, ...lateToolEvents] | ||
| const hasToolCalls = toolCallEvents.length > 0 | ||
| const reason = state.finishReason | ||
| ? { | ||
|
|
@@ -1264,6 +1299,7 @@ export const protocol = Protocol.make({ | |
| tools: ToolStream.empty<number>(), | ||
| pendingTools: {}, | ||
| toolCallEvents: [], | ||
| finishedToolIds: [], | ||
| lifecycle: Lifecycle.initial(), | ||
| reasoningField: request.model.compatibility?.reasoningField, | ||
| reasoningTextObserved: false, | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Calls finalized at the first finish chunk are removed from
toolsbyfinishAll, so a later tool delta can no longer be matched to them. If a host sends the same call again after the finish (sameid,nameandarguments), it starts a second tool and the response has two identicaltoolCalls, so the tool would run twice. Before this change that stream failed. Could late deltas whoseidmatches an already finalized call be ignored, or rejected, rather than starting a new call?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Addressed in bee7f9b: the parser now records the ids of every call finalized in-step and drops trailing deltas whose resolved id matches one, so a replayed call can no longer open as a second call and run twice. Covered by a new test (
ignores a replay of an already finalized tool call) that sends the identical index/id/name/arguments call after the stop and asserts a single tool call, unchanged input, and the "tool-calls" finish upgrade. Full packages/ai suite: 1888 pass / 0 fail.