fix(bifrost): 恢复兼容渠道的工具降级转发 - #653
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
📝 WalkthroughWalkthrough本次变更为 Bifrost 和 CPA 转换路径增加工具约束保真度处理。代码会校验函数白名单、工具选择模式、目标协议兼容性和工具调用历史。无法保留语义时，转换会返回关键语义损失错误并停止发送。请求适配在副本上执行。新增测试覆盖多种协议、目标提供商、流式调用、两轮工具调用、非法约束和原生路由。 Priority: ⬇️ Low Merge Risk: 🟡 Moderate · up to Some converted tool requests can reach providers with disallowed tools or unrepresentable tool history, changing requested tool behavior. Fix these fidelity gates before merging. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 79e6e20c9d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: QUIET
Plan: Advanced
Run ID: 3095df7e-5a98-4663-82a1-8283dbc11d92
📒 Files selected for processing (8)
internal/execution/bifrost/conversion_fidelity.gointernal/execution/bifrost/conversion_fidelity_test.gointernal/execution/bifrost/tool_compatibility.gointernal/execution/bifrost/tool_compatibility_runtime_test.gointernal/execution/bifrost/tool_compatibility_test.gointernal/execution/cpa/adapter.gointernal/execution/cpa/conversion_fidelity.gointernal/execution/cpa/tool_compatibility_test.go
Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e074ad08a4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b5c79fbe54
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to GitHub limitations.
⚠️ Outside diff range comments (2)
internal/execution/cpa/conversion_fidelity.go (1)
83-103: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winWhen no
allowed_toolsis present, this early return skipssubscriptionResponsesToolHistorySupported. Converted Claude and Antigravity Responses requests can therefore dispatch unsupported tool-history items through both unary and streaming paths, where their semantics cannot be preserved. Validate Responses history independently of allowlist presence before returning.internal/execution/bifrost/tool_compatibility.go (1)
17-65: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
needsToolHistoryCheckremains false for unsupported Responses tool history when no function allowlist is present, so converted requests can skip the target-history validation and dispatch with semantic loss. Require the history check for applicable converted Responses requests regardless of allowlist presence.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: QUIET
Plan: Advanced
Run ID: 1d938ac7-ce4a-4470-a1ad-a6beb7850695
📒 Files selected for processing (2)
internal/execution/bifrost/tool_compatibility.gointernal/execution/bifrost/tool_compatibility_test.go
Included review availability: Your plan provides up to 8 included reviews per hour; 5 remain after this review.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 82f26348f7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to GitHub limitations.
⚠️ Outside diff range comments (3)
internal/execution/cpa/conversion_fidelity.go (2)
175-176: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win仅在 snake_case 配置完整时覆盖 Gemini 配置。
当请求同时包含有效的
toolConfig.functionCallingConfig和不含function_calling_config的tool_config时，Line 175 会覆盖config。Line 182 随后将该请求视为未设置白名单并直接返回。tools中未允许的函数因此不会被过滤。仅当
tool_config.function_calling_config存在时才给予 snake_case 配置优先级。添加该组合输入的回归测试。建议修复
if providerKind == channel.ProviderClaude && gjson.GetBytes(body, "tool_config").Exists() { - config = gjson.GetBytes(body, "tool_config.function_calling_config") + legacyConfig := gjson.GetBytes(body, "tool_config.function_calling_config") + if legacyConfig.Exists() { + config = legacyConfig + } }
90-91: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winThe
!presentreturn skipssubscriptionResponsesToolHistorySupported, so converted Responses requests without anallowed_toolschoice can dispatch unsupported history such asfile_search_callto Claude or Antigravity. Validate history independently of the function allowlist before returning, so unrepresentable history is rejected rather than silently altered.internal/execution/bifrost/conversion_fidelity.go (1)
19-25: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win独立启用工具历史检查
对于没有函数 allowlist 的
OpenAIResponses请求，prepareConvertedToolConstraints不会设置needsToolHistoryCheck。finishConvertedPreparation因此可能跳过convertedTargetPreservesToolHistory。该函数会将custom_tool_call判定为 chat fallback、Anthropic 和 Bedrock 不支持的历史。请求可能绕过CriticalSemanticLoss拒绝并进入执行，导致工具历史无法保留。请将工具历史检查与 allowlist 适配分开启用。
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: QUIET
Plan: Advanced
Run ID: 17b0730b-1c52-4385-8cf3-3bf86b29a476
📒 Files selected for processing (8)
internal/execution/bifrost/conversion_fidelity.gointernal/execution/bifrost/conversion_fidelity_test.gointernal/execution/bifrost/count_tokens_test.gointernal/execution/bifrost/executor.gointernal/execution/bifrost/tool_compatibility_runtime_test.gointernal/execution/bifrost/tool_compatibility_test.gointernal/execution/cpa/conversion_fidelity.gointernal/execution/cpa/tool_compatibility_test.go
Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review.
关联 Issue / Related Issue
Closes #651
变更内容 / Change Content
OpenAI Compatible 的 Responses 和 Anthropic 请求会经 SDK 转为 Chat Completions。Codex 的实际请求同时包含普通 function、namespace 与 web_search；SDK 会过滤无法表达的工具。此前本地保真检查把这类 SDK 降级直接转为 422，造成旧版可继续执行的请求在升级后失败。
兼容性：OpenAI Compatible 的 namespace、custom、服务端搜索等工具可能被 SDK 过滤，指定工具或 required 约束也可能被弱化或省略；本 PR 不承诺这些工具在 Chat 上游可用。无数据迁移，不新增依赖、路由或发送链路。Issue #651 缺少真实上游和请求，尚未完成同形验证。
验证：make check；另有 Codex 混合工具、Responses 指定搜索、Anthropic 指定搜索的普通和流式实际出站回归。
自查清单 / Checklist
make check，或在说明中写明无法运行的原因和未验证范围。 / I ranmake check, or documented why it could not run and what remains unverified.