From 298d825757ebe83e1f03735d081bfe04374b6059 Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Thu, 18 Jun 2026 20:07:41 -0700 Subject: [PATCH] #891: clear dead folders left with only cover images / .lrc sidecars MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reorganize leaves a cover.jpg (and other leftovers) behind when it moves an album, and the Empty Folder Cleaner is too conservative to sweep them. Two complementary fixes over one shared 'residual file' predicate (core/library/residual_files.py: junk + cover/scan images + lyric/metadata sidecars), so both features agree on what a dead folder is. 1. Reorganize (proactive): _delete_album_sidecars now sweeps ALL residual files when a source dir has no audio left — previously only a fixed list of cover NAMES, so back.jpg / disc.png / .webp survived and kept the folder un-prunable. 2. Empty Folder Cleaner (the request): new opt-in 'Remove Residual Files' setting (default off) treats a folder holding only images/sidecars/junk as removable — cleans the existing backlog + arbitrary names. Auto-renders as a UI toggle. Safe by construction: whitelist-only (a booklet.pdf / video / .txt is real content and kept), reorganize sweep gated on no-audio, cleaner re-checks at apply time. 20 new tests; 272 reorganize/repair/empty-folder green. --- core/library/residual_files.py | 53 +++++++++++++ core/library_reorganize.py | 40 ++++++---- core/repair_jobs/empty_folder_cleaner.py | 96 +++++++++++++++--------- core/repair_worker.py | 1 + tests/library/test_residual_files.py | 51 +++++++++++++ tests/test_empty_folder_cleaner.py | 48 ++++++++++++ 6 files changed, 238 insertions(+), 51 deletions(-) create mode 100644 core/library/residual_files.py create mode 100644 tests/library/test_residual_files.py diff --git a/core/library/residual_files.py b/core/library/residual_files.py new file mode 100644 index 00000000..5a15831d --- /dev/null +++ b/core/library/residual_files.py @@ -0,0 +1,53 @@ +"""What counts as a *residual* file — a leftover with no value once the audio it +accompanied is gone: OS junk, cover/scan images, and lyric/metadata sidecars. + +Single source of truth shared by: + * the **Reorganize** cleanup, which strips these from a source dir after every + track has moved out (so the empty-dir pruner can take the folder), and + * the **Empty Folder Cleaner** job, which can optionally treat a folder holding + ONLY residual files as removable (#891). + +Defining "disposable" in one place keeps the two features agreeing on what a "dead +folder" is. Pure predicates — no filesystem access — so they're unit-tested in +isolation. The whitelist is deliberately conservative: anything NOT recognized here +(a booklet ``.pdf``, a video, a ``.txt`` note) is treated as real content and kept. +""" + +from __future__ import annotations + +import os + +# OS / tooling junk. +JUNK_FILES = {'.ds_store', 'thumbs.db', 'desktop.ini', '.directory', 'album.nfo~'} +# Cover art + booklet scans. +IMAGE_EXTS = {'.jpg', '.jpeg', '.png', '.gif', '.bmp', '.webp', '.tiff', '.tif'} +# Lyric / metadata / playlist sidecars that are worthless without their audio. +SIDECAR_EXTS = {'.lrc', '.nfo', '.cue', '.m3u', '.m3u8'} + + +def _ext(name: str) -> str: + return os.path.splitext(name or '')[1].lower() + + +def is_junk(name: str) -> bool: + return (name or '').lower() in JUNK_FILES + + +def is_image(name: str) -> bool: + return _ext(name) in IMAGE_EXTS + + +def is_sidecar(name: str) -> bool: + return _ext(name) in SIDECAR_EXTS + + +def is_disposable(name: str) -> bool: + """True if this file is junk, a cover/scan image, or a lyric/metadata sidecar — + i.e. safe to delete from a folder that has no audio left.""" + return is_junk(name) or is_image(name) or is_sidecar(name) + + +__all__ = [ + 'JUNK_FILES', 'IMAGE_EXTS', 'SIDECAR_EXTS', + 'is_junk', 'is_image', 'is_sidecar', 'is_disposable', +] diff --git a/core/library_reorganize.py b/core/library_reorganize.py index e2991682..a5c9e4cd 100644 --- a/core/library_reorganize.py +++ b/core/library_reorganize.py @@ -1847,14 +1847,8 @@ def _prune_empty_album_dirs(artist_dir: str) -> None: # Sidecars that live alongside ONE audio file (same filename stem). _TRACK_SIDECAR_EXTS = ('.lrc', '.nfo', '.txt', '.cue', '.json') -# Sidecars that live at the ALBUM level (one per directory). -_ALBUM_SIDECARS = ( - 'cover.jpg', 'cover.jpeg', 'cover.png', - 'folder.jpg', 'folder.png', - 'front.jpg', 'front.png', - 'album.jpg', 'album.png', - 'artwork.jpg', 'artwork.png', -) +# Album-level leftovers (cover images, .lrc, etc.) are classified by the shared +# `core.library.residual_files.is_disposable` predicate — see `_delete_album_sidecars`. # Audio extensions used to decide whether a source directory still has # tracks the user might care about (i.e. a per-track failure left audio @@ -1954,16 +1948,30 @@ def _delete_track_sidecars(audio_path: str) -> None: def _delete_album_sidecars(src_dir: str) -> None: - """Delete album-level sidecars (cover.jpg, folder.jpg, etc.) from - `src_dir`. Used during end-of-run cleanup when no audio files remain - in the directory. Best-effort — individual failures are debug-logged.""" - for name in _ALBUM_SIDECARS: - sidecar = os.path.join(src_dir, name) - if os.path.isfile(sidecar): + """Delete album-level *residual* files from ``src_dir`` — any cover/scan image, + lyric/metadata sidecar (.lrc/.nfo/.cue/.m3u), or OS junk. Called during + end-of-run cleanup ONLY when no audio remains in the directory, so everything + here is leftover from the album that just moved (#891 — previously this only + removed a fixed list of cover names, so ``back.jpg`` / ``disc.jpg`` / ``.webp`` + survived and kept the folder un-prunable). + + Uses the shared ``is_disposable`` predicate so it agrees with the Empty Folder + Cleaner on what's a dead leftover; anything unrecognized (a booklet ``.pdf``, a + video) is deliberately LEFT. Best-effort — individual failures are debug-logged.""" + from core.library.residual_files import is_disposable + try: + entries = os.listdir(src_dir) + except OSError: + return + for name in entries: + if not is_disposable(name): + continue + full = os.path.join(src_dir, name) + if os.path.isfile(full): try: - os.remove(sidecar) + os.remove(full) except OSError as e: - logger.debug(f"[Reorganize] Couldn't remove album sidecar {sidecar}: {e}") + logger.debug(f"[Reorganize] Couldn't remove residual file {full}: {e}") def _has_remaining_audio(directory: str) -> bool: diff --git a/core/repair_jobs/empty_folder_cleaner.py b/core/repair_jobs/empty_folder_cleaner.py index 0f8f16e3..fe1f5d02 100644 --- a/core/repair_jobs/empty_folder_cleaner.py +++ b/core/repair_jobs/empty_folder_cleaner.py @@ -22,37 +22,36 @@ from __future__ import annotations import os from typing import Iterable, List +from core.library.residual_files import JUNK_FILES, is_disposable, is_junk # noqa: F401 — JUNK_FILES/is_junk re-exported from core.repair_jobs import register_job from core.repair_jobs.base import JobContext, JobResult, RepairJob from utils.logging_config import get_logger logger = get_logger("repair_jobs.empty_folder_cleaner") -# Files that don't count as real content — safe to delete along with the folder. -JUNK_FILES = {'.ds_store', 'thumbs.db', 'desktop.ini', '.directory', 'album.nfo~'} - - -def is_junk(name: str) -> bool: - return (name or '').lower() in JUNK_FILES - def dir_is_removable(files: Iterable[str], surviving_subdirs: Iterable[str], - *, ignore_junk: bool = True) -> bool: + *, ignore_junk: bool = True, ignore_disposable: bool = False) -> bool: """Pure: is a directory safe to remove? Removable iff it has **no surviving subdirectories** and **no real files** — where "no real files" means literally empty, or (when ``ignore_junk``) only - OS-junk files. ``surviving_subdirs`` is the list of child dirs that are NOT - themselves being removed (i.e. still hold content). + OS-junk files, or (when ``ignore_disposable`` — #891) only *residual* files: + junk + cover/scan images + lyric/metadata sidecars. ``ignore_disposable`` is the + broader opt-in that clears the cover.jpg-only folders a reorganize leaves behind. + ``surviving_subdirs`` is the list of child dirs that are NOT themselves being + removed (i.e. still hold content). """ if list(surviving_subdirs): return False files = list(files) if not files: return True - if not ignore_junk: - return False - return all(is_junk(f) for f in files) + if ignore_disposable: + return all(is_disposable(f) for f in files) + if ignore_junk: + return all(is_junk(f) for f in files) + return False @register_job @@ -65,15 +64,18 @@ class EmptyFolderCleanerJob(RepairJob): 'relocations, and deletions — empty artist/album folders, or folders that ' 'hold only OS junk like .DS_Store / Thumbs.db.\n\n' 'A finding is created for each. Applying one deletes the folder (after ' - 're-checking it is still empty). Folders that contain any real file — a ' - 'cover image, an audio track, anything — are never touched, the library ' - 'root is never removed, and it cascades: a folder left empty once its ' - 'empty children are removed is cleaned too.' + 're-checking it is still empty). Folders that contain any real file — an ' + 'audio track, a booklet, anything not recognized as a leftover — are never ' + 'touched, the library root is never removed, and it cascades: a folder left ' + 'empty once its empty children are removed is cleaned too.\n\n' + 'Enable "Also remove image/sidecar-only folders" to clear the cover.jpg / ' + '.lrc leftovers a Library Reorganize leaves behind — folders whose only ' + 'remaining files are cover/scan images or lyric/metadata sidecars.' ) icon = 'repair-icon-folder' default_enabled = False default_interval_hours = 168 # weekly — empties accrue slowly - default_settings = {'remove_junk_files': True} + default_settings = {'remove_junk_files': True, 'remove_residual_files': False} auto_fix = False def scan(self, context: JobContext) -> JobResult: @@ -86,10 +88,15 @@ class EmptyFolderCleanerJob(RepairJob): root = os.path.realpath(root) ignore_junk = True + ignore_disposable = False try: if context.config_manager: ignore_junk = bool(context.config_manager.get( 'repair.jobs.empty_folder_cleaner.remove_junk_files', True)) + # #891: also clear folders left holding only images / .lrc / sidecars + # (what a reorganize leaves behind). Opt-in — default off. + ignore_disposable = bool(context.config_manager.get( + 'repair.jobs.empty_folder_cleaner.remove_residual_files', False)) except Exception: # noqa: S110 — setting read is best-effort; defaults to True pass @@ -108,17 +115,29 @@ class EmptyFolderCleanerJob(RepairJob): surviving = [d for d in dirnames if os.path.join(dirpath, d) not in flagged] - if not dir_is_removable(filenames, surviving, ignore_junk=ignore_junk): + if not dir_is_removable(filenames, surviving, + ignore_junk=ignore_junk, ignore_disposable=ignore_disposable): result.skipped += 1 continue flagged.add(dirpath) junk = [f for f in filenames if is_junk(f)] + # Files that will be swept along with the folder (junk always; images/ + # sidecars only when the residual option is on). + purgeable = [f for f in filenames + if is_junk(f) or (ignore_disposable and is_disposable(f))] + residual = [f for f in purgeable if not is_junk(f)] rel = os.path.relpath(dirpath, root) if context.report_progress: context.report_progress(log_line=f'Empty folder: {rel}', log_type='info') if context.create_finding: try: + if residual: + extra = f' (only {len(residual)} leftover image/sidecar file(s))' + elif junk: + extra = f' (only {len(junk)} junk file(s))' + else: + extra = '' inserted = context.create_finding( job_id=self.job_id, finding_type='empty_folder', @@ -127,13 +146,13 @@ class EmptyFolderCleanerJob(RepairJob): entity_id=dirpath, file_path=dirpath, title=f'Empty folder: {os.path.basename(dirpath) or rel}', - description=(f'"{rel}" holds no music' - + (f' (only {len(junk)} junk file(s))' if junk else '') - + ' — safe to remove.'), + description=(f'"{rel}" holds no music' + extra + ' — safe to remove.'), details={ 'folder_path': dirpath, 'junk_files': junk, + 'purgeable_files': purgeable, 'remove_junk': ignore_junk, + 'remove_disposable': ignore_disposable, }) if inserted: result.findings_created += 1 @@ -158,12 +177,17 @@ class EmptyFolderCleanerJob(RepairJob): def remove_empty_folder(folder_path: str, *, junk_files: List[str], remove_junk: bool, - root: str, listdir, isdir, islink, remove_file, rmdir) -> dict: + root: str, listdir, isdir, islink, remove_file, rmdir, + remove_disposable: bool = False) -> dict: """Pure-ish orchestration for the apply handler — RE-CHECKS the folder is still - removable, then deletes any junk + the folder. Effects injected for testing. + removable, then deletes any purgeable leftovers + the folder. Effects injected + for testing. - Returns ``{'removed': bool, 'error': str|None}``. Refuses to touch the root, a - symlink, a non-dir, or a folder that gained real content since the scan. + With ``remove_disposable`` (#891) the re-check also treats cover images and + lyric/metadata sidecars as removable, and sweeps them before rmdir. Returns + ``{'removed': bool, 'error': str|None}``. Refuses to touch the root, a symlink, a + non-dir, or a folder that gained REAL content (audio, a booklet, anything not + recognized as residual) since the scan. """ if not folder_path or not isdir(folder_path): return {'removed': False, 'error': 'Folder no longer exists'} @@ -172,19 +196,21 @@ def remove_empty_folder(folder_path: str, *, junk_files: List[str], remove_junk: if root and os.path.realpath(folder_path) == os.path.realpath(root): return {'removed': False, 'error': 'Refusing to remove the library root'} - # Re-check at apply time: only junk/empty now? (Anything else = leave it.) + def _purgeable(e: str) -> bool: + return (remove_junk and is_junk(e)) or (remove_disposable and is_disposable(e)) + + # Re-check at apply time: only purgeable leftovers now? (Anything else = leave it.) entries = list(listdir(folder_path)) - real_entries = [e for e in entries if not (remove_junk and is_junk(e))] + real_entries = [e for e in entries if not _purgeable(e)] if real_entries: return {'removed': False, 'error': 'Folder is no longer empty — left untouched'} - if remove_junk: - for j in entries: - if is_junk(j): - try: - remove_file(os.path.join(folder_path, j)) - except Exception: # noqa: S110 — junk best-effort; rmdir below fails loudly if blocked - pass + for e in entries: + if _purgeable(e): + try: + remove_file(os.path.join(folder_path, e)) + except Exception: # noqa: S110 — leftover best-effort; rmdir below fails loudly if blocked + pass try: rmdir(folder_path) except Exception as e: diff --git a/core/repair_worker.py b/core/repair_worker.py index 1394d8b4..9a552a5f 100644 --- a/core/repair_worker.py +++ b/core/repair_worker.py @@ -1567,6 +1567,7 @@ class RepairWorker: resolved, junk_files=details.get('junk_files') or [], remove_junk=bool(details.get('remove_junk', True)), + remove_disposable=bool(details.get('remove_disposable', False)), root=self.transfer_folder, listdir=os.listdir, isdir=os.path.isdir, islink=os.path.islink, remove_file=os.remove, rmdir=os.rmdir, diff --git a/tests/library/test_residual_files.py b/tests/library/test_residual_files.py new file mode 100644 index 00000000..3c030d4e --- /dev/null +++ b/tests/library/test_residual_files.py @@ -0,0 +1,51 @@ +"""#891: the shared 'residual file' classifier — junk + cover/scan images + +lyric/metadata sidecars — used by both the Reorganize cleanup and the Empty +Folder Cleaner, plus the reorganize sweep that uses it. +""" + +from __future__ import annotations + +from pathlib import Path + +from core.library.residual_files import ( + is_disposable, + is_image, + is_junk, + is_sidecar, +) + + +def test_images_classified(): + for n in ('cover.jpg', 'Cover.JPEG', 'folder.png', 'back.webp', 'scan.tiff', 'art.gif'): + assert is_image(n) and is_disposable(n) + + +def test_sidecars_classified(): + for n in ('lyrics.lrc', 'album.nfo', 'disc.cue', 'playlist.m3u', 'x.m3u8'): + assert is_sidecar(n) and is_disposable(n) + + +def test_junk_classified(): + assert is_junk('.DS_Store') and is_disposable('Thumbs.db') + + +def test_real_content_not_disposable(): + # Audio + anything unrecognized (booklet, video, a note) is real content. + for n in ('song.flac', 'track.mp3', 'booklet.pdf', 'movie.mkv', 'readme.txt', 'data.json'): + assert not is_disposable(n), n + + +# ── the reorganize sweep that uses the predicate ────────────────────────────── +def test_delete_album_sidecars_sweeps_all_residual_keeps_real(tmp_path: Path): + from core.library_reorganize import _delete_album_sidecars + + d = tmp_path / 'Old Album' + d.mkdir() + for n in ('cover.jpg', 'back.jpg', 'disc.png', 'lyrics.lrc', 'album.nfo', '.DS_Store'): + (d / n).write_text('x') + (d / 'booklet.pdf').write_text('keep') # unrecognized → must survive + + _delete_album_sidecars(str(d)) + + survivors = {p.name for p in d.iterdir()} + assert survivors == {'booklet.pdf'} # every residual swept, booklet kept diff --git a/tests/test_empty_folder_cleaner.py b/tests/test_empty_folder_cleaner.py index 54f65ca0..776a8e5a 100644 --- a/tests/test_empty_folder_cleaner.py +++ b/tests/test_empty_folder_cleaner.py @@ -36,6 +36,23 @@ def test_is_junk(): assert is_junk('.DS_Store') and is_junk('thumbs.db') and not is_junk('cover.jpg') +# ── #891: residual (image / sidecar only) folders ─────────────────────────── +def test_image_only_dir_kept_by_default_removed_with_residual_opt(): + # Default (junk only): a cover.jpg keeps the folder (the conservative behavior). + assert dir_is_removable(['cover.jpg'], []) is False + # Opt-in: image/sidecar-only folders become removable. + assert dir_is_removable(['cover.jpg'], [], ignore_disposable=True) is True + assert dir_is_removable(['back.jpg', 'lyrics.lrc', '.DS_Store'], [], ignore_disposable=True) is True + assert dir_is_removable(['folder.png', 'album.nfo'], [], ignore_disposable=True) is True + + +def test_residual_opt_still_keeps_real_content(): + # Audio, or anything not recognized as a leftover (a booklet pdf), still blocks. + assert dir_is_removable(['cover.jpg', 'song.flac'], [], ignore_disposable=True) is False + assert dir_is_removable(['cover.jpg', 'booklet.pdf'], [], ignore_disposable=True) is False + assert dir_is_removable([], ['Album'], ignore_disposable=True) is False # surviving subdir + + # ── apply re-check (real FS) ──────────────────────────────────────────────── def _fx(): return dict(listdir=os.listdir, isdir=os.path.isdir, islink=os.path.islink, @@ -72,3 +89,34 @@ def test_apply_refuses_library_root(tmp_path): res = remove_empty_folder(str(root), junk_files=[], remove_junk=True, root=str(root), **_fx()) assert res['removed'] is False and 'root' in res['error'].lower() assert root.exists() + + +def test_apply_sweeps_residual_then_folder_when_enabled(tmp_path): + root = tmp_path / 'lib'; root.mkdir() + d = root / 'Artist' / 'Old Album'; d.mkdir(parents=True) + (d / 'cover.jpg').write_text('img') + (d / 'back.jpg').write_text('img') + (d / 'lyrics.lrc').write_text('la') + res = remove_empty_folder(str(d), junk_files=[], remove_junk=True, + remove_disposable=True, root=str(root), **_fx()) + assert res['removed'] is True and not d.exists() + + +def test_apply_without_residual_opt_leaves_image_folder(tmp_path): + # The default apply (no residual opt) must NOT delete a cover.jpg folder. + root = tmp_path / 'lib'; root.mkdir() + d = root / 'HasCover'; d.mkdir() + (d / 'cover.jpg').write_text('img') + res = remove_empty_folder(str(d), junk_files=[], remove_junk=True, root=str(root), **_fx()) + assert res['removed'] is False and d.exists() + + +def test_apply_residual_opt_still_refuses_real_content(tmp_path): + root = tmp_path / 'lib'; root.mkdir() + d = root / 'Mixed'; d.mkdir() + (d / 'cover.jpg').write_text('img') + (d / 'booklet.pdf').write_text('pdf') # unrecognized → real content + res = remove_empty_folder(str(d), junk_files=[], remove_junk=True, + remove_disposable=True, root=str(root), **_fx()) + assert res['removed'] is False and d.exists() + assert (d / 'booklet.pdf').exists() and (d / 'cover.jpg').exists() # nothing deleted