From afa07690f5235f0b2746992984c58b048f6b995a Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Sat, 13 Jun 2026 16:55:33 -0700 Subject: [PATCH 01/11] Find & Add: match a Spotify 'Title - Remix' query to the base-titled library track MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit wolf's report: Spotify shows 'Calma - Remix', Find & Add searches that literal string, but the library stores the track as just 'Calma' (only the 3:58 duration marks it the remix). The literal LIKE '%calma - remix%' misses, so it fell to the OR-fuzzy fallback which floods on the common word 'remix' (20 unrelated '... remix' hits). Dropping '- Remix' (searching 'Calma') finds it instantly. Fix: search_tracks (and api_search_tracks) now retry on the BASE title — the part before Spotify's ' - ' version separator — BEFORE the OR-fuzzy flood. So 'Calma - Remix' resolves to 'Calma' (or 'Calma (Remix)') and the noise fallback is never reached when the base matches. New core.text.title_match.base_title_before_dash (splits the first spaced ' - '; leaves bare hyphens like 'Up-Tight' alone). Tests: pure helper (3) + real-DB integration reproducing the Calma case, the parenthesized-remix variant, plain-title-unaffected, and no-flood (4). 64 search/match tests green. --- core/text/title_match.py | 18 +++++ database/music_database.py | 28 +++++++- tests/test_base_title_search_fallback.py | 88 ++++++++++++++++++++++++ 3 files changed, 133 insertions(+), 1 deletion(-) create mode 100644 tests/test_base_title_search_fallback.py diff --git a/core/text/title_match.py b/core/text/title_match.py index 85e1c10b..36818fcd 100644 --- a/core/text/title_match.py +++ b/core/text/title_match.py @@ -201,9 +201,27 @@ def numeric_tokens_differ(title_a: str, title_b: str) -> bool: return _digit_tokens(title_a) != _digit_tokens(title_b) +def base_title_before_dash(title: str) -> str: + """The base title before Spotify's ' - ' version separator. + + Spotify renders versions as 'Calma - Remix' / 'Song - Radio Edit' / + 'Track - Remastered 2019'. Libraries (and the files people actually have) + very often store just the base — 'Calma' — so a literal search for + 'Calma - Remix' finds nothing and the OR-fuzzy fallback then floods on the + common qualifier word ('remix' matches every remix). This returns the base + ('Calma') for a base-title search fallback. Splits on the FIRST ' - ' (the + spaced hyphen is Spotify's separator; a bare hyphen inside a word is left + alone). Returns the title unchanged when there's no separator.""" + if not title: + return title + idx = title.find(' - ') + return title[:idx].strip() if idx > 0 else title + + __all__ = [ "titles_plausibly_same", "strip_redundant_context_qualifiers", "strip_subtitle_qualifiers", "numeric_tokens_differ", + "base_title_before_dash", ] diff --git a/database/music_database.py b/database/music_database.py index 0d06a6db..1f6078fb 100644 --- a/database/music_database.py +++ b/database/music_database.py @@ -6887,11 +6887,26 @@ class MusicDatabase: # STRATEGY 1: Try basic SQL LIKE search first (fastest) basic_results = self._search_tracks_basic(cursor, title, artist, limit, server_source, rank_artist) - + if basic_results: logger.debug(f"Basic search found {len(basic_results)} results") return basic_results + # STRATEGY 1b: Spotify renders versions as "Title - Qualifier" + # ("Calma - Remix") but libraries usually store just the base + # ("Calma"), so the literal search misses. Retry on the base title + # BEFORE the OR-fuzzy fallback (which would flood on the common + # qualifier word — every "... remix" matches "remix"). #: Calma - Remix + if title: + from core.text.title_match import base_title_before_dash + base_title = base_title_before_dash(title) + if base_title and base_title != title: + base_results = self._search_tracks_basic( + cursor, base_title, artist, limit, server_source, rank_artist) + if base_results: + logger.debug("Base-title search matched '%s' via '%s'", title, base_title) + return base_results + # STRATEGY 2: Broader fuzzy search - splits into individual words with OR matching fuzzy_results = self._search_tracks_fuzzy_fallback(cursor, title, artist, limit, server_source) if fuzzy_results: @@ -6920,6 +6935,17 @@ class MusicDatabase: if basic_rows: return [dict(r) for r in basic_rows] + # Base-title fallback for Spotify "Title - Qualifier" forms (see + # search_tracks STRATEGY 1b) before the OR-fuzzy flood. + if title: + from core.text.title_match import base_title_before_dash + base_title = base_title_before_dash(title) + if base_title and base_title != title: + base_rows = self._search_tracks_basic_rows( + cursor, base_title, artist, limit, server_source) + if base_rows: + return [dict(r) for r in base_rows] + fuzzy_rows = self._search_tracks_fuzzy_rows(cursor, title, artist, limit, server_source) return [dict(r) for r in fuzzy_rows] except Exception as e: diff --git a/tests/test_base_title_search_fallback.py b/tests/test_base_title_search_fallback.py new file mode 100644 index 00000000..f9c2b936 --- /dev/null +++ b/tests/test_base_title_search_fallback.py @@ -0,0 +1,88 @@ +"""Find & Add: Spotify "Title - Qualifier" must find the base-titled library track. + +wolf's report: Spotify shows "Calma - Remix", Find & Add searches that literal +string, the library stores the track as just "Calma" (only the duration marks it +as the remix) → the literal search misses and the OR-fuzzy fallback floods 20 +unrelated "... remix" hits. Dropping "- Remix" (searching "Calma") finds it. + +Fix: search_tracks retries on the base title (before Spotify's " - " separator) +before the OR-fuzzy flood. +""" + +from __future__ import annotations + +import pytest + +from core.text.title_match import base_title_before_dash +from database.music_database import MusicDatabase + + +# --- pure helper ----------------------------------------------------------- + +def test_base_title_before_dash_strips_spotify_version_suffix(): + assert base_title_before_dash('Calma - Remix') == 'Calma' + assert base_title_before_dash('Closer - Radio Edit') == 'Closer' + assert base_title_before_dash('Crocodile Rock - Remastered 2014') == 'Crocodile Rock' + + +def test_base_title_before_dash_leaves_plain_titles_alone(): + assert base_title_before_dash('Tom Sawyer') == 'Tom Sawyer' + assert base_title_before_dash('21st Century Schizoid Man') == '21st Century Schizoid Man' + assert base_title_before_dash('Up-Tight') == 'Up-Tight' # bare hyphen, not a separator + assert base_title_before_dash('') == '' + + +def test_base_title_before_dash_splits_first_separator_only(): + assert base_title_before_dash('A - B - C') == 'A' + + +# --- integration: the real search path ------------------------------------- + +@pytest.fixture +def db(tmp_path): + return MusicDatabase(str(tmp_path / "music.db")) + + +def _insert(db, tid, title, artist_id, artist_name): + with db._get_connection() as conn: + conn.execute("INSERT OR IGNORE INTO artists (id, name) VALUES (?, ?)", (artist_id, artist_name)) + conn.execute("INSERT OR IGNORE INTO albums (id, title, artist_id) VALUES (?, ?, ?)", + (artist_id, "Alb", artist_id)) + conn.execute( + "INSERT INTO tracks (id, album_id, artist_id, title, track_number, duration, file_path) " + "VALUES (?, ?, ?, ?, 1, 238, ?)", + (tid, artist_id, artist_id, title, f"/m/{tid}.flac"), + ) + conn.commit() + + +def test_spotify_dash_remix_finds_base_titled_track(db): + # Library stores the remix as just "Calma" (the wolf case). + _insert(db, 1, "Calma", 1, "Pedro Capó") + results = db.search_tracks(title="Calma - Remix", rank_artist="Pedro Capó") + titles = [t.title for t in results] + assert "Calma" in titles, "base-title fallback should find 'Calma' for 'Calma - Remix'" + + +def test_spotify_dash_remix_finds_parenthesized_remix(db): + # …and still matches when the library DID label it "(Remix)". + _insert(db, 1, "Calma (Remix)", 1, "Pedro Capó") + results = db.search_tracks(title="Calma - Remix", rank_artist="Pedro Capó") + assert any("Calma" in t.title for t in results) + + +def test_plain_title_unaffected_uses_basic_search(db): + _insert(db, 1, "Tom Sawyer", 1, "Rush") + results = db.search_tracks(title="Tom Sawyer") + assert [t.title for t in results] == ["Tom Sawyer"] + + +def test_dash_query_does_not_flood_when_base_matches(db): + # The base-title retry must short-circuit BEFORE the OR-fuzzy flood, so an + # unrelated "... Remix" track doesn't drown the real one. + _insert(db, 1, "Calma", 1, "Pedro Capó") + _insert(db, 2, "Some Other Song (KAIZ Remix)", 2, "Someone Else") + results = db.search_tracks(title="Calma - Remix", rank_artist="Pedro Capó") + titles = [t.title for t in results] + assert "Calma" in titles + assert "Some Other Song (KAIZ Remix)" not in titles From 2905fe0853601534d0da84abd949e96c46f87f5f Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Mon, 15 Jun 2026 19:56:53 -0700 Subject: [PATCH 02/11] #880: retry 429 mid-walk when paginating Tidal Favorite Tracks (don't truncate) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Favorites collection walker (_iter_collection_resource_ids) broke on ANY non-200 — including a transient 429. So a rate-limit mid-pagination silently truncated the collection: the log shows `status=429` then `Retrieved 98/100`, and the mirror saved 98 of a 524-track favorites list. The auto-sync cycle only "worked" because it dodged the 429. The regular-playlist paginator already retries 429; the collection walker didn't. Fix: retry the same cursor page with backoff (5/10/15/20s, 4 attempts) on 429, mirroring the playlist paginator; 401/403 still bail (+ reconnect flag), other non-200 still break. Regression tests: 429 mid-walk completes the full chain; exhausted retries return partial without hanging; 429 doesn't set reconnect. --- core/tidal_client.py | 26 ++++++++++++++ tests/test_tidal_collection_tracks.py | 49 +++++++++++++++++++++++++++ 2 files changed, 75 insertions(+) diff --git a/core/tidal_client.py b/core/tidal_client.py index a18ab04f..4b542d3d 100644 --- a/core/tidal_client.py +++ b/core/tidal_client.py @@ -1655,6 +1655,8 @@ class TidalClient: ids: List[str] = [] next_path: Optional[str] = None + consecutive_429 = 0 + MAX_PAGE_RETRIES = 4 while True: if next_path: @@ -1687,6 +1689,28 @@ class TidalClient: logger.warning(f"Tidal collection page request failed: {e}") break + # Rate limited mid-walk → retry the SAME cursor page with backoff + # rather than silently truncating the collection. Without this a 429 + # on page ~5 capped a 513-track favorites list at ~98 (issue #880); + # the regular-playlist paginator already retries 429 the same way. + if resp.status_code == 429: + consecutive_429 += 1 + if consecutive_429 <= MAX_PAGE_RETRIES: + backoff = 5.0 * consecutive_429 # 5s, 10s, 15s, 20s + logger.warning( + f"Tidal collection {expected_type} rate limited (429) — " + f"retry {consecutive_429}/{MAX_PAGE_RETRIES} in {backoff}s " + f"({len(ids)} fetched so far)" + ) + time.sleep(backoff) + continue # next_path/cursor unchanged → re-request same page + logger.error( + f"Tidal collection {expected_type} still rate limited after " + f"{MAX_PAGE_RETRIES} retries — returning {len(ids)} IDs " + f"(PARTIAL: the collection may be larger)" + ) + break + if resp.status_code != 200: # 401/403 = scope/permission issue. Token predates the # `collection.read` scope expansion or the user revoked @@ -1704,6 +1728,8 @@ class TidalClient: ) break + consecutive_429 = 0 # a good page → reset the retry budget + try: data = resp.json() except ValueError as e: diff --git a/tests/test_tidal_collection_tracks.py b/tests/test_tidal_collection_tracks.py index b821a93c..88695e99 100644 --- a/tests/test_tidal_collection_tracks.py +++ b/tests/test_tidal_collection_tracks.py @@ -252,6 +252,55 @@ class TestIterCollectionTrackIds: assert ids == [] + def test_429_mid_walk_retries_and_completes(self): + """Regression for #880: a 429 mid-pagination must RETRY the same + cursor page (with backoff), not truncate the collection. Before the + fix a transient 429 on page ~5 capped a 513-track favorites list at + ~98 (the log showed `status=429` then `Retrieved 98/100`).""" + client = _make_authed_client() + # page1 (200) → 429 (transient) → page2 (200, end of chain) + responses = iter([ + _FakeResp(200, _PAGE_ONE), + _FakeResp(429, text=""), # rate limited — retry, don't truncate + _FakeResp(200, _PAGE_TWO), + ]) + with patch.object(client, '_ensure_valid_token', return_value=True), \ + patch('core.tidal_client.requests.get', side_effect=lambda *a, **kw: next(responses)), \ + patch('core.tidal_client.time.sleep'): + ids = client._iter_collection_track_ids() + + assert ids == ['1001', '1002', '1003', '1004', '1005'] # full chain, nothing dropped + + def test_429_does_not_set_reconnect_flag(self): + """A rate-limit is transient, NOT a scope problem — must not tell the + user to reconnect.""" + client = _make_authed_client() + responses = iter([_FakeResp(200, _PAGE_ONE), _FakeResp(429, text=""), _FakeResp(200, _PAGE_TWO)]) + with patch.object(client, '_ensure_valid_token', return_value=True), \ + patch('core.tidal_client.requests.get', side_effect=lambda *a, **kw: next(responses)), \ + patch('core.tidal_client.time.sleep'): + client._iter_collection_track_ids() + + assert client.collection_needs_reconnect() is False + + def test_429_exhausts_retries_returns_partial(self): + """If the 429s never clear, give up after the retry budget and return + what we have (PARTIAL) rather than looping forever.""" + client = _make_authed_client() + + def gen(): + yield _FakeResp(200, _PAGE_ONE) + while True: + yield _FakeResp(429, text="") + + responses = gen() + with patch.object(client, '_ensure_valid_token', return_value=True), \ + patch('core.tidal_client.requests.get', side_effect=lambda *a, **kw: next(responses)), \ + patch('core.tidal_client.time.sleep'): + ids = client._iter_collection_track_ids() + + assert ids == ['1001', '1002', '1003'] # page 1 survives; no hang + # --------------------------------------------------------------------------- # get_collection_tracks_count From 02d6af29edc35a8826e88906970028b6af222cb8 Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Mon, 15 Jun 2026 20:09:53 -0700 Subject: [PATCH 03/11] #879: a failed settings load must never overwrite the saved config Reported by @Lysticity: opening Settings reset the whole config to defaults. The chain: GET /api/settings 500s (their env: ConfigManager missing redacted_config) -> loadSettingsData() called response.json() WITHOUT checking response.ok, so the error body {"error": ...} was treated as settings -> every field populated as `settings.x?.y || ''` blanked to defaults -> autosave then wrote those defaults over the real config. Fix (settings.js): bail BEFORE touching any field when the response isn't ok / is an error body, set window._settingsLoadFailed, and guard BOTH save paths (debouncedAutoSaveSettings + saveSettings) on it. The flag clears on the next successful load. So any load failure (500, lock, network) now leaves the saved config untouched instead of wiping it. The redacted_config method exists in all 2.7.x source + on dev (their 500 looks like a stale/mismatched build), but the UI must not destroy config on ANY failed load. Regression test pins redacted_config stays a callable method on the class (its removal is exactly what 500s the endpoint). --- tests/test_settings_redaction.py | 17 +++++++++++++++++ webui/static/settings.js | 30 +++++++++++++++++++++++++++++- 2 files changed, 46 insertions(+), 1 deletion(-) diff --git a/tests/test_settings_redaction.py b/tests/test_settings_redaction.py index 3aea7a2a..a27e4d25 100644 --- a/tests/test_settings_redaction.py +++ b/tests/test_settings_redaction.py @@ -24,6 +24,23 @@ def _cm(config_data): return cm +# ── #879: the GET /api/settings handler calls config_manager.redacted_config(). +# If that method is ever renamed/removed the endpoint 500s, the web UI treats the +# error body as settings, blanks the form to defaults, and autosaves over the +# user's real config. Pin the method's presence on the class so that can't ship. + +def test_redacted_config_is_a_method_on_the_class(): + assert callable(getattr(ConfigManager, 'redacted_config', None)), ( + "GET /api/settings depends on ConfigManager.redacted_config(); removing or " + "renaming it 500s the settings endpoint and the UI then wipes the config (#879)") + + +def test_redacted_config_callable_on_instance_returns_dict(): + cm = _cm({'spotify': {'client_secret': 'REAL'}}) + out = cm.redacted_config() + assert isinstance(out, dict) and out.get('spotify', {}).get('client_secret') == S + + # ── redacted_config: secrets out, everything else intact ──────────────────── def test_configured_secrets_are_masked(): diff --git a/webui/static/settings.js b/webui/static/settings.js index 1921839b..d00c79d9 100644 --- a/webui/static/settings.js +++ b/webui/static/settings.js @@ -45,6 +45,9 @@ function debouncedAutoSaveSettings() { // fields on load — those aren't user edits and must not trigger a full // save (which re-initializes every backend service client). if (window._suppressSettingsAutoSave) return; + // #879: never auto-save while the last settings load failed — the form is + // showing defaults, not the real config, so saving would wipe it. + if (window._settingsLoadFailed) return; // #827: the Logs tab has no savable settings — its live-viewer controls // (source picker, filters, auto-scroll) were tripping the auto-save and // flooding app.log with "Settings saved" lines, drowning out the logs the @@ -976,6 +979,18 @@ async function loadSettingsData() { const response = await fetch(API.settings); const settings = await response.json(); + // #879: a failed GET /api/settings returns an error body (e.g. {"error": + // "..."} on a 500), NOT real settings. Populating from it blanks every + // field to its default ('settings.spotify?.x || ""'), and the next + // (auto)save then overwrites the user's real config. Abort BEFORE + // touching any field and flag it so saves stay blocked until a good load. + if (!response.ok || !settings || typeof settings !== 'object' || settings.error) { + window._settingsLoadFailed = true; + throw new Error('settings load failed (HTTP ' + response.status + '): ' + + ((settings && settings.error) || 'unexpected response')); + } + window._settingsLoadFailed = false; // good load → saving is safe again + // Populate Spotify settings document.getElementById('spotify-client-id').value = settings.spotify?.client_id || ''; document.getElementById('spotify-client-secret').value = settings.spotify?.client_secret || ''; @@ -1461,7 +1476,10 @@ async function loadSettingsData() { } catch (error) { console.error('Error loading settings:', error); - showToast('Failed to load settings', 'error'); + // #879: any load failure → block saves so a blank/partial form can't be + // written over the real config. Cleared on the next successful load. + window._settingsLoadFailed = true; + showToast('Failed to load settings — reload the page before saving (your saved config is untouched)', 'error'); } } @@ -2861,6 +2879,16 @@ function _getTagConfig(path) { } async function saveSettings(quiet = false) { + // #879: refuse to save if the settings never loaded successfully — the form + // is showing defaults, not the user's real config, so saving would wipe it. + // Cleared automatically on the next successful load (reload the page). + if (window._settingsLoadFailed) { + if (!quiet && typeof showToast === 'function') { + showToast("Settings didn't load — reload the page before saving (your config is untouched)", 'error'); + } + return; + } + // Validate file organization templates before saving const validationErrors = validateFileOrganizationTemplates(); if (validationErrors.length > 0) { From f2f0f5d849f0c5f6588ade1f3c1b07915bece3cc Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Mon, 15 Jun 2026 21:16:18 -0700 Subject: [PATCH 04/11] Sidebar UI: frosted-glass header blur, centered nav badges, admin cleanup MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - .sidebar-header: real frosted-glass blur of content scrolling behind it — made the background translucent (was an opaque base layer), added backdrop-filter blur, and raised the header above the nav (z-index) so nav items actually sit in its backdrop. - .dl-nav-badge: vertically centered on the right (top:50% + translateY) instead of pinned to the top-right corner. - Removed border-top-right-radius from .sidebar and .sidebar-header (square top). - Hide the "My Accounts" + "My Settings" header buttons for admin profiles — both are inert for admin (every service is "Managed in Settings", and My Settings is an empty pointer note); kept for non-admins who get real UI. --- webui/static/init.js | 9 +++++++++ webui/static/style.css | 31 +++++++++++++++++++++---------- 2 files changed, 30 insertions(+), 10 deletions(-) diff --git a/webui/static/init.js b/webui/static/init.js index d339cb18..a8a11b98 100644 --- a/webui/static/init.js +++ b/webui/static/init.js @@ -1116,6 +1116,15 @@ function updateProfileIndicator() { const statusSection = document.querySelector('.status-section--clickable'); if (statusSection) statusSection.classList.toggle('status-section--locked', !currentProfile.is_admin); + // My Accounts (per-profile streaming OAuth) and My Settings (per-profile + // server library) are inert for admin — admin uses the global app account + // for every service and the full Settings page. Hide both for admin; keep + // them for non-admins, who actually get a connect/library UI. + const myAccountsBtn = document.getElementById('my-accounts-btn'); + const personalSettingsBtn = document.getElementById('personal-settings-btn'); + if (myAccountsBtn) myAccountsBtn.style.display = currentProfile.is_admin ? 'none' : ''; + if (personalSettingsBtn) personalSettingsBtn.style.display = currentProfile.is_admin ? 'none' : ''; + indicator.onclick = async () => { const res = await fetch('/api/profiles'); const data = await res.json(); diff --git a/webui/static/style.css b/webui/static/style.css index d3a2d09c..608ee724 100644 --- a/webui/static/style.css +++ b/webui/static/style.css @@ -80,7 +80,6 @@ body { /* Soft translucent borders */ border-right: 1px solid rgba(255, 255, 255, 0.08); border-top: 1px solid rgba(255, 255, 255, 0.12); - border-top-right-radius: 24px; border-bottom-right-radius: 24px; /* Soft floating shadow with inner glow */ @@ -291,17 +290,16 @@ body.reduce-effects .sidebar::after { .sidebar-header { min-height: 115px; - /* Opaque base layered under the accent gradient so nav items scrolling - past the sticky header don't bleed through the translucent stops. */ + /* Translucent so the backdrop-filter blur below is visible — the dark tint + keeps the header readable while nav items scrolling behind it read as a + soft frosted blur instead of a sharp bleed-through. */ background: linear-gradient(180deg, - rgba(var(--accent-rgb), 0.14) 0%, - rgba(var(--accent-rgb), 0.08) 30%, - rgba(var(--accent-rgb), 0.03) 70%, - rgba(18, 18, 18, 1) 100%), - rgb(18, 18, 18); + rgba(var(--accent-rgb), 0.16) 0%, + rgba(var(--accent-rgb), 0.10) 30%, + rgba(24, 24, 24, 0.62) 70%, + rgba(18, 18, 18, 0.72) 100%); border-bottom: 1px solid rgba(255, 255, 255, 0.08); - border-top-right-radius: 20px; padding: 20px 24px; display: flex; flex-direction: column; @@ -309,9 +307,21 @@ body.reduce-effects .sidebar::after { gap: 8px; position: sticky; top: 0; + /* Above the nav (the sidebar gives its children z-index:1) so nav items + scrolling up sit BEHIND the header — i.e. in its backdrop, where the + blur below can actually act on them. Without this they paint in front + and backdrop-filter has nothing to blur. */ + z-index: 2; overflow: hidden; flex-shrink: 0; + /* Intense frosted-glass blur: nav items scrolling up behind the translucent + accent-gradient top now read as a soft blur instead of bleeding through + sharply. (Only visible where the background is translucent — the opaque + base toward the bottom still stops any bleed.) */ + backdrop-filter: blur(28px) saturate(1.3); + -webkit-backdrop-filter: blur(28px) saturate(1.3); + /* Subtle inner glow */ box-shadow: inset 0 1px 0 rgba(255, 255, 255, 0.1); } @@ -60149,8 +60159,9 @@ body.reduce-effects #page-particles-canvas { .dl-nav-badge { position: absolute; - top: 4px; + top: 50%; right: 8px; + transform: translateY(-50%); background: rgb(var(--accent-rgb)); color: #fff; font-size: 0.6rem; From e7814e0acf048851a2cd8516cdff1138e6561671 Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Mon, 15 Jun 2026 21:30:55 -0700 Subject: [PATCH 05/11] #877: Download Discography filters mirror Artist Detail (fix dead EPs + add Live/Comp/Featured) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Download Discography modal exposed only Albums/EPs/Singles, its EPs toggle did nothing, and Live/Compilations/Featured were missing — so you couldn't fine-filter a bulk download the way Artist Detail lets you browse. Root cause: the modal's endpoint (/api/artist//discography) used the base get_artist_discography, which lumps EPs into singles, and the modal only read {albums, singles} — so the EPs bucket was always empty (dead toggle). It also had no content-type (Live/Compilation/Featured) classification at all. - Backend: the endpoint now uses get_artist_detail_discography — the SAME split Artist Detail uses — and returns a separate `eps` list. - Frontend: read `eps`; tag each card with data-is-live/compilation/featured via a new shared _classifyReleaseContent() (also adopted by the Artist Detail cards so the two can't drift); add Live/Compilations/Featured filter buttons; combined category+content filtering. The download payload is built from VISIBLE checked cards, so every toggle now actually changes what downloads. - Regression test: get_artist_detail_discography splits an EP into the eps bucket. --- tests/metadata/test_metadata_discography.py | 23 +++++++ web_server.py | 8 ++- webui/static/library.js | 68 +++++++++++++++------ 3 files changed, 81 insertions(+), 18 deletions(-) diff --git a/tests/metadata/test_metadata_discography.py b/tests/metadata/test_metadata_discography.py index 65ac0624..a5109dbb 100644 --- a/tests/metadata/test_metadata_discography.py +++ b/tests/metadata/test_metadata_discography.py @@ -731,3 +731,26 @@ def test_get_artist_discography_prefers_source_specific_artist_ids(monkeypatch): ) ] assert deezer.album_calls == [] + + +def test_get_artist_detail_discography_splits_eps_from_singles(monkeypatch): + """#877: get_artist_detail_discography must put EPs in their OWN bucket (not + lumped into singles). The Download Discography modal now reads this split, so + its EPs filter has cards to act on and stays in sync with Artist Detail.""" + spotify = _FakeSourceClient( + album_results=[ + _album("a1", "An Album", "2024-01-01", album_type="album"), + _album("e1", "An EP", "2024-02-01", album_type="ep"), + _album("s1", "A Single", "2024-03-01", album_type="single"), + ] + ) + clients = {"deezer": _FakeSourceClient(), "spotify": spotify, "itunes": _FakeSourceClient()} + monkeypatch.setattr(metadata_registry, "get_primary_source", lambda spotify_client_factory=None: "deezer") + monkeypatch.setattr(metadata_registry, "get_source_priority", lambda primary: [primary, "spotify", "itunes"]) + monkeypatch.setattr(metadata_registry, "get_client_for_source", lambda source, **kwargs: clients.get(source)) + + result = metadata_discography.get_artist_detail_discography("artist-1", "Artist One", MetadataLookupOptions()) + + assert [r["id"] for r in result["albums"]] == ["a1"] + assert [r["id"] for r in result["eps"]] == ["e1"] + assert [r["id"] for r in result["singles"]] == ["s1"] diff --git a/web_server.py b/web_server.py index fd53c71a..e53a2711 100644 --- a/web_server.py +++ b/web_server.py @@ -9114,7 +9114,11 @@ def get_artist_discography(artist_id): effective_override_source = 'spotify' from core.metadata.lookup import MetadataLookupOptions - from core.metadata_service import get_artist_discography as _get_artist_discography + # #877: use the artist-DETAIL discography so the Download Discography modal + # gets the SAME release-type split (albums / eps / singles) the Artist + # Detail view shows — EPs were being lumped into singles before, leaving + # the modal's EPs toggle dead. + from core.metadata.discography import get_artist_detail_discography as _get_artist_discography # Server-side per-source ID resolution. Look up the library row # by ANY of the IDs the frontend might send: library DB id, @@ -9192,6 +9196,7 @@ def get_artist_discography(artist_id): ) album_list = discography['albums'] + eps_list = discography.get('eps', []) singles_list = discography['singles'] active_source = discography['source'] source_priority = discography['source_priority'] @@ -9303,6 +9308,7 @@ def get_artist_discography(artist_id): return jsonify({ "albums": album_list, + "eps": eps_list, "singles": singles_list, "source": active_source or (source_priority[0] if source_priority else "unknown"), "artist_info": artist_info, diff --git a/webui/static/library.js b/webui/static/library.js index cd88e264..48f347f8 100644 --- a/webui/static/library.js +++ b/webui/static/library.js @@ -1891,16 +1891,12 @@ function createReleaseCard(release) { // Store mutable reference so stream updates propagate to click handler card._releaseData = release; - // Tag card for content-type filtering - const livePattern = /\b(live)\b|\(live[^)]*\)|\[live[^]]*\]/i; - const compilationPattern = /\b(greatest hits|best of|collection|anthology|essential)\b/i; - const featuredPattern = /\(?\bfeat\.?\s|\bft\.?\s|\bfeaturing\b/i; - const isLive = livePattern.test(release.title || '') || (release.album_type === 'compilation' && livePattern.test(release.title || '')); - const isCompilation = (release.album_type === 'compilation') || compilationPattern.test(release.title || ''); - const isFeatured = featuredPattern.test(release.title || ''); - card.setAttribute("data-is-live", isLive ? "true" : "false"); - card.setAttribute("data-is-compilation", isCompilation ? "true" : "false"); - card.setAttribute("data-is-featured", isFeatured ? "true" : "false"); + // Tag card for content-type filtering (shared classifier — #877, so Artist + // Detail and the Download Discography modal never drift apart). + const cc = _classifyReleaseContent(release); + card.setAttribute("data-is-live", cc.isLive ? "true" : "false"); + card.setAttribute("data-is-compilation", cc.isCompilation ? "true" : "false"); + card.setAttribute("data-is-featured", cc.isFeatured ? "true" : "false"); // Background image — use data-bg-src for IntersectionObserver lazy loading // (observeLazyBackgrounds is called by the caller after appending the grid). @@ -2518,8 +2514,8 @@ async function openDiscographyModal() { const data = await res.json(); if (!data.error) { - discography = { albums: data.albums || [], singles: data.singles || [] }; - if (discography.albums.length > 0 || discography.singles.length > 0) { + discography = { albums: data.albums || [], eps: data.eps || [], singles: data.singles || [] }; + if (discography.albums.length > 0 || discography.eps.length > 0 || discography.singles.length > 0) { artistsPageState.artistDiscography = discography; artistsPageState.sourceOverride = data.source || artistsPageState.sourceOverride || null; // Use metadata source ID for the modal (needed for download API calls) @@ -2569,6 +2565,9 @@ async function openDiscographyModal() { + + +
@@ -2605,21 +2604,36 @@ async function openDiscographyModal() { function _esc(s) { const d = document.createElement('div'); d.textContent = s; return d.innerHTML; } +// #877: single source of truth for content-type classification, shared by the +// Artist Detail cards and the Download Discography modal so they can't drift. +function _classifyReleaseContent(release) { + const t = (release && (release.title || release.name)) || ''; + const livePattern = /\b(live)\b|\(live[^)]*\)|\[live[^\]]*\]/i; + const compilationPattern = /\b(greatest hits|best of|collection|anthology|essential)\b/i; + const featuredPattern = /\(?\bfeat\.?\s|\bft\.?\s|\bfeaturing\b/i; + return { + isLive: livePattern.test(t), + isCompilation: (release && release.album_type === 'compilation') || compilationPattern.test(t), + isFeatured: featuredPattern.test(t), + }; +} + function _renderDiscogCard(release, index, completionData) { const comp = completionData?.albums?.find(c => c.id === release.id) || completionData?.singles?.find(c => c.id === release.id); const status = comp?.status || 'unknown'; const isOwned = status === 'completed'; const isPartial = status === 'partial' || status === 'nearly_complete'; const year = release.release_date ? release.release_date.substring(0, 4) : ''; - const tracks = release.total_tracks || 0; + const tracks = release.total_tracks || release.track_count || 0; const img = release.image_url || ''; + const cc = _classifyReleaseContent(release); const checked = !isOwned; const statusClass = isOwned ? 'owned' : isPartial ? 'partial' : ''; const statusIcon = isOwned ? '✓' : isPartial ? '◐' : ''; const albumName = release.name || release.title || ''; return ` -