Daily branch 2025 12 30 #151
Conversation
- Remove hasFileContent restriction in MessageActions to allow editing messages with files - Refactor MessageEditor to display file previews with remove buttons - Add EditableFile interface for file metadata during editing - Update Messages.tsx to extract and pass file parts to MessageEditor - Update useChatHandlers to pass remaining file IDs on edit - Update regenerateWithNewContent mutation to accept optional fileIds parameter - Properly delete removed files from S3/Convex storage and update aggregates - Fix state reset bug by removing useEffect that synced with initialFiles prop
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughEditor now supports attached files (preview/remove) and returns remaining file IDs on save; front-end propagates remainingFileIds through edit/regenerate handlers to backend, which preserves specified files and deletes others during regeneration. Image validation and provider mappings were also updated. Changes
Sequence DiagramsequenceDiagram
actor User
participant Editor as MessageEditor (UI)
participant Messages as Messages Component
participant Handlers as useChatHandlers
participant Convex as Convex Backend
participant Storage as File Storage
User->>Editor: Open editor with message files
User->>Editor: Remove or keep files, edit text
User->>Editor: Click Save
Editor->>Editor: Compute remainingFileIds
Editor->>Messages: onSave(newContent, remainingFileIds)
Messages->>Handlers: handleEditMessage(msgId, newContent, remainingFileIds)
Note over Handlers: buildUpdatedParts merges text + preserved file parts
Handlers->>Convex: regenerateWithNewContent(msgId, newContent, fileIds=remainingFileIds)
rect `#E6F7FF`
Note over Convex: Preserve requested files, delete others
Convex->>Convex: Determine filesToDelete (not in fileIds)
Convex->>Storage: Delete removed files (S3/remote) — handle errors
Convex->>Convex: Update message.parts and message.file_ids
end
Convex-->>Handlers: return updated message
Handlers-->>Messages: update local state
Messages-->>User: render edited message
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120+ minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
convex/messages.ts (1)
958-958: Consider usingv.array(v.id("files"))for type consistency.The
fileIdsparameter usesv.array(v.string())while other file ID validators in this file usev.id("files")(e.g., line 106). This inconsistency may cause type confusion.However, if the frontend passes plain strings (from
part.fileId), this is acceptable. As per coding guidelines, preferv.id(tableName)for document ID validation when possible.🔎 Proposed fix
- fileIds: v.optional(v.array(v.string())), + fileIds: v.optional(v.array(v.id("files"))),This would require the frontend to cast the fileIds appropriately, but provides better type safety.
app/hooks/useChatHandlers.ts (2)
408-412: Incorrect type cast:messageIdshould remain a string.The mutation
regenerateWithNewContentexpectsmessageId: v.string()(see convex/messages.ts line 956), notId<"messages">. The cast is misleading but harmless since strings pass through.🔎 Proposed fix
await regenerateWithNewContent({ - messageId: messageId as Id<"messages">, + messageId, newContent, fileIds: remainingFileIds, });
485-500: Duplicate code: reusebuildUpdatedPartshelper.The logic at lines 487-496 duplicates
buildUpdatedPartsdefined at lines 419-437. Consider reusing the helper for consistency and maintainability.🔎 Proposed fix
// For temporary chats, send messages up to and including the edited message const messagesUpToEdit = messages.slice(0, editedMessageIndex + 1); const editedMessage = messages[editedMessageIndex]; - - // Build updated parts for the edited message - const updatedParts: any[] = []; - if (newContent.trim()) { - updatedParts.push({ type: "text", text: newContent }); - } - if (remainingFileIds && remainingFileIds.length > 0) { - const remainingFileParts = editedMessage.parts.filter( - (part: any) => part.type === "file" && part.fileId && remainingFileIds.includes(part.fileId) - ); - updatedParts.push(...remainingFileParts); - } messagesUpToEdit[editedMessageIndex] = { ...editedMessage, - parts: updatedParts, + parts: buildUpdatedParts(editedMessage.parts), };
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
app/components/MessageActions.tsxapp/components/MessageEditor.tsxapp/components/Messages.tsxapp/hooks/useChatHandlers.tsconvex/messages.ts
🧰 Additional context used
📓 Path-based instructions (1)
convex/**/*.{ts,js}
📄 CodeRabbit inference engine (.cursor/rules/convex_rules.mdc)
convex/**/*.{ts,js}: ALWAYS use the new function syntax for Convex functions withquery,mutation,actiondecorators that includeargs,returns, andhandlerproperties
Use array validators withv.array()to validate array arguments and schema fields
Use discriminated union types withv.union()andv.literal()for schema validation
Always use thev.null()validator when a function returns a null value
Usev.id(tableName)validator for document ID validation andId<'tableName'>TypeScript type for ID fields
v.bigint()is deprecated; usev.int64()instead for representing signed 64-bit integers
Usev.record()for defining record types;v.map()andv.set()are not supported
Index fields must be queried in the same order they are defined; create separate indexes if you need different query orders
UseinternalQuery,internalMutation, andinternalActionfor private functions that should not be part of the public API
Usequery,mutation, andactionfor public functions that are exposed to the public API
ALWAYS include argument and return validators for all Convex functions includingquery,internalQuery,mutation,internalMutation,action, andinternalAction
Functions that don't return anything should includereturns: v.null()as the output validator
Functions implicitly returnnullin JavaScript if they have no return statement
Usectx.runQueryto call a query from a query, mutation, or action
Usectx.runMutationto call a mutation from a mutation or action
Usectx.runActionto call an action from an action
Only call an action from another action if you need to cross runtimes (e.g., V8 to Node); otherwise extract shared code into a helper async function
Use as few calls from actions to queries and mutations as possible to avoid race conditions from split transaction logic
Pass aFunctionReferencetoctx.runQuery,ctx.runMutation, orctx.runAction; do not pass the callee function directly
When usi...
Files:
convex/messages.ts
🧠 Learnings (1)
📚 Learning: 2025-09-01T12:44:12.626Z
Learnt from: RostyslavManko
Repo: hackerai-tech/hackerai PR: 17
File: convex/fileStorage.ts:131-149
Timestamp: 2025-09-01T12:44:12.626Z
Learning: In convex/fileStorage.ts, serviceKey parameters are made optional (v.optional(v.string())) for queries like getFileTokensByFileIds to simplify local development workflow - developers don't need to configure service keys in the Convex dashboard when working locally. In production, the serviceKey is always provided, ensuring security. This pattern balances developer experience with production security requirements.
Applied to files:
convex/messages.ts
🧬 Code graph analysis (2)
convex/messages.ts (4)
convex/_generated/server.js (2)
mutation(49-49)mutation(49-49)convex/_generated/dataModel.d.ts (1)
Id(48-49)convex/_generated/api.js (2)
internal(22-22)internal(22-22)convex/fileAggregate.ts (1)
fileCountAggregate(13-21)
app/components/Messages.tsx (1)
app/components/MessageEditor.tsx (1)
EditableFile(13-18)
🔇 Additional comments (11)
app/components/MessageActions.tsx (1)
133-134: LGTM! Edit button now available for messages with file attachments.This change correctly enables editing for user messages regardless of whether they contain files, aligning with the new file attachment editing feature.
Note: The
hasFileContentprop (line 31, 55) appears to no longer affect edit button visibility. Consider removing it if unused elsewhere to avoid confusion.convex/messages.ts (2)
1036-1055: LGTM! Parts building logic handles edge cases well.The implementation correctly:
- Preserves text content followed by remaining file parts
- Falls back to text-only part if
newPartsis empty (line 1051)- Clears
file_idswhen no files remain (line 1053)
1012-1034: Good error isolation for file deletions.Each file deletion is wrapped in try/catch, ensuring one failed deletion doesn't block others. The pattern is consistent with
deleteLastAssistantMessage(lines 502-528).app/hooks/useChatHandlers.ts (1)
383-384: LGTM! Extended signature for file retention during edits.The
remainingFileIdsparameter correctly threads through the edit flow, enabling selective file preservation during message regeneration.app/components/Messages.tsx (2)
326-336: LGTM! File parts correctly mapped to EditableFile format.The mapping appropriately:
- Filters for valid file parts with
fileId- Extracts required properties (
fileId,name,mediaType)- Falls back to cached URL when
filePart.urlis unavailable (line 334)
135-149: LGTM! Callback correctly passes remaining file IDs.The signature change and forwarding to
onEditMessageproperly integrates with the editing flow.app/components/MessageEditor.tsx (5)
13-18: LGTM! Clean interface definition for editable files.The
EditableFileinterface provides a clear contract for file data needed during editing, with optional fields for flexibility.
34-37: Good pattern: one-time state initialization.Initializing state from props once (without syncing on updates) correctly preserves user edits during parent re-renders. The comment clarifies this intentional behavior.
58-60: Verify: Token validation excludes existing files.Token counting only considers text content (
countInputTokens(trimmedContent, [])), not the files being retained. This may be intentional since files are already uploaded and counted, but verify this aligns with the intended behavior for edited messages with preserved attachments.
88-147: LGTM! Clean file preview UI with removal capability.The implementation correctly:
- Differentiates image vs document rendering
- Provides accessible remove buttons with appropriate labels
- Handles long filenames with truncation
- Uses appropriate Next.js Image props for dynamic URLs
52-72: LGTM! Save validation correctly handles content-or-files requirement.The validation ensures either text content or files must be present, and
remainingFileIdsis correctly computed from the current files state.
- Add validateImageFile() using createImageBitmap API to detect corrupt images - Only validate LLM-supported formats (PNG, JPEG, WebP, GIF) - Skip validation for SVG and other non-AI image formats - Reject corrupt images before upload with clear error message
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lib/api/chat-handler.ts (1)
537-550: Add an explicit role check before applying metrics to ensure they're only included for assistant messages.The
saveMessagecall applies generation metrics (generationTimeMs, finishReason, usage, regenerationCount) to all messages in the loop without filtering by role. Although themessagesarray typically contains only assistant responses from the SDK, there's no explicit role check to guarantee this. The comment indicates metrics should only be applied to assistant messages—add a condition likeif (message.role === "assistant")around the metrics properties to enforce this intent.
🧹 Nitpick comments (3)
app/components/Messages.tsx (1)
330-346: Consider strengthening the name fallback.The
namefield is required inEditableFile(perapp/components/MessageEditor.tsx, lines 12-17), but the fallback chainfilePart.name || filePart.filename || "File"could technically produceundefinedif both properties are falsy but not explicitly checked. The final"File"fallback should handle this, but the type assertion masks potential issues.🔎 Suggested safer mapping
.map((part) => { const filePart = part as any; return { fileId: filePart.fileId as string, - name: - filePart.name || filePart.filename || "File", + name: filePart.name ?? filePart.filename ?? "File", mediaType: filePart.mediaType, url: filePart.url || getCachedUrl(filePart.fileId as string), } as EditableFile; })}Using nullish coalescing (
??) is more precise for this fallback pattern, as it only falls back onnull/undefinedrather than falsy values like empty strings.app/hooks/useChatHandlers.ts (1)
429-450: Refactor: Extract duplicate parts-building logic.The
buildUpdatedPartshelper (lines 429-450) and the inline logic for temporary chats (lines 498-512) are functionally identical. Consider reusingbuildUpdatedPartsin both code paths to reduce duplication and ensure consistency.🔎 Suggested refactor
// For temporary chats, send messages up to and including the edited message const messagesUpToEdit = messages.slice(0, editedMessageIndex + 1); - const editedMessage = messages[editedMessageIndex]; - - // Build updated parts for the edited message - const updatedParts: any[] = []; - if (newContent.trim()) { - updatedParts.push({ type: "text", text: newContent }); - } - if (remainingFileIds && remainingFileIds.length > 0) { - const remainingFileParts = editedMessage.parts.filter( - (part: any) => - part.type === "file" && - part.fileId && - remainingFileIds.includes(part.fileId), - ); - updatedParts.push(...remainingFileParts); - } messagesUpToEdit[editedMessageIndex] = { - ...editedMessage, - parts: updatedParts, + ...messages[editedMessageIndex], + parts: buildUpdatedParts(messages[editedMessageIndex].parts), };Also applies to: 498-516
convex/messages.ts (1)
1069-1077: Consider using Set for file part filtering.The
.some()call on line 1074 performs O(n) lookup for each file part. Since you already have thekeepSetfrom line 1028, consider reusing it here for consistency and efficiency.🔎 Suggested optimization
// Keep file parts for remaining files if (newFileIds && newFileIds.length > 0) { const existingFileParts = message.parts.filter( (part: any) => part.type === "file" && part.fileId && - newFileIds!.some((id) => id === part.fileId), + keepSet.has(part.fileId), ); newParts.push(...existingFileParts); }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
app/components/MessageEditor.tsxapp/components/Messages.tsxapp/hooks/useChatHandlers.tsconvex/messages.tsconvex/schema.tslib/ai/tools/index.tslib/api/chat-handler.tslib/db/actions.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- lib/ai/tools/index.ts
🧰 Additional context used
📓 Path-based instructions (2)
convex/**/*.{ts,js}
📄 CodeRabbit inference engine (.cursor/rules/convex_rules.mdc)
convex/**/*.{ts,js}: ALWAYS use the new function syntax for Convex functions withquery,mutation,actiondecorators that includeargs,returns, andhandlerproperties
Use array validators withv.array()to validate array arguments and schema fields
Use discriminated union types withv.union()andv.literal()for schema validation
Always use thev.null()validator when a function returns a null value
Usev.id(tableName)validator for document ID validation andId<'tableName'>TypeScript type for ID fields
v.bigint()is deprecated; usev.int64()instead for representing signed 64-bit integers
Usev.record()for defining record types;v.map()andv.set()are not supported
Index fields must be queried in the same order they are defined; create separate indexes if you need different query orders
UseinternalQuery,internalMutation, andinternalActionfor private functions that should not be part of the public API
Usequery,mutation, andactionfor public functions that are exposed to the public API
ALWAYS include argument and return validators for all Convex functions includingquery,internalQuery,mutation,internalMutation,action, andinternalAction
Functions that don't return anything should includereturns: v.null()as the output validator
Functions implicitly returnnullin JavaScript if they have no return statement
Usectx.runQueryto call a query from a query, mutation, or action
Usectx.runMutationto call a mutation from a mutation or action
Usectx.runActionto call an action from an action
Only call an action from another action if you need to cross runtimes (e.g., V8 to Node); otherwise extract shared code into a helper async function
Use as few calls from actions to queries and mutations as possible to avoid race conditions from split transaction logic
Pass aFunctionReferencetoctx.runQuery,ctx.runMutation, orctx.runAction; do not pass the callee function directly
When usi...
Files:
convex/schema.tsconvex/messages.ts
convex/schema.ts
📄 CodeRabbit inference engine (.cursor/rules/convex_rules.mdc)
convex/schema.ts: Always define schema inconvex/schema.tsusingdefineSchemaanddefineTableimported fromconvex/server
System fields_creationTime(v.number()) and_id(v.id(tableName)) are automatically added to all documents; do not redefine them
Always include all index fields in the index name; for example, an index on ['field1', 'field2'] should be named 'by_field1_and_field2'
Files:
convex/schema.ts
🧠 Learnings (2)
📚 Learning: 2025-12-24T18:09:08.574Z
Learnt from: CR
Repo: hackerai-tech/hackerai PR: 0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-12-24T18:09:08.574Z
Learning: Applies to convex/schema.ts : System fields `_creationTime` (v.number()) and `_id` (v.id(tableName)) are automatically added to all documents; do not redefine them
Applied to files:
convex/schema.ts
📚 Learning: 2025-12-24T18:09:08.574Z
Learnt from: CR
Repo: hackerai-tech/hackerai PR: 0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-12-24T18:09:08.574Z
Learning: Applies to convex/schema.ts : Always define schema in `convex/schema.ts` using `defineSchema` and `defineTable` imported from `convex/server`
Applied to files:
convex/schema.ts
🧬 Code graph analysis (4)
app/hooks/useChatHandlers.ts (1)
convex/messages.ts (1)
deleteLastAssistantMessage(464-578)
app/components/MessageEditor.tsx (1)
app/contexts/GlobalState.tsx (1)
useGlobalState(752-758)
app/components/Messages.tsx (1)
app/components/MessageEditor.tsx (1)
EditableFile(13-18)
convex/messages.ts (4)
convex/_generated/server.js (2)
mutation(49-49)mutation(49-49)convex/_generated/dataModel.d.ts (1)
Id(48-49)convex/_generated/api.js (2)
internal(22-22)internal(22-22)convex/fileAggregate.ts (1)
fileCountAggregate(13-21)
🔇 Additional comments (13)
lib/db/actions.ts (1)
56-106: LGTM! Clean extension of saveMessage API.The new optional metadata fields (
model,generationTimeMs,finishReason,usage,regenerationCount) are correctly typed and forwarded to the Convex backend. The flexibleRecord<string, unknown>type forusageappropriately matches thev.any()validator in the schema.convex/schema.ts (1)
66-70: LGTM! Schema extension for message metadata.The new optional fields follow the existing snake_case naming convention and are properly defined. Using
v.any()forusageis reasonable given that usage data varies across AI providers.lib/api/chat-handler.ts (2)
285-290: LGTM! Well-structured metrics tracking initialization.The timing and model tracking variables are properly initialized before the stream starts, enabling accurate calculation of generation time.
379-382: Good privacy enhancement.Disabling conversation storage in XAI's database is a sensible privacy default.
app/hooks/useChatHandlers.ts (2)
306-323: LGTM! Regeneration count tracking properly implemented.The
regenerationCountis correctly extracted from the mutation result and propagated to the regeneration request. The fallback to1handles edge cases where no previous count exists.
389-393: LGTM! Clean signature extension for file ID preservation.The
remainingFileIdsparameter allows the edit flow to preserve selected file attachments, aligning well with the MessageEditor changes.app/components/MessageEditor.tsx (4)
13-18: LGTM! Well-defined EditableFile interface.The interface correctly specifies
fileIdandnameas required fields while keepingmediaTypeandurloptional, providing appropriate flexibility for file handling.
34-37: Good design decision on state initialization.Explicitly avoiding prop synchronization prevents state resets during parent re-renders, which is the correct behavior for an editor component maintaining user changes.
52-72: LGTM! Proper save flow with file ID preservation.The save handler correctly validates that either content or files exist, computes remaining file IDs from the current state, and propagates them via the updated
onSavecallback.
88-147: Clean file preview implementation.The UI appropriately handles both image previews (with Next.js Image) and document placeholders, with accessible remove buttons. The
unoptimizedprop is correctly used for user-uploaded content where dimensions are unknown.convex/messages.ts (3)
107-111: LGTM! Metadata persistence in saveMessage.The new optional fields are correctly validated and persisted, following the Convex coding guidelines for optional validators and field naming conventions.
Also applies to: 179-183
483-483: LGTM! Return type change for regeneration tracking.The return type properly updated from
v.null()tov.object({ regenerationCount: v.number() }), and the implementation correctly increments and returns the count. As per Convex coding guidelines, the return validator accurately reflects the actual return value.Also applies to: 499-501, 572-572
1021-1036: LGTM! File retention logic is correct.The logic properly handles both cases: when
fileIdsis provided (keep specified files), and when undefined (remove all files). The Set-based lookup forkeepSetensures efficient O(1) membership checks.
| await ctx.db.patch(message._id, { | ||
| parts: [{ type: "text", text: args.newContent }], | ||
| parts: | ||
| newParts.length > 0 | ||
| ? newParts | ||
| : [{ type: "text", text: args.newContent }], | ||
| content: args.newContent.trim() || undefined, | ||
| file_ids: undefined, | ||
| file_ids: newFileIds && newFileIds.length > 0 ? newFileIds : undefined, | ||
| update_time: Date.now(), | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, let's examine the function containing the code at lines 1079-1087
sed -n '1050,1110p' convex/messages.tsRepository: hackerai-tech/hackerai
Length of output: 188
🏁 Script executed:
# Search for how empty parts are handled elsewhere
rg -n "parts.*length.*0|parts.*empty|\[\].*parts" --type=ts -C 2 convex/Repository: hackerai-tech/hackerai
Length of output: 48
🏁 Script executed:
# Check for validation on newContent/content arguments
rg -n "newContent|args\.content" --type=ts -C 2 convex/messages.ts | head -100Repository: hackerai-tech/hackerai
Length of output: 741
🏁 Script executed:
#!/bin/bash
# Get the full function context (lines 950-1100)
sed -n '950,1100p' convex/messages.tsRepository: hackerai-tech/hackerai
Length of output: 4810
🏁 Script executed:
# Search for the message schema definition to see if empty parts are valid
rg -n "defineSchema|messages.*schema|parts:" --type=ts -C 3 convex/ | head -150Repository: hackerai-tech/hackerai
Length of output: 6784
🏁 Script executed:
# Check if there's any cleanup/validation of empty parts after this patch
sed -n '1085,1150p' convex/messages.tsRepository: hackerai-tech/hackerai
Length of output: 2344
Inconsistency: empty content fallback uses untrimmed value while the check uses trimmed value.
Line 1064 checks args.newContent.trim() before adding to newParts, but the fallback on line 1083 uses the untrimmed args.newContent. When content is whitespace-only and no files remain, this creates parts: [{ type: "text", text: " " }] with content: undefined, leaving the message in an inconsistent state where the parts array preserves whitespace that was explicitly excluded from the content field. Use args.newContent.trim() in the fallback to match the intent of line 1064, or add input validation to reject empty/whitespace-only messages.
🤖 Prompt for AI Agents
In convex/messages.ts around lines 1064 to 1087, the fallback parts creation
uses the untrimmed args.newContent while the earlier check used
args.newContent.trim(), causing whitespace-only content to produce parts with
spaces but content set to undefined; fix by using args.newContent.trim() for the
fallback parts (and for content assignment) or add validation to
reject/normalize whitespace-only input so both parts and content consistently
use the trimmed value.
Summary by CodeRabbit
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.