From ad657f02a88c49725268781d0f5a00ed8c20a4c4 Mon Sep 17 00:00:00 2001 From: ramonskie Date: Thu, 25 Jun 2026 13:43:24 +0200 Subject: [PATCH 01/49] Fix playlist sync status labels --- tests/test_playlist_sync_status.py | 65 ++++++++++++++++++++++++++++++ web_server.py | 59 ++++++++++++++++++++++++--- webui/static/sync-spotify.js | 2 +- 3 files changed, 119 insertions(+), 7 deletions(-) create mode 100644 tests/test_playlist_sync_status.py diff --git a/tests/test_playlist_sync_status.py b/tests/test_playlist_sync_status.py new file mode 100644 index 00000000..317b8a6f --- /dev/null +++ b/tests/test_playlist_sync_status.py @@ -0,0 +1,65 @@ +from datetime import datetime, timedelta + +from web_server import ( + _format_playlist_sync_status, + _resolve_spotify_playlist_sync_status, +) + + +class _StubDatabase: + def __init__(self, mirrored): + self._mirrored = mirrored + + def get_mirrored_playlist_by_source(self, source, source_playlist_id, profile_id): + assert source == 'spotify' + assert source_playlist_id == 'spotify-playlist-1' + assert profile_id == 7 + return self._mirrored + + +def _status(minutes_ago=0, **overrides): + timestamp = (datetime(2026, 6, 25, 12, 0, 0) - timedelta(minutes=minutes_ago)).isoformat() + return {'last_synced': timestamp, **overrides} + + +def test_resolve_spotify_playlist_sync_status_uses_mirrored_auto_sync_status(): + sync_statuses = { + 'auto_mirror_42': _status(matched_tracks=12), + } + + status = _resolve_spotify_playlist_sync_status( + 'spotify-playlist-1', + sync_statuses, + database=_StubDatabase({'id': 42}), + profile_id=7, + ) + + assert status['matched_tracks'] == 12 + + +def test_resolve_spotify_playlist_sync_status_prefers_newest_status(): + sync_statuses = { + 'spotify-playlist-1': _status(minutes_ago=20, matched_tracks=1), + 'auto_mirror_42': _status(minutes_ago=5, matched_tracks=2), + } + + status = _resolve_spotify_playlist_sync_status( + 'spotify-playlist-1', + sync_statuses, + database=_StubDatabase({'id': 42}), + profile_id=7, + ) + + assert status['matched_tracks'] == 2 + + +def test_format_playlist_sync_status_treats_missing_snapshot_as_synced(): + status = _status() + + assert _format_playlist_sync_status(status, 'current-snapshot') == 'Synced: Jun 25, 12:00' + + +def test_format_playlist_sync_status_marks_snapshot_mismatch_as_last_sync(): + status = _status(snapshot_id='old-snapshot') + + assert _format_playlist_sync_status(status, 'current-snapshot') == 'Last Sync: Jun 25, 12:00' diff --git a/web_server.py b/web_server.py index 4e330c92..63cab778 100644 --- a/web_server.py +++ b/web_server.py @@ -20765,6 +20765,56 @@ def _save_sync_status_file(sync_statuses): except Exception as e: logger.error(f"Error saving sync status: {e}") +def _sync_status_timestamp(status_info): + """Return comparable timestamp for a persisted sync-status record.""" + if not status_info or 'last_synced' not in status_info: + return None + try: + return datetime.fromisoformat(status_info['last_synced']) + except (TypeError, ValueError): + return None + +def _latest_sync_status(*status_infos): + """Pick the newest non-empty sync-status record.""" + candidates = [s for s in status_infos if s] + if not candidates: + return {} + return max(candidates, key=lambda s: _sync_status_timestamp(s) or datetime.min) + +def _mirrored_spotify_sync_status(playlist_id, sync_statuses, *, database=None, profile_id=None): + """Return auto-sync status for a Spotify playlist mirrored into SoulSync.""" + try: + db = database or get_database() + profile = profile_id if profile_id is not None else get_current_profile_id() + mirrored = db.get_mirrored_playlist_by_source('spotify', str(playlist_id), profile) + if not mirrored: + return {} + return sync_statuses.get(f"auto_mirror_{mirrored.get('id')}", {}) + except Exception as e: + logger.debug("Spotify mirrored sync-status lookup failed for %s: %s", playlist_id, e) + return {} + +def _resolve_spotify_playlist_sync_status(playlist_id, sync_statuses, *, database=None, profile_id=None): + """Resolve direct or mirrored sync status for a Spotify playlist card.""" + direct_status = sync_statuses.get(playlist_id, {}) + mirrored_status = _mirrored_spotify_sync_status( + playlist_id, + sync_statuses, + database=database, + profile_id=profile_id, + ) + return _latest_sync_status(direct_status, mirrored_status) + +def _format_playlist_sync_status(status_info, playlist_snapshot): + """Build user-facing sync-status text from persisted status + snapshot.""" + if 'last_synced' not in status_info: + return "Never Synced" + last_sync_time = datetime.fromisoformat(status_info['last_synced']).strftime('%b %d, %H:%M') + stored_snapshot = status_info.get('snapshot_id') + if stored_snapshot and playlist_snapshot and playlist_snapshot != stored_snapshot: + return f"Last Sync: {last_sync_time}" + return f"Synced: {last_sync_time}" + def _update_and_save_sync_status(playlist_id, playlist_name, playlist_owner, snapshot_id, **kwargs): """Updates the sync status for a given playlist and saves to file (same logic as GUI).""" try: @@ -20807,16 +20857,14 @@ def get_spotify_playlists(): # Add regular playlists first for p in playlists: - status_info = sync_statuses.get(p.id, {}) - sync_status = "Never Synced" + status_info = _resolve_spotify_playlist_sync_status(p.id, sync_statuses) # Handle snapshot_id safely - may not exist in core Playlist class playlist_snapshot = getattr(p, 'snapshot_id', '') + sync_status = _format_playlist_sync_status(status_info, playlist_snapshot) if 'last_synced' in status_info: stored_snapshot = status_info.get('snapshot_id') - last_sync_time = datetime.fromisoformat(status_info['last_synced']).strftime('%b %d, %H:%M') - if playlist_snapshot != stored_snapshot: - sync_status = f"Last Sync: {last_sync_time}" + if stored_snapshot and playlist_snapshot and playlist_snapshot != stored_snapshot: logger.info( "Playlist sync status: name=%s id=%s snapshot=%r stored_snapshot=%r result=Needs Sync display=%s", p.name, @@ -20826,7 +20874,6 @@ def get_spotify_playlists(): sync_status, ) else: - sync_status = f"Synced: {last_sync_time}" logger.info( "Playlist sync status: name=%s id=%s snapshot=%r stored_snapshot=%r result=Synced display=%s", p.name, diff --git a/webui/static/sync-spotify.js b/webui/static/sync-spotify.js index 6af72167..cc888f1f 100644 --- a/webui/static/sync-spotify.js +++ b/webui/static/sync-spotify.js @@ -1639,7 +1639,7 @@ function renderSpotifyPlaylists() { container.innerHTML = spotifyPlaylists.map(p => { let statusClass = 'status-never-synced'; if (p.sync_status.startsWith('Synced')) statusClass = 'status-synced'; - if (p.sync_status === 'Needs Sync') statusClass = 'status-needs-sync'; + if (p.sync_status === 'Needs Sync' || p.sync_status.startsWith('Last Sync')) statusClass = 'status-needs-sync'; // This HTML structure creates the interactive playlist cards return ` From 7a8b66fd2ee9e2f5e767d85f6996570f82dd3e9d Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Thu, 25 Jun 2026 13:06:07 -0700 Subject: [PATCH 02/49] Auto-Sync Manager: redesign hourly + weekly boards as horizontal lanes Replace the side-scrolling column board with vertically-stacked interval lanes (hourly) and day lanes Mon-Sun (weekly). Empty intervals/days collapse to thin dashed strips, busy ones grow; scheduled playlists flow as cards within a lane. Kills the horizontal scroll + the wasted whitespace of the old kanban columns, and the two boards now share one cohesive design. Polish: accent gradient wash + gradient interval numerals + count badge on filled lanes, drag-over glow/lift, card pop-in animation, hover states. Also preserves the board's scroll position across the full re-render so dropping/removing a playlist no longer snaps it back to the top. Same drag-and-drop handlers + scheduled-card content reused; old column CSS is now unused (harmless). --- webui/static/auto-sync.js | 61 ++++++++++---- webui/static/style.css | 165 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 211 insertions(+), 15 deletions(-) diff --git a/webui/static/auto-sync.js b/webui/static/auto-sync.js index 8a91d3ad..74f08428 100644 --- a/webui/static/auto-sync.js +++ b/webui/static/auto-sync.js @@ -73,6 +73,17 @@ function autoSyncIntervalLabel(hours) { return `Every ${hours} hour${hours === 1 ? '' : 's'}`; } +// Short cadence word for the lane badge (under the interval number). +function autoSyncLaneCadence(hours) { + if (hours === 1) return 'Hourly'; + if (hours === 12) return 'Twice a day'; + if (hours === 24) return 'Daily'; + if (hours === 168) return 'Weekly'; + if (hours < 24) return `Every ${hours}h`; + const days = hours / 24; + return `Every ${days} days`; +} + // Browser-detected default tz for new schedules. Used when the user // creates a weekly schedule and hasn't picked an explicit tz — falls // back to UTC on browsers where Intl is unavailable (very old ones). @@ -354,6 +365,12 @@ function renderAutoSyncScheduleModal() { const overlay = document.getElementById('auto-sync-schedule-modal'); if (!overlay) return; + // Preserve the visible lane board's scroll across the full re-render so dropping / + // removing a playlist doesn't snap it back to the top (the user used to have to scroll + // back). Targets the ACTIVE tab so it works for both the hourly + weekly boards. + const _prevLanes = overlay.querySelector('.auto-sync-tab-panel.active .auto-sync-lanes'); + const _prevScroll = _prevLanes ? _prevLanes.scrollTop : null; + const { playlists, playlistSchedules, weeklySchedules, automationPipelines, runHistory, runHistoryTotal } = _autoSyncScheduleState; const scheduledCount = Object.keys(playlistSchedules).length + Object.keys(weeklySchedules || {}).length; const enabledCount = Object.values(playlistSchedules).filter(s => s.enabled).length @@ -408,6 +425,11 @@ function renderAutoSyncScheduleModal() { `; populateAutoSyncHistoryList(overlay); bindAutoSyncHistoryCardInteractions(overlay); + + if (_prevScroll != null) { + const nl = overlay.querySelector('.auto-sync-tab-panel.active .auto-sync-lanes'); + if (nl) nl.scrollTop = _prevScroll; + } } function setAutoSyncTab(tab) { @@ -479,17 +501,23 @@ function renderAutoSyncSchedulePanel(playlists, playlistSchedules) { .map(s => parseInt(s?.hours, 10)) .filter(h => Number.isFinite(h) && h > 0 && !AUTO_SYNC_BUCKETS.includes(h)); const allBuckets = [...new Set([...AUTO_SYNC_BUCKETS, ...customHours])].sort((a, b) => a - b); + // Concept 1 — interval LANES (horizontal rows) instead of columns: no side-scroll, + // empty intervals collapse to thin strips, busy ones grow. Same drag handlers + card. const bucketHtml = allBuckets.map(hours => { const assigned = schedulablePlaylists.filter(p => playlistSchedules[p.id]?.hours === hours); const isCustom = !AUTO_SYNC_BUCKETS.includes(hours); + const filled = assigned.length > 0; return ` -
-
- ${autoSyncBucketLabel(hours)}${isCustom ? ' custom' : ''} - ${assigned.length} playlist${assigned.length === 1 ? '' : 's'} +
+
+ ${autoSyncBucketLabel(hours)} + ${_esc(autoSyncLaneCadence(hours))}${isCustom ? ' · custom' : ''} + ${filled ? `${assigned.length}` : ''}
-
- ${assigned.length ? assigned.map(p => autoSyncScheduledCardHtml(p, playlistSchedules[p.id])).join('') : '
Drop hereSchedule playlists at this interval
'} +
+ ${filled + ? assigned.map(p => autoSyncScheduledCardHtml(p, playlistSchedules[p.id])).join('') + : `
+ Drag a playlist here to sync ${_esc(autoSyncIntervalLabel(hours).toLowerCase())}
`}
`; @@ -514,7 +542,7 @@ function renderAutoSyncSchedulePanel(playlists, playlistSchedules) {
${sidebarHtml}${unavailableHtml}
-
${bucketHtml}
+
${bucketHtml}
`; } @@ -598,21 +626,24 @@ function renderAutoSyncWeeklyPanel(playlists, playlistSchedules) { }); }); + // Day LANES (horizontal rows, Mon–Sun) — mirrors the hourly board's lane layout. const dayColumnsHtml = AUTO_SYNC_WEEKDAYS.map(day => { const cards = cardsByDay[day]; - const cardHtml = cards.length + const filled = cards.length > 0; + const cardHtml = filled ? cards.map(({ playlist, schedule }) => autoSyncWeeklyCardHtml(playlist, schedule)).join('') - : '
Drop hereSchedule playlists on this day
'; + : `
+ Drag a playlist here to sync every ${_esc(AUTO_SYNC_WEEKDAY_LABELS[day])}
`; return ` -
-
- ${AUTO_SYNC_WEEKDAY_LABELS[day]} - ${cards.length} playlist${cards.length === 1 ? '' : 's'} +
+ ${_esc(AUTO_SYNC_WEEKDAY_LABELS[day])} + Weekly + ${filled ? `${cards.length}` : ''}
-
${cardHtml}
+
${cardHtml}
`; }).join(''); @@ -637,7 +668,7 @@ function renderAutoSyncWeeklyPanel(playlists, playlistSchedules) {
${sidebarHtml}${unavailableHtml}
-
${dayColumnsHtml}
+
${dayColumnsHtml}
${editorHtml} `; diff --git a/webui/static/style.css b/webui/static/style.css index 4939df93..6b0f8677 100644 --- a/webui/static/style.css +++ b/webui/static/style.css @@ -12465,6 +12465,171 @@ body.helper-mode-active #dashboard-activity-feed:hover { color: rgba(255, 255, 255, 0.45); } +/* ── Hourly board: interval LANES (horizontal rows — Concept 1) ───────────── */ +.auto-sync-lanes { + min-width: 0; + min-height: 0; + overflow-y: auto; + overflow-x: hidden; + padding: 16px 20px; + display: flex; + flex-direction: column; + gap: 10px; +} + +.auto-sync-lane { + display: flex; + align-items: stretch; + gap: 12px; + padding: 11px 14px; + border: 1px solid rgba(255, 255, 255, 0.07); + border-radius: 16px; + background: rgba(255, 255, 255, 0.025); + transition: border-color 0.22s ease, background 0.22s ease, + box-shadow 0.22s ease, transform 0.18s ease; +} + +.auto-sync-lane.empty { + background: transparent; + border-style: dashed; + border-color: rgba(255, 255, 255, 0.08); +} + +.auto-sync-lane.filled { + border-color: rgba(var(--accent-rgb), 0.22); + background: + linear-gradient(100deg, rgba(var(--accent-rgb), 0.07), rgba(var(--accent-rgb), 0.012) 55%), + rgba(255, 255, 255, 0.02); +} + +.auto-sync-lane:hover { + border-color: rgba(var(--accent-rgb), 0.3); +} + +.auto-sync-lane.drag-over { + border-color: rgba(var(--accent-rgb), 0.65); + border-style: solid; + background: rgba(var(--accent-rgb), 0.1); + box-shadow: + inset 0 0 0 1px rgba(var(--accent-rgb), 0.35), + 0 10px 30px rgba(var(--accent-rgb), 0.16); + transform: translateY(-1px); +} + +.auto-sync-lane-badge { + position: relative; + flex: 0 0 86px; + display: flex; + flex-direction: column; + justify-content: center; + gap: 2px; + padding: 4px 14px 4px 4px; + border-right: 1px solid rgba(255, 255, 255, 0.07); +} + +.auto-sync-lane-badge b { + font-size: 21px; + font-weight: 800; + line-height: 1; + letter-spacing: -0.02em; + color: rgba(255, 255, 255, 0.92); +} + +.auto-sync-lane.filled .auto-sync-lane-badge b { + background: linear-gradient(160deg, rgb(var(--accent-rgb)), rgba(var(--accent-rgb), 0.6)); + -webkit-background-clip: text; + background-clip: text; + -webkit-text-fill-color: transparent; +} + +.auto-sync-lane-badge span { + font-size: 10px; + font-weight: 700; + text-transform: uppercase; + letter-spacing: 0.05em; + color: rgba(255, 255, 255, 0.4); + white-space: nowrap; +} + +.auto-sync-lane-count { + position: absolute; + top: 0; + right: 10px; + min-width: 17px; + height: 17px; + padding: 0 4px; + display: grid; + place-items: center; + font-size: 10px; + font-weight: 800; + font-style: normal; + border-radius: 999px; + color: #fff; + background: rgba(var(--accent-rgb), 0.9); + box-shadow: 0 2px 8px rgba(var(--accent-rgb), 0.35); +} + +.auto-sync-lane-track { + flex: 1; + min-width: 0; + display: flex; + flex-wrap: wrap; + align-items: center; + gap: 8px; + padding: 2px 0; +} + +.auto-sync-lane-hint { + display: flex; + align-items: center; + gap: 9px; + padding: 7px 2px; + color: rgba(255, 255, 255, 0.32); + font-size: 12.5px; + font-weight: 500; +} + +.auto-sync-lane-hint-ic { + display: grid; + place-items: center; + width: 22px; + height: 22px; + border-radius: 7px; + border: 1px dashed rgba(255, 255, 255, 0.2); + font-size: 15px; + line-height: 1; +} + +.auto-sync-lane.drag-over .auto-sync-lane-hint { color: rgb(var(--accent-rgb)); } +.auto-sync-lane.drag-over .auto-sync-lane-hint-ic { + border-style: solid; + border-color: rgba(var(--accent-rgb), 0.6); + color: rgb(var(--accent-rgb)); +} + +/* Scheduled cards flow horizontally inside a lane (override the column full-width) */ +.auto-sync-lane .auto-sync-scheduled-card { + width: auto; + min-width: 212px; + max-width: 264px; + margin-bottom: 0; + padding: 10px 12px; + border-radius: 12px; + background: rgba(0, 0, 0, 0.28); + border: 1px solid rgba(255, 255, 255, 0.07); + animation: autoSyncPop 0.24s ease both; +} + +.auto-sync-lane .auto-sync-scheduled-card:hover { + border-color: rgba(var(--accent-rgb), 0.4); + background: rgba(0, 0, 0, 0.36); +} + +@keyframes autoSyncPop { + from { opacity: 0; transform: translateY(5px) scale(0.97); } + to { opacity: 1; transform: none; } +} + .auto-sync-board { min-width: 0; min-height: 0; From 528aedbdfea947c8c991ec1c7b36752071e6773c Mon Sep 17 00:00:00 2001 From: ragnarlotus Date: Thu, 25 Jun 2026 22:54:43 +0200 Subject: [PATCH 03/49] fix(reorganize): include MusicBrainz release IDs --- core/library_reorganize.py | 1 + tests/test_reorganize_canonical_source.py | 24 +++++++++++++++++++++++ 2 files changed, 25 insertions(+) diff --git a/core/library_reorganize.py b/core/library_reorganize.py index 574d5ee7..a7e4fa37 100644 --- a/core/library_reorganize.py +++ b/core/library_reorganize.py @@ -100,6 +100,7 @@ _ALBUM_ID_COLUMNS = { 'deezer': 'deezer_id', 'discogs': 'discogs_id', 'hydrabase': 'soul_id', + 'musicbrainz': 'musicbrainz_release_id', } # Human-facing label for each source. diff --git a/tests/test_reorganize_canonical_source.py b/tests/test_reorganize_canonical_source.py index 711638ed..bb39c436 100644 --- a/tests/test_reorganize_canonical_source.py +++ b/tests/test_reorganize_canonical_source.py @@ -73,3 +73,27 @@ def test_no_canonical_unchanged(monkeypatch): album_data = {"spotify_album_id": "sp1"} source, _, _ = lr._resolve_source(album_data, "spotify") assert source == "spotify" + + +def test_musicbrainz_release_id_is_used_by_priority_walk(monkeypatch): + _patch_fetch(monkeypatch, { + ('musicbrainz', 'mb-release-1'): [{'name': 'x'}], + }) + monkeypatch.setattr( + lr, + 'get_source_priority', + lambda primary: ['musicbrainz', 'spotify'], + ) + + album_data = { + 'musicbrainz_release_id': 'mb-release-1', + } + + source, api_album, items = lr._resolve_source( + album_data, + 'musicbrainz', + ) + + assert source == 'musicbrainz' + assert api_album == {'name': 'musicbrainz:mb-release-1'} + assert items == [{'name': 'x'}] From e3915b63e67fb6e1c3945dd09ebe1d2ce569f85e Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Thu, 25 Jun 2026 13:57:34 -0700 Subject: [PATCH 04/49] Library cards: in-card badges no longer trigger artist-detail navigation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The card is an link and the shell's capture-phase link handler navigated to artist-detail before the grid's bubble-phase badge handler could preventDefault — so clicking the watchlist eye or a source badge opened the detail page (and the badge's own link too). The shell handler now bails when the click lands on an in-card control (.source-card-icon or [data-no-card-nav]), letting the badge do only its own thing. --- webui/static/shell-bridge.js | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/webui/static/shell-bridge.js b/webui/static/shell-bridge.js index 1f93f2c3..6263b6c8 100644 --- a/webui/static/shell-bridge.js +++ b/webui/static/shell-bridge.js @@ -211,6 +211,11 @@ function _handleShellLinkClick(event) { if (!anchor || (anchor.target && anchor.target !== '_self')) return; if (anchor.hasAttribute('download')) return; + // In-card controls (source/watchlist badges, etc.) handle their OWN click — don't let + // this capture-phase handler hijack it into the surrounding card's navigation. Their + // bubble-phase handlers preventDefault, but that runs after capture, so we opt out here. + if (event.target?.closest?.('.source-card-icon, [data-no-card-nav]')) return; + const href = anchor.getAttribute('href'); if (!href || href === '#' || href.startsWith('javascript:')) return; From 602b035bad3422fa9987c62eac44243f54d5df5c Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Thu, 25 Jun 2026 13:57:50 -0700 Subject: [PATCH 05/49] Wing It Pool: review + re-match tracks Wing It auto-matched MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Wing It auto-matches tracks to the server library on a best-effort guess; those tracks are flagged wing_it_fallback in extra_data and count as 'discovered', so the Discovery Pool hides them — there was no way to see or audit the guesses. New 'Wing It Pool' button (next to Discovery Pool on the Mirrored Playlists tab) opens a modal listing them with a per-playlist filter + search; 'Fix Match' reuses the Discovery Pool's fix flow (/api/discovery-pool/fix), and a manual match drops the track from the pool on refresh. No new table or provider hooks needed — the wing-it flag is already persisted, so this is a pure query (get_wing_it_pool / get_wing_it_pool_stats, cloning the failed-pool LIKE pattern) + a /api/wing-it-pool endpoint + a cloned modal. Found 81 wing-it tracks on a real library. Seam-tested (include unverified / exclude manual-matched / scope by playlist+profile). --- database/music_database.py | 55 +++++++++++ tests/test_wing_it_pool.py | 76 +++++++++++++++ web_server.py | 29 ++++++ webui/index.html | 1 + webui/static/stats-automations.js | 149 +++++++++++++++++++++++++++++- 5 files changed, 308 insertions(+), 2 deletions(-) create mode 100644 tests/test_wing_it_pool.py diff --git a/database/music_database.py b/database/music_database.py index 97925c32..be64e2a1 100644 --- a/database/music_database.py +++ b/database/music_database.py @@ -13140,6 +13140,61 @@ class MusicDatabase: logger.error(f"Error getting discovery pool stats: {e}") return {'matched': 0, 'failed': 0} + def get_wing_it_pool(self, profile_id: int = None, playlist_id: int = None) -> list: + """Get tracks that were auto-matched by Wing It mode (best-effort, unverified). + + Wing-it tracks are persisted on the mirrored track's extra_data with + ``wing_it_fallback: true`` (set when a track couldn't match a metadata source and got a + raw-name stub). They count as 'discovered', so the Discovery Pool's failed list excludes + them — this is the only surface that lists them so the user can verify/re-match the guesses. + Excludes any the user has already manually matched (``manual_match: true``). + """ + try: + conn = self._get_connection() + cursor = conn.cursor() + query = """ + SELECT mpt.id, mpt.track_name, mpt.artist_name, mpt.album_name, + mpt.playlist_id, mp.name as playlist_name + FROM mirrored_playlist_tracks mpt + JOIN mirrored_playlists mp ON mpt.playlist_id = mp.id + WHERE mpt.extra_data LIKE '%"wing_it_fallback": true%' + AND mpt.extra_data NOT LIKE '%"manual_match": true%' + """ + params = [] + if playlist_id: + query += " AND mpt.playlist_id = ?" + params.append(playlist_id) + elif profile_id: + query += " AND mp.profile_id = ?" + params.append(profile_id) + query += " ORDER BY mp.name, mpt.track_name" + cursor.execute(query, params) + return [dict(row) for row in cursor.fetchall()] + except Exception as e: + logger.error(f"Error getting wing it pool: {e}") + return [] + + def get_wing_it_pool_stats(self, profile_id: int = None) -> dict: + """Count of unverified Wing It auto-matches (excludes manually-matched).""" + try: + conn = self._get_connection() + cursor = conn.cursor() + query = """ + SELECT COUNT(*) as cnt FROM mirrored_playlist_tracks mpt + JOIN mirrored_playlists mp ON mpt.playlist_id = mp.id + WHERE mpt.extra_data LIKE '%"wing_it_fallback": true%' + AND mpt.extra_data NOT LIKE '%"manual_match": true%' + """ + params = [] + if profile_id: + query += " AND mp.profile_id = ?" + params.append(profile_id) + cursor.execute(query, params) + return {'wing_it': cursor.fetchone()['cnt']} + except Exception as e: + logger.error(f"Error getting wing it pool stats: {e}") + return {'wing_it': 0} + # ==================== Retag Tool Methods ==================== def add_retag_group(self, group_type: str, artist_name: str, album_name: str, diff --git a/tests/test_wing_it_pool.py b/tests/test_wing_it_pool.py new file mode 100644 index 00000000..5b23aa50 --- /dev/null +++ b/tests/test_wing_it_pool.py @@ -0,0 +1,76 @@ +"""Wing It Pool query — surfaces tracks Wing It auto-matched (best-effort guesses). + +Wing-it tracks are persisted as the ``wing_it_fallback: true`` flag on a mirrored track's +extra_data and count as 'discovered', so the Discovery Pool's failed list excludes them. The +Wing It Pool is the only surface that lists them. It must: include unverified wing-it tracks, +exclude ones the user already manually matched, scope by playlist + profile, and never include +plain matched/failed tracks. +""" + +from __future__ import annotations + +import json + +from database.music_database import MusicDatabase + + +def _playlist(db, name, profile_id=1, source_id='pl1'): + with db._get_connection() as conn: + cur = conn.execute( + "INSERT INTO mirrored_playlists (source, source_playlist_id, name, profile_id) VALUES (?,?,?,?)", + ('spotify', source_id, name, profile_id)) + conn.commit() + return cur.lastrowid + + +def _track(db, playlist_id, pos, name, artist, extra): + with db._get_connection() as conn: + conn.execute( + "INSERT INTO mirrored_playlist_tracks (playlist_id, position, track_name, artist_name, extra_data) " + "VALUES (?,?,?,?,?)", + (playlist_id, pos, name, artist, json.dumps(extra) if extra is not None else None)) + conn.commit() + + +WING_IT = {'discovered': True, 'provider': 'wing_it_fallback', 'confidence': 0, 'wing_it_fallback': True} +WING_IT_FIXED = {'discovered': True, 'provider': 'spotify', 'confidence': 1.0, + 'wing_it_fallback': True, 'manual_match': True} +MATCHED = {'discovered': True, 'provider': 'spotify', 'confidence': 0.95} +FAILED = {'discovery_attempted': True, 'discovered': False} + + +def test_lists_only_unverified_wing_it_tracks(tmp_path): + db = MusicDatabase(database_path=str(tmp_path / "w.db")) + pid = _playlist(db, 'Liked Songs') + _track(db, pid, 0, 'Orbital Trans', 'Yoga Mao', WING_IT) # unverified wing-it -> include + _track(db, pid, 1, 'Dopamine', 'Rvdical the Kid', WING_IT_FIXED) # manually fixed -> exclude + _track(db, pid, 2, 'Real Match', 'Some Artist', MATCHED) # normal match -> exclude + _track(db, pid, 3, 'Lost Track', 'Nobody', FAILED) # failed -> exclude (it's Discovery Pool's) + + out = db.get_wing_it_pool(profile_id=1) + assert [t['track_name'] for t in out] == ['Orbital Trans'] + assert out[0]['artist_name'] == 'Yoga Mao' + assert out[0]['playlist_name'] == 'Liked Songs' + assert db.get_wing_it_pool_stats(profile_id=1) == {'wing_it': 1} + + +def test_scopes_by_playlist_and_profile(tmp_path): + db = MusicDatabase(database_path=str(tmp_path / "w2.db")) + a = _playlist(db, 'Playlist A', profile_id=1, source_id='a') + b = _playlist(db, 'Playlist B', profile_id=1, source_id='b') + other = _playlist(db, 'Other Profile', profile_id=2, source_id='c') + _track(db, a, 0, 'A Song', 'AA', WING_IT) + _track(db, b, 0, 'B Song', 'BB', WING_IT) + _track(db, other, 0, 'C Song', 'CC', WING_IT) + + assert {t['track_name'] for t in db.get_wing_it_pool(profile_id=1)} == {'A Song', 'B Song'} + assert [t['track_name'] for t in db.get_wing_it_pool(playlist_id=a)] == ['A Song'] + assert db.get_wing_it_pool_stats(profile_id=1) == {'wing_it': 2} + + +def test_empty_when_no_wing_it(tmp_path): + db = MusicDatabase(database_path=str(tmp_path / "w3.db")) + pid = _playlist(db, 'Clean') + _track(db, pid, 0, 'Matched', 'X', MATCHED) + assert db.get_wing_it_pool(profile_id=1) == [] + assert db.get_wing_it_pool_stats(profile_id=1) == {'wing_it': 0} diff --git a/web_server.py b/web_server.py index 0c81f2f3..c8dd461e 100644 --- a/web_server.py +++ b/web_server.py @@ -35008,6 +35008,35 @@ def get_discovery_pool(): logger.error(f"Error getting discovery pool: {e}") return jsonify({"error": str(e)}), 500 +@app.route('/api/wing-it-pool', methods=['GET']) +def get_wing_it_pool(): + """List Wing It auto-matched tracks (unverified best-effort guesses), optionally per-playlist. + + These are tracks that couldn't match a metadata source and got a raw-name Wing It stub. They + count as 'discovered' so the Discovery Pool hides them — this surfaces them so the user can + verify and re-match. Re-matching reuses the Discovery Pool's /api/discovery-pool/fix endpoint + (both key off the mirrored_playlist_tracks.id), and a manual match drops the track from here. + """ + try: + database = get_database() + profile_id = get_current_profile_id() + playlist_id = request.args.get('playlist_id', type=int) + + tracks = database.get_wing_it_pool(profile_id=profile_id, playlist_id=playlist_id) + stats = database.get_wing_it_pool_stats(profile_id=profile_id) + + playlists = database.get_mirrored_playlists(profile_id=profile_id) + playlist_options = [{'id': p['id'], 'name': p['name']} for p in playlists] + + return jsonify({ + 'tracks': tracks, + 'stats': stats, + 'playlists': playlist_options, + }) + except Exception as e: + logger.error(f"Error getting wing it pool: {e}") + return jsonify({"error": str(e)}), 500 + @app.route('/api/discovery-pool/fix', methods=['POST']) def fix_discovery_pool_track(): """Manually fix a failed discovery by linking a mirrored track to a Spotify/iTunes result.""" diff --git a/webui/index.html b/webui/index.html index 1b83fae9..f3680b89 100644 --- a/webui/index.html +++ b/webui/index.html @@ -2041,6 +2041,7 @@

