-
Notifications
You must be signed in to change notification settings - Fork 3.9k
fix(openai): responses function_call mapping #8935
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: main
Are you sure you want to change the base?
Changes from all commits
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 |
|---|---|---|
|
|
@@ -774,6 +774,90 @@ function toResponseInputContentList( | |
| return list; | ||
| } | ||
|
|
||
| // Synthesize an orchestrator wrapper | ||
| function synthesizeOrchestratorCall( | ||
| toolCalls: ToolCallDelta[], | ||
| input: ResponseInput, | ||
| respId?: string, | ||
| prevReasoningId?: string, | ||
| ) { | ||
| // Build subcalls array for the orchestrator from the expanded toolCalls | ||
| const subcalls = toolCalls.map((tc) => { | ||
| const fname = tc?.function?.name as string | undefined; | ||
| const rawArgs = tc?.function?.arguments as string | undefined; | ||
| let parsedArgs: any = {}; | ||
| try { | ||
| parsedArgs = rawArgs && rawArgs.length ? JSON.parse(rawArgs) : {}; | ||
| } catch { | ||
| parsedArgs = rawArgs ?? {}; | ||
| } | ||
|
|
||
| return { | ||
| // use a single canonical field for the function name | ||
| function_name: fname || "", | ||
| parameters: parsedArgs, | ||
| // preserve original call id when present | ||
| call_id: tc?.id || undefined, | ||
| original_arguments: rawArgs || undefined, | ||
| }; | ||
| }); | ||
|
|
||
| const wrapperArgs = { tool_uses: subcalls }; | ||
|
|
||
| // Derive a single Responses item id for the orchestrator | ||
| const rawWrapperId = respId || prevReasoningId; | ||
| const wrapperItemId = | ||
| rawWrapperId && rawWrapperId.startsWith("fc") | ||
| ? rawWrapperId | ||
| : `fc_${rawWrapperId}`; | ||
|
Collaborator
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. I think this tool call ID modification would mess things up in GUI where we use tool call ID for streaming tool call ouput and a variety of things. If it doesn't match the original many GUI things will fail |
||
|
|
||
| const wrapperCallId = wrapperItemId; | ||
|
|
||
| const functionCallItem: ResponseFunctionToolCall = { | ||
| id: wrapperItemId, | ||
| type: "function_call", | ||
| name: "multi_tool_use", | ||
| arguments: JSON.stringify(wrapperArgs), | ||
| call_id: wrapperCallId, | ||
| }; | ||
|
|
||
| input.push(functionCallItem); | ||
| } | ||
|
|
||
| function emitFunctionCallsFor( | ||
| toolCallsArr: ToolCallDelta[], | ||
| input: ResponseInput, | ||
| respId?: string, | ||
| prevReasoningId?: string, | ||
| ) { | ||
| for (let j = 0; j < toolCallsArr.length; j++) { | ||
| const tc = toolCallsArr[j]; | ||
| const call_id = tc?.id as string | undefined; | ||
|
|
||
| // Prefer respId or prevReasoningId. | ||
| let rawItemId = respId || prevReasoningId || call_id; | ||
| if ((respId || prevReasoningId) && toolCallsArr.length > 1) { | ||
| rawItemId = `${rawItemId}_${j}`; | ||
| } | ||
| if (!rawItemId) { | ||
| // Can't create a valid Responses item id for this call; skip it | ||
| continue; | ||
| } | ||
|
|
||
| const itemId = rawItemId.startsWith("fc") ? rawItemId : `fc_${rawItemId}`; | ||
|
|
||
| const functionCallItem: ResponseFunctionToolCall = { | ||
| id: itemId, | ||
| type: "function_call", | ||
| name: tc.function?.name || "", | ||
| arguments: tc.function?.arguments || "", | ||
| call_id: call_id || respId || prevReasoningId || "", | ||
| }; | ||
|
|
||
| input.push(functionCallItem); | ||
| } | ||
| } | ||
|
|
||
| export function toResponsesInput(messages: ChatMessage[]): ResponseInput { | ||
| const input: ResponseInput = []; | ||
|
|
||
|
|
@@ -816,22 +900,31 @@ export function toResponsesInput(messages: ChatMessage[]): ResponseInput { | |
| const respId = msg.metadata?.responsesOutputItemId as | ||
| | string | ||
| | undefined; | ||
| const prevMsgForThis = messages[i - 1] as ChatMessage | undefined; | ||
|
Collaborator
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. this type casting seems unnecessary |
||
| const prevReasoningId = prevMsgForThis?.metadata?.reasoningId as | ||
| | string | ||
| | undefined; | ||
| const toolCalls = msg.toolCalls as ToolCallDelta[] | undefined; | ||
|
|
||
| if (respId && Array.isArray(toolCalls) && toolCalls.length > 0) { | ||
| // Emit full function_call output item | ||
| const tc = toolCalls[0]; | ||
| const name = tc?.function?.name as string | undefined; | ||
| const args = tc?.function?.arguments as string | undefined; | ||
| const call_id = tc?.id as string | undefined; | ||
| const functionCallItem: ResponseFunctionToolCall = { | ||
| id: respId, | ||
| type: "function_call", | ||
| name: name || "", | ||
| arguments: typeof args === "string" ? args : "{}", | ||
| call_id: call_id || respId, | ||
| }; | ||
| input.push(functionCallItem); | ||
| if (Array.isArray(toolCalls) && toolCalls.length > 0) { | ||
| // If multiple tool calls immediately follow a reasoning (thinking) | ||
| // message, synthesize a single orchestrator function_call so the reasoning | ||
| // item is followed by exactly one action. This avoids protocol violations | ||
| // where a reasoning item is followed by multiple function_call items. | ||
| const shouldSynthesizeOrchestrator = | ||
| toolCalls.length > 1 && !!prevReasoningId; | ||
|
|
||
| if (shouldSynthesizeOrchestrator) { | ||
|
Collaborator
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. consider restructuring so that |
||
| synthesizeOrchestratorCall( | ||
| toolCalls, | ||
| input, | ||
| respId, | ||
| prevReasoningId, | ||
| ); | ||
| } | ||
|
|
||
| // Emit function_call items directly from toolCalls | ||
| emitFunctionCallsFor(toolCalls, input, respId, prevReasoningId); | ||
|
Collaborator
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. consider restructuring so that emitFunctionCallsFor has pure inputs and outputs rather than directly modifying
Collaborator
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. Should this be if else behavior? I.e. if should snythesize, do that, else emit function calls directly? Or I might be misunderstanding |
||
| } else if (respId) { | ||
| // Emit full assistant output message item | ||
| const outputMessageItem: ResponseOutputMessage = { | ||
|
|
||
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.
can use existing safeParseArgs utils for this