feat: migrate memories get to /api/chats/{id}/messages#383
feat: migrate memories get to /api/chats/{id}/messages#383sweetmantech merged 7 commits intotestfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 18 minutes and 38 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds a new Next.js API route Changes
Sequence DiagramsequenceDiagram
participant Client
participant RouteHandler as GET Route
participant ChatHandler as getChatMessagesHandler
participant Validator as validateChatAccess
participant Storage as selectMemories
Client->>RouteHandler: GET /api/chats/{id}/messages
RouteHandler->>ChatHandler: await getChatMessagesHandler(request, id)
ChatHandler->>ChatHandler: validate id (UUID)
alt invalid id
ChatHandler-->>Client: 400 JSON Error (with CORS)
else valid id
ChatHandler->>Validator: validateChatAccess(request, id)
alt access denied (NextResponse)
Validator-->>ChatHandler: NextResponse (error)
ChatHandler-->>Client: 401/403 Response (with CORS)
else access granted
ChatHandler->>Storage: selectMemories(roomId, {ascending:true})
alt storage error / null
Storage-->>ChatHandler: null / error
ChatHandler-->>Client: 500 JSON Error (with CORS)
else memories found
Storage-->>ChatHandler: memories[]
ChatHandler-->>Client: 200 JSON { data: memories } (with CORS)
end
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ❌ 1❌ Failed checks (1 warning)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/api/chats/`[id]/messages/route.ts:
- Around line 21-27: The GET route currently forwards the request to
getMemoriesHandler which returns a nested payload under a data property,
violating the flat-body guideline; either update getMemoriesHandler to return a
flat response object (e.g., root-level fields like memories, meta, error) or
unwrap its result in the GET function before creating the NextResponse so the
response body has root-level fields; locate the GET function and
getMemoriesHandler and change the return shape (or the wrapper in GET) so no {
data: ... } nesting remains.
In `@lib/memories/getMemoriesHandler.ts`:
- Around line 28-49: Wrap the awaited calls to validateChatAccess(...) and
selectMemories(...) in a try/catch inside getMemoriesHandler to catch thrown
dependency errors and return a JSON error response with CORS headers instead of
letting the exception bubble. Specifically, enclose the block that calls
validateChatAccess and selectMemories in try { ... } and in catch (err) return
NextResponse.json({ status: "error", error: "Failed to retrieve memories",
details: String(err) }, { status: 500, headers: getCorsHeaders() }); optionally
log the error before returning to aid debugging, so the handler always respects
the JSON+CORS response contract.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 5d1af798-3ace-48e4-8cf7-1905a26b9c17
⛔ Files ignored due to path filters (1)
lib/memories/__tests__/getMemoriesHandler.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**
📒 Files selected for processing (2)
app/api/chats/[id]/messages/route.tslib/memories/getMemoriesHandler.ts
There was a problem hiding this comment.
♻️ Duplicate comments (1)
lib/chats/getChatMessagesHandler.ts (1)
34-55:⚠️ Potential issue | 🟠 MajorWrap dependency calls to preserve the JSON+CORS error contract.
If
validateChatAccess(...)orselectMemories(...)throws, the handler can bypass your structured JSON+CORS responses and bubble an unhandled 500.Suggested hardening
export async function getChatMessagesHandler( request: NextRequest, id: string, ): Promise<NextResponse> { - const parsedId = chatIdSchema.safeParse(id); - if (!parsedId.success) { - return NextResponse.json( - { - status: "error", - error: parsedId.error.issues[0]?.message || "Invalid chat ID", - }, - { status: 400, headers: getCorsHeaders() }, - ); - } - - const roomResult = await validateChatAccess(request, parsedId.data); - if (roomResult instanceof NextResponse) { - return roomResult; - } - - const memories = await selectMemories(roomResult.room.id, { ascending: true }); - if (memories === null) { + try { + const parsedId = chatIdSchema.safeParse(id); + if (!parsedId.success) { + return NextResponse.json( + { + status: "error", + error: parsedId.error.issues[0]?.message || "Invalid chat ID", + }, + { status: 400, headers: getCorsHeaders() }, + ); + } + + const roomResult = await validateChatAccess(request, parsedId.data); + if (roomResult instanceof NextResponse) { + return roomResult; + } + + const memories = await selectMemories(roomResult.room.id, { ascending: true }); + if (memories === null) { + return NextResponse.json( + { + status: "error", + error: "Failed to retrieve memories", + }, + { status: 500, headers: getCorsHeaders() }, + ); + } + return NextResponse.json( { - status: "error", - error: "Failed to retrieve memories", + data: memories, }, - { status: 500, headers: getCorsHeaders() }, + { status: 200, headers: getCorsHeaders() }, ); + } catch { + return NextResponse.json( + { + status: "error", + error: "Internal server error", + }, + { status: 500, headers: getCorsHeaders() }, + ); } - - return NextResponse.json( - { - data: memories, - }, - { status: 200, headers: getCorsHeaders() }, - ); }As per coding guidelines, "For domain functions, ensure: ... Proper error handling".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/chats/getChatMessagesHandler.ts` around lines 34 - 55, The handler currently calls validateChatAccess(...) and selectMemories(...) directly so any thrown exception can escape and bypass your JSON+CORS error responses; wrap both calls in try/catch blocks inside getChatMessagesHandler (or the function that contains this snippet) so that if validateChatAccess or selectMemories throws you return a well-formed NextResponse.json({ status: "error", error: "<brief message or error.message>" }, { status: 500, headers: getCorsHeaders() }); preserve existing behavior for NextResponse returns from validateChatAccess (i.e., if roomResult instanceof NextResponse return it), and include the same getCorsHeaders() in all error responses.
🧹 Nitpick comments (1)
lib/chats/getChatMessagesHandler.ts (1)
8-24: Avoid duplicate chat ID validation to keep this handler DRY.
idis validated here and then validated again invalidateChatAccess(...)(seelib/chats/validateChatAccess.ts:25-33). Consider centralizing this in one place to avoid drift in validation behavior/messages over time.As per coding guidelines, "Extract shared logic into reusable utilities following Don't Repeat Yourself (DRY) principle".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/chats/getChatMessagesHandler.ts` around lines 8 - 24, This handler duplicates ID validation—remove the chatIdSchema.safeParse block from getChatMessagesHandler and delegate validation to the existing validateChatAccess(id, request) utility; call validateChatAccess with the raw id, handle/return its error response as before, and remove any references to chatIdSchema or parsedId in this file so all chat ID validation lives in validateChatAccess (preserving its error messages/behavior).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@lib/chats/getChatMessagesHandler.ts`:
- Around line 34-55: The handler currently calls validateChatAccess(...) and
selectMemories(...) directly so any thrown exception can escape and bypass your
JSON+CORS error responses; wrap both calls in try/catch blocks inside
getChatMessagesHandler (or the function that contains this snippet) so that if
validateChatAccess or selectMemories throws you return a well-formed
NextResponse.json({ status: "error", error: "<brief message or error.message>"
}, { status: 500, headers: getCorsHeaders() }); preserve existing behavior for
NextResponse returns from validateChatAccess (i.e., if roomResult instanceof
NextResponse return it), and include the same getCorsHeaders() in all error
responses.
---
Nitpick comments:
In `@lib/chats/getChatMessagesHandler.ts`:
- Around line 8-24: This handler duplicates ID validation—remove the
chatIdSchema.safeParse block from getChatMessagesHandler and delegate validation
to the existing validateChatAccess(id, request) utility; call validateChatAccess
with the raw id, handle/return its error response as before, and remove any
references to chatIdSchema or parsedId in this file so all chat ID validation
lives in validateChatAccess (preserving its error messages/behavior).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 0344aff3-37fd-4c45-b47e-794ab00986ba
⛔ Files ignored due to path filters (1)
lib/chats/__tests__/getChatMessagesHandler.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**
📒 Files selected for processing (2)
app/api/chats/[id]/messages/route.tslib/chats/getChatMessagesHandler.ts
✅ Files skipped from review due to trivial changes (1)
- app/api/chats/[id]/messages/route.ts
|
Quick smoke test run against this PR preview deployment. Preview URL: https://recoup-api-git-codex-arpit-migrate-m-af41dc-recoupable-ad724970.vercel.app Results:
Smoke verdict: endpoint is reachable and behaves as expected for valid + invalid ID paths. |
lib/chats/getChatMessagesHandler.ts
Outdated
| const parsedId = chatIdSchema.safeParse(id); | ||
| if (!parsedId.success) { | ||
| return NextResponse.json( | ||
| { | ||
| status: "error", | ||
| error: parsedId.error.issues[0]?.message || "Invalid chat ID", | ||
| }, | ||
| { status: 400, headers: getCorsHeaders() }, | ||
| ); | ||
| } | ||
|
|
||
| const roomResult = await validateChatAccess(request, parsedId.data); | ||
| if (roomResult instanceof NextResponse) { | ||
| return roomResult; | ||
| } |
There was a problem hiding this comment.
Move auth and param parsing to a standalone validate function following existing API endpoints.
Endpoint VerificationTested Result: ✅ 200 OK — returns Response shape matches docs — Each message contains:
Review feedback: Auth + param parsing should be extracted to a |
Move auth + param parsing to standalone validate function. Handler now calls validateGetChatMessagesQuery then selectMemories. 1670/1670 tests pass. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2 participants
Summary
Notes
Summary by CodeRabbit