Conversation
📝 WalkthroughWalkthroughAdded an optional Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes 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
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
✏️ Tip: You can disable this entire section by setting 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 |
commit: |
9ec4fec to
cdce325
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/UIMessages.ts (1)
70-85: Prevent meta.userId from being overwritten by undefined.Because the creators now always set
userId(even asundefined), spreadinguiMessageaftermetacan wipe a providedmeta.userId. This can drop identity when round‑tripping UI messages that lack a userId. Consider coalescing explicitly.🔧 Proposed fix
- ...omit(uiMessage, ["parts", "role", "key", "text"]), + ...omit(uiMessage, ["parts", "role", "key", "text", "userId"]), + userId: uiMessage.userId ?? meta.userId,
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/UIMessages.tssrc/toUIMessages.test.ts
🧰 Additional context used
🧬 Code graph analysis (1)
src/toUIMessages.test.ts (2)
src/UIMessages.ts (1)
toUIMessages(161-184)src/react/index.ts (1)
toUIMessages(3-3)
🔇 Additional comments (5)
src/toUIMessages.test.ts (1)
732-857: Good coverage for userId propagation scenarios.The new tests validate user/system/assistant and grouped assistant behaviors, plus undefined userId handling.
src/UIMessages.ts (4)
35-46: UIMessage surface update looks right.Adding
userId?: stringaligns with the new preservation flow.
282-295: System UI message now preserves userId.Looks good and consistent with the updated UIMessage shape.
343-355: User UI message now preserves userId.Propagation is straightforward and matches the requirement.
369-377: Assistant UI message userId propagation looks correct.Using the first message in the group aligns with the grouping contract and tests.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
When converting MessageDoc to UIMessage via toUIMessages(), the userId field was being lost. This made it difficult to identify which user sent each message in multi-user thread conversations. Changes: - Add userId?: string to UIMessage type definition - Include userId in createSystemUIMessage() - Include userId in createUserUIMessage() - Include userId in createAssistantUIMessage() (from first message) - Add tests for userId preservation across all message types Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
cdce325 to
ab6a8b0
Compare
…-213 Merge upstream PRs get-convex#211, get-convex#212, and get-convex#213 created by @sethconvex
ianmacartney
left a comment
There was a problem hiding this comment.
This works - I figured any metadata returned on MessageDoc would show up, but it's a good point that when it's reduced to a UIMessage some fields get "promoted" to not be per-message but for the whole collection, and userId is something that seems fine to special-case. Doesn't seem like we'll be leaking extra data out, since that data was already present by default
Problem
The
UIMessagetype did not include auserIdfield, making it impossible to identify which user a message belongs to in the UI layer. This information was available in the underlyingMessageDocbut was not propagated toUIMessage.Solution
Add
userIdfield to theUIMessagetype and propagate it during message creation:Propagate in each message type creator:
Fixes #188
Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Tests
✏️ Tip: You can customize this high-level summary in your review settings.