From b0c78c8674628bc4b066de6a78b0d4ee0dcdd317 Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Thu, 4 Jun 2026 08:50:40 -0700 Subject: [PATCH 01/67] =?UTF-8?q?Library=20re-tag=20(1/3):=20pure=20planne?= =?UTF-8?q?r=20=E2=80=94=20match=20source=20tracklist=20+=20per-field=20ta?= =?UTF-8?q?g=20diff?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The testable core for the new library-wide re-tag job. Given a source album's metadata + tracklist and the library tracks' current file tags, it: - matches source tracks to library tracks (disc+track number, then title sim), - computes the per-field diff (old -> new) for the dry-run finding, - builds the minimal write_tags_to_file payload — only fields that actually change under the chosen mode (overwrite vs fill-missing), so applying never touches unrelated/unchanged tags. No IO/network/DB — 10 unit tests cover matching, both modes, blank-source fields, and the album-artist/track-count payload mapping. --- core/library/retag_planner.py | 218 ++++++++++++++++++++++++++++++++++ tests/test_retag_planner.py | 114 ++++++++++++++++++ 2 files changed, 332 insertions(+) create mode 100644 core/library/retag_planner.py create mode 100644 tests/test_retag_planner.py diff --git a/core/library/retag_planner.py b/core/library/retag_planner.py new file mode 100644 index 00000000..e8457aa3 --- /dev/null +++ b/core/library/retag_planner.py @@ -0,0 +1,218 @@ +"""Pure planning logic for the library re-tag job. + +Given a source album's metadata + tracklist and the library's tracks (with their +*current* file tags), this works out — per track — exactly which tags would +change (the dry-run diff the finding shows) and the ``db_data`` payload to feed +``core.tag_writer.write_tags_to_file`` at apply time. + +No file IO, no network, no DB: the job feeds in current tags + fetched source +data, so all the matching/diff logic stays unit-testable. Tags are only ever +ADDED/overwritten per-field — never a full tag-block wipe. +""" + +from __future__ import annotations + +import re +from difflib import SequenceMatcher +from typing import Any, Dict, List, Optional, Tuple + +# Fields this job manages. Keys are the internal/display names; the diff and the +# write payload are both built from these. +MANAGED_FIELDS = ('title', 'artist', 'album', 'year', 'genre', 'track_number', 'disc_number') + +# Modes: overwrite everything the source provides, or only fill blanks. +MODE_OVERWRITE = 'overwrite' +MODE_FILL_MISSING = 'fill_missing' + + +def _get(obj: Any, *keys: str, default=None): + """First non-empty value across keys, from a dict or an object.""" + for k in keys: + v = obj.get(k) if isinstance(obj, dict) else getattr(obj, k, None) + if v not in (None, ''): + return v + return default + + +def _first_artist(obj: Any) -> str: + arts = _get(obj, 'artists', 'artist', 'artist_name') + if isinstance(arts, list) and arts: + a0 = arts[0] + return ((a0.get('name') if isinstance(a0, dict) else str(a0)) or '').strip() + if isinstance(arts, dict): + return (arts.get('name') or '').strip() + return str(arts).strip() if arts else '' + + +def _genres_list(obj: Any) -> List[str]: + g = _get(obj, 'genres', 'genre') + if isinstance(g, list): + return [str(x).strip() for x in g if str(x).strip()] + if isinstance(g, str) and g.strip(): + return [p.strip() for p in g.split(',') if p.strip()] + return [] + + +def _year(obj: Any) -> str: + v = _get(obj, 'year', 'release_date', 'date') + if not v: + return '' + m = re.search(r'\d{4}', str(v)) + return m.group(0) if m else '' + + +def _int_or_none(v) -> Optional[int]: + try: + return int(v) + except (TypeError, ValueError): + return None + + +def _norm_title(s: Any) -> str: + s = (s or '') + s = s.lower() if isinstance(s, str) else str(s).lower() + s = re.sub(r'[\(\[].*?[\)\]]', ' ', s) + s = re.sub(r'[^a-z0-9]+', ' ', s) + return ' '.join(s.split()) + + +def match_source_tracks( + source_tracks: List[Any], + library_tracks: List[Dict[str, Any]], + title_threshold: float = 0.6, +) -> List[Tuple[Dict[str, Any], Optional[Any]]]: + """Pair each library track to a source track. + + Disc+track number is authoritative; falls back to title similarity. A source + track is consumed once. Returns ``[(library_track, source_track_or_None)]`` + in library order, so unmatched library tracks surface as ``None``. + """ + by_pos: Dict[Tuple[int, int], int] = {} + for i, st in enumerate(source_tracks): + t = _int_or_none(_get(st, 'track_number')) + if t is None: + continue + d = _int_or_none(_get(st, 'disc_number', default=1)) or 1 + by_pos.setdefault((d, t), i) + + used: set = set() + pairs: List[Tuple[Dict[str, Any], Optional[Any]]] = [] + for lt in library_tracks: + t = _int_or_none(lt.get('track_number')) + d = _int_or_none(lt.get('disc_number')) or 1 + idx = by_pos.get((d, t)) if t is not None else None + if idx is not None and idx not in used: + used.add(idx) + pairs.append((lt, source_tracks[idx])) + continue + # Title-similarity fallback over still-unused source tracks. + lt_norm = _norm_title(lt.get('title')) + best_idx, best_score = None, 0.0 + if lt_norm: + for i, st in enumerate(source_tracks): + if i in used: + continue + score = SequenceMatcher(None, lt_norm, _norm_title(_get(st, 'name', 'title', 'track_name'))).ratio() + if score > best_score: + best_score, best_idx = score, i + if best_idx is not None and best_score >= title_threshold: + used.add(best_idx) + pairs.append((lt, source_tracks[best_idx])) + else: + pairs.append((lt, None)) + return pairs + + +def _target_for_track(source_track: Any, album_meta: Dict[str, Any]) -> Dict[str, Any]: + """Normalized target tag values from the source for one track.""" + album_artist = _first_artist(album_meta) + track_artist = _first_artist(source_track) or album_artist + return { + 'title': (_get(source_track, 'name', 'title', 'track_name') or '').strip(), + 'artist': album_artist, # album-level artist + 'track_artist': track_artist, # per-track (may equal album artist) + 'album': (_get(album_meta, 'name', 'title', 'album_name') or '').strip(), + 'year': _year(album_meta), + 'genre': _genres_list(album_meta), # list + 'track_number': _int_or_none(_get(source_track, 'track_number')), + 'disc_number': _int_or_none(_get(source_track, 'disc_number', default=1)) or 1, + 'track_count': _int_or_none(_get(album_meta, 'total_tracks', 'track_count')), + } + + +def _current_value(current_tags: Dict[str, Any], field: str): + if field == 'artist': + # _read_tags stores album_artist + artist; prefer album_artist for the album-level compare. + return current_tags.get('album_artist') or current_tags.get('artist') or '' + if field == 'genre': + return current_tags.get('genre') or '' + return current_tags.get(field) + + +def _display(value) -> str: + if isinstance(value, list): + return ', '.join(str(v) for v in value) + return '' if value is None else str(value) + + +def _is_empty(value) -> bool: + if value is None: + return True + if isinstance(value, str): + return value.strip() == '' + if isinstance(value, list): + return len(value) == 0 + return False + + +def plan_track(current_tags: Dict[str, Any], source_track: Any, album_meta: Dict[str, Any], + mode: str = MODE_OVERWRITE) -> Dict[str, Any]: + """Diff one library track's current tags against the source target. + + Returns ``{changes, db_data}`` where ``changes`` is ``{field: {old, new}}`` + for display, and ``db_data`` is the (minimal) payload for + ``write_tags_to_file`` — it contains ONLY the fields that should be written + under ``mode``, so applying never touches unrelated/unchanged tags. + """ + target = _target_for_track(source_track, album_meta) + changes: Dict[str, Dict[str, str]] = {} + db_data: Dict[str, Any] = {} + + for field in MANAGED_FIELDS: + new_val = target.get(field) + if _is_empty(new_val): + continue # source gave us nothing for this field — leave the file alone + old_val = _current_value(current_tags, field) + + if mode == MODE_FILL_MISSING and not _is_empty(old_val): + continue # fill-missing only writes blanks + + old_disp, new_disp = _display(old_val), _display(new_val) + if old_disp == new_disp: + continue # already correct — nothing to write + + changes[field] = {'old': old_disp, 'new': new_disp} + # Map managed field → write_tags_to_file db_data key. + if field == 'title': + db_data['title'] = new_val + elif field == 'artist': + db_data['artist_name'] = new_val # = album artist for the writer + ta = target.get('track_artist') + if ta and ta != new_val: + db_data['track_artist'] = ta + elif field == 'album': + db_data['album_title'] = new_val + elif field == 'year': + db_data['year'] = new_val + elif field == 'genre': + db_data['genres'] = new_val # list + elif field == 'track_number': + db_data['track_number'] = new_val + elif field == 'disc_number': + db_data['disc_number'] = new_val + + # Always carry track_count alongside a track_number write (writers want both). + if 'track_number' in db_data and target.get('track_count'): + db_data['track_count'] = target['track_count'] + + return {'changes': changes, 'db_data': db_data} diff --git a/tests/test_retag_planner.py b/tests/test_retag_planner.py new file mode 100644 index 00000000..2137c425 --- /dev/null +++ b/tests/test_retag_planner.py @@ -0,0 +1,114 @@ +"""Unit tests for the library re-tag planner (pure match + diff + payload).""" + +from __future__ import annotations + +from core.library import retag_planner as rp + + +# ── track matching ── + +def test_match_by_disc_and_track_number(): + src = [ + {'name': 'A', 'track_number': 1, 'disc_number': 1}, + {'name': 'B', 'track_number': 2, 'disc_number': 1}, + ] + lib = [ + {'title': 'wrong title', 'track_number': 2, 'disc_number': 1}, + {'title': 'whatever', 'track_number': 1, 'disc_number': 1}, + ] + pairs = rp.match_source_tracks(src, lib) + assert pairs[0][1]['name'] == 'B' # lib track #2 → source B + assert pairs[1][1]['name'] == 'A' + + +def test_match_by_title_when_no_track_number(): + src = [{'name': 'Bohemian Rhapsody', 'track_number': 1, 'disc_number': 1}] + lib = [{'title': 'Bohemian Rhapsody (Remastered)', 'track_number': None, 'disc_number': 1}] + pairs = rp.match_source_tracks(src, lib) + assert pairs[0][1]['name'] == 'Bohemian Rhapsody' + + +def test_unmatched_library_track_is_none(): + src = [{'name': 'A', 'track_number': 1, 'disc_number': 1}] + lib = [{'title': 'Completely Different', 'track_number': 9, 'disc_number': 1}] + pairs = rp.match_source_tracks(src, lib) + assert pairs[0][1] is None + + +def test_source_track_consumed_once(): + src = [{'name': 'A', 'track_number': 1, 'disc_number': 1}] + lib = [ + {'title': 'A', 'track_number': 1, 'disc_number': 1}, + {'title': 'A again', 'track_number': 1, 'disc_number': 1}, + ] + pairs = rp.match_source_tracks(src, lib) + assert pairs[0][1] is not None + assert pairs[1][1] is None # the one source track was already used + + +# ── per-track diff (overwrite) ── + +ALBUM = {'name': 'Real Album', 'artists': [{'name': 'Real Artist'}], + 'year': '2021-05-01', 'genres': ['Rock', 'Indie'], 'total_tracks': 10} +SRC = {'name': 'Real Title', 'track_number': 3, 'disc_number': 1, + 'artists': [{'name': 'Real Artist'}]} + + +def test_overwrite_reports_changed_fields_only(): + current = {'title': 'Old Title', 'album_artist': 'Real Artist', + 'album': 'Real Album', 'year': '2021', 'genre': 'Rock, Indie', + 'track_number': 3, 'disc_number': 1} + plan = rp.plan_track(current, SRC, ALBUM, mode=rp.MODE_OVERWRITE) + # Only the title differs; everything else already matches → single change. + assert set(plan['changes']) == {'title'} + assert plan['changes']['title'] == {'old': 'Old Title', 'new': 'Real Title'} + assert plan['db_data'].get('title') == 'Real Title' + # Unchanged fields must NOT be in the write payload. + assert 'album_title' not in plan['db_data'] + + +def test_overwrite_writes_album_artist_via_artist_name_key(): + current = {'title': 'Real Title', 'album_artist': 'WRONG Artist', + 'album': 'Real Album', 'year': '2021', 'genre': 'Rock, Indie', + 'track_number': 3, 'disc_number': 1} + plan = rp.plan_track(current, SRC, ALBUM, mode=rp.MODE_OVERWRITE) + assert plan['changes']['artist'] == {'old': 'WRONG Artist', 'new': 'Real Artist'} + assert plan['db_data']['artist_name'] == 'Real Artist' # writer uses artist_name = album artist + + +def test_track_number_write_carries_track_count(): + current = {'title': 'Real Title', 'album_artist': 'Real Artist', 'album': 'Real Album', + 'year': '2021', 'genre': 'Rock, Indie', 'track_number': 99, 'disc_number': 1} + plan = rp.plan_track(current, SRC, ALBUM, mode=rp.MODE_OVERWRITE) + assert plan['db_data']['track_number'] == 3 + assert plan['db_data']['track_count'] == 10 # carried alongside + + +def test_no_changes_when_everything_matches(): + current = {'title': 'Real Title', 'album_artist': 'Real Artist', 'album': 'Real Album', + 'year': '2021', 'genre': 'Rock, Indie', 'track_number': 3, 'disc_number': 1} + plan = rp.plan_track(current, SRC, ALBUM, mode=rp.MODE_OVERWRITE) + assert plan['changes'] == {} + assert plan['db_data'] == {} + + +def test_source_blank_field_never_written(): + album = {'name': 'Real Album', 'artists': [{'name': 'Real Artist'}]} # no year/genres + current = {'title': 'Real Title', 'album_artist': 'Real Artist', 'album': 'Real Album', + 'year': '', 'genre': '', 'track_number': 3, 'disc_number': 1} + plan = rp.plan_track(current, SRC, album, mode=rp.MODE_OVERWRITE) + assert 'year' not in plan['changes'] and 'year' not in plan['db_data'] + assert 'genres' not in plan['db_data'] + + +# ── fill-missing mode ── + +def test_fill_missing_only_writes_blanks(): + current = {'title': 'Keep My Title', 'album_artist': '', 'album': 'Real Album', + 'year': '', 'genre': 'Rock, Indie', 'track_number': 3, 'disc_number': 1} + plan = rp.plan_track(current, SRC, ALBUM, mode=rp.MODE_FILL_MISSING) + # title is present (kept), artist + year are blank (filled). genre present (kept). + assert set(plan['changes']) == {'artist', 'year'} + assert 'title' not in plan['db_data'] # not overwritten in fill-missing + assert plan['db_data']['artist_name'] == 'Real Artist' + assert plan['db_data']['year'] == '2021' From f5c905be867e223c6d7950a1b743a2d5ceecba7c Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Thu, 4 Jun 2026 08:55:38 -0700 Subject: [PATCH 02/67] Library re-tag (2/3 + 3/3): the repair job + in-place apply handler MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit New 'Library Re-tag' repair job (default-OFF, opt-in; weekly when enabled): - Scans every source-matched album (spotify/itunes/deezer/musicbrainz album id), pulls fresh metadata + tracklist from that source, reads each local track's current tags, and uses the planner to compute per-field diffs. - Dry-run by design: scan only CREATES findings — nothing touches a file. Each finding is highly detailed: per-track old->new for every changed field, the source used, the mode, a cover-art action, and any unmatched tracks, plus a summary description. Settings: mode (overwrite | fill_missing), cover_art (replace | fill_missing | skip), source override. - Apply handler (_fix_library_retag in repair_worker): writes each track's planned tags in place via tag_writer.write_tags_to_file (+ batch-embeds cover, refreshes cover.jpg). Only adds/overwrites planned fields — no moves/renames/ re-match. Resolves Docker paths; read-only/unreachable files counted, never crash. Media-server-only / unreachable tracks are skipped. Registered in the job list + fix dispatch. The old per-download Retag Tool is left untouched alongside this for now. --- core/repair_jobs/__init__.py | 1 + core/repair_jobs/library_retag.py | 301 ++++++++++++++++++++++++++++++ core/repair_worker.py | 72 +++++++ 3 files changed, 374 insertions(+) create mode 100644 core/repair_jobs/library_retag.py diff --git a/core/repair_jobs/__init__.py b/core/repair_jobs/__init__.py index 40fd7a04..57996608 100644 --- a/core/repair_jobs/__init__.py +++ b/core/repair_jobs/__init__.py @@ -45,6 +45,7 @@ _JOB_MODULES = [ 'core.repair_jobs.unknown_artist_fixer', 'core.repair_jobs.discography_backfill', 'core.repair_jobs.canonical_version_resolve', + 'core.repair_jobs.library_retag', ] diff --git a/core/repair_jobs/library_retag.py b/core/repair_jobs/library_retag.py new file mode 100644 index 00000000..4f013faf --- /dev/null +++ b/core/repair_jobs/library_retag.py @@ -0,0 +1,301 @@ +"""Library Re-tag Job — rewrite audio tags from a fresh metadata-source pull. + +Re-does ONLY the tagging part of post-processing (tags + cover art), IN PLACE: +no file moves, no renames, no re-matching, no library reorganization. Only +albums that are matched to a metadata source are eligible — the album's stored +source id is used to pull fresh data to write. + +Dry-run by design: scan creates detailed, per-track findings (old -> new for +every field that would change); nothing touches a file until you apply a +finding. The apply handler lives in repair_worker (_fix_library_retag). +""" + +import os + +from core.library.retag_planner import ( + MODE_FILL_MISSING, + MODE_OVERWRITE, + match_source_tracks, + plan_track, +) +from core.metadata.album_tracks import get_album_for_source, get_album_tracks_for_source +from core.metadata_service import get_primary_source, get_source_priority +from core.repair_jobs import register_job +from core.repair_jobs.base import JobContext, JobResult, RepairJob +from utils.logging_config import get_logger + +logger = get_logger("repair_job.library_retag") + +# (source, albums-table column) in resolution-preference order is decided at +# runtime from the configured source priority; this maps source -> column. +_ALBUM_SOURCE_COLUMNS = { + 'spotify': 'spotify_album_id', + 'itunes': 'itunes_album_id', + 'deezer': 'deezer_id', + 'musicbrainz': 'musicbrainz_release_id', +} + + +def _read_current_tags(file_path): + try: + from core.soulsync_client import _read_tags + return _read_tags(file_path) or {} + except Exception as exc: + logger.debug("read tags failed for %s: %s", file_path, exc) + return {} + + +def _track_list(result): + """Normalize a get_album_tracks result into a plain list of track items.""" + if result is None: + return [] + if isinstance(result, list): + return result + if isinstance(result, dict): + for key in ('tracks', 'items', 'data'): + v = result.get(key) + if isinstance(v, list): + return v + v = getattr(result, 'tracks', None) + return v if isinstance(v, list) else [] + + +@register_job +class LibraryRetagJob(RepairJob): + job_id = 'library_retag' + display_name = 'Library Re-tag' + description = 'Rewrites tags + cover art from a fresh metadata-source pull, in place' + help_text = ( + 'Re-tags albums in your library using a fresh pull from the metadata source ' + 'they are matched to — writing the tags (and optionally cover art) directly ' + 'into the files. It only does the tagging step: files are NOT moved, renamed, ' + 're-matched, or reorganized.\n\n' + 'Only albums matched to a metadata source (Spotify / iTunes / Deezer / ' + 'MusicBrainz album id) are eligible, since the source is where the fresh data ' + 'comes from. Each finding lists every tag that would change (old -> new) per ' + 'track so you can review before applying — nothing is written until you do.\n\n' + 'Settings:\n' + '- Mode: "overwrite" rewrites every field the source provides; "fill_missing" ' + 'only fills blank tags (keeps your existing values).\n' + '- Cover art: replace / fill-missing / skip.\n' + '- Source: which matched source to pull from (default: your source priority).' + ) + icon = 'repair-icon-retag' + default_enabled = False + default_interval_hours = 168 + default_settings = { + 'mode': MODE_OVERWRITE, + 'cover_art': 'replace', + 'source': '', + } + setting_options = { + 'mode': [MODE_OVERWRITE, MODE_FILL_MISSING], + 'cover_art': ['replace', 'fill_missing', 'skip'], + 'source': ['', 'spotify', 'itunes', 'deezer', 'musicbrainz'], + } + auto_fix = False + + def _get_settings(self, context: JobContext) -> dict: + merged = dict(self.default_settings) + if context.config_manager: + cfg = context.config_manager.get(f'repair.jobs.{self.job_id}.settings', {}) or {} + merged.update(cfg) + return merged + + def _source_order(self, settings) -> list: + override = (settings.get('source') or '').strip() + if override in _ALBUM_SOURCE_COLUMNS: + return [override] + return [s for s in get_source_priority(get_primary_source()) if s in _ALBUM_SOURCE_COLUMNS] + + def scan(self, context: JobContext) -> JobResult: + result = JobResult() + settings = self._get_settings(context) + mode = settings.get('mode', MODE_OVERWRITE) + cover_mode = settings.get('cover_art', 'replace') + source_order = self._source_order(settings) + if not source_order: + logger.warning("Library re-tag: no usable metadata sources configured") + return result + + # Albums that carry at least one usable source id. + cols = ', '.join(f'al.{c}' for c in _ALBUM_SOURCE_COLUMNS.values()) + try: + with context.db._get_connection() as conn: + cursor = conn.cursor() + cursor.execute(f""" + SELECT al.id, al.title, ar.name, {cols} + FROM albums al + LEFT JOIN artists ar ON ar.id = al.artist_id + WHERE al.title IS NOT NULL AND al.title != '' + """) + albums = cursor.fetchall() + except Exception as e: + logger.error("Library re-tag: album query failed: %s", e, exc_info=True) + result.errors += 1 + return result + + total = len(albums) + if context.update_progress: + context.update_progress(0, total) + if context.report_progress: + context.report_progress(phase=f'Checking {total} albums for tag drift...', total=total) + + for i, row in enumerate(albums): + if context.check_stop(): + return result + if i % 5 == 0 and context.wait_if_paused(): + return result + result.scanned += 1 + if context.update_progress and (i + 1) % 5 == 0: + context.update_progress(i + 1, total) + + album_id, album_title, artist_name = row[0], row[1], row[2] + source_ids = {src: row[3 + idx] for idx, src in enumerate(_ALBUM_SOURCE_COLUMNS)} + + source = next((s for s in source_order if source_ids.get(s)), None) + if not source: + continue # not matched to a usable source — skip + album_source_id = str(source_ids[source]) + + try: + self._scan_album(context, result, album_id, album_title, artist_name, + source, album_source_id, mode, cover_mode) + except Exception as e: + logger.debug("Library re-tag: album %s failed: %s", album_id, e) + result.errors += 1 + + if context.update_progress: + context.update_progress(total, total) + logger.info("Library re-tag scan: %d albums checked, %d findings", + result.scanned, result.findings_created) + return result + + def _scan_album(self, context, result, album_id, album_title, artist_name, + source, album_source_id, mode, cover_mode): + # Local tracks for this album. + with context.db._get_connection() as conn: + cur = conn.cursor() + cur.execute(""" + SELECT id, title, track_number, disc_number, file_path + FROM tracks + WHERE album_id = ? AND file_path IS NOT NULL AND file_path != '' + ORDER BY disc_number, track_number + """, (album_id,)) + library_tracks = [ + {'id': r[0], 'title': r[1], 'track_number': r[2], + 'disc_number': r[3], 'file_path': r[4]} + for r in cur.fetchall() + ] + if not library_tracks: + return + + album_meta = get_album_for_source(source, album_source_id) + source_tracks = _track_list(get_album_tracks_for_source(source, album_source_id)) + if not album_meta or not source_tracks: + logger.debug("Library re-tag: no source data for album %s (%s)", album_id, source) + return + + cover_url = None + for k in ('image_url', 'album_image_url', 'cover_url', 'thumb_url'): + v = album_meta.get(k) if isinstance(album_meta, dict) else getattr(album_meta, k, None) + if v: + cover_url = v + break + + # Cover action (album-level), independent of tag changes. Decided first + # so cover-only albums (tags fine, art missing) still include their + # tracks for the apply to embed art into. + cover_action = self._cover_action(cover_mode, cover_url, library_tracks) + + pairs = match_source_tracks(source_tracks, library_tracks) + track_plans = [] + unmatched = [] + for lib, src in pairs: + if src is None: + unmatched.append(lib['title'] or os.path.basename(lib['file_path'])) + 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']) + 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). + if plan['changes'] or cover_action: + track_plans.append({ + 'file_path': lib['file_path'], + 'track_id': lib['id'], + 'title': lib['title'], + 'changes': plan['changes'], + 'db_data': plan['db_data'], + }) + + tag_change_tracks = sum(1 for tp in track_plans if tp['changes']) + if not tag_change_tracks and not cover_action: + result.skipped += 1 + return + + total_changes = sum(len(tp['changes']) for tp in track_plans) + summary_bits = [] + if tag_change_tracks: + summary_bits.append(f"{tag_change_tracks} track(s), {total_changes} tag change(s)") + if cover_action: + summary_bits.append(f"cover art ({cover_action})") + 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.' + + if context.create_finding: + inserted = context.create_finding( + job_id=self.job_id, + finding_type='library_retag', + severity='info', + entity_type='album', + entity_id=str(album_id), + file_path=None, + title=f'Re-tag: {album_title or "Unknown"} ({tag_change_tracks} track(s))', + description=desc, + details={ + 'album_id': album_id, + 'album_title': album_title, + 'artist': artist_name, + 'source': source, + 'album_source_id': album_source_id, + 'mode': mode, + 'cover_mode': cover_mode, + 'cover_url': cover_url, + 'cover_action': cover_action, + 'tracks': track_plans, # each carries its db_data for a deterministic apply + 'unmatched': unmatched, + }, + ) + if inserted: + result.findings_created += 1 + else: + result.findings_skipped_dedup += 1 + + @staticmethod + def _cover_action(cover_mode, cover_url, library_tracks): + """Return 'replace' / 'fill' / None for the album's cover under the mode.""" + if cover_mode == 'skip' or not cover_url: + return None + if cover_mode == 'replace': + return 'replace' + # fill_missing — only if the album has no art on disk + try: + from core.metadata.art_apply import album_has_art_on_disk + rep = library_tracks[0]['file_path'] if library_tracks else '' + return None if album_has_art_on_disk(rep) else 'fill' + except Exception: + return None + + def estimate_scope(self, context: JobContext) -> int: + try: + with context.db._get_connection() as conn: + cur = conn.cursor() + cur.execute("SELECT COUNT(*) FROM albums WHERE title IS NOT NULL AND title != ''") + row = cur.fetchone() + return row[0] if row else 0 + except Exception: + return 0 diff --git a/core/repair_worker.py b/core/repair_worker.py index 4ab4af0c..ab5650f6 100644 --- a/core/repair_worker.py +++ b/core/repair_worker.py @@ -976,6 +976,7 @@ class RepairWorker: 'unknown_artist': self._fix_unknown_artist, 'acoustid_mismatch': self._fix_acoustid_mismatch, 'missing_discography_track': self._fix_discography_backfill, + 'library_retag': self._fix_library_retag, } handler = handlers.get(finding_type) if not handler: @@ -1356,6 +1357,77 @@ class RepairWorker: msg = 'Updated database thumbnail, but could not write art to files (read-only?)' 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): + """Apply a library re-tag finding: write each track's planned tags in + place (core.tag_writer.write_tags_to_file) + optionally embed/refresh + cover art. Only ADDS/overwrites the planned fields — no moves/renames.""" + tracks = details.get('tracks') or [] + if not tracks: + return {'success': False, 'error': 'No tracks to re-tag in finding'} + + cover_action = details.get('cover_action') + cover_url = details.get('cover_url') + embed_cover = bool(cover_action and cover_url) + download_folder = self._config_manager.get('soulseek.download_path', '') if self._config_manager else None + + # Download the cover once for the whole album (batch embed). + cover_data = None + if embed_cover: + try: + from core.tag_writer import download_cover_art + cover_data = download_cover_art(cover_url) + except Exception as e: + logger.debug("library_retag: cover download failed: %s", e) + embed_cover = embed_cover and cover_data is not None + + from core.tag_writer import write_tags_to_file + written = failed = skipped = 0 + last_dir = None + for t in tracks: + raw = t.get('file_path') + db_data = t.get('db_data') or {} + if not raw or (not db_data and not embed_cover): + skipped += 1 + continue + resolved = _resolve_file_path(raw, self.transfer_folder, download_folder, + config_manager=self._config_manager) or raw + if not os.path.isfile(resolved): + skipped += 1 + continue + try: + res = write_tags_to_file(resolved, db_data, embed_cover=embed_cover, cover_data=cover_data) + if res.get('success'): + written += 1 + last_dir = os.path.dirname(resolved) + else: + failed += 1 + except Exception as e: + logger.warning("library_retag write failed for %s: %s", resolved, e) + failed += 1 + + # Refresh the cover.jpg sidecar to match (replace, or fill when missing). + cover_written = False + if cover_action and cover_data and last_dir: + try: + cover_path = os.path.join(last_dir, 'cover.jpg') + if cover_action == 'replace' or not os.path.exists(cover_path): + with open(cover_path, 'wb') as fh: + fh.write(cover_data[0]) + cover_written = True + except Exception as e: + logger.debug("library_retag: cover.jpg write failed: %s", e) + + if written == 0 and not cover_written: + return {'success': False, + 'error': 'Nothing could be written — files unreachable or read-only?'} + msg = f'Re-tagged {written} track(s)' + if failed: + msg += f' ({failed} failed)' + if cover_written: + msg += ' + refreshed cover.jpg' + return {'success': True, 'action': 'library_retag', 'message': msg, + 'written': written, 'failed': failed, 'skipped': skipped} + def _fix_metadata_gap(self, entity_type, entity_id, file_path, details): """Apply found metadata fields to the track.""" found_fields = details.get('found_fields') From 50b6876d6b0ddc11e23483852b2a8a555505a6ec Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Thu, 4 Jun 2026 09:01:29 -0700 Subject: [PATCH 03/67] Library re-tag: render the per-track old->new diff in the finding card MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Wire library_retag into the repair findings UI: a 'Re-tag' type badge, an 'Apply Tags' fix button, and an expandable detail that shows, per track, every tag that would change as old -> new (plus source/mode/cover-action summary and any unmatched tracks). So the dry-run finding is actually reviewable before you apply it — the rich details_json the job stores now surfaces in the card. --- webui/static/enrichment.js | 37 ++++++++++++++++++++++++++++++++++++- 1 file changed, 36 insertions(+), 1 deletion(-) diff --git a/webui/static/enrichment.js b/webui/static/enrichment.js index efdfbb29..36208c30 100644 --- a/webui/static/enrichment.js +++ b/webui/static/enrichment.js @@ -2725,7 +2725,7 @@ async function loadRepairFindings() { duplicate_tracks: 'Duplicate', incomplete_album: 'Incomplete', path_mismatch: 'Path Mismatch', metadata_gap: 'Missing Metadata', missing_cover_art: 'Missing Art', track_number_mismatch: 'Track Number', - missing_lossy_copy: 'No Lossy Copy' + missing_lossy_copy: 'No Lossy Copy', library_retag: 'Re-tag' }; // Finding types that have an automated fix action @@ -2740,6 +2740,7 @@ async function loadRepairFindings() { missing_lossy_copy: 'Convert', acoustid_mismatch: 'Fix', missing_discography_track: 'Add to Wishlist', + library_retag: 'Apply Tags', }; container.innerHTML = items.map(f => { @@ -2905,6 +2906,40 @@ function _renderFindingDetail(f) { if (f.file_path) rows.push(['Full Path', f.file_path, 'path']); return _gridRows(rows) + _renderPlayButton(f); + case 'library_retag': { + const tracks = Array.isArray(d.tracks) ? d.tracks : []; + const changed = tracks.filter(t => t.changes && Object.keys(t.changes).length); + const meta = []; + if (d.source) meta.push(`Source: ${d.source}`); + if (d.mode) meta.push(`Mode: ${d.mode}`); + if (d.cover_action) meta.push(`Cover: ${d.cover_action}`); + let html = ''; + if (meta.length) { + html += `
${_escFinding(meta.join(' · '))}
`; + } + if (!changed.length && d.cover_action) { + html += `
Tags already correct — this would refresh cover art only.
`; + } + // Per-track old → new diff (cap the rendered list so huge albums stay sane). + changed.slice(0, 40).forEach(t => { + const label = t.title || (t.file_path || '').split(/[\\/]/).pop(); + const rows = Object.entries(t.changes).map(([field, c]) => [ + field.replace(/_/g, ' '), + `${(c.old === '' || c.old == null) ? '∅' : c.old} → ${c.new}`, + 'highlight', + ]); + html += `
${_escFinding(label)}
`; + html += _gridRows(rows); + }); + if (changed.length > 40) { + html += `
…and ${changed.length - 40} more track(s)
`; + } + if (Array.isArray(d.unmatched) && d.unmatched.length) { + html += `
Unmatched (left untouched): ${_escFinding(d.unmatched.join(', '))}
`; + } + return html || '
No changes.
'; + } + case 'acoustid_mismatch': { let html = media + '
'; html += _renderScoreBar(d.fingerprint_score, 'Fingerprint'); From 3ea9da1cbaeb4077c273a98c5dc4cce993f4252b Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Thu, 4 Jun 2026 09:20:34 -0700 Subject: [PATCH 04/67] Tagging: write source IDs too (Write Tags button + library re-tag now complete) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit write_tags_to_file wrote the core fields + cover but never the source IDs (Spotify/iTunes/MusicBrainz) the import post-process embeds. Added a focused source.embed_known_source_ids() that writes ALREADY-KNOWN ids (from db_data) via the canonical, Picard-compatible frame writer the import uses (_write_embedded_metadata) — no API re-fetch, frames correct by construction. write_tags_to_file now calls it whenever db_data carries id keys. Fed from both paths: the enhanced-library 'Write Tags' button now carries the track's spotify/itunes/musicbrainz ids, and the Library Re-tag job stamps the matched album/track source ids onto each track. So both now write the full tag set, not a subset. --- core/metadata/source.py | 54 +++++++++++++++++++++++++++++++ core/repair_jobs/library_retag.py | 24 +++++++++++++- core/tag_writer.py | 18 +++++++++++ web_server.py | 7 ++++ 4 files changed, 102 insertions(+), 1 deletion(-) diff --git a/core/metadata/source.py b/core/metadata/source.py index fcd69569..2be1444c 100644 --- a/core/metadata/source.py +++ b/core/metadata/source.py @@ -1122,6 +1122,60 @@ def extract_source_metadata(context: dict, artist: dict, album_info: dict) -> di return metadata +def embed_known_source_ids(audio_file, metadata: dict) -> list: + """Embed ALREADY-KNOWN source IDs into a file's tags, no API re-fetch. + + For the library re-tag / "Write Tags" paths: ``metadata`` carries flat id + keys already stored in the DB (``spotify_track_id``, ``spotify_album_id``, + ``itunes_track_id``, ``itunes_album_id``, ``musicbrainz_recording_id``, + ``musicbrainz_release_id``, …). Reuses the SAME canonical, format-specific, + Picard-compatible frame writer as the import post-process + (``_write_embedded_metadata``) so the frames never diverge — it just skips + the heavy multi-source re-search that ``embed_source_ids`` does. + + The caller is responsible for saving the file. Returns the canonical tag + names written (for logging/diagnostics). + """ + cfg = get_config_manager() + symbols = get_mutagen_symbols() + if not symbols or audio_file is None: + return [] + try: + id_tags = _collect_source_ids(metadata, cfg) + # MusicBrainz ids aren't in _collect_source_ids (they come from the MB + # processor at import time); add them from the DB when present, using + # the canonical names the frame map already knows. + if cfg.get("musicbrainz.embed_tags", True) is not False: + if metadata.get("musicbrainz_recording_id"): + id_tags["MUSICBRAINZ_RECORDING_ID"] = metadata["musicbrainz_recording_id"] + if metadata.get("musicbrainz_release_id"): + id_tags["MUSICBRAINZ_RELEASE_ID"] = metadata["musicbrainz_release_id"] + if not id_tags: + return [] + pp = _blank_post_process_state() + pp["id_tags"] = id_tags + _write_embedded_metadata(audio_file, metadata, pp, cfg, symbols) + return list(id_tags.keys()) + except Exception as exc: + logger.debug("embed_known_source_ids failed: %s", exc) + return [] + + +def _blank_post_process_state() -> dict: + """A fully-defaulted post-process state so _write_embedded_metadata can run + with only id_tags set (every enrichment branch no-ops on the blanks).""" + return { + "id_tags": {}, "track_title": "", "artist_name": "", "batch_artist_name": None, + "metadata": {}, "recording_mbid": None, "artist_mbid": None, "release_mbid": "", + "mb_genres": [], "isrc": None, "deezer_bpm": None, "deezer_isrc": None, + "tidal_bpm": None, "hifi_bpm": None, "hifi_copyright": None, "audiodb_mood": None, + "audiodb_style": None, "audiodb_genre": None, "tidal_isrc": None, + "tidal_copyright": None, "hifi_isrc": None, "qobuz_isrc": None, + "qobuz_copyright": None, "qobuz_label": None, "lastfm_tags": [], + "lastfm_url": None, "genius_url": None, "release_year": None, + } + + def embed_source_ids(audio_file, metadata: dict, context: dict = None, runtime=None): cfg = get_config_manager() symbols = get_mutagen_symbols() diff --git a/core/repair_jobs/library_retag.py b/core/repair_jobs/library_retag.py index 4f013faf..c6f11353 100644 --- a/core/repair_jobs/library_retag.py +++ b/core/repair_jobs/library_retag.py @@ -45,6 +45,26 @@ def _read_current_tags(file_path): return {} +def _add_source_ids(db_data, source, album_source_id, source_track): + """Stamp the album/track source IDs onto the write payload so the canonical + writer embeds them too (Spotify / iTunes / MusicBrainz).""" + album_key = {'spotify': 'spotify_album_id', 'itunes': 'itunes_album_id', + 'musicbrainz': 'musicbrainz_release_id'}.get(source) + track_key = {'spotify': 'spotify_track_id', 'itunes': 'itunes_track_id', + 'musicbrainz': 'musicbrainz_recording_id'}.get(source) + if album_key and album_source_id: + db_data[album_key] = album_source_id + if track_key: + tid = None + for k in ('id', 'track_id', 'source_track_id'): + v = source_track.get(k) if isinstance(source_track, dict) else getattr(source_track, k, None) + if v: + tid = v + break + if tid: + db_data[track_key] = tid + + def _track_list(result): """Normalize a get_album_tracks result into a plain list of track items.""" if result is None: @@ -222,12 +242,14 @@ class LibraryRetagJob(RepairJob): # 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). if plan['changes'] or cover_action: + db_data = plan['db_data'] + _add_source_ids(db_data, source, album_source_id, src) track_plans.append({ 'file_path': lib['file_path'], 'track_id': lib['id'], 'title': lib['title'], 'changes': plan['changes'], - 'db_data': plan['db_data'], + 'db_data': db_data, }) tag_change_tracks = sum(1 for tp in track_plans if tp['changes']) diff --git a/core/tag_writer.py b/core/tag_writer.py index e6edbc12..02aa8d1e 100644 --- a/core/tag_writer.py +++ b/core/tag_writer.py @@ -294,6 +294,24 @@ def write_tags_to_file(file_path: str, db_data: Dict[str, Any], year, genre_str, track_num, total_tracks, disc_num, bpm, artists_list=artists_list) + # Embed already-known source IDs (Spotify / iTunes / MusicBrainz) from + # db_data, reusing the canonical import-time frame writer — no API + # re-fetch. Only fires when db_data carries id keys, so the plain + # "write the core tags" callers are unaffected. + _src_meta = {k: db_data[k] for k in ( + 'source', 'source_track_id', 'source_album_id', 'source_artist_id', + 'spotify_track_id', 'spotify_album_id', 'spotify_artist_id', + 'itunes_track_id', 'itunes_album_id', 'itunes_artist_id', + 'musicbrainz_recording_id', 'musicbrainz_release_id', + ) if db_data.get(k)} + if _src_meta: + try: + from core.metadata.source import embed_known_source_ids + if embed_known_source_ids(audio, _src_meta): + written.append('source_ids') + except Exception as e: + logger.debug("source-id embed skipped for %s: %s", file_path, e) + # Embed cover art if requested if embed_cover: art_ok = False diff --git a/web_server.py b/web_server.py index b107fccf..2dd699b9 100644 --- a/web_server.py +++ b/web_server.py @@ -9336,6 +9336,13 @@ def _build_library_tag_db_data(track_data, album_genres=None): if artists_list: db_data['artists_list'] = artists_list + # Carry the known source IDs through so they get embedded too (the writer + # only acts on the ones present). These come from t.* on the track row. + for _k in ('spotify_track_id', 'itunes_track_id', 'musicbrainz_recording_id'): + _v = track_data.get(_k) + if _v: + db_data[_k] = _v + return db_data From d91e6a384d4cf802033d311b2342702bb665bea6 Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Thu, 4 Jun 2026 09:33:03 -0700 Subject: [PATCH 05/67] Remove the old Retag Tool (superseded by Library Re-tag job + Write Tags) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The old per-download Retag Tool was limited (only native-pipeline downloads, 100-group cap, manual per-group) and did the wrong thing — it moved/reorganized files instead of just tagging. It's superseded by the new Library Re-tag job (whole-library, in-place) + the enhanced-library 'Write Tags' button. Removed: the post-download record_retag_download ingestion hook (stops writing retag_groups on every download), core/library/retag.py, the web_server state + deps + /api/retag/* endpoints + the tool:retag WebSocket emit, the dashboard card + both modals (index.html), the core.js socket handler, and the tools-page wiring + help entry (wishlist-tools.js). Updated the import-pipeline test. Verified: web_server parses, app + core imports OK, 392 tests pass, no live references to removed symbols. Left as inert (harmless) for a careful follow-up sweep: the retag_groups/ retag_tracks tables + their DB CRUD methods (no longer written/read), and the now-orphaned retag JS helper functions (no entry point/wiring/socket calls them; interspersed with wishlist functions, so not blind-deleted). --- core/imports/pipeline.py | 8 - core/imports/side_effects.py | 85 ------- core/library/retag.py | 350 -------------------------- tests/imports/test_import_pipeline.py | 1 - web_server.py | 168 ------------- webui/index.html | 74 ------ webui/static/core.js | 1 - webui/static/wishlist-tools.js | 43 ---- 8 files changed, 730 deletions(-) delete mode 100644 core/library/retag.py diff --git a/core/imports/pipeline.py b/core/imports/pipeline.py index dc6c9bf4..067e4559 100644 --- a/core/imports/pipeline.py +++ b/core/imports/pipeline.py @@ -40,7 +40,6 @@ from core.imports.side_effects import ( emit_track_downloaded, record_download_provenance, record_library_history_download, - record_retag_download, record_soulsync_library_entry, ) from core.wishlist.resolution import check_and_remove_from_wishlist @@ -892,13 +891,6 @@ def post_process_matched_download(context_key, context, file_path, runtime, meta record_download_provenance(context) record_soulsync_library_entry(context, artist_context, album_info) - try: - if not playlist_folder_mode: - completed_path = context.get('_final_processed_path', final_path) - record_retag_download(context, artist_context, album_info, completed_path) - except Exception as retag_err: - logger.error(f"[Post-Process] Retag data capture failed (non-fatal): {retag_err}") - try: completed_path = context.get('_final_processed_path', final_path) batch_id_for_repair = context.get('batch_id') diff --git a/core/imports/side_effects.py b/core/imports/side_effects.py index 6513aabf..7d9d92c9 100644 --- a/core/imports/side_effects.py +++ b/core/imports/side_effects.py @@ -676,88 +676,3 @@ def record_soulsync_library_entry(context: Dict[str, Any], artist_context: Dict[ logger.info("[SoulSync Library] Added: %s / %s / %s", artist_name, album_name, track_name) except Exception as exc: logger.error("[SoulSync Library] Could not record library entry: %s", exc) - - -def record_retag_download(context: Dict[str, Any], artist_context: Dict[str, Any], album_info: Dict[str, Any], final_path: str) -> None: - """Record a completed download for later re-tagging.""" - try: - db = get_database() - - context = normalize_import_context(context) - artist_context = get_import_context_artist(context) or (artist_context if isinstance(artist_context, dict) else {}) - album_context = get_import_context_album(context) - track_info = get_import_track_info(context) - original_search = get_import_original_search(context) - source = get_import_source(context) - source_ids = get_import_source_ids(context) - - artist_name = extract_artist_name(artist_context) or get_import_clean_artist(context, default="Unknown Artist") - is_album = album_info and album_info.get("is_album", False) - group_type = "album" if is_album else "single" - album_name = album_info.get("album_name", "") if album_info else get_import_clean_album(context, default=original_search.get("album", "Unknown")) - - image_url = album_info.get("album_image_url") if album_info else None - if not image_url: - image_url = album_context.get("image_url", "") - if not image_url and album_context.get("images"): - images = album_context.get("images", []) - if images and isinstance(images[0], dict): - image_url = images[0].get("url", "") - - total_tracks = album_context.get("total_tracks", 1) if album_context else 1 - release_date = album_context.get("release_date", "") if album_context else "" - - spotify_album_id = None - itunes_album_id = None - if source == "spotify": - spotify_album_id = source_ids.get("album_id", "") or None - elif source == "itunes": - itunes_album_id = source_ids.get("album_id", "") or None - - group_id = db.find_retag_group(artist_name, album_name) - if group_id is None: - group_id = db.add_retag_group( - group_type=group_type, - artist_name=artist_name, - album_name=album_name, - image_url=image_url, - spotify_album_id=spotify_album_id, - itunes_album_id=itunes_album_id, - total_tracks=total_tracks, - release_date=release_date, - ) - if group_id is None: - return - - track_number = album_info.get("track_number", 1) if album_info else (track_info.get("track_number", 1) or 1) - disc_number = original_search.get("disc_number") or (album_info.get("disc_number", 1) if album_info else track_info.get("disc_number", 1) or 1) - title = get_import_clean_title( - context, - album_info=album_info, - default=album_info.get("clean_track_name", "Unknown Track") if album_info else "Unknown Track", - ) - file_format = os.path.splitext(str(final_path))[1].lstrip(".").lower() - - source_track_id = None - itunes_track_id = None - if source == "spotify": - source_track_id = source_ids.get("track_id", "") or None - elif source == "itunes": - itunes_track_id = source_ids.get("track_id", "") or None - - if not db.retag_track_exists(group_id, str(final_path)): - db.add_retag_track( - group_id=group_id, - track_number=track_number, - disc_number=disc_number, - title=title, - file_path=str(final_path), - file_format=file_format, - spotify_track_id=source_track_id, - itunes_track_id=itunes_track_id, - ) - logger.info("[Retag] Recorded track for retag: '%s' in '%s'", title, album_name) - - db.trim_retag_groups(100) - except Exception as exc: - logger.error("[Retag] Could not record track for retag: %s", exc) diff --git a/core/library/retag.py b/core/library/retag.py deleted file mode 100644 index f5cf74e7..00000000 --- a/core/library/retag.py +++ /dev/null @@ -1,350 +0,0 @@ -"""Library retag worker. - -`execute_retag(group_id, album_id, deps)` rewrites tags + filenames for a -group of audio files when the user has matched them to a different -album. The worker: - -1. Fetches album + track metadata for the new `album_id` (Spotify or - iTunes — Spotify client transparently falls back). -2. Loads existing files in the retag group from the DB. -3. Matches each existing track to a new Spotify track: - - Priority 1: same disc + track number. - - Priority 2: title similarity >= 0.6 (SequenceMatcher). -4. For each matched pair: - - Re-write metadata tags via `_enhance_file_metadata`. - - Compute the new path via `_build_final_path_for_track` and move - the audio file (plus .lrc / .txt sidecars) if the path changes. - - Drop an orphaned cover.jpg if it's left in an empty directory. - - Clean up empty parent directories left behind. - - Download the new cover art into the new album dir. -5. Update the retag group record with the new artist / album / image / - total_tracks / release_date and the appropriate Spotify-or-iTunes - album ID. -6. Mark the retag state 'finished' (or 'error' on exception). - -The original mutated `retag_state` as a module global. Here it's exposed -through the `RetagDeps` proxy as a Python property so the lifted body -keeps the same `name[key] = value` syntax. The property setter rebinds -the web_server.py reference if needed (currently the function only -mutates in place via .update() and key assignment, so the setter never -fires). -""" - -from __future__ import annotations - -import logging -import os -import traceback -from dataclasses import dataclass -from difflib import SequenceMatcher -from typing import Any, Callable, Optional - -logger = logging.getLogger(__name__) - - -@dataclass -class RetagDeps: - """Bundle of cross-cutting deps the retag worker needs. - - `retag_state` is exposed as a property so the lifted body keeps - `name[key] = value` / `name.update(...)` syntax. - """ - config_manager: Any - retag_lock: Any # threading.Lock - spotify_client: Any - get_audio_quality_string: Callable[[str], str] - enhance_file_metadata: Callable - build_final_path_for_track: Callable - safe_move_file: Callable - cleanup_empty_directories: Callable - download_cover_art: Callable - docker_resolve_path: Callable[[str], str] - _get_retag_state: Callable[[], dict] - _set_retag_state: Callable[[dict], None] - get_database: Callable[[], Any] - # Discord report (Netti93) — retag was clearing the LYRICS / USLT - # tag without rewriting it, while the download pipeline calls - # `generate_lrc_file` after enrichment to refetch + embed lyrics. - # Injected here so retag mirrors the same post-enrichment step. - # Optional for backward compat with any test caller that builds - # RetagDeps without the new field — empty default no-ops the call. - generate_lrc_file: Optional[Callable] = None - - @property - def retag_state(self) -> dict: - return self._get_retag_state() - - @retag_state.setter - def retag_state(self, value: dict) -> None: - self._set_retag_state(value) - - -def execute_retag(group_id, album_id, deps: RetagDeps): - """Execute a retag operation: re-tag files in a group with metadata from a new album match.""" - try: - with deps.retag_lock: - deps.retag_state.update({ - "status": "running", - "phase": "Fetching album metadata...", - "progress": 0, - "current_track": "", - "total_tracks": 0, - "processed": 0, - "error_message": "" - }) - - # 1. Fetch new album metadata from Spotify/iTunes - album_data = deps.spotify_client.get_album(album_id) - if not album_data: - raise ValueError(f"Could not fetch album data for ID: {album_id}") - - album_tracks_response = deps.spotify_client.get_album_tracks(album_id) - if not album_tracks_response: - raise ValueError(f"Could not fetch album tracks for ID: {album_id}") - - album_tracks_items = album_tracks_response.get('items', []) - - # Extract artist info - album_artists = album_data.get('artists', []) - new_artist = album_artists[0] if album_artists else {'name': 'Unknown Artist', 'id': ''} - # Ensure artist is a dict with expected fields - if not isinstance(new_artist, dict): - new_artist = {'name': str(new_artist), 'id': ''} - new_album_name = album_data.get('name', 'Unknown Album') - new_images = album_data.get('images', []) - new_image_url = new_images[0]['url'] if new_images else None - new_release_date = album_data.get('release_date', '') - total_tracks = album_data.get('total_tracks', len(album_tracks_items)) - - # Build spotify track list - spotify_tracks = [] - for item in album_tracks_items: - track_artists = item.get('artists', []) - spotify_tracks.append({ - 'name': item.get('name', ''), - 'track_number': item.get('track_number', 1), - 'disc_number': item.get('disc_number', 1), - 'id': item.get('id', ''), - 'artists': track_artists, - 'duration_ms': item.get('duration_ms', 0) - }) - - total_discs = max((t['disc_number'] for t in spotify_tracks), default=1) - - # 2. Load existing tracks for this group - db = deps.get_database() - existing_tracks = db.get_retag_tracks(group_id) - if not existing_tracks: - raise ValueError(f"No tracks found for retag group {group_id}") - - with deps.retag_lock: - deps.retag_state['total_tracks'] = len(existing_tracks) - deps.retag_state['phase'] = "Matching tracks..." - - # 3. Match existing files to new tracklist - matched_pairs = [] - for existing_track in existing_tracks: - best_match = None - best_score = 0 - - # Priority 1: Match by track number - for st in spotify_tracks: - if (st['track_number'] == existing_track.get('track_number') and - st['disc_number'] == existing_track.get('disc_number', 1)): - best_match = st - best_score = 1.0 - break - - # Priority 2: Match by title similarity - if not best_match: - from difflib import SequenceMatcher - existing_title = (existing_track.get('title') or '').lower().strip() - for st in spotify_tracks: - st_title = (st.get('name') or '').lower().strip() - score = SequenceMatcher(None, existing_title, st_title).ratio() - if score > best_score and score > 0.6: - best_score = score - best_match = st - - if best_match: - matched_pairs.append((existing_track, best_match)) - else: - logger.warning(f"[Retag] No match found for track: '{existing_track.get('title')}'") - matched_pairs.append((existing_track, None)) - - with deps.retag_lock: - deps.retag_state['phase'] = "Retagging files..." - - # 4. Retag each matched track - for existing_track, matched_spotify in matched_pairs: - current_file_path = existing_track.get('file_path', '') - track_title = matched_spotify['name'] if matched_spotify else existing_track.get('title', 'Unknown') - - with deps.retag_lock: - deps.retag_state['current_track'] = track_title - - if not matched_spotify: - with deps.retag_lock: - deps.retag_state['processed'] += 1 - deps.retag_state['progress'] = int(deps.retag_state['processed'] / deps.retag_state['total_tracks'] * 100) - continue - - # Verify file exists - if not os.path.exists(current_file_path): - logger.warning(f"[Retag] File not found, skipping: {current_file_path}") - with deps.retag_lock: - deps.retag_state['processed'] += 1 - deps.retag_state['progress'] = int(deps.retag_state['processed'] / deps.retag_state['total_tracks'] * 100) - continue - - # Build synthetic context for _enhance_file_metadata - track_artists = matched_spotify.get('artists', []) - context = { - 'original_search_result': { - 'spotify_clean_title': matched_spotify['name'], - 'spotify_clean_album': new_album_name, - 'track_number': matched_spotify['track_number'], - 'disc_number': matched_spotify.get('disc_number', 1), - 'artists': track_artists, - 'title': matched_spotify['name'] - }, - 'spotify_album': { - 'id': album_id, - 'name': new_album_name, - 'release_date': new_release_date, - 'total_tracks': total_tracks, - 'image_url': new_image_url, - 'total_discs': total_discs - }, - 'track_info': {'id': matched_spotify['id']}, - 'spotify_artist': new_artist, - '_audio_quality': deps.get_audio_quality_string(current_file_path) or '' - } - - album_info = { - 'is_album': total_tracks > 1, - 'album_name': new_album_name, - 'track_number': matched_spotify['track_number'], - 'disc_number': matched_spotify.get('disc_number', 1), - 'clean_track_name': matched_spotify['name'], - 'album_image_url': new_image_url - } - - # Re-write metadata tags - try: - deps.enhance_file_metadata(current_file_path, context, new_artist, album_info) - logger.info(f"[Retag] Re-tagged: '{track_title}'") - except Exception as meta_err: - logger.error(f"[Retag] Metadata write failed for '{track_title}': {meta_err}") - - # Discord report (Netti93) — `enhance_file_metadata` clears - # ALL tags (incl. USLT lyrics) and rewrites only the source - # metadata. The download pipeline calls `generate_lrc_file` - # after enrichment to refetch + embed lyrics — retag was - # missing that step and dropped the LYRICS tag with no - # rewrite. Mirroring the download path's post-enrichment - # step. Same args, same `lrclib_enabled` config gate, same - # idempotency (skip when sidecar already present). - if deps.generate_lrc_file: - try: - deps.generate_lrc_file(current_file_path, context, new_artist, album_info) - except Exception as lrc_err: - logger.debug("[Retag] generate_lrc_file failed for '%s': %s", track_title, lrc_err) - - # Compute new path and move if different - file_ext = os.path.splitext(current_file_path)[1] - try: - new_path, _ = deps.build_final_path_for_track(context, new_artist, album_info, file_ext) - - if os.path.normpath(current_file_path) != os.path.normpath(new_path): - logger.info(f"[Retag] Moving '{os.path.basename(current_file_path)}' -> '{new_path}'") - old_dir = os.path.dirname(current_file_path) - os.makedirs(os.path.dirname(new_path), exist_ok=True) - deps.safe_move_file(current_file_path, new_path) - - # Move lyrics sidecar file alongside audio file if it exists - for lyrics_ext in ('.lrc', '.txt'): - old_lyrics = os.path.splitext(current_file_path)[0] + lyrics_ext - if os.path.exists(old_lyrics): - new_lyrics = os.path.splitext(new_path)[0] + lyrics_ext - try: - deps.safe_move_file(old_lyrics, new_lyrics) - logger.info(f"[Retag] Moved {lyrics_ext} file alongside audio") - except Exception as lrc_err: - logger.error(f"[Retag] Failed to move {lyrics_ext} file: {lrc_err}") - - # Remove old cover.jpg if directory changed and old dir is now empty of audio - new_dir = os.path.dirname(new_path) - if os.path.normpath(old_dir) != os.path.normpath(new_dir): - old_cover = os.path.join(old_dir, 'cover.jpg') - if os.path.exists(old_cover): - # Check if any audio files remain in old directory - audio_exts = {'.flac', '.mp3', '.m4a', '.ogg', '.opus', '.wav', '.aac'} - remaining_audio = [f for f in os.listdir(old_dir) - if os.path.splitext(f)[1].lower() in audio_exts] - if not remaining_audio: - try: - os.remove(old_cover) - logger.warning("[Retag] Removed orphaned cover.jpg from old directory") - except Exception as e: - logger.debug("remove orphaned cover failed: %s", e) - - # Cleanup old empty directories - transfer_dir = deps.docker_resolve_path(deps.config_manager.get('soulseek.transfer_path', './Transfer')) - deps.cleanup_empty_directories(transfer_dir, current_file_path) - - # Update DB record - db.update_retag_track_path(existing_track['id'], str(new_path)) - current_file_path = new_path - else: - logger.warning(f"[Retag] Path unchanged for '{track_title}', no move needed") - except Exception as move_err: - logger.error(f"[Retag] Path/move failed for '{track_title}': {move_err}") - - # Download cover art to album directory - try: - deps.download_cover_art(album_info, os.path.dirname(current_file_path), context) - except Exception as cover_err: - logger.error(f"[Retag] Cover art download failed: {cover_err}") - - with deps.retag_lock: - deps.retag_state['processed'] += 1 - deps.retag_state['progress'] = int(deps.retag_state['processed'] / deps.retag_state['total_tracks'] * 100) - - # 5. Update the retag group record with new metadata - update_kwargs = { - 'artist_name': new_artist.get('name', 'Unknown Artist'), - 'album_name': new_album_name, - 'image_url': new_image_url, - 'total_tracks': total_tracks, - 'release_date': new_release_date - } - # Set the correct ID field based on Spotify vs iTunes - if str(album_id).isdigit(): - update_kwargs['itunes_album_id'] = album_id - update_kwargs['spotify_album_id'] = None - else: - update_kwargs['spotify_album_id'] = album_id - update_kwargs['itunes_album_id'] = None - - db.update_retag_group(group_id, **update_kwargs) - - with deps.retag_lock: - deps.retag_state.update({ - "status": "finished", - "phase": "Retag complete!", - "progress": 100, - "current_track": "" - }) - logger.info(f"[Retag] Retag operation complete for group {group_id}") - - except Exception as e: - import traceback - logger.error(f"[Retag] Error during retag: {e}") - logger.error(traceback.format_exc()) - with deps.retag_lock: - deps.retag_state.update({ - "status": "error", - "phase": "Error", - "error_message": str(e) - }) diff --git a/tests/imports/test_import_pipeline.py b/tests/imports/test_import_pipeline.py index 13684241..b283cb7e 100644 --- a/tests/imports/test_import_pipeline.py +++ b/tests/imports/test_import_pipeline.py @@ -209,7 +209,6 @@ def test_post_process_matched_download_forwards_separate_metadata_runtime(tmp_pa monkeypatch.setattr(import_pipeline, "record_soulsync_library_entry", _record_library) monkeypatch.setattr(import_pipeline, "check_and_remove_from_wishlist", lambda *args, **kwargs: None) - monkeypatch.setattr(import_pipeline, "record_retag_download", lambda *args, **kwargs: None) context = { "track_info": {"_playlist_folder_mode": True, "_playlist_name": "Playlist"}, diff --git a/web_server.py b/web_server.py index 2dd699b9..04390e3a 100644 --- a/web_server.py +++ b/web_server.py @@ -862,19 +862,6 @@ duplicate_cleaner_state = { duplicate_cleaner_lock = threading.Lock() duplicate_cleaner_executor = ThreadPoolExecutor(max_workers=1, thread_name_prefix="DuplicateCleaner") -# Retag Tool Globals -retag_state = { - "status": "idle", - "phase": "Ready", - "progress": 0, - "current_track": "", - "total_tracks": 0, - "processed": 0, - "error_message": "", -} -retag_lock = threading.Lock() -retag_executor = ThreadPoolExecutor(max_workers=1, thread_name_prefix="RetagWorker") - # Download Missing Tracks Modal State Management # Thread-safe state tracking for modal download functionality. # Shared task/batch state now lives in core.runtime_state. @@ -1709,7 +1696,6 @@ def _shutdown_runtime_components(): (db_update_executor, "db update executor"), (quality_scanner_executor, "quality scanner executor"), (duplicate_cleaner_executor, "duplicate cleaner executor"), - (retag_executor, "retag executor"), (sync_executor, "sync executor"), (missing_download_executor, "missing download executor"), (album_bundle_executor, "album bundle executor"), @@ -14274,45 +14260,6 @@ _download_retry_attempts = {} # {context_key: {'count': N, 'first_attempt': tim _download_retry_max = 10 # Max retries before giving up (10 seconds with 1s poll interval) _download_retry_lock = threading.Lock() -# Retag worker logic lives in core/library/retag.py. -from core.library import retag as _library_retag - - -def _build_retag_deps(): - """Build the RetagDeps bundle from web_server.py globals on each call.""" - from database.music_database import get_database as _get_db - - def _get_state(): - return retag_state - - def _set_state(value): - global retag_state - retag_state = value - - from core.metadata.lyrics import generate_lrc_file as _generate_lrc_file - - return _library_retag.RetagDeps( - config_manager=config_manager, - retag_lock=retag_lock, - spotify_client=spotify_client, - get_audio_quality_string=_get_audio_quality_string, - enhance_file_metadata=_enhance_file_metadata, - build_final_path_for_track=_build_final_path_for_track, - safe_move_file=_safe_move_file, - cleanup_empty_directories=_cleanup_empty_directories, - download_cover_art=_download_cover_art, - docker_resolve_path=docker_resolve_path, - _get_retag_state=_get_state, - _set_retag_state=_set_state, - get_database=_get_db, - generate_lrc_file=_generate_lrc_file, - ) - - -def _execute_retag(group_id, album_id): - return _library_retag.execute_retag(group_id, album_id, _build_retag_deps()) - - def _automatic_wishlist_cleanup_after_db_update(): """Automatic wishlist cleanup that runs after database updates.""" return _cleanup_wishlist_after_db_update(logger=logger) @@ -16461,115 +16408,6 @@ def stop_duplicate_cleaner(): # == RETAG TOOL ENDPOINTS == # =============================== -@app.route('/api/retag/stats', methods=['GET']) -def get_retag_stats(): - """Get retag tool statistics for the dashboard card.""" - from database.music_database import get_database - db = get_database() - stats = db.get_retag_stats() - return jsonify({"success": True, **stats}) - -@app.route('/api/retag/groups', methods=['GET']) -def get_retag_groups(): - """Get all retag groups sorted by artist name.""" - from database.music_database import get_database - db = get_database() - groups = db.get_retag_groups() - return jsonify({"success": True, "groups": groups}) - -@app.route('/api/retag/groups//tracks', methods=['GET']) -def get_retag_group_tracks(group_id): - """Get tracks for a specific retag group.""" - from database.music_database import get_database - db = get_database() - tracks = db.get_retag_tracks(group_id) - return jsonify({"success": True, "tracks": tracks}) - -@app.route('/api/retag/search', methods=['GET']) -def search_retag_albums(): - """Search for albums to use for retagging (uses Spotify/iTunes fallback).""" - query = request.args.get('q', '').strip() - if not query: - return jsonify({"success": False, "error": "Query parameter 'q' is required"}), 400 - - limit = min(int(request.args.get('limit', 12)), 50) - try: - results = spotify_client.search_albums(query, limit=limit) - albums = [] - for a in results: - albums.append({ - 'id': str(a.id), - 'name': a.name, - 'artist': ', '.join(a.artists) if a.artists else 'Unknown Artist', - 'release_date': a.release_date or '', - 'total_tracks': a.total_tracks, - 'image_url': a.image_url, - 'album_type': a.album_type or 'album' - }) - return jsonify({"success": True, "albums": albums}) - except Exception as e: - logger.error(f"[Retag] Album search error: {e}") - return jsonify({"success": False, "error": str(e)}), 500 - -@app.route('/api/retag/execute', methods=['POST']) -def execute_retag(): - """Start a retag operation for a group with a new album match.""" - data = request.get_json() - if not data: - return jsonify({"success": False, "error": "JSON body required"}), 400 - - group_id = data.get('group_id') - album_id = data.get('album_id') - if not group_id or not album_id: - return jsonify({"success": False, "error": "group_id and album_id are required"}), 400 - - with retag_lock: - if retag_state["status"] == "running": - return jsonify({"success": False, "error": "A retag operation is already running"}), 409 - - retag_executor.submit(_execute_retag, group_id, str(album_id)) - return jsonify({"success": True, "message": "Retag operation started"}) - -@app.route('/api/retag/status', methods=['GET']) -def get_retag_status(): - """Get the current retag operation status.""" - with retag_lock: - return jsonify(dict(retag_state)) - -@app.route('/api/retag/groups/', methods=['DELETE']) -def delete_retag_group(group_id): - """Delete a retag group (files are NOT deleted).""" - from database.music_database import get_database - db = get_database() - success = db.delete_retag_group(group_id) - if success: - return jsonify({"success": True}) - else: - return jsonify({"success": False, "error": "Group not found"}), 404 - -@app.route('/api/retag/groups/delete-batch', methods=['POST']) -def delete_retag_groups_batch(): - """Delete multiple retag groups at once.""" - from database.music_database import get_database - data = request.get_json() or {} - group_ids = data.get('group_ids', []) - if not group_ids: - return jsonify({"success": False, "error": "No group IDs provided"}), 400 - db = get_database() - removed = 0 - for gid in group_ids: - if db.delete_retag_group(int(gid)): - removed += 1 - return jsonify({"success": True, "removed": removed}) - -@app.route('/api/retag/groups/clear-all', methods=['POST']) -def clear_all_retag_groups(): - """Delete all retag groups.""" - from database.music_database import get_database - db = get_database() - count = db.delete_all_retag_groups() - return jsonify({"success": True, "removed": count}) - # =============================== # == DOWNLOAD MISSING TRACKS == # =============================== @@ -34968,12 +34806,6 @@ def _emit_tool_progress_loop(): socketio.emit('tool:duplicate-cleaner', state_copy) except Exception as e: logger.debug(f"Error emitting duplicate cleaner status: {e}") - # Retag - try: - with retag_lock: - socketio.emit('tool:retag', dict(retag_state)) - except Exception as e: - logger.debug(f"Error emitting retag status: {e}") # DB Update try: with db_update_lock: diff --git a/webui/index.html b/webui/index.html index f9d3204c..b30f9212 100644 --- a/webui/index.html +++ b/webui/index.html @@ -6708,46 +6708,6 @@
-
-
-

Retag Tool

- -
-

Fix metadata on previously downloaded albums & singles

-
-
- Groups: - 0 -
-
- Tracks: - 0 -
-
- Artists: - 0 -
-
- Status: - Idle -
-
-
- -
-
-

Ready

-
-
-
-
-

0 / 0 tracks (0.0%)

-
-
- - -

Management

@@ -7788,40 +7748,6 @@ -
-
-
-
-

Retag Tool

-
-
- - -
-
- -
-
Loading downloads...
-
-
-
- - -
-
-
-

Search for Correct Album

- -
-
- -
-
-
-
diff --git a/webui/static/core.js b/webui/static/core.js index 48843c3e..44681920 100644 --- a/webui/static/core.js +++ b/webui/static/core.js @@ -487,7 +487,6 @@ function initializeWebSocket() { socket.on('tool:stream', (data) => updateStreamStatusFromData(data)); socket.on('tool:quality-scanner', (data) => updateQualityScanProgressFromData(data)); socket.on('tool:duplicate-cleaner', (data) => updateDuplicateCleanProgressFromData(data)); - socket.on('tool:retag', (data) => updateRetagStatusFromData(data)); socket.on('tool:db-update', (data) => updateDbProgressFromData(data)); socket.on('tool:metadata', (data) => updateMetadataStatusFromData(data)); socket.on('tool:logs', (data) => updateLogsFromData(data)); diff --git a/webui/static/wishlist-tools.js b/webui/static/wishlist-tools.js index d2f2db1a..774ab25f 100644 --- a/webui/static/wishlist-tools.js +++ b/webui/static/wishlist-tools.js @@ -5581,42 +5581,6 @@ const TOOL_HELP_CONTENT = {

This tool replicates the same scan process that runs automatically after completing a download modal - ensuring your new tracks are immediately available in your library!

` }, - 'retag-tool': { - title: 'Retag Tool', - content: ` -

What does this tool do?

-

The Retag Tool lets you fix metadata on files that have already been downloaded and processed. If an album was tagged with wrong metadata, you can search for the correct match and re-apply tags.

- -

How it works

-
    -
  • Browse your past downloads organized by artist
    • -
  • Expand an album or single to see individual tracks
    • -
  • Click Retag to search for the correct album match
    • -
  • Select the right album and confirm — metadata and file paths are updated automatically
    • -
- -

What gets updated?

-
    -
  • File tags: Title, artist, album, track number, genre, cover art
    • -
  • File paths: Files are moved/renamed to match new metadata (based on your path template)
    • -
  • Cover art: cover.jpg is updated in the album folder
    • -
- -

Stats Explained

-
    -
  • Groups: Number of album/single download groups tracked
    • -
  • Tracks: Total individual track files tracked
    • -
  • Artists: Number of unique artists across all groups
    • -
- -

Notes

-
    -
  • Only album and single downloads are tracked (not playlists)
    • -
  • Deleting a group from the list does not delete the files
    • -
  • Only one retag operation can run at a time
    • -
- ` - }, 'discover-page': { title: 'Discover Page Guide', content: ` @@ -7448,11 +7412,6 @@ async function initializeToolsPage() { duplicateCleanButton._toolsWired = true; } - const retagOpenButton = document.getElementById('retag-open-button'); - if (retagOpenButton && !retagOpenButton._toolsWired) { - retagOpenButton.addEventListener('click', openRetagModal); - retagOpenButton._toolsWired = true; - } const mediaScanButton = document.getElementById('media-scan-button'); if (mediaScanButton && !mediaScanButton._toolsWired) { @@ -7472,8 +7431,6 @@ async function initializeToolsPage() { await checkAndShowMediaScanForPlex(); loadBackupList(); initializeToolHelpButtons(); - loadRetagStats(); - checkRetagStatus(); await fetchAndUpdateDbStats(); loadDiscoveryPoolStats(); loadMetadataCacheStats(); From 48debb792681ed3f7d7cc5cfc21ccbe33640374d Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Thu, 4 Jun 2026 09:46:35 -0700 Subject: [PATCH 06/67] Library re-tag: seam tests for the job scan, apply handler, and source-id embed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes the kettui gap — the orchestration was unproven. Injected-fake seam tests (temp sqlite + real empty track files, no metadata APIs / no real tag writes): - embed_known_source_ids: builds the right canonical id_tags from flat db keys, honors the musicbrainz embed gate, no-ops when there's nothing to write. - library_retag scan: produces a detailed finding with the per-track old->new diff + stamped source ids, and skips an album that's already correct. - _add_source_ids: per-source key mapping. - _fix_library_retag apply: writes each track's payload, and reports failure when files are unreachable. 476 tests pass; ruff clean. --- tests/test_embed_known_source_ids.py | 69 ++++++++++++ tests/test_library_retag_job.py | 158 +++++++++++++++++++++++++++ 2 files changed, 227 insertions(+) create mode 100644 tests/test_embed_known_source_ids.py create mode 100644 tests/test_library_retag_job.py diff --git a/tests/test_embed_known_source_ids.py b/tests/test_embed_known_source_ids.py new file mode 100644 index 00000000..9aabf53e --- /dev/null +++ b/tests/test_embed_known_source_ids.py @@ -0,0 +1,69 @@ +"""Seam test: embed_known_source_ids builds the right id_tags and routes them +through the canonical frame writer (no API re-fetch).""" + +import sys +import types +from types import SimpleNamespace + +# Stub spotipy/config so core.metadata.source imports cleanly in tests. +if 'spotipy' not in sys.modules: + sp = types.ModuleType('spotipy'); oa = types.ModuleType('spotipy.oauth2') + sp.Spotify = type('S', (), {}); oa.SpotifyOAuth = oa.SpotifyClientCredentials = type('O', (), {}) + sp.oauth2 = oa; sys.modules['spotipy'] = sp; sys.modules['spotipy.oauth2'] = oa +if 'config.settings' not in sys.modules: + cm = types.ModuleType('config'); sm = types.ModuleType('config.settings') + + class _Cfg: + def get(self, k, d=None): return d + def get_active_media_server(self): return 'plex' + sm.config_manager = _Cfg(); cm.settings = sm + sys.modules['config'] = cm; sys.modules['config.settings'] = sm + +from core.metadata import source as src + + +def test_builds_id_tags_from_flat_keys_and_calls_writer(monkeypatch): + captured = {} + monkeypatch.setattr(src, 'get_mutagen_symbols', lambda: SimpleNamespace()) + monkeypatch.setattr(src, 'get_config_manager', lambda: SimpleNamespace(get=lambda k, d=None: d)) + + def _fake_write(audio, metadata, pp, cfg, symbols): + captured['id_tags'] = dict(pp['id_tags']) + monkeypatch.setattr(src, '_write_embedded_metadata', _fake_write) + + meta = { + 'spotify_track_id': 'sp_t', 'spotify_album_id': 'sp_a', + 'itunes_track_id': 'it_t', + 'musicbrainz_recording_id': 'mb_rec', 'musicbrainz_release_id': 'mb_rel', + } + written = src.embed_known_source_ids(object(), meta) + + assert captured['id_tags']['SPOTIFY_TRACK_ID'] == 'sp_t' + assert captured['id_tags']['SPOTIFY_ALBUM_ID'] == 'sp_a' + assert captured['id_tags']['ITUNES_TRACK_ID'] == 'it_t' + assert captured['id_tags']['MUSICBRAINZ_RECORDING_ID'] == 'mb_rec' + assert captured['id_tags']['MUSICBRAINZ_RELEASE_ID'] == 'mb_rel' + assert set(written) >= {'SPOTIFY_TRACK_ID', 'MUSICBRAINZ_RECORDING_ID'} + + +def test_no_ids_writes_nothing(monkeypatch): + called = {'n': 0} + monkeypatch.setattr(src, 'get_mutagen_symbols', lambda: SimpleNamespace()) + monkeypatch.setattr(src, 'get_config_manager', lambda: SimpleNamespace(get=lambda k, d=None: d)) + monkeypatch.setattr(src, '_write_embedded_metadata', + lambda *a, **k: called.__setitem__('n', called['n'] + 1)) + assert src.embed_known_source_ids(object(), {'title': 'x'}) == [] + assert called['n'] == 0 # nothing to embed → writer never called + + +def test_musicbrainz_gated_off(monkeypatch): + captured = {} + monkeypatch.setattr(src, 'get_mutagen_symbols', lambda: SimpleNamespace()) + # mb embed disabled + monkeypatch.setattr(src, 'get_config_manager', + lambda: SimpleNamespace(get=lambda k, d=None: False if k == 'musicbrainz.embed_tags' else d)) + monkeypatch.setattr(src, '_write_embedded_metadata', + lambda audio, m, pp, cfg, sym: captured.update(pp['id_tags'])) + src.embed_known_source_ids(object(), {'spotify_track_id': 'sp', 'musicbrainz_recording_id': 'mb'}) + assert 'SPOTIFY_TRACK_ID' in captured + assert 'MUSICBRAINZ_RECORDING_ID' not in captured # gated off diff --git a/tests/test_library_retag_job.py b/tests/test_library_retag_job.py new file mode 100644 index 00000000..28ab0c22 --- /dev/null +++ b/tests/test_library_retag_job.py @@ -0,0 +1,158 @@ +"""Seam tests for the Library Re-tag job scan + the apply handler. + +Injected fakes only — no metadata APIs, no real tag writes. A temp sqlite db ++ real (empty) track files exercise the orchestration: scan -> detailed +finding, and finding -> per-track write payload. +""" + +import sqlite3 +import sys +import types +from types import SimpleNamespace + +# Stub optional deps so the modules import in the test env. +if 'spotipy' not in sys.modules: + sp = types.ModuleType('spotipy'); oa = types.ModuleType('spotipy.oauth2') + sp.Spotify = type('S', (), {}); oa.SpotifyOAuth = oa.SpotifyClientCredentials = type('O', (), {}) + sp.oauth2 = oa; sys.modules['spotipy'] = sp; sys.modules['spotipy.oauth2'] = oa +if 'config.settings' not in sys.modules: + cm = types.ModuleType('config'); sm = types.ModuleType('config.settings') + + class _Cfg: + def get(self, k, d=None): return d + def get_active_media_server(self): return 'plex' + sm.config_manager = _Cfg(); cm.settings = sm + sys.modules['config'] = cm; sys.modules['config.settings'] = sm + +from core.repair_jobs import library_retag as lr + + +def _db_with_album(path, track_file, current_title='Old Title'): + conn = sqlite3.connect(path) + c = conn.cursor() + c.execute("CREATE TABLE artists (id INTEGER PRIMARY KEY, name TEXT)") + c.execute("""CREATE TABLE albums (id INTEGER PRIMARY KEY, title TEXT, artist_id INTEGER, + spotify_album_id TEXT, itunes_album_id TEXT, deezer_id TEXT, musicbrainz_release_id TEXT)""") + c.execute("""CREATE TABLE tracks (id INTEGER PRIMARY KEY, album_id INTEGER, title TEXT, + track_number INTEGER, disc_number INTEGER, file_path TEXT)""") + c.execute("INSERT INTO artists (id, name) VALUES (1, 'Real Artist')") + c.execute("INSERT INTO albums (id, title, artist_id, spotify_album_id) VALUES (1, 'Real Album', 1, 'sp_alb')") + c.execute("INSERT INTO tracks (id, album_id, title, track_number, disc_number, file_path) VALUES (1, 1, ?, 1, 1, ?)", + (current_title, track_file)) + conn.commit() + return conn + + +def _context(conn, settings): + findings = [] + return SimpleNamespace( + db=SimpleNamespace(_get_connection=lambda: conn), + config_manager=SimpleNamespace(get=lambda k, d=None: {f'repair.jobs.library_retag.settings': settings}.get(k, d)), + check_stop=lambda: False, wait_if_paused=lambda: False, + update_progress=lambda *a, **k: None, report_progress=lambda *a, **k: None, + create_finding=lambda **kw: (findings.append(kw) or True), + findings=findings, + ) + + +_ALBUM_META = {'name': 'Real Album', 'artists': [{'name': 'Real Artist'}], + 'year': '2021', 'genres': ['Rock'], 'total_tracks': 1, + 'image_url': 'http://art/cover.jpg'} +_SRC_TRACKS = [{'name': 'Real Title', 'track_number': 1, 'disc_number': 1, 'id': 'sp_trk'}] + + +def _patch_source(monkeypatch, current_tags): + monkeypatch.setattr(lr, 'get_album_for_source', lambda s, i: _ALBUM_META) + monkeypatch.setattr(lr, 'get_album_tracks_for_source', lambda s, i: list(_SRC_TRACKS)) + monkeypatch.setattr(lr, '_read_current_tags', lambda p: dict(current_tags)) + + +def test_scan_creates_detailed_finding(tmp_path, monkeypatch): + 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', 'album_artist': 'Real Artist', 'album': 'Real Album', + 'year': '2021', 'genre': 'Rock', 'track_number': 1, 'disc_number': 1, + }) + + result = lr.LibraryRetagJob().scan(ctx) + + assert result.findings_created == 1 + d = ctx.findings[0]['details'] + assert ctx.findings[0]['finding_type'] == 'library_retag' + assert d['source'] == 'spotify' + assert d['cover_action'] == 'replace' + tp = d['tracks'][0] + assert tp['changes']['title'] == {'old': 'Old Title', 'new': 'Real Title'} + # source ids stamped onto the write payload + assert tp['db_data']['spotify_album_id'] == 'sp_alb' + assert tp['db_data']['spotify_track_id'] == 'sp_trk' + assert tp['db_data']['title'] == 'Real Title' + + +def test_scan_skips_album_already_correct(tmp_path, monkeypatch): + track = tmp_path / 'track.flac'; track.write_bytes(b'') + conn = _db_with_album(str(tmp_path / 'm.db'), str(track), current_title='Real Title') + # cover skipped + tags already match → nothing to do + ctx = _context(conn, {'mode': 'overwrite', 'cover_art': 'skip', 'source': 'spotify'}) + _patch_source(monkeypatch, { + 'title': 'Real Title', 'album_artist': 'Real Artist', 'album': 'Real Album', + 'year': '2021', 'genre': 'Rock', 'track_number': 1, 'disc_number': 1, + }) + + result = lr.LibraryRetagJob().scan(ctx) + assert result.findings_created == 0 + assert ctx.findings == [] + assert result.skipped >= 1 + + +def test_add_source_ids_maps_per_source(): + db = {} + lr._add_source_ids(db, 'spotify', 'AL', {'id': 'TR'}) + assert db == {'spotify_album_id': 'AL', 'spotify_track_id': 'TR'} + db2 = {} + lr._add_source_ids(db2, 'musicbrainz', 'REL', {'id': 'REC'}) + assert db2 == {'musicbrainz_release_id': 'REL', 'musicbrainz_recording_id': 'REC'} + + +# ── apply handler ── + +def test_fix_library_retag_writes_each_track(tmp_path, monkeypatch): + import core.repair_worker as rw + track = tmp_path / 'a.flac'; track.write_bytes(b'') + + worker = rw.RepairWorker.__new__(rw.RepairWorker) + worker.db = SimpleNamespace() + worker._config_manager = SimpleNamespace(get=lambda k, d=None: d) + worker.transfer_folder = str(tmp_path) + + monkeypatch.setattr(rw, '_resolve_file_path', lambda p, *a, **k: p) + writes = [] + monkeypatch.setattr('core.tag_writer.write_tags_to_file', + lambda fp, db_data, **k: writes.append((fp, db_data)) or {'success': True}) + + details = { + 'tracks': [{'file_path': str(track), 'db_data': {'title': 'Real Title', 'spotify_track_id': 'sp_trk'}}], + 'cover_action': None, 'cover_url': None, + } + res = worker._fix_library_retag('album', '1', None, details) + assert res['success'] is True + assert res['written'] == 1 + assert writes[0][1]['title'] == 'Real Title' + assert writes[0][1]['spotify_track_id'] == 'sp_trk' + + +def test_fix_library_retag_counts_unreachable(tmp_path, monkeypatch): + import core.repair_worker as rw + worker = rw.RepairWorker.__new__(rw.RepairWorker) + worker.db = SimpleNamespace() + worker._config_manager = SimpleNamespace(get=lambda k, d=None: d) + worker.transfer_folder = str(tmp_path) + monkeypatch.setattr(rw, '_resolve_file_path', lambda p, *a, **k: p) + monkeypatch.setattr('core.tag_writer.write_tags_to_file', lambda *a, **k: {'success': True}) + + details = {'tracks': [{'file_path': str(tmp_path / 'missing.flac'), 'db_data': {'title': 'x'}}], + 'cover_action': None, 'cover_url': None} + res = worker._fix_library_retag('album', '1', None, details) + assert res['success'] is False # nothing written (file missing) From 2328e159a19228da5ea542ab6ac5b26e27f87952 Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Thu, 4 Jun 2026 09:51:00 -0700 Subject: [PATCH 07/67] Tools page: fix section nesting + move Metadata Updater into Metadata & Cache MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two fixes: - The retag-tool-card removal accidentally ate the
that closed the Metadata & Cache grid + section, so the Management section nested inside it. Restored the close — Management is a sibling section again. (div balance back to 1998/1998.) - Moved the Metadata Updater card from 'Database & Scanning' into 'Metadata & Cache' where it belongs. --- webui/index.html | 63 ++++++++++++++++++++++++------------------------ 1 file changed, 32 insertions(+), 31 deletions(-) diff --git a/webui/index.html b/webui/index.html index b30f9212..2ed8e0b2 100644 --- a/webui/index.html +++ b/webui/index.html @@ -6516,37 +6516,6 @@
-
-
-

Metadata Updater

- -
-

Updates artist photos, genres, - and album art from Spotify.

-
- - -
-
-

Current Artist: Not - running

-
-
-
-
-

0 / 0 artists (0.0%) -

-
-
-

Quality Scanner

@@ -6667,6 +6636,37 @@

Metadata & Cache

+
+
+

Metadata Updater

+ +
+

Updates artist photos, genres, + and album art from Spotify.

+
+ + +
+
+

Current Artist: Not + running

+
+
+
+
+

0 / 0 artists (0.0%) +

+
+
+
@@ -6707,6 +6707,7 @@
+
From 0a4c3d7dc88f07547f15466e093c652c05311ee1 Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Thu, 4 Jun 2026 10:06:34 -0700 Subject: [PATCH 08/67] Library re-tag: standard dry-run pattern (shows the Dry Run tag, opt-in auto-apply) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The job was the odd one out — auto_fix=False, no dry_run setting, so it never showed the 'Dry Run' badge the other jobs do (the badge keys off settings.dry_run === true). Aligned it to the standard pattern: - auto_fix=True + dry_run setting defaulting True. Default behavior is unchanged (findings only, nothing written) AND it now shows the Dry Run badge. - Turning dry_run off makes the scan auto-apply in place (result.auto_fixed), no finding — the opt-in 'just retag it' mode. - Extracted a shared apply_track_plans() used by both the scan auto-apply and the repair_worker fix handler (handler now resolves Docker paths then delegates — one code path, no duplication). Tests: dry_run=False auto-applies + writes + no finding; existing dry-run finding/skip/apply tests still green. 410 passing. --- core/repair_jobs/library_retag.py | 69 +++++++++++++++++++++++++++++-- core/repair_worker.py | 68 +++++++----------------------- tests/test_library_retag_job.py | 20 +++++++++ 3 files changed, 101 insertions(+), 56 deletions(-) diff --git a/core/repair_jobs/library_retag.py b/core/repair_jobs/library_retag.py index c6f11353..6219e6c0 100644 --- a/core/repair_jobs/library_retag.py +++ b/core/repair_jobs/library_retag.py @@ -45,6 +45,55 @@ def _read_current_tags(file_path): return {} +def apply_track_plans(track_plans, cover_action=None, cover_url=None) -> dict: + """Write each plan's tags in place (+ optionally embed/refresh cover art), + reusing tag_writer.write_tags_to_file. ``file_path`` on each plan must be a + real, reachable path (caller resolves Docker paths). Shared by the dry-run= + False auto-apply and the repair_worker fix handler. Never raises. + """ + import os as _os + result = {'written': 0, 'failed': 0, 'skipped': 0, 'cover_written': False} + embed_cover = bool(cover_action and cover_url) + cover_data = None + if embed_cover: + try: + from core.tag_writer import download_cover_art + cover_data = download_cover_art(cover_url) + except Exception as e: + logger.debug("retag cover download failed: %s", e) + embed_cover = embed_cover and cover_data is not None + + from core.tag_writer import write_tags_to_file + last_dir = None + for tp in track_plans or []: + fp = tp.get('file_path') + db_data = tp.get('db_data') or {} + if not fp or not _os.path.isfile(fp) or (not db_data and not embed_cover): + result['skipped'] += 1 + continue + try: + res = write_tags_to_file(fp, db_data, embed_cover=embed_cover, cover_data=cover_data) + if res.get('success'): + result['written'] += 1 + last_dir = _os.path.dirname(fp) + else: + result['failed'] += 1 + except Exception as e: + logger.warning("retag write failed for %s: %s", fp, e) + result['failed'] += 1 + + if cover_action and cover_data and last_dir: + try: + cover_path = _os.path.join(last_dir, 'cover.jpg') + if cover_action == 'replace' or not _os.path.exists(cover_path): + with open(cover_path, 'wb') as fh: + fh.write(cover_data[0]) + result['cover_written'] = True + except Exception as e: + logger.debug("retag cover.jpg write failed: %s", e) + return result + + def _add_source_ids(db_data, source, album_source_id, source_track): """Stamp the album/track source IDs onto the write payload so the canonical writer embeds them too (Spotify / iTunes / MusicBrainz).""" @@ -95,6 +144,8 @@ class LibraryRetagJob(RepairJob): 'comes from. Each finding lists every tag that would change (old -> new) per ' 'track so you can review before applying — nothing is written until you do.\n\n' 'Settings:\n' + '- Dry run (default ON): only create findings to review; nothing is written. ' + 'Turn it off to auto-apply on scan.\n' '- Mode: "overwrite" rewrites every field the source provides; "fill_missing" ' 'only fills blank tags (keeps your existing values).\n' '- Cover art: replace / fill-missing / skip.\n' @@ -104,6 +155,7 @@ class LibraryRetagJob(RepairJob): default_enabled = False default_interval_hours = 168 default_settings = { + 'dry_run': True, 'mode': MODE_OVERWRITE, 'cover_art': 'replace', 'source': '', @@ -113,7 +165,7 @@ class LibraryRetagJob(RepairJob): 'cover_art': ['replace', 'fill_missing', 'skip'], 'source': ['', 'spotify', 'itunes', 'deezer', 'musicbrainz'], } - auto_fix = False + auto_fix = True def _get_settings(self, context: JobContext) -> dict: merged = dict(self.default_settings) @@ -133,6 +185,7 @@ class LibraryRetagJob(RepairJob): settings = self._get_settings(context) mode = settings.get('mode', MODE_OVERWRITE) cover_mode = settings.get('cover_art', 'replace') + dry_run = settings.get('dry_run', True) source_order = self._source_order(settings) if not source_order: logger.warning("Library re-tag: no usable metadata sources configured") @@ -180,7 +233,7 @@ class LibraryRetagJob(RepairJob): try: self._scan_album(context, result, album_id, album_title, artist_name, - source, album_source_id, mode, cover_mode) + source, album_source_id, mode, cover_mode, dry_run) except Exception as e: logger.debug("Library re-tag: album %s failed: %s", album_id, e) result.errors += 1 @@ -192,7 +245,7 @@ class LibraryRetagJob(RepairJob): return result def _scan_album(self, context, result, album_id, album_title, artist_name, - source, album_source_id, mode, cover_mode): + source, album_source_id, mode, cover_mode, dry_run=True): # Local tracks for this album. with context.db._get_connection() as conn: cur = conn.cursor() @@ -257,6 +310,16 @@ class LibraryRetagJob(RepairJob): result.skipped += 1 return + # Not dry-run: apply the tags in place now (the track paths were already + # isfile-checked above) and count it as an auto-fix — no finding. + if not dry_run: + res = apply_track_plans(track_plans, cover_action, cover_url) + if res['written'] or res['cover_written']: + result.auto_fixed += 1 + else: + result.errors += 1 + return + total_changes = sum(len(tp['changes']) for tp in track_plans) summary_bits = [] if tag_change_tracks: diff --git a/core/repair_worker.py b/core/repair_worker.py index ab5650f6..2ddca93d 100644 --- a/core/repair_worker.py +++ b/core/repair_worker.py @@ -1365,68 +1365,30 @@ class RepairWorker: if not tracks: return {'success': False, 'error': 'No tracks to re-tag in finding'} - cover_action = details.get('cover_action') - cover_url = details.get('cover_url') - embed_cover = bool(cover_action and cover_url) + # Resolve container/host path mismatches, then delegate to the shared + # apply path the job's auto-fix mode also uses. download_folder = self._config_manager.get('soulseek.download_path', '') if self._config_manager else None - - # Download the cover once for the whole album (batch embed). - cover_data = None - if embed_cover: - try: - from core.tag_writer import download_cover_art - cover_data = download_cover_art(cover_url) - except Exception as e: - logger.debug("library_retag: cover download failed: %s", e) - embed_cover = embed_cover and cover_data is not None - - from core.tag_writer import write_tags_to_file - written = failed = skipped = 0 - last_dir = None + resolved_plans = [] for t in tracks: raw = t.get('file_path') - db_data = t.get('db_data') or {} - if not raw or (not db_data and not embed_cover): - skipped += 1 + if not raw: continue - resolved = _resolve_file_path(raw, self.transfer_folder, download_folder, - config_manager=self._config_manager) or raw - if not os.path.isfile(resolved): - skipped += 1 - continue - try: - res = write_tags_to_file(resolved, db_data, embed_cover=embed_cover, cover_data=cover_data) - if res.get('success'): - written += 1 - last_dir = os.path.dirname(resolved) - else: - failed += 1 - except Exception as e: - logger.warning("library_retag write failed for %s: %s", resolved, e) - failed += 1 + rp = _resolve_file_path(raw, self.transfer_folder, download_folder, + config_manager=self._config_manager) or raw + resolved_plans.append({'file_path': rp, 'db_data': t.get('db_data') or {}}) - # Refresh the cover.jpg sidecar to match (replace, or fill when missing). - cover_written = False - if cover_action and cover_data and last_dir: - try: - cover_path = os.path.join(last_dir, 'cover.jpg') - if cover_action == 'replace' or not os.path.exists(cover_path): - with open(cover_path, 'wb') as fh: - fh.write(cover_data[0]) - cover_written = True - except Exception as e: - logger.debug("library_retag: cover.jpg write failed: %s", e) + from core.repair_jobs.library_retag import apply_track_plans + res = apply_track_plans(resolved_plans, details.get('cover_action'), details.get('cover_url')) - if written == 0 and not cover_written: + if res['written'] == 0 and not res['cover_written']: return {'success': False, 'error': 'Nothing could be written — files unreachable or read-only?'} - msg = f'Re-tagged {written} track(s)' - if failed: - msg += f' ({failed} failed)' - if cover_written: + msg = f"Re-tagged {res['written']} track(s)" + if res['failed']: + msg += f" ({res['failed']} failed)" + if res['cover_written']: msg += ' + refreshed cover.jpg' - return {'success': True, 'action': 'library_retag', 'message': msg, - 'written': written, 'failed': failed, 'skipped': skipped} + return {'success': True, 'action': 'library_retag', 'message': msg, **res} def _fix_metadata_gap(self, entity_type, entity_id, file_path, details): """Apply found metadata fields to the track.""" diff --git a/tests/test_library_retag_job.py b/tests/test_library_retag_job.py index 28ab0c22..58a88960 100644 --- a/tests/test_library_retag_job.py +++ b/tests/test_library_retag_job.py @@ -91,6 +91,26 @@ def test_scan_creates_detailed_finding(tmp_path, monkeypatch): assert tp['db_data']['title'] == 'Real Title' +def test_scan_dry_run_off_auto_applies_no_finding(tmp_path, monkeypatch): + """dry_run=False: scan applies in place (auto_fixed) and creates NO finding.""" + 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': 'skip', 'source': 'spotify', 'dry_run': False}) + _patch_source(monkeypatch, { + 'title': 'Old Title', 'album_artist': 'Real Artist', 'album': 'Real Album', + 'year': '2021', 'genre': 'Rock', 'track_number': 1, 'disc_number': 1, + }) + writes = [] + monkeypatch.setattr('core.tag_writer.write_tags_to_file', + lambda fp, db_data, **k: writes.append(db_data) or {'success': True}) + + result = lr.LibraryRetagJob().scan(ctx) + + assert ctx.findings == [] # no finding in apply mode + assert result.auto_fixed == 1 + assert writes and writes[0]['title'] == 'Real Title' # actually wrote + + def test_scan_skips_album_already_correct(tmp_path, monkeypatch): track = tmp_path / 'track.flac'; track.write_bytes(b'') conn = _db_with_album(str(tmp_path / 'm.db'), str(track), current_title='Real Title') From adbdda7b0eeecaaabce5f5b2fa52c026ff84400a Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Thu, 4 Jun 2026 10:21:30 -0700 Subject: [PATCH 09/67] Library Re-tag: add light/full depth setting, default source to active, fix dropdown CSS - depth setting (light = core tags + matched source ids; full = same multi-source enrichment cascade a fresh download gets, run additively via embed_source_ids). Threaded through scan/finding/auto-apply and the repair_worker fix handler. - source now defaults to 'auto' (= your source priority / active source) instead of blank. - give native
+
+ +
Fetch Spotify metadata without API credentials, for when you can't or don't want to connect Spotify. Unofficial & best-effort (it can break if Spotify changes), and it can't search albums by name or access your library — only used when Spotify isn't authenticated.
+
-
Choose the primary source for artist, album, and track metadata. Spotify can only be selected while an active Spotify session exists. Discogs requires a personal token. MusicBrainz is always available but rate-limited to 1 req/sec.
+
Choose the primary source for artist, album, and track metadata. Spotify needs an active session — or enable Spotify Free above. Discogs requires a personal token. MusicBrainz is always available but rate-limited to 1 req/sec.
diff --git a/webui/static/settings.js b/webui/static/settings.js index 4ef1fb1d..94800cde 100644 --- a/webui/static/settings.js +++ b/webui/static/settings.js @@ -64,7 +64,9 @@ function syncMetadataSourceSelection(source) { function _isMetadataSourceSelectable(source) { if (source === 'spotify') { - return _lastStatusPayload?.spotify?.authenticated === true; + // Selectable with real auth OR when Spotify Free (no-creds) is available. + return _lastStatusPayload?.spotify?.authenticated === true + || _lastStatusPayload?.spotify?.metadata_available === true; } if (source === 'discogs') { const token = document.getElementById('discogs-token'); @@ -1045,6 +1047,8 @@ async function loadSettingsData() { // Populate Metadata source setting document.getElementById('metadata-fallback-source').value = settings.metadata?.fallback_source || 'deezer'; + const spotifyFreeToggle = document.getElementById('metadata-spotify-free'); + if (spotifyFreeToggle) spotifyFreeToggle.checked = settings.metadata?.spotify_free === true; // Populate Hydrabase settings const hbConfig = settings.hydrabase || {}; @@ -2842,7 +2846,8 @@ async function saveSettings(quiet = false) { token: document.getElementById('discogs-token').value, }, metadata: { - fallback_source: metadataSource + fallback_source: metadataSource, + spotify_free: document.getElementById('metadata-spotify-free')?.checked === true }, hydrabase: { url: document.getElementById('hydrabase-url').value, diff --git a/webui/static/shared-helpers.js b/webui/static/shared-helpers.js index a55b7027..7eb1d78e 100644 --- a/webui/static/shared-helpers.js +++ b/webui/static/shared-helpers.js @@ -110,6 +110,10 @@ async function fetchSourceConfiguredMap() { for (const src of SOURCE_ORDER) { if (_ALWAYS_CONFIGURED_SOURCES.has(src)) { map[src] = true; + } else if (src === 'spotify') { + // Spotify Free: available without credentials when the + // opt-in no-creds source is on (metadata_available). + map[src] = !!(data[src] && (data[src].configured || data[src].metadata_available)); } else { map[src] = !!(data[src] && data[src].configured); } From a387814deb654032794d36cb8e618a384cf32ec6 Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Fri, 5 Jun 2026 14:00:50 -0700 Subject: [PATCH 54/67] #798: Spotify Free as a rate-limit bridge for connected users (hybrid) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When Spotify Free is enabled, it now also bridges an official rate-limit ban for authenticated users instead of stalling — search already did this (the gate opens on no-auth OR rate-limit); this extends it to the enrichment worker. - spotify_worker: the rate-limit guard now sleeps only when free CAN'T cover (is_spotify_metadata_available() is False). Purely additive — with Spotify Free off, that's False during a ban and the worker sleeps exactly as before. Verified: toggle OFF + rate-limited -> sleeps (original); toggle ON -> bridges. - Reframed the Settings toggle so connected users know it also covers rate-limits ("Use Spotify Free when Spotify is unavailable or rate-limited"). The official auth path is untouched; free never runs while authed Spotify works normally. --- core/spotify_worker.py | 10 ++++++++-- webui/index.html | 4 ++-- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/core/spotify_worker.py b/core/spotify_worker.py index 844b9eb3..f8e8cc3a 100644 --- a/core/spotify_worker.py +++ b/core/spotify_worker.py @@ -186,8 +186,14 @@ class SpotifyWorker: interruptible_sleep(self._stop_event, 1) continue - # Rate limit guard — if globally rate limited, sleep until ban expires - if self.client.is_rate_limited(): + # Rate limit guard — if globally rate limited, sleep until ban + # expires. EXCEPT: when Spotify Free is available it bridges the + # ban (is_spotify_metadata_available() is True via the no-creds + # source, and the client routes there), so we keep enriching + # instead of stalling. Purely additive: with Spotify Free off, + # is_spotify_metadata_available() is False during a ban and this + # sleeps exactly as before. + if self.client.is_rate_limited() and not self.client.is_spotify_metadata_available(): info = self.client.get_rate_limit_info() remaining = info['remaining_seconds'] if info else 60 logger.debug(f"Spotify globally rate limited, sleeping {remaining}s...") diff --git a/webui/index.html b/webui/index.html index d42a28fc..d5920273 100644 --- a/webui/index.html +++ b/webui/index.html @@ -3847,9 +3847,9 @@
-
Fetch Spotify metadata without API credentials, for when you can't or don't want to connect Spotify. Unofficial & best-effort (it can break if Spotify changes), and it can't search albums by name or access your library — only used when Spotify isn't authenticated.
+
Fetches Spotify metadata without API credentials. Works two ways: as a full Spotify source if you haven't connected your account, and as a bridge that keeps search & enrichment going when your connected Spotify hits a rate-limit. Unofficial & best-effort (can break if Spotify changes); it can't search albums by name or access your library, and never runs while your authenticated Spotify is working normally.
Choose the primary source for artist, album, and track metadata. Spotify needs an active session — or enable Spotify Free above. Discogs requires a personal token. MusicBrainz is always available but rate-limited to 1 req/sec.
From 217a5eda70f2d90d2d131a73a2639f280322f7d2 Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Fri, 5 Jun 2026 14:23:28 -0700 Subject: [PATCH 55/67] #798: Spotify Free as a real dropdown source + automatic rate-limit bridge MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Consistency fix: Spotify Free is now its own entry in the metadata-source dropdown (alongside Spotify / iTunes / Deezer / MusicBrainz) instead of a side-toggle. Stored as fallback_source='spotify' + spotify_free=true so all downstream 'spotify' routing and the spotify_* columns are unchanged. Refined gate model (no toggle): - Connected user (has credentials) -> official; bridges to free AUTOMATICALLY during a rate-limit ban (no opt-in needed). - No-auth user -> must pick 'Spotify Free' in the dropdown; then free serves. - Never opted into Spotify (no creds, didn't pick it) -> free never runs, so no surprise scraping. _free_wanted() = has_credentials OR picked-spotify-free is the guard. - AUTHED + healthy -> official always; free never opens. UI: dropdown gains 'Spotify Free (no credentials)' (selectable when the package is installed — surfaced via status.free_installed, since selecting it is the opt-in and can't depend on having selected it); load/save map the dropdown value to the (fallback_source, spotify_free) pair; old checkbox removed. Gate model pinned by 6 scenario tests (connected/healthy, connected/ratelimited bridge, no-auth picked, no-auth not-opted-in, package-missing). 117 tests green. --- core/spotify_client.py | 53 ++++++++++++++++++++--------- tests/test_spotify_free_metadata.py | 53 +++++++++++++++++++++++++++++ web_server.py | 12 +++++-- webui/index.html | 10 ++---- webui/static/settings.js | 34 +++++++++++++----- 5 files changed, 126 insertions(+), 36 deletions(-) diff --git a/core/spotify_client.py b/core/spotify_client.py index 0da306af..f04ca6d2 100644 --- a/core/spotify_client.py +++ b/core/spotify_client.py @@ -569,26 +569,43 @@ class SpotifyClient: return self._itunes # Fall back to iTunes if no Discogs token return self._itunes - def _free_enabled(self) -> bool: - """Opt-in switch for the no-creds SpotipyFree ('Spotify Free') source. - Default OFF — the user explicitly enables it (Settings), so there's no - surprise web-scraping for people who simply haven't connected Spotify.""" + def _free_selected(self) -> bool: + """Whether the user picked 'Spotify Free' as their metadata source + (the no-creds option, for when they haven't connected Spotify).""" try: return bool(config_manager.get('metadata.spotify_free', False)) except Exception: return False - def _free_available(self) -> bool: - """Whether the no-creds fallback can be offered at all: enabled in - config AND the SpotipyFree package is installed.""" + def _has_spotify_credentials(self) -> bool: + """Whether Spotify client credentials are configured (a 'connected' + user, even if currently rate-limit-banned). Drives the auto-bridge.""" + try: + cfg = config_manager.get_spotify_config() or {} + return bool(cfg.get('client_id') and cfg.get('client_secret')) + except Exception: + return False + + def _free_installed(self) -> bool: from core.spotify_free_metadata import spotify_free_installed - return self._free_enabled() and spotify_free_installed() + return spotify_free_installed() + + def _free_wanted(self) -> bool: + """Does the user want Spotify metadata at all? Either they have + credentials (connected → eligible for the rate-limit auto-bridge) or + they explicitly chose 'Spotify Free'. This is what keeps the free source + from auto-scraping for someone who never opted into Spotify.""" + return self._has_spotify_credentials() or self._free_selected() + + def _free_available(self) -> bool: + """Free CAN serve: package installed AND the user wants Spotify.""" + return self._free_installed() and self._free_wanted() def is_spotify_metadata_available(self) -> bool: """Whether SoulSync can serve Spotify metadata — real auth OR the - no-creds fallback. Availability gates (search resolve, enrichment - worker, watchlist) use THIS instead of ``is_spotify_authenticated()`` - so the fallback is reachable. Does NOT change auth semantics.""" + no-creds source. Availability gates (search resolve, enrichment worker, + watchlist) use THIS instead of ``is_spotify_authenticated()`` so the + free source is reachable. Does NOT change auth semantics.""" from core.spotify_free_metadata import should_offer_spotify_metadata try: authed = self.is_spotify_authenticated() @@ -597,13 +614,15 @@ class SpotifyClient: return should_offer_spotify_metadata(authed, self._free_available()) def _free_active(self) -> bool: - """Whether the no-creds SpotipyFree fallback may serve THIS request. + """Whether the no-creds source may serve THIS request: free available + AND official can't (no auth, or rate-limited). ``is_spotify_authenticated()`` + already returns False during a rate-limit ban; the explicit rate-limit + term covers the brief window before the auth cache refreshes. When authed + + healthy the official path returns first, so this never opens. - Only when official Spotify can't (no auth, or rate-limited — note - ``is_spotify_authenticated()`` already returns False during a rate-limit - ban) AND the fallback is available. When authed + healthy the official - path returns before any fallback, so this never opens. Delegates the - pure decision to ``core.spotify_free_metadata.should_use_free_fallback``.""" + Two activations fall out of this: a no-auth user who chose Spotify Free + (free is their source), and a connected user mid-rate-limit (free bridges + the ban) — see _free_wanted().""" from core.spotify_free_metadata import should_use_free_fallback if not self._free_available(): return False diff --git a/tests/test_spotify_free_metadata.py b/tests/test_spotify_free_metadata.py index a69f3355..ccfff97e 100644 --- a/tests/test_spotify_free_metadata.py +++ b/tests/test_spotify_free_metadata.py @@ -133,3 +133,56 @@ def test_normalize_artist_skips_imageless_sources(): 'visuals': {'avatarImage': {'sources': [{'height': 1}, {'url': 'u'}]}}} out = normalize_artist(raw) assert out['images'] == [{'url': 'u', 'height': None, 'width': None}] + + +# --------------------------------------------------------------------------- +# SpotifyClient gate model — auto-bridge vs explicit opt-in vs no-surprise +# (constructs the client via __new__ so no network/config init runs) +# --------------------------------------------------------------------------- + +from unittest.mock import patch # noqa: E402 +from core.spotify_client import SpotifyClient # noqa: E402 +import core.spotify_free_metadata as _sfm # noqa: E402 + + +def _gate(authed, has_creds, selected, installed, rate_limited): + c = SpotifyClient.__new__(SpotifyClient) + with patch.object(SpotifyClient, 'is_spotify_authenticated', return_value=authed), \ + patch('core.spotify_client.config_manager') as cm, \ + patch('core.spotify_client._is_globally_rate_limited', return_value=rate_limited), \ + patch.object(_sfm, 'spotify_free_installed', return_value=installed): + cm.get_spotify_config.return_value = ( + {'client_id': 'x', 'client_secret': 'y'} if has_creds else {}) + cm.get.side_effect = lambda k, d=None: selected if k == 'metadata.spotify_free' else d + return c.is_spotify_metadata_available(), c._free_active() + + +def test_connected_healthy_uses_official(): + avail, free = _gate(authed=True, has_creds=True, selected=False, installed=True, rate_limited=False) + assert avail is True and free is False # official; free never opens + + +def test_connected_ratelimited_bridges_to_free(): + avail, free = _gate(authed=False, has_creds=True, selected=False, installed=True, rate_limited=True) + assert avail is True and free is True # auto-bridge, no toggle needed + + +def test_no_auth_picked_spotify_free_serves(): + avail, free = _gate(authed=False, has_creds=False, selected=True, installed=True, rate_limited=False) + assert avail is True and free is True + + +def test_no_auth_not_opted_in_does_nothing(): + # The key no-surprise guarantee: a user who never chose Spotify gets no free. + avail, free = _gate(authed=False, has_creds=False, selected=False, installed=True, rate_limited=False) + assert avail is False and free is False + + +def test_selected_but_package_missing_is_graceful(): + avail, free = _gate(authed=False, has_creds=False, selected=True, installed=False, rate_limited=False) + assert avail is False and free is False + + +def test_connected_ratelimited_but_no_package_no_bridge(): + avail, free = _gate(authed=False, has_creds=True, selected=False, installed=False, rate_limited=True) + assert avail is False and free is False diff --git a/web_server.py b/web_server.py index 0ecd0b03..410579e9 100644 --- a/web_server.py +++ b/web_server.py @@ -2318,8 +2318,11 @@ def get_status(): if t.get('status') in ('downloading', 'searching', 'post_processing', 'queued', 'pending'): active_dl_count += 1 - # Spotify Free: tell the UI whether Spotify metadata is available even - # without auth (so the Settings source selector can offer it). + # Spotify Free: tell the UI (a) whether Spotify metadata is currently + # available (auth or free), and (b) whether the SpotipyFree package is + # installed — the latter is what makes 'Spotify Free' selectable in the + # source dropdown (selecting it is the opt-in, so it can't depend on + # already having selected it). spotify_status = dict(metadata_status['spotify']) try: spotify_status['metadata_available'] = bool( @@ -2327,6 +2330,11 @@ def get_status(): ) except Exception: spotify_status['metadata_available'] = bool(spotify_status.get('authenticated')) + try: + from core.spotify_free_metadata import spotify_free_installed + spotify_status['free_installed'] = spotify_free_installed() + except Exception: + spotify_status['free_installed'] = False status_data = { 'metadata_source': metadata_status['metadata_source'], diff --git a/webui/index.html b/webui/index.html index d5920273..3092c057 100644 --- a/webui/index.html +++ b/webui/index.html @@ -3838,21 +3838,15 @@
-
- -
Fetches Spotify metadata without API credentials. Works two ways: as a full Spotify source if you haven't connected your account, and as a bridge that keeps search & enrichment going when your connected Spotify hits a rate-limit. Unofficial & best-effort (can break if Spotify changes); it can't search albums by name or access your library, and never runs while your authenticated Spotify is working normally.
-
-
Choose the primary source for artist, album, and track metadata. Spotify needs an active session — or enable Spotify Free above. Discogs requires a personal token. MusicBrainz is always available but rate-limited to 1 req/sec.
+
Choose the primary source for artist, album, and track metadata. Spotify needs a connected account. Spotify Free pulls the same Spotify data without credentials (unofficial & best-effort — can break if Spotify changes, can't search albums by name, no library access). Either way, a connected Spotify that hits a rate-limit is automatically bridged by the free source if it's installed. Discogs requires a personal token. MusicBrainz is always available but rate-limited to 1 req/sec.
diff --git a/webui/static/settings.js b/webui/static/settings.js index 94800cde..1cff59a0 100644 --- a/webui/static/settings.js +++ b/webui/static/settings.js @@ -64,9 +64,13 @@ function syncMetadataSourceSelection(source) { function _isMetadataSourceSelectable(source) { if (source === 'spotify') { - // Selectable with real auth OR when Spotify Free (no-creds) is available. - return _lastStatusPayload?.spotify?.authenticated === true - || _lastStatusPayload?.spotify?.metadata_available === true; + // Official Spotify needs a connected session. + return _lastStatusPayload?.spotify?.authenticated === true; + } + if (source === 'spotify_free') { + // No-creds Spotify only needs the SpotipyFree package installed — + // selecting it IS the opt-in, so it must NOT depend on having selected it. + return _lastStatusPayload?.spotify?.free_installed === true; } if (source === 'discogs') { const token = document.getElementById('discogs-token'); @@ -1045,10 +1049,13 @@ async function loadSettingsData() { // Populate Discogs settings document.getElementById('discogs-token').value = settings.discogs?.token || ''; - // Populate Metadata source setting - document.getElementById('metadata-fallback-source').value = settings.metadata?.fallback_source || 'deezer'; - const spotifyFreeToggle = document.getElementById('metadata-spotify-free'); - if (spotifyFreeToggle) spotifyFreeToggle.checked = settings.metadata?.spotify_free === true; + // Populate Metadata source setting. 'Spotify Free' is stored as + // fallback_source='spotify' + spotify_free=true (so all downstream + // 'spotify' routing is unchanged) — map it back to the dropdown value. + const _fbSrc = settings.metadata?.fallback_source || 'deezer'; + const _metaSel = (_fbSrc === 'spotify' && settings.metadata?.spotify_free === true) + ? 'spotify_free' : _fbSrc; + document.getElementById('metadata-fallback-source').value = _metaSel; // Populate Hydrabase settings const hbConfig = settings.hydrabase || {}; @@ -2759,12 +2766,19 @@ async function saveSettings(quiet = false) { const discogsTokenPresent = !!discogsTokenInput?.value?.trim(); let metadataSource = metadataSourceSelect?.value || 'deezer'; const spotifySessionActive = _lastStatusPayload?.spotify?.authenticated === true; + const spotifyFreeInstalled = _lastStatusPayload?.spotify?.free_installed === true; if (metadataSource === 'spotify' && !spotifySessionActive) { metadataSource = _metadataSourceFallback('spotify'); if (metadataSourceSelect) metadataSourceSelect.value = metadataSource; if (!quiet) { showToast('Spotify is disconnected, so the primary metadata source was switched.', 'warning'); } + } else if (metadataSource === 'spotify_free' && !spotifyFreeInstalled) { + metadataSource = _metadataSourceFallback('spotify_free'); + if (metadataSourceSelect) metadataSourceSelect.value = metadataSource; + if (!quiet) { + showToast('Spotify Free needs the SpotipyFree package installed.', 'warning'); + } } else if (metadataSource === 'discogs' && !discogsTokenPresent) { metadataSource = _metadataSourceFallback('discogs'); if (metadataSourceSelect) metadataSourceSelect.value = metadataSource; @@ -2846,8 +2860,10 @@ async function saveSettings(quiet = false) { token: document.getElementById('discogs-token').value, }, metadata: { - fallback_source: metadataSource, - spotify_free: document.getElementById('metadata-spotify-free')?.checked === true + // 'Spotify Free' is stored as the spotify source + a flag, so all + // downstream 'spotify' routing is unchanged. + fallback_source: metadataSource === 'spotify_free' ? 'spotify' : metadataSource, + spotify_free: metadataSource === 'spotify_free' }, hydrabase: { url: document.getElementById('hydrabase-url').value, From 0c03803a3075367d429716701dbaecbf799f25ea Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Fri, 5 Jun 2026 14:35:49 -0700 Subject: [PATCH 56/67] =?UTF-8?q?#798:=20fix=20Spotify=20Free=20not=20sele?= =?UTF-8?q?ctable=20=E2=80=94=20WebSocket=20status=20push=20missing=20flag?= =?UTF-8?q?s?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Settings dropdown reverted 'Spotify Free' because _isMetadataSourceSelectable reads _lastStatusPayload.spotify.free_installed, but that payload comes from the WebSocket status:update push (_build_status_payload) — which sent the raw spotify dict. The availability flags were only added to the GET /status endpoint, so the frontend never saw free_installed and bounced the selection. Extract _spotify_status_with_availability() (metadata_available + free_installed) and use it in BOTH _build_status_payload (WebSocket) and get_status (HTTP poll), so they can't drift. Now 'Spotify Free' is selectable when SpotipyFree is installed. --- web_server.py | 42 ++++++++++++++++++++++-------------------- 1 file changed, 22 insertions(+), 20 deletions(-) diff --git a/web_server.py b/web_server.py index 410579e9..a02d9273 100644 --- a/web_server.py +++ b/web_server.py @@ -2318,27 +2318,9 @@ def get_status(): if t.get('status') in ('downloading', 'searching', 'post_processing', 'queued', 'pending'): active_dl_count += 1 - # Spotify Free: tell the UI (a) whether Spotify metadata is currently - # available (auth or free), and (b) whether the SpotipyFree package is - # installed — the latter is what makes 'Spotify Free' selectable in the - # source dropdown (selecting it is the opt-in, so it can't depend on - # already having selected it). - spotify_status = dict(metadata_status['spotify']) - try: - spotify_status['metadata_available'] = bool( - spotify_client and spotify_client.is_spotify_metadata_available() - ) - except Exception: - spotify_status['metadata_available'] = bool(spotify_status.get('authenticated')) - try: - from core.spotify_free_metadata import spotify_free_installed - spotify_status['free_installed'] = spotify_free_installed() - except Exception: - spotify_status['free_installed'] = False - status_data = { 'metadata_source': metadata_status['metadata_source'], - 'spotify': spotify_status, + 'spotify': _spotify_status_with_availability(metadata_status['spotify']), 'media_server': _status_cache['media_server'], 'soulseek': soulseek_data, 'active_media_server': active_server, @@ -34451,6 +34433,26 @@ def import_staging_suggestions(): # WEBSOCKET (SOCKET.IO) EVENT HANDLERS AND BACKGROUND EMITTERS # ================================================================================================ +def _spotify_status_with_availability(spotify_status): + """Augment the spotify status dict with the Spotify-Free availability flags + the UI needs: ``metadata_available`` (search picker) and ``free_installed`` + (Settings source selector — 'Spotify Free' is selectable when the package is + installed, since selecting it is the opt-in). Used by BOTH the /status + endpoint and the WebSocket status push so they never drift.""" + out = dict(spotify_status or {}) + try: + out['metadata_available'] = bool( + spotify_client and spotify_client.is_spotify_metadata_available()) + except Exception: + out['metadata_available'] = bool(out.get('authenticated')) + try: + from core.spotify_free_metadata import spotify_free_installed + out['free_installed'] = spotify_free_installed() + except Exception: + out['free_installed'] = False + return out + + def _build_status_payload(): """Build the same status payload used by GET /status, reading from the cache.""" download_mode = config_manager.get('download_source.mode', 'hybrid') @@ -34470,7 +34472,7 @@ def _build_status_payload(): return { 'metadata_source': metadata_status['metadata_source'], - 'spotify': metadata_status['spotify'], + 'spotify': _spotify_status_with_availability(metadata_status['spotify']), 'media_server': _status_cache.get('media_server', {}), 'soulseek': soulseek_data, 'active_media_server': config_manager.get_active_media_server(), From 4249856984e5cb847f5239e46fffacb99047d5df Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Fri, 5 Jun 2026 15:10:18 -0700 Subject: [PATCH 57/67] #798: make Spotify Free opt-in (not auto-bridge) + clearer help text MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per the cleaner model: the free source only runs for users who explicitly picked 'Spotify Free' — not for every connected user. _free_wanted() is now just _free_selected() (dropped the has-credentials auto-trigger). So: - Plain 'Spotify' user, rate-limited -> waits out the ban as before (no surprise background scraping, no ToS exposure for people who never chose free). - 'Spotify Free' user, no auth -> free serves. - 'Spotify Free' user who also connects an account -> official when healthy, free bridges only during a rate-limit, then switches back. Rewrote the metadata-source help text as a plain per-source list with a clear note on how Spotify Free + a connected account interact. Gate tests updated to pin the opt-in behavior (plain-Spotify ratelimit = no bridge; Spotify-Free ratelimit = bridge). --- core/spotify_client.py | 20 ++++++-------------- tests/test_spotify_free_metadata.py | 18 ++++++++++++++++-- webui/index.html | 8 +++++++- 3 files changed, 29 insertions(+), 17 deletions(-) diff --git a/core/spotify_client.py b/core/spotify_client.py index f04ca6d2..bb7af518 100644 --- a/core/spotify_client.py +++ b/core/spotify_client.py @@ -577,25 +577,17 @@ class SpotifyClient: except Exception: return False - def _has_spotify_credentials(self) -> bool: - """Whether Spotify client credentials are configured (a 'connected' - user, even if currently rate-limit-banned). Drives the auto-bridge.""" - try: - cfg = config_manager.get_spotify_config() or {} - return bool(cfg.get('client_id') and cfg.get('client_secret')) - except Exception: - return False - def _free_installed(self) -> bool: from core.spotify_free_metadata import spotify_free_installed return spotify_free_installed() def _free_wanted(self) -> bool: - """Does the user want Spotify metadata at all? Either they have - credentials (connected → eligible for the rate-limit auto-bridge) or - they explicitly chose 'Spotify Free'. This is what keeps the free source - from auto-scraping for someone who never opted into Spotify.""" - return self._has_spotify_credentials() or self._free_selected() + """Does the user actually want the free source? OPT-IN: only when they + picked 'Spotify Free'. This keeps free from auto-scraping for anyone who + didn't choose it — a plain-'Spotify' user just waits out a rate-limit ban + as before. A user on 'Spotify Free' who also connects an account uses the + official account normally and free only bridges their bans.""" + return self._free_selected() def _free_available(self) -> bool: """Free CAN serve: package installed AND the user wants Spotify.""" diff --git a/tests/test_spotify_free_metadata.py b/tests/test_spotify_free_metadata.py index ccfff97e..e09c367d 100644 --- a/tests/test_spotify_free_metadata.py +++ b/tests/test_spotify_free_metadata.py @@ -162,9 +162,23 @@ def test_connected_healthy_uses_official(): assert avail is True and free is False # official; free never opens -def test_connected_ratelimited_bridges_to_free(): +def test_plain_spotify_ratelimited_does_NOT_bridge(): + # OPT-IN: a user on plain 'Spotify' (didn't pick Spotify Free) waits out a + # rate-limit ban — free does NOT auto-bridge for them. No surprise scraping. avail, free = _gate(authed=False, has_creds=True, selected=False, installed=True, rate_limited=True) - assert avail is True and free is True # auto-bridge, no toggle needed + assert avail is False and free is False + + +def test_spotify_free_user_ratelimited_bridges(): + # A user who DID pick Spotify Free and also connected an account: official + # when healthy, free bridges during a ban. + avail, free = _gate(authed=False, has_creds=True, selected=True, installed=True, rate_limited=True) + assert avail is True and free is True + + +def test_spotify_free_user_healthy_uses_official(): + avail, free = _gate(authed=True, has_creds=True, selected=True, installed=True, rate_limited=False) + assert avail is True and free is False # picked Spotify Free but authed -> official def test_no_auth_picked_spotify_free_serves(): diff --git a/webui/index.html b/webui/index.html index 3092c057..99b951cc 100644 --- a/webui/index.html +++ b/webui/index.html @@ -3846,7 +3846,13 @@
-
Choose the primary source for artist, album, and track metadata. Spotify needs a connected account. Spotify Free pulls the same Spotify data without credentials (unofficial & best-effort — can break if Spotify changes, can't search albums by name, no library access). Either way, a connected Spotify that hits a rate-limit is automatically bridged by the free source if it's installed. Discogs requires a personal token. MusicBrainz is always available but rate-limited to 1 req/sec.
+
Where artist, album, and track metadata comes from:
+ • Spotify — official Spotify; connect your account in the Spotify section below.
+ • Spotify Free — the same Spotify catalog with no account needed. It's unofficial and best-effort (may break if Spotify changes its site), can't search albums by name (album results come from iTunes/Deezer instead), and can't read your personal library or playlists.
+ • Deezer / iTunes — free, no account.
+ • MusicBrainz — free, but limited to 1 request/sec.
+ • Discogs — needs a personal access token.

+ Tip: if you pick Spotify Free and later connect a real Spotify account, it uses the official account normally and only falls back to free while Spotify is rate-limited — then switches back automatically.
From 2604704a27b70c96a7e1a64608584b0d2e001317 Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Fri, 5 Jun 2026 16:02:01 -0700 Subject: [PATCH 58/67] #797: stop AcoustID quarantining correct non-English-artist downloads MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit AcoustID returns a recording's title/artist in their ORIGINAL script (e.g. "久石譲" for Joe Hisaishi) while SoulSync's expected metadata is romanized/English. A correct download then fails verification on two walls: the title can never clear the 0.70 similarity bar cross-script, and the only skip path that ignores the title required a near-perfect 0.95 fingerprint plus a resolved alias. Result: every non-English artist trips it. Two complementary fixes, per the reporter's two ideas. Graceful fix (automatic): - New pure core/matching/script_compat.py detects when two strings are in genuinely different writing systems (CJK/Hangul/Cyrillic/Greek/ Arabic/Hebrew/Thai vs Latin). Accented Latin (Beyoncé, Sigur Rós) stays Latin — no false trigger. - acoustid_verification.py: when the EXPECTED artist and the matched artist span scripts AND the artist is confirmed via the existing MusicBrainz alias bridge, SKIP instead of quarantine, without the 0.95 floor (the 0.80 trust floor already gates the fingerprint). - Deliberately narrow: keyed on the ARTIST spanning scripts + being confirmed. A same-script artist with only a cross-script title keeps the stricter 0.95 floor, so the #607 wrong-file protection (Kendrick R.O.T.C, low-fingerprint Japanese-title) is untouched. Per-request toggle (manual escape hatch): - New "Skip AcoustID verification" checkbox in the download-missing modal beside "Force Download All". - skip_acoustid threads request -> batch -> per-track track_info -> download context (same path as _playlist_folder_mode), landing on the existing _skip_quarantine_check='acoustid' bypass. No new mechanism; only the AcoustID gate is bypassed (integrity/bit-depth still run). Tests: - tests/matching/test_script_compat.py — script-boundary cases. - test_acoustid_skip_logic.py — Joe Hisaishi SKIPs at 0.85; unconfirmed cross-script artist still FAILs; same-script low-fingerprint still FAILs. - test_downloads_candidates.py — toggle injects the bypass; absent toggle keeps verification. Full suite: 5169 passed; only pre-existing soundcloud /app env failures remain. Zero regressions. --- core/acoustid_verification.py | 27 +++++- core/downloads/candidates.py | 11 +++ core/downloads/master.py | 16 ++++ core/matching/script_compat.py | 96 ++++++++++++++++++++ tests/downloads/test_downloads_candidates.py | 39 ++++++++ tests/matching/test_script_compat.py | 79 ++++++++++++++++ tests/test_acoustid_skip_logic.py | 74 +++++++++++++++ web_server.py | 5 + webui/static/downloads.js | 6 ++ webui/static/shared-helpers.js | 4 + 10 files changed, 355 insertions(+), 2 deletions(-) create mode 100644 core/matching/script_compat.py create mode 100644 tests/matching/test_script_compat.py diff --git a/core/acoustid_verification.py b/core/acoustid_verification.py index 24930ec0..42cc5500 100644 --- a/core/acoustid_verification.py +++ b/core/acoustid_verification.py @@ -17,6 +17,7 @@ from utils.logging_config import get_logger from core.acoustid_client import AcoustIDClient from core.matching_engine import MusicMatchingEngine from core.matching.version_mismatch import is_acceptable_version_mismatch +from core.matching.script_compat import is_cross_script_mismatch from core.musicbrainz_client import MusicBrainzClient logger = get_logger("acoustid.verification") @@ -655,10 +656,32 @@ class AcoustIDVerification: and title_sim >= 0.80 and artist_sim >= ARTIST_MATCH_THRESHOLD ) - if language_script_skip or high_confidence_strong_match_skip: + # Issue #797 — the EXPECTED artist and the AcoustID-matched + # artist are written in different scripts (e.g. "Joe Hisaishi" + # vs "久石譲") yet the alias-aware comparison still confirmed + # them as the same artist (artist_sim >= threshold, bridged via + # MusicBrainz aliases). When the artist itself spans scripts the + # title almost always does too — and a romanized-vs-native title + # comparison is meaningless, so it can't be evidence the file is + # wrong. Trust the confirmed artist + the fingerprint (already + # >= MIN_ACOUSTID_SCORE to reach here) and SKIP rather than + # quarantine a correct download of a non-English artist. + # + # Deliberately narrow (the "tight" scope): keyed on the ARTIST + # spanning scripts AND being confirmed. A same-script artist + # with only a cross-script TITLE (romaji artist + kanji title) + # is NOT covered — that case keeps the stricter 0.95 floor + # above, preserving the #607 wrong-file protection. + cross_script_artist_skip = ( + best_score >= MIN_ACOUSTID_SCORE + and artist_sim >= ARTIST_MATCH_THRESHOLD + and is_cross_script_mismatch(expected_artist_name, display_artist) + ) + if (language_script_skip or high_confidence_strong_match_skip + or cross_script_artist_skip): reason = ( "likely same song in different language/script" - if language_script_skip + if (language_script_skip or cross_script_artist_skip) else "title/artist match within tolerance" ) msg = ( diff --git a/core/downloads/candidates.py b/core/downloads/candidates.py index 9510a0f5..ddce91fe 100644 --- a/core/downloads/candidates.py +++ b/core/downloads/candidates.py @@ -326,6 +326,17 @@ def attempt_download_with_candidates(task_id, candidates, track, batch_id=None, "task=%s username=%s filename=%s", task_id, username, os.path.basename(filename), ) + elif track_info and track_info.get('_skip_acoustid'): + # Issue #797 — the album-download request had the + # per-request "Skip AcoustID verification" toggle on. + # Bypass only the AcoustID gate (same as a manual + # pick); integrity + bit-depth still run. + matched_downloads_context[context_key]['_skip_quarantine_check'] = 'acoustid' + logger.info( + "[Context] Skip-AcoustID toggle — bypassing AcoustID for " + "task=%s filename=%s", + task_id, os.path.basename(filename), + ) logger.info(f"[Context] Set is_album_download: {is_album_context} (has clean data: {has_clean_spotify_data})") logger.debug(f"[Debug] Context creation - track_info: {track_info is not None}, playlist_folder_mode: {track_info.get('_playlist_folder_mode', False) if track_info else False}") diff --git a/core/downloads/master.py b/core/downloads/master.py index 3769dca3..dfcb4eab 100644 --- a/core/downloads/master.py +++ b/core/downloads/master.py @@ -347,6 +347,13 @@ def run_full_missing_tracks_process(batch_id, playlist_id, tracks_json, deps: Ma batch_playlist_name = 'Unknown Playlist' batch_playlist_id = playlist_id batch_source_playlist_ref = '' + # Issue #797 — per-request "Skip AcoustID verification" toggle from + # the album-download modal. When set, every track in this batch + # bypasses the AcoustID quarantine gate (the user has chosen to + # trust the metadata over fingerprint disagreement — useful for + # non-English artists whose native-script metadata AcoustID can't + # reconcile with the romanized request). + batch_skip_acoustid = False with tasks_lock: if batch_id in download_batches: force_download_all = download_batches[batch_id].get('force_download_all', False) @@ -362,6 +369,7 @@ def run_full_missing_tracks_process(batch_id, playlist_id, tracks_json, deps: Ma batch_source_playlist_ref = ( download_batches[batch_id].get('source_playlist_ref') or '' ).strip() + batch_skip_acoustid = bool(download_batches[batch_id].get('skip_acoustid', False)) from core.downloads.playlist_folder import ( resolve_playlist_folder_mode_for_batch, @@ -1031,6 +1039,14 @@ def run_full_missing_tracks_process(batch_id, playlist_id, tracks_json, deps: Ma logger.info(f"[Wishlist] Added album context for: '{track_info.get('name')}' -> '{album_ctx['name']}'") + # Issue #797 — propagate the batch-level "skip AcoustID" + # toggle onto each track so the per-track download context + # (built in core/downloads/candidates.py) can set the + # AcoustID quarantine bypass. Mirrors the _playlist_folder_mode + # threading pattern below. + if batch_skip_acoustid: + track_info['_skip_acoustid'] = True + # Add playlist folder mode flag for sync page playlists and wishlist # tracks tied to a mirrored playlist with organize_by_playlist enabled. task_pl_folder_mode = batch_playlist_folder_mode diff --git a/core/matching/script_compat.py b/core/matching/script_compat.py new file mode 100644 index 00000000..88b1cb96 --- /dev/null +++ b/core/matching/script_compat.py @@ -0,0 +1,96 @@ +"""Writing-system (script) compatibility helpers for metadata comparison. + +Issue #797 — AcoustID returns a recording's title/artist in their +*original* script (e.g. ``久石譲`` for Joe Hisaishi) while SoulSync's +expected metadata is romanized / English (``Joe Hisaishi``). A raw +string-similarity comparison between two different writing systems +scores ~0 even when they name the very same artist, so correct +downloads of non-English artists get false-quarantined. + +These pure helpers let callers DETECT that situation — "one side is +written in a non-Latin script, the other in Latin" — so the comparison +logic can stop treating an untranslatable title/artist as evidence the +file is wrong. + +Deliberately conservative: a single accented Latin character (``é``, +``ñ``, ``ü``) is still Latin, NOT a script mismatch. Only genuinely +different writing systems (CJK, Hangul, Cyrillic, Greek, Arabic, +Hebrew, Thai, …) count as "non-Latin". +""" + +from __future__ import annotations + +# Unicode ranges for non-Latin writing systems we treat as a "hard" +# script difference. Latin (incl. Latin-1 Supplement / Extended with +# diacritics) is intentionally absent — accented Latin is still Latin. +# CJK ranges mirror core.matching_engine's issue #722 detection so the +# two stay consistent. +_NONLATIN_RANGES = ( + ('Ͱ', 'Ͽ'), # Greek and Coptic + ('Ѐ', 'ӿ'), # Cyrillic + ('Ԁ', 'ԯ'), # Cyrillic Supplement + ('֐', '׿'), # Hebrew + ('؀', 'ۿ'), # Arabic + ('ݐ', 'ݿ'), # Arabic Supplement + ('฀', '๿'), # Thai + ('⺀', '⻿'), # CJK Radicals Supplement + ('぀', 'ゟ'), # Hiragana + ('゠', 'ヿ'), # Katakana + ('㐀', '䶿'), # CJK Unified Ideographs Extension A + ('一', '鿿'), # CJK Unified Ideographs + ('가', '힯'), # Hangul Syllables + ('豈', '﫿'), # CJK Compatibility Ideographs + ('ｦ', 'ￜ'), # Halfwidth Katakana / Hangul +) + + +def _is_nonlatin_char(c: str) -> bool: + """True when ``c`` belongs to a non-Latin writing system.""" + for lo, hi in _NONLATIN_RANGES: + if lo <= c <= hi: + return True + return False + + +def has_strong_nonlatin(text: str) -> bool: + """True when ``text`` contains at least one non-Latin-script letter. + + Accented Latin (``Beyoncé``, ``Sigur Rós``, ``Mötley Crüe``) returns + False — those are Latin. ``久石譲``, ``Дмитрий``, ``방탄소년단`` return True. + """ + if not text: + return False + return any(_is_nonlatin_char(c) for c in text) + + +def _has_latin_letter(text: str) -> bool: + """True when ``text`` contains an ASCII A–Z / a–z letter.""" + if not text: + return False + return any(('a' <= c <= 'z') or ('A' <= c <= 'Z') for c in text) + + +def is_cross_script_mismatch(a: str, b: str) -> bool: + """True when ``a`` and ``b`` are written in different scripts. + + Specifically: exactly one side uses a non-Latin writing system while + the other is genuine Latin text. This is the signal that a raw + similarity score between the two is meaningless (a romanized name vs + its native-script form), NOT that they name different things. + + Symmetric. Returns False when: + - both sides are Latin (ordinary English-vs-English comparison), + - both sides are non-Latin (same-script comparison still works), + - either side is empty / has no comparable letters. + """ + a_nonlatin = has_strong_nonlatin(a) + b_nonlatin = has_strong_nonlatin(b) + if a_nonlatin == b_nonlatin: + # Same script class on both sides (or neither has non-Latin) — + # similarity comparison is meaningful, no script bridge needed. + return False + # Exactly one side is non-Latin. It's only a true cross-script case + # if the OTHER side is real Latin text (not punctuation / digits). + if a_nonlatin: + return _has_latin_letter(b) + return _has_latin_letter(a) diff --git a/tests/downloads/test_downloads_candidates.py b/tests/downloads/test_downloads_candidates.py index 95ed1328..8a80a3ab 100644 --- a/tests/downloads/test_downloads_candidates.py +++ b/tests/downloads/test_downloads_candidates.py @@ -461,6 +461,45 @@ def test_auto_search_pick_does_not_inject_acoustid_bypass(): assert "_user_manual_pick" not in ctx +def test_skip_acoustid_track_flag_injects_bypass(): + """Issue #797: when the album-download request had the per-request + 'Skip AcoustID verification' toggle on, the master worker stamps + `_skip_acoustid=True` onto each track's track_info. The candidates + helper must propagate that into the post-process context as + `_skip_quarantine_check='acoustid'` so AcoustID never quarantines + this request's files (e.g. correct downloads of non-English artists + whose native-script metadata AcoustID can't reconcile).""" + deps = _build_deps() + _seed_task("t_skip_aid", track_info={"_skip_acoustid": True}) + + candidates = [_Candidate(filename="skip.flac", confidence=0.99)] + track = _Track() + + result = dc.attempt_download_with_candidates("t_skip_aid", candidates, track, batch_id="b1", deps=deps) + + assert result is True + ctx = matched_downloads_context["user1::skip.flac"] + assert ctx["_skip_quarantine_check"] == "acoustid" + # It's the toggle path, NOT a manual pick. + assert "_user_manual_pick" not in ctx + + +def test_no_skip_acoustid_flag_keeps_verification(): + """Without the toggle (no `_skip_acoustid` on track_info), AcoustID + verification must still run — the bypass is opt-in per request.""" + deps = _build_deps() + _seed_task("t_no_skip_aid", track_info={}) # no _skip_acoustid + + candidates = [_Candidate(filename="verify.flac", confidence=0.99)] + track = _Track() + + result = dc.attempt_download_with_candidates("t_no_skip_aid", candidates, track, batch_id="b1", deps=deps) + + assert result is True + ctx = matched_downloads_context["user1::verify.flac"] + assert "_skip_quarantine_check" not in ctx + + def test_equal_confidence_candidates_prefer_better_peer_quality(): """Equal-confidence Soulseek candidates use peer quality as the tiebreaker.""" deps = _build_deps() diff --git a/tests/matching/test_script_compat.py b/tests/matching/test_script_compat.py new file mode 100644 index 00000000..644d16b5 --- /dev/null +++ b/tests/matching/test_script_compat.py @@ -0,0 +1,79 @@ +"""Tests for core/matching/script_compat.py — writing-system detection. + +Issue #797 — these pin the exact boundary that the AcoustID verifier +relies on: accented Latin is still Latin (no false cross-script +trigger), but genuinely different writing systems (CJK / Hangul / +Cyrillic / Greek / Arabic / Hebrew / Thai) ARE flagged so a +romanized-vs-native artist comparison isn't treated as a real mismatch. +""" + +from __future__ import annotations + +import pytest + +from core.matching.script_compat import ( + has_strong_nonlatin, + is_cross_script_mismatch, +) + + +# --------------------------------------------------------------------------- +# has_strong_nonlatin — accented Latin must NOT count +# --------------------------------------------------------------------------- + +@pytest.mark.parametrize('text', [ + 'Beyoncé', 'Sigur Rós', 'Mötley Crüe', 'Joe Hisaishi', + 'Kendrick Lamar', 'Dmitry Yablonsky', 'AC/DC', 'P!nk', + '', ' ', '12345', 'Café del Mar', +]) +def test_latin_and_accented_latin_is_not_nonlatin(text): + assert has_strong_nonlatin(text) is False + + +@pytest.mark.parametrize('text', [ + '久石譲', # kanji (Joe Hisaishi) + '残酷な天使のテーゼ', # kana + kanji + 'Дмитрий Яблонский', # Cyrillic + '방탄소년단', # Hangul (BTS) + 'Σωκράτης', # Greek + 'عمرو دياب', # Arabic + 'שלום', # Hebrew + 'ก้อง สหรัถ', # Thai +]) +def test_real_nonlatin_scripts_detected(text): + assert has_strong_nonlatin(text) is True + + +# --------------------------------------------------------------------------- +# is_cross_script_mismatch — the verifier's gate +# --------------------------------------------------------------------------- + +def test_romanized_vs_native_is_cross_script(): + # The reported case (#797): expected romanized, AcoustID native. + assert is_cross_script_mismatch('Joe Hisaishi', '久石譲') is True + assert is_cross_script_mismatch('Dmitry Yablonsky', 'Дмитрий Яблонский') is True + + +def test_is_symmetric(): + assert is_cross_script_mismatch('久石譲', 'Joe Hisaishi') is True + + +def test_same_latin_both_sides_is_not_mismatch(): + # English-vs-English — comparison is meaningful, no bridge. This is + # the Kendrick R.O.T.C protection surface: must stay False so the + # verifier keeps its strict FAIL path. + assert is_cross_script_mismatch('Kendrick Lamar', 'Kendrick Lamar feat. BJ') is False + assert is_cross_script_mismatch('Crown', 'Crown of Thorns') is False + + +def test_same_nonlatin_both_sides_is_not_mismatch(): + # Both native — same-script similarity still works, don't relax. + assert is_cross_script_mismatch('久石譲', '久石譲') is False + assert is_cross_script_mismatch('久石譲', '坂本龍一') is False + + +def test_empty_or_letterless_other_side_is_not_mismatch(): + # One side non-Latin but the other has no Latin LETTER to bridge to. + assert is_cross_script_mismatch('', '久石譲') is False + assert is_cross_script_mismatch('12345', '久石譲') is False + assert is_cross_script_mismatch('久石譲', ' ') is False diff --git a/tests/test_acoustid_skip_logic.py b/tests/test_acoustid_skip_logic.py index 84c92017..0d820d1b 100644 --- a/tests/test_acoustid_skip_logic.py +++ b/tests/test_acoustid_skip_logic.py @@ -185,6 +185,80 @@ def test_high_score_but_artist_mismatch_no_longer_skipped(verifier): assert result == VerificationResult.FAIL +def test_low_fingerprint_score_never_skipped_same_script_artist(verifier): + """#797 guard — the #607 protection for a SAME-SCRIPT artist (Latin + 'Yoko Takahashi' on both sides) with only a cross-script TITLE must + stay FAIL below the 0.95 floor. The #797 relaxation is keyed on the + ARTIST spanning scripts, which this case is NOT, so nothing changes + here. (Duplicates test_low_fingerprint_score_never_skipped's intent + explicitly against the new code path.)""" + _stub_lookup(verifier, recordings=[ + {'title': '残酷な天使のテーゼ', 'artist': 'Yoko Takahashi'}, + ], best_score=0.85) + + result, _msg = verifier.verify_audio_file( + '/fake/path.flac', + 'Zankoku na Tenshi no Theze', + 'Yoko Takahashi', + ) + assert result == VerificationResult.FAIL + + +# --------------------------------------------------------------------------- +# Issue #797 — non-English ARTIST whose name spans scripts +# --------------------------------------------------------------------------- + + +def test_cross_script_artist_confirmed_via_alias_skips_below_095(verifier): + """#797 headline: requested 'Joe Hisaishi' (romanized), AcoustID + returns the recording with the artist/title in their native kanji + ('久石譲'). The alias bridge confirms 久石譲 IS Joe Hisaishi, the + fingerprint is solid (0.85, above the 0.80 trust floor) but below + the old 0.95 language/script bar. Pre-#797 this FAILed and the + correct file was quarantined. Now it SKIPs.""" + with patch( + 'core.acoustid_verification._resolve_expected_artist_aliases', + return_value=['久石譲'], + ): + _stub_lookup(verifier, recordings=[ + {'title': '風のとおり道', 'artist': '久石譲'}, + ], best_score=0.85) + + result, msg = verifier.verify_audio_file( + '/fake/path.flac', + 'The Path of the Wind', + 'Joe Hisaishi', + ) + assert result == VerificationResult.SKIP + assert 'language/script' in msg.lower() + + +def test_cross_script_artist_NOT_confirmed_still_fails(verifier): + """#797 tight scope: if the alias bridge can't confirm the artist + (lookup returns nothing), we have no positive evidence the kanji + artist IS the expected one — so the #797 relaxation must NOT fire. + The relaxation only rescues a CONFIRMED cross-script artist. + + Constructed so best_rec is set (partial title overlap → non-zero + combined score) and the title stays under the strict threshold, so + the flow reaches the same skip-decision point the rescue lives at — + proving it doesn't fire without a confirmed artist.""" + with patch( + 'core.acoustid_verification._resolve_expected_artist_aliases', + return_value=[], + ): + _stub_lookup(verifier, recordings=[ + {'title': 'Summer Night', 'artist': '久石譲'}, + ], best_score=0.85) + + result, _msg = verifier.verify_audio_file( + '/fake/path.flac', + 'Summer', + 'Joe Hisaishi', + ) + assert result == VerificationResult.FAIL + + def test_old_loose_threshold_no_longer_fires_for_unrelated_titles(verifier): """Pin the negative case for the old loose threshold (title_sim >= 0.55). 'Crown' vs 'Crown of Thorns' had similarity around 0.6 diff --git a/web_server.py b/web_server.py index a02d9273..2978693f 100644 --- a/web_server.py +++ b/web_server.py @@ -18707,6 +18707,10 @@ def start_missing_tracks_process(playlist_id): is_album_download = data.get('is_album_download', False) album_context = data.get('album_context', None) artist_context = data.get('artist_context', None) + # Issue #797 — per-request "Skip AcoustID verification" toggle from the + # album-download modal. Stored on the batch and propagated per-track by + # the master worker so AcoustID never quarantines this request's files. + skip_acoustid = bool(data.get('skip_acoustid', False)) if not tracks: return jsonify({"success": False, "error": "No tracks provided"}), 400 @@ -18774,6 +18778,7 @@ def start_missing_tracks_process(playlist_id): 'album_context': album_context, 'artist_context': artist_context, 'wing_it': wing_it, + 'skip_acoustid': skip_acoustid, # #797 per-request AcoustID bypass 'batch_source': _downloads_history.detect_sync_source(playlist_id), } diff --git a/webui/static/downloads.js b/webui/static/downloads.js index d164f615..85c0f944 100644 --- a/webui/static/downloads.js +++ b/webui/static/downloads.js @@ -2451,6 +2451,11 @@ async function startMissingTracksProcess(playlistId) { const forceDownloadCheckbox = document.getElementById(`force-download-all-${playlistId}`); const forceDownloadAll = forceDownloadCheckbox ? forceDownloadCheckbox.checked : false; + // Issue #797 — per-request "Skip AcoustID verification" toggle. Absent + // checkbox (other call sites) → false, so behavior is unchanged there. + const skipAcoustidCheckbox = document.getElementById(`skip-acoustid-${playlistId}`); + const skipAcoustid = skipAcoustidCheckbox ? skipAcoustidCheckbox.checked : false; + // Check if playlist folder mode toggle is enabled (only for sync page playlists) const playlistFolderMode = typeof isPlaylistOrganizeEnabled === 'function' ? isPlaylistOrganizeEnabled(playlistId) @@ -2495,6 +2500,7 @@ async function startMissingTracksProcess(playlistId) { force_download_all: forceDownloadAll || isWingIt, ignore_manual_matches: forceDownloadAll, wing_it: isWingIt, + skip_acoustid: skipAcoustid, }; // If this is an artist album download, use album name and include full context diff --git a/webui/static/shared-helpers.js b/webui/static/shared-helpers.js index 7eb1d78e..5e1c1e62 100644 --- a/webui/static/shared-helpers.js +++ b/webui/static/shared-helpers.js @@ -1483,6 +1483,10 @@ async function openDownloadMissingModalForArtistAlbum(virtualPlaylistId, playlis Force Download All + ${contextType === 'playlist' ? `