From 6841128dc2a08c5b640d6878d882b98f70490b29 Mon Sep 17 00:00:00 2001
From: Broque Thomas <26755000+Nezreka@users.noreply.github.com>
Date: Wed, 27 May 2026 14:52:02 -0700
Subject: [PATCH] Wishlist: distinguish Queued from Analyzing for
executor-pending batches
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
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.
---
core/downloads/status.py | 10 +++++++++-
core/wishlist/processing.py | 18 ++++++++++++++++--
tests/downloads/test_downloads_status.py | 19 +++++++++++++++++++
tests/wishlist/test_automation.py | 7 ++++++-
webui/static/downloads.js | 13 ++++++++++++-
webui/static/helper.js | 1 +
webui/static/pages-extra.js | 11 ++++++++++-
7 files changed, 73 insertions(+), 6 deletions(-)
diff --git a/core/downloads/status.py b/core/downloads/status.py
index 41229e37..2d2073db 100644
--- a/core/downloads/status.py
+++ b/core/downloads/status.py
@@ -265,7 +265,15 @@ def build_batch_status_data(batch_id: str, batch: dict, live_transfers_lookup: d
if album_bundle:
response_data['album_bundle'] = album_bundle
- if response_data["phase"] == 'analysis':
+ if response_data["phase"] in ('analysis', 'queued'):
+ # Surface analysis_progress for both phases — ``queued`` rows
+ # haven't started analysis yet (processed=0) but the UI still
+ # needs ``total`` so it can render "Queued — N tracks". The
+ # master worker flips phase from ``queued`` to ``analysis`` on
+ # entry (see ``core/downloads/master.py:328``); the wishlist
+ # submission sites pre-populate ``analysis_total`` so the
+ # queued state isn't shape-mismatched against the analysis
+ # state for downstream consumers.
response_data['analysis_progress'] = {
'total': batch.get('analysis_total', 0),
'processed': batch.get('analysis_processed', 0),
diff --git a/core/wishlist/processing.py b/core/wishlist/processing.py
index 2bc7fcb3..48d9126d 100644
--- a/core/wishlist/processing.py
+++ b/core/wishlist/processing.py
@@ -857,7 +857,19 @@ def process_wishlist_automatically(runtime: WishlistAutoProcessingRuntime, autom
)
with runtime.tasks_lock:
runtime.download_batches[album_batch_id] = {
- 'phase': 'analysis',
+ # ``queued`` until the master worker
+ # picks the batch up from the
+ # ``missing_download_executor`` pool
+ # (max_workers=3 by default). The worker
+ # flips phase to ``analysis`` as its
+ # first action — see
+ # ``core/downloads/master.py:328``.
+ # Pre-fix the row was created with
+ # ``analysis`` directly, so a wishlist
+ # run with N > 3 sub-batches looked like
+ # all N were working when really only
+ # 3 were running.
+ 'phase': 'queued',
'playlist_id': playlist_id,
'playlist_name': album_batch_name,
'queue': [],
@@ -904,7 +916,9 @@ def process_wishlist_automatically(runtime: WishlistAutoProcessingRuntime, autom
playlist_name = f"Wishlist (Auto - {current_cycle.capitalize()})"
with runtime.tasks_lock:
runtime.download_batches[batch_id] = {
- 'phase': 'analysis',
+ # See album sub-batch above — ``queued``
+ # until the master worker picks it up.
+ 'phase': 'queued',
'playlist_id': playlist_id,
'playlist_name': playlist_name,
'queue': [],
diff --git a/tests/downloads/test_downloads_status.py b/tests/downloads/test_downloads_status.py
index 64d41b3d..522137e5 100644
--- a/tests/downloads/test_downloads_status.py
+++ b/tests/downloads/test_downloads_status.py
@@ -91,6 +91,25 @@ def test_analysis_phase_includes_analysis_progress_and_results():
assert out['analysis_results'] == [{'track_index': 0, 'found': True}]
+def test_queued_phase_surfaces_analysis_progress_for_ui_count():
+ """A batch in ``queued`` state hasn't been picked up by the
+ executor yet, so analysis_processed is 0. The UI still needs
+ ``analysis_total`` so it can render "Queued — N tracks" instead
+ of showing an empty card. Pre-fix the queued phase fell through
+ to the default branch and the UI lost the track count entirely."""
+ deps, _ = _build_deps()
+ batch = {
+ 'phase': 'queued',
+ 'analysis_total': 17,
+ 'analysis_processed': 0,
+ 'analysis_results': [],
+ }
+ out = st.build_batch_status_data('b1', batch, {}, deps)
+ assert out['phase'] == 'queued'
+ assert out['analysis_progress'] == {'total': 17, 'processed': 0}
+ assert out['analysis_results'] == []
+
+
def test_album_downloading_phase_exposes_bundle_progress_without_task_safety_valve():
deps, _ = _build_deps(config=_FakeConfig({'soulseek.download_timeout': 1}))
download_tasks['t1'] = {
diff --git a/tests/wishlist/test_automation.py b/tests/wishlist/test_automation.py
index 1082d23b..f4453b8a 100644
--- a/tests/wishlist/test_automation.py
+++ b/tests/wishlist/test_automation.py
@@ -228,7 +228,12 @@ def test_process_wishlist_automatically_creates_batch_for_matching_tracks():
assert submitted_args[2][0]["_original_index"] == 0
assert len(batch_map) == 1
batch = next(iter(batch_map.values()))
- assert batch["phase"] == "analysis"
+ # ``queued`` is the initial state — the master worker flips it
+ # to ``analysis`` as its first action when the executor picks
+ # the batch up. Without this, wishlist runs with N > 3
+ # sub-batches all rendered "Analyzing..." simultaneously even
+ # though only 3 workers were running (UI lie).
+ assert batch["phase"] == "queued"
assert batch["playlist_name"] == "Wishlist (Auto - Albums)"
assert batch["analysis_total"] == 1
assert any(kwargs.get("progress") == 50 for _args, kwargs in progress_calls)
diff --git a/webui/static/downloads.js b/webui/static/downloads.js
index 18798508..c44cce45 100644
--- a/webui/static/downloads.js
+++ b/webui/static/downloads.js
@@ -3484,7 +3484,18 @@ function processModalStatusUpdate(playlistId, data) {
// Note: Wishlist modal visibility is now managed by handleWishlistButtonClick() only
// Auto-show logic has been simplified to prevent conflicts
- if (data.phase === 'analysis') {
+ if (data.phase === 'queued') {
+ // Submitted to the executor but no worker has picked it up yet.
+ // ``missing_download_executor`` is bounded (max_workers=3 by
+ // default) so wishlist runs with N > 3 sub-batches park the
+ // rest at this phase. Show distinct text so users don't think
+ // 26 batches are all in-flight at once.
+ const total = data.analysis_progress?.total || 0;
+ const elText = document.getElementById(`analysis-progress-text-${playlistId}`);
+ const elFill = document.getElementById(`analysis-progress-fill-${playlistId}`);
+ if (elText) elText.textContent = `Queued — waiting for worker (${total} tracks)`;
+ if (elFill) elFill.style.width = '0%';
+ } else if (data.phase === 'analysis') {
const progress = data.analysis_progress;
const percent = progress.total > 0 ? (progress.processed / progress.total) * 100 : 0;
document.getElementById(`analysis-progress-fill-${playlistId}`).style.width = `${percent}%`;
diff --git a/webui/static/helper.js b/webui/static/helper.js
index 1861f23e..8fc9419e 100644
--- a/webui/static/helper.js
+++ b/webui/static/helper.js
@@ -3415,6 +3415,7 @@ function closeHelperSearch() {
const WHATS_NEW = {
'2.6.3': [
{ unreleased: true },
+ { title: 'Wishlist: distinguish Queued from Analyzing batches in the UI', desc: 'wishlist runs with more than 3 sub-batches used to render every batch as "Analyzing..." simultaneously — even though the download worker pool only runs 3 at a time. so a 26-album wishlist scan looked like all 26 were working in parallel when really 23 were just sitting in the executor queue waiting their turn. now batches show a distinct "Queued ⏳" state while parked in the executor queue; they flip to "Analyzing..." when a worker actually picks them up. zero behavior change — just less misleading UI.' },
{ title: 'Wishlist: only engage album-bundle when several tracks from the same album are missing', desc: 'auto-wishlist + manual-wishlist runs were promoting every single-track wishlist item to a per-album bundle search — so a wishlist of "26 single tracks from 26 different albums" downloaded full albums (5-42 files each, ~85% wasted bandwidth) and hammered slskd with concurrent searches. now the bundle path only engages when an album has 2 or more missing tracks in the wishlist; single-track items take the cheaper per-track path that already works fine. fewer slskd searches per cycle, less wasted bandwidth, no more downloading the same album three times in a row when staging-match misses a single track. configurable via `wishlist.album_bundle_min_tracks` if you want the old behaviour (set to 1) or stricter (set to 3+).' },
{ title: 'Auto-Sync: schedule playlists by weekday + time, not just by hour interval', desc: 'new Weekly Board tab on the Auto-Sync manager. drag a playlist onto a day column (Mon-Sun) to schedule it for that weekday at the default time, then click the card to open an editor for multi-day picks, custom time, and timezone. existing hourly schedules stay on the Hourly Board tab — one playlist gets exactly one schedule so swapping between weekly and hourly auto-replaces the old one. timezone defaults to whatever your browser reports (e.g. America/Los_Angeles); the editor accepts any IANA tz string. backend already had the plumbing from earlier 2.6.3 commits — this lights up the UI.', page: 'automations' },
{ title: 'Fix: usenet album downloads stuck on "downloading release" even after SAB finished', desc: 'when SAB moves a job from its queue into history (post-processing handoff — verify + unrar window), there\'s a brief gap where the job is in neither. SoulSync\'s poll treated one missing read as terminal failure and gave up silently, leaving the download modal frozen on "downloading release" forever. SAB queue states like `Pp` (post-processing) also weren\'t in the state map, hit the default-error fallback, and hung the poll the same way. now the poll tolerates ~10 seconds of transient misses, treats unmapped queue states as transient instead of silently looping, fires a terminal failed event on every failure path so the modal exits the downloading state, and queries SAB by nzo_id directly instead of paging the last-50-entries history (avoids losing the job on busy servers). same tolerance applies to torrent downloads — covers network blips that used to kill an otherwise healthy download.' },
diff --git a/webui/static/pages-extra.js b/webui/static/pages-extra.js
index a10a7df5..6444db46 100644
--- a/webui/static/pages-extra.js
+++ b/webui/static/pages-extra.js
@@ -2603,7 +2603,16 @@ function _adlRenderBatchPanel() {
// Phase label with icon
let phaseText = '';
let phaseIcon = '';
- if (batch.phase === 'analysis') {
+ if (batch.phase === 'queued') {
+ // Batch is in the executor queue waiting for a worker slot.
+ // ``missing_download_executor`` has max_workers=3 by default,
+ // so wishlist runs with >3 sub-batches park the rest at this
+ // state until a worker frees up. Pre-fix this status rendered
+ // as "Analyzing..." which misled users into thinking 26
+ // batches were all working when really only 3 were running.
+ phaseText = 'Queued';
+ phaseIcon = '⏳';
+ } else if (batch.phase === 'analysis') {
phaseText = 'Analyzing...';
phaseIcon = '';
} else if (batch.phase === 'album_downloading') {