From 565bfdfda7dd4090b659672edf4489b793e9345c Mon Sep 17 00:00:00 2001 From: Andrew Beal Date: Wed, 10 Dec 2025 21:47:27 +0000 Subject: [PATCH] Refactor: move legacy function conversion to BaseAIClass Consolidate convertFunctionCallToText and convertFunctionResponseToText methods from Claude and Gemini into BaseAIClass for code reuse. Update OpenAI to handle missing function call IDs by converting to legacy text format, preventing undefined call_id bugs during cross-provider switches. --- AIClasses/BaseAIClass.ts | 18 ++++++ AIClasses/Claude/Claude.ts | 15 ----- AIClasses/Gemini/Gemini.ts | 15 ----- AIClasses/OpenAI/OpenAI.ts | 60 +++++++++++++------ .../CrossProviderIntegration.test.ts | 55 +++++++++++++++++ 5 files changed, 116 insertions(+), 47 deletions(-) diff --git a/AIClasses/BaseAIClass.ts b/AIClasses/BaseAIClass.ts index bdb9cbf..d77f2f4 100644 --- a/AIClasses/BaseAIClass.ts +++ b/AIClasses/BaseAIClass.ts @@ -124,4 +124,22 @@ export abstract class BaseAIClass implements IAIClass { await this.aiPrompt.userInstruction() ].filter(s => s).join("\n\n"); } + + /** + * Converts a function call to legacy text format for cross-provider compatibility. + * Used when a provider doesn't have the required ID field (e.g., Gemini → Claude/OpenAI). + */ + protected convertFunctionCallToText(parsedContent: StoredFunctionCall): string { + const inputJson = JSON.stringify(parsedContent.functionCall.args); + return `[Legacy Tool Call] ${parsedContent.functionCall.name}\nInput: ${inputJson}`; + } + + /** + * Converts a function response to legacy text format for cross-provider compatibility. + * Used when a provider doesn't have the required ID field (e.g., Gemini → Claude/OpenAI). + */ + protected convertFunctionResponseToText(parsedContent: StoredFunctionResponse): string { + const resultJson = JSON.stringify(parsedContent.functionResponse.response); + return `[Legacy Tool Result] ${parsedContent.functionResponse.name}\nResult: ${resultJson}`; + } } diff --git a/AIClasses/Claude/Claude.ts b/AIClasses/Claude/Claude.ts index a22193d..1cd07d4 100644 --- a/AIClasses/Claude/Claude.ts +++ b/AIClasses/Claude/Claude.ts @@ -229,19 +229,4 @@ export class Claude extends BaseAIClass { } })); } - - /* - If a conversation used another provider it may not have function id's required by Claude. - Instead provide the function call and response as plain text to preserve context without breaking. - */ - - private convertFunctionCallToText(parsedContent: StoredFunctionCall): string { - const inputJson = JSON.stringify(parsedContent.functionCall.args); - return `[Legacy Tool Call] ${parsedContent.functionCall.name}\nInput: ${inputJson}`; - } - - private convertFunctionResponseToText(parsedContent: StoredFunctionResponse): string { - const resultJson = JSON.stringify(parsedContent.functionResponse.response); - return `[Legacy Tool Result] ${parsedContent.functionResponse.name}\nResult: ${resultJson}`; - } } diff --git a/AIClasses/Gemini/Gemini.ts b/AIClasses/Gemini/Gemini.ts index cc45b2b..ebed04b 100644 --- a/AIClasses/Gemini/Gemini.ts +++ b/AIClasses/Gemini/Gemini.ts @@ -232,19 +232,4 @@ export class Gemini extends BaseAIClass { parameters: functionDefinition.parameters as FunctionDeclaration['parameters'] })); } - - /* - If a conversation used another provider it may not have thought signatures required by Gemini 3. - Instead provide the function call and response as plain text to preserve context without breaking. - */ - - private convertFunctionCallToText(parsedContent: import("AIClasses/Schemas/AIFunctionTypes").StoredFunctionCall): string { - const inputJson = JSON.stringify(parsedContent.functionCall.args); - return `[Legacy Tool Call] ${parsedContent.functionCall.name}\nInput: ${inputJson}`; - } - - private convertFunctionResponseToText(parsedContent: import("AIClasses/Schemas/AIFunctionTypes").StoredFunctionResponse): string { - const resultJson = JSON.stringify(parsedContent.functionResponse.response); - return `[Legacy Tool Result] ${parsedContent.functionResponse.name}\nResult: ${resultJson}`; - } } \ No newline at end of file diff --git a/AIClasses/OpenAI/OpenAI.ts b/AIClasses/OpenAI/OpenAI.ts index 667b6b2..0c01fde 100644 --- a/AIClasses/OpenAI/OpenAI.ts +++ b/AIClasses/OpenAI/OpenAI.ts @@ -6,6 +6,7 @@ import { AIProvider, AIProviderURL, toProviderModel } from "Enums/ApiProvider"; import { AIFunctionCall } from "AIClasses/AIFunctionCall"; import { fromString as aiFunctionFromString } from "Enums/AIFunction"; import type { IAIFunctionDefinition } from "AIClasses/FunctionDefinitions/IAIFunctionDefinition"; +import type { StoredFunctionCall, StoredFunctionResponse } from "AIClasses/Schemas/AIFunctionTypes"; import type { ResponseEvent, ResponseOutputTextDelta, ResponseOutputItemDone, ResponseErrorEvent, ResponseFailedEvent, OpenAIFunctionTool, ResponsesAPIInput } from "./OpenAITypes"; import { Exception } from "Helpers/Exception"; import { ApiErrorType } from "Types/ApiError"; @@ -187,22 +188,37 @@ export class OpenAI extends BaseAIClass { const parsedContent = this.parseFunctionCall(content.functionCall); if (parsedContent) { - // Add assistant text message if present - const messageContent = contentToExtract.trim(); - if (messageContent !== "") { + // Check if function call has required id field (for OpenAI Responses API) + if (parsedContent.functionCall.id && parsedContent.functionCall.id.trim() !== "") { + // Add assistant text message if present + const messageContent = contentToExtract.trim(); + if (messageContent !== "") { + results.push({ + role: content.role as "user" | "assistant", + content: messageContent + }); + } + + // Add function call as separate input item + results.push({ + type: "function_call", + call_id: parsedContent.functionCall.id, + name: parsedContent.functionCall.name, + arguments: JSON.stringify(parsedContent.functionCall.args) + }); + } else { + // No id (from Gemini or legacy) - convert to text message + const legacyText = this.convertFunctionCallToText(parsedContent); + const messageContent = contentToExtract.trim(); + const combinedContent = messageContent !== "" + ? `${messageContent}\n\n${legacyText}` + : legacyText; + results.push({ role: content.role as "user" | "assistant", - content: messageContent + content: combinedContent }); } - - // Add function call as separate input item - results.push({ - type: "function_call", - call_id: parsedContent.functionCall.id, - name: parsedContent.functionCall.name, - arguments: JSON.stringify(parsedContent.functionCall.args) - }); } else { // Fall back to regular message if parsing fails results.push({ @@ -218,11 +234,21 @@ export class OpenAI extends BaseAIClass { const parsedContent = this.parseFunctionResponse(contentToExtract); if (parsedContent) { - results.push({ - type: "function_call_output", - call_id: parsedContent.id, - output: JSON.stringify(parsedContent.functionResponse.response) - }); + // Check if response has required id field (for OpenAI Responses API) + if (parsedContent.id && parsedContent.id.trim() !== "") { + results.push({ + type: "function_call_output", + call_id: parsedContent.id, + output: JSON.stringify(parsedContent.functionResponse.response) + }); + } else { + // No id (from Gemini or legacy) - convert to text message + const legacyText = this.convertFunctionResponseToText(parsedContent); + results.push({ + role: content.role as "user" | "assistant", + content: legacyText + }); + } } else { // Fall back to regular user message if parsing fails results.push({ diff --git a/__tests__/AIClasses/CrossProviderIntegration.test.ts b/__tests__/AIClasses/CrossProviderIntegration.test.ts index fe3c2a6..3fa1c3c 100644 --- a/__tests__/AIClasses/CrossProviderIntegration.test.ts +++ b/__tests__/AIClasses/CrossProviderIntegration.test.ts @@ -808,6 +808,61 @@ describe('Cross-Provider Integration - Thought Signature Support', () => { }); }); + it('should convert Gemini function call (no id) to OpenAI legacy format - REGRESSION', () => { + // REGRESSION TEST: Bug discovered where Gemini → OpenAI switching failed + // because OpenAI tried to use undefined call_id + // Gemini function call with thoughtSignature but no id + const geminiFunctionCall = new ConversationContent( + Role.Assistant, + '', + '', + JSON.stringify({ + functionCall: { + name: 'search_vault_files', + args: { query: 'test' } + } + }), + new Date(), + true, + false, + undefined, + 'gemini_signature_123==' // Has thoughtSignature + ); + + const geminiResponse = (() => { + const responseContent = JSON.stringify({ + // Gemini response without id field + functionResponse: { + name: 'search_vault_files', + response: ['file1.md', 'file2.md'] + } + }); + const content = new ConversationContent(Role.User, responseContent, responseContent); + content.isFunctionCallResponse = true; + return content; + })(); + + // OpenAI should convert to legacy text format (not try to use undefined call_id) + const openai = new OpenAI(); + const result = (openai as any).extractContents([geminiFunctionCall, geminiResponse]); + + // Should have 2 items (both converted to messages) + expect(result).toHaveLength(2); + + // First should be a text message with legacy format (NOT a function_call with undefined call_id) + expect(result[0]).toHaveProperty('role'); + expect(result[0]).toHaveProperty('content'); + expect(result[0].content).toContain('[Legacy Tool Call]'); + expect(result[0].content).toContain('search_vault_files'); + expect(result[0]).not.toHaveProperty('type'); // Should be message, not function_call + expect(result[0]).not.toHaveProperty('call_id'); // Should NOT have call_id field + + // Second should be a text message with legacy format + expect(result[1]).toHaveProperty('role'); + expect(result[1]).toHaveProperty('content'); + expect(result[1].content).toContain('[Legacy Tool Result]'); + }); + it('should handle OpenAI → Gemini → Claude round-trip', () => { // Test that function call survives multiple provider switches const conversation = [