From 32464908006531c04861317fa1b64ff7a7c0b637 Mon Sep 17 00:00:00 2001 From: Broque Thomas <26755000+Nezreka@users.noreply.github.com> Date: Sat, 9 May 2026 09:57:33 -0700 Subject: [PATCH] Auto-import: MBID/ISRC fast paths + duration sanity gate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Brings the auto-import matcher to picard / beets / roon parity by reaching for the existing AcoustID-grade infrastructure (typed Album foundation, integrity check thresholds) and layering id-based exact matches on top of the fuzzy scorer. Picard-tagged libraries now land every track with full confidence on the first pass. Three layered phases in `core/imports/album_matching.match_files_to_tracks`: 1. **MBID exact match** — file has `musicbrainz_trackid` tag, source returns the same id → instant pair, full confidence, no fuzzy scoring. Picard's primary identifier; per-recording. 2. **ISRC exact match** — file has `isrc` tag, source returns the same id → same fast-path, slightly lower priority than mbid (isrc can be shared across remasters). Both ids normalised before compare (uppercase + strip dashes/spaces for isrc, lowercase for mbid). 3. **Duration sanity gate** — files in the fuzzy phase whose audio length differs from the candidate track's duration by more than `DURATION_TOLERANCE_MS` (3s, matching the post-download integrity check) are rejected before scoring runs. Defends against the cross-disc / cross-release / wrong-edit problem the integrity check used to catch only AFTER the file had already been moved + tagged + db-inserted. Tag reader (`_read_file_tags`) extended: - Reads `isrc` (uppercased, strip / / spaces normalisation deferred to matcher) - Reads `musicbrainz_trackid` as `mbid` (lowercased) - Reads `audio.info.length` and converts to `duration_ms` to match the metadata-source convention Metadata-source layer (`_build_album_track_entry`) extended: - Propagates `isrc` from top-level OR `external_ids.isrc` (spotify shape — would otherwise be stripped before reaching the matcher) - Propagates `musicbrainz_id` from top-level OR `external_ids.mbid` / `external_ids.musicbrainz` - Without this layer, fast paths would silently never fire in production even though unit tests pass — pinned by `test_album_track_entry_propagates_isrc_and_mbid_from_source` 18 new tests in `tests/imports/test_album_matching_exact_id.py`: - Direct: `find_exact_id_matches` with mbid, isrc, isrc normalisation, mbid > isrc priority, spotify-shape `external_ids.isrc`, no-id empty result, file-used-at-most-once - Direct: `duration_sanity_ok` within / outside tolerance, missing durations defer - End-to-end via `match_files_to_tracks`: mbid match short-circuits fuzzy scoring, id-matched files excluded from fuzzy phase, duration gate rejects wrong-disc collisions in fuzzy phase, normal matches pass through the gate, missing durations fall through, deezer seconds-vs-ms conversion, full picard-tagged 10-track album via mbid only - Production-shape: `_build_album_track_entry` propagates isrc + mbid from spotify-shape (`external_ids.isrc`) AND itunes-shape (top- level `isrc`) Verification: - 35 album-matching tests pass total (17 helper + 18 fast-path) - 23 multi-disc tests still pass after the extension (additive) - Full suite: 2311 passed (+18 new), 1 pre-existing flaky timing test failure (`test_watchdog_warns_about_stuck_workers` — passes in isolation, fails only in full-suite runs, unrelated to this PR) - Ruff clean For users: - Picard / Beets / Mp3Tag-tagged libraries (anyone who's organised their music) get instant perfect-confidence matches every time. - Soulseek-tagged downloads (which usually carry isrc when sourced via metadata-aware soulseekers) get the fast path too. - Naively-named files with no useful tags fall through to the improved fuzzy + duration-gated path — same correctness as before for the common case, much harder for the matcher to confidently pair the wrong file. - One step closer to standalone-DB feature parity with plex / jellyfin / navidrome scanners. Acoustid fingerprint fallback (for files with NO useful tags AND no MBID/ISRC) is the next followup PR. --- core/auto_import_worker.py | 84 +++- core/imports/album_matching.py | 239 +++++++++- core/metadata/album_tracks.py | 24 + tests/imports/test_album_matching_exact_id.py | 412 ++++++++++++++++++ webui/static/helper.js | 1 + 5 files changed, 723 insertions(+), 37 deletions(-) create mode 100644 tests/imports/test_album_matching_exact_id.py diff --git a/core/auto_import_worker.py b/core/auto_import_worker.py index e71fa381..b8dd1e88 100644 --- a/core/auto_import_worker.py +++ b/core/auto_import_worker.py @@ -55,34 +55,72 @@ def _compute_folder_hash(audio_files: List[str]) -> str: def _read_file_tags(file_path: str) -> Dict[str, Any]: - """Read embedded tags from an audio file. Returns dict with title, artist, album, track_number, disc_number, year.""" - result = {'title': '', 'artist': '', 'album': '', 'track_number': 0, 'disc_number': 1, 'year': ''} + """Read embedded tags from an audio file. + + Returns dict with: title, artist, album, track_number, disc_number, + year, isrc, mbid, duration_ms. + + The exact-identifier fields (``isrc``, ``mbid``) and the audio + duration enable the ID-based fast paths + duration sanity gate in + ``core/imports/album_matching.py``. Tagged files (Picard-tagged + libraries always carry MBID; most metadata sources carry ISRC) get + perfect-match identification without going through fuzzy scoring. + + All exact-identifier fields default to empty string when the tag + isn't present — callers treat empty as "not available, fall back to + fuzzy matching". + """ + result = { + 'title': '', 'artist': '', 'album': '', + 'track_number': 0, 'disc_number': 1, 'year': '', + 'isrc': '', 'mbid': '', 'duration_ms': 0, + } try: from mutagen import File as MutagenFile audio = MutagenFile(file_path, easy=True) - if audio and audio.tags: - tags = audio.tags - result['title'] = (tags.get('title', [''])[0] or '').strip() - # Prefer albumartist for album-level identification (per-track artist - # often includes features like "Kendrick Lamar, Drake" which fragment - # consensus when grouping tracks into an album). Fall back to artist - # for files that lack albumartist. - result['artist'] = (tags.get('albumartist', [''])[0] or tags.get('artist', [''])[0] or '').strip() - result['album'] = (tags.get('album', [''])[0] or '').strip() - # Date/year — try 'date' first, fall back to 'year' - date_str = (tags.get('date', [''])[0] or tags.get('year', [''])[0] or '').strip() - if date_str and len(date_str) >= 4: - result['year'] = date_str[:4] - tn = tags.get('tracknumber', ['0'])[0] + if audio: + # Audio length comes off audio.info, not tags. Mutagen returns + # seconds as a float; convert to int milliseconds to match the + # metadata-source convention (Spotify/Deezer/iTunes all return + # duration_ms). + length_s = getattr(getattr(audio, 'info', None), 'length', 0) or 0 try: - result['track_number'] = int(str(tn).split('/')[0]) - except (ValueError, TypeError): - pass - dn = tags.get('discnumber', ['1'])[0] - try: - result['disc_number'] = int(str(dn).split('/')[0]) - except (ValueError, TypeError): + result['duration_ms'] = int(round(float(length_s) * 1000)) + except (TypeError, ValueError): pass + + if audio.tags: + tags = audio.tags + result['title'] = (tags.get('title', [''])[0] or '').strip() + # Prefer albumartist for album-level identification (per-track + # artist often includes features like "Kendrick Lamar, Drake" + # which fragment consensus when grouping tracks into an album). + # Fall back to artist for files that lack albumartist. + result['artist'] = (tags.get('albumartist', [''])[0] or tags.get('artist', [''])[0] or '').strip() + result['album'] = (tags.get('album', [''])[0] or '').strip() + # Date/year — try 'date' first, fall back to 'year' + date_str = (tags.get('date', [''])[0] or tags.get('year', [''])[0] or '').strip() + if date_str and len(date_str) >= 4: + result['year'] = date_str[:4] + tn = tags.get('tracknumber', ['0'])[0] + try: + result['track_number'] = int(str(tn).split('/')[0]) + except (ValueError, TypeError): + pass + dn = tags.get('discnumber', ['1'])[0] + try: + result['disc_number'] = int(str(dn).split('/')[0]) + except (ValueError, TypeError): + pass + # ISRC — International Standard Recording Code. Per-recording + # unique identifier; metadata sources expose it as `isrc` on + # tracks. Picard / Beets both write this tag from MusicBrainz. + result['isrc'] = (tags.get('isrc', [''])[0] or '').strip().upper() + # MusicBrainz Recording ID — Picard's primary identifier. + # Stored in `musicbrainz_trackid` for ID3, or + # `MUSICBRAINZ_TRACKID` for Vorbis comments. Mutagen's easy + # mode normalizes the key. + result['mbid'] = (tags.get('musicbrainz_trackid', [''])[0] or '').strip().lower() except Exception as e: logger.debug(f"Could not read tags from {os.path.basename(file_path)}: {e}") return result diff --git a/core/imports/album_matching.py b/core/imports/album_matching.py index a63b4a80..db1257d0 100644 --- a/core/imports/album_matching.py +++ b/core/imports/album_matching.py @@ -180,6 +180,170 @@ def score_file_against_track( return score +# --------------------------------------------------------------------------- +# Exact-identifier fast paths +# --------------------------------------------------------------------------- +# Tagged libraries (especially Picard / Beets) carry per-recording IDs +# that uniquely identify the track regardless of title spelling, album +# context, or duration drift. When both the file tag AND the metadata +# source's track entry carry the same identifier, no fuzzy matching is +# needed — exact match wins, full confidence, no further scoring. +# +# Order: MBID first (MusicBrainz Recording ID — primary Picard tag), +# then ISRC (International Standard Recording Code — many sources). +# An ISRC can be shared across remasters / region releases of the same +# recording, so MBID is preferred when both are present. + +EXACT_MATCH_CONFIDENCE = 1.0 + + +def _track_identifier(track: Dict[str, Any], key: str) -> str: + """Pull a normalized identifier off a metadata-source track dict. + + Different sources spell ISRC differently — Spotify exposes it on + ``external_ids.isrc``; iTunes uses ``isrc`` directly when present. + MBID lives at ``external_ids.mbid`` for some sources, top-level + ``musicbrainz_id`` / ``mbid`` for others. + """ + if key == 'isrc': + # ISRC normalization: uppercase, strip dashes/spaces. Picard writes + # tags as "USRC1234567" but some sources return "US-RC-12-34567". + for candidate in ( + track.get('isrc'), + (track.get('external_ids') or {}).get('isrc'), + ): + if candidate: + return str(candidate).upper().replace('-', '').replace(' ', '').strip() + return '' + if key == 'mbid': + for candidate in ( + track.get('musicbrainz_id'), + track.get('mbid'), + (track.get('external_ids') or {}).get('mbid'), + (track.get('external_ids') or {}).get('musicbrainz'), + ): + if candidate: + return str(candidate).lower().strip() + return '' + return '' + + +def _file_identifier(file_tags: Dict[str, Any], key: str) -> str: + """Pull a normalized identifier off the file's tag dict.""" + if key == 'isrc': + raw = file_tags.get('isrc') or '' + return str(raw).upper().replace('-', '').replace(' ', '').strip() + if key == 'mbid': + return str(file_tags.get('mbid') or '').lower().strip() + return '' + + +def find_exact_id_matches( + audio_files: List[str], + file_tags: Dict[str, Dict[str, Any]], + tracks: List[Dict[str, Any]], +) -> Dict[str, Any]: + """Pair files to tracks via exact-identifier match (MBID, then ISRC). + + Returns a dict with ``matches`` (one entry per file/track pair that + matched on a shared identifier) + ``used_files`` (set) + + ``used_track_indices`` (set). Caller is responsible for feeding the + leftovers into the fuzzy-scoring path. + + No similarity computation, no I/O. Pure dict-in/dict-out. + """ + matches: List[Dict[str, Any]] = [] + used_files: Set[str] = set() + used_track_indices: Set[int] = set() + + for id_key in ('mbid', 'isrc'): + # Build {identifier_value: track_index} for this key — single pass + # over tracks, lookup is O(1) per file afterwards. + track_index_by_id: Dict[str, int] = {} + for i, track in enumerate(tracks): + if i in used_track_indices: + continue + tid = _track_identifier(track, id_key) + if tid: + track_index_by_id[tid] = i + + if not track_index_by_id: + continue + + for f in audio_files: + if f in used_files: + continue + fid = _file_identifier(file_tags.get(f, {}), id_key) + if not fid: + continue + track_idx = track_index_by_id.get(fid) + if track_idx is None or track_idx in used_track_indices: + continue + matches.append({ + 'track': tracks[track_idx], + 'file': f, + 'confidence': EXACT_MATCH_CONFIDENCE, + 'match_type': id_key, + }) + used_files.add(f) + used_track_indices.add(track_idx) + + return { + 'matches': matches, + 'used_files': used_files, + 'used_track_indices': used_track_indices, + } + + +# --------------------------------------------------------------------------- +# Duration sanity gate +# --------------------------------------------------------------------------- +# A file whose audio length differs from the candidate track's duration +# by more than this tolerance can't possibly be the right track — +# rejecting cross-disc / cross-release / wrong-edit mismatches before +# they hit the post-download integrity check (which catches the same +# problem AFTER the file has been moved). The integrity check stays as +# a defense-in-depth backstop. +# +# Tolerance picked to match the post-download integrity check +# (`integrity check Duration mismatch ... drift > tolerance 3.0s`). +# Same threshold = same intent, two enforcement points. + +DURATION_TOLERANCE_MS = 3000 # ±3 seconds + + +def duration_sanity_ok(file_duration_ms: int, track_duration_ms: int) -> bool: + """True when the file's audio duration is plausibly the track's + duration, OR when either side has no usable duration info. + + "Either side missing" returns True (don't reject when we can't + confirm) — gates only on cases where BOTH sides have a number we + can compare. Files with no length info (rare — corrupt headers, + streamed-only formats) are deferred to the fuzzy scorer. + """ + if not file_duration_ms or not track_duration_ms: + return True + return abs(int(file_duration_ms) - int(track_duration_ms)) <= DURATION_TOLERANCE_MS + + +def _track_duration_ms(track: Dict[str, Any]) -> int: + """Pull track duration in milliseconds. + + Spotify / iTunes return ``duration_ms``. Deezer's ``duration`` is + in seconds. Heuristic: anything below 30000 (would be 30 seconds in + ms — implausibly short for a real track) is treated as seconds and + converted. Beyond 30000 is already milliseconds. + """ + raw = track.get('duration_ms') or track.get('duration') or 0 + try: + value = int(raw) + except (TypeError, ValueError): + return 0 + if 0 < value < 30000: + return value * 1000 + return value + + def match_files_to_tracks( audio_files: List[str], file_tags: Dict[str, Dict[str, Any]], @@ -191,33 +355,73 @@ def match_files_to_tracks( ) -> Dict[str, Any]: """Match staging files to album tracks. + Algorithm (in order): + + 1. **Exact-identifier fast paths** (``find_exact_id_matches``) — + pair files to tracks via shared MBID, then ISRC. Picard-tagged + libraries land here on the first pass with full confidence, + skipping the fuzzy scorer entirely. Each match carries a + ``'match_type': 'mbid' | 'isrc'`` field for downstream + provenance / debug logging. + + 2. **Quality dedup** on remaining files — keep the highest-quality + file per ``(disc, track)`` position. + + 3. **Fuzzy scoring** on remaining files vs remaining tracks — title + + artist + position + album-tag weighted scoring with a duration + sanity gate (files whose audio length is more than + ``DURATION_TOLERANCE_MS`` from the candidate track are rejected + before scoring, regardless of how good the title agreement + looks). + Returns a dict with: - - ``matches``: list of ``{'track': dict, 'file': str, 'confidence': float}``, - one per track that found a file scoring at or above - ``MATCH_THRESHOLD`` + - ``matches``: list of ``{'track': dict, 'file': str, 'confidence': float}``; + exact-id matches additionally carry ``'match_type'``. - ``unmatched_files``: files left over after every track found its - best (or none) + best (or none). - Each file matches at most one track (best-scoring track that - accepted it wins). Each track matches at most one file (the highest- - scoring still-unused file). - - Pure function — no side effects, no I/O, no metadata client. Easy - to unit-test by feeding tag dicts and track dicts directly. + Each file matches at most one track. Each track matches at most one + file. Pure function — no side effects, no I/O, no metadata client. """ - deduped = dedupe_files_by_position(audio_files, file_tags, quality_rank=quality_rank) - matches: List[Dict[str, Any]] = [] used_files: Set[str] = set() + used_track_indices: Set[int] = set() + + # Phase 1 — exact identifiers (MBID, then ISRC). + exact = find_exact_id_matches(audio_files, file_tags, tracks) + matches.extend(exact['matches']) + used_files.update(exact['used_files']) + used_track_indices.update(exact['used_track_indices']) + + # Phase 2 — quality dedup on remaining files. + remaining_files = [f for f in audio_files if f not in used_files] + deduped = dedupe_files_by_position(remaining_files, file_tags, quality_rank=quality_rank) + + # Phase 3 — fuzzy scoring on remaining tracks. + for i, track in enumerate(tracks): + if i in used_track_indices: + continue + + track_duration = _track_duration_ms(track) - for track in tracks: best_file = None best_score = 0.0 for f in deduped: if f in used_files: continue + tags = file_tags.get(f, {}) + + # Duration sanity gate — reject implausible matches before + # title/artist scoring even runs. Defends against the + # cross-disc / cross-release wrong-edit problem the post- + # download integrity check used to catch only AFTER the + # file had already been moved + tagged + DB-inserted. + file_duration = tags.get('duration_ms', 0) or 0 + if not duration_sanity_ok(file_duration, track_duration): + continue + score = score_file_against_track( f, tags, track, target_album=target_album, @@ -235,9 +439,12 @@ def match_files_to_tracks( 'confidence': round(best_score, 3), }) + # Final unmatched list: every file that didn't get used in any + # phase. Includes quality-dedup losers (lower-quality copies of + # files we already matched) so the caller can see the full picture. return { 'matches': matches, - 'unmatched_files': [f for f in deduped if f not in used_files], + 'unmatched_files': [f for f in audio_files if f not in used_files], } @@ -249,7 +456,11 @@ __all__ = [ 'CROSS_DISC_POSITION_WEIGHT', 'ALBUM_WEIGHT', 'MATCH_THRESHOLD', + 'EXACT_MATCH_CONFIDENCE', + 'DURATION_TOLERANCE_MS', 'dedupe_files_by_position', 'score_file_against_track', + 'find_exact_id_matches', + 'duration_sanity_ok', 'match_files_to_tracks', ] diff --git a/core/metadata/album_tracks.py b/core/metadata/album_tracks.py index 37e728a4..09ddb492 100644 --- a/core/metadata/album_tracks.py +++ b/core/metadata/album_tracks.py @@ -320,6 +320,27 @@ def _build_album_track_entry(track_item: Any, album_info: Dict[str, Any], source if isinstance(explicit_value, str): explicit_value = explicit_value.lower() == 'explicit' + # Per-recording exact identifiers — drive the auto-import matcher's + # fast paths (`core.imports.album_matching.find_exact_id_matches`). + # Spotify/Deezer typically expose ISRC inside `external_ids.isrc`; + # iTunes uses top-level `isrc`. MusicBrainz-aware sources expose MBID + # similarly. Stripping these used to be invisible — until the matcher + # learned to use them, then it became "fast paths never trigger in + # production even though the unit tests pass" — pinned by the + # production-shape test in test_album_matching_exact_id.py. + external_ids = _extract_lookup_value(track_item, 'external_ids', default=None) or {} + isrc = ( + _extract_lookup_value(track_item, 'isrc', default='') or '' + or (external_ids.get('isrc') if isinstance(external_ids, dict) else '') + or '' + ) + mbid = ( + _extract_lookup_value(track_item, 'musicbrainz_id', 'mbid', default='') or '' + or (external_ids.get('mbid') if isinstance(external_ids, dict) else '') + or (external_ids.get('musicbrainz') if isinstance(external_ids, dict) else '') + or '' + ) + return { 'id': _extract_lookup_value(track_item, 'id', 'track_id', 'trackId', default='') or '', 'name': _extract_lookup_value(track_item, 'name', 'track_name', 'trackName', default='Unknown Track') or 'Unknown Track', @@ -330,6 +351,9 @@ def _build_album_track_entry(track_item: Any, album_info: Dict[str, Any], source 'explicit': bool(explicit_value), 'preview_url': _extract_lookup_value(track_item, 'preview_url', 'previewUrl'), 'external_urls': _extract_lookup_value(track_item, 'external_urls', default={}) or {}, + 'external_ids': external_ids if isinstance(external_ids, dict) else {}, + 'isrc': str(isrc) if isrc else '', + 'musicbrainz_id': str(mbid) if mbid else '', 'uri': _extract_lookup_value(track_item, 'uri', default='') or '', 'album': album_info, 'source': source, diff --git a/tests/imports/test_album_matching_exact_id.py b/tests/imports/test_album_matching_exact_id.py new file mode 100644 index 00000000..decaa8e1 --- /dev/null +++ b/tests/imports/test_album_matching_exact_id.py @@ -0,0 +1,412 @@ +"""Tests for the ID-based fast paths + duration sanity gate added on +top of the fuzzy matcher in ``core/imports/album_matching.py``. + +This is the "state-of-the-art" matching layer — bringing the auto- +import worker up to parity with what Picard / Beets / Roon do. + +Algorithm (in order, each test pins one phase): + +1. **MBID exact match** — file has ``MUSICBRAINZ_TRACKID`` tag, metadata + source returns the same id → instant pair, full confidence, skip + fuzzy scoring entirely. +2. **ISRC exact match** — file has ``ISRC`` tag, source returns the + same id → same fast-path, slightly lower priority than MBID + (multiple recordings can share an ISRC across remasters/regions). +3. **Duration sanity gate** — file's audio length must be within + ``DURATION_TOLERANCE_MS`` of the candidate track's duration. + Defends against the cross-disc / cross-release / wrong-edit problem + the post-download integrity check used to catch only AFTER files + were already moved. +4. **Fuzzy fallback** — files with no usable IDs and no duration veto + fall through to the existing weighted scorer. +""" + +from __future__ import annotations + +from difflib import SequenceMatcher + +from core.imports.album_matching import ( + DURATION_TOLERANCE_MS, + EXACT_MATCH_CONFIDENCE, + duration_sanity_ok, + find_exact_id_matches, + match_files_to_tracks, +) + + +def _sim(a, b): + return SequenceMatcher(None, (a or '').lower(), (b or '').lower()).ratio() + + +def _qrank(ext): + ranks = {'.flac': 100, '.alac': 95, '.wav': 80, '.aac': 60, + '.ogg': 50, '.opus': 50, '.m4a': 60, '.mp3': 30} + return ranks.get((ext or '').lower(), 0) + + +def _tags(*, title='', artist='', album='', track=0, disc=1, + isrc='', mbid='', duration_ms=0): + return { + 'title': title, 'artist': artist, 'album': album, + 'track_number': track, 'disc_number': disc, 'year': '', + 'isrc': isrc, 'mbid': mbid, 'duration_ms': duration_ms, + } + + +def _api_track(*, name='', track_number=0, disc_number=1, + isrc='', mbid='', duration_ms=0, external_ids=None): + out = { + 'name': name, + 'track_number': track_number, + 'disc_number': disc_number, + 'duration_ms': duration_ms, + 'artists': [], + } + if isrc: + out['isrc'] = isrc + if mbid: + out['musicbrainz_id'] = mbid + if external_ids: + out['external_ids'] = external_ids + return out + + +# --------------------------------------------------------------------------- +# find_exact_id_matches — direct unit tests +# --------------------------------------------------------------------------- + + +def test_mbid_exact_match_pairs_file_to_track(): + """File with MBID tag matches the track carrying the same MBID, + even when title is completely wrong.""" + files = ['/a/scrambled.flac'] + file_tags = { + '/a/scrambled.flac': _tags( + title='Scrambled Filename', mbid='abc-123-mbid', + ), + } + tracks = [ + _api_track(name='Real Track Name', mbid='abc-123-mbid'), + ] + result = find_exact_id_matches(files, file_tags, tracks) + assert len(result['matches']) == 1 + assert result['matches'][0]['file'] == '/a/scrambled.flac' + assert result['matches'][0]['match_type'] == 'mbid' + assert result['matches'][0]['confidence'] == EXACT_MATCH_CONFIDENCE + + +def test_isrc_exact_match_pairs_file_to_track(): + files = ['/a/track.flac'] + file_tags = { + '/a/track.flac': _tags(title='Foo', isrc='USRC11234567'), + } + tracks = [_api_track(name='Real', isrc='USRC11234567')] + result = find_exact_id_matches(files, file_tags, tracks) + assert len(result['matches']) == 1 + assert result['matches'][0]['match_type'] == 'isrc' + + +def test_isrc_normalization_strips_dashes_and_spaces(): + """File tag ``USRC11234567`` should match source ISRC ``US-RC1-12-34567`` + — same identifier, different formatting. Picard writes compact; + some sources return hyphenated.""" + files = ['/a/f.flac'] + file_tags = {'/a/f.flac': _tags(isrc='USRC11234567')} + tracks = [_api_track(name='X', isrc='US-RC1-12-34567')] + result = find_exact_id_matches(files, file_tags, tracks) + assert len(result['matches']) == 1 + + +def test_mbid_takes_priority_over_isrc(): + """When both identifiers are present and they'd point at different + tracks, MBID wins. ISRC can be shared across remasters; MBID is + per-recording.""" + files = ['/a/f.flac'] + file_tags = {'/a/f.flac': _tags(isrc='SAME', mbid='real-mbid')} + tracks = [ + _api_track(name='Wrong Recording', isrc='SAME', mbid='different-mbid'), + _api_track(name='Right Recording', mbid='real-mbid'), + ] + result = find_exact_id_matches(files, file_tags, tracks) + assert len(result['matches']) == 1 + assert result['matches'][0]['track']['name'] == 'Right Recording' + assert result['matches'][0]['match_type'] == 'mbid' + + +def test_isrc_via_external_ids_dict_matches(): + """Spotify exposes ISRC under ``external_ids.isrc``, not as a + top-level field. Matcher must check both shapes.""" + files = ['/a/f.flac'] + file_tags = {'/a/f.flac': _tags(isrc='USRC11234567')} + tracks = [_api_track(name='X', external_ids={'isrc': 'USRC11234567'})] + result = find_exact_id_matches(files, file_tags, tracks) + assert len(result['matches']) == 1 + + +def test_no_id_match_returns_empty(): + """File and track both have IDs, but they don't match → no exact + match. (Caller falls back to fuzzy.)""" + files = ['/a/f.flac'] + file_tags = {'/a/f.flac': _tags(mbid='different-id')} + tracks = [_api_track(name='X', mbid='another-id')] + result = find_exact_id_matches(files, file_tags, tracks) + assert not result['matches'] + + +def test_each_id_match_uses_track_at_most_once(): + """Two files with the same MBID — only the first one wins. Caller + deals with the leftover (probably a duplicate/extra file).""" + files = ['/a/first.flac', '/a/second.flac'] + file_tags = { + '/a/first.flac': _tags(mbid='shared'), + '/a/second.flac': _tags(mbid='shared'), + } + tracks = [_api_track(name='Track', mbid='shared')] + result = find_exact_id_matches(files, file_tags, tracks) + assert len(result['matches']) == 1 + assert len(result['used_files']) == 1 + + +# --------------------------------------------------------------------------- +# duration_sanity_ok — direct unit tests +# --------------------------------------------------------------------------- + + +def test_duration_within_tolerance_passes(): + assert duration_sanity_ok(180_000, 180_000) is True + assert duration_sanity_ok(180_000, 181_500) is True + assert duration_sanity_ok(180_000, 180_000 - DURATION_TOLERANCE_MS) is True + + +def test_duration_outside_tolerance_fails(): + assert duration_sanity_ok(180_000, 180_000 + DURATION_TOLERANCE_MS + 1) is False + assert duration_sanity_ok(180_000, 90_000) is False + # The Mr. Morale Auntie-Diaries-vs-Rich-Interlude case from the bug + # report: 281s file vs 103s expected — gross mismatch, must reject. + assert duration_sanity_ok(281_000, 103_000) is False + + +def test_duration_missing_either_side_passes(): + """Don't reject when we can't confirm. Files with no length info + (corrupt headers, etc.) defer to the fuzzy scorer.""" + assert duration_sanity_ok(0, 180_000) is True + assert duration_sanity_ok(180_000, 0) is True + assert duration_sanity_ok(0, 0) is True + + +# --------------------------------------------------------------------------- +# match_files_to_tracks — end-to-end with the new fast paths +# --------------------------------------------------------------------------- + + +def test_mbid_match_short_circuits_fuzzy_scoring(): + """File with MBID + completely wrong title still matches the right + track via MBID. Demonstrates the fast-path bypassing fuzzy scoring.""" + files = ['/a/file.flac'] + file_tags = { + '/a/file.flac': _tags( + title='Completely Wrong Title', + artist='Wrong Artist', + track=99, disc=99, + mbid='real-mbid', + ), + } + tracks = [ + _api_track(name='Real Title', track_number=1, disc_number=1, mbid='real-mbid'), + ] + result = match_files_to_tracks( + files, file_tags, tracks, + target_album='', similarity=_sim, quality_rank=_qrank, + ) + assert len(result['matches']) == 1 + assert result['matches'][0]['match_type'] == 'mbid' + assert result['matches'][0]['confidence'] == EXACT_MATCH_CONFIDENCE + + +def test_id_matched_files_excluded_from_fuzzy_phase(): + """File matched in phase 1 (exact ID) shouldn't be considered in + phase 3 (fuzzy). Otherwise it could end up matched twice.""" + files = ['/a/exact.flac', '/a/fuzzy.flac'] + file_tags = { + '/a/exact.flac': _tags(title='Track A', mbid='mbid-a'), + '/a/fuzzy.flac': _tags(title='Track B', track=2, disc=1), + } + tracks = [ + _api_track(name='Track A', track_number=1, disc_number=1, mbid='mbid-a'), + _api_track(name='Track B', track_number=2, disc_number=1), + ] + result = match_files_to_tracks( + files, file_tags, tracks, + target_album='', similarity=_sim, quality_rank=_qrank, + ) + assert len(result['matches']) == 2 + file_set = {m['file'] for m in result['matches']} + assert file_set == {'/a/exact.flac', '/a/fuzzy.flac'} + + +def test_duration_gate_rejects_wrong_disc_collision_in_fuzzy_phase(): + """The Mr. Morale bug case re-cast as a duration veto. File has + the audio length of the disc-2 track, API track is the disc-1 track + with the same number. Pre-fix: would have matched on track_number + alone. Post-fix: even after the disc-aware scoring, the duration + gate stops it.""" + files = ['/a/track06.flac'] + file_tags = { + '/a/track06.flac': _tags( + title='', track=6, disc=1, # wrong/missing disc tag + duration_ms=281_000, # actual audio is 4:41 + ), + } + tracks = [ + _api_track( + name='Rich (Interlude)', track_number=6, disc_number=1, + duration_ms=103_000, # 1:43 + ), + ] + result = match_files_to_tracks( + files, file_tags, tracks, + target_album='Mr. Morale', similarity=_sim, quality_rank=_qrank, + ) + # Duration gate rejects → file unmatched (correct). + assert not result['matches'] + assert result['unmatched_files'] == ['/a/track06.flac'] + + +def test_duration_gate_within_tolerance_allows_normal_match(): + """File and track durations agree within tolerance — match proceeds + normally via fuzzy scoring.""" + files = ['/a/track.flac'] + file_tags = { + '/a/track.flac': _tags( + title='Father Time', track=5, disc=1, duration_ms=362_000, + ), + } + tracks = [ + _api_track( + name='Father Time', track_number=5, disc_number=1, + duration_ms=363_500, # 1.5s drift — within 3s tolerance + ), + ] + result = match_files_to_tracks( + files, file_tags, tracks, + target_album='', similarity=_sim, quality_rank=_qrank, + ) + assert len(result['matches']) == 1 + + +def test_no_durations_anywhere_falls_through_to_fuzzy(): + """Either side missing duration → gate doesn't apply, fuzzy + scoring handles it. Catches files with corrupt audio headers.""" + files = ['/a/track.flac'] + file_tags = { + '/a/track.flac': _tags( + title='Father Time', track=5, disc=1, duration_ms=0, + ), + } + tracks = [_api_track(name='Father Time', track_number=5, disc_number=1)] + result = match_files_to_tracks( + files, file_tags, tracks, + target_album='', similarity=_sim, quality_rank=_qrank, + ) + assert len(result['matches']) == 1 + + +def test_deezer_seconds_duration_converted_to_ms(): + """Deezer's API returns ``duration`` in seconds, not ms. The matcher + must convert before applying the tolerance check — otherwise a + 180-second track looks like a 180-millisecond track and fails the + sanity gate against any real file.""" + files = ['/a/track.flac'] + file_tags = { + '/a/track.flac': _tags( + title='Song', track=1, disc=1, duration_ms=180_000, + ), + } + # Deezer-style track — duration is 180 (seconds) + tracks = [{ + 'name': 'Song', 'track_number': 1, 'disc_number': 1, + 'duration': 180, 'artists': [], + }] + result = match_files_to_tracks( + files, file_tags, tracks, + target_album='', similarity=_sim, quality_rank=_qrank, + ) + # 180 seconds → 180_000 ms → matches file's 180_000 ms within tolerance + assert len(result['matches']) == 1 + + +def test_album_track_entry_propagates_isrc_and_mbid_from_source(): + """Production-path guard: the metadata-source layer + (`_build_album_track_entry`) must propagate ISRC + MBID from the + raw track responses, otherwise the matcher's fast paths never fire + in production even though they pass in unit tests. + + Spotify shape: ``external_ids.isrc`` (nested dict). + iTunes shape: top-level ``isrc``. + """ + from core.metadata.album_tracks import _build_album_track_entry + + spotify_shape = { + 'id': 'spotify-track', + 'name': 'Test', + 'external_ids': {'isrc': 'USRC11234567', 'mbid': 'mb-123'}, + 'duration_ms': 200_000, + 'track_number': 1, + 'disc_number': 1, + } + entry = _build_album_track_entry(spotify_shape, {'name': 'Album'}, 'spotify') + assert entry['isrc'] == 'USRC11234567' + assert entry['musicbrainz_id'] == 'mb-123' + + itunes_shape = { + 'id': 'itunes-track', + 'name': 'Test', + 'isrc': 'USRC11234567', + 'duration_ms': 200_000, + 'track_number': 1, + 'disc_number': 1, + } + entry = _build_album_track_entry(itunes_shape, {'name': 'Album'}, 'itunes') + assert entry['isrc'] == 'USRC11234567' + + # No identifiers — entry has empty strings (not None / missing keys), + # so the matcher's `_track_identifier()` returns empty cleanly. + bare_shape = { + 'id': 'bare', 'name': 'Test', + 'duration_ms': 200_000, 'track_number': 1, 'disc_number': 1, + } + entry = _build_album_track_entry(bare_shape, {'name': 'Album'}, 'unknown') + assert entry['isrc'] == '' + assert entry['musicbrainz_id'] == '' + + +def test_picard_tagged_library_full_album_via_mbid_only(): + """Realistic Picard-tagged library: every file has MBID, no useful + title-disc-track agreement needed. Whole album should pair via the + fast path on the first phase.""" + files = [f'/a/picard_{i}.flac' for i in range(1, 11)] + file_tags = { + f: _tags( + title=f'mangled name {i}', # title doesn't help + track=99 - i, disc=99, # position info is wrong + mbid=f'mbid-{i}', + ) + for i, f in enumerate(files, start=1) + } + tracks = [ + _api_track( + name=f'Real Track {i}', + track_number=i, disc_number=1, + mbid=f'mbid-{i}', + ) + for i in range(1, 11) + ] + result = match_files_to_tracks( + files, file_tags, tracks, + target_album='', similarity=_sim, quality_rank=_qrank, + ) + assert len(result['matches']) == 10 + # All matched via MBID, full confidence + for m in result['matches']: + assert m['match_type'] == 'mbid' + assert m['confidence'] == EXACT_MATCH_CONFIDENCE diff --git a/webui/static/helper.js b/webui/static/helper.js index c1058796..4a174c35 100644 --- a/webui/static/helper.js +++ b/webui/static/helper.js @@ -3420,6 +3420,7 @@ const WHATS_NEW = { { title: 'Manual Picks Don\'t Auto-Retry Anymore (And The Modal Always Opens)', desc: 'three follow-on fixes to the manual-search feature once people started actually using it. (1) when the user picked a candidate and that download failed (e.g. soundcloud 404 on a stale track url), the auto-retry monitor would treat it like any other failed auto-attempt — yank the task back to "searching" and pick a different candidate. felt completely wrong from the user\'s perspective: "i picked THIS one, why is it searching for something else?" now manual picks are tagged with a `_user_manual_pick` flag and the auto-retry path bails on it. failure surfaces to the user instead of getting silently fallen-back. (2) non-soulseek manual picks (youtube / tidal / qobuz / hifi / deezer / soundcloud / lidarr) were getting stuck at "downloading 0%" forever even after their engine reported terminal failure. cause: status polling went into a "let monitor handle retry" branch that never fired because manual picks bail on retry — task was orphaned in downloading state. fix: when the engine reports Errored on a manual pick, mark the task failed directly, don\'t defer to the monitor. plus an engine-state fallback path covers the rare race where the orchestrator\'s pre-populated transfer lookup is missing the entry. (3) failed / not_found rows were only clickable when the auto-search had cached candidates — but the whole point of opening the modal now is to RUN a manual search, which doesn\'t need pre-existing candidates. now every failed / not_found / cancelled row opens the modal regardless. (4) one nasty deadlock fix in the process: the new "mark failed" path was synchronously calling `on_download_completed` while holding `tasks_lock`, which itself re-acquires the same lock — `threading.Lock` is non-reentrant so the polling thread wedged forever. while wedged the lock was held → every other endpoint that needed it (including /candidates → can\'t open OTHER modals) hung waiting. moved completion callbacks onto a daemon thread so the lock releases first. (5) manual download worker now runs on its own dedicated thread instead of competing with the batch\'s 3-worker `missing_download_executor` pool — saturated batches no longer queue manual picks indefinitely. all changes are scoped to manual picks only via the `_user_manual_pick` flag — auto-attempt flow is byte-identical to before. 17 unit tests pin the gate behavior (status engine fallback / monitor retry skip / IF-branch failure transition / auto-attempt skip).', page: 'downloads' }, { title: 'Manual Import: Stop Writing "Unknown Artist / album_id / 0 tracks" Garbage', desc: 'github issue #524 (radoslav-orlov): clicking an album in the import page → all imported albums landed in the library as "Unknown Artist" with the raw 10-digit album id as the title and 0 tracks. cause: the click handler `importPageSelectAlbum(albumId)` was passing only the id to the `/api/import/album/match` POST. the search response carried `source` (which metadata source the album_id came from) + `album_name` + `album_artist`, but the click discarded everything except the id. backend `get_artist_album_tracks` then guessed the source via the configured primary-source priority chain — for a non-deezer-primary user clicking a deezer search result, the chain tries spotify/itunes/discogs first against a deezer numeric id, all return None, and the lookup falls through to the failure-fallback dict (`name = album_id`, no artist field, `total_tracks = 0`). that broken metadata then flowed through the import pipeline → soulsync standalone library got the garbage rows. fix: cache album lookup by id when the suggestions / search renderers run, then have `importPageSelectAlbum` pull `source` + `name` + `artist` from the cache and include them in the match POST. backend now also logs a clear warning when source is missing from the match request, so any future caller dropping it shows up in app.log instead of silently corrupting library imports.', page: 'import' }, { title: 'Auto-Import: Multi-Disc Albums No Longer Lose Half The Tracks', desc: 'caught while testing #524 with kendrick lamar mr morale & the big steppers (3 discs). dropped discs 1+2 loose in staging root + disc 3 in its own folder, all perfectly tagged → only 9 tracks ended up imported, the rest got integrity-rejected and quarantined. two related bugs in `auto_import_worker._match_tracks`: (1) the "quality dedup" loop kept `seen_track_nums[track_number] = file` and dropped any later file with the same number as a quality duplicate. on a multi-disc release where every disc has tracks 1..N, that collapses the album to one disc\'s worth of files BEFORE the matcher even runs. fix: dedup keys on `(disc_number, track_number)` tuples instead. (2) the 30% track-number bonus in the match scoring fired whenever `ft[track_number] == track_num` regardless of disc — file tagged (disc=2, track=6, "Auntie Diaries") got the full bonus matching API track (disc=1, track=6, "Rich Interlude"), wrong file → integrity check correctly rejected and quarantined. fix: 30% bonus only when BOTH disc and track numbers agree, with a small consolation bonus for cross-disc collisions so title similarity has to carry the match. 4 new tests pin: dedup preserves all files across discs (18-file regression case), match scoring pairs to correct disc, single-disc albums still match normally, and quality dedup within a single (disc, track) position still picks the higher quality file.', page: 'import' }, + { title: 'Auto-Import: Picard / Beets Tagged Libraries Now Get Perfect Matches', desc: 'follow-on to the multi-disc fix. brought the auto-import matcher up to picard / beets / roon parity — files with per-recording identifiers (musicbrainz id or isrc) now match via exact-id lookup before any fuzzy scoring runs. picard-tagged libraries land every track on the first pass with full confidence, no fuzzy guessing. three layered phases now: (1) MBID exact match — file has `musicbrainz_trackid` tag, source returns the same id → instant pair, full confidence. picard\'s primary identifier. (2) ISRC exact match — file has `isrc` tag, source returns the same id → same fast-path, slightly lower priority than mbid (isrc can be shared across remasters of the same recording). (3) duration sanity gate — files in the fuzzy phase whose audio length differs from the candidate track\'s duration by more than 3s are rejected before scoring runs, regardless of how good the title agreement looks. defends against cross-disc / cross-release / wrong-edit mismatches the post-download integrity check used to catch only AFTER the file had already been moved + tagged + db-inserted. metadata-source layer (`_build_album_track_entry`) also extended to propagate isrc + mbid from raw track responses (spotify uses `external_ids.isrc`, itunes uses top-level `isrc`) — without this, fast paths would never trigger in production even though unit tests pass. 18 new tests pin: mbid + isrc exact matches with normalization (dashes / spacing / case), mbid > isrc priority, fast-path bypassing fuzzy scoring entirely, duration gate rejecting wrong-disc collisions, deezer-seconds-vs-spotify-ms duration unit conversion, full picard-tagged 10-track album matching via mbid only.', page: 'import' }, ], '2.4.3': [ // --- May 8, 2026 — patch release ---