Skip to content

Conversation

@daixijun
Copy link
Contributor

…nsistent with the openai

Ⅰ. Describe what this PR did

修改前, usage 为空时输出空 json 对象, logprobs,finish_reason 字段为空时不输出

image

修改后, usage,finish_reason,logprobs 字段为空时输出 null,保持与官方接口一致
image

Ⅱ. 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

@daixijun daixijun requested review from CH3CHO, cr7258 and rinfx as code owners June 10, 2025 08:28
@lingma-agents
Copy link

lingma-agents bot commented Jun 10, 2025

feat(ai-proxy): 调整流式响应结构以保持与OpenAI接口的一致性

变更文件

文件路径 变更说明
plugins/​wasm-go/​extensions/​ai-proxy/​provider/​bedrock.go 将FinishReason和Usage字段改为指针类型,使用util.Ptr处理空值场景
plugins/​wasm-go/​extensions/​ai-proxy/​provider/​claude.go 修正FinishReason指针化处理,统一Usage结构体指针格式
plugins/​wasm-go/​extensions/​ai-proxy/​provider/​dify.go 强制FinishReason为finish_reason_stop并使用指针,修正Usage结构体引用方式
plugins/​wasm-go/​extensions/​ai-proxy/​provider/​gemini.go 修正Usage字段指针化,处理候选响应的FinishReason指针转换
plugins/​wasm-go/​extensions/​ai-proxy/​provider/​model.go 将FinishReason改为*string类型,Usage改为*usage类型,添加Logprobs字段指针支持
plugins/​wasm-go/​extensions/​ai-proxy/​util/​ptr.go 实现泛型Ptr函数用于安全转换值类型为指针类型

💡 小贴士

与 lingma-agents 交流的方式

📜 直接回复评论
直接回复本条评论,lingma-agents 将自动处理您的请求。例如:

  • 在当前代码中添加详细的注释说明。

  • 请详细介绍一下你说的 LRU 改造方案,并使用伪代码加以说明。

📜 在代码行处标记
在文件的特定位置创建评论并 @lingma-agents。例如:

  • @Lingma-Agent 分析这个方法的性能瓶颈并提供优化建议。

  • @Lingma-Agent 对这个方法生成优化代码。

📜 在讨论中提问
在任何讨论中 @lingma-agents 来获取帮助。例如:

  • @Lingma-Agent 请总结上述讨论并提出解决方案。

  • @Lingma-Agent 请根据讨论内容生成优化代码。

Copy link

@lingma-agents lingma-agents bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔍 代码评审报告

🎯 评审意见概览

严重度 数量 说明
🔴 Blocker 0 阻断性问题,需立即修复。例如:系统崩溃、关键功能不可用或严重安全漏洞。
🟠 Critical 5 严重问题,高优先级修复。例如:核心功能异常或性能瓶颈影响用户体验。
🟡 Major 1 主要问题,建议修复。例如:非核心功能缺陷或代码维护性较差。
🟢 Minor 0 次要问题,酌情优化。例如:代码格式不规范或注释缺失。

总计: 6 个问题


📋 评审意见详情

💡 单文件建议

以下是文件级别的代码建议,聚焦于代码的可读性、可维护性和潜在问题。
🔹 plugins/wasm-go/extensions/ai-proxy/provider/dify.go (2 💬)
🔹 plugins/wasm-go/extensions/ai-proxy/provider/hunyuan.go (1 💬)
🔹 plugins/wasm-go/extensions/ai-proxy/provider/model.go (1 💬)
🔹 plugins/wasm-go/extensions/ai-proxy/provider/spark.go (1 💬)
🔹 plugins/wasm-go/extensions/ai-proxy/provider/vertex.go (1 💬)

🚀 跨文件建议

以下是对代码架构和设计的综合分析,聚焦于跨文件交互、系统一致性和潜在优化空间。
🔍 1. 字段指针类型转换未全面覆盖导致潜在接口不一致

在多个文件中将Usage字段从非指针类型改为指针类型(如usage*usage),但部分转换场景未统一处理。例如:bedrock.go中openAIFormattedChunk.Usage = &usage{...}采用显式指针构造,而vertex.go中Usage: &usage{...}也遵循此模式,但dify.go的Usage: &response.MetaData.Usage直接引用了嵌套指针。需要确保所有Usage字段赋值均采用指针包装,并检查是否存在未转换的遗留用例。此外,FinishReason字段在部分场景(如qwen.go的FinishReason: util.Ptr(qwenChoice.FinishReason))使用Ptr函数,但vertex.go的FinishReason: util.Ptr(candidate.FinishReason)等已规范使用,需确认是否所有字段赋值均采用统一指针包装方式。

