A whole-library migration (ramonskie copied his Lidarr library into /staging) makes the synchronous
staging scan walk + tag-read tens of thousands of files INSIDE the GET request, blowing past
gunicorn's 120s timeout — and because the killed request never warms the cache, every reload
re-times-out. Moves the SAME scan off the request thread; the page reports progress instead of
hanging.
- _scan_staging_records gains an optional `progress` param (additive; default None = unchanged).
Refactored to two passes: a fast walk to collect the audio-file list (total), then the slow
tag-read loop updating scanned. A generation guard stops a scan that finishes AFTER an import
from committing stale records.
- ensure_background_staging_scan(path): idempotent background runner filling the existing cache.
- get_staging_records_or_status(): warm cache or a scan that finishes within a ~3s grace → records
(so small/normal folders still answer in one request, no UX change); else ("scanning", progress).
A scan error is re-raised so the endpoints log + return it exactly as before.
- /staging/files|groups|hints return {scanning, progress} when the scan is still running instead of
blocking; new lightweight /staging/scan-status for cheap progress polling.
Single source preserved (same scan + cache, just off the request thread). 13 new tests (progress,
idempotent ensure, grace ready-vs-scanning, generation guard discards stale, endpoint scanning
shape, error contract, status ready/cold); full import suite 626 green; ruff clean.
Next: phase 2 — the React import page polls scan-status + shows a progress bar, then renders.
Discord (Shdjfgatdif): "if a track isn't imported it should remain there, not be deleted, so we
can retry." He was seeing failed downloads disappear and having to re-download.
Normally a rejected file is QUARANTINED (moved to ss_quarantine, preserved + retryable), not
deleted. But all four quarantine blocks (integrity / silence / quality / acoustid) had the same
fallback: if move_to_quarantine itself raised, os.remove(file_path). On a NAS that move can fail
(cross-device / permissions), so the except fired and the user's download was DELETED — the worst
outcome, and exactly the re-download pain he reported.
fix: on quarantine failure, log and LEAVE the file in place — never delete. The task is still
marked failed and the batch still notified (that code runs after the try/except and never touched
the deleted file), so the only behaviour change is "preserved instead of destroyed". Reviewed
every os.remove in the pipeline: the remaining ones are success-path cleanups (replacing an
existing destination, or removing a redundant download when the track is already in the library at
equal/better quality) — left untouched.
regression test drives the REAL pipeline through integrity-rejection with quarantine forced to
raise, and asserts the source file is preserved while the task is still failed + notified.
1311 imports/downloads/quality tests green, ruff clean.
radoslav-orlov: "create lossy copies of lossless tracks" only recognized FLAC, even though ALAC/
WAV/AIFF/DSD are now quality-profile formats. the FLAC knowledge was hardcoded in 3 separate
places (the import path, the Lossy Converter scan, and the fix executor) — exactly how a format
gets added in one spot but not another.
kettui-style fix — one canonical seam both sites route through, instead of 3 more string edits:
- new core/quality/lossless.py: is_lossless_format / is_lossless_audio_path (pure; injects a
codec probe for the ambiguous .m4a/.mp4 — ALAC vs AAC — so the decision stays testable with no
I/O), LOSSLESS_FORMATS (single source of truth, derived-consistent with model.tier_score), and
the lossy_output_would_overwrite_source safety invariant.
- create_lossy_copy + the Lossy Converter scan + repair_worker._fix_missing_lossy_copy all route
through it. SQL pre-filters by candidate extensions, then each file is confirmed (probing .m4a).
- SAFETY: a lossy copy must never be written over its own source — an .m4a ALAC source + AAC
target lands on the same .m4a path, and ffmpeg runs with -y. all three sites now bail on the
overwrite case BEFORE ffmpeg (the existing delete-original guard was too late — the source was
already clobbered). dropped a vestigial mutagen FLAC import; updated FLAC-only UI strings.
19 tests: full seam coverage (formats, the .m4a ALAC/AAC probe branch, candidate extensions, the
overwrite guard), a tier-model consistency test that fails if the lossless set drifts, and import-
site wiring tests — WAV now converts (was rejected), and the .m4a-ALAC+AAC overwrite case proves
ffmpeg NEVER runs. 286 quality/import/repair tests green, ruff clean.
diegocade1: DSD files (.dsf, ~500MB DSD64) were labeled "Low Quality" and nagged to upgrade.
two independent causes, both fixed (additive — no existing format/behaviour changed):
1) DSF was an unrecognized format -> bottom 'unknown' tier -> "Low Quality":
- source_map: map .dsf/.dff -> 'dsf' (also lights it up in AUDIO_EXTENSIONS, so Soulseek can
match a DSF if one exists)
- model.tier_score: 'dsf' base 102 (just above FLAC) — lands in the lossless range
- probe_audio_quality: add a DSD branch returning format='dsf' (mutagen.dsf for .dsf detail;
.dff classifies lossless without measured detail) instead of None
- settings UI: DSD in RT_LOSSLESS_FORMATS + a "DSD (DSF / DFF)" option in the profile dropdown
2) the actual cause of the screenshot's findings — the truncation guard falsely called DSF
"broken (only ~12% decodes)": ffmpeg decodes DSD to PCM at a different rate than the DSD
container's 2.8 MHz, so astats samples ÷ container-rate massively under-counts. now
detect_broken_audio skips the truncation check for DSD (silence detection still applies).
8 seam tests: dsf/dff -> 'dsf'; dsf tier in lossless range (with + without measured bitrate);
is_dsd_path; and a contrast pair proving the same 12%-decode numbers flag a .flac but skip a
.dsf. 230 quality/import/silence tests green, ruff + JS integrity clean.
ramonskie's thread finding: opening the Import page fires staging files + groups + hints
together, and each one independently os.walk'd the whole staging folder AND mutagen-read every
file's tags — 3x the directory walk and 3x the per-file tag I/O on every page open (the import
scan storm + memory spike on large staging folders).
they all need the same per-file tag data, so scan ONCE: _scan_staging_records walks staging and
reads each file's metadata a single time, returning per-file records that files/groups/hints all
derive from in-memory. a short TTL (6s) + a lock means the three near-simultaneous page-open
requests share one scan instead of each kicking off a full re-read; the lock also prevents
concurrent full scans. hints now derives from the same read_staging_file_metadata the other two
use (same underlying tags) instead of a separate read_tags pass. album/singles process drop the
cache on completion so the list updates immediately after files leave staging.
net: 1 walk + 1 tag-read-per-file on page open instead of 3. 2 tests (shared-scan: 3 endpoints =
1 read per file, not 3; hints updated to the shared reader) + autouse cache-clear fixture; 695
import/staging tests green.
the duration-agreement check used abs() drift, so a file running LONGER than the metadata
(a remaster with a longer outro, an extended cut) was rejected the same as a truncated one —
e.g. A-Ha 'Take on Me' remaster at 228.5s vs 225.0s expected, quarantined for +3.5s.
but a longer file is the OPPOSITE of truncated. make the auto tolerance asymmetric: keep the
tight 3s/5s bound for SHORTER files (the truncation case the check exists for), allow up to
15s in the LONGER direction for version/master differences. a wrong song still trips it (off
by far more than 15s), and a user-pinned tolerance is honoured symmetrically. direction-aware
rejection message too. 4 new tests; 274 integrity/import tests green.
- file_ops.probe_audio_quality: .aiff/.aif were opened with mutagen.wave.WAVE,
which can't parse AIFF — it raised, failed open, and let AIFF silently bypass
the quality filter. Route aiff/aif to mutagen.aiff.AIFF (still the 'wav'
lossless tier).
- test_hifi_preview_guard: _get_hls_manifest gained an expected_duration_s kwarg
and the start tier now comes from quality_tier_for_source (default profile ->
'hires'); accept the kwarg and pin the tier so the chain is deterministic.
- test_quarantine_management: quarantine_group_key intentionally no longer uses
source-specific ids/uri (they break cross-batch sibling matching); assert the
isrc -> normalized-name contract instead.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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.
safe_move_file used shutil.move, which for a CROSS-filesystem move (downloads volume ->
library volume, common in Docker/NAS) copies the file to the FINAL path incrementally. A
media-server real-time watcher (Jellyfin) can catch that partial file mid-write and cache it
with null/incomplete metadata — tracks landing with no disc. Inconsistent because it only bites
cross-fs and races the scan tick; 're-add library' fixes it (rescan reads the now-complete file)
and the on-disk tags are fine — exactly the reported symptoms.
Fix: same-fs uses an atomic os.replace (also overwrites dst); cross-fs copies to a HIDDEN temp
sibling, fsyncs, then atomic os.replace into place (+ temp cleanup on failure). A watcher only
ever sees the COMPLETE file. EXDEV/EPERM/EACCES + the old string check route here, so detection
is strictly broader than before.
Tests: same-fs move, simulated EXDEV routes to the atomic path and leaves no partial temp, helper
completes+cleans, helper cleans temp + preserves source on failure. Existing replace-destination
test still green; 574 imports+relocate tests pass.
Tracks auto-downloaded from the playlist pipeline / wishlist / watchlist landed as
01/1 even though they belong to multi-track albums (wolf/Sokhi; verified live —
Deezer says 'Obelisk' is track 9 of The Grand Mirage, Olives is 3/4, etc.).
Root cause, located in code: discovery doesn't carry a per-track position for
sources whose search/track endpoint omits it (Deezer search, MusicBrainz
recordings — only their ALBUM endpoint has it). detect_album_info_web then set
'track_number': track_info.get('track_number') (= None) and never looked it up
from the album it HAD identified (context.py); the pipeline floored it to 1. The
one helper that does an album lookup only ran for the no-album-context branch and
is gated off by default. Not isolated to Deezer — the gap is source-agnostic.
Fix: when the album is known (album_id present) but the position is missing,
resolve the REAL (track_number, disc_number) from the album's own track list via
the source-agnostic get_album_tracks_for_source — using the album id discovery
already picked (no re-search, no edition guessing). Matches by ISRC -> source
track id -> title. Fail-safe: any miss/error leaves the number untouched, so it
still falls through to the filename exactly as before — never worse than today.
kettui: pure seam core/imports/album_position.resolve_track_position_in_album
(I/O-free, ISRC>id>title priority, skips position-less entries) + a fail-safe
integration wrapper, both covered — 11 tests incl. the 'Obelisk = 9/12' case,
priority resolution, and never-raises-on-fetch-error. 788 import/context/pipeline
tests green, ruff clean.
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.
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.
Answers "does import respect quality?": yes — the pipeline already runs the
quality gate (check_quality_target) BEFORE AcoustID and quarantines files that
don't meet the profile (unless fallback/downsample is on). This adds an explicit
user switch over that behaviour.
- New config import.quality_filter_enabled (default True). When False,
check_quality_target returns None early so EVERY file imports regardless of
quality; the file is still probed and the library Quality Upgrade Scanner
still flags below-profile tracks. Default preserves current behaviour.
- Settings → Library: the Import Settings group is now a collapsible tile
(same pattern as Post-Processing) and gains the "Only import tracks that meet
your quality profile" toggle at the top, alongside replace-lower-quality and
folder-artist-override.
- settings.js populate/collect the new key; config schema default added.
- Tests: key-aware config stub (a blanket-False mock would wrongly disable the
filter) + a new test pinning toggle-OFF = accept below-target file.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1) _resolve_library_file_path now tries the FULL relative path (index 0)
against each base dir, not just suffixes. The library scanner stores clean
"Artist/Album/Track.flac" paths; skipping index 0 dropped the artist folder
so the file never resolved — every quality-scanner probe failed ("20/20
could not be probed"). Now they resolve under the transfer/library dir.
2) Quality gate moved BEFORE the AcoustID check in post_process_matched_download.
- A wrong-quality file is rejected without paying for an AcoustID fingerprint.
- context['_audio_quality'] is set before either gate quarantines, so the
real quality is recorded on the sidecar for EVERY quarantine trigger —
it's known when reviewing/approving any quarantined file.
- force_import still never fires on a quality mismatch (only AcoustID).
normalize_import_context mutates in place, so the moved block keeps its
context fields. New test pins the order + that AcoustID isn't run on a
quality reject.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The actual HiFi/Monochrome bug isn't silence padding — it's a TRUNCATED file:
the container claims the full length (e.g. 3:08) but only ~30s of audio
decodes. silencedetect finds nothing (there's no silent audio, just missing
audio) and ffmpeg's time= even reports 0 with no error, so the duration and
quality guards all pass.
Detect it by decoding and comparing the real audio length (astats sample
count / sample rate) against the container duration: reject when the real
audio covers < 85% of the claimed length. detect_broken_audio() runs this
truncation check first, then the silence-ratio check. Wire it into the guard
that runs at the integrity/length verification point.
Verified on the real file: 'only ~30s actually decodes of a 188s file (16%)';
a normal 180s file is not flagged.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
HiFi/Monochrome HLS assembly can produce a file with the correct container
duration but only ~30s of real audio + silence padding — the duration and
quality guards both pass, so nothing caught it until you listened. Add
core/imports/silence.py: ffmpeg silencedetect over the audio, reject when the
silent fraction exceeds 50%. Wire it into the post-download pipeline with the
same quarantine + next-candidate retry pattern as the quality guard
(trigger='silence'), and surface it via import_rejection_reason. Fails open
when ffmpeg/mutagen are unavailable so tooling problems never quarantine a
legit file.
Also mark 'quality filter' and 'silence guard' failures as recoverable
quarantine rows in the downloads UI (were shown as plain failures).
Verified end-to-end: a 30s-tone + 180s-silence FLAC is flagged '86% silence
(only ~30s audible of 210s)'; a 210s tone passes. 7 parser unit tests.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
check_flac_bit_depth now delegates to check_quality_target, which probes the
real file and treats bit depth as a MINIMUM (24-bit satisfies a 16-bit
target) — the old context-string parsing, per-quality bit_depth_fallback, and
'reject higher bit depth' semantics are gone. Rewrite the wrapper tests to
the probe-based model and update the rejection-reason assertion to the
unified 'quality filter' wording.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Quality guard: rejects with a 'file is X, wanted Y' reason (the string the
track-detail modal surfaces), accepts when a target is met or fallback is on,
skips when unprobeable.
force_import isolation: the 'quality' bypass must not skip the AcoustID check
and vice-versa; a quality reject persists trigger='quality' (not 'acoustid')
in the sidecar — so a quality mismatch never routes through the force_import
path (reserved for AcoustID version-mismatch).
Model: lossy matches a MINIMUM bitrate (>=, a range); lossless matches on bit
depth + sample rate, never exact bitrate, so a FLAC's varying bitrate (mono /
compression) can't falsely reject it. v2->v3 migration preserves order.
47 passing across the quality + guard suites.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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.
