diff --git a/core/repair_jobs/duplicate_detector.py b/core/repair_jobs/duplicate_detector.py index 2b901b25..80b93bee 100644 --- a/core/repair_jobs/duplicate_detector.py +++ b/core/repair_jobs/duplicate_detector.py @@ -1,9 +1,11 @@ """Duplicate Track Detector Job — finds potential duplicate tracks in the library.""" +import os import re from collections import defaultdict from difflib import SequenceMatcher +from core.imports.file_ops import _strip_slskd_dedup_suffix from core.repair_jobs import register_job from core.repair_jobs.base import JobContext, JobResult, RepairJob from utils.logging_config import get_logger @@ -106,115 +108,60 @@ class DuplicateDetectorJob(RepairJob): # Find duplicates within each bucket found_groups = set() # Track IDs already in a group - processed = 0 + processed_holder = {'count': 0} if context.report_progress: context.report_progress(phase=f'Comparing {total} tracks...', total=total) + # Pass 1 — bucket by normalized-title prefix (existing behavior). for _bucket_key, bucket_tracks in buckets.items(): if context.check_stop(): return result + self._scan_bucket( + bucket_tracks=bucket_tracks, + require_metadata_match=True, + title_threshold=title_threshold, + artist_threshold=artist_threshold, + ignore_cross_album=ignore_cross_album, + found_groups=found_groups, + processed_holder=processed_holder, + total=total, + result=result, + context=context, + ) - for i, t1 in enumerate(bucket_tracks): - if context.check_stop(): - return result - - processed += 1 - result.scanned += 1 - - if context.report_progress and processed % 100 == 0: - context.report_progress( - scanned=processed, total=total, - phase=f'Comparing {processed} / {total}', - log_line=f'Checking: {t1["title"]} — {t1["artist"]}', - log_type='info' - ) - - if t1['id'] in found_groups: - continue - - group = [t1] - - for j in range(i + 1, len(bucket_tracks)): - t2 = bucket_tracks[j] - if t2['id'] in found_groups: - continue - - # Compare titles - title_sim = SequenceMatcher(None, t1['norm_title'], t2['norm_title']).ratio() - if title_sim < title_threshold: - continue - - # Compare artists - artist_sim = SequenceMatcher(None, t1['norm_artist'], t2['norm_artist']).ratio() - if artist_sim < artist_threshold: - continue - - # Skip cross-album duplicates — same song on different albums is intentional - if ignore_cross_album and t1['album'] and t2['album'] and t1['album'] != t2['album']: - continue - - # Skip pairs that are the same physical file mounted at - # different roots — e.g. /app/Transfer/... and /media/Music/... - # when the user binds the same host directory into both - # SoulSync and Plex containers. Both rows end up in the DB - # (one from SoulSync's local scan, one from Plex's sync), - # but they point at one file on disk. - if _is_same_physical_file( - t1['file_path'], t2['file_path'], - t1['duration'], t2['duration'], - ): - continue - - group.append(t2) - - if len(group) >= 2: - # Found a duplicate group - for t in group: - found_groups.add(t['id']) - - if context.report_progress: - context.report_progress( - log_line=f'Duplicate: {t1["title"]} — {len(group)} copies', - log_type='skip' - ) - - if context.create_finding: - try: - # Sort group by quality (highest bitrate first) - group.sort(key=lambda t: (t['bitrate'] or 0), reverse=True) - - context.create_finding( - job_id=self.job_id, - finding_type='duplicate_tracks', - severity='info', - entity_type='track', - entity_id=str(group[0]['id']), - file_path=group[0]['file_path'], - title=f'Duplicate: {group[0]["title"]} by {group[0]["artist"]}', - description=f'{len(group)} copies found with similar title/artist', - details={ - 'tracks': [{ - 'id': t['id'], - 'title': t['title'], - 'artist': t['artist'], - 'album': t['album'], - 'file_path': t['file_path'], - 'bitrate': t['bitrate'], - 'duration': t['duration'], - } for t in group], - 'count': len(group), - 'album_thumb_url': group[0].get('album_thumb_url'), - 'artist_thumb_url': group[0].get('artist_thumb_url'), - } - ) - result.findings_created += 1 - except Exception as e: - logger.debug("Error creating duplicate finding: %s", e) - result.errors += 1 - - if context.update_progress and processed % 200 == 0: - context.update_progress(processed, total) + # Pass 2 — re-bucket leftover tracks by canonical filename stem + # (slskd dedup suffix stripped). Catches dupes whose tag metadata + # disagrees because some copies were never properly tagged after + # download — e.g. ``Song.flac`` and ``Song_<19-digit-ts>.flac`` + # land in the library with identical filenames sans the slskd + # dedup tail but get inconsistent ID3 titles from the media-server + # rescan. Pass-1 buckets them apart by title so they never get + # compared. Discord-reported scenario: 7 copies of one OST track + # accumulating in one folder, only 1 caught by the detector. + filename_buckets = self._build_filename_buckets( + buckets=buckets, + found_groups=found_groups, + ) + for _fname_key, fname_tracks in filename_buckets.items(): + if context.check_stop(): + return result + # Filename match is itself strong evidence — a shared canonical + # stem means the files came from the same source download. + # Drop the metadata gates so dedup orphans get caught even + # when their tag titles disagree. + self._scan_bucket( + bucket_tracks=fname_tracks, + require_metadata_match=False, + title_threshold=title_threshold, + artist_threshold=artist_threshold, + ignore_cross_album=ignore_cross_album, + found_groups=found_groups, + processed_holder=processed_holder, + total=total, + result=result, + context=context, + ) if context.update_progress: context.update_progress(total, total) @@ -223,6 +170,142 @@ class DuplicateDetectorJob(RepairJob): result.scanned, result.findings_created) return result + def _scan_bucket( + self, + *, + bucket_tracks, + require_metadata_match, + title_threshold, + artist_threshold, + ignore_cross_album, + found_groups, + processed_holder, + total, + result, + context, + ) -> None: + """Compare every pair within a bucket; emit duplicate groups. + + ``require_metadata_match`` gates the title / artist similarity + thresholds and the cross-album guard. Pass ``False`` for buckets + whose grouping is already strong evidence (e.g. shared canonical + filename) so that dedup orphans with broken / missing tags still + get caught. + """ + for i, t1 in enumerate(bucket_tracks): + if context.check_stop(): + return + + processed_holder['count'] += 1 + result.scanned += 1 + processed = processed_holder['count'] + + if context.report_progress and processed % 100 == 0: + context.report_progress( + scanned=processed, total=total, + phase=f'Comparing {processed} / {total}', + log_line=f'Checking: {t1["title"]} — {t1["artist"]}', + log_type='info' + ) + + if t1['id'] in found_groups: + continue + + group = [t1] + + for j in range(i + 1, len(bucket_tracks)): + t2 = bucket_tracks[j] + if t2['id'] in found_groups: + continue + + if require_metadata_match: + title_sim = SequenceMatcher(None, t1['norm_title'], t2['norm_title']).ratio() + if title_sim < title_threshold: + continue + artist_sim = SequenceMatcher(None, t1['norm_artist'], t2['norm_artist']).ratio() + if artist_sim < artist_threshold: + continue + if ignore_cross_album and t1['album'] and t2['album'] and t1['album'] != t2['album']: + continue + + if _is_same_physical_file( + t1['file_path'], t2['file_path'], + t1['duration'], t2['duration'], + ): + continue + + group.append(t2) + + if len(group) >= 2: + for t in group: + found_groups.add(t['id']) + + if context.report_progress: + context.report_progress( + log_line=f'Duplicate: {t1["title"]} — {len(group)} copies', + log_type='skip' + ) + + if context.create_finding: + try: + group.sort(key=lambda t: (t['bitrate'] or 0), reverse=True) + context.create_finding( + job_id=self.job_id, + finding_type='duplicate_tracks', + severity='info', + entity_type='track', + entity_id=str(group[0]['id']), + file_path=group[0]['file_path'], + title=f'Duplicate: {group[0]["title"]} by {group[0]["artist"]}', + description=f'{len(group)} copies found with similar title/artist', + details={ + 'tracks': [{ + 'id': t['id'], + 'title': t['title'], + 'artist': t['artist'], + 'album': t['album'], + 'file_path': t['file_path'], + 'bitrate': t['bitrate'], + 'duration': t['duration'], + } for t in group], + 'count': len(group), + 'album_thumb_url': group[0].get('album_thumb_url'), + 'artist_thumb_url': group[0].get('artist_thumb_url'), + } + ) + result.findings_created += 1 + except Exception as e: + logger.debug("Error creating duplicate finding: %s", e) + result.errors += 1 + + if context.update_progress and processed_holder['count'] % 200 == 0: + context.update_progress(processed_holder['count'], total) + + def _build_filename_buckets(self, *, buckets, found_groups): + """Re-bucket all tracks by canonical filename stem. + + The slskd dedup suffix (``_<19+ digit timestamp>``) is stripped so + ``Song.flac`` and ``Song_639122324339578022.flac`` collapse to the + same key. Singleton buckets (only one track) are dropped — they + carry no comparison value. + """ + filename_buckets = defaultdict(list) + for bucket_tracks in buckets.values(): + for track in bucket_tracks: + if track['id'] in found_groups: + continue + fp = track.get('file_path') or '' + if not fp: + continue + basename = os.path.basename(str(fp).replace('\\', '/')) + stem, ext = os.path.splitext(basename) + if not stem: + continue + canonical = _strip_slskd_dedup_suffix(stem) + key = (canonical.lower(), ext.lower()) + filename_buckets[key].append(track) + return {k: v for k, v in filename_buckets.items() if len(v) >= 2} + def _get_settings(self, context: JobContext) -> dict: if not context.config_manager: return self.default_settings.copy() diff --git a/core/watchlist_scanner.py b/core/watchlist_scanner.py index 47ee120c..0820aef3 100644 --- a/core/watchlist_scanner.py +++ b/core/watchlist_scanner.py @@ -9,6 +9,7 @@ from datetime import datetime, timezone, timedelta from dataclasses import dataclass import re import time +from difflib import SequenceMatcher import requests from bs4 import BeautifulSoup from database.music_database import get_database, WatchlistArtist @@ -288,6 +289,111 @@ def is_compilation_album(album_name: str) -> bool: return False +# Common qualifying parentheticals appended to album names by Spotify / +# Deezer / iTunes / Discogs that the user's media server (Plex / Navidrome / +# Jellyfin) typically strips out of the file tags. Without normalization, +# fuzzy-comparing the two sides reports a false "different album" verdict — +# the watchlist scanner then thinks the track is missing and re-downloads +# it on every scan. +_ALBUM_QUALIFIER_PATTERNS = [ + r'\bmusic\s+from(?:\s+the)?(?:\s+motion\s+picture)?\b', + r'\boriginal\s+(?:motion\s+picture\s+)?(?:soundtrack|score)\b', + r'\bsoundtrack(?:\s+from(?:\s+the)?(?:\s+motion\s+picture)?)?\b', + r'\bo\.?s\.?t\.?\b', + r'\bdeluxe(?:\s+(?:edition|version))?\b', + r'\bexpanded(?:\s+edition)?\b', + r'\bremaster(?:ed)?(?:\s+(?:\d{4}|edition))?\b', + r'\banniversary(?:\s+edition)?\b', + r'\bspecial\s+edition\b', + r'\bbonus\s+(?:track\s+)?(?:edition|version)\b', + r'\bextended(?:\s+(?:edition|version))?\b', + r'\bexplicit\b', + r'\bclean\s+version\b', +] +_ALBUM_QUALIFIER_RE = re.compile( + '|'.join(_ALBUM_QUALIFIER_PATTERNS), + re.IGNORECASE, +) + + +def _normalize_album_for_match(name: str) -> str: + """Return a canonical form of an album name suitable for fuzzy comparison. + + Strips qualifying parentheticals (``(Music From The Motion Picture)``, + ``[Deluxe Edition]``, ``- Remastered 2011``, etc.) and any leftover + bracketed groups, lowercases, collapses whitespace. The output is meant + for comparison only — never display. + """ + if not name: + return "" + cleaned = name + # Strip the well-known qualifier phrases regardless of whether they + # sit in brackets, after a dash, or bare. + cleaned = _ALBUM_QUALIFIER_RE.sub(' ', cleaned) + # Then strip any other parenthesized / bracketed groups whatsoever — + # 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) + cleaned = re.sub(r'[^a-z0-9 ]+', ' ', cleaned.lower()) + cleaned = re.sub(r'\s+', ' ', cleaned).strip() + return cleaned + + +_VOLUME_MARKER_RE = re.compile( + r'\b(?:vol(?:ume)?|pt|part|disc|book|chapter|episode)\.?\s*(\d+)\b|\b(\d+)\s*$', + re.IGNORECASE, +) + + +def _extract_volume_marker(normalized_name: str): + """Pull the trailing volume / part / disc / standalone-number marker out + of a normalized album name. Used to reject ``"Greatest Hits Volume 1"`` + vs ``"Greatest Hits Volume 2"`` matches that would otherwise pass a + fuzzy ratio test on the heavily-shared prefix. + """ + if not normalized_name: + return None + matches = list(_VOLUME_MARKER_RE.finditer(normalized_name)) + if not matches: + return None + last = matches[-1] + return last.group(1) or last.group(2) + + +def _albums_likely_match(spotify_album: str, lib_album: str, threshold: float = 0.6) -> bool: + """Return True when two album names plausibly identify the same release. + + Designed to swallow naming drift between metadata sources and the + media-server tag scan: ``"Napoleon Dynamite (Music From The Motion + Picture)"`` vs ``"Napoleon Dynamite OST"`` should be the same album, + not two — otherwise the watchlist scanner downloads the track again + every 30 minutes. + """ + if not spotify_album or not lib_album: + return False + norm_a = _normalize_album_for_match(spotify_album) + norm_b = _normalize_album_for_match(lib_album) + if not norm_a or not norm_b: + return False + # Volume / part / disc markers must agree when both sides have one. + # Otherwise ``"Greatest Hits Volume 1"`` and ``"Greatest Hits Volume 2"`` + # would slip past every fuzzy threshold on the shared prefix. + vol_a = _extract_volume_marker(norm_a) + vol_b = _extract_volume_marker(norm_b) + if vol_a and vol_b and vol_a != vol_b: + 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 + return SequenceMatcher(None, norm_a, norm_b).ratio() >= threshold + + @dataclass class ScanResult: """Result of scanning a single artist""" @@ -1961,17 +2067,21 @@ class WatchlistScanner: db_track, confidence = self.database.check_track_exists(query_title, artist_name, confidence_threshold=0.7, server_source=active_server, album=search_album) if db_track and confidence >= 0.7: - # When allow_duplicates is on, only skip if the exact same album + # When allow_duplicates is on, only skip if we believe + # the library copy is on the same album the watchlist + # is asking about. Album name drift between Spotify + # and the media-server scan ("Napoleon Dynamite (Music + # From The Motion Picture)" vs "Napoleon Dynamite OST") + # used to fail a strict 0.85 fuzzy threshold and force + # an infinite redownload loop. if allow_duplicates and album_name: lib_album = getattr(db_track, 'album_title', '') or '' if lib_album: - from difflib import SequenceMatcher - album_sim = SequenceMatcher(None, album_name.lower(), lib_album.lower()).ratio() - if album_sim < 0.85: - logger.info(f"[AllowDup] Different album — allowing: '{original_title}' (wanted: '{album_name}', library: '{lib_album}', sim: {album_sim:.2f})") - continue # Different album — allow it + if _albums_likely_match(album_name, lib_album): + logger.info(f"[AllowDup] Album match — skipping: '{original_title}' (wanted: '{album_name}', library: '{lib_album}')") else: - logger.info(f"[AllowDup] Same album — skipping: '{original_title}' (wanted: '{album_name}', library: '{lib_album}', sim: {album_sim:.2f})") + logger.info(f"[AllowDup] Different album — allowing: '{original_title}' (wanted: '{album_name}', library: '{lib_album}')") + continue # Different album — allow it else: # No album info in library — can't compare, allow it logger.info(f"[AllowDup] No album info in library — allowing: '{original_title}'") diff --git a/tests/test_duplicate_detector_slskd_dedup.py b/tests/test_duplicate_detector_slskd_dedup.py new file mode 100644 index 00000000..c26fb5bb --- /dev/null +++ b/tests/test_duplicate_detector_slskd_dedup.py @@ -0,0 +1,263 @@ +"""Regression tests for duplicate-detector slskd dedup-suffix bucket pass. + +Discord-reported (Mushy): the watchlist re-downloaded the same OST track +seven times — every retry landed in the same album folder with a +slskd dedup tail (``Track_<19-digit-timestamp>.mp3``) appended by slskd +to avoid clobbering the prior copy. The library scan then ingested all +seven files. The duplicate detector found only one of them because the +title-prefix bucket compared on tag titles, which the media-server scan +sometimes parses inconsistently (or leaves blank) for files written by +slskd directly into a library folder. So the seven copies got bucketed +apart by their parsed titles and never compared. + +The fix adds a second pass: re-bucket the leftover tracks (the ones the +title pass didn't already group) by canonical filename stem with the +slskd dedup tail stripped. Files that share a canonical filename + same +extension are grouped without re-checking title/artist similarity — +filename agreement is itself strong evidence the files came from the +same source download. +""" + +from collections import defaultdict +from types import SimpleNamespace + +import pytest + +from core.repair_jobs.duplicate_detector import DuplicateDetectorJob + + +def _make_track( + track_id, + *, + title, + artist="John Swihart", + album="Various Artists - Napoleon Dynamite OST", + file_path="", + bitrate=320, + duration=180.0, +): + """Build the dict shape that `_scan_bucket` and `_build_filename_buckets` expect.""" + from core.repair_jobs.duplicate_detector import _normalize + + return { + 'id': track_id, + 'title': title, + 'norm_title': _normalize(title), + 'artist': artist, + 'norm_artist': _normalize(artist), + 'album': album, + 'file_path': file_path, + 'bitrate': bitrate, + 'duration': duration, + 'album_thumb_url': None, + 'artist_thumb_url': None, + } + + +class _FakeContext: + """Minimal JobContext stand-in for testing scan logic.""" + + def __init__(self): + self.findings = [] + self.create_finding = self._create_finding + self.report_progress = lambda **kw: None + self.update_progress = lambda *a, **kw: None + self.check_stop = lambda: False + + def _create_finding(self, **kwargs): + self.findings.append(kwargs) + + +# --------------------------------------------------------------------------- +# _build_filename_buckets — strips slskd dedup suffix and groups leftovers +# --------------------------------------------------------------------------- + + +class TestBuildFilenameBuckets: + def setup_method(self): + self.job = DuplicateDetectorJob() + + def test_strips_dedup_suffix_and_groups_orphans(self): + """The exact scenario: one canonical file plus six slskd dedup + siblings. All seven must collapse into one bucket.""" + base_dir = "/data/torrents/music/Various Artists - Napoleon Dynamite OST" + canonical = _make_track( + 1, title="Heres Rico Musiq", + file_path=f"{base_dir}/14-john_swihart-heres_rico-musiq.mp3", + ) + siblings = [ + _make_track( + i + 2, + title=f"Heres Rico Musiq Variant {i}", # different parsed title + file_path=f"{base_dir}/14-john_swihart-heres_rico-musiq_{ts}.mp3", + ) + for i, ts in enumerate([ + "639122324339578022", "639126674226945470", "639127689938113502", + "639130174269948395", "639132075826102610", "639132543802519474", + ]) + ] + all_tracks = [canonical, *siblings] + buckets = defaultdict(list) + # All in one synthetic title bucket — irrelevant, _build_filename_buckets + # ignores the title bucket key and only looks at file_path. + buckets['heres'] = all_tracks + + fname_buckets = self.job._build_filename_buckets(buckets=buckets, found_groups=set()) + assert len(fname_buckets) == 1 + bucket = next(iter(fname_buckets.values())) + assert {t['id'] for t in bucket} == {1, 2, 3, 4, 5, 6, 7} + + def test_skips_tracks_already_in_a_group(self): + """`found_groups` carries IDs from the title-bucket pass — those + must not be re-considered.""" + canonical = _make_track(1, title="Song", file_path="/lib/Song.mp3") + sibling = _make_track(2, title="Song", file_path="/lib/Song_639122324339578022.mp3") + buckets = defaultdict(list, song=[canonical, sibling]) + fname_buckets = self.job._build_filename_buckets(buckets=buckets, found_groups={1}) + assert fname_buckets == {} # singleton bucket dropped + + def test_drops_singleton_buckets(self): + """A canonical filename with no siblings carries no comparison + value — keeping it just inflates the inner loop.""" + track = _make_track(1, title="Song", file_path="/lib/Song.mp3") + buckets = defaultdict(list, song=[track]) + assert self.job._build_filename_buckets(buckets=buckets, found_groups=set()) == {} + + def test_different_extensions_bucket_separately(self): + """A .mp3 next to a .flac with the same canonical stem are + different files (different formats), not slskd dedup orphans.""" + mp3 = _make_track(1, title="Song", file_path="/lib/Song.mp3") + flac = _make_track(2, title="Song", file_path="/lib/Song_639122324339578022.flac") + buckets = defaultdict(list, song=[mp3, flac]) + assert self.job._build_filename_buckets(buckets=buckets, found_groups=set()) == {} + + def test_skips_tracks_without_file_path(self): + """Defensive: DB rows can carry NULL file_path — skip them.""" + with_path = _make_track(1, title="Song", file_path="/lib/Song.mp3") + no_path = _make_track(2, title="Song", file_path=None) + buckets = defaultdict(list, song=[with_path, no_path]) + assert self.job._build_filename_buckets(buckets=buckets, found_groups=set()) == {} + + def test_windows_paths_handled(self): + """Backslash separators (Windows) must canonicalize the same as + forward-slash (Linux) so the bucket key is consistent.""" + a = _make_track(1, title="A", file_path=r"C:\music\Various\Song.mp3") + b = _make_track(2, title="B", file_path=r"C:\music\Various\Song_639122324339578022.mp3") + buckets = defaultdict(list, a=[a, b]) + result = self.job._build_filename_buckets(buckets=buckets, found_groups=set()) + assert len(result) == 1 + + +# --------------------------------------------------------------------------- +# Integration: _scan_bucket + filename pass produces the expected finding +# --------------------------------------------------------------------------- + + +class TestFilenameBucketSurfacesFinding: + """End-to-end check that the filename-bucket pass produces a + `duplicate_tracks` finding for the Mushy scenario.""" + + def test_seven_dedup_orphans_caught_as_one_group(self): + job = DuplicateDetectorJob() + ctx = _FakeContext() + + base_dir = "/data/torrents/music/Various Artists - Napoleon Dynamite OST" + tracks = [ + _make_track(1, title="Heres Rico Musiq", + file_path=f"{base_dir}/14-john_swihart-heres_rico-musiq.mp3"), + *[_make_track(i + 2, title=f"unrelated parsed title {i}", + file_path=f"{base_dir}/14-john_swihart-heres_rico-musiq_{ts}.mp3") + for i, ts in enumerate([ + "639122324339578022", "639126674226945470", + "639127689938113502", "639130174269948395", + "639132075826102610", "639132543802519474", + ])], + ] + # All in one filename bucket (canonical stem matches across all 7). + fname_buckets = job._build_filename_buckets( + buckets=defaultdict(list, _=tracks), + found_groups=set(), + ) + bucket = next(iter(fname_buckets.values())) + + result = SimpleNamespace(scanned=0, findings_created=0, errors=0) + job._scan_bucket( + bucket_tracks=bucket, + require_metadata_match=False, # filename match is the evidence + title_threshold=0.85, + artist_threshold=0.80, + ignore_cross_album=False, + found_groups=set(), + processed_holder={'count': 0}, + total=7, + result=result, + context=ctx, + ) + + assert result.findings_created == 1 + finding = ctx.findings[0] + assert finding['finding_type'] == 'duplicate_tracks' + assert finding['details']['count'] == 7 + assert {t['id'] for t in finding['details']['tracks']} == {1, 2, 3, 4, 5, 6, 7} + + +# --------------------------------------------------------------------------- +# Negative coverage: title-pass behavior unchanged for normal scenarios +# --------------------------------------------------------------------------- + + +class TestExistingTitlePassUnchanged: + """The new filename pass must not interfere with the original + title-bucket logic — same album, same artist, similar titles still + detected.""" + + def test_same_track_two_different_files_in_same_album(self): + job = DuplicateDetectorJob() + ctx = _FakeContext() + + a = _make_track(1, title="Hello World", file_path="/lib/01_hello_world.mp3", bitrate=320) + b = _make_track(2, title="Hello World", file_path="/lib/01-hello-world.mp3", bitrate=192) + + # title-pass bucket + result = SimpleNamespace(scanned=0, findings_created=0, errors=0) + job._scan_bucket( + bucket_tracks=[a, b], + require_metadata_match=True, + title_threshold=0.85, + artist_threshold=0.80, + ignore_cross_album=False, + found_groups=set(), + processed_holder={'count': 0}, + total=2, + result=result, + context=ctx, + ) + assert result.findings_created == 1 + + def test_cross_album_skip_still_honored_in_title_pass(self): + """When `ignore_cross_album=True` the title pass must still skip + same-title-different-album pairs. The filename pass would catch + them only if the canonical filenames also matched, which they + won't for legitimate cross-album cases (different folder paths).""" + job = DuplicateDetectorJob() + ctx = _FakeContext() + + a = _make_track(1, title="Hello World", album="Album A", + file_path="/lib/Artist/Album A/Hello World.mp3") + b = _make_track(2, title="Hello World", album="Album B", + file_path="/lib/Artist/Album B/Hello World.mp3") + + result = SimpleNamespace(scanned=0, findings_created=0, errors=0) + job._scan_bucket( + bucket_tracks=[a, b], + require_metadata_match=True, + title_threshold=0.85, + artist_threshold=0.80, + ignore_cross_album=True, + found_groups=set(), + processed_holder={'count': 0}, + total=2, + result=result, + context=ctx, + ) + assert result.findings_created == 0 diff --git a/tests/test_watchlist_album_match.py b/tests/test_watchlist_album_match.py new file mode 100644 index 00000000..7cb17a7d --- /dev/null +++ b/tests/test_watchlist_album_match.py @@ -0,0 +1,241 @@ +"""Regression tests for watchlist album-name matching helpers. + +Discord-reported (Mushy): the watchlist scanner re-downloaded the same +track up to 7 times because Spotify's album name +(``"Napoleon Dynamite (Music From The Motion Picture)"``) and the +media-server scan's album name (``"Napoleon Dynamite OST"``) failed a +strict 0.85 fuzzy threshold. ``is_track_missing_from_library`` then +declared the track missing on every scan and added it back to the +wishlist. + +The fix replaces the raw SequenceMatcher comparison with two pure +helpers — ``_normalize_album_for_match`` (strips qualifying +parentheticals like ``(Music From X)``, ``(Deluxe Edition)``, OST, +Remastered, etc.) and ``_albums_likely_match`` (substring check + +relaxed ratio). These tests pin the behavior so the regression doesn't +return. +""" + +import sys +import types + +import pytest + + +# --------------------------------------------------------------------------- +# Fixture: stub the heavy module-level imports so we can import the +# watchlist_scanner module without a live Spotify client / config DB. +# --------------------------------------------------------------------------- + + +@pytest.fixture(scope="module", autouse=True) +def _stub_imports(): + if "spotipy" not in sys.modules: + spotipy = types.ModuleType("spotipy") + oauth2 = types.ModuleType("spotipy.oauth2") + + class _Dummy: + def __init__(self, *a, **kw): + pass + + spotipy.Spotify = _Dummy + oauth2.SpotifyOAuth = _Dummy + oauth2.SpotifyClientCredentials = _Dummy + 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 _CM: + def get(self, key, default=None): + return default + + def get_active_media_server(self): + return "plex" + + settings_mod.config_manager = _CM() + config_pkg.settings = settings_mod + sys.modules["config"] = config_pkg + sys.modules["config.settings"] = settings_mod + + if "core.matching_engine" not in sys.modules: + me = types.ModuleType("core.matching_engine") + + class _ME: + def clean_title(self, title): + return title + + me.MusicMatchingEngine = _ME + sys.modules["core.matching_engine"] = me + + yield + + +# Imports happen lazily so the stubs above are in place first. +from core.watchlist_scanner import ( # noqa: E402 + _albums_likely_match, + _normalize_album_for_match, +) + + +# --------------------------------------------------------------------------- +# _normalize_album_for_match +# --------------------------------------------------------------------------- + + +@pytest.mark.parametrize( + "raw,expected", + [ + ("Napoleon Dynamite (Music From The Motion Picture)", "napoleon dynamite"), + ("Napoleon Dynamite OST", "napoleon dynamite"), + ("Napoleon Dynamite [Original Soundtrack]", "napoleon dynamite"), + ("Abbey Road (Deluxe Edition)", "abbey road"), + ("Abbey Road (50th Anniversary Edition)", "abbey road"), + ("Hotel California (Remastered)", "hotel california"), + ("Thriller - Remastered 2011", "thriller"), + ("Mr. Morale & The Big Steppers", "mr morale the big steppers"), + ("Random album with NO qualifiers", "random album with no qualifiers"), + ("", ""), + ], +) +def test_normalize_strips_known_qualifiers(raw, expected) -> None: + assert _normalize_album_for_match(raw) == expected + + +def test_normalize_handles_none_safely() -> None: + """Defensive: callers pass DB rows that may have a None album.""" + assert _normalize_album_for_match(None) == "" # type: ignore[arg-type] + + +# --------------------------------------------------------------------------- +# _albums_likely_match — primary regression: the reported scenario +# --------------------------------------------------------------------------- + + +def test_napoleon_dynamite_compilation_naming_drift_treated_as_match() -> None: + """The bug we're fixing: this pair USED to score 0.49 SequenceMatcher + against the raw strings (below 0.85) and trigger an infinite + redownload loop. Must now match.""" + assert _albums_likely_match( + "Napoleon Dynamite (Music From The Motion Picture)", + "Napoleon Dynamite OST", + ) + + +def test_compilation_score_explanation() -> None: + """Document the underlying scenario for future readers — both sides + refer to the same compilation, named differently by Spotify and by + the media-server tag scan.""" + a = "Napoleon Dynamite (Music From The Motion Picture)" + b = "Napoleon Dynamite OST" + # After normalization both collapse to "napoleon dynamite", so the + # equality short-circuit fires before the fuzzy ratio matters. + assert _normalize_album_for_match(a) == _normalize_album_for_match(b) + + +# --------------------------------------------------------------------------- +# _albums_likely_match — positive cases (should match) +# --------------------------------------------------------------------------- + + +@pytest.mark.parametrize( + "spotify_name,lib_name", + [ + # Edition drift + ("Abbey Road (Deluxe Edition)", "Abbey Road"), + ("Abbey Road", "Abbey Road (Deluxe Edition)"), + ("Abbey Road (50th Anniversary Edition)", "Abbey Road [Remastered]"), + # Remaster drift + ("Hotel California (Remastered)", "Hotel California"), + ("Thriller - Remastered 2011", "Thriller"), + # Soundtrack naming variations + ("The Lion King (Original Motion Picture Soundtrack)", "The Lion King OST"), + ("Inception (Music From The Motion Picture)", "Inception Soundtrack"), + # Substring containment + ("Random Access Memories", "Random Access Memories (Bonus Edition)"), + ], +) +def test_likely_match_positive(spotify_name, lib_name) -> None: + assert _albums_likely_match(spotify_name, lib_name) + + +# --------------------------------------------------------------------------- +# _albums_likely_match — negative cases (genuinely different albums) +# --------------------------------------------------------------------------- + + +@pytest.mark.parametrize( + "spotify_name,lib_name", + [ + # Genuinely different albums by the same artist + ("To Pimp a Butterfly", "DAMN."), + ("Thriller", "Bad"), + ("Abbey Road", "Sgt. Pepper's Lonely Hearts Club Band"), + # Same word in title but different album + ("Greatest Hits Volume 1", "Greatest Hits Volume 2"), + ], +) +def test_likely_match_negative(spotify_name, lib_name) -> None: + assert not _albums_likely_match(spotify_name, lib_name) + + +# --------------------------------------------------------------------------- +# _albums_likely_match — defensive cases +# --------------------------------------------------------------------------- + + +def test_empty_inputs_do_not_match() -> None: + """A comparison with a missing side never matches — avoids returning + True for blank-vs-blank which would mask real bugs.""" + assert not _albums_likely_match("", "") + assert not _albums_likely_match("Album", "") + assert not _albums_likely_match("", "Album") + + +def test_none_inputs_do_not_raise() -> None: + """DB rows occasionally carry NULL albums.""" + assert not _albums_likely_match(None, "Album") # type: ignore[arg-type] + assert not _albums_likely_match("Album", None) # type: ignore[arg-type] + + +def test_only_qualifiers_does_not_falsely_match() -> None: + """Two albums whose only common substance is the stripped qualifier + must NOT match — otherwise '(Deluxe Edition)' vs '(Deluxe Edition)' + would collapse to '' == '' and trigger a true.""" + assert not _albums_likely_match("(Deluxe Edition)", "(Deluxe Edition)") + assert not _albums_likely_match("(OST)", "(OST)") + + +# --------------------------------------------------------------------------- +# Volume / part / disc marker disagreement — explicit tests +# --------------------------------------------------------------------------- + + +@pytest.mark.parametrize( + "spotify_name,lib_name", + [ + ("Greatest Hits Vol. 1", "Greatest Hits Vol. 2"), + ("Greatest Hits Volume 1", "Greatest Hits Volume 2"), + ("Best Of, Pt. 1", "Best Of, Pt. 2"), + ("Live in Tokyo Disc 1", "Live in Tokyo Disc 2"), + ("Live Album 1995", "Live Album 1997"), # trailing year as standalone number + ], +) +def test_disagreeing_volume_markers_block_match(spotify_name, lib_name) -> None: + assert not _albums_likely_match(spotify_name, lib_name) + + +@pytest.mark.parametrize( + "spotify_name,lib_name", + [ + ("Greatest Hits Vol. 1", "Greatest Hits Vol. 1 (Remastered)"), + ("Greatest Hits Volume 1", "Greatest Hits Vol 1"), + ], +) +def test_agreeing_volume_markers_still_match(spotify_name, lib_name) -> None: + """Same volume marker should NOT block a match that other rules accept.""" + assert _albums_likely_match(spotify_name, lib_name) + diff --git a/webui/static/helper.js b/webui/static/helper.js index 25ed624b..94459493 100644 --- a/webui/static/helper.js +++ b/webui/static/helper.js @@ -3454,6 +3454,8 @@ const WHATS_NEW = { { title: 'Automation Endpoints Lifted to core/automation', desc: 'internal — moved /api/automations/* CRUD + run + history routes, progress tracking helpers, and signal collection into core/automation/ (api, progress, signals). 383 fewer lines in web_server.py, 72 new tests. action handler registration stays put — those closures are tangled with feature implementations.' }, { title: 'Clean Up slskd Dedup Orphans After Import', desc: 'slskd appends "_" to a download when the destination file already exists (e.g. retried partials, the same track in multiple playlists). the canonical file imported fine but the timestamp-suffixed siblings sat in the downloads folder forever. now they get pruned right after each successful import.', page: 'downloads' }, { title: 'Beatport Tab Hidden Temporarily', desc: 'beatport rolled out cloudflare turnstile on every public page, so the scraper that powered the beatport tab now hits a bot challenge instead of html. their official oauth api is locked behind partner registration that isn\'t open to the public. hid the tab on sync until we find a workaround — backend endpoints are still in code so revival is a one-line html change.', page: 'sync' }, + { title: 'Watchlist No Longer Re-Downloads Compilation Tracks', desc: 'spotify and your media server name compilation albums differently — "napoleon dynamite (music from the motion picture)" vs "napoleon dynamite ost". the watchlist scanner used a strict 0.85 fuzzy threshold against the raw names, which always failed for soundtracks/deluxe-editions, so it kept re-adding the same track to the wishlist every 30 minutes. one user saw the same song download 7 times. now strips qualifier parentheticals (music from..., ost, deluxe edition, remastered) before comparing.', page: 'watchlist' }, + { title: 'Duplicate Detector Catches slskd Dedup Orphans', desc: 'when a track downloaded multiple times, slskd appended "_" to each copy and the media-server scan often parsed inconsistent titles for them — so the duplicate detector\'s title-bucket pass never compared them. added a second pass that re-buckets leftover tracks by canonical filename stem (with the slskd dedup tail stripped). seven copies of the same song in one folder now get caught as one duplicate group.', page: 'library' }, ], '2.4.0': [ // --- April 26, 2026 — Search & Artists unification + reorganize queue ---