From b928f4df439c066cc2e62072e7abc7a4ed4e6f74 Mon Sep 17 00:00:00 2001 From: dev Date: Mon, 15 Jun 2026 19:46:26 +0200 Subject: [PATCH] fix(downloads): always surface all unverified history on Downloads page MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds a dedicated `get_library_history_unverified()` DB query that fetches every library_history row with verification_status IN ('unverified', 'force_imported') with no recency cap. This is loaded unconditionally in `build_unified_downloads_response` — not gated on `len(items) < limit` — so historical unverified entries are never buried by a busy batch filling the 200-row general limit, and entries from weeks/months ago aren't lost in the 50-row recency-ordered history tail. Adds idx_lh_verification_status for query performance and two regression tests. Co-Authored-By: Claude Sonnet 4.6 --- core/downloads/status.py | 30 ++++++++++++ database/music_database.py | 21 ++++++++ tests/downloads/test_downloads_status.py | 62 ++++++++++++++++++++++++ web_server.py | 1 + 4 files changed, 114 insertions(+) diff --git a/core/downloads/status.py b/core/downloads/status.py index 2ac69ed9..8735d4c4 100644 --- a/core/downloads/status.py +++ b/core/downloads/status.py @@ -94,6 +94,10 @@ class StatusDeps: run_async: Optional[Callable] = None on_download_completed: Optional[Callable[[str, str, bool], None]] = None get_persistent_download_history: Optional[Callable[[int], list[dict]]] = None + # Returns ALL library_history rows with verification_status in + # ('unverified', 'force_imported') — no recency limit, so historical + # entries are never buried by the general history tail cap. + get_unverified_download_history: Optional[Callable[[], list[dict]]] = None # Streaming sources the engine fallback applies to. Soulseek goes through @@ -819,6 +823,32 @@ def build_unified_downloads_response(limit: int, deps: StatusDeps) -> dict: 'is_persistent_history': False, }) + # --- Unverified history (unconditional, no limit) --- + # Always load every library_history row that still needs human confirmation + # (verification_status IN ('unverified', 'force_imported')). This is NOT + # gated on len(items) < limit so that historical entries from past batches + # are visible even during a large active batch that would otherwise exhaust + # the limit before the history tail is read. Dedup against live tasks by + # identity so a track currently in post-processing isn't shown twice. + if deps.get_unverified_download_history is not None: + try: + unverified_entries = deps.get_unverified_download_history() or [] + except Exception as exc: + logger.debug("[Downloads] unverified history lookup failed: %s", exc) + unverified_entries = [] + + for entry in unverified_entries: + item = _build_history_download_item(entry) + identity = _download_identity(item.get('title'), item.get('artist'), item.get('album')) + if identity in live_identities: + continue + items.append(item) + live_identities.add(identity) + + # --- General recent-history tail (capped, recency-ordered) --- + # Fills in the completed/verified tail so the full Downloads list looks + # populated. Gated on len(items) < limit so a busy batch doesn't trigger + # an extra DB round-trip when we're already at capacity. if deps.get_persistent_download_history is not None and len(items) < limit: history_limit = min(limit - len(items), _PERSISTENT_HISTORY_TAIL_LIMIT) try: diff --git a/database/music_database.py b/database/music_database.py index 3f0b137b..52591be0 100644 --- a/database/music_database.py +++ b/database/music_database.py @@ -642,6 +642,7 @@ class MusicDatabase: """) cursor.execute("CREATE INDEX IF NOT EXISTS idx_lh_event_type ON library_history (event_type)") cursor.execute("CREATE INDEX IF NOT EXISTS idx_lh_created_at ON library_history (created_at DESC)") + cursor.execute("CREATE INDEX IF NOT EXISTS idx_lh_verification_status ON library_history (verification_status)") # Migration: add download_source column cursor.execute("PRAGMA table_info(library_history)") @@ -13269,6 +13270,26 @@ class MusicDatabase: logger.error(f"Error querying library history: {e}") return [], 0 + def get_library_history_unverified(self) -> list[dict]: + """Return every library_history row that still needs human confirmation. + + Fetches all rows where verification_status is 'unverified' or + 'force_imported', ordered newest-first. No row limit — the full + set must always be visible on the Downloads → Unverified tab. + """ + try: + conn = self._get_connection() + cursor = conn.cursor() + cursor.execute(""" + SELECT * FROM library_history + WHERE verification_status IN ('unverified', 'force_imported') + ORDER BY created_at DESC + """) + return [dict(row) for row in cursor.fetchall()] + except Exception as e: + logger.error("Error querying unverified library history: %s", e) + return [] + def get_library_history_stats(self): """Return counts per event_type and per download_source.""" try: diff --git a/tests/downloads/test_downloads_status.py b/tests/downloads/test_downloads_status.py index 4e56b016..ecce784a 100644 --- a/tests/downloads/test_downloads_status.py +++ b/tests/downloads/test_downloads_status.py @@ -792,6 +792,68 @@ def test_unified_response_caps_persistent_history_tail(): assert out['total'] == 50 +def test_unverified_history_always_loaded_even_when_at_limit(): + """Unverified entries must appear even during a large batch that fills the limit.""" + for i in range(200): + download_tasks[f'live-{i}'] = { + 'track_index': i, + 'status': 'completed', + 'track_info': {'name': f'Track {i}', 'artist': f'Artist {i}', 'album': 'Album'}, + 'verification_status': 'verified', + } + + deps, _ = _build_deps( + persistent_history=lambda limit: [], + ) + deps.get_unverified_download_history = lambda: [ + { + 'id': 999, + 'title': 'Old Unverified', + 'artist_name': 'Forgotten Artist', + 'album_name': 'Old Album', + 'created_at': '2026-01-01 00:00:00', + 'verification_status': 'unverified', + } + ] + + out = st.build_unified_downloads_response(200, deps) + + titles = {d['title'] for d in out['downloads']} + assert 'Old Unverified' in titles, "historical unverified entry must appear regardless of limit" + + unverified = [d for d in out['downloads'] if d.get('verification_status') == 'unverified'] + assert len(unverified) == 1 + assert unverified[0]['task_id'] == 'history-999' + assert unverified[0]['is_persistent_history'] is True + + +def test_unverified_history_deduped_against_live_task(): + """A live task that's still unverified must not appear twice.""" + download_tasks['live-unv'] = { + 'track_index': 0, + 'status': 'completed', + 'track_info': {'name': 'Live Unverified', 'artist': 'Artsy', 'album': 'Alb'}, + 'verification_status': 'unverified', + } + + deps, _ = _build_deps() + deps.get_unverified_download_history = lambda: [ + { + 'id': 77, + 'title': 'Live Unverified', + 'artist_name': 'Artsy', + 'album_name': 'Alb', + 'created_at': '2026-06-15 10:00:00', + 'verification_status': 'unverified', + } + ] + + out = st.build_unified_downloads_response(200, deps) + matches = [d for d in out['downloads'] if d['title'] == 'Live Unverified'] + assert len(matches) == 1, "same track must not be duplicated" + assert matches[0]['is_persistent_history'] is False + + # --------------------------------------------------------------------------- # #836 — a rejected slskd transfer must not hang the task at 'downloading' # forever. The monitor normally retries; when it can't make progress, a backstop diff --git a/web_server.py b/web_server.py index 9cfa139a..0b8aaefb 100644 --- a/web_server.py +++ b/web_server.py @@ -18608,6 +18608,7 @@ def _build_status_deps(): page=1, limit=limit, )[0], + get_unverified_download_history=lambda: get_database().get_library_history_unverified(), )