From f55a4fcf7210c7f6f01d6c11b20de1e1387b2050 Mon Sep 17 00:00:00 2001 From: dev Date: Sun, 14 Jun 2026 21:36:27 +0200 Subject: [PATCH] fix(downloads): stop active tasks from starving terminal rows out of the list MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Root cause of "completed/failed/unverified don't show during a running batch, only after it ends" (F5 didn't help): build_unified_downloads_response sorted live tasks active-first (downloading/searching/queued = priority 0-3, completed/failed = 4-7) then truncated the whole array at items[:limit] (300). During a busy batch the active+queued tasks filled the limit and pushed every terminal task off the end, so /api/downloads/all never returned them — the Completed/Failed/Unverified tabs filter client-side and had nothing to show. Fix: `limit` now bounds only the persistent-history tail. Live in-memory tasks are always returned in full — they're already bounded by the 5-min cleanup automation, and array order is presentation-only since the page filters per tab client-side. Verified with a repro (320 queued + 1 completed + 1 failed → terminal rows were absent at limit=300; now present). Co-Authored-By: Claude Opus 4.8 --- core/downloads/status.py | 11 ++++++-- tests/downloads/test_downloads_status.py | 35 ++++++++++++++++++++++-- 2 files changed, 41 insertions(+), 5 deletions(-) diff --git a/core/downloads/status.py b/core/downloads/status.py index 4e4fec84..2ac69ed9 100644 --- a/core/downloads/status.py +++ b/core/downloads/status.py @@ -839,7 +839,14 @@ def build_unified_downloads_response(limit: int, deps: StatusDeps) -> dict: live_identities.add(identity) appended_history += 1 - # Sort: active first (by priority), then by timestamp desc within each group + # Sort: active first (by priority), then by timestamp desc within each group. + # NOTE: the array order is presentation-only — the Downloads page filters + # client-side per tab. What matters is that EVERY live task is present: an + # earlier `items[:limit]` truncation (active-first) starved completed/failed/ + # unverified rows off the end during a busy batch, so those tabs stayed empty + # until the batch drained. `limit` now bounds only the persistent-history + # tail (handled above); live in-memory tasks are always returned in full + # (they're already bounded by the 5-min cleanup automation). items.sort(key=lambda x: (x['priority'], -x['timestamp'])) # Build batch summaries for the batch context panel @@ -867,7 +874,7 @@ def build_unified_downloads_response(limit: int, deps: StatusDeps) -> dict: return { 'success': True, - 'downloads': items[:limit], + 'downloads': items, 'total': len(items), 'batches': batch_summaries, 'timestamp': time.time(), diff --git a/tests/downloads/test_downloads_status.py b/tests/downloads/test_downloads_status.py index 7050e3e6..4e56b016 100644 --- a/tests/downloads/test_downloads_status.py +++ b/tests/downloads/test_downloads_status.py @@ -667,15 +667,44 @@ def test_unified_response_includes_batch_summaries(): assert bs['queued'] == 1 -def test_unified_response_respects_limit(): +def test_unified_response_returns_all_live_tasks_even_past_limit(): + """`limit` bounds the persistent-history tail, NOT live in-memory tasks. + + Live tasks are already bounded by the 5-min cleanup, and the Downloads page + filters them client-side per tab — truncating them (active-first) starved + completed/failed/unverified rows out of the response during a busy batch so + they never showed until the batch drained. + """ deps, _ = _build_deps() for i in range(20): download_tasks[f't{i}'] = { 'track_index': i, 'status': 'completed', 'track_info': {}, } out = st.build_unified_downloads_response(5, deps) - assert len(out['downloads']) == 5 - assert out['total'] == 20 # total still reflects all + assert len(out['downloads']) == 20 # all live tasks returned + assert out['total'] == 20 + + +def test_unified_response_does_not_truncate_terminal_tasks_behind_active(): + """A busy batch (many queued/active tasks) must not push completed/failed + rows off the end of the response — they're what the Completed/Failed tabs + show during the run.""" + deps, _ = _build_deps() + for i in range(120): + download_tasks[f'q{i}'] = { + 'track_index': i, 'status': 'queued', + 'track_info': {'name': f'Q{i}'}, 'status_change_time': i, + } + download_tasks['done'] = { + 'status': 'completed', 'track_info': {'name': 'DONE'}, 'status_change_time': 999, + } + download_tasks['fail'] = { + 'status': 'failed', 'track_info': {'name': 'FAIL'}, 'status_change_time': 999, + } + out = st.build_unified_downloads_response(100, deps) + titles = {d['title'] for d in out['downloads']} + assert 'DONE' in titles + assert 'FAIL' in titles def test_unified_response_includes_persistent_download_history():