Commit graph

4 commits

Author SHA1 Message Date
BoulderBadgeDad
203142c4a9 Multi-disc: file the track in the disc folder that matches its tag (Sokhi)
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.
2026-06-22 10:56:53 -07:00
BoulderBadgeDad
c11a742e58 Multi-disc albums: never write a disc-less track (floor disc to >=1)
Sokhi: some tracks in a multi-disc album showed up with a null disc in Jellyfin
and floated ungrouped above the disc sections (tracks 3/9/15). Mechanism: the tag
writer only wrote the disc tag when disc_number was truthy, and enrichment CLEARS
all tags before rewriting — so a track whose disc came back 0 / None / '' lost its
disc entirely. Those falsy values slipped through because source.py defaulted with
'is not None' (a literal 0 passed) and context.py's or-chain can yield None; this
happens especially when a track resolves to a different edition than its siblings.

Fix: normalize_disc_number() floors any value to >=1, and enrichment now writes the
disc tag UNCONDITIONALLY (like the track number) so a track is never disc-less.
source.py uses the same floor so the metadata dict (and the 'Disc N' folder org)
stays consistent. Valid multi-disc values are preserved untouched.

Tests: normalize floors 0/None/''/negatives/non-numeric -> 1, preserves 1..4 and
tolerates '2.0'. 1406 enrich/metadata/track-number tests green, ruff clean.

NOTE: this fixes the SYMPTOM (never ungrouped). The deeper cause — a track matching
a DIFFERENT edition/release than its album siblings (the Persona-box-set mismatch in
the sample file; the canonical-version problem) — is separate and still open.
2026-06-22 09:30:04 -07:00
BoulderBadgeDad
d15b3a185d Track "01" bug: recover real track position instead of fabricating 1
Single tracks (esp. Deezer-sourced) imported as "01 - Title" regardless
of their real album position — e.g. Fly Away (track 2 of Greatest Hits)
landed as 01, littering album folders with duplicate "01" files.

Root cause: a Deezer single track is matched via /search/track, which
omits track_position, so the context never carried the real number; then
service.py + context.py fabricated a confident track_number=1 from that
gap. Because the resolver puts that first, the fake 1 beat the source.
It is source-agnostic (slskd-with-Deezer-metadata hits it too) — albums
work because /album/<id>/tracks DOES include positions.

Fix (at the shared import funnel, strictly additive):
- track_number.py: new read_embedded_track_number() (mutagen, local, no
  network) + an optional embedded_track_number arg on resolve_track_number.
  The downloaded file already carries the source-written position (deemix
  wrote it); consult it LAST — only when metadata AND the "NN - Title"
  filename both come up empty — so it can only fill the gap that would
  otherwise hit the default-1 floor. Never overrides a value the pre-fix
  resolver produced (no regression for correctly-named/mistagged files).
- pipeline.py: read the file tag at the resolve step and pass it in.
- De-poison: service.py:217 + context.py default to 0 (the existing
  "unknown" sentinel, like total_tracks), NOT 1 — so the fake 1 no longer
  blocks recovery. Frontend already treats falsy track_number as unknown
  (omits it), so this also drops the bogus "1." in the UI.

13 new resolver tests incl. the no-regression precedence guards; full
imports + wishlist suites green (583), no behavior change for albums.
2026-06-15 23:35:48 -07:00
Broque Thomas
997732ee63 Wishlist: fix three regressions causing all imports to land as track 01 with no year
Real-world regression triggered by the album-bundle work earlier in
2.6.3. Tracks with full Spotify metadata were importing as
``01 - <title>`` under ``Artist - Album/`` (no year), even when the
source filename carried the correct track number and Spotify's
release_date was available.

Investigation via DB inspection of stored wishlist rows:

```
"Never Gonna Give You Up" → track_number=None,  release_date=""
"idfc"                    → track_number=1,    release_date=""
"No Sleep Till Brooklyn"  → track_number=1,    release_date=""
```

Source-of-truth Spotify metadata had release_date AND real track
positions, but the wishlist row was poisoned. Three regressions
compounded the loss:

**Fix A — ``track_object_to_dict`` (``core/wishlist/payloads.py:295``)
preserved only album.name during Track→dict conversion.**

Pre-fix:
```python
album_name = "Unknown Album"
if hasattr(track_object, "album") and track_object.album:
    if hasattr(track_object.album, "name"):
        album_name = track_object.album.name
    else:
        album_name = str(track_object.album)

result = {
    ...
    "album": {"name": album_name},   # ← release_date / images / etc. all dropped
    ...
}
```

