-
Notifications
You must be signed in to change notification settings - Fork 0
feat: auto-create roomId in validateChatRequest when not provided (MYC-3961) #133
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
feat: auto-create roomId in validateChatRequest when not provided (MYC-3961) #133
Conversation
Add assistant message persistence to handleChatGenerate, matching the pattern from /api/emails/inbound. When roomId is provided, the assistant message is saved to the database using insertMemories with filtered content. Changes: - Import insertMemories, getMessages, filterMessageContentForMemories - After generateText completes, persist assistant message if roomId exists - Gracefully handle persistence failures (log but don't fail request) - Add 4 unit tests for message persistence behavior Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Extract shared chat completion persistence logic into reusable utility. Both /api/chat/generate and /api/emails/inbound now use this shared function instead of duplicating the 3-step process. - Create lib/chat/saveChatCompletion.ts encapsulating getMessages + filterMessageContentForMemories + insertMemories - Add 6 unit tests for saveChatCompletion - Update respondToInboundEmail.ts to use saveChatCompletion - Update handleChatGenerate.ts to use saveChatCompletion - Update handleChatGenerate.test.ts to mock new utility Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Match /api/emails/inbound behavior by automatically creating a room when roomId is not provided in chat requests. This ensures conversations are properly tracked even when clients don't specify a roomId upfront. - Import generateUUID and createNewRoom - Generate UUID when roomId is missing - Create room with accountId, generated roomId, artistId, and first message - Handle both UIMessage format (parts) and simple format (content) - Add 6 unit tests for auto room creation behavior Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 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 |
Update tests to reflect new behavior where roomId is auto-created when not provided. The test "does not save message when roomId is not provided" is now "saves message with auto-generated roomId when roomId is not provided" since validateChatRequest auto-creates the room. - Add mocks for generateUUID and createNewRoom - Update test assertion to expect saveChatCompletion to be called - Fixes test file failing to load due to missing Supabase env vars Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Since validateChatRequest now auto-creates roomId when not provided, body.roomId is always defined after validation. The conditional check was dead code that would always evaluate to true. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Return the roomId in the JSON response so clients know which room was used, especially important when roomId is auto-generated. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
When auto-creating a room, the user's initial message was not being saved to the memories table. This caused incomplete conversation history where only assistant responses were stored. Match /api/emails/inbound behavior by calling saveChatCompletion with role: "user" after creating the new room. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Per /api/emails/inbound behavior, user messages should be persisted for ALL requests, not just when creating new rooms. Updated to use insertMemories directly with filterMessageContentForMemories (matching the exact pattern used in validateNewEmailMemory.ts). Changes: - Room creation remains conditional (only when roomId not provided) - User message persistence happens for ALL requests - Use insertMemories + filterMessageContentForMemories directly - Remove saveChatCompletion dependency from validateChatRequest Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…lity Extract common room creation and message persistence logic shared between validateChatRequest.ts (chat API flow) and validateNewEmailMemory.ts (email inbound flow) into a reusable setupConversation utility. The utility: - Auto-creates room if roomId not provided - Persists user message to memories for ALL requests - Supports custom memoryId for deduplication (used by email flow) - Propagates errors for caller handling (e.g., duplicate detection) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
When clients send conversation history, the first message is historical and already exists in the database. Changed to: - Use the LAST message (newest) instead of first (oldest) - Use the message's original ID as memoryId to align with handleChatCompletion's upsert and prevent duplicate entries Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add convertToUiMessages() utility (similar to AI SDK's convertToModelMessages)
that normalizes messages from various formats into UIMessage format:
- Passes through UIMessage format unchanged
- Converts simple { role, content } format to UIMessage
- Handles mixed arrays of both formats
- Preserves existing message IDs or generates new ones
Refactor validateChatRequest to use:
- convertToUiMessages() for format normalization
- validateMessages() to get the last message
This DRYs up the inline message format handling logic.
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Move isUiMessage type guard function from convertToUiMessages.ts to its own file following Single Responsibility Principle. Also improved the type guard to handle edge cases: - null/undefined values - primitive values - non-array parts property Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Replace custom SimpleMessage interface with ModelMessage from the AI SDK, following DRY principle by reusing existing SDK types. Updated extractTextContent helper to handle ModelMessage.content which can be either a string or content parts array. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add mocks for setupConversation, validateMessages, convertToUiMessages, and handleChatCompletion to prevent Supabase serverClient from loading during tests. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Remove unnecessary extractTextContent function since ModelMessage.content is always a string type. Direct assignment is simpler and clearer. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
ModelMessage.content can be either a string or an array of content parts. Added getTextContent helper to extract text from both formats. Added test for content parts array case. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Move getTextContent function from convertToUiMessages.ts to its own file following Single Responsibility Principle. Added 7 tests for the new utility. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
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.
Cursor Bugbot has reviewed your changes and found 3 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
| ...validatedBody, | ||
| accountId, | ||
| orgId, | ||
| roomId: finalRoomId, |
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.
Converted messages not returned causing runtime crash
High Severity
When messages are provided in simple format ({ role, content }), convertToUiMessages generates UIMessage format with IDs and parts arrays, but the converted uiMessages is not assigned back to validatedBody.messages. The returned body still contains the original simple format. Later, handleChatCompletion receives these simple format messages and calls validateMessages(messages) which accesses m.parts.find(...), causing a runtime error since simple format messages have content not parts.
| promptMessage: lastMessage, | ||
| artistId: validatedBody.artistId, | ||
| memoryId: lastMessage.id, | ||
| }); |
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.
Chat flow missing duplicate message error handling
Medium Severity
setupConversation uses insertMemories which throws on duplicate key violations, and its docstring explicitly states callers should handle error code "23505" for duplicates. The email flow in validateNewEmailMemory properly catches and handles this error, but validateChatRequest calls setupConversation without any try-catch. If a client retries a request with the same message ID (common with UIMessage format), the second request fails with a database constraint violation instead of being handled gracefully.
| .filter((part): part is { type: "text"; text: string } => part.type === "text") | ||
| .map((part) => part.text) | ||
| .join(""); | ||
| } |
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.
Missing null check crashes on malformed message content
Medium Severity
The new getTextContent function assumes content is either a string or an array, but doesn't handle undefined or null. Since the schema allows z.array(z.any()) for messages, a malformed message like { role: "user" } (no content property) passes validation. When convertToUiMessages processes it, modelMessage.content is undefined, and getTextContent crashes with "Cannot read property 'filter' of undefined" when it falls through to the array handling branch.
Summary
Test plan
pnpm test)🤖 Generated with Claude Code
Note
Introduces consistent conversation setup and message persistence across chat and email.
roomIdand persists the latest user message invalidateChatRequestviasetupConversation; returns the finalroomIdto clientssetupConversationutility to create rooms (when needed) and insert user memories (with optionalmemoryIdfor deduplication)saveChatCompletionutility and uses it inhandleChatGenerateand email reply flow to store assistant messages;handleChatGeneratenow includesroomIdin the response and tolerates persistence failuresconvertToUiMessages,isUiMessage, andgetTextContenthelpersWritten by Cursor Bugbot for commit d6b9f61. This will update automatically on new commits. Configure here.