danvics/soulsync
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions 1

Wishlist: harden Spotify backfill — poisoned tn=1 can't mask lean album

Browse source 
Residual per-track wishlist downloads (single tracks from different
albums, below the album-bundle threshold) were producing folders
without a year subfolder whenever the wishlist row carried a stale
``track_number=1`` from an older payload default.

Why: ``core/downloads/candidates.py`` had a single API-fetch branch
that served two concerns — resolving the track position AND
hydrating the lean ``spotify_album_context`` (release_date /
total_tracks / cover image) — gated entirely on track_number being
unresolved. When the wishlist row's ``track_number`` happened to
be 1 (a poisoned default rather than a real value), the gate
short-circuited and the album hydration the same call would have
done was skipped. Deezer-sourced discovery matches don't ship
release_date in their search-result album shape, so without the
backfill the folder lost its year.

The two concerns split:
  - track_number resolution keeps its track_info → track object →
    API precedence chain. track_info defaults still win.
  - album hydration runs whenever release_date or total_tracks are
    missing, independent of where (or whether) track_number was
    resolved.

The single API round-trip still serves both — the cost contract
is preserved. The side-effect coupling is gone.

Lifted into ``core/downloads/track_metadata_backfill.py``
(``hydrate_download_metadata``) so the precedence chain is pinned
in isolation. 24 unit tests cover the precedence chain, the
poisoned-tn=1 regression case, defensive non-dict/None inputs,
the cost guard (API called at most once per invocation), and
disc_number resolution.

Also lands the upstream piece: ``core/wishlist/routes.py:_build_track_data``
no longer defaults ``track_number=1`` / ``disc_number=1`` /
``total_tracks=1`` / ``release_date=''`` when the library-modal add
payload omits them. Missing values now flow through as ``None`` so
the downstream pipeline can detect-and-recover instead of locking
to a fake position.
This commit is contained in:
Broque Thomas 2026-05-27 16:47:26 -07:00
parent 47e2630583
commit 45ecf2730d
5 changed files with 743 additions and 52 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

74
core/downloads/candidates.py
View file

