Daily branch 2025 09 04 #20
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughRefactors Chat to accept a route-based Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as User
participant SH as SidebarHeaderContent
participant MSD as MessageSearchDialog
participant CM as Convex.messages.searchMessages
participant R as Router
U->>SH: Click "Search" or press Cmd/Ctrl+K
SH->>MSD: Open dialog
MSD->>MSD: Debounce input (300ms)
alt empty query
MSD->>MSD: Show categorized chats (local/global state)
else query present
MSD->>CM: searchMessages(query, pagination)
CM-->>MSD: results + continueCursor
MSD->>MSD: Render results (dedupe, highlight)
loop when loader visible
MSD->>CM: fetch next page
CM-->>MSD: more results
MSD->>MSD: Append results
end
end
U->>MSD: Select item
MSD->>R: router.push("/c/{chatId}")
MSD->>MSD: Close dialog
sequenceDiagram
autonumber
actor U as User
participant Page as /c/[id]/page.tsx
participant Chat as Chat
participant S as Server
participant R as Router
U->>Page: Navigate to / or /c/{id}
Page->>Chat: <Chat chatId={routeId}>
alt Existing chat (routeId present)
Chat->>S: Fetch chat + paginated messages
S-->>Chat: Data
Chat->>Chat: Sync messages into UI
else New chat (no routeId)
Chat->>Chat: Generate local chatId
U->>Chat: Send first message
Chat->>S: Persist message
S-->>Chat: onFinish callback
Chat->>R: Update URL to /c/{chatId} (replace/push without full navigation)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~70 minutes Possibly related PRs
Poem
✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 15
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
app/contexts/GlobalState.tsx (1)
239-246: Type chatId as Id<'chats'> and remove unused param
- Replace string with Id<"chats">.
- _fromRoute is unused; either use or drop.
-const initializeChat = useCallback((chatId: string, _fromRoute?: boolean) => { +const initializeChat = useCallback((chatId: Id<"chats">) => { setIsSwitchingChats(true); setCurrentChatId(chatId); setShouldFetchMessages(true); setHasActiveChat(true); setTodos([]); setIsTodoPanelExpanded(false); }, []);Also update GlobalStateType:
- currentChatId: string | null; - setCurrentChatId: (chatId: string | null) => void; + currentChatId: Id<"chats"> | null; + setCurrentChatId: (chatId: Id<"chats"> | null) => void; - initializeChat: (chatId: string, fromRoute?: boolean) => void; + initializeChat: (chatId: Id<"chats">) => void;And state:
-const [currentChatId, setCurrentChatId] = useState<string | null>(null); +const [currentChatId, setCurrentChatId] = useState<Id<"chats"> | null>(null);app/components/SidebarHistory.tsx (1)
28-51: Attach/detach scroll listener when the actual element becomes available.
useEffectdepends oncontainerRef, notcontainerRef.current. If.currentis null at mount, the listener may never attach.- React.useEffect(() => { - const handleScroll = () => { + const el = containerRef?.current; + React.useEffect(() => { + const handleScroll = () => { if ( - !containerRef?.current || - !loadMore || + !el || + !loadMore || paginationStatus !== "CanLoadMore" ) { return; } - const { scrollTop, scrollHeight, clientHeight } = containerRef.current; + const { scrollTop, scrollHeight, clientHeight } = el; // Check if we're near the bottom (within 100px) if (scrollTop + clientHeight >= scrollHeight - 100) { loadMore(28); // Load 28 more chats } }; - const container = containerRef?.current; - if (container) { - container.addEventListener("scroll", handleScroll); - return () => container.removeEventListener("scroll", handleScroll); - } - }, [containerRef, loadMore, paginationStatus]); + if (!el) return; + el.addEventListener("scroll", handleScroll); + return () => el.removeEventListener("scroll", handleScroll); + }, [el, loadMore, paginationStatus]);convex/chats.ts (1)
63-90: Return the correct Convex Id type from saveChat.
ctx.db.insertreturnsId<"chats">. Validate withv.id("chats")instead ofv.string().- returns: v.string(), + returns: v.id("chats"), handler: async (ctx, args) => { @@ - const chatId = await ctx.db.insert("chats", { + const chatId = await ctx.db.insert("chats", { id: args.id, title: args.title, user_id: args.userId, update_time: Date.now(), }); return chatId;
🧹 Nitpick comments (26)
middleware.ts (1)
14-15: Allowing manifest.json unauthenticated is correctThis avoids auth-blocking PWA installs and browser fetches.
Consider also allowing:
unauthenticatedPaths: [ "/", "/login", "/signup", "/logout", "/callback", "/privacy-policy", "/terms-of-service", "/manifest.json", + "/robots.txt", + "/sitemap.xml", ],convex/schema.ts (2)
43-55: Optional content + search: ensure null-safety and coverage
- content is optional; queries using withSearchIndex("search_content", ...) must guard against missing content in result handling.
- Filter is user_id. Verify assistant/system messages set user_id to the owning user; otherwise those messages won’t be searchable.
If assistant/system messages may lack user_id, either:
- populate messages.user_id with the chat owner on write; or
- introduce a messages.owner_user_id field (v.string()) dedicated for filtering and index that instead.
27-31: Avoid duplicating system _id with a custom chats.idYou index chats.id (“by_chat_id”), but Convex already provides _id. Duplicating increases drift risk and migration cost.
Consider:
- id: v.string(), ... - }).index("by_chat_id", ["id"]) + // Remove custom id; rely on system _id + })Use ctx.db.get(_id) for direct lookups; drop the redundant index.
app/contexts/GlobalState.tsx (1)
257-261: Deduplicate activateChat logic and align behaviorReuse initializeChat to keep behavior consistent (e.g., switching state).
-const activateChat = useCallback((chatId: string) => { - setCurrentChatId(chatId); - setShouldFetchMessages(true); - setHasActiveChat(true); -}, []); +const activateChat = useCallback((chatId: Id<"chats">) => { + initializeChat(chatId); +}, [initializeChat]);Also update GlobalStateType:
- activateChat: (chatId: string) => void; + activateChat: (chatId: Id<"chats">) => void;Ensure any callers pass Id<"chats">, not plain strings from the router. If router params are strings, narrow them via casting after validation.
app/components/SidebarHistory.tsx (1)
8-18: Strongly type chats with Convex Doc type.Avoid
any[]. Use generated types for correctness and IDE help.-interface SidebarHistoryProps { - chats: any[]; +import type { Doc } from "@/convex/_generated/dataModel"; + +interface SidebarHistoryProps { + chats: Array<Doc<"chats">>;app/hooks/useChats.ts (1)
24-31: Guard global state updates by fetch condition and auth.Prevent unnecessary global resets when
shouldFetchis false or user not ready.- useEffect(() => { - if (paginatedChats.results) { - setChats(paginatedChats.results); - } - }, [paginatedChats.results, setChats]); + useEffect(() => { + if (!user || !shouldFetch) return; + if (paginatedChats.results) { + setChats(paginatedChats.results); + } + }, [user, shouldFetch, paginatedChats.results, setChats]);app/components/ChatItem.tsx (1)
3-3: Auto-focus and select title on open (UX polish).Select the entire title for quick rename.
-import React, { useState, useRef } from "react"; +import React, { useState, useRef, useEffect } from "react";<Dialog open={showRenameDialog} onOpenChange={setShowRenameDialog}> <DialogContent className="sm:max-w-[425px]"> + {/* Focus and select on open */} + {/* eslint-disable-next-line react-hooks/rules-of-hooks */} + {useEffect(() => { + if (showRenameDialog) { + inputRef.current?.focus(); + inputRef.current?.select(); + } + }, [showRenameDialog])}Also applies to: 215-255
convex/chats.ts (3)
171-205: Add an explicit returns validator for the paginated query.Project guideline: always include returns. Define the paginated shape to match
paginate.export const getUserChats = query({ args: { paginationOpts: paginationOptsValidator, }, + returns: v.object({ + page: v.array( + v.object({ + _id: v.id("chats"), + _creationTime: v.number(), + id: v.string(), + title: v.string(), + user_id: v.string(), + finish_reason: v.optional(v.string()), + todos: v.optional( + v.array( + v.object({ + id: v.string(), + content: v.string(), + status: v.union( + v.literal("pending"), + v.literal("in_progress"), + v.literal("completed"), + v.literal("cancelled"), + ), + }), + ), + ), + update_time: v.number(), + }), + ), + isDone: v.boolean(), + continueCursor: v.union(v.string(), v.null()), + }),
46-57: Prefer .unique() when the index enforces uniqueness.If
by_chat_idis unique,.unique()will catch accidental duplicates.- const chat = await ctx.db - .query("chats") - .withIndex("by_chat_id", (q) => q.eq("id", args.id)) - .first(); + const chat = await ctx.db + .query("chats") + .withIndex("by_chat_id", (q) => q.eq("id", args.id)) + .unique();
338-409: Consider chunking or orchestrating “delete all” to avoid timeouts.Iterating all chats/messages in one mutation may exceed execution limits on large accounts. Consider:
- Paginating messages per chat (e.g., batches of 100).
- Or an action orchestrator that iterates and calls an internal mutation per batch.
app/components/SidebarUserNav.tsx (2)
195-199: Confirm “destructive” visual affordance for irreversible actionConsider adding a destructive variant or class to the DropdownMenuItem to visually communicate risk, consistent with the dialog button styling.
-<DropdownMenuItem onClick={() => setShowDeleteDialog(true)}> +<DropdownMenuItem + onClick={() => setShowDeleteDialog(true)} + className="text-destructive focus:text-destructive" +>
207-230: Copy tweak: clarify scope and permanenceSmall clarity upgrade to reduce support tickets: explicitly say “all chats in this account” and that it removes files/feedback too (matching backend behavior).
-<AlertDialogTitle> - Clear your chat history - are you sure? -</AlertDialogTitle> +<AlertDialogTitle>Delete all chats?</AlertDialogTitle> -<AlertDialogDescription> - This action cannot be undone. This will permanently delete all - your chats and remove all associated data from our servers. -</AlertDialogDescription> +<AlertDialogDescription> + This permanently deletes all chats in this account, including related messages, files, and feedback. This action cannot be undone. +</AlertDialogDescription>app/components/SidebarHeader.tsx (2)
86-93: Simplify navigation refresh logicYou can avoid the pathname check by using
router.replace("/")followed byrouter.refresh()when needed. This keeps SPA perf and ensures re-init.-router.push("/"); -// Force a refresh of the page state by using replace if already on home -if (window.location.pathname === "/") { - router.replace("/"); -} +router.replace("/"); +router.refresh?.();
49-56: More robust platform detection
navigator.userAgentcan misidentify iPadOS as Mac. If you only need the modifier hint, consider checkingnavigator.platformand falling back tonavigator.userAgentData?.platform.-const isMac = useMemo( - () => /macintosh|mac os x/i.test(navigator.userAgent), - [], -); +const isMac = useMemo(() => { + const p = (navigator as any).userAgentData?.platform ?? navigator.platform ?? ""; + return /mac/i.test(p); +}, []);app/components/MessageSearchDialog.tsx (2)
128-161: Observer lifecycle: avoid stale observes when status togglesWhen
statusflips between pages, ensure you unobserve before re-observing to prevent duplicate callbacks.- if ( - loaderRef.current && - debouncedQuery.trim() && - searchResults.status === "CanLoadMore" - ) { - observerRef.current.observe(loaderRef.current); - } + if (loaderRef.current) { + observerRef.current.unobserve?.(loaderRef.current); + } + if (loaderRef.current && debouncedQuery.trim() && searchResults.status === "CanLoadMore") { + observerRef.current.observe(loaderRef.current); + }
162-170: Type the click handler with Id<'chats'>Aligns with the stronger ID typing above.
-const handleChatClick = (chatId: string) => { +const handleChatClick = (chatId: Id<"chats">) => {app/components/chat.tsx (2)
135-151: Title/todos sync effect is fine; consider activating chat from route.If the sidebar highlight still depends on global state, add a small effect to keep it in sync with the route ID.
Example (add near other effects):
useEffect(() => { if (routeChatId) activateChat(routeChatId); }, [routeChatId, activateChat]);
263-277: HardcodingisSwitchingChats={false}may hide loading affordances.If you still track
isSwitchingChatsin global state, prefer passing it through or removing it entirely fromMessages.- isSwitchingChats={false} + isSwitchingChats={isSwitchingChats}convex/messages.ts (3)
7-16: Type the message parts helper.Avoid
any[]forparts. Provide a minimal discriminated union to improve safety.-const extractTextFromParts = (parts: any[]): string => { +type TextPart = { type: "text"; text?: string }; +type MessagePart = TextPart | { type: string; [k: string]: unknown }; +const extractTextFromParts = (parts: Array<MessagePart>): string => { return parts - .filter((part) => part.type === "text") - .map((part) => part.text || "") + .filter((part): part is TextPart => part.type === "text") + .map((part) => part.text ?? "") .join(" ") .trim(); };
479-496: N+1 queries for chat titles; batch fetch or cache to reduce latency.Fetching a chat per message scales poorly on large result sets. At minimum, parallelize and cache within the request.
Example sketch:
const chatCache = new Map<string, { title: string; update_time: number }>(); const getChat = async (id: string) => { if (chatCache.has(id)) return chatCache.get(id)!; const c = await ctx.db.query("chats").withIndex("by_chat_id", q => q.eq("id", id)).first(); const v = { title: c?.title ?? "", update_time: c?.update_time ?? 0 }; chatCache.set(id, v); return v; }; // then in the loop: const chat = await getChat(msg.chat_id);
520-530: Consistentupdated_atfor title-only results.Good to prefer chat
update_time. Consider also falling back torecentMessage?.update_timefor consistency ifupdate_timeis missing.components/ui/dialog.tsx (5)
70-76: Make the close icon decorative for screen readers.Prevent the icon from being announced; the sr-only text already provides the accessible name.
Apply this diff:
- <XIcon /> + <XIcon aria-hidden="true" focusable="false" />
72-72: Replace non-standard Tailwind classes (rounded-xs,focus:outline-hidden).Unless you extended Tailwind, these classes are no-ops. Prefer standard tokens.
Apply this diff:
- className="ring-offset-background focus:ring-ring data-[state=open]:bg-accent data-[state=open]:text-muted-foreground absolute top-4 right-4 rounded-xs opacity-70 transition-opacity hover:opacity-100 focus:ring-2 focus:ring-offset-2 focus:outline-hidden disabled:pointer-events-none [&_svg]:pointer-events-none [&_svg]:shrink-0 [&_svg:not([class*='size-'])]:size-4" + className="ring-offset-background focus:ring-ring data-[state=open]:bg-accent data-[state=open]:text-muted-foreground absolute top-4 right-4 rounded-sm opacity-70 transition-opacity hover:opacity-100 focus:ring-2 focus:ring-offset-2 focus:outline-none disabled:pointer-events-none [&_svg]:pointer-events-none [&_svg]:shrink-0 [&_svg:not([class*='size-'])]:size-4"
41-41: Safer stacking: lower overlay z-index below content.Avoid edge cases where overlay could overlap content under custom stacking contexts.
Apply this diff:
- "data-[state=open]:animate-in data-[state=closed]:animate-out data-[state=closed]:fade-out-0 data-[state=open]:fade-in-0 fixed inset-0 z-50 bg-black/50", + "data-[state=open]:animate-in data-[state=closed]:animate-out data-[state=closed]:fade-out-0 data-[state=open]:fade-in-0 fixed inset-0 z-40 bg-black/50",
49-57: Forward refs forDialogContentto match Radix ergonomics.Enables parent components to focus or measure the content node; aligns with common Radix wrapper patterns.
Apply this diff:
-function DialogContent({ - className, - children, - showCloseButton = true, - ...props -}: React.ComponentProps<typeof DialogPrimitive.Content> & { - showCloseButton?: boolean; -}) { +const DialogContent = React.forwardRef< + React.ElementRef<typeof DialogPrimitive.Content>, + React.ComponentPropsWithoutRef<typeof DialogPrimitive.Content> & { + showCloseButton?: boolean; + } +>(function DialogContent( + { className, children, showCloseButton = true, ...props }, + ref, +) { return ( <DialogPortal data-slot="dialog-portal"> <DialogOverlay /> <DialogPrimitive.Content data-slot="dialog-content" + ref={ref} className={cn( "bg-background data-[state=open]:animate-in data-[state=closed]:animate-out data-[state=closed]:fade-out-0 data-[state=open]:fade-in-0 data-[state=closed]:zoom-out-95 data-[state=open]:zoom-in-95 fixed top-[50%] left-[50%] z-50 grid w-full max-w-[calc(100%-2rem)] translate-x-[-50%] translate-y-[-50%] gap-4 rounded-lg border p-6 shadow-lg duration-200 sm:max-w-lg", className, )} {...props} > {children} {showCloseButton && ( <DialogPrimitive.Close data-slot="dialog-close" className="ring-offset-background focus:ring-ring data-[state=open]:bg-accent data-[state=open]:text-muted-foreground absolute top-4 right-4 rounded-sm opacity-70 transition-opacity hover:opacity-100 focus:ring-2 focus:ring-offset-2 focus:outline-none disabled:pointer-events-none [&_svg]:pointer-events-none [&_svg]:shrink-0 [&_svg:not([class*='size-'])]:size-4" > <XIcon aria-hidden="true" focusable="false" /> <span className="sr-only">Close</span> </DialogPrimitive.Close> )} </DialogPrimitive.Content> </DialogPortal> ); -} +});Also applies to: 60-66, 78-81
58-59: Redundantdata-slotonDialogPortalusage.Your wrapper already injects
data-slot; passing it again here is unnecessary.
📜 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 (15)
app/c/[id]/page.tsx(1 hunks)app/components/ChatItem.tsx(5 hunks)app/components/MessageSearchDialog.tsx(1 hunks)app/components/Sidebar.tsx(5 hunks)app/components/SidebarHeader.tsx(1 hunks)app/components/SidebarHistory.tsx(1 hunks)app/components/SidebarUserNav.tsx(5 hunks)app/components/chat.tsx(11 hunks)app/contexts/GlobalState.tsx(8 hunks)app/hooks/useChats.ts(1 hunks)components/ui/dialog.tsx(1 hunks)convex/chats.ts(2 hunks)convex/messages.ts(5 hunks)convex/schema.ts(2 hunks)middleware.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
convex/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/convex_rules.mdc)
convex/**/*.ts: Always use the new Convex function syntax (query/mutation/action objects with args/returns/handler) when defining Convex functions
When a function returns null, include returns: v.null() and return null explicitly
Use internalQuery/internalMutation/internalAction for private functions callable only by other Convex functions; do not expose sensitive logic via public query/mutation/action
Use query/mutation/action only for public API functions
Do not try to register functions via the api or internal objects
Always include argument and return validators for all Convex functions (query/internalQuery/mutation/internalMutation/action/internalAction)
In JS implementations, functions without an explicit return value implicitly return null
Use ctx.runQuery from queries/mutations/actions to call a query
Use ctx.runMutation from mutations/actions to call a mutation
Use ctx.runAction from actions to call an action
Only call an action from another action when crossing runtimes (e.g., V8 to Node); otherwise extract shared helper code
Minimize calls from actions to queries/mutations to avoid race conditions from splitting transactions
Pass FunctionReference values (from api/internal) to ctx.runQuery/ctx.runMutation/ctx.runAction; do not pass function implementations
When calling a function in the same file via ctx.run*, add an explicit return type annotation at the call site to avoid TS circularity
Use the generated api object for public functions and internal object for internal functions from convex/_generated/api.ts
Respect file-based routing for function references: e.g., convex/example.ts export f -> api.example.f; nested paths map to dot-separated namespaces
For paginated queries use paginationOptsValidator in args and .paginate(args.paginationOpts) on a query
v.bigint() is deprecated; use v.int64() for signed 64-bit integers
Use v.record(keys, values) for record-like data; v.map() and v.set() are not supported
For full-text search, use withSearchIndex("ind...
Files:
convex/chats.tsconvex/messages.tsconvex/schema.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/convex_rules.mdc)
**/*.{ts,tsx}: Use Id helper type from ./_generated/dataModel to type document IDs (e.g., Id<'users'>) instead of string
When defining Record types, specify key and value types matching validators (e.g., Record<Id<'users'>, string>)
Be strict with types for document IDs; prefer Id<'table'> over string in function args and variables
Useas constfor string literals in discriminated unions
Declare arrays with explicit generic type: const arr: Array = [...]
Declare records with explicit generic types: const record: Record<KeyType, ValueType> = {...}
Files:
convex/chats.tsmiddleware.tsapp/hooks/useChats.tsapp/components/chat.tsxapp/components/SidebarHeader.tsxapp/c/[id]/page.tsxapp/components/SidebarHistory.tsxapp/components/MessageSearchDialog.tsxapp/components/ChatItem.tsxcomponents/ui/dialog.tsxapp/components/SidebarUserNav.tsxconvex/messages.tsapp/contexts/GlobalState.tsxapp/components/Sidebar.tsxconvex/schema.ts
convex/schema.ts
📄 CodeRabbit inference engine (.cursor/rules/convex_rules.mdc)
convex/schema.ts: Define the Convex schema in convex/schema.ts
Import schema definition functions (defineSchema, defineTable) from convex/server
Understand system fields: _id uses v.id(tableName) and _creationTime uses v.number(); they’re auto-added to all documents
Include all indexed fields in the index name (e.g., by_field1_and_field2 for ["field1","field2"])
Query indexes in the same column order as defined; create separate indexes for alternate orders
Files:
convex/schema.ts
🧠 Learnings (1)
📚 Learning: 2025-08-29T13:33:09.937Z
Learnt from: CR
PR: hackerai-tech/hackerai#0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-08-29T13:33:09.937Z
Learning: Applies to convex/**/*.ts : For paginated queries use paginationOptsValidator in args and .paginate(args.paginationOpts) on a query
Applied to files:
convex/messages.ts
🧬 Code graph analysis (12)
convex/chats.ts (1)
convex/_generated/server.js (2)
mutation(49-49)mutation(49-49)
app/hooks/useChats.ts (1)
app/contexts/GlobalState.tsx (1)
useGlobalState(344-350)
app/components/chat.tsx (1)
app/contexts/GlobalState.tsx (1)
useGlobalState(344-350)
app/components/SidebarHeader.tsx (5)
hooks/use-mobile.ts (1)
useIsMobile(5-21)app/contexts/GlobalState.tsx (1)
useGlobalState(344-350)app/hooks/useChats.ts (1)
useChats(13-32)components/icons/hackerai-svg.tsx (1)
HackerAISVG(8-67)app/components/MessageSearchDialog.tsx (1)
MessageSearchDialog(50-398)
app/c/[id]/page.tsx (1)
app/components/chat.tsx (1)
Chat(28-373)
app/components/MessageSearchDialog.tsx (3)
app/contexts/GlobalState.tsx (1)
useGlobalState(344-350)hooks/use-mobile.ts (1)
useIsMobile(5-21)convex/_generated/dataModel.d.ts (1)
Doc(30-33)
app/components/ChatItem.tsx (3)
app/contexts/GlobalState.tsx (1)
useGlobalState(344-350)hooks/use-mobile.ts (1)
useIsMobile(5-21)convex/chats.ts (2)
deleteChat(210-283)renameChat(288-336)
components/ui/dialog.tsx (1)
lib/utils.ts (1)
cn(16-18)
app/components/SidebarUserNav.tsx (3)
convex/chats.ts (1)
deleteAllChats(341-409)components/ui/dropdown-menu.tsx (4)
DropdownMenuSeparator(252-252)DropdownMenuItem(248-248)DropdownMenuContent(245-245)DropdownMenu(242-242)components/ui/alert-dialog.tsx (8)
AlertDialog(146-146)AlertDialogContent(150-150)AlertDialogHeader(151-151)AlertDialogTitle(153-153)AlertDialogDescription(154-154)AlertDialogFooter(152-152)AlertDialogCancel(156-156)AlertDialogAction(155-155)
convex/messages.ts (1)
convex/_generated/server.js (2)
query(29-29)query(29-29)
app/contexts/GlobalState.tsx (1)
convex/_generated/dataModel.d.ts (1)
Doc(30-33)
app/components/Sidebar.tsx (3)
app/contexts/GlobalState.tsx (1)
useGlobalState(344-350)app/hooks/useChats.ts (1)
useChats(13-32)hooks/use-mobile.ts (1)
useIsMobile(5-21)
🔇 Additional comments (13)
convex/schema.ts (1)
27-31: Search index on chats title looks goodPer-user filter aligns with planned search UX.
app/contexts/GlobalState.tsx (3)
54-57: Chats state typings look goodUsing Doc<"chats">[] here is correct.
254-256: Clearing chatTitle on new chat is appropriatePrevents stale titles.
302-304: Exposing chats and setChats via context looks goodMatches the new useChats hook flow.
convex/chats.ts (1)
258-269: Good addition: feedback cleanup prevents orphans.Thanks for deleting
message.feedback_idwith tolerant error handling.app/components/MessageSearchDialog.tsx (1)
270-326: Doc ID access: confirmchat.idvschat._idYou’re using
chat.idfor routing. Ensure chats in global state exposeid; Convex docs typically use_id. If not mapped, this will break navigation.Run a quick repo search to verify the shape and mapping of chat documents into global state.
app/components/Sidebar.tsx (2)
45-80: Desktop sidebar collapse gating is clean.Conditionally rendering the chat list when collapsed is correct and avoids unnecessary work.
82-121: Mobile overlay path: solid simplification; ensure close behavior is consistent.Looks good. Confirm that the parent overlay click-to-close (in Chat) and the header Close button both call
setChatSidebarOpen(false)to avoid state desyncs.convex/messages.ts (2)
83-92: Persistingcontentderived from parts is good.Storing normalized text enables full‑text search and is backwards compatible.
247-256: Mirroringcontentpersistence in client save path is correct.Keeps searchability consistent for client-inserted messages.
components/ui/dialog.tsx (3)
9-31: Typed wrappers look solid.Good use of
React.ComponentPropsto inherit Radix props and keep API parity.
83-101: Header/Footer slots are clean.Simple, flexible wrappers with sensible defaults.
106-129: Title/Description wrappers are correct.Consider forwarding refs later for parity, but this is fine as-is.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
app/components/Sidebar.tsx (1)
33-39: Type chat IDs with Id<'chats'> and keep highlight in sync with the routeEnsure
currentChatIdisId<'chats'> | null(from ./_generated/dataModel) end-to-end and that the highlighted chat tracks the route param (derive it from the URL or keep global state synced). This was raised earlier; still relevant here asSidebarHistoryreceivescurrentChatId.Run to verify typing and usages:
#!/bin/bash rg -nP --type=ts --type=tsx "(Id<'chats'>|currentChatId)" -C3app/components/SidebarHeader.tsx (1)
118-138: Enable thegroup-hoveroverlayParent is missing the
groupclass, so the overlay never appears.- <div - className="relative flex items-center justify-center mb-2 cursor-pointer focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-ring focus-visible:ring-offset-2 rounded" + <div + className="group relative flex items-center justify-center mb-2 cursor-pointer focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-ring focus-visible:ring-offset-2 rounded"app/components/SidebarUserNav.tsx (1)
3-3: Prevent dialog close during delete and use router navigationKeep the dialog open while the mutation is pending and prefer client-side navigation after deletion. Matches prior guidance.
import React, { useState } from "react"; +import { useRouter } from "next/navigation";const SidebarUserNav = ({ isCollapsed = false }: { isCollapsed?: boolean }) => { + const router = useRouter();const handleDeleteAllChats = async () => { try { setIsDeleting(true); await deleteAllChats(); setShowDeleteDialog(false); - // Optionally redirect to home or show success message - window.location.href = "/"; + router.replace("/"); } catch (error) {- <AlertDialog open={showDeleteDialog} onOpenChange={setShowDeleteDialog}> + <AlertDialog + open={showDeleteDialog} + onOpenChange={(open) => { + if (isDeleting) return; + setShowDeleteDialog(open); + }} + >Also applies to: 49-49, 100-113, 261-261
🧹 Nitpick comments (6)
app/components/Sidebar.tsx (3)
28-30: Avoid duplicate fetching of chats
useChats()is called here and also from the header when the search dialog opens. Consider relying on the global cache the hook already writes to, or passuseChats(false)in one of the places and read from global state to prevent redundant queries.
32-32: Remove unnecessary template literalMinor cleanup.
- <div className={`h-full overflow-y-auto`} ref={scrollContainerRef}> + <div className="h-full overflow-y-auto" ref={scrollContainerRef}>
45-49: Prefer CSS breakpoints over JSisMobilefor widthUse responsive classes and drop the
isMobileprop to reduce layout jank and simplify props.-const DesktopSidebarContent: FC<{ - isMobile: boolean; - handleCloseSidebar: () => void; -}> = ({ isMobile, handleCloseSidebar }) => { +const DesktopSidebarContent: FC<{ + handleCloseSidebar: () => void; +}> = ({ handleCloseSidebar }) => { @@ - className={`${isMobile ? "w-full" : "w-72"}`} + className="w-full md:w-72"- const isMobile = useIsMobile(); + // isMobile no longer needed; width handled via CSS breakpoints- <DesktopSidebarContent - isMobile={isMobile} - handleCloseSidebar={handleCloseSidebar} - /> + <DesktopSidebarContent handleCloseSidebar={handleCloseSidebar} />Additionally, remove the now-unused
useIsMobileimport.Also applies to: 56-56, 82-86, 117-121
app/components/SidebarHeader.tsx (3)
68-80: Don’t hijack Cmd/Ctrl+K while typingSkip the shortcut when focus is inside inputs, textareas, selects, or contentEditable.
useEffect(() => { const handleKeyDown = (e: KeyboardEvent) => { - if ((e.metaKey || e.ctrlKey) && e.key === "k") { + const target = e.target as HTMLElement | null; + const tag = target?.tagName?.toLowerCase(); + const isEditable = + target?.isContentEditable || + tag === "input" || + tag === "textarea" || + tag === "select"; + if (isEditable) return; + if ((e.metaKey || e.ctrlKey) && (e.key === "k" || e.key === "K")) { e.preventDefault(); setIsSearchOpen(true); } };
96-103: Avoid push followed by replace; use refresh when already on “/”Prevents double navigation.
- router.push("/"); - // Force a refresh of the page state by using replace if already on home - if (window.location.pathname === "/") { - router.replace("/"); - } + if (window.location.pathname === "/") { + router.refresh?.(); + } else { + router.push("/"); + }
59-66: Guardnavigatoraccess (minor)Very minor: if this component is ever rendered in a non-browser env, guard
navigator.- const isMac = useMemo( - () => /macintosh|mac os x/i.test(navigator.userAgent), - [], - ); + const isMac = useMemo(() => { + if (typeof navigator === "undefined") return false; + return /macintosh|mac os x/i.test(navigator.userAgent); + }, []);
📜 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)
app/components/Sidebar.tsx(5 hunks)app/components/SidebarHeader.tsx(1 hunks)app/components/SidebarUserNav.tsx(5 hunks)lib/db/actions.ts(0 hunks)
💤 Files with no reviewable changes (1)
- lib/db/actions.ts
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/convex_rules.mdc)
**/*.{ts,tsx}: Use Id helper type from ./_generated/dataModel to type document IDs (e.g., Id<'users'>) instead of string
When defining Record types, specify key and value types matching validators (e.g., Record<Id<'users'>, string>)
Be strict with types for document IDs; prefer Id<'table'> over string in function args and variables
Useas constfor string literals in discriminated unions
Declare arrays with explicit generic type: const arr: Array = [...]
Declare records with explicit generic types: const record: Record<KeyType, ValueType> = {...}
Files:
app/components/SidebarHeader.tsxapp/components/Sidebar.tsxapp/components/SidebarUserNav.tsx
🧬 Code graph analysis (3)
app/components/SidebarHeader.tsx (5)
hooks/use-mobile.ts (1)
useIsMobile(5-21)app/contexts/GlobalState.tsx (1)
useGlobalState(344-350)app/hooks/useChats.ts (1)
useChats(13-32)components/icons/hackerai-svg.tsx (1)
HackerAISVG(8-67)app/components/MessageSearchDialog.tsx (1)
MessageSearchDialog(50-398)
app/components/Sidebar.tsx (3)
app/contexts/GlobalState.tsx (1)
useGlobalState(344-350)app/hooks/useChats.ts (1)
useChats(13-32)hooks/use-mobile.ts (1)
useIsMobile(5-21)
app/components/SidebarUserNav.tsx (5)
app/contexts/GlobalState.tsx (1)
useGlobalState(344-350)app/hooks/useUpgrade.ts (1)
useUpgrade(5-72)convex/chats.ts (1)
deleteAllChats(341-409)components/ui/dropdown-menu.tsx (5)
DropdownMenuItem(248-248)DropdownMenuSub(254-254)DropdownMenuSubTrigger(255-255)DropdownMenuSubContent(256-256)DropdownMenuSeparator(252-252)components/ui/alert-dialog.tsx (8)
AlertDialog(146-146)AlertDialogContent(150-150)AlertDialogHeader(151-151)AlertDialogTitle(153-153)AlertDialogDescription(154-154)AlertDialogFooter(152-152)AlertDialogCancel(156-156)AlertDialogAction(155-155)
🪛 Biome (2.1.2)
app/components/SidebarHeader.tsx
[error] 41-41: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (3)
app/components/SidebarHeader.tsx (1)
115-135: Fix non-working group-hover overlayThe overlay uses group-hover but the parent lacks the group class, so it never appears on hover.
- className="relative flex items-center justify-center mb-2 cursor-pointer focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-ring focus-visible:ring-offset-2 rounded" + className="group relative flex items-center justify-center mb-2 cursor-pointer focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-ring focus-visible:ring-offset-2 rounded"app/components/SidebarUserNav.tsx (2)
101-114: Fix delete flow: pass args to mutation and use router.replace for client-side nav
- Convex mutations with args: {} should be invoked as deleteAllChats({}) to satisfy TS.
- Prefer Next.js router.replace("/") over window.location.href to avoid full reload.
@@ -import React, { useState } from "react"; +import React, { useState } from "react"; +import { useRouter } from "next/navigation"; @@ const SidebarUserNav = ({ isCollapsed = false }: { isCollapsed?: boolean }) => { const { user } = useAuth(); const { hasProPlan, isCheckingProPlan } = useGlobalState(); const { handleUpgrade } = useUpgrade(); + const router = useRouter(); @@ const handleDeleteAllChats = async () => { try { setIsDeleting(true); - await deleteAllChats(); + await deleteAllChats({}); setShowDeleteDialog(false); - // Optionally redirect to home or show success message - window.location.href = "/"; + // Optionally show success toast here + router.replace("/"); } catch (error) { console.error("Failed to delete all chats:", error); // Optionally show error message to user } finally { setIsDeleting(false); } };Also applies to: 3-3, 50-56
261-264: Prevent dialog from closing while deletion is in-flightGuard onOpenChange so ESC/backdrop can’t dismiss during isDeleting.
- <AlertDialog open={showDeleteDialog} onOpenChange={setShowDeleteDialog}> + <AlertDialog + open={showDeleteDialog} + onOpenChange={(open) => { + if (isDeleting) return; + setShowDeleteDialog(open); + }} + >
🧹 Nitpick comments (8)
app/components/SidebarHeader.tsx (5)
4-4: Prefer router.refresh + usePathname over window.locationImport usePathname for route checks, avoiding window usage.
-import { useRouter } from "next/navigation"; +import { useRouter, usePathname } from "next/navigation";
41-42: Add pathname hook usageUse Next’s hook to check the current route.
const router = useRouter(); + const pathname = usePathname();
93-100: Avoid double navigation when already on “/”Use router.refresh() if already on the homepage; otherwise push. This prevents redundant push+replace and removes a direct window reference.
- router.push("/"); - // Force a refresh of the page state by using replace if already on home - if (window.location.pathname === "/") { - router.replace("/"); - } + if (pathname === "/") { + router.refresh(); + } else { + router.push("/"); + }
160-161: Remove unused hover handlers in collapsed search buttonThese update state that isn’t used in the collapsed UI; drop them to avoid needless re-renders.
- onMouseEnter={() => setIsSearchHovered(true)} - onMouseLeave={() => setIsSearchHovered(false)}
115-129: Optional: use a semantic button for the logo triggerA button element gives built-in keyboard semantics and removes the need for manual key handling.
- <div - className="group relative flex items-center justify-center mb-2 cursor-pointer focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-ring focus-visible:ring-offset-2 rounded" - onClick={toggleSidebar} - onKeyDown={(e) => { - if (e.key === "Enter" || e.key === " ") { - if (e.key === " ") { - e.preventDefault(); - } - toggleSidebar(); - } - }} - tabIndex={0} - role="button" - aria-label="Expand sidebar" - > + <button + type="button" + className="group relative flex items-center justify-center mb-2 cursor-pointer focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-ring focus-visible:ring-offset-2 rounded" + onClick={toggleSidebar} + aria-label="Expand sidebar" + > @@ - </div> + </button>app/components/SidebarUserNav.tsx (3)
68-99: DRY external link handlingFactor a tiny helper to avoid repeating window.open boilerplate and ensure consistency.
const handleHelpCenter = () => { - const newWindow = window.open( - NEXT_PUBLIC_HELP_CENTER_URL, - "_blank", - "noopener,noreferrer", - ); - if (newWindow) { - newWindow.opener = null; - } + openExternal(NEXT_PUBLIC_HELP_CENTER_URL); }; - const handleGitHub = () => { - const newWindow = window.open( - "https://github.com/hackerai-tech/hackerai", - "_blank", - "noopener,noreferrer", - ); - if (newWindow) { - newWindow.opener = null; - } - }; + const openExternal = (url: string) => { + const w = window.open(url, "_blank", "noopener,noreferrer"); + if (w) w.opener = null; + }; - const handleXCom = () => { - const newWindow = window.open( - "https://x.com/hackerai_tech", - "_blank", - "noopener,noreferrer", - ); - if (newWindow) { - newWindow.opener = null; - } - }; + const handleGitHub = () => openExternal("https://github.com/hackerai-tech/hackerai"); + const handleXCom = () => openExternal("https://x.com/hackerai_tech");
108-111: Surface user-facing feedback on failureConsole.error is invisible to users. Consider a toast/snackbar to confirm success/failure.
If you’re using shadcn/sonner or a custom toast, I can wire it in here.
254-257: Minor visual consistency: tint LogOut iconMatch other items’ icon tint.
- <LogOut className="mr-2 h-4 w-4" /> + <LogOut className="mr-2 h-4 w-4 text-foreground" />
📜 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)
app/components/ChatHeader.tsx(2 hunks)app/components/SidebarHeader.tsx(1 hunks)app/components/SidebarUserNav.tsx(5 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/convex_rules.mdc)
**/*.{ts,tsx}: Use Id helper type from ./_generated/dataModel to type document IDs (e.g., Id<'users'>) instead of string
When defining Record types, specify key and value types matching validators (e.g., Record<Id<'users'>, string>)
Be strict with types for document IDs; prefer Id<'table'> over string in function args and variables
Useas constfor string literals in discriminated unions
Declare arrays with explicit generic type: const arr: Array = [...]
Declare records with explicit generic types: const record: Record<KeyType, ValueType> = {...}
Files:
app/components/ChatHeader.tsxapp/components/SidebarUserNav.tsxapp/components/SidebarHeader.tsx
🧬 Code graph analysis (2)
app/components/SidebarUserNav.tsx (3)
app/contexts/GlobalState.tsx (1)
useGlobalState(344-350)app/hooks/useUpgrade.ts (1)
useUpgrade(5-72)convex/chats.ts (1)
deleteAllChats(341-409)
app/components/SidebarHeader.tsx (6)
hooks/use-mobile.ts (1)
useIsMobile(5-21)app/contexts/GlobalState.tsx (1)
useGlobalState(344-350)app/hooks/useChats.ts (1)
useChats(13-32)components/icons/hackerai-svg.tsx (1)
HackerAISVG(8-67)app/components/MessageSearchDialog.tsx (1)
MessageSearchDialog(50-398)components/ui/sidebar.tsx (1)
useSidebar(776-776)
🔇 Additional comments (5)
app/components/SidebarHeader.tsx (2)
243-269: Resolved: useSidebar no longer called conditionallySplitting into Desktop/Mobile components removes the conditional hook usage. Good fix.
Also applies to: 271-292
17-17: No change needed for MessageSearchDialog importMessageSearchDialog is exported as a named constant in app/components/MessageSearchDialog.tsx (line 50), so the existing
import { MessageSearchDialog } from "./MessageSearchDialog";is correct.app/components/SidebarUserNav.tsx (2)
226-246: Nice submenu UX polishHelp submenu + icon tinting is consistent and clear. LGTM.
249-252: Destructive action entry reads wellClear label and icon; opens confirm dialog. LGTM.
app/components/ChatHeader.tsx (1)
10-17: Use chatId: Id<'chats'> instead of id: string in ChatHeader
- At top of app/components/ChatHeader.tsx add
import type { Id } from "@/convex/_generated/dataModel";- In ChatHeaderProps replace
id?: string;→chatId?: Id<'chats'>;- In the component signature and JSX replace all usages of
idwithchatId(e.g. default title logic)- In app/components/chat.tsx update the call-site:
<ChatHeader id={routeChatId} … />→<ChatHeader chatId={routeChatId} … />
Verify that"chats"is a valid table name in your generated dataModel.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
app/components/SidebarUserNav.tsx (1)
101-114: Keep dialog open while deleting and avoid full page reload on success
- Prevent closing via ESC/backdrop while
isDeletingis true by guardingonOpenChange.- Prefer client-side navigation with
router.replace("/")instead ofwindow.location.hrefafter deletion.Apply this diff within the selected ranges:
- const handleDeleteAllChats = async () => { + const handleDeleteAllChats = async () => { try { setIsDeleting(true); await deleteAllChats(); setShowDeleteDialog(false); - // Optionally redirect to home or show success message - window.location.href = "/"; + // Redirect home without a full reload + router.replace("/"); } catch (error) { console.error("Failed to delete all chats:", error); // Optionally show error message to user } finally { setIsDeleting(false); } };- <AlertDialog open={showDeleteDialog} onOpenChange={setShowDeleteDialog}> + <AlertDialog + open={showDeleteDialog} + onOpenChange={(open) => { + if (isDeleting) return; + setShowDeleteDialog(open); + }} + >Additions needed outside the selected ranges:
// imports import { useRouter } from "next/navigation"; // inside component (near other hooks) const router = useRouter();Also applies to: 268-291
🧹 Nitpick comments (2)
app/components/SidebarUserNav.tsx (2)
79-99: Drop redundantnewWindow.opener = null(already covered bynoopener)You already pass
"noopener,noreferrer"towindow.open, which prevents settingopener. Setting it tonullis redundant.- const newWindow = window.open( + const newWindow = window.open( "https://github.com/hackerai-tech/hackerai", "_blank", "noopener,noreferrer", ); - if (newWindow) { - newWindow.opener = null; - }Apply similarly in
handleXComandhandleHelpCenter.
64-67: Optional: Use router navigation for logout for consistencyNot required, but consider
router.replace("/logout")for consistency with client-side navigation elsewhere.
📜 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)
app/components/SidebarUserNav.tsx(5 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/convex_rules.mdc)
**/*.{ts,tsx}: Use Id helper type from ./_generated/dataModel to type document IDs (e.g., Id<'users'>) instead of string
When defining Record types, specify key and value types matching validators (e.g., Record<Id<'users'>, string>)
Be strict with types for document IDs; prefer Id<'table'> over string in function args and variables
Useas constfor string literals in discriminated unions
Declare arrays with explicit generic type: const arr: Array = [...]
Declare records with explicit generic types: const record: Record<KeyType, ValueType> = {...}
Files:
app/components/SidebarUserNav.tsx
🧬 Code graph analysis (1)
app/components/SidebarUserNav.tsx (3)
app/contexts/GlobalState.tsx (1)
useGlobalState(344-350)hooks/use-mobile.ts (1)
useIsMobile(5-21)convex/chats.ts (1)
deleteAllChats(341-409)
🔇 Additional comments (2)
app/components/SidebarUserNav.tsx (2)
261-265: VerifyDropdownMenuItemsupportsvariant="destructive"Shadcn/Radix
DropdownMenuItemtypically doesn’t have avariantprop. If your wrapper doesn’t extend it, this may be a type error/no-op.If unsupported, style it explicitly:
-<DropdownMenuItem onClick={handleSignOut} variant="destructive"> +<DropdownMenuItem + onClick={handleSignOut} + className="text-destructive focus:text-destructive data-[highlighted]:bg-destructive/10" +>
226-253: Nested Help menu looks solidAdaptive submenu placement using
useIsMobileand consistent icon tinting read well. No issues spotted.
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
convex/messages.ts (1)
282-288: Avoid.filter(); add an index for (chat_id, role) and use itGuideline: don’t use
filterin DB queries. Define a composite index (e.g.,by_chat_id_role_update_time) and query viawithIndexto fetch the latest assistant message efficiently.Proposed change (schema + query):
- .withIndex("by_chat_id", (q) => q.eq("chat_id", args.chatId)) - .filter((q) => q.eq(q.field("role"), "assistant")) - .order("desc") + // index: index("by_chat_id_role_update_time", ["chat_id", "role", "_creationTime"]) + .withIndex("by_chat_id_role_update_time", (q) => + q.eq("chat_id", args.chatId).eq("role", "assistant"), + ) + .order("desc") .first();If you want, I can draft the
convex/schema.tsindex diff.app/api/chat/route.ts (1)
37-39: Abort the stream when the client aborts to prevent leaks/work continuing server-side.You’re only logging on
req.signal.abort. Tie it to the controller sostreamTexthalts andonAbortruns.Apply this diff:
req.signal.addEventListener("abort", () => { console.log("Request aborted"); + controller.abort(); });
♻️ Duplicate comments (4)
convex/messages.ts (1)
541-567: Fix pagination cursor semantics to be offset-based and nullable when doneUse
nullwhen finished; don’t recycle0/"". Also computenextCursordirectly fromend.- const parsedOffset = args.paginationOpts.cursor - ? parseInt(args.paginationOpts.cursor, 10) || 0 - : 0; - const startIndex = parsedOffset; - const numItems = args.paginationOpts.numItems; - const paginatedResults = combinedResults.slice( - startIndex, - startIndex + numItems, - ); - - const hasMoreItems = startIndex + numItems < combinedResults.length; - const nextOffset = hasMoreItems ? startIndex + numItems : 0; + const numItems = args.paginationOpts.numItems; + const startIndex = + args.paginationOpts.cursor != null + ? Math.max(0, parseInt(String(args.paginationOpts.cursor), 10) || 0) + : 0; + const end = startIndex + numItems; + const paginatedResults = combinedResults.slice(startIndex, end); + const nextCursor = end < combinedResults.length ? String(end) : null; return { page: paginatedResults.map((result) => ({ @@ })), - isDone: startIndex + numItems >= combinedResults.length, - continueCursor: hasMoreItems ? nextOffset.toString() : "", + isDone: nextCursor == null, + continueCursor: nextCursor, };app/components/MessageSearchDialog.tsx (2)
24-36: Type IDs with ConvexIdwhere applicableAt minimum, type
chat_idasId<"chats">. Keepidasstringsince title-only results synthesize an id.-import type { Doc } from "@/convex/_generated/dataModel"; +import type { Doc, Id } from "@/convex/_generated/dataModel"; @@ -interface MessageSearchResult { - id: string; - chat_id: string; +interface MessageSearchResult { + id: string; // message id or synthetic title result id + chat_id: Id<"chats">; content: string; created_at: number; updated_at?: number; chat_title?: string; match_type: "message" | "title" | "both"; }
207-228: Fixed regex highlighting bug—niceUsing escaped pattern, single
iflag, and split-group parity avoidslastIndexpitfalls.app/components/chat.tsx (1)
46-49: Make “existing chat” stateful so it flips after first completion.Deriving from
routeChatIdonly means it never flips for new chats post-first message; fetches (messages/title/todos) won’t run.Apply this diff:
- // Determine if this is an existing chat (has route ID) or new chat - const isExistingChat = !!routeChatId; - const shouldFetchMessages = isExistingChat; + // Track whether this is an existing chat (prop-driven initially, flips after first completion) + const [isExistingChat, setIsExistingChat] = useState<boolean>(!!routeChatId); + const shouldFetchMessages = isExistingChat;
🧹 Nitpick comments (6)
convex/messages.ts (3)
7-16: Type the message parts and harden the extractorAvoid
any[]. Define a validator for parts and use it in args; keeps storage/search consistent and prevents surprises from non-text parts.+// Consider near top-level +const messagePartValidator = v.object({ + type: v.string(), + text: v.optional(v.string()), + // add other supported fields/types as needed +}); // ... export const saveMessage = mutation({ args: { @@ - parts: v.array(v.any()), + parts: v.array(messagePartValidator), @@ }); // ... export const saveAssistantMessageFromClient = mutation({ args: { @@ - parts: v.array(v.any()), + parts: v.array(messagePartValidator),
479-496: Avoid N+1 chat lookups for message resultsYou fetch each chat per message. Cache by
chat_idor prefetch distinct chat ids to reduce round-trips.+const chatCache = new Map<string, { title: string; update_time: number }>(); // inside the loop: -const chat = await ctx.db.query("chats").withIndex("by_chat_id", (q) => q.eq("id", msg.chat_id)).first(); +let chat = chatCache.get(msg.chat_id); +if (!chat) { + const c = await ctx.db + .query("chats") + .withIndex("by_chat_id", (q) => q.eq("id", msg.chat_id)) + .first(); + chat = { title: c?.title ?? "", update_time: c?.update_time ?? msg.update_time }; + chatCache.set(msg.chat_id, chat); +}
151-157: Consistently returncontinueCursor: nullwhen no chatMinor, but aligns with Convex pagination shape.
if (!chatExists) { return { page: [], isDone: true, - continueCursor: "", + continueCursor: null, }; }app/components/MessageSearchDialog.tsx (2)
355-363: Prefer a stable key over index suffixUsing
indexin the key can cause remounts. Considerkey={message.id}if unique across pages; otherwise compose withmessage.chat_idand timestamps.- key={`${message.id}-${index}`} + key={message.id}If
idcan collide (e.g., title results), use a deterministic composite like${message.id}-${message.created_at}.
174-182: TypechatIdparam to matchchat_idtypeIf you adopt
Id<"chats">above, reflect it here for consistency.-const handleChatClick = (chatId: string) => { +const handleChatClick = (chatId: Id<"chats">) => {app/components/chat.tsx (1)
249-253: Pass a stable id to ChatHeader.
id={routeChatId}is undefined for new chats even after you flip state. Prefer the resolvedchatIdso header props stay consistent.Apply this diff:
- hasActiveChat={isExistingChat} + hasActiveChat={isExistingChat} chatTitle={chatTitle} - id={routeChatId} + id={isExistingChat ? chatId : undefined}
📜 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 (5)
app/api/chat/route.ts(1 hunks)app/components/ChatHeader.tsx(2 hunks)app/components/MessageSearchDialog.tsx(1 hunks)app/components/chat.tsx(11 hunks)convex/messages.ts(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- app/components/ChatHeader.tsx
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/convex_rules.mdc)
**/*.{ts,tsx}: Use Id helper type from ./_generated/dataModel to type document IDs (e.g., Id<'users'>) instead of string
When defining Record types, specify key and value types matching validators (e.g., Record<Id<'users'>, string>)
Be strict with types for document IDs; prefer Id<'table'> over string in function args and variables
Useas constfor string literals in discriminated unions
Declare arrays with explicit generic type: const arr: Array = [...]
Declare records with explicit generic types: const record: Record<KeyType, ValueType> = {...}
Files:
app/api/chat/route.tsapp/components/chat.tsxapp/components/MessageSearchDialog.tsxconvex/messages.ts
convex/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/convex_rules.mdc)
convex/**/*.ts: Always use the new Convex function syntax (query/mutation/action objects with args/returns/handler) when defining Convex functions
When a function returns null, include returns: v.null() and return null explicitly
Use internalQuery/internalMutation/internalAction for private functions callable only by other Convex functions; do not expose sensitive logic via public query/mutation/action
Use query/mutation/action only for public API functions
Do not try to register functions via the api or internal objects
Always include argument and return validators for all Convex functions (query/internalQuery/mutation/internalMutation/action/internalAction)
In JS implementations, functions without an explicit return value implicitly return null
Use ctx.runQuery from queries/mutations/actions to call a query
Use ctx.runMutation from mutations/actions to call a mutation
Use ctx.runAction from actions to call an action
Only call an action from another action when crossing runtimes (e.g., V8 to Node); otherwise extract shared helper code
Minimize calls from actions to queries/mutations to avoid race conditions from splitting transactions
Pass FunctionReference values (from api/internal) to ctx.runQuery/ctx.runMutation/ctx.runAction; do not pass function implementations
When calling a function in the same file via ctx.run*, add an explicit return type annotation at the call site to avoid TS circularity
Use the generated api object for public functions and internal object for internal functions from convex/_generated/api.ts
Respect file-based routing for function references: e.g., convex/example.ts export f -> api.example.f; nested paths map to dot-separated namespaces
For paginated queries use paginationOptsValidator in args and .paginate(args.paginationOpts) on a query
v.bigint() is deprecated; use v.int64() for signed 64-bit integers
Use v.record(keys, values) for record-like data; v.map() and v.set() are not supported
For full-text search, use withSearchIndex("ind...
Files:
convex/messages.ts
🧠 Learnings (3)
📚 Learning: 2025-08-29T13:33:09.937Z
Learnt from: CR
PR: hackerai-tech/hackerai#0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-08-29T13:33:09.937Z
Learning: Applies to **/*.{ts,tsx} : Be strict with types for document IDs; prefer Id<'table'> over string in function args and variables
Applied to files:
app/components/MessageSearchDialog.tsx
📚 Learning: 2025-08-29T13:33:09.937Z
Learnt from: CR
PR: hackerai-tech/hackerai#0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-08-29T13:33:09.937Z
Learning: Applies to **/*.{ts,tsx} : Use Id helper type from ./_generated/dataModel to type document IDs (e.g., Id<'users'>) instead of string
Applied to files:
app/components/MessageSearchDialog.tsx
📚 Learning: 2025-08-29T13:33:09.937Z
Learnt from: CR
PR: hackerai-tech/hackerai#0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-08-29T13:33:09.937Z
Learning: Applies to convex/**/*.ts : For paginated queries use paginationOptsValidator in args and .paginate(args.paginationOpts) on a query
Applied to files:
convex/messages.ts
🧬 Code graph analysis (1)
app/components/MessageSearchDialog.tsx (3)
app/contexts/GlobalState.tsx (1)
useGlobalState(344-350)hooks/use-mobile.ts (1)
useIsMobile(5-21)convex/_generated/dataModel.d.ts (1)
Doc(30-33)
🔇 Additional comments (15)
convex/messages.ts (4)
83-92: Persisting derivedcontentlooks goodComputing and storing
contentfor search is correct;undefinedwhen empty avoids storing noise.
247-256: LGTM: client save also persists normalizedcontentSame as above; consistent behavior across server/client paths.
622-627: LGTM: keepcontentin sync on regenerationSetting
contentfromnewContent.trim()keeps search index accurate after edits.
449-463: Index coverage verified
Indexessearch_contenton messages.content,search_titleon chats.title, andby_chat_idon messages.chat_id (and chats.id) are defined as expected.app/components/MessageSearchDialog.tsx (2)
65-73: Correct use ofusePaginatedQuerywith skip and initial pageArgs/skip pattern and
initialNumItemslook good. Will rely on backend returningcontinueCursor: nullwhen done.
125-173: IntersectionObserver teardown and guards look solidYou disconnect previous observer, guard by status, and avoid duplicate loads.
app/api/chat/route.ts (1)
115-121: VerifyproviderOptionsshape
Ensure your AI SDK integration inapp/api/chat/route.ts(lines 116–120) actually consumes the{ provider: { sort: "price" } }format underproviderOptions; if it expects a named provider key (e.g.{ openai: { … } }), this block will be ignored.app/components/chat.tsx (8)
41-44: Stable chatId initialization: LGTM.Keeps a consistent ID across the component’s lifecycle.
159-164: Server-message sync logic: LGTM.Conditioning on
isExistingChatprevents clobbering local in-flight messages for new chats.
179-185: Initial scroll on hydrate: LGTM.Scrolls once messages land for existing chats; dependency list is correct.
221-222: Layout gating: LGTM.
hasMessages || isExistingChatcleanly handles both new and existing chats.
274-274: Explicitly disabling switching state: LGTM.Keeps message UI simple; revisit if you add cross-chat transitions.
311-311: ChatInput visibility rule: LGTM.Always available for existing chats, centered-only mode for brand new chats.
28-28: Approved: All callers updated to usechatId, no staleidprops found.
28-28: Verify ConvexId<'chats'>helper availability
I couldn’t locate the generatedId<'chats'>type—confirm you’ve run Convex codegen and can import it fromconvex/_generated/dataModel.tsbefore updating the prop toId<'chats'>.
| ...(!isPro && { | ||
| providerOptions: { | ||
| provider: { | ||
| sort: "price", | ||
| }, | ||
| }, | ||
| }), |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Fix TS spread of boolean union; use a ternary to avoid “Spread types may only be created from object types”.
...(!isPro && {...}) can resolve to false | {…}, which TypeScript rejects in object spread. Switch to a ternary; also mark the literal as const.
Apply this diff:
- ...(!isPro && {
- providerOptions: {
- provider: {
- sort: "price",
- },
- },
- }),
+ ...(isPro
+ ? {}
+ : {
+ providerOptions: {
+ provider: {
+ sort: "price" as const,
+ },
+ },
+ }),Run to confirm there are no other boolean-union object spreads:
🏁 Script executed:
#!/bin/bash
rg -nP --type=ts --type=tsx '\.\.\s*\((?:!|\w+\s*[=!]==\s*false)[^)]+\)'Length of output: 102
🏁 Script executed:
#!/bin/bash
# Search for boolean-union object spreads across all TS and TSX files
rg -nP -g '*.ts' -g '*.tsx' '\.\.\s*\((?:!|\w+\s*[=!]==\s*false)[^)]+\)'Length of output: 74
🏁 Script executed:
#!/bin/bash
# Search for any spread of a parenthesized expression across TS and TSX files
rg -nP -g '*.ts' -g '*.tsx' '\.\.\.\s*\('Length of output: 327
Fix boolean-union spreads across TS files; use ternary and as const
TypeScript rejects spreading false | object. Replace all ...(cond && {…}) patterns with ternaries that return {} when false, and mark literals as const. For example:
- ...(country && { userLocation: country }),
+ ...(country ? { userLocation: country } : {}),
- ...(process.env.EXA_API_KEY && { apiKey: process.env.EXA_API_KEY }),
+ ...(process.env.EXA_API_KEY
+ ? { apiKey: process.env.EXA_API_KEY }
+ : {}),
- ...(process.env.EXA_API_KEY && { web: allTools.web }),
+ ...(process.env.EXA_API_KEY
+ ? { web: allTools.web }
+ : {}),
- ...(!isPro && {
- providerOptions: {
- provider: { sort: "price" },
- },
- }),
+ ...(isPro
+ ? {}
+ : {
+ providerOptions: {
+ provider: { sort: "price" as const },
+ },
+ }),Applies to:
- lib/ai/tools/web.ts :77
- lib/ai/tools/index.ts :54, 65
- app/api/chat/route.ts :115–121
📝 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.
| ...(!isPro && { | |
| providerOptions: { | |
| provider: { | |
| sort: "price", | |
| }, | |
| }, | |
| }), | |
| ...(isPro | |
| ? {} | |
| : { | |
| providerOptions: { | |
| provider: { sort: "price" as const }, | |
| }, | |
| }), |
🤖 Prompt for AI Agents
In app/api/chat/route.ts around lines 115 to 121 (and also update
lib/ai/tools/web.ts:77 and lib/ai/tools/index.ts:54,65), replace the
boolean-union spread pattern `...(cond && { ... })` with a ternary that yields
the object when true and an empty object when false (e.g. `...(cond ? { ... } :
{})`), and mark any literal provider configuration objects with `as const` to
preserve literal types; ensure each replaced spread returns a plain object so
TypeScript no longer sees `false | object`.
| onFinish: () => { | ||
| // For new chats, navigate to the proper route after first message | ||
| if (!isExistingChat) { | ||
| // Use window.history.replaceState to update URL without triggering navigation | ||
| window.history.replaceState(null, "", `/c/${chatId}`); | ||
| } | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Flip to “existing chat” (and optionally sync global active chat) after first message.
Without flipping, downstream queries won’t start.
Apply this diff:
if (!isExistingChat) {
// Use window.history.replaceState to update URL without triggering navigation
window.history.replaceState(null, "", `/c/${chatId}`);
+ setIsExistingChat(true);
+ // Optional: if GlobalState exposes activateChat, call it here
+ // activateChat?.(chatId);
}Outside this hunk, ensure activateChat is available from GlobalState if you want the sidebar to highlight the chat:
// near other GlobalState picks:
const { /* ... */, setTodos, /* add: */ activateChat } = useGlobalState();🤖 Prompt for AI Agents
In app/components/chat.tsx around lines 118 to 124, after replacing the URL for
a new chat you must flip the local state to mark the chat as an existing chat
(so downstream queries start) and optionally call activateChat from GlobalState
to sync the sidebar highlight; update the onFinish handler to set isExistingChat
(or call the setter) to true after window.history.replaceState and, where
GlobalState is used near other picks, add activateChat to the destructured
values (e.g., const { ..., setTodos, activateChat } = useGlobalState()) and call
activateChat(chatId) if present.
| /** | ||
| * Search messages by content and chat titles with full text search | ||
| */ | ||
| export const searchMessages = query({ | ||
| args: { | ||
| searchQuery: v.string(), | ||
| paginationOpts: paginationOptsValidator, | ||
| }, | ||
| returns: v.object({ | ||
| page: v.array( | ||
| v.object({ | ||
| id: v.string(), | ||
| chat_id: v.string(), | ||
| content: v.string(), | ||
| created_at: v.number(), | ||
| updated_at: v.optional(v.number()), | ||
| chat_title: v.optional(v.string()), | ||
| match_type: v.union( | ||
| v.literal("message"), | ||
| v.literal("title"), | ||
| v.literal("both"), | ||
| ), | ||
| }), | ||
| ), | ||
| isDone: v.boolean(), | ||
| continueCursor: v.union(v.string(), v.null()), | ||
| }), | ||
| handler: async (ctx, args) => { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Return schema ok; ensure cursor is nullable in all responses
You declared continueCursor as string | null—good. Make sure all return sites use null (not "") when done to satisfy usePaginatedQuery semantics.
return {
page: [],
isDone: true,
- continueCursor: "",
+ continueCursor: null,
};Applies here and to other early-return/error paths in this file.
📝 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.
| /** | |
| * Search messages by content and chat titles with full text search | |
| */ | |
| export const searchMessages = query({ | |
| args: { | |
| searchQuery: v.string(), | |
| paginationOpts: paginationOptsValidator, | |
| }, | |
| returns: v.object({ | |
| page: v.array( | |
| v.object({ | |
| id: v.string(), | |
| chat_id: v.string(), | |
| content: v.string(), | |
| created_at: v.number(), | |
| updated_at: v.optional(v.number()), | |
| chat_title: v.optional(v.string()), | |
| match_type: v.union( | |
| v.literal("message"), | |
| v.literal("title"), | |
| v.literal("both"), | |
| ), | |
| }), | |
| ), | |
| isDone: v.boolean(), | |
| continueCursor: v.union(v.string(), v.null()), | |
| }), | |
| handler: async (ctx, args) => { | |
| return { | |
| page: [], | |
| isDone: true, | |
| continueCursor: null, | |
| }; |
🤖 Prompt for AI Agents
In convex/messages.ts around lines 405 to 432, the handler defines
continueCursor as string|null but some return paths may emit an empty string;
update every return in this handler (and scan the rest of this file for similar
early-return/error paths) to return continueCursor: null when there is no next
page instead of an empty string, and ensure any helper/utility used to build
responses follows the same null convention so usePaginatedQuery consumers
receive null for finished pagination.
| const messageResults = await ctx.db | ||
| .query("messages") | ||
| .withSearchIndex("search_content", (q) => | ||
| q.search("content", args.searchQuery).eq("user_id", user.subject), | ||
| ) | ||
| .collect(); | ||
|
|
||
| // Search chats by title | ||
| const chatResults = await ctx.db | ||
| .query("chats") | ||
| .withSearchIndex("search_title", (q) => | ||
| q.search("title", args.searchQuery).eq("user_id", user.subject), | ||
| ) | ||
| .collect(); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Bound full‑text result sets; avoid unbounded .collect()
collect() can fetch arbitrarily large sets per user/query. Cap each search to a sane upper bound to control cost/latency.
- const messageResults = await ctx.db
+ const messageResults = await ctx.db
.query("messages")
.withSearchIndex("search_content", (q) =>
q.search("content", args.searchQuery).eq("user_id", user.subject),
)
- .collect();
+ .take(200); // tune as needed
- const chatResults = await ctx.db
+ const chatResults = await ctx.db
.query("chats")
.withSearchIndex("search_title", (q) =>
q.search("title", args.searchQuery).eq("user_id", user.subject),
)
- .collect();
+ .take(200);📝 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.
| const messageResults = await ctx.db | |
| .query("messages") | |
| .withSearchIndex("search_content", (q) => | |
| q.search("content", args.searchQuery).eq("user_id", user.subject), | |
| ) | |
| .collect(); | |
| // Search chats by title | |
| const chatResults = await ctx.db | |
| .query("chats") | |
| .withSearchIndex("search_title", (q) => | |
| q.search("title", args.searchQuery).eq("user_id", user.subject), | |
| ) | |
| .collect(); | |
| const messageResults = await ctx.db | |
| .query("messages") | |
| .withSearchIndex("search_content", (q) => | |
| q.search("content", args.searchQuery).eq("user_id", user.subject), | |
| ) | |
| .take(200); // tune as needed | |
| // Search chats by title | |
| const chatResults = await ctx.db | |
| .query("chats") | |
| .withSearchIndex("search_title", (q) => | |
| q.search("title", args.searchQuery).eq("user_id", user.subject), | |
| ) | |
| .take(200); |
🤖 Prompt for AI Agents
In convex/messages.ts around lines 449 to 463, the two full‑text searches call
.collect() which can return unbounded result sets; replace .collect() with a
bounded fetch (e.g., chain .take(MAX_SEARCH_RESULTS) or use collect with an
explicit limit) to cap results (suggest using a shared constant like
MAX_SEARCH_RESULTS = 100), apply the cap to both messageResults and chatResults,
and consider returning a paginated cursor or indication that more results exist
if the cap is reached.
| console.error("Failed to search messages:", error); | ||
| return { | ||
| page: [], | ||
| isDone: true, | ||
| continueCursor: "", | ||
| }; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use null cursor on errors for usePaginatedQuery compatibility
Empty string is a valid string; return null to signal exhaustion.
return {
page: [],
isDone: true,
- continueCursor: "",
+ continueCursor: null,
};📝 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.
| console.error("Failed to search messages:", error); | |
| return { | |
| page: [], | |
| isDone: true, | |
| continueCursor: "", | |
| }; | |
| } | |
| console.error("Failed to search messages:", error); | |
| return { | |
| page: [], | |
| isDone: true, | |
| continueCursor: null, | |
| }; | |
| } |
🤖 Prompt for AI Agents
In convex/messages.ts around lines 569 to 575, the error branch returns an empty
string for continueCursor which is a valid value and prevents usePaginatedQuery
from recognizing exhaustion; change the returned continueCursor to null in that
error return and update any related types/signatures (and callers) to accept
string | null so the null value correctly signals no further pages.
Summary by CodeRabbit
New Features
Improvements
UI/UX
Behavior Changes