-
Notifications
You must be signed in to change notification settings - Fork 0
feat: add message persistence to /api/chat/generate (MYC-3957) #132
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: test
Are you sure you want to change the base?
feat: add message persistence to /api/chat/generate (MYC-3957) #132
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 <[email protected]>
|
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 |
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 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
This PR is being reviewed by Cursor Bugbot
Details
You are on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
lib/chat/handleChatGenerate.ts
Outdated
| // as part of the handleChatCredits and handleChatCompletion migrations | ||
| // Save assistant message to database if roomId is provided | ||
| if (body.roomId) { | ||
| const assistantMessage = getMessages(result.text, "assistant")[0]; |
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.
The code assumes getMessages() will always return an array with at least one element, but it can return an empty array, causing a TypeError when accessing [0].id.
View Details
📝 Patch Details
diff --git a/lib/chat/__tests__/handleChatGenerate.test.ts b/lib/chat/__tests__/handleChatGenerate.test.ts
index 5d9f50b..529eeff 100644
--- a/lib/chat/__tests__/handleChatGenerate.test.ts
+++ b/lib/chat/__tests__/handleChatGenerate.test.ts
@@ -490,5 +490,38 @@ describe("handleChatGenerate", () => {
const json = await result.json();
expect(json.text).toBe("Response");
});
+
+ it("gracefully handles empty text from generateText without crashing", async () => {
+ mockGetApiKeyAccountId.mockResolvedValue("account-123");
+
+ mockSetupChatRequest.mockResolvedValue({
+ model: "gpt-4",
+ instructions: "test",
+ system: "test",
+ messages: [],
+ experimental_generateMessageId: vi.fn(),
+ tools: {},
+ providerOptions: {},
+ } as any);
+
+ mockGenerateText.mockResolvedValue({
+ text: "",
+ finishReason: "stop",
+ usage: { promptTokens: 10, completionTokens: 0 },
+ response: { messages: [], headers: {}, body: null },
+ } as any);
+
+ const request = createMockRequest(
+ { prompt: "Hello", roomId: "room-abc" },
+ { "x-api-key": "valid-key" },
+ );
+
+ const result = await handleChatGenerate(request as any);
+
+ expect(result.status).toBe(200);
+ expect(mockInsertMemories).not.toHaveBeenCalled();
+ const json = await result.json();
+ expect(json.text).toBe("");
+ });
});
});
diff --git a/lib/chat/handleChatGenerate.ts b/lib/chat/handleChatGenerate.ts
index cf7c349..b984fba 100644
--- a/lib/chat/handleChatGenerate.ts
+++ b/lib/chat/handleChatGenerate.ts
@@ -34,16 +34,19 @@ export async function handleChatGenerate(request: NextRequest): Promise<Response
// Save assistant message to database if roomId is provided
if (body.roomId) {
- const assistantMessage = getMessages(result.text, "assistant")[0];
- try {
- await insertMemories({
- id: assistantMessage.id,
- room_id: body.roomId,
- content: filterMessageContentForMemories(assistantMessage),
- });
- } catch (error) {
- // Log error but don't fail the request - message persistence is non-critical
- console.error("Failed to persist assistant message:", error);
+ const assistantMessages = getMessages(result.text, "assistant");
+ if (assistantMessages.length > 0) {
+ const assistantMessage = assistantMessages[0];
+ try {
+ await insertMemories({
+ id: assistantMessage.id,
+ room_id: body.roomId,
+ content: filterMessageContentForMemories(assistantMessage),
+ });
+ } catch (error) {
+ // Log error but don't fail the request - message persistence is non-critical
+ console.error("Failed to persist assistant message:", error);
+ }
}
}
Analysis
Missing null check on empty array access causes TypeError in handleChatGenerate
What fails: lib/chat/handleChatGenerate.ts line 37 accesses the first element of an array returned by getMessages() without checking if the array is empty. When result.text is an empty string, getMessages() returns an empty array, causing [0] to be undefined. Accessing .id on undefined throws TypeError: Cannot read properties of undefined (reading 'id').
How to reproduce:
// Call generateText in a scenario where result.text is empty
// This can happen with the AI SDK when using tools with multiple invocations
// (see: https://github.com/vercel/ai/issues/6414)
const result = await generateText({
// ... config with tools ...
// If tools make multiple invocations, generateText can return empty text
});
// When result.text === "", getMessages("", "assistant") returns []
// Accessing [0].id on [] throws TypeErrorResult: TypeError is caught by outer try-catch (line 68), returning 500 error instead of gracefully handling the edge case.
Expected: Endpoint should return 200 with generated response even if text is empty; message persistence should be skipped when text is empty.
Root cause: Per AI SDK issue #6414, generateText() can return empty text when using tools with multiple invocations. The codebase uses tools (setupChatRequest.ts line 28, 31, 40) without explicit constraints to prevent this scenario.
Fix: Added length check before accessing array element:
const assistantMessages = getMessages(result.text, "assistant");
if (assistantMessages.length > 0) {
const assistantMessage = assistantMessages[0];
// ... rest of logic ...
}This gracefully skips message persistence when text is empty, allowing the endpoint to return 200 with the empty response rather than 500 error.
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 <[email protected]>
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 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
Summary
/api/chat/generate, matching the pattern from/api/emails/inboundroomIdis provided in the request body, the assistant message is saved to the databaseTest plan
insertMemoriesis called whenroomIdis providedinsertMemoriesis NOT called whenroomIdis not providedinsertMemoriesfails🤖 Generated with Claude Code
Note
Introduces non-blocking assistant message persistence and factors shared logic into a reusable utility.
saveChatCompletionto converttext→UIMessage, filter content, and insert amemoryhandleChatGenerateto callsaveChatCompletionwhenroomIdis provided; errors are logged but do not affect the responserespondToInboundEmailto usesaveChatCompletioninstead of inlined message→memory logichandleChatGenerateand forsaveChatCompletionend-to-end interactionsWritten by Cursor Bugbot for commit e875e24. This will update automatically on new commits. Configure here.