Forgot to bump the hardcoded label in index.html during the 2.4.0
version commit. _getCurrentVersion() reads this textContent, so the
What's New surfacing logic was still seeing 2.3.
- _SOULSYNC_BASE_VERSION → 2.4.0 (was 2.39).
- Migrate WHATS_NEW key '2.40' → '2.4.0', strip unreleased flags off
the 27 entries shipping in this release, set release date.
- Replace parseFloat() version compare with proper int-tuple semver
comparator — parseFloat('2.4.0') and parseFloat('2.4.1') both return
2.4, which would have made future patch bumps invisible to the
What's New surfacing logic.
Five issues kettui flagged on PR #377:
- Worker race (reorganize_queue.py): _next_queued() picked an item and
released the lock, then re-acquired to flip status='running'. A
cancel() landing in that window marked the item cancelled but the
worker still ran it. Replaced with _claim_next_or_wait() that picks
AND flips under one lock acquisition.
- Wakeup race (reorganize_queue.py): _wakeup.clear() after the empty
check could lose an enqueue's _wakeup.set(), parking a freshly-queued
album for up to 60 seconds. Replaced Lock + Event with a single
threading.Condition; cond.wait() releases and re-acquires atomically
on notify.
- Bulk dedupe (reorganize_queue.py:enqueue_many): looped single-item
enqueue, so a duplicate album_id later in the same batch could slip
through if the worker finished the first copy before the loop
reached the second. Now holds the lock for the whole batch and tracks
a per-batch seen set, so intra-batch duplicates dedupe against each
other and not just pre-existing items.
- Preview button stuck disabled (library.js:loadReorganizePreview):
early returns and thrown errors skipped the re-enable line. Moved
state into a canApply flag committed in finally, so any exit path
lands the button correctly.
- DB helpers swallowing failures (music_database.py): get_album_display_meta
and get_artist_albums_for_reorganize used to catch every Exception
and return None / [], so a real DB outage masqueraded as "album not
found" / "no albums". Now lets exceptions bubble; the route layer
already wraps them as 500.
Tests:
- test_cancel_and_run_are_mutually_exclusive — hammers enqueue+cancel
pairs and asserts the invariant that no successfully-cancelled item
ever ran (catches regressions to the atomic pick).
- test_enqueue_many_dedupes_batch_internal_duplicates — pins the
intra-batch dedupe.
- test_get_album_display_meta_propagates_db_errors and
test_get_artist_albums_for_reorganize_propagates_db_errors — pin
the bubble-up behavior.
Changelog updated in helper.js and version modal.
Replaces the single-slot "one reorganize at a time, return 409 on collision"
model with a per-user FIFO queue. Buttons stay clickable, "Reorganize All"
is one backend call instead of an N-call JS loop, and a status panel mounted
at the top of the artist actions bar shows live progress (active item,
queued count, recent completions) with per-item cancel buttons.
Backend
- core/reorganize_queue.py: singleton queue + worker thread, dedupe-on-
enqueue, cancel rules (queued cancellable, running not), enqueue_many
for bulk operations, progress fan-out via update_active_progress
- core/reorganize_runner.py: factory builds the worker's runner closure
with injected dependencies. Reads config per-call so changing the
download path in Settings takes effect on the next reorganize without
a server restart
- database/music_database.py: get_album_display_meta and
get_artist_albums_for_reorganize — moves the SQL out of route handlers
- web_server.py: thin enqueue/snapshot/cancel/clear endpoints, runner
registration at module load. Old _reorganize_state globals + status
endpoint deleted. Static-asset cache buster (?v=<server-start>)
added so JS/CSS updates ship live without users clearing cache
Frontend
- webui/static/library.js: status panel mount, polling (1.5s when
active, 8s when idle), expand/collapse, per-item cancel, debounced
enhanced-view reload (one reload per artist batch instead of N).
Per-album reorganize button paints with queued/running indicator
and short-circuits to a toast when the album is already in queue
- webui/static/style.css: panel + button styling matching the existing
glass-UI accents
- webui/static/helper.js + version modal: WHATS_NEW entry
Tests (22 new)
- tests/test_reorganize_queue.py (19 tests): FIFO order, dedupe,
per-item source, cancel rules, continue-on-failure, snapshot
shape, progress propagation, bulk enqueue
- tests/test_reorganize_runner.py (4 tests): per-call config reads,
setup-failure summary, dependency injection, progress fan-out
- tests/test_reorganize_db_methods.py (7 tests): SQL JOIN behavior,
ordering, fallback for blank strings, artist isolation
Full suite 549 passed in 27s.
Four changes addressing kettui's PR #377 review comments:
1. **`_finalize_track` no longer over-counts on DB failure (🔴 bug).**
The function previously bailed on DB-update failure but
`_process_one_track` still incremented `summary['moved']`
unconditionally — overstating how many tracks the UI knows are
at their new locations. Fixed by:
- `_finalize_track` now returns ``bool`` (True only when DB row
was updated AND original was dealt with)
- Caller checks the return; on False, records as a failed track
with a clear message ("Track landed at new location but DB
update failed — file is at both old and new paths until library
scan re-indexes")
- Existing `test_db_update_failure_leaves_original_in_place` now
also asserts `moved == 0`, `failed == 1`, and that the error
message names the cause
2. **`executeReorganize` toast no longer says "undefined tracks" (🐛
bug).** `/reorganize` doesn't return `result.total` anymore (the
track count is determined server-side after planning), so the
"Reorganizing undefined tracks..." string was meaningless. Now uses
`result.message` from the backend instead.
3. **`_pollReorganizeStatus` distinguishes completed from skipped
(🟡 risk).** Backend now propagates the orchestrator's status
(`completed` / `no_source_id` / `no_album` / `no_tracks` /
`setup_failed` / `error`) into `_reorganize_state['result_status']`
so the frontend can warn appropriately. Two new helpers:
- `_classifyReorganizeOutcome(state)` — returns 'success' only
when `result_status === 'completed'` AND `failed === 0`;
'warning' otherwise
- `_formatReorganizeResultMessage(state)` — returns a message
specific to the outcome ("Reorganize skipped — album has no
metadata source ID. Run enrichment first." for `no_source_id`,
etc.)
Zero-failure non-completed runs now show as warnings instead of
green checkmarks.
4. **Bulk mode no longer counts skipped albums as succeeded (🟡
risk).** `_executeReorganizeAll`'s loop was treating any HTTP
200 response as success, ignoring the orchestrator's actual
outcome for that album. Fixed by:
- `_waitForReorganizeComplete()` now resolves with the final
state object (was: void)
- Loop checks `finalState.result_status === 'completed'` AND
`finalState.failed === 0` before counting `succeeded++`;
otherwise increments `skipped` (with a per-album warning
toast) or `failed` accordingly
- Final summary toast now reads
"Reorganized N of M albums, K skipped, J failed" and only
shows green when nothing was skipped or failed
All four addressed in a single commit because they form one
coherent UX-correctness fix — the bug bug (#1) and the count-
overstatement bug (#4) both made the user see "everything succeeded"
when reality was different. Together they make the UI honestly
reflect what actually happened.
Files:
- core/library_reorganize.py — `_finalize_track` returns bool,
`_process_one_track` reads it
- web_server.py — `_reorganize_state['result_status']` populated
from orchestrator's summary on success and on exception
- webui/static/library.js — `_classifyReorganizeOutcome` /
`_formatReorganizeResultMessage` helpers, single-album +
bulk-mode flows both consume them
- tests/test_library_reorganize_orchestrator.py — strengthened
the existing DB-failure test to assert moved/failed counts
Credit: kettui — four PR #377 review comments named all of these
precisely with line numbers and severity.
Reported by kettui on PR #374 review:
> api_track_count is not copied during the ratingKey migration, so
> the cache disappears when an album row is rekeyed. Add it to
> enrichment_cols or the next completeness scan will fall back to
> live API lookups again.
When Plex changes an album's ratingKey (after a library rescan), the
sync code rekeys the album row by inserting a new row at the new ID
and copying enrichment columns from the old row. The list of
columns to copy did not include `api_track_count`, so the cached
authoritative track count was lost on rekey — and the next completeness
scan would hit the fallback path that calls back out to the
metadata source's API. Defeats the cache.
Added `api_track_count` to the album-level `enrichment_cols` at
`music_database.py:4724`. The artist-level lists at lines 4238 and
4554 don't need updating — those are for artist rekeys and don't
carry album-scoped fields.
No new test — existing migration code has no test infrastructure
and writing a Plex-mocked one is larger than this fix. Cin will say
if he wants test coverage in his next review pass.
Credit: kettui — PR #374 review comment that flagged the missing
column in the rekey allowlist.
Reported by kettui on PR #374 review: the inline filter that backed
`set_album_api_track_count` only counted rows where `type_ == 'track'`,
but `discogs_client.get_album_tracks` itself accepts both `'track'`
AND empty `type_` as real songs (line 660: `type_ in ('track', '')`).
Releases where Discogs returns some real tracks with an empty `type_`
field would be undercounted, which would silently disagree with the
repair job's fallback `_get_expected_total` path (which calls into
`get_album_tracks_for_source` and therefore uses the client's count).
Extracted the filter into `count_discogs_real_tracks(tracklist)` —
single source of truth for the rule, testable in isolation, and the
worker call site is now a one-liner that names what it's doing. Also
defensive about the input shape: `type_ == None`, missing field, and
empty/None tracklist all handled cleanly.
10 tests pin the behavior:
- empty/missing/None type_ all count as a real track (the kettui case)
- 'heading', 'index', 'sub_track' excluded
- unknown future type strings excluded conservatively
- realistic multi-disc tracklist with mixed shapes counts correctly
- empty/None input returns 0 without raising
Credit: kettui — the PR #374 review comment that flagged this.
Reported on Discord by winecountrygames. The library "Reorganize" tool
had several layered bugs that all traced to the same root cause: the
endpoint reinvented every wheel post-processing already turns — its own
template engine, its own disc-number resolution from file tags, its own
sidecar sweep, its own collision detection — and each had drifted from
the canonical path used by fresh downloads. Reported symptoms:
- 3-disc Aerosmith deluxe collapsed to a flat single-disc layout
- Half the tracks on other albums silently skipped, no error / no count
- Re-runs left empty leftover album folders cluttering the artist dir
Architecture: stop reinventing wheels. Route reorganize through exactly
the same pipeline downloads use. Per-album:
1. Fetch the canonical tracklist from a metadata source (Spotify /
iTunes / Deezer / Discogs / Hydrabase) using the album's stored
source IDs. New `core/library_reorganize.py::plan_album_reorganize`
does this — primary-source-first, fall through priority chain
unless the user picked a specific source in the modal (strict mode).
2. For each local track, find the matching API entry via a scored
candidate matcher. Score components: exact-title (100),
substring-with-length-ratio (40-90), track-number agreement (20).
Hard reject when the two titles have different version
differentiators (Remix vs no-remix means different recordings,
not annotation drift). Below threshold = unmatched, surfaced as
"not in source's tracklist, left in place" rather than silently
mis-routing.
3. Copy the file to a per-album staging directory, build the same
context dict the import flow builds (`spotify_album` /
`track_info` / etc. with `is_album_download=True` so the path
builder enters ALBUM mode, not SINGLE mode), call
`_post_process_matched_download(...)` — same function fresh
downloads use. Post-process handles tagging, multi-disc subfolder
decisions, sidecar regeneration, AcoustID verification.
4. Read `context['_final_processed_path']` to learn where it landed.
Update `tracks.file_path` in the DB BEFORE removing the original
(DB-update failure leaves the file at both locations, recoverable
via library scan; the reverse would orphan the row). Delete
per-track sidecars (post-process recreates them at the new
destination).
3 concurrent workers per album via ThreadPoolExecutor, matching the
download path's per-batch worker count. State mutations all guarded by
a single lock; staging filenames carry a UUID prefix so concurrent
copies of identically-named source files don't overwrite each other.
Source picker in the modal lets the user choose which source to read
the tracklist from. Two endpoints feed it:
- `/api/library/album/<id>/reorganize/sources` — sources for THIS
album that are both authed AND have a stored ID. For the per-
album modal.
- `/api/library/reorganize/sources` — all authed sources globally.
For the bulk "Reorganize All" modal where per-album ID coverage
varies.
When the user picks a specific source, the orchestrator runs in
`strict_source=True` mode (no fallback chain) — picking Spotify means
"use Spotify or fail", not "use Spotify and silently fall back."
Preview endpoint shares the same planning logic as apply via
`preview_album_reorganize` — the destination path comes from the same
`_build_final_path_for_track` post-process uses, so what you see in
the preview is exactly what you get on apply.
Empty destination folders (from earlier failed runs OR from the
current run when post-process creates a dir then fails AcoustID)
get cleaned up after each successful run: walk up to the artist
folder from any successful destination, prune empty album-sibling
folders one level deep. Bounded scope = won't touch unrelated user
dirs.
Web_server.py shrinks by ~450 net lines. The endpoint handler is now
a thin wrapper that builds injected callables (path resolver, post-
process function, DB updater, empty-dir cleaner), spawns a thread
that calls `reorganize_album()`, and returns. All actual logic lives
in `core/library_reorganize.py` where it's unit-testable without
spinning up Flask.
Frontend cleanup: the per-call template input in both reorganize
modals (per-album and bulk) was redundant — the backend always uses
the configured global download template. Removed the input and the
variables-grid reference UI it was for.
39 new unit tests pin every contract:
- source resolution (no_source_id when album has none, fallthrough
chain when primary returns nothing, strict mode bypasses fallback)
- matcher scoring (exact / substring / multi-disc disambiguation /
smart-quote tolerance / dash-vs-parens / bonus-track substring /
Remix-vs-original differentiator rejection / "Real" doesn't false-
match "Real Real Real" / track-number-only no longer fires)
- file safety (DB-update failure leaves original in place, post-
process failure leaves original in place, post-process exception
caught and original preserved, success removes original AND
updates DB in the right order)
- sidecar handling (per-track .lrc/.nfo deleted on success, kept on
failure; album-level cover.jpg/folder.jpg cleaned only when
directory has no remaining audio)
- staging cleanup (recreated between tracks because post-process
nukes it, dir cleaned up on success AND on failure)
- destination-dir prune (empty siblings removed, real album with
files preserved, no recursive sweep)
- source picker (only authed-with-stored-ID sources for per-album,
all authed sources for bulk; strict mode doesn't fall back)
- concurrency (3 workers in flight, state stays consistent under
races, stop_check cuts off pending tasks)
- preview parity (preview produces same destination as apply for
multi-disc; ALBUM mode not SINGLE mode; unmatched/no-path tracks
surfaced with reasons)
Limitations (deliberate punts, NOT in this PR):
- Renamed local titles on multi-disc albums where track_number
also disagrees: matcher returns nothing (track is "not in
source"). Fixable by using duration_ms as a tertiary signal.
- Per-track in-modal source switching with per-album track-count
hints (would need a second API call before opening the modal).
- UI status panel on the artist page during a run — currently
just toasts. Documented as a follow-up PR.
Files:
- core/library_reorganize.py — new module: plan_album_reorganize,
preview_album_reorganize, reorganize_album, available_sources_for_album,
authed_sources, _score_candidate, helpers for staging/post-
processing/finalizing, sidecar + dest-dir cleanup
- core/metadata_service.py — no changes; reused get_album_for_source,
get_album_tracks_for_source, get_source_priority,
get_client_for_source
- web_server.py — three endpoints (preview / apply / sources GETs)
are thin wrappers; -450 net lines
- tests/test_library_reorganize_orchestrator.py — 39 tests covering
every contract above
- webui/static/library.js — source picker UI in both modals; dead
template input + variables-grid removed
- webui/static/style.css — dropdown option styling fix (white-on-
white was unreadable)
Reported on Discord by winecountrygames — his bug report named the
trigger button (Enhanced view → Reorganize All) and both symptoms
(multi-disc collapse, half-album skip), which let the diagnosis go
straight to the architectural problem.
Reported on Discord by winecountrygames — Spotify auth granted, then
re-banned for 4 hours within ~30 seconds, repeatedly. Trace from his
captured log:
< 12:05 [pre-log] Spotify ban active when log starts
15:21:27 First ban EXPIRED → 5-minute post-ban cooldown begins
15:26:27 Cooldown ends, spotify_client.is_authenticated() probe
allowed again → client initialized
15:26:59 First Spotify API call after cooldown — get_artist_albums
for an artist whose discography a background worker was
enriching — gets 429 immediately with no Retry-After
header → new ban activated for 14400s (4 hours)
Root cause: `_POST_BAN_COOLDOWN = 300` (5 minutes) is shorter than
Spotify's actual server-side memory of the previous offense. The
cooldown exists specifically to prevent the "ban expires → we probe →
re-ban" cycle (`spotify_client.py:65-68` documents that intent
explicitly), but the value was wrong: Spotify's server still
considered this user banned 5 minutes after our local ban window
ended, so the very first call after cooldown got slapped.
The 4-hour re-ban itself is correct behavior — `_BASE_MAX_RETRIES_BAN`
fires when spotipy reports "max retries", which means the client
exhausted its internal retry budget on 429s before raising. That's a
severe-ban signal and a long default is the right response.
Fix: bump `_POST_BAN_COOLDOWN` to 1800 seconds (30 min). This is the
smallest change that addresses the immediate "re-probe → re-ban" loop
in the report. 30 minutes is an empirical floor — long enough for
Spotify to actually clear its server-side memory in the cases we've
observed, short enough not to keep functional users locked out beyond
necessary. Can be revisited if reports persist.
What this PR does NOT fix (important context for the same user):
This bump only helps the "ban expires → we re-probe → re-ban" loop.
It does NOT help winecountrygames's other symptom — Spotify being
banned within 30 seconds of his FIRST EVER authorization (no prior
ban). That's a separate failure mode: on first auth, enrichment
workers immediately fan out across the user's library (250 artists
in his case), hammering Spotify endpoints with bulk get_artist_albums
calls before any rate-limit feedback can land. Spotify's hidden
per-endpoint daily quotas — which BoulderBadgeDad has empirically
documented but the global rate limiter doesn't see — flag the burst
and impose a multi-hour cooldown that LOOKS like a bot-detection ban
to us. A proper fix needs a fresh-auth ramp-up: start with very low
Spotify QPS for the first N minutes, scale up only if no rate-limit
feedback arrives. That's a separate PR.
Documented as additional follow-ups (NOT in this change):
- Adaptive cooldown that scales with the size of the previous ban —
a 4-hour MAX_RETRIES ban probably warrants a 1-hour cooldown,
while a 60-second Retry-After-honored ban can resume in 5 minutes.
The system already distinguishes these in `_set_global_rate_limit`,
it just doesn't propagate the distinction to cooldown duration.
- Probe-with-light-call pattern — make the first post-cooldown call
a single inexpensive endpoint (`current_user`) rather than
allowing a background worker's heavy `get_artist_albums` to be
the canary. Failed probe extends cooldown silently instead of
triggering a fresh 4-hour ban.
- Fresh-auth ramp-up (per the limitation above).
Files:
- core/spotify_client.py — `_POST_BAN_COOLDOWN` 300 → 1800. Comment
expanded to cite the report so the value isn't bumped back without
context.
- webui/static/helper.js — WHATS_NEW entry under 2.40 explaining
the change for affected users.
No tests added — the cooldown logic itself is unchanged, only the
constant. Tests asserting on a constant value are theater.
Reported on Discord by winecountrygames — his captured log made the
"ban-expires-to-re-ban" timing chain unambiguous.
The "Link Tidal Account" device-flow UI displayed a verification URL
like `link.tidal.com/XBXYT` that, when clicked, navigated back to the
SoulSync origin (e.g. `http://localhost:8889/link.tidal.com/XBXYT`)
instead of to Tidal's activation page.
Root cause: tidalapi returns `login.verification_uri_complete` as a
schemeless string. settings.js drops it straight into `<a href>`, and
browsers treat schemeless hrefs as same-origin relative URLs.
Normalize the URI in `start_device_auth` — if it doesn't already
start with `http://` or `https://`, prepend `https://`. Same
treatment for the `link.tidal.com/{user_code}` fallback so the
defensive path stays well-formed too.
Reported on Discord by Netti93: with Tidal configured for "HiRes only"
and "Allow Quality Fallback" disabled, tracks were still downloading
successfully — as m4a 320kbps files. Some "successful" downloads were
less than half the file size of the same track pulled via Tidarr/tiddl
from the same Tidal account.
Root cause: Tidal's API silently degrades to the best quality your
account + the track + your region permits. Setting
`session.audio_quality = Quality.hi_res_lossless` and calling
`track.get_stream()` on a track that's only available in AAC returns
an AAC stream with no error. The downloader wrote the m4a file to
disk, the ~7MB size sailed past the 100KB stub threshold, and the
download reported success.
The pre-existing "verify quality wasn't silently downgraded" block
only LOGGED a warning when this happened; it did not fail the tier.
Two knock-on effects:
- Users with "HiRes only, no fallback" got m4a files anyway, which
defeats the setting entirely.
- The worker-level fallback chain (hires → lossless → high → low)
couldn't advance past the first tier, because every tier
"succeeded" at whatever Tidal happened to serve.
Fix: after `track.get_stream()`, compare `stream.audio_quality`
against the tier we asked for using a rank-based ordering:
LOW < HIGH < LOSSLESS < HI_RES < HI_RES_LOSSLESS
- Same tier or higher → accept (so the occasional Tidal upgrade
doesn't get rejected just because it's not an exact match).
- Lower tier → reject THIS tier. The loop `continue`s and the next
fallback tier is tried, or the whole download fails honestly
when the user has fallback disabled. The existing final-error
log already has a hint directing users to enable fallback if
they want automatic Lossless substitution.
- Unrecognized `audioQuality` value (e.g. a new Tidal tier we
haven't mapped) → reject conservatively, so the next fallback
tier gets a chance and the diagnostic log names the unknown
value.
Why the rank-based approach instead of strict equality:
Tidal's API doesn't technically promise an exact-tier match on
serving; on tracks that are flagged in its catalog as a higher
tier, it can serve higher than the session setting. Rejecting
higher-than-asked quality would be user-hostile. And the `HI_RES`
(legacy MQA) value — not in tidalapi's modern `Quality` enum but
possibly still present on old catalog entries — needs to rank
below `HI_RES_LOSSLESS`: users asking for true lossless HiRes
should reject MQA since MQA is a lossy format.
tidalapi's `Quality` enum is a `str` subclass whose VALUES (not
member names) match what the Tidal API returns in the
`audioQuality` field (e.g. `Quality.hi_res_lossless.value ==
'HI_RES_LOSSLESS'`, `Quality.low_320k.value == 'HIGH'`). Both
sides of the comparison are coerced to `str` before use, so the
check is robust to whichever tidalapi version exposes the served
quality as an enum or a plain string.
The check is extracted as `_verify_stream_tier(stream, q_info,
q_key) -> (ok, reason)` at module scope — a pure function with no
I/O, unit-tested independently. Ten tests: match, three upgrade
cases (LOSSLESS → HI_RES_LOSSLESS, LOSSLESS → HI_RES, LOW → any
higher), three downgrade cases (the reported HiRes → AAC, HiRes
Lossless → MQA HiRes, Lossless → AAC), one unrecognized-tier case,
and two defensive paths for older tidalapi builds without
`audio_quality` on the stream object and for QUALITY_MAP entries
that lack `tidal_quality` (e.g. tidalapi wasn't importable at
module load). Test stub updated to use uppercase `Quality` values
matching real tidalapi so case-sensitivity regressions get caught.
Also removed the old codec-string-based warning block — the new
tier check is strictly stronger, and keeping the warning around
would just be dead code waiting to drift out of sync.
Deliberately NOT tackling in this PR (documented as follow-ups):
- Bit-depth verification of HiRes FLAC files via mutagen. The
`stream.audio_quality` tier check catches the main "HiRes
requested, got AAC" case; bit-depth would only matter if Tidal
labeled a stream HI_RES_LOSSLESS but served a 16-bit FLAC
(`Stream.bit_depth` isn't reliable for this — tidalapi defaults
missing `bitDepth` fields to 16, so a trust-the-stream check
would spuriously reject valid HiRes whenever Tidal omits the
field). A proper fix runs mutagen post-download to inspect the
actual file, then decides whether to delete + retry the next
tier — a whole new failure mode with design trade-offs that
deserve their own PR. The support logs don't show this
happening.
- The "manual remap still says Not Found" symptom. Might be
downstream of this same bug (silent-AAC "success" hitting a
later rejection), might be a separate task-state issue. Not
guessing without logs from the retry path.
- Quality-aware stub threshold. 100KB is a reasonable floor for
real stub/preview detection and there's no evidence the
universal threshold is misfiring in the wild.
Field-verified status: desk-verified via unit tests and empirical
checks against a live tidalapi import (confirming the `Quality`
enum's str-subclass behavior). Not yet smoke-tested end-to-end
against a real Tidal account with a HiRes-only-no-fallback
setting — Netti93 or anyone else with that config should notice
either the fix working (non-HiRes tracks fail honestly with a
clear log line) or any regression before wider release.
Files:
- core/tidal_download_client.py — new `_verify_stream_tier` helper
and `_QUALITY_RANK` table at module scope, called in the
download loop after the stream is fetched and before any
bandwidth is spent. Removed the old inline codec-based warning
since the new check supersedes it.
- tests/test_tidal_stream_tier_verification.py — ten tests covering
match / upgrade / downgrade / unknown / defensive paths.
- tests/test_tidal_search_shortening.py — fake `Quality` values
brought in line with tidalapi's real values so both files share
a consistent stub regardless of pytest collection order.
- webui/static/helper.js — WHATS_NEW entry under 2.40 describing
the rank-based tier comparison.
Reported on Discord by Netti93 — the "same account works via
Tidarr" comparison narrowed the cause to SoulSync's download path
rather than an account/region issue.
Reported by sassmastawillis: the Album Completeness maintenance job
scans 3127 albums in 0.1 seconds and reports 0 findings — for every
user, regardless of whether their library is actually complete.
Restoring an older DB surfaced 7 correct findings, so the code logic
works; the DB state is what's making everything look complete.
Root cause: `albums.track_count` is only ever written by server-sync
paths — Plex's `leafCount`/`childCount` and SoulSync standalone's
`len(tracks)`. It's the OBSERVED count of tracks SoulSync has indexed,
which is always exactly what `COUNT(tracks)` returns for that album.
The completeness job treated it as the EXPECTED total and compared it
against the observed count. They're equal by construction, so
`actual >= expected` is always true: skip, 0.1s scan, 0 findings.
Fix: new `api_track_count INTEGER` column on `albums`, written only by
metadata-source code paths. Populated in two places so the scan is
fast and the fallback is robust.
1. Enrichment workers — shared helper `set_album_api_track_count`
in `core/worker_utils.py`. Called by each worker's existing
`_update_album` method alongside its other album-column UPDATEs:
- spotify_worker: `album_obj.total_tracks` from the Spotify Album
dataclass (already in hand, zero new API calls)
- itunes_worker: same, from the iTunes Album dataclass
- deezer_worker: `nb_tracks` from full_data, falling back to
search_data when the full lookup didn't run
- discogs_worker: count of tracklist rows where `type_=='track'`
(Discogs tracklists interleave heading and index rows that
shouldn't count as songs)
Helper skips the write on zero/None/negative/non-numeric inputs
so a source lacking track info can't clobber a good value a
different source already wrote. Caller owns the transaction —
helper just queues an UPDATE on the caller's cursor without
committing, so it batches cleanly with each worker's existing
multi-UPDATE pattern.
Hydrabase worker deliberately not touched — it's a P2P mirror
that doesn't write album metadata to the local DB. Hydrabase-
primary users hit the fallback path below.
2. Album Completeness repair job — new `al.api_track_count` column
in the SELECT, read first in the scan loop. On miss (album never
enriched, or enrichment workers haven't run yet on a fresh
install), falls through to the existing `_get_expected_total()`
API lookup and persists the result via the same shared helper
(wrapped in connection/commit management since the repair job
runs outside a worker's batched transaction).
Also removed `al.track_count` from the scan's SELECT — now unused
since the observed count was the whole source of this bug, and
leaving a dead SELECT would invite a future engineer to re-introduce
the same comparison.
Help text on the job card was reworded so it honestly describes
current behavior ("counts cached during normal enrichment are used
when available; otherwise the job queries a metadata source
directly") rather than the old "active provider first, then others
as fallback" phrasing, which doesn't match how the cache actually
fills — any enrichment worker that runs can populate it, and the
last writer wins. Document-only follow-up if this edge case ever
bites in practice: add a `api_track_count_source` column so the
scan can prefer the configured primary source's count over others
(e.g. deluxe vs. standard edition mismatches). Not worth the
complexity today.
For existing users, the first completeness scan after upgrade is
fast to the extent their library is already enriched: the workers
already ran and populated `api_track_count` on their normal schedule.
For brand-new installs, the scan's fallback path handles the cold
start — slower, but correct, and subsequent scans are fast.
Does NOT affect:
- Download / post-processing / wishlist / sync code paths — none
of them read `track_count` for completeness semantics.
- Plex / Jellyfin / Navidrome / standalone sync — still write
`track_count` exactly as before; `api_track_count` is a separate
column they never touch.
- Other repair jobs.
- Any UI path — same finding schema, just correct counts now.
Files:
- database/music_database.py — idempotent migration adding
`api_track_count INTEGER DEFAULT NULL` to the existing album-column
check block.
- core/worker_utils.py — new `set_album_api_track_count` helper with
the documented skip-on-bad-input contract.
- core/spotify_worker.py, itunes_worker.py, deezer_worker.py,
discogs_worker.py — one-liner call from each `_update_album`.
- core/repair_jobs/album_completeness.py — scan uses the cache;
fallback path persists API-lookup results via the shared helper;
help text updated to match actual behavior.
- tests/test_worker_utils_album_track_count.py — 9 tests covering
the helper's write/skip contract + no-commit invariant.
- tests/test_album_completeness_job.py — 2 tests for the repair
job's fallback-path wrapper.
- webui/static/helper.js — WHATS_NEW entry.
Credit: sassmastawillis spotted the bug; the "restored older DB
finds 7 albums" signal pinpointed DB state over code logic and
made the diagnosis tractable.
Typing "michael jackson" returned 7 identical-looking cards because
MusicBrainz has many different PEOPLE sharing a canonical name — the
King of Pop plus a NZ poet, a photographer, a mashup artist, a
didgeridoo player, and more, all scoring 80+ on exact-name match.
All 7 passed the score filter. All 7 rendered with the same
fallback image because iTunes/Deezer only know the famous one.
Fix dedupes by normalized (lowercase, whitespace-trimmed) name before
building Artist dataclasses. Keeps the highest-scoring entry per name,
so the King of Pop (score 100) wins over the others (all score 80-81).
Artists with genuinely different names stay separate — a search for
"the beatles" still surfaces tribute bands if they're above threshold.
Implementation note: fetch `max(limit*3, 10)` from MB instead of
`limit` directly, so the dedup pool is large enough to still return
`limit` distinct artists after collapsing duplicates. Previously the
raw fetch was capped at the caller's limit, which would have left
fewer-than-requested results after dedup for common names.
3 new tests (49 total):
- Dedupe collapses 5 same-named entries to 1 (keeps highest score).
- Dedup key is case-insensitive and whitespace-normalized.
- Dedup preserves distinct names ("The Beatles" vs "The Beatles Revival"
stay separate).
Live-verified: "michael jackson" now returns 1 card, "kendrick lamar"
returns 1 card.
Credit: kettui spotted duplicate Michael Jackson cards in the search UI.
Three bugs from kettui's follow-up review pass on the MusicBrainz
search PR, all fixed in one commit because they share UI context.
1. Missing artist images on MB artist results
MusicBrainz doesn't store artist images directly. My earlier commit
returned `image_url=None` on every artist result and trusted the
frontend's lazy-loader — but the lazy-loader's `/api/artist/<id>/image?
source=musicbrainz` endpoint had no handler for MusicBrainz, so it
silently returned None and the emoji placeholder stayed.
Fix plumbs the artist name through:
- `renderCompactSection` stashes `data-artist-name` on artist cards.
- `search.js` and `downloads.js` lazy-loaders pass `name=<artist>` as a
query param.
- `/api/artist/<id>/image` accepts an optional `name` param.
- `metadata_service.get_artist_image_url` has a new `musicbrainz`
branch: since MB has no artist art, it searches fallback sources
(iTunes/Deezer by configured priority) for the artist name and
returns the first image found.
Verified live — Metallica/Kendrick Lamar/Daft Punk all resolve to
Deezer artist images via the name lookup.
2. total_tracks off-by-one on tracks with a release
`_recording_to_track` initialized `total_tracks = 1` and then summed
media track-counts on top. For an 11-track album, it reported 12. An
adapter-level regression introduced when the recording-projection
helper was extracted during the main MB refactor.
Fix: initialize at 0, sum normally. Standalone recordings with no
release (can happen for uncredited remixes etc.) still report 1 via
an explicit fallback — so the existing "single track" case isn't
broken.
3. "Artist Album Title" queries buried specific albums in the
discography list
Bare-name queries like "The Beatles Abbey Road" used to resolve "The
Beatles" as the artist and then browse their full discography — Abbey
Road was buried alphabetically among 200+ releases instead of being
the top result.
Fix adds a title-hint extractor. When the query starts with the
resolved artist name followed by more words, the trailing portion is
treated as a title hint. Browse results are filtered to those whose
release-group title contains the hint. If the filter matches nothing,
falls back to text-search with the hint as the title (the "keep the
old split-by-whitespace fallback" path kettui called for). If text-
search also misses, shows the full discography rather than nothing.
10 new tests in tests/test_musicbrainz_search.py (46 total):
- Title-hint extractor: basic match, case-insensitive, whitespace
tolerance, bare-artist-no-hint, artist-not-prefix-no-hint, word-
boundary required (no false splits on "Metallicasomething").
- Browse filtering by title hint.
- Text-search fallback when the title hint matches nothing in browse.
- Bare-artist queries return the full discography unfiltered.
- total_tracks for single-release, multi-disc, and no-release cases.
Clicking a MusicBrainz album returned 404 because the browse-based
search path now stores release-GROUP MBIDs in Album.id, but `get_album`
still hit `/ws/2/release/<mbid>` directly. Release-group MBIDs don't
resolve as release MBIDs — MB 404s. User log:
GET /api/spotify/album/b88655ba...?source=musicbrainz → 404
Error fetching release b88655ba...: 404 Client Error
The fix requires a two-step resolution for the new browse path:
1. Look up the release-group with `inc=releases+artist-credits` to get
the list of releases inside (original + reissues + regional + promo
editions). MB release-groups routinely hold 5-20 releases.
2. Pick a representative release: prefer Official status over Promo,
prefer releases with a real tracklist over stubs, then earliest date.
3. Fetch that release's full tracklist via `get_release`.
Two extra seconds at the 1-rps rate limit, but it's on click, not on
search results rendering.
Structure:
- New `MusicBrainzClient.get_release_group(mbid, includes)` method.
- New `_pick_representative_release(releases)` helper encapsulates the
ranking logic.
- Tracklist projection extracted into `_render_release_as_album` so
both paths share the same shape construction.
- `get_album` tries release-group first; falls back to direct release
lookup when the MBID turns out to be a release from the text-search
fallback path.
- Canonical Album.id stays the release-group MBID so a re-fetch with
the same URL hits the same code path idempotently.
3 new tests (now 33 total):
- End-to-end release-group → release resolution with mocked client
- Fallback to direct release lookup when rg lookup misses
- Representative-release picker ranks correctly
Verified against live API with the exact MBID that 404'd for the user
(b88655ba... for DAMN. by Kendrick Lamar): now returns in 1.2s with
the full 14-track listing (BLOOD., DNA., YAH., ELEMENT., FEEL., ...).
Self-audit catch: my earlier cover-art commit claimed 'the frontend's
<img onerror> fallback handles 404s' — that was wrong. The enhanced
search result images in shared-helpers.js renderCompactSection and all
five gsearch-item/track templates in downloads.js render bare
`<img src="...">` with no fallback. With the MusicBrainz adapter now
emitting Cover Art Archive URLs deterministically (no HEAD probe),
albums that don't have cover art would show the browser's broken-image
icon instead of the emoji placeholder.
Two fallback shapes:
- shared-helpers.js renderCompactSection: the `<img>` sits inside a
card with a sibling placeholder pattern. On error, replace the img's
outerHTML with the placeholder div, matching the shape used when
config.image is missing entirely.
- downloads.js gsearch items: the `<img>` sits inside a
`.gsearch-item-art` div whose default text content is the emoji fallback
(🎤 / 💿 / 🎶 / 🎵). On error, set parentElement.textContent to the
emoji, which wipes the img and shows the glyph. Same shape as the
"no image_url" branch.
Applies to every card type that renders a user-provided image URL so
the fix covers all sources that might return 404s — MB is the most
common offender but iTunes/Deezer/Discogs can all miss too.
Tested against the live MB API: Metallica albums without CAA cover art
now show the 💿 emoji instead of a broken-image icon.
The source-picker refactor introduced a new stable DOM structure inside
`#gsearch-results`:
<div id="gsearch-results"> <!-- max-height: 60vh, flex-col -->
<div id="gsearch-source-row" /> <!-- icon row, controller-rendered -->
<div id="gsearch-fallback-banner" />
<div id="gsearch-body" /> <!-- surface renders results here -->
</div>
But the companion CSS never landed. `#gsearch-body` had default block
layout, so when results exceeded the 60vh cap, they clipped silently
instead of scrolling. The old structure had `.gsearch-results-body`
with `overflow-y: auto; flex: 1` directly inside the panel; that rule
still exists but its selector now matches a nested div with no flex
parent, so `flex: 1` is a no-op and overflow doesn't trigger.
Fix: give the three stable children the right flex behaviour so the
body fills remaining space and scrolls.
- `#gsearch-source-row` and `#gsearch-fallback-banner` stay at natural
height (flex-shrink: 0).
- `#gsearch-body` grows (flex: 1 1 auto), can shrink below content
height (min-height: 0 — this is the critical bit, otherwise flex
items won't shrink below their intrinsic size and overflow never
triggers), and scrolls vertically.
Styled scrollbar matches the rest of the panel (4px, translucent thumb).
Three related fixes to make album/track results look like a real
artist discography instead of a firehose of fan-compiled bootlegs.
1. Drop 'compilation' from the release-group browse primary-type filter.
MB's OR filter (`type=album|ep|single|compilation`) silently breaks
when 'compilation' is included — Metallica drops from 1076 matches
to 82 because `compilation` is a SECONDARY type on MB, not a primary
type. The invalid value corrupts the filter for all types, not just
itself. Now we request `type=album|ep|single` which returns the full
1076; actual compilations (primary=Album + secondary=[Compilation])
are filtered out by the studio-preference logic below.
2. Filter release-groups with non-studio secondary-types
(Live/Compilation/Soundtrack/Remix/Demo/Mixtape/Interview/Audiobook/
Audio drama). For Metallica, the first 100 browse results are 12
studio albums + 83 live bootlegs + 5 compilations — without this
filter the Albums section was dominated by 2019-2021 broadcast
recordings. Falls back to the unfiltered list if filtering leaves
the result set empty (covers live-only niche artists).
3. Sort chronologically ASC by first-release-date. Wikipedia-style
discography ordering — debut album on top, then chronological.
Previous DESC sort put the most recent release on top which, for
prolific artists, meant 2020s material before their classics.
Track side of the same fix:
- Re-orders each recording's `releases` array to put studio releases
first before `_recording_to_track` picks up the first release for
album context. Without this, MB's arbitrary release order often
buried the canonical studio album under random live bootlegs.
- Filters out recordings that only exist on live/compilation release-
groups (keeps the ones with at least one studio release). Falls
back to the full set if the artist has no studio recordings at all.
- Sorts recordings by earliest studio-release year ASC so classic
tracks surface first.
Smoke test against live MB API confirmed:
- Artists: [Metallica score=100]
- Albums: Kill 'Em All (1983) → Ride the Lightning → Master of Puppets
→ ...And Justice for All → Metallica (Black Album) → Load → Reload
→ St. Anger → Death Magnetic → Lulu (2011)
- Tracks: real Metallica recordings (Killing Time, Nothing Else
Matters, Creeping Death, etc.) — a few remastered demos still leak
in where MB metadata quality is thin, but the bulk is correct.
- Total latency: 3.5 seconds.
4 new tests covering the studio filter, live-only fallback, preferred
release ordering, and live-only recording exclusion.
Credit: kettui flagged the poor MB results during PR #371 review.
The previous commit's `browse_artist_recordings` call passed
`inc=releases+artist-credits` — but MusicBrainz's recording browse
endpoint rejects `inc=releases` with HTTP 400. The adapter's error
handler returned an empty list, so the Tracks section stayed empty
even though the fix was supposed to populate it.
Browse without release info is useless for our search UI (tracks
would render with no album), so swap to the fielded Lucene search
`arid:<mbid>` on the `/recording` endpoint. That's the canonical MB
pattern for "find recordings by this artist WITH release context":
- arid: search accepts the artist MBID and returns recordings with
`releases` (release-group, date, media) embedded in each result.
- One API call per lookup, same as browse would have been.
Renamed the method to `search_recordings_by_artist_mbid` so the name
matches its behaviour — it's a search, not a browse. Adapter updated
to call the new name; tests updated to match.
Verified against the live API: Metallica's MBID returns 5 recordings
in ~1.8 seconds (vs the previous 400 error).
26 new unit tests in tests/test_musicbrainz_search.py covering:
- Cover Art URL construction (release + release-group scope, empty MBID,
unknown scope fallback)
- Structured query splitting (hyphen, en-dash, em-dash, bare name, no
false-positive splits on hyphens-inside-words)
- Artist search: score filtering, strict=False call contract, exception
handling, genre extraction from MB tags, mbid/name validation
- Top-artist resolver: memoization by normalized query, sub-threshold
returns None, negative-result caching, empty-query short-circuit
- Album search routing: bare query → browse path, structured query →
text path, no-artist-match falls back to text, text path score filter
- Track search routing: browse path, dedupe-by-title across
live/compilation variants, structured query → text path, text path
score filter
All mock the underlying MusicBrainzClient — no network calls.
Also adds a WHATS_NEW entry under 2.40 explaining the three user-visible
changes: Artists section now populates, album/track results match the
searched artist instead of random title collisions, and search completes
in ~3 seconds instead of 30+.
Cover Art Archive URLs are deterministic from the MBID: a GET either
307-redirects to the image or returns 404. The previous adapter fired
`requests.head(timeout=3)` per search result to probe for the image
first. 10 results × 3s worst-case = up to 30s of blocking HEAD calls
before a search returned.
The probe was defensive overhead — the frontend already handles 404 via
`<img onerror>` fallback. Building the URL deterministically and letting
the browser load it lazily collapses the tail latency to the real MB API
calls (artist-search + browse = ~3s at the 1-rps rate limit).
Also prefer release-group scope over per-release scope when both are
available — release-group covers every edition of an album, so the hit
rate is noticeably higher than pinning to a specific regional release.
Removes now-unused `self._art_cache` and the `requests` import.
Bare name queries (typing 'metallica') now resolve to an artist MBID via
the fuzzy search added in the previous commit, then BROWSE that artist's
release-groups and recordings instead of text-searching release/recording
titles. That's the only way to fix the core garbage-results issue: MB
indexes release/recording titles, not artist names, so 'recording:metallica'
matches random tracks literally titled 'Metallica' (all scoring 100).
Structure:
- `_split_structured_query` — detects 'Artist - Title' / 'Artist – Title' /
'Artist — Title' shapes. When present, text-search is correct (user
gave an explicit title to match).
- `_resolve_top_artist` — memoized per-instance lookup for the top-scoring
artist MBID. Backend fires artists/albums/tracks searches in parallel
against one shared client instance, and albums+tracks both need the
same artist lookup. Cache + lock means one HTTP call instead of three.
- `_release_group_to_album` / `_recording_to_track` — shared projection
helpers between the browse and text paths so both paths return the
same dataclass shape.
Search flow per kind:
- `search_albums('metallica')` → resolve top artist → browse release-groups
with `type=album|ep|single|compilation` → sort by type priority then
release date desc → Album dataclasses for top N.
- `search_tracks('metallica')` → resolve top artist → browse recordings
with `inc=releases+artist-credits` → dedupe by normalized title (MB
has many live/compilation variants of the same song) → sort by release
date desc → Track dataclasses for top N.
- `search_albums('foo - bar')` → structured query → text-search path
(unchanged behavior, now score-filtered to 80+).
- `search_tracks('foo - bar')` → same.
- Both text-search paths also dedupe through `_search_albums_text` /
`_search_tracks_text` helpers, which apply the 80-score filter that
the artist-first path gets free from the resolver's threshold.
Also dedupes text-path tracks through the new `_recording_to_track`
helper, replacing ~60 lines of inline projection code. Net change is
more lines overall (browse + helpers) but the text paths shrank and
the garbage-results issue is fixed.
Credit: kettui flagged the missing Artists section + unusable track
results during PR #371 review.
`MusicBrainzSearchClient.search_artists` has been a `return []` stub
since the feature landed, with a comment claiming the MB tab 'doesn't
show artists.' That's why kettui saw a missing Artists section on the
search page — not a missing render, a hardcoded empty list.
Re-enable it properly:
- New `strict=False` parameter on `MusicBrainzClient.search_artist`
sends a bare Lucene query instead of `artist:"..."`. MusicBrainz
matches bare queries against alias+artist+sortname indexes together,
which is the right behavior for user-facing fuzzy search (finds
typos, aliases, sortname variants). `strict=True` remains the
default for enrichment/AcoustID callers that want exact matches.
- Adapter filters results to `score >= 80`. MB assigns a 0-100 Lucene
score on every hit; the true artist + close variants score 100,
tribute bands and lookalikes typically land in the 40-65 range.
The cutoff keeps "Metallica" (100) and drops "Black Metallica
Tribute Band" (60) without hand-curated lists.
- Results returned as the same `Artist` dataclass used elsewhere in
the search-tab adapter layer. `popularity` carries the MB score
(0-100) so the frontend can sort/highlight top matches if desired.
Add `browse_artist_release_groups(mbid)` and `browse_artist_recordings(mbid)`
to MusicBrainzClient. These hit `/ws/2/release-group?artist=<mbid>` and
`/ws/2/recording?artist=<mbid>` respectively — the correct MusicBrainz
pattern for "give me everything linked to this artist."
Why this matters: our current search adapter calls text-search
(`release?query=...` / `recording?query=...`) for albums and tracks,
which matches entity titles literally. Typing "metallica" hits unrelated
releases titled "Metallica" and recordings named "Metallica" by obscure
bands — every garbage match scores 100 because they're all exact title
matches on the wrong field.
Browse walks the artist→release-group and artist→recording links
directly. Once we know the artist's MBID (from `search_artist`), browse
returns their actual discography instead of title collisions.
No behavior change yet — search adapter still uses the old path. Follow-
up commit wires the new endpoints in.
Reference: https://musicbrainz.org/doc/MusicBrainz_API — "Browse queries
retrieve entities linked to a known entity" vs search.
MusicBrainz mandates a meaningful User-Agent with contact info, warning
that bare strings can trigger IP blocking under load. Our client was
sending `SoulSync/2.3` with no contact — and the search adapter passed
an app version hard-coded at "2.3" that's now stale (UI is at 2.40).
Fix: default contact to the project URL (`https://github.com/Nezreka/SoulSync`)
when no email is supplied, so every request lands as
`SoulSync/<version> ( https://github.com/Nezreka/SoulSync )`. Drop the
search-adapter version suffix to a generic "2" since the exact UI minor
version would add noise to every MB request without helping operators
track issues.
Reference: https://musicbrainz.org/doc/MusicBrainz_API — "it is
important that your application sets a proper User-Agent string."
Clicking 'View Discography' on the Discover hero slideshow was calling
navigateToArtistDetail(id, name) without the third 'source' argument.
loadArtistDetailData then omits the `source` query param, so
/api/artist-detail falls through to a local DB lookup and returns 404
for artists that don't exist in the library — which is nearly every
hero artist, since they come from discover similar-artists.
Regression from the unification PR (93f1941) that rewrote the click
handler to route through the standalone /artist-detail page instead
of the old inline Artists view. The rewrite didn't thread the source.
Backend already includes `artist.source` on each hero entry. Fix:
- Stash artist.source as data-source on #discover-hero-discography
when displayDiscoverHeroArtist populates the card.
- Read data-source in viewDiscoverHeroDiscography and pass it as the
third arg to navigateToArtistDetail, so the eventual API call
includes `?source=itunes/deezer/etc.` and returns the synthesized
discography.
Reproduced by clicking View Discography on a non-Spotify hero artist
(log showed `GET /api/artist-detail/76258852?name=ДЕТИ+RAVE → 404,
Getting artist detail for ID: 76258852 (source=library)`).
Two bugs in the previous review-fix commits, found during a Cin-standard
re-audit:
A) Soulseek handoff stale state.query overrode the global widget's query
The previous fix pre-set basicInput.value before clicking the Search
page's Soulseek icon. But the click triggers onSoulseekSelected with
the controller's CURRENT state.query — which is whatever the user
last typed on /search, not the global widget's query. The Search
page callback then ran `if (query) basicInput.value = query;` and
overwrote the just-set value with the stale one before firing
performDownloadsSearch.
Fix: expose searchController as `_searchPageController` (mirrors
`_searchPageRestoreOnEnter` already at module scope). Global
widget's _gsNavigateToSearchPage syncs `_searchPageController.state.query`
to its own query before clicking the icon. Also added a fallback
for the case where the icon doesn't exist yet (controller still
mid-init): swap sections + run performDownloadsSearch directly.
B) Single _requestSeq token leaked loadingSources across sources
The earlier "stale request" fix used one global _requestSeq. But
when the user switched Spotify → Deezer mid-fetch, the Spotify
abort's catch block bailed (1 !== 2), leaving 'spotify' in
loadingSources forever — permanent spinner on the Spotify icon
even though no fetch was running for it.
Fix: per-source `_sourceRequestIds[src]` map. Same-source
supersession bails (correct), cross-source supersession still
clears the old source's loadingSources entry (correct).
Bonus defensive: submitQuery now invalidates every per-source token
and aborts the in-flight fetch when the query string changes. Catches
the residual edge case where user clears the input — the in-flight
fetch's settle would otherwise write stale data into the just-cleared
state.sources.
Cin flagged that Soulseek was always rendered as configured in the
source picker, even on dev instances with no slskd set up — letting
users click it and fire searches that could never succeed.
Three coordinated changes:
1. web_server.py SERVICE_CONFIG_REGISTRY: add Soulseek entry requiring
`slskd_url`. /api/settings/config-status now reports its real state
alongside every other service.
2. shared-helpers.js _ALWAYS_CONFIGURED_SOURCES: drop 'soulseek'. The
set is now just MusicBrainz + YouTube Music Videos (sources that
genuinely don't need user creds). Soulseek goes through the normal
config-status code path.
3. shared-helpers.js openSettingsForSource: special-case Soulseek to
route to Settings → Downloads tab (where slskd URL field lives,
gated behind the download-source-mode dropdown) and scroll to the
#soulseek-url input. Every other source still routes to Connections
and scrolls to its .stg-service card. Without this, Soulseek's
"click to configure" landed on a Connections card that doesn't
exist (Soulseek's URL/key fields are scoped to the download-source
selection on the Downloads tab).
Two AI-review findings from Cin (kettui) on the source-picker PR:
1. Soulseek handoff from global widget went through metadata flow
_gsNavigateToSearchPage(query, 'soulseek') wrote the query into
#enhanced-search-input and dispatched an input event. The Search
page controller's activeSource was whatever its default was
(spotify, deezer, etc.), so the debounced submitQuery ran the
enhanced /api/enhanced-search flow instead of the raw Soulseek
file search. The `src` parameter was effectively ignored.
Fix: when src === 'soulseek', pre-fill #downloads-search-input
directly and click the Search page's Soulseek icon. The icon click
triggers the controller's onSoulseekSelected callback, which owns
the section swap and re-runs performDownloadsSearch against the
value we just wrote to the basic input.
2. Stale in-flight requests cleared loadingSources after fast retype
createSearchController._fetchSource awaits the fetch result, then
unconditionally mutates state.loadingSources / state.sources in
the settle and catch blocks. When a user typed "abc" → fetch
started → typed "abcd" before the first fetch returned, the
second submitQuery aborted the first fetch and started its own.
The first fetch's catch (AbortError) then ran and cleared
loadingSources for that source — wiping the spinner the new
request had just set, and causing a brief flash of empty/error
state while the new fetch was still in flight.
Fix: monotonic _requestSeq token. Each _fetchSource call captures
the next value (++_requestSeq). Settle / catch blocks (and the
YouTube NDJSON streaming loop) bail before mutating shared state
if requestId !== _requestSeq. Existing abortCtrl behavior unchanged
— this is a layered defense for the catch-clobber pattern that
abort alone can't prevent.
The navigate-back fix from the previous commit was being immediately
undone by the document outside-click handler. Race:
1. Click on sidebar nav-button → button handler runs synchronously,
eventually calling _searchPageRestoreOnEnter → _renderFromState →
showDropdown removes `hidden` class
2. Click event bubbles up to document
3. Document outside-click handler sees dropdown is now visible, sees
the click target is a nav-button (not inside the search wrapper or
the source row), calls hideDropdown → instantly hidden again
Fix: defer the _renderFromState call to setTimeout(0). The macrotask
runs AFTER the click event finishes propagating, so by the time the
dropdown becomes visible, the document outside-click handler has
already short-circuited (it saw the dropdown still hidden).
User reported having to delete + retype the last character of the
query to force a re-render — which worked because the input event
listener fires submitQuery, which routes through the controller
without going through the deferred path.
Cin flagged two related UX issues during PR review:
1. The "Show Results / Hide Results" toggle next to the search bar served
no real purpose — there was nothing else on the Search page worth seeing
instead of results, so toggling visibility was always pointless overhead.
2. Navigating away from /search via a sidebar link dismissed the dropdown
(the click was caught by the outside-click handler). Coming back left
the input populated but the results hidden, requiring a Show Results
click or a fresh search. The cached state was intact in the controller
the whole time — just not rendered.
Both fixed by the same direction: dropdown visibility becomes a pure
function of query state, never user-toggleable. The closure now exposes
`_searchPageRestoreOnEnter` so subsequent calls to `initializeSearchModeToggle`
re-render from the controller's cached state instead of early-returning.
Removes the button HTML, click handler, `updateToggleButtonState` function,
the desktop + responsive CSS for `.enhanced-search-btn`, and the orphaned
`.btn-icon` rule. Net -94 lines.
The hourly `clean_search_history` automation was crashing with
`'DownloadOrchestrator' object has no attribute 'base_url'`. The guard
was written before the orchestrator refactor — `soulseek_client` is now
a DownloadOrchestrator that wraps individual download clients, with the
real Soulseek client sitting at `.soulseek`.
Two other call sites in web_server.py (lines 2634, 3092) already used
the correct `soulseek_client.soulseek.base_url` pattern with a getattr
guard. This call site was missed during the refactor.
Fix: reach through the orchestrator the same way the other sites do.
Both the Search page and the global search widget ran the same source-
picker state machine (query, activeSource, per-query cache, fallbacks,
loading set, configured-source discovery, NDJSON streaming for YouTube,
default-source fall-forward). That was ~380 lines of near-duplicated
logic split across search.js and downloads.js, which meant every bug fix
or behavior tweak had to land twice and inevitably drifted.
createSearchController in shared-helpers.js now owns all of that. Each
surface passes per-surface wiring — a source-row DOM element, a CSS
class prefix, and callbacks for Soulseek handoff + unconfigured-source
redirect — and consumes the controller's state via an onStateChange
callback. The surface files shrink to their actual responsibilities:
results rendering, click handlers, and surface-specific visibility.
Zero UX change. Every keystroke, icon click, cache hit, rate-limit
fallback, and unconfigured-source redirect behaves identically to before
— verified via full pytest suite (395 passed) and node --check on all
three files.
WHATS_NEW entry added under the 2.40 unified-search bucket.
The new components shipped this PR (source icon row, fallback banner,
glow aura, library-empty search CTA) had no responsive styling. On
phones the rows ran fine via horizontal scroll but the chips wasted a
lot of space per icon, the CTA could overflow on narrow screens, and
the aura kept its desktop-sized ellipse for no benefit.
At ≤768px (tablet/phone):
- Enhanced source row: tighter padding, 24x24 glyphs, 72px chip
min-width.
- Global widget source row: even tighter, 20x20 glyphs, 62px chips.
- Fallback banners scale down to match.
- Aura shrinks to a 440x160 / 540x200 ellipse and a 180px-tall strip
so it doesn't eat short mobile viewports.
- Library empty CTA allows text wrap + reduced padding so the
"Search online for "long artist name"" string doesn't break the
layout on narrow screens.
At ≤480px (phone):
- Enhanced chips drop to 44px min-width.
- Global widget chips drop to 40px.
- Both hide the source-name label, showing icon + tooltip only — the
full 8-source row now fits or scrolls minimally at that scale.
- Aura narrows further to 140px tall.
- Library CTA nudges down to 12px font.
Adds a subtle radial glow at the bottom of the viewport that emanates
from the floating search bar, fades outward toward both window corners,
and shrinks vertically as it moves away from the bar. Makes the bar
easier to spot at a glance without a heavy full-width bar or a chrome
strip.
- New `.gsearch-aura` fixed element, 260px tall, full width, pointer
events off. Radial-gradient with the accent color centered at the
bottom middle; colour stops taper 620x230px by default, ramping to
820x280px and brighter when the bar is focused/active.
- `_gsUpdateVisibility` hides the aura on /search alongside the bar
via a simple `.hidden` class.
- Focus handler adds `.active` to the aura in step with the bar;
`_gsDeactivate` removes it. z-index 99990 (below the bar at 99998,
above most page content).
When a user types an artist name into the library search and gets no
hits, the old empty state just said "No artists found — try adjusting
your search or filters." Dead end for the common case of "I searched
for someone I don't own yet."
The empty state now detects when libraryPageState.currentSearch is
non-empty and swaps in a CTA that hands the query off to /search:
"kendrick" isn't in your library
They might be available on a connected metadata source.
[🔍 Search online for "kendrick" →]
Clicking the button navigates to /search, pre-fills the enhanced search
input, and dispatches an input event so the existing debounced search
fires automatically. Uses the same hand-off pattern _gsNavigateToSearchPage
already uses for Soulseek, so the Search page's source-picker flow
picks up naturally from there.
No change to the generic empty state (no query active) or to any other
library page behaviour.
The picker used to render every source whether or not the user had
credentials for it. Clicking Discogs with no token, Hydrabase with no
URL, or Spotify with nothing saved would fire a doomed fetch — at best
a silent empty state, at worst a confusing fallback to another source.
Now the picker reads /api/settings/config-status (the same endpoint the
Settings → Connections page already uses for the green/yellow status
dot) on init and dims icons whose service isn't set up. Clicking a
dimmed icon navigates to Settings → Connections and scrolls to the
relevant service card with a brief accent-coloured pulse to orient
the user.
Sources the backend's SERVICE_CONFIG_REGISTRY doesn't cover
(musicbrainz, youtube_videos, soulseek) are permanently treated as
configured — they need no user credentials, so dimming them would
mislead.
Extra guard: if the user's configured primary metadata source is
itself unconfigured (Spotify saved as primary but no client_id yet),
`_initDefaultSource` falls forward to the first configured source so
the default active icon is never a "set up" chip.
Shared helpers:
- fetchSourceConfiguredMap() centralizes the config-status lookup for
both surfaces. Falls back permissively if the endpoint fails so the
picker never stops working over a network hiccup.
- openSettingsForSource(src) navigates to Settings → Connections and
scrolls to `[data-service=src]`, pulsing a 2.2s accent flash
(.stg-service-flash) so the user doesn't lose their place.
CSS:
- .unconfigured: 42% opacity, 0.7 grayscale filter, subdued hover
state with no transform/glow (feels "look but don't touch"),
defensive override to kill brand glow if somehow active.
- @keyframes stg-service-flash-anim for the scroll-to highlight.
The global search popover already draws its own frosted-glass panel
(via .gsearch-results), so putting another bordered/gradient container
around the source icons inside it read as "panel inside a panel" —
visually noisy and left a dark empty strip on the right when the row
didn't fill the popover width.
Strip the source row's own background/border, center-align the chips
(justify-content: center) so they stay grouped instead of drifting to
the left, and keep a subtle bottom divider so the icons still read as
a distinct control group above the results.
