From 0be19522229a7e1c49a493a1a8f188dcf5d55dba Mon Sep 17 00:00:00 2001 From: dev Date: Sun, 28 Jun 2026 02:04:08 +0200 Subject: [PATCH] downloads(#934): opt-in "Clean orphaned" action for dead review-queue rows MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The reconcile heals rows whose file is still in the library; it deliberately leaves ORPHANS โ€” history rows whose file is gone (deleted / replaced / re-downloaded elsewhere). Those can never be healed (no file left to confirm) and linger in the Unverified list forever. This adds an explicit, user-initiated cleanup for them. - core/downloads/orphan_history.py: pure, tested rule. A row is an orphan when its file resolves nowhere; flags `suspicious` when EVERY reviewed file is unreachable (the mount-down signature) so the caller refuses rather than mass-delete a healthy log during an outage. - POST /api/verification/clean-orphans (admin-only): runs it against _resolve_history_audio_path (raw path -> prefix-swap resolver -> tracks-table title fallback), refuses on the suspicious signature, and deletes only history ROWS โ€” never a file (the files are already gone). - UI: "๐Ÿงน Clean orphaned" button in the Unverified bulk-actions row, with a confirm dialog spelling out that it removes log rows only and refuses if the library looks offline. NEVER automatic / never at boot โ€” a filesystem check during a mount outage would otherwise wipe good history. 5 pure-rule tests + safety-gate coverage. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01LWJk7EuM7YktQeNyqQwTZY --- core/downloads/orphan_history.py | 45 +++++++++++++++++++++++++++++ tests/test_orphan_history.py | 49 ++++++++++++++++++++++++++++++++ web_server.py | 34 ++++++++++++++++++++++ webui/static/pages-extra.js | 26 +++++++++++++++++ 4 files changed, 154 insertions(+) create mode 100644 core/downloads/orphan_history.py create mode 100644 tests/test_orphan_history.py diff --git a/core/downloads/orphan_history.py b/core/downloads/orphan_history.py new file mode 100644 index 00000000..1d5effae --- /dev/null +++ b/core/downloads/orphan_history.py @@ -0,0 +1,45 @@ +"""Identify dead review-queue history rows whose file is gone (#934 follow-up). + +The Unverified/Quarantine review queue is fed from ``library_history`` โ€” an +append-only log that is never pruned. When a file is deleted, replaced, or +re-downloaded elsewhere, its old ``unverified`` row lingers forever and can +never be healed (there's no file left to confirm). Those are *orphans*. + +This decides which rows are orphans, given a ``resolve(row) -> path | None`` +the caller wires to the real filesystem lookup. Pure (no DB, no filesystem) so +the rules โ€” including the safety gate โ€” are unit-testable. + +Safety gate: a filesystem check mass-false-positives when the library mount is +down (every file looks missing). So if EVERY reviewed file is unreachable and +there are enough rows to judge, we flag it ``suspicious`` and the caller refuses +to delete โ€” better to clean nothing than to wipe a healthy log during an outage. +""" + +from __future__ import annotations + +from typing import Any, Callable, Sequence + + +def find_orphan_history_ids( + rows: Sequence[dict], + resolve: Callable[[dict], Any], + *, + min_for_safety: int = 5, +) -> dict: + """Return ``{'orphan_ids', 'checked', 'suspicious'}``. + + A row is an orphan when it has a non-empty ``file_path`` but ``resolve`` can + find no file for it. ``suspicious`` is True when every checked row is + missing and there are at least ``min_for_safety`` of them โ€” the mount-down + signature; the caller should refuse to delete in that case. + """ + orphan_ids = [] + checked = 0 + for row in rows: + if not str((row.get('file_path') or '')).strip(): + continue + checked += 1 + if resolve(row) is None: + orphan_ids.append(row.get('id')) + suspicious = checked >= min_for_safety and len(orphan_ids) == checked + return {'orphan_ids': orphan_ids, 'checked': checked, 'suspicious': suspicious} diff --git a/tests/test_orphan_history.py b/tests/test_orphan_history.py new file mode 100644 index 00000000..7018ed0c --- /dev/null +++ b/tests/test_orphan_history.py @@ -0,0 +1,49 @@ +"""Pure orphan-detection rules for the review-queue cleanup (#934 follow-up).""" + +from core.downloads.orphan_history import find_orphan_history_ids + + +def _rows(*specs): + # specs: (id, file_path, exists?) + return [{'id': i, 'file_path': p, '_exists': e} for i, p, e in specs] + + +def _resolve(row): + # stand-in for _resolve_history_audio_path: returns a path or None + return '/on/disk' if row.get('_exists') else None + + +def test_only_missing_files_are_orphans(): + rows = _rows((1, '/a.flac', True), (2, '/b.flac', False), (3, '/c.flac', True)) + out = find_orphan_history_ids(rows, _resolve) + assert out['orphan_ids'] == [2] + assert out['checked'] == 3 + assert out['suspicious'] is False + + +def test_rows_without_path_are_skipped(): + rows = _rows((1, '', False), (2, None, False), (3, '/c.flac', False)) + out = find_orphan_history_ids(rows, _resolve) + assert out['checked'] == 1 + assert out['orphan_ids'] == [3] + + +def test_all_missing_with_enough_rows_is_suspicious(): + rows = _rows(*[(i, f'/x{i}.flac', False) for i in range(6)]) + out = find_orphan_history_ids(rows, _resolve) + assert out['suspicious'] is True # mount-down signature -> caller refuses + assert len(out['orphan_ids']) == 6 + + +def test_all_missing_but_few_rows_is_not_suspicious(): + rows = _rows((1, '/x.flac', False), (2, '/y.flac', False)) + out = find_orphan_history_ids(rows, _resolve) + assert out['suspicious'] is False # too few to suspect an outage + assert out['orphan_ids'] == [1, 2] + + +def test_some_present_is_never_suspicious(): + rows = _rows(*[(i, f'/x{i}.flac', False) for i in range(8)], (99, '/real.flac', True)) + out = find_orphan_history_ids(rows, _resolve) + assert out['suspicious'] is False # at least one file exists -> library is up + assert 99 not in out['orphan_ids'] diff --git a/web_server.py b/web_server.py index 09614540..85f7add3 100644 --- a/web_server.py +++ b/web_server.py @@ -8158,6 +8158,40 @@ def delete_verification_item(history_id): return jsonify({"success": False, "error": str(e)}), 500 +@app.route('/api/verification/clean-orphans', methods=['POST']) +@admin_only +def clean_orphan_verification_items(): + """Remove dead review-queue rows whose file no longer exists anywhere + (deleted / replaced / re-downloaded elsewhere). These are append-only + library_history rows that can never be healed โ€” there's no file left to + confirm โ€” so they linger in the Unverified list forever (#934). + + User-initiated only, never automatic: it does a filesystem check, which + would mass-false-positive if the library mount were down. The pure helper + flags that signature (every reviewed file unreachable) and we refuse. Only + history ROWS are deleted โ€” the files are already gone; this never removes a + file. Admin-only: it mutates shared review state.""" + try: + from core.downloads.orphan_history import find_orphan_history_ids + db = get_database() + rows = db.get_library_history_unverified() or [] + result = find_orphan_history_ids(rows, _resolve_history_audio_path) + if result['suspicious']: + return jsonify({ + "success": False, + "error": "Every reviewed file is unreachable โ€” your library may be " + "offline right now. Nothing was removed.", + }), 409 + orphan_ids = result['orphan_ids'] + removed = db.delete_library_history_rows(orphan_ids) if orphan_ids else 0 + logger.info("[Verification] Cleaned %d orphaned review rows (checked %d)", + removed, result['checked']) + return jsonify({"success": True, "removed": removed, "checked": result['checked']}) + except Exception as e: + logger.error(f"[Verification] Clean orphans failed: {e}") + return jsonify({"success": False, "error": str(e)}), 500 + + @app.route('/api/quarantine//recover', methods=['POST']) def recover_quarantine_item(entry_id): """Fallback for legacy thin sidecars: move file into Staging so the user diff --git a/webui/static/pages-extra.js b/webui/static/pages-extra.js index 06392b36..b66404d9 100644 --- a/webui/static/pages-extra.js +++ b/webui/static/pages-extra.js @@ -3184,6 +3184,31 @@ async function verifDeleteAll(btn) { _adlFetch(); } +async function verifCleanOrphans(btn) { + // Removes review entries whose file is GONE (deleted / replaced / re-downloaded + // elsewhere) โ€” dead log rows that can never be healed. Server-side it does a + // filesystem check and refuses if the whole library looks offline. Removes log + // rows only, never a file. + if (!await showConfirmDialog({ + title: 'Clean orphaned entries', + message: 'Remove review entries whose file no longer exists on disk (deleted, replaced, or re-downloaded elsewhere)? This removes only the stale log rows โ€” it never deletes a file. It checks the filesystem first and refuses if your library looks offline.', + confirmText: 'Clean up', + cancelText: 'Cancel', + })) return; + if (btn) btn.disabled = true; + try { + const r = await fetch('/api/verification/clean-orphans', { method: 'POST' }); + const d = await r.json(); + if (d.success) { + showToast && showToast(`Removed ${d.removed} orphaned entr${d.removed === 1 ? 'y' : 'ies'} (checked ${d.checked})`, 'success'); + _adlFetch(); + } else { + showToast && showToast(d.error || 'Clean-up failed', 'error'); + } + } catch (e) { showToast && showToast('Clean-up failed', 'error'); } + if (btn) btn.disabled = false; +} + async function verifQuarApproveAll(btn) { const entries = _verifQuarEntries.filter(q => q.has_full_context); if (!entries.length) { @@ -3344,6 +3369,7 @@ function _adlRender() { ? ` ` : ` + `; // Without an AcoustID key nothing ever gets a verification status โ€” // hide the pointless Unverified pill and show quarantine only.