New GET /api/downloads/task/<id>/detail merges the live download task with its
library_history row (the data the Download History cards show) into one payload
the upcoming track-detail modal renders: status kind, title/artist/album,
source, quality, final location, AcoustID verdict, and expected-vs-downloaded.
Assembly + status classification live in core/downloads/track_detail.py as a
pure, importable build_track_detail()/classify_status_kind() (9 unit tests);
the endpoint is thin glue that looks up the matching history row by track.
An invalid API key (and rate limits / missing chromaprint / fingerprint
failures) all collapsed into the same None as a genuine no-match, so:
- every download showed a benign 'AcoustID: Skipped', and
- the 'Test API key' button reported a dead key as VALID
because test_api_key trusted 'no exception = valid' but fingerprint_and_lookup
swallows the error and returns None. A broken AcoustID setup looked completely
normal — which cost a real debugging session to untangle.
- New AcoustIDClient.lookup_with_status() returns a structured result that
distinguishes ok / no_match / error / no_backend / fingerprint_error /
unavailable. fingerprint_and_lookup() stays dict-or-None (library scanner /
auto-import / their tests unchanged) as a thin wrapper over it.
- verify_audio_file() uses it: a real error -> new VerificationResult.ERROR
(-> _acoustid_result='error' -> the existing red 'Error' history badge),
a genuine no-match -> SKIP 'No match in AcoustID database'. ERROR never
quarantines (an outage/bad key must not punish good files).
- test_api_key() now validates via the authoritative direct API call (error
code 4 = invalid key) instead of the swallowed-exception path.
Tests: structured-status distinction, legacy-wrapper contract, verify ERROR vs
SKIP, and test_api_key invalid-vs-accepted. Existing verify tests updated to
stub lookup_with_status (a stub returning just recordings is inferred as ok).
Clicking a quarantined track's status used to open the generic search modal,
identical to a plain failure — no way to review or recover the file. It now
opens a chooser:
- Listen: streams the file in-app via a new /api/quarantine/<id>/stream
endpoint (range-supported; the real audio Content-Type is recovered from the
sidecar since the on-disk file ends in .quarantined).
- Accept & Import: existing /approve (restore + re-import, gates bypassed).
- Search for a different result: the existing candidates modal (old behavior).
Non-quarantine failures (not_found / failed / cancelled) are unchanged — a
single click listener routes by dataset set at render time, so a task that
fails then later quarantines can't end up double-bound.
Also fixes the Accept failure on Windows: the Listen stream holds an open file
handle, so the subsequent restore move hit WinError 32 ('file in use') and the
endpoint mislabeled it 'thin sidecar'. Accept now releases the audio handle
before approving, and approve/recover moves retry briefly on transient OS locks
(_move_with_retry). Accept also auto-falls-back to Recover-to-Staging for
genuinely thin/orphaned sidecars.
Tests: stream-info resolution (sidecar + filename-fallback + missing), and
_move_with_retry success/give-up.
post_process_matched_download_with_verification pops task_id/batch_id out of
the context before running the inner pipeline (so the inner doesn't fire its
own task notifications). But _mark_task_quarantined runs inside that inner call
and reads context['task_id'] — which is now None — so it silently no-op'd.
Result: every download through this wrapper (album-bundle / staging path)
quarantined WITHOUT recording quarantine_entry_id on the task, so the UI had no
handle to manage the file (the status click just fell back to the search modal).
_mark_task_quarantined now also stashes the entry id on the context (survives
the pop), and the wrapper applies it to the real task in both quarantine
branches (integrity + AcoustID). Direct (non-wrapper) callers are unchanged.
Tests: unit coverage for the stash-with/without-task_id behavior, plus a
wrapper-level test proving the entry id reaches the task on integrity quarantine.
HiFi assembles FLAC from HLS segments and demuxes with `ffmpeg -c copy`,
which preserves total_samples=0 in the STREAMINFO of Tidal's fragmented/
streamed FLAC. Every audio frame is present and the file plays fine, but
mutagen computes length = total_samples / sample_rate = 0, so the integrity
check rejected it as 'zero-length audio' and quarantined nearly every HiFi
download. Users confirmed the quarantined files play normally once restored.
Length 0 is not proof of corruption at that point: the file already passed
the size gate, was identified as a real audio format, and has a valid info
block — a genuinely empty/truncated/stub file fails one of those earlier
checks instead. Treat length 0 as 'length unknown': accept the file and skip
the duration cross-check we can't perform without a length. mutagen never
decoded/validated frame data anyway, so this doesn't weaken real corruption
detection — size, parse, format, info-block, and duration-drift guards all
remain.
Tests: a large valid-parse length-0 file (streamed-FLAC signature) is now
accepted; a tiny length-0 stub still fails (size gate fires first).
A Soulseek album bundle stages whichever single folder scored best. If that
folder doesn't contain every track the album needs, the missing tracks were
marked not_found with no fallback — even in hybrid mode where later sources
(Deezer, YouTube, etc.) could fill them. The staging-miss short-circuit fired
for Soulseek because 'soulseek' was lumped into the torrent/usenet source set
when album bundles were added, and album_bundle_partial only reflects whether
the files found IN the folder downloaded, not whether the folder had every
needed track.
Drop 'soulseek' from the short-circuit (keep torrent/usenet). A track not
claimed from the staged Soulseek folder now falls through to the normal
per-track Soulseek search and, in hybrid mode, onward down the configured
chain. Unlike torrent/usenet — where per-track search re-adds the same
release — Soulseek per-track search is a genuine per-file network search, so
this is correct and cheap. Realizes the original author's stated intent
('keep partial bundles from blocking per-track fallback') robustly, since the
partial flag couldn't detect a folder that was simply missing tracks.
Only affects tracks NOT claimed from staging — fully-staged albums claim every
track via try_staging_match and never reach this gate, so working albums are
unchanged. Likely also mitigates #755 (all-album-import failures now fall
through to per-track instead of dying).
Tests: rewrote the two Soulseek staged-miss tests to assert fall-through
(single + hybrid-first); kept the torrent guard; added a usenet guard test.
The user-facing Search-for-Match / Fix popup runs non-strict MB searches.
That path built a bare "track artist" query with no field scoping, so the
artist was just a free fuzzy term — covers and karaoke whose TITLES contained
the artist name outranked the canonical recording. Reproduced live: searching
"Say You Will" / Foreigner returned cover artists with Foreigner absent, and
"Sweet Child O Mine" / Guns N Roses returned only covers (Presnyakov, PMJ…),
never the Guns N' Roses original.
Keep the track/album side loose (no phrase quotes → diacritic + bracketed-
suffix recall, the reason non-strict exists) but field-scope the artist as
artist:(...) so it constrains. The artist value is Lucene-escaped via
_escape_lucene() — without it, names like "Sunn O)))" or "Anthony Green
(Saosin)" would close the artist:( group early (returning unrelated artists)
or break the query (zero results). Same fix applied to search_release.
Verified against the live MB API: both reporter queries now return the real
artist top-to-bottom; diacritic recall is preserved (artist:(Bjork) folds to
Björk); and paren/?/!-laden artist names produce valid, balanced queries.
Tests pin the constructed query string (no network): non-strict scopes and
escapes the artist while keeping the track loose/unquoted; strict path
unchanged; plus _escape_lucene unit coverage.
The save endpoint coerced library_track_id with int(), which rejected
every non-numeric id with "Invalid library track id". Library ids are
str(ratingKey) — numeric for Plex but GUIDs/hashes for Navidrome,
Jellyfin, and other Subsonic servers — and are stored in the TEXT
tracks.id column, so the coercion broke manual matching on every
non-Plex server.
Replace the int() coercion with a normalize_library_track_id() helper
that trims and rejects only empty input, passing the opaque string id
straight through. Plex numeric ids are unaffected (SQLite INTEGER
affinity still stores a clean numeric string as an int, so existing
matches are byte-identical) and no schema migration is needed (the
INTEGER column already stores non-numeric ids as text).
Tests: pure-helper cases (numeric/GUID/whitespace/empty) plus a real-DB
round-trip proving a GUID id saves, reads back unchanged, and enriches.
Investigation (not assumption): the cache's TTL eviction + junk cleanup ARE
correct and DO run automatically every 6h (CacheEvictorJob, auto_fix=True).
The real gap is there's NO SIZE CEILING — TTL-only eviction means 'how big can
it get' = 'however much you fetch within the 30-day window', so heavy
discovery/enrichment legitimately grew metadata_cache_entities to ~1.8M rows /
7.6 GB, bloating the main DB (a factor in the corruption incident).
Fix — add a bounded LRU cap:
- entities_to_evict_for_capacity(total, max_rows): pure decision fn (cap<=0
disables), unit-testable like core.db_integrity.prune_backups.
- MetadataCache.evict_over_capacity(): deletes the least-recently-ACCESSED rows
(uses the already-stored last_accessed_at; NULL = never-touched = evicted
first) down to the ceiling. Default 250k rows, tunable.
- Wired as Phase 5 of CacheEvictorJob — runs LAST, after TTL/junk/orphan/null
cleanup, so it only trims a still-oversized HEALTHY cache.
Verified safe to bound/wipe: audited every cache reader (get_entity/
get_entities_batch/get_search_results/get_entity_detail/browse) — all degrade
to None/[]/empty on miss, treated as 'go fetch'. Nothing depends on a row
existing, so eviction can't break callers.
Tests: tests/metadata/test_cache_capacity_eviction.py (8) — pure-fn coverage +
real temp-DB proof that it drops the LRU rows specifically (not arbitrary) and
NULL-access rows go first. 18 adjacent cache tests still green; ruff clean.
Follow-ups (separate phases, scoped): (2) move the cache to its own bounded
metadata_cache.sqlite3 (no JOINs to library tables — confirmed clean to split;
invalidate-and-rebuild rather than migrate the 7.6GB), (3) kill the
raw_json + 22-extracted-column double storage.
Post-incident hardening. A WAL-mode DB corrupted (most likely an interrupted
write during a hard restart), and the backup routine made it unrecoverable:
it (a) never checked integrity, so src.backup() faithfully copied the corrupt
pages into every rolling backup, and (b) pruned oldest-by-mtime, so each new
corrupt backup evicted the last good one. Result: all snapshots poisoned.
New core/db_integrity.py (pure, unit-tested):
- quick_check()/is_healthy(): fast read-only PRAGMA quick_check probe.
- safe_backup(): verifies the SOURCE is healthy BEFORE the Online-Backup copy
and the RESULT after; refuses + discards rather than save a corrupt copy.
- prune_backups(): rotation that NEVER deletes the most-recent verified-healthy
backup, even to honor max_keep — so a run of bad backups can't drop your last
good snapshot.
Wired into BOTH backup paths (the /api/database/backup endpoint and the
auto_backup_database automation handler) — they now refuse on integrity failure
(409 / error status, existing backups untouched) and prune safely.
Tests: tests/test_db_integrity.py (8) using REAL temp DBs incl. a physically
corrupted one — proves refuse-corrupt-source, discard-corrupt-result, and the
exact incident scenario (newest backups corrupt -> the older healthy one is
protected from pruning). Existing maintenance-handler backup test still green
(29 passed). compile + ruff clean.
NOTE: this prevents silent backup poisoning; it does NOT stop the underlying
corruption. Follow-ups still worth doing: WAL-checkpoint on clean shutdown +
a periodic live-DB integrity alert (so corruption is caught on day 1).
Compared my #730 fix against contributor PR #731 (same independent design).
Grafted their good idea — a confidence bonus when the album's full core phrase
appears intact in the release title (rescues long multi-word names whose token
coverage gets diluted) — and kept my accent-folding, which #731 lacks (their
normalize drops accented chars: Bjork -> 'bj rk').
IMPORTANT: implemented the phrase bonus WORD-BOUNDARY anchored, not as a raw
substring. My first cut used 'phrase in norm_title' (matching #731) and it
immediately reintroduced the substring bug #730 exists to fix — 'heroes'
matched 'superheroes' and the wrong album scored 0.9/passed. PR #731 has this
latent flaw. The regex anchors the phrase to word boundaries so the bonus
fires for real matches only.
Verified: substring trap (Superheroes/Scary Monsters) rejected; edition
suffixes + intact-phrase albums kept. +1 phrase-bonus test (incl. the
word-boundary guard). 126 plugin tests pass; ruff clean.
Co-authored-by: Tyler Richardson-LaPlume <170156756+IamGroot60@users.noreply.github.com>
Review caught that artist_name was added to pick_best_album_release's signature
and threaded through both call sites but never actually used — dead, misleading
code. Removed it from the helper + both callers. Artist-aware gating would be a
deliberate future feature (titles carry the artist inconsistently, so a hard
artist gate would risk the same false-negative class I just fixed); the album
relevance gate already resolves the reported wrong-release bug. No behavior
change. 127 plugin tests pass; compile + ruff clean.
Self-review found a false-negative in the title-relevance gate I just added:
it scored 'fraction of the ALBUM-NAME's words present in the title', so a
stored album name with an edition/remaster suffix the torrent lacks
('Currents (Deluxe)', 'Heroes (2017 Remaster)') scored BELOW the 0.6 floor and
the correct release was wrongly refused -> fell back to per-track. The very
first issue example ('Heroes 2017 Remaster') would have regressed.
Fix: strip edition/format/year NOISE words (deluxe, remaster, edition, flac,
years, bitrates, ...) before scoring, via _significant_words(), with a fallback
to the raw words so an album literally named '1989' or 'Deluxe' isn't emptied
to match-everything. Verified both directions: edition suffixes now KEPT, while
the wrong-album rejection (Scary Monsters for a Heroes request, Superheroes)
still scores 0.
Tests: +2 regression tests (edition-suffix kept; noise/number-only album name).
125 album-bundle/dispatch/plugin tests pass; compile + ruff clean.
Reporter (IamGroot60): requesting an album via a Prowlarr-backed source
(Usenet/Torrent) could download a DIFFERENT album — e.g. asking for Bowie's
'Heroes' downloaded 'Scary Monsters' because the picker ranked purely by
seeders/grabs -> quality -> size with NO title check, and the wrong album had
~16x the grabs. (Confirmed the old picker chose the wrong release on exactly
this scenario.)
Fix (the reporter's proposal):
- album_title_relevance(candidate_title, album_name): word-coverage match,
accent-folded (Bjork != bj rk) and WORD-BOUNDARY (Heroes != Superheroes), so
a wrong album that shares no title words scores 0.
- pick_best_album_release gains album_name/artist_name params and a relevance
gate (floor 0.6) applied BEFORE the seeders/quality/size ranking. When
album_name is given and NOTHING clears the floor, returns None.
- torrent.py + usenet.py call sites pass album_name/artist_name and set
result['fallback'] = True on None, so the dispatcher (source-agnostic
fallback routing) hands off to the per-track flow instead of grabbing a
wrong album. Matches what Soulseek already did via its preflight scorer.
No album_name -> no gating (old behavior preserved for callers without a
title). Tests: 9 new in test_album_bundle.py (relevance math incl. the
substring trap + accent fold, the exact Bowie refuse-and-fallback scenario,
None-when-no-match, and no-gate-without-name). 125 album-bundle/dispatch/plugin
tests pass; compile + ruff clean.
Reporter: album covers render as a top strip then solid grey ('break' on
import) — and it happens regardless of the album-art toggles. That ruled out
the import embed/cover.jpg paths (all toggle-gated) and pointed at the DISPLAY
cache, which every cover view goes through.
Root cause: ImageCache._fetch_and_store streamed the body to a tmp file and
committed it as status='ok' with only a 'total <= 0' (empty) guard. A
dropped/short connection makes requests' iter_content END EARLY WITHOUT
raising, so a PARTIAL image was cached permanently and served forever as a
half-decoded cover. The high-res art change in 2.6.4 (bigger images) makes a
mid-stream cutoff more likely, especially on the reporter's LXC.
Fix: capture the declared Content-Length and, after streaming, reject when
fewer bytes arrived (unlink the tmp file, raise ImageCacheError) so nothing
broken is cached and the next request retries fresh. When the server omits
Content-Length (chunked), we can't detect truncation, so we don't reject —
behavior unchanged there.
Tests (tests/test_image_cache.py): truncated download raises + caches nothing +
a later good fetch still works (differential-verified it's silently cached
without the guard); positive control (declared==actual) caches normally;
no-Content-Length still caches. 6 image-cache tests pass.
Strong-candidate fix: it's a real defect that produces exactly this symptom,
but I can't reproduce the reporter's LXC network to prove it's THE cause.
listening_history was populated ONLY from the media server; the web player
recorded nothing. Now a play heard ~10s logs to listening_history AND bumps
tracks.play_count/last_played — so the existing 'recently played' query reflects
actual SoulSync listening, and the Phase-2 smart-radio recency signal gets real
data.
- core/playback/play_log.build_play_event(): pure, DB-agnostic normalizer from
player payload -> listening_history event shape. Caller supplies the
timestamp (stays pure). Composite/streamed ids never become the int
db_track_id; bool ids rejected; missing title -> skip. 9 unit tests.
- MusicDatabase.record_web_player_play(): inserts the history row + increments
play_count/last_played for the library track in one call.
- /api/library/log-play: thin endpoint, server-side timestamp, best-effort
(logging failure never 500s / never affects playback).
- Frontend: npMaybeLogPlay on timeupdate fires once per track at the 10s
threshold (flag reset in setTrackInfo, set-before-fetch so it can't
double-fire), fully fire-and-forget.
Pure builder is unit-tested; the DB write can't run in-sandbox (real DB throws)
so it's a thin straightforward insert+update. JS + web_server parse clean.
Foundation for multi-listener playback. Today web_server.py keeps ONE global
stream_state dict + one lock (web_server.py:747), so the whole server shares a
single 'currently playing' — every tab/device is a remote for the same
playback and two listeners collide. That global is woven through ~22 sites and
isn't unit-testable where it lives.
Lifted into core/streaming/state.py WITHOUT changing behavior:
- StreamSession: one playback's state, dict-compatible (s['k'], s.get,
s.update, 'k' in s) so existing call sites work unchanged, each with its
OWN RLock so distinct sessions never block/clobber each other.
- StreamStateStore: registry of named sessions; lazy + race-safe create;
DEFAULT session reproduces today's exact single-global behavior. Also
drop()/active_ids()/session_ids() for the eventual per-listener wiring.
web_server.py now binds (DEFAULT) and
. Drop-in: every .update()/[k]/.get()/ site behaves identically. _set_stream_state routes a reassign
through session.replace() so the store's session stays the live object (it's
effectively dead — prepare.py only mutates in place — but safe now).
Honest scope: this is the PROVABLE half of Phase 3. The remaining half (3b:
derive a per-browser session id, per-session Stream/ staging, executor
concurrency, disconnect cleanup) is browser-coupled and can't be verified
without driving 2+ live clients — deferred to a live session. The store API is
already shaped for it.
Tests (tests/streaming/, 33 total):
- test_stream_state_store.py (19): session dict-compat, isolation, lazy
create, drop rules, active_ids, concurrent-create race safety.
- test_stream_state_callsite_compat.py (7): every real web_server access
pattern (library/play, stream/start, status, audio guard, stop, prepare
in-place mutation, set->replace) against the exact object web_server binds.
- test_prepare.py +1: real prepare worker drives an actual StreamSession.
76 streaming+radio tests green; ruff clean; web_server.py parses.
Replaces radio's pure ORDER BY RANDOM() with weighted ranking. Each tier now
fetches a generous random POOL (4x the needed count, floored) and
core/radio/selection ranks it before the collector keeps the best:
score_candidate = play_count(log-damped, w=1.0)
+ lastfm_playcount(log-damped, w=0.5)
- recently_played penalty(w=2.0)
+ stable per-id jitter(w=1.0, hash-derived so runs vary but
tests stay reproducible)
Modest weights so popularity guides without burying lesser-played tracks, and
jitter keeps radio from being identical every run. All intelligence is in pure
functions (rank_candidates / score_candidate) so it's tunable + unit-testable
without SQL.
Defensive: the DB method probes PRAGMA table_info(tracks) and omits
play_count/lastfm_playcount from the SELECT when absent (older DBs predating
the listening-history migration) — the scorer treats missing signals as 0, so
radio degrades to jitter-only instead of crashing on 'no such column'.
Tests (tests/radio/, 43 total):
- score_candidate / rank_candidates: deterministic unit coverage (popularity
ordering, lastfm contribution, recency penalty, garbage→0, stable jitter).
These CANNOT pass against pre-Phase-2 code.
- DB end-to-end: ranking surfaces the heavily-played track first out of a
decoy pool (wiring proof — probabilistic vs old random, documented honestly);
plus a no-rank-columns DB proving the defensive degrade path.
- All Phase-0a behavioral/refactor-equivalence tests still green.
60 radio + adjacent-DB tests pass; ruff clean.
First step of the stream/player/radio revamp (see revamp_plan.md). The radio
algorithm lived inline inside database.music_database.get_radio_tracks as raw
SQL tangled with selection logic — untestable without a live DB (which also
throws in the dev sandbox). Lifted the pure DECISIONS into core/radio/selection.py:
- parse_tags / merge_tags — JSON-or-CSV tag fields → ordered deduped list
- same_artist_cap — tier-1 30%-floored-at-5 cap
- build_like_conditions — OR-of-LIKEs SQL fragment + params per tier
- RadioCollector — dedup + cap + exclude-set + NOT-IN placeholder/value tracking
The DB method keeps the cursor work and now delegates every decision to these
helpers. Faithful extraction, not a rewrite — behavior unchanged.
This is the kettui foundation move: radio is now unit-testable, so Phase 2
(smart ranking — play-count / recency / feature seeding) becomes 'evolve a
tested function' instead of 'rewrite SQL and pray'.
Tests (tests/radio/):
- test_selection.py (22): unit coverage of every extracted helper
- test_get_radio_tracks_db.py (7): drive the REAL get_radio_tracks against
in-memory sqlite — tier fallback, dedup, exclude, file_path filter.
Behavior-pinned: these 7 pass against BOTH old inline and new extracted
code (refactor-equivalence proof). 52 adjacent DB+radio tests green.
The $year template variable was a blind release_date[:4] slice. When
something upstream poisoned release_date with a non-date value — the album
NAME — that slice emitted garbage: 'Mantras (Deluxe)'[:4] -> 'Mant', so
every download landed in 'Mantras (Deluxe) (Mant) [Album]/' instead of
'(2026)' (Tacobell444's screenshot).
Add _extract_year_from_release_date(): returns the leading 4 chars only
when they're a plausible year (isdigit, 1900 < y <= 2100), else ''. Matches
the guard the codebase already uses in soulid_worker._extract_year. A
non-year resolves to '' and the template's existing empty-() cleanup drops
it, so a poisoned release_date can never write rubbish into the path again.
This is the shared post-process path builder
(core/imports/paths.build_final_path_for_track) that DOWNLOADS, reorganize,
and imports all route through, so the guard covers every surface at once.
Defensive fix only — it stops the SYMPTOM regardless of which upstream
writes the album name into release_date. Pinning that upstream needs the
reporter's metadata source + the release_date value from app.log (the
Soulseek + AcoustID + future-dated-album combo is the discriminator);
tracked separately.
Tests (tests/imports/test_import_paths.py): unit coverage for the helper
(real dates kept, names/sentinels/short values rejected) + an integration
test reproducing #745 — a poisoned release_date yields 'Mantras (Deluxe)
[Album]' not '(Mant)' — differential-verified it produces the exact
'(Mant)' folder without the fix. Positive control keeps real (2026). 395
import + reorganize tests green.
The Duplicate Cleaner moves de-duplicated files into <transfer>/deleted/.
If a user's media server scans the transfer folder (e.g. a /music root
holding both the library and the transfer dir), those quarantined files
get real track rows in SoulSync's DB. Reorganize is purely DB-driven —
it acts on each track's stored file_path — so it would dutifully move a
quarantined file back OUT of /deleted to the template location, exactly
what Tacobell444 reported.
We can't stop the rows from existing (they come from the media server,
which the app doesn't control), so the fix is bounded to Reorganize, as
the reporter asked: skip any track whose resolved path is under
<transfer>/deleted. Surfaced as a non-matched 'In deleted/quarantine
folder — skipped' in the preview; apply mirrors it (post-process never
runs, file left in place, counted as skipped).
Detection is anchored to the <transfer>/deleted PREFIX (not a bare
substring) so a real album like 'Deleted Scenes' is kept; falls back to
an exact 'deleted' path-segment match when transfer_dir is unavailable
(mirrors the cleaner's own 'if deleted in dirs' skip). The one
unavoidable ambiguity — an artist folder named exactly 'deleted' at the
transfer root — is pinned in a test as intentional.
Guard added once where both consumers see it: preview_album_reorganize
and the apply worker (_RunContext gains transfer_dir).
Tests: tests/test_reorganize_deleted_quarantine.py (8 unit) +
test_library_reorganize_orchestrator.py (preview + apply integration,
differential-verified they fail without the fix). 128 adjacent
reorganize tests still green.
When the preflight-selected Soulseek folder produces zero usable files —
every transfer failed/aborted/stalled (the Slipknot dead-peer case: all
tracks 'Completed, Aborted' at 0 bytes) — _poll_album_bundle_downloads
returns []. download_album_to_staging used to return that with
fallback=False, so try_dispatch marked the whole batch failed and nothing
was retried elsewhere until the next wishlist run.
Flip that branch to fallback=True so the existing, proven per-track flow
takes over and re-searches every missing track across ALL sources/peers.
This reuses the per-track multi-source robustness instead of reimplementing
candidate-folder retry inside the bundle.
Tests: tests/test_soulseek_album_fallback.py drives the preflight-reuse path
with a stubbed poll — empty poll -> fallback=True (differential-verified it
fails without the fix), healthy poll -> fallback stays False. Downstream
routing (fallback=True -> per-track) already covered by
test_album_bundle_dispatch.py.
Live testing surfaced that slskd reports a peer-side abort as 'Completed,
Aborted' at 0 bytes (peer accepts then drops every transfer). That string
contains 'Completed', so the poll's completed-branch ran first and misread it as
'completed but file missing' — routing it into the #715 unresolved/download_path
grace (gives up after 45s with a misleading 'download_path mismatch' log)
instead of recognizing it as a failure.
Add 'Aborted' and 'Cancelled' to the failure-token check (which runs before the
completed branch), so these resolve immediately and correctly as failed. Test
added for the all-aborted folder.
Two related bugs from the Slipknot album never finishing.
1) _poll_album_bundle_downloads hung when the peer stalled. The finish check
needs every transfer terminal (completed/failed); the #715 grace only covers
'slskd says Completed but file not on disk'. A transfer stuck InProgress /
Queued, or dropped by slskd, is none of those — so it blocked both the finish
AND the grace exit, and the poll spun to the full ~6h timeout.
Add a bundle-level stall guard: track a progress marker (#terminal transfers,
total bytes across pending). If NOTHING advances for _stall_grace (180s) —
no terminal transition AND no pending byte movement — the peer has stalled;
mark the stuck transfers failed so the existing finish/all-failed checks
resolve the bundle with whatever completed (missing tracks then fall back to
the per-track matcher). Conservative: only trips when EVERYTHING is frozen,
so a slow-but-progressing or still-queued transfer is unaffected.
2) Failed batches lingered in the UI forever ('No tracks loaded'). The
auto-cleanup gate removed only complete/error/cancelled phases — 'failed'
(e.g. an album-bundle hard failure) was missing, so it never aged out. Add
'failed' to the terminal set so it's removed after 5 minutes like the others.
Tests (tests/test_soulseek_album_poll_stall.py): stalled peer → gives up with the
completed subset (not the deadline); progressing bundle not falsely stalled;
all-stalled → empty; dropped transfers stall out; clean finish unaffected.
124 download/soulseek tests pass; ruff clean.
make_wishlist_batch_row / _run_wishlist_cycle annotate params with Optional,
but the typing import only had Any/Callable/Dict. Slipped past py_compile +
tests because 'from __future__ import annotations' makes annotations strings
(never evaluated at runtime), but ruff flags it statically (F821).
Stage 2: the manual 'Download Wishlist' flow now calls the same
_run_wishlist_cycle engine the auto timer uses, so a manual scan runs the exact
same code path as an auto scan. The old bespoke manual orchestration (build
payloads + SERIAL inline dispatch) is deleted — its grouping/dispatch was a
near-duplicate of auto's that had already drifted.
Behavior changes (all intended, discussed):
- Manual now dispatches album bundles in PARALLEL (album pool) like auto, instead
of serially on one thread. A single cycle='albums' engine call covers the whole
selection (albums bundled, singles/ungroupable -> per-track residual), so no
'both cycles' pass is needed.
- The manual placeholder batch_id is reused as the engine's first sub-batch
(first_batch_id), so the modal's existing poll target stays valid.
- WishlistManualDownloadRuntime gains album_bundle_executor (wired in web_server,
falls back to the shared pool when unset).
- 'Don't start manual while auto is running' is unchanged — the existing route
guard (is_wishlist_actually_processing -> 409) already covers it; no queue added.
NOT touched: process_wishlist_automatically's behavior (proven by test_automation
staying green in Stage 1) and the per-track download mechanics.
test_manual_download.py rewritten to characterize the new behavior (engine
dispatch via the executor, parallel, placeholder reuse, album-context). Full
wishlist suite green (131); wishlist + automation = 392 passed.
Stage 1 of unifying the auto + manual wishlist flows. Extract the
group -> per-album+residual batches -> register -> dispatch logic that lived
inline in process_wishlist_automatically into a standalone _run_wishlist_cycle()
engine (built on make_wishlist_batch_row). The auto path now just calls it.
Per-flow differences are arguments (auto_initiated stamps the auto-only fields +
selects auto vs manual naming/logging; first_batch_id lets a caller reuse a
pre-created placeholder). Album batches dispatch to the dedicated album pool,
residual to the shared pool (unchanged from #740).
Auto behavior is PROVABLY unchanged: its full characterization suite
(test_automation.py) stays green (10/10), and the whole wishlist suite passes
(131). This commit does NOT touch the manual flow yet (Stage 2) and does not
change what auto does — it only moves auto's logic behind a shared entrypoint
the manual flow will call next.
The auto and manual wishlist flows each built the same ~20-field
download_batches row in separate places (auto album, auto residual, manual
placeholder, manual sub-batches) — four near-identical literals that could (and
did) drift apart, producing subtly different batch shapes between the flows.
Extract make_wishlist_batch_row() as the single source of truth: it emits the
consistent core field set, with the genuinely per-flow differences as explicit
arguments — initial phase ('queued' for auto / 'analysis' for manual), the
auto-only auto_initiated/auto_processing_timestamp/current_cycle via
extra_fields, and album-vs-residual contexts. All four sites now go through it,
so every wishlist batch has an IDENTICAL shape (this also removes the field
drift that confused the modal-hydration code).
Deliberately NOT unified — and left explicit in each caller, per the
'don't cargo-cult genuinely-different code' principle: the grouping decision
(auto groups only on the albums cycle), batch-id allocation (manual reuses the
caller's placeholder id for the first sub-batch), and dispatch (auto
parallel-submits album batches to the dedicated pool + residual to the shared
pool; manual runs them serially on one thread). Those are real behavioral
differences, not duplication.
Behavior-preserving: verified safe to normalize the row shape (grep confirmed
every reader uses .get() with defaults, no key-presence checks). The existing
auto (test_automation.py) and manual (test_manual_download.py) characterization
suites stay green = differential proof of identical behavior. Adds
test_batch_factory.py (core fields, album/residual, extra_fields, no shared
mutable state, consistent key shape). 131 wishlist tests pass.
A 2.6.3 change (c3b88e69) split the wishlist albums cycle into one batch per
album. Each album batch runs an INLINE-BLOCKING soulseek/torrent/usenet
album-bundle download (album_bundle_dispatch.try_dispatch ->
download_album_to_staging) that holds its worker thread for the whole
search+download. All of these were submitted to the shared 3-worker
missing_download_executor -- the same pool used for per-track downloads AND the
manual 'Download Wishlist' analysis.
So a large Album-Completeness 'Fix all' (-> ~819 wishlist tracks -> ~82 per-album
batches) saturated all 3 workers with blocking album downloads; the manual
wishlist analysis could never get a thread ('Library Analysis' stuck on
Pending), the other ~79 batches sat in phase='queued' forever, and auto-cleanup
(which only evicts terminal-phase batches) never cleared them -> jam until
restart. Fixing batch STATUS would not help: the threads are blocked inside the
download, not waiting on a phase flip.
Fix: add a dedicated bounded album_bundle_executor (max_workers=3) and route the
AUTO per-album bundle batches to it, keeping the shared pool free for analysis /
per-track / the manual wishlist (which always starts now). Hung/slow album
downloads can only delay other album downloads, never the user-facing path.
Additive and decoupled; the submit site falls back to the shared pool when the
album pool isn't wired (older callers / tests) so behavior is unchanged there.
The manual path is untouched (it already runs album bundles serially on a single
thread, by design).
Tests (tests/wishlist/test_automation.py): album sub-batches route to the
dedicated pool while the residual per-track batch stays on the shared pool;
fallback-to-shared-pool when no album pool is wired. Existing auto-processing
tests still green (fallback preserves prior behavior). 707 passed across
wishlist + downloads suites.
The same provider ID is stored under inconsistent column names across tables
(deezer_id vs deezer_artist_id vs album_deezer_id vs similar_artist_deezer_id;
spotify/itunes keep an entity qualifier, others don't; musicbrainz uses three
nouns), so code checks 2-5 name variants everywhere.
Add core/source_ids.py as the single source of truth for (provider, entity) ->
column, with accessors that read an ID from a dict/sqlite3.Row robustly
(canonical column first, then known aliases). NO database columns are renamed —
these are the real names today; the registry just centralizes the knowledge.
Targeted adoption (behavior-identical, verified):
- core/artist_source_lookup.SOURCE_ID_FIELD now derives from the registry
instead of duplicating the mapping (values unchanged).
- web_server.py artist-detail builds artist_source_ids via source_id_map(...)
instead of a hand-rolled per-source .get() dict.
Broader call-site adoption deferred as clearly-scoped follow-up.
Tests: tests/test_source_ids_registry.py (canonical columns, alias fallback,
canonical-preferred, sqlite3.Row, source_id_map, SOURCE_ID_FIELD unchanged).
Existing artist_source_lookup + artist_full_detail suites still green.
Reported by CubeComming: importing media keeps the track artist correct
(e.g. Billie Eilish) but changes the album-artist tag ("Albuminterpret") to
"Unknown Artist", breaking grouping in the media server.
Cause: in extract_source_metadata (core/metadata/source.py), album_artist is
seeded from the resolved track artist, then overridden by the album CONTEXT's
first artist. When the album lookup comes back unresolved, that first artist is
the literal "Unknown Artist" placeholder — which is truthy, so it clobbered the
real artist.
Fix: treat "Unknown Artist" (and empty) as a non-value — only let the album
context override the album_artist when it names a real artist. A genuine album
artist (e.g. "Various Artists") still overrides as before.
Tests: tests/metadata/test_album_artist_unknown.py — placeholder doesn't
clobber, real album artist still used, no-album-context falls back to track
artist, empty doesn't clobber. (Pre-existing test_album_mbid_cache.py failures
are an unrelated sandbox DB disk-I/O issue.)
Follow-up to the poll fix, covering the two things that blocked a
successful end-to-end album import once the poll itself stopped
freezing:
1. Staging dir permissions
The album-bundle private staging path defaults to
'storage/album_bundle_staging' -> /app/storage, but /app/storage was
never created or chowned by the image (unlike /app/Staging,
/app/Transfer, etc.), and /app is root-owned. The copy failed with
"[Errno 13] Permission denied: 'storage'" under the non-root soulsync
UID. Added /app/storage to the Dockerfile build-time mkdir+chown and
the entrypoint PUID/PGID chown, exactly like the sibling runtime dirs.
2. Client->local path resolution
Usenet/torrent clients report save paths from inside THEIR OWN
container (e.g. SAB '/data/downloads/music/<album>'); SoulSync often
mounts the same files at a different point ('/app/downloads/<album>').
Feeding the client path straight to the audio walker yields
"No audio files found" though the files are physically present.
New resolve_reported_save_path():
a. use the reported path as-is if readable (mirrored mounts),
b. apply explicit download_source.usenet_path_mappings
({from,to}, Sonarr/Radarr-style) for non-shared layouts,
c. basename fallback under SoulSync's own download roots —
zero-config for the standard shared-volume arr setup.
Wired into both call sites in usenet.py AND torrent.py
(download_album_to_staging + _finalize_download), logging any
translation and including the resolved path in the no-audio error.
Tests: resolver verbatim / explicit-mapping / basename-fallback /
priority / not-found / empty / mapping-miss-then-basename. ruff +
compileall + pytest green (645 in the download suites).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The earlier #721 fix tolerated a ~10s "completed but no save_path"
window, but the real production stall sits upstream of that: SABnzbd
removes a finished download from the queue and runs par2 verify /
repair / unpack *in History*, exposing the live stage in the slot
`status` ('Verifying' / 'Repairing' / 'Extracting' / 'Moving' / ...)
with `storage` empty until the final move. `_parse_history_slot` mapped
EVERY non-'Failed' status to 'completed', so a still-extracting 1.7 GB
FLAC album looked "completed with no save_path" the instant download hit
100%. The poll burned its completed-no-path budget mid-PP and bailed,
freezing the UI on the last download emit (the stuck-at-99%/100%
signature). SAB then finished fine — which is why the job shows
Completed in History but SoulSync never staged it.
Root fix
- `_parse_history_slot` routes `status` through `_map_state`, so PP
stages stay NON-terminal: the poll keeps waiting (as 'downloading')
for as long as post-processing takes and only a real 'Completed'
flips to terminal success. `save_path` is trusted only on true
completion (mid-PP path fields may point at the incomplete dir).
Supporting / defensive
- `UsenetStatus.incomplete_path`: surfaced separately from save_path
(SAB `incomplete_path`) and used by the poll loops as a LAST RESORT
after the completed-no-path window, to recover the case where
`storage` never lands but the files are physically on disk.
- `poll_album_download`: dedicated, configurable completed-no-path
window (~120s via `download_source.album_bundle_completed_no_path_seconds`)
decoupled from the ~10s transient-miss window; incomplete_path
fallback; a 30s heartbeat log so the previously-silent poll loop is
diagnosable.
- `usenet.py` `_download_thread`: per-track parity — it was erroring
immediately on the first completed-no-path read.
- `album_bundle_dispatch.py` / `status.py` / `monitor.py`: use the
project `get_logger` so download-flow logs land in app.log under the
`soulsync.*` namespace (they were console-only before, which hid the
`[Album Bundle] flow failed` line during triage).
Tests
- PP-history state mapping; end-to-end Hunky Dory PP regression
(download -> Verifying/Extracting in History past both budgets ->
Completed+storage -> success); completed-no-path window +
incomplete_path fallback; per-track thread parity. ruff + compileall +
pytest all green (the only local failures are environmental: missing
tzdata + local tools/ffmpeg.exe, neither present on CI).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Final cluster: the four structurally-identical snapshot endpoints
(discover_downloads, artist_bubbles, search_bubbles, beatport_bubbles) ->
core.discovery.endpoints.save_bubble_snapshot(...), wired via
_save_source_bubble_snapshot. All four validate a payload key, persist via
db.save_bubble_snapshot(kind, items, profile_id=...), and return a count +
timestamp; they differ only by:
- payload_key ('downloads' for discover, 'bubbles' for the rest) + its
no_data_error message.
- snapshot_kind, success_noun, and the info/except log subject + noun
("downloads"/"artists"/"albums/tracks"/"charts").
get_database / get_current_profile_id injected; get_json (request.json) invoked
inside the try, preserving the original 400/500 behavior incl. traceback dump.
Tests: +5 (missing key 400, None body 400, happy path with kind/profile/count/
timestamp, discover_downloads variant, exception -> 500). Full discovery suite:
210 passed.
web_server.py: -98 lines.
Ninth cluster: update_<source>_playlist_phase for the five sources sharing the
identical validation + full-message response (Tidal, Deezer, Qobuz,
Spotify-Public, YouTube) -> core.discovery.endpoints.update_playlist_phase(...),
wired via _update_source_playlist_phase + the _PHASE_LIST/_PHASE_LIST_YT
constants.
Per-source params:
- valid_phases — YouTube additionally allows 'parsed'.
- apply_extra_fields — Deezer/Qobuz/Spotify-Public also persist
download_process_id / converted_spotify_playlist_id from the body; Tidal and
YouTube do NOT, so they pass False (kept strictly 1:1 — the generic won't
apply those keys for them even if a caller sent them).
- not_found_message / error_label; get_json invoked inside the try.
NOT folded in: iTunes-Link — uses data.get('phase') (no "Phase not provided"
400) and returns a no-message payload.
Tests: +7 (404, missing-phase 400, invalid 400, happy path with extra-fields
suppressed, extra-fields applied when enabled, YouTube 'parsed' allowed,
exception -> 500). Full discovery suite: 205 passed.
web_server.py: -123 lines.
Eighth cluster, the heavyweights (~110 lines each). The fix-modal
update_<source>_discovery_match for the four sources with the identical
structure (Tidal, Deezer, Qobuz, Spotify-Public) ->
core.discovery.endpoints.update_discovery_match(...), wired via
_update_source_discovery_match. Applies the user-selected Spotify track to the
discovery result (status/artist/album/duration/spotify_data/match-count) and
writes the manual fix to the discovery cache.
Per-source pieces are params:
- source_log_label / error_label.
- original_track_key ('tidal_track' / 'deezer_track' / ...).
- original_artist_getter: Tidal handles string-or-object artists
(first_artist_str_or_obj); the rest assume strings (first_artist_plain).
- web_server helpers (join/extract artist, build_fix_modal_spotify_data,
cache-key, get_database, active-discovery-source) injected.
- get_json passed as a callable and invoked INSIDE the try, preserving the
original's "request.get_json() inside try" behavior (malformed body -> 500).
NOT folded in (genuinely divergent): iTunes-Link (saves spotify_data directly
via a different cache signature), YouTube (multi-key original_track fallback),
ListenBrainz (entirely different unmatch-capable structure, no cache write),
Beatport.
Tests: +9 (extractors; 400/404/400 guards; full happy path with result
mutation + duration formatting + match-count + cache-save args; no-increment
when already found; cache error swallowed; get_json raise -> 500). Full
discovery suite: 198 passed.
web_server.py: -400 lines.
Seventh cluster: start_<source>_sync for the five sources with the identical
flow (Tidal, Deezer, Qobuz, Spotify-Public, YouTube) ->
core.discovery.endpoints.start_sync(...), wired via _start_source_sync.
Validates phase, converts discovery results, seeds sync state, posts a
"... Sync Started" activity item, and submits to the sync executor. Per-source
pieces are params:
- sync_id_prefix (f"{prefix}_{key}"), not_found/not_ready messages, convert_fn.
- name/image accessors: Tidal reads an object (playlist_name_obj/
playlist_image_obj), the rest a dict (playlist_name_strict/playlist_image_dict).
- activity_label vs error_label DIFFER for Spotify-Public ("Spotify Link
Sync Started" activity, "Spotify Public" logs).
- submit_sync_task glue (_submit_sync_task) closes over sync_executor /
_run_sync_task / get_current_profile_id so the helper stays global-free.
NOT folded in: iTunes-Link (no final info log), ListenBrainz (submits the
task WITHOUT a playlist_image_url arg), Beatport (extra debug logging, chart).
Tests: +6 (404, not-ready 400, no-matches 400, full happy path with
state/sync-infra/submit/activity assertions, resync phases allowed,
exception -> 500). Full discovery suite: 189 passed.
web_server.py: -172 lines.
Sixth cluster: the bulk-hydration get_<source>_playlist_states endpoints for
the five sources that build the identical per-entry dict + {"states": [...]}
shape (Tidal, Deezer, Qobuz, Spotify-Public, iTunes-Link) ->
core.discovery.endpoints.get_playlist_states(states, *, error_label,
info_log_label=None), wired via _get_source_playlist_states.
iTunes-Link is the only one of the five without the "Returning N stored ..."
info log, so info_log_label is optional (iTunes passes None to suppress it).
NOT folded in: the YouTube/ListenBrainz get_all_*_playlists endpoints. They
return {"playlists": [...]} (different key) with a different field set
(url / created_at / playlist, no discovery_results) and filter out
mirrored_/profile-scoped entries — genuinely divergent, kept as-is.
Tests: +4 (list build + last_accessed bump + exact shape, empty, optional ids
default None, missing-required-field -> 500). Full discovery suite: 183 passed.
web_server.py: -116 lines.
Fifth cluster: reset_<source>_playlist for the four sources with byte-
identical bodies (Tidal, Deezer, Qobuz, Spotify-Public) ->
core.discovery.endpoints.reset_playlist(states, key, *, label,
not_found_message), wired via _reset_source_playlist. Resets phase/status to
'fresh', clears discovery/sync fields, cancels any discovery_future, and
preserves the original playlist payload.
Left with their own bodies (genuinely divergent):
- YouTube: status -> 'parsed' (not 'fresh'), no download_process_id, logs the
playlist name, "reset to fresh state".
- ListenBrainz: status -> 'cached', logs playlist title, returns
{"success": True, "phase": "fresh"} (different payload), _lb_state_key.
- iTunes-Link: state.update(...), no info log, "iTunes Link reset to fresh
phase".
Tests: +4 (404, full clear + playlist preserved + future cancelled, no-future
path, exception -> 500). Full discovery suite: 179 passed.
web_server.py: -100 lines.
Fourth cluster: get_<source>_discovery_status (all eight sources, Beatport
included) -> core.discovery.endpoints.get_discovery_status(states, key, *,
not_found_message, error_label), wired via _get_source_discovery_status.
Unlike sync-status, the discovery-status response shape is byte-identical
across every source (phase/status/progress/spotify_matches/spotify_total/
results/complete), so Beatport folds in here too. Only the 404 string
("... discovery not found" vs "... playlist not found" vs "Beatport chart
not found") and the except-log label vary. ListenBrainz key via _lb_state_key.
NOT touched this cluster: get_*_playlist_state (the sibling endpoints).
Those genuinely diverge per source — different id-key name (playlist_id /
url_hash / playlist_mbid), presence of url / created_at / download_process_id,
Tidal's playlist.__dict__ serialization, and YouTube's strict (non-.get)
field access. Folding them would need a flag pile that wouldn't be a clean
1:1, so they keep their own bodies.
Tests: +4 (404, full response + last_accessed bump, complete=False when not
'discovered', missing-field -> 500). Full discovery suite: 175 passed.
web_server.py: -155 lines.
Third cluster: the get_<source>_sync_status routes (Tidal, Deezer, Qobuz,
Spotify-Public, iTunes-Link, YouTube, ListenBrainz) -> core.discovery.
endpoints.get_sync_status(...), wired via _get_source_sync_status glue.
This cluster carries the real per-source quirks, all captured 1:1 as params:
- not_found_message (iTunes-Link uses "iTunes Link not found").
- error_label vs activity_subject — these DIFFER for Spotify-Public: the
activity feed says "Spotify Link playlist ..." while the except log says
"Error getting Spotify Public sync status".
- playlist-name accessor, three styles lifted verbatim as named helpers:
playlist_name_attr_or_unknown (Tidal: object .name), playlist_name_strict
(Deezer/Qobuz/Spotify-Public/iTunes: state['playlist']['name'], can raise),
playlist_name_safe (YouTube/ListenBrainz: .get default). The strict getter
preserves the original's behavior of raising -> 500 AFTER phase/sync_progress
were already mutated.
- ListenBrainz key via _lb_state_key (caller-resolved).
Beatport stays separate (different payload: status not sync_status, sync_id,
no lock, chart key).
Tests: +9 (3 name accessors incl. raise/fallback semantics; status 404s,
running-no-mutation, finished+activity, error+revert+activity, and strict-
getter-missing -> 500 after partial mutation). Full discovery suite: 171 passed.
web_server.py: -244 lines.
First cluster of the per-source playlist-discovery deduplication. The
convert_<source>_results_to_spotify_tracks functions (Tidal, Deezer, Qobuz,
Spotify-Public, YouTube, ListenBrainz) plus the already-generic
_convert_link_results_to_spotify_tracks were byte-identical apart from the
source label used in their log line.
Lift the shared body into core/discovery/endpoints.py as
convert_results_to_spotify_tracks(results, source_label); the 7 web_server
functions become 1-line delegations (names/signatures unchanged, so all
callers and behavior are identical — 1:1).
Beatport is intentionally NOT folded in: its converter coerces artist
objects to strings and emits a different track shape (source field, album
dict), so it keeps its own implementation.
Tests: tests/discovery/test_discovery_endpoints.py (12) pin both input
shapes (manual spotify_data / auto spotify_track+found), optional
track/disc numbers, falsy-0 omission, field defaults, skip-on-neither,
order preservation, if/elif precedence, empty input.
web_server.py: -209 lines. Full discovery suite: 151 passed.
Follow-up to the album-art resolution fix. That change upgraded MusicBrainz
Cover Art Archive thumbnails (/front-250) to the bare /front original — but
/front redirects to archive.org, which is unreliable: probing release-group
covers showed intermittent HTTP 500s (same URL 500s one second, serves the
next) and multi-MB originals (2.9 MB seen). The result was the user-reported
flakiness: cover art that "sometimes works, sometimes shows nothing", and a
huge image embedded into every track when it did work.
The sized thumbnails (/front-250, -500, -1200) are served by CAA's own CDN,
not the archive.org redirect — which is why /front-250 (240p) was always
reliable. Upgrade to /front-1200 instead: 1200x1200 is a massive jump from
240p, reliably CDN-served, and a sane ~40 KB instead of multi-MB.
Applied in all three CAA spots for consistency: the _upgrade_art_url helper
(embed + cover.jpg paths) and both prefer_caa ("CCA") blocks, which fetched
the bare /front directly with no fallback — so CCA-on users hit the same
flakiness. _fetch_art_bytes still falls back to the original /front-250 if
/front-1200 is ever refused.
Tests updated to assert the 1200px target, idempotency, and that the bare
/front original is intentionally left untouched.
User report: embedded album art came out ~600x600 while the cover.jpg in
the folder was high-res. The cover.jpg path upgraded the source CDN URL
to its highest resolution, but the tag-embed path fetched the raw URL —
so iTunes art embedded at its 600x600 default, Spotify at 640, Deezer at
1000. The "Write Tags to File" retag path had the same gap (Deezer-only
upgrade), and MusicBrainz art was worse still: every Cover Art Archive
URL is built as the /front-250 thumbnail, so MB-sourced downloads
embedded 250x250.
Factor the resolution upgrade + fetch into two shared helpers in
core/metadata/artwork.py and route every art path through them:
_upgrade_art_url(url) — bump to the source's highest resolution:
- Spotify (i.scdn.co) -> original master (~2000px+)
- iTunes (mzstatic.com) -> 3000x3000
- Deezer (dzcdn) -> 1900x1900
- Cover Art Archive -> /front original (was /front-250)
_fetch_art_bytes(url) — upgrade, fetch, and fall back once to the
original size if the CDN refuses the larger one (non-regressive).
Now consistent across: embed-into-tags (post-process), folder cover.jpg
(post-process), and the enhanced-library "Write Tags to File" retag flow.
The YouTube path already upgraded via Album.from_spotify_album, unchanged.
De-duplicates the per-source upgrade code that was copied across sites
and drops the now-unused urllib import from tag_writer.
Not covered (follow-up): Last.fm / Amazon / Tidal / Qobuz have no
explicit upgrade yet — some already serve full-res, others may hand over
a capped size that passes through unchanged.
Tests: new tests/metadata/test_artwork_resolution.py pins every upgrade
(Spotify 300/640->master, iTunes 100/600->3000, Deezer->1900, CAA
thumbnail->original, unrecognized/empty unchanged) and the fetch
fallback. Updated the two tag_writer fallback tests to patch the network
at its new home in artwork.
Artist-detail discography from MusicBrainz fetched releases via the
artist lookup (`/artist/<mbid>?inc=release-groups`), which MusicBrainz
hard-caps at 25 embedded release-groups and which ignores the `limit`
param entirely. Prolific artists had ~85% of their catalogue silently
dropped — Kendrick Lamar has 167 release-groups on the site but only the
first 25 ever reached SoulSync. Reported by Sokhi: "a lot of albums are
missing when searching vs what's showing on the site."
Switch `get_artist_albums` to walk the paginated browse endpoint
(`/release-group?artist=<mbid>`, offset loop) — the same pattern the
basic-search path already uses — fetching the full catalogue up to the
caller's limit. No type filter and no studio-only filter here: the
artist-detail page wants every primary/secondary type so its tabs mirror
musicbrainz.org. Verified live: now returns all 167 for Kendrick.
Adds 7 tests covering pagination past the cap, offset advance,
short-page stop, limit cap, cross-page dedup, type->bucket mapping, and
a regression pin asserting the capped inc=release-groups lookup is no
longer the discography source.
Two things in this commit. Functional download / matched-download
behaviour is untouched — same JS handlers, same routes for the
download actions, same album-expand interaction.
VISUAL REDESIGN
- Glass search-bar card with accent radial wash + focus ring + pill
primary search button
- Source chip row above the search bar (see below)
- Always-visible compact filter pill row (Type / Format / Sort) —
pills carry both ``bs-filter-pill`` (new visual) and ``filter-btn``
(legacy class for ``resetFilters`` + ``applyFiltersAndSort`` in
wishlist-tools.js to keep working)
- Accent-tinted status pill matching the dashboard / auto-sync look
- Album result cards: glass card with accent left-edge stripe,
52px brand-tinted cover icon, chevron expand indicator, pill
action buttons (Download / Matched Album), accent glow on hover
- Track result cards: glass row with accent stripe, 44px icon,
pill action buttons (Stream / Download / Matched Download)
- Multi-disc separators inside expanded album track lists styled
with the accent treatment
- Responsive: action button columns stack vertically below 900px
New CSS lives in a self-contained ``webui/static/basic-search-v2.css``
sheet linked from index.html. Selectors are scoped to
``#basic-search-section`` for any class that already exists in
style.css (``.album-result-card``, ``.album-icon``, ``.track-*``,
etc.); the new ``bs-*`` prefixed classes for the search bar /
filters / source row / status are unscoped because they only exist
in the new markup. ``!important`` is used on the card-level rules
to defeat the original unscoped ``.album-result-card`` etc. rules
in style.css that would otherwise leak heavyweight padding /
box-shadow / 56px icon styles into the new design.
Also removed ``overflow: hidden`` from the original
``.album-result-card`` and ``.track-result-card`` rules in style.css
— those two classes only render in ``downloads.js`` basic search
results (verified via grep, two render sites only), so the
removal can't impact any other UI.
SOURCE PICKER (hybrid mode)
- New ``GET /api/search/sources`` endpoint returns the list of
active sources from the orchestrator's chain (or the single
active source in single-source mode).
- Frontend renders a chip row above the search bar. Click a chip
to target that source for the next search; the chip's brand
accent fills.
- In single-source mode the lone chip is rendered as a dashed-
border label so the user always knows what they're searching
but can't accidentally try to switch to sources that aren't
configured.
- ``/api/search`` accepts an optional ``source`` body param. When
set, ``core/search/basic.py:run_basic_search`` resolves the
client directly via ``orchestrator.client(source)`` and calls
its ``.search()`` instead of going through the hybrid chain.
- Backwards compatible: omitting ``source`` falls through to the
original ``orchestrator.search()`` call exactly as before.
Unknown source names also fall back to the default — typo
protection.
TESTS (5 new + 6 pre-existing = 11 total in test_search_basic.py)
- source param routes to specific client, NOT orchestrator chain
- no source param preserves original orchestrator-default behaviour
- unknown source name falls back to orchestrator default
- ``run_basic_soulseek_search`` backwards-compat alias preserved
- source-targeted path serialises albums + tracks correctly
101 search-suite tests pass.
Reporter @Sokhii: downloading the Mushoku Tensei Original
Soundtrack II via Apple Music metadata + Tidal download
produced duplicate library entries — same audio file landed
under multiple track positions in the album view.
Root cause (verified by direct probe + isolated repro):
``MusicMatchingEngine.normalize_string`` correctly skipped
unidecode for CJK text (kanji→pinyin would have produced
gibberish — see the inline comment at line 74-76), but then
ran ``re.sub(r'[^a-z0-9\s$]', '', text)`` which stripped EVERY
CJK character. Every Japanese title normalised to ``''``.
``similarity_score`` has an early-out guard
if not str1 or not str2: return 0.0
so EVERY CJK-vs-CJK title comparison returned 0.000.
Downstream effect: the matcher fell back to duration+artist
alone. For an OST album with 24 tracks all by the same artist
with similar durations, multiple iTunes track queries landed
on the SAME Tidal candidate. SoulSync wrote each download to
a different output filename (per the iTunes track position),
so on disk there were N copies of the same audio under
different track numbers. The user's library showed 34 entries
for an album with 24 actual tracks.
Probed iTunes album 1753240110 directly — 24 distinct tracks,
zero (disc, track_number) collisions, both US + JP storefronts.
So the duplicate origin was definitely downstream of metadata
fetch.
Fix: when CJK is detected upstream, the alphanumeric-strip step
also preserves CJK Unified Ideographs + radicals
(⺀-鿿), Hiragana + Katakana (-ヿ), Halfwidth
/ Fullwidth forms (-), and Hangul syllables
(가-). CJK titles now produce a comparable normalised
form instead of an empty string. ``similarity_score`` works as
intended:
'命の灯火' vs '命の灯火' → 1.000 (was 0.000)
'命の灯火' vs '無職転生' → 0.000 (was 0.000, but now from
actual char comparison
not from the empty-string
guard)
Latin-only normalisation is completely unchanged. ``has_cjk``
is False for Latin input, so both the CJK-lowercase branch AND
the new CJK-preserve strip branch are skipped — Latin titles
go through the original unidecode + lowercase + strip path
verbatim. Tested via 4 regression tests that pin the Latin
baseline (simple, unidecode target, $-preservation, identical
+ different similarity scores).
16 new unit tests in ``tests/test_matching_engine_cjk.py``:
- Kanji / Hiragana / Katakana / Hangul / Chinese all survive
- CJK-only strip still removes Latin punctuation in the
CJK branch
- Mixed Latin + CJK lowercases the Latin half
- Identical CJK titles → 1.0
- Disjoint CJK titles → near 0
- Partially overlapping CJK titles → midrange
- CJK doesn't falsely match unrelated Latin
- 4 Latin-baseline regression pins
- Real-world Mushoku Tensei OST scenario
371 text + imports + new CJK tests pass after the fix.
