fix(transformer): preserve item_reference for function_call_output in OpenAI Responses - #106
Merged
Conversation
… OpenAI Responses Fixes #101, #103 Root cause: When Chat Completions clients (codex, cherrystudio) call OpenAI Responses upstream channels with tool calls, the chat→responses conversion loses critical fields: - function_call items lack unique IDs - function_call_output items lack item_reference (required by Responses HTTP API) The upstream rejects with: "function_call_output requires item_reference ids matching each call_id on HTTP requests" Changes: 1. Add ItemReference field to ResponsesItem struct (both inbound and outbound) 2. Generate unique IDs for function_call items using crypto/rand (thread-safe) 3. Build call_id → item_id mapping in convertInputFromMessages 4. Set ItemReference in convertToolMessageToResponses using the mapping 5. Enhanced sanitizeResponsesRawItems to auto-populate missing/null/empty item_reference 6. Add fallback using atomic counter if crypto/rand fails 7. Add 4 tests including null/empty value handling Security fixes (from code review): - Replaced LCG PRNG with crypto/rand to avoid data race on global state - Handle null and empty string item_reference values in sanitizer - Ensure inbound struct preserves item_reference in typed passthrough Coverage: - Fixes chat→responses conversion for tool calls (#101, #103) - Preserves item_reference in raw items passthrough (native Responses clients) - Thread-safe with -race detector validation - All existing tests pass + 4 new tests Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds Changesfunction_call_output item_reference linkage
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
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 |
![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
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/transformer/outbound/openai/response_function_call_test.go`:
- Around line 174-186: The test for MarshalResponsesInputItems is only checking
that function_call_output contains an item_reference field, which can miss a
wrong linkage. Tighten the assertions in response_function_call_test by
capturing both the function_call and function_call_output entries from the
marshaled items, then compare item_reference directly against the originating
function_call.id. Use the existing MarshalResponsesInputItems flow and the
item["type"] handling to locate the right assertions, and verify the IDs match
exactly rather than just checking presence.
In `@internal/transformer/outbound/openai/response.go`:
- Around line 1688-1718: The backfill logic in the function that builds the
call_id-to-item_id map and sanitizes function_call_output items is skipping raw
function_call entries that have a call_id but no id, which leaves item_reference
unset later. Update the mapping step to generate or assign an id for raw
function_call items before populating callIDToItemID, so the later
function_call_output backfill can always resolve item_reference correctly. Keep
the fix localized around the existing item iteration logic and the
callIDToItemID lookup used by the sanitization pass.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: b57420a1-5c94-4be0-ace2-e5a9ce46b020
📒 Files selected for processing (3)
internal/transformer/inbound/openai/response.gointernal/transformer/outbound/openai/response.gointernal/transformer/outbound/openai/response_function_call_test.go
…and tighten item_reference tests sanitizeResponsesRawItems now generates an id for raw function_call entries that have call_id but no id, so the function_call_output backfill always resolves item_reference. Also strengthens TestMarshalResponsesInputItemsPreservesItemReference to assert the reference matches the originating function_call.id rather than just checking presence. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Hureru
added a commit
that referenced
this pull request
Jun 30, 2026
…#92) (#107) * feat(relay): implement HTTP replay mechanism for OpenAI Responses API (#92) 实现本地 HTTP replay 机制，解决上游中转站拒绝 previous_response_id 的兼容性问题。 ## 背景 当 OpenAI Responses HTTP 请求携带 previous_response_id 时，部分上游中转站返回： previous_response_id is only supported on Responses WebSocket v2 同时这些上游的 WebSocket 升级路径不可用（426/101 handshake 失败）。 ## 实现 1. **本地状态存储** (responses_replay_store.go) - 使用 sync.Map 作为进程内存储，key 格式：apiKeyID:groupID:requestModel:hash(responseID) - 每次 HTTP /v1/responses 成功后保存 replay 状态（渠道、key、replay window） - 支持 TTL 自动过期，默认继承 group.SessionKeepTime 2. **请求转换** (relay.go) - 检测到 previous_response_id 时，从本地加载上一次状态 - 调用 BuildReplayRequest 转为自包含形式（合并历史 + 移除 previous_response_id） - 优先复用上一次成功的渠道/key（通过 NewIteratorWithPreference） 3. **可观测性** - 记录 ws_mode=replay 和 ws_recovery=replay 便于排障 - Debug 日志记录状态加载、转换、路由、保存全流程 ## 兼容性 - 与 PR #106 的 item_reference 修复兼容（MarshalResponsesInputItems 现在正确生成 id 和 item_reference） - 复用现有 wsConversationState 和 replay 逻辑，无需数据库迁移 - 不影响 WebSocket 原生续接和 passthrough 模式 ## 测试覆盖 - 单元测试：状态存储、加载、TTL、隔离性 - 集成测试：完整 replay 流程、流式请求、不同 group 隔离、tool calls * fix(relay): address critical HTTP replay issues from code review 修复代码审查中发现的关键问题： ## High Priority 修复 1. **多轮 replay 状态持久化** - 移除 !IsOpenAIExactReplayRequest() 的保存限制 - exact replay 成功后继续保存新状态（基于已有状态累积） - 支持连续多轮 HTTP replay (resp1 -> resp2 -> resp3) 2. **容量控制与主动清理** - 增加最大条目数限制 (10,000) - 增加最大内存限制 (100MB) - 后台定时清理过期条目 (5分钟间隔) - 状态大小估算与容量检查 - 增加统计指标 (entries, totalSize) 3. **历史合并失败回退** - 验证 BuildReplayRequest 是否成功生成 RawInputItems - 合并失败时保留原始 previous_response_id - 放弃本地 replay，允许回退到原生续接 - 增加 warn 日志记录失败原因 ## Medium Priority 改进 4. **增强错误日志** - GetInternalResponse 失败时记录 debug 日志 - 历史合并失败记录 warn 日志 - 容量超限记录 warn 日志 5. **降低碰撞风险** - SHA256 hash 截断从 16 hex (64-bit) 增加到 32 hex (128-bit) ## 测试覆盖 6. **新增测试** - TestHTTPReplayMultiTurnChain: 验证连续 3 轮 replay - TestHTTPReplayFailedMergeKeepsOriginalRequest: 验证合并失败时的回退 ## 影响范围 - 修复后支持稳定的多轮 HTTP replay continuation - 防止内存泄漏和无限增长 - 提升可观测性和故障排查能力 * fix(relay): address second-round code review issues 修复第二轮代码审查中发现的所有关键问题。 ## High Priority 修复 1. **流式响应保存问题** (High 1) - 优先使用 `metrics.InternalResponse`（已由 collectResponse 填充） - 避免二次调用 `GetInternalResponse()` 消耗 streaming 聚合器 - 确保流式请求的 replay state 能正确保存 2. **统计一致性问题** (High 2) - 使用 `Swap()` 原子操作保证统计与 map 一致 - 所有 Delete 路径统一更新 entries/totalSize - `resetResponsesReplayStore()` 同时重置统计 - 容量检查在 Swap 后执行，允许更新已有 key ## Medium Priority 改进 3. **容量检查逻辑** (Medium 1) - 更新已有 key 时只计算 size delta，不检查 entries 上限 - 新 key 才检查容量，超限时回滚 4. **测试改进** (Medium 4) - 修复 `TestHTTPReplayFailedMergeKeepsOriginalRequest` - 使用真正会失败的场景（空 Messages + 空 RawInputItems） - 验证 relay.go 的 fallback 校验逻辑 5. **代码格式** (Low 1) - 运行 gofmt 格式化 responses_replay_store.go ## 技术细节 ### 统计一致性实现 ```go old, loaded := responsesReplayStore.Swap(key, newEntry) if loaded { // 更新：只调整 size 差值 responsesReplayStoreStats.totalSize.Add(delta) } else { // 新增：增加 entries，检查容量，超限则回滚 currentEntries := responsesReplayStoreStats.entries.Add(1) if overflow { Delete + rollback stats } } ``` ### 流式安全保存 ```go // 优先使用已收集的响应（streaming 安全） internalResponse := metrics.InternalResponse if internalResponse == nil { // fallback 到 GetInternalResponse } ``` ## 测试结果 - ✅ 全部 replay 测试通过 - ✅ go test -race 无数据竞争（replay 部分） - ✅ gofmt 检查通过 * fix(relay): use CompareAndDelete for replay store to prevent concurrent deletion race 修复第三轮审查发现的并发安全问题。 ## 问题 在高并发场景下，replay store 的 Delete 操作可能误删并发写入的新 entry： 1. **loadResponsesReplayState()**: goroutine A 判断 entry 过期，准备 Delete(key) 2. **同时**: goroutine B 对同一 key Swap 进新的有效 entry 3. **误删**: goroutine A 执行 Delete(key)，误删了 B 刚写入的新 entry 同样的问题存在于： - sweepExpiredResponsesReplayStates() 删除过期 entry - storeResponsesReplayState() 容量回滚删除 ## 修复 使用 `sync.Map.CompareAndDelete(key, oldValue)` 替代 `Delete(key)`： - 只删除我们检查过的那个 entry - 如果 key 的 value 已被并发修改，删除失败，避免误删 - 统计更新仅在删除成功后执行 ## 代码位置 - loadResponsesReplayState(): L106-108, L112-115 - sweepExpiredResponsesReplayStates(): L63-66, L70-74 - storeResponsesReplayState(): L168-171 ## 技术细节 ```go // Before (有竞态) if entry.expiresAt.After(now) { responsesReplayStore.Delete(key) // 可能误删并发写入的新 entry responsesReplayStoreStats.entries.Add(-1) } // After (并发安全) if entry.expiresAt.After(now) { if responsesReplayStore.CompareAndDelete(key, entry) { // 只删除这个 entry responsesReplayStoreStats.entries.Add(-1) // 仅删除成功时更新 } } ``` * fix(relay): harden replay store capacity check, size estimation, and test coverage - Add capacity rollback when Swap replacement exceeds size limit - Replace fixed-multiplier estimateStateSize with per-field traversal - Fix RawResponsesOutputItems to use function_call instead of function_call_output - Assert multi-turn chain preserves all turns in replayed RawInputItems - Strengthen clone isolation test with nested structure mutations
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #101
Problem
When Chat Completions clients (codex, cherrystudio) call OpenAI Responses upstream channels with tool calls, requests fail with:
Root Cause
The chat→responses protocol conversion was missing two required fields:
function_callitems lack uniqueidfieldfunction_call_outputitems lackitem_referencefield (must point to corresponding function_call's id)Changes
ItemReferencefield toResponsesItemstruct (both inbound and outbound)crypto/rand(thread-safe)convertInputFromMessagesItemReferenceinconvertToolMessageToResponsessanitizeResponsesRawItemsto auto-populate missing/null/empty item_referenceSecurity Fixes (from code review)
Testing
-racedetector validation passedFiles Changed
internal/transformer/outbound/openai/response.gointernal/transformer/inbound/openai/response.gointernal/transformer/outbound/openai/response_function_call_test.go(new)Summary by CodeRabbit
New Features
function_call_outputitems keep a link back to their originatingfunction_call.Bug Fixes
function_callIDs and backfills/correctsitem_referencevalues when they’re absent, empty, or null.Tests