When a wishlist payload arrived as a Track dataclass instead of a
raw spotify_data dict, the Track→dict conversion stripped
release_date, images, album_type, total_tracks, id, and album-level
artists. Every wishlist row added through this path landed in the
DB with ``album={'name': X}`` only.

Post-fix: three branches handle the three album shapes
- ``album_attr`` is a dict → ``dict(album_attr)`` preserves every key
- ``album_attr`` is a sub-object → pull all common Album-dataclass
  attrs (id, release_date, album_type, total_tracks, images, ...)
- ``album_attr`` is a bare string → build a dict from the track
  object's adjacent attrs (release_date, album_id, album_type, ...)
  and surface ``image_url`` as ``album.images``

**Fix B — ``core/discovery/playlist.py:309`` only added
``track_number`` / ``disc_number`` keys when truthy.**

Pre-fix:
```python
matched_data = { 'id': ..., 'name': ..., ... }   # no track_number / disc_number
if track_number:
    matched_data['track_number'] = track_number
if disc_number:
    matched_data['disc_number'] = disc_number
```

Deezer-sourced matches always hit this branch with ``track_number=None``
because the cache enrichment at line 304 reads ``_raw.get('track_number')``
literally, but Deezer's raw shape uses ``track_position``. So the key
was omitted from ``matched_data``, downstream consumers couldn't
distinguish "missing key" from "value is 1", and the chain silently
filled 1.

Post-fix: keys are ALWAYS present (None when unknown). Also adds a
``best_match.track_number`` fallback so the Track-dataclass-mapped
value (which DOES include ``track_position``→``track_number``
mapping) gets used when the cache lookup misses.

**Fix C — Pipeline only consulted ``album_info.track_number`` before
falling to the filename (``core/imports/pipeline.py:645``).**

VA-collection source files like ``417 Fountains of Wayne - Stacys
Mom.flac`` have a leading playlist-position number that isn't the
album track number. The previous chain (album_info → filename →
floor-1) couldn't recover the real position because the filename
extractor either returned 417 (wrong) or None (caught by the floor).
But the wishlist payload's ``track_info.spotify_data.track_number``
HAD the right answer all along — Spotify says Stacy's Mom is track
3 on Welcome Interstate Managers.

Post-fix: resolution chain extracted into ``core/imports/track_number.py:resolve_track_number``
as a pure function:
1. ``album_info.track_number`` (album-bundle dispatch authoritative)
2. ``track_info.track_number`` (per-track flow payload)
3. ``track_info.spotify_data.track_number`` (nested fallback)
4. ``extract_explicit_track_number(file_path)`` (filename, returns
   0 when no numeric prefix — vs the default helper that returns 1)
5. Caller (pipeline) applies the final >=1 floor

Each step coerces to a positive int or falls through to the next.
Pure function = unit-testable in isolation = single place to fix
the rule.

**Test coverage (37 new tests):**

- ``tests/wishlist/test_payloads.py`` (+4) — Track→dict conversion
  preserves full album dict (dict / object / string album shapes) +
  None-track-number stays None.
- ``tests/discovery/test_discovery_playlist.py`` (+2) — matched_data
  always includes track_number/disc_number keys (None when unknown)
  + falls back to best_match attrs when cache misses.
- ``tests/imports/test_track_number_resolver.py`` (+16) — every
  resolution-chain branch pinned: album_info-wins, track_info
  fallback, spotify_data nested, JSON-string parsing, garbage-string
  fall-through, zero / negative / non-numeric / string-numeric
  coercion, filename fallback, explicit extractor vs default
  extractor semantics, defensive None inputs, VA-collection
  filename behaviour, all-sources-missing → None.

1571 wider-suite tests pass (wishlist + imports + discovery +
downloads + metadata). Ruff clean.

**Migration note:** existing wishlist rows that were saved under
the OLD ``track_object_to_dict`` (with stripped album metadata) still
have ``release_date=''`` in the DB blob. Those won't self-heal — the
next attempt loads from the poisoned blob. Users can remove + re-add
those tracks to refresh, or wait for the next sync run that
re-discovers them with full metadata. No automatic migration shipped
in this PR (scope creep — the forward path is fixed, backfill is a
separate concern).
2026-05-27 15:39:22 -07:00