danvics/soulsync
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions 1

Fix popup: anchor artist field in MB search to stop title-collision covers

Browse source 
`/api/musicbrainz/search_tracks` powers the Fix popup's auto-search
cascade for users on MusicBrainz as primary. When both track + artist
fields were filled, `search_tracks_with_artist` always took the bare
keyword path (`<track> <artist>` joined as one query string). MB's
recording-search scorer weights title matches far above artist matches,
so for "Coffee Break" + "Zeds Dead" the top results were Emapea / The
Vidalias / West One Orchestra's "Coffee Break" — three unrelated cover-
title collisions ahead of the canonical Zeds Dead recording. The
endpoint's `rerank_tracks` pass can't fix this when the right answer
is below the API's 50-result cutoff.

Both-fields mode now uses a strict field-scoped Lucene query first
(`recording:"<t>" AND artist:"<a>"`) which anchors the artist and
prunes title-collision covers at the source. `min_score=0` because the
field-scoped query is itself precise; rerank still does final ordering.
Bare query stays as the fallback when strict returns nothing — covers
the diacritic / alias cases the original `strict=False` path was added
for ("Bjork" query vs canonical "Björk" artist where Lucene phrase
match never hits the recording).

Single-field mode (track-only or artist-only) is unchanged: still bare-
query directly, since there's no artist value to anchor.

Also stable-sort results to prefer entries with non-zero `duration_ms`.
MB has multiple recordings per song (single release, album release,
remasters, compilations) and not every recording carries length data.
Without the preference sort, the user sees a 0:00 row first while a
sibling recording with the real 3:04 sits two rows below — matches the
report where MBID-paste lookup of the canonical recording (length 3:04)
contradicted the search-result's 0:00 row for the same song.

Tests:
- new `test_search_tracks_with_artist_strict_first_when_both_fields`
  pins the strict=True call when both fields present
- new `test_search_tracks_with_artist_falls_back_to_bare_when_strict_empty`
  pins the Björk-style fall-through path
- new `test_search_tracks_with_artist_prefers_results_with_known_length`
  pins the length-preference sort
- existing `..._keeps_low_score_for_rerank` updated to side_effect so
  the bare-fallback path is exercised; behaviour pinned identically
- existing `..._uses_bare_query_mode` renamed + repurposed for strict-
  first; old name's behaviour no longer accurate
This commit is contained in:
Broque Thomas 2026-05-26 23:00:19 -07:00
parent 4555ff7eb9
commit acc5eb77ea
3 changed files with 126 additions and 45 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

47
core/musicbrainz_search.py
View file

