diff --git a/.github/workflows/docker-publish.yml b/.github/workflows/docker-publish.yml index a2546ecc..2f948a99 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.0)' + description: 'Version tag (e.g. 2.7.1)' required: true - default: '2.7.0' + default: '2.7.1' jobs: build-and-push: diff --git a/config/config.example.json b/config/config.example.json index efbb1da9..0543822c 100644 --- a/config/config.example.json +++ b/config/config.example.json @@ -56,6 +56,11 @@ "playlist_path": "$playlist/$artist - $title" } }, + "import": { + "staging_path": "./Staging", + "replace_lower_quality": false, + "folder_artist_override": true + }, "lossy_copy": { "enabled": false, "bitrate": "320", @@ -67,4 +72,4 @@ "listenbrainz": { "token": "LISTENBRAINZ_TOKEN" } -} \ No newline at end of file +} diff --git a/config/settings.py b/config/settings.py index c32c5b75..a522d4a7 100644 --- a/config/settings.py +++ b/config/settings.py @@ -684,7 +684,13 @@ class ConfigManager: }, "import": { "staging_path": "./Staging", - "replace_lower_quality": False + "replace_lower_quality": False, + # Use the top Staging folder as the artist (Artist/Album layouts, + # mixtapes). On by default to preserve the long-standing import + # behaviour for existing users. Turn OFF if you stage a mixed pile + # of songs under one container folder, otherwise that folder's name + # overrides every metadata-identified artist (the "soulsync" case). + "folder_artist_override": True }, "m3u_export": { "enabled": False, diff --git a/core/acoustid_verification.py b/core/acoustid_verification.py index 42cc5500..0d10b613 100644 --- a/core/acoustid_verification.py +++ b/core/acoustid_verification.py @@ -22,10 +22,11 @@ from core.musicbrainz_client import MusicBrainzClient logger = get_logger("acoustid.verification") -# Thresholds -MIN_ACOUSTID_SCORE = 0.80 # Minimum AcoustID fingerprint score to trust -TITLE_MATCH_THRESHOLD = 0.70 # Title similarity needed to consider a match -ARTIST_MATCH_THRESHOLD = 0.60 # Artist similarity needed to consider a match +# Thresholds — single definition lives in the shared core; re-exported here so +# existing importers keep working and the values can't drift between paths. +from core.matching.audio_verification import ( # noqa: E402 + MIN_ACOUSTID_SCORE, TITLE_MATCH_THRESHOLD, ARTIST_MATCH_THRESHOLD, +) # Single matching-engine instance so version detection reuses the same patterns # used by the pre-download Soulseek matcher (remix / live / acoustic / @@ -56,166 +57,29 @@ class VerificationResult(Enum): ERROR = "error" # Lookup errored (invalid key / rate limit / no backend) - continue, but flag it -def _normalize(text: str) -> str: - """Normalize a string for comparison: lowercase, strip parentheticals, punctuation.""" - if not text: - return "" - s = text.lower().strip() - # Remove ALL parenthetical suffixes — these are metadata annotations, not core title - # Covers: (Live), (Remastered), (Parody of ...), (from "..." Soundtrack), (feat. ...), etc. - s = re.sub(r'\s*\([^)]*\)', '', s) - # Remove ALL square bracket suffixes: [Live], [Remastered], [Deluxe], etc. - s = re.sub(r'\s*\[[^\]]*\]', '', s) - # Remove trailing featuring info not in parentheses: "feat. ...", "ft. ...", "featuring ..." - s = re.sub(r'\s+(?:feat\.?|ft\.?|featuring)\s+.*$', '', s, flags=re.IGNORECASE) - # Remove dash-separated version tags: "- Vocal", "- Instrumental", "- Acoustic", etc. - s = re.sub(r'\s*-\s*(?:vocal|instrumental|acoustic|live|remix|cover|clean|explicit|radio\s*edit|original\s*mix|extended\s*mix|club\s*mix)\s*$', '', s, flags=re.IGNORECASE) - # Remove soundtrack/source subtitles: ' - From "..." Soundtrack', ' - from the film ...' - s = re.sub(r'\s*-\s*from\s+.+$', '', s, flags=re.IGNORECASE) - # Remove non-alphanumeric except spaces - s = re.sub(r'[^\w\s]', '', s) - # Collapse whitespace - s = re.sub(r'\s+', ' ', s).strip() - return s +# normalize() + similarity() + the alias-aware comparison now live in the shared +# decision core (core/matching/audio_verification.py) so import-time verification +# and the library scan share ONE definition — the <>-strip fix, CJK handling and +# thresholds can't drift apart again. Names kept (`_normalize` etc.) for existing +# importers/tests. +from core.matching.audio_verification import ( # noqa: E402 + normalize as _normalize, + similarity as _similarity, + _alias_aware_artist_sim, + _find_best_title_artist_match as _core_find_best_title_artist_match, + evaluate as _core_evaluate, + Decision as _CoreDecision, +) -def _similarity(a: str, b: str) -> float: - """Calculate similarity between two strings (0.0-1.0) after normalization.""" - na = _normalize(a) - nb = _normalize(b) - if not na or not nb: - return 0.0 - if na == nb: - return 1.0 - return SequenceMatcher(None, na, nb).ratio() - - -def _alias_aware_artist_sim( - expected_artist: str, - actual_artist: str, - aliases: Optional[Any] = None, -) -> float: - """Best artist-similarity across (expected, *aliases) vs actual. - - Issue #442 — when expected and actual are in different scripts - (e.g. `Hiroyuki Sawano` vs `澤野弘之`), raw `_similarity` scores - near 0% even though MusicBrainz aliases bridge them. Routes - through the pure helper so the verifier inherits one shared - contract. - - Returns the highest score across all candidates so existing - threshold checks (>= ARTIST_MATCH_THRESHOLD) keep their - semantics. When `aliases` is None or empty, behaves identically - to the prior raw `_similarity(expected, actual)` call. - - `aliases` accepts two shapes: - - - **Iterable** (list/tuple/set of strings): used directly. Used - by tests that already know the aliases. - - **Callable**: invoked LAZILY only when direct similarity - falls below the threshold. Lets the verifier pass a memoizing - thunk that resolves aliases (DB / cache / live MB) only when - needed. Verifications where the direct match already passes - never trigger the lookup chain — no wasted DB query for the - happy path. - - Diagnostic logging: emits an INFO line whenever an alias rescues - a comparison that direct similarity would have failed. Lets - future bug reports trace which alias triggered which PASS - decision (e.g. "this file passed because alias `澤野弘之` matched - the file's artist tag"). - """ - from core.matching.artist_aliases import artist_names_match - - direct = _similarity(expected_artist, actual_artist) - # Fast path — direct match already passes the threshold OR caller - # supplied no aliases handle. Avoids any lookup work. - if aliases is None: - return direct - if direct >= ARTIST_MATCH_THRESHOLD: - return direct - - # Resolve the iterable. Callable provider invoked NOW (lazily — - # the caller can memoize the result across multiple invocations - # within one verify_audio_file call). - resolved = aliases() if callable(aliases) else aliases - if not resolved: - return direct - - _matched, score = artist_names_match( - expected_artist, - actual_artist, - aliases=resolved, - threshold=ARTIST_MATCH_THRESHOLD, - similarity=_similarity, +def _find_best_title_artist_match(recordings, expected_title, expected_artist, + expected_artist_aliases=None): + """Back-compat wrapper around the shared core matcher (keeps the + ``expected_artist_aliases`` kwarg name for existing callers/tests).""" + return _core_find_best_title_artist_match( + recordings, expected_title, expected_artist, expected_artist_aliases, ) - # Diagnostic — alias rescued a comparison that direct would - # have failed. Worth logging at INFO since it's a user-visible - # decision (file PASS instead of FAIL). One line per rescue - # within a single verify call. - if score >= ARTIST_MATCH_THRESHOLD and direct < ARTIST_MATCH_THRESHOLD: - from core.matching.artist_aliases import best_alias_match - winner, _ = best_alias_match( - expected_artist, actual_artist, resolved, similarity=_similarity, - ) - logger.info( - "Artist alias rescued comparison: expected=%r vs actual=%r " - "(direct sim=%.2f, alias %r → score=%.2f)", - expected_artist, actual_artist, direct, winner, score, - ) - - return score - - -def _find_best_title_artist_match( - recordings: List[Dict[str, Any]], - expected_title: str, - expected_artist: str, - expected_artist_aliases: Optional[Any] = None, -) -> Tuple[Optional[Dict], float, float]: - """ - Find the AcoustID recording that best matches expected title/artist. - - Issue #442 — `expected_artist_aliases` (when supplied) is the - list of alternate spellings for `expected_artist` (Japanese - kanji, Cyrillic, etc.). Accepts either: - - - An iterable of alias strings (used eagerly), or - - A callable returning the list (resolved lazily — only fires - when at least one recording fails direct artist similarity). - - Each recording's artist is scored against (expected, *aliases) - and the best score wins. When the list is empty/omitted/None, - behavior is identical to the prior raw similarity comparison. - - Returns: - (best_recording, title_similarity, artist_similarity) - """ - best_rec = None - best_title_sim = 0.0 - best_artist_sim = 0.0 - best_combined = 0.0 - - for rec in recordings: - title = rec.get('title') or '' - artist = rec.get('artist') or '' - - title_sim = _similarity(expected_title, title) - artist_sim = _alias_aware_artist_sim( - expected_artist, artist, expected_artist_aliases, - ) - # Weight title higher since that's the primary identifier - combined = (title_sim * 0.6) + (artist_sim * 0.4) - - if combined > best_combined: - best_combined = combined - best_rec = rec - best_title_sim = title_sim - best_artist_sim = artist_sim - - return best_rec, best_title_sim, best_artist_sim - # Shared MusicBrainz client for enrichment lookups _mb_client = None @@ -466,241 +330,32 @@ class AcoustIDVerification: ) return _alias_cache['value'] - # Step 4: Find best title/artist match among AcoustID results - best_rec, title_sim, artist_sim = _find_best_title_artist_match( - recordings, expected_track_name, expected_artist_name, - expected_artist_aliases=_aliases_provider, + # Steps 4-5: delegate the PASS/SKIP/FAIL decision to the shared core + # (core/matching/audio_verification.evaluate) so import verification + # and the library scan apply identical logic. + outcome = _core_evaluate( + expected_track_name, expected_artist_name, recordings, + fingerprint_score=best_score, + aliases_provider=_aliases_provider, ) - - if not best_rec: - return VerificationResult.SKIP, "No recordings with title/artist info" - - matched_title = best_rec.get('title', '?') - matched_artist = best_rec.get('artist', '?') - logger.info( - f"Best match: '{matched_title}' by '{matched_artist}' " - f"(title_sim={title_sim:.2f}, artist_sim={artist_sim:.2f})" + "Best match: '%s' by '%s' (title_sim=%.2f, artist_sim=%.2f) -> %s", + outcome.matched_title, outcome.matched_artist, + outcome.title_sim, outcome.artist_sim, outcome.decision.value, ) - - # Step 4b: Version-mismatch gate. - # - # The ``_normalize`` step deliberately strips parentheticals and - # version tags ("(Instrumental)", "- Live", etc) so that legit - # name variations don't fail the title-similarity comparison. - # That same stripping made it impossible to tell a vocal track - # apart from its instrumental: "In My Feelings" and "In My - # Feelings (Instrumental)" both normalize to "in my feelings", - # the title sim ends up 1.0, and the file passes verification - # even though it's the wrong cut. - # - # Detect the version on each side BEFORE normalization runs. - # If the expected track and the AcoustID-matched recording - # disagree on version (one is original, the other is - # instrumental / live / remix / acoustic / etc), reject — the - # fingerprint identified a real song but it's not the one the - # caller asked for. - expected_version = _detect_title_version(expected_track_name) - matched_version = _detect_title_version(matched_title) - if expected_version != matched_version: - # Issue #607 (AfonsoG6): MusicBrainz often stores live - # recordings with bare titles ("Clarity") while the - # release entry carries the venue annotation ("Clarity - # (Live at Blossom Music Center, ...)"). The fingerprint - # correctly identifies the LIVE recording; only the - # title text is bare. Helper accepts the one-sided bare - # case when fingerprint + bare-title + artist all agree. - # Two-sided version mismatches (live vs remix etc) stay - # strict — those are genuinely different recordings. - if is_acceptable_version_mismatch( - expected_version, matched_version, - fingerprint_score=best_score, - title_similarity=title_sim, - artist_similarity=artist_sim, - ): - logger.info( - f"AcoustID version annotation differs (expected={expected_version}, " - f"matched={matched_version}) but fingerprint+title+artist all match — " - f"accepting (likely MB metadata gap on a live/version-annotated recording)" - ) - else: - msg = ( - f"Version mismatch: expected '{expected_track_name}' ({expected_version}) " - f"but file is '{matched_title}' ({matched_version})" - ) - logger.warning(f"AcoustID verification FAILED (version mismatch) - {msg}") - return VerificationResult.FAIL, msg - - # Step 5: Decide pass/fail based on similarity - if title_sim >= TITLE_MATCH_THRESHOLD and artist_sim >= ARTIST_MATCH_THRESHOLD: - msg = ( - f"Audio verified: '{matched_title}' by '{matched_artist}' " - f"matches expected '{expected_track_name}' by '{expected_artist_name}' " - f"(title={title_sim:.0%}, artist={artist_sim:.0%})" - ) - logger.info(f"AcoustID verification PASSED - {msg}") - return VerificationResult.PASS, msg - - # Title matches but artist doesn't — could be a cover/collab OR a - # genuinely different track with the same name. Distinguish the - # two by checking whether the expected artist appears anywhere in - # AcoustID's returned recordings. - if title_sim >= TITLE_MATCH_THRESHOLD and artist_sim < ARTIST_MATCH_THRESHOLD: - # First: if the expected artist is present in ANY recording's - # metadata for this fingerprint, it's likely the right track - # (AcoustID's "best" match just picked the wrong variant). - for rec in recordings: - rec_artist = rec.get('artist', '') - if _alias_aware_artist_sim( - expected_artist_name, rec_artist, _aliases_provider, - ) >= ARTIST_MATCH_THRESHOLD: - msg = ( - f"Audio verified: found '{expected_track_name}' by '{expected_artist_name}' " - f"in AcoustID results" - ) - logger.info(f"AcoustID verification PASSED (secondary match) - {msg}") - return VerificationResult.PASS, msg - - # Expected artist wasn't found anywhere. Decide between: - # - FAIL: clear mismatch, e.g. "Tom Walker" (sim ~0.2) when - # expecting "Maduk" — different song with same name - # - SKIP: ambiguous, e.g. collab / alt credit / formatting - # difference (sim 0.3-0.6) - # - # The 0.3 cutoff catches hard mismatches while preserving the - # benefit of the doubt for borderline artist formatting. - CLEAR_MISMATCH_THRESHOLD = 0.3 - if artist_sim < CLEAR_MISMATCH_THRESHOLD: - msg = ( - f"Audio mismatch: file identified as '{matched_title}' by '{matched_artist}', " - f"expected '{expected_track_name}' by '{expected_artist_name}' " - f"(title={title_sim:.0%}, artist={artist_sim:.0%}) — " - f"expected artist not found in any AcoustID recording" - ) - logger.warning(f"AcoustID verification FAILED (clear artist mismatch) - {msg}") - return VerificationResult.FAIL, msg - - msg = ( - f"Title matches but artist unclear: " - f"AcoustID='{matched_title}' by '{matched_artist}', " - f"expected '{expected_track_name}' by '{expected_artist_name}' " - f"(artist_sim={artist_sim:.0%} — ambiguous, could be cover/collab)" - ) - logger.info(f"AcoustID verification SKIPPED - {msg}") - return VerificationResult.SKIP, msg - - # Title doesn't match — check ALL recordings for any title/artist match - # (the best combined match might not be the right one if there are many results) - # Skip recordings whose version (instrumental/live/etc) disagrees with - # what the caller asked for — the version mismatch above checked - # only the best recording, but a wrong-version variant could still - # win this fallback scan if its bare title matched. - for rec in recordings: - t = rec.get('title') or '' - a = rec.get('artist') or '' - if _detect_title_version(t) != expected_version: - continue - if (_similarity(expected_track_name, t) >= TITLE_MATCH_THRESHOLD and - _alias_aware_artist_sim( - expected_artist_name, a, _aliases_provider, - ) >= ARTIST_MATCH_THRESHOLD): - msg = ( - f"Audio verified: found '{t}' by '{a}' in AcoustID results " - f"matching expected '{expected_track_name}' by '{expected_artist_name}'" - ) - logger.info(f"AcoustID verification PASSED (scan match) - {msg}") - return VerificationResult.PASS, msg - - # No match found — but if fingerprint score is very high (≥0.95) - # AND we have evidence the mismatch is a language/script case - # (rather than two genuinely different songs by the same artist), - # skip rather than quarantine a correct file. Two routes: - # - # (a) Either side of the comparison contains non-ASCII characters - # — strong signal of transliteration / kanji↔roman cases. - # Artist must still be a strong match to use this path. - # (b) Both title AND artist similarity are very high (the song - # is recognizably the same with minor punctuation / casing - # differences that fell below the strict match thresholds). - # - # The OLD logic was ``title_sim >= 0.55 OR artist_sim >= match``. - # That fired for English-vs-English songs by the same artist that - # share NO actual content — e.g. "R.O.T.C (Interlude)" by - # Kendrick Lamar getting accepted as "Rich (Interlude)" by - # Kendrick Lamar because the artist matched perfectly and - # "interlude" was shared in both titles. Reported by user when - # downloading Mr. Morale: three tracks (Rich Interlude, Savior - # Interlude, Savior) all received the wrong R.O.T.C audio file - # because of this leak. - # Use the BEST matching recording's strings here (not - # `recordings[0]`) so the failure message reports the same - # candidate the title/artist similarity scores came from. - # Issue #607 (AfonsoG6) example 1: the prior code mixed - # `recordings[0]`'s strings (which can be empty) with - # `best_rec`'s scores, producing nonsense reasons like - # "file identified as '' by '' (artist=100%)" when a later - # recording in the list scored well on artist. - display_title = matched_title or '?' - display_artist = matched_artist or '?' - has_non_ascii = ( - any(ord(c) > 127 for c in (expected_track_name or '')) - or any(ord(c) > 127 for c in display_title) - ) - language_script_skip = ( - best_score >= 0.95 - and has_non_ascii - and artist_sim >= ARTIST_MATCH_THRESHOLD - ) - high_confidence_strong_match_skip = ( - best_score >= 0.95 - and title_sim >= 0.80 - and artist_sim >= ARTIST_MATCH_THRESHOLD - ) - # Issue #797 — the EXPECTED artist and the AcoustID-matched - # artist are written in different scripts (e.g. "Joe Hisaishi" - # vs "久石譲") yet the alias-aware comparison still confirmed - # them as the same artist (artist_sim >= threshold, bridged via - # MusicBrainz aliases). When the artist itself spans scripts the - # title almost always does too — and a romanized-vs-native title - # comparison is meaningless, so it can't be evidence the file is - # wrong. Trust the confirmed artist + the fingerprint (already - # >= MIN_ACOUSTID_SCORE to reach here) and SKIP rather than - # quarantine a correct download of a non-English artist. - # - # Deliberately narrow (the "tight" scope): keyed on the ARTIST - # spanning scripts AND being confirmed. A same-script artist - # with only a cross-script TITLE (romaji artist + kanji title) - # is NOT covered — that case keeps the stricter 0.95 floor - # above, preserving the #607 wrong-file protection. - cross_script_artist_skip = ( - best_score >= MIN_ACOUSTID_SCORE - and artist_sim >= ARTIST_MATCH_THRESHOLD - and is_cross_script_mismatch(expected_artist_name, display_artist) - ) - if (language_script_skip or high_confidence_strong_match_skip - or cross_script_artist_skip): - reason = ( - "likely same song in different language/script" - if (language_script_skip or cross_script_artist_skip) - else "title/artist match within tolerance" - ) - msg = ( - f"Title/artist mismatch but fingerprint confidence very high ({best_score:.2f}): " - f"AcoustID='{display_title}' by '{display_artist}', " - f"expected '{expected_track_name}' by '{expected_artist_name}' — " - f"{reason}" - ) - logger.info(f"AcoustID verification SKIPPED (high confidence) - {msg}") - return VerificationResult.SKIP, msg - - # Low fingerprint score + no metadata match — file is likely wrong. - msg = ( - f"Audio mismatch: file identified as '{display_title}' by '{display_artist}', " - f"expected '{expected_track_name}' by '{expected_artist_name}' " - f"(title={title_sim:.0%}, artist={artist_sim:.0%})" - ) - logger.warning(f"AcoustID verification FAILED - {msg}") - return VerificationResult.FAIL, msg + _decision_map = { + _CoreDecision.PASS: VerificationResult.PASS, + _CoreDecision.SKIP: VerificationResult.SKIP, + _CoreDecision.FAIL: VerificationResult.FAIL, + } + result = _decision_map[outcome.decision] + if result == VerificationResult.PASS: + logger.info("AcoustID verification PASSED - %s", outcome.reason) + elif result == VerificationResult.FAIL: + logger.warning("AcoustID verification FAILED - %s", outcome.reason) + else: + logger.info("AcoustID verification SKIPPED - %s", outcome.reason) + return result, outcome.reason except Exception as e: # Any unexpected error -> SKIP (fail open) diff --git a/core/auto_import_worker.py b/core/auto_import_worker.py index 2b49bf48..8bfbea34 100644 --- a/core/auto_import_worker.py +++ b/core/auto_import_worker.py @@ -21,6 +21,7 @@ from datetime import datetime from difflib import SequenceMatcher from typing import Any, Callable, Dict, List, Optional +from core.imports.folder_artist import resolve_folder_artist from utils.logging_config import get_logger logger = get_logger("auto_import") @@ -1576,31 +1577,18 @@ class AutoImportWorker: album_name = identification.get('album_name', 'Unknown') image_url = identification.get('image_url', '') - # Parent folder artist override: if the staging folder structure is - # Artist/Albums/AlbumName or Artist/AlbumName, use the parent folder - # as the artist name when the tag-extracted artist looks wrong. - # This handles mixtapes/compilations where embedded tags have DJ names. + # Parent folder artist override via import.folder_artist_override. + # Default on to preserve the legacy Artist/Album staging behavior. + # Users who stage mixed piles under one container folder can turn it off + # to keep the metadata-identified artist. try: - staging_root = self._resolve_staging_path() or self.staging_path - rel_path = os.path.relpath(candidate.path, staging_root) - parts = [p for p in rel_path.replace('\\', '/').split('/') if p] - - # parts[0] = artist folder, parts[1] = album or category subfolder, etc. - # Only attempt override if there's at least 2 levels (artist/album) - folder_artist = None - if len(parts) >= 2: - _category_names = {'albums', 'singles', 'eps', 'compilations', 'mixtapes', - 'discography', 'music', 'downloads'} - if len(parts) >= 3 and parts[1].lower() in _category_names: - # Artist/Albums/AlbumFolder → parts[0] is artist - folder_artist = parts[0] - elif parts[0].lower() not in _category_names: - # Artist/AlbumFolder → parts[0] is artist - folder_artist = parts[0] - - if folder_artist and folder_artist.lower() != artist_name.lower(): - logger.info(f"[Auto-Import] Parent folder artist '{folder_artist}' differs from tag artist '{artist_name}' — using folder artist") - artist_name = folder_artist + if self._config_manager.get('import.folder_artist_override', True): + staging_root = self._resolve_staging_path() or self.staging_path + rel_path = os.path.relpath(candidate.path, staging_root) + folder_artist = resolve_folder_artist(rel_path, artist_name, enabled=True) + if folder_artist: + logger.info(f"[Auto-Import] Parent folder artist '{folder_artist}' differs from tag artist '{artist_name}' — using folder artist") + artist_name = folder_artist except Exception as e: logger.debug("folder artist override failed: %s", e) release_date = identification.get('release_date', '') or album_data.get('release_date', '') diff --git a/core/deezer_client.py b/core/deezer_client.py index 07162228..6f9d96ec 100644 --- a/core/deezer_client.py +++ b/core/deezer_client.py @@ -6,6 +6,7 @@ from typing import Dict, List, Optional, Any from functools import wraps from dataclasses import dataclass from utils.logging_config import get_logger +from core.metadata.artist_album_cache import get_cached_artist_album_items, store_artist_album_items from core.metadata.cache import get_metadata_cache logger = get_logger("deezer_client") @@ -873,17 +874,41 @@ class DeezerClient: Matches iTunesClient.get_artist_albums() interface. Paginates through all results up to the requested limit.""" + cache = get_metadata_cache() + cached_items = get_cached_artist_album_items(cache, 'deezer', artist_id, album_type=album_type, limit=limit) + if cached_items: + try: + requested_types = [t.strip() for t in album_type.split(',')] + cached_albums = [] + for album_data in cached_items: + album = Album.from_deezer_album(album_data) + if album_type != 'album,single': + if album.album_type not in requested_types: + if not (album.album_type == 'ep' and 'single' in requested_types): + continue + cached_albums.append(album) + return cached_albums[:limit] + except Exception as e: + logger.debug("Deezer artist albums cache reuse failed: %s", e) + albums = [] all_raw = [] requested_types = [t.strip() for t in album_type.split(',')] offset = 0 page_size = 100 # Deezer API max per request + complete = True # cleared if pagination breaks on a transient/malformed error while offset < limit: fetch_limit = min(page_size, limit - offset) data = self._api_get(f'artist/{artist_id}/albums', {'limit': fetch_limit, 'index': offset}) - if not data or 'data' not in data or len(data['data']) == 0: + if not data or 'data' not in data: + # Malformed/transient response mid-pagination — what we have is a + # PARTIAL discography. Don't cache it as the full list (mirrors the + # Spotify truncated-fetch guard). #853 follow-up. + complete = False break + if len(data['data']) == 0: + break # No more albums — a clean end of pagination. for album_data in data['data']: all_raw.append(album_data) @@ -900,7 +925,6 @@ class DeezerClient: break # Last page offset += len(data['data']) - cache = get_metadata_cache() # Deezer's /artist/{id}/albums endpoint doesn't include artist info on each album. # Inject it so cached album entities have artist_name for discover page display. artist_stub = None @@ -914,6 +938,11 @@ class DeezerClient: entries.append((str(ad['id']), ad)) if entries: cache.store_entities_bulk('deezer', 'album', entries, skip_if_exists=True) + # Only cache the artist→album-LIST when pagination finished cleanly; a + # partial list would otherwise serve an incomplete discography until TTL. + # (Individual album entities above are complete, so they cache regardless.) + if complete: + store_artist_album_items(cache, 'deezer', artist_id, all_raw, album_type=album_type, limit=limit) logger.info(f"Retrieved {len(albums)} albums for artist {artist_id}") return albums[:limit] diff --git a/core/discogs_client.py b/core/discogs_client.py index 445b7c68..7bafa486 100644 --- a/core/discogs_client.py +++ b/core/discogs_client.py @@ -12,6 +12,7 @@ import re import time import threading import requests +from core.metadata.artist_album_cache import get_cached_artist_album_payload, store_artist_album_items from core.metadata.cache import get_metadata_cache from typing import List, Dict, Any, Optional from dataclasses import dataclass @@ -79,6 +80,51 @@ def _clean_discogs_artist_name(name: Optional[str]) -> str: return _DISCOGS_DISAMBIG_RE.sub('', name).strip() +# --- Discogs album ID typing ------------------------------------------------- +# Discogs has two album object types — masters (/masters/{id}) and releases +# (/releases/{id}) — whose numeric IDs share one space, so release N and master +# N are DIFFERENT albums. A bare numeric ID is therefore ambiguous. We tag the +# type into the ID string ('m12345' / 'r12345') at the point we parse it, so the +# correct endpoint can be chosen later without guessing. (Artist IDs are a single +# namespace and stay untagged.) + +def _discogs_album_kind(data: Dict[str, Any]) -> str: + """Classify a Discogs album payload as 'master' or 'release'. + + Search results and artist-discography items carry an explicit ``type``; + full detail responses don't, but only master detail has ``main_release``.""" + t = (data.get('type') or '').lower() + if t in ('master', 'release'): + return t + return 'master' if 'main_release' in data else 'release' + + +def _tag_discogs_album_id(raw_id: Any, kind: str) -> str: + """``'12345'`` + ``'master'`` -> ``'m12345'``; empty input -> ``''``.""" + s = str(raw_id or '').strip() + if not s: + return '' + return f"{'m' if kind == 'master' else 'r'}{s}" + + +def _discogs_album_endpoints(album_id: Any) -> List[str]: + """Map a (possibly tagged) album ID to the API path(s) to try, in order. + + ``'m12345'`` -> ``['/masters/12345']`` + ``'r12345'`` -> ``['/releases/12345']`` + ``'12345'`` (legacy untagged) -> ``['/releases/12345', '/masters/12345']`` + + Legacy bare IDs are tried release-first because stored IDs originate + overwhelmingly from search / manual-match / collection sync (all releases); + this also self-heals pre-fix bad matches. Returns ``[]`` for unusable input.""" + s = str(album_id or '').strip() + if len(s) > 1 and s[0] in ('m', 'r') and s[1:].isdigit(): + return [f"/{'masters' if s[0] == 'm' else 'releases'}/{s[1:]}"] + if s.isdigit(): + return [f'/releases/{s}', f'/masters/{s}'] + return [] + + # --- Shared dataclasses (same shape as iTunes/Deezer/Spotify) --- @dataclass @@ -304,7 +350,7 @@ class Album: external_urls['discogs_api'] = release_data['resource_url'] return cls( - id=str(release_data.get('id', '')), + id=_tag_discogs_album_id(release_data.get('id', ''), _discogs_album_kind(release_data)), name=title, artists=artists, release_date=release_date, @@ -643,10 +689,13 @@ class DiscogsClient: if cached and cached.get('title'): data = cached else: - # Try as master first (artist discography returns master IDs) - data = self._api_get(f'/masters/{release_id}') - if not data or not data.get('title'): - data = self._api_get(f'/releases/{release_id}') + # Hit the endpoint that matches the ID's type (tag-driven, no guessing). + data = None + for path in _discogs_album_endpoints(release_id): + data = self._api_get(path) + if data and data.get('title'): + break + data = None if not data: return None cache.store_entity('discogs', 'album', release_id, data) @@ -680,26 +729,45 @@ class DiscogsClient: def get_artist_albums(self, artist_id: str, album_type: str = 'album,single', limit: int = 50) -> List[Album]: """Get releases by an artist. Prefers master releases, filters features.""" - # First get the artist name for feature filtering. Strip Discogs - # disambiguation suffix so feature-vs-primary matching below - # compares against the canonical name, not "Beyoncé*". - artist_data = self._api_get(f'/artists/{artist_id}') - artist_name = _clean_discogs_artist_name( - artist_data.get('name', '') if artist_data else '' - ).lower() + cache = get_metadata_cache() + cached_payload = get_cached_artist_album_payload(cache, 'discogs', artist_id, album_type=album_type, limit=limit) + releases = cached_payload.get('_releases') if cached_payload else None + artist_name = '' + if cached_payload: + artist_name = str(cached_payload.get('artist_name') or '').lower() - data = self._api_get(f'/artists/{artist_id}/releases', { - 'sort': 'year', 'sort_order': 'desc', 'per_page': min(limit * 3, 200), - }) - if not data or not data.get('releases'): - return [] + if not isinstance(releases, list) or not releases: + # First get the artist name for feature filtering. Strip Discogs + # disambiguation suffix so feature-vs-primary matching below + # compares against the canonical name, not "Beyoncé*". + artist_data = self._api_get(f'/artists/{artist_id}') + artist_name = _clean_discogs_artist_name( + artist_data.get('name', '') if artist_data else '' + ).lower() + + data = self._api_get(f'/artists/{artist_id}/releases', { + 'sort': 'year', 'sort_order': 'desc', 'per_page': min(limit * 3, 200), + }) + if not data or not data.get('releases'): + return [] + releases = data.get('releases') or [] + store_artist_album_items( + cache, + 'discogs', + artist_id, + releases, + album_type=album_type, + limit=limit, + items_field='_releases', + extra_fields={'artist_name': artist_name}, + ) # Separate masters from individual releases — prefer masters (canonical versions) masters = [] releases_no_master = [] master_titles = set() - for item in data['releases']: + for item in releases: # Skip non-main roles role = item.get('role', 'Main').lower() if role not in ('main', ''): @@ -767,10 +835,13 @@ class DiscogsClient: if cached: return cached - # Try as master first (master IDs are used in artist discography) - data = self._api_get(f'/masters/{release_id}') - if not data or not data.get('tracklist'): - data = self._api_get(f'/releases/{release_id}') + # Hit the endpoint that matches the ID's type (tag-driven, no guessing). + data = None + for path in _discogs_album_endpoints(release_id): + data = self._api_get(path) + if data and data.get('tracklist'): + break + data = None if not data or not data.get('tracklist'): return None @@ -782,7 +853,7 @@ class DiscogsClient: image_url = (primary or images[0]).get('uri') album_info = { - 'id': str(data.get('id', release_id)), + 'id': str(release_id), 'name': data.get('title', ''), 'images': [{'url': image_url, 'height': 600, 'width': 600}] if image_url else [], 'release_date': str(data.get('year', '')) if data.get('year') else '', @@ -871,9 +942,13 @@ class DiscogsClient: cached = cache.get_entity('discogs', 'album', str(release_id)) if cached and cached.get('title'): return cached - data = self._api_get(f'/masters/{release_id}') - if not data or not data.get('title'): - data = self._api_get(f'/releases/{release_id}') + # Hit the endpoint that matches the ID's type (tag-driven, no guessing). + data = None + for path in _discogs_album_endpoints(release_id): + data = self._api_get(path) + if data and data.get('title'): + break + data = None if data: cache.store_entity('discogs', 'album', str(release_id), data) return data diff --git a/core/discogs_worker.py b/core/discogs_worker.py index 994ee259..12454c2f 100644 --- a/core/discogs_worker.py +++ b/core/discogs_worker.py @@ -17,7 +17,7 @@ from typing import Optional, Dict, Any from datetime import datetime, timedelta from utils.logging_config import get_logger from database.music_database import MusicDatabase -from core.discogs_client import DiscogsClient +from core.discogs_client import DiscogsClient, _discogs_album_kind, _tag_discogs_album_id from core.worker_utils import accept_artist_match, interruptible_sleep, set_album_api_track_count logger = get_logger("discogs_worker") @@ -436,7 +436,9 @@ class DiscogsWorker: conn = self.db._get_connection() cursor = conn.cursor() - discogs_id = str(data.get('id', '')) + # Tag the ID with its Discogs type so later re-fetches hit the right + # endpoint (master vs release share one numeric space). + discogs_id = _tag_discogs_album_id(data.get('id', ''), _discogs_album_kind(data)) genres = json.dumps(data.get('genres', [])) styles = json.dumps(data.get('styles', [])) labels = data.get('labels', []) diff --git a/core/discovery/endpoints.py b/core/discovery/endpoints.py index 2d0d2042..54fceb3c 100644 --- a/core/discovery/endpoints.py +++ b/core/discovery/endpoints.py @@ -104,7 +104,13 @@ def cancel_sync( """ try: if key not in states: - return {"error": not_found_message}, 404 + # Idempotent: the live discovery state is gone (a restart wiped the + # in-memory state, or it was already cancelled). Cancelling a sync + # that isn't running is a no-op SUCCESS, not a 404 — otherwise a + # mirrored playlist (e.g. a ListenBrainz weekly) whose state vanished + # is permanently wedged with "playlist not found" and can never be + # re-synced or dismissed (#702). + return {"success": True, "message": f"No active {label} sync to cancel"}, 200 state = states[key] state['last_accessed'] = time.time() diff --git a/core/download_plugins/torrent.py b/core/download_plugins/torrent.py index 8cb76da8..54dfeb56 100644 --- a/core/download_plugins/torrent.py +++ b/core/download_plugins/torrent.py @@ -352,34 +352,60 @@ class TorrentDownloadPlugin(DownloadSourcePlugin): self._finalize_download(download_id, last_save_path) return if status.state == 'error': + # Clean the dead torrent out of the client, or it's left orphaned + # (active in qbit, untracked here) and re-grabbed as a duplicate. + self._cleanup_torrent(torrent_hash, get_stall_action()) self._mark_error(download_id, status.error or "Torrent client reported error") return - if stall.is_stalled(status.downloaded, status.state, time.monotonic()): + if stall.is_stalled(status.downloaded, status.state, time.monotonic(), + size=status.size): self._handle_stalled(download_id, torrent_hash, get_stall_action()) return time.sleep(_POLL_INTERVAL_SECONDS) + # Deadline reached. One last status check closes the race where the + # torrent completed during the final poll interval — finalize it instead + # of deleting a just-finished download's files. Otherwise clean it out of + # the client, or it sits orphaned in qbit (e.g. a metadata-stuck magnet + # that escaped the stall timer) and gets re-grabbed as a duplicate. + try: + final = run_async(adapter.get_status(torrent_hash)) + except Exception: + final = None + if final is not None and final.state in _COMPLETE_STATES: + self._finalize_download(download_id, final.save_path or last_save_path) + return + self._cleanup_torrent(torrent_hash, get_stall_action()) self._mark_error(download_id, "Torrent download timed out") + def _cleanup_torrent(self, torrent_hash: str, action: str) -> None: + """Remove (abandon) or pause a dead/stalled/timed-out torrent in the + client so it isn't left ORPHANED — active in qbit but no longer tracked + here, which makes SoulSync re-grab the same dead torrent as a duplicate + on the next attempt (noldevin). Best-effort: a client error is logged, + not raised, so the download still fails cleanly.""" + adapter = get_active_torrent_adapter() + if adapter is None or not torrent_hash: + return + try: + if action == "pause": + run_async(adapter.pause(torrent_hash)) + else: + # delete_files: a stalled/failed torrent's partial data is junk + # (often just a metadata stub) — don't leave it on disk. + run_async(adapter.remove(torrent_hash, delete_files=True)) + except Exception as e: + logger.warning("Torrent cleanup (%s) on %s failed: %s", + action, torrent_hash[:8] if torrent_hash else "?", e) + def _handle_stalled(self, download_id: str, torrent_hash: str, action: str) -> None: """A torrent made no progress past the stall timeout. Abandon it (remove from client + delete its partial data) or pause it for the user, then fail the download so the worker frees up.""" - adapter = get_active_torrent_adapter() timeout_min = round(get_stall_timeout() / 60, 1) - if adapter is not None: - try: - if action == "pause": - run_async(adapter.pause(torrent_hash)) - else: - # delete_files: a stalled torrent's partial data is junk - # (often just a metadata stub) — don't leave it on disk. - run_async(adapter.remove(torrent_hash, delete_files=True)) - except Exception as e: - logger.warning("Stalled-torrent %s on %s failed: %s", - action, torrent_hash[:8] if torrent_hash else "?", e) + self._cleanup_torrent(torrent_hash, action) verb = "paused" if action == "pause" else "removed" self._mark_error( download_id, diff --git a/core/download_plugins/torrent_stall.py b/core/download_plugins/torrent_stall.py index 036375c8..a4a5dbf2 100644 --- a/core/download_plugins/torrent_stall.py +++ b/core/download_plugins/torrent_stall.py @@ -74,26 +74,50 @@ class StallTracker: def __init__(self, timeout_seconds: float): self.timeout = float(timeout_seconds or 0) self._last_downloaded = -1 # -1 = first observation + self._had_metadata = None # None = first observation; else size>0? self._progress_since = None # monotonic time of last forward movement - def is_stalled(self, downloaded: int, state: str, now: float) -> bool: - """Record this poll's observation; return True iff the torrent has - gone ``timeout`` seconds with no byte progress while in a state - that's supposed to be downloading. + def is_stalled(self, downloaded: int, state: str, now: float, + size: int = None) -> bool: + """Record this poll's observation; return True iff the torrent has gone + ``timeout`` seconds with no real forward progress while in a working state. - ``downloaded`` is cumulative bytes; ``state`` is the adapter-uniform - state; ``now`` is a monotonic timestamp (seconds).""" + ``downloaded`` is cumulative payload bytes; ``state`` is the adapter-uniform + state; ``now`` is a monotonic timestamp; ``size`` is the torrent's total + size in bytes (0/None while still fetching metadata). + + Metadata-phase fix (#852-adjacent torrent report): a magnet stuck + "downloading metadata" reports ``size==0`` and a ``downloaded`` byte + counter that still ticks up from DHT/peer-protocol overhead even though it + makes no actual progress. Treating those bumps as progress reset the stall + clock forever, so a dead magnet never timed out. Now the byte counter only + counts once metadata is in (``size>0``); during the metadata phase the only + thing that counts as progress is *obtaining* the metadata, so a torrent + that can't even do that within the timeout is correctly flagged stalled. + """ if self.timeout <= 0: return False downloaded = int(downloaded or 0) + # size is None when the caller doesn't track it (assume metadata present — + # the old byte-progress behavior); an explicit size==0 is the metadata + # phase (metaDL), where the byte counter is unreliable noise. + has_metadata = size is None or int(size) > 0 - # Forward progress (or first sighting) resets the stall clock. - if self._last_downloaded < 0 or downloaded > self._last_downloaded: - self._last_downloaded = downloaded + # Real forward progress: first sighting, metadata just arrived, or (only + # once we have metadata) more payload bytes. Byte bumps during the + # metadata phase are protocol noise and do NOT count. + progressed = ( + self._had_metadata is None # first poll + or (has_metadata and not self._had_metadata) # got metadata + or (has_metadata and downloaded > self._last_downloaded) # more payload + ) + self._had_metadata = has_metadata + self._last_downloaded = downloaded + + if progressed: self._progress_since = now return False - self._last_downloaded = downloaded # Not in a working state → not a stall (seeding/paused/completed). if state not in STALLABLE_STATES: diff --git a/core/downloads/monitor.py b/core/downloads/monitor.py index 472e521c..60ab4585 100644 --- a/core/downloads/monitor.py +++ b/core/downloads/monitor.py @@ -250,6 +250,11 @@ def requeue_quarantined_task_for_retry(task_id, batch_id, trigger): task.pop('quarantine_entry_id', None) task['status'] = 'searching' task['status_change_time'] = time.time() + # Surface the retry progress to the UI ("attempt 2/5" next to the + # status while the task goes around again). Cleared implicitly on + # completion (UI only renders it for active/queued states). + task['retry_info'] = attempt_desc + task['retry_trigger'] = trigger logger.info( f"[Retry:{trigger}] Re-queuing task {task_id} for next-best candidate " diff --git a/core/downloads/status.py b/core/downloads/status.py index f49a202b..f425cceb 100644 --- a/core/downloads/status.py +++ b/core/downloads/status.py @@ -349,6 +349,12 @@ def build_batch_status_data(batch_id: str, batch: dict, live_transfers_lookup: d 'error_message': task.get('error_message'), # Surface failure reasons to UI 'quarantine_entry_id': task.get('quarantine_entry_id'), 'has_candidates': bool(task.get('cached_candidates')), # Whether search found results (for clickable review) + # 'verified' / 'unverified' / 'force_imported' — set by the + # import pipeline once post-processing finishes. + 'verification_status': task.get('verification_status'), + # "2/5" while the quarantine-retry engine walks candidates. + 'retry_info': task.get('retry_info'), + 'retry_trigger': task.get('retry_trigger'), } _ti = task.get('track_info') if isinstance(task.get('track_info'), dict) else {} task_filename = task.get('filename') or _ti.get('filename') @@ -705,6 +711,7 @@ def _build_history_download_item(entry: dict) -> dict: 'priority': _STATUS_PRIORITY['completed'], 'quality': entry.get('quality') or '', 'file_path': entry.get('file_path') or '', + 'verification_status': entry.get('verification_status'), 'is_persistent_history': True, } @@ -788,6 +795,9 @@ def build_unified_downloads_response(limit: int, deps: StatusDeps) -> dict: 'status': status, 'progress': progress, 'error': task.get('error_message'), + 'verification_status': task.get('verification_status'), + 'retry_info': task.get('retry_info'), + 'retry_trigger': task.get('retry_trigger'), 'batch_id': batch_id, 'batch_name': batch.get('playlist_name') or batch.get('album_name') or '', 'batch_source': batch.get('source_page') or batch.get('initiated_from') or '', diff --git a/core/imports/folder_artist.py b/core/imports/folder_artist.py new file mode 100644 index 00000000..d767b02c --- /dev/null +++ b/core/imports/folder_artist.py @@ -0,0 +1,52 @@ +"""Opt-in "parent folder artist" resolution for imports. + +Historically the auto-import worker derived the artist from the top Staging +folder whenever the path had >=2 levels and that folder wasn't a category word +(albums/singles/eps/...). It did so *unconditionally*, overriding even a +confidently metadata-identified artist — which mass-mislabelled files when a +user staged everything under one container folder (see the "soulsync" incident). + +This module isolates that decision as a pure function so it can be: +- gated behind an opt-in setting (``import.folder_artist_override``, + default on for legacy compatibility), and +- unit-tested without standing up the whole import worker. +""" + +import os + +# Top-level folder names that denote a *category*, not an artist. +DEFAULT_CATEGORY_NAMES = frozenset({ + 'albums', 'singles', 'eps', 'compilations', 'mixtapes', + 'discography', 'music', 'downloads', +}) + + +def resolve_folder_artist(rel_path, identified_artist, enabled, + category_names=DEFAULT_CATEGORY_NAMES): + """Return the folder-derived artist to use, or ``None`` to keep the + already-identified artist. + + When ``enabled`` is False this always returns ``None`` — the import keeps + whatever artist the metadata match produced. Only when explicitly enabled + does it fall back to the staging folder name, and even then never when the + folder already equals the identified artist. + + ``rel_path`` is the candidate's path relative to the staging root. + """ + if not enabled: + return None + + parts = [p for p in rel_path.replace('\\', '/').split('/') if p] + + folder_artist = None + if len(parts) >= 2: + if len(parts) >= 3 and parts[1].lower() in category_names: + # Artist/Albums/AlbumFolder -> parts[0] is the artist + folder_artist = parts[0] + elif parts[0].lower() not in category_names: + # Artist/AlbumFolder -> parts[0] is the artist + folder_artist = parts[0] + + if folder_artist and folder_artist.lower() != (identified_artist or '').lower(): + return folder_artist + return None diff --git a/core/imports/pipeline.py b/core/imports/pipeline.py index 792af6d6..ec027f06 100644 --- a/core/imports/pipeline.py +++ b/core/imports/pipeline.py @@ -182,6 +182,26 @@ def build_import_pipeline_runtime( ) + +def _persist_verification_status(context, final_path): + """Compute + persist the verification status (verified / unverified / + force_imported) for a finished import: embedded tag on the file, plus + context['_verification_status'] for the history row and the Downloads UI. + MUST be called on EVERY success exit of post-processing (main, playlist + folder mode, simple download) — a missed exit means no badge and no tag. + Never raises.""" + try: + from core.matching.verification_status import status_for_import + from core.tag_writer import write_verification_status + status = status_for_import(context) + if status: + context['_verification_status'] = status + if final_path: + write_verification_status(str(final_path), status) + except Exception as _vs_err: + logger.debug(f"verification-status persist skipped: {_vs_err}") + + def post_process_matched_download(context_key, context, file_path, runtime, metadata_runtime=None): on_download_completed = getattr(runtime, "on_download_completed", None) automation_engine = getattr(runtime, "automation_engine", None) @@ -450,6 +470,7 @@ def post_process_matched_download(context_key, context, file_path, runtime, meta logger.info(f"Simple download post-processing complete: {activity_target}") context['_simple_download_completed'] = True context['_final_path'] = str(destination) + _persist_verification_status(context, destination) emit_track_downloaded(context, automation_engine) record_library_history_download(context) record_download_provenance(context) @@ -627,6 +648,8 @@ def post_process_matched_download(context_key, context, file_path, runtime, meta final_path = downsampled_path context['_final_processed_path'] = final_path + _persist_verification_status(context, final_path) + blasphemy_path = create_lossy_copy(final_path) if blasphemy_path: context['_final_processed_path'] = blasphemy_path @@ -944,6 +967,8 @@ def post_process_matched_download(context_key, context, file_path, runtime, meta final_path = downsampled_path context['_final_processed_path'] = final_path + _persist_verification_status(context, final_path) + blasphemy_path = create_lossy_copy(final_path) if blasphemy_path: context['_final_processed_path'] = blasphemy_path @@ -1219,6 +1244,8 @@ def post_process_matched_download_with_verification(context_key, context, file_p with tasks_lock: if task_id in download_tasks: _mark_task_completed(task_id, context.get('track_info')) + if context.get('_verification_status'): + download_tasks[task_id]['verification_status'] = context['_verification_status'] with matched_context_lock: if context_key in matched_downloads_context: del matched_downloads_context[context_key] @@ -1231,6 +1258,8 @@ def post_process_matched_download_with_verification(context_key, context, file_p if task_id in download_tasks: _mark_task_completed(task_id, context.get('track_info')) download_tasks[task_id]['metadata_enhanced'] = True + if context.get('_verification_status'): + download_tasks[task_id]['verification_status'] = context['_verification_status'] redownload_ctx = download_tasks[task_id].get('_redownload_context') with matched_context_lock: diff --git a/core/imports/quarantine.py b/core/imports/quarantine.py index 3df79f85..0e9af594 100644 --- a/core/imports/quarantine.py +++ b/core/imports/quarantine.py @@ -219,6 +219,7 @@ def list_quarantine_entries(quarantine_dir: str) -> List[Dict[str, Any]]: "trigger": sidecar.get("trigger", "unknown"), "source_username": source_username, "source_filename": source_filename, + "thumb_url": _extract_context_thumb(ctx), } ) @@ -226,6 +227,51 @@ def list_quarantine_entries(quarantine_dir: str) -> List[Dict[str, Any]]: return entries +def get_quarantine_entry_context(quarantine_dir: str, entry_id: str) -> Dict[str, Any]: + """The sidecar's embedded pipeline ``context`` dict for one entry. + Returns {} for thin/legacy sidecars, missing entries or read errors.""" + _, sidecar_path = _resolve_entry_paths(quarantine_dir, entry_id) + if not sidecar_path or not os.path.isfile(sidecar_path): + return {} + try: + with open(sidecar_path, encoding="utf-8") as f: + loaded = json.load(f) + ctx = loaded.get("context") if isinstance(loaded, dict) else None + return ctx if isinstance(ctx, dict) else {} + except Exception as exc: + logger.debug("quarantine context read failed for %s: %s", entry_id, exc) + return {} + + +def _extract_context_thumb(ctx: Dict[str, Any]) -> str: + """Album-art URL from a sidecar's pipeline context — same lookup chain the + library-history recorder uses (album/spotify_album image, then album_info, + then the track_info's embedded album images). Empty string when absent.""" + def _first_image(album: Any) -> str: + if not isinstance(album, dict): + return "" + url = album.get("image_url") or "" + if url: + return url + images = album.get("images") or [] + if images and isinstance(images[0], dict): + return images[0].get("url", "") or "" + return "" + + thumb = _first_image(ctx.get("album")) or _first_image(ctx.get("spotify_album")) + if not thumb: + album_info = ctx.get("album_info") + if isinstance(album_info, dict): + thumb = album_info.get("album_image_url", "") or "" + if not thumb: + ti = ctx.get("track_info") + if isinstance(ti, dict): + thumb = _first_image(ti.get("album")) + if not thumb: + thumb = ti.get("image_url", "") or "" + return thumb + + def _resolve_entry_paths(quarantine_dir: str, entry_id: str) -> Tuple[Optional[str], Optional[str]]: """Locate the `.quarantined` file + JSON sidecar for an entry id. diff --git a/core/imports/side_effects.py b/core/imports/side_effects.py index 1c84f287..20fe0017 100644 --- a/core/imports/side_effects.py +++ b/core/imports/side_effects.py @@ -268,6 +268,7 @@ def record_library_history_download(context: Dict[str, Any]) -> None: source_artist=source_artist, origin=origin, origin_context=origin_context, + verification_status=context.get("_verification_status"), ) except Exception as e: logger.debug("library history record failed: %s", e) diff --git a/core/itunes_client.py b/core/itunes_client.py index 6f08dba3..25f9c73a 100644 --- a/core/itunes_client.py +++ b/core/itunes_client.py @@ -5,6 +5,7 @@ import threading from functools import wraps from dataclasses import dataclass from utils.logging_config import get_logger +from core.metadata.artist_album_cache import get_cached_artist_album_items, store_artist_album_items from core.metadata.cache import get_metadata_cache logger = get_logger("itunes_client") @@ -1036,7 +1037,15 @@ class iTunesClient: """ import re - results = self._lookup(id=artist_id, entity='album', limit=min(limit, 200)) + cache = get_metadata_cache() + cache_limit = min(limit, 200) + cached_items = get_cached_artist_album_items(cache, 'itunes', artist_id, album_type=album_type, limit=cache_limit) + cache_hit = False + if cached_items: + results = cached_items + cache_hit = True + else: + results = self._lookup(id=artist_id, entity='album', limit=cache_limit) seen_albums = {} # Track albums by normalized name, prefer explicit versions def normalize_album_name(name: str) -> str: @@ -1098,7 +1107,7 @@ class iTunesClient: # If this is an explicit album, validate it has tracks before keeping it # (Some iTunes explicit albums are broken and return 0 tracks) - if is_explicit: + if is_explicit and not cache_hit: try: test_tracks = self._lookup(id=album.id, entity='song') track_count = len([t for t in test_tracks if t.get('wrapperType') == 'track']) @@ -1124,8 +1133,9 @@ class iTunesClient: if album_data.get('wrapperType') == 'collection' and album_data.get('collectionId'): album_entries.append((str(album_data['collectionId']), album_data)) if album_entries: - cache = get_metadata_cache() cache.store_entities_bulk('itunes', 'album', album_entries, skip_if_exists=True) + if not cache_hit: + store_artist_album_items(cache, 'itunes', artist_id, results, album_type=album_type, limit=cache_limit) logger.info(f"Retrieved {len(albums)} unique albums for artist {artist_id} (filtered from {len(results)} results)") return albums[:limit] diff --git a/core/matching/audio_verification.py b/core/matching/audio_verification.py new file mode 100644 index 00000000..a347ce73 --- /dev/null +++ b/core/matching/audio_verification.py @@ -0,0 +1,267 @@ +"""Shared audio-verification decision core (pure; no file/DB I/O). + +Single source of truth for normalization + the PASS/SKIP/FAIL decision used by +BOTH import-time verification (``core/acoustid_verification.py``) and the library +scan (``core/repair_jobs/acoustid_scanner.py``). Historically each path had its +own ``_normalize`` and decision branches that drifted apart and produced +inconsistent results (a correct cross-script anime-OST track passed at import but +was false-flagged by the scan). Centralising the decision here means the +thresholds, normalization, alias-aware comparison, cross-script handling, version +gate and duration guard are defined exactly once. +""" + +import re +from dataclasses import dataclass +from difflib import SequenceMatcher +from enum import Enum +from typing import Any, List, Optional + +from utils.logging_config import get_logger + +logger = get_logger("audio_verification") + +# Thresholds — the single definition both paths share. +MIN_ACOUSTID_SCORE = 0.80 # Minimum fingerprint score to trust a match. +TITLE_MATCH_THRESHOLD = 0.70 # Title similarity to consider a match. +ARTIST_MATCH_THRESHOLD = 0.60 # Artist similarity to consider a match. +CLEAR_MISMATCH_THRESHOLD = 0.30 # Below this artist sim = clear wrong song. + + +class Decision(Enum): + PASS = "pass" + SKIP = "skip" + FAIL = "fail" + + +@dataclass +class Outcome: + decision: Decision + title_sim: float = 0.0 + artist_sim: float = 0.0 + matched_title: str = "" + matched_artist: str = "" + reason: str = "" + + +def normalize(text: str) -> str: + """Normalize a title/artist for comparison. + + lowercase; strip ``()`` / ``[]`` / ``<>`` annotations (version tags, + performer credits like ````); strip trailing + version / featuring tags; KEEP CJK characters (``\\w`` is unicode-aware) so + Japanese/Chinese/Korean titles produce a comparable form instead of an empty + string; collapse whitespace. + """ + if not text: + return "" + s = text.lower().strip() + # Annotations that are metadata, not core identity. + s = re.sub(r'\s*\([^)]*\)', '', s) + s = re.sub(r'\s*\[[^\]]*\]', '', s) + s = re.sub(r'\s*<[^>]*>', '', s) + # Trailing featuring / version tags. + s = re.sub(r'\s+(?:feat\.?|ft\.?|featuring)\s+.*$', '', s, flags=re.IGNORECASE) + s = re.sub( + r'\s*-\s*(?:vocal|instrumental|acoustic|live|remix|cover|clean|explicit|' + r'radio\s*edit|original\s*mix|extended\s*mix|club\s*mix)\s*$', + '', s, flags=re.IGNORECASE, + ) + s = re.sub(r'\s*-\s*from\s+.+$', '', s, flags=re.IGNORECASE) + # Path/separator punctuation -> space so a title keeps matching a source + # filename that substituted '_' for an illegal '/' or ':' (#851): the on-disk + # "You See Big Girl _ T_T" must normalize the same as "You See Big Girl / T:T". + # Done before the strip below so they become word boundaries, not joins. + s = re.sub(r'[\\/:_]+', ' ', s) + # Drop remaining punctuation but keep word chars (incl. CJK) + spaces. + s = re.sub(r'[^\w\s]', '', s) + s = re.sub(r'\s+', ' ', s).strip() + return s + + +def similarity(a: str, b: str) -> float: + """Similarity (0.0–1.0) between two strings after normalization.""" + na, nb = normalize(a), normalize(b) + if not na or not nb: + return 0.0 + if na == nb: + return 1.0 + return SequenceMatcher(None, na, nb).ratio() + + +_match_engine = None + + +def _detect_title_version(title: str) -> str: + """Version label ('original'/'instrumental'/'live'/'remix'/...) for a title.""" + global _match_engine + if not title: + return 'original' + if _match_engine is None: + from core.matching_engine import MusicMatchingEngine + _match_engine = MusicMatchingEngine() + version_type, _ = _match_engine.detect_version_type(title) + return version_type + + +def _alias_aware_artist_sim(expected_artist: str, actual_artist: str, + aliases: Optional[Any] = None) -> float: + """Best artist similarity across (expected, *aliases) vs actual. + + Bridges cross-script artist comparisons (kanji↔romaji etc) when MusicBrainz + aliases are available. ``aliases`` is an iterable of alias strings, or a + callable resolving them lazily (only invoked when direct similarity falls + below threshold — keeps the happy path lookup-free). + """ + from core.matching.artist_aliases import artist_names_match + + direct = similarity(expected_artist, actual_artist) + if aliases is None: + return direct + if direct >= ARTIST_MATCH_THRESHOLD: + return direct + resolved = aliases() if callable(aliases) else aliases + if not resolved: + return direct + _matched, score = artist_names_match( + expected_artist, actual_artist, aliases=resolved, + threshold=ARTIST_MATCH_THRESHOLD, similarity=similarity, + ) + # Diagnostic: an alias rescued a comparison direct similarity would have + # failed. INFO since it's a user-visible decision (PASS instead of FAIL). + if score >= ARTIST_MATCH_THRESHOLD and direct < ARTIST_MATCH_THRESHOLD: + from core.matching.artist_aliases import best_alias_match + winner, _ = best_alias_match( + expected_artist, actual_artist, resolved, similarity=similarity, + ) + logger.info( + "Artist alias rescued comparison: expected=%r vs actual=%r " + "(direct sim=%.2f, alias %r → score=%.2f)", + expected_artist, actual_artist, direct, winner, score, + ) + return score + + +def _find_best_title_artist_match(recordings, expected_title, expected_artist, + aliases=None): + """Return (best_recording, title_sim, artist_sim) — title weighted higher.""" + best_rec = None + best_title_sim = 0.0 + best_artist_sim = 0.0 + best_combined = 0.0 + for rec in recordings: + title = rec.get('title') or '' + artist = rec.get('artist') or '' + title_sim = similarity(expected_title, title) + artist_sim = _alias_aware_artist_sim(expected_artist, artist, aliases) + combined = (title_sim * 0.6) + (artist_sim * 0.4) + if combined > best_combined: + best_combined = combined + best_rec = rec + best_title_sim = title_sim + best_artist_sim = artist_sim + return best_rec, best_title_sim, best_artist_sim + + +def evaluate(expected_title: str, expected_artist: str, + recordings: List[dict], *, fingerprint_score: float, + aliases_provider: Optional[Any] = None) -> Outcome: + """Decide PASS / SKIP / FAIL for a fingerprinted file against expected + title/artist. Pure: no I/O. Shared by import verification and library scan. + + ``aliases_provider``: iterable or callable of expected-artist aliases + (kanji/cyrillic/etc) used to bridge cross-script comparisons. + + Note: fingerprint-collision duration checks are the caller's responsibility + (the library scan pre-checks the top recording's length before calling this) + so the decision here stays purely about title/artist/version identity. + """ + from core.matching.script_compat import is_cross_script_mismatch + from core.matching.version_mismatch import is_acceptable_version_mismatch + + # No expected artist on record (legacy/compilation rows): compare on title + # only — the old scanner treated this as artist-match=1.0 and a missing DB + # value is no evidence the file is wrong. + no_expected_artist = not normalize(expected_artist or '') + + best_rec, title_sim, artist_sim = _find_best_title_artist_match( + recordings, expected_title, expected_artist, aliases_provider, + ) + if no_expected_artist: + artist_sim = 1.0 + if not best_rec: + return Outcome(Decision.SKIP, reason="No recordings with title/artist info") + + matched_title = best_rec.get('title', '?') or '?' + matched_artist = best_rec.get('artist', '?') or '?' + + def out(dec, reason): + return Outcome(dec, title_sim, artist_sim, matched_title, matched_artist, reason) + + # Version gate: original vs instrumental/live/remix is a real difference. + expected_version = _detect_title_version(expected_title) + matched_version = _detect_title_version(matched_title) + if expected_version != matched_version: + if not is_acceptable_version_mismatch( + expected_version, matched_version, + fingerprint_score=fingerprint_score, + title_similarity=title_sim, artist_similarity=artist_sim, + ): + return out(Decision.FAIL, + f"Version mismatch: expected ({expected_version}) " + f"but file is ({matched_version})") + + # Clean match. + if title_sim >= TITLE_MATCH_THRESHOLD and artist_sim >= ARTIST_MATCH_THRESHOLD: + return out(Decision.PASS, "Audio verified") + + # Title matches, artist doesn't — cover/collab vs genuinely wrong. + if title_sim >= TITLE_MATCH_THRESHOLD and artist_sim < ARTIST_MATCH_THRESHOLD: + for rec in recordings: + if _alias_aware_artist_sim( + expected_artist, rec.get('artist', ''), aliases_provider, + ) >= ARTIST_MATCH_THRESHOLD: + return out(Decision.PASS, "Expected artist found in AcoustID results") + if artist_sim < CLEAR_MISMATCH_THRESHOLD: + return out(Decision.FAIL, + f"Audio mismatch: '{matched_title}' by '{matched_artist}' " + f"— expected artist not found") + return out(Decision.SKIP, "Title matches but artist ambiguous (cover/collab?)") + + # Title doesn't match — scan all recordings for a version-matched hit. + def _title_sim(a, b): + return similarity(a, b) + + def _artist_sim(ea, aa): + return _alias_aware_artist_sim(ea, aa, aliases_provider) + + candidate = None + for rec in recordings: + if _detect_title_version(rec.get('title') or '') != expected_version: + continue + if (similarity(expected_title, rec.get('title') or '') >= TITLE_MATCH_THRESHOLD + and _alias_aware_artist_sim( + expected_artist, rec.get('artist', ''), aliases_provider, + ) >= ARTIST_MATCH_THRESHOLD): + candidate = rec + break + if candidate is not None: + return out(Decision.PASS, "Scan match found in AcoustID results") + + # High-confidence / cross-script skips (don't quarantine a correct file). + has_non_ascii = (any(ord(c) > 127 for c in (expected_title or '')) + or any(ord(c) > 127 for c in matched_title)) + language_script_skip = (fingerprint_score >= 0.95 and has_non_ascii + and artist_sim >= ARTIST_MATCH_THRESHOLD) + high_confidence_strong_match_skip = (fingerprint_score >= 0.95 + and title_sim >= 0.80 + and artist_sim >= ARTIST_MATCH_THRESHOLD) + cross_script_artist_skip = (fingerprint_score >= MIN_ACOUSTID_SCORE + and artist_sim >= ARTIST_MATCH_THRESHOLD + and is_cross_script_mismatch(expected_artist, matched_artist)) + if (language_script_skip or high_confidence_strong_match_skip + or cross_script_artist_skip): + return out(Decision.SKIP, "Likely same song in different language/script") + + return out(Decision.FAIL, + f"Audio mismatch: file identified as '{matched_title}' by " + f"'{matched_artist}', expected '{expected_title}' by '{expected_artist}'") diff --git a/core/matching/history_paths.py b/core/matching/history_paths.py new file mode 100644 index 00000000..8d3c0509 --- /dev/null +++ b/core/matching/history_paths.py @@ -0,0 +1,65 @@ +"""Resolve a ``library_history`` row to a playable on-disk file. + +Lifted out of ``web_server`` so the fallback chain — and its collision-safety — +is an importable, unit-tested seam. This matters because a *destructive* delete +(``/api/verification//delete`` → ``os.remove``) trusts the path this returns: +if the tracks-table fallback guessed the wrong same-title file, delete would +remove the wrong track. The rules below are exactly that guard, and the tests +lock them. + +Side effects are injected so the decision logic is pure: + - ``exists(path) -> bool`` (os.path.exists) + - ``resolve_library_path(raw) -> str | None`` (transfer/download/library prefix swap) + - ``lookup_titled_paths(title) -> list[str]`` (tracks.file_path WHERE LOWER(title)=LOWER(?)) +""" + +from __future__ import annotations + +from typing import Any, Callable, Dict, List, Optional + + +def resolve_history_audio_path( + row: Dict[str, Any], + *, + exists: Callable[[str], bool], + resolve_library_path: Callable[[str], Optional[str]], + lookup_titled_paths: Callable[[str], List[str]], +) -> Optional[str]: + """Return the on-disk path for a history row, or None. Fallback chain: + 1. the recorded path as-is, + 2. the prefix-swap resolver (Docker↔host / transfer→library), + 3. the tracks-table mirror by title (knows the CURRENT path after a rename), + resolved the same way — but only when it can be picked UNAMBIGUOUSLY. + """ + raw_path = (row.get("file_path") or "").strip() + if raw_path and exists(raw_path): + return raw_path + + resolved = resolve_library_path(raw_path) if raw_path else None + if resolved and exists(resolved): + return resolved + + title = (row.get("title") or "").strip() + if not title: + return None + + artist = (row.get("artist_name") or "").strip() + candidates = [p for p in (lookup_titled_paths(title) or []) if p] + + # Same-title collisions across artists exist, and delete() trusts this path, + # so be strict: when the row names an artist, only accept candidates whose + # path mentions it; with no artist, only an unambiguous single candidate. + artist_l = artist.lower() + if artist_l: + candidates = [p for p in candidates if artist_l in p.lower()] + elif len(candidates) != 1: + return None + + for cand in candidates: + cand_resolved = resolve_library_path(cand) + if cand_resolved and exists(cand_resolved): + return cand_resolved + return None + + +__all__ = ["resolve_history_audio_path"] diff --git a/core/matching/verification_status.py b/core/matching/verification_status.py new file mode 100644 index 00000000..f064ea1f --- /dev/null +++ b/core/matching/verification_status.py @@ -0,0 +1,58 @@ +"""Verification-status vocabulary for imported tracks. + +Three states, persisted in the DB (``tracks.verification_status``) AND as an +embedded file tag (``SOULSYNC_VERIFICATION``) so the information survives DB +resets and travels with the file: + +- ``verified`` — clean AcoustID PASS at import time. +- ``unverified`` — AcoustID SKIP (cross-script / ambiguous / no match in + the AcoustID DB). Imported, but not hard-confirmed. +- ``force_imported`` — accepted via the version-mismatch fallback after the + retry budget was exhausted (user opted in via + ``post_processing.accept_version_mismatch_fallback``). + A later library scan will still re-check these but + reports them as informational, clearly marked. + +Quarantined files are never imported, so they carry no status. +""" + +VERIFIED = 'verified' +UNVERIFIED = 'unverified' +FORCE_IMPORTED = 'force_imported' +# Set by the user via the review queue ("yes, this file IS the right track"). +# Outranks machine states: the scanner skips these entirely. +HUMAN_VERIFIED = 'human_verified' + +ALL_STATUSES = (VERIFIED, UNVERIFIED, FORCE_IMPORTED, HUMAN_VERIFIED) + +# The file tag name (Vorbis comment key / ID3 TXXX desc / MP4 freeform). +TAG_NAME = 'SOULSYNC_VERIFICATION' + + +def status_from_acoustid_result(result_value): + """Map an AcoustID verification result string ('pass'/'skip'/...) to a + status. 'disabled'/'error'/unknown return None — no claim either way.""" + if result_value == 'pass': + return VERIFIED + if result_value == 'skip': + return UNVERIFIED + return None + + +def status_for_import(context: dict): + """Status for a just-imported file from its pipeline context. + + Priority order: + 1. A quarantine entry the user explicitly approved ("yes, import this + exact file") is a human decision — ``human_verified``, outranking + whatever the machine said about the candidate earlier. + 2. The version-mismatch fallback flag: a force-accepted file is + ``force_imported`` regardless of what the (earlier, failed) + verification said. + 3. The AcoustID result of this pipeline run. + """ + if context.get('_approved_quarantine_trigger'): + return HUMAN_VERIFIED + if context.get('_version_mismatch_fallback'): + return FORCE_IMPORTED + return status_from_acoustid_result(context.get('_acoustid_result')) diff --git a/core/metadata/artist_album_cache.py b/core/metadata/artist_album_cache.py new file mode 100644 index 00000000..2a138b5c --- /dev/null +++ b/core/metadata/artist_album_cache.py @@ -0,0 +1,90 @@ +"""Shared artist album-list cache helpers for metadata clients.""" + +from __future__ import annotations + +from typing import Any, Optional + + +def make_artist_album_cache_key( + artist_id: str, + album_type: str = 'album,single', + limit: int = 200, + *, + include_limit: bool = True, +) -> str: + """Return the metadata-cache key for an artist album-list query.""" + safe_album_type = str(album_type or 'album,single').replace(',', '_') + base_key = f"{artist_id}_albums_{safe_album_type}" + return f"{base_key}_{limit}" if include_limit else base_key + + +def get_cached_artist_album_items( + cache: Any, + source: str, + artist_id: str, + *, + album_type: str = 'album,single', + limit: int = 200, + include_limit: bool = True, + items_field: str = '_albums', +) -> Optional[list[dict[str, Any]]]: + """Return cached raw artist album-list items, or None on miss/invalid shape.""" + cached = cache.get_entity( + source, + 'artist', + make_artist_album_cache_key(artist_id, album_type, limit, include_limit=include_limit), + ) + if not isinstance(cached, dict): + return None + + items = cached.get(items_field) + return items if isinstance(items, list) and items else None + + +def get_cached_artist_album_payload( + cache: Any, + source: str, + artist_id: str, + *, + album_type: str = 'album,single', + limit: int = 200, + include_limit: bool = True, +) -> Optional[dict[str, Any]]: + """Return the raw cached artist album-list payload, or None on miss.""" + cached = cache.get_entity( + source, + 'artist', + make_artist_album_cache_key(artist_id, album_type, limit, include_limit=include_limit), + ) + return cached if isinstance(cached, dict) else None + + +def store_artist_album_items( + cache: Any, + source: str, + artist_id: str, + items: list[dict[str, Any]], + *, + album_type: str = 'album,single', + limit: int = 200, + include_limit: bool = True, + items_field: str = '_albums', + extra_fields: Optional[dict[str, Any]] = None, +) -> None: + """Store raw artist album-list items in the metadata cache.""" + if not items: + return + + payload: dict[str, Any] = { + 'name': f'albums_{artist_id}', + items_field: items, + } + if extra_fields: + payload.update(extra_fields) + + cache.store_entity( + source, + 'artist', + make_artist_album_cache_key(artist_id, album_type, limit, include_limit=include_limit), + payload, + ) diff --git a/core/repair_jobs/acoustid_scanner.py b/core/repair_jobs/acoustid_scanner.py index 00f900a1..16ceabda 100644 --- a/core/repair_jobs/acoustid_scanner.py +++ b/core/repair_jobs/acoustid_scanner.py @@ -15,6 +15,9 @@ from typing import Optional from core.repair_jobs import register_job from core.repair_jobs.base import JobContext, JobResult, RepairJob from utils.logging_config import get_logger +from core.matching.audio_verification import evaluate, Decision +from core.matching.acoustid_candidates import duration_mismatches_strongly +from core.acoustid_verification import _resolve_expected_artist_aliases logger = get_logger("repair_job.acoustid") @@ -220,116 +223,111 @@ class AcoustIDScannerJob(RepairJob): or expected['artist'] ) - # Normalize and compare - norm_expected_title = _normalize(expected['title']) - norm_aid_title = _normalize(aid_title) - norm_expected_artist = _normalize(expected_artist) - norm_aid_artist = _normalize(aid_artist) - - title_sim = SequenceMatcher(None, norm_expected_title, norm_aid_title).ratio() - # Issue (Foxxify Discord report): AcoustID returns the FULL artist - # credit (e.g. `Okayracer, aldrch & poptropicaslutz!`) while the - # library DB carries only the primary artist (`Okayracer`). Raw - # similarity scores ~43% — well below threshold — so multi-artist - # tracks get flagged as Wrong Song even though the primary IS in - # the credit. Route through the shared `artist_names_match` helper - # which splits the credit on common separators (comma, ampersand, - # feat./ft./with/vs., etc.) and checks each token. Primary-in- - # credit cases now resolve at 100% match instead of 43%. - # - # Pass RAW artist strings (not pre-normalised) so the splitter - # can recognise the separators. The helper applies its own - # case + whitespace normalisation internally per token. - if norm_expected_artist: - from core.matching.artist_aliases import artist_names_match - - _, artist_sim = artist_names_match( - expected_artist, - aid_artist, - threshold=artist_threshold, - ) - else: - artist_sim = 1.0 - - if title_sim >= title_threshold and artist_sim >= artist_threshold: - return - - # Issue #587 (Foxxify) — top recording's metadata mismatched, but - # AcoustID often returns multiple recordings per fingerprint - # (sample collisions, multi-MB-record cases). Check ALL of them - # before flagging — if any candidate's metadata matches expected - # title + artist, the file IS the right song and AcoustID's top - # match was just a wrong-credited recording. - from core.matching.acoustid_candidates import ( - duration_mismatches_strongly, - find_matching_recording, - ) - from core.matching.artist_aliases import artist_names_match - - def _scanner_title_sim(a, b): - return SequenceMatcher(None, _normalize(a), _normalize(b)).ratio() - - def _scanner_artist_sim(expected_a, actual_a): - _, score = artist_names_match(expected_a, actual_a, threshold=artist_threshold) - return score - - candidate_match, _, _ = find_matching_recording( - fp_result.get('recordings') or [], - expected['title'], - expected_artist, - title_threshold=title_threshold, - artist_threshold=artist_threshold, - similarity=_scanner_title_sim, - artist_similarity=_scanner_artist_sim, - ) - if candidate_match is not None: - # A lower-ranked candidate matched — file IS the right song. - # No finding. + # Verification status from the embedded SOULSYNC_VERIFICATION tag. + # Only a human decision short-circuits the scan: the user explicitly + # confirmed the file via the review queue. Everything else (verified / + # unverified / force_imported / untagged) is re-checked; force_imported + # mismatches are reported as informational below since a mismatch + # there is EXPECTED (the user accepted the best candidate). + file_verif_status = None + try: + from core.tag_writer import read_file_tags as _rft + file_verif_status = (_rft(fpath) or {}).get('verification_status') + except Exception: # noqa: S110 — verification tag is optional context; None is fine + pass + if file_verif_status == 'human_verified': + # The user explicitly confirmed this file via the review queue — + # never second-guess a human decision. if context.report_progress: context.report_progress( - log_line=( - f'Resolved (lower-ranked candidate match): {fname} — ' - f'expected "{expected["title"]}" matched candidate ' - f'"{candidate_match.get("title")}" by ' - f'"{candidate_match.get("artist")}"' - ), + log_line=f'Skipped (human-verified): {fname}', log_type='skip') + return + + # Fingerprint-collision guard: when the TOP recording's length is wildly + # different from the file, the fingerprint hit is a hash collision (the + # 17-min mashup → 5-min track case), not a real match — skip BEFORE any + # title/artist/version analysis so it can't surface as a false finding. + try: + file_duration_s = (expected.get('duration_ms') or 0) / 1000.0 + except Exception: + file_duration_s = 0.0 + cand_duration_s = best_recording.get('duration') or best_recording.get('length') + if file_duration_s and duration_mismatches_strongly(file_duration_s, cand_duration_s): + if context.report_progress: + context.report_progress( + log_line=(f'Skipped (duration mismatch suggests fingerprint ' + f'collision): {fname}'), + log_type='skip') + return + + # Decision via the shared verification core — identical logic to import- + # time verification (alias-aware artist match + cross-script SKIP), so the + # scan no longer false-flags correct cross-script tracks. Only a FAIL + # produces a "Wrong download" finding. + _alias_cache = {} + + def _aliases(): + if 'v' not in _alias_cache: + try: + _alias_cache['v'] = _resolve_expected_artist_aliases(expected_artist) + except Exception: + _alias_cache['v'] = [] + return _alias_cache['v'] + + outcome = evaluate( + expected['title'], expected_artist, fp_result['recordings'], + fingerprint_score=best_score, + aliases_provider=_aliases, + ) + + # Persist the scan outcome so it feeds the same review pipeline as + # import-time verification: PASS backfills 'verified' on untagged or + # previously-unverified files; SKIP (ambiguous / cross-script / no + # hard confirmation) marks untagged files 'unverified' so they surface + # in the Downloads-page review queue. force_imported is never blessed + # here (normalize() strips version words, so an instrumental can PASS + # the title check) and 'verified' is never downgraded by a SKIP (the + # import-time check ran with richer candidate metadata). FAIL keeps + # the finding flow below. + new_status = file_verif_status + if outcome.decision == Decision.PASS and file_verif_status in (None, '', 'unverified'): + new_status = 'verified' + elif outcome.decision == Decision.SKIP and not file_verif_status: + new_status = 'unverified' + if new_status: + self._persist_status( + context, track_id, fpath, + (expected.get('file_path') or '').strip() or None, + new_status, write_tag=(new_status != file_verif_status), + expected=expected) + + if outcome.decision != Decision.FAIL: + if context.report_progress: + context.report_progress( + log_line=f'OK ({outcome.decision.value}): {fname} — {outcome.reason}', log_type='ok', ) return - # Issue #587 (Foxxify "17min mashup → 5min track") — duration - # guard against fingerprint hash collisions. When the file's - # actual duration differs from AcoustID's matched recording by - # more than max(60s, 35%), the fingerprint is almost certainly - # a sample/intro collision, not a real recording match. Don't - # produce a confident "Wrong Song" finding. - try: - file_duration_s = (expected.get('duration_ms') or 0) / 1000.0 - except Exception: - file_duration_s = 0 - candidate_duration_s = best_recording.get('duration') - if candidate_duration_s is None and best_recording.get('length'): - candidate_duration_s = best_recording.get('length') - if duration_mismatches_strongly(file_duration_s, candidate_duration_s): - if context.report_progress: - context.report_progress( - log_line=( - f'Skipped (duration mismatch suggests fingerprint collision): ' - f'{fname} — expected {file_duration_s:.0f}s, AcoustID ' - f'candidate {candidate_duration_s:.0f}s' - ), - log_type='skip', - ) - return + title_sim = outcome.title_sim + artist_sim = outcome.artist_sim + matched_title = outcome.matched_title or aid_title + matched_artist = outcome.matched_artist or aid_artist - # Mismatch detected + # Mismatch (FAIL) — create finding. if context.report_progress: context.report_progress( - log_line=f'Mismatch: {fname} — expected "{expected["title"]}", got "{aid_title}"', + log_line=f'Mismatch: {fname} — expected "{expected["title"]}", got "{matched_title}"', log_type='error' ) if context.create_finding: - severity = 'warning' if best_score >= 0.90 else 'info' + _is_force = file_verif_status == 'force_imported' + severity = 'info' if _is_force else ('warning' if best_score >= 0.90 else 'info') + _title = ( + f'Force-imported (fallback): "{expected["title"]}" is actually "{matched_title}"' + if _is_force else + f'Wrong download: "{expected["title"]}" is actually "{matched_title}"' + ) inserted = context.create_finding( job_id=self.job_id, finding_type='acoustid_mismatch', @@ -337,18 +335,18 @@ class AcoustIDScannerJob(RepairJob): entity_type='track', entity_id=str(track_id), file_path=fpath, - title=f'Wrong download: "{expected["title"]}" is actually "{aid_title}"', + title=_title, description=( f'Expected "{expected["title"]}" by {expected_artist}, ' - f'but audio fingerprint matches "{aid_title}" by {aid_artist} ' + f'but audio fingerprint matches "{matched_title}" by {matched_artist} ' f'(fingerprint: {best_score:.0%}, title match: {title_sim:.0%}, ' f'artist match: {artist_sim:.0%})' ), details={ 'expected_title': expected['title'], 'expected_artist': expected_artist, - 'acoustid_title': aid_title, - 'acoustid_artist': aid_artist, + 'acoustid_title': matched_title, + 'acoustid_artist': matched_artist, 'fingerprint_score': round(best_score, 3), 'title_similarity': round(title_sim, 3), 'artist_similarity': round(artist_sim, 3), @@ -356,6 +354,7 @@ class AcoustIDScannerJob(RepairJob): 'artist_thumb_url': expected.get('artist_thumb_url'), 'album_title': expected.get('album_title', ''), 'track_number': expected.get('track_number'), + 'force_imported': file_verif_status == 'force_imported', } ) if inserted: @@ -363,6 +362,56 @@ class AcoustIDScannerJob(RepairJob): else: result.findings_skipped_dedup += 1 + def _persist_status(self, context, track_id, fpath, db_path, status, write_tag, + expected=None): + """Persist a verification status to the file tag (durable, travels with + the file), the tracks row (UI cache) and any library_history rows for + this file (feeds the Unverified review queue on the Downloads page). + ``db_path`` is the unresolved DB-side path — history rows may store + either form, so both are matched. + + Files SoulSync never downloaded have no history row at all — for an + 'unverified' outcome one is inserted (download_source 'acoustid_scan') + so EVERY scan-flagged file lands in the review queue, not just past + downloads. Re-scans then match this row via file_path (no duplicates). + """ + if not status: + return + if write_tag: + try: + from core.tag_writer import write_verification_status + write_verification_status(fpath, status) + except Exception as e: + logger.debug("verification tag write failed for %s: %s", fpath, e) + try: + conn = context.db._get_connection() + cur = conn.cursor() + cur.execute( + "UPDATE tracks SET verification_status = ? WHERE id = ?", + (status, track_id)) + matched = 0 + for p in {p for p in (fpath, db_path) if p}: + 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 {} + cur.execute( + """INSERT INTO library_history + (event_type, title, artist_name, album_name, file_path, + thumb_url, download_source, verification_status) + VALUES ('download', ?, ?, ?, ?, ?, 'acoustid_scan', ?)""", + (exp.get('title') or os.path.basename(fpath), + exp.get('artist') or None, + exp.get('album_title') or None, + db_path or fpath, + exp.get('album_thumb_url') or None, + status)) + getattr(conn, 'commit', lambda: None)() + except Exception as e: + logger.debug("verification_status persist failed for %s: %s", track_id, e) + def _load_db_tracks(self, context: JobContext) -> dict: """Load all tracks from DB keyed by track ID.""" tracks = {} @@ -480,11 +529,3 @@ class AcoustIDScannerJob(RepairJob): finally: if conn: conn.close() - - -def _normalize(text: str) -> str: - t = text.lower() - t = re.sub(r'\(.*?\)', '', t) - t = re.sub(r'\[.*?\]', '', t) - t = re.sub(r'[^a-z0-9 ]', '', t) - return t.strip() diff --git a/core/repair_jobs/relocate.py b/core/repair_jobs/relocate.py new file mode 100644 index 00000000..f034b230 --- /dev/null +++ b/core/repair_jobs/relocate.py @@ -0,0 +1,68 @@ +"""Relocate an AcoustID-mismatched file into Staging for clean re-import (#704). + +The existing 'retag' fix corrects a mismatched file's tags + DB record but leaves +the file in the WRONG artist/album folder on disk — so the library shows the right +title while the file sits under the previous track's artist/album. AcoustID only +yields a title + artist (not a reliable album), so an *in-place* move has no +trustworthy target. + +Instead: retag the file, move it into the staging folder, and drop the stale +``tracks`` row. The auto-import worker (which watches staging) then re-identifies +the file with full metadata and files it in the correct artist/album/track path — +reusing the battle-tested import pipeline rather than guessing a destination here. + +Side effects are injected so the orchestration is a pure, unit-testable seam. +""" + +from __future__ import annotations + +import os +from typing import Any, Callable, Dict, Optional + + +def staging_destination(staging_dir: str, filename: str, + exists: Callable[[str], bool]) -> str: + """A non-colliding path for ``filename`` inside ``staging_dir``. + + If the name is already taken, suffix it ``' (1)'``, ``' (2)'``, … before the + extension — never overwrite an unrelated file already waiting in staging. + """ + base, ext = os.path.splitext(filename) + dest = os.path.join(staging_dir, filename) + n = 1 + while exists(dest): + dest = os.path.join(staging_dir, f"{base} ({n}){ext}") + n += 1 + return dest + + +def relocate_mismatch_to_staging( + resolved_path: str, + staging_dir: str, + tag_updates: Optional[Dict[str, Any]], + *, + write_tags: Callable[[str, Dict[str, Any]], Any], + move_file: Callable[[str, str], Any], + drop_db_row: Callable[[], Any], + exists: Callable[[str], bool], +) -> str: + """Retag (best-effort) → move into staging → drop the stale DB row. + + Returns the staging destination path. Order matters: the DB row is dropped + only AFTER a successful move, so a failed move (which raises) leaves the + library entry intact rather than orphaning it. + """ + if tag_updates: + try: + write_tags(resolved_path, tag_updates) + except Exception: # noqa: S110 — tags are best-effort; re-import re-derives them + # The relocation itself is the point, so don't abort over a tag write. + pass + + dest = staging_destination(staging_dir, os.path.basename(resolved_path), exists) + move_file(resolved_path, dest) # may raise → row NOT dropped (intentional) + drop_db_row() + return dest + + +__all__ = ["staging_destination", "relocate_mismatch_to_staging"] diff --git a/core/repair_worker.py b/core/repair_worker.py index 60c81ff8..27dffe3b 100644 --- a/core/repair_worker.py +++ b/core/repair_worker.py @@ -1983,6 +1983,57 @@ class RepairWorker: return {'success': True, 'action': 'redownload', 'message': f'Added "{expected_title}" to wishlist, removed wrong file'} + if fix_action == 'relocate': + # #704: retag fixes the file's tags but leaves it in the WRONG + # artist/album folder. AcoustID gives only title+artist (no reliable + # album), so move the retagged file into staging and let auto-import + # re-file it correctly with full metadata. Drop the stale tracks row. + resolved = _resolve_file_path(file_path, self.transfer_folder, + config_manager=self._config_manager) + if not resolved or not os.path.exists(resolved): + return {'success': False, 'error': f'File not found: {file_path}'} + staging_path = './Staging' + if self._config_manager: + staging_path = self._config_manager.get('import.staging_path', './Staging') + staging_path = self._resolve_path(staging_path) + try: + os.makedirs(staging_path, exist_ok=True) + except OSError as e: + return {'success': False, 'error': f'Staging folder unavailable: {e}'} + + aid_title = details.get('acoustid_title', '') + aid_artist = details.get('acoustid_artist', '') + tag_updates = {} + if aid_title: + tag_updates['title'] = aid_title + if aid_artist: + tag_updates['artist_name'] = aid_artist + tag_updates['artists_list'] = _split_acoustid_credit(aid_artist) + + def _drop_row(): + if not track_id: + return + conn = self.db._get_connection() + try: + conn.cursor().execute("DELETE FROM tracks WHERE id = ?", (track_id,)) + conn.commit() + finally: + conn.close() + + from core.repair_jobs.relocate import relocate_mismatch_to_staging + from core.tag_writer import write_tags_to_file + from core.imports.file_ops import safe_move_file + try: + dest = relocate_mismatch_to_staging( + resolved, staging_path, tag_updates, + write_tags=write_tags_to_file, move_file=safe_move_file, + drop_db_row=_drop_row, exists=os.path.exists) + except Exception as e: + return {'success': False, 'error': f'Relocate failed: {e}'} + self._cleanup_empty_parents(resolved) # remove the now-empty wrong folder + return {'success': True, 'action': 'relocated', + 'message': f'Moved to staging for re-import: {os.path.basename(dest)}'} + # Default: retag — update DB record to match the actual audio content aid_title = details.get('acoustid_title', '') aid_artist = details.get('acoustid_artist', '') diff --git a/core/security/ws_gate.py b/core/security/ws_gate.py new file mode 100644 index 00000000..cab092c6 --- /dev/null +++ b/core/security/ws_gate.py @@ -0,0 +1,39 @@ +"""WebSocket access gate (#852). + +Flask's ``before_request`` — where the launch-PIN / login gate lives — does NOT +run for the socketio handshake, so an unauthenticated client that removes the +login/PIN overlay (Safari "Hide Distracting Items", devtools, curl) can still open +a socket and receive the live data SoulSync streams over it (downloads, logs, +dashboard, notifications). The connect handler must therefore enforce the same +check the HTTP gate does. + +Pure decision so it's unit-testable; the socketio handler injects the live +session/config/header values. Mirrors the HTTP gate precedence exactly: login +mode (when on) replaces the launch PIN. +""" + +from __future__ import annotations + + +def is_ws_connection_blocked( + *, + require_login: bool, + login_authenticated: bool, + require_pin: bool, + pin_verified: bool, + proxy_authed: bool, +) -> bool: + """True ⇒ reject this WebSocket connection. + + - Login mode on → must be login-authenticated. + - Else PIN on → must have verified the PIN (or be trusted by an auth proxy). + - Neither on → open (matches the HTTP gate's no-op default). + """ + if require_login: + return not login_authenticated + if require_pin: + return not (pin_verified or proxy_authed) + return False + + +__all__ = ["is_ws_connection_blocked"] diff --git a/core/spotify_client.py b/core/spotify_client.py index 17824d09..b5318037 100644 --- a/core/spotify_client.py +++ b/core/spotify_client.py @@ -8,6 +8,7 @@ from functools import wraps from dataclasses import dataclass from utils.logging_config import get_logger from config.settings import config_manager +from core.metadata.artist_album_cache import get_cached_artist_album_items, store_artist_album_items from core.metadata.cache import get_metadata_cache logger = get_logger("spotify_client") @@ -1897,15 +1898,20 @@ class SpotifyClient: cache = get_metadata_cache() fallback_src = self._fallback_source source = fallback_src if self._is_itunes_id(artist_id) else 'spotify' - cache_key = f"{artist_id}_albums_{album_type.replace(',', '_')}" # Check cache first (unless caller needs fresh data) if not skip_cache: - cached = cache.get_entity(source, 'artist', cache_key) - if cached: + cached_items = get_cached_artist_album_items( + cache, + source, + artist_id, + album_type=album_type, + limit=limit, + include_limit=False, + ) + if cached_items: try: - albums_list = cached.get('_albums', cached) if isinstance(cached, dict) else cached - return [Album.from_spotify_album(ad) for ad in albums_list] + return [Album.from_spotify_album(ad) for ad in cached_items] except Exception as e: logger.debug("artist albums cache reuse: %s", e) @@ -1959,7 +1965,15 @@ class SpotifyClient: # complete entities regardless of how many pages we walked. if raw_items: if not truncated: - cache.store_entity('spotify', 'artist', cache_key, {'name': f'albums_{artist_id}', '_albums': raw_items}) + store_artist_album_items( + cache, + 'spotify', + artist_id, + raw_items, + album_type=album_type, + limit=limit, + include_limit=False, + ) # Also cache individual albums opportunistically entries = [(ad.get('id'), ad) for ad in raw_items if ad.get('id')] if entries: diff --git a/core/tag_writer.py b/core/tag_writer.py index b0d4c6fa..816e69cd 100644 --- a/core/tag_writer.py +++ b/core/tag_writer.py @@ -44,6 +44,8 @@ def read_file_tags(file_path: str) -> Dict[str, Any]: 'replaygain_track_peak': None, 'replaygain_album_gain': None, 'replaygain_album_peak': None, + # SoulSync verification status ('verified'/'unverified'/'force_imported') + 'verification_status': None, } if not file_path or not os.path.exists(file_path): @@ -80,6 +82,10 @@ def read_file_tags(file_path: str) -> Dict[str, Any]: except (ValueError, TypeError): pass result['has_cover_art'] = bool(audio.tags.getall('APIC')) + for fr in audio.tags.getall('TXXX'): + if getattr(fr, 'desc', '') == 'SOULSYNC_VERIFICATION' and fr.text: + result['verification_status'] = str(fr.text[0]) + break elif isinstance(audio, (FLAC, OggVorbis)) or type(audio).__name__ == 'OggOpus': # FLAC / OGG @@ -102,6 +108,7 @@ def read_file_tags(file_path: str) -> Dict[str, Any]: else: # OGG doesn't have a standard picture field we can easily check result['has_cover_art'] = False + result['verification_status'] = _vorbis_first(audio, 'soulsync_verification') elif isinstance(audio, MP4): # MP4 / M4A @@ -118,6 +125,12 @@ def read_file_tags(file_path: str) -> Dict[str, Any]: if disk: result['disc_number'] = disk[0][0] if isinstance(disk[0], tuple) else None result['has_cover_art'] = bool(audio.tags.get('covr', [])) if audio.tags else False + vs = (audio.tags or {}).get('----:com.soulsync:VERIFICATION') + if vs: + raw = vs[0] + result['verification_status'] = ( + raw.decode('utf-8', 'ignore') if isinstance(raw, bytes) else str(raw) + ) except Exception as e: result['error'] = str(e) @@ -156,6 +169,39 @@ def is_placeholder_meta(value: Any) -> bool: return s == '' or s in _PLACEHOLDER_META_VALUES +def write_verification_status(file_path: str, status: str) -> bool: + """Embed the SoulSync verification status into the file's tags. + + Vorbis comment ``SOULSYNC_VERIFICATION`` (FLAC/OGG/Opus), ID3 + ``TXXX:SOULSYNC_VERIFICATION`` (MP3), MP4 freeform + ``----:com.soulsync:VERIFICATION``. The tag travels with the file so the + status survives DB resets; the AcoustID scan reads it back via + ``read_file_tags`` to refresh the DB column and to mark force-imported + fallbacks in its findings. Never raises; returns success. + """ + if not status or not file_path or not os.path.exists(file_path): + return False + try: + audio = MutagenFile(file_path) + if audio is None: + return False + if getattr(audio, 'tags', None) is None and hasattr(audio, 'add_tags'): + audio.add_tags() + if isinstance(audio.tags, ID3): + audio.tags.delall('TXXX:SOULSYNC_VERIFICATION') + audio.tags.add(TXXX(encoding=3, desc='SOULSYNC_VERIFICATION', text=[status])) + elif isinstance(audio, MP4): + audio.tags['----:com.soulsync:VERIFICATION'] = [status.encode('utf-8')] + else: + # Vorbis-comment family (FLAC / OggVorbis / OggOpus) + audio['SOULSYNC_VERIFICATION'] = [status] + audio.save() + return True + except Exception as e: + logger.debug("write_verification_status failed for %s: %s", file_path, e) + return False + + def guard_placeholder_overwrite(db_val: Any, file_val: Any) -> Any: """#800 guard: never replace a real file value with a placeholder. diff --git a/database/music_database.py b/database/music_database.py index 9c196d25..a3b2fb03 100644 --- a/database/music_database.py +++ b/database/music_database.py @@ -638,11 +638,29 @@ class MusicDatabase: if 'download_source' not in lh_cols: cursor.execute("ALTER TABLE library_history ADD COLUMN download_source TEXT") logger.info("Added download_source column to library_history") - for _col in ['source_track_id', 'source_track_title', 'source_filename', 'acoustid_result', 'source_artist']: + for _col in ['source_track_id', 'source_track_title', 'source_filename', 'acoustid_result', 'source_artist', 'verification_status']: if _col not in lh_cols: cursor.execute(f"ALTER TABLE library_history ADD COLUMN {_col} TEXT") logger.info(f"Added {_col} column to library_history") + # One-time backfill: derive verification_status for history rows + # written before the column existed (or by pipeline exits that + # missed it) from the acoustid_result those imports already + # recorded (pass->verified, skip->unverified). force_imported + # can't be derived retroactively. Idempotent: only fills NULLs. + cursor.execute(""" + UPDATE library_history SET verification_status = + CASE acoustid_result + WHEN 'pass' THEN 'verified' + WHEN 'skip' THEN 'unverified' + WHEN 'fail' THEN 'force_imported' + END + WHERE verification_status IS NULL + AND acoustid_result IN ('pass', 'skip', 'fail') + """) + if cursor.rowcount: + logger.info("Backfilled verification_status from acoustid_result (%d rows)", cursor.rowcount) + # Migration: download-origin provenance — what TRIGGERED a download # ('watchlist' + artist / 'playlist' + playlist name). Read by the # origin-history modal on the watchlist + sync pages. @@ -2411,6 +2429,11 @@ class MusicDatabase: if 'musicbrainz_match_status' not in tracks_columns: cursor.execute("ALTER TABLE tracks ADD COLUMN musicbrainz_match_status TEXT") added_tracks = True + if 'verification_status' not in tracks_columns: + # 'verified' / 'unverified' / 'force_imported' — set at import, + # refreshed by the AcoustID scan (which reads the file tag). + cursor.execute("ALTER TABLE tracks ADD COLUMN verification_status TEXT") + added_tracks = True if added_tracks: columns_added = True logger.info("Added MusicBrainz columns to tracks table") @@ -13016,7 +13039,7 @@ class MusicDatabase: quality=None, server_source=None, file_path=None, thumb_url=None, download_source=None, source_track_id=None, source_track_title=None, source_filename=None, acoustid_result=None, source_artist=None, - origin=None, origin_context=None): + origin=None, origin_context=None, verification_status=None): """Record a download or import event to the library history table. ``origin``/``origin_context`` record what TRIGGERED the download @@ -13029,11 +13052,12 @@ class MusicDatabase: INSERT INTO library_history (event_type, title, artist_name, album_name, quality, server_source, file_path, thumb_url, download_source, source_track_id, source_track_title, source_filename, - acoustid_result, source_artist, origin, origin_context) - VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) + acoustid_result, source_artist, origin, origin_context, + verification_status) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) """, (event_type, title, artist_name, album_name, quality, server_source, file_path, thumb_url, download_source, source_track_id, source_track_title, source_filename, - acoustid_result, source_artist, origin, origin_context)) + acoustid_result, source_artist, origin, origin_context, verification_status)) conn.commit() return True except Exception as e: diff --git a/tests/discovery/test_cancel_sync_idempotent.py b/tests/discovery/test_cancel_sync_idempotent.py new file mode 100644 index 00000000..f044ca9d --- /dev/null +++ b/tests/discovery/test_cancel_sync_idempotent.py @@ -0,0 +1,45 @@ +"""#702: a mirrored playlist (e.g. a ListenBrainz weekly) whose in-memory +discovery state was wiped by a restart must still cancel/reset cleanly instead of +404-ing into a permanent wedge. cancel_sync is the shared core for YouTube + +ListenBrainz cancel, so its idempotency is the fix.""" + +from __future__ import annotations + +import threading + +from core.discovery.endpoints import cancel_sync + + +def _lock(): + return threading.Lock() + + +def test_cancel_missing_key_is_idempotent_success_not_404(): + body, code = cancel_sync( + {}, 'state_was_wiped', label='YouTube', + not_found_message='YouTube playlist not found', + sync_lock=_lock(), sync_states={}, active_sync_workers={}) + assert code == 200 + assert body.get('success') is True + assert 'not found' not in str(body).lower() # the wedge message must be gone + + +def test_cancel_present_key_cancels_and_clears_worker(): + states = {'h': {'phase': 'syncing', 'sync_playlist_id': 'sp1'}} + sync_states, workers = {}, {'sp1': 'worker'} + body, code = cancel_sync( + states, 'h', label='YouTube', not_found_message='x', + sync_lock=_lock(), sync_states=sync_states, active_sync_workers=workers) + assert code == 200 and body['success'] is True + assert sync_states['sp1'] == {'status': 'cancelled'} + assert 'sp1' not in workers + assert states['h']['phase'] == 'discovered' + assert states['h']['sync_playlist_id'] is None + + +def test_cancel_present_with_no_active_sync_still_succeeds(): + states = {'h': {'phase': 'discovered', 'sync_playlist_id': None}} + body, code = cancel_sync( + states, 'h', label='YouTube', not_found_message='x', + sync_lock=_lock(), sync_states={}, active_sync_workers={}) + assert code == 200 and body['success'] is True diff --git a/tests/discovery/test_discovery_endpoints.py b/tests/discovery/test_discovery_endpoints.py index 5aa80d27..d78dd224 100644 --- a/tests/discovery/test_discovery_endpoints.py +++ b/tests/discovery/test_discovery_endpoints.py @@ -157,11 +157,15 @@ def test_spotify_data_takes_precedence_over_auto_fields(): # cancel_sync # --------------------------------------------------------------------------- -def test_cancel_sync_not_found_returns_404(): +def test_cancel_sync_missing_state_is_idempotent_success(): + # #702: a sync whose in-memory state was wiped (e.g. a restart) must cancel + # cleanly — cancelling a sync that isn't running is a no-op SUCCESS, not a + # 404 that permanently wedges the playlist. body, code = cancel_sync({}, 'missing', label='Tidal', not_found_message='Tidal playlist not found', **_cancel_infra()) - assert code == 404 - assert body == {"error": "Tidal playlist not found"} + assert code == 200 + assert body.get('success') is True + assert 'not found' not in str(body).lower() def test_cancel_sync_cancels_active_worker_and_reverts_state(): diff --git a/tests/imports/test_folder_artist_override.py b/tests/imports/test_folder_artist_override.py new file mode 100644 index 00000000..ad069d2b --- /dev/null +++ b/tests/imports/test_folder_artist_override.py @@ -0,0 +1,84 @@ +"""Folder-artist override can be disabled to keep an identified artist. + +Background +---------- + +The auto-import "parent folder artist override" used to fire unconditionally: +whenever the Staging path had >=2 levels and the top folder wasn't a category +word, it replaced the (already metadata-identified) artist with the top folder +name. A user who staged a mixed pile of singles under one container folder +named ``soulsync`` therefore got the album-artist of *every* file forced to +"soulsync" — even when the track was confidently resolved to a real artist +with a Spotify/MusicBrainz id. Navidrome groups by album-artist, so 65 singles +collapsed under a bogus "soulsync" artist. + +``resolve_folder_artist`` is the extracted, pure decision. It must keep the +identified artist when the feature is OFF, and reproduce the original +folder-derived artist when enabled. +""" + +from core.imports.folder_artist import resolve_folder_artist + + +# --- Disabled: never override ----------------------------------------------- + +def test_disabled_keeps_identified_artist_even_with_artist_album_structure(): + # The 'soulsync' mass mis-file: identified artist is real, folder is a + # generic container. With the feature off it must NOT be overridden. + assert resolve_folder_artist( + "soulsync/Bunny Girl/01 - Bunny Girl.flac", + identified_artist="1nonly, Ciscaux", + enabled=False, + ) is None + + +def test_disabled_returns_none_for_clean_artist_album_path(): + assert resolve_folder_artist( + "AC+DC/Back In Black/01 - Hells Bells.flac", + identified_artist="AC/DC", + enabled=False, + ) is None + + +# --- Enabled: original behaviour -------------------------------------------- + +def test_enabled_uses_top_folder_as_artist_when_it_differs(): + assert resolve_folder_artist( + "soulsync/Bunny Girl/01 - Bunny Girl.flac", + identified_artist="1nonly, Ciscaux", + enabled=True, + ) == "soulsync" + + +def test_enabled_skips_category_folder_and_uses_artist_above_it(): + assert resolve_folder_artist( + "Pink Floyd/Albums/The Wall/01 - In the Flesh.flac", + identified_artist="Some DJ", + enabled=True, + ) == "Pink Floyd" + + +def test_enabled_flat_file_has_no_folder_artist(): + assert resolve_folder_artist( + "01 - Bitch Lasagna.flac", + identified_artist="PewDiePie", + enabled=True, + ) is None + + +def test_enabled_no_override_when_folder_matches_identified(): + # Folder already equals the identified artist (case-insensitive) -> no-op. + assert resolve_folder_artist( + "AC_DC/Back In Black/01 - Hells Bells.flac", + identified_artist="ac_dc", + enabled=True, + ) is None + + +def test_enabled_top_level_category_word_is_not_an_artist(): + # 'singles/Track/file' — top folder is a category, not an artist. + assert resolve_folder_artist( + "singles/Headache/01 - Headache.flac", + identified_artist="Asal", + enabled=True, + ) is None diff --git a/tests/matching/test_acoustid_verification_aliases.py b/tests/matching/test_acoustid_verification_aliases.py index 987130c0..87874eeb 100644 --- a/tests/matching/test_acoustid_verification_aliases.py +++ b/tests/matching/test_acoustid_verification_aliases.py @@ -431,10 +431,10 @@ class TestAliasRescueLogging: records.append(record) handler = _ListHandler(level=_logging.INFO) - # Logger name is `soulsync.acoustid.verification` per - # `core.acoustid_verification`'s `get_logger("acoustid_verification")` - # — dot-separated, NOT underscored. - verifier_logger = _logging.getLogger('soulsync.acoustid.verification') + # The alias-aware comparison now lives in the shared core + # (`core.matching.audio_verification`, logger `audio_verification`), + # which is where the rescue diagnostic is emitted. + verifier_logger = _logging.getLogger('soulsync.audio_verification') verifier_logger.addHandler(handler) prior_level = verifier_logger.level verifier_logger.setLevel(_logging.INFO) diff --git a/tests/matching/test_audio_verification_core.py b/tests/matching/test_audio_verification_core.py new file mode 100644 index 00000000..09f6a2aa --- /dev/null +++ b/tests/matching/test_audio_verification_core.py @@ -0,0 +1,83 @@ +"""Shared audio-verification decision core: normalize() + evaluate(). + +One place for normalization + the PASS/SKIP/FAIL decision used by BOTH import-time +verification and the library AcoustID scan, so the two paths can't drift apart. +""" + +from core.matching.audio_verification import normalize, evaluate, Decision + + +def _rec(title, artist, duration=None): + return {"title": title, "artist": artist, "duration": duration} + + +def test_cross_script_vocal_credit_clean_match_passes(): + # Sawano / 澤野弘之 — <> stripped, alias bridges the artist, + # title matches too -> clean PASS (must never FAIL/quarantine). + out = evaluate( + "Call Your Name", "Sawano Hiroyuki", + [_rec("call your name", "澤野弘之 ")], + fingerprint_score=0.95, + aliases_provider=lambda: ["澤野弘之"], + ) + assert out.decision == Decision.PASS + + +def test_cross_script_ipa_title_skips_not_fails(): + # AcoustID returns an IPA-transcribed title that can't string-match, but the + # artist bridges cross-script -> SKIP (import anyway), never FAIL. + out = evaluate( + "Attack on Titan", "Sawano Hiroyuki", + [_rec("ətˈæk 0N tάɪtn", "澤野弘之")], + fingerprint_score=0.95, + aliases_provider=lambda: ["澤野弘之"], + ) + assert out.decision == Decision.SKIP + + +def test_clean_cross_script_match_passes(): + out = evaluate( + "Xl-Tt", "Sawano Hiroyuki", + [_rec("xl-tt", "澤野弘之")], + fingerprint_score=0.95, + aliases_provider=lambda: ["澤野弘之"], + ) + assert out.decision == Decision.PASS + + +def test_genuine_wrong_song_fails(): + out = evaluate( + "Yellow", "Coldplay", + [_rec("Rich Interlude", "Kendrick Lamar")], + fingerprint_score=0.85, + ) + assert out.decision == Decision.FAIL + + +def test_no_recordings_skips(): + out = evaluate("Whatever", "Someone", [], fingerprint_score=0.9) + assert out.decision == Decision.SKIP + + +def test_normalize_strips_paren_bracket_angle_and_keeps_cjk(): + assert normalize("澤野弘之 ") == "澤野弘之" + assert normalize("Clarity (Live at X) [Remastered]") == "clarity" + assert normalize("Attack on Titan ") == "attack on titan" + + +def test_normalize_strips_version_and_featuring(): + assert normalize("In My Feelings - Instrumental") == "in my feelings" + assert normalize("Song feat. Someone") == "song" + + +def test_normalize_keeps_plain_text(): + assert normalize("Sawano Hiroyuki") == "sawano hiroyuki" + + +def test_empty_expected_artist_does_not_fail(): + # Old scanner treated a missing expected artist as artist-match=1.0 + # (compare title only). The unified core must not FAIL a track just + # because the DB has no artist value. + out = evaluate("Some Track", "", [_rec("Some Track", "Whoever")], + fingerprint_score=0.95) + assert out.decision == Decision.PASS diff --git a/tests/matching/test_history_paths.py b/tests/matching/test_history_paths.py new file mode 100644 index 00000000..140abe91 --- /dev/null +++ b/tests/matching/test_history_paths.py @@ -0,0 +1,87 @@ +"""Seam tests for resolve_history_audio_path — the fallback chain a DESTRUCTIVE +delete trusts. The collision-safety rules (artist filter / single-candidate) are +what stop delete() from removing the wrong same-title file, so they're locked here.""" + +from __future__ import annotations + +from core.matching.history_paths import resolve_history_audio_path + + +def _resolver(existing=(), resolve_map=None, titled=None): + existing = set(existing) + resolve_map = resolve_map or {} + titled = titled or {} + return dict( + exists=lambda p: p in existing, + resolve_library_path=lambda raw: resolve_map.get(raw), + lookup_titled_paths=lambda title: list(titled.get(title.lower(), [])), + ) + + +def test_recorded_path_used_when_it_exists(): + r = resolve_history_audio_path({'file_path': '/m/a.mp3'}, **_resolver(existing={'/m/a.mp3'})) + assert r == '/m/a.mp3' + + +def test_falls_back_to_prefix_resolved_path(): + r = resolve_history_audio_path( + {'file_path': '/transfer/a.mp3'}, + **_resolver(existing={'/library/a.mp3'}, resolve_map={'/transfer/a.mp3': '/library/a.mp3'})) + assert r == '/library/a.mp3' + + +def test_tracks_table_single_candidate_no_artist(): + r = resolve_history_audio_path( + {'file_path': '/gone.mp3', 'title': 'Song'}, + **_resolver(existing={'/library/song.mp3'}, + resolve_map={'/lib/song.mp3': '/library/song.mp3'}, + titled={'song': ['/lib/song.mp3']})) + assert r == '/library/song.mp3' + + +def test_collision_no_artist_multiple_candidates_returns_none(): + # THE safety rule: same title, no artist to disambiguate -> refuse to guess + # (delete() must not remove an arbitrary one of two same-title files). + r = resolve_history_audio_path( + {'file_path': '', 'title': 'Intro'}, + **_resolver(existing={'/library/a/intro.mp3', '/library/b/intro.mp3'}, + resolve_map={'/a/intro.mp3': '/library/a/intro.mp3', '/b/intro.mp3': '/library/b/intro.mp3'}, + titled={'intro': ['/a/intro.mp3', '/b/intro.mp3']})) + assert r is None + + +def test_artist_filter_picks_only_the_matching_path(): + # Two same-title files by different artists -> only the one whose path + # mentions the row's artist is eligible (won't delete the other artist's file). + r = resolve_history_audio_path( + {'file_path': '', 'title': 'Intro', 'artist_name': 'Alpha'}, + **_resolver(existing={'/music/Alpha/intro.mp3', '/music/Beta/intro.mp3'}, + resolve_map={'/Alpha/intro.mp3': '/music/Alpha/intro.mp3', + '/Beta/intro.mp3': '/music/Beta/intro.mp3'}, + titled={'intro': ['/Alpha/intro.mp3', '/Beta/intro.mp3']})) + assert r == '/music/Alpha/intro.mp3' + + +def test_artist_named_but_no_path_mentions_it_returns_none(): + r = resolve_history_audio_path( + {'file_path': '', 'title': 'Intro', 'artist_name': 'Gamma'}, + **_resolver(existing={'/music/Alpha/intro.mp3'}, + resolve_map={'/Alpha/intro.mp3': '/music/Alpha/intro.mp3'}, + titled={'intro': ['/Alpha/intro.mp3']})) + assert r is None + + +def test_no_title_returns_none(): + assert resolve_history_audio_path({'file_path': '/gone.mp3'}, **_resolver()) is None + + +def test_nothing_resolves_returns_none(): + assert resolve_history_audio_path( + {'file_path': '/gone.mp3', 'title': 'X'}, + **_resolver(titled={'x': ['/also/gone.mp3']})) is None + + +def test_empty_lookup_returns_none(): + # DB-error proxy: lookup yields [] -> None (never a stray delete target). + assert resolve_history_audio_path( + {'file_path': '', 'title': 'X'}, **_resolver(titled={})) is None diff --git a/tests/matching/test_normalize_slash_851.py b/tests/matching/test_normalize_slash_851.py new file mode 100644 index 00000000..9b4b3c8c --- /dev/null +++ b/tests/matching/test_normalize_slash_851.py @@ -0,0 +1,24 @@ +"""#851: a '/' or ':' in a title must normalize the same as a source filename +that substituted '_' for them, so the candidate matcher stops rejecting valid +downloads (e.g. Sawano's "You See Big Girl / T:T" vs on-disk "..._ T_T").""" + +from __future__ import annotations + +from core.matching.audio_verification import normalize, similarity + + +def test_slash_colon_title_matches_underscore_source(): + assert normalize("You See Big Girl / T:T") == normalize("You See Big Girl _ T_T") + assert similarity("You See Big Girl / T:T", "You See Big Girl _ T_T") == 1.0 + + +def test_separators_become_word_boundaries(): + assert normalize("Re:Zero") == "re zero" + assert normalize("AC/DC") == "ac dc" + assert normalize("T_T") == "t t" + + +def test_joined_variant_stays_above_title_threshold(): + # Spacing a separator must not drop a joined-variant match below 0.70. + assert similarity("AC/DC", "ACDC") >= 0.70 + assert similarity("12:05", "1205") >= 0.70 diff --git a/tests/matching/test_verification_status.py b/tests/matching/test_verification_status.py new file mode 100644 index 00000000..5e37813e --- /dev/null +++ b/tests/matching/test_verification_status.py @@ -0,0 +1,36 @@ +"""Verification-status vocabulary + mapping (DB column / file tag / UI badge).""" + +from core.matching.verification_status import ( + VERIFIED, UNVERIFIED, FORCE_IMPORTED, HUMAN_VERIFIED, + status_from_acoustid_result, status_for_import, +) + + +def test_acoustid_result_maps_to_status(): + assert status_from_acoustid_result('pass') == VERIFIED + assert status_from_acoustid_result('skip') == UNVERIFIED + # disabled / error / unknown -> no claim either way + assert status_from_acoustid_result('disabled') is None + assert status_from_acoustid_result('error') is None + assert status_from_acoustid_result(None) is None + + +def test_force_import_context_wins_over_acoustid(): + ctx = {'_version_mismatch_fallback': 'instrumental', '_acoustid_result': 'pass'} + assert status_for_import(ctx) == FORCE_IMPORTED + + +def test_status_for_import_falls_back_to_acoustid_result(): + assert status_for_import({'_acoustid_result': 'pass'}) == VERIFIED + assert status_for_import({'_acoustid_result': 'skip'}) == UNVERIFIED + assert status_for_import({}) is None + + +def test_approved_quarantine_import_is_human_verified(): + # The user clicked Approve on a quarantine entry — an explicit human + # decision about THIS file. Outranks both the fallback flag and whatever + # the (earlier, failed) verification said. + ctx = {'_approved_quarantine_trigger': 'acoustid', + '_version_mismatch_fallback': 'instrumental', + '_acoustid_result': 'fail'} + assert status_for_import(ctx) == HUMAN_VERIFIED diff --git a/tests/repair_jobs/test_relocate.py b/tests/repair_jobs/test_relocate.py new file mode 100644 index 00000000..cb8be997 --- /dev/null +++ b/tests/repair_jobs/test_relocate.py @@ -0,0 +1,117 @@ +"""#704: relocate an AcoustID-mismatched file to staging for re-import — pure +orchestration (retag -> move -> drop row) with the side effects injected, plus a +real-file move through safe_move_file.""" + +from __future__ import annotations + +import os + +import pytest + +from core.repair_jobs.relocate import staging_destination, relocate_mismatch_to_staging + + +# ── staging_destination: never overwrite an unrelated staged file ────────── +def test_staging_destination_no_collision(): + assert staging_destination('/stg', 'Song.mp3', exists=lambda p: False) == os.path.join('/stg', 'Song.mp3') + + +def test_staging_destination_suffixes_on_collision(): + taken = {os.path.join('/stg', 'Song.mp3'), os.path.join('/stg', 'Song (1).mp3')} + assert staging_destination('/stg', 'Song.mp3', exists=lambda p: p in taken) == os.path.join('/stg', 'Song (2).mp3') + + +# ── relocate orchestration ───────────────────────────────────────────────── +class _Spy: + def __init__(self): self.tagged = None; self.moved = None; self.dropped = False + def write_tags(self, path, updates): self.tagged = (path, updates) + def move(self, src, dst): self.moved = (src, dst) + def drop(self): self.dropped = True + + +def test_relocate_happy_path(): + s = _Spy() + dest = relocate_mismatch_to_staging( + '/lib/Artist X/Album Y/03 - x.mp3', '/stg', {'title': 'Real Song'}, + write_tags=s.write_tags, move_file=s.move, drop_db_row=s.drop, exists=lambda p: False) + assert s.tagged == ('/lib/Artist X/Album Y/03 - x.mp3', {'title': 'Real Song'}) + assert s.moved == ('/lib/Artist X/Album Y/03 - x.mp3', os.path.join('/stg', '03 - x.mp3')) + assert s.dropped is True + assert dest == os.path.join('/stg', '03 - x.mp3') + + +def test_relocate_tag_failure_still_relocates(): + s = _Spy() + def boom(*a): raise RuntimeError('tag write failed') + dest = relocate_mismatch_to_staging( + '/lib/x.mp3', '/stg', {'title': 'T'}, + write_tags=boom, move_file=s.move, drop_db_row=s.drop, exists=lambda p: False) + assert s.moved and s.dropped and dest == os.path.join('/stg', 'x.mp3') + + +def test_relocate_failed_move_does_not_drop_row(): + # The library row must survive a failed move (no orphaning). + s = _Spy() + def bad_move(src, dst): raise OSError('cross-device move failed') + with pytest.raises(OSError): + relocate_mismatch_to_staging('/lib/x.mp3', '/stg', None, + write_tags=s.write_tags, move_file=bad_move, drop_db_row=s.drop, exists=lambda p: False) + assert s.dropped is False + + +def test_relocate_no_tag_updates_skips_write(): + s = _Spy() + relocate_mismatch_to_staging('/lib/x.mp3', '/stg', None, + write_tags=s.write_tags, move_file=s.move, drop_db_row=s.drop, exists=lambda p: False) + assert s.tagged is None and s.moved is not None + + +# ── real-file move through the actual safe_move_file ─────────────────────── +def test_real_file_moves_into_staging(tmp_path): + from core.imports.file_ops import safe_move_file + lib = tmp_path / 'lib' / 'Artist X' / 'Album Y'; lib.mkdir(parents=True) + src = lib / '03 - wrong.mp3'; src.write_bytes(b'\x00' * 64) + stg = tmp_path / 'Staging'; stg.mkdir() + dropped = [] + dest = relocate_mismatch_to_staging( + str(src), str(stg), None, + write_tags=lambda *a: None, move_file=safe_move_file, + drop_db_row=lambda: dropped.append(True), exists=os.path.exists) + assert not src.exists() # moved out of the wrong folder + assert os.path.exists(dest) # present in staging + assert os.path.dirname(dest) == str(stg) + assert dropped == [True] + + +# ── handler integration: _fix_acoustid_mismatch relocate end-to-end ───────── +def test_relocate_handler_moves_file_and_drops_row(tmp_path): + from database.music_database import MusicDatabase + from core.repair_worker import RepairWorker + + db = MusicDatabase(str(tmp_path / 'm.db')) + lib = tmp_path / 'music' / 'Wrong Artist' / 'Wrong Album' + lib.mkdir(parents=True) + wrong = lib / '03 - wrong.mp3' + wrong.write_bytes(b'\x00' * 64) + staging = tmp_path / 'Staging'; staging.mkdir() + + with db._get_connection() as conn: + conn.execute("INSERT OR REPLACE INTO artists (id, name, server_source) VALUES ('a1','Wrong Artist','plex')") + conn.execute("INSERT OR REPLACE INTO albums (id, title, artist_id) VALUES (10,'Wrong Album','a1')") + conn.execute("INSERT INTO tracks (id, album_id, artist_id, title, track_number, duration, file_path, server_source) " + "VALUES ('t1',10,'a1','Wrong Title',3,100,?, 'plex')", (str(wrong),)) + conn.commit() + + worker = RepairWorker(db) + worker._config_manager = type('C', (), { + 'get': staticmethod(lambda k, d=None: str(staging) if k == 'import.staging_path' else d)})() + + res = worker._fix_acoustid_mismatch( + 'track', 't1', str(wrong), + {'_fix_action': 'relocate', 'acoustid_title': 'Real Song', 'acoustid_artist': 'Real Artist'}) + + assert res['success'] is True and res['action'] == 'relocated' + assert not wrong.exists() # gone from the wrong album folder + assert (staging / '03 - wrong.mp3').exists() # now staged for re-import + with db._get_connection() as conn: + assert conn.execute("SELECT COUNT(*) FROM tracks WHERE id='t1'").fetchone()[0] == 0 # stale row dropped diff --git a/tests/test_acoustid_normalize_angle_annotations.py b/tests/test_acoustid_normalize_angle_annotations.py new file mode 100644 index 00000000..42e34e19 --- /dev/null +++ b/tests/test_acoustid_normalize_angle_annotations.py @@ -0,0 +1,36 @@ +"""`_normalize` must strip ``<...>`` annotations like the AcoustID/MusicBrainz +vocalist credit ``澤野弘之 ``. + +User report: a correct anime-OST track ("Attack on Titan" by "Sawano Hiroyuki") +was false-quarantined. AcoustID returned the artist as +``澤野弘之 ``. The kanji ``澤野弘之`` IS the artist and the +MusicBrainz alias bridge matches it — but `_normalize` stripped ``()`` and +``[]`` annotations, NOT ``<...>``, so the trailing "vocal mika kobayashi" words +diluted the alias comparison down to ~0.28 (below ARTIST_MATCH_THRESHOLD). That +in turn blocked the existing cross-script SKIP safety net (issue #797), which is +gated on ``artist_sim >= threshold``, so the file FAILED and was quarantined. + +Stripping ``<...>`` restores the artist to ``澤野弘之`` so the alias match (and +thus the cross-script SKIP) works. +""" + +from core.acoustid_verification import _normalize, _similarity + + +def test_normalize_strips_angle_bracket_vocalist_annotation(): + assert _normalize("澤野弘之 ") == "澤野弘之" + + +def test_normalize_strips_angle_brackets_latin(): + assert _normalize("Attack on Titan ") == "attack on titan" + + +def test_vocalist_annotation_no_longer_dilutes_artist_similarity(): + # The kanji artist with a vocalist credit must compare as identical to the + # bare kanji artist — this is what lets the alias bridge clear the threshold. + assert _similarity("澤野弘之", "澤野弘之 ") == 1.0 + + +def test_normalize_keeps_plain_text_untouched(): + # Guard: no angle brackets -> unchanged behaviour. + assert _normalize("Sawano Hiroyuki") == "sawano hiroyuki" diff --git a/tests/test_acoustid_scanner.py b/tests/test_acoustid_scanner.py index 1993b099..1c83cac6 100644 --- a/tests/test_acoustid_scanner.py +++ b/tests/test_acoustid_scanner.py @@ -199,7 +199,11 @@ def test_scanner_still_flags_genuine_artist_mismatch(): 'best_score': 0.99, 'recordings': [{ 'title': 'Some Track', - 'artist': 'Different Band, Other Person & Random Featuring', + # Clearly-different multi-value credit (artist sim < 0.30). The + # unified core gives 0.30-0.60 ("ambiguous") the benefit of the + # doubt, so a genuine-mismatch assertion needs an artist that's + # unambiguously different. + 'artist': 'Metallica, Slayer & Anthrax', }], }, ) @@ -468,7 +472,9 @@ def test_scanner_falls_back_to_db_when_file_tag_missing(monkeypatch): 'best_score': 0.99, 'recordings': [{ 'title': 'Some Track', - 'artist': 'Different Band', + # Unambiguously different artist (sim < 0.30) so the unified + # core flags it (0.30-0.60 would be treated as ambiguous). + 'artist': 'Metallica', }], }, ) @@ -779,3 +785,173 @@ def test_scanner_still_flags_when_duration_matches(): ) assert len(captured_findings) == 1 + + +def test_scanner_does_not_flag_cross_script_when_alias_bridges(monkeypatch): + """Anime-OST track: AcoustID returns the kanji artist with a + credit. With the MusicBrainz alias bridging 澤野弘之 ↔ Sawano Hiroyuki, the + unified verification core recognises the match, so the library scan must NOT + create a false 'Wrong download' finding (it did before, stripping all + non-ASCII and never consulting aliases).""" + import core.repair_jobs.acoustid_scanner as scanner_mod + monkeypatch.setattr(scanner_mod, "_resolve_expected_artist_aliases", + lambda name: ["澤野弘之"], raising=False) + job = AcoustIDScannerJob() + captured = [] + context = _make_finding_capturing_context( + track_row=("7", "Call Your Name", "Sawano Hiroyuki", + "/music/cyn.flac", 15, "Attack on Titan OST", None, None), + captured=captured, + ) + fake_acoustid = SimpleNamespace( + fingerprint_and_lookup=lambda fpath: { + 'best_score': 0.97, + 'recordings': [{'title': 'call your name', + 'artist': '澤野弘之 '}], + }, + ) + result = JobResultStub() + job._scan_file('/music/cyn.flac', '7', + {'title': 'Call Your Name', 'artist': 'Sawano Hiroyuki'}, + fake_acoustid, context, result, + fp_threshold=0.85, title_threshold=0.85, artist_threshold=0.6) + assert captured == [], f"cross-script track false-flagged: {captured}" + + +def _force_imported_scan(monkeypatch): + """Drive a scan over a force-imported file whose fingerprint clearly + mismatches. Returns the captured findings.""" + import core.repair_jobs.acoustid_scanner as scanner_mod + monkeypatch.setattr(scanner_mod, "_resolve_expected_artist_aliases", + lambda name: [], raising=False) + monkeypatch.setattr( + 'core.tag_writer.read_file_tags', + lambda fpath: {'artist': None, 'verification_status': 'force_imported'}, + ) + job = AcoustIDScannerJob() + captured = [] + context = _make_finding_capturing_context( + track_row=("42", "Wanted Song", "Real Artist", + "/music/ws.flac", 1, "Album", None, None), + captured=captured, + ) + fake_acoustid = SimpleNamespace( + fingerprint_and_lookup=lambda fpath: { + 'best_score': 0.99, + 'recordings': [{'title': 'Wanted Song - Instrumental', + 'artist': 'Real Artist'}], + }, + ) + result = JobResultStub() + job._scan_file('/music/ws.flac', '42', + {'title': 'Wanted Song', 'artist': 'Real Artist'}, + fake_acoustid, context, result, + fp_threshold=0.85, title_threshold=0.85, artist_threshold=0.6) + return captured + + +def test_force_imported_mismatch_is_reported_as_informational(monkeypatch): + # The user opted into the fallback, so the scan must still TELL them the + # file is e.g. an instrumental — but as 'info', clearly marked, not as a + # red Wrong-download warning. Only human_verified short-circuits the scan. + captured = _force_imported_scan(monkeypatch) + assert len(captured) == 1 + assert captured[0]['severity'] == 'info' + assert captured[0]['details'].get('force_imported') is True + assert 'Force-imported' in captured[0]['title'] + + +def test_human_verified_files_are_never_scanned(monkeypatch): + import core.repair_jobs.acoustid_scanner as scanner_mod + monkeypatch.setattr(scanner_mod, "_resolve_expected_artist_aliases", + lambda name: [], raising=False) + monkeypatch.setattr('core.tag_writer.read_file_tags', + lambda fpath: {'artist': None, 'verification_status': 'human_verified'}) + job = AcoustIDScannerJob() + captured = [] + context = _make_finding_capturing_context( + track_row=("7", "T", "A", "/music/t.flac", 1, "Al", None, None), + captured=captured) + fake = SimpleNamespace(fingerprint_and_lookup=lambda f: { + 'best_score': 0.99, + 'recordings': [{'title': 'Totally Different', 'artist': 'Metallica'}]}) + job._scan_file('/music/t.flac', '7', {'title': 'T', 'artist': 'A'}, + fake, context, JobResultStub(), + fp_threshold=0.85, title_threshold=0.85, artist_threshold=0.6) + assert captured == [] + + +# --------------------------------------------------------------------------- +# Scan-outcome persistence — the scan feeds the same review pipeline as +# import-time verification (tag + tracks row + library_history rows). +# --------------------------------------------------------------------------- + + +def _run_persistence_scan(monkeypatch, *, file_status, aid_artist, expected_artist): + """Drive one _scan_file call and return (status_updates, tag_writes) where + status_updates is the list of (query, params) UPDATEs the scanner ran.""" + import core.repair_jobs.acoustid_scanner as scanner_mod + monkeypatch.setattr(scanner_mod, "_resolve_expected_artist_aliases", + lambda name: [], raising=False) + monkeypatch.setattr( + 'core.tag_writer.read_file_tags', + lambda fpath: {'artist': None, 'verification_status': file_status}) + tag_writes = [] + monkeypatch.setattr( + 'core.tag_writer.write_verification_status', + lambda fpath, status: tag_writes.append((fpath, status)) or True) + job = AcoustIDScannerJob() + captured = [] + context = _make_finding_capturing_context( + track_row=("9", "Call Your Name", expected_artist, + "/music/cyn.flac", 1, "Album", None, None), + captured=captured) + fake = SimpleNamespace(fingerprint_and_lookup=lambda f: { + 'best_score': 0.97, + 'recordings': [{'title': 'Call Your Name', 'artist': aid_artist}]}) + job._scan_file('/music/cyn.flac', '9', + {'title': 'Call Your Name', 'artist': expected_artist}, + fake, context, JobResultStub(), + fp_threshold=0.85, title_threshold=0.85, artist_threshold=0.6) + conn = context.db._get_connection() + updates = [(q, p) for q, p in conn.cursor().executed + if 'verification_status' in q] + return updates, tag_writes, captured + + +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). + updates, tag_writes, captured = _run_persistence_scan( + monkeypatch, file_status=None, + aid_artist='Sawano Hiroyuki', expected_artist='Sawano Hiroyuki') + 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') + 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. + updates, tag_writes, captured = _run_persistence_scan( + monkeypatch, file_status=None, + aid_artist='Mantilla', expected_artist='Metallica') + 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') + for q, p in updates) + + +def test_scan_skip_does_not_downgrade_verified(monkeypatch): + # A SKIP must not downgrade an import-time 'verified' (that check ran with + # richer candidate metadata). Status is refreshed, tag untouched. + updates, tag_writes, captured = _run_persistence_scan( + monkeypatch, file_status='verified', + aid_artist='Mantilla', expected_artist='Metallica') + assert captured == [] + assert tag_writes == [] + assert any('tracks' in q and p == ('verified', '9') for q, p in updates) diff --git a/tests/test_artist_album_list_cache_nonspotify.py b/tests/test_artist_album_list_cache_nonspotify.py new file mode 100644 index 00000000..87f4ff17 --- /dev/null +++ b/tests/test_artist_album_list_cache_nonspotify.py @@ -0,0 +1,165 @@ +from __future__ import annotations + +from unittest.mock import MagicMock + +import core.deezer_client as deezer_mod +import core.discogs_client as discogs_mod +import core.itunes_client as itunes_mod +from core.metadata.artist_album_cache import ( + get_cached_artist_album_items, + get_cached_artist_album_payload, + make_artist_album_cache_key, + store_artist_album_items, +) + + +class MemoryCache: + def __init__(self): + self.entities = {} + + def get_entity(self, source, entity_type, entity_id): + return self.entities.get((source, entity_type, entity_id)) + + def store_entity(self, source, entity_type, entity_id, raw_data): + self.entities[(source, entity_type, entity_id)] = raw_data + + def store_entities_bulk(self, *args, **kwargs): + return None + + +def test_artist_album_cache_helper_round_trips_items_and_payload(): + cache = MemoryCache() + + store_artist_album_items( + cache, + 'deezer', + 'artist-1', + [{'id': 'album-1'}], + album_type='album,single', + limit=200, + extra_fields={'artist_name': 'Artist'}, + ) + + assert make_artist_album_cache_key('artist-1', 'album,single', 200) == 'artist-1_albums_album_single_200' + assert make_artist_album_cache_key('artist-1', 'album,single', 200, include_limit=False) == 'artist-1_albums_album_single' + assert get_cached_artist_album_items(cache, 'deezer', 'artist-1', limit=200) == [{'id': 'album-1'}] + assert get_cached_artist_album_payload(cache, 'deezer', 'artist-1', limit=200)['artist_name'] == 'Artist' + + +def test_deezer_artist_albums_reuses_list_cache(monkeypatch): + cache = MemoryCache() + monkeypatch.setattr(deezer_mod, 'get_metadata_cache', lambda: cache) + client = deezer_mod.DeezerClient.__new__(deezer_mod.DeezerClient) + client._api_get = MagicMock(return_value={ + 'data': [{ + 'id': 123, + 'title': 'Cached Deezer Album', + 'record_type': 'album', + 'release_date': '2024-01-01', + 'nb_tracks': 10, + 'artist': {'id': 7, 'name': 'Artist'}, + }] + }) + + first = client.get_artist_albums('7', limit=200) + second = client.get_artist_albums('7', limit=200) + + assert [album.name for album in first] == ['Cached Deezer Album'] + assert [album.name for album in second] == ['Cached Deezer Album'] + client._api_get.assert_called_once() + + +def test_itunes_artist_albums_reuses_list_cache_and_skips_validation(monkeypatch): + cache = MemoryCache() + monkeypatch.setattr(itunes_mod, 'get_metadata_cache', lambda: cache) + client = itunes_mod.iTunesClient.__new__(itunes_mod.iTunesClient) + client._lookup = MagicMock(side_effect=[ + [{ + 'wrapperType': 'collection', + 'collectionId': 456, + 'collectionName': 'Cached iTunes Album', + 'artistId': 8, + 'artistName': 'Artist', + 'trackCount': 10, + 'collectionExplicitness': 'notExplicit', + 'releaseDate': '2024-01-01T00:00:00Z', + }] + ]) + + first = client.get_artist_albums('8', limit=200) + second = client.get_artist_albums('8', limit=200) + + assert [album.name for album in first] == ['Cached iTunes Album'] + assert [album.name for album in second] == ['Cached iTunes Album'] + client._lookup.assert_called_once_with(id='8', entity='album', limit=200) + + +def test_discogs_artist_albums_reuses_list_cache(monkeypatch): + cache = MemoryCache() + monkeypatch.setattr(discogs_mod, 'get_metadata_cache', lambda: cache) + client = discogs_mod.DiscogsClient.__new__(discogs_mod.DiscogsClient) + client._api_get = MagicMock(side_effect=[ + {'name': 'Artist'}, + {'releases': [{ + 'id': 789, + 'type': 'master', + 'role': 'Main', + 'title': 'Cached Discogs Album', + 'artist': 'Artist', + 'year': 2024, + }]}, + ]) + + first = client.get_artist_albums('9', limit=50) + second = client.get_artist_albums('9', limit=50) + + assert [album.name for album in first] == ['Cached Discogs Album'] + assert [album.name for album in second] == ['Cached Discogs Album'] + assert client._api_get.call_count == 2 + + +def _deezer_album(i): + return { + 'id': i, 'title': f'Album {i}', 'record_type': 'album', + 'release_date': '2024-01-01', 'nb_tracks': 10, + 'artist': {'id': 7, 'name': 'Artist'}, + } + + +def test_deezer_partial_pagination_not_cached(monkeypatch): + """#853 follow-up: a transient/malformed error mid-pagination must NOT cache a + partial discography (mirrors Spotify's truncated-fetch guard) — otherwise an + incomplete album list serves from cache until TTL.""" + cache = MemoryCache() + monkeypatch.setattr(deezer_mod, 'get_metadata_cache', lambda: cache) + client = deezer_mod.DeezerClient.__new__(deezer_mod.DeezerClient) + + full_page = {'data': [_deezer_album(i) for i in range(100)]} # full → forces page 2 + # page 1 ok, page 2 errors (None) → incomplete, on BOTH attempts + client._api_get = MagicMock(side_effect=[full_page, None, full_page, None]) + + first = client.get_artist_albums('7', limit=200) + assert len(first) == 100 # page-1 albums still returned + + second = client.get_artist_albums('7', limit=200) + assert len(second) == 100 + # No partial cache → the second call refetched (4 api calls total), instead of + # serving a permanently-incomplete discography from cache (which would be 2). + assert client._api_get.call_count == 4 + + +def test_deezer_complete_multipage_is_cached(monkeypatch): + """A clean multi-page pagination still caches (guard didn't break the happy path).""" + cache = MemoryCache() + monkeypatch.setattr(deezer_mod, 'get_metadata_cache', lambda: cache) + client = deezer_mod.DeezerClient.__new__(deezer_mod.DeezerClient) + + page1 = {'data': [_deezer_album(i) for i in range(100)]} # full + page2 = {'data': [_deezer_album(i) for i in range(100, 150)]} # short → clean end + client._api_get = MagicMock(side_effect=[page1, page2]) + + first = client.get_artist_albums('7', limit=200) + assert len(first) == 150 + second = client.get_artist_albums('7', limit=200) # served from cache + assert len(second) == 150 + assert client._api_get.call_count == 2 # no refetch → cached diff --git a/tests/test_discogs_collection_id_tagging.py b/tests/test_discogs_collection_id_tagging.py new file mode 100644 index 00000000..a1cdc663 --- /dev/null +++ b/tests/test_discogs_collection_id_tagging.py @@ -0,0 +1,40 @@ +"""#848 follow-up: the Your Albums Discogs collection sync stores album IDs +TAGGED ('r') like search/discography, so every stored Discogs album ID is +uniform and re-fetches route to the correct endpoint (no master/release collision). + +The divergence didn't cause a live bug (the pool dedups by normalized name, and +discogs_release_id is only ever re-fetched — which handles bare too); this locks +in the consistency so a future ID comparison can't be tripped by mixed forms. +""" + +from __future__ import annotations + +import pytest + +from core.discogs_client import _tag_discogs_album_id, _discogs_album_endpoints +from database.music_database import MusicDatabase + + +def test_collection_release_id_is_tagged_and_routes_to_releases_only(tmp_path): + db = MusicDatabase(str(tmp_path / "m.db")) + # Exactly what the Your Albums Discogs sync now passes (collection = releases). + ok = db.upsert_liked_album( + album_name="Some Album", artist_name="Some Artist", + source_service="discogs", + source_id=_tag_discogs_album_id(7361634, "release"), source_id_type="discogs", + profile_id=1, + ) + assert ok + with db._get_connection() as conn: + row = conn.execute( + "SELECT discogs_release_id FROM liked_albums_pool WHERE profile_id = 1" + ).fetchone() + assert row is not None + assert row["discogs_release_id"] == "r7361634" # stored tagged, not bare + # A tagged collection ID routes ONLY to /releases — never /masters — so it + # can't hit the master/release collision #848 fixed. + assert _discogs_album_endpoints("r7361634") == ["/releases/7361634"] + + +def test_tagged_collection_id_never_hits_masters(): + assert "/masters/" not in " ".join(_discogs_album_endpoints(_tag_discogs_album_id(123, "release"))) diff --git a/tests/test_discogs_id_typing.py b/tests/test_discogs_id_typing.py new file mode 100644 index 00000000..f1ca71d4 --- /dev/null +++ b/tests/test_discogs_id_typing.py @@ -0,0 +1,189 @@ +"""Tests for Discogs master-vs-release ID disambiguation. + +Discogs has two album object types — masters (``/masters/{id}``) and releases +(``/releases/{id}``) — whose numeric IDs share ONE namespace, so release N and +master N are different albums. The old fetch code tried ``/masters/{id}`` first +and fell back to ``/releases/{id}``; because ``search_albums`` only ever yields +RELEASE ids, any release id that happened to collide with a real master id +returned a valid-but-WRONG album (the fallback never fired). See +``core.discogs_client`` ID-typing helpers. + +The fix tags the type into the id string ('r12345' / 'm12345') at parse time and +routes each fetch to the matching endpoint, with legacy bare ids tried +release-first (and master only as a fallback). +""" + +import pytest + +from core.discogs_client import ( + Album, + DiscogsClient, + _discogs_album_endpoints, + _discogs_album_kind, + _tag_discogs_album_id, +) + + +# --------------------------------------------------------------------------- +# _discogs_album_endpoints — the routing table +# --------------------------------------------------------------------------- + +def test_release_tagged_id_hits_only_releases(): + assert _discogs_album_endpoints('r12345') == ['/releases/12345'] + + +def test_master_tagged_id_hits_only_masters(): + assert _discogs_album_endpoints('m12345') == ['/masters/12345'] + + +def test_legacy_bare_id_tries_release_first_then_master(): + # The crux of the fix: bare ids are release-first, NOT master-first. + assert _discogs_album_endpoints('12345') == ['/releases/12345', '/masters/12345'] + + +def test_unusable_ids_return_empty(): + assert _discogs_album_endpoints('') == [] + assert _discogs_album_endpoints(None) == [] + assert _discogs_album_endpoints('not-an-id') == [] + # A lone letter prefix with no digits is not a valid tagged id. + assert _discogs_album_endpoints('r') == [] + + +# --------------------------------------------------------------------------- +# _discogs_album_kind / _tag_discogs_album_id — classification + tagging +# --------------------------------------------------------------------------- + +def test_kind_from_explicit_type(): + assert _discogs_album_kind({'type': 'master'}) == 'master' + assert _discogs_album_kind({'type': 'release'}) == 'release' + + +def test_kind_full_master_detail_has_main_release(): + # Full /masters/{id} responses carry no `type`, but do carry `main_release`. + assert _discogs_album_kind({'id': 1, 'main_release': 42, 'title': 'X'}) == 'master' + + +def test_kind_full_release_detail_defaults_release(): + # Full /releases/{id} responses carry no `type` and no `main_release`. + assert _discogs_album_kind({'id': 1, 'title': 'X', 'master_id': 42}) == 'release' + + +def test_tagging(): + assert _tag_discogs_album_id('123', 'master') == 'm123' + assert _tag_discogs_album_id('123', 'release') == 'r123' + assert _tag_discogs_album_id('', 'release') == '' + assert _tag_discogs_album_id(None, 'master') == '' + + +# --------------------------------------------------------------------------- +# Album.from_discogs_release — the single tagging point +# --------------------------------------------------------------------------- + +def test_search_result_tagged_as_release(): + # search_albums uses type=release; results carry type='release'. + album = Album.from_discogs_release({'id': 999, 'title': 'Radiohead - OK Computer', + 'type': 'release'}) + assert album.id == 'r999' + + +def test_discography_master_tagged_as_master(): + album = Album.from_discogs_release({'id': 777, 'title': 'OK Computer', 'type': 'master'}) + assert album.id == 'm777' + + +def test_full_master_detail_tagged_as_master(): + album = Album.from_discogs_release({'id': 777, 'title': 'OK Computer', 'main_release': 12}) + assert album.id == 'm777' + + +# --------------------------------------------------------------------------- +# Fetch routing — the regression lock on the original bug +# --------------------------------------------------------------------------- + +class _FakeCache: + """Always-miss metadata cache.""" + def get_entity(self, *a, **k): + return None + + def store_entity(self, *a, **k): + pass + + def get_search_results(self, *a, **k): + return None + + def store_search_results(self, *a, **k): + pass + + def store_entities_bulk(self, *a, **k): + pass + + +@pytest.fixture +def client(monkeypatch): + monkeypatch.setattr('core.discogs_client.get_metadata_cache', lambda: _FakeCache()) + return DiscogsClient(token='test-token') + + +def _record(client, monkeypatch, responses): + """Replace _api_get with a recorder that returns `responses[path]`.""" + calls = [] + + def fake_api_get(endpoint, params=None): + calls.append(endpoint) + return responses.get(endpoint) + + monkeypatch.setattr(client, '_api_get', fake_api_get) + return calls + + +def test_get_album_release_id_never_hits_masters(client, monkeypatch): + """A release-tagged id must NOT touch /masters — that was the bug.""" + calls = _record(client, monkeypatch, { + '/releases/249504': {'id': 249504, 'title': 'The Real Album', 'artists': [{'name': 'A'}]}, + '/masters/249504': {'id': 249504, 'title': 'A DIFFERENT Album', 'artists': [{'name': 'B'}]}, + }) + result = client.get_album('r249504', include_tracks=False) + assert calls == ['/releases/249504'] # master endpoint never consulted + assert result['name'] == 'The Real Album' + + +def test_get_album_master_id_hits_masters_only(client, monkeypatch): + calls = _record(client, monkeypatch, { + '/masters/777': {'id': 777, 'title': 'Master Album', 'main_release': 1}, + }) + result = client.get_album('m777', include_tracks=False) + assert calls == ['/masters/777'] + assert result['name'] == 'Master Album' + + +def test_legacy_bare_id_release_first(client, monkeypatch): + """Legacy untagged id resolves as a release without ever hitting /masters + when the release lookup succeeds.""" + calls = _record(client, monkeypatch, { + '/releases/249504': {'id': 249504, 'title': 'The Real Album'}, + '/masters/249504': {'id': 249504, 'title': 'A DIFFERENT Album'}, + }) + result = client.get_album('249504', include_tracks=False) + assert calls == ['/releases/249504'] + assert result['name'] == 'The Real Album' + + +def test_legacy_bare_id_falls_back_to_master(client, monkeypatch): + """If the release lookup yields nothing, the bare id still tries master.""" + calls = _record(client, monkeypatch, { + '/releases/777': None, + '/masters/777': {'id': 777, 'title': 'Master Only', 'main_release': 1}, + }) + result = client.get_album('777', include_tracks=False) + assert calls == ['/releases/777', '/masters/777'] + assert result['name'] == 'Master Only' + + +def test_fetch_and_cache_release_id_never_hits_masters(client, monkeypatch): + calls = _record(client, monkeypatch, { + '/releases/249504': {'id': 249504, 'title': 'The Real Album'}, + '/masters/249504': {'id': 249504, 'title': 'A DIFFERENT Album'}, + }) + data = client._fetch_and_cache_album('r249504') + assert calls == ['/releases/249504'] + assert data['title'] == 'The Real Album' diff --git a/tests/test_torrent_cleanup_orphan.py b/tests/test_torrent_cleanup_orphan.py new file mode 100644 index 00000000..f362c68c --- /dev/null +++ b/tests/test_torrent_cleanup_orphan.py @@ -0,0 +1,67 @@ +"""noldevin: a dead torrent (metaDL stuck / errored / timed out) was left ORPHANED +in qbit — cleared from SoulSync but still active in the client, then re-grabbed as +a duplicate. The monitor's terminal exits now call _cleanup_torrent, which removes +(abandon) or pauses it in the client.""" + +from __future__ import annotations + +from unittest.mock import patch + +import pytest + +from core.download_plugins.torrent import TorrentDownloadPlugin + + +class _FakeAdapter: + def __init__(self): + self.removed = [] + self.paused = [] + + async def remove(self, h, delete_files=False): + self.removed.append((h, delete_files)) + + async def pause(self, h): + self.paused.append(h) + + +@pytest.fixture +def plugin(): + with patch('core.download_plugins.torrent.ProwlarrClient'): + yield TorrentDownloadPlugin() + + +def test_abandon_removes_torrent_and_deletes_files(plugin): + fake = _FakeAdapter() + with patch('core.download_plugins.torrent.get_active_torrent_adapter', return_value=fake): + plugin._cleanup_torrent('abc123', 'abandon') + assert fake.removed == [('abc123', True)] # removed + partial data deleted + assert fake.paused == [] + + +def test_pause_action_pauses_not_removes(plugin): + fake = _FakeAdapter() + with patch('core.download_plugins.torrent.get_active_torrent_adapter', return_value=fake): + plugin._cleanup_torrent('abc123', 'pause') + assert fake.paused == ['abc123'] + assert fake.removed == [] + + +def test_no_hash_is_a_noop(plugin): + fake = _FakeAdapter() + with patch('core.download_plugins.torrent.get_active_torrent_adapter', return_value=fake): + plugin._cleanup_torrent('', 'abandon') + plugin._cleanup_torrent(None, 'abandon') + assert fake.removed == [] and fake.paused == [] + + +def test_no_adapter_is_a_noop(plugin): + with patch('core.download_plugins.torrent.get_active_torrent_adapter', return_value=None): + plugin._cleanup_torrent('abc123', 'abandon') # must not raise + + +def test_client_error_is_swallowed(plugin): + class _Boom: + async def remove(self, h, delete_files=False): + raise RuntimeError("qbit down") + with patch('core.download_plugins.torrent.get_active_torrent_adapter', return_value=_Boom()): + plugin._cleanup_torrent('abc123', 'abandon') # best-effort: logged, not raised diff --git a/tests/test_torrent_stall.py b/tests/test_torrent_stall.py index 2f7f5510..efebb365 100644 --- a/tests/test_torrent_stall.py +++ b/tests/test_torrent_stall.py @@ -102,3 +102,38 @@ def test_get_stall_action(raw, expected): with patch.object(ts, "config_manager", _cfg({"download_source.torrent_stall_action": raw})): assert get_stall_action() == expected + + +# ── metadata-phase noise (noldevin #2: metaDL stuck 11h, stall never fired) ── +def test_metadata_phase_byte_noise_does_not_reset_clock(): + """A magnet stuck 'downloading metadata' reports size==0 and a downloaded + counter that still ticks up from DHT/peer overhead. Those bumps must NOT + reset the stall clock, or the dead magnet never times out (the bug).""" + t = StallTracker(timeout_seconds=600) + assert t.is_stalled(0, "downloading", now=0, size=0) is False # first + assert t.is_stalled(16384, "downloading", now=120, size=0) is False # noise bump + assert t.is_stalled(32768, "downloading", now=300, size=0) is False # more noise + assert t.is_stalled(40000, "downloading", now=480, size=0) is False # still under + # Despite the byte counter climbing the whole time, no metadata was obtained + # → stalled at the timeout. + assert t.is_stalled(50000, "downloading", now=600, size=0) is True + + +def test_obtaining_metadata_resets_the_clock(): + """size 0 -> >0 means metadata arrived — real progress, reset the clock.""" + t = StallTracker(timeout_seconds=600) + assert t.is_stalled(0, "downloading", now=0, size=0) is False + assert t.is_stalled(0, "downloading", now=500, size=0) is False # accruing + # metadata arrives at t=550 (size now known) → progress → clock resets + assert t.is_stalled(0, "downloading", now=550, size=10_000_000) is False + assert t.is_stalled(0, "downloading", now=900, size=10_000_000) is False # <600 since reset + assert t.is_stalled(0, "downloading", now=1150, size=10_000_000) is True # 600 later, no bytes + + +def test_real_download_progress_tracked_after_metadata(): + """Once metadata is in, byte progress resets the clock as normal.""" + t = StallTracker(timeout_seconds=600) + assert t.is_stalled(0, "downloading", now=0, size=10_000_000) is False + assert t.is_stalled(500000, "downloading", now=400, size=10_000_000) is False # progress + assert t.is_stalled(500000, "downloading", now=900, size=10_000_000) is False # <600 since + assert t.is_stalled(500000, "downloading", now=1001, size=10_000_000) is True # stalled diff --git a/tests/test_verification_admin_gate.py b/tests/test_verification_admin_gate.py new file mode 100644 index 00000000..b927c458 --- /dev/null +++ b/tests/test_verification_admin_gate.py @@ -0,0 +1,45 @@ +"""#845 follow-up: the mutating verification-review endpoints (delete removes a +file from disk; approve flips verification state) must be admin-only, matching the +Phase 3 destructive-endpoint gating. The read/playback ones stay open.""" + +from __future__ import annotations + +import os +import tempfile + +import pytest + +_TMP = tempfile.mkdtemp(prefix='soulsync-testdb-vgate-') +os.environ['DATABASE_PATH'] = os.path.join(_TMP, 'v.db') +os.environ['SOULSYNC_TEST_DB_READY'] = '1' + +web_server = pytest.importorskip('web_server') + + +@pytest.fixture +def client(): + return web_server.app.test_client() + + +def _as_nonadmin(client): + pid = web_server.get_database().create_profile(name=f'u_{os.urandom(4).hex()}') + with client.session_transaction() as s: + s['profile_id'] = pid + return pid + + +def test_delete_is_admin_only(client): + _as_nonadmin(client) + assert client.post('/api/verification/1/delete').status_code == 403 + + +def test_approve_is_admin_only(client): + _as_nonadmin(client) + assert client.post('/api/verification/1/approve').status_code == 403 + + +def test_admin_not_blocked(client): + # default session resolves to admin (profile 1) — must pass the gate (a + # missing history id yields 404, NOT 403). + assert client.post('/api/verification/999999/delete').status_code != 403 + assert client.post('/api/verification/999999/approve').status_code != 403 diff --git a/tests/test_verification_status_backfill.py b/tests/test_verification_status_backfill.py new file mode 100644 index 00000000..0abf92dd --- /dev/null +++ b/tests/test_verification_status_backfill.py @@ -0,0 +1,53 @@ +"""#845: the migration backfills verification_status for library_history rows +written before the column existed, from the acoustid_result they recorded. Must +map correctly, never overwrite an existing status, leave no-acoustid rows alone, +and be idempotent (it runs on every fresh DB init). + +The backfill runs inside _initialize_database, which is guarded to run once per +path per process — so the tests clear that guard to re-trigger it on the seeded +rows (exercising the real production code path, not a copy of the SQL).""" + +from __future__ import annotations + +import database.music_database as mdb +from database.music_database import MusicDatabase + + +def _reinit(path): + """Force _initialize_database (incl. the backfill) to run again on `path`.""" + mdb._database_initialized_paths.clear() + return MusicDatabase(path) + + +def _statuses(db): + with db._get_connection() as conn: + return {r['title']: r['verification_status'] + for r in conn.execute("SELECT title, verification_status FROM library_history")} + + +def test_backfill_maps_and_preserves(tmp_path): + p = str(tmp_path / "m.db") + db = MusicDatabase(p) + db.add_library_history_entry('import', 'A', acoustid_result='pass') + db.add_library_history_entry('import', 'B', acoustid_result='skip') + db.add_library_history_entry('import', 'C', acoustid_result='fail') + db.add_library_history_entry('import', 'D', acoustid_result='pass', + verification_status='human_verified') # pre-set, must NOT change + db.add_library_history_entry('import', 'E') # no acoustid → stays NULL + + _reinit(p) # run the backfill migration over the seeded rows + + s = _statuses(db) + assert s['A'] == 'verified' + assert s['B'] == 'unverified' + assert s['C'] == 'force_imported' + assert s['D'] == 'human_verified' # existing status preserved (NULL-only) + assert s['E'] is None # no acoustid_result → untouched + + +def test_backfill_is_idempotent(tmp_path): + p = str(tmp_path / "m2.db") + db = MusicDatabase(p) + db.add_library_history_entry('import', 'A', acoustid_result='pass') + _reinit(p); _reinit(p) # run the migration two more times + assert _statuses(db)['A'] == 'verified' diff --git a/tests/test_verification_tag_roundtrip.py b/tests/test_verification_tag_roundtrip.py new file mode 100644 index 00000000..974dd7b0 --- /dev/null +++ b/tests/test_verification_tag_roundtrip.py @@ -0,0 +1,50 @@ +"""SOULSYNC_VERIFICATION file tag: write + read back (travels with the file, +survives DB resets; the AcoustID scan reads it to refresh the DB column).""" + +import shutil +import subprocess + +import pytest + +from core.tag_writer import read_file_tags, write_verification_status + + +pytestmark = pytest.mark.skipif( + shutil.which('ffmpeg') is None, reason='ffmpeg required to build test audio' +) + + +def _make_flac(path): + subprocess.run( + ['ffmpeg', '-loglevel', 'error', '-y', '-f', 'lavfi', + '-i', 'anullsrc=r=44100:cl=mono', '-t', '0.1', str(path)], + check=True, + ) + + +def test_flac_verification_tag_roundtrip(tmp_path): + f = tmp_path / 'x.flac' + _make_flac(f) + assert write_verification_status(str(f), 'force_imported') is True + tags = read_file_tags(str(f)) + assert tags.get('verification_status') == 'force_imported' + + +def test_overwrite_existing_status(tmp_path): + f = tmp_path / 'y.flac' + _make_flac(f) + write_verification_status(str(f), 'unverified') + write_verification_status(str(f), 'verified') + assert read_file_tags(str(f)).get('verification_status') == 'verified' + + +def test_missing_file_returns_false_not_raises(tmp_path): + assert write_verification_status(str(tmp_path / 'nope.flac'), 'verified') is False + + +def test_db_migration_adds_verification_status_column(tmp_path): + from database.music_database import MusicDatabase + db = MusicDatabase(str(tmp_path / 't.db')) + with db._get_connection() as conn: + cols = [r[1] for r in conn.execute('PRAGMA table_info(tracks)').fetchall()] + assert 'verification_status' in cols diff --git a/tests/test_ws_connect_gate.py b/tests/test_ws_connect_gate.py new file mode 100644 index 00000000..d861410d --- /dev/null +++ b/tests/test_ws_connect_gate.py @@ -0,0 +1,84 @@ +"""#852: the socketio handshake bypasses the HTTP launch-PIN/login gate +(before_request doesn't run for it). The connect handler must enforce the same +check, or removing the overlay + opening a socket streams live data unauthenticated.""" + +from __future__ import annotations + +import os +import tempfile + +import pytest + +from core.security.ws_gate import is_ws_connection_blocked as blocked + + +# ── pure gate logic ──────────────────────────────────────────────────────── +def _b(**kw): + base = dict(require_login=False, login_authenticated=False, + require_pin=False, pin_verified=False, proxy_authed=False) + base.update(kw) + return blocked(**base) + + +def test_nothing_on_allows(): + assert _b() is False + + +def test_login_on_unauth_blocks(): + assert _b(require_login=True) is True + + +def test_login_on_authed_allows(): + assert _b(require_login=True, login_authenticated=True) is False + + +def test_pin_on_unverified_blocks(): + assert _b(require_pin=True) is True + + +def test_pin_on_verified_allows(): + assert _b(require_pin=True, pin_verified=True) is False + + +def test_pin_on_proxy_authed_allows(): + assert _b(require_pin=True, proxy_authed=True) is False + + +def test_login_takes_precedence_over_pin(): + # login on + unauth -> blocked even if the PIN was verified + assert _b(require_login=True, require_pin=True, pin_verified=True, proxy_authed=True) is True + + +# ── integration: real socketio connect via the gate ──────────────────────── +_TMP = tempfile.mkdtemp(prefix='soulsync-testdb-wsgate-') +os.environ['DATABASE_PATH'] = os.path.join(_TMP, 'w.db') +os.environ['SOULSYNC_TEST_DB_READY'] = '1' +web_server = pytest.importorskip('web_server') + + +def _pin_on(monkeypatch): + real = web_server.config_manager.get + monkeypatch.setattr(web_server.config_manager, 'get', + lambda k, d=None: True if k == 'security.require_pin_on_launch' else real(k, d)) + + +def test_socket_rejected_when_gate_on_and_unauthenticated(monkeypatch): + _pin_on(monkeypatch) + flask_client = web_server.app.test_client() # no launch_pin_verified + sio = web_server.socketio.test_client(web_server.app, flask_test_client=flask_client) + assert sio.is_connected() is False # the #852 hole, now closed + + +def test_socket_allowed_when_gate_off(): + flask_client = web_server.app.test_client() + sio = web_server.socketio.test_client(web_server.app, flask_test_client=flask_client) + assert sio.is_connected() is True + + +def test_socket_allowed_when_pin_verified(monkeypatch): + _pin_on(monkeypatch) + flask_client = web_server.app.test_client() + with flask_client.session_transaction() as s: + s['launch_pin_verified'] = True + sio = web_server.socketio.test_client(web_server.app, flask_test_client=flask_client) + assert sio.is_connected() is True diff --git a/tests/test_youtube_sync_idempotent_endpoints.py b/tests/test_youtube_sync_idempotent_endpoints.py new file mode 100644 index 00000000..509585e3 --- /dev/null +++ b/tests/test_youtube_sync_idempotent_endpoints.py @@ -0,0 +1,39 @@ +"""#702: cancel/reset/delete of a mirrored-playlist sync whose in-memory state is +gone (restart/eviction) must return success, not 404 'YouTube playlist not found' +— otherwise the playlist is permanently wedged.""" + +from __future__ import annotations + +import os +import tempfile + +import pytest + +_TMP = tempfile.mkdtemp(prefix='soulsync-testdb-ytsync-') +os.environ['DATABASE_PATH'] = os.path.join(_TMP, 'y.db') +os.environ['SOULSYNC_TEST_DB_READY'] = '1' + +web_server = pytest.importorskip('web_server') + + +@pytest.fixture +def client(): + return web_server.app.test_client() + + +_GONE = 'state_wiped_by_restart_hash' + + +def test_cancel_missing_state_is_success(client): + r = client.post(f'/api/youtube/sync/cancel/{_GONE}') + assert r.status_code == 200 and r.get_json().get('success') is True + + +def test_reset_missing_state_is_success(client): + r = client.post(f'/api/youtube/reset/{_GONE}') + assert r.status_code == 200 and r.get_json().get('success') is True + + +def test_delete_missing_state_is_success(client): + r = client.delete(f'/api/youtube/delete/{_GONE}') + assert r.status_code == 200 and r.get_json().get('success') is True diff --git a/web_server.py b/web_server.py index 32173ebe..200cea1a 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.0" +_SOULSYNC_BASE_VERSION = "2.7.1" def _build_version_string(): """Append short commit hash to version when available (e.g. 2.35+abc1234).""" @@ -7578,6 +7578,385 @@ def stream_quarantine_item(entry_id): return jsonify({"error": str(e)}), 500 +def _get_library_history_row(history_id): + """Fetch one full library_history row as a dict (or None).""" + conn = get_database()._get_connection() + cursor = conn.cursor() + cursor.execute("SELECT * FROM library_history WHERE id = ?", (history_id,)) + row = cursor.fetchone() + return dict(row) if row else None + + +def _resolve_history_audio_path(row): + """Resolve a library_history row to a playable on-disk file. + + The recorded path can go stale: Docker↔host prefix differences, or the + media server / organizer renaming files with exotic titles (e.g. + 凸】♀】♂】←Titan) after import. Fallback chain: + 1. the recorded path as-is, + 2. `_resolve_library_file_path` (transfer/download/library prefix swap), + 3. the tracks table — the media-server mirror knows the CURRENT path for + this title+artist even after a rename — resolved the same way. + """ + def _lookup_titled_paths(title): + # tracks-table mirror: paths for this title (knows the CURRENT path + # after a media-server rename). [] on any DB error → resolver returns None. + try: + conn = get_database()._get_connection() + cursor = conn.cursor() + cursor.execute( + "SELECT file_path FROM tracks WHERE file_path IS NOT NULL AND LOWER(title) = LOWER(?)", + (title,)) + return [r[0] for r in cursor.fetchall() if r[0]] + except Exception as e: + logger.debug(f"[Verification] tracks-table path fallback failed: {e}") + return [] + + from core.matching.history_paths import resolve_history_audio_path + return resolve_history_audio_path( + row, + exists=os.path.exists, + resolve_library_path=_resolve_library_file_path, + lookup_titled_paths=_lookup_titled_paths, + ) + + +@app.route('/api/verification//stream', methods=['GET']) +def stream_verification_item(history_id): + """Stream a completed download for the verification review queue (listen + before approving). Path comes ONLY from the history row — no client paths.""" + try: + row = _get_library_history_row(history_id) + if not row: + return jsonify({"error": "History entry not found"}), 404 + file_path = _resolve_history_audio_path(row) + if not file_path: + return jsonify({"error": "File not found on disk"}), 404 + # _AUDIO_MIME_TYPES keys keep the dot ('.flac') — don't strip it, or + # everything falls back to audio/mpeg and FLAC playback breaks. + ext = os.path.splitext(file_path)[1].lower() + mimetype = _AUDIO_MIME_TYPES.get(ext, 'audio/mpeg') + return _serve_audio_file_with_range(file_path, mimetype_override=mimetype) + except Exception as e: + logger.error(f"[Verification] Error streaming history {history_id}: {e}") + return jsonify({"error": str(e)}), 500 + + +@app.route('/api/verification//entry', methods=['GET']) +def get_verification_entry(history_id): + """Full library_history row for one review-queue item — feeds the Audit + Trail modal when opened from the Downloads page (where the history-page + entry cache is not populated).""" + try: + row = _get_library_history_row(history_id) + if not row: + return jsonify({"success": False, "error": "History entry not found"}), 404 + return jsonify({"success": True, "entry": row}) + except Exception as e: + logger.error(f"[Verification] Entry fetch failed for {history_id}: {e}") + return jsonify({"success": False, "error": str(e)}), 500 + + +@app.route('/api/verification/config', methods=['GET']) +def get_verification_config(): + """Whether AcoustID/download-verification is enabled — if not, the review + queue collapses to quarantine-only in the UI.""" + try: + enabled = bool(config_manager.get('acoustid.enabled', False)) + return jsonify({"success": True, "acoustid_enabled": enabled}) + except Exception as e: + return jsonify({"success": True, "acoustid_enabled": True, "error": str(e)}) + + +def _audio_file_duration_ms(path): + """Best-effort duration of an on-disk audio file (0 when unreadable). + mutagen detects the format from content, so this also works for + quarantined files whose extension was swapped to `.quarantined`.""" + try: + import mutagen + mf = mutagen.File(path) + if mf and mf.info and getattr(mf.info, 'length', 0): + return int(mf.info.length * 1000) + except Exception: # noqa: S110 — duration probe is best-effort; fall through to 0 + pass + return 0 + + +def _set_review_play_session(file_path, title, artist, album, mimetype=None): + """Point THIS listener's media-player session at a local file — same + mechanism as /api/library/play, so the bottom player UI drives playback + (seek/stop/volume) instead of an invisible Audio element.""" + sess = _current_stream_state() + with sess.lock: + sess.update({ + "status": "ready", + "progress": 100, + "track_info": { + "title": title or os.path.basename(file_path), + "artist": artist or 'Unknown Artist', + "album": album or '', + }, + "file_path": file_path, + "stream_url": None, + "error_message": None, + "is_library": True, + # Content-Type hint for /stream/audio — needed for quarantined + # files whose on-disk extension is `.quarantined`. Keyed to the + # exact path so a stale hint can never leak onto another file. + "mimetype_override": mimetype, + "mimetype_override_path": file_path if mimetype else None, + }) + + +@app.route('/api/verification//play', methods=['POST']) +def play_verification_item(history_id): + """Load the downloaded file into the media player (review queue ▶).""" + try: + row = _get_library_history_row(history_id) + if not row: + return jsonify({"success": False, "error": "History entry not found"}), 404 + file_path = _resolve_history_audio_path(row) + if not file_path: + return jsonify({"success": False, "error": "File not found on disk"}), 404 + _set_review_play_session( + file_path, row.get('title'), row.get('artist_name'), row.get('album_name')) + return jsonify({"success": True, "track_info": { + "title": row.get('title') or os.path.basename(file_path), + "artist": row.get('artist_name') or '', + "album": row.get('album_name') or '', + "image_url": row.get('thumb_url') or None, + }}) + except Exception as e: + logger.error(f"[Verification] Play failed for {history_id}: {e}") + return jsonify({"success": False, "error": str(e)}), 500 + + +@app.route('/api/verification//compare-stream', methods=['POST']) +def compare_stream_verification_item(history_id): + """Find the expected track on Soulseek/streaming sources for an A/B + comparison — the SAME pipeline as the /search page play button, but fed + server-side so the local file's duration guides candidate ranking (a + missing duration lets e.g. 10-hour YouTube loops win and time out).""" + try: + row = _get_library_history_row(history_id) + if not row: + return jsonify({"success": False, "error": "History entry not found"}), 404 + local = _resolve_history_audio_path(row) + duration_ms = _audio_file_duration_ms(local) if local else 0 + result = _search_stream.stream_search_track( + track_name=row.get('title') or '', + artist_name=row.get('artist_name') or '', + album_name=row.get('album_name') or '', + duration_ms=duration_ms, + config_manager=config_manager, + download_orchestrator=download_orchestrator, + matching_engine=matching_engine, + run_async=run_async, + ) + if result is None: + return jsonify({"success": False, + "error": "No suitable stream candidate found"}), 404 + result['title'] = row.get('title') or '' + result['artist'] = row.get('artist_name') or '' + result['album'] = row.get('album_name') or '' + result['image_url'] = row.get('thumb_url') or None + return jsonify({"success": True, "result": result}) + except Exception as e: + logger.error(f"[Verification] Compare-stream failed for {history_id}: {e}") + return jsonify({"success": False, "error": str(e)}), 500 + + +def _get_quarantine_entry(entry_id): + from core.imports.quarantine import list_quarantine_entries + for entry in list_quarantine_entries(_get_quarantine_dir()): + if entry.get('id') == entry_id: + return entry + return None + + +@app.route('/api/quarantine//play', methods=['POST']) +def play_quarantine_item(entry_id): + """Load a quarantined file into the media player (review queue ▶).""" + try: + from core.imports.quarantine import get_quarantine_entry_stream_info + info = get_quarantine_entry_stream_info(_get_quarantine_dir(), entry_id) + if info is None: + return jsonify({"success": False, "error": "Quarantined file not found"}), 404 + file_path, extension = info + entry = _get_quarantine_entry(entry_id) or {} + title = entry.get('expected_track') or entry.get('original_filename') or os.path.basename(file_path) + _set_review_play_session( + file_path, f"{title} (quarantined)", entry.get('expected_artist'), '', + mimetype=_AUDIO_MIME_TYPES.get(extension, 'audio/mpeg')) + return jsonify({"success": True, "track_info": { + "title": f"{title} (quarantined)", + "artist": entry.get('expected_artist') or '', + "album": '', + }}) + except Exception as e: + logger.error(f"[Quarantine] Play failed for {entry_id}: {e}") + return jsonify({"success": False, "error": str(e)}), 500 + + +@app.route('/api/quarantine//compare-stream', methods=['POST']) +def compare_stream_quarantine_item(entry_id): + """Stream-search the EXPECTED track for a quarantined file (A/B compare), + using the quarantined file's duration to guide candidate ranking.""" + try: + from core.imports.quarantine import get_quarantine_entry_stream_info + entry = _get_quarantine_entry(entry_id) + if not entry: + return jsonify({"success": False, "error": "Quarantine entry not found"}), 404 + track_name = entry.get('expected_track') or '' + artist_name = entry.get('expected_artist') or '' + if not track_name: + return jsonify({"success": False, + "error": "Entry has no expected-track metadata"}), 400 + info = get_quarantine_entry_stream_info(_get_quarantine_dir(), entry_id) + duration_ms = _audio_file_duration_ms(info[0]) if info else 0 + result = _search_stream.stream_search_track( + track_name=track_name, + artist_name=artist_name, + album_name='', + duration_ms=duration_ms, + config_manager=config_manager, + download_orchestrator=download_orchestrator, + matching_engine=matching_engine, + run_async=run_async, + ) + if result is None: + return jsonify({"success": False, + "error": "No suitable stream candidate found"}), 404 + result['title'] = track_name + result['artist'] = artist_name + return jsonify({"success": True, "result": result}) + except Exception as e: + logger.error(f"[Quarantine] Compare-stream failed for {entry_id}: {e}") + return jsonify({"success": False, "error": str(e)}), 500 + + +@app.route('/api/quarantine//entry', methods=['GET']) +def get_quarantine_audit_entry(entry_id): + """Synthesize a library_history-shaped entry from a quarantine sidecar so + the review queue opens the SAME Audit Trail modal for quarantined files + (they were never imported, so no history row exists). ``id`` is None on + purpose — the modal fetches tags/lyrics through ``_file_tags_url``.""" + try: + from core.imports.quarantine import ( + get_quarantine_entry_context, get_quarantine_entry_stream_info) + entry = _get_quarantine_entry(entry_id) + if not entry: + return jsonify({"success": False, "error": "Quarantine entry not found"}), 404 + ctx = get_quarantine_entry_context(_get_quarantine_dir(), entry_id) + info = get_quarantine_entry_stream_info(_get_quarantine_dir(), entry_id) + osr = ctx.get('original_search_result') if isinstance(ctx.get('original_search_result'), dict) else {} + username = (osr.get('username') or '') if isinstance(osr, dict) else '' + streaming = ('tidal', 'youtube', 'qobuz', 'hifi', 'deezer_dl', + 'lidarr', 'soundcloud', 'amazon') + ti = ctx.get('track_info') if isinstance(ctx.get('track_info'), dict) else {} + album_raw = ti.get('album', '') + album_name = album_raw.get('name', '') if isinstance(album_raw, dict) else str(album_raw or '') + synthetic = { + 'id': None, + 'event_type': 'download', + 'title': entry.get('expected_track') or entry.get('original_filename') or '', + 'artist_name': entry.get('expected_artist') or '', + 'album_name': album_name, + 'created_at': entry.get('timestamp') or '', + 'thumb_url': entry.get('thumb_url') or '', + 'file_path': info[0] if info else '', + 'quality': ctx.get('_audio_quality') or '', + 'download_source': username if username in streaming else ('soulseek' if username else ''), + 'source_filename': entry.get('source_filename') or '', + 'source_artist': (osr.get('artist') or '') if isinstance(osr, dict) else '', + 'source_track_title': (osr.get('title') or osr.get('name') or '') if isinstance(osr, dict) else '', + 'acoustid_result': 'fail' if entry.get('trigger') == 'acoustid' else None, + 'verification_status': None, + '_quarantined': True, + '_quarantine_reason': entry.get('reason') or '', + '_file_tags_url': f"/api/quarantine/{entry_id}/file-tags", + } + return jsonify({"success": True, "entry": synthetic}) + except Exception as e: + logger.error(f"[Quarantine] Audit entry failed for {entry_id}: {e}") + return jsonify({"success": False, "error": str(e)}), 500 + + +@app.route('/api/quarantine//file-tags', methods=['GET']) +def get_quarantine_file_tags(entry_id): + """Embedded tags of a quarantined file — feeds the Audit modal's Tags / + Lyrics tabs. mutagen detects the format from content, so the swapped + `.quarantined` extension is no obstacle.""" + try: + from core.imports.quarantine import get_quarantine_entry_stream_info + from core.library.file_tags import read_embedded_tags + info = get_quarantine_entry_stream_info(_get_quarantine_dir(), entry_id) + if info is None: + return jsonify({'success': False, 'error': 'Quarantined file not found'}), 404 + result = read_embedded_tags(info[0]) + return jsonify({'success': True, **result}) + except Exception as e: + logger.error(f"[Quarantine] File-tags failed for {entry_id}: {e}") + return jsonify({'success': False, 'error': str(e)}), 500 + + +@app.route('/api/verification//approve', methods=['POST']) +@admin_only +def approve_verification_item(history_id): + """User confirmed the file IS the right track: set human_verified on the + history row, the file tag, and (best-effort) the tracks row. The AcoustID + scanner skips human-verified files entirely. Admin-only: mutates shared + library/verification state.""" + try: + from core.matching.verification_status import HUMAN_VERIFIED + from core.tag_writer import write_verification_status + db = get_database() + row = _get_library_history_row(history_id) + if not row: + return jsonify({"success": False, "error": "History entry not found"}), 404 + file_path = row.get('file_path') or '' + on_disk = _resolve_history_audio_path(row) + with db._get_connection() as conn: + conn.execute( + "UPDATE library_history SET verification_status = ? WHERE id = ?", + (HUMAN_VERIFIED, history_id)) + # The tracks row may carry either the recorded or the resolved path. + for p in {p for p in (file_path, on_disk) if p}: + conn.execute( + "UPDATE tracks SET verification_status = ? WHERE file_path = ?", + (HUMAN_VERIFIED, p)) + conn.commit() + tag_written = bool(on_disk) and write_verification_status(on_disk, HUMAN_VERIFIED) + return jsonify({"success": True, "tag_written": tag_written}) + except Exception as e: + logger.error(f"[Verification] Approve failed for {history_id}: {e}") + return jsonify({"success": False, "error": str(e)}), 500 + + +@app.route('/api/verification//delete', methods=['POST']) +@admin_only +def delete_verification_item(history_id): + """User decided the file is wrong: delete it from disk and drop the + history row (the media-server mirror cleans the tracks row on next scan). + Admin-only: it removes a file from disk + the library.""" + try: + db = get_database() + row = _get_library_history_row(history_id) + if not row: + return jsonify({"success": False, "error": "History entry not found"}), 404 + on_disk = _resolve_history_audio_path(row) + file_deleted = False + if on_disk and os.path.exists(on_disk): + os.remove(on_disk) + file_deleted = True + logger.info(f"[Verification] Deleted rejected file: {on_disk}") + db.delete_library_history_rows([history_id]) + return jsonify({"success": True, "file_deleted": file_deleted}) + except Exception as e: + logger.error(f"[Verification] Delete failed for {history_id}: {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 @@ -12801,6 +13180,11 @@ def stream_audio(): stream_url = sess.get("stream_url") if not file_path and not stream_url: return jsonify({"error": "No audio file ready for streaming"}), 404 + # Content-Type hint set by the quarantine player (on-disk extension + # is `.quarantined`). Path-keyed so it can't leak onto other files. + mimetype_override = (sess.get("mimetype_override") + if sess.get("mimetype_override_path") == file_path + else None) # Library track played via the media server's stream API (#809). if stream_url: @@ -12808,7 +13192,7 @@ def stream_audio(): return _proxy_stream_url_with_range(stream_url) logger.info(f"Serving audio file: {os.path.basename(file_path)}") - return _serve_audio_file_with_range(file_path) + return _serve_audio_file_with_range(file_path, mimetype_override=mimetype_override) except Exception as e: logger.error(f"Error serving audio file: {e}") return jsonify({"error": str(e)}), 500 @@ -24278,7 +24662,10 @@ def reset_youtube_playlist(url_hash): """Reset YouTube playlist to fresh phase (clear discovery/sync data)""" try: if url_hash not in youtube_playlist_states: - return jsonify({"error": "YouTube playlist not found"}), 404 + # Idempotent: live state gone (restart/eviction) — already "fresh". + # 404 here permanently wedges a mirrored playlist whose state vanished + # (#702); treat a reset of nothing as a success so the UI recovers. + return jsonify({"success": True, "message": "Playlist already reset"}) state = youtube_playlist_states[url_hash] @@ -24310,7 +24697,9 @@ def delete_youtube_playlist(url_hash): """Remove YouTube playlist from backend storage entirely""" try: if url_hash not in youtube_playlist_states: - return jsonify({"error": "YouTube playlist not found"}), 404 + # Idempotent: already gone (restart/eviction) — deleting nothing is a + # success, not a 404 that wedges the UI (#702). + return jsonify({"success": True, "message": "Playlist already removed"}) state = youtube_playlist_states[url_hash] @@ -29638,11 +30027,15 @@ def _fetch_liked_albums(profile_id: int): if discogs_cl.is_authenticated(): logger.info("[Your Albums] Fetching collection from Discogs...") releases = discogs_cl.get_user_collection() + from core.discogs_client import _tag_discogs_album_id for r in releases: database.upsert_liked_album( album_name=r['album_name'], artist_name=r['artist_name'], source_service='discogs', - source_id=str(r['release_id']), source_id_type='discogs', + # Collection items are always releases — store the ID tagged + # ('r') to match search/discography (#848), so every stored + # Discogs album ID is uniform and re-fetches route correctly. + source_id=_tag_discogs_album_id(r['release_id'], 'release'), source_id_type='discogs', image_url=r.get('image_url'), release_date=r.get('release_date', ''), total_tracks=r.get('total_tracks', 0), profile_id=profile_id ) @@ -36107,8 +36500,39 @@ def _emit_download_status_loop(): # --- Socket.IO event handlers --- +def _ws_connection_blocked(): + """#852: mirror the HTTP launch-PIN / login gate for the socketio handshake + (before_request doesn't run for it). Fails OPEN on a config-read error, same + as the HTTP gate, so a broken config never wedges every client.""" + try: + require_login = _require_login_enabled() + require_pin = bool(config_manager.get('security.require_pin_on_launch', False)) if config_manager else False + if not require_login and not require_pin: + return False + proxy_header = (config_manager.get('security.auth_proxy_header', '') or '') if config_manager else '' + from core.security.auth_proxy import trusted_proxy_user + proxy_authed = bool(trusted_proxy_user(request.headers.get, proxy_header)) + from core.security.ws_gate import is_ws_connection_blocked + return is_ws_connection_blocked( + require_login=require_login, + login_authenticated=bool(session.get('login_authenticated', False)), + require_pin=require_pin, + pin_verified=bool(session.get('launch_pin_verified', False)), + proxy_authed=proxy_authed, + ) + except Exception as e: + logger.error(f"[WS gate] check failed, allowing (matches HTTP gate fail-open): {e}") + return False + + @socketio.on('connect') def handle_connect(): + # #852: the launch-PIN / login gate is a before_request hook, which does NOT + # run for the socketio handshake. Without this, removing the client overlay + + # opening a socket streams live data (downloads/logs/dashboard) unauthenticated. + if _ws_connection_blocked(): + logger.warning("Rejected WebSocket connection — access gate active, session not verified (#852)") + return False logger.info("WebSocket client connected") @socketio.on('disconnect') diff --git a/webui/index.html b/webui/index.html index 256a8ad3..58a9e3a7 100644 --- a/webui/index.html +++ b/webui/index.html @@ -2427,6 +2427,7 @@ +
@@ -5565,8 +5566,9 @@ + - Separator between multiple artists in the ARTIST tag + Separator between multiple artists in the ARTIST tag (e.g. & matches MusicBrainz/Picard style)
+ +
+ +
+
+ On by default (legacy behaviour). When enabled, files staged as + Artist/Album/… (or Artist/Albums/Album/…) take the top + folder as the album artist — handy for mixtapes/compilations whose embedded tags + carry DJ names. Turn this off if you drop a mixed pile of songs + under one container folder: otherwise every file's artist gets overwritten with + that folder's name (the cause of the "soulsync" mass-mislabel). With it off, the + metadata-identified artist is always kept. +
diff --git a/webui/static/downloads.js b/webui/static/downloads.js index 3faa2cc6..26303e3b 100644 --- a/webui/static/downloads.js +++ b/webui/static/downloads.js @@ -3676,10 +3676,36 @@ function processModalStatusUpdate(playlistId, data) { } else { switch (task.status) { case 'pending': statusText = '⏸️ Pending'; break; - case 'searching': statusText = '🔍 Searching...'; break; - case 'downloading': statusText = `⏬ Downloading... ${Math.round(task.progress || 0)}%`; break; + case 'searching': + statusText = '🔍 Searching...'; + // Quarantine-retry engine: show which attempt we're on + // ("retry 2/5") while it walks the next-best candidates. + if (task.retry_info) statusText += ` 🔁 retry ${task.retry_info}`; + break; + case 'downloading': + statusText = `⏬ Downloading... ${Math.round(task.progress || 0)}%`; + if (task.retry_info) statusText += ` 🔁 retry ${task.retry_info}`; + break; case 'post_processing': statusText = '⌛ Processing...'; break; - case 'completed': statusText = '✅ Completed'; completedCount++; break; + case 'completed': { + statusText = '✅ Completed'; + // Verification badge — how this file passed verification: + // verified = clean AcoustID pass; unverified = couldn't be + // hard-confirmed (cross-script/ambiguous/no fingerprint match); + // force_imported = accepted as best candidate after the retry + // budget was exhausted (version-mismatch fallback). + if (task.verification_status === 'force_imported') { + statusText += ' ⚑'; + } else if (task.verification_status === 'unverified') { + statusText += ' ⚠'; + } else if (task.verification_status === 'verified') { + statusText += ' ✔'; + } else if (task.verification_status === 'human_verified') { + statusText += ' 🛡✔'; + } + completedCount++; + break; + } case 'not_found': statusText = '🔇 Not Found'; notFoundCount++; break; case 'failed': { // Distinguish quarantine outcomes from generic @@ -3712,7 +3738,14 @@ function processModalStatusUpdate(playlistId, data) { delete statusEl.dataset.quarantineReason; delete statusEl.dataset.quarantineTrack; delete statusEl.dataset.detailOpen; - statusEl.textContent = statusText; + // statusText is static markup only; the verif-badge span is the + // one case that needs HTML. Everything else stays textContent + // (XSS-safe default). + if (statusText.includes('class="verif-badge')) { + statusEl.innerHTML = statusText; + } else { + statusEl.textContent = statusText; + } // Visual-only hooks: the cell carries its state for the badge // styling, the row glows while a track is actively working. statusEl.dataset.state = isQuarantinedTask ? 'quarantined' diff --git a/webui/static/enrichment.js b/webui/static/enrichment.js index a63d93e5..3ec070e0 100644 --- a/webui/static/enrichment.js +++ b/webui/static/enrichment.js @@ -3583,6 +3583,9 @@ function _promptAcoustidAction() { + @@ -3591,7 +3594,7 @@ function _promptAcoustidAction() {
- Retag = update metadata to match actual audio • Re-download = add correct track to wishlist & delete wrong file • Delete = remove file and DB entry + Retag = update metadata in place • Relocate = retag + move to Staging so it's re-imported into the correct artist/album • Re-download = add correct track to wishlist & delete wrong file • Delete = remove file and DB entry
+ + + + + `; +} + +async function verifPlay(hid) { + // Plays the LOCAL file through the global media player (same machinery as + // library playback) — full player UI with seek/stop instead of an + // invisible Audio element that re-renders wiped. + const dl = _adlData.find(d => verifHistoryId(d) === String(hid)); + try { + if (typeof setTrackInfo === 'function') { + setTrackInfo({ + title: (dl && dl.title) || 'Review track', + artist: (dl && dl.artist) || '', + album: (dl && dl.album) || '', + is_library: true, + image_url: (dl && dl.artwork) || null, + }); + } + if (typeof showLoadingAnimation === 'function') showLoadingAnimation(); + const r = await fetch(`/api/verification/${hid}/play`, { method: 'POST' }); + const d = await r.json(); + if (!d.success) throw new Error(d.error || 'Playback failed'); + await startAudioPlayback(); + } catch (e) { + if (typeof hideLoadingAnimation === 'function') hideLoadingAnimation(); + showToast && showToast('Playback failed: ' + e.message, 'error'); + } +} + +async function verifCompare(hid, btn) { + // Same pipeline as the /search page play button — run server-side so the + // local file's duration guides candidate ranking (avoids e.g. 10-hour + // YouTube loops winning the match and timing out). + if (btn) { btn.disabled = true; btn.textContent = '…'; } + showToast && showToast('Searching stream for comparison…', 'info'); + try { + const res = await fetch(`/api/verification/${hid}/compare-stream`, { method: 'POST' }); + const data = await res.json(); + if (data.success && data.result && typeof startStream === 'function') { + await startStream(data.result); + } else { + showToast && showToast(data.error || 'No stream candidate found for comparison', 'error'); + } + } catch (e) { + showToast && showToast('Stream failed: ' + e.message, 'error'); + } + if (btn) { btn.disabled = false; btn.textContent = '⇆'; } +} + +async function verifAudit(hid) { + try { + const r = await fetch(`/api/verification/${hid}/entry`); + const d = await r.json(); + if (d.success && d.entry && typeof openDownloadAuditModal === 'function') { + openDownloadAuditModal(d.entry); + } else { + showToast && showToast(d.error || 'Audit data not available', 'error'); + } + } catch (e) { + showToast && showToast('Audit load failed', 'error'); + } +} + +async function verifApprove(hid, btn) { + if (btn) btn.disabled = true; + try { + const r = await fetch(`/api/verification/${hid}/approve`, { method: 'POST' }); + const d = await r.json(); + if (d.success) { showToast && showToast('Marked as human-verified 🛡✔', 'success'); _adlFetch(); } + else showToast && showToast(d.error || 'Approve failed', 'error'); + } catch (e) { showToast && showToast('Approve failed', 'error'); } + if (btn) btn.disabled = false; +} + +async function verifDelete(hid, btn) { + if (!await showConfirmDialog({ + title: 'Delete Unverified File', + message: 'This permanently deletes the downloaded file from disk and removes the review entry. Cannot be undone.', + confirmText: 'Delete', + cancelText: 'Cancel', + destructive: true, + })) return; + if (btn) btn.disabled = true; + try { + const r = await fetch(`/api/verification/${hid}/delete`, { method: 'POST' }); + const d = await r.json(); + if (d.success) { showToast && showToast('File deleted', 'success'); _adlFetch(); } + else showToast && showToast(d.error || 'Delete failed', 'error'); + } catch (e) { showToast && showToast('Delete failed', 'error'); } + if (btn) btn.disabled = false; +} +// ---- Quarantine sub-view inside the ⚠ filter ---- +// The review queue covers BOTH kinds of suspect files: imported-but- +// unconfirmed (unverified/force_imported) and not-imported-at-all +// (quarantined). One place to listen, compare, approve or delete. +let _verifSubView = 'unverified'; +let _verifQuarEntries = []; +let _verifQuarLoaded = false; +let _verifQuarLoading = false; +// Expanded 🔍 detail panels, keyed by quarantine entry id — survives the +// polling re-render (which rebuilds the rows every few seconds). +const _verifQuarOpenDetails = new Set(); +// null = not fetched yet (assume enabled). Without an AcoustID API key +// nothing ever gets a verification status, so the review queue collapses +// to quarantine-only. +let _verifAcoustidEnabled = null; +let _verifConfigLoading = false; + +async function _verifLoadConfig() { + if (_verifAcoustidEnabled !== null || _verifConfigLoading) return; + _verifConfigLoading = true; + try { + const r = await fetch('/api/verification/config'); + const d = await r.json(); + _verifAcoustidEnabled = !!(d && d.acoustid_enabled); + } catch (e) { _verifAcoustidEnabled = true; } + _verifConfigLoading = false; + if (_verifAcoustidEnabled === false) { + _verifSubView = 'quarantine'; + const pill = document.querySelector('.adl-pill[data-filter="unverified"]'); + if (pill) { + pill.textContent = '🛡 Quarantine'; + pill.title = 'Files that failed import checks and were NOT imported. (AcoustID is not configured, so there is no unverified review queue.)'; + } + if (_adlFilter === 'unverified') { _verifLoadQuarantine(true); _adlRender(); } + } +} + +function verifSetSubView(v) { + if (_verifAcoustidEnabled === false) v = 'quarantine'; + _verifSubView = v === 'quarantine' ? 'quarantine' : 'unverified'; + if (_verifSubView === 'quarantine') _verifLoadQuarantine(true); + _adlRender(); +} + +async function _verifLoadQuarantine(force) { + if (_verifQuarLoading || (_verifQuarLoaded && !force)) return; + _verifQuarLoading = true; + try { + const r = await fetch('/api/quarantine/list'); + const d = await r.json(); + _verifQuarEntries = (d.success && Array.isArray(d.entries)) ? d.entries : []; + } catch (e) { _verifQuarEntries = []; } + _verifQuarLoaded = true; + _verifQuarLoading = false; + if (_adlFilter === 'unverified') _adlRender(); +} + +const _VERIF_QUAR_TRIGGERS = { + integrity: ['DURATION / INTEGRITY', 'verif-rb-int'], + acoustid: ['ACOUSTID MISMATCH', 'verif-rb-force'], + bit_depth: ['BIT DEPTH FILTER', 'verif-rb-int'], +}; + +function _verifQuarRows() { + if (!_verifQuarLoaded) return '
Loading quarantine…
'; + if (!_verifQuarEntries.length) return ''; + let html = ''; + _verifQuarEntries.forEach((q, idx) => { + const title = _adlEsc(q.expected_track || q.original_filename || q.filename || 'Unknown file'); + const meta = [_adlEsc(q.expected_artist || ''), _adlEsc(q.original_filename || '')].filter(Boolean).join(' — '); + const [trigLabel, trigClass] = _VERIF_QUAR_TRIGGERS[q.trigger] || ['QUARANTINED', 'verif-rb-unv']; + const timeAgo = _verifTimeAgo(q.timestamp); + const approveBtn = q.has_full_context + ? `` + : ``; + const details = [ + q.reason ? `
Reason: ${_adlEsc(q.reason)}
` : '', + q.source_username ? `
Source uploader: ${_adlEsc(q.source_username)}
` : '', + q.source_filename ? `
Original Soulseek file: ${_adlEsc(q.source_filename)}
` : '', + q.timestamp ? `
Quarantined: ${_adlEsc(q.timestamp)}
` : '', + ].filter(Boolean).join(''); + const detailsOpen = _verifQuarOpenDetails.has(q.id); + const artHtml = q.thumb_url + ? `` + : '
'; + html += `
+ ${artHtml} +
+
${title}
+ ${meta ? `
${meta}
` : ''} +
${details || 'No further details in the sidecar.'}
+
+
+ ${trigLabel} + ${timeAgo ? `${timeAgo}` : ''} + + + + ${approveBtn} + +
+
`; + }); + return html; +} + +function verifQuarInspect(idx) { + // Open-state lives in a Set keyed by entry id (not the DOM) — the polling + // re-render rebuilds the rows every few seconds and would collapse a + // DOM-only toggle right after the click. + const q = _verifQuarEntries[idx]; + if (!q) return; + if (_verifQuarOpenDetails.has(q.id)) _verifQuarOpenDetails.delete(q.id); + else _verifQuarOpenDetails.add(q.id); + const el = document.getElementById(`verif-quar-details-${idx}`); + if (el) el.style.display = _verifQuarOpenDetails.has(q.id) ? '' : 'none'; +} + +async function verifQuarAudit(idx) { + // Same Audit Trail modal as the unverified rows — the backend synthesizes + // a history-shaped entry from the quarantine sidecar (these files were + // never imported, so no real history row exists). + const q = _verifQuarEntries[idx]; if (!q) return; + try { + const r = await fetch(`/api/quarantine/${encodeURIComponent(q.id)}/entry`); + const d = await r.json(); + if (d.success && d.entry && typeof openDownloadAuditModal === 'function') { + openDownloadAuditModal(d.entry); + } else { + showToast && showToast(d.error || 'Audit data not available', 'error'); + } + } catch (e) { + showToast && showToast('Audit load failed', 'error'); + } +} + +async function verifQuarPlay(idx) { + const q = _verifQuarEntries[idx]; if (!q) return; + try { + if (typeof setTrackInfo === 'function') { + setTrackInfo({ + title: `${q.expected_track || q.original_filename || 'Quarantined file'} (quarantined)`, + artist: q.expected_artist || '', + album: '', + is_library: true, + }); + } + if (typeof showLoadingAnimation === 'function') showLoadingAnimation(); + const r = await fetch(`/api/quarantine/${encodeURIComponent(q.id)}/play`, { method: 'POST' }); + const d = await r.json(); + if (!d.success) throw new Error(d.error || 'Playback failed'); + await startAudioPlayback(); + } catch (e) { + if (typeof hideLoadingAnimation === 'function') hideLoadingAnimation(); + showToast && showToast('Playback failed: ' + e.message, 'error'); + } +} + +async function verifQuarCompare(idx, btn) { + const q = _verifQuarEntries[idx]; if (!q) return; + if (btn) { btn.disabled = true; btn.textContent = '…'; } + showToast && showToast(`Searching stream for "${q.expected_track || ''}"…`, 'info'); + try { + const res = await fetch(`/api/quarantine/${encodeURIComponent(q.id)}/compare-stream`, { method: 'POST' }); + const data = await res.json(); + if (data.success && data.result && typeof startStream === 'function') { + await startStream(data.result); + } else { + showToast && showToast(data.error || 'No stream candidate found for comparison', 'error'); + } + } catch (e) { + showToast && showToast('Stream failed: ' + e.message, 'error'); + } + if (btn) { btn.disabled = false; btn.textContent = '⇆'; } +} + +async function verifQuarApprove(idx, btn) { + const q = _verifQuarEntries[idx]; if (!q) return; + if (btn) btn.disabled = true; + try { + const r = await fetch(`/api/quarantine/${encodeURIComponent(q.id)}/approve`, { method: 'POST' }); + const d = await r.json(); + if (d.success) { + showToast && showToast('Approved — re-importing, will be marked human-verified 🛡✔', 'success'); + _verifLoadQuarantine(true); + } else { + showToast && showToast(d.error || 'Approve failed', 'error'); + } + } catch (e) { showToast && showToast('Approve failed', 'error'); } + if (btn) btn.disabled = false; +} + +async function verifQuarRecover(idx, btn) { + const q = _verifQuarEntries[idx]; if (!q) return; + if (btn) btn.disabled = true; + try { + const r = await fetch(`/api/quarantine/${encodeURIComponent(q.id)}/recover`, { method: 'POST' }); + const d = await r.json(); + if (d.success) { + showToast && showToast('Moved to Staging — finish via the Import page', 'success'); + _verifLoadQuarantine(true); + } else { + showToast && showToast(d.error || 'Recover failed', 'error'); + } + } catch (e) { showToast && showToast('Recover failed', 'error'); } + if (btn) btn.disabled = false; +} + +async function verifQuarDelete(idx, btn) { + const q = _verifQuarEntries[idx]; if (!q) return; + if (!await showConfirmDialog({ + title: 'Delete Quarantined File', + message: 'This permanently removes the file and its metadata sidecar. Cannot be undone.', + confirmText: 'Delete', + cancelText: 'Cancel', + destructive: true, + })) return; + if (btn) btn.disabled = true; + try { + const r = await fetch(`/api/quarantine/${encodeURIComponent(q.id)}`, { method: 'DELETE' }); + const d = await r.json(); + if (d.success) { showToast && showToast('Quarantined file deleted', 'success'); _verifLoadQuarantine(true); } + else showToast && showToast(d.error || 'Delete failed', 'error'); + } catch (e) { showToast && showToast('Delete failed', 'error'); } + if (btn) btn.disabled = false; +} + +function _verifUnverifiedIds() { + const done = ['completed', 'skipped', 'already_owned']; + return _adlData + .filter(d => done.includes(d.status) && + (d.verification_status === 'unverified' || d.verification_status === 'force_imported')) + .map(d => verifHistoryId(d)) + .filter(Boolean); +} + +async function verifApproveAll(btn) { + const ids = _verifUnverifiedIds(); + if (!ids.length) return; + if (!await showConfirmDialog({ + title: 'Approve Unverified Files', + message: `Mark all ${ids.length} unverified entries as human-verified? The AcoustID scanner will skip them from now on.`, + confirmText: 'Approve All', + cancelText: 'Cancel', + })) return; + if (btn) btn.disabled = true; + let ok = 0; + for (const id of ids) { + try { + const r = await fetch(`/api/verification/${id}/approve`, { method: 'POST' }); + const d = await r.json(); + if (d.success) ok++; + } catch (e) {} + } + showToast && showToast(`Approved ${ok}/${ids.length} entries 🛡✔`, ok === ids.length ? 'success' : 'error'); + if (btn) btn.disabled = false; + _adlFetch(); +} + +async function verifDeleteAll(btn) { + const ids = _verifUnverifiedIds(); + if (!ids.length) return; + if (!await showConfirmDialog({ + title: 'Delete Unverified Files', + message: `This permanently deletes all ${ids.length} unverified files from disk and removes their review entries. Cannot be undone.`, + confirmText: 'Delete All', + cancelText: 'Cancel', + destructive: true, + })) return; + if (btn) btn.disabled = true; + let ok = 0; + for (const id of ids) { + try { + const r = await fetch(`/api/verification/${id}/delete`, { method: 'POST' }); + const d = await r.json(); + if (d.success) ok++; + } catch (e) {} + } + showToast && showToast(`Deleted ${ok}/${ids.length} files`, ok === ids.length ? 'success' : 'error'); + if (btn) btn.disabled = false; + _adlFetch(); +} + +async function verifQuarApproveAll(btn) { + const entries = _verifQuarEntries.filter(q => q.has_full_context); + if (!entries.length) { + showToast && showToast('No one-click-approvable entries (legacy sidecars need Recover)', 'info'); + return; + } + if (!await showConfirmDialog({ + title: 'Approve Quarantined Files', + message: `Approve and re-import all ${entries.length} quarantined files? They will be imported into the library marked human-verified.`, + confirmText: 'Approve & Import All', + cancelText: 'Cancel', + })) return; + if (btn) btn.disabled = true; + let ok = 0; + for (const q of entries) { + try { + const r = await fetch(`/api/quarantine/${encodeURIComponent(q.id)}/approve`, { method: 'POST' }); + const d = await r.json(); + if (d.success) ok++; + } catch (e) {} + // Each approve spawns a server-side re-import thread — stagger them. + await new Promise(res => setTimeout(res, 500)); + } + showToast && showToast(`Approved ${ok}/${entries.length} — re-importing as human-verified`, ok === entries.length ? 'success' : 'error'); + if (btn) btn.disabled = false; + _verifLoadQuarantine(true); +} + +async function verifQuarClearAll(btn) { + if (!_verifQuarEntries.length) return; + if (!await showConfirmDialog({ + title: 'Clear Quarantine', + message: `This permanently deletes all ${_verifQuarEntries.length} quarantined files and their metadata sidecars. Cannot be undone.`, + confirmText: 'Delete All', + cancelText: 'Cancel', + destructive: true, + })) return; + if (btn) btn.disabled = true; + try { + const r = await fetch('/api/quarantine/clear', { method: 'POST' }); + const d = await r.json(); + if (d.success) showToast && showToast('Quarantine cleared', 'success'); + else showToast && showToast(d.error || 'Clear failed', 'error'); + } catch (e) { showToast && showToast('Clear failed', 'error'); } + if (btn) btn.disabled = false; + _verifLoadQuarantine(true); +} + +window.verifPlay = verifPlay; +window.verifApprove = verifApprove; +window.verifDelete = verifDelete; +window.verifCompare = verifCompare; +window.verifAudit = verifAudit; +window.verifSetSubView = verifSetSubView; +window.verifApproveAll = verifApproveAll; +window.verifDeleteAll = verifDeleteAll; +window.verifQuarPlay = verifQuarPlay; +window.verifQuarCompare = verifQuarCompare; +window.verifQuarInspect = verifQuarInspect; +window.verifQuarAudit = verifQuarAudit; +window.verifQuarApprove = verifQuarApprove; +window.verifQuarRecover = verifQuarRecover; +window.verifQuarDelete = verifQuarDelete; +window.verifQuarApproveAll = verifQuarApproveAll; +window.verifQuarClearAll = verifQuarClearAll; + function _adlRender() { const list = document.getElementById('adl-list'); const empty = document.getElementById('adl-empty'); @@ -2370,6 +2871,10 @@ function _adlRender() { if (_adlFilter === 'active') filtered = filtered.filter(d => activeStatuses.includes(d.status)); else if (_adlFilter === 'queued') filtered = filtered.filter(d => queuedStatuses.includes(d.status)); else if (_adlFilter === 'completed') filtered = filtered.filter(d => completedStatuses.includes(d.status)); + else if (_adlFilter === 'unverified') filtered = filtered.filter(d => + completedStatuses.includes(d.status) && + (d.verification_status === 'unverified' || d.verification_status === 'force_imported')); + // (review banner injected below when this filter is active) else if (_adlFilter === 'failed') filtered = filtered.filter(d => failedStatuses.includes(d.status)); const completedN = _adlData.filter(d => @@ -2419,6 +2924,48 @@ function _adlRender() { existingBanner.style.display = 'none'; } + // Review queue sub-view toggle: unverified imports ⇄ quarantined files. + let verifBanner = document.getElementById('verif-subview-banner'); + if (_adlFilter === 'unverified') { + _verifLoadConfig(); // no-op once fetched + if (!verifBanner) { + verifBanner = document.createElement('div'); + verifBanner.id = 'verif-subview-banner'; + verifBanner.className = 'adl-batch-filter-banner'; + list.parentNode.insertBefore(verifBanner, list); + } + const quarCount = _verifQuarLoaded ? ` (${_verifQuarEntries.length})` : ''; + const bulkBtns = _verifSubView === 'quarantine' + ? ` + ` + : ` + `; + // Without an AcoustID key nothing ever gets a verification status — + // hide the pointless Unverified pill and show quarantine only. + const unvPill = _verifAcoustidEnabled === false + ? '' + : ``; + verifBanner.innerHTML = ` + ${unvPill} + + + ${bulkBtns}`; + verifBanner.style.display = ''; + if (!_verifQuarLoaded) _verifLoadQuarantine(false); // count for the pill + } else if (verifBanner) { + verifBanner.style.display = 'none'; + } + + if (_adlFilter === 'unverified' && _verifSubView === 'quarantine') { + const qhtml = _verifQuarRows(); + const qEmptyEl = document.getElementById('adl-empty'); + const qEmptyHtml = qEmptyEl ? qEmptyEl.outerHTML : ''; + list.innerHTML = qEmptyHtml + qhtml; + const qNewEmpty = document.getElementById('adl-empty'); + if (qNewEmpty) qNewEmpty.style.display = qhtml ? 'none' : ''; + return; + } + if (filtered.length === 0) { if (empty) empty.style.display = ''; // Clear any existing rows but keep the empty message @@ -2455,6 +3002,7 @@ function _adlRender() { for (const dl of section.items) { const statusClass = _adlStatusClass(dl.status); const statusLabel = _adlStatusLabel(dl.status); + // (verification badge appended next to the label via _adlVerifBadge) const title = _adlEsc(dl.title || 'Unknown Track'); const artist = _adlEsc(dl.artist || ''); const album = _adlEsc(dl.album || ''); @@ -2494,8 +3042,9 @@ function _adlRender() {
- ${statusLabel} + ${statusLabel}${_adlVerifBadge(dl)}${dl.retry_info && (statusClass === 'active' || statusClass === 'queued') ? ` 🔁 ${_adlEsc(String(dl.retry_info))}` : ''}
+ ${_adlReviewActions(dl)} ${cancelBtnHtml} `; } @@ -3126,4 +3675,3 @@ window.adlCancelRow = adlCancelRow; window.adlCancelAll = adlCancelAll; window.adlToggleBatchHistory = adlToggleBatchHistory; window.adlToggleBatchPanel = adlToggleBatchPanel; - diff --git a/webui/static/settings.js b/webui/static/settings.js index 4868a305..6e699dd5 100644 --- a/webui/static/settings.js +++ b/webui/static/settings.js @@ -1297,6 +1297,8 @@ async function loadSettingsData() { // Populate Import settings document.getElementById('import-replace-lower-quality').checked = settings.import?.replace_lower_quality === true; + const _folderArtistEl = document.getElementById('import-folder-artist-override'); + if (_folderArtistEl) _folderArtistEl.checked = settings.import?.folder_artist_override === true; // Populate M3U Export settings document.getElementById('m3u-export-enabled').checked = settings.m3u_export?.enabled === true; @@ -3128,6 +3130,7 @@ async function saveSettings(quiet = false) { }, import: { replace_lower_quality: document.getElementById('import-replace-lower-quality').checked, + folder_artist_override: document.getElementById('import-folder-artist-override')?.checked === true, staging_path: document.getElementById('staging-path').value || './Staging' }, lossy_copy: { diff --git a/webui/static/shared-helpers.js b/webui/static/shared-helpers.js index f5a0b958..620fec26 100644 --- a/webui/static/shared-helpers.js +++ b/webui/static/shared-helpers.js @@ -281,7 +281,12 @@ function createSearchController({ if (resp.ok) { const status = await resp.json(); const ms = status && status.metadata_source; - const src = (ms && typeof ms === 'object') ? ms.source : ms; + let src = (ms && typeof ms === 'object') ? ms.source : ms; + // "Spotify (no auth)" reports as 'spotify_free' for display, but the + // search picker only has a 'spotify' icon (it's the same searchable + // source). Map it so no-auth auto-selects Spotify instead of leaving + // the picker empty (no SOURCE_ORDER icon matches 'spotify_free'). + if (src === 'spotify_free') src = 'spotify'; if (src && SOURCE_LABELS[src]) state.activeSource = src; } } catch (_) { /* best-effort */ } diff --git a/webui/static/style.css b/webui/static/style.css index 2be4fc5a..754d32c7 100644 --- a/webui/static/style.css +++ b/webui/static/style.css @@ -10864,6 +10864,26 @@ body.helper-mode-active #dashboard-activity-feed:hover { border-color: rgba(239, 83, 80, 0.3); color: #ef5350; } +.download-audit-hero-pill-verified { + background: rgba(74, 222, 128, 0.12); + border-color: rgba(74, 222, 128, 0.3); + color: #4ade80; +} +.download-audit-hero-pill-human_verified { + background: rgba(52, 152, 219, 0.12); + border-color: rgba(52, 152, 219, 0.35); + color: #3498db; +} +.download-audit-hero-pill-force_imported { + background: rgba(230, 126, 34, 0.12); + border-color: rgba(230, 126, 34, 0.35); + color: #e67e22; +} +.download-audit-hero-pill-unverified { + background: rgba(241, 196, 15, 0.12); + border-color: rgba(241, 196, 15, 0.35); + color: #f1c40f; +} /* Tab bar */ .download-audit-tabs { @@ -67893,6 +67913,30 @@ body.em-scroll-lock { overflow: hidden; } } .ma-token-input:focus { outline: none; border-color: var(--ma-brand); } +/* Verification badge on completed downloads (verified / unverified / force-imported) */ +.verif-badge { display: inline-block; margin-left: 4px; font-size: 11px; cursor: help; border-radius: 999px; padding: 0 5px; line-height: 16px; } +.verif-badge.verif-ok { color: #2ecc71; background: rgba(46,204,113,0.12); } +.verif-badge.verif-unverified { color: #f1c40f; background: rgba(241,196,15,0.14); } +.verif-badge.verif-force { color: #e67e22; background: rgba(230,126,34,0.16); } +.adl-retry-info { margin-left: 6px; font-size: 11px; color: #e67e22; cursor: help; } +.verif-badge.verif-human { color: #3498db; background: rgba(52,152,219,0.14); } +.verif-actions { display: inline-flex; gap: 6px; margin-left: 10px; align-items: center; } +.verif-act { border: 1px solid rgba(255,255,255,0.18); background: rgba(255,255,255,0.06); color: rgba(255,255,255,0.85); border-radius: 6px; padding: 2px 8px; font-size: 12px; cursor: pointer; line-height: 18px; } +.verif-act:hover { background: rgba(255,255,255,0.14); } +.verif-act-ok:hover { background: rgba(46,204,113,0.25); border-color: rgba(46,204,113,0.5); } +.verif-act-del:hover { background: rgba(231,76,60,0.25); border-color: rgba(231,76,60,0.5); } +.verif-act-play.playing { background: rgba(var(--accent-rgb),0.3); } +/* Review-queue row annotations (library-history-quarantine style) */ +.verif-reason-badge { display: inline-block; font-size: 9.5px; font-weight: 600; letter-spacing: 0.04em; border: 1px solid; border-radius: 999px; padding: 1px 7px; line-height: 14px; white-space: nowrap; cursor: help; } +.verif-rb-unv { color: #f1c40f; border-color: rgba(241,196,15,0.5); } +.verif-rb-force { color: #ef5350; border-color: rgba(239,83,80,0.5); } +.verif-rb-int { color: #facc15; border-color: rgba(250,204,21,0.5); } +.verif-time { font-size: 11px; color: rgba(255,255,255,0.45); white-space: nowrap; } +.verif-quar-details { margin-top: 6px; font-size: 11.5px; color: rgba(255,255,255,0.6); line-height: 1.5; word-break: break-all; } +.verif-detail-label { color: rgba(255,255,255,0.4); margin-right: 4px; } +.verif-banner-spacer { flex: 1; } +.verif-bulk-danger { border-color: rgba(248,113,113,0.4) !important; color: #f87171 !important; } + /* ── Security settings: grouped sub-sections + dependency visuals ── */ .security-subgroup { border: 1px solid rgba(255, 255, 255, 0.07); diff --git a/webui/static/sw.js b/webui/static/sw.js index bce4f22b..4f9619f0 100644 --- a/webui/static/sw.js +++ b/webui/static/sw.js @@ -108,28 +108,82 @@ self.addEventListener('fetch', (event) => { }); +// ── image fetch throttle ─────────────────────────────────────────────── +// A discography page fires 70+ cover-art requests at once. Routed through the +// SW one-for-one, that burst overruns the browser's per-host connection pool +// (~6); the overflow fetches reject, and a cache-first strategy that maps a +// rejection to Response.error() turns each into a hard NS_ERROR_INTERCEPTION_ +// FAILED — a permanently broken, *uncached* image for that load. (It only +// "fixes itself" on reload because the images that did win the race are then +// served from cache, shrinking the burst.) We cap how many image fetches the +// SW runs at once so the rest queue instead of failing. +const MAX_CONCURRENT_IMAGE_FETCHES = 6; +let _activeImageFetches = 0; +const _imageFetchQueue = []; + +function _acquireImageSlot() { + if (_activeImageFetches < MAX_CONCURRENT_IMAGE_FETCHES) { + _activeImageFetches++; + return Promise.resolve(); + } + return new Promise((resolve) => _imageFetchQueue.push(resolve)); +} + +function _releaseImageSlot() { + const next = _imageFetchQueue.shift(); + // Hand the slot straight to the next waiter (count stays put); only + // decrement when nobody is queued. + if (next) next(); + else _activeImageFetches--; +} + +async function _fetchWithRetry(request, retries = 1, backoffMs = 200) { + try { + return await fetch(request); + } catch (err) { + if (retries <= 0) throw err; + // Most failures here are transient connection-cap rejections that + // clear as the burst drains — one short retry recovers the bulk. + await new Promise((resolve) => setTimeout(resolve, backoffMs)); + return _fetchWithRetry(request, retries - 1, backoffMs); + } +} + + // ── strategies ─────────────────────────────────────────────────────── async function _cacheFirst(request, cacheName) { + // Cache lookup first — a hit must never consume a fetch slot. If the cache + // layer itself is unavailable, fall through to a throttled network fetch. + let cache = null; try { - const cache = await caches.open(cacheName); + cache = await caches.open(cacheName); const hit = await cache.match(request); if (hit) return hit; + } catch (err) { + cache = null; + } - const response = await fetch(request); + await _acquireImageSlot(); + try { + const response = await _fetchWithRetry(request); // Only cache successful, opaque-OK responses. Don't cache 404s // / 500s — would pin a bad placeholder for the lifetime of the // cache version. - if (response && (response.ok || response.type === 'opaque')) { + if (cache && response && (response.ok || response.type === 'opaque')) { // Clone before .put — body is consumed otherwise. cache.put(request, response.clone()).catch(() => { /* quota / disk full */ }); } return response; } catch (err) { - // Network failure with no cache hit — let the browser surface - // its standard offline / error UI (returning Response.error() - // is equivalent to letting the fetch reject naturally). - return Response.error(); + // Both attempts failed (offline / connection cap / CDN error). Return a + // benign 504 rather than Response.error(): a network-error response + // from a SW surfaces as NS_ERROR_INTERCEPTION_FAILED in Firefox, which + // hard-fails the . A plain 504 just yields a broken image that + // recovers on the next navigation once the cache is warm. + return new Response('', { status: 504, statusText: 'Image fetch failed' }); + } finally { + _releaseImageSlot(); } } diff --git a/webui/static/wishlist-tools.js b/webui/static/wishlist-tools.js index 64230654..e47fd5d6 100644 --- a/webui/static/wishlist-tools.js +++ b/webui/static/wishlist-tools.js @@ -3799,8 +3799,10 @@ function openDownloadAuditModal(entry) { // Live tag read for the Tags tab. Fire-and-forget on open so the // data is ready by the time the user switches to the Tags tab. - if (entry.id != null) { - fetchAndRenderEmbeddedTags(entry.id); + // Synthetic entries (quarantine review queue) have no history id but + // carry their own tags URL in _file_tags_url. + if (entry.id != null || entry._file_tags_url) { + fetchAndRenderEmbeddedTags(entry.id, entry._file_tags_url); } } @@ -3814,11 +3816,9 @@ function switchAuditTab(tabName) { if (body) body.innerHTML = renderDownloadAuditTabPanel(tabName, _downloadAuditEntry); // Re-fire the live fetch when switching to Tags (in case it // raced past first mount before the user got there). - if (tabName === 'tags' && _downloadAuditEntry.id != null) { - fetchAndRenderEmbeddedTags(_downloadAuditEntry.id); - } - if (tabName === 'lyrics' && _downloadAuditEntry.id != null) { - fetchAndRenderEmbeddedTags(_downloadAuditEntry.id); + if ((tabName === 'tags' || tabName === 'lyrics') && + (_downloadAuditEntry.id != null || _downloadAuditEntry._file_tags_url)) { + fetchAndRenderEmbeddedTags(_downloadAuditEntry.id, _downloadAuditEntry._file_tags_url); } } window.switchAuditTab = switchAuditTab; @@ -3856,7 +3856,14 @@ function renderDownloadAuditHero(entry) { const pills = []; if (entry.download_source) pills.push(_auditHeroPill('source', entry.download_source)); if (entry.quality) pills.push(_auditHeroPill('quality', entry.quality)); - if (entry.acoustid_result) pills.push(_auditHeroPill('verify', formatAcoustidLabel(entry.acoustid_result), entry.acoustid_result)); + const _vsLabels = { verified: 'Verified', unverified: 'Unverified', force_imported: 'Force-imported', human_verified: 'Human verified' }; + if (entry.verification_status && _vsLabels[entry.verification_status]) { + pills.push(_auditHeroPill('verify', _vsLabels[entry.verification_status], entry.verification_status)); + } else if (entry._quarantined) { + pills.push(_auditHeroPill('verify', 'Quarantined', 'fail')); + } else if (entry.acoustid_result) { + pills.push(_auditHeroPill('verify', formatAcoustidLabel(entry.acoustid_result), entry.acoustid_result)); + } return ` ${art} @@ -3957,7 +3964,7 @@ function renderEmbeddedTagsSection(entry) { return renderDownloadAuditTagsTab(entry); } -async function fetchAndRenderEmbeddedTags(historyId) { +async function fetchAndRenderEmbeddedTags(historyId, urlOverride) { const tagsSlot = document.getElementById('download-audit-tags-body'); const lyricsSlot = document.getElementById('download-audit-lyrics-body'); if (!tagsSlot && !lyricsSlot) return; @@ -3967,7 +3974,7 @@ async function fetchAndRenderEmbeddedTags(historyId) { if (lyricsSlot) lyricsSlot.innerHTML = html; }; try { - const resp = await fetch(`/api/library/history/${historyId}/file-tags`); + const resp = await fetch(urlOverride || `/api/library/history/${historyId}/file-tags`); if (!resp.ok) { renderError(`Could not read file tags (HTTP ${resp.status}).`); return; } const data = await resp.json(); if (!data.success) { renderError(data.error || 'Could not read file tags.'); return; } @@ -4263,8 +4270,8 @@ function buildDownloadAuditSteps(entry) { { key: 'verify', title: 'Verification', - status: auditStatusFromAcoustid(entry.acoustid_result), - detail: buildAcoustidDetail(entry.acoustid_result), + status: auditVerifyStatus(entry), + detail: buildVerificationDetail(entry), meta: [], }, (() => { @@ -4381,6 +4388,37 @@ function buildSourceMatchMeta(entry) { return out; } +function auditVerifyStatus(entry) { + // The persisted verification status is the final word — it captures + // outcomes the raw acoustid_result can't express (force-imported after + // N retries, human approval via the review queue). + if (entry._quarantined) return 'error'; + if (entry.verification_status === 'human_verified') return 'complete'; + if (entry.verification_status === 'verified') return 'complete'; + if (entry.verification_status === 'force_imported') return 'partial'; + if (entry.verification_status === 'unverified') return 'partial'; + return auditStatusFromAcoustid(entry.acoustid_result); +} + +function buildVerificationDetail(entry) { + if (entry._quarantined) { + return `Quarantined — the file was NOT imported. ${entry._quarantine_reason || ''}`.trim(); + } + if (entry.verification_status === 'force_imported') { + return 'Force-imported: accepted as the best available candidate after the retry budget was exhausted (version-mismatch fallback). The AcoustID check was bypassed for this final re-import — earlier attempts had failed it.'; + } + if (entry.verification_status === 'human_verified') { + return 'Human verified: you confirmed this file via the review queue. The AcoustID scanner skips it.'; + } + if (entry.verification_status === 'unverified') { + return 'Imported but not hard-confirmed: AcoustID could not verify this file (ambiguous / cross-script metadata / no fingerprint match). Review it under Unverified/Quarantine on the Downloads page.'; + } + if (entry.verification_status === 'verified') { + return 'AcoustID fingerprint matched the expected track.'; + } + return buildAcoustidDetail(entry.acoustid_result); +} + function auditStatusFromAcoustid(result) { if (!result) return 'unknown'; if (result === 'pass') return 'complete';