From 94a0070fa8c9ca7f600e8feec61b27af427acb78 Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Fri, 12 Jun 2026 08:15:23 -0700 Subject: [PATCH] Orphan detector: hard-bail on a mass-orphan flood instead of warn-only MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A DB<->filesystem path mismatch (Docker volume change, remount, Music Paths unset for the container) makes EVERY library file fail to resolve to a DB track, so the orphan detector flags the whole library as orphaned. The mass-orphan check only logged a warning and then created the findings anyway — so a user batch-applying 'move to staging' or 'delete' would relocate or wipe their entire library. Make it a hard skip (create zero findings) like the dead-file cleaner and stale-removal paths already do (#828). Centralise the predicate as is_implausible_orphan_flood() alongside is_implausible_stale_removal() so the rule lives in one tested place. Small genuine orphan sets still surface unchanged — only an implausibly large flood (>50% and >20) is suppressed. Tests: seam cases for the new predicate + scan-level regressions (mass mismatch -> 0 findings; small genuine set -> still reported). --- core/library/stale_guard.py | 41 ++++++++++++++++- core/repair_jobs/orphan_file_detector.py | 41 +++++++++++------ tests/test_orphan_file_detector.py | 58 ++++++++++++++++++++++++ tests/test_stale_guard.py | 27 +++++++++++ 4 files changed, 151 insertions(+), 16 deletions(-) diff --git a/core/library/stale_guard.py b/core/library/stale_guard.py index c614934c..0d40ee68 100644 --- a/core/library/stale_guard.py +++ b/core/library/stale_guard.py @@ -42,4 +42,43 @@ def is_implausible_stale_removal( return missing_count > total_count * max_fraction -__all__ = ["is_implausible_stale_removal", "DEFAULT_MIN_TOTAL", "DEFAULT_MAX_MISSING_FRACTION"] +# The orphan detector walks the transfer folder and flags any audio file whose +# path/title doesn't resolve to a DB track. If the DB's stored paths share a base +# prefix the local filesystem no longer has (remount, Docker volume change, WSL +# hiccup), EVERY file misses and the whole library looks "orphaned" — and a user +# batch-applying "move to staging" on those findings would relocate their entire +# library. Same failure mode as stale-removal, so we skip the whole result when +# the orphan share is implausibly large. Needs an absolute floor too: 3/4 orphans +# in a tiny folder is normal, 4000/5000 is a path mismatch. +DEFAULT_MIN_ORPHANS = 20 +DEFAULT_MAX_ORPHAN_FRACTION = 0.5 + + +def is_implausible_orphan_flood( + orphan_count: int, + total_count: int, + *, + min_orphans: int = DEFAULT_MIN_ORPHANS, + max_fraction: float = DEFAULT_MAX_ORPHAN_FRACTION, +) -> bool: + """True when so many files look orphaned that the DB↔filesystem path mapping is + almost certainly broken (not real orphans) and the scan should create NO + findings — otherwise a batch "move to staging" / "delete" could wipe the + library. Below ``min_orphans`` (absolute) it always returns False so small, + genuine orphan sets still surface. + """ + if total_count <= 0 or orphan_count <= 0: + return False + if orphan_count <= min_orphans: + return False + return orphan_count > total_count * max_fraction + + +__all__ = [ + "is_implausible_stale_removal", + "is_implausible_orphan_flood", + "DEFAULT_MIN_TOTAL", + "DEFAULT_MAX_MISSING_FRACTION", + "DEFAULT_MIN_ORPHANS", + "DEFAULT_MAX_ORPHAN_FRACTION", +] diff --git a/core/repair_jobs/orphan_file_detector.py b/core/repair_jobs/orphan_file_detector.py index 5c6e69cd..1b2b5b5f 100644 --- a/core/repair_jobs/orphan_file_detector.py +++ b/core/repair_jobs/orphan_file_detector.py @@ -218,17 +218,33 @@ class OrphanFileDetectorJob(RepairJob): if context.update_progress and (i + 1) % 50 == 0: context.update_progress(i + 1, total) - # Safety check: if most files look like orphans, it's probably a path - # mismatch between the DB and filesystem — not actual orphans. - orphan_ratio = len(orphan_files) / total if total else 0 - mass_orphan = orphan_ratio > 0.5 and len(orphan_files) > 20 - - if mass_orphan: + # Safety: if most files look like orphans, it's almost certainly a path + # mismatch between the DB and filesystem (remount / Docker volume change), + # NOT real orphans. Creating findings anyway is dangerous — a user batch- + # applying "move to staging" / "delete" on them would relocate or wipe the + # whole library. So we create NO findings here, the same hard skip the + # stale-removal paths use. Fix the path mismatch and real orphans surface. + from core.library.stale_guard import is_implausible_orphan_flood + if is_implausible_orphan_flood(len(orphan_files), total): + pct = (len(orphan_files) / total * 100) if total else 0 logger.warning( - "Mass orphan warning: %d of %d files (%.0f%%) flagged as orphans — " - "this likely indicates a DB path mismatch, not actual orphans", - len(orphan_files), total, orphan_ratio * 100 + "Mass orphan guard: %d of %d files (%.0f%%) flagged as orphans — " + "almost certainly a DB↔filesystem path mismatch, not real orphans. " + "Creating no findings so a batch move/delete can't wipe the library.", + len(orphan_files), total, pct, ) + if context.report_progress: + context.report_progress( + log_line=(f'Skipped: {len(orphan_files)} of {total} files look ' + 'orphaned — likely a DB path mismatch, not real orphans. ' + 'No findings created.'), + log_type='skip', + ) + if context.update_progress: + context.update_progress(total, total) + logger.info("Orphan file scan: %d files scanned, mass-orphan guard tripped " + "(0 findings)", result.scanned) + return result for fpath in orphan_files: if context.report_progress: @@ -243,16 +259,12 @@ class OrphanFileDetectorJob(RepairJob): inserted = context.create_finding( job_id=self.job_id, finding_type='orphan_file', - severity='warning' if mass_orphan else 'info', + severity='info', entity_type='file', entity_id=None, file_path=fpath, title=f'Orphan file: {os.path.basename(fpath)}', description=( - 'Audio file in transfer folder is not tracked in the database. ' - 'WARNING: Mass orphan detection triggered — this may be a path ' - 'mismatch, not actual orphans. Verify before deleting!' - ) if mass_orphan else ( 'Audio file in transfer folder is not tracked in the database' ), details={ @@ -261,7 +273,6 @@ class OrphanFileDetectorJob(RepairJob): 'modified': time.strftime('%Y-%m-%d %H:%M:%S', time.localtime(stat.st_mtime)), 'folder': os.path.dirname(fpath), - 'mass_orphan': mass_orphan, } ) if inserted: diff --git a/tests/test_orphan_file_detector.py b/tests/test_orphan_file_detector.py index 02ba7150..2d691bb6 100644 --- a/tests/test_orphan_file_detector.py +++ b/tests/test_orphan_file_detector.py @@ -52,6 +52,64 @@ def _seed_library(db_path: Path) -> None: conn.close() +def test_mass_orphan_path_mismatch_creates_no_findings(tmp_path: Path) -> None: + """The "transferred to staging" footgun: when the DB's stored paths no longer + match the filesystem (remount / Docker volume change) EVERY file looks + orphaned. The detector must create NO findings then — otherwise a user + batch-applying "move to staging" relocates their whole library. Mirrors the + hard skip the stale-removal paths use. + """ + db_path = tmp_path / "library.sqlite" + _seed_library(db_path) # DB tracks live under /old/prefix/... — nothing on disk matches + + # Drop 30 untracked files (> the 20 absolute floor, and 100% > 50%). + music = tmp_path / "Some Artist" / "Some Album" + music.mkdir(parents=True) + for i in range(30): + (music / f"{i:02d} - Track {i}.mp3").write_bytes(b"unreadable tags; no DB match") + + findings = [] + context = JobContext( + db=_DB(db_path), + transfer_folder=str(tmp_path), + config_manager=None, + create_finding=lambda **kwargs: findings.append(kwargs) or True, + ) + + result = OrphanFileDetectorJob().scan(context) + + assert result.scanned == 30 + assert result.findings_created == 0 + assert findings == [] # hard skip — not even flagged as warnings + + +def test_small_orphan_set_still_surfaces(tmp_path: Path) -> None: + """Below the absolute floor, genuine orphans must still be reported — the + guard only suppresses an implausibly large flood, not normal stray files. + """ + db_path = tmp_path / "library.sqlite" + _seed_library(db_path) + + music = tmp_path / "Stray" / "Files" + music.mkdir(parents=True) + for i in range(3): # 3 orphans — under the 20-file floor + (music / f"{i:02d} - Stray {i}.mp3").write_bytes(b"no DB match") + + findings = [] + context = JobContext( + db=_DB(db_path), + transfer_folder=str(tmp_path), + config_manager=None, + create_finding=lambda **kwargs: findings.append(kwargs) or True, + ) + + result = OrphanFileDetectorJob().scan(context) + + assert result.scanned == 3 + assert result.findings_created == 3 + assert all(f['finding_type'] == 'orphan_file' for f in findings) + + def test_orphan_detector_accepts_picard_albumartist_folder_match(tmp_path: Path) -> None: """Picard paths use albumartist/album (year)/track - title. diff --git a/tests/test_stale_guard.py b/tests/test_stale_guard.py index 6259847e..68db9f0a 100644 --- a/tests/test_stale_guard.py +++ b/tests/test_stale_guard.py @@ -2,6 +2,7 @@ from __future__ import annotations +from core.library.stale_guard import is_implausible_orphan_flood as flood from core.library.stale_guard import is_implausible_stale_removal as g @@ -26,3 +27,29 @@ def test_edge_inputs(): assert g(0, 0) is False assert g(0, 100) is False # nothing missing assert g(5, 5) is True # min_total met, all missing + + +# ── orphan-flood guard: same shape, protects the "move to staging" path ────── + +def test_whole_library_flagged_orphan_is_blocked(): + # 4000/5000 files "orphaned" → a path mismatch, not real orphans. + assert flood(4000, 5000) is True + assert flood(21, 40) is True # just over both floors (>20 and >50%) + + +def test_a_handful_of_real_orphans_still_surface(): + assert flood(3, 4000) is False # a few stray files — report them + assert flood(20, 30) is False # at the absolute floor (not > 20) + assert flood(2000, 4000) is False # exactly 50% is NOT over the threshold + + +def test_orphan_flood_small_folders_never_blocked(): + # A 5-file folder that's all orphans is plausible (manual drop) — don't hide it. + assert flood(5, 5) is False + assert flood(20, 20) is False # below the absolute orphan floor + + +def test_orphan_flood_edge_inputs(): + assert flood(0, 0) is False + assert flood(0, 5000) is False # nothing orphaned + assert flood(5000, 0) is False # nonsense totals don't trip it