@ -757,19 +757,46 @@ class MusicBrainzSearchClient:
`search_tracks`'s structured-query dispatch (`Artist - Track`
splitting, bare-name artist-first browse).
Uses bare-query mode (`strict=False`) — diacritic-folded, hits
alias/sortname indexes, no `AND`-clause that kills recall when
either side mis-matches. Score floor lowered to 20 (vs the search
tab's 80) so MB recordings whose title doesn't literally contain
the artist name still enter the candidate pool — the endpoint's
`rerank_tracks` pass then sorts by artist-match relevance. Without
this, queries like `Army of Me` + `Bjork` only surface covers
(score 73-100) and miss Björk's canonical recording (score 28).
When both fields are present: strict-first, bare-as-fallback. The
strict pass builds a field-scoped Lucene query (`recording:"<t>"
AND artist:"<a>"`) which anchors the artist and prunes title-
collision covers — fixes the "Coffee Break" + "Zeds Dead" case
where MB's title-text-biased scorer surfaced Emapea / Vidalias /
West One Orchestra ahead of the canonical Zeds Dead recording.
`min_score=0` on strict because the field-scoped query is itself
precise, and the endpoint's `rerank_tracks` pass does the final
ordering. Bare query runs only when strict returns nothing —
catches diacritic / alias mismatches (`Bjork` query vs canonical
`Björk` artist) where strict phrase match never hits.
When only one field is present: bare-query mode directly — same
recall-over-precision tradeoff the old single-path took.
Results are stable-sorted to prefer entries with non-zero
`duration_ms`. MB often has several recordings per song (single
release, album release, compilations, remasters) and not every
recording carries length data. Without this, the first match can
be a length-less duplicate while a sibling recording with the
real 3:04 sits two rows down.
"""
if not track and not artist:
return []
return self._search_tracks_text(track, artist or None, limit,
strict=False, min_score=20)
if track and artist:
results = self._search_tracks_text(
track, artist, limit, strict=True, min_score=0
)
if not results:
results = self._search_tracks_text(
track, artist, limit, strict=False, min_score=20
)
else:
results = self._search_tracks_text(
track, artist or None, limit, strict=False, min_score=20
)
results.sort(key=lambda t: 0 if (t.duration_ms or 0) > 0 else 1)
return results
def _pick_representative_release(self, releases: List[Dict[str, Any]]) -> Optional[Dict[str, Any]]:
"""Pick the best release out of a release-group's editions.

123
tests/metadata/test_musicbrainz_search.py
View file

@ -1000,33 +1000,84 @@ def test_get_recording_flat_swallows_client_errors():
# search_tracks_with_artist — Fix-popup cascade adapter
# ---------------------------------------------------------------------------
def test_search_tracks_with_artist_uses_bare_query_mode():
"""The Fix-popup cascade needs MB's bare-query mode so diacritics and
bracketed suffixes don't kill recall. The adapter must pass strict=False
through to the underlying search_recording call."""
def test_search_tracks_with_artist_strict_first_when_both_fields():
"""Both fields present → strict field-scoped Lucene query first
(`recording:"<t>" AND artist:"<a>"`). Fixes the "Coffee Break" +
"Zeds Dead" case where bare query lets MB's title-text-biased
scorer surface unrelated covers ahead of the canonical recording."""
client = MusicBrainzSearchClient()
client._client = MagicMock()
client._client.search_recording.return_value = [
{'id': 'rec-1', 'title': 'Army of Me', 'score': 95,
'releases': [{'id': 'rel-1', 'title': 'Post', 'date': '1995'}],
'artist-credit': [{'name': 'Björk'}]},
{'id': 'rec-1', 'title': 'Coffee Break', 'score': 95,
'length': 184000,
'releases': [{'id': 'rel-1', 'title': 'Coffee Break', 'date': '2015'}],
'artist-credit': [{'name': 'Zeds Dead'}]},
]
tracks = client.search_tracks_with_artist('Army of Me', 'Björk', limit=10)
tracks = client.search_tracks_with_artist('Coffee Break', 'Zeds Dead', limit=10)
# strict=False is the critical bit — fuzzy recall, not phrase precision
# strict=True is the critical bit — anchors artist via Lucene AND clause
client._client.search_recording.assert_called_once_with(
'Army of Me', artist_name='Björk', limit=10, strict=False
'Coffee Break', artist_name='Zeds Dead', limit=10, strict=True
)
assert len(tracks) == 1
assert tracks[0].name == 'Army of Me'
assert 'Björk' in tracks[0].artists
assert tracks[0].name == 'Coffee Break'
assert 'Zeds Dead' in tracks[0].artists
def test_search_tracks_with_artist_falls_back_to_bare_when_strict_empty():
"""Strict phrase match misses diacritic / alias cases ("Bjork" query
vs canonical "Björk" artist). When strict returns nothing, fall
through to bare query so rerank can still surface the right answer."""
client = MusicBrainzSearchClient()
client._client = MagicMock()
client._client.search_recording.side_effect = [
[], # strict pass → no hits (Lucene phrase match fails on diacritic)
[ # bare pass → recall via alias index
{'id': 'rec-canonical', 'title': 'Army of Me', 'score': 28,
'releases': [], 'artist-credit': [{'name': 'Björk'}]},
],
]
tracks = client.search_tracks_with_artist('Army of Me', 'Bjork', limit=10)
assert client._client.search_recording.call_count == 2
first_call = client._client.search_recording.call_args_list[0]
second_call = client._client.search_recording.call_args_list[1]
assert first_call.kwargs['strict'] is True
assert second_call.kwargs['strict'] is False
assert len(tracks) == 1
assert tracks[0].id == 'rec-canonical'
def test_search_tracks_with_artist_prefers_results_with_known_length():
"""MB has multiple recordings per song (single release, album release,
compilations) and not every recording carries length data. Stable
sort moves length-known entries ahead of length-zero duplicates so
the user sees the actionable 3:04 row first, not the 0:00 sibling."""
client = MusicBrainzSearchClient()
client._client = MagicMock()
client._client.search_recording.return_value = [
{'id': 'rec-no-length', 'title': 'Coffee Break', 'score': 100,
'releases': [], 'artist-credit': [{'name': 'Zeds Dead'}]},
{'id': 'rec-with-length', 'title': 'Coffee Break', 'score': 90,
'length': 184000,
'releases': [], 'artist-credit': [{'name': 'Zeds Dead'}]},
]
tracks = client.search_tracks_with_artist('Coffee Break', 'Zeds Dead', limit=10)
# rec-with-length must surface first even though MB scored it lower
assert tracks[0].id == 'rec-with-length'
assert tracks[0].duration_ms == 184000
assert tracks[1].id == 'rec-no-length'
def test_search_tracks_with_artist_handles_missing_artist():
"""Track-only query (no artist) still works — empty string becomes
None, and the underlying client searches recordings without an
artist filter."""
"""Track-only query (no artist) still works — single-field path takes
bare-query mode directly (no strict-first round-trip since there's no
artist to anchor). Empty string becomes None so MB drops the AND
clause."""
client = MusicBrainzSearchClient()
client._client = MagicMock()
client._client.search_recording.return_value = [
@ -1036,7 +1087,6 @@ def test_search_tracks_with_artist_handles_missing_artist():
client.search_tracks_with_artist('Some Song', '', limit=5)
# Empty artist → None passed to the client so MB drops the AND clause
client._client.search_recording.assert_called_once_with(
'Some Song', artist_name=None, limit=5, strict=False
)
@ -1051,33 +1101,33 @@ def test_search_tracks_with_artist_empty_returns_empty_list():
client._client.search_recording.assert_not_called()
def test_search_tracks_with_artist_keeps_low_score_for_rerank():
"""Cascade path uses a low score floor (20) so MB recordings whose
title doesn't literally contain the artist name still enter the
candidate pool — the endpoint's rerank pass surfaces them by
artist-match relevance. Real example: "Army of Me" + "Bjork" — the
canonical Björk recording scores 28 in MB (title doesn't contain
"Bjork"), while title-collision covers like "Army of Me (Bjork)"
score 73-100. Strict 80 floor drops the right answer."""
def test_search_tracks_with_artist_bare_fallback_keeps_low_score_for_rerank():
"""When strict returns nothing and we fall through to bare, the bare
pass uses a low score floor (20) so MB recordings whose title doesn't
literally contain the artist name still enter the candidate pool —
the endpoint's rerank pass surfaces them by artist-match relevance.
Real example: "Army of Me" + "Bjork" — strict fails on the diacritic
mismatch, bare picks up the canonical Björk recording at score 28
while filtering true noise at score 5."""
client = MusicBrainzSearchClient()
client._client = MagicMock()
client._client.search_recording.return_value = [
{'id': 'rec-cover', 'title': 'Army of Me (Bjork)', 'score': 100,
'releases': [], 'artist-credit': [{'name': 'HIRS Collective'}]},
{'id': 'rec-canonical', 'title': 'Army of Me', 'score': 28,
'releases': [], 'artist-credit': [{'name': 'Björk'}]},
{'id': 'rec-noise', 'title': 'Bjork', 'score': 5,
'releases': [], 'artist-credit': [{'name': 'Random'}]},
client._client.search_recording.side_effect = [
[], # strict pass → no hits
[ # bare pass
{'id': 'rec-cover', 'title': 'Army of Me (Bjork)', 'score': 100,
'releases': [], 'artist-credit': [{'name': 'HIRS Collective'}]},
{'id': 'rec-canonical', 'title': 'Army of Me', 'score': 28,
'releases': [], 'artist-credit': [{'name': 'Björk'}]},
{'id': 'rec-noise', 'title': 'Bjork', 'score': 5,
'releases': [], 'artist-credit': [{'name': 'Random'}]},
],
]
tracks = client.search_tracks_with_artist('Army of Me', 'Bjork', limit=50)
ids = [t.id for t in tracks]
# Score=28 canonical Björk recording is kept — the endpoint's rerank
# will surface it by artist match.
assert 'rec-canonical' in ids
assert 'rec-cover' in ids
# Score=5 is below the 20 floor — true garbage still filtered out.
assert 'rec-noise' not in ids
@ -1120,7 +1170,10 @@ def test_search_tracks_text_strict_param_default_true():
def test_search_tracks_with_artist_swallows_client_errors():
"""MB client raising must not crash the endpoint — return [] so the
Fix-popup cascade falls through to the next source."""
Fix-popup cascade falls through to the next source. Both strict and
bare passes swallow exceptions independently, so a strict-pass raise
still lets the bare-pass run; a bare-pass raise after empty strict
returns []."""
client = MusicBrainzSearchClient()
client._client = MagicMock()
client._client.search_recording.side_effect = RuntimeError('network down')

1
webui/static/helper.js
View file

@ -3415,6 +3415,7 @@ function closeHelperSearch() {
const WHATS_NEW = {
'2.6.3': [
{ unreleased: true },
{ title: 'Fix popup: artist + track fields no longer surface unrelated covers', desc: 'separate artist / track inputs in the Fix popup used to dump both into a bare MusicBrainz keyword query — and MB\'s scorer heavily favours title matches, so searching "Coffee Break" + "Zeds Dead" surfaced random "Coffee Break" tracks by other artists ahead of the canonical Zeds Dead one. fields-mode now uses a field-scoped Lucene query that actually anchors the artist, with the old bare query kept as a fallback for diacritic mismatches like "Bjork" vs "Björk" where strict phrase match misses. results also stable-prefer entries with known track length so the canonical 3:04 sibling sits above the 0:00 duplicate.' },
{ title: 'Groundwork: unified playlist source layer', desc: 'first slice of a refactor that\'ll let ListenBrainz, Last.fm radio, and SoulSync Discovery playlists live as Sync-page tabs alongside Spotify / Tidal / Qobuz / YouTube — so they can be mirrored + scheduled like the rest. this commit adds the shared adapter layer all those sources will plug into; no UI changes yet. nothing to do on your end.' },
{ title: 'Auto-Sync refresh now routes through the unified source layer', desc: 'follow-up to the groundwork above. the mirrored-playlist auto-refresh handler used to have a ~190-line if/elif chain branching per source (one branch each for Spotify, Spotify public, Deezer, Tidal, YouTube). now it asks the source registry for the right adapter and calls one refresh method. behavior identical — same matched_data, same Tidal-skip-on-no-auth log, same Spotify-public-prefers-authed-API fallback. unlocks ListenBrainz / Last.fm / SoulSync Discovery as future Sync-page mirror sources without a fresh elif branch each time.' },
{ title: 'Discovery folded into the unified source contract', desc: 'next slice of the groundwork. each playlist source can now answer one extra question — "match these raw tracks against Spotify / iTunes" — through the same adapter interface. Spotify / Tidal / Qobuz / YouTube / Deezer / Spotify-public / iTunes-link / SoulSync-Discovery all answer trivially (their tracks already have provider IDs); ListenBrainz + Last.fm run the matching engine. mirror-refresh now calls this automatically when a source returns MB-metadata-only tracks, so when ListenBrainz becomes a Sync-page tab next commit, its mirrors land already discovered + ready to sync — no separate Discover-page round-trip needed.' },