📌 关键代码:

Usage:             &response.MetaData.Usage
usageResponse.Usage = &usage{...}

⚠️ 潜在风险: 接口不一致可能导致序列化/反序列化失败,或下游服务因类型不匹配出现运行时错误。

🔍 2. Ptr工具函数未在所有需要场景应用导致代码冗余

新增的util.Ptr工具函数用于统一指针包装,但在部分场景仍存在手动构造指针的情况。例如:dify.go的responseUsage = &usage{...}直接使用&运算符,而其他文件如bedrock.go的FinishReason: util.Ptr(...)已采用工具函数。建议将所有显式指针构造替换为util.Ptr,以减少代码碎片并提升可维护性。

📌 关键代码:

responseUsage = &usage{...}

⚠️ 潜在风险: 代码风格不一致增加维护成本,可能引发指针/值类型混淆的潜在bug。

🔍 3. Usage结构体未声明为公共类型导致复用障碍

usage结构体在model.go中定义为私有类型(未加大写字母开头),但多个提供商文件(如bedrock.go、claude.go)直接实例化该类型。这违反Go语言的封装原则,可能导致类型暴露和维护风险。应将usage结构体改为公共类型(首字母大写),或提供工厂方法进行实例化。

📌 关键代码:

type usage struct { ... }

⚠️ 潜在风险: 包外直接访问私有结构体可能导致意外修改,破坏数据封装性。

🔍 4. Streaming响应与完整响应的Usage计算逻辑不一致

在流式响应(如hunyuan.go的convertChunkFromHunyuanToOpenAI)中,Usage字段在每个分片都进行独立赋值,而完整响应(如buildChatCompletionResponse)则使用总用量。这可能导致分片累加与最终响应的数值不一致。需要统一分片用量的累积逻辑和最终结算方式。

📌 关键代码:

Usage: &usage{ PromptTokens: hunyuanFormattedChunk.Usage.PromptTokens ... }

⚠️ 潜在风险: 用量统计不准确可能导致计费错误或资源监控数据失真。

🔍 5. FinishReason转换未统一错误处理

各提供商的stopReason转换函数(如stopReasonBedrock2OpenAI)分散实现,且部分场景(如gemini.go的candidate.FinishReason)直接赋值未经过转换。建议将转换逻辑集中到公共函数,并添加默认错误处理机制,避免未定义的finish_reason值。

📌 关键代码:

choice.FinishReason = util.Ptr(candidate.FinishReason)

⚠️ 潜在风险: 不一致的枚举值可能导致客户端解析异常或状态判断错误。


💡 小贴士

与 lingma-agents 交流的方式

📜 直接回复评论
直接回复本条评论,lingma-agents 将自动处理您的请求。例如:

  • 在当前代码中添加详细的注释说明。

  • 请详细介绍一下你说的 LRU 改造方案,并使用伪代码加以说明。

📜 在代码行处标记
在文件的特定位置创建评论并 @lingma-agents。例如:

  • @Lingma-Agent 分析这个方法的性能瓶颈并提供优化建议。

  • @Lingma-Agent 对这个方法生成优化代码。

📜 在讨论中提问
在任何讨论中 @lingma-agents 来获取帮助。例如:

  • @Lingma-Agent 请总结上述讨论并提出解决方案。

  • @Lingma-Agent 请根据讨论内容生成优化代码。

@codecov-commenter
Copy link

codecov-commenter commented Jun 10, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 46.03%. Comparing base (ef31e09) to head (072bde4).
Report is 556 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##             main    #2391       +/-   ##
===========================================
+ Coverage   35.91%   46.03%   +10.12%     
===========================================
  Files          69       81       +12     
  Lines       11576    13018     +1442     
===========================================
+ Hits         4157     5993     +1836     
+ Misses       7104     6679      -425     
- Partials      315      346       +31     

see 78 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Collaborator

@johnlanni johnlanni left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@johnlanni johnlanni merged commit fb8dd81 into alibaba:main Jun 12, 2025
12 checks passed
@daixijun daixijun deleted the feat/chatcompleteions-streaming branch June 12, 2025 09:10
ink-hz pushed a commit to ink-hz/higress-ai-capability-auth that referenced this pull request Nov 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants