fix(claude): emit tool args before deferring stream close on finish_reason - #6046
fix(claude): emit tool args before deferring stream close on finish_reason#6046denghuinow wants to merge 1 commit into
Conversation
Walkthrough
ChangesClaude stream finish/usage fix
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant OpenAIStream
participant StreamResponseOpenAI2Claude
participant ClaudeStream
OpenAIStream->>StreamResponseOpenAI2Claude: tool_calls finish chunk without usage
StreamResponseOpenAI2Claude->>ClaudeStream: emit input_json_delta
StreamResponseOpenAI2Claude-->>ClaudeStream: defer final message_delta
OpenAIStream->>StreamResponseOpenAI2Claude: usage chunk
StreamResponseOpenAI2Claude->>ClaudeStream: emit final message_delta and stop
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 |
…eason When upstream sends finish_reason without usage in the same chunk, the converter returned early and dropped tool_calls argument deltas from that chunk, producing truncated input_json_delta streams. Co-authored-by: Cursor <cursoragent@cursor.com>
5251a1a to
26481b6
Compare
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 `@service/relayconvert/internal/oai_chat/to_claude_messages_resp.go`:
- Around line 241-263: In the first-chunk finalization branch of the response
conversion flow, defer closing until usage is available: when oaiUsage is nil,
return without calling stopOpenBlocks(), emitting message_stop, or setting
ClaudeConvertInfo.Done. Only perform finalization after usage is present,
matching the multi-chunk path, and add a regression test covering
SendResponseCount == 1 with a later usage-only chunk.
- Around line 402-425: Add an EOF fallback in the done-chunk handling around
`oaiUsage` so the direct `ConvertStream` path always emits the final Claude
`message_delta` and `message_stop` when no usage chunk arrives. Do not return
solely because `oaiUsage` is nil; emit the completion using a zero or otherwise
established fallback usage, while preserving the existing upstream usage
selection and `info.ClaudeConvertInfo.Done` behavior.
🪄 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
Run ID: 776fb58d-13a8-4bde-8f06-1a75fc10d049
📒 Files selected for processing (2)
service/relayconvert/internal/oai_chat/to_claude_messages_resp.goservice/relayconvert/internal/oai_chat/to_claude_messages_resp_test.go
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
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 `@service/relayconvert/internal/oai_chat/to_claude_messages_resp.go`:
- Around line 241-263: In the first-chunk finalization branch of the response
conversion flow, defer closing until usage is available: when oaiUsage is nil,
return without calling stopOpenBlocks(), emitting message_stop, or setting
ClaudeConvertInfo.Done. Only perform finalization after usage is present,
matching the multi-chunk path, and add a regression test covering
SendResponseCount == 1 with a later usage-only chunk.
- Around line 402-425: Add an EOF fallback in the done-chunk handling around
`oaiUsage` so the direct `ConvertStream` path always emits the final Claude
`message_delta` and `message_stop` when no usage chunk arrives. Do not return
solely because `oaiUsage` is nil; emit the completion using a zero or otherwise
established fallback usage, while preserving the existing upstream usage
selection and `info.ClaudeConvertInfo.Done` behavior.
🪄 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
Run ID: 776fb58d-13a8-4bde-8f06-1a75fc10d049
📒 Files selected for processing (2)
service/relayconvert/internal/oai_chat/to_claude_messages_resp.goservice/relayconvert/internal/oai_chat/to_claude_messages_resp_test.go
🛑 Comments failed to post (2)
service/relayconvert/internal/oai_chat/to_claude_messages_resp.go (2)
241-263: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash set -euo pipefail # Locate the file and inspect the relevant section with line numbers. file="service/relayconvert/internal/oai_chat/to_claude_messages_resp.go" wc -l "$file" sed -n '1,140p' "$file" printf '\n--- SNIP ---\n' sed -n '140,320p' "$file" printf '\n--- TEST SEARCH ---\n' rg -n "FinishReasonBeforeUsage|SendResponseCount == 1|message_stop|Done = true|Usage-only|usage-only" service/relayconvert/internal/oai_chat -nRepository: QuantumNous/new-api
Length of output: 13509
🏁 Script executed:
#!/bin/bash set -euo pipefail file="service/relayconvert/internal/oai_chat/to_claude_messages_resp.go" testfile="service/relayconvert/internal/oai_chat/to_claude_messages_resp_test.go" printf '%s\n' '--- FINALIZATION PATH AROUND LINES 392-430 ---' sed -n '392,430p' "$file" printf '\n%s\n' '--- USAGE ASSIGNMENTS ---' rg -n "ClaudeConvertInfo\.Usage|Usage =" "$file" "$testfile" service/relayconvert/internal/oai_chat -n printf '\n%s\n' '--- RELEVANT TESTS ---' sed -n '180,310p' "$testfile"Repository: QuantumNous/new-api
Length of output: 8610
🏁 Script executed:
#!/bin/bash set -euo pipefail file="service/relayconvert/internal/oai_chat/to_claude_messages_resp.go" printf '%s\n' '--- WHERE ClaudeConvertInfo.Usage IS UPDATED ---' rg -n "ClaudeConvertInfo\.Usage\s*=|GetOrCreate.*Usage|Usage\)" "$file" service/relayconvert/internal/oai_chat -n printf '\n%s\n' '--- BOTTOM OF FILE AROUND USAGE HANDLING ---' sed -n '430,475p' "$file"Repository: QuantumNous/new-api
Length of output: 7776
Defer first-chunk finalization until usage is available. When the very first chunk already has
finish_reasonbut no usage, this branch still emitsmessage_stopand setsDone, so a later usage-only chunk is dropped by the top-level guard and final billing data never reaches the client. Match the multi-chunk path here: ifoaiUsageis nil, return without closing; only callstopOpenBlocks()and setDoneonce usage is present. Add a regression test forSendResponseCount == 1.🤖 Prompt for 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. In `@service/relayconvert/internal/oai_chat/to_claude_messages_resp.go` around lines 241 - 263, In the first-chunk finalization branch of the response conversion flow, defer closing until usage is available: when oaiUsage is nil, return without calling stopOpenBlocks(), emitting message_stop, or setting ClaudeConvertInfo.Done. Only perform finalization after usage is present, matching the multi-chunk path, and add a regression test covering SendResponseCount == 1 with a later usage-only chunk.
402-425: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash # Find callers of StreamResponseOpenAI2Claude and any end-of-stream fallback logic. rg -n "StreamResponseOpenAI2Claude" --type=go -B3 -A20 -g '!*_test.go' # Check whether ClaudeConvertInfo.Usage is ever assigned. rg -n "ClaudeConvertInfo\.Usage" --type=go # Check whether upstream requests set stream_options.include_usage for OpenAI-compatible calls. rg -n "include_usage" --type=goRepository: QuantumNous/new-api
Length of output: 157
🏁 Script executed:
#!/bin/bash set -euo pipefail # Map the target file and inspect the relevant function. ast-grep outline service/relayconvert/internal/oai_chat/to_claude_messages_resp.go --view expanded || true sed -n '1,520p' service/relayconvert/internal/oai_chat/to_claude_messages_resp.go | nl -ba | sed -n '220,460p' # Find callers and any end-of-stream fallback logic. rg -n "StreamResponseOpenAI2Claude" --type=go -B3 -A20 -g '!*_test.go' . rg -n "stopOpenBlocks|message_stop|message_delta|Done = true|ClaudeConvertInfo" --type=go -B3 -A8 -g '!*_test.go' service/relayconvert/internal/oai_chat # Check whether ClaudeConvertInfo.Usage is assigned anywhere. rg -n "ClaudeConvertInfo\.Usage|\.Usage\s*=" --type=go service/relayconvert/internal/oai_chat # Check whether upstream requests set include_usage. rg -n "include_usage|stream_options" --type=go .Repository: QuantumNous/new-api
Length of output: 825
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "=== Relevant section of service/relayconvert/internal/oai_chat/to_claude_messages_resp.go ===" sed -n '220,460p' service/relayconvert/internal/oai_chat/to_claude_messages_resp.go echo echo "=== Callers of StreamResponseOpenAI2Claude and surrounding flow ===" rg -n "StreamResponseOpenAI2Claude" --type=go -B4 -A20 -g '!*_test.go' . echo echo "=== Any end-of-stream / forced-close fallback logic in the relayconvert path ===" rg -n "stopOpenBlocks|message_stop|message_delta|Done = true|Force.*close|synthetic.*message_stop|unterminated" --type=go -B4 -A10 service/relayconvert echo echo "=== Where ClaudeConvertInfo.Usage is assigned ===" rg -n "ClaudeConvertInfo\.Usage|\.Usage\s*=" --type=go service/relayconvert echo echo "=== Where include_usage / stream_options are set for OpenAI-compatible requests ===" rg -n "include_usage|stream_options" --type=go .Repository: QuantumNous/new-api
Length of output: 50375
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "=== Stream call site and surrounding EOF handling ===" sed -n '850,930p' service/relayconvert/response_registry.go rg -n "StreamResponseOpenAI2Claude|canonicalUsageFromResponse|EOF|end of stream|flush|final usage|usage-only" --type=go -B4 -A12 service/relayconvert echo echo "=== Exact assignments to ClaudeConvertInfo.Usage or related fields ===" rg -n "ClaudeConvertInfo\.Usage\s*=|Usage\s*=\s*info\.ClaudeConvertInfo\.Usage|Usage\s*:=\s*.*ClaudeConvertInfo" --type=go service echo echo "=== OpenAI stream request construction and include_usage ===" rg -n "include_usage|stream_options|StreamOptions|IncludeUsage" --type=go serviceRepository: QuantumNous/new-api
Length of output: 46563
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "=== Request-side stream options for OpenAI chat upstreams ===" rg -n "IncludeUsage|include_usage|StreamOptions" --type=go service/relayconvert service echo echo "=== Stream finalization / EOF fallback in the response pipeline ===" rg -n "Finalize.*Stream|finalize.*Stream|rememberUsage|Done = true|message_stop|message_delta" --type=go -B4 -A12 service/relayconvert/response_registry.go service/relayconvert/internal/oai_chat service/relayconvert/internal/oai_responses echo echo "=== ClaudeConvertInfo definition and any usage field writes outside this file ===" rg -n "type ClaudeConvertInfo|Usage\s+\*dto\.Usage|ClaudeConvertInfo\.Usage" --type=go serviceRepository: QuantumNous/new-api
Length of output: 50375
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "=== Route registration for OpenAI chat -> Claude messages stream ===" rg -n "OpenAI.*Claude|Claude.*OpenAI|FinalizeStream:|ConvertStream:" --type=go service/relayconvert/response_registry.go service/relayconvert -B4 -A10 echo echo "=== ClaudeConvertInfo usage field definition and writes ===" rg -n "type ClaudeConvertInfo|ClaudeConvertInfo\.Usage|Usage\s+\*dto\.Usage" --type=go service/relayconvert/internal service/relayconvert/response_registry.go -B3 -A6 echo echo "=== Finalizer entrypoints used by response_registry ===" sed -n '324,360p' service/relayconvert/response_registry.goRepository: QuantumNous/new-api
Length of output: 50376
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "=== Stream execution path for direct ConvertStream routes ===" sed -n '470,620p' service/relayconvert/response_registry.go echo echo "=== OpenAI chat -> Claude messages route definition ===" sed -n '48,80p' service/relayconvert/text_converter_registry.go echo echo "=== OpenAI chat request converter (to see whether include_usage is set) ===" sed -n '1,220p' service/relayconvert/internal/oai_chat/to_claude_messages_req.goRepository: QuantumNous/new-api
Length of output: 13029
Add an EOF fallback for the Claude stream. This direct
ConvertStreamroute has no finalizer, andOpenAIChatRequestToClaudeMessagesdoesn’t setinclude_usage, so a provider that ends without a usage chunk will return from theoaiUsage == nilbranch without ever sendingmessage_delta/message_stop.info.ClaudeConvertInfo.Usagealso isn’t populated in this path, so the fallback here won’t fire.🤖 Prompt for 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. In `@service/relayconvert/internal/oai_chat/to_claude_messages_resp.go` around lines 402 - 425, Add an EOF fallback in the done-chunk handling around `oaiUsage` so the direct `ConvertStream` path always emits the final Claude `message_delta` and `message_stop` when no usage chunk arrives. Do not return solely because `oaiUsage` is nil; emit the completion using a zero or otherwise established fallback usage, while preserving the existing upstream usage selection and `info.ClaudeConvertInfo.Done` behavior.
移植自上游 PR QuantumNous#6046。OpenAI→Claude 流式转换在 finish chunk 无 usage 时提前 return 等待 usage-only chunk,但该 early-return 位于 delta 处理之前,finish chunk 里携带的工具参数增量被整块丢弃, 工具 JSON 截断(GLM 等「先 finish 后 usage」上游 + Claude Code 场景必现)。把等待逻辑移到 delta 处理之后的收尾块里,并补两个 流式回归测试(增量完整性 + 拼接后为合法 JSON)。 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Summary
input_json_deltastreams when converting OpenAI-compatible upstream tool-call SSE to Claude Messages format.finish_reasonwithoutusagein the same chunk, the converter now still emits anytool_callsargument deltas from that chunk before deferringmessage_stop.finish_reason→ trailingusagechunk sequence.Related issues
returnonfinish_reasonwithoutusagedrops the lasttool_callsargument delta). Causes 1–2 in that issue (duplicatecontent_block_start, pointer aliasing) are not covered by this PR.Problem
Some OpenAI-compatible upstreams (e.g. GLM) emit streaming chunks in this order:
tool_calls[].function.argumentsfragmentsfinish_reason: tool_calls(often containing the final argument fragment, withoutusage)Previously, step 2 triggered an early return before processing
tool_callsin the same chunk, so the finalpartial_jsonfragments were dropped and clients saw invalid JSON like:{"command": "ls -la /root 2>/dev/null | head -20", "description": "列出 /root 目录下的文件(前20行)Test plan
go test ./service/ -run TestStreamResponseOpenAI2ClaudeManual reproduction (20-run loop)
Set
BASEto your new-api endpoint,KEYto a valid API key, andMODELto an OpenAI-compatible upstream that supports tool calls (e.g.glm-5.2).Expected after fix:
summary: ok=20 fail=0Typical failure before fix: intermittent
Unterminated string/Expecting ',' delimitererrors, e.g.summary: ok=9 fail=11Made with Cursor
Summary by CodeRabbit