feat: rename/delete/add image #2967
Conversation
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
|
Warning Rate limit exceeded@Kitenite has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 16 minutes and 13 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (5)
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdded rename and delete operations to the image operations hook, exposed and wrapped them with UI feedback in the Images tab, and propagated interactive callbacks (rename, delete, add-to-chat) through ImageGrid to ImageItem; ImageItem implements inline rename, delete confirmation, add-to-chat, and drag mime updates. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as User
participant II as ImageItem
participant IG as ImageGrid
participant IT as ImagesTab
participant HOOK as useImageOperations
participant FS as codeEditor (FS)
participant CHAT as editorEngine.chat.context
participant T as toast
rect rgb(245,248,255)
note right of II: Rename flow
U->>II: start rename (newName)
II->>IG: onRename(oldPath, newName)
IG->>IT: onRename(oldPath, newName)
IT->>HOOK: handleRename(oldPath, newName)
HOOK->>FS: readFile(oldPath)
FS-->>HOOK: content
HOOK->>FS: writeFile(newPath, content)
HOOK->>FS: deleteFile(oldPath)
HOOK-->>IT: resolve
IT->>T: success("Image renamed")
alt failure
HOOK-->>IT: throw/error
IT->>T: error("Rename failed")
end
end
rect rgb(245,255,245)
note right of II: Delete flow
U->>II: confirm delete
II->>IG: onDelete(filePath)
IG->>IT: onDelete(filePath)
IT->>HOOK: handleDelete(filePath)
HOOK->>FS: deleteFile(filePath)
HOOK-->>IT: resolve
IT->>T: success("Image deleted")
alt failure
HOOK-->>IT: error
IT->>T: error("Delete failed")
end
end
rect rgb(255,250,240)
note right of II: Add to Chat flow
U->>II: click "Add to Chat"
II->>IG: onAddToChat(imagePath)
IG->>IT: onAddToChat(imagePath)
IT->>CHAT: add(FileMessageContext)
CHAT-->>IT: ack
IT->>T: success("Added to chat")
alt failure
CHAT-->>IT: error
IT->>T: error("Add to chat failed")
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
| } | ||
|
|
||
| // Write to new path and delete old file | ||
| await codeEditor.writeFile(newPath, content); |
There was a problem hiding this comment.
Consider checking if a file already exists at the target (newPath) before renaming to avoid accidental overwrites.
apps/web/client/src/app/project/[id]/_components/left-panel/image-tab/image-item.tsx
Show resolved
Hide resolved
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
| const imageContentData: ImageContentData = { | ||
| fileName: image.name, | ||
| content: content as string, | ||
| mimeType: imageUrl || '', |
There was a problem hiding this comment.
The mimeType property is being assigned the entire data URL value from imageUrl rather than extracting just the MIME type information. This could cause issues downstream where only the actual MIME type (e.g., image/png, image/jpeg) is expected. Consider either:
- Extracting the MIME type portion from the data URL using a regex or string manipulation
- Determining the MIME type based on the file extension
- Using a dedicated function to properly identify the content type
This would ensure the correct content type information is passed to the chat system.
| mimeType: imageUrl || '', | |
| mimeType: imageUrl ? imageUrl.split(';')[0].split(':')[1] : 'image/png', |
Spotted by Diamond
Is this helpful? React 👍 or 👎 to let us know.
apps/web/client/src/app/project/[id]/_components/left-panel/image-tab/image-item.tsx
Show resolved
Hide resolved
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/client/src/app/project/[id]/_components/left-panel/image-tab/image-item.tsx (1)
99-112: Fix incorrect mimeType assignment.Line 108 assigns
imageUrltomimeType, butimageUrlis a data URL (e.g.,"data:image/png;base64,..."or"blob:..."), not a MIME type (e.g.,"image/png"). This will cause incorrect MIME type values in theImageContentData.Apply this diff:
const handleDragStart = (e: React.DragEvent<HTMLDivElement>) => { if (isDisabled) { e.preventDefault(); return; } const imageContentData: ImageContentData = { fileName: image.name, content: content as string, - mimeType: imageUrl, + mimeType: image.mimeType || 'image/*', originPath: image.path, }; onImageDragStart(e, imageContentData); };
🧹 Nitpick comments (6)
apps/web/client/src/app/project/[id]/_components/left-panel/image-tab/index.tsx (4)
50-59: Consider avoiding hardcoded user-facing text.The success and error messages are hardcoded, which violates the coding guidelines requiring the use of next-intl for user-facing text. However, this can be deferred as a recommended refactor.
As per coding guidelines.
61-70: Consider avoiding hardcoded user-facing text.The success and error messages are hardcoded, which violates the coding guidelines requiring the use of next-intl for user-facing text. However, this can be deferred as a recommended refactor.
As per coding guidelines.
84-84: Consider avoiding hardcoded user-facing text.The toast message is hardcoded. Per coding guidelines, user-facing text should use next-intl. However, this can be deferred as a recommended refactor.
As per coding guidelines.
87-87: Consider avoiding hardcoded user-facing text.The toast message is hardcoded. Per coding guidelines, user-facing text should use next-intl. However, this can be deferred as a recommended refactor.
As per coding guidelines.
apps/web/client/src/app/project/[id]/_components/left-panel/image-tab/image-item.tsx (2)
188-198: Consider avoiding hardcoded user-facing text.The text "Add to Chat" should use next-intl per coding guidelines. This applies to all menu items in this dropdown. However, this can be deferred as a recommended refactor.
As per coding guidelines.
250-253: Consider avoiding hardcoded user-facing text.The dialog title and description are hardcoded. Per coding guidelines, user-facing text should use next-intl. However, this can be deferred as a recommended refactor.
As per coding guidelines.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/web/client/src/app/project/[id]/_components/left-panel/image-tab/hooks/use-image-operations.tsx(1 hunks)apps/web/client/src/app/project/[id]/_components/left-panel/image-tab/image-grid.tsx(2 hunks)apps/web/client/src/app/project/[id]/_components/left-panel/image-tab/image-item.tsx(3 hunks)apps/web/client/src/app/project/[id]/_components/left-panel/image-tab/index.tsx(3 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
apps/web/client/src/app/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/app/**/*.tsx: Default to Server Components; add 'use client' when using events, state/effects, browser APIs, or client‑only libraries
Do not use process.env in client code; import env from @/env insteadAvoid hardcoded user-facing text; use next-intl messages/hooks
Files:
apps/web/client/src/app/project/[id]/_components/left-panel/image-tab/image-grid.tsxapps/web/client/src/app/project/[id]/_components/left-panel/image-tab/hooks/use-image-operations.tsxapps/web/client/src/app/project/[id]/_components/left-panel/image-tab/image-item.tsxapps/web/client/src/app/project/[id]/_components/left-panel/image-tab/index.tsx
apps/web/client/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/**/*.{ts,tsx}: Use path aliases @/* and ~/* for imports that map to apps/web/client/src/*
Avoid hardcoded user-facing text; use next-intl messages/hooks insteadUse path aliases @/* and ~/* for imports mapping to src/*
Files:
apps/web/client/src/app/project/[id]/_components/left-panel/image-tab/image-grid.tsxapps/web/client/src/app/project/[id]/_components/left-panel/image-tab/hooks/use-image-operations.tsxapps/web/client/src/app/project/[id]/_components/left-panel/image-tab/image-item.tsxapps/web/client/src/app/project/[id]/_components/left-panel/image-tab/index.tsx
apps/web/client/src/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/**/*.tsx: Create MobX store instances with useState(() => new Store()) for stable references across renders
Keep the active MobX store in a useRef and perform async cleanup with setTimeout(() => storeRef.current?.clear(), 0) to avoid route-change races
Avoid useMemo for creating MobX store instances
Avoid putting the MobX store instance in effect dependency arrays if it causes loops; split concerns by domain
apps/web/client/src/**/*.tsx: Create MobX store instances with useState(() => new Store()) for stable identities across renders
Keep the active MobX store in a useRef and clean up asynchronously with setTimeout(() => storeRef.current?.clear(), 0)
Do not use useMemo to create MobX stores
Avoid placing MobX store instances in effect dependency arrays if it causes loops; split concerns instead
observer components must be client components; place a single client boundary at the feature entry; child observers need not repeat 'use client'
Files:
apps/web/client/src/app/project/[id]/_components/left-panel/image-tab/image-grid.tsxapps/web/client/src/app/project/[id]/_components/left-panel/image-tab/hooks/use-image-operations.tsxapps/web/client/src/app/project/[id]/_components/left-panel/image-tab/image-item.tsxapps/web/client/src/app/project/[id]/_components/left-panel/image-tab/index.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Do not use the any type unless necessary
Files:
apps/web/client/src/app/project/[id]/_components/left-panel/image-tab/image-grid.tsxapps/web/client/src/app/project/[id]/_components/left-panel/image-tab/hooks/use-image-operations.tsxapps/web/client/src/app/project/[id]/_components/left-panel/image-tab/image-item.tsxapps/web/client/src/app/project/[id]/_components/left-panel/image-tab/index.tsx
apps/web/client/src/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Default to Server Components; add 'use client' only when using events, state/effects, browser APIs, or client-only libs
Files:
apps/web/client/src/app/project/[id]/_components/left-panel/image-tab/image-grid.tsxapps/web/client/src/app/project/[id]/_components/left-panel/image-tab/hooks/use-image-operations.tsxapps/web/client/src/app/project/[id]/_components/left-panel/image-tab/image-item.tsxapps/web/client/src/app/project/[id]/_components/left-panel/image-tab/index.tsx
{apps,packages}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Avoid using the any type unless absolutely necessary
Files:
apps/web/client/src/app/project/[id]/_components/left-panel/image-tab/image-grid.tsxapps/web/client/src/app/project/[id]/_components/left-panel/image-tab/hooks/use-image-operations.tsxapps/web/client/src/app/project/[id]/_components/left-panel/image-tab/image-item.tsxapps/web/client/src/app/project/[id]/_components/left-panel/image-tab/index.tsx
🧬 Code graph analysis (3)
apps/web/client/src/app/project/[id]/_components/left-panel/image-tab/hooks/use-image-operations.tsx (2)
packages/utility/src/string.ts (1)
sanitizeFilename(72-88)packages/utility/src/file.ts (1)
isImageFile(101-104)
apps/web/client/src/app/project/[id]/_components/left-panel/image-tab/image-item.tsx (3)
packages/models/src/actions/action.ts (1)
ImageContentData(93-98)packages/ui/src/components/dropdown-menu.tsx (4)
DropdownMenu(214-214)DropdownMenuTrigger(228-228)DropdownMenuContent(216-216)DropdownMenuItem(218-218)packages/ui/src/components/button.tsx (1)
Button(57-57)
apps/web/client/src/app/project/[id]/_components/left-panel/image-tab/index.tsx (2)
apps/web/client/src/app/project/[id]/_components/left-panel/image-tab/hooks/use-image-operations.tsx (1)
useImageOperations(8-99)packages/models/src/chat/message/context.ts (1)
FileMessageContext(23-27)
🔇 Additional comments (4)
apps/web/client/src/app/project/[id]/_components/left-panel/image-tab/hooks/use-image-operations.tsx (1)
84-87: LGTM!The delete implementation is straightforward and correctly validates the code editor availability before attempting deletion.
apps/web/client/src/app/project/[id]/_components/left-panel/image-tab/image-item.tsx (3)
114-124: LGTM!The rename handler correctly validates input, calls the parent handler, and resets state on error. Proper use of
voidfor async operations.
126-133: LGTM!The delete handler correctly invokes the parent callback and handles errors appropriately.
145-152: LGTM!Keyboard handling for rename is properly implemented with Enter to confirm and Escape to cancel.
| const handleRename = async (oldPath: string, newName: string) => { | ||
| if (!codeEditor) throw new Error('Code editor not available'); | ||
|
|
||
| const directory = path.dirname(oldPath); | ||
| const sanitizedName = sanitizeFilename(newName); | ||
| const newPath = path.join(directory, sanitizedName); | ||
|
|
||
| // Check if it's still an image file | ||
| if (!isImageFile(sanitizedName)) { | ||
| throw new Error('File must be an image'); | ||
| } | ||
|
|
||
| // Read the existing file content | ||
| const content = await codeEditor.readFile(oldPath); | ||
| if (!content) { | ||
| throw new Error('Could not read file content'); | ||
| } | ||
|
|
||
| // Write to new path and delete old file | ||
| await codeEditor.writeFile(newPath, content); | ||
| await codeEditor.deleteFile(oldPath); | ||
| }; |
There was a problem hiding this comment.
Add collision check and improve error handling.
The rename operation has two issues:
- File collision risk: No check if
newPathalready exists, which could silently overwrite an existing file. - Partial failure state: If
writeFilesucceeds butdeleteFilefails, you'll have both the old and new files, creating an inconsistent state.
Apply this diff to add collision check:
const handleRename = async (oldPath: string, newName: string) => {
if (!codeEditor) throw new Error('Code editor not available');
const directory = path.dirname(oldPath);
const sanitizedName = sanitizeFilename(newName);
const newPath = path.join(directory, sanitizedName);
// Check if it's still an image file
if (!isImageFile(sanitizedName)) {
throw new Error('File must be an image');
}
+
+ // Check if the target path already exists
+ if (newPath !== oldPath) {
+ try {
+ const existingContent = await codeEditor.readFile(newPath);
+ if (existingContent) {
+ throw new Error('A file with this name already exists');
+ }
+ } catch (error) {
+ // File doesn't exist, which is what we want
+ if (!(error instanceof Error) || !error.message.includes('not found')) {
+ throw error;
+ }
+ }
+ }
// Read the existing file content
const content = await codeEditor.readFile(oldPath);
if (!content) {
throw new Error('Could not read file content');
}
- // Write to new path and delete old file
- await codeEditor.writeFile(newPath, content);
- await codeEditor.deleteFile(oldPath);
+ // Write to new path and delete old file
+ try {
+ await codeEditor.writeFile(newPath, content);
+ await codeEditor.deleteFile(oldPath);
+ } catch (error) {
+ // If write succeeded but delete failed, attempt cleanup
+ try {
+ await codeEditor.deleteFile(newPath);
+ } catch {
+ // Cleanup failed, log the inconsistent state
+ console.error('Failed to cleanup after partial rename failure');
+ }
+ throw error;
+ }
};📝 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 handleRename = async (oldPath: string, newName: string) => { | |
| if (!codeEditor) throw new Error('Code editor not available'); | |
| const directory = path.dirname(oldPath); | |
| const sanitizedName = sanitizeFilename(newName); | |
| const newPath = path.join(directory, sanitizedName); | |
| // Check if it's still an image file | |
| if (!isImageFile(sanitizedName)) { | |
| throw new Error('File must be an image'); | |
| } | |
| // Read the existing file content | |
| const content = await codeEditor.readFile(oldPath); | |
| if (!content) { | |
| throw new Error('Could not read file content'); | |
| } | |
| // Write to new path and delete old file | |
| await codeEditor.writeFile(newPath, content); | |
| await codeEditor.deleteFile(oldPath); | |
| }; | |
| const handleRename = async (oldPath: string, newName: string) => { | |
| if (!codeEditor) throw new Error('Code editor not available'); | |
| const directory = path.dirname(oldPath); | |
| const sanitizedName = sanitizeFilename(newName); | |
| const newPath = path.join(directory, sanitizedName); | |
| // Check if it's still an image file | |
| if (!isImageFile(sanitizedName)) { | |
| throw new Error('File must be an image'); | |
| } | |
| // Check if the target path already exists | |
| if (newPath !== oldPath) { | |
| try { | |
| const existingContent = await codeEditor.readFile(newPath); | |
| if (existingContent) { | |
| throw new Error('A file with this name already exists'); | |
| } | |
| } catch (error) { | |
| // File doesn't exist, which is what we want | |
| if (!(error instanceof Error) || !error.message.includes('not found')) { | |
| throw error; | |
| } | |
| } | |
| } | |
| // Read the existing file content | |
| const content = await codeEditor.readFile(oldPath); | |
| if (!content) { | |
| throw new Error('Could not read file content'); | |
| } | |
| // Write to new path and delete old file | |
| try { | |
| await codeEditor.writeFile(newPath, content); | |
| await codeEditor.deleteFile(oldPath); | |
| } catch (error) { | |
| // If write succeeded but delete failed, attempt cleanup | |
| try { | |
| await codeEditor.deleteFile(newPath); | |
| } catch { | |
| // Cleanup failed, log the inconsistent state | |
| console.error('Failed to cleanup after partial rename failure'); | |
| } | |
| throw error; | |
| } | |
| }; |
🤖 Prompt for AI Agents
In
apps/web/client/src/app/project/[id]/_components/left-panel/image-tab/hooks/use-image-operations.tsx
around lines 60 to 81, the rename flow lacks a collision check and can leave a
partial failure if writeFile succeeds but deleteFile fails; add an existence
check for newPath before writing and throw a clear error if it exists, then
perform writeFile inside a try/catch and if deleteFile fails attempt to rollback
by deleting the newly written file (and surface combined error information), and
ensure all thrown errors include contextual messages (oldPath/newPath) so
callers can handle them.
| onUpload: (files: FileList) => Promise<void>; | ||
| onRename: (oldPath: string, newName: string) => Promise<void>; | ||
| onDelete: (filePath: string) => Promise<void>; | ||
| onAddToChat: (image: any) => void; |
There was a problem hiding this comment.
Replace any with specific type.
The any type violates the coding guidelines. Based on the relevant code snippets, the correct type should be ImageContentData from @onlook/models.
As per coding guidelines.
Apply this diff:
- onAddToChat: (image: any) => void;
+ onAddToChat: (image: ImageContentData) => void;Import the type at the top of the file:
import type { ImageContentData } from '@onlook/models';🤖 Prompt for AI Agents
In
apps/web/client/src/app/project/[id]/_components/left-panel/image-tab/image-grid.tsx
around line 17, the prop type uses `any` for onAddToChat; update the signature
to use the specific `ImageContentData` type and add an import at the top of the
file: import the type from '@onlook/models' (import type { ImageContentData }
from '@onlook/models';) and replace `onAddToChat: (image: any) => void;` with
`onAddToChat: (image: ImageContentData) => void;`.
| const handleAddToChat = () => { | ||
| const imageContentData: ImageContentData = { | ||
| fileName: image.name, | ||
| content: content as string, | ||
| mimeType: imageUrl || '', | ||
| originPath: image.path, | ||
| }; | ||
| onAddToChat(imageContentData); | ||
| }; |
There was a problem hiding this comment.
Fix incorrect mimeType assignment.
Line 139 has the same issue as line 108: mimeType is set to imageUrl (a data URL) instead of the actual MIME type.
Apply this diff:
const handleAddToChat = () => {
const imageContentData: ImageContentData = {
fileName: image.name,
content: content as string,
- mimeType: imageUrl || '',
+ mimeType: image.mimeType || 'image/*',
originPath: image.path,
};
onAddToChat(imageContentData);
};📝 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 handleAddToChat = () => { | |
| const imageContentData: ImageContentData = { | |
| fileName: image.name, | |
| content: content as string, | |
| mimeType: imageUrl || '', | |
| originPath: image.path, | |
| }; | |
| onAddToChat(imageContentData); | |
| }; | |
| const handleAddToChat = () => { | |
| const imageContentData: ImageContentData = { | |
| fileName: image.name, | |
| content: content as string, | |
| mimeType: image.mimeType || 'image/*', | |
| originPath: image.path, | |
| }; | |
| onAddToChat(imageContentData); | |
| }; |
🤖 Prompt for AI Agents
In
apps/web/client/src/app/project/[id]/_components/left-panel/image-tab/image-item.tsx
around lines 135 to 143, the mimeType for ImageContentData is incorrectly set to
imageUrl (a data URL); change the assignment to use the actual MIME type from
the image object (e.g. image.mimeType || ''), ensuring the field contains a MIME
type string rather than a data URL and preserving types when calling
onAddToChat.
| const handleAddToChat = async (imageContentData: any) => { | ||
| try { | ||
| // Convert the image content data to file context for chat | ||
| const fileContext: FileMessageContext = { | ||
| type: MessageContextType.FILE, | ||
| content: '', // File content will be loaded by the chat system | ||
| displayName: imageContentData.fileName, | ||
| path: imageContentData.originPath, | ||
| branchId: branchId, | ||
| }; | ||
|
|
||
| editorEngine.chat.context.addContexts([fileContext]); | ||
| toast.success('Image added to chat'); | ||
| } catch (error) { | ||
| console.error('Failed to add image to chat:', error); | ||
| toast.error('Failed to add image to chat'); | ||
| } | ||
| }; |
There was a problem hiding this comment.
Fix type annotation and clarify empty content field.
Two issues:
- Type violation: Parameter uses
anytype, which violates coding guidelines. - Empty content: Line 77 sets
content: ''with a comment saying it will be loaded by the chat system, butimageContentDataalready contains the content. This seems inconsistent and may cause issues if the chat system expects pre-loaded content.
As per coding guidelines.
Apply this diff:
-const handleAddToChat = async (imageContentData: any) => {
+const handleAddToChat = async (imageContentData: ImageContentData) => {
try {
// Convert the image content data to file context for chat
const fileContext: FileMessageContext = {
type: MessageContextType.FILE,
- content: '', // File content will be loaded by the chat system
+ content: imageContentData.content,
displayName: imageContentData.fileName,
path: imageContentData.originPath,
branchId: branchId,
};Also add the import:
import type { ImageContentData } from '@onlook/models';🤖 Prompt for AI Agents
In
apps/web/client/src/app/project/[id]/_components/left-panel/image-tab/index.tsx
around lines 72-89, the handler uses an any parameter and sets content to an
empty string; change the parameter type to ImageContentData (add import: import
type { ImageContentData } from '@onlook/models') and populate the
FileMessageContext.content with the actual imageContentData content field (e.g.,
imageContentData.content or correct property name), keep the other fields
(displayName, path, branchId) as-is, and remove the any usage so the function
signature and context object are fully typed.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
apps/web/client/src/app/project/[id]/_components/left-panel/image-tab/index.tsx (1)
53-88: Localize toast copy instead of hardcoding stringsAll toast messages here are user-facing and should come from next-intl translations rather than inline string literals so they remain localizable. Please move these messages to the intl catalog and pull them via
useTranslations. As per coding guidelinesapps/web/client/src/app/project/[id]/_components/left-panel/image-tab/image-item.tsx (1)
191-266: Replace hardcoded UI strings with next-intl messagesMenu labels and the delete-dialog title/description are user-visible. Per our guidelines for this path, they should be sourced from next-intl translations instead of inline literals. Please externalize these strings and read them via
useTranslations. As per coding guidelines
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/web/client/src/app/project/[id]/_components/left-panel/image-tab/image-grid.tsx(2 hunks)apps/web/client/src/app/project/[id]/_components/left-panel/image-tab/image-item.tsx(4 hunks)apps/web/client/src/app/project/[id]/_components/left-panel/image-tab/index.tsx(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web/client/src/app/project/[id]/_components/left-panel/image-tab/image-grid.tsx
🧰 Additional context used
📓 Path-based instructions (6)
apps/web/client/src/app/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/app/**/*.tsx: Default to Server Components; add 'use client' when using events, state/effects, browser APIs, or client‑only libraries
Do not use process.env in client code; import env from @/env insteadAvoid hardcoded user-facing text; use next-intl messages/hooks
Files:
apps/web/client/src/app/project/[id]/_components/left-panel/image-tab/image-item.tsxapps/web/client/src/app/project/[id]/_components/left-panel/image-tab/index.tsx
apps/web/client/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/**/*.{ts,tsx}: Use path aliases @/* and ~/* for imports that map to apps/web/client/src/*
Avoid hardcoded user-facing text; use next-intl messages/hooks insteadUse path aliases @/* and ~/* for imports mapping to src/*
Files:
apps/web/client/src/app/project/[id]/_components/left-panel/image-tab/image-item.tsxapps/web/client/src/app/project/[id]/_components/left-panel/image-tab/index.tsx
apps/web/client/src/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/**/*.tsx: Create MobX store instances with useState(() => new Store()) for stable references across renders
Keep the active MobX store in a useRef and perform async cleanup with setTimeout(() => storeRef.current?.clear(), 0) to avoid route-change races
Avoid useMemo for creating MobX store instances
Avoid putting the MobX store instance in effect dependency arrays if it causes loops; split concerns by domain
apps/web/client/src/**/*.tsx: Create MobX store instances with useState(() => new Store()) for stable identities across renders
Keep the active MobX store in a useRef and clean up asynchronously with setTimeout(() => storeRef.current?.clear(), 0)
Do not use useMemo to create MobX stores
Avoid placing MobX store instances in effect dependency arrays if it causes loops; split concerns instead
observer components must be client components; place a single client boundary at the feature entry; child observers need not repeat 'use client'
Files:
apps/web/client/src/app/project/[id]/_components/left-panel/image-tab/image-item.tsxapps/web/client/src/app/project/[id]/_components/left-panel/image-tab/index.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Do not use the any type unless necessary
Files:
apps/web/client/src/app/project/[id]/_components/left-panel/image-tab/image-item.tsxapps/web/client/src/app/project/[id]/_components/left-panel/image-tab/index.tsx
apps/web/client/src/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Default to Server Components; add 'use client' only when using events, state/effects, browser APIs, or client-only libs
Files:
apps/web/client/src/app/project/[id]/_components/left-panel/image-tab/image-item.tsxapps/web/client/src/app/project/[id]/_components/left-panel/image-tab/index.tsx
{apps,packages}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Avoid using the any type unless absolutely necessary
Files:
apps/web/client/src/app/project/[id]/_components/left-panel/image-tab/image-item.tsxapps/web/client/src/app/project/[id]/_components/left-panel/image-tab/index.tsx
🧬 Code graph analysis (2)
apps/web/client/src/app/project/[id]/_components/left-panel/image-tab/image-item.tsx (1)
packages/utility/src/file.ts (1)
getMimeType(85-99)
apps/web/client/src/app/project/[id]/_components/left-panel/image-tab/index.tsx (3)
apps/web/client/src/app/project/[id]/_components/left-panel/image-tab/hooks/use-image-operations.tsx (1)
useImageOperations(8-85)packages/ui/src/components/sonner.tsx (1)
toast(19-19)packages/models/src/chat/message/context.ts (1)
FileMessageContext(23-27)
| // Wait for all updates to complete | ||
| await Promise.all(updatePromises); | ||
|
|
||
| // Finally, rename the actual image file | ||
| await codeEditor.moveFile(oldPath, newPath); |
There was a problem hiding this comment.
The current implementation has a potential race condition: if any reference updates fail but the file rename succeeds, the system could be left in an inconsistent state with broken references. Consider implementing either:
- A transaction-like approach where the file is only renamed after confirming all reference updates succeeded
- A rollback mechanism that reverts any successful reference updates if the file rename fails
This would improve reliability, especially for larger projects with many references to the same image.
| // Wait for all updates to complete | |
| await Promise.all(updatePromises); | |
| // Finally, rename the actual image file | |
| await codeEditor.moveFile(oldPath, newPath); | |
| try { | |
| // Wait for all updates to complete | |
| await Promise.all(updatePromises); | |
| // Only rename the file if all reference updates succeeded | |
| await codeEditor.moveFile(oldPath, newPath); | |
| } catch (error) { | |
| // If any reference updates failed, don't proceed with the file rename | |
| console.error('Failed to update all references, file rename aborted:', error); | |
| throw new Error('Image rename failed: Could not update all references to the image'); | |
| } |
Spotted by Diamond
Is this helpful? React 👍 or 👎 to let us know.
Description
Related Issues
Type of Change
Testing
Screenshots (if applicable)
Additional Notes
Important
Add image management features including rename, delete, and add-to-chat, with updated references and user feedback.
use-image-operations.tsx.image-item.tsxwith options for Add to Chat, Rename, and Delete.updateImageReferencesinimage-references.tsupdates image paths in JS/TS files using AST.updateImageReferencesinimage-references.test.tscover various scenarios.index.tsx.image-item.tsxto prevent accidental deletions.This description was created by
for a4feecf. You can customize this summary. It will automatically update as commits are pushed.
Summary by CodeRabbit
New Features
Improvements