Commit graph

8 commits

Author SHA1 Message Date
BoulderBadgeDad
213592821b #890: strip leading track-number prefix from filename-derived titles
Files named '01 - Sun It Rises.flac' with no embedded title tag leaked the stem,
number and all, into tracks.title as '01 - Sun It Rises' — which never matches the
canonical 'Sun It Rises', so the real track reads as a false 'missing' and albums
sort wrong.

New conservative strip_leading_track_number (paths.py): removes a clear track-number
prefix (zero-padded number, OR a number followed by a real separator+space) while
leaving titles that merely start with a number untouched — '7 Rings', '99 Luftballons',
'50 Ways to Leave Your Lover', '1-800-273-8255', '1979' all preserved. Never reduces
to empty/bare-number/punctuation.

Applied at:
- get_import_clean_title (context.py) — the universal resolver every import path funnels
  through, so the DB title AND the re-written embedded tag come out clean.
- album_matching scorer — so '01 - Sun It Rises' scores against 'Sun It Rises' and the
  file matches its real track (inheriting the clean canonical name).

27 targeted tests + 772 imports/matching green.
2026-06-18 18:35:04 -07:00
BoulderBadgeDad
08b73f0e94 Playlists: remove dead _playlist_folder_mode routing branches (retired flag, now unreachable) 2026-06-12 18:52:57 -07:00
BoulderBadgeDad
1d16ac7978 Downloads: reuse an album's existing folder so batches don't split it (#829)
Tacobell444: when tracks land in an album across multiple batches (a wishlist
run, the Album Completeness job, a missed track re-downloaded later), the folder
is rebuilt from API metadata each time — so when $albumtype or $year come back
blank/different on a later batch, the folder NAME changes and the album splits,
forcing a Reorganize.

Fix: build_final_path_for_track now checks whether the album already lives in a
single folder on disk and, if so, drops the new track there instead of a freshly
templated folder. Match (chosen): exact stored Spotify album id first, then a
STRICT >=0.85 name+artist match (vs the 0.7 used elsewhere) — a wrong match here
misplaces a file. New core/library/existing_album_folder.resolve_existing_album_folder
holds the logic; always-on with template fallback.

Safety rails: only returns a folder UNDER the transfer dir (never a read-only
library/NAS mount), only when the album lives in EXACTLY ONE folder (multiple =
disc subfolders, which DatabaseTrack can't disambiguate — those defer to the
template), and any failure falls through to the template path. Added
MusicDatabase.get_album_by_spotify_album_id for the id-first lookup.

Tests: single-folder reuse, no-match, below-threshold, multi-folder defer,
outside-transfer reject, id-first, missing transfer dir, no-files-on-disk.
8 tests; 1556 path/import/download tests pass (only the known soundcloud
failures remain).
2026-06-09 13:47:25 -07:00
BoulderBadgeDad
3b49ac8280 Fix #767: Library Organizer dry run no longer creates folders
The reorganize preview (dry run) was physically creating destination album
folders, littering the library with empty dirs and making "changes" before the
user ever hit Apply.

Cause: preview_album_reorganize calls build_final_path_for_track purely to
COMPUTE the destination path string — but that shared helper has 9 os.makedirs
side effects (it's also the live download/import path builder, where creating
the dir is correct). So computing the preview path created "Lenka (Expanded
Edition)/" on disk.

Fix: build_final_path_for_track gains create_dirs=True; all 9 makedirs now route
through a gated helper. The reorganize PREVIEW passes create_dirs=False, so a
dry run computes the exact destination path with zero filesystem side effects.
Everything else keeps the default True:
- the download/import post-process flow (still writes files into the dir),
- retag,
- the reorganize APPLY path — verified it goes through post_process_fn (the real
  pipeline → build_final_path_for_track with create_dirs=True), so live moves
  still create their destination dirs. The gate only silences the dry run.

Tests: tests/imports/test_import_paths.py — create_dirs=False computes the
correct path (matching the reported "01 - The Show.flac") but writes NOTHING to
disk (not even the Transfer root); create_dirs=True still creates folders; both
yield an identical path. Updated two reorganize-orchestrator test doubles to
accept the new kwarg. 148 reorganize/paths/retag/pipeline tests pass.

