feat(#723): Implement Memories Page#827
feat(#723): Implement Memories Page#827Prashik-Sasane wants to merge 1 commit intoAOSSIE-Org:mainfrom
Conversation
|
|
WalkthroughA new Memories feature is introduced with a backend endpoint that categorizes images into memory types (SPOTLIGHT, REVISIT, WEEKEND, YEAR), a React-based Memories page with API integration, a refactored story-style MediaView component with auto-play and progress tracking, complete Tauri desktop app scaffolding, and extensive MkDocs-based documentation site generation. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Frontend as Frontend (React)
participant API as Backend API
participant DB as Database
User->>Frontend: Opens Memories page
Frontend->>API: GET /memories
API->>DB: db_get_all_images()
DB-->>API: All images with dates/metadata
rect rgb(200, 220, 255)
Note over API: Categorize images:<br/>SPOTLIGHT (today)<br/>REVISIT (same day/month prev year)<br/>WEEKEND (Sat/Sun)<br/>YEAR (grouped by year)
end
API->>API: Build Memory objects<br/>with 6 image IDs each
API-->>Frontend: { success: true, data: memories }
Frontend->>Frontend: Display memories grid
User->>Frontend: Click on memory
Frontend->>Frontend: Set activeMemory,<br/>filter images
Frontend->>Frontend: Dispatch filtered images<br/>to image slice
rect rgb(200, 255, 220)
Note over Frontend: Story Mode View<br/>Auto-play with pause<br/>Progress bar animation<br/>Keyboard/click nav
end
Frontend->>Frontend: Render StoryProgressBar<br/>+ ImageViewer<br/>+ Title/Subtitle
Frontend->>Frontend: Auto-advance slides<br/>every 4s (if not paused)
User->>Frontend: Close memory view
Frontend->>Frontend: Return to grid
Estimated code review effort🎯 4 (Complex) | ⏱️ ~70 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
Prashik-Sasane
changed the title
|
|
1 similar 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: 16
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🟡 Minor comments (6)
site/sitemap.xml-1-3 (1)
1-3: Empty sitemap provides no SEO value.The sitemap contains no URL entries. While structurally valid, it won't help search engines discover your documentation pages.
If the documentation site has published pages, would you like me to help generate a script to populate this sitemap with the appropriate URLs from your site structure?
site/assets/stylesheets/swagger-ui-dark.css-599-602 (1)
599-602: Remove duplicate background declaration.The
backgroundproperty is declared twice for the same selector. The second declaration on line 600 will override the first on line 599. If both are needed for fallback purposes, consider adding a comment explaining why; otherwise, remove the duplicate.🔎 Proposed fix
If only one is needed:
.swagger-ui select { - background: url("data:image/svg+xml;charset=utf-8,<svg xmlns=\"http://www.w3.org/2000/svg\" viewBox=\"0 0 20 20\"><path d=\"M13.418 7.859a.695.695 0 01.978 0 .68.68 0 010 .969l-3.908 3.83a.697.697 0 01-.979 0l-3.908-3.83a.68.68 0 010-.969.695.695 0 01.978 0L10 11l3.418-3.141z\"/></svg>") right 10px center/20px no-repeat #212121; background: url(data:image/svg+xml;base64,PD94bWwgdmVyc2lvbj0iMS4wIiBlbmNvZGluZz0iVVRGLTgiIHN0YW5kYWxvbmU9Im5vIj8+CjxzdmcKICAgeG1sbnM6ZGM9Imh0dHA6Ly9wdXJsLm9yZy9kYy9lbGVtZW50cy8xLjEvIgogICB4bWxuczpjYz0iaHR0cDovL2NyZWF0aXZlY29tbW9ucy5vcmcvbnMjIgogICB4bWxuczpyZGY9Imh0dHA6Ly93d3cudzMub3JnLzE5OTkvMDIvMjItcmRmLXN5bnRheC1ucyMiCiAgIHhtbG5zOnN2Zz0iaHR0cDovL3d3dy53My5vcmcvMjAwMC9zdmciCiAgIHhtbG5zPSJodHRwOi8vd3d3LnczLm9yZy8yMDAwL3N2ZyIKICAgeG1sbnM6c29kaXBvZGk9Imh0dHA6Ly9zb2RpcG9kaS5zb3VyY2Vmb3JnZS5uZXQvRFREL3NvZGlwb2RpLTAuZHRkIgogICB4bWxuczppbmtzY2FwZT0iaHR0cDovL3d3dy5pbmtzY2FwZS5vcmcvbmFtZXNwYWNlcy9pbmtzY2FwZSIKICAgaW5rc2NhcGU6dmVyc2lvbj0iMS4wICg0MDM1YTRmYjQ5LCAyMDIwLTA1LTAxKSIKICAgc29kaXBvZGk6ZG9jbmFtZT0iZG93bmxvYWQuc3ZnIgogICBpZD0ic3ZnNCIKICAgdmVyc2lvbj0iMS4xIgogICB2aWV3Qm94PSIwIDAgMjAgMjAiPgogIDxtZXRhZGF0YQogICAgIGlkPSJtZXRhZGF0YTEwIj4KICAgIDxyZGY6UkRGPgogICAgICA8Y2M6V29yawogICAgICAgICByZGY6YWJvdXQ9IiI+CiAgICAgICAgPGRjOmZvcm1hdD5pbWFnZS9zdmcreG1sPC9kYzpmb3JtYXQ+CiAgICAgICAgPGRjOnR5cGUKICAgICAgICAgICByZGY6cmVzb3VyY2U9Imh0dHA6Ly9wdXJsLm9yZy9kYy9kY21pdHlwZS9TdGlsbEltYWdlIiAvPgogICAgICA8L2NjOldvcms+CiAgICA8L3JkZjpSREY+CiAgPC9tZXRhZGF0YT4KICA8ZGVmcwogICAgIGlkPSJkZWZzOCIgLz4KICA8c29kaXBvZGk6bmFtZWR2aWV3CiAgICAgaW5rc2NhcGU6Y3VycmVudC1sYXllcj0ic3ZnNCIKICAgICBpbmtzY2FwZTp3aW5kb3ctbWF4aW1pemVkPSIxIgogICAgIGlua3NjYXBlOndpbmRvdy15PSItOSIKICAgICBpbmtzY2FwZTp3aW5kb3cteD0iLTkiCiAgICAgaW5rc2NhcGU6Y3k9IjEwIgogICAgIGlua3NjYXBlOmN4PSIxMCIKICAgICBpbmtzY2FwZTp6b29tPSI0MS41IgogICAgIHNob3dncmlkPSJmYWxzZSIKICAgICBpZD0ibmFtZWR2aWV3NiIKICAgICBpbmtzY2FwZTp3aW5kb3ctaGVpZ2h0PSIxMDAxIgogICAgIGlua3NjYXBlOndpbmRvdy13aWR0aD0iMTkyMCIKICAgICBpbmtzY2FwZTpwYWdlc2hhZG93PSIyIgogICAgIGlua3NjYXBlOnBhZ2VvcGFjaXR5PSIwIgogICAgIGd1aWRldG9sZXJhbmNlPSIxMCIKICAgICBncmlkdG9sZXJhbmNlPSIxMCIKICAgICBvYmplY3R0b2xlcmFuY2U9IjEwIgogICAgIGJvcmRlcm9wYWNpdHk9IjEiCiAgICAgYm9yZGVyY29sb3I9IiM2NjY2NjYiCiAgICAgcGFnZWNvbG9yPSIjZmZmZmZmIiAvPgogIDxwYXRoCiAgICAgc3R5bGU9ImZpbGw6I2ZmZmZmZiIKICAgICBpZD0icGF0aDIiCiAgICAgZD0iTTEzLjQxOCA3Ljg1OWEuNjk1LjY5NSAwIDAxLjk3OCAwIC42OC42OCAwIDAxMCAuOTY5bC0zLjkwOCAzLjgzYS42OTcuNjk3IDAgMDEtLjk3OSAwbC0zLjkwOC0zLjgzYS42OC42OCAwIDAxMC0uOTY5LjY5NS42OTUgMCAwMS45NzggMEwxMCAxMWwzLjQxOC0zLjE0MXoiIC8+Cjwvc3ZnPgo=) right 10px center/20px no-repeat #1e2129; border: 2px solid #41444e; }Committable suggestion skipped: line range outside the PR's diff.
frontend/src/components/Media/MediaView.tsx-114-118 (1)
114-118: Mouse-based pause interaction lacks keyboard accessibility.The
onMouseDown/onMouseUphandlers provide pause functionality for mouse users, but keyboard users cannot access this. The Space key toggles pause, but there's no visual indication of this functionality. Consider adding an aria-label or visible hint.frontend/src/components/Media/StoryProgressBar.tsx-25-34 (1)
25-34: Pausing resets progress to 0% instead of freezing at current position.When
pausedbecomestrue, the animation target changes towidth: '0%', which visually resets the progress bar. Users typically expect a pause to freeze the current progress. Consider preserving the current width when paused.This would require tracking the current progress percentage in state or using Framer Motion's animation controls to properly pause/resume.
frontend/src/pages/Memories/Memories.tsx-102-117 (1)
102-117: Guard against emptyimage_idsarray.If a memory has an empty
image_idsarray,getThumb(m)will callallImages.find((i) => i.id === undefined), which may produce unexpected results. Consider adding a guard.🔎 Proposed fix
const getThumb = (m: Memory) => - allImages.find((i) => i.id === m.image_ids[0]); + m.image_ids.length > 0 ? allImages.find((i) => i.id === m.image_ids[0]) : undefined;Committable suggestion skipped: line range outside the PR's diff.
backend/app/routes/memory.py-59-68 (1)
59-68: Subtitle date may not reflect "today" for SPOTLIGHT.
spotlight_dateis set to the last matching image's date, but for "Spotlight of the day," users would expect today's date. If images were taken at different times today, the subtitle will show the time of the last image processed, not necessarily the most recent.🔎 Proposed fix to always use today's date
if spotlight_ids: memories.append( Memory( id="spotlight-today", type="SPOTLIGHT", title="Spotlight of the day", - subtitle=format_date(spotlight_date), + subtitle=format_date(today), image_ids=limit(spotlight_ids), ) )
🧹 Nitpick comments (17)
frontend/src/src-tauri/Cargo.toml (2)
30-30: Movewinapito Windows-specific dependencies.The
winapicrate is Windows-only and should be placed under a target-specific section to avoid compilation issues or unnecessary dependencies on other platforms.🔎 Proposed fix
-winapi = { version = "0.3", features = ["winuser"] }Then add under the target-specific section at line 43:
[target.'cfg(not(any(target_os = "android", target_os = "ios")))'.dependencies] [target.'cfg(windows)'.dependencies] winapi = { version = "0.3", features = ["winuser"] }
43-43: Empty target-specific dependencies section.This section is defined but has no dependencies listed. Either add the intended dependencies (e.g., move
winapihere with a Windows-specific config) or remove this empty section to avoid confusion.frontend/src/src-tauri/capabilities/migrated.json (1)
7-23: Remove duplicate permission entries.Multiple permissions are duplicated in this manifest:
core:path:default,core:event:default,core:window:default,core:app:default,core:resources:default,core:menu:default,core:tray:defaultappear twice (lines 7-13 and 15-22)fs:defaultappears at lines 25 and 136dialog:defaultappears at lines 89, 90, and 137shell:defaultappears at lines 88 and 135Consolidate to a single occurrence of each permission for clarity and maintainability.
frontend/src/src-tauri/src/main.rs (2)
18-24: Clarify the purpose of the setup hook or remove it.The setup hook resolves and logs the
resources/backendpath but doesn't validate it or store it anywhere. Sinceget_server_path(line 25) performs the same resolution on demand, this setup hook appears redundant. Consider either:
- Adding validation logic (e.g., checking if the path exists) to fail fast during startup, or
- Removing the setup hook entirely if path validation isn't needed at startup.
🔎 Option 1: Add validation to fail fast
.setup(|app| { let resource_path = app .path() .resolve("resources/backend", BaseDirectory::Resource)?; - println!("Resource path: {:?}", resource_path); + if !resource_path.exists() { + return Err(format!("Backend resources not found at {:?}", resource_path).into()); + } + println!("Backend resources validated at: {:?}", resource_path); Ok(()) })🔎 Option 2: Remove redundant setup hook
-.setup(|app| { - let resource_path = app - .path() - .resolve("resources/backend", BaseDirectory::Resource)?; - println!("Resource path: {:?}", resource_path); - Ok(()) -}) .invoke_handler(tauri::generate_handler![services::get_server_path,])
22-22: Consider using a logging framework instead ofprintln!.Using
println!for logging in production code isn't ideal as it goes to stdout, which may not be captured in release builds. Consider using a proper logging framework likelogortracingwith appropriate log levels.frontend/src/src-tauri/src/services/mod.rs (1)
10-10: Consider the implications ofto_string_lossy().Using
to_string_lossy()can replace invalid UTF-8 sequences with the replacement character (�), potentially corrupting paths on systems with non-UTF8 filenames. While rare on modern systems, consider:
- Using
to_str()and returning an error if the path isn't valid UTF-8, or- Documenting that the app requires UTF-8 compatible paths.
🔎 Proposed change using to_str() with proper error handling
- Ok(resource_path.to_string_lossy().to_string()) + resource_path + .to_str() + .ok_or_else(|| "Backend path contains invalid UTF-8".to_string()) + .map(|s| s.to_string())docs/backend/backend_python/openapi.json (1)
38-56: Document the/memories/response schema for consistencyThe
/memories/GET endpoint currently returns an empty schema object in OpenAPI, while other endpoints reference concrete response models. Since the backend exposes a structured memories payload, consider defining a dedicated response schema (e.g.GetMemoriesResponsewith amemories: Memory[]field) undercomponents.schemasand referencing it here. This will make generated docs/SDKs much more useful.site/backend/backend_python/api/swagger-9fec0c86.html (1)
7-51: Swagger UI wiring is correct; minor config redundancy is harmlessThe Swagger UI bundle is correctly pointed at
../openapi.json, with theming and observers wired as expected. The duplicated"validatorUrl": "none"entries are redundant but do not affect behavior.Also applies to: 69-116
frontend/src/api/api-functions/memories.ts (1)
1-10: Memories API helper is correct; consider tightening types and URL
fetchMemoriesis wired correctly tomemoriesEndpoints.getAllMemoriesand returns theAPIResponsepayload as expected. Two optional follow-ups you might consider later:
- Align the endpoint string with the backend’s documented path (
'/memories/') to avoid an extra redirect.- Introduce a more specific response type (e.g.
APIResponse<{ memories: Memory[] }>) once the API surface stabilizes, for stronger typing on the caller side.site/backend/backend_python/database/index.html (1)
768-775: Consider hosting the database diagram locally.The page embeds an iframe from an external service (dbdiagram.io). For improved reliability and offline access, consider:
- Exporting the diagram as a static image
- Hosting it within the project's assets directory
- Maintaining the external link as a fallback option
site/index.html (1)
710-784: Consider extracting inline styles to CSS.The navigation section uses inline styles (e.g.,
style="width:25%",style="display:flex"). For better maintainability, consider:
- Moving these styles to the external CSS files
- Using CSS classes for consistent styling across documentation pages
frontend/src/pages/Memories/Memories.tsx (2)
48-56: Duplicate filtering logic — consider extracting to a shared helper or memoized value.The same filtering expression
allImages.filter((img) => activeMemory.image_ids.includes(img.id))appears in both theuseEffect(line 51-53) and the JSX (line 72-74). This duplicates computation and could diverge if one is updated without the other.🔎 Proposed refactor
+ const filteredImages = useMemo(() => { + if (!activeMemory) return []; + return allImages.filter((img) => activeMemory.image_ids.includes(img.id)); + }, [activeMemory, allImages]); useEffect(() => { if (!activeMemory) return; - const filtered = allImages.filter((img) => - activeMemory.image_ids.includes(img.id), - ); - dispatch(setImages(filtered)); + dispatch(setImages(filteredImages)); - }, [activeMemory, allImages, dispatch]); + }, [filteredImages, dispatch]); // ... later in JSX: <ChronologicalGallery - images={allImages.filter((img) => - activeMemory.image_ids.includes(img.id), - )} + images={filteredImages} showTitle title={activeMemory.title} />Also applies to: 71-74
85-86: MovegetThumboutside the render path or memoize it.
getThumbis redefined on every render. While small, this can cause unnecessary allocations. Consider moving it outside the component or wrapping it inuseCallback.frontend/src/components/Media/MediaView.tsx (1)
82-86: Passdurationprop toStoryProgressBarfor consistency.The auto-play timer uses 4000ms (line 45), which matches the StoryProgressBar's default. However, explicitly passing the
durationprop would make the relationship clearer and prevent bugs if either default changes.🔎 Proposed fix
+ const AUTOPLAY_DURATION = 4000; // ... in auto-play effect: - }, 4000); + }, AUTOPLAY_DURATION); // ... in JSX: <StoryProgressBar total={images.length} current={currentIndex} paused={paused} + duration={AUTOPLAY_DURATION} />site/backend/backend_python/openapi.json (1)
1546-1552: Arrays lackmaxItemsconstraints.Per static analysis (CKV_OPENAPI_21), array properties like
folder_ids(line 1546-1552) lackmaxItemsconstraints. While not critical, adding bounds helps prevent potential DoS via oversized payloads.backend/app/routes/memory.py (2)
19-27: Overly broad exception handling inparse_date.Catching bare
Exceptionmasks potential bugs. Consider catchingValueErrorspecifically, which is whatdatetime.fromisoformatraises for invalid formats.🔎 Proposed fix
def parse_date(img): md = img.get("metadata") or {} date_str = md.get("date_created") if not date_str: return None try: return datetime.fromisoformat(date_str) - except Exception: + except (ValueError, TypeError): return None
44-143: Consider single-pass iteration over images.The endpoint iterates through
imagesfour separate times (SPOTLIGHT, REVISIT, WEEKEND, YEAR). For large libraries, this could be optimized into a single pass that categorizes images into all buckets simultaneously.This aligns with the PR's noted future improvement: "backend query optimization for large libraries."
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (103)
frontend/src/src-tauri/Cargo.lockis excluded by!**/*.lockfrontend/src/src-tauri/assets/PictoPy_Logo.pngis excluded by!**/*.pngfrontend/src/src-tauri/icons/128x128.pngis excluded by!**/*.pngfrontend/src/src-tauri/icons/128x128@2x.pngis excluded by!**/*.pngfrontend/src/src-tauri/icons/32x32.pngis excluded by!**/*.pngfrontend/src/src-tauri/icons/Square107x107Logo.pngis excluded by!**/*.pngfrontend/src/src-tauri/icons/Square142x142Logo.pngis excluded by!**/*.pngfrontend/src/src-tauri/icons/Square150x150Logo.pngis excluded by!**/*.pngfrontend/src/src-tauri/icons/Square284x284Logo.pngis excluded by!**/*.pngfrontend/src/src-tauri/icons/Square30x30Logo.pngis excluded by!**/*.pngfrontend/src/src-tauri/icons/Square310x310Logo.pngis excluded by!**/*.pngfrontend/src/src-tauri/icons/Square44x44Logo.pngis excluded by!**/*.pngfrontend/src/src-tauri/icons/Square71x71Logo.pngis excluded by!**/*.pngfrontend/src/src-tauri/icons/Square89x89Logo.pngis excluded by!**/*.pngfrontend/src/src-tauri/icons/StoreLogo.pngis excluded by!**/*.pngfrontend/src/src-tauri/icons/android/mipmap-hdpi/ic_launcher.pngis excluded by!**/*.pngfrontend/src/src-tauri/icons/android/mipmap-hdpi/ic_launcher_foreground.pngis excluded by!**/*.pngfrontend/src/src-tauri/icons/android/mipmap-hdpi/ic_launcher_round.pngis excluded by!**/*.pngfrontend/src/src-tauri/icons/android/mipmap-mdpi/ic_launcher.pngis excluded by!**/*.pngfrontend/src/src-tauri/icons/android/mipmap-mdpi/ic_launcher_foreground.pngis excluded by!**/*.pngfrontend/src/src-tauri/icons/android/mipmap-mdpi/ic_launcher_round.pngis excluded by!**/*.pngfrontend/src/src-tauri/icons/android/mipmap-xhdpi/ic_launcher.pngis excluded by!**/*.pngfrontend/src/src-tauri/icons/android/mipmap-xhdpi/ic_launcher_foreground.pngis excluded by!**/*.pngfrontend/src/src-tauri/icons/android/mipmap-xhdpi/ic_launcher_round.pngis excluded by!**/*.pngfrontend/src/src-tauri/icons/android/mipmap-xxhdpi/ic_launcher.pngis excluded by!**/*.pngfrontend/src/src-tauri/icons/android/mipmap-xxhdpi/ic_launcher_foreground.pngis excluded by!**/*.pngfrontend/src/src-tauri/icons/android/mipmap-xxhdpi/ic_launcher_round.pngis excluded by!**/*.pngfrontend/src/src-tauri/icons/android/mipmap-xxxhdpi/ic_launcher.pngis excluded by!**/*.pngfrontend/src/src-tauri/icons/android/mipmap-xxxhdpi/ic_launcher_foreground.pngis excluded by!**/*.pngfrontend/src/src-tauri/icons/android/mipmap-xxxhdpi/ic_launcher_round.pngis excluded by!**/*.pngfrontend/src/src-tauri/icons/icon.icois excluded by!**/*.icofrontend/src/src-tauri/icons/icon.pngis excluded by!**/*.pngfrontend/src/src-tauri/icons/ios/AppIcon-20x20@1x.pngis excluded by!**/*.pngfrontend/src/src-tauri/icons/ios/AppIcon-20x20@2x-1.pngis excluded by!**/*.pngfrontend/src/src-tauri/icons/ios/AppIcon-20x20@2x.pngis excluded by!**/*.pngfrontend/src/src-tauri/icons/ios/AppIcon-20x20@3x.pngis excluded by!**/*.pngfrontend/src/src-tauri/icons/ios/AppIcon-29x29@1x.pngis excluded by!**/*.pngfrontend/src/src-tauri/icons/ios/AppIcon-29x29@2x-1.pngis excluded by!**/*.pngfrontend/src/src-tauri/icons/ios/AppIcon-29x29@2x.pngis excluded by!**/*.pngfrontend/src/src-tauri/icons/ios/AppIcon-29x29@3x.pngis excluded by!**/*.pngfrontend/src/src-tauri/icons/ios/AppIcon-40x40@1x.pngis excluded by!**/*.pngfrontend/src/src-tauri/icons/ios/AppIcon-40x40@2x-1.pngis excluded by!**/*.pngfrontend/src/src-tauri/icons/ios/AppIcon-40x40@2x.pngis excluded by!**/*.pngfrontend/src/src-tauri/icons/ios/AppIcon-40x40@3x.pngis excluded by!**/*.pngfrontend/src/src-tauri/icons/ios/AppIcon-512@2x.pngis excluded by!**/*.pngfrontend/src/src-tauri/icons/ios/AppIcon-60x60@2x.pngis excluded by!**/*.pngfrontend/src/src-tauri/icons/ios/AppIcon-60x60@3x.pngis excluded by!**/*.pngfrontend/src/src-tauri/icons/ios/AppIcon-76x76@1x.pngis excluded by!**/*.pngfrontend/src/src-tauri/icons/ios/AppIcon-76x76@2x.pngis excluded by!**/*.pngfrontend/src/src-tauri/icons/ios/AppIcon-83.5x83.5@2x.pngis excluded by!**/*.pngsite/assets/AOSSIE-logo.pngis excluded by!**/*.pngsite/assets/PictoPy-logo.pngis excluded by!**/*.pngsite/assets/backend-architecture.jpegis excluded by!**/*.jpegsite/assets/favicon.pngis excluded by!**/*.pngsite/assets/images/favicon.pngis excluded by!**/*.pngsite/assets/javascripts/bundle.50899def.min.jsis excluded by!**/*.min.jssite/assets/javascripts/bundle.50899def.min.js.mapis excluded by!**/*.map,!**/*.min.js.mapsite/assets/javascripts/lunr/min/lunr.ar.min.jsis excluded by!**/*.min.jssite/assets/javascripts/lunr/min/lunr.da.min.jsis excluded by!**/*.min.jssite/assets/javascripts/lunr/min/lunr.de.min.jsis excluded by!**/*.min.jssite/assets/javascripts/lunr/min/lunr.du.min.jsis excluded by!**/*.min.jssite/assets/javascripts/lunr/min/lunr.el.min.jsis excluded by!**/*.min.jssite/assets/javascripts/lunr/min/lunr.es.min.jsis excluded by!**/*.min.jssite/assets/javascripts/lunr/min/lunr.fi.min.jsis excluded by!**/*.min.jssite/assets/javascripts/lunr/min/lunr.fr.min.jsis excluded by!**/*.min.jssite/assets/javascripts/lunr/min/lunr.he.min.jsis excluded by!**/*.min.jssite/assets/javascripts/lunr/min/lunr.hi.min.jsis excluded by!**/*.min.jssite/assets/javascripts/lunr/min/lunr.hu.min.jsis excluded by!**/*.min.jssite/assets/javascripts/lunr/min/lunr.hy.min.jsis excluded by!**/*.min.jssite/assets/javascripts/lunr/min/lunr.it.min.jsis excluded by!**/*.min.jssite/assets/javascripts/lunr/min/lunr.ja.min.jsis excluded by!**/*.min.jssite/assets/javascripts/lunr/min/lunr.jp.min.jsis excluded by!**/*.min.jssite/assets/javascripts/lunr/min/lunr.kn.min.jsis excluded by!**/*.min.jssite/assets/javascripts/lunr/min/lunr.ko.min.jsis excluded by!**/*.min.jssite/assets/javascripts/lunr/min/lunr.multi.min.jsis excluded by!**/*.min.jssite/assets/javascripts/lunr/min/lunr.nl.min.jsis excluded by!**/*.min.jssite/assets/javascripts/lunr/min/lunr.no.min.jsis excluded by!**/*.min.jssite/assets/javascripts/lunr/min/lunr.pt.min.jsis excluded by!**/*.min.jssite/assets/javascripts/lunr/min/lunr.ro.min.jsis excluded by!**/*.min.jssite/assets/javascripts/lunr/min/lunr.ru.min.jsis excluded by!**/*.min.jssite/assets/javascripts/lunr/min/lunr.sa.min.jsis excluded by!**/*.min.jssite/assets/javascripts/lunr/min/lunr.stemmer.support.min.jsis excluded by!**/*.min.jssite/assets/javascripts/lunr/min/lunr.sv.min.jsis excluded by!**/*.min.jssite/assets/javascripts/lunr/min/lunr.ta.min.jsis excluded by!**/*.min.jssite/assets/javascripts/lunr/min/lunr.te.min.jsis excluded by!**/*.min.jssite/assets/javascripts/lunr/min/lunr.th.min.jsis excluded by!**/*.min.jssite/assets/javascripts/lunr/min/lunr.tr.min.jsis excluded by!**/*.min.jssite/assets/javascripts/lunr/min/lunr.vi.min.jsis excluded by!**/*.min.jssite/assets/javascripts/lunr/min/lunr.zh.min.jsis excluded by!**/*.min.jssite/assets/javascripts/swagger-ui-bundle.js.mapis excluded by!**/*.mapsite/assets/javascripts/swagger-ui-standalone-preset.js.mapis excluded by!**/*.mapsite/assets/javascripts/workers/search.d50fe291.min.jsis excluded by!**/*.min.jssite/assets/javascripts/workers/search.d50fe291.min.js.mapis excluded by!**/*.map,!**/*.min.js.mapsite/assets/screenshots/ai-tagging.pngis excluded by!**/*.pngsite/assets/screenshots/ai-tagging2.pngis excluded by!**/*.pngsite/assets/screenshots/home.pngis excluded by!**/*.pngsite/assets/screenshots/serverRunning.pngis excluded by!**/*.pngsite/assets/screenshots/settings.pngis excluded by!**/*.pngsite/assets/screenshots/virtualEnv.pngis excluded by!**/*.pngsite/assets/stylesheets/main.7e37652d.min.css.mapis excluded by!**/*.mapsite/assets/stylesheets/palette.06af60db.min.css.mapis excluded by!**/*.mapsite/assets/stylesheets/swagger-ui.css.mapis excluded by!**/*.mapsite/sitemap.xml.gzis excluded by!**/*.gz
📒 Files selected for processing (61)
backend/app/routes/memory.pybackend/main.pydocs/backend/backend_python/openapi.jsonfrontend/src/App.cssfrontend/src/api/api-functions/memories.tsfrontend/src/api/apiEndpoints.tsfrontend/src/components/Media/MediaView.tsxfrontend/src/components/Media/StoryProgressBar.tsxfrontend/src/pages/Home/Home.tsxfrontend/src/pages/Memories/Memories.tsxfrontend/src/routes/AppRoutes.tsxfrontend/src/src-tauri/.cargo/config.tomlfrontend/src/src-tauri/.gitignorefrontend/src/src-tauri/Cargo.tomlfrontend/src/src-tauri/Entitlements.plistfrontend/src/src-tauri/Info.plistfrontend/src/src-tauri/build.rsfrontend/src/src-tauri/capabilities/desktop.jsonfrontend/src/src-tauri/capabilities/migrated.jsonfrontend/src/src-tauri/folders_cache.txtfrontend/src/src-tauri/icons/icon.icnsfrontend/src/src-tauri/postinstall.shfrontend/src/src-tauri/src/lib.rsfrontend/src/src-tauri/src/main.rsfrontend/src/src-tauri/src/services/mod.rsfrontend/src/src-tauri/tauri.conf.jsonfrontend/src/types/Media.tsfrontend/src/types/Memory.tssite/404.htmlsite/Manual_Setup_Guide/index.htmlsite/Script_Setup_Guide/index.htmlsite/assets/javascripts/lunr/tinyseg.jssite/assets/javascripts/lunr/wordcut.jssite/assets/javascripts/swagger-ui-bundle.jssite/assets/javascripts/swagger-ui-standalone-preset.jssite/assets/stylesheets/main.7e37652d.min.csssite/assets/stylesheets/palette.06af60db.min.csssite/assets/stylesheets/swagger-ui-dark.csssite/assets/stylesheets/swagger-ui.csssite/assets/swagger-ui/oauth2-redirect.htmlsite/backend/backend_python/api/index.htmlsite/backend/backend_python/api/swagger-9fec0c86.htmlsite/backend/backend_python/database/index.htmlsite/backend/backend_python/directory-structure/index.htmlsite/backend/backend_python/image-processing/index.htmlsite/backend/backend_python/openapi.jsonsite/backend/backend_rust/api/index.htmlsite/frontend/screenshots/index.htmlsite/frontend/state-management/index.htmlsite/index.htmlsite/overview/architecture/index.htmlsite/overview/features/index.htmlsite/postcss.config.jssite/requirements.txtsite/search/search_index.jsonsite/setup/index.htmlsite/sitemap.xmlsite/stylesheets/extra.csssite/stylesheets/output.csssite/stylesheets/tailwind.csssite/tailwind.config.js
💤 Files with no reviewable changes (1)
- frontend/src/pages/Home/Home.tsx
🧰 Additional context used
🧬 Code graph analysis (3)
frontend/src/routes/AppRoutes.tsx (2)
frontend/src/constants/routes.ts (1)
ROUTES(1-12)frontend/src/pages/Memories/Memories.tsx (1)
Memories(17-133)
frontend/src/api/api-functions/memories.ts (3)
frontend/src/types/API.ts (1)
APIResponse(1-8)frontend/src/api/axiosConfig.ts (1)
apiClient(5-12)frontend/src/api/apiEndpoints.ts (1)
memoriesEndpoints(34-36)
backend/app/routes/memory.py (1)
backend/app/database/images.py (1)
db_get_all_images(123-214)
🪛 Biome (2.1.2)
site/stylesheets/output.css
[error] 581-581: Duplicate properties can lead to unexpected behavior and may override previous declarations unintentionally.
transition-property is already defined here.
Remove or rename the duplicate property to ensure consistent styling.
(lint/suspicious/noDuplicateProperties)
site/assets/stylesheets/swagger-ui-dark.css
[error] 600-600: Duplicate properties can lead to unexpected behavior and may override previous declarations unintentionally.
background is already defined here.
Remove or rename the duplicate property to ensure consistent styling.
(lint/suspicious/noDuplicateProperties)
site/assets/javascripts/lunr/tinyseg.js
[error] 184-184: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 184-184: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 184-184: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 185-185: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 185-185: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
site/stylesheets/extra.css
[error] 314-315: unexpected character \
(parse)
🪛 Checkov (3.2.334)
site/backend/backend_python/openapi.json
[high] 1-2926: Ensure that the global security field has rules defined
(CKV_OPENAPI_4)
[high] 1-2926: Ensure that security operations is not empty.
(CKV_OPENAPI_5)
[medium] 1546-1552: Ensure that arrays have a maximum number of items
(CKV_OPENAPI_21)
🪛 Gitleaks (8.30.0)
site/stylesheets/extra.css
[high] 260-261: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🪛 Shellcheck (0.11.0)
frontend/src/src-tauri/postinstall.sh
[warning] 6-6: Declare and assign separately to avoid masking return values.
(SC2155)
🔇 Additional comments (35)
frontend/src/src-tauri/postinstall.sh (1)
16-18: Function invocations are appropriate.The function calls correctly target the backend and sync-microservice resource directories as described in the PR. The logic is sound, assuming the security vulnerabilities in the
update_permissionsfunction are addressed.site/assets/swagger-ui/oauth2-redirect.html (2)
1-8: LGTM: Standard Swagger UI OAuth2 redirect page structure.The HTML structure and strict mode declaration are appropriate for this OAuth2 redirect handler.
1-79: This file is correctly sourced from the official Swagger UI distribution via themkdocs-swagger-ui-tagplugin (v0.7.1), which automatically provides this asset. No modifications have been made, and updates/security patches will be applied through pip dependency management. No action needed.frontend/src/src-tauri/Info.plist (1)
5-6: Verify if camera access is required for this feature.The PR adds a "Memories" feature for viewing existing photos, but this plist requests camera access for WebRTC. Camera access doesn't appear to be needed for displaying memories. If this is boilerplate from a Tauri template, consider removing unused privacy descriptions to avoid unnecessary permission prompts.
frontend/src/src-tauri/.cargo/config.toml (1)
1-11: LGTM!The increased stack size (8MB) is appropriate for a desktop application with image processing capabilities. The linker flags are correctly specified for both MSVC and GNU toolchains.
frontend/src/src-tauri/capabilities/desktop.json (1)
1-6: LGTM!Clean and minimal capability definition with appropriate scope limited to the updater functionality.
frontend/src/src-tauri/src/lib.rs (1)
1-1: LGTM!Clean module declaration exposing the services module.
frontend/src/src-tauri/build.rs (1)
1-3: LGTM!Standard Tauri build script. The
tauri_build::build()function handles the necessary build steps internally.frontend/src/src-tauri/src/main.rs (1)
11-17: Verify that all enabled plugins are necessary.Seven Tauri plugins are enabled, including
shellandprocess, which grant powerful system-level capabilities. Please confirm that each plugin is required for the Memories feature or other app functionality:
tauri_plugin_shell- enables executing shell commandstauri_plugin_process- enables spawning processestauri_plugin_fs,tauri_plugin_dialog,tauri_plugin_opener- filesystem and UI operationstauri_plugin_store,tauri_plugin_updater- data persistence and auto-updatesIf any plugins are unused, removing them reduces the app's attack surface.
frontend/src/src-tauri/src/services/mod.rs (1)
9-9: Error handling is adequate for current use case.Converting errors to
Stringusingmap_err(|e| e.to_string())is appropriate for this simple case where the frontend likely just displays error messages to users. If more sophisticated error handling becomes necessary (e.g., different UI behavior for different error types), consider using a structured error enum.frontend/src/src-tauri/tauri.conf.json (3)
58-62: Review the permissive asset protocol scope.The
assetProtocol.scopeis set to["**"], which allows the frontend to access any asset without restriction. While this may be necessary for the app's media viewing functionality, consider whether a more restrictive scope is feasible. For example, limiting access to specific directories like["$RESOURCE/**", "$APPDATA/**"]would reduce the attack surface if a path traversal vulnerability were discovered.
37-42: Updater configuration looks correct.The hardcoded public key for the Tauri updater is standard practice. The public key is used to verify signatures of updates downloaded from the GitHub releases endpoint, ensuring updates haven't been tampered with. This is the correct approach for secure auto-updates.
63-63: The CSP configuration is correct. Bothlocalhost:8000(main FastAPI backend) andlocalhost:8001(sync microservice) are confirmed in the actual backend code, with HTTP and WebSocket protocols properly configured for each service.frontend/src/src-tauri/.gitignore (1)
1-7: LGTM!Standard gitignore rules for Tauri and Cargo build artifacts. The patterns appropriately exclude compiled files and auto-generated schemas.
frontend/src/types/Media.ts (1)
39-40: LGTM!The optional
titleandsubtitlefields cleanly extend theMediaViewPropsinterface to support story-style metadata display without breaking existing implementations.frontend/src/App.css (1)
217-224: LGTM!Clean fadeIn animation implementation with appropriate timing (0.5s) and fill mode (
forwards). The animation will enhance the Memories feature's UI transitions.frontend/src/routes/AppRoutes.tsx (1)
11-11: LGTM! Clean integration of the Memories feature.The routing update correctly imports and integrates the new Memories component, replacing the placeholder ComingSoon page. The implementation follows the existing routing patterns and properly uses the ROUTES constant.
Also applies to: 25-25
frontend/src/api/apiEndpoints.ts (1)
34-36: LGTM! Endpoint configuration follows established patterns.The new
memoriesEndpointsobject is structured consistently with existing endpoint groups in this file and properly exposes the/memoriesroute for the frontend to consume.backend/main.py (1)
29-29: Memories router import & registration look correctImporting
memories_routerfromapp.routes.memoryand including it without an extra prefix matches the router’s ownprefix="/memories"configuration and aligns with existing route wiring.Also applies to: 127-127
site/backend/backend_python/image-processing/index.html (1)
839-994: Image processing documentation is clear and self-containedThe YOLOv11/FaceNet/DBSCAN pipeline description and parameter tables read well and match the backend’s responsibilities; no code or structural issues from a docs standpoint.
docs/backend/backend_python/openapi.json (1)
1139-1147: OpenAPI schema tweaks preserve semantics and improve clarityWrapping
InputTypein anallOfwith description/default/title, and simplifyingImageInCluster.metadatato anobject|nullunion, maintain existing behavior while making the schema more descriptive. No issues from a compatibility or correctness standpoint.Also applies to: 2202-2279
site/Manual_Setup_Guide/index.html (1)
682-775: Manual setup guide content is coherent and actionableThe fork/clone steps, Tauri frontend setup, Python 3.12 virtualenv instructions, and troubleshooting notes are well-structured and should be sufficient for contributors to get a dev environment running.
site/setup/index.html (1)
759-773: Setup landing page cleanly delegates to CONTRIBUTING.mdUsing this page as a concise hub that points to the main
CONTRIBUTING.mdsetup/troubleshooting sections keeps the docs DRY and easy to maintain.site/backend/backend_rust/api/index.html (1)
781-802: Rust backend API docs accurately describeget_server_pathusageThe command description and Tauri
invoke("get_server_path")example are clear and sufficient for frontend developers to integrate this Rust API.site/backend/backend_python/directory-structure/index.html (1)
797-1024: LGTM - Clear documentation structure.The directory structure documentation provides a comprehensive overview of the Python backend organization, including descriptions of each module's purpose. This will be helpful for developers onboarding to the project.
site/overview/features/index.html (1)
804-908: Well-structured features documentation.The features overview clearly articulates PictoPy's capabilities and technical stack. The categorization (Gallery, Analysis, Privacy, etc.) provides good organization for users and developers.
site/frontend/screenshots/index.html (1)
768-781: LGTM - Screenshots documentation.The screenshots page effectively demonstrates the application's UI with clear descriptions for each section (Gallery, AI Tagging, Settings).
site/assets/stylesheets/palette.06af60db.min.css (1)
1-1: Note: Generated minified CSS file.This is a minified CSS file for the MkDocs Material theme palette. As a generated artifact, detailed review is not applicable. Ensure this file is listed in
.gitignoreif it should be regenerated during builds.site/overview/architecture/index.html (1)
682-711: Excellent architecture documentation.The architecture overview clearly explains the system's components (Frontend, Backend Python, Backend Rust) and their interactions. The inclusion of a visual diagram enhances understanding.
frontend/src/types/Memory.ts (1)
3-26: Memory interface structure looks good.The interface is well-documented with clear JSDoc comments explaining each field's purpose. The optional fields (
subtitle,created_at) provide flexibility for different memory types.However, ensure the
created_atfield matches the backend response format (likely ISO 8601 string).site/404.html (1)
1-768: Auto-generated MkDocs page — no issues identified.This is a standard MkDocs Material-generated 404 page. The structure and styling are consistent with the documentation site.
frontend/src/components/Media/StoryProgressBar.tsx (1)
17-17: Verifytop-27is a valid Tailwind utility.
top-27is not a default Tailwind spacing value (default scale goes 0, 0.5, 1...24, 28, 32...). If this is intentional, ensure the Tailwind config includes this custom value, or usetop-[108px](27 × 4px) for arbitrary values.site/backend/backend_python/api/index.html (1)
1-457: MkDocs Swagger UI integration page — looks good.This is a well-structured documentation page integrating Swagger UI via iframe with proper dark mode synchronization. The JavaScript handles height adjustment and color scheme propagation appropriately.
site/frontend/state-management/index.html (1)
1-1227: Documentation page for Redux state management — looks good.This is a comprehensive documentation page covering the Redux store structure, slices, and usage patterns. The content is well-organized with proper code examples and TypeScript interfaces.
site/backend/backend_python/openapi.json (1)
1-17: Update openapi.json to include the/memoriesendpoint.The
/memoriesrouter is properly registered inmain.py(line 127), but the openapi.json file being reviewed is outdated. The auto-generated spec indocs/backend/backend_python/openapi.jsoncorrectly includes the/memoriesendpoint, whilesite/backend/backend_python/openapi.jsonis missing it. Regenerate or sync this file to reflect the current routes.Additionally, the OpenAPI spec lacks security definitions. While this may be acceptable for a local development API, consider adding a global
securityfield and security scheme definitions for documentation completeness.⛔ Skipped due to learnings
Learnt from: Anjali-Kan Repo: AOSSIE-Org/PictoPy PR: 483 File: docs/backend/backend_python/api.md:1-3 Timestamp: 2025-08-05T01:19:43.473Z Learning: Auto-generated OpenAPI specification files (openapi.json) should not be committed to the repository. Instead, they should be generated at application startup or during the build process to ensure they stay current with the actual API implementation.
| @router.get("/") | ||
| async def get_memories(): | ||
| images = db_get_all_images() | ||
| today = datetime.now() |
There was a problem hiding this comment.
Sync database call in async endpoint blocks the event loop.
db_get_all_images() is a synchronous function (per the relevant code snippet showing it uses sqlite3). Calling it directly from an async def endpoint will block the event loop. Use run_in_executor or convert to async.
🔎 Proposed fix using run_in_executor
+ import asyncio
+ from concurrent.futures import ThreadPoolExecutor
+ executor = ThreadPoolExecutor(max_workers=2)
@router.get("/")
async def get_memories():
- images = db_get_all_images()
+ loop = asyncio.get_event_loop()
+ images = await loop.run_in_executor(executor, db_get_all_images)
today = datetime.now()Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In backend/app/routes/memory.py around lines 38-41, the async endpoint calls the
synchronous db_get_all_images(), which will block the event loop; change the
call to run in a thread executor (or make the DB function async). Specifically,
import asyncio, get the running loop inside the endpoint and replace images =
db_get_all_images() with images = await loop.run_in_executor(None,
db_get_all_images) (or create an async wrapper that calls run_in_executor), then
continue using images as before; ensure any exceptions are propagated or caught
and that the function signature remains async.
| useEffect(() => { | ||
| if (!images.length || paused) return; | ||
|
|
||
| const timer = setTimeout(() => { | ||
| setVisible(false); // fade out | ||
| setTimeout(() => { | ||
| if (currentIndex < images.length - 1) { | ||
| dispatch(setCurrentViewIndex(currentIndex + 1)); | ||
| } else { | ||
| dispatch(closeImageView()); | ||
| onClose?.(); | ||
| } | ||
| setVisible(true); // fade in | ||
| }, 300); | ||
| }, 4000); | ||
|
|
||
| return () => clearTimeout(timer); | ||
| }, [currentIndex, paused, images.length, dispatch, onClose]); |
There was a problem hiding this comment.
Inner timeout may leak if component unmounts during transition.
The cleanup only clears the outer timer, but the inner setTimeout (line 36) can still fire after unmount if the outer timer completes but the component unmounts during the 300ms fade window.
🔎 Proposed fix
useEffect(() => {
if (!images.length || paused) return;
+ let innerTimer: ReturnType<typeof setTimeout>;
const timer = setTimeout(() => {
setVisible(false); // fade out
- setTimeout(() => {
+ innerTimer = setTimeout(() => {
if (currentIndex < images.length - 1) {
dispatch(setCurrentViewIndex(currentIndex + 1));
} else {
dispatch(closeImageView());
onClose?.();
}
setVisible(true); // fade in
}, 300);
}, 4000);
- return () => clearTimeout(timer);
+ return () => {
+ clearTimeout(timer);
+ clearTimeout(innerTimer);
+ };
}, [currentIndex, paused, images.length, dispatch, onClose]);🤖 Prompt for AI Agents
In frontend/src/components/Media/MediaView.tsx around lines 31 to 48, the inner
300ms timeout used for the fade transition can fire after the component unmounts
because only the outer timer is cleared; capture both timeout IDs (outer and
inner) in refs or local variables, clear both timers in the effect cleanup, and
guard any state updates or dispatches with an isMounted flag (or check that
timers weren’t cleared) so no inner callback runs after unmount.
| useEffect(() => { | ||
| fetchAllImages().then((res) => { | ||
| setAllImages(res.data as Image[]); | ||
| }); | ||
| }, []); |
There was a problem hiding this comment.
Add error handling for the image fetch.
The fetchAllImages() call lacks error handling. If the API call fails, the promise rejection is unhandled and allImages remains empty without user feedback.
🔎 Proposed fix
useEffect(() => {
- fetchAllImages().then((res) => {
- setAllImages(res.data as Image[]);
- });
+ fetchAllImages()
+ .then((res) => {
+ setAllImages(res.data as Image[]);
+ })
+ .catch((err) => {
+ console.error('Failed to fetch images:', err);
+ });
}, []);📝 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.
| useEffect(() => { | |
| fetchAllImages().then((res) => { | |
| setAllImages(res.data as Image[]); | |
| }); | |
| }, []); | |
| useEffect(() => { | |
| fetchAllImages() | |
| .then((res) => { | |
| setAllImages(res.data as Image[]); | |
| }) | |
| .catch((err) => { | |
| console.error('Failed to fetch images:', err); | |
| }); | |
| }, []); |
🤖 Prompt for AI Agents
In frontend/src/pages/Memories/Memories.tsx around lines 39 to 43, the
fetchAllImages() promise has no error handling so rejections are unhandled and
the user gets no feedback; wrap the call in a try/catch (or add a .catch) and on
error set an error state (and/or a loading flag) and optionally setAllImages([])
or leave unchanged, then surface a user-visible message or UI state (toast,
inline error, or fallback UI) and log the error for debugging; ensure success
still sets setAllImages(res.data as Image[]) and clean up any loading state in
both success and error paths.
| { | ||
| "identifier": "fs:scope", | ||
| "allow": ["**"] | ||
| }, | ||
| { | ||
| "identifier": "fs:read-all", | ||
| "allow": ["**"] | ||
| }, |
There was a problem hiding this comment.
Overly permissive file system scope grants unrestricted access.
Setting fs:scope and fs:read-all to ["**"] grants the application access to the entire file system. This violates the principle of least privilege and poses a security risk if the application is compromised.
Restrict the scope to only the directories the app needs (e.g., user's pictures folder, app data directory).
🔎 Suggested approach
{
"identifier": "fs:scope",
- "allow": ["**"]
+ "allow": ["$PICTURE/**", "$APPDATA/**", "$DOCUMENT/**"]
},
{
"identifier": "fs:read-all",
- "allow": ["**"]
+ "allow": ["$PICTURE/**", "$APPDATA/**"]
},📝 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.
| { | |
| "identifier": "fs:scope", | |
| "allow": ["**"] | |
| }, | |
| { | |
| "identifier": "fs:read-all", | |
| "allow": ["**"] | |
| }, | |
| { | |
| "identifier": "fs:scope", | |
| "allow": ["$PICTURE/**", "$APPDATA/**", "$DOCUMENT/**"] | |
| }, | |
| { | |
| "identifier": "fs:read-all", | |
| "allow": ["$PICTURE/**", "$APPDATA/**"] | |
| }, |
🤖 Prompt for AI Agents
In frontend/src/src-tauri/capabilities/migrated.json around lines 80–87, the
fs:scope and fs:read-all entries currently use ["**"] which grants full
filesystem access; replace the wildcard with an explicit, minimal list of
directories the app needs (for example the app data directory, user pictures
folder, or other specific absolute or environment-path placeholders), e.g.,
remove "**" and add only the exact paths or platform-aware variables your app
uses, ensure both identifiers are updated to the narrowed list, and validate the
app still functions with the restricted scopes.
| <dict> | ||
| <key>com.apple.security.app-sandbox</key> | ||
| <true/> | ||
| </dict> |
There was a problem hiding this comment.
Missing entitlements for photo gallery functionality.
The sandbox is enabled but no file access entitlements are granted. A photo gallery app typically requires additional entitlements such as:
com.apple.security.files.user-selected.read-only(orread-write) for accessing user-selected photoscom.apple.security.network.clientif the app communicates with the backend server
Without these, the sandboxed app may fail to access photos or connect to services on macOS.
🤖 Prompt for AI Agents
In frontend/src/src-tauri/Entitlements.plist around lines 4 to 7, the app
sandbox is enabled but missing file access and network entitlements required for
photo gallery functionality; add appropriate keys such as
com.apple.security.files.user-selected.read-only (or
com.apple.security.files.user-selected.read-write if your app needs to modify
files) set to true to allow user-selected photo access, and add
com.apple.security.network.client set to true if the app communicates with a
backend, ensuring you only request the minimum privileges needed and update any
build/signing profiles to include these entitlements.
| arr = qp.split("&"); | ||
| arr.forEach(function (v,i,_arr) { _arr[i] = '"' + v.replace('=', '":"') + '"';}); | ||
| qp = qp ? JSON.parse('{' + arr.join() + '}', | ||
| function (key, value) { | ||
| return key === "" ? value : decodeURIComponent(value); | ||
| } | ||
| ) : {}; |
There was a problem hiding this comment.
Consider adding error handling for JSON parsing.
The manual JSON construction from query parameters (lines 21-22) followed by JSON.parse() has no try-catch block. Malformed or malicious query parameters could cause an uncaught exception, breaking the OAuth2 flow.
However, if this is unmodified Swagger UI distribution code, verify whether this error handling is intentionally omitted or handled at a higher level.
🔎 Proposed error handling
+ try {
arr = qp.split("&");
arr.forEach(function (v,i,_arr) { _arr[i] = '"' + v.replace('=', '":"') + '"';});
qp = qp ? JSON.parse('{' + arr.join() + '}',
function (key, value) {
return key === "" ? value : decodeURIComponent(value);
}
) : {};
+ } catch (e) {
+ console.error('Failed to parse OAuth2 parameters:', e);
+ if (window.opener && window.opener.swaggerUIRedirectOauth2) {
+ window.opener.swaggerUIRedirectOauth2.errCb({
+ authId: 'unknown',
+ source: 'auth',
+ level: 'error',
+ message: 'Failed to parse OAuth2 callback parameters'
+ });
+ }
+ window.close();
+ return;
+ }Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In site/assets/swagger-ui/oauth2-redirect.html around lines 21 to 27, the code
constructs a JSON string from query parameters and calls JSON.parse without
error handling, which can throw on malformed input; wrap the JSON.parse call in
a try-catch, on error set qp to an empty object (or sensible default) and
optionally console.warn the parse failure with the raw qp string so the OAuth2
flow doesn't crash; ensure existing behavior for valid input is preserved and do
not swallow the error silently if higher-level handling is required (use logging
to surface it).
| module.exports = { | ||
| plugins: [require("tailwindcss"), require("autoprefixer")], | ||
| }; |
There was a problem hiding this comment.
Update PostCSS configuration for Tailwind v4.
Tailwind v4 has built-in Lightning CSS integration that handles autoprefixing automatically, and uses a different PostCSS plugin package (@tailwindcss/postcss instead of tailwindcss).
🔎 Proposed fix for Tailwind v4 PostCSS setup
module.exports = {
- plugins: [require("tailwindcss"), require("autoprefixer")],
+ plugins: [require("@tailwindcss/postcss")],
};Note: Autoprefixer is no longer needed as Tailwind v4's Lightning CSS engine handles vendor prefixes automatically.
Also ensure you have the correct package installed:
npm install @tailwindcss/postcss
# or
yarn add @tailwindcss/postcssBased on Tailwind v4 documentation: "Built-in Lightning CSS integration handles vendor prefixes, nesting, minification, and imports automatically. No need for separate PostCSS plugins like autoprefixer or postcss-import."
📝 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.
| module.exports = { | |
| plugins: [require("tailwindcss"), require("autoprefixer")], | |
| }; | |
| module.exports = { | |
| plugins: [require("@tailwindcss/postcss")], | |
| }; |
🤖 Prompt for AI Agents
In site/postcss.config.js around lines 1 to 3, update the PostCSS plugins for
Tailwind v4 by replacing the current plugins array so it uses the new Tailwind
PostCSS package and remove autoprefixer; specifically
require("@tailwindcss/postcss") instead of "tailwindcss" and drop
require("autoprefixer"). Also ensure the dependency @tailwindcss/postcss is
installed in package.json (npm install or yarn add) so the new plugin resolves.
|
|
||
| \ | ||
| .md-footer-meta__inner.md-grid { |
There was a problem hiding this comment.
Remove invalid backslash character.
Line 315 contains a standalone backslash (\) which is invalid CSS syntax. This will cause a parse error.
🔎 Proposed fix
}
}
-\
.md-footer-meta__inner.md-grid {
display: flex !important;📝 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.
| \ | |
| .md-footer-meta__inner.md-grid { | |
| .md-footer-meta__inner.md-grid { | |
| display: flex !important; |
🧰 Tools
🪛 Biome (2.1.2)
[error] 314-315: unexpected character \
(parse)
🤖 Prompt for AI Agents
In site/stylesheets/extra.css around lines 314 to 316, there is a stray
backslash on line 315 that produces invalid CSS and causes a parse error; remove
the standalone backslash so the selector ".md-footer-meta__inner.md-grid {" is
directly contiguous with no extraneous characters, then verify the file parses
(and run the CSS linter/build) to ensure no other syntax errors remain.
| @import "tailwindcss/base"; | ||
| @import "tailwindcss/components"; | ||
| @import "tailwindcss/utilities"; |
There was a problem hiding this comment.
Migrate to Tailwind CSS v4 import syntax.
This file uses the deprecated Tailwind v3 import syntax. Tailwind v4 requires a single import statement instead of separate base/components/utilities imports.
🔎 Proposed fix for Tailwind v4 compatibility
-@import "tailwindcss/base";
-@import "tailwindcss/components";
-@import "tailwindcss/utilities";
+@import "tailwindcss";Based on Tailwind v4 documentation: "replace @tailwind base; @tailwind components; @tailwind utilities; with a single @import 'tailwindcss';"
📝 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.
| @import "tailwindcss/base"; | |
| @import "tailwindcss/components"; | |
| @import "tailwindcss/utilities"; | |
| @import "tailwindcss"; |
🤖 Prompt for AI Agents
In site/stylesheets/tailwind.css around lines 1 to 3, the file uses the
deprecated Tailwind v3 multiple @import statements; replace the three lines
(@import "tailwindcss/base"; @import "tailwindcss/components"; @import
"tailwindcss/utilities";) with a single Tailwind v4 import by changing them to
one line: @import 'tailwindcss'; and save the file to restore compatibility with
Tailwind CSS v4.
| /** @type {import('tailwindcss').Config} */ | ||
| module.exports = { | ||
| content: ["./**/*.md"], | ||
| theme: { | ||
| extend: {}, | ||
| }, | ||
| plugins: [], | ||
| }; |
There was a problem hiding this comment.
Tailwind v4 uses CSS-first configuration.
This configuration file uses the Tailwind v3 structure. Tailwind v4 made significant changes: tailwind.config.js is now optional, and configuration should primarily live in CSS using the @theme directive. Content detection is automatic, and you can use @source directives in CSS instead of the content array.
🔎 Migration guidance for Tailwind v4
Based on Tailwind v4 documentation:
- Configuration is now CSS-first: Define theme tokens inside CSS using
@themedirective - Automatic content detection: No manual
contentarray needed - Use
@sourcein CSS: Replace the content array with@sourcedirectives in your CSS file if you need explicit includes/excludes
Option 1: Remove this config file entirely and use CSS-based configuration in your stylesheet.
Option 2: Keep minimal JS config for plugin integration, but move theme to CSS:
/** @type {import('tailwindcss').Config} */
module.exports = {
// Content detection is automatic in v4
// Use @source directives in CSS instead
};In your CSS file:
@import "tailwindcss";
@source "./**/*.md";
@theme {
/* Define custom theme tokens here */
}Based on Tailwind v4 documentation: "CSS-first configuration: define theme tokens, breakpoints, colors, etc. inside CSS using the @theme directive" and "Automatic content detection: Tailwind scans your project intelligently; no manual content array required."
🤖 Prompt for AI Agents
In site/tailwind.config.js around lines 1 to 8, this file uses Tailwind v3-style
JS configuration (content array and theme) but Tailwind v4 is CSS-first with
automatic content detection; fix by either removing the file entirely and moving
all theme tokens and sources into your CSS using @theme and @source directives,
or keep a minimal JS file only for plugin integration (remove the content array
and theme object) and declare theme tokens and any explicit @source in your
stylesheet; ensure any custom tokens, breakpoints, or @source globs are moved
into your main CSS and delete or simplify this JS config accordingly.
1 participant
Summary
This PR introduces a Google Photos–style Memories feature that enhances how users rediscover past moments.
It adds automatic memory generation on the backend and an Instagram-style story viewer on the frontend, allowing users to revisit photos based on time (today, past years, weekends, and yearly highlights) without manual album creation.
Changes Overview
backend/app/routes/memory.pybackend/main.pyMemories.tsxMediaView.tsxStoryProgressBar.tsxmemories.ts,apiEndpoints.tsAppRoutes.tsxMemory.ts,Media.tsDemo / Explanation Video
Video:
MemoriesPage.-.Made.with.Clipchamp.1.mp4
Demonstrates the complete Memories experience, including:
Notes
Future Improvements
Summary by CodeRabbit
Release Notes
New Features
Documentation
Style
✏️ Tip: You can customize this high-level summary in your review settings.