fix(ai): re-open force-completed tool calls when more args arrive - #1019
fix(ai): re-open force-completed tool calls when more args arrive#1019Mohith26 wants to merge 2 commits into
Conversation
|
Warning Review limit reached
Next review available in: 18 minutes Limit details: You’ve used all 8 included reviews currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?Wait for the limit to reset, then comment An organization admin can change what happens after included review limits in Billing. How do review limits work?CodeRabbit enforces per-developer PR review limits within each organization. For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe stream processor reopens tool calls marked complete when later argument deltas arrive. Authoritative or safety-net completion can then rebuild ChangesTool-call input repair
Estimated code review effort: 2 (Simple) | ~15 minutes Possibly related PRs
🚥 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}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/ai/tests/stream-processor.test.ts (1)
829-855: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMake the canonical input intentionally differ from streamed JSON.
The supplied
TOOL_CALL_END.inputequalsJSON.parse(FULL_ARGS), so this test cannot prove the canonical override path ran. Use a distinct canonical value and assert bothpart.inputandparsedArgumentsuse it whileargumentsremainsFULL_ARGS.Proposed regression strengthening
ev.toolEnd('tc-1', 'offerTemplates', { - input: { templateIds: ['mock-gsk-efimosfermin'] }, + input: { templateIds: ['canonical-template-id'] }, }), @@ + expect(part?.arguments).toBe(FULL_ARGS) - expect(part?.input).toEqual({ templateIds: ['mock-gsk-efimosfermin'] }) + expect(part?.input).toEqual({ + templateIds: ['canonical-template-id'], + }) @@ - ).toEqual({ templateIds: ['mock-gsk-efimosfermin'] }) + ).toEqual({ templateIds: ['canonical-template-id'] })🤖 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 `@packages/ai/tests/stream-processor.test.ts` around lines 829 - 855, Strengthen the test around StreamProcessor by making TOOL_CALL_END.input differ from the streamed FULL_ARGS value. Update the canonical input fixture in the toolEnd call, then assert part.input and the tc-1 parsedArguments both equal that distinct canonical value while part.arguments remains FULL_ARGS.
🤖 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 `@packages/ai/tests/stream-processor.test.ts`:
- Around line 776-915: Move the regression test block “text delta interleaved
between TOOL_CALL_ARGS deltas (`#1017`)” from the package-level tests into a
*.test.ts file alongside the source containing StreamProcessor. Preserve all
five test cases and their existing assertions unchanged.
---
Nitpick comments:
In `@packages/ai/tests/stream-processor.test.ts`:
- Around line 829-855: Strengthen the test around StreamProcessor by making
TOOL_CALL_END.input differ from the streamed FULL_ARGS value. Update the
canonical input fixture in the toolEnd call, then assert part.input and the tc-1
parsedArguments both equal that distinct canonical value while part.arguments
remains FULL_ARGS.
🪄 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: 49951d96-af20-4710-9e97-9068fdfd5912
📒 Files selected for processing (3)
.changeset/interleaved-text-tool-input.mdpackages/ai/src/activities/chat/stream/processor.tspackages/ai/tests/stream-processor.test.ts
6a4b458 to
d4e99f9
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
|
Maintainer sweep: rebased onto |
|
View your CI Pipeline Execution ↗ for commit 2713e79
☁️ Nx Cloud last updated this comment at |
|
Thanks for the PR, @Mohith26! 🙌 @AlemTuzlak will take a look. Automated pre-review checks
Automated triage — a human review follows. |
github-actions
Bot
added
the
waiting-on: author
d4e99f9 to
5440c52
Compare
|
Maintainer sweep: rebased onto |
A TEXT_MESSAGE_CONTENT event between two TOOL_CALL_ARGS events force-completes the in-flight tool call, populating part.input from a lenient partial-JSON parse of the arguments received so far. The subsequent authoritative TOOL_CALL_END was then skipped because the call was already input-complete, so the truncated input was never corrected even though the arguments string held the complete JSON. Re-open such an inferred-complete call back to input-streaming when a further TOOL_CALL_ARGS delta arrives, so TOOL_CALL_END (or the RUN_FINISHED safety net) re-finalizes input from the complete arguments string. Fixes TanStack#1017
Regression tests for TanStack#1017: interleaving orders with TOOL_CALL_END, TOOL_CALL_END.input as canonical, the RUN_FINISHED safety net with no TOOL_CALL_END, multiple parallel tool calls, and the internal input-complete -> input-streaming re-open transition.
5440c52 to
2713e79
Compare
|
Maintainer sweep: rebased onto |
3 participants
Fixes #1017, reported by @andrewmcgivery, matching the diagnosis in the issue exactly.
handleTextMessageContentEventcallscompleteAllToolCallsForMessage()on every text delta, which finalizes in-flight tool calls with a lenient partial-JSON parse intoinput.handleToolCallEndEventthen early-skips calls already ininput-complete, so when a TEXT_MESSAGE_CONTENT delta interleaves between two TOOL_CALL_ARGS deltas, the truncated parse becomes permanent even thoughargumentsholds the full correct JSON.The fix is 8 lines in
handleToolCallArgsEvent: a further args delta on aninput-completecall re-opens it toinput-streaming, letting the authoritative TOOL_CALL_END (or the RUN_FINISHED safety net) re-finalizeinputfrom the complete arguments string. Self-correcting across interleaving orders; the existing inferred-completion heuristic is preserved.Patch changeset included. Five regression tests (END-finalized ordering, END-input-canonical, RUN_FINISHED safety net, multiple parallel tool calls, the internal re-open transition) were written fail-first and all fail without the fix, reproducing the issue's exact truncated output. Package suite: 1291 passed, zero new failures; oxlint and tsc clean.
Summary by CodeRabbit
Bug Fixes
Tests