- _SOULSYNC_BASE_VERSION → 2.6.3
- helper.js WHATS_NEW unreleased flag → 'May 27, 2026 — 2.6.3 release'
Note: .github/workflows/docker-publish.yml default version_tag was
also bumped to 2.6.3 locally, but .github is gitignored in this
repo — workflow updates need to land via the GitHub UI separately.
The workflow_dispatch input is overrideable at trigger time
regardless of the default, so this isn't blocking.
Two related leaks in ``storage/album_bundle_staging/<batch_id>/``:
1. **Soulseek bundle cleanup was excluded.** The per-batch cleanup
at the end of a bundle download gated on:
(album_bundle_source or '').lower() in ('torrent', 'usenet')
The comment justified it as "slskd keeps its own completed
folders" — but the Soulseek bundle path ALSO copies completed
files into the private staging dir (``soulseek_client.py:1599``,
``copy_audio_files_atomically(completed, Path(staging_dir))``)
for the per-track workers to claim. Those copies persisted
forever; long-running installs accumulated stale GB. Extended
the cleanup gate's allow-list to include ``soulseek`` so the
per-batch dir is removed on bundle completion — same code path
that already worked for torrent / usenet.
2. **No sweep for orphan dirs.** Any leftover ``<batch_id>``
subdir from a previous-session crash, an errored batch, or a
pre-fix Soulseek bundle stayed on disk forever. Added
``sweep_orphan_album_bundle_staging(staging_root, active_batch_ids)``
that runs ONCE at server startup, before any batch can register
a staging dir. Removes every ``<batch_id>``-shaped subdir
whose id isn't in the active set. Safe by construction:
- Only touches subdirs of the configured staging root.
- Name-shape check (``entry.name == _safe_batch_dirname(entry.name)``)
rejects hand-placed dirs like ``.git`` or stray docs.
- ``shutil.rmtree`` errors log + continue — sweep must not
crash app startup over a permission glitch.
- active_batch_ids normalised through ``_safe_batch_dirname``
so colon-bearing batch_ids match their on-disk form.
Wired into the web_server startup right after the stuck-flags
diagnostic so it fires before anything else touches batches.
Tests
- ``test_downloads_lifecycle.py`` gained one regression test
pinning that Soulseek bundles now have their staging dir
cleaned (sibling to the existing torrent test).
- ``test_album_bundle_staging_sweep.py`` (NEW, 11 tests)
covers: orphan removal with no actives, active dirs preserved,
special-char batch_id normalisation, no-op on missing /empty
/empty-string staging root, non-dir entries skipped, unsafe-
name dirs preserved (.git etc.), partial rmtree failure doesn't
abort the rest, listdir failure returns 0 cleanly, default
None active set, defensive against empty / None entries in
the active set.
488 downloads tests pass.
For users with an existing "clean up old files" automation pointed
at this dir: stop pointing it there if you want — the auto-cleanup
+ startup sweep cover it now. Or leave it as belt-and-suspenders
with a relaxed (1h+) mtime threshold so it can't race a mid-batch
download.
The sync-tabs row had 14 sources jostling for horizontal space —
labels wrapped to 2 lines, the active pill ate disproportionate
room, the whole strip felt cramped and would only get worse as
more sources get added.
Restyled the strip as circular brand-logo chips. Inactive tabs
are 40px discs that show only the source's icon; the currently-
active tab swells into a pill that reveals its label inline.
Hover surfaces the source name as a native tooltip via the
title attr. Each chip carries its source's brand color as a
hover ring + active fill (Spotify green, Tidal orange, Qobuz
blue, Deezer purple, iTunes coral, YouTube red, Beatport green,
LB orange, Last.fm red, SSD teal).
Three sources share a logo with another source (Spotify Link
/ Spotify, Deezer Link / Deezer, iTunes Link / no native iTunes
but same logo family). Each "Link" variant carries a small
chain-link badge bottom-right so the chip disambiguates without
forcing the label to always be visible.
CSS-only swap — same JS handlers, same .active class, same
data-tab routing. HTML edit wraps each tab's label in a
``<span class="sync-tab-label">`` and adds ``data-link="true"``
to the Link variants so the CSS can target them.
Responsive: chips collapse to 36px on laptop / tablet and 32px
on mobile; the divider hides on mobile and gap tightens.
finished the release (#715)
Symptom (user @pavelcreates / @IamGroot60 on 2.6.2):
- Click Download on an album in the search modal
- slskd starts + completes every track of the release
- 22+ minutes after the last completed download, batch flips
to "failed" with no clear log line explaining why
- Per-track Soulseek downloads on the same machine were fine
Root cause: ``core/soulseek_client._resolve_downloaded_album_file``
probed three hard-coded candidate paths to locate each downloaded
file in the slskd download dir:
candidates = [
download_path / remote_filename,
download_path / basename,
download_path / *normalized_path_parts,
]
On the common slskd config ``directories.downloads.username = true``
slskd writes files at ``<download_dir>/<username>/<filename>`` —
none of the three candidates carry a username segment, so the
resolver returned None for every file even though the file was
physically present in a subdir one level deeper. ``_poll_album
_bundle_downloads`` saw 0 completed_paths, kept spinning, and
hit the master deadline (~30 min) before bailing the batch.
Why per-track worked: ``web_server._find_completed_file_robust``
already does a recursive walk-by-basename + path-confirm against
the remote directory components, so any layout slskd writes ends
up resolved. The bundle path didn't go through it.
Fix
- Lifted the robust finder into ``core/downloads/file_finder.py``
as a pure function ``find_completed_audio_file(download_dir,
api_filename, transfer_dir=None) -> (path, location)``. Zero
globals; recursive walk; handles slskd dedup suffix
``_<10+digit-timestamp>``, YouTube / Tidal ``id||title`` encoded
filenames, the AcoustID-quarantine subdir skip, basename
collisions disambiguated by remote-path components, and a
fuzzy-basename fallback above 0.85.
- ``_resolve_downloaded_album_file`` keeps the three-candidate
fast path (cheap probe for the slskd-flat default) but now
delegates to the new helper when none hit, instead of giving up.
- ``_poll_album_bundle_downloads`` tracks "slskd reports
Completed but local resolver returns None" per key. When every
remaining key has been in that state past a 45-second grace
window, the poll exits early with an explicit error pointing at
the likely ``soulseek.download_path`` mismatch instead of
silently spinning until the master deadline.
- ``web_server._find_completed_file_robust`` becomes a thin
delegate so both callers share one finder. Legacy inline impl
kept as ``_find_completed_file_robust_legacy`` for reference;
to be removed next release.
- Fixed misleading ``"(0 tracks, quality=)"`` log on the preflight-
reuse path — was reading attrs off a None ``picked`` object.
Tests (17 new in tests/downloads/test_file_finder.py)
- Flat slskd layout
- Username-prefixed (the #715 case)
- Full remote tree preserved
- Deeply nested username + tree
- File genuinely missing returns None
- Basename collision disambiguated by remote dirs
- Single basename match wins regardless of dirs
- slskd dedup suffix match
- Short ``_<digits>`` (year) not treated as dedup
- AcoustID quarantine subdir skipped
- YouTube / Tidal ``id||title`` encoded filenames
- transfer_dir fallback
- Both dirs miss → (None, None)
- Non-audio files ignored
- Empty api_filename
- Fuzzy match on punctuation variant
- Fuzzy rejects below threshold
475 downloads tests pass after the lift.
The sidebar source-group headers (Spotify / Tidal / Qobuz / Deezer /
YouTube / Last.fm Radio / ListenBrainz / iTunes Link / SoulSync
Discovery / Spotify Link) only showed the source name in caps —
the dashboard equalizer + connections panels both render the
actual brand logo, so the sidebar reading as text-only felt
disconnected.
Added a small (18px) circular brand-logo chip to the left of
each source-group title, sourced from the same URLs the
dashboard equalizer avatars use. Dark glass backdrop + accent
ring + drop-shadow on the logo so the chip stays legible
against either light or dark marks; brightness(0) invert(1)
applied to Tidal / Qobuz / iTunes-Link so their dark-foreground
marks render as white silhouettes against the disc (same
recipe the equalizer overrides use). Last.fm's square avatar
PNG clips to a circle via object-fit: cover.
Sources without a publicly available logo (Beatport, file
imports) fall through to no-chip — the <img onerror> swap
hides the broken image so the header still renders cleanly.
The Auto-Sync manager modal had been carrying its original visual
treatment forward unchanged while the rest of the app moved
toward the glassy / accent-radial / gradient-border aesthetic
the dashboard now sets. Restyled every surface inside the modal
to match.
Strategy: selector-based override layer appended at the end of
``webui/static/style.css`` — every selector in the new block
already exists in the original CSS above; the new block wins on
cascade order. Zero HTML / JS changes; functionality untouched.
Delete the v2 block to revert.
Surfaces restyled
- Modal shell: glass + thin accent border + corner radial wash
+ inner top-edge highlight, matching the dashboard ``.dash-card``
architecture
- Header: gradient-clipped title, accent-tinted eyebrow, hairline
accent separator below, spinning-X close button
- KPI summary tiles: dashboard-style gradient tiles, accent
top-edge glow on hover, gradient stat numbers
- Live monitor strip: accent-tinted glass card, status-colored
borders (running = green, error = red)
- Refresh / intro buttons: pill primary with accent fill + glow
on hover (replaces the bare ghost button)
- Tabs: underline-style with accent fill + soft radial glow on
the active tab (replaces the pill-tab look)
- Sidebar: glass panel, source groups as collapsible-feel cards,
accent border on scheduled playlist tiles, accent ring on the
filter input focus
- Board: subtle accent radial spotlight backdrop; columns are
glass cards with gradient headers + accent drag-over glow
- Drop zones: animated dashed pill with accent radial wash;
accent-tinted text on drag-over
- Scheduled cards: accent left-edge stripe, gradient pill timing
badges, pill "Run now" primary + rotating ghost X — health
variants (failing / warning) re-tint the left-edge stripe
- Run history rows: dashboard recent-activity aesthetic, accent
hover lift, pill status badges with colored borders
- Bulk schedule popover: glass card with accent border, pill
buttons, red ghost for unschedule
- Weekly editor: glass modal matching version-modal vibe,
day-toggle pills (accent fill when active), pill save button
- Empty / monitor-empty states: dashed glass card with subtle
vibe
- Soft accent-tinted scrollbars throughout the modal
When the Weekly Board shipped, its scheduled-card visual diverged
from the hourly board's: weekly cards showed only the playlist
name + weekly label + timezone, while hourly cards already
carried a full action row (Run-now button, unschedule X,
next-run countdown, health badge). Two boards looking like
different apps.
Standardised the weekly card on the hourly shape so a day-column
drop produces the exact same affordances as an interval-bucket
drop:
- Health badge (warning ⚠ / failing !) when recent runs errored
- Source + track-count meta line under the name
- Timing line: weekly label + tz pill + next-run countdown
- Run-now button (disabled while pipeline running, same gating
logic the hourly card already had)
- Unschedule X — calls the weekly-specific helper, leaving
hourly schedules untouched
Click anywhere outside the buttons still opens the weekly editor
for changing days / time / tz. Weekly cards also become
draggable between day columns now — drop on a new column appends
the day to the schedule (matches the multi-day editor flow).
Last.fm ships a square Twitter avatar; clip it to a circle so the
disc reads as a uniform chip. Tidal / Qobuz / Discogs / Amazon
ship dark-foreground marks that disappear against the dark glass
avatar backdrop — invert to a white silhouette so the logo
actually reads. The per-service accent drop-shadow still applies
so the brand color cue is preserved as a glow around the white
silhouette.
Initial-letter glyphs (SP / AM / DZ / ...) read as placeholder
once the brand-logo equalizer disc was visualised — each chip
should carry the service's actual mark. Wired the same logo
URLs the header-action worker orbs already load (Spotify press
asset, iTunes Wikimedia SVG, Deezer brandfetch symbol, Last.fm
avatar, Genius logo, MusicBrainz Wikimedia SVG, AudioDB local
PNG, Tidal / Qobuz / Discogs SVGRepo marks, Amazon local SVG)
into a new _RATE_GAUGE_LOGOS map and rendered as an ``<img>``
inside the avatar disc.
Visual details
- Disc backdrop switched from a solid accent-gradient fill to a
dark glass radial + accent-tinted ring + accent drop-shadow
on the logo. The service color still anchors the chip without
competing with the logo for contrast.
- Logo sized at 75% of the disc for breathing room. Drop-shadow
pops dark / multi-tone marks against the dark backdrop.
- Avatar bumped to 34px / 28px / 26px across desktop / tablet /
mobile so logos read clearly at every breakpoint.
Resilience
- ``<img onerror>`` swaps in an initial-letter glyph span on
load failure (CDN drop, network blip). The ``.rate-eq-avatar
--fallback`` variant restores the original accent-gradient
disc look so the fallback chip still reads as branded.
Asset
- AudioDB ships no public logo URL — saved the existing header-
action base64 PNG (~30 KB) to ``webui/static/audiodb.png`` so
the equalizer can reference it as ``/static/audiodb.png`` like
Amazon already does.
Four upgrades that take the equalizer row from clean to vibey.
All tied together by the same --eq-accent / --eq-glow CSS
variables so future tweaks stay coherent across the four
animation layers.
1. Brand-color avatar disc above each bar. Circular chip with a
2-3 letter glyph (SP / AM / DZ / LF / GN / MB / ADB / TD /
QB / DC / AZ) and a radial gradient using the service's
accent. Inner highlight + drop-shadow for depth; slow halo
pulse when the worker is running. Anchors each capsule to
its identity so the row reads as "these are your services"
not "these are 11 anonymous bars."
2. Peak-flash detector. When ``cpm`` actually steps upward
between socket updates (above a small jitter floor so
near-zero noise doesn't trigger), the peak tip briefly
flares white-hot, the fill flashes brighter, and the
reflection puddle ripples — all on a 650ms one-shot the JS
removes after fire. Mimics a hardware VU meter's peak-
detect LED. Sells the "alive" feeling by tying bar
movement to real call activity, not just continuous
animation.
3. Rolling-counter number animation. The live count under
the bar digit-animates from old→new with easeOutCubic
over 520ms instead of snapping. Per-element animation
handles tracked in a WeakMap so a fast second update
cancels the prior RAF loop instead of fighting it.
Premium-counter feel.
4. Glass-surface reflection puddle. Soft accent-colored
blurred ellipse under each bar; opacity scales with the
real (unclamped) rate via the --eq-glow variable so idle
bars don't pollute the row with permanent ground-light.
Rate-limited bars get a red puddle. Peak-flash briefly
intensifies the puddle so the surface "ripples" with the
call burst. Mounted on the host button (not the track) so
it escapes the track's overflow clipping.
Responsive: avatar disc shrinks to 26px at laptop/tablet,
24px at mobile.
The rate monitor on the dashboard used a 10-column grid of circular
SVG speedometers. With 11 services configured (Amazon was the
straw), the grid produced 10-in-row-1 + 1-orphan-in-row-2, breaking
the dashboard's tile symmetry. Speedometers also wasted ~80% of
their pixels on empty arc — most services sit at 0 cpm most of
the time, so the row visually read as a wall of empty gauges.
Replaced with a VU-meter / equalizer row: one vertical capsule
per service, brand-color gradient filling from the bottom, bar
height tracks ``calls/min ÷ limit``. Music-app native aesthetic,
fits the existing accent-heavy glassy vibe, and symmetric by
design at any service count — services slot into the flex row.
Visual details
- 4% sliver floor on idle bars so the row reads as "everything
alive" instead of "8 dead gauges" — vibe over literal zero
- Continuous shimmer scan when worker is running (vertical wash)
- Slow breathing pulse on idle bars
- Red gradient + faster pulse when rate-limited
- White-hot peak tip glows in the service's accent color
- Status pill below each bar (Running pulses green, Paused amber)
- Big count number floats top-center of the track
Behavior
- Click any bar opens the same detail modal the speedometer used —
no data-flow changes, no API changes, drop-in visual swap.
- Renderer auto-detects the dashboard context (data-card="enrichment")
and routes through the equalizer path; legacy speedometer code
still ships for any non-dashboard mount.
- Responsive: tightens at laptop/tablet breakpoints, wraps to
5-per-row on phones.
Pipeline-driven Auto-Sync runs against any ListenBrainz playlist
(Weekly Jams, Weekly Exploration, Top Discoveries, etc.) would sit
on ``Refreshing: "<name>"`` with no UI updates for 5-7 minutes
before the pipeline progressed. Two real bugs stacked:
1. **Double discovery.** The refresh handler called
``_maybe_discover`` (matching engine, per-track Spotify/iTunes/
Deezer matches) inline for any source returning
``needs_discovery=True`` tracks. Phase 2 of the pipeline then
ran the SAME matching engine via ``run_playlist_discovery_worker``
on the same tracks. The refresh-side run blocked the loop with
zero progress emission; Phase 2's already has the timed
progress-poll pattern. So LB tracks discovered twice, the first
time silently.
Pipeline now sets ``skip_discovery=True`` on its refresh config.
The handler honors the flag and lets Phase 2 handle discovery
end-to-end. Standalone callers (Sync-page tab, registration
action) leave the flag unset so they still get matched_data
on refresh.
2. **No targeted LB refresh.** The LB adapter's ``refresh_playlist``
called ``manager.update_all_playlists()`` — the only refresh
entry-point the manager exposed — which re-pulls every cached
LB playlist's details from the API (~12+ round-trips) even
when only one playlist needed refreshing. Wasteful;
tax-on-everyone for one-playlist work.
Added ``LBManager.refresh_playlist(mbid)`` — reads the cached
playlist_type, fetches just that playlist's details, runs the
normal ``_update_playlist`` upsert path. Defaults type to
``user`` for un-cached mbids so new-playlist discovery still
works. Skips ``_cleanup_old_playlists`` and
``_ensure_rolling_mirrors_from_cache`` (wasted work for a
single-playlist refresh).
Also: killed a silent ``except Exception: pass`` in the LB
adapter's old refresh wrapper that was masking every LB API
failure as a stale-cache hit. Refresh errors now log with full
traceback at warning level and propagate ``None`` so the outer
handler at ``refresh_mirrored.py:104`` counts the error and
surfaces it to the run-history error tally.
Pinned with 12 new unit tests across:
- ``tests/test_listenbrainz_manager.py`` (8): targeted refresh
happy path, unauthenticated guard, empty-mbid guard, upstream
``None`` return, default playlist_type for unknown mbid,
exception propagation, cost guard skipping cleanup, skipped-
when-unchanged signal
- ``tests/test_playlist_sources_adapters.py`` (3): adapter uses
targeted call (not legacy), adapter returns ``None`` on manager
error (not silent swallow), adapter resolves synthetic series
ids before calling the manager
- ``tests/automation/test_handlers_playlist.py`` (1):
skip_discovery flag bypasses ``_maybe_discover`` end-to-end
Residual per-track wishlist downloads (single tracks from different
albums, below the album-bundle threshold) were producing folders
without a year subfolder whenever the wishlist row carried a stale
``track_number=1`` from an older payload default.
Why: ``core/downloads/candidates.py`` had a single API-fetch branch
that served two concerns — resolving the track position AND
hydrating the lean ``spotify_album_context`` (release_date /
total_tracks / cover image) — gated entirely on track_number being
unresolved. When the wishlist row's ``track_number`` happened to
be 1 (a poisoned default rather than a real value), the gate
short-circuited and the album hydration the same call would have
done was skipped. Deezer-sourced discovery matches don't ship
release_date in their search-result album shape, so without the
backfill the folder lost its year.
The two concerns split:
- track_number resolution keeps its track_info → track object →
API precedence chain. track_info defaults still win.
- album hydration runs whenever release_date or total_tracks are
missing, independent of where (or whether) track_number was
resolved.
The single API round-trip still serves both — the cost contract
is preserved. The side-effect coupling is gone.
Lifted into ``core/downloads/track_metadata_backfill.py``
(``hydrate_download_metadata``) so the precedence chain is pinned
in isolation. 24 unit tests cover the precedence chain, the
poisoned-tn=1 regression case, defensive non-dict/None inputs,
the cost guard (API called at most once per invocation), and
disc_number resolution.
Also lands the upstream piece: ``core/wishlist/routes.py:_build_track_data``
no longer defaults ``track_number=1`` / ``disc_number=1`` /
``total_tracks=1`` / ``release_date=''`` when the library-modal add
payload omits them. Missing values now flow through as ``None`` so
the downstream pipeline can detect-and-recover instead of locking
to a fake position.
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).
PR 4 of 4 in the wishlist-album-bundle issue series. UI fix only —
zero behavior change.
User's 26-track wishlist run rendered all 26 sub-batches as
"Analyzing..." simultaneously. Pre-fix the rows were created with
``phase='analysis'`` BEFORE being submitted to ``missing_download_executor``
(max_workers=3 by default), so 23 batches sat in the executor queue
visually identical to the 3 actually running. Misled users into
thinking SoulSync was processing 26 in parallel; really only 3 ever
ran at once with the rest waiting their turn.
Fix:
- Wishlist auto-flow submission sites now create batch rows with
``phase='queued'``.
- The master worker (``core/downloads/master.py:328``) already flipped
phase to ``'analysis'`` as its first action on entry — that
transition becomes the real signal that the executor picked the
batch up.
- ``core/downloads/status.py`` surfaces ``analysis_progress`` for
the ``queued`` phase too so the UI has the track count to render
"Queued — N tracks" instead of an empty card.
- Frontend (``webui/static/pages-extra.js``, ``downloads.js``) renders
"Queued ⏳" for ``phase='queued'`` distinct from the spinner-laden
"Analyzing..." for ``phase='analysis'``.
Scope choices:
- Only the auto-wishlist submission sites flipped this PR
(``core/wishlist/processing.py:860`` album sub-batches +
``core/wishlist/processing.py:907`` residual). The manual-wishlist
sites at ``:451`` and ``:627`` use the same executor + worker, but
those create a caller-allocated batch_id that the frontend polls
immediately — wanted to verify the manual-poll path handles
``queued`` cleanly before flipping those. Trivial follow-up.
- Other submission sites in album_bundle_dispatch / web_server.py /
task_worker.py left untouched — they don't go through the
executor-queue pattern that causes this UI confusion.
Tests:
- Updated ``test_process_wishlist_automatically_creates_batch_for_matching_tracks``
to assert ``phase='queued'`` on creation (was ``'analysis'``); explanatory
comment names the executor-pool reason.
- New ``test_queued_phase_surfaces_analysis_progress_for_ui_count`` in
``tests/downloads/test_downloads_status.py`` pinning the new
``queued ⊂ analysis_progress`` rendering contract.
- 884 tests pass across wishlist + downloads + imports suites.
- Ruff clean on changed Python files; JS syntax OK on changed
webui files.
PR 3 (sibling-completion gate) was investigated and dropped — the
"1/26 finalized" symptom turns out to be downstream of the
staging-match bug (PR 2's instrumentation will catch it on the
user's next reproduction run), not an independent sibling-gate bug.
The gate logic itself is correct.
Two confirmed-from-code-reading bugs in the wishlist retry chain.
Both cause downstream post-process to render every retried file as
``01 - <title>`` without year in the folder path, even when the
source slskd file had the correct track number embedded and Spotify
had the album release date.
**Bug A — track_number defaults to 1 at every link in the chain.**
Pre-fix: ``.get('track_number', 1)`` defaulted at four sites:
- ``core/wishlist/payloads.py:121`` ``ensure_wishlist_track_format``
- ``core/wishlist/payloads.py:282`` Track-object conversion
- ``core/imports/context.py:421`` legacy album-info builder
- ``core/imports/pipeline.py:645`` final processing read
Each step "filled in" 1 when the upstream had dropped the key. The
downstream filename-extract fallback at ``pipeline.py:652`` ONLY
runs when the value is None — pre-filled 1 never matched, so the
fallback never fired, so the source filename's track number (e.g.
``08. No Sleep Till Brooklyn.flac``) was discarded in favour of the
default-1.
Fix: change every default from ``1`` to ``None`` along the chain.
The pipeline already has the right detect-and-recover logic — it
just needs the chain to stop poisoning it. Final ``< 1`` floor at
``pipeline.py:660`` still defaults to 1 as last resort, so callers
that genuinely have nothing still produce a valid number.
**Bug B — release_date dropped from cancelled-task wishlist payload.**
Pre-fix: ``build_cancelled_task_wishlist_payload`` only ``setdefault``ed
``name`` / ``album_type`` / ``images`` on the album dict. The
release_date field copy was load-bearing (when input was a dict, the
``dict(album_raw)`` copy preserved it), but when input was a bare
string the constructed dict had only name + album_type — no
release_date / total_tracks / etc.
Fix:
- Explicit comment on the dict-shape branch that release_date survives
via the unconditional ``dict(album_raw)`` copy + setdefault
semantics — so a future refactor that switches to a stricter copy
doesn't silently strip the field.
- String-shape branch now pulls release_date from
``track_info.album_release_date`` or ``track_info.release_date``
when present so the round-trip preserves the year for the path
template.
- track_data shape itself now carries ``track_number`` / ``disc_number``
at the top level (Bug A intersect — was dropping it entirely).
**Tests:** 4 new in tests/wishlist/test_payloads.py:
- ``test_ensure_wishlist_track_format_preserves_real_track_number``
- ``test_ensure_wishlist_track_format_keeps_missing_track_number_as_none``
- ``test_build_cancelled_task_wishlist_payload_preserves_track_number``
- ``test_build_cancelled_task_wishlist_payload_string_album_pulls_release_date_from_track_info``
14 payload tests pass; 879 across wishlist + imports + downloads
suites still green; 1410 wider suite all pass. Ruff clean.
Commits 2 + 3 of 3 in PR 2/4 of the wishlist-album-bundle issue fix
series. Commit 1 (94ba1d73) instrumented staging-match so the next
wishlist run produces the evidence we need to diagnose bug C
(staging-match silently drops album-bundle wishlist tracks); that
fix lands in a follow-up PR after the user's next reproduction run.
Pre-fix: ``try_staging_match`` silently returned False on three exit
points (empty cache, no track title, low best-score). Could not
diagnose the "track gets staged via album-bundle but never claimed
→ re-added to wishlist → infinite loop" bug from app.log because the
match-attempt + rejection was invisible.
Now every False exit logs at INFO with enough context to debug from
a single grep:
- ``[Staging] No match attempted for <track> — staging cache empty for batch <id>``
- ``[Staging] No match attempted for task <id> — track has empty title``
- ``[Staging] No match for <track> in batch <id> — best candidate <file> (title_sim=X, artist_sim=Y, combined=Z) below 0.75 threshold``
- ``[Staging] No match for <track> in batch <id> — N staging files but none had usable title variants``
Per-candidate skips (no title variants / title_sim < 0.80) log at
DEBUG so the noise stays out of INFO unless explicitly enabled.
Logs the near-miss candidate score on rejection so a 0.74 (one point
below threshold) surfaces as a different kind of bug than a 0.10
(completely wrong file in staging). Same shape SAB's adapter logs
now use for transient-vs-terminal status calls (PR #717).
Zero behavior change — pure logging. Enables the follow-up commit
that actually fixes the staging-match drop, by giving us real evidence
of WHERE the wishlist tracks are being rejected during the user's
next album-bundle run.
24 staging tests still pass; behavior unchanged.
Commit 1 of 3 in PR 2/4 of the wishlist-album-bundle issue fix
series. See ``memory/feedback_always_build_kettui_grade.md`` for
the instrument-before-blind-fix rule that drove this ordering.
Real-world wishlist case the original c3b88e69 design missed: user with
26 missing tracks from 26 different albums. Each item used to promote
to its own album-bundle sub-batch (``min_tracks_per_album=1``), which
downloaded the ENTIRE album (5-42 files) to claim one track. Confirmed
in app.log:
- "Licensed To Ill" downloaded 3 times across cycles (3-4 files each)
- "The Understanding" 17 files for 1 wishlist track
- "Alright, Still" 42 files for 1 wishlist track
- ~85% wasted bandwidth, slskd hammered with 26 concurrent searches
PR 1 of a 4-PR fix series — see commit body footer for the other PRs.
Default ``min_tracks_per_album`` 1 → 2. Single-track wishlist items
fall to ``residual_tracks`` → classic per-track batch (already works,
already efficient). Album-bundle kept for the case it was designed
for: user has 2+ tracks missing from the same album.
Override via the new ``wishlist.album_bundle_min_tracks`` config key:
- 1 = previous behaviour (bundle every item)
- 2 = new default
- 3+ = stricter, for users who want bundle only on bigger gaps
Helper ``_resolve_album_bundle_threshold`` lives in
``core/wishlist/processing.py``. Defensive shape mirrors the existing
config-driven knobs (``get_poll_interval`` / ``get_transient_miss_threshold``):
non-numeric, non-positive, or config-manager-raise all fall back to
the safe default. Three test cases pin the fallback chain.
Both wishlist entry points wired through the same helper:
- ``process_wishlist_automatically`` (auto cycle, line 812)
- ``start_manual_wishlist_download_batch`` (manual run, line 539)
Tests:
- ``tests/wishlist/test_album_grouping.py`` — old ``test_default_threshold_promotes_solo_albums`` flipped to ``test_default_threshold_demotes_solo_albums`` with explanatory docstring naming the real-world cause. New ``test_default_threshold_promotes_multi_track_albums`` pins the 2+ promotion. New ``test_explicit_threshold_one_restores_solo_promotion`` pins that the kwarg still works for opt-back-in.
- ``tests/wishlist/test_processing.py`` — 3 new tests for ``_resolve_album_bundle_threshold``: default-when-config-missing, honors-config-override, falls-back-on-garbage.
- ``tests/wishlist/test_automation.py`` — ``test_wishlist_albums_cycle_splits_into_per_album_batches`` updated to use 2+ tracks per album (5 tracks across 2 albums instead of 3 across 2 with 1 solo). ``test_wishlist_albums_cycle_residual_for_orphan_tracks`` updated to include 2 tracks from Album One so it still promotes.
- ``tests/wishlist/test_manual_download.py`` — same shape update for the manual path test.
- ``tests/wishlist/test_album_grouping.py:test_multiple_albums_emit_separate_groups`` updated to reflect new default (alb1 with 2 tracks promotes, alb2 with 1 track goes residual).
- ``tests/wishlist/test_album_grouping.py:test_nested_track_data_payloads_normalized`` pinned with explicit ``min_tracks_per_album=1`` so the test stays focused on payload-shape parsing, not the threshold rule.
114 wishlist tests pass; 866 across wishlist + automation + downloads +
album_bundle + album_bundle_dispatch suites still green. Ruff clean.
Sibling PRs queued in TaskCreate:
- PR 2 — investigate post-process staging-match miss (the second-order
bug that causes the same album to redownload every cycle when the
staging step doesn't claim the requested track).
- PR 3 — fix sibling-completion gate that fires on first sibling
instead of last (log evidence: run a4945c88 finalized 1/26 batches).
- PR 4 — UI distinguish Queued from Analyzing for batches waiting
on the executor (23/26 batches sit at "Analyzing..." while really
queued at max_workers=3).
PR 3 of the schedule-types feature — see
``memory/project_auto_sync_schedule_types.md``. Backend
``next_run_at`` + ``weekly_time`` trigger handler landed in PRs 1-2.
This PR exposes them in the Auto-Sync manager so users can finally
schedule playlists by day-of-week + time instead of only hourly
intervals.
**UI layout:**
The Auto-Sync modal grows a ``Weekly Board`` tab between
``Hourly Board`` (renamed from ``Schedule Board``) and
``Automation Pipelines``. Same sidebar (mirrored playlists grouped
by source, with filter). Main panel is 7 day columns Mon-Sun
instead of 10 hour buckets. Drag a playlist onto a day column →
creates a single-day weekly schedule at the default time
(09:00 in the browser's IANA tz from
``Intl.DateTimeFormat().resolvedOptions().timeZone``). Click any
scheduled card → opens an editor popover for time, multi-day
toggles, tz override, and unschedule.
Multi-day schedules render under every matching column (Mon-Wed-Fri
schedule appears as three cards, one per column) — matches how
users think about "this playlist runs on Mon AND Wed AND Fri".
**Mutual exclusion:** one schedule per playlist. The save path on
either tab deletes any existing schedule of the OTHER kind before
installing the new one. Backend can technically run both as two
separate automation rows, but two cards under the same playlist
would surprise users and the engine has no merge semantic for
"daily-and-hourly".
**Pure-function helpers** (testable via node:test, matching the
existing ``tests/static/test_auto_sync.mjs`` pattern):
- ``detectBrowserTimezone()`` — Intl tz with UTC fallback for
browsers where Intl is absent.
- ``autoSyncWeeklyTrigger({time, days, tz})`` — defensive payload
builder: garbage time → 09:00, unrecognised days dropped,
missing tz → browser tz.
- ``autoSyncWeeklyFromTrigger(config)`` — inverse parser with
the same defensive shape. Empty days expands to every weekday
(matches ``next_run_at`` engine semantic). Returns null for
non-object configs so ``buildAutoSyncScheduleState`` can route
broken rows to automationPipelines instead of silently
bucketing them as every-day weekly.
- ``autoSyncWeeklyLabel(parsed)`` — sorted "Mon, Wed, Fri @
09:00" / collapses to "Daily @ HH:MM" for full-week / "Unscheduled"
for null. Canonical Mon-Sun ordering regardless of input order.
**Tests:** 26 new node:test cases across ``detectBrowserTimezone``
x1, ``autoSyncWeeklyTrigger`` x6, ``autoSyncWeeklyFromTrigger`` x6,
``autoSyncWeeklyLabel`` x5, and ``buildAutoSyncScheduleState``
weekly bucketing x5 (covering owned weekly_time → weeklySchedules,
hourly stays in playlistSchedules, non-owned falls through to
automationPipelines, legacy-named auto-sync rows still recognised,
garbage trigger_config falls through). All 62 node:test cases pass;
261 across the automation pytest suite still green (zero regression
on PRs 1-2's plumbing). Python wrapper at
``tests/test_auto_sync_js.py`` shells out cleanly.
**CSS** (themed to the existing Auto-Sync gradient + accent
variables):
- 7-column grid for the weekly board, narrower than the 10
hour-bucket layout.
- Editor popover with backdrop-blur, accent-tinted save / delete
buttons, hover states that pick up the user's accent color.
- ``scheduled-elsewhere`` state for playlists with an hourly
schedule visible on the weekly board (dashed border + opacity)
so the user knows a drop will replace, not stack.
**WHATS_NEW entry** under 2.6.3 unreleased — first user-visible
slice of the schedule-types feature.
PR 4 (Monthly UI tab) deferred until weekly proves wanted.
PR 1 (commit 6ad85e27) shipped the ``next_run_at`` pure function as
foundation plumbing. PR 2 wires the engine through it and adds
``monthly_time`` as a real registered trigger type. After this PR
``core/automation_engine.py`` no longer has its own datetime
arithmetic for daily / weekly schedules — every next-run computation
flows through one function with one set of defensive fallbacks.
Net user-visible change: zero (no UI surface for monthly_time yet —
that's PR 3). New ``monthly_time`` trigger is reachable only via
direct API for now.
**Engine refactor:**
- ``_finish_run`` — collapsed three inline branches (daily_time
arithmetic, weekly_time arithmetic, fallback schedule arithmetic)
into a single ``next_run_at(...)`` call with ``_dt_to_db_str``
normalising the aware-UTC result to the engine's naive-UTC string
convention. Retry-delay short-circuit preserved. Exception
swallowing preserved (logged at debug, writes None next_run).
- ``_setup_daily_time_trigger`` + ``_setup_weekly_time_trigger`` +
new ``_setup_monthly_time_trigger`` — three near-identical methods
collapsed into one ``_setup_timed_trigger`` skeleton. Each public
method is now a one-line dispatch passing trigger_type to the
shared helper with a human-readable label for the debug log.
- Existing ``_next_weekly_occurrence`` deleted — its logic now lives
in ``core/automation/schedule.py:_next_weekly`` (lifted in PR 1).
- New ``_dt_to_db_str(dt)`` module-level helper normalises aware-UTC
→ naive-UTC string. Centralised so a tz mistake here surfaces in
one place. Aware non-UTC datetimes converted to UTC first
(defensive against a future bug that passes the wrong tz).
- New ``_resolve_system_default_tz()`` reads the server's local IANA
tz via ``tzlocal``. Cached at module import (the host's tz doesn't
change while the process runs). Falls back to UTC when ``tzlocal``
is missing — defensive for minimal Docker images.
- New ``self._default_tz`` engine attribute reads from
``automation.default_timezone`` config first, falls back to the
system-detected IANA name. Override path lets users on weird
setups pin a specific tz without touching env vars.
**Convergence fix (intentional behaviour change):**
Old ``_setup_daily_time_trigger`` / ``_setup_weekly_time_trigger``
didn't check the DB for an existing future ``next_run`` — they'd
recompute from scratch on every engine startup, overwriting manual
edits or pending retries. The interval path (``_setup_schedule_trigger``)
already had this check. The new shared ``_setup_timed_trigger``
brings daily / weekly in line: existing-future next_run wins over
freshly-computed delay. Treat this as a correctness fix, not a
breaking change — the old behaviour was an inconsistency, not a
deliberate choice.
**Backward-compat:**
- Existing ``schedule`` / ``daily_time`` / ``weekly_time`` rows
continue to work unchanged. The ``_trigger_handlers`` registry
keeps every historic key.
- Existing rows without an explicit ``tz`` field use
``self._default_tz`` (server-local IANA via ``tzlocal``) —
preserves "every Monday 09:00 server-local" behaviour on
non-UTC servers. Pre-fix the engine used naive
``datetime.now()`` which is also server-local; net effect is
identical wall-clock time, just routed through a tz-aware
pipeline that handles DST correctly (the May 2026 "next in 8h"
bug fix class).
- Engine boots even when ``tzlocal`` is missing — the resolver
falls back to UTC silently. Existing tests would catch a hard
dependency on tzlocal here.
**``tzlocal>=5.0`` added to requirements.txt** alongside
``tzdata>=2024.1`` from PR 1. Both libraries are small and stable;
``tzlocal`` returns a clean IANA name across Windows / Linux /
Docker, sidestepping the platform-specific tz detection mess.
**Tests:** 20 new in ``tests/automation/test_engine_schedule_integration.py``:
- ``_dt_to_db_str`` x3 (aware UTC, aware non-UTC converted to UTC,
naive assumed UTC)
- ``_resolve_system_default_tz`` x2 (returns IANA string, falls back
to UTC without tzlocal)
- ``_finish_run`` dispatch through next_run_at for each trigger type
(schedule, daily_time, weekly_time, monthly_time)
- Retry-delay short-circuits next_run_at
- next_run_at returns None → DB next_run cleared
- next_run_at raises → engine swallows + writes None
- Event triggers skipped (no scheduled next-run)
- ``self._default_tz`` passed through to next_run_at
- monthly_time registered in _trigger_handlers
- All historic trigger types kept registered
- ``_setup_monthly_time_trigger`` arms timer + writes DB
- ``_setup_timed_trigger`` honours existing future DB next_run
- Skip-with-log when next_run_at returns None
- End-to-end no-mock smoke for monthly_time
260 automation suite tests pass; the 240 from PR 1's branch plus 20
new integration tests. Ruff clean.
No WHATS_NEW entry — UI doesn't expose monthly_time yet (PR 3),
and the backward-compat path preserves existing daily/weekly
schedule timing.
Self-review pass on ec4a55c1 — applying the standing kettui-grade
rule (see memory/feedback_always_build_kettui_grade.md). Three issues
that would have surfaced on review:
1. Silent tz fallback to UTC
``_resolve_tz`` returned UTC when the IANA name was unknown — no
log, no warning. User on a host without ``tzdata`` who configures
``America/Los_Angeles`` got schedules running silently at UTC
offset with no way to debug. Now logs WARNING once per unknown
name (deduped via ``_UNKNOWN_TZ_WARNED`` set so a misconfigured
row doesn't spam every poll cycle) and the log line names BOTH
real causes — typo or missing tzdata — so the user can fix from
a single grep.
2. ``weeks`` unit drift from engine
I added ``'weeks': 86400*7`` to ``_INTERVAL_MULTIPLIERS`` but the
engine's existing ``_calc_delay_seconds`` only recognises
minutes/hours/days. Until PR 2 collapses both paths through this
function, any row whose config snuck through with ``unit='weeks'``
would get scheduled by the engine as 1-hour and by this function
as 7-day — drift between two live implementations. Dropped
``weeks`` from the map to match the engine. Added a comment
pinning the map to the engine's contract and a regression test
that asserts ``unit='weeks'`` falls back to the same hours
default the engine produces.
3. DST edge cases unverified
The module docstring claims DST-aware via ``zoneinfo`` but no test
pinned the spring-forward gap (02:30 LA on DST-Sunday doesn't
exist) or fall-back ambiguity (01:30 LA on fall-Sunday happens
twice). Three new tests:
- ``test_dst_spring_forward_lands_after_the_gap`` — pins that the
function doesn't crash + lands on a real instant past ``now``.
- ``test_dst_fall_back_handles_ambiguous_local_time`` — pins
zoneinfo's default-earlier-instant resolution for ambiguous
local times (01:30 PDT vs 01:30 PST → picks PDT).
- ``test_weekly_across_dst_boundary_keeps_local_wall_clock`` —
pins that a "every Sunday at 09:00 LA" schedule keeps the
local wall clock across the boundary even though the UTC
equivalent shifts by an hour. This is the exact bug class
that caused the May 2026 "next in 8h" tz mismatch.
Also loosened ``tzdata==2026.2`` to ``tzdata>=2024.1``. IANA tz data
changes a few times a year for real-world DST policy updates; pinning
to one snapshot would freeze the app's tz knowledge to the build date
and miss future government-mandated rule changes.
41 schedule tests pass (5 new); 240 across the full automation suite.
Ruff clean.
Backend plumbing for upcoming weekly + monthly Auto-Sync schedules.
PR 1 of 4 in the schedule-types feature — see
``memory/project_auto_sync_schedule_types.md`` for the full plan.
Net behaviour change in this PR: zero. The automation engine still
computes next_run via its existing inline ``_calc_delay_seconds`` /
``_next_weekly_occurrence`` helpers; this module is unused until PR 2
wires the engine through. Lands separately so the foundation can sit
on dev for a beat before the engine change.
``core/automation/schedule.py:next_run_at(trigger_type, trigger_config,
now_utc, default_tz)``:
- Pure function. ``now_utc`` injected (tests freeze time without
monkeypatching ``datetime.now``); ``default_tz`` injected (so daily /
weekly / monthly schedules compute against the USER's timezone, not
the server's — the same class of bug that produced the May 2026
"Auto-Sync next in 8h" timezone fix).
- Returns aware-UTC ``datetime`` ready to serialise to the DB
``next_run`` column, or ``None`` for unrecognised / event-based
triggers (callers should not write a next_run for those).
- Naive ``now_utc`` inputs are assumed UTC for defensive symmetry
with the engine's DB-string parser convention.
Trigger types covered:
- ``schedule``: ``{interval: N, unit: 'minutes'|'hours'|'days'|'weeks'}``
— matches engine's existing ``_calc_delay_seconds``. Unknown unit
defaults to hours; zero/negative interval clamps to 1 (preserves
the engine's guard against scheduling for the past); non-numeric
interval falls back to 1.
- ``daily_time``: ``{time: 'HH:MM', tz: '<IANA>'}`` — DST-aware via
``zoneinfo``; ``tz`` falls back to ``default_tz``; unknown IANA
string falls back to UTC; garbage ``time`` falls back to 00:00.
- ``weekly_time``: ``{time, days: ['mon',...], tz}`` — empty / all-
invalid ``days`` list means "every day" (matches engine fallback);
abbreviations case-insensitive; 8-day scan finds the next match.
- ``monthly_time``: ``{time, day_of_month: 1-31, tz}`` — NEW shape.
Day clamped to [1, 31]. Months too short for the target day clamp
to the LAST valid day rather than skipping a month (standard cron
convention; running a day early in February is less surprising
than missing the whole month). 12-iteration loop cap so a
pathological config can't infinite-loop.
Tests (36 cases, all passing):
- Interval: every unit, unknown-unit fallback, zero/negative/garbage
interval clamp, tz field ignored on interval (wall-clock-independent).
- Daily: today-at-future-time runs today, today-at-past-time rolls to
tomorrow, exact-match rolls to tomorrow (no schedule-now-then-schedule-
again-immediately), user-tz vs server-tz, default_tz fallback,
garbage time / unknown tz defensive returns.
- Weekly: same-day-still-future qualifies, same-day-past rolls to next
allowed day, wraps across week boundary, empty days = every day,
garbage abbreviations dropped, case-insensitive, tz across day
boundary (LA Wednesday evening is Thursday UTC).
- Monthly: target day this month, rolls to next month when passed,
Feb 31 → Feb 28 / Feb 29 leap year, day_of_month above 31 / below
1 clamp, Dec → Jan year roll, user-tz pre-midnight edge case.
- Result-shape contract: every returned datetime is aware UTC at
offset zero (engine relies on this when serialising to the
``next_run`` string column).
Added ``tzdata==2026.2`` to requirements.txt. Windows ``zoneinfo`` and
minimal Docker base images ship without the system tz database;
without ``tzdata`` ``ZoneInfo('America/Los_Angeles')`` raises
``ZoneInfoNotFoundError`` and the helper silently falls back to UTC.
No WHATS_NEW entry — no user-visible behaviour change in this PR.
PR 2 (engine wire-through) will land the user-facing changelog entry
when ``monthly_time`` becomes a real schedulable trigger.
Follow-up to f13d3395. Five gaps called out on self-review:
1. Per-track inline transient tolerance was duplicated between
usenet.py and torrent.py (~12 lines each, identical) and wasn't
directly tested. Extracted into ``TransientMissCounter`` in
``album_bundle.py`` — small class with ``record_miss()`` returning
True at threshold and ``reset()`` for successful reads. Both
per-track flows AND the lifted ``poll_album_download`` now use
the same counter, so the rule is in one place.
2. Threshold is now config-driven via
``download_source.album_bundle_transient_miss_threshold``
(default 5). Same defensive pattern as ``get_poll_interval`` /
``get_poll_timeout`` — non-positive / non-numeric falls back to
the default. Users with very slow servers (huge multi-disc box
sets, slow disks) can extend the tolerance window without
touching code.
3. SAB state map verified against the canonical Status enum in
``sabnzbd/constants.py`` (sabnzbd/constants.py:~95-118). Dropped
six entries I'd guessed at and couldn't verify in source
(``trying``, ``prop_paused``, ``prop_failed``, ``unpacking``,
``pp``, ``postprocessing``). Kept the verified ``deleted`` (lower-
cased from SAB's ``Deleted``) and added the one real state I'd
missed: ``Propagating`` (SAB's pre-download delay state — maps to
``queued`` since we're waiting on the NZB to be available, not
actively downloading).
4. SAB integration test exercising the queue→history gap end-to-end
through the real adapter HTTP layer. Mocks SAB's queue + history
endpoints with the exact response shapes SAB emits, runs three
gap polls (both endpoints empty), then a recovery poll where the
slot appears in history as Completed. Confirms the TransientMissCounter
absorbs the gap and ``poll_album_download`` returns the save_path
without emitting terminal failure. This was the path I had only
tested at the helper layer before — now pinned end-to-end through
the adapter.
5. SAB state mapping has new tests: every Status value from SAB's
canonical enum must map to a known adapter state (not the 'error'
default fallback), Propagating routes to queued, Deleted routes
to failed. Future SAB state additions that we miss will surface
as 'error' default → transient-miss tolerance → terminal failure
with a clear log line, but the explicit assertion list here means
we'll catch the omission in CI before users do.
Test count after: 537 download-suite tests pass; 21 new
(``TransientMissCounter`` ×4, ``get_transient_miss_threshold`` ×3,
SAB state-coverage ×3, SAB direct ``nzo_ids`` lookup ×5, SAB
queue→history integration ×1, plus the existing helper-layer
coverage from the parent commit). Ruff clean.
User reported usenet album downloads getting stuck on "downloading
release" while SABnzbd reported the job as complete. Container restart
did not help; reproducible on every usenet album download.
Three independent issues all causing the same symptom — the download
modal freezes mid-flow with no error surfaced to the user:
1. SAB queue → history transition window
SAB removes a slot from its queue BEFORE adding it to the history,
and on a busy server (par2 verify, unrar, multi-file move) that
window can span several poll iterations. The poll treated a single
None status as terminal failure ("disappeared from client") and
gave up. Now the poll tolerates up to ~10s of consecutive misses
(5 polls at the default 2s interval) before declaring the job gone.
2. SAB queue states like `Pp` were unmapped
`_SAB_QUEUE_STATE_MAP` didn't cover SAB's `Pp` (post-processing
summary), `Unpacking`, `Trying`, `Deleted`, or the `Prop_paused`
/ `Prop_failed` variants. Unmapped states fell through to the
default-'error' fallback, and the poll loop only treated explicit
'failed' / 'completed' as terminal — 'error' was neither, so the
loop spun until the 6-hour timeout. Map now covers every Status
value from SAB's `sabnzbd/api.py`, and the poll treats the default-
'error' fallback as a transient miss (warn-logged, retry within
the same tolerance window) so a brand-new unmapped state can't
infinite-loop the way `Pp` did here.
3. No terminal failure emit
The poll only logged on failure / timeout / disappeared — never
called the progress callback with 'failed', so the download modal
stayed at the last 'downloading' emit forever. Plumb a 'failed'
emit through every failure exit path so the UI flips out of the
downloading state when the poll gives up.
Plus:
4. SAB direct nzo_ids lookup instead of paging all-history
`_get_status_sync` was fetching the latest 50 history entries on
every poll and iterating to find the target nzo_id. On busy
servers (many recent downloads), the target job could roll past
the 50-entry window and look like a "disappeared" job. Replaced
with a targeted `mode=queue&nzo_ids=<id>` → `mode=history&nzo_ids=<id>`
chain. Falls back to the bulk path for SAB versions that pre-date
the nzo_ids filter — the transient-miss tolerance covers any
short-lived gap there too.
Implementation:
Lifted the album-bundle poll loop out of `usenet.py` and `torrent.py`
into `core/download_plugins/album_bundle.py:poll_album_download` —
near-duplicate implementations are now a single function with deps
injected so it's testable in isolation (kettui's extract-don't-AST-parse
standard; can't unit-test a `time.sleep` loop inside a plugin method).
The lifted helper takes:
- `get_status` callable bound to job_id, so the same loop works for
usenet UsenetStatus and torrent TorrentStatus shapes
- `complete_states` set so torrent's `{'seeding', 'completed'}` and
usenet's `{'completed'}` both Just Work
- `failed_states` set so torrent's `{'error'}` is terminal while
usenet's default-'error' fallback is transient
- `transient_miss_threshold` (default 5 ≈ 10s at 2s poll)
- `sleep` / `monotonic` injectables for deterministic tests
Per-track flows in both plugins gained the same transient-miss
tolerance inline — they don't use the emit pattern (update an
`active_downloads[id]` row dict via lock instead), so reusing the
helper would have required threading a no-op emit through. Inline
fix is small enough.
Tests:
- 11 new tests in `tests/test_album_bundle.py:poll_album_download`
cover the happy path, transient-miss tolerance with recovery,
hard-failure threshold, explicit-failed surface, timeout-emit,
default-'error' transient treatment, shutdown clean exit,
torrent's `seeding`-counts-as-complete, save_path captured across
iterations, and adapter-exception treated as transient miss.
- 521 download-suite tests pass (33 in test_album_bundle, others
pin existing torrent + usenet contracts).
- Ruff clean.
Closes#706.
Discogs uses two disambiguation conventions for duplicate artist names:
- legacy `(N)` numeric suffix: "Bullet (2)", "Madonna (3)"
- newer `*` asterisk suffix: "John Smith*", "Foo*"
Both were leaking through to the UI on artist search and album search,
and worse — through the import path into folder names on disk
(reported: importing yielded folders literally named `Foo*`).
The pre-existing cleanup only handled `(N)` and only at ONE site —
`get_user_collection` (line 469) and one path inside
`extract_track_from_release` (line 448 — `re.sub(r'\s*\(\d+\)$', '',
artist_name)`). Every other surface (artist search, album search,
album-track lookups, get_artist_albums feature matching) returned the
raw Discogs string.
Centralized into `_clean_discogs_artist_name(name)` at module top,
with regex covering both suffixes including repeated forms (`Baz**`,
`Foo (3)*`). Applied at six sites:
- `Artist.from_discogs_artist` (artist search)
- `Album.from_discogs_release` (album search — three fallbacks: array,
string, title-split)
- `Track.from_discogs_track` (track lookup — track-level + release-level
fallback)
- `extract_track_from_release` (replaces the inline `(N)`-only re.sub)
- `get_user_collection` (existing site, now also strips `*`)
- `get_artist_albums` (artist_name used for primary-vs-feature matching;
cleaning prevents `Beyoncé*` from failing equality vs `Beyoncé`)
- `get_album` (artists_list + per-track artists in the tracklist projection)
Tests:
- New `test_clean_discogs_artist_name` parametrized over 14 cases
covering `(N)`, `*`, repeated `**`, combined `(N) *`, whitespace
handling, empty/None defensive returns.
- New `test_get_user_collection_strips_discogs_asterisk_disambiguation`
pinning the asterisk path end-to-end through the collection import
flow (sibling to the existing `(N)` test).
- Existing 37 discogs tests still pass.
Out of scope (separate issue): the same #634 report flagged track-count
and year fields rendering as 0 / empty in Discogs album search. Both
are inherent to Discogs `/database/search` response shape — search
results don't carry `tracklist` (only release detail does) and `year`
is often `0` in search payloads. Fixing requires lazy-fetching release
detail per row, which hits the 25 req/min unauth limit hard. Not
bundled here.
Five pre-existing lint errors on dev baseline (all introduced May 25-26
before this branch was cut) were blocking CI on this PR. Cleared as
courtesy fixes so the merge isn't gated on unrelated tech debt:
- web_server.py:22613 — F811 duplicate `urlparse` import inside
`_parse_itunes_link_url` (already imported at module top, line 20).
Removed from the inline `from urllib.parse import parse_qs, urlparse`;
kept `parse_qs` since that one is only used here.
- core/listenbrainz_manager.py:746 — S110 silenced with
`# noqa: S110 — best-effort lookup, delete proceeds either way`.
Matches the existing project convention used in web_server.py:1693,
core/watchlist/auto_scan.py:463, core/library_reorganize.py:548.
- core/playlists/sources/listenbrainz.py:236 — B905 `zip()` without
explicit `strict=`. Added `strict=False` — preserves existing
behaviour where `matched` can legitimately be shorter than
`match_indices` on partial discover failure.
- core/playlists/sources/listenbrainz.py:273 — S110 silenced with
`# noqa: S110 — caller falls back to last cached playlist on
refresh failure`.
- core/playlists/sources/soulsync_discovery.py:105 — S110 silenced
with `# noqa: S110 — manager persists last_generation_error on
failure; surface existing snapshot`. The existing multi-line
comment that already explained the swallow was rolled into the
noqa justification so the rule + reason live on one line.
Ruff `python -m ruff check .` now passes; 664 discovery + metadata
tests still pass.
Live smoke against `/api/musicbrainz/search_tracks?track=Coffee+Break&artist=Zeds+Dead`
exposed the edge case the tiebreaker implementation couldn't reach:
The canonical Zeds Dead "Coffee Break" recording (mbid 6e2d4a70, length
184000ms) lives on the Coffee Break Single release — album_type='single',
which carries a 0.85 album_type_weight in `score_track`. A sibling
length-less recording (mbid 3b89bf3c) lives on an Album release —
album_type='album', weight 1.0. After multiplying by EXACT_ARTIST_BOOST
the canonical sat at 1.275 while the length-less sibling sat at 1.5.
The previous tiebreaker only kicked in on equal scores, so the
length-less album edition wins and the user sees 0:00 first instead of
the actionable 3:04 row. Bug reproduced: ordering came out
length-less / canonical / Omar-LinX-collab.
Switched `prefer_known_duration` to a 1.25x score boost on recordings
with non-zero duration_ms. The multiplier is sized above the
album-vs-single weight spread (0.176) so length-known recordings can
overcome an album-type penalty when scores would otherwise tie on
title + artist match, but stays small enough that cover/karaoke
penalty (0.05) and variant-tag penalty (0.85) still dominate — a
length-known tribute still loses to a length-less canonical.
Post-fix live response: 6e2d4a70 (canonical, 184000ms) sits first,
8ec2ce3f (Zeds Dead + Omar LinX collab, 153000ms) second, 3b89bf3c
(length-less album edition) third.
Verified Björk diacritic fallback path unaffected — `Bjork` + `Army of
Me` still cascades strict-empty → bare and returns all 10 Björk
recordings.
122 metadata tests pass — the three `prefer_known_duration` cases were
designed to pin behaviour, not the specific multiplier value, so they
all still pass under the boost implementation: ties promote
length-known, relevance still beats length-pref, default-off behaviour
unchanged.
Self-review pass on the prior three commits — kettui-style cleanup
that should have landed first time.
**Length-preference sort ordering (real bug):**
The `search_tracks_with_artist` stable sort that promoted length-known
recordings ran in `core/musicbrainz_search.py`, but the MB endpoint in
`web_server.py:search_musicbrainz_tracks` runs `rerank_tracks` after
it — which re-sorts by relevance score and dropped the length-pref
ordering down to tiebreaker-only. For canonical-same-song MB duplicates
that all score identically the tiebreaker survived, but the
order-of-operations was wrong.
Moved into `rerank_tracks` itself via a new `prefer_known_duration`
flag. Sort key sits between relevance score and the stable-order
tiebreaker so relevance still wins (length only decides ties, never
overrides a higher-relevance match). The MB endpoint opts in via
`prefer_known_duration=True`; Spotify / iTunes / Deezer callers stay
on the default-off path since their search results always include
length. Pinned with three new `TestRerankTracks` cases:
ties-promote-length, relevance-still-wins, default-off-unchanged.
**Route logic lifted to `core/discovery/manual_match.py`:**
Two pieces lived as inline route logic in `web_server.py` — the
`derive_manual_match_provider` fallback chain (payload.source →
active source → 'spotify') used by `update_youtube_discovery_match`,
and the `is_drifted_for_redo` predicate (cached provider differs from
active AND not manual_match) used by `prepare_mirrored_discovery`.
Per kettui's "extract logic from web_server.py, don't AST-parse it"
standard, both helpers now live in `core/discovery/manual_match.py`
with 12 dedicated unit tests covering fallback resolution order,
non-dict payload defenses, manual_match exemption from drift,
absent-provider legacy default, and edge cases.
Side benefits from the lift:
- `match_source` now derived once before the cache-save try block
instead of being duplicated in try + except (the except block existed
only because the original used `match_source` later — pre-computing
killed the duplication).
- `prepare_mirrored_discovery`'s `has_cached` check now reuses
`is_drifted_for_redo` with inverted polarity instead of restating
the field whitelist inline, so a future schema change only has to
land in one place.
- The mirrored-DB persist block now gates on `matched_data is not None`
to avoid a pre-existing latent NameError if the cache-save block
raised before matched_data construction.
**Enhanced toggle localStorage key now profile-scoped:**
`soulsync-library-view-mode` was global — two admin profiles would
share one preference. Wrapped in `_libraryViewModeKey()` which appends
`:${currentProfile.id}` when a profile is loaded, falls back to the
unsuffixed key otherwise (preserves pre-multi-profile saved values).
Tests:
- 12 new in `tests/discovery/test_manual_match.py` pinning both helpers.
- 3 new in `tests/metadata/test_relevance.py` pinning the
`prefer_known_duration` semantics.
- `test_search_tracks_with_artist_prefers_results_with_known_length`
renamed to `_does_not_resort_by_length` since the sort moved out of
this method. 664 tests pass across discovery + metadata suites.
User feedback: the Enhanced view toggle on the artist detail page reset
to Standard on every artist click, so admins who prefer Enhanced had to
re-flip the toggle every single time. Persist the choice in
localStorage and reapply on every artist navigation + page reload.
- `toggleEnhancedView()` writes `soulsync-library-view-mode` to
localStorage on every change.
- `navigateToArtistDetail()` reads the saved value after the standard
reset block runs; if `enhanced` AND `isEnhancedAdmin()` it calls
`toggleEnhancedView(true)` after `loadArtistDetailData` kicks off.
The brief Standard render is hidden as soon as the toggle flips.
- Gated on `isEnhancedAdmin()` so non-admin profiles (which never see
the toggle) can't end up with a stale Enhanced preference being
applied silently.
- Wrapped in try/catch since localStorage is unavailable in some
private-browsing modes.
No backend change; no DB migration needed.
User reported that manually mapping a mirrored-playlist track via the
Fix popup (either by search or by pasting an MBID) worked end-to-end
once — match saved, library track downloaded — but the next Playlist
Pipeline run flipped the track back to "Provider Changed" and forced
them to re-do the manual map every cycle.
Three independent issues were combining to cause this:
1. Hardcoded `provider: 'spotify'` on manual-fix save
`update_youtube_discovery_match` (the endpoint the Fix popup posts
to, also used by mirrored playlists since the frontend routes
`platform === 'mirrored'` through the YouTube endpoint) always
stamped the cached match as Spotify-provided. The Fix-popup cascade
actually queries the user's primary metadata source first and falls
back to Spotify / Deezer / iTunes / MusicBrainz — so a user on
MusicBrainz primary picking an MB result still had it saved as
`provider: 'spotify'`. The next prepare-discovery call (which
compares cached_provider to the active source) then immediately
classified the match as drifted and pending re-discovery. Fixed by
deriving `match_source` from `spotify_track.get('source')` (every
*_search_tracks endpoint stamps `source` on results) with a fallback
to `_get_active_discovery_source()` for the MBID-paste path (which
uses the lean flat shape that doesn't carry source). `matched_data['source']`
and the mirrored `extra_data['provider']` both now use the derived
value. `match_source` is also recomputed in the cache-save except
handler so the downstream mirrored-DB save still has it.
2. Discovery worker re-queueing manual matches as "incomplete"
`run_playlist_discovery_worker` in `core/discovery/playlist.py`
re-adds any track to `undiscovered_tracks` when its `matched_data`
lacks `track_number` or `album.id` / `album.release_date`. The
check was designed as a legacy-fix backfill for old discoveries
that lost those fields to a Track-dataclass stripping bug. But
manual fixes from the popup are *intentionally* lean — search-
result rows don't include `track_number` (none of the search
endpoints return it), and the MBID-lookup flat shape doesn't
carry `album.id` / `release_date` (the recording lookup returns
only `album.name`). So every manual match looked "incomplete" and
got re-discovered every pipeline run, overwriting the user's pick
with whatever the auto-search ranked first. Manual matches now
short-circuit ahead of the incomplete-data branch.
3. `prepare_mirrored_discovery` ignored the `manual_match` flag
Independent of the provider-stamping fix above, the prepare-
discovery endpoint that powers the mirrored-playlist UI did its
own `cached_provider != current_provider` check and didn't honour
manual_match either. Defence in depth — even if a future code
path stamps the wrong provider on a manual match, the flag now
anchors it as cached. `has_cached` also extended so manual
matches with off-provider stamps still count toward the cached
tally for phase classification.
Tests:
- new `test_manual_match_skipped_even_when_matched_data_incomplete`
in `tests/discovery/test_discovery_playlist.py` pins the worker
short-circuit using a realistic MB-shape matched_data (album dict
without id / release_date, no top-level track_number). 16 existing
tests still green; 848 across discovery / metadata / automation
suites pass.
`/api/musicbrainz/search_tracks` powers the Fix popup's auto-search
cascade for users on MusicBrainz as primary. When both track + artist
fields were filled, `search_tracks_with_artist` always took the bare
keyword path (`<track> <artist>` joined as one query string). MB's
recording-search scorer weights title matches far above artist matches,
so for "Coffee Break" + "Zeds Dead" the top results were Emapea / The
Vidalias / West One Orchestra's "Coffee Break" — three unrelated cover-
title collisions ahead of the canonical Zeds Dead recording. The
endpoint's `rerank_tracks` pass can't fix this when the right answer
is below the API's 50-result cutoff.
Both-fields mode now uses a strict field-scoped Lucene query first
(`recording:"<t>" AND artist:"<a>"`) which anchors the artist and
prunes title-collision covers at the source. `min_score=0` because the
field-scoped query is itself precise; rerank still does final ordering.
Bare query stays as the fallback when strict returns nothing — covers
the diacritic / alias cases the original `strict=False` path was added
for ("Bjork" query vs canonical "Björk" artist where Lucene phrase
match never hits the recording).
Single-field mode (track-only or artist-only) is unchanged: still bare-
query directly, since there's no artist value to anchor.
Also stable-sort results to prefer entries with non-zero `duration_ms`.
MB has multiple recordings per song (single release, album release,
remasters, compilations) and not every recording carries length data.
Without the preference sort, the user sees a 0:00 row first while a
sibling recording with the real 3:04 sits two rows below — matches the
report where MBID-paste lookup of the canonical recording (length 3:04)
contradicted the search-result's 0:00 row for the same song.
Tests:
- new `test_search_tracks_with_artist_strict_first_when_both_fields`
pins the strict=True call when both fields present
- new `test_search_tracks_with_artist_falls_back_to_bare_when_strict_empty`
pins the Björk-style fall-through path
- new `test_search_tracks_with_artist_prefers_results_with_known_length`
pins the length-preference sort
- existing `..._keeps_low_score_for_rerank` updated to side_effect so
the bare-fallback path is exercised; behaviour pinned identically
- existing `..._uses_bare_query_mode` renamed + repurposed for strict-
first; old name's behaviour no longer accurate
The sibling-merge aggregator from 7f751202 used "least-complete
phase wins", which made the modal appear frozen during parallel
album bundle downloads. The task table is phase-gated to
downloading/complete/error in downloads.js — so whenever any
sibling was still in album_downloading, the merged phase stayed
there and tasks for the sibling that had advanced past its bundle
never rendered. User reported: both albums downloading on slskd,
modal blank until one completes fully.
Flip the rule: surface the most-advanced live phase so the modal
renders task progress as soon as any sibling reaches it. The
all-siblings-in-album_downloading case still surfaces
album_downloading (bundle progress UI is correct there); error
stays sticky.
Updated WHATS_NEW under 2.6.3 to describe the corrected behavior.
Two new tests pin the regression:
- downloading + album_downloading → downloading
- album_downloading + album_downloading → album_downloading
Phase 1c.2.1 splits each wishlist run across multiple
``download_batches`` rows (per-album bundle dispatch). The
download-missing modal opens against the original batch_id
allocated by ``start_manual_wishlist_download_batch`` /
``process_wishlist_automatically``. Pre-fix that batch_id was
just one sibling among N, so the modal went stale as soon as the
primary sub-batch finished — subsequent albums downloaded fine
but no live status reached the UI.
Fix: backend merges every sibling sub-batch's tasks +
analysis_results into the response keyed under the originally-
requested batch_id. Modal sees one unified view of the whole run
without knowing about the split. Frontend untouched.
Architecture (Kettui standards):
- ``core/downloads/wishlist_aggregator.py`` — pure
``merge_wishlist_run_status(primary, siblings)`` helper.
No IO, no runtime state, no globals. Lifted out of
``status.py`` so the merge contract can be pinned via unit
tests without standing up the live ``download_batches`` /
``download_tasks`` state.
- ``core/downloads/status.py``'s ``build_batched_status`` now
pre-indexes ``download_batches`` by ``wishlist_run_id`` inside
the existing ``tasks_lock`` snapshot, then runs the merge
helper whenever a requested batch has a sibling.
Merge rules pinned by 12 tests:
- ``track_index`` re-indexed globally 0..N-1 across the merged
``analysis_results`` so the modal's ``data-track-index`` DOM
keys don't collide between siblings. Tasks' ``track_index``
follows the same remap so the analysis-results ↔ tasks
cross-reference stays intact.
- ``task_id`` is uuid per task — no collision concern.
- Phase: error is sticky; otherwise the LEAST-complete
pre-terminal phase wins (analysis < album_downloading <
downloading). All-complete returns ``complete``; mixed
complete + active returns ``downloading`` so the modal stays
alive until every sibling lands.
- ``album_bundle``: picks whichever sibling currently has an
active bundle download (state in
``{searching, downloading, downloading_release, staging}``).
Falls back to the first non-empty bundle so a completed run
still shows a progress bar.
- ``analysis_progress`` summed across siblings.
- ``active_count`` summed; ``max_concurrent`` keeps primary's
value as the representative.
- ``playlist_id`` + ``playlist_name`` preserved from the primary
(the row the modal originally opened against).
Legacy single-batch wishlist runs (no ``wishlist_run_id`` on the
batch) skip the merge entirely — passthrough. Back-compat by
absence.
1108 tests across downloads + wishlist + automation + imports +
playlist-sources + lb-series suites green. 12 new aggregator
tests pin the merge contract.
Closes the open UX gap from the Phase 1c.2.1 ship — modal now
tracks every sibling sub-batch's progress for the full duration
of the wishlist run.
Phase 1c.2.1 splits each wishlist invocation into per-album sub-
batches so the album-bundle dispatch can engage once per album.
Side effect: the completion handler ``finalize_auto_wishlist_completion``
ran end-of-run logic (cycle toggle + state reset + automation
event emit) once per BATCH, so a 2-album run fired the cycle
toggle twice + emitted two ``wishlist_processing_completed``
events. The cycle landed at the right value either way but the
state machine had become per-batch instead of per-run.
Fix: reify "wishlist run" as a first-class concept via a shared
``wishlist_run_id`` UUID. Generated once per wishlist invocation
in both the auto- and manual-wishlist paths, stamped on every
sub-batch row in ``download_batches``.
``finalize_auto_wishlist_completion`` now reads the completing
batch's ``wishlist_run_id`` and, when present, scans
``download_batches`` for siblings still in pre-terminal phases.
If any sibling is still active, the per-batch summary records
but the cycle toggle + state reset + automation emit are
deferred. Only the last completing sibling fires the run-level
finalization. Legacy single-batch runs (no run_id field) keep
their toggle-immediately behavior — back-compat by absence.
The run_id also lays groundwork for frontend grouping (one
logical row in the Downloads view per wishlist run instead of N
sibling rows), but that UX work is deferred.
3 new tests in ``test_processing.py`` pin: defer-when-siblings-
active, toggle-when-last-sibling-done, back-compat-without-run_id.
1 new assertion in ``test_automation.py`` confirms all sub-batches
of one auto-wishlist invocation share the same run_id. 309 tests
across wishlist + automation suites green.
Notes: dispatch concurrency unchanged — sub-batches still run via
the shared download worker pool. Slskd serializes per-uploader at
its own layer (same uploader = automatic queue, different
uploaders = legit parallel), so SoulSync-side serial enforcement
would duplicate work the right layer already handles.
The Phase-1 fix (commit c3b88e69) only extended the per-album
bundle dispatch to ``process_wishlist_automatically``. The manual
"Run Wishlist Now" path goes through
``_prepare_and_run_manual_wishlist_batch`` instead, so the
behavior didn't change for users who triggered downloads from the
Wishlist tab UI — they still saw N per-track Soulseek searches
when N missing tracks all came from one album.
Caught in a real-app test: user added Katy Perry's PRISM (Deluxe)
to the wishlist + clicked "Download Wishlist" → app log shows
``_prepare_and_run_manual_wishlist_batch:421`` running a single
batch with 16 tracks + per-track searches firing one by one
("katy perry prism deluxe legendary lovers", "katy perry prism
deluxe roar", etc.), no album-bundle dispatch.
Fix:
- ``_prepare_and_run_manual_wishlist_batch`` now runs the same
``group_wishlist_tracks_by_album`` helper after filtering. For
each detected album, it builds a sub-batch with
``is_album_download=True`` + populated album/artist context.
Residual tracks (no resolvable album metadata) land in a single
per-track residual batch.
- The first sub-batch re-uses the caller-allocated ``batch_id``
so the frontend's existing poll against it keeps working;
additional sub-batches get fresh ids materialized into
``download_batches`` so they show up in the Downloads view.
- Sub-batches dispatch serially — each ``run_full_missing_tracks_process``
call blocks until the album-bundle staging + per-track tasks
complete before the next album's bundle search fires.
New test ``test_manual_wishlist_splits_into_per_album_sub_batches``
pins the contract — multi-album wishlist content with
nested-spotify_data shape produces N master-worker calls (one per
album), each batch carries the album_context, first sub-batch
re-uses the original batch_id. 106 wishlist tests + 1099 across
the broader suite green.
Adding 16 Katy Perry PRISM tracks to wishlist + clicking download
should now fire ONE slskd album-bundle search for the release
instead of 16 individual searches.
Auto-wishlist's "albums" cycle used to dump every missing album
track into one batch and run per-track Soulseek / Prowlarr searches
for each (~50 searches for a typical scan). The album-bundle
dispatch (introduced in 2.5.9 for explicit album downloads) was
gated on ``is_album_download=True`` + populated
``album_context``/``artist_context``, none of which the wishlist
batch ever set — so wishlist runs always took the per-track flow
even when 12 missing tracks all belonged to the same album.
Fix: split wishlist albums-cycle tracks into per-album sub-batches
at submission time. Each sub-batch carries its own album context,
trips the existing dispatch gate, and engages one slskd / torrent
/ usenet album-bundle search per album. Tracks the helper can't
group (no album metadata, no artist) fall through to a residual
per-track batch.
- New ``core/wishlist/album_grouping.py``:
``group_wishlist_tracks_by_album(tracks)`` returns
``WishlistGroupingResult(album_groups, residual_tracks)``.
Pure function — extracts album_id (or name-normalized fallback)
+ primary artist + album context from each track's nested
spotify_data, buckets, and threshold-promotes. Independent of
runtime state so it can be unit-tested without the wishlist
executor.
- ``core/wishlist/processing.py``: when ``current_cycle ==
'albums'``, run the grouping helper, submit one batch per album
with ``is_album_download=True`` + the group's album/artist
context, then a single residual batch for orphans. Singles
cycle path unchanged.
- 9 new tests in ``test_album_grouping.py`` pin the bucketing
contract (empty / single album / multi album / orphan / threshold
/ nested payloads / no-id fallback / no artist).
- 2 new tests in ``test_automation.py`` exercise the per-album
split end-to-end through ``process_wishlist_automatically``:
multi-album batch → two sub-batches each with album context;
mixed orphan + real album → one bundle batch + one residual.
1099 tests across wishlist + imports + downloads + automation +
playlist-sources + staging-provenance + track-number-repair
suites green. WHATS_NEW entry added under 2.6.3.
Now when an auto-wishlist scan finds 12 missing tracks from
Ryoto's "Cha-La Head-Cha-La", it runs ONE slskd / Prowlarr
album-bundle search for the release instead of 12 per-track
searches.
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.
The bulk rolling-mirror ensure path was instrumented with INFO
lines + a WARNING on SELECT failure (commit 5378b726) while we
chased down why only one rolling mirror was being created — turned
out the issue was simply needing two refresh cycles after the
rolling code shipped. Diagnostic served its purpose, removing
the noise from every LB refresh now.
- Dropped per-walk + per-match + summary INFO lines from
``_ensure_rolling_mirrors_from_cache`` — the loop is silent.
- Reverted the outer SELECT failure catch from ``logger.warning``
back to ``logger.debug``.
- Kept the per-placeholder ``Pre-created rolling mirror
placeholder`` INFO line in ``_ensure_rolling_series_mirror``
since it's a genuine one-shot event (only fires when a new
placeholder is actually inserted, not on every refresh).
BoulderBadgeDad