-
Notifications
You must be signed in to change notification settings - Fork 902
feat(ai-proxy): add responses support for doubao #2509
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
Conversation
Signed-off-by: Xijun Dai <[email protected]>
新增火山方舟大模型Responses接口支持变更概述新功能
重构
变更文件
时序图sequenceDiagram
participant HC as HttpContext
participant DP as doubaoProvider
HC->>DP: HandleRequest("/v1/responses")
DP->>DP: GetApiName() → ApiNameResponses
DP->>DP: TransformRequestBody()
DP-->>DP: 删除不支持参数
DP-->>DP: 记录删除失败日志
DP-->>HC: 返回处理后的请求体
💡 小贴士与 lingma-agents 交流的方式📜 直接回复评论
📜 在代码行处标记
📜 在讨论中提问
Change OverviewNew Features
Refactor
Change file
Sequence chartsequenceDiagram
participant HC as HttpContext
participant DP as doubaoProvider
HC->>DP: HandleRequest("/v1/responses")
DP->>DP: GetApiName() → ApiNameResponses
DP->>DP: TransformRequestBody()
DP-->>DP: Delete parameters not supported
DP-->>DP: Log deletion failed log
DP-->>HC: Returns the processed request body
💡 TipsHow to communicate with lingma-agents📜 Reply to comments directly
**📜 Mark ** at line of code
📜 Ask a question during discussion
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2509 +/- ##
===========================================
+ Coverage 35.91% 46.04% +10.13%
===========================================
Files 69 81 +12
Lines 11576 13020 +1444
===========================================
+ Hits 4157 5995 +1838
+ Misses 7104 6680 -424
- Partials 315 345 +30 🚀 New features to boost your workflow:
|
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.
🔎 代码评审报告
🎯 评审意见概览
| 严重度 | 数量 | 说明 |
|---|---|---|
| 🔴 Blocker | 0 | 阻断性问题,需立即修复。例如:系统崩溃、关键功能不可用或严重安全漏洞。 |
| 🟠 Critical | 2 | 严重问题,高优先级修复。例如:核心功能异常或性能瓶颈影响用户体验。 |
| 🟡 Major | 1 | 主要问题,建议修复。例如:非核心功能缺陷或代码维护性较差。 |
| 🟢 Minor | 0 | 次要问题,酬情优化。例如:代码格式不规范或注释缺失。 |
总计: 3 个问题
📋 评审意见详情
💡 代码实现建议
以下是文件级别的代码建议,聚焦于代码的可读性、可维护性和潜在问题。
🔹 plugins/wasm-go/extensions/ai-proxy/main.go (1 💬)
- 路径中的版本号与常量定义不一致,导致API名称无法正确识别 (L409-L411)
🔹 plugins/wasm-go/extensions/ai-proxy/provider/doubao.go (2 💬)
- 参数删除错误处理不充分,导致错误被忽略 (L85-L90)
- 使用Contains检查路径可能导致误匹配,应改为更精确的检查方式 (L114-L116)
🚀 架构设计建议
以下是对代码架构和设计的综合分析,聚焦于跨文件交互、系统一致性和潜在优化空间。
🔍1. 路径版本号不一致导致API名称识别风险
在main.go中使用'/v1/responses'路径检查,而doubao.go中定义的常量为'/api/v3/responses',路径版本号不一致可能导致API名称无法正确匹配。跨文件的路径定义需统一版本规范以确保系统一致性。
📌 关键代码
+ if strings.HasSuffix(path, "/v1/responses") {+ doubaoResponsesPath = "/api/v3/responses"路径版本不一致将导致API路由逻辑失效,请求无法正确分发到目标接口,影响功能可用性。
🔍2. 路径匹配方式存在误判风险
在doubao.go的GetApiName方法中,使用strings.Contains检查路径是否包含doubaoResponsesPath,可能导致路径误匹配(如'/api/v3/responses/test'会被错误识别)。应采用精确路径解析或正则表达式增强匹配准确性。
📌 关键代码
+ if strings.Contains(path, doubaoResponsesPath) {误匹配将导致请求被错误路由,可能引发安全漏洞或功能异常。
🔍3. JSON操作技术债务问题
在TransformRequestBody方法中直接使用sjson操作原始字节流处理JSON参数,未采用结构体解析方式。此方式易引发字段误操作,且难以维护。建议引入DTO对象进行类型安全操作。
📌 关键代码
+ body, err = sjson.DeleteBytes(body, param)字符串操作易受JSON格式变化影响,增加未来维护成本并可能引入隐蔽错误。
🔍4. 跨功能代码重复问题
TransformRequestBody中对不同API的请求体修改逻辑(如参数删除/添加)采用分散实现,未提取公共方法。建议将JSON操作封装为复用函数以减少冗余代码。
📌 关键代码
+ for _, param := range []string{"parallel_tool_calls", ... } { body, err = sjson.DeleteBytes(body, param) ... }+ body, err = sjson.SetBytes(body, "watermark", false)重复代码将导致维护时需要同步修改多个位置,增加人为错误风险。
审查详情
📒 文件清单 (2 个文件)
📝 变更: 2 个文件
📝 变更文件:
plugins/wasm-go/extensions/ai-proxy/main.goplugins/wasm-go/extensions/ai-proxy/provider/doubao.go
💡 小贴士
与 lingma-agents 交流的方式
📜 直接回复评论
直接回复本条评论,lingma-agents 将自动处理您的请求。例如:
-
在当前代码中添加详细的注释说明。
-
请详细介绍一下你说的 LRU 改造方案,并使用伪代码加以说明。
📜 在代码行处标记
在文件的特定位置创建评论并 @lingma-agents。例如:
-
@lingma-agents 分析这个方法的性能瓶颈并提供优化建议。
-
@lingma-agents 对这个方法生成优化代码。
📜 在讨论中提问
在任何讨论中 @lingma-agents 来获取帮助。例如:
-
@lingma-agents 请总结上述讨论并提出解决方案。
-
@lingma-agents 请根据讨论内容生成优化代码。
| if strings.HasSuffix(path, "/v1/responses") { | ||
| return provider.ApiNameResponses | ||
| } |
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.
路径中的版本号与常量定义不一致,导致API名称无法正确识别
🟠 Critical | 🐞 Bugs
📋 问题详情
在getApiName函数中新增的条件检查路径是否以“/v1/responses”结尾,而doubao.go中定义的doubaoResponsesPath是“/api/v3/responses”。这两个路径的版本号(v1 vs v3)和结构不一致,导致条件判断失败,无法正确返回ApiNameResponses。需统一路径版本和结构。
💡 解决方案
将路径检查中的“/v1/responses”改为与doubao.go中定义的doubaoResponsesPath一致的“/api/v3/responses”:
-if strings.HasSuffix(path, "/v1/responses") {
+if strings.HasSuffix(path, "/api/v3/responses") {您的反馈对我们很重要!(建议右键在新标签页中打开以下链接)
| for _, param := range []string{"parallel_tool_calls", "tool_choice", "stream_options"} { | ||
| body, err = sjson.DeleteBytes(body, param) | ||
| if err != nil { | ||
| log.Warnf("[doubao] failed to delete %s in request body, err: %v", param, err) | ||
| } | ||
| } |
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.
参数删除错误处理不充分,导致错误被忽略
🟠 Critical | 🐞 Bugs
📋 问题详情
在TransformRequestBody函数中,当sjson.DeleteBytes发生错误时仅记录警告并继续处理,但未返回错误可能导致后续逻辑异常。建议在首次错误时立即返回错误。
💡 解决方案
在参数删除失败时立即返回错误:
@@ -88,6 +88,7 @@
log.Warnf("[doubao] failed to delete %s in request body, err: %v", param, err)
+ return nil, err
}
}您的反馈对我们很重要!(建议右键在新标签页中打开以下链接)
| if strings.Contains(path, doubaoResponsesPath) { | ||
| return ApiNameResponses | ||
| } |
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.
使用Contains检查路径可能导致误匹配,应改为更精确的检查方式
🟡 Major | 🐞 Bugs
📋 问题详情
在GetApiName函数中,使用strings.Contains检查路径是否包含doubaoResponsesPath(“/api/v3/responses”)可能导致误匹配,例如路径“/api/v3/responsesomething”也会触发该条件。建议改为使用strings.HasSuffix确保路径结尾精确匹配。
💡 解决方案
将Contains改为HasSuffix确保路径精确匹配:
-if strings.Contains(path, doubaoResponsesPath) {
+if strings.HasSuffix(path, doubaoResponsesPath) {您的反馈对我们很重要!(建议右键在新标签页中打开以下链接)
CH3CHO
left a 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.
LGTM
Signed-off-by: Xijun Dai <[email protected]>
Ⅰ. Describe what this PR did
Ⅱ. 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