- new soulseek.search_min_delay_seconds knob forces a gap between
consecutive searches; smooths the burst pattern that trips ISP
anti-abuse (Reddit report: Bell Canada cuts the WAN after rapid
peer-connection spikes) even when the existing 35/220 sliding-window
cap isn't hit
- throttle math lifted to a pure compute_search_wait_seconds helper so
the gate logic is testable independent of asyncio.sleep + the
singleton client
- new field on settings → connections → soulseek; default 0 = disabled
so existing users see no change
15 helper-boundary tests pin defaults / no-throttle, sliding-window
cap (legacy), min-delay (the new burst-smoother), max-of-both gates,
and defensive paths.
- music_source / spotify_connected / spotify_rate_limited were reading
a non-existent 'spotify' key on _status_cache and silently falling
through to the missing-value default (always 'unknown' / False).
Routed through the canonical accessors get_primary_source +
get_spotify_status now.
- added hydrabase_connected, youtube_available, hifi_instance_count,
and always_available_metadata_sources so the debug dump reflects
the full service surface
- removed a local re-import of get_spotify_status that was making
python 3.12 treat the name as function-scoped, breaking the new
lambda above it (NameError on free variable) — module-level import
already exists
11 endpoint-level tests pin music_source / spotify_* / hydrabase_* /
youtube_available / always_available_metadata_sources / hifi_instance_count
and the defensive fall-through paths when each lookup raises.
- new track_already_owned helper wraps db.check_track_exists at
the same confidence threshold the discography backfill repair job
uses (0.7) — name+artist+album, format-agnostic so blasphemy-mode
libraries (flac → mp3 + delete original) match correctly
- endpoint runs the check after the artist + content-type filters and
before add_to_wishlist, so a second discography click on the same
artist no longer re-queues every track that already downloaded
- per-album response carries a new tracks_skipped_owned counter
alongside the existing artist/content/wishlist skip categories
Discord report (Skowl).
- drop tracks where the requested artist isn't named in track.artists
(keeps features, drops compilation / appears_on contamination)
- honor watchlist.global_include_live/remixes/acoustic/instrumentals
the same way the discography backfill repair job already does
- surface per-album skip counts in the ndjson stream (artist mismatch
+ content filter) so the ui can show what was filtered
Closes#559.
GitHub issue #558: clicking Auto-Fill / Fix Selected on the Album
Completeness findings page returned a flat "Could not determine album
folder from existing tracks" error with no diagnostic. Reporter is on
Navidrome on Docker — the path resolver in
`core/library/path_resolver.py` couldn't find any of the album's tracks
on disk because Navidrome's Subsonic API doesn't expose filesystem
library paths the way Plex's API does (probed in #476). Default
settings → `library.music_paths` empty → no base directories to probe →
silent None. User had no signal about what to configure.
Not a regression of #476 — that fix targeted Plex auto-discovery and
worked correctly for it. Navidrome was never covered because the
protocol gives the resolver nothing to probe.
Fix scoped to the diagnostic surface, not auto-magic discovery:
- Added `resolve_library_file_path_with_diagnostic` returning
`(resolved, ResolveAttempt)`. ResolveAttempt records what the resolver
tried — `raw_path_existed`, `base_dirs_tried`, `had_config_manager`,
`had_plex_client`. Pure data, no rendering opinions.
- Legacy `resolve_library_file_path` becomes a thin wrapper that
drops the attempt; every existing call site is unchanged.
- `RepairWorker._fix_incomplete_album` now uses the diagnostic helper
and renders a multi-part error via `_build_unresolvable_album_folder_error`:
names the active media server, shows one sample DB-recorded path,
lists every base directory the resolver actually probed, and points
the user at Settings → Library → Music Paths as the actionable fix.
- Distinguishes empty-base-dirs vs tried-and-failed cases so the user
knows whether to add a mount or fix the existing one.
- No auto-probing of common Docker conventions (`/music`, `/media`, etc).
Speculative — could resolve to wrong dirs on the suffix-walk if a
conventional path happens to contain a partial collision. User stays
in control.
12 new tests:
- 7 in `tests/library/test_path_resolver.py`: tuple-shape contract,
raw-path-existed short-circuit, base-dirs listed even on walk
failure, had-flags reflect caller inputs, no-base-dirs returns
None with empty attempt, legacy `resolve_library_file_path`
delegates correctly across happy / suffix-walk / failure paths.
- 8 in `tests/test_repair_worker_unresolvable_folder_error.py`:
active server name in error, sample DB path verbatim, base dirs
listed, empty-base-dirs phrased differently, Settings hint always
present, defensive against None attempt / missing sample / missing
config_manager.
Full pytest sweep: 2774 passed.
Reported: Clear History button on the Import page left zombie rows
behind. Every survivor showed "⧗ Processing" status from 2-9 days ago.
Trace: `_record_in_progress` inserts a `status='processing'` row up-front
so the UI can render the in-flight import while it runs; `_finalize_result`
updates it to `completed`/`failed` when the import finishes. When the
worker is killed mid-import (server restart, crash), the row never gets
finalized — stays at `processing` forever. The clear-history endpoint's
SQL `DELETE ... WHERE status IN (...)` listed every terminal status but
omitted `processing`, so zombies survived every click.
Fix: add `processing` to the delete list, but guard against nuking
genuinely-live imports by intersecting against the worker's
`_snapshot_active()` map — any folder hash currently registered in
`_active_imports` is excluded from the delete via an `AND folder_hash
NOT IN (...)` clause. `pending_review` deliberately left out so user
still has to approve/reject those explicitly.
One endpoint touched (`/api/auto-import/clear-completed` in
web_server.py). No worker changes — guard reuses the existing
`_snapshot_active()` method that the UI poller already calls.
5 new tests in `tests/imports/test_auto_import_clear_completed_endpoint.py`:
- Zombie `processing` rows swept, live `processing` row preserved
(folder_hash currently in `_active_imports` survives)
- Response count matches actual delete count
- Empty active-set branch (unparameterized DELETE) — pinned because
an empty SQL `IN ()` would be a syntax error
- Worker-unavailable returns 500 (pre-existing guard not regressed)
- `pending_review` rows always survive — never auto-swept
Full pytest sweep: 2758 passed (one pre-existing flaky timing test
on `test_import_singles_parallel.py` failed under full-suite CPU load,
passes in isolation in 2.95s — unrelated to this change).
Discord report: 16 Bandcamp indie albums sat in staging because
auto-import couldn't identify them, but the manual search bar at the
bottom of the Import Music tab found the same albums fine. Trace:
`_search_metadata_source` only queried `get_primary_source()` — single
source, no fallback. Meanwhile `search_import_albums` (manual search bar)
already iterated `get_source_priority(get_primary_source())` and broke
on the first source with results. Asymmetric behavior, same album: manual
worked, auto-import didn't.
Fix: lift `_search_metadata_source` to use the same source-chain pattern.
Try primary first; if it returns nothing OR scores below the 0.4
threshold, fall through to the next source in priority order. First
source producing a strong-enough match wins. Result dict carries the
`source` that actually matched (not the primary name) so downstream
`_match_tracks` calls the right client. Defensive per-source try/except
so a rate-limited or auth-failed source doesn't abort the chain.
Unconfigured sources (client=None) silently skipped.
Cin-shape lift: scoring math extracted to pure `_score_album_search_result`
helper so the weight tweaks (album 50% / artist 20% / track-count 30%)
are pinned at the function boundary, independent of the orchestrator
(per-source iteration, exception containment, threshold check). Weight
constants exposed at module level (`_ALBUM_NAME_WEIGHT`,
`_ARTIST_NAME_WEIGHT`, `_TRACK_COUNT_WEIGHT`) — greppable, bumpable in
one place. Pre-extraction these were magic numbers inline.
27 new tests:
- 9 integration tests in `test_auto_import_multi_source_fallback.py`:
primary-success path unchanged (no fallback fires, only primary client
called), primary-empty falls through, primary-weak-score falls through,
first fallback success stops the chain (no wasted API calls on
remaining sources), all-sources-fail returns None, per-source
exception contained, unconfigured-source skipped, result `source`
field reflects winning source, `identification_confidence` from
winning source.
- 18 helper tests in `test_album_search_scoring.py`: weights sum to
1.0, album weight dominant (invariant pin), perfect-match returns 1.0,
per-component contribution (album / artist / track-count), Bandcamp
vs streaming track-count mismatch (7-files vs 4-tracks case still
scores ~0.87 above threshold), zero-track-count and zero-file
guards, huge-mismatch non-negative guard, list-of-strings artist
shape, missing `.name` / `.artists` / `None` total_tracks edge cases.
Backwards compatible: single-source users see no change — chain just
has one entry. Existing test `test_search_metadata_source_extracts_artist_id_from_dict_artist`
needed one extra patch line for `get_source_priority`.
Full pytest sweep: 2754 passed.
Two follow-ups to the multi-artist tag settings PR:
1. Deezer contributors upgrade — closes the "known limitation"
flagged in the prior commit. Deezer's `/search` endpoint only
returns the primary artist for each track; the full contributors
array (feat., remix collaborators, producers credited as artists)
lives on `/track/<id>` and gets parsed by `_build_enhanced_track`.
Without the upgrade Deezer-sourced tracks never got multi-artist
tags even with the right settings on.
Fix in `core/metadata/source.py`: when source==deezer AND the
search response had a single artist AND a track_id is available,
fetch full track details via `get_deezer_client().get_track_details`
and replace `all_artists` with the upgraded list.
- One extra API call per affected Deezer track
- Skipped when search already returned multiple (no-op fast path)
- Skipped for non-Deezer sources (Spotify/Tidal/iTunes search
responses already include all artists)
- Skipped when no track_id is available
- Defensive try/except: on /track/<id> failure (network error,
deezer client unavailable), fall through to the search-result
list — never lose the data we already had
2. Double-append guard hardened with a word-boundary regex.
Prior commit checked for `"feat." not in title.lower() and "(ft."
not in title.lower()` — too narrow. Source platforms produce
wildly different feat-marker conventions: "(feat. X)", "(Feat X)",
"(FEAT X)", "(Featuring X)", "[feat. X]", "ft. X" (no parens),
"FT. X", etc. Any of these as the SOURCE title would cause a
double-append: `"Track (Feat X) (feat. Y)"`.
Replaced with `re.search(r'\b(?:feat|feat\.|featuring|ft|ft\.)\b',
title, IGNORECASE)`. Word-boundary regex catches every common
variant. Substring matches like "Aftermath" containing `ft`
correctly fall through to the append path (pinned by a regression
test).
16 new tests (29 total in the file):
- 9 parametrized variants of the double-append guard
- 1 substring guard ("Aftermath")
- 6 Deezer upgrade scenarios (fires when expected, doesn't fire
for non-Deezer / multi-artist search / no track_id, defensive
fall-through on failure, no false-positive when /track/<id>
confirms single artist)
Full pytest 2727 passed.
Three settings on Settings → Metadata → Tags were partially or
completely unimplemented. Reporter (Netti93) traced each one.
(1) `write_multi_artist` only "worked" because of a never-populated
`_artists_list` field. `core/metadata/source.py` built
`metadata["artist"]` as a hardcoded ", "-joined string but never
assigned `metadata["_artists_list"]`. `core/metadata/enrichment.py`
line 107 reads that field and gates the multi-value tag write
on `len(_artists_list) > 1` — always saw an empty list, silently
no-op'd the write.
(2) `artist_separator` (default ", ") was referenced in the UI +
settings.js save path but ZERO Python code read the value. Every
multi-artist track ended up with hardcoded ", " regardless of
what the user picked.
(3) `feat_in_title` (when true: pull featured artists into the title
as " (feat. X, Y)" and leave only primary in the ARTIST tag —
Picard convention) had no implementation at all.
Fix in source.py:
* Populate `_artists_list` from the search response's artists array
* Read `feat_in_title` and `artist_separator` configs
* When `feat_in_title=True` and >1 artist: ARTIST = primary only,
append "(feat. X, Y)" to title with double-append guard
* Else: ARTIST = artists joined with `artist_separator`
* Single-artist case unaffected by either setting
Double-append guard uses a word-boundary regex catching all common
"feat" variants source platforms produce — `feat`, `feat.`,
`featuring`, `ft`, `ft.` — case-insensitive. Substring matches
(e.g. "Aftermath" containing "ft") correctly fall through to the
append path.
Fix in enrichment.py ID3 branch:
* TPE1 stays as the display string (with separator or primary-only
per the user's settings)
* Multi-value list goes to a separate `TXXX:Artists` frame (Picard
convention) when `write_multi_artist` is on
* Pre-fix the ID3 path wrote TPE1 twice — single-string then list
— and the second `add` overwrote the first, clobbering both the
configured separator AND the feat_in_title semantics. Vorbis path
was already correct (separate "artist" + "artists" keys).
Known limitation (flagged in WHATS_NEW): Deezer's `/search` endpoint
only returns the primary artist. The full contributors array lives
on `/track/<id>`. Enrichment uses search-result data so Deezer-
sourced tracks may still get only the primary artist until a follow-
up commit wires the per-track contributors fetch into the enrichment
flow. Spotify, Tidal, and iTunes search responses include all
artists so they work now.
23 new tests in `tests/metadata/test_multi_artist_tag_settings.py`:
* `_artists_list` populated for multi/single/no-artist cases
* `artist_separator` drives ARTIST string (default ", " + custom
";" + custom "; " + " & ")
* Single-artist case unaffected by either setting
* `feat_in_title=True` pulls featured to title, leaves primary in
ARTIST
* `feat_in_title` no-op for single artist
* Double-append guard recognizes 9 source-title variants ("(feat.
X)", "(Feat. X)", "(FEAT X)", "(feat X)", "(Featuring X)",
"[feat. X]", "ft. X", "(ft X)", "FT. X")
* Substring guard test pins "Aftermath" doesn't false-positive
* Combined-settings precedence: feat_in_title wins ARTIST string
but `_artists_list` carries everyone for multi-value tag
Full pytest 2711 passed.
Track enrichment was stuck in a constant retry loop. Logs showed
nothing but `Read timed out. (read timeout=10)` from
`lookup_track_by_id` repeating against the same track ID. AudioDB
itself was being hammered nonstop with no progress.
Cause: when an entity already has `audiodb_id` populated (from a
manual match or earlier scan) but `audiodb_match_status` is still
NULL — an inconsistent state some import paths can leave behind —
the worker tries a direct ID lookup. If that lookup fails (returns
None on timeout, which AudioDB's `track.php` endpoint hits
frequently because it's slow), the prior code logged "preserving
manual match" and returned WITHOUT marking status. Row stayed NULL
→ queue's NULL-status filter picked it up next tick → tried direct
lookup → timed out → returned → infinite loop.
The "preserve manual match" intent was correct: don't fall through
to the name-search path because that could overwrite a manually-set
`audiodb_id` with a wrong guess. Bug was the missing `_mark_status`
call before the early return.
Fix:
* `_process_item` direct-lookup-failure branch now calls
`_mark_status(item_type, item_id, 'error')` before returning. The
existing `audiodb_id` is preserved (column not touched). Queue's
NULL-status filter no longer re-picks the row.
* `_get_next_item` retry-cutoff queue priorities (4/5/6) extended
from `audiodb_match_status = 'not_found'` to
`audiodb_match_status IN ('not_found', 'error')`. Same `retry_days`
window. Transient AudioDB outages still recover automatically;
permanently-broken IDs eventually get re-attempted once a month
rather than staying errored forever.
5 new tests in `tests/test_audiodb_worker_stuck_track.py` use a real
SQLite DB (not mocks) so the SQL queries are actually exercised:
- lookup-returns-None marks status='error' (no infinite loop)
- lookup-raises-exception marks status='error' (defensive)
- lookup-success preserves the existing match-success path
- error-status row past retry-cutoff gets picked up again
- error-status row within cutoff stays skipped (loop prevention
works)
Only triggers for entities in the inconsistent `audiodb_id` set +
`match_status` NULL state. Happy path and already-matched /
already-not-found rows unchanged. Full pytest 2698 passed.
Closes#553.
Discord report: container refused to start after pulling latest.
Logs showed `mkdir: cannot create directory '/app/Staging':
Permission denied`. `set -e` in entrypoint.sh then aborted the script
and the container restart-looped.
Root cause traced to commit 70e1750 (2026-05-08, image-bloat fix):
the Dockerfile chown was changed from `chown -R /app` to a scoped
chown on specific subdirs to avoid a redundant layer that was
duplicating the entire /app tree. Side effects:
1. `/app` itself went from soulsync:soulsync (via the recursive walk)
to root:root (Docker WORKDIR default — never re-chowned).
2. `/app/Staging` was the only runtime mount-point dir NOT pre-baked
into the image — every other bind-mountable dir (config, logs,
downloads, Transfer, MusicVideos, scripts) was in the Dockerfile's
`mkdir -p` + `chown` list. Staging was left to the entrypoint.
On rootless Docker / Podman where in-container "root" maps to a host
UID, the entrypoint mkdir on `/app/Staging` could fail with EACCES
depending on the bind-mount path's host ownership.
Fix has three parts:
1. **Dockerfile** — added `/app/Staging` to the runtime mkdir +
scoped chown list. Closes the asymmetry with the other bind-
mountable dirs. Image now ships with the directory pre-baked
owned soulsync:soulsync so the entrypoint mkdir is a guaranteed
no-op even when bind-mount perms are weird.
2. **entrypoint.sh mkdir + chown** — both now have `|| true` so any
future bind-mount permission quirk surfaces as a log line, not
a `set -e` crash + restart loop. Previously only the chown had
the `|| true` suffix; mkdir was bare.
3. **entrypoint.sh writability audit** — new loop at the end of
the setup phase runs `gosu soulsync test -w "$dir"` against
every bind-mountable dir. When a dir isn't writable by the
soulsync user, logs a loud warning with the exact host-side
`chown` command needed to fix it. Catches the underlying bind-
mount perm issue that the restart-loop fix would otherwise mask
(container starts but auto-import / downloads write into
unwritable dirs and fail silently). This is the diagnostic that
would have surfaced the root cause without needing the user to
share a container-restart screenshot.
Zero behavior change for users whose containers were already
starting fine. Defensive against the rootless/podman config that
broke after the image-bloat refactor.
Verified shell syntax with `bash -n entrypoint.sh`. Full pytest
2693 passed (no Python touched).
Two-part fix to the Your Albums "Download Missing" flow on Discover.
Part A — UX redesign
The prior `downloadMissingYourAlbums()` ran a per-album loop that
fired direct-download tasks via `openDownloadMissingModalForYouTube`.
Reported as silently failing — "Queuing 2/2" toast with no actual
transfer activity. Even when downloads worked, bypassing the
wishlist meant no retry / dedup / rate-limit / source-fallback
handling.
Replaced with a selectable-grid modal mirroring the Download
Discography pattern from the library page. Click the download
button → opens a checkbox grid showing every missing album (cover,
title, artist, year, track count, source) → user picks what they
actually want → click "Add to Wishlist" → each album's tracks get
resolved + queued through the existing wishlist auto-download
processor. NDJSON progress stream renders ✓/✗ per album.
New JS helpers:
- `_openYourAlbumsBatchModal(missingAlbums)` — builds the modal
- `_renderYourAlbumsBatchCard(row, index)` — per-album card
- `_yourAlbumsBatchSelectAll(select)` — bulk toggle
- `_updateYourAlbumsBatchFooterCount()` — live count + button text
- `_closeYourAlbumsBatchModal()` — overlay teardown
- `_startYourAlbumsBatchAddToWishlist()` — submit handler, NDJSON
progress consumer
- `_yourAlbumsPickSource(album)` — picks the single best source-id
per row (priority: spotify → deezer → tidal → discogs)
Reuses the `.discog-*` CSS classes from the library Download
Discography modal — no new CSS. Reuses the existing
`/api/artist/<id>/download-discography` endpoint. The endpoint's URL
artist_id param is functionally unused (per-album payload carries
everything — verified by reading the endpoint body), so the modal
posts with placeholder `your-albums` and gets multi-artist
resolution for free without backend changes.
Part B — Tidal album resolution
Reported as the original bug: clicking download on Tidal-only albums
did nothing because `/api/discover/album/<source>/<album_id>` had no
`tidal` branch and `tidal_client` had no `get_album_tracks` method.
`core/tidal_client.py`: new `get_album_tracks(album_id, limit=None)`
method. Two-phase: cursor-walk
`/v2/albums/<id>/relationships/items?include=items` for track refs +
position metadata (`meta.trackNumber` + `meta.volumeNumber`),
batch-hydrate via existing `_get_tracks_batch` for artist/album
names. Returns `Track` objects with `track_number` and `disc_number`
attached. Sort by (disc, track) so multi-disc compilations render in
album order.
`web_server.py`: new `'tidal'` source branch in
`/api/discover/album/<source>/<album_id>`. Resolves album metadata
via `get_album`, tracks via `get_album_tracks`, cover art via inline
`?include=coverArt` lookup. Same response shape as Spotify/Deezer
branches.
`webui/static/discover.js`:
- `tidal_album_id` added to `trySources` for the single-album click
flow (`openYourAlbumDownload`)
- Same source picker drives the new batch modal
- Virtual-id generation includes `tidal_album_id` so Tidal-only
albums get stable identifiers across discover-album-* / your-
albums-* contexts
10 new tests in `tests/test_tidal_album_tracks.py` pin:
- Single-page walk + hydration
- Multi-page cursor chain
- Multi-disc sort order (disc 1 → 2 in track order each)
- `limit` short-circuit at page boundary
- No-token short-circuit (no API call)
- HTTP error returns empty
- 429 raises (propagates to `rate_limited` decorator for retry)
- Forward-compat type filter (skips non-track entries)
- Partial-batch hydration failure containment
- Empty-album short-circuit (no batch call)
Full pytest: 2693 passed.
Follow-up to the prior compilation-album scanner fix. That patch
made the scanner read `tracks.track_artist` (per-track artist
column) via COALESCE so compilation tracks would compare against
the right value. But tracks downloaded BEFORE the `track_artist`
column existed have track_artist=NULL — COALESCE falls back to
album artist (the curator) and the wrong-comparison case returns.
Fix: explicit 3-tier resolution in `_scan_file`:
1. DB `tracks.track_artist` if populated → trust it. Respects
manual edits from the enhanced library view (user who curated
the DB value but didn't re-tag the file gets their edit
respected, not overridden by stale file tag).
2. File's ARTIST tag via mutagen if present → use it. Tidal /
Spotify / Deezer all write the per-track artist into the
audio file at download time regardless of SoulSync's DB
schema, so it's ground truth even when the DB column is
stale or NULL. File is already open for fingerprinting so
mutagen tag-read is essentially free.
3. Album artist → final fallback for files without proper ARTIST
tags AND no DB track_artist. Existing pre-fix behavior.
`_load_db_tracks` SELECT now surfaces `track_artist` (raw, may be
empty/NULL via NULLIF) and `album_artist` separately in addition
to the COALESCE'd `artist` field — so `_scan_file` can tell the
difference between 'DB has a curated value' and 'DB fell back to
album artist'. Without this distinction, the file-tag fallback
would create false positives when DB is curated but file is stale.
5 new tests (11 total in the file) pin:
- File-tag-trumps-DB resolves the legacy NULL case (DB says
'Andromedik' (album curator), file says 'Eclypse', AcoustID
says 'Eclypse' → no flag)
- Tag-missing falls back to album artist (preserves existing
genuine-mismatch contract — file without tag + AcoustID
mismatch still flags)
- Mutagen exception swallowed (debug log, fall-through)
- File-tag matches DB → no behavioral change
- DB curated value trumps stale file tag (false-positive guard
— user edited DB without re-tagging file shouldn't get flagged)
Two existing test fixtures (`_make_context` callers) updated to
the new 10-column row shape.
SQL behavior verified empirically against real SQLite: NULL and
empty-string both flow through NULLIF → None in Python →
file-tag-fallback path. Modern populated values trump file tag.
Discord: Discover → Your Albums (and Your Artists) was returning
nothing for Tidal users regardless of how many albums/artists they'd
favorited. Audit found `get_favorite_albums` and `get_favorite_artists`
called the deprecated `/v2/favorites?filter[type]=ALBUMS|ARTISTS`
endpoint — that endpoint returns 404 for personal favorites because
it's scoped to collections the third-party app created itself. The
V1 fallback (`/v1/users/<id>/favorites/...`) is also dead because
modern OAuth tokens carry `collection.read` instead of the legacy
`r_usr` scope V1 demands (returns 403).
Same root cause as the favorited-tracks fix from #502.
Fix: rewire to the working V2 user-collection endpoints —
`/v2/userCollectionAlbums/me/relationships/items` and
`/v2/userCollectionArtists/me/relationships/items` — using the
same cursor-paginated pattern shipped for tracks.
Architecture:
* ID enumeration lifted into a generic
`_iter_collection_resource_ids(path, expected_type, max_ids)`
helper so tracks / albums / artists all share one walker. Three
thin wrappers preserve the per-resource public surface
(`_iter_collection_track_ids`, `_iter_collection_album_ids`,
`_iter_collection_artist_ids`). Net deduped ~80 lines that would
otherwise be three near-identical copies.
* Batch hydration via `/v2/{albums|artists}?filter[id]=...&include=...`
with extended JSON:API include semantics. One request returns up
to 20 albums + their artists + cover artworks all in `included[]`
(or 20 artists + their profile artworks). Three static helpers
parse the response:
- `_build_included_maps(included)` → indexes the array by type
so per-resource lookup is O(1) per relationship ref
- `_first_artist_name(rels, artists_map)` → resolves primary
artist from relationships block; '' on missing/unknown
- `_first_artwork_url(rel, artworks_map)` → picks `files[0]`
(Tidal returns artwork files largest-first, so this gets the
highest-resolution variant — typically 1280×1280)
* Public methods (`get_favorite_albums`, `get_favorite_artists`)
preserve the prior return shape — list of dicts matching what
`database.upsert_liked_album` / `upsert_liked_artist` consume —
so the discover aggregator path in `web_server.py` stays
byte-identical. No caller changes needed.
* Deleted ~240 lines of dead code: the V2-favorites paths AND the
V1 fallback paths from the old method bodies. Both are dead
against modern OAuth tokens.
24 new tests in `tests/test_tidal_favorite_albums_artists.py` pin:
* Cursor-walker dispatch (album/artist iters pass correct path +
expected_type to the generic walker)
* Included-map building (groups by type, skips items missing id)
* Artist + artwork relationship resolution (full + missing rels +
unknown id + no files cases)
* Batch hydration parse for albums (full attributes, missing
relationships fall through to defaults, type-filter excludes
non-album entries, `filter[id]` param is comma-joined)
* Batch hydration parse for artists (same shape coverage)
* End-to-end orchestrator behavior (walk → batch → return,
empty-input short-circuits without API call, BATCH_SIZE chunking
on 41 IDs → 20/20/1, exception-from-iter returns [])
Endpoint paths empirically verified against live Tidal API:
`userCollectionArtists/me/relationships/items` returned 200 + 5
real artist refs for the test account. `userCollectionAlbums/...`
returned 200 + empty (account has 0 album favorites currently)
but the response shape is correct. The deprecated
`/v2/favorites?filter[type]=ALBUMS` returned 404. The V1
`/v1/users/<id>/favorites/albums` returned 403 with explicit
"Token is missing required scope. Required scopes: r_usr" message.
WHATS_NEW entry under existing '2.5.1' block.
Full pytest: 2678 passed.
Discord report (CJFC, 2026-04-26): syncing a Spotify playlist to the
server overwrote anything manually added to the server-side playlist.
The fix adds a per-sync mode picker next to the Sync button on the
playlist details modal — Replace (default, current delete-recreate
behavior) or Append only (preserves existing tracks, only adds new
ones). Useful when the source platform caps playlist size and the
user is manually building beyond it on the server.
Implementation:
* New `append_to_playlist(name, tracks)` method on Plex / Jellyfin /
Navidrome clients. Each uses the server's NATIVE append API:
- Plex: `existing_playlist.addItems(new_tracks)`
- Jellyfin: `POST /Playlists/<id>/Items?Ids=...&UserId=...`
- Navidrome: Subsonic `updatePlaylist?songIdToAdd=...`
Falls back to `create_playlist` when the playlist doesn't exist
yet (first sync). No delete-recreate, no backup playlist created
(preserves playlist creation date + metadata + non-soulsync-managed
tracks).
* Dedup-by-server-native-id (ratingKey for Plex, GUID for Jellyfin,
song-id for Navidrome) — never re-adds a track already on the
playlist. Server-native identity, not fuzzy title+artist match,
so it can't false-collide.
* `sync_service.sync_playlist` accepts `sync_mode='replace'|'append'`
kwarg. Single if/else branch dispatches to `append_to_playlist` or
`update_playlist`. Threaded through `core/discovery/sync.run_sync_task`
and the `/api/sync/start` HTTP handler. Validation on the API rejects
unknown mode strings (defaults to 'replace').
* Frontend: per-playlist `<select id="sync-mode-${id}">` rendered next
to the Sync button in both modal renderers (sync-spotify.js for
Spotify playlists, sync-services.js for Deezer ARL playlists).
`startPlaylistSync` reads the select at click time; missing select
(other callers like discover.js) defaults to 'replace' so backward
compat preserved without per-call-site updates.
* SoulSync standalone has no playlist methods at all and the modal
hides the Sync button entirely on it via `_isSoulsyncStandalone` —
dispatch never reaches that path, no defensive fallback needed.
15 new tests pin per-server append behavior:
- missing playlist → create_playlist delegation
- dedup filtering (existing IDs skipped, only new tracks added)
- empty new-track set short-circuits without API call
- failure paths return False without raising
- contract listing (KNOWN_PER_SERVER_METHODS includes
'append_to_playlist'; Plex / Jellyfin / Navidrome all implement)
Plus tests/discovery/test_discovery_sync.py fake `sync_playlist`
fixture got `sync_mode='replace'` default to match the new signature
(was breaking after the kwarg add; now passing).
WHATS_NEW entry under new '2.6.0' block (hidden by
`_getLatestWhatsNewVersion` until next release bump).
Closes CJFC discord request.
Adds the user's Tidal favorited tracks ("My Collection" in the Tidal
app) as a virtual playlist alongside their real playlists, mirroring
how Spotify's "Liked Songs" is treated.
Reporter (yug1900) located the working endpoint after the prior
`/v2/favorites?filter[type]=TRACKS` attempt returned empty data —
that endpoint is scoped to collections the third-party app created
itself, not personal favorites. Real endpoint:
GET /v2/userCollectionTracks/me/relationships/items
?countryCode=US&locale=en-US&include=items
Cursor-paginated (20 per page, follow `links.next` with
`page[cursor]=...` until exhausted). Response only carries
track-level attributes — artist + album NAMES come back as
relationship-link stubs, not embedded data.
Implementation:
* Two-phase fetch — `_iter_collection_track_ids` walks the cursor
chain to enumerate every track id (cheap, IDs only), then
`get_collection_tracks` batch-hydrates 20 IDs at a time through
the existing `_get_tracks_batch` helper which already knows how
to `include=artists,albums`. No duplication of the JSON:API
artist/album parse, no new dataclass shape.
* Virtual playlist `tidal-favorites` appended to the end of
`/api/tidal/playlists`. ID intentionally has no colon —
sync-services.js renderer interpolates IDs into CSS selectors
via template literals (`#tidal-card-${p.id} .foo`) and a `:`
would parse as a CSS pseudo-class operator.
* `tidal_client.get_playlist("tidal-favorites")` recognizes the
virtual id and dispatches to the collection path internally, so
every per-id consumer gets it for free: detail endpoint, mirror
auto-refresh automation, "build Spotify discovery from Tidal
playlist" flow.
OAuth scope expansion:
* Added `collection.read` to both OAuth flows (the
`core/tidal_client.py::authenticate` standalone path AND the
`web_server.py::auth_tidal` web flow — they were independent
scope strings that both needed updating).
* Added `prompt=consent` to both flows — without it Tidal silently
returns a token carrying only the ORIGINAL scope set even after
re-authentication, because Tidal treats the existing
authorization as still valid.
* New `disconnect()` method + `POST /api/tidal/disconnect`
endpoint + Disconnect button next to Authenticate in Settings →
Connections → Tidal — required for users whose existing token
predates the scope expansion (forces a clean grant).
Reconnect-needed UI hint:
* `_collection_needs_reconnect` flag set on 401/403 from the
collection endpoint, cleared on next successful walk, NOT set
on 5xx (transient server errors must not falsely tell the user
to reconnect).
* Listing endpoint reads the flag and surfaces a placeholder card
titled "Favorite Tracks (reconnect Tidal to enable)" with a
description pointing at Settings, so the user has something
visible to act on instead of a silently missing row.
Diagnostic logging — collection request URL + response status +
first 300 bytes of body now logged at info level so future "why
is my collection empty" reports can be diagnosed from app.log
without needing live reproduction.
22 new tests pin: cursor walk (full chain, max-ids cap mid-page +
at page boundary), auth gates (no token / 401 / 403 all bail
clean), reconnect-flag lifecycle (set on 401/403, cleared on next
successful walk, NOT set on 5xx), forward-compat type filter
(non-track entries skipped), count helper, batch hydration
delegation + chunking at the 20-per-batch cap, partial-batch
failure containment, virtual-id dispatch (real playlist ids still
flow through the normal path).
Closes#502.
Phase B of foxxify discord report. Pre-#524 manual-import bug left
some albums in the library with `artist=Unknown Artist` and `album.title
= <numeric album_id>`. Reorganize couldn't place them (no usable
metadata source ID) and emitted a generic "run enrichment first" hint
that doesn't apply — enrichment can't fix these rows. The right tool
is the existing `Fix Unknown Artists` repair job (reads file tags,
re-resolves metadata, re-tags + moves files).
Discoverability gap, not a logic gap. Reorganize now detects the bad-
metadata shape (Unknown Artist OR album.title that's a 6+ digit
numeric id) and emits a clear "run the Fix Unknown Artists repair
job" hint at both reason-emit sites (planner + executor). No
duplication of fixer logic.
WHATS_NEW entry covers both Phase A (orphan-format sibling handling,
already committed in d944a16) and Phase B since they ship in the same
PR for the same reporter.
20 new tests pin helpers + reason routing.
Discord report (Skowl): downloaded a compilation album ("High Tea
Music: Vol 1") where every track has a different artist (Eclypse,
Andromedik, T & Sugah, Gourski, etc.) and the AcoustID scanner
flagged every single track as Wrong Song. The file tags had the
correct per-track artist (e.g. "Eclypse" for "City Lights"), but
the scanner compared against the album-level artist ("Andromedik",
the curator). Raw similarity 12% → Wrong Song flag.
# Why the prior multi-value fix didn't help
Foxxify's case (just-merged PR): AcoustID returned multi-value
credit "Okayracer, aldrch & poptropicaslutz!" — primary IS in the
credit. Splitting found it.
Skowl's case: both sides single-value but DIFFERENT artists.
Splitter has nothing to find — Eclypse simply isn't in "Andromedik".
Different bug.
# Cause
Scanner SQL at `core/repair_jobs/acoustid_scanner.py:281` joined
the `artists` table via `tracks.artist_id` which points at the
ALBUM artist (the curator/label-name applied to every row in a
compilation). The `tracks.track_artist` column already holds the
correct per-track artist for compilations — populated by every
server-scan path (Plex `originalTitle`, Jellyfin `ArtistItems`,
Navidrome per-track `artist`) AND the auto-import / direct-download
post-process flow (`record_soulsync_library_entry` writes it when
different from album artist). Scanner just wasn't reading it.
# Fix
```sql
SELECT t.id, t.title,
COALESCE(NULLIF(t.track_artist, ''), ar.name) AS artist,
...
```
Prefers per-track artist when populated, falls back to album artist
for legacy rows / single-artist albums where `track_artist` is NULL.
`NULLIF(t.track_artist, '')` handles the empty-string-instead-of-null
case some legacy rows might have.
# Composes with Foxxify's multi-value fix
For the rare compilation track where AcoustID ALSO returns a
multi-value credit (e.g. compilation track has multiple credited
performers), both paths work together — `track_artist` gives the
correct expected primary, then the helper splits the credit and
finds it.
# Tests added (2)
- `test_load_db_tracks_prefers_track_artist_for_compilation` —
reporter's exact case: track with `track_artist='Eclypse'` AND
`artist_id` pointing at album artist 'Andromedik' resolves to
'Eclypse'. Second track with NULL `track_artist` falls back to
album artist 'Andromedik' (single-artist + legacy compat).
- `test_load_db_tracks_falls_back_when_track_artist_empty_string`
— empty string in `track_artist` (some legacy rows) → NULLIF
returns NULL → COALESCE falls back to album artist.
Both use a real SQLite DB so the COALESCE/NULLIF logic + JOIN
runs against actual schema (SimpleNamespace fakes can't simulate
JOINs).
# Verification
- 6/6 scanner tests pass (2 new + 4 existing)
- 2586 full suite passes (+2 from prior commit)
- Ruff clean
Discord report (Foxxify): the AcoustID scanner repair job flagged
multi-artist tracks as Wrong Song because AcoustID returns the
FULL credit ("Okayracer, aldrch & poptropicaslutz!") while the
library DB carries only the primary artist ("Okayracer"). Raw
SequenceMatcher similarity scored ~43% — well below the 60%
threshold — so the scanner created a finding even though the
audio was correct. User couldn't fix without lowering the global
artist threshold to ~30% (which would let real mismatches through).
# Fix
Extended the shared `core/matching/artist_aliases.py::artist_names_match`
helper (originally lifted for #441) with credit-token splitting.
When the actual artist string contains common separators —
- punctuation: `,` `&` `;` `/` `+`
- keywords (whitespace-bounded): `feat.` `ft.` `featuring` `with`
`vs.` `x`
— the helper splits into individual contributors and checks each
against the expected artist. Primary-in-credit cases now resolve
at 100% instead of 43%.
Two pattern groups because punctuation separators don't need
surrounding whitespace, but keyword separators MUST be
whitespace-bounded — otherwise we'd split artists with `x` /
`with` etc. in their names ("JAY-X" → "JAY-" / "" issue).
Composes with the existing alias path: cross-script multi-artist
credits ("Hiroyuki Sawano" expected, "澤野弘之, FeaturedJp"
actual) work via alias-token-against-credit-token compare.
# Wire-in
Scanner at `core/repair_jobs/acoustid_scanner.py:202` replaces
the raw `SequenceMatcher` call with `artist_names_match`. Pass
RAW artist strings (not pre-normalised by `_normalize`) so the
splitter can recognise separators — `_normalize` strips ALL
punctuation, which destroyed the very tokens the splitter needs.
The AcoustID post-download verifier (`core/acoustid_verification.py`)
already routes through `_alias_aware_artist_sim` which calls the
same helper — gets the multi-value benefit automatically without
a separate wire-in.
# New `split_artist_credit` exported helper
Pure-function helper for callers who want token-level access to
the credit list (debugging, UI, future per-token enrichment). Same
splitter logic, exposed as a top-level function.
# Tests added (14)
`tests/matching/test_artist_aliases.py` (+11):
- `TestSplitArtistCredit` — parametrised across 12 credit-string
formats (comma, ampersand, semicolon, slash, plus, feat./ft./
featuring, with, vs., x, single-token, empty), drops empty
tokens, strips per-token whitespace
- `TestMultiValueCreditMatching` — reporter's exact case
(Okayracer in 3-artist credit → 100%), primary in middle/end of
credit, genuine-mismatch still fails, single-token actual falls
through to direct compare, multi-value composes with aliases,
threshold still respected
`tests/test_acoustid_scanner.py` (+3):
- Reporter's case end-to-end through `_scan_file` — fingerprint
99% / title 100% / multi-artist credit → no finding created
- Genuine artist mismatch still creates finding (no false
suppression of real mismatches)
- `JobResultStub` minimal scaffold for the integration tests
# Verification
- 14 new tests pass (49 helper + 5 scanner total in their files)
- 110 matching + scanner tests pass total
- 2584 full suite passes (+25 from baseline 2559)
- Ruff clean
- Reporter's exact case (Okayracer in `Okayracer, aldrch &
poptropicaslutz!`) now scores 100% match → no Wrong Song flag
Discord report (Tim): downloaded cover art via Deezer metadata
source came out visibly blurry in Navidrome / on phones — large
displays exposed the limited resolution.
# Cause
Deezer's API returns `cover_xl` URLs at 1000×1000. The underlying
CDN actually serves up to 1900×1900 by rewriting the size segment
in the URL path (same trick the iTunes mzstatic + Spotify scdn
upgrades already use). SoulSync wasn't doing the rewrite — every
Deezer-sourced cover got embedded at 1000×1000 regardless of how
much higher resolution the CDN had available.
# Verified empirically
```
$ for size in 1000 1400 1800 1900 2000; do curl -I "...{size}x{size}-..."; done
1000: 200 OK 106 KB
1400: 200 OK 198 KB
1800: 200 OK 331 KB
1900: 200 OK 371 KB
2000: 403 Forbidden
```
1900 is the safe ceiling. Above that the CDN returns 403. CDN
serves source-native bytes when source < target (smaller-source
albums get same bytes whether we ask for 1000 or 1900), so asking
for 1900 universally is safe.
# Fix
New `_upgrade_deezer_cover_url(url, target_size=1900)` helper in
`core/deezer_client.py`. Pure function, mirrors the
`_upgrade_spotify_image_url` pattern that already lives in
`core/spotify_client.py`. Defensive on every input shape:
- Empty / None → returned as-is
- Non-Deezer URL (no `dzcdn`) → returned as-is
- No size segment in URL → returned as-is
- Already at/above target → returned as-is (idempotent, never
downgrades)
Applied at both cover-download sites:
- `core/metadata/artwork.py::download_cover_art` — auto post-process
flow. Mirrors the existing iTunes mzstatic upgrade right above it.
- `core/tag_writer.py::download_cover_art` — enhanced library view's
"Write Tags to File" feature.
# Scope discipline
- Helper applied at the DOWNLOAD boundary, not the source extraction
point in `deezer_client.py`. Means cached entries in the metadata
cache + DB row `image_url` columns keep the original 1000×1000 URL
Deezer's API returned. Future CDN behavior changes only affect the
download path, not stored data.
- Pre-existing `prefer_caa_art` toggle (Settings → Library →
Post-Processing) untouched — orthogonal workaround for users who
want even higher quality (MusicBrainz Cover Art Archive, often
3000×3000+).
- iTunes / Spotify upgrade paths untouched — they already worked.
# Tests added (16)
`tests/metadata/test_deezer_cover_url_upgrade.py`:
- Standard upgrade: default target 1900 on cover URL, alternate
dzcdn host (`e-cdns-images.dzcdn.net` vs `cdn-images.dzcdn.net`),
artist picture URLs (same path pattern), 500×500 source upgrades
too
- Custom target size: smaller target = no-op (never downgrade),
larger target works
- Idempotent: already at/above target returned unchanged
- Defensive on non-Deezer URLs: parametrised across 5 hosts
(Spotify scdn, iTunes mzstatic, MB CAA, Last.fm, random) — all
returned untouched
- Defensive on malformed Deezer URL (no size segment) → returned
as-is
- Empty / None handling
# Verification
- 16/16 helper tests pass
- 560/560 metadata + imports tests pass (no regression)
- 2559 full suite passes
- Ruff clean
# Bug
Plex servers with the music library named anything other than "Music"
(Música, Musique, Musik, Musica, 音乐, موسيقى, etc.) hit this error
after every import cycle:
soulsync.plex_client - ERROR - Failed to trigger library scan
for 'Music': Invalid library section: Music
soulsync.web_scan_manager - ERROR - Failed to initiate PLEX
library scan via web
Side effect: `wishlist.processing` kept reporting "Missing from
media server after sync" for tracks that DID import correctly, so
they got perpetually re-added to the wishlist.
# Root cause
`_find_music_library` correctly auto-detects the music section by
`section.type == 'artist'` and stores it on `self.music_library` —
works for any locale because the type is language-neutral. Read
methods (`get_artists`, etc.) route through `_get_music_sections`
which returns `[self.music_library]`, so they never had the bug.
But `trigger_library_scan` and `is_library_scanning` ignored
`self.music_library` and called
`self.server.library.section(library_name)` directly with the
hardcoded `"Music"` default. `server.library.section('Music')`
raises `NotFound` on any server whose section isn't literally
named "Music".
# Fix
Both methods now prefer `self.music_library` first, fall back to
literal `library_name` lookup only when auto-detection hasn't
populated the cached reference (test fixtures, edge cases).
`is_library_scanning`'s activity-feed match also corrected to
filter by the resolved section's actual title — the prior code
matched `library_name.lower() in activity_title.lower()` which
defaults to "music" and would never match activities for
non-English sections.
`trigger_library_scan`'s success log line now surfaces the actual
section title (`Música`) instead of the unused `library_name`
default ("Music") — confusing when debugging on non-English servers.
# Tests added (13)
`tests/media_server/test_plex_non_english_section_name.py`:
- `test_uses_auto_detected_section_regardless_of_locale` — parametrised
across 6 locale variants (Música, Musique, Musik, Musica, 音乐, موسيقى).
Each verifies trigger_library_scan calls the auto-detected
section's `update()`, NOT a literal-name fallback. Stub raises
AssertionError on `server.library.section()` so a regression that
re-introduces the fallback fails loudly.
- `test_falls_back_to_literal_lookup_when_no_auto_detection` —
backward compat: music_library=None → literal lookup as before.
- `test_explicit_library_name_arg_used_only_when_no_auto_detection` —
auto-detected wins over explicit kwarg when both available.
- `test_logs_correct_section_label_on_success` — log line surfaces
resolved section title.
- 4 symmetric tests for is_library_scanning covering refreshing-attr
check, activity-feed title match, no-match for unrelated sections,
fallback path.
# Verification
- 13 new tests pass
- 84/84 media_server tests pass (no regression in the existing
Plex / Jellyfin / Navidrome suite)
- 2458 full suite passes (+13 from baseline)
- Ruff clean
Live-API verification revealed advanced-syntax queries hurt more
than they help on this endpoint. Switching the import-modal Deezer
search back to free-text + local rerank.
# What live testing showed
Hit Deezer's public API with both query forms for the issue #534
case (`Dirty White Boy` + `Foreigner`):
**Free-text (`q=Dirty White Boy Foreigner`):**
- Returns 21 results
- Real Foreigner Head Games studio cut at #1
- Live versions at #2-10
- Karaoke / cover variants at #11-15
**Advanced (`q=track:"Dirty White Boy" artist:"Foreigner"`):**
- Returns 12 results
- "(2008 Remaster)" at #1 — canonical Head Games cut MISSING from
top 8 entirely
- Live + alt-album versions follow
Advanced syntax DOES filter karaoke at the API level (none in the
12-result set vs. 5 at positions 11-15 in free-text), but it has
its own ranking bias that surfaces remasters / "Best Of" cuts
ahead of the canonical recording. Net regression for the user-
facing goal.
# Fix
1. Endpoint reverts to free-text query with local rerank applied.
2. Local rerank gains "remaster" / "remastered" / "reissue"
patterns under VARIANT_TAG_PATTERNS (soft 0.4× penalty — user
may want them but they shouldn't outrank the original).
3. Client kwarg support (`track=` / `artist=` / `album=`) preserved
for future opt-in callers (e.g. exact-match flows where API-
level filtering matters more than ranking).
# Verified end-to-end against live Deezer API
Re-ran the exact #534 case through the live API + new rerank.
Top 15 results post-rerank:
1. Dirty White Boy — Foreigner — Head Games ← REAL CUT AT TOP
2-10. Various Live versions
11-15. Karaoke / cover / tribute variants ← BURIED
Real Foreigner Head Games studio cut at #1, exactly the user's
ask.
# Tests
- `test_relevance.py` — variant tag patterns extended; existing
tests still pass (50 tests).
- `test_search_match_endpoints.py::test_joins_track_and_artist_into_free_text_query`
— replaces `test_passes_track_and_artist_as_kwargs`; verifies
endpoint sends free-text join, NOT field-scoped kwargs (the
prior test asserted the wrong direction now).
- Karaoke-burying assertion at the endpoint still pins the
user-visible behaviour.
- Client kwarg path tests untouched (still pin advanced-syntax
construction for future opt-in callers).
# Verification
- 75 relevance + endpoint + query tests pass
- 2445 full suite passes
- Ruff clean
- Live Deezer API shows real cut at #1 post-rerank
Defensive followup to the relevance fix. Deezer's advanced search
syntax (`artist:"X"`) is documented as substring match, but in
practice it's brittle on artist name variants ("Foreigner [US]",
"The Foreigner") and on tracks indexed under non-canonical title
spellings. When the advanced query returns nothing, we'd previously
land at "No matches" — a regression vs. pre-fix behaviour where
free-text would have returned a less-relevant but non-empty set.
Fix: when the advanced query returns 0 results AND the caller used
field-scoped kwargs, fall back to a free-text join of the same
kwargs and re-query. Caller-side rerank still tightens whatever the
fallback returns, so the worst-case post-fix behaviour is the
pre-fix behaviour — never strictly worse.
Pulled the cache + parse + store dance into a private helper
(`_search_tracks_with_query`) so the orchestration can call it
twice (advanced → fallback) without code duplication. Single API
call when the advanced query has results — no wasted requests.
Diagnostic logger.debug fires when the fallback triggers so we can
see in production whether it's happening (and to which queries).
# Tests added (4)
- `test_falls_back_to_free_text_when_advanced_empty` — advanced
query returns 0, free-text returns hits; client returns the
free-text hits + both API calls fire.
- `test_no_fallback_when_advanced_query_has_results` — single hit
on advanced query → no second API call.
- `test_no_fallback_when_legacy_free_text_call` — legacy callers
already exhausted the only path; empty result is final.
- `test_no_fallback_when_query_unchanged` — empty kwargs path
doesn't trigger the fallback branch (used_advanced=False).
# Existing tests updated
The 4 prior `TestSearchTracksQueryWiring` + `TestSearchTracksCacheKey`
tests were stubbing `_api_get` to return empty `{'data': []}` and
asserting `assert_called_once`. With the new fallback, those stubs
trigger a second API call and the assertions break — even though
the FIRST call construction is what the tests cared about. Updated
the stubs to return one fake hit so the fallback doesn't fire, and
switched to `call_args_list[0]` for first-call inspection.
# Verification
- 18/18 deezer query tests pass (14 prior + 4 new)
- 2445 full suite passes (+4 from prior commit)
- Ruff clean
# Background
User reported (#534) that the import-modal "Search for Match" dialog
returned irrelevant results when Deezer was the metadata source.
Searching `Dirty White Boy` + `Foreigner` returned 5+ karaoke /
"originally performed by" / "in the style of" / "re-recorded" /
tribute-band results ranked above the actual Foreigner studio cut
from Head Games. User had to scroll past the junk every time, or
fall back to iTunes search which is much slower.
# Root cause — two layers
1. **Endpoint joined `track + artist` into free-text query.**
`/api/deezer/search_tracks` was passing `q=Dirty White Boy Foreigner`
to Deezer's `/search/track` API. Deezer fuzzy-matches that
string across title / lyrics / artist / album / contributors and
orders by global popularity — anything that appears across many
compilations outranks the canonical recording.
2. **No local rerank.** None of the search-modal endpoints applied
any post-filtering. Deezer's API order shipped straight to the
user.
# Fix — same architectural shape Cin would build
## Layer 1: field-scoped query at the client boundary
`core/deezer_client.py::search_tracks()` now accepts optional
`track`, `artist`, `album` kwargs. When provided, builds Deezer's
advanced search syntax: `q=track:"X" artist:"Y" album:"Z"`. Massive
relevance improvement because each term matches the right field
instead of fuzzy-matching everywhere.
Backward compat preserved: legacy free-text `query=` callers still
work unchanged. Field-scoped path takes precedence when both are
provided. Empty input fast-fails without an API call. Embedded
double-quotes stripped (Deezer's syntax has no escape mechanism).
## Layer 2: provider-neutral relevance reranker
New `core/metadata/relevance.py` module — pure-function rerank over
the canonical `Track` dataclass. Composable scoring:
- **Cover/karaoke patterns** (multiplier 0.05, effectively buries):
matches "karaoke", "originally performed by", "in the style of",
"made famous by", "tribute", "vocal version", "backing track",
"cover version", "re-recorded", "cover by", etc. across title,
album, AND artist fields. Catches the screenshot's exact junk:
artist credits like "Pop Music Workshop" / "The Karaoke Channel"
/ "Foreigner Tribute Band".
- **Variant tags** (multiplier 0.4): live / acoustic / demo /
instrumental / remix / radio edit / club mix etc. — softer
penalty since the user MAY want them. Skipped entirely when the
expected_title contains the same tag (so searching
"Track (Live)" still ranks Live versions first).
- **Exact artist boost** (multiplier 1.5): primary artist exactly
matches expected_artist after normalisation. Single strongest
signal for "this is the canonical recording".
- **Title + artist similarity** via SequenceMatcher (parentheticals
+ punctuation stripped before comparison).
- **Album-type weighting**: album=1.0 > single/ep=0.85 > compilation=0.7.
Compilations are more likely tribute / karaoke repackages.
Each component is a standalone function so tests pin them
individually without standing up the full pipeline.
## Wired at three search-modal endpoints
- `/api/deezer/search_tracks` — uses both layers (field-scoped
query + rerank).
- `/api/itunes/search_tracks` — uses rerank only (iTunes API has
no advanced-syntax search, but karaoke / cover variants still
leak through and need the local penalty).
- `/api/spotify/search_tracks` — already builds field-scoped
`track:X artist:Y` query; rerank added as the consistency safety
net so all three sources behave the same from the user's
perspective.
Other Deezer call sites (matching engine, watchlist scanner,
auto-import single-track ID) deliberately not touched in this PR
— they have their own elaborate scoring pipelines tuned to their
specific contexts and aren't surfacing the user-reported issue.
Per Cin: "don't refactor beyond what the task requires."
# Tests
71 new tests across 3 files:
- `tests/metadata/test_relevance.py` (50 tests) — every scoring
component pinned individually + the issue #534 screenshot
reproduced as a regression test (real Foreigner cut wins after
rerank, karaoke variants drop to bottom).
- `tests/metadata/test_deezer_search_query.py` (14 tests) —
advanced-syntax query construction, field-scoped wiring at the
client boundary, free-text path unchanged, kwargs win when
ambiguous, limit clamping, cache key consistency.
- `tests/imports/test_search_match_endpoints.py` (7 tests) —
end-to-end through Flask test client: Deezer endpoint passes
kwargs not joined query; karaoke buried at bottom for all three
sources; legacy query param still works without rerank.
# Verification
- 2441 full suite passes (+71 from baseline 2370)
- 0 failures (the prior watchdog flake fix held)
- Ruff clean across all changed files
- JS parses clean (`node -c webui/static/helper.js`)
# Architectural standards followed
- **Logic at the right boundary.** Query construction lives in the
client (every caller benefits from one change). Rerank lives in
a neutral module (`core/metadata/relevance.py`) over the
canonical `Track` dataclass — works for any source, not Deezer-
specific.
- **Explicit > implicit.** Every scoring rule has its own named
function. Pattern tables are module-level constants tests can
introspect.
- **Scope discipline.** Audited every Deezer search call site;
fixed the user-reported one + the consistent siblings. Did NOT
speculatively normalise every Deezer call across the codebase.
- **Backward compat.** Free-text `query=` callers untouched. Kwargs
added to existing client method signature with safe defaults.
- **Tests pin contract at correct boundary.** Pure-function rerank
tests don't mock anything; client-query tests stub at `_api_get`;
endpoint tests run through the real Flask app.
Two pre-existing parity gaps in `record_soulsync_library_entry` that
the prior parity commits left untouched. Both close real holes
between auto-import writes and what the soulsync_client deep scan
would have produced.
# Gap 1: Album duration was the first-imported track's duration
`record_soulsync_library_entry` is called once per track. The album
INSERT only fires for the FIRST track of a new album (subsequent
tracks find the album row already exists). The INSERT was passing
`duration_ms` — `track_info["duration_ms"]` — as the album's
`duration` column. That's the duration of one track, not the album
total. Compare to `SoulSyncAlbum.duration` in soulsync_client which
is `sum(t.duration for t in self._tracks)`.
Fix:
- Worker computes `album_total_duration_ms = sum(...)` across every
matched track and threads it onto context as
`album.duration_ms`.
- side_effects reads that value (or falls back to the per-track
duration for legacy non-auto-import callers) and writes it as the
album row's `duration`.
# Gap 2: Re-imports of the same artist/album were insert-only
When the SELECT-by-id or SELECT-by-name found an existing soulsync
artist or album row, the function skipped completely — no UPDATE
path. Meant: artist genres / thumb / source-id reflected ONLY
whatever the FIRST imported album supplied, never refreshing as
more albums by that artist landed. Ten more imports later, the
artist row still held whatever the first random import wrote.
Conservative fix: when an existing row matches, run an UPDATE that
fills only the columns whose current value is NULL or empty. Never
overwrites populated values — protects manual edits +
enrichment-worker writes the same way the scanner UPDATE path
preserves enrichment columns.
Implementation note: the empty-check happens in Python, NOT SQL.
Initial pass tried `COALESCE(NULLIF(col, ''), NULLIF(col, 0), ?)`
but SQLite's `NULLIF(text_col, 0)` returns the original text value
instead of NULL — different types, no coercion. So the SQL-only
conditional was unreliable on text columns. New helper does
`SELECT cols FROM table WHERE id`, compares each column in Python,
and emits UPDATE clauses only for the ones that need filling.
Allowlist defense: f-string column names go through
`_SOULSYNC_FILLABLE_COLUMNS` validation before interpolation.
Misuse adding new columns without an allowlist update fails closed
(logger.debug + skip).
# Tests added (4)
- `test_album_duration_uses_album_total_not_single_track` —
album with single-track context carrying explicit
`album.duration_ms = 2_500_000` writes 2_500_000 to the album row,
not the per-track 200_000 fallback.
- `test_re_import_fills_empty_artist_fields` — first import lands
artist with empty thumb + empty genres; second import for same
artist with thumb + genres present updates the existing row.
- `test_re_import_does_not_clobber_populated_artist_fields` —
first import writes rich genres + thumb; second import with
worse / different metadata leaves the existing row untouched.
- `test_re_import_fills_empty_source_id_when_missing` — first
import had no source artist ID; second import does — fills the
empty `spotify_artist_id` column on the existing row.
# Verification
- 10/10 side-effects tests pass (including 4 new + 4 from prior
parity commit + 2 history/provenance)
- 217 imports tests pass (no regression)
- 2369 full suite passes (+4 from prior, +22 PR-total from baseline 2347)
- 1 pre-existing flake (`test_watchdog_warns_about_stuck_workers`,
passes in isolation, unrelated)
- Ruff clean
Cin pre-review followup. Two small parity gaps the prior commits left
open:
# 1. Genre tags land on the standalone artists row
`soulsync_client._scan_transfer` aggregates the GENRE tag across every
track in an album and surfaces it on `SoulSyncAlbum.genres` (which the
DatabaseUpdateWorker writes to the artists+albums row). Auto-import
was hardcoding `'spotify_artist': {'genres': []}` so the imported
artists row landed with empty genres — felt hollow compared to a
Plex/Jellyfin scan, which both pull genres from their respective APIs.
Fix:
- `_read_file_tags` now reads the GENRE tag (mutagen easy mode handles
MP3/FLAC/M4A consistently; some files carry multiple genres so it's
always returned as a list).
- `_process_matches` aggregates genres from each matched file's tags
into a deduped insertion-order list. Dedup is case-insensitive but
preserves original casing — so "Hip-Hop, Rap, Trap" reads naturally
in the JSON column instead of "hip-hop, rap, trap".
- Worker context's `spotify_artist['genres']` carries the aggregated
list, which `record_soulsync_library_entry` already filters via
`core.genre_filter.filter_genres` and writes to the artists row.
# 2. Defensive str() cast for ISRC + MBID
`_build_album_track_entry` already coerces ISRC + MBID to string today
(via `str(isrc) if isrc else ''`). But if a future metadata-source
client returns int / None for either ID, the worker would propagate
the wrong type and side_effects.py's `.strip()` would AttributeError.
Cheap insurance: explicit `str()` cast in the worker before assignment
to track_info. Future-proofs against client drift.
# Tests added (3, in test_auto_import_context_shape.py):
- `test_context_aggregates_genres_from_track_tags` — multi-file
album with overlapping genre lists produces deduped, insertion-
ordered, original-case-preserved result. Stubs `_read_file_tags`
with monkeypatch so we don't need real audio.
- `test_context_genres_empty_when_no_tags` — files without GENRE
tag → empty list. Standalone library write handles gracefully
(genres column stays empty / NULL).
- `test_context_isrc_mbid_coerced_to_string` — hostile types
(int 12345678, None, int 999) coerced to safe strings before
reaching track_info.
# Verification
- 14/14 context-shape tests pass (11 prior + 3 new)
- 213 imports tests pass (no regression)
- 2365 full suite passes (+3 from prior, +18 PR-total)
- 1 pre-existing flake (`test_watchdog_warns_about_stuck_workers`,
passes in isolation)
- Ruff clean
Cin pre-review followup to the standalone library parity commit. The
prior commit fixed `spotify_artist['id']` from the wrong copy-paste
value (`identification['album_id']`) to read from
`identification['artist_id']`, but the identification dict produced
by `_search_metadata_source` and `_search_single_track` never set
`artist_id` — both extracted artist NAME from the search response
and discarded the source ID sitting right next to it. Net effect of
the prior commit: artists row source-id stayed NULL, just for a more
honest reason than before.
Now properly extracted:
- `_search_metadata_source` reads `best_result.artists[0]['id']`
alongside the artist name and returns it on the identification dict
as `artist_id`.
- `_search_single_track` does the same for single-track identification.
- `_identify_single`'s tag-based-confidence path forwards
`result.get('artist_id')` so the artist source-id propagates even
when high-confidence local tags override the search result's name.
Result: identification dict now carries `artist_id` whenever the
metadata source returned an artist with an ID. The worker context
already plumbs it onto `spotify_artist['id']` and
`spotify_album['artists'][0]['id']`, so the standalone library write
finally populates `<source>_artist_id` on the artists row.
Tests added (3, in `test_auto_import_context_shape.py`):
- `test_context_artist_id_uses_identification_artist_id` — when the
identification dict carries `artist_id`, context propagates it
onto `spotify_artist['id']` AND
`spotify_album['artists'][0]['id']`. Pins that the prior copy-
paste bug (artist['id'] = album_id) doesn't return.
- `test_context_artist_id_is_empty_when_identification_missing_it` —
fallback case (filename-only identification): context gets empty
string, NOT album_id. Honest failure mode.
- `test_search_metadata_source_extracts_artist_id_from_dict_artist`
— black-box test of `_search_metadata_source`: feed it a
spotify-shaped result with `artists[0]['id']` and verify
identification dict carries it forward.
Verification:
- 11/11 context-shape tests pass (8 prior + 3 new)
- 210 imports tests pass (no regression)
- 2362 full suite passes (+3 from prior commit, +15 PR-total)
- 1 pre-existing flake (`test_watchdog_warns_about_stuck_workers`,
passes in isolation)
- Ruff clean
# Background
SoulSync standalone is meant to be a full replacement for Plex /
Jellyfin / Navidrome — files imported via auto-import (or any other
import path) should land in the database with the same field richness
a media-server scan would write. They weren't.
# Gaps fixed
The auto-import worker built a context dict for each track and handed
it to `_post_process_matched_download` (the same callback the regular
download flow uses). That dict was missing three things downstream
needed:
1. **No `source` field anywhere.** `record_soulsync_library_entry`
reads `get_import_source(context)` to pick the source-aware ID
columns (`spotify_track_id` / `deezer_id` / `itunes_track_id` /
etc.) on the artists / albums / tracks rows. With no source, the
resolver returned an empty string → `get_library_source_id_columns("")`
returned an empty dict → the `UPDATE tracks SET <source>_id = ?`
blocks were silently skipped. Result: every auto-imported track
landed with NULL on every source-id column. Watchlist scans
(which match by stable source IDs to detect "this track is already
in library") couldn't recognise these rows and would re-download
them on the next pass.
2. **No `_download_username='auto_import'`.** Both
`record_library_history_download` and `record_download_provenance`
default to "Soulseek" when no `username` is in the context. Every
staging-folder import was being labelled as a Soulseek download
in library history + provenance — false signal in the UI.
3. **No per-recording IDs (`isrc`, `musicbrainz_recording_id`) on
track_info.** The Navidrome scanner already writes
`musicbrainz_recording_id` directly to the tracks row when present.
Picard-tagged libraries always carry MBID; metadata sources
(Spotify via MusicBrainz enrichment, Deezer, etc.) carry ISRC.
Auto-import had access to both via the metadata-source response
but didn't propagate them — so the soulsync row went in with
NULL on both columns.
# Changes
**`core/auto_import_worker.py` — `_process_matches`:**
- Top-level `'source': source` (from `identification['source']`)
- `'_download_username': 'auto_import'`
- `track_info['isrc']`, `track_info['musicbrainz_recording_id']` —
pulled from the per-track payload returned by the metadata source
- `track_info['album_id']` — back-reference so source-aware ID
resolution works on sources whose API nests album under
`track.album.id` rather than `track.album_id`
- `spotify_artist['id']` now correctly carries the artist's source ID
(was `identification['album_id']`, a copy-paste bug from the
original implementation that made artist-id resolution fall back
to fuzzy matching)
- `spotify_album['artists'][0]['id']` carries artist source ID for
the same resolution path
**`core/imports/side_effects.py`:**
- `record_library_history_download` source_map: add
`"auto_import": "Auto-Import"` — tags imported tracks correctly
- `record_download_provenance` source_service: add
`"auto_import": "auto_import"` — provenance shows real source
- `record_soulsync_library_entry` track INSERT: now includes
`musicbrainz_recording_id` + `isrc` columns (matches
`insert_or_update_media_track`'s shape for Navidrome /
Plex / Jellyfin scans). Both default to NULL when not present.
# Behavior preserved
- Files still land in the same library template path (no path-build
change)
- Other media-server flows (Plex / Jellyfin / Navidrome users)
unaffected — `record_soulsync_library_entry` still gates on
`get_active_media_server() == "soulsync"`. Auto-import on those
servers continues to drop the file in the library folder + emits
`batch_complete` for the scan-trigger automation, same as before.
- Direct downloads (search → Download button) unaffected — they
already passed `source` + `username` correctly.
# Tests added
`tests/imports/test_auto_import_context_shape.py` (8 tests, new file):
- Worker context carries `source` for every metadata source
(parametrised across spotify / deezer / itunes / discogs)
- `_download_username='auto_import'` set unconditionally
- ISRC + MBID propagate from track payload to track_info when present
- ISRC + MBID default to empty string when absent (downstream
normalises to NULL at write time)
- track_info includes album-id back-reference
`tests/imports/test_import_side_effects.py` (4 new tests + 2 schema
column adds):
- `record_soulsync_library_entry` writes mbid + isrc columns when
present in track_info
- Deezer source maps to deezer_id column (regression case for
source-aware column resolver)
- `record_library_history_download` labels `_download_username=
'auto_import'` as "Auto-Import" not "Soulseek"
- `record_download_provenance` registers source_service as
"auto_import" not "soulseek"
# Verification
- 8/8 new context-shape tests pass
- 6/6 side-effects tests pass (4 new + 2 existing)
- 207 imports tests pass
- 2359 full suite passes (+12 from baseline 2347, no regressions)
- 1 pre-existing flake (`test_watchdog_warns_about_stuck_workers`,
passes in isolation, unrelated to this change)
- Ruff clean
Per the semver workflow the version string only bumps at release
time, so the running dev work on the 2.4.3 line should stay listed
under 2.4.3 (not pre-create a 2.4.4 block). Merged the prior
'2.4.4' key's six dev entries into the top of '2.4.3', above the
existing "May 8, 2026 — 2.4.3 release" date marker, with a
"Unreleased — 2.4.3 patch work" date marker so the visual split
between unreleased + released entries is preserved.
`_getLatestWhatsNewVersion` resolves to the current build version
(2.4.3 in `_SOULSYNC_BASE_VERSION`); with the 2.4.4 key gone, the
helper modal now surfaces the dev work alongside the released
entries when the user opens "What's New", instead of being silently
hidden until a future build bump.
The release-time bump remains the canonical step that splits
"unreleased" entries off into their own version block — done as
the last commit on dev before merging dev → main.
No code changes — pure WHATS_NEW reorganisation.
# Concurrency model
Pre-refactor concurrency was emergent + unbounded:
- The worker's `_run` thread called `_scan_cycle` every 60s,
processing candidates synchronously in a for-loop.
- The `/api/auto-import/scan-now` endpoint spawned a fresh
`threading.Thread(target=_scan_cycle)` per click — extra parallel
scan cycles on top of the timer.
- Multiple "Scan Now" clicks during in-flight processing → multiple
threads racing on `_processing_paths` / `_folder_snapshots` state,
no upper bound on concurrent scanners.
- `stop()` didn't wait for in-flight processing — could leave file
moves / tag writes / DB inserts mid-flight.
Refactor to the pattern Cin uses elsewhere (`missing_download_executor`,
`sync_executor`, `import_singles_executor` all use
`ThreadPoolExecutor(max_workers=3, thread_name_prefix=...)`):
- **One scan thread** — both timer + manual triggers go through
`trigger_scan()`, gated by a non-blocking `_scan_lock`. Duplicate
triggers no-op instead of stacking parallel scanners.
- **Bounded executor** — `ThreadPoolExecutor` (default 3 workers,
configurable via `auto_import.max_workers`) runs per-candidate
work. Each candidate runs to completion in its own pool thread;
up to N candidates run in parallel.
- `_scan_and_submit()` is fast — just enumeration + executor submit,
returns immediately, doesn't block on per-candidate work.
- `_process_one_candidate(candidate)` holds the per-candidate logic
identical to the old for-loop body, lifted into a method so the
pool can run multiple instances concurrently.
- `_submitted_hashes` set + lock dedupes candidates across the
timer + manual triggers so a candidate already queued / running
doesn't get re-submitted.
- `stop()` calls `executor.shutdown(wait=True)` — clean shutdown,
no orphaned file ops.
# Per-candidate UI state isolation
The executor refactor opened two concurrency holes that the old
sequential model masked. Both fixed in this commit:
1. **Scalar UI fields stomped across pool workers.** Pre-refactor
`_current_folder` / `_current_status` / `_current_track_*` were
safe under the sequential model — only one candidate processed
at a time, so the fields tracked the in-flight one. With three
pool workers writing the same fields, the polling UI saw garbage
like "Processing AlbumA, track 7/14: SongFromAlbumB".
Replaced with `_active_imports: Dict[hash, _ActiveImport]` keyed
on folder_hash, gated by `_active_lock`. Each pool worker owns
its own entry. Helpers `_register_active` / `_update_active` /
`_unregister_active` / `_snapshot_active` are the only API.
2. **Stats counters not thread-safe.** `self._stats[k] += 1` is
read-modify-write — under load, parallel pool workers drop
increments. New `_stats_lock` + `_bump_stat()` helper wraps every
mutation. `get_status()` reads under the same lock and returns
a copy.
# Endpoint change
`/api/auto-import/scan-now` no longer spawns its own scan thread —
calls `auto_import_worker.trigger_scan()` (which routes through the
shared lock + executor). Multiple clicks while a scan is in flight
no-op deterministically. Endpoint still wraps the call in a daemon
thread so the HTTP response returns immediately even if the staging
walk is slow.
# Backward compat
The scalar `_current_folder` / `_current_status` / `_current_track_*`
fields are preserved as **read-only properties** that resolve to the
FIRST active import. The existing `get_status()` payload still
includes those fields populated from the first entry — single-import
UIs (and the test fixture) keep working unchanged. New
`active_imports` array exposes the full multi-candidate state for
parallel-aware UIs.
# Behavior preserved
- Per-candidate identify / match / process logic byte-identical
- Live-progress state preserved (per candidate now)
- Stability gate / already-processed dedup preserved
- `_record_in_progress` / `_finalize_result` UI rows preserved
- Tag-based loose-file grouping unchanged
# Behavior changes
- Multiple albums process IN PARALLEL up to `max_workers`
- "Scan Now" while scan in progress no-ops (was: spawned another)
- `stop()` waits for in-flight pool work via `shutdown(wait=True)`
- Auto-import card now lists each in-flight album (one line per
active import) instead of a single shared progress line
# UI
`webui/static/stats-automations.js`:
- Progress widget reads `active_imports` array, renders one line
per in-flight album with per-candidate status / track index
- Falls back to the legacy summary line when payload doesn't
carry `active_imports` (older backend)
- Per-row "live processing" lookup now matches by `folder_hash`
through the array instead of by `folder_name` against scalars
# Tests added (`tests/imports/test_auto_import_executor.py`)
- Pool config: default max_workers=3, configurable via constructor
+ via `auto_import.max_workers` config, floors at 1
- Scan lock: 5 concurrent `trigger_scan()` calls run only 1 scan
while lock held; releases properly so subsequent triggers run
- Executor dispatch: 5 candidates → 5 process calls via the pool
- Bounded parallelism: max_workers=3 caps at 3 concurrent;
max_workers=2 caps at 2
- Cross-trigger dedup: candidate submitted in scan A doesn't get
re-submitted by scan B while still in-flight
- Graceful shutdown: `stop()` blocks until in-flight pool work
finishes
- Per-candidate state isolation: 2 parallel workers updating their
own candidate state don't interfere — each candidate's
track_index / track_name / folder_name reads back exactly as
written for that hash
- `get_status()` returns coherent `active_imports` array with
one entry per in-flight candidate; aggregate top-level
`current_status` is 'processing' when any entry is processing
- Unregister removes only that candidate, others stay visible
- Stats counter thread-safety: 1000 parallel bumps land at 1000
(the read-modify-write race regresses without the lock)
- `get_status()` stats snapshot is a copy, not a live reference
# Verification
- 17 new tests pass (executor + state isolation)
- 2347 full suite passes (1 pre-existing flaky test —
`test_watchdog_warns_about_stuck_workers` — passes in isolation,
unrelated)
- Ruff clean
Brings the auto-import matcher to picard / beets / roon parity by
reaching for the existing AcoustID-grade infrastructure (typed Album
foundation, integrity check thresholds) and layering id-based exact
matches on top of the fuzzy scorer. Picard-tagged libraries now land
every track with full confidence on the first pass.
Three layered phases in `core/imports/album_matching.match_files_to_tracks`:
1. **MBID exact match** — file has `musicbrainz_trackid` tag, source
returns the same id → instant pair, full confidence, no fuzzy
scoring. Picard's primary identifier; per-recording.
2. **ISRC exact match** — file has `isrc` tag, source returns the same
id → same fast-path, slightly lower priority than mbid (isrc can
be shared across remasters). Both ids normalised before compare
(uppercase + strip dashes/spaces for isrc, lowercase for mbid).
3. **Duration sanity gate** — files in the fuzzy phase whose audio
length differs from the candidate track's duration by more than
`DURATION_TOLERANCE_MS` (3s, matching the post-download integrity
check) are rejected before scoring runs. Defends against the
cross-disc / cross-release / wrong-edit problem the integrity
check used to catch only AFTER the file had already been moved +
tagged + db-inserted.
Tag reader (`_read_file_tags`) extended:
- Reads `isrc` (uppercased, strip / / spaces normalisation deferred
to matcher)
- Reads `musicbrainz_trackid` as `mbid` (lowercased)
- Reads `audio.info.length` and converts to `duration_ms` to match
the metadata-source convention
Metadata-source layer (`_build_album_track_entry`) extended:
- Propagates `isrc` from top-level OR `external_ids.isrc` (spotify
shape — would otherwise be stripped before reaching the matcher)
- Propagates `musicbrainz_id` from top-level OR `external_ids.mbid`
/ `external_ids.musicbrainz`
- Without this layer, fast paths would silently never fire in
production even though unit tests pass — pinned by
`test_album_track_entry_propagates_isrc_and_mbid_from_source`
18 new tests in `tests/imports/test_album_matching_exact_id.py`:
- Direct: `find_exact_id_matches` with mbid, isrc, isrc normalisation,
mbid > isrc priority, spotify-shape `external_ids.isrc`, no-id
empty result, file-used-at-most-once
- Direct: `duration_sanity_ok` within / outside tolerance, missing
durations defer
- End-to-end via `match_files_to_tracks`: mbid match short-circuits
fuzzy scoring, id-matched files excluded from fuzzy phase, duration
gate rejects wrong-disc collisions in fuzzy phase, normal matches
pass through the gate, missing durations fall through, deezer
seconds-vs-ms conversion, full picard-tagged 10-track album via
mbid only
- Production-shape: `_build_album_track_entry` propagates isrc + mbid
from spotify-shape (`external_ids.isrc`) AND itunes-shape (top-
level `isrc`)
Verification:
- 35 album-matching tests pass total (17 helper + 18 fast-path)
- 23 multi-disc tests still pass after the extension (additive)
- Full suite: 2311 passed (+18 new), 1 pre-existing flaky timing test
failure (`test_watchdog_warns_about_stuck_workers` — passes in
isolation, fails only in full-suite runs, unrelated to this PR)
- Ruff clean
For users:
- Picard / Beets / Mp3Tag-tagged libraries (anyone who's organised
their music) get instant perfect-confidence matches every time.
- Soulseek-tagged downloads (which usually carry isrc when sourced
via metadata-aware soulseekers) get the fast path too.
- Naively-named files with no useful tags fall through to the
improved fuzzy + duration-gated path — same correctness as before
for the common case, much harder for the matcher to confidently
pair the wrong file.
- One step closer to standalone-DB feature parity with plex /
jellyfin / navidrome scanners. Acoustid fingerprint fallback
(for files with NO useful tags AND no MBID/ISRC) is the next
followup PR.
Caught while live-testing the #524 fix with kendrick lamar
mr morale & the big steppers (3 discs). User dropped discs 1+2
loose in staging root + disc 3 in its own folder, every file
perfectly tagged with disc_number/track_number/title — only 9
tracks ended up in the library, the rest got integrity-rejected
and quarantined.
Two related bugs in `AutoImportWorker._match_tracks`:
1. **Quality dedup keyed on track_number alone.** The dedup loop
kept `seen_track_nums[track_number] = file` and dropped any later
file with the same number, treating it as a quality duplicate.
On a multi-disc release where every disc has tracks 1..N, that
collapses the album to one disc's worth of files BEFORE the
matcher runs. User's 18 loose disc-1+disc-2 files reduced to 9
before any title/disc info was even consulted.
2. **Match scoring ignored disc_number.** The 30% track-number bonus
fired whenever `ft[track_number] == track_num` regardless of disc.
File with tag (disc=2, track=6, "Auntie Diaries", 281s) got the
full bonus matching API track (disc=1, track=6, "Rich Interlude",
103s) — wrong file → wrong destination → integrity check correctly
rejected and quarantined the file. Same for tracks 7, 8, 9.
Fix:
- Dedup keys on `(disc_number, track_number)` tuples — multi-disc
files with parallel numbering all survive.
- Match scoring's 30% bonus only when BOTH disc AND track agree.
Cross-disc same-track-number collisions get a small 5% consolation
bonus so title similarity has to carry the match (covers cases
where tag disc info is missing or wrong).
- API track disc_number read from `disc_number` (Spotify) /
`disk_number` (Deezer) / `discNumber` (iTunes) defaulting to 1.
4 new pinning tests in `tests/imports/test_auto_import_multi_disc_matching.py`:
- 18-file 2-disc regression case (dedup preserves all)
- (disc=2, track=6) file matches API (disc=2, track=6) track, not
the disc-1 same-numbered track
- Single-disc albums still match normally (no regression)
- Quality dedup within a single (disc, track) position still picks
higher-quality format (.flac over .mp3)
Verification:
- 2268 full pytest suite passes (+4 new), 1 skipped, 0 failed
- Ruff clean
Same branch as the #524 fix because both surfaced from the same
import session — easier reviewer context if they ship together.
radoslav-orlov reported every imported album landing in the soulsync
standalone library as "Unknown Artist" + the raw 10-digit album id
as the title + 0 tracks. Audit traced it to the click handler in the
import page dropping the source-of-the-album_id on its way to the
backend match endpoint.
Root cause:
`importPageSelectAlbum(albumId)` (the onclick on every suggestion /
search-result card) only passed the album_id string. The full search
response carried `source`, `name`, and `artist` per row — the
backend's `get_artist_album_tracks` needs source so it can route the
lookup to the metadata source the id actually came from. Without it,
the source chain tries each source's `get_album(id)` against an id
shaped for a different source — a Deezer numeric id against
Spotify's id format returns 404, against iTunes's collectionId range
returns 404, etc. — and falls through to the failure-fallback dict
in `get_artist_album_tracks`:
{
'success': False,
'album': {'name': album_name or album_id, 'total_tracks': 0,
'release_date': '', ...}, # no artist field at all
'tracks': [],
}
That broken album dict then flowed through `build_album_import_context`
→ post-processing pipeline → `record_soulsync_library_entry`, writing
"Unknown Artist" + album_id-as-title + 0 tracks rows into the
soulsync standalone library tables.
Why hybrid users hit it most: a Spotify-primary user searching for an
album → search returns the Spotify result PLUS Deezer fallbacks
(via `_search_albums_for_source`'s priority chain). Clicking a Deezer
fallback row then sent only the Deezer id to /album/match without
flagging that source — Spotify-first chain failed against the Deezer
id and the broken fallback got written.
Fix:
Frontend (`webui/static/stats-automations.js`):
- New `importPageState._albumLookup: { albumId: { id, name, artist,
source } }` populated by both card renderers (`_renderSuggestionCard`
+ the search-results render block) before they emit the onclick.
- `importPageSelectAlbum` reads source / name / artist from that
cache and includes them in the match POST body, so the backend
routes to the correct provider's `get_album` on the very first try.
- `_escAttr` applied to album_id in the onclick (defensive — ids
shouldn't contain quotes but `_escAttr` was already being used on
every other field interpolated into onclick attributes).
Backend (`web_server.py:import_album_match`):
- Defensive log warning when source is missing from the request body.
Catches any future regression where another caller (curl /
third-party / new UI flow) drops source again — it'll show up as
a visible warning in app.log instead of silently corrupting the
library.
Verification:
- Full pytest suite: 2264 passed, 1 skipped, 0 failed
- Ruff clean
- JS syntax clean
- Manual repro requires a real user flow (search albums on the
import page → click one → import) which isn't covered by the
existing unit tests; reviewer should verify against issue #524's
steps before merge.
Three follow-on fixes to the manual-search candidates modal once people
started actually using it:
1. NDJSON streaming. Manual search waited for every source to return
before showing anything. Now streams one event per source as each
completes — header line, source_results per source, done terminator.
Frontend appends rows incrementally via response.body.getReader().
2. Manual picks no longer auto-retry on failure. New _user_manual_pick
flag set on the task in /download-candidate. Both monitor retry
paths (not-in-live-transfers stuck + Errored state) bail on the
flag. Surfaces the failure to the user instead of silently picking
a different candidate via fresh search.
3. Non-Soulseek manual picks (youtube/tidal/qobuz/hifi/deezer/
soundcloud/lidarr) no longer stuck at "downloading 0%" forever. The
live_transfers IF branch now marks manual-pick tasks failed
directly when the engine reports Errored, instead of deferring to
the monitor (which bails on manual picks). Engine fallback in else
branch covers the rare race where the orchestrator's pre-populated
transfer lookup is missing the entry.
Plus a deadlock fix discovered along the way: the new failure path
synchronously called on_download_completed while holding tasks_lock,
which itself re-acquires the same Lock — non-reentrant
threading.Lock self-deadlocked the polling thread. While wedged, every
other endpoint that needed the lock (including /candidates → other
failed rows couldn't open modals) hung waiting. Moved completion
callbacks onto a daemon thread so the lock releases first.
Plus failed/not_found/cancelled rows are now ALWAYS clickable (not
just when the auto-search cached candidates) — the modal carries the
manual search bar, which is the user's recourse for empty results.
Plus manual download worker now runs on a dedicated thread instead of
competing with the batch's 3-worker missing_download_executor pool —
saturated batches no longer queue manual picks indefinitely.
All scoped to manual picks via the _user_manual_pick flag — auto
attempt flow byte-identical to before. Engine fallback gated on the
flag too so auto attempts in the else branch keep the original
do-nothing behavior (safety valve handles the stuck-forever case).
Also dropped _handle_failed_download from web_server.py — defined
but had no callers (dead code).
17 new unit tests pin the gate behavior:
- engine fallback: Errored/Cancelled/Succeeded/InProgress transitions,
manual-pick gate, terminal-state skip, soulseek skip, missing
download_id skip, engine returning None, orchestrator exception
- monitor: manual-pick skips not-in-live-transfers retry + Errored
retry
- IF-branch end-to-end: Errored marks failed, "Completed, Errored"
hits failure branch, auto attempts defer to monitor
Manual-search endpoint tests rewritten for NDJSON: 11 cases (validation,
single-source dispatch, parallel "all" dispatch, one-event-per-source
streaming shape, unconfigured-source skip + reject, header metadata,
per-source exception isolation).
Full suite 2259 passed, 1 skipped.
When an auto-download fails or returns "not found" with leftover
candidates, the user can already click the status cell to open a
modal showing those candidates and pick a different one. This adds
a manual search bar to that modal — type any query, hit search,
get a fresh round of results without having to bail out and start
over from the main search page.
Solves the case where the auto-query was bad (featured artist not
in title, parentheticals like "(Remastered 2019)" tripping the
matcher, slight artist-name variants, transliteration) but the
file genuinely exists on the source.
Frontend (downloads.js)
- Added a manual-search section above the existing auto-candidates
table inside the candidates modal.
- Source picker is smart per download mode:
- Single-source mode (soulseek-only / youtube-only / etc) shows
a "Searching X" label, no dropdown.
- Hybrid mode shows a dropdown with "All sources" default + every
configured source. Picking "All" runs parallel searches across
them and tags each result row with its source badge.
- Only configured sources show up; unconfigured are hidden.
- Validation: button disabled until query length >= 2, "Type at
least 2 characters" hint until threshold crosses.
- Loading state on search button while the request is in flight.
- Manual results render in a separate table above the existing
auto-candidates table, using the same row template (file /
quality / size / duration / user / ⬇ button) so the renderer
helper is shared.
- Click ⬇ reuses the existing `downloadCandidate(taskId, candidate,
trackName)` flow — same retry path, same AcoustID verification
when the file lands, no shortcut around the safety net.
- Re-running the search with a different query replaces the
previous manual results.
Backend (web_server.py)
- Extended `GET /api/downloads/task/<id>/candidates` response with:
- `download_mode` (e.g. 'hybrid', 'soulseek')
- `available_sources` (list of configured source IDs + labels)
- `source` field on each candidate (purely additive — frontend
auto-renderer ignores it on legacy code paths, manual-search
renderer uses it for the badge)
- Added `POST /api/downloads/task/<id>/manual-search`:
- Body: `{ query, source: 'all' | <source_id> }`
- Validates query length (>=2 trimmed) → 400
- Validates source against the configured-sources gate → 400
(rejects unconfigured sources even when explicitly named)
- For 'all': parallel `ThreadPoolExecutor` dispatch across every
configured download source, merged results
- For specific source: just that source
- Returns same shape as `/candidates` so the frontend renderer
is reused
- New module-level helpers: `_STREAMING_SOURCE_NAMES`,
`_infer_candidate_source`, `_serialize_candidate`,
`_list_available_download_sources`. The existing `/candidates`
endpoint also goes through `_serialize_candidate` so the source
badge is consistent across both flows.
Behavior preserved
- Existing modal layout / candidates table / ⬇ button are
byte-identical when the user doesn't use manual search.
- `downloadCandidate()` JS function untouched.
- `/candidates` and `/download-candidate` endpoints
backwards-compatible — only NEW fields added, nothing changed
or removed.
Tests
`tests/test_manual_search_endpoint.py` — 10 tests:
- `test_manual_search_validates_query_length`
- `test_manual_search_validates_source` (whitelist gate)
- `test_manual_search_handles_task_not_found` (404)
- `test_manual_search_dispatches_to_configured_source_only`
- `test_manual_search_all_dispatches_parallel`
- `test_manual_search_skips_unconfigured_sources`
- `test_manual_search_rejects_unconfigured_source_explicitly`
- `test_manual_search_returns_same_shape_as_candidates`
- `test_manual_search_single_source_mode_lists_source` (verifies
`available_sources` reflects the active mode)
- `test_manual_search_isolates_per_source_exceptions` (one source
throwing doesn't kill the merged result)
2242/2242 full suite green (was 2232 + 10 new). Ruff clean.
JS parses clean.
- `_SOULSYNC_BASE_VERSION` 2.4.2 → 2.4.3
- helper.js — flip 2.4.3 WHATS_NEW header to "May 8, 2026 — 2.4.3
release"; bump fallback default from 2.4.2 → 2.4.3
- docker-publish.yml — manual-trigger default tag 2.4.2 → 2.4.3
Drive-by — make sidebar version + version-modal subtitle dynamic.
The sidebar version button (`v2.4.1`) and version-modal subtitle
(`Version 2.4.1 — Latest Changes`) were hardcoded text in the HTML.
2.4.2 shipped without these getting bumped — silent drift, easy to
miss at every release.
Added a Flask context_processor that injects `soulsync_version` and
`soulsync_base_version` into every template, then templated the two
hardcoded values:
v{{ soulsync_base_version }}
Version {{ soulsync_base_version }} — Latest Changes
Now bumping `_SOULSYNC_BASE_VERSION` updates the UI everywhere it's
rendered. No more "I forgot to bump the sidebar" at release.
2232/2232 full suite green. Ruff clean. JS parses clean.
Four selection-quality fixes on the SoulSync-made discover playlists.
None change public method signatures; all are tightenings on what's
already there.
(1) Diversity for Hidden Gems + Discovery Shuffle
Both used to be `RANDOM() LIMIT N` with no diversity. Could return
50 tracks from one artist or 20 from one album if the discovery
pool happened to be skewed. Both now over-fetch 3x and run the
existing `_apply_diversity_filter`:
- Hidden Gems: max 2 per album, 3 per artist
- Discovery Shuffle: max 2 per album, 2 per artist (tighter — shuffle
should feel maximally varied)
(2) Source-aware popularity thresholds
`popularity >= 60` for "Popular Picks" and `popularity < 40` for
"Hidden Gems" was Spotify-shaped (0-100 scale). Deezer writes its
`rank` value into that column (often six-digit integers); iTunes
writes nothing meaningful. For Deezer-primary users:
- Popular Picks pulled essentially everything (rank >= 60 = all)
- Hidden Gems pulled essentially nothing (rank < 40 = none)
New `_get_popularity_thresholds(source)` helper returns per-source
values:
- Spotify: (60, 40) — the existing 0-100 scale
- Deezer: (500_000, 100_000) — ballpark from real rank values
- iTunes / unknown: (None, None) — skip the popularity filter
entirely, fall back to random + diversity
`get_popular_picks` and `get_hidden_gems` now consult the helper.
When threshold is None they skip the popularity SQL filter. Diversity
+ ID gate still apply.
(3) Push genre keyword filter into SQL
`get_genre_playlist` used to fetch `limit=1_000_000` rows into Python
then run a substring keyword filter on `artist_genres`. Bad on big
discovery pools.
Now the keyword OR chain is generated as SQL placeholders:
AND (artist_genres LIKE ? OR artist_genres LIKE ? OR ...)
Each placeholder gets `f'%{keyword.lower()}%'` via `extra_params`.
`fetch_limit` drops back to `limit * 10`. `_genre_matches` Python
helper deleted (only intra-file caller; verified via grep).
Parent-genre expansion via `GENRE_MAPPING` preserved — keywords list
feeds the LIKE chain unchanged.
(4) Filter out tracks already in library
Discovery pool can include tracks the user already owns. Hidden Gems
/ Shuffle / Popular Picks shouldn't surface those.
`_select_discovery_tracks` gained `exclude_owned: bool = True`
parameter. When True, adds a correlated NOT EXISTS subquery against
the `tracks` table covering all 3 source IDs:
AND NOT EXISTS (
SELECT 1 FROM tracks t WHERE
(t.spotify_track_id IS NOT NULL AND t.spotify_track_id = discovery_pool.spotify_track_id)
OR (t.itunes_track_id IS NOT NULL AND t.itunes_track_id = discovery_pool.itunes_track_id)
OR (t.deezer_id IS NOT NULL AND t.deezer_id = discovery_pool.deezer_track_id)
)
Note column-name asymmetry: tracks.deezer_id vs
discovery_pool.deezer_track_id. Inline comment marks the trap. All
5 public discovery methods automatically benefit (default True).
Seasonal Playlist doesn't go through the helper so it's unaffected
(curated content, dedup is wrong intent there).
Tests
12 new tests in `tests/test_personalized_playlists_id_gate.py` (27
total in the file):
- Hidden Gems + Discovery Shuffle apply diversity (cap proven by
inserting 10 same-artist + same-album rows and asserting return
count ≤ per-album cap)
- Popularity thresholds: Spotify (60, 40), Deezer larger scale,
iTunes None / None
- Popular Picks skips threshold filter when None
- Genre playlist pushes filter to SQL (parent + child genre expansion)
- Owned-track exclusion: filtered when match, kept when no match,
opt-out flag works
- Deezer column-name asymmetry pinned (regression footgun)
Test fixture re-added the minimal `tracks` table (4 columns: id,
spotify_track_id, itunes_track_id, deezer_id) — only what the new
NOT EXISTS subquery needs to join. Plus `insert_library_track`
helper.
Verification
- 27/27 in this test file pass (15 prior + 12 new)
- 2232/2232 full suite green
- ruff clean
LOC delta:
- core/personalized_playlists.py: 1030 → 1101 (+71)
- tests/test_personalized_playlists_id_gate.py: 352 → 616 (+264)
Owner decision: not worth shipping. The four library-driven personalized
sections were stubbed returning [] for ages because their schema
prereqs didn't exist; the prior commit re-enabled them by routing
through a new `_select_library_tracks` helper. Owner reviewed and chose
to delete the sections entirely instead.
Removed everywhere:
- `core/personalized_playlists.py` — `get_recently_added`,
`get_top_tracks`, `get_forgotten_favorites`, `get_familiar_favorites`
+ the `_select_library_tracks` helper (no other callers; verified
via grep).
- `web_server.py` — 4 route handlers
(`/api/discover/personalized/recently-added`, `top-tracks`,
`forgotten-favorites`, `familiar-favorites`).
- `webui/index.html` — 4 `<div class="discover-section">` blocks
(`#personalized-recently-added`, `#personalized-top-tracks`,
`#personalized-forgotten-favorites`,
`#personalized-familiar-favorites`).
- `webui/static/discover.js` — 4 load functions
(`loadPersonalizedRecentlyAdded`, `loadPersonalizedTopTracks`,
`loadPersonalizedForgottenFavorites`, `loadFamiliarFavorites`),
plus their entries in `loadDiscoverPage`'s Promise.all, plus
4 module-level state vars + 6 dead branches across
`openDownloadModalForDiscoverPlaylist` / `startDiscoverPlaylistSync`
and the sync-progress / rehydrate dispatchers.
- `webui/static/helper.js` — 4 tooltip / docs entries.
- `webui/static/sync-spotify.js` — 1 stale rehydrate dispatcher
branch (`discover_familiar_favorites`) caught during the global
grep pass.
- `tests/test_personalized_playlists_id_gate.py` — 3 library-method
tests + the test infrastructure that supported them
(`tracks` schema, `insert_library_track` helper). Documentation
header updated to reflect the deletion.
Net: -527 / +2 lines across 7 files.
What stays:
- Daily Mixes (also in personalized package, intentionally paused —
separate decision).
- Popular Picks + Hidden Gems + Discovery Shuffle (alive, not
affected by this deletion).
- All 14 tests in the personalized-playlists test file still pass.
- The PersonalizedPlaylistsService lift from the prior commit
(`_select_discovery_tracks` etc) — those are still in active use
by the surviving discovery_pool methods.
DISCOVER_TRACK_SELECTION_REVIEW.md at repo root contains historical
references to the four deleted endpoints. Treated as historical
context (same policy as WHATS_NEW), left alone.
2219/2219 full suite green (was 2222 - 3 deleted tests = 2219).
JS parses clean, ruff clean.
Both tabbed-browser sections — Time Machine ("Decade") and Browse by
Genre — re-implemented the same lifecycle by hand: fetch tabs list,
render the tab strip, attach click handlers, fetch content per tab,
render track list with sync + download action buttons + sync-status
block, handle empty/error/loading states. ~314 lines of identical
boilerplate split across two browsers.
Lifted into one shared `createTabbedBrowserSection(config)` helper.
Each browser is now a thin wrapper:
```js
const ctrl = createTabbedBrowserSection({
id: 'decade-browser',
tabsContainerEl: '#decade-tabs',
contentContainerEl: '#decade-content',
fetchTabs: async () => { ... },
renderTabButton: (tab, isActive) => `<button>...</button>`,
fetchTabContent: async (tab) => { ... },
renderTabContent: (tracks, tab) => `...`,
onTabContentRendered: (tab, contentEl) => { ... },
emptyMessage / errorMessage,
});
```
Migrated:
- `loadDecadeBrowserTabs` 85 → 3 lines
- `loadDecadeTracks` 67 → 3 lines
- `loadGenreBrowserTabs` 92 → 3 lines
- `loadGenreTracks` 70 → 3 lines
Helper: ~125 lines + ~100 lines of per-browser config blocks +
~25 lines of shared `_renderTabbedTrackList` (the two browsers had
byte-identical track-row markup so it lifted cleanly).
Public function names preserved — the four migrated functions stay
on the same signature so existing callers (`loadDiscoverPage`,
refresh buttons, inline handlers) don't change.
Side effects preserved — `decadeTracksCache[year]`, `activeDecade`,
`genreTracksCache[name]`, `activeGenre`, `availableGenres` still
mutated at the same lifecycle moments. The decade-specific
`startDecadeSync(decade)` and genre-specific `startGenreSync(name)`
sync-button handlers stay where they are; they're click handlers
attached to rendered content, not part of the tab lifecycle.
What didn't fit (intentionally left alone):
- `_renderCompactTrackRow` (the existing shared track-row helper) is
NOT used by the tabbed browsers — they had their own template
with a `track_data_json` fallback chain `_renderCompactTrackRow`
doesn't do. Unifying these two would change behavior for
non-tabbed sections, so the tabbed-browser variant lives as
`_renderTabbedTrackList`. Future cleanup could merge them by
giving `_renderCompactTrackRow` an opt-in fallback flag.
- `switchDecadeTab` / `switchGenreTab` still know about cache shape
so they can skip refetch on already-loaded tabs. Keeping that
in the per-browser switch is fine — it's a click handler, not
lifecycle.
Net: 8546 → 8578 LOC on `discover.js` (+32). Helper boilerplate
offsets the line count, but the win is single-source-of-truth, not
raw line reduction.
`node --check` clean. 2222/2222 full suite green.
Three changes tightening the controller before opening the PR.
DROP MAGIC `extractItems` DEFAULTS
Controller used to auto-pull `data.items` / `data.albums` /
`data.artists` / `data.tracks` / `data.results` when no extractor
was supplied. Removed the fallback chain — every section now MUST
provide an explicit `extractItems(data) => array`. Validated at
register-time so misuse fails immediately, not silently on first
load against an endpoint that happened to return two arrays.
Cin standard: explicit > implicit. Magic key-grabbing could pick
the wrong one in edge cases (e.g. an endpoint returning both
`data.albums` and `data.results` would have grabbed albums when
the section actually wanted results).
All 10 existing controller call sites already passed explicit
extractors, so no migration churn — this is purely tightening the
contract for future sections.
REPLACE `renderItems` NULL-RETURN CONVENTION WITH `manualDom: true`
Your Albums and similar sections that delegate to existing renderers
that target a CHILD element of `contentEl` used to signal "leave the
container alone" by returning null/undefined from `renderItems`. That
convention is easy to confuse with an accidental missing-return error.
Replaced with an explicit `manualDom: true` config flag. Renderer is
still called for its side-effects, controller just skips the innerHTML
swap. Clearer intent at the call site. Updated `loadYourAlbums` to
use the new flag.
PIN THE CONTROLLER CONTRACT WITH JS TESTS
Added `tests/static/test_discover_section_controller.mjs` — 32 tests
covering the controller's lifecycle contract:
- Config validation (every required field, mutual exclusivity of
fetchUrl/data, type checks on contentEl)
- Happy-path fetch → parse → render
- Empty state (default empty render, hideWhenEmpty + sectionEl,
success=false treated as empty, custom isSuccess override)
- Stale state (fires when isStale returns true, wins over empty,
custom renderStale override)
- Error state (HTTP non-ok, fetch throws, showErrorToast fires
window.showToast, default off doesn't fire)
- No-fetch `data:` mode (value + function form, doesn't call fetch)
- manualDom mode (skips innerHTML swap, still calls renderer)
- Callable `fetchUrl` (resolved at load time, refresh re-resolves)
- Load coalescing (concurrent loads share one fetch)
- Refresh bypasses coalescing (re-fires fetch every call)
- Hook error containment (throwing renderer/onSuccess hooks don't
crash the controller)
Runs via Node's stable built-in `--test` runner — no package.json,
no jest/vitest dependency, no compile step. Just `node --test`.
Pytest wrapper at `tests/test_discover_section_controller_js.py`
shells out to node and asserts clean exit, so the JS tests fail
the regular pytest sweep if the controller contract drifts.
Skipped gracefully when node isn't available or is < 22.
Closes the "controller is a contract, pin it at the test boundary"
gap that Cin would have flagged on review.
VERIFICATION
- 2205/2205 full pytest suite green (was 2204 + 1 new wrapper)
- 32/32 `node --test` pass on the controller test file directly
- ruff clean
- node --check clean on all touched JS files
Follow-up to the controller migration commits. Closes out the
extension list the per-section migrations surfaced as needed.
CONTROLLER EXTENSIONS
- Callable `fetchUrl: () => string` — resolves the seasonal-playlist
recreate-on-key-change hack from the prior commit.
- No-fetch `data:` mode — value or `() => value`. Lets render-only
sections like Seasonal Albums use the controller without inventing
a fake endpoint. Mutually exclusive with `fetchUrl`; validated up
front so misuse fails at register-time.
- `beforeLoad(ctx)` hook — runs before the spinner shows. Lets
dynamically-inserted sections like Because You Listen To ensure
their `contentEl` exists before the visibility check.
- `onSuccess(data, ctx)` hook — runs after the success gate but
before isEmpty / isStale. Cleaner home for sibling header /
subtitle / button updates than folding them into renderItems.
- `isStale(items, data)` + `onStale(ctx)` + `renderStale(items, data)`
+ `staleMessage` — third render state for "data is empty BUT
upstream is still discovering". Stale wins over empty when both
apply. Default stale UI is the same spinner block used elsewhere.
- `showErrorToast: true` config — opens a global `showToast(...)` in
addition to the in-section error block. Default off; sections that
have no recovery action shouldn't shout at the user.
- `renderItems` returning null/undefined now leaves contentEl
untouched. Lets a renderer do its own DOM manipulation (e.g.
delegating to an existing grid-render fn that targets a child
element) without fighting the controller's innerHTML swap.
MIGRATED THE 2 SKIPPED SECTIONS
- `loadYourAlbums` — uses `isStale`/`onStale`/`renderStale` for the
stale-fetch state, `onSuccess` for the subtitle/filters/download
side-effects, `hideWhenEmpty` + `sectionEl` for the truly-empty
case, `renderItems` returning null since it delegates to the
existing `_renderYourAlbumsGrid` + `_renderYourAlbumsPagination`.
- `loadSeasonalAlbums` — uses no-fetch `data:` mode because the
parent `loadSeasonalContent` already fetched the season payload.
`beforeLoad` updates the sibling title/subtitle text.
ERROR TOASTS ON ALL MIGRATED SECTIONS
Every migrated section now has `showErrorToast: true`. Section load
failures surface a global toast instead of silently spinning forever
or swallowing into console.debug. Same pattern JohnBaumb #369 asked
for at the Python layer, applied at the UI layer.
SHARED SYNC-STATUS BLOCK
Lifted the duplicated decade-tab + genre-tab sync-status HTML
(✓ completed | ⏳ pending | ✗ failed | percentage) into a single
`_renderSyncStatusBlock(idPrefix)` helper. Two call sites now share
one implementation. ListenBrainz playlists keep their own block
because the semantics differ — matching progress (total / matched /
failed) vs download progress.
DEAD-SECTION AUDIT — NONE DEAD
Audited the 13 supposedly-dead hidden sections from
DISCOVER_REVIEW.md. All 13 are alive: gated on user data (discovery
pool, library content, metadata cache) and self-surface when their
data exists via `style.display = 'block'` on the success path. The
review's grep missed the toggle. No deletions made.
DAILY MIXES ORPHAN CALL
Removed the orphaned `loadPersonalizedDailyMixes()` call from
`blockDiscoveryArtist` — Daily Mixes is intentionally paused (its
load call in `loadDiscoverPage` is commented out) so refreshing it
from the post-block hook was a no-op.
2204/2204 full suite green. JS parses clean (`node --check`).
Follow-up to the foundation commit. Drops the hand-rolled
try/catch + spinner injection + empty-state HTML + error-swallow
in seven sections by routing them through
`createDiscoverSectionController`. Each section keeps its existing
public function name + signature so callers, refresh buttons, and
dashboard wiring don't notice the swap.
Migrated:
- `loadDiscoverReleaseRadar` (Fresh Tape)
- `loadDiscoverWeekly` (The Archives)
- `loadDecadeBrowser` (Time Machine intro carousel)
- `loadGenreBrowser` (Browse by Genre intro carousel)
- `loadSeasonalPlaylist` (Seasonal Mix)
- `loadYourArtists`
- `loadBecauseYouListenTo`
Skipped (don't fit the controller's single-fetch / single-render-target
shape):
- `loadYourAlbums` — paginated grid + filters, updates four separate
UI elements (subtitle, filter chips, download button, grid).
- `loadSeasonalAlbums` — receives pre-fetched data from
`loadSeasonalContent`; no fetch URL to satisfy.
Hidden / dead sections (~13 of them — `loadPersonalized*`,
`loadDiscoveryShuffle`, `loadFamiliarFavorites`, `loadCache*`)
untouched in this pass. Separate audit commit will surface or kill
them.
Two side-effects worth noting:
- `loadDecadeBrowser` and `loadGenreBrowser` migrated for
completeness, but neither appears wired into `loadDiscoverPage` or
any inline handler. May be dead code — flagged for the audit pass.
- `loadSeasonalPlaylist` needs a per-load fetch URL (varies by
`currentSeasonKey`); worked around by recreating the controller
when the key changes. Cleaner option: extend the controller to
accept a `fetchUrl: () => string` callable form. Tracked in the
follow-up extension list below.
Controller extension candidates surfaced for follow-up:
- Callable `fetchUrl` (resolves the seasonal playlist
recreate-on-key-change hack)
- Explicit `isStale` / `onStale` hook (so Your Artists doesn't
fold stale handling into renderItems)
- `beforeLoad` / `ensureContentEl` hook (so Because You Listen To
can let the controller own the dynamic container creation)
- No-fetch `data:` mode (so render-only sections like Seasonal
Albums can use the controller too)
- `onSuccess(data)` hook (cleaner home for header / subtitle
side-effects vs folding them into renderItems)
Net: -76 lines in `discover.js` even after adding the per-section
render helpers. 2204/2204 full suite green. JS parses clean.
Every section on the discover page (Recent Releases, Your Artists,
Your Albums, Seasonal Albums, Seasonal Mix, Fresh Tape, The Archives,
Build Playlist, Time Machine, Browse by Genre, ListenBrainz Playlists,
Because You Listen To, plus ~13 hidden sections) currently
re-implements the same lifecycle by hand:
1. show a loading spinner in the carousel container
2. fetch the section's endpoint
3. parse the response, decide if the data is empty
4. either render the items, show an empty-state, or show an error
5. wire post-render handlers (download buttons, hover behavior, etc)
6. maybe expose refresh()
~30 sections worth of duplicated boilerplate, all subtly drifting.
Different empty-state messages. Different error handling (some
`console.debug`, some silently swallowed, some leave the spinner
spinning forever). Different sync-status icons (✓/⏳/✗ vs ♪/✓/✗).
No consistent error toast.
Lifted the lifecycle into a shared `createDiscoverSectionController`
in `webui/static/discover-section-controller.js`. Renderers stay
per-section because section data shapes legitimately differ — album
cards vs artist circles vs playlist tiles vs track rows. The
controller is the wrapper, not a forced visual abstraction.
Foundation contract:
createDiscoverSectionController({
id: 'recent-releases', // for diagnostic logging
contentEl: '#carousel', // selector or Element
fetchUrl: '/api/discover/...',
extractItems: (data) => [...], // pull list from response
renderItems: (items, data, ctx) => '<html>',
onRendered: (ctx) => { ... }, // optional post-render hook
loadingMessage / emptyMessage / errorMessage: copy
sectionEl + hideWhenEmpty: optional whole-section visibility
isSuccess / isEmpty: optional gate overrides
})
Returns `{ load, refresh, destroy, getState }`. Validates config up
front so misuse fails at register-time, not silently on load. Coalesces
concurrent loads (same in-flight promise returned) so a double-click
or repeated trigger doesn't double-fetch. `refresh()` bypasses the
coalesce so the refresh button always re-fires. Errors are logged
(console.debug by default, console.error when verboseErrors=true).
Renderer hook errors are caught + logged so a buggy render callback
can't tear down the controller — keeps the page resilient.
Migrated `Recent Releases` as the proof — simplest album-card shape,
no source-gating, no refresh button. Verified the contract covers it
end-to-end. The legacy `loadDiscoverRecentReleases` entry-point stays
public so existing callers don't change; internally it lazy-builds
the controller and triggers `load()`.
NOT in this commit:
- Other section migrations (one section per follow-up commit, keeps
reviews small + lets us sequence the work)
- Registry-driven section list (so the dead-section audit becomes
registry deletions instead of section-by-section removal)
- Global error toast wrapper
- Per-section "requires X primary source" gate
- Sync-status icon renderer unification
Once every section is on the controller, the discover-page cleanup
work (kill the 13 dead sections, standardize sync-status icons, add
error toasts) becomes single-line registry-level edits instead of
30 separate section-by-section rewrites.
2204/2204 full suite green. JS parses clean (`node --check`). Manual
smoke deferred until follow-up commits — Recent Releases unchanged
on the wire (same endpoint, same payload shape, same render output).
- `web_server.py` — `_SOULSYNC_BASE_VERSION` 2.4.1 → 2.4.2
- `webui/static/helper.js` — flip the 2.4.2 WHATS_NEW header from
"Unreleased — 2.4.2 dev cycle" to "May 7, 2026 — 2.4.2 release"
so the per-version block stops being filtered out by
`_getLatestWhatsNewVersion`. Also bumps the safety-net default
inside that helper from 2.4.1 → 2.4.2.
- `.github/workflows/docker-publish.yml` — manual-trigger default
tag bumped to match.
Drive-by fix: escaped a stray single quote in the `Internal: Download
Engine` 2.4.2 entry that broke `node --check` on the file
(`orchestrator.client('soulseek')` inside a single-quoted desc string
silently terminated the string mid-entry). Pre-existing, unrelated to
the bump but caught while validating JS parse for the release.
VERSION_MODAL_SECTIONS not rotated in this commit — separate
editorial pass.
Closes#513 (s66jones).
The artist detail page already showed a "Popular on Last.fm" sidebar —
list of an artist's top tracks by playcount, with a play button per row
but no download action. Issue #513 wanted a way to grab those tracks
the same way zotify let users grab "top X songs" without pulling the
full discography.
Pulls from the configured primary metadata source (Spotify
`artist_top_tracks`, Deezer `/artist/{id}/top`) when available, falls
back to the existing Last.fm display-only mode for sources that don't
expose popularity ranking (iTunes / Discogs / MusicBrainz). Source
label in the section title shifts to match.
Each row gets a hover-revealed download button that wishlists the
single track via the existing /api/add-album-to-wishlist endpoint
(preserves the track's real album metadata, so the wishlist worker
later places the file in its proper album folder).
A "Download All" footer button opens the standard download modal in
PLAYLIST context, not album context — the virtual playlist_id is
`top_tracks_<source>_<artistId>` which doesn't match any of the
album-prefix checks in `startMissingTracksProcess` (downloads.js).
That keeps `is_album_download=false`, so the master worker doesn't
inject a wrapper context as `_explicit_album_context`. Each track
downloads using its own real album metadata, files land in proper
per-album folders on disk (not a fake "Top Tracks" folder).
Backend additions:
- `SpotifyClient.get_artist_top_tracks(artist_id, country, limit)` —
wraps `spotipy.artist_top_tracks`, returns up to 10 tracks for the
market (Spotify's API cap). UI-side limit trim only.
- `DeezerClient.get_artist_top_tracks(artist_id, limit)` — wraps
`/artist/{id}/top?limit=N`, converts Deezer's raw shape to the same
Spotify-compatible dict layout (id, name, artists, album with
album_type / total_tracks / images, duration_ms, track_number,
disc_number) so downstream code doesn't branch on source.
- `GET /api/artist/<id>/top-tracks` — dispatches to whichever client
matches the primary source. Resolves per-source artist IDs from the
DB row first (matching what /discography already does) so a Spotify
ID in the URL still works when Deezer is primary, and vice versa.
Returns `{success, source, tracks, resolved_artist_id}` on hit;
`{success: False, reason: 'unsupported_source' | 'spotify_not_authenticated'
| 'deezer_unavailable' | 'no_tracks_found'}` on miss so the frontend
can decide whether to fall through to Last.fm.
Frontend:
- `_loadArtistTopTracks` tries the metadata source first, falls
through to the legacy `/api/artist/0/lastfm-top-tracks` call if the
source can't deliver. Section title and per-row UI shift based on
which source answered.
- New per-row `.hero-top-track-download` button (hover-revealed).
- New `.hero-top-tracks-download-all` footer button — only visible
when metadata-source mode rendered the list (Last.fm fallback hides
it since rows have no track IDs to download).
Tests: 10 new tests pin the client methods —
- Spotify: returns track list, honors UI limit cap, returns empty when
unauthed / artist_id missing / API throws.
- Deezer: shape conversion to Spotify-compatible dict, empty when no
data / artist_id missing, limit clamping at upper bound, default
fallback when limit=0, malformed entries skipped.
The Flask endpoint dispatcher itself isn't covered by the new test
file because importing web_server at test-collection time spins up
worker threads that race with caplog-using tests elsewhere in the
suite (specifically test_library_reorganize_orchestrator). Endpoint
verified manually; the underlying client methods (the load-bearing
logic) are covered.
2204/2204 full suite green (was 2194 + 10 new).
Broque Thomas