fix(ai-proxy): fix gemini provider missing finishReason #2408
Conversation
Signed-off-by: Xijun Dai <[email protected]>
修复Gemini Provider的finishReason格式及流式响应缺失问题变更概述问题修复
变更统计
变更文件
💡 小贴士与 lingma-agents 交流的方式📜 直接回复评论
📜 在代码行处标记
📜 在讨论中提问
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2408 +/- ##
===========================================
+ Coverage 35.91% 46.05% +10.14%
===========================================
Files 69 81 +12
Lines 11576 13018 +1442
===========================================
+ Hits 4157 5995 +1838
+ Misses 7104 6678 -426
- Partials 315 345 +30 🚀 New features to boost your workflow:
|
![lingma-agents[bot]](https://avatars.githubusercontent.com/in/1198481?s=60&v=4){kind=small}
There was a problem hiding this comment.
🔎 代码评审报告
🎯 评审意见概览
| 严重度 | 数量 | 说明 |
|---|---|---|
| 🔴 Blocker | 0 | 阻断性问题，需立即修复。例如：系统崩溃、关键功能不可用或严重安全漏洞。 |
| 🟠 Critical | 1 | 严重问题，高优先级修复。例如：核心功能异常或性能瓶颈影响用户体验。 |
| 🟡 Major | 0 | 主要问题，建议修复。例如：非核心功能缺陷或代码维护性较差。 |
| 🟢 Minor | 0 | 次要问题，酬情优化。例如：代码格式不规范或注释缺失。 |
总计: 1 个问题
📋 评审意见详情
💡 代码实现建议
以下是文件级别的代码建议，聚焦于代码的可读性、可维护性和潜在问题。
🔹 plugins/wasm-go/extensions/ai-proxy/provider/gemini.go (1 💬)
- 避免在流式响应中硬编码索引[0] (L563)
🚀 架构设计建议
以下是对代码架构和设计的综合分析，聚焦于跨文件交互、系统一致性和潜在优化空间。
🔍1. 跨文件代码重复：GeminiProvider中FinishReason处理逻辑未抽象化
在gemini.go的buildChatCompletionResponse和buildChatCompletionStreamResponse两个方法中，均存在对FinishReason的处理逻辑（如转换为小写并赋值给choice.FinishReason）。该逻辑在两个独立方法中重复实现，违反了DRY原则。若后续需要修改处理逻辑（例如调整大小写转换规则或增加校验），需要同步修改多个位置，增加维护成本。建议将该逻辑提取为公共方法，例如CreateChoiceWithFinishReason。
📌 关键代码
choice.FinishReason = util.Ptr(strings.ToLower(candidate.FinishReason))if geminiResp.Candidates[0].FinishReason != "" {
choice.FinishReason = util.Ptr(strings.ToLower(geminiResp.Candidates[0].FinishReason))
}未来维护时可能导致逻辑不一致，若需要修改处理规则需多处修改，增加人为错误风险
🔍2. 架构一致性风险：其他提供商的FinishReason处理方式未统一
当前PR仅修复了Gemini提供商的FinishReason问题，但需确认其他AI提供商（如OpenAI、Anthropic等）的响应处理是否遵循相同的规范。若其他提供商未统一进行FinishReason的小写转换或格式标准化，可能导致下游服务接收到不一致的响应格式，影响系统兼容性。建议建立统一的响应转换层或规范文档。
不同提供商返回的FinishReason格式差异可能导致前端或调用方解析逻辑混乱，引发业务错误
🔍3. 潜在空指针风险：未处理Candidates数组为空的情况
在buildChatCompletionStreamResponse方法中，直接使用Candidates[0]而未验证数组是否为空。虽然现有文件建议已指出硬编码索引问题，但需确保在架构层面增加空值检查机制。若Gemini接口返回空Candidates数组，可能导致panic或无效数据流输出。
📌 关键代码
if len(geminiResp.Candidates) > 0 && len(geminiResp.Candidates[0].Content.Parts) > 0 {
...处理逻辑...
}当Candidates数组为空时可能引发数组越界错误，导致服务崩溃或返回错误响应
🔍4. 扩展性不足：硬编码的字段访问方式限制未来扩展
当前代码直接访问geminiResp.Candidates[0].FinishReason等具体字段，若Gemini API未来返回结构发生变化（例如字段名称变更、新增多候选结果），将需要修改多处代码。建议通过配置化或抽象接口方式解耦具体字段访问逻辑，例如使用字段映射表或结构体标签。
API变更时需大量修改代码，增加维护成本和版本兼容风险
审查详情
📒 文件清单 （1 个文件）
📝 变更: 1 个文件
📝 变更文件:
plugins/wasm-go/extensions/ai-proxy/provider/gemini.go
💡 小贴士
与 lingma-agents 交流的方式
📜 直接回复评论
直接回复本条评论，lingma-agents 将自动处理您的请求。例如：
-
在当前代码中添加详细的注释说明。
-
请详细介绍一下你说的 LRU 改造方案，并使用伪代码加以说明。
📜 在代码行处标记
在文件的特定位置创建评论并 @lingma-agents。例如：
-
@lingma-agents 分析这个方法的性能瓶颈并提供优化建议。
-
@lingma-agents 对这个方法生成优化代码。
📜 在讨论中提问
在任何讨论中 @lingma-agents 来获取帮助。例如：
-
@lingma-agents 请总结上述讨论并提出解决方案。
-
@lingma-agents 请根据讨论内容生成优化代码。
| var choice chatCompletionChoice | ||
| if len(geminiResp.Candidates) > 0 && len(geminiResp.Candidates[0].Content.Parts) > 0 { | ||
| choice.Delta = &chatMessage{Content: geminiResp.Candidates[0].Content.Parts[0].Text} | ||
| if geminiResp.Candidates[0].FinishReason != "" { |
There was a problem hiding this comment.
避免在流式响应中硬编码索引[0]
🟠 Critical | 🐞 Bugs
📋 问题详情
buildChatCompletionStreamResponse中直接使用Candidates[0]和Parts[0]访问数组元素，未处理数组为空或索引越界的场景。这违反了可靠性原则，可能导致运行时panic。
💡 解决方案
修改建议：
- if len(geminiResp.Candidates) > 0 && len(geminiResp.Candidates[0].Content.Parts) > 0 {
+ if len(geminiResp.Candidates) > 0 {
+ candidate := geminiResp.Candidates[0]
+ if len(candidate.Content.Parts) > 0 {- 将嵌套的索引访问拆分为多层判断
- 避免重复访问
geminiResp.Candidates[0]字段 - 需补充单元测试覆盖Candidates/Parts为空的场景
您的反馈对我们很重要！(建议右键在新标签页中打开以下链接)
Signed-off-by: Xijun Dai <[email protected]> Co-authored-by: Se7en <[email protected]>
3 participants
Ⅰ. Describe what this PR did
修复 finishReason
Ⅱ. Does this pull request fix one issue?
Ⅲ. Why don't you add test cases (unit test/integration test)?
Ⅳ. Describe how to verify it
Ⅴ. Special notes for reviews
Ⅰ. Describe what this PR did
Fix finishReason
Ⅱ. Does this pull request fix one issue?
Ⅲ. Why don't you add test cases (unit test/integration test)?
Ⅳ. Describe how to verify it
V. Special notes for reviews