Chat feature #414
Chat feature #414
Conversation
WalkthroughAdds 1:1 real-time chat: Prisma Conversation and Message models and Profile relations, TRPC chat procedures backed by Prisma, server in-memory SSE pub/sub and /api/chat/stream endpoint, frontend SSE consumers and UI wiring, grapheme-aware validation (1–500 graphemes), useContact composable, and related UI adjustments. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Browser
participant UI as Chat UI
participant SSEAPI as /api/chat/stream
participant TRPC as trpc.chat
participant DB as Prisma
participant SSEsrv as sse (in-memory)
Browser->>UI: open conversation (conversationId)
UI->>SSEAPI: GET /api/chat/stream?conversationId=CID
SSEAPI->>SSEsrv: subscribe "conversation:CID"
SSEAPI-->>UI: SSE connected + conversation.init
Browser->>TRPC: sendMessage({conversationId:CID, content})
TRPC->>DB: create message & update conversation (transaction)
TRPC->>SSEsrv: publish "conversation:CID" (message.created)
TRPC->>SSEsrv: publish "inbox:A" & "inbox:B" (conversation.updated)
SSEsrv-->>SSEAPI: delivers events
SSEAPI-->>UI: SSE events received → UI appends message / updates inbox
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Pre-merge checks (1 passed, 3 warnings, 1 inconclusive)❌ Failed checks (3 warnings, 1 inconclusive)
✅ Passed checks (1 passed)
Poem
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. ✨ Finishing Touches
🧪 Generate unit tests
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 |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
components/chat/ChatConversation.vue (1)
256-267: Enforce the 500-character input limit in the UI (acceptance criteria).Add maxlength to the input and provide subtle user feedback.
- <input + <input v-model="newMessage" type="text" placeholder="Type a message..." class="flex-1 border rounded-l-lg px-4 py-2 focus:outline-none focus:ring-2 focus:ring-primary dark:bg-gray-800 dark:border-gray-700" :disabled="isSending" + :maxlength="500" />Optional (below the form) show a counter:
- </form> + </form> + <div class="mt-1 text-xs text-gray-500 text-right"> + {{ newMessage.length }}/500 + </div>
🧹 Nitpick comments (25)
schemas/chat.ts (2)
52-53: Fix typo in validation message and make intent explicit."participantis" → "participant is". Also, "Exactly one participant…" reads clearer for 1:1 chat.
- .length(1, 'One participantis required for 1:1 chat'), + .length(1, 'Exactly one participant is required for 1:1 chat'),
8-12: Keep Message.content validation consistent with send path.
messageSchemacurrently allows any non-empty string (including whitespace-only and >500 chars). Align with the send schema to keep read/write validation consistent.- content: z.string().min(1, 'Message cannot be empty'), + content: z.string().transform((s) => s.trim()).min(1, 'Message cannot be empty').max(500),Note: If legacy data may exceed 500 chars, gate
.max(500)behind a safeParse at the API boundary instead of changing this canonical schema.server/utils/sse.ts (2)
7-15: Send an initial comment/heartbeat to open the stream and keep it alive.Some intermediaries buffer until a first chunk; periodic pings help long-lived connections.
export function setupSSE(event: H3Event) { setResponseHeaders(event, { 'Content-Type': 'text/event-stream', 'Cache-Control': 'no-cache, no-transform', Connection: 'keep-alive', 'X-Accel-Buffering': 'no', }) - return event.node.res + const res = event.node.res + // Kick-start the stream and set a ping; callers can clearInterval on close. + res.write(':ok\n\n') + return res }
5-5: In-memory pub/sub won’t scale across instances.If you deploy multiple Node processes/servers, SSE listeners connected to instance A won’t see publishes on instance B. Plan Redis (or similar) pub/sub for channel fanout; keep this module’s API stable so you can swap the backend later.
server/trpc/routers/chat.ts (7)
13-17: Guard type of session.profile.id or use a narrower context type.
Sessionfrom next-auth doesn’t haveprofileby default. If you added module augmentation, fine; otherwise this won’t typecheck.If not augmented, consider:
type Ctx = { session?: { profile?: { id?: string } } } const requireProfileId = (ctx: Ctx) => { const id = ctx.session?.profile?.id if (!id) throw new TRPCError({ code: 'UNAUTHORIZED' }) return id }
19-21: Prefer a locale-independent key sorter.
localeComparecan vary with locale; IDs are ASCII, so a simple comparator is clearer.-const pairKeyFor = (a: string, b: string) => - [a, b].sort((x, y) => ('' + x).localeCompare(y)).join('-') +const pairKeyFor = (a: string, b: string) => + (a < b ? `${a}-${b}` : `${b}-${a}`)
131-144: Reduce PII/log noise and enforce event type at compile time.
- Gate logs behind DEBUG.
- After switching
ChatEvent = z.infer<typeof chatEventSchema>, you can remove the casts and optionally usesatisfiesto type-check without widening.- console.log('Publishing to conversation:', conv.id) - publish(`conversation:${conv.id}`, { + if (process.env.DEBUG?.includes('chat')) { + console.debug('Publishing to conversation:', conv.id) + } + publish(`conversation:${conv.id}`, { type: 'message.created', conversationId: conv.id, messageId: msg.id, - } as ChatEvent) + }) @@ - console.log('Publishing to inbox:', other) - publish(`inbox:${other}`, { + if (process.env.DEBUG?.includes('chat')) { + console.debug('Publishing to inbox:', other) + } + publish(`inbox:${other}`, { type: 'conversation.updated', conversationId: conv.id, lastMessageId: msg.id, - } as ChatEvent) + } satisfies ChatEvent)
24-46: Data shape consistency between list and detail.
getConversationsreturnsparticipants: [{ profileId }, …]whilegetConversationreturnsa,bandmessageswithsender. Ensure the frontend is prepared for both shapes or standardize on one schema to reduce mapping logic.
71-90: Validate “other” profile existence before creating a conversation.Relying on FK to fail leads to 500s. Return NOT_FOUND if the profile doesn’t exist.
+ const exists = await prisma.profile.findUnique({ where: { id: other }, select: { id: true } }) + if (!exists) throw new TRPCError({ code: 'NOT_FOUND' })
103-112: Consider NOT_FOUND over FORBIDDEN for missing conversation.Access control is already enforced in the
whereclause; a truly missing result reads better as NOT_FOUND.
94-99: Remove unsafeas ChatEventcasts.Once
ChatEventis derived from the schema, the object literals will be checked without casts. For runtime safety, you canchatEventSchema.parse(evt)beforepublish.Example:
publish(channel, chatEventSchema.parse(evt))Also applies to: 132-136, 141-144
prisma/schema.prisma (1)
589-643: Minor: UpdatedAt default duplication on CalendarEvent.
updatedAt @updatedAtalready sets the value on updates; the@default(now())is fine for inserts but redundant. Optional cleanup only.components/chat/ChatList.vue (5)
28-31: Remove unnecessary currentUser dependency in getOtherParticipant.The backend guarantees receiver is the “other participant”. The currentUser check is redundant and causes extra recomputations when the session changes.
Apply this diff:
-function getOtherParticipant(conversation: any) { - if (!conversation?.receiver || !currentUser.value) return null - // Your backend already provides the 'receiver' which is the other participant - return conversation.receiver -} +function getOtherParticipant(conversation: any) { + // Backend provides 'receiver' as the other participant + return conversation?.receiver ?? null +}
87-89: Render a user-friendly error message.Rendering the raw error object will show “[object Object]”. Prefer error.message with a fallback.
- <div v-else-if="error" class="p-4 text-red-500"> - {{ error }} - </div> + <div v-else-if="error" class="p-4 text-red-500"> + {{ (error as any)?.message || 'Failed to load conversations.' }} + </div>
114-118: Improve avatar accessibility with descriptive alt text.Use the participant’s name for the alt attribute for better a11y.
- <img - v-if="getOtherParticipant(conversation)?.photo" - :src="getOtherParticipant(conversation)?.photo" - alt="Profile" - class="h-12 w-12 rounded-full object-cover" - /> + <img + v-if="getOtherParticipant(conversation)?.photo" + :src="getOtherParticipant(conversation)?.photo" + :alt="`Profile photo of ${getOtherParticipant(conversation)?.name || 'user'}`" + class="h-12 w-12 rounded-full object-cover" + />
15-24: Consider subscribing to the inbox SSE channel to refresh instantly (reduce reliance on polling).Given the backend exposes an inbox: channel, you can open an SSE connection here (without conversationId) and refresh on message.created or conversation.updated to reflect new conversations/unreads immediately. This aligns with “messages appear instantly” and “in-app notifications” acceptance criteria.
Example (add alongside the existing interval; onUnmounted cleanup included):
let inboxEs: EventSource | null = null onMounted(() => { if (!currentUser.value?.id) return inboxEs = new EventSource('/api/chat/stream') // server derives inbox:<profileId> inboxEs.addEventListener('message.created', () => refresh()) inboxEs.addEventListener('conversation.updated', () => refresh()) }) onUnmounted(() => { inboxEs?.close() inboxEs = null })
34-43: Deduplicate getInitials/formatTime across components.Both ChatList and ChatConversation define identical helpers. Extract into a composable or utility to keep logic in one place and improve testability.
Example:
// composables/useFormatters.ts export function useFormatters() { const getInitials = (name?: string) => !name ? '?' : name.split(' ').map(p => p[0]).join('').toUpperCase().slice(0, 2) const formatTime = (ts?: string | Date) => { /* …same logic… */ } return { getInitials, formatTime } }Then:
const { getInitials, formatTime } = useFormatters()Also applies to: 45-74
server/api/chat/stream.get.ts (2)
31-41: Unify cleanup to avoid duplicate end/unsubscribe and ensure hb is always cleared.There are multiple listeners on 'close' and 'aborted', and hb is only cleared on 'close'. Consolidate into a single idempotent cleanup.
- const unsubscribe = subscribe(channel, res) - console.log('Subscribed to channel:', channel) - const close = () => { - unsubscribe() - res.end() - } - event.node.req.on('close', close) - event.node.req.on('aborted', close) - const hb = setInterval(() => res.write(':hb\n\n'), 25000) - event.node.req.on('close', () => clearInterval(hb)) + const unsubscribe = subscribe(channel, res) + console.log('Subscribed to channel:', channel) + const hb = setInterval(() => res.write(':hb\n\n'), 25000) + let cleaned = false + const cleanup = () => { + if (cleaned) return + cleaned = true + clearInterval(hb) + try { unsubscribe() } catch {} + try { res.end() } catch {} + } + event.node.req.on('close', cleanup) + event.node.req.on('aborted', cleanup)
7-14: Reduce production log verbosity and avoid PII in logs.Logging profileId and raw queries in production can leak PII. Gate logs behind a debug flag or remove entirely.
- console.log('SSE Connection started') + if (process.env.NODE_ENV !== 'production') console.log('SSE Connection started') ... - console.log('👤 Profile ID:', profileId) - console.log('📝 Query:', q) + if (process.env.NODE_ENV !== 'production') { + console.log('👤 Profile ID:', profileId) + console.log('📝 Query:', q) + } ... - console.log('Channel:', channel) + if (process.env.NODE_ENV !== 'production') console.log('Channel:', channel)components/chat/ChatConversation.vue (6)
37-43: Fix log message and ensure consistent cleanup.Minor nit: The log mentions “ChatList” in ChatConversation. Update to avoid confusion.
- console.log('🔌 Closing ChatList SSE connection') + console.log('🔌 Closing ChatConversation SSE connection')
138-156: Guard against oversize messages and avoid double refresh if SSE echoes the sent message.
- Enforce the 500-char limit in sendMessage too.
- Consider skipping the post-send refresh if SSE reliably delivers your own message back (prevents duplicate reloads).
async function sendMessage() { - if (!newMessage.value.trim() || isSending.value) return + const content = newMessage.value.trim() + if (!content || isSending.value) return + if (content.length > 500) { + // Optionally surface a toast here + return + } try { isSending.value = true await $client.chat.sendMessage.mutate({ conversationId: props.conversationId, - content: newMessage.value.trim(), + content, }) newMessage.value = '' - await refresh() + // If SSE publishes message.created to this conversation for the sender too, + // rely on SSE to refresh. Otherwise, uncomment the next line: + // await refresh()
60-67: Type useAsyncData to avoid casting later.Typing the data removes the need for as ConversationData.
-const { - data: conversation, - pending: isLoading, - error, - refresh, -} = useAsyncData(`conversation-${props.conversationId}`, () => - $client.chat.getConversation.query({ conversationId: props.conversationId }) -) +const { + data: conversation, + pending: isLoading, + error, + refresh, +} = useAsyncData<ConversationData>( + `conversation-${props.conversationId}`, + () => $client.chat.getConversation.query({ conversationId: props.conversationId }) +)
79-86: Avoid deep watch on messages; watch the length or last id instead.Deep watchers can be noisy. Scrolling on new message is satisfied by watching collection length.
-watch( - () => conversation.value?.messages, - () => { - scrollToBottom() - }, - { deep: true } -) +watch( + () => conversation.value?.messages?.length, + () => scrollToBottom() +)
188-192: Improve avatar alt text accessibility.- <img + <img v-if="otherParticipant?.photo" :src="otherParticipant?.photo" - alt="Profile" + :alt="`Profile photo of ${otherParticipant?.name || 'user'}`" class="h-8 w-8 rounded-full object-cover" />
14-21: Handle prop changes for conversationId (route reuse).If this component instance is reused across route changes, the EventSource will continue listening to the old conversation. Add a watcher to recreate the stream when props.conversationId changes.
Example:
watch(() => props.conversationId, (id, old) => { if (id === old) return eventSource?.close() eventSource = new EventSource(`/api/chat/stream?conversationId=${id}`) // reattach listeners... }, { flush: 'post' })
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (9)
cli/prisma/schema.prisma(5 hunks)components/chat/ChatConversation.vue(1 hunks)components/chat/ChatList.vue(2 hunks)pages/chat/index.vue(0 hunks)prisma/schema.prisma(5 hunks)schemas/chat.ts(1 hunks)server/api/chat/stream.get.ts(1 hunks)server/trpc/routers/chat.ts(1 hunks)server/utils/sse.ts(1 hunks)
💤 Files with no reviewable changes (1)
- pages/chat/index.vue
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: ci
- GitHub Check: test
🔇 Additional comments (5)
components/chat/ChatList.vue (2)
10-10: Consistent loading flag naming (LGTM).Renaming pending to isLoading improves readability and keeps naming consistent with ChatConversation.vue.
26-32: Confirmed:getConversationsreturns a fullreceiverobject withnameandphoto
- In server/trpc/routers/chat.ts, the
getConversationsquery uses
include: { lastMessage: true, a: true, b: true }
then assignsreceiverfromc.aorc.b, which are fullProfileobjects (including thenameandphotofields).server/api/chat/stream.get.ts (1)
1-42: Please ignore the Prisma‐import suggestion in this handlerThe SSE endpoint in
server/api/chat/stream.get.tsdoes not perform any direct database queries or “membership checks” itself—those are encapsulated within yoursubscribe()implementation (imported via#imports). There is noprismaimport in this file, and no compilation issue will arise here from the Prisma client path.If you’d like to verify Prisma imports, make sure your actual DB-access code (e.g. your Nuxt plugin or middleware) is using the client exported in
server/prisma.ts:
server/prisma.tsexports the client as
export const prisma = new PrismaClient({...})- Your plugins or middleware should import it via your project alias (e.g.
import { prisma } from '~/server/prisma')No changes are needed in
stream.get.ts.Likely an incorrect or invalid review comment.
components/chat/ChatConversation.vue (2)
18-31: Confirm SSE event naming; default onmessage vs. named events.If the server publishes named events (event: message.created), onmessage won’t fire. In that case, switch to addEventListener('message.created', …).
Fallback pattern:
eventSource!.addEventListener('message.created', (e: MessageEvent) => { const data = JSON.parse(e.data) if (data.conversationId === props.conversationId) refresh() })If your server writes only default events (no event: header), your current onmessage is fine.
72-77: otherParticipant derivation (LGTM).The aId/bId approach is clear and avoids brittle participant filtering logic.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
schemas/chat.ts (1)
82-97: Schema-derived ChatEvent removes drift—nice.Switching to z.infer and making lastMessageId optional resolves previous comments and eliminates casts downstream. LGTM.
server/trpc/routers/chat.ts (1)
70-99: New conversation flow: looks solid and idempotent.pairKeyFor prevents duplicates; publishing to the other user’s inbox without a bogus lastMessageId aligns with the updated schema. LGTM.
prisma/schema.prisma (2)
645-665: 1:1 lastMessage relation enforced—good.Making lastMessageId unique and wiring the named relation matches the intended one-to-one. This aligns with earlier feedback.
667-682: Back-relation cardinality fixed.lastMessageFor is now singular/optional, consistent with the 1:1 mapping. ✅
🧹 Nitpick comments (8)
schemas/chat.ts (2)
52-53: Fix typo in validation message."participantis" → "participant is" (or "Exactly one participant is required for 1:1 chat.").
- .length(1, 'One participantis required for 1:1 chat'), + .length(1, 'Exactly one participant is required for 1:1 chat'),
60-67: Align read schema with write rules (500 grapheme cap).sendMessageSchema enforces 1–500 graphemes, but messageSchema only enforces non-empty. Tighten messageSchema to reflect persisted constraints so consumers can rely on a stable contract.
export const messageSchema = z.object({ id: z.string(), conversationId: z.string(), senderId: z.string(), - content: z.string().min(1, 'Message cannot be empty'), + content: z + .string() + .transform((s) => s.trim()) + .refine( + (s) => graphemeCount(s) >= 1 && graphemeCount(s) <= 500, + 'Message must be 1–500 characters.' + ), createdAt: z.date().or(z.string()), updatedAt: z.date().or(z.string()), isDeleted: z.boolean().default(false), })If you prefer to avoid transform at this layer, at least add a max-grapheme check to prevent >500 payloads from slipping in when schemas are used for parsing DB responses.
server/api/chat/stream.get.ts (3)
22-37: Defer SSE header setup until after auth checks.You set SSE headers before returning 401. Minor, but better to short-circuit unauthenticated requests first to avoid unnecessary header writes.
-export default defineEventHandler(async (event) => { - console.log('SSE Connection started') - const res = setupSSE(event) +export default defineEventHandler(async (event) => { + console.log('SSE Connection started') const q = getQuery(event) const session = await getServerSession(event) const profileId = session?.profile?.id + const res = setupSSE(event)Alternatively, move setupSSE(event) just after the session and channel validations (right before subscribe).
46-51: Return 400 for missing/invalid channel.This is a bad request rather than unauthorized. Consider 400 to make debugging easier.
- setResponseStatus(event, 401) + setResponseStatus(event, 400)
62-67: Send an initial comment or event to flush the stream.Some proxies/clients buffer until data arrives. Emit a tiny prelude after subscribing.
const unsubscribe = subscribe(channel, res) console.log('Subscribed to channel:', channel) + // Flush an initial event/comment to open the stream for certain proxies + res.write(': connected\n\n')server/trpc/routers/chat.ts (3)
24-46: Minimize data surface: select only needed fields.You currently include full Profile and Message records. Prefer select to limit PII and payload. Also, participants array can include lightweight fields used by the UI.
- const convs = await prisma.conversation.findMany({ + const convs = await prisma.conversation.findMany({ where: { OR: [{ aId: me }, { bId: me }] }, orderBy: { updatedAt: 'desc' }, - include: { lastMessage: true, a: true, b: true }, + include: { + lastMessage: { + select: { id: true, content: true, createdAt: true, senderId: true }, + }, + a: { select: { id: true, name: true, username: true, photo: true } }, + b: { select: { id: true, name: true, username: true, photo: true } }, + }, })Then map participants to expose only what the client needs:
- participants: [{ profileId: c.aId }, { profileId: c.bId }], + participants: [ + { profileId: c.a.id, name: c.a.name ?? undefined, username: c.a.username ?? undefined, photo: c.a.photo ?? undefined }, + { profileId: c.b.id, name: c.b.name ?? undefined, username: c.b.username ?? undefined, photo: c.b.photo ?? undefined }, + ],
51-67: Apply the same minimal selection for getConversation.Limit included sender and participants fields to what the chat UI actually renders.
- const c = await prisma.conversation.findFirst({ + const c = await prisma.conversation.findFirst({ where: { id: input.conversationId, OR: [{ aId: me }, { bId: me }] }, include: { messages: { orderBy: { createdAt: 'asc' }, take: 200, - include: { sender: true }, + include: { + sender: { select: { id: true, name: true, username: true, photo: true } }, + }, }, - lastMessage: true, - a: true, - b: true, + lastMessage: { + select: { id: true, content: true, createdAt: true, senderId: true }, + }, + a: { select: { id: true, name: true, username: true, photo: true } }, + b: { select: { id: true, name: true, username: true, photo: true } }, }, })
117-118: Avoid redundant trimming.content was already trimmed by sendMessageSchema’s transform; you can drop the second trim.
- content: input.content.trim(), + content: input.content,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (6)
cli/prisma/schema.prisma(5 hunks)prisma/schema.prisma(5 hunks)schemas/chat.ts(1 hunks)server/api/chat/stream.get.ts(1 hunks)server/trpc/routers/chat.ts(1 hunks)server/utils/sse.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- server/utils/sse.ts
- cli/prisma/schema.prisma
🧰 Additional context used
🧬 Code graph analysis (2)
server/trpc/routers/chat.ts (4)
server/trpc/init.ts (2)
router(32-32)publicProcedure(31-31)server/prisma.ts (1)
prisma(5-8)schemas/chat.ts (2)
createConversationSchema(49-53)sendMessageSchema(58-67)server/utils/sse.ts (1)
publish(23-33)
server/api/chat/stream.get.ts (3)
server/prisma.ts (1)
prisma(5-8)server/utils/sse.ts (2)
setupSSE(7-15)subscribe(17-21)schemas/chat.ts (1)
ChatChannel(97-97)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: ci
- GitHub Check: test
🔇 Additional comments (5)
schemas/chat.ts (1)
60-78: Grapheme-aware length validation: great improvement.Using Intl.Segmenter with a code point fallback cleanly aligns with the “including emojis” acceptance criterion. Nice.
server/trpc/routers/chat.ts (2)
112-131: Good use of interactive transaction for create+update atomicity.This fixes the interleaving window and keeps lastMessageId consistent. Nice.
133-146: Event payloads match schema and channels—LGTM.message.created to conversation and conversation.updated to the recipient’s inbox are consistent with ChatEvent and acceptance criteria for instant updates.
prisma/schema.prisma (2)
637-643: updatedAt has both @default(now()) and @updatedat—verify compatibility.In many Prisma versions, @updatedat already sets the create-time value and forbids @default on the same field. Please run prisma validate to confirm; if flagged, drop the @default for consistency with other models.
- updatedAt DateTime @default(now()) @updatedAt + updatedAt DateTime @updatedAtIf your current Prisma version allows both, consider removing @default(now()) anyway to match the rest of the schema (e.g., Event, Conversation, Message).
200-204: New relations on Profile are coherent.Calendars, conversations, and messages are correctly linked and indexed. No issues spotted.
cli/prisma/schema.prisma
Outdated
| pairKey String @unique // `${min(aId,bId)}-${max(aId,bId)}` | ||
| aId String | ||
| bId String | ||
| aLastSeenAt DateTime @default(now()) |
There was a problem hiding this comment.
For now 'pairKey' helps prevent duplicate conversations between users also used to find existing ones i can try to come up with a newer solution if you would prefer here
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
server/trpc/routers/chat.ts (1)
94-99: Past comment addressed: no empty lastMessageId on new conv eventsYou’ve removed lastMessageId from the new conversation publish payload. This resolves the previously flagged schema mismatch.
🧹 Nitpick comments (5)
server/trpc/routers/chat.ts (4)
19-38: Conversations query and view model are solid; consider pagination and consistent shape
- Query correctly scopes to the current user and sorts by updatedAt desc.
- Returning receiver is convenient for the UI.
- Optional: add pagination (cursor/limit) to avoid returning an unbounded list as usage scales.
- Optional: align the returned shape with createConversation/getConversation for consistency (e.g., always include receiver or provide a shared DTO).
43-60: Membership filter is correct; consider findUnique + explicit checkfindFirst with OR guards access in a single query. If you prefer stricter semantics, fetch by id via findUnique and then 403 if me not in {aId,bId}. Current approach is acceptable.
64-99: Conversation creation flow LGTM; also notify the creator’s inboxDedup via pairKey and the subsequent publish to the other user’s inbox look good. For symmetry and instant updates on the creator’s Chat list, publish to the creator’s inbox as well.
Apply this diff:
publish(`inbox:${other}`, { type: 'conversation.updated', conversationId: created.id, }) + publish(`inbox:${me}`, { + type: 'conversation.updated', + conversationId: created.id, + })
133-146: Remove or gate server-side console logsconsole.log in server paths will spam logs in production and can leak identifiers. Prefer debug-level logging behind an env flag.
Apply this diff:
- console.log('Publishing to conversation:', conv.id) publish(`conversation:${conv.id}`, { type: 'message.created', conversationId: conv.id, messageId: msg.id, }) - - const other = conv.aId === me ? conv.bId : conv.aId - console.log('Publishing to inbox:', other) + const other = conv.aId === me ? conv.bId : conv.aId publish(`inbox:${other}`, { type: 'conversation.updated', conversationId: conv.id, lastMessageId: msg.id, })components/chat/ChatConversation.vue (1)
195-200: Improve avatar alt text for accessibilityMatch ChatList’s descriptive alt text for screen readers.
Apply this diff:
- alt="Profile" + :alt="`Profile photo of ${otherParticipant?.name || 'user'}`"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (6)
components/chat/ChatConversation.vue(4 hunks)components/chat/ChatList.vue(3 hunks)schemas/chat.ts(1 hunks)server/api/chat/stream.get.ts(1 hunks)server/trpc/routers/chat.ts(1 hunks)server/utils/sse.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- server/utils/sse.ts
- schemas/chat.ts
- server/api/chat/stream.get.ts
🧰 Additional context used
🧬 Code graph analysis (1)
server/trpc/routers/chat.ts (4)
server/trpc/init.ts (2)
router(32-32)publicProcedure(31-31)server/prisma.ts (1)
prisma(5-8)schemas/chat.ts (2)
createConversationSchema(49-53)sendMessageSchema(58-67)server/utils/sse.ts (1)
publish(32-49)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: test
- GitHub Check: ci
🔇 Additional comments (8)
components/chat/ChatList.vue (3)
35-36: Receiver resolution looks goodReturning conversation.receiver aligns with the router’s getConversations shape and simplifies the UI.
92-93: Better error message UXGood fallback to a friendly message while still surfacing error.message when present.
120-121: Nice accessibility improvement on avatar alt textDescriptive alt text improves screen reader experience.
server/trpc/routers/chat.ts (3)
9-13: Verify session typing for ctx.session.profile.idrequireProfileId reads ctx.session.profile.id but Session from next-auth doesn’t include profile by default. Ensure your next-auth module augmentation (or your TRPC ctx type) reflects profile: { id: string } to avoid unsound casts.
If not already done, add a module augmentation similar to:
// next-auth.d.ts declare module 'next-auth' { interface Session { profile?: { id: string } } }
15-15: Deterministic pair key LGTMLexicographic pairKey keeps a/b stable and enables a unique index; the aId/bId derivation then matches the sorted order.
103-131: Atomic send + conversation update LGTMInteractive transaction ensures lastMessageId and lastSeenAt remain consistent under concurrent sends. Good inclusion of sender for immediate UI use.
components/chat/ChatConversation.vue (2)
16-31: SSE setup/teardown is cleanReusing a single EventSource, refreshing on message.created, and closing on unmount are all correct.
86-92: Auto-scroll watch is targeted and efficientWatching only messages.length avoids deep watchers and is performant.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
components/chat/ChatList.vue (2)
28-31: Fix SSE listener: server emits only default "message" events; named event listeners never fireThe SSE publisher writes only "data:" lines with no "event:" field, so addEventListener('conversation.updated', …) will never trigger. Use onmessage and parse evt.data.type. Also add onerror for observability.
Apply this minimal diff:
- inboxEs.addEventListener('conversation.updated', () => { - refresh() - }) + inboxEs.onmessage = (evt) => { + try { + const data = JSON.parse(evt.data) + if (data?.type === 'conversation.updated') { + refresh() + } + } catch (e) { + console.warn('Invalid inbox SSE payload', e) + } + } + inboxEs.onerror = (err) => { + console.error('Inbox SSE error', err) + }
24-37: Open/close the inbox SSE reactively when auth changesIf currentUser.id arrives after mount or changes (login/logout, switch account), the stream is not opened/closed accordingly. Wrap EventSource creation in a function and watch currentUser.value?.id to manage lifecycle.
Proposed refactor:
+function openInboxStream() { + if (inboxEs) return + inboxEs = new EventSource('/api/chat/stream') + inboxEs.onmessage = (evt) => { + try { + const data = JSON.parse(evt.data) + if (data?.type === 'conversation.updated') { + refresh() + } + } catch (e) { + console.warn('Invalid inbox SSE payload', e) + } + } + inboxEs.onerror = (err) => { + console.error('Inbox SSE error', err) + } +} + onMounted(() => { - if (currentUser.value?.id) { - inboxEs = new EventSource('/api/chat/stream') - - inboxEs.addEventListener('conversation.updated', () => { - refresh() - }) - } + if (currentUser.value?.id) openInboxStream() + + watch( + () => currentUser.value?.id, + (id, prev) => { + if (id && !prev) openInboxStream() + if (!id && inboxEs) { + inboxEs.close() + inboxEs = null + } + }, + { immediate: false } + ) onUnmounted(() => { inboxEs?.close() inboxEs = null }) })Also applies to: 21-21
🧹 Nitpick comments (6)
components/chat/ChatList.vue (1)
146-150: Minor: avoid repeated getLastMessage calls per rowYou call getLastMessage(conversation) twice. Cache locally in the template scope or compute once in script for readability and micro-optimization.
Example (template-scoped v-slot variable via a small wrapper):
- <span v-if="getLastMessage(conversation)?.createdAt" class="text-xs text-gray-500"> - {{ formatTime(getLastMessage(conversation).createdAt) }} - </span> + <span v-if="(last => last?.createdAt)(getLastMessage(conversation))" class="text-xs text-gray-500"> + {{ (last => last ? formatTime(last.createdAt) : '')(getLastMessage(conversation)) }} + </span>Or move last = getLastMessage(conversation) into a render helper that returns { content, createdAt }.
Also applies to: 154-155
server/trpc/routers/chat.ts (3)
51-70: Optional: combine membership check into the query to prevent existence leakReturning UNAUTHORIZED after fetching an existing conversation can leak existence to non-participants. Consider filtering by membership in the where clause and return NOT_FOUND if absent.
- const c = await prisma.conversation.findFirst({ - where: { id: input.conversationId }, + const c = await prisma.conversation.findFirst({ + where: { id: input.conversationId, OR: [{ aId: me }, { bId: me }] }, include: { messages: { orderBy: { createdAt: 'asc' }, take: 200, include: { sender: true }, }, a: true, b: true, }, }) - if (!c) throw new TRPCError({ code: 'NOT_FOUND' }) - if (c.aId !== me && c.bId !== me) { - throw new TRPCError({ code: 'UNAUTHORIZED' }) - } + if (!c) throw new TRPCError({ code: 'NOT_FOUND' })
124-129: Minor: redundant trim after schema transformsendMessageSchema already trims and grapheme-validates. You can drop the extra .trim() here.
- content: input.content.trim(), + content: input.content,
142-155: Reduce console noise or guard logsConsole logs are useful during development but noisy in production. Consider removing or guarding behind an env flag.
- console.log('Publishing to conversation:', conv.id) + if (process.env.NODE_ENV !== 'production') { + console.log('Publishing to conversation:', conv.id) + } @@ - console.log('Publishing to inbox:', other) + if (process.env.NODE_ENV !== 'production') { + console.log('Publishing to inbox:', other) + }prisma/schema.prisma (2)
645-663: Optional: add an index on updatedAt for faster sorting in large inboxesIf conversation counts get big, indexing updatedAt can help ORDER BY queries.
model Conversation { id String @id @default(cuid()) pairKey String @unique // `${min(aId,bId)}-${max(aId,bId)}` aId String bId String aLastSeenAt DateTime @default(now()) bLastSeenAt DateTime @default(now()) @@ @@index([aId]) @@index([bId]) + @@index([updatedAt]) }
615-643: Optional: updatedAt doesn’t need @default(now())Prisma sets @updatedat on create and update. The extra @default(now()) is redundant and can be dropped to simplify migrations.
- updatedAt DateTime @default(now()) @updatedAt + updatedAt DateTime @updatedAt
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
components/chat/ChatList.vue(4 hunks)prisma/schema.prisma(4 hunks)server/api/chat/stream.get.ts(1 hunks)server/trpc/routers/chat.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- server/api/chat/stream.get.ts
🧰 Additional context used
🧬 Code graph analysis (1)
server/trpc/routers/chat.ts (4)
server/trpc/init.ts (2)
router(32-32)publicProcedure(31-31)server/prisma.ts (1)
prisma(5-8)schemas/chat.ts (2)
createConversationSchema(49-53)sendMessageSchema(58-67)server/utils/sse.ts (1)
publish(32-49)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: test
- GitHub Check: ci
🔇 Additional comments (11)
components/chat/ChatList.vue (3)
126-127: Nice accessibility and error UX improvementsAlt text now describes the profile photo, and error rendering shows a message fallback. Good improvements.
Also applies to: 98-99
21-21: Housekeeping: null out the EventSource after close (already done)Good practice to avoid dangling references.
7-12: Confirmed server-side ordering matchesgetLastMessageassumptionThe
getConversationsresolver inserver/trpc/routers/chat.tsalready applies:messages: { orderBy: { createdAt: 'desc' }, take: 1, include: { sender: true }, },(around lines 27–31) which guarantees that
conversation.messages[0]is the most recent message. No further changes are needed—just ensure this remains in sync if the server query is ever modified.server/trpc/routers/chat.ts (5)
9-13: Auth guard helper is concise and correctCentralizing the UNAUTHORIZED check via requireProfileId keeps procedures lean.
21-46: Conversations query shape and mapping look good
- Includes a and b profiles plus latest message with sender; sorted by updatedAt desc.
- Returns a receiver field tailored to the caller.
This aligns with the list UI and avoids extra round trips.
122-140: Solid: interactive transaction for message + last-seenWrapping message creation and last-seen update in a single transaction keeps state consistent and ensures updatedAt bumps for ordering.
104-109: Event payloads are lean and consistentPublishing conversation.updated without extraneous fields keeps the client-side refresh logic straightforward.
Also applies to: 151-155
21-24: Performance note: ordering by updatedAt is compatible with your update patternSince you update aLastSeenAt/bLastSeenAt on send, updatedAt advances; the list will sort properly by recent activity as required by the UX.
prisma/schema.prisma (3)
645-663: Conversation model is minimal and correct for 1:1
- Unique pairKey enforces dedup.
- aId/bId with a/b relations are clear.
- Ordering by updatedAt on the app side will work as updatedAt mutates when you update lastSeen.
665-679: Message model and indices look goodIndex on (conversationId, createdAt) supports fetching latest messages efficiently.
200-204: Nice: back-relations for chat entities on ProfileKeeping messages and ConversationAsA/B anchored on Profile simplifies traversal and permission checks.
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (7)
components/chat/ChatList.vue (4)
61-70: Harden getInitials against whitespace and multi‑space namesHandles names like " " and collapses multiple spaces; keeps at most two initials.
function getInitials(name: string | undefined) { - if (!name) return '?' - return name - .split(' ') - .map((part) => part[0]) - .join('') - .toUpperCase() - .substring(0, 2) + const n = name?.trim() + if (!n) return '?' + return n + .split(/\s+/) + .filter(Boolean) + .slice(0, 2) + .map((part) => part.charAt(0)) + .join('') + .toUpperCase() }
73-101: Defensive date parsingAvoid rendering "Invalid Date" in edge cases (bad or server‑serialized values).
function formatTime(timestamp: string | Date) { if (!timestamp) return '' const date = new Date(timestamp) + if (isNaN(date.getTime())) return '' const now = new Date()
7-11: Type the conversation shape to avoidanyTighten types to match server payload and prevent runtime slips.
-//get the last Message from messages array -const getLastMessage = (conversation: any) => { +// get the last message from messages array +type ConversationForList = { + id: string + messages?: Array<{ id: string; content: string; createdAt: string | Date }> +} +const getLastMessage = (conversation: ConversationForList) => { if (!conversation.messages?.length) return null return conversation.messages[0] }
163-167: Avoid double compute of last message per rowCall getLastMessage once per conversation row (micro‑opt). E.g., extract a small child component that computes it once, or precompute a map keyed by id.
Also applies to: 171-172
server/trpc/routers/chat.ts (1)
143-152: Remove console noise from hot pathAvoid leaking ids in logs; rely on observability if needed.
- console.log('Publishing to conversation:', conv.id) publish(`conversation:${conv.id}`, { type: 'message.created', conversationId: conv.id, messageId: msg.id, }) - const other = conv.aId === me ? conv.bId : conv.aId - console.log('Publishing to inbox:', other) + const other = conv.aId === me ? conv.bId : conv.aId publish(`inbox:${other}`, { type: 'conversation.updated', conversationId: conv.id, })cli/prisma/schema.prisma (2)
645-663: Add composite indexes to speed up inbox listingYour query filters by aId/bId and sorts by updatedAt; add covering indexes.
model Conversation { id String @id @default(cuid()) pairKey String @unique // `${min(aId,bId)}-${max(aId,bId)}` aId String bId String aLastSeenAt DateTime @default(now()) bLastSeenAt DateTime @default(now()) ... - @@index([aId]) - @@index([bId]) + @@index([aId]) + @@index([bId]) + @@index([aId, updatedAt]) + @@index([bId, updatedAt]) }
645-663: Optional: prevent self‑conversations at DB levelIf desired, add a check constraint via a raw SQL migration (Prisma lacks model‑level @@check).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
cli/prisma/schema.prisma(4 hunks)components/chat/ChatConversation.vue(4 hunks)components/chat/ChatList.vue(4 hunks)server/trpc/routers/chat.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- components/chat/ChatConversation.vue
🧰 Additional context used
🧬 Code graph analysis (1)
server/trpc/routers/chat.ts (4)
server/trpc/init.ts (2)
router(32-32)publicProcedure(31-31)server/prisma.ts (1)
prisma(5-8)schemas/chat.ts (2)
createConversationSchema(49-53)sendMessageSchema(58-67)server/utils/sse.ts (1)
publish(32-49)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: test
- GitHub Check: ci
🔇 Additional comments (2)
server/trpc/routers/chat.ts (1)
19-46: LGTM: stable mapping and last message includeQuery shape matches the frontend’s expectations (messages[0] newest).
cli/prisma/schema.prisma (1)
200-204: LGTM: relations for chat models attached to Profile and DanceStyleRelations are consistent with Conversation/Message and won’t break existing maps.
Also applies to: 368-373
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (4)
cli/prisma/schema.prisma (1)
615-643: Minor: drop redundant default on updatedAt
updatedAtalready auto-sets;@default(now())is superfluous.Apply:
- updatedAt DateTime @default(now()) @updatedAt + updatedAt DateTime @updatedAtserver/trpc/routers/chat.ts (3)
149-152: Include messageId in conversation.updated to let inbox update without refetchHelps “appears instantly” without an extra round-trip.
Apply:
publish(`inbox:${other}`, { type: 'conversation.updated', conversationId: conv.id, + lastMessageId: msg.id, })And similarly when creating a new conversation, omit
lastMessageId(already done).Also applies to: 102-106
140-145: Remove noisy logs or guard themAvoid server log spam in prod.
Apply:
- console.log('Publishing to conversation:', conv.id) + if (process.env.NODE_ENV !== 'production') { + console.log('Publishing to conversation:', conv.id) + } ... - console.log('Publishing to inbox:', other) + if (process.env.NODE_ENV !== 'production') { + console.log('Publishing to inbox:', other) + }Also applies to: 148-149
85-91: Nit: narrow includes on a/b for existing conversation fetchYou already narrowed elsewhere; make it consistent.
Apply:
- const existing = await prisma.conversation.findUnique({ - where: { pairKey: key }, - include: { a: true, b: true }, - }) + const existing = await prisma.conversation.findUnique({ + where: { pairKey: key }, + include: { + a: { select: { id: true, name: true, photo: true } }, + b: { select: { id: true, name: true, photo: true } }, + }, + })
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
cli/prisma/schema.prisma(4 hunks)components/chat/ChatList.vue(4 hunks)prisma/schema.prisma(4 hunks)server/trpc/routers/chat.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- prisma/schema.prisma
- components/chat/ChatList.vue
🧰 Additional context used
🧬 Code graph analysis (2)
server/trpc/routers/chat.ts (4)
server/trpc/init.ts (2)
router(32-32)publicProcedure(31-31)server/prisma.ts (1)
prisma(5-8)schemas/chat.ts (2)
createConversationSchema(49-53)sendMessageSchema(58-67)server/utils/sse.ts (1)
publish(32-49)
cli/prisma/schema.prisma (1)
cli/calendars/ical_import.ts (1)
saveCalendarData(153-340)
🔇 Additional comments (1)
server/trpc/routers/chat.ts (1)
34-45: I need the snippet to confirm what fields the UI consumes inparticipants. Please provide the output of the above command so I can verify if minimal profile info is required in the API response.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
prisma/schema.prisma (3)
589-613: Sync issue resolved: Calendar model matches CLI schema.The Calendar model is identical to the CLI version, maintaining consistency.
645-666: Schema sync maintained: Conversation model is consistent.The Conversation model matches the CLI schema with proper 1:1 lastMessage relationship enforcement.
668-682: Message model properly synchronized between schemas.The Message model with the 500-character limit and proper relations is consistent across both schemas.
🧹 Nitpick comments (2)
cli/prisma/schema.prisma (1)
615-643: Consider adding indexes for common query patterns.The CalendarEvent model is well-structured, but consider adding an index on
eventIdif you frequently query calendar events by their associated event.@@unique([calendarId, providerItemId]) @@index([startDate]) @@index([calendarId, startDate]) @@index([facebookId]) + @@index([eventId])server/trpc/routers/chat.ts (1)
145-153: Consider removing or converting console.log statements to structured logging.Debug console.log statements should be removed or replaced with a proper logging framework before production deployment.
- console.log('Publishing to conversation:', conv.id) publish(`conversation:${conv.id}`, { type: 'message.created', conversationId: conv.id, messageId: msg.id, }) const other = conv.aId === me ? conv.bId : conv.aId - console.log('Publishing to inbox:', other) publish(`inbox:${other}`, {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
cli/prisma/schema.prisma(4 hunks)prisma/schema.prisma(4 hunks)server/trpc/routers/chat.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
server/trpc/routers/chat.ts (4)
server/trpc/init.ts (2)
router(32-32)publicProcedure(31-31)server/prisma.ts (1)
prisma(5-8)schemas/chat.ts (2)
createConversationSchema(49-53)sendMessageSchema(58-67)server/utils/sse.ts (1)
publish(32-49)
prisma/schema.prisma (1)
cli/calendars/ical_import.ts (1)
saveCalendarData(153-340)
cli/prisma/schema.prisma (1)
cli/calendars/ical_import.ts (2)
saveCalendarData(153-340)tx(291-311)
🔇 Additional comments (8)
cli/prisma/schema.prisma (3)
200-203: LGTM! Chat-related relations properly added to Profile model.The new relations for calendars, conversations, and messages are correctly defined with appropriate naming conventions and back-references.
589-613: LGTM! Calendar model is well-structured.The Calendar model properly tracks synchronization state and event counts. The composite unique constraint on
[profileId, url]prevents duplicate calendars per profile.
668-682: LGTM! Message model properly enforces the 500-character limit.The database-level constraint using
@db.VarChar(500)aligns with the acceptance criteria and prevents bypassing application-level validation.prisma/schema.prisma (1)
200-203: LGTM! Chat relations added consistently with CLI schema.The Profile model extensions match the CLI schema exactly.
server/trpc/routers/chat.ts (4)
19-46: LGTM! Efficient conversation list query with proper field selection.The implementation correctly uses the
lastMessagerelation to avoid N+1 queries and limits exposed fields to only what's needed (id, name, photo).
49-69: LGTM! Proper authorization and data filtering in getConversation.The query correctly filters by participant at the database level and returns NOT_FOUND for unauthorized access, preventing resource enumeration attacks.
72-109: LGTM! Robust conversation creation with deduplication.The implementation correctly:
- Validates participant existence
- Prevents self-conversations
- Uses deterministic aId/bId assignment
- Handles deduplication via pairKey
- Publishes SSE events for real-time updates
112-159: LGTM! Atomic message creation with proper transaction handling.The implementation correctly uses a transaction to atomically create the message and update the conversation's
lastMessageIdand last-seen timestamps, ensuring data consistency.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (6)
composables/useProfile.ts (1)
32-32: Normalize claimed to a boolean to avoid undefined statesMap as a strict boolean so downstream gating is predictable.
- result.claimed = profile.claimed + result.claimed = Boolean(profile.claimed)components/ProfileCard.vue (2)
10-39: Avoid double conversation creation; surface failures to the userAdd a pending guard and user-facing error toast. This also lets you disable the button.
const currentUser = computed(() => session.value?.profile) const handleContact = async () => { + if (pending.value) return if (!profile.claimed) { - console.error('This profile is not clalimed') + console.error('This profile is not claimed') toast.error(`You can't message this profile yet.`) return } // Check if a user is logged in if (!currentUser.value) { return navigateTo('/login') } // Check if the user is trying to message themself if (currentUser.value.id === profile.id) { console.error("You can't message yourself.") return } //else start a conversation try { + pending.value = true const conversation = await $client.chat.createConversation.mutate({ participantIds: [profile.id], }) if (conversation?.id) { navigateTo(`/chat/${conversation.id}`) } } catch (error) { console.error('Failed to create or get conversation', error) + toast.error('Unable to start a conversation. Please try again.') + } finally { + pending.value = false } } + +const pending = ref(false)
12-12: Fix typo in log message“clalimed” → “claimed”.
- console.error('This profile is not clalimed') + console.error('This profile is not claimed')components/profile/ProfileLayout.vue (3)
53-81: Guard against rapid clicks; add user-facing error feedbackMirror the pending guard and toast so UX is consistent and prevents duplicate conversations.
const handleContact = async () => { + if (pending.value) return if (!props.profile.claimed) { - console.error('This profile is not clalimed') + console.error('This profile is not claimed') toast.error(`You can't message this profile yet.`) return } // Check if a user is logged in if (!currentUser.value) { return navigateTo('/login') } // Check if the user is trying to message themself if (currentUser.value.id === props.profile.id) { console.error("You can't message yourself.") return } //else start a conversation try { + pending.value = true const conversation = await $client.chat.createConversation.mutate({ participantIds: [props.profile.id], }) if (conversation?.id) { navigateTo(`/chat/${conversation.id}`) } } catch (error) { console.error('Failed to create or get conversation', error) + toast.error('Unable to start a conversation. Please try again.') + } finally { + pending.value = false } } + +const pending = ref(false)
55-55: Fix typo in log message“clalimed” → “claimed”.
- console.error('This profile is not clalimed') + console.error('This profile is not claimed')
53-81: Deduplicate handleContact across componentsThe contact logic is duplicated here and in components/ProfileCard.vue. Extract to a composable (e.g., useContact(profile)) to keep behavior consistent.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
components/ProfileCard.vue(2 hunks)components/profile/ProfileLayout.vue(2 hunks)composables/useProfile.ts(1 hunks)schemas/profile.ts(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: egemengunel
PR: we-dance/v4#414
File: cli/prisma/schema.prisma:645-666
Timestamp: 2025-09-01T06:51:31.640Z
Learning: In server/trpc/routers/chat.ts, the sendMessage mutation already properly uses prisma.$transaction() to wrap both message creation and conversation.lastMessageId update, ensuring referential integrity between Message and Conversation models.
🧬 Code graph analysis (3)
components/ProfileCard.vue (1)
server/trpc/routers/chat.ts (1)
newMessage(173-186)
composables/useProfile.ts (1)
cli/importer/profile.ts (1)
addProfile(319-459)
schemas/profile.ts (1)
cli/importer/profile.ts (1)
addProfile(319-459)
🔇 Additional comments (2)
schemas/profile.ts (2)
36-36: Verify create/update flows includeid
Ensure all profile create/update inputs (importers, admin tools, seeds) supplyid. If using UUIDs, validate withz.string().uuid(); otherwise enforce non-empty with.min(1):- id: z.string(), + id: z.string().min(1),No direct calls to
profileSchema.parse()or.safeParse()were found—please manually confirm that no callers are missingid.
45-45: claimed is already server-controlled in write schemas
Update schemas/profile.ts only for read operations; write endpoints use a separateprofileUpdateSchemathat omitsclaimed(and no create schema exposes it), so clients cannot setclaimed.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
components/profile/ProfileLayout.vue (1)
270-276: Gating matches acceptance criteria; previous feedback addressed.
Button is shown only for claimed profiles and disabled when pending.
🧹 Nitpick comments (7)
components/profile/ProfileLayout.vue (1)
270-276: Minor a11y: expose loading state.
Add aria-busy to reflect pending to assistive tech.<Button v-if="profile.claimed" class="w-full" variant="secondary" :disabled="pending" + :aria-busy="pending" @click="handleContact" >composables/useContact.ts (6)
5-9: Capture current route for post-login return.
Will be used to redirect back after login.const { $client } = useNuxtApp() const { session } = useAppAuth() + const route = useRoute()
10-16: Make the return type explicit.
Helps TS catch accidental returns.- const startConversation = async (profile: Profile) => { + const startConversation = async (profile: Profile): Promise<void> => {
18-21: Improve login redirect UX and typings.
Preserve “next” and avoid returning a non-void value.- if (!currentUser.value) { - return navigateTo('/login') - } + if (!currentUser.value) { + await navigateTo({ path: '/login', query: { next: route.fullPath } }) + return + }
22-26: Use a user-visible toast instead of console.error.
Console is invisible to end users.- if (currentUser.value.id === profile.id) { - console.error("You can't message yourself.") - return - } + if (currentUser.value.id === profile.id) { + toast.info("You can't message yourself.") + return + }
31-37: Await navigation and handle missing conversation id.
Avoid silent no-op if id is absent.- if (conversation?.id) { - navigateTo(`/chat/${conversation.id}`) - } + if (conversation?.id) { + await navigateTo(`/chat/${conversation.id}`) + } else { + toast.error('Could not start the conversation. Please try again.') + }
37-41: Surface failures to users.
Show a toast on error; keep console for diagnostics.- } catch (error) { - console.error('Failed to create or get conversation', error) + } catch (error) { + toast.error('Failed to start the conversation. Please try again.') + console.error('Failed to create or get conversation', error)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
components/ProfileCard.vue(2 hunks)components/profile/ProfileLayout.vue(3 hunks)composables/useContact.ts(1 hunks)composables/useProfile.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- composables/useProfile.ts
- components/ProfileCard.vue
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: egemengunel
PR: we-dance/v4#414
File: cli/prisma/schema.prisma:645-666
Timestamp: 2025-09-01T06:51:31.640Z
Learning: In server/trpc/routers/chat.ts, the sendMessage mutation already properly uses prisma.$transaction() to wrap both message creation and conversation.lastMessageId update, ensuring referential integrity between Message and Conversation models.
🧬 Code graph analysis (1)
composables/useContact.ts (3)
composables/useAppAuth.ts (1)
useAppAuth(1-98)schemas/profile.ts (1)
Profile(170-170)server/trpc/routers/chat.ts (2)
newId(137-163)conversation(125-132)
🔇 Additional comments (3)
components/profile/ProfileLayout.vue (2)
5-8: Good extraction: messaging flow moved to composable.
Clean import and usage of useContact with pending and startConversation.
50-53: Delegation looks right.
handleContact correctly delegates to startConversation(profile).composables/useContact.ts (1)
10-12: Concurrency guard is correct.
The early pending check prevents double taps.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
server/trpc/routers/chat.ts (1)
107-111: 👍 Omitted empty lastMessageId in new-conversation eventThis addresses the earlier suggestion to not send empty fields.
🧹 Nitpick comments (5)
server/trpc/routers/chat.ts (5)
53-66: Add pagination and index for messagesAscending + take:200 is OK for MVP; add cursor-based pagination and ensure an index on (conversationId, createdAt) to keep fetches predictable as chats grow.
126-131: Avoid double-trimming; rely on zod transform
sendMessageSchemaalready trims. Remove the extra.trim().Apply:
- content: input.content.trim(), + content: input.content,
148-160: Drop console.log in server path or gate with env/loggerUse your app logger or remove noisy logs in production.
Apply:
- console.log('Publishing to conversation:', conv.id) publish(`conversation:${conv.id}`, { type: 'message.created', conversationId: conv.id, messageId: msg.id, }) - - const other = conv.aId === me ? conv.bId : conv.aId - console.log('Publishing to inbox:', other) + const other = conv.aId === me ? conv.bId : conv.aId publish(`inbox:${other}`, { type: 'conversation.updated', conversationId: conv.id, })
157-160: Optionally include lastMessageId in inbox updateHelps the chat list update without an immediate fetch.
Apply (if ChatEvent.lastMessageId is optional as intended):
publish(`inbox:${other}`, { type: 'conversation.updated', conversationId: conv.id, + lastMessageId: msg.id, })
149-153: Observe delivery to improve reliabilityCapture
publish()return count for metrics/alerts (e.g., zero deliveries for online users may indicate SSE issues).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
components/profile/ProfileLayout.vue(3 hunks)composables/useContact.ts(1 hunks)server/trpc/routers/chat.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- components/profile/ProfileLayout.vue
- composables/useContact.ts
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: egemengunel
PR: we-dance/v4#414
File: cli/prisma/schema.prisma:645-666
Timestamp: 2025-09-01T06:51:31.688Z
Learning: In server/trpc/routers/chat.ts, the sendMessage mutation already properly uses prisma.$transaction() to wrap both message creation and conversation.lastMessageId update, ensuring referential integrity between Message and Conversation models.
📚 Learning: 2025-09-01T06:51:31.688Z
Learnt from: egemengunel
PR: we-dance/v4#414
File: cli/prisma/schema.prisma:645-666
Timestamp: 2025-09-01T06:51:31.688Z
Learning: In server/trpc/routers/chat.ts, the sendMessage mutation already properly uses prisma.$transaction() to wrap both message creation and conversation.lastMessageId update, ensuring referential integrity between Message and Conversation models.
Applied to files:
server/trpc/routers/chat.ts
🧬 Code graph analysis (1)
server/trpc/routers/chat.ts (4)
server/trpc/init.ts (2)
router(32-32)publicProcedure(31-31)server/prisma.ts (1)
prisma(5-8)schemas/chat.ts (2)
createConversationSchema(49-53)sendMessageSchema(58-67)server/utils/sse.ts (1)
publish(32-49)
🔇 Additional comments (5)
server/trpc/routers/chat.ts (5)
8-13: Auth guard + minimal ctx typing look solidClean, focused, and correct.
19-33: Lean projection + lastMessage relation: good callEfficient shape for the chat list; avoids N+1.
75-85: Preflight checks look goodSelf-chat prevented; participant existence validated.
125-146: Atomic message create + lastMessageId update looks correctInteractive transaction ensures consistency with lastSeen. Good.
15-15: Conversation.pairKey is already marked @unique in schema.prisma; no further action required.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
components/chat/ChatConversation.vue (1)
100-106: Make useAsyncData key reactive to avoid cache collisions when conversationId changes.Passing conversationKey.value freezes the key at mount; the watch re-fetches but still writes under the old key. Prefer a reactive key function and drop the manual watch.
-} = useAsyncData( - conversationKey.value, - () => - $client.chat.getConversation.query({ - conversationId: props.conversationId, - }), - { watch: [() => props.conversationId] } -) +} = useAsyncData( + () => `conversation-${props.conversationId}`, + () => $client.chat.getConversation.query({ + conversationId: props.conversationId, + }) +)
🧹 Nitpick comments (5)
components/chat/ChatConversation.vue (5)
78-91: otherParticipant resolution is tied to a/b schema; add fallback for participants[].Server mocks (and possibly prod) expose participants arrays. Add a safe fallback to keep header avatar/name working across both shapes.
-const otherParticipant = computed(() => { - if (!conversation.value || !currentUser.value) return null - - const conv = conversation.value as ConversationData - const currentUserId = currentUser.value?.id - - // Return the participant that's not the current user - return conv.aId === currentUserId ? conv.b : conv.a -}) +const otherParticipant = computed(() => { + if (!conversation.value || !currentUser.value) return null + const me = currentUser.value.id + const conv: any = conversation.value + if (conv?.aId && conv?.bId) { + return conv.aId === me ? conv.b : conv.a + } + if (Array.isArray(conv?.participants)) { + return conv.participants.find((p: any) => p.profileId !== me) ?? null + } + return null +})Also applies to: 108-116
176-183: Disable Send for whitespace-only input (UX polish).Button can be enabled for spaces-only input; sendMessage then no-ops. Use trimmed count for the disabled state to prevent confusing clicks.
-const charCount = computed(() => graphemeCount(newMessage.value)) +const charCount = computed(() => graphemeCount(newMessage.value)) +const trimmedCharCount = computed(() => graphemeCount(newMessage.value.trim()))- if (!content || isSending.value) return + if (!content || isSending.value) return-:disabled="charCount === 0 || charCount > 500 || isSending" +:disabled="trimmedCharCount === 0 || trimmedCharCount > 500 || isSending"Also applies to: 324-325
120-124: Avoid forcing scroll if the user is reading older messages.Auto-scrolling on every length change can yank the viewport. Consider only scrolling when near bottom.
-function scrollToBottom() { - nextTick(() => { - if (messagesContainer.value) { - messagesContainer.value.scrollTop = messagesContainer.value.scrollHeight - } - }) -} +function scrollToBottom(force = false) { + nextTick(() => { + const el = messagesContainer.value + if (!el) return + const nearBottom = el.scrollHeight - el.scrollTop - el.clientHeight < 64 + if (force || nearBottom) { + el.scrollTop = el.scrollHeight + } + }) +}-onMounted(() => { - scrollToBottom() +onMounted(() => { + scrollToBottom(true)
275-277: Avoid leaking raw error messages to end users.Swap to a friendly message and log details to console for diagnostics.
- <div v-else-if="error" class="p-4 text-red-500"> - {{ error?.message || 'Failed to load conversation' }} - </div> + <div v-else-if="error" class="p-4 text-red-500"> + Failed to load conversation. Please try again. + <span class="sr-only">Error:</span> + </div> + <!-- dev-only logging --> + <script setup lang="ts"> + if (process.dev) watch(() => error, e => { if (e) console.error(e) }) + </script>
312-320: Acceptance criteria mention a textarea; confirm single-line input is acceptable.If multi-line messages are desired now or later, switch to a textarea and handle Enter-to-send vs Shift+Enter for newline.
-<input - v-model="newMessage" - type="text" - placeholder="Type a message..." - class="flex-1 border rounded-l-lg px-4 py-2 focus:outline-none focus:ring-2 focus:ring-primary dark:bg-gray-800 dark:border-gray-700" - :disabled="isSending" -/> +<textarea + v-model="newMessage" + rows="1" + placeholder="Type a message..." + class="flex-1 border rounded-l-lg px-4 py-2 resize-none focus:outline-none focus:ring-2 focus:ring-primary dark:bg-gray-800 dark:border-gray-700" + :disabled="isSending" + @keydown.enter.exact.prevent="sendMessage" + @keydown.enter.shift.stop +/>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
components/chat/ChatConversation.vue(7 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
components/chat/ChatConversation.vue (1)
server/trpc/routers/chat.ts (2)
newMessage(173-186)newId(137-163)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: ci
- GitHub Check: test
🔇 Additional comments (2)
components/chat/ChatConversation.vue (2)
2-2: Grapheme-aware limit is implemented correctly and consistently.Importing graphemeCount, using charCount in UI, and disabling send when >500 aligns frontend with backend validation.
Also applies to: 13-13, 324-325, 334-336
19-49: SSE lifecycle management looks solid.Closing previous EventSource on switch and on unmount, deduping messages, and throttled markAsRead are all correct.
Also applies to: 51-66, 68-76
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
cli/prisma/schema.prisma (2)
656-675: Add 1:1 lastMessage to avoid N+1 and simplify listing.Persist a pointer to the last message; it avoids per-conversation subqueries and aligns with how UIs render previews.
Apply:
model Conversation { id String @id @default(cuid()) pairKey String @unique // `${min(aId,bId)}-${max(aId,bId)}` aId String bId String - aLastSeenAt DateTime? - bLastSeenAt DateTime? + aLastSeenAt DateTime? + bLastSeenAt DateTime? + + // O(1) last message + lastMessageId String? @unique + lastMessage Message? @relation("LastMessage", fields: [lastMessageId], references: [id]) messages Message[] @relation("ConversationMessages")And in Message (see comment below) add the back-relation.
677-690: Back-relation for lastMessage (only if you add it).To complete the 1:1 relation:
model Message { id String @id @default(cuid()) conversationId String senderId String - content String @db.Text + content String @db.Text createdAt DateTime @default(now()) updatedAt DateTime @updatedAt conversation Conversation @relation("ConversationMessages", fields: [conversationId], references: [id], onDelete: Cascade) sender Profile @relation(fields: [senderId], references: [id]) + lastMessageFor Conversation? @relation("LastMessage") @@index([conversationId, createdAt]) @@index([senderId, createdAt]) }
🧹 Nitpick comments (10)
server/api/chat/stream.get.ts (2)
22-35: Defer SSE headers until after auth/channel checks.Avoid sending text/event-stream headers on 4xx responses; set status and end first, then switch to SSE for successful paths.
- const res = setupSSE(event) + const res = event.node.res const q = getQuery(event) const session = await getServerSession(event) const profileId = session?.profile?.id @@ if (!channel) { setResponseStatus(event, 400) res.end() return } + // Only now upgrade to SSE + setupSSE(event)
78-80: Optional: advertise reconnection delay to clients.Adding an SSE retry directive improves resilience without client changes.
const unsubscribe = subscribe(channel, res) + // Advise client reconnection backoff (in ms) + res.write('retry: 10000\n') const hb = setInterval(() => res.write(':hb\n\n'), 25000)components/chat/ChatConversation.vue (5)
29-33: Encode the conversationId in the SSE URL.Prevents issues if IDs ever contain non-URL-safe chars.
- eventSource = new EventSource(`/api/chat/stream?conversationId=${id}`) + eventSource = new EventSource( + `/api/chat/stream?conversationId=${encodeURIComponent(id)}` + )
66-71: Avoid sticky error state; EventSource auto-reconnects.Set a transient “reconnecting…” indicator or clear error on open to avoid locking the UI in error after recovery.
eventSource.onerror = (err) => { console.error('SSE CONNECTION ERROR:', err) - error.value = new Error('Connection to chat server failed.') - isLoading.value = false + // Keep UI usable; let auto-reconnect happen + error.value = new Error('Reconnecting…') + isLoading.value = false } + eventSource.onopen = () => { + error.value = null + }
128-137: Harden getInitials against extra whitespace.Filter empty parts to avoid undefined when names have double spaces.
function getInitials(name: string | null | undefined) { if (!name) return '?' return name - .split(' ') - .map((part) => part[0]) + .split(' ') + .filter(Boolean) + .map((part) => part[0]) .join('') .toUpperCase() .substring(0, 2) }
305-331: Disable Send based on trimmed length to match server-side empty check.Currently, a whitespace-only message leaves the button enabled (charCount > 0) but sendMessage aborts. Use trimmed count for the button.
+const trimmedCharCount = computed(() => graphemeCount(newMessage.value.trim()))- :disabled="charCount === 0 || charCount > 500 || isSending" + :disabled="trimmedCharCount === 0 || charCount > 500 || isSending"
255-256: Accessibility: announce new messages.Add aria-live to the message container so screen readers announce incoming messages.
- <div class="flex-1 overflow-y-auto p-4 space-y-4" ref="messagesContainer"> + <div + class="flex-1 overflow-y-auto p-4 space-y-4" + ref="messagesContainer" + aria-live="polite" + >schemas/chat.ts (1)
5-12: Align messageSchema content validation with sendMessageSchema.Ensure stored/streamed messages also conform to the 1–500 grapheme rule and trimmed content.
export const messageSchema = z.object({ id: z.string(), conversationId: z.string(), senderId: z.string(), - content: z.string().min(1, 'Message cannot be empty'), + content: z + .string() + .transform((s) => s.trim()) + .refine( + (s) => graphemeCount(s) >= 1 && graphemeCount(s) <= 500, + 'Message must be 1–500 characters.' + ), createdAt: z.date().or(z.string()), updatedAt: z.date().or(z.string()), })cli/prisma/schema.prisma (2)
656-675: Guard against self-conversations and ensure canonical pairs.Either enforce
aId != bIdat the app layer, or add a DB CHECK via a raw migration. Also, since @@unique([aId,bId]) isn’t symmetric, rely on pairKey consistently and ensure it’s always built as min/max in service code.Would you like a short migration adding a CHECK (aId <> bId) and a test to assert canonical pairKey generation?
677-690: DB doesn’t enforce the 500‑grapheme input limit—confirm intent.content is Text (unbounded). If you want DB bounding, VarChar(500) counts code points, not graphemes, and can reject valid emoji sequences. Options:
- Keep Text; enforce 1–500 graphemes in app (current). Optionally cap bytes with a CHECK (e.g., octet_length(content) <= 4000) via a raw migration.
- Or accept code-point semantics and switch to VarChar(500).
Prefer which? I can generate the raw SQL CHECK migration if you want the soft cap.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
cli/prisma/schema.prisma(3 hunks)components/chat/ChatConversation.vue(8 hunks)schemas/chat.ts(2 hunks)server/api/chat/stream.get.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
components/chat/ChatConversation.vue (1)
server/trpc/routers/chat.ts (2)
newMessage(173-186)newId(137-163)
server/api/chat/stream.get.ts (3)
server/prisma.ts (1)
prisma(5-8)server/utils/sse.ts (2)
setupSSE(7-15)subscribe(17-30)schemas/chat.ts (1)
ChatChannel(90-90)
schemas/chat.ts (2)
schemas/profile.ts (1)
profileSchema(35-168)server/trpc/routers/chat.ts (2)
newMessage(173-186)newId(137-163)
🔇 Additional comments (10)
server/api/chat/stream.get.ts (3)
7-19: Membership check looks correct.Using findUnique + participant comparison is fine for 1:1 conversations.
48-76: Initial payload shape aligns with frontend needs.Includes messages (with senders) and both participants; ordered and capped. Looks good.
78-96: Unified cleanup is solid.Single guarded cleanup, clears heartbeat, unsubscribes, and ends response on both 'close' and 'aborted'.
schemas/chat.ts (3)
37-46: Grapheme-aware validation for send input looks good.Trim + 1–500 graphemes matches acceptance criteria.
48-57: Grapheme counter is appropriate.Intl.Segmenter with code point fallback is a sensible approach.
61-89: Event types are consistent with the frontend consumer.Includes rich payloads for init and messages; derived ChatEvent type avoids drift. Looks good.
cli/prisma/schema.prisma (4)
204-207: Profile ↔ Chat relations look consistent with Conversation/Message.Fields and relation names match the models below. No blockers.
298-299: No-op change.Looks like formatting/whitespace only.
648-648: No-op change.Formatting-only around updatedAt.
677-690: No additional sender check needed
Theprisma.conversation.findFirstquery already restrictsconvto whereaId === me || bId === mebefore callingtx.message.create, ensuring thesenderIdis a conversation participant.
| model Conversation { | ||
| id String @id @default(cuid()) | ||
| pairKey String @unique // `${min(aId,bId)}-${max(aId,bId)}` | ||
| aId String | ||
| bId String | ||
| aLastSeenAt DateTime? | ||
| bLastSeenAt DateTime? | ||
|
|
||
| messages Message[] @relation("ConversationMessages") | ||
|
|
||
| a Profile @relation("AProfile", fields: [aId], references: [id]) | ||
| b Profile @relation("BProfile", fields: [bId], references: [id]) | ||
|
|
||
| createdAt DateTime @default(now()) | ||
| updatedAt DateTime @updatedAt | ||
|
|
||
| @@unique([aId, bId]) | ||
| @@index([aId, updatedAt]) | ||
| @@index([bId, updatedAt]) | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Keep Conversation.updatedAt in sync with new messages.
If lists are ordered by Conversation.updatedAt, bump it transactionally when inserting a Message.
Run:
If absent, I can propose a tiny service change.
🏁 Script executed:
#!/bin/bash
# Expect to see tx.conversation.update({ data: { updatedAt: new Date() } }) in the same $transaction as message.create
rg -n -C3 'message\.create' server | sed -n '1,200p'
rg -n -C3 'conversation\.update\([^)]*updatedAt' serverLength of output: 1009
Bump Conversation.updatedAt transactionally when inserting a Message. In server/trpc/routers/chat.ts inside the prisma.$transaction callback (around line 168), after
await tx.message.create({ … })add
await tx.conversation.update({
where: { id: conv.id },
data: { updatedAt: new Date() },
})to keep lists ordered by updatedAt in sync.
🤖 Prompt for AI Agents
In server/trpc/routers/chat.ts around line 168 (while inside the
prisma.$transaction), when creating a new message with await
tx.message.create(…), also update the parent conversation's updatedAt within the
same transaction by calling tx.conversation.update with where: { id: conv.id }
and data: { updatedAt: new Date() } so conversation lists remain correctly
ordered by updatedAt; ensure this update is performed inside the existing
transaction block immediately after the message create.
- Introduces a minimal `chatProfileSchema` to accurately type the data payload, resolving type errors. - Removes the now-redundant `getConversation` procedure.
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)
components/chat/ChatConversation.vue (1)
316-323: Use a textarea to meet acceptance criteria.Spec requires “a textarea to compose messages.” Switch input to textarea (multi-line, emoji-friendly).
- <input - v-model="newMessage" - type="text" - placeholder="Type a message..." - class="flex-1 border rounded-l-lg px-4 py-2 focus:outline-none focus:ring-2 focus:ring-primary dark:bg-gray-800 dark:border-gray-700" - :disabled="isSending" - /> + <textarea + v-model="newMessage" + placeholder="Type a message..." + rows="2" + class="flex-1 border rounded-l-lg px-4 py-2 focus:outline-none focus:ring-2 focus:ring-primary dark:bg-gray-800 dark:border-gray-700 resize-none" + :disabled="isSending" + />
♻️ Duplicate comments (1)
schemas/chat.ts (1)
88-91: Minimal conversation.updated payload: good change.Dropping lastMessageId avoids empty-string placeholders and simplifies producers/consumers.
🧹 Nitpick comments (6)
schemas/chat.ts (1)
24-31: Optional: Align messageSchema’s content semantics with send-time rules.messageSchema enforces
.min(1)by code units, while sendMessageSchema uses grapheme-based limits and trimming. To avoid semantic drift, either mirror the grapheme rule here or relax messageSchema to reflect persisted data without re-validating length.Example (if you prefer mirroring send-time rules):
export const messageSchema = z.object({ id: z.string(), conversationId: z.string(), senderId: z.string(), - content: z.string().min(1, 'Message cannot be empty'), + content: z + .string() + .transform((s) => s.trim()) + .refine((s) => graphemeCount(s) >= 1 && graphemeCount(s) <= 500, 'Message must be 1–500 characters.'), createdAt: z.date().or(z.string()), updatedAt: z.date().or(z.string()), })components/chat/ChatConversation.vue (3)
86-118: Harden SSE parsing with runtime validation (use chatEventSchema.safeParse).Avoid
as ChatEventcasts; validate payloads to prevent runtime crashes on malformed events.-<script setup lang="ts"> -import { graphemeCount, type ChatEvent } from '~/schemas/chat' +<script setup lang="ts"> +import { graphemeCount, chatEventSchema, type ChatEvent } from '~/schemas/chat' @@ - eventSource.onmessage = (event) => { - try { - const data = JSON.parse(event.data) as ChatEvent + eventSource.onmessage = (event) => { + try { + const raw = JSON.parse(event.data) + const parsed = chatEventSchema.safeParse(raw) + if (!parsed.success) { + console.warn('SSE schema validation failed:', parsed.error) + return + } + const data: ChatEvent = parsed.data if (data.type === 'conversation.init') { conversation.value = data.conversation isLoading.value = false // Wait for the DOM to update with the messages before scrolling nextTick(() => { scrollToBottom() }) - } else if ( - data.type === `message.created` && + } else if ( + data.type === 'message.created' && data.conversationId === id && data.message ) {Also applies to: 2-2
31-31: Disable “Send” on whitespace-only input (trim before counting).UI now enables the button for spaces; compute charCount on trimmed text so the button state matches backend validation.
-const charCount = computed(() => graphemeCount(newMessage.value)) +const charCount = computed(() => graphemeCount(newMessage.value.trim()))Also applies to: 326-326
96-100: Minor: Consistent quoting.Use single quotes like elsewhere for
message.created.- } else if ( - data.type === `message.created` && + } else if ( + data.type === 'message.created' &&server/trpc/routers/chat.ts (2)
41-57: Optional: Return lastMessage instead of messages[0] for clarity.You fetch a single message but surface it as an array. Exposing
lastMessagereduces client-side ambiguity.- const lastMessage = c.messages[0] + const lastMessage = c.messages[0] @@ - return { + return { id: c.id, aId: c.aId, bId: c.bId, createdAt: c.createdAt, updatedAt: c.updatedAt, - messages: c.messages, + lastMessage, receiver, hasUnread, }(Propagate this rename to the consumer if applicable.)
195-214: Publish a self-inbox update after markAsRead (keeps ChatList in-sync across tabs).Optional but useful if the user has multiple tabs open; ensures their other views update without manual refresh.
await prisma.conversation.update({ where: { id: conv.id }, data: { ...(conv.aId === me ? { aLastSeenAt: new Date() } : { bLastSeenAt: new Date() }), }, }) - return { success: true } + publish(`inbox:${me}`, { + type: 'conversation.updated', + conversationId: conv.id, + }) + return { success: true }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
components/chat/ChatConversation.vue(7 hunks)schemas/chat.ts(2 hunks)server/trpc/routers/chat.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
server/trpc/routers/chat.ts (4)
server/trpc/init.ts (2)
router(32-32)publicProcedure(31-31)server/prisma.ts (1)
prisma(5-8)schemas/chat.ts (2)
createConversationSchema(47-49)sendMessageSchema(54-63)server/utils/sse.ts (1)
publish(32-49)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: ci
- GitHub Check: test
🔇 Additional comments (4)
schemas/chat.ts (3)
3-13: Grapheme-aware counting: LGTMCorrect, robust handling of emojis/ZWJ with Intl.Segmenter and a sane fallback.
53-62: Backend validation matches acceptance (1–500 graphemes, trimmed).This aligns with the requirements and prevents empty/whitespace-only sends.
94-94: Type derives from schema: LGTMUsing
z.infer<typeof chatEventSchema>prevents drift and removes casts elsewhere.server/trpc/routers/chat.ts (1)
20-24: Auth guard pattern: LGTMFiltering by participant in the DB plus
requireProfileIdyields clean NOT_FOUND on unauthorized access.Also applies to: 139-144
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
pages/chat/index.vue (1)
2-3: Fix stale-search race, browser-safe timer type, and add cleanupOlder responses can overwrite newer results; NodeJS.Timeout typing breaks in browsers; no unmount cleanup. Reuse the prior approach with a request token and clear timers on unmount.
Apply:
- import { ref } from 'vue' + import { ref, onUnmounted } from 'vue' @@ -// Search users with debounce -let searchTimeout: NodeJS.Timeout | null = null +// Search users with debounce +let searchTimeout: ReturnType<typeof setTimeout> | null = null +let lastRequestToken = 0 function searchUsers() { - selectedUser.value = null - if (searchTimeout) clearTimeout(searchTimeout) - - if (!searchQuery.value.trim()) { - searchResults.value = [] - return - } + if (searchTimeout) clearTimeout(searchTimeout) + const q = searchQuery.value.trim() + if (selectedUser.value && q !== selectedUser.value.name) { + selectedUser.value = null + } + if (!q) { + lastRequestToken++ + searchResults.value = [] + isSearching.value = false + return + } isSearching.value = true searchTimeout = setTimeout(async () => { - try { - const results = await $client.profiles.search.query({ - query: searchQuery.value.trim(), - }) - - searchResults.value = results - } catch (error) { - console.error('Error searching users:', error) - } finally { - isSearching.value = false - } + const token = ++lastRequestToken + try { + const results = await $client.profiles.search.query({ query: q }) + if (token !== lastRequestToken) return + searchResults.value = results + } catch (error) { + if (token === lastRequestToken) console.error('Error searching users:', error) + } finally { + if (token === lastRequestToken) isSearching.value = false + } }, 300) } + +onUnmounted(() => { + if (searchTimeout) clearTimeout(searchTimeout) + lastRequestToken++ +})Also applies to: 15-41
server/trpc/routers/chat.ts (2)
25-35: Prefer lastMessage relation over messages.take(1) (if schema has lastMessageId)Avoids per-conversation sort/limit on Messages and matches a denormalized pointer.
If Conversation has lastMessageId/lastMessage, consider:
include: { a: { select: { id: true, name: true, photo: true } }, b: { select: { id: true, name: true, photo: true } }, - messages: - { - orderBy: { createdAt: 'desc' }, - take: 1, - include: { - sender: { select: { id: true, name: true, photo: true } }, - }, - }, + lastMessage: { + include: { + sender: { select: { id: true, name: true, photo: true } }, + }, + }, }, @@ - const lastMessage = c.messages[0] + const lastMessage = c.lastMessage ?? null @@ - messages: c.messages, + // keep existing shape for UI by wrapping lastMessage + messages: lastMessage ? [lastMessage] : [],Run to verify schema support:
#!/bin/bash # Checks for lastMessageId relation in Prisma schemas rg -n --type=prisma 'lastMessageId|lastMessage\s*:' -C2Also applies to: 41-41, 54-54
145-165: Also set lastMessageId in the same transaction (if field exists)Keeps Conversation pointer consistent and removes need for messages.take(1) elsewhere.
- await tx.conversation.update({ + await tx.conversation.update({ where: { id: conv.id }, data: { + // if Conversation has lastMessageId + ...(typeof (tx as any) !== 'undefined' && { lastMessageId: createdMessage.id }), ...(conv.aId === me ? { aLastSeenAt: new Date() } : { bLastSeenAt: new Date() }), }, })Check schema support:
#!/bin/bash rg -n --type=prisma -C2 'model Conversation|lastMessageId'
🧹 Nitpick comments (10)
schemas/chat.ts (1)
28-35: DRY the content validation to avoid driftDefine a single contentSchema and reuse it in both messageSchema and sendMessageSchema.
Apply:
+const contentSchema = z + .string() + .transform((s) => s.trim()) + .refine( + (s) => graphemeCount(s) >= 1 && graphemeCount(s) <= 500, + 'Message must be 1–500 characters.' + ) export const messageSchema = z.object({ id: z.string(), conversationId: z.string(), senderId: z.string(), - content: z - .string() - .transform((s) => s.trim()) - .refine( - (s) => graphemeCount(s) >= 1 && graphemeCount(s) <= 500, - 'Message must be 1–500 characters.' - ), + content: contentSchema, createdAt: z.date().or(z.string()), updatedAt: z.date().or(z.string()), }) export const sendMessageSchema = z.object({ conversationId: z.string(), - content: z - .string() - .transform((s) => s.trim()) - .refine( - (s) => graphemeCount(s) >= 1 && graphemeCount(s) <= 500, - 'Message must be 1–500 characters.' - ), + content: contentSchema, })Also applies to: 62-69
pages/chat/index.vue (2)
10-12: Strongly type search results and selectionAvoid any/any[]. Use your tRPC outputs or a minimal local type to match the template fields (id, name, username, photo).
Apply:
-const searchResults = ref<any[]>([]) -const selectedUser = ref<any>(null) +type SearchUser = { id: string; name: string; username: string; photo: string | null } +const searchResults = ref<SearchUser[]>([]) +const selectedUser = ref<SearchUser | null>(null)
68-76: Make getInitials resilient to extra whitespace and empty namesFilter blanks and provide a fallback.
Apply:
-function getInitials(name: string) { - return name - .split(' ') - .map((part) => part[0]) - .join('') - .toUpperCase() - .substring(0, 2) -} +function getInitials(name: string) { + const initials = name + .trim() + .split(' ') + .filter(Boolean) + .map((part) => part[0] ?? '') + .join('') + .toUpperCase() + .substring(0, 2) + return initials || '?' +}components/chat/ChatConversation.vue (4)
85-87: Fix typo in log message“Recieved” → “Received”.
Apply:
- console.warn('Recieved invalid SSE event:', parsed.error) + console.warn('Received invalid SSE event:', parsed.error)
92-99: Guard init events by ID and clear transient errors on valid trafficExtra safety if streams race; also clears any prior error once valid events arrive.
Apply:
- if (data.type === 'conversation.init') { - conversation.value = data.conversation + // Clear any transient error on valid traffic + error.value = null + if (data.type === 'conversation.init' && data.conversation.id === id) { + conversation.value = data.conversation isLoading.value = false
109-111: Scroll after DOM updates when appending messagesEnsure the new bubble is rendered before measuring/scrolling.
Apply:
- messages.push(data.message) - scrollToBottom() + messages.push(data.message) + nextTick(() => { + scrollToBottom() + })
124-127: Treat SSE onerror as transientEventSource auto-reconnects. Showing a persistent error can be misleading; keep isLoading false but let the next valid event clear error (see prior comment).
Apply:
eventSource.onerror = (err) => { console.error('SSE CONNECTION ERROR:', err) - error.value = new Error('Connection to chat server failed.') - isLoading.value = false + error.value = new Error('Connection hiccup. Reconnecting…') + isLoading.value = false }server/trpc/routers/chat.ts (3)
88-99: Minor: reuse a single timestamp in txUse one now = new Date() to avoid tiny drift between fields and improve testability.
- try { + try { + const now = new Date() const created = await tx.conversation.create({ data: { pairKey: key, aId, bId, - ...(aId === me - ? { aLastSeenAt: new Date() } - : { bLastSeenAt: new Date() }), + ...(aId === me ? { aLastSeenAt: now } : { bLastSeenAt: now }), },
167-174: Simplify event payload constructionSender is already minimally selected; the remap is redundant.
- const messageForEvent = { - ...msg, - sender: { - id: msg.sender.id, - name: msg.sender.name, - photo: msg.sender.photo, - }, - } + const messageForEvent = msg
194-218: Optional: notify the other participant on markAsReadIf UI shows read status/unread badges for the other user, also publish to their inbox.
- .mutation(async ({ ctx, input }) => { + .mutation(async ({ ctx, input }) => { const me = requireProfileId(ctx) - const conv = await prisma.conversation.findFirst({ + const conv = await prisma.conversation.findFirst({ where: { id: input.conversationId, OR: [{ aId: me }, { bId: me }] }, - select: { id: true, aId: true }, + select: { id: true, aId: true, bId: true }, }) @@ publish(`inbox:${me}`, { type: 'conversation.updated', conversationId: conv.id, }) + const other = conv.aId === me ? conv.bId : conv.aId + publish(`inbox:${other}`, { + type: 'conversation.updated', + conversationId: conv.id, + })
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (6)
components/chat/ChatConversation.vue(7 hunks)components/chat/ChatList.vue(4 hunks)pages/chat/index.vue(2 hunks)schemas/chat.ts(1 hunks)server/trpc/routers/chat.ts(1 hunks)server/utils/sse.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- server/utils/sse.ts
- components/chat/ChatList.vue
🧰 Additional context used
🧬 Code graph analysis (1)
server/trpc/routers/chat.ts (4)
server/trpc/init.ts (2)
router(32-32)publicProcedure(31-31)server/prisma.ts (1)
prisma(5-8)schemas/chat.ts (2)
createConversationSchema(53-55)sendMessageSchema(60-69)server/utils/sse.ts (1)
publish(32-49)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: ci
- GitHub Check: test
🔇 Additional comments (10)
schemas/chat.ts (2)
3-13: Grapheme-aware length: LGTMSegmenter fast-path with a code-point fallback is correct and aligns with the 1–500 requirement.
95-101: No remaininglastMessageIdreferences found;conversation.updatedhandlers correctly use onlyconversationId.components/chat/ChatConversation.vue (1)
48-57: Good cleanup on unmountEventSource and timers are closed/cleared. Prevents leaks across navigations.
server/trpc/routers/chat.ts (7)
9-14: Good fix: narrow ctx typing for session.profilePrevents next-auth Session type mismatches; simple and correct.
16-16: Stable pair key derivationDeterministic and collision-safe for string IDs. LGTM.
105-120: Nice: handles P2002 duplicate-create raceGood UX under concurrency; returns winner instead of 500.
123-130: Real-time UX: notify both inboxesGood addition; initiator sees ChatList update immediately too.
175-181: Event shape looks consistentIncludes message and IDs; consumers can render immediately. LGTM.
182-190: Nice: publish conversation.updated to both sidesEnsures list ordering/unread counts refresh for sender and recipient.
22-24: No action needed – appropriate indexes already defined
TheConversationmodel already includes:
@uniqueonpairKey@@unique([aId, bId])@@index([aId, updatedAt])@@index([bId, updatedAt])
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (7)
schemas/chat.ts (2)
70-95: Event payload polish: drop duplicate messageId (it’s in message.id).Keeps the envelope lean and avoids dual sources of truth.
Apply:
z.object({ type: z.literal('message.created'), conversationId: z.string(), - messageId: z.string(), message: messageSchema.extend({ sender: chatProfileSchema, }), }),
70-95: Consider coercing dates to Date objects at the schema edge.z.date().or(z.string()) propagates unions; consumers often prefer Dates.
Possible approach:
- createdAt: z.date().or(z.string()), - updatedAt: z.date().or(z.string()), + createdAt: z.coerce.date(), + updatedAt: z.coerce.date(),And mirror this in messageSchema and the extended shapes.
components/chat/ChatConversation.vue (5)
73-79: Encode conversationId in the SSE URL.Prevents surprises if IDs ever include special characters.
- eventSource = new EventSource(`/api/chat/stream?conversationId=${id}`) + eventSource = new EventSource( + `/api/chat/stream?conversationId=${encodeURIComponent(id)}` + )
92-99: Avoid double nextTick before scrolling.scrollToBottom already waits for nextTick; remove the outer nextTick call.
- // Wait for the DOM to update with the messages before scrolling - nextTick(() => { - scrollToBottom() - }) + scrollToBottom()Also applies to: 163-170
60-71: Optional UX: clear composer when switching threads.Keeps per-thread drafts from leaking across conversations.
(newId, oldId) => { if (newId && newId !== oldId) { isLoading.value = true conversation.value = null error.value = null + newMessage.value = '' subscribeToConversation(newId) markAsRead(newId) } }
130-136: Reuse computed charCount to avoid recomputing graphemes.Minor micro-optimization and single source of truth.
- if (graphemeCount(content) > 500) { + if (charCount.value > 500) { // Optionally surface a toast here for use return }
124-128: SSE error path: consider surface-level retry notice.EventSource auto-reconnects; a lightweight user hint (toast) improves UX during transient failures.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
components/chat/ChatConversation.vue(7 hunks)pages/chat/index.vue(2 hunks)schemas/chat.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- pages/chat/index.vue
🧰 Additional context used
🧬 Code graph analysis (2)
components/chat/ChatConversation.vue (1)
server/trpc/routers/chat.ts (2)
newMessage(173-186)conversation(125-132)
schemas/chat.ts (1)
server/trpc/routers/chat.ts (3)
newMessage(173-186)newId(137-163)conversation(125-132)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test
🔇 Additional comments (7)
schemas/chat.ts (6)
3-13: Grapheme counting helper looks solid.Good Intl.Segmenter usage with a safe fallback.
37-40: Message.content schema alignment.Switch to contentSchema is correct and keeps frontend/backend in sync on grapheme limits.
97-97: Type derives from schema — nice.This removes drift and casts.
99-99: Channel typing is clear and future-proof.Template literal union is concise.
44-51: All code references updated toaId/bId; no legacyparticipantsfound.
23-31: Ensure trimming happens before validation (avoid whitespace-only messages sneaking through).Refine may run before transform depending on Zod chaining; make trimming unambiguously happen first.
Apply:
-// Content Schema -const contentSchema = z - .string() - .transform((s) => s.trim()) - .refine( - (s) => graphemeCount(s) >= 1 && graphemeCount(s) <= 500, - 'Message must be 1–500 characters.' - ) +// Content Schema (preprocess to trim, then validate graphemes) +const contentSchema = z.preprocess( + (v) => (typeof v === 'string' ? v.trim() : v), + z + .string() + .refine( + (s) => { + const n = graphemeCount(s) + return n >= 1 && n <= 500 + }, + 'Message must be 1–500 characters.' + ) +)Likely an incorrect or invalid review comment.
components/chat/ChatConversation.vue (1)
31-32: Frontend/Backend character counting aligned.Using graphemeCount for the counter keeps UX consistent with server rules.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (7)
components/chat/ChatConversation.vue (4)
95-101: Clear sticky error state; reset on successful events and on reconnectErrors persist after auto-reconnect. Clear error on valid events and add onopen to reset after reconnection.
if (data.type === 'conversation.init') { conversation.value = data.conversation isLoading.value = false + error.value = null scrollToBottom() } else if ( @@ } - eventSource.onerror = (err) => { + eventSource.onopen = () => { + error.value = null + } + eventSource.onerror = (err) => { console.error('SSE CONNECTION ERROR:', err) error.value = new Error('Connection to chat server failed.') isLoading.value = false }Also applies to: 123-128
330-331: Disable “Send” when over 500 graphemesPrevents avoidable round-trips and matches the toast validation.
- :disabled="charCount === 0 || isSending" + :disabled="charCount === 0 || charCount > 500 || isSending"
31-33: Count graphemes on raw input for UX; keep trim only for sendingUsers expect the counter to reflect what they type, not the trimmed value.
-const charCount = computed(() => graphemeCount(newMessage.value.trim())) +const charCount = computed(() => graphemeCount(newMessage.value))
134-135: Minor copy tweak“cannot” is preferred over “can not”.
- toast.error('Message can not be longer than 500 characters.') + toast.error('Message cannot be longer than 500 characters.')server/trpc/routers/chat.ts (3)
197-216: Broadcast read status to the other participant as well (real-time unread badge)Publishing only to the reader’s inbox delays the other side’s ChatList updates. Also select bId to compute “other”.
- const conv = await prisma.conversation.findFirst({ + const conv = await prisma.conversation.findFirst({ where: { id: input.conversationId, OR: [{ aId: me }, { bId: me }] }, - select: { id: true, aId: true }, + select: { id: true, aId: true, bId: true }, }) @@ publish(`inbox:${me}`, { type: 'conversation.updated', conversationId: conv.id, }) + const other = conv.aId === me ? conv.bId : conv.aId + if (other && other !== me) { + publish(`inbox:${other}`, { + type: 'conversation.updated', + conversationId: conv.id, + }) + } return { success: true }
48-57: Expose lastMessage alongside messages[0] (non-breaking)Makes consumer code clearer while keeping the existing shape.
return { id: c.id, aId: c.aId, bId: c.bId, createdAt: c.createdAt, updatedAt: c.updatedAt, messages: c.messages, + lastMessage, receiver, hasUnread, }
20-21: Consider a protected procedure helperYou consistently gate with requireProfileId; a protectedProcedure (auth middleware) would centralize this.
If you’d like, I can sketch a minimal auth middleware and replace publicProcedure here.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
components/chat/ChatConversation.vue(7 hunks)composables/useContact.ts(1 hunks)schemas/chat.ts(1 hunks)server/trpc/routers/chat.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- composables/useContact.ts
- schemas/chat.ts
🧰 Additional context used
🧬 Code graph analysis (1)
server/trpc/routers/chat.ts (4)
server/trpc/init.ts (2)
router(32-32)publicProcedure(31-31)server/prisma.ts (1)
prisma(5-8)schemas/chat.ts (2)
createConversationSchema(56-58)sendMessageSchema(63-66)server/utils/sse.ts (1)
publish(32-49)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: ci
- GitHub Check: test
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
server/trpc/routers/chat.ts (1)
145-165: Atomicity on sendMessage looks good; add lastMessageId if/when available.You’re already using a transaction and publishing after commit. Just ensure the conversation’s “last message” pointer is updated inside the same tx (see earlier diff) when you introduce lastMessageId.
🧹 Nitpick comments (3)
server/trpc/routers/chat.ts (3)
22-36: Rename “messages” to “lastMessage” in the list response (optional).The list returns an array with at most one element, which is semantically a single last message. Renaming avoids confusion and future API debt.
If you keep the current response for compatibility, consider adding lastMessage alongside messages now and deprecate messages later.
Also applies to: 48-57
88-99: Micro: compute [aId, bId] only when creating.Saves a tiny bit of work on the common early-return path.
Apply:
- const [aId, bId] = me < other ? [me, other] : [other, me] try { + const [aId, bId] = me < other ? [me, other] : [other, me] const created = await tx.conversation.create({
22-35: DB indexes to match query patterns.Ensure the following indexes exist to keep these queries fast at scale:
- conversation(pairKey) UNIQUE
- conversation(aId), conversation(bId)
- message(conversationId, createdAt DESC)
I can draft the Prisma schema index annotations if helpful.
Also applies to: 139-143, 197-201
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
components/chat/ChatConversation.vue(7 hunks)server/trpc/routers/chat.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- components/chat/ChatConversation.vue
🧰 Additional context used
🧬 Code graph analysis (1)
server/trpc/routers/chat.ts (4)
server/trpc/init.ts (2)
router(32-32)publicProcedure(31-31)server/prisma.ts (1)
prisma(5-8)schemas/chat.ts (2)
createConversationSchema(56-58)sendMessageSchema(63-66)server/utils/sse.ts (1)
publish(32-49)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test
🔇 Additional comments (1)
server/trpc/routers/chat.ts (1)
123-130: Good: publish conversation.updated to both participants.Ensures both chat lists refresh in real time after create and send. No action needed.
Also applies to: 181-189
| data: { | ||
| conversationId: conv.id, | ||
| senderId: me, | ||
| content: input.content, | ||
| }, | ||
| include: { | ||
| sender: { select: { id: true, name: true, photo: true } }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Trim message content on the server to hard-prevent whitespace-only sends.
Schema likely guards this, but a final .trim() is cheap defense-in-depth and aligns with “prevent empty messages”.
Apply:
- content: input.content,
+ content: input.content.trim(),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| data: { | |
| conversationId: conv.id, | |
| senderId: me, | |
| content: input.content, | |
| }, | |
| include: { | |
| sender: { select: { id: true, name: true, photo: true } }, | |
| data: { | |
| conversationId: conv.id, | |
| senderId: me, | |
| content: input.content.trim(), | |
| }, | |
| include: { | |
| sender: { select: { id: true, name: true, photo: true } }, |
🤖 Prompt for AI Agents
In server/trpc/routers/chat.ts around lines 147 to 153, the message content is
persisted directly from input.content; trim the content on the server to prevent
whitespace-only messages by creating a trimmed variable (e.g. const content =
input.content.trim()), validate that content is non-empty (throw or return an
error if empty), and use that trimmed content in the data payload and any
further logic instead of input.content.
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 (2)
prisma/schema.prisma (2)
5-7: Fix Prisma preview feature rename to satisfy CI.
fullTextSearchwas renamed for PostgreSQL. UsefullTextSearchPostgresto clear the pipeline warning.generator client { provider = "prisma-client-js" - previewFeatures = ["fullTextSearch", "relationJoins"] + previewFeatures = ["fullTextSearchPostgres", "relationJoins"] }
9-13: Standardize PostgreSQL provider in both Prisma schemasIn both
prisma/schema.prismaandcli/prisma/schema.prisma, update thedbdatasource to use"postgresql"instead of"postgres"and then run your migration:datasource db { - provider = "postgres" + provider = "postgresql" url = env("DATABASE_URL") directUrl = env("DIRECT_URL") }
🧹 Nitpick comments (3)
prisma/schema.prisma (1)
674-676: Redundant unique with pairKey; consider dropping@@unique([aId, bId]).
pairKeyalready enforces uniqueness for the sorted pair. Keeping both adds write overhead without extra guarantees.model Conversation { @@ - @@unique([aId, bId]) @@index([aId, updatedAt]) @@index([bId, updatedAt]) }server/trpc/routers/chat.ts (2)
67-73: Optional: enforce “message only claimed profiles” at server.UI hides the button for unclaimed, but the API currently allows creating a conversation with any existing profile id. If policy requires, add
claimed: trueto the lookup.const otherProfile = await prisma.profile.findUnique({ - where: { id: other }, - select: { id: true }, + where: { id: other }, + select: { id: true, claimed: true }, }) -if (!otherProfile) { +if (!otherProfile || !otherProfile.claimed) { throw new TRPCError({ code: 'NOT_FOUND' }) }
143-164: Trim and revalidate message content server-side.Defense-in-depth: prevent whitespace-only submits regardless of client validation.
- const createdMessage = await tx.message.create({ + const content = input.content.trim() + if (!content) throw new TRPCError({ code: 'BAD_REQUEST' }) + const createdMessage = await tx.message.create({ data: { conversationId: conv.id, senderId: me, - content: input.content, + content, },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
prisma/schema.prisma(4 hunks)server/trpc/routers/chat.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-09T06:33:48.443Z
Learnt from: egemengunel
PR: we-dance/v4#414
File: server/trpc/routers/chat.ts:24-35
Timestamp: 2025-09-09T06:33:48.443Z
Learning: In the WeDance chat system, lastMessageId and lastMessage fields were removed from the Conversation schema and should not be reintroduced. The team prefers to avoid schema changes for chat ordering improvements.
Applied to files:
server/trpc/routers/chat.tsprisma/schema.prisma
🧬 Code graph analysis (1)
server/trpc/routers/chat.ts (4)
server/trpc/init.ts (2)
router(32-32)publicProcedure(31-31)server/prisma.ts (1)
prisma(5-8)schemas/chat.ts (2)
createConversationSchema(56-58)sendMessageSchema(63-66)server/utils/sse.ts (1)
publish(32-49)
🪛 GitHub Actions: ci
prisma/schema.prisma
[warning] 1-1: Prisma: preview feature 'fullTextSearch' renamed to 'fullTextSearchPostgres' for provider 'postgresql'. Update your schema accordingly.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test
🔇 Additional comments (8)
prisma/schema.prisma (3)
656-677: Confirm intentional re-introduction of lastMessage fields.Retrieved learnings note the team removed
lastMessageId/lastMessagepreviously and preferred avoiding schema changes. This PR adds them back and the code depends on them (ordering, updates). Confirm this direction is now the agreed one across server and CLI schemas.If confirmed, LGTM on the 1:1 shape (
lastMessageId String? @unique,lastMessageFor Conversation?).
606-606: Verify removal of onDelete behavior on Calendar → Profile.You removed explicit
onDeletefromcalendar_owner. Ensure this matches desired deletion semantics (no cascading calendar cleanup on profile deletion).
679-693: Message model changes LGTM.
content @db.Textand the new index on[senderId, createdAt]align with grapheme-aware 500-limit in app code and expected queries.server/trpc/routers/chat.ts (5)
9-14: Good fix: minimal ctx typing for profile id.This avoids relying on next-auth’s Session shape. Clean and precise.
22-34: Ordering by relatedlastMessage.createdAtrequires Prisma support; confirm runtime behavior for conversations with no lastMessage.With
relationJoinspreview, this should work; records lackinglastMessagemay sort last. Confirm Prisma version compatibility and that empty threads still list correctly.
75-119: Nice: race-safe create with P2002 recovery and deterministic aId/bId.Good transaction and minimal selects to avoid PII exposure.
174-189: Event fanout looks correct.Publishing to conversation channel and both inboxes ensures real-time updates on both sides.
192-223: markAsRead will bump updatedAt; safe given list orders by lastMessage.No change needed; noting behavior is intentional with the new ordering.
This feature completes the chat functionality implemented with the backend implementation using SSE.
closes #79
Summary by CodeRabbit
New Features
Behavior Changes
Data / UI
Chores