Conversation
- 处理:Codex 请求强制输出 store:false,并在 Claude→Codex 转换中显式设置。
- 相关:internal/converter/types_codex.go, internal/converter/
claude_to_codex.go
- Unsupported parameter: max_output_tokens
- 处理:上游前把 max_output_tokens 改写为 max_tokens 并移除旧字段。
- 相关:internal/adapter/provider/codex/adapter.go, internal/adapter/provider/
codex/adapter_test.go
- Unknown parameter: input[i].role
- 处理:非 message 类型的 input 项去掉 role,保留 message 的 role;同时去掉
Claude→Codex 中 function_call 的 role。
- 相关:internal/adapter/provider/codex/adapter.go, internal/adapter/provider/
cliproxyapi_codex/adapter.go, internal/converter/claude_to_codex.go
- Invalid input[i].id ... Expected an ID that begins with 'fc'
- 处理:function_call 的 id 统一前缀 fc_;并在清洗阶段兜底修正。
- 相关:internal/converter/claude_to_codex.go, internal/adapter/provider/codex/
adapter.go, internal/adapter/provider/cliproxyapi_codex/adapter.go
- Missing required parameter: input[i].output
- 处理:tool_result 内容统一转成字符串;若缺 output 字段则补空字符串。
- 相关:internal/converter/claude_to_codex.go, internal/adapter/provider/codex/
adapter.go, internal/adapter/provider/cliproxyapi_codex/adapter.go
📝 Walkthrough演练此 PR 为 Codex API 请求引入了载荷清理和规范化,包括将 max_output_tokens 迁移至 max_tokens、规范化输入数组字段(移除 role 字段、为 function_call ID 添加前缀)以及更新 Claude 至 Codex 的转换逻辑。 变更
预估代码审查工作量🎯 3 (中等) | ⏱️ ~25 分钟 可能相关的 Issue
可能相关的 PR
建议审阅者
诗
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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.
Actionable comments posted: 2
♻️ Duplicate comments (1)
internal/adapter/provider/cliproxyapi_codex/adapter.go (1)
265-268:function_call的空id未添加fc_前缀(与codex/adapter.go相同问题)此处与
codex/adapter.go存在相同的空 ID 处理缺失问题,见codex/adapter.go中的 review 意见。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/adapter/provider/cliproxyapi_codex/adapter.go` around lines 265 - 268, The code currently only prefixes non-empty function_call ids with "fc_" (in the block handling itemType == "function_call"), leaving empty ids unchanged; update the logic in the adapter.go snippet so that when itemType == "function_call" you always set input.%d.id to "fc_"+id (even if id == ""), using sjson.SetBytes with fmt.Sprintf("input.%d.id", i), and only skip when the id already has the "fc_" prefix (i.e., check strings.HasPrefix(id, "fc_") before setting).
🧹 Nitpick comments (1)
internal/adapter/provider/codex/adapter_test.go (1)
17-17: 测试数据未覆盖function_call的fc_ID 规范化和function_call_output的output默认值逻辑
- 测试中
input[1](function_call)没有id字段,无法覆盖非空id的fc_前缀补全逻辑(id != ""分支)。- 测试输入中没有
function_call_output类型的条目,output缺失时的默认值补全路径未被覆盖。建议补充测试用例:
// function_call with non-fc_ id {"type":"function_call","id":"toolu_01","name":"t","arguments":"{}"} // function_call_output without output field {"type":"function_call_output","call_id":"toolu_01"}并断言
input.N.id以"fc_"开头,input.M.output存在且为空字符串。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/adapter/provider/codex/adapter_test.go` at line 17, The test currently builds `body` without covering the `function_call` ID normalization (the `id != ""` branch that prepends "fc_") and the `function_call_output` default `output` filling path; update the test in adapter_test.go to include a `function_call` entry with a non-`fc_` id (e.g., `{"type":"function_call","id":"toolu_01","name":"t","arguments":"{}"}`) and a `function_call_output` entry missing `output` (e.g., `{"type":"function_call_output","call_id":"toolu_01"}`), then after invoking the code assert that the resulting parsed `input[N].id` starts with `"fc_"` and that `input[M].output` exists and is the empty string (""), covering the `fc_` prefixing logic and `output` defaulting logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@internal/adapter/provider/cliproxyapi_codex/adapter.go`:
- Around line 256-278: The sanitizeCodexPayload implementation duplicates the
input-normalization loop also present in applyCodexRequestTuning; extract that
shared loop into a single exported helper (e.g., NormalizeCodexInput or
NormalizeInputItems) in a new internal package (suggest internal/codexutil) and
replace the body of sanitizeCodexPayload and the input-handling section of
applyCodexRequestTuning to call that helper, preserving the same behavior for
item types "message", "function_call" (prefix id with "fc_" when missing), and
"function_call_output" (ensure empty "output" exists) and keeping use of
gjson/sjson updates via byte slices; update imports and call sites accordingly
so both adapters reuse the centralized logic.
In `@internal/adapter/provider/codex/adapter.go`:
- Around line 583-587: In the function_call handling block (inside the if
itemType == "function_call" branch), always ensure the entry has an id that
starts with "fc_": check item.Get("id").String() and if it already exists but
doesn't start with "fc_" prefix, set it to "fc_"+id using sjson.SetBytes(body,
fmt.Sprintf("input.%d.id", i), ...); if the id is empty, generate a default
prefixed id (e.g., "fc_"+ a deterministic fallback such as fmt.Sprintf("%d", i)
or a UUID) and set that into body as well so no function_call is left without an
fc_ prefixed id.
---
Duplicate comments:
In `@internal/adapter/provider/cliproxyapi_codex/adapter.go`:
- Around line 265-268: The code currently only prefixes non-empty function_call
ids with "fc_" (in the block handling itemType == "function_call"), leaving
empty ids unchanged; update the logic in the adapter.go snippet so that when
itemType == "function_call" you always set input.%d.id to "fc_"+id (even if id
== ""), using sjson.SetBytes with fmt.Sprintf("input.%d.id", i), and only skip
when the id already has the "fc_" prefix (i.e., check strings.HasPrefix(id,
"fc_") before setting).
---
Nitpick comments:
In `@internal/adapter/provider/codex/adapter_test.go`:
- Line 17: The test currently builds `body` without covering the `function_call`
ID normalization (the `id != ""` branch that prepends "fc_") and the
`function_call_output` default `output` filling path; update the test in
adapter_test.go to include a `function_call` entry with a non-`fc_` id (e.g.,
`{"type":"function_call","id":"toolu_01","name":"t","arguments":"{}"}`) and a
`function_call_output` entry missing `output` (e.g.,
`{"type":"function_call_output","call_id":"toolu_01"}`), then after invoking the
code assert that the resulting parsed `input[N].id` starts with `"fc_"` and that
`input[M].output` exists and is the empty string (""), covering the `fc_`
prefixing logic and `output` defaulting logic.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
internal/adapter/provider/cliproxyapi_codex/adapter.gointernal/adapter/provider/codex/adapter.gointernal/adapter/provider/codex/adapter_test.gointernal/converter/claude_to_codex.go
📜 Review details
🧰 Additional context used
🧬 Code graph analysis (1)
internal/converter/claude_to_codex.go (1)
internal/converter/types_codex.go (2)
CodexInputItem(23-32)CodexOutput(57-66)
🔇 Additional comments (2)
internal/converter/claude_to_codex.go (2)
114-121: 无需采取行动
convertClaudeToolResultContentToString函数已在internal/converter/claude_to_openai_helpers.go中定义,在同一包内可访问,不存在编译问题。
204-211: 响应路径中CallID的fc_前缀处理逻辑已正确处理了所有情况代码逻辑(lines 204-210)正确地处理了两种上游数据格式:
- 当
block.ID = "call_1"时:callID = "call_1",itemID = "fc_call_1"✓- 当
block.ID = "fc_call_1"时:callID = "call_1",itemID = "fc_call_1"✓
callID和itemID在任何情况下都不会相同。该逻辑与 lines 95-103 的请求路径处理方式一致,确保了转换链路中的一致性。
| func sanitizeCodexPayload(body []byte) []byte { | ||
| if input := gjson.GetBytes(body, "input"); input.IsArray() { | ||
| for i, item := range input.Array() { | ||
| itemType := item.Get("type").String() | ||
| if itemType != "message" { | ||
| if item.Get("role").Exists() { | ||
| body, _ = sjson.DeleteBytes(body, fmt.Sprintf("input.%d.role", i)) | ||
| } | ||
| } | ||
| if itemType == "function_call" { | ||
| if id := item.Get("id").String(); id != "" && !strings.HasPrefix(id, "fc_") { | ||
| body, _ = sjson.SetBytes(body, fmt.Sprintf("input.%d.id", i), "fc_"+id) | ||
| } | ||
| } | ||
| if itemType == "function_call_output" { | ||
| if !item.Get("output").Exists() { | ||
| body, _ = sjson.SetBytes(body, fmt.Sprintf("input.%d.output", i), "") | ||
| } | ||
| } | ||
| } | ||
| } | ||
| return body | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
sanitizeCodexPayload 与 applyCodexRequestTuning 中的逻辑重复
sanitizeCodexPayload 的实现与 internal/adapter/provider/codex/adapter.go 中 applyCodexRequestTuning 的 input 规范化循环(Lines 575–594)逻辑完全相同。两处代码需同步维护,容易产生分歧。
建议将共用的 input 规范化逻辑提取到一个公共包(例如 internal/codexutil)或通过参数传递,由两个 adapter 复用。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@internal/adapter/provider/cliproxyapi_codex/adapter.go` around lines 256 -
278, The sanitizeCodexPayload implementation duplicates the input-normalization
loop also present in applyCodexRequestTuning; extract that shared loop into a
single exported helper (e.g., NormalizeCodexInput or NormalizeInputItems) in a
new internal package (suggest internal/codexutil) and replace the body of
sanitizeCodexPayload and the input-handling section of applyCodexRequestTuning
to call that helper, preserving the same behavior for item types "message",
"function_call" (prefix id with "fc_" when missing), and "function_call_output"
(ensure empty "output" exists) and keeping use of gjson/sjson updates via byte
slices; update imports and call sites accordingly so both adapters reuse the
centralized logic.
| if itemType == "function_call" { | ||
| if id := item.Get("id").String(); id != "" && !strings.HasPrefix(id, "fc_") { | ||
| body, _ = sjson.SetBytes(body, fmt.Sprintf("input.%d.id", i), "fc_"+id) | ||
| } | ||
| } |
There was a problem hiding this comment.
function_call 的空 id 未添加 fc_ 前缀
当 function_call 条目的 id 为空字符串时,id != "" 条件不成立,不会添加 fc_ 前缀,也不会生成任何 ID。Codex API 要求 function_call 的 id 必须以 fc_ 开头,空 ID 将导致上游返回验证错误。
如果空 id 是合法的上游输入,建议补充生成一个默认 fc_ ID:
🐛 建议修复
if itemType == "function_call" {
- if id := item.Get("id").String(); id != "" && !strings.HasPrefix(id, "fc_") {
- body, _ = sjson.SetBytes(body, fmt.Sprintf("input.%d.id", i), "fc_"+id)
+ id := item.Get("id").String()
+ if id == "" {
+ body, _ = sjson.SetBytes(body, fmt.Sprintf("input.%d.id", i), "fc_"+uuid.NewString())
+ } else if !strings.HasPrefix(id, "fc_") {
+ body, _ = sjson.SetBytes(body, fmt.Sprintf("input.%d.id", i), "fc_"+id)
}
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@internal/adapter/provider/codex/adapter.go` around lines 583 - 587, In the
function_call handling block (inside the if itemType == "function_call" branch),
always ensure the entry has an id that starts with "fc_": check
item.Get("id").String() and if it already exists but doesn't start with "fc_"
prefix, set it to "fc_"+id using sjson.SetBytes(body, fmt.Sprintf("input.%d.id",
i), ...); if the id is empty, generate a default prefixed id (e.g., "fc_"+ a
deterministic fallback such as fmt.Sprintf("%d", i) or a UUID) and set that into
body as well so no function_call is left without an fc_ prefixed id.
claude_to_codex.go
codex/adapter_test.go
Claude→Codex 中 function_call 的 role。
cliproxyapi_codex/adapter.go, internal/converter/claude_to_codex.go
adapter.go, internal/adapter/provider/cliproxyapi_codex/adapter.go
adapter.go, internal/adapter/provider/cliproxyapi_codex/adapter.go
Summary by CodeRabbit
发行说明
Bug 修复
改进