fix: preserve parallel Claude tool block lifecycle in OpenAI→Claude stream conversion - #6394
Open
qianqiu-2020 wants to merge 1 commit into
Open
Conversation
Contributor
|
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 (3)
🚧 Files skipped from review as they are similar to previous changes (3)
WalkthroughOpenAI-to-Claude streaming conversion now tracks multiple concurrently open tool-use blocks by index, guards delta and stop events, resets tracking between tool phases, and tests valid block lifecycles. ChangesParallel tool-call lifecycle
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant OpenAIStream
participant StreamResponseOpenAI2Claude
participant ClaudeContentBlocks
OpenAIStream->>StreamResponseOpenAI2Claude: tool-call deltas by index
StreamResponseOpenAI2Claude->>ClaudeContentBlocks: start one tool_use block per index
StreamResponseOpenAI2Claude->>ClaudeContentBlocks: emit input_json_delta for open indexes
StreamResponseOpenAI2Claude->>ClaudeContentBlocks: stop started tool_use blocks
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
service/convert_stream_test.go (1)
12-62: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for the actual ghost-delta scenario from
#4389.This test validates the happy-path lifecycle well, but never exercises the new guard's negative branch — i.e., a delta/stop arriving for a block index that's already closed or never opened (the packed-final-chunk case the source comment explicitly calls out). Since that's the concrete bug being fixed, a case asserting such stray events are dropped (or don't panic/emit invalid events) would directly protect the regression path.
Based on path instructions,
**/*_test.goguidelines require tests to "protect real behavior, API contracts, billing/accounting invariants, compatibility, or regression paths" — this is the regression path the PR title cites but leaves partially untested.🤖 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/convert_stream_test.go` around lines 12 - 62, Add a regression case in TestStreamResponseOpenAI2ClaudeParallelToolCallsHaveValidBlockLifecycle that sends a packed final chunk or otherwise produces a delta/stop for a tool-call index that was never opened or was already closed. Assert StreamResponseOpenAI2Claude drops the stray events without panicking and emits no invalid lifecycle events, while preserving the existing valid start/delta/stop assertions.Source: Path instructions
service/relayconvert/internal/oai_chat/to_claude_messages_resp.go (1)
155-180: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider unifying the two tool-block start/delta loops.
Both loops implement the same start-block → mark-open → guarded-delta pattern, but Loop A (init chunk) relies on an implicit invariant (block is always started here, so the open-check is skipped) while Loop B (streaming chunk) makes the same check explicit because its start is conditional. This works today, but the divergence is exactly the kind of subtle asymmetry that could reintroduce the ghost-delta bug (
#4389) if one loop is edited without the other.Extracting a shared helper (e.g.
startToolBlockIfNeeded/emitToolDeltaIfOpen) that both call sites use would centralize the open-index invariant and make future edits safer. Two call sites justify a package-level helper here.Also applies to: 305-357
🤖 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 155 - 180, Unify the tool-block start and guarded-delta behavior used by the initialization and streaming loops. Extract package-level helpers such as startToolBlockIfNeeded and emitToolDeltaIfOpen, then update both loop paths around ClaudeConvertInfo.ToolCallOpenIndexes and claudeResponses to use them, ensuring deltas are emitted only for open blocks and preserving existing index handling.
🤖 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.
Nitpick comments:
In `@service/convert_stream_test.go`:
- Around line 12-62: Add a regression case in
TestStreamResponseOpenAI2ClaudeParallelToolCallsHaveValidBlockLifecycle that
sends a packed final chunk or otherwise produces a delta/stop for a tool-call
index that was never opened or was already closed. Assert
StreamResponseOpenAI2Claude drops the stray events without panicking and emits
no invalid lifecycle events, while preserving the existing valid
start/delta/stop assertions.
In `@service/relayconvert/internal/oai_chat/to_claude_messages_resp.go`:
- Around line 155-180: Unify the tool-block start and guarded-delta behavior
used by the initialization and streaming loops. Extract package-level helpers
such as startToolBlockIfNeeded and emitToolDeltaIfOpen, then update both loop
paths around ClaudeConvertInfo.ToolCallOpenIndexes and claudeResponses to use
them, ensuring deltas are emitted only for open blocks and preserving existing
index handling.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 76c21e3a-2e45-4688-b678-2a353875fc18
📒 Files selected for processing (3)
relay/common/relay_info.goservice/convert_stream_test.goservice/relayconvert/internal/oai_chat/to_claude_messages_resp.go
This was referenced Jul 22, 2026
qianqiu-2020
force-pushed
the
fix/claude-parallel-tool-block-lifecycle
branch
from
July 28, 2026 02:54
3c25052 to
9fb4eeb
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@relaykit/relayconvert/internal/oai_chat/to_claude_messages_resp_test.go`:
- Around line 256-277: Extend the event validation in the test’s event loop to
capture each content_block_delta event’s PartialJson payload by block index.
Assert that the collected values include {"city":"Tokyo"} for the expected block
and {} for the other, while preserving the existing ordering and start/stop
assertions.
In `@relaykit/relayconvert/internal/oai_chat/to_claude_messages_resp.go`:
- Around line 177-187: Update both content_block_start emission paths in
relaykit/relayconvert/internal/oai_chat/to_claude_messages_resp.go at lines
177-187 and 349-360 to require a non-empty Function.Name and an index not
already present in ToolCallOpenIndexes before emitting a start; preserve the
existing state update after emission. In
relaykit/relayconvert/internal/oai_chat/to_claude_messages_resp_test.go lines
229-277, add a replayed name for an existing index and assert that no duplicate
content_block_start is produced.
- Around line 28-35: Replace the range scan in
relaykit/relayconvert/internal/oai_chat/to_claude_messages_resp.go:28-35 with
deterministic iteration over the tracked open indexes, stopping only active
keys. In
relaykit/relayconvert/internal/oai_chat/to_claude_messages_resp.go:168-187,
validate or normalize negative and unbounded initial tool indexes before
emitting Claude blocks; apply the same validation to subsequent tool deltas in
relaykit/relayconvert/internal/oai_chat/to_claude_messages_resp.go:336-377.
🪄 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: b9cd67b4-9535-47c0-8a0e-bd58342090c3
📒 Files selected for processing (3)
relaykit/relayconvert/convmeta/meta.gorelaykit/relayconvert/internal/oai_chat/to_claude_messages_resp.gorelaykit/relayconvert/internal/oai_chat/to_claude_messages_resp_test.go
qianqiu-2020
force-pushed
the
fix/claude-parallel-tool-block-lifecycle
branch
from
July 28, 2026 03:11
9fb4eeb to
d9de5e7
Compare
Contributor
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 `@relaykit/relayconvert/internal/oai_chat/to_claude_messages_resp_test.go`:
- Around line 324-333: Update the event validation in the test around the
content_block_start loop to collect every content_block_start event’s index,
including non-zero indexes, and assert the collected indexes exactly equal [0].
Remove the current non-zero-index filtering and starts counter while preserving
the existing replay scenario coverage.
- Around line 276-283: Strengthen the event lifecycle assertions in this test’s
content_block_stop handling to reject duplicate stops for the same block. Before
marking stopped[idx], assert that the block has not already been stopped, while
preserving the existing started-before-stop check and final started/stopped
equality assertion.
🪄 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: 7ddb2390-f82d-4cd4-924c-5ae90439c379
📒 Files selected for processing (3)
relaykit/relayconvert/convmeta/meta.gorelaykit/relayconvert/internal/oai_chat/to_claude_messages_resp.gorelaykit/relayconvert/internal/oai_chat/to_claude_messages_resp_test.go
🚧 Files skipped from review as they are similar to previous changes (2)
- relaykit/relayconvert/convmeta/meta.go
- relaykit/relayconvert/internal/oai_chat/to_claude_messages_resp.go
qianqiu-2020
force-pushed
the
fix/claude-parallel-tool-block-lifecycle
branch
from
July 28, 2026 03:25
d9de5e7 to
50b4ad8
Compare
11 tasks
Author
|
@Calcium-Ion Please help me review this PR. |
neimaravila
pushed a commit
to neimaravila/new-api
that referenced
this pull request
Aug 6, 2026
DeepSeek V4 breaks Claude Code two ways through the OpenAI->Claude stream conversion: the sglang dsv4 parser emits a trailing content='\n' after tool_calls, which puts a text block after tool_use and trips "Content block not found"; and a reasoning run that spends the whole max_tokens budget on thinking closes with no non-thinking content block at all, which Anthropic's streaming protocol does not allow. PR QuantumNous#6629 fixes both. It collides with PR QuantumNous#6394, which we already carry, in two places: the ClaudeConvertInfo struct tail, where both append a field, and the tool-call branch, where QuantumNous#6394 inserted the ToolCallOpenIndexes initialisation the new HasContentBlock assignment was anchored to. Kept both fields and re-anchored the assignment; the rest is verbatim. The fixes do not overlap in behaviour, since appendEmptyTextFallback only runs when no text or tool_use block was ever emitted. Verified over production with scripts/apply-patches.sh: all fourteen apply in order, relaykit builds with GOWORK=off, and both PRs' tests pass together -- ParallelToolCallsHaveValidBlockLifecycle and ReplayedToolNameDoesNotDuplicate Start from QuantumNous#6394 alongside DiscardsTrailingTextAfterToolUse, AppendsEmptyTextForThinkingOnlyStream and ThinkingThenTextDoesNotGetFallback from QuantumNous#6629. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018ZsKS6An5YHvZTW3cpVTNX
10 tasks
11 tasks
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.
Important
📝 变更描述 / Description
在 OpenAI 流式响应转 Claude SSE（
StreamResponseOpenAI2Claude）时，并行 tool_use 路径会产生 "ghost"content_block_delta——发往一个从未content_block_start或已被content_block_stop关闭的 block index。新版 Claude Code（Opus 4.7 捆绑版本起）对 SSE 状态机做严格校验，遇到这种 delta 直接报API Error: Content block not found。我用 GLM-5.2 复现了该问题：并行工具调用时，GLM-5.2 把多个工具参数打包在最后一个 chunk 一次性返回（而非逐个流式 delta），转换器就会向已关闭/未 start 的 block index 发 delta，触发
Content block not found。根因有三处：
input_json_delta守卫只判断len(Arguments) > 0，未校验目标 block 是否仍 open。当上游在最后一个 chunk 把多个工具参数打包返回、且此前某 block 已被stopOpenBlocks关闭时，delta 落到已关闭的 index 上。stopOpenBlocks对base..base+maxOffset的每个 offset 无条件发content_block_stop，但 offset 可能不连续或部分 index 从未 start，产生 stop-only 幽灵 block。SendResponseCount == 1）只取ToolCalls[0]单独建 block 0，忽略并行工具，导致后续 chunk 的 offset≥1 工具在 base=0 上发 delta 却没有对应 start。修复方式：给
ClaudeConvertInfo增加ToolCallOpenIndexes map[int]bool，精确记录哪些 block index 当前 open：content_block_start时置ToolCallOpenIndexes[idx] = truestopOpenBlocks只对ToolCallOpenIndexes[blockIndex]为 true 的 index 发content_block_stopstopOpenBlocksAndAdvance清空ToolCallOpenIndexesinput_json_delta守卫改为len(Arguments) > 0 && ToolCallOpenIndexes[idx]ToolCalls，对每个工具建独立content_block_start并登记 open这是与 sub2api #4193（PR #4294）同型的缺陷：其
resToAnthHandleFuncArgsDone直接用state.ContentBlockIndex向已关闭 block 发 delta，而Delta路径用OutputIndexToBlockIdx。两项目独立实现、同根因，本 PR 用 "open-index 登记" 在 OpenAI→Claude 方向堵住同类问题。🚀 变更类型 / Type of change
🔗 关联任务 / Related Issue
✅ 提交前检查项 / Checklist
Bug fix，我已提交或关联对应 Issue，且不会将设计取舍、预期不一致或理解偏差直接归类为 bug。📸 运行证明 / Proof of Work
Result:
新增回归测试
TestStreamResponseOpenAI2ClaudeParallelToolCallsHaveValidBlockLifecycle断言：每个content_block_delta之前必有content_block_start、之后未被content_block_stop，且started == stopped，覆盖了 sub2api #4193 中 "index=4 从未有 start 却收到 delta" 的同类场景。Summary by CodeRabbit