From 6e622d30f12054f76309d20a3841519703130d39 Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Mon, 22 Jun 2026 08:53:24 -0700 Subject: [PATCH] #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")