fix: stricter file caching #2844
Conversation
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
WalkthroughUnified sandbox file-change handling to always read remote content and update cache, limited JSX mapping to JSX text files; cache sync after copy. Added tooltipped image actions in the image folder UI. Image provider and image scanning now source and filter image paths from the image manager with guarded scanning and stricter IMAGE_FOLDER prefix checks. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant FW as File Watcher
participant SB as Sandbox
participant RFS as Remote FS
participant C as Cache
participant MAP as Mapper (JSX)
rect rgba(230,245,255,0.5)
FW->>SB: fileChanged(path)
SB->>RFS: read(path)
alt read OK
RFS-->>SB: remoteFile(content, type)
SB->>C: update(path, content)
opt if type == text AND isJsxFile(path) AND content != cache
SB->>MAP: trigger mapping for path
end
else read failed
SB-->>FW: log error and return
end
end
sequenceDiagram
autonumber
participant U as User
participant SB as Sandbox
participant RFS as Remote FS
participant C as Cache
rect rgba(240,255,240,0.5)
U->>SB: copy(src, dest)
SB->>RFS: copy(src -> dest)
alt copy OK
SB->>RFS: read(dest)
RFS-->>SB: remoteFile(content)
SB->>C: update(dest, content)
else copy/read failed
SB-->>U: error
end
end
sequenceDiagram
autonumber
participant UI as Left Panel (UI)
participant IMG as ImageStore
participant SB as Sandbox
rect rgba(255,250,230,0.5)
UI->>IMG: tabChanged -> IMAGES
IMG->>IMG: check isIndexing / isIndexed
alt indexed and not indexing
IMG->>SB: read activeSandbox.files
SB-->>IMG: files[]
IMG->>IMG: filter paths starting with IMAGE_FOLDER & isImageFile
IMG-->>UI: update imagePaths
else not ready
IMG-->>UI: do nothing
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/web/client/src/components/store/editor/sandbox/index.ts (1)
731-731: Rename relies solely on watcher; add a fallback for stale-cache riskIf a provider fails to emit a rename event or the watcher is inactive, the cache won’t update. Add a guarded fallback.
- // Note: Cache update handled by file watcher rename event + // Note: Cache update handled by file watcher rename event + // Fallback: if no watcher is active, eagerly update cache to avoid stale state + if (!this.fileWatcher) { + await this.fileSync.rename(normalizedOldPath, normalizedNewPath); + this.fileEventBus.publish({ + type: 'rename', + paths: [normalizedOldPath, normalizedNewPath], + timestamp: Date.now(), + }); + }Please confirm all deployed providers reliably emit rename events for
renameFileoperations. If not, the fallback above should be kept.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/web/client/src/components/store/editor/sandbox/index.ts(3 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
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 instead
Files:
apps/web/client/src/components/store/editor/sandbox/index.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Do not use the any type unless necessary
Files:
apps/web/client/src/components/store/editor/sandbox/index.ts
🔇 Additional comments (1)
apps/web/client/src/components/store/editor/sandbox/index.ts (1)
552-565: Avoid false-negative change detection when cache mutates in placeupdateCache → FileCacheManager.setFile → UnifiedCacheManager.set; if that set mutates the cached object by reference, comparing after update can hide real changes. Capture previous text content before updating the cache and compare against it.
Apply this diff:
- // Always read the remote file and update the cache, regardless of file type - const remoteFile = await this.readRemoteFile(normalizedPath); - if (!remoteFile) { - console.error(`File content for ${normalizedPath} not found in remote`); - return; - } - - // Always update the cache with the fresh remote file content - this.fileSync.updateCache(remoteFile); - - // For text files, also process for mapping if content has changed - if (remoteFile.type === 'text' && this.isJsxFile(normalizedPath)) { - if (remoteFile.content !== cachedFile?.content) { - await this.processFileForMapping(remoteFile); - } - } + // Always read the remote file; capture previous text content before updating cache + const prevTextContent = cachedFile?.type === 'text' ? cachedFile.content : undefined; + const remoteFile = await this.readRemoteFile(normalizedPath); + if (!remoteFile) { + console.error(`File content for ${normalizedPath} not found in remote`); + return; + } + const shouldProcess = + remoteFile.type === 'text' && + this.isJsxFile(normalizedPath) && + remoteFile.content !== prevTextContent; + // Update the cache with the fresh remote file content + this.fileSync.updateCache(remoteFile); + if (shouldProcess) { + await this.processFileForMapping(remoteFile); + }Confirm whether UnifiedCacheManager.set mutates existing entries in-place; if it does, merge this change.
| // Read and cache the copied file | ||
| const copiedFile = await this.readRemoteFile(normalizedTargetPath); | ||
| if (copiedFile) { | ||
| this.fileSync.updateCache(copiedFile); | ||
| } | ||
|
|
There was a problem hiding this comment.
Copying directories leaves cache stale; handle directory targets
After copyFiles, attempting to read a directory via readRemoteFile returns null and the directory cache isn’t updated. This desynchronizes the cache on directory copies (common when recursive is true).
Apply this diff:
- // Read and cache the copied file
- const copiedFile = await this.readRemoteFile(normalizedTargetPath);
- if (copiedFile) {
- this.fileSync.updateCache(copiedFile);
- }
+ // Sync cache for the copy target (file or directory)
+ const stat = await this.session.provider.statFile({
+ args: { path: normalizedTargetPath },
+ });
+ if (stat?.type === 'directory') {
+ this.fileSync.updateDirectoryCache(normalizedTargetPath);
+ // Optional: list + seed child entries if provider doesn't emit add events for children.
+ // (Rely on watcher by default.)
+ } else {
+ const copiedFile = await this.readRemoteFile(normalizedTargetPath);
+ if (copiedFile) {
+ this.fileSync.updateCache(copiedFile);
+ }
+ }📝 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.
| // Read and cache the copied file | |
| const copiedFile = await this.readRemoteFile(normalizedTargetPath); | |
| if (copiedFile) { | |
| this.fileSync.updateCache(copiedFile); | |
| } | |
| // Sync cache for the copy target (file or directory) | |
| const stat = await this.session.provider.statFile({ | |
| args: { path: normalizedTargetPath }, | |
| }); | |
| if (stat?.type === 'directory') { | |
| this.fileSync.updateDirectoryCache(normalizedTargetPath); | |
| // Optional: list + seed child entries if provider doesn't emit add events for children. | |
| // (Rely on watcher by default.) | |
| } else { | |
| const copiedFile = await this.readRemoteFile(normalizedTargetPath); | |
| if (copiedFile) { | |
| this.fileSync.updateCache(copiedFile); | |
| } | |
| } |
🤖 Prompt for AI Agents
In apps/web/client/src/components/store/editor/sandbox/index.ts around lines 659
to 664, the code calls readRemoteFile after copyFiles and skips cache updates
when readRemoteFile returns null, which leaves the cache stale for directory
targets; change the logic so that if readRemoteFile returns null you detect
whether the normalizedTargetPath is a directory (e.g., via readRemoteDirectory
or a metadata/stat call), and when it is update the fileSync cache with a
directory entry (and for recursive copies, fetch and update the directory
contents recursively or refresh the parent directory listing) so the cache
reflects the newly copied directory structure.
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
| const remoteFile = await this.readRemoteFile(normalizedPath); | ||
| if (!remoteFile) { | ||
| console.error(`File content for ${normalizedPath} not found in remote`); | ||
| return; | ||
| } |
There was a problem hiding this comment.
Logic error: The code returns early if remoteFile is falsy, but the comment on line 552 states it should 'always update cache regardless of file type'. This creates inconsistent behavior where some file change events are silently ignored. The function should either handle the null case appropriately (perhaps by removing the file from cache) or the early return should be reconsidered to match the stated behavior.
| const remoteFile = await this.readRemoteFile(normalizedPath); | |
| if (!remoteFile) { | |
| console.error(`File content for ${normalizedPath} not found in remote`); | |
| return; | |
| } | |
| const remoteFile = await this.readRemoteFile(normalizedPath); | |
| if (!remoteFile) { | |
| console.error(`File content for ${normalizedPath} not found in remote`); | |
| // Remove file from cache instead of returning early | |
| this.removeFromCache(normalizedPath); | |
| return; | |
| } |
Spotted by Diamond
Is this helpful? React 👍 or 👎 to let us know.
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
| async (isIndexingFiles) => { | ||
| if (!isIndexingFiles) { | ||
| await this.scanImages(); | ||
| () => { |
There was a problem hiding this comment.
The reaction’s tracking function returns only isIndexing and isIndexed. If you intend to trigger a scan when the images tab is activated, consider including leftPanelTab in the tracked object.
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
apps/web/client/src/components/store/editor/image/index.ts (1)
200-204: Log level/message tweak for unavailable file list.When files is undefined, it’s a transient state; prefer warn and clearer wording.
- if (!files) { - console.error('No files found in image folder'); - return; - } + if (!files) { + console.warn('Sandbox files list unavailable; skipping image scan'); + return; + }
📜 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/folder/index.tsx(3 hunks)apps/web/client/src/app/project/[id]/_components/left-panel/image-tab/providers/folder-provider.tsx(1 hunks)apps/web/client/src/components/store/editor/image/index.ts(4 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
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 instead
Files:
apps/web/client/src/app/project/[id]/_components/left-panel/image-tab/providers/folder-provider.tsxapps/web/client/src/app/project/[id]/_components/left-panel/image-tab/folder/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 instead
Files:
apps/web/client/src/app/project/[id]/_components/left-panel/image-tab/providers/folder-provider.tsxapps/web/client/src/components/store/editor/image/index.tsapps/web/client/src/app/project/[id]/_components/left-panel/image-tab/folder/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
Files:
apps/web/client/src/app/project/[id]/_components/left-panel/image-tab/providers/folder-provider.tsxapps/web/client/src/app/project/[id]/_components/left-panel/image-tab/folder/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/providers/folder-provider.tsxapps/web/client/src/components/store/editor/image/index.tsapps/web/client/src/app/project/[id]/_components/left-panel/image-tab/folder/index.tsx
🧠 Learnings (1)
📚 Learning: 2025-09-14T01:44:21.199Z
Learnt from: CR
PR: onlook-dev/onlook#0
File: AGENTS.md:0-0
Timestamp: 2025-09-14T01:44:21.199Z
Learning: Applies to apps/web/client/src/app/**/*.tsx : Default to Server Components; add 'use client' when using events, state/effects, browser APIs, or client‑only libraries
Applied to files:
apps/web/client/src/app/project/[id]/_components/left-panel/image-tab/folder/index.tsx
🧬 Code graph analysis (2)
apps/web/client/src/components/store/editor/image/index.ts (3)
apps/web/client/src/components/store/editor/sandbox/index.ts (1)
files(319-321)packages/constants/src/editor.ts (1)
DefaultSettings(38-69)packages/utility/src/file.ts (1)
isImageFile(101-104)
apps/web/client/src/app/project/[id]/_components/left-panel/image-tab/folder/index.tsx (2)
packages/ui/src/components/tooltip.tsx (4)
Tooltip(72-72)TooltipTrigger(72-72)TooltipPortal(72-72)TooltipContent(72-72)packages/ui/src/components/button.tsx (1)
Button(57-57)
🔇 Additional comments (4)
apps/web/client/src/components/store/editor/image/index.ts (2)
2-2: Good use of type-only imports.Keeps runtime clean and improves perf/emit.
195-199: Re-entrancy guard is solid.The try/finally ensures _isScanning resets on all paths.
apps/web/client/src/app/project/[id]/_components/left-panel/image-tab/folder/index.tsx (2)
1-2: Correctly marked as a Client Component.Required due to state, effects, and event handlers.
255-255: Icon sizing change looks consistent.No issues spotted.
| <Tooltip> | ||
| <TooltipTrigger asChild> | ||
| <Button | ||
| variant={'default'} | ||
| size={'icon'} | ||
| className="p-2 w-fit h-fit text-foreground-primary border-border-primary hover:border-border-onlook bg-background-secondary hover:bg-background-onlook border" | ||
| onClick={() => handleCreateFolder(currentFolder)} | ||
| disabled={isAnyOperationLoading} | ||
| > | ||
| <Icons.DirectoryPlus className="h-4 w-4" /> | ||
| </Button> | ||
| </TooltipTrigger> | ||
| <TooltipPortal> | ||
| <TooltipContent> | ||
| <p>Create a folder</p> | ||
| </TooltipContent> | ||
| </TooltipPortal> | ||
| </Tooltip> |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Localize tooltip text and add an accessible label to the icon-only “Create folder” button.
Complies with next-intl/i18n guideline and improves a11y.
Apply this diff:
- <Tooltip>
+ <Tooltip>
<TooltipTrigger asChild>
<Button
variant={'default'}
size={'icon'}
className="p-2 w-fit h-fit text-foreground-primary border-border-primary hover:border-border-onlook bg-background-secondary hover:bg-background-onlook border"
onClick={() => handleCreateFolder(currentFolder)}
disabled={isAnyOperationLoading}
+ aria-label={t('images.createFolder')}
>
<Icons.DirectoryPlus className="h-4 w-4" />
</Button>
</TooltipTrigger>
<TooltipPortal>
- <TooltipContent>
- <p>Create a folder</p>
+ <TooltipContent>
+ <p>{t('images.createFolder')}</p>
</TooltipContent>
</TooltipPortal>
</Tooltip>Add this outside the shown range:
import { useTranslations } from 'next-intl';
// inside Folder component:
const t = useTranslations();If creation is intended only at the root, gate the button with currentFolder === rootDir as well.
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file='apps/web/client/src/app/project/[id]/_components/left-panel/image-tab/folder/index.tsx'
echo "LOOKUP: $file"
if [ -f "$file" ]; then
echo "=== FILE FOUND ==="
sed -n '1,320p' "$file"
else
echo "=== FILE NOT FOUND AT EXACT PATH ==="
echo "Searching for likely files..."
rg -n --hidden --glob '!node_modules' "image-tab" -S || true
rg -n --hidden --glob '!node_modules' "folder/index.tsx" -S || true
rg -n --hidden --glob '!node_modules' "Create a folder" -S || true
rg -n --hidden --glob '!node_modules' "TooltipContent" -S || true
rg -n --hidden --glob '!node_modules' "useTranslations" -S || true
rg -n --hidden --glob '!node_modules' "aria-label" -S || true
rg -n --hidden --glob '!node_modules' "handleCreateFolder" -S || true
rg -n --hidden --glob '!node_modules' "currentFolder" -S || true
fiLength of output: 12963
Localize tooltip text and add accessible label to the icon-only "Create folder" button.
- Add: import { useTranslations } from 'next-intl'; and inside Folder: const t = useTranslations();
- Replace TooltipContent
Create a folder
with{t('images.createFolder')}
and add aria-label={t('images.createFolder')} to the icon-only Create Folder Button. - Gate the header Create Folder button so it only renders when currentFolder === rootDir (or use the existing showCreateButton).
- Also localize other visible strings ("Search images" placeholder, "Loading images...", "Refresh Images", "Upload an image") and add aria-labels to other icon-only buttons (Back/ArrowLeft, Refresh, Upload).
🤖 Prompt for AI Agents
In
apps/web/client/src/app/project/[id]/_components/left-panel/image-tab/folder/index.tsx
around lines 211-228, tooltip text is hardcoded and the icon-only "Create
folder" button lacks an accessible label and conditional rendering; import
useTranslations from 'next-intl' and instantiate it (const t =
useTranslations()), replace the hardcoded TooltipContent <p>Create a folder</p>
with the localized key {t('images.createFolder')}, add
aria-label={t('images.createFolder')} to the icon-only Create Folder Button, and
gate that Create Folder button so it only renders when currentFolder === rootDir
or when showCreateButton is true; additionally, localize visible strings such as
the "Search images" placeholder, "Loading images...", "Refresh Images", and
"Upload an image" using t(...) and add aria-labels to other icon-only buttons
(Back/ArrowLeft, Refresh, Upload) to ensure accessibility.
| <Tooltip> | ||
| <TooltipTrigger asChild> | ||
| <Button | ||
| variant={'default'} | ||
| size={'icon'} | ||
| className="p-2 w-fit h-fit text-foreground-primary border-border-primary hover:border-border-onlook bg-background-secondary hover:bg-background-onlook border" | ||
| onClick={() => editorEngine.image.scanImages()} | ||
| > | ||
| <Icons.Reload className="w-4 h-4" /> | ||
| </Button> | ||
| </TooltipTrigger> | ||
| <TooltipPortal> | ||
| <TooltipContent> | ||
| <p>Refresh Images</p> | ||
| </TooltipContent> | ||
| </TooltipPortal> | ||
| </Tooltip> |
There was a problem hiding this comment.
Disable “Refresh Images” while scanning; localize text; add aria-label; avoid unhandled promise.
Prevents spam-clicks; aligns with i18n; clarifies intent.
Apply this diff:
<Tooltip>
<TooltipTrigger asChild>
<Button
variant={'default'}
size={'icon'}
className="p-2 w-fit h-fit text-foreground-primary border-border-primary hover:border-border-onlook bg-background-secondary hover:bg-background-onlook border"
- onClick={() => editorEngine.image.scanImages()}
+ onClick={() => { void editorEngine.image.scanImages(); }}
+ disabled={isAnyOperationLoading || editorEngine.image.isScanning}
+ aria-label={t('images.refresh')}
>
<Icons.Reload className="w-4 h-4" />
</Button>
</TooltipTrigger>
<TooltipPortal>
<TooltipContent>
- <p>Refresh Images</p>
+ <p>{t('images.refresh')}</p>
</TooltipContent>
</TooltipPortal>
</Tooltip>📝 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.
| <Tooltip> | |
| <TooltipTrigger asChild> | |
| <Button | |
| variant={'default'} | |
| size={'icon'} | |
| className="p-2 w-fit h-fit text-foreground-primary border-border-primary hover:border-border-onlook bg-background-secondary hover:bg-background-onlook border" | |
| onClick={() => editorEngine.image.scanImages()} | |
| > | |
| <Icons.Reload className="w-4 h-4" /> | |
| </Button> | |
| </TooltipTrigger> | |
| <TooltipPortal> | |
| <TooltipContent> | |
| <p>Refresh Images</p> | |
| </TooltipContent> | |
| </TooltipPortal> | |
| </Tooltip> | |
| <Tooltip> | |
| <TooltipTrigger asChild> | |
| <Button | |
| variant={'default'} | |
| size={'icon'} | |
| className="p-2 w-fit h-fit text-foreground-primary border-border-primary hover:border-border-onlook bg-background-secondary hover:bg-background-onlook border" | |
| onClick={() => { void editorEngine.image.scanImages(); }} | |
| disabled={isAnyOperationLoading || editorEngine.image.isScanning} | |
| aria-label={t('images.refresh')} | |
| > | |
| <Icons.Reload className="w-4 h-4" /> | |
| </Button> | |
| </TooltipTrigger> | |
| <TooltipPortal> | |
| <TooltipContent> | |
| <p>{t('images.refresh')}</p> | |
| </TooltipContent> | |
| </TooltipPortal> | |
| </Tooltip> |
🤖 Prompt for AI Agents
In
apps/web/client/src/app/project/[id]/_components/left-panel/image-tab/folder/index.tsx
around lines 229-245, the Refresh Images button should be disabled while a scan
is in progress, use a localized tooltip/aria-label, and avoid an unhandled
promise from scanImages. Update the Button to set
disabled={editorEngine.image.isScanning} (or the appropriate scanning flag),
replace the hardcoded tooltip and aria-label with the i18n call (e.g.
t('refreshImages')) and add aria-label using that same localized string, and
change the onClick to an async handler that awaits
editorEngine.image.scanImages() inside try/catch (or attach .catch()) so errors
are handled. Ensure the tooltip content uses the localized string as well.
| // Use the image manager's scanned image paths instead of sandbox files | ||
| return editorEngine.image.imagePaths.filter(image => { | ||
| if (image.startsWith(folder.fullPath)) { | ||
| // Check if this is a direct child (not in a subdirectory) | ||
| const relativePath = image.slice(folder.fullPath.length); | ||
| // Remove leading slash if present | ||
| const cleanRelativePath = relativePath.startsWith('/') ? relativePath.slice(1) : relativePath; | ||
| // Only include if it's a direct child (no additional path separators) | ||
| if (!cleanRelativePath.includes('/')) { | ||
| return isImageFile(image); | ||
| } | ||
| return !cleanRelativePath.includes('/') && cleanRelativePath.length > 0; | ||
| } | ||
| return false; | ||
| }); | ||
| }, [editorEngine.activeSandbox.files]); | ||
| }, [editorEngine.image.imagePaths]); | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Fix direct-child check: require a path separator after the folder to prevent “publications” matching “public”.
Also fine to rely on image.imagePaths (already filtered to images).
Apply this diff:
- return editorEngine.image.imagePaths.filter(image => {
- if (image.startsWith(folder.fullPath)) {
- // Check if this is a direct child (not in a subdirectory)
- const relativePath = image.slice(folder.fullPath.length);
- // Remove leading slash if present
- const cleanRelativePath = relativePath.startsWith('/') ? relativePath.slice(1) : relativePath;
- // Only include if it's a direct child (no additional path separators)
- return !cleanRelativePath.includes('/') && cleanRelativePath.length > 0;
- }
- return false;
- });
+ return editorEngine.image.imagePaths.filter(image => {
+ const prefix = `${folder.fullPath}/`;
+ if (!image.startsWith(prefix)) return false;
+ const cleanRelativePath = image.slice(prefix.length);
+ return !cleanRelativePath.includes('/') && cleanRelativePath.length > 0;
+ });Please confirm activeSandbox emits image paths without a leading slash (e.g., "public/foo.png"). If it can include "/public/...", we should normalize before comparisons.
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "PWD: $(pwd)"
echo
echo "1) Find occurrences of 'imagePaths' and editorEngine usage"
rg -n --hidden -S "imagePaths" -g '!.git' -C3 || true
rg -n --hidden -S "\\beditorEngine\\b" -g '!.git' -C3 || true
echo
echo "2) Show the folder-provider file (if present)"
FILE="apps/web/client/src/app/project/[id]/_components/left-panel/image-tab/providers/folder-provider.tsx"
if [ -f "$FILE" ]; then
echo "File found: $FILE"
echo "---- begin file (1..520) ----"
sed -n '1,520p' "$FILE"
echo "---- end file ----"
else
echo "File not found at exact path: $FILE"
echo "Searching for similarly named files..."
rg -n --hidden -S "folder-provider\\.tsx|left-panel/image-tab/providers" -g '!.git' -C2 || true
fi
echo
echo "3) Search where imagePaths are populated / normalized"
rg -n --hidden -S "imagePaths\\s*[:=]" -g '!.git' -C3 || true
rg -n --hidden -S "setImagePaths|addImagePaths|push\\([^)]*image" -g '!.git' -C3 || true
rg -n --hidden -S "scanImages|scan.*image|listImages|list_images|list_images\\(|walkSync|readdirSync|glob\\(" -g '!.git' -C3 || true
echo
echo "4) Search for 'activeSandbox' and any code that emits sandbox paths"
rg -n --hidden -S "activeSandbox|sandbox" -g '!.git' -C3 || true
echo
echo "5) Search for '/public' or leading-slash path building / normalization"
rg -n --hidden -S "/public|\"/public|'\/public|^/public" -g '!.git' -C3 || true
rg -n --hidden -S "replace\\(\\s*['\"]/|^\\s*['\"]/|^\\s*/" -g '!.git' -C3 || true
echo
echo "Done."Length of output: 15745
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "PWD: $(pwd)"
echo
echo "1) Locate editor store / useEditorEngine"
rg -n --hidden -uu "useEditorEngine" -S -g '!.git' -C3 || true
echo
echo "2) List files under components/store (if present)"
ls -la apps/web/client/src/components/store || true
echo
echo "3) Show files in that directory (editor.*) if present"
for f in apps/web/client/src/components/store/editor* apps/web/client/src/components/store/*editor*; do
if [ -f "$f" ]; then
echo "---- FILE: $f ----"
sed -n '1,240p' "$f"
echo "---- END FILE: $f ----"
echo
fi
done
echo "4) Grep for imagePaths, activeSandbox, directories occurrences (fallback to grep)"
echo
grep -RIn --line-number "imagePaths" || true
echo
grep -RIn --line-number "activeSandbox" || true
echo
grep -RIn --line-number "directories" || true
echo
echo "Done."Length of output: 28207
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Printing image store:"
echo "----------------------------------------"
sed -n '1,320p' apps/web/client/src/components/store/editor/image/index.ts || true
echo
echo "Printing sandbox store (to see how files/directories are listed):"
echo "----------------------------------------"
sed -n '1,520p' apps/web/client/src/components/store/editor/sandbox/index.ts || true
echo
echo "Search for IMAGE_FOLDER / DefaultSettings definitions:"
rg -n --hidden "IMAGE_FOLDER|DefaultSettings" || true
echo
echo "Show any files that mention IMAGE_FOLDER:"
rg -n --hidden --line-number "IMAGE_FOLDER" -S || trueLength of output: 129954
Require a trailing '/' when matching direct children (fix ambiguous prefix match)
- Apply the suggested diff in apps/web/client/src/app/project/[id]/_components/left-panel/image-tab/providers/folder-provider.tsx: match using const prefix =
${folder.fullPath}/and slice by prefix.length so "publications" will not match "public". - Verified: DefaultSettings.IMAGE_FOLDER === 'public' and editorEngine.image.imagePaths are populated from activeSandbox.files as "public/..." (no leading slash), so no extra normalization is required; if a provider can emit "/public/...", strip a leading '/' before comparing.
🤖 Prompt for AI Agents
In
apps/web/client/src/app/project/[id]/_components/left-panel/image-tab/providers/folder-provider.tsx
around lines 391 to 404, the current prefix check can produce ambiguous matches
(e.g., "public" matching "publications"); change the logic to build a prefix
const prefix = `${folder.fullPath}/` and compare against a normalized image path
(strip a leading '/' if present) using image.startsWith(prefix), then slice by
prefix.length to get the relative child name and only include entries without
additional '/' and with length > 0; ensure you normalize folder.fullPath
similarly if providers may emit a leading '/'.
| () => { | ||
| return { | ||
| isIndexing: this.editorEngine.activeSandbox.isIndexing, | ||
| isIndexed: this.editorEngine.activeSandbox.isIndexed, | ||
| }; | ||
| }, | ||
| (sandboxStatus) => { | ||
| if (this.editorEngine.state.leftPanelTab !== LeftPanelTabValue.IMAGES) { | ||
| return; | ||
| } | ||
| if (sandboxStatus.isIndexed && !sandboxStatus.isIndexing) { | ||
| this.scanImages(); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Reaction misses leftPanelTab in its tracked derivation; IMAGES tab activation won’t trigger a scan.
Include the tab in the reaction’s data source so switching to the IMAGES tab fires scanImages when indexed.
Apply this diff:
- this.indexingReactionDisposer = reaction(
- () => {
- return {
- isIndexing: this.editorEngine.activeSandbox.isIndexing,
- isIndexed: this.editorEngine.activeSandbox.isIndexed,
- };
- },
- (sandboxStatus) => {
- if (this.editorEngine.state.leftPanelTab !== LeftPanelTabValue.IMAGES) {
- return;
- }
- if (sandboxStatus.isIndexed && !sandboxStatus.isIndexing) {
- this.scanImages();
- }
- },
- );
+ this.indexingReactionDisposer = reaction(
+ () => ({
+ tab: this.editorEngine.state.leftPanelTab,
+ isIndexing: this.editorEngine.activeSandbox.isIndexing,
+ isIndexed: this.editorEngine.activeSandbox.isIndexed,
+ }),
+ ({ tab, isIndexing, isIndexed }) => {
+ if (tab !== LeftPanelTabValue.IMAGES) return;
+ if (isIndexed && !isIndexing) {
+ void this.scanImages();
+ }
+ },
+ );📝 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.
| () => { | |
| return { | |
| isIndexing: this.editorEngine.activeSandbox.isIndexing, | |
| isIndexed: this.editorEngine.activeSandbox.isIndexed, | |
| }; | |
| }, | |
| (sandboxStatus) => { | |
| if (this.editorEngine.state.leftPanelTab !== LeftPanelTabValue.IMAGES) { | |
| return; | |
| } | |
| if (sandboxStatus.isIndexed && !sandboxStatus.isIndexing) { | |
| this.scanImages(); | |
| } | |
| this.indexingReactionDisposer = reaction( | |
| () => ({ | |
| tab: this.editorEngine.state.leftPanelTab, | |
| isIndexing: this.editorEngine.activeSandbox.isIndexing, | |
| isIndexed: this.editorEngine.activeSandbox.isIndexed, | |
| }), | |
| ({ tab, isIndexing, isIndexed }) => { | |
| if (tab !== LeftPanelTabValue.IMAGES) return; | |
| if (isIndexed && !isIndexing) { | |
| void this.scanImages(); | |
| } | |
| }, | |
| ); |
🤖 Prompt for AI Agents
In apps/web/client/src/components/store/editor/image/index.ts around lines 21 to
33, the reaction only tracks sandbox indexing state so switching the left panel
to IMAGES won’t trigger scanImages; update the derivation to also return the
current leftPanelTab (e.g., include leftPanelTab:
this.editorEngine.state.leftPanelTab) and adjust the reaction callback to read
that field so changes to the tab cause the reaction to re-run and call
this.scanImages() when the sandbox is indexed and not indexing.
| this._imagePaths = files.filter((file: string) => file.startsWith(DefaultSettings.IMAGE_FOLDER) && isImageFile(file)); | ||
| } catch (error) { |
There was a problem hiding this comment.
Avoid false positives: enforce folder boundary when filtering (public vs publications).
Use ${DefaultSettings.IMAGE_FOLDER}/ as the prefix.
Apply this diff:
- this._imagePaths = files.filter((file: string) => file.startsWith(DefaultSettings.IMAGE_FOLDER) && isImageFile(file));
+ const prefix = `${DefaultSettings.IMAGE_FOLDER}/`;
+ this._imagePaths = files.filter((file: string) => file.startsWith(prefix) && isImageFile(file));📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| this._imagePaths = files.filter((file: string) => file.startsWith(DefaultSettings.IMAGE_FOLDER) && isImageFile(file)); | |
| } catch (error) { | |
| const prefix = `${DefaultSettings.IMAGE_FOLDER}/`; | |
| this._imagePaths = files.filter((file: string) => file.startsWith(prefix) && isImageFile(file)); | |
| } catch (error) { |
🤖 Prompt for AI Agents
In apps/web/client/src/components/store/editor/image/index.ts around lines
209-210, the filter currently uses file.startsWith(DefaultSettings.IMAGE_FOLDER)
which can match sibling folders (causing false positives); change the prefix
check to require the folder boundary by using
file.startsWith(`${DefaultSettings.IMAGE_FOLDER}/`) so only files directly under
the IMAGE_FOLDER path are accepted, and keep the existing isImageFile(file)
check.
Description
Related Issues
Type of Change
Testing
Screenshots (if applicable)
Additional Notes
Important
Improves file caching and synchronization in the editor, enhancing reliability and performance for image and JSX file handling.
sandbox/index.ts.sandbox/index.ts.sandbox/index.ts.image/index.ts.folder-provider.tsxandimage/index.ts.This description was created by
for c60429f. You can customize this summary. It will automatically update as commits are pushed.
Summary by CodeRabbit
New Features
Bug Fixes
Performance
Stability