From df19317dacae76134b35d88185c63516eff96f0d Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Sat, 6 Jun 2026 13:38:13 -0700 Subject: [PATCH] Library Re-tag: cover-mode scans stop producing unappliable "(0 track(s))" findings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On path-mapped setups (Docker mounts etc.) the scan checked a bare os.path.isfile() on the raw DB path — false for EVERY track — while the apply handler resolves container/host mismatches. With a cover-art mode set, the cover_action kept the album past the "anything to do?" gate, so every album produced a finding with an empty tracks list whose apply could only ever fail with "No tracks to re-tag in finding". - the scan now resolves each track path with the same resolver the apply handler uses (resolve_library_file_path) before reachability checks and current-tag reads; plans carry the resolved path - a finding can never be created with zero tracks — cover-action albums with no usable tracks are skipped, with a debug log of why (unreachable/unmatched counts) and the counts surfaced in the finding description - unmatched-but-reachable tracks now get an art-only plan (empty db_data) so album cover art covers ALL the album's files, not just source-matched ones — apply_track_plans already treats empty db_data as a pure cover embed and counts a failed cover download as skipped, never failed (now locked by tests) - cover-only findings are titled "(cover art, N track(s))" instead of the puzzling "(0 track(s))" Tests: +5 (mapped paths resolve into plans, cover-with-nothing-reachable creates no finding, unmatched -> art-only plan, art-only plan embeds cover, failed cover download -> skipped). 87 passed across retag/repair/tag_writer. --- core/repair_jobs/library_retag.py | 55 +++++++++++++-- tests/test_library_retag_job.py | 109 ++++++++++++++++++++++++++++++ 2 files changed, 157 insertions(+), 7 deletions(-) diff --git a/core/repair_jobs/library_retag.py b/core/repair_jobs/library_retag.py index ad1325e4..1617943e 100644 --- a/core/repair_jobs/library_retag.py +++ b/core/repair_jobs/library_retag.py @@ -12,6 +12,7 @@ finding. The apply handler lives in repair_worker (_fix_library_retag). import os +from core.library.path_resolver import resolve_library_file_path from core.library.retag_planner import ( MODE_FILL_MISSING, MODE_OVERWRITE, @@ -366,15 +367,41 @@ class LibraryRetagJob(RepairJob): cover_action = self._cover_action(cover_mode, cover_url, library_tracks) pairs = match_source_tracks(source_tracks, library_tracks) + download_folder = (context.config_manager.get('soulseek.download_path', '') + if context.config_manager else None) track_plans = [] unmatched = [] + unreachable = 0 for lib, src in pairs: + # Resolve container/host path mismatches the same way the apply + # handler does. The old bare os.path.isfile() on the raw DB path + # failed for EVERY track on path-mapped setups (Docker mounts), so + # cover-mode scans produced "(0 track(s))" findings that the apply + # then rejected with "No tracks to re-tag in finding". + rp = resolve_library_file_path( + lib['file_path'], + transfer_folder=getattr(context, 'transfer_folder', None), + download_folder=download_folder, + config_manager=context.config_manager, + ) + if not rp: + unreachable += 1 + continue # genuinely unreachable from this process if src is None: unmatched.append(lib['title'] or os.path.basename(lib['file_path'])) + # No source match means no re-tag — but album cover art still + # applies to the file, so cover modes include an art-only plan + # (empty db_data: apply embeds art and writes no tags). + if cover_action: + track_plans.append({ + 'file_path': rp, + 'track_id': lib['id'], + 'title': lib['title'], + 'changes': [], + 'db_data': {}, + }) continue - if not os.path.isfile(lib['file_path']): - continue # not reachable at the stored path — skip (apply resolves paths) - current = _read_current_tags(lib['file_path']) + current = _read_current_tags(rp) plan = plan_track(current, src, album_meta, mode=mode) # Include a track when its tags change, OR when there's a cover action # to apply to it (db_data may be empty — apply embeds art either way). @@ -382,7 +409,7 @@ class LibraryRetagJob(RepairJob): db_data = plan['db_data'] _add_source_ids(db_data, source, album_source_id, src) tp = { - 'file_path': lib['file_path'], + 'file_path': rp, 'track_id': lib['id'], 'title': lib['title'], 'changes': plan['changes'], @@ -394,7 +421,14 @@ class LibraryRetagJob(RepairJob): track_plans.append(tp) tag_change_tracks = sum(1 for tp in track_plans if tp['changes']) - if not tag_change_tracks and not cover_action: + if (not tag_change_tracks and not cover_action) or not track_plans: + # Nothing actionable. The second clause covers cover-action albums + # where no track is reachable/included — creating a finding there + # gives an unappliable "(0 track(s))" entry. + if cover_action and not track_plans: + logger.debug( + "Library re-tag: album %s skipped — cover action but no usable tracks " + "(%d unreachable, %d unmatched)", album_id, unreachable, len(unmatched)) result.skipped += 1 return @@ -419,7 +453,14 @@ class LibraryRetagJob(RepairJob): desc = (f'Album "{album_title}" by {artist_name or "Unknown"} would be re-tagged from ' f'{source} ({", ".join(summary_bits)}).') if unmatched: - desc += f' {len(unmatched)} track(s) could not be matched to the source and are left untouched.' + desc += (f' {len(unmatched)} track(s) could not be matched to the source — ' + f'tags left untouched{" (cover art still applied)" if cover_action else ""}.') + if unreachable: + desc += f' {unreachable} track(s) not reachable on disk and skipped.' + + # Cover-only findings say so instead of the puzzling "(0 track(s))". + title_what = (f'{tag_change_tracks} track(s)' if tag_change_tracks + else f'cover art, {len(track_plans)} track(s)') if context.create_finding: inserted = context.create_finding( @@ -429,7 +470,7 @@ class LibraryRetagJob(RepairJob): entity_type='album', entity_id=str(album_id), file_path=None, - title=f'Re-tag: {album_title or "Unknown"} ({tag_change_tracks} track(s))', + title=f'Re-tag: {album_title or "Unknown"} ({title_what})', description=desc, details={ 'album_id': album_id, diff --git a/tests/test_library_retag_job.py b/tests/test_library_retag_job.py index d0405827..9c45ef74 100644 --- a/tests/test_library_retag_job.py +++ b/tests/test_library_retag_job.py @@ -284,3 +284,112 @@ def test_fix_library_retag_counts_unreachable(tmp_path, monkeypatch): 'cover_action': None, 'cover_url': None} res = worker._fix_library_retag('album', '1', None, details) assert res['success'] is False # nothing written (file missing) + + +# --------------------------------------------------------------------------- +# Cover-art scans on path-mapped setups (the "(0 track(s))" / "No tracks to +# re-tag in finding" report): the scan must resolve DB paths the same way the +# apply handler does, never emit an empty finding, and give unmatched tracks +# the album art. +# --------------------------------------------------------------------------- + +def test_scan_resolves_mapped_paths_instead_of_skipping(tmp_path, monkeypatch): + """DB stores a container path the scan process can't see directly; the + resolver maps it to the real file. Before the fix the bare isfile() check + dropped every track and cover-mode scans produced unappliable 0-track + findings.""" + real = tmp_path / 'track.flac'; real.write_bytes(b'') + raw = '/container/music/track.flac' # not a real path here + conn = _db_with_album(str(tmp_path / 'm.db'), raw, current_title='Old Title') + ctx = _context(conn, {'mode': 'overwrite', 'cover_art': 'replace', 'source': 'spotify'}) + _patch_source(monkeypatch, { + 'title': 'Old Title', 'album_artist': 'Real Artist', 'album': 'Real Album', + 'year': '2021', 'genre': 'Rock', 'track_number': 1, 'disc_number': 1, + }) + monkeypatch.setattr(lr, 'resolve_library_file_path', + lambda p, **k: str(real) if p == raw else None) + + result = lr.LibraryRetagJob().scan(ctx) + + assert result.findings_created == 1 + tracks = ctx.findings[0]['details']['tracks'] + assert len(tracks) == 1 + assert tracks[0]['file_path'] == str(real) # plan carries the RESOLVED path + + +def test_cover_scan_with_no_reachable_tracks_creates_no_finding(tmp_path, monkeypatch): + """Cover action set but no track resolvable: skip the album entirely. + The old behavior created a '(0 track(s))' finding whose apply always + failed with 'No tracks to re-tag in finding'.""" + conn = _db_with_album(str(tmp_path / 'm.db'), '/container/music/gone.flac') + ctx = _context(conn, {'mode': 'overwrite', 'cover_art': 'replace', 'source': 'spotify'}) + _patch_source(monkeypatch, {'title': 'Old Title'}) + monkeypatch.setattr(lr, 'resolve_library_file_path', lambda p, **k: None) + + result = lr.LibraryRetagJob().scan(ctx) + + assert ctx.findings == [] + assert result.findings_created == 0 + assert result.skipped == 1 + + +def test_cover_scan_includes_unmatched_tracks_as_art_only(tmp_path, monkeypatch): + """A track with no source match can't be re-tagged, but album cover art + still applies to it — cover-mode scans include an art-only plan (empty + db_data) and the finding title says 'cover art', not '(0 track(s))'.""" + track = tmp_path / 'track.flac'; track.write_bytes(b'') + conn = _db_with_album(str(tmp_path / 'm.db'), str(track), current_title='Old Title') + ctx = _context(conn, {'mode': 'overwrite', 'cover_art': 'replace', 'source': 'spotify'}) + _patch_source(monkeypatch, {'title': 'Old Title'}) + # Source tracklist that matches NOTHING in the library. + monkeypatch.setattr(lr, 'get_album_tracks_for_source', + lambda s, i: [{'name': 'Zzz Unrelated Song', 'track_number': 9, + 'disc_number': 9, 'id': 'zz'}]) + + result = lr.LibraryRetagJob().scan(ctx) + + assert result.findings_created == 1 + f = ctx.findings[0] + assert 'cover art' in f['title'] + tracks = f['details']['tracks'] + assert len(tracks) == 1 + assert not tracks[0]['changes'] and tracks[0]['db_data'] == {} # art-only plan + + +def test_apply_art_only_plan_embeds_cover(tmp_path, monkeypatch): + """The art-only plans the cover-mode scan now emits (empty db_data) must go + through apply_track_plans as a WRITE (cover embed), not a skip/failure.""" + track = tmp_path / 'track.flac'; track.write_bytes(b'') + calls = [] + monkeypatch.setattr('core.tag_writer.download_cover_art', + lambda url: (b'img-bytes', 'image/jpeg')) + monkeypatch.setattr('core.tag_writer.write_tags_to_file', + lambda fp, db_data, **k: calls.append((fp, db_data, k)) or {'success': True}) + + res = lr.apply_track_plans( + [{'file_path': str(track), 'db_data': {}}], + cover_action='replace', cover_url='http://art/cover.jpg', + ) + + assert res['written'] == 1 and res['failed'] == 0 + fp, db_data, kwargs = calls[0] + assert db_data == {} and kwargs['embed_cover'] is True + assert kwargs['cover_data'] == (b'img-bytes', 'image/jpeg') + assert res['cover_written'] is True # cover.jpg written next to the track + + +def test_apply_art_only_plan_skips_when_cover_download_fails(tmp_path, monkeypatch): + """If the cover can't be downloaded there's nothing to write for an + art-only plan — it must count as skipped, never failed.""" + track = tmp_path / 'track.flac'; track.write_bytes(b'') + monkeypatch.setattr('core.tag_writer.download_cover_art', + lambda url: (_ for _ in ()).throw(RuntimeError('net down'))) + monkeypatch.setattr('core.tag_writer.write_tags_to_file', + lambda fp, db_data, **k: {'success': True}) + + res = lr.apply_track_plans( + [{'file_path': str(track), 'db_data': {}}], + cover_action='replace', cover_url='http://art/cover.jpg', + ) + + assert res == {'written': 0, 'failed': 0, 'skipped': 1, 'cover_written': False}