feat(library): local artist images + picker UI#33
Merged
Conversation
Closes #31. Adds a folder-cover-style fallback for artist images: the scanner now walks up to three parent directories from each track and links any `artist.jpg`/`performer.png`/`<artist_name>.jpg` it finds to the `artist.artwork_id` foreign key (no migration required). Resolution priority in `get_artist_detail` becomes local sidecar → Deezer cache → live Deezer fetch, so offline libraries no longer need network access for artist photos. A new `rescan_local_artist_images` command (exposed as Settings → Library → Local artist images) backfills libraries scanned before this feature shipped. Strict name-match in single-folder layouts prevents album covers from being mistaken for artist photos, and the `Various Artists` sentinel is explicitly excluded.
Hover overlay on the artist photo opens a Spotify-style picker modal with three actions: search Deezer for a photo, upload a local file (magic-byte validated jpg/png/webp), or remove the current image so the resolution chain falls back to the Deezer cache. Backed by four new Tauri commands in commands::deezer (search, set-from-deezer, set-from-file, clear). User picks overwrite `artist.artwork_id` unconditionally — an explicit choice beats both local sidecar resolution and any cached Deezer fetch.
github-actions
Bot
added
scope: frontend
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughAdds local artist image discovery during scans, backend Deezer/file artwork commands with download caps, a React picker modal and artist-detail edit flow, a settings rescan command/UI, TypeScript invoke wrappers, i18n across locales, docs, tests, and small formatting/dep bumps. ChangesLocal and Manual Artist Image Workflow
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning Billing warning: we have not been able to collect payment for this subscription for more than 72 hours. Please update the payment method or pay any pending invoices in Billing to avoid service interruption. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 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 `@docs/features/library.md`:
- Line 83: The doc sentence claiming rescan picks a single representative track
per artist is inaccurate; update the sentence to reflect that
rescan_local_artist_images (commands/scan.rs::rescan_local_artist_images) will
probe multiple tracks per artist (up to 16) to improve hit rate when running
extract_artist_image, so change the wording from "picks a representative track"
to something like "probes up to 16 tracks per artist" and mention that
already-linked rows remain filtered at the SQL level.
In `@src-tauri/src/commands/deezer.rs`:
- Around line 650-654: The UPDATE statements that set artwork_id currently
ignore whether any row was affected; capture the result of .execute(&pool).await
into a variable, call .rows_affected() and if it is 0 return an Err (e.g., via
anyhow::anyhow! or the crate's error type) indicating "artist not found" so the
caller/UI sees failure; apply this change to the UPDATE block shown (the
sqlx::query("UPDATE artist SET artwork_id = ? WHERE id = ?") call) and the
similar UPDATEs at the other two occurrences (the blocks around the second and
third occurrences referenced in the comment).
- Around line 640-641: The code reads entire images into memory
(download_image_bytes -> used where bytes are hashed with blake3::hash) without
size limits; update the image download/load path to enforce a hard size cap
(e.g. 5–10 MiB) before buffering: modify download_image_bytes (or replace with a
new download_image_bytes_capped) to stream the response/local file, accumulate
into a bounded buffer and return an error if the payload exceeds the cap, and
update the call sites (the places that compute hash from bytes at the
blake3::hash(...) uses around the shown locations) to handle the error instead
of assuming full payload was read.
In `@src-tauri/src/commands/scan.rs`:
- Around line 1431-1451: The call to maybe_link_artist_images(...) happens
before the track_artist rebuild, so when current_count != splits.len() newly
created artist IDs are skipped; move the artist-image linking to after the
branch that rebuilds track_artist entries (the code that checks current_count vs
splits.len() and inserts new artist rows for existing_track_id using tx), or
call maybe_link_artist_images a second time after that rebuild completes,
ensuring you pass the same tx, raw, current_ids/updated IDs, track_path and
artwork_dir so newly created artist IDs are included.
In `@src/components/common/ArtistImagePickerModal.tsx`:
- Around line 55-56: The Deezer search needs a request-sequencing guard so late
responses don't clobber newer results: add a requestIdRef = useRef(0) and,
inside the async search function (the handler that currently uses debounceRef to
call Deezer), increment requestIdRef.current before firing the request, capture
it in a local const requestId, and after the awaited fetch only call setResults
if requestId === requestIdRef.current; optionally also consider using an
AbortController per request and abort the previous controller when starting a
new search. Reference debounceRef and the async search handler (the function
that performs the Deezer fetch) when making this change.
In `@src/components/views/ArtistDetailView.tsx`:
- Around line 277-285: The edit overlay button currently uses absolute inset-0
and covers the whole image so onDoubleClick on the <img> never fires; update the
overlay so it doesn't intercept image clicks — either (preferred) add
pointer-events-none to the overlay container and pointer-events-auto to the
visible interactive button (keep setIsImagePickerOpen and the aria attributes),
or shrink the button hit area (remove absolute inset-0 and use explicit
width/height and placement like right-2 bottom-2) so the <img> can receive
double-clicks/lightbox events while still preserving the edit affordance.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: f4351237-fbd4-4f30-b99f-bdb1f18b2d75
📒 Files selected for processing (27)
CLAUDE.mddocs/features/library.mdsrc-tauri/src/commands/deezer.rssrc-tauri/src/commands/scan.rssrc-tauri/src/lib.rssrc/components/common/ArtistImagePickerModal.tsxsrc/components/views/ArtistDetailView.tsxsrc/components/views/SettingsView.tsxsrc/i18n/locales/ar.jsonsrc/i18n/locales/de.jsonsrc/i18n/locales/en.jsonsrc/i18n/locales/es.jsonsrc/i18n/locales/fr.jsonsrc/i18n/locales/hi.jsonsrc/i18n/locales/id.jsonsrc/i18n/locales/it.jsonsrc/i18n/locales/ja.jsonsrc/i18n/locales/kr.jsonsrc/i18n/locales/nl.jsonsrc/i18n/locales/pt-BR.jsonsrc/i18n/locales/pt.jsonsrc/i18n/locales/ru.jsonsrc/i18n/locales/tr.jsonsrc/i18n/locales/zh-CN.jsonsrc/i18n/locales/zh-TW.jsonsrc/lib/tauri/deezer.tssrc/lib/tauri/library.ts
- docs/library: clarify rescan_local_artist_images probes up to 16 tracks per artist (not "a representative track") - deezer commands: return AppError when UPDATE artist affects 0 rows so the UI surfaces "artist not found" instead of silently succeeding - download_image_bytes: enforce 10 MiB cap, early-reject via Content-Length and bound the streaming buffer to prevent OOM from a hostile or chunked-without-length response - scan: move maybe_link_artist_images AFTER the track_artist rebuild so newly created artist IDs (when split count changes) are also probed for sidecar images - ArtistImagePickerModal: add requestIdRef so a slow earlier Deezer search can't clobber the results of a newer query - ArtistDetailView: shrink the pencil overlay to a corner button with pointer-events tweaks so the image keeps receiving double-click events (lightbox)
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@src-tauri/src/commands/scan.rs`:
- Around line 1835-1851: The loop in rescan_local_artist_images is performing
individual writes (link_local_artist_image) causing SQLite lock churn; wrap the
work in a transaction by calling pool.begin() to get a transaction (tx) and use
that transaction as the mutable connection when calling
link_local_artist_image(&mut tx, artist_id, &cover). Track a counter and commit
tx.commit().await every ~200 writes, then start a new tx = pool.begin().await;
also ensure you acquire no separate connection inside the inner loop (remove
pool.acquire()) and use extract_artist_image as-is for reads while writes use
the open transaction.
In `@src/components/common/ArtistImagePickerModal.tsx`:
- Around line 269-277: The fan-count suffix is hardcoded as "fans" in
ArtistImagePickerModal.tsx (the JSX block that renders hit.nb_fan), causing
untranslated UI; replace the literal "fans" with a localized string via your
i18n helper (e.g., use t('fans') or t('artist.fans', { count: hit.nb_fan }) or
the project's pluralization API) so the suffix is translated and respects
pluralization, and add the corresponding key(s) to the translation files; ensure
the existing numeric formatting logic for K/M remains unchanged and only the
trailing word is localized.
- Around line 73-100: When abandoning a search (either because isOpen is
false/tab !== "deezer" or trimmed.length < 2) advance/invalidate the in-flight
requestIdRef by incrementing requestIdRef.current so any pending
searchArtistsDeezer then()/.catch() handlers do not apply outdated results;
specifically, in the early-return branch for (!isOpen || tab !== "deezer") and
in the branch for trimmed.length < 2 (after clearing debounceRef) increment
requestIdRef.current and ensure you still clear any timeouts and call
setResults([])/setIsSearching(false)/setError(null) as appropriate to keep UI
consistent (referencing requestIdRef, debounceRef, searchArtistsDeezer,
setResults, setIsSearching, setError).
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 96eeffc3-0b04-4bbb-85d4-3c27c1087476
⛔ Files ignored due to path filters (2)
bun.lockis excluded by!**/*.locksrc-tauri/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (6)
docs/features/library.mdpackage.jsonsrc-tauri/src/commands/deezer.rssrc-tauri/src/commands/scan.rssrc/components/common/ArtistImagePickerModal.tsxsrc/components/views/ArtistDetailView.tsx
- scan: wrap rescan_local_artist_images in a single transaction committed every 200 writes (same pattern as scan_folder_inner) so the WAL fsyncs once per batch instead of per artist - ArtistImagePickerModal: localize the hardcoded "fans" suffix via i18next plural keys (`artistImagePicker.fansCount_*`) across all 17 locales; only the trailing word is translated, K/M number formatting is preserved - ArtistImagePickerModal: bump requestIdRef when abandoning a search (modal closed, tab switched, or query shorter than 2 chars) so any in-flight Deezer response is discarded as stale and the UI state is reset consistently
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #31.
Summary
artist.jpg/<artist_name>.jpgfiles from up to 3 parent directories and links them via the existingartist.artwork_idcolumn (no migration). Covers both common layouts:<root>/<Artist>/<Album>/trackand<root>/<Album>/trackwith<Artist>.jpgbeside it.get_artist_detailbecomes local sidecar → Deezer cache → live Deezer fetch, so offline libraries get real artist photos without network access.rescan_local_artist_images(Settings → Library → Local artist images) — needed because the scanner's mtime/size fast path otherwise skips folders whose audio files haven't changed.What's new
Backend (
src-tauri/src/commands/)scan.rs::extract_artist_image+maybe_link_artist_images— walks ancestors, matchesARTIST_IMAGE_STEMS = [artist, performer, band]or any stem whosecanonical_nameequals the artist's. Strict name-match prevents mistakingcover.jpgfor an artist photo in single-folder layouts.Various Artistsexplicitly excluded.scan.rs::rescan_local_artist_images— backfill command for libraries scanned before this feature.deezer.rs—search_artists_deezer,set_artist_artwork_from_deezer,set_artist_artwork_from_file,clear_artist_artwork. The twoset_*overwriteartwork_idunconditionally (explicit user pick > auto-resolution).Frontend
ArtistDetailViewprefersartwork_pathoverpicture_path, guards against late-arriving Deezer responses clobbering a local image, and renders a hover-revealed pencil overlay on the artist photo.ArtistImagePickerModalmirrorsCoverPickerModal(Deezer search tab + local file tab + remove action).i18n —
settings.localArtistImagesandartistImagePickerkeys propagated to all 17 locales with native translations.Docs —
CLAUDE.mdfeature catalogue anddocs/features/library.mdupdated with the resolution chain, layout examples, and manual-override flow.Test plan
bun run typecheckcleanbun run lintcleancargo check --manifest-path src-tauri/Cargo.toml --all-targetscleancargo test --manifest-path src-tauri/Cargo.toml --lib commands::scan::tests— 4 new tests pass (matches stem in parent dir / canonical-name match / ignores unrelated cover.jpg / empty canonical bail-out)artist.jpgin an artist folder, run a folder scan, confirm the photo appears on the artist page<Artist>.jpgnext to an album folder, scan, confirm matchSummary by CodeRabbit
New Features
Documentation
Localization