From 123eb6139f19d4bf8a2c42a366a7b35a11696059 Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Thu, 11 Jun 2026 16:51:50 -0700 Subject: [PATCH 01/64] =?UTF-8?q?Artist=20detail:=20"DB=20Record"=20inspec?= =?UTF-8?q?tor=20=E2=80=94=20everything=20the=20DB=20knows=20about=20an=20?= =?UTF-8?q?artist?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A small glowing button at the bottom-right of the artist hero (library artists only) opens a programmer-style modal showing the COMPLETE artists DB row — every source id + match status, cached bios / tags / similar / urls, soul_id, timestamps, the lot (62 columns) — plus owned album/track counts. - Backend: GET /api/artist//record returns the full row with JSON-text columns (genres, aliases, lastfm_tags/similar, discogs_urls, …) decoded into real arrays/objects, + album/track counts. 404 for non-library artists. - Frontend: editor-themed modal (Tokyo-night tokens) with a Fields tab (copyable, filterable key/value rows) and a syntax-highlighted JSON tab. Copy-all-as-JSON, per-value copy (HTTP/Docker clipboard fallback), and Save .json. Esc / click-out to close. Helpers namespaced (_arecEsc) so they can't clobber the shared globals. Tests: endpoint returns the full row with decoded JSON + counts; 404 for a missing artist. 64 script-split integrity tests still green; ruff clean. --- tests/test_artist_db_record_endpoint.py | 59 +++++++ web_server.py | 53 ++++++ webui/static/library.js | 222 ++++++++++++++++++++++++ webui/static/style.css | 200 +++++++++++++++++++++ 4 files changed, 534 insertions(+) create mode 100644 tests/test_artist_db_record_endpoint.py diff --git a/tests/test_artist_db_record_endpoint.py b/tests/test_artist_db_record_endpoint.py new file mode 100644 index 00000000..d2671019 --- /dev/null +++ b/tests/test_artist_db_record_endpoint.py @@ -0,0 +1,59 @@ +"""GET /api/artist//record — the artist-detail "DB Record" inspector source. +Returns the full artists row (JSON text columns decoded) + owned counts, 404 if +the artist isn't in the library.""" + +from __future__ import annotations + +import os +import sqlite3 +import tempfile + +import pytest + +_TMP = tempfile.mkdtemp(prefix='soulsync-testdb-arec-') +os.environ['DATABASE_PATH'] = os.path.join(_TMP, 'a.db') +os.environ['SOULSYNC_TEST_DB_READY'] = '1' + +web_server = pytest.importorskip('web_server') + + +@pytest.fixture +def client(): + return web_server.app.test_client() + + +def _insert_artist(): + db = web_server.get_database() + conn = db._get_connection() + try: + conn.execute( + "INSERT OR REPLACE INTO artists (id, name, genres, musicbrainz_id, " + "musicbrainz_match_status, lastfm_listeners) VALUES (?,?,?,?,?,?)", + ('99001', 'Test Artist', '["rock", "metal"]', + 'mbid-123', 'matched', 4242), + ) + conn.commit() + finally: + conn.close() + + +def test_record_returns_full_row_with_decoded_json(client): + _insert_artist() + r = client.get('/api/artist/99001/record') + assert r.status_code == 200 + body = r.get_json() + assert body['success'] is True + rec = body['record'] + assert rec['name'] == 'Test Artist' + assert rec['genres'] == ['rock', 'metal'] # JSON text decoded to a list + assert rec['musicbrainz_id'] == 'mbid-123' + assert rec['musicbrainz_match_status'] == 'matched' + assert rec['lastfm_listeners'] == 4242 + assert 'counts' in body and 'albums' in body['counts'] and 'tracks' in body['counts'] + assert body['artist_id'] == '99001' + + +def test_missing_artist_is_404(client): + r = client.get('/api/artist/does-not-exist-77777/record') + assert r.status_code == 404 + assert r.get_json()['success'] is False diff --git a/web_server.py b/web_server.py index 200cea1a..13206f45 100644 --- a/web_server.py +++ b/web_server.py @@ -9316,6 +9316,59 @@ def get_album_tracks(album_id): logger.exception("Error fetching album tracks for album %s", album_id) return jsonify({"error": str(e)}), 500 +@app.route('/api/artist//record', methods=['GET']) +def get_artist_db_record(artist_id): + """Return the COMPLETE database record for a library artist — every column of + the ``artists`` row (all source IDs + match statuses, cached bios / tags / + similar / urls, timestamps, soul_id, etc.) plus owned album/track counts. + + Powers the artist-detail "DB Record" inspector. JSON-encoded text columns + (genres, aliases, lastfm_tags/similar, discogs_urls, …) are decoded into real + arrays/objects so the dump is clean rather than escaped strings. + """ + try: + database = get_database() + conn = database._get_connection() + try: + cur = conn.cursor() + cur.execute("SELECT * FROM artists WHERE id = ?", (str(artist_id),)) + row = cur.fetchone() + if row is None: + return jsonify({"success": False, "error": "Artist not found in library"}), 404 + + record = {} + for key in row.keys(): + val = row[key] + if isinstance(val, str): + s = val.strip() + if s and s[0] in '[{': + try: + val = json.loads(s) + except Exception: # noqa: S110 — leave non-JSON text as-is + pass + record[key] = val + + counts = {} + for label, table in (('albums', 'albums'), ('tracks', 'tracks')): + try: + cur.execute(f"SELECT COUNT(*) FROM {table} WHERE artist_id = ?", (str(artist_id),)) + counts[label] = cur.fetchone()[0] + except Exception: + counts[label] = None + finally: + conn.close() + + return jsonify({ + "success": True, + "artist_id": str(artist_id), + "counts": counts, + "record": record, + }) + except Exception as e: + logger.error(f"Artist DB record fetch failed for {artist_id}: {e}") + return jsonify({"success": False, "error": str(e)}), 500 + + @app.route('/api/artist//download-discography', methods=['POST']) def download_discography(artist_id): """Add selected albums from an artist's discography to the wishlist. diff --git a/webui/static/library.js b/webui/static/library.js index 4eaf6c03..4adcda46 100644 --- a/webui/static/library.js +++ b/webui/static/library.js @@ -1226,6 +1226,9 @@ function populateArtistDetailPage(data) { // Update hero section with image, name, and stats updateArtistHeroSection(artist, discography); + // "DB Record" inspector button (library artists only) + setupArtistRecordButton(artist); + // Update genres (if element exists) updateArtistGenres(artist.genres); @@ -8788,3 +8791,222 @@ async function _mlmDeleteMatch(id) { } // ================================= + + +// ════════════════════════════════════════════════════════════════════════════ +// Artist "DB Record" inspector — everything the database knows about an artist. +// A small glowing button at the bottom-right of the hero opens a programmer-style +// modal: a copyable field table + syntax-highlighted raw JSON, with copy-all and +// save-as-JSON. Library artists only (source artists have no DB row). +// ════════════════════════════════════════════════════════════════════════════ + +let _artistRecordData = null; // last-fetched { artist_id, counts, record } + +function setupArtistRecordButton(artist) { + const hero = document.getElementById('artist-hero-section'); + if (!hero) return; + let btn = document.getElementById('artist-db-record-btn'); + + const isLibrary = !!(artist && artist.id && document.body.dataset.artistSource === 'library'); + if (!isLibrary) { if (btn) btn.style.display = 'none'; return; } + + if (!btn) { + btn = document.createElement('button'); + btn.id = 'artist-db-record-btn'; + btn.className = 'artist-db-record-btn'; + btn.type = 'button'; + btn.title = 'Inspect everything the database knows about this artist'; + btn.innerHTML = + 'DB Record'; + hero.appendChild(btn); + } + btn.style.display = ''; + btn.onclick = () => openArtistRecordModal(artist.id, artist.name || 'Artist'); +} + +async function openArtistRecordModal(artistId, artistName) { + // Clean any prior instance + const existing = document.getElementById('artist-record-overlay'); + if (existing) existing.remove(); + + const overlay = document.createElement('div'); + overlay.id = 'artist-record-overlay'; + overlay.className = 'arec-overlay'; + overlay.innerHTML = + ''; + document.body.appendChild(overlay); + requestAnimationFrame(() => overlay.classList.add('visible')); + + const close = () => { + overlay.classList.remove('visible'); + document.removeEventListener('keydown', onKey); + setTimeout(() => overlay.remove(), 220); + }; + const onKey = (e) => { if (e.key === 'Escape') close(); }; + document.addEventListener('keydown', onKey); + overlay.addEventListener('click', (e) => { if (e.target === overlay) close(); }); + overlay.querySelector('#arec-close').onclick = close; + + // Fetch the record + let payload; + try { + const res = await fetch(`/api/artist/${encodeURIComponent(artistId)}/record`); + payload = await res.json(); + if (!payload || !payload.success) throw new Error((payload && payload.error) || 'Request failed'); + } catch (err) { + document.getElementById('arec-body').innerHTML = + '
Could not load record: ' + _arecEsc(err.message || String(err)) + '
'; + return; + } + + _artistRecordData = payload; + const record = payload.record || {}; + const counts = payload.counts || {}; + + // Footer stat line + const fieldCount = Object.keys(record).length; + const matched = Object.entries(record).filter(([k, v]) => /match_status$/.test(k) && v === 'matched').length; + document.getElementById('arec-footer').innerHTML = + '' + fieldCount + ' fields' + + '' + (counts.albums != null ? counts.albums : '–') + ' albums' + + '' + (counts.tracks != null ? counts.tracks : '–') + ' tracks' + + '' + matched + ' sources matched' + + 'id ' + _arecEsc(String(payload.artist_id)) + ''; + + _arecRenderFields(record); + + // Toolbar wiring + overlay.querySelectorAll('.arec-tab').forEach(tab => { + tab.onclick = () => { + overlay.querySelectorAll('.arec-tab').forEach(t => t.classList.remove('active')); + tab.classList.add('active'); + const filterEl = document.getElementById('arec-filter'); + if (tab.dataset.tab === 'json') { _arecRenderJson(record); filterEl.style.visibility = 'hidden'; } + else { _arecRenderFields(record); filterEl.style.visibility = ''; _arecApplyFilter(filterEl.value); } + }; + }); + document.getElementById('arec-filter').addEventListener('input', (e) => _arecApplyFilter(e.target.value)); + document.getElementById('arec-copy').onclick = () => + _arecCopy(JSON.stringify(record, null, 2), 'Full record copied as JSON'); + document.getElementById('arec-download').onclick = () => _arecDownload(record, artistName); +} + +function _arecRenderFields(record) { + const body = document.getElementById('arec-body'); + if (!body) return; + const rows = Object.entries(record).map(([key, val]) => { + const isEmpty = val === null || val === undefined || val === ''; + let display, copyVal; + if (isEmpty) { display = 'null'; copyVal = ''; } + else if (typeof val === 'object') { + copyVal = JSON.stringify(val); + display = '' + _arecEsc(JSON.stringify(val)) + ''; + } else { + copyVal = String(val); + display = _arecEsc(String(val)); + } + return '
' + + '' + _arecEsc(key) + '' + + '' + display + '' + + '' + + '
'; + }).join(''); + body.innerHTML = '
' + rows + '
'; + body.querySelectorAll('.arec-rowcopy').forEach(b => { + b.onclick = () => _arecCopy(b.getAttribute('data-copy'), 'Value copied'); + }); +} + +function _arecRenderJson(record) { + const body = document.getElementById('arec-body'); + if (!body) return; + body.innerHTML = '
' + _jsonSyntaxHighlight(record) + '
'; +} + +function _arecApplyFilter(q) { + q = (q || '').trim().toLowerCase(); + document.querySelectorAll('#arec-body .arec-row').forEach(row => { + row.style.display = (!q || row.dataset.field.includes(q)) ? '' : 'none'; + }); +} + +function _jsonSyntaxHighlight(obj) { + let json = JSON.stringify(obj, null, 2); + json = json.replace(/&/g, '&').replace(//g, '>'); + return json.replace(/("(\\u[a-zA-Z0-9]{4}|\\[^u]|[^\\"])*"(\s*:)?|\b(true|false)\b|\bnull\b|-?\d+(?:\.\d*)?(?:[eE][+\-]?\d+)?)/g, (m) => { + let cls = 'tok-num'; + if (/^"/.test(m)) cls = /:$/.test(m) ? 'tok-key' : 'tok-str'; + else if (/true|false/.test(m)) cls = 'tok-bool'; + else if (/null/.test(m)) cls = 'tok-null'; + return '' + m + ''; + }); +} + +function _arecCopy(text, label) { + text = text == null ? '' : String(text); + const done = () => (typeof showToast === 'function') && showToast(label || 'Copied', 'success'); + if (navigator.clipboard && window.isSecureContext) { + navigator.clipboard.writeText(text).then(done).catch(() => _arecCopyFallback(text, done)); + } else { _arecCopyFallback(text, done); } +} + +function _arecCopyFallback(text, done) { + const ta = document.createElement('textarea'); + ta.value = text; + ta.style.cssText = 'position:fixed;left:-9999px'; + document.body.appendChild(ta); + ta.select(); + try { document.execCommand('copy'); } catch (e) { /* ignore */ } + document.body.removeChild(ta); + done(); +} + +function _arecDownload(record, artistName) { + const safe = String(artistName || 'artist').replace(/[^a-z0-9._-]+/gi, '_').slice(0, 60) || 'artist'; + const blob = new Blob([JSON.stringify(record, null, 2)], { type: 'application/json' }); + const url = URL.createObjectURL(blob); + const a = document.createElement('a'); + a.href = url; + a.download = safe + '_db_record.json'; + document.body.appendChild(a); + a.click(); + a.remove(); + setTimeout(() => URL.revokeObjectURL(url), 1000); + if (typeof showToast === 'function') showToast('Saved ' + a.download, 'success'); +} + +function _arecEsc(s) { + return String(s == null ? '' : s) + .replace(/&/g, '&').replace(//g, '>'); +} +function _arecEscAttr(s) { + return _arecEsc(s).replace(/"/g, '"'); +} diff --git a/webui/static/style.css b/webui/static/style.css index 754d32c7..69f45953 100644 --- a/webui/static/style.css +++ b/webui/static/style.css @@ -67977,3 +67977,203 @@ body.em-scroll-lock { overflow: hidden; } font-weight: 500; margin-bottom: 6px; } + + +/* ════════════════════════════════════════════════════════════════════════ + Artist "DB Record" inspector — button + programmer-style modal + ════════════════════════════════════════════════════════════════════════ */ +#artist-hero-section { position: relative; } + +.artist-db-record-btn { + position: absolute; + bottom: 14px; + right: 16px; + z-index: 6; + display: inline-flex; + align-items: center; + gap: 7px; + padding: 7px 13px; + font: 600 11.5px/1 'JetBrains Mono', ui-monospace, 'SF Mono', Menlo, Consolas, monospace; + letter-spacing: 0.04em; + color: #cdd6f4; + background: linear-gradient(135deg, rgba(20,24,34,0.72), rgba(14,17,23,0.82)); + border: 1px solid rgba(122,162,247,0.35); + border-radius: 999px; + cursor: pointer; + backdrop-filter: blur(8px); + box-shadow: 0 4px 18px rgba(0,0,0,0.35), inset 0 0 0 1px rgba(255,255,255,0.03), + 0 0 0 0 rgba(122,162,247,0.0); + opacity: 0.82; + transition: transform .2s cubic-bezier(.4,0,.2,1), box-shadow .25s ease, + opacity .2s ease, border-color .25s ease, color .2s ease; +} +.artist-db-record-btn svg { color: #7aa2f7; transition: color .2s ease, transform .3s ease; } +.artist-db-record-btn:hover { + opacity: 1; + transform: translateY(-2px); + color: #fff; + border-color: rgba(122,162,247,0.7); + box-shadow: 0 8px 26px rgba(0,0,0,0.45), inset 0 0 0 1px rgba(255,255,255,0.05), + 0 0 18px rgba(122,162,247,0.35); +} +.artist-db-record-btn:hover svg { color: #9ece6a; transform: rotate(-6deg) scale(1.08); } +.artist-db-record-btn:active { transform: translateY(0); } + +/* ── Overlay + card ── */ +.arec-overlay { + position: fixed; inset: 0; z-index: 10050; + display: flex; align-items: center; justify-content: center; + padding: 24px; + background: rgba(8,10,14,0.7); + backdrop-filter: blur(10px); + opacity: 0; transition: opacity .22s ease; +} +.arec-overlay.visible { opacity: 1; } + +.arec-card { + width: min(760px, 96vw); + max-height: min(82vh, 820px); + display: flex; flex-direction: column; + background: linear-gradient(180deg, #11151c, #0d1017); + border: 1px solid rgba(122,162,247,0.22); + border-radius: 16px; + box-shadow: 0 30px 80px rgba(0,0,0,0.6), 0 0 0 1px rgba(255,255,255,0.02), + 0 0 60px rgba(122,162,247,0.08); + overflow: hidden; + transform: translateY(10px) scale(0.985); + transition: transform .26s cubic-bezier(.34,1.3,.5,1); + font-family: ui-sans-serif, system-ui, sans-serif; +} +.arec-overlay.visible .arec-card { transform: translateY(0) scale(1); } + +.arec-header { + display: flex; align-items: center; justify-content: space-between; + padding: 14px 16px 12px; + border-bottom: 1px solid rgba(255,255,255,0.06); + background: linear-gradient(180deg, rgba(122,162,247,0.06), transparent); +} +.arec-title { + display: flex; align-items: center; gap: 8px; + font: 700 14px/1 'JetBrains Mono', ui-monospace, Menlo, monospace; + color: #e6ebff; letter-spacing: 0.02em; +} +.arec-dot { + width: 9px; height: 9px; border-radius: 50%; + background: #9ece6a; box-shadow: 0 0 10px #9ece6a; + animation: arecPulse 2.4s ease-in-out infinite; +} +@keyframes arecPulse { 0%,100% { opacity: 1; } 50% { opacity: 0.4; } } +.arec-sub { + margin-top: 4px; font-size: 12px; color: #8b93b0; + font-family: ui-monospace, Menlo, monospace; +} +.arec-close { + background: none; border: none; color: #8b93b0; font-size: 24px; + line-height: 1; cursor: pointer; padding: 0 4px; transition: color .15s ease, transform .15s ease; +} +.arec-close:hover { color: #ff6b6b; transform: scale(1.12); } + +/* ── Toolbar ── */ +.arec-toolbar { + display: flex; align-items: center; gap: 10px; + padding: 10px 14px; + border-bottom: 1px solid rgba(255,255,255,0.06); + flex-wrap: wrap; +} +.arec-tabs { display: inline-flex; background: rgba(255,255,255,0.04); border-radius: 8px; padding: 2px; } +.arec-tab { + border: none; background: none; cursor: pointer; + padding: 5px 12px; border-radius: 6px; + font: 600 11.5px/1 'JetBrains Mono', ui-monospace, monospace; + color: #8b93b0; transition: all .15s ease; +} +.arec-tab.active { background: rgba(122,162,247,0.22); color: #cdd6f4; } +.arec-tab:not(.active):hover { color: #cdd6f4; } +.arec-filter { + flex: 1; min-width: 120px; + padding: 7px 11px; + background: rgba(0,0,0,0.35); + border: 1px solid rgba(255,255,255,0.08); + border-radius: 8px; + color: #cdd6f4; + font: 12px/1 ui-monospace, Menlo, monospace; + outline: none; transition: border-color .15s ease, box-shadow .15s ease; +} +.arec-filter:focus { border-color: rgba(122,162,247,0.5); box-shadow: 0 0 0 3px rgba(122,162,247,0.12); } +.arec-actions { display: inline-flex; gap: 8px; } +.arec-btn { + display: inline-flex; align-items: center; gap: 6px; + padding: 7px 12px; + background: rgba(122,162,247,0.1); + border: 1px solid rgba(122,162,247,0.25); + border-radius: 8px; + color: #c0caf5; cursor: pointer; + font: 600 11.5px/1 'JetBrains Mono', ui-monospace, monospace; + transition: all .15s ease; +} +.arec-btn:hover { background: rgba(122,162,247,0.2); border-color: rgba(122,162,247,0.55); color: #fff; } +.arec-btn svg { opacity: 0.85; } + +/* ── Body ── */ +.arec-body { + flex: 1; overflow: auto; padding: 8px 6px 8px 14px; + font-family: ui-monospace, 'JetBrains Mono', Menlo, Consolas, monospace; +} +.arec-loading, .arec-error { padding: 28px; text-align: center; color: #8b93b0; font-size: 13px; } +.arec-error { color: #ff8585; } + +.arec-fields { display: flex; flex-direction: column; } +.arec-row { + display: grid; grid-template-columns: minmax(140px, 210px) 1fr auto; + gap: 10px; align-items: start; + padding: 6px 8px; border-radius: 7px; + border-bottom: 1px solid rgba(255,255,255,0.035); + transition: background .12s ease; +} +.arec-row:hover { background: rgba(122,162,247,0.07); } +.arec-row.is-empty { opacity: 0.5; } +.arec-key { color: #7aa2f7; font-size: 12px; font-weight: 600; word-break: break-all; } +.arec-val { color: #c8d0e8; font-size: 12px; word-break: break-word; white-space: pre-wrap; } +.arec-val .arec-json { color: #9ece6a; } +.arec-val .arec-null, .arec-null { color: #565f89; font-style: italic; } +.arec-rowcopy { + background: none; border: none; cursor: pointer; + color: #565f89; font-size: 13px; padding: 1px 5px; border-radius: 5px; + opacity: 0; transition: opacity .12s ease, color .12s ease, background .12s ease; +} +.arec-row:hover .arec-rowcopy { opacity: 1; } +.arec-rowcopy:hover { color: #9ece6a; background: rgba(255,255,255,0.06); } + +.arec-code { + margin: 0; padding: 6px 8px; font-size: 12px; line-height: 1.55; + color: #c0caf5; white-space: pre; tab-size: 2; +} +.arec-code .tok-key { color: #7aa2f7; } +.arec-code .tok-str { color: #9ece6a; } +.arec-code .tok-num { color: #ff9e64; } +.arec-code .tok-bool { color: #bb9af7; } +.arec-code .tok-null { color: #565f89; font-style: italic; } + +/* ── Footer ── */ +.arec-footer { + display: flex; align-items: center; gap: 16px; flex-wrap: wrap; + padding: 10px 16px; + border-top: 1px solid rgba(255,255,255,0.06); + background: rgba(0,0,0,0.25); + font: 11px/1 'JetBrains Mono', ui-monospace, monospace; + color: #8b93b0; +} +.arec-footer b { color: #cdd6f4; } +.arec-footer .arec-id { margin-left: auto; color: #565f89; } + +/* scrollbar */ +.arec-body::-webkit-scrollbar { width: 10px; } +.arec-body::-webkit-scrollbar-thumb { background: rgba(122,162,247,0.25); border-radius: 8px; border: 2px solid transparent; background-clip: padding-box; } +.arec-body::-webkit-scrollbar-thumb:hover { background: rgba(122,162,247,0.45); background-clip: padding-box; } + +@media (max-width: 640px) { + .artist-db-record-btn span { display: none; } + .artist-db-record-btn { padding: 8px; } + .arec-row { grid-template-columns: 1fr; gap: 2px; } + .arec-rowcopy { opacity: 1; justify-self: end; margin-top: -22px; } +} From a8206012aec1fa405e35e4d49f27f154f267ce1f Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Thu, 11 Jun 2026 17:24:47 -0700 Subject: [PATCH 02/64] dev: make the dev-server bind host opt-in via SOULSYNC_WEB_BIND_HOST MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The dev gunicorn config hard-bound 127.0.0.1:8008, so the dev server was unreachable from any other device — even via the host's own LAN IP. Read the bind host/port from SOULSYNC_WEB_BIND_HOST / SOULSYNC_WEB_BIND_PORT (same vars the direct web_server.py run already uses), defaulting to the existing 127.0.0.1:8008. Default behavior is unchanged (localhost-only), so other devs notice nothing. To reach the dev server from another device on the LAN, opt in explicitly: SOULSYNC_WEB_BIND_HOST=0.0.0.0 python dev.py --- gunicorn.dev.conf.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/gunicorn.dev.conf.py b/gunicorn.dev.conf.py index 43068ff3..026d003c 100644 --- a/gunicorn.dev.conf.py +++ b/gunicorn.dev.conf.py @@ -3,7 +3,11 @@ from pathlib import Path import os -bind = "127.0.0.1:8008" +# Localhost-only by default (a dev server shouldn't expose itself to the LAN +# without asking). To reach it from another device on your network, opt in with +# SOULSYNC_WEB_BIND_HOST=0.0.0.0 python dev.py +# then browse to http://:8008 (and allow port 8008 in the firewall). +bind = f"{os.environ.get('SOULSYNC_WEB_BIND_HOST', '127.0.0.1')}:{os.environ.get('SOULSYNC_WEB_BIND_PORT', '8008')}" worker_class = "gthread" workers = 1 threads = 4 From 31ebe96f76ada26847bf0bf4c8adabc70bb04f49 Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Thu, 11 Jun 2026 17:29:12 -0700 Subject: [PATCH 03/64] dev: add `--lan` flag to dev.py to expose the dev server on the network Typing SOULSYNC_WEB_BIND_HOST=0.0.0.0 every launch is awkward. `python dev.py --lan` sets it for you (binds 0.0.0.0 so other devices can reach it); plain `python dev.py` stays localhost-only. Set before build_backend_env() so it propagates to both the direct (Windows) and gunicorn backend modes. --- dev.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/dev.py b/dev.py index 95ca24bd..ea86e1bc 100755 --- a/dev.py +++ b/dev.py @@ -316,6 +316,14 @@ def watch_and_run_backend() -> None: def main() -> int: + # `--lan` exposes the dev server on the network (binds 0.0.0.0) so you can + # reach it from another device at http://:8008. Default stays + # localhost-only. Set before build_backend_env() so both backend modes see it. + if '--lan' in sys.argv: + os.environ['SOULSYNC_WEB_BIND_HOST'] = '0.0.0.0' + print('LAN mode ON — dev server reachable from other devices on your network ' + '(http://:8008). Allow port 8008 through the firewall if needed.') + if not (ROOT_DIR / 'webui' / 'node_modules').is_dir(): print('webui/node_modules is missing.') print('Run: cd webui && npm ci') From b5b9d6e5f40286471ac041df2054acb7b05b5102 Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Thu, 11 Jun 2026 17:38:24 -0700 Subject: [PATCH 04/64] #852 tests: cover the login-mode WS gate (the reported bypass was the login modal) The integration test only exercised the launch-PIN path; the actual #852 report is the "Sign in to SoulSync" username/password modal. Add login-mode cases: socketio connect is rejected when require_login is on + unauthenticated, allowed once login_authenticated. Confirms the WS gate covers both overlays. --- tests/test_ws_connect_gate.py | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/tests/test_ws_connect_gate.py b/tests/test_ws_connect_gate.py index d861410d..0876ac51 100644 --- a/tests/test_ws_connect_gate.py +++ b/tests/test_ws_connect_gate.py @@ -82,3 +82,26 @@ def test_socket_allowed_when_pin_verified(monkeypatch): s['launch_pin_verified'] = True sio = web_server.socketio.test_client(web_server.app, flask_test_client=flask_client) assert sio.is_connected() is True + + +# ── login mode (the "Sign in to SoulSync" path — #852 report was this one) ── +def _login_on(monkeypatch): + real = web_server.config_manager.get + monkeypatch.setattr(web_server.config_manager, 'get', + lambda k, d=None: True if k == 'security.require_login' else real(k, d)) + + +def test_socket_rejected_when_login_required_and_unauthenticated(monkeypatch): + _login_on(monkeypatch) + flask_client = web_server.app.test_client() # no login_authenticated + sio = web_server.socketio.test_client(web_server.app, flask_test_client=flask_client) + assert sio.is_connected() is False # login overlay can't be bypassed via WS + + +def test_socket_allowed_when_login_authenticated(monkeypatch): + _login_on(monkeypatch) + flask_client = web_server.app.test_client() + with flask_client.session_transaction() as s: + s['login_authenticated'] = True + sio = web_server.socketio.test_client(web_server.app, flask_test_client=flask_client) + assert sio.is_connected() is True From 68acf89b8393933940a70a62586e2d069bf23469 Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Thu, 11 Jun 2026 18:08:02 -0700 Subject: [PATCH 05/64] =?UTF-8?q?#852:=20hide=20the=20whole=20app=20behind?= =?UTF-8?q?=20the=20lock=20screen=20=E2=80=94=20bypass=20reveals=20a=20bla?= =?UTF-8?q?nk=20page?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Beckid's ask: bypassing the login/PIN overlay shouldn't show the app pages at all, not even the (data-less) chrome. The overlay was cosmetic-on-top; the static shell sat behind it, so "Hide Distracting Items" exposed the empty UI. Now the lock screens add body.app-locked, and a CSS rule hides every body child except the two lock overlays themselves (display:none !important). Safari's hide-element trick can only ADD hiding — it can't undo this rule — so removing the overlay leaves a blank page. initApp() drops the class once authenticated (first line, before component layout init). Defense-in-depth on top of the server-side HTTP + WebSocket gating, which already blocks any actual data. Targeted + safe: the app shows by default (no blank-screen risk); only an active lock hides it. Profile picker (not a security lock) is unaffected. --- webui/static/init.js | 10 ++++++++++ webui/static/style.css | 10 ++++++++++ 2 files changed, 20 insertions(+) diff --git a/webui/static/init.js b/webui/static/init.js index e6ea4816..4da3795f 100644 --- a/webui/static/init.js +++ b/webui/static/init.js @@ -404,6 +404,10 @@ async function initProfileSystem() { function showLoginScreen() { const overlay = document.getElementById('login-overlay'); if (!overlay) return; + // Hide the entire app while locked, so removing the overlay (Safari "Hide + // Distracting Items", devtools) reveals nothing — not even the empty chrome. + // initApp() reveals it again on a successful sign-in (#852). + document.body.classList.add('app-locked'); overlay.style.display = 'flex'; const u = document.getElementById('login-username'); if (u) setTimeout(() => u.focus(), 50); @@ -507,6 +511,8 @@ async function submitRecoveryReset() { function showLaunchPinScreen() { const overlay = document.getElementById('launch-pin-overlay'); if (!overlay) return; + // Hide the whole app while locked — bypassing the overlay reveals nothing (#852). + document.body.classList.add('app-locked'); overlay.style.display = 'flex'; const input = document.getElementById('launch-pin-input'); @@ -2322,6 +2328,10 @@ async function _continueAppInit() { } function initApp() { + // Unlocked / authenticated — reveal the app (the lock screens hide it via + // body.app-locked so a bypassed overlay shows nothing). Do this FIRST so + // component init below measures real layout, not a display:none container. + document.body.classList.remove('app-locked'); // Initialize components initializeNavigation(); initializeMobileNavigation(); diff --git a/webui/static/style.css b/webui/static/style.css index 69f45953..954ff067 100644 --- a/webui/static/style.css +++ b/webui/static/style.css @@ -68177,3 +68177,13 @@ body.em-scroll-lock { overflow: hidden; } .arec-row { grid-template-columns: 1fr; gap: 2px; } .arec-rowcopy { opacity: 1; justify-self: end; margin-top: -22px; } } + + +/* #852: while the launch-PIN / login lock screen is up, hide EVERYTHING in the + body except the lock overlays themselves — so removing the overlay (Safari + "Hide Distracting Items", devtools) reveals a blank page, not the empty chrome + or any of the on-demand modals/sidebars. The hide-element trick can only ADD + hiding, never undo this rule. initApp() drops body.app-locked once authenticated. */ +body.app-locked > *:not(#launch-pin-overlay):not(#login-overlay):not(script):not(style):not(noscript) { + display: none !important; +} From 29c8f11403b399c8781c5cf88fe34d58f2e3266b Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Thu, 11 Jun 2026 18:41:12 -0700 Subject: [PATCH 06/64] #437: add ReplayGain Filler library-maintenance job MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Post-processing applies ReplayGain only to slskd/WebUI downloads — content added via Lidarr, the REST API, or by hand never got it, and there was no way to (re)apply RG to existing tracks or fix ones where analysis failed (raised in #437 + comments). New ReplayGain Filler repair job (sibling of Lyrics/Cover Art fillers): scans for tracks with no ReplayGain track-gain tag and creates a finding per track; the scan only READS tags (cheap) and no-ops when ffmpeg is absent. Applying a finding runs the same ffmpeg ebur128 analysis the import pipeline uses (gain = ref - LUFS) and writes the RG tags in place — no moves, no re-matching. Opt-in (default off), schedulable like the other maintenance jobs. Wired: job registry, repair_worker apply handler (_fix_missing_replaygain) + fixable-types, and the findings UI (label / fix-button / detail rows). Tests: pure needs_replaygain decision (missing/blank/present/+0.00-is-tagged) + the apply handler's analyze→compute→write seam with the pipeline gain formula, ffmpeg-absent + missing-file guards, and registration. 93 repair tests green. --- core/repair_jobs/__init__.py | 1 + core/repair_jobs/replaygain_filler.py | 197 ++++++++++++++++++++++++++ core/repair_worker.py | 30 +++- tests/test_replaygain_filler_job.py | 82 +++++++++++ webui/static/enrichment.js | 7 + 5 files changed, 316 insertions(+), 1 deletion(-) create mode 100644 core/repair_jobs/replaygain_filler.py create mode 100644 tests/test_replaygain_filler_job.py diff --git a/core/repair_jobs/__init__.py b/core/repair_jobs/__init__.py index 4a60b2e0..1e2c3e11 100644 --- a/core/repair_jobs/__init__.py +++ b/core/repair_jobs/__init__.py @@ -34,6 +34,7 @@ _JOB_MODULES = [ 'core.repair_jobs.acoustid_scanner', 'core.repair_jobs.missing_cover_art', 'core.repair_jobs.missing_lyrics', + 'core.repair_jobs.replaygain_filler', 'core.repair_jobs.expired_download_cleaner', 'core.repair_jobs.metadata_gap_filler', 'core.repair_jobs.album_completeness', diff --git a/core/repair_jobs/replaygain_filler.py b/core/repair_jobs/replaygain_filler.py new file mode 100644 index 00000000..bcda36bc --- /dev/null +++ b/core/repair_jobs/replaygain_filler.py @@ -0,0 +1,197 @@ +"""ReplayGain Filler maintenance job (#437) — the loudness sibling of the Lyrics +and Cover Art fillers. + +Post-processing applies ReplayGain to slskd/WebUI downloads, but content that +enters the library another way — Lidarr, the REST API, manual adds — never gets +it, and there was no way to (re)apply RG to existing tracks or fix the ones where +analysis failed (a recurring ask on #437). + +This scans the library for tracks with no ReplayGain track-gain tag and creates a +finding for each. Applying a finding runs the same ffmpeg ebur128 analysis the +import pipeline uses and writes the RG tags in place — no moves, no re-matching. + +Scan only READS tags (cheap); the expensive ffmpeg analysis happens on apply. +Requires ffmpeg (RG analysis can't run without it), so the scan no-ops when ffmpeg +isn't on PATH rather than surfacing findings that could never be applied. +""" + +from __future__ import annotations + +import os +from typing import Any, Dict, Optional + +from core.library.path_resolver import resolve_library_file_path +from core.repair_jobs import register_job +from core.repair_jobs.base import JobContext, JobResult, RepairJob +from utils.logging_config import get_logger + +logger = get_logger("repair_jobs.replaygain_filler") + + +def needs_replaygain(rg_tags: Optional[Dict[str, Any]]) -> bool: + """Pure decision: does this track need ReplayGain written? + + True when the track-gain tag is absent or blank. ``rg_tags`` is the dict from + ``core.replaygain.read_replaygain_tags`` (keys: track_gain, track_peak, …). + A present track_gain — even "+0.00 dB" — counts as already-tagged. + """ + if not rg_tags: + return True + val = rg_tags.get('track_gain') + return val is None or str(val).strip() == '' + + +def _resolve(file_path: str) -> Optional[str]: + """Resolve a stored library path to one this process can read (Docker/host + prefix mapping), falling back to the raw path if it's already a real file.""" + resolved = resolve_library_file_path(file_path) if file_path else None + if not resolved and file_path and os.path.isfile(file_path): + resolved = file_path + return resolved + + +@register_job +class ReplayGainFillerJob(RepairJob): + job_id = 'replaygain_filler' + display_name = 'ReplayGain Filler' + description = 'Finds tracks with no ReplayGain tag and analyzes + writes loudness tags' + help_text = ( + 'Scans your library for tracks that have no ReplayGain track-gain tag — ' + 'common for albums added by Lidarr, the REST API, or by hand, which skip ' + "the download post-processing where ReplayGain normally runs.\n\n" + 'A finding is created for each. Applying one runs the same ffmpeg loudness ' + 'analysis (EBU R128) the import pipeline uses and writes the ReplayGain ' + 'tags in place — no files are moved or renamed. This also lets you re-fill ' + 'tracks where the original analysis failed.\n\n' + 'Requires ffmpeg to be installed (the analysis cannot run without it).' + ) + icon = 'repair-icon-replaygain' + default_enabled = False + default_interval_hours = 48 + default_settings = {} + auto_fix = False + + def scan(self, context: JobContext) -> JobResult: + result = JobResult() + + try: + from core.replaygain import is_ffmpeg_available, read_replaygain_tags + except Exception as e: + logger.warning("[ReplayGain Filler] replaygain module unavailable: %s", e) + return result + if not is_ffmpeg_available(): + logger.info("[ReplayGain Filler] ffmpeg not available — skipping scan " + "(analysis cannot run without it)") + return result + + rows = [] + conn = None + try: + conn = context.db._get_connection() + cursor = conn.cursor() + cursor.execute(""" + SELECT t.id, t.title, ar.name, t.file_path + FROM tracks t + LEFT JOIN artists ar ON ar.id = t.artist_id + WHERE t.file_path IS NOT NULL AND t.file_path != '' + """) + rows = cursor.fetchall() + except Exception as e: + logger.error("[ReplayGain Filler] Error reading tracks: %s", e, exc_info=True) + result.errors += 1 + return result + finally: + if conn: + conn.close() + + total = len(rows) + if context.update_progress: + context.update_progress(0, total) + if context.report_progress: + context.report_progress(phase=f'Checking ReplayGain on {total} tracks...', total=total) + + for i, row in enumerate(rows): + if context.check_stop(): + return result + if i % 10 == 0 and context.wait_if_paused(): + return result + + track_id, title, artist_name, file_path = row[:4] + result.scanned += 1 + + resolved = _resolve(file_path) + if not resolved: + # Can't read the file from here → can't analyze it on apply either. + result.skipped += 1 + continue + + try: + rg = read_replaygain_tags(resolved) + except Exception as e: + logger.debug("[ReplayGain Filler] tag read failed for '%s': %s", title, e) + result.skipped += 1 + continue + + if not needs_replaygain(rg): + result.skipped += 1 + if context.update_progress and (i + 1) % 25 == 0: + context.update_progress(i + 1, total) + continue + + if context.report_progress: + context.report_progress( + scanned=i + 1, total=total, + log_line=f'No ReplayGain: {title} — {artist_name or "Unknown"}', + log_type='info') + + if context.create_finding: + try: + inserted = context.create_finding( + job_id=self.job_id, + finding_type='missing_replaygain', + severity='info', + entity_type='track', + entity_id=str(track_id), + file_path=file_path, + title=f'No ReplayGain: {title or "Unknown"}', + description=(f'"{title}" by {artist_name or "Unknown"} has no ' + 'ReplayGain tag — loudness can be analyzed + written.'), + details={ + 'track_id': track_id, + 'track_title': title, + 'artist': artist_name, + 'file_path': file_path, + }) + if inserted: + result.findings_created += 1 + else: + result.findings_skipped_dedup += 1 + except Exception as e: + logger.debug("[ReplayGain Filler] create finding failed for track %s: %s", track_id, e) + result.errors += 1 + + if context.update_progress and (i + 1) % 10 == 0: + context.update_progress(i + 1, total) + + if context.update_progress: + context.update_progress(total, total) + logger.info("[ReplayGain Filler] %d tracks checked, %d missing ReplayGain, %d skipped", + result.scanned, result.findings_created, result.skipped) + return result + + def estimate_scope(self, context: JobContext) -> int: + conn = None + try: + conn = context.db._get_connection() + cursor = conn.cursor() + cursor.execute(""" + SELECT COUNT(*) FROM tracks + WHERE file_path IS NOT NULL AND file_path != '' + """) + row = cursor.fetchone() + return row[0] if row else 0 + except Exception: + return 0 + finally: + if conn: + conn.close() diff --git a/core/repair_worker.py b/core/repair_worker.py index 27dffe3b..9babe245 100644 --- a/core/repair_worker.py +++ b/core/repair_worker.py @@ -964,6 +964,7 @@ class RepairWorker: 'track_number_mismatch': self._fix_track_number, 'missing_cover_art': self._fix_missing_cover_art, 'missing_lyrics': self._fix_missing_lyrics, + 'missing_replaygain': self._fix_missing_replaygain, 'expired_download': self._fix_expired_download, 'metadata_gap': self._fix_metadata_gap, 'duplicate_tracks': self._fix_duplicates, @@ -1479,6 +1480,33 @@ class RepairWorker: return {'success': False, 'error': 'Could not fetch lyrics (no longer available?)'} return {'success': True, 'action': 'applied_lyrics', 'message': 'Wrote lyrics (.lrc) + embedded'} + def _fix_missing_replaygain(self, entity_type, entity_id, file_path, details): + """Apply a missing-ReplayGain finding: run the same ffmpeg ebur128 loudness + analysis the import pipeline uses and write the RG tags in place (#437).""" + raw_path = details.get('file_path') or file_path + if not raw_path: + return {'success': False, 'error': 'No file path in finding'} + download_folder = self._config_manager.get('soulseek.download_path', '') if self._config_manager else None + resolved = _resolve_file_path(raw_path, self.transfer_folder, download_folder, + config_manager=self._config_manager) or raw_path + if not os.path.isfile(resolved): + return {'success': False, 'error': f'File not found on disk: {os.path.basename(raw_path)}'} + try: + from core.replaygain import (analyze_track, write_replaygain_tags, + is_ffmpeg_available, RG_REFERENCE_LUFS) + if not is_ffmpeg_available(): + return {'success': False, 'error': 'ffmpeg not available — cannot analyze ReplayGain'} + lufs, peak_dbfs = analyze_track(resolved) + gain_db = RG_REFERENCE_LUFS - lufs # same formula as the import pipeline + ok = write_replaygain_tags(resolved, gain_db, peak_dbfs) + except Exception as e: + logger.error("ReplayGain fix failed for %s: %s", os.path.basename(raw_path), e) + return {'success': False, 'error': str(e)} + if not ok: + return {'success': False, 'error': 'Could not write ReplayGain tags'} + return {'success': True, 'action': 'applied_replaygain', + 'message': f'Wrote ReplayGain ({gain_db:+.2f} dB)'} + def _fix_expired_download(self, entity_type, entity_id, file_path, details): """Apply an expired-download finding: delete the file + library row + history entry, via the same helper the cleaner's auto mode uses.""" @@ -3284,7 +3312,7 @@ class RepairWorker: 'album_mbid_mismatch', 'album_tag_inconsistency', 'incomplete_album', 'path_mismatch', - 'missing_lossy_copy', + 'missing_lossy_copy', 'missing_replaygain', 'missing_discography_track', 'acoustid_mismatch') placeholders = ','.join(['?'] * len(fixable_types)) where_parts = [f"finding_type IN ({placeholders})", "status = 'pending'"] diff --git a/tests/test_replaygain_filler_job.py b/tests/test_replaygain_filler_job.py new file mode 100644 index 00000000..bce087be --- /dev/null +++ b/tests/test_replaygain_filler_job.py @@ -0,0 +1,82 @@ +"""ReplayGain Filler job (#437) — fills ReplayGain on library content that skipped +download post-processing (Lidarr / REST API / manual adds). Pure flag decision + +the apply handler's analyze→compute→write seam (ffmpeg mocked).""" + +from __future__ import annotations + +from types import SimpleNamespace +from unittest.mock import patch + +from core.repair_jobs.replaygain_filler import needs_replaygain +from core.repair_worker import RepairWorker + + +# ── pure decision: does a track need ReplayGain? ──────────────────────────── +def test_needs_rg_when_no_tags(): + assert needs_replaygain(None) is True + + +def test_needs_rg_when_track_gain_missing(): + assert needs_replaygain({'track_gain': None, 'track_peak': None}) is True + + +def test_needs_rg_when_track_gain_blank(): + assert needs_replaygain({'track_gain': ' '}) is True + + +def test_no_rg_needed_when_gain_present(): + assert needs_replaygain({'track_gain': '-6.50 dB'}) is False + + +def test_zero_gain_counts_as_tagged(): + # A legitimate "+0.00 dB" is already analyzed — must NOT be re-flagged forever. + assert needs_replaygain({'track_gain': '+0.00 dB'}) is False + + +# ── apply handler: analyze → compute gain → write (ffmpeg mocked) ──────────── +def _worker(): + w = RepairWorker(database=SimpleNamespace()) + w._config_manager = None + return w + + +def test_apply_writes_rg_with_pipeline_gain_formula(tmp_path): + f = tmp_path / 'song.flac' + f.write_bytes(b'\x00' * 64) + written = {} + + def fake_write(path, gain, peak, *a, **k): + written.update(path=path, gain=gain, peak=peak) + return True + + with patch('core.replaygain.is_ffmpeg_available', return_value=True), \ + patch('core.replaygain.analyze_track', return_value=(-12.0, -1.5)), \ + patch('core.replaygain.write_replaygain_tags', side_effect=fake_write), \ + patch('core.replaygain.RG_REFERENCE_LUFS', -18.0): + res = _worker()._fix_missing_replaygain('track', '1', str(f), {'file_path': str(f)}) + + assert res['success'] is True and res['action'] == 'applied_replaygain' + # gain = reference - lufs = -18.0 - (-12.0) = -6.0 (same as the import pipeline) + assert written['gain'] == -6.0 + assert written['peak'] == -1.5 + assert written['path'] == str(f) + + +def test_apply_errors_without_ffmpeg(tmp_path): + f = tmp_path / 's.flac' + f.write_bytes(b'\x00' * 64) + with patch('core.replaygain.is_ffmpeg_available', return_value=False): + res = _worker()._fix_missing_replaygain('track', '1', str(f), {'file_path': str(f)}) + assert res['success'] is False and 'ffmpeg' in res['error'].lower() + + +def test_apply_errors_when_file_missing(): + res = _worker()._fix_missing_replaygain( + 'track', '1', '/no/such/file.flac', {'file_path': '/no/such/file.flac'}) + assert res['success'] is False + + +def test_job_is_registered_and_opt_in(): + from core.repair_jobs import get_all_jobs + j = get_all_jobs().get('replaygain_filler') + assert j is not None and j.default_enabled is False diff --git a/webui/static/enrichment.js b/webui/static/enrichment.js index 3ec070e0..91affc21 100644 --- a/webui/static/enrichment.js +++ b/webui/static/enrichment.js @@ -2735,6 +2735,7 @@ async function loadRepairFindings() { path_mismatch: 'Path Mismatch', metadata_gap: 'Missing Metadata', missing_cover_art: 'Missing Art', track_number_mismatch: 'Track Number', missing_lyrics: 'Missing Lyrics', expired_download: 'Expired', + missing_replaygain: 'No ReplayGain', missing_lossy_copy: 'No Lossy Copy', library_retag: 'Re-tag' }; @@ -2745,6 +2746,7 @@ async function loadRepairFindings() { track_number_mismatch: 'Fix', missing_cover_art: 'Apply Art', missing_lyrics: 'Apply Lyrics', + missing_replaygain: 'Apply RG', expired_download: 'Delete', metadata_gap: 'Apply', duplicate_tracks: 'Keep Best', @@ -3138,6 +3140,11 @@ function _renderFindingDetail(f) { if (d.album_title) rows.push(['Album', d.album_title]); return _gridRows(rows); + case 'missing_replaygain': + if (d.track_title) rows.push(['Track', d.track_title]); + if (d.artist) rows.push(['Artist', d.artist]); + return _gridRows(rows); + case 'expired_download': if (d.title) rows.push(['Track', d.title]); if (d.artist) rows.push(['Artist', d.artist]); From e046a2add4b3e7d499c4c3990c503fa13d8b1c5f Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Thu, 11 Jun 2026 19:24:31 -0700 Subject: [PATCH 07/64] Login mode: let the admin set a member's login password (Manage Profiles) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes the gap where "Require login" was effectively admin-only: a member with no password can't sign in and can't bootstrap one themselves (can't log in to reach the setting). The set-password endpoint already allowed admin→anyone — this adds the missing UI. Each non-admin row in Manage Profiles gets a lock-icon button that opens an inline form to set / change / remove that member's LOGIN password (separate from the quick-switch PIN), with a confirm field + a hint explaining when it's used. Admin rows don't get it (admin manages their own in Settings → Security, which keeps its anti-lockout). textContent-only rendering, so a profile name can't inject markup. Test: admin sets a member's password → the member can then authenticate (verify_profile_password) and a wrong password fails; admin can clear it back to no-login. 64 script-split integrity tests green. --- tests/test_member_login_password.py | 54 ++++++++++++++ webui/static/init.js | 111 ++++++++++++++++++++++++++++ 2 files changed, 165 insertions(+) create mode 100644 tests/test_member_login_password.py diff --git a/tests/test_member_login_password.py b/tests/test_member_login_password.py new file mode 100644 index 00000000..f6248cdf --- /dev/null +++ b/tests/test_member_login_password.py @@ -0,0 +1,54 @@ +"""Admin sets a member's LOGIN password (the gap behind 'non-admins can't log in +when Require Login is on'). The endpoint already allowed admin→anyone; this locks +that the round-trip actually lets the member authenticate.""" + +from __future__ import annotations + +import os +import tempfile + +import pytest + +_TMP = tempfile.mkdtemp(prefix='soulsync-testdb-memberpw-') +os.environ['DATABASE_PATH'] = os.path.join(_TMP, 'm.db') +os.environ['SOULSYNC_TEST_DB_READY'] = '1' + +web_server = pytest.importorskip('web_server') + + +@pytest.fixture +def client(): + return web_server.app.test_client() + + +def _make_member(db, pid=77): + conn = db._get_connection() + try: + conn.execute("INSERT OR REPLACE INTO profiles (id, name, is_admin) VALUES (?,?,0)", (pid, 'Member')) + conn.commit() + finally: + conn.close() + + +def test_admin_sets_member_password_then_member_can_authenticate(client): + db = web_server.get_database() + _make_member(db) + assert db.verify_profile_password(77, 'secret123') is False # no password → can't log in + + r = client.post('/api/profiles/77/set-password', json={'password': 'secret123'}) + assert r.status_code == 200 + body = r.get_json() + assert body['success'] is True and body['has_password'] is True + + assert db.verify_profile_password(77, 'secret123') is True # member can now authenticate + assert db.verify_profile_password(77, 'wrong') is False + + +def test_admin_can_clear_member_password(client): + db = web_server.get_database() + _make_member(db, pid=78) + client.post('/api/profiles/78/set-password', json={'password': 'pw12345'}) + assert db.verify_profile_password(78, 'pw12345') is True + r = client.post('/api/profiles/78/set-password', json={'password': ''}) + assert r.status_code == 200 + assert db.verify_profile_password(78, 'pw12345') is False # cleared → no login again diff --git a/webui/static/init.js b/webui/static/init.js index 4da3795f..0c552aea 100644 --- a/webui/static/init.js +++ b/webui/static/init.js @@ -1924,6 +1924,18 @@ async function loadProfileManageList() { actions.appendChild(editBtn); if (!p.is_admin) { + // Set/change the LOGIN password (separate from the quick-switch PIN; + // used when "Require login" is on). A member with no password can't + // sign in and can't self-bootstrap one, so the admin sets it here. + const pwBtn = document.createElement('button'); + pwBtn.className = 'profile-password-btn' + (p.has_password ? ' has-password' : ''); + pwBtn.dataset.id = p.id; + pwBtn.dataset.name = p.name; + pwBtn.dataset.hasPassword = p.has_password ? '1' : '0'; + pwBtn.title = p.has_password ? 'Change login password' : 'Set login password (for Require Login mode)'; + pwBtn.innerHTML = ''; + actions.appendChild(pwBtn); + const delBtn = document.createElement('button'); delBtn.className = 'profile-delete-btn'; delBtn.dataset.id = p.id; @@ -1948,6 +1960,11 @@ async function loadProfileManageList() { }; }); + // Bind set-login-password buttons + list.querySelectorAll('.profile-password-btn').forEach(btn => { + btn.onclick = () => showProfilePasswordForm(btn.dataset.id, btn.dataset.name, btn.dataset.hasPassword === '1'); + }); + // Bind delete buttons list.querySelectorAll('.profile-delete-btn').forEach(btn => { btn.onclick = async () => { @@ -1968,6 +1985,100 @@ async function loadProfileManageList() { checkAdminPinRequired(); } +function showProfilePasswordForm(profileId, name, hasPassword) { + const list = document.getElementById('profile-manage-list'); + // One inline form at a time — drop any edit/password form already open. + ['profile-password-form', 'profile-edit-form'].forEach(id => { + const el = document.getElementById(id); if (el) el.remove(); + }); + + const form = document.createElement('div'); + form.id = 'profile-password-form'; + form.className = 'profile-edit-form'; + + const title = document.createElement('div'); + title.style.cssText = 'font-weight:600;margin-bottom:4px;'; + title.textContent = 'Login password — ' + name; // textContent = XSS-safe + form.appendChild(title); + + const hint = document.createElement('div'); + hint.style.cssText = 'font-size:0.8em;color:rgba(255,255,255,0.5);margin-bottom:8px;line-height:1.4;'; + hint.textContent = 'Used when "Require login" is on (separate from the quick-switch PIN). ' + + (hasPassword ? 'This profile has a password set.' + : "This profile has no password yet — it can't sign in until you set one."); + form.appendChild(hint); + + const pw = document.createElement('input'); + pw.type = 'password'; pw.className = 'profile-input'; + pw.placeholder = 'New password'; pw.autocomplete = 'new-password'; + const confirm = document.createElement('input'); + confirm.type = 'password'; confirm.className = 'profile-input'; + confirm.placeholder = 'Confirm password'; confirm.autocomplete = 'new-password'; + form.appendChild(pw); form.appendChild(confirm); + + const msg = document.createElement('div'); + msg.style.cssText = 'font-size:0.8em;margin:6px 0;display:none;'; + form.appendChild(msg); + const showMsg = (t, ok) => { + msg.textContent = t; msg.style.color = ok ? '#10b981' : '#ef4444'; msg.style.display = 'block'; + }; + + const post = async (password, okMsg, okType) => { + const res = await fetch('/api/profiles/' + encodeURIComponent(profileId) + '/set-password', { + method: 'POST', headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ password }), + }); + const data = await res.json(); + if (data.success) { + form.remove(); + loadProfileManageList(); + if (typeof showToast === 'function') showToast(okMsg, okType); + return true; + } + showMsg(data.error || 'Failed to update password', false); + return false; + }; + + const btnRow = document.createElement('div'); + btnRow.style.cssText = 'display:flex;gap:8px;margin-top:8px;flex-wrap:wrap;'; + + const saveBtn = document.createElement('button'); + saveBtn.className = 'btn btn--primary'; + saveBtn.textContent = 'Save password'; + saveBtn.onclick = async () => { + const p1 = pw.value, p2 = confirm.value; + if (!p1 || !p1.trim()) { showMsg('Enter a password', false); return; } + if (p1.length < 4) { showMsg('Use at least 4 characters', false); return; } + if (p1 !== p2) { showMsg("Passwords don't match", false); return; } + saveBtn.disabled = true; + try { if (!await post(p1, 'Login password set for ' + name, 'success')) saveBtn.disabled = false; } + catch (e) { showMsg('Connection error', false); saveBtn.disabled = false; } + }; + btnRow.appendChild(saveBtn); + + if (hasPassword) { + const clearBtn = document.createElement('button'); + clearBtn.className = 'btn'; + clearBtn.textContent = 'Remove password'; + clearBtn.onclick = async () => { + clearBtn.disabled = true; + try { if (!await post('', 'Login password removed', 'info')) clearBtn.disabled = false; } + catch (e) { showMsg('Connection error', false); clearBtn.disabled = false; } + }; + btnRow.appendChild(clearBtn); + } + + const cancelBtn = document.createElement('button'); + cancelBtn.className = 'btn'; + cancelBtn.textContent = 'Cancel'; + cancelBtn.onclick = () => form.remove(); + btnRow.appendChild(cancelBtn); + + form.appendChild(btnRow); + list.appendChild(form); + pw.focus(); +} + function showProfileEditForm(profileId, currentName, currentColor, currentAvatarUrl, profileSettings = {}) { const list = document.getElementById('profile-manage-list'); // Remove any existing edit form From 5b52d579c523d8db19f225d0366d358e68ca31a8 Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Thu, 11 Jun 2026 19:48:50 -0700 Subject: [PATCH 08/64] Login mode: enforce "every profile has a password" at every write-point (no gaps) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Invariant: while security.require_login is on, every profile must have a login password or it's locked out. Previously only the admin's own anti-lockout existed, so members could be stranded (created without a password, or login flipped on while passwordless members existed). Closed all the write-points: core/security/login_provisioning.py (pure policy, single source of truth): - members_without_password(profiles) — non-admin profiles that can't sign in - create_needs_password(require_login) / removing_password_strands(require_login) Wired into web_server: - create_profile: while login is on, a new member must be given a password (400 otherwise) and it's set on creation. - enable-login (settings save): refuses to turn login on while any member lacks a password — lists them — same shape as the existing admin anti-lockout. - set-password: refuses to CLEAR a password while login is on (would strand them). UI: Create Profile form gains a login-password field (alongside the optional PIN); the Manage Profiles per-member password button (prior commit) covers existing members + changes. Tests: pure policy seam + endpoint enforcement (create blocked w/o password when on, allowed w/ password, no friction when off, clear blocked when on). 442 profile/settings/auth tests green; ruff clean. --- core/security/login_provisioning.py | 45 +++++++++++++ tests/test_login_provisioning.py | 97 +++++++++++++++++++++++++++++ web_server.py | 30 +++++++++ webui/index.html | 1 + webui/static/init.js | 3 + 5 files changed, 176 insertions(+) create mode 100644 core/security/login_provisioning.py create mode 100644 tests/test_login_provisioning.py diff --git a/core/security/login_provisioning.py b/core/security/login_provisioning.py new file mode 100644 index 00000000..3102f77d --- /dev/null +++ b/core/security/login_provisioning.py @@ -0,0 +1,45 @@ +"""Login-mode password provisioning policy. + +Invariant: while ``security.require_login`` is on, every profile must have a login +password — otherwise it's fail-closed locked out (usable only after the admin +provisions one). That's not a security hole (no-password = can't get in), but it's +a usability gap, and the point here is to make it impossible to OPEN one from any +write-point: creating a profile, clearing a password, or flipping login mode on. + +These are pure decisions so they're the single source of truth + unit-testable; +web_server wires them into the create / set-password / enable-login endpoints, and +the UI mirrors them. +""" + +from __future__ import annotations + +from typing import Any, Dict, List, Optional + + +def members_without_password(profiles: Optional[List[Dict[str, Any]]]) -> List[Dict[str, Any]]: + """Non-admin profiles with no login password — they can't sign in once login + mode is on. The admin is covered separately by its own anti-lockout, so it's + excluded here. Returns ``[{'id', 'name'}, …]`` (empty = no gap).""" + out: List[Dict[str, Any]] = [] + for p in (profiles or []): + if not p.get('is_admin') and not p.get('has_password'): + out.append({'id': p.get('id'), 'name': p.get('name')}) + return out + + +def create_needs_password(require_login: bool, is_admin: bool = False) -> bool: + """A non-admin profile created while login mode is on must carry a password, + or it's born unable to sign in.""" + return bool(require_login) and not is_admin + + +def removing_password_strands(require_login: bool) -> bool: + """Clearing a profile's password while login mode is on would lock it out.""" + return bool(require_login) + + +__all__ = [ + "members_without_password", + "create_needs_password", + "removing_password_strands", +] diff --git a/tests/test_login_provisioning.py b/tests/test_login_provisioning.py new file mode 100644 index 00000000..fd9d2e70 --- /dev/null +++ b/tests/test_login_provisioning.py @@ -0,0 +1,97 @@ +"""No-gaps invariant: while login mode is on, every profile must have a login +password. Pure policy seam + endpoint enforcement at every write-point (create, +clear, enable-login).""" + +from __future__ import annotations + +import os +import tempfile + +import pytest + +from core.security.login_provisioning import ( + members_without_password, create_needs_password, removing_password_strands) + + +# ── pure policy ───────────────────────────────────────────────────────────── +def test_members_without_password_flags_only_passwordless_nonadmins(): + profiles = [ + {'id': 1, 'name': 'Admin', 'is_admin': True, 'has_password': False}, # admin: own anti-lockout + {'id': 2, 'name': 'HasPw', 'is_admin': False, 'has_password': True}, # fine + {'id': 3, 'name': 'NoPw', 'is_admin': False, 'has_password': False}, # stranded + ] + out = members_without_password(profiles) + assert out == [{'id': 3, 'name': 'NoPw'}] + + +def test_members_without_password_empty_when_all_set(): + assert members_without_password([{'id': 2, 'is_admin': False, 'has_password': True}]) == [] + assert members_without_password(None) == [] + + +def test_create_needs_password_only_when_login_on_and_nonadmin(): + assert create_needs_password(True) is True + assert create_needs_password(False) is False + assert create_needs_password(True, is_admin=True) is False + + +def test_removing_password_strands_only_when_login_on(): + assert removing_password_strands(True) is True + assert removing_password_strands(False) is False + + +# ── endpoint enforcement ──────────────────────────────────────────────────── +_TMP = tempfile.mkdtemp(prefix='soulsync-testdb-prov-') +os.environ['DATABASE_PATH'] = os.path.join(_TMP, 'p.db') +os.environ['SOULSYNC_TEST_DB_READY'] = '1' +web_server = pytest.importorskip('web_server') + + +@pytest.fixture +def client(): + return web_server.app.test_client() + + +def _login_on(monkeypatch, on=True): + real = web_server.config_manager.get + monkeypatch.setattr(web_server.config_manager, 'get', + lambda k, d=None: on if k == 'security.require_login' else real(k, d)) + + +def _auth(c): + # Turning login mode on activates the HTTP gate — authenticate the session as + # admin so the request reaches the endpoint (we're testing the endpoint logic). + with c.session_transaction() as sess: + sess['login_authenticated'] = True + sess['profile_id'] = 1 + + +def test_create_without_password_blocked_when_login_on(monkeypatch, client): + _login_on(monkeypatch, True); _auth(client) + r = client.post('/api/profiles', json={'name': 'NoPwMember'}) + assert r.status_code == 400 + assert 'login' in r.get_json()['error'].lower() + + +def test_create_with_password_succeeds_when_login_on(monkeypatch, client): + _login_on(monkeypatch, True); _auth(client) + r = client.post('/api/profiles', json={'name': 'PwMember', 'password': 'secret9'}) + assert r.status_code == 200 and r.get_json()['success'] is True + pid = r.get_json()['profile_id'] + assert web_server.get_database().verify_profile_password(pid, 'secret9') is True + + +def test_create_without_password_fine_when_login_off(monkeypatch, client): + _login_on(monkeypatch, False) + r = client.post('/api/profiles', json={'name': 'PinOnlyMember'}) + assert r.status_code == 200 and r.get_json()['success'] is True # no friction when off + + +def test_clear_password_blocked_when_login_on(monkeypatch, client): + db = web_server.get_database() + r = client.post('/api/profiles', json={'name': 'Clearable', 'password': 'x12345'}) + pid = r.get_json()['profile_id'] + _login_on(monkeypatch, True); _auth(client) + r2 = client.post(f'/api/profiles/{pid}/set-password', json={'password': ''}) + assert r2.status_code == 400 and 'login mode' in r2.get_json()['error'].lower() + assert db.verify_profile_password(pid, 'x12345') is True # still set diff --git a/web_server.py b/web_server.py index 13206f45..2dbc1907 100644 --- a/web_server.py +++ b/web_server.py @@ -3120,6 +3120,17 @@ def handle_settings(): if not get_database().profile_has_password(1): return jsonify({"success": False, "error": "Set an admin password before enabling login mode."}), 400 + # No-gaps: every member must have a password too, or they'd be + # locked out the moment login turns on. + from core.security.login_provisioning import members_without_password + _stranded = members_without_password(get_database().get_all_profiles()) + if _stranded: + _names = ', '.join(str(m.get('name') or '?') for m in _stranded) + return jsonify({"success": False, + "error": f"These members have no login password and " + f"couldn't sign in: {_names}. Set their passwords " + f"in Manage Profiles first.", + "members_without_password": _stranded}), 400 if 'active_media_server' in new_settings: config_manager.set_active_media_server(new_settings['active_media_server']) @@ -25562,6 +25573,15 @@ def create_profile(): from werkzeug.security import generate_password_hash pin_hash = generate_password_hash(pin, method='pbkdf2:sha256') + # No-gaps: while login mode is on, a new member must be born with a login + # password or they could never sign in. + password = (data.get('password') or '').strip() + from core.security.login_provisioning import create_needs_password + if create_needs_password(_require_login_enabled()) and not password: + return jsonify({'success': False, + 'error': 'Login mode is on — give this profile a login ' + 'password so they can sign in.'}), 400 + # Profile settings: home_page, allowed_pages, can_download home_page = data.get('home_page') or None allowed_pages = data.get('allowed_pages') # list or None @@ -25586,6 +25606,9 @@ def create_profile(): if profile_id is None: return jsonify({'success': False, 'error': 'Profile name already exists'}), 409 + if password: + database.set_profile_password(profile_id, password) + return jsonify({'success': True, 'profile_id': profile_id}) except Exception as e: return jsonify({'success': False, 'error': str(e)}), 500 @@ -26000,6 +26023,13 @@ def set_profile_password_endpoint(profile_id): return jsonify({'success': False, 'error': 'Unauthorized'}), 403 data = request.json or {} password = data.get('password', '') + # No-gaps: clearing a password while login mode is on would lock that + # profile out — refuse it (delete the profile instead if that's intended). + from core.security.login_provisioning import removing_password_strands + if not (password or '').strip() and removing_password_strands(_require_login_enabled()): + return jsonify({'success': False, + 'error': "Can't remove this password while login mode is on — " + "that profile couldn't sign in."}), 400 ok = database.set_profile_password(profile_id, password) return jsonify({'success': bool(ok), 'has_password': database.profile_has_password(profile_id)}) except Exception as e: diff --git a/webui/index.html b/webui/index.html index 58a9e3a7..1adba33d 100644 --- a/webui/index.html +++ b/webui/index.html @@ -131,6 +131,7 @@ +
- qBittorrent: WebUI port (default 8080). Transmission: RPC port (default 9091). Deluge: WebUI port (default 8112). + qBittorrent: WebUI port (default 8080). Transmission: RPC port (default 9091). Deluge: WebUI port (default 8112). Aria2: RPC port (default 6800) — leave Username blank and put your --rpc-secret in the Password field.
From f8652c106b0cdd0d9fabf27c88f54e665e59d8a5 Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Thu, 11 Jun 2026 22:48:58 -0700 Subject: [PATCH 13/64] Watchlist: export the roster to JSON / CSV / text (corruption's request) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An "Export" button on the watchlist filter bar opens a modal (same aesthetic as the artist DB-record inspector) to export your whole watchlist roster — each artist's name + source IDs (spotify / musicbrainz / deezer / discogs / itunes / amazon), with an optional "external links" toggle that adds the discography URLs built from those IDs. Live preview, copy, and download in the chosen format. - core/exports/watchlist_export.py: pure builder (json/csv/txt + links, present-IDs only, deterministic columns) — the single source of truth, fully unit-tested. - GET /api/watchlist/export?format=&links= shapes the roster + returns it (with X-Export-Count / X-Export-Ext headers for the modal). - Frontend reuses the DB-record helpers (_jsonSyntaxHighlight / _arecCopy). Tests (8): builder across json/csv/txt, links on/off, present-ids-only, empty + bad-format fallback, mime/ext, and endpoint wiring. ruff clean; 64 integrity green. Scoped to the watchlist for v1; library-wide export + a "library contents" (owned albums/tracks) option are natural follow-ups. --- core/exports/__init__.py | 1 + core/exports/watchlist_export.py | 96 +++++++++++++++++++++++++++++++ tests/test_watchlist_export.py | 88 +++++++++++++++++++++++++++++ web_server.py | 29 ++++++++++ webui/index.html | 4 ++ webui/static/library.js | 97 ++++++++++++++++++++++++++++++++ webui/static/style.css | 10 ++++ 7 files changed, 325 insertions(+) create mode 100644 core/exports/__init__.py create mode 100644 core/exports/watchlist_export.py create mode 100644 tests/test_watchlist_export.py diff --git a/core/exports/__init__.py b/core/exports/__init__.py new file mode 100644 index 00000000..41b21484 --- /dev/null +++ b/core/exports/__init__.py @@ -0,0 +1 @@ +"""Data export builders.""" diff --git a/core/exports/watchlist_export.py b/core/exports/watchlist_export.py new file mode 100644 index 00000000..404c768b --- /dev/null +++ b/core/exports/watchlist_export.py @@ -0,0 +1,96 @@ +"""Export a watchlist (or library) artist roster to JSON / CSV / plain text +(corruption's request). + +Pure shaping + formatting so it's the single source of truth and unit-testable — +web_server fetches the artists and hands them here; the UI just picks options and +downloads. Always exports the name + whatever source IDs each artist has; the +``include_links`` option adds the external discography URLs built from those IDs. +""" + +from __future__ import annotations + +import csv +import io +import json +from typing import Any, Dict, List, Optional + +# Source id field → external URL builder. +_LINKS = { + 'spotify_artist_id': lambda i: f'https://open.spotify.com/artist/{i}', + 'musicbrainz_artist_id': lambda i: f'https://musicbrainz.org/artist/{i}', + 'deezer_artist_id': lambda i: f'https://www.deezer.com/artist/{i}', + 'discogs_artist_id': lambda i: f'https://www.discogs.com/artist/{i}', + 'itunes_artist_id': lambda i: f'https://music.apple.com/artist/{i}', +} +# Order is stable so CSV columns + JSON keys are deterministic. +_ID_FIELDS = ['spotify_artist_id', 'musicbrainz_artist_id', 'deezer_artist_id', + 'discogs_artist_id', 'itunes_artist_id', 'amazon_artist_id'] + +VALID_FORMATS = ('json', 'csv', 'txt') + + +def _name(a: Dict[str, Any]) -> str: + return str(a.get('artist_name') or a.get('name') or '').strip() + + +def _short(field: str) -> str: + return field.replace('_artist_id', '') + + +def _row(a: Dict[str, Any], include_links: bool) -> Dict[str, Any]: + row: Dict[str, Any] = {'name': _name(a)} + for f in _ID_FIELDS: + if a.get(f): + row[f] = str(a[f]) + if include_links: + links = {_short(f): b(a[f]) for f, b in _LINKS.items() if a.get(f)} + if links: + row['links'] = links + return row + + +def build_watchlist_export(artists: Optional[List[Dict[str, Any]]], + fmt: str = 'json', include_links: bool = False) -> str: + """Return the roster serialized in ``fmt`` (json | csv | txt). + + - ``txt`` → one artist name per line. + - ``csv`` → name + each source-id column (+ a *_url column per service when + ``include_links``). + - ``json`` → a list of objects: name, present source ids, and a ``links`` map + when ``include_links``. + """ + artists = artists or [] + fmt = (fmt or 'json').lower() + if fmt not in VALID_FORMATS: + fmt = 'json' + + if fmt == 'txt': + return '\n'.join(n for n in (_name(a) for a in artists) if n) + + if fmt == 'csv': + cols = ['name'] + _ID_FIELDS + if include_links: + cols += [f'{_short(f)}_url' for f in _LINKS] + out = io.StringIO() + w = csv.writer(out) + w.writerow(cols) + for a in artists: + line = [_name(a)] + [str(a.get(f) or '') for f in _ID_FIELDS] + if include_links: + line += [_LINKS[f](a[f]) if a.get(f) else '' for f in _LINKS] + w.writerow(line) + return out.getvalue() + + return json.dumps([_row(a, include_links) for a in artists], indent=2, ensure_ascii=False) + + +def export_mime_and_ext(fmt: str): + """(content-type, file extension) for a format.""" + return { + 'json': ('application/json', 'json'), + 'csv': ('text/csv', 'csv'), + 'txt': ('text/plain', 'txt'), + }.get((fmt or 'json').lower(), ('application/json', 'json')) + + +__all__ = ['build_watchlist_export', 'export_mime_and_ext', 'VALID_FORMATS'] diff --git a/tests/test_watchlist_export.py b/tests/test_watchlist_export.py new file mode 100644 index 00000000..41171291 --- /dev/null +++ b/tests/test_watchlist_export.py @@ -0,0 +1,88 @@ +"""Watchlist roster export builder (corruption's request) — JSON / CSV / txt, +optional external links, deterministic columns.""" + +from __future__ import annotations + +import csv +import io +import json + +from core.exports.watchlist_export import build_watchlist_export, export_mime_and_ext + + +_ARTISTS = [ + {'artist_name': 'Rob Zombie', 'spotify_artist_id': 'sp1', + 'musicbrainz_artist_id': 'mb1', 'deezer_artist_id': 'dz1'}, + {'artist_name': 'Nobody IDs', 'spotify_artist_id': None}, +] + + +def test_txt_is_names_one_per_line(): + out = build_watchlist_export(_ARTISTS, fmt='txt') + assert out == 'Rob Zombie\nNobody IDs' + + +def test_json_includes_present_ids_only(): + out = json.loads(build_watchlist_export(_ARTISTS, fmt='json')) + assert out[0]['name'] == 'Rob Zombie' + assert out[0]['spotify_artist_id'] == 'sp1' and out[0]['musicbrainz_artist_id'] == 'mb1' + assert 'deezer_artist_id' in out[0] + assert out[1] == {'name': 'Nobody IDs'} # null id dropped, no links key + + +def test_json_links_when_requested(): + out = json.loads(build_watchlist_export(_ARTISTS, fmt='json', include_links=True)) + assert out[0]['links']['spotify'] == 'https://open.spotify.com/artist/sp1' + assert out[0]['links']['musicbrainz'] == 'https://musicbrainz.org/artist/mb1' + assert 'links' not in out[1] # no ids → no links + + +def test_csv_header_and_rows(): + out = build_watchlist_export(_ARTISTS, fmt='csv') + rows = list(csv.reader(io.StringIO(out))) + assert rows[0][0] == 'name' and 'spotify_artist_id' in rows[0] + assert rows[1][0] == 'Rob Zombie' + assert rows[2][0] == 'Nobody IDs' + + +def test_csv_adds_url_columns_with_links(): + out = build_watchlist_export(_ARTISTS, fmt='csv', include_links=True) + header = next(csv.reader(io.StringIO(out))) + assert 'spotify_url' in header and 'discogs_url' in header + + +def test_empty_and_bad_format(): + assert build_watchlist_export([], fmt='txt') == '' + assert build_watchlist_export(None, fmt='json') == '[]' + assert build_watchlist_export(_ARTISTS, fmt='nonsense').startswith('[') # falls back to json + + +def test_mime_and_ext(): + assert export_mime_and_ext('csv') == ('text/csv', 'csv') + assert export_mime_and_ext('txt') == ('text/plain', 'txt') + assert export_mime_and_ext('weird') == ('application/json', 'json') + + +# ── endpoint wiring (empty watchlist → valid shapes + headers) ────────────── +import os, tempfile # noqa: E402 +os.environ['DATABASE_PATH'] = os.path.join(tempfile.mkdtemp(prefix='soulsync-testdb-wlexp-'), 'w.db') +os.environ['SOULSYNC_TEST_DB_READY'] = '1' +import pytest # noqa: E402 +web_server = pytest.importorskip('web_server') + + +@pytest.fixture +def client(): + return web_server.app.test_client() + + +def test_export_endpoint_wiring(client): + r = client.get('/api/watchlist/export?format=json') + assert r.status_code == 200 + assert r.data.decode().strip() == '[]' + assert r.headers.get('X-Export-Count') == '0' + assert r.headers.get('X-Export-Ext') == 'json' + + r2 = client.get('/api/watchlist/export?format=csv&links=1') + assert r2.status_code == 200 and r2.headers.get('X-Export-Ext') == 'csv' + assert 'spotify_url' in r2.data.decode().splitlines()[0] # header row with links diff --git a/web_server.py b/web_server.py index 2dbc1907..3460c054 100644 --- a/web_server.py +++ b/web_server.py @@ -26722,6 +26722,35 @@ def get_watchlist_artists(): logger.error(f"Error getting watchlist artists: {e}") return jsonify({"success": False, "error": str(e)}), 500 + +@app.route('/api/watchlist/export', methods=['GET']) +def export_watchlist(): + """Export the watchlist roster (name + source IDs, optionally external links) + as json / csv / txt. Returns the content for the export modal to preview + + download (X-Export-Count / X-Export-Ext headers carry the metadata).""" + try: + from core.exports.watchlist_export import build_watchlist_export, export_mime_and_ext + fmt = (request.args.get('format', 'json') or 'json').lower() + include_links = request.args.get('links', '') in ('1', 'true', 'yes') + database = get_database() + artists = [{ + 'artist_name': a.artist_name, + 'spotify_artist_id': a.spotify_artist_id, + 'itunes_artist_id': a.itunes_artist_id, + 'deezer_artist_id': getattr(a, 'deezer_artist_id', None), + 'discogs_artist_id': getattr(a, 'discogs_artist_id', None), + 'musicbrainz_artist_id': getattr(a, 'musicbrainz_artist_id', None), + 'amazon_artist_id': getattr(a, 'amazon_artist_id', None), + } for a in database.get_watchlist_artists(profile_id=get_current_profile_id())] + content = build_watchlist_export(artists, fmt=fmt, include_links=include_links) + mime, ext = export_mime_and_ext(fmt) + return Response(content, mimetype=mime, + headers={'X-Export-Count': str(len(artists)), 'X-Export-Ext': ext}) + except Exception as e: + logger.error(f"Watchlist export failed: {e}") + return jsonify({"success": False, "error": str(e)}), 500 + + @app.route('/api/watchlist/add', methods=['POST']) def add_to_watchlist(): """Add an artist to the watchlist""" diff --git a/webui/index.html b/webui/index.html index 545b79b2..e1b7884c 100644 --- a/webui/index.html +++ b/webui/index.html @@ -2507,6 +2507,10 @@ 👁️ Watch All Unwatched +
diff --git a/webui/static/library.js b/webui/static/library.js index 4adcda46..65b8e02a 100644 --- a/webui/static/library.js +++ b/webui/static/library.js @@ -9010,3 +9010,100 @@ function _arecEsc(s) { function _arecEscAttr(s) { return _arecEsc(s).replace(/"/g, '"'); } + + +// ════════════════════════════════════════════════════════════════════════════ +// Watchlist export — bulk export the watchlist roster to JSON / CSV / text, with +// optional external discography links. Reuses the DB-record modal aesthetic + +// helpers (_jsonSyntaxHighlight / _arecCopy / _arecEsc). #export-request +// ════════════════════════════════════════════════════════════════════════════ +async function openWatchlistExportModal() { + const existing = document.getElementById('wl-export-overlay'); + if (existing) existing.remove(); + + const overlay = document.createElement('div'); + overlay.id = 'wl-export-overlay'; + overlay.className = 'arec-overlay'; + overlay.innerHTML = + ''; + document.body.appendChild(overlay); + requestAnimationFrame(() => overlay.classList.add('visible')); + + let fmt = 'json', links = false, content = ''; + + const close = () => { + overlay.classList.remove('visible'); + document.removeEventListener('keydown', onKey); + setTimeout(() => overlay.remove(), 220); + }; + const onKey = (e) => { if (e.key === 'Escape') close(); }; + document.addEventListener('keydown', onKey); + overlay.addEventListener('click', (e) => { if (e.target === overlay) close(); }); + overlay.querySelector('#wlx-close').onclick = close; + + const refresh = async () => { + const body = document.getElementById('wlx-body'); + body.innerHTML = '
Building export…
'; + try { + const res = await fetch('/api/watchlist/export?format=' + fmt + '&links=' + (links ? '1' : '0')); + content = await res.text(); + const count = res.headers.get('X-Export-Count') || '?'; + document.getElementById('wlx-footer').innerHTML = + '' + count + ' artists' + fmt.toUpperCase() + ''; + if (fmt === 'json') { + let parsed; try { parsed = JSON.parse(content || '[]'); } catch (e) { parsed = []; } + body.innerHTML = '
' + _jsonSyntaxHighlight(parsed) + '
'; + } else { + body.innerHTML = '
' + _arecEsc(content || '(empty)') + '
'; + } + } catch (err) { + body.innerHTML = '
Export failed: ' + _arecEsc(err.message || String(err)) + '
'; + } + }; + + overlay.querySelectorAll('#wlx-format .arec-tab').forEach(t => { + t.onclick = () => { + overlay.querySelectorAll('#wlx-format .arec-tab').forEach(x => x.classList.remove('active')); + t.classList.add('active'); + fmt = t.dataset.fmt; + refresh(); + }; + }); + document.getElementById('wlx-links').addEventListener('change', (e) => { links = e.target.checked; refresh(); }); + document.getElementById('wlx-copy').onclick = () => _arecCopy(content, 'Export copied'); + document.getElementById('wlx-download').onclick = () => { + const ext = fmt; + const mime = fmt === 'json' ? 'application/json' : (fmt === 'csv' ? 'text/csv' : 'text/plain'); + const blob = new Blob([content || ''], { type: mime }); + const url = URL.createObjectURL(blob); + const a = document.createElement('a'); + a.href = url; a.download = 'watchlist_export.' + ext; + document.body.appendChild(a); a.click(); a.remove(); + setTimeout(() => URL.revokeObjectURL(url), 1000); + if (typeof showToast === 'function') showToast('Saved watchlist_export.' + ext, 'success'); + }; + + refresh(); +} diff --git a/webui/static/style.css b/webui/static/style.css index 94c1f058..33341f3c 100644 --- a/webui/static/style.css +++ b/webui/static/style.css @@ -68243,3 +68243,13 @@ body.app-locked > *:not(#launch-pin-overlay):not(#login-overlay):not(script):not 0%, 100% { box-shadow: 0 0 0 0 rgba(239, 68, 68, 0.0); } 50% { box-shadow: 0 0 0 4px rgba(239, 68, 68, 0.18); } } + + +/* Watchlist export modal — reuses the .arec-* card; just the option label + button */ +.wlx-opt { + display: inline-flex; align-items: center; gap: 6px; + font: 12px/1 'JetBrains Mono', ui-monospace, monospace; + color: #8b93b0; cursor: pointer; user-select: none; +} +.wlx-opt input { accent-color: #7aa2f7; } +.watchlist-export-btn .watchlist-all-icon { font-weight: 700; } From a789fb71c0f5bee47eca05354aab2572f26c1eab Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Thu, 11 Jun 2026 22:59:21 -0700 Subject: [PATCH 14/64] Library export: export the whole library roster too (corruption's request) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Extends the watchlist export to the full library. The exporter is now general (core/exports/artist_export.py, renamed from watchlist_export) — adds tidal/qobuz links and an extra_fields passthrough, so the library export also carries lastfm/genius URLs + soul_id, and an optional "library counts" toggle adds owned album/track counts per artist. - GET /api/library/artists/export?format=&links=&contents= — pulls every artists row, normalizes onto the canonical *_artist_id keys, optionally GROUP-BY counts for album/track totals. - The export modal is now openArtistExportModal(scope): "Export Library" button in the library header + the existing "Export" on the watchlist bar (a thin wrapper). Library mode shows the extra "library counts" toggle. Tests (11): builder across formats + the new tidal/qobuz links + extra_fields columns; watchlist + library endpoint wiring. 64 integrity green; ruff clean. --- .../{watchlist_export.py => artist_export.py} | 47 ++++++++----- ...chlist_export.py => test_artist_export.py} | 54 +++++++++++--- web_server.py | 70 ++++++++++++++++++- webui/index.html | 4 ++ webui/static/library.js | 29 +++++--- 5 files changed, 168 insertions(+), 36 deletions(-) rename core/exports/{watchlist_export.py => artist_export.py} (56%) rename tests/{test_watchlist_export.py => test_artist_export.py} (53%) diff --git a/core/exports/watchlist_export.py b/core/exports/artist_export.py similarity index 56% rename from core/exports/watchlist_export.py rename to core/exports/artist_export.py index 404c768b..ac022a42 100644 --- a/core/exports/watchlist_export.py +++ b/core/exports/artist_export.py @@ -1,10 +1,12 @@ -"""Export a watchlist (or library) artist roster to JSON / CSV / plain text +"""Export an artist roster — watchlist OR library — to JSON / CSV / plain text (corruption's request). Pure shaping + formatting so it's the single source of truth and unit-testable — -web_server fetches the artists and hands them here; the UI just picks options and -downloads. Always exports the name + whatever source IDs each artist has; the -``include_links`` option adds the external discography URLs built from those IDs. +web_server fetches the artists (normalizing each source's fields onto the canonical +``*_artist_id`` keys below) and hands them here; the UI just picks options and +downloads. Always exports the name + whatever source IDs each artist has; +``include_links`` adds external discography URLs; ``extra_fields`` passes through +source-specific extras (e.g. library album/track counts) in a stable order. """ from __future__ import annotations @@ -14,17 +16,21 @@ import io import json from typing import Any, Dict, List, Optional -# Source id field → external URL builder. +# Canonical id field → external URL builder. _LINKS = { 'spotify_artist_id': lambda i: f'https://open.spotify.com/artist/{i}', 'musicbrainz_artist_id': lambda i: f'https://musicbrainz.org/artist/{i}', 'deezer_artist_id': lambda i: f'https://www.deezer.com/artist/{i}', 'discogs_artist_id': lambda i: f'https://www.discogs.com/artist/{i}', 'itunes_artist_id': lambda i: f'https://music.apple.com/artist/{i}', + 'tidal_artist_id': lambda i: f'https://tidal.com/artist/{i}', + 'qobuz_artist_id': lambda i: f'https://www.qobuz.com/artist/{i}', } -# Order is stable so CSV columns + JSON keys are deterministic. +# Stable order so CSV columns + JSON keys are deterministic. amazon carries an id +# but no clean public URL. _ID_FIELDS = ['spotify_artist_id', 'musicbrainz_artist_id', 'deezer_artist_id', - 'discogs_artist_id', 'itunes_artist_id', 'amazon_artist_id'] + 'discogs_artist_id', 'itunes_artist_id', 'tidal_artist_id', + 'qobuz_artist_id', 'amazon_artist_id'] VALID_FORMATS = ('json', 'csv', 'txt') @@ -37,11 +43,14 @@ def _short(field: str) -> str: return field.replace('_artist_id', '') -def _row(a: Dict[str, Any], include_links: bool) -> Dict[str, Any]: +def _row(a: Dict[str, Any], include_links: bool, extra_fields: List[str]) -> Dict[str, Any]: row: Dict[str, Any] = {'name': _name(a)} for f in _ID_FIELDS: if a.get(f): row[f] = str(a[f]) + for f in extra_fields: + if a.get(f) not in (None, ''): + row[f] = a[f] if include_links: links = {_short(f): b(a[f]) for f, b in _LINKS.items() if a.get(f)} if links: @@ -49,17 +58,19 @@ def _row(a: Dict[str, Any], include_links: bool) -> Dict[str, Any]: return row -def build_watchlist_export(artists: Optional[List[Dict[str, Any]]], - fmt: str = 'json', include_links: bool = False) -> str: +def build_artist_export(artists: Optional[List[Dict[str, Any]]], + fmt: str = 'json', include_links: bool = False, + extra_fields: Optional[List[str]] = None) -> str: """Return the roster serialized in ``fmt`` (json | csv | txt). - ``txt`` → one artist name per line. - - ``csv`` → name + each source-id column (+ a *_url column per service when - ``include_links``). - - ``json`` → a list of objects: name, present source ids, and a ``links`` map - when ``include_links``. + - ``csv`` → name + each source-id column + ``extra_fields`` columns (+ a + *_url column per service when ``include_links``). + - ``json`` → a list of objects: name, present source ids, present extras, and + a ``links`` map when ``include_links``. """ artists = artists or [] + extra_fields = list(extra_fields or []) fmt = (fmt or 'json').lower() if fmt not in VALID_FORMATS: fmt = 'json' @@ -68,7 +79,7 @@ def build_watchlist_export(artists: Optional[List[Dict[str, Any]]], return '\n'.join(n for n in (_name(a) for a in artists) if n) if fmt == 'csv': - cols = ['name'] + _ID_FIELDS + cols = ['name'] + _ID_FIELDS + extra_fields if include_links: cols += [f'{_short(f)}_url' for f in _LINKS] out = io.StringIO() @@ -76,12 +87,14 @@ def build_watchlist_export(artists: Optional[List[Dict[str, Any]]], w.writerow(cols) for a in artists: line = [_name(a)] + [str(a.get(f) or '') for f in _ID_FIELDS] + line += [str(a.get(f) if a.get(f) is not None else '') for f in extra_fields] if include_links: line += [_LINKS[f](a[f]) if a.get(f) else '' for f in _LINKS] w.writerow(line) return out.getvalue() - return json.dumps([_row(a, include_links) for a in artists], indent=2, ensure_ascii=False) + return json.dumps([_row(a, include_links, extra_fields) for a in artists], + indent=2, ensure_ascii=False) def export_mime_and_ext(fmt: str): @@ -93,4 +106,4 @@ def export_mime_and_ext(fmt: str): }.get((fmt or 'json').lower(), ('application/json', 'json')) -__all__ = ['build_watchlist_export', 'export_mime_and_ext', 'VALID_FORMATS'] +__all__ = ['build_artist_export', 'export_mime_and_ext', 'VALID_FORMATS'] diff --git a/tests/test_watchlist_export.py b/tests/test_artist_export.py similarity index 53% rename from tests/test_watchlist_export.py rename to tests/test_artist_export.py index 41171291..2f750c3d 100644 --- a/tests/test_watchlist_export.py +++ b/tests/test_artist_export.py @@ -7,7 +7,7 @@ import csv import io import json -from core.exports.watchlist_export import build_watchlist_export, export_mime_and_ext +from core.exports.artist_export import build_artist_export, export_mime_and_ext _ARTISTS = [ @@ -18,12 +18,12 @@ _ARTISTS = [ def test_txt_is_names_one_per_line(): - out = build_watchlist_export(_ARTISTS, fmt='txt') + out = build_artist_export(_ARTISTS, fmt='txt') assert out == 'Rob Zombie\nNobody IDs' def test_json_includes_present_ids_only(): - out = json.loads(build_watchlist_export(_ARTISTS, fmt='json')) + out = json.loads(build_artist_export(_ARTISTS, fmt='json')) assert out[0]['name'] == 'Rob Zombie' assert out[0]['spotify_artist_id'] == 'sp1' and out[0]['musicbrainz_artist_id'] == 'mb1' assert 'deezer_artist_id' in out[0] @@ -31,14 +31,14 @@ def test_json_includes_present_ids_only(): def test_json_links_when_requested(): - out = json.loads(build_watchlist_export(_ARTISTS, fmt='json', include_links=True)) + out = json.loads(build_artist_export(_ARTISTS, fmt='json', include_links=True)) assert out[0]['links']['spotify'] == 'https://open.spotify.com/artist/sp1' assert out[0]['links']['musicbrainz'] == 'https://musicbrainz.org/artist/mb1' assert 'links' not in out[1] # no ids → no links def test_csv_header_and_rows(): - out = build_watchlist_export(_ARTISTS, fmt='csv') + out = build_artist_export(_ARTISTS, fmt='csv') rows = list(csv.reader(io.StringIO(out))) assert rows[0][0] == 'name' and 'spotify_artist_id' in rows[0] assert rows[1][0] == 'Rob Zombie' @@ -46,15 +46,15 @@ def test_csv_header_and_rows(): def test_csv_adds_url_columns_with_links(): - out = build_watchlist_export(_ARTISTS, fmt='csv', include_links=True) + out = build_artist_export(_ARTISTS, fmt='csv', include_links=True) header = next(csv.reader(io.StringIO(out))) assert 'spotify_url' in header and 'discogs_url' in header def test_empty_and_bad_format(): - assert build_watchlist_export([], fmt='txt') == '' - assert build_watchlist_export(None, fmt='json') == '[]' - assert build_watchlist_export(_ARTISTS, fmt='nonsense').startswith('[') # falls back to json + assert build_artist_export([], fmt='txt') == '' + assert build_artist_export(None, fmt='json') == '[]' + assert build_artist_export(_ARTISTS, fmt='nonsense').startswith('[') # falls back to json def test_mime_and_ext(): @@ -86,3 +86,39 @@ def test_export_endpoint_wiring(client): r2 = client.get('/api/watchlist/export?format=csv&links=1') assert r2.status_code == 200 and r2.headers.get('X-Export-Ext') == 'csv' assert 'spotify_url' in r2.data.decode().splitlines()[0] # header row with links + + +# ── library-side: extra services + extra_fields passthrough ───────────────── +_LIB = [{ + 'name': 'Rob Zombie', 'spotify_artist_id': 'sp1', 'tidal_artist_id': 'td1', + 'qobuz_artist_id': 'qz1', 'lastfm_url': 'https://last.fm/x', 'soul_id': 'soul_abc', + 'album_count': 10, 'track_count': 159, +}] + + +def test_tidal_qobuz_links_and_extra_fields_json(): + out = json.loads(build_artist_export(_LIB, fmt='json', include_links=True, + extra_fields=['lastfm_url', 'soul_id', 'album_count', 'track_count'])) + a = out[0] + assert a['tidal_artist_id'] == 'td1' and a['qobuz_artist_id'] == 'qz1' + assert a['links']['tidal'] == 'https://tidal.com/artist/td1' + assert a['links']['qobuz'] == 'https://www.qobuz.com/artist/qz1' + assert a['lastfm_url'] == 'https://last.fm/x' and a['soul_id'] == 'soul_abc' + assert a['album_count'] == 10 and a['track_count'] == 159 + + +def test_extra_fields_become_csv_columns(): + out = build_artist_export(_LIB, fmt='csv', extra_fields=['album_count', 'track_count']) + header = next(csv.reader(io.StringIO(out))) + assert 'album_count' in header and 'track_count' in header + assert 'tidal_artist_id' in header # new service column present + + +def test_library_export_endpoint_wiring(client): + r = client.get('/api/library/artists/export?format=json&contents=1&links=1') + assert r.status_code == 200 + assert r.data.decode().strip() == '[]' # empty test DB + assert r.headers.get('X-Export-Count') == '0' + r2 = client.get('/api/library/artists/export?format=csv&contents=1') + header = r2.data.decode().splitlines()[0] + assert 'album_count' in header and 'track_count' in header diff --git a/web_server.py b/web_server.py index 3460c054..a731e95f 100644 --- a/web_server.py +++ b/web_server.py @@ -26729,7 +26729,7 @@ def export_watchlist(): as json / csv / txt. Returns the content for the export modal to preview + download (X-Export-Count / X-Export-Ext headers carry the metadata).""" try: - from core.exports.watchlist_export import build_watchlist_export, export_mime_and_ext + from core.exports.artist_export import build_artist_export, export_mime_and_ext fmt = (request.args.get('format', 'json') or 'json').lower() include_links = request.args.get('links', '') in ('1', 'true', 'yes') database = get_database() @@ -26742,7 +26742,7 @@ def export_watchlist(): 'musicbrainz_artist_id': getattr(a, 'musicbrainz_artist_id', None), 'amazon_artist_id': getattr(a, 'amazon_artist_id', None), } for a in database.get_watchlist_artists(profile_id=get_current_profile_id())] - content = build_watchlist_export(artists, fmt=fmt, include_links=include_links) + content = build_artist_export(artists, fmt=fmt, include_links=include_links) mime, ext = export_mime_and_ext(fmt) return Response(content, mimetype=mime, headers={'X-Export-Count': str(len(artists)), 'X-Export-Ext': ext}) @@ -26751,6 +26751,72 @@ def export_watchlist(): return jsonify({"success": False, "error": str(e)}), 500 +@app.route('/api/library/artists/export', methods=['GET']) +def export_library_artists(): + """Export the WHOLE library artist roster (name + every source id/url we have, + optional external links, optional owned album/track counts) as json/csv/txt.""" + try: + from core.exports.artist_export import build_artist_export, export_mime_and_ext + fmt = (request.args.get('format', 'json') or 'json').lower() + include_links = request.args.get('links', '') in ('1', 'true', 'yes') + include_contents = request.args.get('contents', '') in ('1', 'true', 'yes') + database = get_database() + conn = database._get_connection() + try: + cur = conn.cursor() + cur.execute(""" + SELECT id, name, spotify_artist_id, musicbrainz_id, deezer_id, + discogs_id, itunes_artist_id, tidal_id, qobuz_id, amazon_id, + lastfm_url, genius_url, soul_id + FROM artists ORDER BY name COLLATE NOCASE + """) + cols = [d[0] for d in cur.description] + rows = [dict(zip(cols, r, strict=False)) for r in cur.fetchall()] + + counts = {} + if include_contents: + for table, key in (('albums', 'album_count'), ('tracks', 'track_count')): + try: + for aid, n in cur.execute( + f"SELECT artist_id, COUNT(*) FROM {table} GROUP BY artist_id"): + counts.setdefault(str(aid), {})[key] = n + except Exception: # noqa: S110 — counts are best-effort + pass + finally: + conn.close() + + # Normalize onto the canonical *_artist_id keys the exporter expects. + artists = [] + for r in rows: + c = counts.get(str(r['id']), {}) + artists.append({ + 'name': r['name'], + 'spotify_artist_id': r['spotify_artist_id'], + 'musicbrainz_artist_id': r['musicbrainz_id'], + 'deezer_artist_id': r['deezer_id'], + 'discogs_artist_id': r['discogs_id'], + 'itunes_artist_id': r['itunes_artist_id'], + 'tidal_artist_id': r['tidal_id'], + 'qobuz_artist_id': r['qobuz_id'], + 'amazon_artist_id': r['amazon_id'], + 'lastfm_url': r['lastfm_url'], + 'genius_url': r['genius_url'], + 'soul_id': r['soul_id'], + 'album_count': c.get('album_count'), + 'track_count': c.get('track_count'), + }) + extra = ['lastfm_url', 'genius_url', 'soul_id'] + if include_contents: + extra += ['album_count', 'track_count'] + content = build_artist_export(artists, fmt=fmt, include_links=include_links, extra_fields=extra) + mime, ext = export_mime_and_ext(fmt) + return Response(content, mimetype=mime, + headers={'X-Export-Count': str(len(artists)), 'X-Export-Ext': ext}) + except Exception as e: + logger.error(f"Library artist export failed: {e}") + return jsonify({"success": False, "error": str(e)}), 500 + + @app.route('/api/watchlist/add', methods=['POST']) def add_to_watchlist(): """Add an artist to the watchlist""" diff --git a/webui/index.html b/webui/index.html index e1b7884c..dbefb488 100644 --- a/webui/index.html +++ b/webui/index.html @@ -2488,6 +2488,10 @@ Artists + diff --git a/webui/static/library.js b/webui/static/library.js index 65b8e02a..88ac92a0 100644 --- a/webui/static/library.js +++ b/webui/static/library.js @@ -9017,7 +9017,14 @@ function _arecEscAttr(s) { // optional external discography links. Reuses the DB-record modal aesthetic + // helpers (_jsonSyntaxHighlight / _arecCopy / _arecEsc). #export-request // ════════════════════════════════════════════════════════════════════════════ -async function openWatchlistExportModal() { +function openWatchlistExportModal() { return openArtistExportModal('watchlist'); } + +async function openArtistExportModal(scope) { + scope = scope || 'watchlist'; + const isLib = scope === 'library'; + const endpoint = isLib ? '/api/library/artists/export' : '/api/watchlist/export'; + const fileBase = isLib ? 'library_artists' : 'watchlist'; + const existing = document.getElementById('wl-export-overlay'); if (existing) existing.remove(); @@ -9025,11 +9032,13 @@ async function openWatchlistExportModal() { overlay.id = 'wl-export-overlay'; overlay.className = 'arec-overlay'; overlay.innerHTML = - ' @@ -2511,10 +2511,6 @@ 👁️ Watch All Unwatched - diff --git a/webui/static/library.js b/webui/static/library.js index 88ac92a0..cd88e264 100644 --- a/webui/static/library.js +++ b/webui/static/library.js @@ -9017,13 +9017,11 @@ function _arecEscAttr(s) { // optional external discography links. Reuses the DB-record modal aesthetic + // helpers (_jsonSyntaxHighlight / _arecCopy / _arecEsc). #export-request // ════════════════════════════════════════════════════════════════════════════ -function openWatchlistExportModal() { return openArtistExportModal('watchlist'); } - -async function openArtistExportModal(scope) { - scope = scope || 'watchlist'; - const isLib = scope === 'library'; - const endpoint = isLib ? '/api/library/artists/export' : '/api/watchlist/export'; - const fileBase = isLib ? 'library_artists' : 'watchlist'; +async function openArtistExportModal(initialScope) { + // One export modal for both rosters — pick Watchlist or Library inside. + let scope = initialScope || 'watchlist'; + const epOf = (s) => s === 'library' ? '/api/library/artists/export' : '/api/watchlist/export'; + const fileOf = (s) => s === 'library' ? 'library_artists' : 'watchlist'; const existing = document.getElementById('wl-export-overlay'); if (existing) existing.remove(); @@ -9035,10 +9033,11 @@ async function openArtistExportModal(scope) { '' + '' + - (isLib ? '' : '') + + '' + '
' + '' + '' + @@ -9063,6 +9062,17 @@ async function openArtistExportModal(scope) { let fmt = 'json', links = false, contents = false, content = ''; + const applyScopeUI = () => { + // "library counts" only applies to the library roster. + document.getElementById('wlx-contents-wrap').style.display = (scope === 'library') ? '' : 'none'; + if (scope !== 'library') { + contents = false; + const cb = document.getElementById('wlx-contents'); + if (cb) cb.checked = false; + } + }; + applyScopeUI(); + const close = () => { overlay.classList.remove('visible'); document.removeEventListener('keydown', onKey); @@ -9077,12 +9087,13 @@ async function openArtistExportModal(scope) { const body = document.getElementById('wlx-body'); body.innerHTML = '
Building export…
'; try { - const res = await fetch(endpoint + '?format=' + fmt + '&links=' + (links ? '1' : '0') - + (isLib && contents ? '&contents=1' : '')); + const res = await fetch(epOf(scope) + '?format=' + fmt + '&links=' + (links ? '1' : '0') + + (scope === 'library' && contents ? '&contents=1' : '')); content = await res.text(); const count = res.headers.get('X-Export-Count') || '?'; document.getElementById('wlx-footer').innerHTML = - '' + count + ' artists' + fmt.toUpperCase() + ''; + '' + count + ' ' + (scope === 'library' ? 'library' : 'watchlist') + ' artists' + + '' + fmt.toUpperCase() + ''; if (fmt === 'json') { let parsed; try { parsed = JSON.parse(content || '[]'); } catch (e) { parsed = []; } body.innerHTML = '
' + _jsonSyntaxHighlight(parsed) + '
'; @@ -9094,6 +9105,16 @@ async function openArtistExportModal(scope) { } }; + overlay.querySelectorAll('#wlx-scope .arec-tab').forEach(t => { + t.onclick = () => { + if (t.dataset.scope === scope) return; + overlay.querySelectorAll('#wlx-scope .arec-tab').forEach(x => x.classList.remove('active')); + t.classList.add('active'); + scope = t.dataset.scope; + applyScopeUI(); + refresh(); + }; + }); overlay.querySelectorAll('#wlx-format .arec-tab').forEach(t => { t.onclick = () => { overlay.querySelectorAll('#wlx-format .arec-tab').forEach(x => x.classList.remove('active')); @@ -9103,8 +9124,7 @@ async function openArtistExportModal(scope) { }; }); document.getElementById('wlx-links').addEventListener('change', (e) => { links = e.target.checked; refresh(); }); - const _contentsEl = document.getElementById('wlx-contents'); - if (_contentsEl) _contentsEl.addEventListener('change', (e) => { contents = e.target.checked; refresh(); }); + document.getElementById('wlx-contents').addEventListener('change', (e) => { contents = e.target.checked; refresh(); }); document.getElementById('wlx-copy').onclick = () => _arecCopy(content, 'Export copied'); document.getElementById('wlx-download').onclick = () => { const ext = fmt; @@ -9112,10 +9132,10 @@ async function openArtistExportModal(scope) { const blob = new Blob([content || ''], { type: mime }); const url = URL.createObjectURL(blob); const a = document.createElement('a'); - a.href = url; a.download = fileBase + '_export.' + ext; + a.href = url; a.download = fileOf(scope) + '_export.' + ext; document.body.appendChild(a); a.click(); a.remove(); setTimeout(() => URL.revokeObjectURL(url), 1000); - if (typeof showToast === 'function') showToast('Saved ' + fileBase + '_export.' + ext, 'success'); + if (typeof showToast === 'function') showToast('Saved ' + fileOf(scope) + '_export.' + ext, 'success'); }; refresh(); From 749bc274b37bb23cd267abb6df0a1852a8a4a256 Mon Sep 17 00:00:00 2001 From: nick2000713 Date: Fri, 12 Jun 2026 12:54:03 +0200 Subject: [PATCH 16/64] fix: treat colon as separator in normalize_string so T:T matches T_T --- core/matching_engine.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/core/matching_engine.py b/core/matching_engine.py index 15f91f02..bd60aafe 100644 --- a/core/matching_engine.py +++ b/core/matching_engine.py @@ -115,7 +115,8 @@ class MusicMatchingEngine: # Replace common separators with spaces to preserve word boundaries. # Include hyphen in separator replacement for artist names like "AC/DC" vs "AC-DC" # Include '&' so "Pig&Dan" becomes "Pig Dan" (matches "Pig & Dan" on Soulseek) - text = re.sub(r'[._/&-]', ' ', text) + # Include ':' so "T:T" becomes "T T" (matches "T_T" stored with underscores on Soulseek) + text = re.sub(r'[._/&:\-]', ' ', text) # Keep alphanumeric characters, spaces, AND the '$' sign. # When CJK was detected upstream, also preserve CJK Unified From 94a0070fa8c9ca7f600e8feec61b27af427acb78 Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Fri, 12 Jun 2026 08:15:23 -0700 Subject: [PATCH 17/64] Orphan detector: hard-bail on a mass-orphan flood instead of warn-only MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A DB<->filesystem path mismatch (Docker volume change, remount, Music Paths unset for the container) makes EVERY library file fail to resolve to a DB track, so the orphan detector flags the whole library as orphaned. The mass-orphan check only logged a warning and then created the findings anyway — so a user batch-applying 'move to staging' or 'delete' would relocate or wipe their entire library. Make it a hard skip (create zero findings) like the dead-file cleaner and stale-removal paths already do (#828). Centralise the predicate as is_implausible_orphan_flood() alongside is_implausible_stale_removal() so the rule lives in one tested place. Small genuine orphan sets still surface unchanged — only an implausibly large flood (>50% and >20) is suppressed. Tests: seam cases for the new predicate + scan-level regressions (mass mismatch -> 0 findings; small genuine set -> still reported). --- core/library/stale_guard.py | 41 ++++++++++++++++- core/repair_jobs/orphan_file_detector.py | 41 +++++++++++------ tests/test_orphan_file_detector.py | 58 ++++++++++++++++++++++++ tests/test_stale_guard.py | 27 +++++++++++ 4 files changed, 151 insertions(+), 16 deletions(-) diff --git a/core/library/stale_guard.py b/core/library/stale_guard.py index c614934c..0d40ee68 100644 --- a/core/library/stale_guard.py +++ b/core/library/stale_guard.py @@ -42,4 +42,43 @@ def is_implausible_stale_removal( return missing_count > total_count * max_fraction -__all__ = ["is_implausible_stale_removal", "DEFAULT_MIN_TOTAL", "DEFAULT_MAX_MISSING_FRACTION"] +# The orphan detector walks the transfer folder and flags any audio file whose +# path/title doesn't resolve to a DB track. If the DB's stored paths share a base +# prefix the local filesystem no longer has (remount, Docker volume change, WSL +# hiccup), EVERY file misses and the whole library looks "orphaned" — and a user +# batch-applying "move to staging" on those findings would relocate their entire +# library. Same failure mode as stale-removal, so we skip the whole result when +# the orphan share is implausibly large. Needs an absolute floor too: 3/4 orphans +# in a tiny folder is normal, 4000/5000 is a path mismatch. +DEFAULT_MIN_ORPHANS = 20 +DEFAULT_MAX_ORPHAN_FRACTION = 0.5 + + +def is_implausible_orphan_flood( + orphan_count: int, + total_count: int, + *, + min_orphans: int = DEFAULT_MIN_ORPHANS, + max_fraction: float = DEFAULT_MAX_ORPHAN_FRACTION, +) -> bool: + """True when so many files look orphaned that the DB↔filesystem path mapping is + almost certainly broken (not real orphans) and the scan should create NO + findings — otherwise a batch "move to staging" / "delete" could wipe the + library. Below ``min_orphans`` (absolute) it always returns False so small, + genuine orphan sets still surface. + """ + if total_count <= 0 or orphan_count <= 0: + return False + if orphan_count <= min_orphans: + return False + return orphan_count > total_count * max_fraction + + +__all__ = [ + "is_implausible_stale_removal", + "is_implausible_orphan_flood", + "DEFAULT_MIN_TOTAL", + "DEFAULT_MAX_MISSING_FRACTION", + "DEFAULT_MIN_ORPHANS", + "DEFAULT_MAX_ORPHAN_FRACTION", +] diff --git a/core/repair_jobs/orphan_file_detector.py b/core/repair_jobs/orphan_file_detector.py index 5c6e69cd..1b2b5b5f 100644 --- a/core/repair_jobs/orphan_file_detector.py +++ b/core/repair_jobs/orphan_file_detector.py @@ -218,17 +218,33 @@ class OrphanFileDetectorJob(RepairJob): if context.update_progress and (i + 1) % 50 == 0: context.update_progress(i + 1, total) - # Safety check: if most files look like orphans, it's probably a path - # mismatch between the DB and filesystem — not actual orphans. - orphan_ratio = len(orphan_files) / total if total else 0 - mass_orphan = orphan_ratio > 0.5 and len(orphan_files) > 20 - - if mass_orphan: + # Safety: if most files look like orphans, it's almost certainly a path + # mismatch between the DB and filesystem (remount / Docker volume change), + # NOT real orphans. Creating findings anyway is dangerous — a user batch- + # applying "move to staging" / "delete" on them would relocate or wipe the + # whole library. So we create NO findings here, the same hard skip the + # stale-removal paths use. Fix the path mismatch and real orphans surface. + from core.library.stale_guard import is_implausible_orphan_flood + if is_implausible_orphan_flood(len(orphan_files), total): + pct = (len(orphan_files) / total * 100) if total else 0 logger.warning( - "Mass orphan warning: %d of %d files (%.0f%%) flagged as orphans — " - "this likely indicates a DB path mismatch, not actual orphans", - len(orphan_files), total, orphan_ratio * 100 + "Mass orphan guard: %d of %d files (%.0f%%) flagged as orphans — " + "almost certainly a DB↔filesystem path mismatch, not real orphans. " + "Creating no findings so a batch move/delete can't wipe the library.", + len(orphan_files), total, pct, ) + if context.report_progress: + context.report_progress( + log_line=(f'Skipped: {len(orphan_files)} of {total} files look ' + 'orphaned — likely a DB path mismatch, not real orphans. ' + 'No findings created.'), + log_type='skip', + ) + if context.update_progress: + context.update_progress(total, total) + logger.info("Orphan file scan: %d files scanned, mass-orphan guard tripped " + "(0 findings)", result.scanned) + return result for fpath in orphan_files: if context.report_progress: @@ -243,16 +259,12 @@ class OrphanFileDetectorJob(RepairJob): inserted = context.create_finding( job_id=self.job_id, finding_type='orphan_file', - severity='warning' if mass_orphan else 'info', + severity='info', entity_type='file', entity_id=None, file_path=fpath, title=f'Orphan file: {os.path.basename(fpath)}', description=( - 'Audio file in transfer folder is not tracked in the database. ' - 'WARNING: Mass orphan detection triggered — this may be a path ' - 'mismatch, not actual orphans. Verify before deleting!' - ) if mass_orphan else ( 'Audio file in transfer folder is not tracked in the database' ), details={ @@ -261,7 +273,6 @@ class OrphanFileDetectorJob(RepairJob): 'modified': time.strftime('%Y-%m-%d %H:%M:%S', time.localtime(stat.st_mtime)), 'folder': os.path.dirname(fpath), - 'mass_orphan': mass_orphan, } ) if inserted: diff --git a/tests/test_orphan_file_detector.py b/tests/test_orphan_file_detector.py index 02ba7150..2d691bb6 100644 --- a/tests/test_orphan_file_detector.py +++ b/tests/test_orphan_file_detector.py @@ -52,6 +52,64 @@ def _seed_library(db_path: Path) -> None: conn.close() +def test_mass_orphan_path_mismatch_creates_no_findings(tmp_path: Path) -> None: + """The "transferred to staging" footgun: when the DB's stored paths no longer + match the filesystem (remount / Docker volume change) EVERY file looks + orphaned. The detector must create NO findings then — otherwise a user + batch-applying "move to staging" relocates their whole library. Mirrors the + hard skip the stale-removal paths use. + """ + db_path = tmp_path / "library.sqlite" + _seed_library(db_path) # DB tracks live under /old/prefix/... — nothing on disk matches + + # Drop 30 untracked files (> the 20 absolute floor, and 100% > 50%). + music = tmp_path / "Some Artist" / "Some Album" + music.mkdir(parents=True) + for i in range(30): + (music / f"{i:02d} - Track {i}.mp3").write_bytes(b"unreadable tags; no DB match") + + findings = [] + context = JobContext( + db=_DB(db_path), + transfer_folder=str(tmp_path), + config_manager=None, + create_finding=lambda **kwargs: findings.append(kwargs) or True, + ) + + result = OrphanFileDetectorJob().scan(context) + + assert result.scanned == 30 + assert result.findings_created == 0 + assert findings == [] # hard skip — not even flagged as warnings + + +def test_small_orphan_set_still_surfaces(tmp_path: Path) -> None: + """Below the absolute floor, genuine orphans must still be reported — the + guard only suppresses an implausibly large flood, not normal stray files. + """ + db_path = tmp_path / "library.sqlite" + _seed_library(db_path) + + music = tmp_path / "Stray" / "Files" + music.mkdir(parents=True) + for i in range(3): # 3 orphans — under the 20-file floor + (music / f"{i:02d} - Stray {i}.mp3").write_bytes(b"no DB match") + + findings = [] + context = JobContext( + db=_DB(db_path), + transfer_folder=str(tmp_path), + config_manager=None, + create_finding=lambda **kwargs: findings.append(kwargs) or True, + ) + + result = OrphanFileDetectorJob().scan(context) + + assert result.scanned == 3 + assert result.findings_created == 3 + assert all(f['finding_type'] == 'orphan_file' for f in findings) + + def test_orphan_detector_accepts_picard_albumartist_folder_match(tmp_path: Path) -> None: """Picard paths use albumartist/album (year)/track - title. diff --git a/tests/test_stale_guard.py b/tests/test_stale_guard.py index 6259847e..68db9f0a 100644 --- a/tests/test_stale_guard.py +++ b/tests/test_stale_guard.py @@ -2,6 +2,7 @@ from __future__ import annotations +from core.library.stale_guard import is_implausible_orphan_flood as flood from core.library.stale_guard import is_implausible_stale_removal as g @@ -26,3 +27,29 @@ def test_edge_inputs(): assert g(0, 0) is False assert g(0, 100) is False # nothing missing assert g(5, 5) is True # min_total met, all missing + + +# ── orphan-flood guard: same shape, protects the "move to staging" path ────── + +def test_whole_library_flagged_orphan_is_blocked(): + # 4000/5000 files "orphaned" → a path mismatch, not real orphans. + assert flood(4000, 5000) is True + assert flood(21, 40) is True # just over both floors (>20 and >50%) + + +def test_a_handful_of_real_orphans_still_surface(): + assert flood(3, 4000) is False # a few stray files — report them + assert flood(20, 30) is False # at the absolute floor (not > 20) + assert flood(2000, 4000) is False # exactly 50% is NOT over the threshold + + +def test_orphan_flood_small_folders_never_blocked(): + # A 5-file folder that's all orphans is plausible (manual drop) — don't hide it. + assert flood(5, 5) is False + assert flood(20, 20) is False # below the absolute orphan floor + + +def test_orphan_flood_edge_inputs(): + assert flood(0, 0) is False + assert flood(0, 5000) is False # nothing orphaned + assert flood(5000, 0) is False # nonsense totals don't trip it From 550fca0fe5ef6e76c3ecc0cbff8437131430564a Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Fri, 12 Jun 2026 13:28:24 -0700 Subject: [PATCH 18/64] webui: sync organize-by-playlist toggles + stop dashboard poller 401-spam while locked MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - The download-modal 'Organize by Playlist' toggle had no onchange, so flipping it never saved or synced the saved per-playlist preference. Add the handler (source auto-derived from the ref) so both controls read/write the one organize_by_playlist value — manual action persists, the other reflects it. - loadDashboardSyncHistory polled /api/sync/history every 30s even while the launch-PIN/login gate was active, 401-spamming the log. Skip when locked, and on a 401 (stale session after a restart) surface the unlock screen so it self-heals instead of spamming. --- webui/static/pages-extra.js | 17 +++++++++++++++++ webui/static/shared-helpers.js | 4 +++- 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/webui/static/pages-extra.js b/webui/static/pages-extra.js index 08790589..ecb246ae 100644 --- a/webui/static/pages-extra.js +++ b/webui/static/pages-extra.js @@ -1916,11 +1916,28 @@ setInterval(() => { }, 30000); async function loadDashboardSyncHistory() { + // Don't poll the auth-gated sync-history endpoint while the app is locked — + // it would 401 every 30s cycle (the result is discarded anyway). Resumes + // automatically on unlock (init.js removes 'app-locked'). + if (document.body.classList.contains('app-locked')) return; const container = document.getElementById('sync-history-cards'); if (!container) return; try { const response = await fetch('/api/sync/history?limit=10'); + if (response.status === 401) { + // Session lapsed (e.g. the server restarted) while this tab still + // believed it was unlocked, so the guard above couldn't fire. Surface + // the correct unlock screen — both add 'app-locked', which stops the + // poll until the user re-authenticates (same as a fresh page load). + const info = await response.json().catch(() => ({})); + if (info.login_required && typeof showLoginScreen === 'function') { + showLoginScreen(); + } else if (typeof showLaunchPinScreen === 'function') { + showLaunchPinScreen(); + } + return; + } if (!response.ok) return; const data = await response.json(); diff --git a/webui/static/shared-helpers.js b/webui/static/shared-helpers.js index 620fec26..9c2866aa 100644 --- a/webui/static/shared-helpers.js +++ b/webui/static/shared-helpers.js @@ -1007,9 +1007,11 @@ function normalizePlaylistOrganizeRef(playlistRef, source = 'spotify') { } function downloadMissingModalOrganizeCheckboxHtml(playlistId) { + const safeId = String(playlistId).replace(/'/g, "\\'"); return ` `; } From 37431ea82b28365eddfd7c56c4555772c708d783 Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Fri, 12 Jun 2026 13:28:26 -0700 Subject: [PATCH 19/64] Downloads: additively surface each track's real file path (analysis + completed) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The download analysis already matches every track to a library row via check_track_exists / manual match, then discarded the result. Keep it: each analysis_results entry now carries matched_file_path + matched_track_id (the owned file's real location, or None). Symmetrically, a completed download task now records final_file_path (where the import landed). Purely additive, no behavior change, no new matching, zero perf cost — just stops throwing away what the pipeline already computed. This is the foundation for playlist materialization: owned + downloaded tracks both report where their real file is, so the folder can be built by name match, not source IDs. --- core/downloads/master.py | 27 +++++++++++++++++++++++---- core/imports/pipeline.py | 5 +++++ 2 files changed, 28 insertions(+), 4 deletions(-) diff --git a/core/downloads/master.py b/core/downloads/master.py index ce30b8d5..7a00ebf5 100644 --- a/core/downloads/master.py +++ b/core/downloads/master.py @@ -505,13 +505,19 @@ def run_full_missing_tracks_process(batch_id, playlist_id, tracks_json, deps: Ma track_name = track_data.get('name', '') artists = track_data.get('artists', []) found, confidence = False, 0.0 + # Additive payload: the owned library track (DatabaseTrack) when this + # item is found in the library, so downstream (playlist materialization) + # knows WHERE the real file is without re-matching. None when not owned. + matched_track = None # Manual library matches are authoritative unless the user explicitly # requested a force re-download from the normal download modal. _stid = track_data.get('spotify_track_id') or track_data.get('source_track_id') or track_data.get('id', '') - if not ignore_manual_matches and _stid and _mlm.get_match_for_track( - db, batch_profile_id, track_data, default_source=batch_source - ): + _manual_match = ( + _mlm.get_match_for_track(db, batch_profile_id, track_data, default_source=batch_source) + if (not ignore_manual_matches and _stid) else None + ) + if _manual_match: logger.info(f"[Manual Match] '{track_name}' already matched in library — skipping download") try: deps.check_and_remove_track_from_wishlist_by_metadata(track_data) @@ -523,6 +529,8 @@ def run_full_missing_tracks_process(batch_id, playlist_id, tracks_json, deps: Ma 'found': True, 'confidence': 1.0, 'match_reason': 'manual_library_match', + 'matched_file_path': _manual_match.get('library_file_path'), + 'matched_track_id': _manual_match.get('library_track_id'), }) continue @@ -568,8 +576,10 @@ def run_full_missing_tracks_process(batch_id, playlist_id, tracks_json, deps: Ma # Direct title match (try both raw and normalized) if track_name_lower in album_tracks_map: found, confidence = True, 1.0 + matched_track = album_tracks_map[track_name_lower] elif _normalized_source_title and _normalized_source_title in album_tracks_map: found, confidence = True, 1.0 + matched_track = album_tracks_map[_normalized_source_title] else: # Fuzzy match against album tracks using string similarity. # Compare BOTH the raw and normalized source titles — @@ -577,14 +587,17 @@ def run_full_missing_tracks_process(batch_id, playlist_id, tracks_json, deps: Ma # matching when the album doesn't imply version # context (helper returns the input unchanged). best_sim = 0.0 + best_track = None for db_title_lower, _db_track in album_tracks_map.items(): sim_raw = db._string_similarity(track_name_lower, db_title_lower) sim_norm = db._string_similarity(_normalized_source_title, db_title_lower) if _normalized_source_title else 0.0 sim = max(sim_raw, sim_norm) if sim > best_sim: best_sim = sim + best_track = _db_track if best_sim >= 0.7: found, confidence = True, best_sim + matched_track = best_track else: # Fall back to global per-track search for this track # When allow_duplicates is on for album downloads, skip global @@ -605,6 +618,7 @@ def run_full_missing_tracks_process(batch_id, playlist_id, tracks_json, deps: Ma ) if db_track and track_confidence >= 0.7: found, confidence = True, track_confidence + matched_track = db_track break elif allow_duplicates and batch_is_album: # Allow duplicates + album download + album not in DB yet → treat all as missing @@ -624,10 +638,15 @@ def run_full_missing_tracks_process(batch_id, playlist_id, tracks_json, deps: Ma ) if db_track and track_confidence >= 0.7: found, confidence = True, track_confidence + matched_track = db_track break analysis_results.append({ - 'track_index': track_index, 'track': track_data, 'found': found, 'confidence': confidence + 'track_index': track_index, 'track': track_data, 'found': found, 'confidence': confidence, + # Additive: real on-disk location of the owned track (None when not + # owned), so playlist materialization links the right file. + 'matched_file_path': getattr(matched_track, 'file_path', None), + 'matched_track_id': getattr(matched_track, 'id', None), }) # WISHLIST REMOVAL: If track is found in database, check if it should be removed from wishlist diff --git a/core/imports/pipeline.py b/core/imports/pipeline.py index ec027f06..22117377 100644 --- a/core/imports/pipeline.py +++ b/core/imports/pipeline.py @@ -695,6 +695,7 @@ def post_process_matched_download(context_key, context, file_path, runtime, meta if task_id in download_tasks: download_tasks[task_id]['stream_processed'] = True download_tasks[task_id]['status'] = 'completed' + download_tasks[task_id]['final_file_path'] = context.get('_final_processed_path') logger.info(f"[Playlist Folder Mode] Marked task {task_id} as completed") _notify_download_completed(batch_id, task_id, success=True) return @@ -1026,6 +1027,10 @@ def post_process_matched_download(context_key, context, file_path, runtime, meta if task_id in download_tasks: download_tasks[task_id]['stream_processed'] = True download_tasks[task_id]['status'] = 'completed' + # Additive: record where the imported file landed so downstream + # (playlist materialization) knows the real path of a freshly + # downloaded track without re-resolving it. + download_tasks[task_id]['final_file_path'] = context.get('_final_processed_path') logger.info(f"[Post-Process] Marked task {task_id} as completed") _notify_download_completed(batch_id, task_id, success=True) From 3a6cb8cda50060c39f5f37ba06e476a97ab57bcd Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Fri, 12 Jun 2026 13:30:48 -0700 Subject: [PATCH 20/64] Playlists: config (separate root + symlink/copy) + pure materializer seam MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - settings: playlists.materialize_path (separate root, mapped apart from the music library so the media server never double-scans it) + materialize_mode (symlink|copy). - core/playlists/materialize.py: pure filesystem engine that (re)builds a playlist folder of relative symlinks (or copies) into the real library — idempotent, prunes stale entries, disambiguates filename collisions, never escapes the root, and auto-falls-back to copy when the FS can't symlink. No DB, no app state; ops injectable. 13 unit tests. Isolated + additive — nothing live calls this yet (stitcher/trigger/routing come next). --- config/settings.py | 11 ++ core/playlists/materialize.py | 214 +++++++++++++++++++++++++++++ tests/test_playlist_materialize.py | 143 +++++++++++++++++++ 3 files changed, 368 insertions(+) create mode 100644 core/playlists/materialize.py create mode 100644 tests/test_playlist_materialize.py diff --git a/config/settings.py b/config/settings.py index a522d4a7..ff232ad6 100644 --- a/config/settings.py +++ b/config/settings.py @@ -696,6 +696,17 @@ class ConfigManager: "enabled": False, "entry_base_path": "" }, + "playlists": { + # Where "Organize by playlist" materializes playlist folders. + # MUST be a separate root from the music library so the media + # server (and the maintenance jobs) never scan it — otherwise the + # same track would show up twice. Mapped separately for Docker. + "materialize_path": "./Playlists", + # "symlink" (relative links, ~zero disk) or "copy" (real + # duplicates for FAT/USB/DAPs that can't follow links). Symlink + # auto-falls back to copy when the filesystem can't link. + "materialize_mode": "symlink" + }, "youtube": { "cookies_browser": "", # "", "chrome", "firefox", "edge", "brave", "opera", "safari" "download_delay": 3, # seconds between sequential downloads diff --git a/core/playlists/materialize.py b/core/playlists/materialize.py new file mode 100644 index 00000000..f48a9001 --- /dev/null +++ b/core/playlists/materialize.py @@ -0,0 +1,214 @@ +"""Materialize a playlist as a folder of links into the real music library. + +A playlist folder is a **view**, not storage. Every entry points at the one real +file that already lives in the library (``Artist/Album/track.ext``); a track is +never stored twice no matter how many playlists it's in. Two modes: + +- ``symlink`` (default): a *relative* symlink to the real file — ~zero disk, and + relative so the tree stays valid if the parent folder is moved. +- ``copy``: a real duplicate of the file — for filesystems/players that can't + follow symlinks (FAT USB sticks, some DAPs) or when a self-contained, + portable folder is wanted. + +Symlinks silently fail or are unsupported on a lot of real setups (Windows +without the privilege, SMB/CIFS shares, FAT/exFAT). So when a symlink can't be +created we **fall back to a copy automatically** — the folder is always fully +populated, never left with dangling links. + +This module is pure filesystem mechanics: no DB, no app state. Given a list of +real file paths, a playlists root, a playlist name and a mode, it (re)builds the +folder to match. That makes it the single unit-tested source of truth for "how a +playlist folder looks on disk", and means the folder is a *derived view* that can +be rebuilt from scratch at any time. Filesystem ops are injectable so the +behaviour — including the symlink→copy fallback — is testable without depending +on the host filesystem's symlink support. +""" + +from __future__ import annotations + +import os +import shutil +from dataclasses import dataclass, field +from typing import Callable, List, Optional, Sequence + +from core.imports.paths import sanitize_filename + +MATERIALIZE_MODES = ("symlink", "copy") +DEFAULT_MODE = "symlink" + + +def normalize_mode(mode: Optional[str]) -> str: + """Coerce a config value to a valid mode (``symlink`` default).""" + m = (mode or "").strip().lower() + return m if m in MATERIALIZE_MODES else DEFAULT_MODE + + +@dataclass +class RebuildSummary: + """Outcome of rebuilding one playlist folder. ``copied`` may be non-zero even + in symlink mode when the fallback kicked in (flagged by ``fellback``).""" + playlist_dir: str = "" + linked: int = 0 + copied: int = 0 + unchanged: int = 0 + removed_stale: int = 0 + missing_source: int = 0 + failed: int = 0 + fellback: bool = False + mode_requested: str = DEFAULT_MODE + errors: List[str] = field(default_factory=list) + + +def playlist_dir_for(playlists_root: str, playlist_name: str) -> str: + """Absolute path of one playlist's folder, sanitized and guaranteed to stay + directly under ``playlists_root`` (defends against ``..`` / separators in a + playlist name).""" + root = os.path.abspath(playlists_root) + safe_name = sanitize_filename(playlist_name or "").strip() or "Unnamed Playlist" + candidate = os.path.abspath(os.path.join(root, safe_name)) + if os.path.dirname(candidate) != root: + safe_name = sanitize_filename(os.path.basename(candidate)) or "Unnamed Playlist" + candidate = os.path.join(root, safe_name) + return candidate + + +def _desired_entries(playlist_dir: str, real_paths: Sequence[str]) -> "list[tuple[str, str]]": + """Map each real file to a flat destination inside ``playlist_dir``, preserving + the source filename. On a basename collision between two *different* sources, + disambiguate with a numeric suffix rather than silently overwriting.""" + entries: list[tuple[str, str]] = [] + used: dict[str, str] = {} # dest basename -> source real path + for real in real_paths: + if not real: + continue + base = os.path.basename(real) + name = base + stem, ext = os.path.splitext(base) + counter = 1 + while name in used and used[name] != os.path.abspath(real): + counter += 1 + name = f"{stem} ({counter}){ext}" + used[name] = os.path.abspath(real) + entries.append((os.path.abspath(real), os.path.join(playlist_dir, name))) + return entries + + +def _symlink_is_current(dest: str, rel_target: str) -> bool: + try: + return os.path.islink(dest) and os.readlink(dest) == rel_target + except OSError: + return False + + +def _remove_entry(path: str) -> None: + """Remove an existing file/symlink at ``path`` (incl. a broken symlink).""" + if os.path.islink(path) or os.path.exists(path): + try: + os.remove(path) + except OSError: + pass + + +def materialize_one( + real_path: str, + dest_path: str, + mode: str = DEFAULT_MODE, + *, + symlink_fn: Callable[[str, str], None] = os.symlink, + copy_fn: Callable[[str, str], object] = shutil.copy2, +) -> str: + """Create one playlist entry at ``dest_path`` pointing at ``real_path``. + + Idempotent: a correct existing entry is left alone. In ``symlink`` mode a + relative link is used; if it can't be created (unsupported FS, no privilege) + it falls back to a copy so the entry is never left broken. Returns one of: + ``'linked'``, ``'copied'``, ``'unchanged'``, ``'fellback'`` (symlink + requested but copied), ``'missing'`` (source gone).""" + if not real_path or not os.path.exists(real_path): + return "missing" + + dest_dir = os.path.dirname(dest_path) + os.makedirs(dest_dir, exist_ok=True) + rel_target = os.path.relpath(os.path.abspath(real_path), start=dest_dir) + + if mode == "copy": + if os.path.isfile(dest_path) and not os.path.islink(dest_path): + return "unchanged" + _remove_entry(dest_path) + copy_fn(real_path, dest_path) + return "copied" + + # symlink mode + if _symlink_is_current(dest_path, rel_target): + return "unchanged" + _remove_entry(dest_path) + try: + symlink_fn(rel_target, dest_path) + return "linked" + except (OSError, NotImplementedError): + copy_fn(real_path, dest_path) + return "fellback" + + +def rebuild_playlist_folder( + playlists_root: str, + playlist_name: str, + real_paths: Sequence[str], + mode: str = DEFAULT_MODE, + *, + prune_stale: bool = True, + symlink_fn: Callable[[str, str], None] = os.symlink, + copy_fn: Callable[[str, str], object] = shutil.copy2, +) -> RebuildSummary: + """(Re)build ``playlists_root//`` so it contains exactly one + entry per real file in ``real_paths`` — adding missing entries, leaving correct + ones untouched, and (when ``prune_stale``) removing entries no longer present. + Idempotent and safe to re-run any time. Filesystem ops are injectable.""" + mode = normalize_mode(mode) + pdir = playlist_dir_for(playlists_root, playlist_name) + summary = RebuildSummary(playlist_dir=pdir, mode_requested=mode) + os.makedirs(pdir, exist_ok=True) + + entries = _desired_entries(pdir, real_paths) + keep = {dest for _real, dest in entries} + + for real, dest in entries: + try: + outcome = materialize_one(real, dest, mode, symlink_fn=symlink_fn, copy_fn=copy_fn) + except OSError as e: + summary.failed += 1 + summary.errors.append(f"{os.path.basename(dest)}: {e}") + continue + if outcome == "linked": + summary.linked += 1 + elif outcome == "copied": + summary.copied += 1 + elif outcome == "fellback": + summary.copied += 1 + summary.fellback = True + elif outcome == "unchanged": + summary.unchanged += 1 + elif outcome == "missing": + summary.missing_source += 1 + + if prune_stale and os.path.isdir(pdir): + for name in os.listdir(pdir): + full = os.path.join(pdir, name) + if full in keep: + continue + if os.path.islink(full) or os.path.isfile(full): + _remove_entry(full) + summary.removed_stale += 1 + + return summary + + +__all__ = [ + "MATERIALIZE_MODES", + "DEFAULT_MODE", + "normalize_mode", + "RebuildSummary", + "playlist_dir_for", + "materialize_one", + "rebuild_playlist_folder", +] diff --git a/tests/test_playlist_materialize.py b/tests/test_playlist_materialize.py new file mode 100644 index 00000000..daf397e8 --- /dev/null +++ b/tests/test_playlist_materialize.py @@ -0,0 +1,143 @@ +"""Playlist materialization seam — a playlist folder is a derived view of links +into the real library. Locks down: symlink vs copy, the auto-fallback when +symlinks aren't supported, idempotency, stale-link pruning, collision handling, +and that nothing is ever written outside the playlists root.""" + +from __future__ import annotations + +import os +from pathlib import Path + +from core.playlists.materialize import ( + DEFAULT_MODE, + materialize_one, + normalize_mode, + playlist_dir_for, + rebuild_playlist_folder, +) + + +def _library(tmp_path: Path) -> list[str]: + a = tmp_path / "Music" / "Daft Punk" / "Discovery" / "One More Time.mp3" + b = tmp_path / "Music" / "Queen" / "A Night at the Opera" / "Bohemian Rhapsody.mp3" + for f in (a, b): + f.parent.mkdir(parents=True, exist_ok=True) + f.write_bytes(b"audio") + return [str(a), str(b)] + + +def test_normalize_mode(): + assert normalize_mode("symlink") == "symlink" + assert normalize_mode("COPY") == "copy" + assert normalize_mode("") == DEFAULT_MODE + assert normalize_mode(None) == DEFAULT_MODE + assert normalize_mode("nonsense") == DEFAULT_MODE + + +def test_symlink_mode_creates_relative_links(tmp_path: Path): + real = _library(tmp_path) + s = rebuild_playlist_folder(str(tmp_path / "Playlists"), "Road Trip", real, mode="symlink") + assert s.linked == 2 and s.copied == 0 and not s.fellback + link = Path(s.playlist_dir) / "One More Time.mp3" + assert link.is_symlink() + assert not os.path.isabs(os.readlink(link)) # relative for portability + assert link.resolve() == Path(real[0]).resolve() + assert link.read_bytes() == b"audio" + + +def test_symlink_mode_idempotent(tmp_path: Path): + real = _library(tmp_path) + root = str(tmp_path / "Playlists") + rebuild_playlist_folder(root, "Mix", real, mode="symlink") + s2 = rebuild_playlist_folder(root, "Mix", real, mode="symlink") + assert s2.unchanged == 2 and s2.linked == 0 and s2.removed_stale == 0 + + +def test_copy_mode_duplicates_real_files(tmp_path: Path): + real = _library(tmp_path) + s = rebuild_playlist_folder(str(tmp_path / "Playlists"), "USB", real, mode="copy") + assert s.copied == 2 and s.linked == 0 + f = Path(s.playlist_dir) / "Bohemian Rhapsody.mp3" + assert f.is_file() and not f.is_symlink() and f.read_bytes() == b"audio" + + +def test_copy_mode_idempotent(tmp_path: Path): + real = _library(tmp_path) + root = str(tmp_path / "Playlists") + rebuild_playlist_folder(root, "Mix", real, mode="copy") + s2 = rebuild_playlist_folder(root, "Mix", real, mode="copy") + assert s2.unchanged == 2 and s2.copied == 0 + + +def test_falls_back_to_copy_when_symlinks_unsupported(tmp_path: Path): + real = _library(tmp_path) + + def _no_symlinks(target, link): + raise OSError("symlinks not supported here") + + s = rebuild_playlist_folder(str(tmp_path / "Playlists"), "Car", real, + mode="symlink", symlink_fn=_no_symlinks) + assert s.fellback is True and s.copied == 2 and s.linked == 0 + f = Path(s.playlist_dir) / "One More Time.mp3" + assert f.is_file() and not f.is_symlink() and f.read_bytes() == b"audio" + + +def test_rebuild_prunes_entries_no_longer_in_playlist(tmp_path: Path): + real = _library(tmp_path) + root = str(tmp_path / "Playlists") + rebuild_playlist_folder(root, "Mix", real, mode="symlink") # 2 entries + s = rebuild_playlist_folder(root, "Mix", real[:1], mode="symlink") # drop one + assert s.removed_stale == 1 + pdir = Path(s.playlist_dir) + assert (pdir / "One More Time.mp3").exists() + assert not (pdir / "Bohemian Rhapsody.mp3").exists() + + +def test_prune_stale_can_be_disabled(tmp_path: Path): + real = _library(tmp_path) + root = str(tmp_path / "Playlists") + rebuild_playlist_folder(root, "Mix", real, mode="symlink") + s = rebuild_playlist_folder(root, "Mix", real[:1], mode="symlink", prune_stale=False) + assert s.removed_stale == 0 + assert (Path(s.playlist_dir) / "Bohemian Rhapsody.mp3").exists() + + +def test_switching_mode_replaces_links_with_copies(tmp_path: Path): + real = _library(tmp_path) + root = str(tmp_path / "Playlists") + rebuild_playlist_folder(root, "Mix", real, mode="symlink") + s = rebuild_playlist_folder(root, "Mix", real, mode="copy") + f = Path(s.playlist_dir) / "One More Time.mp3" + assert f.is_file() and not f.is_symlink() and s.copied == 2 + + +def test_basename_collision_is_disambiguated_not_overwritten(tmp_path: Path): + p1 = tmp_path / "Music" / "A" / "AlbumA" / "01 - Intro.mp3" + p2 = tmp_path / "Music" / "B" / "AlbumB" / "01 - Intro.mp3" + for f, data in ((p1, b"one"), (p2, b"two")): + f.parent.mkdir(parents=True, exist_ok=True) + f.write_bytes(data) + s = rebuild_playlist_folder(str(tmp_path / "Playlists"), "Dup", [str(p1), str(p2)], mode="copy") + pdir = Path(s.playlist_dir) + assert (pdir / "01 - Intro.mp3").read_bytes() == b"one" + assert (pdir / "01 - Intro (2).mp3").read_bytes() == b"two" # second kept, not lost + assert s.copied == 2 + + +def test_missing_source_is_counted_not_fatal(tmp_path: Path): + real = _library(tmp_path) + s = rebuild_playlist_folder(str(tmp_path / "Playlists"), "Mix", + real + [str(tmp_path / "gone.mp3")], mode="symlink") + assert s.linked == 2 and s.missing_source == 1 + + +def test_playlist_name_cannot_escape_root(tmp_path: Path): + root = tmp_path / "Playlists" + nasty = playlist_dir_for(str(root), "../../etc/evil") + assert os.path.abspath(nasty).startswith(os.path.abspath(str(root)) + os.sep) + + +def test_materialize_one_missing_source(tmp_path: Path): + dest = tmp_path / "Playlists" / "X" / "x.mp3" + assert materialize_one(str(tmp_path / "nope.mp3"), str(dest), "symlink") == "missing" + assert not dest.exists() From bef73d855d5a0336b60fa2fa8cd5eb4d1b4b6195 Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Fri, 12 Jun 2026 13:32:53 -0700 Subject: [PATCH 21/64] Playlists: stitch a batch's owned+downloaded paths into the materializer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit materialize_playlist_from_batch(batch, download_tasks, config) collects the real on-disk path of every resolved track from the batch's OWN payload — owned via analysis_results.matched_file_path, downloaded via tasks.final_file_path — runs each through the playback path resolver (Docker-correct), de-dupes, and hands the list to rebuild_playlist_folder. Gated on playlist_folder_mode. No re-matching, no source IDs, no mirrored-playlist lookup — works for any organize-by-playlist download including the all-owned case. 5 tests. Still isolated; the triggers wire it in next. --- core/playlists/materialize_service.py | 77 +++++++++++++++ tests/test_playlist_materialize_service.py | 103 +++++++++++++++++++++ 2 files changed, 180 insertions(+) create mode 100644 core/playlists/materialize_service.py create mode 100644 tests/test_playlist_materialize_service.py diff --git a/core/playlists/materialize_service.py b/core/playlists/materialize_service.py new file mode 100644 index 00000000..0f6cc891 --- /dev/null +++ b/core/playlists/materialize_service.py @@ -0,0 +1,77 @@ +"""Build a playlist's materialized folder from a FINISHED organize-by-playlist +download batch. + +The batch already carries the real on-disk location of every resolved track: + + - **owned** tracks → ``analysis_results[*]['matched_file_path']`` (the library + matcher's result, captured during analysis), and + - **downloaded** tracks → ``download_tasks[tid]['final_file_path']`` (where the + import landed). + +So this is pure stitching + filesystem work: **no re-matching, no source-ID +lookup, no mirrored-playlist resolution**. It works for any organize-by-playlist +download, mirrored or not, and for the all-owned case (where nothing downloads). + +Each stored path is run through ``resolve_library_file_path`` — the same resolver +playback uses — so a DB path that's host-formatted (Docker) maps to the real file +the container can see; a freshly-downloaded path already exists and passes +through unchanged. +""" + +from __future__ import annotations + +from typing import Any, List, Optional + +from core.imports.paths import docker_resolve_path +from core.playlists.materialize import ( + RebuildSummary, + normalize_mode, + rebuild_playlist_folder, +) + + +def collect_batch_real_paths(batch: dict, download_tasks: dict, *, config_manager) -> List[str]: + """Real on-disk paths of every track in a finished batch that resolved — + owned (from analysis) + downloaded (from completed tasks) — resolved to this + process's filesystem and de-duplicated, in playlist order then download order.""" + from core.library.path_resolver import resolve_library_file_path + + out: List[str] = [] + seen = set() + + def _add(stored_path: Any) -> None: + if not stored_path: + return + real = resolve_library_file_path(str(stored_path), config_manager=config_manager) + if real and real not in seen: + seen.add(real) + out.append(real) + + for res in (batch.get("analysis_results") or []): + if res.get("found"): + _add(res.get("matched_file_path")) + + for tid in (batch.get("queue") or []): + task = (download_tasks or {}).get(tid) or {} + if task.get("status") == "completed": + _add(task.get("final_file_path")) + + return out + + +def materialize_playlist_from_batch(batch: dict, download_tasks: dict, config_manager) -> Optional[RebuildSummary]: + """(Re)build the playlist folder for a finished organize-by-playlist batch. + + Returns the :class:`RebuildSummary`, or ``None`` when the batch isn't an + organize-by-playlist batch. Reads ``playlists.materialize_path`` / + ``playlists.materialize_mode`` from config.""" + if not batch or not batch.get("playlist_folder_mode"): + return None + name = batch.get("playlist_name") or "Unknown Playlist" + real_paths = collect_batch_real_paths(batch, download_tasks, config_manager=config_manager) + root = docker_resolve_path(config_manager.get("playlists.materialize_path", "./Playlists")) + mode = normalize_mode(config_manager.get("playlists.materialize_mode", "symlink")) + return rebuild_playlist_folder(root, name, real_paths, mode) + + +__all__ = ["collect_batch_real_paths", "materialize_playlist_from_batch"] diff --git a/tests/test_playlist_materialize_service.py b/tests/test_playlist_materialize_service.py new file mode 100644 index 00000000..e61d999a --- /dev/null +++ b/tests/test_playlist_materialize_service.py @@ -0,0 +1,103 @@ +"""Playlist materialize SERVICE — builds the folder from a finished batch's own +payload (owned matched_file_path + downloaded final_file_path). Locks down: it +stitches owned + downloaded, ignores not-found/not-completed, de-dupes, gates on +the organize flag, and never depends on source IDs or a mirrored playlist.""" + +from __future__ import annotations + +from pathlib import Path + +from core.playlists.materialize_service import ( + collect_batch_real_paths, + materialize_playlist_from_batch, +) + + +class _Cfg: + def __init__(self, root, mode="symlink"): + self._d = {"playlists.materialize_path": root, "playlists.materialize_mode": mode} + + def get(self, key, default=None): + return self._d.get(key, default) + + +def _mk(tmp_path: Path, *names) -> list[str]: + paths = [] + for n in names: + f = tmp_path / "Music" / n + f.parent.mkdir(parents=True, exist_ok=True) + f.write_bytes(b"audio") + paths.append(str(f)) + return paths + + +def test_collect_stitches_owned_and_downloaded(tmp_path: Path): + owned, downloaded = _mk(tmp_path, "Owned.mp3"), _mk(tmp_path, "Fresh.mp3") + batch = { + "analysis_results": [ + {"found": True, "matched_file_path": owned[0]}, + {"found": False, "matched_file_path": None}, # not owned → skip + ], + "queue": ["t1", "t2"], + } + tasks = { + "t1": {"status": "completed", "final_file_path": downloaded[0]}, + "t2": {"status": "failed", "final_file_path": None}, # not completed → skip + } + cfg = _Cfg(str(tmp_path / "Playlists")) + paths = collect_batch_real_paths(batch, tasks, config_manager=cfg) + assert paths == [owned[0], downloaded[0]] # owned first, then downloaded + + +def test_collect_dedupes(tmp_path: Path): + owned = _mk(tmp_path, "Same.mp3") + batch = { + "analysis_results": [{"found": True, "matched_file_path": owned[0]}], + "queue": ["t1"], + } + tasks = {"t1": {"status": "completed", "final_file_path": owned[0]}} # same file + cfg = _Cfg(str(tmp_path / "Playlists")) + assert collect_batch_real_paths(batch, tasks, config_manager=cfg) == [owned[0]] + + +def test_materialize_from_batch_all_owned(tmp_path: Path): + """The all-owned case (no downloads) — folder built entirely from analysis.""" + owned = _mk(tmp_path, "A.mp3", "B.mp3") + batch = { + "playlist_folder_mode": True, + "playlist_name": "Smack That", + "analysis_results": [ + {"found": True, "matched_file_path": owned[0]}, + {"found": True, "matched_file_path": owned[1]}, + ], + "queue": [], + } + cfg = _Cfg(str(tmp_path / "Playlists")) + summary = materialize_playlist_from_batch(batch, {}, cfg) + assert summary is not None and summary.linked == 2 + pdir = Path(summary.playlist_dir) + assert pdir == tmp_path / "Playlists" / "Smack That" + assert (pdir / "A.mp3").resolve() == Path(owned[0]).resolve() + assert (pdir / "B.mp3").resolve() == Path(owned[1]).resolve() + + +def test_materialize_from_batch_owned_plus_downloaded(tmp_path: Path): + owned, downloaded = _mk(tmp_path, "Owned.mp3"), _mk(tmp_path, "Fresh.mp3") + batch = { + "playlist_folder_mode": True, + "playlist_name": "Mix", + "analysis_results": [{"found": True, "matched_file_path": owned[0]}], + "queue": ["t1"], + } + tasks = {"t1": {"status": "completed", "final_file_path": downloaded[0]}} + cfg = _Cfg(str(tmp_path / "Playlists"), mode="copy") + summary = materialize_playlist_from_batch(batch, tasks, cfg) + assert summary.copied == 2 + pdir = Path(summary.playlist_dir) + assert (pdir / "Owned.mp3").is_file() and (pdir / "Fresh.mp3").is_file() + + +def test_materialize_skipped_when_not_organize(tmp_path: Path): + batch = {"playlist_folder_mode": False, "playlist_name": "X", "analysis_results": [], "queue": []} + assert materialize_playlist_from_batch(batch, {}, _Cfg(str(tmp_path / "Playlists"))) is None + assert not (tmp_path / "Playlists").exists() From aa5d747327798bfb867c51cf672c3267cdd8ad20 Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Fri, 12 Jun 2026 13:37:59 -0700 Subject: [PATCH 22/64] Playlists: wire materialize triggers + retire per-track routing flag MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Routing (step 5): organize-by-playlist tracks no longer set the per-track _playlist_folder_mode flag, so they import NORMALLY into Artist/Album — exactly what a normal download does. _playlist_name provenance is kept (origin.py). - Triggers (step 4): build the playlist folder from the batch's own payload at both end-of-flow points — the all-owned path in master.py (no downloads, so the lifecycle never runs) and the batch-complete hook in lifecycle.py (after downloads). Both gated on playlist_folder_mode, both non-fatal. Works for the all-owned case (the smack test that did nothing before) and for mixed owned/downloaded, with no source-ID or mirrored-playlist dependency. The materialized folder uses the default ./Playlists root + symlink mode until the Settings UI is added. Updated the master test to assert the new contract (provenance kept, routing flag gone). 979 tests pass. --- core/downloads/lifecycle.py | 18 ++++++++++++++ core/downloads/master.py | 31 +++++++++++++++++++++--- tests/downloads/test_downloads_master.py | 12 ++++++--- 3 files changed, 54 insertions(+), 7 deletions(-) diff --git a/core/downloads/lifecycle.py b/core/downloads/lifecycle.py index adec519e..a267f291 100644 --- a/core/downloads/lifecycle.py +++ b/core/downloads/lifecycle.py @@ -549,6 +549,24 @@ def on_download_completed(batch_id: str, task_id: str, success: bool, deps: Life except Exception as m3u_err: logger.error(f"[M3U] Error regenerating M3U on batch complete: {m3u_err}") + # PLAYLIST MATERIALIZE: if this was an organize-by-playlist batch, + # (re)build the playlist's folder of links/copies now that every + # track is imported. Built from the batch's own payload (owned + + # downloaded real paths) — non-fatal, derived view. (materialize.py) + if batch.get('playlist_folder_mode'): + try: + from core.playlists.materialize_service import materialize_playlist_from_batch + _mat = materialize_playlist_from_batch(batch, download_tasks, deps.config_manager) + if _mat: + logger.info( + f"[Playlist Folder] Rebuilt '{_mat.playlist_dir}': " + f"{_mat.linked} linked, {_mat.copied} copied, " + f"{_mat.unchanged} unchanged, {_mat.removed_stale} stale removed" + + (" (symlinks unsupported here → copied)" if _mat.fellback else "") + ) + except Exception as _mat_err: + logger.error(f"[Playlist Folder] Materialize failed (non-fatal): {_mat_err}") + # REPAIR: Scan all album folders from this batch for track number issues if deps.repair_worker: deps.repair_worker.process_batch(batch_id) diff --git a/core/downloads/master.py b/core/downloads/master.py index 7a00ebf5..de136b85 100644 --- a/core/downloads/master.py +++ b/core/downloads/master.py @@ -785,6 +785,25 @@ def run_full_missing_tracks_process(batch_id, playlist_id, tracks_json, deps: Ma logger.warning("[Auto-Wishlist] No missing tracks found - calling auto-completion handler to toggle cycle and reschedule") deps.missing_download_executor.submit(deps.process_failed_tracks_to_wishlist_exact_with_auto_completion, batch_id) + # Organize-by-playlist with NOTHING to download (every track already + # owned): the batch never enters the download/lifecycle path, so build + # the playlist folder here from the owned files the analysis matched. + # Gated + non-fatal; runs once after analysis, not in the per-track loop. + if effective_playlist_folder_mode: + try: + from core.playlists.materialize_service import materialize_playlist_from_batch + _batch = download_batches.get(batch_id) + if _batch is not None: + _mat = materialize_playlist_from_batch(_batch, download_tasks, deps.config_manager) + if _mat: + logger.info( + f"[Playlist Folder] Rebuilt '{_mat.playlist_dir}' (all owned): " + f"{_mat.linked} linked, {_mat.copied} copied, {_mat.removed_stale} stale removed" + + (" (symlinks unsupported here → copied)" if _mat.fellback else "") + ) + except Exception as _mat_err: + logger.error(f"[Playlist Folder] All-owned materialize failed (non-fatal): {_mat_err}") + return logger.warning(f" transitioning batch {batch_id} to download phase with {len(missing_tracks)} tracks.") @@ -1163,7 +1182,13 @@ def run_full_missing_tracks_process(batch_id, playlist_id, tracks_json, deps: Ma task_pl_folder_mode = True task_pl_name = wl_pl_name or wl_mirrored.get('name') or batch_playlist_name if task_pl_folder_mode: - track_info['_playlist_folder_mode'] = True + # Organize-by-playlist now imports each track NORMALLY into the + # Artist/Album library (i.e. exactly what a normal download does) + # — the playlist folder is built as links/copies AFTER the batch + # from the real library files. So we deliberately DON'T set + # `_playlist_folder_mode` (which routed the real file into a flat + # Music// dump). We keep `_playlist_name` + source_info + # — they're download provenance (core/downloads/origin.py). track_info['_playlist_name'] = task_pl_name if batch_source_playlist_ref: track_info['source_info'] = { @@ -1172,8 +1197,8 @@ def run_full_missing_tracks_process(batch_id, playlist_id, tracks_json, deps: Ma 'source': batch_source, } logger.info( - f"[Task Creation] Added playlist folder mode for: " - f"{track_info.get('name')} → {task_pl_name}" + f"[Task Creation] Organize-by-playlist (normal import + " + f"materialize after batch): {track_info.get('name')} → {task_pl_name}" ) else: logger.debug( diff --git a/tests/downloads/test_downloads_master.py b/tests/downloads/test_downloads_master.py index c2c5b758..b425e4dc 100644 --- a/tests/downloads/test_downloads_master.py +++ b/tests/downloads/test_downloads_master.py @@ -1048,8 +1048,12 @@ def test_wishlist_album_grouping_uses_shared_rich_album_context(monkeypatch): assert images == {'http://img'} -def test_playlist_folder_mode_propagates(monkeypatch): - """Playlist folder mode flag carried through to track_info.""" +def test_organize_by_playlist_keeps_provenance_not_routing(monkeypatch): + """Organize-by-playlist no longer routes the file per-track. Each track imports + NORMALLY into Artist/Album (what a normal download does); the playlist folder + is built as links AFTER the batch. So the old routing flag + `_playlist_folder_mode` is NOT set, but `_playlist_name` provenance IS kept + (core/downloads/origin.py).""" db = _FakeDB() monkeypatch.setattr('database.music_database.MusicDatabase', lambda: db) @@ -1062,8 +1066,8 @@ def test_playlist_folder_mode_propagates(monkeypatch): task_id = download_batches['B15']['queue'][0] info = download_tasks[task_id]['track_info'] - assert info['_playlist_folder_mode'] is True - assert info['_playlist_name'] == 'My Mix' + assert '_playlist_folder_mode' not in info # routing retired → normal import + assert info['_playlist_name'] == 'My Mix' # provenance preserved # --------------------------------------------------------------------------- From 87621b7191686310e73d863ce1c9a85d92230174 Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Fri, 12 Jun 2026 14:04:34 -0700 Subject: [PATCH 23/64] Playlists: Settings UI (path + symlink/copy + rebuild button) + rebuild endpoint MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Settings: 'Playlists Folder' path field (Unlock pattern, separate-root help text), a Symlinks/Copies selector, and a 'Rebuild playlist folders now' button (standard test-button style). Wired through PATH_INPUT_IDS / load / save, plus 'playlists' added to the settings save allowlist so it persists. - POST /api/playlists/materialize/rebuild → rebuild_organized_playlists_from_db: rebuilds every organize-by-playlist folder from CURRENT ownership, re-matching each track with check_track_exists (name, not IDs) so it self-heals after a reorganize / membership change. +1 test. 70 materialize tests + JS integrity pass; settings round-trip wiring verified. --- core/playlists/materialize_service.py | 44 +++++++++++++++++++- tests/test_playlist_materialize_service.py | 47 ++++++++++++++++++++++ web_server.py | 26 +++++++++++- webui/index.html | 26 ++++++++++++ webui/static/settings.js | 46 +++++++++++++++++++++ 5 files changed, 187 insertions(+), 2 deletions(-) diff --git a/core/playlists/materialize_service.py b/core/playlists/materialize_service.py index 0f6cc891..9be1e2b6 100644 --- a/core/playlists/materialize_service.py +++ b/core/playlists/materialize_service.py @@ -74,4 +74,46 @@ def materialize_playlist_from_batch(batch: dict, download_tasks: dict, config_ma return rebuild_playlist_folder(root, name, real_paths, mode) -__all__ = ["collect_batch_real_paths", "materialize_playlist_from_batch"] +def rebuild_organized_playlists_from_db(db, config_manager, *, profile_id: int = 1): + """Rebuild every "organize by playlist" folder from CURRENT library ownership — + for the manual "Rebuild" button. Unlike the post-download path (which uses the + batch's payload), there's no batch here, so each playlist's owned files are + re-derived via the app's own matcher — ``check_track_exists`` (by name+artist), + NOT source IDs — then resolved to disk and handed to the materializer. This + self-heals the folders after a library reorganize moves files or membership + changes. Returns a list of ``(playlist_name, RebuildSummary)``.""" + from core.library.path_resolver import resolve_library_file_path + + root = docker_resolve_path(config_manager.get("playlists.materialize_path", "./Playlists")) + mode = normalize_mode(config_manager.get("playlists.materialize_mode", "symlink")) + results = [] + for pl in (db.get_mirrored_playlists(profile_id) or []): + if not pl.get("organize_by_playlist"): + continue + real_paths: List[str] = [] + seen = set() + for t in (db.get_mirrored_playlist_tracks(pl["id"]) or []): + title = (t.get("track_name") or "").strip() + artist = (t.get("artist_name") or "").strip() + if not title: + continue + try: + db_track, conf = db.check_track_exists(title, artist, confidence_threshold=0.7) + except Exception: + continue + if db_track is None or conf < 0.7: + continue + real = resolve_library_file_path(getattr(db_track, "file_path", None), config_manager=config_manager) + if real and real not in seen: + seen.add(real) + real_paths.append(real) + name = pl.get("name") or "Unnamed Playlist" + results.append((name, rebuild_playlist_folder(root, name, real_paths, mode))) + return results + + +__all__ = [ + "collect_batch_real_paths", + "materialize_playlist_from_batch", + "rebuild_organized_playlists_from_db", +] diff --git a/tests/test_playlist_materialize_service.py b/tests/test_playlist_materialize_service.py index e61d999a..36c8372d 100644 --- a/tests/test_playlist_materialize_service.py +++ b/tests/test_playlist_materialize_service.py @@ -10,9 +10,40 @@ from pathlib import Path from core.playlists.materialize_service import ( collect_batch_real_paths, materialize_playlist_from_batch, + rebuild_organized_playlists_from_db, ) +class _Track: + """Mimics database.DatabaseTrack — what check_track_exists returns.""" + def __init__(self, file_path): + self.file_path = file_path + + +class _RebuildDB: + """One organized playlist (Mix) + one not (Off); check_track_exists matches + by NAME via `owned` (track_name -> file_path), so no source IDs involved.""" + def __init__(self, owned): + self.owned = owned + + def get_mirrored_playlists(self, profile_id=1): + return [ + {"id": 1, "name": "Mix", "organize_by_playlist": True}, + {"id": 2, "name": "Off", "organize_by_playlist": False}, + ] + + def get_mirrored_playlist_tracks(self, pid): + if pid == 1: + return [{"track_name": "A", "artist_name": "x"}, + {"track_name": "Gone", "artist_name": "y"}] # not owned + return [{"track_name": "B", "artist_name": "z"}] + + def check_track_exists(self, title, artist, confidence_threshold=0.8, + server_source=None, album=None, candidate_tracks=None): + fp = self.owned.get(title) + return (_Track(fp), 1.0) if fp else (None, 0.0) + + class _Cfg: def __init__(self, root, mode="symlink"): self._d = {"playlists.materialize_path": root, "playlists.materialize_mode": mode} @@ -101,3 +132,19 @@ def test_materialize_skipped_when_not_organize(tmp_path: Path): batch = {"playlist_folder_mode": False, "playlist_name": "X", "analysis_results": [], "queue": []} assert materialize_playlist_from_batch(batch, {}, _Cfg(str(tmp_path / "Playlists"))) is None assert not (tmp_path / "Playlists").exists() + + +def test_rebuild_from_db_only_organized_and_owned(tmp_path: Path): + """The manual button: rebuild every organized playlist by re-matching with + check_track_exists (name), linking only owned tracks.""" + f = tmp_path / "Music" / "A.mp3" + f.parent.mkdir(parents=True, exist_ok=True) + f.write_bytes(b"audio") + db = _RebuildDB({"A": str(f)}) # 'Gone' + the 'Off' playlist's 'B' not owned + cfg = _Cfg(str(tmp_path / "Playlists")) + results = rebuild_organized_playlists_from_db(db, cfg, profile_id=1) + assert len(results) == 1 # only Mix (organize on) + name, s = results[0] + assert name == "Mix" and s.linked == 1 # only A owned; Gone skipped + assert (tmp_path / "Playlists" / "Mix" / "A.mp3").exists() + assert not (tmp_path / "Playlists" / "Off").exists() diff --git a/web_server.py b/web_server.py index a731e95f..ea8f6d4e 100644 --- a/web_server.py +++ b/web_server.py @@ -3135,7 +3135,7 @@ def handle_settings(): if 'active_media_server' in new_settings: config_manager.set_active_media_server(new_settings['active_media_server']) - for service in ['spotify', 'plex', 'jellyfin', 'navidrome', 'soulseek', 'download_source', 'settings', 'database', 'metadata_enhancement', 'file_organization', 'playlist_sync', 'tidal', 'tidal_download', 'qobuz', 'hifi_download', 'deezer_download', 'amazon_download', 'lidarr_download', 'prowlarr', 'torrent_client', 'usenet_client', 'listenbrainz', 'acoustid', 'lastfm', 'genius', 'import', 'lossy_copy', 'listening_stats', 'ui_appearance', 'youtube', 'content_filter', 'itunes', 'm3u_export', 'musicbrainz', 'deezer', 'audiodb', 'metadata', 'hydrabase', 'security', 'discogs', 'library', 'discover', 'wishlist', 'genre_whitelist', 'post_processing']: + for service in ['spotify', 'plex', 'jellyfin', 'navidrome', 'soulseek', 'download_source', 'settings', 'database', 'metadata_enhancement', 'file_organization', 'playlist_sync', 'tidal', 'tidal_download', 'qobuz', 'hifi_download', 'deezer_download', 'amazon_download', 'lidarr_download', 'prowlarr', 'torrent_client', 'usenet_client', 'listenbrainz', 'acoustid', 'lastfm', 'genius', 'import', 'lossy_copy', 'listening_stats', 'ui_appearance', 'youtube', 'content_filter', 'itunes', 'm3u_export', 'musicbrainz', 'deezer', 'audiodb', 'metadata', 'hydrabase', 'security', 'discogs', 'library', 'discover', 'wishlist', 'genre_whitelist', 'post_processing', 'playlists']: if service in new_settings: for key, value in new_settings[service].items(): config_manager.set(f'{service}.{key}', value) @@ -3818,6 +3818,30 @@ def get_mirrored_playlists_list(): except Exception as e: return jsonify({"playlists": [], "spotify_authenticated": False}), 200 +@app.route('/api/playlists/materialize/rebuild', methods=['POST']) +def rebuild_playlist_materialization_endpoint(): + """(Re)build every "organize by playlist" folder from current library ownership + (the manual Settings button). Safe to call any time — it's a derived view and + only links tracks the user actually owns.""" + try: + from core.playlists.materialize_service import rebuild_organized_playlists_from_db + database = get_database() + profile_id = get_current_profile_id() + results = rebuild_organized_playlists_from_db(database, config_manager, profile_id=profile_id) + return jsonify({ + 'success': True, + 'count': len(results), + 'results': [{ + 'playlist': name, 'folder': s.playlist_dir, + 'linked': s.linked, 'copied': s.copied, 'unchanged': s.unchanged, + 'removed_stale': s.removed_stale, 'missing_source': s.missing_source, + 'failed': s.failed, 'fellback': s.fellback, + } for name, s in results], + }) + except Exception as e: + logger.error(f"[Playlist Materialize] Rebuild endpoint failed: {e}") + return jsonify({'success': False, 'error': str(e)}), 500 + @app.route('/api/setup/status', methods=['GET']) def setup_status_endpoint(): """Check if first-run setup has been completed.""" diff --git a/webui/index.html b/webui/index.html index bbedad3f..dca19e71 100644 --- a/webui/index.html +++ b/webui/index.html @@ -4549,6 +4549,32 @@
+
+ +
+ + +
+
+ Where Organize by playlist builds playlist folders — as shortcuts into your library, so songs are never stored twice. Keep this outside your music library folder so your media server doesn't scan the same tracks twice. +
+
+ +
+ + +
+ Symlinks use almost no space. Copies are self-contained. Symlinks fall back to copies automatically where the filesystem can't link (Windows without privileges, some network shares, FAT/exFAT drives). +
+ +
+
+
diff --git a/webui/static/settings.js b/webui/static/settings.js index 6e699dd5..8ba12156 100644 --- a/webui/static/settings.js +++ b/webui/static/settings.js @@ -1129,6 +1129,8 @@ async function loadSettingsData() { document.getElementById('transfer-path').value = settings.soulseek?.transfer_path || './Transfer'; document.getElementById('staging-path').value = settings.import?.staging_path || './Staging'; document.getElementById('music-videos-path').value = settings.library?.music_videos_path || './MusicVideos'; + document.getElementById('playlists-materialize-path').value = settings.playlists?.materialize_path || './Playlists'; + document.getElementById('playlists-materialize-mode').value = settings.playlists?.materialize_mode || 'symlink'; // Populate Download Source settings document.getElementById('download-source-mode').value = settings.download_source?.mode || 'soulseek'; @@ -3133,6 +3135,10 @@ async function saveSettings(quiet = false) { folder_artist_override: document.getElementById('import-folder-artist-override')?.checked === true, staging_path: document.getElementById('staging-path').value || './Staging' }, + playlists: { + materialize_path: document.getElementById('playlists-materialize-path').value || './Playlists', + materialize_mode: document.getElementById('playlists-materialize-mode').value || 'symlink' + }, lossy_copy: { enabled: document.getElementById('lossy-copy-enabled').checked, codec: document.getElementById('lossy-copy-codec').value, @@ -4628,6 +4634,7 @@ const PATH_INPUT_IDS = { transfer: 'transfer-path', staging: 'staging-path', 'music-videos': 'music-videos-path', + 'playlists-materialize': 'playlists-materialize-path', 'm3u-entry-base': 'm3u-entry-base-path' }; @@ -4647,5 +4654,44 @@ function togglePathLock(pathType, btn) { } } +// Manually (re)build every "organize by playlist" folder from current library +// ownership — mirrors the automatic rebuild that runs after a playlist download. +async function rebuildPlaylistFolders() { + const btn = document.getElementById('playlists-rebuild-btn'); + const status = document.getElementById('playlists-rebuild-status'); + if (!btn) return; + const original = btn.textContent; + btn.disabled = true; + btn.textContent = 'Rebuilding…'; + if (status) { status.style.color = ''; status.textContent = 'Rebuilding playlist folders…'; } + try { + const res = await fetch('/api/playlists/materialize/rebuild', { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: '{}' + }); + const data = await res.json(); + if (!res.ok || !data.success) throw new Error(data.error || 'Rebuild failed'); + const n = data.count || 0; + let linked = 0, copied = 0, removed = 0; + (data.results || []).forEach(r => { + linked += r.linked || 0; copied += r.copied || 0; removed += r.removed_stale || 0; + }); + if (status) { + status.style.color = '#4caf50'; + status.textContent = (n === 0) + ? 'No "organize by playlist" playlists to rebuild yet.' + : `Rebuilt ${n} playlist folder${n === 1 ? '' : 's'} — ${linked} linked, ${copied} copied, ${removed} stale removed.`; + } + if (typeof showToast === 'function') showToast('Playlist folders rebuilt', 'success'); + } catch (e) { + if (status) { status.style.color = '#f44336'; status.textContent = 'Rebuild failed: ' + (e.message || e); } + if (typeof showToast === 'function') showToast('Playlist rebuild failed', 'error'); + } finally { + btn.disabled = false; + btn.textContent = original; + } +} + // =============================== From 997e76b6b488baf50d86ae70b03533bf8e8c795e Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Fri, 12 Jun 2026 15:30:47 -0700 Subject: [PATCH 24/64] Playlists: one path-independent reconcile after every batch (closes wishlist gap) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replaces the two organize-only triggers with a single reconcile_batch_playlists called at both batch-completion points. It groups the batch's newly-resolved tracks by their per-track playlist provenance: - the batch's OWN organize playlist → full (re)build with prune, and - a track that completed for a DIFFERENT playlist (e.g. a WISHLIST fulfilling a track that belongs to an organize playlist) → ADDED to that folder, no prune. So a late wishlist arrival now lands in its playlist folder immediately, instead of only on the next sync/manual rebuild — the folder = the playlist's owned members, kept true on every ownership change regardless of download path. Uses the paths the batch already captured (no DB re-match, no waiting on the server scan/sync). Non-fatal. 3 new reconcile tests (organize full-rebuild, wishlist add-without-prune, plain batch no-op). 983 downloads/imports/playlist tests pass. --- core/downloads/lifecycle.py | 33 ++++++------- core/downloads/master.py | 5 +- core/playlists/materialize_service.py | 53 ++++++++++++++++++++ tests/test_playlist_materialize_service.py | 57 ++++++++++++++++++++++ 4 files changed, 128 insertions(+), 20 deletions(-) diff --git a/core/downloads/lifecycle.py b/core/downloads/lifecycle.py index a267f291..68054ccc 100644 --- a/core/downloads/lifecycle.py +++ b/core/downloads/lifecycle.py @@ -549,23 +549,22 @@ def on_download_completed(batch_id: str, task_id: str, success: bool, deps: Life except Exception as m3u_err: logger.error(f"[M3U] Error regenerating M3U on batch complete: {m3u_err}") - # PLAYLIST MATERIALIZE: if this was an organize-by-playlist batch, - # (re)build the playlist's folder of links/copies now that every - # track is imported. Built from the batch's own payload (owned + - # downloaded real paths) — non-fatal, derived view. (materialize.py) - if batch.get('playlist_folder_mode'): - try: - from core.playlists.materialize_service import materialize_playlist_from_batch - _mat = materialize_playlist_from_batch(batch, download_tasks, deps.config_manager) - if _mat: - logger.info( - f"[Playlist Folder] Rebuilt '{_mat.playlist_dir}': " - f"{_mat.linked} linked, {_mat.copied} copied, " - f"{_mat.unchanged} unchanged, {_mat.removed_stale} stale removed" - + (" (symlinks unsupported here → copied)" if _mat.fellback else "") - ) - except Exception as _mat_err: - logger.error(f"[Playlist Folder] Materialize failed (non-fatal): {_mat_err}") + # PLAYLIST MATERIALIZE: one path-independent reconcile — drop this + # batch's newly-resolved tracks into the right Playlists// + # folders. Covers an organize-by-playlist download AND a late + # wishlist arrival (via each track's playlist provenance). Built + # from the batch's own captured paths — non-fatal, derived view. + try: + from core.playlists.materialize_service import reconcile_batch_playlists + for _pl_name, _mat in reconcile_batch_playlists(batch, download_tasks, deps.config_manager): + logger.info( + f"[Playlist Folder] Rebuilt '{_mat.playlist_dir}': " + f"{_mat.linked} linked, {_mat.copied} copied, " + f"{_mat.unchanged} unchanged, {_mat.removed_stale} stale removed" + + (" (symlinks unsupported here → copied)" if _mat.fellback else "") + ) + except Exception as _mat_err: + logger.error(f"[Playlist Folder] Materialize failed (non-fatal): {_mat_err}") # REPAIR: Scan all album folders from this batch for track number issues if deps.repair_worker: diff --git a/core/downloads/master.py b/core/downloads/master.py index de136b85..b18b9b00 100644 --- a/core/downloads/master.py +++ b/core/downloads/master.py @@ -791,11 +791,10 @@ def run_full_missing_tracks_process(batch_id, playlist_id, tracks_json, deps: Ma # Gated + non-fatal; runs once after analysis, not in the per-track loop. if effective_playlist_folder_mode: try: - from core.playlists.materialize_service import materialize_playlist_from_batch + from core.playlists.materialize_service import reconcile_batch_playlists _batch = download_batches.get(batch_id) if _batch is not None: - _mat = materialize_playlist_from_batch(_batch, download_tasks, deps.config_manager) - if _mat: + for _pl_name, _mat in reconcile_batch_playlists(_batch, download_tasks, deps.config_manager): logger.info( f"[Playlist Folder] Rebuilt '{_mat.playlist_dir}' (all owned): " f"{_mat.linked} linked, {_mat.copied} copied, {_mat.removed_stale} stale removed" diff --git a/core/playlists/materialize_service.py b/core/playlists/materialize_service.py index 9be1e2b6..89759cbf 100644 --- a/core/playlists/materialize_service.py +++ b/core/playlists/materialize_service.py @@ -74,6 +74,58 @@ def materialize_playlist_from_batch(batch: dict, download_tasks: dict, config_ma return rebuild_playlist_folder(root, name, real_paths, mode) +def reconcile_batch_playlists(batch: dict, download_tasks: dict, config_manager): + """One post-batch step: drop the batch's newly-resolved tracks into the right + ``Playlists//`` folders. Path-independent — covers an organize-by-playlist + download AND a late wishlist arrival — using the per-track playlist provenance + each download already carries, plus the file paths the batch already captured + (so it needs no DB re-match and no waiting on the server scan/sync). + + - The batch's OWN organize playlist is fully rebuilt from the batch payload + (owned + downloaded), pruning tracks no longer present. + - A track that completed for a DIFFERENT playlist (e.g. a wishlist + fulfillment) is ADDED to that playlist's folder — no prune, since this + batch isn't that playlist's full membership (removals are handled by a + full rebuild on the next sync / the manual button). + + Returns a list of ``(playlist_name, RebuildSummary)``. Callers wrap non-fatally.""" + from core.library.path_resolver import resolve_library_file_path + + results = [] + root = docker_resolve_path(config_manager.get("playlists.materialize_path", "./Playlists")) + mode = normalize_mode(config_manager.get("playlists.materialize_mode", "symlink")) + batch_name = batch.get("playlist_name") if batch.get("playlist_folder_mode") else None + + # 1. The batch's own organize playlist → full materialize (prunes removed). + if batch_name: + summary = materialize_playlist_from_batch(batch, download_tasks, config_manager) + if summary: + results.append((batch_name, summary)) + + # 2. Tracks that completed for OTHER playlists (e.g. wishlist fulfilling a + # track that belongs to an organize playlist) → add-only into that folder. + extra: dict = {} + for tid in (batch.get("queue") or []): + task = (download_tasks or {}).get(tid) or {} + if task.get("status") != "completed" or not task.get("final_file_path"): + continue + pname = (task.get("track_info") or {}).get("_playlist_name") + if pname and pname != batch_name: + extra.setdefault(pname, []).append(task["final_file_path"]) + + for name, stored in extra.items(): + real, seen = [], set() + for p in stored: + r = resolve_library_file_path(str(p), config_manager=config_manager) + if r and r not in seen: + seen.add(r) + real.append(r) + if real: + results.append((name, rebuild_playlist_folder(root, name, real, mode, prune_stale=False))) + + return results + + def rebuild_organized_playlists_from_db(db, config_manager, *, profile_id: int = 1): """Rebuild every "organize by playlist" folder from CURRENT library ownership — for the manual "Rebuild" button. Unlike the post-download path (which uses the @@ -115,5 +167,6 @@ def rebuild_organized_playlists_from_db(db, config_manager, *, profile_id: int = __all__ = [ "collect_batch_real_paths", "materialize_playlist_from_batch", + "reconcile_batch_playlists", "rebuild_organized_playlists_from_db", ] diff --git a/tests/test_playlist_materialize_service.py b/tests/test_playlist_materialize_service.py index 36c8372d..4c810045 100644 --- a/tests/test_playlist_materialize_service.py +++ b/tests/test_playlist_materialize_service.py @@ -11,6 +11,7 @@ from core.playlists.materialize_service import ( collect_batch_real_paths, materialize_playlist_from_batch, rebuild_organized_playlists_from_db, + reconcile_batch_playlists, ) @@ -134,6 +135,62 @@ def test_materialize_skipped_when_not_organize(tmp_path: Path): assert not (tmp_path / "Playlists").exists() +def test_reconcile_organize_batch_full_rebuild(tmp_path: Path): + """An organize-by-playlist batch → its own folder is fully (re)built from the + batch payload.""" + owned, fresh = _mk(tmp_path, "Owned.mp3"), _mk(tmp_path, "Fresh.mp3") + batch = { + "playlist_folder_mode": True, + "playlist_name": "Mix", + "analysis_results": [{"found": True, "matched_file_path": owned[0]}], + "queue": ["t1"], + } + tasks = {"t1": {"status": "completed", "final_file_path": fresh[0], + "track_info": {"_playlist_name": "Mix"}}} # same playlist as batch + cfg = _Cfg(str(tmp_path / "Playlists")) + results = reconcile_batch_playlists(batch, tasks, cfg) + assert len(results) == 1 # only Mix (no double-add) + name, s = results[0] + assert name == "Mix" and s.linked == 2 + pdir = Path(s.playlist_dir) + assert (pdir / "Owned.mp3").exists() and (pdir / "Fresh.mp3").exists() + + +def test_reconcile_wishlist_adds_to_other_playlist_without_pruning(tmp_path: Path): + """The wishlist gap: a wishlist batch (not organize) downloads a track that + belongs to an organize playlist → it's ADDED to that playlist's folder, and + the folder's existing entries are NOT pruned.""" + # Pre-seed Smack That with an existing symlink (a track from a prior batch). + existing = _mk(tmp_path, "Existing.mp3")[0] + from core.playlists.materialize import rebuild_playlist_folder + rebuild_playlist_folder(str(tmp_path / "Playlists"), "Smack That", [existing], "symlink") + + # A WISHLIST batch (playlist_folder_mode False) completes a track tagged for + # the "Smack That" organize playlist. + late = _mk(tmp_path, "Late.mp3")[0] + batch = {"playlist_folder_mode": False, "playlist_name": "wishlist", + "analysis_results": [], "queue": ["w1"]} + tasks = {"w1": {"status": "completed", "final_file_path": late, + "track_info": {"_playlist_name": "Smack That"}}} + cfg = _Cfg(str(tmp_path / "Playlists")) + + results = reconcile_batch_playlists(batch, tasks, cfg) + assert len(results) == 1 and results[0][0] == "Smack That" + pdir = tmp_path / "Playlists" / "Smack That" + assert (pdir / "Late.mp3").exists() # the wishlist track was ADDED + assert (pdir / "Existing.mp3").exists() # and the prior entry was NOT pruned + + +def test_reconcile_noop_for_plain_batch(tmp_path: Path): + """A normal (non-organize, no provenance) batch → nothing happens.""" + f = _mk(tmp_path, "X.mp3")[0] + batch = {"playlist_folder_mode": False, "playlist_name": "album", + "analysis_results": [], "queue": ["a1"]} + tasks = {"a1": {"status": "completed", "final_file_path": f, "track_info": {}}} + assert reconcile_batch_playlists(batch, tasks, _Cfg(str(tmp_path / "Playlists"))) == [] + assert not (tmp_path / "Playlists").exists() + + def test_rebuild_from_db_only_organized_and_owned(tmp_path: Path): """The manual button: rebuild every organized playlist by re-matching with check_track_exists (name), linking only owned tracks.""" From d160c486ec6dbd02f06a87662dacc5f877f27296 Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Fri, 12 Jun 2026 15:49:13 -0700 Subject: [PATCH 25/64] Playlists: mirror-update trigger prunes removed tracks (the other half) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Symmetric to the post-download reconcile (which handles ADDITIONS): when a playlist's membership is re-synced (the mirror step — scheduled refresh or the manual mirror endpoint), rebuild its folder from current membership WITH prune IF it's organize-by-playlist. So a track that just LEFT the playlist has its symlink cleaned up the instant membership changes, not only on the next download. Factored a shared _rebuild_one_from_db (used by the manual 'Rebuild' button and the mirror hook) + rebuild_mirrored_playlist_if_organized. Gated to organized playlists, non-fatal at both mirror call sites. Now the invariant 'folder = the playlist's current owned members' holds on every change: additions caught at download, removals caught at mirror. 2 new tests (removed track pruned; non-organized skipped). 985 + 277 tests pass. --- core/automation/handlers/refresh_mirrored.py | 12 +++ core/playlists/materialize_service.py | 82 ++++++++++++-------- tests/test_playlist_materialize_service.py | 46 +++++++++++ web_server.py | 9 +++ 4 files changed, 118 insertions(+), 31 deletions(-) diff --git a/core/automation/handlers/refresh_mirrored.py b/core/automation/handlers/refresh_mirrored.py index 323b1205..0f1de31e 100644 --- a/core/automation/handlers/refresh_mirrored.py +++ b/core/automation/handlers/refresh_mirrored.py @@ -292,6 +292,18 @@ def _commit_refresh( image_url=pl.get('image_url'), ) + # Membership just changed — if this playlist is organize-by-playlist, rebuild + # its folder (with prune) so a track that LEFT the playlist has its symlink + # cleaned up now. Gated to organized playlists, non-fatal — never disturbs + # the refresh. (Additions are handled by the post-download reconcile.) + try: + from core.playlists.materialize_service import rebuild_mirrored_playlist_if_organized + rebuild_mirrored_playlist_if_organized( + db, deps.config_manager, pl.get('id'), profile_id=pl.get('profile_id', 1) + ) + except Exception as _mat_err: + deps.logger.debug(f"[Playlist Folder] mirror-refresh cleanup skipped: {_mat_err}") + if old_ids != new_ids: added = len(new_ids - old_ids) removed = len(old_ids - new_ids) diff --git a/core/playlists/materialize_service.py b/core/playlists/materialize_service.py index 89759cbf..bcc2c905 100644 --- a/core/playlists/materialize_service.py +++ b/core/playlists/materialize_service.py @@ -126,42 +126,61 @@ def reconcile_batch_playlists(batch: dict, download_tasks: dict, config_manager) return results -def rebuild_organized_playlists_from_db(db, config_manager, *, profile_id: int = 1): - """Rebuild every "organize by playlist" folder from CURRENT library ownership — - for the manual "Rebuild" button. Unlike the post-download path (which uses the - batch's payload), there's no batch here, so each playlist's owned files are - re-derived via the app's own matcher — ``check_track_exists`` (by name+artist), - NOT source IDs — then resolved to disk and handed to the materializer. This - self-heals the folders after a library reorganize moves files or membership - changes. Returns a list of ``(playlist_name, RebuildSummary)``.""" +def _rebuild_one_from_db(db, config_manager, playlist: dict): + """Rebuild ONE playlist's folder from its CURRENT membership × ownership — + re-matching each member via the app's own ``check_track_exists`` (by name, not + source IDs), resolving to disk, and rebuilding WITH prune. Because it's driven + by current membership, a track that has LEFT the playlist drops out of the set + and its symlink is pruned. Returns ``(playlist_name, RebuildSummary)``.""" from core.library.path_resolver import resolve_library_file_path root = docker_resolve_path(config_manager.get("playlists.materialize_path", "./Playlists")) mode = normalize_mode(config_manager.get("playlists.materialize_mode", "symlink")) - results = [] - for pl in (db.get_mirrored_playlists(profile_id) or []): - if not pl.get("organize_by_playlist"): + real_paths: List[str] = [] + seen = set() + for t in (db.get_mirrored_playlist_tracks(playlist["id"]) or []): + title = (t.get("track_name") or "").strip() + artist = (t.get("artist_name") or "").strip() + if not title: continue - real_paths: List[str] = [] - seen = set() - for t in (db.get_mirrored_playlist_tracks(pl["id"]) or []): - title = (t.get("track_name") or "").strip() - artist = (t.get("artist_name") or "").strip() - if not title: - continue - try: - db_track, conf = db.check_track_exists(title, artist, confidence_threshold=0.7) - except Exception: - continue - if db_track is None or conf < 0.7: - continue - real = resolve_library_file_path(getattr(db_track, "file_path", None), config_manager=config_manager) - if real and real not in seen: - seen.add(real) - real_paths.append(real) - name = pl.get("name") or "Unnamed Playlist" - results.append((name, rebuild_playlist_folder(root, name, real_paths, mode))) - return results + try: + db_track, conf = db.check_track_exists(title, artist, confidence_threshold=0.7) + except Exception: + continue + if db_track is None or conf < 0.7: + continue + real = resolve_library_file_path(getattr(db_track, "file_path", None), config_manager=config_manager) + if real and real not in seen: + seen.add(real) + real_paths.append(real) + name = playlist.get("name") or "Unnamed Playlist" + return name, rebuild_playlist_folder(root, name, real_paths, mode) # prune_stale=True + + +def rebuild_organized_playlists_from_db(db, config_manager, *, profile_id: int = 1): + """Rebuild EVERY "organize by playlist" folder from current library ownership — + for the manual "Rebuild" button. Self-heals after a library reorganize moves + files or membership changes. Returns a list of ``(playlist_name, summary)``.""" + return [ + _rebuild_one_from_db(db, config_manager, pl) + for pl in (db.get_mirrored_playlists(profile_id) or []) + if pl.get("organize_by_playlist") + ] + + +def rebuild_mirrored_playlist_if_organized(db, config_manager, playlist_id, *, profile_id: int = 1): + """Mirror-update hook: after a playlist's membership is re-synced, rebuild its + folder (with prune) IF it's organize-by-playlist — so a track that just LEFT + the playlist has its symlink cleaned up the instant membership changes (the + mirror image of the post-download reconcile that handles additions). Returns + the summary, or ``None`` when the playlist isn't organized / can't be found.""" + if playlist_id is None: + return None + pl = db.get_mirrored_playlist(playlist_id) + if not pl or not pl.get("organize_by_playlist"): + return None + _name, summary = _rebuild_one_from_db(db, config_manager, pl) + return summary __all__ = [ @@ -169,4 +188,5 @@ __all__ = [ "materialize_playlist_from_batch", "reconcile_batch_playlists", "rebuild_organized_playlists_from_db", + "rebuild_mirrored_playlist_if_organized", ] diff --git a/tests/test_playlist_materialize_service.py b/tests/test_playlist_materialize_service.py index 4c810045..b770ca15 100644 --- a/tests/test_playlist_materialize_service.py +++ b/tests/test_playlist_materialize_service.py @@ -10,6 +10,7 @@ from pathlib import Path from core.playlists.materialize_service import ( collect_batch_real_paths, materialize_playlist_from_batch, + rebuild_mirrored_playlist_if_organized, rebuild_organized_playlists_from_db, reconcile_batch_playlists, ) @@ -39,6 +40,12 @@ class _RebuildDB: {"track_name": "Gone", "artist_name": "y"}] # not owned return [{"track_name": "B", "artist_name": "z"}] + def get_mirrored_playlist(self, playlist_id): + for pl in self.get_mirrored_playlists(): + if pl["id"] == playlist_id: + return pl + return None + def check_track_exists(self, title, artist, confidence_threshold=0.8, server_source=None, album=None, candidate_tracks=None): fp = self.owned.get(title) @@ -191,6 +198,45 @@ def test_reconcile_noop_for_plain_batch(tmp_path: Path): assert not (tmp_path / "Playlists").exists() +def test_mirror_cleanup_prunes_removed_track(tmp_path: Path): + """Mirror-update hook: after a track LEAVES the playlist, its symlink is pruned.""" + a = tmp_path / "Music" / "A.mp3" + gone = tmp_path / "Music" / "Gone.mp3" + for f in (a, gone): + f.parent.mkdir(parents=True, exist_ok=True) + f.write_bytes(b"audio") + + class _DB: + def get_mirrored_playlist(self, pid): + return {"id": 1, "name": "Mix", "organize_by_playlist": True} if pid == 1 else None + + def get_mirrored_playlist_tracks(self, pid): + return [{"track_name": "A", "artist_name": "x"}] # 'Gone' was removed upstream + + def check_track_exists(self, title, artist, confidence_threshold=0.8, + server_source=None, album=None, candidate_tracks=None): + fp = {"A": str(a), "Gone": str(gone)}.get(title) + return (_Track(fp), 1.0) if fp else (None, 0.0) + + from core.playlists.materialize import rebuild_playlist_folder + rebuild_playlist_folder(str(tmp_path / "Playlists"), "Mix", [str(a), str(gone)], "symlink") + assert (tmp_path / "Playlists" / "Mix" / "Gone.mp3").exists() # both present before + + summary = rebuild_mirrored_playlist_if_organized(_DB(), _Cfg(str(tmp_path / "Playlists")), 1, profile_id=1) + assert summary is not None + pdir = tmp_path / "Playlists" / "Mix" + assert (pdir / "A.mp3").exists() + assert not (pdir / "Gone.mp3").exists() # pruned on mirror update + + +def test_mirror_cleanup_skips_non_organized(tmp_path: Path): + db = _RebuildDB({}) + cfg = _Cfg(str(tmp_path / "Playlists")) + assert rebuild_mirrored_playlist_if_organized(db, cfg, 2, profile_id=1) is None # Off (organize=0) + assert rebuild_mirrored_playlist_if_organized(db, cfg, 999, profile_id=1) is None # unknown + assert rebuild_mirrored_playlist_if_organized(db, cfg, None, profile_id=1) is None + + def test_rebuild_from_db_only_organized_and_owned(tmp_path: Path): """The manual button: rebuild every organized playlist by re-matching with check_track_exists (name), linking only owned tracks.""" diff --git a/web_server.py b/web_server.py index ea8f6d4e..fd2fb80d 100644 --- a/web_server.py +++ b/web_server.py @@ -33682,6 +33682,15 @@ def mirror_playlist_endpoint(): if playlist_id is None: return jsonify({"error": "Failed to mirror playlist"}), 500 + # Membership just changed — if organize-by-playlist, rebuild its folder + # (with prune) so a removed track's symlink is cleaned up now. Gated + + # non-fatal, so it can never break the mirror response. + try: + from core.playlists.materialize_service import rebuild_mirrored_playlist_if_organized + rebuild_mirrored_playlist_if_organized(database, config_manager, playlist_id, profile_id=profile_id) + except Exception as _mat_err: + logger.debug("[Playlist Folder] mirror-time cleanup skipped: %s", _mat_err) + try: if automation_engine: automation_engine.emit('mirrored_playlist_created', { From 6e86bac6eb55b6e9ea61cfd2c7d1254a786d8a20 Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Fri, 12 Jun 2026 16:35:11 -0700 Subject: [PATCH 26/64] M3U: skip the auto-save no-op when export is disabled (fixes ~30s analysis jam) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The download modal auto-saves an M3U on every render (save_to_disk, no force). When m3u_export.enabled is off it writes nothing — but only AFTER ~30s of per-track DB search + fuzzy matching, which it then discards; fired repeatedly during analysis it jammed the batch (0 tasks, user cancels). Bail out at the top of generate_playlist_m3u for exactly that case (save_to_disk and not force and not enabled). Manual 'Export as M3U' sends force=True and content-only requests send save_to_disk=False — both unaffected. Pre-existing bug, unrelated to the playlist-folder feature, but it was blocking the discovery->download flow. --- web_server.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/web_server.py b/web_server.py index fd2fb80d..6963ee2c 100644 --- a/web_server.py +++ b/web_server.py @@ -2695,6 +2695,15 @@ def generate_playlist_m3u(): save_to_disk = data.get('save_to_disk', False) force = data.get('force', False) + # The download modal auto-saves an M3U (save_to_disk, no force) on every + # render. When M3U export is disabled it would do nothing anyway — but only + # AFTER ~30s of per-track DB search + fuzzy matching below, which it then + # throws away (and which, fired repeatedly, jams the analysis). Bail out + # immediately for that case. The manual "Export as M3U" sends force=True and + # is unaffected; a content-only request (save_to_disk False) also proceeds. + if save_to_disk and not force and not config_manager.get('m3u_export.enabled', False): + return jsonify({"success": True, "skipped": True, "reason": "m3u_export disabled"}) + raw_base = config_manager.get('m3u_export.entry_base_path', '') or '' entry_base_path = raw_base.rstrip('/\\') From 7fb1b115f05584da32f7929dec1d87b18167494d Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Fri, 12 Jun 2026 16:49:52 -0700 Subject: [PATCH 27/64] Playlists: also run the reconcile on the V2 batch-completion path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit on_download_completed and check_batch_completion_v2 are duplicate completion paths. Monitor-detected downloads (Deezer / slskd-monitor / verification-worker imports) finish the batch via the V2 path, but the materialize reconcile was only added to on_download_completed — so those batches never built playlist folders (no '[Playlist Folder] Rebuilt' line at all). Add the same non-fatal reconcile to the V2 path. Now all three completion points (both lifecycle paths + the master.py all-owned path) materialize. 550 tests pass. --- core/downloads/lifecycle.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/core/downloads/lifecycle.py b/core/downloads/lifecycle.py index 68054ccc..e8abeede 100644 --- a/core/downloads/lifecycle.py +++ b/core/downloads/lifecycle.py @@ -752,6 +752,22 @@ def check_batch_completion_v2(batch_id: str, deps: LifecycleDeps) -> Optional[bo deps.download_monitor.stop_monitoring(batch_id) _cleanup_private_album_bundle_staging(batch_id, batch) + # PLAYLIST MATERIALIZE: same reconcile as the primary completion path + # (on_download_completed). Monitor-detected downloads complete via THIS + # V2 path, so the reconcile must run here too or playlist folders never + # get built for them. Path-independent, non-fatal, derived view. + try: + from core.playlists.materialize_service import reconcile_batch_playlists + for _pl_name, _mat in reconcile_batch_playlists(batch, download_tasks, deps.config_manager): + logger.info( + f"[Playlist Folder] Rebuilt '{_mat.playlist_dir}': " + f"{_mat.linked} linked, {_mat.copied} copied, " + f"{_mat.unchanged} unchanged, {_mat.removed_stale} stale removed" + + (" (symlinks unsupported here → copied)" if _mat.fellback else "") + ) + except Exception as _mat_err: + logger.error(f"[Playlist Folder] Materialize failed (non-fatal): {_mat_err}") + # REPAIR: Scan all album folders from this batch for track number issues if deps.repair_worker: deps.repair_worker.process_batch(batch_id) From 4d7267e906c89ed8a626d3d2241ee1fcd8c23683 Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Fri, 12 Jun 2026 17:08:27 -0700 Subject: [PATCH 28/64] Playlists: reconcile rebuilds touched playlists from the LIBRARY, not task fields MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The reconcile read each completed task's final_file_path to find paths — but not every import path sets it (the verification worker marks the task completed without it), so tracks that imported via that path were silently dropped (user saw 3 of 5 symlinks). Root cause: leaning on a fragile per-task field. Now reconcile_batch_playlists identifies the organize playlists the batch touched (its own + any reached via a completed track's source_info provenance) and rebuilds each from CURRENT library ownership via _rebuild_one_from_db (check_track_exists over membership). It just asks the library what's owned, so it's robust to HOW a track imported (modal worker / slskd monitor / verification worker) and still prunes tracks that left. Takes a db handle; all three callers pass MusicDatabase(). Reconcile tests rewritten for the DB-rebuild form (organize batch, wishlist provenance, non-organize skip, plain no-op). 973 downloads/imports/playlist tests pass. --- core/downloads/lifecycle.py | 6 +- core/downloads/master.py | 3 +- core/playlists/materialize_service.py | 88 +++++++++++---------- tests/test_playlist_materialize_service.py | 90 +++++++++++----------- 4 files changed, 99 insertions(+), 88 deletions(-) diff --git a/core/downloads/lifecycle.py b/core/downloads/lifecycle.py index e8abeede..cd7e38fc 100644 --- a/core/downloads/lifecycle.py +++ b/core/downloads/lifecycle.py @@ -556,7 +556,8 @@ def on_download_completed(batch_id: str, task_id: str, success: bool, deps: Life # from the batch's own captured paths — non-fatal, derived view. try: from core.playlists.materialize_service import reconcile_batch_playlists - for _pl_name, _mat in reconcile_batch_playlists(batch, download_tasks, deps.config_manager): + from database.music_database import MusicDatabase + for _pl_name, _mat in reconcile_batch_playlists(MusicDatabase(), batch, download_tasks, deps.config_manager): logger.info( f"[Playlist Folder] Rebuilt '{_mat.playlist_dir}': " f"{_mat.linked} linked, {_mat.copied} copied, " @@ -758,7 +759,8 @@ def check_batch_completion_v2(batch_id: str, deps: LifecycleDeps) -> Optional[bo # get built for them. Path-independent, non-fatal, derived view. try: from core.playlists.materialize_service import reconcile_batch_playlists - for _pl_name, _mat in reconcile_batch_playlists(batch, download_tasks, deps.config_manager): + from database.music_database import MusicDatabase + for _pl_name, _mat in reconcile_batch_playlists(MusicDatabase(), batch, download_tasks, deps.config_manager): logger.info( f"[Playlist Folder] Rebuilt '{_mat.playlist_dir}': " f"{_mat.linked} linked, {_mat.copied} copied, " diff --git a/core/downloads/master.py b/core/downloads/master.py index b18b9b00..aac93bbf 100644 --- a/core/downloads/master.py +++ b/core/downloads/master.py @@ -792,9 +792,10 @@ def run_full_missing_tracks_process(batch_id, playlist_id, tracks_json, deps: Ma if effective_playlist_folder_mode: try: from core.playlists.materialize_service import reconcile_batch_playlists + from database.music_database import MusicDatabase as _MDB _batch = download_batches.get(batch_id) if _batch is not None: - for _pl_name, _mat in reconcile_batch_playlists(_batch, download_tasks, deps.config_manager): + for _pl_name, _mat in reconcile_batch_playlists(_MDB(), _batch, download_tasks, deps.config_manager): logger.info( f"[Playlist Folder] Rebuilt '{_mat.playlist_dir}' (all owned): " f"{_mat.linked} linked, {_mat.copied} copied, {_mat.removed_stale} stale removed" diff --git a/core/playlists/materialize_service.py b/core/playlists/materialize_service.py index bcc2c905..02219858 100644 --- a/core/playlists/materialize_service.py +++ b/core/playlists/materialize_service.py @@ -74,55 +74,63 @@ def materialize_playlist_from_batch(batch: dict, download_tasks: dict, config_ma return rebuild_playlist_folder(root, name, real_paths, mode) -def reconcile_batch_playlists(batch: dict, download_tasks: dict, config_manager): - """One post-batch step: drop the batch's newly-resolved tracks into the right - ``Playlists//`` folders. Path-independent — covers an organize-by-playlist - download AND a late wishlist arrival — using the per-track playlist provenance - each download already carries, plus the file paths the batch already captured - (so it needs no DB re-match and no waiting on the server scan/sync). +def reconcile_batch_playlists(db, batch: dict, download_tasks: dict, config_manager, *, profile_id: int = 1): + """One post-batch step: rebuild every organize-by-playlist playlist this batch + TOUCHED, from CURRENT library ownership. - - The batch's OWN organize playlist is fully rebuilt from the batch payload - (owned + downloaded), pruning tracks no longer present. - - A track that completed for a DIFFERENT playlist (e.g. a wishlist - fulfillment) is ADDED to that playlist's folder — no prune, since this - batch isn't that playlist's full membership (removals are handled by a - full rebuild on the next sync / the manual button). + Touched playlists = the batch's own organize playlist + any playlist a completed + track belongs to (via the per-track ``source_info`` provenance — covers a + wishlist batch fulfilling a track that belongs to an organize playlist). Each is + rebuilt via ``_rebuild_one_from_db`` (``check_track_exists`` over its membership), + so it's robust to HOW a track imported — modal worker, slskd monitor, or the + verification worker, which don't all set the same task fields. It simply asks the + library what's owned, and prunes tracks that have left the playlist. - Returns a list of ``(playlist_name, RebuildSummary)``. Callers wrap non-fatally.""" - from core.library.path_resolver import resolve_library_file_path + Returns ``[(playlist_name, RebuildSummary)]``. Callers wrap non-fatally.""" + import json as _json - results = [] - root = docker_resolve_path(config_manager.get("playlists.materialize_path", "./Playlists")) - mode = normalize_mode(config_manager.get("playlists.materialize_mode", "symlink")) - batch_name = batch.get("playlist_name") if batch.get("playlist_folder_mode") else None + # Collect the (ref, source) of every organize playlist this batch touched. + wanted, seen_ref = [], set() - # 1. The batch's own organize playlist → full materialize (prunes removed). - if batch_name: - summary = materialize_playlist_from_batch(batch, download_tasks, config_manager) - if summary: - results.append((batch_name, summary)) + def _want(ref, source): + ref = str(ref or "").strip() + if not ref: + return + key = (ref, source or "spotify") + if key not in seen_ref: + seen_ref.add(key) + wanted.append(key) + + if batch.get("playlist_folder_mode"): + _want(batch.get("source_playlist_ref") or batch.get("playlist_id"), + batch.get("batch_source") or "spotify") - # 2. Tracks that completed for OTHER playlists (e.g. wishlist fulfilling a - # track that belongs to an organize playlist) → add-only into that folder. - extra: dict = {} for tid in (batch.get("queue") or []): task = (download_tasks or {}).get(tid) or {} - if task.get("status") != "completed" or not task.get("final_file_path"): + if task.get("status") != "completed": continue - pname = (task.get("track_info") or {}).get("_playlist_name") - if pname and pname != batch_name: - extra.setdefault(pname, []).append(task["final_file_path"]) - - for name, stored in extra.items(): - real, seen = [], set() - for p in stored: - r = resolve_library_file_path(str(p), config_manager=config_manager) - if r and r not in seen: - seen.add(r) - real.append(r) - if real: - results.append((name, rebuild_playlist_folder(root, name, real, mode, prune_stale=False))) + si = (task.get("track_info") or {}).get("source_info") or {} + if isinstance(si, str): + try: + si = _json.loads(si) + except Exception: + si = {} + if isinstance(si, dict) and si.get("playlist_id"): + _want(si["playlist_id"], si.get("source") or "spotify") + results, seen_id = [], set() + for ref, source in wanted: + try: + pl = db.resolve_mirrored_playlist(ref, profile_id, default_source=source) + except Exception: + pl = None + if not pl or not pl.get("organize_by_playlist") or pl.get("id") in seen_id: + continue + seen_id.add(pl.get("id")) + try: + results.append(_rebuild_one_from_db(db, config_manager, pl)) + except Exception: + continue return results diff --git a/tests/test_playlist_materialize_service.py b/tests/test_playlist_materialize_service.py index b770ca15..4280d71b 100644 --- a/tests/test_playlist_materialize_service.py +++ b/tests/test_playlist_materialize_service.py @@ -30,8 +30,8 @@ class _RebuildDB: def get_mirrored_playlists(self, profile_id=1): return [ - {"id": 1, "name": "Mix", "organize_by_playlist": True}, - {"id": 2, "name": "Off", "organize_by_playlist": False}, + {"id": 1, "name": "Mix", "source_playlist_id": "PL1", "organize_by_playlist": True}, + {"id": 2, "name": "Off", "source_playlist_id": "PL2", "organize_by_playlist": False}, ] def get_mirrored_playlist_tracks(self, pid): @@ -46,6 +46,12 @@ class _RebuildDB: return pl return None + def resolve_mirrored_playlist(self, ref, profile_id=1, *, default_source="spotify"): + for pl in self.get_mirrored_playlists(): + if str(ref) in (pl["source_playlist_id"], str(pl["id"]), pl["name"]): + return pl + return None + def check_track_exists(self, title, artist, confidence_threshold=0.8, server_source=None, album=None, candidate_tracks=None): fp = self.owned.get(title) @@ -142,59 +148,53 @@ def test_materialize_skipped_when_not_organize(tmp_path: Path): assert not (tmp_path / "Playlists").exists() -def test_reconcile_organize_batch_full_rebuild(tmp_path: Path): - """An organize-by-playlist batch → its own folder is fully (re)built from the - batch payload.""" - owned, fresh = _mk(tmp_path, "Owned.mp3"), _mk(tmp_path, "Fresh.mp3") - batch = { - "playlist_folder_mode": True, - "playlist_name": "Mix", - "analysis_results": [{"found": True, "matched_file_path": owned[0]}], - "queue": ["t1"], - } - tasks = {"t1": {"status": "completed", "final_file_path": fresh[0], - "track_info": {"_playlist_name": "Mix"}}} # same playlist as batch +def test_reconcile_organize_batch_rebuilds_from_library(tmp_path: Path): + """An organize batch → its playlist is rebuilt from the LIBRARY (membership × + check_track_exists), not from fragile per-task fields. Owned members link, + non-owned drop out.""" + a = _mk(tmp_path, "A.mp3")[0] # Mix membership = A, Gone; only A owned + db = _RebuildDB({"A": a}) + batch = {"playlist_folder_mode": True, "playlist_name": "Mix", + "source_playlist_ref": "PL1", "batch_source": "spotify", "queue": []} cfg = _Cfg(str(tmp_path / "Playlists")) - results = reconcile_batch_playlists(batch, tasks, cfg) - assert len(results) == 1 # only Mix (no double-add) + results = reconcile_batch_playlists(db, batch, {}, cfg) + assert len(results) == 1 name, s = results[0] - assert name == "Mix" and s.linked == 2 - pdir = Path(s.playlist_dir) - assert (pdir / "Owned.mp3").exists() and (pdir / "Fresh.mp3").exists() + assert name == "Mix" and s.linked == 1 # A owned; Gone not in library → skipped + assert (tmp_path / "Playlists" / "Mix" / "A.mp3").exists() -def test_reconcile_wishlist_adds_to_other_playlist_without_pruning(tmp_path: Path): - """The wishlist gap: a wishlist batch (not organize) downloads a track that - belongs to an organize playlist → it's ADDED to that playlist's folder, and - the folder's existing entries are NOT pruned.""" - # Pre-seed Smack That with an existing symlink (a track from a prior batch). - existing = _mk(tmp_path, "Existing.mp3")[0] - from core.playlists.materialize import rebuild_playlist_folder - rebuild_playlist_folder(str(tmp_path / "Playlists"), "Smack That", [existing], "symlink") - - # A WISHLIST batch (playlist_folder_mode False) completes a track tagged for - # the "Smack That" organize playlist. - late = _mk(tmp_path, "Late.mp3")[0] - batch = {"playlist_folder_mode": False, "playlist_name": "wishlist", - "analysis_results": [], "queue": ["w1"]} - tasks = {"w1": {"status": "completed", "final_file_path": late, - "track_info": {"_playlist_name": "Smack That"}}} +def test_reconcile_wishlist_track_rebuilds_its_playlist(tmp_path: Path): + """The wishlist gap: a wishlist batch (not organize) completes a track whose + provenance points to an organize playlist → that playlist gets rebuilt from the + library, regardless of which import path set which task field.""" + a = _mk(tmp_path, "A.mp3")[0] + db = _RebuildDB({"A": a}) + batch = {"playlist_folder_mode": False, "playlist_name": "wishlist", "queue": ["w1"]} + tasks = {"w1": {"status": "completed", + "track_info": {"source_info": {"playlist_id": "PL1", "source": "spotify"}}}} cfg = _Cfg(str(tmp_path / "Playlists")) + results = reconcile_batch_playlists(db, batch, tasks, cfg) + assert len(results) == 1 and results[0][0] == "Mix" + assert (tmp_path / "Playlists" / "Mix" / "A.mp3").exists() # Mix rebuilt from the library - results = reconcile_batch_playlists(batch, tasks, cfg) - assert len(results) == 1 and results[0][0] == "Smack That" - pdir = tmp_path / "Playlists" / "Smack That" - assert (pdir / "Late.mp3").exists() # the wishlist track was ADDED - assert (pdir / "Existing.mp3").exists() # and the prior entry was NOT pruned + +def test_reconcile_skips_non_organized_provenance(tmp_path: Path): + """A completed track pointing to a NON-organize playlist (Off / PL2) is ignored.""" + db = _RebuildDB({"B": _mk(tmp_path, "B.mp3")[0]}) + batch = {"playlist_folder_mode": False, "playlist_name": "wishlist", "queue": ["w1"]} + tasks = {"w1": {"status": "completed", + "track_info": {"source_info": {"playlist_id": "PL2", "source": "spotify"}}}} + assert reconcile_batch_playlists(db, batch, tasks, _Cfg(str(tmp_path / "Playlists"))) == [] + assert not (tmp_path / "Playlists" / "Off").exists() def test_reconcile_noop_for_plain_batch(tmp_path: Path): """A normal (non-organize, no provenance) batch → nothing happens.""" - f = _mk(tmp_path, "X.mp3")[0] - batch = {"playlist_folder_mode": False, "playlist_name": "album", - "analysis_results": [], "queue": ["a1"]} - tasks = {"a1": {"status": "completed", "final_file_path": f, "track_info": {}}} - assert reconcile_batch_playlists(batch, tasks, _Cfg(str(tmp_path / "Playlists"))) == [] + db = _RebuildDB({}) + batch = {"playlist_folder_mode": False, "playlist_name": "album", "queue": ["a1"]} + tasks = {"a1": {"status": "completed", "track_info": {}}} # no source_info + assert reconcile_batch_playlists(db, batch, tasks, _Cfg(str(tmp_path / "Playlists"))) == [] assert not (tmp_path / "Playlists").exists() From c6e077fefec07954fec668dcb5a88bd8788ad498 Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Fri, 12 Jun 2026 17:19:45 -0700 Subject: [PATCH 29/64] Playlists: batch toggle force-rebuilds its own folder (row flag = provenance only) + resolve diagnostics --- core/playlists/materialize_service.py | 31 ++++++++++++++++------ tests/test_playlist_materialize_service.py | 15 +++++++++++ 2 files changed, 38 insertions(+), 8 deletions(-) diff --git a/core/playlists/materialize_service.py b/core/playlists/materialize_service.py index 02219858..c8121b86 100644 --- a/core/playlists/materialize_service.py +++ b/core/playlists/materialize_service.py @@ -28,6 +28,9 @@ from core.playlists.materialize import ( normalize_mode, rebuild_playlist_folder, ) +from utils.logging_config import get_logger + +logger = get_logger("playlists.materialize") def collect_batch_real_paths(batch: dict, download_tasks: dict, *, config_manager) -> List[str]: @@ -90,20 +93,24 @@ def reconcile_batch_playlists(db, batch: dict, download_tasks: dict, config_mana import json as _json # Collect the (ref, source) of every organize playlist this batch touched. + # (ref, source, force). force=True for the batch's OWN playlist: the per-download + # "organize by playlist" toggle (batch playlist_folder_mode) is the user's + # intent, so it rebuilds regardless of the saved row preference. Provenance + # playlists (force=False) require the saved organize_by_playlist flag. wanted, seen_ref = [], set() - def _want(ref, source): + def _want(ref, source, force): ref = str(ref or "").strip() if not ref: return key = (ref, source or "spotify") if key not in seen_ref: seen_ref.add(key) - wanted.append(key) + wanted.append((ref, source or "spotify", force)) if batch.get("playlist_folder_mode"): _want(batch.get("source_playlist_ref") or batch.get("playlist_id"), - batch.get("batch_source") or "spotify") + batch.get("batch_source") or "spotify", True) for tid in (batch.get("queue") or []): task = (download_tasks or {}).get(tid) or {} @@ -116,20 +123,28 @@ def reconcile_batch_playlists(db, batch: dict, download_tasks: dict, config_mana except Exception: si = {} if isinstance(si, dict) and si.get("playlist_id"): - _want(si["playlist_id"], si.get("source") or "spotify") + _want(si["playlist_id"], si.get("source") or "spotify", False) results, seen_id = [], set() - for ref, source in wanted: + for ref, source, force in wanted: try: pl = db.resolve_mirrored_playlist(ref, profile_id, default_source=source) - except Exception: + except Exception as e: + logger.debug("[Playlist Folder] resolve failed for ref=%s source=%s: %s", ref, source, e) pl = None - if not pl or not pl.get("organize_by_playlist") or pl.get("id") in seen_id: + if not pl: + logger.info("[Playlist Folder] no mirrored playlist resolved for ref=%s source=%s " + "(force=%s) — folder not built", ref, source, force) + continue + if pl.get("id") in seen_id: + continue + if not force and not pl.get("organize_by_playlist"): continue seen_id.add(pl.get("id")) try: results.append(_rebuild_one_from_db(db, config_manager, pl)) - except Exception: + except Exception as e: + logger.error("[Playlist Folder] rebuild failed for '%s': %s", pl.get("name"), e) continue return results diff --git a/tests/test_playlist_materialize_service.py b/tests/test_playlist_materialize_service.py index 4280d71b..fbb3332e 100644 --- a/tests/test_playlist_materialize_service.py +++ b/tests/test_playlist_materialize_service.py @@ -164,6 +164,21 @@ def test_reconcile_organize_batch_rebuilds_from_library(tmp_path: Path): assert (tmp_path / "Playlists" / "Mix" / "A.mp3").exists() +def test_reconcile_batch_playlist_rebuilds_even_if_row_flag_off(tmp_path: Path): + """The batch's OWN playlist rebuilds because the per-download toggle (batch + playlist_folder_mode) is the intent — even when the saved organize_by_playlist + row flag is off (the common case: user flips the download-modal toggle, never + the saved preference). 'Off' (PL2) has organize_by_playlist=False on the row.""" + b = _mk(tmp_path, "B.mp3")[0] # Off's membership = B + db = _RebuildDB({"B": b}) + batch = {"playlist_folder_mode": True, "playlist_name": "Off", + "source_playlist_ref": "PL2", "batch_source": "spotify", "queue": []} + cfg = _Cfg(str(tmp_path / "Playlists")) + results = reconcile_batch_playlists(db, batch, {}, cfg) + assert len(results) == 1 and results[0][0] == "Off" + assert (tmp_path / "Playlists" / "Off" / "B.mp3").exists() + + def test_reconcile_wishlist_track_rebuilds_its_playlist(tmp_path: Path): """The wishlist gap: a wishlist batch (not organize) completes a track whose provenance points to an organize playlist → that playlist gets rebuilt from the From 47889387ad0876c87379ca8e295bab04a58637c1 Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Fri, 12 Jun 2026 17:27:35 -0700 Subject: [PATCH 30/64] Playlists: resolve synthetic mirrored batch refs (youtube_mirrored_/auto_mirror_) to PK --- core/playlists/source_refs.py | 25 +++++++++++++++++++++++ database/music_database.py | 8 ++++++-- tests/test_resolve_mirrored_playlist.py | 27 +++++++++++++++++++++++++ 3 files changed, 58 insertions(+), 2 deletions(-) diff --git a/core/playlists/source_refs.py b/core/playlists/source_refs.py index 22909895..4cf03534 100644 --- a/core/playlists/source_refs.py +++ b/core/playlists/source_refs.py @@ -19,6 +19,31 @@ from urllib.parse import parse_qs, urlparse _SPOTIFY_ID_RE = re.compile(r"^[A-Za-z0-9]{16,32}$") +# Synthetic batch playlist_id prefixes that wrap a mirrored_playlists PK. +# Download/discovery flows build a batch playlist_id as f"{prefix}{pk}" — e.g. +# auto_mirror_ (core/automation/handlers/sync_playlist.py), youtube_mirrored_ +# (YouTube discovery), and mirrored_ (web_server url hashes). The trailing +# digits are the mirrored_playlists primary key, NOT an upstream source id, so a +# (source, source_playlist_id) lookup will never match them. +_MIRRORED_PK_PREFIXES = ("youtube_mirrored_", "auto_mirror_", "mirrored_") + + +def extract_mirrored_pk(playlist_ref: object) -> Optional[int]: + """Return the mirrored_playlists PK from a synthetic batch ref, else None. + + Handles the synthetic forms above plus a bare numeric ref. Anything else + (a real upstream source id) returns None so the caller falls back to a + (source, source_playlist_id) lookup. + """ + ref = str(playlist_ref or "").strip() + if not ref: + return None + for prefix in _MIRRORED_PK_PREFIXES: + if ref.startswith(prefix): + tail = ref[len(prefix):] + return int(tail) if tail.isdigit() else None + return int(ref) if ref.isdigit() else None + @dataclass(frozen=True) class MirroredSourceRef: diff --git a/database/music_database.py b/database/music_database.py index a3b2fb03..ef0705e1 100644 --- a/database/music_database.py +++ b/database/music_database.py @@ -13704,8 +13704,12 @@ class MusicDatabase: row = self.get_mirrored_playlist_by_source(default_source, ref, profile_id) if row: return row - if ref.isdigit(): - return self.get_mirrored_playlist(int(ref)) + # Fallback: bare numeric ref or a synthetic batch id (auto_mirror_, + # youtube_mirrored_, mirrored_) whose trailing digits are the PK. + from core.playlists.source_refs import extract_mirrored_pk + pk = extract_mirrored_pk(ref) + if pk is not None: + return self.get_mirrored_playlist(pk) return None def set_mirrored_playlist_organize_by_playlist( diff --git a/tests/test_resolve_mirrored_playlist.py b/tests/test_resolve_mirrored_playlist.py index 75d3feea..fa45b007 100644 --- a/tests/test_resolve_mirrored_playlist.py +++ b/tests/test_resolve_mirrored_playlist.py @@ -55,3 +55,30 @@ def test_empty_refs_return_none(tmp_path): assert db.resolve_mirrored_playlist(None) is None assert db.resolve_mirrored_playlist('') is None assert db.resolve_mirrored_playlist(' ') is None + + +def test_synthetic_mirrored_batch_ref_resolves_by_pk(tmp_path): + """A discovery/mirror batch carries a synthetic playlist_id like + youtube_mirrored_ with batch source 'mirrored'. (source, source_playlist_id) + can't match it — it must resolve via the embedded PK. Regression for the + organize-by-playlist 'all found but no folder built' report.""" + db = MusicDatabase(str(tmp_path / "m.db")) + pk = db.mirror_playlist(source='youtube', source_playlist_id='abc123XYZ', + name='My Mirror', tracks=[], profile_id=1) + assert pk + for ref in (f'youtube_mirrored_{pk}', f'auto_mirror_{pk}', f'mirrored_{pk}'): + row = db.resolve_mirrored_playlist(ref, profile_id=1, default_source='mirrored') + assert row is not None and row['id'] == pk, ref + + +def test_extract_mirrored_pk_pure(): + from core.playlists.source_refs import extract_mirrored_pk + assert extract_mirrored_pk('youtube_mirrored_63') == 63 + assert extract_mirrored_pk('auto_mirror_7') == 7 + assert extract_mirrored_pk('mirrored_12') == 12 + assert extract_mirrored_pk('42') == 42 # bare PK + assert extract_mirrored_pk('908622995') == 908622995 # numeric upstream id (PK fallback only) + assert extract_mirrored_pk('37i9dQZF1DXcBWIGoYBM5M') is None # real spotify id + assert extract_mirrored_pk('youtube_mirrored_') is None # no digits + assert extract_mirrored_pk('') is None + assert extract_mirrored_pk(None) is None From 577bba30aa428c8b3cbcf1c30679dfae24f04818 Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Fri, 12 Jun 2026 17:34:06 -0700 Subject: [PATCH 31/64] Playlists: all-owned trigger makes batch dict authoritative + logs when nothing rebuilt --- core/downloads/master.py | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/core/downloads/master.py b/core/downloads/master.py index aac93bbf..e3d0cb57 100644 --- a/core/downloads/master.py +++ b/core/downloads/master.py @@ -795,7 +795,23 @@ def run_full_missing_tracks_process(batch_id, playlist_id, tracks_json, deps: Ma from database.music_database import MusicDatabase as _MDB _batch = download_batches.get(batch_id) if _batch is not None: - for _pl_name, _mat in reconcile_batch_playlists(_MDB(), _batch, download_tasks, deps.config_manager): + # We KNOW the intent is organize-by-playlist here (the gate + # above). The line-431 sync only writes the dict field when + # effective and NOT batch_playlist_folder_mode, so when the + # toggle itself drove it the dict field can still be falsy — + # which makes reconcile build no batch ref. Make the dict + # authoritative so reconcile sees the batch's own playlist. + _batch['playlist_folder_mode'] = True + if effective_playlist_name: + _batch['playlist_name'] = effective_playlist_name + _results = reconcile_batch_playlists(_MDB(), _batch, download_tasks, deps.config_manager) + if not _results: + logger.info( + f"[Playlist Folder] All-owned: nothing rebuilt for " + f"ref={_batch.get('source_playlist_ref') or _batch.get('playlist_id')} " + f"source={_batch.get('batch_source')}" + ) + for _pl_name, _mat in _results: logger.info( f"[Playlist Folder] Rebuilt '{_mat.playlist_dir}' (all owned): " f"{_mat.linked} linked, {_mat.copied} copied, {_mat.removed_stale} stale removed" From 08b73f0e94665da1cf4e044ee311a38f3745fd71 Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Fri, 12 Jun 2026 18:52:57 -0700 Subject: [PATCH 32/64] Playlists: remove dead _playlist_folder_mode routing branches (retired flag, now unreachable) --- core/downloads/candidates.py | 1 - core/imports/paths.py | 35 -------- core/imports/pipeline.py | 153 ----------------------------------- 3 files changed, 189 deletions(-) diff --git a/core/downloads/candidates.py b/core/downloads/candidates.py index ddce91fe..d9f20777 100644 --- a/core/downloads/candidates.py +++ b/core/downloads/candidates.py @@ -339,7 +339,6 @@ def attempt_download_with_candidates(task_id, candidates, track, batch_id=None, ) logger.info(f"[Context] Set is_album_download: {is_album_context} (has clean data: {has_clean_spotify_data})") - logger.debug(f"[Debug] Context creation - track_info: {track_info is not None}, playlist_folder_mode: {track_info.get('_playlist_folder_mode', False) if track_info else False}") # Update task with successful download info with tasks_lock: diff --git a/core/imports/paths.py b/core/imports/paths.py index 6546509f..69de9cc7 100644 --- a/core/imports/paths.py +++ b/core/imports/paths.py @@ -438,7 +438,6 @@ def build_final_path_for_track(context, artist_context, album_info, file_ext, cr original_search = get_import_original_search(context) album_context = get_import_context_album(context) source = get_import_source(context) - playlist_folder_mode = track_info.get("_playlist_folder_mode", False) artist_name = extract_artist_name(artist_context) source_info = track_info.get("source_info") or {} @@ -466,40 +465,6 @@ def build_final_path_for_track(context, artist_context, album_info, file_ext, cr total_tracks = (album_context.get("total_tracks", 0) or 0) if album_context else 0 album_type_display = get_album_type_display(raw_album_type, total_tracks) - if playlist_folder_mode: - playlist_name = track_info.get("_playlist_name", "Unknown Playlist") - track_name = get_import_clean_title(context, default=original_search.get("title", "Unknown Track")) - _artists = original_search.get("artists") or track_info.get("artists") or [] - - template_context = { - "artist": artist_name, - "albumartist": artist_name, - "album": track_name, - "title": track_name, - "playlist_name": playlist_name, - "track_number": 1, - "disc_number": 1, - "year": year, - "quality": context.get("_audio_quality", ""), - "albumtype": album_type_display, - "_artists_list": _artists, - "_itunes_artist_id": str(artist_context.get("id", "")) if isinstance(artist_context, dict) and str(artist_context.get("id", "")).isdigit() and source == "itunes" else None, - } - - folder_path, filename_base = get_file_path_from_template(template_context, "playlist_path") - if folder_path and filename_base: - final_path = os.path.join(transfer_dir, folder_path, filename_base + file_ext) - _ensure_dir(os.path.join(transfer_dir, folder_path), exist_ok=True) - return final_path, True - - playlist_name_sanitized = sanitize_filename(playlist_name) - playlist_dir = os.path.join(transfer_dir, playlist_name_sanitized) - _ensure_dir(playlist_dir, exist_ok=True) - artist_name_sanitized = sanitize_filename(template_context["artist"]) - track_name_sanitized = sanitize_filename(track_name) - new_filename = f"{artist_name_sanitized} - {track_name_sanitized}{file_ext}" - return os.path.join(playlist_dir, new_filename), True - if album_info and album_info.get("is_album"): clean_track_name = get_import_clean_title(context, album_info=album_info, default=original_search.get("title", "Unknown Track")) track_number = _coerce_int(album_info.get("track_number", 1), 1) diff --git a/core/imports/pipeline.py b/core/imports/pipeline.py index 22117377..b6a077a9 100644 --- a/core/imports/pipeline.py +++ b/core/imports/pipeline.py @@ -543,163 +543,10 @@ def post_process_matched_download(context_key, context, file_path, runtime, meta context['artist'] = artist_context - playlist_folder_mode = track_info.get("_playlist_folder_mode", False) logger.debug(f"[Debug] Post-processing - track_info type: {type(track_info)}, is None: {track_info is None}, is empty: {not track_info}") - logger.debug(f"[Debug] Post-processing - playlist_folder_mode: {playlist_folder_mode}") if track_info: logger.debug(f"[Debug] Post-processing - track_info keys: {list(track_info.keys())}") - if playlist_folder_mode: - playlist_name = track_info.get("_playlist_name", "Unknown Playlist") - logger.info(f"[Playlist Folder Mode] Organizing in playlist folder: {playlist_name}") - - file_ext = os.path.splitext(file_path)[1] - final_path, _ = build_final_path_for_track(context, artist_context, None, file_ext) - logger.info(f"Playlist mode final path: '{final_path}'") - - if not os.path.exists(file_path): - if os.path.exists(final_path): - logger.info( - f"[Playlist Folder Mode] Source gone but destination exists — already processed by another thread: " - f"{os.path.basename(final_path)}" - ) - context['_final_processed_path'] = final_path - return - pp_logger.info(f"[inner] EXCEPTION in post-processing for {context_key}: Source file not found and destination does not exist: {file_path}") - raise FileNotFoundError(f"Source file not found and destination does not exist: {file_path}") - - context['_audio_quality'] = get_audio_quality_string(file_path) - if context['_audio_quality']: - logger.info(f"Audio quality detected: {context['_audio_quality']}") - - _skip_bit_depth = _should_skip_quarantine_check(context, 'bit_depth') - rejection_reason = None if _skip_bit_depth else check_flac_bit_depth(file_path, context) - if _skip_bit_depth: - logger.info(f"[BitDepth] Skipped (user approval) for {_basename}") - if rejection_reason: - try: - quarantine_path = move_to_quarantine( - file_path, - context, - rejection_reason, - automation_engine, - trigger='bit_depth', - ) - _mark_task_quarantined(context, quarantine_path) - logger.info(f"File quarantined due to bit depth filter: {quarantine_path}") - except Exception as quarantine_error: - logger.error(f"Quarantine failed ({quarantine_error}), deleting file: {file_path}") - try: - os.remove(file_path) - except Exception as e: - logger.debug("delete quarantine fallback: %s", e) - - context['_bitdepth_rejected'] = True - with matched_context_lock: - if context_key in matched_downloads_context: - del matched_downloads_context[context_key] - - task_id = context.get('task_id') - batch_id = context.get('batch_id') - if task_id: - with tasks_lock: - if task_id in download_tasks: - download_tasks[task_id]['status'] = 'failed' - download_tasks[task_id]['error_message'] = f"Bit depth filter: {rejection_reason}" - if task_id and batch_id: - _notify_download_completed(batch_id, task_id, success=False) - return - - try: - logger.warning( - f"[Metadata Input] Playlist mode - artist: '{artist_context.get('name', 'MISSING')}' " - f"(id: {artist_context.get('id', 'MISSING')})" - ) - enhance_file_metadata(file_path, context, artist_context, None, runtime=metadata_runtime) - except Exception as meta_err: - import traceback - pp_logger.info(f"[inner] Metadata enhancement FAILED for {context_key}: {meta_err}\n{traceback.format_exc()}") - if should_wipe_tags_on_enhancement_failure(has_clean_metadata): - wipe_source_tags(file_path) - else: - logger.warning( - "[Metadata] Enhancement failed but import has clean/matched metadata — " - "preserving the file's existing tags (not wiping): %s", - os.path.basename(file_path)) - - logger.info(f"Moving '{os.path.basename(file_path)}' to '{final_path}'") - safe_move_file(file_path, final_path) - context['_final_processed_path'] = final_path - cleanup_slskd_dedup_siblings(file_path) - - if config_manager.get('post_processing.replaygain_enabled', False): - try: - from core.replaygain import analyze_track as _rg_analyze, write_replaygain_tags as _rg_write, is_ffmpeg_available as _rg_ffmpeg_ok, RG_REFERENCE_LUFS as _RG_REF - if _rg_ffmpeg_ok(): - lufs, peak_dbfs = _rg_analyze(final_path) - gain_db = _RG_REF - lufs - _rg_write(final_path, gain_db, peak_dbfs) - pp_logger.info(f"ReplayGain: {gain_db:+.2f} dB — {os.path.basename(final_path)}") - except Exception as rg_err: - pp_logger.debug(f"ReplayGain analysis skipped: {rg_err}") - - downsampled_path = downsample_hires_flac(final_path, context) - if downsampled_path: - final_path = downsampled_path - context['_final_processed_path'] = final_path - - _persist_verification_status(context, final_path) - - blasphemy_path = create_lossy_copy(final_path) - if blasphemy_path: - context['_final_processed_path'] = blasphemy_path - - downloads_path = docker_resolve_path(config_manager.get('soulseek.download_path', './downloads')) - cleanup_empty_directories(downloads_path, file_path) - - logger.info(f"[Playlist Folder Mode] Post-processing complete: {final_path}") - - try: - check_and_remove_from_wishlist(context) - except Exception as wishlist_error: - logger.error(f"[Playlist Folder] Error checking wishlist removal: {wishlist_error}") - - emit_track_downloaded(context, automation_engine) - record_library_history_download(context) - record_download_provenance(context) - - try: - pf_album_info = build_import_album_info(context, force_album=False) - if not pf_album_info or not pf_album_info.get("album_name"): - pf_album_info = { - "is_album": True, - "album_name": playlist_name, - "track_number": track_info.get("track_number", 1) or 1, - "disc_number": track_info.get("disc_number", 1) or 1, - "clean_track_name": get_import_clean_title( - context, - default=get_import_original_search(context).get("title", "Unknown"), - ), - "source": get_import_source(context) or "spotify", - } - elif not pf_album_info.get("is_album"): - pf_album_info["is_album"] = True - record_soulsync_library_entry(context, artist_context, pf_album_info) - except Exception as lib_err: - logger.error(f"[Playlist Folder] SoulSync library registration failed: {lib_err}") - - task_id = context.get('task_id') - batch_id = context.get('batch_id') - if task_id and batch_id: - with tasks_lock: - if task_id in download_tasks: - download_tasks[task_id]['stream_processed'] = True - download_tasks[task_id]['status'] = 'completed' - download_tasks[task_id]['final_file_path'] = context.get('_final_processed_path') - logger.info(f"[Playlist Folder Mode] Marked task {task_id} as completed") - _notify_download_completed(batch_id, task_id, success=True) - return - is_album_download = bool(context.get("is_album_download", False)) album_info = build_import_album_info(context, force_album=is_album_download) From 0384de7c8dd767c91a203c53d74eb70473e051d5 Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Fri, 12 Jun 2026 19:03:04 -0700 Subject: [PATCH 33/64] Settings: hide Playlist Path Template field (no longer drives symlink-view naming; kept in DOM for save/load) --- webui/index.html | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/webui/index.html b/webui/index.html index dca19e71..6a37eede 100644 --- a/webui/index.html +++ b/webui/index.html @@ -5558,7 +5558,12 @@ Variables: $albumartist, $artist, $artistletter, $title, $track (01), $album, $albumtype, $year, $quality (filename only). Use ${var} to append text: ${albumtype}s → Singles
-
+ +