fix: handle finish_reason "length" and stream-end without finish_reason in processToolCalls (fixes #425)#426
Open
proyectoauraorg wants to merge 5 commits into
Conversation
…provider Merging 12 new test cases covering completePrompt, streaming resilience, and edge cases.
Dependabot bump. Compatible: project requires node>=20.20.2, uuid v14 requires node>=20.
# Conflicts: # pnpm-lock.yaml
…on in processToolCalls When a streaming tool call is interrupted by token exhaustion, tool calls ended up with empty nativeArgs because processToolCalls() only finalized on finish_reason 'tool_calls'. This affected any OpenAI-compatible provider when max_tokens was hit mid-tool-call. Changes: - processToolCalls(): Also trigger tool_call_end on finish_reason 'length' - OpenAiHandler.createMessage(): Add post-loop cleanup for orphaned tool calls - handleStreamResponse(): Add post-loop cleanup for orphaned tool calls - MimoHandler.createMessage(): Add post-loop cleanup for orphaned tool calls - presentAssistantMessage.ts: Improve error message with actionable guidance Tests added: - openai.spec.ts: finish_reason 'length' triggers tool_call_end - openai.spec.ts: stream-end without finish_reason triggers post-loop cleanup - mimo.spec.ts: MiMo-style choices:[] termination triggers tool_call_end - mimo.spec.ts: finish_reason 'length' triggers tool_call_end Fixes Zoo-Code-Org#425
proyectoauraorg
requested review from
JamesRobert20,
edelauna,
hannesrudolph,
navedmerchant and
taltas
as code owners
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThis PR finalizes active streaming tool calls when streams end due to token exhaustion or missing finish_reason: it emits ChangesStreaming Tool Call Finalization for Token Limits
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
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. 🔧 ESLint
src/api/providers/__tests__/mimo.spec.tsESLint skipped: missing config or dependency (missing-dependency). The ESLint configuration references a package that is not available in the sandbox. 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.
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 `@src/api/providers/openai.ts`:
- Around line 474-482: The orphaned-tool cleanup currently runs inside the
streaming chunk loop and prematurely emits tool_call_end and clears
activeToolCallIds on the first partial delta; move the block that iterates
activeToolCallIds and yields { type: "tool_call_end", id } to execute after the
stream loop completes (i.e., once end-of-stream/stream termination is reached)
so activeToolCallIds persists across subsequent chunks; update the logic that
currently references activeToolCallIds.clear() to only run in that post-loop
finalization path (referencing activeToolCallIds and the yield { type:
"tool_call_end", id } emission).
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: f4d4619d-2c46-4967-a641-ce33d197baba
📒 Files selected for processing (5)
src/api/providers/__tests__/mimo.spec.tssrc/api/providers/__tests__/openai.spec.tssrc/api/providers/mimo.tssrc/api/providers/openai.tssrc/core/assistant-message/presentAssistantMessage.ts
Comment on lines
+474
to
+482
| // Finalize any tool calls that weren't explicitly ended by finish_reason. | ||
| // This handles cases where the stream terminates without a proper finish_reason | ||
| // (e.g., some OpenAI-compatible proxies emit choices: [] when tokens are exhausted). | ||
| if (activeToolCallIds.size > 0) { | ||
| for (const id of activeToolCallIds) { | ||
| yield { type: "tool_call_end", id } | ||
| } | ||
| activeToolCallIds.clear() | ||
| } |
Contributor
There was a problem hiding this comment.
Move the orphaned-tool cleanup outside the stream loop.
This block still runs on every chunk, not at end-of-stream. Line 477 will emit tool_call_end after the first partial tool-call delta and clear activeToolCallIds, so the O3 streaming path can finalize tools before later argument chunks arrive.
Proposed fix
for await (const chunk of stream) {
const delta = chunk.choices?.[0]?.delta
const finishReason = chunk.choices?.[0]?.finish_reason
if (delta) {
if (delta.content) {
yield {
type: "text",
text: delta.content,
}
}
yield* this.processToolCalls(delta, finishReason, activeToolCallIds)
}
if (chunk.usage) {
yield {
type: "usage",
inputTokens: chunk.usage.prompt_tokens || 0,
outputTokens: chunk.usage.completion_tokens || 0,
}
}
-
- // Finalize any tool calls that weren't explicitly ended by finish_reason.
- // This handles cases where the stream terminates without a proper finish_reason
- // (e.g., some OpenAI-compatible proxies emit choices: [] when tokens are exhausted).
- if (activeToolCallIds.size > 0) {
- for (const id of activeToolCallIds) {
- yield { type: "tool_call_end", id }
- }
- activeToolCallIds.clear()
- }
}
+
+ // Finalize any tool calls that weren't explicitly ended by finish_reason.
+ // This handles cases where the stream terminates without a proper finish_reason
+ // (e.g., some OpenAI-compatible proxies emit choices: [] when tokens are exhausted).
+ if (activeToolCallIds.size > 0) {
+ for (const id of activeToolCallIds) {
+ yield { type: "tool_call_end", id }
+ }
+ activeToolCallIds.clear()
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // Finalize any tool calls that weren't explicitly ended by finish_reason. | |
| // This handles cases where the stream terminates without a proper finish_reason | |
| // (e.g., some OpenAI-compatible proxies emit choices: [] when tokens are exhausted). | |
| if (activeToolCallIds.size > 0) { | |
| for (const id of activeToolCallIds) { | |
| yield { type: "tool_call_end", id } | |
| } | |
| activeToolCallIds.clear() | |
| } | |
| for await (const chunk of stream) { | |
| const delta = chunk.choices?.[0]?.delta | |
| const finishReason = chunk.choices?.[0]?.finish_reason | |
| if (delta) { | |
| if (delta.content) { | |
| yield { | |
| type: "text", | |
| text: delta.content, | |
| } | |
| } | |
| yield* this.processToolCalls(delta, finishReason, activeToolCallIds) | |
| } | |
| if (chunk.usage) { | |
| yield { | |
| type: "usage", | |
| inputTokens: chunk.usage.prompt_tokens || 0, | |
| outputTokens: chunk.usage.completion_tokens || 0, | |
| } | |
| } | |
| } | |
| // Finalize any tool calls that weren't explicitly ended by finish_reason. | |
| // This handles cases where the stream terminates without a proper finish_reason | |
| // (e.g., some OpenAI-compatible proxies emit choices: [] when tokens are exhausted). | |
| if (activeToolCallIds.size > 0) { | |
| for (const id of activeToolCallIds) { | |
| yield { type: "tool_call_end", id } | |
| } | |
| activeToolCallIds.clear() | |
| } |
🤖 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 `@src/api/providers/openai.ts` around lines 474 - 482, The orphaned-tool
cleanup currently runs inside the streaming chunk loop and prematurely emits
tool_call_end and clears activeToolCallIds on the first partial delta; move the
block that iterates activeToolCallIds and yields { type: "tool_call_end", id }
to execute after the stream loop completes (i.e., once end-of-stream/stream
termination is reached) so activeToolCallIds persists across subsequent chunks;
update the logic that currently references activeToolCallIds.clear() to only run
in that post-loop finalization path (referencing activeToolCallIds and the yield
{ type: "tool_call_end", id } emission).
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
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.
Summary
Fixes #425. When a streaming tool call is interrupted by token exhaustion, tool calls end up with empty
nativeArgsbecauseprocessToolCalls()only finalizes onfinish_reason: "tool_calls". This affects any OpenAI-compatible provider whenmax_tokensis hit mid-tool-call.Root Cause
processToolCalls()inopenai.ts:498only emitstool_call_endwhenfinishReason === "tool_calls". It ignores:finish_reason: "length"— standard OpenAI behavior when output tokens are exhaustedfinish_reason— behavior of some proxies (e.g., MiMo emitschoices: [])Additionally, neither
OpenAiHandler.createMessage()norMimoHandler.createMessage()had post-loop cleanup for orphanedactiveToolCallIds.Changes
Core fix —
openai.tsprocessToolCalls(): Also triggertool_call_endonfinish_reason: "length"createMessage(): Add post-loop cleanup — finalize orphaned tool calls when stream endshandleStreamResponse(): Same post-loop cleanup for the alternate streaming pathProvider fix —
mimo.tsMimoHandler.createMessage(): Add post-loop cleanup for MiMo-specific streaming path (MiMo proxy emitschoices: []withoutfinish_reasonwhen tokens are exhausted)UX improvement —
presentAssistantMessage.tsTests
openai.spec.tsfinish_reason: "length"triggerstool_call_end(standard OpenAI behavior)finish_reasontriggers post-loop cleanup (proxy termination)mimo.spec.tschoices: []termination triggerstool_call_endvia post-loop cleanupfinish_reason: "length"triggerstool_call_endAll 134 tests pass (54 openai + 47 mimo + 33 assistant-message).
Test Plan
cd src && npx vitest run api/providers/__tests__/openai.spec.ts— 54/54 ✅cd src && npx vitest run api/providers/__tests__/mimo.spec.ts— 47/47 ✅cd src && npx vitest run core/assistant-message— 33/33 ✅Related Issues
missing nativeArgs) but different root cause (strict schema + truthy check in parser). Our fix addresses a complementary gap in the core streaming handler.Alignment with CONTRIBUTING.md
This fix directly improves reliability for all OpenAI-compatible providers when output tokens are exhausted mid-tool-call.
Summary by CodeRabbit
Bug Fixes
Tests
Chores