fix(downloads): always surface all unverified history on Downloads page
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 <noreply@anthropic.com>
This commit is contained in:
parent
0f7e15363b
commit
b928f4df43
4 changed files with 114 additions and 0 deletions
|
|
@ -94,6 +94,10 @@ class StatusDeps:
|
||||||
run_async: Optional[Callable] = None
|
run_async: Optional[Callable] = None
|
||||||
on_download_completed: Optional[Callable[[str, str, bool], None]] = None
|
on_download_completed: Optional[Callable[[str, str, bool], None]] = None
|
||||||
get_persistent_download_history: Optional[Callable[[int], list[dict]]] = 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
|
# 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,
|
'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:
|
if deps.get_persistent_download_history is not None and len(items) < limit:
|
||||||
history_limit = min(limit - len(items), _PERSISTENT_HISTORY_TAIL_LIMIT)
|
history_limit = min(limit - len(items), _PERSISTENT_HISTORY_TAIL_LIMIT)
|
||||||
try:
|
try:
|
||||||
|
|
|
||||||
|
|
@ -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_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_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
|
# Migration: add download_source column
|
||||||
cursor.execute("PRAGMA table_info(library_history)")
|
cursor.execute("PRAGMA table_info(library_history)")
|
||||||
|
|
@ -13269,6 +13270,26 @@ class MusicDatabase:
|
||||||
logger.error(f"Error querying library history: {e}")
|
logger.error(f"Error querying library history: {e}")
|
||||||
return [], 0
|
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):
|
def get_library_history_stats(self):
|
||||||
"""Return counts per event_type and per download_source."""
|
"""Return counts per event_type and per download_source."""
|
||||||
try:
|
try:
|
||||||
|
|
|
||||||
|
|
@ -792,6 +792,68 @@ def test_unified_response_caps_persistent_history_tail():
|
||||||
assert out['total'] == 50
|
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'
|
# #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
|
# forever. The monitor normally retries; when it can't make progress, a backstop
|
||||||
|
|
|
||||||
|
|
@ -18608,6 +18608,7 @@ def _build_status_deps():
|
||||||
page=1,
|
page=1,
|
||||||
limit=limit,
|
limit=limit,
|
||||||
)[0],
|
)[0],
|
||||||
|
get_unverified_download_history=lambda: get_database().get_library_history_unverified(),
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue