-
Notifications
You must be signed in to change notification settings - Fork 51
feat: file upload #17
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 comprehensive file-attachment support across client and server: upload UI (paste/drag/select), previews and image viewer, Convex storage schema and endpoints, server-side ingestion/tokenization, file-aware truncation/processing, and model selection switching to a vision model when media files are present. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Client
participant ChatInput
participant useFileUpload
participant GlobalState
participant ConvexFileAPI as Convex.fileStorage
participant Storage as Convex Storage
participant Server as /api/chat
participant Proc as chat-processor
participant Provider as myProvider
User->>ChatInput: Paste / Drop / Select files
ChatInput->>useFileUpload: handleFileUploadEvent / handlePasteEvent / handleDrop
useFileUpload->>ConvexFileAPI: generateUploadUrl()
ConvexFileAPI->>Storage: provide upload URL
useFileUpload->>Storage: PUT file
Storage-->>useFileUpload: storageId
useFileUpload->>ConvexFileAPI: saveFile (saveFileToDb action)
ConvexFileAPI-->>useFileUpload: { url, fileId, tokens }
useFileUpload->>GlobalState: addUploadedFile (url, fileId, tokens)
User->>ChatInput: Submit (text + uploadedFiles)
ChatInput->>Server: POST /api/chat { messages }
Server->>lib/utils/file-token-utils: truncateMessagesWithFileTokens(messages)
Server->>Proc: processChatMessages(truncatedMessages)
Proc-->>Server: { executionMode, processedMessages, hasMediaFiles }
alt hasMediaFiles
Server->>Provider: languageModel("vision-model")
else
Server->>Provider: languageModel("agent-model")
end
Server-->>Client: streamed response
Client->>GlobalState: clearUploadedFiles()
sequenceDiagram
autonumber
participant Client
participant Route as /api/chat
participant TokenSvc as file-token-utils
participant Proc as processChatMessages
participant DB as lib/db/actions
participant Provider as myProvider
Client->>Route: POST /api/chat { messages }
Route->>TokenSvc: truncateMessagesWithFileTokens(messages)
TokenSvc-->>Route: truncatedMessages
Route->>Proc: processChatMessages(truncatedMessages)
Proc->>DB: transformStorageIdsToUrls / getDocumentContentForFiles
Proc-->>Route: { processedMessages, hasMediaFiles }
alt hasMediaFiles == true
Route->>Provider: select "vision-model"
else
Route->>Provider: select "agent-model"
end
Provider-->>Route: model stream
Route-->>Client: streamed response
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (9)
✨ 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 (
|
- Move useEffect hook before early return to ensure hooks are called in consistent order - Fixes react-hooks/rules-of-hooks linting error - Maintains same functional behavior while following React Hook rules
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: 10
🧹 Nitpick comments (20)
.env.local.example (1)
36-36: Make the new env var actionable and consistent.Add "=" and a sample value like other model vars to avoid confusion.
-# NEXT_PUBLIC_VISION_MODEL +# NEXT_PUBLIC_VISION_MODEL= +# e.g. google/gemini-2.5-flashnext.config.ts (1)
4-11: Optionally scope image domains tighter.If you know the exact Convex deployment host(s), prefer explicit hostnames over a wildcard to reduce attack surface.
- { - protocol: "https", - hostname: "*.convex.cloud", - }, + // Replace with your actual Convex deployment hostname(s) + { + protocol: "https", + hostname: "YOUR-DEPLOYMENT.convex.cloud", + },lib/chat/chat-processor.ts (1)
7-30: Type-safe and simpler media check.Use array predicates and explicit typing to avoid any and improve readability.
-export function hasMediaFiles(messages: UIMessage[]): boolean { - for (const message of messages) { - if (message.role === "user" && message.parts) { - for (const part of message.parts) { - if (part.type === "file" && part.mediaType) { - // Check for image files - if (part.mediaType.startsWith("image/")) { - return true; - } - // Check for PDF files - if (part.mediaType === "application/pdf") { - return true; - } - } - } - } - } - return false; -} +type UIPart = + | { type: "text"; text?: string } + | { type: "file"; mediaType?: string | null }; + +export function hasMediaFiles(messages: ReadonlyArray<UIMessage>): boolean { + return messages.some( + (m) => + m.role === "user" && + Array.isArray(m.parts) && + (m.parts as Array<UIPart>).some( + (p) => + p.type === "file" && + typeof p.mediaType === "string" && + (p.mediaType.startsWith("image/") || + p.mediaType === "application/pdf"), + ), + ); +}convex/schema.ts (1)
34-35: Schema field looks right; consider defaulting to an empty array.Using v.id("_storage") is correct for storage references. To simplify downstream null checks, consider making storage_ids a non-optional array (migrating older docs to []).
- storage_ids: v.optional(v.array(v.id("_storage"))), // Track uploaded file storage IDs for cleanup + storage_ids: v.array(v.id("_storage")), // Track uploaded file storage IDs for cleanupIf you keep it optional for backward-compatibility, ensure all readers coalesce to [].
Also, long-term consider tightening parts from v.any() to a discriminated union for stronger type-safety.
app/components/ScrollToBottomButton.tsx (1)
23-33: Prevent unintended form submits; set button type.Add type="button" to avoid accidental form submission when rendered inside a form.
- <button + <button + type="button" onClick={onClick} className="bg-background border border-border rounded-full p-2 shadow-lg hover:shadow-xl transition-all duration-200 hover:scale-105 flex items-center justify-center" aria-label="Scroll to bottom" tabIndex={0} >If this is intended to float over content, consider restoring absolute/fixed positioning on the wrapper.
lib/utils/file-utils.ts (2)
34-38: Enforce MAX_FILES_LIMIT and validate before uploading.You defined helpers but don’t use them here. Early validation improves UX and avoids wasted bandwidth.
export async function uploadFilesToConvex( files: FileList, generateUploadUrl: () => Promise<string>, getFileUrls: (storageIds: Array<Id<"_storage">>) => Promise<(string | null)[]>, ): Promise<FileMessagePart[]> { + // Enforce limits and validate + if (files.length > MAX_FILES_LIMIT) { + throw new Error(`You can upload up to ${MAX_FILES_LIMIT} files at once.`); + } + + Array.from(files).forEach((file) => { + const { valid, error } = validateFile(file); + if (!valid) throw new Error(error || "Invalid file."); + }); @@ export async function uploadSingleFileToConvex( file: File, generateUploadUrl: () => Promise<string>, ): Promise<Id<"_storage">> { + const { valid, error } = validateFile(file); + if (!valid) throw new Error(error || "Invalid file.");Also applies to: 64-79, 84-104
71-79: Avoid empty string URLs; propagate nullability or handle gracefully.Falling back to "" forces consumers to branch on empty string. Prefer null and reflect that in the FileMessagePart type (url: string | null), or log and omit parts whose URL failed to resolve. If changing the type is too broad now, at least add a TODO and ensure renderers treat "" as “no URL”.
app/contexts/GlobalState.tsx (1)
194-201: Use the helper for consistency.InitializeNewChat manually clears uploaded files; use clearUploadedFiles() for consistency with initializeChat.
const initializeNewChat = useCallback(() => { setCurrentChatId(null); setShouldFetchMessages(false); setHasActiveChat(false); setTodos([]); setIsTodoPanelExpanded(false); - setUploadedFiles([]); + clearUploadedFiles(); }, []);app/hooks/useChatHandlers.ts (1)
73-79: Consider adding null checks for file properties.While the code filters for
uploadedandurl, it doesn't validate thatfileis not null before creating the FileUIObject.Add an additional safety check:
// Get file objects from uploaded files - URLs are already resolved in global state const fileObjects: FileUIObject[] = uploadedFiles .filter((file) => file.uploaded && file.url) + .filter((uploadedFile) => uploadedFile.file != null) .map((uploadedFile) => createFileUIObject(uploadedFile.file, uploadedFile.url!), );app/components/FileUploadPreview.tsx (3)
56-77: Async preview generation is well-handled, but consider memory management.The base64 conversion and caching logic is robust with proper error handling. However, storing many large images as base64 strings could consume significant memory.
Consider implementing a cache size limit or TTL to prevent memory issues with many large files:
// Use ref to store base64 previews to avoid regenerating them -const previewCache = useRef<Map<string, string>>(new Map()); +const previewCache = useRef<Map<string, { preview: string; timestamp: number }>>(new Map()); +const MAX_CACHE_SIZE = 50; // Limit cache to 50 items +const CACHE_TTL = 5 * 60 * 1000; // 5 minutes TTL const generateFileKey = useCallback((file: File): string => { return `${file.name}_${file.size}_${file.lastModified}`; }, []); +const cleanupCache = useCallback(() => { + const now = Date.now(); + // Remove expired entries + for (const [key, value] of previewCache.current.entries()) { + if (now - value.timestamp > CACHE_TTL) { + previewCache.current.delete(key); + } + } + // If still over limit, remove oldest entries + if (previewCache.current.size > MAX_CACHE_SIZE) { + const sortedEntries = Array.from(previewCache.current.entries()) + .sort((a, b) => a[1].timestamp - b[1].timestamp); + for (let i = 0; i < sortedEntries.length - MAX_CACHE_SIZE; i++) { + previewCache.current.delete(sortedEntries[i][0]); + } + } +}, []);
139-146: Fix TypeScript non-null assertion in onClick handler.The code uses a non-null assertion (
filePreview.preview!) even though the preview is already checked to be truthy in the conditional rendering.Remove the non-null assertion since it's redundant:
<button className="h-full w-full overflow-hidden relative" onClick={() => handleImageClick( - filePreview.preview!, + filePreview.preview, filePreview.file.name, ) } >
207-214: Consider memoizing the ImageViewer render condition.The ImageViewer is conditionally rendered based on
selectedImage && selectedImage.src, but the component also checks!!selectedImagein theisOpenprop, which is redundant.Simplify the conditional rendering:
-{selectedImage && selectedImage.src && ( +{selectedImage && ( <ImageViewer - isOpen={!!selectedImage} + isOpen={true} onClose={() => setSelectedImage(null)} - imageSrc={selectedImage.src} + imageSrc={selectedImage?.src || ""} imageAlt={selectedImage.alt} /> )}app/components/Messages.tsx (1)
161-167: Add null safety check for message.parts.While the code correctly separates file parts from non-file parts, it should handle the case where
message.partsmight be undefined or null.- // Separate file parts from other parts for user messages - const fileParts = message.parts.filter( - (part) => part.type === "file", - ); - const nonFileParts = message.parts.filter( - (part) => part.type !== "file", - ); + // Separate file parts from other parts for user messages + const fileParts = (message.parts || []).filter( + (part) => part.type === "file", + ); + const nonFileParts = (message.parts || []).filter( + (part) => part.type !== "file", + );convex/messages.ts (1)
457-485: Consider performance optimization for bulk file deletion.When deleting all messages in a chat, the current implementation deletes files sequentially. For chats with many messages and files, consider batching or parallel deletion for better performance.
// Delete all files and messages for (const message of messages) { // Clean up files associated with this message if (message.storage_ids && message.storage_ids.length > 0) { - for (const storageId of message.storage_ids) { - try { - await ctx.storage.delete(storageId); - } catch (error) { - console.error(`Failed to delete file ${storageId}:`, error); - // Continue with deletion even if file cleanup fails - } - } + // Delete files in parallel for better performance + await Promise.allSettled( + message.storage_ids.map(async (storageId) => { + try { + await ctx.storage.delete(storageId); + } catch (error) { + console.error(`Failed to delete file ${storageId}:`, error); + } + }) + ); } // Delete the message await ctx.db.delete(message._id); }app/components/ChatInput.tsx (2)
86-96: Optimize paste event listener attachment.The paste event listener is recreated on every render due to the
handlePasteEventdependency. Consider using a stable reference or memoizing the handler.+ const handlePaste = useCallback((e: ClipboardEvent) => { + // Only handle paste if the textarea is focused + if (textareaRef.current === document.activeElement) { + handlePasteEvent(e); + } + }, [handlePasteEvent]); + // Handle paste events for image uploads useEffect(() => { - const handlePaste = (e: ClipboardEvent) => { - // Only handle paste if the textarea is focused - if (textareaRef.current === document.activeElement) { - handlePasteEvent(e); - } - }; - document.addEventListener("paste", handlePaste); return () => { document.removeEventListener("paste", handlePaste); }; - }, [handlePasteEvent]); + }, [handlePaste]);Add the import:
-import { useEffect, useRef } from "react"; +import { useEffect, useRef, useCallback } from "react";
116-116: Consider adding file type restrictions.The file input accepts all file types (
accept="*"). Consider restricting to specific file types if there are limitations on what the system can process.- accept="*" + accept="image/*,.pdf,.txt,.doc,.docx"Or keep it as is if all file types are truly supported.
app/components/FilePartRenderer.tsx (4)
57-57: Strongly type ConvexFilePart props.This removes the need for downstream casts and improves narrowing.
-const ConvexFilePart = ({ part, partId }: { part: any; partId: string }) => { +const ConvexFilePart = ({ part, partId }: { part: FilePart; partId: string }) => {After typing, you can drop the cast on Line 62 and rely on the guard:
{ storageId: part.storageId! }.
102-105: Remove duplicate innerContainerClass branches.Both branches are identical; simplify to a single constant.
- const innerContainerClass = isMultipleImages - ? "bg-token-main-surface-secondary text-token-text-tertiary relative flex items-center justify-center overflow-hidden" - : "bg-token-main-surface-secondary text-token-text-tertiary relative flex items-center justify-center overflow-hidden"; + const innerContainerClass = + "bg-token-main-surface-secondary text-token-text-tertiary relative flex items-center justify-center overflow-hidden";
123-130: Avoid hard-coded Next/Image dimensions; prefer real metadata.Hard-coded 902x2048 can distort aspect ratios. Use stored width/height from upload metadata or switch to a wrapper with known dimensions.
Example using metadata if available:
- <Image - src={actualUrl} - alt={altText} - width={902} - height={2048} - className={imageClass} - style={{ maxWidth: "100%", height: "auto" }} - /> + <Image + src={actualUrl} + alt={altText} + width={part.width ?? 1024} + height={part.height ?? 1024} + className={imageClass} + style={{ maxWidth: "100%", height: "auto" }} + />If no metadata exists, consider capturing it at upload time and adding it to the part.
148-165: Minor: broaden fallback subtitle based on mediaType/extension.“Document” is generic; using the extension or mediaType improves UX.
- subtitle="Document" + subtitle={part.mediaType ?? (part.name?.split('.').pop() || "Document")}
📜 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 (21)
.env.local.example(1 hunks)app/api/chat/route.ts(2 hunks)app/components/ChatInput.tsx(5 hunks)app/components/FilePartRenderer.tsx(1 hunks)app/components/FileUploadPreview.tsx(1 hunks)app/components/ImageViewer.tsx(1 hunks)app/components/MessagePartHandler.tsx(1 hunks)app/components/Messages.tsx(3 hunks)app/components/ScrollToBottomButton.tsx(1 hunks)app/components/chat.tsx(4 hunks)app/contexts/GlobalState.tsx(7 hunks)app/hooks/useChatHandlers.ts(4 hunks)app/hooks/useFileUpload.ts(1 hunks)convex/chats.ts(1 hunks)convex/messages.ts(7 hunks)convex/schema.ts(1 hunks)lib/actions/index.ts(1 hunks)lib/ai/providers.ts(1 hunks)lib/chat/chat-processor.ts(3 hunks)lib/utils/file-utils.ts(1 hunks)next.config.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{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/MessagePartHandler.tsxlib/actions/index.tslib/ai/providers.tsnext.config.tsapp/components/ScrollToBottomButton.tsxlib/utils/file-utils.tsapp/hooks/useFileUpload.tsconvex/schema.tsapp/components/ImageViewer.tsxlib/chat/chat-processor.tsapp/api/chat/route.tsconvex/chats.tsapp/components/FilePartRenderer.tsxapp/components/ChatInput.tsxapp/contexts/GlobalState.tsxapp/components/FileUploadPreview.tsxapp/hooks/useChatHandlers.tsapp/components/chat.tsxconvex/messages.tsapp/components/Messages.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/schema.tsconvex/chats.tsconvex/messages.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 (4)
📚 Learning: 2025-08-29T13:33:09.901Z
Learnt from: CR
PR: hackerai-tech/hackerai#0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-08-29T13:33:09.901Z
Learning: Applies to convex/**/*.ts : Store and handle files as Blob objects; convert to/from Blob when using Convex storage
Applied to files:
lib/utils/file-utils.tsconvex/messages.ts
📚 Learning: 2025-08-29T13:33:09.901Z
Learnt from: CR
PR: hackerai-tech/hackerai#0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-08-29T13:33:09.901Z
Learning: Applies to convex/**/*.ts : Use ctx.storage.getUrl(fileId) to obtain signed URLs; it returns null if the file does not exist
Applied to files:
lib/utils/file-utils.ts
📚 Learning: 2025-08-29T13:33:09.901Z
Learnt from: CR
PR: hackerai-tech/hackerai#0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-08-29T13:33:09.901Z
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
📚 Learning: 2025-08-29T13:33:09.901Z
Learnt from: CR
PR: hackerai-tech/hackerai#0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-08-29T13:33:09.901Z
Learning: Applies to convex/**/*.ts : Queries do not support .delete(); collect results and call ctx.db.delete(_id) per document
Applied to files:
convex/messages.ts
🧬 Code graph analysis (11)
app/components/ScrollToBottomButton.tsx (1)
app/contexts/GlobalState.tsx (1)
useGlobalState(279-285)
lib/utils/file-utils.ts (1)
convex/messages.ts (2)
generateUploadUrl(307-319)getFileUrls(343-361)
app/hooks/useFileUpload.ts (3)
app/contexts/GlobalState.tsx (1)
useGlobalState(279-285)convex/messages.ts (3)
generateUploadUrl(307-319)deleteFile(366-381)getFileUrl(324-338)lib/utils/file-utils.ts (5)
uploadSingleFileToConvex(84-104)MAX_FILES_LIMIT(196-196)validateFile(150-159)UploadedFileState(21-28)createFileMessagePartFromUploadedFile(201-216)
app/api/chat/route.ts (2)
lib/chat/chat-processor.ts (2)
processChatMessages(68-111)hasMediaFiles(12-30)lib/ai/providers.ts (1)
myProvider(4-16)
app/components/FilePartRenderer.tsx (2)
convex/_generated/dataModel.d.ts (1)
Id(48-49)app/components/ImageViewer.tsx (1)
ImageViewer(11-136)
app/components/ChatInput.tsx (4)
app/contexts/GlobalState.tsx (1)
useGlobalState(279-285)app/hooks/useFileUpload.ts (1)
useFileUpload(14-254)app/components/FileUploadPreview.tsx (1)
FileUploadPreview(26-217)app/components/ScrollToBottomButton.tsx (1)
ScrollToBottomButton(10-34)
app/contexts/GlobalState.tsx (1)
lib/utils/file-utils.ts (1)
UploadedFileState(21-28)
app/components/FileUploadPreview.tsx (2)
lib/utils/file-utils.ts (4)
UploadedFileState(21-28)isImageFile(189-191)fileToBase64(177-184)formatFileSize(164-172)app/components/ImageViewer.tsx (1)
ImageViewer(11-136)
app/hooks/useChatHandlers.ts (2)
convex/messages.ts (1)
generateUploadUrl(307-319)lib/utils/file-utils.ts (2)
FileUIObject(14-19)createFileUIObject(109-116)
convex/messages.ts (3)
convex/_generated/dataModel.d.ts (1)
Id(48-49)convex/_generated/server.js (4)
mutation(49-49)mutation(49-49)query(29-29)query(29-29)convex/_generated/api.js (2)
internal(22-22)internal(22-22)
app/components/Messages.tsx (2)
app/components/FilePartRenderer.tsx (1)
FilePartRenderer(16-181)app/components/MessagePartHandler.tsx (1)
MessagePartHandler(16-70)
🔇 Additional comments (21)
lib/actions/index.ts (1)
27-27: Prompt wording change LGTM.The assistant phrasing reads better and stays aligned with the feature.
app/components/ScrollToBottomButton.tsx (1)
15-19: Confirm UX: hiding the button when the Todo panel is expanded.Previously position shifted; now the button is fully hidden when isTodoPanelExpanded. If the intent is to keep it visible above the panel, we should restore positioning rather than hide it.
lib/utils/file-utils.ts (1)
5-12: I’ll check existing imports and usages ofIdto determine the correct alias and confirm table name.app/contexts/GlobalState.tsx (1)
185-193: The scripts are queued for execution.convex/chats.ts (1)
262-280: LGTM! File cleanup logic is well-implemented with proper error handling.The implementation correctly handles file cleanup when deleting a chat. The use of try-catch around individual file deletions ensures that the chat deletion process continues even if some files fail to delete, preventing orphaned records.
app/api/chat/route.ts (2)
71-77: LGTM! Media-aware model selection is properly implemented.The code correctly destructures
hasMediaFilesfrom theprocessChatMessagesresult and uses it to conditionally select the appropriate model.
99-101: Remove model availability validation
Selected models (“vision-model”/“agent-model”) are statically configured with fallbacks for unset env vars, somyProvider.languageModel(selectedModel)will always return a valid model—this runtime check is redundant.lib/ai/providers.ts (2)
9-11: LGTM! Vision model configuration is properly added.The addition of the
vision-modelentry with appropriate fallback configuration is well-structured and follows the existing pattern.
12-14: Verify intentional default title-generator-model changeThe default model for
title-generator-modelwas updated from"qwen/qwen3-coder"to"google/gemini-2.5-flash"in lib/ai/providers.ts (line 12). This provider is used in lib/actions/index.ts (around line 50) for title generation. This change is unrelated to the file upload feature—please confirm it’s intentional for this PR.app/hooks/useChatHandlers.ts (1)
54-110: File handling logic is robust with proper error recovery.The implementation includes:
- Proper validation of uploaded files before submission
- Graceful fallback to text-only messages if file processing fails
- Clean state management after submission
app/components/FileUploadPreview.tsx (1)
36-42: Excellent use of caching for performance optimization.The implementation uses a ref-based cache with a stable file identity key to avoid regenerating base64 previews. This is a smart optimization that prevents unnecessary re-computation when the component re-renders.
app/components/Messages.tsx (2)
5-5: LGTM! File part rendering integration looks good.The import of
FilePartRendererfollows the established pattern and properly integrates with the message rendering flow.
198-210: File rendering implementation is well-structured.The file parts are rendered cleanly in a flex container with appropriate spacing and alignment for user messages. The use of a unique key pattern for file parts is correct.
app/components/chat.tsx (2)
295-297: Props are correctly passed to ChatInput.The integration of
hasMessages,isAtBottom, andonScrollToBottomprops enables proper scroll management and file upload UI behavior.
59-59: Convert File objects to Blob before persisting to Convex storage
I didn’t find any File-to-Blob conversion foruploadedFilesinapp/contexts/GlobalState.tsxor related upload utilities; please verify and implement this step before storing files with Convex.convex/messages.ts (2)
204-216: Good defensive programming with file cleanup.The error handling for file deletion is properly implemented - failures are logged but don't block the main operation, which is the correct approach for cleanup operations.
387-432: Comprehensive file cleanup implementation.The
deleteMessageWithFilesmutation properly handles authentication, ownership verification, and gracefully handles file deletion failures. The pattern of continuing even when individual file deletions fail is appropriate.app/hooks/useFileUpload.ts (1)
27-54: Remove Blob conversion suggestion. File objects inherit from Blob, and uploadSingleFileToConvex accepts the File directly as the request body—no additional conversion is needed.Likely an incorrect or invalid review comment.
app/components/ChatInput.tsx (1)
265-274: ScrollToBottomButton integration looks good.The button is properly positioned and conditionally rendered based on the presence of the callback prop.
app/components/FilePartRenderer.tsx (2)
65-81: Guard remote URLs and verify Next/Image allowlistEnsure
actualUrlis validated to onlyhttp(s)and update the error check accordingly. Confirm thatnext.config.{js,ts}includes animages.remotePatternsordomainsentry covering your file host domains.// Determine the actual URL to use const actualUrl = part.url || fileUrl; + const isSafeUrl = + typeof actualUrl === "string" && /^https?:\/\//i.test(actualUrl); if (shouldFetchUrl && fileUrl === undefined) { // Loading state... } - if (!actualUrl || (shouldFetchUrl && fileUrl === null)) { + if (!actualUrl || !isSafeUrl || (shouldFetchUrl && fileUrl === null)) { // File not found or error }
9-14: Use strict types for FilePartRendererPropsAlign with TSX guidelines: type
messageIdasId<'messages'>and avoidanyforpart.-interface FilePartRendererProps { - part: any; - partIndex: number; - messageId: string; - totalFileParts?: number; -} +interface FilePartRendererProps { + part: FilePart; + partIndex: number; + messageId: Id<"messages">; + totalFileParts?: number; +}Define or import a matching
FileParttype:type FilePart = { url?: string | null; storageId?: Id<'_storage'> | null; mediaType?: string | null; name?: string | null; filename?: string | null; };Verify all
<FilePartRenderer … />call sites pass the updated types.
…iles - Modified handlePasteEvent to call preventDefault() when files detected - Updated ChatInput paste handler to work with improved file detection - Ensures clean UX when pasting files without text clutter
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
♻️ Duplicate comments (2)
app/hooks/useFileUpload.ts (2)
128-145: Stabilize upload indices to avoid race conditions.Indexing based on a changing uploadedFiles length can mis-target updates when multiple files are added quickly. Capture a base index first.
- const startFileUploads = useCallback( - (files: File[]) => { - files.forEach((file, index) => { + const startFileUploads = useCallback( + (files: File[]) => { + const baseIndex = uploadedFiles.length; + files.forEach((file, index) => { // Add file as "uploading" state immediately const uploadState: UploadedFileState = { file, uploading: true, uploaded: false, }; addUploadedFile(uploadState); // Start upload in background - const uploadIndex = uploadedFiles.length + index; + const uploadIndex = baseIndex + index; uploadFileToConvex(file, uploadIndex); }); }, - [uploadedFiles.length, addUploadedFile], + [uploadedFiles.length, addUploadedFile, uploadFileToConvex], );
221-223: Replace any-cast with Id<"_storage">.Aligns with repo guidelines and prior feedback.
- await deleteFile({ storageId: uploadedFile.storageId as any }); + await deleteFile({ + storageId: uploadedFile.storageId as Id<"_storage">, + });
🧹 Nitpick comments (6)
app/components/DragDropOverlay.tsx (2)
16-23: Add basic accessibility semantics to the overlay container.Expose the region purpose to screen readers so the instruction text is discoverable.
- <div - className={`absolute inset-0 z-50 flex items-center justify-center transition-colors duration-200 ${ + <div + role="region" + aria-label="Drop files to upload" + className={`absolute inset-0 z-50 flex items-center justify-center transition-colors duration-200 ${ isDragOver ? "bg-accent/30 backdrop-blur-sm" : "bg-muted/20 backdrop-blur-sm" }`} >
31-35: Mark the Upload icon as decorative.Prevents redundant announcement by screen readers.
- <Upload + <Upload + aria-hidden="true" + focusable="false" className={`w-12 h-12 mb-4 transition-all duration-200 ${ isDragOver ? "text-foreground scale-110" : "text-muted-foreground" }`} />app/hooks/useFileUpload.ts (3)
283-292: Show overlay only when files are being dragged.Prevents overlay on text/HTML drags.
- if (e.dataTransfer?.items && e.dataTransfer.items.length > 0) { + if ( + e.dataTransfer?.items && + Array.from(e.dataTransfer.items).some((it) => it.kind === "file") + ) { setShowDragOverlay(true); }
294-304: Clamp drag counter to avoid negative values and stuck overlay.Nested dragleave events can underflow the counter.
- dragCounterRef.current--; - - if (dragCounterRef.current === 0) { + dragCounterRef.current = Math.max(0, dragCounterRef.current - 1); + if (dragCounterRef.current === 0) { setShowDragOverlay(false); setIsDragOver(false); }
88-125: Remove unused parameter or incorporate it in messages.
sourceis unused; either drop it or use it to tailor toasts.- const showProcessingFeedback = useCallback( - ( - result: FileProcessingResult, - source: FileSource, - hasRemainingSlots: boolean = true, - ) => { + const showProcessingFeedback = useCallback( + (result: FileProcessingResult, hasRemainingSlots: boolean = true) => { const messages: string[] = [];And update the callers accordingly.
app/components/chat.tsx (1)
216-235: Add a dragend listener for extra resiliency.Some browsers don’t reliably fire dragleave on the document; dragend helps ensure cleanup.
useEffect(() => { const handleDocumentDragEnter = (e: DragEvent) => handleDragEnter(e); const handleDocumentDragLeave = (e: DragEvent) => handleDragLeave(e); const handleDocumentDragOver = (e: DragEvent) => handleDragOver(e); const handleDocumentDrop = (e: DragEvent) => handleDrop(e); + const handleDocumentDragEnd = () => { + handleDragLeave(new DragEvent("dragleave")); + }; document.addEventListener("dragenter", handleDocumentDragEnter); document.addEventListener("dragleave", handleDocumentDragLeave); document.addEventListener("dragover", handleDocumentDragOver); document.addEventListener("drop", handleDocumentDrop); + document.addEventListener("dragend", handleDocumentDragEnd); return () => { document.removeEventListener("dragenter", handleDocumentDragEnter); document.removeEventListener("dragleave", handleDocumentDragLeave); document.removeEventListener("dragover", handleDocumentDragOver); document.removeEventListener("drop", handleDocumentDrop); + document.removeEventListener("dragend", handleDocumentDragEnd); }; }, [handleDragEnter, handleDragLeave, handleDragOver, handleDrop]);
📜 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/ChatInput.tsx(5 hunks)app/components/DragDropOverlay.tsx(1 hunks)app/components/chat.tsx(9 hunks)app/hooks/useFileUpload.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- app/components/ChatInput.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/components/DragDropOverlay.tsxapp/hooks/useFileUpload.tsapp/components/chat.tsx
🧠 Learnings (3)
📚 Learning: 2025-08-29T13:33:09.901Z
Learnt from: CR
PR: hackerai-tech/hackerai#0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-08-29T13:33:09.901Z
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/hooks/useFileUpload.ts
📚 Learning: 2025-08-29T13:33:09.901Z
Learnt from: CR
PR: hackerai-tech/hackerai#0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-08-29T13:33:09.901Z
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/hooks/useFileUpload.ts
📚 Learning: 2025-08-29T13:33:09.901Z
Learnt from: CR
PR: hackerai-tech/hackerai#0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-08-29T13:33:09.901Z
Learning: Applies to convex/**/*.ts : Use ctx.storage.getUrl(fileId) to obtain signed URLs; it returns null if the file does not exist
Applied to files:
app/hooks/useFileUpload.ts
🧬 Code graph analysis (2)
app/hooks/useFileUpload.ts (3)
app/contexts/GlobalState.tsx (1)
useGlobalState(279-285)convex/messages.ts (3)
generateUploadUrl(307-319)deleteFile(366-381)getFileUrl(324-338)lib/utils/file-utils.ts (5)
MAX_FILES_LIMIT(196-196)validateFile(150-159)UploadedFileState(21-28)uploadSingleFileToConvex(84-104)createFileMessagePartFromUploadedFile(201-216)
app/components/chat.tsx (2)
app/hooks/useFileUpload.ts (1)
useFileUpload(23-352)app/components/DragDropOverlay.tsx (1)
DragDropOverlay(10-43)
🔇 Additional comments (3)
app/hooks/useFileUpload.ts (1)
37-40: Keep useMutation for getFileUrls and rename the variable to match the API.
getFileUrls is exported as a mutation returning an array (see convex/messages.ts), so switching to useConvex() is incorrect. Simply changeconst getFileUrl = useMutation(api.messages.getFileUrls);to
const getFileUrls = useMutation(api.messages.getFileUrls);Likely an incorrect or invalid review comment.
app/components/chat.tsx (2)
198-207: LGTM: clean integration of upload/drag state.Hook wiring looks correct and keeps Chat free of upload details.
366-370: LGTM: overlay placement and props are correct.Overlay is constrained to the main content container and won’t cover sidebars.
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
♻️ Duplicate comments (3)
app/hooks/useFileUpload.ts (3)
174-203: Fix type casting and use single file URL query.The code uses array-based query
getFileUrlsfor a single file and casts the storage ID. Use the single file query instead.Apply this fix to use the correct query and remove casting:
- // Fetch the URL immediately after upload using the query - const urls = await convex.query(api.messages.getFileUrls, { - storageIds: [storageId as Id<"_storage">], - }); - const url = urls[0]; + // Fetch the URL immediately after upload using the single file query + const url = await convex.query(api.messages.getFileUrl, { + storageId: storageId as Id<"_storage">, + });
219-235: Fix storage ID type casting.The code casts storage ID to
anywhich violates type safety guidelines.Apply this fix to remove the unsafe cast:
- await deleteFile({ - storageId: uploadedFile.storageId as Id<"_storage">, - }); + await deleteFile({ + storageId: uploadedFile.storageId as Id<"_storage">, + });Note: The real fix requires updating the
UploadedFileState.storageIdtype inlib/utils/file-utils.tsto use proper Id typing.
174-174: Add useCallback to prevent stale closures.The
uploadFileToConvexfunction should be wrapped inuseCallbackto prevent stale closures and ensure dependencies are properly tracked.- const uploadFileToConvex = async (file: File, uploadIndex: number) => { + const uploadFileToConvex = useCallback(async (file: File, uploadIndex: number) => { try { const storageId = await uploadSingleFileToConvex(file, generateUploadUrl); // Fetch the URL immediately after upload using the query const url = await convex.query(api.messages.getFileUrl, { storageId: storageId as Id<"_storage">, }); // Update the upload state to completed with storage ID and URL updateUploadedFile(uploadIndex, { uploading: false, uploaded: true, storageId, url: url || undefined, }); } catch (error) { console.error("Failed to upload file:", error); // Update the upload state to error updateUploadedFile(uploadIndex, { uploading: false, uploaded: false, error: error instanceof Error ? error.message : "Upload failed", }); toast.error( `Failed to upload ${file.name}: ${error instanceof Error ? error.message : "Unknown error"}`, ); } - }; + }, [convex, generateUploadUrl, updateUploadedFile]);
🧹 Nitpick comments (4)
app/components/ImageViewer.tsx (4)
32-46: Lock background scroll while the modal is openPrevents the page from scrolling behind the overlay.
// Handle Escape key press useEffect(() => { if (!isOpen) return; @@ return () => { window.removeEventListener("keydown", handleKeyDown); }; }, [isOpen, onClose]); + + // Lock body scroll while open + useEffect(() => { + if (!isOpen) return; + const prev = document.body.style.overflow; + document.body.style.overflow = "hidden"; + return () => { + document.body.style.overflow = prev; + }; + }, [isOpen]);
104-116: Avoid duplicate IDs; generate unique ARIA idsMultiple instances could duplicate ids in the DOM. Use React useId.
-import { useState, useEffect, useRef } from "react"; +import { useState, useEffect, useRef, useId } from "react"; @@ }: ImageViewerProps) => { const [isImageLoading, setIsImageLoading] = useState(true); const dialogRef = useRef<HTMLDivElement>(null); + const titleId = useId(); + const descId = useId(); @@ - aria-labelledby="image-viewer-title" - aria-describedby="image-viewer-description" + aria-labelledby={titleId} + aria-describedby={descId} @@ - <div id="image-viewer-title" className="sr-only"> + <div id={titleId} className="sr-only"> Image Viewer </div> - <div id="image-viewer-description" className="sr-only"> + <div id={descId} className="sr-only"> {imageAlt} </div>Also applies to: 2-2
107-107: Fix invalid Tailwind classUse focus:outline-none instead of non-existent focus:outline-hidden.
- className="radix-state-open:animate-contentShow shadow-xl focus:outline-hidden relative" + className="radix-state-open:animate-contentShow shadow-xl focus:outline-none relative"
130-147: Simplify Image sizing; remove conflicting inline width/heightInline width/height:auto overrides Tailwind h-full w-full. Let the container cap size.
<Image className={`h-full w-full object-contain transition-opacity duration-300 ${ isImageLoading ? "opacity-0" : "opacity-100" }`} src={imageSrc} alt={imageAlt} width={1200} height={800} - style={{ - maxHeight: "85vh", - maxWidth: "90vw", - height: "auto", - width: "auto", - }} sizes="(max-width: 768px) 90vw, 85vw" onLoad={handleImageLoad} onError={handleImageError} />
📜 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/components/ImageViewer.tsx(1 hunks)app/hooks/useFileUpload.ts(1 hunks)convex/messages.ts(5 hunks)lib/chat/chat-processor.ts(3 hunks)lib/utils/file-utils.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- lib/chat/chat-processor.ts
- lib/utils/file-utils.ts
🧰 Additional context used
📓 Path-based instructions (2)
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
**/*.{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/messages.tsapp/hooks/useFileUpload.tsapp/components/ImageViewer.tsx
🧠 Learnings (12)
📚 Learning: 2025-08-29T13:33:09.901Z
Learnt from: CR
PR: hackerai-tech/hackerai#0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-08-29T13:33:09.901Z
Learning: Applies to convex/**/*.ts : Use ctx.storage.getUrl(fileId) to obtain signed URLs; it returns null if the file does not exist
Applied to files:
convex/messages.tsapp/hooks/useFileUpload.ts
📚 Learning: 2025-08-29T13:33:09.901Z
Learnt from: CR
PR: hackerai-tech/hackerai#0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-08-29T13:33:09.901Z
Learning: Applies to convex/**/*.ts : Queries do not support .delete(); collect results and call ctx.db.delete(_id) per document
Applied to files:
convex/messages.ts
📚 Learning: 2025-08-29T13:33:09.901Z
Learnt from: CR
PR: hackerai-tech/hackerai#0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-08-29T13:33:09.901Z
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/hooks/useFileUpload.ts
📚 Learning: 2025-08-29T13:33:09.901Z
Learnt from: CR
PR: hackerai-tech/hackerai#0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-08-29T13:33:09.901Z
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/hooks/useFileUpload.ts
📚 Learning: 2025-08-29T13:33:09.901Z
Learnt from: CR
PR: hackerai-tech/hackerai#0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-08-29T13:33:09.901Z
Learning: Applies to convex/**/*.ts : Minimize calls from actions to queries/mutations to avoid race conditions from splitting transactions
Applied to files:
app/hooks/useFileUpload.ts
📚 Learning: 2025-08-29T13:33:09.901Z
Learnt from: CR
PR: hackerai-tech/hackerai#0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-08-29T13:33:09.901Z
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:
app/hooks/useFileUpload.ts
📚 Learning: 2025-08-29T13:33:09.901Z
Learnt from: CR
PR: hackerai-tech/hackerai#0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-08-29T13:33:09.901Z
Learning: Applies to convex/**/*.ts : Use ctx.runQuery from queries/mutations/actions to call a query
Applied to files:
app/hooks/useFileUpload.ts
📚 Learning: 2025-08-29T13:33:09.901Z
Learnt from: CR
PR: hackerai-tech/hackerai#0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-08-29T13:33:09.901Z
Learning: Applies to convex/**/*.ts : Use query/mutation/action only for public API functions
Applied to files:
app/hooks/useFileUpload.ts
📚 Learning: 2025-08-29T13:33:09.901Z
Learnt from: CR
PR: hackerai-tech/hackerai#0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-08-29T13:33:09.901Z
Learning: Applies to convex/**/*.ts : Store and handle files as Blob objects; convert to/from Blob when using Convex storage
Applied to files:
app/hooks/useFileUpload.ts
📚 Learning: 2025-08-29T13:33:09.901Z
Learnt from: CR
PR: hackerai-tech/hackerai#0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-08-29T13:33:09.901Z
Learning: Applies to convex/**/*.ts : Use ctx.runMutation from mutations/actions to call a mutation
Applied to files:
app/hooks/useFileUpload.ts
📚 Learning: 2025-08-29T13:33:09.901Z
Learnt from: CR
PR: hackerai-tech/hackerai#0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-08-29T13:33:09.901Z
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:
app/hooks/useFileUpload.ts
📚 Learning: 2025-08-29T13:33:09.901Z
Learnt from: CR
PR: hackerai-tech/hackerai#0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-08-29T13:33:09.901Z
Learning: Applies to convex/**/*.ts : Always include argument and return validators for all Convex functions (query/internalQuery/mutation/internalMutation/action/internalAction)
Applied to files:
app/hooks/useFileUpload.ts
🧬 Code graph analysis (2)
convex/messages.ts (1)
convex/_generated/server.js (4)
mutation(49-49)mutation(49-49)query(29-29)query(29-29)
app/hooks/useFileUpload.ts (4)
app/contexts/GlobalState.tsx (1)
useGlobalState(279-285)convex/messages.ts (2)
generateUploadUrl(286-298)deleteFile(345-360)lib/utils/file-utils.ts (5)
MAX_FILES_LIMIT(197-197)validateFile(151-160)UploadedFileState(21-28)uploadSingleFileToConvex(85-105)createFileMessagePartFromUploadedFile(202-217)convex/_generated/dataModel.d.ts (1)
Id(48-49)
🔇 Additional comments (12)
app/components/ImageViewer.tsx (1)
20-31: Nice: loading reset on imageSrc change and focus management on openBoth behaviors address common modal and image UX pitfalls. Good job.
convex/messages.ts (7)
182-196: LGTM on file cleanup implementation.The storage cleanup logic properly handles errors without blocking the main deletion operation.
242-247: LGTM on clearing storage references.Properly clears storage references when replacing content with text.
260-271: LGTM on message deletion cleanup.The file cleanup loop properly handles errors and continues processing even if individual file deletions fail.
286-298: LGTM on authentication and upload URL generation.Proper authentication check and correct use of Convex storage API.
303-317: LGTM on single file URL retrieval.Proper authentication and use of
ctx.storage.getUrlwhich returns null for non-existent files.
322-340: LGTM on multiple file URLs retrieval.Efficient batch processing with Promise.all and proper return type annotation.
345-360: LGTM on file deletion endpoint.Proper authentication and storage deletion implementation.
app/hooks/useFileUpload.ts (4)
141-142: LGTM on index calculation fix.The index calculation has been properly fixed to use the current iteration index instead of
indexOf.
268-272: LGTM on message parts conversion.Proper filtering of null values and correct usage of the file utility functions.
275-285: LGTM on upload status helpers.Clean implementations for checking upload completion and progress.
288-338: LGTM on drag and drop handlers.Proper event handling with drag counter to manage nested drag events and correct state management.
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: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
convex/messages.ts (3)
136-142: Also persist storage_ids for client-saved messagesMirror the same extraction/persistence in saveMessageFromClient so cleanup is consistent for all messages.
- await ctx.db.insert("messages", { + const storageIds: Array<Id<"_storage">> = []; + for (const part of args.parts) { + if (part?.type === "file" && part.storageId) { + storageIds.push(part.storageId as Id<"_storage">); + } + } + + await ctx.db.insert("messages", { id: args.id, chat_id: args.chatId, role: args.role, parts: args.parts, + storage_ids: storageIds.length > 0 ? storageIds : undefined, update_time: Date.now(), });
174-179: Avoid .filter in Convex queries; use a composite indexDo not use filter in queries. Define an index on (chat_id, role) and query via withIndex.
- 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(); + // Requires schema index: messages.by_chat_id_role (chat_id asc, role asc) + 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();Be sure to add the index in convex/schema.ts.
225-231: Don’t query _id via withIndex; use ctx.db.getUsing withIndex("by_id") on _id is invalid. Fetch by id with ctx.db.get.
- const message = await ctx.db - .query("messages") - .withIndex("by_id", (q) => - q.eq("_id", args.messageId as Id<"messages">), - ) - .first(); + const message = await ctx.db.get(args.messageId);
♻️ Duplicate comments (2)
convex/messages.ts (1)
35-43: Track storage_ids on insert to enable cleanupExtract file storage IDs from parts and persist them; otherwise downstream cleanup won’t work. Also use proper Id typing. This aligns with prior suggestion.
- // Extract storage IDs from file parts for cleanup tracking - - await ctx.db.insert("messages", { + // Extract storage IDs from file parts for cleanup tracking + const storageIds: Array<Id<"_storage">> = []; + for (const part of args.parts) { + if (part?.type === "file" && part.storageId) { + storageIds.push(part.storageId as Id<"_storage">); + } + } + + await ctx.db.insert("messages", { id: args.id, chat_id: args.chatId, role: args.role, parts: args.parts, + storage_ids: storageIds.length > 0 ? storageIds : undefined, update_time: Date.now(), });app/components/FilePartRenderer.tsx (1)
57-64: Pass undefined to skip Convex useQuery (not the string "skip").This also lets you drop the Id cast if part is typed as suggested.
- const fileUrl = useQuery( - api.fileStorage.getFileUrl, - shouldFetchUrl ? { storageId: part.storageId as Id<"_storage"> } : "skip", - ); + const fileUrl = useQuery( + api.fileStorage.getFileUrl, + shouldFetchUrl ? { storageId: part.storageId! } : undefined, + );
🧹 Nitpick comments (6)
convex/messages.ts (1)
249-273: Avoid .collect() for potentially large deletesPrefer async iteration to reduce memory and align with Convex best practices.
- const messages = await ctx.db - .query("messages") - .withIndex("by_chat_id", (q) => - q - .eq("chat_id", message.chat_id) - .gt("_creationTime", message._creationTime), - ) - .collect(); - - for (const msg of messages) { + const subsequent = ctx.db + .query("messages") + .withIndex("by_chat_id", (q) => + q.eq("chat_id", message.chat_id).gt("_creationTime", message._creationTime), + ); + + for await (const msg of subsequent) { // Clean up files associated with this message if (msg.storage_ids && msg.storage_ids.length > 0) { for (const storageId of msg.storage_ids) { try { await ctx.storage.delete(storageId); } catch (error) { console.error(`Failed to delete file ${storageId}:`, error); // Continue with deletion even if file cleanup fails } } } await ctx.db.delete(msg._id); }app/hooks/useChatHandlers.ts (2)
13-13: Narrow sendMessage typing for safer payloadsAvoid any; define a minimal payload type to prevent accidental shape drift.
- sendMessage: (message?: any, options?: { body?: any }) => void; + sendMessage: ( + message?: { text?: string; files?: Array<FileUIObject> }, + options?: { body?: Record<string, unknown> }, + ) => void;
53-58: Normalize input once and reuse; keep fallback consistentTrim once and use the same value in both primary and fallback sends.
const handleSubmit = async (e: React.FormEvent) => { e.preventDefault(); - // Allow submission if there's text input or uploaded files - const hasValidFiles = uploadedFiles.some((f) => f.uploaded && f.url); - if (input.trim() || hasValidFiles) { + // Allow submission if there's text input or uploaded files + const trimmed = input.trim(); + const hasValidFiles = uploadedFiles.some((f) => f.uploaded && f.url); + if (trimmed || hasValidFiles) { @@ - sendMessage( - { - text: input.trim() || undefined, - files: fileObjects.length > 0 ? fileObjects : undefined, - }, + sendMessage( + { + text: trimmed || undefined, + files: fileObjects.length > 0 ? fileObjects : undefined, + }, { body: { mode, todos, }, }, ); } catch (error) { console.error("Failed to process files:", error); // Fallback to text-only message if file processing fails - sendMessage( - { text: input }, + sendMessage( + { text: trimmed }, { body: { mode, todos, }, }, ); } clearInput(); clearUploadedFiles();Also applies to: 72-79, 80-91, 92-104, 106-108
app/components/FilePartRenderer.tsx (2)
39-55: Remove ineffective key prop inside child component.Keys should be set where the component is instantiated in a list, not inside the component’s own DOM tree.
- <div - key={partId} - className="p-2 w-full max-w-80 min-w-64 border rounded-lg bg-background" - > + <div className="p-2 w-full max-w-80 min-w-64 border rounded-lg bg-background">
102-105: Simplify redundant conditional.Both branches set the same class string.
- const innerContainerClass = isMultipleImages - ? "bg-token-main-surface-secondary text-token-text-tertiary relative flex items-center justify-center overflow-hidden" - : "bg-token-main-surface-secondary text-token-text-tertiary relative flex items-center justify-center overflow-hidden"; + const innerContainerClass = + "bg-token-main-surface-secondary text-token-text-tertiary relative flex items-center justify-center overflow-hidden";app/components/ChatInput.tsx (1)
114-122: Use a standards-compliant accept value.accept="" isn’t a valid pattern; use "/*" to allow all file types.
- accept="*" + accept="*/*"
📜 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 (9)
app/components/AttachmentButton.tsx(1 hunks)app/components/ChatInput.tsx(5 hunks)app/components/FilePartRenderer.tsx(1 hunks)app/hooks/useChatHandlers.ts(4 hunks)app/hooks/useFileUpload.ts(1 hunks)components/ui/popover.tsx(1 hunks)convex/fileStorage.ts(1 hunks)convex/messages.ts(4 hunks)package.json(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- package.json
🚧 Files skipped from review as they are similar to previous changes (1)
- app/hooks/useFileUpload.ts
🧰 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:
components/ui/popover.tsxconvex/messages.tsapp/components/ChatInput.tsxapp/components/AttachmentButton.tsxconvex/fileStorage.tsapp/hooks/useChatHandlers.tsapp/components/FilePartRenderer.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.tsconvex/fileStorage.ts
🧠 Learnings (2)
📚 Learning: 2025-08-29T13:33:09.901Z
Learnt from: CR
PR: hackerai-tech/hackerai#0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-08-29T13:33:09.901Z
Learning: Applies to convex/**/*.ts : Use ctx.storage.getUrl(fileId) to obtain signed URLs; it returns null if the file does not exist
Applied to files:
convex/messages.tsconvex/fileStorage.tsapp/components/FilePartRenderer.tsx
📚 Learning: 2025-08-29T13:33:09.901Z
Learnt from: CR
PR: hackerai-tech/hackerai#0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-08-29T13:33:09.901Z
Learning: Applies to convex/**/*.ts : Queries do not support .delete(); collect results and call ctx.db.delete(_id) per document
Applied to files:
convex/messages.ts
🧬 Code graph analysis (5)
components/ui/popover.tsx (1)
lib/utils.ts (1)
cn(8-10)
app/components/ChatInput.tsx (5)
app/contexts/GlobalState.tsx (1)
useGlobalState(279-285)app/hooks/useFileUpload.ts (1)
useFileUpload(24-364)app/components/FileUploadPreview.tsx (1)
FileUploadPreview(26-217)app/components/AttachmentButton.tsx (1)
AttachmentButton(19-98)app/components/ScrollToBottomButton.tsx (1)
ScrollToBottomButton(10-34)
app/components/AttachmentButton.tsx (2)
app/contexts/GlobalState.tsx (1)
useGlobalState(279-285)app/hooks/useUpgrade.ts (1)
useUpgrade(5-72)
app/hooks/useChatHandlers.ts (1)
lib/utils/file-utils.ts (2)
FileUIObject(14-19)createFileUIObject(110-117)
app/components/FilePartRenderer.tsx (2)
convex/_generated/dataModel.d.ts (1)
Id(48-49)app/components/ImageViewer.tsx (1)
ImageViewer(11-152)
🔇 Additional comments (6)
convex/messages.ts (2)
182-196: File cleanup on delete: solidPer-message storage cleanup with per-file try/catch is correct and resilient.
242-247: Unsetting optional field: verify semanticsSetting storage_ids to undefined may not remove the field. If you want to clear references, set [] explicitly or confirm Convex supports unsetting via undefined in patch.
- await ctx.db.patch(message._id, { + await ctx.db.patch(message._id, { parts: [{ type: "text", text: args.newContent }], - storage_ids: undefined, // Clear file references when replacing with text + storage_ids: [], // Clear file references when replacing with text update_time: Date.now(), });If you rely on unsetting, please confirm behavior and adjust validators accordingly.
app/components/AttachmentButton.tsx (1)
41-63: Pro path UX looks goodTooltip + ghost icon button with proper aria-label and disabled handling reads clean.
convex/fileStorage.ts (1)
17-24: Double-check entitlement source for Pro gatingConfirm that user.entitlements includes "pro-monthly-plan" for your auth provider; otherwise uploads may be blocked for valid Pro users. If entitlements are elsewhere, call a server-side helper (e.g., internal.users.hasProPlan).
app/components/FilePartRenderer.tsx (1)
124-131: No changes needed: Next/Image remotePatterns already covers Convex URLs. TheremotePatternsarray in next.config.ts includes{ protocol: "https", hostname: "*.convex.cloud" }, so Convex storage domains are permitted.app/components/ChatInput.tsx (1)
141-145: Enter-to-send calls the unified submit without type issues.With handleSubmit now accepting SyntheticEvent, this call path is type-safe and won’t break consumers expecting FormEvent.
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: 8
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)
56-61: Add returns validator to comply with function schema guideline.Keep it broad if exact pagination type is cumbersome.
export const getMessagesByChatId = query({ args: { chatId: v.string(), paginationOpts: paginationOptsValidator, }, + returns: v.any(),
174-181: Avoid filter; use a compound index to fetch last assistant message.Filter scans; prefer withIndex("by_chat_id_and_role"). Add this index in schema.
- 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_and_role", (q) => + q.eq("chat_id", args.chatId).eq("role", "assistant"), + ) + .order("desc") + .first();
225-231: Fetch by _id with ctx.db.get instead of querying an index.Simpler and O(1).
- const message = await ctx.db - .query("messages") - .withIndex("by_id", (q) => - q.eq("_id", args.messageId as Id<"messages">), - ) - .first(); + const message = await ctx.db.get(args.messageId);app/hooks/useChatHandlers.ts (2)
149-153: Prefer Id types for document IDs; avoid castsType the parameter as
Id<"messages">and drop the cast.- const handleEditMessage = async (messageId: string, newContent: string) => { + const handleEditMessage = async (messageId: Id<"messages">, newContent: string) => { @@ - await regenerateWithNewContent({ - messageId: messageId as Id<"messages">, + await regenerateWithNewContent({ + messageId, newContent, });
8-18: TypechatIdasId<"chats">in propsAligns with repo typing guidelines and avoids accidental cross-table usage.
-interface UseChatHandlersProps { - chatId: string; +interface UseChatHandlersProps { + chatId: Id<"chats">;Also applies to: 20-28
♻️ Duplicate comments (6)
lib/chat/chat-processor.ts (1)
83-88: Good: detect media after truncation/URL resolution.This addresses earlier feedback about false positives when attachments are truncated.
app/components/FilePartRenderer.tsx (2)
57-65: Do not pass "skip" to useQuery; pass undefined.Prevents invalid-args calls and fixes types. This also addresses earlier feedback.
- const ConvexFilePart = memo(({ part, partId }: { part: any; partId: string }) => { + const ConvexFilePart = memo(({ part, partId }: { part: FileMessagePart; partId: string }) => { @@ - const fileUrl = useQuery( - api.fileStorage.getFileUrl, - shouldFetchUrl ? { storageId: part.storageId as Id<"_storage"> } : "skip", - ); + const fileUrl = useQuery( + api.fileStorage.getFileUrl, + shouldFetchUrl ? { storageId: part.storageId as Id<"_storage"> } : undefined, + );
9-14: Strongly type props (avoid any/string) per guidelines.Use Convex Id and shared file part type.
-import { Id } from "@/convex/_generated/dataModel"; +import { Id } from "@/convex/_generated/dataModel"; +import type { FileMessagePart } from "@/types/file"; @@ -interface FilePartRendererProps { - part: any; +interface FilePartRendererProps { + part: FileMessagePart; partIndex: number; - messageId: string; + messageId: Id<"messages">; totalFileParts?: number; }convex/fileStorage.ts (3)
33-47: IDOR:getFileUrllacks authorization by resource ownershipAny authenticated user can resolve URLs for arbitrary storage IDs. Require a
messageIdand verify the message belongs to the caller and references thestorageId.-import { mutation, query } from "./_generated/server"; +import { mutation, query } from "./_generated/server"; +import { internal } from "./_generated/api"; @@ -export const getFileUrl = query({ - args: { - storageId: v.id("_storage"), - }, +export const getFileUrl = query({ + args: { + messageId: v.id("messages"), + storageId: v.id("_storage"), + }, returns: v.union(v.string(), v.null()), handler: async (ctx, args) => { const user = await ctx.auth.getUserIdentity(); @@ - return await ctx.storage.getUrl(args.storageId); + // Verify chat ownership and that the message references this storageId + const message = await ctx.db.get(args.messageId); + if (!message) return null; + await ctx.runQuery(internal.chats.verifyChatOwnership as unknown as any, { + chatId: message.chat_id, + userId: user.subject, + }); + if (!message.storage_ids?.some((id: string) => id === args.storageId)) { + return null; + } + return await ctx.storage.getUrl(args.storageId); }, });Note: add a proper type for
internal.chats.verifyChatOwnershipif available to avoid theas any.
52-70: IDOR: batch URL fetch must enforce ownership per messageRequire
messageId, verify ownership, and only resolve URLs that the message references.-export const getFileUrls = query({ - args: { - storageIds: v.array(v.id("_storage")), - }, +export const getFileUrls = query({ + args: { + messageId: v.id("messages"), + storageIds: v.array(v.id("_storage")), + }, returns: v.array(v.union(v.string(), v.null())), handler: async (ctx, args) => { const user = await ctx.auth.getUserIdentity(); @@ - const urls = await Promise.all( - args.storageIds.map((storageId) => ctx.storage.getUrl(storageId)), - ); + const message = await ctx.db.get(args.messageId); + if (!message) return []; + await ctx.runQuery(internal.chats.verifyChatOwnership as unknown as any, { + chatId: message.chat_id, + userId: user.subject, + }); + const allowed = new Set((message.storage_ids ?? []) as string[]); + const urls = await Promise.all( + args.storageIds.map((storageId) => + allowed.has(storageId) ? ctx.storage.getUrl(storageId) : Promise.resolve(null), + ), + ); return urls; }, });
96-111: IDOR:deleteFileallows deletion by arbitrary storageIdRequire
messageIdand verify ownership and that the message references the storageId before deletion.-export const deleteFile = mutation({ - args: { - storageId: v.id("_storage"), - }, +export const deleteFile = mutation({ + args: { + messageId: v.id("messages"), + storageId: v.id("_storage"), + }, returns: v.null(), handler: async (ctx, args) => { const user = await ctx.auth.getUserIdentity(); @@ - await ctx.storage.delete(args.storageId); + const message = await ctx.db.get(args.messageId); + if (!message) return null; + await ctx.runQuery(internal.chats.verifyChatOwnership as unknown as any, { + chatId: message.chat_id, + userId: user.subject, + }); + if (message.storage_ids?.some((id: string) => id === args.storageId)) { + await ctx.storage.delete(args.storageId); + } return null; }, });
🧹 Nitpick comments (9)
lib/db/actions.ts (2)
89-97: Avoid cast on storageIds when calling mutation.With the function typed above, no cast needed.
- storageIds: - storageIds.length > 0 ? (storageIds as Id<"_storage">[]) : undefined, + storageIds: storageIds.length > 0 ? storageIds : undefined,
208-298: Strong-type storageIdsToFetch and map; minor safety nits.Also avoid mutating via JSON deep-clone if performance becomes a concern.
export async function transformStorageIdsToUrls( messages: UIMessage[], ): Promise<UIMessage[]> { // Create a deep copy of messages to avoid mutation const updatedMessages = JSON.parse(JSON.stringify(messages)) as UIMessage[]; // Collect all storageIds that need URL fetching - const storageIdsToFetch: string[] = []; - const storageIdToFilePartMap = new Map< - string, + const storageIdsToFetch: Array<Id<"_storage">> = []; + const storageIdToFilePartMap = new Map< + Id<"_storage">, Array<{ messageIndex: number; partIndex: number }> >(); @@ - const storageId = part.storageId || part.url; - if (storageId && !storageId.startsWith("http")) { + const storageId = + (part.storageId as Id<"_storage"> | undefined) ?? + (!part.url?.startsWith("http") ? (part.url as Id<"_storage">) : undefined); + if (storageId) { if (!storageIdToFilePartMap.has(storageId)) { storageIdsToFetch.push(storageId); storageIdToFilePartMap.set(storageId, []); } storageIdToFilePartMap .get(storageId)! .push({ messageIndex, partIndex }); }Optional: replace JSON clone with a structural copy using map/reduce if this becomes hot.
app/components/FilePartRenderer.tsx (2)
2-8: Remove unused import.FileText isn’t used.
-import { Loader2, FileText, AlertCircle, File } from "lucide-react"; +import { Loader2, AlertCircle, File } from "lucide-react";
149-167: Comparator dependencies: include mediaType to avoid stale renders.You already compare mediaType at the memo wrapper; keep it in the renderedFilePart deps too for consistency.
- }, [messageId, partIndex, part.url, part.storageId, part.name, part.filename]); + }, [messageId, partIndex, part.url, part.storageId, part.name, part.filename, part.mediaType]);app/hooks/useChatHandlers.ts (1)
183-188: Stabilize handlers with useCallbackPrevents unnecessary re-renders in consumers; low-priority.
lib/utils/file-utils.ts (3)
1-1: Remove unused importFileUIObjectIt’s not referenced.
-import { FileMessagePart, FileUIObject, UploadedFileState } from "@/types/file"; +import { FileMessagePart, UploadedFileState } from "@/types/file";
7-11: Docstring contradicts implementationComment says URLs must be fetched separately, but this function already fetches URLs via
getFileUrls. Update the note.
62-82: Harden response parsing and type the return asId<"_storage">Validate JSON shape before returning; surface a clear error if missing.
-export async function uploadSingleFileToConvex( - file: File, - generateUploadUrl: () => Promise<string>, -): Promise<string> { +export async function uploadSingleFileToConvex( + file: File, + generateUploadUrl: () => Promise<string>, +): Promise<Id<"_storage">> { @@ - const { storageId } = await result.json(); - return storageId; + const json = (await result.json()) as { storageId?: string }; + if (!json.storageId) { + throw new Error("Upload succeeded but no storageId was returned"); + } + return json.storageId as Id<"_storage">;convex/fileStorage.ts (1)
75-91: Service-key query: consider scoping by message/chat when possibleNot blocking, but if feasible pass
messageId/chatIdand apply the same reference filtering to reduce blast radius even with a valid service key.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (9)
app/api/chat/route.ts(2 hunks)app/components/FilePartRenderer.tsx(1 hunks)app/hooks/useChatHandlers.ts(5 hunks)convex/fileStorage.ts(1 hunks)convex/messages.ts(8 hunks)lib/chat/chat-processor.ts(3 hunks)lib/db/actions.ts(3 hunks)lib/utils/file-utils.ts(1 hunks)types/file.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- app/api/chat/route.ts
🧰 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:
types/file.tslib/db/actions.tslib/chat/chat-processor.tsapp/components/FilePartRenderer.tsxconvex/messages.tslib/utils/file-utils.tsconvex/fileStorage.tsapp/hooks/useChatHandlers.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.tsconvex/fileStorage.ts
🧠 Learnings (3)
📚 Learning: 2025-08-29T13:33:09.901Z
Learnt from: CR
PR: hackerai-tech/hackerai#0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-08-29T13:33:09.901Z
Learning: Applies to convex/**/*.ts : Use ctx.storage.getUrl(fileId) to obtain signed URLs; it returns null if the file does not exist
Applied to files:
app/components/FilePartRenderer.tsxconvex/messages.tslib/utils/file-utils.tsconvex/fileStorage.ts
📚 Learning: 2025-08-29T13:33:09.901Z
Learnt from: CR
PR: hackerai-tech/hackerai#0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-08-29T13:33:09.901Z
Learning: Applies to convex/**/*.ts : Queries do not support .delete(); collect results and call ctx.db.delete(_id) per document
Applied to files:
convex/messages.ts
📚 Learning: 2025-08-29T13:33:09.901Z
Learnt from: CR
PR: hackerai-tech/hackerai#0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-08-29T13:33:09.901Z
Learning: Applies to convex/**/*.ts : Store and handle files as Blob objects; convert to/from Blob when using Convex storage
Applied to files:
lib/utils/file-utils.ts
🧬 Code graph analysis (7)
types/file.ts (1)
lib/db/actions.ts (1)
FileUIObject(196-202)
lib/db/actions.ts (1)
convex/_generated/dataModel.d.ts (1)
Id(48-49)
lib/chat/chat-processor.ts (2)
lib/db/actions.ts (1)
transformStorageIdsToUrls(208-298)lib/moderation.ts (1)
getModerationResult(5-60)
app/components/FilePartRenderer.tsx (2)
convex/_generated/dataModel.d.ts (1)
Id(48-49)app/components/ImageViewer.tsx (1)
ImageViewer(11-152)
lib/utils/file-utils.ts (2)
convex/fileStorage.ts (2)
generateUploadUrl(8-28)getFileUrls(52-70)types/file.ts (2)
FileMessagePart(1-8)UploadedFileState(18-25)
convex/fileStorage.ts (1)
convex/chats.ts (1)
validateServiceKey(6-10)
app/hooks/useChatHandlers.ts (1)
convex/_generated/api.js (2)
api(21-21)api(21-21)
🔇 Additional comments (10)
types/file.ts (1)
18-25: Strong-type UploadedFileState.storageId and align optional url.Matches the actual upload lifecycle.
export interface UploadedFileState { file: File; uploading: boolean; uploaded: boolean; error?: string; - storageId?: string; - url?: string; // Store the resolved URL + storageId?: Id<"_storage">; + url?: string; // Resolved signed URL }⛔ 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.901Z Learning: Applies to convex/**/*.ts : Use ctx.storage.getUrl(fileId) to obtain signed URLs; it returns null if the file does not existlib/chat/chat-processor.ts (3)
13-31: Media detection reads well and is side-effect free.Logic is correct for images/PDFs. Consider extending to video/* later if needed.
109-114: Return shape addition looks good.Adding hasMediaFiles and processedMessages is clear and non-breaking.
93-99: No client-side imports of chat-processor
chat-processor is only imported in the server-side API route (app/api/chat/route.ts); server-only dependency risk resolved.lib/db/actions.ts (1)
10-10: Server-only usage confirmed. No client-side modules importlib/db/actionsor its dependentlib/chat/chat-processor(only used inapp/api/chat/route.ts), so the service-role key remains server-only.convex/messages.ts (2)
242-246: Good: clear storage_ids on text replacement.Prevents orphaned references.
259-273: Per-message file cleanup before delete is solid.Granular try/catch is appropriate.
lib/utils/file-utils.ts (2)
87-104: LGTM: file-part creation aligns with type contractUses
name,mediaType,size,url,storageIdcorrectly.
167-182: LGTM: null-safe helper for uploaded file stateAppropriate guard on
uploaded,url, andstorageId.convex/fileStorage.ts (1)
8-28: LGTM: upload URL gated by auth and entitlementGood entitlement check before
generateUploadUrl.
app/components/FilePartRenderer.tsx
Outdated
| // Memoize ConvexFilePart to prevent unnecessary re-renders and re-queries | ||
| const ConvexFilePart = memo(({ part, partId }: { part: any; partId: string }) => { | ||
| // Use direct URL if available, otherwise fetch from storageId (for legacy messages) | ||
| const shouldFetchUrl = part.storageId && !part.url; | ||
| const fileUrl = useQuery( | ||
| api.fileStorage.getFileUrl, | ||
| shouldFetchUrl ? { storageId: part.storageId as Id<"_storage"> } : "skip", | ||
| ); | ||
|
|
||
| // Determine the actual URL to use | ||
| const actualUrl = part.url || fileUrl; | ||
|
|
||
| if (shouldFetchUrl && fileUrl === undefined) { | ||
| // Loading state for legacy storageId-based files | ||
| return ( | ||
| <FilePreviewCard | ||
| partId={partId} | ||
| icon={<Loader2 className="h-6 w-6 text-white animate-spin" />} | ||
| fileName={part.name || part.filename || "Unknown file"} | ||
| subtitle="Loading file..." | ||
| /> | ||
| ); | ||
| } | ||
|
|
||
| if (!actualUrl || (shouldFetchUrl && fileUrl === null)) { | ||
| // File not found or error | ||
| return ( | ||
| <FilePreviewCard | ||
| partId={partId} | ||
| icon={<AlertCircle className="h-6 w-6 text-white" />} | ||
| fileName={part.name || part.filename || "Unknown file"} | ||
| subtitle="File not found" | ||
| /> | ||
| ); | ||
| } | ||
|
|
||
| // Handle image files | ||
| if (part.mediaType?.startsWith("image/")) { | ||
| const altText = part.name || `Uploaded image ${partIndex + 1}`; | ||
| const isMultipleImages = totalFileParts > 1; | ||
|
|
||
| // Different styling for single vs multiple images | ||
| const containerClass = isMultipleImages | ||
| ? "overflow-hidden rounded-lg" | ||
| : "overflow-hidden rounded-lg max-w-64"; | ||
|
|
||
| const innerContainerClass = isMultipleImages | ||
| ? "bg-token-main-surface-secondary text-token-text-tertiary relative flex items-center justify-center overflow-hidden" | ||
| : "bg-token-main-surface-secondary text-token-text-tertiary relative flex items-center justify-center overflow-hidden"; | ||
|
|
||
| const buttonClass = isMultipleImages | ||
| ? "overflow-hidden rounded-lg" | ||
| : "overflow-hidden rounded-lg w-full"; | ||
|
|
||
| const imageClass = isMultipleImages | ||
| ? "aspect-square object-cover object-center h-32 w-32 rounded-se-2xl rounded-ee-sm overflow-hidden transition-opacity duration-300 opacity-100" | ||
| : "w-full h-auto max-h-96 max-w-64 object-contain rounded-lg transition-opacity duration-300 opacity-100"; | ||
|
|
||
| return ( | ||
| <div key={partId} className={containerClass}> | ||
| <div className={innerContainerClass}> | ||
| <button | ||
| onClick={() => setSelectedImage({ src: actualUrl, alt: altText })} | ||
| className={buttonClass} | ||
| aria-label={`View ${altText} in full size`} | ||
| type="button" | ||
| > | ||
| <Image | ||
| src={actualUrl} | ||
| alt={altText} | ||
| width={902} | ||
| height={2048} | ||
| className={imageClass} | ||
| style={{ maxWidth: "100%", height: "auto" }} | ||
| /> | ||
| </button> | ||
| </div> | ||
| </div> | ||
| ); | ||
| } | ||
|
|
||
| // Handle all non-image files with the new UI | ||
| return ( | ||
| <FilePreviewCard | ||
| partId={partId} | ||
| icon={<File className="h-6 w-6 text-white" />} | ||
| fileName={part.name || part.filename || "Document"} | ||
| subtitle="Document" | ||
| /> | ||
| ); | ||
| }); |
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.
🛠️ Refactor suggestion
Move ConvexFilePart and FilePreviewCard to module scope.
Defining memoized components inside the parent recreates component types on each render, causing unmount/remount and re-fetches. Hoist them to top-level to stabilize identity and avoid extra queries.
- const FilePreviewCard = useMemo(() => ({ ... }) => ( ... ), []);
- const ConvexFilePart = memo(({ part, partId }: { part: FileMessagePart; partId: string }) => {
- // ...
- });
+// Move outside the component (top-level in this module):
+// export const FilePreviewCard = memo(function FilePreviewCard(...) { ... })
+// export const ConvexFilePart = memo(function ConvexFilePart({ part, partId }: { part: FileMessagePart; partId: string }) { ... })If helpful, I can provide a full hoisted version.
Committable suggestion skipped: line range outside the PR's diff.
| // Allow submission if there's text input or uploaded files | ||
| const hasValidFiles = uploadedFiles.some((f) => f.uploaded && f.url); | ||
| if (input.trim() || hasValidFiles) { | ||
| if (!hasActiveChat) { |
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.
🛠️ Refactor suggestion
Empty payload can be sent (files gated by url, not storageId); also wrong file-part shape
- Guard uses
uploaded && url, but mapping requiresstorageId, so you can submit with no text and zero valid files, producing{ text: undefined, files: undefined }. - File part uses
filenamebut the type usesname, andsizeis omitted.
- // Allow submission if there's text input or uploaded files
- const hasValidFiles = uploadedFiles.some((f) => f.uploaded && f.url);
- if (input.trim() || hasValidFiles) {
+ // Precompute valid files (must have storageId + url)
+ const validFiles = uploadedFiles.filter(
+ (f) => f.uploaded && f.url && f.storageId
+ );
+ const hasValidFiles = validFiles.length > 0;
+ if (input.trim() || hasValidFiles) {
@@
- try {
- // Get file objects from uploaded files - URLs are already resolved in global state
- const validFiles = uploadedFiles.filter(
- (file) => file.uploaded && file.url && file.storageId,
- );
-
- sendMessage(
+ try {
+ sendMessage(
{
- text: input.trim() || undefined,
- files:
- validFiles.length > 0
- ? validFiles.map((uploadedFile) => ({
- type: "file" as const,
- filename: uploadedFile.file.name,
- mediaType: uploadedFile.file.type,
- url: uploadedFile.url!,
- storageId: uploadedFile.storageId!,
- }))
- : undefined,
+ text: input.trim() || undefined,
+ files: hasValidFiles
+ ? validFiles.map((f) => ({
+ type: "file" as const,
+ name: f.file.name,
+ mediaType: f.file.type,
+ size: f.file.size,
+ url: f.url!, // resolved earlier in upload flow
+ storageId: f.storageId!,
+ }))
+ : undefined,
},
{
body: {
mode,
todos,
},
},
);
} catch (error) {
console.error("Failed to process files:", error);
// Fallback to text-only message if file processing fails
- sendMessage(
- { text: input },
- {
- body: {
- mode,
- todos,
- },
- },
- );
+ if (input.trim()) {
+ sendMessage(
+ { text: input.trim() },
+ { body: { mode, todos } },
+ );
+ } else {
+ return; // avoid sending an empty message
+ }
}
@@
- clearInput();
- clearUploadedFiles();
+ clearInput();
+ clearUploadedFiles();To avoid duplication and ensure shape consistency, consider using the helper from lib/utils:
Additional import (outside the shown range):
import { createFileMessagePartFromUploadedFile } from "@/lib/utils/file-utils";
Then replace the mapping with:
files: validFiles.map(f => createFileMessagePartFromUploadedFile(f)!).filter(Boolean as any)
Also applies to: 73-99, 114-116
🤖 Prompt for AI Agents
In app/hooks/useChatHandlers.ts around lines 56 to 59, the current guard and
mapping allow empty payloads because it checks uploaded && url while the mapper
expects storageId, and it builds file parts with incorrect keys (uses filename,
omits size). Update the guard to derive validFiles = uploadedFiles.filter(f =>
f.uploaded && f.storageId) and only allow submission if input.trim() ||
validFiles.length > 0; import createFileMessagePartFromUploadedFile from
"@/lib/utils/file-utils" and replace manual mapping with files: validFiles.map(f
=> createFileMessagePartFromUploadedFile(f)!).filter(Boolean as any) so the
shape (name, size, storageId, etc.) is correct; ensure you omit the files field
entirely when validFiles.length === 0. Apply the same changes to the other
occurrences at lines 73-99 and 114-116.
lib/db/actions.ts
Outdated
| /** | ||
| * Extract storage IDs from message parts | ||
| * @param parts - Array of message parts | ||
| * @returns Array of storage IDs found in file parts | ||
| */ | ||
| function extractStorageIdsFromParts( | ||
| parts: UIMessagePart<any, any>[], | ||
| ): string[] { | ||
| const storageIds: string[] = []; | ||
|
|
||
| for (const part of parts) { | ||
| if (part.type === "file") { | ||
| // Check if storageId exists directly | ||
| if ((part as any).storageId) { | ||
| storageIds.push((part as any).storageId); | ||
| } | ||
| // Also check url field as it might contain storageId (before transformation) | ||
| else if ((part as any).url && typeof (part as any).url === "string") { | ||
| // Assume url contains storageId if it doesn't start with http | ||
| const url = (part as any).url; | ||
| if (!url.startsWith("http")) { | ||
| storageIds.push(url); | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| return storageIds; | ||
| } | ||
|
|
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.
🛠️ Refactor suggestion
Type storage IDs with Id<'_storage'> and stop overloading url.
Return typed IDs; tolerate legacy url-as-id but mark as migration path.
-import { UIMessagePart } from "ai";
+import { UIMessagePart } from "ai";
+import type { Id } from "@/convex/_generated/dataModel";
/**
* Extract storage IDs from message parts
* @param parts - Array of message parts
* @returns Array of storage IDs found in file parts
*/
-function extractStorageIdsFromParts(
- parts: UIMessagePart<any, any>[],
-): string[] {
- const storageIds: string[] = [];
+function extractStorageIdsFromParts(
+ parts: Array<UIMessagePart<any, any>>,
+): Array<Id<"_storage">> {
+ const storageIds: Array<Id<"_storage">> = [];
for (const part of parts) {
if (part.type === "file") {
// Check if storageId exists directly
if ((part as any).storageId) {
- storageIds.push((part as any).storageId);
+ storageIds.push((part as any).storageId as Id<"_storage">);
}
// Also check url field as it might contain storageId (before transformation)
else if ((part as any).url && typeof (part as any).url === "string") {
// Assume url contains storageId if it doesn't start with http
const url = (part as any).url;
if (!url.startsWith("http")) {
- storageIds.push(url);
+ storageIds.push(url as Id<"_storage">);
}
}
}
}
return storageIds;
}📝 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.
| /** | |
| * Extract storage IDs from message parts | |
| * @param parts - Array of message parts | |
| * @returns Array of storage IDs found in file parts | |
| */ | |
| function extractStorageIdsFromParts( | |
| parts: UIMessagePart<any, any>[], | |
| ): string[] { | |
| const storageIds: string[] = []; | |
| for (const part of parts) { | |
| if (part.type === "file") { | |
| // Check if storageId exists directly | |
| if ((part as any).storageId) { | |
| storageIds.push((part as any).storageId); | |
| } | |
| // Also check url field as it might contain storageId (before transformation) | |
| else if ((part as any).url && typeof (part as any).url === "string") { | |
| // Assume url contains storageId if it doesn't start with http | |
| const url = (part as any).url; | |
| if (!url.startsWith("http")) { | |
| storageIds.push(url); | |
| } | |
| } | |
| } | |
| } | |
| return storageIds; | |
| } | |
| import { UIMessagePart } from "ai"; | |
| import type { Id } from "@/convex/_generated/dataModel"; | |
| /** | |
| * Extract storage IDs from message parts | |
| * @param parts - Array of message parts | |
| * @returns Array of storage IDs found in file parts | |
| */ | |
| function extractStorageIdsFromParts( | |
| parts: Array<UIMessagePart<any, any>>, | |
| ): Array<Id<"_storage">> { | |
| const storageIds: Array<Id<"_storage">> = []; | |
| for (const part of parts) { | |
| if (part.type === "file") { | |
| // Check if storageId exists directly | |
| if ((part as any).storageId) { | |
| storageIds.push((part as any).storageId as Id<"_storage">); | |
| } | |
| // Also check url field as it might contain storageId (before transformation) | |
| else if ((part as any).url && typeof (part as any).url === "string") { | |
| // Assume url contains storageId if it doesn't start with http | |
| const url = (part as any).url; | |
| if (!url.startsWith("http")) { | |
| storageIds.push(url as Id<"_storage">); | |
| } | |
| } | |
| } | |
| } | |
| return storageIds; | |
| } |
🤖 Prompt for AI Agents
In lib/db/actions.ts around lines 12 to 41, the helper currently returns plain
strings and overloads the file.url field as a legacy storage-id; update it to
return typed IDs (Id<'_storage'>[]) and stop treating url as the primary
identifier. Change the function signature to return Id<'_storage'>[]; prefer
part.storageId when present and validate/convert it to Id<'_storage'>; only
accept legacy url values as a migration fallback when url is a non-http string
and convert/cast it to Id<'_storage'> with a clear comment indicating migration
usage; add narrow type guards so you don’t cast everything to any and ensure the
returned array elements are typed as Id<'_storage'>.
lib/db/actions.ts
Outdated
| export interface FileUIObject { | ||
| type: "file"; | ||
| filename: string; | ||
| mediaType: string; | ||
| url: string; | ||
| storageId: string; | ||
| } |
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.
🛠️ Refactor suggestion
Remove duplicated FileUIObject type here; import from types/file.ts.
Prevents drift.
-export interface FileUIObject {
- type: "file";
- filename: string;
- mediaType: string;
- url: string;
- storageId: string;
-}
+// import type { FileUIObject } from "@/types/file";Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In lib/db/actions.ts around lines 196 to 202, there's a duplicated FileUIObject
type declared locally; remove this local type definition and instead import the
canonical FileUIObject from types/file.ts (add the appropriate import at the top
of the file), update any references to use the imported type, and remove the
exported declaration to prevent type drift and duplication.
| export interface FileMessagePart { | ||
| type: "file"; | ||
| mediaType: string; | ||
| storageId: string; | ||
| name: string; | ||
| size: number; | ||
| url: string; // Always include URL for immediate rendering | ||
| } |
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.
Fix type contract: make url optional and use Convex Id for storageId.
Current shape conflicts with usage where url may be missing pre-resolution and storageId should be Id<'_storage'>.
+import type { Id } from "@/convex/_generated/dataModel";
export interface FileMessagePart {
type: "file";
mediaType: string;
- storageId: string;
- name: string;
- size: number;
- url: string; // Always include URL for immediate rendering
+ storageId?: Id<"_storage">;
+ name?: string;
+ size?: number;
+ url?: string; // May be absent until resolved; don't overload with storageId
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In types/file.ts around lines 1 to 8, the FileMessagePart interface requires url
and uses a plain string for storageId which conflicts with usage; change
storageId's type to Id<'_storage'> and make url optional (url?: string). Also
ensure the Convex Id type is imported where this file resides (e.g., import type
{ Id } from the Convex types module) so the new storageId type compiles.
types/file.ts
Outdated
| export interface FileUIObject { | ||
| type: "file"; | ||
| filename: string; | ||
| mediaType: string; | ||
| url: string; | ||
| storageId: string; | ||
| } |
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.
🛠️ Refactor suggestion
Deduplicate FileUIObject and strong-type storageId.
This interface is duplicated in lib/db/actions.ts and uses string for storageId. Keep a single source of truth here and import it elsewhere.
export interface FileUIObject {
type: "file";
filename: string;
mediaType: string;
url: string;
- storageId: string;
+ storageId: Id<"_storage">;
}Follow-up in lib/db/actions.ts: remove the local interface and import this one.
📝 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.
| export interface FileUIObject { | |
| type: "file"; | |
| filename: string; | |
| mediaType: string; | |
| url: string; | |
| storageId: string; | |
| } | |
| export interface FileUIObject { | |
| type: "file"; | |
| filename: string; | |
| mediaType: string; | |
| url: string; | |
| storageId: Id<"_storage">; | |
| } |
🤖 Prompt for AI Agents
types/file.ts around lines 10 to 16: the FileUIObject interface is duplicated
elsewhere and storageId is currently just a string; replace the plain string
with a stronger type (create and export a StorageId branded/alias type e.g. type
StorageId = string & { __brand: "StorageId" } or an exported opaque alias) and
export the FileUIObject interface from this file as the single source of truth;
then update lib/db/actions.ts to remove its local interface and import
FileUIObject (and StorageId if needed) from types/file.ts, and update any usages
to accept the new StorageId type (adjust constructors/parsers/creators to
cast/produce StorageId where appropriate).
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: 8
♻️ Duplicate comments (3)
lib/db/actions.ts (1)
12-40: Strongly type storage IDs and limit url-as-id to migration only.Return Id<'_storage'>[], add a narrow non-http guard, and avoid unsafe casts. This mirrors earlier feedback.
-import { UIMessagePart } from "ai"; +import { UIMessagePart } from "ai"; +import type { Id } from "@/convex/_generated/dataModel"; @@ -function extractStorageIdsFromParts( - parts: UIMessagePart<any, any>[], -): string[] { - const storageIds: string[] = []; +function extractStorageIdsFromParts( + parts: Array<UIMessagePart<any, any>>, +): Array<Id<"_storage">> { + const storageIds: Array<Id<"_storage">> = []; + const isNonHttpString = (v: unknown): v is string => + typeof v === "string" && !/^https?:\/\//i.test(v); @@ - if (part.type === "file") { + if (part.type === "file") { // Check if storageId exists directly - if ((part as any).storageId) { - storageIds.push((part as any).storageId); + if ((part as any).storageId) { + storageIds.push((part as any).storageId as Id<"_storage">); } // Also check url field as it might contain storageId (before transformation) - else if ((part as any).url && typeof (part as any).url === "string") { - // Assume url contains storageId if it doesn't start with http - const url = (part as any).url; - if (!url.startsWith("http")) { - storageIds.push(url); - } + else { + const url = (part as any).url; + if (isNonHttpString(url)) { + // Migration fallback: url temporarily holds storageId (non-http) + storageIds.push(url as Id<"_storage">); + } } } } return storageIds; }types/file.ts (1)
1-8: Fix type contract: make url optional and use Convex Id for storageId.Aligns with how parts are created/resolved and removes casts elsewhere.
+import type { Id } from "@/convex/_generated/dataModel"; export interface FileMessagePart { type: "file"; mediaType: string; - storageId: string; - name: string; - size: number; - url: string; // Always include URL for immediate rendering + storageId?: Id<"_storage">; + name?: string; + size?: number; + url?: string; // May be absent until resolved }lib/utils/file-utils.ts (1)
12-57: Don’t emit empty URLs; type storage IDs with Id<'_storage'>.Empty strings break rendering; filter out failures and use strong Id typing end-to-end. Mirrors earlier feedback.
+import type { Id } from "@/convex/_generated/dataModel"; @@ export async function uploadFilesToConvex( files: FileList, generateUploadUrl: () => Promise<string>, - getFileUrls: (storageIds: string[]) => Promise<(string | null)[]>, + getFileUrls: (storageIds: Array<Id<"_storage">>) => Promise<Array<string | null>>, ): Promise<FileMessagePart[]> { @@ - const { storageId } = await result.json(); + const { storageId } = await result.json(); @@ - const storageIds = uploadResults.map((result) => result.storageId); + const storageIds = uploadResults.map( + (result) => result.storageId as Id<"_storage">, + ); @@ - return uploadResults.map((result, index) => ({ - type: "file" as const, - mediaType: result.file.type, - storageId: result.storageId, - name: result.file.name, - size: result.file.size, - url: urls[index] || "", // Fallback to empty string if URL fetch failed - })); + return uploadResults + .map((result, index) => { + const url = urls[index]; + if (!url) return null; + return { + type: "file" as const, + mediaType: result.file.type, + storageId: result.storageId as Id<"_storage">, + name: result.file.name, + size: result.file.size, + url, + } as FileMessagePart; + }) + .filter(Boolean) as FileMessagePart[];
🧹 Nitpick comments (7)
lib/db/actions.ts (2)
258-285: Preserve Id typing through the fetch/update steps.Keep the Id<'_storage'> type for requests and map updates.
- const urls = await convex.query(api.fileStorage.getFileUrlsWithServiceKey, { + const urls = await convex.query(api.fileStorage.getFileUrlsWithServiceKey, { serviceKey: process.env.CONVEX_SERVICE_ROLE_KEY!, - storageIds: storageIdsToFetch as Id<"_storage">[], + storageIds: storageIdsToFetch, }); @@ - const storageId = storageIdsToFetch[i]; + const storageId = storageIdsToFetch[i]; const url = urls[i]; const filePartPositions = storageIdToFilePartMap.get(storageId);
203-205: Avoid JSON-based deep clone; use structuredClone for safer cloning.JSON drops symbols/undefined and can mangle types; structuredClone is available in modern runtimes.
- const updatedMessages = JSON.parse(JSON.stringify(messages)) as UIMessage[]; + const updatedMessages = typeof structuredClone === "function" + ? structuredClone(messages) + : (JSON.parse(JSON.stringify(messages)) as UIMessage[]);app/hooks/useFileUpload.ts (3)
124-142: Stabilize upload indices with a baseIndex variable.Current math works but is non-obvious; capturing baseIndex improves clarity and guards future changes.
- const startFileUploads = useCallback( - (files: File[]) => { - files.forEach((file, index) => { + const startFileUploads = useCallback( + (files: File[]) => { + const baseIndex = uploadedFiles.length; + files.forEach((file, index) => { // Add file as "uploading" state immediately const uploadState: UploadedFileState = { file, uploading: true, uploaded: false, }; addUploadedFile(uploadState); // Start upload in background - const uploadIndex = uploadedFiles.length + index; + const uploadIndex = baseIndex + index; uploadFileToConvex(file, uploadIndex); }); }, - [uploadedFiles.length, addUploadedFile], + [uploadedFiles.length, addUploadedFile], );
243-267: Use React event types for paste handler.Use React.ClipboardEvent to match React’s synthetic events.
- const handlePasteEvent = async (event: ClipboardEvent): Promise<boolean> => { + const handlePasteEvent = async ( + event: React.ClipboardEvent, + ): Promise<boolean> => {
290-340: Use React drag event types for DnD handlers.Prefer React.DragEvent for consistency and better typing in JSX.
- const handleDragEnter = useCallback((e: DragEvent) => { + const handleDragEnter = useCallback((e: React.DragEvent) => { @@ - const handleDragLeave = useCallback((e: DragEvent) => { + const handleDragLeave = useCallback((e: React.DragEvent) => { @@ - const handleDragOver = useCallback((e: DragEvent) => { + const handleDragOver = useCallback((e: React.DragEvent) => { @@ - const handleDrop = useCallback( - async (e: DragEvent) => { + const handleDrop = useCallback( + async (e: React.DragEvent) => {types/file.ts (1)
59-59: Optional: export a const source list and derive the union.Improves reuse and guards typos.
-export type FileSource = "upload" | "paste" | "drop"; +export const FILE_SOURCES = ["upload", "paste", "drop"] as const; +export type FileSource = (typeof FILE_SOURCES)[number];lib/utils/file-utils.ts (1)
87-104: Use satisfies to keep the literal type for 'type: "file"'.Minor typing boost; optional.
- return { + return { type: "file" as const, mediaType: uploadedFile.file.type, storageId: uploadedFile.storageId, name: uploadedFile.file.name, size: uploadedFile.file.size, url: uploadedFile.url, - }; + } satisfies FileMessagePart;
📜 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 (7)
app/components/FilePartRenderer.tsx(1 hunks)app/components/FileUploadPreview.tsx(1 hunks)app/contexts/GlobalState.tsx(7 hunks)app/hooks/useFileUpload.ts(1 hunks)lib/db/actions.ts(3 hunks)lib/utils/file-utils.ts(1 hunks)types/file.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- app/components/FilePartRenderer.tsx
- app/components/FileUploadPreview.tsx
- app/contexts/GlobalState.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:
lib/utils/file-utils.tsapp/hooks/useFileUpload.tstypes/file.tslib/db/actions.ts
🧠 Learnings (11)
📚 Learning: 2025-08-29T13:33:09.901Z
Learnt from: CR
PR: hackerai-tech/hackerai#0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-08-29T13:33:09.901Z
Learning: Applies to convex/**/*.ts : Store and handle files as Blob objects; convert to/from Blob when using Convex storage
Applied to files:
lib/utils/file-utils.tsapp/hooks/useFileUpload.ts
📚 Learning: 2025-08-29T13:33:09.901Z
Learnt from: CR
PR: hackerai-tech/hackerai#0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-08-29T13:33:09.901Z
Learning: Applies to convex/**/*.ts : Use ctx.storage.getUrl(fileId) to obtain signed URLs; it returns null if the file does not exist
Applied to files:
lib/utils/file-utils.tsapp/hooks/useFileUpload.tstypes/file.ts
📚 Learning: 2025-08-29T13:33:09.901Z
Learnt from: CR
PR: hackerai-tech/hackerai#0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-08-29T13:33:09.901Z
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/hooks/useFileUpload.tstypes/file.ts
📚 Learning: 2025-08-29T13:33:09.901Z
Learnt from: CR
PR: hackerai-tech/hackerai#0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-08-29T13:33:09.901Z
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/hooks/useFileUpload.tstypes/file.ts
📚 Learning: 2025-08-29T13:33:09.901Z
Learnt from: CR
PR: hackerai-tech/hackerai#0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-08-29T13:33:09.901Z
Learning: Applies to convex/**/*.ts : Minimize calls from actions to queries/mutations to avoid race conditions from splitting transactions
Applied to files:
app/hooks/useFileUpload.ts
📚 Learning: 2025-08-29T13:33:09.901Z
Learnt from: CR
PR: hackerai-tech/hackerai#0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-08-29T13:33:09.901Z
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:
app/hooks/useFileUpload.ts
📚 Learning: 2025-08-29T13:33:09.901Z
Learnt from: CR
PR: hackerai-tech/hackerai#0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-08-29T13:33:09.901Z
Learning: Applies to convex/**/*.ts : Use ctx.runQuery from queries/mutations/actions to call a query
Applied to files:
app/hooks/useFileUpload.ts
📚 Learning: 2025-08-29T13:33:09.901Z
Learnt from: CR
PR: hackerai-tech/hackerai#0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-08-29T13:33:09.901Z
Learning: Applies to convex/**/*.ts : Use query/mutation/action only for public API functions
Applied to files:
app/hooks/useFileUpload.ts
📚 Learning: 2025-08-29T13:33:09.901Z
Learnt from: CR
PR: hackerai-tech/hackerai#0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-08-29T13:33:09.901Z
Learning: Applies to convex/**/*.ts : Use ctx.runMutation from mutations/actions to call a mutation
Applied to files:
app/hooks/useFileUpload.ts
📚 Learning: 2025-08-29T13:33:09.901Z
Learnt from: CR
PR: hackerai-tech/hackerai#0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-08-29T13:33:09.901Z
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:
app/hooks/useFileUpload.ts
📚 Learning: 2025-08-29T13:33:09.901Z
Learnt from: CR
PR: hackerai-tech/hackerai#0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-08-29T13:33:09.901Z
Learning: Applies to convex/**/*.ts : Always include argument and return validators for all Convex functions (query/internalQuery/mutation/internalMutation/action/internalAction)
Applied to files:
app/hooks/useFileUpload.ts
🧬 Code graph analysis (3)
lib/utils/file-utils.ts (2)
convex/fileStorage.ts (2)
generateUploadUrl(8-28)getFileUrls(52-70)types/file.ts (2)
FileMessagePart(1-8)UploadedFileState(10-17)
app/hooks/useFileUpload.ts (5)
app/contexts/GlobalState.tsx (1)
useGlobalState(279-285)convex/fileStorage.ts (2)
generateUploadUrl(8-28)deleteFile(96-111)types/file.ts (3)
FileProcessingResult(52-57)FileSource(59-59)UploadedFileState(10-17)lib/utils/file-utils.ts (4)
MAX_FILES_LIMIT(162-162)validateFile(116-125)uploadSingleFileToConvex(62-82)createFileMessagePartFromUploadedFile(167-182)convex/_generated/dataModel.d.ts (1)
Id(48-49)
lib/db/actions.ts (1)
convex/_generated/dataModel.d.ts (1)
Id(48-49)
| const uploadFileToConvex = async (file: File, uploadIndex: number) => { | ||
| try { | ||
| const storageId = await uploadSingleFileToConvex(file, generateUploadUrl); | ||
|
|
||
| // Fetch the URL immediately after upload using the query | ||
| const url = await convex.query(api.fileStorage.getFileUrl, { | ||
| storageId: storageId as Id<"_storage">, | ||
| }); | ||
|
|
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.
🛠️ Refactor suggestion
Eliminate casts by returning Id<'_storage'> from uploader.
Have uploadSingleFileToConvex return Id<'_storage'> to keep type safety throughout. Update call sites accordingly.
- const url = await convex.query(api.fileStorage.getFileUrl, {
- storageId: storageId as Id<"_storage">,
- });
+ const url = await convex.query(api.fileStorage.getFileUrl, {
+ storageId,
+ });Follow-up: change uploadSingleFileToConvex’s return type to Promise<Id<'_storage'>> (see lib/utils/file-utils.ts suggestion) and UploadedFileState.storageId to Id<'_storage'> (see types/file.ts).
📝 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 uploadFileToConvex = async (file: File, uploadIndex: number) => { | |
| try { | |
| const storageId = await uploadSingleFileToConvex(file, generateUploadUrl); | |
| // Fetch the URL immediately after upload using the query | |
| const url = await convex.query(api.fileStorage.getFileUrl, { | |
| storageId: storageId as Id<"_storage">, | |
| }); | |
| const uploadFileToConvex = async (file: File, uploadIndex: number) => { | |
| try { | |
| const storageId = await uploadSingleFileToConvex(file, generateUploadUrl); | |
| // Fetch the URL immediately after upload using the query | |
| - const url = await convex.query(api.fileStorage.getFileUrl, { | |
| - storageId: storageId as Id<"_storage">, | |
| const url = await convex.query(api.fileStorage.getFileUrl, { | |
| storageId, | |
| }); |
🤖 Prompt for AI Agents
In app/hooks/useFileUpload.ts around lines 177 to 185, the code casts storageId
to Id<'_storage'> after calling uploadSingleFileToConvex; update
uploadSingleFileToConvex to return Promise<Id<'_storage'>> and change all call
sites (including this one) to accept that typed return value and remove the "as
Id<'_storage'>" cast. Also update the UploadedFileState.storageId type to
Id<'_storage'> (per lib/utils/file-utils.ts and types/file.ts suggestions) so
the type flows through without ad-hoc casting.
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: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
convex/schema.ts (1)
29-38: Add composite index to support assistant-message lookup without .filter.messages.ts queries last assistant message using a post-filter; per guidelines, prefer an index. Add an index on ["chat_id","role"].
Apply in this file:
messages: defineTable({ id: v.string(), chat_id: v.string(), role: v.string(), parts: v.array(v.any()), file_ids: v.optional(v.array(v.id("files"))), update_time: v.number(), }) .index("by_message_id", ["id"]) - .index("by_chat_id", ["chat_id"]), + .index("by_chat_id", ["chat_id"]) + .index("by_chat_id_and_role", ["chat_id", "role"]),convex/messages.ts (3)
231-235: Invalid index usage for _id; use ctx.db.get instead.There is no by_id index on messages, and indexing _id is not supported. Fetch by document id directly.
- const message = await ctx.db - .query("messages") - .withIndex("by_id", (q) => - q.eq("_id", args.messageId as Id<"messages">), - ) - .first(); + const message = await ctx.db.get(args.messageId as Id<"messages">);
174-181: Avoid .filter in queries; use the new composite index.Move role filtering into the index to comply with Convex best practices and improve performance.
- 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_and_role", (q) => + q.eq("chat_id", args.chatId).eq("role", "assistant"), + ) + .order("desc") + .first();Note: Requires the schema index suggested in convex/schema.ts.
56-61: Add returns validator to query.Per guidelines, include a returns validator (use v.any() for the paginated result if you don’t want to spell out the shape).
export const getMessagesByChatId = query({ args: { chatId: v.string(), paginationOpts: paginationOptsValidator, }, + returns: v.any(),app/contexts/GlobalState.tsx (1)
210-217: Consider also clearing uploaded files in initializeChat.While
initializeNewChatcorrectly clears uploaded files,initializeChatdoesn't. This could lead to files from a previous chat appearing in a new chat when switching between chats.const initializeChat = useCallback((chatId: string) => { setIsSwitchingChats(true); setCurrentChatId(chatId); setShouldFetchMessages(true); setHasActiveChat(true); setTodos([]); setIsTodoPanelExpanded(false); + setUploadedFiles([]); }, []);
♻️ Duplicate comments (14)
app/components/FilePartRenderer.tsx (2)
19-50: Move FilePreviewCard outside the component to prevent re-creation on each render.Defining the component inside
useMemostill creates a new component definition on mount. This can cause unnecessary re-renders and re-fetches.Move both
FilePreviewCardandConvexFilePartto the module scope:+const FilePreviewCard = memo(function FilePreviewCard({ + partId, + icon, + fileName, + subtitle, +}: { + partId: string; + icon: React.ReactNode; + fileName: string; + subtitle: string; +}) { + return ( + <div + key={partId} + className="p-2 w-full max-w-80 min-w-64 border rounded-lg bg-background" + > + <div className="flex flex-row items-center gap-2"> + <div className="relative h-10 w-10 shrink-0 overflow-hidden rounded-lg bg-[#FF5588] flex items-center justify-center"> + {icon} + </div> + <div className="overflow-hidden"> + <div className="truncate font-semibold text-sm">{fileName}</div> + <div className="text-muted-foreground truncate text-xs"> + {subtitle} + </div> + </div> + </div> + </div> + ); +}); const FilePartRendererComponent = ({ part, partIndex, messageId, totalFileParts = 1, }: FilePartRendererProps) => { const [selectedImage, setSelectedImage] = useState<{ src: string; alt: string; } | null>(null); - // Memoize file preview component to prevent unnecessary re-renders - const FilePreviewCard = useMemo(() => { - const PreviewCard = ({ - // ... component definition - }) => ( - // ... component JSX - ); - PreviewCard.displayName = "FilePreviewCard"; - return PreviewCard; - }, []);
53-127: Move ConvexFilePart outside the component.Same issue as FilePreviewCard - defining it inside the component causes re-creation.
Move
ConvexFilePartto module scope similar to the FilePreviewCard suggestion above.lib/db/actions.ts (2)
57-66: Unsafe cast to Id<"files">[] - update extractFileIdsFromParts to return typed IDs.Following the earlier refactor suggestion for
extractFileIdsFromParts, this cast would become unnecessary.After updating
extractFileIdsFromPartsto returnId<"files">[]:// Extract file IDs from file parts const fileIds = extractFileIdsFromParts(message.parts); return await convex.mutation(api.messages.saveMessage, { serviceKey: process.env.CONVEX_SERVICE_ROLE_KEY!, id: message.id, chatId, role: message.role, parts: message.parts, - fileIds: fileIds.length > 0 ? (fileIds as Id<"files">[]) : undefined, + fileIds: fileIds.length > 0 ? fileIds : undefined, });
177-222: Type fileIdsToFetch as Id<"files">[] and avoid any casts.The function uses loose typing with
string[]and casts toanymultiple times. Follow the coding guidelines to use proper Convex ID types.+import { FilePart } from "@/types/file"; + // Collect all fileIds that need URL fetching - const fileIdsToFetch: string[] = []; + const fileIdsToFetch: Array<Id<"files">> = []; const fileIdToFilePartMap = new Map< - string, + Id<"files">, Array<{ messageIndex: number; partIndex: number }> >(); for ( let messageIndex = 0; messageIndex < updatedMessages.length; messageIndex++ ) { const message = updatedMessages[messageIndex]; if (!message.parts) continue; for (let partIndex = 0; partIndex < message.parts.length; partIndex++) { - const part = message.parts[partIndex] as any; + const part = message.parts[partIndex] as unknown as FilePart; if (part.type === "file") { // If already has HTTP URL, continue - if (part.url && part.url.startsWith("http")) { + if (part.url && /^https?:\/\//i.test(part.url)) { continue; } // Extract fileId that needs URL fetching const fileId = part.fileId; if (fileId) { + const typedFileId = fileId as Id<"files">; - if (!fileIdToFilePartMap.has(fileId)) { - fileIdsToFetch.push(fileId); - fileIdToFilePartMap.set(fileId, []); + if (!fileIdToFilePartMap.has(typedFileId)) { + fileIdsToFetch.push(typedFileId); + fileIdToFilePartMap.set(typedFileId, []); } - fileIdToFilePartMap.get(fileId)!.push({ messageIndex, partIndex }); + fileIdToFilePartMap.get(typedFileId)!.push({ messageIndex, partIndex }); } } } }And update the API call:
const urls = await convex.query(api.fileStorage.getFileUrlsByFileIds, { serviceKey: process.env.CONVEX_SERVICE_ROLE_KEY!, - fileIds: fileIdsToFetch as Id<"files">[], + fileIds: fileIdsToFetch, });types/file.ts (4)
1-8: Use Id<"files"> for fileId instead of plain string.According to the coding guidelines and retrieved learnings, document IDs should use the
Idhelper type.+import type { Id } from "@/convex/_generated/dataModel"; + export interface FileMessagePart { type: "file"; mediaType: string; - fileId: string; // Database file ID for backend operations + fileId: Id<"files">; // Database file ID for backend operations name: string; size: number; url: string; // Always include URL for immediate rendering }
10-18: Use Id<"files"> for fileId in UploadedFileState.Maintain consistency with typed IDs across all interfaces.
export interface UploadedFileState { file: File; uploading: boolean; uploaded: boolean; error?: string; - fileId?: string; // Database file ID for backend operations + fileId?: Id<"files">; // Database file ID for backend operations url?: string; // Store the resolved URL tokens?: number; // Token count for the file }
20-27: Use Id<"files"> for fileId in FilePart.Keep all file ID references consistently typed.
// File part interface for rendering components export interface FilePart { url?: string; - fileId?: string; + fileId?: Id<"files">; name?: string; filename?: string; mediaType?: string; }
29-35: Type messageId with Id<"messages">.The messageId should use the Convex ID type for consistency.
// Props for FilePartRenderer component export interface FilePartRendererProps { part: FilePart; partIndex: number; - messageId: string; + messageId: Id<"messages">; totalFileParts?: number; }lib/chat/chat-processor.ts (1)
148-149: Media detection aligns with the correct payload.The media detection now correctly operates on
messagesWithUrlsafter document content has been added and non-media file parts removed, ensuring accurate model selection.app/hooks/useFileUpload.ts (2)
193-194: Correct usage of typed file ID.The deleteFile call correctly casts the fileId to
Id<"files">, which is the expected type for the Convex mutation.
243-244: Consistent type handling for deleteFile.The type cast to
Id<"files">is properly applied, matching the expected type for the Convex mutation.app/hooks/useChatHandlers.ts (3)
12-12: Consider adding type safety to the sendMessage signature.The
sendMessagefunction usesanytypes which reduces type safety. Consider defining proper types for the message and options parameters.
79-96: Fix incorrect property name in file mapping.The file part mapping uses
filenamebut should usenameto match the expected type. Also consider including thesizeproperty.files: validFiles.length > 0 ? validFiles.map((uploadedFile) => ({ type: "file" as const, - filename: uploadedFile.file.name, + name: uploadedFile.file.name, mediaType: uploadedFile.file.type, + size: uploadedFile.file.size, url: uploadedFile.url!, fileId: uploadedFile.fileId!, })) : undefined,
105-117: Consider preventing empty message submission in error handler.The error handler could send an empty message if there's no text input. Add a check to prevent this.
} catch (error) { console.error("Failed to process files:", error); // Fallback to text-only message if file processing fails - sendMessage( - { text: input }, - { - body: { - mode, - todos, - }, - }, - ); + if (input.trim()) { + sendMessage( + { text: input.trim() }, + { + body: { + mode, + todos, + }, + }, + ); + } }
🧹 Nitpick comments (14)
package.json (1)
18-18: Server-only deps: ensure no accidental client/edge bundling.pdf-parse, langchain, and isbinaryfile should remain server-only. Verify they aren’t imported from client/edge code paths to avoid bundle/compat issues. If needed, guard with dynamic imports and ensure routes using them run on the Node runtime.
Also applies to: 24-24, 40-41, 47-47
convex/schema.ts (1)
40-49: Make file_token_size optional or default to 0.New files may not have tokens at creation time; consider v.optional(v.number()) or initializing to 0 when inserting.
convex/messages.ts (3)
246-251: Unset optional field: confirm undefined is intended to clear file_ids.Convex allows unsetting optional fields via undefined; just confirm this behavior is desired here when replacing with text.
187-199: Nit: rename loop variable to fileId for clarity.These are files table IDs, not storage IDs. Improves readability.
- for (const storageId of lastAssistantMessage.file_ids) { + for (const fileId of lastAssistantMessage.file_ids) { try { - const file = await ctx.db.get(storageId); + const file = await ctx.db.get(fileId); if (file) { await ctx.storage.delete(file.storage_id); await ctx.db.delete(file._id); } } catch (error) { - console.error(`Failed to delete file ${storageId}:`, error); + console.error(`Failed to delete file ${fileId}:`, error);
263-278: Consider parallelizing file cleanup.Deletion is safe to run concurrently per message; Promise.allSettled can reduce latency on many files.
- if (msg.file_ids && msg.file_ids.length > 0) { - for (const fileId of msg.file_ids) { - try { - const file = await ctx.db.get(fileId); - if (file) { - await ctx.storage.delete(file.storage_id); - await ctx.db.delete(file._id); - } - } catch (error) { - console.error(`Failed to delete file ${fileId}:`, error); - // Continue with deletion even if file cleanup fails - } - } - } + if (msg.file_ids && msg.file_ids.length > 0) { + await Promise.allSettled( + msg.file_ids.map(async (fileId) => { + try { + const file = await ctx.db.get(fileId); + if (file) { + await ctx.storage.delete(file.storage_id); + await ctx.db.delete(file._id); + } + } catch (error) { + console.error(`Failed to delete file ${fileId}:`, error); + } + }), + ); + }lib/token-utils.ts (2)
40-46: Avoid double-counting identical file parts in a single message.If the same file appears multiple times in one message, consider counting it once.
- const fileTokenCount = message.parts - .filter((part) => part.type === "file") - .reduce((total, part) => { - const fileId = (part as any).fileId; - return total + (fileId ? fileTokens[fileId] || 0 : 0); - }, 0); + const seen = new Set<string>(); + const fileTokenCount = message.parts + .filter((part) => part.type === "file") + .reduce((total, part) => { + const fileId = (part as any).fileId as string | undefined; + if (!fileId || seen.has(fileId)) return total; + seen.add(fileId); + return total + (fileTokens[fileId] || 0); + }, 0);
32-38: Type the file part to avoid any-casts.Define a FilePart type with fileId: string and use a type guard for part.type === "file".
lib/utils/file-token-utils.ts (1)
4-4: Remove unused import.The
ChatSDKErrorimport is not used anywhere in this file.-import { ChatSDKError } from "../errors";convex/fileActions.ts (2)
79-99: Handle unknown file types more explicitly.The default case tries to process unknown file types, but TypeScript doesn't understand that "unknown" is being passed as
any. Consider making the fallback handling more explicit.default: { + // For unsupported file types, check if binary // Check if the original file is binary before text conversion const blob = file as Blob; const fileBuffer = Buffer.from(await blob.arrayBuffer()); const isBinary = await isBinaryFile(fileBuffer); if (isBinary) { // For binary files, create a single chunk with empty content and 0 tokens return [ { content: "", tokens: 0, }, ]; } else { // For non-binary files, convert to text and process as txt const textDecoder = new TextDecoder("utf-8"); const cleanText = textDecoder.decode(fileBuffer); return await processTxtFile(new Blob([cleanText])); } }
197-200: Avoid passing "unknown" as any to processFile.The code passes
"unknown" as anyto bypass TypeScript checks, which could hide type errors.if (!detectedType) { - // Use default processing for unknown file types - const chunks = await processFile(file, { - fileType: "unknown" as any, - prepend, - }); + // Process unknown files directly without going through processFile + const blob = file as Blob; + const fileBuffer = Buffer.from(await blob.arrayBuffer()); + const isBinary = await isBinaryFile(fileBuffer); + + const chunks = isBinary + ? [{ content: "", tokens: 0 }] + : await processTxtFile(blob); + validateTokenLimit(chunks, fileName || "unknown"); return chunks; }app/hooks/useFileUpload.ts (1)
127-144: Consider memoizing uploadFileToConvex to avoid stale closures.The
uploadFileToConvexfunction is defined inline but references external dependencies. Consider wrapping it withuseCallbackto prevent potential stale closure issues.- const uploadFileToConvex = async (file: File, uploadIndex: number) => { + const uploadFileToConvex = useCallback(async (file: File, uploadIndex: number) => { try { const { fileId, url, tokens } = await uploadSingleFileToConvex( file, generateUploadUrl, saveFile, ); // ... rest of the function } catch (error) { // ... error handling } - }; + }, [generateUploadUrl, saveFile, getTotalTokens, deleteFile, removeUploadedFile, updateUploadedFile]);Then update the dependency array of
startFileUploads:- [uploadedFiles.length, addUploadedFile], + [uploadedFiles.length, addUploadedFile, uploadFileToConvex],convex/fileStorage.ts (3)
14-27: Counting all files can be O(n); consider a streaming early-exit helper for the 100 capThis query collects all rows, which can be heavy for large accounts. Either:
- keep this exact-count API but introduce a separate internalQuery that returns a boolean and early-exits at 100, or
- switch to streaming and stop after 101 to bound work.
Example early-exit variant:
export const countUserFiles = internalQuery({ args: { userId: v.string(), }, returns: v.number(), handler: async (ctx, args) => { - const files = await ctx.db - .query("files") - .withIndex("by_user_id", (q) => q.eq("user_id", args.userId)) - .collect(); - - return files.length; + let count = 0; + const q = ctx.db + .query("files") + .withIndex("by_user_id", (q) => q.eq("user_id", args.userId)); + for await (const _ of q) { + count++; + if (count >= 101) break; // bound work + } + return count; }, });
98-126: Delete flow looks good; ownership validated and null return matches schema
You verify ownership, delete from storage, then DB. Consider wrapping storage delete in a try/catch to avoid hard failures on already-deleted blobs, depending on desired idempotency.
206-229: Persist-time validation and normalization
Add lightweight validation to prevent bad data:handler: async (ctx, args) => { + if (args.size < 0 || args.fileTokenSize < 0) { + throw new Error("Invalid file metadata: size and token size must be non-negative"); + } + // Optional: normalize/trim content to a bounded size if present + const MAX_CONTENT_BYTES = 1_000_000; // ~1MB, adjust as needed + const content = + args.content && args.content.length > MAX_CONTENT_BYTES + ? args.content.slice(0, MAX_CONTENT_BYTES) + : args.content; @@ const fileId = await ctx.db.insert("files", { @@ - content: args.content, + content, });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files 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 (18)
app/api/chat/route.ts(3 hunks)app/components/ChatInput.tsx(4 hunks)app/components/FilePartRenderer.tsx(1 hunks)app/contexts/GlobalState.tsx(7 hunks)app/hooks/useChatHandlers.ts(4 hunks)app/hooks/useFileUpload.ts(1 hunks)convex/chats.ts(1 hunks)convex/fileActions.ts(1 hunks)convex/fileStorage.ts(1 hunks)convex/messages.ts(8 hunks)convex/schema.ts(1 hunks)lib/chat/chat-processor.ts(4 hunks)lib/db/actions.ts(3 hunks)lib/token-utils.ts(3 hunks)lib/utils/file-token-utils.ts(1 hunks)lib/utils/file-utils.ts(1 hunks)package.json(2 hunks)types/file.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- convex/chats.ts
- app/components/ChatInput.tsx
🧰 Additional context used
📓 Path-based instructions (4)
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/fileActions.tsconvex/messages.tsconvex/schema.tsconvex/fileStorage.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/fileActions.tsapp/hooks/useChatHandlers.tsapp/hooks/useFileUpload.tslib/utils/file-token-utils.tslib/chat/chat-processor.tsapp/api/chat/route.tslib/utils/file-utils.tslib/token-utils.tstypes/file.tsconvex/messages.tsapp/components/FilePartRenderer.tsxconvex/schema.tsapp/contexts/GlobalState.tsxconvex/fileStorage.tslib/db/actions.ts
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/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 (13)
📓 Common learnings
Learnt from: CR
PR: hackerai-tech/hackerai#0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-08-29T13:33:09.901Z
Learning: Applies to convex/**/*.ts : Store and handle files as Blob objects; convert to/from Blob when using Convex storage
📚 Learning: 2025-08-29T13:33:09.901Z
Learnt from: CR
PR: hackerai-tech/hackerai#0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-08-29T13:33:09.901Z
Learning: Applies to convex/**/*.ts : Store and handle files as Blob objects; convert to/from Blob when using Convex storage
Applied to files:
convex/fileActions.tsapp/hooks/useFileUpload.tslib/utils/file-utils.tstypes/file.ts
📚 Learning: 2025-08-29T13:33:09.901Z
Learnt from: CR
PR: hackerai-tech/hackerai#0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-08-29T13:33:09.901Z
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/hooks/useFileUpload.tstypes/file.ts
📚 Learning: 2025-08-29T13:33:09.901Z
Learnt from: CR
PR: hackerai-tech/hackerai#0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-08-29T13:33:09.901Z
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/hooks/useFileUpload.tstypes/file.ts
📚 Learning: 2025-08-29T13:33:09.901Z
Learnt from: CR
PR: hackerai-tech/hackerai#0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-08-29T13:33:09.901Z
Learning: Applies to convex/**/*.ts : Use ctx.storage.getUrl(fileId) to obtain signed URLs; it returns null if the file does not exist
Applied to files:
app/hooks/useFileUpload.tslib/utils/file-utils.tstypes/file.tsconvex/messages.tsapp/components/FilePartRenderer.tsxconvex/fileStorage.ts
📚 Learning: 2025-08-29T13:33:09.901Z
Learnt from: CR
PR: hackerai-tech/hackerai#0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-08-29T13:33:09.901Z
Learning: Applies to convex/**/*.ts : Minimize calls from actions to queries/mutations to avoid race conditions from splitting transactions
Applied to files:
app/hooks/useFileUpload.ts
📚 Learning: 2025-08-29T13:33:09.901Z
Learnt from: CR
PR: hackerai-tech/hackerai#0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-08-29T13:33:09.901Z
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:
app/hooks/useFileUpload.ts
📚 Learning: 2025-08-29T13:33:09.901Z
Learnt from: CR
PR: hackerai-tech/hackerai#0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-08-29T13:33:09.901Z
Learning: Applies to convex/**/*.ts : Use ctx.runQuery from queries/mutations/actions to call a query
Applied to files:
app/hooks/useFileUpload.ts
📚 Learning: 2025-08-29T13:33:09.901Z
Learnt from: CR
PR: hackerai-tech/hackerai#0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-08-29T13:33:09.901Z
Learning: Applies to convex/**/*.ts : Use query/mutation/action only for public API functions
Applied to files:
app/hooks/useFileUpload.ts
📚 Learning: 2025-08-29T13:33:09.901Z
Learnt from: CR
PR: hackerai-tech/hackerai#0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-08-29T13:33:09.901Z
Learning: Applies to convex/**/*.ts : Use ctx.runMutation from mutations/actions to call a mutation
Applied to files:
app/hooks/useFileUpload.ts
📚 Learning: 2025-08-29T13:33:09.901Z
Learnt from: CR
PR: hackerai-tech/hackerai#0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-08-29T13:33:09.901Z
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:
app/hooks/useFileUpload.ts
📚 Learning: 2025-08-29T13:33:09.901Z
Learnt from: CR
PR: hackerai-tech/hackerai#0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-08-29T13:33:09.901Z
Learning: Applies to convex/**/*.ts : Always include argument and return validators for all Convex functions (query/internalQuery/mutation/internalMutation/action/internalAction)
Applied to files:
app/hooks/useFileUpload.ts
📚 Learning: 2025-08-29T13:33:09.901Z
Learnt from: CR
PR: hackerai-tech/hackerai#0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-08-29T13:33:09.901Z
Learning: Applies to convex/**/*.ts : Queries do not support .delete(); collect results and call ctx.db.delete(_id) per document
Applied to files:
convex/messages.ts
🧬 Code graph analysis (11)
convex/fileActions.ts (2)
lib/utils/file-utils.ts (1)
isImageFile(115-117)types/file.ts (3)
FileItemChunk(63-66)ProcessFileOptions(71-74)SupportedFileType(69-69)
app/hooks/useChatHandlers.ts (1)
app/contexts/GlobalState.tsx (1)
useGlobalState(297-303)
app/hooks/useFileUpload.ts (6)
app/contexts/GlobalState.tsx (1)
useGlobalState(297-303)convex/fileStorage.ts (2)
generateUploadUrl(32-66)deleteFile(98-126)convex/fileActions.ts (1)
saveFile(356-449)types/file.ts (2)
FileProcessingResult(53-58)FileSource(60-60)lib/utils/file-utils.ts (5)
MAX_FILES_LIMIT(122-122)validateFile(76-85)uploadSingleFileToConvex(7-42)MAX_TOTAL_TOKENS(127-127)createFileMessagePartFromUploadedFile(132-147)convex/_generated/dataModel.d.ts (1)
Id(48-49)
lib/utils/file-token-utils.ts (2)
convex/_generated/dataModel.d.ts (1)
Id(48-49)lib/token-utils.ts (1)
truncateMessagesToTokenLimit(53-77)
lib/chat/chat-processor.ts (2)
lib/db/actions.ts (2)
transformStorageIdsToUrls(170-249)getDocumentContentForFiles(256-295)lib/utils/file-token-utils.ts (1)
extractAllFileIdsFromMessages(72-83)
app/api/chat/route.ts (4)
lib/utils/file-token-utils.ts (1)
truncateMessagesWithFileTokens(92-104)lib/db/actions.ts (1)
handleInitialChatAndUserMessage(73-136)lib/chat/chat-processor.ts (2)
processChatMessages(122-176)hasMediaFiles(13-31)lib/ai/providers.ts (1)
myProvider(4-16)
lib/utils/file-utils.ts (4)
convex/fileStorage.ts (1)
generateUploadUrl(32-66)convex/fileActions.ts (1)
saveFile(356-449)convex/_generated/dataModel.d.ts (1)
Id(48-49)types/file.ts (2)
UploadedFileState(10-18)FileMessagePart(1-8)
app/components/FilePartRenderer.tsx (2)
types/file.ts (2)
FilePartRendererProps(30-35)FilePart(21-27)app/components/ImageViewer.tsx (1)
ImageViewer(11-152)
app/contexts/GlobalState.tsx (1)
types/file.ts (1)
UploadedFileState(10-18)
convex/fileStorage.ts (1)
convex/chats.ts (1)
validateServiceKey(6-10)
lib/db/actions.ts (2)
lib/utils/file-token-utils.ts (1)
extractFileIdsFromParts(18-33)convex/_generated/dataModel.d.ts (1)
Id(48-49)
🔇 Additional comments (18)
lib/token-utils.ts (1)
29-49: Token accounting extended to include file parts — looks good.app/api/chat/route.ts (4)
66-71: Verify handleInitialChatAndUserMessage signature.You’re passing UIMessage[]; earlier versions expected {id, parts}[]. Ensure the function now accepts UIMessage[]. If not, adapt:
- const { isNewChat } = await handleInitialChatAndUserMessage({ + const { isNewChat } = await handleInitialChatAndUserMessage({ chatId, userId, - messages: truncatedMessages, + messages: truncatedMessages.map((m) => ({ id: m.id, parts: m.parts })), regenerate, });
75-82: Good: media-aware processing flow.Passing truncatedMessages into processChatMessages and branching model selection off hasMediaFiles is a clean separation.
103-110: Model selection wiring LGTM.Selecting vision-model when media files are present and using processedMessages for the final call is correct.
158-169: Persisting streamed messages: ensure fileIds are included.Confirm that saveMessage in lib/db/actions.ts extracts and forwards fileIds so they’re stored in messages.file_ids.
lib/chat/chat-processor.ts (3)
13-31: LGTM! Well-documented media detection helper.The function correctly checks for images and PDFs by inspecting media types in file parts from user messages.
72-117: LGTM! Clean implementation for document content injection.The function properly removes non-media file parts and prepends document content to the first user message, maintaining proper part structure.
133-135: Error handling in transformStorageIdsToUrls is implemented correctly. The function wraps the URL fetch and update logic in a try/catch, logs any errors, and returns the original messages on failure. No further changes needed.app/hooks/useFileUpload.ts (2)
3-4: Import structure looks good.The imports correctly use the Convex hooks and include the necessary Id type from the generated data model.
181-185: LGTM! Clean integration with the saveFile action.The implementation correctly passes the saveFile action to uploadSingleFileToConvex and handles the returned file metadata properly.
Previous concerns about type casting have been addressed by updating the function signatures to work with the proper types.
lib/utils/file-utils.ts (3)
7-42: LGTM! Clean implementation with proper error handling.The function correctly uploads files to Convex storage and handles the saveFile action to obtain file metadata. The error messages are informative and the flow is clear.
47-64: LGTM! Proper validation before creating message parts.The function correctly validates that both fileId and url are present before creating the message part, ensuring data integrity.
132-147: LGTM! Complementary helper with null-safe handling.This helper complements
createFileMessagePartby returning null instead of throwing when prerequisites aren't met, making it suitable for filtering operations.app/hooks/useChatHandlers.ts (1)
55-61: Add proper async/await handling.Good addition of the upload check, and the async signature is properly added to support it.
app/contexts/GlobalState.tsx (2)
175-179: LGTM! Clean token calculation implementation.The function correctly calculates total tokens from uploaded files using reduce with proper null checking.
184-199: LGTM! Well-structured CRUD operations for file state.The file state management functions are properly memoized with useCallback and follow React best practices for state updates.
convex/fileStorage.ts (2)
1-10: Imports are consistent with Convex guidelines
Using internalQuery/internalMutation for private endpoints and storage APIs is aligned with the rules. Keep using ctx.storage.getUrl(storageId); it returns null if the blob is missing.
71-93: EnsureCONVEX_SERVICE_ROLE_KEYis set in all environments
validateServiceKey is invoked across your Convex queries (fileStorage.ts, messages.ts, chats.ts) and in lib/db/actions.ts—if the environment variable is unset, service-key–based authorization is effectively disabled.
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: 6
🧹 Nitpick comments (5)
lib/utils/file-transform-utils.ts (5)
41-44: Avoid JSON stringify clone for potential non-serializable parts
JSON.parse(JSON.stringify(...))can drop Blob/Date/undefined. PreferstructuredClone.-const updatedMessages = JSON.parse(JSON.stringify(messages)) as UIMessage[]; +const updatedMessages = structuredClone(messages) as UIMessage[];
99-114: Process conversions concurrently (with a cap) to reduce latencyThe loop awaits each file sequentially; this will be slow with multiple PDFs.
-// Process each file -for (const [fileId, file] of filesToProcess) { - if (!file.url) continue; - - const finalUrl = file.mediaType === "application/pdf" - ? await convertPdfToBase64Url(file.url, fileId) - : file.url; - - // Update all file parts with the final URL - file.positions.forEach(({ messageIndex, partIndex }) => { - const filePart = updatedMessages[messageIndex].parts![partIndex] as any; - if (filePart.type === "file") { - filePart.url = finalUrl; - } - }); -} +// Process each file concurrently (simple cap = 4) +const entries = Array.from(filesToProcess.entries()); +const CONCURRENCY = 4; +for (let i = 0; i < entries.length; i += CONCURRENCY) { + const slice = entries.slice(i, i + CONCURRENCY); + await Promise.all( + slice.map(async ([fileId, file]) => { + if (!file.url) return; + const finalUrl = + file.mediaType === "application/pdf" + ? await convertPdfToBase64Url(file.url!, fileId) + : file.url!; + file.positions.forEach(({ messageIndex, partIndex }) => { + const filePart = updatedMessages[messageIndex].parts![partIndex] as any; + if (filePart.type === "file") filePart.url = finalUrl; + }); + }) + ); +}
129-140: Reduce verbose logs; avoid logging large base64 lengthsLog noise in prod; lengths can reveal file sizes.
-console.log(`Converting PDF to base64 for fileId: ${fileId}`); +if (process.env.NODE_ENV !== "production") { + console.log(`Converting PDF to base64 for fileId: ${fileId}`); +} - if (base64Data) { - console.log(`Successfully converted PDF to base64, length: ${base64Data.length}`); + if (base64Data) { + if (process.env.NODE_ENV !== "production") { + console.log(`Successfully converted PDF to base64`); + } return `data:application/pdf;base64,${base64Data}`; } else { - console.log(`Failed to convert PDF to base64 for fileId: ${fileId}`); + if (process.env.NODE_ENV !== "production") { + console.log(`Failed to convert PDF to base64 for fileId: ${fileId}`); + } return url; }
171-181: Return type alignment for fileIdsWithContentIf you adopt Id<'files'> for inputs, also emit Id-typed IDs.
-const fileIdsWithContent = documentsWithContent.map(file => file.id); +const fileIdsWithContent: Array<Id<"files">> = documentsWithContent.map(file => file.id as Id<"files">);
78-121: Fallback behavior on errors: preserve original vs partially updatedCurrent catch returns original
messages, discarding any earlier updates. Consider returningupdatedMessagesif partial progress completed.- } catch (error) { + } catch (error) { console.error("Failed to transform file URLs:", error); - return messages; + return updatedMessages; }
📜 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)
lib/chat/chat-processor.ts(4 hunks)lib/db/actions.ts(3 hunks)lib/utils/file-transform-utils.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- lib/db/actions.ts
- lib/chat/chat-processor.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:
lib/utils/file-transform-utils.ts
🧠 Learnings (3)
📓 Common learnings
Learnt from: CR
PR: hackerai-tech/hackerai#0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-08-29T13:33:09.901Z
Learning: Applies to convex/**/*.ts : Store and handle files as Blob objects; convert to/from Blob when using Convex storage
📚 Learning: 2025-08-29T13:33:09.901Z
Learnt from: CR
PR: hackerai-tech/hackerai#0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-08-29T13:33:09.901Z
Learning: Applies to convex/**/*.ts : Use ctx.storage.getUrl(fileId) to obtain signed URLs; it returns null if the file does not exist
Applied to files:
lib/utils/file-transform-utils.ts
📚 Learning: 2025-08-29T13:33:09.901Z
Learnt from: CR
PR: hackerai-tech/hackerai#0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-08-29T13:33:09.901Z
Learning: Applies to convex/**/*.ts : Store and handle files as Blob objects; convert to/from Blob when using Convex storage
Applied to files:
lib/utils/file-transform-utils.ts
🧬 Code graph analysis (1)
lib/utils/file-transform-utils.ts (1)
convex/_generated/dataModel.d.ts (1)
Id(48-49)
🔇 Additional comments (3)
lib/utils/file-transform-utils.ts (3)
31-39: General: good structure and defensive returnsEarly exits and deep copy make the function safe to call repeatedly.
91-97: Index-based mapping is safe given current API contract
ThegetFileUrlsByFileIdsConvex query usesargs.fileIds.map(...)andPromise.allto return an array of URLs (ornull) in the exact order of the inputfileIds, so indexing intofetchedUrlsaligns correctly with eachfileId.
1-9: Remove CONVEX_SERVICE_ROLE_KEY guard—unused ConvexHttpClient fromconvex/browseronly requires the public URL; the service role key isn’t consumed here and shouldn’t be exposed. The existingimport "server-only"already prevents this module from running in Edge or client bundles.Likely an incorrect or invalid review comment.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation