-
Notifications
You must be signed in to change notification settings - Fork 1.5k
fix(@mastra/core): fix reasoning content loss due to interleaved chun… #11494
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…k order This commit addresses an issue where reasoning content was lost when the `text-start` chunk arrived before the `reasoning-end` chunk. The fix ensures that `text-start` does not trigger a reset of the reasoning state, allowing the accumulated reasoning deltas to be preserved correctly. Additionally, comprehensive tests have been added to verify the correct handling of interleaved chunks in various scenarios.
🦋 Changeset detectedLatest commit: 6346d11 The changes in this PR will be included in the next version bump. This PR includes changesets to release 17 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughThis patch release fixes a streaming bug in Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
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 |
🚨 Redirect Validation FailedThe redirect validation found issues in Action Required: Review and fix the redirect configuration. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.changeset/sharp-mammals-repair.md (1)
5-7: Consider simplifying the language to focus on user-facing outcomes.The description is technically accurate but uses internal terminology like "accumulated reasoning deltas" and "reasoning state reset" that may not be meaningful to developers using the library. Consider rephrasing to focus on the observable behavior:
Suggested revision:
Fixed an issue where reasoning content would be lost during streaming with certain model providers (e.g., ZAI/glm-4.6). When a model sends text chunks before finishing reasoning, the reasoning content is now correctly preserved in the final message instead of being cleared.This maintains technical accuracy while being more accessible to the target audience of developers using @mastra/core.
Based on coding guidelines for changeset files.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.changeset/sharp-mammals-repair.mdpackages/core/src/agent/__tests__/reasoning-interleaved.test.tspackages/core/src/loop/workflows/agentic-execution/llm-execution-step.ts
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Run
pnpm typecheckto validate TypeScript types across all packages
Files:
packages/core/src/loop/workflows/agentic-execution/llm-execution-step.tspackages/core/src/agent/__tests__/reasoning-interleaved.test.ts
**/*.{ts,tsx,js,jsx,json,md}
📄 CodeRabbit inference engine (CLAUDE.md)
Run
pnpm prettier:formatto format code andpnpm formatto run linting with auto-fix across all packages
Files:
packages/core/src/loop/workflows/agentic-execution/llm-execution-step.tspackages/core/src/agent/__tests__/reasoning-interleaved.test.ts
packages/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
packages/**/*.{ts,tsx}: All packages must use TypeScript with strict type checking enabled
Use telemetry decorators for observability across components
Files:
packages/core/src/loop/workflows/agentic-execution/llm-execution-step.tspackages/core/src/agent/__tests__/reasoning-interleaved.test.ts
.changeset/*.md
⚙️ CodeRabbit configuration file
.changeset/*.md: Changeset files are really important for keeping track of changes in the project. They'll be used to generate release notes and inform users about updates.Review the changeset file according to these guidelines:
- The target audience are developers
- Write short, direct sentences that anyone can understand. Avoid commit messages, technical jargon, and acronyms. Use action-oriented verbs (Added, Fixed, Improved, Deprecated, Removed)
- Avoid generic phrases like "Update code", "Miscellaneous improvements", or "Bug fixes"
- Highlight outcomes! What does change for the end user? Do not focus on internal implementation details
- Add context like links to issues or PRs when relevant
- If the change is a breaking change or is adding a new feature, ensure that a code example is provided. This code example should show the public API usage (the before and after). Do not show code examples of internal implementation details.
- Keep the formatting easy-to-read and scannable. If necessary, use bullet points or multiple paragraphs (Use bold text as the heading for these sections, do not use markdown headings).
- For larger, more substantial changes, also answer the "Why" behind the changes
- Each changeset file contains a YAML frontmatter at the top. It will be one or more package names followed by a colon and the type of change (patch, minor, major). Do not modify this frontmatter. Check that the description inside the changeset file only applies to the packages listed in the frontmatter. Do not allow descriptions that mention changes to packages not listed in the frontmatter. In these cases, the user must create a separate changeset file for those packages.
Files:
.changeset/sharp-mammals-repair.md
**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{test,spec}.{ts,tsx}: Use Vitest for testing framework in test files
Co-locate test files with source code using naming patterns like *.test.ts or *.spec.ts
Files:
packages/core/src/agent/__tests__/reasoning-interleaved.test.ts
🧠 Learnings (6)
📓 Common learnings
Learnt from: TheIsrael1
Repo: mastra-ai/mastra PR: 9247
File: packages/core/src/loop/network/index.ts:357-393
Timestamp: 2025-10-29T09:37:59.778Z
Learning: In the agent network routing completion stream (packages/core/src/loop/network/index.ts), when `completionStream.error` is true, the first stream should not have written any text (currentTextIdx should be 0), so the fallback reset of currentText and currentTextIdx does not cause duplication.
📚 Learning: 2025-12-30T15:02:58.132Z
Learnt from: taofeeq-deru
Repo: mastra-ai/mastra PR: 11486
File: packages/core/src/loop/workflows/agentic-execution/llm-execution-step.ts:643-678
Timestamp: 2025-12-30T15:02:58.132Z
Learning: In `packages/core/src/loop/workflows/agentic-execution/llm-execution-step.ts`, the system guarantees that `metadata.suspendedTools` and `metadata.pendingToolApprovals` never contain resumed tools, so no filtering is needed when using metadata directly.
Applied to files:
packages/core/src/loop/workflows/agentic-execution/llm-execution-step.ts
📚 Learning: 2025-10-29T09:37:59.778Z
Learnt from: TheIsrael1
Repo: mastra-ai/mastra PR: 9247
File: packages/core/src/loop/network/index.ts:357-393
Timestamp: 2025-10-29T09:37:59.778Z
Learning: In the agent network routing completion stream (packages/core/src/loop/network/index.ts), when `completionStream.error` is true, the first stream should not have written any text (currentTextIdx should be 0), so the fallback reset of currentText and currentTextIdx does not cause duplication.
Applied to files:
packages/core/src/loop/workflows/agentic-execution/llm-execution-step.ts.changeset/sharp-mammals-repair.mdpackages/core/src/agent/__tests__/reasoning-interleaved.test.ts
📚 Learning: 2025-11-24T16:42:04.244Z
Learnt from: CR
Repo: mastra-ai/mastra PR: 0
File: packages/codemod/AGENTS.md:0-0
Timestamp: 2025-11-24T16:42:04.244Z
Learning: Applies to packages/codemod/src/test/__fixtures__/**/*.ts : Create test fixtures by copying examples DIRECTLY from migration guides in `docs/src/content/en/guides/migrations/upgrade-to-v1/` without hallucinating or inventing changes
Applied to files:
packages/core/src/agent/__tests__/reasoning-interleaved.test.ts
📚 Learning: 2025-11-24T16:42:04.244Z
Learnt from: CR
Repo: mastra-ai/mastra PR: 0
File: packages/codemod/AGENTS.md:0-0
Timestamp: 2025-11-24T16:42:04.244Z
Learning: Applies to packages/codemod/src/test/__fixtures__/**/*.ts : In output fixtures, ensure all NEGATIVE test cases remain EXACTLY IDENTICAL to their input fixture counterparts to verify the codemod only transforms intended patterns
Applied to files:
packages/core/src/agent/__tests__/reasoning-interleaved.test.ts
📚 Learning: 2025-11-24T16:42:04.244Z
Learnt from: CR
Repo: mastra-ai/mastra PR: 0
File: packages/codemod/AGENTS.md:0-0
Timestamp: 2025-11-24T16:42:04.244Z
Learning: Applies to packages/codemod/src/test/**/*.test.ts : Include test cases for multiple occurrences of the transformation pattern, aliased imports, type imports, and mixed imports to verify the codemod works consistently
Applied to files:
packages/core/src/agent/__tests__/reasoning-interleaved.test.ts
🧬 Code graph analysis (1)
packages/core/src/loop/workflows/agentic-execution/llm-execution-step.ts (1)
packages/core/src/stream/base/output.ts (1)
chunk(1333-1336)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: test
- GitHub Check: test
- GitHub Check: Lint
- GitHub Check: Prebuild
- GitHub Check: test
- GitHub Check: test (express)
- GitHub Check: test (hono)
- GitHub Check: test
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (4)
packages/core/src/loop/workflows/agentic-execution/llm-execution-step.ts (1)
115-132: LGTM! The fix correctly preserves reasoning state when text-start arrives early.The addition of
chunk.type !== 'text-start'to the exclusion list is the minimal, targeted fix needed. The guard now correctly prevents text-start from clearingreasoningDeltasbeforereasoning-endcan save them (lines 256-290). The explanatory comment clearly documents why this exception is necessary for providers like ZAI/glm-4.6.The separation of concerns is maintained: text-start still triggers text delta flushing (lines 87-113) while now also preserving reasoning state until
reasoning-endarrives.packages/core/src/agent/__tests__/reasoning-interleaved.test.ts (3)
1-87: Excellent test setup with clear documentation.The file header (lines 1-14) provides valuable context linking to issue #11480, and
createInterleavedReasoningMockModelaccurately reproduces the problematic chunk sequence. The mock's stream order (lines 46-84) precisely matches the reported bug scenario:reasoning-start → reasoning-delta → text-start → reasoning-end.
98-149: LGTM! Core test case directly validates the fix.This test comprehensively verifies that reasoning content is preserved when text-start arrives before reasoning-end. The key assertion at lines 140-148 confirms
reasoningPart.detailsis populated with the expected reasoning text, which would have been empty before the fix.
154-285: Strong coverage of edge cases.Test 2 (lines 154-193) validates the reasoning accessor still functions correctly with interleaved chunks, and test 3 (lines 198-285) ensures multiple
reasoning-deltachunks are concatenated properly even when text-start arrives mid-sequence. Together these provide robust regression protection.
mastra-ai#11494) ## Description …k order This commit addresses an issue where reasoning content was lost when the `text-start` chunk arrived before the `reasoning-end` chunk. The fix ensures that `text-start` does not trigger a reset of the reasoning state, allowing the accumulated reasoning deltas to be preserved correctly. Additionally, comprehensive tests have been added to verify the correct handling of interleaved chunks in various scenarios. ## Related Issue(s) fixes mastra-ai#11480 ## Type of Change - [x] Bug fix (non-breaking change that fixes an issue) - [ ] New feature (non-breaking change that adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to change) - [ ] Documentation update - [ ] Code refactoring - [ ] Performance improvement - [ ] Test update ## Checklist - [ ] I have made corresponding changes to the documentation (if applicable) - [x] I have added tests that prove my fix is effective or that my feature works <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit ## Release Notes * **Bug Fixes** * Fixed streaming reasoning content handling to prevent reasoning data loss when text chunks arrive in unexpected order. * **Tests** * Added test suite for interleaved reasoning and text streaming scenarios to ensure proper content preservation. <sub>✏️ Tip: You can customize this high-level summary in your review settings.</sub> <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Description
…k order
This commit addresses an issue where reasoning content was lost when the
text-startchunk arrived before thereasoning-endchunk. The fix ensures thattext-startdoes not trigger a reset of the reasoning state, allowing the accumulated reasoning deltas to be preserved correctly. Additionally, comprehensive tests have been added to verify the correct handling of interleaved chunks in various scenarios.Related Issue(s)
fixes #11480
Type of Change
Checklist
Summary by CodeRabbit
Release Notes
Bug Fixes
Tests
✏️ Tip: You can customize this high-level summary in your review settings.