fix(metadata): surface MusicBrainz 'Other' release-groups in discography (#650)
S-Bryce reported that for some artists (Vocaloid producers, JP indie
acts, niche Western indie) the artist detail page was missing whole
release-groups visible on musicbrainz.org. Downloaded tracks from
those release-groups appeared in artist track counts but were not
bound to any visible album / single card — orphan "ghost" tracks the
user couldn't browse to.
Two duplicated bugs fed each other:
1. `core/musicbrainz_search.py` browsed MB release-groups with
`release_types=['album', 'ep', 'single']`. MB's primary-type
vocabulary is {Album, Single, EP, Broadcast, Other} — music
videos, one-off web releases, and broadcast singles use Other.
Pre-fix the filter dropped them at the API layer.
2. Three sites duplicated the same "raw primary-type → internal
album_type" mapping with slightly different vocabularies and all
silently defaulted unknown values (including 'Other') to 'album':
core/musicbrainz_search.py `_map_release_type`
core/metadata/types.py inline `{single:single, ep:ep}.get(...)`
core/metadata/cache.py Deezer-specific record_type guard
Letting Other through the filter without a real mapper would have
placed music videos in the Albums view alongside LPs — visually
misleading.
Fix shape:
- New `core/metadata/release_type.py` — single canonical mapper
consumed by every provider's raw→Album projection. Knows the full
MB vocabulary including 'other' and 'broadcast'; routes both into
the singles bucket since they're functionally single-track
releases. Compilation secondary-type override preserved (MB's
canonical Greatest-Hits pattern is `primary=Album,
secondary=[Compilation]`).
- `core/musicbrainz_search.py` `_map_release_type` becomes a thin
alias for the new helper so the six internal call sites stay
intact. API filter gains 'other'.
- `core/metadata/types.py` Album projection drops its inline mini-
mapper and calls the canonical helper. Now also handles the
compilation secondary-type override it was previously missing.
- The Deezer-specific cache.py guard stays as-is — Deezer's
record_type vocabulary is closed (album|single|ep), not affected
by this issue.
Verified end-to-end against MB for S-Bryce's artist (`46196b9c-affa-
4616-b53b-e967c8bd70e0`, inabakumori): pre-fix returned 22 release-
groups; post-fix returns 27, with the 5 extra all landing in the
Singles section with album_type='single' as intended.
23 new unit tests pin the mapper contract (case-insensitive primary
types, compilation secondary override, Other/Broadcast → single,
unknown → album default preserved, defensive empty/None inputs).
2 new tests in test_musicbrainz_search pin the API filter inclusion
of 'other' and the round-trip into the Singles bucket. All 516
existing metadata tests still green — refactor leaves historical
behaviour for {album, ep, single, compilation} unchanged.
This commit is contained in:
Broque Thomas
parent
54e4ba843f
commit
987409508b
6 changed files with 313 additions and 16 deletions
93
core/metadata/release_type.py
Normal file
93
core/metadata/release_type.py
Normal file
|
|
@ -0,0 +1,93 @@
|
|||
"""Canonical mapping from raw provider release-type vocabulary to the
|
||||
internal `album_type` field that drives discography binning + UI.
|
||||
|
||||
Why this exists
|
||||
---------------
|
||||
Three sites historically duplicated the same "best-effort primary-type
|
||||
→ album_type" mapping, each with a slightly different vocabulary:
|
||||
|
||||
core/musicbrainz_search.py: `_map_release_type` knew about
|
||||
{album, single, ep, compilation}, defaulted unknown → 'album'.
|
||||
core/metadata/types.py: inline `{single: single, ep: ep}.get(...)` —
|
||||
didn't even know about 'compilation', also defaulted → 'album'.
|
||||
core/metadata/cache.py: Deezer-specific record_type validator —
|
||||
intentionally narrow, kept here for its provider.
|
||||
|
||||
Issue #650 (S-Bryce) reported that MusicBrainz tags music videos and
|
||||
some legitimate singles with primary-type=`Other`, which both mappers
|
||||
silently routed to `album_type='album'`. Combined with the API-level
|
||||
filter at `musicbrainz_search.search_albums` (which only requested
|
||||
`type=album|ep|single` from MB and dropped 'Other' entirely), users
|
||||
with MB-as-primary saw entire release-groups go missing from artist
|
||||
discography views, and downloaded tracks from those release-groups
|
||||
appeared as orphan "ghost" tracks bound to no album card.
|
||||
|
||||
Fix shape: one shared mapper consumed by every provider's
|
||||
`raw → Album dataclass` projection. Knows about 'other' and
|
||||
'broadcast' (MB's two remaining primary-type vocabulary words) and
|
||||
maps them to 'single' so they land in the Singles section of the
|
||||
artist detail page — they're almost always single-track music
|
||||
releases (music videos, broadcast singles, one-off web releases).
|
||||
Falling through to 'album' was the original sin — places them in
|
||||
Albums view where they look misleading and clutter the proper LP
|
||||
list.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from typing import List, Optional
|
||||
|
||||
|
||||
# MB primary-type vocabulary as of 2026 — `Album | Single | EP |
|
||||
# Broadcast | Other`. Compilation is a *secondary* type; querying MB
|
||||
# with type=compilation silently breaks (returns ~10% of expected
|
||||
# results) — see musicbrainz_client.browse_artist_release_groups docs.
|
||||
_AUDIO_OTHER_PRIMARY_TYPES = frozenset({'other', 'broadcast'})
|
||||
|
||||
|
||||
def map_release_group_type(primary_type: Optional[str],
|
||||
secondary_types: Optional[List[str]] = None) -> str:
|
||||
"""Project a raw provider release-group primary-type + secondary-types
|
||||
into the internal `album_type` value the UI binning expects.
|
||||
|
||||
Returns one of: `'album'`, `'single'`, `'ep'`, `'compilation'`.
|
||||
|
||||
Mapping rules:
|
||||
|
||||
- `single` / `ep` pass through unchanged.
|
||||
- `compilation` (primary or secondary) becomes `'compilation'`. The
|
||||
compilation secondary-type check is required because MB's
|
||||
canonical pattern is `primary=Album, secondary=[Compilation]`.
|
||||
- `other` / `broadcast` become `'single'`. Almost always
|
||||
single-track music releases (music videos, one-off web drops,
|
||||
broadcast singles). Placing them in Singles is the pragmatic
|
||||
bucket — they're not LPs, but excluding them entirely (the
|
||||
pre-fix behaviour) hid legitimate tracks.
|
||||
- Anything else (including empty/None) → `'album'`. Matches the
|
||||
pre-fix default so no existing classifications shift.
|
||||
|
||||
`secondary_types` is optional because the legacy types.py call site
|
||||
doesn't have access to it from the same level of raw structure.
|
||||
Pass `None` (or omit) for the secondary-types-unavailable path.
|
||||
"""
|
||||
pt = (primary_type or '').strip().lower()
|
||||
|
||||
if pt == 'single':
|
||||
return 'single'
|
||||
if pt == 'ep':
|
||||
return 'ep'
|
||||
if pt == 'compilation':
|
||||
return 'compilation'
|
||||
|
||||
# Secondary-type override: MB's compilation albums always carry
|
||||
# `primary=Album` with `secondary=[Compilation]`, so the primary
|
||||
# check above can't catch them.
|
||||
if secondary_types:
|
||||
normalized = {str(s).strip().lower() for s in secondary_types if s}
|
||||
if 'compilation' in normalized:
|
||||
return 'compilation'
|
||||
|
||||
if pt in _AUDIO_OTHER_PRIMARY_TYPES:
|
||||
return 'single'
|
||||
|
||||
return 'album'
|
||||
|
|
@ -400,10 +400,16 @@ class Album:
|
|||
if raw.get('barcode'):
|
||||
external_ids['barcode'] = _str(raw['barcode'])
|
||||
|
||||
# MB `release-group` carries the album-level type (album/single/ep)
|
||||
# MB `release-group` carries the album-level type (album/single/ep/
|
||||
# compilation/other/broadcast). Centralized mapper handles the
|
||||
# full vocabulary including 'other' / 'broadcast' (issue #650 —
|
||||
# music videos and one-off releases) so this projection matches
|
||||
# the search-adapter projection in `core/musicbrainz_search.py`.
|
||||
from core.metadata.release_type import map_release_group_type
|
||||
rg = raw.get('release-group') or {}
|
||||
primary_type = _str(rg.get('primary-type'), default='Album').lower()
|
||||
album_type = {'single': 'single', 'ep': 'ep'}.get(primary_type, 'album')
|
||||
primary_type = _str(rg.get('primary-type'), default='Album')
|
||||
secondary_types = rg.get('secondary-types') or []
|
||||
album_type = map_release_group_type(primary_type, secondary_types)
|
||||
if rg.get('id'):
|
||||
external_ids['musicbrainz_release_group'] = _str(rg['id'])
|
||||
|
||||
|
|
|
|||
|
|
@ -114,18 +114,12 @@ def _extract_title_hint(query: str, artist_name: str) -> Optional[str]:
|
|||
return None
|
||||
|
||||
|
||||
def _map_release_type(primary_type: str, secondary_types: List[str] = None) -> str:
|
||||
"""Map MusicBrainz release group type to standard album_type."""
|
||||
pt = (primary_type or '').lower()
|
||||
if pt == 'album':
|
||||
return 'album'
|
||||
elif pt == 'single':
|
||||
return 'single'
|
||||
elif pt == 'ep':
|
||||
return 'ep'
|
||||
elif pt == 'compilation' or 'compilation' in (secondary_types or []):
|
||||
return 'compilation'
|
||||
return 'album'
|
||||
# Thin module-level alias retained so callers inside this file keep
|
||||
# working without touching every call site. The canonical implementation
|
||||
# (including the 'other' / 'broadcast' handling that fixes issue #650)
|
||||
# lives in `core/metadata/release_type.py` so every provider's `raw →
|
||||
# Album` projection shares one mapper.
|
||||
from core.metadata.release_type import map_release_group_type as _map_release_type
|
||||
|
||||
|
||||
class MusicBrainzSearchClient:
|
||||
|
|
@ -365,7 +359,15 @@ class MusicBrainzSearchClient:
|
|||
# the filter silently breaks. Actual compilations
|
||||
# (primary-type=Album with secondary-types=[Compilation])
|
||||
# are handled by the studio-preference filter below.
|
||||
release_types=['album', 'ep', 'single'],
|
||||
# 'other' added per issue #650 — MB tags music videos
|
||||
# and one-off web/broadcast releases with primary=Other,
|
||||
# and many artists (Vocaloid producers, indie acts, JP
|
||||
# solo artists) have legitimate singles classified
|
||||
# there. Pre-fix this filter dropped them at the API
|
||||
# layer, hiding tracks the user had downloaded.
|
||||
# `map_release_group_type` routes 'other' into the
|
||||
# singles bucket so they appear in the right UI section.
|
||||
release_types=['album', 'ep', 'single', 'other'],
|
||||
limit=100,
|
||||
)
|
||||
|
||||
|
|
|
|||
|
|
@ -680,6 +680,61 @@ def test_search_albums_bare_artist_no_hint_no_filter():
|
|||
assert 'Revolver' in titles
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Issue #650 — 'Other' primary-type release-groups must surface
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def test_search_albums_browse_filter_requests_other_primary_type():
|
||||
"""Issue #650: pre-fix the MB browse filter requested only
|
||||
`album|ep|single`, dropping every primary-type=`Other` release-group
|
||||
at the API layer. For artists like Vocaloid producers and JP indie
|
||||
acts whose music videos / one-off web releases are tagged Other,
|
||||
that hid legitimate tracks. Pin that the filter now includes
|
||||
'other' so those release-groups round-trip into the discography."""
|
||||
client = MusicBrainzSearchClient()
|
||||
client._client = MagicMock()
|
||||
client._client.search_artist.return_value = [_mk_artist('Inabakumori', 'mb-i', score=100)]
|
||||
client._client.browse_artist_release_groups.return_value = []
|
||||
|
||||
client.search_albums('inabakumori', limit=10)
|
||||
|
||||
# Inspect the actual call args — the API filter is the lever that
|
||||
# decides whether MB returns Other-typed groups at all.
|
||||
args, kwargs = client._client.browse_artist_release_groups.call_args
|
||||
requested_types = kwargs.get('release_types') or (args[1] if len(args) > 1 else None)
|
||||
assert requested_types is not None, \
|
||||
"browse_artist_release_groups must receive an explicit release_types filter"
|
||||
assert 'other' in requested_types, \
|
||||
f"'other' must be in the requested types so #650 Other-typed releases surface; got {requested_types}"
|
||||
|
||||
|
||||
def test_search_albums_other_type_release_groups_appear_as_singles():
|
||||
"""When MB returns an Other-typed release-group (music video,
|
||||
one-off web release), it must arrive in the discography as an
|
||||
Album dataclass with album_type='single' — so the downstream
|
||||
binner in `core/metadata/discography.py` routes it to the Singles
|
||||
section rather than burying it among LPs."""
|
||||
client = MusicBrainzSearchClient()
|
||||
client._client = MagicMock()
|
||||
client._client.search_artist.return_value = [_mk_artist('Inabakumori', 'mb-i', score=100)]
|
||||
client._client.browse_artist_release_groups.return_value = [
|
||||
{'id': 'rg-mv', 'title': 'ロストアンブレラ', 'primary-type': 'Other',
|
||||
'first-release-date': '2018-02-27', 'secondary-types': []},
|
||||
{'id': 'rg-single', 'title': 'ラグトレイン', 'primary-type': 'Single',
|
||||
'first-release-date': '2020-01-01', 'secondary-types': []},
|
||||
]
|
||||
|
||||
albums = client.search_albums('inabakumori', limit=10)
|
||||
|
||||
by_id = {a.id: a for a in albums}
|
||||
assert 'rg-mv' in by_id, "Other-typed release-group must survive the filter and arrive in the result"
|
||||
assert by_id['rg-mv'].album_type == 'single', \
|
||||
"Other-typed release-group must map to album_type='single' so it lands in the Singles section"
|
||||
# Pre-existing single behaviour unchanged.
|
||||
assert by_id['rg-single'].album_type == 'single'
|
||||
|
||||
|
||||
def test_recording_to_track_total_tracks_matches_media_count():
|
||||
"""Regression: total_tracks was initialized at 1 and summed with media
|
||||
track-counts, producing an off-by-one. An 11-track album reported 12."""
|
||||
|
|
|
|||
140
tests/metadata/test_release_type.py
Normal file
140
tests/metadata/test_release_type.py
Normal file
|
|
@ -0,0 +1,140 @@
|
|||
"""Tests for the canonical release-type mapper.
|
||||
|
||||
Covers issue #650 — MusicBrainz's `Other` and `Broadcast` primary
|
||||
types previously defaulted to `album_type='album'`, hiding music
|
||||
videos and one-off releases from artist discography views. The mapper
|
||||
now routes them to `single` so they land in the Singles bucket of the
|
||||
artist detail page.
|
||||
|
||||
Also pins the existing mappings (album/ep/single/compilation) so the
|
||||
refactor of three sibling type-mappers into one shared helper doesn't
|
||||
drift the historical behaviour.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import pytest
|
||||
|
||||
from core.metadata.release_type import map_release_group_type
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Pin existing primary-type mappings (no regression from refactor)
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
@pytest.mark.parametrize("primary_type,expected", [
|
||||
("album", "album"),
|
||||
("Album", "album"), # MB returns title-cased values
|
||||
("ALBUM", "album"),
|
||||
("single", "single"),
|
||||
("Single", "single"),
|
||||
("ep", "ep"),
|
||||
("EP", "ep"),
|
||||
("compilation", "compilation"),
|
||||
("Compilation", "compilation"),
|
||||
])
|
||||
def test_known_primary_types_map_canonically(primary_type, expected):
|
||||
"""Pin: case-insensitive primary-type mapping for the four
|
||||
canonical types every consumer relied on pre-refactor."""
|
||||
assert map_release_group_type(primary_type) == expected
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Issue #650 — 'Other' and 'Broadcast' primary types
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
@pytest.mark.parametrize("primary_type", ["other", "Other", "OTHER"])
|
||||
def test_other_primary_type_routes_to_singles(primary_type):
|
||||
"""Issue #650: MB tags music videos and one-off web releases with
|
||||
`primary-type=Other`. They're functionally single-track releases,
|
||||
so route them to `single` (lands in Singles section). Pre-fix
|
||||
they fell through to the `album` default — placed in Albums view
|
||||
where they cluttered the LP list AND, paired with the API filter,
|
||||
were sometimes dropped from the discography entirely."""
|
||||
assert map_release_group_type(primary_type) == "single"
|
||||
|
||||
|
||||
@pytest.mark.parametrize("primary_type", ["broadcast", "Broadcast"])
|
||||
def test_broadcast_primary_type_routes_to_singles(primary_type):
|
||||
"""Broadcasts (radio sessions, one-off live single transmissions)
|
||||
are also single-track in practice. Same routing as 'Other'."""
|
||||
assert map_release_group_type(primary_type) == "single"
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Secondary-type compilation handling
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def test_compilation_secondary_type_overrides_album_primary():
|
||||
"""MB's canonical compilation pattern is `primary=Album,
|
||||
secondary=[Compilation]`. The compilation secondary check must
|
||||
fire even when the primary is Album, so 'Greatest Hits' style
|
||||
releases land in the compilation bucket."""
|
||||
assert map_release_group_type("Album", ["Compilation"]) == "compilation"
|
||||
|
||||
|
||||
def test_compilation_secondary_type_case_insensitive():
|
||||
"""Secondary-type matching tolerates case + whitespace variations
|
||||
in the provider response."""
|
||||
assert map_release_group_type("Album", ["compilation"]) == "compilation"
|
||||
assert map_release_group_type("Album", [" Compilation "]) == "compilation"
|
||||
|
||||
|
||||
def test_other_secondary_types_do_not_override_primary():
|
||||
"""Only 'compilation' is checked as a secondary-type override.
|
||||
Other MB secondary types (Live, Remix, Soundtrack, etc.) belong
|
||||
to the discography filter at the search-adapter layer, not the
|
||||
type mapper."""
|
||||
assert map_release_group_type("Album", ["Live"]) == "album"
|
||||
assert map_release_group_type("Single", ["Remix"]) == "single"
|
||||
|
||||
|
||||
def test_compilation_secondary_overrides_other_primary():
|
||||
"""An 'Other' release tagged as Compilation lands in compilation,
|
||||
not singles — secondary-type compilation is the strongest
|
||||
classification signal."""
|
||||
assert map_release_group_type("Other", ["Compilation"]) == "compilation"
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Empty / unknown / defensive
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def test_empty_primary_type_defaults_to_album():
|
||||
"""Pin: empty / None primary-type still defaults to 'album' so
|
||||
consumers that build records without complete provider data don't
|
||||
suddenly land in a different bucket."""
|
||||
assert map_release_group_type("") == "album"
|
||||
assert map_release_group_type(None) == "album"
|
||||
|
||||
|
||||
def test_unknown_primary_type_defaults_to_album():
|
||||
"""Pin: a primary-type value we don't know about defaults to
|
||||
'album'. Matches the pre-refactor fall-through so new MB
|
||||
vocabulary doesn't accidentally cause a behaviour shift."""
|
||||
assert map_release_group_type("audiobook") == "album"
|
||||
assert map_release_group_type("video") == "album"
|
||||
|
||||
|
||||
def test_secondary_types_none_is_safe():
|
||||
"""Pin: omitting secondary_types (legacy types.py call site) still
|
||||
works — None and missing-arg both treated as no-secondary-types."""
|
||||
assert map_release_group_type("Album", None) == "album"
|
||||
assert map_release_group_type("Album") == "album"
|
||||
|
||||
|
||||
def test_secondary_types_with_none_entries_skipped():
|
||||
"""Defensive: provider responses occasionally include None or empty
|
||||
string in the secondary-types list. The mapper must not crash."""
|
||||
assert map_release_group_type("Album", [None, "", "Compilation"]) == "compilation"
|
||||
assert map_release_group_type("Album", [None, ""]) == "album"
|
||||
|
||||
|
||||
def test_whitespace_in_primary_type_normalized():
|
||||
"""Defensive: a stray-whitespace primary-type still classifies."""
|
||||
assert map_release_group_type(" single ") == "single"
|
||||
assert map_release_group_type(" Other ") == "single"
|
||||
|
|
@ -3425,6 +3425,7 @@ const WHATS_NEW = {
|
|||
{ title: 'Fix popup: MusicBrainz added to the auto-search cascade', desc: 'the Fix Track Match modal used to query only Spotify → Deezer → iTunes for the auto-search, leaving MusicBrainz out of the loop entirely — even for users with MusicBrainz set as their primary metadata source. now MB is part of the cascade. when MB is your primary, it gets queried first; otherwise it sits as the last fallback. catches niche / non-mainstream / canonical-with-diacritics recordings that the commercial sources miss. Discogs is intentionally absent — Discogs has no track-level search API.' },
|
||||
{ title: 'Fix: Docker basic-search streaming silently failed under rootless Docker', desc: 'the streaming "Play" flow on the basic search page tried to create `/app/Stream` lazily at runtime, which fails silently when the container runs under rootless Docker / Podman (in-container root can\'t write to `/app`). pre-baked the directory at image build time, matching the same pattern that fixed `/app/Staging` earlier in the cycle. non-persistent — no volume needed.' },
|
||||
{ title: 'Fix: slskd-unreachable log spam during non-Soulseek downloads', desc: 'when slskd was configured but not actually running (or unreachable on its configured port), the `/api/downloads/status` polling loop fanned out to every download plugin including Soulseek, producing one `ERROR - Cannot connect to host ... [Name or service not known]` log line per poll for the entire duration of any download — visible spam even when the user wasn\'t using Soulseek at all. Connection failures now emit one WARNING with actionable context (start slskd, or clear the slskd_url if you don\'t use Soulseek) and demote subsequent failures to debug. The flag resets on the next successful slskd response so a later outage warns again.' },
|
||||
{ title: 'Fix: MusicBrainz "Other" release-groups now visible in discography', desc: 'MB tags music videos, one-off web releases, and broadcast singles with `primary-type=Other` — common pattern for Vocaloid producers, JP indie artists, and some Western indie acts. The release-group browse filter only requested `album|ep|single`, dropping every Other-typed group at the API layer. Combined with the inline type mapper defaulting unknown primary types to "album", this hid legitimate tracks from the artist detail page and left downloaded tracks orphaned (visible in track counts but not bound to any visible album / single card). Centralised the type-mapper into one shared helper (`core/metadata/release_type.py`) used by every provider\'s raw→Album projection, added `Other` and `Broadcast` handling that routes those release-groups into the Singles section, and added `other` to the MB API filter. For inabakumori, this surfaces 5 previously-invisible releases. 23 unit tests pin the mapper contract; existing 65 search-adapter tests still green.' },
|
||||
],
|
||||
'2.5.5': [
|
||||
{ date: 'May 17, 2026 — 2.5.5 release' },
|
||||
|
|
|
|||
Loading…
Reference in a new issue