-
-
Notifications
You must be signed in to change notification settings - Fork 3.1k
支持 tool_choice 参数转换,优化错误处理 #2540
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?
支持 tool_choice 参数转换,优化错误处理 #2540
Conversation
WalkthroughAdds runtime conversion of OpenAI Changes
Sequence Diagram(s)(omitted) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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.
Actionable comments posted: 1
🧹 Nitpick comments (2)
relay/channel/gemini/relay-gemini.go (2)
1025-1039: Consider adding inline comments for consistency.The non-streaming handler at lines 970-990 includes helpful inline comments explaining each finish reason. For maintainability, consider adding similar comments here.
🔎 Proposed improvement for consistency
case "SAFETY": + // Safety filter triggered choice.FinishReason = &constant.FinishReasonContentFilter case "RECITATION": + // Recitation (citation) detected choice.FinishReason = &constant.FinishReasonContentFilter case "BLOCKLIST": + // Blocklist triggered choice.FinishReason = &constant.FinishReasonContentFilter case "PROHIBITED_CONTENT": + // Prohibited content detected choice.FinishReason = &constant.FinishReasonContentFilter case "SPII": + // Sensitive personally identifiable information choice.FinishReason = &constant.FinishReasonContentFilter case "OTHER": + // Other reasons choice.FinishReason = &constant.FinishReasonContentFilter
1427-1429: Consider lazy initialization for efficiency.The
configstruct is allocated before verifying thattoolChoiceis a supported type. IftoolChoiceis neither a string nor a supported map, this allocation is wasted.🔎 Proposed optimization
- config := &dto.ToolConfig{ - FunctionCallingConfig: &dto.FunctionCallingConfig{}, - } - // Handle string values: "auto", "none", "required" if toolChoiceStr, ok := toolChoice.(string); ok { + config := &dto.ToolConfig{ + FunctionCallingConfig: &dto.FunctionCallingConfig{}, + } switch toolChoiceStr {Apply similar changes for the map case.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
relay/channel/gemini/relay-gemini.go
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-08-08T17:12:43.157Z
Learnt from: RedwindA
Repo: QuantumNous/new-api PR: 1537
File: relay/gemini_handler.go:330-342
Timestamp: 2025-08-08T17:12:43.157Z
Learning: In the new-api repository, the `GeminiEmbeddingHandler` function in `relay/gemini_handler.go` is designed specifically for native Gemini embedding requests and therefore does not require the `ConvertGeminiRequest` step that is used in the chat handler. The embedding requests are already in the native Gemini format and don't need conversion.
Applied to files:
relay/channel/gemini/relay-gemini.go
📚 Learning: 2025-06-21T03:37:41.726Z
Learnt from: 9Ninety
Repo: QuantumNous/new-api PR: 1273
File: relay/channel/gemini/relay-gemini.go:97-116
Timestamp: 2025-06-21T03:37:41.726Z
Learning: In relay/channel/gemini/relay-gemini.go, the thinking budget calculation logic (including the MaxOutputTokens multiplication) was introduced in PR #1247. PR #1273 focused specifically on decoupling the thoughts summary feature from thinking budget settings and did not modify the existing thinking budget behavior.
Applied to files:
relay/channel/gemini/relay-gemini.go
🧬 Code graph analysis (1)
relay/channel/gemini/relay-gemini.go (2)
dto/gemini.go (2)
ToolConfig(46-49)FunctionCallingConfig(51-54)types/error.go (2)
NewOpenAIError(237-260)ErrorCodeEmptyResponse(75-75)
🔇 Additional comments (3)
relay/channel/gemini/relay-gemini.go (3)
359-365: LGTM!The integration is correctly placed after tools are set and the nil check appropriately prevents unnecessary configuration when no tool_choice is specified.
970-990: LGTM!The explicit case handling with inline comments improves code clarity and maintainability, even though all cases map to
ContentFilter. This documents the expected Gemini finish reasons and makes future modifications easier.
1253-1267: LGTM!The error handling correctly differentiates between blocked prompts (returning 400 with the block reason) and unexpected empty responses (returning 500). The nil-check chain is safe and the error messages are informative.
a6f14f4 to
b6a25d9
Compare
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
relay/channel/gemini/relay-gemini.go (1)
1420-1477: Tool_choice conversion implementation correctly maps OpenAI to Gemini with proper edge case handling.The function correctly maps all documented OpenAI
tool_choiceformats to GeminifunctionCallingConfigmodes per the official Gemini API documentation:
- "auto" → "AUTO" ✓
- "none" → "NONE" ✓
- "required" → "ANY" ✓
- Object format with function name extraction → "ANY" + allowedFunctionNames ✓
The implementation returns
nilfor unsupported map structures (whentype != "function"), which is appropriate.One optional consideration: the default case for unknown string values (lines 1448–1450) currently defaults to "AUTO". While this follows the principle of least surprise for unrecognized values, returning
nilwould be more explicit about rejecting unsupported tool_choice values. This depends on your desired API strictness, so it's optional:case "required": config.FunctionCallingConfig.Mode = "ANY" default: - // Unknown string value, default to AUTO - config.FunctionCallingConfig.Mode = "AUTO" + // Unknown string value, return nil to reject unsupported values + return nil }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
relay/channel/gemini/relay-gemini.go
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-08-08T17:12:43.157Z
Learnt from: RedwindA
Repo: QuantumNous/new-api PR: 1537
File: relay/gemini_handler.go:330-342
Timestamp: 2025-08-08T17:12:43.157Z
Learning: In the new-api repository, the `GeminiEmbeddingHandler` function in `relay/gemini_handler.go` is designed specifically for native Gemini embedding requests and therefore does not require the `ConvertGeminiRequest` step that is used in the chat handler. The embedding requests are already in the native Gemini format and don't need conversion.
Applied to files:
relay/channel/gemini/relay-gemini.go
📚 Learning: 2025-06-21T03:37:41.726Z
Learnt from: 9Ninety
Repo: QuantumNous/new-api PR: 1273
File: relay/channel/gemini/relay-gemini.go:97-116
Timestamp: 2025-06-21T03:37:41.726Z
Learning: In relay/channel/gemini/relay-gemini.go, the thinking budget calculation logic (including the MaxOutputTokens multiplication) was introduced in PR #1247. PR #1273 focused specifically on decoupling the thoughts summary feature from thinking budget settings and did not modify the existing thinking budget behavior.
Applied to files:
relay/channel/gemini/relay-gemini.go
🔇 Additional comments (4)
relay/channel/gemini/relay-gemini.go (4)
360-365: LGTM! Clean integration of tool_choice conversion.The implementation correctly converts OpenAI's
tool_choiceparameter to Gemini'stoolConfigonly when tools are present andtool_choiceis specified. The comment clearly documents the mapping behavior.
970-987: Excellent enhancement to finish reason handling.The comprehensive mapping of Gemini-specific finish reasons (SAFETY, RECITATION, BLOCKLIST, PROHIBITED_CONTENT, SPII, OTHER) to
content_filteris well-documented and provides better visibility into why responses were filtered. The inline comments are particularly helpful for future maintainers.
1019-1048: Consistent finish reason handling across streaming and non-streaming paths.The finish reason mappings in the streaming handler match the non-streaming implementation, ensuring consistent behavior. The addition of explanatory comments and the defensive default case handling improve code maintainability.
1263-1276: Improved error handling for empty responses.The enhanced error handling properly distinguishes between blocked prompts (with specific BlockReason) and genuinely empty responses. This provides much better diagnostics for users. The error codes
ErrorCodePromptBlockedandErrorCodeEmptyResponseare correctly defined and used with appropriate HTTP status codes (400 for blocked, 500 for empty response).
问题描述
问题 1: tool_choice 参数不生效
使用 OpenAI 兼容格式调用 Gemini API 时,
tool_choice参数没有被转换为 Gemini 的toolConfig.functionCallingConfig,导致:tool_choice: "required"不生效,AI 可能返回纯文本而不是调用工具tool_choice: "none"不生效,AI 仍然可能调用工具问题 2: FinishReason 处理不完整
Gemini API 返回的 FinishReason 有多种值(SAFETY, RECITATION, BLOCKLIST 等),但除了 STOP 和 MAX_TOKENS 外,其他都被简单地转换为 content_filter,缺少明确注释。
问题 3: Candidates 为空时返回空响应
当 Gemini 因安全过滤返回空 Candidates 时,错误处理代码被注释掉了,导致返回空响应而不是有意义的错误信息。
解决方案
1. tool_choice 转换
添加
tool_choice到toolConfig.functionCallingConfig的转换:"auto""AUTO""none""NONE""required""ANY"{"type": "function", "function": {"name": "xxx"}}"ANY"+allowedFunctionNames2. FinishReason 完整映射
添加所有 Gemini 特有的 FinishReason 值处理:
3. 空 Candidates 错误处理
启用空 Candidates 的错误处理:
PromptFeedback.BlockReason,返回具体的阻止原因修改的文件
relay/channel/gemini/relay-gemini.go参考文档
Summary by CodeRabbit
New Features
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.