@ -36,6 +36,7 @@ import os
from dataclasses import dataclass
from typing import Any, Callable
from core.downloads.track_metadata_backfill import hydrate_download_metadata
from core.runtime_state import (
download_tasks,
matched_context_lock,
@ -247,55 +248,30 @@ def attempt_download_with_candidates(task_id, candidates, track, batch_id=None,
enhanced_payload['artists'] = [{'name': artist} for artist in track.artists] if track.artists else []
logger.info(f"[Context] Using clean Spotify metadata - Album: '{track.album}', Title: '{track.name}'")
# Get track_number and disc_number — prefer track data we already have,
# fall back to detailed API call only if needed
got_track_number = False
# 1. Try track_info (from frontend, has album track data)
tn = track_info.get('track_number', 0) if isinstance(track_info, dict) else 0
dn = (track_info.get('disc_number') or 1) if isinstance(track_info, dict) else 1
if tn and tn > 0:
enhanced_payload['track_number'] = tn
enhanced_payload['disc_number'] = dn
got_track_number = True
logger.info(f"[Context] Added track_number from track_info: {tn}, disc_number: {dn}")
# 2. Try the track object itself (from album tracks response)
if not got_track_number and hasattr(track, 'track_number') and track.track_number:
enhanced_payload['track_number'] = track.track_number
enhanced_payload['disc_number'] = getattr(track, 'disc_number', 1) or 1
got_track_number = True
logger.info(f"[Context] Added track_number from track object: {track.track_number}, disc_number: {enhanced_payload['disc_number']}")
# 3. Last resort — fetch from metadata source API
if not got_track_number and hasattr(track, 'id') and track.id:
try:
detailed_track = deps.spotify_client.get_track_details(track.id)
if detailed_track and detailed_track.get('track_number'):
enhanced_payload['track_number'] = detailed_track['track_number']
enhanced_payload['disc_number'] = detailed_track.get('disc_number') or 1
got_track_number = True
logger.info(f"[Context] Added track_number from API: {detailed_track['track_number']}, disc_number: {enhanced_payload['disc_number']}")
# Backfill album metadata from detailed track when context
# has incomplete data (missing release_date, total_tracks, etc.)
if isinstance(detailed_track.get('album'), dict):
dt_album = detailed_track['album']
if not spotify_album_context.get('release_date') and dt_album.get('release_date'):
spotify_album_context['release_date'] = dt_album['release_date']
logger.info(f"[Context] Backfilled release_date from API: {dt_album['release_date']}")
if not spotify_album_context.get('album_type') and dt_album.get('album_type'):
spotify_album_context['album_type'] = dt_album['album_type']
if not spotify_album_context.get('total_tracks') and dt_album.get('total_tracks'):
spotify_album_context['total_tracks'] = dt_album['total_tracks']
if not spotify_album_context.get('id') and dt_album.get('id'):
spotify_album_context['id'] = dt_album['id']
if not spotify_album_context.get('image_url') and dt_album.get('images'):
spotify_album_context['image_url'] = dt_album['images'][0].get('url', '')
except Exception as e:
logger.error(f"[Context] API track details failed: {e}")
if not got_track_number:
# Resolve track_number / disc_number and hydrate
# lean album context. Extracted to
# track_metadata_backfill.hydrate_download_metadata
# — see that module for the precedence chain.
# Why the extract: the inline pre-fix coupled
# album-backfill to the "track_number missing"
# branch. When wishlist payloads carried a poisoned
# default-1 track_number (older routes.py used
# ``.get('track_number', 1)``) the API call short-
# circuited and the lean album_context (no
# release_date / total_tracks for Deezer-sourced
# discovery matches) survived untouched, producing
# folders without a year subfolder.
resolved = hydrate_download_metadata(
track, track_info, spotify_album_context, deps.spotify_client,
)
if resolved.track_number is not None:
enhanced_payload['track_number'] = resolved.track_number
enhanced_payload['disc_number'] = resolved.disc_number
logger.info(
f"[Context] Added track_number from {resolved.source}: "
f"{resolved.track_number}, disc_number: {resolved.disc_number}"
)
else:
enhanced_payload.setdefault('track_number', 0)
enhanced_payload.setdefault('disc_number', 1)
logger.warning("[Context] No track_number found from any source")

175
core/downloads/track_metadata_backfill.py Normal file
View file

@ -0,0 +1,175 @@
"""Track-position resolution + album-context hydration helper.
Lifted out of ``core/downloads/candidates.py`` to break a quiet
regression. The pre-extract code fetched detailed track data from
Spotify only when ``track_number`` was missing — the same API call
*also* backfilled the lean ``spotify_album_context`` (release_date,
total_tracks, image_url) but only as a side-effect of the
track_number branch. When a wishlist row carried a poisoned
``track_number=1`` (older payload helpers defaulted missing values
to 1), the conditional short-circuited, the API call never fired,
and the album context stayed lean — producing folders without a
year subfolder for residual per-track wishlist downloads.
The fix splits the two concerns: ``track_number`` resolution
follows its precedence chain (track_info → track object → API),
but album hydration runs whenever ``spotify_album_context`` is
missing any of release_date / total_tracks regardless of whether
``track_number`` was already known. A single API call still serves
both — the side-effect coupling is gone but the network cost
isn't paid twice.
"""
from __future__ import annotations
import logging
from typing import Any, Dict, NamedTuple, Optional
logger = logging.getLogger(__name__)
class ResolvedTrackMetadata(NamedTuple):
"""Result of ``hydrate_download_metadata``.
``track_number`` is ``None`` when every source — track_info,
track object, API — failed to produce a positive integer. The
caller (candidates.py) treats ``None`` as "fall back to the
setdefault(0)" path so the existing 0-floor sentinel behaviour
is preserved.
"""
track_number: Optional[int]
disc_number: int
source: str # 'track_info' | 'track_object' | 'api' | 'none'
def _positive_int(value: Any) -> Optional[int]:
if isinstance(value, bool):
return None
if isinstance(value, int):
return value if value > 0 else None
if isinstance(value, str) and value.isdigit():
coerced = int(value)
return coerced if coerced > 0 else None
return None
def _album_is_lean(album_context: Any) -> bool:
"""An album context is "lean" when it lacks release_date OR
total_tracks. Both fields are required downstream:
``release_date`` drives the year folder, ``total_tracks`` drives
the TRCK tag denominator + the album-vs-single classification
in ``build_import_album_info``."""
if not isinstance(album_context, dict):
return True
if not album_context.get('release_date'):
return True
if not album_context.get('total_tracks'):
return True
return False
def _backfill_album_context(
album_context: Dict[str, Any], detailed_track: Dict[str, Any]
) -> None:
"""Copy missing fields from ``detailed_track['album']`` into the
in-place album_context dict. Existing values are preserved —
never overwritten — because the caller's context may carry
fields the API doesn't (e.g. enhanced search shapes can include
artists arrays the bare track endpoint omits)."""
dt_album = detailed_track.get('album')
if not isinstance(dt_album, dict) or not isinstance(album_context, dict):
return
for key in ('release_date', 'album_type', 'total_tracks', 'id'):
if not album_context.get(key) and dt_album.get(key):
album_context[key] = dt_album[key]
if not album_context.get('image_url'):
images = dt_album.get('images')
if isinstance(images, list) and images:
first = images[0]
if isinstance(first, dict) and first.get('url'):
album_context['image_url'] = first['url']
def hydrate_download_metadata(
track: Any,
track_info: Any,
spotify_album_context: Dict[str, Any],
spotify_client: Any,
) -> ResolvedTrackMetadata:
"""Resolve track position and hydrate lean album context.
Steps:
1. ``track_info['track_number']`` when positive
2. ``track.track_number`` when truthy
3. ``spotify_client.get_track_details(track.id)`` — fires when
EITHER track_number unresolved OR album_context lean. The
same call serves both concerns; only one round-trip per task.
``spotify_album_context`` is mutated in place when API returns
richer data. Returns the resolved track_number / disc_number /
source. ``track_number=None`` signals "no usable value found";
the caller decides whether to floor it to 1 or leave 0.
"""
ti = track_info if isinstance(track_info, dict) else {}
# Step 1: track_info top-level — wishlist + frontend payloads.
tn = _positive_int(ti.get('track_number'))
if tn is not None:
dn = _positive_int(ti.get('disc_number')) or 1
source = 'track_info'
else:
tn = None
dn = 1
source = 'none'
# Step 2: track object — Spotify Track dataclass from search.
if tn is None:
track_tn = getattr(track, 'track_number', None)
coerced = _positive_int(track_tn)
if coerced is not None:
tn = coerced
dn = _positive_int(getattr(track, 'disc_number', None)) or 1
source = 'track_object'
needs_api_for_tn = tn is None
needs_api_for_album = _album_is_lean(spotify_album_context)
track_id = getattr(track, 'id', None)
if (needs_api_for_tn or needs_api_for_album) and track_id:
try:
detailed = spotify_client.get_track_details(track_id)
except Exception as e: # noqa: BLE001 — defensive log + continue
logger.error("[Context] API track details failed: %s", e)
detailed = None
if isinstance(detailed, dict):
if needs_api_for_tn:
api_tn = _positive_int(detailed.get('track_number'))
if api_tn is not None:
tn = api_tn
dn = _positive_int(detailed.get('disc_number')) or 1
source = 'api'
logger.info(
"[Context] Resolved track_number=%d disc_number=%d from API",
tn, dn,
)
if needs_api_for_album and isinstance(spotify_album_context, dict):
_backfill_album_context(spotify_album_context, detailed)
logger.info(
"[Context] Backfilled album context from API "
"(release_date=%r, total_tracks=%r)",
spotify_album_context.get('release_date'),
spotify_album_context.get('total_tracks'),
)
return ResolvedTrackMetadata(track_number=tn, disc_number=dn, source=source)
__all__ = [
'ResolvedTrackMetadata',
'hydrate_download_metadata',
]

34
core/wishlist/routes.py
View file

@ -47,6 +47,26 @@ def _build_album_images(album: Dict[str, Any]) -> list[dict[str, Any]]:
def _build_track_data(track: Dict[str, Any], album: Dict[str, Any]) -> Dict[str, Any]:
"""Project a wishlist-modal add payload into the canonical
wishlist track shape.
Pre-fix this used default values (``track_number=1``,
``disc_number=1``, ``total_tracks=1``, ``release_date=''``) when
the upstream UI omitted a field. That silently poisoned every
wishlist row added from the library "add to wishlist" modal:
track_number locked to 1 regardless of source position, year
dropped from folder paths because release_date was empty. The
library modal flow is what's used for "add this album's missing
tracks to wishlist" and "add this playlist to wishlist" bulk
actions — the most common user path, so the regression was
everywhere.
Now preserve missing values explicitly (None for numeric
positions, omit-or-empty for release_date) so the downstream
import pipeline can detect-and-recover via
``core/imports/track_number.py:resolve_track_number`` instead
of locking to 1.
"""
album_images = _build_album_images(album)
return {
"id": track.get("id"),
@ -58,12 +78,20 @@ def _build_track_data(track: Dict[str, Any], album: Dict[str, Any]) -> Dict[str,
"artists": album.get("artists", []),
"images": album_images,
"album_type": album.get("album_type", "album"),
# release_date stays as whatever the upstream sent
# (including '' when truly unknown). Path template
# gracefully omits the year when empty; we don't fake
# a date.
"release_date": album.get("release_date", ""),
"total_tracks": album.get("total_tracks", 1),
# total_tracks=None preserves "we don't know"; UI uses
# this for category classification + path math. Pre-fix
# default of 1 mislabelled multi-track albums as singles.
"total_tracks": album.get("total_tracks"),
},
"duration_ms": track.get("duration_ms", 0),
"track_number": track.get("track_number", 1),
"disc_number": track.get("disc_number", 1),
# Numeric positions: None when missing, not 1.
"track_number": track.get("track_number"),
"disc_number": track.get("disc_number"),
"explicit": track.get("explicit", False),
"popularity": track.get("popularity", 0),
"preview_url": track.get("preview_url"),

511
tests/downloads/test_track_metadata_backfill.py Normal file
View file

@ -0,0 +1,511 @@
"""Tests for ``core/downloads/track_metadata_backfill.py``.
Real-world regression these tests pin: wishlist rows carrying a
poisoned ``track_number=1`` (older payload helpers defaulted
missing values to 1) used to prevent the Spotify-API backfill
that hydrated lean ``spotify_album_context`` (release_date,
total_tracks). Result: residual per-track wishlist downloads
produced folders without a year subfolder when the wishlist row
came from a Deezer-sourced discovery match.
The split-concern fix:
- ``track_number`` precedence: track_info → track object → API
- album hydration: runs whenever release_date / total_tracks
missing, INDEPENDENT of whether track_number was already known
- single API call serves both — no double round-trip
"""
from __future__ import annotations
from dataclasses import dataclass
from typing import Any, Optional
from unittest.mock import MagicMock
import pytest
from core.downloads.track_metadata_backfill import (
ResolvedTrackMetadata,
hydrate_download_metadata,
)
@dataclass
class _FakeTrack:
"""Stand-in for the Spotify Track dataclass — only the attrs the
backfill helper reads."""
id: Optional[str] = 'spotify_track_id'
track_number: Optional[int] = None
disc_number: Optional[int] = None
def _api_payload(
track_number: Optional[int] = 5,
disc_number: int = 1,
release_date: str = '2013-10-22',
total_tracks: int = 16,
album_id: str = 'spotify_album_id',
image_url: str = 'https://i.scdn.co/cover.jpg',
album_type: str = 'album',
) -> dict:
"""Build a plausible ``spotify_client.get_track_details`` response."""
images: list[dict[str, Any]] = []
if image_url:
images.append({'url': image_url, 'height': 640, 'width': 640})
return {
'id': 'spotify_track_id',
'name': 'Roar',
'track_number': track_number,
'disc_number': disc_number,
'album': {
'id': album_id,
'name': 'PRISM (Deluxe)',
'release_date': release_date,
'total_tracks': total_tracks,
'album_type': album_type,
'images': images,
},
}
# ---------------------------------------------------------------------------
# Track-number precedence chain.
# ---------------------------------------------------------------------------
def test_track_info_track_number_wins_over_track_object():
"""track_info has a real value → use it, skip lower-priority sources."""
client = MagicMock()
album_ctx = {'release_date': '2013-10-22', 'total_tracks': 16}
resolved = hydrate_download_metadata(
_FakeTrack(track_number=9), # would resolve to 9 if track_info empty
{'track_number': 5, 'disc_number': 2},
album_ctx,
client,
)
assert resolved == ResolvedTrackMetadata(track_number=5, disc_number=2, source='track_info')
client.get_track_details.assert_not_called()
def test_track_object_used_when_track_info_missing():
"""track_info has no track_number → fall to track.track_number."""
client = MagicMock()
album_ctx = {'release_date': '2013-10-22', 'total_tracks': 16}
resolved = hydrate_download_metadata(
_FakeTrack(track_number=7, disc_number=2),
{}, # no track_number key
album_ctx,
client,
)
assert resolved == ResolvedTrackMetadata(track_number=7, disc_number=2, source='track_object')
client.get_track_details.assert_not_called()
def test_api_used_when_track_info_and_track_object_missing():
"""No local source has track_number → fire the API."""
client = MagicMock()
client.get_track_details.return_value = _api_payload(track_number=8, disc_number=1)
album_ctx = {'release_date': '2013-10-22', 'total_tracks': 16}
resolved = hydrate_download_metadata(
_FakeTrack(track_number=None),
{},
album_ctx,
client,
)
assert resolved == ResolvedTrackMetadata(track_number=8, disc_number=1, source='api')
client.get_track_details.assert_called_once_with('spotify_track_id')
def test_zero_track_number_in_track_info_treated_as_missing():
"""track_info.track_number=0 is a sentinel for "missing", not a
valid position — fall through to the next source."""
client = MagicMock()
client.get_track_details.return_value = _api_payload(track_number=3)
album_ctx = {'release_date': '2013-10-22', 'total_tracks': 16}
resolved = hydrate_download_metadata(
_FakeTrack(track_number=None),
{'track_number': 0},
album_ctx,
client,
)
assert resolved.track_number == 3
assert resolved.source == 'api'
def test_string_track_number_coerced_to_int():
"""Some legacy payloads pass track_number as a string. Coerce, not reject."""
client = MagicMock()
album_ctx = {'release_date': '2013-10-22', 'total_tracks': 16}
resolved = hydrate_download_metadata(
_FakeTrack(),
{'track_number': '11'},
album_ctx,
client,
)
assert resolved.track_number == 11
assert resolved.source == 'track_info'
def test_boolean_track_number_rejected_not_treated_as_int():
"""Python ints include bools — ``True == 1`` would erroneously
pass the >0 gate. Reject explicitly so a malformed payload
falls through instead of being mistaken for track 1."""
client = MagicMock()
client.get_track_details.return_value = _api_payload(track_number=4)
album_ctx = {'release_date': '2013-10-22', 'total_tracks': 16}
resolved = hydrate_download_metadata(
_FakeTrack(),
{'track_number': True},
album_ctx,
client,
)
assert resolved.track_number == 4
assert resolved.source == 'api'
# ---------------------------------------------------------------------------
# THE REGRESSION FIX: album backfill runs even when track_number known.
# ---------------------------------------------------------------------------
def test_poisoned_default_track_number_does_NOT_block_album_backfill():
"""Regression pin. track_info carries the poisoned ``track_number=1``
legacy default; album_context is lean (Deezer-sourced discovery
match, no release_date, no total_tracks). Pre-fix: API skipped
because tn>0, folder lost year. Post-fix: API still fires for
album hydration; track_number stays at 1 (caller's precedence),
but release_date / total_tracks now populate."""
client = MagicMock()
client.get_track_details.return_value = _api_payload(
track_number=4, # the API knows the *real* track number
release_date='2007-03-12',
total_tracks=12,
)
album_ctx = {'name': 'Welcome Interstate Managers'} # lean
resolved = hydrate_download_metadata(
_FakeTrack(id='spotify_track_id', track_number=None),
{'track_number': 1}, # POISONED default
album_ctx,
client,
)
# track_number stays at 1 — track_info precedence is sacred,
# we don't second-guess it. The album fix is what matters.
assert resolved.track_number == 1
assert resolved.source == 'track_info'
# CRITICAL: album_context got hydrated despite track_number being "resolved".
assert album_ctx['release_date'] == '2007-03-12'
assert album_ctx['total_tracks'] == 12
# API call still fired — for the album, even though tn already known.
client.get_track_details.assert_called_once_with('spotify_track_id')
def test_rich_album_context_skips_api_when_track_number_resolved():
"""Cost guard. Both concerns satisfied locally → no API call.
Keeps the network-cost contract from the pre-extract code."""
client = MagicMock()
album_ctx = {
'name': 'PRISM (Deluxe)',
'release_date': '2013-10-22',
'total_tracks': 16,
}
resolved = hydrate_download_metadata(
_FakeTrack(),
{'track_number': 5},
album_ctx,
client,
)
assert resolved.track_number == 5
client.get_track_details.assert_not_called()
def test_api_fires_for_lean_album_even_with_track_object_track_number():
"""track_number from the Track dataclass (search-result level) but
album_context still lean — API must fire for album hydration."""
client = MagicMock()
client.get_track_details.return_value = _api_payload()
album_ctx = {'name': 'PRISM (Deluxe)'} # lean
resolved = hydrate_download_metadata(
_FakeTrack(track_number=9, disc_number=1),
{},
album_ctx,
client,
)
assert resolved.track_number == 9
assert resolved.source == 'track_object'
assert album_ctx['release_date'] == '2013-10-22'
assert album_ctx['total_tracks'] == 16
client.get_track_details.assert_called_once()
def test_album_with_only_release_date_still_triggers_backfill_for_total_tracks():
"""release_date populated but total_tracks missing → still lean →
API fires to fill total_tracks. Either missing field triggers."""
client = MagicMock()
client.get_track_details.return_value = _api_payload()
album_ctx = {'release_date': '2013-10-22'} # total_tracks missing
hydrate_download_metadata(_FakeTrack(), {'track_number': 5}, album_ctx, client)
assert album_ctx['total_tracks'] == 16
client.get_track_details.assert_called_once()
def test_album_with_only_total_tracks_still_triggers_backfill_for_release_date():
"""Inverse: total_tracks present but release_date missing → still lean."""
client = MagicMock()
client.get_track_details.return_value = _api_payload()
album_ctx = {'total_tracks': 16} # release_date missing
hydrate_download_metadata(_FakeTrack(), {'track_number': 5}, album_ctx, client)
assert album_ctx['release_date'] == '2013-10-22'
client.get_track_details.assert_called_once()
# ---------------------------------------------------------------------------
# Album backfill: preserve existing values, never overwrite.
# ---------------------------------------------------------------------------
def test_album_backfill_does_not_overwrite_existing_release_date():
"""Caller's release_date is sacred — only fill when absent. Source
of truth may legitimately diverge between context and API (e.g.
region-specific releases). Don't second-guess the caller."""
client = MagicMock()
client.get_track_details.return_value = _api_payload(release_date='2099-01-01')
album_ctx = {'release_date': '2013-10-22', 'total_tracks': 16, 'name': 'Real'}
hydrate_download_metadata(_FakeTrack(), {'track_number': 1}, album_ctx, client)
# Not lean → API never called.
client.get_track_details.assert_not_called()
assert album_ctx['release_date'] == '2013-10-22'
def test_album_backfill_fills_missing_image_url_from_images_array():
"""API responses use the Spotify ``images`` array shape. Helper
flattens to the ``image_url`` string the path builder reads."""
client = MagicMock()
client.get_track_details.return_value = _api_payload(
image_url='https://i.scdn.co/cover.jpg',
)
album_ctx = {'name': 'PRISM'} # lean
hydrate_download_metadata(_FakeTrack(), {}, album_ctx, client)
assert album_ctx['image_url'] == 'https://i.scdn.co/cover.jpg'
def test_album_backfill_handles_empty_images_array_gracefully():
"""API response with empty ``images`` array (rare but possible
on tracks the album-cover service hasn't indexed yet) must not
crash the resolver."""
client = MagicMock()
payload = _api_payload(image_url='') # produces images=[]
client.get_track_details.return_value = payload
album_ctx = {'name': 'PRISM'}
hydrate_download_metadata(_FakeTrack(), {}, album_ctx, client)
# No image_url set, but other fields hydrated.
assert 'image_url' not in album_ctx
assert album_ctx['release_date'] == '2013-10-22'
def test_album_backfill_skips_when_detailed_album_not_dict():
"""API response with a non-dict ``album`` field (defensive — old
enrichment caches stored string album names) is safely skipped."""
client = MagicMock()
client.get_track_details.return_value = {
'id': 'spotify_track_id',
'track_number': 3,
'album': 'Just a string',
}
album_ctx = {'name': 'PRISM'}
resolved = hydrate_download_metadata(_FakeTrack(), {}, album_ctx, client)
# track_number resolved, album_context untouched.
assert resolved.track_number == 3
assert 'release_date' not in album_ctx
# ---------------------------------------------------------------------------
# Defensive: missing IDs, exceptions, non-dict inputs.
# ---------------------------------------------------------------------------
def test_no_track_id_skips_api_call():
"""Without ``track.id`` the API can't be queried. Returns whatever
was resolved locally, no exception."""
client = MagicMock()
album_ctx = {'name': 'PRISM'} # lean — but no ID to fix it
resolved = hydrate_download_metadata(
_FakeTrack(id=None),
{},
album_ctx,
client,
)
assert resolved.track_number is None
assert resolved.source == 'none'
client.get_track_details.assert_not_called()
def test_api_exception_does_not_propagate():
"""Network blip / Spotify 5xx must not crash the download pipeline.
Returns whatever was resolved before the failed call."""
client = MagicMock()
client.get_track_details.side_effect = RuntimeError('429 rate limited')
album_ctx = {'name': 'PRISM'} # lean
resolved = hydrate_download_metadata(
_FakeTrack(),
{'track_number': 5}, # already have tn from track_info
album_ctx,
client,
)
assert resolved.track_number == 5
assert 'release_date' not in album_ctx # backfill failed silently
def test_api_returns_none_does_not_mutate_context():
"""API call that returns None (no result for this Spotify ID) is
a no-op for album hydration."""
client = MagicMock()
client.get_track_details.return_value = None
album_ctx = {'name': 'PRISM'}
resolved = hydrate_download_metadata(_FakeTrack(), {}, album_ctx, client)
assert resolved.track_number is None
assert 'release_date' not in album_ctx
def test_non_dict_track_info_treated_as_empty():
"""Defensive — a malformed task payload with a non-dict track_info
falls through to the track object / API instead of crashing."""
client = MagicMock()
client.get_track_details.return_value = _api_payload(track_number=6)
album_ctx = {'release_date': '2013-10-22', 'total_tracks': 16}
resolved = hydrate_download_metadata(
_FakeTrack(),
'not a dict', # type: ignore[arg-type]
album_ctx,
client,
)
assert resolved.source == 'api'
assert resolved.track_number == 6
def test_non_dict_album_context_treated_as_lean():
"""Defensive — a None album_context is treated as lean, so the API
still fires but mutation is silently skipped (helper can't mutate
a non-dict in place)."""
client = MagicMock()
client.get_track_details.return_value = _api_payload()
# Should not raise — passing None for album_context just means
# the backfill no-ops (can't mutate None) but the call signature
# is preserved.
resolved = hydrate_download_metadata(
_FakeTrack(),
{'track_number': 5},
None, # type: ignore[arg-type]
client,
)
# tn resolved from track_info, no crash.
assert resolved.track_number == 5
# ---------------------------------------------------------------------------
# disc_number resolution.
# ---------------------------------------------------------------------------
def test_disc_number_from_track_info_paired_with_track_number():
"""When track_info supplies track_number, its disc_number rides along."""
client = MagicMock()
album_ctx = {'release_date': '2013-10-22', 'total_tracks': 16}
resolved = hydrate_download_metadata(
_FakeTrack(),
{'track_number': 5, 'disc_number': 2},
album_ctx,
client,
)
assert resolved.disc_number == 2
def test_disc_number_defaults_to_1_when_only_track_number_present():
"""track_info has track_number but no disc_number → disc=1."""
client = MagicMock()
album_ctx = {'release_date': '2013-10-22', 'total_tracks': 16}
resolved = hydrate_download_metadata(
_FakeTrack(),
{'track_number': 5},
album_ctx,
client,
)
assert resolved.disc_number == 1
def test_disc_number_zero_from_api_floored_to_1():
"""API response with disc_number=0 (some niche releases) gets
floored to 1 — albums are 1-indexed."""
client = MagicMock()
client.get_track_details.return_value = _api_payload(track_number=5, disc_number=0)
album_ctx = {'release_date': '2013-10-22', 'total_tracks': 16}
resolved = hydrate_download_metadata(_FakeTrack(), {}, album_ctx, client)
assert resolved.disc_number == 1
# ---------------------------------------------------------------------------
# Cost guard: API called at most once.
# ---------------------------------------------------------------------------
def test_api_called_at_most_once_per_invocation():
"""Even when API serves both concerns (track_number AND album),
only one ``get_track_details`` call is made."""
client = MagicMock()
client.get_track_details.return_value = _api_payload()
album_ctx = {'name': 'PRISM'} # lean
hydrate_download_metadata(_FakeTrack(track_number=None), {}, album_ctx, client)
assert client.get_track_details.call_count == 1
if __name__ == '__main__':
pytest.main([__file__, '-v'])

1
webui/static/helper.js
View file

@ -3415,6 +3415,7 @@ function closeHelperSearch() {
const WHATS_NEW = {
'2.6.3': [
{ unreleased: true },
{ title: 'Wishlist: harden Spotify backfill so a poisoned track_number can\'t mask a lean album', desc: 'follow-up to the earlier wishlist import-path work. residual per-track wishlist downloads (single tracks from different albums, falling below the album-bundle threshold) were producing folders without a year subfolder whenever the underlying wishlist row had a track_number=1 from an older default — the candidate dispatcher\'s "fall back to Spotify API" branch was gated entirely on track_number being missing, but the same API call was also the only thing that hydrated the lean album_context (release_date / total_tracks / cover art) when the original discovery match came from Deezer\'s search endpoint which doesn\'t carry those fields. so any row whose track_number happened to look "filled" (1 from a default) would short-circuit the API call and the year disappeared from the folder path even though the API knew it. split the two concerns: track_number resolution keeps its track_info → track object → API precedence, but album hydration now runs whenever release_date or total_tracks are missing regardless of where track_number came from. one network round-trip still serves both. lifted into core/downloads/track_metadata_backfill.py with 24 unit tests pinning every branch — including the regression: poisoned default-1 track_number does NOT block album backfill anymore.' },
{ title: 'Wishlist: fix imports landing as track 01 with no year in folder name', desc: 'follow-up to the earlier wishlist album-bundle work. tracks with rich Spotify metadata were still importing as `01 - <title>` with no year in the folder path because three regressions stacked: the Track→dict conversion in payload helpers dropped everything except `album.name` (silently throwing away release_date / images / album_type / total_tracks), Deezer-sourced discovery matches saved their payloads without `track_number` / `disc_number` keys at all (Deezer uses `track_position` while the cache lookup read `track_number` literally), and the import pipeline only consulted `album_info.track_number` before falling to the filename — which fails for VA-collection source files like `417 Fountains of Wayne - Stacys Mom.flac` where the leading number is a playlist position not the album track. all three patched, with the track_number resolution chain lifted into `core/imports/track_number.py` as a pure function with 18 unit tests pinning every branch.' },
{ title: 'Wishlist: distinguish Queued from Analyzing batches in the UI', desc: 'wishlist runs with more than 3 sub-batches used to render every batch as "Analyzing..." simultaneously — even though the download worker pool only runs 3 at a time. so a 26-album wishlist scan looked like all 26 were working in parallel when really 23 were just sitting in the executor queue waiting their turn. now batches show a distinct "Queued ⏳" state while parked in the executor queue; they flip to "Analyzing..." when a worker actually picks them up. zero behavior change — just less misleading UI.' },
{ title: 'Wishlist: only engage album-bundle when several tracks from the same album are missing', desc: 'auto-wishlist + manual-wishlist runs were promoting every single-track wishlist item to a per-album bundle search — so a wishlist of "26 single tracks from 26 different albums" downloaded full albums (5-42 files each, ~85% wasted bandwidth) and hammered slskd with concurrent searches. now the bundle path only engages when an album has 2 or more missing tracks in the wishlist; single-track items take the cheaper per-track path that already works fine. fewer slskd searches per cycle, less wasted bandwidth, no more downloading the same album three times in a row when staging-match misses a single track. configurable via `wishlist.album_bundle_min_tracks` if you want the old behaviour (set to 1) or stricter (set to 3+).' },