Introduce a boot-phase guard so gunicorn worker import never blocks on Spotify, Qobuz, Deezer, or Tidal network validation. Network auth checks run only after module initialization completes.
Avoid blocking Spotify auth probes during gunicorn worker import and add a request timeout to the auth probe client so unreachable API calls cannot stall startup indefinitely.
Co-authored-by: Cursor <cursoragent@cursor.com>
For exporting a mirrored playlist back to Spotify:
- The OAuth scope string was duplicated verbatim in 5 places (spotify_client, the per-profile
registry, and 3 web_server callbacks) — a drift hazard where the authorize URL and token
exchange could request different scopes and silently re-prompt/deny. Extracted ONE
SPOTIFY_OAUTH_SCOPE constant and pointed all 5 at it, and added playlist-modify-public/private
there. Existing users re-auth once to grant write; reads are unaffected.
- SpotifyClient.create_or_update_playlist(name, track_ids, existing_id=None): creates a playlist
owned by the authed user, or replaces an existing one's tracks in place (idempotent re-export).
Chunks at Spotify's 100-track cap. A pre-scope token gets a clear "reconnect Spotify" message
instead of a raw 403. Returns {success, playlist_id, url, added, error}.
6 tests: create-new adds tracks, update replaces (no create), >100 chunking, empty → error (no API
calls), not-authed → error, insufficient-scope → reconnect message. 268 spotify/oauth tests green,
ruff clean. Additive — read paths and existing tokens unchanged.
Next: Deezer write via the ARL gw-light gateway, then the export-job branch + endpoint + modal.
The Wing It pool "Fix Match" search returned "no results" for everything (even obvious
tracks). Root cause: /api/spotify/search_tracks built a Spotify field-filtered query
(track:X artist:Y) and handed it to spotify_client.search_tracks, which falls back to the
user's configured source when official Spotify isn't serving the request. The fallback
(Deezer here) got the raw Spotify `track:…artist:…` syntax it can't parse and aborted the
connection (RemoteDisconnected) — so the user's perfectly working Deezer failed ONLY on
this path, on this query format. The iTunes and Deezer search endpoints already dropped
field syntax for exactly this reason; the Spotify one was the lone holdout.
fix:
- new pure helper relevance.build_combined_search_query(track, artist, legacy) — plain,
source-agnostic query; documents WHY field syntax is wrong here. the endpoint already
reranks by expected title/artist, so precision is recovered without the brittle syntax.
- the Spotify endpoint uses it (now consistent with iTunes/Deezer).
- frontend (searchPoolFix): surface the real error (auth / 500 / upstream abort) instead
of masking everything as "No results found" — which is what made this undiagnosable.
5 helper tests incl. the regression (output must contain no 'track:'/'artist:' syntax).
654 metadata/search tests green, 64 script-integrity green, ruff clean.
clicking Download Discography → Add all to wishlist added ~1 track every 15-30s. trace: the
endpoint's per-track library-ownership check (track_already_owned → check_track_exists) ran the
LEGACY path — firing search_tracks for every title-variation × artist-variation, per track. on
a large library and an artist you own NOTHING of, STRATEGY-1 (indexed LIKE) always missed and
fell through to the fuzzy fallback (full-table scan), ~10-15 scans/track = the 15-30s. metadata
fetch was never the bottleneck (deezer returns each album in ~1s).
fix: pre-fetch the artist's owned tracks ONCE (get_candidate_albums_for_artist →
get_candidate_tracks_for_albums) and pass candidate_tracks to check_track_exists's batched
in-memory path — the same path the discography backfill job + completion-stream already use.
pass an EMPTY list (not None) when nothing is owned so the owns-nothing case still takes the
fast path → instant. per-track cost drops from ~20s to ~1ms.
safe: track_already_owned's only real caller is this endpoint; the new param is optional
(None = unchanged legacy behaviour for any other caller). normal ownership still detected (same
artist-variation breadth); the one divergence is a track owned ONLY via a compilation → a
harmless redundant wishlist add, which is the endpoint's explicitly-accepted failure mode and
already how the backfill job behaves. 4 new tests; 1134 discog/metadata/wishlist tests green.
#929 added 'musicbrainz' to library_reorganize._ALBUM_ID_COLUMNS but not to the
canonical layer, breaking the equality invariant test (canonical reads exactly what
reorganize reads). add musicbrainz to CANONICAL_ALBUM_SOURCES, and move it from the
'can't pin' param group to the 'pins' group in the manual-lock tests — now consistent,
and forward-compatible with pinning a deliberately-matched MB edition. inert at runtime
today (mb isn't in the manual source selector, so should_pin is never called with it).
540 canonical/reorganize tests green.
A Spotify Free (no-auth) user saw 'Showing Discogs results - not from your
primary source (Deezer)' on the manual album-import search. Root cause:
get_primary_source() deliberately downgrades an unauthenticated Spotify to the
working fallback (deezer) so client routing always yields a usable client - and
the import payload reused that FUNCTIONAL value for the LABEL. The free source
has no album-name search (SpotifyFreeMetadataClient.search_albums() returns []),
so falling back for results is correct; only the label was wrong.
Fix: get_primary_source_label() preserves the user's configured intent (Spotify
Free reads as 'spotify') without touching client routing or the search chain.
The import album/track/suggestions payloads now return the label; the functional
source still drives the hydrabase-enqueue + fallback chain. Banner now reads
'not from your primary source (Spotify)'.
Tests: seam tests for get_primary_source_label + route regression pinning the
label/functional decoupling; updated 4 existing import-route tests.
Confirmed from Sokhi's FLAC tags + screenshot: disc-2/3 tracks land in the 'Disc 1'
folder, collapsing every disc's track 3/4/5/6 into one folder. Root cause: the
import pipeline syncs the resolved TRACK number into album_info (so the folder
matches the tag — pipeline.py '[FIX] Updated album_info track_number') but never
did the same for DISC. So the 'Disc N' folder (built from album_info.disc_number,
often 1) used a different disc than the embedded tag (resolved per-track in
source.py — e.g. 2/3 from a MusicBrainz multi-medium release).
Fix: one SHARED resolver, resolve_disc_for_track(original_search, album_info),
used by BOTH source.py (the tag) and the pipeline (which now writes it back into
album_info before building the path). Same function + same inputs (the pipeline
pulls the identical get_import_original_search(context)), so folder and tag can
never disagree. Returns the first valid positive disc (per-track, then album),
else 1 — a falsy/unknown per-track disc falls through to the album instead of
flooring early.
Tests: resolver preference/fallback/floor + an explicit folder==tag lockstep check
incl. Sokhi's per-track-2/album-1 case. 2122 import/pipeline/metadata tests green.
Sokhi: some tracks in a multi-disc album showed up with a null disc in Jellyfin
and floated ungrouped above the disc sections (tracks 3/9/15). Mechanism: the tag
writer only wrote the disc tag when disc_number was truthy, and enrichment CLEARS
all tags before rewriting — so a track whose disc came back 0 / None / '' lost its
disc entirely. Those falsy values slipped through because source.py defaulted with
'is not None' (a literal 0 passed) and context.py's or-chain can yield None; this
happens especially when a track resolves to a different edition than its siblings.
Fix: normalize_disc_number() floors any value to >=1, and enrichment now writes the
disc tag UNCONDITIONALLY (like the track number) so a track is never disc-less.
source.py uses the same floor so the metadata dict (and the 'Disc N' folder org)
stays consistent. Valid multi-disc values are preserved untouched.
Tests: normalize floors 0/None/''/negatives/non-numeric -> 1, preserves 1..4 and
tolerates '2.0'. 1406 enrich/metadata/track-number tests green, ruff clean.
NOTE: this fixes the SYMPTOM (never ungrouped). The deeper cause — a track matching
a DIFFERENT edition/release than its album siblings (the Persona-box-set mismatch in
the sample file; the canonical-version problem) — is separate and still open.
A MusicBrainz album resolves its art at RELEASE-GROUP scope even for a concrete
release (musicbrainz_search _release_to_album -> _cached_art prefers the rg mbid).
On the Cover Art Archive a release-group front is a single REPRESENTATIVE cover
(CAA picks one release to stand for the group, ~always the standard edition), so
a special edition like "Clair Obscur: Expedition 33 (Gustave Edition)" got the
standard art baked into cover.jpg + embedded tags at download time.
Add core/metadata/caa_art.fetch_release_preferred_art: try the specific release
own /release/<mbid>/front first, fall back to the existing release-group/provider
URL only when the release has no art of its own (404 -> None). Wire it into both
download_cover_art (cover.jpg) and embed_album_art_metadata (tags) so they stay
in sync. min_bytes defaults to 0 so the fallback keeps its prior behavior — this
can only ever ADD a better edition match, never strip a cover that showed before.
Caveat (documented, not fixed here): this only helps when the resolved release IS
the right edition. If the upstream match picked the standard release/group, that
is the separate canonical-version matching problem.
Tests: pure helper (prefers release art, falls back on 404/tiny/exception, no-mbid
passes through, nothing-available -> None). 667 metadata/artwork/deezer/hifi tests pass.
Sokhi: tracks occasionally land in Rockbox's 'untagged' bucket after a
'processing failed'. enhance_file_metadata saves the file with tags CLEARED up
front (so stale tags never linger), then runs the failure-prone external steps
(source-id embed, cover-art fetch). The core tags (album/artist/title/track from
the matched context) are written to the in-memory object BEFORE those steps, but
the on-disk file is still the cleared one until the final save.
The #764 fix made the error handler restore ART — but gated the re-save on there
being original art to restore. So a file with NO embedded art that hit a
mid-enrichment crash threw away its in-memory core tags and was left on disk as
the up-front clear saved it: untagged. Now the handler always persists the
in-memory tags (restoring art when present), so a crash leaves a correctly-tagged
file (album tag intact -> right bucket) instead of an empty one. Regression test
drives the real enhance_file_metadata against an art-less FLAC.
Status checks asked is_spotify_authenticated() (official OAuth only) instead of
is_spotify_metadata_available(), so a Spotify-Free primary read as disconnected.
get_primary_source_status had spotify_free awareness but it was dead code:
get_client_for_source('spotify') returns None unless officially authed, so the
free-availability probe never had a client. Fetch the client directly for that
check; add the missing free branch to the dashboard test message. Seam + regression tests.
Review/regression fix: the shared-app gate I added wrongly required a token
cache even for profiles that set their OWN app creds (legacy), breaking the
per-profile caching tests. Now a per-profile client is built when the profile
has its own app creds OR has connected (its own token cache) — otherwise it
falls back to the global/admin client. Made the fallback-safety tests
order-independent (monkeypatch get_spotify_client to a sentinel) since the real
global client isn't a stable singleton across the suite.
10 metadata-cache + resolution tests pass together.
First service of the per-profile playlist-auth feature. Each profile connects
its OWN Spotify account through the shared (admin's) app, getting its own token;
used for that profile's playlist reads. Admin + unconnected profiles + all
background workers keep using the global/admin client — fully non-regressive.
- Shared-app OAuth: get_spotify_client_for_profile + the /auth/spotify init &
callback now use the GLOBAL app creds (falling back from any legacy per-profile
app creds) with the profile's own token cache, and show_dialog=true forces the
account chooser so a user can't silently inherit the admin's Spotify session.
The builder gates on the profile's own token cache existing — no cache → global.
- My Accounts modal (new, all-profile-accessible via the profile bar): one-click
Connect/Disconnect Spotify + connection status (account name). GET
/api/profiles/me/connections + POST .../spotify/disconnect; admin's Spotify is
read-only here (managed in Settings).
- Wired the request-scoped reads to the per-profile client: the playlist LIST,
the playlist TRACKS view, liked-songs count, and user info — so a connected
user sees and opens THEIR OWN (incl. private) playlists, not the admin's.
Tests: builder falls back to the global client for admin/None/unconnected (the
non-regression guarantee); connections status reports unconnected; admin
disconnect rejected. 124 profile/spotify/gate/integrity tests pass.
Still on the global account (next step): sync/download jobs run in background
workers with no profile context — stamping the requesting profile onto the job
is the remaining wiring. Other services (Tidal/Deezer/Qobuz/Last.fm/ListenBrainz)
follow this same pattern.
Follow-up to the modal fix: the sidebar Service Status + dashboard service card
also mislabeled "Spotify (no auth)" as plain "Spotify". They read the status
`source`, which came straight from metadata.fallback_source ('spotify') with no
awareness of the metadata.spotify_free flag.
- get_primary_source_status now reports a DISPLAY source of 'spotify_free' when
fallback_source='spotify' + metadata.spotify_free is set (the raw 'spotify' is
still used for the auth/connected checks), and treats the free path's
availability as "connected" so the dot isn't falsely red on a no-auth setup.
- getMetadataSourceLabel maps 'spotify_free' → "Spotify (no auth)"; the status
presentation treats spotify_free as part of the Spotify family (session /
rate-limit / cooldown display still work). Added a SOURCE_LABELS entry.
- testDashboardConnection normalizes spotify_free → spotify (the only logic
consumer of the source value — the dashboard Test button).
Routing is unchanged (the real source stays 'spotify' + free flag); this is
purely the display layer. Settings was always correct. 64 integrity tests pass;
the 2 failing soundcloud tests are pre-existing (confirmed identical on a clean
tree).
Vicky-2418: Download Discography skipped some albums/singles as "No New Track"
even on a brand-new artist, while manual one-by-one worked. The log showed the
real reason wasn't ownership at all — it was "N skipped (artist mismatch)".
Root cause (confirmed against the iTunes API, not guessed): iTunes returns a
collab as ONE combined string. Narvent's "Miss You (Ambient Remix)" is credited
'TRVNSPORTER, Narvent & SKVLENT'. track_artist_matches did an exact full-string
compare ('trvnsporter, narvent & skvlent' == 'narvent' → False), so every
collaborator's discography entry was dropped — despite the #559 comment claiming
it "keeps features" (it didn't, because features arrive combined, not as a list).
Fix: match the requested artist as a COMPONENT of the credit — split on the
common separators (, & ; / feat ft featuring vs x), while still including the
full string so band names with internal separators ("Florence + the Machine")
match exactly. Component matching stays exact per name, so true contamination
(the #559 case — artist not credited at all) is still dropped, and substrings
("Drakeo the Ruler" ≠ "Drake") still don't match.
Also corrects an over-conservative #559 test that asserted "Drake & Future"
shouldn't match "Drake" — it should; Drake is a credited collaborator. The guard
drops uncredited artists, not legit collabs packed into one string.
Tests: the exact real case (Narvent in the combined credit) + each collaborator,
contamination still dropped, feat/ft/featuring/x forms, no substring false
positive. 36 filter tests + 747 discography/metadata tests pass.
THE bug behind "embeds art but never writes cover.jpgs": _fix_missing_cover_art
passed `details['album_folder']` (= dirname of the raw DB path, e.g. Jellyfin's
/data/music/...) as the target folder. That path doesn't exist inside the
SoulSync container — only the resolved /app/... path does. So apply_art_to_album_
files' `os.path.isdir(target_dir)` was False and the ENTIRE cover.jpg block was
silently skipped: embedding still worked (it uses the resolved paths) and the DB
thumb updated, but the sidecar was never written. Exactly Sokhi's symptom + the
"Cover art already present — database thumbnail updated" toast (cover_written
stayed False).
Fix:
- _fix_missing_cover_art: derive the folder from the RESOLVED file
(os.path.dirname(resolved[0])), never the raw album_folder.
- apply_art_to_album_files: bulletproof it — if the passed folder doesn't exist,
fall back to the real directory of the files instead of silently skipping.
Tests: a non-existent folder still writes cover.jpg to the real file dir. 1348
cover/art/repair tests pass.
Sokhi: after the earlier flag fix, scans returned 0 matches — albums with
embedded art but no cover.jpg were treated as fully arted and skipped, so their
cover.jpg never got written.
Root cause: the scan used album_has_art_on_disk (True if EITHER embedded art OR
a cover.jpg exists), conflating the two. Now it checks them separately:
- a local album is flagged if it lacks embedded art, OR it lacks a cover.jpg
sidecar AND cover.jpg writing is enabled (metadata_enhancement.cover_art_
download — Boulder: "only scan for cover.jpgs when enabled").
- an album that has embedded art but no sidecar is fixable even when the API
finds no art: the apply writes cover.jpg from the EXISTING embedded art.
apply_art_to_album_files now writes the cover.jpg sidecar by extracting the
album's own embedded art (new extract_embedded_art) — consistent with the
files, no API call — and only falls back to download_cover_art when there's
nothing embedded to extract. _fix_missing_cover_art no longer bails on a
missing artwork_url when sidecar_from_embedded is set.
Tests: scan flags embedded-but-no-cover.jpg (incl. when API finds nothing),
still skips albums with both, still flags artless albums; apply writes cover.jpg
from embedded art (no download), falls back to download when none, skips when a
sidecar already exists; extract_embedded_art unit tests. 1344 cover/art/repair
tests pass.
CubeComming: manual imports of large hi-res Qobuz FLACs came out as empty
shells — no audio, no tags, no length/bitrate. Root cause: mutagen's in-place
save() rewrites the file, and it's NOT atomic; if the process is interrupted or
OOM-killed mid-write (he also reported the memory-growth issue #802), the file
is left truncated — audio and tags gone.
save_audio_file (the chokepoint both the enrichment tag-write AND wipe_source_
tags route through) now saves atomically: copy the original to a temp in the
same dir, write the modified tags into the copy, verify it's still valid audio
(duration > 0), then os.replace() it in. The original is untouched until that
atomic swap, so a crash can only orphan the temp — never destroy the user's
file. Falls back to the plain in-place save (byte-identical to before) when the
atomic path can't run, so nothing is ever left worse off. tag_writer's
write_tags_to_file routes through the same helper.
Verified the atomic path works with REAL mutagen on a real FLAC (audio length
preserved 1.0s→1.0s, tag written, temp cleaned). Tests: replace-on-success,
original-survives-save-failure, corrupt-temp-rejected, no-filename-plain-save,
+ a real-FLAC round-trip (skips without ffmpeg). 2443 import/metadata/tag tests
pass.
Sokhi: filler works but doesn't write cover.jpg. Cause: the sidecar write
(download_cover_art) respects the import-time "Download cover.jpg to album
folder" toggle, while embedding ignores it — so with that toggle off, art
embeds but no sidecar is written.
A job literally called Cover Art Filler should produce the complete art when
you explicitly run it. download_cover_art gains force=True (bypasses the toggle)
and the filler's apply path passes it. The import pipeline still calls without
force, so it keeps honoring the user's setting.
Tests: filler passes force=True; existing cover-write mocks updated for the new
kwarg. 1732 art/cover/repair/import tests pass.
Sokhi still hit the read-only error after the statvfs fix (6c3e285a). Root
cause was a gap that fix didn't cover: read_only_fs was only set from the
per-file EMBED write, but download_cover_art SWALLOWS its own cover.jpg EROFS
(logs "Error downloading cover.jpg" and returns). So when an album's tracks
already have embedded art, the embed loop is skipped, only the cover.jpg
sidecar write runs, its EROFS is swallowed, and the filler reported success
while spamming the log — exactly Sokhi's case.
Fix (no blast radius — download_cover_art still never re-raises, since its
import-pipeline callers aren't wrapped): on EROFS it now records
'_cover_read_only' on the passed context instead of just logging; apply_art_to_
album_files passes a capture dict and promotes that to read_only_fs. So a
cover-only read-only album now correctly surfaces the read-only message instead
of a silent success.
Tests: +1 — embed skipped (track already arted) + cover.jpg read-only →
read_only_fs True. 472 cover/art/repair tests pass.
Sokhi got a read-only error from the cover-art filler with NO ':ro' in his
compose. Root cause: my earlier Tim fix added a statvfs pre-flight that bailed
when f_flag & ST_RDONLY — but union/FUSE/network filesystems (mergerfs,
rclone, NFS), ubiquitous in self-hosted setups, misreport those mount flags.
A perfectly writable library could be flagged read-only and blocked. statvfs
is a guess; the only honest test is whether an actual write raises EROFS.
- Removed the statvfs pre-flight entirely. Read-only is now detected solely
from a real EROFS on the embed write, which also fast-fails the remaining
files (so no statvfs needed for the fast-fail Tim wanted either).
- Broadened the user message: a genuine read-only mount isn't always ':ro' —
could be a read-only host/NFS/SMB mount or a mergerfs read-only branch.
Tests: writable FS succeeds even when statvfs would claim read-only (the
regression), real-EROFS-on-write still flagged + bails the rest, EACCES still
not conflated with EROFS. Dropped the now-moot Windows-statvfs test (statvfs
is no longer referenced). 445 art/cover/repair tests pass.
Boulder's lock-in question caught it: the read-only pre-flight called
os.statvfs unconditionally, which doesn't exist on Windows, and the
AttributeError wasn't covered by the except OSError — the whole cover-art
apply would have crashed for every Windows install (docker images are
Linux, so the reporter was fine; the maintainer wasn't). getattr-guarded
now: no statvfs -> skip the pre-flight, per-file EROFS detection (errno is
cross-platform) still active. Test pins the no-statvfs path.
Tim (Discord): cover-art automation fails with '[Errno 30] Read-only file
system' on every file; he chmod 777'd and nothing changed — because EROFS is
the KERNEL refusing writes to a docker ':ro' volume mount, which no chmod
can fix. SoulSync's response was a wall of per-file warnings and a fix
result that still said success with a soft "(read-only?)" hint.
- apply_art_to_album_files now pre-flights the album folder with statvfs
(asks the kernel, writes nothing): a read-only mount short-circuits the
whole album instead of failing file by file. Belt: a per-file/cover EROFS
(overlay quirks where statvfs lies) still sets the flag.
- the repair worker's apply now FAILS the finding with the actual cure:
"remove ':ro' from the volume mapping and recreate the container — chmod
cannot change this". EACCES (a real permissions problem chmod CAN fix)
deliberately keeps the old soft path.
Tests: RO mount short-circuits before any file/cover write, save-time EROFS
still flagged, EACCES not conflated with EROFS. 29 art/repair tests pass.
Sokhi (continued from #806): volume-numbered series ('B小町 …キャラクター
ソングCD Vol.2' / 'Vol.2.5' / 'Vol.4' / 'Vol.4.5') got each other's art from
both normal downloads and the retag tool. Two distinct holes, one principle:
1. The art picker's _album_matches validates by significant-token SUBSET —
built to tolerate '(Deluxe)'/'- Remastered' suffixes. CJK strips out of
the normalizer entirely, so Vol.4 → {b,tv,cd,vol,4}, a clean subset of
Vol.4.5's {b,tv,cd,vol,4,5}: the wrong volume validated as "the same
album with a suffix". Affected every fuzzy art source (iTunes, Deezer,
AudioDB, Spotify) in downloads, retag, and the missing-art repair.
2. MusicBrainz match_release scores by string similarity — Vol.4 vs Vol.4.5
is 0.973, so the wrong volume could win the match outright, and its MBID
then feeds Cover Art Archive with NO downstream validation (CAA is
MBID-keyed, trusted by design). With Sokhi's MB metadata source this is
the likely path in his logs (his release-group 404s push re-matching).
The shared rule (core.text.title_match.numeric_tokens_differ): digit-bearing
tokens must be IDENTICAL between the two titles. A number on one side only —
volume, part, sequel, remaster year — is a different release, never a
suffix. '1989' vs '1989 (Deluxe)' still matches (digits shared); 'Album' vs
'Album 2' now rejects (sequels!). Art picker rejects outright (falls through
to next source / the download's own art — the designed cost of a false
reject); MB matcher halves the candidate's confidence, landing it below the
70 gate while the exact-volume result is untouched.
Tests: helper truth table, the exact reported pairs through _album_matches,
and match_release end-to-end (wrong volume alone → no match beats a wrong
MBID; exact volume beats near-identical wrong one despite lower MB score).
828 matching/metadata + 301 musicbrainz/retag/artwork tests pass.
Boulder: "Taylor Swift shows only 8 albums, nothing before 2022, no singles,
no EPs" — for every artist (actually: every WATCHLIST artist). Traced live:
get_artist_albums caches its result under an UNQUALIFIED key (no limit/page
info), and the watchlist's new-release probe (limit=5, max_pages=1 — the
April "reduce watchlist API calls ~90%" optimization) stored its truncated
single page in that same slot. The artist detail page reads the cache first,
so a watchlisted artist's page showed only the newest handful of releases —
newest-first, hence "nothing before 2022" — re-poisoned on every scan, with a
30-day TTL. When the source-priority fetch comes back tiny, the page's
fallback path quietly serves it, so the symptom looked like a discography
filter bug. Not related to the #808 matching change (that is a pure max(),
provably additive).
Three pieces:
- get_artist_albums tracks whether the fetch stopped while more pages
existed (truncated) and only caches COMPLETE discographies. Individual
albums keep their opportunistic caching — they're complete entities
regardless of pagination. A small real discography that fits one page
stays cacheable even under max_pages=1.
- MetadataCache.purge_artist_album_lists(): delete the already-poisoned
album-list entries (TTL would have kept them for weeks); lists rebuild
lazily on the next artist-page visit.
- one-time startup purge in web_server, config-guarded
(maintenance.album_cache_purge_v1), mirroring the startup-repair pattern.
Tests: truncated probe never stores the list (but still returns its page),
complete multi-page fetch caches, and a genuinely-small one-page discography
under max_pages=1 still caches. 1087 spotify/cache/watchlist/artist tests
pass.
Watchlist scans add announced albums on purpose (so singles download the day
they drop), but the future-dated tracks leaked into two hot paths:
- Fresh Tape / Release Radar: future albums got NEGATIVE days_old, and the
recency score (100 - days*7) has no upper clamp — prereleases weren't just
slipping into the radar, they were mathematically FAVORED above every
released track. That's the "50% prerelease" report. The builder now skips
confidently-future albums (and clamps days_old to 0 as a belt).
- Wishlist processing: every auto cycle burned a full Soulseek search +
timeout per unreleased track (~60 tracks/cycle for the reporter). Both the
auto and manual flows now skip future-dated tracks with a counted log line.
They STAY in the wishlist and join the cycle automatically the day their
release date passes — no state, the date check is per-cycle. An explicit
manual track selection overrides the gate (the user asked for those).
The gate (core/metadata/release_dates.py, pure + tested) is conservative by
design: Spotify dates come as YYYY / YYYY-MM / YYYY-MM-DD, and a track only
gates when its date is CONFIDENTLY future at its stated precision. Release
day counts as released; garbage or missing dates never block anything
(including out-of-range months/days, which fall back a precision level).
Tests: 6 covering all precisions, the release-day boundary, garbage
tolerance, dict shapes, and ordered partitioning. 403 wishlist/watchlist
tests pass.
The lock-in pass caught the cost hole: art is fetched PER TRACK, and the old
code never touched archive.org at all — so an archive.org outage was free,
while the new native-first chain would pay a 10s timeout on every track
(a 12-track album = +2 minutes, exactly the import-slowness class we spent
today killing). One failed original now puts originals on a 10-minute
cooldown: subsequent fetches go straight to the 1200px CDN midpoint (the
pre-#806 behavior, full speed) and recover automatically when the cooldown
expires. Locked by a test: track 1 pays the failure once, track 2 never
touches the original. (Also: the missing time import the first run caught.)
The CAA branch of _upgrade_art_url capped art at the /front-1200 thumbnail —
a deliberate flakiness trade-off, but the policy had rotted into inconsistency:
iTunes art already shipped at 3000x3000, and bare /front URLs (release-group
lookups — exactly what the Re-tag flow produces) bypassed the cap entirely,
which is how Sokhi observed retag delivering full-res while downloads got 1200.
CAA URLs now upgrade to the bare /front ORIGINAL (native res, frequently
3000px+). The flakiness concern that motivated the old cap is handled where it
belongs, in the fetch: _fetch_art_bytes now walks an attempt chain — original
-> /front-1200 midpoint -> the original sized thumbnail — so a flaky
archive.org degrades to the old 1200px behavior, never below it.
Tests updated to the new contract (+3 chain tests: native-first, flaky
degrades to 1200 not 250, full chain ends at the thumbnail). 623 metadata +
1267 art-path tests pass.
Measured during a live album download: ~4m15s per track in post-processing
(normal is ~20s), with the time vanishing silently inside embed_source_ids —
up to 5 MusicBrainz calls per track crawling against a degraded musicbrainz.org
while the MB enrichment worker kept eating the same ~1 req/s per-IP budget.
Only Spotify/Last.fm/Genius were in the yield set; MusicBrainz, Deezer, iTunes,
Discogs etc. kept grinding through downloads.
Policy (new core/enrichment/yield_policy, tested):
- downloads active -> ALL enrichment workers yield (post-processing touches
every metadata source). listening-stats (local-only) and repair
(user-scheduled) intentionally keep running.
- discovery active -> the API-contention five yield (spotify/itunes/deezer/
discogs/hydrabase) — discovery never paused anything before, despite the
pause helper literally defaulting to label='discovery'.
- user overrides and user-paused bookkeeping keep their existing semantics;
the dashboard yield_reason label now says WHICH foreground work caused it.
Observability (the 4-minute silence can never come back):
- every source lookup is timed; >2s logs a warning NAMING the source and
duration (core/metadata/source.py _call_source_lookup)
- the pipeline always logs "Metadata enhancement took X.Xs" per track
7 policy tests (incl. the motivating case: MB yields to downloads, keeps
running during discovery); 277 pipeline/enrichment tests pass.
A single enriched against the deluxe gets every source ID pointing at the deluxe,
so the organizer filed it as e.g. track 2 of a 10-track album. Root cause: the
canonical resolver only ever scored the editions already linked — the correct
single was never even a candidate, and the misfit deluxe scored so low (0.1,
below the 0.5 floor) that nothing got pinned and the priority-walk grabbed the
deluxe anyway.
Fix, in three tested layers:
- resolve_canonical_for_album gains a fetch_alternates seam: when no linked
edition clears the floor, it scores the source's OTHER editions of the same
release and re-picks by best fit (dedup, injected, pure).
- default_fetch_alternates lists the artist's editions and keeps the same-release
ones (edition-blind name match: Deluxe / - Single / [Remastered] all collapse),
returning their tracklists. Favors recall; the scorer is the precision gate.
- _resolve_source does the misfit check inline: it fit-scores the walked edition
and only on a clear misfit searches for a better edition, then persists the pin
on apply (Track Number Repair + future runs agree). Cost-neutral and behavior-
identical for well-fitting albums (no extra API calls); strict_source and the
#758 manual lock are never overridden.
Tests: +4 resolver (expand/no-expand/dedupe/back-compat), +7 alternates (name
matcher + fetcher over fake APIs + cap), +3 organizer end-to-end (misfit->single
+pin, well-fit->no-expand, strict->no-expand). 300 passed across the reorganize
+ canonical family, lint clean.
Users manually match an album to the regular edition, but enrichment/
repair keeps treating it as the deluxe (missing songs, renumbered tracks).
Root cause: an album has TWO identities — the enrichment match
(spotify_album_id, which manual-match sets and the worker already honors)
and a SEPARATE canonical version pin (canonical_album_id, added by #777).
The canonical pin is what track-number repair / reorganize / missing-track
detection actually read, and library_manual_match never wrote it — so it
was resolved independently and landed on the deluxe edition.
(So #777 did NOT solve #758: it added canonical pinning, but manual
matches didn't write the pin.)
Fix: a manual ALBUM match on a canonical-recognised source now also pins
AND locks the canonical version to the chosen release:
- new canonical_locked column (same migration pattern as the other
canonical cols).
- set_album_canonical(..., locked=False) gains an atomic WHERE-clause
guard: an auto write can't overwrite a locked pin; a manual write
(locked=True) always wins. get_album_canonical exposes `locked`.
- library_manual_match pins canonical for album matches via the pure
should_pin_manual_canonical(entity_type, source).
The auto resolve job already skips already-pinned albums, so the lock is
protected on two fronts; the new guard also covers any future
re-resolution. A new manual match still overrides.
18 tests: the pure gate (+ a sync-invariant test vs _ALBUM_ID_COLUMNS)
and the DB lock seam (auto can't clobber a manual lock; manual overrides;
auto-over-auto still works). Additive — locked defaults False, so the
auto path is unchanged unless a manual lock exists. Full suite clean.
write_tags_to_file wrote the core fields + cover but never the source IDs
(Spotify/iTunes/MusicBrainz) the import post-process embeds. Added a focused
source.embed_known_source_ids() that writes ALREADY-KNOWN ids (from db_data)
via the canonical, Picard-compatible frame writer the import uses
(_write_embedded_metadata) — no API re-fetch, frames correct by construction.
write_tags_to_file now calls it whenever db_data carries id keys.
Fed from both paths: the enhanced-library 'Write Tags' button now carries the
track's spotify/itunes/musicbrainz ids, and the Library Re-tag job stamps the
matched album/track source ids onto each track. So both now write the full tag
set, not a subset.
Verification found a non-additive edge: embed_album_art_metadata uses FLAC
add_picture(), which APPENDS — so applying to an album where some tracks already
had art would have added a duplicate embedded picture. The apply now checks each
file and skips any that already carry art (shared _audio_has_art helper), so it
only ever ADDS art to files missing it. Test covers the skip (no re-embed).
Previously the filler only flagged albums whose DB thumb_url was empty and, on
apply, only updated that DB thumb_url — so albums whose files had no embedded
art and no cover.jpg (but whose DB row had a URL) were never found, and even
'applying' art never touched the files. That's the reported 'doesn't scan all
albums' gap.
New core.metadata.art_apply (reuses the post-processing standard so the user's
album_art_order is honored):
- album_has_art_on_disk(): cheap-first check — folder cover.jpg/folder.jpg
sidecar, then embedded art in a representative track (FLAC/ID3/MP4/Vorbis).
- apply_art_to_album_files(): embeds via embed_album_art_metadata + writes
cover.jpg via download_cover_art; only ADDS art (never rewrites the user's
tags); read-only/unwritable files are skipped + counted, never crash.
Scan now examines every titled album and flags it when art is missing in the DB
OR on disk. Apply embeds into the album's audio files + writes cover.jpg in
addition to the DB thumbnail (media-server-only albums fall back to DB-only).
Tests cover sidecar/embedded detection, the cheap-first short-circuit, and the
apply orchestration (embeds each file + cover.jpg; read-only failures counted).
The worker's WARNING observability proved the '38 errors' were almost all
MusicMap returning 404 (artist has no map page) — a genuine not-found, not a
fetch failure. But iter_musicmap_similar_artist_events flattened every
RequestException to status_code 502, and the worker maps 400/404 -> not_found
/ everything-else -> error, so these inflated the error count.
Surface the real HTTP status from the exception's response (404 stays 404),
falling back to 502 only when there's no response (timeout/connection drop,
which is correctly still an error eligible for retry).
Regression tests: 404 -> 404 (not_found), timeout -> 502 (error), 500 stays
error, plus an end-to-end worker check that a 404 result marks 'not_found'
and stores nothing.
- B023: default_fetch_tracklist built a per-item lambda closing over the loop
variable `it`. Replaced with a module-level _item_get(item, key, default)
helper (takes the item as a param — no closure). Behavior unchanged; the
dict/object normalization test still passes.
- S110: the two best-effort guards in the canonical job (skip-already-pinned
read, estimate_scope active-server read) now carry `# noqa: S110 — <reason>`,
matching the repo's existing convention for intentional swallow-and-continue.
ruff check passes on all canonical files + tests; 30 affected tests green.
Per request, pack each finding with everything available WITHOUT extra API
calls (kettui: reuse what's already fetched, read the album row we already
loaded, degrade per-field, keep it tested):
- Pinned release's track titles — already fetched during scoring, so free
(capped at 60 to bound details_json).
- From the album row (free): year, DB track count, total duration, genres-free
context, and the album's currently-linked source IDs.
- file_track_titles (your library's titles) for a side-by-side with the release.
- Artist + album thumbs (artist via the guarded lookup) and names.
_describe_pin now renders: "Artist — Album (year)", the fit breakdown, "Currently
linked: … → pinning X", "Beat: <alternatives>", and the release tracklist — so
the card is judge-able at a glance, and the structured fields are in details for
a richer UI.
NOT included (would cost an extra per-album API fetch, left as opt-in): the
*release's* own year/type/cover/URL from get_album_for_source, vs the library's.
Tests: _describe_pin rich-render (year/linked/tracklist), resolver release-titles,
orchestration free-context fields. 94 canonical + reorganize regression pass.
Findings now carry artist_thumb_url alongside album_thumb_url (same key the
track-repair findings use, so the findings UI already renders it).
Fetched via a guarded _lookup_artist_thumb() — checks the artists table has a
thumb_url column first and swallows any error — rather than adding ar.thumb_url
to the shared load_album_and_tracks SELECT. The shared-loader approach was
tried first and REVERTED: it crashed reorganize on schemas whose artists table
has no thumb_url column (caught by 40 orchestrator tests). The lookup only runs
for albums that actually resolve, so it adds no cost to the no-source-id
short-circuit majority.
Tests: orchestration test asserts artist_name + album_thumb_url + artist_thumb_url
flow through. 47 canonical + 104 canonical/reorganize regression tests pass.
Live-run feedback: "Best-fit release: deezer (665666731), score 1.0" is too thin
to trust/accept. Each finding now explains WHY:
- score_release_detail() exposes the per-signal breakdown (count/duration/title)
instead of just the blended score.
- resolve_canonical_for_album returns an enriched result: the breakdown,
file_track_count vs release_track_count, and a `candidates` list of every
source it scored (so a finding can show what the winner beat).
- resolve_and_store adds album/artist/thumb context from the row it already
loaded (no extra query). Storage still only reads source/album_id/score.
- The job builds a real description via _describe_pin(), e.g.:
"Pin deezer release 665666731 (confidence 100%).
Fit to your library: 11 files vs 11 tracks on this release — track count
100%, durations 100%, titles 100%.
Beat: spotify 65% (17 tk)."
and a clearer title ("Pin deezer as canonical: <artist> — <album>").
Tests: resolver enrichment (breakdown + candidate comparison fields), and
_describe_pin (judge-able text incl. the beaten alternatives, and honest "n/a"
for a missing signal). 42 canonical tests pass.
Note: the description string carries the judge-able info regardless of UI; how
the findings tab renders the extra details keys (thumb image, candidates table)
is still UI-dependent and unverified.
Feedback from the live dry-run: the job was pinning whichever source best fit
the files regardless of which source it was, which was surprising — users
expect it to respect their active metadata source. Made it a per-job setting
instead of a baked-in policy.
source_selection (default 'active_preferred'):
- active_preferred — use the active/primary metadata source's release when the
album has an ID for it AND it clears the score floor; otherwise fall back to
the best-fit among the other sources. Respects the configured source but
self-heals when that link is clearly broken (below floor / no ID).
- active_only — only ever the active source; never considers others.
- best_fit — previous behavior: whichever source matches the files best.
resolve_canonical_for_album gains mode + primary_source; the orchestration
threads the primary source through; the job reads source_selection from its
settings. Note: active_preferred respects the active source as long as it clears
the floor, so it will NOT override a deluxe-vs-standard mismatch on the primary
(#767-Bug2) — that's what best_fit is for; the choice is now the user's.
Tests: per-mode coverage in test_canonical_resolver.py (active_preferred uses
primary when it fits, falls back when primary is below floor, keeps primary even
when another fits better; active_only pins primary / never falls back; best_fit
unchanged), orchestration default-mode test, and the setting default. 39
canonical tests pass.
The populate trigger that turns the (until now dormant) feature on. Until a user
enables and runs this job, no album has a canonical -> both read sides (Stages
3-4) fall back -> zero behavior change. So the whole feature ships safely off.
- core/repair_jobs/canonical_version_resolve.py — "Resolve Canonical Album
Versions". Iterates the active server's albums, skips ones already pinned, and
calls the tested resolve_and_store_canonical_for_album per album. Opt-in
(default_enabled=False) and dry-run-by-default: resolving compares an album's
candidate releases across sources (metadata-source API calls, once per album),
so it's deliberately user-triggered. Dry run reports a finding per album it
would pin; live mode stores. Registered in _JOB_MODULES.
- core/metadata/canonical_resolver.py — resolve_and_store gains store=True; the
job's dry run passes store=False to resolve-without-writing.
Tests: tests/test_canonical_version_job.py (6) — registered, opt-in + dry-run
defaults, live resolves+stores (auto_fixed), dry run creates findings without
persisting, already-pinned albums skipped. Registry loads all 19 jobs cleanly.
145 tests across the full feature + reorganize/track-repair/DB regression pass.
Completes Stage 2's populate path. Still dormant — no consumer calls it yet.
- resolve_and_store_canonical_for_album(db, album_id, ...): loads the album's
source IDs + its tracks' (duration_ms, title) from the DB via the SAME
loader the Reorganizer uses (load_album_and_tracks + _extract_source_ids), so
the canonical is chosen over exactly the source IDs the reorganizer sees;
scores off the DB track rows (the library's view of the files — no per-file
disk reads), resolves the best fit, and persists it. Returns the stored result
or None when unresolved.
- default_fetch_tracklist(): production fetcher wrapping
get_album_tracks_for_source, normalising to {title, track_number, duration_ms}
(duration best-effort; sec->ms; absent -> scorer leans on count+title).
Design note: chose LAZY resolution (Stages 3-4 consumers call this when they hit
an album with no canonical) over a standalone backfill repair job — no new
scheduling/UI surface, resolves only when a tool actually needs it, and stays
gated (NULL canonical = today's behavior).
Tests: tests/test_canonical_orchestration.py (5) — end-to-end on a real temp DB
(11 files pick the 11-track release over a 17-track deluxe and persist it),
no-source-ids -> None, missing-album -> None, and default_fetch_tracklist
normalization (dict items, seconds->ms) + failure -> None. All canonical +
DB-migration tests green.
Turns the Stage-1 scorer into an end-to-end resolver + persists the result.
Still DORMANT — no consumer reads it yet, so zero behavior change.
- core/metadata/canonical_resolver.py — resolve_canonical_for_album(): builds
candidate releases from the album's per-source IDs (in source-priority order),
fetches each tracklist via an INJECTED fetch_tracklist (so it's unit-testable
without live APIs), scores them with pick_canonical_release, and returns the
best-fit {source, album_id, score}. Skips sources with no id / failed fetch;
returns None when there are no files, no candidates, or nothing clears the
confidence floor.
- database/music_database.py — set_album_canonical() / get_album_canonical()
write/read the Stage-1 columns. get returns None when unresolved, which every
consumer will treat as "fall back to today's behavior".
Tests: tests/test_canonical_resolver.py (7) — best-fit beats priority, priority
breaks true ties, skips missing-id/failed-fetch sources, None on
no-candidates/no-files/below-floor, score rounding. tests/test_canonical_db.py
(4) — set/get round-trip incl. timestamp, unresolved -> None, overwrite,
missing-album -> False. 34 canonical + DB-migration tests pass.
Remaining for Stage 2 (the trigger): read on-disk file durations/titles for an
album, gather its source IDs, call the resolver, store — wired via a backfill
repair job + an enrichment hook. Then Stages 3-4 wire the Reorganizer and Track
Number Repair to READ the pinned canonical.
First stage of the canonical-album-version fix (#765 + #767-Bug2). Pins ONE
canonical (source, album_id) per album, chosen by best-fit to the user's actual
files, so the Reorganizer, Track Number Repair, and tagging stop re-resolving
independently and contradicting each other.
Ships DORMANT — nothing reads or writes the new data yet, so zero behavior
change. Later stages populate (Stage 2) and consume (Stages 3-4) it.
- core/metadata/canonical_version.py — pure scorer (the testable heart):
score_release_against_files() rates a candidate release by track-count fit +
duration alignment (greedy nearest within ±3s) + title overlap, dropping and
renormalizing missing signals so it never crashes on sparse metadata.
pick_canonical_release() takes candidates in source-priority order, picks the
best fit, breaks ties toward the earlier (higher-priority) candidate so the
choice is DETERMINISTIC — that determinism is what makes every tool agree
(#765), while count/duration fit picks the right EDITION (#767-Bug2). A
confidence floor (default 0.5) means a low-confidence guess is never pinned.
- database/music_database.py — additive, nullable columns on albums
(canonical_source / canonical_album_id / canonical_score /
canonical_resolved_at), guarded by the existing PRAGMA-table_info pattern.
NULL = unresolved = every consumer falls back to today's behavior.
Tests: tests/test_canonical_version.py (11) — edition discrimination (11 files
-> standard, 17 -> deluxe), deterministic priority tiebreak, duration
disambiguation on count ties, graceful degradation (no durations / counts only /
fuzzy titles), confidence floor, empty-input safety. tests/test_canonical_
columns_migration.py (4) — fresh DB has the columns, they're nullable w/ NULL
default, migration is idempotent, and it ALTERs them onto an old albums table.
60 DB/schema regression tests still pass.
enhance_file_metadata rebuilds tags from scratch: for FLAC it calls
clear_pictures(), for MP3/MP4 it clears the whole tag block — and it does
this UP FRONT, then saves the file, long before it tries to fetch and embed
the replacement art. So every way the re-embed could come up empty left the
file saved with the original art destroyed and nothing put back:
- extract_source_metadata returns nothing -> early save, no embed
- no album-art URL / art download fails / rejected by the min-size guard
-> embed_album_art_metadata returns early without adding a picture
- art embedding disabled in config -> embed skipped entirely
- embed raises mid-enrichment -> file left cleared on disk
This is the "cover art gets corrupted/destroyed during import" half of #764
(continuation of #755); distinct from #750's truncated-cache DISPLAY bug.
Fix: new core/metadata/art_preservation.py snapshots the existing art
(the live Picture / APIC / MP4Cover objects, so they re-apply verbatim)
BEFORE the clear, and restores it before each save IFF the file currently
has none. Wired into all three exit paths in enhance_file_metadata
(no-metadata early return, the final save, and the except handler). The
restore is a strict no-op when art is already present, so the happy path —
new art embedded — is byte-for-byte unchanged: it never clobbers or
duplicates a freshly-embedded cover. embed_album_art_metadata now returns a
bool so the intent (embedded / didn't) is explicit.
Tests:
- tests/test_art_preservation.py (5) — snapshot/restore round-trips through
real mutagen FLAC + ID3 objects; restore no-ops when new art is present.
- tests/test_enrichment_art_preservation.py (4) — runs the REAL
enhance_file_metadata over a real FLAC with embedded art and asserts the
art survives on disk for missing-metadata / failed-embed / embed-raises,
and is correctly REPLACED (exactly one picture, new bytes) on success.
1019 tests pass across the metadata/enrichment/imports/acoustid suites.
Follow-up to the preferred-art feature. Real test runs showed a source could
win on priority while handing back a small cover: Cover Art Archive is
volunteer-uploaded with no size floor, so CAA-first gave a 599x531 (Taylor
Swift) and a 600x600 (Kendrick GNX) -- front-1200 only caps the max, so a
~600px upload stays ~600px -- and Deezer/iTunes lower in the order never got a
turn.
Fix:
- Minimum-resolution guard: artwork._min_size_art_validator builds the
resolver's validate hook -- it fetches each candidate, caches the bytes (so
the winner isn't fetched twice), and accepts art only when its shortest side
>= metadata_enhancement.min_art_size (default 1000px; 0 disables). Art that's
too small is a miss, so the resolver falls through to the next source instead
of winning on priority. Unmeasurable images are accepted (don't over-reject;
fallback is still today's art). Wired into both embed_album_art_metadata and
download_cover_art.
- iTunes art upgraded to /3000x3000bb/ (was the 600px default) so it
contributes high-res when it wins.
- select_preferred_art_url gains a validate passthrough to the resolver.
- config default metadata_enhancement.min_art_size: 1000.
Effect: with an order like caa > deezer > spotify > itunes, a ~600px CAA upload
is now skipped and Deezer's ~1900px wins -- consistent big art. (Spotify art
often maxes ~640px, so it's skipped at the 1000 floor in favor of bigger
sources; lower min_art_size to ~640 to allow it.)
Tests: tests/metadata/test_art_min_size.py (6 -- incl. the real 599x531 and
600x600 cases, shortest-side logic, unmeasurable-accept, no-bytes-reject,
0-disables) + iTunes max-res upgrade test. Full metadata suite green (617).
