diff --git a/.github/workflows/docker-publish.yml b/.github/workflows/docker-publish.yml
index 91daceac..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.9)'
+ description: 'Version tag (e.g. 2.8.0)'
required: true
- default: '2.7.9'
+ default: '2.8.0'
jobs:
build-and-push:
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/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/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 dd37aee3..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
@@ -13795,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:
@@ -13857,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 c2e865ac..19659f76 100644
--- a/pr_description.md
+++ b/pr_description.md
@@ -1,49 +1,44 @@
-# soulsync 2.7.9 — `dev` → `main`
+# soulsync 2.8.0 — `dev` → `main`
-a big one. the headline is the new **best-quality download system + a real quality profile**, plus a much smarter **Discover** page, a new **Wing It Pool**, a redesigned **Auto-Sync** board, and a pile of reported fixes (multi-disc albums, playlist sync labels, the import-vs-quarantine race).
+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
-### best-quality downloads + a real quality profile
-downloads are now driven by a **ranked-target quality profile** instead of a fixed preference. you order the formats you want (drag to reorder — FLAC 24/192 down to mp3, every format controllable, with "all lossless / all lossy" group shortcuts), and:
-- **best-quality search mode** pools candidates across *every* source per query and grabs the highest-quality copy that meets your profile — not just the first/fastest match. priority mode is still there, now with an opt-in **"rank-based download order"** toggle if you want quality-first ordering there too.
-- each streaming source's download tier is derived from the one global profile, so you set it once.
-- AAC is an opt-in tier; old per-source Hi-Res preferences migrate into the profile automatically.
+### 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.
-### quarantine, cleaned up + safer
-- the quarantine view is **consolidated into the Downloads page** as a filter (no separate place to check), with real audio quality shown on the rows and approve/retry handled inline.
-- **AcoustID fail-closed mode** (opt-in): only import tracks that actually verify, so a wrong file never lands in your library.
-- **silence + truncated-download guards** catch mostly-silent preview files and downloads that are shorter than their container claims, before they import.
-- a **library quality check** runs as a repair job and can flag files that are upgradeable to your preferred quality.
+### 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.)*
-### Discover got a lot smarter
-- **"Based On Your Listening"** — a new artist row, ranked from who you actually *play* the most (consensus + recency weighted), with a "because you listen to X, Y" reason on each card.
-- **"Your Listening Mix"** — a playable track playlist built from those artists' top tracks. works on **any** metadata source (falls back to Deezer's public API), not just Spotify.
-- **Fresh Tape** actually fills now — it was starving down to 5–10 tracks because future-dated albums ate the candidate budget.
-- the **SoulSync Discovery** tab on the Sync page now lists *every* playlist kind (incl. the Listening Mix) so you can mirror + auto-sync them.
+### 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.)*
-### Wing It Pool
-a new button next to **Discovery Pool** on the Mirrored Playlists tab. Wing It auto-matches tracks it couldn't match to metadata on a best-effort guess — those were invisible until now. the Wing It Pool opens to a two-card view (**guesses to review** + **resolved**) so you can verify or re-match what it guessed.
-
-### Auto-Sync Manager redesign
-the scheduling board no longer scrolls sideways through a wall of columns. intervals (hourly) and days (weekly) are now **horizontal lanes** — empty ones collapse, busy ones grow, and the scroll position holds when you add a playlist.
+### 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).
---
## fixes
-- **multi-disc albums showed disc-2 tracks as "missing" / under disc 1 (#927)** — the library scan never read the disc number, so every track was stored as disc 1. it now captures the real disc from Jellyfin/Plex/Navidrome at scan time. *(re-scan your library once to backfill existing tracks.)*
-- **playlists always said "Never Synced" (#925)** — auto-synced/mirrored playlists were only checked against the direct-sync status, never their auto-sync status. fixed (thanks @ramonskie).
-- **tracks imported while quarantined / shown "completed" (#928)** — a race let both the browser poll and the download monitor post-process the same finished download. an atomic claim now ensures exactly one path handles it (thanks @nick2000713).
-- **library card badges hijacked the click** — clicking the watchlist eye or a source badge on an artist card also opened the artist detail page (and the badge's own link). badges now do only their own thing.
+- **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.
---
-## under the hood
-- music automations page no longer shows video-app automations (they live in the shared engine DB).
-- quality-settings tile tidied up — collapsible ⓘ help instead of walls of text, proper reset button, dropped the redundant per-source "quality is global" notes.
-- download clients don't crash on init when the download path can't be created.
+## performance + UI
+
+- **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_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_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_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 b87f2ba7..15b5de83 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.9"
+_SOULSYNC_BASE_VERSION = "2.8.0"
def _build_version_string():
"""Append short commit hash to version when available (e.g. 2.35+abc1234)."""
@@ -2837,9 +2837,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()
@@ -2917,7 +2923,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')
@@ -2978,6 +2985,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"""
@@ -7533,13 +7595,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)
@@ -8158,6 +8220,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
@@ -9652,6 +9754,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
@@ -9741,7 +9864,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
@@ -18978,10 +19102,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
@@ -28223,22 +28354,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}")
@@ -38499,6 +38618,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 30925a59..cfc502c5 100644
--- a/webui/index.html
+++ b/webui/index.html
@@ -6263,10 +6263,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).