Does NOT fix the second half of #767 (Expanded Edition picked over the standard
album). That is NOT a reorganizer bug: the library album row was linked to the
deluxe release at enrichment time (its stored spotify_album_id/itunes_album_id/
deezer_id points at "Lenka (Expanded Edition)"), and the reorganizer faithfully
reorganizes to whatever the album is linked to. The real fix is in album
enrichment's edition preference — tracked separately.
2026-06-02 10:32:06 -07:00
BoulderBadgeDad
443257915c Path builder: validate $year, never blind-slice release_date (#745)
The $year template variable was a blind release_date[:4] slice. When
something upstream poisoned release_date with a non-date value — the album
NAME — that slice emitted garbage: 'Mantras (Deluxe)'[:4] -> 'Mant', so
every download landed in 'Mantras (Deluxe) (Mant) [Album]/' instead of
'(2026)' (Tacobell444's screenshot).

Add _extract_year_from_release_date(): returns the leading 4 chars only
when they're a plausible year (isdigit, 1900 < y <= 2100), else ''. Matches
the guard the codebase already uses in soulid_worker._extract_year. A
non-year resolves to '' and the template's existing empty-() cleanup drops
it, so a poisoned release_date can never write rubbish into the path again.

This is the shared post-process path builder
(core/imports/paths.build_final_path_for_track) that DOWNLOADS, reorganize,
and imports all route through, so the guard covers every surface at once.

Defensive fix only — it stops the SYMPTOM regardless of which upstream
writes the album name into release_date. Pinning that upstream needs the
reporter's metadata source + the release_date value from app.log (the
Soulseek + AcoustID + future-dated-album combo is the discriminator);
tracked separately.

Tests (tests/imports/test_import_paths.py): unit coverage for the helper
(real dates kept, names/sentinels/short values rejected) + an integration
test reproducing #745 — a poisoned release_date yields 'Mantras (Deluxe)
[Album]' not '(Mant)' — differential-verified it produces the exact
'(Mant)' folder without the fix. Positive control keeps real (2026). 395
import + reorganize tests green.
2026-05-30 00:31:14 -07:00
Broque Thomas
aa54bed818 Surface silent exceptions across remaining modules — ~70 sites
Final sweep. Covers:
- Downloads: candidates / lifecycle / master / monitor / wishlist_failed
- Metadata: source / registry / cache / common / artwork (+ plex_client)
- Imports: pipeline / resolution / file_ops / paths / guards
- Library: path_resolver / retag / duplicate_cleaner
- Stats / playlists / wishlist / discovery / automation / enrichment
- Misc: hydrabase_client, soulsync_client, tag_writer, debug_info,
  api_call_tracker, album_consistency, beatport_unified_scraper,
  reorganize_runner, seasonal_discovery, lidarr_download_client,
  services/sync_service.py, automation_engine, automation/progress

Two `_e` renames in imports/file_ops.py (outer scope binding `e`).
A few finally-block sites in metadata/album_mbid_cache.py,
library/track_identity.py, listening_stats_worker.py, watchlist/
auto_scan.py left silent — same reason as the rest of the sweep
(logger calls during cleanup paths can themselves raise).

Refs #369
2026-05-07 10:28:58 -07:00
Broque Thomas
d97d105b97 fix: substitute \$cdnum in download paths and skip auto disc folder when template uses it
User report: multi-disc albums on the latest dev had literal "\$cdnum"
in their filenames instead of the expected "CDxx" label, plus a
redundant "Disc N" folder on top of the in-filename label.

Two bugs in core/imports/paths.py:

1. _replace_template_variables (the substitution helper used by every
   download path builder) had no handling for \$cdnum or \${cdnum}. The
   matching helper in web_server.py and core/repair_jobs/library_reorganize.py
   did the substitution; this one didn't, so production downloads passed
   the placeholder through unchanged. Added a cdnum_value computation
   (CD%02d when total_discs > 1, empty otherwise) plus the corresponding
   bracket_map entry and \$cdnum replace before \$track (matches the
   ordering in the other path builders).

2. The album-path branch of build_final_path_for_track auto-injected a
   "Disc N" folder whenever total_discs > 1, suppressed only when the
   template contained \$disc. Templates using \$cdnum (or \${disc} /
   \${discnum} / \${cdnum}) got both a "CDxx" label in the filename and
   the auto folder. Widened the user_controls_disc check to cover all
   the disc-bearing placeholders.

Bonus cleanup along the way:

- Folder-part stripping now drops a leading \$cdnum token (mirrors the
  existing \$disc / \$discnum / \$quality strip — defensive against an
  empty cdnum landing alone in a folder segment).
- Filename cleanup now strips a leading "  - " left behind when \$cdnum
  expands to empty on a single-disc album (mirrors the same regex in
  library_reorganize.py).
- album_template config access switched from the dotted-path key to the
  nested-dict access pattern used by the rest of the function — handles
  both production config_manager and the flat _Config used in tests.

Tests: 4 new under tests/imports/test_import_paths.py
- multi-disc cdnum substitution produces "CD02"
- single-disc cdnum collapses to empty
- folder-part containing only \$cdnum is dropped
- build_final_path_for_track with \$cdnum template produces no auto
  "Disc N" folder

Full suite: 1276 passing (was 1272). Ruff clean.
2026-04-28 21:32:24 -07:00
Antti Kettunen
e10df4caf2
Rehome import helpers into core/imports
- Move import flow modules into a dedicated package
- Update app and test imports to the new namespace
- Group the import-focused tests under tests/imports
2026-04-27 19:54:44 +03:00
Renamed from core/import_paths.py (Browse further)