Mirrored Playlists

+
diff --git a/webui/static/stats-automations.js b/webui/static/stats-automations.js index f08e2e23..b7fae9dc 100644 --- a/webui/static/stats-automations.js +++ b/webui/static/stats-automations.js @@ -1290,6 +1290,147 @@ function closeDiscoveryPoolModal() { loadDiscoveryPoolStats(); } +// ── Wing It Pool ─────────────────────────────────────────────────────────── +// Lists tracks Wing It auto-matched on a best-effort guess (extra_data +// wing_it_fallback flag). They count as 'discovered' so the Discovery Pool hides +// them — this is the only place to review + re-match the guesses. Re-match reuses +// the Discovery Pool's openPoolFixModal / /api/discovery-pool/fix (same track id); +// a manual match drops the row from here on the next refresh. +let _wingItPoolOverlay = null; +let _wingItPoolData = null; +let _wingItPoolPlaylistFilter = null; + +async function openWingItPoolModal(playlistId = null) { + _wingItPoolPlaylistFilter = playlistId; + let url = '/api/wing-it-pool'; + if (playlistId) url += `?playlist_id=${playlistId}`; + try { + const res = await fetch(url); + _wingItPoolData = await res.json(); + } catch (err) { + showToast('Failed to load Wing It pool', 'error'); + return; + } + if (_wingItPoolOverlay) _wingItPoolOverlay.remove(); + + const overlay = document.createElement('div'); + overlay.className = 'modal-overlay'; + overlay.id = 'wing-it-pool-overlay'; + overlay.onclick = (e) => { if (e.target === overlay) closeWingItPoolModal(); }; + + const playlistOptions = (_wingItPoolData.playlists || []) + .map(p => ``) + .join(''); + const count = (_wingItPoolData.tracks || []).length; + + overlay.innerHTML = ` +
+
+
+

Wing It Pool

