Merge pull request #545 from Nezreka/fix/reorganize-orphan-formats-and-unknown-artist-cleanup

Fix/reorganize orphan formats and unknown artist cleanup
This commit is contained in:
BoulderBadgeDad 2026-05-10 20:26:40 -07:00 committed by GitHub
commit c0c3fb7ca5
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 455 additions and 9 deletions

View file

@ -174,6 +174,45 @@ def authed_sources() -> List[dict]:
return out
_UNKNOWN_ARTIST_NAMES = {'unknown artist', 'unknown', ''}
def _is_unknown_artist(artist_name: Optional[str]) -> bool:
if not artist_name:
return True
return str(artist_name).strip().lower() in _UNKNOWN_ARTIST_NAMES
def _looks_like_album_id_title(album_title: Optional[str]) -> bool:
"""Pre-#524 manual-import bug left some albums with a numeric
album_id stored as `albums.title`. Detect that shape so reorganize
can point the user at Unknown Artist Fixer instead of the generic
'run enrichment' hint."""
if not album_title:
return False
stripped = str(album_title).strip()
return len(stripped) >= 6 and stripped.isdigit()
def _unresolvable_reason(album_data: dict, primary_source: str, strict_source: bool) -> str:
"""Reason text for albums reorganize can't place. Surfaces the
Unknown Artist Fixer hint when the row matches the bad-metadata
shape (Unknown Artist OR album-id-as-title) that fixer reads
file tags + re-resolves metadata, which reorganize itself doesn't
do."""
artist = album_data.get('artist_name')
title = album_data.get('title')
if _is_unknown_artist(artist) or _looks_like_album_id_title(title):
return (
"Album has placeholder metadata (Unknown Artist or numeric "
"title) — run the 'Fix Unknown Artists' repair job to "
"recover real artist/album from file tags before reorganize"
)
if strict_source:
return f"Source '{primary_source}' has no usable tracklist for this album"
return "No metadata source ID for this album"
def _resolve_source(album_data: dict, primary_source: str, strict_source: bool = False):
"""Walk the configured source priority looking for the first source
we have an ID for AND that returns a usable tracklist.
@ -552,11 +591,7 @@ def plan_album_reorganize(
album_data, primary_source, strict_source=strict_source
)
if not source:
reason = (
f"Source '{primary_source}' has no usable tracklist for this album"
if strict_source else
"No metadata source ID for this album"
)
reason = _unresolvable_reason(album_data, primary_source, strict_source)
return {
'status': 'no_source_id', 'source': None, 'api_album': None,
'total_discs': 1,
@ -1051,6 +1086,23 @@ def _finalize_track(ctx: _RunContext, track_id, resolved_src, new_path) -> bool:
with ctx.state_lock:
ctx.src_dirs_touched.add(os.path.dirname(resolved_src))
ctx.dst_dirs_touched.add(os.path.dirname(new_path))
# Discord report (Foxxify): users with lossy-copy enabled have
# `track.flac` AND `track.opus` side-by-side. The DB tracks ONE
# (the lossy copy). Reorganize used to move only the canonical
# and leave the orphan behind, blocking empty-folder cleanup.
# Move sibling-format audio to the same destination dir BEFORE
# removing the canonical source, preserving both formats with
# the canonical's renamed stem.
siblings = _find_sibling_audio_files(resolved_src)
for sibling_src in siblings:
moved_to = _move_sibling_to_destination(sibling_src, new_path)
if moved_to:
logger.debug(
"[Reorganize] Moved sibling-format file alongside canonical: %s",
moved_to,
)
try:
os.remove(resolved_src)
except OSError as rm_err:
@ -1208,13 +1260,19 @@ def reorganize_album(
)
if plan['status'] == 'no_source_id':
summary['status'] = 'no_source_id'
summary['errors'].append({
'error': (
if _is_unknown_artist(album_data.get('artist_name')) or _looks_like_album_id_title(album_data.get('title')):
err_text = (
f"Album '{album_data.get('title', '?')}' has placeholder metadata "
"(Unknown Artist or numeric title) — run the 'Fix Unknown Artists' "
"repair job to recover real artist/album from file tags first."
)
else:
err_text = (
f"No reachable metadata source ID for '{album_data.get('title', '?')}'"
"run enrichment first to populate at least one of "
"spotify_album_id / itunes_album_id / deezer_id / discogs_id / soul_id."
),
})
)
summary['errors'].append({'error': err_text})
return summary
source = plan['source']
@ -1450,6 +1508,80 @@ _AUDIO_EXTS = frozenset(
)
def _find_sibling_audio_files(audio_path: str) -> list:
"""Find OTHER audio files at the same source directory that share
the canonical file's stem.
Discord report (Foxxify): users with the lossy-copy feature
enabled end up with `track.flac` AND `track.opus` side-by-side.
Reorganize is DB-driven and only knows about ONE file per track
(the lossy copy in library), so the other format gets left behind
in the old location while the canonical moves to the new
destination. Cleanup never fires because the source dir still has
audio.
This helper returns the orphan-format paths so the caller can
move them alongside the canonical to the new destination dir.
Same stem + audio extension + NOT the canonical itself.
Returns empty list when source dir doesn't exist or read fails
(defensive never raises).
"""
src_dir = os.path.dirname(audio_path)
if not os.path.isdir(src_dir):
return []
stem = os.path.splitext(os.path.basename(audio_path))[0]
canonical_basename = os.path.basename(audio_path)
siblings = []
try:
entries = os.listdir(src_dir)
except OSError:
return []
for name in entries:
if name == canonical_basename:
continue
sibling_stem, ext = os.path.splitext(name)
if sibling_stem != stem:
continue
if ext.lower() not in _AUDIO_EXTS:
continue
full = os.path.join(src_dir, name)
if os.path.isfile(full):
siblings.append(full)
return siblings
def _move_sibling_to_destination(sibling_src: str, canonical_dst: str) -> Optional[str]:
"""Move a sibling-format audio file to the same destination
directory as the canonical, preserving its extension.
Example: canonical at ``/library/Artist/Album/01 Track.opus`` +
sibling source ``/old/01 Track.flac`` destination ``/library/
Artist/Album/01 Track.flac``. The destination filename uses the
canonical's stem (post-template-rename) + the sibling's original
extension so a renamed canonical gets matching siblings.
Returns the destination path on success, None on failure (logged
at warning, doesn't raise — sibling moves are best-effort).
"""
dst_dir = os.path.dirname(canonical_dst)
canonical_stem = os.path.splitext(os.path.basename(canonical_dst))[0]
_, sibling_ext = os.path.splitext(sibling_src)
sibling_dst = os.path.join(dst_dir, canonical_stem + sibling_ext)
if os.path.normpath(sibling_src) == os.path.normpath(sibling_dst):
return sibling_dst # already at the right place
try:
os.makedirs(dst_dir, exist_ok=True)
shutil.move(sibling_src, sibling_dst)
return sibling_dst
except OSError as e:
logger.warning(
"[Reorganize] Couldn't move sibling-format file %s%s: %s",
sibling_src, sibling_dst, e,
)
return None
def _delete_track_sidecars(audio_path: str) -> None:
"""Delete per-track sidecars (.lrc / .nfo / .txt / .cue / .json) that
sit alongside `audio_path` and share its filename stem. Best-effort

View file

@ -0,0 +1,174 @@
"""Pin orphan-format handling in library_reorganize.
Discord report (Foxxify): users with the lossy-copy feature enabled
end up with `track.flac` AND `track.opus` side-by-side. Reorganize is
DB-driven and only knows about ONE file per track (the lossy copy in
the library), so the other format used to get left behind in the old
location while the canonical moved to the new destination. Cleanup
never fired because the source dir still had audio.
Post-fix the reorganize finalisation step finds sibling-stem audio
files at the source and moves them to the same destination dir as
the canonical, preserving both formats.
"""
from __future__ import annotations
import os
from core.library_reorganize import (
_find_sibling_audio_files,
_move_sibling_to_destination,
)
# ---------------------------------------------------------------------------
# Sibling detection
# ---------------------------------------------------------------------------
class TestFindSiblingAudioFiles:
def test_finds_flac_when_opus_is_canonical(self, tmp_path):
"""Reporter's exact case: lossy-copy `.opus` is the
canonical (DB-tracked); `.flac` is the orphan to move."""
opus = tmp_path / "01 Track.opus"
flac = tmp_path / "01 Track.flac"
opus.write_bytes(b"opus-data")
flac.write_bytes(b"flac-data")
siblings = _find_sibling_audio_files(str(opus))
assert siblings == [str(flac)]
def test_finds_opus_when_flac_is_canonical(self):
"""Symmetric direction — works either way."""
# tmp_path fixture handled by next test inline
import tempfile, pathlib
tmp = pathlib.Path(tempfile.mkdtemp())
flac = tmp / "X.flac"
opus = tmp / "X.opus"
flac.write_bytes(b"a"); opus.write_bytes(b"b")
siblings = _find_sibling_audio_files(str(flac))
assert siblings == [str(opus)]
def test_excludes_canonical_itself(self, tmp_path):
"""Canonical must NOT appear in its own sibling list."""
canonical = tmp_path / "X.opus"
canonical.write_bytes(b"data")
siblings = _find_sibling_audio_files(str(canonical))
assert siblings == []
def test_excludes_different_stem(self, tmp_path):
"""Different track in same dir shouldn't be flagged as
sibling only same-stem files."""
canonical = tmp_path / "01 Track One.opus"
other_track = tmp_path / "02 Track Two.flac"
canonical.write_bytes(b"a"); other_track.write_bytes(b"b")
siblings = _find_sibling_audio_files(str(canonical))
assert siblings == []
def test_excludes_non_audio_extensions(self, tmp_path):
"""Sidecars (.lrc, .nfo, .txt) handled by separate sidecar
helper must not appear in audio-sibling list."""
canonical = tmp_path / "X.opus"
sidecar = tmp_path / "X.lrc"
nfo = tmp_path / "X.nfo"
canonical.write_bytes(b"a")
sidecar.write_bytes(b"lyrics")
nfo.write_bytes(b"info")
siblings = _find_sibling_audio_files(str(canonical))
assert siblings == []
def test_finds_multiple_siblings(self, tmp_path):
"""User could have 3+ formats: .flac + .opus + .mp3."""
opus = tmp_path / "X.opus"
flac = tmp_path / "X.flac"
mp3 = tmp_path / "X.mp3"
opus.write_bytes(b"a"); flac.write_bytes(b"b"); mp3.write_bytes(b"c")
siblings = _find_sibling_audio_files(str(opus))
# All formats other than canonical
assert sorted(siblings) == sorted([str(flac), str(mp3)])
def test_missing_source_dir_returns_empty(self, tmp_path):
"""Defensive: source dir vanished mid-reorganize. Return
empty, don't raise."""
siblings = _find_sibling_audio_files(str(tmp_path / "nonexistent" / "X.opus"))
assert siblings == []
# ---------------------------------------------------------------------------
# Sibling move
# ---------------------------------------------------------------------------
class TestMoveSiblingToDestination:
def test_moves_to_same_dir_as_canonical_with_renamed_stem(self, tmp_path):
"""Canonical's renamed stem propagates to siblings — so a
renamed `.opus` (`01 Track.opus`) gets a matching `.flac`
(`01 Track.flac`) at the new location, even if source was
`track-original-name.flac`."""
src_dir = tmp_path / "old"
dst_dir = tmp_path / "Artist" / "Album"
src_dir.mkdir()
sibling_src = src_dir / "track-original-name.flac"
sibling_src.write_bytes(b"flac-data")
canonical_dst = dst_dir / "01 Track.opus"
result = _move_sibling_to_destination(str(sibling_src), str(canonical_dst))
# Sibling at new location with canonical's renamed stem +
# sibling's original extension
expected = dst_dir / "01 Track.flac"
assert result == str(expected)
assert expected.exists()
assert expected.read_bytes() == b"flac-data"
# Source removed
assert not sibling_src.exists()
def test_creates_destination_dir_if_missing(self, tmp_path):
src = tmp_path / "old" / "X.flac"
src.parent.mkdir()
src.write_bytes(b"data")
canonical_dst = tmp_path / "new" / "X.opus"
result = _move_sibling_to_destination(str(src), str(canonical_dst))
assert result is not None
assert (tmp_path / "new" / "X.flac").exists()
def test_no_op_when_source_equals_destination(self, tmp_path):
"""Defensive: if sibling is already at the destination (e.g.
idempotent re-run), return path without raising."""
f = tmp_path / "X.flac"
f.write_bytes(b"data")
canonical_dst = tmp_path / "X.opus"
result = _move_sibling_to_destination(str(f), str(canonical_dst))
# Sibling stays put (same dir as canonical destination)
assert f.exists()
assert result == str(f)
def test_returns_none_on_failure(self, tmp_path, monkeypatch):
"""OS error on move → returns None, doesn't raise. Caller
treats as best-effort (sibling stays at old location, user
sees it next reorganize run)."""
src = tmp_path / "old" / "X.flac"
src.parent.mkdir()
src.write_bytes(b"data")
def fake_move(s, d):
raise OSError("disk full")
monkeypatch.setattr('core.library_reorganize.shutil.move', fake_move)
result = _move_sibling_to_destination(
str(src), str(tmp_path / "new" / "X.opus"),
)
assert result is None

View file

@ -0,0 +1,139 @@
"""Pin the unresolvable-reason hint in library_reorganize.
Discord report (Foxxify) Phase B: stuck "Unknown Artist / <album_id>"
folders left over from the pre-#524 manual-import bug. Reorganize
couldn't move them (no usable metadata source ID) and emitted a
generic "run enrichment first" message but enrichment can't fix
these rows. The right tool is the existing Unknown Artist Fixer
repair job (reads file tags, re-resolves metadata, re-tags + moves
file). These tests pin the detection helpers + reason text so the
hint stays correct as the file evolves.
"""
from __future__ import annotations
from core.library_reorganize import (
_is_unknown_artist,
_looks_like_album_id_title,
_unresolvable_reason,
)
class TestIsUnknownArtist:
def test_unknown_artist_string(self):
assert _is_unknown_artist("Unknown Artist") is True
def test_unknown_artist_lowercase(self):
assert _is_unknown_artist("unknown artist") is True
def test_unknown_artist_with_whitespace(self):
assert _is_unknown_artist(" Unknown Artist ") is True
def test_unknown_alone(self):
"""Some import paths set just 'Unknown' (no 'Artist' suffix)."""
assert _is_unknown_artist("Unknown") is True
def test_empty_string(self):
assert _is_unknown_artist("") is True
def test_none(self):
assert _is_unknown_artist(None) is True
def test_real_artist(self):
assert _is_unknown_artist("Radiohead") is False
def test_artist_containing_unknown_substring(self):
"""Substring 'unknown' shouldn't trigger — only the exact
placeholder names. Real artists can contain that word."""
assert _is_unknown_artist("Unknown Mortal Orchestra") is False
class TestLooksLikeAlbumIdTitle:
def test_long_numeric_string_is_album_id(self):
"""Reporter's case: album.title set to the numeric album_id
by the pre-#524 manual-import bug."""
assert _looks_like_album_id_title("1234567890") is True
def test_six_digit_minimum(self):
"""Edge: 5 digits is too short to be a real album_id pattern
could just be an album titled '12345'. Cutoff is 6+."""
assert _looks_like_album_id_title("12345") is False
assert _looks_like_album_id_title("123456") is True
def test_alphanumeric_is_not_album_id(self):
"""Real album titles with numbers (Blink-182, Sum 41, etc.)
must not trigger."""
assert _looks_like_album_id_title("Sum 41") is False
assert _looks_like_album_id_title("1999") is False # short
def test_empty_string(self):
assert _looks_like_album_id_title("") is False
def test_none(self):
assert _looks_like_album_id_title(None) is False
def test_real_album_title(self):
assert _looks_like_album_id_title("In Rainbows") is False
def test_whitespace_stripped(self):
"""Defensive: leading/trailing whitespace shouldn't fool the
detector."""
assert _looks_like_album_id_title(" 1234567890 ") is True
class TestUnresolvableReason:
def test_unknown_artist_routes_to_fixer_hint(self):
"""Reporter's exact case — Unknown Artist row should point
at the Fix Unknown Artists repair job, not generic
enrichment advice."""
reason = _unresolvable_reason(
{'artist_name': 'Unknown Artist', 'title': 'Some Album'},
primary_source='deezer',
strict_source=False,
)
assert "Fix Unknown Artists" in reason
assert "placeholder metadata" in reason
def test_album_id_title_routes_to_fixer_hint(self):
"""Reverse case — album.title is a numeric album_id."""
reason = _unresolvable_reason(
{'artist_name': 'Real Artist', 'title': '9876543210'},
primary_source='deezer',
strict_source=False,
)
assert "Fix Unknown Artists" in reason
def test_real_album_with_no_source_id_keeps_enrichment_hint(self):
"""Sanity: real artist + real title but no source ID still
gets the generic enrichment hint. Don't mis-route normal
no-source-ID albums into the fixer flow."""
reason = _unresolvable_reason(
{'artist_name': 'Radiohead', 'title': 'In Rainbows'},
primary_source='deezer',
strict_source=False,
)
assert "Fix Unknown Artists" not in reason
assert "No metadata source ID" in reason
def test_strict_source_path_keeps_strict_text(self):
"""When strict_source=True and the row is fine (real artist
+ real title), the existing strict-source message is
preserved. Hint only fires for the bad-metadata shape."""
reason = _unresolvable_reason(
{'artist_name': 'Radiohead', 'title': 'In Rainbows'},
primary_source='spotify',
strict_source=True,
)
assert "Fix Unknown Artists" not in reason
assert "spotify" in reason.lower()
assert "tracklist" in reason
def test_strict_source_with_unknown_artist_prefers_fixer_hint(self):
"""Bad-metadata shape wins over strict-source — Unknown
Artist always needs the fixer regardless of source mode."""
reason = _unresolvable_reason(
{'artist_name': 'Unknown Artist', 'title': 'Whatever'},
primary_source='spotify',
strict_source=True,
)
assert "Fix Unknown Artists" in reason

View file

@ -3416,6 +3416,7 @@ const WHATS_NEW = {
'2.4.3': [
// --- post-release patch work on the 2.4.3 line — entries hidden by _getLatestWhatsNewVersion until the build version bumps ---
{ date: 'Unreleased — 2.4.3 patch work' },
{ title: 'Library Reorganize: Stop Leaving Orphan Audio Files Behind + Hint For Unknown-Artist Rows', desc: 'discord report (foxxify): library reorganize wasn\'t organizing everything. two distinct gaps. (A) lossy-copy users have `track.flac` AND `track.opus` side-by-side at the source; the db only knows about ONE of those (whichever is the canonical library entry). reorganize moved the canonical, left the other format orphaned at the old location, and the empty-folder cleanup never fired because the source dir still had audio in it. fix: at the per-track finalize step the reorganize code now scans the source dir for sibling-stem audio files (same filename stem, audio extension, different format), moves them to the same destination dir as the canonical with the renamed stem + their original extension, then proceeds with the existing source removal + cleanup. preserves both formats post-move so users keep their flac archive AND their opus library copy. (B) old "Unknown Artist / album_id / 0 tracks" rows left over from the pre-#524 manual-import bug couldn\'t be relocated because the album row has no usable metadata source id — reorganize emitted a generic "run enrichment first" message that doesn\'t apply (enrichment can\'t fix these rows; they need their real metadata recovered from file tags). these are the existing `Fix Unknown Artists` repair job\'s domain — reads file tags, re-resolves artist/album/track via configured metadata source, re-tags + moves. reorganize now detects the bad-metadata shape (Unknown Artist OR album.title that\'s a 6+ digit numeric id) and emits a clear "run the Fix Unknown Artists repair job to recover real artist/album from file tags first" hint instead, pointing the user at the right tool. fixer was already implemented and handles the case end-to-end — discoverability gap, not a logic gap. 31 new tests pin: orphan-format detection (canonical-vs-sibling, multi-format, defensive on missing source dir, sidecar exclusion), sibling-move with renamed-stem propagation + dst-dir creation + idempotent re-runs + os-failure handling, and the unknown-artist-hint detection helpers (placeholder names, numeric-id title detection at 6+ digit cutoff, real-album-with-no-source-id keeps the generic enrichment hint, strict-source mode preserved when artist/title look fine).', page: 'library' },
{ title: 'AcoustID Scanner: Compilation Albums No Longer Flag Every Track', desc: 'discord report (skowl): downloaded a compilation album like "high tea music: vol 1" where every track has a different artist (eclypse, andromedik, t & sugah, gourski, himmes, sektor, lexurus, etc.) and the acoustid scanner flagged every single track as wrong song — the file tag had the correct per-track artist (e.g. "eclypse" for "city lights") but the scanner compared against the album-level artist ("andromedik", the curator). raw similarity 12% → wrong song flag. the multi-value-credit fix from the prior pr (foxxify) didn\'t help because both sides were single-value but DIFFERENT artists. cause: scanner sql joined `artists` table via `tracks.artist_id` which points at the ALBUM artist, not the per-track artist. but `tracks.track_artist` column was already populated with the correct per-track value by every server scan + auto-import path that handles compilations. scanner just wasn\'t reading it. fix: changed the scanner select to `COALESCE(NULLIF(t.track_artist, \'\'), ar.name)` — prefers per-track artist when populated, falls back to album artist for legacy rows / single-artist albums where track_artist is null. NULLIF handles the empty-string-instead-of-null case for legacy data. composes with foxxify\'s multi-value fix — for the rare compilation track where acoustid ALSO returns a multi-value credit, both paths work together. 2 new tests pin: compilation track uses per-track artist (reporter\'s exact case), null/empty track_artist falls back to album artist via coalesce.', page: 'library' },
{ title: 'AcoustID Scanner: Multi-Artist Songs No Longer Flagged As Wrong', desc: 'discord report (foxxify): the acoustid scanner repair job was flagging multi-artist tracks as "wrong song" because acoustid returns the full credit ("okayracer, aldrch & poptropicaslutz!") while the library db carries only the primary artist ("okayracer"). raw similarity scored ~43% — well below the 60% threshold — so the scanner created a wrong-song finding even though the audio was correct. user couldn\'t fix without lowering the global artist threshold to ~30% (which would let real mismatches through). cause: scanner used raw `SequenceMatcher` comparison that doesn\'t recognise the primary artist is just one of several contributors in the credit string. fix: extended the shared `core/matching/artist_aliases.py::artist_names_match` helper (lifted in #441) with credit-token splitting on common separators (comma, ampersand, semicolon, slash, plus, "feat.", "ft.", "featuring", "with", "vs.", "x"). when actual artist contains separators, helper splits into individual contributors and checks each against expected — primary-in-credit cases now resolve at 100% instead of 43%. composes with existing alias path so cross-script multi-artist credits ("hiroyuki sawano" expected, "澤野弘之, featured" actual) work too. wired into `core/repair_jobs/acoustid_scanner.py` — replaces the raw similarity call. acoustid post-download verifier already used the helper from #441 so it inherits the same fix automatically. 14 new tests pin: split-by-separator across 12 credit-string formats, primary at start/middle/end of credit, no-mask on genuine mismatches, single-token actual falls through to direct compare, multi-value composes with aliases, threshold still respected, end-to-end scanner integration with reporter\'s exact case (okayracer in okayracer-aldrch-poptropicaslutz credit → no finding), end-to-end scanner still flags genuine mismatches.', page: 'library' },
{ title: 'Deezer Cover Art: Embedded Covers No Longer Look Blurry', desc: 'discord report (tim): downloaded cover art via deezer metadata source came out visibly blurry in navidrome and on phones — particularly noticeable on large displays. cause: deezer\'s api returns `cover_xl` urls at 1000×1000 but the underlying cdn serves up to 1900×1900 by rewriting the size segment in the url path. soulsync wasn\'t doing the rewrite — same as iTunes mzstatic and spotify scdn already get upgraded. now `_upgrade_deezer_cover_url` (mirrors `_upgrade_spotify_image_url` pattern) rewrites the cdn url to request 1900×1900 before download. cdn serves source-native size when source < target so asking for 1900 on smaller-source albums returns the same bytes (no upscaling, no failure). applied at both download sites — auto post-process flow + the enhanced library view\'s "write tags to file" feature. existing `prefer_caa_art` toggle in settings → library → post-processing remains as the orthogonal workaround for users who want even higher quality (musicbrainz cover art archive, often 3000×3000+). 16 new tests pin: standard upgrade, alternate dzcdn host, artist picture urls, custom target sizes, idempotency on already-upgraded urls, defensive on non-deezer urls (spotify/itunes/caa/lastfm/random), empty/none handling.', page: 'settings' },