-
Notifications
You must be signed in to change notification settings - Fork 187
fix(chat): tool calls fixes & e2e tests #675
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
Changes from all commits
dae3cfe
662a125
70b28fc
100bffb
001bf4b
8713827
adf7189
537c296
c0931c3
c720c14
e78c186
b146ff3
2183636
09debba
5da0c8c
9f65813
b9c7bd4
6bbb009
aa5e80b
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 |
|---|---|---|
|
|
@@ -440,6 +440,12 @@ export async function prepareRequestBody( | |
| effectiveTemperature = 1; | ||
| } | ||
|
|
||
| // Check if messages contain existing tool calls or tool results | ||
| // If so, use Chat Completions API instead of Responses API | ||
| const hasExistingToolCalls = messages.some( | ||
| (msg: any) => msg.tool_calls || msg.role === "tool", | ||
| ); | ||
|
|
||
| // Check if the model supports responses API (default to true if reasoning is enabled) | ||
| const providerMapping = modelDef?.providers.find( | ||
| (p) => p.providerId === "openai", | ||
|
|
@@ -448,8 +454,8 @@ export async function prepareRequestBody( | |
| (providerMapping as ProviderModelMapping)?.supportsResponsesApi !== | ||
| false; | ||
|
|
||
| if (supportsReasoning && supportsResponsesApi) { | ||
| // Transform to responses API format (now supports tools as well) | ||
| if (supportsReasoning && supportsResponsesApi && !hasExistingToolCalls) { | ||
| // Transform to responses API format (only when no existing tool calls) | ||
| const responsesBody: OpenAIResponsesRequestBody = { | ||
| model: usedModel, | ||
| input: processedMessages, | ||
|
|
@@ -672,11 +678,19 @@ export async function prepareRequestBody( | |
| if (tools && tools.length > 0) { | ||
| requestBody.tools = [ | ||
| { | ||
| functionDeclarations: tools.map((tool) => ({ | ||
| name: tool.function.name, | ||
| description: tool.function.description, | ||
| parameters: tool.function.parameters, | ||
| })), | ||
| functionDeclarations: tools.map((tool: any) => { | ||
| // Remove additionalProperties and $schema from parameters as Google doesn't accept them | ||
| const { | ||
| additionalProperties: _additionalProperties, | ||
| $schema: _$schema, | ||
| ...cleanParameters | ||
| } = tool.function.parameters || {}; | ||
| return { | ||
| name: tool.function.name, | ||
| description: tool.function.description, | ||
| parameters: cleanParameters, | ||
| }; | ||
| }), | ||
|
Comment on lines
+681
to
+693
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. 🛠️ Refactor suggestion Sanitize tool parameter schemas deeply (nested Only top-level keys are stripped. Nested objects may still carry these fields, causing Google 400s. Consider a recursive prune before assigning Apply this refactor in the mapper: function stripMeta(o: unknown): unknown {
if (!o || typeof o !== "object") return o;
if (Array.isArray(o)) return o.map(stripMeta);
// shallow clone and recurse
const { additionalProperties, $schema, ...rest } = o as Record<string, unknown>;
for (const k of Object.keys(rest)) rest[k] = stripMeta(rest[k]);
return rest;
}Then: - const { additionalProperties: _additionalProperties, $schema: _$schema, ...cleanParameters } = tool.function.parameters || {};
+ const cleanParameters = stripMeta(tool.function.parameters || {});🤖 Prompt for AI Agents |
||
| }, | ||
| ]; | ||
| } | ||
|
|
@@ -742,6 +756,7 @@ export function getProviderEndpoint( | |
| token?: string, | ||
| stream?: boolean, | ||
| supportsReasoning?: boolean, | ||
| hasExistingToolCalls?: boolean, | ||
| ): string { | ||
| let modelName = model; | ||
| if (model && model !== "custom") { | ||
|
|
@@ -866,7 +881,8 @@ export function getProviderEndpoint( | |
| return `${url}/api/paas/v4/chat/completions`; | ||
| case "openai": | ||
| // Use responses endpoint for reasoning models that support responses API | ||
| if (supportsReasoning && model) { | ||
| // but not when there are existing tool calls in the conversation | ||
| if (supportsReasoning && model && !hasExistingToolCalls) { | ||
| const modelDef = models.find((m) => m.id === model); | ||
| const providerMapping = modelDef?.providers.find( | ||
| (p) => p.providerId === "openai", | ||
|
|
@@ -989,6 +1005,7 @@ export async function validateProviderKey( | |
| provider === "google-ai-studio" ? token : undefined, | ||
| false, // validation doesn't need streaming | ||
| false, // supportsReasoning - disable for validation | ||
| false, // hasExistingToolCalls - disable for validation | ||
| ); | ||
|
|
||
| // Use prepareRequestBody to create the validation payload | ||
|
|
||
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.
🛠️ Refactor suggestion
Type strictly: avoid any per repo guideline.
Use BaseMessage[] and unknown for json.
📝 Committable suggestion
🤖 Prompt for AI Agents