From 26368a80ab24f25f25bb924db9b40b637dde3fd0 Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Tue, 9 Jun 2026 13:08:41 -0700 Subject: [PATCH] Dead File Cleaner: don't flag a whole library when paths just aren't reachable (#828) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit macstainless: a Plex-on-macOS user running SoulSync in Docker had all 5,250 tracks flagged "dead" even though the files exist and play in Plex. Root cause: the DB stores paths as Plex reported them (/Volumes/Core/Music/...), which don't exist inside SoulSync's container. resolve_library_file_path() returns None for "couldn't find it at any known base dir" — and for a mis-mounted library that's EVERY track, not a deletion. The job treated None as "file deleted" and created a finding per track. Fix mirrors the existing transfer-folder abort: collect unresolvable tracks, and if at least max_unresolved_fraction (default 0.5) of the library is unresolvable once it's past min_tracks_for_guard (default 25), treat it as a path-mapping/ mount problem — abort with an actionable message (Docker mount / Settings → Library → Music Paths) and create ZERO findings. A small fraction unresolvable is still reported as genuine dead files, and tiny libraries (< min) report as before. Both thresholds are configurable per the job's settings. Tests: mass-unresolvable aborts with no findings; a lone dead file among real ones is still reported; a small all-dead library still reports; thresholds configurable. 54 repair tests pass. --- core/repair_jobs/dead_file_cleaner.py | 132 ++++++++++++++++++------- tests/test_dead_file_cleaner_guard.py | 135 ++++++++++++++++++++++++++ 2 files changed, 232 insertions(+), 35 deletions(-) create mode 100644 tests/test_dead_file_cleaner_guard.py diff --git a/core/repair_jobs/dead_file_cleaner.py b/core/repair_jobs/dead_file_cleaner.py index f0b3fb9d..ac9a0263 100644 --- a/core/repair_jobs/dead_file_cleaner.py +++ b/core/repair_jobs/dead_file_cleaner.py @@ -36,7 +36,17 @@ class DeadFileCleanerJob(RepairJob): icon = 'repair-icon-deadfile' default_enabled = True default_interval_hours = 24 - default_settings = {} + default_settings = { + # Mass-false-positive guard: if at least this fraction of tracks resolve + # to no file on disk, treat it as a path-mapping/mount problem (SoulSync + # can't SEE the library — e.g. Docker, or library.music_paths unset for + # this environment) rather than thousands of individually-deleted files, + # and abort without creating findings. Mirrors the transfer-folder abort. + 'max_unresolved_fraction': 0.5, + # ...but only once the library is at least this big — a small library can + # legitimately have a high dead fraction. + 'min_tracks_for_guard': 25, + } auto_fix = False def scan(self, context: JobContext) -> JobResult: @@ -87,9 +97,30 @@ class DeadFileCleanerJob(RepairJob): if context.config_manager: download_folder = context.config_manager.get('soulseek.download_path', '') + # Mass-false-positive guard thresholds (see default_settings). + max_unresolved_fraction = 0.5 + min_tracks_for_guard = 25 + if context.config_manager: + try: + max_unresolved_fraction = float(context.config_manager.get( + self.get_config_key('max_unresolved_fraction'), 0.5)) + except (TypeError, ValueError): + max_unresolved_fraction = 0.5 + try: + min_tracks_for_guard = int(context.config_manager.get( + self.get_config_key('min_tracks_for_guard'), 25)) + except (TypeError, ValueError): + min_tracks_for_guard = 25 + if context.report_progress: context.report_progress(phase=f'Checking {total} tracks...', total=total) + # Collect unresolvable tracks first; decide whether they're genuine dead + # files or a systemic path problem AFTER the full pass (below). A "None" + # from the resolver means "couldn't find it at any known base dir" — which + # for a mis-mounted library is EVERY track, not a real deletion. + dead_rows = [] + for i, row in enumerate(tracks): if context.check_stop(): return result @@ -112,44 +143,75 @@ class DeadFileCleanerJob(RepairJob): config_manager=context.config_manager) if resolved is None: - # File is truly missing — create finding - if context.report_progress: - context.report_progress( - log_line=f'Missing: {title or "Unknown"} — {os.path.basename(file_path)}', - log_type='error' - ) - if context.create_finding: - try: - inserted = context.create_finding( - job_id=self.job_id, - finding_type='dead_file', - severity='warning', - entity_type='track', - entity_id=str(track_id), - file_path=file_path, - title=f'Missing file: {title or "Unknown"}', - description=f'Track "{title}" by {artist_name or "Unknown"} points to a file that no longer exists', - details={ - 'track_id': track_id, - 'title': title, - 'artist': artist_name, - 'album': album_title, - 'original_path': file_path, - 'album_thumb_url': album_thumb or None, - 'artist_thumb_url': artist_thumb or None, - } - ) - if inserted: - result.findings_created += 1 - else: - result.findings_skipped_dedup += 1 - except Exception as e: - logger.debug("Error creating dead file finding for track %s: %s", track_id, e) - result.errors += 1 + dead_rows.append(row) if context.update_progress and (i + 1) % 100 == 0: context.update_progress(i + 1, total) + # Mass-false-positive guard: a large fraction of unresolvable paths almost + # always means SoulSync can't SEE the library (Docker mount, or + # Settings → Library → Music Paths not set for this environment), NOT that + # thousands of files were individually deleted. Refuse to flag and say so + # — same principle as the transfer-folder abort above. (#828: a Plex-on- + # macOS user in Docker had all 5,250 tracks flagged because their stored + # /Volumes/... paths don't exist inside the container.) + if (dead_rows + and result.scanned >= min_tracks_for_guard + and len(dead_rows) >= result.scanned * max_unresolved_fraction): + logger.error( + "Dead file scan: %d/%d tracks unresolvable (>= %.0f%%) — aborting without " + "creating findings; this is a path-mapping/mount problem, not deleted files.", + len(dead_rows), result.scanned, max_unresolved_fraction * 100) + result.errors += 1 + if context.report_progress: + context.report_progress( + phase='Aborted — too many unreachable paths', + log_line=(f"{len(dead_rows)} of {result.scanned} tracks point to paths SoulSync " + f"can't reach — almost always a path-mapping issue (Docker mount, or " + f"Settings → Library → Music Paths), not deleted files. No findings created."), + log_type='error' + ) + if context.update_progress: + context.update_progress(total, total) + return result + + # A small fraction unresolvable — treat as genuine dead files and report. + for row in dead_rows: + track_id, title, artist_name, album_title, file_path, album_thumb, artist_thumb = row + if context.report_progress: + context.report_progress( + log_line=f'Missing: {title or "Unknown"} — {os.path.basename(file_path)}', + log_type='error' + ) + if context.create_finding: + try: + inserted = context.create_finding( + job_id=self.job_id, + finding_type='dead_file', + severity='warning', + entity_type='track', + entity_id=str(track_id), + file_path=file_path, + title=f'Missing file: {title or "Unknown"}', + description=f'Track "{title}" by {artist_name or "Unknown"} points to a file that no longer exists', + details={ + 'track_id': track_id, + 'title': title, + 'artist': artist_name, + 'album': album_title, + 'original_path': file_path, + 'album_thumb_url': album_thumb or None, + 'artist_thumb_url': artist_thumb or None, + } + ) + if inserted: + result.findings_created += 1 + else: + result.findings_skipped_dedup += 1 + except Exception as e: + logger.debug("Error creating dead file finding for track %s: %s", track_id, e) + result.errors += 1 + if context.update_progress: context.update_progress(total, total) diff --git a/tests/test_dead_file_cleaner_guard.py b/tests/test_dead_file_cleaner_guard.py new file mode 100644 index 00000000..1ea90afe --- /dev/null +++ b/tests/test_dead_file_cleaner_guard.py @@ -0,0 +1,135 @@ +"""Dead File Cleaner — mass-false-positive guard (#828). + +macstainless: a Plex-on-macOS user running SoulSync in Docker had all 5,250 +tracks flagged "dead" because their stored /Volumes/... paths don't exist inside +the container. The resolver returning None means "couldn't find it at any known +base dir" — for a mis-mounted library that's EVERY track, not a real deletion. +The job now refuses to flag when a large fraction is unresolvable (a path-mapping +problem) and reports it as such, mirroring the existing transfer-folder abort. +""" + +from __future__ import annotations + +from core.repair_jobs.base import JobContext +from core.repair_jobs.dead_file_cleaner import DeadFileCleanerJob + + +class _Cur: + def __init__(self, rows): + self._rows = rows + + def execute(self, *a, **k): + pass + + def fetchall(self): + return self._rows + + def fetchone(self): + return [len(self._rows)] + + def close(self): + pass + + +class _Conn: + def __init__(self, rows): + self._rows = rows + + def cursor(self): + return _Cur(self._rows) + + def close(self): + pass + + +class _Db: + def __init__(self, rows): + self._rows = rows + + def _get_connection(self): + return _Conn(self._rows) + + +class _Cfg: + def __init__(self, overrides=None): + self._o = overrides or {} + + def get(self, key, default=None): + return self._o.get(key, default) + + +def _row(i, path): + # (track_id, title, artist, album, file_path, album_thumb, artist_thumb) + return (i, f"Track {i}", "Yellowcard", "Ocean Avenue", path, None, None) + + +def _run(rows, transfer_folder, cfg_overrides=None): + findings = [] + cfg = _Cfg({'soulseek.download_path': '', **(cfg_overrides or {})}) + ctx = JobContext( + db=_Db(rows), + transfer_folder=str(transfer_folder), + config_manager=cfg, + create_finding=lambda **kw: (findings.append(kw) or True), + ) + res = DeadFileCleanerJob().scan(ctx) + return res, findings + + +def test_mass_unresolvable_aborts_without_findings(tmp_path): + # 30 tracks all pointing to a /Volumes path that doesn't exist in this env + # -> systemic path problem -> abort, zero findings, one error. + rows = [_row(i, f"/Volumes/Core/Music/Plex/Yellowcard/{i}.mp3") for i in range(30)] + res, findings = _run(rows, tmp_path) + assert findings == [] + assert res.findings_created == 0 + assert res.errors >= 1 + assert res.scanned == 30 + + +def test_few_unresolvable_creates_findings(tmp_path): + # 4 real (resolvable) files + 1 genuinely missing -> fraction 0.2 < 0.5 -> + # the one dead file IS reported. + rows = [] + for i in range(4): + f = tmp_path / f"real_{i}.mp3" + f.write_text("x") + rows.append(_row(i, str(f))) + rows.append(_row(99, "/no/such/path/dead.mp3")) + res, findings = _run(rows, tmp_path, + {'repair.jobs.dead_file_cleaner.min_tracks_for_guard': 4}) + assert res.findings_created == 1 + assert len(findings) == 1 + assert findings[0]['entity_id'] == '99' + assert res.errors == 0 + + +def test_small_library_all_dead_still_reports(tmp_path): + # 3 dead tracks, below the default min_tracks_for_guard (25) -> guard doesn't + # apply -> all 3 reported (a tiny library can legitimately be all-dead). + rows = [_row(i, f"/no/such/{i}.mp3") for i in range(3)] + res, findings = _run(rows, tmp_path) + assert res.findings_created == 3 + + +def test_guard_thresholds_configurable(tmp_path): + # Lower min to 4; all 4 dead -> fraction 1.0 >= 0.5 -> abort. + rows = [_row(i, f"/no/such/{i}.mp3") for i in range(4)] + res, findings = _run(rows, tmp_path, + {'repair.jobs.dead_file_cleaner.min_tracks_for_guard': 4}) + assert res.findings_created == 0 + assert res.errors >= 1 + + +def test_healthy_library_no_abort_no_findings(tmp_path): + # 30 fully-resolvable tracks -> 0 dead -> neither aborts nor flags anything. + rows = [] + for i in range(30): + f = tmp_path / f"ok_{i}.mp3" + f.write_text("x") + rows.append(_row(i, str(f))) + res, findings = _run(rows, tmp_path) + assert res.findings_created == 0 + assert res.errors == 0 + assert res.scanned == 30 + assert findings == []