167 commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
Broque Thomas
|
75fe04907f |
Wire SoundCloud as a first-class download source
Plug the previously-built SoundcloudClient (PR #478, the build-and-verify phase) into every place a download source needs to appear. Follows the same wiring contract as Tidal/Qobuz/HiFi/Deezer/Lidarr — orchestrator routing, hybrid-mode picker, search dispatch, queue/cancel/clear, provenance + library history, sidebar source label, settings UI all work plug-and-play. Backend wiring: - `core/download_orchestrator.py` — import SoundcloudClient, _safe_init it at startup, add to _client() lookup, get_source_status(), check_connection's sources_to_check default, search source_names map, search_and_download_best _streaming_sources tuple, download source_map + source_names, and every iteration loop in reload_settings download-path-update / get_all_downloads / get_download_status / cancel_download (route + iterate) / clear_all_completed_downloads / cancel_all_downloads. - `core/downloads/monitor.py` — added SoundCloud to the per-client loop that fetches active downloads outside the orchestrator (uses getattr fallback for older soulseek_client snapshots). - `core/downloads/task_worker.py` — added SoundCloud (and Lidarr, which was missing too — bonus fix) to source_clients dict for hybrid fallback dispatch. - `core/downloads/validation.py` — added 'soundcloud' to _streaming_sources so SoundCloud results go through the matching engine validation path instead of the Soulseek quality-filter path. - `core/imports/side_effects.py` — three call sites: source_map for download_source label written to library_history, streaming-source guard for the `||`-encoded stream_id parsing, and source_service map for provenance recording. All three now include 'soundcloud'. - `web_server.py` — five streaming-source detection tuples updated. New `/api/soundcloud/status` endpoint returns {available, configured, reachable} mirroring the Deezer/HiFi status-endpoint pattern; reachability runs a real cheap yt-dlp search so the settings Test Connection button gives a meaningful pass/fail signal. - `config/settings.py` — added empty `soundcloud_download` defaults block so future tier-2 OAuth (SoundCloud Go+ session) doesn't have to migrate existing configs. Frontend: - `webui/index.html` — new `<option value="soundcloud">` in the download-source-mode dropdown, SoundCloud added to both hidden legacy hybrid-source selects, new settings container with info text + Test Connection button. - `webui/static/settings.js` — HYBRID_SOURCES entry (with the SoundCloud cloud SVG icon), _hybridSourceEnabled default, updateDownloadSourceUI container display, allSources for legacy hybrid picker, testSoundcloudConnection function (hits the new status endpoint, color-codes the result), saveSettings soundcloud_download empty block. - `webui/static/shared-helpers.js` — sidebar source-name map includes SoundCloud + Lidarr (Lidarr was also missing, bonus fix). - `webui/static/helper.js` — WHATS_NEW entry under '2.4.2' dev cycle describing the user-visible change in the chill terse voice. Tests: - `tests/test_download_orchestrator_soundcloud.py` — 14 integration tests verifying the wiring: client constructed at startup, _client lookup resolves 'soundcloud', get_source_status includes it, download dispatcher routes username='soundcloud' to the SoundCloud client (and unknown usernames still fall back to Soulseek), hybrid search iterates SoundCloud when in order and skips it cleanly when unconfigured, get_all_downloads / get_download_status / cancel / clear walk SoundCloud, soundcloud-only mode dispatches only to SoundCloud, _streaming_sources tuple in validation includes 'soundcloud'. - `tests/downloads/test_download_orchestrator.py` — added `soundcloud` to the test fixture's _build_orchestrator helper so the new orchestrator attribute doesn't AttributeError in pre- existing tests that bypass __init__. Verified: - Full suite green (1728 passed, 2 deselected for soundcloud_live) - Ruff clean - Live SoundCloud-only mode search returns 25 SoundCloud tracks for "kendrick lamar luther" in <2s, returning properly-shaped TrackResult objects with username='soundcloud' and dispatch-key filename ready for the download path. Out of scope (intentional deferrals): - SoundCloud Go+ OAuth tier (256 kbps AAC) — anonymous-only for now. Adding auth later is a settings-page extension, no orchestrator changes needed. - Album/playlist support — SoundCloud has playlists but they don't map to the album model the rest of SoulSync expects. Singles only. |
||
|
Broque Thomas
|
d8437c87c6 |
Fix Album Completeness Auto-Fill on Docker / shared-library setups (#476)
GitHub issue #476 (gabistek, Docker on Arch host): "Auto-Fill" / "Fix Selected" on the Album Completeness findings page returned "Could not determine album folder from existing tracks" for every album. Reproduces on any setup where the media-server library lives outside the SoulSync transfer/download folders — Docker is the headline case but native installs that point Plex at a NAS via SMB hit it too. Root cause: `core/repair_worker.py:_resolve_file_path` only probed the transfer + download folders. Docker users have their Plex/Jellyfin library bind-mounted at /music (or similar) — neither configured in SoulSync. Every existing track got silently treated as missing, so `album_folder` stayed None and the fix workflow bailed. The same incomplete logic was duplicated four more times in the repair_jobs/ modules, all with the same bug. Album Completeness was just the most user-visible — the same setups were also producing false "missing file" findings from Dead File Cleaner, silent skips in MBID Mismatch Detector, etc. The web server already had the correct logic at `web_server.py:_resolve_library_file_path` (probes transfer + download + Plex-reported library locations + user-configured library.music_paths). The repair workers had never been updated to match. Fix: - New `core/library/path_resolver.py` extracts the union logic into a single shared function `resolve_library_file_path()`. Probes (in order, deduped): explicit transfer/download kwargs, config-derived soulseek.transfer_path/download_path, Plex-reported library locations (when a plex_client is passed), user-configured library.music_paths. Each defensive: malformed config or a flaky Plex client degrades to the dirs that did succeed. - `core/repair_worker.py:_resolve_file_path` becomes a delegating wrapper preserving the legacy signature, with a new `config_manager` kwarg. All 15 in-tree call sites updated to thread `self._config_manager` through. - `core/repair_jobs/dead_file_cleaner.py`, `mbid_mismatch_detector.py`, and `lossy_converter.py` get the same treatment: duplicate function replaced with a thin wrapper, call sites pass `context.config_manager`. - `core/repair_jobs/acoustid_scanner.py` and `unknown_artist_fixer.py` (which used to import from repair_worker) now call the shared resolver directly with `context.config_manager`. Side benefit: every other repair job (Dead File Cleaner, MBID Mismatch Detector, Lossy Converter, AcoustID Scanner, Unknown Artist Fixer) also stops missing files in the media-server library mount. Single fix unblocks five user-visible features. Tests: `tests/library/test_path_resolver.py` — 20 cases covering all four base-dir sources, suffix-walk algorithm, dedup, defensive paths (None plex client, malformed config entries, raising config_manager.get, broken plex attribute access), Docker path translation. Full suite 1677 passed locally. WHATS_NEW entry under '2.4.2' dev cycle. |
||
|
Broque Thomas
|
42f3026eef |
Reject broken downloads before tagging via universal integrity check
Discord report (fresh.dumbledore [VRN]): slskd sometimes ships broken files
(truncated transfers, corrupt FLAC, wrong file substituted on filename match).
They flowed through post-processing and only surfaced later — Plex/Jellyfin
scan failures, dead-air playback, duplicate detector tripping over the wrong
length. By that point the file was already tagged, copied, mirrored to the
media server, and recorded in provenance.
New module `core/imports/file_integrity.py`:
- `check_audio_integrity(path, expected_duration_ms=None) -> IntegrityResult`
- Three tiered checks, cheapest to most expensive:
1. File size sanity (catches 0-byte stubs and stub transfers)
2. Mutagen parse (catches header damage, wrong-format-with-right-extension)
3. Duration agreement vs. metadata source's expected length, ±3s tolerance
(5s for tracks over 10 minutes — long tracks naturally drift more)
- Returns IntegrityResult with `ok`, human-readable `reason`, and per-check
`checks` dict for debugging
- Never raises; pathological inputs return ok=False with explanation
Pipeline integration in `core/imports/pipeline.py:post_process_matched_download`:
- Hooks between the existing file-stability wait and AcoustID verification
- On failure: quarantine via existing `move_to_quarantine` helper, mark task
failed with descriptive error, clear matched-context, fire
`on_download_completed(success=False)` so the slot is released for retry
- Mirrors the existing AcoustID-failure path so retry behavior stays consistent
- Wrapped in try/except so an unexpected failure inside the check itself
cannot block downloads — logs and continues
This is intentionally tier 1: universal across formats, no external deps.
A future tier could verify FLAC STREAMINFO MD5 by decoding audio (needs
flac binary or libflac wrapper) — skipped for now since tier 1 catches the
dominant Discord-reported cases (truncated, 0-byte, wrong file).
Tests:
- `tests/imports/test_file_integrity.py` — 14 cases covering all three check
tiers, edge cases (zero/negative expected duration, long-track wider
tolerance, caller tolerance override), and the mutagen-unavailable
degradation path
- `tests/imports/test_import_pipeline.py` — two existing tests use 5-byte
fixture files that the new check would reject; they monkeypatch the
integrity check since they're testing plumbing (notification +
metadata_runtime forwarding), not integrity behavior
WHATS_NEW entry under '2.4.2' dev cycle.
|
||
|
Broque Thomas
|
cdd408b6f3 |
Auto-import: live card updates + multi-disc + featured-artist tag fixes
The 'Live Per-Track Progress' work shipped a backend in-progress row + top-of-tab
progress text but the history cards themselves stayed visually stale during
processing — lowercase "processing" badge, neutral styling, no per-track hint.
Smoke-testing also surfaced two latent identification bugs that prevented
multi-disc rips with features (Kendrick GKMC Deluxe) from importing at all.
Card-level live progress (`webui/static/stats-automations.js`):
- Cache `/api/auto-import/status` response in `_autoImportLastStatus`; poller
awaits status before re-rendering results so the card has the live data.
- Add 'processing' entries to statusLabels / statusIcons / statusClass.
- When card folder_name matches `current_folder`, swap the meta line to
`track N/M: <track name>` and tag the matching row in the expanded list
as `auto-import-track-row-active`; prior rows tag as `-row-done`.
Card styling (`webui/static/style.css`):
- `.auto-import-processing` blue left border, `.auto-import-badge-processing`
pulse animation, active/done track-row classes.
Multi-disc enumeration (`core/auto_import_worker.py:_scan_directory`):
- Old code skipped disc folders during recursion AND only attached them to a
parent that had its own loose audio. A folder containing only `Disc 1/`,
`Disc 2/` was invisible. Now: when a directory has only disc subdirs and no
loose audio, treat that directory itself as the album candidate. Disc folders
still skipped when standing alone.
- Add `FolderCandidate.is_staging_root` flag (set when the staging dir itself
becomes the candidate via this path) so identification can refuse to use the
meaningless folder name.
Tag identification (`core/auto_import_worker.py:_identify_from_tags`):
- Per-track `artist` tag fragmented consensus on albums with features
("Kendrick Lamar" / "Kendrick Lamar, Drake" / "Kendrick Lamar, Dr. Dre"
produced 3 separate `(album, artist)` keys for one album). Now group by
album first, then pick the most-common artist within that album group.
- `_read_file_tags` now prefers `albumartist` over `artist` for album-level
identity; falls back to `artist` for files without albumartist.
- Add INFO-level log when tag identification rejects, showing top albums and
their counts so the user can diagnose multi-disc / tagging issues.
Folder-name false-match guard (`core/auto_import_worker.py:_identify_folder`):
- When `is_staging_root` is set, skip the folder-name strategy entirely. Logs
the skip and falls through to AcoustID. Without this, dropping disc folders
directly into staging caused the scanner to search the metadata source for
the literal name "Staging", which false-matched against random albums (e.g.
"Stamina, Dinos" — a French rap album — at 13% confidence).
What's New entries added under 2.4.2 dev cycle.
|
||
|
Broque Thomas
|
783c543c3e |
Auto-import: live per-track progress + in-progress history row
User reported (Mushy / generally) that dropping an album into the staging folder left the auto-import history blank for the entire processing window — sometimes 5+ minutes for a full album. Pre- existing UX gap, not caused by the recent context-builder refactor. Two root causes: 1. ``_record_result`` only fired AFTER ``_process_matches`` returned. For a 14-track album with ~30s/track post-processing, that meant ~7 minutes of zero rows in auto_import_history → nothing for ``/api/auto-import/results`` to return → empty UI. 2. ``_current_status`` only ever transitioned between 'idle' and 'scanning' — never 'processing'. ``get_status()`` had no per- track index/name fields, so the UI had no way to render "Processing track 3/14: Mine" even if it wanted to. Fix: - New ``_record_in_progress`` inserts a status='processing' row up-front (before the per-track loop starts) so the UI sees the import the moment it begins. Returns the row id. - New ``_finalize_result`` updates that same row with the final outcome (completed/failed) when processing finishes. One row per album, not per track — keeps the history list clean. - Both share ``_serialize_match_data`` (extracted from the original ``_record_result``) so the in-progress row carries the same match payload shape the existing review UI already understands. - ``_process_matches`` updates ``_current_track_index``, ``_current_track_total``, and ``_current_track_name`` BEFORE each per-track callback fires, so a polling UI sees consistent "processing N/M: <name>" snapshots. - ``_scan_cycle`` flips ``_current_status`` to 'processing' before the per-album loop, resets it + the per-track fields after. Defensive ``finally`` clears progress even if the inner code path raised. - ``get_status()`` exposes the new fields so the UI's existing /api/auto-import/status polling picks them up. - Frontend (stats-automations.js): renders the new ``current_status='processing'`` state with track index/total/name in the existing progress bar element. New 'processing' status class for styling parity with 'scanning'. 8 regression tests in tests/imports/test_auto_import_live_progress.py: - get_status surfaces the new fields with sane defaults - track_index advances 1, 2, 3 during a 3-track loop - track_total set BEFORE the first callback fires (no '1/0' flicker) - _record_in_progress writes status='processing' with no processed_at - _finalize_result updates the same row to completed + processed_at, no second insert - _finalize_result with failed status leaves processed_at NULL - _finalize_result with row_id=None is a safe no-op - Per-track fields cleared by _scan_cycle's finally block Full pytest 1643 passed; ruff clean. |
||
|
Broque Thomas
|
29089b35b3 |
Honor configured Tidal redirect_uri, drop request-host fallback
Reported case (Foxxify): Tidal returned error 1002 ("Invalid redirect
URI") on every authentication attempt for users accessing SoulSync
from a network IP. User had ``http://127.0.0.1:8889/tidal/callback``
registered in his Tidal Developer Portal — matching the SoulSync UI
default and docs.
Root cause: the /auth/tidal route at web_server.py:5594-5598 had a
"fallback: dynamically set based on request host" branch that fired
when ``tidal.redirect_uri`` config was empty AND the request didn't
come from localhost. That fallback overrode the TidalClient
constructor's safe default (``http://127.0.0.1:<port>/tidal/callback``)
with a uri built from request.host like
``http://192.168.x.x:8889/tidal/callback``. Tidal compares strings
exactly so this never matched the documented portal registration and
the user got 1002 before the consent screen even rendered.
The trap is the SoulSync settings UI displays the default URI as the
placeholder + "Current Redirect URI" display — but the placeholder
never gets saved to config unless the user explicitly clicks Save.
Most users who follow the docs (register the displayed default with
Tidal, then click Authenticate) hit the empty-config path and the
broken fallback.
Fix: drop the request-host fallback. Empty config falls back to the
constructor default that matches the documented portal registration.
The existing post-auth swap-step in the instructions page below
handles the Docker / remote-access case as designed:
1. SoulSync sends 127.0.0.1:8889 in the authorize URL → matches
portal → Tidal accepts.
2. User authorizes → Tidal redirects browser to 127.0.0.1:8889
(which fails locally — nothing on user's machine listens there).
3. Instructions tell user to swap 127.0.0.1 with the host they're
accessing SoulSync from.
4. Swapped URL hits the container's exposed callback port → auth
completes.
8 regression tests in tests/test_tidal_auth_redirect_uri.py:
- Configured redirect_uri sent verbatim (localhost / custom port /
explicit network IP)
- Empty config falls back to constructor default — NOT request.host
(the actual reported scenario, with explicit assertion message
warning if the bug returns)
- Empty config + localhost access uses the same default (sanity)
Full pytest 1635 passed; ruff clean.
|
||
|
Broque Thomas
|
34ba26f5c8 |
Persist source IDs at download time + backfill onto tracks on sync
Followup to fix/watchlist-external-id-match. The companion PR closed the demand side — the watchlist scanner asks for tracks by external IDs before falling back to fuzzy. But for users on Plex / Jellyfin / Navidrome the supply side was still broken: tracks.spotify_track_id (and the other ID columns) only got populated by the asynchronous enrichment workers, sometimes hours after the file was actually written. During that window the ID match fell through to fuzzy and the bug returned. We were already collecting every ID during post-processing — they live in the `pp` dict in core/metadata/source.py:embed_source_ids and get embedded into file tags. We just dropped the in-memory copy afterwards. This PR persists them and uses them: - Schema migration adds spotify_track_id / itunes_track_id / deezer_track_id / tidal_track_id / qobuz_track_id / musicbrainz_recording_id / audiodb_id / soul_id / isrc columns + indexes to the existing track_downloads table (already keyed by file_path). - core/metadata/source.py:embed_source_ids exposes pp["id_tags"] and the resolved ISRC back to the import context as _embedded_id_tags / _isrc. - core/imports/side_effects.py:record_download_provenance reads those context fields and passes them to db.record_track_download, which now accepts the new ID kwargs and persists them. - New db.get_provenance_by_file_path with exact + basename-suffix fallback (handles container mount-root differences between download-time path and media-server-reported path). - New db.backfill_track_external_ids_from_provenance copies IDs from track_downloads onto a tracks row idempotently — COALESCE on every column preserves any value the enrichment worker already wrote (enrichment is more authoritative for late binding). - database/music_database.py:insert_or_update_media_track (the single insertion point used by every Plex / Jellyfin / Navidrome sync) calls the backfill immediately after each INSERT/UPDATE. - New core/library/track_identity.py:find_provenance_by_external_id used as a second-tier fallback in watchlist_scanner.is_track_missing _from_library — catches the window between download and media-server sync. Caller checks os.path.exists on the provenance file_path before treating it as "already in library" so a deleted file doesn't prevent re-download. Effect: freshly downloaded files become ID-recognizable to the watchlist on the very next scan, no enrichment-wait window. 19 regression tests in tests/test_provenance_id_persistence.py: - Schema migration adds expected columns + indexes - record_track_download persists every ID kwarg - record_track_download backward-compat (old kwargs still work) - get_provenance_by_file_path: exact match, basename fallback for mount-root differences, multi-record latest-wins, defensive None - backfill: copies all IDs, preserves existing via COALESCE, no-op when no provenance exists - find_provenance_by_external_id: per-ID lookup, ISRC cross-bridge, OR semantics, latest-wins on multiple matches Out of scope: backfilling provenance for files downloaded BEFORE this PR (their track_downloads rows don't carry the new IDs). Those continue to wait for enrichment. Acceptable — only affects historical files; new downloads benefit immediately. Full pytest 1625 passed; ruff clean. |
||
|
Broque Thomas
|
ecb8939c80 |
Match library tracks by external IDs before fuzzy in watchlist scan
Reported case (CAL): a track already on disk got re-downloaded by the
watchlist scanner on every scan. Library DB had stale album metadata
for the file (track tagged on album "Left Alone") while the metadata
source reported it on a different album ("NPC" single). The
title+artist+album fuzzy block correctly said the album names didn't
match and declared the track missing — but the file's stable external
IDs (Spotify ID, ISRC, etc.) unambiguously identified it as the same
recording.
The earlier compilation-album fix (PR #461) handled qualifier drift
("OST" vs "Music From The Motion Picture"). This case is two
genuinely different album names referring to the same song.
Fix: provider-neutral external-ID short-circuit before the fuzzy
block in `is_track_missing_from_library`. Pulls every recognized ID
off the source track (Spotify / iTunes / Deezer / Tidal / Qobuz /
MusicBrainz / AudioDB / Hydrabase / ISRC), runs a single SELECT
against the indexed external-ID columns on the `tracks` table, and
treats any hit as "track exists in library — don't re-download".
If no IDs are available (older imports without enrichment, library
scans that didn't populate external IDs), falls through to the
existing fuzzy logic so the safety net stays intact.
New `core/library/track_identity.py` module with two helpers:
- `extract_external_ids(track)`: handles dict and object-style track
shapes, direct-field aliases (spotify_id / spotify_track_id /
SPOTIFY_TRACK_ID), and provider-disambiguated native `id` fields
(when track has `provider='deezer'` and `id='X'`, treats X as a
Deezer ID).
- `find_library_track_by_external_id(db, external_ids,
server_source)`: builds an OR of indexed column matches with
IS NOT NULL guards, optional server_source filter that also
passes legacy NULL rows, single-row LIMIT.
ISRC bridges across providers — a library track imported via Deezer
can be matched against a Spotify scan when both sides carry the
same ISRC.
43 regression tests in `tests/test_library_track_identity.py`:
- 9 ID-extraction tests for direct fields (Spotify / iTunes / Deezer /
ISRC / MBID / AudioDB / Hydrabase)
- 8 ID-extraction tests via the provider field (8 providers + source
alias + missing-provider-ignored)
- 7 mixed/defensive tests (multiple IDs, object-style, empty strings,
None track, numeric coercion)
- 8 lookup tests (per-provider + ISRC cross-bridge)
- 3 OR-semantics tests
- 4 server_source filter tests
- 2 ID-column-map sanity tests
Full pytest 1606 passed; ruff clean.
|
||
|
Broque Thomas
|
486116c34f |
Honor lossy_copy.delete_original after successful conversion
Reported case (CAL): with lossy_copy.enabled=True, lossy_copy.delete_original=True, and codec=mp3, every download left both the original FLAC AND the converted MP3 in the target folder. Users opting into a lossy-only library ended up dual-format on every import. Root cause: ``core/imports/file_ops.py:create_lossy_copy`` reads ``lossy_copy.codec`` and ``lossy_copy.bitrate`` from config but never reads ``lossy_copy.delete_original``. The setting is only consulted by the pre-move source-vanished check at ``core/imports/pipeline.py:651`` (so the pipeline knows to look for a lossy variant when the FLAC has already moved on), but no code path actually deletes the source after conversion. Fix: after ffmpeg returns success and the QUALITY tag is written, check ``lossy_copy.delete_original`` and ``os.remove`` the original when enabled. Belt-and-suspenders: - Same-path guard (``os.path.normpath(out_path) != os.path.normpath(final_path)``) prevents accidentally wiping the just-converted file if a future codec choice somehow resolves out_path to the source path. - ``FileNotFoundError`` is treated as success (concurrent worker / dedup cleanup got there first). - Other ``OSError`` (permission denied, locked file) is logged but doesn't propagate — the conversion already succeeded, the user just has to clean up the original manually. Failure paths skip the delete: - ffmpeg returns non-zero → returns None, original stays - lossy_copy.enabled=False → early return before conversion runs - delete_original=False (default) → original stays 7 regression tests cover honored-when-enabled, kept-when-disabled, default-keep, ffmpeg-failure-path, lossy-disabled-path, racing-delete, and locked-file paths. Full pytest 1563 passed; ruff clean. Note: this PR does NOT address the second bug CAL mentioned (track re-downloaded despite already existing on disk). That symptom is caused by stale album metadata on the user's existing files — the library DB has the track tagged on a different album than the metadata source reports — combined with wishlist.allow_duplicate_tracks defaulting to True. Same class of issue partially addressed in PR fix/watchlist-redownload-and-duplicate-detection but compilation- album drift is the only currently-handled case. Tracking separately. |
||
|
Broque Thomas
|
99dbe265de |
Sync Qobuz auth to enrichment worker after login
Discord-reported (Foxxify): logging in to Qobuz via the Connect button on Settings showed "Connected: <username> (Active)" but underneath an error said "Qobuz not authenticated...", and the dashboard indicator stayed yellow. Saving settings or reloading the tab didn't help. Root cause: SoulSync runs two QobuzClient instances side by side — one through soulseek_client.qobuz for the /api/qobuz/auth/* endpoints, and a second owned by the enrichment worker thread for thread safety. The login flow only updated the auth-flow instance's in-memory state (plus persisted to config). The dashboard's "configured" check at web_server.py:3371 reads ``qobuz_enrichment_worker.client.user_auth_token`` — the WORKER's instance — which still believed itself unauthenticated. The connection-test step at core/connection_test.py:370 hits the same worker instance for the same reason. Fix: add ``QobuzClient.reload_credentials()`` — a public, network-free method that re-reads the saved session from config and updates the instance's in-memory state + session headers. Call it on the enrichment worker's client immediately after a successful ``/api/qobuz/auth/login``, ``/api/qobuz/auth/token``, or ``/api/qobuz/auth/logout`` so the two instances stay in lockstep without waiting for the next process restart. Unlike the existing ``_restore_session()`` this skips the network probe — the caller has just authenticated, so the token is known good. A small ``_sync_qobuz_credentials_to_worker()`` helper in web_server.py wraps the call so all three endpoints share one path. 10 new regression tests cover the populate / clear / partial-config paths plus the actual two-instance-sync scenario from the bug report. Full pytest 1555 passed (the one pre-existing flake in test_tidal_auth_instructions.py is order-dependent and unrelated). |
||
|
Antti Kettunen
|
a2176af00e
|
Rename metadata source status selectors
- Switch the dashboard/sidebar service-status card from spotify-branded ids to metadata-source ids - Update the shared status helpers to target the renamed metadata-source card - Keep the actual Spotify auth and settings UI unchanged |
||
|
Antti Kettunen
|
36267618a3
|
Rename status cache to metadata_source
Expose the primary metadata provider status under a generic cache key and update the websocket fixture plus frontend readers to match. |
||
|
Broque Thomas
|
7e32618f86 |
Drop old per-service enrichment routes after registry cutover
Followup to the enrichment-bubble registry consolidation. The
dashboard polling + click handlers all hit
/api/enrichment/<service>/{status,pause,resume} now, so the 30
hand-rolled per-service routes in web_server.py have zero callers
and can come out:
/api/musicbrainz/{status,pause,resume}
/api/audiodb/{status,pause,resume}
/api/discogs/{status,pause,resume}
/api/deezer/{status,pause,resume}
/api/spotify-enrichment/{status,pause,resume}
/api/itunes-enrichment/{status,pause,resume}
/api/lastfm-enrichment/{status,pause,resume}
/api/genius-enrichment/{status,pause,resume}
/api/tidal-enrichment/{status,pause,resume}
/api/qobuz-enrichment/{status,pause,resume}
Worker init blocks stay (they still construct the workers + persist
pause state). Section comment headers are preserved with a one-line
note pointing readers at the new generic blueprint.
Test fixtures in tests/conftest.py and
tests/metadata/test_enrichment_events.py also updated to use the
new URL paths so they reflect production reality. They were
synthetic stubs that never depended on the production routes —
purely cosmetic alignment.
Net: ~510 lines deleted from web_server.py. Full pytest 1541
passed; ruff clean.
|
||
|
Broque Thomas
|
98c04cf332 |
Consolidate enrichment bubble routes behind a service registry
The dashboard's enrichment-status bubbles (MusicBrainz, AudioDB,
Discogs, Deezer, Spotify, iTunes, Last.fm, Genius, Tidal, Qobuz) each
had its own copy-pasted /status, /pause, /resume route in web_server.py
— 30 routes that differed only in the worker reference and a couple
of per-service quirks (Spotify's rate-limit guard, Last.fm/Genius
yield-override behavior, Tidal/Qobuz extra status fields).
Replace them with a registry-driven blueprint:
- core/enrichment/services.py declares an EnrichmentService dataclass
with worker_getter, config_paused_key, pre_resume_check,
auto_pause_token, and extra_status_defaults — all variation captured
as data, no branching on service id.
- core/enrichment/api.py exposes a Flask blueprint with three routes
(/api/enrichment/<service>/{status,pause,resume}). Per-service
quirks are honored via the descriptor: Spotify's rate-limit ban
still returns 429 with `rate_limited: true`, Last.fm/Genius still
drop the auto-pause token and add the yield override, Tidal/Qobuz
still merge `authenticated: false` into the fallback payload.
- web_server.py registers all 10 services after their workers
initialize, wires the host-side hooks (config_manager.set,
_download_auto_paused.discard, _download_yield_override.add), and
registers the blueprint.
- webui/static/enrichment.js polling + click handlers now hit the
generic endpoints. The per-service `update<Service>StatusFromData`
functions are unchanged — they still process the same payload.
This is the cutover step. Old per-service routes are intentionally
left in place as a fallback during the soak period — they currently
have zero callers in the codebase and will be deleted in a follow-up
patch once production has run on the new pipeline for a few days.
27 new tests in tests/test_enrichment_services.py cover the registry
behavior + every quirk path through the generic blueprint (rate-limit
guard, auto-pause token cleanup, persisted-pause config keys, extra
default fields, worker-not-initialized fallback, exceptions). Full
suite 1541 passed; ruff clean.
|
||
|
Broque Thomas
|
0c4fad033d |
Show artist breadcrumb on sidebar Library button when on artist-detail page
Artist-detail is a "pseudo-page" reachable from Library, the unified
Search page, and the global search popover. It has no [data-page]
match in the sidebar, so navigateToPage's bulk-active-removal left
every nav button unhighlighted while the user was viewing an artist —
the sidebar offered no visual anchor for where they were.
Now:
- navigateToPage('artist-detail') falls back to highlighting the
Library button when no [data-page] match exists, anchoring the
sidebar to the canonical home for artist detail views.
- A new _updateSidebarLibraryBreadcrumb() helper rewrites the Library
button label between plain "Library" and a "Library / Artist Name"
breadcrumb based on currentPage + artistDetailPageState. Long names
(>14 chars) truncate with an ellipsis; the full name shows on hover
via the title attribute.
- Called from navigateToPage (entering / leaving the page) and from
loadArtistDetailData (covers same-page artist switches in the
similar-artist chain where currentPage stays 'artist-detail').
CSS adds .nav-text-root / .nav-text-sep / .nav-text-context selectors
so the "Library" anchor word stays visually dominant while the
separator and artist name dim to a secondary tier — readable but not
competing for attention.
Pure visual change. No backend touched. No new tests (DOM-only).
|
||
|
Broque Thomas
|
84810b4de4 |
Bump version to 2.4.1
Patch release wrapping up the 2.4.1 dev cycle. Highlights: - Watchlist no longer re-downloads compilation/soundtrack tracks (#458 dedup orphan cleanup + the album-match fix work in tandem to stop the loop). - Duplicate detector catches slskd dedup orphans via a second filename-bucket pass. - Beatport tab hidden temporarily — Cloudflare Turnstile blocks the scraper and the official OAuth API is closed to public devs. - Service worker for cover art + installable PWA manifest. - Browser caching for static assets (1y) and discover pages (5min). - Socket.IO same-origin default + admin-only /api/settings. Files updated: - web_server.py: _SOULSYNC_BASE_VERSION 2.4.0 -> 2.4.1 - webui/index.html: sidebar version button + modal subtitle - webui/static/helper.js: WHATS_NEW dev-cycle marker -> release date, fallback version in _getLatestWhatsNewVersion, 8 new VERSION_MODAL_SECTIONS entries promoted from this cycle - .github/workflows/docker-publish.yml: workflow_dispatch default version_tag updated to 2.4.1 |
||
|
Broque Thomas
|
6e61890551 |
Stop watchlist re-downloading compilation tracks; catch slskd dedup orphans
Two related bugs reported on Discord by Mushy. 1. The watchlist re-downloaded the same OST track up to 7 times. ``is_track_missing_from_library`` compared Spotify's album name and the media-server scan's album name with a raw SequenceMatcher at a strict 0.85 threshold. Compilations and soundtracks routinely fail this — Spotify reports ``"Napoleon Dynamite (Music From The Motion Picture)"`` while the Plex / Navidrome / Jellyfin tag scan saves it as ``"Napoleon Dynamite OST"``. Raw similarity ≈ 0.49, so the scanner declared the track missing on every 30-minute scan and added it back to the wishlist. The wishlist then issued a fresh download. slskd appended ``_<19-digit-ns-timestamp>`` to each new copy because the target file already existed, and the user ended up with seven copies of one song in one folder. Fix: extract two pure helpers — ``_normalize_album_for_match`` strips qualifier parentheticals (Music From X, OST, Deluxe Edition, Remastered, Anniversary, etc.) and trailing dash-clauses; ``_albums_likely_match`` checks equality after normalization, substring containment, and a relaxed 0.6 fuzzy ratio. A volume / part / disc / standalone-trailing-number guard rejects pairs like ``"Greatest Hits Vol. 1"`` vs ``"Greatest Hits Vol. 2"`` so the relaxed threshold doesn't introduce false positives on serialized releases. After this change the Napoleon Dynamite case collapses to ``"napoleon dynamite" == "napoleon dynamite"`` via the equality short-circuit and the redownload loop dies. 2. The duplicate detector found only one of the seven dupe files. The detector buckets tracks by the first 4 chars of their normalized tag title. Files written by slskd directly into a library folder often get inconsistent (or blank) tags from the media-server rescan, so the seven copies were bucketed apart by parsed title and never compared. Fix: refactor the per-bucket comparison into ``_scan_bucket``, then add a second pass — ``_build_filename_buckets`` re-buckets leftover tracks by canonical filename stem (slskd dedup tail stripped via ``_strip_slskd_dedup_suffix``, same regex the import-cleanup PR uses) plus extension. Filename agreement is itself strong evidence the files came from the same source download, so the second pass calls ``_scan_bucket`` with ``require_metadata_match=False`` to skip the title / artist / cross-album gates. The same-physical-file guard still runs so bind-mount duplicates aren't flagged. 72 new regression tests across two files cover the album-match helpers (28 tests including the Napoleon Dynamite scenario, 7 volume disagreements, 8 positive/negative pairs, 5 defensive cases) and the new filename-bucket pass (16 tests across bucket construction, scan integration, and existing title-pass behavior). Full pytest 1509 passed; ruff clean. Reported by Mushy in Discord. |
||
|
Broque Thomas
|
ab884292d1 |
Hide Beatport tab temporarily
Some checks failed
Compile the app and run tests / sanity-check (push) Has been cancelled
Beatport added Cloudflare Turnstile to every public page on beatport.com. The unified scraper now receives bot-challenge HTML instead of real content, so all /api/beatport/* endpoints return 500 with "Could not fetch Beatport homepage". The official Beatport v4 API is locked behind OAuth application registration that isn't open to the public — confirmed via the docs at api.beatport.com/v4/docs and community projects (beets-beatport4). The public docs SPA client_id only accepts browser-based flows (post-message redirect URI), which can't be driven server-side. Hide the Beatport tab on the Sync page so users stop hitting the broken endpoints. Backend routes and beatport_unified_scraper.py stay in code — revival is a one-attribute HTML change once Cloudflare relaxes or a workaround is found. Reported via the homepage 500 spam in user logs. |
||
|
Broque Thomas
|
46d8e15674 |
Prune slskd dedup orphans after import
slskd appends "_<19-digit unix-nanosecond timestamp>" to a downloaded filename when the destination already contains a same-named file (concurrent downloads of the same track, partial-file retries after a connection drop, cancelled-then-redownloaded files, the same track surfacing in multiple synced playlists). The file-finder code already recognized the suffix when matching a download to its source — but after the canonical file moved into the library, the leftover "_<timestamp>" siblings sat orphaned in the downloads folder forever. Reported on Discord by Shdjfgatdif. cleanup_slskd_dedup_siblings() runs at the end of each successful import (3 safe_move_file sites in pipeline.py) and prunes any remaining siblings that strip down to the canonical stem with the same extension. Conservative match (>= 18 trailing digits) keeps legitimate filenames like "Track 5" and "Album 1995" untouched. Per- file unlink failures are swallowed so a single locked file doesn't block the rest. 17 regression tests cover the suffix-strip primitive, orphan removal, no-op cases, mismatched extensions, subdirectories, and partial-failure recovery. |
||
|
Antti Kettunen
|
55603be14c
|
Clarify Spotify auth flow and sync UI
- Send Spotify auth completion back to the opener so the settings page refreshes immediately - Make the local auth flow go straight through to Spotify instead of showing the temporary instruction page - Keep the remote/docker instruction page available for manual callback setups - Sync Spotify status, connect/disconnect buttons, and metadata source selection after auth and disconnect - Keep the disconnect behavior aligned with the active primary metadata source |
||
|
Broque Thomas
|
6cdcf778f3 |
Lift /api/automations/* into core/automation/
Routes moved to thin parse-args/jsonify handlers; logic now lives in three focused modules under core/automation/. 436 lines deleted from web_server.py; 53 added back as wrappers. Module split: - core/automation/api.py — CRUD + run + history helpers. Each function takes (database, automation_engine, ...) explicitly and returns (response_body, http_status). Includes signal cycle detection preflight checks for create + update. - core/automation/progress.py — owns the in-memory progress state dict + lock (mirroring the original web_server.py globals as module-level shared state so all callers see one view), init/update/history helpers, and the WebSocket emit loop. - core/automation/signals.py — collect_known_signals for the builder autocomplete. Out of scope (deferred): - _register_automation_handlers — the 23+ action handler closures stay in web_server.py because each one is tightly coupled to feature- specific implementations (wishlist, watchlist, library scan, etc.). - Worker functions (_process_wishlist_automatically, etc.) — belong with their feature lifts. - _run_sync_task / _run_playlist_discovery_worker — sync + discovery PRs. Behavior preserved 1:1: - Same route response shapes + status codes - Same JSON field hydration (trigger_config, action_config, notify_config, last_result, then_actions) - Same backward-compat: empty then_actions + notify_type set → synthesize then_actions from notify_type/notify_config - Same signal cycle detection behavior on create + update - Same system-automation protection on delete + duplicate - Same reschedule/cancel logic on toggle + bulk-toggle + update - Same progress state shape (status, progress, phase, current_item, log capped at 50, started_at/finished_at, action_type) - Same emit-on-finish socketio push from update_progress - Same emit loop semantics (1s tick, snapshot active states, reap finished after window) Pre-existing bugs preserved (will fix in follow-up PRs): - emit_progress_loop uses naive datetime.now() against tz-aware started_at/finished_at, so the timeout-zombie check raises TypeError → caught → never fires, and the cleanup-after-window check raises → caught → state is reaped on FIRST tick regardless of the window. Tests document this behavior so the next PR can flip them to the corrected expectation. Tests: 72 new under tests/automation/ (signals 10, progress 24, api 38). Full suite: 861 passing (was 789). Ruff clean. |
||
|
Broque Thomas
|
fd7b56e58c |
Lift /api/search and /api/enhanced-search/* into core/search/
Routes moved to thin parse-args/jsonify handlers; logic now lives in six focused modules under core/search/. 720 lines deleted from web_server.py; 109 added back as wrappers; ~700 lines of new core code plus ~700 lines of tests. Module split: - core/search/cache.py — TTL+LRU cache for enhanced-search responses, keyed by (query, active_server, fallback_source, hydrabase_active, source_tag) so config changes don't poison stale entries. - core/search/sources.py — per-kind metadata search (artists/albums/ tracks) and the multi-kind ThreadPoolExecutor that fans them out. - core/search/library_check.py — library + wishlist presence check with Plex thumb URL resolution; profile-aware wishlist with legacy fallback for older DBs missing the profile_id column. - core/search/stream.py — single-track preview search; effective stream mode resolution, query-variant generation, retry walk, matching engine integration. - core/search/basic.py — flat Soulseek file search, quality-sorted. - core/search/orchestrator.py — main enhanced-search dispatch (short-query fast path, single-source bypass, hydrabase-primary fan out, alternate source list builder), NDJSON streaming generator for /source/<src>, and the SearchDeps dataclass that bundles the cross-cutting deps. Routes pass clients (spotify, hydrabase, hydrabase_worker, soulseek) and helpers (config_manager, fix_artist_image_url, _is_hydrabase_active, _get_metadata_fallback_*, _run_background_ comparison, run_async, dev_mode_enabled_provider) into core/search via a SearchDeps bundle built per-request. fix_artist_image_url stays in web_server.py because it touches 31 other call sites. Behavior preserved 1:1: - Same response shapes (db_artists, spotify_artists, spotify_albums, spotify_tracks, primary_source, metadata_source, alternate_sources, source_available) - Same NDJSON line ordering (artists/albums/tracks as they finish, plus done marker) - Same per-kind exception swallowing - Same hydrabase-worker mirror on dev mode - Same cache key shape (5-tuple) and TTL/LRU semantics - Same stream-track effective-mode resolution including the Soulseek-coerce-to-YouTube edge case - Same library-check Plex thumb URL rewriting and wishlist fallback for older DBs Tests: 94 new (cache TTL/LRU/key, sources happy/partial/all-fail, library presence with library + wishlist + thumbs, stream effective mode + query gen + retry, orchestrator client resolution + short query + single source + fan-out alternates + hydrabase primary + NDJSON drain). Full suite: 788 passing (was 694). Ruff clean. |
||
|
Broque Thomas
|
f51b75da7e |
Lift /api/stats/* and /api/listening-stats/* into core/stats/
Stats route logic moves into core/stats/queries.py as pure-ish functions that take dependencies (database, image-url fixer, listening worker) as arguments. The 13 route handlers in web_server.py shrink to thin parse-args / jsonify wrappers. What moved to core/stats/queries.py: - stats_cached: 3-key metadata cache lookup + image url fix-up - stats_overview / timeline / genres / library_health / db_storage - stats_top_artists / top_albums / top_tracks: top-N + DB enrichment - stats_recent: listening_history readback - stats_resolve_track: title+artist -> file_path lookup for playback - listening_stats_sync: spawns daemon thread that runs worker._poll - listening_stats_status: stats payload, with None-worker fallback shape No behavior change. Same response shapes, same error handling, same silent-except on per-row enrichment failure. fix_artist_image_url stays in web_server.py and is passed through as a callback so we don't have to lift its config_manager / media-server dependencies in this PR. Adds tests/stats/test_stats_queries.py — 27 tests covering happy paths, edge cases, image-url plumbing, worker glue. Ruff clean. 694 tests pass (was 667 + 27 new). |
||
|
Broque Thomas
|
f11b91a5c6 |
Service worker for cover art + PWA manifest
Addresses #365 (reported by JohnBaumb), parts 3 & 5. Client-side IDB / sessionStorage data cache (part 4) deferred to its own PR. Cover art on Library and Discover used to re-fetch from the source CDN on every page visit. Now a service worker caches images locally in CacheStorage with cache-first strategy — second visit serves art instantly with zero network round-trips. PWA manifest added so the app is installable to home screen / desktop. Service worker (`webui/static/sw.js`): - Cache-first for images: 10 known CDN hosts (Spotify, Last.fm, Apple, Deezer, Discogs, MusicBrainz CAA, YouTube thumbnails) plus the local `/api/image-proxy` endpoint plus same-origin .png/.jpg/ .webp/.gif/.svg paths. Cross-origin file-extension matches are refused so we don't accidentally cache trackers. - Stale-while-revalidate for `/static/*`: serve cached instantly, refresh in background. Combined with the existing `?v=static_v` cache-bust, deploys still ship live (different query → different cache entry, old ages out). - HTML / API / everything else: no caching, pass through. - Cache-versioned (CACHE_VERSION = 'v1'); activate handler wipes any cache whose name doesn't match the current version. - skipWaiting + clients.claim so deploys propagate to open tabs without requiring a full close-and-reopen. PWA manifest (`webui/static/manifest.json`): - Standalone display mode, theme color #1db954 (matches --accent-rgb). - Two icons (192, 512) with both `any` and `maskable` purpose, generated from favicon.png with aspect-preserving transparent padding so the existing logo lands inside the safe zone for OS-applied masks. Wiring: - `web_server.py` adds a `/sw.js` route that serves the file from root scope (a service worker only controls URLs at or below its served path; `/static/sw.js` would scope to `/static/*` only). `Cache-Control: no-cache` on the SW response so deploys propagate on next page load instead of being pinned by the 1yr static cache the rest of /static/ uses. - `webui/index.html` adds the manifest link, theme-color meta, and an apple-touch-icon for iOS. - `webui/static/init.js` registers the SW on `window.load`. Feature-detected — no-op on browsers without serviceWorker support or on non-secure origins (SW requires https or localhost). One bug caught + fixed during line-by-line self-review: `_staleWhileRevalidate` could return null to `respondWith()` when both the cache miss AND the network fetch failed (the `.catch(() => null)` collapsed the rejection to null, which then short-circuited through the falsy chain). Now explicitly awaits the network promise and falls back to `Response.error()` when it resolves to null — matches the `_cacheFirst` pattern. Browser-verified: sw.js registers, status "activated and is running" in DevTools. 603 tests pass. |
||
|
Broque Thomas
|
b0e7dae7c6 |
Cache static assets 1y + cache discover GETs 5min
Addresses #365 (reported by JohnBaumb), parts 1 & 2 of the proposal. Service worker, client-side IDB/sessionStorage, and PWA manifest deferred to follow-up PRs. 1. Static asset cache (CSS/JS/icons/fonts). `SEND_FILE_MAX_AGE_DEFAULT` flipped from 0 to 31536000 (1 year) in production. Safe because every static URL is bust-tagged with `?v=static_v` (computed once per process start), so each server restart effectively invalidates every cached asset for every user. Within a single deploy, repeat page loads hit zero round-trips on static files — was a 304 round-trip per asset before. Dev override (`SOULSYNC_WEB_DEV_NO_CACHE=1`) keeps it at 0 so iterating on JS/CSS doesn't need a server restart between edits. Collateral fixes from the bump: - Music streaming endpoint (L16140): `response.headers.add('Cache-Control', 'no-cache')` → bracket-assign. Under the old max-age=0, send_file set `no-cache` and `.add()` duplicated harmlessly. Under the new max-age=31536000, `.add()` would APPEND a second Cache-Control value → two conflicting headers, browser-undefined behavior. Bracket-assign replaces. - Backup download endpoint (L25181): explicit `Cache-Control: no-store` on the response so DB backups don't inherit the new long max-age — sensitive content, must never cache. 2. Discover GET browser cache (5 min). New `@app.after_request` hook scoped to `/api/discover/` and `/api/discovery/` paths, GET method, 2xx responses only. Sets `Cache-Control: public, max-age=300`. Skipped when the endpoint already set its own Cache-Control. Toggling between Discover sections within 5 min serves from browser cache, no backend hit. Try/except wraps the hook body and logs a warning if anything throws — never let a header-tagging bug turn a successful response into a 500. (Logging instead of `pass` since silent except-pass is exactly the anti-pattern issue #369 is about.) Audited every other Cache-Control set site in web_server.py — only the two `send_file` callers needed adjustment. Range-branch streaming uses `Response()` directly, unaffected by the config change. 603 tests pass. |
||
|
Broque Thomas
|
01b7d50311 |
Gate /api/settings endpoints behind admin profile
Closes #370 (reported by JohnBaumb). The /api/settings endpoint and three siblings (/log-level, /config-status, /verify) had no auth check — any logged-in profile could read or modify service tokens, OAuth secrets, and API keys. Cin's "minimum" suggestion from the issue: gate to admin profile. Added an `admin_only` decorator near `get_current_profile_id` that returns 403 when the current profile isn't admin (id=1). Applied to all four endpoints. Auth model note (documented in the decorator docstring): SoulSync's existing model is "trust local network" — single-admin / no-multi- profile installs default `get_current_profile_id()` to 1, so the gate is a no-op for solo users. The decorator is meaningful in multi-profile setups where non-admin sessions exist. Tightening to real per-request auth is out of scope. Did NOT consolidate with api/settings.py (Cin's "better" suggestion): that endpoint uses API-key auth (for external tools), the web_server.py copy uses session/profile auth (for the web UI). Different consumers, different auth models — merging would break one or the other. 603 tests pass. |
||
|
Broque Thomas
|
efd2960629 |
Merge remote-tracking branch 'origin/dev' into fix/socketio-cors-wildcard
# Conflicts: # webui/static/helper.js |
||
|
Broque Thomas
|
22fda5dd94 |
Trim yt-dlp pin comment, drop misleading WHATS_NEW page link
Self-review nits on PR #384: - requirements.txt: 5-line comment for one pin → 1 line. Rationale lives in commit body and #367; no need to repeat in-tree. - helper.js: dropped `page: 'settings'` from the yt-dlp WHATS_NEW entry. Settings page has no yt-dlp UI; the link would have navigated users somewhere irrelevant. 553 tests pass. |
||
|
Broque Thomas
|
77a781caba |
Pin yt-dlp in requirements.txt, drop pip install from entrypoint
Closes #367 (reported by JohnBaumb). The Docker entrypoint ran `pip install -U yt-dlp --quiet --no-cache-dir` on every container start. Three problems with that: - Non-deterministic startup: each restart could pick up a different yt-dlp version, making "works on my machine" debugging harder. - Network dependency at boot: PyPI being slow/unreachable gated the app coming up. - In-place upgrades inside running containers can race with active yt-dlp invocations and aren't a great pattern. Picked Option A from the issue: pin to an exact version in requirements.txt (`yt-dlp==2026.3.17`) and remove the entrypoint install entirely. yt-dlp comes baked into the image now via the existing `pip install -r requirements.txt` in the Dockerfile. Tradeoff: YouTube fixes ship via SoulSync releases now instead of "next container restart". The pin is documented inline with how to bump it. Net change: -3 entrypoint lines, requirements.txt pin tightened, WHATS_NEW '2.4.1' block opened (entries hidden until version bumps). 553 tests pass. |
||
|
Broque Thomas
|
013eebf350 |
Lock down Socket.IO CORS — same-origin default + opt-in allow-list
Closes #366 (reported by JohnBaumb). Socket.IO was initialized with `cors_allowed_origins='*'`, accepting WebSocket connections from any origin. A malicious site could open a WS to a user's local SoulSync instance and exfiltrate live progress / toast / activity events. This commit: - Defaults to engineio's same-origin behavior (`cors_allowed_origins=None`), which automatically honors X-Forwarded-Host so reverse proxies that send that header (Caddy / Traefik by default, properly-configured Nginx) work transparently. - Adds a `security.cors_origins` config setting + Settings → Security textarea where users behind unusual proxies / Electron wrappers / cross-origin integrations can whitelist their origin. Accepts comma or newline separated values; `*` on its own line opts back into the legacy wildcard with a startup-warning log. - Logs a clear warning the first time engineio rejects each unique origin, naming the rejected Origin and request Host and pointing users to the settings field. Without this, engineio silently 403s the upgrade and the user just sees a half-broken UI with no clue why. Threadsafe dedup so a hostile origin can't spam logs. Logic lives in `core/socketio_cors.py` (resolver, rejection predictor, dedup logger class, startup-status emitter) — pure functions, no Flask dependency. `web_server.py` adds 23 lines of wiring and imports. Important catch during review: my first pass used `cors_allowed_origins=[]` as the "secure default." Reading engineio's source revealed `[]` actually means "DISABLE CORS HANDLING" (engineio/server.py:202: `if cors_allowed_origins != []:`) — identical security to `'*'`. Fixed to use `None` (engineio's actual same-origin sentinel) and pinned with a regression test that asserts the resolver never returns `[]` for any input shape. Tests: - tests/test_socketio_cors.py — 45 unit tests covering 19 resolver shape cases (None, empty, whitespace, comma, newline, garbage types, lists), the `[]`-must-never-be-returned security regression, 12 rejection prediction cases, X-Forwarded-Host handling, dedup logger behavior, threadsafe race (8 threads × 50 hammers → exactly 1 warning), and startup-status emitter outputs. Frontend: - Settings → Security gains an "Allowed WebSocket Origins" textarea with help text explaining same-origin default + when to add a domain + the `*` opt-out. - helper.js — new '2.4.1' WHATS_NEW block (hidden until version bump) with a chill-voice entry describing the change. Conftest.py left at `'*'` — test environment, no security concern. 598 tests pass. |
||
|
Broque Thomas
|
04ff287c72 |
Rewrite changelog entries in user voice
Trimmed the WHATS_NEW '2.4.0' block (27 entries) and the full VERSION_MODAL_SECTIONS array (23 sections) from the diagnostic-paragraph style I'd been defaulting to into something terse and casual: - Descriptions are 1-2 short sentences instead of multi-clause writeups. - Modal feature bullets capped at 3-7 short items each. - Stripped parenthetical credits from titles (no more "(kettui Review)", "(Images, Counts, Title Hints)" — those belong in git history, not UI). - Lowercase casual tone throughout description bodies. - No reporter handles in entry text. Net: 176 insertions / 194 deletions. helper.js parses, 553 tests pass. |
||
|
Broque Thomas
|
7714b51a50 |
Lift version modal data into helper.js, delete /api/version-info
The version modal pulled its content from /api/version-info — a 295-line
hand-curated Python dict in web_server.py. The "What's New" panel pulled
its content from WHATS_NEW in helper.js. Same release notes, two files,
two languages, hand-edited at every release — drift was inevitable
(and happened: the kettui-fix entries I added recently differed in
detail between the two surfaces).
This commit makes helper.js the single editing surface:
- Adds VERSION_MODAL_SECTIONS const in helper.js right beside WHATS_NEW,
with a comment block documenting the relationship: WHATS_NEW is the
per-version detailed log used by the helper popover; VERSION_MODAL_SECTIONS
is the curated highlight reel shown by the sidebar version button. Both
edited at release time, both in the same file.
- Rewires showVersionInfo() in downloads.js to read from those consts
directly. No backend round-trip; the changelog content ships in the
same JS bundle the browser already loaded.
- Deletes the /api/version-info route and its 295-line version_data dict.
- Updates the line-39 comment to drop the now-stale "version-info endpoint"
reference.
Note: this is collocation, not true unification. WHATS_NEW and
VERSION_MODAL_SECTIONS are still two distinct structures with overlapping
content, linked by a comment convention rather than a shared schema. A
deeper refactor (e.g. a `featured` flag on WHATS_NEW entries that the
modal aggregates) was rejected as out-of-scope — the curated section
titles ("Earlier in v2.3", "Recent Fixes") aren't 1:1 mappable to
WHATS_NEW entries. Saving for a follow-up if the drift problem persists.
Risk audit:
- Load order: helper.js loads at line 7967, downloads.js at line 7873.
Both classic scripts execute synchronously before any clickable
interaction, so showVersionInfo (only invoked on the version-button
onclick) always sees both consts defined.
- populateVersionModal() unchanged — receives the same {title, subtitle,
sections: [{title, description, features, usage_note?}]} shape.
- Stale-cache window during deploy: old downloads.js hitting a 404 on
the deleted endpoint falls through to the existing catch + toast path
("Failed to load version information"). Cache-buster ?v=static_v
resolves on next page load.
553 tests pass. helper.js + downloads.js parse cleanly. No residual
references to /api/version-info anywhere in the repo.
|
||
|
Broque Thomas
|
8ed6ccbb4e |
Bump version to 2.4.0 for dev → main release
- _SOULSYNC_BASE_VERSION → 2.4.0 (was 2.39).
- Migrate WHATS_NEW key '2.40' → '2.4.0', strip unreleased flags off
the 27 entries shipping in this release, set release date.
- Replace parseFloat() version compare with proper int-tuple semver
comparator — parseFloat('2.4.0') and parseFloat('2.4.1') both return
2.4, which would have made future patch bumps invisible to the
What's New surfacing logic.
|
||
|
Broque Thomas
|
37aefd2ff1 |
Reorganize queue: race + dedupe fixes from kettui review
Five issues kettui flagged on PR #377: - Worker race (reorganize_queue.py): _next_queued() picked an item and released the lock, then re-acquired to flip status='running'. A cancel() landing in that window marked the item cancelled but the worker still ran it. Replaced with _claim_next_or_wait() that picks AND flips under one lock acquisition. - Wakeup race (reorganize_queue.py): _wakeup.clear() after the empty check could lose an enqueue's _wakeup.set(), parking a freshly-queued album for up to 60 seconds. Replaced Lock + Event with a single threading.Condition; cond.wait() releases and re-acquires atomically on notify. - Bulk dedupe (reorganize_queue.py:enqueue_many): looped single-item enqueue, so a duplicate album_id later in the same batch could slip through if the worker finished the first copy before the loop reached the second. Now holds the lock for the whole batch and tracks a per-batch seen set, so intra-batch duplicates dedupe against each other and not just pre-existing items. - Preview button stuck disabled (library.js:loadReorganizePreview): early returns and thrown errors skipped the re-enable line. Moved state into a canApply flag committed in finally, so any exit path lands the button correctly. - DB helpers swallowing failures (music_database.py): get_album_display_meta and get_artist_albums_for_reorganize used to catch every Exception and return None / [], so a real DB outage masqueraded as "album not found" / "no albums". Now lets exceptions bubble; the route layer already wraps them as 500. Tests: - test_cancel_and_run_are_mutually_exclusive — hammers enqueue+cancel pairs and asserts the invariant that no successfully-cancelled item ever ran (catches regressions to the atomic pick). - test_enqueue_many_dedupes_batch_internal_duplicates — pins the intra-batch dedupe. - test_get_album_display_meta_propagates_db_errors and test_get_artist_albums_for_reorganize_propagates_db_errors — pin the bubble-up behavior. Changelog updated in helper.js and version modal. |
||
|
Broque Thomas
|
d6094a3587 |
Library reorganize: FIFO queue with live status panel
Replaces the single-slot "one reorganize at a time, return 409 on collision" model with a per-user FIFO queue. Buttons stay clickable, "Reorganize All" is one backend call instead of an N-call JS loop, and a status panel mounted at the top of the artist actions bar shows live progress (active item, queued count, recent completions) with per-item cancel buttons. Backend - core/reorganize_queue.py: singleton queue + worker thread, dedupe-on- enqueue, cancel rules (queued cancellable, running not), enqueue_many for bulk operations, progress fan-out via update_active_progress - core/reorganize_runner.py: factory builds the worker's runner closure with injected dependencies. Reads config per-call so changing the download path in Settings takes effect on the next reorganize without a server restart - database/music_database.py: get_album_display_meta and get_artist_albums_for_reorganize — moves the SQL out of route handlers - web_server.py: thin enqueue/snapshot/cancel/clear endpoints, runner registration at module load. Old _reorganize_state globals + status endpoint deleted. Static-asset cache buster (?v=<server-start>) added so JS/CSS updates ship live without users clearing cache Frontend - webui/static/library.js: status panel mount, polling (1.5s when active, 8s when idle), expand/collapse, per-item cancel, debounced enhanced-view reload (one reload per artist batch instead of N). Per-album reorganize button paints with queued/running indicator and short-circuits to a toast when the album is already in queue - webui/static/style.css: panel + button styling matching the existing glass-UI accents - webui/static/helper.js + version modal: WHATS_NEW entry Tests (22 new) - tests/test_reorganize_queue.py (19 tests): FIFO order, dedupe, per-item source, cancel rules, continue-on-failure, snapshot shape, progress propagation, bulk enqueue - tests/test_reorganize_runner.py (4 tests): per-call config reads, setup-failure summary, dependency injection, progress fan-out - tests/test_reorganize_db_methods.py (7 tests): SQL JOIN behavior, ordering, fallback for blank strings, artist isolation Full suite 549 passed in 27s. |
||
|
Broque Thomas
|
98c85f928e |
Merge remote-tracking branch 'origin/dev' into fix/reorganize-via-post-process-pipeline
# Conflicts: # webui/static/helper.js |
||
|
Broque Thomas
|
cb67773998 |
Merge remote-tracking branch 'origin/dev' into fix/album-completeness-api-track-count
# Conflicts: # webui/static/helper.js |
||
|
Broque Thomas
|
2b15260b88 |
Reorganize: route library files through the post-processing pipeline
Reported on Discord by winecountrygames. The library "Reorganize" tool
had several layered bugs that all traced to the same root cause: the
endpoint reinvented every wheel post-processing already turns — its own
template engine, its own disc-number resolution from file tags, its own
sidecar sweep, its own collision detection — and each had drifted from
the canonical path used by fresh downloads. Reported symptoms:
- 3-disc Aerosmith deluxe collapsed to a flat single-disc layout
- Half the tracks on other albums silently skipped, no error / no count
- Re-runs left empty leftover album folders cluttering the artist dir
Architecture: stop reinventing wheels. Route reorganize through exactly
the same pipeline downloads use. Per-album:
1. Fetch the canonical tracklist from a metadata source (Spotify /
iTunes / Deezer / Discogs / Hydrabase) using the album's stored
source IDs. New `core/library_reorganize.py::plan_album_reorganize`
does this — primary-source-first, fall through priority chain
unless the user picked a specific source in the modal (strict mode).
2. For each local track, find the matching API entry via a scored
candidate matcher. Score components: exact-title (100),
substring-with-length-ratio (40-90), track-number agreement (20).
Hard reject when the two titles have different version
differentiators (Remix vs no-remix means different recordings,
not annotation drift). Below threshold = unmatched, surfaced as
"not in source's tracklist, left in place" rather than silently
mis-routing.
3. Copy the file to a per-album staging directory, build the same
context dict the import flow builds (`spotify_album` /
`track_info` / etc. with `is_album_download=True` so the path
builder enters ALBUM mode, not SINGLE mode), call
`_post_process_matched_download(...)` — same function fresh
downloads use. Post-process handles tagging, multi-disc subfolder
decisions, sidecar regeneration, AcoustID verification.
4. Read `context['_final_processed_path']` to learn where it landed.
Update `tracks.file_path` in the DB BEFORE removing the original
(DB-update failure leaves the file at both locations, recoverable
via library scan; the reverse would orphan the row). Delete
per-track sidecars (post-process recreates them at the new
destination).
3 concurrent workers per album via ThreadPoolExecutor, matching the
download path's per-batch worker count. State mutations all guarded by
a single lock; staging filenames carry a UUID prefix so concurrent
copies of identically-named source files don't overwrite each other.
Source picker in the modal lets the user choose which source to read
the tracklist from. Two endpoints feed it:
- `/api/library/album/<id>/reorganize/sources` — sources for THIS
album that are both authed AND have a stored ID. For the per-
album modal.
- `/api/library/reorganize/sources` — all authed sources globally.
For the bulk "Reorganize All" modal where per-album ID coverage
varies.
When the user picks a specific source, the orchestrator runs in
`strict_source=True` mode (no fallback chain) — picking Spotify means
"use Spotify or fail", not "use Spotify and silently fall back."
Preview endpoint shares the same planning logic as apply via
`preview_album_reorganize` — the destination path comes from the same
`_build_final_path_for_track` post-process uses, so what you see in
the preview is exactly what you get on apply.
Empty destination folders (from earlier failed runs OR from the
current run when post-process creates a dir then fails AcoustID)
get cleaned up after each successful run: walk up to the artist
folder from any successful destination, prune empty album-sibling
folders one level deep. Bounded scope = won't touch unrelated user
dirs.
Web_server.py shrinks by ~450 net lines. The endpoint handler is now
a thin wrapper that builds injected callables (path resolver, post-
process function, DB updater, empty-dir cleaner), spawns a thread
that calls `reorganize_album()`, and returns. All actual logic lives
in `core/library_reorganize.py` where it's unit-testable without
spinning up Flask.
Frontend cleanup: the per-call template input in both reorganize
modals (per-album and bulk) was redundant — the backend always uses
the configured global download template. Removed the input and the
variables-grid reference UI it was for.
39 new unit tests pin every contract:
- source resolution (no_source_id when album has none, fallthrough
chain when primary returns nothing, strict mode bypasses fallback)
- matcher scoring (exact / substring / multi-disc disambiguation /
smart-quote tolerance / dash-vs-parens / bonus-track substring /
Remix-vs-original differentiator rejection / "Real" doesn't false-
match "Real Real Real" / track-number-only no longer fires)
- file safety (DB-update failure leaves original in place, post-
process failure leaves original in place, post-process exception
caught and original preserved, success removes original AND
updates DB in the right order)
- sidecar handling (per-track .lrc/.nfo deleted on success, kept on
failure; album-level cover.jpg/folder.jpg cleaned only when
directory has no remaining audio)
- staging cleanup (recreated between tracks because post-process
nukes it, dir cleaned up on success AND on failure)
- destination-dir prune (empty siblings removed, real album with
files preserved, no recursive sweep)
- source picker (only authed-with-stored-ID sources for per-album,
all authed sources for bulk; strict mode doesn't fall back)
- concurrency (3 workers in flight, state stays consistent under
races, stop_check cuts off pending tasks)
- preview parity (preview produces same destination as apply for
multi-disc; ALBUM mode not SINGLE mode; unmatched/no-path tracks
surfaced with reasons)
Limitations (deliberate punts, NOT in this PR):
- Renamed local titles on multi-disc albums where track_number
also disagrees: matcher returns nothing (track is "not in
source"). Fixable by using duration_ms as a tertiary signal.
- Per-track in-modal source switching with per-album track-count
hints (would need a second API call before opening the modal).
- UI status panel on the artist page during a run — currently
just toasts. Documented as a follow-up PR.
Files:
- core/library_reorganize.py — new module: plan_album_reorganize,
preview_album_reorganize, reorganize_album, available_sources_for_album,
authed_sources, _score_candidate, helpers for staging/post-
processing/finalizing, sidecar + dest-dir cleanup
- core/metadata_service.py — no changes; reused get_album_for_source,
get_album_tracks_for_source, get_source_priority,
get_client_for_source
- web_server.py — three endpoints (preview / apply / sources GETs)
are thin wrappers; -450 net lines
- tests/test_library_reorganize_orchestrator.py — 39 tests covering
every contract above
- webui/static/library.js — source picker UI in both modals; dead
template input + variables-grid removed
- webui/static/style.css — dropdown option styling fix (white-on-
white was unreadable)
Reported on Discord by winecountrygames — his bug report named the
trigger button (Enhanced view → Reorganize All) and both symptoms
(multi-disc collapse, half-album skip), which let the diagnosis go
straight to the architectural problem.
|
||
|
Broque Thomas
|
252121ca96 |
Bump Spotify post-ban cooldown from 5 min to 30 min
Reported on Discord by winecountrygames — Spotify auth granted, then
re-banned for 4 hours within ~30 seconds, repeatedly. Trace from his
captured log:
< 12:05 [pre-log] Spotify ban active when log starts
15:21:27 First ban EXPIRED → 5-minute post-ban cooldown begins
15:26:27 Cooldown ends, spotify_client.is_authenticated() probe
allowed again → client initialized
15:26:59 First Spotify API call after cooldown — get_artist_albums
for an artist whose discography a background worker was
enriching — gets 429 immediately with no Retry-After
header → new ban activated for 14400s (4 hours)
Root cause: `_POST_BAN_COOLDOWN = 300` (5 minutes) is shorter than
Spotify's actual server-side memory of the previous offense. The
cooldown exists specifically to prevent the "ban expires → we probe →
re-ban" cycle (`spotify_client.py:65-68` documents that intent
explicitly), but the value was wrong: Spotify's server still
considered this user banned 5 minutes after our local ban window
ended, so the very first call after cooldown got slapped.
The 4-hour re-ban itself is correct behavior — `_BASE_MAX_RETRIES_BAN`
fires when spotipy reports "max retries", which means the client
exhausted its internal retry budget on 429s before raising. That's a
severe-ban signal and a long default is the right response.
Fix: bump `_POST_BAN_COOLDOWN` to 1800 seconds (30 min). This is the
smallest change that addresses the immediate "re-probe → re-ban" loop
in the report. 30 minutes is an empirical floor — long enough for
Spotify to actually clear its server-side memory in the cases we've
observed, short enough not to keep functional users locked out beyond
necessary. Can be revisited if reports persist.
What this PR does NOT fix (important context for the same user):
This bump only helps the "ban expires → we re-probe → re-ban" loop.
It does NOT help winecountrygames's other symptom — Spotify being
banned within 30 seconds of his FIRST EVER authorization (no prior
ban). That's a separate failure mode: on first auth, enrichment
workers immediately fan out across the user's library (250 artists
in his case), hammering Spotify endpoints with bulk get_artist_albums
calls before any rate-limit feedback can land. Spotify's hidden
per-endpoint daily quotas — which BoulderBadgeDad has empirically
documented but the global rate limiter doesn't see — flag the burst
and impose a multi-hour cooldown that LOOKS like a bot-detection ban
to us. A proper fix needs a fresh-auth ramp-up: start with very low
Spotify QPS for the first N minutes, scale up only if no rate-limit
feedback arrives. That's a separate PR.
Documented as additional follow-ups (NOT in this change):
- Adaptive cooldown that scales with the size of the previous ban —
a 4-hour MAX_RETRIES ban probably warrants a 1-hour cooldown,
while a 60-second Retry-After-honored ban can resume in 5 minutes.
The system already distinguishes these in `_set_global_rate_limit`,
it just doesn't propagate the distinction to cooldown duration.
- Probe-with-light-call pattern — make the first post-cooldown call
a single inexpensive endpoint (`current_user`) rather than
allowing a background worker's heavy `get_artist_albums` to be
the canary. Failed probe extends cooldown silently instead of
triggering a fresh 4-hour ban.
- Fresh-auth ramp-up (per the limitation above).
Files:
- core/spotify_client.py — `_POST_BAN_COOLDOWN` 300 → 1800. Comment
expanded to cite the report so the value isn't bumped back without
context.
- webui/static/helper.js — WHATS_NEW entry under 2.40 explaining
the change for affected users.
No tests added — the cooldown logic itself is unchanged, only the
constant. Tests asserting on a constant value are theater.
Reported on Discord by winecountrygames — his captured log made the
"ban-expires-to-re-ban" timing chain unambiguous.
|
||
|
Broque Thomas
|
a9f827ef42 |
Reject Tidal streams that silently downgrade from the requested quality
Reported on Discord by Netti93: with Tidal configured for "HiRes only"
and "Allow Quality Fallback" disabled, tracks were still downloading
successfully — as m4a 320kbps files. Some "successful" downloads were
less than half the file size of the same track pulled via Tidarr/tiddl
from the same Tidal account.
Root cause: Tidal's API silently degrades to the best quality your
account + the track + your region permits. Setting
`session.audio_quality = Quality.hi_res_lossless` and calling
`track.get_stream()` on a track that's only available in AAC returns
an AAC stream with no error. The downloader wrote the m4a file to
disk, the ~7MB size sailed past the 100KB stub threshold, and the
download reported success.
The pre-existing "verify quality wasn't silently downgraded" block
only LOGGED a warning when this happened; it did not fail the tier.
Two knock-on effects:
- Users with "HiRes only, no fallback" got m4a files anyway, which
defeats the setting entirely.
- The worker-level fallback chain (hires → lossless → high → low)
couldn't advance past the first tier, because every tier
"succeeded" at whatever Tidal happened to serve.
Fix: after `track.get_stream()`, compare `stream.audio_quality`
against the tier we asked for using a rank-based ordering:
LOW < HIGH < LOSSLESS < HI_RES < HI_RES_LOSSLESS
- Same tier or higher → accept (so the occasional Tidal upgrade
doesn't get rejected just because it's not an exact match).
- Lower tier → reject THIS tier. The loop `continue`s and the next
fallback tier is tried, or the whole download fails honestly
when the user has fallback disabled. The existing final-error
log already has a hint directing users to enable fallback if
they want automatic Lossless substitution.
- Unrecognized `audioQuality` value (e.g. a new Tidal tier we
haven't mapped) → reject conservatively, so the next fallback
tier gets a chance and the diagnostic log names the unknown
value.
Why the rank-based approach instead of strict equality:
Tidal's API doesn't technically promise an exact-tier match on
serving; on tracks that are flagged in its catalog as a higher
tier, it can serve higher than the session setting. Rejecting
higher-than-asked quality would be user-hostile. And the `HI_RES`
(legacy MQA) value — not in tidalapi's modern `Quality` enum but
possibly still present on old catalog entries — needs to rank
below `HI_RES_LOSSLESS`: users asking for true lossless HiRes
should reject MQA since MQA is a lossy format.
tidalapi's `Quality` enum is a `str` subclass whose VALUES (not
member names) match what the Tidal API returns in the
`audioQuality` field (e.g. `Quality.hi_res_lossless.value ==
'HI_RES_LOSSLESS'`, `Quality.low_320k.value == 'HIGH'`). Both
sides of the comparison are coerced to `str` before use, so the
check is robust to whichever tidalapi version exposes the served
quality as an enum or a plain string.
The check is extracted as `_verify_stream_tier(stream, q_info,
q_key) -> (ok, reason)` at module scope — a pure function with no
I/O, unit-tested independently. Ten tests: match, three upgrade
cases (LOSSLESS → HI_RES_LOSSLESS, LOSSLESS → HI_RES, LOW → any
higher), three downgrade cases (the reported HiRes → AAC, HiRes
Lossless → MQA HiRes, Lossless → AAC), one unrecognized-tier case,
and two defensive paths for older tidalapi builds without
`audio_quality` on the stream object and for QUALITY_MAP entries
that lack `tidal_quality` (e.g. tidalapi wasn't importable at
module load). Test stub updated to use uppercase `Quality` values
matching real tidalapi so case-sensitivity regressions get caught.
Also removed the old codec-string-based warning block — the new
tier check is strictly stronger, and keeping the warning around
would just be dead code waiting to drift out of sync.
Deliberately NOT tackling in this PR (documented as follow-ups):
- Bit-depth verification of HiRes FLAC files via mutagen. The
`stream.audio_quality` tier check catches the main "HiRes
requested, got AAC" case; bit-depth would only matter if Tidal
labeled a stream HI_RES_LOSSLESS but served a 16-bit FLAC
(`Stream.bit_depth` isn't reliable for this — tidalapi defaults
missing `bitDepth` fields to 16, so a trust-the-stream check
would spuriously reject valid HiRes whenever Tidal omits the
field). A proper fix runs mutagen post-download to inspect the
actual file, then decides whether to delete + retry the next
tier — a whole new failure mode with design trade-offs that
deserve their own PR. The support logs don't show this
happening.
- The "manual remap still says Not Found" symptom. Might be
downstream of this same bug (silent-AAC "success" hitting a
later rejection), might be a separate task-state issue. Not
guessing without logs from the retry path.
- Quality-aware stub threshold. 100KB is a reasonable floor for
real stub/preview detection and there's no evidence the
universal threshold is misfiring in the wild.
Field-verified status: desk-verified via unit tests and empirical
checks against a live tidalapi import (confirming the `Quality`
enum's str-subclass behavior). Not yet smoke-tested end-to-end
against a real Tidal account with a HiRes-only-no-fallback
setting — Netti93 or anyone else with that config should notice
either the fix working (non-HiRes tracks fail honestly with a
clear log line) or any regression before wider release.
Files:
- core/tidal_download_client.py — new `_verify_stream_tier` helper
and `_QUALITY_RANK` table at module scope, called in the
download loop after the stream is fetched and before any
bandwidth is spent. Removed the old inline codec-based warning
since the new check supersedes it.
- tests/test_tidal_stream_tier_verification.py — ten tests covering
match / upgrade / downgrade / unknown / defensive paths.
- tests/test_tidal_search_shortening.py — fake `Quality` values
brought in line with tidalapi's real values so both files share
a consistent stub regardless of pytest collection order.
- webui/static/helper.js — WHATS_NEW entry under 2.40 describing
the rank-based tier comparison.
Reported on Discord by Netti93 — the "same account works via
Tidarr" comparison narrowed the cause to SoulSync's download path
rather than an account/region issue.
|
||
|
Broque Thomas
|
a60546929e |
Fix Album Completeness job reporting zero findings for every album
Reported by sassmastawillis: the Album Completeness maintenance job
scans 3127 albums in 0.1 seconds and reports 0 findings — for every
user, regardless of whether their library is actually complete.
Restoring an older DB surfaced 7 correct findings, so the code logic
works; the DB state is what's making everything look complete.
Root cause: `albums.track_count` is only ever written by server-sync
paths — Plex's `leafCount`/`childCount` and SoulSync standalone's
`len(tracks)`. It's the OBSERVED count of tracks SoulSync has indexed,
which is always exactly what `COUNT(tracks)` returns for that album.
The completeness job treated it as the EXPECTED total and compared it
against the observed count. They're equal by construction, so
`actual >= expected` is always true: skip, 0.1s scan, 0 findings.
Fix: new `api_track_count INTEGER` column on `albums`, written only by
metadata-source code paths. Populated in two places so the scan is
fast and the fallback is robust.
1. Enrichment workers — shared helper `set_album_api_track_count`
in `core/worker_utils.py`. Called by each worker's existing
`_update_album` method alongside its other album-column UPDATEs:
- spotify_worker: `album_obj.total_tracks` from the Spotify Album
dataclass (already in hand, zero new API calls)
- itunes_worker: same, from the iTunes Album dataclass
- deezer_worker: `nb_tracks` from full_data, falling back to
search_data when the full lookup didn't run
- discogs_worker: count of tracklist rows where `type_=='track'`
(Discogs tracklists interleave heading and index rows that
shouldn't count as songs)
Helper skips the write on zero/None/negative/non-numeric inputs
so a source lacking track info can't clobber a good value a
different source already wrote. Caller owns the transaction —
helper just queues an UPDATE on the caller's cursor without
committing, so it batches cleanly with each worker's existing
multi-UPDATE pattern.
Hydrabase worker deliberately not touched — it's a P2P mirror
that doesn't write album metadata to the local DB. Hydrabase-
primary users hit the fallback path below.
2. Album Completeness repair job — new `al.api_track_count` column
in the SELECT, read first in the scan loop. On miss (album never
enriched, or enrichment workers haven't run yet on a fresh
install), falls through to the existing `_get_expected_total()`
API lookup and persists the result via the same shared helper
(wrapped in connection/commit management since the repair job
runs outside a worker's batched transaction).
Also removed `al.track_count` from the scan's SELECT — now unused
since the observed count was the whole source of this bug, and
leaving a dead SELECT would invite a future engineer to re-introduce
the same comparison.
Help text on the job card was reworded so it honestly describes
current behavior ("counts cached during normal enrichment are used
when available; otherwise the job queries a metadata source
directly") rather than the old "active provider first, then others
as fallback" phrasing, which doesn't match how the cache actually
fills — any enrichment worker that runs can populate it, and the
last writer wins. Document-only follow-up if this edge case ever
bites in practice: add a `api_track_count_source` column so the
scan can prefer the configured primary source's count over others
(e.g. deluxe vs. standard edition mismatches). Not worth the
complexity today.
For existing users, the first completeness scan after upgrade is
fast to the extent their library is already enriched: the workers
already ran and populated `api_track_count` on their normal schedule.
For brand-new installs, the scan's fallback path handles the cold
start — slower, but correct, and subsequent scans are fast.
Does NOT affect:
- Download / post-processing / wishlist / sync code paths — none
of them read `track_count` for completeness semantics.
- Plex / Jellyfin / Navidrome / standalone sync — still write
`track_count` exactly as before; `api_track_count` is a separate
column they never touch.
- Other repair jobs.
- Any UI path — same finding schema, just correct counts now.
Files:
- database/music_database.py — idempotent migration adding
`api_track_count INTEGER DEFAULT NULL` to the existing album-column
check block.
- core/worker_utils.py — new `set_album_api_track_count` helper with
the documented skip-on-bad-input contract.
- core/spotify_worker.py, itunes_worker.py, deezer_worker.py,
discogs_worker.py — one-liner call from each `_update_album`.
- core/repair_jobs/album_completeness.py — scan uses the cache;
fallback path persists API-lookup results via the shared helper;
help text updated to match actual behavior.
- tests/test_worker_utils_album_track_count.py — 9 tests covering
the helper's write/skip contract + no-commit invariant.
- tests/test_album_completeness_job.py — 2 tests for the repair
job's fallback-path wrapper.
- webui/static/helper.js — WHATS_NEW entry.
Credit: sassmastawillis spotted the bug; the "restored older DB
finds 7 albums" signal pinpointed DB state over code logic and
made the diagnosis tractable.
|
||
|
Broque Thomas
|
b3722449fc |
MusicBrainz: Fix artist images, total_tracks off-by-one, and Artist+Title queries
Three bugs from kettui's follow-up review pass on the MusicBrainz search PR, all fixed in one commit because they share UI context. 1. Missing artist images on MB artist results MusicBrainz doesn't store artist images directly. My earlier commit returned `image_url=None` on every artist result and trusted the frontend's lazy-loader — but the lazy-loader's `/api/artist/<id>/image? source=musicbrainz` endpoint had no handler for MusicBrainz, so it silently returned None and the emoji placeholder stayed. Fix plumbs the artist name through: - `renderCompactSection` stashes `data-artist-name` on artist cards. - `search.js` and `downloads.js` lazy-loaders pass `name=<artist>` as a query param. - `/api/artist/<id>/image` accepts an optional `name` param. - `metadata_service.get_artist_image_url` has a new `musicbrainz` branch: since MB has no artist art, it searches fallback sources (iTunes/Deezer by configured priority) for the artist name and returns the first image found. Verified live — Metallica/Kendrick Lamar/Daft Punk all resolve to Deezer artist images via the name lookup. 2. total_tracks off-by-one on tracks with a release `_recording_to_track` initialized `total_tracks = 1` and then summed media track-counts on top. For an 11-track album, it reported 12. An adapter-level regression introduced when the recording-projection helper was extracted during the main MB refactor. Fix: initialize at 0, sum normally. Standalone recordings with no release (can happen for uncredited remixes etc.) still report 1 via an explicit fallback — so the existing "single track" case isn't broken. 3. "Artist Album Title" queries buried specific albums in the discography list Bare-name queries like "The Beatles Abbey Road" used to resolve "The Beatles" as the artist and then browse their full discography — Abbey Road was buried alphabetically among 200+ releases instead of being the top result. Fix adds a title-hint extractor. When the query starts with the resolved artist name followed by more words, the trailing portion is treated as a title hint. Browse results are filtered to those whose release-group title contains the hint. If the filter matches nothing, falls back to text-search with the hint as the title (the "keep the old split-by-whitespace fallback" path kettui called for). If text- search also misses, shows the full discography rather than nothing. 10 new tests in tests/test_musicbrainz_search.py (46 total): - Title-hint extractor: basic match, case-insensitive, whitespace tolerance, bare-artist-no-hint, artist-not-prefix-no-hint, word- boundary required (no false splits on "Metallicasomething"). - Browse filtering by title hint. - Text-search fallback when the title hint matches nothing in browse. - Bare-artist queries return the full discography unfiltered. - total_tracks for single-release, multi-disc, and no-release cases. |
||
|
Broque Thomas
|
394ac73877 |
MusicBrainz: Tests for new search behavior + WHATS_NEW entry
26 new unit tests in tests/test_musicbrainz_search.py covering: - Cover Art URL construction (release + release-group scope, empty MBID, unknown scope fallback) - Structured query splitting (hyphen, en-dash, em-dash, bare name, no false-positive splits on hyphens-inside-words) - Artist search: score filtering, strict=False call contract, exception handling, genre extraction from MB tags, mbid/name validation - Top-artist resolver: memoization by normalized query, sub-threshold returns None, negative-result caching, empty-query short-circuit - Album search routing: bare query → browse path, structured query → text path, no-artist-match falls back to text, text path score filter - Track search routing: browse path, dedupe-by-title across live/compilation variants, structured query → text path, text path score filter All mock the underlying MusicBrainzClient — no network calls. Also adds a WHATS_NEW entry under 2.40 explaining the three user-visible changes: Artists section now populates, album/track results match the searched artist instead of random title collisions, and search completes in ~3 seconds instead of 30+. |
||
|
Broque Thomas
|
253e4d1e4a |
Fix Discover hero 'View Discography' 404ing on source-only artists
Clicking 'View Discography' on the Discover hero slideshow was calling
navigateToArtistDetail(id, name) without the third 'source' argument.
loadArtistDetailData then omits the `source` query param, so
/api/artist-detail falls through to a local DB lookup and returns 404
for artists that don't exist in the library — which is nearly every
hero artist, since they come from discover similar-artists.
Regression from the unification PR (
|
||
|
Broque Thomas
|
325292ce5a |
Treat Soulseek as configurable in source picker (require slskd_url)
Cin flagged that Soulseek was always rendered as configured in the source picker, even on dev instances with no slskd set up — letting users click it and fire searches that could never succeed. Three coordinated changes: 1. web_server.py SERVICE_CONFIG_REGISTRY: add Soulseek entry requiring `slskd_url`. /api/settings/config-status now reports its real state alongside every other service. 2. shared-helpers.js _ALWAYS_CONFIGURED_SOURCES: drop 'soulseek'. The set is now just MusicBrainz + YouTube Music Videos (sources that genuinely don't need user creds). Soulseek goes through the normal config-status code path. 3. shared-helpers.js openSettingsForSource: special-case Soulseek to route to Settings → Downloads tab (where slskd URL field lives, gated behind the download-source-mode dropdown) and scroll to the #soulseek-url input. Every other source still routes to Connections and scrolls to its .stg-service card. Without this, Soulseek's "click to configure" landed on a Connections card that doesn't exist (Soulseek's URL/key fields are scoped to the download-source selection on the Downloads tab). |
||
|
Broque Thomas
|
005c6ad73a |
Fix Soulseek handoff routing + stale-request flash on fast retype
Two AI-review findings from Cin (kettui) on the source-picker PR: 1. Soulseek handoff from global widget went through metadata flow _gsNavigateToSearchPage(query, 'soulseek') wrote the query into #enhanced-search-input and dispatched an input event. The Search page controller's activeSource was whatever its default was (spotify, deezer, etc.), so the debounced submitQuery ran the enhanced /api/enhanced-search flow instead of the raw Soulseek file search. The `src` parameter was effectively ignored. Fix: when src === 'soulseek', pre-fill #downloads-search-input directly and click the Search page's Soulseek icon. The icon click triggers the controller's onSoulseekSelected callback, which owns the section swap and re-runs performDownloadsSearch against the value we just wrote to the basic input. 2. Stale in-flight requests cleared loadingSources after fast retype createSearchController._fetchSource awaits the fetch result, then unconditionally mutates state.loadingSources / state.sources in the settle and catch blocks. When a user typed "abc" → fetch started → typed "abcd" before the first fetch returned, the second submitQuery aborted the first fetch and started its own. The first fetch's catch (AbortError) then ran and cleared loadingSources for that source — wiping the spinner the new request had just set, and causing a brief flash of empty/error state while the new fetch was still in flight. Fix: monotonic _requestSeq token. Each _fetchSource call captures the next value (++_requestSeq). Settle / catch blocks (and the YouTube NDJSON streaming loop) bail before mutating shared state if requestId !== _requestSeq. Existing abortCtrl behavior unchanged — this is a layered defense for the catch-clobber pattern that abort alone can't prevent. |
||
|
Broque Thomas
|
258644fd9f |
Drop Show/Hide Results button + auto-restore cached results on navigate-back
Cin flagged two related UX issues during PR review: 1. The "Show Results / Hide Results" toggle next to the search bar served no real purpose — there was nothing else on the Search page worth seeing instead of results, so toggling visibility was always pointless overhead. 2. Navigating away from /search via a sidebar link dismissed the dropdown (the click was caught by the outside-click handler). Coming back left the input populated but the results hidden, requiring a Show Results click or a fresh search. The cached state was intact in the controller the whole time — just not rendered. Both fixed by the same direction: dropdown visibility becomes a pure function of query state, never user-toggleable. The closure now exposes `_searchPageRestoreOnEnter` so subsequent calls to `initializeSearchModeToggle` re-render from the controller's cached state instead of early-returning. Removes the button HTML, click handler, `updateToggleButtonState` function, the desktop + responsive CSS for `.enhanced-search-btn`, and the orphaned `.btn-icon` rule. Net -94 lines. |
||
|
Broque Thomas
|
77d20e9aa8 |
Fix Clean Search History automation AttributeError on DownloadOrchestrator
The hourly `clean_search_history` automation was crashing with `'DownloadOrchestrator' object has no attribute 'base_url'`. The guard was written before the orchestrator refactor — `soulseek_client` is now a DownloadOrchestrator that wraps individual download clients, with the real Soulseek client sitting at `.soulseek`. Two other call sites in web_server.py (lines 2634, 3092) already used the correct `soulseek_client.soulseek.base_url` pattern with a getattr guard. This call site was missed during the refactor. Fix: reach through the orchestrator the same way the other sites do. |
||
|
Broque Thomas
|
9f63280677 |
Extract source-picker into shared createSearchController factory
Both the Search page and the global search widget ran the same source- picker state machine (query, activeSource, per-query cache, fallbacks, loading set, configured-source discovery, NDJSON streaming for YouTube, default-source fall-forward). That was ~380 lines of near-duplicated logic split across search.js and downloads.js, which meant every bug fix or behavior tweak had to land twice and inevitably drifted. createSearchController in shared-helpers.js now owns all of that. Each surface passes per-surface wiring — a source-row DOM element, a CSS class prefix, and callbacks for Soulseek handoff + unconfigured-source redirect — and consumes the controller's state via an onStateChange callback. The surface files shrink to their actual responsibilities: results rendering, click handlers, and surface-specific visibility. Zero UX change. Every keystroke, icon click, cache hit, rate-limit fallback, and unconfigured-source redirect behaves identically to before — verified via full pytest suite (395 passed) and node --check on all three files. WHATS_NEW entry added under the 2.40 unified-search bucket. |
||
|
Broque Thomas
|
553ede8de9 |
Update interactive help + What's New for the source-picker redesign
The existing 2.40 WHATS_NEW entry described the short-lived "Search from" dropdown that preceded this redesign. Updated to describe the icon row + per-query cache + rate-limit fallback banner + global widget parity that actually ships. Click-for-help annotations and the "First Download" tour now point at `#enh-source-row` (the new icon container) instead of the deleted `.search-source-picker-container` dropdown and the deleted `.enh-source-tabs` post-search tab bar. Adjusted the enhanced-search tips so "multi-source tabs compare results" doesn't mislead — the icons above the bar are how you compare now. Version stays at 2.39 — the 2.40 WHATS_NEW section is accumulating under the "Search & Artists unification" umbrella and will publish when the whole 2.40 cycle ships. |