-
Notifications
You must be signed in to change notification settings - Fork 517
Fix: Delay in Image Visibility After Adding Large Folder (#591) #660
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
base: main
Are you sure you want to change the base?
Conversation
WalkthroughThe backend image processing refactors from bulk multi-image preparation to per-image preparation with batch insertion, committing to the database every 50 records. The frontend adds polling to track image count updates after folder addition, exposing an Changes
Sequence DiagramssequenceDiagram
participant Client
participant Backend
participant DB
Client->>Backend: Request batch image processing
loop Per image in folder
Backend->>Backend: Prepare single image record
Backend->>Backend: Add to batch accumulator
alt Batch full (50 records)
Backend->>DB: Bulk insert batch
Backend->>Backend: Clear accumulator, reset count
DB-->>Backend: Success
end
end
alt Remaining records
Backend->>DB: Flush final batch
DB-->>Backend: Success
end
Backend-->>Client: Processing complete
sequenceDiagram
participant User
participant Frontend
participant Backend
participant QueryClient
User->>Frontend: Add folder
Frontend->>Backend: POST /add_folder
Backend-->>Frontend: Success
Frontend->>QueryClient: Start polling interval
loop Every ~1 second
Frontend->>Backend: GET image count
Backend-->>Frontend: Current image count
Frontend->>Frontend: Check if count stable
alt Count stable (3 increments)
Frontend->>QueryClient: Stop polling
Frontend->>QueryClient: Clear interval
end
end
Frontend-->>User: Folder added, images loading
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related issues
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (4)
backend/app/utils/images.py (2)
41-83: Batching logic looks sound; consider propagating DB insertion failuresThe new batching loop (with
BATCH_SIZE = 50) correctly:
- Prepares records per image,
- Flushes on batch size or last image, and
- Keeps a running
total_processedcount.Two improvements worth considering:
Check
db_bulk_insert_imagesresult
Right now the boolean return is ignored, so a failed insert still incrementstotal_processedand the function ultimately returnsTrue. If inserts can fail (e.g., DB unavailable), you may want to:
- Log a more explicit error, and
- Either break out and return
Falseor at least avoid incrementingtotal_processedfor failed batches.Make
BATCH_SIZEconfigurable (optional)
If you anticipate very large folders or different storage backends, makingBATCH_SIZEa module-level constant or reading it from config would ease tuning without code changes.Also applies to: 93-99
154-188: Single-image preparation helper is clear; tighten typing and edge handlingThe new
image_util_prepare_single_image_recordshelper is straightforward and reuses existing thumbnail/metadata utilities. A couple of refinements:
- Type hints: Prefer more explicit typing, e.g.
- Return type:
Optional[Dict[str, Any]]instead ofDict | None.- This matches the actual payload shape and plays nicer with type checkers.
- Folder ID falsy check:
if not folder_id:will treat0as “no folder”, even thoughimage_util_find_folder_id_for_imageis annotated to returnint. If0can’t happen in practice that’s fine, but consider explicitly checkingis Nonefor future-proofing.These don’t change behavior but improve static safety and clarity.
frontend/src/hooks/useFolder.ts (2)
17-21: Global polling refs work butNodeJS.Timeouttype may be brittleStoring the interval and counters at module scope is a reasonable way to keep polling alive across hook remounts and ensure only one active interval.
For stronger TS portability (browser + Tauri + Node), consider changing the interval type to:
const pollIntervalRef: { current: ReturnType<typeof setInterval> | null } = { current: null, };This avoids depending on the
NodeJSnamespace and still works withclearInterval.
26-75: Polling logic is reasonable; guard against never-ending polling if images data stays emptyThe success callback and follow-up
useEffectcorrectly:
- Immediately invalidate
['images']to show the first batch,- Clear any prior interval before starting a new one, and
- Stop polling after three consecutive stable (> 0) image-count readings.
Two edge cases to consider:
When
['images']data shape differs or is never fetched
You assumequeryClient.getQueryData(['images'])returns something like{ data: any[] }. If:
- The query isn’t mounted yet, or
- The data shape is different,
currentImageCountwill remain0, so the> 0condition never holds andstableCountIterationsRefnever increments. Result: polling continues indefinitely every 2 seconds.You could:
- Add a max-iterations or timeout safeguard, or
- Explicitly bail out if
imagesDataisundefinedfor several cycles.Logging-only effect
TheuseEffectthat logs success and clears polling on error is fine, but make sure future refactors don’t start mutating state that depends on stale closures there (right now it’s only reading global refs, so you’re safe).Returning
isAddingFolder: addFolderPendingis a nice touch for the UI to show a pending state.Also applies to: 77-90, 110-114
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
backend/app/utils/images.py(4 hunks)frontend/src/api/api-functions/folders.ts(1 hunks)frontend/src/hooks/useFolder.ts(3 hunks)frontend/src/hooks/useFolderOperations.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
backend/app/utils/images.py (1)
backend/app/database/images.py (1)
db_bulk_insert_images(89-120)
🔇 Additional comments (2)
frontend/src/api/api-functions/folders.ts (1)
74-84: Formatting-only change here; behavior is unchangedThe return shape and logic of
getFoldersTaggingStatusremain the same; this closing brace tweak has no functional impact.frontend/src/hooks/useFolderOperations.tsx (1)
163-179: No functional change in default export
useFolderOperationsremains both a named and default export; this line change is effectively formatting-only and doesn’t alter runtime behavior.
Fix for Issue #591 — Delay in Image Visibility After Adding Folder
fixes #591
When a Large folder with many images (~200) is added, images appear on the home tab only after all thumbnails are generated and inserted in the database. This created a visible delay of ~8 seconds.
✔ What I changed:
🔍 How I tested:
🚀 Result:
Images now appear gradually and the user no longer experiences long delays.
Please review and let me know if any adjustments are needed.
Summary by CodeRabbit
New Features
Bug Fixes
Performance
✏️ Tip: You can customize this high-level summary in your review settings.