+
+ ${count} to review + +
+
+ × +
+ +
+
+
+ ⚡ Auto-matched by Wing It — verify or re-match + +
+
+
+
+ +
+
+
+ +
+
+
+ `; + + document.body.appendChild(overlay); + overlay.style.display = 'flex'; + _wingItPoolOverlay = overlay; + renderWingItPoolList(); +} + +function renderWingItPoolList() { + const container = document.getElementById('wing-it-list-content'); + if (!container || !_wingItPoolData) return; + + const searchEl = document.getElementById('wing-it-list-search'); + const query = (searchEl ? searchEl.value : '').toLowerCase().trim(); + let tracks = _wingItPoolData.tracks || []; + if (query) { + tracks = tracks.filter(t => + (t.track_name || '').toLowerCase().includes(query) || + (t.artist_name || '').toLowerCase().includes(query) || + (t.playlist_name || '').toLowerCase().includes(query) + ); + } + if (tracks.length === 0) { + container.innerHTML = query + ? '
No Wing It tracks match your filter.
' + : '
No Wing It tracks — nothing was auto-matched on a guess.
'; + return; + } + container.innerHTML = tracks.map(t => ` +
+
+
${_esc(t.track_name)}
+
+ ${_esc(t.artist_name)} + ${_esc(t.playlist_name)} +
+
+ +
+ `).join(''); +} + +async function filterWingItPool(playlistId) { + _wingItPoolPlaylistFilter = playlistId || null; + let url = '/api/wing-it-pool'; + if (playlistId) url += `?playlist_id=${playlistId}`; + try { + const res = await fetch(url); + _wingItPoolData = await res.json(); + } catch (e) { + showToast('Failed to filter Wing It pool', 'error'); + return; + } + renderWingItPoolList(); + const countEl = document.getElementById('wing-it-header-count'); + if (countEl) { + const c = (_wingItPoolData.tracks || []).length; + countEl.textContent = `${c} to review`; + countEl.classList.toggle('pool-header-failed-highlight', c > 0); + } +} + +// Re-fetch + re-render the open Wing It pool (used after a Fix Match resolves a track). +function refreshWingItPool() { + filterWingItPool(_wingItPoolPlaylistFilter || ''); +} + +function closeWingItPoolModal() { + if (_wingItPoolOverlay) { + _wingItPoolOverlay.remove(); + _wingItPoolOverlay = null; + } + _wingItPoolData = null; +} + function showPoolCategories() { _discoveryPoolView = 'categories'; const grid = document.getElementById('pool-category-grid'); @@ -1700,8 +1841,12 @@ async function selectPoolFixTrack(track) { if (data.success) { showToast(`Matched: ${track.name}`, 'success'); closePoolFixModal(); - // Refresh pool data - filterDiscoveryPool(_discoveryPoolPlaylistFilter || ''); + // Refresh whichever pool the fix was launched from (Discovery or Wing It). + if (typeof _wingItPoolOverlay !== 'undefined' && _wingItPoolOverlay) { + refreshWingItPool(); + } else { + filterDiscoveryPool(_discoveryPoolPlaylistFilter || ''); + } } else { showToast(data.error || 'Failed to fix track', 'error'); } From 9847d6f0a9bd94f825289b64b89dceb851139a3b Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Thu, 25 Jun 2026 14:09:36 -0700 Subject: [PATCH 06/49] Wing It Pool: two-card landing (review + resolved), matching the Discovery Pool MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Opens to the same category-card landing the Discovery Pool uses, with two cards: 'guesses to review' (unverified wing-it) and 'resolved manually' (ones you've Fixed) — click to drill in, Back to return. Previously it jumped straight to a single list. To populate the resolved list, the /fix endpoint now stamps was_wing_it on the rewritten extra_data (the wing_it_fallback flag is otherwise lost on fix), and get_wing_it_pool gained a resolved flag + the stats return both counts. Fixing/re-matching from either card refreshes in place. Seam test updated for both states. --- database/music_database.py | 62 ++++++++------ tests/test_wing_it_pool.py | 33 +++++--- web_server.py | 5 +- webui/static/stats-automations.js | 134 +++++++++++++++++++++++++----- 4 files changed, 172 insertions(+), 62 deletions(-) diff --git a/database/music_database.py b/database/music_database.py index be64e2a1..e73aa59a 100644 --- a/database/music_database.py +++ b/database/music_database.py @@ -13140,25 +13140,36 @@ class MusicDatabase: logger.error(f"Error getting discovery pool stats: {e}") return {'matched': 0, 'failed': 0} - def get_wing_it_pool(self, profile_id: int = None, playlist_id: int = None) -> list: - """Get tracks that were auto-matched by Wing It mode (best-effort, unverified). + # Wing It Pool: two states on a mirrored track's extra_data. Both key off wing_it_fallback, + # which is set by the wing-it stub and SURVIVES a manual fix (update_mirrored_track_extra_data + # merges rather than replaces), so the only difference is the manual_match flag: + # needs attention : wing_it_fallback=true AND NOT manual_match (unverified guess) + # resolved : wing_it_fallback=true AND manual_match=true (user fixed it — incl. fixes + # made before this feature existed, since the flag was never wiped) + _WING_IT_ATTENTION = ("mpt.extra_data LIKE '%\"wing_it_fallback\": true%' " + "AND mpt.extra_data NOT LIKE '%\"manual_match\": true%'") + _WING_IT_RESOLVED = ("mpt.extra_data LIKE '%\"wing_it_fallback\": true%' " + "AND mpt.extra_data LIKE '%\"manual_match\": true%'") - Wing-it tracks are persisted on the mirrored track's extra_data with - ``wing_it_fallback: true`` (set when a track couldn't match a metadata source and got a - raw-name stub). They count as 'discovered', so the Discovery Pool's failed list excludes - them — this is the only surface that lists them so the user can verify/re-match the guesses. - Excludes any the user has already manually matched (``manual_match: true``). + def get_wing_it_pool(self, profile_id: int = None, playlist_id: int = None, + resolved: bool = False) -> list: + """Get Wing It tracks — the unverified guesses (default) or the ones you've resolved. + + Wing-it tracks are persisted on extra_data with ``wing_it_fallback: true`` (a best-effort + stub when a track couldn't match a metadata source). They count as 'discovered', so the + Discovery Pool hides them — this is the only surface that lists them. ``resolved=True`` + returns the ones a manual match has since fixed (carrying the ``was_wing_it`` marker). """ try: conn = self._get_connection() cursor = conn.cursor() - query = """ + where = self._WING_IT_RESOLVED if resolved else self._WING_IT_ATTENTION + query = f""" SELECT mpt.id, mpt.track_name, mpt.artist_name, mpt.album_name, - mpt.playlist_id, mp.name as playlist_name + mpt.playlist_id, mp.name as playlist_name, mpt.extra_data FROM mirrored_playlist_tracks mpt JOIN mirrored_playlists mp ON mpt.playlist_id = mp.id - WHERE mpt.extra_data LIKE '%"wing_it_fallback": true%' - AND mpt.extra_data NOT LIKE '%"manual_match": true%' + WHERE {where} """ params = [] if playlist_id: @@ -13175,25 +13186,26 @@ class MusicDatabase: return [] def get_wing_it_pool_stats(self, profile_id: int = None) -> dict: - """Count of unverified Wing It auto-matches (excludes manually-matched).""" + """Counts for both Wing It states: unverified (``wing_it``) + resolved (``matched``).""" try: conn = self._get_connection() cursor = conn.cursor() - query = """ - SELECT COUNT(*) as cnt FROM mirrored_playlist_tracks mpt - JOIN mirrored_playlists mp ON mpt.playlist_id = mp.id - WHERE mpt.extra_data LIKE '%"wing_it_fallback": true%' - AND mpt.extra_data NOT LIKE '%"manual_match": true%' - """ - params = [] - if profile_id: - query += " AND mp.profile_id = ?" - params.append(profile_id) - cursor.execute(query, params) - return {'wing_it': cursor.fetchone()['cnt']} + + def _count(where): + q = (f"SELECT COUNT(*) as cnt FROM mirrored_playlist_tracks mpt " + f"JOIN mirrored_playlists mp ON mpt.playlist_id = mp.id WHERE {where}") + params = [] + if profile_id: + q += " AND mp.profile_id = ?" + params.append(profile_id) + cursor.execute(q, params) + return cursor.fetchone()['cnt'] + + return {'wing_it': _count(self._WING_IT_ATTENTION), + 'matched': _count(self._WING_IT_RESOLVED)} except Exception as e: logger.error(f"Error getting wing it pool stats: {e}") - return {'wing_it': 0} + return {'wing_it': 0, 'matched': 0} # ==================== Retag Tool Methods ==================== diff --git a/tests/test_wing_it_pool.py b/tests/test_wing_it_pool.py index 5b23aa50..81b693a3 100644 --- a/tests/test_wing_it_pool.py +++ b/tests/test_wing_it_pool.py @@ -33,8 +33,11 @@ def _track(db, playlist_id, pos, name, artist, extra): WING_IT = {'discovered': True, 'provider': 'wing_it_fallback', 'confidence': 0, 'wing_it_fallback': True} -WING_IT_FIXED = {'discovered': True, 'provider': 'spotify', 'confidence': 1.0, - 'wing_it_fallback': True, 'manual_match': True} +# A resolved wing-it track: /fix MERGES extra_data, so wing_it_fallback survives alongside the +# new manual_match flag — that pairing is what marks it resolved (no separate marker needed). +WING_IT_RESOLVED = {'discovered': True, 'provider': 'spotify', 'confidence': 1.0, + 'wing_it_fallback': True, 'manual_match': True, + 'matched_data': {'name': 'Dopamine (Real)'}} MATCHED = {'discovered': True, 'provider': 'spotify', 'confidence': 0.95} FAILED = {'discovery_attempted': True, 'discovered': False} @@ -42,16 +45,20 @@ FAILED = {'discovery_attempted': True, 'discovered': False} def test_lists_only_unverified_wing_it_tracks(tmp_path): db = MusicDatabase(database_path=str(tmp_path / "w.db")) pid = _playlist(db, 'Liked Songs') - _track(db, pid, 0, 'Orbital Trans', 'Yoga Mao', WING_IT) # unverified wing-it -> include - _track(db, pid, 1, 'Dopamine', 'Rvdical the Kid', WING_IT_FIXED) # manually fixed -> exclude - _track(db, pid, 2, 'Real Match', 'Some Artist', MATCHED) # normal match -> exclude - _track(db, pid, 3, 'Lost Track', 'Nobody', FAILED) # failed -> exclude (it's Discovery Pool's) + _track(db, pid, 0, 'Orbital Trans', 'Yoga Mao', WING_IT) # unverified -> attention + _track(db, pid, 1, 'Dopamine', 'Rvdical the Kid', WING_IT_RESOLVED) # resolved -> matched list + _track(db, pid, 2, 'Real Match', 'Some Artist', MATCHED) # normal match -> neither + _track(db, pid, 3, 'Lost Track', 'Nobody', FAILED) # failed -> Discovery Pool's - out = db.get_wing_it_pool(profile_id=1) - assert [t['track_name'] for t in out] == ['Orbital Trans'] - assert out[0]['artist_name'] == 'Yoga Mao' - assert out[0]['playlist_name'] == 'Liked Songs' - assert db.get_wing_it_pool_stats(profile_id=1) == {'wing_it': 1} + attention = db.get_wing_it_pool(profile_id=1) + assert [t['track_name'] for t in attention] == ['Orbital Trans'] + assert attention[0]['artist_name'] == 'Yoga Mao' + assert attention[0]['playlist_name'] == 'Liked Songs' + + resolved = db.get_wing_it_pool(profile_id=1, resolved=True) + assert [t['track_name'] for t in resolved] == ['Dopamine'] + + assert db.get_wing_it_pool_stats(profile_id=1) == {'wing_it': 1, 'matched': 1} def test_scopes_by_playlist_and_profile(tmp_path): @@ -65,7 +72,7 @@ def test_scopes_by_playlist_and_profile(tmp_path): assert {t['track_name'] for t in db.get_wing_it_pool(profile_id=1)} == {'A Song', 'B Song'} assert [t['track_name'] for t in db.get_wing_it_pool(playlist_id=a)] == ['A Song'] - assert db.get_wing_it_pool_stats(profile_id=1) == {'wing_it': 2} + assert db.get_wing_it_pool_stats(profile_id=1) == {'wing_it': 2, 'matched': 0} def test_empty_when_no_wing_it(tmp_path): @@ -73,4 +80,4 @@ def test_empty_when_no_wing_it(tmp_path): pid = _playlist(db, 'Clean') _track(db, pid, 0, 'Matched', 'X', MATCHED) assert db.get_wing_it_pool(profile_id=1) == [] - assert db.get_wing_it_pool_stats(profile_id=1) == {'wing_it': 0} + assert db.get_wing_it_pool_stats(profile_id=1) == {'wing_it': 0, 'matched': 0} diff --git a/web_server.py b/web_server.py index c8dd461e..067d944a 100644 --- a/web_server.py +++ b/web_server.py @@ -35023,6 +35023,7 @@ def get_wing_it_pool(): playlist_id = request.args.get('playlist_id', type=int) tracks = database.get_wing_it_pool(profile_id=profile_id, playlist_id=playlist_id) + matched = database.get_wing_it_pool(profile_id=profile_id, playlist_id=playlist_id, resolved=True) stats = database.get_wing_it_pool_stats(profile_id=profile_id) playlists = database.get_mirrored_playlists(profile_id=profile_id) @@ -35030,6 +35031,7 @@ def get_wing_it_pool(): return jsonify({ 'tracks': tracks, + 'matched': matched, 'stats': stats, 'playlists': playlist_options, }) @@ -35080,7 +35082,8 @@ def fix_discovery_pool_track(): 'source': 'spotify', } - # Update the mirrored track's extra_data + # Update the mirrored track's extra_data (merges, so a wing-it track keeps its + # wing_it_fallback flag — that + manual_match is how the Wing It Pool lists resolved guesses). extra_data = { 'discovered': True, 'provider': 'spotify', diff --git a/webui/static/stats-automations.js b/webui/static/stats-automations.js index b7fae9dc..9073be1a 100644 --- a/webui/static/stats-automations.js +++ b/webui/static/stats-automations.js @@ -1298,10 +1298,12 @@ function closeDiscoveryPoolModal() { // a manual match drops the row from here on the next refresh. let _wingItPoolOverlay = null; let _wingItPoolData = null; +let _wingItPoolView = 'categories'; // 'categories' | 'attention' | 'matched' let _wingItPoolPlaylistFilter = null; async function openWingItPoolModal(playlistId = null) { _wingItPoolPlaylistFilter = playlistId; + _wingItPoolView = 'categories'; let url = '/api/wing-it-pool'; if (playlistId) url += `?playlist_id=${playlistId}`; try { @@ -1321,7 +1323,8 @@ async function openWingItPoolModal(playlistId = null) { const playlistOptions = (_wingItPoolData.playlists || []) .map(p => ``) .join(''); - const count = (_wingItPoolData.tracks || []).length; + const attentionCount = (_wingItPoolData.tracks || []).length; + const matchedCount = (_wingItPoolData.matched || []).length; overlay.innerHTML = `
@@ -1329,7 +1332,8 @@ async function openWingItPoolModal(playlistId = null) {

Wing It Pool

- ${count} to review + ${attentionCount} to review + ${matchedCount} resolved
@@ -1361,16 +1389,45 @@ async function openWingItPoolModal(playlistId = null) { document.body.appendChild(overlay); overlay.style.display = 'flex'; _wingItPoolOverlay = overlay; +} + +function showWingItList(view) { + _wingItPoolView = view; + const grid = document.getElementById('wing-it-category-grid'); + const listView = document.getElementById('wing-it-list-view'); + const title = document.getElementById('wing-it-list-title'); + const search = document.getElementById('wing-it-list-search'); + if (grid) grid.style.display = 'none'; + if (listView) listView.style.display = 'block'; + if (title) title.textContent = view === 'matched' + ? '✓ Resolved Wing It guesses' : '⚡ Guesses to review'; + if (search) search.value = ''; renderWingItPoolList(); } +function showWingItCategories() { + _wingItPoolView = 'categories'; + const grid = document.getElementById('wing-it-category-grid'); + const listView = document.getElementById('wing-it-list-view'); + if (grid) grid.style.display = 'grid'; + if (listView) listView.style.display = 'none'; +} + +function _wingItMatchedName(t) { + try { + const md = JSON.parse(t.extra_data || '{}').matched_data || {}; + return md.name || ''; + } catch (e) { return ''; } +} + function renderWingItPoolList() { const container = document.getElementById('wing-it-list-content'); if (!container || !_wingItPoolData) return; const searchEl = document.getElementById('wing-it-list-search'); const query = (searchEl ? searchEl.value : '').toLowerCase().trim(); - let tracks = _wingItPoolData.tracks || []; + const isMatched = _wingItPoolView === 'matched'; + let tracks = (isMatched ? _wingItPoolData.matched : _wingItPoolData.tracks) || []; if (query) { tracks = tracks.filter(t => (t.track_name || '').toLowerCase().includes(query) || @@ -1379,23 +1436,44 @@ function renderWingItPoolList() { ); } if (tracks.length === 0) { + const emptyMsg = isMatched + ? 'No resolved Wing It tracks yet — ones you Fix here will land in this list.' + : 'No Wing It guesses to review.'; container.innerHTML = query - ? '
No Wing It tracks match your filter.
' - : '
No Wing It tracks — nothing was auto-matched on a guess.
'; + ? '
No tracks match your filter.
' + : `
${emptyMsg}
`; return; } - container.innerHTML = tracks.map(t => ` -
-
-
${_esc(t.track_name)}
-
- ${_esc(t.artist_name)} - ${_esc(t.playlist_name)} + container.innerHTML = tracks.map(t => { + if (isMatched) { + const matchedName = _wingItMatchedName(t); + return ` +
+
+
${_esc(t.track_name)}
+
+ ${_esc(t.artist_name)} + ${matchedName ? `→${_esc(matchedName)}` : ''} + ${_esc(t.playlist_name)} +
+
+
+ `; + } + return ` +
+
+
${_esc(t.track_name)}
+
+ ${_esc(t.artist_name)} + ${_esc(t.playlist_name)} +
+
+
- -
- `).join(''); + `; + }).join(''); } async function filterWingItPool(playlistId) { @@ -1409,13 +1487,23 @@ async function filterWingItPool(playlistId) { showToast('Failed to filter Wing It pool', 'error'); return; } + _updateWingItHeaderCounts(); + if (_wingItPoolView === 'categories') return; renderWingItPoolList(); - const countEl = document.getElementById('wing-it-header-count'); - if (countEl) { - const c = (_wingItPoolData.tracks || []).length; - countEl.textContent = `${c} to review`; - countEl.classList.toggle('pool-header-failed-highlight', c > 0); - } +} + +function _updateWingItHeaderCounts() { + if (!_wingItPoolData) return; + const a = (_wingItPoolData.tracks || []).length; + const m = (_wingItPoolData.matched || []).length; + const aEl = document.getElementById('wing-it-header-attention'); + const mEl = document.getElementById('wing-it-header-matched'); + const aCat = document.getElementById('wing-it-cat-attention-count'); + const mCat = document.getElementById('wing-it-cat-matched-count'); + if (aEl) { aEl.textContent = `${a} to review`; aEl.classList.toggle('pool-header-failed-highlight', a > 0); } + if (mEl) mEl.textContent = `${m} resolved`; + if (aCat) aCat.textContent = a; + if (mCat) mCat.textContent = m; } // Re-fetch + re-render the open Wing It pool (used after a Fix Match resolves a track). From 086d153d775c1e82dcc7deabf725f8f0cea19fb9 Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Thu, 25 Jun 2026 15:40:17 -0700 Subject: [PATCH 07/49] Multi-disc (#927): capture real disc number at media-server scan time MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every library track was stored with disc_number=1 because the Jellyfin/Plex/Navidrome scan parsed the track number but never the disc field. Multi-disc albums collapsed onto disc 1, so disc-2+ tracks were mis-filed (shown under disc 1) and flagged 'missing' — the frontend title-fallback band-aid couldn't recover it (breaks on iTunes title mismatches). Now the shared insert_or_update_media_track reads the disc number (Jellyfin .discNumber=ParentIndexNumber, Navidrome .discNumber, Plex .parentIndex), floors to >=1, and stores it in the INSERT + UPDATE. The disc_number column is ensured on init (it was only added by a migration that doesn't run on fresh installs, so the new INSERT would have hard-failed for new users). The enhanced album view already carries disc_number through (SELECT * -> dict), so the display fixes itself once the column is populated — a re-scan backfills existing libraries. Seam-tested across Jellyfin/Navidrome/Plex shapes + the floor-to-1 + re-scan-update cases. --- core/jellyfin_client.py | 1 + core/navidrome_client.py | 1 + database/music_database.py | 30 ++++++++++-- tests/test_media_track_disc_number.py | 68 +++++++++++++++++++++++++++ 4 files changed, 95 insertions(+), 5 deletions(-) create mode 100644 tests/test_media_track_disc_number.py diff --git a/core/jellyfin_client.py b/core/jellyfin_client.py index be4a7a26..fa4f61b0 100644 --- a/core/jellyfin_client.py +++ b/core/jellyfin_client.py @@ -105,6 +105,7 @@ class JellyfinTrack: self.title = jellyfin_data.get('Name', 'Unknown Track') self.duration = jellyfin_data.get('RunTimeTicks', 0) // 10000 # Convert from ticks to milliseconds self.trackNumber = jellyfin_data.get('IndexNumber') + self.discNumber = jellyfin_data.get('ParentIndexNumber') # multi-disc: disc number self.year = jellyfin_data.get('ProductionYear') self.userRating = jellyfin_data.get('UserData', {}).get('Rating') self.addedAt = self._parse_date(jellyfin_data.get('DateCreated')) diff --git a/core/navidrome_client.py b/core/navidrome_client.py index 98536950..a51f5b2b 100644 --- a/core/navidrome_client.py +++ b/core/navidrome_client.py @@ -110,6 +110,7 @@ class NavidromeTrack: self.title = navidrome_data.get('title', 'Unknown Track') self.duration = navidrome_data.get('duration', 0) * 1000 # Convert to milliseconds self.trackNumber = navidrome_data.get('track') + self.discNumber = navidrome_data.get('discNumber') # multi-disc: disc number self.year = navidrome_data.get('year') self.userRating = navidrome_data.get('userRating') self.addedAt = self._parse_date(navidrome_data.get('created')) diff --git a/database/music_database.py b/database/music_database.py index 3427dc64..dd37aee3 100644 --- a/database/music_database.py +++ b/database/music_database.py @@ -1171,6 +1171,13 @@ class MusicDatabase: if track_cols and 'year' not in track_cols: cursor.execute("ALTER TABLE tracks ADD COLUMN year INTEGER") logger.info("Repaired missing year column on tracks table (#910)") + # #927 — multi-disc fix: the scan now writes a real disc_number, but the column + # was only ever added by a separate migration that doesn't run on fresh installs, + # so the new INSERT/UPDATE would hard-fail with "no column named disc_number". + # Same shape as the year repair above: additive, defaults to 1, ensured on every DB. + if track_cols and 'disc_number' not in track_cols: + cursor.execute("ALTER TABLE tracks ADD COLUMN disc_number INTEGER DEFAULT 1") + logger.info("Repaired missing disc_number column on tracks table (#927)") cursor.execute("PRAGMA table_info(albums)") album_cols = {c[1] for c in cursor.fetchall()} @@ -6635,6 +6642,19 @@ class MusicDatabase: track_id = str(track_obj.ratingKey) title = track_obj.title track_number = getattr(track_obj, 'trackNumber', None) + # Multi-disc: capture the disc number so multi-disc albums don't all + # collapse onto disc 1 (which mis-files disc-2+ tracks and flags them + # "missing"). Jellyfin/Navidrome wrappers set .discNumber; plexapi's Track + # exposes .parentIndex. Floor to >=1 — a missing/0 disc is disc 1. + _raw_disc = getattr(track_obj, 'discNumber', None) + if _raw_disc is None: + _raw_disc = getattr(track_obj, 'parentIndex', None) + try: + disc_number = int(_raw_disc) + if disc_number < 1: + disc_number = 1 + except (TypeError, ValueError): + disc_number = 1 duration = getattr(track_obj, 'duration', None) # Get file path and media info (Plex-specific, Jellyfin may not have these) @@ -6726,9 +6746,9 @@ class MusicDatabase: if is_new_track: cursor.execute(""" INSERT INTO tracks - (id, album_id, artist_id, title, track_number, duration, file_path, bitrate, file_size, server_source, track_artist, musicbrainz_recording_id, updated_at) - VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, CURRENT_TIMESTAMP) - """, (track_id, album_id, artist_id, title, track_number, duration, file_path, bitrate, file_size, server_source, track_artist, mbid)) + (id, album_id, artist_id, title, track_number, disc_number, duration, file_path, bitrate, file_size, server_source, track_artist, musicbrainz_recording_id, updated_at) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, CURRENT_TIMESTAMP) + """, (track_id, album_id, artist_id, title, track_number, disc_number, duration, file_path, bitrate, file_size, server_source, track_artist, mbid)) else: # Update server-provided fields only — preserves spotify_track_id, deezer_id, # isrc, bpm, and all other enrichment data. file_size uses @@ -6737,7 +6757,7 @@ class MusicDatabase: # an existing value. cursor.execute(""" UPDATE tracks - SET album_id = ?, artist_id = ?, title = ?, track_number = ?, + SET album_id = ?, artist_id = ?, title = ?, track_number = ?, disc_number = ?, duration = ?, file_path = ?, bitrate = ?, file_size = COALESCE(?, file_size), server_source = ?, @@ -6745,7 +6765,7 @@ class MusicDatabase: musicbrainz_recording_id = COALESCE(?, musicbrainz_recording_id), updated_at = CURRENT_TIMESTAMP WHERE id = ? - """, (album_id, artist_id, title, track_number, duration, file_path, bitrate, file_size, server_source, track_artist, mbid, track_id)) + """, (album_id, artist_id, title, track_number, disc_number, duration, file_path, bitrate, file_size, server_source, track_artist, mbid, track_id)) conn.commit() diff --git a/tests/test_media_track_disc_number.py b/tests/test_media_track_disc_number.py new file mode 100644 index 00000000..6bbc4af1 --- /dev/null +++ b/tests/test_media_track_disc_number.py @@ -0,0 +1,68 @@ +"""Multi-disc fix (#927): the media-server scan must store the real disc number. + +Every track was stored with disc_number=1 because the Jellyfin/Plex/Navidrome scan never +read the disc field — so multi-disc albums collapsed onto disc 1, mis-filing disc-2+ tracks +and flagging them "missing". insert_or_update_media_track now reads the disc number off the +track object (Jellyfin/Navidrome `.discNumber`, Plex `.parentIndex`), floored to >=1. +""" + +from __future__ import annotations + +from types import SimpleNamespace + +from database.music_database import MusicDatabase + + +def _db(tmp_path): + db = MusicDatabase(database_path=str(tmp_path / "t.db")) + with db._get_connection() as c: + c.execute("INSERT OR IGNORE INTO artists (id, name) VALUES ('art1', 'Evan Call')") + c.execute("INSERT OR IGNORE INTO albums (id, artist_id, title) VALUES ('alb1', 'art1', 'Frieren OST')") + c.commit() + return db + + +def _track(rating_key, title, track_number, **kw): + return SimpleNamespace(ratingKey=rating_key, title=title, trackNumber=track_number, + duration=200000, **kw) + + +def _disc_of(db, track_id): + with db._get_connection() as c: + row = c.execute("SELECT disc_number, track_number FROM tracks WHERE id = ?", (track_id,)).fetchone() + return (row['disc_number'], row['track_number']) + + +def test_jellyfin_navidrome_disc_number_stored(tmp_path): + db = _db(tmp_path) + # .discNumber is what the Jellyfin (ParentIndexNumber) + Navidrome (discNumber) wrappers set. + db.insert_or_update_media_track(_track('t-d2', 'Waltz for Stark and Fern', 34, discNumber=2), 'alb1', 'art1', 'jellyfin') + db.insert_or_update_media_track(_track('t-d1', 'The Magic Within', 32, discNumber=1), 'alb1', 'art1', 'jellyfin') + assert _disc_of(db, 't-d2') == (2, 34) + assert _disc_of(db, 't-d1') == (1, 32) + + +def test_plex_parent_index_used_as_disc(tmp_path): + db = _db(tmp_path) + # plexapi Track has no .discNumber — disc comes from .parentIndex. + db.insert_or_update_media_track(_track('t-plex', 'Disc 2 Track', 5, parentIndex=2), 'alb1', 'art1', 'plex') + assert _disc_of(db, 't-plex') == (2, 5) + + +def test_missing_or_bad_disc_floors_to_one(tmp_path): + db = _db(tmp_path) + db.insert_or_update_media_track(_track('t-none', 'No Disc', 1), 'alb1', 'art1', 'jellyfin') # no disc attr + db.insert_or_update_media_track(_track('t-zero', 'Zero Disc', 2, discNumber=0), 'alb1', 'art1', 'jellyfin') + db.insert_or_update_media_track(_track('t-str', 'Junk Disc', 3, discNumber='x'), 'alb1', 'art1', 'jellyfin') + assert _disc_of(db, 't-none')[0] == 1 + assert _disc_of(db, 't-zero')[0] == 1 + assert _disc_of(db, 't-str')[0] == 1 + + +def test_update_path_backfills_disc_on_rescan(tmp_path): + db = _db(tmp_path) + # First scan (old behavior simulated): no disc -> 1. Re-scan with the real disc -> updated. + db.insert_or_update_media_track(_track('t-x', 'Track', 7), 'alb1', 'art1', 'jellyfin') + assert _disc_of(db, 't-x') == (1, 7) + db.insert_or_update_media_track(_track('t-x', 'Track', 7, discNumber=3), 'alb1', 'art1', 'jellyfin') + assert _disc_of(db, 't-x') == (3, 7) From 30ff0bde4933906189415c066b0c23d2e1188162 Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Thu, 25 Jun 2026 16:11:51 -0700 Subject: [PATCH 08/49] Release 2.7.9: version bump + What's New / version modal + PR description + docker-publish default tag - _SOULSYNC_BASE_VERSION -> 2.7.9; docker-publish default version_tag -> 2.7.9 - pr_description.md rewritten for 2.7.9 (best-quality downloads + quality profile #896, Discover listening recs + Listening Mix #913, Wing It Pool, Auto-Sync lane redesign, multi-disc #927, sync labels #925, post-processing race #928) - WHATS_NEW: 2.7.8 block -> 2.7.9 (+ brief earlier-versions); VERSION_MODAL_SECTIONS: 2.7.9 highlights promoted, 2.7.8 rolled into an Earlier aggregator - RELEASE_2.7.9_discord.md: mini Discord post --- .github/workflows/docker-publish.yml | 4 +- RELEASE_2.7.9_discord.md | 13 +++++ pr_description.md | 55 +++++++++++------- web_server.py | 2 +- webui/static/helper.js | 85 ++++++++++++++++++---------- 5 files changed, 104 insertions(+), 55 deletions(-) create mode 100644 RELEASE_2.7.9_discord.md diff --git a/.github/workflows/docker-publish.yml b/.github/workflows/docker-publish.yml index a08b63a5..91daceac 100644 --- a/.github/workflows/docker-publish.yml +++ b/.github/workflows/docker-publish.yml @@ -9,9 +9,9 @@ on: workflow_dispatch: inputs: version_tag: - description: 'Version tag (e.g. 2.7.8)' + description: 'Version tag (e.g. 2.7.9)' required: true - default: '2.7.8' + default: '2.7.9' jobs: build-and-push: diff --git a/RELEASE_2.7.9_discord.md b/RELEASE_2.7.9_discord.md new file mode 100644 index 00000000..80651c39 --- /dev/null +++ b/RELEASE_2.7.9_discord.md @@ -0,0 +1,13 @@ +**SoulSync 2.7.9** is out 🎉 a big one. + +🎚️ **Best-quality downloads** — downloads now follow a ranked quality profile you drag to order (FLAC 24/192 → mp3). best-quality mode grabs the highest-quality copy across *every* source; priority mode gets an opt-in rank-based order toggle. quarantine is folded into the Downloads page + safer imports (AcoustID fail-closed, silence/truncation guards). + +🎧 **Discover got smart** — "Based On Your Listening" ranks artists from who you *actually play*, and "Your Listening Mix" is a playable track playlist of their top tracks (works on any source). Fresh Tape fills properly now. + +⚡ **Wing It Pool** — a new spot next to Discovery Pool to review + re-match the tracks Wing It guessed at (they used to be invisible). + +🔁 **Auto-Sync redesign** — the scheduling board is now clean horizontal lanes instead of a side-scrolling column wall. + +🐛 **Fixes** — multi-disc albums no longer show disc-2 as "missing" (#927), playlists no longer stuck on "Never Synced" (#925), and tracks can't import while quarantined (#928). thanks @ramonskie + @nick2000713 🙏 + +⚠️ **re-scan your library once** so the multi-disc fix can backfill disc numbers on existing tracks. enjoy! 🎶 diff --git a/pr_description.md b/pr_description.md index 3b270a6e..c2e865ac 100644 --- a/pr_description.md +++ b/pr_description.md @@ -1,38 +1,49 @@ -# soulsync 2.7.8 — `dev` → `main` +# soulsync 2.7.9 — `dev` → `main` -a feature patch on top of 2.7.7 — playlists can now be put back in order on the server, you can re-wishlist a missed track straight from sync history, plus a couple of reported fixes. +a big one. the headline is the new **best-quality download system + a real quality profile**, plus a much smarter **Discover** page, a new **Wing It Pool**, a redesigned **Auto-Sync** board, and a pile of reported fixes (multi-disc albums, playlist sync labels, the import-vs-quarantine race). --- ## what's new -### align playlists — server order, not just contents -the server-playlist editor only ever cared about *which* tracks were on the server, never their order — and it rendered the server column in the source's order, so a playlist with the right tracks in the wrong sequence read as "in sync" when it wasn't. now it tells the truth: -- an **"out of order"** badge appears when the tracks match but the sequence differs (relative order, so missing/extra tracks don't false-flag it), and a **read-only view** shows the server's *actual* order with cover art. -- a new **"Align playlists"** action reorders the server playlist to match the source — **Plex** (in-place via moveItem), **Navidrome** (ordered rewrite), and **Jellyfin** (Move endpoint), all of which preserve the playlist's identity/poster. two choices for server-only extras: **mirror source** (drop them) or **keep extras** (park them at the end). it's order-only — it never adds the missing tracks (that's a normal sync's job) and never touches metadata, just reshuffles ids already on the server. +### best-quality downloads + a real quality profile +downloads are now driven by a **ranked-target quality profile** instead of a fixed preference. you order the formats you want (drag to reorder — FLAC 24/192 down to mp3, every format controllable, with "all lossless / all lossy" group shortcuts), and: +- **best-quality search mode** pools candidates across *every* source per query and grabs the highest-quality copy that meets your profile — not just the first/fastest match. priority mode is still there, now with an opt-in **"rank-based download order"** toggle if you want quality-first ordering there too. +- each streaming source's download tier is derived from the one global profile, so you set it once. +- AAC is an opt-in tier; old per-source Hi-Res preferences migrate into the profile automatically. -### re-add to wishlist from sync history -in the dashboard's **Recent Syncs → details**, the "→ Wishlist" status on an unmatched track is now a button — click it to re-add that exact track to the wishlist with the **same context the sync used** (source playlist, cover art, everything), so it's indistinguishable from the original auto-add. the re-add and the live sync now build the *identical* payload from one shared path, so the cover and album/single classification carry through. wing-it fallback stubs (tracks that couldn't be resolved to real metadata) are correctly shown as **"Unmatched"** and aren't re-addable — matching what the sync itself does. +### quarantine, cleaned up + safer +- the quarantine view is **consolidated into the Downloads page** as a filter (no separate place to check), with real audio quality shown on the rows and approve/retry handled inline. +- **AcoustID fail-closed mode** (opt-in): only import tracks that actually verify, so a wrong file never lands in your library. +- **silence + truncated-download guards** catch mostly-silent preview files and downloads that are shorter than their container claims, before they import. +- a **library quality check** runs as a repair job and can flag files that are upgradeable to your preferred quality. -### fixes -- **import search said "Deezer" for Spotify Free users (#922)** — manual album-import told no-auth Spotify users that Deezer was their primary source. the functional source legitimately downgrades to a working fallback (the free path has no album-name search), but the *label* should name what you configured. now it reads "Spotify." -- **iTunes albums >50 tracks could still truncate (#918 follow-up)** — the limit=200 fix only helped fresh fetches; albums cached at 50 before the fix kept serving 50 from the persistent cache. now a cached tracklist shorter than the album's known track count self-heals on next load. +### Discover got a lot smarter +- **"Based On Your Listening"** — a new artist row, ranked from who you actually *play* the most (consensus + recency weighted), with a "because you listen to X, Y" reason on each card. +- **"Your Listening Mix"** — a playable track playlist built from those artists' top tracks. works on **any** metadata source (falls back to Deezer's public API), not just Spotify. +- **Fresh Tape** actually fills now — it was starving down to 5–10 tracks because future-dated albums ate the candidate budget. +- the **SoulSync Discovery** tab on the Sync page now lists *every* playlist kind (incl. the Listening Mix) so you can mirror + auto-sync them. -### under the hood -- `.gitignore` now covers **all** `database/*.db` (+ wal/shm/backup), not just `music_library` — so the video db and any future db can't be committed by accident. +### Wing It Pool +a new button next to **Discovery Pool** on the Mirrored Playlists tab. Wing It auto-matches tracks it couldn't match to metadata on a best-effort guess — those were invisible until now. the Wing It Pool opens to a two-card view (**guesses to review** + **resolved**) so you can verify or re-match what it guessed. + +### Auto-Sync Manager redesign +the scheduling board no longer scrolls sideways through a wall of columns. intervals (hourly) and days (weekly) are now **horizontal lanes** — empty ones collapse, busy ones grow, and the scroll position holds when you add a playlist. --- -## a brief recap of what came before -2.7.7 was a fix-heavy patch — the metadata-parity fix so downloads tag + path right without a manual reorganize (#915), the listening-recs foundation (#913), jellyfin atomic writes, and a big reported-issue sweep (#905/#908–#912/#914/#916–#918). 2.7.6 exported playlists TO listenbrainz + youtube liked-music sync; 2.7.5 matching & artwork accuracy; 2.7.4 re-identify; 2.7.3 the Quality Upgrade Finder; 2.7.2 playlist-folder mirroring; 2.7.1 download verification; 2.7.0 made multi-user real. +## fixes + +- **multi-disc albums showed disc-2 tracks as "missing" / under disc 1 (#927)** — the library scan never read the disc number, so every track was stored as disc 1. it now captures the real disc from Jellyfin/Plex/Navidrome at scan time. *(re-scan your library once to backfill existing tracks.)* +- **playlists always said "Never Synced" (#925)** — auto-synced/mirrored playlists were only checked against the direct-sync status, never their auto-sync status. fixed (thanks @ramonskie). +- **tracks imported while quarantined / shown "completed" (#928)** — a race let both the browser poll and the download monitor post-process the same finished download. an atomic claim now ensures exactly one path handles it (thanks @nick2000713). +- **library card badges hijacked the click** — clicking the watchlist eye or a source badge on an artist card also opened the artist detail page (and the badge's own link). badges now do only their own thing. --- -## tests -additive + scoped — the new write paths are their own routes that don't touch the normal sync. new seam/regression suites for the order-status detection (incl. the reported "moved to #2" case + missing/extra false-flag guards), the pure align-rewrite planner (mirror vs keep-extras, never-injects-a-foreign-track, stale-data rejection), the sync re-add payload (a direct parity assertion that the re-add == the live-sync payload, plus the wing-it skip), and the `get_primary_source_label` fix (#922). iTunes self-heal proven against the real persistent-cache shape. relevant suites green; `ruff check` clean. +## under the hood +- music automations page no longer shows video-app automations (they live in the shared engine DB). +- quality-settings tile tidied up — collapsible ⓘ help instead of walls of text, proper reset button, dropped the redundant per-source "quality is global" notes. +- download clients don't crash on init when the download path can't be created. -## post-merge -- [ ] tag `v2.7.8` on `main` -- [ ] docker-publish with `version_tag: 2.7.8` -- [ ] discord announce (auto-fired by the workflow) -- [ ] reply on #922 and the #918 follow-up +--- diff --git a/web_server.py b/web_server.py index 21f594af..b87f2ba7 100644 --- a/web_server.py +++ b/web_server.py @@ -40,7 +40,7 @@ logger = setup_logging(_log_level, _log_path) # App version — single source of truth for backup metadata, system-info, update check, etc. # Semver: MAJOR.MINOR.PATCH. Bump at each dev→main release. -_SOULSYNC_BASE_VERSION = "2.7.8" +_SOULSYNC_BASE_VERSION = "2.7.9" def _build_version_string(): """Append short commit hash to version when available (e.g. 2.35+abc1234).""" diff --git a/webui/static/helper.js b/webui/static/helper.js index fdcc78a7..ddcac5cd 100644 --- a/webui/static/helper.js +++ b/webui/static/helper.js @@ -3404,13 +3404,19 @@ function closeHelperSearch() { const WHATS_NEW = { // Convention: keep only the CURRENT release here, plus a single brief // "Earlier versions" summary entry. Don't accumulate old per-version blocks. - '2.7.8': [ - { date: 'June 2026 — 2.7.8 release' }, - { title: 'Align playlists — fix the order on the server', desc: 'the server-playlist editor only cared about WHICH tracks were on the server, never their order — and it drew the server column in the source\'s order, so a right-tracks-wrong-sequence playlist read as "in sync" when it wasn\'t. now an "out of order" badge appears when the tracks match but the sequence differs (relative order, so missing/extra don\'t false-flag), with a read-only view of the server\'s ACTUAL order. and a new "Align playlists" action reorders the server to match the source — Plex, Navidrome and Jellyfin, all preserving the playlist\'s identity/poster. order-only: never adds missing tracks, never touches metadata.', page: 'sync' }, - { title: 'Re-add to wishlist from sync history', desc: 'in Recent Syncs → details, the "→ Wishlist" status on an unmatched track is now a button — click it to re-add that exact track with the SAME context the sync used (source playlist, cover art, everything). the re-add and the live sync build the identical payload from one shared path, so the cover and album/single classification carry through. wing-it stubs (couldn\'t be resolved) show as "Unmatched" and aren\'t re-addable, matching the sync.', page: 'dashboard' }, - { title: 'Import search said "Deezer" for Spotify Free (#922)', desc: 'manual album-import told no-auth Spotify users that Deezer was their primary source. the functional source legitimately falls back (the free path has no album-name search), but the label should name what you configured — now it reads "Spotify".', page: 'import' }, - { title: 'iTunes albums over 50 still truncating (#918 follow-up)', desc: 'the limit=200 fix only helped fresh fetches; albums cached at 50 before it kept serving 50 from the persistent cache. now a cached tracklist shorter than the album\'s known track count self-heals on next load.', page: 'library' }, - { title: 'Earlier versions', desc: '2.7.7 was fix-heavy — downloads tag + path right without a manual reorganize (#915), the listening-recs foundation (#913), jellyfin atomic writes, and a big reported-issue sweep (#905/#908–#912/#914/#916–#918). 2.7.6 exported playlists TO listenbrainz + youtube liked-music sync; 2.7.5 matching & artwork accuracy; 2.7.4 re-identify; 2.7.3 the Quality Upgrade Finder; 2.7.2 playlist-folder mirroring; 2.7.1 download verification; 2.7.0 made multi-user real.' }, + '2.7.9': [ + { date: 'June 2026 — 2.7.9 release' }, + { title: 'Best-quality downloads + a real quality profile', desc: 'downloads now follow a ranked-target quality profile — drag to order every format (FLAC 24/192 → mp3, with "all lossless / all lossy" shortcuts). best-quality search mode pools candidates across EVERY source per query and grabs the highest-quality copy that meets your profile; priority mode gains an opt-in "rank-based download order" toggle. each streaming source\'s tier comes from the one profile, AAC is an opt-in tier, and old per-source Hi-Res prefs migrate in automatically.', page: 'settings' }, + { title: 'Quarantine cleaned up + safer imports', desc: 'the quarantine view is folded into the Downloads page as a filter (real audio quality on the rows, inline approve/retry). opt-in AcoustID fail-closed mode only imports tracks that actually verify; silence + truncated-download guards catch preview/short files before they import; a library quality check runs as a repair job and flags upgradeable files.', page: 'downloads' }, + { title: 'Discover — Based On Your Listening + Listening Mix', desc: 'a new artist row ranked from who you actually PLAY the most (consensus + recency weighted, with a "because you listen to X" reason), plus "Your Listening Mix" — a playable track playlist of those artists\' top tracks that works on ANY metadata source (Deezer public fallback). the SoulSync Discovery sync tab now lists every playlist kind so you can mirror + auto-sync them.', page: 'discover' }, + { title: 'Fresh Tape fills properly now', desc: 'it was starving down to 5–10 tracks because future-dated/announced albums sorted to the top and ate the candidate budget before being skipped. released albums now fill it.', page: 'discover' }, + { title: 'Wing It Pool', desc: 'a new button next to Discovery Pool on the Mirrored Playlists tab. Wing It auto-matches tracks it couldn\'t match to metadata on a best-effort guess — previously invisible. the Wing It Pool shows a two-card view (guesses to review + resolved) so you can verify or re-match them.', page: 'sync' }, + { title: 'Auto-Sync Manager redesign', desc: 'the scheduling board no longer scrolls sideways through a wall of columns — intervals (hourly) and days (weekly) are now horizontal lanes; empty ones collapse, busy ones grow, and the scroll holds when you drop a playlist in.', page: 'sync' }, + { title: 'Multi-disc albums fixed (#927)', desc: 'the scan never read the disc number, so every track stored as disc 1 — disc-2+ tracks showed as "missing" or under disc 1. it now captures the real disc from Jellyfin/Plex/Navidrome at scan time. re-scan once to backfill existing tracks.', page: 'library' }, + { title: 'Playlists always said "Never Synced" (#925)', desc: 'auto-synced/mirrored playlists were only checked against the direct-sync status, never their auto-sync status, so they read "Never Synced" even when synced. fixed (thanks @ramonskie).', page: 'sync' }, + { title: 'Tracks imported while quarantined (#928)', desc: 'a race let both the browser poll and the download monitor post-process the same finished download — imported despite quarantine, or "completed" while quarantined. an atomic claim now ensures exactly one path handles it (thanks @nick2000713).', page: 'downloads' }, + { title: 'Library card badges hijacked the click', desc: 'clicking the watchlist eye or a source badge on an artist card also opened the artist detail page (and the badge\'s own link). badges now do only their own thing — only the card opens detail.', page: 'library' }, + { title: 'Earlier versions', desc: '2.7.8 added Align Playlists (fix the order on the server) + re-add-to-wishlist from sync history, plus the #922 Spotify-Free label and #918 iTunes-cache fixes. 2.7.7 was a fix sweep; 2.7.6 listenbrainz/youtube export; 2.7.3 the Quality Upgrade Finder; 2.7.0 made multi-user real.' }, ], }; @@ -3441,47 +3447,66 @@ const WHATS_NEW = { // usage_note?: 'optional hint shown at the bottom' } const VERSION_MODAL_SECTIONS = [ { - title: "Align playlists — fix the order on the server", - description: "the headline — the server-playlist editor can now put a playlist back in the SOURCE's order, not just sync which tracks are on it.", + title: "Best-quality downloads + a real quality profile", + description: "the headline — downloads now follow a ranked-target quality profile instead of a fixed preference.", features: [ - "an \"out of order\" badge when the tracks match but the sequence differs (relative order, so missing/extra tracks don't false-flag it), plus a read-only view of the server's ACTUAL order with cover art", - "a new \"Align playlists\" action reorders the server to match the source — Plex, Navidrome and Jellyfin, all preserving the playlist's identity/poster", - "two choices for server-only extras (mirror source = drop them, or keep extras at the end); order-only — never adds missing tracks, never touches metadata", + "drag to order the formats you want (FLAC 24/192 → mp3, every format controllable, with \"all lossless / all lossy\" group shortcuts)", + "best-quality search mode pools candidates across EVERY source per query and grabs the highest-quality copy that meets your profile — not just the first/fastest match", + "priority mode keeps its behavior, with an opt-in \"rank-based download order\" toggle; each streaming source's tier comes from the one profile; AAC is an opt-in tier, and old per-source Hi-Res prefs migrate in", + "quarantine folded into the Downloads page (real quality on the rows, inline approve/retry), opt-in AcoustID fail-closed mode, and silence/truncation guards that catch preview + short files before import", ], }, { - title: "Re-add to wishlist from sync history", - description: "in Recent Syncs → details, re-wishlist a track the sync couldn't place — with the exact same context.", + title: "Discover, a lot smarter", + description: "Discover now learns from what you actually play.", features: [ - "the \"→ Wishlist\" status on an unmatched track is now a button; click it to re-add that exact track with the SAME context the sync used (source playlist, cover art, everything)", - "the re-add and the live sync build the identical payload from one shared path, so the cover and album/single classification carry through", - "wing-it stubs (couldn't be resolved to real metadata) show as \"Unmatched\" and aren't re-addable, matching what the sync does", + "\"Based On Your Listening\" — a new artist row ranked by who you play the most (consensus + recency weighted), with a \"because you listen to X, Y\" reason on each card", + "\"Your Listening Mix\" — a playable track playlist from those artists' top tracks; works on ANY metadata source (falls back to Deezer's public API), not just Spotify", + "Fresh Tape fills properly now — it was starving to 5–10 tracks because future-dated albums ate the candidate budget; and the SoulSync Discovery sync tab lists every playlist kind so you can mirror + auto-sync them", + ], + }, + { + title: "Wing It Pool", + description: "a place to review the tracks Wing It guessed at — they used to be invisible.", + features: [ + "a new button next to Discovery Pool on the Mirrored Playlists tab; Wing It auto-matches tracks it couldn't match to metadata on a best-effort guess", + "opens to a two-card view — guesses to review + resolved — so you can verify or re-match what it guessed (re-match reuses the same fix flow as Discovery Pool)", + ], + }, + { + title: "Auto-Sync Manager redesign", + description: "the scheduling board no longer scrolls sideways through a wall of columns.", + features: [ + "intervals (hourly) and days (weekly) are now horizontal lanes — empty ones collapse to thin strips, busy ones grow", + "the scroll position holds when you drop a playlist in (it used to snap back to the start every time)", ], }, { title: "Recent fixes", - description: "a couple of reported fixes.", + description: "reported bugs squashed this cycle.", features: [ - "#922 — manual import told Spotify Free users their primary source was \"Deezer\"; the search legitimately falls back (the free path can't search albums by name), but the label now reads \"Spotify\"", - "#918 follow-up — iTunes albums over 50 tracks could still show 50 from a stale cache; a cached tracklist shorter than the album's known track count now self-heals on next load", + "#927 — multi-disc albums showed disc-2 tracks as \"missing\" or under disc 1; the scan never read the disc number. it now captures the real disc from Jellyfin/Plex/Navidrome at scan time (re-scan once to backfill existing tracks)", + "#925 — auto-synced/mirrored playlists always read \"Never Synced\" because only the direct-sync status was checked, never the auto-sync one (thanks @ramonskie)", + "#928 — a race let both the browser poll and the download monitor post-process the same finished download, so a track could import while quarantined; an atomic claim fixes it (thanks @nick2000713)", + "library artist-card badges (the watchlist eye + source links) no longer hijack the click into the artist detail page — they do only their own thing", ], }, { - title: "Earlier in 2.7.7", - description: "2.7.7 was fix-heavy — downloads tag + path right the first time without a manual reorganize (#915), the listening-recs foundation (#913), jellyfin atomic writes, and a big reported-issue sweep.", + title: "Earlier in 2.7.8", + description: "2.7.8 was about playlist order + a couple of reported fixes.", features: [ - "#915 — post-processing + redownload now pull the full album from your PRIMARY metadata source, so the $year, real release date and album type land right the first time", - "#913 — listening-driven recommendations: ranks artists you'd love but don't own, scored by consensus from your top-played", - "#905/#908/#911/#918/#916/#914/#917/#910/#909/#912 — navidrome playlists doubling, youtube ~100 cap, wrong redownload edition, iTunes 50-track truncation, multi-disc shown missing, and the rest of the batch", + "Align playlists — reorder the server playlist to match the source (Plex/Navidrome/Jellyfin), with an \"out of order\" badge; order-only, never adds missing tracks", + "re-add a missed track to the wishlist straight from Recent Syncs → details, with the exact same context the sync used", + "#922 — import label said \"Deezer\" for Spotify Free users (now reads \"Spotify\"); #918 — iTunes albums over 50 tracks self-heal from a stale 50-track cache", ], }, { - title: "Earlier in 2.7.6 / 2.7.5", - description: "2.7.6 went the OTHER way with playlists — exporting them TO listenbrainz (#903) — plus youtube liked-music sync (#902), a deep-scan data-loss guard (#904), and dashboard perf. before that, 2.7.5 was a fix-heavy cycle — matching & artwork accuracy plus a few quality-of-life features.", + title: "Earlier in 2.7.7 / 2.7.6 / 2.7.5", + description: "2.7.7 was fix-heavy (downloads tag + path right the first time #915, the listening-recs foundation #913, a big reported-issue sweep). 2.7.6 exported playlists TO listenbrainz (#903) + youtube liked-music sync (#902); 2.7.5 was matching & artwork accuracy plus quality-of-life features.", features: [ - "special-edition cover art (a \"Gustave Edition\" uses its OWN cover), deezer track numbers, and the \"The\" duplicate fix", - "HiFi 30-second previews disguised as full songs are caught and rejected (#895)", - "import M3U / M3U8 playlists (#893), name files inside playlist folders, find & add remembered, ignore-list management (#897), Unraid template fixes (#899)", + "#915 — post-processing + redownload pull the full album from your PRIMARY metadata source, so $year / release date / album type land right the first time", + "HiFi 30-second previews disguised as full songs are caught and rejected (#895); special-edition cover art, deezer track numbers, the \"The\" duplicate fix", + "import M3U / M3U8 playlists (#893), ignore-list management (#897), Unraid template fixes (#899), and the rest of the #905–#918 batch", ], }, { From c033656fdfdea3efa9b83fcb26701a49d5195b97 Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Thu, 25 Jun 2026 16:30:52 -0700 Subject: [PATCH 09/49] Popular Picks: fix empty result for deezer (popularity threshold scale mismatch) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The discovery pool synthesizes deezer popularity onto a 0-100 score (base 45 + bonuses, capped at 100), but _get_popularity_thresholds had deezer on the raw-rank scale (500000/100000). So Popular Picks' 'popularity >= 500000' matched nothing — empty for every deezer-primary user — while Hidden Gems' '< 100000' caught the whole pool. Deezer thresholds now sit on the 0-100 scale (60/50, like Spotify's 60/40). Tested. --- core/personalized_playlists.py | 7 ++++-- tests/test_popularity_thresholds.py | 37 +++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 2 deletions(-) create mode 100644 tests/test_popularity_thresholds.py diff --git a/core/personalized_playlists.py b/core/personalized_playlists.py index 1f395eea..7b7b4738 100644 --- a/core/personalized_playlists.py +++ b/core/personalized_playlists.py @@ -111,7 +111,10 @@ class PersonalizedPlaylistsService: Either value can be None to skip that filter for that source. - Spotify: 60 / 40 (the existing 0-100 popularity scale) - - Deezer: 500000 / 100000 (rank values — ballpark from real data) + - Deezer: 60 / 50 (ALSO 0-100 — the discovery pool SYNTHESIZES deezer popularity to a + 0-100 score at scan time, capped at 100; it is NOT Deezer's raw rank. The old + 500000/100000 rank thresholds matched nothing, so Popular Picks came back empty for + deezer users while Hidden Gems' < 100000 caught the whole pool.) - iTunes / others: None / None (no popularity data; fall back to no threshold filter, just diversity-on-random) @@ -121,7 +124,7 @@ class PersonalizedPlaylistsService: if normalized == 'spotify': return 60, 40 if normalized == 'deezer': - return 500_000, 100_000 + return 60, 50 # iTunes, hydrabase, anything else — no usable popularity data return None, None diff --git a/tests/test_popularity_thresholds.py b/tests/test_popularity_thresholds.py new file mode 100644 index 00000000..c9842d01 --- /dev/null +++ b/tests/test_popularity_thresholds.py @@ -0,0 +1,37 @@ +"""Popular Picks was empty for deezer users — wrong popularity-threshold scale. + +The discovery pool synthesizes deezer popularity onto a 0-100 score (base 45 + bonuses, capped +at 100), but _get_popularity_thresholds had deezer on the raw-rank scale (500000/100000). So +`popularity >= 500000` matched nothing (Popular Picks empty) while `< 100000` matched the whole +pool (Hidden Gems caught everything). Thresholds must stay on the 0-100 scale. +""" + +from __future__ import annotations + +from core.personalized_playlists import PersonalizedPlaylistsService + + +def _svc(): + return PersonalizedPlaylistsService(database=None) + + +def test_deezer_thresholds_are_on_the_0_100_scale(): + popular_min, hidden_max = _svc()._get_popularity_thresholds('deezer') + assert (popular_min, hidden_max) == (60, 50) # was (500000, 100000) — unreachable + assert 0 < popular_min <= 100 and 0 < hidden_max <= 100 + assert popular_min > hidden_max # popular tier sits above the hidden tier + + +def test_spotify_thresholds_unchanged(): + assert _svc()._get_popularity_thresholds('spotify') == (60, 40) + + +def test_case_insensitive(): + assert _svc()._get_popularity_thresholds('Deezer') == (60, 50) + assert _svc()._get_popularity_thresholds('SPOTIFY') == (60, 40) + + +def test_sources_without_popularity_skip_the_filter(): + assert _svc()._get_popularity_thresholds('itunes') == (None, None) + assert _svc()._get_popularity_thresholds('musicbrainz') == (None, None) + assert _svc()._get_popularity_thresholds('') == (None, None) From 92025f5fb3b435fad2a7300772097d6a5d6a6186 Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Thu, 25 Jun 2026 16:41:00 -0700 Subject: [PATCH 10/49] lint: add strict=True to engine search zip() (B905) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit results comes from asyncio.gather over to_search, so they're always equal length — strict=True asserts the invariant and satisfies ruff B905. Carried in with #896. --- core/download_engine/engine.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/download_engine/engine.py b/core/download_engine/engine.py index 1d555432..de1aca38 100644 --- a/core/download_engine/engine.py +++ b/core/download_engine/engine.py @@ -480,7 +480,7 @@ class DownloadEngine: return_exceptions=True, ) - for (source_name, _), result in zip(to_search, results): + for (source_name, _), result in zip(to_search, results, strict=True): if isinstance(result, Exception): logger.warning(f"{source_name} search failed: {result}") contributions.append(f"{source_name}=error") From 580d9eb0f51a38cd192a6cb94698a1a1f3391118 Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Thu, 25 Jun 2026 16:55:53 -0700 Subject: [PATCH 11/49] test: correct deezer popularity-threshold test to the 0-100 scale MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit test_personalized_playlists_id_gate asserted the OLD (wrong) deezer thresholds (>=100000, raw-rank assumption) — the same bug fixed in c033656f. The discovery pool synthesizes deezer popularity to 0-100, so the test now asserts (60, 50). This is the CI failure from running the full suite (my -k subset missed it). --- tests/test_personalized_playlists_id_gate.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/tests/test_personalized_playlists_id_gate.py b/tests/test_personalized_playlists_id_gate.py index e19c7732..a7ca4adb 100644 --- a/tests/test_personalized_playlists_id_gate.py +++ b/tests/test_personalized_playlists_id_gate.py @@ -429,14 +429,15 @@ def test_popularity_thresholds_spotify_returns_60_40(): assert hidden_max == 40 -def test_popularity_thresholds_deezer_returns_higher_scale(): - """Deezer writes `rank` (raw integer, often 100k+) into the popularity - column, so thresholds must be in that range — Spotify's 60/40 would - classify almost every Deezer track as a hidden gem.""" +def test_popularity_thresholds_deezer_on_0_100_scale(): + """Corrected: the discovery pool SYNTHESIZES deezer popularity onto a 0-100 score (capped + at 100) at scan time — it is NOT Deezer's raw rank. The old 500000/100000 rank thresholds + matched nothing, so Popular Picks came back empty for deezer-primary users while Hidden + Gems' < 100000 caught the whole pool. Thresholds must stay on the 0-100 scale.""" svc = PersonalizedPlaylistsService(_FakeDatabase()) popular_min, hidden_max = svc._get_popularity_thresholds('deezer') - assert popular_min is not None and popular_min >= 100_000 - assert hidden_max is not None and hidden_max >= 50_000 + assert (popular_min, hidden_max) == (60, 50) + assert 0 < popular_min <= 100 and 0 < hidden_max <= 100 assert popular_min > hidden_max From 8a6b02b3fdeeefaad2b665479d2571e0bcabe0da Mon Sep 17 00:00:00 2001 From: ragnarlotus Date: Fri, 26 Jun 2026 03:10:07 +0200 Subject: [PATCH 12/49] test(reorganize): verify MusicBrainz integration --- tests/test_reorganize_canonical_source.py | 79 ++++++++++++++++++++--- 1 file changed, 69 insertions(+), 10 deletions(-) diff --git a/tests/test_reorganize_canonical_source.py b/tests/test_reorganize_canonical_source.py index bb39c436..f7a2939b 100644 --- a/tests/test_reorganize_canonical_source.py +++ b/tests/test_reorganize_canonical_source.py @@ -7,7 +7,11 @@ canonical_source/canonical_album_id, and an explicit user source pick from __future__ import annotations +from unittest.mock import MagicMock + import core.library_reorganize as lr +import core.metadata.registry as metadata_registry +from core.musicbrainz_search import MusicBrainzSearchClient def _patch_fetch(monkeypatch, tracklists): @@ -76,24 +80,79 @@ def test_no_canonical_unchanged(monkeypatch): def test_musicbrainz_release_id_is_used_by_priority_walk(monkeypatch): - _patch_fetch(monkeypatch, { - ('musicbrainz', 'mb-release-1'): [{'name': 'x'}], - }) + client = MusicBrainzSearchClient() + client._client = MagicMock() + client._client.get_release_group.return_value = None + client._client.get_release.return_value = { + "id": "mb-release-1", + "title": "Test Album", + "date": "2024-01-01", + "artist-credit": [{"name": "Test Artist"}], + "release-group": { + "id": "mb-group-1", + "primary-type": "Album", + "secondary-types": [], + }, + "media": [ + { + "position": 1, + "tracks": [ + { + "id": "track-1", + "number": "1", + "position": 1, + "length": 180000, + "recording": { + "id": "recording-1", + "title": "Test Track", + "artist-credit": [{"name": "Test Artist"}], + "length": 180000, + }, + }, + ], + }, + ], + } + + monkeypatch.setattr( + metadata_registry, + "get_musicbrainz_client", + lambda *args, **kwargs: client, + ) monkeypatch.setattr( lr, - 'get_source_priority', - lambda primary: ['musicbrainz', 'spotify'], + "get_source_priority", + lambda primary: ["musicbrainz", "spotify"], ) album_data = { - 'musicbrainz_release_id': 'mb-release-1', + "musicbrainz_release_id": "mb-release-1", } source, api_album, items = lr._resolve_source( album_data, - 'musicbrainz', + "musicbrainz", ) - assert source == 'musicbrainz' - assert api_album == {'name': 'musicbrainz:mb-release-1'} - assert items == [{'name': 'x'}] + assert source == "musicbrainz" + assert api_album["id"] == "mb-release-1" + assert api_album["name"] == "Test Album" + assert items == api_album["tracks"] + assert items == [ + { + "id": "recording-1", + "name": "Test Track", + "artists": [{"name": "Test Artist"}], + "duration_ms": 180000, + "track_number": 1, + "disc_number": 1, + }, + ] + client._client.get_release_group.assert_any_call( + "mb-release-1", + includes=["releases", "artist-credits"], + ) + client._client.get_release.assert_any_call( + "mb-release-1", + includes=["recordings", "artist-credits", "release-groups"], + ) From a6364ac2836e8d59d7365cd1c95445cfcdd4e911 Mon Sep 17 00:00:00 2001 From: ragnarlotus Date: Fri, 26 Jun 2026 06:22:07 +0200 Subject: [PATCH 13/49] fix album completeness canonical edition matching --- core/repair_jobs/album_completeness.py | 154 ++++++++++++++----- tests/test_album_completeness_job.py | 204 ++++++++++++++++++++++++- 2 files changed, 319 insertions(+), 39 deletions(-) diff --git a/core/repair_jobs/album_completeness.py b/core/repair_jobs/album_completeness.py index 24b1260f..5744a5bf 100644 --- a/core/repair_jobs/album_completeness.py +++ b/core/repair_jobs/album_completeness.py @@ -21,9 +21,10 @@ class AlbumCompletenessJob(RepairJob): help_text = ( 'Compares the number of tracks you have for each album against the expected total ' 'from your configured metadata sources. Counts cached during normal enrichment are ' - 'used when available; otherwise the job queries a metadata source directly. Albums ' - 'where tracks are missing get flagged as findings with details about which tracks ' - 'are absent.\n\n' + 'used when no canonical edition is pinned; otherwise the exact canonical source and ' + 'album ID are queried directly. The same canonical tracklist is used for both the ' + 'expected total and the missing-track calculation. Albums where tracks are missing ' + 'get flagged as findings with details about which tracks are absent.\n\n' 'Useful for catching partial downloads or albums where some tracks failed to download. ' 'You can use the Download Missing feature from the album page to fill gaps.\n\n' 'Settings:\n' @@ -56,6 +57,7 @@ class AlbumCompletenessJob(RepairJob): has_itunes = False has_deezer = False has_api_track_count = False + has_canonical = False try: conn = context.db._get_connection() cursor = conn.cursor() @@ -67,6 +69,10 @@ class AlbumCompletenessJob(RepairJob): has_deezer = 'deezer_id' in columns has_discogs = 'discogs_id' in columns has_hydrabase = 'soul_id' in columns + has_canonical = ( + 'canonical_source' in columns + and 'canonical_album_id' in columns + ) # Detect the `api_track_count` column — older DBs may not have it # yet (migration runs on app start, but repair-job code mustn't @@ -101,8 +107,13 @@ class AlbumCompletenessJob(RepairJob): select_cols.append(('al.discogs_id', 'discogs_album_id')) if has_hydrabase: select_cols.append(('al.soul_id', 'hydrabase_album_id')) + if has_canonical: + select_cols.extend([ + ('al.canonical_source', 'canonical_source'), + ('al.canonical_album_id', 'canonical_album_id'), + ]) - # WHERE: album has at least one source ID + # WHERE: album has at least one source ID or a complete canonical pair where_parts = ["(al.spotify_album_id IS NOT NULL AND al.spotify_album_id != '')"] if has_itunes: where_parts.append("(al.itunes_album_id IS NOT NULL AND al.itunes_album_id != '')") @@ -112,6 +123,11 @@ class AlbumCompletenessJob(RepairJob): where_parts.append("(al.discogs_id IS NOT NULL AND al.discogs_id != '')") if has_hydrabase: where_parts.append("(al.soul_id IS NOT NULL AND al.soul_id != '')") + if has_canonical: + where_parts.append( + "(al.canonical_source IS NOT NULL AND al.canonical_source != '' " + "AND al.canonical_album_id IS NOT NULL AND al.canonical_album_id != '')" + ) where_clause = ' OR '.join(where_parts) select_sql = ', '.join(f'{expr} AS {alias}' for expr, alias in select_cols) @@ -159,6 +175,8 @@ class AlbumCompletenessJob(RepairJob): deezer_album_id = row[column_index['deezer_album_id']] if 'deezer_album_id' in column_index else None discogs_album_id = row[column_index['discogs_album_id']] if 'discogs_album_id' in column_index else None hydrabase_album_id = row[column_index['hydrabase_album_id']] if 'hydrabase_album_id' in column_index else None + canonical_source = row[column_index['canonical_source']] if 'canonical_source' in column_index else None + canonical_album_id = row[column_index['canonical_album_id']] if 'canonical_album_id' in column_index else None # Cached authoritative track count from a prior API lookup (NULL # on unscanned albums and on DBs predating the column migration). cached_api_count = row[column_index['api_track_count']] if 'api_track_count' in column_index else None @@ -181,20 +199,41 @@ class AlbumCompletenessJob(RepairJob): 'hydrabase': hydrabase_album_id or '', } - # Expected total comes from the metadata provider, NOT from - # al.track_count — that column holds the observed count from - # server syncs (Plex leafCount, SoulSync standalone len(tracks)) - # which by definition always equals actual_count and made the - # job skip every album. Use the cached api_track_count if a - # prior scan already looked it up; otherwise hit the API and - # persist the answer for next time. - expected_total = cached_api_count - if not expected_total: - expected_total = self._get_expected_total(context, primary_source, album_ids) - # Only persist positive results. Zero/None would keep - # re-triggering the lookup on every scan. - if expected_total and expected_total > 0 and has_api_track_count: - self._save_api_track_count(context, album_id, expected_total) + # A complete canonical pair identifies one exact edition, + # independently of provider. Its tracklist must drive both the + # expected total and the missing-track calculation; neither a + # cached count nor another source may silently replace it. + canonical_items = None + resolved_source = primary_source + resolved_album_id = self._get_album_id_for_source(primary_source, album_ids) or '' + + if canonical_source and canonical_album_id: + api_tracks = self._get_album_tracks( + str(canonical_source), + str(canonical_album_id), + ) + canonical_items = self._extract_track_items(api_tracks) + expected_total = len(canonical_items) + resolved_source = str(canonical_source) + resolved_album_id = str(canonical_album_id) + else: + # Preserve the existing behavior for albums that have not + # resolved a canonical edition yet. + expected_total = cached_api_count + if not expected_total: + expected_total = self._get_expected_total( + context, + primary_source, + album_ids, + ) + # Only persist positive results. Zero/None would keep + # re-triggering the lookup on every scan. + if expected_total and expected_total > 0 and has_api_track_count: + self._save_api_track_count( + context, + album_id, + expected_total, + ) # Skip singles/EPs based on expected track count (not local count) if expected_total and expected_total < min_tracks: @@ -224,8 +263,20 @@ class AlbumCompletenessJob(RepairJob): context.update_progress(i + 1, total) continue - # Album is incomplete — try to find which tracks are missing - missing_tracks = self._find_missing_tracks(context, primary_source, album_id, album_ids) + # Album is incomplete — identify missing tracks from the same + # canonical edition used for the expected total when one is pinned. + missing_tracks = self._find_missing_tracks( + context, + primary_source, + album_id, + album_ids, + resolved_source=( + resolved_source + if canonical_items is not None + else None + ), + resolved_items=canonical_items, + ) if context.report_progress: context.report_progress( @@ -250,8 +301,10 @@ class AlbumCompletenessJob(RepairJob): 'album_id': album_id, 'album_title': title, 'artist': artist_name, - 'primary_source': primary_source, - 'primary_album_id': self._get_album_id_for_source(primary_source, album_ids) or '', + 'primary_source': resolved_source, + 'primary_album_id': resolved_album_id, + 'canonical_source': canonical_source or '', + 'canonical_album_id': canonical_album_id or '', 'spotify_album_id': spotify_album_id or '', 'itunes_album_id': itunes_album_id or '', 'deezer_album_id': deezer_album_id or '', @@ -316,8 +369,19 @@ class AlbumCompletenessJob(RepairJob): return 0 - def _find_missing_tracks(self, context, primary_source, album_id, album_ids): - """Identify which specific tracks are missing using the active metadata provider first.""" + def _find_missing_tracks( + self, + context, + primary_source, + album_id, + album_ids, + resolved_source=None, + resolved_items=None, + ): + """Identify missing tracks from one resolved metadata edition. + + Without a supplied edition, preserve the existing source-priority lookup. + """ # Get track numbers we already have owned_numbers = set() conn = None @@ -336,19 +400,33 @@ class AlbumCompletenessJob(RepairJob): if conn: conn.close() - api_tracks = None - for source in get_source_priority(primary_source): - source_album_id = self._get_album_id_for_source(source, album_ids) - if not source_album_id: - continue - api_tracks = self._get_album_tracks(source, source_album_id) - if self._extract_track_items(api_tracks): - break + if resolved_items is None: + api_tracks = None + for source in get_source_priority(primary_source): + source_album_id = self._get_album_id_for_source( + source, + album_ids, + ) + if not source_album_id: + continue + + api_tracks = self._get_album_tracks( + source, + source_album_id, + ) + if self._extract_track_items(api_tracks): + resolved_source = source + break + + items = self._extract_track_items(api_tracks) + else: + items = resolved_items - items = self._extract_track_items(api_tracks) if not items: return [] + track_source = resolved_source or primary_source + # All supported provider responses expose the same core fields once normalized. # items[].track_number, items[].name, items[].disc_number, items[].id, items[].artists missing_tracks = [] @@ -365,7 +443,7 @@ class AlbumCompletenessJob(RepairJob): 'track_number': tn, 'name': item.get('name', ''), 'disc_number': item.get('disc_number', 1), - 'source': item.get('_source', primary_source), + 'source': item.get('_source', track_source), 'source_track_id': item.get('id', ''), 'track_id': item.get('id', ''), 'spotify_track_id': item.get('id', ''), @@ -430,6 +508,14 @@ class AlbumCompletenessJob(RepairJob): where_parts.append("(discogs_id IS NOT NULL AND discogs_id != '')") if 'soul_id' in columns: where_parts.append("(soul_id IS NOT NULL AND soul_id != '')") + if ( + 'canonical_source' in columns + and 'canonical_album_id' in columns + ): + where_parts.append( + "(canonical_source IS NOT NULL AND canonical_source != '' " + "AND canonical_album_id IS NOT NULL AND canonical_album_id != '')" + ) cursor.execute(f""" SELECT COUNT(*) FROM albums diff --git a/tests/test_album_completeness_job.py b/tests/test_album_completeness_job.py index 8d58a03c..bd0885cd 100644 --- a/tests/test_album_completeness_job.py +++ b/tests/test_album_completeness_job.py @@ -420,6 +420,8 @@ class _SharedMemoryDB: deezer_id TEXT, discogs_id TEXT, soul_id TEXT, + canonical_source TEXT, + canonical_album_id TEXT, track_count INTEGER, api_track_count INTEGER ); @@ -442,13 +444,41 @@ class _SharedMemoryDB: ) self._keepalive.commit() - def insert_album(self, album_id, artist_id, title, *, spotify_id=None, - track_count=None, api_track_count=None): + def insert_album( + self, + album_id, + artist_id, + title, + *, + spotify_id=None, + canonical_source=None, + canonical_album_id=None, + track_count=None, + api_track_count=None, + ): self._keepalive.execute( """INSERT INTO albums - (id, artist_id, title, spotify_album_id, track_count, api_track_count) - VALUES (?, ?, ?, ?, ?, ?)""", - (album_id, artist_id, title, spotify_id, track_count, api_track_count), + ( + id, + artist_id, + title, + spotify_album_id, + canonical_source, + canonical_album_id, + track_count, + api_track_count + ) + VALUES (?, ?, ?, ?, ?, ?, ?, ?)""", + ( + album_id, + artist_id, + title, + spotify_id, + canonical_source, + canonical_album_id, + track_count, + api_track_count, + ), ) self._keepalive.commit() @@ -543,6 +573,170 @@ def test_scan_uses_cached_api_track_count_without_expected_total_lookup(monkeypa assert finding['details']['actual_tracks'] == 10 +def test_scan_uses_exact_canonical_edition_for_count_and_missing_tracks( + monkeypatch, +): + db = _SharedMemoryDB() + db.insert_artist('a1', 'Test Artist') + db.insert_album( + 'alb-canonical', + 'a1', + 'Canonical Album', + spotify_id='sp-other-edition', + canonical_source='deezer', + canonical_album_id='dz-canonical', + track_count=2, + api_track_count=99, + ) + db.insert_tracks('alb-canonical', 2) + + calls = [] + + def get_tracks(source, album_id): + calls.append((source, album_id)) + + if source == 'deezer' and album_id == 'dz-canonical': + return { + 'items': [ + { + 'id': f'dz-{number}', + 'name': f'Canonical Track {number}', + 'track_number': number, + 'disc_number': 1, + 'artists': [], + } + for number in range(1, 5) + ], + } + + if source == 'spotify' and album_id == 'sp-other-edition': + return { + 'items': [ + { + 'id': f'sp-{number}', + 'name': f'Other Edition Track {number}', + 'track_number': number, + 'disc_number': 1, + 'artists': [], + } + for number in range(1, 13) + ], + } + + return None + + monkeypatch.setattr( + album_completeness_module, + 'get_album_tracks_for_source', + get_tracks, + ) + monkeypatch.setattr( + album_completeness_module, + 'get_primary_source', + lambda: 'spotify', + ) + monkeypatch.setattr( + album_completeness_module, + 'get_source_priority', + lambda primary: ['spotify', 'deezer'], + ) + + findings = [] + context = _make_job_context( + db, + create_finding=lambda **kwargs: (findings.append(kwargs) or True), + ) + + result = AlbumCompletenessJob().scan(context) + + assert result.findings_created == 1 + assert calls == [('deezer', 'dz-canonical')] + + details = findings[0]['details'] + + assert details['primary_source'] == 'deezer' + assert details['primary_album_id'] == 'dz-canonical' + assert details['canonical_source'] == 'deezer' + assert details['canonical_album_id'] == 'dz-canonical' + assert details['expected_tracks'] == 4 + assert details['actual_tracks'] == 2 + assert [ + track['track_number'] + for track in details['missing_tracks'] + ] == [3, 4] + assert all( + track['source'] == 'deezer' + for track in details['missing_tracks'] + ) + + +def test_scan_does_not_fallback_when_canonical_edition_is_unavailable( + monkeypatch, +): + db = _SharedMemoryDB() + db.insert_artist('a1', 'Test Artist') + db.insert_album( + 'alb-unavailable-canonical', + 'a1', + 'Unavailable Canonical Album', + spotify_id='sp-other-edition', + canonical_source='deezer', + canonical_album_id='dz-unavailable', + track_count=2, + api_track_count=99, + ) + db.insert_tracks('alb-unavailable-canonical', 2) + + calls = [] + + def get_tracks(source, album_id): + calls.append((source, album_id)) + + if source == 'deezer' and album_id == 'dz-unavailable': + return {'items': []} + + return { + 'items': [ + { + 'id': f'sp-{number}', + 'name': f'Other Edition Track {number}', + 'track_number': number, + 'disc_number': 1, + 'artists': [], + } + for number in range(1, 11) + ], + } + + monkeypatch.setattr( + album_completeness_module, + 'get_album_tracks_for_source', + get_tracks, + ) + monkeypatch.setattr( + album_completeness_module, + 'get_primary_source', + lambda: 'spotify', + ) + monkeypatch.setattr( + album_completeness_module, + 'get_source_priority', + lambda primary: ['spotify', 'deezer'], + ) + + findings = [] + context = _make_job_context( + db, + create_finding=lambda **kwargs: (findings.append(kwargs) or True), + ) + + result = AlbumCompletenessJob().scan(context) + + assert result.findings_created == 0 + assert findings == [] + assert calls == [('deezer', 'dz-unavailable')] + + def test_scan_falls_back_to_api_and_persists_count_on_cache_miss(monkeypatch): """Integration: when api_track_count is NULL, scan calls the API, gets the expected total, caches it, and creates the finding.""" From 85d59048460f0f48c53fe3d21050e66e3b48c45a Mon Sep 17 00:00:00 2001 From: ragnarlotus Date: Sat, 27 Jun 2026 00:04:06 +0200 Subject: [PATCH 14/49] fix album completeness fragmented rows --- core/repair_jobs/album_completeness.py | 1239 ++++++++++++++--- ...test_album_completeness_fragmented_rows.py | 496 +++++++ 2 files changed, 1574 insertions(+), 161 deletions(-) create mode 100644 tests/test_album_completeness_fragmented_rows.py diff --git a/core/repair_jobs/album_completeness.py b/core/repair_jobs/album_completeness.py index 5744a5bf..98c68d80 100644 --- a/core/repair_jobs/album_completeness.py +++ b/core/repair_jobs/album_completeness.py @@ -1,5 +1,10 @@ """Album Completeness Checker Job — finds albums missing tracks.""" +import re +import unicodedata +from collections import defaultdict +from difflib import SequenceMatcher + from core.metadata_service import ( get_album_tracks_for_source, get_primary_source, @@ -23,8 +28,10 @@ class AlbumCompletenessJob(RepairJob): 'from your configured metadata sources. Counts cached during normal enrichment are ' 'used when no canonical edition is pinned; otherwise the exact canonical source and ' 'album ID are queried directly. The same canonical tracklist is used for both the ' - 'expected total and the missing-track calculation. Albums where tracks are missing ' - 'get flagged as findings with details about which tracks are absent.\n\n' + 'expected total and the missing-track calculation. Fragmented library rows are ' + 'combined only when they safely match that same canonical edition. Albums where ' + 'tracks are missing get flagged as findings with details about which tracks are ' + 'absent.\n\n' 'Useful for catching partial downloads or albums where some tracks failed to download. ' 'You can use the Download Missing feature from the album page to fill gaps.\n\n' 'Settings:\n' @@ -51,24 +58,28 @@ class AlbumCompletenessJob(RepairJob): min_completion_pct = settings.get('min_completion_pct', 0) primary_source = self._get_primary_source() - # Fetch all albums with ANY external source ID — not just Spotify + # Fetch all albums with ANY external source ID — not just Spotify. albums = [] conn = None has_itunes = False has_deezer = False + has_discogs = False + has_hydrabase = False + has_musicbrainz = False has_api_track_count = False has_canonical = False try: conn = context.db._get_connection() cursor = conn.cursor() - # Check which source columns exist (older DBs may lack some) + # Check which source columns exist (older DBs may lack some). cursor.execute("PRAGMA table_info(albums)") columns = {row[1] for row in cursor.fetchall()} has_itunes = 'itunes_album_id' in columns has_deezer = 'deezer_id' in columns has_discogs = 'discogs_id' in columns has_hydrabase = 'soul_id' in columns + has_musicbrainz = 'musicbrainz_release_id' in columns has_canonical = ( 'canonical_source' in columns and 'canonical_album_id' in columns @@ -90,6 +101,7 @@ class AlbumCompletenessJob(RepairJob): # from metadata-source enrichment) or a live API lookup. select_cols = [ ('al.id', 'album_id'), + ('al.artist_id', 'artist_id'), ('al.title', 'album_title'), ('ar.name', 'artist_name'), ('al.spotify_album_id', 'spotify_album_id'), @@ -107,22 +119,41 @@ class AlbumCompletenessJob(RepairJob): select_cols.append(('al.discogs_id', 'discogs_album_id')) if has_hydrabase: select_cols.append(('al.soul_id', 'hydrabase_album_id')) + if has_musicbrainz: + select_cols.append( + ('al.musicbrainz_release_id', 'musicbrainz_album_id') + ) if has_canonical: select_cols.extend([ ('al.canonical_source', 'canonical_source'), ('al.canonical_album_id', 'canonical_album_id'), ]) - # WHERE: album has at least one source ID or a complete canonical pair - where_parts = ["(al.spotify_album_id IS NOT NULL AND al.spotify_album_id != '')"] + # WHERE: album has at least one source ID or a complete canonical pair. + where_parts = [ + "(al.spotify_album_id IS NOT NULL AND al.spotify_album_id != '')", + ] if has_itunes: - where_parts.append("(al.itunes_album_id IS NOT NULL AND al.itunes_album_id != '')") + where_parts.append( + "(al.itunes_album_id IS NOT NULL AND al.itunes_album_id != '')" + ) if has_deezer: - where_parts.append("(al.deezer_id IS NOT NULL AND al.deezer_id != '')") + where_parts.append( + "(al.deezer_id IS NOT NULL AND al.deezer_id != '')" + ) if has_discogs: - where_parts.append("(al.discogs_id IS NOT NULL AND al.discogs_id != '')") + where_parts.append( + "(al.discogs_id IS NOT NULL AND al.discogs_id != '')" + ) if has_hydrabase: - where_parts.append("(al.soul_id IS NOT NULL AND al.soul_id != '')") + where_parts.append( + "(al.soul_id IS NOT NULL AND al.soul_id != '')" + ) + if has_musicbrainz: + where_parts.append( + "(al.musicbrainz_release_id IS NOT NULL " + "AND al.musicbrainz_release_id != '')" + ) if has_canonical: where_parts.append( "(al.canonical_source IS NOT NULL AND al.canonical_source != '' " @@ -130,7 +161,10 @@ class AlbumCompletenessJob(RepairJob): ) where_clause = ' OR '.join(where_parts) - select_sql = ', '.join(f'{expr} AS {alias}' for expr, alias in select_cols) + select_sql = ', '.join( + f'{expr} AS {alias}' + for expr, alias in select_cols + ) cursor.execute(f""" SELECT {select_sql} FROM albums al @@ -139,8 +173,20 @@ class AlbumCompletenessJob(RepairJob): WHERE {where_clause} GROUP BY al.id """) - albums = cursor.fetchall() - column_index = {alias: idx for idx, (_, alias) in enumerate(select_cols)} + raw_rows = cursor.fetchall() + column_index = { + alias: idx + for idx, (_, alias) in enumerate(select_cols) + } + albums = [ + { + alias: row[idx] + for alias, idx in column_index.items() + } + for row in raw_rows + ] + for order, album in enumerate(albums): + album['_scan_order'] = order except Exception as e: logger.error("Error fetching albums: %s", e, exc_info=True) result.errors += 1 @@ -149,76 +195,105 @@ class AlbumCompletenessJob(RepairJob): if conn: conn.close() - total = len(albums) + # Rows that share a source ID with a canonical album are candidates for + # the same release. A sibling joins the logical album only when at least + # one of its local tracks strictly matches the canonical tracklist. + work_items = self._prepare_work_items(context, albums) + + total = len(work_items) if context.update_progress: context.update_progress(0, total) - logger.info("Checking completeness of %d albums", total) + logger.info("Checking completeness of %d logical albums", total) if context.report_progress: - context.report_progress(phase=f'Checking {total} albums...', total=total) + context.report_progress( + phase=f'Checking {total} logical albums...', + total=total, + ) - for i, row in enumerate(albums): + for i, work_item in enumerate(work_items): if context.check_stop(): return result if i % 10 == 0 and context.wait_if_paused(): return result - album_id = row[column_index['album_id']] - title = row[column_index['album_title']] - artist_name = row[column_index['artist_name']] - spotify_album_id = row[column_index['spotify_album_id']] - actual_count = row[column_index['actual_count']] - album_thumb = row[column_index['album_thumb_url']] - artist_thumb = row[column_index['artist_thumb_url']] - itunes_album_id = row[column_index['itunes_album_id']] if 'itunes_album_id' in column_index else None - deezer_album_id = row[column_index['deezer_album_id']] if 'deezer_album_id' in column_index else None - discogs_album_id = row[column_index['discogs_album_id']] if 'discogs_album_id' in column_index else None - hydrabase_album_id = row[column_index['hydrabase_album_id']] if 'hydrabase_album_id' in column_index else None - canonical_source = row[column_index['canonical_source']] if 'canonical_source' in column_index else None - canonical_album_id = row[column_index['canonical_album_id']] if 'canonical_album_id' in column_index else None - # Cached authoritative track count from a prior API lookup (NULL - # on unscanned albums and on DBs predating the column migration). - cached_api_count = row[column_index['api_track_count']] if 'api_track_count' in column_index else None + row = work_item['row'] + album_id = row['album_id'] + title = row['album_title'] + artist_name = row['artist_name'] + spotify_album_id = row['spotify_album_id'] + actual_count = int(row['actual_count'] or 0) + album_thumb = row['album_thumb_url'] + artist_thumb = row['artist_thumb_url'] + itunes_album_id = row.get('itunes_album_id') + deezer_album_id = row.get('deezer_album_id') + discogs_album_id = row.get('discogs_album_id') + hydrabase_album_id = row.get('hydrabase_album_id') + musicbrainz_album_id = row.get('musicbrainz_album_id') + canonical_source = row.get('canonical_source') + canonical_album_id = row.get('canonical_album_id') + cached_api_count = row.get('api_track_count') result.scanned += 1 if context.report_progress: context.report_progress( - scanned=i + 1, total=total, + scanned=i + 1, + total=total, phase=f'Checking {i + 1} / {total}', - log_line=f'Album: {title or "Unknown"} — {artist_name or "Unknown"}', - log_type='info' + log_line=( + f'Album: {title or "Unknown"} — ' + f'{artist_name or "Unknown"}' + ), + log_type='info', ) - album_ids = { - 'spotify': spotify_album_id or '', - 'itunes': itunes_album_id or '', - 'deezer': deezer_album_id or '', - 'discogs': discogs_album_id or '', - 'hydrabase': hydrabase_album_id or '', - } - - # A complete canonical pair identifies one exact edition, - # independently of provider. Its tracklist must drive both the - # expected total and the missing-track calculation; neither a - # cached count nor another source may silently replace it. - canonical_items = None + album_ids = self._source_ids_from_row(row) resolved_source = primary_source - resolved_album_id = self._get_album_id_for_source(primary_source, album_ids) or '' + resolved_album_id = ( + self._get_album_id_for_source(primary_source, album_ids) + or '' + ) + related_album_ids = ( + work_item.get('related_album_ids') + or [album_id] + ) + raw_local_count = work_item.get('raw_local_count') + missing_tracks = None + # A complete canonical pair is authoritative. `_prepare_work_items` + # performs exactly one lookup per canonical group and stores the + # resulting list — including an empty list when the lookup failed. if canonical_source and canonical_album_id: - api_tracks = self._get_album_tracks( - str(canonical_source), - str(canonical_album_id), - ) - canonical_items = self._extract_track_items(api_tracks) + canonical_items = work_item.get('canonical_items', []) expected_total = len(canonical_items) resolved_source = str(canonical_source) resolved_album_id = str(canonical_album_id) + + if canonical_items: + actual_count = int( + work_item.get( + 'effective_actual_count', + actual_count, + ) + ) + owned_reference_indexes = set( + work_item.get( + 'owned_reference_indexes', + set(), + ) + ) + missing_tracks = self._build_missing_tracks( + canonical_items, + owned_reference_indexes, + resolved_source, + ) + if raw_local_count is None: + raw_local_count = actual_count else: - # Preserve the existing behavior for albums that have not - # resolved a canonical edition yet. + # Preserve the existing behavior for albums without a pinned + # canonical edition. expected_total = cached_api_count if not expected_total: expected_total = self._get_expected_total( @@ -226,16 +301,18 @@ class AlbumCompletenessJob(RepairJob): primary_source, album_ids, ) - # Only persist positive results. Zero/None would keep - # re-triggering the lookup on every scan. - if expected_total and expected_total > 0 and has_api_track_count: + if ( + expected_total + and expected_total > 0 + and has_api_track_count + ): self._save_api_track_count( context, album_id, expected_total, ) - # Skip singles/EPs based on expected track count (not local count) + # Skip singles/EPs based on expected track count (not local count). if expected_total and expected_total < min_tracks: result.skipped += 1 if context.update_progress and (i + 1) % 5 == 0: @@ -248,13 +325,15 @@ class AlbumCompletenessJob(RepairJob): context.update_progress(i + 1, total) continue - # Skip albums with zero local tracks — nothing to auto-fill from - if actual_count == 0: + effective_raw_local_count = ( + raw_local_count + if raw_local_count is not None + else int(row['actual_count'] or 0) + ) + if actual_count == 0 and effective_raw_local_count == 0: result.skipped += 1 continue - # Skip albums below minimum completion percentage - # (filters out "1 track from a playlist import" false positives) if min_completion_pct > 0 and expected_total > 0: completion = (actual_count / expected_total) * 100 if completion < min_completion_pct: @@ -263,26 +342,23 @@ class AlbumCompletenessJob(RepairJob): context.update_progress(i + 1, total) continue - # Album is incomplete — identify missing tracks from the same - # canonical edition used for the expected total when one is pinned. - missing_tracks = self._find_missing_tracks( - context, - primary_source, - album_id, - album_ids, - resolved_source=( - resolved_source - if canonical_items is not None - else None - ), - resolved_items=canonical_items, - ) + if missing_tracks is None: + missing_tracks = self._find_missing_tracks( + context, + primary_source, + album_id, + album_ids, + ) if context.report_progress: context.report_progress( - log_line=f'Incomplete: {title or "Unknown"} ({actual_count}/{expected_total})', - log_type='skip' + log_line=( + f'Incomplete: {title or "Unknown"} ' + f'({actual_count}/{expected_total})' + ), + log_type='skip', ) + if context.create_finding: try: inserted = context.create_finding( @@ -292,10 +368,13 @@ class AlbumCompletenessJob(RepairJob): entity_type='album', entity_id=str(album_id), file_path=None, - title=f'Incomplete: {title or "Unknown"} ({actual_count}/{expected_total})', + title=( + f'Incomplete: {title or "Unknown"} ' + f'({actual_count}/{expected_total})' + ), description=( - f'Album "{title}" by {artist_name or "Unknown"} has {actual_count} of ' - f'{expected_total} tracks' + f'Album "{title}" by {artist_name or "Unknown"} ' + f'has {actual_count} of {expected_total} tracks' ), details={ 'album_id': album_id, @@ -310,19 +389,31 @@ class AlbumCompletenessJob(RepairJob): 'deezer_album_id': deezer_album_id or '', 'discogs_album_id': discogs_album_id or '', 'hydrabase_album_id': hydrabase_album_id or '', + 'musicbrainz_album_id': ( + musicbrainz_album_id or '' + ), 'expected_tracks': expected_total, 'actual_tracks': actual_count, + 'raw_local_tracks': effective_raw_local_count, + 'related_album_ids': [ + str(value) + for value in related_album_ids + ], 'missing_tracks': missing_tracks, 'album_thumb_url': album_thumb or None, 'artist_thumb_url': artist_thumb or None, - } + }, ) if inserted: result.findings_created += 1 else: result.findings_skipped_dedup += 1 except Exception as e: - logger.debug("Error creating completeness finding for album %s: %s", album_id, e) + logger.debug( + "Error creating completeness finding for album %s: %s", + album_id, + e, + ) result.errors += 1 if context.update_progress and (i + 1) % 5 == 0: @@ -331,42 +422,831 @@ class AlbumCompletenessJob(RepairJob): if context.update_progress: context.update_progress(total, total) - logger.info("Completeness check: %d albums checked, %d incomplete found", - result.scanned, result.findings_created) + logger.info( + "Completeness check: %d logical albums checked, %d incomplete found", + result.scanned, + result.findings_created, + ) return result - def _save_api_track_count(self, context, album_id, count): - """Persist a metadata-API track count via the shared worker helper. + def _prepare_work_items(self, context, albums): + """Collapse safely validated fragments into logical canonical albums.""" + groups = self._build_candidate_groups(albums) + work_items = [] - Enrichment workers call `set_album_api_track_count` inside their own - `_update_album` transaction. Here we're in the repair job's fallback - path (the album wasn't enriched yet), so we own the connection + - commit ourselves. A cache-write failure must never break the scan, - so all errors are swallowed into the debug log. - """ + for group in groups: + anchor = group['anchor'] + members = group['members'] + canonical_source = anchor.get('canonical_source') or '' + canonical_album_id = anchor.get('canonical_album_id') or '' + + if canonical_source and canonical_album_id: + canonical_tracks = self._get_album_tracks( + str(canonical_source), + str(canonical_album_id), + ) + canonical_items = self._extract_track_items( + canonical_tracks + ) + + if canonical_items: + local_by_album = { + str(member['album_id']): self._load_local_tracks( + context, + [member['album_id']], + ) + for member in members + } + anchor_tracks = local_by_album.get( + str(anchor['album_id']), + [], + ) + + anchor_reference, _ = self._match_tracks( + canonical_items, + anchor_tracks, + str(canonical_source), + ) + # The anchor's persisted disc/track slots remain + # authoritative even when titles or durations drift. + anchor_reference.update( + self._reference_slots_for_local_tracks( + canonical_items, + anchor_tracks, + ) + ) + + included = [anchor] + excluded = [] + supplemental_reference = set() + + for member in members: + if member is anchor: + continue + + member_tracks = local_by_album.get( + str(member['album_id']), + [], + ) + matched_reference, _ = ( + self._match_fragment_tracks( + canonical_items, + member_tracks, + str(canonical_source), + ) + ) + if matched_reference: + included.append(member) + supplemental_reference.update( + matched_reference - anchor_reference + ) + else: + excluded.append(member) + + combined_local = [] + for member in included: + combined_local.extend( + local_by_album.get( + str(member['album_id']), + [], + ) + ) + + owned_reference_indexes = ( + anchor_reference | supplemental_reference + ) + work_items.append({ + 'row': anchor, + 'canonical_items': canonical_items, + 'related_album_ids': [ + member['album_id'] + for member in included + ], + 'raw_local_count': len(combined_local), + 'effective_actual_count': ( + int(anchor.get('actual_count') or 0) + + len(supplemental_reference) + ), + 'owned_reference_indexes': ( + owned_reference_indexes + ), + '_scan_order': min( + member['_scan_order'] + for member in included + ), + }) + + for member in excluded: + work_items.append( + self._independent_work_item( + member, + canonical_items=( + canonical_items + if self._same_canonical_pair( + member, + canonical_source, + canonical_album_id, + ) + else None + ), + ) + ) + continue + + # The canonical lookup was attempted and returned no usable + # tracklist. Canonical rows must not fall back to another + # provider; non-canonical siblings remain independent. + for member in members: + work_items.append( + self._independent_work_item( + member, + canonical_items=( + [] + if self._same_canonical_pair( + member, + canonical_source, + canonical_album_id, + ) + else None + ), + ) + ) + continue + + for member in members: + work_items.append( + self._independent_work_item(member) + ) + + work_items.sort(key=lambda item: item['_scan_order']) + return work_items + + def _independent_work_item(self, row, canonical_items=None): + item = { + 'row': row, + 'related_album_ids': [row['album_id']], + '_scan_order': row['_scan_order'], + } + if canonical_items is not None: + item['canonical_items'] = canonical_items + return item + + def _same_canonical_pair( + self, + row, + canonical_source, + canonical_album_id, + ): + return ( + str(row.get('canonical_source') or '') + == str(canonical_source) + and str(row.get('canonical_album_id') or '') + == str(canonical_album_id) + ) + + def _build_candidate_groups(self, albums): + """Group non-canonical rows around unambiguous canonical anchors.""" + canonical_groups = defaultdict(list) + + for album in albums: + source = album.get('canonical_source') or '' + canonical_id = album.get('canonical_album_id') or '' + if source and canonical_id: + key = ( + str(album.get('artist_id') or ''), + str(source), + str(canonical_id), + ) + canonical_groups[key].append(album) + + canonical_row_ids = { + str(row['album_id']) + for rows in canonical_groups.values() + for row in rows + } + + alias_to_groups = defaultdict(set) + for key, anchor_rows in canonical_groups.items(): + for row in anchor_rows: + artist_id = str(row.get('artist_id') or '') + for source, source_id in ( + self._source_ids_from_row(row).items() + ): + if source_id: + alias_to_groups[ + (artist_id, source, str(source_id)) + ].add(key) + + canonical_source = ( + row.get('canonical_source') or '' + ) + canonical_id = ( + row.get('canonical_album_id') or '' + ) + alias_to_groups[ + ( + artist_id, + str(canonical_source), + str(canonical_id), + ) + ].add(key) + + assigned_candidate_ids = set() + groups = [] + + for key, anchor_rows in canonical_groups.items(): + anchor = max( + anchor_rows, + key=lambda row: ( + int(row.get('actual_count') or 0), + 1 if row.get('album_title') else 0, + -int(row.get('_scan_order') or 0), + ), + ) + members = list(anchor_rows) + + for row in albums: + row_id = str(row['album_id']) + if ( + row_id in canonical_row_ids + or row_id in assigned_candidate_ids + ): + continue + + artist_id = str(row.get('artist_id') or '') + matches = set() + for source, source_id in ( + self._source_ids_from_row(row).items() + ): + if source_id: + matches.update( + alias_to_groups.get( + ( + artist_id, + source, + str(source_id), + ), + set(), + ) + ) + + if matches == {key}: + members.append(row) + assigned_candidate_ids.add(row_id) + + groups.append({ + 'anchor': anchor, + 'members': sorted( + members, + key=lambda row: row['_scan_order'], + ), + '_scan_order': min( + row['_scan_order'] + for row in members + ), + }) + + grouped_ids = canonical_row_ids | assigned_candidate_ids + for row in albums: + if str(row['album_id']) in grouped_ids: + continue + groups.append({ + 'anchor': row, + 'members': [row], + '_scan_order': row['_scan_order'], + }) + + groups.sort(key=lambda group: group['_scan_order']) + return groups + + def _source_ids_from_row(self, row): + """Return every stored release ID available on an album row.""" + return { + 'spotify': row.get('spotify_album_id') or '', + 'itunes': row.get('itunes_album_id') or '', + 'deezer': row.get('deezer_album_id') or '', + 'discogs': row.get('discogs_album_id') or '', + 'hydrabase': row.get('hydrabase_album_id') or '', + 'musicbrainz': row.get('musicbrainz_album_id') or '', + } + + def _load_local_tracks(self, context, album_ids): + """Load local tracks while tolerating older track-table schemas.""" + ids = [ + str(album_id) + for album_id in album_ids + if album_id is not None + ] + if not ids: + return [] + + placeholders = ','.join('?' for _ in ids) conn = None try: conn = context.db._get_connection() cursor = conn.cursor() - set_album_api_track_count(cursor, album_id, count) - conn.commit() + cursor.execute("PRAGMA table_info(tracks)") + columns = { + row[1] + for row in cursor.fetchall() + } + + select_cols = [ + ( + 'id' if 'id' in columns else 'NULL', + 'id', + ), + ('album_id', 'album_id'), + ( + 'title' if 'title' in columns else "''", + 'title', + ), + ( + 'track_number' + if 'track_number' in columns + else 'NULL', + 'track_number', + ), + ( + 'disc_number' + if 'disc_number' in columns + else '1', + 'disc_number', + ), + ( + 'duration' + if 'duration' in columns + else '0', + 'duration', + ), + ( + 'musicbrainz_recording_id' + if 'musicbrainz_recording_id' in columns + else "''", + 'musicbrainz_recording_id', + ), + ] + select_sql = ', '.join( + f'{expression} AS {alias}' + for expression, alias in select_cols + ) + cursor.execute( + f""" + SELECT {select_sql} + FROM tracks + WHERE album_id IN ({placeholders}) + """, + ids, + ) + return [ + { + 'id': row[0], + 'album_id': row[1], + 'title': row[2] or '', + 'track_number': row[3], + 'disc_number': ( + row[4] + if row[4] is not None + else 1 + ), + 'duration_ms': row[5] or 0, + 'musicbrainz_recording_id': row[6] or '', + } + for row in cursor.fetchall() + ] except Exception as e: - logger.debug("Failed to cache api_track_count for album %s: %s", album_id, e) + logger.debug( + "Failed loading local tracks for albums %s: %s", + ids, + e, + ) + return [] finally: if conn: conn.close() - def _get_expected_total(self, context, primary_source, album_ids): - """Try to get the expected track count from the active metadata provider first.""" + def _normalize_title(self, value): + text = unicodedata.normalize('NFKD', str(value or '')) + text = ''.join( + char + for char in text + if not unicodedata.combining(char) + ) + text = text.casefold() + text = re.sub(r'[\W_]+', ' ', text, flags=re.UNICODE) + return ' '.join(text.split()) + + def _as_int(self, value, default=None): + if value is None or value == '': + return default + if isinstance(value, bool): + return int(value) + if isinstance(value, (int, float)): + return int(value) + match = re.search(r'\d+', str(value)) + return int(match.group(0)) if match else default + + def _reference_duration_ms(self, item): + value = item.get('duration_ms') + if value not in (None, ''): + try: + return int(float(value)) + except (TypeError, ValueError): + return 0 + + value = item.get('duration') + if value not in (None, ''): + try: + return int(float(value) * 1000) + except (TypeError, ValueError): + return 0 + return 0 + + def _reference_slots_for_local_tracks( + self, + reference_items, + local_tracks, + ): + """Map unambiguous local disc/track slots to reference indexes.""" + slots = defaultdict(list) + for index, item in enumerate(reference_items): + number = self._as_int(item.get('track_number')) + disc = self._as_int( + item.get('disc_number'), + 1, + ) + if number is not None: + slots[(disc, number)].append(index) + + matched = set() + for track in local_tracks: + number = self._as_int(track.get('track_number')) + disc = self._as_int( + track.get('disc_number'), + 1, + ) + indexes = slots.get((disc, number), []) + if len(indexes) == 1: + matched.add(indexes[0]) + return matched + + def _track_match_score( + self, + reference, + local, + reference_source, + ): + """Return a score when a local track plausibly matches a reference.""" + reference_id = str(reference.get('id') or '') + local_mbid = str( + local.get('musicbrainz_recording_id') or '' + ) + if ( + reference_source == 'musicbrainz' + and reference_id + and reference_id == local_mbid + ): + return 1000 + + reference_number = self._as_int( + reference.get('track_number') + ) + local_number = self._as_int( + local.get('track_number') + ) + reference_disc = self._as_int( + reference.get('disc_number'), + 1, + ) + local_disc = self._as_int( + local.get('disc_number'), + 1, + ) + + same_number = ( + reference_number is not None + and local_number is not None + and reference_number == local_number + ) + same_slot = same_number and reference_disc == local_disc + + reference_title = self._normalize_title( + reference.get('name') + or reference.get('title') + ) + local_title = self._normalize_title( + local.get('title') + ) + title_ratio = ( + SequenceMatcher( + None, + reference_title, + local_title, + ).ratio() + if reference_title and local_title + else 0.0 + ) + exact_title = bool( + reference_title + and reference_title == local_title + ) + + reference_duration = self._reference_duration_ms( + reference + ) + local_duration = ( + self._as_int( + local.get('duration_ms'), + 0, + ) + or 0 + ) + has_durations = ( + reference_duration > 0 + and local_duration > 0 + ) + duration_close = ( + has_durations + and abs( + reference_duration - local_duration + ) <= 15000 + ) + + if same_slot and title_ratio >= 0.65: + return ( + 800 + + int(title_ratio * 100) + + (30 if duration_close else 0) + ) + if same_slot and duration_close: + return 740 + int(title_ratio * 100) + if exact_title: + return ( + 700 + + (50 if duration_close else 0) + + (20 if same_number else 0) + ) + if ( + title_ratio >= 0.92 + and (duration_close or not has_durations) + ): + return 650 + int(title_ratio * 100) + if ( + same_number + and title_ratio >= 0.80 + and duration_close + ): + return 620 + int(title_ratio * 100) + return None + + def _fragment_track_match_score( + self, + reference, + local, + reference_source, + ): + """Use stricter matching when accepting a sibling fragment.""" + reference_id = str(reference.get('id') or '') + local_mbid = str( + local.get('musicbrainz_recording_id') or '' + ) + if ( + reference_source == 'musicbrainz' + and reference_id + and reference_id == local_mbid + ): + return 1000 + + reference_title = self._normalize_title( + reference.get('name') + or reference.get('title') + ) + local_title = self._normalize_title( + local.get('title') + ) + if not reference_title or not local_title: + return None + + title_ratio = SequenceMatcher( + None, + reference_title, + local_title, + ).ratio() + exact_title = reference_title == local_title + + reference_number = self._as_int( + reference.get('track_number') + ) + local_number = self._as_int( + local.get('track_number') + ) + reference_disc = self._as_int( + reference.get('disc_number'), + 1, + ) + local_disc = self._as_int( + local.get('disc_number'), + 1, + ) + same_slot = ( + reference_number is not None + and local_number is not None + and reference_number == local_number + and reference_disc == local_disc + ) + + reference_duration = self._reference_duration_ms( + reference + ) + local_duration = ( + self._as_int( + local.get('duration_ms'), + 0, + ) + or 0 + ) + duration_close = ( + reference_duration > 0 + and local_duration > 0 + and abs( + reference_duration - local_duration + ) <= 15000 + ) + + if exact_title: + return ( + 900 + + (30 if same_slot else 0) + + (20 if duration_close else 0) + ) + if ( + title_ratio >= 0.95 + and (same_slot or duration_close) + ): + return 800 + int(title_ratio * 100) + if ( + title_ratio >= 0.85 + and same_slot + and duration_close + ): + return 700 + int(title_ratio * 100) + return None + + def _match_fragment_tracks( + self, + reference_items, + local_tracks, + reference_source, + ): + """One-to-one strict match used only for candidate siblings.""" + return self._greedy_match( + reference_items, + local_tracks, + reference_source, + self._fragment_track_match_score, + ) + + def _match_tracks( + self, + reference_items, + local_tracks, + reference_source, + ): + """One-to-one general match for tracks already on the anchor.""" + return self._greedy_match( + reference_items, + local_tracks, + reference_source, + self._track_match_score, + ) + + def _greedy_match( + self, + reference_items, + local_tracks, + reference_source, + scorer, + ): + candidates = [] + for reference_index, reference in enumerate( + reference_items + ): + for local_index, local in enumerate(local_tracks): + score = scorer( + reference, + local, + reference_source, + ) + if score is not None: + candidates.append( + ( + score, + reference_index, + local_index, + ) + ) + + candidates.sort(reverse=True) + matched_reference = set() + matched_local = set() + + for _, reference_index, local_index in candidates: + if ( + reference_index in matched_reference + or local_index in matched_local + ): + continue + matched_reference.add(reference_index) + matched_local.add(local_index) + + return matched_reference, matched_local + + def _build_missing_tracks( + self, + reference_items, + matched_reference, + reference_source, + ): + missing_tracks = [] + for index, item in enumerate(reference_items): + if index in matched_reference: + continue + + track_artists = [] + for artist in item.get('artists', []): + if isinstance(artist, dict): + track_artists.append( + artist.get('name', '') + ) + elif isinstance(artist, str): + track_artists.append(artist) + + source_track_id = item.get('id', '') + missing_tracks.append({ + 'track_number': item.get('track_number'), + 'name': ( + item.get('name') + or item.get('title') + or '' + ), + 'disc_number': item.get('disc_number', 1), + 'source': item.get( + '_source', + reference_source, + ), + 'source_track_id': source_track_id, + 'track_id': source_track_id, + 'spotify_track_id': source_track_id, + 'duration_ms': self._reference_duration_ms( + item + ), + 'artists': track_artists, + }) + return missing_tracks + + def _save_api_track_count(self, context, album_id, count): + """Persist a metadata-API track count via the shared worker helper.""" + conn = None + try: + conn = context.db._get_connection() + cursor = conn.cursor() + set_album_api_track_count( + cursor, + album_id, + count, + ) + conn.commit() + except Exception as e: + logger.debug( + "Failed to cache api_track_count for album %s: %s", + album_id, + e, + ) + finally: + if conn: + conn.close() + + def _get_expected_total( + self, + context, + primary_source, + album_ids, + ): + """Get the expected count from the active provider priority.""" for source in get_source_priority(primary_source): - album_id = self._get_album_id_for_source(source, album_ids) + album_id = self._get_album_id_for_source( + source, + album_ids, + ) if not album_id: continue - api_tracks = self._get_album_tracks(source, album_id) + api_tracks = self._get_album_tracks( + source, + album_id, + ) items = self._extract_track_items(api_tracks) if items: return len(items) - return 0 def _find_missing_tracks( @@ -378,22 +1258,20 @@ class AlbumCompletenessJob(RepairJob): resolved_source=None, resolved_items=None, ): - """Identify missing tracks from one resolved metadata edition. - - Without a supplied edition, preserve the existing source-priority lookup. - """ - # Get track numbers we already have + """Identify missing tracks from one resolved metadata edition.""" owned_numbers = set() conn = None try: conn = context.db._get_connection() cursor = conn.cursor() cursor.execute( - "SELECT track_number FROM tracks WHERE album_id = ? AND track_number IS NOT NULL", - (album_id,) + "SELECT track_number FROM tracks " + "WHERE album_id = ? " + "AND track_number IS NOT NULL", + (album_id,), ) - for tr in cursor.fetchall(): - owned_numbers.add(tr[0]) + for track in cursor.fetchall(): + owned_numbers.add(track[0]) except Exception: return [] finally: @@ -402,14 +1280,17 @@ class AlbumCompletenessJob(RepairJob): if resolved_items is None: api_tracks = None - for source in get_source_priority(primary_source): - source_album_id = self._get_album_id_for_source( - source, - album_ids, + for source in get_source_priority( + primary_source + ): + source_album_id = ( + self._get_album_id_for_source( + source, + album_ids, + ) ) if not source_album_id: continue - api_tracks = self._get_album_tracks( source, source_album_id, @@ -417,7 +1298,6 @@ class AlbumCompletenessJob(RepairJob): if self._extract_track_items(api_tracks): resolved_source = source break - items = self._extract_track_items(api_tracks) else: items = resolved_items @@ -426,64 +1306,77 @@ class AlbumCompletenessJob(RepairJob): return [] track_source = resolved_source or primary_source - - # All supported provider responses expose the same core fields once normalized. - # items[].track_number, items[].name, items[].disc_number, items[].id, items[].artists - missing_tracks = [] - for item in items: - tn = item.get('track_number') - if tn and tn not in owned_numbers: - track_artists = [] - for a in item.get('artists', []): - if isinstance(a, dict): - track_artists.append(a.get('name', '')) - elif isinstance(a, str): - track_artists.append(a) - missing_tracks.append({ - 'track_number': tn, - 'name': item.get('name', ''), - 'disc_number': item.get('disc_number', 1), - 'source': item.get('_source', track_source), - 'source_track_id': item.get('id', ''), - 'track_id': item.get('id', ''), - 'spotify_track_id': item.get('id', ''), - 'duration_ms': item.get('duration_ms', 0), - 'artists': track_artists, - }) - return missing_tracks + matched_reference = { + index + for index, item in enumerate(items) + if ( + item.get('track_number') + and item.get('track_number') in owned_numbers + ) + } + return self._build_missing_tracks( + items, + matched_reference, + track_source, + ) def _get_settings(self, context: JobContext) -> dict: if not context.config_manager: return self.default_settings.copy() - cfg = context.config_manager.get(f'repair.jobs.{self.job_id}.settings', {}) + cfg = context.config_manager.get( + f'repair.jobs.{self.job_id}.settings', + {}, + ) merged = self.default_settings.copy() merged.update(cfg) return merged def _get_primary_source(self) -> str: - """Return the active metadata source used for source prioritization.""" + """Return the active metadata source for prioritization.""" try: return get_primary_source() except Exception: return 'deezer' - def _get_album_id_for_source(self, source: str, album_ids: dict) -> str: + def _get_album_id_for_source( + self, + source: str, + album_ids: dict, + ) -> str: return album_ids.get(source, '') - def _get_album_tracks(self, source: str, album_id: str): + def _get_album_tracks( + self, + source: str, + album_id: str, + ): """Fetch album tracks from a specific source.""" try: - return get_album_tracks_for_source(source, album_id) + return get_album_tracks_for_source( + source, + album_id, + ) except Exception as e: - logger.debug("Error getting %s album tracks for %s: %s", source.capitalize(), album_id, e) + logger.debug( + "Error getting %s album tracks for %s: %s", + source.capitalize(), + album_id, + e, + ) return None def _extract_track_items(self, api_tracks): - """Normalize album track responses to a list of item dicts.""" + """Normalize provider responses to a list of track dicts.""" if not api_tracks: return [] if isinstance(api_tracks, dict): - items = api_tracks.get('items') or [] + items = ( + api_tracks.get('items') + or api_tracks.get('tracks') + or [] + ) + if isinstance(items, dict): + items = items.get('items') or [] return items if items else [] if isinstance(api_tracks, list): return api_tracks @@ -495,26 +1388,50 @@ class AlbumCompletenessJob(RepairJob): conn = context.db._get_connection() cursor = conn.cursor() - # Check which columns exist cursor.execute("PRAGMA table_info(albums)") - columns = {row[1] for row in cursor.fetchall()} + columns = { + row[1] + for row in cursor.fetchall() + } - where_parts = ["(spotify_album_id IS NOT NULL AND spotify_album_id != '')"] + where_parts = [ + "(spotify_album_id IS NOT NULL " + "AND spotify_album_id != '')", + ] if 'itunes_album_id' in columns: - where_parts.append("(itunes_album_id IS NOT NULL AND itunes_album_id != '')") + where_parts.append( + "(itunes_album_id IS NOT NULL " + "AND itunes_album_id != '')" + ) if 'deezer_id' in columns: - where_parts.append("(deezer_id IS NOT NULL AND deezer_id != '')") + where_parts.append( + "(deezer_id IS NOT NULL " + "AND deezer_id != '')" + ) if 'discogs_id' in columns: - where_parts.append("(discogs_id IS NOT NULL AND discogs_id != '')") + where_parts.append( + "(discogs_id IS NOT NULL " + "AND discogs_id != '')" + ) if 'soul_id' in columns: - where_parts.append("(soul_id IS NOT NULL AND soul_id != '')") + where_parts.append( + "(soul_id IS NOT NULL " + "AND soul_id != '')" + ) + if 'musicbrainz_release_id' in columns: + where_parts.append( + "(musicbrainz_release_id IS NOT NULL " + "AND musicbrainz_release_id != '')" + ) if ( 'canonical_source' in columns and 'canonical_album_id' in columns ): where_parts.append( - "(canonical_source IS NOT NULL AND canonical_source != '' " - "AND canonical_album_id IS NOT NULL AND canonical_album_id != '')" + "(canonical_source IS NOT NULL " + "AND canonical_source != '' " + "AND canonical_album_id IS NOT NULL " + "AND canonical_album_id != '')" ) cursor.execute(f""" diff --git a/tests/test_album_completeness_fragmented_rows.py b/tests/test_album_completeness_fragmented_rows.py new file mode 100644 index 00000000..bfec2a16 --- /dev/null +++ b/tests/test_album_completeness_fragmented_rows.py @@ -0,0 +1,496 @@ +import sqlite3 +import sys +import types +import uuid + + +class _DummyConfigManager: + def get(self, key, default=None): + return default + + def get_active_media_server(self): + return "plex" + + +if "spotipy" not in sys.modules: + spotipy = types.ModuleType("spotipy") + + class _DummySpotify: + def __init__(self, *args, **kwargs): + pass + + oauth2 = types.ModuleType("spotipy.oauth2") + + class _DummyOAuth: + def __init__(self, *args, **kwargs): + pass + + spotipy.Spotify = _DummySpotify + oauth2.SpotifyOAuth = _DummyOAuth + oauth2.SpotifyClientCredentials = _DummyOAuth + spotipy.oauth2 = oauth2 + sys.modules["spotipy"] = spotipy + sys.modules["spotipy.oauth2"] = oauth2 + +if "config.settings" not in sys.modules: + config_pkg = types.ModuleType("config") + settings_mod = types.ModuleType("config.settings") + + settings_mod.config_manager = _DummyConfigManager() + config_pkg.settings = settings_mod + sys.modules["config"] = config_pkg + sys.modules["config.settings"] = settings_mod + +from core.repair_jobs.album_completeness import AlbumCompletenessJob +import core.repair_jobs.album_completeness as album_completeness_module + + +class _SharedMemoryDB: + def __init__(self): + self.uri = ( + f"file:testdb_{uuid.uuid4().hex}" + "?mode=memory&cache=shared" + ) + self._keepalive = sqlite3.connect(self.uri, uri=True) + self._keepalive.executescript( + """ + CREATE TABLE artists ( + id TEXT PRIMARY KEY, + name TEXT, + thumb_url TEXT + ); + + CREATE TABLE albums ( + id TEXT PRIMARY KEY, + artist_id TEXT, + title TEXT, + thumb_url TEXT, + spotify_album_id TEXT, + itunes_album_id TEXT, + deezer_id TEXT, + discogs_id TEXT, + soul_id TEXT, + musicbrainz_release_id TEXT, + canonical_source TEXT, + canonical_album_id TEXT, + api_track_count INTEGER + ); + + CREATE TABLE tracks ( + id TEXT PRIMARY KEY, + album_id TEXT, + title TEXT, + track_number INTEGER, + disc_number INTEGER, + duration INTEGER, + musicbrainz_recording_id TEXT + ); + """ + ) + self._keepalive.commit() + + def _get_connection(self): + return sqlite3.connect(self.uri, uri=True) + + def insert_artist(self, artist_id, name): + self._keepalive.execute( + """ + INSERT INTO artists (id, name) + VALUES (?, ?) + """, + (artist_id, name), + ) + self._keepalive.commit() + + def insert_album( + self, + album_id, + artist_id, + title, + *, + spotify_id=None, + musicbrainz_id=None, + canonical_source=None, + canonical_album_id=None, + api_track_count=None, + ): + self._keepalive.execute( + """ + INSERT INTO albums ( + id, + artist_id, + title, + spotify_album_id, + musicbrainz_release_id, + canonical_source, + canonical_album_id, + api_track_count + ) + VALUES (?, ?, ?, ?, ?, ?, ?, ?) + """, + ( + album_id, + artist_id, + title, + spotify_id, + musicbrainz_id, + canonical_source, + canonical_album_id, + api_track_count, + ), + ) + self._keepalive.commit() + + def insert_track( + self, + album_id, + number, + title, + *, + disc=1, + duration_ms=180000, + mbid=None, + ): + track_id = f"{album_id}-{disc}-{number}-{uuid.uuid4().hex}" + self._keepalive.execute( + """ + INSERT INTO tracks ( + id, + album_id, + title, + track_number, + disc_number, + duration, + musicbrainz_recording_id + ) + VALUES (?, ?, ?, ?, ?, ?, ?) + """, + ( + track_id, + album_id, + title, + number, + disc, + duration_ms, + mbid, + ), + ) + self._keepalive.commit() + + +def _context(db, findings): + return types.SimpleNamespace( + db=db, + transfer_folder='', + config_manager=_DummyConfigManager(), + spotify_client=None, + is_spotify_rate_limited=lambda: False, + stop_event=None, + create_finding=lambda **kwargs: ( + findings.append(kwargs) or True + ), + should_stop=None, + is_paused=None, + update_progress=None, + report_progress=None, + check_stop=lambda: False, + wait_if_paused=lambda: False, + ) + + +def _canonical_tracks(count, *, prefix="Canonical"): + return { + "items": [ + { + "id": f"track-{number}", + "name": f"{prefix} Track {number}", + "track_number": number, + "disc_number": 1, + "duration_ms": 180000 + number, + "artists": [], + } + for number in range(1, count + 1) + ], + } + + +def test_scan_groups_validated_fragmented_rows_into_one_finding( + monkeypatch, +): + db = _SharedMemoryDB() + db.insert_artist("artist-1", "Artist") + db.insert_album( + "anchor", + "artist-1", + "Album", + spotify_id="shared-release", + canonical_source="deezer", + canonical_album_id="canonical-release", + ) + db.insert_album( + "fragment", + "artist-1", + "ALBUM", + spotify_id="shared-release", + api_track_count=2, + ) + + db.insert_track("anchor", 1, "Canonical Track 1") + db.insert_track("anchor", 2, "Canonical Track 2") + db.insert_track("fragment", 3, "Canonical Track 3") + db.insert_track("fragment", 4, "Canonical Track 4") + + calls = [] + + def get_tracks(source, album_id): + calls.append((source, album_id)) + assert source == "deezer" + assert album_id == "canonical-release" + return _canonical_tracks(5) + + monkeypatch.setattr( + album_completeness_module, + "get_album_tracks_for_source", + get_tracks, + ) + monkeypatch.setattr( + album_completeness_module, + "get_primary_source", + lambda: "spotify", + ) + monkeypatch.setattr( + album_completeness_module, + "get_source_priority", + lambda primary: ["spotify", "deezer"], + ) + + findings = [] + result = AlbumCompletenessJob().scan( + _context(db, findings) + ) + + assert result.scanned == 1 + assert result.findings_created == 1 + assert calls == [("deezer", "canonical-release")] + + details = findings[0]["details"] + assert details["album_id"] == "anchor" + assert details["expected_tracks"] == 5 + assert details["actual_tracks"] == 4 + assert details["raw_local_tracks"] == 4 + assert details["related_album_ids"] == [ + "anchor", + "fragment", + ] + assert [ + track["track_number"] + for track in details["missing_tracks"] + ] == [5] + + +def test_shared_id_without_track_match_stays_independent( + monkeypatch, +): + db = _SharedMemoryDB() + db.insert_artist("artist-1", "Artist") + db.insert_album( + "anchor", + "artist-1", + "Album", + spotify_id="shared-release", + canonical_source="deezer", + canonical_album_id="canonical-release", + ) + db.insert_album( + "unrelated", + "artist-1", + "Different Album", + spotify_id="shared-release", + api_track_count=1, + ) + + db.insert_track("anchor", 1, "Canonical Track 1") + db.insert_track( + "unrelated", + 99, + "Completely Unrelated", + duration_ms=900000, + ) + + calls = [] + + def get_tracks(source, album_id): + calls.append((source, album_id)) + return _canonical_tracks(3) + + monkeypatch.setattr( + album_completeness_module, + "get_album_tracks_for_source", + get_tracks, + ) + monkeypatch.setattr( + album_completeness_module, + "get_primary_source", + lambda: "spotify", + ) + monkeypatch.setattr( + album_completeness_module, + "get_source_priority", + lambda primary: ["spotify", "deezer"], + ) + + findings = [] + result = AlbumCompletenessJob().scan( + _context(db, findings) + ) + + assert result.scanned == 2 + assert result.findings_created == 1 + assert calls == [("deezer", "canonical-release")] + assert findings[0]["entity_id"] == "anchor" + assert findings[0]["details"]["related_album_ids"] == [ + "anchor", + ] + + +def test_fragment_grouping_never_crosses_artist_boundary( + monkeypatch, +): + db = _SharedMemoryDB() + db.insert_artist("artist-1", "Artist One") + db.insert_artist("artist-2", "Artist Two") + db.insert_album( + "anchor", + "artist-1", + "Album", + spotify_id="shared-release", + canonical_source="deezer", + canonical_album_id="canonical-release", + ) + db.insert_album( + "other-artist", + "artist-2", + "Album", + spotify_id="shared-release", + api_track_count=1, + ) + + db.insert_track("anchor", 1, "Canonical Track 1") + db.insert_track( + "other-artist", + 2, + "Canonical Track 2", + ) + + calls = [] + + def get_tracks(source, album_id): + calls.append((source, album_id)) + return _canonical_tracks(3) + + monkeypatch.setattr( + album_completeness_module, + "get_album_tracks_for_source", + get_tracks, + ) + monkeypatch.setattr( + album_completeness_module, + "get_primary_source", + lambda: "spotify", + ) + monkeypatch.setattr( + album_completeness_module, + "get_source_priority", + lambda primary: ["spotify", "deezer"], + ) + + findings = [] + result = AlbumCompletenessJob().scan( + _context(db, findings) + ) + + assert result.scanned == 2 + assert result.findings_created == 1 + assert calls == [("deezer", "canonical-release")] + assert findings[0]["details"]["related_album_ids"] == [ + "anchor", + ] + + +def test_musicbrainz_recording_id_validates_fragment( + monkeypatch, +): + db = _SharedMemoryDB() + db.insert_artist("artist-1", "Artist") + db.insert_album( + "anchor", + "artist-1", + "Album", + spotify_id="shared-release", + musicbrainz_id="mb-release", + canonical_source="musicbrainz", + canonical_album_id="mb-release", + ) + db.insert_album( + "fragment", + "artist-1", + "Album Fragment", + spotify_id="shared-release", + api_track_count=1, + ) + + db.insert_track( + "anchor", + 1, + "Canonical Track 1", + mbid="track-1", + ) + db.insert_track( + "fragment", + 99, + "Wrong title and position", + duration_ms=999999, + mbid="track-2", + ) + + calls = [] + + def get_tracks(source, album_id): + calls.append((source, album_id)) + return _canonical_tracks(3) + + monkeypatch.setattr( + album_completeness_module, + "get_album_tracks_for_source", + get_tracks, + ) + monkeypatch.setattr( + album_completeness_module, + "get_primary_source", + lambda: "spotify", + ) + monkeypatch.setattr( + album_completeness_module, + "get_source_priority", + lambda primary: ["spotify", "musicbrainz"], + ) + + findings = [] + result = AlbumCompletenessJob().scan( + _context(db, findings) + ) + + assert result.scanned == 1 + assert result.findings_created == 1 + assert calls == [("musicbrainz", "mb-release")] + + details = findings[0]["details"] + assert details["actual_tracks"] == 2 + assert details["related_album_ids"] == [ + "anchor", + "fragment", + ] + assert [ + track["track_number"] + for track in details["missing_tracks"] + ] == [3] From 31637e00962a5d75ecbb16126a4fee99a88d2532 Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Fri, 26 Jun 2026 19:04:24 -0700 Subject: [PATCH 15/49] canonical: recognize musicbrainz as a readable album source (PR #929 follow-up) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #929 added 'musicbrainz' to library_reorganize._ALBUM_ID_COLUMNS but not to the canonical layer, breaking the equality invariant test (canonical reads exactly what reorganize reads). add musicbrainz to CANONICAL_ALBUM_SOURCES, and move it from the 'can't pin' param group to the 'pins' group in the manual-lock tests — now consistent, and forward-compatible with pinning a deliberately-matched MB edition. inert at runtime today (mb isn't in the manual source selector, so should_pin is never called with it). 540 canonical/reorganize tests green. --- core/metadata/canonical_version.py | 2 +- tests/test_canonical_manual_lock.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/core/metadata/canonical_version.py b/core/metadata/canonical_version.py index 9871d89e..1f54a1e8 100644 --- a/core/metadata/canonical_version.py +++ b/core/metadata/canonical_version.py @@ -211,7 +211,7 @@ def pick_canonical_release( # core.library_reorganize._ALBUM_ID_COLUMNS — a test pins them in sync). A manual # match on any of these should pin/lock the canonical version (#758); a match on # a source the canonical tools don't read (e.g. lastfm) has no version to pin. -CANONICAL_ALBUM_SOURCES = frozenset({'spotify', 'itunes', 'deezer', 'discogs', 'hydrabase'}) +CANONICAL_ALBUM_SOURCES = frozenset({'spotify', 'itunes', 'deezer', 'discogs', 'hydrabase', 'musicbrainz'}) def should_pin_manual_canonical(entity_type: str, source: str) -> bool: diff --git a/tests/test_canonical_manual_lock.py b/tests/test_canonical_manual_lock.py index ad9fd99e..54f9bcea 100644 --- a/tests/test_canonical_manual_lock.py +++ b/tests/test_canonical_manual_lock.py @@ -24,7 +24,7 @@ from database.music_database import MusicDatabase # should_pin_manual_canonical — pure # --------------------------------------------------------------------------- -@pytest.mark.parametrize('source', ['spotify', 'itunes', 'deezer', 'discogs', 'hydrabase']) +@pytest.mark.parametrize('source', ['spotify', 'itunes', 'deezer', 'discogs', 'hydrabase', 'musicbrainz']) def test_pins_album_on_recognised_source(source): assert should_pin_manual_canonical('album', source) is True @@ -34,7 +34,7 @@ def test_does_not_pin_non_album(entity): assert should_pin_manual_canonical(entity, 'spotify') is False -@pytest.mark.parametrize('source', ['lastfm', 'genius', 'musicbrainz', 'audiodb', 'tidal']) +@pytest.mark.parametrize('source', ['lastfm', 'genius', 'audiodb', 'tidal']) def test_does_not_pin_source_canonical_cant_read(source): # No album-version data the canonical tools read → nothing to pin. assert should_pin_manual_canonical('album', source) is False From e96d62432f9372a2494d21e3b9b58b7c2058da5f Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Fri, 26 Jun 2026 19:56:11 -0700 Subject: [PATCH 16/49] test(album-completeness): stop polluting sys.modules (fix flaky suite failure) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit the file faked spotipy + config.settings in sys.modules at import time with no teardown. the fake config.settings had no ConfigManager, so depending on collection order it leaked into tests/test_config_save_retry and intermittently failed the full suite. the real modules import fine in the test env (spotipy is installed, config.settings has both ConfigManager + config_manager), so the stubs were pure liability — removed them. album tests still pass (10), the album+config combo that errored now passes (17), 573 repair/ config/canonical tests green. --- tests/test_album_completeness_job.py | 34 ++++------------------------ 1 file changed, 4 insertions(+), 30 deletions(-) diff --git a/tests/test_album_completeness_job.py b/tests/test_album_completeness_job.py index bd0885cd..4cca27fc 100644 --- a/tests/test_album_completeness_job.py +++ b/tests/test_album_completeness_job.py @@ -1,4 +1,3 @@ -import sys import types @@ -10,35 +9,10 @@ class _DummyConfigManager: return "plex" -if "spotipy" not in sys.modules: - spotipy = types.ModuleType("spotipy") - - class _DummySpotify: - def __init__(self, *args, **kwargs): - pass - - oauth2 = types.ModuleType("spotipy.oauth2") - - class _DummyOAuth: - def __init__(self, *args, **kwargs): - pass - - spotipy.Spotify = _DummySpotify - oauth2.SpotifyOAuth = _DummyOAuth - oauth2.SpotifyClientCredentials = _DummyOAuth - spotipy.oauth2 = oauth2 - sys.modules["spotipy"] = spotipy - sys.modules["spotipy.oauth2"] = oauth2 - -if "config.settings" not in sys.modules: - config_pkg = types.ModuleType("config") - settings_mod = types.ModuleType("config.settings") - - settings_mod.config_manager = _DummyConfigManager() - config_pkg.settings = settings_mod - sys.modules["config"] = config_pkg - sys.modules["config.settings"] = settings_mod - +# NOTE: deliberately no sys.modules stubbing of spotipy / config.settings here. Both import +# fine in the test env, and faking them globally (with no teardown) leaked into other files — +# it left a config.settings with no ConfigManager, intermittently breaking +# tests/test_config_save_retry depending on collection order. from core.repair_jobs.album_completeness import AlbumCompletenessJob import core.repair_jobs.album_completeness as album_completeness_module From 7e2d2db08df067870dcdda866839119a1d26dc43 Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Fri, 26 Jun 2026 20:30:20 -0700 Subject: [PATCH 17/49] watchlist: don't fuse different editions as the same album (Sokhi: Expedition 33) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit _normalize_album_for_match stripped ANY trailing '- clause', so a real distinguishing subtitle ('Clair Obscur: Expedition 33 - Nos vies en Lumière (Bonus Edition)') collapsed to the same name as the OST → _albums_likely_match treated them as one album → the watchlist marked unowned tracks of one edition as owned via the other and under-wishlisted. - strip a trailing '- ...' clause ONLY when every token in it is an edition/format qualifier (+ connectors / year-ordinal): '- Single', '- Acoustic Version', '- 2011 Remaster' still collapse, but real subtitles ('- Nos vies en Lumière', '- Volume 2', '- Live in Berlin') are kept. Avoids the inverse regression (a same-album pair splitting into a redownload loop), which a naive narrow strip would have caused. - drop the loose substring shortcut + raise the fuzzy floor 0.6→0.85; genuine drift already collapses to an EXACT match, so the looseness only ever produced false fuses. blast radius: _albums_likely_match has exactly one caller (the allow-duplicates skip). 48 album-match tests pass (qualifier-suffix merges + edition-subtitle splits) + 219 watchlist. --- core/watchlist_scanner.py | 33 ++++++++++++++++++++++------- tests/test_watchlist_album_match.py | 24 +++++++++++++++++++++ 2 files changed, 49 insertions(+), 8 deletions(-) diff --git a/core/watchlist_scanner.py b/core/watchlist_scanner.py index 55d03105..f8eda5e3 100644 --- a/core/watchlist_scanner.py +++ b/core/watchlist_scanner.py @@ -328,6 +328,22 @@ _ALBUM_QUALIFIER_RE = re.compile( re.IGNORECASE, ) +# A trailing "- ..." clause is stripped ONLY when EVERY token in it is an edition/format +# qualifier (+ connectors / a year-ordinal). So "- Single", "- Acoustic Version", "- 2011 +# Remaster" collapse to the base name, but a real distinguishing subtitle ("- Nos vies en +# Lumière", "- Live in Berlin") is kept — the bug was a blanket "- anything$" strip that +# erased subtitles and fused different editions (Sokhi: Expedition 33 OST vs Bonus Edition). +_DASH_QUALIFIER_WORD = ( + r'live|acoustic|electric|instrumental|unplugged|mono|stereo|demos?|reissue|' + r'remix(?:es)?|edit(?:ed)?|radio|single|ep|lp|version|mix(?:es)?|sessions?|bootleg|' + r'covers?|original|redux|deluxe|expanded|remaster(?:ed)?|anniversary|special|' + r'edition|bonus|extended|explicit|clean|soundtrack|ost|score' +) +_TRAILING_DASH_QUALIFIER_RE = re.compile( + r'\s*-\s*(?:(?:' + _DASH_QUALIFIER_WORD + r'|the|a|and|&|\+|\d+(?:st|nd|rd|th)?)\b[\s\-]*)+$', + re.IGNORECASE, +) + def _normalize_album_for_match(name: str) -> str: """Return a canonical form of an album name suitable for fuzzy comparison. @@ -347,8 +363,9 @@ def _normalize_album_for_match(name: str) -> str: # they're almost always edition or commentary noise, not part of the # album's identifying name. cleaned = re.sub(r'\s*[\(\[][^\)\]]*[\)\]]\s*', ' ', cleaned) - # Trailing dash-clauses ("Album - Remastered", "Album - Live") - cleaned = re.sub(r'\s*-\s*[^-]+$', '', cleaned) + # Trailing dash-clause, but ONLY when it's entirely edition/format qualifiers — a real + # subtitle is preserved (see _TRAILING_DASH_QUALIFIER_RE). + cleaned = _TRAILING_DASH_QUALIFIER_RE.sub(' ', cleaned) cleaned = re.sub(r'[^a-z0-9 ]+', ' ', cleaned.lower()) cleaned = re.sub(r'\s+', ' ', cleaned).strip() return cleaned @@ -382,7 +399,7 @@ def _extract_volume_marker(normalized_name: str): return last.group(1) or last.group(2) -def _albums_likely_match(spotify_album: str, lib_album: str, threshold: float = 0.6) -> bool: +def _albums_likely_match(spotify_album: str, lib_album: str, threshold: float = 0.85) -> bool: """Return True when two album names plausibly identify the same release. Designed to swallow naming drift between metadata sources and the @@ -406,11 +423,11 @@ def _albums_likely_match(spotify_album: str, lib_album: str, threshold: float = return False if norm_a == norm_b: return True - # After normalization the shorter name often becomes a prefix / - # substring of the longer one ("napoleon dynamite" ⊂ "napoleon - # dynamite music from the motion picture" before stripping). - if norm_a in norm_b or norm_b in norm_a: - return True + # No loose substring shortcut: after qualifier-stripping, a short name being a + # prefix of a longer one is usually a DIFFERENT edition carrying a real subtitle + # ("clair obscur expedition 33" ⊂ "clair obscur expedition 33 nos vies en lumiere"), + # not naming drift. Genuine drift collapses to an EXACT match above; everything else + # must clear a high overall-similarity bar. return SequenceMatcher(None, norm_a, norm_b).ratio() >= threshold diff --git a/tests/test_watchlist_album_match.py b/tests/test_watchlist_album_match.py index cbe9fbf2..2ededda9 100644 --- a/tests/test_watchlist_album_match.py +++ b/tests/test_watchlist_album_match.py @@ -156,6 +156,13 @@ def test_compilation_score_explanation() -> None: ("Inception (Music From The Motion Picture)", "Inception Soundtrack"), # Substring containment ("Random Access Memories", "Random Access Memories (Bonus Edition)"), + # Dash-suffixed qualifiers must still collapse — these are the SAME album, so + # treating them as different would re-wishlist/redownload forever (the failure the + # original blanket strip guarded against; the narrowed strip must keep covering it). + ("Album Name", "Album Name - Single"), + ("Album Name", "Album Name - Acoustic Version"), + ("Hotel California", "Hotel California - 2013 Remaster"), + ("Album", "Album - The Remixes"), ], ) def test_likely_match_positive(spotify_name, lib_name) -> None: @@ -176,12 +183,29 @@ def test_likely_match_positive(spotify_name, lib_name) -> None: ("Abbey Road", "Sgt. Pepper's Lonely Hearts Club Band"), # Same word in title but different album ("Greatest Hits Volume 1", "Greatest Hits Volume 2"), + # Sokhi: distinct editions of the same franchise — the OST vs a bonus edition + # with a real subtitle. USED to collapse to the same normalized name (the blanket + # trailing-dash strip removed "- Nos vies en Lumière"), so the watchlist marked + # unowned OST tracks as owned via the bonus edition. Must be DIFFERENT albums. + ("Clair Obscur: Expedition 33: Original Soundtrack", + "Clair Obscur: Expedition 33 - Nos vies en Lumière (Bonus Edition)"), + # A real subtitle after a dash must not be stripped down to the base name. + ("The Album", "The Album - A Whole Different Subtitle"), ], ) def test_likely_match_negative(spotify_name, lib_name) -> None: assert not _albums_likely_match(spotify_name, lib_name) +def test_real_subtitle_after_dash_is_preserved() -> None: + # the regression's root cause: a meaningful subtitle must survive normalization, + # while a recognized qualifier after a dash ("- Live", "- 2011") still collapses. + assert _normalize_album_for_match("Clair Obscur: Expedition 33 - Nos vies en Lumière") \ + != _normalize_album_for_match("Clair Obscur: Expedition 33") + assert _normalize_album_for_match("Some Album - Live") == _normalize_album_for_match("Some Album") + assert _normalize_album_for_match("Some Album - 2011") == _normalize_album_for_match("Some Album") + + # --------------------------------------------------------------------------- # _albums_likely_match — defensive cases # --------------------------------------------------------------------------- From b1f061a2a8e61377ba732d5ad25e7c3f42c25dbb Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Fri, 26 Jun 2026 21:45:41 -0700 Subject: [PATCH 18/49] manual search: float the pasted Qobuz/Tidal track to the top (#932) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit a pasted track link IS resolved + searched, but the 'bubble the exact track to the top' step read getattr(t,'id') — and TrackResult has no top-level id (the source id lives in _source_metadata['track_id']). so the bubble was a silent no-op: the linked track sat buried among fuzzy text-search lookalikes and the user saw unrelated tracks. qobuz made it worse — _qobuz_to_track_result never stamped _source_metadata at all, so the track had no id to match. - stamp _source_metadata={'source':'qobuz','track_id':...} on qobuz TrackResults (mirrors tidal) - extract the bubble into pure, tested helpers (linked_track_id / bubble_linked_track_first) that read _source_metadata['track_id'] — fixes it for tidal too, str/int-safe, stable no-op 19 track-link tests (+6 new) + 87 qobuz/download tests green. --- core/downloads/track_link.py | 24 +++++++++++++++- core/qobuz_client.py | 4 +++ tests/downloads/test_track_link.py | 44 ++++++++++++++++++++++++++++++ web_server.py | 12 ++++---- 4 files changed, 77 insertions(+), 7 deletions(-) diff --git a/core/downloads/track_link.py b/core/downloads/track_link.py index e6dec8bd..3565a5ff 100644 --- a/core/downloads/track_link.py +++ b/core/downloads/track_link.py @@ -13,9 +13,31 @@ Pure + import-safe: parsing only, no network. from __future__ import annotations import re -from typing import Any, Optional, Tuple +from typing import Any, List, Optional, Tuple from urllib.parse import urlparse + +def linked_track_id(track: Any) -> str: + """The source track id stamped on a search result, read from + ``_source_metadata['track_id']`` — the field every ID-downloadable source + (Tidal, Qobuz) records. Empty string when absent. ``TrackResult`` has no + top-level ``id``, so callers must NOT use ``getattr(t, 'id')`` (that always + missed and left the pasted-link bubble a silent no-op — #932).""" + meta = getattr(track, '_source_metadata', None) + if not isinstance(meta, dict): + return '' + return str(meta.get('track_id') or '') + + +def bubble_linked_track_first(tracks: List[Any], link_track_id: str) -> List[Any]: + """Float the result whose source id matches a pasted link to the top so the + user sees the EXACT track they linked, not a fuzzy text-search lookalike + (#813/#932). Stable + a graceful no-op when no result carries the id.""" + if not link_track_id or not tracks: + return tracks + target = str(link_track_id) + return sorted(tracks, key=lambda t: linked_track_id(t) != target) + # host substring → download source id. Only ID-downloadable streaming sources. _HOSTS = ( ('tidal.com', 'tidal'), diff --git a/core/qobuz_client.py b/core/qobuz_client.py index 1c7c3b6f..e90e934a 100644 --- a/core/qobuz_client.py +++ b/core/qobuz_client.py @@ -1074,6 +1074,10 @@ class QobuzClient(DownloadSourcePlugin): title=title, album=album_name, track_number=track.get('track_number'), + # Stamp the Qobuz track id so a pasted-link manual search can float the + # exact track to the top (#932). Without this the bubble never matched + # and the linked track stayed buried among fuzzy lookalikes. + _source_metadata={'source': 'qobuz', 'track_id': str(track.get('id') or '')}, ) # Stamp real API quality so the global ranker sees actual diff --git a/tests/downloads/test_track_link.py b/tests/downloads/test_track_link.py index 39dfe8a1..06cbc1bb 100644 --- a/tests/downloads/test_track_link.py +++ b/tests/downloads/test_track_link.py @@ -78,3 +78,47 @@ def test_payload_non_dict_or_empty(): assert q('tidal', None) is None assert q('tidal', {}) is None assert q('qobuz', 'garbage') is None + + +# ── bubble the pasted-link track to the top (#932) ── + +from types import SimpleNamespace + +from core.downloads.track_link import linked_track_id, bubble_linked_track_first + + +def _result(track_id=None): + """Mimic a TrackResult: NO top-level `id`, the source id lives in + _source_metadata['track_id'] (or absent entirely).""" + meta = {'source': 'qobuz', 'track_id': track_id} if track_id is not None else None + return SimpleNamespace(_source_metadata=meta, title='t') + + +def test_linked_track_id_reads_source_metadata(): + assert linked_track_id(_result('296427754')) == '296427754' + + +def test_linked_track_id_empty_when_absent(): + # the exact #932 trap: there is no top-level `id`, so getattr(t,'id') would miss. + r = _result() + assert not hasattr(r, 'id') + assert linked_track_id(r) == '' + + +def test_bubble_floats_exact_track_to_top(): + fuzzy_a, exact, fuzzy_b = _result('111'), _result('296427754'), _result('222') + out = bubble_linked_track_first([fuzzy_a, exact, fuzzy_b], '296427754') + assert out[0] is exact # exact track surfaced first + assert out[1:] == [fuzzy_a, fuzzy_b] # stable order for the rest + + +def test_bubble_handles_int_vs_str_id(): + out = bubble_linked_track_first([_result('999'), _result('296427754')], 296427754) + assert linked_track_id(out[0]) == '296427754' + + +def test_bubble_noop_when_nothing_matches_or_empty(): + a, b = _result('1'), _result('2') + assert bubble_linked_track_first([a, b], '296427754') == [a, b] # unchanged + assert bubble_linked_track_first([], '296427754') == [] + assert bubble_linked_track_first([a, b], '') == [a, b] diff --git a/web_server.py b/web_server.py index b87f2ba7..19f9bc52 100644 --- a/web_server.py +++ b/web_server.py @@ -7533,13 +7533,13 @@ def manual_search_for_task(task_id): "error": error, }) + "\n" continue - # Pasted-link exact match: bubble the track whose id matches - # the link to the top so the user sees the exact version - # first (graceful no-op if ids don't line up). + # Pasted-link exact match: bubble the track whose source id + # matches the link to the top so the user sees the exact version + # first. Reads _source_metadata['track_id'] (TrackResult has no + # top-level id) — the old getattr(t,'id') always missed (#932). if src_name == link_source and link_track_id and tracks: - tracks = sorted( - tracks, - key=lambda t: str(getattr(t, 'id', '')) != str(link_track_id)) + from core.downloads.track_link import bubble_linked_track_first + tracks = bubble_linked_track_first(tracks, link_track_id) serialized = [] for t in tracks: s = _serialize_candidate(t, source_override=src_name) From eddaea2f93a22384f66cc6b66ff3e4f96daca5a8 Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Sat, 27 Jun 2026 00:37:35 -0700 Subject: [PATCH 19/49] watchlist history: record automatic scans too (#933) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit save_watchlist_scan_run had a single caller — the manual scan endpoint. the automatic/ scheduled path (process_watchlist_scan_automatically) ran the full scan but never wrote a history row, so nightly scans never showed up in the History modal — only manual ones. - new shared helper persist_scan_run(database, state, ...) extracts the run from the finished watchlist_scan_state and writes one history row - the automatic path now stamps scan_run_id/scan_track_events and calls it - the manual path is refactored onto the same helper so the two can't drift apart again - history is global (no profile filter), so the all-profiles nightly scan records one aggregate row (profile_id None → 1, never NULL) tests: 4 new persist_scan_run seam tests (real DB) + 2 new auto-scan integration tests proving the auto path actually records (completed + cancelled, exactly once). 420 watchlist/automation tests green. --- core/watchlist/auto_scan.py | 17 ++++++++- core/watchlist/scan_history.py | 51 +++++++++++++++++++++++++ tests/test_watchlist_scan_history.py | 56 ++++++++++++++++++++++++++++ tests/watchlist/test_auto_scan.py | 38 +++++++++++++++++++ web_server.py | 20 ++-------- 5 files changed, 165 insertions(+), 17 deletions(-) create mode 100644 core/watchlist/scan_history.py diff --git a/core/watchlist/auto_scan.py b/core/watchlist/auto_scan.py index 759d43f7..e26ab998 100644 --- a/core/watchlist/auto_scan.py +++ b/core/watchlist/auto_scan.py @@ -185,7 +185,11 @@ def process_watchlist_scan_automatically(automation_id=None, profile_id=None, de 'results': [], 'summary': {}, 'error': None, - 'cancel_requested': False + 'cancel_requested': False, + # #933: stamp these so this scan lands in the History modal too — + # the scanner fills scan_track_events; persist_scan_run reads both. + 'scan_run_id': datetime.now().strftime('%Y%m%d-%H%M%S'), + 'scan_track_events': [], } scan_results = [] @@ -294,6 +298,17 @@ def process_watchlist_scan_automatically(automation_id=None, profile_id=None, de total_added_to_wishlist = deps.watchlist_scan_state.get('summary', {}).get('tracks_added_to_wishlist', 0) logger.warning("Automatic watchlist scan cancelled — skipping post-scan steps") + # #933: record this run in the History ledger — same helper the manual + # scan uses, so scheduled scans show up alongside manual ones. + try: + from core.watchlist.scan_history import persist_scan_run + persist_scan_run( + database, deps.watchlist_scan_state, + profile_id=profile_id, was_cancelled=was_cancelled, + ) + except Exception as _hist_err: + logger.error(f"Failed to persist watchlist scan run: {_hist_err}") + # Post-scan steps — skip if cancelled if not was_cancelled: # Populate discovery pool from similar artists (per-profile) diff --git a/core/watchlist/scan_history.py b/core/watchlist/scan_history.py new file mode 100644 index 00000000..a87011ca --- /dev/null +++ b/core/watchlist/scan_history.py @@ -0,0 +1,51 @@ +"""Persist a finished watchlist scan to the History ledger (#831 / #933). + +Both the manual scan (``web_server.start_watchlist_scan``) and the automatic +scan (``core.watchlist.auto_scan.process_watchlist_scan_automatically``) finish +with the same ``watchlist_scan_state`` shape, but only the manual path used to +record a history row — so scheduled/nightly scans never showed up in the +History modal (#933). This single helper is the shared seam: both paths call it, +so they can't drift apart again. + +Pure except for the one ``database.save_watchlist_scan_run`` call — the field +extraction is unit-testable with a fake database. +""" + +from __future__ import annotations + +from datetime import datetime +from typing import Any, Dict, Optional + + +def _iso(value: Any) -> Optional[str]: + """ISO-format a datetime; pass through an already-stringified timestamp.""" + if value is None: + return None + return value.isoformat() if hasattr(value, 'isoformat') else str(value) + + +def persist_scan_run(database: Any, state: Dict[str, Any], *, + profile_id: Any, was_cancelled: bool) -> bool: + """Record one watchlist scan run + its track ledger from ``state``. + + Reads the counts/timestamps/ledger off the live ``watchlist_scan_state`` the + scanner just finished writing, and writes a single history row. ``run_id`` + comes from ``state['scan_run_id']`` (both paths stamp it); a timestamp + fallback keeps it from ever colliding if that's somehow missing. Returns the + DB call's truthiness; callers wrap in their own try/except so a history-write + failure never breaks the scan. + """ + summary = state.get('summary') or {} + run_id = state.get('scan_run_id') or datetime.now().strftime('%Y%m%d-%H%M%S') + return database.save_watchlist_scan_run( + run_id=run_id, + profile_id=profile_id if profile_id else 1, + status='cancelled' if was_cancelled else 'completed', + started_at=_iso(state.get('started_at')), + completed_at=_iso(state.get('completed_at')) or datetime.now().isoformat(), + total_artists=summary.get('total_artists', state.get('total_artists', 0)), + artists_scanned=summary.get('successful_scans', 0), + tracks_found=state.get('tracks_found_this_scan', 0), + tracks_added=state.get('tracks_added_this_scan', 0), + track_events=state.get('scan_track_events') or [], + ) diff --git a/tests/test_watchlist_scan_history.py b/tests/test_watchlist_scan_history.py index 3b6f4924..7281e230 100644 --- a/tests/test_watchlist_scan_history.py +++ b/tests/test_watchlist_scan_history.py @@ -47,6 +47,62 @@ def test_resave_is_idempotent_on_run_id(db): assert len(runs) == 1 and runs[0]['tracks_added'] == 11 +# ── persist_scan_run: the shared seam both scan paths use (#933) ── + +from datetime import datetime + +from core.watchlist.scan_history import persist_scan_run + + +def _state(**over): + """A watchlist_scan_state as the scanner leaves it when a scan finishes.""" + s = { + 'scan_run_id': 'auto-run-1', + 'started_at': datetime(2026, 6, 26, 2, 0, 0), + 'completed_at': datetime(2026, 6, 26, 2, 4, 0), + 'total_artists': 40, + 'tracks_found_this_scan': 7, + 'tracks_added_this_scan': 3, + 'scan_track_events': _events(n_added=3, n_skipped=1), + 'summary': {'total_artists': 40, 'successful_scans': 40, + 'new_tracks_found': 7, 'tracks_added_to_wishlist': 3}, + } + s.update(over) + return s + + +def test_persist_scan_run_records_a_history_row(db): + # the #933 fix: an automatic (all-profiles → profile_id=None) scan must land in History. + assert persist_scan_run(db, _state(), profile_id=None, was_cancelled=False) is True + runs = db.get_watchlist_scan_runs() + assert len(runs) == 1 + r = runs[0] + assert (r['run_id'], r['status'], r['tracks_found'], r['tracks_added']) == \ + ('auto-run-1', 'completed', 7, 3) + assert r['profile_id'] == 1 # None coerced to a concrete profile, never NULL + # the per-run ledger came through too + assert [e['status'] for e in db.get_watchlist_scan_run_events('auto-run-1')] == \ + ['added', 'added', 'added', 'skipped'] + + +def test_persist_scan_run_cancelled_status(db): + persist_scan_run(db, _state(scan_run_id='c1'), profile_id=2, was_cancelled=True) + assert db.get_watchlist_scan_runs()[0]['status'] == 'cancelled' + + +def test_persist_scan_run_accepts_datetime_or_iso_string(db): + # state timestamps may be datetime (auto path) or already-iso strings — both must persist. + persist_scan_run(db, _state(scan_run_id='dt', completed_at='2026-06-26T02:09:00'), + profile_id=1, was_cancelled=False) + assert db.get_watchlist_scan_runs()[0]['run_id'] == 'dt' + + +def test_persist_scan_run_tolerates_sparse_state(db): + # a bare/early-finished state must not raise — history-write must never break a scan. + assert persist_scan_run(db, {'scan_run_id': 'sparse'}, profile_id=None, was_cancelled=False) + assert db.get_watchlist_scan_runs()[0]['tracks_added'] == 0 + + def test_prune_keeps_most_recent(db): for i in range(1, 8): db.save_watchlist_scan_run( diff --git a/tests/watchlist/test_auto_scan.py b/tests/watchlist/test_auto_scan.py index 4ad70258..6ac83413 100644 --- a/tests/watchlist/test_auto_scan.py +++ b/tests/watchlist/test_auto_scan.py @@ -81,6 +81,11 @@ class _FakeDB: self._watchlist_artists = watchlist_artists or [] self._lb_profiles = lb_profiles or [] self.database_path = '/tmp/test.db' + self.scan_runs_saved = [] + + def save_watchlist_scan_run(self, **kwargs): + self.scan_runs_saved.append(kwargs) + return True def get_all_profiles(self): return self._profiles @@ -237,6 +242,39 @@ def test_successful_scan_runs_post_steps(patched_modules): assert deps._state_ref[0]['summary']['new_tracks_found'] == 2 +def test_successful_scan_records_history_run(patched_modules): + """#933: the automatic scan must persist a History row (it used to skip this, + so only manual scans appeared in History).""" + scanner, db = patched_modules + deps = _build_deps() + + autosc.process_watchlist_scan_automatically(automation_id='a1', deps=deps) + + assert len(db.scan_runs_saved) == 1 # exactly one row, no double-record + saved = db.scan_runs_saved[0] + assert saved['status'] == 'completed' + assert saved['artists_scanned'] == 1 # one successful _ScanResult + assert saved['run_id'] # a run id was stamped + + +def test_cancelled_scan_still_records_history_run(patched_modules, monkeypatch): + """A cancelled scan is recorded too, with status='cancelled'.""" + scanner, db = patched_modules + deps = _build_deps() + # Make the scan observe a cancel request mid-run. + orig = scanner.scan_watchlist_artists + def _cancel_during(artists, *, scan_state, progress_callback, cancel_check): + scan_state['cancel_requested'] = True + return orig(artists, scan_state=scan_state, progress_callback=progress_callback, + cancel_check=cancel_check) + monkeypatch.setattr(scanner, 'scan_watchlist_artists', _cancel_during) + + autosc.process_watchlist_scan_automatically(automation_id='a1', deps=deps) + + assert len(db.scan_runs_saved) == 1 + assert db.scan_runs_saved[0]['status'] == 'cancelled' + + def test_completion_emits_automation_event(patched_modules): """Successful scan emits 'watchlist_scan_completed' on automation_engine.""" scanner, _ = patched_modules diff --git a/web_server.py b/web_server.py index 19f9bc52..09614540 100644 --- a/web_server.py +++ b/web_server.py @@ -28223,22 +28223,10 @@ def start_watchlist_scan(): # #831 round 2: persist this run + its track ledger so the # Watchlist History modal can show what every past scan did. try: - _state = watchlist_scan_state - get_database().save_watchlist_scan_run( - run_id=_state.get('scan_run_id') or datetime.now().strftime('%Y%m%d-%H%M%S'), - profile_id=scan_profile_id, - status='cancelled' if was_cancelled else 'completed', - started_at=(_state.get('started_at').isoformat() - if _state.get('started_at') else None), - completed_at=(_state.get('completed_at') or datetime.now()).isoformat() - if not isinstance(_state.get('completed_at'), str) - else _state.get('completed_at'), - total_artists=(_state.get('summary') or {}).get('total_artists', - _state.get('total_artists', 0)), - artists_scanned=(_state.get('summary') or {}).get('successful_scans', 0), - tracks_found=_state.get('tracks_found_this_scan', 0), - tracks_added=_state.get('tracks_added_this_scan', 0), - track_events=_state.get('scan_track_events') or [], + from core.watchlist.scan_history import persist_scan_run + persist_scan_run( + get_database(), watchlist_scan_state, + profile_id=scan_profile_id, was_cancelled=was_cancelled, ) except Exception as _hist_err: logger.error(f"Failed to persist watchlist scan run: {_hist_err}") From 3a92571c719c97e353996e363bc2964fd1814ee8 Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Sat, 27 Jun 2026 09:52:39 -0700 Subject: [PATCH 20/49] downloads: stop AcoustID scan duplicating history rows / leaving verified tracks 'unverified' (#934) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit the AcoustID scanner matched library_history rows by EXACT file_path, but that path is frozen at import time while the file moves afterward (media-server import / reorganize) — so tracks.file_path (what the scan reads) no longer equals it. two failures resulted, both introduced in 37ea6604: verified status never reached the history row (verified tracks kept showing 'unverified'), and a fresh acoustid_scan row was INSERTed every run (5551 rows for 3675 songs). - new pure, tested matcher (core/downloads/history_match.py): exact path → filename guarded by title; prefers a real download row over a synthetic scan row. - _persist_status now HEALS the matched row's path + status (so future scans match cleanly), DELETES synthetic acoustid_scan duplicates by exact path (collision-free, never a real row), and inserts only when the file genuinely has no row. - a full AcoustID job now self-cleans existing duplicates — no destructive bulk migration. 8 matcher + 4 real-DB heal/dedup/insert tests; existing scanner tests updated to the new seam (heal vs insert). 1076 acoustid/verification/download tests green. --- core/downloads/history_match.py | 62 ++++++++++++++++++ core/repair_jobs/acoustid_scanner.py | 41 ++++++++++-- tests/test_acoustid_history_heal.py | 95 ++++++++++++++++++++++++++++ tests/test_acoustid_scanner.py | 44 ++++++++----- tests/test_history_match.py | 63 ++++++++++++++++++ 5 files changed, 282 insertions(+), 23 deletions(-) create mode 100644 core/downloads/history_match.py create mode 100644 tests/test_acoustid_history_heal.py create mode 100644 tests/test_history_match.py diff --git a/core/downloads/history_match.py b/core/downloads/history_match.py new file mode 100644 index 00000000..dadea5e1 --- /dev/null +++ b/core/downloads/history_match.py @@ -0,0 +1,62 @@ +"""Match a file back to its download-history row when its path has drifted (#934). + +``library_history.file_path`` is frozen at import time, but the file moves afterward +(media-server import, library reorganize) and ``tracks.file_path`` — what the AcoustID +scanner reads — no longer equals it. Matching on the exact path alone then fails twice: +the verification status never reaches the history row (verified tracks read "unverified"), +and a fresh ``acoustid_scan`` row gets inserted every run (thousands of duplicates). + +This module picks the canonical history row by exact path first, then by FILENAME guarded +by a title check — so a shared filename ("01 - Intro.flac") can never heal the wrong song. +Pure (no DB) so the matching rules are unit-testable; the caller does the SQL. +""" + +from __future__ import annotations + +import os +from typing import Iterable, Optional, Sequence, Tuple + + +def _norm_title(value) -> str: + """Alphanumeric-only lowercase form, so "Song (Remaster)" vs "song remaster" + style drift between the download tag and the media-server tag still agrees.""" + return ''.join(ch for ch in str(value or '').lower() if ch.isalnum()) + + +def like_filename_filter(basename: str) -> str: + """A ``LIKE ... ESCAPE '\\'`` pattern that coarsely matches rows whose path ends + in ``basename``. Escapes the LIKE metacharacters (``%`` ``_`` ``\\``) — filenames + routinely contain underscores. Callers MUST still confirm with an exact basename + compare (``pick_history_row`` does), since ``'%name'`` also matches ``'xname'``.""" + esc = basename.replace('\\', '\\\\').replace('%', '\\%').replace('_', '\\_') + return '%' + esc + + +def pick_history_row(candidates: Sequence[Tuple], *, current_paths: Iterable[str], + basename: str, title: str) -> Optional[int]: + """Return the id of the history row to update for this file, or None. + + ``candidates``: ``(id, file_path, title, download_source)`` rows the DB pre-filtered + (exact path or filename LIKE). A row matches when its path equals the current path OR + its filename matches AND its title agrees — the title guard prevents a shared filename + ("01 - Intro.flac") from healing a different song's row. Among matches a REAL download + row is preferred over a synthetic ``acoustid_scan`` row, so the scanner heals the + genuine record and the caller can delete the synthetic duplicate. None when nothing + matches safely (caller then inserts a fresh row — the "file SoulSync never downloaded" + intent).""" + paths = {p for p in current_paths if p} + want = _norm_title(title) + matches: list = [] # (id, is_exact, is_real) + for cid, cpath, ctitle, csource in candidates: + is_real = csource != 'acoustid_scan' + if cpath and cpath in paths: + matches.append((cid, True, is_real)) + elif (basename and cpath and os.path.basename(cpath) == basename + and (not want or not _norm_title(ctitle) or _norm_title(ctitle) == want)): + matches.append((cid, False, is_real)) + if not matches: + return None + # Prefer a REAL download row over a synthetic acoustid_scan row; within that, prefer an + # exact-path match over a filename match. Stable, so ties keep DB order (first/oldest id). + matches.sort(key=lambda m: (m[2], m[1]), reverse=True) + return matches[0][0] diff --git a/core/repair_jobs/acoustid_scanner.py b/core/repair_jobs/acoustid_scanner.py index 16ceabda..b52ef6aa 100644 --- a/core/repair_jobs/acoustid_scanner.py +++ b/core/repair_jobs/acoustid_scanner.py @@ -389,14 +389,43 @@ class AcoustIDScannerJob(RepairJob): cur.execute( "UPDATE tracks SET verification_status = ? WHERE id = ?", (status, track_id)) - matched = 0 + exp = expected or {} + # Find the canonical history row for this file. The stored path is frozen at + # import time while the file has since moved (media-server import / reorganize), + # so an exact-path match alone misses it — then the status never lands and a + # duplicate row gets inserted every scan (#934). Match exact path first, then + # filename guarded by title, and HEAL the row's path so future scans match cleanly. + from core.downloads.history_match import pick_history_row, like_filename_filter + current = fpath or db_path + basename = os.path.basename(current) if current else '' + clauses, params = [], [] for p in {p for p in (fpath, db_path) if p}: + clauses.append("file_path = ?") + params.append(p) + if basename: + clauses.append("file_path LIKE ? ESCAPE '\\'") + params.append(like_filename_filter(basename)) + row_id = None + if clauses: cur.execute( - "UPDATE library_history SET verification_status = ? WHERE file_path = ?", - (status, p)) - matched += max(getattr(cur, 'rowcount', 0) or 0, 0) - if status == 'unverified' and matched == 0: - exp = expected or {} + "SELECT id, file_path, title, download_source FROM library_history WHERE " + + " OR ".join(clauses), + params) + row_id = pick_history_row( + cur.fetchall(), + current_paths=(fpath, db_path), + basename=basename, title=exp.get('title') or '') + if row_id is not None: + cur.execute( + "UPDATE library_history SET verification_status = ?, file_path = ? WHERE id = ?", + (status, current, row_id)) + # Drop synthetic scan-created duplicates for this exact file (the #934 + # leftovers). Exact path → collision-free; never touches a real download row. + cur.execute( + "DELETE FROM library_history WHERE id != ? AND download_source = 'acoustid_scan' " + "AND file_path = ?", + (row_id, current)) + elif status == 'unverified': cur.execute( """INSERT INTO library_history (event_type, title, artist_name, album_name, file_path, diff --git a/tests/test_acoustid_history_heal.py b/tests/test_acoustid_history_heal.py new file mode 100644 index 00000000..02108e10 --- /dev/null +++ b/tests/test_acoustid_history_heal.py @@ -0,0 +1,95 @@ +"""#934: the AcoustID scanner heals the download-history row when the file moved, +instead of leaving it stuck 'unverified' and inserting duplicate scan rows. + +Seeds the exact bug shape (a real download row at the OLD import path + a synthetic +'acoustid_scan' duplicate at the NEW library path) and drives the real _persist_status, +asserting it collapses to one correct, verified row. +""" + +from __future__ import annotations + +import types + +import pytest + +from core.repair_jobs.acoustid_scanner import AcoustIDScannerJob +from database.music_database import MusicDatabase + + +@pytest.fixture() +def db(tmp_path): + return MusicDatabase(str(tmp_path / 'm.db')) + + +def _scanner(): + # _persist_status uses only `context`, never instance state — bypass __init__. + return AcoustIDScannerJob.__new__(AcoustIDScannerJob) + + +def _rows(db): + with db._get_connection() as conn: + return [dict(r) for r in conn.execute( + "SELECT file_path, download_source, verification_status FROM library_history")] + + +OLD = '/downloads/transfer/Artist/01 - Song.flac' # frozen import path in history +NEW = '/music/Artist/Album/01 - Song.flac' # where the file lives now (tracks path) + + +def test_verify_heals_drifted_row_and_drops_synthetic_dup(db): + # the bug's leftover state: a stuck real row + a synthetic scan dup for the same song. + db.add_library_history_entry('download', 'Song', file_path=OLD, + download_source='soulseek', verification_status='unverified') + db.add_library_history_entry('download', 'Song', file_path=NEW, + download_source='acoustid_scan', verification_status='unverified') + + _scanner()._persist_status( + types.SimpleNamespace(db=db), track_id='t1', fpath=NEW, db_path=NEW, + status='verified', write_tag=False, expected={'title': 'Song'}) + + rows = _rows(db) + assert len(rows) == 1 # synthetic dup deleted, no new insert + assert rows[0]['download_source'] == 'soulseek' # the REAL row survived + assert rows[0]['verification_status'] == 'verified' + assert rows[0]['file_path'] == NEW # path healed to current location + + +def test_idempotent_no_growth_on_rescan(db): + db.add_library_history_entry('download', 'Song', file_path=OLD, + download_source='soulseek', verification_status='unverified') + ctx = types.SimpleNamespace(db=db) + for _ in range(3): + _scanner()._persist_status(ctx, track_id='t1', fpath=NEW, db_path=NEW, + status='verified', write_tag=False, expected={'title': 'Song'}) + rows = _rows(db) + assert len(rows) == 1 and rows[0]['verification_status'] == 'verified' + + +def test_unknown_file_inserts_one_row_then_dedups(db): + # a file SoulSync never downloaded → first scan inserts one review-queue row... + ctx = types.SimpleNamespace(db=db) + _scanner()._persist_status(ctx, track_id='t1', fpath=NEW, db_path=NEW, + status='unverified', write_tag=False, + expected={'title': 'Song', 'artist': 'Artist'}) + assert len(_rows(db)) == 1 + # ...and a rescan matches it (no duplicate). + _scanner()._persist_status(ctx, track_id='t1', fpath=NEW, db_path=NEW, + status='unverified', write_tag=False, + expected={'title': 'Song', 'artist': 'Artist'}) + rows = _rows(db) + assert len(rows) == 1 and rows[0]['download_source'] == 'acoustid_scan' + + +def test_does_not_heal_wrong_song_with_same_filename(db): + # different song, same filename, different title → must stay untouched (no false heal). + db.add_library_history_entry('download', 'A Different Song', file_path='/other/01 - Song.flac', + download_source='soulseek', verification_status='verified') + _scanner()._persist_status( + types.SimpleNamespace(db=db), track_id='t1', fpath=NEW, db_path=NEW, + status='unverified', write_tag=False, expected={'title': 'Song'}) + rows = _rows(db) + # the unrelated row is untouched, and the unknown file got its own new row. + paths = {r['file_path'] for r in rows} + assert '/other/01 - Song.flac' in paths and NEW in paths + other = next(r for r in rows if r['file_path'] == '/other/01 - Song.flac') + assert other['verification_status'] == 'verified' # not corrupted diff --git a/tests/test_acoustid_scanner.py b/tests/test_acoustid_scanner.py index 1c83cac6..7057c968 100644 --- a/tests/test_acoustid_scanner.py +++ b/tests/test_acoustid_scanner.py @@ -4,8 +4,9 @@ from core.repair_jobs.acoustid_scanner import AcoustIDScannerJob class _FakeCursor: - def __init__(self, rows): + def __init__(self, rows, lib_rows=None): self._rows = rows + self._lib_rows = lib_rows or [] self.executed = [] def execute(self, query, params=None): @@ -13,6 +14,10 @@ class _FakeCursor: return self def fetchall(self): + # The #934 history-match SELECT gets its own (id, file_path, title, source) + # rows; the tracks scan query gets the track rows. + if self.executed and 'FROM library_history' in self.executed[-1][0]: + return self._lib_rows return self._rows def fetchone(self): @@ -20,8 +25,8 @@ class _FakeCursor: class _FakeConnection: - def __init__(self, rows): - self._cursor = _FakeCursor(rows) + def __init__(self, rows, lib_rows=None): + self._cursor = _FakeCursor(rows, lib_rows) def cursor(self): return self._cursor @@ -107,12 +112,13 @@ def test_scan_handles_mixed_track_id_types(monkeypatch): # and finds the primary artist at 100%, suppressing the false flag. -def _make_finding_capturing_context(track_row, captured): +def _make_finding_capturing_context(track_row, captured, lib_rows=None): """Context that captures any create_finding calls into the `captured` list. Tests assert against this list to verify whether the scanner created a finding (false positive) or correctly - skipped (multi-value match resolved).""" - conn = _FakeConnection([track_row]) + skipped (multi-value match resolved). ``lib_rows`` seeds the + library_history match SELECT (#934).""" + conn = _FakeConnection([track_row], lib_rows) config_manager = SimpleNamespace( get=lambda key, default=None: default, set=lambda *args, **kwargs: None, @@ -887,9 +893,10 @@ def test_human_verified_files_are_never_scanned(monkeypatch): # --------------------------------------------------------------------------- -def _run_persistence_scan(monkeypatch, *, file_status, aid_artist, expected_artist): +def _run_persistence_scan(monkeypatch, *, file_status, aid_artist, expected_artist, lib_rows=None): """Drive one _scan_file call and return (status_updates, tag_writes) where - status_updates is the list of (query, params) UPDATEs the scanner ran.""" + status_updates is the list of (query, params) UPDATEs the scanner ran. + ``lib_rows`` seeds the library_history match SELECT (#934).""" import core.repair_jobs.acoustid_scanner as scanner_mod monkeypatch.setattr(scanner_mod, "_resolve_expected_artist_aliases", lambda name: [], raising=False) @@ -905,7 +912,7 @@ def _run_persistence_scan(monkeypatch, *, file_status, aid_artist, expected_arti context = _make_finding_capturing_context( track_row=("9", "Call Your Name", expected_artist, "/music/cyn.flac", 1, "Album", None, None), - captured=captured) + captured=captured, lib_rows=lib_rows) fake = SimpleNamespace(fingerprint_and_lookup=lambda f: { 'best_score': 0.97, 'recordings': [{'title': 'Call Your Name', 'artist': aid_artist}]}) @@ -920,29 +927,32 @@ def _run_persistence_scan(monkeypatch, *, file_status, aid_artist, expected_arti def test_scan_pass_backfills_verified_status(monkeypatch): - # Untagged file + clean fingerprint PASS → the scan backfills 'verified' - # into the tag, the tracks row AND library_history (review-queue feed). + # Clean fingerprint PASS → the scan backfills 'verified' into the tag, the tracks + # row AND the file's library_history row. The history row's path drifted since + # download (file moved), so the scan heals it by id (#934) rather than missing it. updates, tag_writes, captured = _run_persistence_scan( monkeypatch, file_status=None, - aid_artist='Sawano Hiroyuki', expected_artist='Sawano Hiroyuki') + aid_artist='Sawano Hiroyuki', expected_artist='Sawano Hiroyuki', + lib_rows=[(1, '/downloads/old/cyn.flac', 'Call Your Name', 'soulseek')]) assert captured == [] assert tag_writes == [('/music/cyn.flac', 'verified')] assert any('tracks' in q and p == ('verified', '9') for q, p in updates) - assert any('library_history' in q and p == ('verified', '/music/cyn.flac') + # healed by id, status set + path refreshed to the file's current location. + assert any('library_history' in q and p == ('verified', '/music/cyn.flac', 1) for q, p in updates) def test_scan_skip_marks_untagged_file_unverified(monkeypatch): # Title matches but the artist is ambiguous (cover/collab band?) → SKIP. - # An untagged file gets 'unverified' so it surfaces in the Downloads-page - # review queue instead of silently passing or being deleted. + # An untagged file SoulSync never downloaded (no history row) gets a fresh + # 'unverified' row INSERTed so it surfaces in the Downloads-page review queue. updates, tag_writes, captured = _run_persistence_scan( monkeypatch, file_status=None, - aid_artist='Mantilla', expected_artist='Metallica') + aid_artist='Mantilla', expected_artist='Metallica') # no lib_rows → no existing row assert captured == [] assert tag_writes == [('/music/cyn.flac', 'unverified')] assert any('tracks' in q and p == ('unverified', '9') for q, p in updates) - assert any('library_history' in q and p == ('unverified', '/music/cyn.flac') + assert any('INSERT INTO library_history' in q and p[-1] == 'unverified' for q, p in updates) diff --git a/tests/test_history_match.py b/tests/test_history_match.py new file mode 100644 index 00000000..bfb05e8d --- /dev/null +++ b/tests/test_history_match.py @@ -0,0 +1,63 @@ +"""Pure matcher that re-links a moved file to its download-history row (#934).""" + +from core.downloads.history_match import pick_history_row, like_filename_filter + + +# (id, file_path, title, download_source) +def _row(i, path, title='', source='soulseek'): + return (i, path, title, source) + + +def test_exact_current_path_wins(): + cands = [_row(1, '/old/song.flac', 'Song'), _row(2, '/lib/song.flac', 'Song')] + assert pick_history_row(cands, current_paths=('/lib/song.flac', None), + basename='song.flac', title='Song') == 2 + + +def test_falls_back_to_filename_when_path_drifted(): + # history has the OLD import path; scanner only knows the NEW library path. + cands = [_row(7, '/downloads/transfer/Artist/01 - Song.flac', 'Song')] + assert pick_history_row(cands, current_paths=('/music/Artist/Album/01 - Song.flac', None), + basename='01 - Song.flac', title='Song') == 7 + + +def test_title_guard_blocks_shared_filename_collision(): + # two different songs both named "01 - Intro.flac" — must NOT heal the wrong one. + cands = [_row(1, '/a/01 - Intro.flac', 'Album A Intro'), + _row(2, '/b/01 - Intro.flac', 'Album B Intro')] + got = pick_history_row(cands, current_paths=('/c/01 - Intro.flac', None), + basename='01 - Intro.flac', title='Album B Intro') + assert got == 2 + + +def test_title_drift_still_matches_after_normalization(): + cands = [_row(5, '/old/track.flac', 'Song (Remastered)')] + assert pick_history_row(cands, current_paths=('/new/track.flac', None), + basename='track.flac', title='song remastered') == 5 + + +def test_prefers_real_download_row_over_synthetic_scan_row(): + # the #934 collapse: a real download row (drifted path) + a synthetic scan dup at the + # exact current path. The REAL row must win, so the synthetic one can be deleted. + cands = [_row(10, '/old/song.flac', 'Song', source='soulseek'), + _row(11, '/lib/song.flac', 'Song', source='acoustid_scan')] + assert pick_history_row(cands, current_paths=('/lib/song.flac', None), + basename='song.flac', title='Song') == 10 + + +def test_no_basename_match_returns_none(): + cands = [_row(1, '/x/other.flac', 'Other')] + assert pick_history_row(cands, current_paths=('/x/wanted.flac', None), + basename='wanted.flac', title='Wanted') is None + + +def test_filename_only_substring_is_not_a_match(): + # '/x/mysong.flac' must NOT satisfy basename 'song.flac' + cands = [_row(1, '/x/mysong.flac', 'My Song')] + assert pick_history_row(cands, current_paths=('/x/song.flac', None), + basename='song.flac', title='Song') is None + + +def test_like_filter_escapes_metacharacters(): + # underscores/percents in filenames must not become LIKE wildcards + assert like_filename_filter('a_b%c.flac') == r'%a\_b\%c.flac' From a42cf3b8657988b1865dce38142f40f6681415ac Mon Sep 17 00:00:00 2001 From: dev Date: Sat, 27 Jun 2026 19:22:36 +0200 Subject: [PATCH 21/49] downloads(#934): instant startup reconcile of stuck-'unverified' history from tracks truth MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Complements the AcoustID-scan-time heal: re-links library_history rows still showing 'unverified' to the verified/human_verified status their file already carries in the tracks table — matching exact path AND basename, so a file that moved (media-server import / reorganize) heals even though the stored history path is frozen. Upgrade-only and non-destructive (no deletes, no bulk migration). Why this is needed on top of the scan-time fix: - It clears the EXISTING backlog (e.g. 5551 rows) on the next restart with NO re-fingerprinting and no AcoustID API calls — the file's status is already in tracks from the prior scan; this just propagates it to the frozen history row. - It covers human_verified files, which the AcoustID scan skips entirely (file_verif_status == 'human_verified' returns early), so their stale history rows would otherwise never heal. Runs once on DB init (cheap, idempotent). 5 real-sqlite tests. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01LWJk7EuM7YktQeNyqQwTZY --- database/music_database.py | 57 +++++++++ tests/test_unverified_history_reconcile.py | 140 +++++++++++++++++++++ 2 files changed, 197 insertions(+) create mode 100644 tests/test_unverified_history_reconcile.py diff --git a/database/music_database.py b/database/music_database.py index dd37aee3..07a85687 100644 --- a/database/music_database.py +++ b/database/music_database.py @@ -1000,6 +1000,10 @@ class MusicDatabase: self._init_manual_library_match_table() self._backfill_mirrored_track_source_ids() + # Self-heal the Unverified review queue: lift history rows stuck at + # 'unverified' whose file has since been verified (issue #934). Cheap, + # idempotent (only touches rows that need it), so it's safe every boot. + self.reconcile_unverified_history_from_tracks() def _backfill_mirrored_track_source_ids(self) -> int: """One-time, idempotent: assign a stable source_track_id to mirrored tracks @@ -13857,6 +13861,59 @@ class MusicDatabase: logger.error("Error querying unverified library history: %s", e) return [] + def reconcile_unverified_history_from_tracks(self) -> int: + """Heal library_history rows stuck at 'unverified' whose underlying file + has since been confirmed in the tracks table (AcoustID scan PASS or a + human decision). Matches by exact path AND basename — the same physical + file keeps its filename across path-form differences (relative vs + absolute, library moved/reorganized, different mount), which is why an + exact-path-only heal left thousands of already-verified files showing as + Unverified (issue #934). + + Upgrade-only and non-destructive: it only lifts 'unverified' rows to the + confirmed status, never downgrades and never deletes. Returns the number + of rows healed. Genuinely-unverified rows and orphans (no matching + track) are left untouched. + """ + healed = 0 + try: + conn = self._get_connection() + cursor = conn.cursor() + rank = {'verified': 1, 'human_verified': 2} + by_path = {} + by_base = {} + cursor.execute( + "SELECT file_path, verification_status FROM tracks " + "WHERE verification_status IN ('verified', 'human_verified') " + "AND file_path IS NOT NULL AND file_path != ''") + for fp, st in cursor.fetchall(): + if not fp: + continue + by_path[fp] = st + base = os.path.basename(fp) + if base and rank.get(st, 0) >= rank.get(by_base.get(base), 0): + by_base[base] = st + updates = [] + cursor.execute( + "SELECT id, file_path FROM library_history " + "WHERE verification_status = 'unverified' " + "AND file_path IS NOT NULL AND file_path != ''") + for rid, fp in cursor.fetchall(): + target = by_path.get(fp) or by_base.get(os.path.basename(fp or '')) + if target: + updates.append((target, rid)) + for status, rid in updates: + cursor.execute( + "UPDATE library_history SET verification_status = ? WHERE id = ?", + (status, rid)) + healed += 1 + if healed: + conn.commit() + logger.info("Reconciled %d unverified history rows from tracks truth", healed) + except Exception as e: + logger.error("Error reconciling unverified history: %s", e) + return healed + def get_library_history_stats(self): """Return counts per event_type and per download_source.""" try: diff --git a/tests/test_unverified_history_reconcile.py b/tests/test_unverified_history_reconcile.py new file mode 100644 index 00000000..e614b4d3 --- /dev/null +++ b/tests/test_unverified_history_reconcile.py @@ -0,0 +1,140 @@ +"""Issue #934 — one-time reconcile that clears the existing backlog of +``library_history`` rows stuck at 'unverified' even though the file has since +been verified (by an AcoustID scan, or human-confirmed). Heals from the +``tracks`` truth, matching exact path AND basename (so a reorganized/moved file +heals too), upgrade-only. Never deletes anything.""" + +import sqlite3 +import sys +import types + +if "spotipy" not in sys.modules: # match the suite's lightweight stubs + spotipy = types.ModuleType("spotipy") + spotipy.Spotify = object + oauth2 = types.ModuleType("spotipy.oauth2") + oauth2.SpotifyOAuth = object + oauth2.SpotifyClientCredentials = object + spotipy.oauth2 = oauth2 + sys.modules["spotipy"] = spotipy + sys.modules["spotipy.oauth2"] = oauth2 + +if "config.settings" not in sys.modules: + config_pkg = types.ModuleType("config") + settings_mod = types.ModuleType("config.settings") + + class _DummyConfigManager: + def get(self, key, default=None): + return default + + def get_active_media_server(self): + return "primary" + + settings_mod.config_manager = _DummyConfigManager() + config_pkg.settings = settings_mod + sys.modules["config"] = config_pkg + sys.modules["config.settings"] = settings_mod + +from database.music_database import MusicDatabase # noqa: E402 + + +class _NonClosingConn: + def __init__(self, real): + self._real = real + + def cursor(self): + return self._real.cursor() + + def commit(self): + return self._real.commit() + + def close(self): + pass + + def __enter__(self): + return self + + def __exit__(self, *args): + pass + + +class _InMemoryDB(MusicDatabase): + def __init__(self): + self._conn = sqlite3.connect(":memory:") + self._conn.row_factory = sqlite3.Row + self._conn.execute( + "CREATE TABLE tracks (id INTEGER PRIMARY KEY, file_path TEXT, " + "verification_status TEXT)" + ) + self._conn.execute( + "CREATE TABLE library_history (" + "id INTEGER PRIMARY KEY AUTOINCREMENT, event_type TEXT, title TEXT, " + "artist_name TEXT, album_name TEXT, file_path TEXT, " + "download_source TEXT, verification_status TEXT, " + "created_at TIMESTAMP DEFAULT CURRENT_TIMESTAMP)" + ) + + def _get_connection(self): + return _NonClosingConn(self._conn) + + def _add_track(self, tid, path, status): + self._conn.execute( + "INSERT INTO tracks (id, file_path, verification_status) VALUES (?,?,?)", + (tid, path, status)) + self._conn.commit() + + def _add_history(self, path, status, title="Song"): + self._conn.execute( + "INSERT INTO library_history (event_type, title, file_path, " + "verification_status) VALUES ('download', ?, ?, ?)", + (title, path, status)) + self._conn.commit() + + def _status_of(self, hid): + return self._conn.execute( + "SELECT verification_status FROM library_history WHERE id = ?", (hid,) + ).fetchone()[0] + + +def test_reconcile_heals_exact_path_match(): + db = _InMemoryDB() + db._add_track(1, "/lib/A/01 - Song.flac", "verified") + db._add_history("/lib/A/01 - Song.flac", "unverified") + healed = db.reconcile_unverified_history_from_tracks() + assert healed == 1 + assert db._status_of(1) == "verified" + + +def test_reconcile_heals_by_basename_when_path_form_differs(): + db = _InMemoryDB() + db._add_track(1, "/library/Artist/Album/01 - Song.flac", "verified") + # History stored the transfer-folder path; basename still matches. + db._add_history("/transfer/Artist - Album/01 - Song.flac", "unverified") + healed = db.reconcile_unverified_history_from_tracks() + assert healed == 1 + assert db._status_of(1) == "verified" + + +def test_reconcile_propagates_human_verified(): + db = _InMemoryDB() + db._add_track(1, "/lib/01 - Song.flac", "human_verified") + db._add_history("/lib/01 - Song.flac", "unverified") + db.reconcile_unverified_history_from_tracks() + assert db._status_of(1) == "human_verified" + + +def test_reconcile_leaves_genuinely_unverified_rows(): + db = _InMemoryDB() + db._add_track(1, "/lib/01 - Song.flac", "unverified") # track itself unconfirmed + db._add_history("/lib/01 - Song.flac", "unverified") + healed = db.reconcile_unverified_history_from_tracks() + assert healed == 0 + assert db._status_of(1) == "unverified" + + +def test_reconcile_leaves_orphans_untouched(): + db = _InMemoryDB() + # No track references this file at all (deleted / re-downloaded elsewhere). + db._add_history("/lib/gone.flac", "unverified") + healed = db.reconcile_unverified_history_from_tracks() + assert healed == 0 + assert db._status_of(1) == "unverified" From 027cf74d4758c6b196457696bb9eda9bb52f6e64 Mon Sep 17 00:00:00 2001 From: dev Date: Sat, 27 Jun 2026 19:27:28 +0200 Subject: [PATCH 22/49] ui(downloads): Unverified review rows get the Quarantine-style card design MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Quarantine sub-view was reworked into rich cards (artwork, source line, row-click to expand an inline detail panel, consistent action cluster) but the Unverified sub-view was left on the generic download-row layout that opened a modal on click. Bring it to parity: - dedicated _verifUnverifiedRowHtml / _verifUnverifiedRows renderer, used via a sibling branch in _adlRender (mirrors the quarantine sub-view branch). - row click toggles an inline details panel (why flagged, download source, quality, file, downloaded-at), open-state keyed by stable id so it survives the 2 s poll re-render — same pattern as verifQuarInspect. - reuses the existing verif-quar-* / verif-actions / adl-row CSS (no new styles) and the existing play/compare/audit/approve/delete handlers. - NO grouping: each unverified import is its own track (grouping only makes sense for the quarantine alternates), per design intent. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01LWJk7EuM7YktQeNyqQwTZY --- webui/static/pages-extra.js | 90 +++++++++++++++++++++++++++++++++++++ 1 file changed, 90 insertions(+) diff --git a/webui/static/pages-extra.js b/webui/static/pages-extra.js index 560ac261..06392b36 100644 --- a/webui/static/pages-extra.js +++ b/webui/static/pages-extra.js @@ -2760,6 +2760,84 @@ async function verifDelete(hid, btn) { } catch (e) { showToast && showToast('Delete failed', 'error'); } if (btn) btn.disabled = false; } + +// ---- Unverified review rows (Quarantine-style cards) ---- +// The Unverified sub-view used to piggyback on the generic download row and +// open a modal on click. These give it the same card design the Quarantine +// sub-view got — row click expands an inline detail panel in place — minus the +// grouping (each unverified import is its own track, nothing to group). +let _verifUnvData = []; +const _verifUnvOpenDetails = new Set(); + +function _verifUnvKey(dl) { + return verifHistoryId(dl) || dl.task_id || ''; +} + +function _verifUnvDetailRows(dl) { + const reason = dl.verification_status === 'force_imported' + ? 'Accepted as the best available candidate after the retry budget was exhausted (version-mismatch fallback). A library AcoustID scan reports these as informational.' + : 'AcoustID could not hard-confirm this file (ambiguous / cross-script metadata / no fingerprint match). Imported, but not verified.'; + const source = dl.batch_source || dl.batch_name || ''; + return [ + `
Why flagged: ${_adlEsc(reason)}
`, + source ? `
Download source: ${_adlEsc(source)}
` : '', + dl.quality ? `
Quality: ${_adlEsc(dl.quality)}
` : '', + dl.file_path ? `
File: ${_adlEsc(dl.file_path)}
` : '', + dl.created_at ? `
Downloaded: ${_adlEsc(dl.created_at)}
` : '', + ].filter(Boolean).join(''); +} + +function _verifUnverifiedRowHtml(dl, idx) { + const hid = verifHistoryId(dl); + const title = _adlEsc(dl.title || 'Unknown Track'); + const meta = [_adlEsc(dl.artist || ''), _adlEsc(dl.album || '')].filter(Boolean).join(' · '); + const source = _adlEsc(dl.batch_source || dl.batch_name || ''); + const timeAgo = _verifTimeAgo(dl.created_at); + const artHtml = dl.artwork + ? `` + : '
'; + const detailsOpen = _verifUnvOpenDetails.has(_verifUnvKey(dl)); + const actions = hid ? ` + + + + + ` : ''; + return `
+ ${artHtml} +
+
${title}
+ ${meta ? `
${meta}
` : ''} + ${source ? `
${source}
` : ''} +
${_verifUnvDetailRows(dl)}
+
+
+ ${_verifReasonBadge(dl)} + ${dl.quality ? `${_adlEsc(dl.quality)}` : ''} + ${timeAgo ? `${timeAgo}` : ''} + ${actions} +
+
`; +} + +function _verifUnverifiedRows(items) { + _verifUnvData = items; + if (!items.length) return ''; + return items.map((dl, idx) => _verifUnverifiedRowHtml(dl, idx)).join(''); +} + +function verifUnvInspect(idx) { + // Open-state lives in a Set keyed by the row's stable id (not the DOM) so it + // survives the 2 s polling re-render — same pattern as verifQuarInspect. + const dl = _verifUnvData[idx]; + if (!dl) return; + const key = _verifUnvKey(dl); + if (_verifUnvOpenDetails.has(key)) _verifUnvOpenDetails.delete(key); + else _verifUnvOpenDetails.add(key); + const el = document.getElementById(`verif-unv-details-${idx}`); + if (el) el.style.display = _verifUnvOpenDetails.has(key) ? '' : 'none'; +} + // ---- Quarantine sub-view inside the ⚠ filter ---- // The review queue covers BOTH kinds of suspect files: imported-but- // unconfirmed (unverified/force_imported) and not-imported-at-all @@ -3293,6 +3371,18 @@ function _adlRender() { return; } + // Unverified review sub-view: render the Quarantine-style cards (inline + // expandable details), not the generic download rows. + if (_adlFilter === 'unverified' && _verifSubView === 'unverified') { + const uhtml = _verifUnverifiedRows(filtered); + const uEmptyEl = document.getElementById('adl-empty'); + const uEmptyHtml = uEmptyEl ? uEmptyEl.outerHTML : ''; + list.innerHTML = uEmptyHtml + uhtml; + const uNewEmpty = document.getElementById('adl-empty'); + if (uNewEmpty) uNewEmpty.style.display = uhtml ? 'none' : ''; + return; + } + if (filtered.length === 0) { if (empty) empty.style.display = ''; // Clear any existing rows but keep the empty message From 28a539a8400744c700cfbd01799fdc1a7fee3ba6 Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Sat, 27 Jun 2026 10:48:07 -0700 Subject: [PATCH 23/49] ui: default Background Particles OFF (#935) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit the full-page particle canvas runs a continuous requestAnimationFrame loop behind every page — real GPU cost, and multiple users hit GPU strain until they found the toggle. flip the default to off; the eye candy is opt-in now. - init.js: runtime flag defaults false unless localStorage is explicitly 'true' - settings.js: config read is now '=== true' (default off) instead of '!== false' - index.html: checkbox no longer 'checked' by default; hint reworded existing users who explicitly enabled it (localStorage/config 'true') keep it on; the existing '!== false' runtime guards still work since the flag is now always set explicitly. --- webui/index.html | 4 ++-- webui/static/init.js | 7 ++++--- webui/static/settings.js | 2 +- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/webui/index.html b/webui/index.html index 30925a59..cfc502c5 100644 --- a/webui/index.html +++ b/webui/index.html @@ -6263,10 +6263,10 @@
- Animated particle effects behind each page. Disable to reduce GPU usage. + Animated particle effects behind each page. Off by default — enable for the eye candy (uses GPU).