fix(translator): preserve built-in tools (web_search) to Responses API #553
Conversation
- Pass through non-function tool definitions like web_search - Translate tool_choice for built-in tools and function tools - Add regression tests for built-in tool passthrough
Summary of ChangesHello @XInTheDark, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request resolves an issue where built-in tools, such as "web_search", were inadvertently dropped during the translation process between OpenAI Chat Completions and the Responses API (Codex). The changes ensure that these tools and their associated "tool_choice" configurations are consistently maintained across both translation directions, thereby preventing data loss and enabling the full functionality of built-in tools within the API ecosystem. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request addresses an issue where built-in tools like web_search were being dropped during API translation. The changes correctly preserve these tools when converting from OpenAI Chat Completions to the Codex/Responses API, and also in the reverse direction. The implementation looks correct and new tests have been added to verify the fix.
My review includes a couple of suggestions for improvement. I've pointed out some code duplication in the tool name shortening logic that could be refactored for better maintainability. I've also suggested enhancing the new tests to cover more complex scenarios, such as requests with a mix of built-in and function tools, to make them more robust and prevent potential regressions.
| name := tc.Get("function.name").String() | ||
| if name != "" { | ||
| if short, ok := originalToolNameMap[name]; ok { | ||
| name = short | ||
| } else { | ||
| name = shortenNameIfNeeded(name) | ||
| } | ||
| } |
There was a problem hiding this comment.
This logic for shortening a tool name appears to be duplicated in a few places in this file (e.g., for handling tool_calls around line 208 and tools around line 292). To improve maintainability and reduce code duplication, consider extracting this into a helper function. This would also ensure consistent handling of tool name shortening across different parts of the request translation.
| @@ -0,0 +1,54 @@ | |||
| package test | |||
There was a problem hiding this comment.
The added tests are a great start for verifying the new functionality. To make them more robust and prevent future regressions, consider expanding them to cover scenarios with mixed tool types. For both TestOpenAIToCodex_PreservesBuiltinTools and TestOpenAIResponsesToOpenAI_PreservesBuiltinTools, adding a test case with both a built-in tool (e.g., web_search) and a function tool would ensure that the changes don't interfere with existing functionality. Using a table-driven test structure could help organize these cases cleanly.
|
Please attach the failed request payload for our verification. |
2 participants
Problem
Fix
Tests