Files named '01 - Sun It Rises.flac' with no embedded title tag leaked the stem,
number and all, into tracks.title as '01 - Sun It Rises' — which never matches the
canonical 'Sun It Rises', so the real track reads as a false 'missing' and albums
sort wrong.
New conservative strip_leading_track_number (paths.py): removes a clear track-number
prefix (zero-padded number, OR a number followed by a real separator+space) while
leaving titles that merely start with a number untouched — '7 Rings', '99 Luftballons',
'50 Ways to Leave Your Lover', '1-800-273-8255', '1979' all preserved. Never reduces
to empty/bare-number/punctuation.
Applied at:
- get_import_clean_title (context.py) — the universal resolver every import path funnels
through, so the DB title AND the re-written embedded tag come out clean.
- album_matching scorer — so '01 - Sun It Rises' scores against 'Sun It Rises' and the
file matches its real track (inheriting the clean canonical name).
27 targeted tests + 772 imports/matching green.
Picking the release a track is ALREADY in deleted the file: the re-import lands at
the same path, record_soulsync_library_entry skips the insert (row exists), so no
new row is created — then delete_replaced_track removed that very row AND unlinked
the file (the freshly-imported one). This was the 'assumption' I documented but
never enforced.
Bulletproof guard: the pipeline writes its landing path into context['_final_processed_path'];
_process_matches captures it onto the candidate, and _finalize_rematch_hint passes
those new_paths to delete_replaced_track. If the old file canonically equals where
the import landed, it's a NO-OP — the row and file are kept (that file IS the
re-imported track). Canonical compare folds symlinks/case/sep.
So same-release re-identify is now a harmless re-tag-in-place; only a genuine re-home
(different path) deletes the old. 114 auto-import + rematch tests green.
Two real bugs surfaced in testing:
1. Original file not deleted on replace: delete_replaced_track checked os.path.exists
on the RAW stored DB path (a Docker/media-server view), so it removed the row but
orphaned the file. Now takes a resolve_fn (wired to resolve_library_file_path in
the worker) and unlinks the RESOLVED real path.
2. No year / wrong album context: build_identification_from_hint set is_single=True,
routing re-identify through _match_tracks' singles fast-path — which never fetches
the chosen album, so the re-import got a bare stub (no release_date, total_tracks=1).
Added force_album_match so the matcher FETCHES the chosen release even for a lone
staged file → the track inherits the real album's year, in-album track number, and
art. Holds for single-type releases too (they have a year as well).
Normal single-import behavior unchanged (force_album_match absent → same path).
112 auto-import + rematch tests green.
The showpiece: a focused 'which release does this track belong to?' chooser.
Source tabs (default active), pre-seeded search, the same song surfaced across
single/EP/album with color-coded type badges, ISRC-ranked, replace-original
toggle (on by default). Glassy panel, blurred hero art, shimmer/spinner states,
hover-lift result cards — matched to the app's modal language.
Backend:
- core/imports/rematch_apply.py: pure staged_destination + build_reidentify_hint,
injectable stage_file_for_reidentify (COPIES the file, never moves — original
safe until re-import succeeds). 6 tests.
- POST /api/reidentify/apply (admin-only): resolve_hint_fields → stage file →
create_hint → nudge the worker. Replace deletes the old row only on success.
Frontend: modal markup (index.html), full stylesheet (style.css), and the
openReidentifyModal/search/select/confirm flow (library.js). Not yet reachable
from a button — Phase 5 wires it.
Search any configured source (tabs, default active) and surface the SAME song
across its collections (single/EP/album) so the user can pick which release a
track should be filed under.
- core/imports/rematch_search.py: pure normalize + injected client factory.
search_release_candidates() → lightweight display rows from typed search_tracks
(title/artist/release/type badge/year/count/art/isrc/track_id); resolve_hint_fields()
runs ONCE on the picked row via get_track_details to pull the album_id (+ isrc/
track#/disc) the hint needs. infer_release_type() handles Spotify's missing 'EP'
(multi-track 'single' → EP badge); filing is driven by real album_id, not the label.
- GET /api/reidentify/sources (tabs) + GET /api/reidentify/search (rows). Graceful
empty on dead source / blank query / client error — never raises.
14 tests. Inert until the modal (Phase 4) calls it.
When a staged single-file candidate carries a re-identify hint, the worker builds
the identification straight from the user-chosen release (album_id/source) and
skips the guessing tiers — so the ambiguity that mis-filed the track is gone. No
hint → byte-identical to before (the lookup returns (None, None), fail-safe on any
DB error). A hinted import auto-processes (explicit user choice), still gated on the
global auto_process pref.
After the re-import lands, _finalize_rematch_hint consumes the hint and (if replace
was chosen) deletes the old row + file via delete_replaced_track — deferred to
success so a failed import never loses the original. Safe by construction: unlink
only when no surviving row references the file, and the modal never offers the
track's current release so old path != new path.
All hint logic lives in auto_import_worker.py + the pure rematch_hints helpers —
pipeline.py / side_effects.py untouched. 18 tests; full auto-import suite green.
A single-use, user-designated 'which release does this track belong to' answer.
Written when the user picks a release in the Re-identify modal and the file is
staged; the import flow will read it at the top of matching and consume it.
- rematch_hints table (additive, IF NOT EXISTS + indexes) keyed on staged_path
with content_hash as a rename-proof fallback.
- core/imports/rematch_hints.py: pure DB seam over an injected cursor
(create/find/consume/list) + a cheap size+head+tail file fingerprint.
- exempt_dedup baked into the hint (a re-identify must bypass dedup-skip);
replace_track_id carried for deferred post-success cleanup.
Inert until wired (Phase 5) — nothing calls it yet. 9 seam tests.
detect_album_info_web gains a last-resort step: when a track matched a SINGLE
with no usable album context, look up the parent ALBUM that contains it (via
get_artist_albums_for_source + get_artist_album_tracks) and promote to it, so it
groups with its album-mates and gets the album's cover instead of the single's.
GATED behind metadata_enhancement.single_to_album (default OFF) — it's a
per-import metadata lookup, so it's opt-in, matching the canonical-version
pattern. Fully fail-safe: flag off, no source, or any client error/miss -> None,
so the track stays exactly as matched (never worse than today). The promoted
album name is forced past get_import_clean_album (which otherwise pins the
single's name) so grouping + tags use the album. 4 glue seam tests added
(promote-when-enabled, disabled-by-default, no-match, client-raises); 462
import-suite tests pass.
When a track matches a SINGLE release it carries the single's name/id and the
canonical grouping files it apart from its album-mates -> mixed cover art
(Sokhi). This re-homes it onto the album that actually contains it.
The selection is a pure, CONSERVATIVE function and the lookup loop takes injected
fetchers, so both are unit-testable without a live client. It only re-homes a
track when a real 'album'-type release's tracklist contains that EXACT track
(qualifier-tolerant) — never promotes a genuine standalone single, never guesses
(a wrong promotion would mis-home a real single, the inverse bug). Fail-safe: any
miss/error -> None (track stays as matched). 13 seam tests. Wiring next.
Sokhi: songs in one album get mismatched cover art. Root cause is upstream of
the repair jobs (which correctly apply one cover per album_id): the standalone
import grouped albums by the album NAME hash (artist::album_name), so the SAME
release split into multiple album rows whenever the name string drifted, and the
cover-art/re-tag jobs then dressed each split row in its own art.
Foundation (new imports only; existing rows untouched): a pure, seam-testable
helper find_existing_soulsync_album_id() resolves the album row by precedence
name-hash id -> source RELEASE id -> (title, artist). When an import carries a
metadata-source album id, a differently-named import of the SAME release now
unifies into one row instead of splitting. Source-column lookup is allow-listed
(it's spliced into SQL) and guarded so a source without a dedicated album column
(Deezer) falls through to the name match instead of breaking the import.
Deliberate scope: this does NOT merge a track that genuinely matched a SINGLE
(a different release id) into its parent album — that needs single->album
resolution upstream and is the next step; this is the grouping substrate it will
feed. 10 seam tests (canonical unify, single-vs-album stays separate, precedence,
allowlist, server-source scope, missing-column fallthrough).
Single tracks (esp. Deezer-sourced) imported as "01 - Title" regardless
of their real album position — e.g. Fly Away (track 2 of Greatest Hits)
landed as 01, littering album folders with duplicate "01" files.
Root cause: a Deezer single track is matched via /search/track, which
omits track_position, so the context never carried the real number; then
service.py + context.py fabricated a confident track_number=1 from that
gap. Because the resolver puts that first, the fake 1 beat the source.
It is source-agnostic (slskd-with-Deezer-metadata hits it too) — albums
work because /album/<id>/tracks DOES include positions.
Fix (at the shared import funnel, strictly additive):
- track_number.py: new read_embedded_track_number() (mutagen, local, no
network) + an optional embedded_track_number arg on resolve_track_number.
The downloaded file already carries the source-written position (deemix
wrote it); consult it LAST — only when metadata AND the "NN - Title"
filename both come up empty — so it can only fill the gap that would
otherwise hit the default-1 floor. Never overrides a value the pre-fix
resolver produced (no regression for correctly-named/mistagged files).
- pipeline.py: read the file tag at the resolve step and pass it in.
- De-poison: service.py:217 + context.py default to 0 (the existing
"unknown" sentinel, like total_tracks), NOT 1 — so the fake 1 no longer
blocks recovery. Frontend already treats falsy track_number as unknown
(omits it), so this also drops the bogus "1." in the UI.
13 new resolver tests incl. the no-regression precedence guards; full
imports + wishlist suites green (583), no behavior change for albums.
Multiple failed source attempts at one song each land in quarantine as
separate entries. Group them by the *intended* target (sidecar context
track_info isrc -> id -> uri, falling back to normalized artist|title for
legacy thin sidecars) — an exact relationship across siblings, since the
bad files' own tags differ but the target track is constant.
- core: quarantine_group_key() + find_quarantine_siblings() seams; list
entries now carry group_key.
- approve endpoint: remove_siblings flag auto-deletes the other attempts
once one is accepted (captured BEFORE approve restores the file out of
quarantine, or the id lookup would resolve nothing). Scoped to the
quarantine manager; download-modal chooser + version-mismatch fallback
pass no flag and are unaffected.
- UI: multi-member groups render as a collapsible parent row (album art +
'N alternatives'); singletons unchanged. Toast reports removed count.
- 11 tests incl. ordering regression for capture-before-approve.
Third round of the multi-artist report. The earlier fixes (Deezer contributors
upgrade, _artists_list, feat_in_title/artist_separator) were all in place and
correct — but gated on source == 'deezer', and on the real Search → Download
Now path NOTHING carried the source: core/search/sources.py serialized tracks
with no source field, search.js's enrichedTrack didn't add one, so
get_import_source() resolved '' and the whole Deezer-specific block silently
skipped. Files were tagged with only the primary artist until a Retag (which
rebuilds context with the source set — exactly why retagging always fixed it).
The earlier tests passed because they set context['source'] directly — the one
field the real flow never had (same mock-drift as the #823 append tests).
Reproduced with Netti93's exact track (deezer 3966840171) through the real
extract_source_metadata: before — source '', artists ['August Burns Red'];
after — source 'deezer', contributors fetched, artists ['August Burns Red',
'Polaris'], title 'Sonic Salvation (feat. Polaris)' per feat_in_title.
Fix, three layers:
- core/search/sources.py: serialized tracks/albums/artists carry "source"
(the canonical name the orchestrator already passes; '' when unnamed).
- core/imports/context.py get_import_source: also reads '_source' from the
nested dicts (track_info/original_search/album/artist) — additionally fixes
the discography/wishlist flows, which always passed '_source' that nothing
read.
- search.js: enrichedTrack + the album-download path carry source through to
the download task.
Tests: real-payload staging-shaped contexts (source in track_info, '_source'
shape, and the pre-fix sourceless shape staying safe — mocked Deezer client),
serializer source-field tests, resolver fallback tests; exact-shape serializer
tests updated for the new key. 1977 import/metadata/search tests pass (the
only 2 failures are the known soundcloud ones).
CubeComming #804: importing Coldplay "Yellow" (the 269s Parachutes album track,
correctly tagged) was quarantined — "Duration mismatch: file is 269.2s, expected
266.0s (drift 3.2s > tolerance 3.0s)". The expected 266s came from a re-resolved
*single* edition, not the file's actual album. The duration-agreement integrity
check exists to catch truncated/wrong slskd TRANSFERS — but a manual import is
the user's own already-tagged file being sorted, so checking it against a
re-resolved release just manufactures false quarantines.
Fix: both manual-import paths (singles + album) now mark the context
is_local_import; the integrity check skips the duration-agreement leg for local
imports via expected_duration_for_check() (new pure helper). The size +
mutagen-parse legs still run, so genuinely broken files are still caught — only
the release-vs-file duration comparison is skipped, and only for manual imports.
slskd downloads are completely unaffected.
This does NOT change the deeper matching (file still groups under Singles vs the
Parachutes album — the #767 canonical-version family); it stops the false
quarantine so the file imports.
Tests: 4 on the helper (local skips, download keeps, zero/None/garbage, string
coercion) + updated the routes context assertion. 557 import/integrity tests pass.
CI failed all 7 requeue tests that passed locally. Root cause is a real
shipping bug, not test flake: config/settings.py's default template set
retry_next_candidate_on_mismatch: False ("Default off — opt-in") while the
monitor reads it with inline default True and the PR documents it as ON.
Outcome split the userbase: a FRESH install (or CI's clean runner) gets the
template key = retry engine silently OFF; an existing config.json lacks the
key = inline True wins = engine ON. Same code, opposite behavior, decided by
install age.
- template aligned to True (the documented + approved default; existing
installs already behave this way via the inline default)
- the requeue tests now pin the toggle ON via the wiring helper instead of
reading the runner's ambient config — CI's fresh defaults vs a dev's
lived-in config.json must never decide whether they pass. _patch_config
composes (it wraps the pinned get and falls through).
64 retry-engine tests pass; fresh-default simulation confirms the toggle
resolves True.
Some tracks don't exist on the sources in the wanted cut — every copy is, say,
the instrumental. The retry engine correctly rejects each (version mismatch) and
gives up, leaving the track missing. New opt-in fallback: once a track's AcoustID
retries are fully exhausted, if every quarantined candidate for it failed the
SAME version mismatch (same matched version, e.g. all instrumental) and there are
>= N of them, accept the best (first-tried = oldest = highest-confidence) one.
Safety rules (core/imports/version_mismatch_fallback.py):
- Version mismatches only. Audio/artist mismatches (different recording) and
integrity/duration failures (truncated/wrong file) never participate.
- All qualifying entries must share the same matched version; a mix
(instrumental + live) is ambiguous → no acceptance.
- Re-import bypasses ONLY the AcoustID gate; integrity/duration/bit-depth still
run, so a truncated or genuinely wrong file is never let through here.
- Reuses the existing quarantine approve_quarantine_entry + re-verify dispatch.
Wired at the AcoustID give-up point in the verification wrapper. Two new
post_processing settings surfaced in the Retry Logic tile (default off):
accept_version_mismatch_fallback + version_mismatch_min_count.
Pure decision core + orchestration covered by tests (11). Acceptance logged at
WARNING with track + matched version.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Each AcoustID/integrity quarantine retry re-ran the FULL search (all queries,
all sources) before picking the next-best candidate — so a track that failed
verification a dozen times re-queried Soulseek a dozen times (~3 min/cycle in
the field). The next-best pick was already sitting in cached_candidates.
Now the monitor flags the re-queue as a quarantine retry; the worker walks the
already-found candidates first (skipping used + budget-exhausted sources) and
hands them straight to the download path — no search. A source is searched
exactly once: once its candidates are cached, later quarantine retries exclude
it (searched_sources) so the hybrid chain falls through to a not-yet-searched
source instead of re-querying the spent one. Fresh downloads and the monitor's
dead-connection/stuck retries clear searched_sources and search fresh, so the
only re-search is for a genuinely new source or a dead peer.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
In exhaustive retry mode, a source that spent its whole per-source budget
(query_count × retries_per_query) gave up and failed the track outright —
never trying the other configured sources. For tracks where Soulseek has a
deep pool of wrong peers (e.g. an AcoustID title mismatch every copy shares),
the budget tripped long before HiFi/Tidal/… were ever reached.
Now, when a source's budget is spent, the monitor marks it exhausted on the
task and re-queues so the worker excludes it from the next hybrid search,
falling through to the next source in the chain. Each new source spends its
own fresh budget. The task only fails once no fallback source remains (or the
absolute total ceiling trips) — single-source mode still fails immediately,
since there's nothing to fall back to.
task_worker folds the exhausted-source set into both the orchestrator search
exclusion and the hybrid-fallback source list.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Adds an opt-in exhaustive mode to the quarantine-retry path. Default
behaviour is unchanged: a single global cap (MAX_QUARANTINE_RETRIES=5).
When post_processing.retry_exhaustive is on, each source gets its OWN
retry budget sized as query_count x retries_per_query. Soulseek peers
collapse to one 'soulseek' bucket; streaming plugins keep their name.
The worker now records query_count on the task; the budget scales with
the track's real query count. Loop protection is threefold: per-source
cap, used_sources exhaustion (the natural terminator), and an absolute
ceiling (MAX_TOTAL_QUARANTINE_RETRIES=100).
New settings (config + WebUI): retry_next_candidate_on_mismatch (master),
retry_exhaustive, retries_per_query (default 5).
Tests: 6 new cases covering per-source budgeting, source separation,
Soulseek-peer bucketing, query_count default, and the absolute ceiling.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
When a downloaded file is quarantined because AcoustID verification or the
integrity/duration check fails, the task no longer dead-ends as failed — it
re-runs the worker on the next-best candidate, skipping the quarantined source.
Reuses the monitor's existing transfer-error retry machinery (used_sources +
cached_candidates + worker re-dispatch), just triggered from the post-process
verification wrapper's two quarantine branches instead of only on transfer
errors. Universal across sources (Soulseek, HiFi, Tidal, etc.) since all
batch/sync downloads funnel through post_process_matched_download_with_verification.
- monitor.requeue_quarantined_task_for_retry(): marks bad source used, resets
task to searching, resubmits worker. Guards: manual picks, cancelled tasks,
missing source id, and a MAX_QUARANTINE_RETRIES=5 loop cap.
- Opt-out via post_processing.retry_next_candidate_on_mismatch (default on).
- Manual quarantine approve is unaffected (_skip_quarantine_check='all' bypasses
the checks, so no quarantine flag, so no retry).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The old per-download Retag Tool was limited (only native-pipeline downloads,
100-group cap, manual per-group) and did the wrong thing — it moved/reorganized
files instead of just tagging. It's superseded by the new Library Re-tag job
(whole-library, in-place) + the enhanced-library 'Write Tags' button.
Removed: the post-download record_retag_download ingestion hook (stops writing
retag_groups on every download), core/library/retag.py, the web_server state +
deps + /api/retag/* endpoints + the tool:retag WebSocket emit, the dashboard
card + both modals (index.html), the core.js socket handler, and the tools-page
wiring + help entry (wishlist-tools.js). Updated the import-pipeline test.
Verified: web_server parses, app + core imports OK, 392 tests pass, no live
references to removed symbols.
Left as inert (harmless) for a careful follow-up sweep: the retag_groups/
retag_tracks tables + their DB CRUD methods (no longer written/read), and the
now-orphaned retag JS helper functions (no entry point/wiring/socket calls them;
interspersed with wishlist functions, so not blind-deleted).
Persist organize_by_playlist on mirrored playlists and run playlist-folder
downloads from the auto-sync pipeline instead of the global wishlist phase.
Register SoulSync library rows after playlist-folder post-processing, route
failed organize batches to the wishlist correctly, and skip sync-time
unmatched wishlist only when organize download handles retries.
Invalidate stale playlist track caches on refresh (Spotify and Deezer ARL),
re-mirror on refetch, and improve standalone playlist modals (re-analysis,
Open in Mirrored). Add filesystem missing-track detection and tests.
Co-authored-by: Cursor <cursoragent@cursor.com>
The reorganize preview (dry run) was physically creating destination album
folders, littering the library with empty dirs and making "changes" before the
user ever hit Apply.
Cause: preview_album_reorganize calls build_final_path_for_track purely to
COMPUTE the destination path string — but that shared helper has 9 os.makedirs
side effects (it's also the live download/import path builder, where creating
the dir is correct). So computing the preview path created "Lenka (Expanded
Edition)/" on disk.
Fix: build_final_path_for_track gains create_dirs=True; all 9 makedirs now route
through a gated helper. The reorganize PREVIEW passes create_dirs=False, so a
dry run computes the exact destination path with zero filesystem side effects.
Everything else keeps the default True:
- the download/import post-process flow (still writes files into the dir),
- retag,
- the reorganize APPLY path — verified it goes through post_process_fn (the real
pipeline → build_final_path_for_track with create_dirs=True), so live moves
still create their destination dirs. The gate only silences the dry run.
Tests: tests/imports/test_import_paths.py — create_dirs=False computes the
correct path (matching the reported "01 - The Show.flac") but writes NOTHING to
disk (not even the Transfer root); create_dirs=True still creates folders; both
yield an identical path. Updated two reorganize-orchestrator test doubles to
accept the new kwarg. 148 reorganize/paths/retag/pipeline tests pass.
Does NOT fix the second half of #767 (Expanded Edition picked over the standard
album). That is NOT a reorganizer bug: the library album row was linked to the
deluxe release at enrichment time (its stored spotify_album_id/itunes_album_id/
deezer_id points at "Lenka (Expanded Edition)"), and the reorganizer faithfully
reorganizes to whatever the album is linked to. The real fix is in album
enrichment's edition preference — tracked separately.
The manual-import routes (album + singles) call post_process_matched_download
directly. When the pipeline quarantines a file — integrity / AcoustID / FLAC
bit-depth — or hits the race guard, it sets a context flag and RETURNS
NORMALLY (it only marks the task failed + notifies when there's a task_id,
which manual imports don't have). So the inner pipeline raised no exception,
and routes.py counted `processed += 1` for a file that had just been moved to
ss_quarantine, not the library. Result: the UI shows a green "Done" while the
track silently vanished — exactly the #764 report (Coldplay - Yellow.flac ->
ss_quarantine, but "Done").
The download path already handles this in
post_process_matched_download_with_verification (it reads the same flags and
marks the task failed); only the manual-import routes were missing the check.
Fix: new pure helper import_rejection_reason(context) returns a human-readable
reason for any terminal rejection (_integrity_failure_msg / _acoustid_quarantined
/ _bitdepth_rejected / _race_guard_failed) or None for a clean import. Both
manual-import routes now consult it: album_process reports the track in
`errors` instead of counting it processed; process_single_import_file returns
("error", reason) instead of ("ok", ...). Verified every move_to_quarantine
call site (4, all in pipeline.py) sets one of those flags, so no quarantine
path slips through. This also delivers the "direct display of the error" the
reporter asked for — the reason now surfaces in the response `errors` list.
Does NOT address the reverse symptom ("failed even though it moved correctly")
— not yet root-caused — nor the separate bit-depth hole on the download-path
wrapper.
Tests: tests/imports/test_import_rejection_reason.py (10) — each trigger
detected, falsy flags ignored, deterministic ordering, plus two route-level
tests driving the REAL process_single_import_file (quarantine -> "error";
clean -> "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).
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.
Real-world regression triggered by the album-bundle work earlier in
2.6.3. Tracks with full Spotify metadata were importing as
``01 - <title>`` under ``Artist - Album/`` (no year), even when the
source filename carried the correct track number and Spotify's
release_date was available.
Investigation via DB inspection of stored wishlist rows:
```
"Never Gonna Give You Up" → track_number=None, release_date=""
"idfc" → track_number=1, release_date=""
"No Sleep Till Brooklyn" → track_number=1, release_date=""
```
Source-of-truth Spotify metadata had release_date AND real track
positions, but the wishlist row was poisoned. Three regressions
compounded the loss:
**Fix A — ``track_object_to_dict`` (``core/wishlist/payloads.py:295``)
preserved only album.name during Track→dict conversion.**
Pre-fix:
```python
album_name = "Unknown Album"
if hasattr(track_object, "album") and track_object.album:
if hasattr(track_object.album, "name"):
album_name = track_object.album.name
else:
album_name = str(track_object.album)
result = {
...
"album": {"name": album_name}, # ← release_date / images / etc. all dropped
...
}
```
When a wishlist payload arrived as a Track dataclass instead of a
raw spotify_data dict, the Track→dict conversion stripped
release_date, images, album_type, total_tracks, id, and album-level
artists. Every wishlist row added through this path landed in the
DB with ``album={'name': X}`` only.
Post-fix: three branches handle the three album shapes
- ``album_attr`` is a dict → ``dict(album_attr)`` preserves every key
- ``album_attr`` is a sub-object → pull all common Album-dataclass
attrs (id, release_date, album_type, total_tracks, images, ...)
- ``album_attr`` is a bare string → build a dict from the track
object's adjacent attrs (release_date, album_id, album_type, ...)
and surface ``image_url`` as ``album.images``
**Fix B — ``core/discovery/playlist.py:309`` only added
``track_number`` / ``disc_number`` keys when truthy.**
Pre-fix:
```python
matched_data = { 'id': ..., 'name': ..., ... } # no track_number / disc_number
if track_number:
matched_data['track_number'] = track_number
if disc_number:
matched_data['disc_number'] = disc_number
```
Deezer-sourced matches always hit this branch with ``track_number=None``
because the cache enrichment at line 304 reads ``_raw.get('track_number')``
literally, but Deezer's raw shape uses ``track_position``. So the key
was omitted from ``matched_data``, downstream consumers couldn't
distinguish "missing key" from "value is 1", and the chain silently
filled 1.
Post-fix: keys are ALWAYS present (None when unknown). Also adds a
``best_match.track_number`` fallback so the Track-dataclass-mapped
value (which DOES include ``track_position``→``track_number``
mapping) gets used when the cache lookup misses.
**Fix C — Pipeline only consulted ``album_info.track_number`` before
falling to the filename (``core/imports/pipeline.py:645``).**
VA-collection source files like ``417 Fountains of Wayne - Stacys
Mom.flac`` have a leading playlist-position number that isn't the
album track number. The previous chain (album_info → filename →
floor-1) couldn't recover the real position because the filename
extractor either returned 417 (wrong) or None (caught by the floor).
But the wishlist payload's ``track_info.spotify_data.track_number``
HAD the right answer all along — Spotify says Stacy's Mom is track
3 on Welcome Interstate Managers.
Post-fix: resolution chain extracted into ``core/imports/track_number.py:resolve_track_number``
as a pure function:
1. ``album_info.track_number`` (album-bundle dispatch authoritative)
2. ``track_info.track_number`` (per-track flow payload)
3. ``track_info.spotify_data.track_number`` (nested fallback)
4. ``extract_explicit_track_number(file_path)`` (filename, returns
0 when no numeric prefix — vs the default helper that returns 1)
5. Caller (pipeline) applies the final >=1 floor
Each step coerces to a positive int or falls through to the next.
Pure function = unit-testable in isolation = single place to fix
the rule.
**Test coverage (37 new tests):**
- ``tests/wishlist/test_payloads.py`` (+4) — Track→dict conversion
preserves full album dict (dict / object / string album shapes) +
None-track-number stays None.
- ``tests/discovery/test_discovery_playlist.py`` (+2) — matched_data
always includes track_number/disc_number keys (None when unknown)
+ falls back to best_match attrs when cache misses.
- ``tests/imports/test_track_number_resolver.py`` (+16) — every
resolution-chain branch pinned: album_info-wins, track_info
fallback, spotify_data nested, JSON-string parsing, garbage-string
fall-through, zero / negative / non-numeric / string-numeric
coercion, filename fallback, explicit extractor vs default
extractor semantics, defensive None inputs, VA-collection
filename behaviour, all-sources-missing → None.
1571 wider-suite tests pass (wishlist + imports + discovery +
downloads + metadata). Ruff clean.
**Migration note:** existing wishlist rows that were saved under
the OLD ``track_object_to_dict`` (with stripped album metadata) still
have ``release_date=''`` in the DB blob. Those won't self-heal — the
next attempt loads from the poisoned blob. Users can remove + re-add
those tracks to refresh, or wait for the next sync run that
re-discovers them with full metadata. No automatic migration shipped
in this PR (scope creep — the forward path is fixed, backfill is a
separate concern).
Soulseek album-bundle (and any other release-staging path) was
importing every file with ``track_number=1`` because the staging
metadata reader used the auto-import-flavor filename extractor:
``extract_track_number_from_filename`` returns 1 when the basename
has no ``NN -`` prefix. That's the right default for the loose
auto-import flow (single file in, no upstream metadata to lean
on), but completely wrong for staging-cache reads:
- For an album-bundle download the user has authoritative track
numbers in the Spotify track list flowing through to
``track_info`` for each task.
- ``try_staging_match`` in ``core/downloads/staging.py`` was
meant to use those numbers when the staged file's own metadata
doesn't have them.
- But the staging cache populated ``track_number=1`` for every
untagged bare-title file (e.g. ``Cha-La Head-Cha-La.flac``), the
album-bundle resolution branch reads file-side first, sees 1,
and short-circuits the rest of the chain.
Fix:
- New ``extract_explicit_track_number`` in
``core/imports/filename.py`` — strict variant that returns
``0`` when no numeric prefix is visible. Docstring explicitly
contrasts with the legacy 1-defaulting helper so future
callers pick the right one.
- ``read_staging_file_metadata`` in ``core/imports/staging.py``
now uses the strict extractor, so the staging file dict
carries ``track_number=0`` ("unknown") instead of ``1`` for
untagged bare-title files.
- The legacy ``extract_track_number_from_filename`` keeps its
1-default behavior so auto-import callers + the post-process
template fallbacks are unchanged; it's now implemented in
terms of the strict variant.
- Tag-side parsing also tightened to require ``> 0`` before
overriding the filename-derived value.
3 new tests pin the contracts:
- ``test_extract_explicit_track_number_returns_zero_when_no_prefix``
- ``test_read_staging_file_metadata_returns_zero_track_when_unknown``
- existing ``test_extract_track_number_from_filename_handles_common_patterns``
now explicitly comments why bare filenames keep returning 1.
758 tests across imports + downloads + repair + staging-provenance
suites green. WHATS_NEW entry added under 2.6.3.
Reported against an album-bundle download of Ryoto's
"Cha-La Head-Cha-La" where slskd staged 15 untagged FLAC files
named after the song titles only.
Expand matched MusicBrainz release groups into concrete releases for specific album searches so import users can choose the correct edition by track count, format, country, and disambiguation. Preserve distinct MusicBrainz release IDs instead of deduping same-title variants, carry release metadata through import matching, and surface those details on album result cards. Add coverage for variant preservation and release-group expansion.
Import album search silently fell through to the next source in
METADATA_SOURCE_PRIORITY when the configured primary returned zero
matches — intentional behavior shared with the auto-import worker
(see core/auto_import_worker.py:1316). With MusicBrainz selected and
a query MB couldn't resolve, users saw Deezer cards with no indication
their primary was bypassed.
Backend now echoes `primary_source` on /api/import/search/albums,
/api/import/search/tracks, and /api/import/staging/suggestions.
Frontend renders a per-card 'via {source}' badge when the served
source differs from the primary, plus a banner above the grid when
every card came from a fallback source. Fallback semantics unchanged.
Also collapses an inline duplicate of _renderSuggestionCard inside
importPageSearchAlbum into a single shared renderer.
Regression test pins the contract: response carries primary_source +
per-album source when the chain falls back.
Adds torrent/usenet as release-oriented download sources with album-bundle staging, live progress reporting, and post-processing that selects the requested audio file from completed releases instead of blindly importing the first file.
Keeps album-bundle behavior gated to single-source torrent/usenet album downloads, excludes release sources from hybrid album per-track searches, and allows hybrid non-album tracks to use release results safely.
Improves staged-release matching for featured/bonus track filenames while preserving version mismatches, records torrent/usenet provenance in library history, and updates service/status UI labels.
Covers the flow with focused lifecycle, status, staging, validation, task worker, post-processing, and import side-effect tests.
When a file failed AcoustID verification and got quarantined, the next
auto-wishlist cycle would search for the same track, the deterministic
quality picker would re-select the same (uploader, filename) source,
re-download it, and re-quarantine it. Users woke up to hundreds of
duplicate .quarantined entries from a single bad upload — same source
URL repeatedly, byte-for-byte identical files.
Root cause: `SoulseekClient.filter_results_by_quality_preference` ranks
candidates by quality + bitrate density only. Quarantine history wasn't
consulted, so a high-bitrate FLAC upload with a wrong-track AcoustID
fingerprint kept winning the picker against every other candidate.
Fix shape:
- New helper `core/imports/quarantine.py::get_quarantined_source_keys`
reads every quarantine sidecar's `context.original_search_result`
and returns the set of `(username, filename)` tuples for O(1)
membership checks. Sidecars missing the context field (legacy thin
sidecars written pre-Feb 2026, or orphaned files) and corrupt JSON
are skipped silently — defensive against transient FS / encoding
issues.
- `SoulseekClient._drop_quarantined_sources` runs the membership
filter against incoming TrackResults, drops matches, logs a single
INFO line with the skip count. Called first inside
`filter_results_by_quality_preference` so all four callers
(search-and-download, master worker, validation, orchestrator)
benefit transparently.
- Approving or deleting a quarantine entry removes its sidecar, so
the dedup key disappears from the set on the next search — gives
the user a way to opt back in to a previously-quarantined source
without restarting the app.
7 helper tests cover: missing dir, empty dir, well-formed sidecars
collected as tuples, legacy sidecars skipped, empty source fields
skipped (so empty-string keys can't accidentally drop unrelated
results), corrupt JSON tolerated, duplicate quarantines collapse.
5 integration tests pin: clean candidates pass, known-bad candidates
drop, missing quarantine dir returns input unchanged, filesystem
errors swallowed (defensive), full `filter_results_by_quality_preference`
runs the dedup BEFORE the quality picker — so a high-quality
quarantined source can't win on bitrate.
692 existing download + import tests still green. Cosmetic surface
of the fix is invisible — same UX as today when no quarantine entries
exist; loop only kicks in once a sidecar has been written.
Out of scope: bulk-select / multi-delete UI for the quarantine tab —
S-Bryce mentioned this as a separate pain point in the issue, but
it's its own UX work, not a one-commit drive-by.
Closes#584. Quarantined files used to sit in ss_quarantine/ with a
thin sidecar — no UI, no recovery, no way to see what got dropped.
This adds the management surface the user needs without going to the
filesystem.
UI: new "Quarantine" button on the downloads page header opens a
modal with every quarantined file (filename, expected track/artist,
reason, when, size). Three actions per row:
- Approve (one-click): restores the file, re-runs the post-process
pipeline with ONLY the failing check skipped, lands in the library
with full tags + lyrics + scan
- Recover (legacy fallback): moves to Staging for thin-sidecar
entries that lack the embedded context Approve needs
- Delete: permanent removal of file + sidecar
Per-check bypass: context['_skip_quarantine_check'] = 'integrity' /
'acoustid' / 'bit_depth'. Skips ONLY the named check — other quality
gates stay live. No blanket bypass-all flag.
Sidecar expansion: move_to_quarantine now persists the full
json-serializable context via serialize_quarantine_context (drops
non-JSON-safe values, walks nested dicts/lists/sets, str-coerces
unknown objects) plus the trigger name. Existing thin sidecars are
detected and routed to Recover instead of Approve.
Pure helpers in core/imports/quarantine.py: list_quarantine_entries
/ delete_quarantine_entry / approve_quarantine_entry /
recover_to_staging / serialize_quarantine_context. 27 tests pin
every shape: orphan files / orphan sidecars / corrupt sidecars /
collision-safe filename restoration / full-context vs thin-sidecar
dispatch / json round-trip safety.
Four new endpoints in web_server.py — thin glue around the helpers:
GET /api/quarantine/list, DELETE /api/quarantine/<id>,
POST /api/quarantine/<id>/approve, POST /api/quarantine/<id>/recover.
Download modal status differentiates "🛡️ Quarantined" from
"❌ Failed" so recoverable files are visible at a glance — checked
against the error_message text, no schema change needed.
Pipeline changes are three minimal per-check conditionals at the
existing quarantine sites in core/imports/pipeline.py. Each
move_to_quarantine call now passes its trigger name so the sidecar
records which check fired.
Full suite: 2992 passed.
Previously hardcoded at 3s (5s for tracks >10min) — files drifting
past that got quarantined with no user override. Live recordings,
alternate masterings, and some legitimate uploads routinely drift
further.
New setting `post_processing.duration_tolerance_seconds`. Default 0
means "use auto-scaled defaults" (unchanged behavior for users who
don't touch it). Positive value overrides the per-track defaults.
Capped at 60s — past that the check is effectively off.
Logic lifted to pure helper `resolve_duration_tolerance` in
file_integrity.py. Coerces every plausible input (None / empty /
zero / negative / unparseable / above-cap / numeric string / float)
to either a float override or None for auto. 12 tests pin every
shape.
Wired into `core/imports/pipeline.py` at the integrity-check call
site — runs for ALL matched downloads (Soulseek / Tidal / Qobuz /
HiFi / YouTube / Deezer-direct) since they all share that pipeline.
Settings UI input under Settings → Metadata → Post-Processing.
Move the remaining manual import endpoint logic out of web_server.py and into core.imports.routes behind ImportRouteRuntime. The Flask endpoints now stay as thin compatibility wrappers for album/track search, album match/process, single-file import processing, and batched singles processing.
Keep legacy test patch points intact by re-exporting build_album_import_match_payload from web_server and routing singles_process through an injected process_single_import_file callable. This preserves existing route-level monkeypatch behavior while keeping the extracted helper testable.
Add focused helper coverage for Hydrabase enqueueing, search limit clamping, album match payload forwarding, album import side effects, single-file worker outcomes, malformed manual matches, and singles aggregation/injected-worker behavior.
Verification: py_compile and git diff --check passed locally; bundled-Python smoke covered the extracted helpers. Claude reran the project tests and reported all tests passing.
Move import staging files/groups/hints/suggestions controller logic out of web_server.py and into core.imports.routes behind an ImportRouteRuntime dependency object. Keep the existing Flask routes as thin compatibility wrappers so the UI endpoint surface stays unchanged.
Add focused tests for staging file filtering, album grouping, hint generation, cached suggestions, empty missing staging paths, and error payloads from failed path/metadata reads.
Verification: py_compile passed for web_server.py, core/imports/routes.py, and tests/imports/test_import_routes.py. A bundled-Python smoke pass covered the extracted helper behavior; pytest was not available in this Windows shell because the bundled Python lacks pytest and the repo venv is WSL/Linux-only here.
Reported: Clear History button on the Import page left zombie rows
behind. Every survivor showed "⧗ Processing" status from 2-9 days ago.
Trace: `_record_in_progress` inserts a `status='processing'` row up-front
so the UI can render the in-flight import while it runs; `_finalize_result`
updates it to `completed`/`failed` when the import finishes. When the
worker is killed mid-import (server restart, crash), the row never gets
finalized — stays at `processing` forever. The clear-history endpoint's
SQL `DELETE ... WHERE status IN (...)` listed every terminal status but
omitted `processing`, so zombies survived every click.
Fix: add `processing` to the delete list, but guard against nuking
genuinely-live imports by intersecting against the worker's
`_snapshot_active()` map — any folder hash currently registered in
`_active_imports` is excluded from the delete via an `AND folder_hash
NOT IN (...)` clause. `pending_review` deliberately left out so user
still has to approve/reject those explicitly.
One endpoint touched (`/api/auto-import/clear-completed` in
web_server.py). No worker changes — guard reuses the existing
`_snapshot_active()` method that the UI poller already calls.
5 new tests in `tests/imports/test_auto_import_clear_completed_endpoint.py`:
- Zombie `processing` rows swept, live `processing` row preserved
(folder_hash currently in `_active_imports` survives)
- Response count matches actual delete count
- Empty active-set branch (unparameterized DELETE) — pinned because
an empty SQL `IN ()` would be a syntax error
- Worker-unavailable returns 500 (pre-existing guard not regressed)
- `pending_review` rows always survive — never auto-swept
Full pytest sweep: 2758 passed (one pre-existing flaky timing test
on `test_import_singles_parallel.py` failed under full-suite CPU load,
passes in isolation in 2.95s — unrelated to this change).
Discord report: 16 Bandcamp indie albums sat in staging because
auto-import couldn't identify them, but the manual search bar at the
bottom of the Import Music tab found the same albums fine. Trace:
`_search_metadata_source` only queried `get_primary_source()` — single
source, no fallback. Meanwhile `search_import_albums` (manual search bar)
already iterated `get_source_priority(get_primary_source())` and broke
on the first source with results. Asymmetric behavior, same album: manual
worked, auto-import didn't.
Fix: lift `_search_metadata_source` to use the same source-chain pattern.
Try primary first; if it returns nothing OR scores below the 0.4
threshold, fall through to the next source in priority order. First
source producing a strong-enough match wins. Result dict carries the
`source` that actually matched (not the primary name) so downstream
`_match_tracks` calls the right client. Defensive per-source try/except
so a rate-limited or auth-failed source doesn't abort the chain.
Unconfigured sources (client=None) silently skipped.
Cin-shape lift: scoring math extracted to pure `_score_album_search_result`
helper so the weight tweaks (album 50% / artist 20% / track-count 30%)
are pinned at the function boundary, independent of the orchestrator
(per-source iteration, exception containment, threshold check). Weight
constants exposed at module level (`_ALBUM_NAME_WEIGHT`,
`_ARTIST_NAME_WEIGHT`, `_TRACK_COUNT_WEIGHT`) — greppable, bumpable in
one place. Pre-extraction these were magic numbers inline.
27 new tests:
- 9 integration tests in `test_auto_import_multi_source_fallback.py`:
primary-success path unchanged (no fallback fires, only primary client
called), primary-empty falls through, primary-weak-score falls through,
first fallback success stops the chain (no wasted API calls on
remaining sources), all-sources-fail returns None, per-source
exception contained, unconfigured-source skipped, result `source`
field reflects winning source, `identification_confidence` from
winning source.
- 18 helper tests in `test_album_search_scoring.py`: weights sum to
1.0, album weight dominant (invariant pin), perfect-match returns 1.0,
per-component contribution (album / artist / track-count), Bandcamp
vs streaming track-count mismatch (7-files vs 4-tracks case still
scores ~0.87 above threshold), zero-track-count and zero-file
guards, huge-mismatch non-negative guard, list-of-strings artist
shape, missing `.name` / `.artists` / `None` total_tracks edge cases.
Backwards compatible: single-source users see no change — chain just
has one entry. Existing test `test_search_metadata_source_extracts_artist_id_from_dict_artist`
needed one extra patch line for `get_source_priority`.
Full pytest sweep: 2754 passed.
Live-API verification revealed advanced-syntax queries hurt more
than they help on this endpoint. Switching the import-modal Deezer
search back to free-text + local rerank.
# What live testing showed
Hit Deezer's public API with both query forms for the issue #534
case (`Dirty White Boy` + `Foreigner`):
**Free-text (`q=Dirty White Boy Foreigner`):**
- Returns 21 results
- Real Foreigner Head Games studio cut at #1
- Live versions at #2-10
- Karaoke / cover variants at #11-15
**Advanced (`q=track:"Dirty White Boy" artist:"Foreigner"`):**
- Returns 12 results
- "(2008 Remaster)" at #1 — canonical Head Games cut MISSING from
top 8 entirely
- Live + alt-album versions follow
Advanced syntax DOES filter karaoke at the API level (none in the
12-result set vs. 5 at positions 11-15 in free-text), but it has
its own ranking bias that surfaces remasters / "Best Of" cuts
ahead of the canonical recording. Net regression for the user-
facing goal.
# Fix
1. Endpoint reverts to free-text query with local rerank applied.
2. Local rerank gains "remaster" / "remastered" / "reissue"
patterns under VARIANT_TAG_PATTERNS (soft 0.4× penalty — user
may want them but they shouldn't outrank the original).
3. Client kwarg support (`track=` / `artist=` / `album=`) preserved
for future opt-in callers (e.g. exact-match flows where API-
level filtering matters more than ranking).
# Verified end-to-end against live Deezer API
Re-ran the exact #534 case through the live API + new rerank.
Top 15 results post-rerank:
1. Dirty White Boy — Foreigner — Head Games ← REAL CUT AT TOP
2-10. Various Live versions
11-15. Karaoke / cover / tribute variants ← BURIED
Real Foreigner Head Games studio cut at #1, exactly the user's
ask.
# Tests
- `test_relevance.py` — variant tag patterns extended; existing
tests still pass (50 tests).
- `test_search_match_endpoints.py::test_joins_track_and_artist_into_free_text_query`
— replaces `test_passes_track_and_artist_as_kwargs`; verifies
endpoint sends free-text join, NOT field-scoped kwargs (the
prior test asserted the wrong direction now).
- Karaoke-burying assertion at the endpoint still pins the
user-visible behaviour.
- Client kwarg path tests untouched (still pin advanced-syntax
construction for future opt-in callers).
# Verification
- 75 relevance + endpoint + query tests pass
- 2445 full suite passes
- Ruff clean
- Live Deezer API shows real cut at #1 post-rerank
# Background
User reported (#534) that the import-modal "Search for Match" dialog
returned irrelevant results when Deezer was the metadata source.
Searching `Dirty White Boy` + `Foreigner` returned 5+ karaoke /
"originally performed by" / "in the style of" / "re-recorded" /
tribute-band results ranked above the actual Foreigner studio cut
from Head Games. User had to scroll past the junk every time, or
fall back to iTunes search which is much slower.
# Root cause — two layers
1. **Endpoint joined `track + artist` into free-text query.**
`/api/deezer/search_tracks` was passing `q=Dirty White Boy Foreigner`
to Deezer's `/search/track` API. Deezer fuzzy-matches that
string across title / lyrics / artist / album / contributors and
orders by global popularity — anything that appears across many
compilations outranks the canonical recording.
2. **No local rerank.** None of the search-modal endpoints applied
any post-filtering. Deezer's API order shipped straight to the
user.
# Fix — same architectural shape Cin would build
## Layer 1: field-scoped query at the client boundary
`core/deezer_client.py::search_tracks()` now accepts optional
`track`, `artist`, `album` kwargs. When provided, builds Deezer's
advanced search syntax: `q=track:"X" artist:"Y" album:"Z"`. Massive
relevance improvement because each term matches the right field
instead of fuzzy-matching everywhere.
Backward compat preserved: legacy free-text `query=` callers still
work unchanged. Field-scoped path takes precedence when both are
provided. Empty input fast-fails without an API call. Embedded
double-quotes stripped (Deezer's syntax has no escape mechanism).
## Layer 2: provider-neutral relevance reranker
New `core/metadata/relevance.py` module — pure-function rerank over
the canonical `Track` dataclass. Composable scoring:
- **Cover/karaoke patterns** (multiplier 0.05, effectively buries):
matches "karaoke", "originally performed by", "in the style of",
"made famous by", "tribute", "vocal version", "backing track",
"cover version", "re-recorded", "cover by", etc. across title,
album, AND artist fields. Catches the screenshot's exact junk:
artist credits like "Pop Music Workshop" / "The Karaoke Channel"
/ "Foreigner Tribute Band".
- **Variant tags** (multiplier 0.4): live / acoustic / demo /
instrumental / remix / radio edit / club mix etc. — softer
penalty since the user MAY want them. Skipped entirely when the
expected_title contains the same tag (so searching
"Track (Live)" still ranks Live versions first).
- **Exact artist boost** (multiplier 1.5): primary artist exactly
matches expected_artist after normalisation. Single strongest
signal for "this is the canonical recording".
- **Title + artist similarity** via SequenceMatcher (parentheticals
+ punctuation stripped before comparison).
- **Album-type weighting**: album=1.0 > single/ep=0.85 > compilation=0.7.
Compilations are more likely tribute / karaoke repackages.
Each component is a standalone function so tests pin them
individually without standing up the full pipeline.
## Wired at three search-modal endpoints
- `/api/deezer/search_tracks` — uses both layers (field-scoped
query + rerank).
- `/api/itunes/search_tracks` — uses rerank only (iTunes API has
no advanced-syntax search, but karaoke / cover variants still
leak through and need the local penalty).
- `/api/spotify/search_tracks` — already builds field-scoped
`track:X artist:Y` query; rerank added as the consistency safety
net so all three sources behave the same from the user's
perspective.
Other Deezer call sites (matching engine, watchlist scanner,
auto-import single-track ID) deliberately not touched in this PR
— they have their own elaborate scoring pipelines tuned to their
specific contexts and aren't surfacing the user-reported issue.
Per Cin: "don't refactor beyond what the task requires."
# Tests
71 new tests across 3 files:
- `tests/metadata/test_relevance.py` (50 tests) — every scoring
component pinned individually + the issue #534 screenshot
reproduced as a regression test (real Foreigner cut wins after
rerank, karaoke variants drop to bottom).
- `tests/metadata/test_deezer_search_query.py` (14 tests) —
advanced-syntax query construction, field-scoped wiring at the
client boundary, free-text path unchanged, kwargs win when
ambiguous, limit clamping, cache key consistency.
- `tests/imports/test_search_match_endpoints.py` (7 tests) —
end-to-end through Flask test client: Deezer endpoint passes
kwargs not joined query; karaoke buried at bottom for all three
sources; legacy query param still works without rerank.
# Verification
- 2441 full suite passes (+71 from baseline 2370)
- 0 failures (the prior watchdog flake fix held)
- Ruff clean across all changed files
- JS parses clean (`node -c webui/static/helper.js`)
# Architectural standards followed
- **Logic at the right boundary.** Query construction lives in the
client (every caller benefits from one change). Rerank lives in
a neutral module (`core/metadata/relevance.py`) over the
canonical `Track` dataclass — works for any source, not Deezer-
specific.
- **Explicit > implicit.** Every scoring rule has its own named
function. Pattern tables are module-level constants tests can
introspect.
- **Scope discipline.** Audited every Deezer search call site;
fixed the user-reported one + the consistent siblings. Did NOT
speculatively normalise every Deezer call across the codebase.
- **Backward compat.** Free-text `query=` callers untouched. Kwargs
added to existing client method signature with safe defaults.
- **Tests pin contract at correct boundary.** Pure-function rerank
tests don't mock anything; client-query tests stub at `_api_get`;
endpoint tests run through the real Flask app.
Two pre-existing parity gaps in `record_soulsync_library_entry` that
the prior parity commits left untouched. Both close real holes
between auto-import writes and what the soulsync_client deep scan
would have produced.
# Gap 1: Album duration was the first-imported track's duration
`record_soulsync_library_entry` is called once per track. The album
INSERT only fires for the FIRST track of a new album (subsequent
tracks find the album row already exists). The INSERT was passing
`duration_ms` — `track_info["duration_ms"]` — as the album's
`duration` column. That's the duration of one track, not the album
total. Compare to `SoulSyncAlbum.duration` in soulsync_client which
is `sum(t.duration for t in self._tracks)`.
Fix:
- Worker computes `album_total_duration_ms = sum(...)` across every
matched track and threads it onto context as
`album.duration_ms`.
- side_effects reads that value (or falls back to the per-track
duration for legacy non-auto-import callers) and writes it as the
album row's `duration`.
# Gap 2: Re-imports of the same artist/album were insert-only
When the SELECT-by-id or SELECT-by-name found an existing soulsync
artist or album row, the function skipped completely — no UPDATE
path. Meant: artist genres / thumb / source-id reflected ONLY
whatever the FIRST imported album supplied, never refreshing as
more albums by that artist landed. Ten more imports later, the
artist row still held whatever the first random import wrote.
Conservative fix: when an existing row matches, run an UPDATE that
fills only the columns whose current value is NULL or empty. Never
overwrites populated values — protects manual edits +
enrichment-worker writes the same way the scanner UPDATE path
preserves enrichment columns.
Implementation note: the empty-check happens in Python, NOT SQL.
Initial pass tried `COALESCE(NULLIF(col, ''), NULLIF(col, 0), ?)`
but SQLite's `NULLIF(text_col, 0)` returns the original text value
instead of NULL — different types, no coercion. So the SQL-only
conditional was unreliable on text columns. New helper does
`SELECT cols FROM table WHERE id`, compares each column in Python,
and emits UPDATE clauses only for the ones that need filling.
Allowlist defense: f-string column names go through
`_SOULSYNC_FILLABLE_COLUMNS` validation before interpolation.
Misuse adding new columns without an allowlist update fails closed
(logger.debug + skip).
# Tests added (4)
- `test_album_duration_uses_album_total_not_single_track` —
album with single-track context carrying explicit
`album.duration_ms = 2_500_000` writes 2_500_000 to the album row,
not the per-track 200_000 fallback.
- `test_re_import_fills_empty_artist_fields` — first import lands
artist with empty thumb + empty genres; second import for same
artist with thumb + genres present updates the existing row.
- `test_re_import_does_not_clobber_populated_artist_fields` —
first import writes rich genres + thumb; second import with
worse / different metadata leaves the existing row untouched.
- `test_re_import_fills_empty_source_id_when_missing` — first
import had no source artist ID; second import does — fills the
empty `spotify_artist_id` column on the existing row.
# Verification
- 10/10 side-effects tests pass (including 4 new + 4 from prior
parity commit + 2 history/provenance)
- 217 imports tests pass (no regression)
- 2369 full suite passes (+4 from prior, +22 PR-total from baseline 2347)
- 1 pre-existing flake (`test_watchdog_warns_about_stuck_workers`,
passes in isolation, unrelated)
- Ruff clean
Cin pre-review followup. Two small parity gaps the prior commits left
open:
# 1. Genre tags land on the standalone artists row
`soulsync_client._scan_transfer` aggregates the GENRE tag across every
track in an album and surfaces it on `SoulSyncAlbum.genres` (which the
DatabaseUpdateWorker writes to the artists+albums row). Auto-import
was hardcoding `'spotify_artist': {'genres': []}` so the imported
artists row landed with empty genres — felt hollow compared to a
Plex/Jellyfin scan, which both pull genres from their respective APIs.
Fix:
- `_read_file_tags` now reads the GENRE tag (mutagen easy mode handles
MP3/FLAC/M4A consistently; some files carry multiple genres so it's
always returned as a list).
- `_process_matches` aggregates genres from each matched file's tags
into a deduped insertion-order list. Dedup is case-insensitive but
preserves original casing — so "Hip-Hop, Rap, Trap" reads naturally
in the JSON column instead of "hip-hop, rap, trap".
- Worker context's `spotify_artist['genres']` carries the aggregated
list, which `record_soulsync_library_entry` already filters via
`core.genre_filter.filter_genres` and writes to the artists row.
# 2. Defensive str() cast for ISRC + MBID
`_build_album_track_entry` already coerces ISRC + MBID to string today
(via `str(isrc) if isrc else ''`). But if a future metadata-source
client returns int / None for either ID, the worker would propagate
the wrong type and side_effects.py's `.strip()` would AttributeError.
Cheap insurance: explicit `str()` cast in the worker before assignment
to track_info. Future-proofs against client drift.
# Tests added (3, in test_auto_import_context_shape.py):
- `test_context_aggregates_genres_from_track_tags` — multi-file
album with overlapping genre lists produces deduped, insertion-
ordered, original-case-preserved result. Stubs `_read_file_tags`
with monkeypatch so we don't need real audio.
- `test_context_genres_empty_when_no_tags` — files without GENRE
tag → empty list. Standalone library write handles gracefully
(genres column stays empty / NULL).
- `test_context_isrc_mbid_coerced_to_string` — hostile types
(int 12345678, None, int 999) coerced to safe strings before
reaching track_info.
# Verification
- 14/14 context-shape tests pass (11 prior + 3 new)
- 213 imports tests pass (no regression)
- 2365 full suite passes (+3 from prior, +18 PR-total)
- 1 pre-existing flake (`test_watchdog_warns_about_stuck_workers`,
passes in isolation)
- Ruff clean
Cin pre-review followup to the standalone library parity commit. The
prior commit fixed `spotify_artist['id']` from the wrong copy-paste
value (`identification['album_id']`) to read from
`identification['artist_id']`, but the identification dict produced
by `_search_metadata_source` and `_search_single_track` never set
`artist_id` — both extracted artist NAME from the search response
and discarded the source ID sitting right next to it. Net effect of
the prior commit: artists row source-id stayed NULL, just for a more
honest reason than before.
Now properly extracted:
- `_search_metadata_source` reads `best_result.artists[0]['id']`
alongside the artist name and returns it on the identification dict
as `artist_id`.
- `_search_single_track` does the same for single-track identification.
- `_identify_single`'s tag-based-confidence path forwards
`result.get('artist_id')` so the artist source-id propagates even
when high-confidence local tags override the search result's name.
Result: identification dict now carries `artist_id` whenever the
metadata source returned an artist with an ID. The worker context
already plumbs it onto `spotify_artist['id']` and
`spotify_album['artists'][0]['id']`, so the standalone library write
finally populates `<source>_artist_id` on the artists row.
Tests added (3, in `test_auto_import_context_shape.py`):
- `test_context_artist_id_uses_identification_artist_id` — when the
identification dict carries `artist_id`, context propagates it
onto `spotify_artist['id']` AND
`spotify_album['artists'][0]['id']`. Pins that the prior copy-
paste bug (artist['id'] = album_id) doesn't return.
- `test_context_artist_id_is_empty_when_identification_missing_it` —
fallback case (filename-only identification): context gets empty
string, NOT album_id. Honest failure mode.
- `test_search_metadata_source_extracts_artist_id_from_dict_artist`
— black-box test of `_search_metadata_source`: feed it a
spotify-shaped result with `artists[0]['id']` and verify
identification dict carries it forward.
Verification:
- 11/11 context-shape tests pass (8 prior + 3 new)
- 210 imports tests pass (no regression)
- 2362 full suite passes (+3 from prior commit, +15 PR-total)
- 1 pre-existing flake (`test_watchdog_warns_about_stuck_workers`,
passes in isolation)
- Ruff clean
# Background
SoulSync standalone is meant to be a full replacement for Plex /
Jellyfin / Navidrome — files imported via auto-import (or any other
import path) should land in the database with the same field richness
a media-server scan would write. They weren't.
# Gaps fixed
The auto-import worker built a context dict for each track and handed
it to `_post_process_matched_download` (the same callback the regular
download flow uses). That dict was missing three things downstream
needed:
1. **No `source` field anywhere.** `record_soulsync_library_entry`
reads `get_import_source(context)` to pick the source-aware ID
columns (`spotify_track_id` / `deezer_id` / `itunes_track_id` /
etc.) on the artists / albums / tracks rows. With no source, the
resolver returned an empty string → `get_library_source_id_columns("")`
returned an empty dict → the `UPDATE tracks SET <source>_id = ?`
blocks were silently skipped. Result: every auto-imported track
landed with NULL on every source-id column. Watchlist scans
(which match by stable source IDs to detect "this track is already
in library") couldn't recognise these rows and would re-download
them on the next pass.
2. **No `_download_username='auto_import'`.** Both
`record_library_history_download` and `record_download_provenance`
default to "Soulseek" when no `username` is in the context. Every
staging-folder import was being labelled as a Soulseek download
in library history + provenance — false signal in the UI.
3. **No per-recording IDs (`isrc`, `musicbrainz_recording_id`) on
track_info.** The Navidrome scanner already writes
`musicbrainz_recording_id` directly to the tracks row when present.
Picard-tagged libraries always carry MBID; metadata sources
(Spotify via MusicBrainz enrichment, Deezer, etc.) carry ISRC.
Auto-import had access to both via the metadata-source response
but didn't propagate them — so the soulsync row went in with
NULL on both columns.
# Changes
**`core/auto_import_worker.py` — `_process_matches`:**
- Top-level `'source': source` (from `identification['source']`)
- `'_download_username': 'auto_import'`
- `track_info['isrc']`, `track_info['musicbrainz_recording_id']` —
pulled from the per-track payload returned by the metadata source
- `track_info['album_id']` — back-reference so source-aware ID
resolution works on sources whose API nests album under
`track.album.id` rather than `track.album_id`
- `spotify_artist['id']` now correctly carries the artist's source ID
(was `identification['album_id']`, a copy-paste bug from the
original implementation that made artist-id resolution fall back
to fuzzy matching)
- `spotify_album['artists'][0]['id']` carries artist source ID for
the same resolution path
**`core/imports/side_effects.py`:**
- `record_library_history_download` source_map: add
`"auto_import": "Auto-Import"` — tags imported tracks correctly
- `record_download_provenance` source_service: add
`"auto_import": "auto_import"` — provenance shows real source
- `record_soulsync_library_entry` track INSERT: now includes
`musicbrainz_recording_id` + `isrc` columns (matches
`insert_or_update_media_track`'s shape for Navidrome /
Plex / Jellyfin scans). Both default to NULL when not present.
# Behavior preserved
- Files still land in the same library template path (no path-build
change)
- Other media-server flows (Plex / Jellyfin / Navidrome users)
unaffected — `record_soulsync_library_entry` still gates on
`get_active_media_server() == "soulsync"`. Auto-import on those
servers continues to drop the file in the library folder + emits
`batch_complete` for the scan-trigger automation, same as before.
- Direct downloads (search → Download button) unaffected — they
already passed `source` + `username` correctly.
# Tests added
`tests/imports/test_auto_import_context_shape.py` (8 tests, new file):
- Worker context carries `source` for every metadata source
(parametrised across spotify / deezer / itunes / discogs)
- `_download_username='auto_import'` set unconditionally
- ISRC + MBID propagate from track payload to track_info when present
- ISRC + MBID default to empty string when absent (downstream
normalises to NULL at write time)
- track_info includes album-id back-reference
`tests/imports/test_import_side_effects.py` (4 new tests + 2 schema
column adds):
- `record_soulsync_library_entry` writes mbid + isrc columns when
present in track_info
- Deezer source maps to deezer_id column (regression case for
source-aware column resolver)
- `record_library_history_download` labels `_download_username=
'auto_import'` as "Auto-Import" not "Soulseek"
- `record_download_provenance` registers source_service as
"auto_import" not "soulseek"
# Verification
- 8/8 new context-shape tests pass
- 6/6 side-effects tests pass (4 new + 2 existing)
- 207 imports tests pass
- 2359 full suite passes (+12 from baseline 2347, no regressions)
- 1 pre-existing flake (`test_watchdog_warns_about_stuck_workers`,
passes in isolation, unrelated to this change)
- Ruff clean
