feat: ship the v1.1 application and modular AI overhaul#356
feat: ship the v1.1 application and modular AI overhaul#356Abhash-Chakraborty wants to merge 21 commits into
Conversation
️✅ There are no secrets present in this pull request anymore.If these secrets were true positive and are still valid, we highly recommend you to revoke them. 🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request. |
|
Warning Review limit reached
Next review available in: 37 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (13)
📝 WalkthroughWalkthroughThis release adds modular CPU, NVIDIA, mock, and no-AI deployment profiles; runtime capability resolution; cookie-based accounts; private GPS maps; vault restore and thumbnails; shared timeline media UI; an accessible application shell; persistent upload monitoring; and expanded CI, publishing, documentation, and test coverage. ChangesFind v1.1 platform overhaul
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested labels: Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
PR Context Summary
Suggested issue links
Use |
486885f to
538146c
Compare
538146c to
16bd8d2
Compare
Abhash-Chakraborty
added
enhancement
|
Release-candidate checks are now fully green on the final head: backend, frontend unit tests, production build, Chromium E2E, five Compose profiles, dependency review, three-OS hardware fallback matrix, CodeQL, the reference-source guard, and GitGuardian all pass. Marking ready for maintainer review. |
|
@macroscope-app review Please review this PR against its linked issue, local-first privacy rules, and the current Find repo instructions. |
|
Macroscope has since reviewed this pull request. An earlier review was skipped by a cost limit; a review has now completed, so that notice no longer applies. |
ApprovabilityVerdict: Needs human review Diff is too large for automated approval analysis. A human reviewer should evaluate this PR. You can customize Macroscope's approvability policy. Learn more. |
There was a problem hiding this comment.
Pull request overview
This PR prepares the v1.1.0 release candidate by consolidating the UI into a shared, focus-safe app shell, adding local account + vault session management, introducing an opt-in offline private photo map, and overhauling the runtime/AI packaging into modular Compose artifacts (no-ai/mock/cpu/nvidia) with CI + docs updates.
Changes:
- Replace the home page with a direct redirect to the photo timeline and standardize route chrome via a new
AppShell. - Add cookie-backed local accounts, in-memory-only vault sessions, and an opt-in offline map surface + APIs.
- Split runtime artifacts and Compose topology, add publishing workflow(s), and refresh docs/tests for the new modular profiles.
Reviewed changes
Copilot reviewed 130 out of 135 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| README.md | Updates runtime profile + setup docs |
| GSSOC_CONTRIBUTOR_GUIDE.md | Updates contributor commands to new compose files |
| frontend/src/lib/viewer-preload.ts | Adds per-asset accessible alt field |
| frontend/src/lib/api.ts | Adds auth/settings/runtime/map client contracts |
| frontend/src/components/vault/VaultUnlock.tsx | Uses vault client helper + improved 400 handling |
| frontend/src/components/vault/vault-client.ts | Adds authenticated vault API wrapper |
| frontend/src/components/private-map.module.css | Styles offline private map UI |
| frontend/src/components/map-timeline-panel.tsx | Cluster selection timeline + viewer integration |
| frontend/src/components/map-privacy-settings.tsx | Settings card + opt-in switch for map |
| frontend/src/components/justified-grid.tsx | SSR/jsdom deterministic layout fallback |
| frontend/src/components/CursorGlow.tsx | Removes cursor glow effect |
| frontend/src/components/asset-viewer.tsx | Provides meaningful alt text in viewer |
| frontend/src/app/trash/page.tsx | Moves trash browsing to TimelineMediaView |
| frontend/src/app/public/shared/[key]/page.tsx | Uses TimelineMediaView for shared albums |
| frontend/src/app/people/page.tsx | Uses TimelineMediaView for person media list |
| frontend/src/app/page.tsx | Redirects home to /timeline |
| frontend/src/app/layout.tsx | Replaces legacy nav with AppShell |
| frontend/src/app/globals.css | Shell/layout CSS updates + reduced-motion handling |
| frontend/src/app/clusters/page.tsx | Uses TimelineMediaView for cluster members |
| frontend/src/app/auth/setup/page.tsx | Adds local account setup page |
| frontend/src/app/auth/login/page.tsx | Adds local/shared-mode login page |
| frontend/src/app/archive/page.tsx | Moves archive browsing to TimelineMediaView |
| frontend/src/app/albums/page.tsx | Adds accessible album cover alt text |
| frontend/src/app/albums/[id]/page.tsx | Moves album browsing/viewer into TimelineMediaView |
| frontend/src/tests/vault-store.test.ts | Verifies vault token stays in-memory only |
| frontend/src/tests/vault-gallery.test.tsx | Tests vault timeline + authenticated fetches |
| frontend/src/tests/timeline-media-view.test.tsx | Tests timeline grouping, scrubber, viewer open |
| frontend/src/tests/settings-page.test.tsx | Tests AI/map settings + runtime reporting |
| frontend/src/tests/root-page.test.tsx | Tests home redirect behavior |
| frontend/src/tests/public-shared-timeline.test.tsx | Ensures share viewer uses only share URLs |
| frontend/src/tests/private-map-style.test.ts | Validates offline map style + bounds logic |
| frontend/src/tests/media-timeline.test.ts | Tests grouping + scroll/scrubber mapping math |
| frontend/src/tests/map-page.test.tsx | Tests opt-in state, filters, cluster timeline open |
| frontend/src/tests/asset-viewer.test.tsx | Tests viewer alt fallback and behavior |
| frontend/src/tests/archive-trash.test.tsx | Updates tests for timeline-based archive/trash |
| frontend/src/tests/app-shell.test.tsx | Verifies drawer a11y, focus trap, shortcuts |
| frontend/src/tests/album-detail-viewer.test.tsx | Updates album viewer tests for timeline view |
| frontend/src/tests/account-auth.test.tsx | Tests login/setup/local/shared mode flows |
| frontend/src-tauri/tauri.conf.json | Bumps desktop version to 1.1.0 |
| frontend/src-tauri/Cargo.toml | Bumps desktop crate version to 1.1.0 |
| frontend/src-tauri/Cargo.lock | Bumps locked desktop version metadata |
| frontend/public/maps/NATURAL_EARTH_NOTICE.md | Adds attribution/notice for map data |
| frontend/package.json | Bumps frontend version + adds MapLibre |
| frontend/e2e/app-shell.spec.ts | Updates Playwright expectations for new shell/routes |
| docs/plans/not-started/storage-provider-neutrality-adr.md | Updates compose file references |
| docs/plans/not-started/remote-acceleration.md | Updates modular profile references |
| docs/plans/not-started/desktop-runtime-comparison.md | Updates modular compose wording |
| docs/overhaul/inventory/lane-h-mobile-desktop.md | Removes stale planning doc |
| docs/guides/testsprite-ci.md | Updates CI stack to compose.mock.yml |
| docs/guides/hardware-acceleration.md | Documents install-time profiles + runtime reporting |
| docs/guides/common-setup-errors.md | Updates troubleshooting commands to new compose |
| docker-compose.yml | Removes legacy monolithic compose |
| docker-compose.light.yml | Removes legacy light compose |
| CONTRIBUTING.md | Updates compose commands + license statement |
| compose.yml | Adds default NVIDIA modular compose file |
| compose.no-ai.yml | Adds no-AI modular compose file |
| compose.mock.yml | Adds mock modular compose file |
| compose.cpu.yml | Adds CPU modular compose file |
| compose.base.yml | Adds shared service definitions for all profiles |
| CHANGELOG.md | Adds v1.1.0 changelog entry |
| backend/tests/test_thumbnails.py | Patches worker runtime seam for tests + adds metadata-only test |
| backend/tests/test_status.py | Adds worker runtime/health assertions |
| backend/tests/test_settings.py | Expands settings/runtime endpoint coverage |
| backend/tests/test_search.py | Binds runtime in search tests |
| backend/tests/test_runtime_profile.py | Adds modular runtime resolution tests |
| backend/tests/test_model_manager.py | Sets current ML mode for processor tests |
| backend/tests/test_map.py | Adds privacy/scoping tests for map markers |
| backend/tests/test_hybrid_embedding.py | Switches to current_ml_mode seam |
| backend/tests/test_gallery.py | Adds /original redirect tests + hidden coverage |
| backend/tests/test_exif.py | Adds focused GPS extraction tests |
| backend/tests/test_clustering_edge_cases.py | Patches worker runtime seam for tests |
| backend/tests/test_cluster_images_regression.py | Patches worker runtime seam for tests |
| backend/tests/test_auth.py | Adds cookie-session coverage + account settings tests |
| backend/src/find_api/workers/processors.py | Uses bound runtime mode, errors on unsupported modes |
| backend/src/find_api/utils/exif.py | Adds GPS extraction helper and safety handling |
| backend/src/find_api/routers/status.py | Adds worker health/runtime fields to status |
| backend/src/find_api/routers/search.py | Resolves/binds runtime per request; improves cache partitioning |
| backend/src/find_api/routers/map.py | Adds private map marker endpoint |
| backend/src/find_api/routers/gallery.py | Adds scoped /image/{id}/original redirect route |
| backend/src/find_api/routers/config.py | Adds /api/config/runtime + richer settings model |
| backend/src/find_api/models/media.py | Adds latitude/longitude fields for map |
| backend/src/find_api/models/app_setting.py | Updates setting-store docs for new runtime prefs |
| backend/src/find_api/ml/object_detector.py | Keys model cache by bound accel mode |
| backend/src/find_api/ml/face_detector.py | Uses bound accel mode for provider resolution |
| backend/src/find_api/ml/clusterer.py | Avoids GPU backend when accel pinned to CPU |
| backend/src/find_api/ml/clip_embedder.py | Keys model cache by bound accel mode |
| backend/src/find_api/ml/captioner.py | Keys model cache by bound accel mode |
| backend/src/find_api/main.py | Uses backend package version + registers map router |
| backend/src/find_api/core/model_manager.py | Publishes per-process runtime snapshot |
| backend/src/find_api/core/hardware.py | Uses bound accel mode at job boundaries |
| backend/src/find_api/core/dependencies.py | Supports HttpOnly cookie sessions + bearer header |
| backend/src/find_api/core/database.py | Adds GPS columns/indexes in init |
| backend/src/find_api/core/config.py | Adds build profile + AI/map defaults; refines remote validation |
| backend/src/find_api/init.py | Bumps backend version to 1.1.0 |
| backend/pyproject.toml | Splits optional deps into mock/cpu/nvidia; bumps version |
| backend/Dockerfile | Builds per-profile dependencies; locks uv sync |
| backend/alembic/versions/20260712_media_gps.py | Adds GPS migration for media table |
| AGENTS.md | Updates compose file references in agent guidelines |
| .gitignore | Keeps reference-app/ ignored |
| .github/workflows/testsprite.yml | Starts compose.mock.yml stack |
| .github/workflows/publish.yml | Adds tag/manual GHCR publish for modular images |
| .github/workflows/manual-ghcr-publish.yml | Removes old publishing workflow |
| .github/workflows/ci.yml | Adds reference boundary gate + validates modular compose |
| .env.example | Adds build profile + AI/map settings guidance |
| .dockerignore | Ensures reference app never enters docker contexts |
Files not reviewed (1)
- frontend/pnpm-lock.yaml: Generated file
|
Copilot's two actionable findings are addressed in 853b593 and both inline threads are resolved. The final head is green again across CI, Chromium E2E, TestSprite, CodeQL, dependency review, Macroscope correctness, GitGuardian, and the three-OS hardware matrix. Remaining gate: independent human approval. |
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (13)
backend/tests/test_search.py (1)
106-111: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRedundant re-patching of runtime already stubbed by the autouse fixture.
The autouse
clear_cache_between_testsfixture already monkeypatchesresolve_runtime/bind_runtime/reset_runtimefor every test. These helper-localpatch(...)blocks re-stub the same three names again, but with a different mock shape (MagicMock(applied_mode="mock")instead of the fixture'sSimpleNamespace(applied_mode="mock", configured_accel_mode="cpu"), droppingconfigured_accel_mode). This is harmless today only because nothing in these tests currently asserts onconfigured_accel_mode, but it's a latent inconsistency that could mask real behavior if the router starts branching on that attribute.Consider either removing these now-redundant local patches (relying on the autouse fixture), or if a helper-specific override is genuinely needed, keep the mock shape consistent with the fixture's
SimpleNamespace.Also applies to: 471-476
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/tests/test_search.py` around lines 106 - 111, Remove the redundant local patch(...) blocks for resolve_runtime, bind_runtime, and reset_runtime in the affected test helpers, relying on the autouse clear_cache_between_tests fixture. If a helper-specific override is required, preserve the fixture’s complete mock shape, including configured_accel_mode="cpu", rather than using a reduced MagicMock.backend/alembic/versions/20260712_media_gps.py (1)
18-21: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winConsider a composite index for bbox queries.
Separate single-column indexes on
latitudeandlongitudesupport single-predicate filtering, but the map endpoint filters on both simultaneously via bbox (west/east/south/north), as exercised inbackend/tests/test_map.py. A composite(latitude, longitude)index (or a spatial/GiST index if PostGIS is available) would serve these two-predicate range queries more efficiently than relying on bitmap-AND of two separate single-column indexes.♻️ Proposed composite index
def upgrade() -> None: op.add_column("media", sa.Column("latitude", sa.Float(), nullable=True)) op.add_column("media", sa.Column("longitude", sa.Float(), nullable=True)) - op.create_index("ix_media_latitude", "media", ["latitude"]) - op.create_index("ix_media_longitude", "media", ["longitude"]) + op.create_index( + "ix_media_latitude_longitude", "media", ["latitude", "longitude"] + ) def downgrade() -> None: - op.drop_index("ix_media_longitude", table_name="media") - op.drop_index("ix_media_latitude", table_name="media") + op.drop_index("ix_media_latitude_longitude", table_name="media") op.drop_column("media", "longitude") op.drop_column("media", "latitude")🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/alembic/versions/20260712_media_gps.py` around lines 18 - 21, Replace the separate latitude and longitude indexes in the media migration with a composite index covering both columns, such as `(latitude, longitude)`, to support the map endpoint’s combined bbox range filters. Update the migration’s upgrade and downgrade paths consistently, preserving the nullable columns and using the existing index naming conventions..github/workflows/ci.yml (1)
220-249: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider adding
timeout-minutesto thedependency-reviewjob.The
compose-checkjob setstimeout-minutes: 10, but thedependency-reviewjob doesn't have a timeout. Adding one would prevent the job from hanging indefinitely and keep timeout hygiene consistent across CI jobs.♻️ Suggested addition
dependency-review: if: github.event_name == 'pull_request' runs-on: ubuntu-latest + timeout-minutes: 5 permissions: contents: read🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/ci.yml around lines 220 - 249, Add a finite timeout-minutes value to the dependency-review job, matching the timeout hygiene used by the nearby compose-check job, while leaving its existing trigger, permissions, checkout, and dependency-review steps unchanged.backend/src/find_api/core/config.py (1)
47-61: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDocument new settings in
.env.example.
FIND_BUILD_PROFILE,AI_ENABLED, andMAP_ENABLEDare new environment-configurable fields. Please confirm.env.exampledocuments them with their defaults/allowed values so local setup stays consistent with the config surface.
As per coding guidelines, "Use.env.examplefor documented local configuration."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/src/find_api/core/config.py` around lines 47 - 61, Update .env.example to document FIND_BUILD_PROFILE with its development default and allowed values, AI_ENABLED with its true default, and MAP_ENABLED with its false default, matching the configuration declarations in the settings class.Source: Coding guidelines
backend/src/find_api/core/database.py (1)
261-272: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winConsider a composite index for bbox queries.
ix_media_latitudeandix_media_longitudeare separate single-column indexes. The new/map/markersendpoint (backend/src/find_api/routers/map.py) filters on both columns simultaneously for bounding-box queries; a composite index on(latitude, longitude)would generally serve combined range predicates more efficiently than two single-column B-tree indexes (which Postgres can only combine via bitmap-AND).♻️ Suggested composite index
conn.execute( text( - "CREATE INDEX IF NOT EXISTS ix_media_latitude " - "ON media (latitude)" + "CREATE INDEX IF NOT EXISTS ix_media_lat_lon " + "ON media (latitude, longitude)" ) ) - conn.execute( - text( - "CREATE INDEX IF NOT EXISTS ix_media_longitude " - "ON media (longitude)" - ) - )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/src/find_api/core/database.py` around lines 261 - 272, Update the index creation logic in the database initialization flow to add a composite index covering media latitude and longitude for the bounding-box queries used by the map markers endpoint. Use a distinct, descriptive index name and preserve the existing single-column indexes unless they are explicitly no longer needed.backend/src/find_api/routers/config.py (2)
71-72: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winRepeated hardware detection without caching.
Both
/config/hardwareand the new/config/runtimecalldetect_capabilities()/resolve_execution()on every request. If capability probing has any real cost (e.g., CUDA queries), consider caching the result for a short TTL, since these are informational endpoints likely polled by the settings UI.Also applies to: 99-100
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/src/find_api/routers/config.py` around lines 71 - 72, Cache the capability report and resolved execution plan used by the `/config/hardware` and `/config/runtime` handlers, with a short TTL so repeated polling reuses recent results while still reflecting hardware changes. Update the flows around `detect_capabilities()` and `resolve_execution()` to read or refresh the shared cache rather than probing on every request.
40-42: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the unused
_effective_accel_modehelper.get_hardware_capabilities()now readsload_runtime_preferences(db).accel_modedirectly, so this function is dead code.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/src/find_api/routers/config.py` around lines 40 - 42, Remove the unused _effective_accel_mode helper, including its docstring, from the configuration router module. Leave get_hardware_capabilities() using load_runtime_preferences(db).accel_mode directly and preserve all other setting access behavior.backend/src/find_api/routers/map.py (1)
80-93: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winUnbounded marker result set.
_marker_query(...).all()has noLIMIT/pagination. For libraries with many geotagged photos this returns every matching row on every request. The docstring implies the design intent is client-side clustering over the full dataset, but a runaway result set (tens of thousands of rows) could still degrade response time/memory on large libraries. Consider a sane upper bound (e.g., aLIMITwith atotal_estimate/truncatedflag) as a safeguard even if the UI expects "all" points.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/src/find_api/routers/map.py` around lines 80 - 93, Bound the marker query result in the map router by applying a sane maximum before materializing rows, while preserving the existing ordering. Expose whether the result was truncated (and, if supported by the response contract, a total estimate) so clients can distinguish capped results from the complete dataset; update the surrounding response logic and documentation as needed.backend/src/find_api/workers/jobs.py (1)
327-346: 🗄️ Data Integrity & Integration | 🔵 Trivial | 💤 Low valueKeep clustering skip payloads consistent
cluster_imagesandcluster_facesnow returnstatus: "skipped"for the AI-disabled/non-full paths, but the other skip exits still omitstatus. Add it to all skip returns, or remove it here, so the job result shape stays uniform.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/src/find_api/workers/jobs.py` around lines 327 - 346, Update all skip-return paths in cluster_images and cluster_faces to use a consistent result shape: each must include status: "skipped" alongside the existing fields, including AI-disabled and non-full-mode exits. Preserve the current status values and payload messages for successful and failed results.backend/pyproject.toml (1)
36-51: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueInconsistent pinning strategy within the same extras groups.
Most ML deps here are exact-pinned (
torch==2.8.0,onnxruntime==1.17.0,insightface==0.7.3,scikit-learn==1.5.0, ...) but a few use floating lower bounds (open-clip-torch>=2.24.0,ultralytics>=8.2.0,timm>=0.9.16,einops>=0.8.0,paddlepaddle>=3.2.2,<3.3,paddleocr>=3.5.0,<4). Mixing strategies within the same optional-dependency group makes builds less reproducible for the floating ones.Also applies to: 52-67
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/pyproject.toml` around lines 36 - 51, Standardize dependency version constraints in the cpu and corresponding optional-dependency extras so ML packages use exact pins consistently instead of floating lower bounds or ranges. Update open-clip-torch, ultralytics, timm, einops, paddlepaddle, and paddleocr to explicit tested versions while preserving the existing dependency set.frontend/src/components/vault/vault-client.ts (1)
1-79: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider consolidating these vault API helpers into
frontend/src/lib/api.ts.This new module defines API types/functions (
VaultListItem,unlockVault,listVaultItems,lockVaultSession,fetchVaultThumbnail,fetchVaultOriginal,restoreVaultItem) outside offrontend/src/lib/api.ts, which the project guidelines call out as the canonical home for frontend API types/helpers.As per coding guidelines: "Keep frontend API types and helpers in
frontend/src/lib/api.tswhen changing contracts" and "Keep shared frontend API types and functions infrontend/src/lib/api.ts."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/components/vault/vault-client.ts` around lines 1 - 79, Move the vault API types and helpers—VaultListItem, unlockVault, listVaultItems, lockVaultSession, fetchVaultThumbnail, fetchVaultOriginal, restoreVaultItem, and isExpiredVaultSession—into the canonical frontend API module, frontend/src/lib/api.ts, and update imports at their call sites to use that module. Remove the duplicated definitions from vault-client.ts while preserving their existing request behavior and exports.Source: Coding guidelines
frontend/src/__tests__/timeline-media-view.test.tsx (1)
5-22: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract
FakeResizeObserverinto a shared test utility.The identical
FakeResizeObserverclass is duplicated across at least 5 test files:timeline-media-view.test.tsx,archive-trash.test.tsx,public-shared-timeline.test.tsx,vault-gallery.test.tsx, andalbum-detail-viewer.test.tsx. Extracting it to a shared helper (e.g.,frontend/src/__tests__/helpers/resize-observer.ts) would eliminate drift risk and simplify maintenance.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/__tests__/timeline-media-view.test.tsx` around lines 5 - 22, Extract the duplicated FakeResizeObserver class into a shared test helper, preserving its callback behavior and ResizeObserver interface. Update timeline-media-view.test.tsx and the other identified test files to import and use the shared helper, removing their local class definitions.backend/tests/test_runtime_profile.py (1)
64-83: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReset assertion only covers
map_enabled.The test name says it verifies "all runtime seams and reset," but after
reset_runtime(tokens)onlycurrent_map_enabled()is re-asserted;current_ml_mode()andcurrent_accel_mode()aren't checked post-reset, so a reset bug in either seam wouldn't be caught here.♻️ Proposed fix
finally: reset_runtime(tokens) assert current_map_enabled() is False + assert current_ml_mode() == "disabled" + assert current_accel_mode() == "auto"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/tests/test_runtime_profile.py` around lines 64 - 83, Extend test_bound_job_preferences_drive_all_runtime_seams_and_reset to assert current_ml_mode() and current_accel_mode() after reset_runtime(tokens), alongside the existing current_map_enabled() assertion, verifying that all runtime seams restore their patched values.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/publish.yml:
- Around line 24-26: Move packages: write out of the workflow-level permissions
and add it only to the backend and web jobs, which push images. Keep contents:
read available at the workflow level and leave the metadata job with read-only
permissions.
In `@backend/src/find_api/routers/auth.py`:
- Around line 287-298: Update the profile-update commit flow in the visible user
update handler to catch SQLAlchemy IntegrityError, roll back the session, and
raise HTTPException 409 with the existing username-conflict response, matching
the established handling in setup_instance. Keep db.refresh(user) only after a
successful commit.
In `@compose.base.yml`:
- Around line 61-62: Update the published port mappings for the api and web
services in the Compose configuration to bind to 127.0.0.1, changing both
8000:8000 and 3000:3000 mappings while preserving their container ports.
- Line 105: Remove the hardcoded password from the Redis URL fallback in the RQ
worker command. Reuse the existing REDIS_URL environment configuration defined
elsewhere in compose.base.yml, ensuring the worker does not embed credentials or
introduce a second fallback connection string.
In `@frontend/src/app/gallery/page.tsx`:
- Around line 1316-1326: Update the download link in the gallery item rendering
around the Download icon so it always downloads through the frontend origin,
rather than using resolveMediaUrl with a potentially cross-origin API URL. Reuse
the existing same-origin API proxy route if available, or implement a blob-based
download flow that preserves item.filename and the current download behavior.
In `@frontend/src/app/globals.css`:
- Around line 188-190: Update the text-rendering declaration in the global
stylesheet to use the lowercase keyword required by the Stylelint
value-keyword-case rule, while leaving the surrounding font-synthesis and
font-smoothing declarations unchanged.
In `@frontend/src/components/app-shell.tsx`:
- Around line 41-140: Add a pathname-keyed effect in AppShell that closes the
drawer by calling setDrawerOpen(false) whenever the route changes, while
preserving the existing Escape handling and drawer focus cleanup. Ensure route
changes from browser navigation or programmatic navigation reset the drawer and
restore normal body overflow through the existing drawer effect.
- Around line 262-296: Update the app-shell sibling regions around the mobile
drawer—header, desktop aside, and main content—to apply inert and aria-hidden
while drawerOpen is true, while preserving their normal accessible state when
closed. Keep the drawer’s existing inert behavior unchanged and ensure
background scrolling remains locked as already implemented.
In `@frontend/src/components/vault/VaultGallery.tsx`:
- Around line 370-389: Update the restore action state in VaultGallery so
in-flight restore IDs are tracked locally rather than inferred from the shared
restoreMutation.variables. Add/remove each item ID around the restoreMutation
request and set the Restore button’s disabled state to
pendingRestoreIds.has(item.id), preserving the existing loading indicator for
the active item.
---
Nitpick comments:
In @.github/workflows/ci.yml:
- Around line 220-249: Add a finite timeout-minutes value to the
dependency-review job, matching the timeout hygiene used by the nearby
compose-check job, while leaving its existing trigger, permissions, checkout,
and dependency-review steps unchanged.
In `@backend/alembic/versions/20260712_media_gps.py`:
- Around line 18-21: Replace the separate latitude and longitude indexes in the
media migration with a composite index covering both columns, such as
`(latitude, longitude)`, to support the map endpoint’s combined bbox range
filters. Update the migration’s upgrade and downgrade paths consistently,
preserving the nullable columns and using the existing index naming conventions.
In `@backend/pyproject.toml`:
- Around line 36-51: Standardize dependency version constraints in the cpu and
corresponding optional-dependency extras so ML packages use exact pins
consistently instead of floating lower bounds or ranges. Update open-clip-torch,
ultralytics, timm, einops, paddlepaddle, and paddleocr to explicit tested
versions while preserving the existing dependency set.
In `@backend/src/find_api/core/config.py`:
- Around line 47-61: Update .env.example to document FIND_BUILD_PROFILE with its
development default and allowed values, AI_ENABLED with its true default, and
MAP_ENABLED with its false default, matching the configuration declarations in
the settings class.
In `@backend/src/find_api/core/database.py`:
- Around line 261-272: Update the index creation logic in the database
initialization flow to add a composite index covering media latitude and
longitude for the bounding-box queries used by the map markers endpoint. Use a
distinct, descriptive index name and preserve the existing single-column indexes
unless they are explicitly no longer needed.
In `@backend/src/find_api/routers/config.py`:
- Around line 71-72: Cache the capability report and resolved execution plan
used by the `/config/hardware` and `/config/runtime` handlers, with a short TTL
so repeated polling reuses recent results while still reflecting hardware
changes. Update the flows around `detect_capabilities()` and
`resolve_execution()` to read or refresh the shared cache rather than probing on
every request.
- Around line 40-42: Remove the unused _effective_accel_mode helper, including
its docstring, from the configuration router module. Leave
get_hardware_capabilities() using load_runtime_preferences(db).accel_mode
directly and preserve all other setting access behavior.
In `@backend/src/find_api/routers/map.py`:
- Around line 80-93: Bound the marker query result in the map router by applying
a sane maximum before materializing rows, while preserving the existing
ordering. Expose whether the result was truncated (and, if supported by the
response contract, a total estimate) so clients can distinguish capped results
from the complete dataset; update the surrounding response logic and
documentation as needed.
In `@backend/src/find_api/workers/jobs.py`:
- Around line 327-346: Update all skip-return paths in cluster_images and
cluster_faces to use a consistent result shape: each must include status:
"skipped" alongside the existing fields, including AI-disabled and non-full-mode
exits. Preserve the current status values and payload messages for successful
and failed results.
In `@backend/tests/test_runtime_profile.py`:
- Around line 64-83: Extend
test_bound_job_preferences_drive_all_runtime_seams_and_reset to assert
current_ml_mode() and current_accel_mode() after reset_runtime(tokens),
alongside the existing current_map_enabled() assertion, verifying that all
runtime seams restore their patched values.
In `@backend/tests/test_search.py`:
- Around line 106-111: Remove the redundant local patch(...) blocks for
resolve_runtime, bind_runtime, and reset_runtime in the affected test helpers,
relying on the autouse clear_cache_between_tests fixture. If a helper-specific
override is required, preserve the fixture’s complete mock shape, including
configured_accel_mode="cpu", rather than using a reduced MagicMock.
In `@frontend/src/__tests__/timeline-media-view.test.tsx`:
- Around line 5-22: Extract the duplicated FakeResizeObserver class into a
shared test helper, preserving its callback behavior and ResizeObserver
interface. Update timeline-media-view.test.tsx and the other identified test
files to import and use the shared helper, removing their local class
definitions.
In `@frontend/src/components/vault/vault-client.ts`:
- Around line 1-79: Move the vault API types and helpers—VaultListItem,
unlockVault, listVaultItems, lockVaultSession, fetchVaultThumbnail,
fetchVaultOriginal, restoreVaultItem, and isExpiredVaultSession—into the
canonical frontend API module, frontend/src/lib/api.ts, and update imports at
their call sites to use that module. Remove the duplicated definitions from
vault-client.ts while preserving their existing request behavior and exports.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 73a57ec6-e860-447a-958f-c3d902c72399
⛔ Files ignored due to path filters (3)
backend/uv.lockis excluded by!**/*.lockfrontend/pnpm-lock.yamlis excluded by!**/pnpm-lock.yamlfrontend/src-tauri/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (132)
.dockerignore.env.example.github/workflows/ci.yml.github/workflows/manual-ghcr-publish.yml.github/workflows/publish.yml.github/workflows/testsprite.yml.gitignoreAGENTS.mdCHANGELOG.mdCONTRIBUTING.mdGSSOC_CONTRIBUTOR_GUIDE.mdREADME.mdbackend/Dockerfilebackend/alembic/versions/20260712_media_gps.pybackend/pyproject.tomlbackend/src/find_api/__init__.pybackend/src/find_api/core/config.pybackend/src/find_api/core/database.pybackend/src/find_api/core/dependencies.pybackend/src/find_api/core/hardware.pybackend/src/find_api/core/model_manager.pybackend/src/find_api/core/runtime_profile.pybackend/src/find_api/main.pybackend/src/find_api/ml/captioner.pybackend/src/find_api/ml/clip_embedder.pybackend/src/find_api/ml/clusterer.pybackend/src/find_api/ml/face_detector.pybackend/src/find_api/ml/object_detector.pybackend/src/find_api/models/app_setting.pybackend/src/find_api/models/media.pybackend/src/find_api/routers/auth.pybackend/src/find_api/routers/config.pybackend/src/find_api/routers/gallery.pybackend/src/find_api/routers/map.pybackend/src/find_api/routers/search.pybackend/src/find_api/routers/status.pybackend/src/find_api/routers/vault.pybackend/src/find_api/utils/exif.pybackend/src/find_api/workers/jobs.pybackend/src/find_api/workers/processors.pybackend/tests/test_auth.pybackend/tests/test_cluster_images_regression.pybackend/tests/test_clustering_edge_cases.pybackend/tests/test_exif.pybackend/tests/test_gallery.pybackend/tests/test_hybrid_embedding.pybackend/tests/test_map.pybackend/tests/test_model_manager.pybackend/tests/test_runtime_profile.pybackend/tests/test_search.pybackend/tests/test_settings.pybackend/tests/test_status.pybackend/tests/test_thumbnails.pybackend/tests/test_vault.pycompose.base.ymlcompose.cpu.ymlcompose.mock.ymlcompose.no-ai.ymlcompose.ymldocker-compose.light.ymldocker-compose.ymldocs/guides/common-setup-errors.mddocs/guides/hardware-acceleration.mddocs/guides/testsprite-ci.mddocs/overhaul/inventory/lane-a-timeline-grid.mddocs/overhaul/inventory/lane-b-albums-sharing.mddocs/overhaul/inventory/lane-c-archive-favorites-trash.mddocs/overhaul/inventory/lane-d-viewer-slideshow.mddocs/overhaul/inventory/lane-e-backend-api.mddocs/overhaul/inventory/lane-f-ml.mddocs/overhaul/inventory/lane-g-settings.mddocs/overhaul/inventory/lane-h-mobile-desktop.mddocs/plans/not-started/desktop-runtime-comparison.mddocs/plans/not-started/remote-acceleration.mddocs/plans/not-started/storage-provider-neutrality-adr.mdfrontend/e2e/app-shell.spec.tsfrontend/package.jsonfrontend/public/maps/NATURAL_EARTH_NOTICE.mdfrontend/public/maps/ne_110m_land.geojsonfrontend/src-tauri/Cargo.tomlfrontend/src-tauri/tauri.conf.jsonfrontend/src/__tests__/account-auth.test.tsxfrontend/src/__tests__/album-detail-viewer.test.tsxfrontend/src/__tests__/app-shell.test.tsxfrontend/src/__tests__/archive-trash.test.tsxfrontend/src/__tests__/asset-viewer.test.tsxfrontend/src/__tests__/map-page.test.tsxfrontend/src/__tests__/media-timeline.test.tsfrontend/src/__tests__/private-map-style.test.tsfrontend/src/__tests__/public-shared-timeline.test.tsxfrontend/src/__tests__/root-page.test.tsxfrontend/src/__tests__/settings-page.test.tsxfrontend/src/__tests__/timeline-media-view.test.tsxfrontend/src/__tests__/vault-gallery.test.tsxfrontend/src/__tests__/vault-store.test.tsfrontend/src/app/account/page.tsxfrontend/src/app/albums/[id]/page.tsxfrontend/src/app/albums/page.tsxfrontend/src/app/archive/page.tsxfrontend/src/app/auth/login/page.tsxfrontend/src/app/auth/setup/page.tsxfrontend/src/app/clusters/page.tsxfrontend/src/app/gallery/page.tsxfrontend/src/app/globals.cssfrontend/src/app/layout.tsxfrontend/src/app/map/page.tsxfrontend/src/app/page.tsxfrontend/src/app/people/page.tsxfrontend/src/app/public/shared/[key]/page.tsxfrontend/src/app/search/page.tsxfrontend/src/app/settings/page.tsxfrontend/src/app/timeline/page.tsxfrontend/src/app/trash/page.tsxfrontend/src/components/CursorGlow.tsxfrontend/src/components/NavBar.tsxfrontend/src/components/ai-runtime-settings.tsxfrontend/src/components/app-shell.tsxfrontend/src/components/asset-viewer.tsxfrontend/src/components/hardware-accel-settings.tsxfrontend/src/components/justified-grid.tsxfrontend/src/components/map-privacy-settings.tsxfrontend/src/components/map-timeline-panel.tsxfrontend/src/components/private-map.module.cssfrontend/src/components/private-map.tsxfrontend/src/components/timeline-media-view.tsxfrontend/src/components/vault/VaultGallery.tsxfrontend/src/components/vault/VaultUnlock.tsxfrontend/src/components/vault/vault-client.tsfrontend/src/lib/api.tsfrontend/src/lib/media-timeline.tsfrontend/src/lib/viewer-preload.tsplan.md
💤 Files with no reviewable changes (13)
- .github/workflows/manual-ghcr-publish.yml
- docs/overhaul/inventory/lane-a-timeline-grid.md
- docs/overhaul/inventory/lane-d-viewer-slideshow.md
- docs/overhaul/inventory/lane-b-albums-sharing.md
- docs/overhaul/inventory/lane-e-backend-api.md
- docker-compose.light.yml
- frontend/src/components/CursorGlow.tsx
- docs/overhaul/inventory/lane-g-settings.md
- docs/overhaul/inventory/lane-c-archive-favorites-trash.md
- docs/overhaul/inventory/lane-f-ml.md
- .gitignore
- docs/overhaul/inventory/lane-h-mobile-desktop.md
- docker-compose.yml
Abhash-Chakraborty
added
ready-to-merge
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/pyproject.toml (1)
21-22: 🚀 Performance & Scalability | 🟡 Minor | ⚡ Quick winKeep the
sklearnimport out of module scopebackend/src/find_api/ml/clusterer.pystill importsHDBSCANat import time, so this path will raise unlessscikit-learnis present in the worker environment. Move it inside the helper or keep the dependency in the matching extra.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/pyproject.toml` around lines 21 - 22, Move the HDBSCAN/sklearn import in clusterer.py out of module scope and into the helper that uses it, or ensure scikit-learn is included in the matching dependency extra. Keep module import safe when scikit-learn is unavailable in the worker environment.
🧹 Nitpick comments (6)
.env.example (1)
109-109: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winKeep
BLIP_MODELin the linter’s canonical order.
dotenv-linterreports thatBLIP_MODELshould appear beforeCLIP_MODEL. Reordering it avoids a persistent lint warning without changing behavior.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.env.example at line 109, Reorder the BLIP_MODEL entry in .env.example so it appears before CLIP_MODEL, preserving both variable values and behavior.Source: Linters/SAST tools
frontend/src/lib/theme.ts (1)
6-20: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winSafeguard DOM APIs against server-side rendering (SSR) environments.
resolveThemeandapplyThemePreferenceaccesswindowanddocumentwithout checking if they are defined. While currently only called from client-side hooks, they are exported utilities. If they are ever invoked during Next.js SSR (e.g., for computing initial server state), this will throw aReferenceErrorand crash the render.Adding a simple
typeofguard prevents unexpected panics in the future.🛡️ Proposed fixes
export function resolveTheme(preference: ThemePreference): "light" | "dark" { if (preference !== "system") return preference; + if (typeof window === "undefined") return "light"; return window.matchMedia?.("(prefers-color-scheme: dark)").matches ? "dark" : "light"; } export function applyThemePreference(preference: ThemePreference) { + if (typeof document === "undefined") return; const theme = resolveTheme(preference); document.documentElement.classList.remove("light", "dark");🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/lib/theme.ts` around lines 6 - 20, Guard the browser API access in resolveTheme and applyThemePreference with typeof checks so both exported utilities are safe when invoked during SSR. Preserve the existing client-side theme resolution and DOM updates, while returning a safe theme fallback from resolveTheme and skipping DOM mutations in applyThemePreference when window or document is unavailable.frontend/src/app/search/page.tsx (2)
292-299: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winEmpty
labelonFeedbackRatingproduces a malformed aria-label.With
label="",FeedbackRating'saria-label={\Rate ${label} ${rating} out of 5`}` renders as "Rate 3 out of 5" (double space, no context). Pass a meaningful label (e.g. the image filename) for a clean screen-reader announcement.♻️ Proposed fix
<FeedbackRating - label="" + label={result.metadata.filename} onRate={(rating) => submitSearchRating(result.media_id, rating) } />🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/app/search/page.tsx` around lines 292 - 299, Update the FeedbackRating usage in the search result rendering to pass a meaningful non-empty label, such as the result’s image filename or other available media identifier, instead of "". Keep the existing submitSearchRating callback unchanged.
211-231: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winUse
next/linkfor internal gallery navigation.These thumbnails link to
/gallery?media=...via plain<a>tags, causing full page reloads instead of client-side navigation.♻️ Proposed fix
-import Image from "next/image"; +import Image from "next/image"; +import Link from "next/link";- <a - key={item.id} - href={`/gallery?media=${item.id}`} - className="group relative aspect-square overflow-hidden rounded-xl bg-[color:var(--surface-soft)] outline-none focus-visible:ring-2 focus-visible:ring-[color:var(--blue)]" - > + <Link + key={item.id} + href={`/gallery?media=${item.id}`} + className="group relative aspect-square overflow-hidden rounded-xl bg-[color:var(--surface-soft)] outline-none focus-visible:ring-2 focus-visible:ring-[color:var(--blue)]" + >🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/app/search/page.tsx` around lines 211 - 231, Replace the plain anchor wrapping each search result thumbnail with Next.js `Link` for the internal `/gallery?media=${item.id}` navigation, preserving the existing key, classes, image content, and destination.frontend/src/__tests__/search-page.test.tsx (1)
7-7: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMock wiring is correct, but consider covering the new URL-driven search init.
getGallerymock is properly added and matches therecentQuerycall inpage.tsx. Consider adding a test that renders with a?q=search param to exercise the new initial-query effect and the "Recently uploaded" panel, since that logic has no coverage here.Also applies to: 18-18, 89-89
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/__tests__/search-page.test.tsx` at line 7, Add a test in the search page test suite that renders the page with a ?q= search parameter, verifies the initial query is applied by the URL-driven initialization effect, and confirms the “Recently uploaded” panel is displayed. Reuse the existing getGallery mock and test setup, covering the corresponding initial render behavior without changing unrelated cases.backend/src/find_api/routers/vault.py (1)
481-501: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winPossible redundant credential establishment in
setup_vault.
_create_vault_config(db, payload.passphrase)is called immediately followed by_replace_vault_credentials(db, payload.passphrase, ...). If_create_vault_configalso derives a master key (as it appears to, based on its use for lazy vault creation elsewhere), this performs two full Argon2id derivations back-to-back for a single setup call, which is deliberately expensive. Please confirm whether_create_vault_confighere is only being used to guarantee theid = 1row exists before_replace_vault_credentials's UPDATE-only statement runs (which would otherwise silently affect zero rows on a fresh setup) — if so, consider a lighter row-creation helper instead of a full credential derivation.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/src/find_api/routers/vault.py` around lines 481 - 501, The setup_vault flow redundantly derives credentials by calling _create_vault_config before _replace_vault_credentials. Confirm the row-creation requirement and replace _create_vault_config in setup_vault with a lightweight helper that ensures the id=1 vault configuration row exists without deriving a master key, while preserving _replace_vault_credentials as the sole credential derivation and update step.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@backend/src/find_api/routers/vault.py`:
- Around line 214-297: Update the caller of _migrate_legacy_encrypted_items in
the unlock/password-rotation flow so migration failures are caught, logged with
context, and do not propagate as an authentication or password-change failure.
Preserve successful migration behavior while allowing the flow to continue when
a legacy row is missing or corrupt; avoid changing credential replacement logic.
In `@backend/src/find_api/workers/rq_worker.py`:
- Line 6: Update the SimpleWorker import in rq_worker.py to import it from the
rq.worker module, replacing the nonexistent rq.worker.worker_classes path so the
worker starts correctly with RQ 2.6.1.
In `@backend/tests/test_vault.py`:
- Around line 181-244: Extend TestVaultCredentials with a round-trip test for
POST /api/vault/password, using the setup session token to change the passphrase
and rotate the recovery code. Verify the response and stored recovery hash,
confirm the old passphrase no longer unlocks, and confirm the new passphrase
does; mirror the existing test_setup_status_and_recovery_round_trip flow and
reset rate limits where needed.
In `@frontend/src/app/search/page.tsx`:
- Around line 59-65: Replace the activeQuery comparison in the URL-seeded
initialization effect with a one-shot useRef guard, re-adding useRef to the
React import as needed. Initialize query, activeQuery, and the mutation only
once from the initial q parameter, while preserving Clear and subsequent
searches without reapplying the unchanged URL value.
In `@frontend/src/components/universal-search.tsx`:
- Around line 157-171: Update the universal search component’s clear-search
handler to restore focus to the input before the clear button unmounts. Declare
an input ref, attach it to the search input, and have the “Clear search” button
handler clear the query and call focus through that ref.
In `@frontend/src/components/vault/VaultUnlock.tsx`:
- Around line 16-29: Update the status-derived initialization flow in
VaultUnlock so status.isError is handled explicitly rather than allowing
initialized to default to true after getVaultStatus fails. Ensure query errors
do not render the “Unlock vault” form, while preserving the existing pending
behavior and normal initialized/uninitialized rendering paths.
- Around line 177-190: Update the recovery toggle rendering in VaultUnlock so it
requires both initialized and status.data?.recovery_available to be true. Keep
the existing mode-switching behavior unchanged, and hide the button when
recovery is unavailable.
In `@README.md`:
- Line 190: Update the detector name in the README sentence describing the
worker’s loaded models to match the established `YOLO26 nano` naming used
elsewhere, while leaving the other model names and metadata description
unchanged.
---
Outside diff comments:
In `@backend/pyproject.toml`:
- Around line 21-22: Move the HDBSCAN/sklearn import in clusterer.py out of
module scope and into the helper that uses it, or ensure scikit-learn is
included in the matching dependency extra. Keep module import safe when
scikit-learn is unavailable in the worker environment.
---
Nitpick comments:
In @.env.example:
- Line 109: Reorder the BLIP_MODEL entry in .env.example so it appears before
CLIP_MODEL, preserving both variable values and behavior.
In `@backend/src/find_api/routers/vault.py`:
- Around line 481-501: The setup_vault flow redundantly derives credentials by
calling _create_vault_config before _replace_vault_credentials. Confirm the
row-creation requirement and replace _create_vault_config in setup_vault with a
lightweight helper that ensures the id=1 vault configuration row exists without
deriving a master key, while preserving _replace_vault_credentials as the sole
credential derivation and update step.
In `@frontend/src/__tests__/search-page.test.tsx`:
- Line 7: Add a test in the search page test suite that renders the page with a
?q= search parameter, verifies the initial query is applied by the URL-driven
initialization effect, and confirms the “Recently uploaded” panel is displayed.
Reuse the existing getGallery mock and test setup, covering the corresponding
initial render behavior without changing unrelated cases.
In `@frontend/src/app/search/page.tsx`:
- Around line 292-299: Update the FeedbackRating usage in the search result
rendering to pass a meaningful non-empty label, such as the result’s image
filename or other available media identifier, instead of "". Keep the existing
submitSearchRating callback unchanged.
- Around line 211-231: Replace the plain anchor wrapping each search result
thumbnail with Next.js `Link` for the internal `/gallery?media=${item.id}`
navigation, preserving the existing key, classes, image content, and
destination.
In `@frontend/src/lib/theme.ts`:
- Around line 6-20: Guard the browser API access in resolveTheme and
applyThemePreference with typeof checks so both exported utilities are safe when
invoked during SSR. Preserve the existing client-side theme resolution and DOM
updates, while returning a safe theme fallback from resolveTheme and skipping
DOM mutations in applyThemePreference when window or document is unavailable.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3676aae2-ecbb-4558-aa75-3853165483a7
⛔ Files ignored due to path filters (2)
backend/uv.lockis excluded by!**/*.lockfrontend/src-tauri/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (47)
.env.example.github/workflows/publish.ymlREADME.mdbackend/alembic/versions/20260714_vault_credentials_and_storage_mode.pybackend/pyproject.tomlbackend/src/find_api/__init__.pybackend/src/find_api/core/config.pybackend/src/find_api/core/database.pybackend/src/find_api/core/model_manager.pybackend/src/find_api/core/sqlite_queue.pybackend/src/find_api/ml/captioner.pybackend/src/find_api/models/vault.pybackend/src/find_api/routers/auth.pybackend/src/find_api/routers/vault.pybackend/src/find_api/workers/jobs.pybackend/src/find_api/workers/rq_worker.pybackend/tests/test_rq_worker.pybackend/tests/test_status.pybackend/tests/test_vault.pycompose.base.ymldocs/guides/hardware-acceleration.mddocs/guides/real-ml-troubleshooting.mddocs/index.mddocs/plans/partial/vault-encryption-design.mdfrontend/biome.jsonfrontend/package.jsonfrontend/src-tauri/Cargo.tomlfrontend/src-tauri/tauri.conf.jsonfrontend/src/__tests__/app-shell.test.tsxfrontend/src/__tests__/search-page.test.tsxfrontend/src/__tests__/vault-gallery.test.tsxfrontend/src/app/gallery/page.tsxfrontend/src/app/globals.cssfrontend/src/app/search/page.tsxfrontend/src/app/settings/page.tsxfrontend/src/components/NavBar.tsxfrontend/src/components/app-shell.tsxfrontend/src/components/appearance-settings.tsxfrontend/src/components/justified-grid.tsxfrontend/src/components/map-privacy-settings.tsxfrontend/src/components/timeline-media-view.tsxfrontend/src/components/universal-search.tsxfrontend/src/components/vault/VaultGallery.tsxfrontend/src/components/vault/VaultUnlock.tsxfrontend/src/components/vault/vault-client.tsfrontend/src/lib/theme.tsplan.md
🚧 Files skipped from review as they are similar to previous changes (18)
- frontend/src-tauri/Cargo.toml
- backend/src/find_api/init.py
- frontend/src-tauri/tauri.conf.json
- backend/tests/test_status.py
- frontend/src/components/map-privacy-settings.tsx
- frontend/src/tests/vault-gallery.test.tsx
- backend/src/find_api/core/config.py
- frontend/src/tests/app-shell.test.tsx
- compose.base.yml
- frontend/src/components/timeline-media-view.tsx
- frontend/src/components/NavBar.tsx
- .github/workflows/publish.yml
- backend/src/find_api/core/model_manager.py
- frontend/src/app/settings/page.tsx
- frontend/src/components/vault/VaultGallery.tsx
- frontend/src/app/gallery/page.tsx
- frontend/src/app/globals.css
- backend/src/find_api/routers/auth.py
| {initialized && ( | ||
| <button | ||
| type="button" | ||
| onClick={() => { | ||
| setMode(mode === "recover" ? "unlock" : "recover"); | ||
| setError(null); | ||
| }} | ||
| className="text-sm text-[color:var(--silver)] underline-offset-4 hover:underline" | ||
| > | ||
| {mode === "recover" | ||
| ? "Back to unlock" | ||
| : "Forgot your vault password?"} | ||
| </button> | ||
| )} |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- VaultUnlock outline ---'
ast-grep outline frontend/src/components/vault/VaultUnlock.tsx --view expanded || true
echo '--- api outline ---'
ast-grep outline frontend/src/lib/api.ts --view expanded || true
echo '--- VaultUnlock relevant lines ---'
nl -ba frontend/src/components/vault/VaultUnlock.tsx | sed -n '1,260p'
echo '--- api relevant lines ---'
nl -ba frontend/src/lib/api.ts | sed -n '1,260p'Repository: Abhash-Chakraborty/Find
Length of output: 13379
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- VaultUnlock.tsx ---'
sed -n '1,260p' frontend/src/components/vault/VaultUnlock.tsx
echo '--- search vault status in api.ts ---'
rg -n "vault|recovery_available|getVaultStatus|VaultStatus" frontend/src/lib/api.ts frontend/src/components -S
echo '--- api.ts around vault-related matches ---'
python3 - <<'PY'
from pathlib import Path
p = Path("frontend/src/lib/api.ts")
text = p.read_text()
for needle in ["getVaultStatus", "recovery_available", "VaultStatus", "vault"]:
idx = text.find(needle)
if idx != -1:
start = max(0, text.rfind("\n", 0, idx-400))
end = text.find("\n", idx+800)
if end == -1:
end = len(text)
print(f"\n### Context for {needle} ###")
print(text[start:end])
PYRepository: Abhash-Chakraborty/Find
Length of output: 12576
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- search backend vault symbols ---'
rg -n "/api/vault/status|recovery_available|recovery_code_hash|recoverVault|/api/vault/recover|vault" . \
-g '!frontend/src/components/vault/vault-client.ts' \
-g '!frontend/src/components/vault/VaultUnlock.tsx' \
-g '!frontend/node_modules/**' \
-g '!node_modules/**' \
-g '!dist/**' \
-g '!build/**' -S
echo '--- candidate backend files ---'
fd -a "vault" . || trueRepository: Abhash-Chakraborty/Find
Length of output: 50379
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- backend vault routes around status/recover ---'
sed -n '460,570p' backend/src/find_api/routers/vault.py
echo '--- backend vault tests around recovery availability ---'
sed -n '180,230p' backend/tests/test_vault.pyRepository: Abhash-Chakraborty/Find
Length of output: 6235
Gate the recovery toggle on recovery_available
/vault/recover rejects vaults without a stored recovery hash, so showing “Forgot your vault password?” there leaves users in a dead-end. Hide it unless status.data?.recovery_available is true.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/src/components/vault/VaultUnlock.tsx` around lines 177 - 190, Update
the recovery toggle rendering in VaultUnlock so it requires both initialized and
status.data?.recovery_available to be true. Keep the existing mode-switching
behavior unchanged, and hide the button when recovery is unavailable.
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
frontend/src/app/timeline/page.tsx (2)
60-74: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winArchive/trash mutations swallow errors silently.
Neither
archiveMutationnortrashMutationhas anonErrorhandler, so a failed archive/trash request (e.g. network error) leaves the item visually removed from local state expectations with no feedback — actually worse: sinceonSuccessis what adds the id toremovedIds, a failure leaves the item in the grid with no toast telling the user the action failed. Other mutations in this codebase (reprocessMutationinimage-preview-modal.tsx,clusterMutationinclusters/page.tsx) surface failures viatoast.error.🔧 Proposed fix
+import { toast } from "sonner"; ... const archiveMutation = useMutation({ mutationFn: (mediaId: number) => setArchive(mediaId, true), onSuccess: ({ id }) => { setRemovedIds((cur) => new Set(cur).add(id)); queryClient.invalidateQueries({ queryKey: ["archive"] }); }, + onError: () => { + toast.error("Failed to archive photo. Please try again."); + }, }); const trashMutation = useMutation({ mutationFn: (mediaId: number) => trashImage(mediaId), onSuccess: ({ id }) => { setRemovedIds((cur) => new Set(cur).add(id)); queryClient.invalidateQueries({ queryKey: ["trash"] }); }, + onError: () => { + toast.error("Failed to move photo to trash. Please try again."); + }, });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/app/timeline/page.tsx` around lines 60 - 74, Add onError handlers to both archiveMutation and trashMutation that surface request failures through the existing toast.error mechanism, matching the error-feedback pattern used by reprocessMutation and clusterMutation. Keep the current onSuccess behavior unchanged.
30-45: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
likedOnlyshould react to?liked=truechanges
NavBarlinks Favorites to/timeline?liked=true, so this page can stay mounted while only the query string changes. BecauselikedOnlyis captured once fromwindow.location.search, the title anduseTimeline({ liked })filter can stay stuck on the previous mode until a full reload. Switch this touseSearchParams()so URL changes update the view, and add a regression for same-route navigation.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/app/timeline/page.tsx` around lines 30 - 45, Replace the one-time window.location-based initialization of likedOnly in the timeline page with Next.js useSearchParams(), deriving the value from the current "liked" query parameter so same-route URL changes update the title and useTimeline filter. Add a regression test covering navigation between timeline URLs with and without liked=true.frontend/src/app/albums/[id]/page.tsx (1)
97-117: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winAlbum asset count goes stale after archive/trash/delete.
removeMutationandcoverMutationboth callinvalidate()(which refreshes["album", albumId]and["albums"]in addition to["album-assets", albumId]), butarchiveMutation(99-104),trashMutation(110-115), and the viewer'sonDeletedcallback (258-260) do not. The item disappears from the grid viaremovedIds, butalbum.asset_countshown in the header (line 158) — and the album's card on/albums— will keep showing the pre-archive/trash/delete count until something else happens to invalidate those queries.🐛 Proposed fix
const archiveMutation = useMutation({ mutationFn: (mediaId: number) => setArchive(mediaId, true), onSuccess: ({ id }) => { setRemovedIds((cur) => new Set(cur).add(id)); queryClient.invalidateQueries({ queryKey: ["album-assets", albumId] }); + queryClient.invalidateQueries({ queryKey: ["album", albumId] }); + queryClient.invalidateQueries({ queryKey: ["albums"] }); queryClient.invalidateQueries({ queryKey: ["archive"] }); toast.success("Archived"); }, onError: () => toast.error("Couldn't archive"), }); const trashMutation = useMutation({ mutationFn: (mediaId: number) => trashImage(mediaId), onSuccess: ({ id }) => { setRemovedIds((cur) => new Set(cur).add(id)); queryClient.invalidateQueries({ queryKey: ["album-assets", albumId] }); + queryClient.invalidateQueries({ queryKey: ["album", albumId] }); + queryClient.invalidateQueries({ queryKey: ["albums"] }); queryClient.invalidateQueries({ queryKey: ["trash"] }); toast.success("Moved to trash"); }, onError: () => toast.error("Couldn't move to trash"), });and in
renderViewer'sonDeleted:- onDeleted={(id) => - setRemovedIds((current) => new Set(current).add(id)) - } + onDeleted={(id) => { + setRemovedIds((current) => new Set(current).add(id)); + invalidate(); + }}Also applies to: 258-260
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/app/albums/`[id]/page.tsx around lines 97 - 117, Update archiveMutation, trashMutation, and the viewer renderViewer onDeleted callback to call the existing invalidate() helper after successful archive, trash, or delete operations. Preserve their current removedIds updates, query invalidations, and toast behavior while ensuring album and albums counts refresh alongside album-assets.
🧹 Nitpick comments (3)
frontend/src/app/clusters/page.tsx (1)
252-259: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract a shared
useAiAvailability/useRuntimeConfighook. Both pages independently fetchruntime-configand derive an identicalaiUnavailable = runtime ? !runtime.ai_enabled : falseplus repeated disabled/title messaging for AI-gated buttons. Centralizing this would also make it structurally harder to miss gating a button (as happened with the "Run clustering" empty-state button inclusters/page.tsx).
frontend/src/app/clusters/page.tsx#L252-L259: replace the inlineuseQuery(["runtime-config"], getRuntimeConfig)+aiUnavailablederivation with a shared hook exporting{ aiUnavailable, unavailableMessage }.frontend/src/app/people/page.tsx#L206-L210: consume the same shared hook instead of re-derivingaiUnavailablelocally.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/app/clusters/page.tsx` around lines 252 - 259, The runtime AI availability logic is duplicated across the clusters and people pages. In frontend/src/app/clusters/page.tsx lines 252-259, add or consume a shared useAiAvailability/useRuntimeConfig hook that returns aiUnavailable and unavailableMessage, replacing the local runtime-config query and derivation while preserving clustering-specific messaging; in frontend/src/app/people/page.tsx lines 206-210, consume the same hook instead of deriving aiUnavailable locally, and apply its shared unavailable message to all AI-gated buttons.frontend/src/components/asset-viewer.tsx (1)
88-94: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate body-scroll-lock logic in two modals. Both
AssetViewerandAlbumAssetPickerindependently capture/restoredocument.body.style.overflow, which is a DRY violation and risks a stale unlock if two such overlays ever stack.
frontend/src/components/asset-viewer.tsx#L88-L94: extract this effect into a shareduseBodyScrollLock()hook.frontend/src/components/album-asset-picker.tsx#L38-L49: reuse the same shared hook instead of re-implementing the overflow capture/restore.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/components/asset-viewer.tsx` around lines 88 - 94, Extract the document-body overflow effect from AssetViewer into a shared useBodyScrollLock() hook, then replace the duplicate effect in frontend/src/components/asset-viewer.tsx:88-94 and frontend/src/components/album-asset-picker.tsx:38-49 with that hook. Preserve capturing the previous overflow value and restoring it during cleanup, while centralizing the behavior to avoid stale unlocks when overlays stack.backend/tests/test_settings.py (1)
76-86: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winNo test for the
trash_retention_days = 0("never delete") case.
-1is tested as rejected, but0— whichpurge_expired_trashtreats as a meaningful "disable auto-purge" value — isn't asserted as accepted anywhere in this file. Worth a dedicated case given it's a documented, user-facing mode ("never-delete").✅ Proposed test addition
assert ( client.put("/api/settings", json={"trash_retention_days": -1}).status_code == 422 ) + + def test_trash_retention_zero_disables_purge(self, client): + response = client.put("/api/settings", json={"trash_retention_days": 0}) + assert response.status_code == 200 + assert response.json()["trash_retention_days"] == 0🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/tests/test_settings.py` around lines 76 - 86, Add coverage in test_trash_retention_is_dashboard_configurable for setting trash_retention_days to 0, asserting the update succeeds and the returned and subsequently fetched settings preserve 0 as the documented never-delete value, while leaving the existing positive and negative cases intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@backend/src/find_api/routers/gallery.py`:
- Around line 784-786: Wrap the purge_expired_trash call in get_trash with
exception handling so database, cluster-removal, or other purge failures are
logged without propagating into the trash listing response. Preserve the
existing purge-on-open behavior and continue returning the listing when purging
fails. Add focused tests covering both purge failure not blocking GET /trash and
purge invocation during the trash request.
In `@CHANGELOG.md`:
- Around line 7-33: Add a distinct [1.1.1] changelog section between [1.1.2] and
[1.1.0], documenting the shipped vault setup/recovery/rotation/auto-lock,
sidebar collapse, and header/search changes described in the release plan and PR
scope.
In `@frontend/src/app/clusters/page.tsx`:
- Around line 252-259: Update the "Run clustering" button in the
no-stable-clusters empty state to include aiUnavailable in its disabled
condition, matching isClusterButtonDisabled and the equivalent people page
behavior while preserving the existing isClusterActionBusy check.
In `@frontend/src/components/album-asset-picker.tsx`:
- Around line 99-218: Update the album picker dialog component to move focus to
the dialog when it opens, contain Tab and Shift+Tab navigation within the
dialog, and restore focus to the triggering element when onClose runs. Use the
existing dialog section and close behavior, ensuring focusable elements behind
the overlay cannot receive focus.
In `@frontend/src/components/image-preview-modal.tsx`:
- Around line 246-273: Update both history URL updates in the syncUrl effects to
call window.history.replaceState directly instead of
History.prototype.replaceState.call. Preserve the existing window.history.state
as the first argument in both calls, while retaining the current URLs and the {
findImageViewer: true } marker behavior as applicable.
In `@frontend/src/components/trash-retention-settings.tsx`:
- Around line 32-37: Make the retention explanation in the Info tooltip
accessible from the keyboard and to screen readers. Update the Info wrapper in
the trash retention settings component to use an appropriate focusable,
semantically labeled tooltip trigger while preserving the existing explanatory
text and visual styling.
---
Outside diff comments:
In `@frontend/src/app/albums/`[id]/page.tsx:
- Around line 97-117: Update archiveMutation, trashMutation, and the viewer
renderViewer onDeleted callback to call the existing invalidate() helper after
successful archive, trash, or delete operations. Preserve their current
removedIds updates, query invalidations, and toast behavior while ensuring album
and albums counts refresh alongside album-assets.
In `@frontend/src/app/timeline/page.tsx`:
- Around line 60-74: Add onError handlers to both archiveMutation and
trashMutation that surface request failures through the existing toast.error
mechanism, matching the error-feedback pattern used by reprocessMutation and
clusterMutation. Keep the current onSuccess behavior unchanged.
- Around line 30-45: Replace the one-time window.location-based initialization
of likedOnly in the timeline page with Next.js useSearchParams(), deriving the
value from the current "liked" query parameter so same-route URL changes update
the title and useTimeline filter. Add a regression test covering navigation
between timeline URLs with and without liked=true.
---
Nitpick comments:
In `@backend/tests/test_settings.py`:
- Around line 76-86: Add coverage in
test_trash_retention_is_dashboard_configurable for setting trash_retention_days
to 0, asserting the update succeeds and the returned and subsequently fetched
settings preserve 0 as the documented never-delete value, while leaving the
existing positive and negative cases intact.
In `@frontend/src/app/clusters/page.tsx`:
- Around line 252-259: The runtime AI availability logic is duplicated across
the clusters and people pages. In frontend/src/app/clusters/page.tsx lines
252-259, add or consume a shared useAiAvailability/useRuntimeConfig hook that
returns aiUnavailable and unavailableMessage, replacing the local runtime-config
query and derivation while preserving clustering-specific messaging; in
frontend/src/app/people/page.tsx lines 206-210, consume the same hook instead of
deriving aiUnavailable locally, and apply its shared unavailable message to all
AI-gated buttons.
In `@frontend/src/components/asset-viewer.tsx`:
- Around line 88-94: Extract the document-body overflow effect from AssetViewer
into a shared useBodyScrollLock() hook, then replace the duplicate effect in
frontend/src/components/asset-viewer.tsx:88-94 and
frontend/src/components/album-asset-picker.tsx:38-49 with that hook. Preserve
capturing the previous overflow value and restoring it during cleanup, while
centralizing the behavior to avoid stale unlocks when overlays stack.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3990a1de-fec1-44da-86d5-79658b33757b
⛔ Files ignored due to path filters (2)
backend/uv.lockis excluded by!**/*.lockfrontend/src-tauri/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (38)
CHANGELOG.mdbackend/pyproject.tomlbackend/src/find_api/__init__.pybackend/src/find_api/routers/config.pybackend/src/find_api/routers/gallery.pybackend/tests/test_settings.pydocs/plans/partial/vault-encryption-design.mdfrontend/package.jsonfrontend/src-tauri/Cargo.tomlfrontend/src-tauri/tauri.conf.jsonfrontend/src/__tests__/a11y-smoke.test.tsxfrontend/src/__tests__/album-detail-viewer.test.tsxfrontend/src/__tests__/asset-viewer.test.tsxfrontend/src/__tests__/hardware-accel-settings.test.tsxfrontend/src/__tests__/settings-page.test.tsxfrontend/src/__tests__/timeline-page.test.tsxfrontend/src/app/account/page.tsxfrontend/src/app/albums/[id]/page.tsxfrontend/src/app/archive/page.tsxfrontend/src/app/clusters/page.tsxfrontend/src/app/duplicates/page.tsxfrontend/src/app/globals.cssfrontend/src/app/image/[id]/page.tsxfrontend/src/app/people/page.tsxfrontend/src/app/search/page.tsxfrontend/src/app/settings/page.tsxfrontend/src/app/timeline/page.tsxfrontend/src/app/trash/page.tsxfrontend/src/app/upload/page.tsxfrontend/src/components/album-asset-picker.tsxfrontend/src/components/app-shell.tsxfrontend/src/components/asset-viewer.tsxfrontend/src/components/hardware-accel-settings.tsxfrontend/src/components/image-preview-modal.tsxfrontend/src/components/private-map.tsxfrontend/src/components/trash-retention-settings.tsxfrontend/src/lib/api.tsplan.md
🚧 Files skipped from review as they are similar to previous changes (19)
- backend/src/find_api/init.py
- frontend/src-tauri/Cargo.toml
- frontend/package.json
- frontend/src/tests/asset-viewer.test.tsx
- frontend/src/app/archive/page.tsx
- frontend/src-tauri/tauri.conf.json
- docs/plans/partial/vault-encryption-design.md
- frontend/src/app/trash/page.tsx
- frontend/src/app/settings/page.tsx
- frontend/src/tests/settings-page.test.tsx
- frontend/src/components/hardware-accel-settings.tsx
- frontend/src/app/account/page.tsx
- frontend/src/components/private-map.tsx
- frontend/src/lib/api.ts
- backend/pyproject.toml
- frontend/src/components/app-shell.tsx
- frontend/src/app/globals.css
- backend/src/find_api/routers/config.py
- frontend/src/app/search/page.tsx
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/src/app/upload/page.tsx (1)
270-279: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winExclude historical uploads from current progress.
trackedUploadsincludes persisted indexed items. Consequently, subsequent uploads bypass liveuploadProgressand average current work with old 100% entries, producing inflated progress.Proposed fix
const progressPercent = - trackedUploads.length > 0 + isUploading + ? uploadProgress + : activeJobs.length > 0 ? Math.round( - trackedUploads.reduce((total, item) => { + activeJobs.reduce((total, item) => { return total + getUploadItemProgress(item); - }, 0) / trackedUploads.length, + }, 0) / activeJobs.length, ) - : isUploading - ? uploadProgress - : 0; + : 0;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/app/upload/page.tsx` around lines 270 - 279, Update the progress calculation near progressPercent to exclude persisted historical items from trackedUploads before averaging. Use only uploads belonging to the current upload operation; when none are active, preserve the existing uploadProgress fallback while isUploading and 0 otherwise.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@frontend/src/components/upload-status-indicator.tsx`:
- Around line 18-57: Stabilize the polling effect in the activeJobs flow by
deriving a primitive string from the active job IDs and using it as the
dependency instead of the activeJobs array reference. Ensure the effect only
restarts and immediately polls when the active job ID set changes, while
retaining the 2500ms interval and existing update/invalidation behavior.
In `@frontend/src/store/uploadQueueStore.ts`:
- Around line 73-78: Update hydrateResults so uploaded results without a job_id
are marked completed rather than queued, preventing no-AI imports from remaining
pending. Preserve the existing queued state for uploaded results that include a
job_id, and retain the current handling for other statuses.
---
Outside diff comments:
In `@frontend/src/app/upload/page.tsx`:
- Around line 270-279: Update the progress calculation near progressPercent to
exclude persisted historical items from trackedUploads before averaging. Use
only uploads belonging to the current upload operation; when none are active,
preserve the existing uploadProgress fallback while isUploading and 0 otherwise.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6c634d97-abcb-489d-8b95-0fa358e3cbbd
📒 Files selected for processing (31)
CHANGELOG.mdREADME.mdbackend/src/find_api/routers/gallery.pybackend/src/find_api/routers/vault.pybackend/src/find_api/workers/rq_worker.pybackend/tests/test_gallery.pybackend/tests/test_settings.pybackend/tests/test_vault.pyfrontend/e2e/app-shell.spec.tsfrontend/src/__tests__/search-page.test.tsxfrontend/src/__tests__/timeline-page.test.tsxfrontend/src/__tests__/upload-queue.test.tsfrontend/src/app/albums/[id]/page.tsxfrontend/src/app/clusters/page.tsxfrontend/src/app/people/page.tsxfrontend/src/app/providers.tsxfrontend/src/app/search/page.tsxfrontend/src/app/timeline/page.tsxfrontend/src/app/upload/page.tsxfrontend/src/components/album-asset-picker.tsxfrontend/src/components/app-shell.tsxfrontend/src/components/asset-viewer.tsxfrontend/src/components/image-preview-modal.tsxfrontend/src/components/trash-retention-settings.tsxfrontend/src/components/universal-search.tsxfrontend/src/components/upload-status-indicator.tsxfrontend/src/components/vault/VaultUnlock.tsxfrontend/src/lib/api.tsfrontend/src/lib/use-ai-availability.tsfrontend/src/lib/use-body-scroll-lock.tsfrontend/src/store/uploadQueueStore.ts
🚧 Files skipped from review as they are similar to previous changes (17)
- backend/src/find_api/workers/rq_worker.py
- backend/tests/test_gallery.py
- frontend/src/components/universal-search.tsx
- frontend/src/app/timeline/page.tsx
- frontend/src/components/album-asset-picker.tsx
- frontend/src/components/vault/VaultUnlock.tsx
- frontend/src/components/asset-viewer.tsx
- frontend/src/components/app-shell.tsx
- CHANGELOG.md
- frontend/src/app/albums/[id]/page.tsx
- backend/src/find_api/routers/gallery.py
- README.md
- frontend/src/tests/timeline-page.test.tsx
- frontend/src/app/search/page.tsx
- frontend/src/components/image-preview-modal.tsx
- frontend/src/lib/api.ts
- backend/src/find_api/routers/vault.py
| const activeJobs = useMemo( | ||
| () => | ||
| items.filter( | ||
| (item) => | ||
| item.job_id && | ||
| item.status === "uploaded" && | ||
| item.processingState !== "indexed" && | ||
| item.processingState !== "failed", | ||
| ), | ||
| [items], | ||
| ); | ||
|
|
||
| useEffect(() => { | ||
| if (activeJobs.length === 0) return; | ||
| let cancelled = false; | ||
| const poll = async () => { | ||
| const settled = await Promise.allSettled( | ||
| activeJobs.map((item) => getJobStatus(item.job_id ?? "")), | ||
| ); | ||
| if (cancelled) return; | ||
| let terminalUpdate = false; | ||
| for (const result of settled) { | ||
| if (result.status !== "fulfilled") continue; | ||
| updateJob(result.value); | ||
| terminalUpdate ||= | ||
| result.value.status === "finished" || | ||
| result.value.status === "failed"; | ||
| } | ||
| if (terminalUpdate) { | ||
| void queryClient.invalidateQueries({ queryKey: ["gallery"] }); | ||
| void queryClient.invalidateQueries({ queryKey: ["gallery-infinite"] }); | ||
| } | ||
| }; | ||
| void poll(); | ||
| const interval = window.setInterval(() => void poll(), 2500); | ||
| return () => { | ||
| cancelled = true; | ||
| window.clearInterval(interval); | ||
| }; | ||
| }, [activeJobs, queryClient, updateJob]); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
Unthrottled polling loop (runaway API spam).
The activeJobs array creates a new reference every time items changes in the store. When poll() completes and calls updateJob(), the store updates, triggering a re-render. This provides a new activeJobs reference, which causes the useEffect to clear its interval and immediately call poll() again. This entirely bypasses the 2500ms interval and results in an unthrottled request loop hitting the backend API.
To fix this, derive a primitive string of the active job IDs to use in the dependency array. This guarantees the effect only re-runs (and immediately polls) when the set of jobs actually changes, rather than when a job's internal state updates.
🔒️ Proposed fix to stabilize the effect dependencies
const updateJob = useUploadQueueStore((state) => state.updateJob);
const clearCompleted = useUploadQueueStore((state) => state.clearCompleted);
const activeJobs = useMemo(
() =>
items.filter(
(item) =>
item.job_id &&
item.status === "uploaded" &&
item.processingState !== "indexed" &&
item.processingState !== "failed",
),
[items],
);
+ const activeJobIds = useMemo(
+ () => activeJobs.map((item) => item.job_id ?? "").sort().join(","),
+ [activeJobs],
+ );
useEffect(() => {
- if (activeJobs.length === 0) return;
+ if (!activeJobIds) return;
let cancelled = false;
+ const jobIdsToPoll = activeJobIds.split(",");
+
const poll = async () => {
const settled = await Promise.allSettled(
- activeJobs.map((item) => getJobStatus(item.job_id ?? "")),
+ jobIdsToPoll.map((id) => getJobStatus(id)),
);
if (cancelled) return;
let terminalUpdate = false;
for (const result of settled) {
if (result.status !== "fulfilled") continue;
updateJob(result.value);
terminalUpdate ||=
result.value.status === "finished" ||
result.value.status === "failed";
}
if (terminalUpdate) {
void queryClient.invalidateQueries({ queryKey: ["gallery"] });
void queryClient.invalidateQueries({ queryKey: ["gallery-infinite"] });
}
};
void poll();
const interval = window.setInterval(() => void poll(), 2500);
return () => {
cancelled = true;
window.clearInterval(interval);
};
- }, [activeJobs, queryClient, updateJob]);
+ }, [activeJobIds, queryClient, updateJob]);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const activeJobs = useMemo( | |
| () => | |
| items.filter( | |
| (item) => | |
| item.job_id && | |
| item.status === "uploaded" && | |
| item.processingState !== "indexed" && | |
| item.processingState !== "failed", | |
| ), | |
| [items], | |
| ); | |
| useEffect(() => { | |
| if (activeJobs.length === 0) return; | |
| let cancelled = false; | |
| const poll = async () => { | |
| const settled = await Promise.allSettled( | |
| activeJobs.map((item) => getJobStatus(item.job_id ?? "")), | |
| ); | |
| if (cancelled) return; | |
| let terminalUpdate = false; | |
| for (const result of settled) { | |
| if (result.status !== "fulfilled") continue; | |
| updateJob(result.value); | |
| terminalUpdate ||= | |
| result.value.status === "finished" || | |
| result.value.status === "failed"; | |
| } | |
| if (terminalUpdate) { | |
| void queryClient.invalidateQueries({ queryKey: ["gallery"] }); | |
| void queryClient.invalidateQueries({ queryKey: ["gallery-infinite"] }); | |
| } | |
| }; | |
| void poll(); | |
| const interval = window.setInterval(() => void poll(), 2500); | |
| return () => { | |
| cancelled = true; | |
| window.clearInterval(interval); | |
| }; | |
| }, [activeJobs, queryClient, updateJob]); | |
| const activeJobs = useMemo( | |
| () => | |
| items.filter( | |
| (item) => | |
| item.job_id && | |
| item.status === "uploaded" && | |
| item.processingState !== "indexed" && | |
| item.processingState !== "failed", | |
| ), | |
| [items], | |
| ); | |
| const activeJobIds = useMemo( | |
| () => activeJobs.map((item) => item.job_id ?? "").sort().join(","), | |
| [activeJobs], | |
| ); | |
| useEffect(() => { | |
| if (!activeJobIds) return; | |
| let cancelled = false; | |
| const jobIdsToPoll = activeJobIds.split(","); | |
| const poll = async () => { | |
| const settled = await Promise.allSettled( | |
| jobIdsToPoll.map((id) => getJobStatus(id)), | |
| ); | |
| if (cancelled) return; | |
| let terminalUpdate = false; | |
| for (const result of settled) { | |
| if (result.status !== "fulfilled") continue; | |
| updateJob(result.value); | |
| terminalUpdate ||= | |
| result.value.status === "finished" || | |
| result.value.status === "failed"; | |
| } | |
| if (terminalUpdate) { | |
| void queryClient.invalidateQueries({ queryKey: ["gallery"] }); | |
| void queryClient.invalidateQueries({ queryKey: ["gallery-infinite"] }); | |
| } | |
| }; | |
| void poll(); | |
| const interval = window.setInterval(() => void poll(), 2500); | |
| return () => { | |
| cancelled = true; | |
| window.clearInterval(interval); | |
| }; | |
| }, [activeJobIds, queryClient, updateJob]); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/src/components/upload-status-indicator.tsx` around lines 18 - 57,
Stabilize the polling effect in the activeJobs flow by deriving a primitive
string from the active job IDs and using it as the dependency instead of the
activeJobs array reference. Ensure the effect only restarts and immediately
polls when the active job ID set changes, while retaining the 2500ms interval
and existing update/invalidation behavior.
| function hydrateResults(response: UploadResponse): UploadQueueItem[] { | ||
| return response.results.map((result) => ({ | ||
| ...result, | ||
| jobStatus: result.status === "uploaded" ? "queued" : undefined, | ||
| processingState: result.status === "uploaded" ? "queued" : undefined, | ||
| })); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Treat successful uploads without a job as completed.
Every uploaded result is marked queued, but an item without job_id can never receive updateJob. In no-AI mode this leaves successfully imported photos permanently showing as queued.
Proposed fix
function hydrateResults(response: UploadResponse): UploadQueueItem[] {
return response.results.map((result) => ({
...result,
- jobStatus: result.status === "uploaded" ? "queued" : undefined,
- processingState: result.status === "uploaded" ? "queued" : undefined,
+ jobStatus:
+ result.status === "uploaded" && result.job_id ? "queued" : undefined,
+ processingState:
+ result.status === "uploaded"
+ ? result.job_id
+ ? "queued"
+ : "indexed"
+ : undefined,
}));
}📝 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.
| function hydrateResults(response: UploadResponse): UploadQueueItem[] { | |
| return response.results.map((result) => ({ | |
| ...result, | |
| jobStatus: result.status === "uploaded" ? "queued" : undefined, | |
| processingState: result.status === "uploaded" ? "queued" : undefined, | |
| })); | |
| function hydrateResults(response: UploadResponse): UploadQueueItem[] { | |
| return response.results.map((result) => ({ | |
| ...result, | |
| jobStatus: | |
| result.status === "uploaded" && result.job_id ? "queued" : undefined, | |
| processingState: | |
| result.status === "uploaded" | |
| ? result.job_id | |
| ? "queued" | |
| : "indexed" | |
| : undefined, | |
| })); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/src/store/uploadQueueStore.ts` around lines 73 - 78, Update
hydrateResults so uploaded results without a job_id are marked completed rather
than queued, preventing no-AI imports from remaining pending. Preserve the
existing queued state for uploaded results that include a job_id, and retain the
current handling for other statuses.
Summary
Ships the v1.1.1 local-first application overhaul, including the final image-rendering, preview, vault, shell, search, Settings, and idle-worker health fixes. The local
reference-app/checkout remains ignored and untracked.Closes #338
Closes #346
Type of change
What changed
Privacy and modularity
reference-app/is ignored, excluded from Docker contexts, and rejected by CI if tracked.Screenshots / recordings (for UI changes)
Computer-driven live testing was performed against the seeded local Docker stack. Captures are intentionally not committed; README capture work remains tracked in #335.
How to test
cd backend && uv run ruff check . && uv run ruff format --check . && uv run pytest tests/ -q— 516 passed, 5 skipped.cd frontend && pnpm check && pnpm test && pnpm build— 253 passed; all 20 Next.js routes built.cd frontend/src-tauri && cargo check --locked— succeeds at v1.1.1.docker compose -f <profile> config --quiet— succeeds for default, base, no-AI, mock, and CPU configurations.Follow-up gates
Model-level NVIDIA inference under sustained load, constrained CPU/low-RAM wall-clock budgets, and a manual screen-reader pass remain hardware/human release gates tracked in #339-#345 and are not claimed here.
Checklist
GSSoC'26 checklist
Summary by CodeRabbit