fix: preserve tool call stop reason in Anthropic streaming fallback#3640
fix: preserve tool call stop reason in Anthropic streaming fallback#3640dicnunz wants to merge 2 commits into
Conversation
|
Payment note: I saw this was an active company OSS bug with a narrow reproduction and no public bounty. If this saves the team time and you want to settle it as a small paid microfix, $10 via this checkout works: https://nicdunz.gumroad.com/l/slop-kill-pass?wanted=true No action needed if you prefer to review it as a normal OSS contribution; the fix and focused tests are submitted either way. |
|
|
|
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 (1)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThis PR fixes stop_reason propagation in OpenAI-to-Anthropic chat stream conversion. It adds a helper to map Chat finish_reason to Responses stop_reason, applies the helper to both streaming and non-streaming response conversions, and validates the behavior with schema-level and provider-level tests. ChangesStopReason Propagation for Chat Responses
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.12.2)level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies" 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: 1
🤖 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 `@core/providers/anthropic/compaction_test.go`:
- Around line 774-787: The test currently only asserts the intermediate
message_delta stop_reason; add an assertion that a terminal message_stop event
exists in events and that its StopReason equals AnthropicStopReasonToolUse.
Locate the loop over events (which finds messageDelta using
AnthropicStreamEventTypeMessageDelta) and similarly scan for an event with type
AnthropicStreamEventTypeMessageStop (e.g., assign to messageStop), verify
messageStop and messageStop.Delta (or equivalent terminal field) are non-nil,
and assert *messageStop.Delta.StopReason == AnthropicStopReasonToolUse to
prevent mixed-stream regression.
🪄 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: 26417184-df5f-48ee-8b32-8cb1ddc2b3d4
📒 Files selected for processing (3)
core/providers/anthropic/compaction_test.gocore/schemas/mux.gocore/schemas/mux_test.go
Confidence Score: 5/5The change is narrowly scoped to populating a previously-unset field; no existing behavior is removed and unknown finish reasons continue to return nil, leaving all current fallback paths intact. The fix is small, mechanically straightforward, and the new helper delegates to the already-tested No files require special attention. Important Files Changed
Reviews (2): Last reviewed commit: "test: assert Anthropic message stop afte..." | Re-trigger Greptile |
|
Addressed the valid CodeRabbit test coverage point in 2eb3900. The mixed text + tool-use regression test now also asserts that a terminal I did not put a stop reason on |
Summary
Fixes #3638.
When OpenAI-compatible chat streaming is converted into Bifrost Responses stream events, the terminal event now preserves recognized chat
finish_reasonvalues asResponse.StopReason.That lets the Anthropic serializer map a mixed text + tool-call stream ending with
finish_reason: "tool_calls"to:{"stop_reason":"tool_use"}instead of falling back to
end_turn.What changed
stop,tool_calls,length) on non-stream Responses conversion.stop/tool_calls->completedlength->incompletewithmax_output_tokensTests