Merge pull request #1235 from afarber/501-remove-duplicated-if-check

fix: remove redundant if-check and add tests for OpenAI converter

packages/core/src/core/openaiContentGenerator/converter.test.ts

@@ -7,7 +7,13 @@
 import { describe, it, expect, beforeEach } from 'vitest';
 import { OpenAIContentConverter } from './converter.js';
 import type { StreamingToolCallParser } from './streamingToolCallParser.js';
-import type { GenerateContentParameters, Content } from '@google/genai';
+import {
+  Type,
+  type GenerateContentParameters,
+  type Content,
+  type Tool,
+  type CallableTool,
+} from '@google/genai';
 import type OpenAI from 'openai';
 
 describe('OpenAIContentConverter', () => {
@@ -202,4 +208,338 @@ describe('OpenAIContentConverter', () => {
       );
     });
   });
+
+  describe('convertGeminiToolsToOpenAI', () => {
+    it('should convert Gemini tools with parameters field', async () => {
+      const geminiTools = [
+        {
+          functionDeclarations: [
+            {
+              name: 'get_weather',
+              description: 'Get weather for a location',
+              parameters: {
+                type: Type.OBJECT,
+                properties: {
+                  location: { type: Type.STRING },
+                },
+                required: ['location'],
+              },
+            },
+          ],
+        },
+      ] as Tool[];
+
+      const result = await converter.convertGeminiToolsToOpenAI(geminiTools);
+
+      expect(result).toHaveLength(1);
+      expect(result[0]).toEqual({
+        type: 'function',
+        function: {
+          name: 'get_weather',
+          description: 'Get weather for a location',
+          parameters: {
+            type: 'object',
+            properties: {
+              location: { type: 'string' },
+            },
+            required: ['location'],
+          },
+        },
+      });
+    });
+
+    it('should convert MCP tools with parametersJsonSchema field', async () => {
+      // MCP tools use parametersJsonSchema which contains plain JSON schema (not Gemini types)
+      const mcpTools = [
+        {
+          functionDeclarations: [
+            {
+              name: 'read_file',
+              description: 'Read a file from disk',
+              parametersJsonSchema: {
+                type: 'object',
+                properties: {
+                  path: { type: 'string' },
+                },
+                required: ['path'],
+              },
+            },
+          ],
+        },
+      ] as Tool[];
+
+      const result = await converter.convertGeminiToolsToOpenAI(mcpTools);
+
+      expect(result).toHaveLength(1);
+      expect(result[0]).toEqual({
+        type: 'function',
+        function: {
+          name: 'read_file',
+          description: 'Read a file from disk',
+          parameters: {
+            type: 'object',
+            properties: {
+              path: { type: 'string' },
+            },
+            required: ['path'],
+          },
+        },
+      });
+    });
+
+    it('should handle CallableTool by resolving tool function', async () => {
+      const callableTools = [
+        {
+          tool: async () => ({
+            functionDeclarations: [
+              {
+                name: 'dynamic_tool',
+                description: 'A dynamically resolved tool',
+                parameters: {
+                  type: Type.OBJECT,
+                  properties: {},
+                },
+              },
+            ],
+          }),
+        },
+      ] as CallableTool[];
+
+      const result = await converter.convertGeminiToolsToOpenAI(callableTools);
+
+      expect(result).toHaveLength(1);
+      expect(result[0].function.name).toBe('dynamic_tool');
+    });
+
+    it('should skip functions without name or description', async () => {
+      const geminiTools = [
+        {
+          functionDeclarations: [
+            {
+              name: 'valid_tool',
+              description: 'A valid tool',
+            },
+            {
+              name: 'missing_description',
+              // no description
+            },
+            {
+              // no name
+              description: 'Missing name',
+            },
+          ],
+        },
+      ] as Tool[];
+
+      const result = await converter.convertGeminiToolsToOpenAI(geminiTools);
+
+      expect(result).toHaveLength(1);
+      expect(result[0].function.name).toBe('valid_tool');
+    });
+
+    it('should handle tools without functionDeclarations', async () => {
+      const emptyTools: Tool[] = [
+        {} as Tool,
+        { functionDeclarations: [] },
+      ];
+
+      const result = await converter.convertGeminiToolsToOpenAI(emptyTools);
+
+      expect(result).toHaveLength(0);
+    });
+
+    it('should handle functions without parameters', async () => {
+      const geminiTools: Tool[] = [
+        {
+          functionDeclarations: [
+            {
+              name: 'no_params_tool',
+              description: 'A tool without parameters',
+            },
+          ],
+        },
+      ];
+
+      const result = await converter.convertGeminiToolsToOpenAI(geminiTools);
+
+      expect(result).toHaveLength(1);
+      expect(result[0].function.parameters).toBeUndefined();
+    });
+
+    it('should not mutate original parametersJsonSchema', async () => {
+      const originalSchema = {
+        type: 'object',
+        properties: { foo: { type: 'string' } },
+      };
+      const mcpTools: Tool[] = [
+        {
+          functionDeclarations: [
+            {
+              name: 'test_tool',
+              description: 'Test tool',
+              parametersJsonSchema: originalSchema,
+            },
+          ],
+        } as Tool,
+      ];
+
+      const result = await converter.convertGeminiToolsToOpenAI(mcpTools);
+
+      // Verify the result is a copy, not the same reference
+      expect(result[0].function.parameters).not.toBe(originalSchema);
+      expect(result[0].function.parameters).toEqual(originalSchema);
+    });
+  });
+
+  describe('convertGeminiToolParametersToOpenAI', () => {
+    it('should convert type names to lowercase', () => {
+      const params = {
+        type: 'OBJECT',
+        properties: {
+          count: { type: 'INTEGER' },
+          amount: { type: 'NUMBER' },
+          name: { type: 'STRING' },
+        },
+      };
+
+      const result = converter.convertGeminiToolParametersToOpenAI(params);
+
+      expect(result).toEqual({
+        type: 'object',
+        properties: {
+          count: { type: 'integer' },
+          amount: { type: 'number' },
+          name: { type: 'string' },
+        },
+      });
+    });
+
+    it('should convert string numeric constraints to numbers', () => {
+      const params = {
+        type: 'object',
+        properties: {
+          value: {
+            type: 'number',
+            minimum: '0',
+            maximum: '100',
+            multipleOf: '0.5',
+          },
+        },
+      };
+
+      const result = converter.convertGeminiToolParametersToOpenAI(params);
+      const properties = result?.['properties'] as Record<string, unknown>;
+
+      expect(properties?.['value']).toEqual({
+        type: 'number',
+        minimum: 0,
+        maximum: 100,
+        multipleOf: 0.5,
+      });
+    });
+
+    it('should convert string length constraints to integers', () => {
+      const params = {
+        type: 'object',
+        properties: {
+          text: {
+            type: 'string',
+            minLength: '1',
+            maxLength: '100',
+          },
+          items: {
+            type: 'array',
+            minItems: '0',
+            maxItems: '10',
+          },
+        },
+      };
+
+      const result = converter.convertGeminiToolParametersToOpenAI(params);
+      const properties = result?.['properties'] as Record<string, unknown>;
+
+      expect(properties?.['text']).toEqual({
+        type: 'string',
+        minLength: 1,
+        maxLength: 100,
+      });
+      expect(properties?.['items']).toEqual({
+        type: 'array',
+        minItems: 0,
+        maxItems: 10,
+      });
+    });
+
+    it('should handle nested objects', () => {
+      const params = {
+        type: 'object',
+        properties: {
+          nested: {
+            type: 'object',
+            properties: {
+              deep: {
+                type: 'INTEGER',
+                minimum: '0',
+              },
+            },
+          },
+        },
+      };
+
+      const result = converter.convertGeminiToolParametersToOpenAI(params);
+      const properties = result?.['properties'] as Record<string, unknown>;
+      const nested = properties?.['nested'] as Record<string, unknown>;
+      const nestedProperties = nested?.['properties'] as Record<string, unknown>;
+
+      expect(nestedProperties?.['deep']).toEqual({
+        type: 'integer',
+        minimum: 0,
+      });
+    });
+
+    it('should handle arrays', () => {
+      const params = {
+        type: 'array',
+        items: {
+          type: 'INTEGER',
+        },
+      };
+
+      const result = converter.convertGeminiToolParametersToOpenAI(params);
+
+      expect(result).toEqual({
+        type: 'array',
+        items: {
+          type: 'integer',
+        },
+      });
+    });
+
+    it('should return undefined for null or non-object input', () => {
+      expect(
+        converter.convertGeminiToolParametersToOpenAI(
+          null as unknown as Record<string, unknown>,
+        ),
+      ).toBeNull();
+      expect(
+        converter.convertGeminiToolParametersToOpenAI(
+          undefined as unknown as Record<string, unknown>,
+        ),
+      ).toBeUndefined();
+    });
+
+    it('should not mutate the original parameters', () => {
+      const original = {
+        type: 'OBJECT',
+        properties: {
+          count: { type: 'INTEGER' },
+        },
+      };
+      const originalCopy = JSON.parse(JSON.stringify(original));
+
+      converter.convertGeminiToolParametersToOpenAI(original);
+
+      expect(original).toEqual(originalCopy);
+    });
+  });
 });

packages/core/src/core/openaiContentGenerator/converter.ts

@@ -193,13 +193,11 @@ export class OpenAIContentConverter {
             // Handle both Gemini tools (parameters) and MCP tools (parametersJsonSchema)
             if (func.parametersJsonSchema) {
               // MCP tool format - use parametersJsonSchema directly
-              if (func.parametersJsonSchema) {
+              // Create a shallow copy to avoid mutating the original object
-                // Create a shallow copy to avoid mutating the original object
+              const paramsCopy = {
-                const paramsCopy = {
+                ...(func.parametersJsonSchema as Record<string, unknown>),
-                  ...(func.parametersJsonSchema as Record<string, unknown>),
+              };
-                };
+              parameters = paramsCopy;
-                parameters = paramsCopy;
-              }
             } else if (func.parameters) {
               // Gemini tool format - convert parameters to OpenAI format
               parameters = this.convertGeminiToolParametersToOpenAI(