downloads(#934): opt-in "Clean orphaned" action for dead review-queue rows
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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LWJk7EuM7YktQeNyqQwTZY
This commit is contained in:
parent
27ea2ee735
commit
0be1952222
4 changed files with 154 additions and 0 deletions
45
core/downloads/orphan_history.py
Normal file
45
core/downloads/orphan_history.py
Normal file
|
|
@ -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}
|
||||||
49
tests/test_orphan_history.py
Normal file
49
tests/test_orphan_history.py
Normal file
|
|
@ -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']
|
||||||
|
|
@ -8158,6 +8158,40 @@ def delete_verification_item(history_id):
|
||||||
return jsonify({"success": False, "error": str(e)}), 500
|
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/<entry_id>/recover', methods=['POST'])
|
@app.route('/api/quarantine/<entry_id>/recover', methods=['POST'])
|
||||||
def recover_quarantine_item(entry_id):
|
def recover_quarantine_item(entry_id):
|
||||||
"""Fallback for legacy thin sidecars: move file into Staging so the user
|
"""Fallback for legacy thin sidecars: move file into Staging so the user
|
||||||
|
|
|
||||||
|
|
@ -3184,6 +3184,31 @@ async function verifDeleteAll(btn) {
|
||||||
_adlFetch();
|
_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) {
|
async function verifQuarApproveAll(btn) {
|
||||||
const entries = _verifQuarEntries.filter(q => q.has_full_context);
|
const entries = _verifQuarEntries.filter(q => q.has_full_context);
|
||||||
if (!entries.length) {
|
if (!entries.length) {
|
||||||
|
|
@ -3344,6 +3369,7 @@ function _adlRender() {
|
||||||
? `<button class="adl-filter-banner-clear" onclick="verifQuarApproveAll(this)" title="Approve + re-import every quarantined file (marked human-verified)">✔ Approve all</button>
|
? `<button class="adl-filter-banner-clear" onclick="verifQuarApproveAll(this)" title="Approve + re-import every quarantined file (marked human-verified)">✔ Approve all</button>
|
||||||
<button class="adl-filter-banner-clear verif-bulk-danger" onclick="verifQuarClearAll(this)" title="Permanently delete every quarantined file">🗑 Clear all</button>`
|
<button class="adl-filter-banner-clear verif-bulk-danger" onclick="verifQuarClearAll(this)" title="Permanently delete every quarantined file">🗑 Clear all</button>`
|
||||||
: `<button class="adl-filter-banner-clear" onclick="verifApproveAll(this)" title="Mark every listed entry as human-verified">✔ Approve all</button>
|
: `<button class="adl-filter-banner-clear" onclick="verifApproveAll(this)" title="Mark every listed entry as human-verified">✔ Approve all</button>
|
||||||
|
<button class="adl-filter-banner-clear" onclick="verifCleanOrphans(this)" title="Remove dead entries whose file no longer exists (deleted / replaced). Removes log rows only — never a file.">🧹 Clean orphaned</button>
|
||||||
<button class="adl-filter-banner-clear verif-bulk-danger" onclick="verifDeleteAll(this)" title="Delete every listed file from disk and remove its entry">🗑 Delete all</button>`;
|
<button class="adl-filter-banner-clear verif-bulk-danger" onclick="verifDeleteAll(this)" title="Delete every listed file from disk and remove its entry">🗑 Delete all</button>`;
|
||||||
// Without an AcoustID key nothing ever gets a verification status —
|
// Without an AcoustID key nothing ever gets a verification status —
|
||||||
// hide the pointless Unverified pill and show quarantine only.
|
// hide the pointless Unverified pill and show quarantine only.
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue