-
Notifications
You must be signed in to change notification settings - Fork 51
Daily branch 2025 09 05 #21
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Caution Review failedThe pull request is closed. WalkthroughAdds stateful chat lifecycle and not-found handling, closes sidebars during chat creation/navigation, introduces a Convex-aware client error boundary, adds user customization backend + UI, refactors system prompts for personalization, extends model/provider selection, adds UI primitives (Badge/Label/Select), and tweaks layout sizing to h-full. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant U as User
participant CH as ChatHeader
participant GS as GlobalState
participant R as Router
participant C as ChatPage
U->>CH: Tap mobile "New Chat"
CH->>GS: closeSidebar()
CH->>GS: setChatSidebarOpen(false) [if mobile]
CH->>GS: initializeNewChat()
CH->>R: push("/")
Note right of C #DDDDDD: Chat observes global state
C->>GS: read currentChatId
GS-->>C: currentChatId = null (new)
C->>C: generate chatId, clear title, isExistingChat=false
Note over C: on first message...
C->>R: push("/c/{chatId}")
C->>C: set isExistingChat = true
sequenceDiagram
autonumber
participant U as User
participant MSD as MessageSearchDialog
participant GS as GlobalState
participant R as Router
U->>MSD: Select chat from search
MSD->>GS: closeSidebar()
alt isMobile
MSD->>GS: setChatSidebarOpen(false)
end
MSD->>R: push("/c/{chatId}")
MSD->>MSD: onClose()
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
✨ Finishing Touches
🧪 Generate unit tests
Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (4)
app/components/ChatHeader.tsx (2)
19-19: Explicitly type theisExistingChatprop.While the default value is provided, consider using TypeScript's boolean type explicitly in the interface for clarity.
- isExistingChat?: boolean; + isExistingChat?: boolean;
47-48: Simplify theisInChatassignment.The intermediate variable
isInChatis directly assigned fromisExistingChatwithout transformation. Consider usingisExistingChatdirectly or add a comment explaining why this alias is needed.- // Check if we're currently in a chat (use isExistingChat prop for accurate state) - const isInChat = isExistingChat; + // Using isExistingChat directly to determine if we're in a chat + const isInChat = isExistingChat;app/components/chat.tsx (2)
51-69: Reset todos during new-chat initialization to avoid state leakage.When starting a new chat (
currentChatId === null), existing todos can linger untilchatDataloads. Clear them eagerly here to keep the UI consistent.if (currentChatId === null) { setChatId(uuidv4()); setIsExistingChat(false); setChatTitle(null); + setTodos([]); // Messages will be cleared below after useChat is ready return; } -}, [routeChatId, currentChatId, setChatTitle]); +}, [routeChatId, currentChatId, setChatTitle, setTodos]);
154-160: Unify reset logic in one place (optional).You already have a “Unified reset” effect above. Consider removing this separate clearing effect and doing both title/todos/messages resets together to reduce ordering-related surprises.
-// Clear messages when starting a new chat (after useChat hook is ready) -useEffect(() => { - if (!routeChatId && currentChatId === null) { - setMessages([]); - } -}, [routeChatId, currentChatId, setMessages]); +// Optional: handled in the unified reset effect
📜 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/ChatHeader.tsx(5 hunks)app/components/MessageSearchDialog.tsx(2 hunks)app/components/SidebarHeader.tsx(1 hunks)app/components/chat.tsx(4 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/SidebarHeader.tsxapp/components/MessageSearchDialog.tsxapp/components/ChatHeader.tsxapp/components/chat.tsx
🧬 Code graph analysis (3)
app/components/MessageSearchDialog.tsx (1)
app/contexts/GlobalState.tsx (1)
useGlobalState(344-350)
app/components/ChatHeader.tsx (2)
app/contexts/GlobalState.tsx (1)
useGlobalState(344-350)hooks/use-mobile.ts (1)
useIsMobile(5-21)
app/components/chat.tsx (1)
app/contexts/GlobalState.tsx (1)
useGlobalState(344-350)
🔇 Additional comments (8)
app/components/SidebarHeader.tsx (1)
93-94: LGTM! Navigation simplified.The updated flow correctly uses global state for chat initialization rather than forcing page refreshes.
app/components/MessageSearchDialog.tsx (2)
175-185: Consider the UX impact of closing the sidebar before navigation.The new flow closes the sidebar before navigation, which may cause a visual jump. Consider whether this order is intentional for UX purposes or if navigation should happen first for smoother transitions.
Do you want to test the visual transition to ensure it provides a smooth user experience?
56-56: No action needed:closeSidebaris correctly exported by GlobalState
ThecloseSidebarfunction is declared inGlobalStateand returned in the context value ofuseGlobalState(), so destructuring it inMessageSearchDialog.tsxis valid.app/components/ChatHeader.tsx (2)
224-226: Review the conditional logic for chat title display.The condition
isExistingChat && chatData === undefinedwill show an empty string whenisExistingChatis true butchatDatais still loading. Verify this is the intended behavior rather than showing a loading state.Is showing an empty string while loading the intended behavior, or should there be a loading indicator?
32-39: All new GlobalState functions are correctly included in the provider value and exposed via useGlobalState.app/components/chat.tsx (3)
47-50: LGTM: explicit isExistingChat toggle enables clean “new → existing” transition.Using
isExistingChatto gate fetches viashouldFetchMessagesavoids invalid-ID queries during the first message.
143-145: Ensure global state reflects the active chat after first completion.After flipping to existing, consider also updating global state (e.g.,
setCurrentChatId) so other UI (sidebars, header actions) can rely on a single source of truth. If another component already does this, ignore.
39-41: Drop this check—currentChatIdis set byuseChatHandlers. The Chat component imports and uses theuseChatHandlershook, which invokessetCurrentChatIdwhen a new chat is created.Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (2)
app/page.tsx (1)
26-26: Optional: add dynamic viewport fallbackConsider ensuring full-height on iOS with dynamic toolbars.
-<div className="h-full bg-background flex flex-col overflow-hidden"> +<div className="h-full min-h-[100dvh] bg-background flex flex-col overflow-hidden">app/layout.tsx (1)
99-100: Optional: account for safe areasWith
viewport-fit=cover, add safe-area padding where needed (header/footer) to avoid clipped content on notched devices.Example (globals.css):
:root { --sa-top: env(safe-area-inset-top); --sa-bottom: env(safe-area-inset-bottom); } .header { padding-top: max(var(--sa-top), 0px); } .footer { padding-bottom: max(var(--sa-bottom), 0px); }
📜 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/c/[id]/page.tsx(2 hunks)app/components/chat.tsx(5 hunks)app/layout.tsx(1 hunks)app/page.tsx(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- app/c/[id]/page.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- app/components/chat.tsx
🧰 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/page.tsxapp/layout.tsx
🔇 Additional comments (3)
app/page.tsx (1)
26-26: Switch to h-full looks goodMatches RootLayout changes and fixes 100vh issues on mobile.
app/layout.tsx (2)
95-95: html h-full is correctEnsures the h-full chain works for descendants.
104-104: body h-full is consistentKeeps page containers sizing predictably.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (3)
lib/ai/providers.ts (1)
6-8: Addition of "ask-model" looks good; confirm env var exposure is intentional.If this value is only used server-side, prefer a non-public env var to avoid accidental client exposure. Otherwise, no blockers.
Would you like me to replace
NEXT_PUBLIC_ASK_MODELwith a server-only env var and centralize model defaults?app/api/chat/route.ts (2)
109-116: Simplify and type the model selection.Avoid mutation and narrow the type for safety.
Apply:
- let selectedModel = ""; - if (mode === "ask") { - selectedModel = "ask-model"; - } else { - selectedModel = "agent-model"; - } - selectedModel = hasMediaFiles ? "vision-model" : selectedModel; + type ModelKey = "ask-model" | "agent-model" | "vision-model"; + const selectedModel: ModelKey = + hasMediaFiles ? "vision-model" : mode === "ask" ? "ask-model" : "agent-model";Confirm that overriding ask+media to vision is the intended behavior for all ask flows.
121-131: Validate OpenRouter sort values and include optional max_price
- “price” and “latency” are both valid
sortoptions (the full set is"price" | "throughput" | "latency").- To prevent unexpected spend spikes, add a
max_priceobject underopenrouter.provider, for example:providerOptions: { openrouter: { provider: { sort: isPro ? "latency" : "price", max_price: { prompt: 1, // max $/M prompt tokens completion: 2 // max $/M completion tokens } } } }
📜 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 (2)
app/api/chat/route.ts(1 hunks)lib/ai/providers.ts(1 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:
lib/ai/providers.tsapp/api/chat/route.ts
🧬 Code graph analysis (1)
app/api/chat/route.ts (3)
lib/chat/chat-processor.ts (1)
hasMediaFiles(17-35)lib/ai/providers.ts (1)
myProvider(4-19)lib/system-prompt.ts (1)
systemPrompt(11-114)
- Fix ConvexError in getMessagesByChatId to return empty results instead of throwing when chat doesn't exist - Hide chat title in header when isChatNotFound is true - Update ConvexErrorBoundary to avoid duplicate CHAT_NOT_FOUND handling - Provide better UX for accessing non-existent chat URLs Changes: - convex/messages.ts: Handle CHAT_NOT_FOUND gracefully in getMessagesByChatId query - app/components/ChatHeader.tsx: Add isChatNotFound prop to conditionally hide title - app/components/chat.tsx: Pass isChatNotFound state to ChatHeader - app/components/ConvexErrorBoundary.tsx: Remove redundant CHAT_NOT_FOUND handling Follows Convex error handling best practices for better user experience.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
convex/messages.ts (3)
102-106: Preserve ConvexError details instead of wrapping with a generic ErrorWrapping loses error codes used by the UI. Re-throw ConvexError as-is and only wrap unknown errors.
- } catch (error) { - console.error("Failed to save message:", error); - throw new Error("Failed to save message"); - } + } catch (error) { + console.error("Failed to save message:", error); + if (error instanceof ConvexError) throw error; + throw new ConvexError({ code: "SAVE_MESSAGE_FAILED", message: "Failed to save message" }); + }
295-301: Avoid.filterin queries; add an index and usewithIndexConvex guideline: don’t use
.filter; use an index. Define a composite index(chat_id, role)and query it.- const lastAssistantMessage = await ctx.db - .query("messages") - .withIndex("by_chat_id", (q) => q.eq("chat_id", args.chatId)) - .filter((q) => q.eq(q.field("role"), "assistant")) - .order("desc") - .first(); + const lastAssistantMessage = await ctx.db + .query("messages") + .withIndex("by_chat_id_role", q => + q.eq("chat_id", args.chatId).eq("role", "assistant"), + ) + .order("desc") + .first();Add to schema (outside this file):
// convex/schema.ts defineTable("messages", { // ... }).index("by_chat_id_role", ["chat_id", "role"]);
375-389: Handle ConvexError codes explicitly; remove boolean checkReturn [] for CHAT_NOT_FOUND, rethrow CHAT_UNAUTHORIZED, and avoid relying on string matching.
- // Verify chat ownership - if chat doesn't exist, return empty array - const chatExists: boolean = await ctx.runQuery( - internal.messages.verifyChatOwnership, - { - chatId: args.chatId, - userId: args.userId, - }, - ); - - if (!chatExists) { - // Chat doesn't exist yet (new chat), return empty array - return []; - } + // Verify chat ownership - throws ConvexError if missing/unauthorized + await ctx.runQuery(internal.messages.verifyChatOwnership, { + chatId: args.chatId, + userId: args.userId, + }); @@ - if (error instanceof Error && error.message.includes("Unauthorized")) { - throw error; - } - return []; + if (error instanceof ConvexError) { + if (error.data?.code === "CHAT_NOT_FOUND") return []; + if (error.data?.code === "CHAT_UNAUTHORIZED") throw error; + } + return [];Also applies to: 406-412
app/components/chat.tsx (2)
71-83: Guard Convex queries with a typed Id; don’t pass local UUIDs.Passing a plain string risks type drift and invalid Convex IDs. Use the derived chatConvexId and the shouldFetchMessages gate.
- const paginatedMessages = usePaginatedQuery( - api.messages.getMessagesByChatId, - shouldFetchMessages ? { chatId } : "skip", - { initialNumItems: 28 }, - ); + const paginatedMessages = usePaginatedQuery( + api.messages.getMessagesByChatId, + shouldFetchMessages && chatConvexId ? { chatId: chatConvexId } : "skip", + { initialNumItems: 28 }, + ); @@ - const chatData = useQuery( - api.chats.getChatById, - shouldFetchMessages ? { id: chatId } : "skip", - ); + const chatData = useQuery( + api.chats.getChatById, + shouldFetchMessages && chatConvexId ? { id: chatConvexId } : "skip", + );
140-146: Tie message fetching to a persisted chat ID, not the UI flagWindow.history.replaceState won’t update the routeChatId prop, so gating Convex queries on isExistingChat will fire with the temporary UUID. Keep setIsExistingChat(true) for the UI, but gate usePaginatedQuery/useQuery on a valid persisted ID instead (the initial routeChatId or the ID returned by the backend). When you receive the persisted Convex chat ID, call setChatId(persistedId) and (optionally) update the URL to
/c/${persistedId}.
♻️ Duplicate comments (1)
app/components/chat.tsx (1)
283-292: Header receives stale/undefined id after replaceState; pass live chatId.Same issue noted earlier: routeChatId stays undefined after replaceState; header actions need the active id.
- <ChatHeader + <ChatHeader hasMessages={hasMessages} hasActiveChat={isExistingChat} chatTitle={chatTitle} - id={routeChatId} + id={shouldFetchMessages ? (chatId as Id<"chats">) : undefined} chatData={chatData} chatSidebarOpen={chatSidebarOpen} isExistingChat={isExistingChat} isChatNotFound={isChatNotFound} />If you prefer to avoid casts, you can pass id={chatData?.id}.
🧹 Nitpick comments (9)
convex/messages.ts (6)
73-87: Remove unreachable boolean branch after verifyChatOwnershipverifyChatOwnership returns true or throws; it never returns false. The
!chatExistscheck is dead code and can be removed.Apply:
- if (existingMessage) { - return null; - } else { - const chatExists: boolean = await ctx.runQuery( - internal.messages.verifyChatOwnership, - { - chatId: args.chatId, - userId: args.userId, - }, - ); - - if (!chatExists) { - throw new Error("Chat not found"); - } - } + if (existingMessage) { + return null; + } + await ctx.runQuery(internal.messages.verifyChatOwnership, { + chatId: args.chatId, + userId: args.userId, + });
145-149: Nit: prefer null for empty cursorYou return an empty string, but the return type allows null. Consider returning null for “no cursor” consistency.
246-256: Drop unreachable false-path and rely on ConvexErrorSame pattern: the boolean check after
verifyChatOwnershipis unreachable. Also keep ConvexError semantics for the client.- const chatExists: boolean = await ctx.runQuery( - internal.messages.verifyChatOwnership, - { - chatId: args.chatId, - userId: user.subject, - }, - ); - - if (!chatExists) { - throw new Error("Chat not found"); - } + await ctx.runQuery(internal.messages.verifyChatOwnership, { + chatId: args.chatId, + userId: user.subject, + });Additionally, consider throwing ConvexError for the unauthenticated case above for consistency:
- if (!user) { - throw new Error("Unauthorized: User not authenticated"); - } + if (!user) { + throw new ConvexError({ code: "UNAUTHORIZED", message: "User not authenticated" }); + }
311-321: Remove unreachable false-path after ownership verificationThis check can’t trigger; simplify.
- const chatExists: boolean = await ctx.runQuery( - internal.messages.verifyChatOwnership, - { - chatId: args.chatId, - userId: user.subject, - }, - ); - - if (!chatExists) { - throw new Error("Chat not found"); - } + await ctx.runQuery(internal.messages.verifyChatOwnership, { + chatId: args.chatId, + userId: user.subject, + });
490-507: Cut N+1 lookups: cache chat records per chat_id in this loopYou query each chat for every message. Cache once per chat_id to reduce DB calls.
- // Add message results + // Add message results + const chatCache = new Map<string, any>(); for (const msg of messageResults) { - 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 === undefined) { + chat = await ctx.db + .query("chats") + .withIndex("by_chat_id", (q) => q.eq("id", msg.chat_id)) + .first(); + chatCache.set(msg.chat_id, chat ?? null); + }
606-612: Simplify: remove boolean check after ownership verify; keep ConvexError behaviorSame simplification as above.
- } else { - // Verify chat ownership - const chatExists: boolean = await ctx.runQuery( - internal.messages.verifyChatOwnership, - { - chatId: message.chat_id, - userId: user.subject, - }, - ); - - if (!chatExists) { - throw new Error("Chat not found"); - } - } + } else { + // Verify chat ownership (throws on failure) + await ctx.runQuery(internal.messages.verifyChatOwnership, { + chatId: message.chat_id, + userId: user.subject, + }); + }Also consider throwing ConvexError for unauthenticated at Line 603 for consistency:
- if (!user) { - throw new Error("Unauthorized: User not authenticated"); - } + if (!user) { + throw new ConvexError({ code: "UNAUTHORIZED", message: "User not authenticated" }); + }Also applies to: 620-631
app/components/ConvexErrorBoundary.tsx (1)
52-74: Optional: add a reset mechanism to recover without unmountingConsider supporting a
resetKeys?: React.DependencyListprop and resetting state when any key changes (route/chat id). This avoids “stuck” error screens after transient issues.app/components/ChatHeader.tsx (1)
60-74: Memoize handleNewChat to avoid re-rendersMinor polish; keeps props stable when passed down.
- const handleNewChat = () => { + const handleNewChat = React.useCallback(() => { // Close computer sidebar when creating new chat closeSidebar(); // Close chat sidebar when creating new chat on mobile screens if (isMobile) { setChatSidebarOpen(false); } // Initialize new chat state using global state function initializeNewChat(); // Navigate to homepage - Chat component will respond to global state changes router.push("/"); - }; + }, [closeSidebar, isMobile, setChatSidebarOpen, initializeNewChat, router]);app/components/chat.tsx (1)
253-256: Simplify not-found detection.Minor cleanup; equivalent logic.
-const isChatNotFound = - isExistingChat && chatData === null && shouldFetchMessages; +const isChatNotFound = shouldFetchMessages && chatData === null;
📜 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/ChatHeader.tsx(5 hunks)app/components/ConvexErrorBoundary.tsx(1 hunks)app/components/chat.tsx(5 hunks)convex/messages.ts(4 hunks)
🧰 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/components/ConvexErrorBoundary.tsxconvex/messages.tsapp/components/ChatHeader.tsxapp/components/chat.tsx
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 (10)
📚 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 : Use internalQuery/internalMutation/internalAction for private functions callable only by other Convex functions; do not expose sensitive logic via public query/mutation/action
Applied to files:
convex/messages.ts
📚 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 : Always include argument and return validators for all Convex functions (query/internalQuery/mutation/internalMutation/action/internalAction)
Applied to files:
convex/messages.ts
📚 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 : Use query/mutation/action only for public API functions
Applied to files:
convex/messages.ts
📚 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 : Use ctx.runQuery from queries/mutations/actions to call a query
Applied to files:
convex/messages.ts
📚 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/schema.ts : Import schema definition functions (defineSchema, defineTable) from convex/server
Applied to files:
convex/messages.ts
📚 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
📚 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 : Use the generated api object for public functions and internal object for internal functions from convex/_generated/api.ts
Applied to files:
convex/messages.ts
📚 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 : Minimize calls from actions to queries/mutations to avoid race conditions from splitting transactions
Applied to files:
convex/messages.ts
📚 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 : Always use the new Convex function syntax (query/mutation/action objects with args/returns/handler) when defining Convex functions
Applied to files:
convex/messages.ts
📚 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 : Pass FunctionReference values (from api/internal) to ctx.runQuery/ctx.runMutation/ctx.runAction; do not pass function implementations
Applied to files:
convex/messages.ts
🧬 Code graph analysis (3)
convex/messages.ts (1)
convex/_generated/api.js (2)
internal(22-22)internal(22-22)
app/components/ChatHeader.tsx (2)
app/contexts/GlobalState.tsx (1)
useGlobalState(344-350)hooks/use-mobile.ts (1)
useIsMobile(5-21)
app/components/chat.tsx (4)
app/contexts/GlobalState.tsx (1)
useGlobalState(344-350)app/components/ConvexErrorBoundary.tsx (1)
ConvexErrorBoundary(17-74)app/components/Messages.tsx (1)
Messages(47-338)app/components/ChatInput.tsx (1)
ChatInput(40-296)
🔇 Additional comments (17)
convex/messages.ts (3)
31-34: Good: explicit ConvexError codes for not-found and unauthorizedClear, actionable error codes will simplify frontend handling.
Also applies to: 36-39
153-156: LGTM: rely on internal verification and exceptionsDirectly calling the internal query and letting it throw is the right approach.
198-206: LGTM: graceful handling of CHAT_NOT_FOUND and selective rethrowThis matches the new frontend error boundary behavior.
Also applies to: 208-210, 212-217
app/components/ConvexErrorBoundary.tsx (1)
31-44: Nice: surface ConvexError codes via toastsFocused handling of CHAT_UNAUTHORIZED keeps UX coherent.
app/components/ChatHeader.tsx (2)
225-231: Title logic reads well for existing/new/not-found statesThe conditions align with the new state flags.
236-247: Good mobile UX: inline New Chat control gated by auth and chat stateClear and minimal.
app/components/chat.tsx (11)
27-27: Good addition: error boundary wrapping.Import looks correct and is used appropriately below.
52-70: Reset effect: OK, but ensure no accidental fetches after new-chat init.With the above shouldFetchMessages change, this effect won’t accidentally turn on server fetching for a local-UUID chat after onFinish.
If you expect routeChatId to change at runtime, confirm that the parent page actually updates the prop (replaceState alone won’t). Otherwise, this effect is fine.
155-161: New-chat message clear looks good.Clears local state only when starting a brand-new chat; scoped and safe.
178-194: Server-to-UI sync is fine, but guard by persisted state only.With the fetch guard change, this will only overwrite messages for persisted chats, which is intended.
236-247: Pass typed id into handlers once persisted.Follow-up after adopting Id typing: ensure downstream handlers that persist messages/files accept Id<"chats"> where appropriate.
I can scan handler signatures and propose precise types if you’d like.
258-276: Layout/Boundary changes LGTM.Switch to h-full and boundary wrapping improves resilience without affecting flow.
301-307: User-facing copy LGTM.Clear and concise not-found message.
312-325: Messages props look consistent with pagination API.No issues; loadMore and status are correctly forwarded.
361-371: ChatInput render conditions are correct.Hidden for not-found and shown for existing or with messages.
394-401: Mobile Computer Sidebar overlay: OK.Z-index and containment look sane; no scroll trap issues obvious.
404-418: Mobile Chat Sidebar overlay: OK.Backdrop click-to-close and stopPropagation are correctly wired.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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)
app/components/chat.tsx (1)
139-147: Bug: replaceState uses client chatId (UUID) → route 404 and triggers Not Found UIAfter first completion, URL changes to /c/{uuid} while Convex expects Id<'chats'>; downstream queries 404 and set isChatNotFound. Update URL only once the server chat id is known.
- if (!isExistingChat) { - // Use window.history.replaceState to update URL without triggering navigation - window.history.replaceState(null, "", `/c/${chatId}`); - // Flip the state so it becomes an existing chat - setIsExistingChat(true); - } + if (!isExistingChat) { + // Flip the state; URL will be updated when chatData?.id is available + setIsExistingChat(true); + }Add this effect (outside the range) to update the URL when Convex id is known:
useEffect(() => { if (isExistingChat && chatData?.id) { const newPath = `/c/${chatData.id}`; if (window.location.pathname !== newPath) { window.history.replaceState(null, "", newPath); } } }, [isExistingChat, chatData?.id]);
♻️ Duplicate comments (3)
app/components/chat.tsx (3)
27-27: Import Convex Id helper to type chat document IDsBring in Id<"chats"> for typed Convex IDs per repo guidelines.
import { ConvexErrorBoundary } from "./ConvexErrorBoundary"; +import type { Id } from "@/convex/_generated/dataModel";
44-51: Type chatId as Id<'chats'> | string, gate fetches by route id, and derive chatConvexIdPrevents passing a client UUID to Convex and aligns with repo typing rules.
- const [chatId, setChatId] = useState<string>(() => { - return routeChatId || uuidv4(); - }); - - // Track whether this is an existing chat (prop-driven initially, flips after first completion) - const [isExistingChat, setIsExistingChat] = useState<boolean>(!!routeChatId); - const shouldFetchMessages = isExistingChat; + const [chatId, setChatId] = useState<Id<"chats"> | string>(() => { + return routeChatId || uuidv4(); + }); + + // UI concern only; data loading must be gated by the route id + const [isExistingChat, setIsExistingChat] = useState<boolean>(!!routeChatId); + const isRouteExistingChat = !!routeChatId; + const shouldFetchMessages = isRouteExistingChat; + // Convex-safe id derived only when route provides a persisted id + const chatConvexId = isRouteExistingChat ? (chatId as Id<"chats">) : undefined;
283-292: Header receives stale/undefined id after replaceState; pass live Convex idUse the derived chatConvexId so header actions (share/rename/delete) work reliably.
<ChatHeader hasMessages={hasMessages} hasActiveChat={isExistingChat} chatTitle={chatTitle} - id={routeChatId} + id={chatConvexId} chatData={chatData} chatSidebarOpen={chatSidebarOpen} isExistingChat={isExistingChat} isChatNotFound={isChatNotFound} />Check ChatHeader prop type expects an Id<'chats'> or optional:
#!/bin/bash rg -nC3 -P 'ChatHeaderProps|export default function ChatHeader' app/components/ChatHeader.tsx
🧹 Nitpick comments (5)
app/components/chat.tsx (5)
52-70: Cast routeChatId to Id<'chats'> when syncing stateKeeps state typed; no behavior change.
- if (routeChatId) { - setChatId(routeChatId); + if (routeChatId) { + setChatId(routeChatId as Id<"chats">); setIsExistingChat(true); return; }
155-161: Double-clear messages on new chat start; consider consolidatingThis effect may fight with other message initializations. Gate additionally on messages.length > 0 or fold into the reset effect.
- useEffect(() => { - if (!routeChatId && currentChatId === null) { - setMessages([]); - } - }, [routeChatId, currentChatId, setMessages]); + useEffect(() => { + if (!routeChatId && currentChatId === null && messages.length > 0) { + setMessages([]); + } + }, [routeChatId, currentChatId, messages.length, setMessages]);
253-256: Simplify not-found checkSince shouldFetchMessages mirrors route existence, this reduces redundancy.
- const isChatNotFound = - isExistingChat && chatData === null && shouldFetchMessages; + const isChatNotFound = shouldFetchMessages && chatData === null;
268-275: Controlled SidebarProvider doesn’t need defaultOpenDefault is ignored when open/onOpenChange are provided.
- <SidebarProvider - open={chatSidebarOpen} - onOpenChange={setChatSidebarOpen} - defaultOpen={true} - > + <SidebarProvider open={chatSidebarOpen} onOpenChange={setChatSidebarOpen}>
312-325: Hard-coded isSwitchingChats=falseConsider reflecting actual switching to suppress pagination scroll loads during route changes.
- isSwitchingChats={false} + isSwitchingChats={!routeChatId && !isExistingChat}
📜 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/chat.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/chat.tsx
🧬 Code graph analysis (1)
app/components/chat.tsx (4)
app/contexts/GlobalState.tsx (1)
useGlobalState(344-350)app/components/ConvexErrorBoundary.tsx (1)
ConvexErrorBoundary(17-74)app/components/Messages.tsx (1)
Messages(47-338)app/components/ChatInput.tsx (1)
ChatInput(40-296)
🔇 Additional comments (2)
app/components/chat.tsx (2)
258-276: LGTM: Error boundary and overlay structureWrapping with ConvexErrorBoundary and scoping the drag overlay/mobile overlays look good.
Also applies to: 386-391, 395-401, 403-418
71-76: Ignore guard on nonexistentchatConvexId
The variablechatConvexIdisn’t declared or derived anywhere in the codebase, so this change can’t be applied.Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🧹 Nitpick comments (20)
convex/schema.ts (2)
71-79: Consider consistent timestamp naming across tables.Other tables use
update_time(number). This one addsupdated_at. For consistency and query ergonomics, consider renaming toupdate_time(if not widely consumed yet).Apply if feasible:
- updated_at: v.number(), + update_time: v.number(),
79-79: Enforce single customization doc per user.Indexing by
user_idis good. Verify write paths ensure at most one doc per user and reads use.unique()onby_user_idto prevent accidental duplicates.Example usage:
// query const doc = await ctx.db.query("user_customization") .withIndex("by_user_id", q => q.eq("user_id", userId)) .unique(); // throws if >1 // mutation (upsert-ish) const existing = await ctx.db.query("user_customization") .withIndex("by_user_id", q => q.eq("user_id", userId)) .unique().catch(() => null); if (existing) { await ctx.db.patch(existing._id, patch); } else { await ctx.db.insert("user_customization", { user_id: userId, ...init }); }components/ui/label.tsx (1)
5-5: Drop VariantProps from Label; no variants are defined.
VariantProps<typeof labelVariants>adds unused props and can confuse consumers. Keepcvaif you want, but removeVariantPropsfrom the prop type.Apply:
-import { cva, type VariantProps } from "class-variance-authority"; +import { cva } from "class-variance-authority";- React.ComponentPropsWithoutRef<typeof LabelPrimitive.Root> & - VariantProps<typeof labelVariants> + React.ComponentPropsWithoutRef<typeof LabelPrimitive.Root>Also applies to: 15-17
components/ui/badge.tsx (1)
26-36: Use span + forwardRef for inline semantics and API parity.Badges are typically inline; forwardRef matches other UI wrappers.
Apply:
-export interface BadgeProps - extends React.HTMLAttributes<HTMLDivElement>, - VariantProps<typeof badgeVariants> {} - -function Badge({ className, variant, ...props }: BadgeProps) { - return ( - <div className={cn(badgeVariants({ variant }), className)} {...props} /> - ); -} +export interface BadgeProps + extends React.HTMLAttributes<HTMLSpanElement>, + VariantProps<typeof badgeVariants> {} + +const Badge = React.forwardRef<HTMLSpanElement, BadgeProps>( + ({ className, variant, ...props }, ref) => ( + <span + ref={ref} + className={cn(badgeVariants({ variant }), className)} + {...props} + /> + ), +); +Badge.displayName = "Badge";components/ui/select.tsx (1)
21-24: Tailwind class “shadow-xs” is non-standard; use “shadow-sm”.Prevents relying on a possibly undefined utility.
Apply:
- "flex h-9 w-full items-center justify-between whitespace-nowrap rounded-md border border-input bg-transparent px-3 py-2 text-sm shadow-xs ring-offset-background placeholder:text-muted-foreground focus:outline-none focus:ring-2 focus:ring-ring focus:ring-offset-2 disabled:cursor-not-allowed disabled:opacity-50 [&>span]:line-clamp-1", + "flex h-9 w-full items-center justify-between whitespace-nowrap rounded-md border border-input bg-transparent px-3 py-2 text-sm shadow-sm ring-offset-background placeholder:text-muted-foreground focus:outline-none focus:ring-2 focus:ring-ring focus:ring-offset-2 disabled:cursor-not-allowed disabled:opacity-50 [&>span]:line-clamp-1",lib/system-prompt.ts (3)
37-42: Normalize personality input before lookup.Trim/lowercase to handle “Nerd”, “ NERD ”, etc., and fall back safely.
-const getPersonalityInstructions = (personality?: string): string => { - if (!personality || !(personality in PERSONALITY_INSTRUCTIONS)) { - return ""; - } - return PERSONALITY_INSTRUCTIONS[personality as PersonalityType]; -}; +const getPersonalityInstructions = (personality?: string): string => { + if (!personality) return ""; + const key = personality.trim().toLowerCase() as PersonalityType; + return PERSONALITY_INSTRUCTIONS[key] ?? ""; +};
4-9: Remove redundantas conston a typed object.You already typed
DATE_FORMAT_OPTIONSasIntl.DateTimeFormatOptions;as constadds no value here.-const DATE_FORMAT_OPTIONS: Intl.DateTimeFormatOptions = { +const DATE_FORMAT_OPTIONS: Intl.DateTimeFormatOptions = { weekday: "long", year: "numeric", month: "long", day: "numeric", -} as const; +};
254-255: Join sections with a newline for cleaner prompt separation.Prevents accidental tag concatenation and improves readability.
-return sections.filter(Boolean).join(""); +return sections.filter(Boolean).join("\n");lib/db/actions.ts (2)
225-228: Log the error for observability.Silent
nullhinders debugging and SLOs.- } catch (error) { - // If no customization found or error, return null - return null; - } + } catch (error) { + console.error("Failed to fetch user customization:", error); + // If no customization found or error, return null + return null; + }
215-229: Consider centralizing the UserCustomization type.The interface lives in
lib/system-prompt.ts. Extract totypes/userCustomization.tsand reuse here to keep API contracts consistent.Want me to open a follow-up PR to introduce a shared type and update call sites?
app/api/chat/route.ts (2)
89-91: Fetch user customization in parallel to reduce latency.Start the fetch early and await right before use to shave RTT from the critical path.
- // Get user customization data - const userCustomization = await getUserCustomization({ userId }); + // Start fetching user customization early (await later before use) + const userCustomizationPromise = getUserCustomization({ userId });- const result = streamText({ + const userCustomization = await userCustomizationPromise; + const result = streamText({ model: myProvider.languageModel(selectedModel), system: systemPrompt(mode, executionMode, userCustomization),Also applies to: 121-124
113-120: Condense model selection logic.Equivalent and easier to scan.
- let selectedModel = ""; - if (mode === "ask") { - selectedModel = "ask-model"; - } else { - selectedModel = "agent-model"; - } - selectedModel = hasMediaFiles ? "vision-model" : selectedModel; + const selectedModel = + hasMediaFiles ? "vision-model" : mode === "ask" ? "ask-model" : "agent-model";app/components/SidebarUserNav.tsx (1)
234-237: Style the destructive action for clarity.Consider applying a destructive style to “Delete all chats” to reduce misclick risk (e.g., text-destructive + subtle red hover).
- <DropdownMenuItem onClick={() => setShowDeleteDialog(true)}> + <DropdownMenuItem + onClick={() => setShowDeleteDialog(true)} + className="text-destructive focus:text-destructive focus:bg-destructive/10" + >app/components/CustomizeHackerAIDialog.tsx (3)
31-49: Type arrays explicitly and narrow literals.Follow repo guidelines by adding explicit generics and “as const” for discriminated unions.
-const predefinedTraits = [ +const predefinedTraits: Array<string> = [ "Methodical", "Detail-oriented", "Thorough", "Risk-aware", "Tool-savvy", ]; -const personalityOptions = [ +type PersonalityValue = "default" | "cynic" | "robot" | "listener" | "nerd"; +const personalityOptions: Array<{ + value: PersonalityValue; + label: string; + description: string; +}> = [ { value: "default", label: "Default", description: "" }, { value: "cynic", label: "Cynic", description: "Critical and sarcastic" }, { value: "robot", label: "Robot", description: "Efficient and blunt" }, { value: "listener", label: "Listener", description: "Thoughtful and supportive", }, { value: "nerd", label: "Nerd", description: "Exploratory and enthusiastic" }, ]; +// Also narrow state: +// const [personality, setPersonality] = useState<PersonalityValue>("default");
80-86: Avoid duplicate traits when clicking chips repeatedly.Simple dedupe keeps UX clean.
- const handleAddTrait = (trait: string) => { - if (trait) { - const currentText = traitsText.trim(); - const newText = currentText ? `${currentText}, ${trait}` : trait; - setTraitsText(newText); - } - }; + const handleAddTrait = (trait: string) => { + if (!trait) return; + const list = traitsText + .split(",") + .map((t) => t.trim()) + .filter(Boolean); + if (!list.includes(trait)) { + setTraitsText([...list, trait].join(", ")); + } + };
88-105: Disable inputs while saving or show error feedback (optional).You already disable buttons; consider disabling fields or showing a toast on failure for clearer UX.
convex/userCustomization.ts (4)
25-29: Prefer .unique() when querying by unique index.Avoid race/ambiguity; it throws if multiple match and documents intent.
- const existing = await ctx.db - .query("user_customization") - .withIndex("by_user_id", (q) => q.eq("user_id", identity.subject)) - .first(); + const existing = await ctx.db + .query("user_customization") + .withIndex("by_user_id", (q) => q.eq("user_id", identity.subject)) + .unique();Apply similarly to the read queries below.
59-71: Consider v.int64() for timestamps.If you want stricter typing for epoch millis, switch updated_at to v.int64().
- updated_at: v.number(), + updated_at: v.int64(),
78-86: Use .unique() on reads by user_id.These reads should be unique by design; prefer .unique() to surface duplicates early.
- const customization = await ctx.db + const customization = await ctx.db .query("user_customization") .withIndex("by_user_id", (q) => q.eq("user_id", identity.subject)) - .first(); + .unique();And similarly for args.userId in the backend reader.
Also applies to: 125-133
106-121: Architectural follow-up: prefer internalQuery or server client auth over passing service keys as args.Longer-term, expose this as internalQuery and have a public function call it, or use the Convex server client with the service role key instead of sending it as an argument.
📜 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 ignored due to path filters (2)
convex/_generated/api.d.tsis excluded by!**/_generated/**pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (12)
app/api/chat/route.ts(3 hunks)app/components/CustomizeHackerAIDialog.tsx(1 hunks)app/components/SidebarUserNav.tsx(5 hunks)components/ui/badge.tsx(1 hunks)components/ui/label.tsx(1 hunks)components/ui/select.tsx(1 hunks)convex/messages.ts(4 hunks)convex/schema.ts(1 hunks)convex/userCustomization.ts(1 hunks)lib/db/actions.ts(1 hunks)lib/system-prompt.ts(3 hunks)package.json(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- convex/messages.ts
🧰 Additional context used
📓 Path-based instructions (4)
package.json
📄 CodeRabbit inference engine (.cursor/rules/convex_rules.mdc)
Add @types/node to devDependencies when using Node.js built-in modules
Files:
package.json
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/schema.tsconvex/userCustomization.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
**/*.{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/schema.tslib/db/actions.tsapp/api/chat/route.tsconvex/userCustomization.tsapp/components/SidebarUserNav.tsxapp/components/CustomizeHackerAIDialog.tsxcomponents/ui/label.tsxcomponents/ui/badge.tsxcomponents/ui/select.tsxlib/system-prompt.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 convex/schema.ts : Define the Convex schema in convex/schema.ts
Applied to files:
convex/schema.ts
📚 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/schema.ts : Import schema definition functions (defineSchema, defineTable) from convex/server
Applied to files:
convex/schema.ts
📚 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/schema.ts : Understand system fields: _id uses v.id(tableName) and _creationTime uses v.number(); they’re auto-added to all documents
Applied to files:
convex/schema.ts
🧬 Code graph analysis (9)
lib/db/actions.ts (1)
convex/userCustomization.ts (1)
getUserCustomization(59-101)
app/api/chat/route.ts (5)
convex/userCustomization.ts (1)
getUserCustomization(59-101)lib/db/actions.ts (1)
getUserCustomization(215-229)lib/chat/chat-processor.ts (1)
hasMediaFiles(17-35)lib/ai/providers.ts (1)
myProvider(4-19)lib/system-prompt.ts (1)
systemPrompt(214-255)
convex/userCustomization.ts (2)
lib/db/actions.ts (1)
getUserCustomization(215-229)convex/chats.ts (1)
validateServiceKey(5-9)
app/components/SidebarUserNav.tsx (1)
app/components/CustomizeHackerAIDialog.tsx (1)
CustomizeHackerAIDialog(51-258)
app/components/CustomizeHackerAIDialog.tsx (4)
components/ui/dialog.tsx (6)
Dialog(133-133)DialogContent(135-135)DialogHeader(138-138)DialogTitle(141-141)DialogDescription(136-136)DialogFooter(137-137)components/ui/label.tsx (1)
Label(26-26)components/ui/select.tsx (5)
Select(150-150)SelectTrigger(153-153)SelectValue(152-152)SelectContent(154-154)SelectItem(156-156)components/ui/button.tsx (1)
Button(59-59)
components/ui/label.tsx (1)
lib/utils.ts (1)
cn(16-18)
components/ui/badge.tsx (1)
lib/utils.ts (1)
cn(16-18)
components/ui/select.tsx (1)
lib/utils.ts (1)
cn(16-18)
lib/system-prompt.ts (2)
types/chat.ts (1)
ChatMode(4-4)types/agent.ts (1)
ExecutionMode(24-24)
🔇 Additional comments (11)
package.json (1)
25-27: Radix additions look good.New @radix-ui/react-label and @radix-ui/react-select deps align with the new UI components. @types/node is already present in devDependencies per guideline.
lib/system-prompt.ts (3)
182-189: Confirm tool name exposure matches available tools.The text references a specific
todo_writetool. Ensure this name matches an actually-registered tool and aligns with rule 3 (don’t expose tool names to users in responses).Do you want this section to avoid naming the tool explicitly (e.g., “task management tools”)?
190-208: Verify sandbox environment versions.Hard-coding versions (e.g., Python 3.12.10, Node 20.19.2, Go 1.24.2) can drift from the real sandbox and mislead actions. Prefer sourcing dynamically or updating via a single config.
Would you like a follow-up to read versions from the sandbox on first use and cache them?
213-256: Nice modularization of the system prompt.Clear sectioning, conditional assembly, and user bio integration make the prompt easier to evolve.
app/api/chat/route.ts (1)
125-136: Double-check provider options schema.Ensure
providerOptions.openrouter.provider.sortaccepts "price" | "latency" in youraiprovider wrapper; some clients expect top-levelopenrouter: { sort }.Would you like me to verify against the current provider SDK docs?
app/components/SidebarUserNav.tsx (4)
13-14: Dialog integration wiring looks good.New Settings import and dialog import are correct and scoped to the sidebar only. No issues.
Also applies to: 40-40
229-233: Nice UX addition: “Customize HackerAI”.Menu item correctly toggles the dialog via state; placement in the primary section makes sense.
239-239: Good separator placement.Separating account/actions from help improves scanability.
276-281: Modal mounting is correct.Rendering the dialog outside the dropdown avoids focus traps; onOpenChange is wired to state.
convex/userCustomization.ts (2)
106-148: No client-side usage detectedVerified that getUserCustomizationForBackend is only referenced in lib/db/actions.ts and not imported in any client-side files.
30-38: Do not insert documents with undefined fields.Convex rejects undefined on insert; build insert payload without undefineds, but keep undefined on patch to unset fields.
- const customizationData = { - user_id: identity.subject, - nickname: args.nickname?.trim() || undefined, - occupation: args.occupation?.trim() || undefined, - personality: args.personality?.trim() || undefined, - traits: args.traits?.trim() || undefined, - additional_info: args.additional_info?.trim() || undefined, - updated_at: Date.now(), - }; + const now = Date.now(); + const insertDoc = { + user_id: identity.subject, + ...(args.nickname?.trim() ? { nickname: args.nickname.trim() } : {}), + ...(args.occupation?.trim() ? { occupation: args.occupation.trim() } : {}), + ...(args.personality?.trim() ? { personality: args.personality.trim() } : {}), + ...(args.traits?.trim() ? { traits: args.traits.trim() } : {}), + ...(args.additional_info?.trim() ? { additional_info: args.additional_info.trim() } : {}), + updated_at: now, + }; + const patchDoc = { + nickname: args.nickname?.trim() || undefined, + occupation: args.occupation?.trim() || undefined, + personality: args.personality?.trim() || undefined, + traits: args.traits?.trim() || undefined, + additional_info: args.additional_info?.trim() || undefined, + updated_at: now, + }; if (existing) { - // Update existing customization - await ctx.db.patch(existing._id, customizationData); + await ctx.db.patch(existing._id, patchDoc); } else { - // Create new customization - await ctx.db.insert("user_customization", customizationData); + await ctx.db.insert("user_customization", insertDoc); }Also applies to: 40-46
⛔ Skipped due to learnings
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 : Use ctx.db.patch to shallow-merge updates into an existing document; it throws if the document does not existLearnt 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 : Use ctx.db.replace to fully replace an existing document; it throws if the document does not exist
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (9)
lib/rate-limit.ts (2)
30-31: Make sliding window duration configurable via env var.Hardcoding "5 h" bakes policy into code. Prefer an env to allow runtime tuning without redeploys.
const ratelimit = new Ratelimit({ redis: new Redis({ url: redisUrl, token: redisToken, }), - limiter: Ratelimit.slidingWindow(requestLimit, "5 h"), + // e.g. "5 h", "30 m", "10 s" + limiter: Ratelimit.slidingWindow( + requestLimit, + process.env.RATE_LIMIT_WINDOW || "5 h", + ), });
7-9: Type userId as Id<'users'> (per guidelines).Per repo guidelines, prefer precise Id<'table'> over string for document IDs.
-export const checkRateLimit = async ( - userId: string, - isPro: boolean, -): Promise<void> => { +import type { Id } from "@/convex/_generated/dataModel"; +export const checkRateLimit = async ( + userId: Id<"users">, + isPro: boolean, +): Promise<void> => {app/components/ChatInput.tsx (4)
86-93: Minor UX: close the dropdown when opening the upgrade dialog.Radix DropdownMenu typically closes on item click, but if it remains open in some paths, explicitly closing avoids focus traps.
If you manage dropdown open state, call setOpen(false) before setAgentUpgradeDialogOpen(true).
94-99: Expose upgrade errors to the user.handleUpgrade surfaces errors in useUpgrade, but the dialog doesn’t display upgradeError.
const handleUpgradeClick = async () => { await handleUpgrade(); // Don't close popover here - let it stay open to show loading state // The popover will naturally close when user navigates to Stripe };Add error rendering in the dialog (see separate diff below).
255-288: Avoid allowing Agent selection while plan check is pending.Currently,
hasProPlan || isCheckingProPlanenables Agent and sets mode to "agent", which can briefly select Agent and then be reverted by the effect or cause a 403 server error. Disable the option while checking to prevent flicker and errors.- {hasProPlan || isCheckingProPlan ? ( + {hasProPlan ? ( <DropdownMenuItem onClick={() => setMode("agent")} className="cursor-pointer" > <Infinity className="w-4 h-4 mr-2" /> <div className="flex flex-col"> <div className="flex items-center gap-2"> <span className="font-medium">Agent</span> </div> <span className="text-xs text-muted-foreground"> Hack, test, secure anything </span> </div> </DropdownMenuItem> - ) : ( + ) : isCheckingProPlan ? ( + <DropdownMenuItem disabled className="opacity-60 cursor-not-allowed"> + <Infinity className="w-4 h-4 mr-2" /> + <div className="flex flex-col"> + <span className="font-medium">Agent</span> + <span className="text-xs text-muted-foreground">Checking plan…</span> + </div> + </DropdownMenuItem> + ) : ( <DropdownMenuItem onClick={handleAgentModeClick} className="cursor-pointer" > <Infinity className="w-4 h-4 mr-2" /> <div className="flex flex-col flex-1"> <div className="flex items-center gap-2"> <span className="font-medium">Agent</span> <span className="flex items-center gap-1 rounded-full py-1 px-2 text-xs font-medium bg-[#F1F1FB] text-[#5D5BD0] hover:bg-[#E4E4F6] dark:bg-[#373669] dark:text-[#DCDBF6] dark:hover:bg-[#414071] border-0 transition-all duration-200"> PRO </span> </div> <span className="text-xs text-muted-foreground"> Hack, test, secure anything </span> </div> </DropdownMenuItem> )}Verification: Try rapidly toggling Agent while network throttling causes
isCheckingProPlanto remain true; the item should be disabled and not change mode.
354-376: Dialog: render upgrade errors and a brief note about redirect behavior.Surface
upgradeErrorand reduce confusion if Stripe is blocked or returns an error.<DialogContent className="sm:max-w-[500px]"> <DialogHeader> <DialogTitle>Upgrade to Pro</DialogTitle> <DialogDescription> Get access to Agent mode and unlock advanced hacking, testing, and security features with Pro. </DialogDescription> </DialogHeader> + {/* Error feedback */} + {upgradeError && ( + <div className="text-sm text-destructive mb-3" role="alert"> + {upgradeError} + </div> + )} <Button onClick={handleUpgradeClick} disabled={upgradeLoading} className="w-full" size="lg" > {upgradeLoading ? "Loading..." : "Upgrade to Pro"} </Button> + <p className="mt-2 text-xs text-muted-foreground"> + You’ll be redirected to checkout. Keep this tab open. + </p> </DialogContent>app/api/chat/route.ts (3)
97-99: Fetching user customization early is fine, but handle latency.If Convex is slow or unavailable,
getUserCustomizationreturns null (per actions.ts), so this is safe. Consider parallelizing withprocessChatMessagesif needed.
133-143: Provider sort policy looks good; consider parameterizing.Sorting by price for free and latency for Pro is sensible. If you anticipate A/B tests, consider moving this to a small helper that can read a feature flag.
50-55: Type strictness for IDs (optional).Per guidelines, prefer
Id<'chats'>forchatIdandId<'users'>foruserIdthroughout the route to catch mixups at compile time. This may require adjusting the client request typing.
📜 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/api/chat/route.ts(4 hunks)app/components/ChatInput.tsx(5 hunks)lib/rate-limit.ts(1 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:
lib/rate-limit.tsapp/api/chat/route.tsapp/components/ChatInput.tsx
🧬 Code graph analysis (2)
app/api/chat/route.ts (5)
lib/errors.ts (1)
ChatSDKError(36-74)lib/db/actions.ts (1)
getUserCustomization(215-229)lib/chat/chat-processor.ts (1)
hasMediaFiles(17-35)lib/ai/providers.ts (1)
myProvider(4-19)lib/system-prompt.ts (1)
systemPrompt(214-255)
app/components/ChatInput.tsx (3)
app/contexts/GlobalState.tsx (1)
useGlobalState(344-350)app/hooks/useFileUpload.ts (1)
useFileUpload(20-377)app/hooks/useUpgrade.ts (1)
useUpgrade(5-72)
🔇 Additional comments (3)
app/components/ChatInput.tsx (1)
79-85: Good guard: auto-revert agent mode for non‑Pro.This prevents stale "agent" state when entitlement changes. LGTM.
app/api/chat/route.ts (2)
60-67: Server-side enforcement for Agent mode is correct.403 on non‑Pro agent attempts aligns with the new UI gating and prevents bypasses. Good placement early in the handler.
131-131: Nice: systemPrompt now personalizes with userCustomization.Good integration point; no issues spotted.
Summary by CodeRabbit
New Features
Model / Backend Behavior
UI/UX
Pro / Access
Other