From 7207ec61fbdf7b7df2249a0dc3ce69ff76060cc0 Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Sun, 7 Jun 2026 13:24:46 -0700 Subject: [PATCH] Cover Art Filler: fix album art or artist art independently (Pache711) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pache711: a cover-art finding showed the (correct) found album art next to a (wrong) artist image with one "Apply Art" button — no way to take one and skip the other. Turned out "Apply Art" only ever applied ALBUM art anyway; the artist image was display-only context, so the bundling was an illusion the UI created. Now the finding is genuinely multi-target: - scan (missing_cover_art.py): also searches for an artist image (always, so a WRONG existing one can be replaced — Boulder's call), name-matched exactly. Stored as found_artist_url only when it differs from the current artist thumb, so nothing is offered when there's nothing to change. - apply (_fix_missing_cover_art): honors a target via _fix_action — 'album' (default, unchanged "Apply Art" behavior: DB thumb + embed + cover.jpg), 'artist' (the artist's DB image), or 'both'. New _fix_artist_art sets artists.thumb_url for the album's artist. - UI: each found image gets its own apply button — "Use for album" / "Use for artist". Applying either resolves the finding, so taking the correct one and ignoring the wrong one IS "fix one, dismiss the other". Current artist art shows as "(current)" context with no button. Default stays album-only, so the plain Apply Art button and every existing caller behave exactly as before. Tests: 5 on the apply targets (artist-only / album-only / default / both / missing-url) against a real SQLite DB, plus the existing cover-art suite updated for the new artist search. 107 repair/ cover-art/UI-integrity tests pass. --- core/repair_jobs/missing_cover_art.py | 41 ++++++++ core/repair_worker.py | 61 ++++++++++-- tests/test_cover_art_targets.py | 130 ++++++++++++++++++++++++++ tests/test_missing_cover_art.py | 7 +- webui/static/enrichment.js | 52 +++++++++-- webui/static/style.css | 16 ++++ 6 files changed, 291 insertions(+), 16 deletions(-) create mode 100644 tests/test_cover_art_targets.py diff --git a/core/repair_jobs/missing_cover_art.py b/core/repair_jobs/missing_cover_art.py index c4ae85b9..eb3a8bf1 100644 --- a/core/repair_jobs/missing_cover_art.py +++ b/core/repair_jobs/missing_cover_art.py @@ -203,6 +203,12 @@ class MissingCoverArtJob(RepairJob): log_line=f'Found art: {title or "Unknown"}', log_type='success' ) + # Also search for an artist image so the finding can offer it as + # an independently-applyable target (Pache711). Searched even + # when the artist already has art, so a wrong existing image can + # be swapped; the UI only surfaces it when it differs from the + # current one. + found_artist_url = self._find_artist_art(artist_name, source_priority) # Create finding for user to approve if context.create_finding: try: @@ -222,6 +228,13 @@ class MissingCoverArtJob(RepairJob): 'found_artwork_url': artwork_url, 'spotify_album_id': spotify_album_id, 'artist_thumb_url': artist_thumb or None, + # Found artist image (None if none matched, or it + # equals the current one — nothing to offer then). + 'found_artist_url': ( + found_artist_url + if found_artist_url and found_artist_url != artist_thumb + else None + ), # Where the files live + what was missing, so the # apply can embed into the audio + write cover.jpg. 'album_folder': os.path.dirname(rep_path) if rep_path else None, @@ -287,6 +300,34 @@ class MissingCoverArtJob(RepairJob): logger.debug("%s art lookup failed for '%s': %s", source.capitalize(), title, e) return None + def _find_artist_art(self, artist_name, source_priority): + """Search the configured sources for an artist image, in priority + order. Returns the first confidently name-matched artist image URL, + or None. Mirrors _try_source but for artists (Pache711: let the + Cover Art Filler offer artist art as its own fixable target).""" + if not artist_name: + return None + for source in source_priority: + client = get_client_for_source(source) + if not client or not hasattr(client, 'search_artists'): + continue + try: + for res in (client.search_artists(artist_name, limit=5) or []): + r_name = getattr(res, 'name', None) + if isinstance(res, dict): + r_name = res.get('name') + # Exact significant-word match — never hang a wrong artist + # photo on someone just because the search was fuzzy. + if not r_name or _name_tokens(r_name) != _name_tokens(artist_name): + continue + url = self._extract_artwork_url(res) + if url: + return url + except Exception as e: + logger.debug("%s artist-art lookup failed for '%s': %s", + source.capitalize(), artist_name, e) + return None + @staticmethod def _result_title_artist(item): """Pull (title, artist) from a search result that may be a dict or an diff --git a/core/repair_worker.py b/core/repair_worker.py index 88d698d0..c6993d98 100644 --- a/core/repair_worker.py +++ b/core/repair_worker.py @@ -1277,17 +1277,58 @@ class RepairWorker: logger.error("Error fixing track number for %s: %s", file_path, e) return {'success': False, 'error': str(e)} + def _fix_artist_art(self, album_id, details): + """Apply the found ARTIST image to the album's artist (DB thumb only — + artist art has no per-file embed). Pache711: independently applyable + from the album art on the same finding.""" + artist_url = details.get('found_artist_url') + if not artist_url: + return {'success': False, 'error': 'No artist image found in finding details'} + conn = None + try: + conn = self.db._get_connection() + cursor = conn.cursor() + cursor.execute( + "UPDATE artists SET thumb_url = ?, updated_at = CURRENT_TIMESTAMP " + "WHERE id = (SELECT artist_id FROM albums WHERE id = ?)", + (artist_url, album_id)) + conn.commit() + if cursor.rowcount == 0: + return {'success': False, 'error': 'Artist not found for this album'} + finally: + if conn: + conn.close() + return {'success': True, 'action': 'applied_artist_art', + 'message': 'Applied artist image'} + def _fix_missing_cover_art(self, entity_type, entity_id, file_path, details): - """Apply found artwork: update the DB thumbnail AND embed art into the - album's audio files + write cover.jpg (using the post-processing - standard, so the user's album_art_order preference is honored).""" - artwork_url = details.get('found_artwork_url') - if not artwork_url: - return {'success': False, 'error': 'No artwork URL found in finding details'} + """Apply found artwork. ``_fix_action`` selects the target (Pache711): + 'album' (default — DB thumb + embed into files + cover.jpg), 'artist' + (the artist's DB image), or 'both'. Defaulting to 'album' keeps the + plain "Apply Art" button behaving exactly as before.""" + target = (details.get('_fix_action') or 'album').strip().lower() + if target not in ('album', 'artist', 'both'): + target = 'album' + album_id = details.get('album_id') or entity_id if not album_id: return {'success': False, 'error': 'No album ID associated with this finding'} + # Artist-only path: nothing to do with album files. + if target == 'artist': + return self._fix_artist_art(album_id, details) + + artist_result = None + if target == 'both': + artist_result = self._fix_artist_art(album_id, details) + + artwork_url = details.get('found_artwork_url') + if not artwork_url: + # 'both' but no album art — report the artist outcome if that ran. + if artist_result is not None: + return artist_result + return {'success': False, 'error': 'No artwork URL found in finding details'} + conn = None track_paths = [] album_title = details.get('album_title') @@ -1332,8 +1373,10 @@ class RepairWorker: if not resolved: # Media-server-only album (no local files): DB thumbnail is all we can set. - return {'success': True, 'action': 'applied_cover_art', - 'message': 'Applied cover art to album (database only — no local files found)'} + msg = 'Applied cover art to album (database only — no local files found)' + if artist_result is not None and artist_result.get('success'): + msg += ' + applied artist image' + return {'success': True, 'action': 'applied_cover_art', 'message': msg} from core.metadata.art_apply import apply_art_to_album_files metadata = { @@ -1367,6 +1410,8 @@ class RepairWorker: if embedded == 0 and not art_result.get('cover_written'): # DB updated but nothing reached disk (e.g. permissions). msg = 'Updated database thumbnail, but could not write art to files (read-only?)' + if artist_result is not None and artist_result.get('success'): + msg += ' + applied artist image' return {'success': True, 'action': 'applied_cover_art', 'message': msg, 'art_result': art_result} def _fix_library_retag(self, entity_type, entity_id, file_path, details): diff --git a/tests/test_cover_art_targets.py b/tests/test_cover_art_targets.py new file mode 100644 index 00000000..3c4de992 --- /dev/null +++ b/tests/test_cover_art_targets.py @@ -0,0 +1,130 @@ +"""Per-target cover-art apply (Pache711: 'select one or the other to fix'). + +A missing-cover-art finding now offers album art AND artist art as +independently applyable targets. _fix_missing_cover_art routes on _fix_action: +'album' (default), 'artist', or 'both'. Verified against a real SQLite DB so +the UPDATE statements are exercised. +""" + +from __future__ import annotations + +import sys +import types + +if "spotipy" not in sys.modules: + spotipy = types.ModuleType("spotipy") + spotipy.Spotify = type("S", (), {}) + oauth2 = types.ModuleType("spotipy.oauth2") + oauth2.SpotifyOAuth = oauth2.SpotifyClientCredentials = type("O", (), {}) + spotipy.oauth2 = oauth2 + sys.modules["spotipy"] = spotipy + sys.modules["spotipy.oauth2"] = oauth2 + +if "config.settings" not in sys.modules: + config_pkg = types.ModuleType("config") + settings_mod = types.ModuleType("config.settings") + + class _Cfg: + def get(self, key, default=None): + return default + + def get_active_media_server(self): + return "plex" + + settings_mod.config_manager = _Cfg() + config_pkg.settings = settings_mod + sys.modules["config"] = config_pkg + sys.modules["config.settings"] = settings_mod + +import sqlite3 + +import pytest + +from core.repair_worker import RepairWorker + + +class _DB: + def __init__(self, path): + self.path = str(path) + conn = self._get_connection() + c = conn.cursor() + c.execute("CREATE TABLE artists (id TEXT PRIMARY KEY, name TEXT, thumb_url TEXT, updated_at TEXT)") + c.execute("CREATE TABLE albums (id TEXT PRIMARY KEY, title TEXT, artist_id TEXT, thumb_url TEXT, musicbrainz_release_id TEXT, updated_at TEXT)") + c.execute("CREATE TABLE tracks (id TEXT PRIMARY KEY, album_id TEXT, file_path TEXT)") + c.execute("INSERT INTO artists VALUES ('ar1', 'Forre Sterra', 'http://old/artist.jpg', NULL)") + c.execute("INSERT INTO albums VALUES ('al1', 'For You', 'ar1', NULL, NULL, NULL)") + conn.commit() + conn.close() + + def _get_connection(self): + return sqlite3.connect(self.path) + + +def _worker(tmp_path): + w = RepairWorker.__new__(RepairWorker) + w.db = _DB(tmp_path / "m.db") + w.transfer_folder = str(tmp_path) + w._config_manager = None + return w + + +def _thumbs(w): + conn = w.db._get_connection() + c = conn.cursor() + alb = c.execute("SELECT thumb_url FROM albums WHERE id='al1'").fetchone()[0] + art = c.execute("SELECT thumb_url FROM artists WHERE id='ar1'").fetchone()[0] + conn.close() + return alb, art + + +DETAILS = { + 'album_id': 'al1', 'album_title': 'For You', 'artist': 'Forre Sterra', + 'found_artwork_url': 'http://new/album.jpg', + 'found_artist_url': 'http://new/artist.jpg', +} + + +def test_artist_only_sets_artist_leaves_album(tmp_path): + w = _worker(tmp_path) + res = w._fix_missing_cover_art('album', 'al1', None, {**DETAILS, '_fix_action': 'artist'}) + assert res['success'] and res['action'] == 'applied_artist_art' + album_thumb, artist_thumb = _thumbs(w) + assert artist_thumb == 'http://new/artist.jpg' # artist updated + assert album_thumb is None # album untouched + + +def test_album_only_sets_album_leaves_artist(tmp_path): + w = _worker(tmp_path) + res = w._fix_missing_cover_art('album', 'al1', None, {**DETAILS, '_fix_action': 'album'}) + assert res['success'] + album_thumb, artist_thumb = _thumbs(w) + assert album_thumb == 'http://new/album.jpg' # album updated + assert artist_thumb == 'http://old/artist.jpg' # artist left as-is + + +def test_default_action_is_album_only(tmp_path): + # No _fix_action → behaves exactly like the old "Apply Art" (album only). + w = _worker(tmp_path) + w._fix_missing_cover_art('album', 'al1', None, dict(DETAILS)) + album_thumb, artist_thumb = _thumbs(w) + assert album_thumb == 'http://new/album.jpg' + assert artist_thumb == 'http://old/artist.jpg' + + +def test_both_sets_album_and_artist(tmp_path): + w = _worker(tmp_path) + res = w._fix_missing_cover_art('album', 'al1', None, {**DETAILS, '_fix_action': 'both'}) + assert res['success'] + album_thumb, artist_thumb = _thumbs(w) + assert album_thumb == 'http://new/album.jpg' + assert artist_thumb == 'http://new/artist.jpg' + assert 'artist image' in res['message'] + + +def test_artist_action_without_found_artist_url_fails_cleanly(tmp_path): + w = _worker(tmp_path) + res = w._fix_missing_cover_art('album', 'al1', None, + {**DETAILS, 'found_artist_url': None, '_fix_action': 'artist'}) + assert res['success'] is False + album_thumb, artist_thumb = _thumbs(w) + assert artist_thumb == 'http://old/artist.jpg' # nothing changed diff --git a/tests/test_missing_cover_art.py b/tests/test_missing_cover_art.py index 2994ad33..c32ba009 100644 --- a/tests/test_missing_cover_art.py +++ b/tests/test_missing_cover_art.py @@ -189,8 +189,13 @@ def test_missing_cover_art_uses_configured_art_sources(monkeypatch): result = mca.MissingCoverArtJob().scan(context) assert result.findings_created == 1 + # ALBUM art came from the configured order — the album source-priority loop + # was skipped (if it had run, the URL would be the fake client's art). assert findings[0]['details']['found_artwork_url'] == 'https://configured/art.jpg' - assert consulted == [] # source-priority loop skipped when configured art wins + # Artist-art search (Pache711) is a SEPARATE lookup that does consult the + # sources; the fake client has no search_artists, so it finds nothing and + # no artist target is offered. + assert findings[0]['details']['found_artist_url'] is None def test_missing_cover_art_uses_primary_when_prefer_unset(monkeypatch): diff --git a/webui/static/enrichment.js b/webui/static/enrichment.js index 9df8ea6c..dc97dc25 100644 --- a/webui/static/enrichment.js +++ b/webui/static/enrichment.js @@ -3092,21 +3092,34 @@ function _renderFindingDetail(f) { if (d.album_title) rows.push(['Album', d.album_title]); if (d.spotify_album_id) rows.push(['Spotify ID', d.spotify_album_id]); let artHtml = ''; - // Show artist image + found artwork side by side - if (d.artist_thumb_url || d.found_artwork_url) { + // Each found image is independently applyable (Pache711: "fix one, + // dismiss the other"). Per-image Apply buttons let the user take the + // correct album art and skip a wrong artist image, or vice versa. + if (d.artist_thumb_url || d.found_artwork_url || d.found_artist_url) { artHtml += '
'; + if (d.found_artwork_url) { + artHtml += `
+ Found album art + Found Album Art + +
`; + } + // Current artist image (context) — no apply, it's what's there now. if (d.artist_thumb_url) { artHtml += `
- Artist - ${_escFinding(d.artist || 'Artist')} + ${_escFinding(d.artist || 'Artist')} (current)
`; } - if (d.found_artwork_url) { + // Found artist image — the applyable replacement. + if (d.found_artist_url) { artHtml += `
- Found artwork - Found Artwork + Found Artist Art +
`; } artHtml += '
'; @@ -3382,6 +3395,31 @@ async function selectDuplicateToKeep(findingId, keepTrackId) { } } +async function applyCoverArtTarget(id, target) { + // Per-image apply for a cover-art finding (Pache711). target: 'album' | + // 'artist'. Applying either resolves the finding, so picking the correct + // image and ignoring the wrong one = "fix one, dismiss the other". + try { + const response = await fetch(`/api/repair/findings/${id}/fix`, { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ fix_action: target }), + }); + const result = await response.json(); + if (result.success) { + showToast(result.message || `Applied ${target} art`, 'success'); + } else { + showToast(result.error || `Failed to apply ${target} art`, 'error'); + } + loadRepairFindingsDashboard(); + loadRepairFindings(); + updateRepairStatus(); + } catch (error) { + console.error('Error applying cover art target:', error); + showToast('Error applying art', 'error'); + } +} + async function fixRepairFinding(id, findingType) { // Orphan files require user to choose an action let fixAction = null; diff --git a/webui/static/style.css b/webui/static/style.css index 00f8986e..b236beff 100644 --- a/webui/static/style.css +++ b/webui/static/style.css @@ -53477,6 +53477,22 @@ tr.tag-diff-same { line-height: 1.3; font-weight: 500; } +/* Per-image apply button on cover-art findings (Pache711: fix one or the other) */ +.repair-art-apply-btn { + padding: 5px 14px; + font-size: 11px; + font-weight: 600; + border-radius: 999px; + border: 1px solid rgba(74, 222, 128, 0.4); + background: rgba(74, 222, 128, 0.12); + color: #4ade80; + cursor: pointer; + transition: background 0.18s ease, transform 0.18s ease; +} +.repair-art-apply-btn:hover { + background: rgba(74, 222, 128, 0.22); + transform: translateY(-1px); +} /* Play button for findings */ .repair-finding-play-btn {