Orphan detector: hard-bail on a mass-orphan flood instead of warn-only
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).
This commit is contained in:
parent
cbab4234ef
commit
94a0070fa8
4 changed files with 151 additions and 16 deletions
|
|
@ -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",
|
||||
]
|
||||
|
|
|
|||
|
|
@ -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:
|
||||
|
|
|
|||
|
|
@ -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.
|
||||
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
Loading…
Reference in a new issue