From 0df1d55ed6f9e1c7c121ec6955241987c5e07422 Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Mon, 22 Jun 2026 08:47:24 -0700 Subject: [PATCH 01/23] Sync: re-resolve a manual match against live Plex when its stored key went stale MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit wolf39us: a Find & Add manual match got dropped on auto-sync (60->57) with a 404 fetching the stored Plex ratingKey. Root cause: Plex re-keys tracks on a metadata refresh, and the SoulSync DB id IS that ratingKey — so until a SoulSync rescan BOTH the durable match's library_track_id AND the file-path self-heal (which reads the same DB) land on the dead key, fetchItem 404s, the track falls through to fuzzy (also the dead key) and is dropped. Fix: when both DB-side lookups miss on Plex, re-resolve the manual match against LIVE Plex by the matched track's metadata, disambiguated by the stored file path so the user's EXACT chosen track wins among multiple versions; return the current live track and heal the stored id + sync cache. A manual match is now never dropped or silently re-matched — it 'always overrides' as asked. Scoped to Plex (DB-backed servers materialize off the DB and don't have this re-key problem). Seam tests: file-path picks the right version over a live alternate, basename fallback for server-vs-local paths, no-file-match falls back to top hit (never drop), no results/empty title -> None, and a broken client never raises. --- services/sync_service.py | 87 +++++++++++++++++++- tests/sync/test_reresolve_live_plex.py | 108 +++++++++++++++++++++++++ 2 files changed, 193 insertions(+), 2 deletions(-) create mode 100644 tests/sync/test_reresolve_live_plex.py diff --git a/services/sync_service.py b/services/sync_service.py index 172dbd5d..c1213f1a 100644 --- a/services/sync_service.py +++ b/services/sync_service.py @@ -28,6 +28,71 @@ def _artist_name(artist) -> str: return name return str(artist) if artist is not None else '' + +def _plex_track_file(plex_track) -> str: + """Best-effort file path of a live Plex track object (media[0].parts[0].file).""" + try: + return plex_track.media[0].parts[0].file or '' + except Exception: + return '' + + +def reresolve_manual_match_live_plex(cache_db, media_client, m, *, profile_id, + source_track_id, server_source): + """Re-resolve a manual match whose stored Plex ratingKey went stale. + + Plex re-keys tracks on a metadata refresh/optimize, and the SoulSync DB id is + itself that old ratingKey — so until a SoulSync rescan, BOTH the stored + ``library_track_id`` and the file-path self-heal land on the same dead key and + ``fetchItem`` 404s. The only live source of truth is Plex, so search it by the + matched track's metadata, disambiguate by the stored file path (so the user's + EXACT chosen track wins when several versions exist), heal the stored id, and + return the current live Plex track. Best-effort: returns the live track or + None, never raises — a manual match must never be dropped on a transient miss. + """ + try: + title = (m.get('source_title') or '').strip() + artist = (m.get('source_artist') or '').strip() + file_path = (m.get('library_file_path') or '').strip() + if not title: + return None + results = media_client.search_tracks(title, artist, limit=15) or [] + if not results: + return None + + import os as _os + want_base = _os.path.basename(file_path.replace('\\', '/')) if file_path else '' + chosen = None + if want_base: + for t in results: + live = getattr(t, '_original_plex_track', None) + p = _plex_track_file(live) if live is not None else '' + if p and _os.path.basename(p.replace('\\', '/')) == want_base: + chosen = t + break + if chosen is None: + chosen = results[0] # search_tracks already ranks artist→title; best effort + live = getattr(chosen, '_original_plex_track', None) + if live is None or not hasattr(live, 'ratingKey'): + return None + + # Heal the stored id (+ the cache, done by the caller) so the next sync is + # fast and doesn't repeat the live search. + try: + cache_db.save_manual_library_match( + profile_id, m.get('source') or 'spotify', str(source_track_id), str(live.ratingKey), + source_title=m.get('source_title'), source_artist=m.get('source_artist'), + source_album=m.get('source_album'), + server_source=server_source, + library_file_path=file_path or _plex_track_file(live), + ) + except Exception as _heal_err: + logger.debug("manual-match heal (save) failed: %s", _heal_err) + return live + except Exception: + return None + + @dataclass class SyncResult: playlist_name: str @@ -643,15 +708,33 @@ class PlaylistSyncService: # Self-heals a stale library id via the stored file path. try: from core.artists.map import get_current_profile_id + _profile_id = get_current_profile_id() m = cache_db.find_manual_library_match_by_source_track_id( - get_current_profile_id(), str(spotify_id), active_server) + _profile_id, str(spotify_id), active_server) if m: actual_track = _materialize(m.get('library_track_id')) if not actual_track and m.get('library_file_path'): new_id = cache_db.find_track_id_by_file_path(m['library_file_path']) actual_track = _materialize(new_id) + # Plex re-keys tracks on a metadata refresh, and the SoulSync + # DB id IS that ratingKey — so both lookups above can land on + # the same stale key and 404. Re-resolve against LIVE Plex by + # the matched track's metadata so a manual match is NEVER + # dropped or silently re-matched by the fuzzy path below. + if not actual_track and server_type == "plex": + actual_track = reresolve_manual_match_live_plex( + cache_db, media_client, m, + profile_id=_profile_id, source_track_id=spotify_id, + server_source=active_server) + if actual_track and spotify_id: + try: + cache_db.save_sync_match_cache( + spotify_id, original_title, _artist_name(spotify_track.artists[0]) if spotify_track.artists else '', + active_server, actual_track.ratingKey, getattr(actual_track, 'title', original_title), 1.0) + except Exception as _cache_err: + logger.debug("sync cache heal failed: %s", _cache_err) if actual_track: - logger.debug(f"Durable manual match hit: '{original_title}' → {m.get('library_track_id')}") + logger.info(f"Durable manual match honored for '{original_title}' → {getattr(actual_track, 'ratingKey', m.get('library_track_id'))}") return actual_track, 1.0 except Exception as durable_err: logger.debug(f"Durable manual match lookup error: {durable_err}") diff --git a/tests/sync/test_reresolve_live_plex.py b/tests/sync/test_reresolve_live_plex.py new file mode 100644 index 00000000..5de3bf49 --- /dev/null +++ b/tests/sync/test_reresolve_live_plex.py @@ -0,0 +1,108 @@ +"""Plex re-keys tracks on a metadata refresh, so a durable manual match's stored +ratingKey (and the SoulSync DB id, which IS that ratingKey) can both go stale at +once — every DB-side lookup lands on the same dead key and fetchItem 404s, so the +manually-matched track gets dropped on sync (wolf39us). The fix re-resolves the +match against LIVE Plex by the matched track's metadata, disambiguated by the +stored file path so the user's EXACT chosen track wins, and heals the stored id. +""" + +from __future__ import annotations + +from services.sync_service import reresolve_manual_match_live_plex + + +class _PlexTrack: + def __init__(self, rating_key, file): + self.ratingKey = rating_key + self.title = f"track-{rating_key}" + + class _Part: + def __init__(self, f): self.file = f + class _Media: + def __init__(self, f): self.parts = [_Part(f)] + self.media = [_Media(file)] + + +class _TrackInfo: + def __init__(self, plex_track): + self._original_plex_track = plex_track + + +class _MediaClient: + def __init__(self, results): + self._results = results + self.calls = [] + def search_tracks(self, title, artist, limit=15): + self.calls.append((title, artist)) + return self._results + + +class _CacheDb: + def __init__(self): + self.healed = [] + def save_manual_library_match(self, profile_id, source, source_track_id, library_track_id, **meta): + self.healed.append({"id": library_track_id, "source_track_id": source_track_id, **meta}) + return True + + +_MATCH = { + "source": "spotify", "source_title": "It's the End of the World", + "source_artist": "R.E.M.", "source_album": "Document", + "library_file_path": "/music/REM/Document/05 - Its the End.flac", + "library_track_id": 39161, # stale +} + + +def test_picks_the_track_matching_the_stored_file_path(): + # Two live candidates (a different version + the real one); the stored file + # path must select the user's exact track, with its CURRENT ratingKey. + wrong = _TrackInfo(_PlexTrack(50001, "/music/REM/Live/05 - Its the End (Live).flac")) + right = _TrackInfo(_PlexTrack(39167, "/music/REM/Document/05 - Its the End.flac")) + mc = _MediaClient([wrong, right]) + db = _CacheDb() + live = reresolve_manual_match_live_plex( + db, mc, _MATCH, profile_id=1, source_track_id="sp1", server_source="plex") + assert live is not None and live.ratingKey == 39167 # current key, not stale 39161 + assert mc.calls == [("It's the End of the World", "R.E.M.")] + # healed the stored id to the fresh ratingKey + assert db.healed and db.healed[0]["id"] == "39167" + assert db.healed[0]["source_track_id"] == "sp1" + + +def test_basename_match_handles_server_vs_local_path(): + # stored path is a local path; the Plex part.file is a container path — same basename. + m = dict(_MATCH, library_file_path="D:\\Music\\REM\\05 - Its the End.flac") + right = _TrackInfo(_PlexTrack(39167, "/data/Music/REM/05 - Its the End.flac")) + live = reresolve_manual_match_live_plex( + _CacheDb(), _MediaClient([right]), m, profile_id=1, source_track_id="sp1", server_source="plex") + assert live.ratingKey == 39167 + + +def test_no_file_match_falls_back_to_top_result(): + only = _TrackInfo(_PlexTrack(40000, "/music/somewhere/else.flac")) + live = reresolve_manual_match_live_plex( + _CacheDb(), _MediaClient([only]), _MATCH, profile_id=1, source_track_id="sp1", server_source="plex") + assert live.ratingKey == 40000 # never drop a manual match — best-effort top hit + + +def test_no_results_returns_none_and_does_not_heal(): + db = _CacheDb() + assert reresolve_manual_match_live_plex( + db, _MediaClient([]), _MATCH, profile_id=1, source_track_id="sp1", server_source="plex") is None + assert db.healed == [] + + +def test_missing_title_returns_none(): + m = dict(_MATCH, source_title="") + assert reresolve_manual_match_live_plex( + _CacheDb(), _MediaClient([_TrackInfo(_PlexTrack(1, "/x.flac"))]), m, + profile_id=1, source_track_id="sp1", server_source="plex") is None + + +def test_never_raises_on_a_broken_media_client(): + class _Boom: + def search_tracks(self, *a, **k): + raise RuntimeError("plex down") + # a transient Plex error must not bubble — the caller falls through, not crash. + assert reresolve_manual_match_live_plex( + _CacheDb(), _Boom(), _MATCH, profile_id=1, source_track_id="sp1", server_source="plex") is None From 6e622d30f12054f76309d20a3841519703130d39 Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Mon, 22 Jun 2026 08:53:24 -0700 Subject: [PATCH 02/23] #901: give file-import playlist tracks a stable id so manual matches stick MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A Find & Add on a file-import (CSV/M3U/TXT) playlist track was silently dropped and the track re-appeared as 'extra' (radoslav-orlov). Root cause: unlike Spotify/YouTube (native ids), file-import + iTunes-only tracks arrive with an EMPTY source_track_id — and the whole manual-match system keys on it. _persist_find_and_add_match is a no-op on an empty id, and find_manual_library_match_by_source_track_id returns None for one, so the match can be neither recorded nor looked up. That's the youtube-vs-file difference the reporter noticed. Fix: stable_source_track_id() derives a DETERMINISTIC 'file:' id from the track identity (artist|title|album, normalized) when there's no native id; mirror_playlist assigns it so the SAME song gets the SAME id across re-imports/discovery — exactly what the match lookup needs. Native ids are used verbatim; bonus: discovery extra_data now survives a re-import for these tracks too. Tests: helper (native passthrough, deterministic + case/field-insensitive, distinct per song, empty-on-no-title, file: prefix); mirror_playlist integration (file tracks get stable distinct ids, stable across re-import, native ids untouched). 319 playlist/ sync/discovery/mirrored tests green. --- core/playlists/source_refs.py | 29 ++++++++++++ database/music_database.py | 14 +++--- tests/database/test_mirrored_playlists.py | 32 +++++++++++++ .../playlists/test_stable_source_track_id.py | 45 +++++++++++++++++++ 4 files changed, 115 insertions(+), 5 deletions(-) create mode 100644 tests/playlists/test_stable_source_track_id.py diff --git a/core/playlists/source_refs.py b/core/playlists/source_refs.py index 4cf03534..0e915ca4 100644 --- a/core/playlists/source_refs.py +++ b/core/playlists/source_refs.py @@ -19,6 +19,35 @@ from urllib.parse import parse_qs, urlparse _SPOTIFY_ID_RE = re.compile(r"^[A-Za-z0-9]{16,32}$") + +def stable_source_track_id(track: Mapping, existing: Optional[str] = None) -> str: + """A stable per-track id for a mirrored-playlist track. + + Spotify / YouTube / Deezer tracks carry a native id. File-import (CSV / M3U / + TXT) and iTunes-only sources don't — they arrive with an empty + ``source_track_id``. The whole manual-match system (Find & Add ↔ sync) keys on + ``source_track_id``, and an empty key can neither be recorded (the persist is a + no-op) nor looked up — so a manual match on a file-import track is silently + dropped and the track re-appears as "extra" (#901). + + When a native id is present it's used verbatim. Otherwise we derive a + DETERMINISTIC id from the track's identity (artist|title|album, normalized) so + the SAME song gets the SAME id across re-imports and discovery passes — which + is exactly what the match lookup needs. Prefixed ``file:`` so it's recognizable + and never collides with a real upstream id. Returns '' only when there's no + usable identity at all (no title).""" + native = (existing if existing is not None else track.get("source_track_id")) or "" + native = str(native).strip() + if native: + return native + title = str(track.get("track_name") or track.get("name") or "").strip().lower() + if not title: + return "" + artist = str(track.get("artist_name") or track.get("artist") or "").strip().lower() + album = str(track.get("album_name") or track.get("album") or "").strip().lower() + digest = hashlib.md5(f"{artist}|{title}|{album}".encode("utf-8")).hexdigest()[:16] + return f"file:{digest}" + # Synthetic batch playlist_id prefixes that wrap a mirrored_playlists PK. # Download/discovery flows build a batch playlist_id as f"{prefix}{pk}" — e.g. # auto_mirror_ (core/automation/handlers/sync_playlist.py), youtube_mirrored_ diff --git a/database/music_database.py b/database/music_database.py index 7fb473bd..f047e1d9 100644 --- a/database/music_database.py +++ b/database/music_database.py @@ -13933,16 +13933,20 @@ class MusicDatabase: logger.debug("Failed to preserve mirrored playlist extra_data: %s", e) # Replace all tracks + from core.playlists.source_refs import stable_source_track_id cursor.execute("DELETE FROM mirrored_playlist_tracks WHERE playlist_id=?", (playlist_id,)) for i, t in enumerate(tracks): + # File-import / iTunes-only tracks arrive with no native id; give + # them a DETERMINISTIC one so a Find & Add manual match can be + # recorded and found (it keys on source_track_id) instead of being + # silently dropped and re-appearing as "extra" (#901). + sid = stable_source_track_id(t) extra = t.get('extra_data') if extra and not isinstance(extra, str): extra = json.dumps(extra) # Restore preserved discovery data if the incoming track doesn't have its own - if not extra: - sid = t.get('source_track_id') - if sid and sid in old_extra_map: - extra = old_extra_map[sid] + if not extra and sid and sid in old_extra_map: + extra = old_extra_map[sid] cursor.execute(""" INSERT INTO mirrored_playlist_tracks (playlist_id, position, track_name, artist_name, album_name, duration_ms, image_url, source_track_id, extra_data) @@ -13951,7 +13955,7 @@ class MusicDatabase: playlist_id, i + 1, t.get('track_name', ''), t.get('artist_name', ''), t.get('album_name', ''), t.get('duration_ms', 0), - t.get('image_url'), t.get('source_track_id'), extra + t.get('image_url'), sid or None, extra )) conn.commit() logger.info(f"Mirrored playlist '{name}' ({source}) with {len(tracks)} tracks") diff --git a/tests/database/test_mirrored_playlists.py b/tests/database/test_mirrored_playlists.py index 8da26480..ff563e68 100644 --- a/tests/database/test_mirrored_playlists.py +++ b/tests/database/test_mirrored_playlists.py @@ -60,3 +60,35 @@ def test_mirror_playlist_refresh_preserves_existing_description(tmp_path): assert refreshed_id == playlist_id playlist = db.get_mirrored_playlist(playlist_id) assert playlist["description"] == "https://open.spotify.com/playlist/abc" + + +def test_file_import_tracks_get_a_stable_source_track_id(tmp_path): + # #901: file-import tracks arrive with no source_track_id; mirror_playlist must + # assign a deterministic one so a Find & Add manual match can key on it (and so + # discovery extra_data survives a re-import). + db = MusicDatabase(str(tmp_path / "music.db")) + file_tracks = [ + {"track_name": "Slow Ride", "artist_name": "Foghat", "album_name": "Fool for the City"}, + {"track_name": "I Gotta Feeling", "artist_name": "The Black Eyed Peas"}, + ] + pid = db.mirror_playlist(source="file", source_playlist_id="myfile", name="From File", + tracks=file_tracks, profile_id=1) + rows = db.get_mirrored_playlist_tracks(pid) + ids = [r["source_track_id"] for r in rows] + assert all(i and i.startswith("file:") for i in ids) # no empty ids + assert len(set(ids)) == 2 # distinct per song + + # Re-import the SAME file → SAME ids (stable), so a recorded match still keys. + db.mirror_playlist(source="file", source_playlist_id="myfile", name="From File", + tracks=list(file_tracks), profile_id=1) + rows2 = db.get_mirrored_playlist_tracks(pid) + assert [r["source_track_id"] for r in rows2] == ids + + +def test_native_ids_still_used_verbatim(tmp_path): + db = MusicDatabase(str(tmp_path / "music.db")) + pid = db.mirror_playlist(source="spotify", source_playlist_id="sp", name="Sp", + tracks=[{"track_name": "S", "artist_name": "A", "source_track_id": "spotify123"}], + profile_id=1) + rows = db.get_mirrored_playlist_tracks(pid) + assert rows[0]["source_track_id"] == "spotify123" # native id untouched diff --git a/tests/playlists/test_stable_source_track_id.py b/tests/playlists/test_stable_source_track_id.py new file mode 100644 index 00000000..10c9af72 --- /dev/null +++ b/tests/playlists/test_stable_source_track_id.py @@ -0,0 +1,45 @@ +"""#901: a manual match (Find & Add) on a file-import playlist track was silently +dropped and the track re-appeared as "extra". Root cause: file-import / iTunes-only +tracks arrive with an EMPTY source_track_id, and the whole manual-match system keys +on it — an empty key can't be persisted (no-op) or looked up. Fix: derive a stable +deterministic id from the track's identity so matches stick like they do for +Spotify/YouTube (which carry native ids). +""" + +from __future__ import annotations + +from core.playlists.source_refs import stable_source_track_id + + +def test_native_id_is_used_verbatim(): + assert stable_source_track_id({"source_track_id": "2fdfsGuqb6SBX5ocoBWHUd"}) == "2fdfsGuqb6SBX5ocoBWHUd" + # explicit existing wins over the dict + assert stable_source_track_id({"source_track_id": "x"}, existing="y") == "y" + + +def test_file_track_gets_a_deterministic_prefixed_id(): + t = {"track_name": "Slow Ride", "artist_name": "Foghat", "album_name": "Fool for the City"} + a = stable_source_track_id(t) + assert a.startswith("file:") and len(a) == len("file:") + 16 + # SAME song → SAME id across calls/re-imports (what the match lookup needs) + assert stable_source_track_id(dict(t)) == a + + +def test_identity_is_case_and_field_insensitive_but_distinguishes_songs(): + base = {"track_name": "Slow Ride", "artist_name": "Foghat", "album_name": "Fool for the City"} + same = {"name": "slow ride", "artist": "FOGHAT", "album": "fool for the city"} # alt field names + case + assert stable_source_track_id(base) == stable_source_track_id(same) + # a different song gets a different id + assert stable_source_track_id(base) != stable_source_track_id( + {"track_name": "I Just Want to Make Love to You", "artist_name": "Foghat"}) + + +def test_empty_id_when_no_title(): + assert stable_source_track_id({"artist_name": "Foghat"}) == "" + assert stable_source_track_id({}) == "" + + +def test_never_collides_with_a_real_upstream_id(): + # the file: prefix keeps synthetic ids out of the spotify/youtube id space + fid = stable_source_track_id({"track_name": "x", "artist_name": "y"}) + assert ":" in fid and not fid.replace("file:", "").startswith("file") From 1ad80d77a63a98022aee4d06fb6e8a0a313fcd60 Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Mon, 22 Jun 2026 09:06:34 -0700 Subject: [PATCH 03/23] =?UTF-8?q?#901:=20one-time=20backfill=20=E2=80=94?= =?UTF-8?q?=20stable=20ids=20for=20EXISTING=20file-import=20mirrored=20tra?= =?UTF-8?q?cks?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The mirror_playlist fix only assigns stable ids to newly-imported playlists, so a user with an existing file-import playlist would still have empty-id rows (and dead Find & Add matches) until a manual re-import. Add an idempotent startup backfill that assigns the SAME stable id a fresh import would to any mirrored track missing one — so existing matches start sticking with no re-import. Runs once per db/process (the init is guarded), only touches empty-id rows (no-op afterward), native ids untouched. Tests: backfill fills empty ids with the exact fresh-import id, is idempotent (2nd run = 0), and leaves native ids alone. --- database/music_database.py | 33 +++++++++++++++++++++++ tests/database/test_mirrored_playlists.py | 33 +++++++++++++++++++++++ 2 files changed, 66 insertions(+) diff --git a/database/music_database.py b/database/music_database.py index f047e1d9..20e6b217 100644 --- a/database/music_database.py +++ b/database/music_database.py @@ -992,6 +992,39 @@ class MusicDatabase: raise self._init_manual_library_match_table() + self._backfill_mirrored_track_source_ids() + + def _backfill_mirrored_track_source_ids(self) -> int: + """One-time, idempotent: assign a stable source_track_id to mirrored tracks + that have none (file-import / iTunes-only playlists imported before #901), so + their existing Find & Add matches start sticking without a manual re-import. + Only touches empty-id rows, so it's a no-op once they're filled.""" + from core.playlists.source_refs import stable_source_track_id + updated = 0 + try: + with self._get_connection() as conn: + cursor = conn.cursor() + cursor.execute(""" + SELECT id, track_name, artist_name, album_name + FROM mirrored_playlist_tracks + WHERE source_track_id IS NULL OR source_track_id = '' + """) + rows = cursor.fetchall() + for r in rows: + sid = stable_source_track_id({ + 'track_name': r['track_name'], 'artist_name': r['artist_name'], + 'album_name': r['album_name']}) + if sid: + cursor.execute( + "UPDATE mirrored_playlist_tracks SET source_track_id = ? WHERE id = ?", + (sid, r['id'])) + updated += 1 + conn.commit() + if updated: + logger.info("Backfilled stable source_track_id on %d mirrored tracks (#901)", updated) + except Exception as e: + logger.error("mirrored track source_id backfill failed: %s", e) + return updated # Bump when the schema's generation meaningfully changes. Stamped into # PRAGMA user_version as a backstop indicator; nothing GATES on it yet. diff --git a/tests/database/test_mirrored_playlists.py b/tests/database/test_mirrored_playlists.py index ff563e68..c1d0b6b6 100644 --- a/tests/database/test_mirrored_playlists.py +++ b/tests/database/test_mirrored_playlists.py @@ -92,3 +92,36 @@ def test_native_ids_still_used_verbatim(tmp_path): profile_id=1) rows = db.get_mirrored_playlist_tracks(pid) assert rows[0]["source_track_id"] == "spotify123" # native id untouched + + +def test_backfill_fills_existing_empty_ids_idempotently(tmp_path): + # #901 backfill: a file-import playlist mirrored BEFORE the fix has empty-id rows. + # The backfill assigns the SAME stable ids a fresh import would, so existing + # Find & Add matches start working without a re-import. + db = MusicDatabase(str(tmp_path / "music.db")) + pid = db.mirror_playlist(source="file", source_playlist_id="old", name="Old", + tracks=[{"track_name": "Slow Ride", "artist_name": "Foghat"}], profile_id=1) + # simulate a pre-fix row: blank out the id + with db._get_connection() as conn: + conn.execute("UPDATE mirrored_playlist_tracks SET source_track_id = '' WHERE playlist_id = ?", (pid,)) + conn.commit() + + n = db._backfill_mirrored_track_source_ids() + assert n == 1 + rows = db.get_mirrored_playlist_tracks(pid) + from core.playlists.source_refs import stable_source_track_id + assert rows[0]["source_track_id"] == stable_source_track_id( + {"track_name": "Slow Ride", "artist_name": "Foghat"}) # same id a fresh import gives + + # idempotent — second run touches nothing + assert db._backfill_mirrored_track_source_ids() == 0 + + +def test_backfill_leaves_native_ids_untouched(tmp_path): + db = MusicDatabase(str(tmp_path / "music.db")) + pid = db.mirror_playlist(source="spotify", source_playlist_id="sp", name="Sp", + tracks=[{"track_name": "S", "artist_name": "A", "source_track_id": "spotify123"}], + profile_id=1) + db._backfill_mirrored_track_source_ids() + rows = db.get_mirrored_playlist_tracks(pid) + assert rows[0]["source_track_id"] == "spotify123" From c11a742e584fda9c9e4d00fe53b61b625d96cb4b Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Mon, 22 Jun 2026 09:30:04 -0700 Subject: [PATCH 04/23] Multi-disc albums: never write a disc-less track (floor disc to >=1) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Sokhi: some tracks in a multi-disc album showed up with a null disc in Jellyfin and floated ungrouped above the disc sections (tracks 3/9/15). Mechanism: the tag writer only wrote the disc tag when disc_number was truthy, and enrichment CLEARS all tags before rewriting — so a track whose disc came back 0 / None / '' lost its disc entirely. Those falsy values slipped through because source.py defaulted with 'is not None' (a literal 0 passed) and context.py's or-chain can yield None; this happens especially when a track resolves to a different edition than its siblings. Fix: normalize_disc_number() floors any value to >=1, and enrichment now writes the disc tag UNCONDITIONALLY (like the track number) so a track is never disc-less. source.py uses the same floor so the metadata dict (and the 'Disc N' folder org) stays consistent. Valid multi-disc values are preserved untouched. Tests: normalize floors 0/None/''/negatives/non-numeric -> 1, preserves 1..4 and tolerates '2.0'. 1406 enrich/metadata/track-number tests green, ruff clean. NOTE: this fixes the SYMPTOM (never ungrouped). The deeper cause — a track matching a DIFFERENT edition/release than its album siblings (the Persona-box-set mismatch in the sample file; the canonical-version problem) — is separate and still open. --- core/imports/track_number.py | 21 ++++++++++++++ core/metadata/enrichment.py | 17 +++++++---- core/metadata/source.py | 6 +++- tests/imports/test_normalize_disc_number.py | 31 +++++++++++++++++++++ 4 files changed, 68 insertions(+), 7 deletions(-) create mode 100644 tests/imports/test_normalize_disc_number.py diff --git a/core/imports/track_number.py b/core/imports/track_number.py index 63b53b03..cbe0e251 100644 --- a/core/imports/track_number.py +++ b/core/imports/track_number.py @@ -159,3 +159,24 @@ def resolve_track_number( # value the pre-fix resolver would have used. A correctly-named file # with a stale/wrong embedded tag is therefore never regressed. return _coerce_positive(embedded_track_number) + + +def normalize_disc_number(value) -> int: + """Coerce a disc value to a positive int, defaulting to 1. + + Every track in a multi-disc album MUST carry a disc number, or Jellyfin/Plex + leave the disc-less ones floating ungrouped above the disc sections (Sokhi's + "tracks 3/9/15 at the top"). Upstream sources can hand back 0, None, '', or a + non-numeric string for some tracks — especially when a track resolved to a + different edition than its siblings — and the tag-writer only wrote the disc + tag when it was truthy, so those tracks lost it entirely on the clear-then- + rewrite. Flooring to >=1 here means a track is never written disc-less. + """ + try: + n = int(value) # int, float, or clean int-string + except (TypeError, ValueError): + try: + n = int(float(str(value).strip())) # tolerate "2.0" + except (TypeError, ValueError): + return 1 + return n if n >= 1 else 1 diff --git a/core/metadata/enrichment.py b/core/metadata/enrichment.py index e3642419..69716a6c 100644 --- a/core/metadata/enrichment.py +++ b/core/metadata/enrichment.py @@ -131,6 +131,14 @@ def enhance_file_metadata(file_path: str, context: dict, artist: dict, album_inf track_num_str = format_track_number_tag( metadata.get('track_number'), metadata.get('total_tracks') ) + # Disc number is written UNCONDITIONALLY (floored to >=1), like the + # track number above. The old code only wrote it when truthy, so a + # track whose disc came back 0/None/'' (e.g. matched to a different + # edition) lost its disc tag on the clear-then-rewrite and floated + # ungrouped above the disc sections in Jellyfin/Plex (Sokhi). + from core.imports.track_number import normalize_disc_number + _disc_num = normalize_disc_number(metadata.get('disc_number')) + disc_num_str = str(_disc_num) write_multi = cfg.get("metadata_enhancement.tags.write_multi_artist", False) artists_list = metadata.get("_artists_list", []) @@ -162,8 +170,7 @@ def enhance_file_metadata(file_path: str, context: dict, artist: dict, album_inf if metadata.get("genre"): audio_file.tags.add(symbols.TCON(encoding=3, text=[metadata["genre"]])) audio_file.tags.add(symbols.TRCK(encoding=3, text=[track_num_str])) - if metadata.get("disc_number"): - audio_file.tags.add(symbols.TPOS(encoding=3, text=[str(metadata["disc_number"])])) + audio_file.tags.add(symbols.TPOS(encoding=3, text=[disc_num_str])) elif is_vorbis_like(audio_file, symbols): if metadata.get("title"): audio_file["title"] = [metadata["title"]] @@ -180,8 +187,7 @@ def enhance_file_metadata(file_path: str, context: dict, artist: dict, album_inf if metadata.get("genre"): audio_file["genre"] = [metadata["genre"]] audio_file["tracknumber"] = [track_num_str] - if metadata.get("disc_number"): - audio_file["discnumber"] = [str(metadata["disc_number"])] + audio_file["discnumber"] = [disc_num_str] elif isinstance(audio_file, symbols.MP4): if metadata.get("title"): audio_file["\xa9nam"] = [metadata["title"]] @@ -198,8 +204,7 @@ def enhance_file_metadata(file_path: str, context: dict, artist: dict, album_inf audio_file["trkn"] = [format_track_number_tuple( metadata.get("track_number"), metadata.get("total_tracks") )] - if metadata.get("disc_number"): - audio_file["disk"] = [(metadata["disc_number"], 0)] + audio_file["disk"] = [(_disc_num, 0)] embed_source_ids(audio_file, metadata, context, runtime=runtime) diff --git a/core/metadata/source.py b/core/metadata/source.py index ab44b241..9f9a9190 100644 --- a/core/metadata/source.py +++ b/core/metadata/source.py @@ -1092,7 +1092,11 @@ def extract_source_metadata(context: dict, artist: dict, album_info: dict) -> di disc_num = original_search.get("disc_number") if disc_num is None and album_info: disc_num = album_info.get("disc_number") - metadata["disc_number"] = disc_num if disc_num is not None else 1 + # Floor to >=1: a 0 / '' / non-numeric disc must not slip through (the old + # `is not None` let a literal 0 past, which then read as disc-less downstream + # and ungrouped the track in Jellyfin/Plex). Also feeds the "Disc N" folder org. + from core.imports.track_number import normalize_disc_number + metadata["disc_number"] = normalize_disc_number(disc_num) if album_ctx and album_ctx.get("release_date"): release_date = _normalize_release_date_tag(album_ctx.get("release_date")) diff --git a/tests/imports/test_normalize_disc_number.py b/tests/imports/test_normalize_disc_number.py new file mode 100644 index 00000000..a2c4e6dd --- /dev/null +++ b/tests/imports/test_normalize_disc_number.py @@ -0,0 +1,31 @@ +"""Sokhi: some tracks in a multi-disc album got a null disc in Jellyfin and floated +ungrouped above the disc sections. Root cause: the tag-writer only wrote the disc +tag when disc_number was truthy, and upstream a 0 / None / '' (esp. when a track +matched a different edition than its siblings) slipped through — so on the +clear-then-rewrite those tracks lost their disc entirely. normalize_disc_number +floors any value to >=1 so a track is never written disc-less.""" + +from __future__ import annotations + +import pytest + +from core.imports.track_number import normalize_disc_number + + +@pytest.mark.parametrize("value,expected", [ + (1, 1), (2, 2), (4, 4), + ("1", 1), ("3", 3), (" 2 ", 2), + (0, 1), ("0", 1), # the bug: 0 must floor to 1, not vanish + (None, 1), ("", 1), (" ", 1), + (-1, 1), ("-2", 1), # negatives floor to 1 + ("abc", 1), ("1/4", 1), # non-numeric -> 1 (never raises) + (2.0, 2), # float-ish via str() +]) +def test_normalize_disc_number(value, expected): + assert normalize_disc_number(value) == expected + + +def test_valid_multidisc_values_preserved(): + # a real disc on a 4xLP must survive untouched + for d in (1, 2, 3, 4): + assert normalize_disc_number(d) == d From 203142c4a9898a6fa3236d767085207d262bd9bd Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Mon, 22 Jun 2026 10:56:53 -0700 Subject: [PATCH 05/23] Multi-disc: file the track in the disc folder that matches its tag (Sokhi) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Confirmed from Sokhi's FLAC tags + screenshot: disc-2/3 tracks land in the 'Disc 1' folder, collapsing every disc's track 3/4/5/6 into one folder. Root cause: the import pipeline syncs the resolved TRACK number into album_info (so the folder matches the tag — pipeline.py '[FIX] Updated album_info track_number') but never did the same for DISC. So the 'Disc N' folder (built from album_info.disc_number, often 1) used a different disc than the embedded tag (resolved per-track in source.py — e.g. 2/3 from a MusicBrainz multi-medium release). Fix: one SHARED resolver, resolve_disc_for_track(original_search, album_info), used by BOTH source.py (the tag) and the pipeline (which now writes it back into album_info before building the path). Same function + same inputs (the pipeline pulls the identical get_import_original_search(context)), so folder and tag can never disagree. Returns the first valid positive disc (per-track, then album), else 1 — a falsy/unknown per-track disc falls through to the album instead of flooring early. Tests: resolver preference/fallback/floor + an explicit folder==tag lockstep check incl. Sokhi's per-track-2/album-1 case. 2122 import/pipeline/metadata tests green. --- core/imports/pipeline.py | 10 ++++++ core/imports/track_number.py | 29 ++++++++++++++++ core/metadata/source.py | 14 ++++---- tests/imports/test_normalize_disc_number.py | 38 +++++++++++++++++++++ 4 files changed, 83 insertions(+), 8 deletions(-) diff --git a/core/imports/pipeline.py b/core/imports/pipeline.py index ad6a0801..700bc51a 100644 --- a/core/imports/pipeline.py +++ b/core/imports/pipeline.py @@ -655,6 +655,16 @@ def post_process_matched_download(context_key, context, file_path, runtime, meta album_info['clean_track_name'] = clean_track_name logger.info(f"[FIX] Updated album_info track_number to {track_number} for consistent metadata") + # Sync the disc number the SAME way (and via the SAME resolver) the embedded + # tag will use — otherwise the "Disc N" folder is built from album_info's + # original disc while the tag takes the per-track disc, so a disc-2/3 track + # lands in the Disc 1 folder and every disc's tracks collapse together (Sokhi). + from core.imports.track_number import resolve_disc_for_track + _resolved_disc = resolve_disc_for_track(get_import_original_search(context), album_info) + if album_info.get('disc_number') != _resolved_disc: + logger.info(f"[FIX] Updated album_info disc_number to {_resolved_disc} for consistent metadata") + album_info['disc_number'] = _resolved_disc + final_path, _ = build_final_path_for_track(context, artist_context, album_info, file_ext) logger.info(f"Resolved path: '{final_path}'") context['_final_processed_path'] = final_path diff --git a/core/imports/track_number.py b/core/imports/track_number.py index cbe0e251..808577c7 100644 --- a/core/imports/track_number.py +++ b/core/imports/track_number.py @@ -180,3 +180,32 @@ def normalize_disc_number(value) -> int: except (TypeError, ValueError): return 1 return n if n >= 1 else 1 + + +def resolve_disc_for_track(original_search, album_info) -> int: + """The disc number for a track — resolved IDENTICALLY for the 'Disc N' folder + (import pipeline) and the embedded tag (metadata.source), so the two can never + disagree. + + Sokhi: the pipeline synced the resolved track_number into album_info (so the + folder matched the tag) but never did the same for disc — the folder used + album_info's original disc (often 1) while the tag took the per-track disc + (e.g. 2/3 from a MusicBrainz multi-medium release). Result: a disc-2/3 track + landed in the Disc 1 folder, collapsing every disc's tracks into one folder. + + Returns the first VALID positive disc — the per-track search's, else the album + context's — else 1. A falsy/unknown (0/None/'') per-track disc falls through to + the album rather than flooring early. Single source of truth so both call sites + stay in lockstep.""" + for src in ((original_search or {}), (album_info or {})): + raw = src.get("disc_number") + try: + n = int(raw) + except (TypeError, ValueError): + try: + n = int(float(str(raw).strip())) + except (TypeError, ValueError): + continue + if n >= 1: + return n + return 1 diff --git a/core/metadata/source.py b/core/metadata/source.py index 9f9a9190..b3b1f4ee 100644 --- a/core/metadata/source.py +++ b/core/metadata/source.py @@ -1089,14 +1089,12 @@ def extract_source_metadata(context: dict, artist: dict, album_info: dict) -> di metadata["track_number"] = 1 metadata["total_tracks"] = 1 - disc_num = original_search.get("disc_number") - if disc_num is None and album_info: - disc_num = album_info.get("disc_number") - # Floor to >=1: a 0 / '' / non-numeric disc must not slip through (the old - # `is not None` let a literal 0 past, which then read as disc-less downstream - # and ungrouped the track in Jellyfin/Plex). Also feeds the "Disc N" folder org. - from core.imports.track_number import normalize_disc_number - metadata["disc_number"] = normalize_disc_number(disc_num) + # Resolve via the SHARED resolver so the embedded tag and the "Disc N" folder + # (computed in the import pipeline from album_info) can never disagree — same + # function, same inputs. Floors to >=1 (a 0/''/non-numeric disc must not read + # as disc-less and ungroup the track in Jellyfin/Plex). + from core.imports.track_number import resolve_disc_for_track + metadata["disc_number"] = resolve_disc_for_track(original_search, album_info) if album_ctx and album_ctx.get("release_date"): release_date = _normalize_release_date_tag(album_ctx.get("release_date")) diff --git a/tests/imports/test_normalize_disc_number.py b/tests/imports/test_normalize_disc_number.py index a2c4e6dd..c7f392e5 100644 --- a/tests/imports/test_normalize_disc_number.py +++ b/tests/imports/test_normalize_disc_number.py @@ -29,3 +29,41 @@ def test_valid_multidisc_values_preserved(): # a real disc on a 4xLP must survive untouched for d in (1, 2, 3, 4): assert normalize_disc_number(d) == d + + +# ── resolve_disc_for_track: the FOLDER and the TAG must use the same disc ────── + +from core.imports.track_number import resolve_disc_for_track + + +def test_resolve_disc_prefers_per_track_search_then_album(): + # per-track disc wins (this is the value the tag uses) — so the folder, which + # now calls the SAME resolver with the SAME inputs, lands on the same disc. + assert resolve_disc_for_track({"disc_number": 3}, {"disc_number": 1}) == 3 + # falls back to album context when the per-track search has none + assert resolve_disc_for_track({}, {"disc_number": 2}) == 2 + assert resolve_disc_for_track({"disc_number": None}, {"disc_number": 2}) == 2 + # both missing -> floored default 1 + assert resolve_disc_for_track({}, {}) == 1 + assert resolve_disc_for_track(None, None) == 1 + + +def test_resolve_disc_floors_bad_values(): + assert resolve_disc_for_track({"disc_number": 0}, {"disc_number": 5}) == 5 # 0 is falsy -> fall to album + assert resolve_disc_for_track({"disc_number": "2"}, {}) == 2 + assert resolve_disc_for_track({"disc_number": "junk"}, {}) == 1 + + +def test_folder_and_tag_resolve_identically(): + # the regression that matters: given the same (original_search, album_info), + # source.py (tag) and the pipeline (folder) get the IDENTICAL disc. + cases = [ + ({"disc_number": 2}, {"disc_number": 1}), # Sokhi's case: per-track 2, album 1 + ({"disc_number": 3}, {"disc_number": 1}), + ({}, {"disc_number": 1}), + ({"disc_number": 0}, {"disc_number": 1}), + ] + for osrch, ainfo in cases: + folder_disc = resolve_disc_for_track(osrch, ainfo) # what the pipeline writes to album_info + tag_disc = resolve_disc_for_track(osrch, ainfo) # what source.py writes to the tag + assert folder_disc == tag_disc From 89018bb6b34e013919bc383e9e4aa62be376f8fc Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Mon, 22 Jun 2026 12:06:57 -0700 Subject: [PATCH 06/23] CSS: .sidebar-header z-index 2 -> 1 Drop the header to the same stacking level as the nav instead of forcing it above. --- webui/static/style.css | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/webui/static/style.css b/webui/static/style.css index 8d679da8..456d6a41 100644 --- a/webui/static/style.css +++ b/webui/static/style.css @@ -307,11 +307,9 @@ body.reduce-effects .sidebar::after { gap: 8px; position: sticky; top: 0; - /* Above the nav (the sidebar gives its children z-index:1) so nav items - scrolling up sit BEHIND the header — i.e. in its backdrop, where the - blur below can actually act on them. Without this they paint in front - and backdrop-filter has nothing to blur. */ - z-index: 2; + /* Same stacking level as the nav (z-index:1) — sits in the normal sidebar + flow rather than forced above everything. */ + z-index: 1; overflow: hidden; flex-shrink: 0; From b95a8f539edcea930f6cc0b354f05f825d16aa32 Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Mon, 22 Jun 2026 13:32:24 -0700 Subject: [PATCH 07/23] Auto-download: resolve an album track's real position from its album (not 1/1) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Tracks auto-downloaded from the playlist pipeline / wishlist / watchlist landed as 01/1 even though they belong to multi-track albums (wolf/Sokhi; verified live — Deezer says 'Obelisk' is track 9 of The Grand Mirage, Olives is 3/4, etc.). Root cause, located in code: discovery doesn't carry a per-track position for sources whose search/track endpoint omits it (Deezer search, MusicBrainz recordings — only their ALBUM endpoint has it). detect_album_info_web then set 'track_number': track_info.get('track_number') (= None) and never looked it up from the album it HAD identified (context.py); the pipeline floored it to 1. The one helper that does an album lookup only ran for the no-album-context branch and is gated off by default. Not isolated to Deezer — the gap is source-agnostic. Fix: when the album is known (album_id present) but the position is missing, resolve the REAL (track_number, disc_number) from the album's own track list via the source-agnostic get_album_tracks_for_source — using the album id discovery already picked (no re-search, no edition guessing). Matches by ISRC -> source track id -> title. Fail-safe: any miss/error leaves the number untouched, so it still falls through to the filename exactly as before — never worse than today. kettui: pure seam core/imports/album_position.resolve_track_position_in_album (I/O-free, ISRC>id>title priority, skips position-less entries) + a fail-safe integration wrapper, both covered — 11 tests incl. the 'Obelisk = 9/12' case, priority resolution, and never-raises-on-fetch-error. 788 import/context/pipeline tests green, ruff clean. --- core/imports/album_position.py | 94 +++++++++++++++++++++ core/imports/context.py | 62 ++++++++++++-- tests/imports/test_album_position.py | 119 +++++++++++++++++++++++++++ 3 files changed, 269 insertions(+), 6 deletions(-) create mode 100644 core/imports/album_position.py create mode 100644 tests/imports/test_album_position.py diff --git a/core/imports/album_position.py b/core/imports/album_position.py new file mode 100644 index 00000000..7da1b14a --- /dev/null +++ b/core/imports/album_position.py @@ -0,0 +1,94 @@ +"""Resolve a track's position WITHIN its album's track list. + +The bug this fixes: a track auto-downloaded from the playlist pipeline / wishlist / +watchlist is identified as belonging to an album, but the per-track position is +unknown — Deezer's search/track and MusicBrainz's recording lookups don't carry a +track position (only their album endpoint does). ``detect_album_info_web`` then +leaves ``track_number = None``, the import pipeline falls through to the default-1 +floor, and the file lands as ``01/1`` even though the album is known +(``core/imports/context.py``). Verified live: e.g. Deezer says "Obelisk" is track +9 of *The Grand Mirage*, but it was tagged 1/1. + +This is the pure matcher: given the album's track list (fetched by the caller via +``core.metadata.album_tracks.get_album_tracks_for_source`` — so this stays +source-agnostic and I/O-free) plus the track's own identifiers, return its real +``(track_number, disc_number)``. Match priority is by reliability: + +1. **ISRC** — an exact recording identity; trusted immediately. +2. **source track id** — exact within this album. +3. **normalized title** — last resort. + +Returns ``(None, None)`` on no confident match, so the caller keeps its existing +behaviour (never worse than today). +""" + +from __future__ import annotations + +import re +from typing import Any, List, Optional, Tuple + + +def _norm_title(value: Any) -> str: + """Lower, strip punctuation, collapse whitespace — for tolerant title match.""" + s = re.sub(r"[^\w\s]", "", str(value or "").lower()) + return re.sub(r"\s+", " ", s).strip() + + +def _pos_int(value: Any) -> Optional[int]: + try: + n = int(value) + except (TypeError, ValueError): + return None + return n if n >= 1 else None + + +def resolve_track_position_in_album( + album_tracks: List[dict], + *, + title: str = "", + track_id: str = "", + isrc: str = "", +) -> Tuple[Optional[int], Optional[int]]: + """Return ``(track_number, disc_number)`` for this track within ``album_tracks``, + or ``(None, None)`` when no confident match is found. + + ``album_tracks`` is the list under ``get_album_tracks_for_source(...)['tracks']`` + — each entry has ``track_number`` / ``disc_number`` / ``id`` / ``name`` / ``isrc``. + Entries without a valid positive ``track_number`` are skipped. Pure: no I/O. + """ + if not album_tracks: + return (None, None) + + want_isrc = str(isrc or "").strip().upper() + want_id = str(track_id or "").strip() + want_title = _norm_title(title) + + by_id: Optional[Tuple[int, int]] = None + by_title: Optional[Tuple[int, int]] = None + + for t in album_tracks: + if not isinstance(t, dict): + continue + tn = _pos_int(t.get("track_number")) + if tn is None: + continue + dn = _pos_int(t.get("disc_number")) or 1 + + # 1) ISRC — exact recording. Win immediately. + if want_isrc and str(t.get("isrc") or "").strip().upper() == want_isrc: + return (tn, dn) + # 2) source track id — exact within the album. + if by_id is None and want_id and str(t.get("id") or "").strip() == want_id: + by_id = (tn, dn) + # 3) normalized title — last resort. + if by_title is None and want_title and _norm_title(t.get("name")) == want_title: + by_title = (tn, dn) + + if by_id is not None: + return by_id + if by_title is not None: + return by_title + return (None, None) + + +__all__ = ["resolve_track_position_in_album"] diff --git a/core/imports/context.py b/core/imports/context.py index 9ed2a909..4e15c918 100644 --- a/core/imports/context.py +++ b/core/imports/context.py @@ -433,16 +433,24 @@ def detect_album_info_web(context, artist_context=None): track_name.strip().lower(), artist_name.strip().lower(), }: + _tn = track_info.get("track_number") + _dn = track_info.get("disc_number") + # The album is identified but discovery often doesn't carry the per-track + # POSITION — Deezer's search/track and MusicBrainz's recording lookups omit + # it (only their album endpoint has it). Without a position the pipeline + # falls through to the default-1 floor and files an album track as 01/1 + # (e.g. Deezer says "Obelisk" is track 9 of The Grand Mirage). Resolve the + # REAL position from the album's own track list when we have its id. + # Fail-safe: leaves the numbers untouched on any miss, so behaviour is + # never worse than the old preserve-None-and-fall-through. + if _tn is None: + _tn, _dn = _resolve_album_position_from_source(context, artist_context, _dn) return build_import_album_info( context, album_info={ "album_name": album_name, - # Preserve missing numbers as None so the import pipeline - # can fall through to ``extract_track_number_from_filename`` - # at ``core/imports/pipeline.py:652`` instead of locking - # to track/disc 01 for every wishlist re-attempt. - "track_number": track_info.get("track_number"), - "disc_number": track_info.get("disc_number"), + "track_number": _tn, + "disc_number": _dn, "album_image_url": album_ctx.get("image_url", ""), "confidence": 0.5, }, @@ -454,6 +462,48 @@ def detect_album_info_web(context, artist_context=None): return _resolve_single_to_parent_album(context, artist_context) +def _resolve_album_position_from_source(context, artist_context, current_disc): + """Look up a track's real ``(track_number, disc_number)`` from its album's track + list, for the case where the album is known but discovery didn't carry a + position (Deezer/MusicBrainz search omit it). + + Uses the SAME album id discovery already resolved (``get_import_source_ids`` → + ``album_id``), so it re-homes the track onto its own album with no re-search and + no edition guessing. Matches by ISRC → source track id → title via the pure + ``core.imports.album_position`` seam. Returns ``(None, current_disc)`` on any + miss/error so the caller falls back exactly as before — never worse than today. + """ + try: + source = get_import_source(context) + ids = get_import_source_ids(context) + album_id = str(ids.get("album_id") or "") + if not source or not album_id: + return None, current_disc + + from core.metadata.album_tracks import get_album_tracks_for_source + payload = get_album_tracks_for_source(source, album_id) or {} + tracks = payload.get("tracks") or [] + if not tracks: + return None, current_disc + + track_info = get_import_track_info(context) + original_search = get_import_original_search(context) + title = (track_info.get("name") or original_search.get("title") or "").strip() + isrc = str(track_info.get("isrc") or original_search.get("isrc") or "") + + from core.imports.album_position import resolve_track_position_in_album + tn, dn = resolve_track_position_in_album( + tracks, title=title, track_id=str(ids.get("track_id") or ""), isrc=isrc) + if tn is not None: + logger.info("album-position: resolved '%s' to track %s/disc %s from album %s (%s)", + title, tn, dn, album_id, source) + return tn, (dn if dn is not None else current_disc) + return None, current_disc + except Exception as e: + logger.debug("album-position resolution failed: %s", e) + return None, current_disc + + def _resolve_single_to_parent_album(context, artist_context): """A single-matched track -> a promoted album_info for its parent album, or None. GATED by ``metadata_enhancement.single_to_album`` (default OFF — it's a diff --git a/tests/imports/test_album_position.py b/tests/imports/test_album_position.py new file mode 100644 index 00000000..d22b7ab7 --- /dev/null +++ b/tests/imports/test_album_position.py @@ -0,0 +1,119 @@ +"""A track auto-downloaded from the playlist pipeline / wishlist / watchlist is +identified as belonging to an album, but Deezer's search/track and MusicBrainz's +recording lookups don't carry a track POSITION — so detect_album_info_web left +track_number=None, the pipeline floored it to 1, and album tracks landed as 01/1 +(verified live: Deezer says "Obelisk" is track 9 of The Grand Mirage, tagged 1/1). + +The fix resolves the real position from the album's OWN track list. These pin the +pure matcher and the (fail-safe) integration wrapper. +""" + +from __future__ import annotations + +import pytest + +from core.imports.album_position import resolve_track_position_in_album + + +def _album_12(): + # a realistic 12-track album payload shape (get_album_tracks_for_source -> 'tracks') + return [ + {"id": f"t{i}", "name": n, "track_number": i, "disc_number": 1, + "isrc": f"ISRC{i:03d}"} + for i, n in enumerate( + ["Intro", "Drift", "Mirage", "Haze", "Pulse", "Glow", "Echo", "Tide", + "Obelisk", "Comet", "Dawn", "Outro"], start=1) + ] + + +# ── pure matcher ───────────────────────────────────────────────────────────── + +def test_resolves_position_by_title(): + tn, dn = resolve_track_position_in_album(_album_12(), title="Obelisk") + assert (tn, dn) == (9, 1) + + +def test_title_match_is_case_and_punctuation_insensitive(): + tracks = [{"id": "t1", "name": "Lueur Déclinante!!!", "track_number": 3, "disc_number": 1}] + tn, _ = resolve_track_position_in_album(tracks, title="lueur déclinante") + assert tn == 3 + + +def test_resolves_by_isrc_exactly(): + tn, dn = resolve_track_position_in_album(_album_12(), isrc="isrc009") # case-insensitive + assert (tn, dn) == (9, 1) + + +def test_resolves_by_track_id(): + tn, _ = resolve_track_position_in_album(_album_12(), track_id="t9") + assert tn == 9 + + +def test_isrc_beats_id_beats_title_on_conflict(): + # craft a list where ISRC, id, and title each point at a DIFFERENT track + tracks = [ + {"id": "byid", "name": "other", "track_number": 2, "disc_number": 1, "isrc": "X"}, + {"id": "z", "name": "WANT", "track_number": 3, "disc_number": 1, "isrc": "Y"}, + {"id": "z2", "name": "other2", "track_number": 4, "disc_number": 1, "isrc": "WANTISRC"}, + ] + # all three signals provided -> ISRC wins (track 4) + tn, _ = resolve_track_position_in_album(tracks, title="WANT", track_id="byid", isrc="wantisrc") + assert tn == 4 + # no ISRC -> id wins (track 2) + tn, _ = resolve_track_position_in_album(tracks, title="WANT", track_id="byid") + assert tn == 2 + # only title -> title wins (track 3) + tn, _ = resolve_track_position_in_album(tracks, title="want") + assert tn == 3 + + +def test_carries_disc_number(): + tracks = [{"id": "t1", "name": "B-Side", "track_number": 2, "disc_number": 2}] + assert resolve_track_position_in_album(tracks, title="B-Side") == (2, 2) + + +def test_no_match_returns_none(): + assert resolve_track_position_in_album(_album_12(), title="Not On This Album") == (None, None) + assert resolve_track_position_in_album([], title="x") == (None, None) + assert resolve_track_position_in_album(None, title="x") == (None, None) + + +def test_skips_entries_without_a_valid_position(): + tracks = [ + {"id": "t1", "name": "Obelisk", "track_number": 0}, # 0 -> skip + {"id": "t2", "name": "Obelisk", "track_number": None}, # None -> skip + {"id": "t3", "name": "Obelisk", "track_number": "junk"}, # junk -> skip + ] + assert resolve_track_position_in_album(tracks, title="Obelisk") == (None, None) + + +# ── integration wrapper (fail-safe, real album lookup) ─────────────────────── + +def test_wrapper_resolves_from_source_album(monkeypatch): + import core.imports.context as ctx + monkeypatch.setattr("core.metadata.album_tracks.get_album_tracks_for_source", + lambda source, album_id: {"tracks": _album_12()}) + context = {"source": "deezer", + "track_info": {"id": "t9", "name": "Obelisk", "deezer_album_id": "232620572"}} + tn, dn = ctx._resolve_album_position_from_source(context, {}, 1) + assert tn == 9 and dn == 1 + + +def test_wrapper_is_failsafe_on_empty_or_missing(monkeypatch): + import core.imports.context as ctx + # no album id at all -> keeps current disc, no number + assert ctx._resolve_album_position_from_source({"source": "deezer", "track_info": {}}, {}, 1) == (None, 1) + # fetcher returns nothing -> fail-safe + monkeypatch.setattr("core.metadata.album_tracks.get_album_tracks_for_source", + lambda source, album_id: {"tracks": []}) + context = {"source": "deezer", "track_info": {"name": "Obelisk", "deezer_album_id": "x"}} + assert ctx._resolve_album_position_from_source(context, {}, 2) == (None, 2) + + +def test_wrapper_never_raises_on_fetch_error(monkeypatch): + import core.imports.context as ctx + def _boom(source, album_id): + raise RuntimeError("deezer down") + monkeypatch.setattr("core.metadata.album_tracks.get_album_tracks_for_source", _boom) + context = {"source": "deezer", "track_info": {"name": "Obelisk", "deezer_album_id": "x"}} + assert ctx._resolve_album_position_from_source(context, {}, 1) == (None, 1) From 49592f898c3e5f12b2d46cd2f794406ad4229e9d Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Mon, 22 Jun 2026 14:41:04 -0700 Subject: [PATCH 08/23] =?UTF-8?q?#902:=20YouTube=20Liked=20Music=20sync=20?= =?UTF-8?q?=E2=80=94=20paste=20a=20cookies.txt=20(server/Docker=20auth)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Private YT Music playlists (a user's Liked Music, list=LM) need auth, but the only cookie option was cookiesfrombrowser — a browser on the same machine as SoulSync, useless on a headless/Docker box (and locked to whatever account that browser happens to be signed into). Add a 'Paste cookies.txt' mode so users can supply the exact session they want from any machine. - core/youtube_cookies.py: pure seam — build_youtube_cookie_opts (cookiefile vs cookiesfrombrowser precedence, mutually exclusive, fail-safe on a missing file), looks_like_cookiefile (needs a real cookie row; rejects junk/header-only), write_pasted_cookiefile (validate + 0600 write; blank/junk never clobbers a saved file). - _youtube_cookie_opts() delegates to the seam, so every yt-dlp call site gets it. - /api/settings pops cookies_paste before the generic persist, validates (400 on junk), writes config/youtube_cookies.txt, stores only the path (blob never hits config.json). - Settings dropdown gains 'Paste cookies.txt'; selecting it reveals a textarea. - tests/test_youtube_cookies.py: precedence, validation, fail-safe write (11 tests). --- .gitignore | 1 + core/youtube_cookies.py | 105 ++++++++++++++++++++++++++++++++++ tests/test_youtube_cookies.py | 100 ++++++++++++++++++++++++++++++++ web_server.py | 35 +++++++++--- webui/index.html | 16 +++++- webui/static/settings.js | 22 +++++++ 6 files changed, 271 insertions(+), 8 deletions(-) create mode 100644 core/youtube_cookies.py create mode 100644 tests/test_youtube_cookies.py diff --git a/.gitignore b/.gitignore index 8857f8d5..6e78b278 100644 --- a/.gitignore +++ b/.gitignore @@ -12,6 +12,7 @@ __pycache__/ # User-specific files (auto-created by the app if missing) config/config.json +config/youtube_cookies.txt database/music_library.db database/music_library.db-shm database/music_library.db-wal diff --git a/core/youtube_cookies.py b/core/youtube_cookies.py new file mode 100644 index 00000000..c61086fe --- /dev/null +++ b/core/youtube_cookies.py @@ -0,0 +1,105 @@ +"""YouTube cookie options for yt-dlp — a browser store *or* a pasted cookies.txt. + +Settings → YouTube offers two ways to authenticate yt-dlp: + +* a **browser dropdown** (Chrome/Firefox/…) → yt-dlp ``cookiesfrombrowser``, which + reads a logged-in browser's cookie store *on the same machine as SoulSync*. Great + for local installs, useless on a headless server / Docker box (no browser there). +* a **"Paste cookies.txt"** mode → yt-dlp ``cookiefile``, a Netscape-format cookie + file the user exports (e.g. with a "Get cookies.txt LOCALLY" extension) and pastes + in. This is the only path that works for server/Docker users, and it's what makes + *private* playlists — a user's "Liked Music" (``list=LM``) — actually visible. + +This module centralises the precedence and the pasted-file validation so the live +opts (:func:`build_youtube_cookie_opts`) and the settings-save write agree, and so +the seam is unit-testable without I/O. The web layer owns *where* the file lives +(next to ``config.json``); this module only decides the opts and validates content. +""" + +from __future__ import annotations + +import os +from typing import Any, Dict + +# Sentinel dropdown value meaning "use a pasted cookies.txt file" rather than a +# browser name. Anything else non-empty is treated as a browser for cookiesfrombrowser. +PASTE_MODE = "custom" + + +def build_youtube_cookie_opts( + mode: Any, + cookiefile_path: str = "", + *, + cookiefile_exists: bool = False, +) -> Dict[str, Any]: + """Return the yt-dlp cookie options for a given Settings→YouTube ``mode``. Pure. + + * ``mode == PASTE_MODE`` → ``{'cookiefile': path}`` when the file exists, else + ``{}`` (a stale/missing path must never become a broken cookiefile arg). + * ``mode`` is any other non-empty string → ``{'cookiesfrombrowser': (mode,)}``. + * ``mode`` falsy → ``{}`` (anonymous; public playlists only). + + Precedence is structural: a browser name is never ``PASTE_MODE``, so the two + cookie sources can't both be emitted. No I/O here — the caller passes + ``cookiefile_exists`` (the ``os.path.exists`` result) so this stays pure. + """ + m = str(mode or "").strip() + if m == PASTE_MODE: + if cookiefile_path and cookiefile_exists: + return {"cookiefile": str(cookiefile_path)} + return {} + if m: + return {"cookiesfrombrowser": (m,)} + return {} + + +def looks_like_cookiefile(content: Any) -> bool: + """True when ``content`` plausibly is a Netscape/Mozilla ``cookies.txt``. + + Requires at least one real cookie row — a non-comment line with >= 6 TAB-separated + fields (domain, flag, path, secure, expiry, name[, value]). The ``# Netscape HTTP + Cookie File`` header alone is NOT enough: a header-only paste carries no auth and + would silently save a useless file. This guards the save path so pasting junk (a + URL, JSON, or just the header) is rejected up front instead of being written out + and making yt-dlp raise mid-extraction. + """ + if not content or not isinstance(content, str): + return False + for raw in content.splitlines(): + line = raw.rstrip("\n") + if not line or line.lstrip().startswith("#"): + continue + if len(line.split("\t")) >= 6: + return True + return False + + +def write_pasted_cookiefile(content: Any, dest_path: str) -> str: + """Validate + write a pasted ``cookies.txt`` to ``dest_path``. + + Returns the written path on success, or ``""`` when the content is empty / + doesn't look like a cookie file / can't be written — in which case the caller + leaves any existing file untouched (a blank save must not wipe a saved cookie). + Best-effort ``0600`` perms since the file holds live session secrets. + """ + if not looks_like_cookiefile(content): + return "" + try: + text = content if content.endswith("\n") else content + "\n" + with open(dest_path, "w", encoding="utf-8") as fh: + fh.write(text) + try: + os.chmod(dest_path, 0o600) + except OSError: + pass + return str(dest_path) + except OSError: + return "" + + +__all__ = [ + "PASTE_MODE", + "build_youtube_cookie_opts", + "looks_like_cookiefile", + "write_pasted_cookiefile", +] diff --git a/tests/test_youtube_cookies.py b/tests/test_youtube_cookies.py new file mode 100644 index 00000000..0d7d3a04 --- /dev/null +++ b/tests/test_youtube_cookies.py @@ -0,0 +1,100 @@ +"""Settings → YouTube cookie options: browser store vs a pasted cookies.txt. + +#902: syncing a YouTube *Music* "Liked Music" playlist (list=LM) needs auth, and on +a server/Docker box there's no local browser for cookiesfrombrowser to read — so we +let users paste a cookies.txt (yt-dlp cookiefile). These pin the precedence (so the +two cookie sources can never both be emitted), the paste validation (junk must not be +written out and break yt-dlp), and the fail-safe write (a blank save never wipes a +saved file). +""" + +from __future__ import annotations + +from core.youtube_cookies import ( + PASTE_MODE, + build_youtube_cookie_opts, + looks_like_cookiefile, + write_pasted_cookiefile, +) + +NETSCAPE = ( + "# Netscape HTTP Cookie File\n" + ".youtube.com\tTRUE\t/\tTRUE\t1999999999\tLOGIN_INFO\tsecretvalue\n" + ".youtube.com\tTRUE\t/\tTRUE\t1999999999\tSID\tanother\n" +) + + +# ── precedence (pure opts) ────────────────────────────────────────────────── + +def test_empty_mode_is_anonymous(): + assert build_youtube_cookie_opts("") == {} + assert build_youtube_cookie_opts(None) == {} + + +def test_browser_mode_uses_cookiesfrombrowser(): + assert build_youtube_cookie_opts("firefox") == {"cookiesfrombrowser": ("firefox",)} + + +def test_paste_mode_uses_cookiefile_when_present(): + opts = build_youtube_cookie_opts(PASTE_MODE, "/cfg/youtube_cookies.txt", cookiefile_exists=True) + assert opts == {"cookiefile": "/cfg/youtube_cookies.txt"} + + +def test_paste_mode_without_a_real_file_is_anonymous_not_broken(): + # stale/missing path must NOT become a cookiefile arg yt-dlp would choke on + assert build_youtube_cookie_opts(PASTE_MODE, "/cfg/gone.txt", cookiefile_exists=False) == {} + assert build_youtube_cookie_opts(PASTE_MODE, "", cookiefile_exists=True) == {} + + +def test_sources_are_mutually_exclusive(): + # a browser name is never PASTE_MODE, so cookiefile + cookiesfrombrowser can't co-occur + for mode in ("chrome", "firefox", PASTE_MODE, ""): + opts = build_youtube_cookie_opts(mode, "/x.txt", cookiefile_exists=True) + assert not ("cookiefile" in opts and "cookiesfrombrowser" in opts) + + +# ── paste validation ──────────────────────────────────────────────────────── + +def test_accepts_netscape_header_and_cookie_rows(): + assert looks_like_cookiefile(NETSCAPE) is True + # no header but a valid tab-separated cookie row still counts + assert looks_like_cookiefile(".youtube.com\tTRUE\t/\tTRUE\t123\tSID\tv") is True + + +def test_rejects_junk_paste(): + assert looks_like_cookiefile("") is False + assert looks_like_cookiefile(" ") is False + assert looks_like_cookiefile(None) is False + assert looks_like_cookiefile("https://music.youtube.com/playlist?list=LM") is False + assert looks_like_cookiefile('{"cookies": []}') is False + assert looks_like_cookiefile("# Netscape HTTP Cookie File\n# only comments\n") is False + + +# ── fail-safe write ───────────────────────────────────────────────────────── + +def test_write_persists_valid_cookiefile(tmp_path): + dest = tmp_path / "youtube_cookies.txt" + out = write_pasted_cookiefile(NETSCAPE, str(dest)) + assert out == str(dest) + assert dest.read_text().startswith("# Netscape HTTP Cookie File") + + +def test_write_appends_trailing_newline(tmp_path): + dest = tmp_path / "c.txt" + write_pasted_cookiefile(NETSCAPE.rstrip("\n"), str(dest)) + assert dest.read_text().endswith("\n") + + +def test_write_refuses_junk_and_leaves_no_file(tmp_path): + dest = tmp_path / "c.txt" + assert write_pasted_cookiefile("not a cookie file", str(dest)) == "" + assert not dest.exists() + + +def test_write_refuses_junk_without_clobbering_existing(tmp_path): + # a blank/garbage save must NOT wipe a previously-saved cookie file + dest = tmp_path / "c.txt" + write_pasted_cookiefile(NETSCAPE, str(dest)) + before = dest.read_text() + assert write_pasted_cookiefile("", str(dest)) == "" + assert dest.read_text() == before diff --git a/web_server.py b/web_server.py index 7be627af..dc957222 100644 --- a/web_server.py +++ b/web_server.py @@ -3126,6 +3126,22 @@ def handle_settings(): f"in Manage Profiles first.", "members_without_password": _stranded}), 400 + # YouTube pasted cookies.txt (server/Docker path): pull it out BEFORE the + # generic persist so the raw cookie blob never lands in config.json — it's + # secret + bulky. We validate up front and store only a file path. + _yt_in = new_settings.get('youtube') + _yt_paste = _yt_in.pop('cookies_paste', None) if isinstance(_yt_in, dict) else None + if _yt_paste is not None and str(_yt_paste).strip(): + from core.youtube_cookies import looks_like_cookiefile, write_pasted_cookiefile + if not looks_like_cookiefile(_yt_paste): + return jsonify({"success": False, + "error": "That doesn't look like a cookies.txt file. Export it " + "with a 'Get cookies.txt LOCALLY' browser extension and " + "paste the whole file."}), 400 + _cookie_path = str(config_manager.config_path.parent / "youtube_cookies.txt") + if write_pasted_cookiefile(_yt_paste, _cookie_path): + config_manager.set('youtube.cookies_file', _cookie_path) + if 'active_media_server' in new_settings: config_manager.set_active_media_server(new_settings['active_media_server']) @@ -14910,15 +14926,20 @@ def clean_youtube_artist(artist_string): def _youtube_cookie_opts(): """yt-dlp cookie options matching the rest of the app (Settings → YouTube). - Per-video extraction needs these to get past YouTube's bot checks.""" - opts = {} + + Per-video extraction needs these to get past YouTube's bot checks, and private + playlists (a user's "Liked Music", list=LM) need them to be visible at all. The + dropdown is either a browser name (cookiesfrombrowser, local installs) or the + PASTE_MODE sentinel, in which case we point yt-dlp at the pasted cookies.txt that + server/Docker users supply. Precedence + emptiness live in core.youtube_cookies.""" + from core.youtube_cookies import build_youtube_cookie_opts try: - cb = config_manager.get('youtube.cookies_browser', '') - if cb: - opts['cookiesfrombrowser'] = (cb,) + mode = config_manager.get('youtube.cookies_browser', '') + path = config_manager.get('youtube.cookies_file', '') + exists = bool(path) and os.path.exists(path) + return build_youtube_cookie_opts(mode, path, cookiefile_exists=exists) except Exception: # noqa: S110 - cookie config is best-effort; resolve still works without it - pass - return opts + return {} def _fetch_youtube_video_artist(video_id, cookie_opts): diff --git a/webui/index.html b/webui/index.html index 5f1a3d3d..970255ba 100644 --- a/webui/index.html +++ b/webui/index.html @@ -5038,10 +5038,24 @@ +
If YouTube shows "Sign in to confirm you're not a bot", select your browser here. - SoulSync will use your browser's YouTube cookies to authenticate. + SoulSync will use your browser's YouTube cookies to authenticate. Reading a browser + only works when that browser is on the same machine as SoulSync — on a + server/Docker box, choose Paste cookies.txt instead. +
+ +
diff --git a/webui/static/settings.js b/webui/static/settings.js index 8c550783..babf7463 100644 --- a/webui/static/settings.js +++ b/webui/static/settings.js @@ -1223,6 +1223,25 @@ async function loadSettingsData() { // Populate YouTube settings document.getElementById('youtube-cookies-browser').value = settings.youtube?.cookies_browser || ''; document.getElementById('youtube-download-delay').value = settings.youtube?.download_delay ?? 3; + // Show the cookies.txt paste box only in "custom" mode. We never echo the + // stored cookie back to the UI (it's secret + lives in a file, not config); + // if one is already saved, say so via placeholder so a blank save won't wipe it. + const _ytCookieSel = document.getElementById('youtube-cookies-browser'); + const _ytPasteBox = document.getElementById('youtube-cookies-paste'); + const _ytPasteGroup = document.getElementById('youtube-cookies-paste-group'); + if (_ytCookieSel && _ytPasteGroup) { + const _toggleYtPaste = () => { + _ytPasteGroup.style.display = _ytCookieSel.value === 'custom' ? '' : 'none'; + }; + if (_ytPasteBox && settings.youtube?.cookies_file) { + _ytPasteBox.placeholder = 'A cookies.txt is saved. Paste again to replace it, or leave blank to keep it.'; + } + _toggleYtPaste(); + if (!_ytCookieSel.dataset.pasteToggleBound) { + _ytCookieSel.addEventListener('change', _toggleYtPaste); + _ytCookieSel.dataset.pasteToggleBound = '1'; + } + } // Update UI based on download source mode updateDownloadSourceUI(); @@ -3223,6 +3242,9 @@ async function saveSettings(quiet = false) { youtube: { cookies_browser: document.getElementById('youtube-cookies-browser').value, download_delay: parseInt(document.getElementById('youtube-download-delay').value) || 3, + // Raw cookies.txt blob — backend validates, writes it to a file, and stores + // only the path (never echoed back). Blank = keep any already-saved file. + cookies_paste: document.getElementById('youtube-cookies-paste')?.value || '', }, security: { require_pin_on_launch: document.getElementById('security-require-pin')?.checked || false, From 9454970a83214a1e1e2fd887349adb0ad7842178 Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Mon, 22 Jun 2026 15:01:49 -0700 Subject: [PATCH 09/23] perf(dashboard): sidebar header sweep animates transform, not left MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The .sidebar-header::after ambient sweep animated `left` (-100% -> 140%) on an 8s infinite loop — forcing a layout recalc every frame it's on screen, on the sidebar that's present on every page. Convert to transform: translateX() with a pixel-identical travel path (element is 60% of header width, so translateX(400%) == the old 240%-of-header sweep) + will-change. Compositor-only now; no per-frame layout. Zero visual change. --- webui/static/style.css | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/webui/static/style.css b/webui/static/style.css index 456d6a41..432bc341 100644 --- a/webui/static/style.css +++ b/webui/static/style.css @@ -340,14 +340,20 @@ body.reduce-effects .sidebar::after { rgba(var(--accent-rgb), 0.06) 60%, transparent 100% ); + /* Animate transform (compositor-only) instead of `left` (which forced a + layout recalc every frame). The element width is 60% of the header, so + translateX(400%) = 240% of header width — the same travel `left:-100%` + → `left:140%` produced. Pixel-identical path, no per-frame layout. */ + transform: translateX(0); + will-change: transform, opacity; animation: sidebar-header-sweep 8s ease-in-out infinite; pointer-events: none; } @keyframes sidebar-header-sweep { - 0%, 100% { left: -100%; opacity: 0; } + 0%, 100% { transform: translateX(0); opacity: 0; } 10% { opacity: 1; } - 50% { left: 140%; opacity: 1; } + 50% { transform: translateX(400%); opacity: 1; } 60% { opacity: 0; } } From b5b71df3facb7a318c5bc9820e42431cfe0eca82 Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Mon, 22 Jun 2026 15:02:19 -0700 Subject: [PATCH 10/23] perf(dashboard): trim blur radii (orbs 40->28px, header backdrop 28->18px) GPU fill/blur cost scales with radius. The sidebar aura orbs already fade to transparent at 70% of their gradient, so dropping their blur 40->28px shrinks the composited bounding box with no perceptible softness loss. The frosted header's backdrop-filter is re-blurred whenever the orbs drift behind it; 28->18px cuts that per-frame work ~a third while keeping the frosted look. Relief is biggest on weak GPUs (the machines people complain about). --- webui/static/style.css | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/webui/static/style.css b/webui/static/style.css index 432bc341..ac2efa01 100644 --- a/webui/static/style.css +++ b/webui/static/style.css @@ -108,7 +108,10 @@ body { position: fixed; pointer-events: none; border-radius: 50%; - filter: blur(40px); + /* The radial-gradient already fades to transparent at 70%, so a smaller blur + still reads as a soft aura — but shrinks the composited (orb + feather) + bounding box and the blur work. 40px -> 28px: ~imperceptible, cheaper fill. */ + filter: blur(28px); will-change: transform, opacity; } @@ -317,8 +320,11 @@ body.reduce-effects .sidebar::after { accent-gradient top now read as a soft blur instead of bleeding through sharply. (Only visible where the background is translucent — the opaque base toward the bottom still stops any bleed.) */ - backdrop-filter: blur(28px) saturate(1.3); - -webkit-backdrop-filter: blur(28px) saturate(1.3); + /* Backdrop blur is re-evaluated whenever content drifts behind it (the + ambient orbs do). Cost scales with radius — 28px -> 18px keeps the frosted + read while cutting the per-frame blur work by ~a third. */ + backdrop-filter: blur(18px) saturate(1.3); + -webkit-backdrop-filter: blur(18px) saturate(1.3); /* Subtle inner glow */ box-shadow: inset 0 1px 0 rgba(255, 255, 255, 0.1); From 4b58d8079ee4fc94f3a884a49d346ffc2851dac6 Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Mon, 22 Jun 2026 15:12:25 -0700 Subject: [PATCH 11/23] perf(dashboard): auto-enable performance mode on weak hardware (device-scoped) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit People report SoulSync working their machine hard at idle. On likely-weak devices (<=2 cores, or <=2GB, or low on both: <=4 cores AND <=4GB) auto-enable reduce-effects once and toast why ('lower-power device — turn effects back on in Settings'). Device-scoped via localStorage on purpose: a weak laptop must not flip the server setting for the user's other machines. Acts only when this device has no stored preference (null), so it runs at most once and never overrides an explicit choice. Conservative thresholds avoid flagging capable boxes (a 4-core/8GB laptop isn't touched; Firefox/Safari, which don't expose deviceMemory, only trip on <=2 cores). Settings-load now prefers the device-level localStorage value over the server default, so opening Settings no longer clobbers the per-device (auto or manual) choice. --- webui/static/init.js | 41 ++++++++++++++++++++++++++++++++++++++++ webui/static/settings.js | 10 ++++++++-- 2 files changed, 49 insertions(+), 2 deletions(-) diff --git a/webui/static/init.js b/webui/static/init.js index a8a11b98..aea68fab 100644 --- a/webui/static/init.js +++ b/webui/static/init.js @@ -213,6 +213,47 @@ function applyReduceEffects(enabled) { // Bootstrap accent and reduce-effects from localStorage instantly (prevents flash) (function () { + // Auto performance mode on likely-weak hardware. Only acts when this device has + // NO stored preference yet (null) — so it runs at most once and never overrides + // a choice the user (or a prior auto-run) made. Device-scoped via localStorage on + // purpose: a weak laptop shouldn't flip the server setting for the user's other + // machines. Mobile already disables these effects elsewhere, so skip it here. + if (localStorage.getItem('soulsync-reduce-effects') === null) { + const ua = navigator.userAgent || ''; + const isMobile = window.innerWidth <= 768 || /Mobi|Android|iPhone|iPad|iPod/i.test(ua); + const cores = navigator.hardwareConcurrency || 0; // widely supported + const mem = navigator.deviceMemory || 0; // Chromium only; 0 elsewhere + // Conservative — avoid flagging capable machines: <=2 cores, or <=2GB, or a + // low-mid box that's low on BOTH (<=4 cores AND <=4GB). A 4-core/8GB laptop + // (mem>4) is NOT flagged; Firefox/Safari (mem unknown) only trip on <=2 cores. + const weak = !isMobile && ( + (cores > 0 && cores <= 2) || + (mem > 0 && mem <= 2) || + (cores > 0 && cores <= 4 && mem > 0 && mem <= 4) + ); + if (weak) { + localStorage.setItem('soulsync-reduce-effects', '1'); + window._autoPerfModeApplied = true; // show the explainer toast once the UI is up + } + } + + if (window._autoPerfModeApplied) { + // Toast lives in downloads.js (loaded separately) — retry until it's defined. + const fireToast = (tries) => { + if (typeof showToast === 'function') { + showToast('Performance mode is on — this looks like a lower-power device. ' + + 'Turn effects back on in Settings → Appearance.', 'info'); + } else if (tries < 40) { + setTimeout(() => fireToast(tries + 1), 250); + } + }; + if (document.readyState === 'loading') { + document.addEventListener('DOMContentLoaded', () => fireToast(0)); + } else { + fireToast(0); + } + } + if (localStorage.getItem('soulsync-reduce-effects') === '1') { document.body.classList.add('reduce-effects'); window._reduceEffectsActive = true; diff --git a/webui/static/settings.js b/webui/static/settings.js index babf7463..0711e14b 100644 --- a/webui/static/settings.js +++ b/webui/static/settings.js @@ -1388,8 +1388,14 @@ async function loadSettingsData() { if (workerOrbsCheckbox) workerOrbsCheckbox.checked = workerOrbsEnabled; applyWorkerOrbsSetting(workerOrbsEnabled); - // Reduce effects toggle - const reduceEffects = settings.ui_appearance?.reduce_effects === true; // default false + // Reduce effects toggle. This flag is device-scoped: localStorage (set by the + // live toggle and by weak-hardware auto-detect) is the source of truth for THIS + // machine; the server value is only the cross-device default used when this + // device has never chosen. Prefer localStorage when present so opening Settings + // doesn't clobber an auto-enabled (or manually-set) per-device choice. + const serverReduce = settings.ui_appearance?.reduce_effects === true; // default false + const localReduce = localStorage.getItem('soulsync-reduce-effects'); // '1' | '0' | null + const reduceEffects = localReduce !== null ? (localReduce === '1') : serverReduce; const reduceCheckbox = document.getElementById('reduce-effects-enabled'); if (reduceCheckbox) reduceCheckbox.checked = reduceEffects; applyReduceEffects(reduceEffects); From b73389adea32a1c0fd6af709c7928624ccafe9de Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Mon, 22 Jun 2026 15:23:16 -0700 Subject: [PATCH 12/23] perf(dashboard): cache particle glow sprites instead of per-frame gradients MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The dashboard particle preset built a fresh createRadialGradient + arc-fill for all 50 glows every frame. But each glow's gradient is a fixed size/colour for the particle's life — only the pulse ALPHA changes per frame, and canvas multiplies image alpha by ctx.globalAlpha. So bake the gradient once into a per-particle offscreen canvas (full alpha) and drawImage it each frame with globalAlpha = pulse (times the incoming globalAlpha, so transition fades stay identical). Rebuilt only when the accent colour changes. Pixel-identical output: same colour, same linear falloff, same source-over; sprite rendered at ceil(glowSize) then downscaled to exact glowSize. Drops 50 gradient allocations + arc-fills per frame to 50 cached blits. Scoped to the dashboard preset only (smallest blast radius); other presets untouched. --- webui/static/particles.js | 41 +++++++++++++++++++++++++++++++-------- 1 file changed, 33 insertions(+), 8 deletions(-) diff --git a/webui/static/particles.js b/webui/static/particles.js index 2b446806..4869bfdb 100644 --- a/webui/static/particles.js +++ b/webui/static/particles.js @@ -101,6 +101,27 @@ // Each preset: { count, init(p, i), update(p, time, i), draw(p, ctx, accent, time), // optional: initExtras(), drawGlobal(ctx, particles, accent, time, extras) } + // Pre-rendered glow sprite. A dashboard particle's glow is a radial gradient + // (its accent colour -> transparent) of a SIZE that's fixed for the particle's + // life — the only thing changing each frame is the pulse ALPHA. Since canvas + // multiplies fillStyle/image alpha by ctx.globalAlpha, we can bake the gradient + // ONCE into an offscreen canvas at full alpha and drawImage it each frame with + // globalAlpha = pulse, instead of building a fresh createRadialGradient + arc-fill + // for all 50 particles every frame. Output is pixel-identical; just far cheaper. + function buildGlowSprite(col, glowSize) { + const s = Math.max(1, Math.ceil(glowSize)); + const c = document.createElement('canvas'); + c.width = s * 2; + c.height = s * 2; + const g = c.getContext('2d'); + const grad = g.createRadialGradient(s, s, 0, s, s, s); + grad.addColorStop(0, `rgba(${col}, 1)`); + grad.addColorStop(1, 'rgba(0,0,0,0)'); + g.fillStyle = grad; + g.fillRect(0, 0, s * 2, s * 2); + return c; + } + const PRESETS = { // ── DASHBOARD — network nodes, connections, data packets, shooting stars ── @@ -130,16 +151,20 @@ const pulse = 0.5 + 0.5 * Math.sin(time * 1.5 + p.phase); const col = shiftAccent(accent, p.hueShift); - // Glow — hubs get bigger, brighter glow + // Glow — hubs get bigger, brighter glow. Cached sprite blit (alpha at + // blit time); pixel-identical to the old per-frame radial gradient. const glowSize = p.isHub ? p.radius * 6 : p.radius * 4; const glowAlpha = p.isHub ? (0.18 + pulse * 0.12) : (0.10 + pulse * 0.07); - const glow = ctx.createRadialGradient(p.x, p.y, 0, p.x, p.y, glowSize); - glow.addColorStop(0, `rgba(${col}, ${glowAlpha})`); - glow.addColorStop(1, 'rgba(0,0,0,0)'); - ctx.beginPath(); - ctx.arc(p.x, p.y, glowSize, 0, Math.PI * 2); - ctx.fillStyle = glow; - ctx.fill(); + if (!p._glowCanvas || p._glowCol !== col || p._glowR !== glowSize) { + p._glowCanvas = buildGlowSprite(col, glowSize); // rebuilt only on accent change + p._glowCol = col; + p._glowR = glowSize; + } + // Multiply by the incoming globalAlpha so transition fades match exactly. + const _glowPrevAlpha = ctx.globalAlpha; + ctx.globalAlpha = _glowPrevAlpha * glowAlpha; + ctx.drawImage(p._glowCanvas, p.x - glowSize, p.y - glowSize, glowSize * 2, glowSize * 2); + ctx.globalAlpha = _glowPrevAlpha; // Core const coreAlpha = p.isHub ? (0.5 + pulse * 0.3) : (0.3 + pulse * 0.2); From df6815c2cc9d3bfb22eac58b70dad022c50ecffb Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Mon, 22 Jun 2026 17:10:40 -0700 Subject: [PATCH 13/23] perf(dashboard): remove invisible card blur + redundant shadow layers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Profiling the actually-painted dashboard found two pure-waste GPU costs (no visual payoff), reclaimable with zero degradation: - backdrop-filter on cards whose backgrounds are already 90-99% opaque, so the blur is invisible: service-card (x3), stat-card-dashboard (x3), activity-feed-container. Dropped the filter, nudged opacity to ~0.97 so the unblurred sliver is imperceptible. - redundant/near-invisible box-shadow layers on the two biggest elements: page-shell (near-fullscreen — collapsed two stacked outer shadows to one) and sidebar (dropped a duplicate layer + a 0 0 60px accent glow at 6% opacity that's barely visible but a costly 60px-blur pass). Targets the DURING-USE cost, not idle. sidebar-header keeps its blur (genuinely translucent), and the cursor blob is untouched (that one's a real visual tradeoff). --- webui/static/style.css | 31 +++++++++++++++++-------------- 1 file changed, 17 insertions(+), 14 deletions(-) diff --git a/webui/static/style.css b/webui/static/style.css index ac2efa01..c43e4c74 100644 --- a/webui/static/style.css +++ b/webui/static/style.css @@ -82,11 +82,11 @@ body { border-top: 1px solid rgba(255, 255, 255, 0.12); border-bottom-right-radius: 24px; - /* Soft floating shadow with inner glow */ + /* Soft floating shadow with inner glow — dropped the 0 4px 16px duplicate and the + 0 0 60px accent glow (60px blur at 6% opacity: nearly invisible but a costly + shadow pass). One outer + the two insets read the same. */ box-shadow: - 0 8px 32px rgba(0, 0, 0, 0.3), - 0 4px 16px rgba(0, 0, 0, 0.2), - 0 0 60px rgba(var(--accent-rgb), 0.06), + 0 8px 30px rgba(0, 0, 0, 0.34), inset 0 1px 0 rgba(255, 255, 255, 0.1), inset 0 -1px 0 rgba(0, 0, 0, 0.2); @@ -8256,10 +8256,11 @@ body.helper-mode-active #dashboard-activity-feed:hover { border: 1px solid rgba(255, 255, 255, 0.08); border-top: 1px solid rgba(255, 255, 255, 0.12); margin: 20px; - /* Soft floating shadow */ + /* Soft floating shadow — one outer layer instead of two stacked (this is a + near-fullscreen element, so each shadow pass is expensive); the single + slightly-stronger blur reads the same. */ box-shadow: - 0 8px 32px rgba(0, 0, 0, 0.3), - 0 4px 16px rgba(0, 0, 0, 0.2), + 0 8px 28px rgba(0, 0, 0, 0.34), inset 0 1px 0 rgba(255, 255, 255, 0.08); } @@ -8915,8 +8916,9 @@ body.helper-mode-active #dashboard-activity-feed:hover { } .service-card { - background: rgba(18, 18, 22, 0.9); - backdrop-filter: blur(12px); + /* backdrop-filter dropped: the 90% opaque bg made blur(12px) invisible (x3 cards, + pure GPU waste). Bumped to 0.97 so the now-unblurred sliver is imperceptible. */ + background: rgba(18, 18, 22, 0.97); border-radius: 16px; border: 1px solid rgba(255, 255, 255, 0.06); padding: 20px 22px; @@ -9687,8 +9689,9 @@ body.helper-mode-active #dashboard-activity-feed:hover { } .stat-card-dashboard { - background: rgba(18, 18, 22, 0.9); - backdrop-filter: blur(12px); + /* backdrop-filter dropped: 90% opaque bg made blur(12px) invisible (x3, GPU waste); + 0.97 keeps it looking solid. */ + background: rgba(18, 18, 22, 0.97); border-radius: 16px; border: 1px solid rgba(255, 255, 255, 0.06); padding: 20px; @@ -10094,9 +10097,9 @@ body.helper-mode-active #dashboard-activity-feed:hover { .activity-feed-container { /* Premium glassmorphic foundation */ background: linear-gradient(135deg, - rgba(26, 26, 26, 0.95) 0%, - rgba(18, 18, 18, 0.98) 100%); - backdrop-filter: blur(12px) saturate(1.1); + rgba(26, 26, 26, 0.97) 0%, + rgba(18, 18, 18, 0.99) 100%); + /* backdrop-filter dropped: 95-99% opaque bg already hid the blur (GPU waste). */ border-radius: 16px; border: 1px solid rgba(255, 255, 255, 0.08); border-top: 1px solid rgba(255, 255, 255, 0.12); From 01c51a3c0e8d9691f03f51dd49e69e20caad4807 Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Mon, 22 Jun 2026 17:53:49 -0700 Subject: [PATCH 14/23] #904: guard standalone Deep Scan against relocating a desynced library MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Standalone _run_soulsync_deep_scan did a path-only diff (untracked = transfer files not in the soulsync DB) and shutil.move'd EVERY untracked file to Staging — no guard. When the DB is empty/out of sync with disk (volume swap, DB reset, external Picard tag edits) but Transfer holds the real library, that flags the whole library as untracked and relocates all of it; Phase 5 then deletes the rows, and with Staging cleanup on the files are gone for good. Reporter lost ~1,500 tracks into Staging. The stale_guard the orphan detector + media-server deep scan already use (#828, #908) was never wired into this path. Fix: - core/library/standalone_scan.py (pure, tested): plan_standalone_deep_scan() diffs untracked (separator-normalized) and decides whether the move is safe. Blocks when the untracked share is implausibly large (>20 files AND >50% of Transfer — the desync signature, via is_implausible_orphan_flood) or when the user marked Transfer permanent. A normal batch of new arrivals still moves. - web_server: consult the planner before Phase 4; on block, move NOTHING, leave files in place, and surface a loud warning + activity item. Guard Phase 5 deletes too (skip on desync-block or implausible stale share). - 'Transfer is my permanent library — never move files out' toggle (import.transfer_is_permanent) in Settings. - tests/library/test_standalone_scan.py: seam coverage + the #904 regression (empty DB + 1,500 files -> blocked, nothing moved). No behavior change for in-sync libraries; the guard only trips on the desync pattern. --- core/library/standalone_scan.py | 104 +++++++++++++++++++++++ tests/library/test_standalone_scan.py | 115 ++++++++++++++++++++++++++ web_server.py | 61 ++++++++++++-- webui/index.html | 15 ++++ webui/static/settings.js | 3 + 5 files changed, 289 insertions(+), 9 deletions(-) create mode 100644 core/library/standalone_scan.py create mode 100644 tests/library/test_standalone_scan.py diff --git a/core/library/standalone_scan.py b/core/library/standalone_scan.py new file mode 100644 index 00000000..014ca36f --- /dev/null +++ b/core/library/standalone_scan.py @@ -0,0 +1,104 @@ +"""Decision logic for the SoulSync standalone Deep Scan's untracked → Staging move. + +The standalone deep scan (``_run_soulsync_deep_scan`` in web_server) walks the +Transfer folder, diffs it against the ``soulsync`` rows in the DB, and relocates +every file it can't find a DB record for into Staging for auto-import. That's fine +when Transfer is a scratch/landing area — files arrive, get moved, imported, and +recorded, so a later scan only ever sees a few genuinely-new arrivals. + +It is a DATA-LOSS trap when the DB is empty or out of sync with disk (a volume +swap, a DB reset, external tag edits) while Transfer holds the user's real library: +a path-only diff then flags the *entire* library as "untracked" and the scan +relocates all of it (issue #904). The same failure mode the orphan detector and the +media-server deep scan already guard against (``core.library.stale_guard``) — this +path just never used the guard. + +This module is the pure, testable decision: given the Transfer file set, the DB's +known paths, and the user's "Transfer is my permanent library" preference, decide +WHICH files are untracked and WHETHER it's safe to relocate them. The web layer does +only the I/O (walk/move/delete) based on the returned plan. +""" + +from __future__ import annotations + +from typing import Iterable, Set + +from core.library.stale_guard import ( + DEFAULT_MAX_ORPHAN_FRACTION, + DEFAULT_MIN_ORPHANS, + is_implausible_orphan_flood, +) + +# Block reason codes (web layer turns these into a user-facing warning). +BLOCK_NONE = "" +BLOCK_TRANSFER_PERMANENT = "transfer_permanent" +BLOCK_DESYNC = "desync" + + +def _norm(path: str) -> str: + """Normalize a path for cross-platform comparison (Windows vs Unix separators).""" + return str(path).replace("\\", "/") + + +def diff_untracked(transfer_files: Iterable[str], db_paths: Iterable[str]) -> Set[str]: + """Files present in ``transfer_files`` but with no matching ``db_paths`` record. + + Comparison is separator-normalized, so a DB path stored with one separator style + still matches the on-disk path. Pure — no I/O. Returns the original (un-normalized) + transfer paths so the caller can act on the real filesystem entries. + """ + db_norm = {_norm(p) for p in db_paths if p} + return {f for f in transfer_files if _norm(f) not in db_norm} + + +def plan_standalone_deep_scan( + transfer_files: Iterable[str], + db_paths: Iterable[str], + *, + never_move: bool = False, + min_untracked: int = DEFAULT_MIN_ORPHANS, + max_fraction: float = DEFAULT_MAX_ORPHAN_FRACTION, +) -> dict: + """Plan the untracked → Staging move for a standalone deep scan. Pure — no I/O. + + Returns a dict: + * ``untracked`` (set[str]) — Transfer files with no DB record. + * ``move_blocked`` (bool) — True when the untracked files must NOT be relocated. + * ``block_reason`` (str) — ``BLOCK_TRANSFER_PERMANENT`` / ``BLOCK_DESYNC`` / "". + + The move is blocked when either: + * ``never_move`` is set (the user marked Transfer as their permanent library), or + * the untracked share is implausibly large (> ``min_untracked`` files AND + > ``max_fraction`` of the folder) — the empty/desynced-DB signature, where a + path-only diff would relocate the whole library. Below that floor a normal + batch of new arrivals still moves as before. + + ``move_blocked`` is only ever True when there ARE untracked files; an empty scan + or a clean library returns ``move_blocked=False`` with no reason. + """ + transfer_set = set(transfer_files) # concrete (handles generators) + dedups + untracked = diff_untracked(transfer_set, db_paths) + total = len(transfer_set) + n_untracked = len(untracked) + + if n_untracked == 0: + return {"untracked": untracked, "move_blocked": False, "block_reason": BLOCK_NONE} + + if never_move: + return {"untracked": untracked, "move_blocked": True, "block_reason": BLOCK_TRANSFER_PERMANENT} + + if is_implausible_orphan_flood( + n_untracked, total, min_orphans=min_untracked, max_fraction=max_fraction + ): + return {"untracked": untracked, "move_blocked": True, "block_reason": BLOCK_DESYNC} + + return {"untracked": untracked, "move_blocked": False, "block_reason": BLOCK_NONE} + + +__all__ = [ + "diff_untracked", + "plan_standalone_deep_scan", + "BLOCK_NONE", + "BLOCK_TRANSFER_PERMANENT", + "BLOCK_DESYNC", +] diff --git a/tests/library/test_standalone_scan.py b/tests/library/test_standalone_scan.py new file mode 100644 index 00000000..a0781c9a --- /dev/null +++ b/tests/library/test_standalone_scan.py @@ -0,0 +1,115 @@ +"""Standalone Deep Scan planner — the #904 data-loss guard. + +The scan relocates Transfer files the DB doesn't know about into Staging. With a +path-only diff, an empty/desynced DB makes the WHOLE library look untracked and the +scan moved all of it (reporter lost ~1,500 tracks into Staging). These pin the guard: +a normal batch of new arrivals still moves; an implausibly large untracked share (the +desync signature) or a 'permanent library' opt-out blocks the move instead. +""" + +from __future__ import annotations + +from core.library.standalone_scan import ( + BLOCK_DESYNC, + BLOCK_NONE, + BLOCK_TRANSFER_PERMANENT, + diff_untracked, + plan_standalone_deep_scan, +) + + +def _files(prefix, n, start=0): + return {f"{prefix}/track{i}.flac" for i in range(start, start + n)} + + +# ── diff_untracked (pure path diff) ────────────────────────────────────────── + +def test_diff_basic(): + transfer = {"/m/a.flac", "/m/b.flac", "/m/c.flac"} + db = {"/m/a.flac", "/m/b.flac"} + assert diff_untracked(transfer, db) == {"/m/c.flac"} + + +def test_diff_is_separator_normalized(): + # DB stored a Windows-style path; the on-disk path uses forward slashes → still a match + transfer = {"/m/Artist/x.flac"} + db = {"\\m\\Artist\\x.flac"} + assert diff_untracked(transfer, db) == set() + + +def test_diff_all_untracked_when_db_empty(): + transfer = _files("/lib", 5) + assert diff_untracked(transfer, set()) == transfer + + +# ── plan: normal (move allowed) ────────────────────────────────────────────── + +def test_clean_library_not_blocked(): + transfer = _files("/lib", 1000) + plan = plan_standalone_deep_scan(transfer, transfer) # all known + assert plan["untracked"] == set() + assert plan["move_blocked"] is False + assert plan["block_reason"] == BLOCK_NONE + + +def test_normal_new_arrivals_move(): + # DB knows 990; 10 new files dropped in → small share, moves as before + known = _files("/lib", 990) + transfer = known | _files("/lib", 10, start=990) + plan = plan_standalone_deep_scan(transfer, known) + assert len(plan["untracked"]) == 10 + assert plan["move_blocked"] is False + + +def test_small_fresh_import_under_floor_moves(): + # A tiny brand-new folder (under the absolute floor) isn't second-guessed + transfer = _files("/lib", 5) + plan = plan_standalone_deep_scan(transfer, set()) + assert len(plan["untracked"]) == 5 + assert plan["move_blocked"] is False + + +# ── plan: the #904 guard (move blocked) ────────────────────────────────────── + +def test_regression_904_empty_db_full_library_blocks(): + # Empty DB + a real 1,500-track library → 100% untracked → BLOCKED, nothing moved + transfer = _files("/library", 1500) + plan = plan_standalone_deep_scan(transfer, set()) + assert len(plan["untracked"]) == 1500 + assert plan["move_blocked"] is True + assert plan["block_reason"] == BLOCK_DESYNC + + +def test_majority_untracked_blocks(): + # 600 of 1000 unknown (60%, over the 50% line) → desync, blocked + known = _files("/lib", 400) + transfer = known | _files("/lib", 600, start=400) + plan = plan_standalone_deep_scan(transfer, known) + assert plan["move_blocked"] is True + assert plan["block_reason"] == BLOCK_DESYNC + + +def test_minority_untracked_just_under_threshold_moves(): + # 400 of 1000 unknown (40%, under 50%) → still treated as a batch, not a desync + known = _files("/lib", 600) + transfer = known | _files("/lib", 400, start=600) + plan = plan_standalone_deep_scan(transfer, known) + assert plan["move_blocked"] is False + + +def test_never_move_blocks_even_small_sets(): + # Permanent-library opt-out: block regardless of fraction + known = _files("/lib", 990) + transfer = known | _files("/lib", 10, start=990) + plan = plan_standalone_deep_scan(transfer, known, never_move=True) + assert len(plan["untracked"]) == 10 + assert plan["move_blocked"] is True + assert plan["block_reason"] == BLOCK_TRANSFER_PERMANENT + + +def test_never_move_with_nothing_untracked_is_not_blocked(): + # Nothing to move → not a "blocked" outcome even with the toggle on + transfer = _files("/lib", 100) + plan = plan_standalone_deep_scan(transfer, transfer, never_move=True) + assert plan["untracked"] == set() + assert plan["move_blocked"] is False diff --git a/web_server.py b/web_server.py index dc957222..f9f51132 100644 --- a/web_server.py +++ b/web_server.py @@ -16374,16 +16374,40 @@ def _run_soulsync_deep_scan(): logger.info(f"[SoulSync Deep Scan] {len(db_paths)} tracks in soulsync DB") - # Phase 3: Find untracked files (in Transfer but not in DB) - untracked = transfer_files - db_paths - # Also check with normalized paths (Windows vs Unix separators) - if untracked: - db_paths_normalized = {p.replace('\\', '/') for p in db_paths} - untracked = {f for f in untracked if f.replace('\\', '/') not in db_paths_normalized} + # Phase 3: Plan the untracked → Staging move, with the data-loss guard (#904). + # A path-only diff treats EVERY file the DB doesn't know about as "a new arrival + # to relocate". When the DB is empty/out of sync with disk (volume swap, DB reset, + # external tag edits) but Transfer holds the real library, that flags the whole + # library as untracked and relocates all of it. The planner refuses the move when + # the untracked share is implausibly large (the desync signature) or when the user + # marked Transfer as their permanent library — leaving files in place and warning. + from core.library.standalone_scan import ( + plan_standalone_deep_scan, BLOCK_TRANSFER_PERMANENT, BLOCK_DESYNC, + ) + never_move = bool(config_manager.get('import.transfer_is_permanent', False)) + plan = plan_standalone_deep_scan(transfer_files, db_paths, never_move=never_move) + untracked = plan['untracked'] + move_blocked = plan['move_blocked'] + block_reason = plan['block_reason'] - # Phase 4: Move untracked files to Staging for auto-import + # Phase 4: Move untracked files to Staging for auto-import — unless guarded. moved_count = 0 - if untracked and os.path.isdir(staging_path): + blocked_count = 0 + if untracked and move_blocked: + blocked_count = len(untracked) + if block_reason == BLOCK_TRANSFER_PERMANENT: + warn = (f"Deep scan: {blocked_count} file(s) in Transfer aren't in the database, " + f"but Transfer is marked your permanent library — nothing was moved.") + else: # BLOCK_DESYNC + pct = round(100 * blocked_count / max(1, len(transfer_files))) + warn = (f"Deep scan STOPPED to protect your library: {blocked_count} of " + f"{len(transfer_files)} files in Transfer ({pct}%) aren't in the database. " + f"That usually means the database is out of sync with disk, not that you " + f"have {blocked_count} new files — so NOTHING was moved. Re-sync/import " + f"before scanning, or enable 'Transfer is my permanent library'.") + logger.warning(f"[SoulSync Deep Scan] {warn}") + add_activity_item("", "SoulSync Deep Scan — move blocked", warn, "Now") + elif untracked and os.path.isdir(staging_path): _db_update_phase_callback('moving_untracked') for file_path in untracked: try: @@ -16415,6 +16439,23 @@ def _run_soulsync_deep_scan(): stale_track_ids.append(db_path) stale_count += 1 + # Guard the deletes the same way as the move (#904): if a desync blocked the + # move, the DB<->disk mapping is unreliable, so os.path.exists may be lying for + # every file — don't delete rows. Also independently skip when the stale share + # is implausibly large (storage unreachable / remount), mirroring the orphan guard. + from core.library.stale_guard import is_implausible_stale_removal + if move_blocked and block_reason == BLOCK_DESYNC: + if stale_track_ids: + logger.warning(f"[SoulSync Deep Scan] Skipping removal of {stale_count} 'stale' " + f"records — move was blocked for desync, mapping is unreliable.") + stale_track_ids = [] + stale_count = 0 + elif is_implausible_stale_removal(stale_count, len(db_paths)): + logger.warning(f"[SoulSync Deep Scan] Skipping removal of {stale_count}/{len(db_paths)} " + f"'stale' records — implausibly large share, storage likely unreachable.") + stale_track_ids = [] + stale_count = 0 + # Remove stale records if stale_track_ids: try: @@ -16447,9 +16488,11 @@ def _run_soulsync_deep_scan(): summary = f"Deep scan complete: {len(transfer_files)} files scanned" if moved_count > 0: summary += f", {moved_count} untracked files moved to Staging" + if blocked_count > 0: + summary += f", {blocked_count} untracked files LEFT IN PLACE (move blocked — see warning)" if stale_count > 0: summary += f", {stale_count} stale records removed" - if moved_count == 0 and stale_count == 0: + if moved_count == 0 and blocked_count == 0 and stale_count == 0: summary += " — library is clean" logger.info(f"[SoulSync Deep Scan] {summary}") diff --git a/webui/index.html b/webui/index.html index 970255ba..0534c2c9 100644 --- a/webui/index.html +++ b/webui/index.html @@ -6456,6 +6456,21 @@ that folder's name (the cause of the "soulsync" mass-mislabel). With it off, the metadata-identified artist is always kept.
+ +
+ +
+
+ Off by default. Turn this on if your Transfer folder is your + real, permanent library (served by Navidrome/Plex/etc.) rather than a scratch + drop area. A Deep Scan then never relocates files it doesn't recognize out to + Staging — it leaves them in place and just reports them. Even with this off, a + Deep Scan now refuses to move files when the database looks badly out of sync + with disk, as a safety guard against wiping a library (issue #904). +
diff --git a/webui/static/settings.js b/webui/static/settings.js index 0711e14b..e2ddcec4 100644 --- a/webui/static/settings.js +++ b/webui/static/settings.js @@ -1341,6 +1341,8 @@ async function loadSettingsData() { document.getElementById('import-replace-lower-quality').checked = settings.import?.replace_lower_quality === true; const _folderArtistEl = document.getElementById('import-folder-artist-override'); if (_folderArtistEl) _folderArtistEl.checked = settings.import?.folder_artist_override === true; + const _transferPermEl = document.getElementById('import-transfer-permanent'); + if (_transferPermEl) _transferPermEl.checked = settings.import?.transfer_is_permanent === true; // Populate M3U Export settings document.getElementById('m3u-export-enabled').checked = settings.m3u_export?.enabled === true; @@ -3216,6 +3218,7 @@ async function saveSettings(quiet = false) { import: { replace_lower_quality: document.getElementById('import-replace-lower-quality').checked, folder_artist_override: document.getElementById('import-folder-artist-override')?.checked === true, + transfer_is_permanent: document.getElementById('import-transfer-permanent')?.checked === true, staging_path: document.getElementById('staging-path').value || './Staging' }, playlists: { From fb9d88ea6ac646352f96c1b302949c72b22adb5f Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Mon, 22 Jun 2026 20:25:44 -0700 Subject: [PATCH 15/23] =?UTF-8?q?#903:=20playlist=20export=20to=20ListenBr?= =?UTF-8?q?ainz=20=E2=80=94=20pure=20cores=20(JSPF=20builder=20+=20MBID=20?= =?UTF-8?q?resolver)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase 1 of exporting mirrored playlists to ListenBrainz. Two pure, fully-tested seams, zero runtime wiring yet (additive, no regression): - core/exports/jspf_export.py: build_jspf(title, tracks) -> ({"playlist": {...}}, summary). LB's POST /1/playlist/create requires every track to carry a string identifier 'https://musicbrainz.org/recording/' (text-only tracks are rejected), so tracks without a valid recording-MBID UUID are dropped and counted in the coverage summary. - core/exports/mbid_resolver.py: resolve_recording_mbid(artist, title, sources) — the cheapest-first waterfall (cache -> DB -> file tag -> MusicBrainz) as a pure function over injected (label, fn) sources. Short-circuits expensive lookups, treats a raising source as a miss (one flaky MB call can't fail the export), reports the resolving source label. API spec confirmed against LB docs: POST /1/playlist/create, 'Authorization: Token ', {"playlist": {"title", "track": [{"identifier": "", title, creator, album}]}}. 13 tests. --- core/exports/jspf_export.py | 89 +++++++++++++++++++++++++++++ core/exports/mbid_resolver.py | 88 ++++++++++++++++++++++++++++ tests/exports/__init__.py | 0 tests/exports/test_jspf_export.py | 72 +++++++++++++++++++++++ tests/exports/test_mbid_resolver.py | 75 ++++++++++++++++++++++++ 5 files changed, 324 insertions(+) create mode 100644 core/exports/jspf_export.py create mode 100644 core/exports/mbid_resolver.py create mode 100644 tests/exports/__init__.py create mode 100644 tests/exports/test_jspf_export.py create mode 100644 tests/exports/test_mbid_resolver.py diff --git a/core/exports/jspf_export.py b/core/exports/jspf_export.py new file mode 100644 index 00000000..3d77cd16 --- /dev/null +++ b/core/exports/jspf_export.py @@ -0,0 +1,89 @@ +"""Build a JSPF playlist (ListenBrainz-compatible) from resolved SoulSync tracks. + +ListenBrainz's ``POST /1/playlist/create`` requires JSPF where **every track carries a +``identifier`` of ``https://musicbrainz.org/recording/``** — text-only +entries (title/creator alone) are rejected. So a track can only be exported once we've +resolved its MusicBrainz *recording* MBID (see ``mbid_resolver``); tracks without one are +dropped here and surfaced to the user as "unmatched". + +Pure + I/O-free: callers pass already-resolved track dicts, this returns the JSPF dict +(and a small coverage summary). The same JSPF is used for both the downloadable ``.jspf`` +file and the direct create-playlist POST, so there's one source of truth for the shape. +""" + +from __future__ import annotations + +import re +from typing import Any, Dict, List, Tuple + +MB_RECORDING_PREFIX = "https://musicbrainz.org/recording/" + +# A MusicBrainz MBID is a canonical UUID. Validate to avoid emitting garbage identifiers +# that LB would reject (or, worse, that silently point nowhere). +_UUID_RE = re.compile( + r"^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$", re.IGNORECASE +) + + +def is_valid_recording_mbid(mbid: Any) -> bool: + """True when ``mbid`` is a well-formed MusicBrainz UUID.""" + return bool(mbid) and isinstance(mbid, str) and bool(_UUID_RE.match(mbid.strip())) + + +def _track_entry(track: Dict[str, Any]) -> Dict[str, Any] | None: + """Build one JSPF track entry, or None if the track has no valid recording MBID.""" + mbid = (track.get("recording_mbid") or "").strip() if isinstance(track.get("recording_mbid"), str) else "" + if not is_valid_recording_mbid(mbid): + return None + entry: Dict[str, Any] = {"identifier": f"{MB_RECORDING_PREFIX}{mbid}"} + # Optional, human-friendly fields — LB ignores them on create but they make the + # downloaded .jspf readable and round-trippable. + if track.get("title"): + entry["title"] = str(track["title"]) + if track.get("artist"): + entry["creator"] = str(track["artist"]) + if track.get("album"): + entry["album"] = str(track["album"]) + return entry + + +def build_jspf( + title: str, + tracks: List[Dict[str, Any]], + *, + creator: str = "", +) -> Tuple[Dict[str, Any], Dict[str, int]]: + """Build a ListenBrainz-compatible JSPF dict from resolved tracks. + + ``tracks`` is an ordered list of dicts with ``recording_mbid`` (required to be + included), plus optional ``title`` / ``artist`` / ``album``. Tracks without a valid + recording MBID are skipped (LB rejects them). + + Returns ``(jspf, summary)`` where ``jspf`` is ``{"playlist": {...}}`` and ``summary`` + is ``{"total", "included", "skipped"}`` for the coverage display. + """ + jspf_tracks: List[Dict[str, Any]] = [] + for t in tracks or []: + if not isinstance(t, dict): + continue + entry = _track_entry(t) + if entry is not None: + jspf_tracks.append(entry) + + playlist: Dict[str, Any] = { + "title": (title or "SoulSync Export").strip() or "SoulSync Export", + "track": jspf_tracks, + } + if creator: + playlist["creator"] = str(creator) + + total = sum(1 for t in (tracks or []) if isinstance(t, dict)) + summary = { + "total": total, + "included": len(jspf_tracks), + "skipped": total - len(jspf_tracks), + } + return {"playlist": playlist}, summary + + +__all__ = ["build_jspf", "is_valid_recording_mbid", "MB_RECORDING_PREFIX"] diff --git a/core/exports/mbid_resolver.py b/core/exports/mbid_resolver.py new file mode 100644 index 00000000..f5b11972 --- /dev/null +++ b/core/exports/mbid_resolver.py @@ -0,0 +1,88 @@ +"""Resolve a playlist track's MusicBrainz *recording* MBID, cheapest source first. + +A ListenBrainz playlist export needs each track's recording MBID (``jspf_export``). A +SoulSync track can supply it from several places, in increasing cost: + +1. **resolution cache** — a prior (artist,title)->mbid result (persistent; reused across + playlists and runs, so the same song never costs twice). +2. **library DB** — ``tracks.musicbrainz_recording_id`` (set by the MusicBrainz + enrichment worker). +3. **file tags** — ``MUSICBRAINZ_RECORDING_ID`` written into the audio file on import + post-processing (catches tracks enriched at import but not via the worker). +4. **MusicBrainz lookup** — a live ``match_recording(artist, title)`` (rate-limited + ~1 req/s; the slow tail — only hit when 1–3 miss). + +This module is the **pure waterfall**: the caller passes ordered ``(label, fn)`` sources, +each ``fn(artist, title) -> mbid | None``, and ``resolve_recording_mbid`` returns the +first valid hit plus its label (for the live status / stats). The actual I/O (DB query, +mutagen read, MB request, cache read/write) lives in the export job that wires the real +sources — so this stays trivially unit-testable and short-circuits correctly. +""" + +from __future__ import annotations + +import re +from typing import Any, Callable, List, Optional, Tuple + +# Source labels (also used in the live-status breakdown). +SRC_CACHE = "cache" +SRC_DB = "db" +SRC_FILE = "file" +SRC_MUSICBRAINZ = "musicbrainz" +SRC_NONE = None + +_UUID_RE = re.compile( + r"^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$", re.IGNORECASE +) + +Source = Tuple[str, Callable[[str, str], Optional[str]]] + + +def _valid(mbid: Any) -> Optional[str]: + """Return the trimmed MBID if it's a well-formed UUID, else None.""" + if not isinstance(mbid, str): + return None + m = mbid.strip() + return m if _UUID_RE.match(m) else None + + +def normalize_key(artist: Any, title: Any) -> str: + """Stable cache key for an (artist, title) pair — lower, punctuation-stripped, + whitespace-collapsed — so trivial variations share a cache entry.""" + def _n(v: Any) -> str: + s = re.sub(r"[^\w\s]", "", str(v or "").lower()) + return re.sub(r"\s+", " ", s).strip() + return f"{_n(artist)}␟{_n(title)}" + + +def resolve_recording_mbid( + artist: str, + title: str, + sources: List[Source], +) -> Tuple[Optional[str], Optional[str]]: + """Walk ``sources`` in order; return ``(mbid, label)`` of the first that yields a + valid recording MBID, or ``(None, None)`` when every source misses. + + Each source is ``(label, fn)`` and ``fn(artist, title)`` returns an MBID or None. A + source that raises is treated as a miss (never aborts the waterfall) — so one flaky + lookup (e.g. a MusicBrainz timeout) can't fail the whole export. Short-circuits: a + later/expensive source isn't called once an earlier one hits. + """ + for label, fn in sources or []: + try: + mbid = _valid(fn(artist, title)) + except Exception: + mbid = None + if mbid: + return (mbid, label) + return (None, None) + + +__all__ = [ + "resolve_recording_mbid", + "normalize_key", + "SRC_CACHE", + "SRC_DB", + "SRC_FILE", + "SRC_MUSICBRAINZ", +] diff --git a/tests/exports/__init__.py b/tests/exports/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/tests/exports/test_jspf_export.py b/tests/exports/test_jspf_export.py new file mode 100644 index 00000000..684265d5 --- /dev/null +++ b/tests/exports/test_jspf_export.py @@ -0,0 +1,72 @@ +"""JSPF builder for ListenBrainz playlist export (#903). + +LB's create-playlist requires every track to carry a recording-MBID identifier; text-only +tracks are rejected. These pin the shape (top-level {"playlist": {...}}, string identifier +in the exact MB recording URL form), that tracks without a valid MBID are dropped, and that +the coverage summary counts included vs skipped. +""" + +from __future__ import annotations + +from core.exports.jspf_export import MB_RECORDING_PREFIX, build_jspf, is_valid_recording_mbid + +MBID_A = "e8f9b188-f819-4e43-ab0f-4bd26ce9ff56" +MBID_B = "8f3471b5-7e6a-4c1f-9c1a-2b2b2b2b2b2b" + + +def test_valid_mbid_check(): + assert is_valid_recording_mbid(MBID_A) is True + assert is_valid_recording_mbid("not-a-uuid") is False + assert is_valid_recording_mbid("") is False + assert is_valid_recording_mbid(None) is False + + +def test_top_level_shape_and_identifier_format(): + jspf, summary = build_jspf("My Playlist", [ + {"recording_mbid": MBID_A, "title": "Gold", "artist": "Spandau Ballet", "album": "True"}, + ]) + assert set(jspf.keys()) == {"playlist"} + pl = jspf["playlist"] + assert pl["title"] == "My Playlist" + assert len(pl["track"]) == 1 + t = pl["track"][0] + # identifier is a STRING in the exact MB recording form (per the LB/JSPF spec) + assert t["identifier"] == f"{MB_RECORDING_PREFIX}{MBID_A}" + assert isinstance(t["identifier"], str) + assert t["title"] == "Gold" + assert t["creator"] == "Spandau Ballet" # artist -> creator + assert t["album"] == "True" + assert summary == {"total": 1, "included": 1, "skipped": 0} + + +def test_tracks_without_valid_mbid_are_dropped(): + jspf, summary = build_jspf("P", [ + {"recording_mbid": MBID_A, "title": "Keep"}, + {"recording_mbid": "", "title": "No MBID"}, + {"recording_mbid": "garbage", "title": "Bad MBID"}, + {"title": "Missing key entirely"}, + ]) + assert [t["title"] for t in jspf["playlist"]["track"]] == ["Keep"] + assert summary == {"total": 4, "included": 1, "skipped": 3} + + +def test_order_is_preserved(): + jspf, _ = build_jspf("P", [ + {"recording_mbid": MBID_A, "title": "first"}, + {"recording_mbid": MBID_B, "title": "second"}, + ]) + assert [t["title"] for t in jspf["playlist"]["track"]] == ["first", "second"] + + +def test_optional_fields_omitted_when_absent(): + jspf, _ = build_jspf("P", [{"recording_mbid": MBID_A}]) + t = jspf["playlist"]["track"][0] + assert "title" not in t and "creator" not in t and "album" not in t + + +def test_creator_and_title_defaults(): + jspf, summary = build_jspf("", [], creator="SoulSync") + assert jspf["playlist"]["title"] == "SoulSync Export" # blank -> default + assert jspf["playlist"]["creator"] == "SoulSync" + assert jspf["playlist"]["track"] == [] + assert summary == {"total": 0, "included": 0, "skipped": 0} diff --git a/tests/exports/test_mbid_resolver.py b/tests/exports/test_mbid_resolver.py new file mode 100644 index 00000000..9be12a1d --- /dev/null +++ b/tests/exports/test_mbid_resolver.py @@ -0,0 +1,75 @@ +"""MBID resolution waterfall for playlist export (#903). + +Pins: cheapest-source-first short-circuit (don't hit MusicBrainz when the DB has it), +invalid MBIDs are treated as misses, a raising source doesn't abort the export, and the +resolving source label is reported (for the live status breakdown). +""" + +from __future__ import annotations + +from core.exports.mbid_resolver import ( + SRC_CACHE, + SRC_DB, + SRC_MUSICBRAINZ, + normalize_key, + resolve_recording_mbid, +) + +MBID = "e8f9b188-f819-4e43-ab0f-4bd26ce9ff56" +MBID2 = "8f3471b5-7e6a-4c1f-9c1a-2b2b2b2b2b2b" + + +def _src(label, value): + return (label, lambda a, t: value) + + +def test_returns_first_hit_with_label(): + mbid, label = resolve_recording_mbid("A", "T", [_src(SRC_DB, MBID), _src(SRC_MUSICBRAINZ, MBID2)]) + assert (mbid, label) == (MBID, SRC_DB) + + +def test_short_circuits_expensive_sources(): + called = {"mb": False} + def mb(a, t): + called["mb"] = True + return MBID2 + mbid, label = resolve_recording_mbid("A", "T", [_src(SRC_CACHE, MBID), (SRC_MUSICBRAINZ, mb)]) + assert (mbid, label) == (MBID, SRC_CACHE) + assert called["mb"] is False # cache hit -> MusicBrainz never queried + + +def test_falls_through_misses_to_later_source(): + mbid, label = resolve_recording_mbid("A", "T", [ + _src(SRC_CACHE, None), + _src(SRC_DB, ""), + _src(SRC_MUSICBRAINZ, MBID2), + ]) + assert (mbid, label) == (MBID2, SRC_MUSICBRAINZ) + + +def test_invalid_mbid_is_a_miss(): + mbid, label = resolve_recording_mbid("A", "T", [ + _src(SRC_DB, "not-a-uuid"), + _src(SRC_MUSICBRAINZ, MBID), + ]) + assert (mbid, label) == (MBID, SRC_MUSICBRAINZ) + + +def test_raising_source_does_not_abort(): + def boom(a, t): + raise RuntimeError("MusicBrainz timeout") + mbid, label = resolve_recording_mbid("A", "T", [ + (SRC_DB, boom), + _src(SRC_MUSICBRAINZ, MBID), + ]) + assert (mbid, label) == (MBID, SRC_MUSICBRAINZ) + + +def test_all_miss_returns_none(): + assert resolve_recording_mbid("A", "T", [_src(SRC_DB, None), _src(SRC_MUSICBRAINZ, None)]) == (None, None) + assert resolve_recording_mbid("A", "T", []) == (None, None) + + +def test_normalize_key_is_stable_across_variations(): + assert normalize_key("The Beatles", "Hey Jude!") == normalize_key("the beatles", "hey jude") + assert normalize_key("A", "X") != normalize_key("B", "X") From c6b5cd976376b508e96c5f5587f7ec72e7b703fe Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Mon, 22 Jun 2026 20:28:31 -0700 Subject: [PATCH 16/23] #903: ListenBrainz client create_playlist (create + batched item/add) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase 2. Add create_playlist(title, tracks, public) to the LB client: POST /playlist/create for the MBID, then add tracks in batches of 100 (MAX_RECORDINGS_PER_ADD) via the item/add endpoint so 1k-track playlists don't hit a single-request cap. Returns a result dict {success, playlist_mbid, playlist_url, added, requested, error} and never raises — partial add failures are reported honestly (playlist created, added count accurate). Extends the existing token-auth client; additive. 4 mocked-network tests (batching, auth, failure). --- core/listenbrainz_client.py | 75 ++++++++++++++++ .../test_listenbrainz_create_playlist.py | 86 +++++++++++++++++++ 2 files changed, 161 insertions(+) create mode 100644 tests/exports/test_listenbrainz_create_playlist.py diff --git a/core/listenbrainz_client.py b/core/listenbrainz_client.py index 2ec6ed0e..0f21acfe 100644 --- a/core/listenbrainz_client.py +++ b/core/listenbrainz_client.py @@ -158,6 +158,81 @@ class ListenBrainzClient: return True + def create_playlist(self, title: str, tracks: List[Dict], public: bool = False) -> Dict: + """Create a playlist on ListenBrainz and add its tracks (#903). + + ``tracks`` are JSPF track dicts — each MUST carry an ``identifier`` of the form + ``https://musicbrainz.org/recording/`` (LB rejects text-only tracks; the + caller drops un-resolved tracks via ``core.exports.jspf_export``). + + Robust for large playlists: create an empty playlist (title only) to get the + playlist MBID, then add tracks in batches of ``MAX_TRACKS_PER_ADD`` (LB's + ``MAX_RECORDINGS_PER_ADD`` cap) via the ``item/add`` endpoint — so a 1k-track + playlist doesn't blow a single-request limit. + + Returns ``{success, playlist_mbid, playlist_url, added, requested, error}``. + Never raises — failures are reported in the dict so the export job can surface them. + """ + MAX_TRACKS_PER_ADD = 100 + PLAYLIST_EXT = "https://musicbrainz.org/doc/jspf#playlist" + result = {"success": False, "playlist_mbid": None, "playlist_url": None, + "added": 0, "requested": len(tracks or []), "error": None} + + if not self.is_authenticated(): + result["error"] = "ListenBrainz not authenticated (no token/username)" + return result + + headers = {"Authorization": f"Token {self.token}", "Content-Type": "application/json"} + + # 1) Create the (empty) playlist — title + visibility only. + create_body = {"playlist": { + "title": (title or "SoulSync Export").strip() or "SoulSync Export", + "extension": {PLAYLIST_EXT: {"public": bool(public)}}, + }} + try: + resp = self._make_request_with_retry( + "POST", f"{self.base_url}/playlist/create", json=create_body, headers=headers + ) + except Exception as e: + result["error"] = f"create request failed: {e}" + return result + if not resp or resp.status_code not in (200, 201): + result["error"] = f"create returned {resp.status_code if resp else 'no response'}" + return result + try: + playlist_mbid = (resp.json() or {}).get("playlist_mbid") + except Exception: + playlist_mbid = None + if not playlist_mbid: + result["error"] = "create succeeded but no playlist_mbid in response" + return result + + result["playlist_mbid"] = playlist_mbid + result["playlist_url"] = f"https://listenbrainz.org/playlist/{playlist_mbid}" + + # 2) Add tracks in capped batches. + added = 0 + for i in range(0, len(tracks or []), MAX_TRACKS_PER_ADD): + batch = tracks[i:i + MAX_TRACKS_PER_ADD] + add_body = {"playlist": {"track": batch}} + try: + r = self._make_request_with_retry( + "POST", f"{self.base_url}/playlist/{playlist_mbid}/item/add", + json=add_body, headers=headers, + ) + if r and r.status_code in (200, 201): + added += len(batch) + else: + logger.warning(f"ListenBrainz item/add batch failed: " + f"{r.status_code if r else 'no response'}") + except Exception as e: + logger.error(f"ListenBrainz item/add error: {e}") + + result["added"] = added + # Playlist was created even if some adds failed — report partial success honestly. + result["success"] = True + return result + def get_playlists_created_for_user(self, count: int = 25, offset: int = 0) -> List[Dict]: """ Fetch playlists created FOR the user (recommendations, personalized playlists) diff --git a/tests/exports/test_listenbrainz_create_playlist.py b/tests/exports/test_listenbrainz_create_playlist.py new file mode 100644 index 00000000..a87cb440 --- /dev/null +++ b/tests/exports/test_listenbrainz_create_playlist.py @@ -0,0 +1,86 @@ +"""ListenBrainz create_playlist client method (#903). + +Pins the create -> batched item/add flow against a mocked network: large playlists are +added in <=100 batches (LB's MAX_RECORDINGS_PER_ADD), the new playlist MBID/URL are +returned, and failures are reported (never raised) so the export job can surface them. +""" + +from __future__ import annotations + +from core.listenbrainz_client import ListenBrainzClient + + +class _Resp: + def __init__(self, status, body=None): + self.status_code = status + self._body = body or {} + + def json(self): + return self._body + + +def _client(): + # token='' (not None) skips the network token-validation in __init__; then fake auth. + c = ListenBrainzClient(token="") + c.token = "tok" + c.username = "user" + return c + + +def _tracks(n): + return [{"identifier": f"https://musicbrainz.org/recording/{i:08d}-0000-0000-0000-000000000000"} + for i in range(n)] + + +def test_create_then_batched_add(monkeypatch): + c = _client() + calls = [] + + def fake_req(method, url, **kw): + calls.append(url) + if url.endswith("/playlist/create"): + return _Resp(200, {"status": "ok", "playlist_mbid": "PL-MBID"}) + return _Resp(200, {"status": "ok"}) + + monkeypatch.setattr(c, "_make_request_with_retry", fake_req) + res = c.create_playlist("My Playlist", _tracks(250)) + + assert res["success"] is True + assert res["playlist_mbid"] == "PL-MBID" + assert res["playlist_url"] == "https://listenbrainz.org/playlist/PL-MBID" + assert res["added"] == 250 + # 1 create + 3 add batches (100 + 100 + 50) + assert sum(1 for u in calls if u.endswith("/playlist/create")) == 1 + assert sum(1 for u in calls if "/item/add" in u) == 3 + + +def test_not_authenticated_is_reported_not_raised(): + c = ListenBrainzClient(token="") # no username -> not authenticated + res = c.create_playlist("x", _tracks(1)) + assert res["success"] is False + assert "authenticated" in (res["error"] or "") + + +def test_create_failure_reported(monkeypatch): + c = _client() + monkeypatch.setattr(c, "_make_request_with_retry", lambda *a, **k: _Resp(400, {})) + res = c.create_playlist("x", _tracks(5)) + assert res["success"] is False + assert res["playlist_mbid"] is None + assert "400" in (res["error"] or "") + + +def test_create_ok_but_partial_add_failure(monkeypatch): + c = _client() + + def fake_req(method, url, **kw): + if url.endswith("/playlist/create"): + return _Resp(200, {"playlist_mbid": "PL"}) + return _Resp(500, {}) # every add fails + + monkeypatch.setattr(c, "_make_request_with_retry", fake_req) + res = c.create_playlist("x", _tracks(10)) + # Playlist WAS created -> success True, but added=0 (honest partial report) + assert res["success"] is True + assert res["playlist_mbid"] == "PL" + assert res["added"] == 0 From 42ff13d5173a9cae1c72cdb469f790bde559fb7c Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Mon, 22 Jun 2026 20:31:35 -0700 Subject: [PATCH 17/23] #903: persistent recording-MBID cache + export orchestrator MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase 3. Additive backbone for the export job: - mb_recording_cache table (IF NOT EXISTS) + core/exports/recording_mbid_cache.py: persistent (artist,title)->recording_mbid cache, mirrors album_mbid_cache (lazy DB, error-degrades to miss). The MusicBrainz tail is ~1 req/s, so a resolved MBID is remembered once and reused across every export/playlist. - core/exports/playlist_export.py: resolve_playlist_tracks(tracks, resolve_fn) — walks tracks, dedups repeated songs within a run (resolve once), builds the ordered pseudo-playlist, tallies live stats (resolved/unmatched/deduped/by_source). Pure (I/O injected via resolve_fn + progress callback), so dedup + accounting are unit-tested with no DB/network. 5 tests. No wiring into runtime yet; nothing existing touched except the additive table. --- core/exports/playlist_export.py | 92 +++++++++++++++++++ core/exports/recording_mbid_cache.py | 126 ++++++++++++++++++++++++++ database/music_database.py | 12 +++ tests/exports/test_playlist_export.py | 74 +++++++++++++++ 4 files changed, 304 insertions(+) create mode 100644 core/exports/playlist_export.py create mode 100644 core/exports/recording_mbid_cache.py create mode 100644 tests/exports/test_playlist_export.py diff --git a/core/exports/playlist_export.py b/core/exports/playlist_export.py new file mode 100644 index 00000000..1105a09b --- /dev/null +++ b/core/exports/playlist_export.py @@ -0,0 +1,92 @@ +"""Orchestrate resolving a playlist's tracks to recording MBIDs for export (#903). + +This is the testable heart of the export job: walk the playlist's tracks, resolve each to a +MusicBrainz recording MBID via an injected ``resolve_fn`` (which the job wires to the +cache -> DB -> file -> MusicBrainz waterfall), dedup repeated songs within the run so they +only cost one resolution, build the ordered "pseudo-playlist" of resolved tracks, and tally +live stats (resolved / unmatched / per-source / deduped) for the on-card status display. + +Pure: all I/O (DB, file reads, MusicBrainz, cache) is behind ``resolve_fn`` and the optional +``on_progress`` callback, so the dedup + accounting logic is unit-testable without any +network or database. The returned ``resolved`` list feeds straight into ``jspf_export``. +""" + +from __future__ import annotations + +from typing import Any, Callable, Dict, List, Optional, Tuple + +from core.exports.mbid_resolver import normalize_key + +# resolve_fn(artist, title) -> (recording_mbid|None, source_label|None) +ResolveFn = Callable[[str, str], Tuple[Optional[str], Optional[str]]] +ProgressFn = Callable[[int, int, Dict[str, Any]], None] + + +def _field(track: Dict[str, Any], *names: str) -> str: + """First non-empty value among ``names`` (handles both playlist + LB-cache shapes).""" + for n in names: + v = track.get(n) + if v: + return str(v) + return "" + + +def resolve_playlist_tracks( + tracks: List[Dict[str, Any]], + resolve_fn: ResolveFn, + *, + on_progress: Optional[ProgressFn] = None, +) -> Dict[str, Any]: + """Resolve every track to a recording MBID and build the export pseudo-playlist. + + ``tracks`` items may use ``artist``/``artist_name`` and ``title``/``track_name`` and + ``album``/``album_name`` (both the mirrored-playlist and LB-cache shapes are accepted). + + Returns ``{"resolved": [...], "stats": {...}}`` where each resolved entry is + ``{artist, title, album, recording_mbid}`` (recording_mbid is None when unmatched), + in original order, and stats carries ``total, resolved, unmatched, deduped, + by_source`` for the live display. + """ + total = len(tracks or []) + memo: Dict[str, Tuple[Optional[str], Optional[str]]] = {} + resolved: List[Dict[str, Any]] = [] + stats: Dict[str, Any] = { + "total": total, "resolved": 0, "unmatched": 0, "deduped": 0, "by_source": {}, + } + + for i, t in enumerate(tracks or []): + if not isinstance(t, dict): + t = {} + artist = _field(t, "artist", "artist_name", "creator") + title = _field(t, "title", "track_name", "name") + album = _field(t, "album", "album_name", "release_name") + key = normalize_key(artist, title) + + if key in memo: + mbid, source = memo[key] + stats["deduped"] += 1 + fresh = False + else: + mbid, source = resolve_fn(artist, title) + memo[key] = (mbid, source) + fresh = True + + resolved.append({"artist": artist, "title": title, "album": album, "recording_mbid": mbid}) + + if mbid: + stats["resolved"] += 1 + if fresh and source: + stats["by_source"][source] = stats["by_source"].get(source, 0) + 1 + else: + stats["unmatched"] += 1 + + if on_progress is not None: + try: + on_progress(i + 1, total, stats) + except Exception: # noqa: S110 — a progress-display error must never fail the export + pass + + return {"resolved": resolved, "stats": stats} + + +__all__ = ["resolve_playlist_tracks"] diff --git a/core/exports/recording_mbid_cache.py b/core/exports/recording_mbid_cache.py new file mode 100644 index 00000000..203050b9 --- /dev/null +++ b/core/exports/recording_mbid_cache.py @@ -0,0 +1,126 @@ +"""Persistent (artist,title) -> MusicBrainz recording-MBID cache for playlist export. + +The export waterfall (``core.exports.mbid_resolver``) ends in a live MusicBrainz lookup +that's rate-limited to ~1 req/s — the slow tail of exporting a big playlist. Remembering a +resolved recording MBID ONCE means the same song never costs a second lookup, across every +future export and every playlist it appears in. + +Mirrors ``core.metadata.album_mbid_cache`` exactly: a tiny SQLite table, lazy DB accessor, +every function wrapped so any DB error degrades to a cache miss / no-op. If this module +breaks, exports still work — they just re-resolve via the live waterfall like a cold cache. +Key is the normalized ``track_key`` from ``mbid_resolver.normalize_key(artist, title)``. +""" + +from __future__ import annotations + +import threading +from typing import Optional + +from utils.logging_config import get_logger + +logger = get_logger("exports.recording_mbid_cache") + +_db_factory_lock = threading.Lock() +_db_factory = None + + +def _get_database(): + """Resolve the MusicDatabase singleton lazily; None on any failure (treated as miss).""" + global _db_factory + with _db_factory_lock: + if _db_factory is None: + try: + from database.music_database import get_database + _db_factory = get_database + except Exception as exc: + logger.warning(f"Recording-MBID cache: could not load database module: {exc}") + return None + try: + return _db_factory() + except Exception as exc: + logger.warning(f"Recording-MBID cache: database accessor failed: {exc}") + return None + + +def lookup(track_key: str) -> Optional[str]: + """Read a cached recording MBID for ``track_key``; None on miss or any DB error.""" + if not track_key: + return None + db = _get_database() + if db is None: + return None + conn = None + try: + conn = db._get_connection() + cursor = conn.cursor() + cursor.execute( + "SELECT recording_mbid FROM mb_recording_cache WHERE track_key = ? LIMIT 1", + (track_key,), + ) + row = cursor.fetchone() + if row: + return (row[0] if not hasattr(row, "keys") else row["recording_mbid"]) or None + except Exception as exc: + logger.debug(f"Recording-MBID cache lookup failed: {exc}") + finally: + if conn is not None: + try: + conn.close() + except Exception: # noqa: S110 — finally cleanup + pass + return None + + +def record(track_key: str, recording_mbid: str) -> bool: + """Persist ``track_key`` -> ``recording_mbid`` (idempotent). False on any failure.""" + if not track_key or not recording_mbid: + return False + db = _get_database() + if db is None: + return False + conn = None + try: + conn = db._get_connection() + cursor = conn.cursor() + cursor.execute( + "INSERT OR REPLACE INTO mb_recording_cache " + "(track_key, recording_mbid, updated_at) VALUES (?, ?, CURRENT_TIMESTAMP)", + (track_key, recording_mbid), + ) + conn.commit() + return True + except Exception as exc: + logger.debug(f"Recording-MBID cache record failed: {exc}") + return False + finally: + if conn is not None: + try: + conn.close() + except Exception: # noqa: S110 — finally cleanup + pass + + +def clear_all() -> bool: + """Wipe the cache (tests / forced re-resolve).""" + db = _get_database() + if db is None: + return False + conn = None + try: + conn = db._get_connection() + cursor = conn.cursor() + cursor.execute("DELETE FROM mb_recording_cache") + conn.commit() + return True + except Exception as exc: + logger.warning(f"Recording-MBID cache clear failed: {exc}") + return False + finally: + if conn is not None: + try: + conn.close() + except Exception: # noqa: S110 — finally cleanup + pass + + +__all__ = ["lookup", "record", "clear_all"] diff --git a/database/music_database.py b/database/music_database.py index 20e6b217..d097b0cf 100644 --- a/database/music_database.py +++ b/database/music_database.py @@ -2085,6 +2085,18 @@ class MusicDatabase: "ON mb_album_release_cache (release_mbid)" ) + # Persistent (artist,title) -> recording MBID cache for playlist export (#903). + # The MusicBrainz tail of the export waterfall is rate-limited (~1 req/s), so a + # resolved recording MBID is remembered ONCE and reused for that song across every + # future export and playlist. Additive: empty until the export feature writes it. + cursor.execute(""" + CREATE TABLE IF NOT EXISTS mb_recording_cache ( + track_key TEXT PRIMARY KEY, + recording_mbid TEXT NOT NULL, + updated_at TIMESTAMP DEFAULT CURRENT_TIMESTAMP + ) + """) + # Discovery artist blacklist — artists users never want to see in discovery cursor.execute(""" CREATE TABLE IF NOT EXISTS discovery_artist_blacklist ( diff --git a/tests/exports/test_playlist_export.py b/tests/exports/test_playlist_export.py new file mode 100644 index 00000000..d0af07e5 --- /dev/null +++ b/tests/exports/test_playlist_export.py @@ -0,0 +1,74 @@ +"""Playlist export orchestrator (#903): dedup + stats + progress accounting. + +Pins: repeated songs resolve once (deduped), per-source breakdown counts only fresh +resolutions, unmatched tracks carry recording_mbid=None but stay in order, alternate +field shapes (artist_name/track_name) are accepted, and a throwing progress callback +never fails the export. +""" + +from __future__ import annotations + +from core.exports.playlist_export import resolve_playlist_tracks + +MBID = "e8f9b188-f819-4e43-ab0f-4bd26ce9ff56" +MBID2 = "8f3471b5-7e6a-4c1f-9c1a-2b2b2b2b2b2b" + + +def test_resolves_and_keeps_order_and_unmatched(): + table = {("A", "T1"): (MBID, "db"), ("A", "T2"): (None, None)} + rf = lambda a, t: table.get((a, t), (None, None)) + out = resolve_playlist_tracks( + [{"artist": "A", "title": "T1"}, {"artist": "A", "title": "T2"}], rf + ) + res = out["resolved"] + assert [r["title"] for r in res] == ["T1", "T2"] + assert res[0]["recording_mbid"] == MBID + assert res[1]["recording_mbid"] is None + assert out["stats"]["resolved"] == 1 + assert out["stats"]["unmatched"] == 1 + assert out["stats"]["by_source"] == {"db": 1} + + +def test_dedup_resolves_repeated_song_once(): + calls = {"n": 0} + def rf(a, t): + calls["n"] += 1 + return (MBID, "musicbrainz") + tracks = [{"artist": "A", "title": "Song"}, {"artist": "a", "title": "song"}, # same (normalized) + {"artist": "A", "title": "Song"}] + out = resolve_playlist_tracks(tracks, rf) + assert calls["n"] == 1 # resolve_fn called once for 3 identical + assert out["stats"]["deduped"] == 2 + assert out["stats"]["resolved"] == 3 # all three tracks still get the mbid + assert out["stats"]["by_source"] == {"musicbrainz": 1} # counted once (fresh only) + assert all(r["recording_mbid"] == MBID for r in out["resolved"]) + + +def test_accepts_alternate_field_names(): + rf = lambda a, t: (MBID, "db") if (a, t) == ("Artist", "Track") else (None, None) + out = resolve_playlist_tracks( + [{"artist_name": "Artist", "track_name": "Track", "album_name": "Alb"}], rf + ) + r = out["resolved"][0] + assert r["artist"] == "Artist" and r["title"] == "Track" and r["album"] == "Alb" + assert r["recording_mbid"] == MBID + + +def test_progress_called_per_track_and_safe_when_throwing(): + seen = [] + def prog(done, total, stats): + seen.append((done, total)) + raise RuntimeError("display blew up") + out = resolve_playlist_tracks( + [{"artist": "A", "title": "x"}, {"artist": "B", "title": "y"}], + lambda a, t: (MBID, "db"), + on_progress=prog, + ) + assert seen == [(1, 2), (2, 2)] # called each track despite raising + assert out["stats"]["resolved"] == 2 # export still completed + + +def test_empty_playlist(): + out = resolve_playlist_tracks([], lambda a, t: (None, None)) + assert out["resolved"] == [] + assert out["stats"]["total"] == 0 From d430dd8b71038d5ac89c0dd23bc8a2da08517455 Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Mon, 22 Jun 2026 20:35:20 -0700 Subject: [PATCH 18/23] #903: real source wiring for the export MBID waterfall Phase 4. core/exports/export_sources.py supplies the real I/O behind each waterfall source and assembles resolve_fn: cache -> DB (tracks.musicbrainz_recording_id via text match) -> file tag (UFID/musicbrainz_trackid) -> live MusicBrainz match_recording. Every source is fail-safe (any error -> None -> fall through, export never breaks). A fresh non-cache hit is written back to the persistent cache so the same song is free next export. Sources are injectable; build_resolve_fn wiring (cache short-circuit + write-back) is unit-tested. 4 tests. --- core/exports/export_sources.py | 184 +++++++++++++++++++++++++++ tests/exports/test_export_sources.py | 59 +++++++++ 2 files changed, 243 insertions(+) create mode 100644 core/exports/export_sources.py create mode 100644 tests/exports/test_export_sources.py diff --git a/core/exports/export_sources.py b/core/exports/export_sources.py new file mode 100644 index 00000000..48db313b --- /dev/null +++ b/core/exports/export_sources.py @@ -0,0 +1,184 @@ +"""Wire the real cheapest-first sources for the export MBID waterfall (#903). + +``mbid_resolver`` is the pure waterfall; this module supplies the real I/O behind each +source and assembles the ``resolve_fn`` the export job uses: + +1. **cache** — ``recording_mbid_cache`` (persistent (artist,title)->mbid). +2. **DB** — a text-matched library track's ``tracks.musicbrainz_recording_id``. +3. **file** — ``MUSICBRAINZ_RECORDING_ID`` tag of that track's file (when the DB row had + no recording id but the file was tagged on import). +4. **MusicBrainz** — live ``match_recording(track, artist)`` (rate-limited tail). + +Every source is wrapped so any failure (missing table, unreadable file, MB timeout) returns +None — the waterfall just falls through, the export never breaks. ``build_resolve_fn`` also +writes a fresh non-cache hit back to the cache so the next export of the same song is free. +""" + +from __future__ import annotations + +import threading +from typing import Callable, Optional, Tuple + +from utils.logging_config import get_logger + +from core.exports.mbid_resolver import ( + SRC_CACHE, + SRC_DB, + SRC_FILE, + SRC_MUSICBRAINZ, + normalize_key, + resolve_recording_mbid, +) + +logger = get_logger("exports.export_sources") + + +def _db_match(artist: str, title: str) -> Tuple[Optional[str], Optional[str]]: + """Text-match a library track by (artist, title); return (recording_mbid, file_path). + Either may be None. Fail-safe — any DB error returns (None, None).""" + if not title: + return (None, None) + try: + from database.music_database import get_database + db = get_database() + conn = db._get_connection() + try: + cur = conn.cursor() + cur.execute( + "SELECT t.musicbrainz_recording_id, t.file_path " + "FROM tracks t JOIN artists a ON t.artist_id = a.id " + "WHERE LOWER(t.title) = LOWER(?) AND LOWER(a.name) = LOWER(?) " + "LIMIT 1", + (title, artist), + ) + row = cur.fetchone() + if not row: + return (None, None) + mbid = row[0] if not hasattr(row, "keys") else row["musicbrainz_recording_id"] + fpath = row[1] if not hasattr(row, "keys") else row["file_path"] + return ((mbid or None), (fpath or None)) + finally: + try: + conn.close() + except Exception: # noqa: S110 + pass + except Exception as exc: + logger.debug(f"export db_match failed for '{artist} - {title}': {exc}") + return (None, None) + + +def db_recording_mbid(artist: str, title: str) -> Optional[str]: + """Recording MBID stored on a matched library track (``musicbrainz_recording_id``).""" + return _db_match(artist, title)[0] + + +def file_recording_mbid(artist: str, title: str) -> Optional[str]: + """Recording MBID read from the matched track's file tag (set on import post-processing).""" + _mbid, fpath = _db_match(artist, title) + if not fpath: + return None + try: + from mutagen import File as MutagenFile + audio = MutagenFile(fpath) + if audio is None or not getattr(audio, "tags", None): + return None + tags = audio.tags + # ID3 UFID (MusicBrainz), Vorbis/MP4 musicbrainz_trackid, etc. + for key in ("UFID:http://musicbrainz.org", "musicbrainz_trackid", + "MUSICBRAINZ_TRACKID", "----:com.apple.iTunes:MusicBrainz Track Id"): + try: + val = tags.get(key) + except Exception: + val = None + if not val: + continue + if hasattr(val, "data"): # ID3 UFID frame + val = val.data.decode("utf-8", "ignore") + if isinstance(val, (list, tuple)): + val = val[0] if val else "" + if isinstance(val, bytes): + val = val.decode("utf-8", "ignore") + val = str(val).strip() + if val: + return val + except Exception as exc: + logger.debug(f"export file_recording_mbid failed for {fpath}: {exc}") + return None + + +_mb_service = None +_mb_service_lock = threading.Lock() + + +def _get_mb_service(): + """Shared MusicBrainzService (client + cache + DB), created lazily so importing this + module never triggers a DB/network connection on paths that don't export.""" + global _mb_service + if _mb_service is None: + with _mb_service_lock: + if _mb_service is None: + from core.musicbrainz_service import MusicBrainzService + from database.music_database import get_database + _mb_service = MusicBrainzService(get_database()) + return _mb_service + + +def musicbrainz_recording_mbid(artist: str, title: str) -> Optional[str]: + """Live MusicBrainz ``match_recording`` — the rate-limited tail.""" + if not title: + return None + try: + svc = _get_mb_service() + if not svc: + return None + result = svc.match_recording(title, artist) + if result and result.get("mbid"): + return result["mbid"] + except Exception as exc: + logger.debug(f"export musicbrainz_recording_mbid failed for '{artist} - {title}': {exc}") + return None + + +def build_resolve_fn( + *, + db_fn: Callable[[str, str], Optional[str]] = db_recording_mbid, + file_fn: Callable[[str, str], Optional[str]] = file_recording_mbid, + mb_fn: Callable[[str, str], Optional[str]] = musicbrainz_recording_mbid, + cache_lookup: Optional[Callable[[str], Optional[str]]] = None, + cache_record: Optional[Callable[[str, str], bool]] = None, +) -> Callable[[str, str], Tuple[Optional[str], Optional[str]]]: + """Assemble the export ``resolve_fn(artist, title) -> (mbid, source_label)``. + + Runs cache -> DB -> file -> MusicBrainz, and writes a fresh (non-cache) hit back to the + persistent cache. All sources are injectable so the wiring is unit-testable; defaults + use the real cache module. + """ + if cache_lookup is None or cache_record is None: + from core.exports import recording_mbid_cache as _cache + cache_lookup = cache_lookup or _cache.lookup + cache_record = cache_record or _cache.record + + def resolve_fn(artist: str, title: str) -> Tuple[Optional[str], Optional[str]]: + sources = [ + (SRC_CACHE, lambda a, t: cache_lookup(normalize_key(a, t))), + (SRC_DB, db_fn), + (SRC_FILE, file_fn), + (SRC_MUSICBRAINZ, mb_fn), + ] + mbid, label = resolve_recording_mbid(artist, title, sources) + if mbid and label and label != SRC_CACHE: + try: + cache_record(normalize_key(artist, title), mbid) + except Exception: # noqa: S110 — cache write is best-effort + pass + return (mbid, label) + + return resolve_fn + + +__all__ = [ + "build_resolve_fn", + "db_recording_mbid", + "file_recording_mbid", + "musicbrainz_recording_mbid", +] diff --git a/tests/exports/test_export_sources.py b/tests/exports/test_export_sources.py new file mode 100644 index 00000000..bb019ea7 --- /dev/null +++ b/tests/exports/test_export_sources.py @@ -0,0 +1,59 @@ +"""Export source wiring (#903): waterfall order + cache write-back. + +build_resolve_fn assembles cache -> DB -> file -> MusicBrainz and writes a fresh +(non-cache) hit back to the cache. Pins: a cache hit short-circuits everything and is +NOT re-written; a DB/MB hit IS written back; misses fall through; the resolving label is +returned. +""" + +from __future__ import annotations + +from core.exports.export_sources import build_resolve_fn +from core.exports.mbid_resolver import SRC_CACHE, SRC_DB, SRC_MUSICBRAINZ + +MBID = "e8f9b188-f819-4e43-ab0f-4bd26ce9ff56" + + +def _wire(db=None, file=None, mb=None, cache=None): + recorded = {} + store = dict(cache or {}) + fn = build_resolve_fn( + db_fn=lambda a, t: (db or {}).get((a, t)), + file_fn=lambda a, t: (file or {}).get((a, t)), + mb_fn=lambda a, t: (mb or {}).get((a, t)), + cache_lookup=lambda k: store.get(k), + cache_record=lambda k, m: recorded.__setitem__(k, m) or True, + ) + return fn, recorded + + +def test_cache_hit_short_circuits_and_is_not_rewritten(): + from core.exports.mbid_resolver import normalize_key + fn, recorded = _wire( + cache={normalize_key("A", "T"): MBID}, + db={("A", "T"): "should-not-reach"}, + ) + mbid, label = fn("A", "T") + assert (mbid, label) == (MBID, SRC_CACHE) + assert recorded == {} # cache hit -> no write-back + + +def test_db_hit_is_written_back_to_cache(): + from core.exports.mbid_resolver import normalize_key + fn, recorded = _wire(db={("A", "T"): MBID}) + mbid, label = fn("A", "T") + assert (mbid, label) == (MBID, SRC_DB) + assert recorded == {normalize_key("A", "T"): MBID} # fresh hit cached for next time + + +def test_falls_through_to_musicbrainz_and_caches(): + fn, recorded = _wire(db={}, file={}, mb={("A", "T"): MBID}) + mbid, label = fn("A", "T") + assert (mbid, label) == (MBID, SRC_MUSICBRAINZ) + assert list(recorded.values()) == [MBID] + + +def test_all_miss_returns_none_and_no_write(): + fn, recorded = _wire() + assert fn("A", "T") == (None, None) + assert recorded == {} From 5ca88d96c189f3cb1e50f9680c86181b9613396e Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Mon, 22 Jun 2026 20:37:18 -0700 Subject: [PATCH 19/23] #903: backend endpoints for playlist export to ListenBrainz/JSPF MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase 5. Three additive routes + an in-memory job registry (new globals, no existing code touched): - POST /api/playlists//export/listenbrainz {mode: download|push} — spawns a background thread that loads the mirrored playlist's tracks, resolves each to a recording MBID via the waterfall, builds the JSPF, and (push) creates the playlist on ListenBrainz. Returns job_id. - GET /api/playlists/export/status/ — live status (phase/done/total/coverage) for the card to poll; omits the heavy JSPF blob. - GET /api/playlists/export/download/ — downloads the built .jspf. Reuses the tested cores (build_resolve_fn, resolve_playlist_tracks, build_jspf, create_playlist). --- web_server.py | 107 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 107 insertions(+) diff --git a/web_server.py b/web_server.py index f9f51132..c2bd38b8 100644 --- a/web_server.py +++ b/web_server.py @@ -27163,6 +27163,113 @@ def export_watchlist(): return jsonify({"success": False, "error": str(e)}), 500 +# ── Playlist export to ListenBrainz / JSPF (#903) ──────────────────────────── +# Resolve each mirrored-playlist track to a MusicBrainz recording MBID (cache -> DB -> +# file tag -> live MB), build a JSPF, and either hand it back as a download or create the +# playlist directly on ListenBrainz. Runs in a background thread with live status the +# mirrored-playlist card polls. Entirely additive — new routes + an in-memory job registry. +_playlist_export_jobs = {} +_playlist_export_jobs_lock = threading.Lock() + + +def _run_playlist_export(job_id, playlist_id, title, mode): + job = _playlist_export_jobs[job_id] + try: + from core.exports.export_sources import build_resolve_fn + from core.exports.playlist_export import resolve_playlist_tracks + from core.exports.jspf_export import build_jspf + + db = get_database() + tracks = db.get_mirrored_playlist_tracks(int(playlist_id)) + job['total'] = len(tracks) + job['phase'] = 'resolving' + + resolve_fn = build_resolve_fn() + + def on_progress(done, total, stats): + job['done'] = done + job['stats'] = dict(stats) + + out = resolve_playlist_tracks(tracks, resolve_fn, on_progress=on_progress) + jspf, summary = build_jspf(title, out['resolved'], creator='SoulSync') + job['summary'] = summary + job['jspf'] = jspf + job['stats'] = out['stats'] + + if mode == 'push': + job['phase'] = 'pushing' + from core.listenbrainz_client import ListenBrainzClient + client = ListenBrainzClient() + res = client.create_playlist(title, jspf['playlist']['track']) + job['push'] = res + if res.get('success'): + job['phase'] = 'done' + else: + job['phase'] = 'error' + job['error'] = res.get('error') or 'ListenBrainz push failed' + else: + job['phase'] = 'done' + except Exception as e: + logger.error(f"[Playlist Export] job {job_id} failed: {e}") + job['phase'] = 'error' + job['error'] = str(e) + + +@app.route('/api/playlists//export/listenbrainz', methods=['POST']) +def start_playlist_export_listenbrainz(playlist_id): + """Start a background export of a mirrored playlist to ListenBrainz/JSPF. + + Body: {"mode": "download"|"push"} (default "download"). Returns {job_id} to poll.""" + try: + body = request.get_json(silent=True) or {} + mode = 'push' if body.get('mode') == 'push' else 'download' + db = get_database() + meta = db.get_mirrored_playlist(int(playlist_id)) or {} + title = (meta.get('name') or meta.get('title') or 'SoulSync Export').strip() or 'SoulSync Export' + + import uuid + job_id = uuid.uuid4().hex + with _playlist_export_jobs_lock: + _playlist_export_jobs[job_id] = { + 'job_id': job_id, 'playlist_id': str(playlist_id), 'title': title, + 'mode': mode, 'phase': 'starting', 'done': 0, 'total': 0, + 'stats': {}, 'error': None, + } + t = threading.Thread(target=_run_playlist_export, args=(job_id, playlist_id, title, mode), daemon=True) + t.start() + return jsonify({"success": True, "job_id": job_id}) + except Exception as e: + logger.error(f"Playlist export start failed: {e}") + return jsonify({"success": False, "error": str(e)}), 500 + + +@app.route('/api/playlists/export/status/', methods=['GET']) +def playlist_export_status(job_id): + """Live status for an export job (polled by the mirrored-playlist card).""" + job = _playlist_export_jobs.get(job_id) + if not job: + return jsonify({"success": False, "error": "unknown job"}), 404 + # Don't ship the full JSPF on every poll — just status + coverage. + out = {k: v for k, v in job.items() if k != 'jspf'} + out['has_download'] = bool(job.get('jspf')) + return jsonify({"success": True, "job": out}) + + +@app.route('/api/playlists/export/download/', methods=['GET']) +def playlist_export_download(job_id): + """Download a completed export's JSPF file.""" + job = _playlist_export_jobs.get(job_id) + if not job or not job.get('jspf'): + return jsonify({"success": False, "error": "no export available"}), 404 + import json as _json + safe = re.sub(r'[^\w\-]+', '_', (job.get('title') or 'playlist')).strip('_') or 'playlist' + return Response( + _json.dumps(job['jspf'], indent=2, ensure_ascii=False), + mimetype='application/jspf+json', + headers={'Content-Disposition': f'attachment; filename="{safe}.jspf"'}, + ) + + @app.route('/api/library/artists/export', methods=['GET']) def export_library_artists(): """Export the WHOLE library artist roster (name + every source id/url we have, From c06ef6bb3467e163bbd50d323d53d006a6b82839 Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Mon, 22 Jun 2026 20:40:03 -0700 Subject: [PATCH 20/23] #903: mirrored-playlist card export button + live status UI MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase 6 (UI). Adds an export button to the mirrored-playlist card's hover action row (next to rename/link/delete). Click -> a small on-brand modal to pick a destination (Sync to ListenBrainz directly, or Download .jspf). Starts the background export, then polls status and shows live progress on the card ('Matching 340/1000 · 312 matched' -> 'Synced · 947/1000 matched · view'). Reuses the tested backend job/endpoints; additive (new button + CSS + JS functions, existing card render untouched apart from the inserted button). --- webui/static/stats-automations.js | 97 +++++++++++++++++++++++++++++++ webui/static/style.css | 17 ++++++ 2 files changed, 114 insertions(+) diff --git a/webui/static/stats-automations.js b/webui/static/stats-automations.js index bf07d547..63d1c760 100644 --- a/webui/static/stats-automations.js +++ b/webui/static/stats-automations.js @@ -601,7 +601,9 @@ function renderMirroredCard(p, container) { + + `; card.addEventListener('click', () => { const st = youtubePlaylistStates[hash]; @@ -644,6 +646,101 @@ function renderMirroredCard(p, container) { } } +// ── Playlist export to ListenBrainz / JSPF (#903) ─────────────────────────── +// Export button on the mirrored-playlist card -> pick a destination -> background job +// resolves each track to a MusicBrainz recording MBID (cache/DB/file/MusicBrainz) and either +// downloads the .jspf or pushes the playlist straight to ListenBrainz, with live status on the card. +function exportMirroredPlaylist(playlistId, name) { + const existing = document.getElementById('pl-export-modal'); + if (existing) existing.remove(); + const overlay = document.createElement('div'); + overlay.id = 'pl-export-modal'; + overlay.style.cssText = 'position:fixed;inset:0;z-index:10000;display:flex;align-items:center;justify-content:center;background:rgba(0,0,0,0.6);backdrop-filter:blur(4px);'; + overlay.innerHTML = ` +
+
Export playlist
+
${_esc(name)} → ListenBrainz
+ + +
Each track is matched to its MusicBrainz recording ID (cached → library → file tag → MusicBrainz). Tracks without an ID can't be included — you'll see how many matched.
+
+
`; + overlay.addEventListener('click', (e) => { if (e.target === overlay) overlay.remove(); }); + overlay.querySelectorAll('.pl-export-choice').forEach(btn => { + btn.addEventListener('click', () => { + const mode = btn.dataset.mode; + overlay.remove(); + _startPlaylistExport(playlistId, mode, name); + }); + }); + document.body.appendChild(overlay); +} + +async function _startPlaylistExport(playlistId, mode, name) { + _setExportStatus(playlistId, `Starting export…`); + try { + const resp = await fetch(`/api/playlists/${playlistId}/export/listenbrainz`, { + method: 'POST', headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ mode }), + }); + const data = await resp.json(); + if (!data.success || !data.job_id) { + _setExportStatus(playlistId, `Export failed to start`); + return; + } + _pollPlaylistExport(data.job_id, playlistId, mode, name); + } catch (e) { + _setExportStatus(playlistId, `Export error`); + } +} + +async function _pollPlaylistExport(jobId, playlistId, mode, name) { + try { + const resp = await fetch(`/api/playlists/export/status/${jobId}`); + const data = await resp.json(); + const job = data.job || {}; + const st = job.stats || {}; + if (job.phase === 'resolving') { + const pct = job.total ? Math.round(100 * (job.done || 0) / job.total) : 0; + _setExportStatus(playlistId, `Matching ${job.done || 0}/${job.total || 0} (${pct}%)${st.resolved != null ? ` · ${st.resolved} matched` : ''}`); + } else if (job.phase === 'pushing') { + _setExportStatus(playlistId, `Pushing to ListenBrainz…`); + } else if (job.phase === 'done') { + const sum = job.summary || {}; + const cov = `${sum.included || 0}/${sum.total || 0} matched${sum.skipped ? ` · ${sum.skipped} unmatched` : ''}`; + if (mode === 'download') { + window.location = `/api/playlists/export/download/${jobId}`; + _setExportStatus(playlistId, `Downloaded · ${cov}`, 8000); + } else { + const url = (job.push && job.push.playlist_url) || ''; + _setExportStatus(playlistId, `Synced to ListenBrainz · ${cov}` + (url ? ` view` : ''), 12000); + if (typeof showToast === 'function') showToast(`Playlist synced to ListenBrainz (${cov})`, 'success'); + } + return; + } else if (job.phase === 'error') { + _setExportStatus(playlistId, `${_esc(job.error || 'Export failed')}`, 10000); + return; + } + setTimeout(() => _pollPlaylistExport(jobId, playlistId, mode, name), 1000); + } catch (e) { + setTimeout(() => _pollPlaylistExport(jobId, playlistId, mode, name), 2000); + } +} + +function _setExportStatus(playlistId, html, autoHideMs) { + const el = document.getElementById(`export-status-${playlistId}`); + if (!el) return; + el.innerHTML = html; + el.style.display = ''; + if (autoHideMs) setTimeout(() => { if (el) el.style.display = 'none'; }, autoHideMs); +} + function updateMirroredCardPhase(urlHash, phase) { // Update the state phase (updateYouTubeCardPhase skips this for mirrored playlists due to no cardElement) const state = youtubePlaylistStates[urlHash]; diff --git a/webui/static/style.css b/webui/static/style.css index c43e4c74..20483d19 100644 --- a/webui/static/style.css +++ b/webui/static/style.css @@ -14541,6 +14541,7 @@ body.helper-mode-active #dashboard-activity-feed:hover { .mirrored-card-delete, .mirrored-card-clear, .mirrored-card-link, +.mirrored-card-export, .mirrored-card-rename { background: rgba(255, 255, 255, 0.06); border: 1px solid rgba(255, 255, 255, 0.08); @@ -14562,11 +14563,27 @@ body.helper-mode-active #dashboard-activity-feed:hover { .mirrored-playlist-card:hover .mirrored-card-delete, .mirrored-playlist-card:hover .mirrored-card-clear, .mirrored-playlist-card:hover .mirrored-card-link, +.mirrored-playlist-card:hover .mirrored-card-export, .mirrored-playlist-card:hover .mirrored-card-rename, .mirrored-playlist-card:hover .mirrored-card-pipeline { opacity: 1; } +.mirrored-card-export:hover { + color: rgba(var(--accent-rgb), 1); + background: rgba(var(--accent-rgb), 0.15); + border-color: rgba(var(--accent-rgb), 0.3); + transform: scale(1.1); +} + +/* Live export status line on the card (Matching N/M, Synced, etc.) */ +.mirrored-card-export-status { + flex-basis: 100%; + font-size: 11.5px; + margin-top: 6px; + padding-left: 2px; +} + .mirrored-card-delete:hover { color: #ff4444; background: rgba(255, 68, 68, 0.15); From 5f1ec9ed7eda70aab565a80562a2c173979d0866 Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Mon, 22 Jun 2026 22:28:07 -0700 Subject: [PATCH 21/23] #903: fix mirrored-card layout break from export status div The live export status was a separate flex child with flex-basis:100%, which became a greedy item in the card's flex row and squished the info column to min-content (text wrapping vertically). Inject the status into the card's existing .card-meta line instead (same approach as the pipeline phase indicator) so it sits inline and leaves the row intact. Removes the offending div + CSS. --- webui/static/stats-automations.js | 20 ++++++++++++++------ webui/static/style.css | 10 ++++------ 2 files changed, 18 insertions(+), 12 deletions(-) diff --git a/webui/static/stats-automations.js b/webui/static/stats-automations.js index 63d1c760..922e6e0d 100644 --- a/webui/static/stats-automations.js +++ b/webui/static/stats-automations.js @@ -603,7 +603,6 @@ function renderMirroredCard(p, container) { - `; card.addEventListener('click', () => { const st = youtubePlaylistStates[hash]; @@ -734,11 +733,20 @@ async function _pollPlaylistExport(jobId, playlistId, mode, name) { } function _setExportStatus(playlistId, html, autoHideMs) { - const el = document.getElementById(`export-status-${playlistId}`); - if (!el) return; - el.innerHTML = html; - el.style.display = ''; - if (autoHideMs) setTimeout(() => { if (el) el.style.display = 'none'; }, autoHideMs); + // Inject into the card's existing .card-meta line (same approach as the pipeline phase + // indicator) so the status sits inline and never disrupts the card's flex row. + const card = document.getElementById(`mirrored-card-${playlistId}`); + if (!card) return; + const meta = card.querySelector('.card-meta'); + if (!meta) return; + let span = meta.querySelector('.export-status-span'); + if (!span) { + span = document.createElement('span'); + span.className = 'export-status-span'; + meta.appendChild(span); + } + span.innerHTML = html; + if (autoHideMs) setTimeout(() => { if (span && span.parentNode) span.remove(); }, autoHideMs); } function updateMirroredCardPhase(urlHash, phase) { diff --git a/webui/static/style.css b/webui/static/style.css index 20483d19..47a0e66c 100644 --- a/webui/static/style.css +++ b/webui/static/style.css @@ -14576,12 +14576,10 @@ body.helper-mode-active #dashboard-activity-feed:hover { transform: scale(1.1); } -/* Live export status line on the card (Matching N/M, Synced, etc.) */ -.mirrored-card-export-status { - flex-basis: 100%; - font-size: 11.5px; - margin-top: 6px; - padding-left: 2px; +/* Live export status, injected inline into the card's .card-meta line. */ +.export-status-span { + font-size: inherit; + white-space: nowrap; } .mirrored-card-delete:hover { From ab8f82af2e45d81017c47ef9190f59a6f0c86e12 Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Mon, 22 Jun 2026 22:36:29 -0700 Subject: [PATCH 22/23] #903: re-export updates the same ListenBrainz playlist in place (no duplicates) Re-running an export created a new LB playlist every time (LB keys on MBID, not name, and create always mints a new one). Now remember which LB playlist a mirror was pushed to and update it in place: - listenbrainz_client: refactor batched-add into _add_tracks_in_batches; add get_playlist_track_count, delete_playlist, update_playlist (verify exists -> clear items via item/delete -> re-add -> edit title; reports gone=True if deleted on LB), and create_or_update_playlist (update when we have a prior MBID, else create; falls back to create if the remembered one was deleted). Stable URL/MBID across re-syncs. - playlist_export_targets table + get/set_playlist_export_target: remember (mirror, target) -> LB MBID. - export job consults/stores the target so push updates in place. +6 mocked tests (clear+re-add same mbid, gone-fallback, create-or-update branches, delete). API endpoints (item/delete, playlist/edit, playlist/delete, GET count) confirmed against LB docs; live round-trip pending explicit auth. --- core/listenbrainz_client.py | 162 +++++++++++++----- database/music_database.py | 49 ++++++ .../test_listenbrainz_create_playlist.py | 74 ++++++++ web_server.py | 6 +- 4 files changed, 249 insertions(+), 42 deletions(-) diff --git a/core/listenbrainz_client.py b/core/listenbrainz_client.py index 0f21acfe..af457f3a 100644 --- a/core/listenbrainz_client.py +++ b/core/listenbrainz_client.py @@ -158,40 +158,78 @@ class ListenBrainzClient: return True + _MAX_TRACKS_PER_ADD = 100 # ListenBrainz MAX_RECORDINGS_PER_ADD + _PLAYLIST_EXT = "https://musicbrainz.org/doc/jspf#playlist" + + def _lb_headers(self) -> Dict: + return {"Authorization": f"Token {self.token}", "Content-Type": "application/json"} + + def _add_tracks_in_batches(self, playlist_mbid: str, tracks: List[Dict]) -> int: + """Add JSPF tracks to a playlist in <=100-track batches; return how many were added.""" + added = 0 + headers = self._lb_headers() + for i in range(0, len(tracks or []), self._MAX_TRACKS_PER_ADD): + batch = tracks[i:i + self._MAX_TRACKS_PER_ADD] + try: + r = self._make_request_with_retry( + "POST", f"{self.base_url}/playlist/{playlist_mbid}/item/add", + json={"playlist": {"track": batch}}, headers=headers, + ) + if r and r.status_code in (200, 201): + added += len(batch) + else: + logger.warning(f"ListenBrainz item/add batch failed: " + f"{r.status_code if r else 'no response'}") + except Exception as e: + logger.error(f"ListenBrainz item/add error: {e}") + return added + + def get_playlist_track_count(self, playlist_mbid: str): + """Current track count of an LB playlist, or None if it can't be fetched (gone/404).""" + try: + r = self._make_request_with_retry( + "GET", f"{self.base_url}/playlist/{playlist_mbid}", + params={"fetch_metadata": "false"}, + headers={"Authorization": f"Token {self.token}"}, + ) + if r and r.status_code == 200: + return len(((r.json() or {}).get("playlist") or {}).get("track", [])) + except Exception as e: + logger.debug(f"ListenBrainz get playlist count failed: {e}") + return None + + def delete_playlist(self, playlist_mbid: str) -> bool: + """Delete an LB playlist. True on success.""" + try: + r = self._make_request_with_retry( + "POST", f"{self.base_url}/playlist/{playlist_mbid}/delete", headers=self._lb_headers() + ) + return bool(r and r.status_code in (200, 201)) + except Exception as e: + logger.error(f"ListenBrainz delete playlist error: {e}") + return False + def create_playlist(self, title: str, tracks: List[Dict], public: bool = False) -> Dict: - """Create a playlist on ListenBrainz and add its tracks (#903). + """Create a NEW playlist on ListenBrainz and add its tracks (#903). ``tracks`` are JSPF track dicts — each MUST carry an ``identifier`` of the form - ``https://musicbrainz.org/recording/`` (LB rejects text-only tracks; the - caller drops un-resolved tracks via ``core.exports.jspf_export``). - - Robust for large playlists: create an empty playlist (title only) to get the - playlist MBID, then add tracks in batches of ``MAX_TRACKS_PER_ADD`` (LB's - ``MAX_RECORDINGS_PER_ADD`` cap) via the ``item/add`` endpoint — so a 1k-track - playlist doesn't blow a single-request limit. - - Returns ``{success, playlist_mbid, playlist_url, added, requested, error}``. - Never raises — failures are reported in the dict so the export job can surface them. + ``https://musicbrainz.org/recording/`` (LB rejects text-only tracks). Creates + an empty playlist for the MBID, then adds tracks in <=100 batches. Returns + ``{success, playlist_mbid, playlist_url, added, requested, error, updated}``. Never raises. """ - MAX_TRACKS_PER_ADD = 100 - PLAYLIST_EXT = "https://musicbrainz.org/doc/jspf#playlist" result = {"success": False, "playlist_mbid": None, "playlist_url": None, - "added": 0, "requested": len(tracks or []), "error": None} - + "added": 0, "requested": len(tracks or []), "error": None, "updated": False} if not self.is_authenticated(): result["error"] = "ListenBrainz not authenticated (no token/username)" return result - headers = {"Authorization": f"Token {self.token}", "Content-Type": "application/json"} - - # 1) Create the (empty) playlist — title + visibility only. create_body = {"playlist": { "title": (title or "SoulSync Export").strip() or "SoulSync Export", - "extension": {PLAYLIST_EXT: {"public": bool(public)}}, + "extension": {self._PLAYLIST_EXT: {"public": bool(public)}}, }} try: resp = self._make_request_with_retry( - "POST", f"{self.base_url}/playlist/create", json=create_body, headers=headers + "POST", f"{self.base_url}/playlist/create", json=create_body, headers=self._lb_headers() ) except Exception as e: result["error"] = f"create request failed: {e}" @@ -209,30 +247,72 @@ class ListenBrainzClient: result["playlist_mbid"] = playlist_mbid result["playlist_url"] = f"https://listenbrainz.org/playlist/{playlist_mbid}" - - # 2) Add tracks in capped batches. - added = 0 - for i in range(0, len(tracks or []), MAX_TRACKS_PER_ADD): - batch = tracks[i:i + MAX_TRACKS_PER_ADD] - add_body = {"playlist": {"track": batch}} - try: - r = self._make_request_with_retry( - "POST", f"{self.base_url}/playlist/{playlist_mbid}/item/add", - json=add_body, headers=headers, - ) - if r and r.status_code in (200, 201): - added += len(batch) - else: - logger.warning(f"ListenBrainz item/add batch failed: " - f"{r.status_code if r else 'no response'}") - except Exception as e: - logger.error(f"ListenBrainz item/add error: {e}") - - result["added"] = added - # Playlist was created even if some adds failed — report partial success honestly. + result["added"] = self._add_tracks_in_batches(playlist_mbid, tracks) result["success"] = True return result + def update_playlist(self, playlist_mbid: str, title: str, tracks: List[Dict], public: bool = False) -> Dict: + """Replace an existing LB playlist's contents IN PLACE (stable URL/MBID) (#903). + + Verifies the playlist still exists, clears its current items, re-adds the new tracks, + and updates the title. If the playlist is gone (deleted on LB), returns success=False + with ``gone=True`` so the caller can fall back to creating a fresh one. + Returns the same shape as ``create_playlist`` plus ``updated=True``. + """ + result = {"success": False, "playlist_mbid": playlist_mbid, + "playlist_url": f"https://listenbrainz.org/playlist/{playlist_mbid}", + "added": 0, "requested": len(tracks or []), "error": None, + "updated": True, "gone": False} + if not self.is_authenticated(): + result["error"] = "ListenBrainz not authenticated (no token/username)" + return result + + count = self.get_playlist_track_count(playlist_mbid) + if count is None: + result["error"] = "playlist not found on ListenBrainz" + result["gone"] = True + return result + + headers = self._lb_headers() + # Clear existing items (one range delete from the top). + if count > 0: + try: + self._make_request_with_retry( + "POST", f"{self.base_url}/playlist/{playlist_mbid}/item/delete", + json={"index": 0, "count": count}, headers=headers, + ) + except Exception as e: + logger.warning(f"ListenBrainz item/delete (clear) failed: {e}") + + result["added"] = self._add_tracks_in_batches(playlist_mbid, tracks) + + # Refresh the title (best-effort — content already replaced). + try: + self._make_request_with_retry( + "POST", f"{self.base_url}/playlist/edit/{playlist_mbid}", + json={"playlist": {"title": (title or "SoulSync Export").strip() or "SoulSync Export"}}, + headers=headers, + ) + except Exception as e: + logger.debug(f"ListenBrainz playlist title edit failed: {e}") + + result["success"] = True + return result + + def create_or_update_playlist(self, title: str, tracks: List[Dict], + existing_mbid: str = None, public: bool = False) -> Dict: + """Update the existing LB playlist in place when we've pushed this one before, else + create a fresh one — so re-exporting the same SoulSync playlist never duplicates it. + Falls back to create if the remembered playlist was deleted on LB.""" + if existing_mbid: + res = self.update_playlist(existing_mbid, title, tracks, public) + if res.get("success"): + return res + # Remembered playlist gone/failed -> create a new one instead of erroring out. + logger.info(f"ListenBrainz playlist {existing_mbid} unavailable for update " + f"({res.get('error')}); creating a new one.") + return self.create_playlist(title, tracks, public) + def get_playlists_created_for_user(self, count: int = 25, offset: int = 0) -> List[Dict]: """ Fetch playlists created FOR the user (recommendations, personalized playlists) diff --git a/database/music_database.py b/database/music_database.py index d097b0cf..1b140874 100644 --- a/database/music_database.py +++ b/database/music_database.py @@ -2097,6 +2097,19 @@ class MusicDatabase: ) """) + # Remember which external playlist a mirrored playlist was exported to, so a + # re-export UPDATES it in place instead of creating a duplicate (#903). Keyed by + # (mirrored playlist, target service) -> the target's playlist id (LB recording MBID). + cursor.execute(""" + CREATE TABLE IF NOT EXISTS playlist_export_targets ( + mirrored_playlist_id INTEGER NOT NULL, + target TEXT NOT NULL, + target_playlist_mbid TEXT NOT NULL, + updated_at TIMESTAMP DEFAULT CURRENT_TIMESTAMP, + PRIMARY KEY (mirrored_playlist_id, target) + ) + """) + # Discovery artist blacklist — artists users never want to see in discovery cursor.execute(""" CREATE TABLE IF NOT EXISTS discovery_artist_blacklist ( @@ -14167,6 +14180,42 @@ class MusicDatabase: logger.error(f"Error updating custom_name for playlist {playlist_id}: {e}") return False + def get_playlist_export_target(self, mirrored_playlist_id: int, target: str) -> Optional[str]: + """The external playlist id this mirror was last exported to (or None). #903.""" + try: + with self._get_connection() as conn: + cur = conn.cursor() + cur.execute( + "SELECT target_playlist_mbid FROM playlist_export_targets " + "WHERE mirrored_playlist_id = ? AND target = ? LIMIT 1", + (int(mirrored_playlist_id), target), + ) + row = cur.fetchone() + if row: + return (row[0] if not hasattr(row, "keys") else row["target_playlist_mbid"]) or None + except Exception as e: + logger.debug(f"get_playlist_export_target failed: {e}") + return None + + def set_playlist_export_target(self, mirrored_playlist_id: int, target: str, target_mbid: str) -> bool: + """Remember the external playlist id for this mirror (idempotent). #903.""" + if not target_mbid: + return False + try: + with self._get_connection() as conn: + cur = conn.cursor() + cur.execute( + "INSERT OR REPLACE INTO playlist_export_targets " + "(mirrored_playlist_id, target, target_playlist_mbid, updated_at) " + "VALUES (?, ?, ?, CURRENT_TIMESTAMP)", + (int(mirrored_playlist_id), target, target_mbid), + ) + conn.commit() + return True + except Exception as e: + logger.debug(f"set_playlist_export_target failed: {e}") + return False + def get_mirrored_playlist_tracks(self, playlist_id: int) -> List[Dict]: """Return all tracks for a mirrored playlist ordered by position.""" try: diff --git a/tests/exports/test_listenbrainz_create_playlist.py b/tests/exports/test_listenbrainz_create_playlist.py index a87cb440..72021e80 100644 --- a/tests/exports/test_listenbrainz_create_playlist.py +++ b/tests/exports/test_listenbrainz_create_playlist.py @@ -84,3 +84,77 @@ def test_create_ok_but_partial_add_failure(monkeypatch): assert res["success"] is True assert res["playlist_mbid"] == "PL" assert res["added"] == 0 + + +# ── update-in-place + create-or-update (#903 no-duplicate re-export) ────────── + +def _route(existing_count=None): + """Build a fake _make_request_with_retry; records calls. existing_count=None -> GET 404.""" + calls = [] + def fake(method, url, **kw): + calls.append((method, url, kw.get("json"))) + if method == "GET" and "/playlist/" in url: + if existing_count is None: + return _Resp(404, {}) + return _Resp(200, {"playlist": {"track": [{} for _ in range(existing_count)]}}) + if url.endswith("/playlist/create"): + return _Resp(200, {"playlist_mbid": "NEW-MBID"}) + return _Resp(200, {}) # item/add, item/delete, edit, delete + return fake, calls + + +def test_update_playlist_clears_then_re_adds_same_mbid(monkeypatch): + c = _client() + fake, calls = _route(existing_count=3) + monkeypatch.setattr(c, "_make_request_with_retry", fake) + res = c.update_playlist("EXIST-MBID", "New Title", _tracks(5)) + assert res["success"] is True and res["updated"] is True + assert res["playlist_mbid"] == "EXIST-MBID" # stable URL/MBID + assert res["added"] == 5 + urls = [u for _, u, _ in calls] + assert any("/item/delete" in u for u in urls) # cleared existing + assert any("/item/add" in u for u in urls) # re-added + assert any("/playlist/edit/EXIST-MBID" in u for u in urls) # title refreshed + + +def test_update_playlist_reports_gone(monkeypatch): + c = _client() + fake, _ = _route(existing_count=None) # GET 404 -> gone + monkeypatch.setattr(c, "_make_request_with_retry", fake) + res = c.update_playlist("DEAD-MBID", "T", _tracks(2)) + assert res["success"] is False + assert res["gone"] is True + + +def test_create_or_update_updates_when_existing(monkeypatch): + c = _client() + fake, _ = _route(existing_count=2) + monkeypatch.setattr(c, "_make_request_with_retry", fake) + res = c.create_or_update_playlist("T", _tracks(4), existing_mbid="EXIST") + assert res["updated"] is True and res["playlist_mbid"] == "EXIST" + + +def test_create_or_update_falls_back_to_create_when_gone(monkeypatch): + c = _client() + fake, _ = _route(existing_count=None) # remembered playlist deleted on LB + monkeypatch.setattr(c, "_make_request_with_retry", fake) + res = c.create_or_update_playlist("T", _tracks(4), existing_mbid="DEAD") + assert res["success"] is True + assert res["updated"] is False # fell back to create + assert res["playlist_mbid"] == "NEW-MBID" + + +def test_create_or_update_creates_when_no_existing(monkeypatch): + c = _client() + fake, _ = _route() + monkeypatch.setattr(c, "_make_request_with_retry", fake) + res = c.create_or_update_playlist("T", _tracks(3), existing_mbid=None) + assert res["playlist_mbid"] == "NEW-MBID" and res["updated"] is False + + +def test_delete_playlist(monkeypatch): + c = _client() + monkeypatch.setattr(c, "_make_request_with_retry", lambda *a, **k: _Resp(200, {})) + assert c.delete_playlist("MBID") is True + monkeypatch.setattr(c, "_make_request_with_retry", lambda *a, **k: _Resp(404, {})) + assert c.delete_playlist("MBID") is False diff --git a/web_server.py b/web_server.py index c2bd38b8..4ce8787f 100644 --- a/web_server.py +++ b/web_server.py @@ -27200,9 +27200,13 @@ def _run_playlist_export(job_id, playlist_id, title, mode): job['phase'] = 'pushing' from core.listenbrainz_client import ListenBrainzClient client = ListenBrainzClient() - res = client.create_playlist(title, jspf['playlist']['track']) + # Re-export updates the same LB playlist in place instead of duplicating it (#903). + existing = db.get_playlist_export_target(int(playlist_id), 'listenbrainz') + res = client.create_or_update_playlist(title, jspf['playlist']['track'], existing_mbid=existing) job['push'] = res if res.get('success'): + if res.get('playlist_mbid'): + db.set_playlist_export_target(int(playlist_id), 'listenbrainz', res['playlist_mbid']) job['phase'] = 'done' else: job['phase'] = 'error' From cefddd73b700cc7117481faf54d016ad8b42a884 Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Mon, 22 Jun 2026 22:48:21 -0700 Subject: [PATCH 23/23] Release 2.7.6: bump version + What's New / version modal + PR description + docker-publish default tag - _SOULSYNC_BASE_VERSION 2.7.5 -> 2.7.6 - docker-publish.yml default version_tag 2.7.5 -> 2.7.6 - pr_description.md rewritten for 2.7.6 (ListenBrainz export #903, YouTube Liked Music #902, Deep Scan data-loss guard #904, dashboard performance, #901/multi-disc/track-number fixes) - helper.js WHATS_NEW: new 2.7.6 block + earlier-versions summary - helper.js VERSION_MODAL_SECTIONS: 2.7.6 highlights lead; 2.7.5 rolled down ruff check . clean app-wide; export/#904/cookie suites green (54). --- .github/workflows/docker-publish.yml | 4 +- pr_description.md | 42 ++++++++-------- web_server.py | 2 +- webui/static/helper.js | 71 ++++++++++++++++------------ 4 files changed, 64 insertions(+), 55 deletions(-) diff --git a/.github/workflows/docker-publish.yml b/.github/workflows/docker-publish.yml index 5fa038f3..dc089154 100644 --- a/.github/workflows/docker-publish.yml +++ b/.github/workflows/docker-publish.yml @@ -9,9 +9,9 @@ on: workflow_dispatch: inputs: version_tag: - description: 'Version tag (e.g. 2.7.5)' + description: 'Version tag (e.g. 2.7.6)' required: true - default: '2.7.5' + default: '2.7.6' jobs: build-and-push: diff --git a/pr_description.md b/pr_description.md index 17de1921..501ebcc7 100644 --- a/pr_description.md +++ b/pr_description.md @@ -1,43 +1,41 @@ -# soulsync 2.7.5 — `dev` → `main` +# soulsync 2.7.6 — `dev` → `main` -patch release on top of 2.7.4. a fix-heavy cycle — matching & artwork accuracy, the HiFi preview mess — plus a few quality-of-life features (M3U import, per-playlist file naming, ignore-list management). +patch release on top of 2.7.5. the headline is going the *other* way with playlists — exporting them TO listenbrainz — plus youtube liked-music sync, a deep-scan data-loss guard, and a round of dashboard performance work. --- ## what's new -### matching & metadata accuracy -- **deezer track numbers** — a track grabbed from a deezer playlist/wishlist now gets its REAL in-album track number (e.g. "Apologize" = 16 on *Shock Value*), not the playlist index. deezer's playlist/search results don't carry the position, so we resolve it from the album endpoint. -- **special-edition cover art** — a special edition (e.g. *Clair Obscur: Expedition 33 (Gustave Edition)*) no longer ends up with the standard edition's art. musicbrainz albums were resolving cover art at release-GROUP scope (a single representative cover, ~always the standard), so a pinned release now prefers its OWN cover and only falls back to the group/provider art when the release has none of its own. -- **"The" dedup** — wanting "I Gotta Feeling" by "The Black Eyed Peas" when you own it under "Black Eyed Peas" (or vice-versa) no longer fails to match and re-downloads a duplicate. the matcher now searches both forms across a leading "The"; the confidence scorer still has the final say, so it can't merge genuinely different artists. +### export playlists to listenbrainz (#903) +soulsync already pulls playlists IN from everywhere — now it can push one back OUT. every mirrored-playlist card gets a 📤 export button: pick **download .jspf** (a standard playlist file you can hand-upload anywhere) or **sync to listenbrainz** (creates the playlist straight on your LB account). each track is matched to its musicbrainz *recording* id via a cheapest-first waterfall — cache → your library (`musicbrainz_recording_id`) → the file's own tag → a live musicbrainz lookup — with the result cached so the same song never costs twice. live "matching N/M · X matched" status shows on the card, and re-syncing **updates the same LB playlist in place** instead of making duplicates. tracks that can't be resolved to an MBID are skipped (LB requires them) and counted so you see the coverage. -### HiFi previews (#895) -HiFi was serving 30-second preview files dressed up as full songs (full length faked in the header). soulsync now rejects them three ways — short preview manifests, faked-header files that decode to ~30s, and a lossless-bitrate sanity check — and ABORTS the HiFi source instead of cascading down into a lower-tier copy of the same preview. (this was also an upstream Tidal-ban outage; the guard means you get a clean fail + fallback instead of a broken file.) +### youtube liked-music sync (#902) +you can now sync your youtube music **Liked Music** playlist (`music.youtube.com/playlist?list=LM`). it's a private playlist, so it needs auth — and the existing "read a browser's cookies" option only works when the browser is on the same machine. added a **"paste cookies.txt"** option in Settings → YouTube so server/docker installs (and anyone on a browser like Zen that yt-dlp can't read) can supply their login from anywhere. -### playlists -- **M3U / M3U8 import (#893)** — the "import from file" tool now reads M3U/M3U8 playlists (the most common file-playlist format, and the one soulsync itself exports). parses extended `#EXTINF` (artist/title/duration) and simple path-only playlists, and round-trips with soulsync's own export. -- **organize-by-playlist file naming** — an opt-in template (`$position - $artist - $title`) renames the files INSIDE each playlist folder so they sort/play the way you want (e.g. in playlist order on a dumb player). filename only — validated to reject "/" and require `$title` — defaults to empty (keep the library filename), and works for both symlink and copy modes. -- **find & add is remembered** — a manual match you set on a synced playlist is no longer forgotten on the next auto-sync. replace-mode re-matched from scratch and ignored your durable pick; the matcher now consults the durable manual-match table, not just the volatile cache a library rescan wipes. +### deep scan won't relocate your library (#904) +a standalone Deep Scan moves files it doesn't recognize into Staging for import. if the DB was empty/out of sync with disk (a volume swap, a DB reset, external tag edits), it treated your **entire** library as "unrecognized" and relocated all of it — one user lost ~1,500 tracks into Staging. now a guard refuses the move when the unrecognized share is implausibly large (the desync signature), leaves everything in place, and warns instead. plus a **"Transfer is my permanent library — never move files out"** toggle for people whose Transfer folder *is* their live library. -### ignore-list (#897) -- the wishlist ignore-list now has a "🚫 Ignored" button right on the wishlist page — it was buried in a modal most people never opened. -- manually re-adding a previously-cancelled track no longer gets silently blocked by the ignore-list. an explicit user add now bypasses + clears the ignore while keeping the real source type (so the Albums/Singles split is unaffected). +### dashboard performance +a pass at the "soulsync makes my GPU work hard just sitting there" complaints. the sidebar sweep animates `transform` instead of `left` (no per-frame layout), particle glows are pre-rendered sprites instead of per-frame gradients, blur radii + redundant/invisible card shadows are trimmed, and low-power machines auto-drop to performance mode. all the visible effects stay; they're just cheaper to draw. -### docker / packaging (#899) -the Unraid template pointed its `TemplateURL` / `Icon` at a dead third-party repo — now points at the canonical files in this repo (raw URLs, not the HTML `/blob/` ones), and maps `/app/MusicVideos` so music-video downloads land on a share instead of an anonymous volume. +### fixes +- **file-import manual matches stick (#901)** — a manual match on a file-imported playlist track is no longer forgotten on re-sync (the tracks now carry a stable id; existing ones are backfilled once). +- **manual match heals a stale Plex key** — a Find & Add match whose stored Plex ratingKey went stale is now re-resolved against a live Plex search instead of silently breaking. +- **multi-disc albums** — a track now files into the Disc folder that matches its own disc tag (no more disc-2 tracks landing in Disc 1), and a track is never written disc-less. +- **auto-download track numbers** — a track auto-grabbed from the playlist pipeline / wishlist / watchlist now gets its real in-album position instead of being tagged `1/1`. --- ## a brief recap of what came before -2.7.4 was **re-identify** (re-file an imported track under the right release without re-downloading) plus library/import cleanups (#889/#890/#891). 2.7.3 added the Quality Upgrade Finder and the wishlist ignore-list (#874). 2.7.2 brought playlist-folder mirroring + server-playlist M3U export; 2.7.1 added download verification + a review queue (#852); 2.7.0 made multi-user real — per-profile accounts, opt-in login, reverse-proxy support. +2.7.5 was a fix-heavy cycle — matching & artwork accuracy, the HiFi preview mess (#895) — plus M3U import (#893), ignore-list management (#897), and per-playlist file naming. 2.7.4 was re-identify; 2.7.3 the Quality Upgrade Finder + ignore-list (#874); 2.7.2 playlist-folder mirroring + M3U export; 2.7.1 download verification + a review queue (#852); 2.7.0 made multi-user real. --- ## tests -additive + gated — every new behavior is opt-in or defaults to today's behavior. new seam/regression tests across deezer track positions, the HiFi preview guards, the "The" dedup, M3U parsing, the ignore-list manual-add bypass, the playlist item-naming template, and the release-scope cover-art helper. relevant suites green; `ruff check .` clean app-wide. +additive + fail-safe — new behavior is opt-in or guarded, nothing existing rewired. new seam/regression suites across the listenbrainz export (JSPF build, the MBID waterfall + dedup, the persistent cache, the LB create/update-in-place client), the youtube cookie precedence, and the #904 deep-scan guard (incl. the 1,500-file regression). the listenbrainz push + update-in-place were also validated live against a real account. relevant suites green; `ruff check` clean on touched modules. ## post-merge -- [ ] tag `v2.7.5` on `main` -- [ ] docker-publish with `version_tag: 2.7.5` +- [ ] tag `v2.7.6` on `main` +- [ ] docker-publish with `version_tag: 2.7.6` - [ ] discord announce (auto-fired by the workflow) -- [ ] reply on #893 / #895 / #897 / #899 +- [ ] reply on #902 / #903 / #904 diff --git a/web_server.py b/web_server.py index 4ce8787f..eaa6bb47 100644 --- a/web_server.py +++ b/web_server.py @@ -40,7 +40,7 @@ logger = setup_logging(_log_level, _log_path) # App version — single source of truth for backup metadata, system-info, update check, etc. # Semver: MAJOR.MINOR.PATCH. Bump at each dev→main release. -_SOULSYNC_BASE_VERSION = "2.7.5" +_SOULSYNC_BASE_VERSION = "2.7.6" def _build_version_string(): """Append short commit hash to version when available (e.g. 2.35+abc1234).""" diff --git a/webui/static/helper.js b/webui/static/helper.js index 52eb7528..10173057 100644 --- a/webui/static/helper.js +++ b/webui/static/helper.js @@ -3404,18 +3404,17 @@ function closeHelperSearch() { const WHATS_NEW = { // Convention: keep only the CURRENT release here, plus a single brief // "Earlier versions" summary entry. Don't accumulate old per-version blocks. - '2.7.5': [ - { date: 'June 2026 — 2.7.5 release' }, - { title: 'Special-edition cover art', desc: 'a special edition (e.g. *Clair Obscur: Expedition 33 (Gustave Edition)*) no longer ends up with the standard edition\'s art. musicbrainz albums resolved cover art at release-GROUP scope (one representative cover, ~always the standard), so a pinned release now prefers its OWN cover and only falls back to the group/provider art when the release has none.', page: 'library' }, - { title: 'Deezer track numbers', desc: 'a track grabbed from a deezer playlist/wishlist now gets its REAL in-album track number (e.g. "Apologize" = 16 on Shock Value), not the playlist index — resolved from deezer\'s album endpoint, which playlist/search results don\'t carry.', page: 'downloads' }, - { title: '"The" duplicate fix', desc: 'wanting "I Gotta Feeling" by "The Black Eyed Peas" when you own it under "Black Eyed Peas" (or vice-versa) no longer fails to match and re-downloads a duplicate. the matcher now searches both forms across a leading "The"; the scorer still has the final say.', page: 'downloads' }, - { title: 'HiFi previews rejected (#895)', desc: 'HiFi was serving 30-second preview files dressed up as full songs (faked length in the header). soulsync now catches them — short manifests, faked-header files that decode to ~30s, and a lossless-bitrate check — and aborts the HiFi source instead of cascading into a lower-tier copy of the same preview.', page: 'downloads' }, - { title: 'Import M3U / M3U8 playlists (#893)', desc: 'the "import from file" tool now reads M3U/M3U8 playlists — extended #EXTINF (artist/title/duration) and simple path-only files — and round-trips with soulsync\'s own M3U export.', page: 'sync' }, - { title: 'Name files in playlist folders', desc: 'an opt-in template ($position - $artist - $title) renames the files INSIDE each Organize-by-Playlist folder so they sort/play the way you want. filename only (rejects "/", requires $title), defaults to keeping the library filename, works for symlink + copy modes.', page: 'settings' }, - { title: 'Manage the ignore-list (#897)', desc: 'the wishlist ignore-list now has a "🚫 Ignored" button right on the wishlist page (it was buried in a modal). and manually re-adding a previously-cancelled track no longer gets silently blocked — an explicit add bypasses + clears the ignore.', page: 'downloads' }, - { title: 'Find & Add is remembered', desc: 'a manual match you set on a synced playlist is no longer forgotten on the next auto-sync. replace-mode re-matched from scratch; the matcher now honors the durable manual-match, not just the volatile cache a rescan wipes.', page: 'sync' }, - { title: 'Unraid template fixes (#899)', desc: 'the Unraid template now points its TemplateURL / Icon at the canonical files in this repo (raw URLs, not the dead third-party ones), and maps /app/MusicVideos so music-video downloads land on a share.', page: 'settings' }, - { title: 'Earlier versions', desc: '2.7.4 added re-identify (re-file an imported track under the right release without re-downloading) + library/import cleanups (#889/#890/#891). 2.7.3 added the Quality Upgrade Finder and the wishlist ignore-list (#874); 2.7.2 brought playlist-folder mirroring + server-playlist M3U export; 2.7.1 added download verification + a review queue (#852); 2.7.0 made multi-user real — per-profile streaming accounts, opt-in login, reverse-proxy support.' }, + '2.7.6': [ + { date: 'June 2026 — 2.7.6 release' }, + { title: 'Export playlists to ListenBrainz (#903)', desc: 'soulsync already pulls playlists IN — now it can push one OUT. every mirrored-playlist card gets a 📤 export button: download a standard .jspf file, or sync the playlist straight to your ListenBrainz account. each track is matched to its musicbrainz recording id (cache → your library → file tag → live musicbrainz), with live "matching N/M" status on the card. re-syncing updates the SAME LB playlist in place instead of duplicating it.', page: 'sync' }, + { title: 'YouTube Liked Music sync (#902)', desc: 'you can now sync your youtube music "Liked Music" playlist (music.youtube.com/playlist?list=LM). it\'s private, so it needs auth — added a "paste cookies.txt" option in Settings → YouTube so server/docker installs (or browsers yt-dlp can\'t read, like Zen) can supply their login from anywhere.', page: 'settings' }, + { title: 'Deep Scan won\'t relocate your library (#904)', desc: 'a standalone Deep Scan moves unrecognized files into Staging. if the DB was empty/out of sync with disk, it treated your ENTIRE library as unrecognized and relocated all of it. now a guard refuses the move when the unrecognized share is implausibly large, leaves files in place, and warns — plus a "Transfer is my permanent library — never move files out" toggle.', page: 'settings' }, + { title: 'Dashboard performance', desc: 'a pass at "soulsync works my GPU hard just sitting there". the sidebar sweep animates transform not left, particle glows are cached sprites, blur radii + redundant/invisible card shadows are trimmed, and low-power machines auto-drop to performance mode. the effects all stay — they\'re just cheaper to draw.', page: 'dashboard' }, + { title: 'File-import manual matches stick (#901)', desc: 'a manual match on a file-imported playlist track is no longer forgotten on re-sync — the tracks now carry a stable id (existing ones backfilled once), so your pick survives.', page: 'sync' }, + { title: 'Manual match heals a stale Plex key', desc: 'a Find & Add match whose stored Plex ratingKey went stale is now re-resolved against a live Plex search instead of silently breaking on the next sync.', page: 'sync' }, + { title: 'Multi-disc albums', desc: 'a track now files into the Disc folder that matches its own disc tag (no more disc-2 tracks landing in Disc 1), and a track is never written disc-less.', page: 'downloads' }, + { title: 'Auto-download track numbers', desc: 'a track auto-grabbed from the playlist pipeline / wishlist / watchlist now gets its real in-album position instead of being tagged 1/1.', page: 'downloads' }, + { title: 'Earlier versions', desc: '2.7.5 was a fix-heavy cycle — matching & artwork accuracy, the HiFi preview mess (#895), M3U import (#893), ignore-list management (#897), per-playlist file naming. 2.7.4 added re-identify; 2.7.3 the Quality Upgrade Finder + ignore-list (#874); 2.7.2 playlist-folder mirroring + M3U export; 2.7.1 download verification + a review queue (#852); 2.7.0 made multi-user real.' }, ], }; @@ -3446,37 +3445,49 @@ const WHATS_NEW = { // usage_note?: 'optional hint shown at the bottom' } const VERSION_MODAL_SECTIONS = [ { - title: "Matching & metadata accuracy", - description: "a batch of fixes so downloads land with the right numbers, art, and no duplicates.", + title: "Export playlists to ListenBrainz (#903)", + description: "soulsync already pulls playlists IN from everywhere — now it can push one back OUT.", features: [ - "special-edition cover art — a pinned release (e.g. a \"Gustave Edition\") now uses its OWN cover instead of musicbrainz's release-group representative (usually the standard edition)", - "deezer track numbers — a track from a deezer playlist/wishlist gets its REAL in-album track number (e.g. \"Apologize\" = 16 on Shock Value), resolved from the album endpoint, not the playlist index", - "\"The\" dedup — \"The Black Eyed Peas\" and \"Black Eyed Peas\" now match across a leading \"The\", so a track you already own doesn't fail to match and re-download a duplicate", + "📤 export button on every mirrored-playlist card — download a standard .jspf file, or sync the playlist straight onto your ListenBrainz account", + "each track is matched to its musicbrainz recording id via a cheapest-first waterfall — cache → your library → the file's own tag → a live musicbrainz lookup — and the result is cached so the same song never costs twice", + "live \"matching N/M · X matched\" status on the card, and re-syncing updates the SAME LB playlist in place instead of making duplicates", + ], + usage_note: "find it on the 📤 button when you hover a mirrored playlist card. tracks that can't be resolved to a musicbrainz id are skipped (LB requires them) and counted, so you see the coverage.", + }, + { + title: "YouTube Liked Music sync (#902)", + description: "sync your youtube music \"Liked Music\" playlist (music.youtube.com/playlist?list=LM).", + features: [ + "it's a private playlist, so it needs auth — the existing browser-cookie option only works when the browser is on the same machine as soulsync", + "added a \"paste cookies.txt\" option in Settings → YouTube so server/docker installs (or browsers yt-dlp can't read, like Zen) can supply their login from anywhere", ], }, { - title: "HiFi previews (#895)", - description: "HiFi was serving 30-second preview files disguised as full songs (full length faked in the header).", + title: "Deep Scan won't relocate your library (#904)", + description: "a standalone Deep Scan moves unrecognized files into Staging — which went very wrong when the DB was out of sync with disk.", features: [ - "rejects them three ways — short preview manifests, faked-header files that decode to ~30s, and a lossless-bitrate sanity check", - "aborts the HiFi source instead of cascading into a lower-tier copy of the same preview — you get a clean fail + fallback, not a broken file", + "if the DB was empty/desynced, a scan treated your ENTIRE library as unrecognized and relocated all of it (one user lost ~1,500 tracks into Staging)", + "now a guard refuses the move when the unrecognized share is implausibly large (the desync signature), leaves everything in place, and warns instead", + "new \"Transfer is my permanent library — never move files out\" toggle for people whose Transfer folder IS their live library", ], }, { - title: "Playlists", - description: "import from more formats, name files your way, and keep your manual matches.", + title: "Dashboard performance + fixes", + description: "lighter dashboard, and a handful of matching/import fixes.", features: [ - "#893 — import M3U / M3U8 playlists in the \"import from file\" tool (extended #EXTINF + simple path-only files); round-trips with soulsync's own M3U export", - "organize-by-playlist file naming — an opt-in template ($position - $artist - $title) renames the files inside each playlist folder so they sort/play in order; filename only, defaults to keeping the library name, works for symlink + copy", - "find & add is remembered — a manual match on a synced playlist survives the next auto-sync; the matcher honors the durable manual-match, not just the volatile cache a rescan wipes", + "dashboard — the sidebar sweep animates transform not left, particle glows are cached sprites, blur/shadow excess is trimmed, and low-power machines auto-drop to performance mode (the effects all stay, just cheaper to draw)", + "#901 — a manual match on a file-imported playlist track now survives re-sync (stable ids, existing ones backfilled)", + "a Find & Add match whose stored Plex key went stale is re-resolved against a live Plex search instead of breaking", + "multi-disc albums file each track in the Disc folder matching its tag (no disc-less / wrong-disc tracks); auto-grabbed tracks get their real in-album number instead of 1/1", ], }, { - title: "Ignore-list & packaging", - description: "manage the wishlist ignore-list, and a couple of Unraid template fixes.", + title: "Earlier in 2.7.5", + description: "a fix-heavy cycle — matching & artwork accuracy plus a few quality-of-life features.", features: [ - "#897 — a \"🚫 Ignored\" button now lives right on the wishlist page (it was buried in a modal), and manually re-adding a previously-cancelled track no longer gets silently blocked", - "#899 — the Unraid template points TemplateURL / Icon at the canonical files in this repo (not a dead third-party repo) and maps /app/MusicVideos so music-video downloads land on a share", + "special-edition cover art (a \"Gustave Edition\" uses its OWN cover), deezer track numbers, and the \"The\" duplicate fix", + "HiFi 30-second previews disguised as full songs are caught and rejected (#895)", + "import M3U / M3U8 playlists (#893), name files inside playlist folders, find & add remembered, ignore-list management (#897), Unraid template fixes (#899)", ], }, {