From 1d16ac79783d3fee0694a7696ec84c88dd8f00f5 Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Tue, 9 Jun 2026 13:47:25 -0700 Subject: [PATCH 01/67] Downloads: reuse an album's existing folder so batches don't split it (#829) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Tacobell444: when tracks land in an album across multiple batches (a wishlist run, the Album Completeness job, a missed track re-downloaded later), the folder is rebuilt from API metadata each time — so when $albumtype or $year come back blank/different on a later batch, the folder NAME changes and the album splits, forcing a Reorganize. Fix: build_final_path_for_track now checks whether the album already lives in a single folder on disk and, if so, drops the new track there instead of a freshly templated folder. Match (chosen): exact stored Spotify album id first, then a STRICT >=0.85 name+artist match (vs the 0.7 used elsewhere) — a wrong match here misplaces a file. New core/library/existing_album_folder.resolve_existing_album_folder holds the logic; always-on with template fallback. Safety rails: only returns a folder UNDER the transfer dir (never a read-only library/NAS mount), only when the album lives in EXACTLY ONE folder (multiple = disc subfolders, which DatabaseTrack can't disambiguate — those defer to the template), and any failure falls through to the template path. Added MusicDatabase.get_album_by_spotify_album_id for the id-first lookup. Tests: single-folder reuse, no-match, below-threshold, multi-folder defer, outside-transfer reject, id-first, missing transfer dir, no-files-on-disk. 8 tests; 1556 path/import/download tests pass (only the known soundcloud failures remain). --- core/imports/paths.py | 39 ++++++++ core/library/existing_album_folder.py | 129 ++++++++++++++++++++++++++ database/music_database.py | 40 +++++++- tests/test_existing_album_folder.py | 114 +++++++++++++++++++++++ 4 files changed, 320 insertions(+), 2 deletions(-) create mode 100644 core/library/existing_album_folder.py create mode 100644 tests/test_existing_album_folder.py diff --git a/core/imports/paths.py b/core/imports/paths.py index 5744744e..6546509f 100644 --- a/core/imports/paths.py +++ b/core/imports/paths.py @@ -587,6 +587,45 @@ def build_final_path_for_track(context, artist_context, album_info, file_ext, cr disc_label = _get_config_manager().get("file_organization.disc_label", "Disc") folder_path, filename_base = get_file_path_from_template(template_context, "album_path") + + # #829: if this album already lives in a single folder on disk, drop the + # new track there instead of a freshly-templated folder — this is what + # keeps an album from splitting when $albumtype/$year drift between + # batches (wishlist, Album Completeness, a missed track later). Strict + # match + transfer-dir-only + single-folder-only inside the resolver; + # any miss falls through to the template path below. Best-effort. + reuse_folder = None + if filename_base: + try: + from core.library.existing_album_folder import resolve_existing_album_folder + from database.music_database import get_database + try: + _active_server = _get_config_manager().get_active_media_server() + except Exception: + _active_server = None + _spotify_album_id = (album_context.get("id") + if album_context and str(source).startswith("spotify") else None) + _expected_tracks = None + if album_context and album_context.get("total_tracks"): + _expected_tracks = _coerce_int(album_context.get("total_tracks"), 0) or None + reuse_folder = resolve_existing_album_folder( + db=get_database(), + transfer_dir=transfer_dir, + album_name=album_info.get("album_name"), + album_artist=template_context.get("albumartist"), + spotify_album_id=_spotify_album_id, + active_server=_active_server, + expected_track_count=_expected_tracks, + config_manager=_get_config_manager(), + ) + except Exception as _reuse_err: + logger.debug("[Existing Album Folder] lookup failed: %s", _reuse_err) + reuse_folder = None + if reuse_folder and filename_base: + final_path = os.path.join(reuse_folder, filename_base + file_ext) + _ensure_dir(reuse_folder, exist_ok=True) + return final_path, True + if folder_path and filename_base: if total_discs > 1 and not user_controls_disc: disc_folder = f"{disc_label} {disc_number}" diff --git a/core/library/existing_album_folder.py b/core/library/existing_album_folder.py new file mode 100644 index 00000000..926ab173 --- /dev/null +++ b/core/library/existing_album_folder.py @@ -0,0 +1,129 @@ +"""Reuse an album's existing on-disk folder for new downloads (#829). + +When tracks are added to an album across multiple batches (a wishlist run, the +Album Completeness job, a missed track re-downloaded later), the destination +folder is normally rebuilt from API metadata each time. If ``$albumtype`` or +``$year`` come back blank/different on a later batch, the folder *name* changes +and the album splits across folders — forcing a Reorganize afterwards. + +This resolves the folder the album *already* lives in so the new track joins its +existing files instead. Matching is deliberately conservative: the exact stored +Spotify album id first (definitive), then a STRICT (>= 0.85) name+artist match — +higher than the 0.7 used elsewhere, because a wrong match here misplaces a file. + +Safety rails: + * Only ever returns a folder UNDER the transfer dir (the managed download + tree) — never a read-only library/NAS mount the resolver happens to find. + * Only reuses when the album lives in EXACTLY ONE folder on disk. Multiple + folders means disc subfolders (DatabaseTrack carries no disc number, so we + can't safely pick the right one) — those defer to the template path. + * Any failure returns None — the caller falls back to the normal template. +""" + +from __future__ import annotations + +import os +from typing import Any, Optional + +from core.library.path_resolver import resolve_library_file_path +from utils.logging_config import get_logger + +logger = get_logger("library.existing_album_folder") + +# Strict — a wrong album match drops the file in the wrong folder. +_STRICT_ALBUM_CONFIDENCE = 0.85 + + +def _is_under(child: str, parent: str) -> bool: + """True if ``child`` is the same as or inside ``parent`` (normalized).""" + try: + child_n = os.path.normcase(os.path.normpath(os.path.abspath(child))) + parent_n = os.path.normcase(os.path.normpath(os.path.abspath(parent))) + return child_n == parent_n or child_n.startswith(parent_n + os.sep) + except Exception: + return False + + +def _find_album(db: Any, spotify_album_id: Optional[str], album_name: Optional[str], + album_artist: Optional[str], active_server: Optional[str], + expected_track_count: Optional[int]): + """Stored Spotify id first, then a strict name+artist match. None on no match.""" + if spotify_album_id: + try: + album = db.get_album_by_spotify_album_id(spotify_album_id) + if album: + return album + except Exception as e: + logger.debug("album-by-spotify-id lookup failed: %s", e) + if album_name and album_artist: + try: + match, confidence = db.check_album_exists_with_editions( + title=album_name, artist=album_artist, + confidence_threshold=_STRICT_ALBUM_CONFIDENCE, + expected_track_count=expected_track_count, + server_source=active_server, + ) + if match and confidence >= _STRICT_ALBUM_CONFIDENCE: + return match + except Exception as e: + logger.debug("strict album name+artist match failed: %s", e) + return None + + +def resolve_existing_album_folder( + *, + db: Any, + transfer_dir: Optional[str], + album_name: Optional[str] = None, + album_artist: Optional[str] = None, + spotify_album_id: Optional[str] = None, + active_server: Optional[str] = None, + expected_track_count: Optional[int] = None, + config_manager: Any = None, + resolver=resolve_library_file_path, +) -> Optional[str]: + """Return the on-disk folder an existing album lives in (so a new track joins + it) or None to fall back to the templated path. See module docstring.""" + if not transfer_dir or not os.path.isdir(transfer_dir): + return None + if not db: + return None + + album = _find_album(db, spotify_album_id, album_name, album_artist, + active_server, expected_track_count) + if not album: + return None + + try: + tracks = db.get_tracks_by_album(album.id) + except Exception as e: + logger.debug("get_tracks_by_album(%s) failed: %s", getattr(album, 'id', '?'), e) + return None + + folders = set() + for t in tracks: + file_path = getattr(t, 'file_path', None) + if not file_path: + continue + try: + resolved = resolver(file_path, transfer_folder=transfer_dir, + config_manager=config_manager) + except Exception: + resolved = None + if not resolved: + continue + folder = os.path.dirname(resolved) + if _is_under(folder, transfer_dir): + folders.add(os.path.normpath(folder)) + + # Single folder under the transfer dir → reuse it. Zero (nothing on disk yet) + # or many (disc subfolders) → let the template decide. + if len(folders) == 1: + reuse = next(iter(folders)) + logger.info("[Existing Album Folder] Reusing '%s' for album '%s'", + reuse, getattr(album, 'title', album_name)) + return reuse + return None + + +__all__ = ["resolve_existing_album_folder"] diff --git a/database/music_database.py b/database/music_database.py index 5acbf0af..202b2d4c 100644 --- a/database/music_database.py +++ b/database/music_database.py @@ -6260,11 +6260,47 @@ class MusicDatabase: )) return tracks - + except Exception as e: logger.error(f"Error getting tracks for album {album_id}: {e}") return [] - + + def get_album_by_spotify_album_id(self, spotify_album_id: str) -> Optional[DatabaseAlbum]: + """Fetch a single album by its (enriched) Spotify album id, or None. + + Used by the download path builder (#829) to reuse an album's existing + on-disk folder when re-downloading into the same album — matching the + exact stored Spotify id before falling back to fuzzy name+artist. + """ + if not spotify_album_id: + return None + try: + conn = self._get_connection() + cursor = conn.cursor() + cursor.execute(""" + SELECT albums.*, artists.name as artist_name + FROM albums + JOIN artists ON albums.artist_id = artists.id + WHERE albums.spotify_album_id = ? + LIMIT 1 + """, (spotify_album_id,)) + row = cursor.fetchone() + if not row: + return None + genres = json.loads(row['genres']) if row['genres'] else None + album = DatabaseAlbum( + id=row['id'], artist_id=row['artist_id'], title=row['title'], + year=row['year'], thumb_url=row['thumb_url'], genres=genres, + track_count=row['track_count'], duration=row['duration'], + created_at=datetime.fromisoformat(row['created_at']) if row['created_at'] else None, + updated_at=datetime.fromisoformat(row['updated_at']) if row['updated_at'] else None, + ) + album.artist_name = row['artist_name'] + return album + except Exception as e: + logger.error(f"Error getting album by spotify_album_id {spotify_album_id}: {e}") + return None + def search_artists(self, query: str, limit: int = 50, server_source: str = None) -> List[DatabaseArtist]: """Search artists by name, optionally filtered by server source. Uses diacritic-insensitive matching so 'Tiesto' finds 'Tiësto'.""" diff --git a/tests/test_existing_album_folder.py b/tests/test_existing_album_folder.py new file mode 100644 index 00000000..24b6f82b --- /dev/null +++ b/tests/test_existing_album_folder.py @@ -0,0 +1,114 @@ +"""Reuse an album's existing on-disk folder for new downloads (#829). + +Tacobell444: tracks added to an album across batches split into different folders +when $albumtype/$year drift. The resolver finds the album's existing single +folder (under the transfer dir) so the new track joins it. These pin the safety +rails: strict match, transfer-dir-only, single-folder-only, id-first. +""" + +from __future__ import annotations + +import os +from types import SimpleNamespace + +from core.library.existing_album_folder import resolve_existing_album_folder + + +class _FakeDb: + def __init__(self, album=None, album_conf=0.0, tracks=None, by_spotify=None): + self._album = album + self._album_conf = album_conf + self._tracks = tracks or [] + self._by_spotify = by_spotify + + def get_album_by_spotify_album_id(self, sid): + return self._by_spotify + + def check_album_exists_with_editions(self, title, artist, confidence_threshold=0.8, + expected_track_count=None, server_source=None, **kw): + return (self._album, self._album_conf) + + def get_tracks_by_album(self, album_id): + return self._tracks + + +def _track(path): + return SimpleNamespace(file_path=path) + + +def _album(id=1, title="Ocean Avenue"): + return SimpleNamespace(id=id, title=title) + + +def _mkfile(folder, name): + folder.mkdir(parents=True, exist_ok=True) + f = folder / name + f.write_text("x") + return str(f) + + +def test_reuses_single_folder_under_transfer(tmp_path): + album_dir = tmp_path / "Yellowcard - Ocean Avenue" + f1 = _mkfile(album_dir, "01 - Way Away.mp3") + f2 = _mkfile(album_dir, "02 - Breathing.mp3") + db = _FakeDb(album=_album(), album_conf=0.95, tracks=[_track(f1), _track(f2)]) + out = resolve_existing_album_folder( + db=db, transfer_dir=str(tmp_path), + album_name="Ocean Avenue", album_artist="Yellowcard") + assert out == os.path.normpath(str(album_dir)) + + +def test_no_match_returns_none(tmp_path): + db = _FakeDb(album=None, album_conf=0.0) + assert resolve_existing_album_folder( + db=db, transfer_dir=str(tmp_path), album_name="X", album_artist="Y") is None + + +def test_below_strict_threshold_returns_none(tmp_path): + f = _mkfile(tmp_path / "A", "1.mp3") + # 0.80 < the resolver's 0.85 strict gate -> not reused. + db = _FakeDb(album=_album(), album_conf=0.80, tracks=[_track(f)]) + assert resolve_existing_album_folder( + db=db, transfer_dir=str(tmp_path), album_name="A", album_artist="B") is None + + +def test_multi_folder_defers_to_template(tmp_path): + f1 = _mkfile(tmp_path / "Album" / "Disc 01", "1.mp3") + f2 = _mkfile(tmp_path / "Album" / "Disc 02", "1.mp3") + db = _FakeDb(album=_album(), album_conf=0.95, tracks=[_track(f1), _track(f2)]) + assert resolve_existing_album_folder( + db=db, transfer_dir=str(tmp_path), album_name="A", album_artist="B") is None + + +def test_folder_outside_transfer_returns_none(tmp_path): + f = _mkfile(tmp_path / "outside", "1.mp3") + transfer = tmp_path / "transfer" + transfer.mkdir() + db = _FakeDb(album=_album(), album_conf=0.95, tracks=[_track(f)]) + assert resolve_existing_album_folder( + db=db, transfer_dir=str(transfer), album_name="A", album_artist="B") is None + + +def test_id_first_match_skips_name_lookup(tmp_path): + album_dir = tmp_path / "Album" + f = _mkfile(album_dir, "1.mp3") + # name match would FAIL (album=None); the stored spotify id hits. + db = _FakeDb(album=None, album_conf=0.0, tracks=[_track(f)], by_spotify=_album()) + out = resolve_existing_album_folder( + db=db, transfer_dir=str(tmp_path), spotify_album_id="sp123", + album_name="X", album_artist="Y") + assert out == os.path.normpath(str(album_dir)) + + +def test_missing_transfer_dir_returns_none(tmp_path): + db = _FakeDb(album=_album(), album_conf=0.95, tracks=[]) + assert resolve_existing_album_folder( + db=db, transfer_dir=str(tmp_path / "nope"), album_name="A", album_artist="B") is None + + +def test_album_with_no_files_on_disk_returns_none(tmp_path): + # Album matched but its tracks have no resolvable file -> nothing to reuse. + db = _FakeDb(album=_album(), album_conf=0.95, + tracks=[_track("/gone/1.mp3"), _track(None)]) + assert resolve_existing_album_folder( + db=db, transfer_dir=str(tmp_path), album_name="A", album_artist="B") is None From db65e783c7acdabcf0a7a9f99db8a4104a0a1e2c Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Tue, 9 Jun 2026 14:35:54 -0700 Subject: [PATCH 02/67] Discography: keep collab tracks credited as one combined string (#830) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Vicky-2418: Download Discography skipped some albums/singles as "No New Track" even on a brand-new artist, while manual one-by-one worked. The log showed the real reason wasn't ownership at all — it was "N skipped (artist mismatch)". Root cause (confirmed against the iTunes API, not guessed): iTunes returns a collab as ONE combined string. Narvent's "Miss You (Ambient Remix)" is credited 'TRVNSPORTER, Narvent & SKVLENT'. track_artist_matches did an exact full-string compare ('trvnsporter, narvent & skvlent' == 'narvent' → False), so every collaborator's discography entry was dropped — despite the #559 comment claiming it "keeps features" (it didn't, because features arrive combined, not as a list). Fix: match the requested artist as a COMPONENT of the credit — split on the common separators (, & ; / feat ft featuring vs x), while still including the full string so band names with internal separators ("Florence + the Machine") match exactly. Component matching stays exact per name, so true contamination (the #559 case — artist not credited at all) is still dropped, and substrings ("Drakeo the Ruler" ≠ "Drake") still don't match. Also corrects an over-conservative #559 test that asserted "Drake & Future" shouldn't match "Drake" — it should; Drake is a credited collaborator. The guard drops uncredited artists, not legit collabs packed into one string. Tests: the exact real case (Narvent in the combined credit) + each collaborator, contamination still dropped, feat/ft/featuring/x forms, no substring false positive. 36 filter tests + 747 discography/metadata tests pass. --- core/metadata/discography_filters.py | 42 ++++++++++++++++++---- tests/metadata/test_discography_filters.py | 41 ++++++++++++++++++--- 2 files changed, 72 insertions(+), 11 deletions(-) diff --git a/core/metadata/discography_filters.py b/core/metadata/discography_filters.py index 4e1e0ecd..c5985fdf 100644 --- a/core/metadata/discography_filters.py +++ b/core/metadata/discography_filters.py @@ -28,8 +28,31 @@ single source of truth. from __future__ import annotations +import re from typing import Any, Dict, List, Optional +# Split a combined artist credit into its individual artists. Sources like +# iTunes return collabs as ONE string ("TRVNSPORTER, Narvent & SKVLENT"), not a +# list — so an exact full-string compare drops every collaborator's discography +# entry (#830). Split on the common credit separators; " and " / " with " are +# deliberately excluded (too many real band names contain them). +_ARTIST_CREDIT_SPLIT_RE = re.compile( + r"\s*[,&;/]\s*|\s+(?:feat\.?|ft\.?|featuring|vs\.?|x)\s+", + re.IGNORECASE, +) + + +def _artist_credit_components(name: str) -> List[str]: + """Return the individual artist names within a (possibly combined) credit, + always including the full string itself (so exact band names with internal + separators still match).""" + name = (name or "").strip() + if not name: + return [] + parts = [name] + parts.extend(p.strip() for p in _ARTIST_CREDIT_SPLIT_RE.split(name) if p.strip()) + return parts + from core.watchlist_scanner import ( is_acoustic_version, is_instrumental_version, @@ -47,10 +70,12 @@ def track_artist_matches(track_artists: Any, requested_artist_name: str) -> bool what the discography fetch returns), or the list-of-dicts shape that some upstreams pass directly. Both are accepted. - Returns True for primary-artist tracks AND feature appearances — - the requested artist need only be one of the listed artists. Only - drops tracks where the requested artist isn't named at all (the - cross-artist compilation case from #559). + Returns True for primary-artist tracks AND feature/collab appearances — + the requested artist need only be one of the credited artists, INCLUDING + when a source (iTunes, etc.) packs the collab into one combined string like + "TRVNSPORTER, Narvent & SKVLENT" (#830). Only drops tracks where the + requested artist isn't credited at all (the cross-artist compilation case + from #559). """ if not requested_artist_name: # No artist to compare against — don't filter; let the caller @@ -70,8 +95,13 @@ def track_artist_matches(track_artists: Any, requested_artist_name: str) -> bool name = entry.get('name', '') or '' else: name = str(entry or '') - if name.strip().lower() == target: - return True + # Match the requested artist as a component of the credit, so combined + # collab strings ("A, B & C") keep B's discography entry. Component + # matching is still exact per-name, so true contamination (the artist + # genuinely absent) is dropped exactly as before. + for component in _artist_credit_components(name): + if component.strip().lower() == target: + return True return False diff --git a/tests/metadata/test_discography_filters.py b/tests/metadata/test_discography_filters.py index 400e0885..8c297d05 100644 --- a/tests/metadata/test_discography_filters.py +++ b/tests/metadata/test_discography_filters.py @@ -61,6 +61,35 @@ class TestTrackArtistMatches: assert track_artist_matches(['DRAKE'], 'Drake') is True assert track_artist_matches(['Drake'], 'drake') is True + def test_combined_collab_credit_string_matches_component(self): + """#830 (Vicky-2418): iTunes packs a collab into ONE string. The + requested artist is one of several credited — must match. This is the + exact real case from the report (Narvent's 'Miss You (Ambient Remix)').""" + credit = ['TRVNSPORTER, Narvent & SKVLENT'] + assert track_artist_matches(credit, 'Narvent') is True + assert track_artist_matches(credit, 'TRVNSPORTER') is True + assert track_artist_matches(credit, 'SKVLENT') is True + + def test_combined_credit_still_drops_absent_artist(self): + """The #559 contamination guard survives: an artist genuinely absent + from a combined credit is still dropped.""" + assert track_artist_matches(['TRVNSPORTER, Narvent & SKVLENT'], 'Drake') is False + + def test_feat_forms_match_component(self): + for credit in (['Drake feat. Narvent'], ['Drake ft. Narvent'], + ['Drake featuring Narvent'], ['Drake x Narvent']): + assert track_artist_matches(credit, 'Narvent') is True + + def test_no_substring_false_positive(self): + """Component matching is exact per-name — a longer name that merely + contains the target must NOT match.""" + assert track_artist_matches(['Narventos'], 'Narvent') is False + + def test_band_name_with_internal_separator_still_matches_exactly(self): + """A real band name containing a separator still matches as the full + string (we keep the whole credit as a candidate alongside the split).""" + assert track_artist_matches(['Florence + the Machine'], 'Florence + the Machine') is True + def test_match_handles_whitespace_padding(self): """Trailing whitespace in either side mustn't break the match.""" assert track_artist_matches([' Drake '], 'Drake') is True @@ -87,12 +116,14 @@ class TestTrackArtistMatches: assert track_artist_matches([{'name': 'Drake'}], 'Drake') is True assert track_artist_matches([{'name': 'Random'}], 'Drake') is False - def test_substring_does_not_match(self): - """A song by "Drake & Future" should not match "Drake" via - substring — that's exactly the false-positive case the bug - report describes. Exact full-name match only.""" - assert track_artist_matches(['Drake & Future'], 'Drake') is False + def test_substring_does_not_match_but_component_does(self): + """No SUBSTRING matching — "Drakeo the Ruler" must not match "Drake". + But a combined collab credit IS component-matched (#830): "Drake & + Future" matches "Drake" because Drake is genuinely one of the credited + artists. The #559 guard drops artists who aren't credited AT ALL, not + legit collaborators packed into one string by sources like iTunes.""" assert track_artist_matches(['Drakeo the Ruler'], 'Drake') is False + assert track_artist_matches(['Drake & Future'], 'Drake') is True # --------------------------------------------------------------------------- From ca040d2c1074314fb4de1781f3cadf6f901b2738 Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Tue, 9 Jun 2026 14:53:30 -0700 Subject: [PATCH 03/67] Discography UI: show WHY tracks were skipped, not a flat "No new tracks" (#830) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The per-album status only looked at added + the generic wishlist-skip count, so anything else (other-artist credit, already owned, content-filtered) showed the misleading "No new tracks" — which is what made Vicky-2418's artist-mismatch skips look like "you already have it." The backend already streams the full breakdown (tracks_skipped_artist/owned/filter); the UI just ignored it. New shared _discogItemStatus(data) builds an accurate line from all the counts: "4 by other artists", "13 already owned", "1 added, 2 already owned", etc. Replaces the duplicated 4-line block at both render sites. Frontend only; JS syntax + per-case output verified. --- webui/static/library.js | 26 ++++++++++++++++++-------- 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/webui/static/library.js b/webui/static/library.js index eb8dc6b0..34823ce6 100644 --- a/webui/static/library.js +++ b/webui/static/library.js @@ -2794,10 +2794,7 @@ async function startDiscographyDownload() { item.classList.remove('active'); if (data.status === 'done') { - const parts = []; - if (data.tracks_added > 0) parts.push(`${data.tracks_added} added`); - if (data.tracks_skipped > 0) parts.push(`${data.tracks_skipped} skipped`); - statusEl.textContent = parts.join(', ') || 'No new tracks'; + statusEl.textContent = _discogItemStatus(data); iconEl.innerHTML = data.tracks_added > 0 ? '✓' : '—'; item.classList.add(data.tracks_added > 0 ? 'done' : 'skipped'); } else if (data.status === 'error') { @@ -2814,6 +2811,22 @@ async function startDiscographyDownload() { } } +// Build a clear per-album status from the discography stream payload. The +// backend already reports WHY tracks weren't added — other-artist credit, +// already owned/queued, or content-filtered — so surface that instead of a +// misleading "No new tracks" (#830: collab tracks dropped for "artist mismatch" +// looked identical to "you already have it"). +function _discogItemStatus(data) { + const parts = []; + const added = data.tracks_added || 0; + if (added > 0) parts.push(`${added} added`); + if ((data.tracks_skipped_owned || 0) > 0) parts.push(`${data.tracks_skipped_owned} already owned`); + if ((data.tracks_skipped || 0) > 0) parts.push(`${data.tracks_skipped} already queued`); + if ((data.tracks_skipped_artist || 0) > 0) parts.push(`${data.tracks_skipped_artist} by other artists`); + if ((data.tracks_skipped_filter || 0) > 0) parts.push(`${data.tracks_skipped_filter} filtered out`); + return parts.join(', ') || 'No tracks'; +} + function _handleDiscogProgress(data) { if (data.type === 'album') { const item = document.getElementById(`discog-prog-${data.album_id}`); @@ -2826,10 +2839,7 @@ function _handleDiscogProgress(data) { statusEl.textContent = `Processing ${data.tracks_total} tracks...`; item.classList.add('active'); } else if (data.status === 'done') { - const parts = []; - if (data.tracks_added > 0) parts.push(`${data.tracks_added} added`); - if (data.tracks_skipped > 0) parts.push(`${data.tracks_skipped} skipped`); - statusEl.textContent = parts.join(', ') || 'No new tracks'; + statusEl.textContent = _discogItemStatus(data); iconEl.innerHTML = data.tracks_added > 0 ? '✓' : '—'; item.classList.remove('active'); item.classList.add(data.tracks_added > 0 ? 'done' : 'skipped'); From 0939585620d14cefe07f796b3ffb8d945988ce51 Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Tue, 9 Jun 2026 15:38:49 -0700 Subject: [PATCH 04/67] Matcher: bracketed subtitles no longer read as different songs (#825) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit carlosjfcasero round 2 (manual-add fix didn't help — different path). His log pinned it: the mirrored sync auto-added 'Llamando a la tierra (Serenade From the Stars)' by M-Clan every run even though his library has the song (stored bare). Reproduced exactly: the subtitle restates no album context, so the #808 context strip keeps it, and the length-ratio penalty in _calculate_track_confidence crushes the pair to 0.142 (needs 0.7). Sync → "missing" → wishlist, forever; and the cleanup uses the SAME matcher, so it deterministically never removed it. Self-reinforcing. Fix at the matcher seam (benefits sync, cleanup, downloads, discography alike): core/text/title_match.strip_subtitle_qualifiers(title, other) strips a bracketed qualifier only when it (a) isn't restated in the other title, (b) contains no version-marker token (EN + ES: live/remix/acoustic/version/ dueto/directo/vivo/...), and (c) introduces no new digit token ('(Pt. 2)', '(2007)' stay different releases). Wired as a third comparison variant in _calculate_track_confidence with its own length guard. Verified against his log's other unmatched tracks: '(Live)' 0.15, '(Dueto 2007)' 0.179, '(Versión 1988)' 0.167 all still correctly blocked — version qualifiers keep their meaning; the M-Clan case goes 0.142 → 1.0 in both directions. Also: sync's check_track_exists call now passes album= (cleanup already did), enabling the album-aware fallback for multi-artist albums during sync. Tests: tests/test_subtitle_qualifier_match.py — the reported case verbatim (end-to-end through check_track_exists, both directions, batched candidate path included), EN+ES version qualifiers still blocked, numeric guard, '#769 Dani California' and '#808 OurVinyl' guards still hold. 1396 matcher/wishlist/sync tests pass. --- core/text/title_match.py | 72 +++++++++++ database/music_database.py | 18 +++ services/sync_service.py | 4 + tests/test_subtitle_qualifier_match.py | 158 +++++++++++++++++++++++++ 4 files changed, 252 insertions(+) create mode 100644 tests/test_subtitle_qualifier_match.py diff --git a/core/text/title_match.py b/core/text/title_match.py index 95f232f2..85e1c10b 100644 --- a/core/text/title_match.py +++ b/core/text/title_match.py @@ -115,6 +115,77 @@ def strip_redundant_context_qualifiers(title: str, *context_texts: str) -> str: return re.sub(r"\s+", " ", out).strip() +# Qualifier tokens that mark a genuinely DIFFERENT recording/cut — these must +# keep blocking a match. Union of the matching-engine keyword lists plus the +# Spanish markers seen in real libraries (#825: 'En Directo…', 'Versión 1988', +# 'Dueto 2007'). Titles reaching the matcher are unidecode-normalized, so the +# ASCII forms ('version') cover the accented ones ('versión'). +_VERSION_MARKER_TOKENS = frozenset({ + # English + "remix", "mix", "rmx", "live", "acoustic", "unplugged", "instrumental", + "karaoke", "demo", "demos", "edit", "version", "versions", "remaster", + "remastered", "slowed", "reverb", "sped", "spedup", "speedup", "extended", + "club", "mashup", "bootleg", "cover", "covers", "reprise", "session", + "sessions", "mono", "stereo", "duet", "rework", "dub", "vip", "single", + "radio", "alt", "alternate", "alternative", "take", "edition", "orchestral", + "symphonic", "piano", "acapella", "cappella", "nightcore", + # Distinct-track qualifiers — '(Interlude)' etc. are SEPARATE short tracks + # that share the base name with the full song; never treat as subtitles. + "interlude", "intro", "outro", "skit", "freestyle", "medley", "snippet", + # Part/volume markers whose number can be non-numeric ('Pt. II') — the + # digit guard below only catches actual digits. + "pt", "part", "vol", "ii", "iii", "iv", "vi", "vii", "viii", + # Spanish (unidecode-normalized; 'versión' → 'version' is covered above) + "directo", "vivo", "dueto", +}) + + +def strip_subtitle_qualifiers(title: str, other_title: str) -> str: + """Remove bracketed qualifiers that are SUBTITLES, not version markers. + + #825 (carlosjfcasero): the wishlist held 'Llamando a la tierra (Serenade + From the Stars)' — the song's official subtitle — while the library track + was the bare 'Llamando a la tierra'. The qualifier appears in no album or + counterpart title, so :func:`strip_redundant_context_qualifiers` keeps it, + and the length-ratio penalty then crushes an obviously-same song to ~0.14. + The sync matcher reported it missing on every run (re-adding it to the + wishlist) and the cleanup — same matcher — could never remove it. + + A qualifier is stripped only when ALL of: + * its text does not appear in ``other_title`` (if it does, the direct + comparison already handles it); + * it contains no version-marker token ('(Live)', '(Versión 1988)', + '(Dueto 2007)' keep blocking — they are different recordings); + * it introduces no digit token absent from ``other_title`` ('(Pt. 2)', + '(2007)' are different releases, never subtitles). + + Inputs should be normalized the same way the caller compares them + (lowercased / unidecode'd), like strip_redundant_context_qualifiers. + """ + if not title: + return title + + other = (other_title or "").casefold() + other_tokens = set(_TOKEN_RE.findall(other)) + + def _drop(match: re.Match) -> str: + inner = match.group(1).strip().casefold() + if not inner: + return " " + # Restated in the counterpart title — leave for the direct comparison. + if re.search(r"\b" + re.escape(inner) + r"\b", other): + return match.group(0) + tokens = _TOKEN_RE.findall(inner) + if any(t in _VERSION_MARKER_TOKENS for t in tokens): + return match.group(0) + if any(any(c.isdigit() for c in t) and t not in other_tokens for t in tokens): + return match.group(0) + return " " + + out = _QUALIFIER_RE.sub(_drop, title) + return re.sub(r"\s+", " ", out).strip() + + def numeric_tokens_differ(title_a: str, title_b: str) -> bool: """True when the digit-bearing tokens of two titles differ — 'Vol.4' vs 'Vol.4.5', 'Album' vs 'Album 2'. A numeric difference is a different @@ -133,5 +204,6 @@ def numeric_tokens_differ(title_a: str, title_b: str) -> bool: __all__ = [ "titles_plausibly_same", "strip_redundant_context_qualifiers", + "strip_subtitle_qualifiers", "numeric_tokens_differ", ] diff --git a/database/music_database.py b/database/music_database.py index 202b2d4c..0fc68c4c 100644 --- a/database/music_database.py +++ b/database/music_database.py @@ -7668,6 +7668,24 @@ class MusicDatabase: ctx_sim *= ctx_ratio # 'Believe' vs 'Believe In Me' still penalised best_title_similarity = max(best_title_similarity, ctx_sim) + # #825: a bracketed qualifier that is a SUBTITLE — not a version + # marker and not numeric — is the same song. 'Llamando a la tierra + # (Serenade From the Stars)' vs the library's bare 'Llamando a la + # tierra': the subtitle restates nothing (so #808 keeps it) and the + # length penalty crushed the pair to ~0.14 — sync re-added it to + # the wishlist forever and cleanup (same matcher) never removed it. + # Version qualifiers ('(Live)', '(Versión 1988)', '(Dueto 2007)') + # are kept by the helper, so their mismatch penalty still stands. + from core.text.title_match import strip_subtitle_qualifiers + sub_search = strip_subtitle_qualifiers(search_title_norm, db_title_norm) + sub_db = strip_subtitle_qualifiers(db_title_norm, search_title_norm) + if (sub_search, sub_db) != (search_title_norm, db_title_norm) and sub_search and sub_db: + sub_sim = self._string_similarity(sub_search, sub_db) + sub_ratio = min(len(sub_search), len(sub_db)) / max(len(sub_search), len(sub_db)) + if sub_ratio < 0.7: + sub_sim *= sub_ratio # stripped forms still length-guarded + best_title_similarity = max(best_title_similarity, sub_sim) + # Word-level guard: SequenceMatcher's char ratio over-credits # different songs that share a long substring or only a stopword # ("Dani California" vs "Californication" = 0.67; "Under The Bridge" diff --git a/services/sync_service.py b/services/sync_service.py index 803f2e8e..b48aa051 100644 --- a/services/sync_service.py +++ b/services/sync_service.py @@ -665,6 +665,10 @@ class PlaylistSyncService: _try_title, _try_artist, confidence_threshold=0.7, server_source=active_server, candidate_tracks=artist_candidates, + # #825: album context enables the album-aware fallback + # (multi-artist albums filed under another artist) — + # the cleanup path already passes it; sync didn't. + album=getattr(spotify_track, 'album', None) or None, ) if _cand_conf > confidence: db_track, confidence = _cand_track, _cand_conf diff --git a/tests/test_subtitle_qualifier_match.py b/tests/test_subtitle_qualifier_match.py new file mode 100644 index 00000000..a96150dc --- /dev/null +++ b/tests/test_subtitle_qualifier_match.py @@ -0,0 +1,158 @@ +"""#825: bracketed SUBTITLES must not block library-presence matching. + +carlosjfcasero's case (round 2): the mirrored-playlist sync auto-added +'Llamando a la tierra (Serenade From the Stars)' by M-Clan to the wishlist on +every run, even though his library has the song (stored bare). The subtitle is +the song's official parenthetical — it restates no album context, so the #808 +strip kept it, and the length-ratio penalty crushed the pair to ~0.14. The +sync matcher reported it missing forever AND the wishlist cleanup (the same +matcher) could never remove it. + +Fix: a bracketed qualifier with no version-marker token and no new numeric +token is a subtitle — compare with it stripped. Version qualifiers ('(Live)', +'(Versión 1988)', '(Dueto 2007)') still block, both EN and ES. +""" + +from __future__ import annotations + +import pytest + +from core.text.title_match import strip_subtitle_qualifiers +from database.music_database import MusicDatabase + + +# ── the pure helper ────────────────────────────────────────────────────────── + +def test_subtitle_is_stripped(): + out = strip_subtitle_qualifiers( + 'llamando a la tierra (serenade from the stars)', 'llamando a la tierra') + assert out == 'llamando a la tierra' + + +def test_version_markers_kept_english(): + for q in ('live', 'remix', 'acoustic', 'instrumental', 'demo', 'radio edit'): + assert strip_subtitle_qualifiers(f'song ({q})', 'song') == f'song ({q})' + + +def test_version_markers_kept_spanish(): + assert strip_subtitle_qualifiers('song (version 1988)', 'song') == 'song (version 1988)' + assert strip_subtitle_qualifiers('song (dueto 2007)', 'song') == 'song (dueto 2007)' + assert strip_subtitle_qualifiers('song (en directo en el liceu / 2008)', 'song') \ + == 'song (en directo en el liceu / 2008)' + + +def test_new_numeric_token_kept(): + # '(Pt. 2)' / '(2007)' are different releases, never subtitles. + assert strip_subtitle_qualifiers('song (pt. 2)', 'song') == 'song (pt. 2)' + assert strip_subtitle_qualifiers('song (2007)', 'song') == 'song (2007)' + + +def test_distinct_track_qualifiers_kept(): + # '(Interlude)' etc. are SEPARATE short tracks sharing the base name — + # treating them as subtitles would wrongly count the full song as owned. + for q in ('interlude', 'intro', 'outro', 'skit', 'freestyle'): + assert strip_subtitle_qualifiers(f'song ({q})', 'song') == f'song ({q})' + + +def test_roman_numeral_parts_kept(): + # No digits, so the numeric guard alone can't catch these. + assert strip_subtitle_qualifiers('song (pt. ii)', 'song') == 'song (pt. ii)' + assert strip_subtitle_qualifiers('song (part two)', 'song') == 'song (part two)' + assert strip_subtitle_qualifiers('song (vol. iii)', 'song') == 'song (vol. iii)' + + +def test_numeric_token_shared_with_other_title_is_fine(): + # The digit appears on the other side too — not a new release marker. + assert strip_subtitle_qualifiers('song 2007 (the ballad)', 'song 2007') == 'song 2007' + + +def test_qualifier_restated_in_other_title_left_for_direct_compare(): + full = 'song (the ballad)' + assert strip_subtitle_qualifiers(full, 'song (the ballad)') == full + + +def test_empty_and_plain_untouched(): + assert strip_subtitle_qualifiers('', 'x') == '' + assert strip_subtitle_qualifiers('plain title', 'other') == 'plain title' + + +# ── end to end through check_track_exists (sync + cleanup contract) ────────── + +@pytest.fixture() +def lib_db(tmp_path): + db = MusicDatabase(str(tmp_path / 'm.db')) + conn = db._get_connection() + c = conn.cursor() + c.execute("INSERT INTO artists (id, name, server_source) VALUES ('a1', 'M-Clan', 'jellyfin')") + c.execute("""INSERT INTO albums (id, title, artist_id, server_source) + VALUES ('al1', 'Usar y tirar', 'a1', 'jellyfin')""") + c.execute("""INSERT INTO tracks (id, album_id, artist_id, title, file_path, server_source) + VALUES ('t1', 'al1', 'a1', 'Llamando a la tierra', '/m/llamando.mp3', 'jellyfin')""") + c.execute("""INSERT INTO tracks (id, album_id, artist_id, title, file_path, server_source) + VALUES ('t2', 'al1', 'a1', 'Carolina', '/m/carolina.mp3', 'jellyfin')""") + conn.commit() + conn.close() + return db + + +def test_825_subtitled_search_matches_bare_library_track(lib_db): + """The reported case verbatim: playlist title carries the subtitle, the + library stores the bare title — must match (sync) and clean (cleanup).""" + match, conf = lib_db.check_track_exists( + 'Llamando a la tierra (Serenade From the Stars)', 'M-Clan', + confidence_threshold=0.7, server_source='jellyfin', + ) + assert match is not None and conf >= 0.7 + assert match.title == 'Llamando a la tierra' + + +def test_825_reverse_direction_matches(lib_db): + """Library could equally store the FULL title while the playlist has the + bare one — both directions must match.""" + conn = lib_db._get_connection() + c = conn.cursor() + c.execute("""INSERT INTO tracks (id, album_id, artist_id, title, file_path, server_source) + VALUES ('t3', 'al1', 'a1', 'Maggie (despierta)', '/m/maggie.mp3', 'jellyfin')""") + conn.commit() + conn.close() + match, conf = lib_db.check_track_exists( + 'Maggie', 'M-Clan', confidence_threshold=0.7, server_source='jellyfin') + assert match is not None and conf >= 0.7 + + +def test_live_version_still_blocked(lib_db): + match, conf = lib_db.check_track_exists( + 'Llamando a la tierra (Live)', 'M-Clan', + confidence_threshold=0.7, server_source='jellyfin', + ) + assert conf < 0.7 + + +def test_spanish_version_qualifiers_still_blocked(lib_db): + for title in ('Carolina (Versión 1988)', 'Carolina (Dueto 2007)', + 'Carolina (En Directo / 2005)'): + match, conf = lib_db.check_track_exists( + title, 'M-Clan', confidence_threshold=0.7, server_source='jellyfin') + assert conf < 0.7, title + + +def test_different_song_prefix_still_blocked(lib_db): + """Non-bracketed extensions are untouched — the length penalty stands.""" + match, conf = lib_db.check_track_exists( + 'Carolina en mi mente y otras cosas', 'M-Clan', + confidence_threshold=0.7, server_source='jellyfin', + ) + assert conf < 0.7 + + +def test_batched_candidate_path_also_fixed(lib_db): + """The sync matcher uses the candidate_tracks (batched) path — the fix must + apply there too, not just the SQL-variation path.""" + candidates = lib_db.search_tracks(artist='M-Clan', limit=50, server_source='jellyfin') + assert candidates + match, conf = lib_db.check_track_exists( + 'Llamando a la tierra (Serenade From the Stars)', 'M-Clan', + confidence_threshold=0.7, server_source='jellyfin', + candidate_tracks=candidates, + ) + assert match is not None and conf >= 0.7 From e32e2e5e14e327af3b52b32a26f624bc2733fc2a Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Tue, 9 Jun 2026 16:31:05 -0700 Subject: [PATCH 05/67] =?UTF-8?q?Sync:=20append=20mode=20actually=20dedupe?= =?UTF-8?q?s=20=E2=80=94=20stop=20re-adding=20the=20whole=20playlist=20(#8?= =?UTF-8?q?23)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit carlosjfcasero round 2: after 6fa956d6 append stopped recreating the playlist, but every sync re-appended ALL matched tracks — every track N times. His log shows it plainly: "added 22 new tracks to 'Disney' (skipped 0 already present)" on a playlist that already had those 22. Root cause: the dedupe read `{t.id for t in get_playlist_tracks(...)}` — but JellyfinTrack only defines `ratingKey`, never `id`, so the existing-ids set was ALWAYS empty and everything looked new. NavidromeTrack is also ratingKey-only, so the Navidrome append had the identical bug. Plex (plexapi ratingKey) was fine. The existing tests were green because they mocked existing tracks as SimpleNamespace(id=...) — encoding the same wrong assumption as the code. Fix: - New pure planner plan_playlist_append(current, desired) in core/sync/playlist_edit.py (next to the reconcile planner): order-preserving, drops already-present ids, dedupes within desired, stringifies (Emby numeric vs string safe). - Jellyfin/Emby: existing ids fetched from the canonical /Playlists/{id}/Items endpoint (same as reconcile — works for Jellyfin GUIDs and Emby numeric ids), ratingKey fallback if that request fails. - Navidrome: dedupe on ratingKey (the attribute that actually exists). Tests: planner (skip-present incl. the reporter's unchanged-playlist case, desired-order, dupes-within-desired, int/str ids, empties) + the append-mode suite rewritten to pin the REAL shapes (raw Items dicts for Jellyfin, ratingKey objects for Navidrome) + a new fallback-path test. 524 playlist/sync/jellyfin/navidrome tests pass. --- core/jellyfin_client.py | 34 +++++++++++++---- core/navidrome_client.py | 19 +++++++--- core/sync/playlist_edit.py | 29 ++++++++++++++ tests/test_playlist_edit.py | 35 +++++++++++++++++ tests/test_server_playlist_append_mode.py | 46 ++++++++++++++++++----- 5 files changed, 140 insertions(+), 23 deletions(-) diff --git a/core/jellyfin_client.py b/core/jellyfin_client.py index 8ee4eb74..f24020d5 100644 --- a/core/jellyfin_client.py +++ b/core/jellyfin_client.py @@ -1566,20 +1566,40 @@ class JellyfinClient(MediaServerClient): return self.create_playlist(playlist_name, tracks) playlist_id = existing_playlist.id - existing_tracks = self.get_playlist_tracks(playlist_id) - existing_ids = { - str(t.id) for t in existing_tracks if hasattr(t, 'id') and t.id - } + # #823 round 2: the old dedupe read `t.id` off get_playlist_tracks() + # results — but JellyfinTrack only defines `ratingKey`, so the + # existing-ids set was ALWAYS empty and every sync re-appended the + # whole matched list ("added 22 ... skipped 0" on a playlist that + # already had them, every track N times). Fetch the playlist's items + # from the canonical /Playlists/{id}/Items endpoint (the same one + # reconcile uses — works on Jellyfin GUIDs and Emby numeric ids) and + # dedupe on the raw item Id; fall back to ratingKey if that fails. + existing_ids = set() + items_resp = self._make_request( + f'/Playlists/{playlist_id}/Items', {'UserId': self.user_id}) + if items_resp: + for item in items_resp.get('Items', []): + iid = str(item.get('Id') or '') + if iid: + existing_ids.add(iid) + else: + existing_ids = { + str(getattr(t, 'ratingKey', '') or '') + for t in self.get_playlist_tracks(playlist_id) + } - {''} - new_track_ids = [] + desired_ids = [] for t in tracks: tid = None if hasattr(t, 'id'): tid = str(t.id) if t.id else None elif isinstance(t, dict): tid = str(t.get('Id') or t.get('id') or '') - if tid and tid not in existing_ids and self._is_valid_guid(tid): - new_track_ids.append(tid) + if tid and self._is_valid_guid(tid): + desired_ids.append(tid) + + from core.sync.playlist_edit import plan_playlist_append + new_track_ids = plan_playlist_append(existing_ids, desired_ids) if not new_track_ids: logger.info( diff --git a/core/navidrome_client.py b/core/navidrome_client.py index 085a4257..5a0677ee 100644 --- a/core/navidrome_client.py +++ b/core/navidrome_client.py @@ -1102,12 +1102,16 @@ class NavidromeClient(MediaServerClient): return self.create_playlist(playlist_name, tracks) primary = existing_playlists[0] - existing_tracks = self.get_playlist_tracks(primary.id) + # #823 round 2: the old dedupe read `t.id` — but NavidromeTrack only + # defines `ratingKey`, so the existing-ids set was ALWAYS empty and + # every sync re-appended the whole matched list (every track N + # times). Same bug as the Jellyfin append; dedupe on ratingKey. existing_ids = { - str(t.id) for t in existing_tracks if hasattr(t, 'id') and t.id - } + str(getattr(t, 'ratingKey', '') or '') + for t in self.get_playlist_tracks(primary.id) + } - {''} - new_track_ids = [] + desired_ids = [] for t in tracks: tid = None if hasattr(t, 'ratingKey'): @@ -1116,8 +1120,11 @@ class NavidromeClient(MediaServerClient): tid = str(t.id) if t.id else None elif isinstance(t, dict): tid = str(t.get('id') or '') - if tid and tid not in existing_ids: - new_track_ids.append(tid) + if tid: + desired_ids.append(tid) + + from core.sync.playlist_edit import plan_playlist_append + new_track_ids = plan_playlist_append(existing_ids, desired_ids) if not new_track_ids: logger.info( diff --git a/core/sync/playlist_edit.py b/core/sync/playlist_edit.py index 80325238..a3423302 100644 --- a/core/sync/playlist_edit.py +++ b/core/sync/playlist_edit.py @@ -109,6 +109,34 @@ def plan_playlist_reconcile( return {"add": add, "remove": remove} +def plan_playlist_append( + current_ids: List[str], + desired_ids: List[str], +) -> List[str]: + """Plan an append: which desired ids are NOT already in the playlist. + + Used by ``sync_mode='append'`` (#823 round 2): the Jellyfin/Emby and + Navidrome appends deduped with ``{t.id for t in existing}`` — but their + track wrappers only define ``ratingKey``, never ``id``, so the existing-ids + set was ALWAYS empty and every sync re-appended the full matched list + (every track N times, "skipped 0 already present"). Pure planner so the + dedupe logic is testable; the caller fetches current ids however its + server API works and applies the returned adds. + + Order-preserving and duplicate-safe: desired order is kept, ids already + present are dropped, and duplicates WITHIN desired are emitted once. + """ + current_set = {str(t) for t in current_ids} + out: List[str] = [] + seen = set() + for d in desired_ids: + tid = str(d) + if tid and tid not in current_set and tid not in seen: + seen.add(tid) + out.append(tid) + return out + + VALID_SYNC_MODES = ("replace", "append", "reconcile") @@ -129,6 +157,7 @@ __all__ = [ "plan_playlist_add", "remove_one_occurrence", "plan_playlist_reconcile", + "plan_playlist_append", "normalize_sync_mode", "VALID_SYNC_MODES", ] diff --git a/tests/test_playlist_edit.py b/tests/test_playlist_edit.py index 66caae40..7c402992 100644 --- a/tests/test_playlist_edit.py +++ b/tests/test_playlist_edit.py @@ -161,3 +161,38 @@ def test_normalize_falls_back_for_unknown(): def test_normalize_all_real_modes_pass_through(): for m in ('replace', 'append', 'reconcile'): assert normalize_sync_mode(m, 'replace') == m + + +# ── plan_playlist_append (#823 round 2) ────────────────────────────────────── +# The Jellyfin/Emby + Navidrome appends deduped on `t.id`, but their track +# wrappers only define `ratingKey` — the existing-ids set was always empty, so +# every sync re-appended the full matched list (every track N times, +# "skipped 0 already present"). The planner is the now-testable dedupe. + +from core.sync.playlist_edit import plan_playlist_append # noqa: E402 + + +def test_append_skips_already_present(): + # carlosjfcasero's case: second sync of an unchanged playlist adds NOTHING. + assert plan_playlist_append(['a', 'b', 'c'], ['a', 'b', 'c']) == [] + + +def test_append_adds_only_new_in_desired_order(): + assert plan_playlist_append(['a', 'c'], ['a', 'b', 'c', 'd']) == ['b', 'd'] + + +def test_append_to_empty_playlist_adds_all(): + assert plan_playlist_append([], ['a', 'b']) == ['a', 'b'] + + +def test_append_dedupes_within_desired(): + assert plan_playlist_append(['x'], ['a', 'a', 'x', 'b', 'a']) == ['a', 'b'] + + +def test_append_stringifies_ids(): + # Emby numeric ids may arrive as ints on one side and strings on the other. + assert plan_playlist_append([16607838], ['16607838', '999']) == ['999'] + + +def test_append_ignores_empty_ids(): + assert plan_playlist_append(['a'], ['', 'b']) == ['b'] diff --git a/tests/test_server_playlist_append_mode.py b/tests/test_server_playlist_append_mode.py index 8624fb1a..97af0c83 100644 --- a/tests/test_server_playlist_append_mode.py +++ b/tests/test_server_playlist_append_mode.py @@ -173,13 +173,19 @@ class TestJellyfinAppendToPlaylist: mock_create.assert_called_once_with("Test", new_tracks) def test_filters_out_already_present_tracks(self): - """Reporter's exact case for Jellyfin — only new GUIDs go in.""" + """Reporter's exact case for Jellyfin — only new GUIDs go in. + + #823 round 2: existing ids now come from the canonical + /Playlists/{id}/Items request ({'Items':[{'Id': ...}]}) — the old + get_playlist_tracks path returned JellyfinTrack objects that only + define `ratingKey`, so the previous `t.id` dedupe was always empty and + every sync re-appended everything. These mocks pin the REAL shape.""" client = _make_jellyfin_client() existing_playlist = SimpleNamespace(id='pl-1') - existing_tracks = [ - SimpleNamespace(id='aaaaaaaa-bbbb-cccc-dddd-000000000001'), - SimpleNamespace(id='aaaaaaaa-bbbb-cccc-dddd-000000000002'), - ] + items_resp = {'Items': [ + {'Id': 'aaaaaaaa-bbbb-cccc-dddd-000000000001'}, + {'Id': 'aaaaaaaa-bbbb-cccc-dddd-000000000002'}, + ]} incoming = [ SimpleNamespace(id='aaaaaaaa-bbbb-cccc-dddd-000000000001'), # present SimpleNamespace(id='aaaaaaaa-bbbb-cccc-dddd-000000000003'), # NEW @@ -194,7 +200,7 @@ class TestJellyfinAppendToPlaylist: with patch.object(client, 'ensure_connection', return_value=True), \ patch.object(client, 'get_playlist_by_name', return_value=existing_playlist), \ - patch.object(client, 'get_playlist_tracks', return_value=existing_tracks), \ + patch.object(client, '_make_request', return_value=items_resp), \ patch.object(client, '_is_valid_guid', return_value=True), \ patch('core.jellyfin_client.requests.post', side_effect=fake_post): result = client.append_to_playlist("Test", incoming) @@ -206,9 +212,26 @@ class TestJellyfinAppendToPlaylist: def test_short_circuits_when_no_new_tracks(self): client = _make_jellyfin_client() existing_playlist = SimpleNamespace(id='pl-1') - existing_tracks = [SimpleNamespace(id='guid-1')] + items_resp = {'Items': [{'Id': 'guid-1'}]} with patch.object(client, 'ensure_connection', return_value=True), \ patch.object(client, 'get_playlist_by_name', return_value=existing_playlist), \ + patch.object(client, '_make_request', return_value=items_resp), \ + patch.object(client, '_is_valid_guid', return_value=True), \ + patch('core.jellyfin_client.requests.post') as mock_post: + result = client.append_to_playlist("Test", [SimpleNamespace(id='guid-1')]) + assert result is True + mock_post.assert_not_called() + + def test_falls_back_to_ratingkey_tracks_when_items_request_fails(self): + """When /Playlists/{id}/Items fails, dedupe falls back to + get_playlist_tracks — reading ratingKey (the attr that EXISTS on + JellyfinTrack), not the never-present `.id` of the original bug.""" + client = _make_jellyfin_client() + existing_playlist = SimpleNamespace(id='pl-1') + existing_tracks = [SimpleNamespace(ratingKey='guid-1')] + with patch.object(client, 'ensure_connection', return_value=True), \ + patch.object(client, 'get_playlist_by_name', return_value=existing_playlist), \ + patch.object(client, '_make_request', return_value=None), \ patch.object(client, 'get_playlist_tracks', return_value=existing_tracks), \ patch.object(client, '_is_valid_guid', return_value=True), \ patch('core.jellyfin_client.requests.post') as mock_post: @@ -221,7 +244,7 @@ class TestJellyfinAppendToPlaylist: existing_playlist = SimpleNamespace(id='pl-1') with patch.object(client, 'ensure_connection', return_value=True), \ patch.object(client, 'get_playlist_by_name', return_value=existing_playlist), \ - patch.object(client, 'get_playlist_tracks', return_value=[]), \ + patch.object(client, '_make_request', return_value={'Items': []}), \ patch.object(client, '_is_valid_guid', return_value=True), \ patch('core.jellyfin_client.requests.post', return_value=SimpleNamespace(status_code=500, text='server error')): @@ -256,9 +279,12 @@ class TestNavidromeAppendToPlaylist: mock_create.assert_called_once() def test_filters_out_already_present_tracks_and_calls_subsonic(self): + # #823 round 2: existing tracks are NavidromeTrack objects, which only + # define `ratingKey` — the old `t.id` dedupe was always empty. These + # mocks pin the REAL attribute so the dedupe is actually exercised. client = _make_navidrome_client() existing_playlists = [SimpleNamespace(id='pl-1', title='Test')] - existing_tracks = [SimpleNamespace(id='100'), SimpleNamespace(id='101')] + existing_tracks = [SimpleNamespace(ratingKey='100'), SimpleNamespace(ratingKey='101')] incoming = [ SimpleNamespace(id='100'), # present SimpleNamespace(id='102'), # NEW @@ -287,7 +313,7 @@ class TestNavidromeAppendToPlaylist: def test_short_circuits_when_no_new_tracks(self): client = _make_navidrome_client() existing_playlists = [SimpleNamespace(id='pl-1', title='Test')] - existing_tracks = [SimpleNamespace(id='100')] + existing_tracks = [SimpleNamespace(ratingKey='100')] with patch.object(client, 'ensure_connection', return_value=True), \ patch.object(client, 'get_playlists_by_name', return_value=existing_playlists), \ patch.object(client, 'get_playlist_tracks', return_value=existing_tracks), \ From bcd69c8baa8be29a0eb0558007d09db54aedc460 Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Tue, 9 Jun 2026 17:10:03 -0700 Subject: [PATCH 06/67] =?UTF-8?q?Multi-artist=20tags:=20Search=20=E2=86=92?= =?UTF-8?q?=20Download=20Now=20finally=20knows=20its=20metadata=20source?= =?UTF-8?q?=20(Netti93)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Third round of the multi-artist report. The earlier fixes (Deezer contributors upgrade, _artists_list, feat_in_title/artist_separator) were all in place and correct — but gated on source == 'deezer', and on the real Search → Download Now path NOTHING carried the source: core/search/sources.py serialized tracks with no source field, search.js's enrichedTrack didn't add one, so get_import_source() resolved '' and the whole Deezer-specific block silently skipped. Files were tagged with only the primary artist until a Retag (which rebuilds context with the source set — exactly why retagging always fixed it). The earlier tests passed because they set context['source'] directly — the one field the real flow never had (same mock-drift as the #823 append tests). Reproduced with Netti93's exact track (deezer 3966840171) through the real extract_source_metadata: before — source '', artists ['August Burns Red']; after — source 'deezer', contributors fetched, artists ['August Burns Red', 'Polaris'], title 'Sonic Salvation (feat. Polaris)' per feat_in_title. Fix, three layers: - core/search/sources.py: serialized tracks/albums/artists carry "source" (the canonical name the orchestrator already passes; '' when unnamed). - core/imports/context.py get_import_source: also reads '_source' from the nested dicts (track_info/original_search/album/artist) — additionally fixes the discography/wishlist flows, which always passed '_source' that nothing read. - search.js: enrichedTrack + the album-download path carry source through to the download task. Tests: real-payload staging-shaped contexts (source in track_info, '_source' shape, and the pre-fix sourceless shape staying safe — mocked Deezer client), serializer source-field tests, resolver fallback tests; exact-shape serializer tests updated for the new key. 1977 import/metadata/search tests pass (the only 2 failures are the known soundcloud ones). --- core/imports/context.py | 16 ++-- core/search/sources.py | 17 ++++ tests/imports/test_import_context.py | 17 ++++ .../test_multi_artist_tag_settings.py | 90 +++++++++++++++++++ tests/search/test_search_sources.py | 45 ++++++++++ webui/static/search.js | 14 ++- 6 files changed, 193 insertions(+), 6 deletions(-) diff --git a/core/imports/context.py b/core/imports/context.py index 73459949..f718f014 100644 --- a/core/imports/context.py +++ b/core/imports/context.py @@ -129,30 +129,36 @@ def get_import_search_result(context: Optional[Dict[str, Any]]) -> Dict[str, Any def get_import_source(context: Optional[Dict[str, Any]]) -> str: + # Several track payloads carry the metadata source under "_source" rather + # than "source" (the discography/wishlist dicts, frontend search results). + # Only the context-level "_source" was honored (normalize_import_context); + # the nested dicts were checked for "source" alone, so a Deezer-sourced + # Download Now resolved to '' and source-specific metadata logic (the + # Deezer contributors upgrade for multi-artist tags) never ran (Netti93). if not isinstance(context, dict): return "" - source = context.get("source") + source = context.get("source") or context.get("_source") if source: return str(source) track_info = get_import_track_info(context) - source = _first_value(track_info, "source", default="") + source = _first_value(track_info, "source", "_source", default="") if source: return str(source) original_search = get_import_original_search(context) - source = _first_value(original_search, "source", default="") + source = _first_value(original_search, "source", "_source", default="") if source: return str(source) album = get_import_context_album(context) - source = _first_value(album, "source", default="") + source = _first_value(album, "source", "_source", default="") if source: return str(source) artist = get_import_context_artist(context) - source = _first_value(artist, "source", default="") + source = _first_value(artist, "source", "_source", default="") return str(source) if source else "" diff --git a/core/search/sources.py b/core/search/sources.py index 006bf8f0..e5d66805 100644 --- a/core/search/sources.py +++ b/core/search/sources.py @@ -35,6 +35,7 @@ def search_kind(client, query: str, kind: str, source_name: Optional[str] = None artists.append({ "id": artist.id, "name": artist.name, + "source": source_name or "", "image_url": artist.image_url, "external_urls": artist.external_urls or {}, }) @@ -52,6 +53,7 @@ def search_kind(client, query: str, kind: str, source_name: Optional[str] = None "id": album.id, "name": album.name, "artist": artist_name, + "source": source_name or "", "image_url": album.image_url, "release_date": album.release_date, "total_tracks": album.total_tracks, @@ -78,6 +80,21 @@ def search_kind(client, query: str, kind: str, source_name: Optional[str] = None "id": track.id, "name": track.name, "artist": artist_name, + # The REAL artist list, not the joined display string above. + # Spotify/Tidal/iTunes searches return collabs as a list; + # collapsing them to one "A, B" string made the import + # pipeline tag downloads with a single combined artist + # (resolve_track_artists saw one value). The frontend keeps + # using "artist" for display. + "artists": list(track.artists or []), + # Which metadata source this result came from. Travels with + # the payload through Download Now -> download task -> + # import context, where extract_source_metadata needs it to + # run source-specific logic (the Deezer contributors + # upgrade for multi-artist tags — Netti93's report: without + # it get_import_source() resolved '' and collab tracks + # were tagged with only the primary artist until a Retag). + "source": source_name or "", "album": track.album, "duration_ms": track.duration_ms, "image_url": track.image_url, diff --git a/tests/imports/test_import_context.py b/tests/imports/test_import_context.py index e71584b7..5ce80835 100644 --- a/tests/imports/test_import_context.py +++ b/tests/imports/test_import_context.py @@ -338,3 +338,20 @@ def test_detect_album_info_web_forces_album_when_track_and_artist_differ(): assert album_info["album_name"] == "Album One" assert album_info["track_number"] == 4 assert album_info["disc_number"] == 2 + + +def test_get_import_source_reads_underscore_source_from_nested_dicts(): + """Netti93 multi-artist fix: many track payloads carry '_source' (the + discography/wishlist dicts) or 'source' only inside track_info (search + results). get_import_source must resolve all of them — previously only + the context-level keys worked, so direct downloads resolved '' and + source-specific metadata logic never ran.""" + from core.imports.context import get_import_source + + assert get_import_source({"track_info": {"source": "deezer"}}) == "deezer" + assert get_import_source({"track_info": {"_source": "deezer"}}) == "deezer" + assert get_import_source({"original_search_result": {"_source": "itunes"}}) == "itunes" + assert get_import_source({"_source": "tidal"}) == "tidal" + # context-level 'source' still wins over nested + assert get_import_source({"source": "spotify", "track_info": {"_source": "deezer"}}) == "spotify" + assert get_import_source({}) == "" diff --git a/tests/metadata/test_multi_artist_tag_settings.py b/tests/metadata/test_multi_artist_tag_settings.py index b8de390f..dc835bed 100644 --- a/tests/metadata/test_multi_artist_tag_settings.py +++ b/tests/metadata/test_multi_artist_tag_settings.py @@ -561,3 +561,93 @@ class TestDeezerDirectDownloadFlow: assert meta["_artists_list"] == ["FAYAN", "Dalton"] assert meta["artist"] == "FAYAN;Dalton" assert meta["title"] == "VERLIEBT IN MICH" + + +class TestSourceResolutionFromRealDownloadPayload: + """Netti93 round 3: the prior tests set context['source'] — a field the + REAL Search → Download Now flow never had. The serialized search results + carried no source at all, so get_import_source() resolved '' and the + Deezer contributors upgrade never ran on direct downloads (single-artist + tags until a Retag). These pin the actual payload shapes end to end.""" + + def _staging_context(self, track_info): + # Shape built by core/downloads/staging.py:457 for a stream download. + return { + "track_info": track_info, + "spotify_artist": {"name": "August Burns Red", "id": None}, + "spotify_album": {"name": "Death Below"}, + "original_search_result": { + "username": "tidal", "filename": "/x.flac", + "title": "Sonic Salvation", "artist": "August Burns Red", + "spotify_clean_title": "Sonic Salvation", + "spotify_clean_album": "Death Below", + "spotify_clean_artist": "August Burns Red", + "track_number": 1, "disc_number": 1, + }, + "is_album_download": False, + "staging_source": True, + } + + def _fake_deezer(self): + return SimpleNamespace(get_track_details=MagicMock(return_value={ + "id": "3966840171", "name": "Sonic Salvation", + "artists": ["August Burns Red", "Polaris"], + })) + + def test_source_in_track_info_triggers_upgrade(self): + """The fixed flow: serialized search result carries source='deezer' + inside track_info (no context-level source) → upgrade fires.""" + from core.metadata import source as src_module + + track_info = { + "id": "3966840171", "name": "Sonic Salvation", + "source": "deezer", + "artists": ["August Burns Red"], + "album": {"name": "Death Below", "id": None}, + } + fake_deezer = self._fake_deezer() + with patch.object(src_module, "get_config_manager", + return_value=_make_cfg({"metadata_enhancement.tags.write_multi_artist": True})), \ + patch("core.metadata.get_deezer_client", return_value=fake_deezer): + meta = src_module.extract_source_metadata( + self._staging_context(track_info), {"name": "August Burns Red", "id": ""}, {}) + + assert meta["_artists_list"] == ["August Burns Red", "Polaris"] + fake_deezer.get_track_details.assert_called_once_with("3966840171") + + def test_underscore_source_shape_also_triggers_upgrade(self): + """Discography/wishlist payloads carry '_source' instead of 'source' — + get_import_source must honor that shape too.""" + from core.metadata import source as src_module + + track_info = { + "id": "3966840171", "name": "Sonic Salvation", + "_source": "deezer", + "artists": ["August Burns Red"], + } + fake_deezer = self._fake_deezer() + with patch.object(src_module, "get_config_manager", return_value=_make_cfg()), \ + patch("core.metadata.get_deezer_client", return_value=fake_deezer): + meta = src_module.extract_source_metadata( + self._staging_context(track_info), {"name": "August Burns Red", "id": ""}, {}) + + assert meta["_artists_list"] == ["August Burns Red", "Polaris"] + fake_deezer.get_track_details.assert_called_once_with("3966840171") + + def test_sourceless_payload_still_no_upgrade(self): + """A payload with no source anywhere (pre-fix shape) must not crash — + and must not call the Deezer API blindly.""" + from core.metadata import source as src_module + + track_info = { + "id": "3966840171", "name": "Sonic Salvation", + "artists": ["August Burns Red"], + } + fake_deezer = self._fake_deezer() + with patch.object(src_module, "get_config_manager", return_value=_make_cfg()), \ + patch("core.metadata.get_deezer_client", return_value=fake_deezer): + meta = src_module.extract_source_metadata( + self._staging_context(track_info), {"name": "August Burns Red", "id": ""}, {}) + + assert meta["_artists_list"] == ["August Burns Red"] + fake_deezer.get_track_details.assert_not_called() diff --git a/tests/search/test_search_sources.py b/tests/search/test_search_sources.py index f6949027..eb5fb7fd 100644 --- a/tests/search/test_search_sources.py +++ b/tests/search/test_search_sources.py @@ -84,6 +84,7 @@ def test_search_kind_artists_returns_normalized_dicts(): assert result == [{ 'id': 'id1', 'name': 'Pink Floyd', + 'source': 'spotify', 'image_url': 'thumb.jpg', 'external_urls': {'spotify': 'url'}, }] @@ -137,6 +138,8 @@ def test_search_kind_tracks_returns_full_shape(): 'id': 't1', 'name': 'Money', 'artist': 'Pink Floyd', + 'artists': ['Pink Floyd'], + 'source': '', 'album': 'DSOTM', 'duration_ms': 383000, 'image_url': 'm.jpg', @@ -201,3 +204,45 @@ def test_search_source_all_fail_returns_empty_lists(): client = _Client(fail={'artists', 'albums', 'tracks'}) result = sources.search_source('q', client, 'spotify') assert result == {'artists': [], 'albums': [], 'tracks': [], 'available': True} + + +# ── source field on serialized results (Netti93 multi-artist fix) ─────────── +# Search results must carry which metadata source they came from: the payload +# travels Download Now → download task → import context, where the Deezer +# contributors upgrade (multi-artist tags) is gated on source == 'deezer'. +# Without it get_import_source() resolved '' and collab tracks were tagged +# with only the primary artist until a Retag. + +def _full_client(): + return _Client( + artists=[_Artist('id1', 'A')], + albums=[_Album('a1', 'Album', artists=['A'])], + tracks=[_Track('t1', 'T', artists=['A'], album='Album')], + ) + + +def test_serialized_tracks_carry_source(): + out = sources.search_kind(_full_client(), "q", "tracks", source_name="deezer") + assert out and all(t["source"] == "deezer" for t in out) + + +def test_serialized_albums_and_artists_carry_source(): + albums = sources.search_kind(_full_client(), "q", "albums", source_name="deezer") + artists = sources.search_kind(_full_client(), "q", "artists", source_name="deezer") + assert albums and all(a["source"] == "deezer" for a in albums) + assert artists and all(a["source"] == "deezer" for a in artists) + + +def test_serialized_source_empty_when_unnamed(): + # hydrabase path calls without a source_name — emit '' not the class name. + out = sources.search_kind(_full_client(), "q", "tracks") + assert out and all(t["source"] == "" for t in out) + + +def test_serialized_tracks_carry_real_artists_list(): + """Collabs must survive as a LIST — the joined "A, B" display string made + downloads tag a single combined artist (Marcus's report).""" + client = _Client(tracks=[_Track('t1', 'Collab', artists=['Artist A', 'Artist B'], album='X')]) + out = sources.search_kind(client, 'q', 'tracks', source_name='spotify') + assert out[0]['artists'] == ['Artist A', 'Artist B'] + assert out[0]['artist'] == 'Artist A, Artist B' # display string unchanged diff --git a/webui/static/search.js b/webui/static/search.js index 54110af1..721df256 100644 --- a/webui/static/search.js +++ b/webui/static/search.js @@ -750,6 +750,9 @@ function initializeSearchModeToggle() { // Enrich each track with full album object (needed for wishlist functionality) const enrichedTracks = albumData.tracks.map(track => ({ ...track, + // Carry the metadata source for source-specific import logic + // (Deezer contributors upgrade for multi-artist tags). + source: track.source || album.source || null, album: { name: albumData.name, id: albumData.id, @@ -902,7 +905,16 @@ function initializeSearchModeToggle() { const enrichedTrack = { id: track.id, name: track.name, - artists: [track.artist], // Convert string to array for modal compatibility + // Carry the metadata source so the import pipeline can run + // source-specific logic (Deezer contributors upgrade for + // multi-artist tags) on the downloaded file. + source: track.source || null, + // Prefer the real artist list (Spotify/Tidal collabs) over the + // joined "A, B" display string, so downloads get proper + // multi-artist tags instead of one combined artist. + artists: (Array.isArray(track.artists) && track.artists.length) + ? track.artists + : [track.artist], album: { name: track.album, id: null, From e8cde40d222f10dc7d64d9aa60d155adb313a019 Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Tue, 9 Jun 2026 20:14:02 -0700 Subject: [PATCH 07/67] Watchlist: show WHICH tracks a scan found/added + group Download Origins (#831) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Tacobell444 (#707 follow-up): the scan summary said "New tracks: 19 • Added to wishlist: 10" with no way to see which tracks those were — you had to scan your wishlist and guess what was new. Scan ledger: the scanner now records a per-run scan_track_events list (track, artist, album, thumb, status added|skipped — skipped = found-new but declined by add_to_wishlist: already queued or blocklisted; capped at 500). The status endpoint already serializes scan_state, so the payload flows free. The completed (and cancelled) scan summary on the Watchlist page gets a "Show tracks" toggle expanding a styled list — Added section + Skipped section with badges, reusing the live-feed row styling. Download Origins grouping: the modal now groups entries by what triggered them (watchlist artist / playlist name) with collapsible headers + counts instead of a flat list with a per-row badge. Entries arrive newest-first so groups order themselves by their newest download. Same row markup, checkboxes/delete intact. Provenance: watchlist adds now stamp scan_run_id into wishlist source_info, so per-run grouping is queryable later (future "what did run X add" views). Tests: per-run ledger seam test (added + skipped statuses, album/artist fields, FIFO unchanged). 316 watchlist/wishlist tests pass; JS syntax-checked. --- core/watchlist_scanner.py | 37 +++++++-- tests/test_watchlist_scanner_scan.py | 54 ++++++++++++++ web_server.py | 9 ++- webui/static/api-monitor.js | 46 ++++++++++++ webui/static/origin-history.js | 35 ++++++++- webui/static/style.css | 108 +++++++++++++++++++++++++++ 6 files changed, 277 insertions(+), 12 deletions(-) diff --git a/core/watchlist_scanner.py b/core/watchlist_scanner.py index 239cfba0..c99841eb 100644 --- a/core/watchlist_scanner.py +++ b/core/watchlist_scanner.py @@ -1376,14 +1376,34 @@ class WatchlistScanner: if scan_state is not None: scan_state['tracks_found_this_scan'] += 1 - if self.add_track_to_wishlist(track, album_data, artist): + added = self.add_track_to_wishlist( + track, album_data, artist, + scan_run_id=(scan_state or {}).get('scan_run_id', ''), + ) + + track_artists = track.get('artists', []) + track_artist_name = track_artists[0].get('name', 'Unknown Artist') if track_artists else 'Unknown Artist' + + # #831: per-run ledger so the completed-scan + # summary can list WHICH tracks the counts mean. + # 'skipped' = found-new but add_to_wishlist + # declined (already queued in the wishlist, or + # the artist is blocklisted). Capped for sanity. + if scan_state is not None: + events = scan_state.setdefault('scan_track_events', []) + if len(events) < 500: + events.append({ + 'track_name': track_name, + 'artist_name': track_artist_name, + 'album_name': album_name, + 'album_image_url': album_image_url, + 'status': 'added' if added else 'skipped', + }) + + if added: artist_added_tracks += 1 if scan_state is not None: scan_state['tracks_added_this_scan'] += 1 - - track_artists = track.get('artists', []) - track_artist_name = track_artists[0].get('name', 'Unknown Artist') if track_artists else 'Unknown Artist' - if scan_state is not None: scan_state['recent_wishlist_additions'].insert(0, { 'track_name': track_name, 'artist_name': track_artist_name, @@ -2224,7 +2244,8 @@ class WatchlistScanner: logger.warning(f"Error checking if track exists: {track_name}: {e}") return True # Assume missing if we can't check - def add_track_to_wishlist(self, track, album, watchlist_artist: WatchlistArtist) -> bool: + def add_track_to_wishlist(self, track, album, watchlist_artist: WatchlistArtist, + scan_run_id: str = '') -> bool: """Add a missing track to the wishlist""" try: # Handle both dict and object track/album formats @@ -2306,7 +2327,9 @@ class WatchlistScanner: 'watchlist_artist_name': watchlist_artist.artist_name, 'watchlist_artist_id': watchlist_artist.spotify_artist_id, 'album_name': album_name, - 'scan_timestamp': datetime.now().isoformat() + 'scan_timestamp': datetime.now().isoformat(), + # #831: groups wishlist rows by the scan run that added them. + 'scan_run_id': scan_run_id or '', }, profile_id=getattr(watchlist_artist, 'profile_id', 1) ) diff --git a/tests/test_watchlist_scanner_scan.py b/tests/test_watchlist_scanner_scan.py index 6c08efa0..f02eab45 100644 --- a/tests/test_watchlist_scanner_scan.py +++ b/tests/test_watchlist_scanner_scan.py @@ -1177,3 +1177,57 @@ def test_match_to_spotify_uses_strict_lookup(): assert result is None assert spotify_client.search_calls == [("Artist One", 5, False)] + + +def test_scan_records_per_run_track_ledger(monkeypatch): + """#831: the scan keeps a full per-run ledger (scan_track_events) of found + tracks — 'added' vs 'skipped' (wishlist dup / blocklisted) — so the UI can + list WHICH tracks the 'New tracks / Added to wishlist' counts refer to.""" + monkeypatch.setattr(watchlist_scanner_module, "DELAY_BETWEEN_ARTISTS", 0) + monkeypatch.setattr(watchlist_scanner_module, "DELAY_BETWEEN_ALBUMS", 0) + + artist = _build_artist() + album = types.SimpleNamespace(id="album-1", name="Album One") + album_data = { + "name": "Album One", + "images": [{"url": "https://example.com/album.jpg"}], + "tracks": { + "items": [ + {"id": "t1", "name": "Added Track", "track_number": 1, + "disc_number": 1, "artists": [{"name": "Artist One"}]}, + {"id": "t2", "name": "Skipped Track", "track_number": 2, + "disc_number": 1, "artists": [{"name": "Artist One"}]}, + ] + }, + } + scanner = _build_scanner(album_data, [artist]) + scanner._database.has_fresh_similar_artists = lambda *args, **kwargs: False + + monkeypatch.setattr(scanner, "_backfill_missing_ids", lambda *args, **kwargs: None) + monkeypatch.setattr(scanner, "get_artist_image_url", lambda *_a, **_k: "") + monkeypatch.setattr(scanner, "get_artist_discography_for_watchlist", lambda *_a, **_k: [album]) + monkeypatch.setattr(scanner, "_get_lookback_period_setting", lambda: "30") + monkeypatch.setattr(scanner, "_get_rescan_cutoff", lambda: None) + monkeypatch.setattr(scanner, "_should_include_release", lambda *_a, **_k: True) + monkeypatch.setattr(scanner, "_should_include_track", lambda *_a, **_k: True) + monkeypatch.setattr(scanner, "is_track_missing_from_library", lambda *_a, **_k: True) + # First add succeeds, second is declined (already queued / blocklisted). + _add_results = iter([True, False]) + monkeypatch.setattr(scanner, "add_track_to_wishlist", + lambda *_a, **_k: next(_add_results)) + monkeypatch.setattr(scanner, "update_artist_scan_timestamp", lambda *_a, **_k: True) + monkeypatch.setattr(scanner, "update_similar_artists", lambda *_a, **_k: True) + monkeypatch.setattr(scanner, "_backfill_similar_artists_fallback_ids", lambda *_a, **_k: 0) + + scan_state = {"scan_run_id": "20260609-1"} + scanner.scan_watchlist_artists([artist], scan_state=scan_state) + + events = scan_state["scan_track_events"] + assert [(e["track_name"], e["status"]) for e in events] == [ + ("Added Track", "added"), + ("Skipped Track", "skipped"), + ] + assert events[0]["album_name"] == "Album One" + assert events[0]["artist_name"] == "Artist One" + # The 10-item live FIFO only carries the ADDED one, as before. + assert [a["track_name"] for a in scan_state["recent_wishlist_additions"]] == ["Added Track"] diff --git a/web_server.py b/web_server.py index bd396d6c..4f4aba3b 100644 --- a/web_server.py +++ b/web_server.py @@ -25952,9 +25952,14 @@ def start_watchlist_scan(): 'current_track_name': '', 'tracks_found_this_scan': 0, 'tracks_added_this_scan': 0, - 'recent_wishlist_additions': [] + 'recent_wishlist_additions': [], + # #831: full per-run ledger of found tracks (added vs + # skipped) so the completed-scan summary can list WHICH + # tracks the "New tracks / Added to wishlist" counts mean. + 'scan_track_events': [], + 'scan_run_id': datetime.now().strftime('%Y%m%d-%H%M%S'), }) - + scan_results = [] # Pause enrichment workers during scan to reduce API contention diff --git a/webui/static/api-monitor.js b/webui/static/api-monitor.js index 0d51d85b..c5b4d2a5 100644 --- a/webui/static/api-monitor.js +++ b/webui/static/api-monitor.js @@ -3189,6 +3189,50 @@ async function startWatchlistScan() { /** * Poll watchlist scan status */ +// #831 (Tacobell444): the scan summary said "New tracks: 19 • Added to +// wishlist: 10" with no way to see WHICH tracks. The scan now ships a per-run +// ledger (scan_track_events: track/artist/album/thumb + added|skipped) and +// this renders it as an expandable list under the completion summary. +function renderWatchlistScanTrackLedger(events) { + if (!Array.isArray(events) || events.length === 0) return ''; + const added = events.filter(e => e.status === 'added'); + const skipped = events.filter(e => e.status !== 'added'); + + const row = (e) => ` +
+ +
+
${escapeHtml(e.track_name || '')}
+
${escapeHtml(e.artist_name || '')}${e.album_name ? ' — ' + escapeHtml(e.album_name) : ''}
+
+ ${e.status === 'added' + ? 'added' + : 'skipped'} +
`; + + const section = (label, list) => list.length + ? `
${label} (${list.length})
${list.map(row).join('')}` + : ''; + + return ` + + `; +} + +function toggleWatchlistScanTracks(btn) { + const list = btn.parentElement.querySelector('.watchlist-scan-tracks'); + if (!list) return; + const open = list.style.display !== 'none'; + list.style.display = open ? 'none' : 'block'; + btn.innerHTML = (open ? 'Show tracks ' : 'Hide tracks ') + + `${open ? '▾' : '▴'}`; +} + function handleWatchlistScanData(data) { const button = document.getElementById('scan-watchlist-btn'); const liveActivity = document.getElementById('watchlist-live-activity'); @@ -3295,6 +3339,7 @@ function handleWatchlistScanData(data) { • Added to wishlist: ${addedTracks} + ${renderWatchlistScanTrackLedger(data.scan_track_events)} `; } @@ -3341,6 +3386,7 @@ function handleWatchlistScanData(data) { • Added to wishlist: ${addedTracks} + ${renderWatchlistScanTrackLedger(data.scan_track_events)} `; } diff --git a/webui/static/origin-history.js b/webui/static/origin-history.js index 090e2980..c525588d 100644 --- a/webui/static/origin-history.js +++ b/webui/static/origin-history.js @@ -99,7 +99,8 @@ function _renderOriginEntries() { return; } const ctxLabel = _originActiveTab === 'watchlist' ? 'Watchlist artist' : 'Playlist'; - body.innerHTML = _originEntries.map(e => { + + const entryRow = (e) => { const checked = _originSelected.has(e.id) ? 'checked' : ''; const thumb = e.thumb_url ? `${escapeHtml(e.title || 'Unknown')}
${escapeHtml(e.artist_name || '')}${e.album_name ? ' — ' + escapeHtml(e.album_name) : ''}
- ${escapeHtml(e.origin_context || '—')} ${e.quality ? `${escapeHtml(e.quality)}` : ''}
${escapeHtml(_originFormatTime(e.created_at))}
+
${entries.map(entryRow).join('')}
+ `).join(''); _updateOriginDeleteButton(); } +function toggleOriginGroup(btn) { + const bodyEl = btn.parentElement.querySelector('.origin-group-body'); + const caret = btn.querySelector('.origin-group-caret'); + if (!bodyEl) return; + const open = bodyEl.style.display !== 'none'; + bodyEl.style.display = open ? 'none' : ''; + if (caret) caret.textContent = open ? '▸' : '▾'; +} + function toggleOriginEntry(id, on) { if (on) _originSelected.add(id); else _originSelected.delete(id); _updateOriginDeleteButton(); diff --git a/webui/static/style.css b/webui/static/style.css index cc22d3b2..28942dda 100644 --- a/webui/static/style.css +++ b/webui/static/style.css @@ -19891,6 +19891,65 @@ body.helper-mode-active #dashboard-activity-feed:hover { margin-bottom: 10px; } +/* #831: expandable per-run track ledger under the scan summary */ +.watchlist-scan-tracks-toggle { + margin-top: 10px; + padding: 5px 14px; + background: rgba(255, 255, 255, 0.06); + border: 1px solid rgba(255, 255, 255, 0.12); + border-radius: 14px; + color: #ccc; + font-size: 12px; + cursor: pointer; + transition: background 0.15s ease; +} + +.watchlist-scan-tracks-toggle:hover { + background: rgba(255, 255, 255, 0.12); + color: #fff; +} + +.watchlist-scan-tracks { + margin-top: 10px; + max-height: 280px; + overflow-y: auto; + text-align: left; +} + +.watchlist-scan-tracks-section { + font-size: 11px; + font-weight: 600; + text-transform: uppercase; + letter-spacing: 0.4px; + color: #888; + margin: 10px 0 4px; +} + +.watchlist-scan-tracks .watchlist-live-addition-item { + position: relative; +} + +.watchlist-scan-track-badge { + margin-left: auto; + flex-shrink: 0; + padding: 2px 8px; + border-radius: 10px; + font-size: 10px; + font-weight: 600; + text-transform: uppercase; + letter-spacing: 0.3px; +} + +.watchlist-scan-track-badge.added { + background: rgba(80, 200, 120, 0.15); + color: #6fd99a; +} + +.watchlist-scan-track-badge.skipped { + background: rgba(255, 255, 255, 0.08); + color: #999; +} + /* Watchlist Search */ .watchlist-search-input { @@ -66708,6 +66767,55 @@ body.em-scroll-lock { overflow: hidden; } color: #f87171 !important; } +/* #831: origins grouped by what triggered them (artist / playlist) */ +.origin-group { + margin-bottom: 6px; +} + +.origin-group-header { + display: flex; + align-items: center; + gap: 8px; + width: 100%; + padding: 8px 12px; + background: rgba(255, 255, 255, 0.04); + border: 1px solid rgba(255, 255, 255, 0.08); + border-radius: 8px; + cursor: pointer; + text-align: left; + transition: background 0.15s ease; +} + +.origin-group-header:hover { + background: rgba(255, 255, 255, 0.08); +} + +.origin-group-caret { + color: rgba(255, 255, 255, 0.45); + font-size: 11px; + width: 12px; +} + +.origin-group-name { + flex: 1; + overflow: hidden; + text-overflow: ellipsis; + white-space: nowrap; + font-size: 13px; + font-weight: 600; + color: rgb(var(--accent-light-rgb)); +} + +.origin-group-count { + flex: 0 0 auto; + font-size: 11px; + color: rgba(255, 255, 255, 0.45); +} + +.origin-group-body { + padding: 4px 0 2px 6px; +} + /* ── Blocklist modal (artist/album/track bans) ── */ .blocklist-modal { position: relative; From 9f12bdfef6532ef9f10633864a9d09bd023d94fe Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Tue, 9 Jun 2026 20:35:16 -0700 Subject: [PATCH 08/67] Watchlist: bespoke live scan deck + persistent per-run Scan History (#831 round 2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Boulder: the live display was a cramped ~600px box showing a fraction of the data the scan already tracks, with no animation and no history. Live scan deck (replaces the three-column box, full width): - Header: pulsing live dot, "x / y artists" progress text, and two live counter chips (found / added) that pop when they change. - Animated progress bar (artist index / total) with a shimmer sweep. - Stage: artist avatar with accent glow + name + readable phase line ("Checking album 2 of 5"), album art + album + current track. - "Added to wishlist this run" feed: taller, bigger art, slide-in animation that plays once per new track (feed re-renders only when it changes). - All data was already in scan_state (current_artist_index, total_artists, tracks_found/added_this_scan, current_phase) — just never displayed. The legacy fullscreen-modal markup shares element ids and lacks the new ones, so it keeps working untouched. Scan History (persistent): - New watchlist_scan_runs table — one row per run (status, timestamps, artists/found/added counts) + the full track ledger JSON. Saved at scan completion AND cancellation; idempotent on run_id; pruned to the last 100 runs. Wishlist rows erode as tracks download, so this is the durable record. - GET /api/watchlist/scan/history (runs) + /history//tracks (ledger). - New History button on the Watchlist page → modal in the origins/blocklist house style: run cards (date, cancelled chip, artists/found/added stats) expanding into the Added / Skipped track lists with art and badges. Tests: save+fetch with ledger, idempotent re-save, prune keeps newest, unknown-run empty, cancelled runs recorded. 398 watchlist/wishlist/history tests pass; JS syntax-checked; all rendered strings escaped. --- database/music_database.py | 90 ++++++++ tests/test_watchlist_scan_history.py | 68 ++++++ web_server.py | 46 ++++ webui/index.html | 56 +++-- webui/static/api-monitor.js | 78 +++++-- webui/static/style.css | 331 +++++++++++++++++++++++++++ webui/static/watchlist-history.js | 149 ++++++++++++ 7 files changed, 792 insertions(+), 26 deletions(-) create mode 100644 tests/test_watchlist_scan_history.py create mode 100644 webui/static/watchlist-history.js diff --git a/database/music_database.py b/database/music_database.py index 0fc68c4c..d87e92de 100644 --- a/database/music_database.py +++ b/database/music_database.py @@ -649,6 +649,27 @@ class MusicDatabase: logger.info(f"Added {_col} column to library_history") cursor.execute("CREATE INDEX IF NOT EXISTS idx_lh_origin ON library_history (origin, created_at DESC)") + # Watchlist scan history (#831 round 2) — one row per scan run with + # its full track ledger (added/skipped), so the Watchlist page can + # show what every past run did. Wishlist rows erode as tracks + # download, so this is the durable record. + cursor.execute(""" + CREATE TABLE IF NOT EXISTS watchlist_scan_runs ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + run_id TEXT NOT NULL UNIQUE, + profile_id INTEGER DEFAULT 1, + status TEXT NOT NULL, + started_at TIMESTAMP, + completed_at TIMESTAMP, + total_artists INTEGER DEFAULT 0, + artists_scanned INTEGER DEFAULT 0, + tracks_found INTEGER DEFAULT 0, + tracks_added INTEGER DEFAULT 0, + track_events TEXT + ) + """) + cursor.execute("CREATE INDEX IF NOT EXISTS idx_wsr_completed ON watchlist_scan_runs (completed_at DESC)") + # Auto-import history — tracks auto-import scan results and processing status cursor.execute(""" CREATE TABLE IF NOT EXISTS auto_import_history ( @@ -12544,6 +12565,75 @@ class MusicDatabase: logger.debug(f"Error adding library history entry: {e}") return False + def save_watchlist_scan_run(self, run_id, profile_id=1, status='completed', + started_at=None, completed_at=None, + total_artists=0, artists_scanned=0, + tracks_found=0, tracks_added=0, + track_events=None, keep_last=100) -> bool: + """Persist one watchlist scan run + its track ledger (#831 round 2). + + Idempotent on run_id (re-saving a run replaces it). Prunes the table to + the most recent ``keep_last`` runs so history can't grow unbounded.""" + try: + conn = self._get_connection() + cursor = conn.cursor() + cursor.execute(""" + INSERT OR REPLACE INTO watchlist_scan_runs + (run_id, profile_id, status, started_at, completed_at, + total_artists, artists_scanned, tracks_found, tracks_added, + track_events) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?) + """, (run_id, profile_id, status, started_at, completed_at, + total_artists, artists_scanned, tracks_found, tracks_added, + json.dumps(track_events or []))) + cursor.execute(""" + DELETE FROM watchlist_scan_runs WHERE id NOT IN ( + SELECT id FROM watchlist_scan_runs + ORDER BY completed_at DESC, id DESC LIMIT ? + ) + """, (keep_last,)) + conn.commit() + return True + except Exception as e: + logger.error(f"Error saving watchlist scan run {run_id}: {e}") + return False + + def get_watchlist_scan_runs(self, limit=30, profile_id=None): + """Recent watchlist scan runs, newest first — WITHOUT track ledgers + (fetch those per-run via get_watchlist_scan_run_events).""" + try: + conn = self._get_connection() + cursor = conn.cursor() + where = "WHERE profile_id = ?" if profile_id is not None else "" + params = ([profile_id] if profile_id is not None else []) + [limit] + cursor.execute(f""" + SELECT run_id, profile_id, status, started_at, completed_at, + total_artists, artists_scanned, tracks_found, tracks_added + FROM watchlist_scan_runs {where} + ORDER BY completed_at DESC, id DESC LIMIT ? + """, params) + return [dict(row) for row in cursor.fetchall()] + except Exception as e: + logger.error(f"Error getting watchlist scan runs: {e}") + return [] + + def get_watchlist_scan_run_events(self, run_id): + """The track ledger (added/skipped events) for one scan run.""" + try: + conn = self._get_connection() + cursor = conn.cursor() + cursor.execute( + "SELECT track_events FROM watchlist_scan_runs WHERE run_id = ?", + (run_id,)) + row = cursor.fetchone() + if not row or not row['track_events']: + return [] + events = json.loads(row['track_events']) + return events if isinstance(events, list) else [] + except Exception as e: + logger.error(f"Error getting watchlist scan run events for {run_id}: {e}") + return [] + def get_origin_cleanup_candidates(self): """Origin-tracked downloads (watchlist/playlist) annotated with the matching library track's play_count, for the Expired Download Cleaner. diff --git a/tests/test_watchlist_scan_history.py b/tests/test_watchlist_scan_history.py new file mode 100644 index 00000000..3b6f4924 --- /dev/null +++ b/tests/test_watchlist_scan_history.py @@ -0,0 +1,68 @@ +"""Watchlist scan history persistence (#831 round 2). + +Every scan run is saved to watchlist_scan_runs with its track ledger so the +Watchlist History modal can show what each past run added — wishlist rows +erode as tracks download, so this table is the durable record. +""" + +from __future__ import annotations + +import pytest + +from database.music_database import MusicDatabase + + +@pytest.fixture() +def db(tmp_path): + return MusicDatabase(str(tmp_path / 'm.db')) + + +def _events(n_added=2, n_skipped=1): + evs = [{'track_name': f'Added {i}', 'artist_name': 'A', 'album_name': 'Al', + 'album_image_url': '', 'status': 'added'} for i in range(n_added)] + evs += [{'track_name': f'Skipped {i}', 'artist_name': 'A', 'album_name': 'Al', + 'album_image_url': '', 'status': 'skipped'} for i in range(n_skipped)] + return evs + + +def test_save_and_fetch_run_with_ledger(db): + assert db.save_watchlist_scan_run( + 'run-1', status='completed', + started_at='2026-06-09T20:00:00', completed_at='2026-06-09T20:05:00', + total_artists=63, artists_scanned=63, tracks_found=19, tracks_added=10, + track_events=_events()) + runs = db.get_watchlist_scan_runs() + assert len(runs) == 1 + r = runs[0] + assert (r['run_id'], r['status'], r['tracks_found'], r['tracks_added']) == \ + ('run-1', 'completed', 19, 10) + events = db.get_watchlist_scan_run_events('run-1') + assert [e['status'] for e in events] == ['added', 'added', 'skipped'] + + +def test_resave_is_idempotent_on_run_id(db): + db.save_watchlist_scan_run('run-1', tracks_added=10) + db.save_watchlist_scan_run('run-1', tracks_added=11) + runs = db.get_watchlist_scan_runs() + assert len(runs) == 1 and runs[0]['tracks_added'] == 11 + + +def test_prune_keeps_most_recent(db): + for i in range(1, 8): + db.save_watchlist_scan_run( + f'run-{i}', completed_at=f'2026-06-09T20:0{i}:00', keep_last=5) + runs = db.get_watchlist_scan_runs() + assert len(runs) == 5 + assert runs[0]['run_id'] == 'run-7' # newest first + assert all(r['run_id'] != 'run-1' for r in runs) # oldest pruned + + +def test_events_for_unknown_run_empty(db): + assert db.get_watchlist_scan_run_events('nope') == [] + + +def test_cancelled_run_recorded(db): + db.save_watchlist_scan_run('run-c', status='cancelled', tracks_added=3, + track_events=_events(1, 0)) + r = db.get_watchlist_scan_runs()[0] + assert r['status'] == 'cancelled' diff --git a/web_server.py b/web_server.py index 4f4aba3b..922bb85a 100644 --- a/web_server.py +++ b/web_server.py @@ -25996,6 +25996,29 @@ def start_watchlist_scan(): else: logger.warning("Watchlist scan cancelled — skipping post-scan steps") + # #831 round 2: persist this run + its track ledger so the + # Watchlist History modal can show what every past scan did. + try: + _state = watchlist_scan_state + get_database().save_watchlist_scan_run( + run_id=_state.get('scan_run_id') or datetime.now().strftime('%Y%m%d-%H%M%S'), + profile_id=scan_profile_id, + status='cancelled' if was_cancelled else 'completed', + started_at=(_state.get('started_at').isoformat() + if _state.get('started_at') else None), + completed_at=(_state.get('completed_at') or datetime.now()).isoformat() + if not isinstance(_state.get('completed_at'), str) + else _state.get('completed_at'), + total_artists=(_state.get('summary') or {}).get('total_artists', + _state.get('total_artists', 0)), + artists_scanned=(_state.get('summary') or {}).get('successful_scans', 0), + tracks_found=_state.get('tracks_found_this_scan', 0), + tracks_added=_state.get('tracks_added_this_scan', 0), + track_events=_state.get('scan_track_events') or [], + ) + except Exception as _hist_err: + logger.error(f"Failed to persist watchlist scan run: {_hist_err}") + # Post-scan steps — skip if cancelled if not was_cancelled: # Populate discovery pool from similar artists @@ -26152,6 +26175,29 @@ def get_watchlist_scan_status(): logger.error(f"Error getting watchlist scan status: {e}") return jsonify({"success": False, "error": str(e)}), 500 +@app.route('/api/watchlist/scan/history', methods=['GET']) +def get_watchlist_scan_history(): + """Recent watchlist scan runs (counts only — ledgers fetched per run).""" + try: + limit = min(int(request.args.get('limit', 30) or 30), 100) + runs = get_database().get_watchlist_scan_runs(limit=limit) + return jsonify({"success": True, "runs": runs}) + except Exception as e: + logger.error(f"Error getting watchlist scan history: {e}") + return jsonify({"success": False, "error": str(e)}), 500 + + +@app.route('/api/watchlist/scan/history//tracks', methods=['GET']) +def get_watchlist_scan_history_tracks(run_id): + """The track ledger (added/skipped) for one past scan run.""" + try: + events = get_database().get_watchlist_scan_run_events(run_id) + return jsonify({"success": True, "events": events}) + except Exception as e: + logger.error(f"Error getting watchlist scan history tracks: {e}") + return jsonify({"success": False, "error": str(e)}), 500 + + @app.route('/api/watchlist/scan/cancel', methods=['POST']) def cancel_watchlist_scan(): """Cancel a running watchlist scan""" diff --git a/webui/index.html b/webui/index.html index a0c5722b..2ebf113d 100644 --- a/webui/index.html +++ b/webui/index.html @@ -6975,20 +6975,45 @@