-
Notifications
You must be signed in to change notification settings - Fork 406
fix(agent): skip reasoning-only assistant messages to prevent empty m… #709
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
base: main
Are you sure you want to change the base?
Conversation
|
We require contributors to sign our Contributor License Agreement, and we don"t have @Cprakhar on file. In order for us to review and merge your code, please follow our CLA guidelines. |
|
@Cprakhar is attempting to deploy a commit to the stagewise Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughA new changeset entry documents a patch for Changes
Sequence DiagramsequenceDiagram
participant Stream as UI Stream
participant Parser as parseUiStream
participant Filter as Message Filter
participant History as Chat History
Stream->>Parser: Emit assistant message with parts
Parser->>Filter: Check message parts
alt All parts are reasoning or step-start
Filter-->>Parser: Skip message
Note over Parser,History: Message filtered out
else Contains user-visible content
Filter->>History: Add message to chat
Note over History: Message surfaced to user
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Possibly related PRs
Suggested labels
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Tip 📝 Customizable high-level summaries are now available in beta!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example instruction:
Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later. 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.
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)
agent/client/src/Agent.ts (1)
680-690: LGTM! Logic correctly implements the fix for the reported issue.The filtering logic properly prevents assistant messages containing only
reasoningorstep-startparts from being added to the chat state. The condition ordering is safe—parts?.length > 0ensurespartsis defined before calling.every().Optional enhancement: Consider also filtering assistant messages with empty or undefined
partsarrays to be more defensive:// Skip assistant messages that only contain non-user-visible reasoning or step-start parts if ( uiMessage.role === 'assistant' && - uiMessage.parts?.length > 0 && - uiMessage.parts.every( - (part) => - part.type === 'reasoning' || part.type === 'step-start', - ) + (!uiMessage.parts || + uiMessage.parts.length === 0 || + uiMessage.parts.every( + (part) => + part.type === 'reasoning' || part.type === 'step-start', + )) ) { continue; }This would also catch any assistant messages with no content, though the current implementation correctly addresses the specific bug reported in #705.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.changeset/itchy-files-doubt.md(1 hunks)agent/client/src/Agent.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
NEVER use any. NEVER use unknown unless absolutely necessary.
Files:
agent/client/src/Agent.ts
🧠 Learnings (1)
📚 Learning: 2025-08-17T19:03:32.418Z
Learnt from: CR
Repo: stagewise-io/stagewise PR: 0
File: toolbar/core/CLAUDE.md:0-0
Timestamp: 2025-08-17T19:03:32.418Z
Learning: Applies to toolbar/core/src/hooks/agent/**/*.{ts,tsx} : Place agent connectivity hooks under `src/hooks/agent`, with individual interface capabilities covered by individual hooks
Applied to files:
agent/client/src/Agent.ts
🔇 Additional comments (1)
.changeset/itchy-files-doubt.md (1)
1-5: LGTM!The changeset correctly documents the patch and provides a clear description of the fix that aligns with the PR objectives.
|
@Cprakhar thanks for your contribution! We would be ready to approve this once everything checks out and the CLA is signed! @juliangoetze should be an easy merge i guess? |
What
Skip adding reasoning-only assistant messages to chat state in
Agent.parseUiStream.Why
Fixes #705 - empty messages were being created when assistant only had reasoning/step-start parts, leading to "Agent failed: Bad Request [ce]" errors.
How
Added a guard that filters out assistant messages containing only
reasoningorstep-startparts before adding to karton state.Testing
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.