diff --git a/backend/app/api/downloads.py b/backend/app/api/downloads.py index f42cc09..06d96f6 100644 --- a/backend/app/api/downloads.py +++ b/backend/app/api/downloads.py @@ -114,6 +114,109 @@ async def get_download(download_id: int): return jsonify(response) +def _plan_bulk_retry(sorted_failures, sources_by_id, recent_cutoff): + """Decide which failed downloads to retry given per-source dedup + recent-success filter. + + Args: + sorted_failures: Download rows sorted by (source_id ASC, created_at DESC) — + the first occurrence of each source_id is the most recent failure. + sources_by_id: dict mapping source_id -> Source (for `last_success` lookup). + recent_cutoff: datetime — any source whose last_success is >= this is skipped. + + Returns (retry_ids, summary) where summary has skip counts per reason. + """ + seen_sources = set() + retry_ids = [] + skipped_dup = 0 + skipped_recent = 0 + skipped_no_source = 0 + + for failure in sorted_failures: + if failure.source_id is None: + skipped_no_source += 1 + continue + if failure.source_id in seen_sources: + skipped_dup += 1 + continue + seen_sources.add(failure.source_id) + + source = sources_by_id.get(failure.source_id) + if source and source.last_success and source.last_success >= recent_cutoff: + skipped_recent += 1 + continue + + retry_ids.append(failure.id) + + return retry_ids, { + "retried": len(retry_ids), + "skipped_duplicate_source": skipped_dup, + "skipped_recent_success": skipped_recent, + "skipped_no_source": skipped_no_source, + } + + +@bp.route("/retry-failed-bulk", methods=["POST"]) +async def retry_failed_bulk(): + """Bulk-retry non-superseded failed downloads, with smart dedup. + + Applies two filters that the old per-row Dashboard loop missed: + 1. Per-source dedup — a source with N historical failures enqueues once. + 2. Recent-success skip — a source whose `last_success` is within the + recent window (default 24h, overridable via `recent_window_hours`) + is considered healthy; we leave its stale failures alone rather + than pile on redundant retry work. + """ + data = await request.get_json() or {} + recent_window_hours = max(0, int(data.get("recent_window_hours", 24))) + recent_cutoff = utcnow() - timedelta(hours=recent_window_hours) + + async with current_app.db_engine.connect() as conn: + async with AsyncSession(bind=conn) as session: + failures_result = await session.execute( + select(Download) + .where( + Download.status == DownloadStatus.FAILED, + Download.superseded == False, + ) + .order_by(Download.source_id.asc(), Download.created_at.desc()) + ) + failures = list(failures_result.scalars().all()) + + source_ids = {f.source_id for f in failures if f.source_id is not None} + sources_by_id = {} + if source_ids: + sources_result = await session.execute( + select(Source).where(Source.id.in_(source_ids)) + ) + sources_by_id = {s.id: s for s in sources_result.scalars().all()} + + retry_ids, summary = _plan_bulk_retry(failures, sources_by_id, recent_cutoff) + + for failure in failures: + if failure.id in retry_ids: + failure.status = DownloadStatus.QUEUED + failure.error_type = None + failure.error_message = None + failure.started_at = None + failure.completed_at = None + + await session.commit() + + for download_id in retry_ids: + process_download.delay(download_id) + + current_app.logger.info( + f"retry-failed-bulk: {summary} (recent_window_hours={recent_window_hours})" + ) + + return jsonify({ + "message": f"Queued {summary['retried']} retries", + **summary, + "total_failures_considered": len(failures), + "recent_window_hours": recent_window_hours, + }) + + @bp.route("//retry", methods=["POST"]) async def retry_download(download_id: int): """Retry a failed download.""" diff --git a/backend/tests/api/__init__.py b/backend/tests/api/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/backend/tests/api/test_retry_failed_bulk.py b/backend/tests/api/test_retry_failed_bulk.py new file mode 100644 index 0000000..0da3f29 --- /dev/null +++ b/backend/tests/api/test_retry_failed_bulk.py @@ -0,0 +1,124 @@ +"""Unit tests for the planner helper behind POST /downloads/retry-failed-bulk. + +The helper decides which non-superseded failed downloads get re-queued, with +per-source dedup and a "recent success" skip window. These tests exercise the +dedup + filter logic on plain objects (no DB). +""" +from datetime import datetime, timedelta, timezone +from types import SimpleNamespace + +from app.api.downloads import _plan_bulk_retry + + +def _failure(failure_id, source_id, created_at): + return SimpleNamespace(id=failure_id, source_id=source_id, created_at=created_at) + + +def _source(source_id, last_success): + return SimpleNamespace(id=source_id, last_success=last_success) + + +# Tests expect failures already sorted (source_id ASC, created_at DESC) — same +# ordering the endpoint's SQL query produces. + +def test_no_failures_returns_empty_plan(): + retry_ids, summary = _plan_bulk_retry([], {}, datetime.now(timezone.utc)) + assert retry_ids == [] + assert summary == { + "retried": 0, + "skipped_duplicate_source": 0, + "skipped_recent_success": 0, + "skipped_no_source": 0, + } + + +def test_single_failure_retried_when_source_never_succeeded(): + now = datetime.now(timezone.utc) + failures = [_failure(1, source_id=10, created_at=now - timedelta(hours=48))] + sources = {10: _source(10, last_success=None)} + + retry_ids, summary = _plan_bulk_retry(failures, sources, now - timedelta(hours=24)) + + assert retry_ids == [1] + assert summary["retried"] == 1 + + +def test_dedup_keeps_only_most_recent_per_source(): + """Given 3 failures for same source (sorted by created_at DESC), only the first survives.""" + now = datetime.now(timezone.utc) + failures = [ + _failure(301, source_id=10, created_at=now - timedelta(hours=1)), # most recent + _failure(302, source_id=10, created_at=now - timedelta(hours=24)), + _failure(303, source_id=10, created_at=now - timedelta(days=7)), + ] + sources = {10: _source(10, last_success=None)} + + retry_ids, summary = _plan_bulk_retry(failures, sources, now - timedelta(hours=24)) + + assert retry_ids == [301] + assert summary["skipped_duplicate_source"] == 2 + + +def test_skip_source_with_recent_success(): + """Source that succeeded 2 hours ago should be skipped against a 24h window.""" + now = datetime.now(timezone.utc) + failures = [_failure(1, source_id=10, created_at=now - timedelta(days=7))] + sources = {10: _source(10, last_success=now - timedelta(hours=2))} + + retry_ids, summary = _plan_bulk_retry(failures, sources, now - timedelta(hours=24)) + + assert retry_ids == [] + assert summary["skipped_recent_success"] == 1 + + +def test_retry_source_with_old_success(): + """Source that succeeded 48h ago should still retry against a 24h window.""" + now = datetime.now(timezone.utc) + failures = [_failure(1, source_id=10, created_at=now - timedelta(hours=1))] + sources = {10: _source(10, last_success=now - timedelta(hours=48))} + + retry_ids, summary = _plan_bulk_retry(failures, sources, now - timedelta(hours=24)) + + assert retry_ids == [1] + assert summary["skipped_recent_success"] == 0 + + +def test_orphaned_failure_without_source_id_is_skipped_separately(): + """Failures with source_id=None shouldn't explode — tallied under a distinct bucket.""" + now = datetime.now(timezone.utc) + failures = [ + _failure(1, source_id=None, created_at=now), + _failure(2, source_id=10, created_at=now), + ] + sources = {10: _source(10, last_success=None)} + + retry_ids, summary = _plan_bulk_retry(failures, sources, now - timedelta(hours=24)) + + assert retry_ids == [2] + assert summary["skipped_no_source"] == 1 + assert summary["skipped_duplicate_source"] == 0 + + +def test_mixed_scenario_combines_all_rules(): + """One healthy source (recent success, skipped), one broken source (multiple failures, dedup'd).""" + now = datetime.now(timezone.utc) + failures = [ + _failure(101, source_id=10, created_at=now - timedelta(hours=1)), # healthy source, skip + _failure(201, source_id=20, created_at=now - timedelta(hours=2)), # broken, pick this one + _failure(202, source_id=20, created_at=now - timedelta(days=1)), # broken, dedup + _failure(203, source_id=20, created_at=now - timedelta(days=7)), # broken, dedup + ] + sources = { + 10: _source(10, last_success=now - timedelta(hours=3)), # healthy + 20: _source(20, last_success=None), # never succeeded + } + + retry_ids, summary = _plan_bulk_retry(failures, sources, now - timedelta(hours=24)) + + assert retry_ids == [201] + assert summary == { + "retried": 1, + "skipped_duplicate_source": 2, + "skipped_recent_success": 1, + "skipped_no_source": 0, + } diff --git a/frontend/src/services/api.js b/frontend/src/services/api.js index 5365412..75c4c31 100644 --- a/frontend/src/services/api.js +++ b/frontend/src/services/api.js @@ -35,6 +35,7 @@ export const downloadsApi = { list: (params = {}) => api.get('/downloads', { params }), get: (id) => api.get(`/downloads/${id}`), retry: (id) => api.post(`/downloads/${id}/retry`), + retryFailedBulk: (recentWindowHours = 24) => api.post('/downloads/retry-failed-bulk', { recent_window_hours: recentWindowHours }), stats: (params = {}) => api.get('/downloads/stats', { params }), recentActivity: (params = {}) => api.get('/downloads/recent-activity', { params }), resetOrphaned: (thresholdMinutes = 0) => api.post('/downloads/reset-orphaned', { threshold_minutes: thresholdMinutes }), diff --git a/frontend/src/stores/downloads.js b/frontend/src/stores/downloads.js index 92ff0ca..0309a22 100644 --- a/frontend/src/stores/downloads.js +++ b/frontend/src/stores/downloads.js @@ -43,6 +43,10 @@ export const useDownloadsStore = defineStore('downloads', () => { return await downloadsApi.retry(id) } + async function retryFailedBulk(recentWindowHours = 24) { + return await downloadsApi.retryFailedBulk(recentWindowHours) + } + async function resetOrphaned(thresholdMinutes = 0) { return await downloadsApi.resetOrphaned(thresholdMinutes) } @@ -63,6 +67,7 @@ export const useDownloadsStore = defineStore('downloads', () => { fetchRecentActivity, fetchStats, retryDownload, + retryFailedBulk, resetOrphaned, requeueStale, } diff --git a/frontend/src/views/Dashboard.vue b/frontend/src/views/Dashboard.vue index 72c129e..00d3f4e 100644 --- a/frontend/src/views/Dashboard.vue +++ b/frontend/src/views/Dashboard.vue @@ -592,18 +592,20 @@ async function checkAllSources() { async function retryAllFailed() { retryingAll.value = true try { - const response = await downloadsStore.fetchDownloads({ status: 'failed', per_page: 100, exclude_superseded: true }) - const failed = response.items - let retried = 0 - for (const dl of failed) { - try { - await downloadsStore.retryDownload(dl.id) - retried++ - } catch (e) { - console.error(`Failed to retry ${dl.id}:`, e) - } - } - notifications.success(`Queued retry for ${retried} downloads`) + const response = await downloadsStore.retryFailedBulk() + const { + retried, + skipped_duplicate_source: dupSkipped = 0, + skipped_recent_success: recentSkipped = 0, + skipped_no_source: noSourceSkipped = 0, + } = response.data + + const parts = [`Queued ${retried} ${retried === 1 ? 'retry' : 'retries'}`] + if (dupSkipped) parts.push(`${dupSkipped} duplicate-source skipped`) + if (recentSkipped) parts.push(`${recentSkipped} recently succeeded, skipped`) + if (noSourceSkipped) parts.push(`${noSourceSkipped} orphaned, skipped`) + notifications.success(parts.join(' · ')) + await downloadsStore.fetchStats() await fetchActiveDownloads() } catch (error) {