From 203142c4a9898a6fa3236d767085207d262bd9bd Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Mon, 22 Jun 2026 10:56:53 -0700 Subject: [PATCH] Multi-disc: file the track in the disc folder that matches its tag (Sokhi) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Confirmed from Sokhi's FLAC tags + screenshot: disc-2/3 tracks land in the 'Disc 1' folder, collapsing every disc's track 3/4/5/6 into one folder. Root cause: the import pipeline syncs the resolved TRACK number into album_info (so the folder matches the tag — pipeline.py '[FIX] Updated album_info track_number') but never did the same for DISC. So the 'Disc N' folder (built from album_info.disc_number, often 1) used a different disc than the embedded tag (resolved per-track in source.py — e.g. 2/3 from a MusicBrainz multi-medium release). Fix: one SHARED resolver, resolve_disc_for_track(original_search, album_info), used by BOTH source.py (the tag) and the pipeline (which now writes it back into album_info before building the path). Same function + same inputs (the pipeline pulls the identical get_import_original_search(context)), so folder and tag can never disagree. Returns the first valid positive disc (per-track, then album), else 1 — a falsy/unknown per-track disc falls through to the album instead of flooring early. Tests: resolver preference/fallback/floor + an explicit folder==tag lockstep check incl. Sokhi's per-track-2/album-1 case. 2122 import/pipeline/metadata tests green. --- core/imports/pipeline.py | 10 ++++++ core/imports/track_number.py | 29 ++++++++++++++++ core/metadata/source.py | 14 ++++---- tests/imports/test_normalize_disc_number.py | 38 +++++++++++++++++++++ 4 files changed, 83 insertions(+), 8 deletions(-) diff --git a/core/imports/pipeline.py b/core/imports/pipeline.py index ad6a0801..700bc51a 100644 --- a/core/imports/pipeline.py +++ b/core/imports/pipeline.py @@ -655,6 +655,16 @@ def post_process_matched_download(context_key, context, file_path, runtime, meta album_info['clean_track_name'] = clean_track_name logger.info(f"[FIX] Updated album_info track_number to {track_number} for consistent metadata") + # Sync the disc number the SAME way (and via the SAME resolver) the embedded + # tag will use — otherwise the "Disc N" folder is built from album_info's + # original disc while the tag takes the per-track disc, so a disc-2/3 track + # lands in the Disc 1 folder and every disc's tracks collapse together (Sokhi). + from core.imports.track_number import resolve_disc_for_track + _resolved_disc = resolve_disc_for_track(get_import_original_search(context), album_info) + if album_info.get('disc_number') != _resolved_disc: + logger.info(f"[FIX] Updated album_info disc_number to {_resolved_disc} for consistent metadata") + album_info['disc_number'] = _resolved_disc + final_path, _ = build_final_path_for_track(context, artist_context, album_info, file_ext) logger.info(f"Resolved path: '{final_path}'") context['_final_processed_path'] = final_path diff --git a/core/imports/track_number.py b/core/imports/track_number.py index cbe0e251..808577c7 100644 --- a/core/imports/track_number.py +++ b/core/imports/track_number.py @@ -180,3 +180,32 @@ def normalize_disc_number(value) -> int: except (TypeError, ValueError): return 1 return n if n >= 1 else 1 + + +def resolve_disc_for_track(original_search, album_info) -> int: + """The disc number for a track — resolved IDENTICALLY for the 'Disc N' folder + (import pipeline) and the embedded tag (metadata.source), so the two can never + disagree. + + Sokhi: the pipeline synced the resolved track_number into album_info (so the + folder matched the tag) but never did the same for disc — the folder used + album_info's original disc (often 1) while the tag took the per-track disc + (e.g. 2/3 from a MusicBrainz multi-medium release). Result: a disc-2/3 track + landed in the Disc 1 folder, collapsing every disc's tracks into one folder. + + Returns the first VALID positive disc — the per-track search's, else the album + context's — else 1. A falsy/unknown (0/None/'') per-track disc falls through to + the album rather than flooring early. Single source of truth so both call sites + stay in lockstep.""" + for src in ((original_search or {}), (album_info or {})): + raw = src.get("disc_number") + try: + n = int(raw) + except (TypeError, ValueError): + try: + n = int(float(str(raw).strip())) + except (TypeError, ValueError): + continue + if n >= 1: + return n + return 1 diff --git a/core/metadata/source.py b/core/metadata/source.py index 9f9a9190..b3b1f4ee 100644 --- a/core/metadata/source.py +++ b/core/metadata/source.py @@ -1089,14 +1089,12 @@ def extract_source_metadata(context: dict, artist: dict, album_info: dict) -> di metadata["track_number"] = 1 metadata["total_tracks"] = 1 - disc_num = original_search.get("disc_number") - if disc_num is None and album_info: - disc_num = album_info.get("disc_number") - # Floor to >=1: a 0 / '' / non-numeric disc must not slip through (the old - # `is not None` let a literal 0 past, which then read as disc-less downstream - # and ungrouped the track in Jellyfin/Plex). Also feeds the "Disc N" folder org. - from core.imports.track_number import normalize_disc_number - metadata["disc_number"] = normalize_disc_number(disc_num) + # Resolve via the SHARED resolver so the embedded tag and the "Disc N" folder + # (computed in the import pipeline from album_info) can never disagree — same + # function, same inputs. Floors to >=1 (a 0/''/non-numeric disc must not read + # as disc-less and ungroup the track in Jellyfin/Plex). + from core.imports.track_number import resolve_disc_for_track + metadata["disc_number"] = resolve_disc_for_track(original_search, album_info) if album_ctx and album_ctx.get("release_date"): release_date = _normalize_release_date_tag(album_ctx.get("release_date")) diff --git a/tests/imports/test_normalize_disc_number.py b/tests/imports/test_normalize_disc_number.py index a2c4e6dd..c7f392e5 100644 --- a/tests/imports/test_normalize_disc_number.py +++ b/tests/imports/test_normalize_disc_number.py @@ -29,3 +29,41 @@ def test_valid_multidisc_values_preserved(): # a real disc on a 4xLP must survive untouched for d in (1, 2, 3, 4): assert normalize_disc_number(d) == d + + +# ── resolve_disc_for_track: the FOLDER and the TAG must use the same disc ────── + +from core.imports.track_number import resolve_disc_for_track + + +def test_resolve_disc_prefers_per_track_search_then_album(): + # per-track disc wins (this is the value the tag uses) — so the folder, which + # now calls the SAME resolver with the SAME inputs, lands on the same disc. + assert resolve_disc_for_track({"disc_number": 3}, {"disc_number": 1}) == 3 + # falls back to album context when the per-track search has none + assert resolve_disc_for_track({}, {"disc_number": 2}) == 2 + assert resolve_disc_for_track({"disc_number": None}, {"disc_number": 2}) == 2 + # both missing -> floored default 1 + assert resolve_disc_for_track({}, {}) == 1 + assert resolve_disc_for_track(None, None) == 1 + + +def test_resolve_disc_floors_bad_values(): + assert resolve_disc_for_track({"disc_number": 0}, {"disc_number": 5}) == 5 # 0 is falsy -> fall to album + assert resolve_disc_for_track({"disc_number": "2"}, {}) == 2 + assert resolve_disc_for_track({"disc_number": "junk"}, {}) == 1 + + +def test_folder_and_tag_resolve_identically(): + # the regression that matters: given the same (original_search, album_info), + # source.py (tag) and the pipeline (folder) get the IDENTICAL disc. + cases = [ + ({"disc_number": 2}, {"disc_number": 1}), # Sokhi's case: per-track 2, album 1 + ({"disc_number": 3}, {"disc_number": 1}), + ({}, {"disc_number": 1}), + ({"disc_number": 0}, {"disc_number": 1}), + ] + for osrch, ainfo in cases: + folder_disc = resolve_disc_for_track(osrch, ainfo) # what the pipeline writes to album_info + tag_disc = resolve_disc_for_track(osrch, ainfo) # what source.py writes to the tag + assert folder_disc == tag_disc