diff --git a/core/imports/filename.py b/core/imports/filename.py index 92ff026a..7fa9c420 100644 --- a/core/imports/filename.py +++ b/core/imports/filename.py @@ -15,8 +15,32 @@ _TRACK_PATTERNS = ( def extract_track_number_from_filename(filename: str, title: str = None) -> int: - """Extract track number from a filename. Returns 1 if not found.""" - basename = os.path.splitext(os.path.basename(filename))[0].strip() + """Extract track number from a filename. Returns 1 if not found. + + Use ``extract_explicit_track_number`` instead when the caller needs + to distinguish "track 1" from "unknown" — staging-file readers in + particular MUST NOT conflate a bare title (no numeric prefix) with + track 1, or every untagged album-bundle file gets imported as + ``track_number=1`` and downstream callers can't recover the real + number from authoritative metadata (Spotify track list, etc.). + """ + num = extract_explicit_track_number(filename) + return num if num > 0 else 1 + + +def extract_explicit_track_number(filename: str) -> int: + """Extract a track number only when the filename visibly carries one. + + Returns the parsed track number when the basename starts with a + recognizable numeric prefix (``"01 - Title"``, ``"1-03 Title"``, + ``"(01) Title"``, ``"[01] Title"``); returns ``0`` when no such + prefix is present. This is the contract staging readers want — + "unknown" must stay unknown so a downstream consumer with better + info (Spotify metadata, MusicBrainz, etc.) can fill it in. + """ + basename = os.path.splitext(os.path.basename(str(filename or "")))[0].strip() + if not basename: + return 0 match = re.match(r"^\d[\-\.](\d{1,2})\s*[\-\.]\s*", basename) if match: @@ -30,7 +54,7 @@ def extract_track_number_from_filename(filename: str, title: str = None) -> int: if 1 <= num <= 999: return num - return 1 + return 0 def parse_filename_metadata(filename: str) -> Dict[str, Any]: diff --git a/core/imports/staging.py b/core/imports/staging.py index 748cbb11..76a42beb 100644 --- a/core/imports/staging.py +++ b/core/imports/staging.py @@ -7,7 +7,10 @@ import threading from typing import Any, Dict, Iterable, List, Optional, Tuple from core.imports.paths import docker_resolve_path -from core.imports.filename import extract_track_number_from_filename +from core.imports.filename import ( + extract_explicit_track_number, + extract_track_number_from_filename, +) from utils.logging_config import get_logger logger = get_logger("imports.staging") @@ -103,12 +106,19 @@ def read_staging_file_metadata(file_path: str, filename: Optional[str] = None) - if not albumartist: albumartist = artist - track_number = extract_track_number_from_filename(filename or file_path) + # Use the strict extractor here: when the filename has no visible + # track-number prefix, return 0 instead of pretending it's track 1. + # Downstream consumers (staging match in core/downloads/staging.py) + # will then fall through to authoritative metadata (track_info from + # the original Spotify / API source) rather than locking the import + # to track_number=1 for every file in the bundle. + track_number = extract_explicit_track_number(filename or file_path) try: - # Preserve tag-based numbers when present, but still fall back to the filename parser. tag_track_number = _first_tag("tracknumber", "track_number") if tag_track_number: - track_number = int(str(tag_track_number).split("/")[0].strip() or track_number) + parsed_tag = int(str(tag_track_number).split("/")[0].strip()) + if parsed_tag > 0: + track_number = parsed_tag except (TypeError, ValueError): pass diff --git a/tests/imports/test_import_file_ops.py b/tests/imports/test_import_file_ops.py index 16e68ad7..ce9ecd4a 100644 --- a/tests/imports/test_import_file_ops.py +++ b/tests/imports/test_import_file_ops.py @@ -5,16 +5,40 @@ from core.imports.file_ops import ( cleanup_empty_directories, safe_move_file, ) -from core.imports.filename import extract_track_number_from_filename +from core.imports.filename import ( + extract_explicit_track_number, + extract_track_number_from_filename, +) from core.imports.staging import read_staging_file_metadata def test_extract_track_number_from_filename_handles_common_patterns(): assert extract_track_number_from_filename("01 - Song.mp3") == 1 assert extract_track_number_from_filename("1-03 - Song.mp3") == 3 + # Bare filename keeps the auto-import-friendly default of 1 — there's + # no upstream metadata to recover from in that flow. assert extract_track_number_from_filename("Artist - Song.mp3") == 1 +def test_extract_explicit_track_number_returns_zero_when_no_prefix(): + """Staging readers need to distinguish 'track 1' from 'unknown'. + + Pinned because: + - the legacy extractor defaults to 1 (auto-import semantics), + - staging file scanners that conflate the two end up writing every + file in an untagged album bundle to track_number=1. + """ + # Bare titles with no numeric prefix → 0 (unknown). + assert extract_explicit_track_number("Artist - Song.mp3") == 0 + assert extract_explicit_track_number("Cha-La Head-Cha-La.flac") == 0 + assert extract_explicit_track_number("") == 0 + # Real prefixes still parse correctly. + assert extract_explicit_track_number("01 - Song.mp3") == 1 + assert extract_explicit_track_number("(03) Song.mp3") == 3 + # Disc-track format requires a separator after the track number. + assert extract_explicit_track_number("1-07 - Song.mp3") == 7 + + def test_safe_move_file_replaces_existing_destination(tmp_path): src = tmp_path / "source.flac" dst_dir = tmp_path / "dest" @@ -92,6 +116,27 @@ def test_read_staging_file_metadata_falls_back_to_filename_track_number(monkeypa assert metadata["disc_number"] == 1 +def test_read_staging_file_metadata_returns_zero_track_when_unknown(monkeypatch, tmp_path): + """Bare filename + no tags → track_number=0, not 1. + + Pre-fix this returned 1 because the filename extractor's default + was 1. The bug caused every untagged file in an album-bundle + download to land in the staging cache with track_number=1, which + then short-circuited the downstream resolution chain that should + have picked up the real number from track_info. + """ + file_path = tmp_path / "Cha-La Head-Cha-La.flac" + file_path.write_text("fake") + + fake_mutagen = types.ModuleType("mutagen") + fake_mutagen.File = lambda path, easy=True: None + monkeypatch.setitem(sys.modules, "mutagen", fake_mutagen) + + metadata = read_staging_file_metadata(str(file_path), file_path.name) + + assert metadata["track_number"] == 0 + + def test_read_staging_file_metadata_uses_filename_fallbacks_when_tags_are_invalid(monkeypatch, tmp_path): file_path = tmp_path / "02 - Song Three.flac" file_path.write_text("fake") diff --git a/webui/static/helper.js b/webui/static/helper.js index bc1bd645..ce7a9108 100644 --- a/webui/static/helper.js +++ b/webui/static/helper.js @@ -3421,6 +3421,7 @@ const WHATS_NEW = { { title: 'ListenBrainz Sync tab', desc: 'new ListenBrainz tab on the Sync page, between Beatport and Import. lists your "For You" / "My Playlists" / "Collaborative" LB playlists in one place. clicking a card kicks off the same discovery → sync → mirror flow you already get from the Discover page (no duplicate UI behind the scenes, just a new entry point). once mirrored, LB playlists participate in Auto-Sync schedules + pipeline automations like any other source. needs ListenBrainz connected in Settings → Connections.', page: 'sync' }, { title: 'Last.fm Radio Sync tab', desc: 'sibling to the ListenBrainz tab — lists your generated Last.fm Radio playlists alongside the rest of the Sync sources. same discovery → mirror flow under the hood, just a different entry point. new Last.fm radios are still generated from the Discover page by picking a seed track; this tab is for syncing existing ones. mirrors auto-trim when Last.fm Radio cache rotates so old radios don\'t pile up.', page: 'sync' }, { title: 'SoulSync Discovery Sync tab', desc: 'last of the unified-tab trio. surfaces your personalized SoulSync Discovery playlists (decade mixes, hidden gems, popular picks, daily mixes, discovery shuffle, etc.) on the Sync page. clicking a card regenerates the playlist + mirrors it under a stable synthetic id, so the same mirror updates in place every Auto-Sync refresh. tracks come out already matched against Spotify / iTunes / Deezer so there\'s no discovery hop — straight to download / sync.', page: 'sync' }, + { title: 'Fix: album-bundle downloads all landing as track 1', desc: 'soulseek album-bundle downloads (and any other untagged release-staging path) were importing every track with track_number=1. the staging-file reader was using the auto-import\'s filename extractor that defaults to 1 when no NN- prefix is present in the filename — so for albums like Ryoto\'s "Cha-La Head-Cha-La" where slskd hands you bare titles, every file got "track 1" stamped on it. now the staging path uses a strict extractor that returns 0 when it can\'t see an explicit prefix, so the downstream resolver correctly falls through to the authoritative Spotify metadata and the right track numbers land in the library.' }, ], '2.6.2': [ { date: 'May 24, 2026 — 2.6.2 release' },