fix: prevent duplicate tool calls in Responses-to-Chat streaming - #6225
Conversation
When a function call was already registered under its output_index key via response.output_item.added, the synthetic events built from the terminal response.completed output carry no output_index and resolve to a different item-based key. ensureToolForEvent then created a second tool index and resent the full arguments, so Chat Completions clients received the same tool call twice. Reuse the tool registered under itemIDToKey/callIDToKey before creating a new one, and alias the new key to the existing tool. Co-Authored-By: Claude Fable 5 <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 Run ID: 📒 Files selected for processing (2)
WalkthroughThe streaming Responses-to-Chat converter now reuses existing tool state when terminal events use a different key format. A unit test verifies that repeated terminal tool data produces one tool index, the expected arguments, and a ChangesResponses-to-Chat tool deduplication
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
…ntumNous#6225) When a function call was already registered under its output_index key via response.output_item.added, the synthetic events built from the terminal response.completed output carry no output_index and resolve to a different item-based key. ensureToolForEvent then created a second tool index and resent the full arguments, so Chat Completions clients received the same tool call twice. Reuse the tool registered under itemIDToKey/callIDToKey before creating a new one, and alias the new key to the existing tool. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…ntumNous#6225) When a function call was already registered under its output_index key via response.output_item.added, the synthetic events built from the terminal response.completed output carry no output_index and resolve to a different item-based key. ensureToolForEvent then created a second tool index and resent the full arguments, so Chat Completions clients received the same tool call twice. Reuse the tool registered under itemIDToKey/callIDToKey before creating a new one, and alias the new key to the existing tool. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Synced upstream/main (8739c05 → 5a6c53d, 49 commits) into custom. Conflicts resolved by keeping custom side per user instruction: - controller/user.go: kept custom subscription filter + total_topup sort - model/user.go: kept custom queryUsers delegation (GetAllUsers/GetAllUsersFiltered/SearchUsers) - model/channel_cache.go: kept custom performance monitoring (RecordCacheSync) - web/default/src/features/users/components/users-columns.tsx: kept custom imports (formatNumber, cn) - web/default/src/features/users/components/users-table.tsx: kept custom controlled sorting + server-side ordering + subscriptionFilter Removed upstream test model/user_pagination_test.go — depends on the upstream sortOptions-based GetAllUsers/SearchUsers signatures that custom side does not expose (custom uses queryUsers with subStatus/orderBy/orderDir). Verified: go build ./... and web/default tsc --noEmit both pass. Notable upstream fixes absorbed: - QuantumNous#6225 prevent duplicate tool calls in Responses→Chat streaming - QuantumNous#6194 server-side sorting for user list (fix paged-data client-sort bug) - QuantumNous#6134 prevent large quota values from overflowing - QuantumNous#6168 purge authentication data on hard user deletion - QuantumNous#6096 golang.org/x/crypto 0.51.0 → 0.52.0 - Deadlock fix in channel_cache.go (InvalidatePricingCache lock ordering) NOT absorbed — custom side keeps performance monitoring branch. Manual review needed if pricing cache staleness is observed.
Sync 14 upstream commits. The dominant change is structural: upstream promoted the frontend from web/default to the web/ root and deleted the classic theme entirely, so most of the diff is renames and deletions. Notable upstream work: - refactor(auth): stateless dashboard tokens replacing sessions (QuantumNous#6329) - feat(channel): upstream model discovery for Codex and advanced custom channels (QuantumNous#6184, QuantumNous#5971) - fix: CAS status update prevents duplicate suno task refunds (QuantumNous#6074) - fix: no duplicate tool calls in Responses-to-Chat streaming (QuantumNous#6225) Fork-side resolutions: - Drop the classic theme, following upstream. electron/ and the release/electron-build workflows stay deleted as this fork already removed them; GHCR publishing continues via docker-build.yml. - UserBase keeps the fork's per-user Ratio alongside upstream's new Role, AuthVersion and CacheSchema fields. GetUserCache adopts upstream's cache-population path, which returns ToBaseUser() and so still carries Ratio. - web-router keeps the fork's dynamic index injector (SystemName/Logo templating) on top of upstream's renamed frontendFS. serveIndex collapses to the single-frontend WebAssets now that classic is gone. - Restore the fork's invoices feature (5 files), which git's rename detection dropped during the web/default -> web/ move, and re-register its route in routeTree.gen.ts. Backend builds and the full Go test suite passes. The frontend is not yet type-checked or built locally. Generated with [Claude Code](https://claude.ai/code) via [Happy](https://happy.engineering) Co-Authored-By: Claude <noreply@anthropic.com> Co-Authored-By: Happy <yesreply@happy.engineering>
…ntumNous#6225) When a function call was already registered under its output_index key via response.output_item.added, the synthetic events built from the terminal response.completed output carry no output_index and resolve to a different item-based key. ensureToolForEvent then created a second tool index and resent the full arguments, so Chat Completions clients received the same tool call twice. Reuse the tool registered under itemIDToKey/callIDToKey before creating a new one, and alias the new key to the existing tool. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…ntumNous#6225) When a function call was already registered under its output_index key via response.output_item.added, the synthetic events built from the terminal response.completed output carry no output_index and resolve to a different item-based key. ensureToolForEvent then created a second tool index and resent the full arguments, so Chat Completions clients received the same tool call twice. Reuse the tool registered under itemIDToKey/callIDToKey before creating a new one, and alias the new key to the existing tool. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
📝 变更描述 / Description
Responses-to-Chat 流式转换里,工具调用的 key 有两种形态:流式事件带
output_index,注册成output:<n>;response.completed的完整output在terminalOutputChunks里被构造成没有output_index的合成事件,算出来是item:<id>。ensureToolForEvent之前只查toolByKey,key 对不上就直接建新 tool——于是同一个 function call 拿到第二个tool_callsindex,完整 arguments 被重发一遍。对自动执行工具的 Agent 客户端,这等于同一个工具被调用两次。而 OpenAI 官方的
response.completed本来就携带完整output(含已流式发送过的 function_call item),这是必须正确处理的正常事件形态。修复:
ensureToolForEvent在直接 key 未命中时,先用已登记的itemIDToKey、callIDToKey反查已存在的 tool,找到就把新 key alias 过去;全部未命中才创建新 tool。这和同文件里非流式路径ResponsesBufferedAccumulator.findToolIndex的既有语义一致(它本来就会按 item ID 反查再新建)。terminal-only 工具调用(部分兼容上游只在终止事件里给出工具调用)不受影响,反查不到时仍走原有新建逻辑,对应的既有测试保持通过。🚀 变更类型 / Type of change
🔗 关联任务 / Related Issue
✅ 提交前检查项 / Checklist
Bug fix,我已提交或关联对应 Issue,且不会将设计取舍、预期不一致或理解偏差直接归类为 bug。📸 运行证明 / Proof of Work
新增回归测试
TestResponsesStreamEventToChatChunksDoesNotResendToolOnTerminalOutput(事件序列:output_item.added->function_call_arguments.delta-> 携带完整 output 的response.completed),修复前失败、修复后通过。修复前(main @ a63364d):
修复后:
既有测试全部通过,terminal-only 工具调用场景(
TestResponsesStreamEventToChatChunksUsesTerminalDoneOutput)不受影响。另已本地验证:多个不同工具仍分配不同 index;流式只发送部分 arguments、终止事件携带完整 arguments 时,只在同一 index 下补发差量。本 PR 由 AI 辅助完成(AI-assisted),我已逐行审阅并在本地验证。
🤖 Generated with Claude Code
Summary by CodeRabbit