From 5eae24b8bb68ffbaae02059825821a8d7db32e6e Mon Sep 17 00:00:00 2001 From: Broque Thomas <26755000+Nezreka@users.noreply.github.com> Date: Tue, 12 May 2026 21:09:16 -0700 Subject: [PATCH] Fix $albumtype defaulting to album for non-Spotify sources MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - legacy duck-typed builder only checked the `album_type` key; deezer uses `record_type`, tidal uses `type` (uppercase), some flattened musicbrainz shapes use `primary-type` — all defaulted to album, so EPs and singles ended up filed under Album/ in user templates that reference $albumtype - widen lookup to album_type / record_type / type / primary-type and route through new pure `_normalize_album_type` helper that case-folds + validates against the canonical token set (album / single / ep / compilation), unknown → album - typed-converter path (spotify / deezer / itunes / discogs / mb / hydrabase / qobuz) unchanged — those were already correct Discord report (CAL). --- core/metadata/album_tracks.py | 34 ++++- .../metadata/test_album_type_normalization.py | 132 ++++++++++++++++++ webui/static/helper.js | 5 + 3 files changed, 170 insertions(+), 1 deletion(-) create mode 100644 tests/metadata/test_album_type_normalization.py diff --git a/core/metadata/album_tracks.py b/core/metadata/album_tracks.py index 09ddb492..266134a8 100644 --- a/core/metadata/album_tracks.py +++ b/core/metadata/album_tracks.py @@ -267,6 +267,33 @@ def _build_album_info_typed(album_data: Dict[str, Any], album_id: str, return ctx +_ALBUM_TYPE_CANONICAL = {'album', 'single', 'ep', 'compilation'} + + +def _normalize_album_type(value: Any, default: str = 'album') -> str: + """Map a raw album-type value from any source to the canonical + lowercase token the path templates use (``album`` / ``single`` / + ``ep`` / ``compilation``). + + Different metadata sources expose the album type under different + keys AND with different casing — Tidal returns ``ALBUM``, MB + returns ``Album``, Deezer's ``record_type`` is already lowercase. + Without this normalization the legacy duck-typed builder accepted + only Spotify-shaped lowercase ``album_type``, so every other + source's discography ended up filed under ``Album/`` regardless + of actual type (Discord report, CAL, 2026-05-12). + + Unknown values fall back to ``default`` rather than passing + through — keeps stray strings out of the path template. + """ + if value is None: + return default + v = str(value).strip().lower() + if not v: + return default + return v if v in _ALBUM_TYPE_CANONICAL else default + + def _build_album_info_legacy(album_data: Any, album_id: str, album_name: str, artist_name: str) -> Dict[str, Any]: """Original duck-typed extraction. Kept as the fallback when the @@ -310,7 +337,12 @@ def _build_album_info_legacy(album_data: Any, album_id: str, 'image_url': image_url, 'images': images, 'release_date': _extract_lookup_value(album_data, 'release_date', default='') or '', - 'album_type': _extract_lookup_value(album_data, 'album_type', default='album') or 'album', + 'album_type': _normalize_album_type( + _extract_lookup_value( + album_data, 'album_type', 'record_type', 'type', 'primary-type', + default=None, + ) + ), 'total_tracks': _extract_lookup_value(album_data, 'total_tracks', 'track_count', default=0) or 0, } diff --git a/tests/metadata/test_album_type_normalization.py b/tests/metadata/test_album_type_normalization.py new file mode 100644 index 00000000..c628ef53 --- /dev/null +++ b/tests/metadata/test_album_type_normalization.py @@ -0,0 +1,132 @@ +"""Pin `_normalize_album_type` + the legacy fallback's multi-key +lookup for `album_type`. + +Discord report (CAL, 2026-05-12): downloading an artist's discography +with `$albumtype` in the path template put every release under +`Album/` regardless of actual type — EPs, singles, all dumped into +`Album/`. Trace: `_build_album_info_legacy` only checked the +`album_type` key. Different sources expose the type under different +names (Deezer `record_type`, Tidal/MB `type` / `primary-type`, often +uppercase). Spotify-shaped lowercase `album_type` was the only path +that worked; everything else defaulted to `album`. + +Fix widens the legacy lookup to check `album_type`, `record_type`, +`type`, `primary-type` and routes the value through +`_normalize_album_type` which lowercases, validates against the +canonical token set, and falls back to `album` for unknowns. + +These tests pin both the normalizer (pure helper) and the wired +behavior in `_build_album_info_legacy` (smoke). +""" + +from __future__ import annotations + +import pytest + +from core.metadata.album_tracks import ( + _build_album_info_legacy, + _normalize_album_type, +) + + +# --------------------------------------------------------------------------- +# Pure helper +# --------------------------------------------------------------------------- + + +class TestNormalizeAlbumType: + @pytest.mark.parametrize('raw', ['album', 'ALBUM', 'Album', ' Album ']) + def test_album_variants_normalize_to_album(self, raw): + assert _normalize_album_type(raw) == 'album' + + @pytest.mark.parametrize('raw', ['single', 'SINGLE', 'Single']) + def test_single_variants_normalize_to_single(self, raw): + assert _normalize_album_type(raw) == 'single' + + @pytest.mark.parametrize('raw', ['ep', 'EP', 'Ep']) + def test_ep_variants_normalize_to_ep(self, raw): + assert _normalize_album_type(raw) == 'ep' + + def test_compilation_preserved(self): + """Spotify exposes 'compilation' as a distinct type. Preserve + it so users with a `$albumtype` template get a separate folder + instead of compilations getting demoted into `album/`.""" + assert _normalize_album_type('compilation') == 'compilation' + + @pytest.mark.parametrize('raw', [None, '', ' ']) + def test_empty_inputs_return_default(self, raw): + assert _normalize_album_type(raw) == 'album' + + def test_unknown_value_returns_default(self): + """Stray strings (e.g. 'mixtape', 'box-set') don't pass through + — they'd produce nonsense folder names. Default to album.""" + assert _normalize_album_type('mixtape') == 'album' + assert _normalize_album_type('box-set') == 'album' + + def test_custom_default_honored(self): + assert _normalize_album_type('weird', default='single') == 'single' + assert _normalize_album_type(None, default='ep') == 'ep' + + def test_non_string_value_handled(self): + """Defensive: source might hand us an int / bool / dict. + Should not crash.""" + assert _normalize_album_type(0) == 'album' + assert _normalize_album_type({'name': 'foo'}) == 'album' + + +# --------------------------------------------------------------------------- +# Legacy builder — alt-key support +# --------------------------------------------------------------------------- + + +class TestLegacyBuilderAlbumTypeAltKeys: + """The bug was sources whose `album_type` lives under an alt key. + Pin each known shape produces the correct canonical token.""" + + def _build(self, album_data): + return _build_album_info_legacy( + album_data, album_id='id1', album_name='Test', artist_name='Artist', + ) + + def test_spotify_shape_album_type_key(self): + info = self._build({'album_type': 'single'}) + assert info['album_type'] == 'single' + + def test_deezer_shape_record_type_key(self): + """Deezer's API returns `record_type` not `album_type`. CAL's + EPs returned `record_type='ep'` but the legacy reader missed it + and defaulted to album.""" + info = self._build({'record_type': 'ep'}) + assert info['album_type'] == 'ep' + + def test_tidal_shape_type_key_uppercase(self): + """Tidal returns `type='ALBUM'/'EP'/'SINGLE'`. Uppercase + alt + key = double-miss before the fix.""" + info = self._build({'type': 'EP'}) + assert info['album_type'] == 'ep' + + def test_musicbrainz_shape_primary_type_key(self): + """Some flattened MB shapes carry `primary-type` at the top + level (typed path handles release-group nesting; legacy hits + the flattened cases).""" + info = self._build({'primary-type': 'Single'}) + assert info['album_type'] == 'single' + + def test_album_type_wins_when_multiple_keys_present(self): + """When both `album_type` AND `record_type` exist, prefer the + Spotify-canonical key. `_extract_lookup_value` checks left to + right — pin that ordering.""" + info = self._build({'album_type': 'album', 'record_type': 'ep'}) + assert info['album_type'] == 'album' + + def test_no_type_key_defaults_to_album(self): + """Source response with no type field at all → defaults to + `album` (legacy behavior preserved for genuinely-missing data).""" + info = self._build({'name': 'Some Album'}) + assert info['album_type'] == 'album' + + def test_unknown_type_value_defaults_to_album(self): + """`type='Mixtape'` → not in canonical set → default. Prevents + a stray value from poisoning the path template.""" + info = self._build({'type': 'Mixtape'}) + assert info['album_type'] == 'album' diff --git a/webui/static/helper.js b/webui/static/helper.js index 6728c900..75075e09 100644 --- a/webui/static/helper.js +++ b/webui/static/helper.js @@ -3413,6 +3413,11 @@ function closeHelperSearch() { // projects that span multiple commits before shipping. Strip the flag at // release time and add a real `date:` line at the top of the version block. const WHATS_NEW = { + '2.5.2': [ + // --- post-release patch work on the 2.5.2 line — entries hidden by _getLatestWhatsNewVersion until the build version bumps --- + { date: 'Unreleased — 2.5.2 patch work' }, + { title: '$albumtype Folder Template Now Splits EPs / Singles For Non-Spotify Sources', desc: 'discord report (cal): downloading an artist\'s discography with `$albumtype` in the path template put every release under `Album/` regardless of actual type — eps, singles, all dumped into the album folder. trace: the legacy duck-typed album-info builder at `core/metadata/album_tracks.py:_build_album_info_legacy` only checked the `album_type` key. spotify uses `album_type` (lowercase) so spotify discographies worked. but deezer\'s api uses `record_type`, tidal uses `type` (uppercase ALBUM/EP/SINGLE), and some flattened musicbrainz shapes use `primary-type` — none of those matched, all defaulted to `album`. fix: widen the legacy lookup to check `album_type` / `record_type` / `type` / `primary-type` and route the value through a new pure `_normalize_album_type` helper that lowercases + validates against the canonical token set (`album` / `single` / `ep` / `compilation`) and falls back to `album` for unknowns. typed-converter path for spotify / deezer / itunes / discogs / musicbrainz / hydrabase / qobuz unchanged — they were already correct. tidal users were the main offender (no typed converter for dict-shaped tidal data). 24 new tests pin: case-insensitive normalization for each canonical type, compilation preserved (spotify supports it), unknown values default to album, defensive against none / empty / non-string inputs, multi-key precedence (`album_type` wins over `record_type`), each known source shape produces correct token.', page: 'tools' }, + ], '2.5.1': [ // --- May 12, 2026 — 2.5.1 release --- { date: 'May 12, 2026 — 2.5.1 release' },