fix(downloads): stop active tasks from starving terminal rows out of the list
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 <noreply@anthropic.com>
This commit is contained in:
parent
e594ac9799
commit
f55a4fcf72
2 changed files with 41 additions and 5 deletions
|
|
@ -839,7 +839,14 @@ def build_unified_downloads_response(limit: int, deps: StatusDeps) -> dict:
|
||||||
live_identities.add(identity)
|
live_identities.add(identity)
|
||||||
appended_history += 1
|
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']))
|
items.sort(key=lambda x: (x['priority'], -x['timestamp']))
|
||||||
|
|
||||||
# Build batch summaries for the batch context panel
|
# Build batch summaries for the batch context panel
|
||||||
|
|
@ -867,7 +874,7 @@ def build_unified_downloads_response(limit: int, deps: StatusDeps) -> dict:
|
||||||
|
|
||||||
return {
|
return {
|
||||||
'success': True,
|
'success': True,
|
||||||
'downloads': items[:limit],
|
'downloads': items,
|
||||||
'total': len(items),
|
'total': len(items),
|
||||||
'batches': batch_summaries,
|
'batches': batch_summaries,
|
||||||
'timestamp': time.time(),
|
'timestamp': time.time(),
|
||||||
|
|
|
||||||
|
|
@ -667,15 +667,44 @@ def test_unified_response_includes_batch_summaries():
|
||||||
assert bs['queued'] == 1
|
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()
|
deps, _ = _build_deps()
|
||||||
for i in range(20):
|
for i in range(20):
|
||||||
download_tasks[f't{i}'] = {
|
download_tasks[f't{i}'] = {
|
||||||
'track_index': i, 'status': 'completed', 'track_info': {},
|
'track_index': i, 'status': 'completed', 'track_info': {},
|
||||||
}
|
}
|
||||||
out = st.build_unified_downloads_response(5, deps)
|
out = st.build_unified_downloads_response(5, deps)
|
||||||
assert len(out['downloads']) == 5
|
assert len(out['downloads']) == 20 # all live tasks returned
|
||||||
assert out['total'] == 20 # total still reflects all
|
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():
|
def test_unified_response_includes_persistent_download_history():
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue