diff --git a/.github/workflows/docker-publish.yml b/.github/workflows/docker-publish.yml index a08b63a5..8b1d29d3 100644 --- a/.github/workflows/docker-publish.yml +++ b/.github/workflows/docker-publish.yml @@ -9,9 +9,9 @@ on: workflow_dispatch: inputs: version_tag: - description: 'Version tag (e.g. 2.7.8)' + description: 'Version tag (e.g. 2.8.0)' required: true - default: '2.7.8' + default: '2.8.0' jobs: build-and-push: diff --git a/RELEASE_2.7.9_discord.md b/RELEASE_2.7.9_discord.md new file mode 100644 index 00000000..80651c39 --- /dev/null +++ b/RELEASE_2.7.9_discord.md @@ -0,0 +1,13 @@ +**SoulSync 2.7.9** is out πŸŽ‰ a big one. + +🎚️ **Best-quality downloads** β€” downloads now follow a ranked quality profile you drag to order (FLAC 24/192 β†’ mp3). best-quality mode grabs the highest-quality copy across *every* source; priority mode gets an opt-in rank-based order toggle. quarantine is folded into the Downloads page + safer imports (AcoustID fail-closed, silence/truncation guards). + +🎧 **Discover got smart** β€” "Based On Your Listening" ranks artists from who you *actually play*, and "Your Listening Mix" is a playable track playlist of their top tracks (works on any source). Fresh Tape fills properly now. + +⚑ **Wing It Pool** β€” a new spot next to Discovery Pool to review + re-match the tracks Wing It guessed at (they used to be invisible). + +πŸ” **Auto-Sync redesign** β€” the scheduling board is now clean horizontal lanes instead of a side-scrolling column wall. + +πŸ› **Fixes** β€” multi-disc albums no longer show disc-2 as "missing" (#927), playlists no longer stuck on "Never Synced" (#925), and tracks can't import while quarantined (#928). thanks @ramonskie + @nick2000713 πŸ™ + +⚠️ **re-scan your library once** so the multi-disc fix can backfill disc numbers on existing tracks. enjoy! 🎢 diff --git a/core/download_engine/engine.py b/core/download_engine/engine.py index 1d555432..de1aca38 100644 --- a/core/download_engine/engine.py +++ b/core/download_engine/engine.py @@ -480,7 +480,7 @@ class DownloadEngine: return_exceptions=True, ) - for (source_name, _), result in zip(to_search, results): + for (source_name, _), result in zip(to_search, results, strict=True): if isinstance(result, Exception): logger.warning(f"{source_name} search failed: {result}") contributions.append(f"{source_name}=error") diff --git a/core/downloads/history_match.py b/core/downloads/history_match.py new file mode 100644 index 00000000..dadea5e1 --- /dev/null +++ b/core/downloads/history_match.py @@ -0,0 +1,62 @@ +"""Match a file back to its download-history row when its path has drifted (#934). + +``library_history.file_path`` is frozen at import time, but the file moves afterward +(media-server import, library reorganize) and ``tracks.file_path`` β€” what the AcoustID +scanner reads β€” no longer equals it. Matching on the exact path alone then fails twice: +the verification status never reaches the history row (verified tracks read "unverified"), +and a fresh ``acoustid_scan`` row gets inserted every run (thousands of duplicates). + +This module picks the canonical history row by exact path first, then by FILENAME guarded +by a title check β€” so a shared filename ("01 - Intro.flac") can never heal the wrong song. +Pure (no DB) so the matching rules are unit-testable; the caller does the SQL. +""" + +from __future__ import annotations + +import os +from typing import Iterable, Optional, Sequence, Tuple + + +def _norm_title(value) -> str: + """Alphanumeric-only lowercase form, so "Song (Remaster)" vs "song remaster" + style drift between the download tag and the media-server tag still agrees.""" + return ''.join(ch for ch in str(value or '').lower() if ch.isalnum()) + + +def like_filename_filter(basename: str) -> str: + """A ``LIKE ... ESCAPE '\\'`` pattern that coarsely matches rows whose path ends + in ``basename``. Escapes the LIKE metacharacters (``%`` ``_`` ``\\``) β€” filenames + routinely contain underscores. Callers MUST still confirm with an exact basename + compare (``pick_history_row`` does), since ``'%name'`` also matches ``'xname'``.""" + esc = basename.replace('\\', '\\\\').replace('%', '\\%').replace('_', '\\_') + return '%' + esc + + +def pick_history_row(candidates: Sequence[Tuple], *, current_paths: Iterable[str], + basename: str, title: str) -> Optional[int]: + """Return the id of the history row to update for this file, or None. + + ``candidates``: ``(id, file_path, title, download_source)`` rows the DB pre-filtered + (exact path or filename LIKE). A row matches when its path equals the current path OR + its filename matches AND its title agrees β€” the title guard prevents a shared filename + ("01 - Intro.flac") from healing a different song's row. Among matches a REAL download + row is preferred over a synthetic ``acoustid_scan`` row, so the scanner heals the + genuine record and the caller can delete the synthetic duplicate. None when nothing + matches safely (caller then inserts a fresh row β€” the "file SoulSync never downloaded" + intent).""" + paths = {p for p in current_paths if p} + want = _norm_title(title) + matches: list = [] # (id, is_exact, is_real) + for cid, cpath, ctitle, csource in candidates: + is_real = csource != 'acoustid_scan' + if cpath and cpath in paths: + matches.append((cid, True, is_real)) + elif (basename and cpath and os.path.basename(cpath) == basename + and (not want or not _norm_title(ctitle) or _norm_title(ctitle) == want)): + matches.append((cid, False, is_real)) + if not matches: + return None + # Prefer a REAL download row over a synthetic acoustid_scan row; within that, prefer an + # exact-path match over a filename match. Stable, so ties keep DB order (first/oldest id). + matches.sort(key=lambda m: (m[2], m[1]), reverse=True) + return matches[0][0] diff --git a/core/downloads/orphan_history.py b/core/downloads/orphan_history.py new file mode 100644 index 00000000..838f07f1 --- /dev/null +++ b/core/downloads/orphan_history.py @@ -0,0 +1,55 @@ +"""Identify dead review-queue history rows whose file is gone (#934 follow-up). + +The Unverified/Quarantine review queue is fed from ``library_history`` β€” an +append-only log that is never pruned. When a file is deleted, replaced, or +re-downloaded elsewhere, its old ``unverified`` row lingers forever and can +never be healed (there's no file left to confirm). Those are *orphans*. + +This decides which rows are orphans, given a ``resolve(row) -> path | None`` +the caller wires to the real filesystem lookup. Pure (no DB, no filesystem) so +the rules β€” including the safety gate β€” are unit-testable. + +Safety gate: a filesystem check mass-false-positives when the library mount is +down (every file looks missing). So if EVERY reviewed file is unreachable and +there are enough rows to judge, we flag it ``suspicious`` and the caller refuses +to delete β€” better to clean nothing than to wipe a healthy log during an outage. +""" + +from __future__ import annotations + +from typing import Any, Callable, Sequence + + +def find_orphan_history_ids( + rows: Sequence[dict], + resolve: Callable[[dict], Any], + *, + min_for_safety: int = 5, + deletable: Callable[[dict], bool] | None = None, +) -> dict: + """Return ``{'orphan_ids', 'checked', 'suspicious'}``. + + A row is an orphan when it has a non-empty ``file_path`` but ``resolve`` can + find no file for it. ``suspicious`` is True when every checked row is + missing and there are at least ``min_for_safety`` of them β€” the mount-down + signature; the caller should refuse to delete in that case. + + ``deletable`` (optional) protects rows from removal WITHOUT weakening the + safety gate: a protected row still counts toward ``checked`` and the + all-missing signal (so e.g. a few unverified orphans can't be swept during a + mount outage just because protected rows were filtered out first), but it + never appears in ``orphan_ids``. Default: every missing row is deletable. + """ + orphan_ids = [] + checked = 0 + missing = 0 + for row in rows: + if not str((row.get('file_path') or '')).strip(): + continue + checked += 1 + if resolve(row) is None: + missing += 1 + if deletable is None or deletable(row): + orphan_ids.append(row.get('id')) + suspicious = checked >= min_for_safety and missing == checked + return {'orphan_ids': orphan_ids, 'checked': checked, 'suspicious': suspicious} diff --git a/core/downloads/track_link.py b/core/downloads/track_link.py index e6dec8bd..3565a5ff 100644 --- a/core/downloads/track_link.py +++ b/core/downloads/track_link.py @@ -13,9 +13,31 @@ Pure + import-safe: parsing only, no network. from __future__ import annotations import re -from typing import Any, Optional, Tuple +from typing import Any, List, Optional, Tuple from urllib.parse import urlparse + +def linked_track_id(track: Any) -> str: + """The source track id stamped on a search result, read from + ``_source_metadata['track_id']`` β€” the field every ID-downloadable source + (Tidal, Qobuz) records. Empty string when absent. ``TrackResult`` has no + top-level ``id``, so callers must NOT use ``getattr(t, 'id')`` (that always + missed and left the pasted-link bubble a silent no-op β€” #932).""" + meta = getattr(track, '_source_metadata', None) + if not isinstance(meta, dict): + return '' + return str(meta.get('track_id') or '') + + +def bubble_linked_track_first(tracks: List[Any], link_track_id: str) -> List[Any]: + """Float the result whose source id matches a pasted link to the top so the + user sees the EXACT track they linked, not a fuzzy text-search lookalike + (#813/#932). Stable + a graceful no-op when no result carries the id.""" + if not link_track_id or not tracks: + return tracks + target = str(link_track_id) + return sorted(tracks, key=lambda t: linked_track_id(t) != target) + # host substring β†’ download source id. Only ID-downloadable streaming sources. _HOSTS = ( ('tidal.com', 'tidal'), diff --git a/core/imports/file_integrity.py b/core/imports/file_integrity.py index 4891cfd1..46542e53 100644 --- a/core/imports/file_integrity.py +++ b/core/imports/file_integrity.py @@ -52,6 +52,14 @@ _DEFAULT_LENGTH_TOLERANCE_S = 3.0 _LENGTH_TOLERANCE_LONG_TRACK_S = 5.0 _LONG_TRACK_THRESHOLD_S = 600.0 # 10 minutes +# A file that runs LONGER than the expected metadata is the opposite of a truncated +# download β€” it's almost always a different master/version (a remaster with a longer +# outro, an extended fade, an album cut vs the radio edit). The duration check exists to +# catch TRUNCATION (short files) and wildly-wrong matches, so on the auto default we allow +# more drift in the longer direction and keep the tight bound for short files. A wrong-song +# match still trips this β€” it's usually off by far more than 15s. (#937) +_LONGER_VERSION_TOLERANCE_S = 15.0 + # Upper bound for the user-configurable override. Anything past 60s # means the check is effectively off β€” cap defends against accidental # nonsense like 9999 making logs misleading. Users who genuinely want @@ -242,18 +250,32 @@ def check_audio_integrity( if expected_length_s > _LONG_TRACK_THRESHOLD_S else _DEFAULT_LENGTH_TOLERANCE_S ) + user_pinned_tolerance = False + else: + user_pinned_tolerance = True checks["length_tolerance_s"] = length_tolerance_s - drift_s = abs(actual_length_s - expected_length_s) + # Positive drift = the file runs LONGER than expected (not truncation). On the auto + # default, give the longer direction more room so legit longer masters/versions aren't + # quarantined (#937); a user-pinned tolerance is honoured symmetrically. + signed_drift_s = actual_length_s - expected_length_s + drift_s = abs(signed_drift_s) checks["length_drift_s"] = drift_s + effective_tolerance_s = length_tolerance_s + if signed_drift_s > 0 and not user_pinned_tolerance: + effective_tolerance_s = max(length_tolerance_s, _LONGER_VERSION_TOLERANCE_S) + checks["effective_tolerance_s"] = effective_tolerance_s - if drift_s > length_tolerance_s: + if drift_s > effective_tolerance_s: + runs_long = signed_drift_s > 0 return IntegrityResult( ok=False, reason=f"Duration mismatch: file is {actual_length_s:.1f}s, " f"expected {expected_length_s:.1f}s " - f"(drift {drift_s:.1f}s > tolerance {length_tolerance_s:.1f}s) β€” " - "likely truncated download or wrong file matched", + f"(drift {drift_s:.1f}s > tolerance {effective_tolerance_s:.1f}s) β€” " + + ("runs longer than expected β€” likely a different version/master or wrong file" + if runs_long + else "likely truncated download or wrong file matched"), checks=checks, ) diff --git a/core/jellyfin_client.py b/core/jellyfin_client.py index be4a7a26..fa4f61b0 100644 --- a/core/jellyfin_client.py +++ b/core/jellyfin_client.py @@ -105,6 +105,7 @@ class JellyfinTrack: self.title = jellyfin_data.get('Name', 'Unknown Track') self.duration = jellyfin_data.get('RunTimeTicks', 0) // 10000 # Convert from ticks to milliseconds self.trackNumber = jellyfin_data.get('IndexNumber') + self.discNumber = jellyfin_data.get('ParentIndexNumber') # multi-disc: disc number self.year = jellyfin_data.get('ProductionYear') self.userRating = jellyfin_data.get('UserData', {}).get('Rating') self.addedAt = self._parse_date(jellyfin_data.get('DateCreated')) diff --git a/core/library_reorganize.py b/core/library_reorganize.py index 574d5ee7..a7e4fa37 100644 --- a/core/library_reorganize.py +++ b/core/library_reorganize.py @@ -100,6 +100,7 @@ _ALBUM_ID_COLUMNS = { 'deezer': 'deezer_id', 'discogs': 'discogs_id', 'hydrabase': 'soul_id', + 'musicbrainz': 'musicbrainz_release_id', } # Human-facing label for each source. diff --git a/core/metadata/canonical_version.py b/core/metadata/canonical_version.py index 9871d89e..1f54a1e8 100644 --- a/core/metadata/canonical_version.py +++ b/core/metadata/canonical_version.py @@ -211,7 +211,7 @@ def pick_canonical_release( # core.library_reorganize._ALBUM_ID_COLUMNS β€” a test pins them in sync). A manual # match on any of these should pin/lock the canonical version (#758); a match on # a source the canonical tools don't read (e.g. lastfm) has no version to pin. -CANONICAL_ALBUM_SOURCES = frozenset({'spotify', 'itunes', 'deezer', 'discogs', 'hydrabase'}) +CANONICAL_ALBUM_SOURCES = frozenset({'spotify', 'itunes', 'deezer', 'discogs', 'hydrabase', 'musicbrainz'}) def should_pin_manual_canonical(entity_type: str, source: str) -> bool: diff --git a/core/metadata/discography_filters.py b/core/metadata/discography_filters.py index c5985fdf..0761d6d0 100644 --- a/core/metadata/discography_filters.py +++ b/core/metadata/discography_filters.py @@ -167,6 +167,7 @@ def track_already_owned( album_name: str, server_source: Optional[str], confidence_threshold: float = 0.7, + candidate_tracks: Optional[List[Any]] = None, ) -> bool: """Return True if the track is already in the user's library. @@ -186,6 +187,15 @@ def track_already_owned( original deleted) matches just fine β€” track_name + artist + album don't change with format. + ``candidate_tracks`` (when not None) is the artist's library tracks, + pre-fetched ONCE by the caller, so the check scores in-memory instead + of firing per-track fuzzy SQL scans against the whole library. Pass an + empty list for an artist the user owns nothing of β€” it still routes + through the fast in-memory path (scores against zero candidates β†’ + instant "not owned") rather than the slow per-track search. None + preserves the original per-track-SQL behaviour for callers that don't + pre-fetch. + Returns False on any exception so a transient DB hiccup doesn't silently nuke a discography fetch β€” a redundant wishlist add is much cheaper to recover from than a missed track. @@ -198,6 +208,7 @@ def track_already_owned( confidence_threshold=confidence_threshold, server_source=server_source, album=album_name or None, + candidate_tracks=candidate_tracks, ) except Exception: return False diff --git a/core/navidrome_client.py b/core/navidrome_client.py index 98536950..a51f5b2b 100644 --- a/core/navidrome_client.py +++ b/core/navidrome_client.py @@ -110,6 +110,7 @@ class NavidromeTrack: self.title = navidrome_data.get('title', 'Unknown Track') self.duration = navidrome_data.get('duration', 0) * 1000 # Convert to milliseconds self.trackNumber = navidrome_data.get('track') + self.discNumber = navidrome_data.get('discNumber') # multi-disc: disc number self.year = navidrome_data.get('year') self.userRating = navidrome_data.get('userRating') self.addedAt = self._parse_date(navidrome_data.get('created')) diff --git a/core/personalized_playlists.py b/core/personalized_playlists.py index 1f395eea..7b7b4738 100644 --- a/core/personalized_playlists.py +++ b/core/personalized_playlists.py @@ -111,7 +111,10 @@ class PersonalizedPlaylistsService: Either value can be None to skip that filter for that source. - Spotify: 60 / 40 (the existing 0-100 popularity scale) - - Deezer: 500000 / 100000 (rank values β€” ballpark from real data) + - Deezer: 60 / 50 (ALSO 0-100 β€” the discovery pool SYNTHESIZES deezer popularity to a + 0-100 score at scan time, capped at 100; it is NOT Deezer's raw rank. The old + 500000/100000 rank thresholds matched nothing, so Popular Picks came back empty for + deezer users while Hidden Gems' < 100000 caught the whole pool.) - iTunes / others: None / None (no popularity data; fall back to no threshold filter, just diversity-on-random) @@ -121,7 +124,7 @@ class PersonalizedPlaylistsService: if normalized == 'spotify': return 60, 40 if normalized == 'deezer': - return 500_000, 100_000 + return 60, 50 # iTunes, hydrabase, anything else β€” no usable popularity data return None, None diff --git a/core/qobuz_client.py b/core/qobuz_client.py index 1c7c3b6f..e90e934a 100644 --- a/core/qobuz_client.py +++ b/core/qobuz_client.py @@ -1074,6 +1074,10 @@ class QobuzClient(DownloadSourcePlugin): title=title, album=album_name, track_number=track.get('track_number'), + # Stamp the Qobuz track id so a pasted-link manual search can float the + # exact track to the top (#932). Without this the bubble never matched + # and the linked track stayed buried among fuzzy lookalikes. + _source_metadata={'source': 'qobuz', 'track_id': str(track.get('id') or '')}, ) # Stamp real API quality so the global ranker sees actual diff --git a/core/repair_jobs/__init__.py b/core/repair_jobs/__init__.py index 477eab1d..a8ed2fb6 100644 --- a/core/repair_jobs/__init__.py +++ b/core/repair_jobs/__init__.py @@ -52,6 +52,7 @@ _JOB_MODULES = [ 'core.repair_jobs.canonical_version_resolve', 'core.repair_jobs.library_retag', 'core.repair_jobs.quality_upgrade', + 'core.repair_jobs.short_preview_track', ] diff --git a/core/repair_jobs/acoustid_scanner.py b/core/repair_jobs/acoustid_scanner.py index 16ceabda..b52ef6aa 100644 --- a/core/repair_jobs/acoustid_scanner.py +++ b/core/repair_jobs/acoustid_scanner.py @@ -389,14 +389,43 @@ class AcoustIDScannerJob(RepairJob): cur.execute( "UPDATE tracks SET verification_status = ? WHERE id = ?", (status, track_id)) - matched = 0 + exp = expected or {} + # Find the canonical history row for this file. The stored path is frozen at + # import time while the file has since moved (media-server import / reorganize), + # so an exact-path match alone misses it β€” then the status never lands and a + # duplicate row gets inserted every scan (#934). Match exact path first, then + # filename guarded by title, and HEAL the row's path so future scans match cleanly. + from core.downloads.history_match import pick_history_row, like_filename_filter + current = fpath or db_path + basename = os.path.basename(current) if current else '' + clauses, params = [], [] for p in {p for p in (fpath, db_path) if p}: + clauses.append("file_path = ?") + params.append(p) + if basename: + clauses.append("file_path LIKE ? ESCAPE '\\'") + params.append(like_filename_filter(basename)) + row_id = None + if clauses: cur.execute( - "UPDATE library_history SET verification_status = ? WHERE file_path = ?", - (status, p)) - matched += max(getattr(cur, 'rowcount', 0) or 0, 0) - if status == 'unverified' and matched == 0: - exp = expected or {} + "SELECT id, file_path, title, download_source FROM library_history WHERE " + + " OR ".join(clauses), + params) + row_id = pick_history_row( + cur.fetchall(), + current_paths=(fpath, db_path), + basename=basename, title=exp.get('title') or '') + if row_id is not None: + cur.execute( + "UPDATE library_history SET verification_status = ?, file_path = ? WHERE id = ?", + (status, current, row_id)) + # Drop synthetic scan-created duplicates for this exact file (the #934 + # leftovers). Exact path β†’ collision-free; never touches a real download row. + cur.execute( + "DELETE FROM library_history WHERE id != ? AND download_source = 'acoustid_scan' " + "AND file_path = ?", + (row_id, current)) + elif status == 'unverified': cur.execute( """INSERT INTO library_history (event_type, title, artist_name, album_name, file_path, diff --git a/core/repair_jobs/album_completeness.py b/core/repair_jobs/album_completeness.py index 24b1260f..aa058d18 100644 --- a/core/repair_jobs/album_completeness.py +++ b/core/repair_jobs/album_completeness.py @@ -1,5 +1,10 @@ """Album Completeness Checker Job β€” finds albums missing tracks.""" +import re +import unicodedata +from collections import defaultdict +from difflib import SequenceMatcher + from core.metadata_service import ( get_album_tracks_for_source, get_primary_source, @@ -21,9 +26,12 @@ class AlbumCompletenessJob(RepairJob): help_text = ( 'Compares the number of tracks you have for each album against the expected total ' 'from your configured metadata sources. Counts cached during normal enrichment are ' - 'used when available; otherwise the job queries a metadata source directly. Albums ' - 'where tracks are missing get flagged as findings with details about which tracks ' - 'are absent.\n\n' + 'used when no canonical edition is pinned; otherwise the exact canonical source and ' + 'album ID are queried directly. The same canonical tracklist is used for both the ' + 'expected total and the missing-track calculation. Fragmented library rows are ' + 'combined only when they safely match that same canonical edition. Albums where ' + 'tracks are missing get flagged as findings with details about which tracks are ' + 'absent.\n\n' 'Useful for catching partial downloads or albums where some tracks failed to download. ' 'You can use the Download Missing feature from the album page to fill gaps.\n\n' 'Settings:\n' @@ -50,23 +58,32 @@ class AlbumCompletenessJob(RepairJob): min_completion_pct = settings.get('min_completion_pct', 0) primary_source = self._get_primary_source() - # Fetch all albums with ANY external source ID β€” not just Spotify + # Fetch all albums with ANY external source ID β€” not just Spotify. albums = [] conn = None has_itunes = False has_deezer = False + has_discogs = False + has_hydrabase = False + has_musicbrainz = False has_api_track_count = False + has_canonical = False try: conn = context.db._get_connection() cursor = conn.cursor() - # Check which source columns exist (older DBs may lack some) + # Check which source columns exist (older DBs may lack some). cursor.execute("PRAGMA table_info(albums)") columns = {row[1] for row in cursor.fetchall()} has_itunes = 'itunes_album_id' in columns has_deezer = 'deezer_id' in columns has_discogs = 'discogs_id' in columns has_hydrabase = 'soul_id' in columns + has_musicbrainz = 'musicbrainz_release_id' in columns + has_canonical = ( + 'canonical_source' in columns + and 'canonical_album_id' in columns + ) # Detect the `api_track_count` column β€” older DBs may not have it # yet (migration runs on app start, but repair-job code mustn't @@ -84,6 +101,7 @@ class AlbumCompletenessJob(RepairJob): # from metadata-source enrichment) or a live API lookup. select_cols = [ ('al.id', 'album_id'), + ('al.artist_id', 'artist_id'), ('al.title', 'album_title'), ('ar.name', 'artist_name'), ('al.spotify_album_id', 'spotify_album_id'), @@ -101,20 +119,52 @@ class AlbumCompletenessJob(RepairJob): select_cols.append(('al.discogs_id', 'discogs_album_id')) if has_hydrabase: select_cols.append(('al.soul_id', 'hydrabase_album_id')) + if has_musicbrainz: + select_cols.append( + ('al.musicbrainz_release_id', 'musicbrainz_album_id') + ) + if has_canonical: + select_cols.extend([ + ('al.canonical_source', 'canonical_source'), + ('al.canonical_album_id', 'canonical_album_id'), + ]) - # WHERE: album has at least one source ID - where_parts = ["(al.spotify_album_id IS NOT NULL AND al.spotify_album_id != '')"] + # WHERE: album has at least one source ID or a complete canonical pair. + where_parts = [ + "(al.spotify_album_id IS NOT NULL AND al.spotify_album_id != '')", + ] if has_itunes: - where_parts.append("(al.itunes_album_id IS NOT NULL AND al.itunes_album_id != '')") + where_parts.append( + "(al.itunes_album_id IS NOT NULL AND al.itunes_album_id != '')" + ) if has_deezer: - where_parts.append("(al.deezer_id IS NOT NULL AND al.deezer_id != '')") + where_parts.append( + "(al.deezer_id IS NOT NULL AND al.deezer_id != '')" + ) if has_discogs: - where_parts.append("(al.discogs_id IS NOT NULL AND al.discogs_id != '')") + where_parts.append( + "(al.discogs_id IS NOT NULL AND al.discogs_id != '')" + ) if has_hydrabase: - where_parts.append("(al.soul_id IS NOT NULL AND al.soul_id != '')") + where_parts.append( + "(al.soul_id IS NOT NULL AND al.soul_id != '')" + ) + if has_musicbrainz: + where_parts.append( + "(al.musicbrainz_release_id IS NOT NULL " + "AND al.musicbrainz_release_id != '')" + ) + if has_canonical: + where_parts.append( + "(al.canonical_source IS NOT NULL AND al.canonical_source != '' " + "AND al.canonical_album_id IS NOT NULL AND al.canonical_album_id != '')" + ) where_clause = ' OR '.join(where_parts) - select_sql = ', '.join(f'{expr} AS {alias}' for expr, alias in select_cols) + select_sql = ', '.join( + f'{expr} AS {alias}' + for expr, alias in select_cols + ) cursor.execute(f""" SELECT {select_sql} FROM albums al @@ -123,8 +173,20 @@ class AlbumCompletenessJob(RepairJob): WHERE {where_clause} GROUP BY al.id """) - albums = cursor.fetchall() - column_index = {alias: idx for idx, (_, alias) in enumerate(select_cols)} + raw_rows = cursor.fetchall() + column_index = { + alias: idx + for idx, (_, alias) in enumerate(select_cols) + } + albums = [ + { + alias: row[idx] + for alias, idx in column_index.items() + } + for row in raw_rows + ] + for order, album in enumerate(albums): + album['_scan_order'] = order except Exception as e: logger.error("Error fetching albums: %s", e, exc_info=True) result.errors += 1 @@ -133,70 +195,124 @@ class AlbumCompletenessJob(RepairJob): if conn: conn.close() - total = len(albums) + # Rows that share a source ID with a canonical album are candidates for + # the same release. A sibling joins the logical album only when at least + # one of its local tracks strictly matches the canonical tracklist. + work_items = self._prepare_work_items(context, albums) + + total = len(work_items) if context.update_progress: context.update_progress(0, total) - logger.info("Checking completeness of %d albums", total) + logger.info("Checking completeness of %d logical albums", total) if context.report_progress: - context.report_progress(phase=f'Checking {total} albums...', total=total) + context.report_progress( + phase=f'Checking {total} logical albums...', + total=total, + ) - for i, row in enumerate(albums): + for i, work_item in enumerate(work_items): if context.check_stop(): return result if i % 10 == 0 and context.wait_if_paused(): return result - album_id = row[column_index['album_id']] - title = row[column_index['album_title']] - artist_name = row[column_index['artist_name']] - spotify_album_id = row[column_index['spotify_album_id']] - actual_count = row[column_index['actual_count']] - album_thumb = row[column_index['album_thumb_url']] - artist_thumb = row[column_index['artist_thumb_url']] - itunes_album_id = row[column_index['itunes_album_id']] if 'itunes_album_id' in column_index else None - deezer_album_id = row[column_index['deezer_album_id']] if 'deezer_album_id' in column_index else None - discogs_album_id = row[column_index['discogs_album_id']] if 'discogs_album_id' in column_index else None - hydrabase_album_id = row[column_index['hydrabase_album_id']] if 'hydrabase_album_id' in column_index else None - # Cached authoritative track count from a prior API lookup (NULL - # on unscanned albums and on DBs predating the column migration). - cached_api_count = row[column_index['api_track_count']] if 'api_track_count' in column_index else None + row = work_item['row'] + album_id = row['album_id'] + title = row['album_title'] + artist_name = row['artist_name'] + spotify_album_id = row['spotify_album_id'] + actual_count = int(row['actual_count'] or 0) + album_thumb = row['album_thumb_url'] + artist_thumb = row['artist_thumb_url'] + itunes_album_id = row.get('itunes_album_id') + deezer_album_id = row.get('deezer_album_id') + discogs_album_id = row.get('discogs_album_id') + hydrabase_album_id = row.get('hydrabase_album_id') + musicbrainz_album_id = row.get('musicbrainz_album_id') + canonical_source = row.get('canonical_source') + canonical_album_id = row.get('canonical_album_id') + cached_api_count = row.get('api_track_count') result.scanned += 1 if context.report_progress: context.report_progress( - scanned=i + 1, total=total, + scanned=i + 1, + total=total, phase=f'Checking {i + 1} / {total}', - log_line=f'Album: {title or "Unknown"} β€” {artist_name or "Unknown"}', - log_type='info' + log_line=( + f'Album: {title or "Unknown"} β€” ' + f'{artist_name or "Unknown"}' + ), + log_type='info', ) - album_ids = { - 'spotify': spotify_album_id or '', - 'itunes': itunes_album_id or '', - 'deezer': deezer_album_id or '', - 'discogs': discogs_album_id or '', - 'hydrabase': hydrabase_album_id or '', - } + album_ids = self._source_ids_from_row(row) + resolved_source = primary_source + resolved_album_id = ( + self._get_album_id_for_source(primary_source, album_ids) + or '' + ) + related_album_ids = ( + work_item.get('related_album_ids') + or [album_id] + ) + raw_local_count = work_item.get('raw_local_count') + missing_tracks = None - # Expected total comes from the metadata provider, NOT from - # al.track_count β€” that column holds the observed count from - # server syncs (Plex leafCount, SoulSync standalone len(tracks)) - # which by definition always equals actual_count and made the - # job skip every album. Use the cached api_track_count if a - # prior scan already looked it up; otherwise hit the API and - # persist the answer for next time. - expected_total = cached_api_count - if not expected_total: - expected_total = self._get_expected_total(context, primary_source, album_ids) - # Only persist positive results. Zero/None would keep - # re-triggering the lookup on every scan. - if expected_total and expected_total > 0 and has_api_track_count: - self._save_api_track_count(context, album_id, expected_total) + # A complete canonical pair is authoritative. `_prepare_work_items` + # performs exactly one lookup per canonical group and stores the + # resulting list β€” including an empty list when the lookup failed. + if canonical_source and canonical_album_id: + canonical_items = work_item.get('canonical_items', []) + expected_total = len(canonical_items) + resolved_source = str(canonical_source) + resolved_album_id = str(canonical_album_id) - # Skip singles/EPs based on expected track count (not local count) + if canonical_items: + actual_count = int( + work_item.get( + 'effective_actual_count', + actual_count, + ) + ) + owned_reference_indexes = set( + work_item.get( + 'owned_reference_indexes', + set(), + ) + ) + missing_tracks = self._build_missing_tracks( + canonical_items, + owned_reference_indexes, + resolved_source, + ) + if raw_local_count is None: + raw_local_count = actual_count + else: + # Preserve the existing behavior for albums without a pinned + # canonical edition. + expected_total = cached_api_count + if not expected_total: + expected_total = self._get_expected_total( + context, + primary_source, + album_ids, + ) + if ( + expected_total + and expected_total > 0 + and has_api_track_count + ): + self._save_api_track_count( + context, + album_id, + expected_total, + ) + + # Skip singles/EPs based on expected track count (not local count). if expected_total and expected_total < min_tracks: result.skipped += 1 if context.update_progress and (i + 1) % 5 == 0: @@ -209,13 +325,15 @@ class AlbumCompletenessJob(RepairJob): context.update_progress(i + 1, total) continue - # Skip albums with zero local tracks β€” nothing to auto-fill from - if actual_count == 0: + effective_raw_local_count = ( + raw_local_count + if raw_local_count is not None + else int(row['actual_count'] or 0) + ) + if actual_count == 0 and effective_raw_local_count == 0: result.skipped += 1 continue - # Skip albums below minimum completion percentage - # (filters out "1 track from a playlist import" false positives) if min_completion_pct > 0 and expected_total > 0: completion = (actual_count / expected_total) * 100 if completion < min_completion_pct: @@ -224,14 +342,23 @@ class AlbumCompletenessJob(RepairJob): context.update_progress(i + 1, total) continue - # Album is incomplete β€” try to find which tracks are missing - missing_tracks = self._find_missing_tracks(context, primary_source, album_id, album_ids) + if missing_tracks is None: + missing_tracks = self._find_missing_tracks( + context, + primary_source, + album_id, + album_ids, + ) if context.report_progress: context.report_progress( - log_line=f'Incomplete: {title or "Unknown"} ({actual_count}/{expected_total})', - log_type='skip' + log_line=( + f'Incomplete: {title or "Unknown"} ' + f'({actual_count}/{expected_total})' + ), + log_type='skip', ) + if context.create_finding: try: inserted = context.create_finding( @@ -241,35 +368,52 @@ class AlbumCompletenessJob(RepairJob): entity_type='album', entity_id=str(album_id), file_path=None, - title=f'Incomplete: {title or "Unknown"} ({actual_count}/{expected_total})', + title=( + f'Incomplete: {title or "Unknown"} ' + f'({actual_count}/{expected_total})' + ), description=( - f'Album "{title}" by {artist_name or "Unknown"} has {actual_count} of ' - f'{expected_total} tracks' + f'Album "{title}" by {artist_name or "Unknown"} ' + f'has {actual_count} of {expected_total} tracks' ), details={ 'album_id': album_id, 'album_title': title, 'artist': artist_name, - 'primary_source': primary_source, - 'primary_album_id': self._get_album_id_for_source(primary_source, album_ids) or '', + 'primary_source': resolved_source, + 'primary_album_id': resolved_album_id, + 'canonical_source': canonical_source or '', + 'canonical_album_id': canonical_album_id or '', 'spotify_album_id': spotify_album_id or '', 'itunes_album_id': itunes_album_id or '', 'deezer_album_id': deezer_album_id or '', 'discogs_album_id': discogs_album_id or '', 'hydrabase_album_id': hydrabase_album_id or '', + 'musicbrainz_album_id': ( + musicbrainz_album_id or '' + ), 'expected_tracks': expected_total, 'actual_tracks': actual_count, + 'raw_local_tracks': effective_raw_local_count, + 'related_album_ids': [ + str(value) + for value in related_album_ids + ], 'missing_tracks': missing_tracks, 'album_thumb_url': album_thumb or None, 'artist_thumb_url': artist_thumb or None, - } + }, ) if inserted: result.findings_created += 1 else: result.findings_skipped_dedup += 1 except Exception as e: - logger.debug("Error creating completeness finding for album %s: %s", album_id, e) + logger.debug( + "Error creating completeness finding for album %s: %s", + album_id, + e, + ) result.errors += 1 if context.update_progress and (i + 1) % 5 == 0: @@ -278,134 +422,1017 @@ class AlbumCompletenessJob(RepairJob): if context.update_progress: context.update_progress(total, total) - logger.info("Completeness check: %d albums checked, %d incomplete found", - result.scanned, result.findings_created) + logger.info( + "Completeness check: %d logical albums checked, %d incomplete found", + result.scanned, + result.findings_created, + ) return result - def _save_api_track_count(self, context, album_id, count): - """Persist a metadata-API track count via the shared worker helper. + def _prepare_work_items(self, context, albums): + """Collapse safely validated fragments into logical canonical albums.""" + groups = self._build_candidate_groups(albums) + work_items = [] - Enrichment workers call `set_album_api_track_count` inside their own - `_update_album` transaction. Here we're in the repair job's fallback - path (the album wasn't enriched yet), so we own the connection + - commit ourselves. A cache-write failure must never break the scan, - so all errors are swallowed into the debug log. - """ + for group in groups: + # This prep phase front-loads the canonical API lookups + track + # matching, so honour Stop/Pause here too β€” otherwise a stop would + # be ignored until every group is processed. + check_stop = getattr(context, 'check_stop', None) + if callable(check_stop) and check_stop(): + break + + anchor = group['anchor'] + members = group['members'] + canonical_source = anchor.get('canonical_source') or '' + canonical_album_id = anchor.get('canonical_album_id') or '' + + if canonical_source and canonical_album_id: + canonical_tracks = self._get_album_tracks( + str(canonical_source), + str(canonical_album_id), + ) + canonical_items = self._extract_track_items( + canonical_tracks + ) + + if canonical_items: + local_by_album = { + str(member['album_id']): self._load_local_tracks( + context, + [member['album_id']], + ) + for member in members + } + anchor_tracks = local_by_album.get( + str(anchor['album_id']), + [], + ) + + # The anchor's persisted disc/track slots remain + # authoritative even when titles or durations drift. + anchor_reference = self._owned_reference_for_tracks( + canonical_items, + anchor_tracks, + str(canonical_source), + ) + + included = [anchor] + excluded = [] + supplemental_reference = set() + + for member in members: + if member is anchor: + continue + + member_tracks = local_by_album.get( + str(member['album_id']), + [], + ) + matched_reference, _ = ( + self._match_fragment_tracks( + canonical_items, + member_tracks, + str(canonical_source), + ) + ) + if matched_reference: + included.append(member) + supplemental_reference.update( + matched_reference - anchor_reference + ) + else: + excluded.append(member) + + combined_local = [] + for member in included: + combined_local.extend( + local_by_album.get( + str(member['album_id']), + [], + ) + ) + + owned_reference_indexes = ( + anchor_reference | supplemental_reference + ) + work_items.append({ + 'row': anchor, + 'canonical_items': canonical_items, + 'related_album_ids': [ + member['album_id'] + for member in included + ], + 'raw_local_count': len(combined_local), + 'effective_actual_count': ( + int(anchor.get('actual_count') or 0) + + len(supplemental_reference) + ), + 'owned_reference_indexes': ( + owned_reference_indexes + ), + '_scan_order': min( + member['_scan_order'] + for member in included + ), + }) + + for member in excluded: + # An excluded member that is itself pinned to this + # canonical edition is still evaluated against it (no + # fallback). It must report only the tracks it *doesn't* + # own β€” compute its own owned slots from its local + # tracks, exactly like the anchor, instead of leaving + # the set empty (which would flag the whole tracklist as + # missing, including tracks the row already has). + if self._same_canonical_pair( + member, + canonical_source, + canonical_album_id, + ): + member_tracks = local_by_album.get( + str(member['album_id']), + [], + ) + member_owned = ( + self._owned_reference_for_tracks( + canonical_items, + member_tracks, + str(canonical_source), + ) + ) + work_items.append( + self._independent_work_item( + member, + canonical_items=canonical_items, + owned_reference_indexes=member_owned, + effective_actual_count=len( + member_owned + ), + ) + ) + else: + work_items.append( + self._independent_work_item(member) + ) + continue + + # The canonical lookup was attempted and returned no usable + # tracklist. Canonical rows must not fall back to another + # provider; non-canonical siblings remain independent. + for member in members: + work_items.append( + self._independent_work_item( + member, + canonical_items=( + [] + if self._same_canonical_pair( + member, + canonical_source, + canonical_album_id, + ) + else None + ), + ) + ) + continue + + for member in members: + work_items.append( + self._independent_work_item(member) + ) + + work_items.sort(key=lambda item: item['_scan_order']) + return work_items + + def _independent_work_item( + self, + row, + canonical_items=None, + owned_reference_indexes=None, + effective_actual_count=None, + ): + item = { + 'row': row, + 'related_album_ids': [row['album_id']], + '_scan_order': row['_scan_order'], + } + if canonical_items is not None: + item['canonical_items'] = canonical_items + if owned_reference_indexes is not None: + item['owned_reference_indexes'] = owned_reference_indexes + if effective_actual_count is not None: + item['effective_actual_count'] = effective_actual_count + return item + + def _same_canonical_pair( + self, + row, + canonical_source, + canonical_album_id, + ): + return ( + str(row.get('canonical_source') or '') + == str(canonical_source) + and str(row.get('canonical_album_id') or '') + == str(canonical_album_id) + ) + + def _build_candidate_groups(self, albums): + """Group non-canonical rows around unambiguous canonical anchors.""" + canonical_groups = defaultdict(list) + + for album in albums: + source = album.get('canonical_source') or '' + canonical_id = album.get('canonical_album_id') or '' + if source and canonical_id: + key = ( + str(album.get('artist_id') or ''), + str(source), + str(canonical_id), + ) + canonical_groups[key].append(album) + + canonical_row_ids = { + str(row['album_id']) + for rows in canonical_groups.values() + for row in rows + } + + alias_to_groups = defaultdict(set) + for key, anchor_rows in canonical_groups.items(): + for row in anchor_rows: + artist_id = str(row.get('artist_id') or '') + for source, source_id in ( + self._source_ids_from_row(row).items() + ): + if source_id: + alias_to_groups[ + (artist_id, source, str(source_id)) + ].add(key) + + canonical_source = ( + row.get('canonical_source') or '' + ) + canonical_id = ( + row.get('canonical_album_id') or '' + ) + alias_to_groups[ + ( + artist_id, + str(canonical_source), + str(canonical_id), + ) + ].add(key) + + # Assign each non-canonical row to a group in ONE pass over the albums + # (O(N)), instead of rescanning every album for every group (O(G*N), + # which degrades badly once many editions are pinned). A row joins a + # group only when its stored IDs resolve to exactly one canonical group + # β€” identical to the old `matches == {key}` rule, just computed once. + assigned_candidate_ids = set() + candidates_by_group = defaultdict(list) + for row in albums: + row_id = str(row['album_id']) + if row_id in canonical_row_ids: + continue + + artist_id = str(row.get('artist_id') or '') + matches = set() + for source, source_id in ( + self._source_ids_from_row(row).items() + ): + if source_id: + matches.update( + alias_to_groups.get( + (artist_id, source, str(source_id)), + set(), + ) + ) + + if len(matches) == 1: + (key,) = tuple(matches) + candidates_by_group[key].append(row) + assigned_candidate_ids.add(row_id) + + groups = [] + for key, anchor_rows in canonical_groups.items(): + anchor = max( + anchor_rows, + key=lambda row: ( + int(row.get('actual_count') or 0), + 1 if row.get('album_title') else 0, + -int(row.get('_scan_order') or 0), + ), + ) + members = list(anchor_rows) + candidates_by_group.get(key, []) + + groups.append({ + 'anchor': anchor, + 'members': sorted( + members, + key=lambda row: row['_scan_order'], + ), + '_scan_order': min( + row['_scan_order'] + for row in members + ), + }) + + grouped_ids = canonical_row_ids | assigned_candidate_ids + for row in albums: + if str(row['album_id']) in grouped_ids: + continue + groups.append({ + 'anchor': row, + 'members': [row], + '_scan_order': row['_scan_order'], + }) + + groups.sort(key=lambda group: group['_scan_order']) + return groups + + def _source_ids_from_row(self, row): + """Return every stored release ID available on an album row.""" + return { + 'spotify': row.get('spotify_album_id') or '', + 'itunes': row.get('itunes_album_id') or '', + 'deezer': row.get('deezer_album_id') or '', + 'discogs': row.get('discogs_album_id') or '', + 'hydrabase': row.get('hydrabase_album_id') or '', + 'musicbrainz': row.get('musicbrainz_album_id') or '', + } + + def _load_local_tracks(self, context, album_ids): + """Load local tracks while tolerating older track-table schemas.""" + ids = [ + str(album_id) + for album_id in album_ids + if album_id is not None + ] + if not ids: + return [] + + placeholders = ','.join('?' for _ in ids) conn = None try: conn = context.db._get_connection() cursor = conn.cursor() - set_album_api_track_count(cursor, album_id, count) - conn.commit() + cursor.execute("PRAGMA table_info(tracks)") + columns = { + row[1] + for row in cursor.fetchall() + } + + select_cols = [ + ( + 'id' if 'id' in columns else 'NULL', + 'id', + ), + ('album_id', 'album_id'), + ( + 'title' if 'title' in columns else "''", + 'title', + ), + ( + 'track_number' + if 'track_number' in columns + else 'NULL', + 'track_number', + ), + ( + 'disc_number' + if 'disc_number' in columns + else '1', + 'disc_number', + ), + ( + 'duration' + if 'duration' in columns + else '0', + 'duration', + ), + ( + 'musicbrainz_recording_id' + if 'musicbrainz_recording_id' in columns + else "''", + 'musicbrainz_recording_id', + ), + ] + select_sql = ', '.join( + f'{expression} AS {alias}' + for expression, alias in select_cols + ) + cursor.execute( + f""" + SELECT {select_sql} + FROM tracks + WHERE album_id IN ({placeholders}) + """, + ids, + ) + return [ + { + 'id': row[0], + 'album_id': row[1], + 'title': row[2] or '', + 'track_number': row[3], + 'disc_number': ( + row[4] + if row[4] is not None + else 1 + ), + 'duration_ms': row[5] or 0, + 'musicbrainz_recording_id': row[6] or '', + } + for row in cursor.fetchall() + ] except Exception as e: - logger.debug("Failed to cache api_track_count for album %s: %s", album_id, e) + logger.debug( + "Failed loading local tracks for albums %s: %s", + ids, + e, + ) + return [] finally: if conn: conn.close() - def _get_expected_total(self, context, primary_source, album_ids): - """Try to get the expected track count from the active metadata provider first.""" + def _normalize_title(self, value): + text = unicodedata.normalize('NFKD', str(value or '')) + text = ''.join( + char + for char in text + if not unicodedata.combining(char) + ) + text = text.casefold() + text = re.sub(r'[\W_]+', ' ', text, flags=re.UNICODE) + return ' '.join(text.split()) + + def _as_int(self, value, default=None): + if value is None or value == '': + return default + if isinstance(value, bool): + return int(value) + if isinstance(value, (int, float)): + return int(value) + match = re.search(r'\d+', str(value)) + return int(match.group(0)) if match else default + + def _reference_duration_ms(self, item): + value = item.get('duration_ms') + if value not in (None, ''): + try: + return int(float(value)) + except (TypeError, ValueError): + return 0 + + value = item.get('duration') + if value not in (None, ''): + try: + return int(float(value) * 1000) + except (TypeError, ValueError): + return 0 + return 0 + + def _reference_slots_for_local_tracks( + self, + reference_items, + local_tracks, + ): + """Map unambiguous local disc/track slots to reference indexes.""" + slots = defaultdict(list) + for index, item in enumerate(reference_items): + number = self._as_int(item.get('track_number')) + disc = self._as_int( + item.get('disc_number'), + 1, + ) + if number is not None: + slots[(disc, number)].append(index) + + matched = set() + for track in local_tracks: + number = self._as_int(track.get('track_number')) + disc = self._as_int( + track.get('disc_number'), + 1, + ) + indexes = slots.get((disc, number), []) + if len(indexes) == 1: + matched.add(indexes[0]) + return matched + + def _owned_reference_for_tracks( + self, + reference_items, + local_tracks, + reference_source, + ): + """Reference indexes a set of local tracks owns: fuzzy one-to-one + matches plus the persisted disc/track slots (authoritative even when + titles or durations drift). Used for the anchor and for any excluded + sibling that is still evaluated against this canonical edition.""" + owned, _ = self._match_tracks( + reference_items, + local_tracks, + reference_source, + ) + owned.update( + self._reference_slots_for_local_tracks( + reference_items, + local_tracks, + ) + ) + return owned + + def _track_match_score( + self, + reference, + local, + reference_source, + ): + """Return a score when a local track plausibly matches a reference.""" + reference_id = str(reference.get('id') or '') + local_mbid = str( + local.get('musicbrainz_recording_id') or '' + ) + if ( + reference_source == 'musicbrainz' + and reference_id + and reference_id == local_mbid + ): + return 1000 + + reference_number = self._as_int( + reference.get('track_number') + ) + local_number = self._as_int( + local.get('track_number') + ) + reference_disc = self._as_int( + reference.get('disc_number'), + 1, + ) + local_disc = self._as_int( + local.get('disc_number'), + 1, + ) + + same_number = ( + reference_number is not None + and local_number is not None + and reference_number == local_number + ) + same_slot = same_number and reference_disc == local_disc + + reference_title = self._normalize_title( + reference.get('name') + or reference.get('title') + ) + local_title = self._normalize_title( + local.get('title') + ) + title_ratio = ( + SequenceMatcher( + None, + reference_title, + local_title, + ).ratio() + if reference_title and local_title + else 0.0 + ) + exact_title = bool( + reference_title + and reference_title == local_title + ) + + reference_duration = self._reference_duration_ms( + reference + ) + local_duration = ( + self._as_int( + local.get('duration_ms'), + 0, + ) + or 0 + ) + has_durations = ( + reference_duration > 0 + and local_duration > 0 + ) + duration_close = ( + has_durations + and abs( + reference_duration - local_duration + ) <= 15000 + ) + + if same_slot and title_ratio >= 0.65: + return ( + 800 + + int(title_ratio * 100) + + (30 if duration_close else 0) + ) + if same_slot and duration_close: + return 740 + int(title_ratio * 100) + if exact_title: + return ( + 700 + + (50 if duration_close else 0) + + (20 if same_number else 0) + ) + if ( + title_ratio >= 0.92 + and (duration_close or not has_durations) + ): + return 650 + int(title_ratio * 100) + if ( + same_number + and title_ratio >= 0.80 + and duration_close + ): + return 620 + int(title_ratio * 100) + return None + + def _fragment_track_match_score( + self, + reference, + local, + reference_source, + ): + """Use stricter matching when accepting a sibling fragment.""" + reference_id = str(reference.get('id') or '') + local_mbid = str( + local.get('musicbrainz_recording_id') or '' + ) + if ( + reference_source == 'musicbrainz' + and reference_id + and reference_id == local_mbid + ): + return 1000 + + reference_title = self._normalize_title( + reference.get('name') + or reference.get('title') + ) + local_title = self._normalize_title( + local.get('title') + ) + if not reference_title or not local_title: + return None + + title_ratio = SequenceMatcher( + None, + reference_title, + local_title, + ).ratio() + exact_title = reference_title == local_title + + reference_number = self._as_int( + reference.get('track_number') + ) + local_number = self._as_int( + local.get('track_number') + ) + reference_disc = self._as_int( + reference.get('disc_number'), + 1, + ) + local_disc = self._as_int( + local.get('disc_number'), + 1, + ) + same_slot = ( + reference_number is not None + and local_number is not None + and reference_number == local_number + and reference_disc == local_disc + ) + + reference_duration = self._reference_duration_ms( + reference + ) + local_duration = ( + self._as_int( + local.get('duration_ms'), + 0, + ) + or 0 + ) + duration_close = ( + reference_duration > 0 + and local_duration > 0 + and abs( + reference_duration - local_duration + ) <= 15000 + ) + + if exact_title: + return ( + 900 + + (30 if same_slot else 0) + + (20 if duration_close else 0) + ) + if ( + title_ratio >= 0.95 + and (same_slot or duration_close) + ): + return 800 + int(title_ratio * 100) + if ( + title_ratio >= 0.85 + and same_slot + and duration_close + ): + return 700 + int(title_ratio * 100) + return None + + def _match_fragment_tracks( + self, + reference_items, + local_tracks, + reference_source, + ): + """One-to-one strict match used only for candidate siblings.""" + return self._greedy_match( + reference_items, + local_tracks, + reference_source, + self._fragment_track_match_score, + ) + + def _match_tracks( + self, + reference_items, + local_tracks, + reference_source, + ): + """One-to-one general match for tracks already on the anchor.""" + return self._greedy_match( + reference_items, + local_tracks, + reference_source, + self._track_match_score, + ) + + def _greedy_match( + self, + reference_items, + local_tracks, + reference_source, + scorer, + ): + candidates = [] + for reference_index, reference in enumerate( + reference_items + ): + for local_index, local in enumerate(local_tracks): + score = scorer( + reference, + local, + reference_source, + ) + if score is not None: + candidates.append( + ( + score, + reference_index, + local_index, + ) + ) + + candidates.sort(reverse=True) + matched_reference = set() + matched_local = set() + + for _, reference_index, local_index in candidates: + if ( + reference_index in matched_reference + or local_index in matched_local + ): + continue + matched_reference.add(reference_index) + matched_local.add(local_index) + + return matched_reference, matched_local + + def _build_missing_tracks( + self, + reference_items, + matched_reference, + reference_source, + ): + missing_tracks = [] + for index, item in enumerate(reference_items): + if index in matched_reference: + continue + + track_artists = [] + for artist in item.get('artists', []): + if isinstance(artist, dict): + track_artists.append( + artist.get('name', '') + ) + elif isinstance(artist, str): + track_artists.append(artist) + + source_track_id = item.get('id', '') + missing_tracks.append({ + 'track_number': item.get('track_number'), + 'name': ( + item.get('name') + or item.get('title') + or '' + ), + 'disc_number': item.get('disc_number', 1), + 'source': item.get( + '_source', + reference_source, + ), + 'source_track_id': source_track_id, + 'track_id': source_track_id, + 'spotify_track_id': source_track_id, + 'duration_ms': self._reference_duration_ms( + item + ), + 'artists': track_artists, + }) + return missing_tracks + + def _save_api_track_count(self, context, album_id, count): + """Persist a metadata-API track count via the shared worker helper.""" + conn = None + try: + conn = context.db._get_connection() + cursor = conn.cursor() + set_album_api_track_count( + cursor, + album_id, + count, + ) + conn.commit() + except Exception as e: + logger.debug( + "Failed to cache api_track_count for album %s: %s", + album_id, + e, + ) + finally: + if conn: + conn.close() + + def _get_expected_total( + self, + context, + primary_source, + album_ids, + ): + """Get the expected count from the active provider priority.""" for source in get_source_priority(primary_source): - album_id = self._get_album_id_for_source(source, album_ids) + album_id = self._get_album_id_for_source( + source, + album_ids, + ) if not album_id: continue - api_tracks = self._get_album_tracks(source, album_id) + api_tracks = self._get_album_tracks( + source, + album_id, + ) items = self._extract_track_items(api_tracks) if items: return len(items) - return 0 - def _find_missing_tracks(self, context, primary_source, album_id, album_ids): - """Identify which specific tracks are missing using the active metadata provider first.""" - # Get track numbers we already have + def _find_missing_tracks( + self, + context, + primary_source, + album_id, + album_ids, + resolved_source=None, + resolved_items=None, + ): + """Identify missing tracks from one resolved metadata edition.""" owned_numbers = set() conn = None try: conn = context.db._get_connection() cursor = conn.cursor() cursor.execute( - "SELECT track_number FROM tracks WHERE album_id = ? AND track_number IS NOT NULL", - (album_id,) + "SELECT track_number FROM tracks " + "WHERE album_id = ? " + "AND track_number IS NOT NULL", + (album_id,), ) - for tr in cursor.fetchall(): - owned_numbers.add(tr[0]) + for track in cursor.fetchall(): + owned_numbers.add(track[0]) except Exception: return [] finally: if conn: conn.close() - api_tracks = None - for source in get_source_priority(primary_source): - source_album_id = self._get_album_id_for_source(source, album_ids) - if not source_album_id: - continue - api_tracks = self._get_album_tracks(source, source_album_id) - if self._extract_track_items(api_tracks): - break + if resolved_items is None: + api_tracks = None + for source in get_source_priority( + primary_source + ): + source_album_id = ( + self._get_album_id_for_source( + source, + album_ids, + ) + ) + if not source_album_id: + continue + api_tracks = self._get_album_tracks( + source, + source_album_id, + ) + if self._extract_track_items(api_tracks): + resolved_source = source + break + items = self._extract_track_items(api_tracks) + else: + items = resolved_items - items = self._extract_track_items(api_tracks) if not items: return [] - # All supported provider responses expose the same core fields once normalized. - # items[].track_number, items[].name, items[].disc_number, items[].id, items[].artists - missing_tracks = [] - for item in items: - tn = item.get('track_number') - if tn and tn not in owned_numbers: - track_artists = [] - for a in item.get('artists', []): - if isinstance(a, dict): - track_artists.append(a.get('name', '')) - elif isinstance(a, str): - track_artists.append(a) - missing_tracks.append({ - 'track_number': tn, - 'name': item.get('name', ''), - 'disc_number': item.get('disc_number', 1), - 'source': item.get('_source', primary_source), - 'source_track_id': item.get('id', ''), - 'track_id': item.get('id', ''), - 'spotify_track_id': item.get('id', ''), - 'duration_ms': item.get('duration_ms', 0), - 'artists': track_artists, - }) - return missing_tracks + track_source = resolved_source or primary_source + matched_reference = { + index + for index, item in enumerate(items) + if ( + item.get('track_number') + and item.get('track_number') in owned_numbers + ) + } + return self._build_missing_tracks( + items, + matched_reference, + track_source, + ) def _get_settings(self, context: JobContext) -> dict: if not context.config_manager: return self.default_settings.copy() - cfg = context.config_manager.get(f'repair.jobs.{self.job_id}.settings', {}) + cfg = context.config_manager.get( + f'repair.jobs.{self.job_id}.settings', + {}, + ) merged = self.default_settings.copy() merged.update(cfg) return merged def _get_primary_source(self) -> str: - """Return the active metadata source used for source prioritization.""" + """Return the active metadata source for prioritization.""" try: return get_primary_source() except Exception: return 'deezer' - def _get_album_id_for_source(self, source: str, album_ids: dict) -> str: + def _get_album_id_for_source( + self, + source: str, + album_ids: dict, + ) -> str: return album_ids.get(source, '') - def _get_album_tracks(self, source: str, album_id: str): + def _get_album_tracks( + self, + source: str, + album_id: str, + ): """Fetch album tracks from a specific source.""" try: - return get_album_tracks_for_source(source, album_id) + return get_album_tracks_for_source( + source, + album_id, + ) except Exception as e: - logger.debug("Error getting %s album tracks for %s: %s", source.capitalize(), album_id, e) + logger.debug( + "Error getting %s album tracks for %s: %s", + source.capitalize(), + album_id, + e, + ) return None def _extract_track_items(self, api_tracks): - """Normalize album track responses to a list of item dicts.""" + """Normalize provider responses to a list of track dicts.""" if not api_tracks: return [] if isinstance(api_tracks, dict): - items = api_tracks.get('items') or [] + items = ( + api_tracks.get('items') + or api_tracks.get('tracks') + or [] + ) + if isinstance(items, dict): + items = items.get('items') or [] return items if items else [] if isinstance(api_tracks, list): return api_tracks @@ -417,19 +1444,51 @@ class AlbumCompletenessJob(RepairJob): conn = context.db._get_connection() cursor = conn.cursor() - # Check which columns exist cursor.execute("PRAGMA table_info(albums)") - columns = {row[1] for row in cursor.fetchall()} + columns = { + row[1] + for row in cursor.fetchall() + } - where_parts = ["(spotify_album_id IS NOT NULL AND spotify_album_id != '')"] + where_parts = [ + "(spotify_album_id IS NOT NULL " + "AND spotify_album_id != '')", + ] if 'itunes_album_id' in columns: - where_parts.append("(itunes_album_id IS NOT NULL AND itunes_album_id != '')") + where_parts.append( + "(itunes_album_id IS NOT NULL " + "AND itunes_album_id != '')" + ) if 'deezer_id' in columns: - where_parts.append("(deezer_id IS NOT NULL AND deezer_id != '')") + where_parts.append( + "(deezer_id IS NOT NULL " + "AND deezer_id != '')" + ) if 'discogs_id' in columns: - where_parts.append("(discogs_id IS NOT NULL AND discogs_id != '')") + where_parts.append( + "(discogs_id IS NOT NULL " + "AND discogs_id != '')" + ) if 'soul_id' in columns: - where_parts.append("(soul_id IS NOT NULL AND soul_id != '')") + where_parts.append( + "(soul_id IS NOT NULL " + "AND soul_id != '')" + ) + if 'musicbrainz_release_id' in columns: + where_parts.append( + "(musicbrainz_release_id IS NOT NULL " + "AND musicbrainz_release_id != '')" + ) + if ( + 'canonical_source' in columns + and 'canonical_album_id' in columns + ): + where_parts.append( + "(canonical_source IS NOT NULL " + "AND canonical_source != '' " + "AND canonical_album_id IS NOT NULL " + "AND canonical_album_id != '')" + ) cursor.execute(f""" SELECT COUNT(*) FROM albums diff --git a/core/repair_jobs/short_preview_track.py b/core/repair_jobs/short_preview_track.py new file mode 100644 index 00000000..4335db47 --- /dev/null +++ b/core/repair_jobs/short_preview_track.py @@ -0,0 +1,267 @@ +"""Repair job: detect ~30s PREVIEW clips and re-fetch the full track. + +The HiFi endpoint (and occasionally others) sometimes deliver a ~30-second preview/sample +instead of the full song. Those land in the library looking like real tracks. This job +finds short tracks, looks up the EXPECTED length from the track's metadata source, and β€” +when the source says the real track is meaningfully longer than the file β€” flags it as a +preview clip. + +Approving the finding (in repair_worker._fix_short_preview_track) deletes the preview file, +drops the DB row so the track goes missing again, and re-adds it to the wishlist with a full +payload so the real version gets downloaded. The scan itself ONLY creates findings β€” nothing +is deleted, removed, or wishlisted without the user approving, exactly like the other tools. + +Conservative by design (it deletes a file): a track is only flagged when the source confirms +it should be much longer. Genuine short tracks (intros, skits, interludes β€” where the source +agrees the track is short) are left alone, and tracks whose length can't be verified from a +source are skipped, never flagged. +""" + +from __future__ import annotations + +from typing import Any, Dict, Optional + +from core.repair_jobs import register_job +from core.repair_jobs.base import JobContext, JobResult, RepairJob +from utils.logging_config import get_logger + +logger = get_logger("repair_jobs.short_preview") + + +def _art_from_details(details: Dict[str, Any]) -> Optional[str]: + """Pull a renderable album-art URL out of a get_track_details() response. The cleaned dict + doesn't carry images, but raw_data does: Spotify β†’ raw_data.album.images[0].url, iTunes β†’ + raw_data.artworkUrl100 (upscaled). Returns None if neither is present.""" + raw = (details or {}).get("raw_data") or {} + album = raw.get("album") + if isinstance(album, dict): + images = album.get("images") + if isinstance(images, list) and images and isinstance(images[0], dict) and images[0].get("url"): + return images[0]["url"] + art = raw.get("artworkUrl100") or raw.get("artworkUrl60") or raw.get("artworkUrl30") + if art: + # iTunes serves tiny thumbnails by default; bump to a usable size. + for small in ("100x100bb", "60x60bb", "30x30bb"): + art = art.replace(small, "600x600bb") + return art + return None + + +@register_job +class ShortPreviewTrackJob(RepairJob): + job_id = "short_preview_track" + display_name = "Preview Clip Cleanup" + description = ( + "Finds ~30s preview clips that slipped in instead of the full song (common from the " + "HiFi endpoint) and re-fetches the real track." + ) + help_text = ( + "Some downloads β€” especially via the HiFi source β€” deliver a ~30-second preview clip " + "instead of the full song. They look like normal tracks in your library. This job scans " + "for short tracks, checks how long the track ACTUALLY is from its metadata source " + "(Spotify / iTunes / MusicBrainz), and flags any whose real length is much greater than " + "the file β€” i.e. a preview.\n\n" + "Approving a finding deletes the preview file, removes the track from the database (so it " + "shows as missing), and re-adds it to your Wishlist so the full version downloads.\n\n" + "It's conservative: genuine short tracks (intros, skits) where the source agrees the track " + "is short are left alone, and tracks whose length can't be verified are skipped.\n\n" + "Settings:\n" + " - max_duration_seconds: only tracks at or below this length are considered (default 30).\n" + " - min_expected_drift_seconds: the source must say the real track is at least this many " + "seconds longer than the file before it's flagged (default 30)." + ) + icon = "scissors" + default_enabled = False + default_interval_hours = 168 # weekly + default_settings = { + "max_duration_seconds": 30, + "min_expected_drift_seconds": 30, + } + setting_options: Dict[str, list] = {} + auto_fix = False + + def _setting_int(self, context: JobContext, key: str, default: int) -> int: + cm = getattr(context, "config_manager", None) + if cm is None: + return default + try: + return int(cm.get(self.get_config_key(key), default) or default) + except (TypeError, ValueError): + return default + + def scan(self, context: JobContext) -> JobResult: + result = JobResult() + max_dur_s = self._setting_int(context, "max_duration_seconds", 30) + min_drift_s = self._setting_int(context, "min_expected_drift_seconds", 30) + max_dur_ms = max_dur_s * 1000 + + conn = context.db._get_connection() + try: + cursor = conn.cursor() + cursor.execute( + """ + SELECT t.id, t.title, t.duration, t.file_path, + t.spotify_track_id, t.itunes_track_id, t.musicbrainz_recording_id, + ar.name AS artist_name, ar.thumb_url AS artist_thumb, + al.title AS album_title, al.thumb_url AS album_thumb + FROM tracks t + LEFT JOIN artists ar ON ar.id = t.artist_id + LEFT JOIN albums al ON al.id = t.album_id + WHERE t.duration IS NOT NULL AND t.duration > 0 AND t.duration <= ? + AND t.file_path IS NOT NULL AND t.file_path != '' + """, + (max_dur_ms,), + ) + rows = [dict(r) for r in cursor.fetchall()] + finally: + conn.close() + + total = len(rows) + if context.report_progress: + try: + context.report_progress(phase=f"Checking {total} short tracks for previews…", total=total) + except Exception: # noqa: S110 β€” progress is best-effort + pass + + for i, row in enumerate(rows): + if context.check_stop() or context.wait_if_paused(): + break + result.scanned += 1 + + title = row["title"] or "Unknown" + artist = row["artist_name"] or "Unknown" + # Live progress EVERY track β€” the source lookup below is a network call, so without + # per-track reporting the UI looks frozen at "Starting…" (the #937-follow-up report). + if context.update_progress: + try: + context.update_progress(i + 1, total) + except Exception: # noqa: S110 β€” best-effort + pass + if context.report_progress: + try: + context.report_progress( + phase=f"Checking {i + 1}/{total} short tracks for previews…", + log_line=f"{artist} β€” {title}", scanned=i + 1, total=total, + ) + except Exception: # noqa: S110 β€” best-effort + pass + + file_dur_s = (row["duration"] or 0) / 1000.0 + source = self._lookup_source(context, row) + + # Can't verify the real length β†’ never flag (a delete must be backed by evidence). + if source is None: + result.skipped += 1 + continue + expected_dur_s = source["duration_s"] + # Prefer the source's album art (a renderable CDN url) over the library thumb, which + # is often empty/non-renderable for un-enriched HiFi previews β†’ art-less wishlist orb. + album_image = source.get("album_image") or row["album_thumb"] + + # Source agrees the track is short (genuine intro/skit) β†’ leave it alone. Only a + # source that says the real track is MUCH longer than the file marks a preview. + if (expected_dur_s - file_dur_s) < min_drift_s: + result.skipped += 1 + continue + + if context.create_finding: + title = row["title"] or "Unknown" + artist = row["artist_name"] or "Unknown" + try: + inserted = context.create_finding( + job_id=self.job_id, + finding_type="short_preview_track", + severity="warning", + entity_type="track", + entity_id=str(row["id"]), + file_path=row["file_path"], + title=f"Preview clip: {artist} - {title}", + description=( + f'File is {file_dur_s:.0f}s but "{title}" by {artist} is ' + f"{expected_dur_s:.0f}s at the source β€” looks like a preview clip. " + "Approve to delete it and re-download the full version." + ), + details={ + "track_id": row["id"], + "title": row["title"], + "artist": row["artist_name"], + "album": row["album_title"], + "album_thumb_url": album_image, + "artist_thumb_url": row["artist_thumb"], + "file_duration_s": round(file_dur_s, 1), + "expected_duration_s": round(expected_dur_s, 1), + "original_path": row["file_path"], + }, + ) + if inserted: + result.findings_created += 1 + else: + result.findings_skipped_dedup += 1 + except Exception as exc: + logger.debug("create_finding failed for track %s: %s", row["id"], exc) + result.errors += 1 + + return result + + def _lookup_source(self, context: JobContext, row: Dict[str, Any]) -> Optional[Dict[str, Any]]: + """Look the track up at its metadata source: returns {'duration_s', 'album_image'} or + None when no source id is usable / the lookup fails. The SAME lookup that confirms the + real length also carries the album art (in raw_data), which we capture so the re-wishlist + isn't art-less when the library album thumb is missing (the #937-follow-up: HiFi previews + on un-enriched albums). Every metadata client exposes get_track_details(id).""" + + def _build(details) -> Optional[Dict[str, Any]]: + ms = (details or {}).get("duration_ms") + if not ms or ms <= 0: + return None + return {"duration_s": ms / 1000.0, "album_image": _art_from_details(details)} + + # Spotify β€” pass allow_fallback=False. The default fallback scrapes the configured + # metadata source, which is slow and can BLOCK a scan loop indefinitely when the + # official API isn't authed (the #937-follow-up hang). Official-only is fast and + # returns None cleanly when unavailable, so we just move to the next source. + sp_id = row.get("spotify_track_id") + if sp_id and context.spotify_client and not context.is_spotify_rate_limited(): + try: + r = _build(context.spotify_client.get_track_details(str(sp_id), allow_fallback=False)) + if r: + return r + except TypeError: + pass # older client without the flag β€” skip, don't risk the slow path + except Exception as exc: + logger.debug("spotify lookup failed for %s: %s", sp_id, exc) + + # iTunes (public API, no auth, fast) then MusicBrainz. + for source_id, client in ( + (row.get("itunes_track_id"), context.itunes_client), + (row.get("musicbrainz_recording_id"), context.mb_client), + ): + if not source_id or client is None: + continue + getter = getattr(client, "get_track_details", None) + if getter is None: + continue + try: + r = _build(getter(str(source_id))) + if r: + return r + except Exception as exc: + logger.debug("lookup failed for %s: %s", source_id, exc) + return None + + def estimate_scope(self, context: JobContext) -> int: + try: + max_dur_ms = self._setting_int(context, "max_duration_seconds", 30) * 1000 + conn = context.db._get_connection() + try: + cursor = conn.cursor() + cursor.execute( + "SELECT COUNT(*) FROM tracks WHERE duration > 0 AND duration <= ? " + "AND file_path IS NOT NULL AND file_path != ''", + (max_dur_ms,), + ) + return (cursor.fetchone() or [0])[0] + finally: + conn.close() + except Exception: + return 0 diff --git a/core/repair_worker.py b/core/repair_worker.py index 64f9864a..55b682f7 100644 --- a/core/repair_worker.py +++ b/core/repair_worker.py @@ -997,6 +997,7 @@ class RepairWorker: 'quality_upgrade': self._fix_quality_upgrade, 'missing_discography_track': self._fix_discography_backfill, 'library_retag': self._fix_library_retag, + 'short_preview_track': self._fix_short_preview_track, } handler = handlers.get(finding_type) if not handler: @@ -1211,6 +1212,118 @@ class RepairWorker: if conn: conn.close() + def _fix_short_preview_track(self, entity_type, entity_id, file_path, details): + """Approve a preview-clip finding: delete the ~30s preview file, drop its DB row, and + re-add the track to the wishlist (full payload) so the real version downloads. Mirrors + the dead-file 'redownload' payload + the acoustid-mismatch file delete. (Tools #937-adj) + """ + if not entity_id: + return {'success': False, 'error': 'No track ID associated with this finding'} + conn = None + try: + conn = self.db._get_connection() + cursor = conn.cursor() + cursor.execute(""" + SELECT t.id, t.title, t.track_number, t.duration, t.bitrate, + t.spotify_track_id, t.itunes_track_id, t.deezer_id, t.isrc, + ar.name AS artist_name, ar.spotify_artist_id, + al.title AS album_title, al.spotify_album_id, + al.record_type, al.track_count, al.year, al.thumb_url AS album_thumb + FROM tracks t + LEFT JOIN artists ar ON ar.id = t.artist_id + LEFT JOIN albums al ON al.id = t.album_id + WHERE t.id = ? + """, (entity_id,)) + row = cursor.fetchone() + if not row: + return {'success': False, 'error': 'Track not found in database'} + + track_name = row['title'] or details.get('title', 'Unknown') + artist_name = row['artist_name'] or details.get('artist', 'Unknown Artist') + album_title = row['album_title'] or details.get('album', '') + + wishlist_id = (row['spotify_track_id'] + or row['itunes_track_id'] + or row['deezer_id'] + or f"preview_redl_{entity_id}") + + # Prefer the finding's stored art (the scan captures the metadata source's CDN image) + # over the library album thumb, which is often empty for un-enriched HiFi previews. + album_images = [] + album_thumb = details.get('album_thumb_url') or row['album_thumb'] + if album_thumb: + album_images = [{'url': album_thumb}] + + spotify_track_data = { + 'id': wishlist_id, + 'name': track_name, + 'artists': [{'name': artist_name}], + 'album': { + 'name': album_title or track_name, + 'id': row['spotify_album_id'] or '', + 'release_date': str(row['year']) if row['year'] else '', + 'images': album_images, + 'album_type': row['record_type'] or 'album', + 'total_tracks': row['track_count'] or 0, + 'artists': [{'name': artist_name}], + }, + 'duration_ms': int((details.get('expected_duration_s') or 0) * 1000) or (row['duration'] or 0), + 'track_number': row['track_number'] or 1, + 'disc_number': 1, + 'explicit': False, + 'external_urls': {}, + 'popularity': 0, + 'preview_url': None, + 'uri': f"spotify:track:{row['spotify_track_id']}" if row['spotify_track_id'] else '', + 'is_local': False, + } + + source_info = { + 'original_path': file_path or details.get('original_path', ''), + 'album_title': album_title, + 'artist': artist_name, + 'reason': 'preview_clip_redownload', + } + + added = self.db.add_to_wishlist( + spotify_track_data, + failure_reason='Preview clip β€” re-downloading full track', + source_type='redownload', + source_info=source_info, + ) + if not added: + return {'success': False, 'error': 'Failed to add to wishlist (may already exist or be blocklisted)'} + + # Delete the preview file (path resolved like the other delete tools). + deleted_file = False + target_path = file_path or details.get('original_path') + if target_path: + download_folder = self._config_manager.get('soulseek.download_path', '') if self._config_manager else None + resolved = _resolve_file_path(target_path, self.transfer_folder, + download_folder=download_folder, + config_manager=self._config_manager) + if resolved and os.path.exists(resolved): + try: + os.remove(resolved) + deleted_file = True + except Exception as e: + logger.warning("Could not delete preview file %s: %s", resolved, e) + + # Drop the DB row so the track shows as missing. + cursor.execute("DELETE FROM tracks WHERE id = ?", (entity_id,)) + conn.commit() + + return {'success': True, 'action': 'added_to_wishlist', + 'message': (f'Deleted preview clip and re-wishlisted "{track_name}" for full download' + if deleted_file else + f'Re-wishlisted "{track_name}" (preview file already gone)')} + except Exception as e: + logger.error("Preview-clip fix failed for track %s: %s", entity_id, e) + return {'success': False, 'error': str(e)} + finally: + if conn: + conn.close() + def _fix_orphan_file(self, entity_type, entity_id, file_path, details): """Handle an orphan file β€” move to staging or delete based on user choice. @@ -3419,7 +3532,7 @@ class RepairWorker: 'incomplete_album', 'path_mismatch', 'missing_lossy_copy', 'missing_replaygain', 'empty_folder', 'missing_discography_track', 'acoustid_mismatch', - 'quality_upgrade') + 'quality_upgrade', 'short_preview_track') placeholders = ','.join(['?'] * len(fixable_types)) where_parts = [f"finding_type IN ({placeholders})", "status = 'pending'"] params = list(fixable_types) diff --git a/core/watchlist/auto_scan.py b/core/watchlist/auto_scan.py index 759d43f7..e26ab998 100644 --- a/core/watchlist/auto_scan.py +++ b/core/watchlist/auto_scan.py @@ -185,7 +185,11 @@ def process_watchlist_scan_automatically(automation_id=None, profile_id=None, de 'results': [], 'summary': {}, 'error': None, - 'cancel_requested': False + 'cancel_requested': False, + # #933: stamp these so this scan lands in the History modal too β€” + # the scanner fills scan_track_events; persist_scan_run reads both. + 'scan_run_id': datetime.now().strftime('%Y%m%d-%H%M%S'), + 'scan_track_events': [], } scan_results = [] @@ -294,6 +298,17 @@ def process_watchlist_scan_automatically(automation_id=None, profile_id=None, de total_added_to_wishlist = deps.watchlist_scan_state.get('summary', {}).get('tracks_added_to_wishlist', 0) logger.warning("Automatic watchlist scan cancelled β€” skipping post-scan steps") + # #933: record this run in the History ledger β€” same helper the manual + # scan uses, so scheduled scans show up alongside manual ones. + try: + from core.watchlist.scan_history import persist_scan_run + persist_scan_run( + database, deps.watchlist_scan_state, + profile_id=profile_id, was_cancelled=was_cancelled, + ) + 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 (per-profile) diff --git a/core/watchlist/scan_history.py b/core/watchlist/scan_history.py new file mode 100644 index 00000000..a87011ca --- /dev/null +++ b/core/watchlist/scan_history.py @@ -0,0 +1,51 @@ +"""Persist a finished watchlist scan to the History ledger (#831 / #933). + +Both the manual scan (``web_server.start_watchlist_scan``) and the automatic +scan (``core.watchlist.auto_scan.process_watchlist_scan_automatically``) finish +with the same ``watchlist_scan_state`` shape, but only the manual path used to +record a history row β€” so scheduled/nightly scans never showed up in the +History modal (#933). This single helper is the shared seam: both paths call it, +so they can't drift apart again. + +Pure except for the one ``database.save_watchlist_scan_run`` call β€” the field +extraction is unit-testable with a fake database. +""" + +from __future__ import annotations + +from datetime import datetime +from typing import Any, Dict, Optional + + +def _iso(value: Any) -> Optional[str]: + """ISO-format a datetime; pass through an already-stringified timestamp.""" + if value is None: + return None + return value.isoformat() if hasattr(value, 'isoformat') else str(value) + + +def persist_scan_run(database: Any, state: Dict[str, Any], *, + profile_id: Any, was_cancelled: bool) -> bool: + """Record one watchlist scan run + its track ledger from ``state``. + + Reads the counts/timestamps/ledger off the live ``watchlist_scan_state`` the + scanner just finished writing, and writes a single history row. ``run_id`` + comes from ``state['scan_run_id']`` (both paths stamp it); a timestamp + fallback keeps it from ever colliding if that's somehow missing. Returns the + DB call's truthiness; callers wrap in their own try/except so a history-write + failure never breaks the scan. + """ + summary = state.get('summary') or {} + run_id = state.get('scan_run_id') or datetime.now().strftime('%Y%m%d-%H%M%S') + return database.save_watchlist_scan_run( + run_id=run_id, + profile_id=profile_id if profile_id else 1, + status='cancelled' if was_cancelled else 'completed', + started_at=_iso(state.get('started_at')), + completed_at=_iso(state.get('completed_at')) or datetime.now().isoformat(), + total_artists=summary.get('total_artists', state.get('total_artists', 0)), + artists_scanned=summary.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 [], + ) diff --git a/core/watchlist_scanner.py b/core/watchlist_scanner.py index 55d03105..f8eda5e3 100644 --- a/core/watchlist_scanner.py +++ b/core/watchlist_scanner.py @@ -328,6 +328,22 @@ _ALBUM_QUALIFIER_RE = re.compile( re.IGNORECASE, ) +# A trailing "- ..." clause is stripped ONLY when EVERY token in it is an edition/format +# qualifier (+ connectors / a year-ordinal). So "- Single", "- Acoustic Version", "- 2011 +# Remaster" collapse to the base name, but a real distinguishing subtitle ("- Nos vies en +# LumiΓ¨re", "- Live in Berlin") is kept β€” the bug was a blanket "- anything$" strip that +# erased subtitles and fused different editions (Sokhi: Expedition 33 OST vs Bonus Edition). +_DASH_QUALIFIER_WORD = ( + r'live|acoustic|electric|instrumental|unplugged|mono|stereo|demos?|reissue|' + r'remix(?:es)?|edit(?:ed)?|radio|single|ep|lp|version|mix(?:es)?|sessions?|bootleg|' + r'covers?|original|redux|deluxe|expanded|remaster(?:ed)?|anniversary|special|' + r'edition|bonus|extended|explicit|clean|soundtrack|ost|score' +) +_TRAILING_DASH_QUALIFIER_RE = re.compile( + r'\s*-\s*(?:(?:' + _DASH_QUALIFIER_WORD + r'|the|a|and|&|\+|\d+(?:st|nd|rd|th)?)\b[\s\-]*)+$', + re.IGNORECASE, +) + def _normalize_album_for_match(name: str) -> str: """Return a canonical form of an album name suitable for fuzzy comparison. @@ -347,8 +363,9 @@ def _normalize_album_for_match(name: str) -> str: # they're almost always edition or commentary noise, not part of the # album's identifying name. cleaned = re.sub(r'\s*[\(\[][^\)\]]*[\)\]]\s*', ' ', cleaned) - # Trailing dash-clauses ("Album - Remastered", "Album - Live") - cleaned = re.sub(r'\s*-\s*[^-]+$', '', cleaned) + # Trailing dash-clause, but ONLY when it's entirely edition/format qualifiers β€” a real + # subtitle is preserved (see _TRAILING_DASH_QUALIFIER_RE). + cleaned = _TRAILING_DASH_QUALIFIER_RE.sub(' ', cleaned) cleaned = re.sub(r'[^a-z0-9 ]+', ' ', cleaned.lower()) cleaned = re.sub(r'\s+', ' ', cleaned).strip() return cleaned @@ -382,7 +399,7 @@ def _extract_volume_marker(normalized_name: str): return last.group(1) or last.group(2) -def _albums_likely_match(spotify_album: str, lib_album: str, threshold: float = 0.6) -> bool: +def _albums_likely_match(spotify_album: str, lib_album: str, threshold: float = 0.85) -> bool: """Return True when two album names plausibly identify the same release. Designed to swallow naming drift between metadata sources and the @@ -406,11 +423,11 @@ def _albums_likely_match(spotify_album: str, lib_album: str, threshold: float = return False if norm_a == norm_b: return True - # After normalization the shorter name often becomes a prefix / - # substring of the longer one ("napoleon dynamite" βŠ‚ "napoleon - # dynamite music from the motion picture" before stripping). - if norm_a in norm_b or norm_b in norm_a: - return True + # No loose substring shortcut: after qualifier-stripping, a short name being a + # prefix of a longer one is usually a DIFFERENT edition carrying a real subtitle + # ("clair obscur expedition 33" βŠ‚ "clair obscur expedition 33 nos vies en lumiere"), + # not naming drift. Genuine drift collapses to an EXACT match above; everything else + # must clear a high overall-similarity bar. return SequenceMatcher(None, norm_a, norm_b).ratio() >= threshold diff --git a/core/wishlist/routes.py b/core/wishlist/routes.py index c9536828..5fe27fd9 100644 --- a/core/wishlist/routes.py +++ b/core/wishlist/routes.py @@ -8,6 +8,8 @@ import threading from dataclasses import dataclass from typing import Any, Callable, Dict +from core.metadata import normalize_image_url +from core.metadata.artwork import is_internal_image_host from core.wishlist.reporting import build_wishlist_stats_payload from core.wishlist.selection import prepare_wishlist_tracks_for_display from core.wishlist.service import get_wishlist_service @@ -210,6 +212,74 @@ def set_wishlist_cycle(runtime: WishlistRouteRuntime, cycle: str) -> tuple[Dict[ return {"error": str(exc)}, 500 +def _needs_image_fix(url: str | None) -> bool: + """True when an image URL won't render in the browser as-is β€” a media-server RELATIVE + path (/library/.., /Items/.., /rest/..) or an internal/localhost host. Spotify/iTunes CDN + URLs render directly and are left untouched, so already-working items never change.""" + if not url or not isinstance(url, str): + return False + if url.startswith('/') and not url.startswith('//'): + return True + if url.startswith('http://') or url.startswith('https://'): + return is_internal_image_host(url) + return False + + +def _enrich_wishlist_images(tracks: list[dict[str, Any]], db: Any) -> dict[str, str]: + """Make wishlist art browser-renderable using the library data we already have. + + The library stores album/artist art as media-server RELATIVE paths (e.g. Plex + /library/metadata/..) which don't render in a browser . Normal wishlist items carry + Spotify CDN URLs (fine), but library-sourced items β€” re-downloads and preview-clip + re-fetches β€” carry the relative path, so their art comes up blank. We fix two things here, + on read, so it also repairs items already sitting in the wishlist: + + 1. Normalize each track's album.images[*].url that needs it (relative/internal only β€” + CDN URLs are left as-is to avoid regressing items that already render). + 2. Build an artist-name -> normalized library photo map so the nebula can show artist + photos for non-watchlist artists (it otherwise only has watchlisted-artist photos). + """ + artist_names: set[str] = set() + for track in tracks: + sd = track.get('spotify_data') + if isinstance(sd, dict): + album = sd.get('album') + if isinstance(album, dict): + images = album.get('images') + if isinstance(images, list): + for img in images: + if isinstance(img, dict) and _needs_image_fix(img.get('url')): + fixed = normalize_image_url(img['url']) + if fixed: + img['url'] = fixed + name = track.get('artist_name') + if name and name != 'Unknown Artist': + artist_names.add(name) + + artist_images: dict[str, str] = {} + if not artist_names: + return artist_images + try: + conn = db._get_connection() + try: + placeholders = ','.join('?' * len(artist_names)) + rows = conn.execute( + f"SELECT name, thumb_url FROM artists " + f"WHERE name IN ({placeholders}) AND thumb_url IS NOT NULL AND thumb_url != ''", + list(artist_names), + ).fetchall() + finally: + conn.close() + for row in rows: + name, thumb = row[0], row[1] + fixed = normalize_image_url(thumb) if _needs_image_fix(thumb) else thumb + if name and fixed: + artist_images[name.lower()] = fixed + except Exception as exc: # noqa: BLE001 β€” art is cosmetic, never fail the tracks endpoint + logger.debug("Could not build wishlist artist-image map: %s", exc) + return artist_images + + def get_wishlist_tracks( runtime: WishlistRouteRuntime, *, @@ -242,6 +312,9 @@ def get_wishlist_tracks( prepared["duplicates_found"], ) + # Make library-sourced art renderable + supply artist photos (see _enrich_wishlist_images). + artist_images = _enrich_wishlist_images(prepared["tracks"], db) + if category: runtime.logger.info( "Wishlist filter: %s/%s tracks in '%s' category (limit: %s)", @@ -250,9 +323,18 @@ def get_wishlist_tracks( category, limit or "none", ) - return {"tracks": prepared["tracks"], "category": category, "total": prepared["total"]}, 200 + return { + "tracks": prepared["tracks"], + "category": category, + "total": prepared["total"], + "artist_images": artist_images, + }, 200 - return {"tracks": prepared["tracks"], "total": prepared["total"]}, 200 + return { + "tracks": prepared["tracks"], + "total": prepared["total"], + "artist_images": artist_images, + }, 200 except Exception as exc: runtime.logger.error("Error getting wishlist tracks: %s", exc) return {"error": str(exc)}, 500 diff --git a/core/youtube_client.py b/core/youtube_client.py index a1cef67b..8cd4b400 100644 --- a/core/youtube_client.py +++ b/core/youtube_client.py @@ -38,6 +38,20 @@ from core.download_plugins.types import SearchResult, TrackResult, AlbumResult, logger = get_logger("youtube_client") +def _resolve_cookie_opts() -> dict: + """yt-dlp cookie options from Settings β†’ YouTube: either a browser store OR a + pasted cookies.txt. The 'Paste cookies.txt' dropdown value is the sentinel + 'custom' β€” which must become a yt-dlp ``cookiefile`` pointing at the saved file, + NOT be passed through as a browser name (yt-dlp rejects: 'unsupported browser: + custom'). Delegates to the shared, tested precedence in core.youtube_cookies.""" + from config.settings import config_manager + from core.youtube_cookies import build_youtube_cookie_opts + mode = config_manager.get('youtube.cookies_browser', '') + cookiefile = config_manager.get('youtube.cookies_file', '') + exists = bool(cookiefile) and os.path.exists(cookiefile) + return build_youtube_cookie_opts(mode, cookiefile, cookiefile_exists=exists) + + @dataclass class YouTubeSearchResult: """YouTube search result with metadata parsing""" @@ -220,11 +234,9 @@ class YouTubeClient(DownloadSourcePlugin): 'age_limit': None, # Don't skip age-restricted } - # Cookie support β€” use browser cookies for YouTube auth - from config.settings import config_manager - cookies_browser = config_manager.get('youtube.cookies_browser', '') - if cookies_browser: - self.download_opts['cookiesfrombrowser'] = (cookies_browser,) + # Cookie support β€” a logged-in browser store OR a pasted cookies.txt + # (the 'custom' paste mode resolves to a cookiefile, not a browser name). + self.download_opts.update(_resolve_cookie_opts()) # Track current download progress (mirrors Soulseek transfer tracking) self.current_download_id: Optional[str] = None @@ -309,11 +321,12 @@ class YouTubeClient(DownloadSourcePlugin): """Reload YouTube settings from config (called when settings are saved).""" from config.settings import config_manager self._download_delay = config_manager.get('youtube.download_delay', 3) - cookies_browser = config_manager.get('youtube.cookies_browser', '') - if cookies_browser: - self.download_opts['cookiesfrombrowser'] = (cookies_browser,) - elif 'cookiesfrombrowser' in self.download_opts: - del self.download_opts['cookiesfrombrowser'] + # Clear both cookie sources, then re-apply from current settings (browser + # store or pasted cookies.txt) so a mode switch doesn't leave a stale arg. + self.download_opts.pop('cookiesfrombrowser', None) + self.download_opts.pop('cookiefile', None) + _cookie_opts = _resolve_cookie_opts() + self.download_opts.update(_cookie_opts) # Reload download path new_path = Path(config_manager.get('soulseek.download_path', './downloads')) @@ -323,7 +336,7 @@ class YouTubeClient(DownloadSourcePlugin): self.download_opts['outtmpl'] = str(self.download_path / '%(title)s.%(ext)s') logger.info(f"YouTube download path updated to: {self.download_path}") - logger.info(f"YouTube settings reloaded (delay={self._download_delay}s, cookies={'enabled' if cookies_browser else 'disabled'})") + logger.info(f"YouTube settings reloaded (delay={self._download_delay}s, cookies={'enabled' if _cookie_opts else 'disabled'})") async def check_connection(self) -> bool: """ @@ -728,7 +741,6 @@ class YouTubeClient(DownloadSourcePlugin): loop = asyncio.get_event_loop() def _search(): - from config.settings import config_manager ydl_opts = { 'quiet': True, 'no_warnings': True, @@ -736,9 +748,7 @@ class YouTubeClient(DownloadSourcePlugin): 'default_search': 'ytsearch', 'user_agent': 'Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36', } - cookies_browser = config_manager.get('youtube.cookies_browser', '') - if cookies_browser: - ydl_opts['cookiesfrombrowser'] = (cookies_browser,) + ydl_opts.update(_resolve_cookie_opts()) search_query = self._escape_ytsearch_query(query) with yt_dlp.YoutubeDL(ydl_opts) as ydl: @@ -806,7 +816,6 @@ class YouTubeClient(DownloadSourcePlugin): loop = asyncio.get_event_loop() def _search(): - from config.settings import config_manager ydl_opts = { 'quiet': True, 'no_warnings': True, @@ -816,9 +825,7 @@ class YouTubeClient(DownloadSourcePlugin): } # Add cookie support for search (avoids bot detection) - cookies_browser = config_manager.get('youtube.cookies_browser', '') - if cookies_browser: - ydl_opts['cookiesfrombrowser'] = (cookies_browser,) + ydl_opts.update(_resolve_cookie_opts()) search_query = self._escape_ytsearch_query(query) with yt_dlp.YoutubeDL(ydl_opts) as ydl: @@ -1087,10 +1094,12 @@ class YouTubeClient(DownloadSourcePlugin): # On retry, try different strategies if attempt == 1: - # Drop browser cookies β€” authenticated sessions sometimes get restricted formats - if 'cookiesfrombrowser' in download_opts: - logger.info(f"Retry {attempt + 1}/{max_retries} without browser cookies") + # Drop cookies β€” authenticated sessions (browser store OR a + # pasted cookies.txt) sometimes get restricted formats. + if 'cookiesfrombrowser' in download_opts or 'cookiefile' in download_opts: + logger.info(f"Retry {attempt + 1}/{max_retries} without cookies") download_opts.pop('cookiesfrombrowser', None) + download_opts.pop('cookiefile', None) else: logger.info(f"Retry {attempt + 1}/{max_retries} with web_creator client") download_opts['extractor_args'] = { @@ -1100,6 +1109,7 @@ class YouTubeClient(DownloadSourcePlugin): logger.info(f"Retry {attempt + 1}/{max_retries} with 'best' format (video fallback)") download_opts['format'] = 'best' download_opts.pop('cookiesfrombrowser', None) + download_opts.pop('cookiefile', None) download_opts.pop('extractor_args', None) @@ -1160,8 +1170,6 @@ class YouTubeClient(DownloadSourcePlugin): Final file path if successful, None otherwise """ try: - from config.settings import config_manager - def _progress_hook(d): if progress_callback and d.get('status') == 'downloading': total = d.get('total_bytes') or d.get('total_bytes_estimate') or 0 @@ -1180,9 +1188,7 @@ class YouTubeClient(DownloadSourcePlugin): 'user_agent': 'Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36', } - cookies_browser = config_manager.get('youtube.cookies_browser', '') - if cookies_browser: - download_opts['cookiesfrombrowser'] = (cookies_browser,) + download_opts.update(_resolve_cookie_opts()) with yt_dlp.YoutubeDL(download_opts) as ydl: info = ydl.extract_info(video_url, download=True) diff --git a/database/music_database.py b/database/music_database.py index 8b0edcb7..b8702752 100644 --- a/database/music_database.py +++ b/database/music_database.py @@ -1000,6 +1000,10 @@ class MusicDatabase: self._init_manual_library_match_table() self._backfill_mirrored_track_source_ids() + # Self-heal the Unverified review queue: lift history rows stuck at + # 'unverified' whose file has since been verified (issue #934). Cheap, + # idempotent (only touches rows that need it), so it's safe every boot. + self.reconcile_unverified_history_from_tracks() def _backfill_mirrored_track_source_ids(self) -> int: """One-time, idempotent: assign a stable source_track_id to mirrored tracks @@ -1171,6 +1175,13 @@ class MusicDatabase: if track_cols and 'year' not in track_cols: cursor.execute("ALTER TABLE tracks ADD COLUMN year INTEGER") logger.info("Repaired missing year column on tracks table (#910)") + # #927 β€” multi-disc fix: the scan now writes a real disc_number, but the column + # was only ever added by a separate migration that doesn't run on fresh installs, + # so the new INSERT/UPDATE would hard-fail with "no column named disc_number". + # Same shape as the year repair above: additive, defaults to 1, ensured on every DB. + if track_cols and 'disc_number' not in track_cols: + cursor.execute("ALTER TABLE tracks ADD COLUMN disc_number INTEGER DEFAULT 1") + logger.info("Repaired missing disc_number column on tracks table (#927)") cursor.execute("PRAGMA table_info(albums)") album_cols = {c[1] for c in cursor.fetchall()} @@ -6635,6 +6646,19 @@ class MusicDatabase: track_id = str(track_obj.ratingKey) title = track_obj.title track_number = getattr(track_obj, 'trackNumber', None) + # Multi-disc: capture the disc number so multi-disc albums don't all + # collapse onto disc 1 (which mis-files disc-2+ tracks and flags them + # "missing"). Jellyfin/Navidrome wrappers set .discNumber; plexapi's Track + # exposes .parentIndex. Floor to >=1 β€” a missing/0 disc is disc 1. + _raw_disc = getattr(track_obj, 'discNumber', None) + if _raw_disc is None: + _raw_disc = getattr(track_obj, 'parentIndex', None) + try: + disc_number = int(_raw_disc) + if disc_number < 1: + disc_number = 1 + except (TypeError, ValueError): + disc_number = 1 duration = getattr(track_obj, 'duration', None) # Get file path and media info (Plex-specific, Jellyfin may not have these) @@ -6726,9 +6750,9 @@ class MusicDatabase: if is_new_track: cursor.execute(""" INSERT INTO tracks - (id, album_id, artist_id, title, track_number, duration, file_path, bitrate, file_size, server_source, track_artist, musicbrainz_recording_id, updated_at) - VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, CURRENT_TIMESTAMP) - """, (track_id, album_id, artist_id, title, track_number, duration, file_path, bitrate, file_size, server_source, track_artist, mbid)) + (id, album_id, artist_id, title, track_number, disc_number, duration, file_path, bitrate, file_size, server_source, track_artist, musicbrainz_recording_id, updated_at) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, CURRENT_TIMESTAMP) + """, (track_id, album_id, artist_id, title, track_number, disc_number, duration, file_path, bitrate, file_size, server_source, track_artist, mbid)) else: # Update server-provided fields only β€” preserves spotify_track_id, deezer_id, # isrc, bpm, and all other enrichment data. file_size uses @@ -6737,7 +6761,7 @@ class MusicDatabase: # an existing value. cursor.execute(""" UPDATE tracks - SET album_id = ?, artist_id = ?, title = ?, track_number = ?, + SET album_id = ?, artist_id = ?, title = ?, track_number = ?, disc_number = ?, duration = ?, file_path = ?, bitrate = ?, file_size = COALESCE(?, file_size), server_source = ?, @@ -6745,7 +6769,7 @@ class MusicDatabase: musicbrainz_recording_id = COALESCE(?, musicbrainz_recording_id), updated_at = CURRENT_TIMESTAMP WHERE id = ? - """, (album_id, artist_id, title, track_number, duration, file_path, bitrate, file_size, server_source, track_artist, mbid, track_id)) + """, (album_id, artist_id, title, track_number, disc_number, duration, file_path, bitrate, file_size, server_source, track_artist, mbid, track_id)) conn.commit() @@ -13141,6 +13165,73 @@ class MusicDatabase: logger.error(f"Error getting discovery pool stats: {e}") return {'matched': 0, 'failed': 0} + # Wing It Pool: two states on a mirrored track's extra_data. Both key off wing_it_fallback, + # which is set by the wing-it stub and SURVIVES a manual fix (update_mirrored_track_extra_data + # merges rather than replaces), so the only difference is the manual_match flag: + # needs attention : wing_it_fallback=true AND NOT manual_match (unverified guess) + # resolved : wing_it_fallback=true AND manual_match=true (user fixed it β€” incl. fixes + # made before this feature existed, since the flag was never wiped) + _WING_IT_ATTENTION = ("mpt.extra_data LIKE '%\"wing_it_fallback\": true%' " + "AND mpt.extra_data NOT LIKE '%\"manual_match\": true%'") + _WING_IT_RESOLVED = ("mpt.extra_data LIKE '%\"wing_it_fallback\": true%' " + "AND mpt.extra_data LIKE '%\"manual_match\": true%'") + + def get_wing_it_pool(self, profile_id: int = None, playlist_id: int = None, + resolved: bool = False) -> list: + """Get Wing It tracks β€” the unverified guesses (default) or the ones you've resolved. + + Wing-it tracks are persisted on extra_data with ``wing_it_fallback: true`` (a best-effort + stub when a track couldn't match a metadata source). They count as 'discovered', so the + Discovery Pool hides them β€” this is the only surface that lists them. ``resolved=True`` + returns the ones a manual match has since fixed (carrying the ``was_wing_it`` marker). + """ + try: + conn = self._get_connection() + cursor = conn.cursor() + where = self._WING_IT_RESOLVED if resolved else self._WING_IT_ATTENTION + query = f""" + SELECT mpt.id, mpt.track_name, mpt.artist_name, mpt.album_name, + mpt.playlist_id, mp.name as playlist_name, mpt.extra_data + FROM mirrored_playlist_tracks mpt + JOIN mirrored_playlists mp ON mpt.playlist_id = mp.id + WHERE {where} + """ + params = [] + if playlist_id: + query += " AND mpt.playlist_id = ?" + params.append(playlist_id) + elif profile_id: + query += " AND mp.profile_id = ?" + params.append(profile_id) + query += " ORDER BY mp.name, mpt.track_name" + cursor.execute(query, params) + return [dict(row) for row in cursor.fetchall()] + except Exception as e: + logger.error(f"Error getting wing it pool: {e}") + return [] + + def get_wing_it_pool_stats(self, profile_id: int = None) -> dict: + """Counts for both Wing It states: unverified (``wing_it``) + resolved (``matched``).""" + try: + conn = self._get_connection() + cursor = conn.cursor() + + def _count(where): + q = (f"SELECT COUNT(*) as cnt FROM mirrored_playlist_tracks mpt " + f"JOIN mirrored_playlists mp ON mpt.playlist_id = mp.id WHERE {where}") + params = [] + if profile_id: + q += " AND mp.profile_id = ?" + params.append(profile_id) + cursor.execute(q, params) + return cursor.fetchone()['cnt'] + + return {'wing_it': _count(self._WING_IT_ATTENTION), + 'matched': _count(self._WING_IT_RESOLVED)} + except Exception as e: + logger.error(f"Error getting wing it pool stats: {e}") + return {'wing_it': 0, 'matched': 0} + # ==================== Retag Tool Methods ==================== def add_retag_group(self, group_type: str, artist_name: str, album_name: str, @@ -13708,6 +13799,28 @@ class MusicDatabase: logger.debug(f"Error deleting history rows: {e}") return 0 + def clear_completed_download_history(self) -> int: + """Delete the persisted completed-download history shown on the Downloads + page (every event_type='download' row). This also clears the verification + review queue, since those unverified/force_imported rows ARE download-history + rows β€” that's intended: 'Clear Completed' empties the list. It only removes + HISTORY rows; the actual files and their `tracks` entries are untouched, so + nothing in the library is lost β€” only the 'needs verification' review flags. + Returns the number of rows removed.""" + conn = None + try: + conn = self._get_connection() + cursor = conn.cursor() + cursor.execute("DELETE FROM library_history WHERE event_type = 'download'") + conn.commit() + return cursor.rowcount + except Exception as e: + logger.error("Error clearing completed download history: %s", e) + return 0 + finally: + if conn: + conn.close() + def delete_track_by_file_path(self, file_path): """Delete a library track row whose stored path matches. Returns count.""" if not file_path: @@ -13770,6 +13883,107 @@ class MusicDatabase: logger.error("Error querying unverified library history: %s", e) return [] + def reconcile_unverified_history_from_tracks(self) -> int: + """Heal library_history rows stuck at 'unverified' whose underlying file + has since been confirmed in the tracks table (AcoustID scan PASS or a + human decision). Matches by exact path AND basename β€” the same physical + file keeps its filename across path-form differences (relative vs + absolute, library moved/reorganized, different mount), which is why an + exact-path-only heal left thousands of already-verified files showing as + Unverified (issue #934). + + A basename match is title-guarded: a shared track-number filename + ("01 - Intro.flac") must NOT heal a different song. When both the history + row and the candidate track carry a title they have to agree + (alphanumeric-lowercase) β€” the same guard the AcoustID matcher uses. When + a title is missing on either side we can't tell which file the basename + refers to, so we only heal if that basename is unambiguous (a single + verified candidate). An exact-path match needs no guard. + + Upgrade-only and non-destructive: it only lifts 'unverified' rows to the + confirmed status, never downgrades and never deletes. Returns the number + of rows healed. Genuinely-unverified rows and orphans (no matching + track) are left untouched. + """ + healed = 0 + conn = None + try: + conn = self._get_connection() + cursor = conn.cursor() + + def _norm(value): + return ''.join(ch for ch in str(value or '').lower() if ch.isalnum()) + + # Load the stuck rows first. Cheap early-out when nothing is stuck β€” + # and their paths/basenames scope the tracks scan below, so the + # lookup dicts stay proportional to the (small) review queue instead + # of the whole library. + cursor.execute( + "SELECT id, file_path, title FROM library_history " + "WHERE verification_status = 'unverified' " + "AND file_path IS NOT NULL AND file_path != ''") + stuck_rows = cursor.fetchall() + if not stuck_rows: + return 0 + needed_paths = {fp for _, fp, _ in stuck_rows if fp} + needed_bases = {os.path.basename(fp) for _, fp, _ in stuck_rows if fp} + + rank = {'verified': 1, 'human_verified': 2} + by_path = {} # exact path -> status (unambiguous; no title guard) + by_base = {} # basename -> list of (norm_title, status) + cursor.execute( + "SELECT file_path, verification_status, title FROM tracks " + "WHERE verification_status IN ('verified', 'human_verified') " + "AND file_path IS NOT NULL AND file_path != ''") + for fp, st, ttitle in cursor.fetchall(): + if not fp: + continue + base = os.path.basename(fp) + # Skip verified tracks that can't possibly match a queued row. + if fp not in needed_paths and base not in needed_bases: + continue + if rank.get(st, 0) >= rank.get(by_path.get(fp), 0): + by_path[fp] = st + if base: + by_base.setdefault(base, []).append((_norm(ttitle), st)) + + updates = [] + for rid, fp, rtitle in stuck_rows: + target = by_path.get(fp) + if not target: + want = _norm(rtitle) + candidates = by_base.get(os.path.basename(fp or ''), ()) + best = 0 + for ttitle, st in candidates: + if want and ttitle: + # Both titled: must agree. + if want != ttitle: + continue + elif len(candidates) > 1: + # Title missing on a side AND the basename collides + # across verified files β€” can't tell which one this + # row is, so don't risk healing the wrong song. + continue + if rank.get(st, 0) >= best: + best = rank.get(st, 0) + target = st + if target: + updates.append((target, rid)) + for status, rid in updates: + cursor.execute( + "UPDATE library_history SET verification_status = ? WHERE id = ?", + (status, rid)) + healed += 1 + if healed: + conn.commit() + logger.info("Reconciled %d unverified history rows from tracks truth", healed) + except Exception as e: + logger.error("Error reconciling unverified history: %s", e) + finally: + if conn: + conn.close() + return healed + def get_library_history_stats(self): """Return counts per event_type and per download_source.""" try: diff --git a/pr_description.md b/pr_description.md index 3b270a6e..19659f76 100644 --- a/pr_description.md +++ b/pr_description.md @@ -1,38 +1,44 @@ -# soulsync 2.7.8 β€” `dev` β†’ `main` +# soulsync 2.8.0 β€” `dev` β†’ `main` -a feature patch on top of 2.7.7 β€” playlists can now be put back in order on the server, you can re-wishlist a missed track straight from sync history, plus a couple of reported fixes. +mostly a quality + reliability release. the headline is a big cleanup of the **Unverified review queue** (it stops inflating and self-heals), a new **Preview Clip Cleanup** tool, smarter **Album Completeness** for split/fragmented albums, and a real pass on **dashboard performance** (especially Firefox/Zen). plus a pile of reported fixes. --- ## what's new -### align playlists β€” server order, not just contents -the server-playlist editor only ever cared about *which* tracks were on the server, never their order β€” and it rendered the server column in the source's order, so a playlist with the right tracks in the wrong sequence read as "in sync" when it wasn't. now it tells the truth: -- an **"out of order"** badge appears when the tracks match but the sequence differs (relative order, so missing/extra tracks don't false-flag it), and a **read-only view** shows the server's *actual* order with cover art. -- a new **"Align playlists"** action reorders the server playlist to match the source β€” **Plex** (in-place via moveItem), **Navidrome** (ordered rewrite), and **Jellyfin** (Move endpoint), all of which preserve the playlist's identity/poster. two choices for server-only extras: **mirror source** (drop them) or **keep extras** (park them at the end). it's order-only β€” it never adds the missing tracks (that's a normal sync's job) and never touches metadata, just reshuffles ids already on the server. +### Preview Clip Cleanup (new Tools job) +the HiFi source sometimes hands back a ~30-second **preview clip** instead of the full song, and it lands looking like a normal track. the new job scans your short tracks, checks how long each one *should* be from its metadata source, and flags the previews. approve a finding and it deletes the clip, drops it from the library, and re-wishlists the full version. each finding has a **β–Ά Play** button + a file-length-vs-real-length readout so you can confirm it's busted before approving. conservative by design β€” genuine short tracks and anything it can't verify are left alone. -### re-add to wishlist from sync history -in the dashboard's **Recent Syncs β†’ details**, the "β†’ Wishlist" status on an unmatched track is now a button β€” click it to re-add that exact track to the wishlist with the **same context the sync used** (source playlist, cover art, everything), so it's indistinguishable from the original auto-add. the re-add and the live sync now build the *identical* payload from one shared path, so the cover and album/single classification carry through. wing-it fallback stubs (tracks that couldn't be resolved to real metadata) are correctly shown as **"Unmatched"** and aren't re-addable β€” matching what the sync itself does. +### the Unverified queue stops inflating + self-heals (#934) +big one for anyone who saw thousands of "unverified" rows. the AcoustID scan was creating a fresh history row every run and leaving already-verified files stuck as "unverified" (a frozen import-time path that stopped matching once the file moved). now: +- scans no longer duplicate rows and heal on the spot, and +- a one-time **reconcile** on startup clears the existing backlog from your library's truth β€” no re-scan needed, including human-verified files a scan skips, and +- a **🧹 Clean orphaned** button removes dead rows whose file is genuinely gone (with a safety gate that refuses to run if your library looks offline). +the Unverified review rows also got the nicer Quarantine-style cards (artwork, inline details). *(thanks @nick2000713 for #938.)* -### fixes -- **import search said "Deezer" for Spotify Free users (#922)** β€” manual album-import told no-auth Spotify users that Deezer was their primary source. the functional source legitimately downgrades to a working fallback (the free path has no album-name search), but the *label* should name what you configured. now it reads "Spotify." -- **iTunes albums >50 tracks could still truncate (#918 follow-up)** β€” the limit=200 fix only helped fresh fetches; albums cached at 50 before the fix kept serving 50 from the persistent cache. now a cached tracklist shorter than the album's known track count self-heals on next load. +### Album Completeness handles split albums (#936, #929, #931) +a physical album split across multiple library rows used to show every fragment as falsely "incomplete." it now groups the validated fragments into one logical album and emits a single correct finding β€” grouping by a shared id **and** validating at the track level, so unrelated rows never get fused. also recognizes MusicBrainz as a readable album source. *(thanks @ragnarlotus.)* -### under the hood -- `.gitignore` now covers **all** `database/*.db` (+ wal/shm/backup), not just `music_library` β€” so the video db and any future db can't be committed by accident. +### Clear Completed is back on the Downloads page +since completed downloads now persist across restart, the **Clear Completed** button had gone missing for them. it's back β€” it clears the live list *and* the persisted history so the page actually empties and stays empty (your files are untouched). --- -## a brief recap of what came before -2.7.7 was a fix-heavy patch β€” the metadata-parity fix so downloads tag + path right without a manual reorganize (#915), the listening-recs foundation (#913), jellyfin atomic writes, and a big reported-issue sweep (#905/#908–#912/#914/#916–#918). 2.7.6 exported playlists TO listenbrainz + youtube liked-music sync; 2.7.5 matching & artwork accuracy; 2.7.4 re-identify; 2.7.3 the Quality Upgrade Finder; 2.7.2 playlist-folder mirroring; 2.7.1 download verification; 2.7.0 made multi-user real. +## fixes + +- **pasted YouTube cookies threw `unsupported browser: "custom"` (Docker)** β€” the client passed the "Paste cookies.txt" mode through as a browser name instead of using the cookies file. now it loads the pasted `cookies.txt` correctly β€” the only auth path that works on a headless/Docker box. *(thanks HellRa1SeR.)* +- **longer remasters quarantined as "truncated" (#937)** β€” the duration check was symmetric, so a remaster running a few seconds *longer* than the metadata got rejected like a truncated download. it's asymmetric now: short files stay strict, longer versions get room. *(thanks @diegocade1.)* +- **"Add to Wishlist" from an artist discography was painfully slow** β€” ~15–30s *per track* on a large library, because the per-track library-ownership check fell through to a full-table fuzzy scan. it now matches in-memory against the artist's tracks once β€” effectively instant. +- **wishlist art was blank for re-downloads / preview re-fetches** β€” library-sourced items stored a relative media-server path that doesn't render in a browser. they're normalized on read now, so album + artist art show up (fixes already-saved items too). +- **watchlist didn't record automatic scans (#933)** + **watchlist fused different editions of an album as one.** +- **manual search:** a pasted Qobuz/Tidal track now floats to the top of results (#932). +- **Popular Picks came up empty on Deezer** β€” a popularity-threshold scale mismatch. --- -## tests -additive + scoped β€” the new write paths are their own routes that don't touch the normal sync. new seam/regression suites for the order-status detection (incl. the reported "moved to #2" case + missing/extra false-flag guards), the pure align-rewrite planner (mirror vs keep-extras, never-injects-a-foreign-track, stale-data rejection), the sync re-add payload (a direct parity assertion that the re-add == the live-sync payload, plus the wing-it skip), and the `get_primary_source_label` fix (#922). iTunes self-heal proven against the real persistent-cache shape. relevant suites green; `ruff check` clean. +## performance + UI -## post-merge -- [ ] tag `v2.7.8` on `main` -- [ ] docker-publish with `version_tag: 2.7.8` -- [ ] discord announce (auto-fired by the workflow) -- [ ] reply on #922 and the #918 follow-up +- **dashboard GPU usage, especially on Firefox/Zen (#935)** β€” frosted-glass blur, cursor-glow blobs, and the worker-orb animation were repainting every frame. trimmed the worst offenders, made the orb loop hold a steady framerate on Firefox instead of dropping to ~1fps, and set **Background Particles OFF by default**. the dashboard system-memory tile now also shows SoulSync's own RAM. +- **bounded memory growth (#802)** β€” browsing every page used to climb RSS into the GBs (plexapi's XML trees deferring GC) and could lock the app up. a lightweight sweeper now collects + hands memory back to the OS as it grows, so it sawtooths and settles instead of climbing. + +--- diff --git a/tests/downloads/test_track_link.py b/tests/downloads/test_track_link.py index 39dfe8a1..06cbc1bb 100644 --- a/tests/downloads/test_track_link.py +++ b/tests/downloads/test_track_link.py @@ -78,3 +78,47 @@ def test_payload_non_dict_or_empty(): assert q('tidal', None) is None assert q('tidal', {}) is None assert q('qobuz', 'garbage') is None + + +# ── bubble the pasted-link track to the top (#932) ── + +from types import SimpleNamespace + +from core.downloads.track_link import linked_track_id, bubble_linked_track_first + + +def _result(track_id=None): + """Mimic a TrackResult: NO top-level `id`, the source id lives in + _source_metadata['track_id'] (or absent entirely).""" + meta = {'source': 'qobuz', 'track_id': track_id} if track_id is not None else None + return SimpleNamespace(_source_metadata=meta, title='t') + + +def test_linked_track_id_reads_source_metadata(): + assert linked_track_id(_result('296427754')) == '296427754' + + +def test_linked_track_id_empty_when_absent(): + # the exact #932 trap: there is no top-level `id`, so getattr(t,'id') would miss. + r = _result() + assert not hasattr(r, 'id') + assert linked_track_id(r) == '' + + +def test_bubble_floats_exact_track_to_top(): + fuzzy_a, exact, fuzzy_b = _result('111'), _result('296427754'), _result('222') + out = bubble_linked_track_first([fuzzy_a, exact, fuzzy_b], '296427754') + assert out[0] is exact # exact track surfaced first + assert out[1:] == [fuzzy_a, fuzzy_b] # stable order for the rest + + +def test_bubble_handles_int_vs_str_id(): + out = bubble_linked_track_first([_result('999'), _result('296427754')], 296427754) + assert linked_track_id(out[0]) == '296427754' + + +def test_bubble_noop_when_nothing_matches_or_empty(): + a, b = _result('1'), _result('2') + assert bubble_linked_track_first([a, b], '296427754') == [a, b] # unchanged + assert bubble_linked_track_first([], '296427754') == [] + assert bubble_linked_track_first([a, b], '') == [a, b] diff --git a/tests/imports/test_file_integrity.py b/tests/imports/test_file_integrity.py index 21a2f810..cb75e3aa 100644 --- a/tests/imports/test_file_integrity.py +++ b/tests/imports/test_file_integrity.py @@ -201,6 +201,60 @@ def test_rejects_truncated_file(tmp_path: Path) -> None: assert result.checks["length_drift_s"] > 3.0 +# ── #937: a file that runs LONGER than expected is a version/master difference, not +# truncation β€” it gets more leeway, while SHORTER files stay tight. ── + +def test_accepts_longer_master_beyond_short_tolerance(tmp_path: Path) -> None: + """The reported case (A-Ha remaster): file runs ~3.5s LONGER than the metadata. + Past the 3s short-tolerance but a remaster, not a bad download β€” must pass.""" + f = tmp_path / "remaster.wav" + _write_minimal_wav(f, duration_s=9.0) # 9.0s file vs 5.5s expected β†’ +3.5s longer + + result = file_integrity.check_audio_integrity(str(f), expected_duration_ms=5500) + + assert result.ok is True + assert result.checks["length_check"] == "passed" + assert result.checks["effective_tolerance_s"] == pytest.approx(15.0) + + +def test_shorter_file_still_tight_after_longer_loosening(tmp_path: Path) -> None: + """Loosening the LONGER direction must not loosen truncation detection β€” a file + 3.5s SHORTER than expected is still rejected at the 3s tolerance.""" + f = tmp_path / "short.wav" + _write_minimal_wav(f, duration_s=5.5) # 5.5s vs 9.0s expected β†’ -3.5s shorter + + result = file_integrity.check_audio_integrity(str(f), expected_duration_ms=9000) + + assert result.ok is False + assert "truncated" in result.reason.lower() + assert result.checks["effective_tolerance_s"] == pytest.approx(3.0) + + +def test_wildly_longer_file_still_rejected(tmp_path: Path) -> None: + """A different/wrong song that happens to run long is still caught β€” +25s blows + past even the 15s longer-tolerance.""" + f = tmp_path / "wronglong.wav" + _write_minimal_wav(f, duration_s=30.0) # 30s vs 5s expected β†’ +25s + + result = file_integrity.check_audio_integrity(str(f), expected_duration_ms=5000) + + assert result.ok is False + assert "longer than expected" in result.reason.lower() + + +def test_user_pinned_tolerance_is_symmetric(tmp_path: Path) -> None: + """An explicit user tolerance is honoured in BOTH directions β€” the longer-direction + loosening only applies to the auto default, not a value the user pinned.""" + f = tmp_path / "long.wav" + _write_minimal_wav(f, duration_s=9.0) # +4s longer + + result = file_integrity.check_audio_integrity( + str(f), expected_duration_ms=5000, length_tolerance_s=2.0) + + assert result.ok is False + assert result.checks["effective_tolerance_s"] == pytest.approx(2.0) + + def test_rejects_wrong_file_substituted(tmp_path: Path) -> None: """A 10-second clip masquerading as a 3-minute album track. slskd matched on a similar filename but the actual content is a snippet.""" diff --git a/tests/metadata/test_discography_filters.py b/tests/metadata/test_discography_filters.py index 8c297d05..cb6ba194 100644 --- a/tests/metadata/test_discography_filters.py +++ b/tests/metadata/test_discography_filters.py @@ -301,6 +301,37 @@ class TestTrackAlreadyOwned: track_already_owned(db, 'Track', 'Artist', 'Album X', 'plex') assert db.calls[0]['album'] == 'Album X' + def test_candidate_tracks_threaded_to_batched_path(self): + """The discography endpoint pre-fetches the artist's owned tracks once + and passes them so check_track_exists scores in-memory instead of firing + per-track fuzzy SQL β€” the fix for ~15-30s/track on a large library.""" + owned = [SimpleNamespace(title='Owned')] + db = _FakeDB((object(), 0.9)) + track_already_owned( + db, 'Owned', 'Artist', 'Album', 'plex', candidate_tracks=owned, + ) + # The pre-fetched candidates must reach check_track_exists verbatim. + assert db.calls[0]['candidate_tracks'] is owned + + def test_empty_candidate_list_still_uses_batched_path(self): + """Owns-nothing case: an EMPTY list (not None) must be forwarded so the + check takes the fast in-memory path (scores against zero candidates β†’ + instant 'not owned') instead of falling back to the slow per-track SQL.""" + db = _FakeDB((None, 0.0)) + result = track_already_owned( + db, 'Anything', 'Artist', 'Album', 'plex', candidate_tracks=[], + ) + assert result is False + # [] is forwarded (not coerced to None) β€” that's what keeps it fast. + assert db.calls[0]['candidate_tracks'] == [] + + def test_default_omits_candidate_tracks_for_legacy_callers(self): + """Callers that don't pre-fetch get None β†’ check_track_exists keeps its + original per-track-SQL behaviour. No other caller is forced to change.""" + db = _FakeDB((object(), 0.9)) + track_already_owned(db, 'Track', 'Artist', 'Album', 'plex') + assert db.calls[0]['candidate_tracks'] is None + def test_passes_server_source_to_check(self): """Active media server scopes the lookup so the skip check only fires on tracks the user can actually see in their diff --git a/tests/repair_jobs/test_short_preview_track.py b/tests/repair_jobs/test_short_preview_track.py new file mode 100644 index 00000000..3d05eb00 --- /dev/null +++ b/tests/repair_jobs/test_short_preview_track.py @@ -0,0 +1,191 @@ +"""Preview-clip cleanup job (#937-adjacent): flag ~30s preview clips whose source says the +real track is much longer, then on approval delete the file + drop the row + re-wishlist.""" + +from __future__ import annotations + +from pathlib import Path + +import pytest + +from core.repair_jobs.base import JobContext +from core.repair_jobs.short_preview_track import ShortPreviewTrackJob +from core.repair_worker import RepairWorker +from database.music_database import MusicDatabase + + +def _seed(db: MusicDatabase): + conn = db._get_connection() + conn.execute("INSERT OR IGNORE INTO artists (id, name) VALUES ('ar1', 'A-ha')") + conn.execute("INSERT INTO albums (id, artist_id, title) VALUES ('al1', 'ar1', 'Hunting High and Low')") + conn.commit() + conn.close() + + +def _track(db, tid: int, duration_ms, path, spotify_id=None): + # tid is an INTEGER id, exactly like production (tracks.id is INTEGER PRIMARY KEY) β€” so the + # test exercises the real round-trip: the finding stores str(id) and the fix queries WHERE id=?. + conn = db._get_connection() + conn.execute( + "INSERT INTO tracks (id, artist_id, album_id, title, duration, file_path, spotify_track_id) " + "VALUES (?, 'ar1', 'al1', ?, ?, ?, ?)", + (tid, f"Track {tid}", duration_ms, path, spotify_id), + ) + conn.commit() + conn.close() + + +class _FakeSpotify: + """get_track_details(id) -> {'duration_ms': N}. 'sp_long' is a full song; else short.""" + def get_track_details(self, track_id, **_): + return {'duration_ms': 200_000} if track_id == 'sp_long' else {'duration_ms': 28_000} + + +def _ctx(db, findings, spotify=None): + return JobContext( + db=db, transfer_folder='/tmp', config_manager=None, + spotify_client=spotify, + create_finding=lambda **kw: findings.append(kw) or True, + should_stop=lambda: False, is_paused=lambda: False, + ) + + +# ── scan ── + +def test_scan_flags_preview_skips_genuine_short_and_unverifiable(tmp_path: Path): + db = MusicDatabase(str(tmp_path / 'm.db')) + _seed(db) + _track(db, 1, 28_000, '/m/p.flac', spotify_id='sp_long') # id 1: 28s file, source 200s β†’ FLAG + _track(db, 2, 28_000, '/m/i.flac', spotify_id='sp_short') # id 2: 28s file, source 28s β†’ skip (genuine) + _track(db, 3, 28_000, '/m/m.flac', spotify_id=None) # id 3: 28s, no source id β†’ skip (unverifiable) + _track(db, 4, 200_000, '/m/l.flac', spotify_id='sp_long') # id 4: 200s β†’ not scanned (>30s) + + findings = [] + result = ShortPreviewTrackJob().scan(_ctx(db, findings, _FakeSpotify())) + + assert len(findings) == 1 + f = findings[0] + assert f['finding_type'] == 'short_preview_track' + assert f['entity_id'] == '1' # str(int id), as create_finding stores it + assert f['entity_type'] == 'track' + assert f['details']['expected_duration_s'] == pytest.approx(200.0) + assert result.findings_created == 1 + assert result.scanned == 3 # the 200s track is excluded by the query, not scanned + assert result.skipped == 2 # skit + noid + + +def test_scan_creates_no_finding_when_source_agrees_short(tmp_path: Path): + db = MusicDatabase(str(tmp_path / 'm.db')) + _seed(db) + _track(db, 1, 28_000, '/m/i.flac', spotify_id='sp_short') # source also says 28s + findings = [] + ShortPreviewTrackJob().scan(_ctx(db, findings, _FakeSpotify())) + assert findings == [] + + +def test_estimate_scope_counts_short_tracks(tmp_path: Path): + db = MusicDatabase(str(tmp_path / 'm.db')) + _seed(db) + _track(db, 1, 28_000, '/m/a.flac', spotify_id='sp_long') + _track(db, 2, 10_000, '/m/b.flac', spotify_id='sp_short') + _track(db, 3, 200_000, '/m/c.flac', spotify_id='sp_long') # >30s, excluded + assert ShortPreviewTrackJob().estimate_scope(_ctx(db, [], _FakeSpotify())) == 2 + + +# ── fix (approval) ── + +def test_fix_deletes_file_removes_row_and_wishlists(tmp_path: Path): + db = MusicDatabase(str(tmp_path / 'm.db')) + _seed(db) + preview = tmp_path / 'preview.flac' + preview.write_bytes(b'fake audio bytes') + _track(db, 1, 28_000, str(preview), spotify_id='sp1') + + captured = {} + db.add_to_wishlist = lambda spotify_track_data, **kw: captured.update( + {'data': spotify_track_data, 'kw': kw}) or True + + w = RepairWorker.__new__(RepairWorker) + w.db = db + w.transfer_folder = str(tmp_path) + w._config_manager = None + + res = w._fix_short_preview_track( + 'track', '1', str(preview), # entity_id is the string the finding stored + {'expected_duration_s': 225.0, 'original_path': str(preview)}) + + assert res['success'] is True + assert not preview.exists() # preview file deleted + assert captured['data']['name'] == 'Track 1' # re-wishlisted with payload + assert captured['data']['duration_ms'] == 225_000 # uses the real (expected) length + assert captured['kw'].get('source_type') == 'redownload' + conn = db._get_connection() + remaining = conn.execute("SELECT COUNT(*) FROM tracks WHERE id=1").fetchone()[0] + conn.close() + assert remaining == 0 # DB row dropped β†’ track missing again + + +# ── album art capture (so the re-wishlisted item isn't art-less) ── + +def test_scan_captures_source_album_art_into_finding(tmp_path: Path): + """The duration lookup's raw_data carries the source CDN album art β€” capture it so the + re-wishlist isn't art-less when the library thumb is empty (the reported bug).""" + db = MusicDatabase(str(tmp_path / 'm.db')) + _seed(db) + _track(db, 1, 28_000, '/m/p.flac', spotify_id='sp_long') + + class _SpWithArt: + def get_track_details(self, track_id, **_): + return {'duration_ms': 200_000, + 'raw_data': {'album': {'images': [{'url': 'https://cdn/cover.jpg'}]}}} + + findings = [] + ShortPreviewTrackJob().scan(_ctx(db, findings, _SpWithArt())) + assert len(findings) == 1 + assert findings[0]['details']['album_thumb_url'] == 'https://cdn/cover.jpg' + + +def test_art_from_itunes_artwork_is_upscaled(): + from core.repair_jobs.short_preview_track import _art_from_details + d = {'raw_data': {'artworkUrl100': 'https://is1.mzstatic.com/a/100x100bb.jpg'}} + assert _art_from_details(d) == 'https://is1.mzstatic.com/a/600x600bb.jpg' + + +def test_fix_uses_finding_art_for_wishlist_payload(tmp_path: Path): + db = MusicDatabase(str(tmp_path / 'm.db')) + _seed(db) + preview = tmp_path / 'preview.flac' + preview.write_bytes(b'fake audio') + _track(db, 1, 28_000, str(preview), spotify_id='sp1') + + captured = {} + db.add_to_wishlist = lambda spotify_track_data, **kw: captured.update( + {'data': spotify_track_data}) or True + + w = RepairWorker.__new__(RepairWorker) + w.db = db + w.transfer_folder = str(tmp_path) + w._config_manager = None + + w._fix_short_preview_track('track', '1', str(preview), + {'expected_duration_s': 200.0, + 'album_thumb_url': 'https://cdn/art.jpg'}) + assert captured['data']['album']['images'] == [{'url': 'https://cdn/art.jpg'}] + + +def test_fix_missing_file_still_wishlists_and_drops_row(tmp_path: Path): + """If the preview file is already gone, still re-wishlist + drop the row (idempotent-ish).""" + db = MusicDatabase(str(tmp_path / 'm.db')) + _seed(db) + _track(db, 1, 28_000, str(tmp_path / 'gone.flac'), spotify_id='sp2') + db.add_to_wishlist = lambda spotify_track_data, **kw: True + + w = RepairWorker.__new__(RepairWorker) + w.db = db + w.transfer_folder = str(tmp_path) + w._config_manager = None + + res = w._fix_short_preview_track('track', '1', str(tmp_path / 'gone.flac'), {}) + assert res['success'] is True + conn = db._get_connection() + assert conn.execute("SELECT COUNT(*) FROM tracks WHERE id=1").fetchone()[0] == 0 + conn.close() diff --git a/tests/test_acoustid_history_heal.py b/tests/test_acoustid_history_heal.py new file mode 100644 index 00000000..02108e10 --- /dev/null +++ b/tests/test_acoustid_history_heal.py @@ -0,0 +1,95 @@ +"""#934: the AcoustID scanner heals the download-history row when the file moved, +instead of leaving it stuck 'unverified' and inserting duplicate scan rows. + +Seeds the exact bug shape (a real download row at the OLD import path + a synthetic +'acoustid_scan' duplicate at the NEW library path) and drives the real _persist_status, +asserting it collapses to one correct, verified row. +""" + +from __future__ import annotations + +import types + +import pytest + +from core.repair_jobs.acoustid_scanner import AcoustIDScannerJob +from database.music_database import MusicDatabase + + +@pytest.fixture() +def db(tmp_path): + return MusicDatabase(str(tmp_path / 'm.db')) + + +def _scanner(): + # _persist_status uses only `context`, never instance state β€” bypass __init__. + return AcoustIDScannerJob.__new__(AcoustIDScannerJob) + + +def _rows(db): + with db._get_connection() as conn: + return [dict(r) for r in conn.execute( + "SELECT file_path, download_source, verification_status FROM library_history")] + + +OLD = '/downloads/transfer/Artist/01 - Song.flac' # frozen import path in history +NEW = '/music/Artist/Album/01 - Song.flac' # where the file lives now (tracks path) + + +def test_verify_heals_drifted_row_and_drops_synthetic_dup(db): + # the bug's leftover state: a stuck real row + a synthetic scan dup for the same song. + db.add_library_history_entry('download', 'Song', file_path=OLD, + download_source='soulseek', verification_status='unverified') + db.add_library_history_entry('download', 'Song', file_path=NEW, + download_source='acoustid_scan', verification_status='unverified') + + _scanner()._persist_status( + types.SimpleNamespace(db=db), track_id='t1', fpath=NEW, db_path=NEW, + status='verified', write_tag=False, expected={'title': 'Song'}) + + rows = _rows(db) + assert len(rows) == 1 # synthetic dup deleted, no new insert + assert rows[0]['download_source'] == 'soulseek' # the REAL row survived + assert rows[0]['verification_status'] == 'verified' + assert rows[0]['file_path'] == NEW # path healed to current location + + +def test_idempotent_no_growth_on_rescan(db): + db.add_library_history_entry('download', 'Song', file_path=OLD, + download_source='soulseek', verification_status='unverified') + ctx = types.SimpleNamespace(db=db) + for _ in range(3): + _scanner()._persist_status(ctx, track_id='t1', fpath=NEW, db_path=NEW, + status='verified', write_tag=False, expected={'title': 'Song'}) + rows = _rows(db) + assert len(rows) == 1 and rows[0]['verification_status'] == 'verified' + + +def test_unknown_file_inserts_one_row_then_dedups(db): + # a file SoulSync never downloaded β†’ first scan inserts one review-queue row... + ctx = types.SimpleNamespace(db=db) + _scanner()._persist_status(ctx, track_id='t1', fpath=NEW, db_path=NEW, + status='unverified', write_tag=False, + expected={'title': 'Song', 'artist': 'Artist'}) + assert len(_rows(db)) == 1 + # ...and a rescan matches it (no duplicate). + _scanner()._persist_status(ctx, track_id='t1', fpath=NEW, db_path=NEW, + status='unverified', write_tag=False, + expected={'title': 'Song', 'artist': 'Artist'}) + rows = _rows(db) + assert len(rows) == 1 and rows[0]['download_source'] == 'acoustid_scan' + + +def test_does_not_heal_wrong_song_with_same_filename(db): + # different song, same filename, different title β†’ must stay untouched (no false heal). + db.add_library_history_entry('download', 'A Different Song', file_path='/other/01 - Song.flac', + download_source='soulseek', verification_status='verified') + _scanner()._persist_status( + types.SimpleNamespace(db=db), track_id='t1', fpath=NEW, db_path=NEW, + status='unverified', write_tag=False, expected={'title': 'Song'}) + rows = _rows(db) + # the unrelated row is untouched, and the unknown file got its own new row. + paths = {r['file_path'] for r in rows} + assert '/other/01 - Song.flac' in paths and NEW in paths + other = next(r for r in rows if r['file_path'] == '/other/01 - Song.flac') + assert other['verification_status'] == 'verified' # not corrupted diff --git a/tests/test_acoustid_scanner.py b/tests/test_acoustid_scanner.py index 1c83cac6..7057c968 100644 --- a/tests/test_acoustid_scanner.py +++ b/tests/test_acoustid_scanner.py @@ -4,8 +4,9 @@ from core.repair_jobs.acoustid_scanner import AcoustIDScannerJob class _FakeCursor: - def __init__(self, rows): + def __init__(self, rows, lib_rows=None): self._rows = rows + self._lib_rows = lib_rows or [] self.executed = [] def execute(self, query, params=None): @@ -13,6 +14,10 @@ class _FakeCursor: return self def fetchall(self): + # The #934 history-match SELECT gets its own (id, file_path, title, source) + # rows; the tracks scan query gets the track rows. + if self.executed and 'FROM library_history' in self.executed[-1][0]: + return self._lib_rows return self._rows def fetchone(self): @@ -20,8 +25,8 @@ class _FakeCursor: class _FakeConnection: - def __init__(self, rows): - self._cursor = _FakeCursor(rows) + def __init__(self, rows, lib_rows=None): + self._cursor = _FakeCursor(rows, lib_rows) def cursor(self): return self._cursor @@ -107,12 +112,13 @@ def test_scan_handles_mixed_track_id_types(monkeypatch): # and finds the primary artist at 100%, suppressing the false flag. -def _make_finding_capturing_context(track_row, captured): +def _make_finding_capturing_context(track_row, captured, lib_rows=None): """Context that captures any create_finding calls into the `captured` list. Tests assert against this list to verify whether the scanner created a finding (false positive) or correctly - skipped (multi-value match resolved).""" - conn = _FakeConnection([track_row]) + skipped (multi-value match resolved). ``lib_rows`` seeds the + library_history match SELECT (#934).""" + conn = _FakeConnection([track_row], lib_rows) config_manager = SimpleNamespace( get=lambda key, default=None: default, set=lambda *args, **kwargs: None, @@ -887,9 +893,10 @@ def test_human_verified_files_are_never_scanned(monkeypatch): # --------------------------------------------------------------------------- -def _run_persistence_scan(monkeypatch, *, file_status, aid_artist, expected_artist): +def _run_persistence_scan(monkeypatch, *, file_status, aid_artist, expected_artist, lib_rows=None): """Drive one _scan_file call and return (status_updates, tag_writes) where - status_updates is the list of (query, params) UPDATEs the scanner ran.""" + status_updates is the list of (query, params) UPDATEs the scanner ran. + ``lib_rows`` seeds the library_history match SELECT (#934).""" import core.repair_jobs.acoustid_scanner as scanner_mod monkeypatch.setattr(scanner_mod, "_resolve_expected_artist_aliases", lambda name: [], raising=False) @@ -905,7 +912,7 @@ def _run_persistence_scan(monkeypatch, *, file_status, aid_artist, expected_arti context = _make_finding_capturing_context( track_row=("9", "Call Your Name", expected_artist, "/music/cyn.flac", 1, "Album", None, None), - captured=captured) + captured=captured, lib_rows=lib_rows) fake = SimpleNamespace(fingerprint_and_lookup=lambda f: { 'best_score': 0.97, 'recordings': [{'title': 'Call Your Name', 'artist': aid_artist}]}) @@ -920,29 +927,32 @@ def _run_persistence_scan(monkeypatch, *, file_status, aid_artist, expected_arti def test_scan_pass_backfills_verified_status(monkeypatch): - # Untagged file + clean fingerprint PASS β†’ the scan backfills 'verified' - # into the tag, the tracks row AND library_history (review-queue feed). + # Clean fingerprint PASS β†’ the scan backfills 'verified' into the tag, the tracks + # row AND the file's library_history row. The history row's path drifted since + # download (file moved), so the scan heals it by id (#934) rather than missing it. updates, tag_writes, captured = _run_persistence_scan( monkeypatch, file_status=None, - aid_artist='Sawano Hiroyuki', expected_artist='Sawano Hiroyuki') + aid_artist='Sawano Hiroyuki', expected_artist='Sawano Hiroyuki', + lib_rows=[(1, '/downloads/old/cyn.flac', 'Call Your Name', 'soulseek')]) assert captured == [] assert tag_writes == [('/music/cyn.flac', 'verified')] assert any('tracks' in q and p == ('verified', '9') for q, p in updates) - assert any('library_history' in q and p == ('verified', '/music/cyn.flac') + # healed by id, status set + path refreshed to the file's current location. + assert any('library_history' in q and p == ('verified', '/music/cyn.flac', 1) for q, p in updates) def test_scan_skip_marks_untagged_file_unverified(monkeypatch): # Title matches but the artist is ambiguous (cover/collab band?) β†’ SKIP. - # An untagged file gets 'unverified' so it surfaces in the Downloads-page - # review queue instead of silently passing or being deleted. + # An untagged file SoulSync never downloaded (no history row) gets a fresh + # 'unverified' row INSERTed so it surfaces in the Downloads-page review queue. updates, tag_writes, captured = _run_persistence_scan( monkeypatch, file_status=None, - aid_artist='Mantilla', expected_artist='Metallica') + aid_artist='Mantilla', expected_artist='Metallica') # no lib_rows β†’ no existing row assert captured == [] assert tag_writes == [('/music/cyn.flac', 'unverified')] assert any('tracks' in q and p == ('unverified', '9') for q, p in updates) - assert any('library_history' in q and p == ('unverified', '/music/cyn.flac') + assert any('INSERT INTO library_history' in q and p[-1] == 'unverified' for q, p in updates) diff --git a/tests/test_album_completeness_fragmented_rows.py b/tests/test_album_completeness_fragmented_rows.py new file mode 100644 index 00000000..b12b9c3b --- /dev/null +++ b/tests/test_album_completeness_fragmented_rows.py @@ -0,0 +1,628 @@ +import sqlite3 +import sys +import types +import uuid + + +class _DummyConfigManager: + def get(self, key, default=None): + return default + + def get_active_media_server(self): + return "plex" + + +if "spotipy" not in sys.modules: + spotipy = types.ModuleType("spotipy") + + class _DummySpotify: + def __init__(self, *args, **kwargs): + pass + + oauth2 = types.ModuleType("spotipy.oauth2") + + class _DummyOAuth: + def __init__(self, *args, **kwargs): + pass + + spotipy.Spotify = _DummySpotify + oauth2.SpotifyOAuth = _DummyOAuth + oauth2.SpotifyClientCredentials = _DummyOAuth + spotipy.oauth2 = oauth2 + sys.modules["spotipy"] = spotipy + sys.modules["spotipy.oauth2"] = oauth2 + +if "config.settings" not in sys.modules: + config_pkg = types.ModuleType("config") + settings_mod = types.ModuleType("config.settings") + + settings_mod.config_manager = _DummyConfigManager() + config_pkg.settings = settings_mod + sys.modules["config"] = config_pkg + sys.modules["config.settings"] = settings_mod + +from core.repair_jobs.album_completeness import AlbumCompletenessJob +import core.repair_jobs.album_completeness as album_completeness_module + + +class _SharedMemoryDB: + def __init__(self): + self.uri = ( + f"file:testdb_{uuid.uuid4().hex}" + "?mode=memory&cache=shared" + ) + self._keepalive = sqlite3.connect(self.uri, uri=True) + self._keepalive.executescript( + """ + CREATE TABLE artists ( + id TEXT PRIMARY KEY, + name TEXT, + thumb_url TEXT + ); + + CREATE TABLE albums ( + id TEXT PRIMARY KEY, + artist_id TEXT, + title TEXT, + thumb_url TEXT, + spotify_album_id TEXT, + itunes_album_id TEXT, + deezer_id TEXT, + discogs_id TEXT, + soul_id TEXT, + musicbrainz_release_id TEXT, + canonical_source TEXT, + canonical_album_id TEXT, + api_track_count INTEGER + ); + + CREATE TABLE tracks ( + id TEXT PRIMARY KEY, + album_id TEXT, + title TEXT, + track_number INTEGER, + disc_number INTEGER, + duration INTEGER, + musicbrainz_recording_id TEXT + ); + """ + ) + self._keepalive.commit() + + def _get_connection(self): + return sqlite3.connect(self.uri, uri=True) + + def insert_artist(self, artist_id, name): + self._keepalive.execute( + """ + INSERT INTO artists (id, name) + VALUES (?, ?) + """, + (artist_id, name), + ) + self._keepalive.commit() + + def insert_album( + self, + album_id, + artist_id, + title, + *, + spotify_id=None, + musicbrainz_id=None, + canonical_source=None, + canonical_album_id=None, + api_track_count=None, + ): + self._keepalive.execute( + """ + INSERT INTO albums ( + id, + artist_id, + title, + spotify_album_id, + musicbrainz_release_id, + canonical_source, + canonical_album_id, + api_track_count + ) + VALUES (?, ?, ?, ?, ?, ?, ?, ?) + """, + ( + album_id, + artist_id, + title, + spotify_id, + musicbrainz_id, + canonical_source, + canonical_album_id, + api_track_count, + ), + ) + self._keepalive.commit() + + def insert_track( + self, + album_id, + number, + title, + *, + disc=1, + duration_ms=180000, + mbid=None, + ): + track_id = f"{album_id}-{disc}-{number}-{uuid.uuid4().hex}" + self._keepalive.execute( + """ + INSERT INTO tracks ( + id, + album_id, + title, + track_number, + disc_number, + duration, + musicbrainz_recording_id + ) + VALUES (?, ?, ?, ?, ?, ?, ?) + """, + ( + track_id, + album_id, + title, + number, + disc, + duration_ms, + mbid, + ), + ) + self._keepalive.commit() + + +def _context(db, findings): + return types.SimpleNamespace( + db=db, + transfer_folder='', + config_manager=_DummyConfigManager(), + spotify_client=None, + is_spotify_rate_limited=lambda: False, + stop_event=None, + create_finding=lambda **kwargs: ( + findings.append(kwargs) or True + ), + should_stop=None, + is_paused=None, + update_progress=None, + report_progress=None, + check_stop=lambda: False, + wait_if_paused=lambda: False, + ) + + +def _canonical_tracks(count, *, prefix="Canonical"): + return { + "items": [ + { + "id": f"track-{number}", + "name": f"{prefix} Track {number}", + "track_number": number, + "disc_number": 1, + "duration_ms": 180000 + number, + "artists": [], + } + for number in range(1, count + 1) + ], + } + + +def test_scan_groups_validated_fragmented_rows_into_one_finding( + monkeypatch, +): + db = _SharedMemoryDB() + db.insert_artist("artist-1", "Artist") + db.insert_album( + "anchor", + "artist-1", + "Album", + spotify_id="shared-release", + canonical_source="deezer", + canonical_album_id="canonical-release", + ) + db.insert_album( + "fragment", + "artist-1", + "ALBUM", + spotify_id="shared-release", + api_track_count=2, + ) + + db.insert_track("anchor", 1, "Canonical Track 1") + db.insert_track("anchor", 2, "Canonical Track 2") + db.insert_track("fragment", 3, "Canonical Track 3") + db.insert_track("fragment", 4, "Canonical Track 4") + + calls = [] + + def get_tracks(source, album_id): + calls.append((source, album_id)) + assert source == "deezer" + assert album_id == "canonical-release" + return _canonical_tracks(5) + + monkeypatch.setattr( + album_completeness_module, + "get_album_tracks_for_source", + get_tracks, + ) + monkeypatch.setattr( + album_completeness_module, + "get_primary_source", + lambda: "spotify", + ) + monkeypatch.setattr( + album_completeness_module, + "get_source_priority", + lambda primary: ["spotify", "deezer"], + ) + + findings = [] + result = AlbumCompletenessJob().scan( + _context(db, findings) + ) + + assert result.scanned == 1 + assert result.findings_created == 1 + assert calls == [("deezer", "canonical-release")] + + details = findings[0]["details"] + assert details["album_id"] == "anchor" + assert details["expected_tracks"] == 5 + assert details["actual_tracks"] == 4 + assert details["raw_local_tracks"] == 4 + assert details["related_album_ids"] == [ + "anchor", + "fragment", + ] + assert [ + track["track_number"] + for track in details["missing_tracks"] + ] == [5] + + +def test_shared_id_without_track_match_stays_independent( + monkeypatch, +): + db = _SharedMemoryDB() + db.insert_artist("artist-1", "Artist") + db.insert_album( + "anchor", + "artist-1", + "Album", + spotify_id="shared-release", + canonical_source="deezer", + canonical_album_id="canonical-release", + ) + db.insert_album( + "unrelated", + "artist-1", + "Different Album", + spotify_id="shared-release", + api_track_count=1, + ) + + db.insert_track("anchor", 1, "Canonical Track 1") + db.insert_track( + "unrelated", + 99, + "Completely Unrelated", + duration_ms=900000, + ) + + calls = [] + + def get_tracks(source, album_id): + calls.append((source, album_id)) + return _canonical_tracks(3) + + monkeypatch.setattr( + album_completeness_module, + "get_album_tracks_for_source", + get_tracks, + ) + monkeypatch.setattr( + album_completeness_module, + "get_primary_source", + lambda: "spotify", + ) + monkeypatch.setattr( + album_completeness_module, + "get_source_priority", + lambda primary: ["spotify", "deezer"], + ) + + findings = [] + result = AlbumCompletenessJob().scan( + _context(db, findings) + ) + + assert result.scanned == 2 + assert result.findings_created == 1 + assert calls == [("deezer", "canonical-release")] + assert findings[0]["entity_id"] == "anchor" + assert findings[0]["details"]["related_album_ids"] == [ + "anchor", + ] + + +def test_excluded_canonical_sibling_reports_only_its_missing_tracks( + monkeypatch, +): + """A second row pinned to the SAME canonical edition whose tracks fail the + strict fragment match is still evaluated against that edition β€” but it must + report only the tracks it does NOT own, not the whole tracklist. Regression + for the excluded-sibling bug (it previously flagged every canonical track as + missing, including the ones the row already had).""" + db = _SharedMemoryDB() + db.insert_artist("artist-1", "Artist") + # Anchor: complete, titles match the canonical edition. + db.insert_album( + "anchor", + "artist-1", + "Album", + canonical_source="deezer", + canonical_album_id="canonical-release", + ) + for number in range(1, 6): + db.insert_track("anchor", number, f"Canonical Track {number}") + + # Sibling: same canonical pair, owns tracks 1-3 by NUMBER but with blank + # titles β†’ fails the strict fragment match β†’ excluded from the anchor group. + db.insert_album( + "sibling", + "artist-1", + "Album", + canonical_source="deezer", + canonical_album_id="canonical-release", + ) + for number in range(1, 4): + db.insert_track("sibling", number, "") + + monkeypatch.setattr( + album_completeness_module, + "get_album_tracks_for_source", + lambda source, album_id: _canonical_tracks(5), + ) + monkeypatch.setattr( + album_completeness_module, + "get_primary_source", + lambda: "deezer", + ) + monkeypatch.setattr( + album_completeness_module, + "get_source_priority", + lambda primary: ["deezer"], + ) + + findings = [] + AlbumCompletenessJob().scan(_context(db, findings)) + + sibling_finding = next( + f for f in findings if f["entity_id"] == "sibling" + ) + details = sibling_finding["details"] + # Owns tracks 1-3 (by number) β†’ "3 of 5", and only 2 missing (4 & 5), + # NOT the full 5 β€” and the count stays internally consistent. + assert details["actual_tracks"] == 3 + assert details["expected_tracks"] == 5 + missing_numbers = sorted( + t["track_number"] for t in details["missing_tracks"] + ) + assert missing_numbers == [4, 5] + + +def test_candidate_matching_two_canonical_groups_stays_independent(): + """A candidate whose shared ID resolves to MORE THAN ONE canonical group is + ambiguous and must not be fused into either. Locks the `len(matches) == 1` + rule the one-pass grouping relies on. (`_build_candidate_groups` is pure, so + drive it directly with album dicts.)""" + def _album(album_id, order, **extra): + base = { + 'album_id': album_id, 'artist_id': 'artist-1', + 'album_title': album_id, 'actual_count': 1, '_scan_order': order, + 'spotify_album_id': '', 'itunes_album_id': '', 'deezer_album_id': '', + 'discogs_album_id': '', 'hydrabase_album_id': '', + 'musicbrainz_album_id': '', 'canonical_source': '', + 'canonical_album_id': '', + } + base.update(extra) + return base + + # Two distinct canonical editions (same artist) that share spotify-X, plus a + # candidate that also carries spotify-X β†’ it resolves to BOTH groups. + albums = [ + _album('anchor-a', 0, spotify_album_id='shared-X', + canonical_source='deezer', canonical_album_id='canonical-a'), + _album('anchor-b', 1, spotify_album_id='shared-X', + canonical_source='deezer', canonical_album_id='canonical-b'), + _album('candidate', 2, spotify_album_id='shared-X'), + ] + + groups = AlbumCompletenessJob()._build_candidate_groups(albums) + candidate_groups = [ + g for g in groups + if any(m['album_id'] == 'candidate' for m in g['members']) + ] + # Candidate is its OWN singleton group, never fused into A or B. + assert len(candidate_groups) == 1 + assert [m['album_id'] for m in candidate_groups[0]['members']] == [ + 'candidate' + ] + + +def test_unambiguous_candidate_joins_its_single_group(): + """The complement: a candidate that resolves to exactly one canonical group + joins it (the one-pass path produces the same membership as before).""" + def _album(album_id, order, **extra): + base = { + 'album_id': album_id, 'artist_id': 'artist-1', + 'album_title': album_id, 'actual_count': 1, '_scan_order': order, + 'spotify_album_id': '', 'itunes_album_id': '', 'deezer_album_id': '', + 'discogs_album_id': '', 'hydrabase_album_id': '', + 'musicbrainz_album_id': '', 'canonical_source': '', + 'canonical_album_id': '', + } + base.update(extra) + return base + + albums = [ + _album('anchor', 0, spotify_album_id='shared-X', + canonical_source='deezer', canonical_album_id='canonical-a'), + _album('candidate', 1, spotify_album_id='shared-X'), + ] + groups = AlbumCompletenessJob()._build_candidate_groups(albums) + assert len(groups) == 1 + assert sorted(m['album_id'] for m in groups[0]['members']) == [ + 'anchor', 'candidate' + ] + + +def test_fragment_grouping_never_crosses_artist_boundary( + monkeypatch, +): + db = _SharedMemoryDB() + db.insert_artist("artist-1", "Artist One") + db.insert_artist("artist-2", "Artist Two") + db.insert_album( + "anchor", + "artist-1", + "Album", + spotify_id="shared-release", + canonical_source="deezer", + canonical_album_id="canonical-release", + ) + db.insert_album( + "other-artist", + "artist-2", + "Album", + spotify_id="shared-release", + api_track_count=1, + ) + + db.insert_track("anchor", 1, "Canonical Track 1") + db.insert_track( + "other-artist", + 2, + "Canonical Track 2", + ) + + calls = [] + + def get_tracks(source, album_id): + calls.append((source, album_id)) + return _canonical_tracks(3) + + monkeypatch.setattr( + album_completeness_module, + "get_album_tracks_for_source", + get_tracks, + ) + monkeypatch.setattr( + album_completeness_module, + "get_primary_source", + lambda: "spotify", + ) + monkeypatch.setattr( + album_completeness_module, + "get_source_priority", + lambda primary: ["spotify", "deezer"], + ) + + findings = [] + result = AlbumCompletenessJob().scan( + _context(db, findings) + ) + + assert result.scanned == 2 + assert result.findings_created == 1 + assert calls == [("deezer", "canonical-release")] + assert findings[0]["details"]["related_album_ids"] == [ + "anchor", + ] + + +def test_musicbrainz_recording_id_validates_fragment( + monkeypatch, +): + db = _SharedMemoryDB() + db.insert_artist("artist-1", "Artist") + db.insert_album( + "anchor", + "artist-1", + "Album", + spotify_id="shared-release", + musicbrainz_id="mb-release", + canonical_source="musicbrainz", + canonical_album_id="mb-release", + ) + db.insert_album( + "fragment", + "artist-1", + "Album Fragment", + spotify_id="shared-release", + api_track_count=1, + ) + + db.insert_track( + "anchor", + 1, + "Canonical Track 1", + mbid="track-1", + ) + db.insert_track( + "fragment", + 99, + "Wrong title and position", + duration_ms=999999, + mbid="track-2", + ) + + calls = [] + + def get_tracks(source, album_id): + calls.append((source, album_id)) + return _canonical_tracks(3) + + monkeypatch.setattr( + album_completeness_module, + "get_album_tracks_for_source", + get_tracks, + ) + monkeypatch.setattr( + album_completeness_module, + "get_primary_source", + lambda: "spotify", + ) + monkeypatch.setattr( + album_completeness_module, + "get_source_priority", + lambda primary: ["spotify", "musicbrainz"], + ) + + findings = [] + result = AlbumCompletenessJob().scan( + _context(db, findings) + ) + + assert result.scanned == 1 + assert result.findings_created == 1 + assert calls == [("musicbrainz", "mb-release")] + + details = findings[0]["details"] + assert details["actual_tracks"] == 2 + assert details["related_album_ids"] == [ + "anchor", + "fragment", + ] + assert [ + track["track_number"] + for track in details["missing_tracks"] + ] == [3] diff --git a/tests/test_album_completeness_job.py b/tests/test_album_completeness_job.py index 8d58a03c..4cca27fc 100644 --- a/tests/test_album_completeness_job.py +++ b/tests/test_album_completeness_job.py @@ -1,4 +1,3 @@ -import sys import types @@ -10,35 +9,10 @@ class _DummyConfigManager: return "plex" -if "spotipy" not in sys.modules: - spotipy = types.ModuleType("spotipy") - - class _DummySpotify: - def __init__(self, *args, **kwargs): - pass - - oauth2 = types.ModuleType("spotipy.oauth2") - - class _DummyOAuth: - def __init__(self, *args, **kwargs): - pass - - spotipy.Spotify = _DummySpotify - oauth2.SpotifyOAuth = _DummyOAuth - oauth2.SpotifyClientCredentials = _DummyOAuth - spotipy.oauth2 = oauth2 - sys.modules["spotipy"] = spotipy - sys.modules["spotipy.oauth2"] = oauth2 - -if "config.settings" not in sys.modules: - config_pkg = types.ModuleType("config") - settings_mod = types.ModuleType("config.settings") - - settings_mod.config_manager = _DummyConfigManager() - config_pkg.settings = settings_mod - sys.modules["config"] = config_pkg - sys.modules["config.settings"] = settings_mod - +# NOTE: deliberately no sys.modules stubbing of spotipy / config.settings here. Both import +# fine in the test env, and faking them globally (with no teardown) leaked into other files β€” +# it left a config.settings with no ConfigManager, intermittently breaking +# tests/test_config_save_retry depending on collection order. from core.repair_jobs.album_completeness import AlbumCompletenessJob import core.repair_jobs.album_completeness as album_completeness_module @@ -420,6 +394,8 @@ class _SharedMemoryDB: deezer_id TEXT, discogs_id TEXT, soul_id TEXT, + canonical_source TEXT, + canonical_album_id TEXT, track_count INTEGER, api_track_count INTEGER ); @@ -442,13 +418,41 @@ class _SharedMemoryDB: ) self._keepalive.commit() - def insert_album(self, album_id, artist_id, title, *, spotify_id=None, - track_count=None, api_track_count=None): + def insert_album( + self, + album_id, + artist_id, + title, + *, + spotify_id=None, + canonical_source=None, + canonical_album_id=None, + track_count=None, + api_track_count=None, + ): self._keepalive.execute( """INSERT INTO albums - (id, artist_id, title, spotify_album_id, track_count, api_track_count) - VALUES (?, ?, ?, ?, ?, ?)""", - (album_id, artist_id, title, spotify_id, track_count, api_track_count), + ( + id, + artist_id, + title, + spotify_album_id, + canonical_source, + canonical_album_id, + track_count, + api_track_count + ) + VALUES (?, ?, ?, ?, ?, ?, ?, ?)""", + ( + album_id, + artist_id, + title, + spotify_id, + canonical_source, + canonical_album_id, + track_count, + api_track_count, + ), ) self._keepalive.commit() @@ -543,6 +547,170 @@ def test_scan_uses_cached_api_track_count_without_expected_total_lookup(monkeypa assert finding['details']['actual_tracks'] == 10 +def test_scan_uses_exact_canonical_edition_for_count_and_missing_tracks( + monkeypatch, +): + db = _SharedMemoryDB() + db.insert_artist('a1', 'Test Artist') + db.insert_album( + 'alb-canonical', + 'a1', + 'Canonical Album', + spotify_id='sp-other-edition', + canonical_source='deezer', + canonical_album_id='dz-canonical', + track_count=2, + api_track_count=99, + ) + db.insert_tracks('alb-canonical', 2) + + calls = [] + + def get_tracks(source, album_id): + calls.append((source, album_id)) + + if source == 'deezer' and album_id == 'dz-canonical': + return { + 'items': [ + { + 'id': f'dz-{number}', + 'name': f'Canonical Track {number}', + 'track_number': number, + 'disc_number': 1, + 'artists': [], + } + for number in range(1, 5) + ], + } + + if source == 'spotify' and album_id == 'sp-other-edition': + return { + 'items': [ + { + 'id': f'sp-{number}', + 'name': f'Other Edition Track {number}', + 'track_number': number, + 'disc_number': 1, + 'artists': [], + } + for number in range(1, 13) + ], + } + + return None + + monkeypatch.setattr( + album_completeness_module, + 'get_album_tracks_for_source', + get_tracks, + ) + monkeypatch.setattr( + album_completeness_module, + 'get_primary_source', + lambda: 'spotify', + ) + monkeypatch.setattr( + album_completeness_module, + 'get_source_priority', + lambda primary: ['spotify', 'deezer'], + ) + + findings = [] + context = _make_job_context( + db, + create_finding=lambda **kwargs: (findings.append(kwargs) or True), + ) + + result = AlbumCompletenessJob().scan(context) + + assert result.findings_created == 1 + assert calls == [('deezer', 'dz-canonical')] + + details = findings[0]['details'] + + assert details['primary_source'] == 'deezer' + assert details['primary_album_id'] == 'dz-canonical' + assert details['canonical_source'] == 'deezer' + assert details['canonical_album_id'] == 'dz-canonical' + assert details['expected_tracks'] == 4 + assert details['actual_tracks'] == 2 + assert [ + track['track_number'] + for track in details['missing_tracks'] + ] == [3, 4] + assert all( + track['source'] == 'deezer' + for track in details['missing_tracks'] + ) + + +def test_scan_does_not_fallback_when_canonical_edition_is_unavailable( + monkeypatch, +): + db = _SharedMemoryDB() + db.insert_artist('a1', 'Test Artist') + db.insert_album( + 'alb-unavailable-canonical', + 'a1', + 'Unavailable Canonical Album', + spotify_id='sp-other-edition', + canonical_source='deezer', + canonical_album_id='dz-unavailable', + track_count=2, + api_track_count=99, + ) + db.insert_tracks('alb-unavailable-canonical', 2) + + calls = [] + + def get_tracks(source, album_id): + calls.append((source, album_id)) + + if source == 'deezer' and album_id == 'dz-unavailable': + return {'items': []} + + return { + 'items': [ + { + 'id': f'sp-{number}', + 'name': f'Other Edition Track {number}', + 'track_number': number, + 'disc_number': 1, + 'artists': [], + } + for number in range(1, 11) + ], + } + + monkeypatch.setattr( + album_completeness_module, + 'get_album_tracks_for_source', + get_tracks, + ) + monkeypatch.setattr( + album_completeness_module, + 'get_primary_source', + lambda: 'spotify', + ) + monkeypatch.setattr( + album_completeness_module, + 'get_source_priority', + lambda primary: ['spotify', 'deezer'], + ) + + findings = [] + context = _make_job_context( + db, + create_finding=lambda **kwargs: (findings.append(kwargs) or True), + ) + + result = AlbumCompletenessJob().scan(context) + + assert result.findings_created == 0 + assert findings == [] + assert calls == [('deezer', 'dz-unavailable')] + + def test_scan_falls_back_to_api_and_persists_count_on_cache_miss(monkeypatch): """Integration: when api_track_count is NULL, scan calls the API, gets the expected total, caches it, and creates the finding.""" diff --git a/tests/test_canonical_manual_lock.py b/tests/test_canonical_manual_lock.py index ad9fd99e..54f9bcea 100644 --- a/tests/test_canonical_manual_lock.py +++ b/tests/test_canonical_manual_lock.py @@ -24,7 +24,7 @@ from database.music_database import MusicDatabase # should_pin_manual_canonical β€” pure # --------------------------------------------------------------------------- -@pytest.mark.parametrize('source', ['spotify', 'itunes', 'deezer', 'discogs', 'hydrabase']) +@pytest.mark.parametrize('source', ['spotify', 'itunes', 'deezer', 'discogs', 'hydrabase', 'musicbrainz']) def test_pins_album_on_recognised_source(source): assert should_pin_manual_canonical('album', source) is True @@ -34,7 +34,7 @@ def test_does_not_pin_non_album(entity): assert should_pin_manual_canonical(entity, 'spotify') is False -@pytest.mark.parametrize('source', ['lastfm', 'genius', 'musicbrainz', 'audiodb', 'tidal']) +@pytest.mark.parametrize('source', ['lastfm', 'genius', 'audiodb', 'tidal']) def test_does_not_pin_source_canonical_cant_read(source): # No album-version data the canonical tools read β†’ nothing to pin. assert should_pin_manual_canonical('album', source) is False diff --git a/tests/test_clear_completed_download_history.py b/tests/test_clear_completed_download_history.py new file mode 100644 index 00000000..e84e1cae --- /dev/null +++ b/tests/test_clear_completed_download_history.py @@ -0,0 +1,103 @@ +"""'Clear Completed' on the Downloads page deletes ALL persisted completed-download +history (every event_type='download' row), including the unverified review-queue rows +β€” the user wants the list emptied, and those unverified rows ARE download-history rows. +It only removes HISTORY rows; the actual files / `tracks` entries are untouched, so the +library is never affected β€” only the 'needs verification' flags. (Clear-button restoration.)""" + +import sqlite3 +import sys +import types + +if "spotipy" not in sys.modules: # match the suite's lightweight stubs + spotipy = types.ModuleType("spotipy") + spotipy.Spotify = object + oauth2 = types.ModuleType("spotipy.oauth2") + oauth2.SpotifyOAuth = object + oauth2.SpotifyClientCredentials = object + spotipy.oauth2 = oauth2 + sys.modules["spotipy"] = spotipy + sys.modules["spotipy.oauth2"] = oauth2 + +if "config.settings" not in sys.modules: + config_pkg = types.ModuleType("config") + settings_mod = types.ModuleType("config.settings") + + class _DummyConfigManager: + def get(self, key, default=None): + return default + + def get_active_media_server(self): + return "primary" + + settings_mod.config_manager = _DummyConfigManager() + config_pkg.settings = settings_mod + sys.modules["config"] = config_pkg + sys.modules["config.settings"] = settings_mod + +from database.music_database import MusicDatabase # noqa: E402 + + +class _NonClosingConn: + def __init__(self, real): + self._real = real + + def cursor(self): + return self._real.cursor() + + def commit(self): + return self._real.commit() + + def close(self): + pass + + +class _InMemoryDB(MusicDatabase): + def __init__(self): + self._conn = sqlite3.connect(":memory:") + self._conn.row_factory = sqlite3.Row + self._conn.execute( + "CREATE TABLE library_history (" + "id INTEGER PRIMARY KEY AUTOINCREMENT, event_type TEXT NOT NULL, " + "title TEXT, file_path TEXT, verification_status TEXT)") + + def _get_connection(self): + return _NonClosingConn(self._conn) + + def _add(self, event_type, status, title="Song"): + self._conn.execute( + "INSERT INTO library_history (event_type, title, verification_status) " + "VALUES (?, ?, ?)", (event_type, title, status)) + self._conn.commit() + + def _statuses(self): + return [r[0] for r in self._conn.execute( + "SELECT verification_status FROM library_history ORDER BY id").fetchall()] + + +def test_clears_all_completed_download_rows_including_unverified(): + db = _InMemoryDB() + db._add("download", "verified") + db._add("download", "human_verified") + db._add("download", None) # legacy / unscored completed + db._add("download", "unverified") # review queue β€” also cleared (user chose clear-all) + db._add("download", "force_imported") + removed = db.clear_completed_download_history() + assert removed == 5 + assert db._statuses() == [] + + +def test_does_not_touch_non_download_history(): + """Only event_type='download' rows are the Downloads-page tail; imports etc. stay.""" + db = _InMemoryDB() + db._add("download", "verified") + db._add("import", "verified") + removed = db.clear_completed_download_history() + assert removed == 1 + # the import row survives + rows = db._conn.execute("SELECT event_type FROM library_history").fetchall() + assert [r[0] for r in rows] == ["import"] + + +def test_empty_history_returns_zero(): + db = _InMemoryDB() + assert db.clear_completed_download_history() == 0 diff --git a/tests/test_history_match.py b/tests/test_history_match.py new file mode 100644 index 00000000..bfb05e8d --- /dev/null +++ b/tests/test_history_match.py @@ -0,0 +1,63 @@ +"""Pure matcher that re-links a moved file to its download-history row (#934).""" + +from core.downloads.history_match import pick_history_row, like_filename_filter + + +# (id, file_path, title, download_source) +def _row(i, path, title='', source='soulseek'): + return (i, path, title, source) + + +def test_exact_current_path_wins(): + cands = [_row(1, '/old/song.flac', 'Song'), _row(2, '/lib/song.flac', 'Song')] + assert pick_history_row(cands, current_paths=('/lib/song.flac', None), + basename='song.flac', title='Song') == 2 + + +def test_falls_back_to_filename_when_path_drifted(): + # history has the OLD import path; scanner only knows the NEW library path. + cands = [_row(7, '/downloads/transfer/Artist/01 - Song.flac', 'Song')] + assert pick_history_row(cands, current_paths=('/music/Artist/Album/01 - Song.flac', None), + basename='01 - Song.flac', title='Song') == 7 + + +def test_title_guard_blocks_shared_filename_collision(): + # two different songs both named "01 - Intro.flac" β€” must NOT heal the wrong one. + cands = [_row(1, '/a/01 - Intro.flac', 'Album A Intro'), + _row(2, '/b/01 - Intro.flac', 'Album B Intro')] + got = pick_history_row(cands, current_paths=('/c/01 - Intro.flac', None), + basename='01 - Intro.flac', title='Album B Intro') + assert got == 2 + + +def test_title_drift_still_matches_after_normalization(): + cands = [_row(5, '/old/track.flac', 'Song (Remastered)')] + assert pick_history_row(cands, current_paths=('/new/track.flac', None), + basename='track.flac', title='song remastered') == 5 + + +def test_prefers_real_download_row_over_synthetic_scan_row(): + # the #934 collapse: a real download row (drifted path) + a synthetic scan dup at the + # exact current path. The REAL row must win, so the synthetic one can be deleted. + cands = [_row(10, '/old/song.flac', 'Song', source='soulseek'), + _row(11, '/lib/song.flac', 'Song', source='acoustid_scan')] + assert pick_history_row(cands, current_paths=('/lib/song.flac', None), + basename='song.flac', title='Song') == 10 + + +def test_no_basename_match_returns_none(): + cands = [_row(1, '/x/other.flac', 'Other')] + assert pick_history_row(cands, current_paths=('/x/wanted.flac', None), + basename='wanted.flac', title='Wanted') is None + + +def test_filename_only_substring_is_not_a_match(): + # '/x/mysong.flac' must NOT satisfy basename 'song.flac' + cands = [_row(1, '/x/mysong.flac', 'My Song')] + assert pick_history_row(cands, current_paths=('/x/song.flac', None), + basename='song.flac', title='Song') is None + + +def test_like_filter_escapes_metacharacters(): + # underscores/percents in filenames must not become LIKE wildcards + assert like_filename_filter('a_b%c.flac') == r'%a\_b\%c.flac' diff --git a/tests/test_media_track_disc_number.py b/tests/test_media_track_disc_number.py new file mode 100644 index 00000000..6bbc4af1 --- /dev/null +++ b/tests/test_media_track_disc_number.py @@ -0,0 +1,68 @@ +"""Multi-disc fix (#927): the media-server scan must store the real disc number. + +Every track was stored with disc_number=1 because the Jellyfin/Plex/Navidrome scan never +read the disc field β€” so multi-disc albums collapsed onto disc 1, mis-filing disc-2+ tracks +and flagging them "missing". insert_or_update_media_track now reads the disc number off the +track object (Jellyfin/Navidrome `.discNumber`, Plex `.parentIndex`), floored to >=1. +""" + +from __future__ import annotations + +from types import SimpleNamespace + +from database.music_database import MusicDatabase + + +def _db(tmp_path): + db = MusicDatabase(database_path=str(tmp_path / "t.db")) + with db._get_connection() as c: + c.execute("INSERT OR IGNORE INTO artists (id, name) VALUES ('art1', 'Evan Call')") + c.execute("INSERT OR IGNORE INTO albums (id, artist_id, title) VALUES ('alb1', 'art1', 'Frieren OST')") + c.commit() + return db + + +def _track(rating_key, title, track_number, **kw): + return SimpleNamespace(ratingKey=rating_key, title=title, trackNumber=track_number, + duration=200000, **kw) + + +def _disc_of(db, track_id): + with db._get_connection() as c: + row = c.execute("SELECT disc_number, track_number FROM tracks WHERE id = ?", (track_id,)).fetchone() + return (row['disc_number'], row['track_number']) + + +def test_jellyfin_navidrome_disc_number_stored(tmp_path): + db = _db(tmp_path) + # .discNumber is what the Jellyfin (ParentIndexNumber) + Navidrome (discNumber) wrappers set. + db.insert_or_update_media_track(_track('t-d2', 'Waltz for Stark and Fern', 34, discNumber=2), 'alb1', 'art1', 'jellyfin') + db.insert_or_update_media_track(_track('t-d1', 'The Magic Within', 32, discNumber=1), 'alb1', 'art1', 'jellyfin') + assert _disc_of(db, 't-d2') == (2, 34) + assert _disc_of(db, 't-d1') == (1, 32) + + +def test_plex_parent_index_used_as_disc(tmp_path): + db = _db(tmp_path) + # plexapi Track has no .discNumber β€” disc comes from .parentIndex. + db.insert_or_update_media_track(_track('t-plex', 'Disc 2 Track', 5, parentIndex=2), 'alb1', 'art1', 'plex') + assert _disc_of(db, 't-plex') == (2, 5) + + +def test_missing_or_bad_disc_floors_to_one(tmp_path): + db = _db(tmp_path) + db.insert_or_update_media_track(_track('t-none', 'No Disc', 1), 'alb1', 'art1', 'jellyfin') # no disc attr + db.insert_or_update_media_track(_track('t-zero', 'Zero Disc', 2, discNumber=0), 'alb1', 'art1', 'jellyfin') + db.insert_or_update_media_track(_track('t-str', 'Junk Disc', 3, discNumber='x'), 'alb1', 'art1', 'jellyfin') + assert _disc_of(db, 't-none')[0] == 1 + assert _disc_of(db, 't-zero')[0] == 1 + assert _disc_of(db, 't-str')[0] == 1 + + +def test_update_path_backfills_disc_on_rescan(tmp_path): + db = _db(tmp_path) + # First scan (old behavior simulated): no disc -> 1. Re-scan with the real disc -> updated. + db.insert_or_update_media_track(_track('t-x', 'Track', 7), 'alb1', 'art1', 'jellyfin') + assert _disc_of(db, 't-x') == (1, 7) + db.insert_or_update_media_track(_track('t-x', 'Track', 7, discNumber=3), 'alb1', 'art1', 'jellyfin') + assert _disc_of(db, 't-x') == (3, 7) diff --git a/tests/test_orphan_history.py b/tests/test_orphan_history.py new file mode 100644 index 00000000..166ca5b7 --- /dev/null +++ b/tests/test_orphan_history.py @@ -0,0 +1,87 @@ +"""Pure orphan-detection rules for the review-queue cleanup (#934 follow-up).""" + +from core.downloads.orphan_history import find_orphan_history_ids + + +def _rows(*specs): + # specs: (id, file_path, exists?) + return [{'id': i, 'file_path': p, '_exists': e} for i, p, e in specs] + + +def _resolve(row): + # stand-in for _resolve_history_audio_path: returns a path or None + return '/on/disk' if row.get('_exists') else None + + +def test_only_missing_files_are_orphans(): + rows = _rows((1, '/a.flac', True), (2, '/b.flac', False), (3, '/c.flac', True)) + out = find_orphan_history_ids(rows, _resolve) + assert out['orphan_ids'] == [2] + assert out['checked'] == 3 + assert out['suspicious'] is False + + +def test_rows_without_path_are_skipped(): + rows = _rows((1, '', False), (2, None, False), (3, '/c.flac', False)) + out = find_orphan_history_ids(rows, _resolve) + assert out['checked'] == 1 + assert out['orphan_ids'] == [3] + + +def test_all_missing_with_enough_rows_is_suspicious(): + rows = _rows(*[(i, f'/x{i}.flac', False) for i in range(6)]) + out = find_orphan_history_ids(rows, _resolve) + assert out['suspicious'] is True # mount-down signature -> caller refuses + assert len(out['orphan_ids']) == 6 + + +def test_all_missing_but_few_rows_is_not_suspicious(): + rows = _rows((1, '/x.flac', False), (2, '/y.flac', False)) + out = find_orphan_history_ids(rows, _resolve) + assert out['suspicious'] is False # too few to suspect an outage + assert out['orphan_ids'] == [1, 2] + + +def test_some_present_is_never_suspicious(): + rows = _rows(*[(i, f'/x{i}.flac', False) for i in range(8)], (99, '/real.flac', True)) + out = find_orphan_history_ids(rows, _resolve) + assert out['suspicious'] is False # at least one file exists -> library is up + assert 99 not in out['orphan_ids'] + + +def _status_rows(*specs): + # specs: (id, file_path, exists?, verification_status) + return [{'id': i, 'file_path': p, '_exists': e, 'verification_status': s} + for i, p, e, s in specs] + + +def _deletable_unverified(row): + return row.get('verification_status') == 'unverified' + + +def test_deletable_protects_rows_from_orphan_ids(): + """force_imported rows must never be swept, even when their file is gone.""" + rows = _status_rows( + (1, '/a.flac', False, 'unverified'), + (2, '/b.flac', False, 'force_imported'), # gone, but protected + ) + out = find_orphan_history_ids(rows, _resolve, deletable=_deletable_unverified) + assert out['orphan_ids'] == [1] # only the unverified orphan + assert 2 not in out['orphan_ids'] + + +def test_protected_rows_still_count_toward_mount_down_gate(): + """Regression guard (#938 follow-up): protecting rows must NOT shrink the + safety gate. A few unverified + many force_imported, ALL unreachable (mount + outage) must still read 'suspicious' so nothing is deleted β€” even though only + the unverified ones would otherwise be deletable.""" + rows = _status_rows( + (1, '/a.flac', False, 'unverified'), + (2, '/b.flac', False, 'unverified'), + *[(i, f'/x{i}.flac', False, 'force_imported') for i in range(3, 8)], + ) + out = find_orphan_history_ids(rows, _resolve, deletable=_deletable_unverified) + assert out['checked'] == 7 # all rows counted + assert out['suspicious'] is True # gate fires on all-missing β†’ refuse + # (caller refuses on suspicious, so orphan_ids is moot β€” but it's only the unverified) + assert set(out['orphan_ids']) == {1, 2} diff --git a/tests/test_personalized_playlists_id_gate.py b/tests/test_personalized_playlists_id_gate.py index e19c7732..a7ca4adb 100644 --- a/tests/test_personalized_playlists_id_gate.py +++ b/tests/test_personalized_playlists_id_gate.py @@ -429,14 +429,15 @@ def test_popularity_thresholds_spotify_returns_60_40(): assert hidden_max == 40 -def test_popularity_thresholds_deezer_returns_higher_scale(): - """Deezer writes `rank` (raw integer, often 100k+) into the popularity - column, so thresholds must be in that range β€” Spotify's 60/40 would - classify almost every Deezer track as a hidden gem.""" +def test_popularity_thresholds_deezer_on_0_100_scale(): + """Corrected: the discovery pool SYNTHESIZES deezer popularity onto a 0-100 score (capped + at 100) at scan time β€” it is NOT Deezer's raw rank. The old 500000/100000 rank thresholds + matched nothing, so Popular Picks came back empty for deezer-primary users while Hidden + Gems' < 100000 caught the whole pool. Thresholds must stay on the 0-100 scale.""" svc = PersonalizedPlaylistsService(_FakeDatabase()) popular_min, hidden_max = svc._get_popularity_thresholds('deezer') - assert popular_min is not None and popular_min >= 100_000 - assert hidden_max is not None and hidden_max >= 50_000 + assert (popular_min, hidden_max) == (60, 50) + assert 0 < popular_min <= 100 and 0 < hidden_max <= 100 assert popular_min > hidden_max diff --git a/tests/test_playlist_sync_status.py b/tests/test_playlist_sync_status.py new file mode 100644 index 00000000..317b8a6f --- /dev/null +++ b/tests/test_playlist_sync_status.py @@ -0,0 +1,65 @@ +from datetime import datetime, timedelta + +from web_server import ( + _format_playlist_sync_status, + _resolve_spotify_playlist_sync_status, +) + + +class _StubDatabase: + def __init__(self, mirrored): + self._mirrored = mirrored + + def get_mirrored_playlist_by_source(self, source, source_playlist_id, profile_id): + assert source == 'spotify' + assert source_playlist_id == 'spotify-playlist-1' + assert profile_id == 7 + return self._mirrored + + +def _status(minutes_ago=0, **overrides): + timestamp = (datetime(2026, 6, 25, 12, 0, 0) - timedelta(minutes=minutes_ago)).isoformat() + return {'last_synced': timestamp, **overrides} + + +def test_resolve_spotify_playlist_sync_status_uses_mirrored_auto_sync_status(): + sync_statuses = { + 'auto_mirror_42': _status(matched_tracks=12), + } + + status = _resolve_spotify_playlist_sync_status( + 'spotify-playlist-1', + sync_statuses, + database=_StubDatabase({'id': 42}), + profile_id=7, + ) + + assert status['matched_tracks'] == 12 + + +def test_resolve_spotify_playlist_sync_status_prefers_newest_status(): + sync_statuses = { + 'spotify-playlist-1': _status(minutes_ago=20, matched_tracks=1), + 'auto_mirror_42': _status(minutes_ago=5, matched_tracks=2), + } + + status = _resolve_spotify_playlist_sync_status( + 'spotify-playlist-1', + sync_statuses, + database=_StubDatabase({'id': 42}), + profile_id=7, + ) + + assert status['matched_tracks'] == 2 + + +def test_format_playlist_sync_status_treats_missing_snapshot_as_synced(): + status = _status() + + assert _format_playlist_sync_status(status, 'current-snapshot') == 'Synced: Jun 25, 12:00' + + +def test_format_playlist_sync_status_marks_snapshot_mismatch_as_last_sync(): + status = _status(snapshot_id='old-snapshot') + + assert _format_playlist_sync_status(status, 'current-snapshot') == 'Last Sync: Jun 25, 12:00' diff --git a/tests/test_popularity_thresholds.py b/tests/test_popularity_thresholds.py new file mode 100644 index 00000000..c9842d01 --- /dev/null +++ b/tests/test_popularity_thresholds.py @@ -0,0 +1,37 @@ +"""Popular Picks was empty for deezer users β€” wrong popularity-threshold scale. + +The discovery pool synthesizes deezer popularity onto a 0-100 score (base 45 + bonuses, capped +at 100), but _get_popularity_thresholds had deezer on the raw-rank scale (500000/100000). So +`popularity >= 500000` matched nothing (Popular Picks empty) while `< 100000` matched the whole +pool (Hidden Gems caught everything). Thresholds must stay on the 0-100 scale. +""" + +from __future__ import annotations + +from core.personalized_playlists import PersonalizedPlaylistsService + + +def _svc(): + return PersonalizedPlaylistsService(database=None) + + +def test_deezer_thresholds_are_on_the_0_100_scale(): + popular_min, hidden_max = _svc()._get_popularity_thresholds('deezer') + assert (popular_min, hidden_max) == (60, 50) # was (500000, 100000) β€” unreachable + assert 0 < popular_min <= 100 and 0 < hidden_max <= 100 + assert popular_min > hidden_max # popular tier sits above the hidden tier + + +def test_spotify_thresholds_unchanged(): + assert _svc()._get_popularity_thresholds('spotify') == (60, 40) + + +def test_case_insensitive(): + assert _svc()._get_popularity_thresholds('Deezer') == (60, 50) + assert _svc()._get_popularity_thresholds('SPOTIFY') == (60, 40) + + +def test_sources_without_popularity_skip_the_filter(): + assert _svc()._get_popularity_thresholds('itunes') == (None, None) + assert _svc()._get_popularity_thresholds('musicbrainz') == (None, None) + assert _svc()._get_popularity_thresholds('') == (None, None) diff --git a/tests/test_reorganize_canonical_source.py b/tests/test_reorganize_canonical_source.py index 711638ed..f7a2939b 100644 --- a/tests/test_reorganize_canonical_source.py +++ b/tests/test_reorganize_canonical_source.py @@ -7,7 +7,11 @@ canonical_source/canonical_album_id, and an explicit user source pick from __future__ import annotations +from unittest.mock import MagicMock + import core.library_reorganize as lr +import core.metadata.registry as metadata_registry +from core.musicbrainz_search import MusicBrainzSearchClient def _patch_fetch(monkeypatch, tracklists): @@ -73,3 +77,82 @@ def test_no_canonical_unchanged(monkeypatch): album_data = {"spotify_album_id": "sp1"} source, _, _ = lr._resolve_source(album_data, "spotify") assert source == "spotify" + + +def test_musicbrainz_release_id_is_used_by_priority_walk(monkeypatch): + client = MusicBrainzSearchClient() + client._client = MagicMock() + client._client.get_release_group.return_value = None + client._client.get_release.return_value = { + "id": "mb-release-1", + "title": "Test Album", + "date": "2024-01-01", + "artist-credit": [{"name": "Test Artist"}], + "release-group": { + "id": "mb-group-1", + "primary-type": "Album", + "secondary-types": [], + }, + "media": [ + { + "position": 1, + "tracks": [ + { + "id": "track-1", + "number": "1", + "position": 1, + "length": 180000, + "recording": { + "id": "recording-1", + "title": "Test Track", + "artist-credit": [{"name": "Test Artist"}], + "length": 180000, + }, + }, + ], + }, + ], + } + + monkeypatch.setattr( + metadata_registry, + "get_musicbrainz_client", + lambda *args, **kwargs: client, + ) + monkeypatch.setattr( + lr, + "get_source_priority", + lambda primary: ["musicbrainz", "spotify"], + ) + + album_data = { + "musicbrainz_release_id": "mb-release-1", + } + + source, api_album, items = lr._resolve_source( + album_data, + "musicbrainz", + ) + + assert source == "musicbrainz" + assert api_album["id"] == "mb-release-1" + assert api_album["name"] == "Test Album" + assert items == api_album["tracks"] + assert items == [ + { + "id": "recording-1", + "name": "Test Track", + "artists": [{"name": "Test Artist"}], + "duration_ms": 180000, + "track_number": 1, + "disc_number": 1, + }, + ] + client._client.get_release_group.assert_any_call( + "mb-release-1", + includes=["releases", "artist-credits"], + ) + client._client.get_release.assert_any_call( + "mb-release-1", + includes=["recordings", "artist-credits", "release-groups"], + ) diff --git a/tests/test_unverified_history_reconcile.py b/tests/test_unverified_history_reconcile.py new file mode 100644 index 00000000..a87f0917 --- /dev/null +++ b/tests/test_unverified_history_reconcile.py @@ -0,0 +1,191 @@ +"""Issue #934 β€” one-time reconcile that clears the existing backlog of +``library_history`` rows stuck at 'unverified' even though the file has since +been verified (by an AcoustID scan, or human-confirmed). Heals from the +``tracks`` truth, matching exact path AND basename (so a reorganized/moved file +heals too), upgrade-only. Never deletes anything.""" + +import sqlite3 +import sys +import types + +if "spotipy" not in sys.modules: # match the suite's lightweight stubs + spotipy = types.ModuleType("spotipy") + spotipy.Spotify = object + oauth2 = types.ModuleType("spotipy.oauth2") + oauth2.SpotifyOAuth = object + oauth2.SpotifyClientCredentials = object + spotipy.oauth2 = oauth2 + sys.modules["spotipy"] = spotipy + sys.modules["spotipy.oauth2"] = oauth2 + +if "config.settings" not in sys.modules: + config_pkg = types.ModuleType("config") + settings_mod = types.ModuleType("config.settings") + + class _DummyConfigManager: + def get(self, key, default=None): + return default + + def get_active_media_server(self): + return "primary" + + settings_mod.config_manager = _DummyConfigManager() + config_pkg.settings = settings_mod + sys.modules["config"] = config_pkg + sys.modules["config.settings"] = settings_mod + +from database.music_database import MusicDatabase # noqa: E402 + + +class _NonClosingConn: + def __init__(self, real): + self._real = real + + def cursor(self): + return self._real.cursor() + + def commit(self): + return self._real.commit() + + def close(self): + pass + + def __enter__(self): + return self + + def __exit__(self, *args): + pass + + +class _InMemoryDB(MusicDatabase): + def __init__(self): + self._conn = sqlite3.connect(":memory:") + self._conn.row_factory = sqlite3.Row + self._conn.execute( + "CREATE TABLE tracks (id INTEGER PRIMARY KEY, file_path TEXT, " + "title TEXT, verification_status TEXT)" + ) + self._conn.execute( + "CREATE TABLE library_history (" + "id INTEGER PRIMARY KEY AUTOINCREMENT, event_type TEXT, title TEXT, " + "artist_name TEXT, album_name TEXT, file_path TEXT, " + "download_source TEXT, verification_status TEXT, " + "created_at TIMESTAMP DEFAULT CURRENT_TIMESTAMP)" + ) + + def _get_connection(self): + return _NonClosingConn(self._conn) + + def _add_track(self, tid, path, status, title="Song"): + self._conn.execute( + "INSERT INTO tracks (id, file_path, title, verification_status) VALUES (?,?,?,?)", + (tid, path, title, status)) + self._conn.commit() + + def _add_history(self, path, status, title="Song"): + self._conn.execute( + "INSERT INTO library_history (event_type, title, file_path, " + "verification_status) VALUES ('download', ?, ?, ?)", + (title, path, status)) + self._conn.commit() + + def _status_of(self, hid): + return self._conn.execute( + "SELECT verification_status FROM library_history WHERE id = ?", (hid,) + ).fetchone()[0] + + +def test_reconcile_heals_exact_path_match(): + db = _InMemoryDB() + db._add_track(1, "/lib/A/01 - Song.flac", "verified") + db._add_history("/lib/A/01 - Song.flac", "unverified") + healed = db.reconcile_unverified_history_from_tracks() + assert healed == 1 + assert db._status_of(1) == "verified" + + +def test_reconcile_heals_by_basename_when_path_form_differs(): + db = _InMemoryDB() + db._add_track(1, "/library/Artist/Album/01 - Song.flac", "verified") + # History stored the transfer-folder path; basename still matches. + db._add_history("/transfer/Artist - Album/01 - Song.flac", "unverified") + healed = db.reconcile_unverified_history_from_tracks() + assert healed == 1 + assert db._status_of(1) == "verified" + + +def test_reconcile_propagates_human_verified(): + db = _InMemoryDB() + db._add_track(1, "/lib/01 - Song.flac", "human_verified") + db._add_history("/lib/01 - Song.flac", "unverified") + db.reconcile_unverified_history_from_tracks() + assert db._status_of(1) == "human_verified" + + +def test_reconcile_leaves_genuinely_unverified_rows(): + db = _InMemoryDB() + db._add_track(1, "/lib/01 - Song.flac", "unverified") # track itself unconfirmed + db._add_history("/lib/01 - Song.flac", "unverified") + healed = db.reconcile_unverified_history_from_tracks() + assert healed == 0 + assert db._status_of(1) == "unverified" + + +def test_reconcile_leaves_orphans_untouched(): + db = _InMemoryDB() + # No track references this file at all (deleted / re-downloaded elsewhere). + db._add_history("/lib/gone.flac", "unverified") + healed = db.reconcile_unverified_history_from_tracks() + assert healed == 0 + assert db._status_of(1) == "unverified" + + +def test_reconcile_basename_collision_does_not_false_heal(): + """Two different songs share the track-number filename '01 - Intro.flac'. + Only one is a verified track; the OTHER's stale history row must NOT inherit + that verified status just because the filename collides (title guard).""" + db = _InMemoryDB() + db._add_track(1, "/lib/AlbumA/01 - Intro.flac", "verified", title="Intro A") + # A genuinely different, still-unverified song with the same filename. + db._add_history("/transfer/AlbumB/01 - Intro.flac", "unverified", title="Intro B") + healed = db.reconcile_unverified_history_from_tracks() + assert healed == 0 + assert db._status_of(1) == "unverified" + + +def test_reconcile_basename_heals_when_titles_agree(): + """Same filename, same song (path drifted) β€” titles agree, so it heals.""" + db = _InMemoryDB() + db._add_track(1, "/lib/AlbumA/01 - Intro.flac", "verified", title="Intro") + db._add_history("/transfer/old/01 - Intro.flac", "unverified", title="Intro") + healed = db.reconcile_unverified_history_from_tracks() + assert healed == 1 + assert db._status_of(1) == "verified" + + +def test_reconcile_basename_heals_when_history_title_missing(): + """Legacy history rows may have no title β€” fall back to filename-only match + (mirrors the scanner matcher's allowance) so they still heal.""" + db = _InMemoryDB() + db._add_track(1, "/lib/A/01 - Song.flac", "verified", title="Whatever") + db._add_history("/transfer/old/01 - Song.flac", "unverified", title="") + healed = db.reconcile_unverified_history_from_tracks() + assert healed == 1 + assert db._status_of(1) == "verified" + + +def test_reconcile_titleless_row_does_not_heal_on_basename_collision(): + """Follow-up hardening (#938 review): a title-less history row must NOT heal + via filename when that basename collides across MORE THAN ONE verified track β€” + we can't tell which song it is, so healing would risk marking a genuinely + unverified import 'verified'. Unique-basename title-less heal still works + (see test_reconcile_basename_heals_when_history_title_missing).""" + db = _InMemoryDB() + # Two DIFFERENT verified songs that happen to share the generic filename. + db._add_track(1, "/lib/AlbumA/01 - Intro.flac", "verified", title="Intro A") + db._add_track(2, "/lib/AlbumB/01 - Intro.flac", "verified", title="Intro B") + # A stale, title-less history row with that same basename. + db._add_history("/transfer/X/01 - Intro.flac", "unverified", title="") + healed = db.reconcile_unverified_history_from_tracks() + assert healed == 0 + assert db._status_of(1) == "unverified" diff --git a/tests/test_watchlist_album_match.py b/tests/test_watchlist_album_match.py index cbe9fbf2..2ededda9 100644 --- a/tests/test_watchlist_album_match.py +++ b/tests/test_watchlist_album_match.py @@ -156,6 +156,13 @@ def test_compilation_score_explanation() -> None: ("Inception (Music From The Motion Picture)", "Inception Soundtrack"), # Substring containment ("Random Access Memories", "Random Access Memories (Bonus Edition)"), + # Dash-suffixed qualifiers must still collapse β€” these are the SAME album, so + # treating them as different would re-wishlist/redownload forever (the failure the + # original blanket strip guarded against; the narrowed strip must keep covering it). + ("Album Name", "Album Name - Single"), + ("Album Name", "Album Name - Acoustic Version"), + ("Hotel California", "Hotel California - 2013 Remaster"), + ("Album", "Album - The Remixes"), ], ) def test_likely_match_positive(spotify_name, lib_name) -> None: @@ -176,12 +183,29 @@ def test_likely_match_positive(spotify_name, lib_name) -> None: ("Abbey Road", "Sgt. Pepper's Lonely Hearts Club Band"), # Same word in title but different album ("Greatest Hits Volume 1", "Greatest Hits Volume 2"), + # Sokhi: distinct editions of the same franchise β€” the OST vs a bonus edition + # with a real subtitle. USED to collapse to the same normalized name (the blanket + # trailing-dash strip removed "- Nos vies en LumiΓ¨re"), so the watchlist marked + # unowned OST tracks as owned via the bonus edition. Must be DIFFERENT albums. + ("Clair Obscur: Expedition 33: Original Soundtrack", + "Clair Obscur: Expedition 33 - Nos vies en LumiΓ¨re (Bonus Edition)"), + # A real subtitle after a dash must not be stripped down to the base name. + ("The Album", "The Album - A Whole Different Subtitle"), ], ) def test_likely_match_negative(spotify_name, lib_name) -> None: assert not _albums_likely_match(spotify_name, lib_name) +def test_real_subtitle_after_dash_is_preserved() -> None: + # the regression's root cause: a meaningful subtitle must survive normalization, + # while a recognized qualifier after a dash ("- Live", "- 2011") still collapses. + assert _normalize_album_for_match("Clair Obscur: Expedition 33 - Nos vies en LumiΓ¨re") \ + != _normalize_album_for_match("Clair Obscur: Expedition 33") + assert _normalize_album_for_match("Some Album - Live") == _normalize_album_for_match("Some Album") + assert _normalize_album_for_match("Some Album - 2011") == _normalize_album_for_match("Some Album") + + # --------------------------------------------------------------------------- # _albums_likely_match β€” defensive cases # --------------------------------------------------------------------------- diff --git a/tests/test_watchlist_scan_history.py b/tests/test_watchlist_scan_history.py index 3b6f4924..7281e230 100644 --- a/tests/test_watchlist_scan_history.py +++ b/tests/test_watchlist_scan_history.py @@ -47,6 +47,62 @@ def test_resave_is_idempotent_on_run_id(db): assert len(runs) == 1 and runs[0]['tracks_added'] == 11 +# ── persist_scan_run: the shared seam both scan paths use (#933) ── + +from datetime import datetime + +from core.watchlist.scan_history import persist_scan_run + + +def _state(**over): + """A watchlist_scan_state as the scanner leaves it when a scan finishes.""" + s = { + 'scan_run_id': 'auto-run-1', + 'started_at': datetime(2026, 6, 26, 2, 0, 0), + 'completed_at': datetime(2026, 6, 26, 2, 4, 0), + 'total_artists': 40, + 'tracks_found_this_scan': 7, + 'tracks_added_this_scan': 3, + 'scan_track_events': _events(n_added=3, n_skipped=1), + 'summary': {'total_artists': 40, 'successful_scans': 40, + 'new_tracks_found': 7, 'tracks_added_to_wishlist': 3}, + } + s.update(over) + return s + + +def test_persist_scan_run_records_a_history_row(db): + # the #933 fix: an automatic (all-profiles β†’ profile_id=None) scan must land in History. + assert persist_scan_run(db, _state(), profile_id=None, was_cancelled=False) is True + 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']) == \ + ('auto-run-1', 'completed', 7, 3) + assert r['profile_id'] == 1 # None coerced to a concrete profile, never NULL + # the per-run ledger came through too + assert [e['status'] for e in db.get_watchlist_scan_run_events('auto-run-1')] == \ + ['added', 'added', 'added', 'skipped'] + + +def test_persist_scan_run_cancelled_status(db): + persist_scan_run(db, _state(scan_run_id='c1'), profile_id=2, was_cancelled=True) + assert db.get_watchlist_scan_runs()[0]['status'] == 'cancelled' + + +def test_persist_scan_run_accepts_datetime_or_iso_string(db): + # state timestamps may be datetime (auto path) or already-iso strings β€” both must persist. + persist_scan_run(db, _state(scan_run_id='dt', completed_at='2026-06-26T02:09:00'), + profile_id=1, was_cancelled=False) + assert db.get_watchlist_scan_runs()[0]['run_id'] == 'dt' + + +def test_persist_scan_run_tolerates_sparse_state(db): + # a bare/early-finished state must not raise β€” history-write must never break a scan. + assert persist_scan_run(db, {'scan_run_id': 'sparse'}, profile_id=None, was_cancelled=False) + assert db.get_watchlist_scan_runs()[0]['tracks_added'] == 0 + + def test_prune_keeps_most_recent(db): for i in range(1, 8): db.save_watchlist_scan_run( diff --git a/tests/test_wing_it_pool.py b/tests/test_wing_it_pool.py new file mode 100644 index 00000000..81b693a3 --- /dev/null +++ b/tests/test_wing_it_pool.py @@ -0,0 +1,83 @@ +"""Wing It Pool query β€” surfaces tracks Wing It auto-matched (best-effort guesses). + +Wing-it tracks are persisted as the ``wing_it_fallback: true`` flag on a mirrored track's +extra_data and count as 'discovered', so the Discovery Pool's failed list excludes them. The +Wing It Pool is the only surface that lists them. It must: include unverified wing-it tracks, +exclude ones the user already manually matched, scope by playlist + profile, and never include +plain matched/failed tracks. +""" + +from __future__ import annotations + +import json + +from database.music_database import MusicDatabase + + +def _playlist(db, name, profile_id=1, source_id='pl1'): + with db._get_connection() as conn: + cur = conn.execute( + "INSERT INTO mirrored_playlists (source, source_playlist_id, name, profile_id) VALUES (?,?,?,?)", + ('spotify', source_id, name, profile_id)) + conn.commit() + return cur.lastrowid + + +def _track(db, playlist_id, pos, name, artist, extra): + with db._get_connection() as conn: + conn.execute( + "INSERT INTO mirrored_playlist_tracks (playlist_id, position, track_name, artist_name, extra_data) " + "VALUES (?,?,?,?,?)", + (playlist_id, pos, name, artist, json.dumps(extra) if extra is not None else None)) + conn.commit() + + +WING_IT = {'discovered': True, 'provider': 'wing_it_fallback', 'confidence': 0, 'wing_it_fallback': True} +# A resolved wing-it track: /fix MERGES extra_data, so wing_it_fallback survives alongside the +# new manual_match flag β€” that pairing is what marks it resolved (no separate marker needed). +WING_IT_RESOLVED = {'discovered': True, 'provider': 'spotify', 'confidence': 1.0, + 'wing_it_fallback': True, 'manual_match': True, + 'matched_data': {'name': 'Dopamine (Real)'}} +MATCHED = {'discovered': True, 'provider': 'spotify', 'confidence': 0.95} +FAILED = {'discovery_attempted': True, 'discovered': False} + + +def test_lists_only_unverified_wing_it_tracks(tmp_path): + db = MusicDatabase(database_path=str(tmp_path / "w.db")) + pid = _playlist(db, 'Liked Songs') + _track(db, pid, 0, 'Orbital Trans', 'Yoga Mao', WING_IT) # unverified -> attention + _track(db, pid, 1, 'Dopamine', 'Rvdical the Kid', WING_IT_RESOLVED) # resolved -> matched list + _track(db, pid, 2, 'Real Match', 'Some Artist', MATCHED) # normal match -> neither + _track(db, pid, 3, 'Lost Track', 'Nobody', FAILED) # failed -> Discovery Pool's + + attention = db.get_wing_it_pool(profile_id=1) + assert [t['track_name'] for t in attention] == ['Orbital Trans'] + assert attention[0]['artist_name'] == 'Yoga Mao' + assert attention[0]['playlist_name'] == 'Liked Songs' + + resolved = db.get_wing_it_pool(profile_id=1, resolved=True) + assert [t['track_name'] for t in resolved] == ['Dopamine'] + + assert db.get_wing_it_pool_stats(profile_id=1) == {'wing_it': 1, 'matched': 1} + + +def test_scopes_by_playlist_and_profile(tmp_path): + db = MusicDatabase(database_path=str(tmp_path / "w2.db")) + a = _playlist(db, 'Playlist A', profile_id=1, source_id='a') + b = _playlist(db, 'Playlist B', profile_id=1, source_id='b') + other = _playlist(db, 'Other Profile', profile_id=2, source_id='c') + _track(db, a, 0, 'A Song', 'AA', WING_IT) + _track(db, b, 0, 'B Song', 'BB', WING_IT) + _track(db, other, 0, 'C Song', 'CC', WING_IT) + + assert {t['track_name'] for t in db.get_wing_it_pool(profile_id=1)} == {'A Song', 'B Song'} + assert [t['track_name'] for t in db.get_wing_it_pool(playlist_id=a)] == ['A Song'] + assert db.get_wing_it_pool_stats(profile_id=1) == {'wing_it': 2, 'matched': 0} + + +def test_empty_when_no_wing_it(tmp_path): + db = MusicDatabase(database_path=str(tmp_path / "w3.db")) + pid = _playlist(db, 'Clean') + _track(db, pid, 0, 'Matched', 'X', MATCHED) + assert db.get_wing_it_pool(profile_id=1) == [] + assert db.get_wing_it_pool_stats(profile_id=1) == {'wing_it': 0, 'matched': 0} diff --git a/tests/test_youtube_cookies.py b/tests/test_youtube_cookies.py index 0d7d3a04..ea852435 100644 --- a/tests/test_youtube_cookies.py +++ b/tests/test_youtube_cookies.py @@ -98,3 +98,35 @@ def test_write_refuses_junk_without_clobbering_existing(tmp_path): before = dest.read_text() assert write_pasted_cookiefile("", str(dest)) == "" assert dest.read_text() == before + + +# ── regression: youtube_client must USE the helper, not pass 'custom' as a browser ── +# (Docker bug: pasted cookies threw yt-dlp 'unsupported browser: "custom"' because the +# client built cookiesfrombrowser=('custom',) instead of a cookiefile.) + +def test_resolve_cookie_opts_routes_custom_to_cookiefile(monkeypatch, tmp_path): + import core.youtube_client as yt + cookiefile = tmp_path / "youtube_cookies.txt" + cookiefile.write_text(".youtube.com\tTRUE\t/\tTRUE\t123\tSID\tv\n") + cfg = {'youtube.cookies_browser': 'custom', 'youtube.cookies_file': str(cookiefile)} + monkeypatch.setattr('config.settings.config_manager.get', + lambda k, d=None: cfg.get(k, d)) + opts = yt._resolve_cookie_opts() + assert opts == {'cookiefile': str(cookiefile)} + assert 'cookiesfrombrowser' not in opts # never the bogus browser arg + + +def test_resolve_cookie_opts_browser_mode_unchanged(monkeypatch): + import core.youtube_client as yt + cfg = {'youtube.cookies_browser': 'firefox', 'youtube.cookies_file': ''} + monkeypatch.setattr('config.settings.config_manager.get', + lambda k, d=None: cfg.get(k, d)) + assert yt._resolve_cookie_opts() == {'cookiesfrombrowser': ('firefox',)} + + +def test_resolve_cookie_opts_custom_missing_file_is_anonymous(monkeypatch): + import core.youtube_client as yt + cfg = {'youtube.cookies_browser': 'custom', 'youtube.cookies_file': '/nope/gone.txt'} + monkeypatch.setattr('config.settings.config_manager.get', + lambda k, d=None: cfg.get(k, d)) + assert yt._resolve_cookie_opts() == {} # not a broken cookiefile arg diff --git a/tests/watchlist/test_auto_scan.py b/tests/watchlist/test_auto_scan.py index 4ad70258..6ac83413 100644 --- a/tests/watchlist/test_auto_scan.py +++ b/tests/watchlist/test_auto_scan.py @@ -81,6 +81,11 @@ class _FakeDB: self._watchlist_artists = watchlist_artists or [] self._lb_profiles = lb_profiles or [] self.database_path = '/tmp/test.db' + self.scan_runs_saved = [] + + def save_watchlist_scan_run(self, **kwargs): + self.scan_runs_saved.append(kwargs) + return True def get_all_profiles(self): return self._profiles @@ -237,6 +242,39 @@ def test_successful_scan_runs_post_steps(patched_modules): assert deps._state_ref[0]['summary']['new_tracks_found'] == 2 +def test_successful_scan_records_history_run(patched_modules): + """#933: the automatic scan must persist a History row (it used to skip this, + so only manual scans appeared in History).""" + scanner, db = patched_modules + deps = _build_deps() + + autosc.process_watchlist_scan_automatically(automation_id='a1', deps=deps) + + assert len(db.scan_runs_saved) == 1 # exactly one row, no double-record + saved = db.scan_runs_saved[0] + assert saved['status'] == 'completed' + assert saved['artists_scanned'] == 1 # one successful _ScanResult + assert saved['run_id'] # a run id was stamped + + +def test_cancelled_scan_still_records_history_run(patched_modules, monkeypatch): + """A cancelled scan is recorded too, with status='cancelled'.""" + scanner, db = patched_modules + deps = _build_deps() + # Make the scan observe a cancel request mid-run. + orig = scanner.scan_watchlist_artists + def _cancel_during(artists, *, scan_state, progress_callback, cancel_check): + scan_state['cancel_requested'] = True + return orig(artists, scan_state=scan_state, progress_callback=progress_callback, + cancel_check=cancel_check) + monkeypatch.setattr(scanner, 'scan_watchlist_artists', _cancel_during) + + autosc.process_watchlist_scan_automatically(automation_id='a1', deps=deps) + + assert len(db.scan_runs_saved) == 1 + assert db.scan_runs_saved[0]['status'] == 'cancelled' + + def test_completion_emits_automation_event(patched_modules): """Successful scan emits 'watchlist_scan_completed' on automation_engine.""" scanner, _ = patched_modules diff --git a/tests/wishlist/test_image_enrichment.py b/tests/wishlist/test_image_enrichment.py new file mode 100644 index 00000000..3eeaa672 --- /dev/null +++ b/tests/wishlist/test_image_enrichment.py @@ -0,0 +1,91 @@ +"""Wishlist art enrichment (read-path): library-sourced items (re-downloads, preview-clip +re-fetches) store media-server RELATIVE thumb paths that don't render in a browser, and the +nebula only has artist photos for watchlisted artists. _enrich_wishlist_images fixes both on +read β€” normalizing relative/internal image URLs (leaving CDN URLs untouched) and building an +artist-name -> library-photo map β€” so even items already sitting in the wishlist get fixed.""" + +from __future__ import annotations + +import sqlite3 + +import pytest + +from core.wishlist import routes + + +class _DB: + def __init__(self, artists): + self._conn = sqlite3.connect(":memory:") + self._conn.execute("CREATE TABLE artists (name TEXT, thumb_url TEXT)") + self._conn.executemany("INSERT INTO artists VALUES (?, ?)", artists) + self._conn.commit() + + def _get_connection(self): + return self._conn + + +@pytest.fixture(autouse=True) +def _stub_normalize(monkeypatch): + # Deterministic stand-in for the real Plex/Jellyfin URL rebuild. + monkeypatch.setattr(routes, "normalize_image_url", lambda u: f"PROXY({u})") + + +def test_needs_image_fix_predicate(): + assert routes._needs_image_fix("/library/metadata/1/thumb/2") is True + assert routes._needs_image_fix("/Items/x/Images/Primary") is True + assert routes._needs_image_fix("http://localhost:32400/library/x") is True + assert routes._needs_image_fix("https://i.scdn.co/image/ab") is False + assert routes._needs_image_fix("https://is1.mzstatic.com/600x600bb.jpg") is False + assert routes._needs_image_fix("") is False + assert routes._needs_image_fix(None) is False + + +def test_relative_album_image_is_normalized(): + tracks = [{"artist_name": "A", + "spotify_data": {"album": {"images": [{"url": "/library/metadata/9/thumb/1"}]}}}] + routes._enrich_wishlist_images(tracks, _DB([])) + assert tracks[0]["spotify_data"]["album"]["images"][0]["url"] == "PROXY(/library/metadata/9/thumb/1)" + + +def test_cdn_album_image_is_left_untouched(): + """Items that already render must not change β€” guards against regressing normal wishlist art.""" + url = "https://i.scdn.co/image/ab67616d" + tracks = [{"artist_name": "A", "spotify_data": {"album": {"images": [{"url": url}]}}}] + routes._enrich_wishlist_images(tracks, _DB([])) + assert tracks[0]["spotify_data"]["album"]["images"][0]["url"] == url + + +def test_builds_artist_image_map_from_library(): + tracks = [ + {"artist_name": "Modest Mouse", "spotify_data": {"album": {"images": []}}}, + {"artist_name": "Unknown Artist", "spotify_data": {}}, # skipped + ] + db = _DB([("Modest Mouse", "/library/metadata/111/thumb/9"), + ("Other Band", "/library/metadata/222/thumb/9")]) # not in wishlist β†’ not returned + amap = routes._enrich_wishlist_images(tracks, db) + assert amap == {"modest mouse": "PROXY(/library/metadata/111/thumb/9)"} + + +def test_artist_with_empty_thumb_is_omitted(): + tracks = [{"artist_name": "NoArt", "spotify_data": {"album": {"images": []}}}] + amap = routes._enrich_wishlist_images(tracks, _DB([("NoArt", "")])) + assert amap == {} + + +def test_already_proxied_artist_thumb_not_double_wrapped(): + """A library thumb that's already a CDN/proxy URL passes through unchanged (idempotent).""" + tracks = [{"artist_name": "B", "spotify_data": {"album": {"images": []}}}] + amap = routes._enrich_wishlist_images(tracks, _DB([("B", "https://i.scdn.co/image/cdn")])) + assert amap == {"b": "https://i.scdn.co/image/cdn"} + + +def test_handles_string_or_missing_spotify_data_gracefully(): + tracks = [ + {"artist_name": "A", "spotify_data": "not-a-dict"}, + {"artist_name": "B"}, + {"spotify_data": {"album": {"images": [{"url": "/library/x"}]}}}, # no artist_name + ] + amap = routes._enrich_wishlist_images(tracks, _DB([("A", "/library/a")])) + # third track's relative album image still gets fixed + assert tracks[2]["spotify_data"]["album"]["images"][0]["url"] == "PROXY(/library/x)" + assert amap == {"a": "PROXY(/library/a)"} diff --git a/web_server.py b/web_server.py index 84713700..f965741f 100644 --- a/web_server.py +++ b/web_server.py @@ -40,7 +40,7 @@ logger = setup_logging(_log_level, _log_path) # App version β€” single source of truth for backup metadata, system-info, update check, etc. # Semver: MAJOR.MINOR.PATCH. Bump at each devβ†’main release. -_SOULSYNC_BASE_VERSION = "2.7.8" +_SOULSYNC_BASE_VERSION = "2.8.0" def _build_version_string(): """Append short commit hash to version when available (e.g. 2.35+abc1234).""" @@ -2908,9 +2908,15 @@ def _build_system_stats(): uptime_seconds = time.time() - start_time uptime = str(timedelta(seconds=int(uptime_seconds))) - # Get memory usage + # Get memory usage β€” global system %, plus SoulSync's own resident memory (RSS), + # so the dashboard can show "system load" and "how much RAM SoulSync itself uses". memory = psutil.virtual_memory() memory_usage = f"{memory.percent}%" + try: + _proc_rss_mb = psutil.Process(os.getpid()).memory_info().rss / (1024 * 1024) + process_memory = f"{_proc_rss_mb:.0f} MB" if _proc_rss_mb < 1024 else f"{_proc_rss_mb / 1024:.1f} GB" + except Exception: + process_memory = None # Count active downloads from download_batches (batches that are currently downloading) active_downloads = len([batch_id for batch_id, batch_data in download_batches.items() @@ -2988,7 +2994,8 @@ def _build_system_stats(): 'download_speed': download_speed_str, 'active_syncs': active_syncs, 'uptime': uptime, - 'memory_usage': memory_usage + 'memory_usage': memory_usage, + 'process_memory': process_memory } @app.route('/api/system/stats') @@ -3049,6 +3056,61 @@ def debug_memory_stop(): return jsonify({'error': str(e)}), 500 +@app.route('/api/debug/memory/objects') +def debug_memory_objects(): + """One-shot memory breakdown by live object type (plain gc β€” NO tracemalloc, so it + won't lock up a loaded app). Hit this once when RSS is high to see which type/cache + dominates: a big 'count' reveals accumulation (a cache that never evicts); a big + 'mb' under bytes/str reveals blob retention. Pinpoints the leak without tracing.""" + import gc + import sys + from collections import defaultdict + try: + gc.collect() + objs = gc.get_objects() + counts = defaultdict(int) + sizes = defaultdict(int) + biggest = [] + for o in objs: + tn = type(o).__name__ + counts[tn] += 1 + try: + sz = sys.getsizeof(o) + except Exception: + sz = 0 + sizes[tn] += sz + if sz > 1_000_000 and isinstance(o, (dict, list, set, frozenset, bytes, bytearray, str, tuple)): + try: + biggest.append((sz, tn, len(o))) + except Exception: # noqa: S110 β€” debug stat, len() on an exotic obj is non-fatal + pass + biggest.sort(reverse=True) + rss = None + try: + import psutil + rss = round(psutil.Process(os.getpid()).memory_info().rss / (1024 * 1024), 1) + except Exception: # noqa: S110 β€” psutil optional; rss stays None in the debug payload + pass + return jsonify({ + 'rss_mb': rss, + 'total_objects': len(objs), + 'top_by_size_mb': [ + {'type': t, 'count': counts[t], 'mb': round(sizes[t] / (1024 * 1024), 1)} + for t, _ in sorted(sizes.items(), key=lambda x: x[1], reverse=True)[:20] + ], + 'top_by_count': [ + {'type': t, 'count': c, 'mb': round(sizes[t] / (1024 * 1024), 1)} + for t, c in sorted(counts.items(), key=lambda x: x[1], reverse=True)[:20] + ], + 'biggest_containers': [ + {'mb': round(s / (1024 * 1024), 1), 'type': t, 'len': ln} + for s, t, ln in biggest[:15] + ], + }) + except Exception as e: + return jsonify({'error': str(e)}), 500 + + @app.route('/api/activity/feed') def get_activity_feed(): """Get recent activity feed for dashboard""" @@ -7604,13 +7666,13 @@ def manual_search_for_task(task_id): "error": error, }) + "\n" continue - # Pasted-link exact match: bubble the track whose id matches - # the link to the top so the user sees the exact version - # first (graceful no-op if ids don't line up). + # Pasted-link exact match: bubble the track whose source id + # matches the link to the top so the user sees the exact version + # first. Reads _source_metadata['track_id'] (TrackResult has no + # top-level id) β€” the old getattr(t,'id') always missed (#932). if src_name == link_source and link_track_id and tracks: - tracks = sorted( - tracks, - key=lambda t: str(getattr(t, 'id', '')) != str(link_track_id)) + from core.downloads.track_link import bubble_linked_track_first + tracks = bubble_linked_track_first(tracks, link_track_id) serialized = [] for t in tracks: s = _serialize_candidate(t, source_override=src_name) @@ -8229,6 +8291,46 @@ def delete_verification_item(history_id): return jsonify({"success": False, "error": str(e)}), 500 +@app.route('/api/verification/clean-orphans', methods=['POST']) +@admin_only +def clean_orphan_verification_items(): + """Remove dead review-queue rows whose file no longer exists anywhere + (deleted / replaced / re-downloaded elsewhere). These are append-only + library_history rows that can never be healed β€” there's no file left to + confirm β€” so they linger in the Unverified list forever (#934). + + User-initiated only, never automatic: it does a filesystem check, which + would mass-false-positive if the library mount were down. The pure helper + flags that signature (every reviewed file unreachable) and we refuse. Only + history ROWS are deleted β€” the files are already gone; this never removes a + file. Admin-only: it mutates shared review state.""" + try: + from core.downloads.orphan_history import find_orphan_history_ids + db = get_database() + # get_library_history_unverified() returns unverified + force_imported rows. + # Check ALL of them so the mount-down gate sees the true count, but only + # DELETE 'unverified' orphans β€” 'force_imported' is a deliberate user + # decision (accepted a version mismatch) and stays for human approval. + rows = db.get_library_history_unverified() or [] + result = find_orphan_history_ids( + rows, _resolve_history_audio_path, + deletable=lambda r: r.get('verification_status') == 'unverified') + if result['suspicious']: + return jsonify({ + "success": False, + "error": "Every reviewed file is unreachable β€” your library may be " + "offline right now. Nothing was removed.", + }), 409 + orphan_ids = result['orphan_ids'] + removed = db.delete_library_history_rows(orphan_ids) if orphan_ids else 0 + logger.info("[Verification] Cleaned %d orphaned review rows (checked %d)", + removed, result['checked']) + return jsonify({"success": True, "removed": removed, "checked": result['checked']}) + except Exception as e: + logger.error(f"[Verification] Clean orphans failed: {e}") + return jsonify({"success": False, "error": str(e)}), 500 + + @app.route('/api/quarantine//recover', methods=['POST']) def recover_quarantine_item(entry_id): """Fallback for legacy thin sidecars: move file into Staging so the user @@ -9723,6 +9825,27 @@ def download_discography(artist_id): except Exception as e: logger.debug("active media server lookup failed: %s", e) + # Pre-fetch the artist's owned library tracks ONCE so the per-track + # ownership check scores in-memory instead of firing fuzzy SQL scans + # against the whole library for every track (which, on a large library + # and an artist the user owns nothing of, was ~15-30s PER TRACK β€” every + # title/artist variation fell through to a full-table fuzzy fallback). + # Same batched path the discography backfill job + completion-stream use. + # Crucially we pass an empty list (not None) when nothing is owned, so the + # owns-nothing case still takes the fast in-memory path β†’ instant. + owned_candidate_tracks = [] + try: + cand_albums = db.get_candidate_albums_for_artist( + artist_name, server_source=active_server + ) + if cand_albums: + owned_candidate_tracks = db.get_candidate_tracks_for_albums( + [a.id for a in cand_albums] + ) or [] + except Exception as _cand_err: + logger.debug("Discography: candidate pre-fetch failed for %s: %s", artist_name, _cand_err) + owned_candidate_tracks = [] + total_added = 0 total_skipped = 0 total_skipped_artist = 0 @@ -9812,7 +9935,8 @@ def download_discography(artist_id): # Same library-ownership check the discography # backfill repair job uses. Format-agnostic so # Blasphemy mode (FLACβ†’MP3) doesn't false-miss. - if track_already_owned(db, track_name, hint_artist, album_name, active_server): + if track_already_owned(db, track_name, hint_artist, album_name, active_server, + candidate_tracks=owned_candidate_tracks): skipped_owned += 1 continue @@ -19049,10 +19173,17 @@ def get_batch_history(): @app.route('/api/downloads/clear-completed', methods=['POST']) def clear_completed_downloads(): - """Remove completed/failed/cancelled tasks from the download tracker.""" + """Clear completed/failed downloads from the Downloads page: the live + session tasks AND the persisted download-history tail (so the list actually + empties and stays empty across restart). Rows still awaiting verification + (unverified / force_imported) are preserved β€” they belong to the review + queue, not this cleanup.""" try: cleared = _downloads_cancel.clear_completed_local() - return jsonify({'success': True, 'cleared': cleared}) + history_cleared = get_database().clear_completed_download_history() + return jsonify({'success': True, 'cleared': cleared, + 'history_cleared': history_cleared, + 'total_cleared': cleared + history_cleared}) except Exception as e: logger.error(f"Error clearing completed downloads: {e}") return jsonify({'success': False, 'error': str(e)}), 500 @@ -20881,6 +21012,56 @@ def _save_sync_status_file(sync_statuses): except Exception as e: logger.error(f"Error saving sync status: {e}") +def _sync_status_timestamp(status_info): + """Return comparable timestamp for a persisted sync-status record.""" + if not status_info or 'last_synced' not in status_info: + return None + try: + return datetime.fromisoformat(status_info['last_synced']) + except (TypeError, ValueError): + return None + +def _latest_sync_status(*status_infos): + """Pick the newest non-empty sync-status record.""" + candidates = [s for s in status_infos if s] + if not candidates: + return {} + return max(candidates, key=lambda s: _sync_status_timestamp(s) or datetime.min) + +def _mirrored_spotify_sync_status(playlist_id, sync_statuses, *, database=None, profile_id=None): + """Return auto-sync status for a Spotify playlist mirrored into SoulSync.""" + try: + db = database or get_database() + profile = profile_id if profile_id is not None else get_current_profile_id() + mirrored = db.get_mirrored_playlist_by_source('spotify', str(playlist_id), profile) + if not mirrored: + return {} + return sync_statuses.get(f"auto_mirror_{mirrored.get('id')}", {}) + except Exception as e: + logger.debug("Spotify mirrored sync-status lookup failed for %s: %s", playlist_id, e) + return {} + +def _resolve_spotify_playlist_sync_status(playlist_id, sync_statuses, *, database=None, profile_id=None): + """Resolve direct or mirrored sync status for a Spotify playlist card.""" + direct_status = sync_statuses.get(playlist_id, {}) + mirrored_status = _mirrored_spotify_sync_status( + playlist_id, + sync_statuses, + database=database, + profile_id=profile_id, + ) + return _latest_sync_status(direct_status, mirrored_status) + +def _format_playlist_sync_status(status_info, playlist_snapshot): + """Build user-facing sync-status text from persisted status + snapshot.""" + if 'last_synced' not in status_info: + return "Never Synced" + last_sync_time = datetime.fromisoformat(status_info['last_synced']).strftime('%b %d, %H:%M') + stored_snapshot = status_info.get('snapshot_id') + if stored_snapshot and playlist_snapshot and playlist_snapshot != stored_snapshot: + return f"Last Sync: {last_sync_time}" + return f"Synced: {last_sync_time}" + def _update_and_save_sync_status(playlist_id, playlist_name, playlist_owner, snapshot_id, **kwargs): """Updates the sync status for a given playlist and saves to file (same logic as GUI).""" try: @@ -20923,16 +21104,14 @@ def get_spotify_playlists(): # Add regular playlists first for p in playlists: - status_info = sync_statuses.get(p.id, {}) - sync_status = "Never Synced" + status_info = _resolve_spotify_playlist_sync_status(p.id, sync_statuses) # Handle snapshot_id safely - may not exist in core Playlist class playlist_snapshot = getattr(p, 'snapshot_id', '') + sync_status = _format_playlist_sync_status(status_info, playlist_snapshot) if 'last_synced' in status_info: stored_snapshot = status_info.get('snapshot_id') - last_sync_time = datetime.fromisoformat(status_info['last_synced']).strftime('%b %d, %H:%M') - if playlist_snapshot != stored_snapshot: - sync_status = f"Last Sync: {last_sync_time}" + if stored_snapshot and playlist_snapshot and playlist_snapshot != stored_snapshot: logger.info( "Playlist sync status: name=%s id=%s snapshot=%r stored_snapshot=%r result=Needs Sync display=%s", p.name, @@ -20942,7 +21121,6 @@ def get_spotify_playlists(): sync_status, ) else: - sync_status = f"Synced: {last_sync_time}" logger.info( "Playlist sync status: name=%s id=%s snapshot=%r stored_snapshot=%r result=Synced display=%s", p.name, @@ -28247,22 +28425,10 @@ def start_watchlist_scan(): # #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 [], + from core.watchlist.scan_history import persist_scan_run + persist_scan_run( + get_database(), watchlist_scan_state, + profile_id=scan_profile_id, was_cancelled=was_cancelled, ) except Exception as _hist_err: logger.error(f"Failed to persist watchlist scan run: {_hist_err}") @@ -35124,6 +35290,37 @@ def get_discovery_pool(): logger.error(f"Error getting discovery pool: {e}") return jsonify({"error": str(e)}), 500 +@app.route('/api/wing-it-pool', methods=['GET']) +def get_wing_it_pool(): + """List Wing It auto-matched tracks (unverified best-effort guesses), optionally per-playlist. + + These are tracks that couldn't match a metadata source and got a raw-name Wing It stub. They + count as 'discovered' so the Discovery Pool hides them β€” this surfaces them so the user can + verify and re-match. Re-matching reuses the Discovery Pool's /api/discovery-pool/fix endpoint + (both key off the mirrored_playlist_tracks.id), and a manual match drops the track from here. + """ + try: + database = get_database() + profile_id = get_current_profile_id() + playlist_id = request.args.get('playlist_id', type=int) + + tracks = database.get_wing_it_pool(profile_id=profile_id, playlist_id=playlist_id) + matched = database.get_wing_it_pool(profile_id=profile_id, playlist_id=playlist_id, resolved=True) + stats = database.get_wing_it_pool_stats(profile_id=profile_id) + + playlists = database.get_mirrored_playlists(profile_id=profile_id) + playlist_options = [{'id': p['id'], 'name': p['name']} for p in playlists] + + return jsonify({ + 'tracks': tracks, + 'matched': matched, + 'stats': stats, + 'playlists': playlist_options, + }) + except Exception as e: + logger.error(f"Error getting wing it pool: {e}") + return jsonify({"error": str(e)}), 500 + @app.route('/api/discovery-pool/fix', methods=['POST']) def fix_discovery_pool_track(): """Manually fix a failed discovery by linking a mirrored track to a Spotify/iTunes result.""" @@ -35167,7 +35364,8 @@ def fix_discovery_pool_track(): 'source': 'spotify', } - # Update the mirrored track's extra_data + # Update the mirrored track's extra_data (merges, so a wing-it track keeps its + # wing_it_fallback flag β€” that + manual_match is how the Wing It Pool lists resolved guesses). extra_data = { 'discovered': True, 'provider': 'spotify', @@ -38491,6 +38689,58 @@ def start_runtime_services(): # Initialize app start time for uptime tracking app.start_time = time.time() + # Growth-triggered garbage collection (#802 / resource usage). plexapi builds large XML + # Element trees whose nodes reference each other in cycles; Python's generational GC + # parks those in gen2 and sweeps it rarely, so they accumulate and RSS climbs (measured: + # ~300MB fresh -> 1.8-2.2GB after browsing every page, most of it reclaimable cyclic + # garbage a full gc.collect() frees instantly). A FIXED timer overshoots β€” browsing piles + # garbage faster than once-a-minute catches it. Instead, poll RSS cheaply and collect as + # soon as it has grown GROW_MB since the last sweep, so it kicks in DURING a heavy browse + # and caps the peak near (floor + GROW_MB) instead of letting it run to 2GB+. A backstop + # interval still collects slow accumulation when idle. + def _growth_triggered_gc(): + import gc + CHECK_SECONDS = 8 # cheap RSS poll cadence + GROW_MB = 200 # collect once RSS climbs this much since the last sweep + BACKSTOP_SECONDS = 120 # ...or at least this often regardless + try: + import psutil + proc = psutil.Process(os.getpid()) + rss_mb = lambda: proc.memory_info().rss / (1024 * 1024) + except Exception: + rss_mb = lambda: 0.0 + # glibc malloc_trim hands freed arenas back to the OS β€” WITHOUT it gc.collect() frees + # the Python objects but glibc hoards the memory, so RSS never drops and the peak still + # runs to 2GB+. Best-effort: absent on musl/Alpine or non-Linux, where we just skip it + # (gc still helps, RSS just sticks closer to the high-water mark there). + _trim = None + try: + import ctypes + import ctypes.util + _libc = ctypes.CDLL(ctypes.util.find_library('c') or 'libc.so.6') + if hasattr(_libc, 'malloc_trim'): + _trim = _libc.malloc_trim + except Exception: # noqa: S110 β€” malloc_trim absent on musl/non-Linux; just skip it + pass + floor = rss_mb() + last = time.time() + while True: + time.sleep(CHECK_SECONDS) + try: + if (rss_mb() - floor) >= GROW_MB or (time.time() - last) >= BACKSTOP_SECONDS: + gc.collect() + if _trim is not None: + try: + _trim(0) + except Exception: # noqa: S110 β€” best-effort OS reclaim, never fatal + pass + floor = rss_mb() + last = time.time() + except Exception: # noqa: S110 β€” best-effort housekeeping, never crash the app + pass + threading.Thread(target=_growth_triggered_gc, daemon=True, name='gc-sweeper').start() + logger.info("GC sweeper started (collect + malloc_trim on +200MB growth, backstop 120s)") + # Register action handlers and start automation engine _register_automation_handlers() if automation_engine: diff --git a/webui/index.html b/webui/index.html index 46df8390..19c515dd 100644 --- a/webui/index.html +++ b/webui/index.html @@ -2048,6 +2048,7 @@

Mirrored Playlists

+
@@ -6411,10 +6412,10 @@
- Animated particle effects behind each page. Disable to reduce GPU usage. + Animated particle effects behind each page. Off by default β€” enable for the eye candy (uses GPU).