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.
412 lines
15 KiB
Python
412 lines
15 KiB
Python
"""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
|