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

Merge pull request #569 from Nezreka/dev

Browse source 
Dev
This commit is contained in:
BoulderBadgeDad 2026-05-12 20:10:48 -07:00 committed by GitHub
commit 1296bb9c1d
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
47 changed files with 5975 additions and 468 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
.github/workflows/docker-publish.yml vendored
View file

@ -9,9 +9,9 @@ on:
workflow_dispatch:
inputs:
version_tag:
description: 'Version tag (e.g. 2.4.3)'
description: 'Version tag (e.g. 2.5.1)'
required: true
default: '2.4.3'
default: '2.5.1'
jobs:
build-and-push:

9
Dockerfile
View file

@ -57,8 +57,13 @@ COPY --chown=soulsync:soulsync . .
# Create runtime mount-point directories the app expects to exist.
# NOTE: /app/data is for database FILES, /app/database is the Python package
RUN mkdir -p /app/config /app/data /app/logs /app/downloads /app/Transfer /app/MusicVideos /app/scripts && \
chown soulsync:soulsync /app/config /app/data /app/logs /app/downloads /app/Transfer /app/MusicVideos /app/scripts
# NOTE: /app/Staging is required even though most users bind-mount it —
# the entrypoint mkdir runs early and is gated by `set -e`, so a missing
# pre-baked directory would crash the container into a restart loop on
# rootless Docker/Podman where in-container "root" can't write to /app.
# Pre-baking the dir here makes the entrypoint mkdir a guaranteed no-op.
RUN mkdir -p /app/config /app/data /app/logs /app/downloads /app/Transfer /app/Staging /app/MusicVideos /app/scripts && \
chown soulsync:soulsync /app/config /app/data /app/logs /app/downloads /app/Transfer /app/Staging /app/MusicVideos /app/scripts
# Create defaults directory and copy template files
# These will be used by entrypoint.sh to initialize empty volumes

9
config/settings.py
View file

@ -466,7 +466,14 @@ class ConfigManager:
"download_path": "./downloads",
"transfer_path": "./Transfer",
"max_peer_queue": 0,
"download_timeout": 600
"download_timeout": 600,
# Reddit report (YeloMelo95, Bell Canada): the existing
# 35-per-220s sliding-window cap allows all 35 searches in
# rapid succession before throttling — that burst trips ISP
# anti-abuse. This knob forces a min gap between consecutive
# searches even when the window cap isn't hit. 0 = disabled
# (preserves prior behavior).
"search_min_delay_seconds": 0,
},
"download_source": {
"mode": "soulseek", # Options: "soulseek", "youtube", "tidal", "qobuz", "hifi", "hybrid"

41
core/audiodb_worker.py
View file

@ -200,42 +200,45 @@ class AudioDBWorker:
if row:
return {'type': 'track', 'id': row[0], 'name': row[1], 'artist': row[2], 'artist_audiodb_id': row[3]}
# Priority 4: Retry 'not_found' artists after retry_days
not_found_cutoff = datetime.now() - timedelta(days=self.retry_days)
# Priority 4: Retry 'not_found' OR 'error' artists after retry_days.
# 'error' status covers transient AudioDB outages (timeouts, 500s)
# that the issue-#553 fix marks rather than leaving NULL — without
# this retry path those rows would stay errored forever.
retry_cutoff = datetime.now() - timedelta(days=self.retry_days)
cursor.execute("""
SELECT id, name
FROM artists
WHERE audiodb_match_status = 'not_found' AND audiodb_last_attempted < ?
WHERE audiodb_match_status IN ('not_found', 'error') AND audiodb_last_attempted < ?
ORDER BY audiodb_last_attempted ASC
LIMIT 1
""", (not_found_cutoff,))
""", (retry_cutoff,))
row = cursor.fetchone()
if row:
logger.info(f"Retrying artist '{row[1]}' (last attempted before cutoff)")
return {'type': 'artist', 'id': row[0], 'name': row[1]}
# Priority 5: Retry 'not_found' albums
# Priority 5: Retry 'not_found' OR 'error' albums
cursor.execute("""
SELECT a.id, a.title, ar.name AS artist_name, ar.audiodb_id AS artist_audiodb_id
FROM albums a
JOIN artists ar ON a.artist_id = ar.id
WHERE a.audiodb_match_status = 'not_found' AND a.audiodb_last_attempted < ?
WHERE a.audiodb_match_status IN ('not_found', 'error') AND a.audiodb_last_attempted < ?
ORDER BY a.audiodb_last_attempted ASC
LIMIT 1
""", (not_found_cutoff,))
""", (retry_cutoff,))
row = cursor.fetchone()
if row:
return {'type': 'album', 'id': row[0], 'name': row[1], 'artist': row[2], 'artist_audiodb_id': row[3]}
# Priority 6: Retry 'not_found' tracks
# Priority 6: Retry 'not_found' OR 'error' tracks
cursor.execute("""
SELECT t.id, t.title, ar.name AS artist_name, ar.audiodb_id AS artist_audiodb_id
FROM tracks t
JOIN artists ar ON t.artist_id = ar.id
WHERE t.audiodb_match_status = 'not_found' AND t.audiodb_last_attempted < ?
WHERE t.audiodb_match_status IN ('not_found', 'error') AND t.audiodb_last_attempted < ?
ORDER BY t.audiodb_last_attempted ASC
LIMIT 1
""", (not_found_cutoff,))
""", (retry_cutoff,))
row = cursor.fetchone()
if row:
return {'type': 'track', 'id': row[0], 'name': row[1], 'artist': row[2], 'artist_audiodb_id': row[3]}
@ -374,8 +377,22 @@ class AudioDBWorker:
return
except Exception as e:
logger.warning(f"Direct lookup failed for existing AudioDB ID {existing_id}: {e}")
# Direct lookup failed — don't overwrite manual match
logger.debug(f"Preserving manual match for {item_type} '{item_name}' (AudioDB ID: {existing_id})")
# Direct lookup returned no metadata (None) or raised — don't
# fall through to the name-search path below, which could
# overwrite a manually-matched audiodb_id with a wrong guess.
# Mark status='error' so the queue's NULL-status filter stops
# re-picking this row on every tick (issue #553: AudioDB
# `track.php` timeouts caused infinite enrichment loops as
# the row was repeatedly picked + re-attempted because it
# never left the NULL state). The error-retry priority block
# in `_get_next_item` re-attempts after `retry_days` so
# transient AudioDB outages still recover automatically.
self._mark_status(item_type, item_id, 'error')
self.stats['errors'] += 1
logger.debug(
f"Preserving manual match for {item_type} '{item_name}' "
f"(AudioDB ID: {existing_id}); marked error pending retry"
)
return
if item_type == 'artist':

218
core/auto_import_worker.py
View file

@ -199,6 +199,53 @@ def _quality_rank(ext: str) -> int:
return ranks.get(ext.lower(), 1)
# Weight constants for `_score_album_search_result` — exposed at module
# level so they're greppable + bumpable in one place. Pre-fix these were
# magic numbers inline.
_ALBUM_NAME_WEIGHT = 0.5 # title fuzzy similarity
_ARTIST_NAME_WEIGHT = 0.2 # primary artist fuzzy similarity (skipped when target is empty)
_TRACK_COUNT_WEIGHT = 0.3 # how close the source's track count is to the file count
def _score_album_search_result(album_result, target_album: str,
target_artist: Optional[str],
file_count: int) -> float:
"""Pure scoring helper for `_search_metadata_source`.
Weights how well an `album_result` from a metadata source's
`search_albums` matches the search inputs. Returns float in [0.0, 1.0].
Pre-extraction this lived inline in the loop body; lifting it out
lets the weight math be pinned independently of the orchestrator
(per-source iteration, exception containment, threshold check).
`album_result` is expected to expose:
- `.name` (str)
- `.artists` (list of dict-like with 'name', optional 'id') or list[str]
- `.total_tracks` (int, optional)
"""
score = 0.0
# Album name similarity (default 50%)
name = getattr(album_result, 'name', '') or ''
score += _similarity(target_album, name) * _ALBUM_NAME_WEIGHT
# Artist similarity (default 20%) — only when target_artist provided
if target_artist:
artists = getattr(album_result, 'artists', None) or []
r_artist = artists[0] if artists else ''
if isinstance(r_artist, dict):
r_artist = r_artist.get('name', '')
score += _similarity(target_artist, str(r_artist)) * _ARTIST_NAME_WEIGHT
# Track count match (default 30%) — only when both sides have a count
r_tracks = getattr(album_result, 'total_tracks', 0) or 0
if r_tracks > 0 and file_count > 0:
count_ratio = 1.0 - abs(r_tracks - file_count) / max(r_tracks, file_count)
score += max(0.0, count_ratio) * _TRACK_COUNT_WEIGHT
return score
class AutoImportWorker:
"""Background worker that watches the staging folder and auto-imports music.
@ -1260,87 +1307,120 @@ class AutoImportWorker:
def _search_metadata_source(self, artist: Optional[str], album: str,
method: str, candidate: FolderCandidate,
query: str = None) -> Optional[Dict]:
"""Search the active metadata source for an album match."""
"""Search configured metadata sources for an album match.
Iterates `get_source_priority(get_primary_source())` so primary
is tried first and the rest are tried as fallback. Returns the
FIRST source whose best result clears the 0.4 score threshold.
Pre-fix this only queried the primary, which meant indie/niche
albums missing from the user's primary (e.g. Bandcamp releases
not on Spotify) failed auto-import even when manual search
could find them on Tidal/Deezer. The manual search bar at the
bottom of the Import tab already iterates the full source
chain via `search_import_albums` — this aligns auto-import
with that behavior.
"""
try:
from core.metadata_service import get_primary_source, get_client_for_source
source = get_primary_source()
client = get_client_for_source(source)
if not client or not hasattr(client, 'search_albums'):
return None
from core.metadata_service import (
get_primary_source,
get_source_priority,
get_client_for_source,
)
primary_source = get_primary_source()
source_chain = get_source_priority(primary_source)
search_query = query or (f"{artist} {album}" if artist else album)
results = client.search_albums(search_query, limit=5)
if not results:
return None
# Score each result
best_result = None
best_score = 0
for source in source_chain:
client = get_client_for_source(source)
if not client or not hasattr(client, 'search_albums'):
continue
for r in results:
score = 0
# Album name similarity (50%)
score += _similarity(album, r.name) * 0.5
# Artist similarity (20%)
if artist:
r_artist = r.artists[0] if hasattr(r, 'artists') and r.artists else ''
if isinstance(r_artist, dict):
r_artist = r_artist.get('name', '')
score += _similarity(artist, str(r_artist)) * 0.2
# Track count match (30%)
r_tracks = getattr(r, 'total_tracks', 0) or 0
try:
results = client.search_albums(search_query, limit=5)
except Exception as e:
# Per-source failures (rate limit, auth, transient HTTP)
# shouldn't abort the fallback chain. Log + continue.
logger.debug(
f"Auto-import: search_albums failed on {source}: {e}"
)
continue
if not results:
continue
# Score each result via the pure helper. Helper is
# tested independently in
# `tests/imports/test_album_search_scoring.py` so the
# weight math is pinned at the function boundary, not
# through the orchestrator path.
file_count = len(candidate.audio_files)
if r_tracks > 0 and file_count > 0:
count_ratio = 1.0 - abs(r_tracks - file_count) / max(r_tracks, file_count)
score += max(0, count_ratio) * 0.3
best_result = None
best_score = 0.0
for r in results:
score = _score_album_search_result(r, album, artist, file_count)
if score > best_score:
best_score = score
best_result = r
if score > best_score:
best_score = score
best_result = r
if not best_result or best_score < 0.4:
# Primary returned weak/no match — fall through to next source
if source != primary_source:
logger.debug(
f"Auto-import: {source} best score {best_score:.2f} "
f"below threshold for '{album}', trying next source"
)
continue
if not best_result or best_score < 0.4:
return None
# Get image
image_url = ''
if hasattr(best_result, 'image_url'):
image_url = best_result.image_url or ''
elif hasattr(best_result, 'images') and best_result.images:
img = best_result.images[0]
image_url = img.get('url', '') if isinstance(img, dict) else str(img)
# Get image
image_url = ''
if hasattr(best_result, 'image_url'):
image_url = best_result.image_url or ''
elif hasattr(best_result, 'images') and best_result.images:
img = best_result.images[0]
image_url = img.get('url', '') if isinstance(img, dict) else str(img)
r_artist = ''
r_artist_id = ''
if hasattr(best_result, 'artists') and best_result.artists:
a = best_result.artists[0]
if isinstance(a, dict):
r_artist = a.get('name', str(a))
# Surface the metadata-source artist ID so the
# standalone-library write can land it on the right
# `<source>_artist_id` column. Without this the
# artists row gets created but with NULL on the
# source-id, and watchlist scans can't recognise
# the artist as already in library by stable ID.
r_artist_id = str(a.get('id', '') or '')
else:
r_artist = str(a)
r_artist = ''
r_artist_id = ''
if hasattr(best_result, 'artists') and best_result.artists:
a = best_result.artists[0]
if isinstance(a, dict):
r_artist = a.get('name', str(a))
# Surface the metadata-source artist ID so the
# standalone-library write can land it on the right
# `<source>_artist_id` column. Without this the
# artists row gets created but with NULL on the
# source-id, and watchlist scans can't recognise
# the artist as already in library by stable ID.
r_artist_id = str(a.get('id', '') or '')
else:
r_artist = str(a)
# Get release date
release_date = getattr(best_result, 'release_date', '') or ''
# Get release date
release_date = getattr(best_result, 'release_date', '') or ''
if source != primary_source:
logger.info(
f"Auto-import: identified '{album}' via fallback "
f"source {source!r} (score {best_score:.2f}, primary "
f"{primary_source!r} returned nothing usable)"
)
return {
'album_id': best_result.id,
'album_name': best_result.name,
'artist_name': r_artist or artist or '',
'artist_id': r_artist_id,
'image_url': image_url,
'release_date': release_date,
'total_tracks': getattr(best_result, 'total_tracks', 0),
'source': source,
'method': method,
'identification_confidence': best_score,
}
return {
'album_id': best_result.id,
'album_name': best_result.name,
'artist_name': r_artist or artist or '',
'artist_id': r_artist_id,
'image_url': image_url,
'release_date': release_date,
'total_tracks': getattr(best_result, 'total_tracks', 0),
'source': source,
'method': method,
'identification_confidence': best_score,
}
return None
except Exception as e:
logger.debug(f"Metadata search failed for '{album}': {e}")

49
core/debug_info.py
View file

@ -14,7 +14,12 @@ from pathlib import Path
from flask import jsonify, request
from config.settings import config_manager
from core.metadata.registry import get_spotify_client
from core.metadata.registry import (
get_spotify_client,
get_primary_source,
is_hydrabase_enabled,
)
from core.metadata.status import get_spotify_status
logger = logging.getLogger(__name__)
@ -183,21 +188,47 @@ def get_debug_info():
info['paths']['music_videos_path'] = music_videos_path
info['paths']['music_videos_path_exists'] = os.path.isdir(music_videos_path)
# Services from status cache
spotify_cache = _status_cache.get('spotify', {})
# Services. `_status_cache` only carries 'media_server' and 'soulseek'
# (no 'spotify' key) so anything we used to read from `spotify_cache`
# silently defaulted to the missing-value fallback — that's the
# "music_source: unknown" bug. Spotify status now comes from the
# canonical `get_spotify_status` accessor; primary metadata source
# comes from `get_primary_source` (which already accounts for the
# auth-fallback chain — Spotify drops back to Deezer when not
# authenticated).
media_server_cache = _status_cache.get('media_server', {})
soulseek_cache = _status_cache.get('soulseek', {})
spotify_status = _safe_check(lambda: get_spotify_status(spotify_client=spotify_client), default={})
if not isinstance(spotify_status, dict):
spotify_status = {}
info['services'] = {
'music_source': spotify_cache.get('source', 'unknown'),
'spotify_connected': spotify_cache.get('connected', False),
'spotify_rate_limited': spotify_cache.get('rate_limited', False),
'music_source': _safe_check(get_primary_source, default='unknown') or 'unknown',
'spotify_connected': bool(spotify_status.get('connected', False)),
'spotify_rate_limited': bool(spotify_status.get('rate_limited', False)),
'media_server_type': media_server_cache.get('type', 'none'),
'media_server_connected': media_server_cache.get('connected', False),
'soulseek_connected': soulseek_cache.get('connected', False),
'download_source': config_manager.get('download_source.mode', 'hybrid'),
'tidal_connected': _safe_check(lambda: bool(tidal_client and tidal_client.is_authenticated())),
'qobuz_connected': _safe_check(lambda: bool(qobuz_enrichment_worker and qobuz_enrichment_worker.client and qobuz_enrichment_worker.client.is_authenticated())),
'hydrabase_connected': _safe_check(is_hydrabase_enabled),
# YouTube is URL-based via yt-dlp — no auth, always reachable as
# long as the binary is installed. Surfaced so the debug dump
# documents that YouTube is one of the available download sources
# rather than implying it doesn't exist.
'youtube_available': True,
}
# HiFi instance count — separate from connection status because each
# instance is its own independent endpoint with its own auth state.
info['services']['hifi_instance_count'] = _safe_check(
lambda: len(get_database().get_hifi_instances()), default=0
)
# Always-available public metadata sources (no auth, no per-user
# connection state). Listed so the debug dump reflects the full
# metadata surface SoulSync queries from, not just the auth-gated ones.
info['services']['always_available_metadata_sources'] = [
'deezer', 'itunes', 'musicbrainz',
]
# Enrichment workers
workers = {}
@ -300,7 +331,11 @@ def get_debug_info():
# API rate monitor — current calls/min, 24h totals, peaks, rate limit events
try:
from core.api_call_tracker import api_call_tracker
from core.metadata.status import get_spotify_status
# `get_spotify_status` is already imported at module level. A
# local re-import here would make Python treat the name as a
# function-scoped local for the WHOLE body, breaking the lambda
# at the top of get_debug_info that closes over the module-level
# binding (Python 3.12 NameError on free variables).
rates = api_call_tracker.get_all_rates()
info['api_rates'] = rates
# Rich 24h debug summary with peaks, totals, per-endpoint breakdown, events

4
core/discovery/sync.py
View file

@ -49,7 +49,7 @@ class SyncDeps:
sync_lock: Any # threading.Lock
def run_sync_task(playlist_id, playlist_name, tracks_json, automation_id=None, profile_id=1, playlist_image_url='', deps: SyncDeps = None):
def run_sync_task(playlist_id, playlist_name, tracks_json, automation_id=None, profile_id=1, playlist_image_url='', deps: SyncDeps = None, sync_mode: str = 'replace'):
"""The actual sync function that runs in the background thread."""
sync_states = deps.sync_states
sync_lock = deps.sync_lock
@ -359,7 +359,7 @@ def run_sync_task(playlist_id, playlist_name, tracks_json, automation_id=None, p
sync_service._skip_wishlist = is_wing_it
# Run the sync (this is a blocking call within this thread)
result = deps.run_async(sync_service.sync_playlist(playlist, download_missing=False, profile_id=profile_id))
result = deps.run_async(sync_service.sync_playlist(playlist, download_missing=False, profile_id=profile_id, sync_mode=sync_mode))
# Clear progress callback immediately to prevent race condition where a
# late-firing progress callback overwrites the "finished" state below

72
core/jellyfin_client.py
View file

@ -1533,6 +1533,78 @@ class JellyfinClient(MediaServerClient):
logger.debug(f"Could not set playlist poster for '{playlist_name}': {e}")
return False
def append_to_playlist(self, playlist_name: str, tracks) -> bool:
"""Append tracks to an existing playlist (creates it if missing).
Differs from `update_playlist`: never deletes existing tracks,
never recreates the playlist, no backup. Used by sync mode
'append' so user-added tracks on the server playlist survive
re-syncing the source. Dedupe-by-Id ensures we don't re-add
tracks the playlist already contains."""
if not self.ensure_connection():
return False
try:
existing_playlist = self.get_playlist_by_name(playlist_name)
if not existing_playlist:
logger.info(
f"Jellyfin append: playlist '{playlist_name}' doesn't exist yet — "
f"creating with {len(tracks)} tracks"
)
return self.create_playlist(playlist_name, tracks)
playlist_id = existing_playlist.id
existing_tracks = self.get_playlist_tracks(playlist_id)
existing_ids = {
str(t.id) for t in existing_tracks if hasattr(t, 'id') and t.id
}
new_track_ids = []
for t in tracks:
tid = None
if hasattr(t, 'id'):
tid = str(t.id) if t.id else None
elif isinstance(t, dict):
tid = str(t.get('Id') or t.get('id') or '')
if tid and tid not in existing_ids and self._is_valid_guid(tid):
new_track_ids.append(tid)
if not new_track_ids:
logger.info(
f"Jellyfin append: no new tracks to add to '{playlist_name}' "
f"(all matched tracks already present)"
)
return True
import requests
batch_size = 100
total_added = 0
for i in range(0, len(new_track_ids), batch_size):
batch = new_track_ids[i:i + batch_size]
add_url = f"{self.base_url}/Playlists/{playlist_id}/Items"
add_params = {'Ids': ','.join(batch), 'UserId': self.user_id}
resp = requests.post(
add_url, params=add_params,
headers={'X-Emby-Token': self.api_key}, timeout=30,
)
if resp.status_code in (200, 204):
total_added += len(batch)
else:
logger.error(
f"Jellyfin append batch failed: HTTP {resp.status_code} - "
f"{resp.text[:200]}"
)
return False
logger.info(
f"Jellyfin append: added {total_added} new tracks to '{playlist_name}' "
f"(skipped {len(tracks) - total_added} already present or invalid)"
)
return True
except Exception as e:
logger.error(f"Error appending to Jellyfin playlist '{playlist_name}': {e}")
return False
def update_playlist(self, playlist_name: str, tracks) -> bool:
"""Update an existing playlist or create it if it doesn't exist"""
if not self.ensure_connection():

82
core/library/path_resolver.py
View file

@ -32,7 +32,8 @@ from any background worker.
from __future__ import annotations
import os
from typing import Any, Iterable, Optional
from dataclasses import dataclass, field
from typing import Any, Iterable, List, Optional, Tuple
from utils.logging_config import get_logger
@ -40,6 +41,33 @@ from utils.logging_config import get_logger
logger = get_logger("library.path_resolver")
@dataclass
class ResolveAttempt:
"""Diagnostic record for a single `resolve_library_file_path` call.
Returned by `resolve_library_file_path_with_diagnostic` so callers
that need to surface a useful error message (instead of just a
silent None) can describe what was tried. Pure data — no side
effects, no rendering opinions.
Fields:
raw_path_existed: True if `os.path.exists(file_path)` returned
True at the start of the resolver. When this is True the
resolver short-circuits and `base_dirs_tried` will be empty.
base_dirs_tried: The ordered list of base directories the
resolver suffix-walked against (already filtered by
`os.path.isdir`).
had_config_manager: Whether a config_manager was supplied. Useful
for distinguishing "no candidates discovered" from "couldn't
even read config to discover".
had_plex_client: Same, for the Plex API probe.
"""
raw_path_existed: bool = False
base_dirs_tried: List[str] = field(default_factory=list)
had_config_manager: bool = False
had_plex_client: bool = False
def _docker_resolve_path(path_str: Any) -> Optional[str]:
"""Translate Windows-style paths to the Docker container layout.
@ -149,16 +177,52 @@ def resolve_library_file_path(
The first existing path on disk, or None when no match is found.
Never raises — failure is the None return.
"""
resolved, _ = resolve_library_file_path_with_diagnostic(
file_path,
transfer_folder=transfer_folder,
download_folder=download_folder,
config_manager=config_manager,
plex_client=plex_client,
)
return resolved
def resolve_library_file_path_with_diagnostic(
file_path: Any,
*,
transfer_folder: Optional[str] = None,
download_folder: Optional[str] = None,
config_manager: Any = None,
plex_client: Any = None,
) -> Tuple[Optional[str], ResolveAttempt]:
"""Same as ``resolve_library_file_path`` but also returns a
``ResolveAttempt`` describing what the resolver tried.
Use this when you need to surface a useful "we tried X, Y, Z" error
to the user instead of a silent None. Issue #558 (gabistek, Navidrome
on Docker): the resolver was returning None because Navidrome doesn't
expose library filesystem paths via API (unlike Plex), and the user
hadn't configured ``library.music_paths``. The Album Completeness
fix endpoint surfaced a generic "Could not determine album folder"
error with no diagnostic — user had no way to know what to configure.
"""
attempt = ResolveAttempt(
had_config_manager=config_manager is not None,
had_plex_client=plex_client is not None,
)
if not isinstance(file_path, str) or not file_path:
return None
return None, attempt
if os.path.exists(file_path):
return file_path
attempt.raw_path_existed = True
return file_path, attempt
path_parts = file_path.replace("\\", "/").split("/")
base_dirs = _collect_base_dirs(transfer_folder, download_folder, config_manager, plex_client)
attempt.base_dirs_tried = list(base_dirs)
if not base_dirs:
return None
return None, attempt
# Skip index 0 to avoid drive-letter / leading-slash artifacts
# (e.g. "E:" or "" from a leading "/").
@ -166,8 +230,12 @@ def resolve_library_file_path(
for i in range(1, len(path_parts)):
candidate = os.path.join(base, *path_parts[i:])
if os.path.exists(candidate):
return candidate
return None
return candidate, attempt
return None, attempt
__all__ = ["resolve_library_file_path"]
__all__ = [
"ResolveAttempt",
"resolve_library_file_path",
"resolve_library_file_path_with_diagnostic",
]

1
core/media_server/contract.py
View file

@ -97,6 +97,7 @@ KNOWN_PER_SERVER_METHODS = (
'get_library_stats',
'create_playlist',
'update_playlist',
'append_to_playlist',
'copy_playlist',
'get_all_playlists',
'get_playlist_by_name',

182
core/metadata/discography_filters.py Normal file
View file

@ -0,0 +1,182 @@
"""Track-level filters for the user-facing Download Discography flow.
GitHub issue #559 (trackhacs): clicking "Download Discography" on an
artist also pulled in tracks where the artist's name appeared in the
title of someone else's song. Two failure modes underneath:
1. **Cross-artist tracks.** Spotify's `artist_albums` endpoint returns
compilation / appears_on / various-artists albums where the requested
artist is featured on one or two tracks. The endpoint then added
*every* track from those albums to the wishlist, including tracks by
unrelated artists that just happened to mention the requested artist
in the title.
2. **Remix / live / acoustic / instrumental versions.** The watchlist
scanner has user-toggleable filters for these (default: exclude),
stored at `watchlist.global_include_*`. The discography backfill
repair job already honors them. The user-facing Download Discography
endpoint did not — those filters never fired for one-off discography
downloads, so users got remix-ladder bloat.
These helpers live alongside the existing `core.metadata.discography`
because they belong to the same conceptual layer (discography fetch
results, pre-wishlist) and are independently testable. The watchlist
content-type detectors (``is_remix_version`` etc.) are reused from
``core.watchlist_scanner`` rather than re-implemented — same patterns,
single source of truth.
"""
from __future__ import annotations
from typing import Any, Dict, List, Optional
from core.watchlist_scanner import (
is_acoustic_version,
is_instrumental_version,
is_live_version,
is_remix_version,
)
def track_artist_matches(track_artists: Any, requested_artist_name: str) -> bool:
"""Return True if the requested artist appears in the track's
artists list (case-insensitive exact-name membership).
`track_artists` can be the list-of-strings shape produced by
``core.metadata.album_tracks._normalize_track_artists`` (which is
what the discography fetch returns), or the list-of-dicts shape
that some upstreams pass directly. Both are accepted.
Returns True for primary-artist tracks AND feature appearances —
the requested artist need only be one of the listed artists. Only
drops tracks where the requested artist isn't named at all (the
cross-artist compilation case from #559).
"""
if not requested_artist_name:
# No artist to compare against — don't filter; let the caller
# decide. Defensive: avoids dropping every track when the
# caller forgot to pass the artist name.
return True
target = requested_artist_name.strip().lower()
if not target:
return True
if not track_artists:
return False
for entry in track_artists:
if isinstance(entry, dict):
name = entry.get('name', '') or ''
else:
name = str(entry or '')
if name.strip().lower() == target:
return True
return False
def content_type_skip_reason(
track_name: str,
album_name: str,
settings: Dict[str, Any],
) -> Optional[str]:
"""Return a short skip-reason string if the track is a content type
the user has chosen to exclude, else None.
`settings` is a dict keyed by the same names as the watchlist
globals (``include_live`` / ``include_remixes`` / ``include_acoustic``
/ ``include_instrumentals``). All default to False — i.e. exclude
by default — matching the watchlist scanner's default contract.
"""
if not settings.get('include_live', False) and is_live_version(track_name, album_name):
return 'live'
if not settings.get('include_remixes', False) and is_remix_version(track_name, album_name):
return 'remix'
if not settings.get('include_acoustic', False) and is_acoustic_version(track_name, album_name):
return 'acoustic'
if not settings.get('include_instrumentals', False) and is_instrumental_version(track_name, album_name):
return 'instrumental'
return None
def load_global_content_filter_settings(config_manager: Any) -> Dict[str, Any]:
"""Read the four watchlist content-type globals from config.
Centralises the key names so the endpoint and the helper agree on
where the settings live. All four default to False (exclude) — same
contract as the watchlist scanner.
"""
if config_manager is None:
return {
'include_live': False,
'include_remixes': False,
'include_acoustic': False,
'include_instrumentals': False,
}
try:
return {
'include_live': bool(config_manager.get('watchlist.global_include_live', False)),
'include_remixes': bool(config_manager.get('watchlist.global_include_remixes', False)),
'include_acoustic': bool(config_manager.get('watchlist.global_include_acoustic', False)),
'include_instrumentals': bool(config_manager.get('watchlist.global_include_instrumentals', False)),
}
except Exception:
return {
'include_live': False,
'include_remixes': False,
'include_acoustic': False,
'include_instrumentals': False,
}
def track_already_owned(
db: Any,
track_name: str,
requested_artist: str,
album_name: str,
server_source: Optional[str],
confidence_threshold: float = 0.7,
) -> bool:
"""Return True if the track is already in the user's library.
Discord report (Skowl): clicking "Download Discography" twice on
the same artist re-queued every track instead of skipping the
half already on disk. Trace: the endpoint added each track to the
wishlist via ``db.add_to_wishlist``, which only dedups against the
wishlist itself — once a wishlist track downloads it leaves the
wishlist, so the second discography click re-inserted everything.
The discography backfill repair job already runs the same check
via ``db.check_track_exists`` — this helper centralises the
contract so the user-facing endpoint matches that behavior.
`check_track_exists` is name+artist+album based, format-agnostic.
Skowl's "Blasphemy mode" library (FLAC converted to MP3 then
original deleted) matches just fine — track_name + artist + album
don't change with format.
Returns False on any exception so a transient DB hiccup doesn't
silently nuke a discography fetch — a redundant wishlist add is
much cheaper to recover from than a missed track.
"""
if not requested_artist or not track_name:
return False
try:
match, confidence = db.check_track_exists(
track_name, requested_artist,
confidence_threshold=confidence_threshold,
server_source=server_source,
album=album_name or None,
)
except Exception:
return False
return bool(match) and confidence >= confidence_threshold
__all__ = [
'track_artist_matches',
'content_type_skip_reason',
'load_global_content_filter_settings',
'track_already_owned',
]

14
core/metadata/enrichment.py
View file

@ -110,9 +110,21 @@ def enhance_file_metadata(file_path: str, context: dict, artist: dict, album_inf
if metadata.get("title"):
audio_file.tags.add(symbols.TIT2(encoding=3, text=[metadata["title"]]))
if metadata.get("artist"):
# TPE1 = display artist string (already joined by
# source.py with the configured separator, or
# primary-only when feat_in_title is on).
audio_file.tags.add(symbols.TPE1(encoding=3, text=[metadata["artist"]]))
# When write_multi_artist is on, ALSO write the
# multi-value list to a TXXX:Artists frame (Picard
# convention). Keeps TPE1 as the display string AND
# exposes the per-artist list for media servers
# that read ARTISTS. Pre-fix this path overwrote
# TPE1 with the list, which clobbered the
# configured separator + feat_in_title semantics.
if write_multi and len(artists_list) > 1:
audio_file.tags.add(symbols.TPE1(encoding=3, text=artists_list))
audio_file.tags.add(
symbols.TXXX(encoding=3, desc='Artists', text=list(artists_list))
)
if metadata.get("album_artist"):
audio_file.tags.add(symbols.TPE2(encoding=3, text=[metadata["album_artist"]]))
if metadata.get("album"):

80
core/metadata/source.py
View file

@ -917,8 +917,84 @@ def extract_source_metadata(context: dict, artist: dict, album_info: dict) -> di
all_artists.append(artist_item)
else:
all_artists.append(str(artist_item))
metadata["artist"] = ", ".join(all_artists)
logger.info("Metadata: Using all artists: '%s'", metadata["artist"])
# Deezer upgrade path: Deezer's `/search` endpoint only returns
# the primary artist for each track. The full contributors
# array (feat., remix collaborators, producers credited as
# artists) lives on `/track/<id>` and gets parsed by
# `_build_enhanced_track`. Without this upgrade Deezer-sourced
# tracks never get multi-artist tags even with the right
# settings on. One extra API call per Deezer-sourced track,
# only when the search response had a single artist (so it's
# a no-op when search already returned multiple).
if (source == "deezer" and len(all_artists) == 1
and source_ids.get("track_id")):
try:
from core.metadata import get_deezer_client
deezer = get_deezer_client()
if deezer:
full = deezer.get_track_details(str(source_ids["track_id"]))
if full and isinstance(full.get("artists"), list) and len(full["artists"]) > 1:
upgraded = [a for a in full["artists"] if a]
if upgraded:
logger.info(
"Metadata: Deezer contributors upgrade — search returned "
"%d artist, /track/<id> returned %d (%s)",
len(all_artists), len(upgraded), upgraded,
)
all_artists = upgraded
except Exception as e:
logger.debug("Deezer contributors upgrade failed: %s", e)
# Store the multi-artist list so the enrichment writer can emit
# proper multi-value ARTIST tags (TPE1 multi-value for ID3,
# "artists" key for Vorbis) when `write_multi_artist` is on.
# Without this assignment the field was always empty and the
# multi-artist write path silently no-op'd.
metadata["_artists_list"] = list(all_artists)
# `feat_in_title` (when true): pull featured artists out of the
# ARTIST tag entirely and append "(feat. X, Y)" to the title.
# Matches Picard / Beets convention and lets media servers
# group by primary artist instead of treating "A, B & C" as a
# distinct artist string.
# `artist_separator`: when feat_in_title is off (or there's
# only one artist) and write_multi_artist is on, this is the
# delimiter used to join all artists into the single ARTIST
# string. Picard defaults to "; " — we default to ", " to
# preserve historical behavior for users who haven't touched
# the setting.
feat_in_title = cfg.get("metadata_enhancement.tags.feat_in_title", False)
artist_separator = cfg.get("metadata_enhancement.tags.artist_separator", ", ")
if feat_in_title and len(all_artists) > 1:
metadata["artist"] = all_artists[0]
featured = all_artists[1:]
existing_title = metadata.get("title", "") or ""
# Don't double-append if the title already carries the
# featured artists. Source titles vary: "(feat. X)",
# "(featuring X)", "(ft. X)", "ft. X" (no parens), "[feat X]"
# (no period, brackets), etc. Word-boundary regex catches
# `feat`, `feat.`, `featuring`, `ft`, `ft.` regardless of
# surrounding punctuation. Case-insensitive.
import re as _feat_re
already_has_feat = bool(_feat_re.search(
r'\b(?:feat|feat\.|featuring|ft|ft\.)\b',
existing_title,
_feat_re.IGNORECASE,
))
if existing_title and not already_has_feat:
metadata["title"] = f"{existing_title} (feat. {', '.join(featured)})"
logger.info(
"Metadata: feat_in_title — primary='%s', featured=%s, title='%s'",
metadata["artist"], featured, metadata["title"],
)
else:
metadata["artist"] = artist_separator.join(all_artists)
logger.info(
"Metadata: Using all artists joined with %r: '%s'",
artist_separator, metadata["artist"],
)
else:
metadata["artist"] = artist_dict.get("name", "") or get_import_clean_artist(context)
logger.info("Metadata: Using primary artist: '%s'", metadata["artist"])

67
core/navidrome_client.py
View file

@ -974,6 +974,73 @@ class NavidromeClient(MediaServerClient):
matches.append(playlist)
return matches
def append_to_playlist(self, playlist_name: str, tracks) -> bool:
"""Append tracks to an existing playlist (creates it if missing).
Differs from `update_playlist`: never deletes existing tracks,
never recreates the playlist, no backup. Used by sync mode
'append' so user-added tracks on the server playlist survive
re-syncing the source. Dedupe-by-id ensures we don't re-add
tracks the playlist already contains."""
if not self.ensure_connection():
return False
try:
existing_playlists = self.get_playlists_by_name(playlist_name)
if not existing_playlists:
logger.info(
f"Navidrome append: playlist '{playlist_name}' doesn't exist yet — "
f"creating with {len(tracks)} tracks"
)
return self.create_playlist(playlist_name, tracks)
primary = existing_playlists[0]
existing_tracks = self.get_playlist_tracks(primary.id)
existing_ids = {
str(t.id) for t in existing_tracks if hasattr(t, 'id') and t.id
}
new_track_ids = []
for t in tracks:
tid = None
if hasattr(t, 'ratingKey'):
tid = str(t.ratingKey)
elif hasattr(t, 'id'):
tid = str(t.id) if t.id else None
elif isinstance(t, dict):
tid = str(t.get('id') or '')
if tid and tid not in existing_ids:
new_track_ids.append(tid)
if not new_track_ids:
logger.info(
f"Navidrome append: no new tracks to add to '{playlist_name}' "
f"(all matched tracks already present)"
)
return True
# Subsonic updatePlaylist: `songIdToAdd` accepts repeated values
# (requests serializes list values as repeated query/form params).
params = {
'playlistId': primary.id,
'songIdToAdd': new_track_ids,
}
response = self._make_request('updatePlaylist', params)
if response and response.get('status') == 'ok':
logger.info(
f"Navidrome append: added {len(new_track_ids)} new tracks to "
f"'{playlist_name}' (skipped {len(tracks) - len(new_track_ids)} "
f"already present)"
)
return True
logger.error(
f"Failed to append to Navidrome playlist '{playlist_name}'"
)
return False
except Exception as e:
logger.error(f"Error appending to Navidrome playlist '{playlist_name}': {e}")
return False
def update_playlist(self, playlist_name: str, tracks) -> bool:
"""Update an existing playlist or create it if it doesn't exist. Handles duplicates."""
if not self.ensure_connection():

47
core/plex_client.py
View file

@ -620,6 +620,53 @@ class PlexClient(MediaServerClient):
logger.error(f"Error copying playlist '{source_name}' to '{target_name}': {e}")
return False
def append_to_playlist(self, playlist_name: str, tracks: List[TrackInfo]) -> bool:
"""Append tracks to an existing playlist (creates it if missing).
Differs from `update_playlist`: never deletes existing tracks,
never recreates the playlist, no backup. Used by sync mode
'append' so user-added tracks on the server playlist survive
re-syncing the source. Dedupe-by-ratingKey ensures we don't
re-add tracks the playlist already contains."""
if not self.ensure_connection():
return False
try:
try:
existing_playlist = self.server.playlist(playlist_name)
except NotFound:
logger.info(
f"Plex append: playlist '{playlist_name}' doesn't exist yet — "
f"creating with {len(tracks)} tracks"
)
return self.create_playlist(playlist_name, tracks)
existing_keys = {
str(t.ratingKey) for t in existing_playlist.items()
if hasattr(t, 'ratingKey')
}
new_tracks = [
t for t in tracks
if hasattr(t, 'ratingKey') and str(t.ratingKey) not in existing_keys
]
if not new_tracks:
logger.info(
f"Plex append: no new tracks to add to '{playlist_name}' "
f"(all {len(tracks)} matched-tracks already present)"
)
return True
existing_playlist.addItems(new_tracks)
logger.info(
f"Plex append: added {len(new_tracks)} new tracks to '{playlist_name}' "
f"(skipped {len(tracks) - len(new_tracks)} already present)"
)
return True
except Exception as e:
logger.error(f"Error appending to Plex playlist '{playlist_name}': {e}")
return False
def update_playlist(self, playlist_name: str, tracks: List[TrackInfo]) -> bool:
if not self.ensure_connection():
return False

50
core/repair_jobs/acoustid_scanner.py
View file

@ -192,10 +192,38 @@ class AcoustIDScannerJob(RepairJob):
if not aid_title:
return
# Resolve which artist value to compare against, in priority order:
# 1. DB `track_artist` (per-track, manually curated or scanner-
# populated) — trust it when populated. Respects user edits
# from the enhanced library view.
# 2. File's ARTIST tag — ground truth for what's on disk.
# Catches legacy compilation tracks where `track_artist`
# column is NULL because they were downloaded before that
# column existed; the file itself has the correct per-
# track artist (Tidal/Spotify/Deezer all write it).
# 3. Album artist — final fallback for files without proper
# ARTIST tags AND no DB track_artist.
track_artist = (expected.get('track_artist') or '').strip()
if track_artist:
expected_artist = track_artist
else:
file_artist = None
try:
from core.tag_writer import read_file_tags
file_tags = read_file_tags(fpath)
file_artist = (file_tags.get('artist') or '').strip() or None
except Exception as e:
logger.debug("file-tag artist read failed for %s: %s", fname, e)
expected_artist = (
file_artist
or (expected.get('album_artist') or '').strip()
or expected['artist']
)
# Normalize and compare
norm_expected_title = _normalize(expected['title'])
norm_aid_title = _normalize(aid_title)
norm_expected_artist = _normalize(expected['artist'])
norm_expected_artist = _normalize(expected_artist)
norm_aid_artist = _normalize(aid_artist)
title_sim = SequenceMatcher(None, norm_expected_title, norm_aid_title).ratio()
@ -216,7 +244,7 @@ class AcoustIDScannerJob(RepairJob):
from core.matching.artist_aliases import artist_names_match
_, artist_sim = artist_names_match(
expected['artist'],
expected_artist,
aid_artist,
threshold=artist_threshold,
)
@ -243,14 +271,14 @@ class AcoustIDScannerJob(RepairJob):
file_path=fpath,
title=f'Wrong download: "{expected["title"]}" is actually "{aid_title}"',
description=(
f'Expected "{expected["title"]}" by {expected["artist"]}, '
f'Expected "{expected["title"]}" by {expected_artist}, '
f'but audio fingerprint matches "{aid_title}" by {aid_artist} '
f'(fingerprint: {best_score:.0%}, title match: {title_sim:.0%}, '
f'artist match: {artist_sim:.0%})'
),
details={
'expected_title': expected['title'],
'expected_artist': expected['artist'],
'expected_artist': expected_artist,
'acoustid_title': aid_title,
'acoustid_artist': aid_artist,
'fingerprint_score': round(best_score, 3),
@ -284,11 +312,21 @@ class AcoustIDScannerJob(RepairJob):
# import path when different from album artist) and fall
# back to the album artist only when the per-track column
# is NULL or empty (legacy rows / single-artist albums).
# Load `track_artist` (raw, may be empty) AND `album_artist`
# separately so `_scan_file` can tell the difference between
# 'DB has a curated per-track value' and 'DB fell back to
# album artist'. The COALESCE'd `artist` field is kept as a
# convenience for the existing `expected['artist']` consumers
# that want a single resolved value, but the resolution
# priority that actually drives the comparison is reproduced
# in `_scan_file`: track_artist → file tag → album_artist.
cursor.execute("""
SELECT t.id, t.title,
COALESCE(NULLIF(t.track_artist, ''), ar.name) AS artist,
t.file_path, t.track_number,
al.title AS album_title, al.thumb_url, ar.thumb_url
al.title AS album_title, al.thumb_url, ar.thumb_url,
NULLIF(t.track_artist, '') AS track_artist,
ar.name AS album_artist
FROM tracks t
LEFT JOIN artists ar ON ar.id = t.artist_id
LEFT JOIN albums al ON al.id = t.album_id
@ -312,6 +350,8 @@ class AcoustIDScannerJob(RepairJob):
'album_title': row[5] or '',
'album_thumb_url': row[6] or None,
'artist_thumb_url': row[7] or None,
'track_artist': row[8] or '', # raw (may be empty)
'album_artist': row[9] or '',
}
except Exception as e:
logger.error("Error loading tracks from DB: %s", e)

123
core/repair_worker.py
View file

@ -12,6 +12,7 @@ import os
import re
import shutil
import sys
import sqlite3
import threading
import time
import uuid
@ -1921,6 +1922,54 @@ class RepairWorker:
# Default
return '{num:02d} - {title}'
def _build_unresolvable_album_folder_error(self, attempt, sample_db_path):
"""Render a diagnostic error string for the Album Completeness
"couldn't find existing track on disk" failure mode.
Pre-fix this returned a flat
"Could not determine album folder from existing tracks"
which left users (especially Navidrome / Jellyfin Docker setups
where the resolver can't auto-discover library mounts) with no
way to know what to fix. The new message names the active media
server, shows one sample DB-recorded path, and lists the base
directories the resolver actually probed.
Args:
attempt: ``ResolveAttempt`` from the last resolver call.
May be ``None`` if no attempt was recorded (defensive).
sample_db_path: One example ``tracks.file_path`` value from
the album. Helps the user see what their media server is
reporting so they know what to mount / configure.
"""
active_server = 'unknown'
if self._config_manager is not None:
try:
getter = getattr(self._config_manager, 'get_active_media_server', None)
if callable(getter):
active_server = getter() or 'unknown'
else:
active_server = self._config_manager.get('active_media_server', 'unknown') or 'unknown'
except Exception as e:
logger.debug("active media server lookup failed: %s", e)
lines = [
"Could not find any existing track from this album on disk.",
f"Active media server: {active_server}.",
]
if sample_db_path:
lines.append(f"Example DB-recorded path: {sample_db_path}")
if attempt is not None:
if attempt.base_dirs_tried:
joined = ', '.join(attempt.base_dirs_tried)
lines.append(f"Probed base directories: {joined}")
else:
lines.append("No base directories were available to probe.")
lines.append(
"Fix: Settings → Library → Music Paths → add the path where "
"this container can read your library files."
)
return ' '.join(lines)
def _fix_incomplete_album(self, entity_type, entity_id, file_path, details):
"""Auto-fill an incomplete album by finding missing tracks in the library.
@ -1963,14 +2012,24 @@ class RepairWorker:
download_folder = self._config_manager.get('soulseek.download_path', '')
album_folder = None
last_attempt = None
sample_db_path = None
for t in existing_tracks:
resolved = _resolve_file_path(t.file_path, self.transfer_folder, download_folder, config_manager=self._config_manager)
from core.library.path_resolver import resolve_library_file_path_with_diagnostic
resolved, attempt = resolve_library_file_path_with_diagnostic(
t.file_path, transfer_folder=self.transfer_folder,
download_folder=download_folder, config_manager=self._config_manager,
)
last_attempt = attempt
if sample_db_path is None and isinstance(t.file_path, str) and t.file_path:
sample_db_path = t.file_path
if resolved and os.path.exists(resolved):
album_folder = os.path.dirname(resolved)
break
if not album_folder:
return {'success': False, 'error': 'Could not determine album folder from existing tracks'}
return {'success': False,
'error': self._build_unresolvable_album_folder_error(last_attempt, sample_db_path)}
# Detect filename pattern
resolved_paths = []
@ -2231,6 +2290,46 @@ class RepairWorker:
album_folder, filename_pattern, download_folder):
"""Move or copy a candidate track into the album folder and update DB."""
try:
def _fallback_server_source():
if getattr(candidate, 'server_source', None):
return candidate.server_source
if self._config_manager:
getter = getattr(self._config_manager, 'get_active_media_server', None)
if callable(getter):
return getter() or 'plex'
return self._config_manager.get('active_media_server', 'plex')
return 'plex'
def _resolve_target_context(cursor):
cursor.execute(
"""
SELECT artist_id, server_source
FROM tracks
WHERE album_id = ?
ORDER BY track_number, title
LIMIT 1
""",
(album_id,),
)
row = cursor.fetchone()
if row:
return row[0] or candidate.artist_id, row[1] or _fallback_server_source()
try:
cursor.execute(
"SELECT artist_id, server_source FROM albums WHERE id = ? LIMIT 1",
(album_id,),
)
except sqlite3.OperationalError:
row = None
else:
row = cursor.fetchone()
if row:
return row[0] or candidate.artist_id, row[1] or _fallback_server_source()
return candidate.artist_id, _fallback_server_source()
# Resolve source file
src_path = _resolve_file_path(candidate.file_path, self.transfer_folder, download_folder, config_manager=self._config_manager)
if not src_path or not os.path.exists(src_path):
@ -2264,18 +2363,15 @@ class RepairWorker:
# Update existing DB record to point to new album and path
conn = self.db._get_connection()
cursor = conn.cursor()
# Get the target album's artist_id for consistency
cursor.execute("SELECT artist_id FROM tracks WHERE album_id = ? LIMIT 1", (album_id,))
artist_row = cursor.fetchone()
target_artist_id = artist_row[0] if artist_row else candidate.artist_id
target_artist_id, target_server_source = _resolve_target_context(cursor)
cursor.execute("""
UPDATE tracks
SET album_id = ?, artist_id = ?, title = ?,
file_path = ?, track_number = ?,
file_path = ?, track_number = ?, server_source = ?,
updated_at = CURRENT_TIMESTAMP
WHERE id = ?
""", (album_id, target_artist_id, track_name,
target_path, track_number, candidate.id))
target_path, track_number, target_server_source, candidate.id))
# Clean up the source single's album if it's now empty
cursor.execute("SELECT COUNT(*) FROM tracks WHERE album_id = ?", (candidate.album_id,))
@ -2300,17 +2396,14 @@ class RepairWorker:
conn = self.db._get_connection()
cursor = conn.cursor()
# Get artist_id from existing album tracks
cursor.execute("SELECT artist_id FROM tracks WHERE album_id = ? LIMIT 1", (album_id,))
artist_row = cursor.fetchone()
target_artist_id = artist_row[0] if artist_row else candidate.artist_id
target_artist_id, target_server_source = _resolve_target_context(cursor)
cursor.execute("""
INSERT INTO tracks (id, album_id, artist_id, title, track_number, duration,
file_path, bitrate, created_at, updated_at)
VALUES (?, ?, ?, ?, ?, ?, ?, ?, CURRENT_TIMESTAMP, CURRENT_TIMESTAMP)
file_path, bitrate, server_source, created_at, updated_at)
VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, CURRENT_TIMESTAMP, CURRENT_TIMESTAMP)
""", (new_track_id, album_id, target_artist_id, track_name, track_number,
candidate.duration, target_path, candidate.bitrate))
candidate.duration, target_path, candidate.bitrate, target_server_source))
conn.commit()
finally:

123
core/soulseek_client.py
View file

@ -48,18 +48,87 @@ _SLSKD_DEFAULT_TIMEOUT = aiohttp.ClientTimeout(
)
# Search-rate-limit defaults. Pre-fix these were hardcoded magic numbers
# inside `SoulseekClient.__init__`. Lifted to module level so they're
# greppable + bumpable in one place, and so the reddit-reported case
# (Bell Canada anti-abuse trips on slskd peer-connection bursts) can
# tune them via `soulseek.search_*` config without touching code.
_DEFAULT_MAX_PER_WINDOW = 35
_DEFAULT_WINDOW_SECONDS = 220
_DEFAULT_MIN_DELAY_SECONDS = 0 # 0 = disabled (preserves prior behavior)
def compute_search_wait_seconds(
timestamps: List[float],
last_search_at: float,
now: float,
*,
max_per_window: int,
window_seconds: float,
min_delay_seconds: float,
) -> float:
"""Pure scheduler for the slskd search throttle.
Returns how many seconds the caller should sleep before issuing
the next search. ``timestamps`` is the list of recent search
timestamps already pruned to the current window (caller's job).
``last_search_at`` is the timestamp of the most recent search
(0.0 if there hasn't been one). ``now`` is the current monotonic /
wall-clock time (caller chooses — pure function only does math).
Two independent gates, return the larger:
1. **Sliding-window cap** — when ``len(timestamps) >= max_per_window``,
sleep until the oldest timestamp ages out of the window. Same
semantics as the pre-fix hardcoded behavior.
2. **Min-delay between searches** — when ``min_delay_seconds > 0``,
sleep until at least that many seconds have passed since
``last_search_at``. Smooths bursts even when the window isn't
full — this is the actual fix for the Reddit-reported case where
Bell Canada's anti-abuse trips on the rapid peer-connection
bursts that 35 back-to-back searches generate.
Returns 0.0 (no wait) when ``min_delay_seconds`` is 0 / negative
AND the window isn't full. Pure: no I/O, no side effects, no
mutation of the inputs.
"""
window_wait = 0.0
if max_per_window > 0 and len(timestamps) >= max_per_window:
oldest = timestamps[0]
window_wait = max(0.0, oldest + window_seconds - now)
delay_wait = 0.0
if min_delay_seconds > 0 and last_search_at > 0:
elapsed = now - last_search_at
delay_wait = max(0.0, min_delay_seconds - elapsed)
return max(window_wait, delay_wait)
class SoulseekClient(DownloadSourcePlugin):
def __init__(self):
self.base_url: Optional[str] = None
self.api_key: Optional[str] = None
self.download_path: Path = Path("./downloads")
self.active_searches: Dict[str, bool] = {} # search_id -> still_active
# Rate limiting for searches
self.search_timestamps: List[float] = [] # Track search timestamps
self.max_searches_per_window = 35 # Conservative limit to prevent Soulseek bans
self.rate_limit_window = 220 # seconds (3 minutes 40 seconds)
# Rate limiting for searches. Cap + window stay hardcoded —
# nobody has reported issues with the 35/220 defaults. The
# min-delay knob is the actual fix for the Reddit-reported
# case (Bell Canada anti-abuse cuts the WAN after rapid
# peer-connection bursts) — smooths bursts even when the
# sliding-window cap isn't hit. 0 = disabled (preserves prior
# behavior).
self.search_timestamps: List[float] = []
self._last_search_at: float = 0.0
self.max_searches_per_window = _DEFAULT_MAX_PER_WINDOW
self.rate_limit_window = _DEFAULT_WINDOW_SECONDS
self.search_min_delay_seconds = float(
config_manager.get('soulseek.search_min_delay_seconds', _DEFAULT_MIN_DELAY_SECONDS)
or _DEFAULT_MIN_DELAY_SECONDS
)
self._setup_client()
def _setup_client(self):
@ -103,22 +172,36 @@ class SoulseekClient(DownloadSourcePlugin):
self.search_timestamps = [ts for ts in self.search_timestamps if ts > cutoff_time]
async def _wait_for_rate_limit(self):
"""Wait if necessary to respect rate limiting"""
"""Wait if necessary to respect search rate limits.
Delegates the wait math to ``compute_search_wait_seconds`` so
the throttle logic is testable independently of asyncio.sleep
and the singleton client. Two gates apply (max wins): sliding-
window cap on searches per N seconds, plus optional min-delay
between consecutive searches (the burst-smoother).
"""
self._clean_old_timestamps()
if len(self.search_timestamps) >= self.max_searches_per_window:
# Calculate how long to wait
oldest_timestamp = self.search_timestamps[0]
wait_time = oldest_timestamp + self.rate_limit_window - time.time()
if wait_time > 0:
logger.info(f"Rate limit reached ({len(self.search_timestamps)}/{self.max_searches_per_window} searches). Waiting {wait_time:.1f} seconds...")
await asyncio.sleep(wait_time)
# Clean up again after waiting
self._clean_old_timestamps()
wait_time = compute_search_wait_seconds(
self.search_timestamps,
self._last_search_at,
time.time(),
max_per_window=self.max_searches_per_window,
window_seconds=self.rate_limit_window,
min_delay_seconds=self.search_min_delay_seconds,
)
if wait_time > 0:
logger.info(
f"Search rate limit: waiting {wait_time:.1f}s "
f"({len(self.search_timestamps)}/{self.max_searches_per_window} in window, "
f"min_delay={self.search_min_delay_seconds:.1f}s)"
)
await asyncio.sleep(wait_time)
self._clean_old_timestamps()
# Record this search attempt
self.search_timestamps.append(time.time())
now = time.time()
self.search_timestamps.append(now)
self._last_search_at = now
def get_rate_limit_status(self) -> Dict[str, Any]:
"""Get current rate limiting status"""

635
core/tidal_client.py
View file

@ -1075,6 +1075,121 @@ class TidalClient:
logger.error(f"Error getting Tidal album {album_id}: {e}")
return None
@rate_limited
def get_album_tracks(self, album_id: str, limit: Optional[int] = None) -> List[Track]:
"""Fetch every track on an album with full artist + name + duration
metadata hydrated.
Two-phase: walk `/v2/albums/{id}/relationships/items?include=items`
cursor chain to enumerate track IDs (with their position metadata —
`meta.trackNumber` + `meta.volumeNumber` for multi-disc), then
feed the IDs through the existing `_get_tracks_batch` helper for
artist + album-name resolution.
Returns a list of `Track` dataclasses with `track_number` and
`disc_number` attached as ad-hoc attributes so callers that need
per-position info (download modal, virtual playlist build) can
read them. Backend `/api/discover/album/<source>/<album_id>`
serializes these to the same shape Spotify/Deezer return."""
if not self._ensure_valid_token():
return []
# Phase 1: enumerate track IDs + position metadata via cursor pagination.
# The relationship endpoint pages at 20 items by default. The `meta`
# dict on each ref carries `trackNumber` + `volumeNumber` (multi-disc).
track_meta_by_id: Dict[str, Dict[str, int]] = {}
track_ids: List[str] = []
next_path: Optional[str] = None
while True:
if next_path:
url = (next_path if next_path.startswith('http')
else f"https://openapi.tidal.com/v2{next_path}")
params = None
else:
url = f"{self.base_url}/albums/{album_id}/relationships/items"
params = {'countryCode': 'US', 'include': 'items'}
try:
resp = self.session.get(
url, params=params,
headers={'accept': 'application/vnd.api+json'},
timeout=15,
)
except Exception as e:
logger.debug(f"Tidal album-tracks page request failed: {e}")
break
if resp.status_code != 200:
if resp.status_code == 429:
raise Exception("Rate limited (429) on get_album_tracks")
logger.debug(
f"Tidal album-tracks page returned {resp.status_code}: {resp.text[:200]}"
)
break
try:
data = resp.json()
except ValueError:
break
for item in data.get('data', []):
if item.get('type') != 'tracks':
continue
tid = item.get('id')
if not tid:
continue
tid = str(tid)
meta = item.get('meta', {}) or {}
track_meta_by_id[tid] = {
'track_number': int(meta.get('trackNumber') or 0),
'disc_number': int(meta.get('volumeNumber') or 1),
}
track_ids.append(tid)
if limit is not None and len(track_ids) >= limit:
break
if limit is not None and len(track_ids) >= limit:
break
next_path = data.get('links', {}).get('next')
if not next_path:
break
time.sleep(0.3)
if not track_ids:
return []
# Phase 2: batch hydrate via existing helper (artists + album names).
# Annotate each Track with position metadata so callers can build the
# per-track-number payload the download pipeline expects.
hydrated: List[Track] = []
for i in range(0, len(track_ids), self._COLLECTION_BATCH_SIZE):
batch_ids = track_ids[i:i + self._COLLECTION_BATCH_SIZE]
try:
batch_tracks = self._get_tracks_batch(batch_ids)
except Exception as e:
logger.debug(f"Tidal album-tracks batch hydration failed: {e}")
continue
for t in batch_tracks:
meta = track_meta_by_id.get(str(t.id), {})
t.track_number = meta.get('track_number', 0)
t.disc_number = meta.get('disc_number', 1)
hydrated.append(t)
# Tidal's relationship walk returns tracks in album order; the
# batch endpoint may not preserve order. Sort by (disc, track)
# so the modal renders the album top-down.
hydrated.sort(key=lambda t: (
getattr(t, 'disc_number', 1),
getattr(t, 'track_number', 0),
))
logger.info(
f"Retrieved {len(hydrated)}/{len(track_ids)} tracks for Tidal album {album_id}"
)
return hydrated
@rate_limited
def get_track(self, track_id: str) -> Optional[Dict]:
"""Get full track details by Tidal ID."""
@ -1455,244 +1570,14 @@ class TidalClient:
logger.error(f"Error getting Tidal user info: {e}")
return None
def get_favorite_artists(self, limit: int = 200) -> list:
"""Fetch user's favorite artists from Tidal.
Returns list of dicts with tidal_id, name, image_url."""
try:
if not self._ensure_valid_token():
logger.debug("Tidal not authenticated — cannot fetch favorites")
return []
user_id, api_version = self._get_user_id()
if not user_id:
logger.warning("Could not get Tidal user ID for favorites")
return []
artists = []
if api_version == 'v2':
# V2 API: /v2/favorites with filter
offset = 0
while len(artists) < limit:
try:
headers = self.session.headers.copy()
headers['accept'] = 'application/vnd.api+json'
resp = requests.get(
f"{self.base_url}/favorites",
params={
'countryCode': 'US',
'filter[user.id]': user_id,
'filter[type]': 'ARTISTS',
'include': 'artists',
'page[limit]': min(50, limit - len(artists)),
'page[offset]': offset
},
headers=headers, timeout=15
)
if resp.status_code != 200:
logger.debug(f"Tidal V2 favorites returned {resp.status_code}, trying V1")
break
data = resp.json()
# Parse included artists
included = data.get('included', [])
if not included:
items = data.get('data', [])
if not items:
break
# Try to extract from data items directly
for item in items:
attrs = item.get('attributes', {})
name = attrs.get('name', '')
if name:
img = None
img_data = item.get('relationships', {}).get('image', {}).get('data', {})
if isinstance(img_data, dict) and img_data.get('id'):
img = f"https://resources.tidal.com/images/{img_data['id'].replace('-', '/')}/750x750.jpg"
artists.append({'tidal_id': item.get('id', ''), 'name': name, 'image_url': img})
else:
for inc in included:
if inc.get('type') == 'artists':
attrs = inc.get('attributes', {})
img = None
img_rel = inc.get('relationships', {}).get('image', {}).get('data', {})
if isinstance(img_rel, dict) and img_rel.get('id'):
img = f"https://resources.tidal.com/images/{img_rel['id'].replace('-', '/')}/750x750.jpg"
artists.append({
'tidal_id': str(inc.get('id', '')),
'name': attrs.get('name', ''),
'image_url': img,
})
if not data.get('links', {}).get('next'):
break
offset += 50
import time
time.sleep(0.5)
except Exception as e:
logger.debug(f"Tidal V2 favorites error: {e}")
break
# Fallback to V1 API if V2 returned nothing
if not artists:
try:
offset = 0
while len(artists) < limit:
resp = self.session.get(
f"{self.alt_base_url}/users/{user_id}/favorites/artists",
params={'countryCode': 'US', 'limit': min(50, limit - len(artists)), 'offset': offset},
timeout=15
)
if resp.status_code != 200:
logger.debug(f"Tidal V1 favorites returned {resp.status_code}")
break
data = resp.json()
items = data.get('items', [])
if not items:
break
for item in items:
a = item.get('item', item)
img_id = (a.get('picture') or '').replace('-', '/')
img = f"https://resources.tidal.com/images/{img_id}/750x750.jpg" if img_id else None
artists.append({
'tidal_id': str(a.get('id', '')),
'name': a.get('name', ''),
'image_url': img,
})
total = data.get('totalNumberOfItems', 0)
offset += len(items)
if offset >= total:
break
import time
time.sleep(0.5)
except Exception as e:
logger.debug(f"Tidal V1 favorites error: {e}")
logger.info(f"Retrieved {len(artists)} favorite artists from Tidal")
return artists
except Exception as e:
logger.error(f"Error fetching Tidal favorite artists: {e}")
return []
def get_favorite_albums(self, limit: int = 200) -> list:
"""Fetch user's favorite albums from Tidal.
Returns list of dicts with tidal_id, album_name, artist_name, image_url, release_date, total_tracks."""
try:
if not self._ensure_valid_token():
logger.debug("Tidal not authenticated — cannot fetch favorite albums")
return []
user_id, api_version = self._get_user_id()
if not user_id:
logger.warning("Could not get Tidal user ID for favorite albums")
return []
albums = []
if api_version == 'v2':
offset = 0
while len(albums) < limit:
try:
headers = self.session.headers.copy()
headers['accept'] = 'application/vnd.api+json'
resp = requests.get(
f"{self.base_url}/favorites",
params={
'countryCode': 'US',
'filter[user.id]': user_id,
'filter[type]': 'ALBUMS',
'include': 'albums',
'page[limit]': min(50, limit - len(albums)),
'page[offset]': offset
},
headers=headers, timeout=15
)
if resp.status_code != 200:
logger.debug(f"Tidal V2 favorite albums returned {resp.status_code}, trying V1")
break
data = resp.json()
included = data.get('included', [])
items = included if included else data.get('data', [])
if not items:
break
for item in items:
if included and item.get('type') not in ('albums', 'album'):
continue
attrs = item.get('attributes', {})
title = attrs.get('title', '')
if not title:
continue
img = None
img_rel = item.get('relationships', {}).get('image', {}).get('data', {})
if isinstance(img_rel, dict) and img_rel.get('id'):
img = f"https://resources.tidal.com/images/{img_rel['id'].replace('-', '/')}/750x750.jpg"
artist_name = ''
artist_rel = attrs.get('artists', [{}])
if artist_rel and isinstance(artist_rel, list):
artist_name = artist_rel[0].get('name', '') if isinstance(artist_rel[0], dict) else ''
albums.append({
'tidal_id': str(item.get('id', '')),
'album_name': title,
'artist_name': artist_name,
'image_url': img,
'release_date': attrs.get('releaseDate', ''),
'total_tracks': attrs.get('numberOfTracks', 0),
})
if not data.get('links', {}).get('next'):
break
offset += 50
import time
time.sleep(0.5)
except Exception as e:
logger.debug(f"Tidal V2 favorite albums error: {e}")
break
# Fallback to V1 API
if not albums:
try:
offset = 0
while len(albums) < limit:
resp = self.session.get(
f"{self.alt_base_url}/users/{user_id}/favorites/albums",
params={'countryCode': 'US', 'limit': min(50, limit - len(albums)), 'offset': offset},
timeout=15
)
if resp.status_code != 200:
logger.debug(f"Tidal V1 favorite albums returned {resp.status_code}")
break
data = resp.json()
items = data.get('items', [])
if not items:
break
for item in items:
a = item.get('item', item)
img_id = (a.get('cover') or '').replace('-', '/')
img = f"https://resources.tidal.com/images/{img_id}/750x750.jpg" if img_id else None
artist_name = ''
if isinstance(a.get('artist'), dict):
artist_name = a['artist'].get('name', '')
elif isinstance(a.get('artists'), list) and a['artists']:
artist_name = a['artists'][0].get('name', '')
albums.append({
'tidal_id': str(a.get('id', '')),
'album_name': a.get('title', ''),
'artist_name': artist_name,
'image_url': img,
'release_date': a.get('releaseDate', ''),
'total_tracks': a.get('numberOfTracks', 0),
})
total = data.get('totalNumberOfItems', 0)
offset += len(items)
if offset >= total:
break
import time
time.sleep(0.5)
except Exception as e:
logger.debug(f"Tidal V1 favorite albums error: {e}")
logger.info(f"Retrieved {len(albums)} favorite albums from Tidal")
return albums
except Exception as e:
logger.error(f"Error fetching Tidal favorite albums: {e}")
return []
# `get_favorite_artists` and `get_favorite_albums` were defined here
# against the legacy `/v2/favorites?filter[type]=...` endpoint with a
# V1 fallback. Both paths are dead in 2026: V2 returns 404 for
# personal favorites (it's scoped to third-party-app-created
# collections only), and V1 returns 403 because modern OAuth tokens
# carry `collection.read` instead of the legacy `r_usr` scope V1
# demands. Replaced by the V2 user-collection endpoints below — see
# the "Favorited albums + artists" section near the end of this class.
# ------------------------------------------------------------------
# User Collection ("Favorite Tracks" — Tidal calls this "My Collection")
@ -1732,15 +1617,23 @@ class TidalClient:
# a user-actionable hint instead of silently hiding the row.
_COLLECTION_TRACKS_PATH = "userCollectionTracks/me/relationships/items"
_COLLECTION_ALBUMS_PATH = "userCollectionAlbums/me/relationships/items"
_COLLECTION_ARTISTS_PATH = "userCollectionArtists/me/relationships/items"
_COLLECTION_BATCH_SIZE = 20 # Tidal `filter[id]` page cap
@rate_limited
def _iter_collection_track_ids(self, max_ids: Optional[int] = None) -> List[str]:
"""Walk the cursor-paginated collection endpoint and return the
list of track IDs in the user's Favorite Tracks.
def _iter_collection_resource_ids(self, path: str, expected_type: str,
max_ids: Optional[int] = None) -> List[str]:
"""Walk a cursor-paginated collection endpoint and return the
list of resource IDs (tracks / albums / artists).
``max_ids`` caps the walk early — used by callers that only
need a count or a partial list. Returns ``[]`` when not
Generic across all three favorited-resource endpoints — the
only differences between them are the path segment and the
``type`` field on each ``data[]`` entry. Pagination, auth,
scope-failure detection, and the diagnostic logging are
identical.
``max_ids`` caps the walk early. Returns ``[]`` when not
authenticated or when the endpoint refuses (e.g. token without
``collection.read`` scope). On 401/403 also flips
``self._collection_needs_reconnect = True`` so the caller can
@ -1750,10 +1643,10 @@ class TidalClient:
self._collection_needs_reconnect = False
if not self._ensure_valid_token():
logger.debug("Tidal not authenticated — cannot fetch collection tracks")
logger.debug("Tidal not authenticated — cannot fetch collection %s", expected_type)
return []
track_ids: List[str] = []
ids: List[str] = []
next_path: Optional[str] = None
while True:
@ -1763,7 +1656,7 @@ class TidalClient:
url = next_path if next_path.startswith('http') else f"https://openapi.tidal.com/v2{next_path}"
params = None
else:
url = f"{self.base_url}/{self._COLLECTION_TRACKS_PATH}"
url = f"{self.base_url}/{path}"
params = {
'countryCode': 'US',
'locale': 'en-US',
@ -1811,13 +1704,13 @@ class TidalClient:
break
for item in data.get('data', []):
if item.get('type') != 'tracks':
if item.get('type') != expected_type:
continue
tid = item.get('id')
if tid:
track_ids.append(str(tid))
if max_ids is not None and len(track_ids) >= max_ids:
return track_ids
rid = item.get('id')
if rid:
ids.append(str(rid))
if max_ids is not None and len(ids) >= max_ids:
return ids
next_path = data.get('links', {}).get('next')
if not next_path:
@ -1825,7 +1718,25 @@ class TidalClient:
time.sleep(0.3) # Cursor pagination courtesy delay
return track_ids
return ids
def _iter_collection_track_ids(self, max_ids: Optional[int] = None) -> List[str]:
"""Favorited tracks — thin wrapper over the generic walker."""
return self._iter_collection_resource_ids(
self._COLLECTION_TRACKS_PATH, 'tracks', max_ids,
)
def _iter_collection_album_ids(self, max_ids: Optional[int] = None) -> List[str]:
"""Favorited albums — thin wrapper over the generic walker."""
return self._iter_collection_resource_ids(
self._COLLECTION_ALBUMS_PATH, 'albums', max_ids,
)
def _iter_collection_artist_ids(self, max_ids: Optional[int] = None) -> List[str]:
"""Favorited artists — thin wrapper over the generic walker."""
return self._iter_collection_resource_ids(
self._COLLECTION_ARTISTS_PATH, 'artists', max_ids,
)
def collection_needs_reconnect(self) -> bool:
"""True when the most recent collection fetch hit a 401/403 —
@ -1879,6 +1790,224 @@ class TidalClient:
logger.error(f"Error fetching Tidal collection tracks: {e}")
return []
# ------------------------------------------------------------------
# Favorited albums + artists — V2 collection endpoints
# ------------------------------------------------------------------
#
# Same problem the tracks side hit on issue #502: the prior
# `/v2/favorites?filter[type]=ALBUMS|ARTISTS` endpoints are
# deprecated (404) and the V1 fallback (`/v1/users/<id>/favorites/
# albums|artists`) returns 403 because modern OAuth tokens with
# `collection.read` scope don't have the legacy `r_usr` scope V1
# requires. Discord-reported symptom: Discover → Your Albums (and
# Your Artists) section shows nothing for Tidal users regardless
# of how many albums/artists they've favorited.
#
# Fix mirrors the tracks path:
# 1) Cursor-walk `/v2/userCollection{Albums|Artists}/me/relationships/items`
# via `_iter_collection_album_ids` / `_iter_collection_artist_ids`
# (lifted into the generic `_iter_collection_resource_ids` helper).
# 2) Batch-hydrate via `/v2/{albums|artists}?filter[id]=...&include=...`
# with single-request fan-out (artists+coverArt for albums,
# profileArt for artists). Parses JSON:API `included[]` for
# artist names + image URLs.
#
# Public surface preserves the existing return shape — list of
# dicts matching what `database.upsert_liked_album` /
# `upsert_liked_artist` consume — so web_server.py callers
# (`/api/discover/your-albums-fetch` and equivalent) stay
# byte-identical.
@rate_limited
def _get_albums_batch(self, album_ids: List[str]) -> List[Dict[str, Any]]:
"""Batch-fetch album metadata + cover art + artist names in
one request via JSON:API extended-include semantics. Returns
list of dicts matching `database.upsert_liked_album` kwargs."""
if not album_ids:
return []
try:
params = {
'countryCode': 'US',
'include': 'artists,coverArt',
'filter[id]': ','.join(album_ids),
}
headers = {'accept': 'application/vnd.api+json'}
resp = self.session.get(
f"{self.base_url}/albums",
params=params, headers=headers, timeout=15,
)
if resp.status_code != 200:
logger.debug(
f"Tidal albums batch returned {resp.status_code}: {resp.text[:200]}"
)
return []
data = resp.json()
artists_by_id, artworks_by_id = self._build_included_maps(data.get('included', []))
results: List[Dict[str, Any]] = []
for item in data.get('data', []):
if item.get('type') != 'albums':
continue
attrs = item.get('attributes', {})
rels = item.get('relationships', {})
results.append({
'tidal_id': str(item.get('id', '')),
'album_name': attrs.get('title', '') or '',
'artist_name': self._first_artist_name(rels, artists_by_id),
'image_url': self._first_artwork_url(rels.get('coverArt', {}), artworks_by_id),
'release_date': attrs.get('releaseDate', '') or '',
'total_tracks': int(attrs.get('numberOfItems') or 0),
})
return results
except Exception as e:
logger.debug(f"Tidal _get_albums_batch error: {e}")
return []
@rate_limited
def _get_artists_batch(self, artist_ids: List[str]) -> List[Dict[str, Any]]:
"""Batch-fetch artist metadata + profile image. Returns list
of dicts matching the prior `get_favorite_artists` shape
(`tidal_id`, `name`, `image_url`)."""
if not artist_ids:
return []
try:
params = {
'countryCode': 'US',
'include': 'profileArt',
'filter[id]': ','.join(artist_ids),
}
headers = {'accept': 'application/vnd.api+json'}
resp = self.session.get(
f"{self.base_url}/artists",
params=params, headers=headers, timeout=15,
)
if resp.status_code != 200:
logger.debug(
f"Tidal artists batch returned {resp.status_code}: {resp.text[:200]}"
)
return []
data = resp.json()
_, artworks_by_id = self._build_included_maps(data.get('included', []))
results: List[Dict[str, Any]] = []
for item in data.get('data', []):
if item.get('type') != 'artists':
continue
attrs = item.get('attributes', {})
rels = item.get('relationships', {})
results.append({
'tidal_id': str(item.get('id', '')),
'name': attrs.get('name', '') or '',
'image_url': self._first_artwork_url(rels.get('profileArt', {}), artworks_by_id),
})
return results
except Exception as e:
logger.debug(f"Tidal _get_artists_batch error: {e}")
return []
@staticmethod
def _build_included_maps(included: List[Dict[str, Any]]):
"""Index a JSON:API `included[]` array by resource type so the
per-resource lookup in batch-hydrate is O(1) per relationship
ref rather than O(n)."""
artists_by_id: Dict[str, Dict[str, Any]] = {}
artworks_by_id: Dict[str, Dict[str, Any]] = {}
for inc in included:
inc_id = str(inc.get('id', ''))
if not inc_id:
continue
inc_type = inc.get('type')
if inc_type == 'artists':
artists_by_id[inc_id] = inc
elif inc_type == 'artworks':
artworks_by_id[inc_id] = inc
return artists_by_id, artworks_by_id
@staticmethod
def _first_artist_name(relationships: Dict[str, Any],
artists_by_id: Dict[str, Dict[str, Any]]) -> str:
"""Resolve the primary artist name from a relationships block
+ included-artists map. Returns '' if not resolvable so the
upsert path doesn't trip on None."""
artist_refs = relationships.get('artists', {}).get('data', [])
if not artist_refs:
return ''
first_id = str(artist_refs[0].get('id', ''))
artist_obj = artists_by_id.get(first_id, {})
return artist_obj.get('attributes', {}).get('name', '') or ''
@staticmethod
def _first_artwork_url(artwork_relationship: Dict[str, Any],
artworks_by_id: Dict[str, Dict[str, Any]]) -> Optional[str]:
"""Resolve the largest cover/profile image URL from an artwork
relationship + included-artworks map. Tidal returns files
largest-first so picking files[0] gets the highest-resolution
variant (typically 1280×1280)."""
refs = artwork_relationship.get('data', [])
if not refs:
return None
first_id = str(refs[0].get('id', ''))
artwork = artworks_by_id.get(first_id, {})
files = artwork.get('attributes', {}).get('files', [])
if not files:
return None
return files[0].get('href')
def get_favorite_albums(self, limit: int = 200) -> List[Dict[str, Any]]:
"""Fetch user's favorited albums via the V2 user-collection
endpoint. Replaces the prior `/v2/favorites` + V1-fallback
path which is now dead (V2 endpoint deprecated, V1 returns
403 for modern OAuth tokens lacking `r_usr` scope).
Returns list of dicts matching `database.upsert_liked_album`
kwargs — the discover.py 'Your Albums' aggregator iterates
these and writes them to the liked_albums table."""
try:
album_ids = self._iter_collection_album_ids(max_ids=limit)
if not album_ids:
return []
results: List[Dict[str, Any]] = []
for i in range(0, len(album_ids), self._COLLECTION_BATCH_SIZE):
batch = album_ids[i:i + self._COLLECTION_BATCH_SIZE]
results.extend(self._get_albums_batch(batch))
logger.info(
f"Retrieved {len(results)}/{len(album_ids)} favorite albums from Tidal"
)
return results
except Exception as e:
logger.error(f"Error fetching Tidal favorite albums: {e}")
return []
def get_favorite_artists(self, limit: int = 200) -> List[Dict[str, Any]]:
"""Fetch user's favorited artists via the V2 user-collection
endpoint. Replaces the prior `/v2/favorites` + V1-fallback
path (dead for the same reason as `get_favorite_albums`).
Returns list of dicts matching the prior shape (`tidal_id`,
`name`, `image_url`) so web_server.py's `/api/discover/
your-artists-fetch` aggregator path stays byte-identical."""
try:
artist_ids = self._iter_collection_artist_ids(max_ids=limit)
if not artist_ids:
return []
results: List[Dict[str, Any]] = []
for i in range(0, len(artist_ids), self._COLLECTION_BATCH_SIZE):
batch = artist_ids[i:i + self._COLLECTION_BATCH_SIZE]
results.extend(self._get_artists_batch(batch))
logger.info(
f"Retrieved {len(results)}/{len(artist_ids)} favorite artists from Tidal"
)
return results
except Exception as e:
logger.error(f"Error fetching Tidal favorite artists: {e}")
return []
# Global instance
_tidal_client = None

24
entrypoint.sh
View file

@ -72,9 +72,31 @@ chown soulsync:soulsync /app/config/settings.py 2>/dev/null || true
# Ensure all directories exist with correct ownership.
# Only the directory nodes themselves need chown here — the recursive chown
# above already ran if UIDs changed, so avoid walking the whole tree every start.
mkdir -p /app/config /app/data /app/logs /app/downloads /app/Transfer /app/Staging
# Both the mkdir and chown tolerate failure (`|| true`): the Dockerfile
# pre-bakes every dir and bind-mounted volumes from the host already exist
# at this point, so the only failure modes are:
# - rootless Docker/Podman where in-container root maps to a host UID
# that can't write to a bind-mounted path (mkdir EACCES)
# - read-only mounts or NFS with squashed root (chown EPERM)
# Pre-mid-2026 the chown line had `|| true` but mkdir didn't — combined
# with `set -e`, a permission-denied mkdir crashed the container into a
# restart loop. Both lines are now best-effort.
mkdir -p /app/config /app/data /app/logs /app/downloads /app/Transfer /app/Staging 2>/dev/null || true
chown soulsync:soulsync /app/config /app/data /app/logs /app/downloads /app/Transfer /app/Staging 2>/dev/null || true
# Writability audit — surface a loud warning if any bind-mounted dir
# isn't writable by the soulsync user. The restart-loop fix above makes
# the container start regardless, but a non-writable Staging / downloads
# / Transfer will fail silently inside the app (auto-import quarantine,
# download writes). Better to log now than to debug missing files later.
for dir in /app/config /app/data /app/logs /app/downloads /app/Transfer /app/Staging /app/MusicVideos /app/scripts; do
if [ -d "$dir" ] && ! gosu soulsync test -w "$dir" 2>/dev/null; then
echo "⚠️ WARNING: $dir is not writable by soulsync (uid $(id -u soulsync))."
echo " Host bind-mount perms likely mismatch the PUID/PGID env vars."
echo " Fix on host: chown -R $(id -u soulsync):$(id -g soulsync) $(echo $dir | sed 's|/app/|./|')"
fi
done
echo "✅ Configuration initialized successfully"
# Display final user info

19
services/sync_service.py
View file

@ -171,7 +171,7 @@ class PlaylistSyncService:
failed_tracks=failed_tracks
))
async def sync_playlist(self, playlist: SpotifyPlaylist, download_missing: bool = False, profile_id: int = None) -> SyncResult:
async def sync_playlist(self, playlist: SpotifyPlaylist, download_missing: bool = False, profile_id: int = None, sync_mode: str = 'replace') -> SyncResult:
self._active_profile_id = profile_id
# Check if THIS specific playlist is already syncing
if playlist.name in self.syncing_playlists:
@ -314,9 +314,20 @@ class PlaylistSyncService:
logger.error("No active media client available for playlist sync")
sync_success = False
else:
logger.info(f"Syncing playlist '{playlist.name}' to {server_type.upper()} server")
# THE FIX: Ensure we are passing the correct, native track objects to the client
sync_success = media_client.update_playlist(playlist.name, valid_tracks)
logger.info(
f"Syncing playlist '{playlist.name}' to {server_type.upper()} server "
f"(mode: {sync_mode})"
)
# sync_mode == 'append' preserves user-added tracks on the server
# playlist (CJFC discord report) — never deletes, only adds new
# ones via the per-server `append_to_playlist`. The sync UI
# hides the Sync button entirely on SoulSync standalone (which
# has no playlist methods), so every client that reaches this
# point implements both methods.
if sync_mode == 'append':
sync_success = media_client.append_to_playlist(playlist.name, valid_tracks)
else:
sync_success = media_client.update_playlist(playlist.name, valid_tracks)
synced_tracks = len(plex_tracks) if sync_success else 0
failed_tracks = len(playlist.tracks) - synced_tracks - downloaded_tracks

2
tests/discovery/test_discovery_sync.py
View file

@ -73,7 +73,7 @@ class _FakeSyncService:
def clear_progress_callback(self, playlist_name):
self.cleared_callbacks.append(playlist_name)
async def sync_playlist(self, playlist, download_missing=False, profile_id=1):
async def sync_playlist(self, playlist, download_missing=False, profile_id=1, sync_mode='replace'):
if self._raise_on_sync:
raise self._raise_on_sync
return self._sync_result

217
tests/imports/test_album_search_scoring.py Normal file
View file

@ -0,0 +1,217 @@
"""Pin `_score_album_search_result` weight math.
Helper extracted from the inline scoring block inside
`_search_metadata_source` (auto-import album identification). Lifting
it to a pure function lets each weight be tested in isolation
without mocking the full source-chain orchestrator.
Weights (constants in `core.auto_import_worker`):
- `_ALBUM_NAME_WEIGHT` = 0.5
- `_ARTIST_NAME_WEIGHT` = 0.2 (skipped when target_artist falsy)
- `_TRACK_COUNT_WEIGHT` = 0.3 (skipped when either side has 0 tracks)
Maximum score is 1.0 when all three components match perfectly. The
0.4 threshold in the orchestrator means a result needs at least one
strong signal plus a partial second — pure track-count match alone
(0.3) is below threshold.
"""
from __future__ import annotations
from types import SimpleNamespace
import pytest
from core.auto_import_worker import (
_ALBUM_NAME_WEIGHT,
_ARTIST_NAME_WEIGHT,
_TRACK_COUNT_WEIGHT,
_score_album_search_result,
)
def _result(name: str, artist_name: str = "", total_tracks: int = 0):
"""Minimal album-result stub matching the shape `search_albums`
returns. `artists` is the list-of-dicts shape every adapter uses."""
artists = [{"name": artist_name}] if artist_name else []
return SimpleNamespace(name=name, artists=artists, total_tracks=total_tracks)
# ---------------------------------------------------------------------------
# Component weights — pinned at the boundary
# ---------------------------------------------------------------------------
class TestWeightConstants:
def test_weights_sum_to_one(self):
"""Total weight budget = 1.0. If a weight is bumped without
adjusting another, perfect-match score drifts above/below 1.0
and the 0.4 threshold semantics shift silently."""
total = _ALBUM_NAME_WEIGHT + _ARTIST_NAME_WEIGHT + _TRACK_COUNT_WEIGHT
assert total == pytest.approx(1.0, abs=1e-9), (
f"Weights must sum to 1.0; got {total}"
)
def test_album_weight_is_dominant(self):
"""Album name has 50% — strongest signal. If artist or track
count weight ever exceeds album weight, the matching semantics
flip (e.g. a wrong-album-right-count result could outscore
a right-album-wrong-count one). Pin so a future weight tweak
doesn't break this invariant."""
assert _ALBUM_NAME_WEIGHT > _ARTIST_NAME_WEIGHT
assert _ALBUM_NAME_WEIGHT > _TRACK_COUNT_WEIGHT
# ---------------------------------------------------------------------------
# Perfect match → 1.0
# ---------------------------------------------------------------------------
class TestPerfectMatch:
def test_all_three_perfect_returns_one(self):
r = _result("Test Album", "Test Artist", total_tracks=10)
score = _score_album_search_result(r, "Test Album", "Test Artist", file_count=10)
assert score == pytest.approx(1.0, abs=1e-9)
# ---------------------------------------------------------------------------
# Album name component
# ---------------------------------------------------------------------------
class TestAlbumNameWeight:
def test_exact_album_match_no_other_signals(self):
"""Album name matches perfectly but no artist provided and
no track count match — score is just 50%."""
r = _result("Test Album", total_tracks=0)
score = _score_album_search_result(r, "Test Album", target_artist=None, file_count=0)
assert score == pytest.approx(_ALBUM_NAME_WEIGHT, abs=1e-9)
def test_completely_different_album_zero_album_component(self):
r = _result("Totally Different", total_tracks=0)
score = _score_album_search_result(r, "Test Album", target_artist=None, file_count=0)
# SequenceMatcher would return some small non-zero similarity even
# for fully different strings, so just verify it's well below 0.5
assert score < _ALBUM_NAME_WEIGHT * 0.5
# ---------------------------------------------------------------------------
# Artist component
# ---------------------------------------------------------------------------
class TestArtistWeight:
def test_artist_match_adds_full_artist_weight(self):
r = _result("Album", "Artist", total_tracks=0)
with_artist = _score_album_search_result(r, "Album", "Artist", file_count=0)
without_artist = _score_album_search_result(r, "Album", None, file_count=0)
# Difference = artist weight
assert with_artist - without_artist == pytest.approx(_ARTIST_NAME_WEIGHT, abs=1e-9)
def test_target_artist_none_skips_artist_component(self):
"""When target_artist is falsy (None / empty), artist weight
contributes zero — not a penalty, not a bonus. Lets album-
only searches (e.g. from a folder name with no artist info)
still hit the threshold via album + track count alone."""
r = _result("Album", "WrongArtist", total_tracks=10)
score = _score_album_search_result(r, "Album", None, file_count=10)
# Album + track count perfect = 0.5 + 0.3 = 0.8 (artist weight skipped)
assert score == pytest.approx(_ALBUM_NAME_WEIGHT + _TRACK_COUNT_WEIGHT, abs=1e-9)
def test_string_artist_not_dict_still_scored(self):
"""Some adapters return `artists` as list-of-strings instead
of list-of-dicts. Helper must handle both shapes."""
r = SimpleNamespace(name="Album", artists=["Just A String"], total_tracks=0)
score_string = _score_album_search_result(r, "Album", "Just A String", 0)
assert score_string >= _ALBUM_NAME_WEIGHT + _ARTIST_NAME_WEIGHT - 0.05
def test_empty_artists_list_treats_as_no_artist(self):
r = _result("Album", artist_name="", total_tracks=0) # no artist
score = _score_album_search_result(r, "Album", "Some Target", file_count=0)
# Artist sim against empty string is 0 → no artist weight contribution
assert score == pytest.approx(_ALBUM_NAME_WEIGHT, abs=1e-9)
# ---------------------------------------------------------------------------
# Track count component
# ---------------------------------------------------------------------------
class TestTrackCountWeight:
def test_exact_track_count_match_full_weight(self):
r = _result("Album", total_tracks=10)
score = _score_album_search_result(r, "Album", None, file_count=10)
# Album (perfect) + track count (perfect) — no artist component
assert score == pytest.approx(_ALBUM_NAME_WEIGHT + _TRACK_COUNT_WEIGHT, abs=1e-9)
def test_off_by_one_track_count_near_full(self):
"""1 track off out of 10 → ratio = 1.0 - 1/10 = 0.9 → 0.9 * 0.3 = 0.27"""
r = _result("Album", total_tracks=10)
score = _score_album_search_result(r, "Album", None, file_count=9)
expected = _ALBUM_NAME_WEIGHT + (0.9 * _TRACK_COUNT_WEIGHT)
assert score == pytest.approx(expected, abs=1e-9)
def test_bandcamp_vs_streaming_track_count_mismatch(self):
"""Reporter's exact case: Bandcamp 7-track album vs Spotify
4-track release. Track count ratio = 1.0 - 3/7 = ~0.571.
With perfect album + artist match, total = 0.5 + 0.2 + 0.171
= 0.871 → comfortably above the 0.4 threshold so the album
still identifies despite the count mismatch."""
r = _result("Work in Progress", "Godly the Ruler", total_tracks=4)
score = _score_album_search_result(r, "Work in Progress", "Godly the Ruler", file_count=7)
assert score > 0.4, (
f"Bandcamp-vs-streaming case must still pass threshold; got {score:.3f}"
)
# Sanity bound — score should land around 0.87
assert 0.85 < score < 0.90
def test_zero_track_count_from_source_skips_track_component(self):
"""Some search responses don't include total_tracks. Helper
must not penalize — just skip the track-count component."""
r = _result("Album", total_tracks=0)
score = _score_album_search_result(r, "Album", None, file_count=10)
# Only album component contributes
assert score == pytest.approx(_ALBUM_NAME_WEIGHT, abs=1e-9)
def test_zero_file_count_skips_track_component(self):
"""Defensive: candidate has 0 files (somehow). Don't divide
by zero or skew the score."""
r = _result("Album", total_tracks=10)
score = _score_album_search_result(r, "Album", None, file_count=0)
assert score == pytest.approx(_ALBUM_NAME_WEIGHT, abs=1e-9)
def test_huge_mismatch_track_count_no_negative_contribution(self):
"""File count 1, source track count 100 → ratio = 1 - 99/100
= 0.01. Tiny but non-negative. `max(0, ...)` guards against
any future formula change introducing a negative."""
r = _result("Album", total_tracks=100)
score = _score_album_search_result(r, "Album", None, file_count=1)
assert score >= _ALBUM_NAME_WEIGHT # at least the album component
# ---------------------------------------------------------------------------
# Edge cases — defensive
# ---------------------------------------------------------------------------
class TestEdgeCases:
def test_album_result_without_name_attribute(self):
"""If the result somehow lacks `.name` (unusual adapter
return), helper falls back to '' and scores 0 album sim."""
r = SimpleNamespace(artists=[], total_tracks=0) # no `.name`
score = _score_album_search_result(r, "Test Album", None, file_count=0)
assert score == 0.0
def test_album_result_without_artists_attribute(self):
"""If `.artists` is missing, treat as empty list."""
r = SimpleNamespace(name="Album", total_tracks=0) # no .artists
score = _score_album_search_result(r, "Album", "Target Artist", file_count=0)
# Album matches perfectly; artist sim against missing is 0
assert score == pytest.approx(_ALBUM_NAME_WEIGHT, abs=1e-9)
def test_album_result_with_none_total_tracks(self):
"""Some adapters return None for missing total_tracks instead
of 0. `getattr(..., 'total_tracks', 0) or 0` should handle it."""
r = SimpleNamespace(name="Album", artists=[], total_tracks=None)
score = _score_album_search_result(r, "Album", None, file_count=10)
assert score == pytest.approx(_ALBUM_NAME_WEIGHT, abs=1e-9)

232
tests/imports/test_auto_import_clear_completed_endpoint.py Normal file
View file

@ -0,0 +1,232 @@
"""Pin /api/auto-import/clear-completed behavior.
Reported case: Clear History button on the Import page left zombie
rows behind — every survivor showed "⧗ Processing" from 2-9 days ago.
Trace: `_record_in_progress` inserts a `status='processing'` row up-front
so the UI can render the in-flight import; `_finalize_result` updates
it to `completed`/`failed` when the import finishes. If the worker is
killed mid-import (server restart, crash), the row never gets finalized
and stays at `processing` forever. The endpoint's SQL delete-list
omitted `processing`, so zombies survived every click.
Fix added `processing` to the delete list, BUT guards against nuking
genuinely-live imports by intersecting against the worker's
`_snapshot_active()` map — any folder hash currently registered there
is excluded from the delete.
These tests pin:
- `processing` rows ARE swept (no longer zombies)
- Live `processing` rows (folder hash currently in `_active_imports`) survive
- `pending_review` survives (user still must approve/reject)
- `completed` / `approved` / `failed` / `needs_identification` /
`rejected` rows still get swept (unchanged contract)
- Count returned in the JSON response matches the actual delete count
- Empty active set falls through to the unparameterized DELETE
"""
from __future__ import annotations
import sqlite3
from contextlib import contextmanager
from unittest.mock import MagicMock
import pytest
# ---------------------------------------------------------------------------
# Fixtures
# ---------------------------------------------------------------------------
@pytest.fixture
def app_test_client():
import web_server
web_server.app.config['TESTING'] = True
with web_server.app.test_client() as client:
yield client
@pytest.fixture
def seeded_db(tmp_path):
"""Real sqlite DB with the auto_import_history table populated by
a mix of statuses + folder hashes. Returns a (connection_factory,
rows_seeded) tuple. The factory is a `_get_connection` lookalike
that returns the same connection so the endpoint sees the same data."""
db_path = str(tmp_path / 'test.db')
conn = sqlite3.connect(db_path, check_same_thread=False)
conn.row_factory = sqlite3.Row
cursor = conn.cursor()
cursor.execute("""
CREATE TABLE auto_import_history (
id INTEGER PRIMARY KEY AUTOINCREMENT,
folder_name TEXT NOT NULL,
folder_path TEXT NOT NULL,
folder_hash TEXT,
status TEXT NOT NULL DEFAULT 'scanning',
confidence REAL DEFAULT 0.0,
album_id TEXT,
album_name TEXT,
artist_name TEXT,
image_url TEXT,
total_files INTEGER DEFAULT 0,
matched_files INTEGER DEFAULT 0,
match_data TEXT,
identification_method TEXT,
error_message TEXT,
created_at TIMESTAMP DEFAULT CURRENT_TIMESTAMP,
updated_at TIMESTAMP DEFAULT CURRENT_TIMESTAMP,
processed_at TIMESTAMP
)
""")
seeds = [
# (folder_name, folder_hash, status)
('Album A', 'hash-completed-1', 'completed'),
('Album B', 'hash-approved-1', 'approved'),
('Album C', 'hash-failed-1', 'failed'),
('Album D', 'hash-needsid-1', 'needs_identification'),
('Album E', 'hash-rejected-1', 'rejected'),
('Zombie F', 'hash-zombie-1', 'processing'), # stale
('Zombie G', 'hash-zombie-2', 'processing'), # stale
('Live Import H', 'hash-LIVE', 'processing'), # active — must survive
('Awaiting Review I', 'hash-review-1', 'pending_review'), # must survive
]
for name, fh, status in seeds:
cursor.execute(
"INSERT INTO auto_import_history (folder_name, folder_path, folder_hash, status) "
"VALUES (?, ?, ?, ?)",
(name, f"/staging/{name}", fh, status),
)
conn.commit()
# Build a fake DB object whose `_get_connection` returns a context
# manager wrapping the live connection (matches the endpoint's
# `with db._get_connection() as conn` usage).
@contextmanager
def _conn_cm():
yield conn
fake_db = MagicMock()
fake_db._get_connection = _conn_cm
return fake_db, conn
def _statuses_remaining(conn):
cur = conn.cursor()
cur.execute("SELECT folder_name, status FROM auto_import_history ORDER BY id")
return [(row['folder_name'], row['status']) for row in cur.fetchall()]
# ---------------------------------------------------------------------------
# Tests
# ---------------------------------------------------------------------------
class TestClearCompletedEndpoint:
def test_sweeps_zombie_processing_rows_but_keeps_live(self, app_test_client, seeded_db, monkeypatch):
"""The bug: a `processing` row that's been there for days is a
zombie (server restart killed `_finalize_result`). Endpoint must
sweep it. But a `processing` row whose folder_hash is currently
registered in `_active_imports` is a LIVE import — must survive
or the UI loses its in-flight row mid-run."""
fake_db, conn = seeded_db
# Worker reports one live import — folder_hash hash-LIVE
fake_worker = MagicMock()
fake_worker._snapshot_active.return_value = [{'folder_hash': 'hash-LIVE'}]
monkeypatch.setattr('web_server.auto_import_worker', fake_worker)
monkeypatch.setattr('web_server.get_database', lambda: fake_db)
resp = app_test_client.post('/api/auto-import/clear-completed')
assert resp.status_code == 200
body = resp.get_json()
assert body['success'] is True
survivors = _statuses_remaining(conn)
names = {n for n, _ in survivors}
# Live import survives
assert 'Live Import H' in names, (
f"Live in-flight processing row was deleted; survivors={names}"
)
# Pending review survives — user still must approve/reject
assert 'Awaiting Review I' in names
# Both zombies swept
assert 'Zombie F' not in names
assert 'Zombie G' not in names
# Standard terminal-status rows swept
for completed_name in ('Album A', 'Album B', 'Album C', 'Album D', 'Album E'):
assert completed_name not in names, f"{completed_name} should have been deleted"
def test_count_in_response_matches_actual_deletes(self, app_test_client, seeded_db, monkeypatch):
"""JSON response carries the rowcount so the UI toast can show
accurate `Cleared N items`."""
fake_db, conn = seeded_db
fake_worker = MagicMock()
fake_worker._snapshot_active.return_value = [{'folder_hash': 'hash-LIVE'}]
monkeypatch.setattr('web_server.auto_import_worker', fake_worker)
monkeypatch.setattr('web_server.get_database', lambda: fake_db)
resp = app_test_client.post('/api/auto-import/clear-completed')
body = resp.get_json()
# 9 rows seeded; 7 deletable (5 terminal + 2 zombie processing);
# 2 survive (1 live, 1 pending_review)
assert body['count'] == 7, f"Expected 7 deletes; got {body['count']}"
cur = conn.cursor()
cur.execute("SELECT COUNT(*) FROM auto_import_history")
assert cur.fetchone()[0] == 2
def test_empty_active_set_takes_unparameterized_path(self, app_test_client, seeded_db, monkeypatch):
"""When no live imports are running, the SQL skips the `AND
folder_hash NOT IN (...)` clause. Pinned because an empty
`IN ()` is a SQL syntax error in sqlite — the branch matters."""
fake_db, conn = seeded_db
# Remove the live row so all `processing` rows are zombies
cur = conn.cursor()
cur.execute("DELETE FROM auto_import_history WHERE folder_hash = ?", ('hash-LIVE',))
conn.commit()
fake_worker = MagicMock()
fake_worker._snapshot_active.return_value = [] # nothing active
monkeypatch.setattr('web_server.auto_import_worker', fake_worker)
monkeypatch.setattr('web_server.get_database', lambda: fake_db)
resp = app_test_client.post('/api/auto-import/clear-completed')
assert resp.status_code == 200
body = resp.get_json()
assert body['success'] is True
# 5 terminal + 2 zombie processing = 7. Pending_review (1) survives.
assert body['count'] == 7
survivors = _statuses_remaining(conn)
assert len(survivors) == 1
assert survivors[0][1] == 'pending_review'
def test_worker_unavailable_returns_500(self, app_test_client, monkeypatch):
"""If the auto-import worker isn't initialised, the endpoint
bails early — no DB access, clear error. Pre-fix this branch
was already in place; pinning ensures the active-hash refactor
didn't accidentally start touching the worker before the guard."""
monkeypatch.setattr('web_server.auto_import_worker', None)
resp = app_test_client.post('/api/auto-import/clear-completed')
assert resp.status_code == 500
body = resp.get_json()
assert body['success'] is False
assert 'not available' in body['error'].lower()
def test_pending_review_always_survives(self, app_test_client, seeded_db, monkeypatch):
"""Specific pin for the deliberate `pending_review` exclusion.
Even when no imports are active and every other status is being
swept, `pending_review` rows must be left alone — user-action
required, not automatic cleanup."""
fake_db, conn = seeded_db
fake_worker = MagicMock()
fake_worker._snapshot_active.return_value = []
monkeypatch.setattr('web_server.auto_import_worker', fake_worker)
monkeypatch.setattr('web_server.get_database', lambda: fake_db)
app_test_client.post('/api/auto-import/clear-completed')
cur = conn.cursor()
cur.execute("SELECT COUNT(*) FROM auto_import_history WHERE status = 'pending_review'")
assert cur.fetchone()[0] == 1, (
"pending_review rows must never be swept by clear-completed"
)

1
tests/imports/test_auto_import_context_shape.py
View file

@ -480,6 +480,7 @@ def test_search_metadata_source_extracts_artist_id_from_dict_artist():
worker = AutoImportWorker(database=MagicMock(), process_callback=lambda *a, **k: None)
with patch("core.metadata_service.get_primary_source", return_value="spotify"), \
patch("core.metadata_service.get_source_priority", return_value=["spotify"]), \
patch("core.metadata_service.get_client_for_source", return_value=fake_client):
result = worker._search_metadata_source(
"Test Artist", "Test Album", "tags", candidate,

365
tests/imports/test_auto_import_multi_source_fallback.py Normal file
View file

@ -0,0 +1,365 @@
"""Pin auto-import multi-source fallback for album identification.
Discord report (mushy, paraphrased): 16 Bandcamp albums sat in staging
because auto-import couldn't identify them. Manual search at the
bottom of the Import Music tab found the same albums fine via Tidal
or Deezer — the user's primary metadata source (Spotify) just didn't
have them.
Root cause: `_search_metadata_source` only queried the primary source.
The manual `search_import_albums` path already iterates the full
`get_source_priority(get_primary_source())` chain and breaks on first
source that returns results. This brings auto-import to parity.
Fix semantics (option C — "primary first, fall through on weak"):
- Try primary source first
- Score result; if best ≥ 0.4 → return with that source
- Otherwise fall through to next source in priority order
- First source that produces a result above threshold wins
- Returns None only if ALL sources fail / score below threshold
Tests pin:
- Primary success path unchanged (returns primary result, no fallback fired)
- Primary returns nothing → fallback fires to next source
- Primary scores below threshold → fallback fires
- First fallback succeeds → no further sources queried
- All sources fail → None
- Per-source exception is contained (doesn't abort the chain)
- Result `source` field reflects WHICH source actually matched
- `identification_confidence` is the score from the winning source
"""
from __future__ import annotations
from types import SimpleNamespace
from unittest.mock import MagicMock, patch
import pytest
from core.auto_import_worker import AutoImportWorker, FolderCandidate
def _make_album(name: str, artist_name: str, total_tracks: int,
album_id: str = "alb-id", artist_id: str = "art-id"):
"""Build a fake album result matching `search_albums` return shape."""
return SimpleNamespace(
id=album_id,
name=name,
artists=[{"id": artist_id, "name": artist_name}],
total_tracks=total_tracks,
image_url="https://img.example/cover.jpg",
release_date="2024-01-01",
)
def _make_worker():
"""Bare AutoImportWorker bypassing __init__ side effects."""
return AutoImportWorker(database=MagicMock(), process_callback=lambda *a, **k: None)
def _make_candidate(file_count: int = 7, name: str = "TestAlbum"):
"""Folder candidate with N files (no actual disk reads)."""
return FolderCandidate(
path=f"/staging/{name}",
name=name,
audio_files=[f"/staging/{name}/{i:02d}.flac" for i in range(1, file_count + 1)],
)
# ---------------------------------------------------------------------------
# Primary success path — fallback never fires
# ---------------------------------------------------------------------------
class TestPrimarySuccess:
def test_primary_returns_strong_match_no_fallback(self):
"""Pre-fix behavior preserved: if primary scores above 0.4,
return its result and don't touch other sources."""
worker = _make_worker()
candidate = _make_candidate(file_count=10)
spotify_client = MagicMock()
spotify_client.search_albums.return_value = [
_make_album("Test Album", "Test Artist", total_tracks=10),
]
tidal_client = MagicMock() # should NEVER be called
def client_dispatch(source, **kwargs):
return {"spotify": spotify_client, "tidal": tidal_client}.get(source)
with patch("core.metadata_service.get_primary_source", return_value="spotify"), \
patch("core.metadata_service.get_source_priority",
return_value=["spotify", "tidal", "deezer"]), \
patch("core.metadata_service.get_client_for_source",
side_effect=client_dispatch):
result = worker._search_metadata_source(
"Test Artist", "Test Album", "tags", candidate,
)
assert result is not None
assert result["source"] == "spotify"
assert result["album_name"] == "Test Album"
spotify_client.search_albums.assert_called_once()
tidal_client.search_albums.assert_not_called()
# ---------------------------------------------------------------------------
# Primary fails — fallback fires
# ---------------------------------------------------------------------------
class TestFallbackOnNoResults:
def test_primary_empty_falls_through_to_next_source(self):
"""Reporter's exact case: Spotify doesn't have the Bandcamp
indie album. Tidal does. Auto-import must find it via Tidal."""
worker = _make_worker()
candidate = _make_candidate(file_count=7)
spotify_client = MagicMock()
spotify_client.search_albums.return_value = [] # not on Spotify
tidal_client = MagicMock()
tidal_client.search_albums.return_value = [
_make_album("Work in Progress", "Godly the Ruler", total_tracks=7,
album_id="tidal-alb-1", artist_id="tidal-art-1"),
]
def client_dispatch(source, **kwargs):
return {"spotify": spotify_client, "tidal": tidal_client}.get(source)
with patch("core.metadata_service.get_primary_source", return_value="spotify"), \
patch("core.metadata_service.get_source_priority",
return_value=["spotify", "tidal", "deezer"]), \
patch("core.metadata_service.get_client_for_source",
side_effect=client_dispatch):
result = worker._search_metadata_source(
"Godly the Ruler", "Work in Progress", "tags", candidate,
)
assert result is not None
assert result["source"] == "tidal", "Result must carry the source that actually matched"
assert result["album_id"] == "tidal-alb-1"
assert result["artist_id"] == "tidal-art-1"
spotify_client.search_albums.assert_called_once()
tidal_client.search_albums.assert_called_once()
class TestFallbackOnWeakScore:
def test_primary_below_threshold_falls_through(self):
"""Primary returns results but none score above 0.4 (e.g.
wrong-album false-matches). Fall through to next source for
a stronger match."""
worker = _make_worker()
candidate = _make_candidate(file_count=7)
spotify_client = MagicMock()
# Wrong album — name barely matches, no artist match, wrong track count
spotify_client.search_albums.return_value = [
_make_album("Different", "Wrong Artist", total_tracks=2),
]
deezer_client = MagicMock()
deezer_client.search_albums.return_value = [
_make_album("Work in Progress", "Godly the Ruler", total_tracks=7),
]
def client_dispatch(source, **kwargs):
return {"spotify": spotify_client, "deezer": deezer_client}.get(source)
with patch("core.metadata_service.get_primary_source", return_value="spotify"), \
patch("core.metadata_service.get_source_priority",
return_value=["spotify", "deezer"]), \
patch("core.metadata_service.get_client_for_source",
side_effect=client_dispatch):
result = worker._search_metadata_source(
"Godly the Ruler", "Work in Progress", "tags", candidate,
)
assert result is not None
assert result["source"] == "deezer"
assert result["album_name"] == "Work in Progress"
# Both clients called — primary returned weak, fallback picked up
spotify_client.search_albums.assert_called_once()
deezer_client.search_albums.assert_called_once()
# ---------------------------------------------------------------------------
# Chain semantics
# ---------------------------------------------------------------------------
class TestChainSemantics:
def test_first_fallback_success_stops_chain(self):
"""When fallback succeeds, no further sources are queried.
Don't waste API budget on Deezer if Tidal already gave us a
strong result."""
worker = _make_worker()
candidate = _make_candidate(file_count=10)
spotify_client = MagicMock()
spotify_client.search_albums.return_value = []
tidal_client = MagicMock()
tidal_client.search_albums.return_value = [
_make_album("Test", "Artist", total_tracks=10),
]
deezer_client = MagicMock() # should NEVER be called
def client_dispatch(source, **kwargs):
return {"spotify": spotify_client,
"tidal": tidal_client,
"deezer": deezer_client}.get(source)
with patch("core.metadata_service.get_primary_source", return_value="spotify"), \
patch("core.metadata_service.get_source_priority",
return_value=["spotify", "tidal", "deezer"]), \
patch("core.metadata_service.get_client_for_source",
side_effect=client_dispatch):
result = worker._search_metadata_source("Artist", "Test", "tags", candidate)
assert result is not None
assert result["source"] == "tidal"
deezer_client.search_albums.assert_not_called()
def test_all_sources_fail_returns_none(self):
"""If every source returns nothing or scores below threshold,
the whole search returns None (caller proceeds to next
identification strategy)."""
worker = _make_worker()
candidate = _make_candidate(file_count=7)
empty_client = MagicMock()
empty_client.search_albums.return_value = []
with patch("core.metadata_service.get_primary_source", return_value="spotify"), \
patch("core.metadata_service.get_source_priority",
return_value=["spotify", "tidal", "deezer"]), \
patch("core.metadata_service.get_client_for_source",
return_value=empty_client):
result = worker._search_metadata_source(
"Unknown Artist", "Nonexistent Album", "tags", candidate,
)
assert result is None
# All 3 sources got queried
assert empty_client.search_albums.call_count == 3
def test_per_source_exception_does_not_abort_chain(self):
"""If one source raises (rate limit, auth, transient HTTP),
the chain continues to the next source instead of aborting
the whole identification attempt."""
worker = _make_worker()
candidate = _make_candidate(file_count=10)
spotify_client = MagicMock()
spotify_client.search_albums.side_effect = RuntimeError("rate limit")
tidal_client = MagicMock()
tidal_client.search_albums.return_value = [
_make_album("Test", "Artist", total_tracks=10),
]
def client_dispatch(source, **kwargs):
return {"spotify": spotify_client, "tidal": tidal_client}.get(source)
with patch("core.metadata_service.get_primary_source", return_value="spotify"), \
patch("core.metadata_service.get_source_priority",
return_value=["spotify", "tidal"]), \
patch("core.metadata_service.get_client_for_source",
side_effect=client_dispatch):
result = worker._search_metadata_source("Artist", "Test", "tags", candidate)
assert result is not None
assert result["source"] == "tidal", (
"Primary exception must not block the fallback chain"
)
def test_unconfigured_source_skipped_gracefully(self):
"""If `get_client_for_source` returns None for a source
(user hasn't configured it), skip and continue."""
worker = _make_worker()
candidate = _make_candidate(file_count=10)
tidal_client = MagicMock()
tidal_client.search_albums.return_value = [
_make_album("Test", "Artist", total_tracks=10),
]
# Spotify returns None (no client configured); Tidal works
def client_dispatch(source, **kwargs):
if source == "spotify":
return None
return {"tidal": tidal_client}.get(source)
with patch("core.metadata_service.get_primary_source", return_value="spotify"), \
patch("core.metadata_service.get_source_priority",
return_value=["spotify", "tidal"]), \
patch("core.metadata_service.get_client_for_source",
side_effect=client_dispatch):
result = worker._search_metadata_source("Artist", "Test", "tags", candidate)
assert result is not None
assert result["source"] == "tidal"
# ---------------------------------------------------------------------------
# Result shape preservation
# ---------------------------------------------------------------------------
class TestResultShape:
def test_result_carries_correct_source_for_downstream_match(self):
"""`_match_tracks` reads `identification['source']` to know
which client to ask for the album's tracklist. Result MUST
carry the source that actually matched, not the primary
source name."""
worker = _make_worker()
candidate = _make_candidate(file_count=8)
spotify_client = MagicMock()
spotify_client.search_albums.return_value = []
deezer_client = MagicMock()
deezer_client.search_albums.return_value = [
_make_album("Test", "Artist", total_tracks=8, album_id="dz-123"),
]
def client_dispatch(source, **kwargs):
return {"spotify": spotify_client, "deezer": deezer_client}.get(source)
with patch("core.metadata_service.get_primary_source", return_value="spotify"), \
patch("core.metadata_service.get_source_priority",
return_value=["spotify", "deezer"]), \
patch("core.metadata_service.get_client_for_source",
side_effect=client_dispatch):
result = worker._search_metadata_source("Artist", "Test", "tags", candidate)
assert result["source"] == "deezer"
assert result["album_id"] == "dz-123", (
"Album ID must be the Deezer ID so _match_tracks queries "
"Deezer's get_album with the right ID format"
)
def test_identification_confidence_reflects_winning_source(self):
"""`identification_confidence` is used in the overall-confidence
formula and the 0.9 / 0.7 cascade thresholds. It must be the
score from the source that actually matched."""
worker = _make_worker()
candidate = _make_candidate(file_count=10)
spotify_client = MagicMock()
spotify_client.search_albums.return_value = []
# Perfect match on Tidal — all 3 weights at max → score = 1.0
tidal_client = MagicMock()
tidal_client.search_albums.return_value = [
_make_album("Test Album", "Test Artist", total_tracks=10),
]
def client_dispatch(source, **kwargs):
return {"spotify": spotify_client, "tidal": tidal_client}.get(source)
with patch("core.metadata_service.get_primary_source", return_value="spotify"), \
patch("core.metadata_service.get_source_priority",
return_value=["spotify", "tidal"]), \
patch("core.metadata_service.get_client_for_source",
side_effect=client_dispatch):
result = worker._search_metadata_source(
"Test Artist", "Test Album", "tags", candidate,
)
assert result["identification_confidence"] == pytest.approx(1.0, abs=0.01)

136
tests/library/test_path_resolver.py
View file

@ -403,3 +403,139 @@ def test_docker_resolve_path_pass_through_outside_docker(monkeypatch) -> None:
monkeypatch.setattr(os.path, "exists",
lambda p: False if p == "/.dockerenv" else real_exists(p))
assert path_resolver._docker_resolve_path("H:\\Music\\track.flac") == "H:\\Music\\track.flac"
# ---------------------------------------------------------------------------
# Diagnostic helper — issue #558 (gabistek, Navidrome on Docker)
# ---------------------------------------------------------------------------
#
# `resolve_library_file_path_with_diagnostic` returns
# `(resolved, ResolveAttempt)` so callers can render a useful error
# instead of a silent None. Pre-fix the Album Completeness "Auto-Fill"
# button surfaced "Could not determine album folder from existing
# tracks" with no diagnostic, leaving Navidrome users (whose Subsonic
# API doesn't expose library paths the way Plex's does) with no signal
# about what to configure.
from core.library.path_resolver import ( # noqa: E402
ResolveAttempt,
resolve_library_file_path_with_diagnostic,
)
class TestResolveAttemptShape:
def test_returns_tuple_of_path_and_attempt(self, tmp_path: Path) -> None:
real = tmp_path / "track.flac"
real.write_bytes(b"a")
result = resolve_library_file_path_with_diagnostic(str(real))
assert isinstance(result, tuple)
assert len(result) == 2
path, attempt = result
assert path == str(real)
assert isinstance(attempt, ResolveAttempt)
def test_raw_path_existed_true_when_short_circuit(self, tmp_path: Path) -> None:
"""Happy path → resolver short-circuits at the first
`os.path.exists` check; `base_dirs_tried` stays empty."""
real = tmp_path / "track.flac"
real.write_bytes(b"a")
_, attempt = resolve_library_file_path_with_diagnostic(str(real))
assert attempt.raw_path_existed is True
assert attempt.base_dirs_tried == []
def test_raw_path_existed_false_when_walking(self, tmp_path: Path) -> None:
"""When the raw path doesn't exist but the suffix-walk finds it,
the attempt should report `raw_path_existed=False` and list the
base dir that succeeded among `base_dirs_tried`."""
# Create the file under a real base dir at a different parent
base = tmp_path / "library"
target = base / "Artist" / "Album" / "track.flac"
target.parent.mkdir(parents=True)
target.write_bytes(b"a")
# DB stores it as if scanned at /music/Artist/Album/track.flac
db_path = "/music/Artist/Album/track.flac"
path, attempt = resolve_library_file_path_with_diagnostic(
db_path, transfer_folder=str(base),
)
assert path == str(target), f"suffix-walk should have found the file under {base}"
assert attempt.raw_path_existed is False
assert str(base) in attempt.base_dirs_tried
class TestDiagnosticForFailedResolves:
def test_no_base_dirs_returns_none_with_empty_attempt(self) -> None:
"""No transfer/download/config/plex → resolver can't probe.
Diagnostic must report empty `base_dirs_tried` so the caller can
render a "no probe sources configured" hint."""
path, attempt = resolve_library_file_path_with_diagnostic(
"/music/Artist/Album/track.flac",
)
assert path is None
assert attempt.raw_path_existed is False
assert attempt.base_dirs_tried == []
assert attempt.had_config_manager is False
assert attempt.had_plex_client is False
def test_base_dirs_listed_even_when_walk_fails(self, tmp_path: Path) -> None:
"""When base dirs exist but the suffix-walk doesn't find the
file, `base_dirs_tried` must still report what was probed.
Lets the caller surface "we tried X, Y, Z" in the error."""
base = tmp_path / "transfer"
base.mkdir()
# Don't create the target file — the walk will fail
path, attempt = resolve_library_file_path_with_diagnostic(
"/music/Artist/Album/missing.flac",
transfer_folder=str(base),
)
assert path is None
assert str(base) in attempt.base_dirs_tried
def test_had_flags_track_caller_inputs(self, tmp_path: Path) -> None:
"""`had_config_manager` / `had_plex_client` reflect what the
caller passed in — useful for distinguishing 'caller didn't
wire up the optional input' from 'optional input was wired up
but produced no usable base dirs'."""
config = MagicMock()
config.get.return_value = "" # no config-driven paths
plex = SimpleNamespace(server=None, music_library=None)
path, attempt = resolve_library_file_path_with_diagnostic(
"/music/Artist/track.flac",
config_manager=config,
plex_client=plex,
)
assert path is None
assert attempt.had_config_manager is True
assert attempt.had_plex_client is True
class TestBackwardsCompat:
def test_existing_resolve_function_delegates_to_diagnostic(self, tmp_path: Path) -> None:
"""The non-diagnostic `resolve_library_file_path` is now a thin
wrapper that drops the attempt. Pin that the legacy signature
still returns the same path values across all the cases the
old function covered, so existing callers don't see drift."""
real = tmp_path / "track.flac"
real.write_bytes(b"a")
# Happy path
assert resolve_library_file_path(str(real)) == str(real)
# Suffix-walk path
base = tmp_path / "lib"
target = base / "Artist" / "Album" / "track.flac"
target.parent.mkdir(parents=True)
target.write_bytes(b"a")
result = resolve_library_file_path(
"/music/Artist/Album/track.flac",
transfer_folder=str(base),
)
assert result == str(target)
# Failure path
assert resolve_library_file_path(
"/music/Artist/missing.flac",
transfer_folder=str(base),
) is None

322
tests/metadata/test_discography_filters.py Normal file
View file

@ -0,0 +1,322 @@
"""Pin track-level filters used by the Download Discography endpoint.
GitHub issue #559 (trackhacs): "Download Discography" on an artist
pulled in tracks where the artist's name appeared in the title of
someone else's song. Two failure modes:
1. Cross-artist tracks — compilations / appears_on albums brought in
tracks by unrelated artists. Fixed by `track_artist_matches`.
2. Remix / live / acoustic / instrumental versions never honored the
watchlist content-type filters for one-off discography downloads.
Fixed by `content_type_skip_reason`.
These helpers live in ``core.metadata.discography_filters``. Tests pin
behavior at the function boundary so the wiring inside
``web_server.download_discography`` doesn't need an endpoint test to
catch a filter regression.
"""
from __future__ import annotations
from types import SimpleNamespace
import pytest
from core.metadata.discography_filters import (
content_type_skip_reason,
load_global_content_filter_settings,
track_already_owned,
track_artist_matches,
)
# ---------------------------------------------------------------------------
# track_artist_matches
# ---------------------------------------------------------------------------
class TestTrackArtistMatches:
def test_primary_artist_matches(self):
"""When the requested artist is the track's primary artist,
match. This is the most common case — non-feature tracks on an
artist's own album."""
assert track_artist_matches(['Drake', 'Future'], 'Drake') is True
def test_featured_artist_matches(self):
"""When the requested artist appears as a feature (anywhere in
the list, not just position 0), still match. Keeping feature
appearances is intentional — they're legit discography entries."""
assert track_artist_matches(['Lil Wayne', 'Drake', 'Kanye West'], 'Drake') is True
def test_unrelated_artist_drops(self):
"""The bug case: a compilation track by an unrelated artist
that just mentions the requested artist in the title. The
artists list contains only the actual performer(s); filter
drops it."""
assert track_artist_matches(['Random Artist'], 'Drake') is False
def test_match_is_case_insensitive(self):
"""Source data can be cased inconsistently across providers."""
assert track_artist_matches(['drake'], 'Drake') is True
assert track_artist_matches(['DRAKE'], 'Drake') is True
assert track_artist_matches(['Drake'], 'drake') is True
def test_match_handles_whitespace_padding(self):
"""Trailing whitespace in either side mustn't break the match."""
assert track_artist_matches([' Drake '], 'Drake') is True
assert track_artist_matches(['Drake'], ' Drake ') is True
def test_empty_artists_list_drops(self):
"""No artists on the track → can't be by anyone → drop."""
assert track_artist_matches([], 'Drake') is False
assert track_artist_matches(None, 'Drake') is False
def test_empty_requested_artist_keeps(self):
"""Defensive: if the caller forgot to pass the requested artist,
don't drop every track — let the caller's other filters decide.
Better to keep too much than to silently drop everything."""
assert track_artist_matches(['Drake'], '') is True
assert track_artist_matches(['Drake'], ' ') is True
assert track_artist_matches(['Drake'], None) is True
def test_accepts_list_of_dicts_shape(self):
"""Some upstreams pass `[{'name': 'Drake', 'id': '...'}]`
directly instead of the normalized list-of-strings. Helper
must handle both — easier than forcing a normalization step
at the call site."""
assert track_artist_matches([{'name': 'Drake'}], 'Drake') is True
assert track_artist_matches([{'name': 'Random'}], 'Drake') is False
def test_substring_does_not_match(self):
"""A song by "Drake & Future" should not match "Drake" via
substring — that's exactly the false-positive case the bug
report describes. Exact full-name match only."""
assert track_artist_matches(['Drake & Future'], 'Drake') is False
assert track_artist_matches(['Drakeo the Ruler'], 'Drake') is False
# ---------------------------------------------------------------------------
# content_type_skip_reason
# ---------------------------------------------------------------------------
_ALL_OFF = {
'include_live': False,
'include_remixes': False,
'include_acoustic': False,
'include_instrumentals': False,
}
_ALL_ON = {
'include_live': True,
'include_remixes': True,
'include_acoustic': True,
'include_instrumentals': True,
}
class TestContentTypeSkipReason:
def test_returns_none_for_plain_track(self):
"""Default settings exclude all four content types, but a plain
original studio track shouldn't trigger any of them."""
assert content_type_skip_reason('Hotline Bling', 'Views', _ALL_OFF) is None
def test_remix_skipped_when_excluded(self):
"""Default: remixes off → "(Remix)" track gets skipped with
reason 'remix'."""
assert content_type_skip_reason('Hotline Bling (Remix)', 'Views', _ALL_OFF) == 'remix'
def test_remix_kept_when_included(self):
"""When the user opts in via include_remixes, the same track
passes through."""
assert content_type_skip_reason('Hotline Bling (Remix)', 'Views', _ALL_ON) is None
def test_live_skipped_when_excluded(self):
assert content_type_skip_reason('Hotline Bling (Live)', 'Views', _ALL_OFF) == 'live'
def test_acoustic_skipped_when_excluded(self):
assert content_type_skip_reason('Hotline Bling (Acoustic)', 'Views', _ALL_OFF) == 'acoustic'
def test_instrumental_skipped_when_excluded(self):
assert content_type_skip_reason('Hotline Bling (Instrumental)', 'Views', _ALL_OFF) == 'instrumental'
def test_first_match_wins(self):
"""If a track somehow matches multiple categories (e.g. a live
remix), it's reported under the first one checked. Order is
live → remix → acoustic → instrumental. Stable for telemetry
and for the user-facing skip-counter aggregation."""
# "Live Remix" — both live and remix patterns fire. Live first.
reason = content_type_skip_reason('Hotline Bling (Live Remix)', 'Views', _ALL_OFF)
assert reason == 'live'
def test_settings_missing_keys_default_to_exclude(self):
"""Defensive: caller passes an empty dict / partial dict.
Missing keys treated as False (exclude) — same as the watchlist
scanner contract. A remix passed with `{}` still gets skipped."""
assert content_type_skip_reason('Track (Remix)', 'Album', {}) == 'remix'
# ---------------------------------------------------------------------------
# load_global_content_filter_settings
# ---------------------------------------------------------------------------
class TestLoadGlobalSettings:
def test_reads_all_four_settings(self):
cfg = SimpleNamespace()
cfg.get = lambda key, default=None: {
'watchlist.global_include_live': True,
'watchlist.global_include_remixes': False,
'watchlist.global_include_acoustic': True,
'watchlist.global_include_instrumentals': False,
}.get(key, default)
result = load_global_content_filter_settings(cfg)
assert result == {
'include_live': True,
'include_remixes': False,
'include_acoustic': True,
'include_instrumentals': False,
}
def test_defaults_all_false_when_config_manager_missing(self):
"""No config_manager → all four default to False (exclude).
Same defaults the watchlist scanner uses for unconfigured artists."""
result = load_global_content_filter_settings(None)
assert result == {
'include_live': False,
'include_remixes': False,
'include_acoustic': False,
'include_instrumentals': False,
}
def test_config_get_raising_falls_back_to_defaults(self):
"""Defensive: if `config_manager.get` raises (corrupted config,
backend offline, etc.), helper returns all-False defaults
rather than crashing the discography fetch."""
cfg = SimpleNamespace()
def _boom(*_a, **_k):
raise RuntimeError('config backend exploded')
cfg.get = _boom
result = load_global_content_filter_settings(cfg)
assert result['include_live'] is False
assert result['include_remixes'] is False
def test_setting_values_coerced_to_bool(self):
"""Config can store as int / string — coerce defensively so
downstream callers can rely on the bool contract."""
cfg = SimpleNamespace()
cfg.get = lambda key, default=None: {
'watchlist.global_include_live': 1, # int truthy
'watchlist.global_include_remixes': '', # empty string falsy
'watchlist.global_include_acoustic': 'on', # string truthy
'watchlist.global_include_instrumentals': 0,
}.get(key, default)
result = load_global_content_filter_settings(cfg)
assert result['include_live'] is True
assert result['include_remixes'] is False
assert result['include_acoustic'] is True
assert result['include_instrumentals'] is False
# ---------------------------------------------------------------------------
# track_already_owned
# ---------------------------------------------------------------------------
class _FakeDB:
"""Minimal stub for the parts of MusicDatabase the helper touches."""
def __init__(self, response):
self._response = response
self.calls = []
def check_track_exists(self, title, artist, **kwargs):
self.calls.append({'title': title, 'artist': artist, **kwargs})
if isinstance(self._response, Exception):
raise self._response
return self._response
class TestTrackAlreadyOwned:
def test_returns_true_when_match_clears_threshold(self):
"""Skowl's case: the second discography click finds the track
already in library at confidence ≥ 0.7 → skip."""
db = _FakeDB((object(), 0.85))
assert track_already_owned(
db, 'Hotline Bling', 'Drake', 'Views', 'plex',
) is True
def test_returns_false_when_no_match(self):
"""Library doesn't have it → return False so the caller queues
the track. (None track returned with confidence 0.0.)"""
db = _FakeDB((None, 0.0))
assert track_already_owned(
db, 'New Song', 'Drake', 'New Album', 'plex',
) is False
def test_returns_false_when_match_below_threshold(self):
"""A weak fuzzy match shouldn't count — better to over-queue
than to silently drop a real missing track. Mirrors the
backfill repair job's `if db_track and confidence >= 0.7` guard."""
db = _FakeDB((object(), 0.5)) # below default 0.7
assert track_already_owned(
db, 'Sort Of Like Hotline Bling', 'Drake', 'Views', 'plex',
) is False
def test_passes_album_to_check(self):
"""Album param is what enables album-aware matching for
multi-artist albums in `check_track_exists`. Pin it gets through."""
db = _FakeDB((object(), 0.9))
track_already_owned(db, 'Track', 'Artist', 'Album X', 'plex')
assert db.calls[0]['album'] == 'Album X'
def test_passes_server_source_to_check(self):
"""Active media server scopes the lookup so the skip check
only fires on tracks the user can actually see in their
library through their currently-active server."""
db = _FakeDB((object(), 0.9))
track_already_owned(db, 'Track', 'Artist', 'Album', 'navidrome')
assert db.calls[0]['server_source'] == 'navidrome'
def test_empty_album_passed_as_none(self):
"""Empty-string album becomes None so check_track_exists's
album-aware fallback doesn't try to match against ''."""
db = _FakeDB((None, 0.0))
track_already_owned(db, 'Track', 'Artist', '', 'plex')
assert db.calls[0]['album'] is None
def test_missing_track_or_artist_returns_false_without_calling_db(self):
"""Don't fire a DB call when we have nothing to match against —
defensive AND avoids polluting query logs with empty lookups."""
db = _FakeDB((object(), 0.9))
assert track_already_owned(db, '', 'Artist', 'Album', 'plex') is False
assert track_already_owned(db, 'Track', '', 'Album', 'plex') is False
assert track_already_owned(db, '', '', 'Album', 'plex') is False
assert db.calls == [], "DB must not be called when track or artist is empty"
def test_db_exception_returns_false(self):
"""If the DB call raises (lock contention, schema mismatch,
whatever), treat as 'not owned' and let the caller queue.
A redundant wishlist add is much cheaper to recover from
than a missed track."""
db = _FakeDB(RuntimeError('db locked'))
assert track_already_owned(db, 'Track', 'Artist', 'Album', 'plex') is False
def test_custom_confidence_threshold_honored(self):
"""Caller can tighten or loosen the threshold. 0.95 means only
very-high-confidence matches count as owned."""
db = _FakeDB((object(), 0.8))
# Default threshold (0.7): match counts
assert track_already_owned(db, 'Track', 'Artist', 'Album', 'plex') is True
# Tighter threshold (0.95): same match doesn't count
assert track_already_owned(
db, 'Track', 'Artist', 'Album', 'plex',
confidence_threshold=0.95,
) is False
def test_none_server_source_passes_through(self):
"""When the caller can't determine the active server, pass
None — `check_track_exists` falls back to a cross-server search."""
db = _FakeDB((None, 0.0))
track_already_owned(db, 'Track', 'Artist', 'Album', None)
assert db.calls[0]['server_source'] is None

470
tests/metadata/test_multi_artist_tag_settings.py Normal file
View file

@ -0,0 +1,470 @@
"""Pin multi-artist tag-write settings (issue: 'Multi artists settings not working').
Three settings under `metadata_enhancement.tags`:
- `write_multi_artist` (bool) — write a separate multi-value tag
listing every artist (TXXX:Artists for ID3, "artists" key for
Vorbis). Picard convention.
- `artist_separator` (string, default ", ") — delimiter used to
join multiple artists into the single ARTIST/TPE1 string.
- `feat_in_title` (bool) — when true, ARTIST/TPE1 carries ONLY
the primary artist; featured artists get pulled out and
appended to the title as " (feat. X, Y)".
Reporter (Netti93): all three were partially or completely
unimplemented.
- Bug 1: `_artists_list` field read by enrichment.py was never
populated by source.py → multi-value writes silently no-op'd.
- Bug 2: `artist_separator` referenced in UI but ZERO Python code
read it → always hardcoded ", ".
- Bug 3: `feat_in_title` referenced in UI but ZERO Python code
read it → no implementation at all.
These tests pin the fixed `extract_source_metadata` behavior:
- `_artists_list` populated whenever search response has multiple artists
- `artist_separator` config drives the join character for ARTIST string
- `feat_in_title` pulls featured artists into title, leaves only
primary in ARTIST string
- Title-already-has-feat case isn't double-appended
- Single-artist case unaffected by either setting
"""
from __future__ import annotations
from types import SimpleNamespace
from unittest.mock import MagicMock, patch
import pytest
def _make_cfg(overrides=None):
"""Stub config_manager. Defaults match the unset-config case
so each test can selectively override."""
overrides = overrides or {}
defaults = {
"metadata_enhancement.enabled": True,
"metadata_enhancement.tags.write_multi_artist": False,
"metadata_enhancement.tags.feat_in_title": False,
"metadata_enhancement.tags.artist_separator": ", ",
}
full = {**defaults, **overrides}
cfg = MagicMock()
cfg.get.side_effect = lambda key, default=None: full.get(key, default)
return cfg
def _build_context(artists_list):
"""Minimal context dict matching what extract_source_metadata reads."""
return {
"original_search_result": {
"title": "Sample Track",
"artists": [{"name": a} for a in artists_list],
},
"source": "spotify",
}
def _call_extract(artists_list, cfg_overrides=None):
"""Helper: patches config + calls extract_source_metadata, returns the
metadata dict. Avoids the broader source-specific embedding loop by
only using fields the multi-artist branch touches."""
from core.metadata import source as src_module
context = _build_context(artists_list)
artist_dict = {"name": artists_list[0] if artists_list else ""}
album_info = {"album_name": "Sample Album"}
with patch.object(src_module, "get_config_manager", return_value=_make_cfg(cfg_overrides)):
return src_module.extract_source_metadata(context, artist_dict, album_info)
# ---------------------------------------------------------------------------
# Bug 1: `_artists_list` populated
# ---------------------------------------------------------------------------
class TestArtistsListPopulated:
def test_multiple_artists_populate_list(self):
"""Reporter's first bug — `_artists_list` field was always
empty. Verify it now contains every artist from the search
response."""
meta = _call_extract(["Eminem", "Dr. Dre", "50 Cent"])
assert meta.get("_artists_list") == ["Eminem", "Dr. Dre", "50 Cent"]
def test_single_artist_still_populates_list(self):
"""Edge: even single-artist case populates the list (length 1).
Avoids special-casing downstream — `len(_artists_list) > 1`
check in enrichment.py is the gate."""
meta = _call_extract(["Solo Artist"])
assert meta.get("_artists_list") == ["Solo Artist"]
def test_no_artists_falls_through(self):
"""When search response has no artists list, falls through to
the single-artist branch — no `_artists_list` written."""
from core.metadata import source as src_module
context = {
"original_search_result": {"title": "T", "artists": None},
"source": "spotify",
}
with patch.object(src_module, "get_config_manager", return_value=_make_cfg()):
meta = src_module.extract_source_metadata(context, {"name": "X"}, {})
assert "_artists_list" not in meta or meta.get("_artists_list") in (None, [])
# ---------------------------------------------------------------------------
# Bug 2: artist_separator drives ARTIST string
# ---------------------------------------------------------------------------
class TestArtistSeparator:
def test_default_separator_is_comma_space(self):
"""Default preserves historical behavior — joining with ', '
so users who haven't set the config see no behavior change."""
meta = _call_extract(["A", "B", "C"])
assert meta["artist"] == "A, B, C"
def test_semicolon_separator(self):
"""Reporter's exact case: artist_separator=';'. Picard convention."""
meta = _call_extract(["A", "B", "C"], cfg_overrides={
"metadata_enhancement.tags.artist_separator": ";",
})
assert meta["artist"] == "A;B;C"
def test_separator_with_space(self):
"""Many users prefer '; ' (semi + space). Whatever string
the user puts in the config gets used verbatim — no trimming."""
meta = _call_extract(["A", "B"], cfg_overrides={
"metadata_enhancement.tags.artist_separator": "; ",
})
assert meta["artist"] == "A; B"
def test_separator_unused_for_single_artist(self):
"""Single-artist case: separator irrelevant, ARTIST is just
the one name. No spurious trailing/leading separator."""
meta = _call_extract(["Solo"], cfg_overrides={
"metadata_enhancement.tags.artist_separator": ";",
})
assert meta["artist"] == "Solo"
# ---------------------------------------------------------------------------
# Bug 3: feat_in_title — pull featured into title
# ---------------------------------------------------------------------------
class TestFeatInTitle:
def test_feat_in_title_pulls_featured_to_title(self):
"""Reporter's third bug. With feat_in_title=true, ARTIST holds
only primary; title gets " (feat. ...)" appended for
all-but-first."""
meta = _call_extract(["Eminem", "Dr. Dre", "50 Cent"], cfg_overrides={
"metadata_enhancement.tags.feat_in_title": True,
})
assert meta["artist"] == "Eminem"
assert "(feat. Dr. Dre, 50 Cent)" in meta["title"]
def test_feat_in_title_off_uses_separator(self):
"""When feat_in_title is off (default), all artists join the
ARTIST string per `artist_separator`. Title stays unchanged."""
meta = _call_extract(["A", "B", "C"], cfg_overrides={
"metadata_enhancement.tags.feat_in_title": False,
"metadata_enhancement.tags.artist_separator": " & ",
})
assert meta["artist"] == "A & B & C"
assert "feat" not in meta["title"].lower()
def test_feat_in_title_skips_when_only_one_artist(self):
"""Single-artist case: feat_in_title is a no-op. ARTIST = the
single name, title untouched."""
meta = _call_extract(["Solo"], cfg_overrides={
"metadata_enhancement.tags.feat_in_title": True,
})
assert meta["artist"] == "Solo"
assert "feat" not in meta["title"].lower()
def test_feat_in_title_no_double_append_when_title_already_has_feat(self):
"""Defensive: if the source title already includes 'feat.' or
'(ft.', don't append again. Common on remixes / collabs where
the platform stores the featured artist in the track name."""
from core.metadata import source as src_module
context = {
"original_search_result": {
"title": "Track (feat. Already Listed)",
"artists": [{"name": "Primary"}, {"name": "Featured"}],
},
"source": "spotify",
}
cfg_overrides = {"metadata_enhancement.tags.feat_in_title": True}
with patch.object(src_module, "get_config_manager", return_value=_make_cfg(cfg_overrides)):
meta = src_module.extract_source_metadata(context, {"name": "Primary"}, {})
# Primary still pulled out of ARTIST...
assert meta["artist"] == "Primary"
# ...but title NOT double-appended (would be "(feat. X) (feat. Y)")
assert meta["title"].count("feat.") == 1
@pytest.mark.parametrize("source_title", [
"Track (feat. X)", # standard parens + period
"Track (Feat. X)", # capitalized
"Track (FEAT X)", # all caps, no period
"Track (feat X)", # no period, parens
"Track (Featuring X)", # full word
"Track [feat. X]", # square brackets
"Track ft. X", # ft + period, no parens/brackets
"Track (ft X)", # ft no period, parens
"Track FT. X", # FT all caps
])
def test_double_append_guard_recognizes_feat_variants(self, source_title):
"""Defensive: source platforms (spotify / tidal / deezer) use
wildly different title conventions for featured artists. Guard
must recognize all of them so we never double-append."""
from core.metadata import source as src_module
context = {
"original_search_result": {
"title": source_title,
"artists": [{"name": "Primary"}, {"name": "Featured"}],
},
"source": "spotify",
}
cfg_overrides = {"metadata_enhancement.tags.feat_in_title": True}
with patch.object(src_module, "get_config_manager", return_value=_make_cfg(cfg_overrides)):
meta = src_module.extract_source_metadata(context, {"name": "Primary"}, {})
# Title left unchanged — no double-append for any variant
assert meta["title"] == source_title, (
f"Variant {source_title!r} got double-appended → {meta['title']!r}"
)
def test_double_append_guard_does_NOT_falsely_match_substrings(self):
"""Sanity: word-boundary regex must NOT match 'ft' or 'feat'
as part of bigger words like 'aftermath', 'shaft', 'feature'.
Otherwise titles containing those words would skip the
legitimate (feat. X) append."""
from core.metadata import source as src_module
context = {
"original_search_result": {
"title": "Aftermath", # contains 'ft' as substring
"artists": [{"name": "Primary"}, {"name": "Featured"}],
},
"source": "spotify",
}
cfg_overrides = {"metadata_enhancement.tags.feat_in_title": True}
with patch.object(src_module, "get_config_manager", return_value=_make_cfg(cfg_overrides)):
meta = src_module.extract_source_metadata(context, {"name": "Primary"}, {})
# Should APPEND because 'ft' inside 'Aftermath' isn't a
# standalone "ft" feature marker
assert "(feat. Featured)" in meta["title"]
# ---------------------------------------------------------------------------
# Integration — settings combine correctly
# ---------------------------------------------------------------------------
class TestSettingsCombination:
def test_feat_in_title_overrides_separator_for_artist_string(self):
"""When BOTH settings are on, feat_in_title wins for the
ARTIST string (primary only). Separator is irrelevant in
that branch but `_artists_list` still carries every artist
for the multi-value tag write."""
meta = _call_extract(["A", "B", "C"], cfg_overrides={
"metadata_enhancement.tags.feat_in_title": True,
"metadata_enhancement.tags.artist_separator": ";",
})
assert meta["artist"] == "A"
assert "(feat. B, C)" in meta["title"]
# Multi-value list still complete — write_multi_artist would
# use this regardless of feat_in_title.
assert meta["_artists_list"] == ["A", "B", "C"]
def test_all_three_off_default_behavior_preserved(self):
"""Sanity: unset config → joined ARTIST, no title change,
list still populated. Picks up no behavior change for users
who haven't touched the settings."""
meta = _call_extract(["A", "B"])
assert meta["artist"] == "A, B"
assert meta["title"] == "Sample Track"
assert meta["_artists_list"] == ["A", "B"]
# ---------------------------------------------------------------------------
# Deezer-specific: upgrade single-artist search results via /track/<id>
# ---------------------------------------------------------------------------
#
# Deezer's `/search` endpoint only returns the primary artist for each
# track. The full contributors array (feat., remix collaborators,
# producers credited as artists) lives on `/track/<id>`. Reporter said
# their Retag flow worked because it called the per-track endpoint, but
# the initial enrichment used search-result data and missed the
# contributors. The fix: when source==deezer AND search returned only
# one artist AND a track_id is available, fetch the full track details
# and upgrade the artists list.
class TestDeezerContributorsUpgrade:
def test_upgrades_when_deezer_search_returns_single_artist(self):
"""Reporter's exact case: Deezer track with multiple
contributors, search returns just the primary, /track/<id>
returns all 3. Upgrade path fetches the full set."""
from core.metadata import source as src_module
context = {
"original_search_result": {
"title": "Collab Track",
"artists": [{"name": "Primary"}], # Only one — search-response shape
},
"source": "deezer",
}
# track_id resolved via original_search_result.id by get_import_source_ids
context["original_search_result"]["id"] = "12345"
fake_deezer = SimpleNamespace(get_track_details=MagicMock(return_value={
"id": "12345",
"name": "Collab Track",
"artists": ["Primary", "Featured1", "Featured2"], # Full contributors
}))
cfg_overrides = {"metadata_enhancement.tags.artist_separator": "; "}
with patch.object(src_module, "get_config_manager", return_value=_make_cfg(cfg_overrides)), \
patch("core.metadata.get_deezer_client", return_value=fake_deezer):
meta = src_module.extract_source_metadata(context, {"name": "Primary"}, {})
# Upgraded list reaches the multi-value tag
assert meta["_artists_list"] == ["Primary", "Featured1", "Featured2"]
# And the joined ARTIST string respects the separator
assert meta["artist"] == "Primary; Featured1; Featured2"
fake_deezer.get_track_details.assert_called_once_with("12345")
def test_no_upgrade_when_search_already_returned_multiple(self):
"""When search already has multiple artists, skip the upgrade —
no extra API call needed."""
from core.metadata import source as src_module
context = {
"original_search_result": {
"title": "T",
"artists": [{"name": "A"}, {"name": "B"}], # Already multi
},
"source": "deezer",
}
# track_id resolved via original_search_result.id by get_import_source_ids
context["original_search_result"]["id"] = "12345"
fake_deezer = SimpleNamespace(get_track_details=MagicMock())
with patch.object(src_module, "get_config_manager", return_value=_make_cfg()), \
patch("core.metadata.get_deezer_client", return_value=fake_deezer):
meta = src_module.extract_source_metadata(context, {"name": "A"}, {})
assert meta["_artists_list"] == ["A", "B"]
# No upgrade call — search already had what we needed
fake_deezer.get_track_details.assert_not_called()
def test_no_upgrade_for_non_deezer_sources(self):
"""Spotify/iTunes/Tidal already return multi-artist in search,
so the Deezer-specific upgrade path must NOT fire for them.
Otherwise we'd be making redundant API calls."""
from core.metadata import source as src_module
context = {
"original_search_result": {
"title": "T",
"artists": [{"name": "A"}],
},
"source": "spotify",
"source_track_id": "12345",
}
fake_deezer = SimpleNamespace(get_track_details=MagicMock())
with patch.object(src_module, "get_config_manager", return_value=_make_cfg()), \
patch("core.metadata.get_deezer_client", return_value=fake_deezer):
meta = src_module.extract_source_metadata(context, {"name": "A"}, {})
# Single artist preserved, no Deezer upgrade attempted
assert meta["_artists_list"] == ["A"]
fake_deezer.get_track_details.assert_not_called()
def test_upgrade_failure_falls_through_to_search_result(self):
"""Defensive: if /track/<id> fails (network error, deezer
client unavailable), fall through to the search-result list.
Don't lose the single-artist data we already had."""
from core.metadata import source as src_module
context = {
"original_search_result": {
"title": "T",
"artists": [{"name": "Primary"}],
},
"source": "deezer",
}
# track_id resolved via original_search_result.id by get_import_source_ids
context["original_search_result"]["id"] = "12345"
fake_deezer = SimpleNamespace(get_track_details=MagicMock(
side_effect=RuntimeError("network down"),
))
with patch.object(src_module, "get_config_manager", return_value=_make_cfg()), \
patch("core.metadata.get_deezer_client", return_value=fake_deezer):
meta = src_module.extract_source_metadata(context, {"name": "Primary"}, {})
# Search-result list preserved
assert meta["_artists_list"] == ["Primary"]
assert meta["artist"] == "Primary"
def test_upgrade_returns_same_count_no_change(self):
"""Edge: /track/<id> returns the same single artist (track
genuinely has one artist on Deezer too). Should preserve the
list without false-positive upgrade."""
from core.metadata import source as src_module
context = {
"original_search_result": {
"title": "T",
"artists": [{"name": "Solo"}],
},
"source": "deezer",
}
# track_id resolved via original_search_result.id by get_import_source_ids
context["original_search_result"]["id"] = "12345"
fake_deezer = SimpleNamespace(get_track_details=MagicMock(return_value={
"id": "12345",
"artists": ["Solo"], # Same single artist confirmed
}))
with patch.object(src_module, "get_config_manager", return_value=_make_cfg()), \
patch("core.metadata.get_deezer_client", return_value=fake_deezer):
meta = src_module.extract_source_metadata(context, {"name": "Solo"}, {})
assert meta["_artists_list"] == ["Solo"]
def test_no_upgrade_when_no_track_id(self):
"""Edge: source==deezer but no track_id. Can't call
/track/<id> without an id. Don't attempt the upgrade."""
from core.metadata import source as src_module
context = {
"original_search_result": {
"title": "T",
"artists": [{"name": "Primary"}],
},
"source": "deezer",
"source_track_id": "", # Missing
}
fake_deezer = SimpleNamespace(get_track_details=MagicMock())
with patch.object(src_module, "get_config_manager", return_value=_make_cfg()), \
patch("core.metadata.get_deezer_client", return_value=fake_deezer):
meta = src_module.extract_source_metadata(context, {"name": "Primary"}, {})
assert meta["_artists_list"] == ["Primary"]
fake_deezer.get_track_details.assert_not_called()

268
tests/test_acoustid_scanner.py
View file

@ -54,8 +54,11 @@ def _make_context(rows):
def test_load_db_tracks_skips_null_ids_and_normalizes_track_ids():
job = AcoustIDScannerJob()
context = _make_context([
(None, "Broken Track", "Artist", "/music/broken.flac", 1, "Album", None, None),
(42, "Good Track", "Artist", "/music/good.flac", 2, "Album", "album-thumb", "artist-thumb"),
# 10 columns: id, title, artist (COALESCE'd), file_path, track_number,
# album_title, album_thumb, artist_thumb, track_artist (raw, may be ''),
# album_artist.
(None, "Broken Track", "Artist", "/music/broken.flac", 1, "Album", None, None, "", "Artist"),
(42, "Good Track", "Artist", "/music/good.flac", 2, "Album", "album-thumb", "artist-thumb", "", "Artist"),
])
tracks = job._load_db_tracks(context)
@ -68,8 +71,11 @@ def test_load_db_tracks_skips_null_ids_and_normalizes_track_ids():
def test_scan_handles_mixed_track_id_types(monkeypatch):
job = AcoustIDScannerJob()
context = _make_context([
(None, "Broken Track", "Artist", "/music/broken.flac", 1, "Album", None, None),
(42, "Good Track", "Artist", "/music/good.flac", 2, "Album", "album-thumb", "artist-thumb"),
# 10 columns: id, title, artist (COALESCE'd), file_path, track_number,
# album_title, album_thumb, artist_thumb, track_artist (raw, may be ''),
# album_artist.
(None, "Broken Track", "Artist", "/music/broken.flac", 1, "Album", None, None, "", "Artist"),
(42, "Good Track", "Artist", "/music/good.flac", 2, "Album", "album-thumb", "artist-thumb", "", "Artist"),
])
monkeypatch.setattr(job, "_resolve_path", lambda file_path, _context: file_path)
@ -360,3 +366,257 @@ def test_load_db_tracks_falls_back_when_track_artist_empty_string():
# Empty string in track_artist → NULLIF returns NULL → COALESCE
# falls back to album artist
assert tracks['t1']['artist'] == 'Album Artist'
# ---------------------------------------------------------------------------
# File-tag fallback for legacy compilation tracks — Skowl Discord follow-up
# ---------------------------------------------------------------------------
#
# Skowl reported that the AcoustID Scanner was STILL flagging his
# compilation tracks even after the COALESCE(track_artist, album_artist)
# fix shipped. Cause: his tracks were downloaded BEFORE the
# `tracks.track_artist` column existed, so for those rows
# `track_artist IS NULL` and COALESCE falls back to the ALBUM artist
# (the curator) — same wrong-comparison the prior fix was supposed to
# eliminate.
#
# The audio file's ARTIST tag is ground truth for what's on disk:
# Tidal/Spotify/Deezer all write the per-track artist into the file's
# tag at download time, regardless of the SoulSync DB schema. Reading
# it during the scan closes the gap without requiring a DB backfill
# of the legacy rows. These tests pin:
# - File ARTIST tag trumps DB-resolved expected artist when present
# (Skowl's exact case: file says 'Eclypse', DB says 'Andromedik',
# AcoustID returns 'Eclypse' → no finding)
# - Missing file tag falls through to DB value (preserves
# pre-fix behavior for tracks without proper file tags)
# - mutagen failure is swallowed → falls through to DB
# - File tag matches DB → no behavioral change
def test_scanner_uses_file_tag_artist_over_db_for_legacy_compilation(monkeypatch):
"""Skowl's exact case verbatim:
DB row: artist_id → 'Andromedik' (album artist), track_artist=NULL
File tag: ARTIST='Eclypse' (Tidal-tagged correctly)
AcoustID: artist='Eclypse'
Pre-fix: expected='Andromedik' vs actual='Eclypse' → flag
Post-fix: file tag trumps DB → expected='Eclypse' → no flag
"""
job = AcoustIDScannerJob()
captured_findings = []
context = _make_finding_capturing_context(
track_row=("city-lights", "City Lights", "Andromedik",
"/music/eclypse-city-lights.opus", 1,
"High Tea Music: Vol 1", None, None),
captured=captured_findings,
)
fake_acoustid = SimpleNamespace(
fingerprint_and_lookup=lambda fpath: {
'best_score': 0.99,
'recordings': [{
'title': 'City Lights',
'artist': 'Eclypse',
}],
},
)
# Patch read_file_tags to return Tidal's correct per-track artist.
# The scanner imports lazily inside _scan_file so we patch the
# source module's symbol.
monkeypatch.setattr(
'core.tag_writer.read_file_tags',
lambda fpath: {'artist': 'Eclypse', 'title': 'City Lights'},
)
result = JobResultStub()
job._scan_file(
'/music/eclypse-city-lights.opus',
'city-lights',
{'title': 'City Lights', 'artist': 'Andromedik'}, # DB-resolved expected
fake_acoustid,
context,
result,
fp_threshold=0.85,
title_threshold=0.85,
artist_threshold=0.6,
)
assert captured_findings == [], (
f"Expected no finding (file tag matches AcoustID); got {captured_findings}"
)
def test_scanner_falls_back_to_db_when_file_tag_missing(monkeypatch):
"""Defensive: file has no ARTIST tag (rare but possible for
non-standard formats / damaged files). MUST fall back to DB
expected value. Otherwise the fix would BREAK the existing
'flag genuine mismatches' contract for files without tags."""
job = AcoustIDScannerJob()
captured_findings = []
context = _make_finding_capturing_context(
track_row=("99", "Some Track", "Foreigner",
"/music/track.flac", 1, "Album", None, None),
captured=captured_findings,
)
fake_acoustid = SimpleNamespace(
fingerprint_and_lookup=lambda fpath: {
'best_score': 0.99,
'recordings': [{
'title': 'Some Track',
'artist': 'Different Band',
}],
},
)
# File has no ARTIST tag (read_file_tags returns None for the field)
monkeypatch.setattr(
'core.tag_writer.read_file_tags',
lambda fpath: {'artist': None},
)
result = JobResultStub()
job._scan_file(
'/music/track.flac',
'99',
{'title': 'Some Track', 'artist': 'Foreigner'},
fake_acoustid,
context,
result,
fp_threshold=0.85,
title_threshold=0.85,
artist_threshold=0.6,
)
# Should still flag — file tag was missing, fell back to DB
# ('Foreigner') vs AcoustID ('Different Band') mismatch
assert len(captured_findings) == 1, (
f"Expected finding (file tag missing → DB fallback → genuine mismatch); got {captured_findings}"
)
def test_scanner_swallows_file_tag_read_exception(monkeypatch):
"""Defensive: mutagen errors mid-read shouldn't crash the scan
— must log + fall back to DB value gracefully."""
job = AcoustIDScannerJob()
captured_findings = []
context = _make_finding_capturing_context(
track_row=("99", "Track", "RealArtist",
"/music/corrupted.mp3", 1, "Album", None, None),
captured=captured_findings,
)
fake_acoustid = SimpleNamespace(
fingerprint_and_lookup=lambda fpath: {
'best_score': 0.99,
'recordings': [{'title': 'Track', 'artist': 'RealArtist'}],
},
)
def boom(fpath):
raise RuntimeError("mutagen exploded on corrupted file")
monkeypatch.setattr('core.tag_writer.read_file_tags', boom)
result = JobResultStub()
job._scan_file(
'/music/corrupted.mp3',
'99',
{'title': 'Track', 'artist': 'RealArtist'},
fake_acoustid,
context,
result,
fp_threshold=0.85,
title_threshold=0.85,
artist_threshold=0.6,
)
# No finding — DB matches AcoustID after the fallback
assert captured_findings == []
def test_scanner_trusts_curated_db_track_artist_over_stale_file_tag(monkeypatch):
"""The flip side of Skowl's case — user manually corrected
`track_artist` in the DB via the enhanced library view but
didn't re-tag the file. Pre-refactor 'file tag always wins'
would flag this as a false positive (file says wrong, DB says
right, AcoustID matches DB). Post-refactor: DB track_artist
is the curated source of truth when populated → file tag is
only consulted when DB is empty. No spurious flag.
This is why `_load_db_tracks` surfaces `track_artist` as a
separate field instead of just the COALESCE'd `artist`:
`_scan_file` needs to distinguish 'DB has a curated value'
from 'DB fell back to album artist'."""
job = AcoustIDScannerJob()
captured_findings = []
context = _make_finding_capturing_context(
track_row=("99", "Track", "AlbumArtist",
"/music/track.flac", 1, "Album", None, None),
captured=captured_findings,
)
fake_acoustid = SimpleNamespace(
fingerprint_and_lookup=lambda fpath: {
'best_score': 0.99,
'recordings': [{'title': 'Track', 'artist': 'Eclypse'}],
},
)
# File has wrong tag (stale — user edited DB but didn't re-tag),
# DB has correct value, AcoustID matches DB.
monkeypatch.setattr(
'core.tag_writer.read_file_tags',
lambda fpath: {'artist': 'WrongStaleTag'},
)
result = JobResultStub()
job._scan_file(
'/music/track.flac', '99',
# Simulates the post-refactor _load_db_tracks output:
# track_artist populated (curated) takes priority over file tag.
{'title': 'Track', 'artist': 'Eclypse',
'track_artist': 'Eclypse', 'album_artist': 'AlbumArtist'},
fake_acoustid, context, result,
fp_threshold=0.85, title_threshold=0.85, artist_threshold=0.6,
)
assert captured_findings == [], (
f"DB curated value must trump stale file tag; got {captured_findings}"
)
def test_scanner_file_tag_matches_db_no_behavioral_change(monkeypatch):
"""Sanity: when file tag and DB agree, behavior is identical to
the pre-fix path. No double-counting, no spurious findings."""
job = AcoustIDScannerJob()
captured_findings = []
context = _make_finding_capturing_context(
track_row=("99", "Track", "RealArtist",
"/music/track.flac", 1, "Album", None, None),
captured=captured_findings,
)
fake_acoustid = SimpleNamespace(
fingerprint_and_lookup=lambda fpath: {
'best_score': 0.99,
'recordings': [{'title': 'Track', 'artist': 'RealArtist'}],
},
)
monkeypatch.setattr(
'core.tag_writer.read_file_tags',
lambda fpath: {'artist': 'RealArtist'},
)
result = JobResultStub()
job._scan_file(
'/music/track.flac', '99',
{'title': 'Track', 'artist': 'RealArtist'},
fake_acoustid, context, result,
fp_threshold=0.85, title_threshold=0.85, artist_threshold=0.6,
)
assert captured_findings == []

300
tests/test_audiodb_worker_stuck_track.py Normal file
View file

@ -0,0 +1,300 @@
"""Pin AudioDB worker doesn't infinite-loop on direct-ID-lookup
failures.
Issue #553: when an entity already has `audiodb_id` populated
(from manual match or earlier scan) but `audiodb_match_status` is
NULL, the worker tries a direct ID lookup. If that lookup fails
(returns None on timeout — AudioDB's `track.php` endpoint is slow
and 10s timeouts are common), the prior code returned WITHOUT
marking status. Result: row stayed in NULL state, queue picked it
up next tick, retried, timed out, returned again — infinite loop.
User saw constant requests with no progress.
The fix:
- Mark status='error' so the queue's NULL-status filter stops
picking the row on every tick
- Add 'error' to the retry-after-cutoff queries (priorities 4-6)
so transient AudioDB outages still recover automatically after
`retry_days`
- Preserve the existing `audiodb_id` (don't overwrite it via
name-search fallback — original "preserve manual match" intent)
These tests pin:
- Direct-lookup-returns-None marks status='error' (no infinite loop)
- Direct-lookup-raises-exception marks status='error'
- Direct-lookup-success preserves existing match-success path
- 'error' status is included in retry-cutoff queue so eventual
recovery happens
"""
from __future__ import annotations
import sqlite3
from datetime import datetime, timedelta
from types import SimpleNamespace
from unittest.mock import MagicMock
import pytest
from core.audiodb_worker import AudioDBWorker
def _make_real_db_with_audiodb_columns(tmp_path):
"""Build a minimal SQLite DB with the artist/album/track schema
the worker needs. Real SQLite (not mocks) so the SQL queries
actually exercise the column names + retry-cutoff logic."""
db_path = tmp_path / "audiodb_test.db"
conn = sqlite3.connect(str(db_path))
conn.executescript("""
CREATE TABLE artists (
id INTEGER PRIMARY KEY,
name TEXT,
audiodb_id TEXT,
audiodb_match_status TEXT,
audiodb_last_attempted DATETIME,
updated_at DATETIME
);
CREATE TABLE albums (
id INTEGER PRIMARY KEY,
title TEXT,
artist_id INTEGER,
audiodb_id TEXT,
audiodb_match_status TEXT,
audiodb_last_attempted DATETIME,
updated_at DATETIME
);
CREATE TABLE tracks (
id INTEGER PRIMARY KEY,
title TEXT,
artist_id INTEGER,
audiodb_id TEXT,
audiodb_match_status TEXT,
audiodb_last_attempted DATETIME,
updated_at DATETIME
);
""")
conn.commit()
conn.close()
class _RealDB:
def _get_connection(self):
return sqlite3.connect(str(db_path))
return _RealDB(), db_path
def _make_worker(db, fake_client):
"""Build a worker with a real DB + mocked AudioDB client.
Skip __init__ side effects (config load, thread start)."""
worker = AudioDBWorker.__new__(AudioDBWorker)
worker.db = db
worker.client = fake_client
worker.retry_days = 30
worker.stats = {'matched': 0, 'not_found': 0, 'errors': 0, 'pending': 0}
worker.current_item = None
worker.running = False
worker.paused = False
worker.thread = None
return worker
# ---------------------------------------------------------------------------
# Issue #553 — direct-ID lookup failure no longer infinite-loops
# ---------------------------------------------------------------------------
class TestDirectLookupFailureMarksError:
def test_lookup_returns_none_marks_status_error(self, tmp_path):
"""Reporter's exact scenario: track has audiodb_id set,
match_status is NULL. AudioDB times out → lookup returns None.
Pre-fix: return without marking → infinite loop next tick.
Post-fix: mark status='error' → queue stops re-picking."""
db, db_path = _make_real_db_with_audiodb_columns(tmp_path)
# Seed a track with audiodb_id populated, status NULL
with sqlite3.connect(str(db_path)) as seed_conn:
seed_conn.execute(
"INSERT INTO artists (id, name) VALUES (?, ?)",
(1, 'Test Artist'),
)
seed_conn.execute(
"INSERT INTO tracks (id, title, artist_id, audiodb_id, audiodb_match_status) "
"VALUES (?, ?, ?, ?, ?)",
(32743988, 'Sweet Talk', 1, '12345', None),
)
seed_conn.commit()
# AudioDB client returns None on timeout (matches lookup_track_by_id behavior)
fake_client = SimpleNamespace(
lookup_artist_by_id=MagicMock(return_value=None),
lookup_album_by_id=MagicMock(return_value=None),
lookup_track_by_id=MagicMock(return_value=None),
)
worker = _make_worker(db, fake_client)
item = {
'type': 'track',
'id': 32743988,
'name': 'Sweet Talk',
'artist': 'Test Artist',
'artist_audiodb_id': None,
}
worker._process_item(item)
# Verify status was marked (no longer NULL → queue won't re-pick)
with sqlite3.connect(str(db_path)) as verify:
row = verify.execute(
"SELECT audiodb_match_status, audiodb_id, audiodb_last_attempted "
"FROM tracks WHERE id = ?",
(32743988,),
).fetchone()
assert row[0] == 'error', f"Expected status='error' to break loop; got {row[0]!r}"
# audiodb_id preserved (manual match not overwritten)
assert row[1] == '12345', f"audiodb_id must NOT be cleared; got {row[1]!r}"
# last_attempted set so retry-cutoff logic can re-pick later
assert row[2] is not None, "audiodb_last_attempted must be set for retry logic"
# Stats updated
assert worker.stats['errors'] == 1
def test_lookup_raises_exception_marks_status_error(self, tmp_path):
"""Defensive: if the AudioDB client itself raises (not just
returns None) the same loop-protection must apply. Some
client paths re-raise on certain error classes."""
db, db_path = _make_real_db_with_audiodb_columns(tmp_path)
with sqlite3.connect(str(db_path)) as seed_conn:
seed_conn.execute(
"INSERT INTO artists (id, name) VALUES (?, ?)",
(1, 'X'),
)
seed_conn.execute(
"INSERT INTO tracks (id, title, artist_id, audiodb_id, audiodb_match_status) "
"VALUES (?, ?, ?, ?, ?)",
(99, 'Y', 1, '67890', None),
)
seed_conn.commit()
fake_client = SimpleNamespace(
lookup_artist_by_id=MagicMock(side_effect=RuntimeError("boom")),
lookup_album_by_id=MagicMock(side_effect=RuntimeError("boom")),
lookup_track_by_id=MagicMock(side_effect=RuntimeError("read timeout")),
)
worker = _make_worker(db, fake_client)
item = {'type': 'track', 'id': 99, 'name': 'Y', 'artist': 'X',
'artist_audiodb_id': None}
worker._process_item(item)
with sqlite3.connect(str(db_path)) as verify:
row = verify.execute(
"SELECT audiodb_match_status FROM tracks WHERE id = ?",
(99,),
).fetchone()
assert row[0] == 'error'
def test_lookup_success_preserves_existing_path(self, tmp_path):
"""Sanity: when direct lookup SUCCEEDS, the existing match-
success path runs (update + stats['matched'] += 1). Don't
regress the happy path."""
db, db_path = _make_real_db_with_audiodb_columns(tmp_path)
with sqlite3.connect(str(db_path)) as seed_conn:
seed_conn.execute("INSERT INTO artists (id, name) VALUES (?, ?)", (1, 'A'))
seed_conn.execute(
"INSERT INTO tracks (id, title, artist_id, audiodb_id) "
"VALUES (?, ?, ?, ?)",
(50, 'T', 1, '111'),
)
seed_conn.commit()
fake_client = SimpleNamespace(
lookup_artist_by_id=MagicMock(),
lookup_album_by_id=MagicMock(),
lookup_track_by_id=MagicMock(return_value={
'idTrack': '111',
'strTrack': 'T',
'idArtist': '999',
}),
)
worker = _make_worker(db, fake_client)
# Stub the per-entity update method so we don't need every column
worker._update_track = MagicMock()
worker._verify_artist_id = MagicMock(return_value=True)
item = {'type': 'track', 'id': 50, 'name': 'T', 'artist': 'A',
'artist_audiodb_id': None}
worker._process_item(item)
worker._update_track.assert_called_once()
assert worker.stats['matched'] == 1
assert worker.stats['errors'] == 0
# ---------------------------------------------------------------------------
# Retry queue includes 'error' status — transient outages eventually recover
# ---------------------------------------------------------------------------
class TestErrorRetryAfterCutoff:
def test_error_track_picked_up_after_cutoff(self, tmp_path):
"""After fix #553, rows marked 'error' get a 30-day retry
cutoff — same treatment as 'not_found'. Without this they'd
stay errored forever after a transient AudioDB outage."""
db, db_path = _make_real_db_with_audiodb_columns(tmp_path)
# Seed a track marked 'error' with last_attempted older than retry_days.
# Artist must be marked 'matched' too — otherwise priority 1 (NULL-status
# artists) wins over priority 6 (error/not_found track retry).
old_attempt = datetime.now() - timedelta(days=31)
with sqlite3.connect(str(db_path)) as seed_conn:
seed_conn.execute(
"INSERT INTO artists (id, name, audiodb_match_status) VALUES (?, ?, ?)",
(1, 'A', 'matched'),
)
seed_conn.execute(
"INSERT INTO tracks (id, title, artist_id, audiodb_match_status, audiodb_last_attempted) "
"VALUES (?, ?, ?, ?, ?)",
(10, 'OldErrored', 1, 'error', old_attempt),
)
seed_conn.commit()
fake_client = SimpleNamespace() # not called for queue check
worker = _make_worker(db, fake_client)
item = worker._get_next_item()
assert item is not None, "Expected error-status track past retry cutoff to be picked up"
assert item['type'] == 'track'
assert item['id'] == 10
def test_error_track_NOT_picked_within_cutoff(self, tmp_path):
"""Sanity: rows marked 'error' but recently-attempted should
NOT be picked. Otherwise the retry-cutoff doesn't actually
rate-limit retries and we're back to the loop."""
db, db_path = _make_real_db_with_audiodb_columns(tmp_path)
# Just-attempted (within cutoff). Artist marked matched
# so priority 1 doesn't intercept the queue check.
recent_attempt = datetime.now() - timedelta(days=1)
with sqlite3.connect(str(db_path)) as seed_conn:
seed_conn.execute(
"INSERT INTO artists (id, name, audiodb_match_status) VALUES (?, ?, ?)",
(1, 'A', 'matched'),
)
seed_conn.execute(
"INSERT INTO tracks (id, title, artist_id, audiodb_match_status, audiodb_last_attempted) "
"VALUES (?, ?, ?, ?, ?)",
(20, 'RecentErrored', 1, 'error', recent_attempt),
)
seed_conn.commit()
worker = _make_worker(db, SimpleNamespace())
item = worker._get_next_item()
assert item is None, (
"Recently-attempted error rows must NOT be picked up — that's "
"the loop-prevention mechanism"
)

167
tests/test_debug_info_services.py Normal file
View file

@ -0,0 +1,167 @@
"""Pin the `info['services']` block returned by /api/debug-info.
Pre-fix the `music_source` field always rendered as "unknown" because
the code read `_status_cache.get('spotify', {})` — but the cache only
ever holds 'media_server' and 'soulseek' keys, so the fallback always
fired. Same problem (silently) for `spotify_connected` and
`spotify_rate_limited`. Hydrabase was missing entirely.
Fix routes those reads through the canonical accessors:
- `music_source` → `core.metadata.registry.get_primary_source` (which
already accounts for the auth-fallback chain — Spotify → Deezer when
unauthenticated)
- `spotify_connected` / `spotify_rate_limited` →
`core.metadata.status.get_spotify_status`
- `hydrabase_connected` → `core.metadata.registry.is_hydrabase_enabled`
- `youtube_available` → constant True (URL-based, no auth)
- `hifi_instance_count` → `db.get_hifi_instances`
- `always_available_metadata_sources` → static list of public-API
sources (Deezer / iTunes / MusicBrainz)
"""
from __future__ import annotations
from contextlib import contextmanager
from unittest.mock import patch
import pytest
@pytest.fixture
def app_test_client():
import web_server
web_server.app.config['TESTING'] = True
with web_server.app.test_client() as client:
yield client
@contextmanager
def _patched_endpoint(
primary_source='spotify',
spotify_status=None,
hydrabase_enabled=False,
primary_source_raises=False,
spotify_status_raises=False,
hydrabase_raises=False,
):
"""Patch the three module-level lookups inside `core.debug_info`
and yield. Each can either return a fixed value or raise — the
`*_raises` flags select which."""
if spotify_status is None:
spotify_status = {'connected': True, 'rate_limited': False}
def _boom(*_a, **_k):
raise RuntimeError('forced failure for test')
primary_patch = patch(
'core.debug_info.get_primary_source',
side_effect=_boom if primary_source_raises else None,
return_value=None if primary_source_raises else primary_source,
)
spotify_patch = patch(
'core.debug_info.get_spotify_status',
side_effect=_boom if spotify_status_raises else None,
return_value=None if spotify_status_raises else spotify_status,
)
hydrabase_patch = patch(
'core.debug_info.is_hydrabase_enabled',
side_effect=_boom if hydrabase_raises else None,
return_value=None if hydrabase_raises else hydrabase_enabled,
)
with primary_patch, spotify_patch, hydrabase_patch:
yield
def _services(client):
resp = client.get('/api/debug-info')
assert resp.status_code == 200
return resp.get_json()['services']
def test_music_source_uses_primary_source_not_status_cache(app_test_client):
"""The bug: music_source always read 'unknown' because it pulled
from a non-existent 'spotify' key in `_status_cache`. Fix routes
it through `get_primary_source` which is the actual authority."""
with _patched_endpoint(primary_source='tidal'):
services = _services(app_test_client)
assert services['music_source'] == 'tidal'
def test_music_source_falls_back_to_unknown_when_lookup_raises(app_test_client):
"""Defensive: if `get_primary_source` itself blows up, the field
still renders as 'unknown' rather than crashing the whole endpoint."""
with _patched_endpoint(primary_source_raises=True):
services = _services(app_test_client)
assert services['music_source'] == 'unknown'
def test_spotify_connected_uses_get_spotify_status(app_test_client):
"""`spotify_connected` was reading `_status_cache.get('spotify', {})`,
which never had the key. Routed through `get_spotify_status` now."""
with _patched_endpoint(spotify_status={'connected': True, 'rate_limited': False}):
services = _services(app_test_client)
assert services['spotify_connected'] is True
def test_spotify_rate_limited_uses_get_spotify_status(app_test_client):
with _patched_endpoint(spotify_status={'connected': True, 'rate_limited': True}):
services = _services(app_test_client)
assert services['spotify_rate_limited'] is True
def test_spotify_status_lookup_failure_does_not_break_endpoint(app_test_client):
"""`get_spotify_status` raises → both spotify_* fields default to
False rather than 500'ing the whole debug dump."""
with _patched_endpoint(spotify_status_raises=True):
services = _services(app_test_client)
assert services['spotify_connected'] is False
assert services['spotify_rate_limited'] is False
def test_hydrabase_connected_present(app_test_client):
"""Hydrabase status was never surfaced in debug info even though
it's an active metadata source. Now reported."""
with _patched_endpoint(hydrabase_enabled=True):
services = _services(app_test_client)
assert services['hydrabase_connected'] is True
def test_hydrabase_disconnected_when_disabled(app_test_client):
with _patched_endpoint(hydrabase_enabled=False):
services = _services(app_test_client)
assert services['hydrabase_connected'] is False
def test_hydrabase_lookup_failure_defaults_false(app_test_client):
with _patched_endpoint(hydrabase_raises=True):
services = _services(app_test_client)
assert services['hydrabase_connected'] is False
def test_youtube_available_always_true(app_test_client):
"""YouTube is URL-based via yt-dlp, no auth, always available.
Surfaced so the dump documents it as a download source."""
with _patched_endpoint():
services = _services(app_test_client)
assert services['youtube_available'] is True
def test_always_available_metadata_sources_listed(app_test_client):
"""Public-API metadata sources (no auth, no per-user state) listed
so the debug dump reflects the full metadata surface."""
with _patched_endpoint():
services = _services(app_test_client)
available = services['always_available_metadata_sources']
assert 'deezer' in available
assert 'itunes' in available
assert 'musicbrainz' in available
def test_hifi_instance_count_present(app_test_client):
"""HiFi instance count exposed because each instance is a separate
endpoint with its own auth state — single connected/disconnected
bool wouldn't capture the actual config."""
with _patched_endpoint():
services = _services(app_test_client)
assert 'hifi_instance_count' in services
assert isinstance(services['hifi_instance_count'], int)

13
tests/test_repair_worker_album_fill.py
View file

@ -62,6 +62,7 @@ def test_perform_album_fill_copy_branch_generates_track_id(tmp_path, monkeypatch
duration INTEGER,
file_path TEXT,
bitrate INTEGER,
server_source TEXT,
created_at TEXT,
updated_at TEXT
)
@ -69,10 +70,10 @@ def test_perform_album_fill_copy_branch_generates_track_id(tmp_path, monkeypatch
)
conn.execute(
"""
INSERT INTO tracks (id, album_id, artist_id, title, track_number, duration, file_path, bitrate, created_at, updated_at)
VALUES (?, ?, ?, ?, ?, ?, ?, ?, CURRENT_TIMESTAMP, CURRENT_TIMESTAMP)
INSERT INTO tracks (id, album_id, artist_id, title, track_number, duration, file_path, bitrate, server_source, created_at, updated_at)
VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, CURRENT_TIMESTAMP, CURRENT_TIMESTAMP)
""",
("target-track-1", "target-album", "target-artist", "Existing Track", 1, 180000, str(src_path), 320),
("target-track-1", "target-album", "target-artist", "Existing Track", 1, 180000, str(src_path), 320, "navidrome"),
)
conn.commit()
@ -89,6 +90,7 @@ def test_perform_album_fill_copy_branch_generates_track_id(tmp_path, monkeypatch
duration=180000,
file_path=str(src_path),
bitrate=320,
server_source="soulsync",
),
SimpleNamespace(
id="source-track-2",
@ -97,6 +99,7 @@ def test_perform_album_fill_copy_branch_generates_track_id(tmp_path, monkeypatch
duration=181000,
file_path=str(src_path),
bitrate=320,
server_source="soulsync",
),
]
@ -120,6 +123,7 @@ def test_perform_album_fill_copy_branch_generates_track_id(tmp_path, monkeypatch
duration=180000,
file_path=str(src_path),
bitrate=320,
server_source="soulsync",
),
album_id="target-album",
album_title="Target Album",
@ -137,7 +141,7 @@ def test_perform_album_fill_copy_branch_generates_track_id(tmp_path, monkeypatch
with sqlite3.connect(db_path) as verify_conn:
row = verify_conn.execute(
"SELECT id, title, file_path FROM tracks WHERE title = ?",
"SELECT id, title, file_path, server_source FROM tracks WHERE title = ?",
("New Track",),
).fetchone()
assert row is not None
@ -145,4 +149,5 @@ def test_perform_album_fill_copy_branch_generates_track_id(tmp_path, monkeypatch
assert row[0].startswith("album_fill_source-track-1_deadbeef")
assert row[1] == "New Track"
assert Path(row[2]).exists()
assert row[3] == "navidrome"
assert verify_conn.execute("SELECT COUNT(*) FROM tracks WHERE id IS NULL").fetchone()[0] == 0

184
tests/test_repair_worker_unresolvable_folder_error.py Normal file
View file

@ -0,0 +1,184 @@
"""Pin the diagnostic error string from
``RepairWorker._build_unresolvable_album_folder_error``.
GitHub issue #558 (gabistek, Navidrome on Docker / Arch host): the
Album Completeness Auto-Fill button surfaced a flat "Could not
determine album folder from existing tracks" error with no diagnostic.
Reporter is on Navidrome, which (unlike Plex) has no API that exposes
filesystem library paths — so the resolver returns None whenever the
DB-recorded path doesn't already exist as-is in SoulSync's container
view AND the user hasn't manually configured Settings → Library →
Music Paths.
The fix replaces the flat string with a multi-part diagnostic naming
the active media server, showing one sample DB path, listing the base
directories the resolver actually probed, and pointing the user at the
config that would unblock them. These tests pin each part so future
copy edits don't accidentally drop the actionable hint or the sample
path.
"""
from __future__ import annotations
import sys
import types
from types import SimpleNamespace
# ── Stub modules that the import of core.repair_worker pulls in ──
if "spotipy" not in sys.modules:
spotipy = types.ModuleType("spotipy")
class _DummySpotify:
def __init__(self, *args, **kwargs):
pass
oauth2 = types.ModuleType("spotipy.oauth2")
class _DummyOAuth:
def __init__(self, *args, **kwargs):
pass
spotipy.Spotify = _DummySpotify
oauth2.SpotifyOAuth = _DummyOAuth
oauth2.SpotifyClientCredentials = _DummyOAuth
spotipy.oauth2 = oauth2
sys.modules["spotipy"] = spotipy
sys.modules["spotipy.oauth2"] = oauth2
if "config.settings" not in sys.modules:
config_pkg = types.ModuleType("config")
settings_mod = types.ModuleType("config.settings")
class _DummyConfigManager:
def get(self, key, default=None):
return default
def get_active_media_server(self):
return "plex"
settings_mod.config_manager = _DummyConfigManager()
config_pkg.settings = settings_mod
sys.modules["config"] = config_pkg
sys.modules["config.settings"] = settings_mod
from core.library.path_resolver import ResolveAttempt
from core.repair_worker import RepairWorker
def _make_worker(active_server="plex"):
"""Bare RepairWorker with a config_manager that reports the given
active media server. We never run the full job — just exercise the
diagnostic builder."""
worker = RepairWorker(database=SimpleNamespace())
cfg = SimpleNamespace()
cfg.get_active_media_server = lambda: active_server
cfg.get = lambda key, default=None: default
worker._config_manager = cfg
return worker
def test_error_names_active_media_server():
"""User needs to know which server's path conventions are at play
so they can set the right mount in Settings."""
worker = _make_worker(active_server="navidrome")
msg = worker._build_unresolvable_album_folder_error(
ResolveAttempt(base_dirs_tried=["/app/Transfer"]),
"/music/Artist/Album/track.flac",
)
assert "navidrome" in msg.lower(), (
f"Active server name must appear in error; got: {msg}"
)
def test_error_includes_sample_db_path():
"""One concrete path lets the user see what their media server
is reporting — usually enough to reverse-engineer the right mount."""
worker = _make_worker()
sample = "/music/Kendrick Lamar/Mr. Morale/01 - United in Grief.flac"
msg = worker._build_unresolvable_album_folder_error(
ResolveAttempt(base_dirs_tried=["/app/Transfer"]),
sample,
)
assert sample in msg, (
f"Sample DB path must appear verbatim in error; got: {msg}"
)
def test_error_lists_base_dirs_tried():
"""User needs to know what the resolver probed — otherwise they
can't tell whether to add a new mount or whether the existing one
just doesn't match the recorded path."""
worker = _make_worker()
attempt = ResolveAttempt(
base_dirs_tried=["/app/Transfer", "/downloads", "/library"],
)
msg = worker._build_unresolvable_album_folder_error(attempt, "/music/x.flac")
for base in attempt.base_dirs_tried:
assert base in msg, f"Probed base dir {base!r} missing from error: {msg}"
def test_error_calls_out_no_base_dirs_when_empty():
"""When the resolver had nothing to probe, that's a different
failure mode than "tried 3 dirs and failed" — the user needs
different action. Pin that the message distinguishes them."""
worker = _make_worker()
msg = worker._build_unresolvable_album_folder_error(
ResolveAttempt(base_dirs_tried=[]),
"/music/x.flac",
)
assert "no base director" in msg.lower(), (
f"Empty-base-dirs case must surface 'no base directories'; got: {msg}"
)
def test_error_always_includes_settings_hint():
"""The actionable fix line must always appear regardless of which
failure mode fired. This is the part the user needs to act on."""
worker = _make_worker()
for attempt in (
ResolveAttempt(base_dirs_tried=[]),
ResolveAttempt(base_dirs_tried=["/app/Transfer"]),
None,
):
msg = worker._build_unresolvable_album_folder_error(attempt, "/music/x.flac")
assert "Settings" in msg, f"Settings hint missing for attempt={attempt}; got: {msg}"
assert "Music Paths" in msg, f"Music Paths hint missing for attempt={attempt}; got: {msg}"
def test_error_handles_none_attempt_defensively():
"""If for some reason no ResolveAttempt is collected (e.g. zero
existing tracks loop never ran), the helper must not crash. It
can omit the probe-detail line but must still emit the actionable
Settings hint."""
worker = _make_worker()
msg = worker._build_unresolvable_album_folder_error(None, "/music/x.flac")
assert "Settings" in msg, f"None attempt must still emit Settings hint; got: {msg}"
assert "/music/x.flac" in msg
def test_error_handles_missing_sample_path():
"""If we couldn't sample a DB path (e.g. all entries had None
file_path), the path line is omitted but the rest of the message
still renders."""
worker = _make_worker()
msg = worker._build_unresolvable_album_folder_error(
ResolveAttempt(base_dirs_tried=["/app/Transfer"]),
None,
)
assert "Settings" in msg
# No sample-path line means no "Example DB-recorded path" prefix
assert "Example DB-recorded path:" not in msg
def test_error_handles_missing_config_manager():
"""RepairWorker may be constructed without a config_manager; the
builder shouldn't crash and should fall back to 'unknown' for the
server name rather than blowing up."""
worker = RepairWorker(database=SimpleNamespace())
worker._config_manager = None
msg = worker._build_unresolvable_album_folder_error(
ResolveAttempt(base_dirs_tried=[]), "/music/x.flac",
)
assert "unknown" in msg.lower()

333
tests/test_server_playlist_append_mode.py Normal file
View file

@ -0,0 +1,333 @@
"""Pin server-playlist sync 'append' mode behavior.
Discord report (CJFC, 2026-04-26): syncing a Spotify playlist to the
server overwrote anything the user had manually added to the server-
side playlist. The fix adds a per-sync mode toggle:
- 'replace' (default, current behavior) — delete + recreate
- 'append' — keep existing tracks, only add new ones
Each server client (Plex / Jellyfin / Navidrome) gets a new
`append_to_playlist(name, tracks)` method that:
- Falls back to `create_playlist` when the playlist doesn't exist yet
- Fetches existing track IDs and dedupes incoming tracks against them
- Uses the server's NATIVE append API (no delete-recreate)
`sync_service.sync_playlist` accepts `sync_mode` and dispatches to
`append_to_playlist` when set to 'append'. Falls back to
`update_playlist` (replace semantics) when the client doesn't
implement append (e.g. SoulSync standalone has no playlist methods
at all).
These tests pin:
- Per-server append: missing playlist → create_playlist delegation
- Per-server append: existing IDs filtered out (no double-adds)
- Per-server append: empty new-track set short-circuits without API call
- Per-server append: failure paths return False without raising
- sync_service dispatch: mode='append' calls append_to_playlist
- sync_service dispatch: mode='replace' calls update_playlist (default)
- sync_service dispatch: missing append_to_playlist method → falls back to update_playlist
"""
from __future__ import annotations
from types import SimpleNamespace
from unittest.mock import MagicMock, patch
import pytest
# ---------------------------------------------------------------------------
# Plex append_to_playlist
# ---------------------------------------------------------------------------
from core.plex_client import PlexClient
def _make_plex_client():
client = PlexClient.__new__(PlexClient)
client.server = MagicMock()
client.music_library = MagicMock()
client._all_libraries_mode = False
client._connection_attempted = True
client._is_connecting = False
client._last_connection_check = 0
client._connection_check_interval = 30
return client
class TestPlexAppendToPlaylist:
def test_falls_back_to_create_when_playlist_missing(self):
"""Reporter's playlist may not exist on the server yet (first
sync). Append mode should create it instead of erroring."""
from plexapi.exceptions import NotFound
client = _make_plex_client()
client.server.playlist = MagicMock(side_effect=NotFound("not found"))
new_tracks = [SimpleNamespace(ratingKey='100'), SimpleNamespace(ratingKey='101')]
with patch.object(client, 'ensure_connection', return_value=True), \
patch.object(client, 'create_playlist', return_value=True) as mock_create:
result = client.append_to_playlist("Test Playlist", new_tracks)
assert result is True
mock_create.assert_called_once_with("Test Playlist", new_tracks)
def test_filters_out_already_present_tracks(self):
"""Reporter's exact case: server playlist has tracks A, B
already; sync brings A, B, C. Only C should be added.
Existing tracks must NOT be re-added (would create
duplicates)."""
client = _make_plex_client()
existing_playlist = MagicMock()
existing_playlist.items = MagicMock(return_value=[
SimpleNamespace(ratingKey='100'), # track A
SimpleNamespace(ratingKey='101'), # track B
])
existing_playlist.addItems = MagicMock()
client.server.playlist = MagicMock(return_value=existing_playlist)
incoming = [
SimpleNamespace(ratingKey='100'), # already present
SimpleNamespace(ratingKey='101'), # already present
SimpleNamespace(ratingKey='102'), # NEW — only this should be added
]
with patch.object(client, 'ensure_connection', return_value=True):
result = client.append_to_playlist("Test Playlist", incoming)
assert result is True
# Only the new track passed to addItems
called_with = existing_playlist.addItems.call_args[0][0]
assert len(called_with) == 1
assert called_with[0].ratingKey == '102'
def test_short_circuits_when_all_tracks_already_present(self):
"""All incoming tracks already on the playlist → no API call,
return True (no-op success)."""
client = _make_plex_client()
existing_playlist = MagicMock()
existing_playlist.items = MagicMock(return_value=[
SimpleNamespace(ratingKey='100'),
SimpleNamespace(ratingKey='101'),
])
existing_playlist.addItems = MagicMock()
client.server.playlist = MagicMock(return_value=existing_playlist)
incoming = [SimpleNamespace(ratingKey='100'), SimpleNamespace(ratingKey='101')]
with patch.object(client, 'ensure_connection', return_value=True):
result = client.append_to_playlist("Test Playlist", incoming)
assert result is True
existing_playlist.addItems.assert_not_called()
def test_returns_false_when_not_connected(self):
"""Defensive: ensure_connection False → return False, no API
call. Caller treats as a normal failure."""
client = _make_plex_client()
with patch.object(client, 'ensure_connection', return_value=False):
result = client.append_to_playlist("Test Playlist", [
SimpleNamespace(ratingKey='100'),
])
assert result is False
def test_swallows_exceptions_returns_false(self):
"""Plex SDK errors mid-append shouldn't crash the sync — log
+ return False so the caller can fall back."""
client = _make_plex_client()
client.server.playlist = MagicMock(side_effect=RuntimeError("plex down"))
with patch.object(client, 'ensure_connection', return_value=True):
result = client.append_to_playlist("Test Playlist", [
SimpleNamespace(ratingKey='100'),
])
assert result is False
# ---------------------------------------------------------------------------
# Jellyfin append_to_playlist
# ---------------------------------------------------------------------------
from core.jellyfin_client import JellyfinClient
def _make_jellyfin_client():
client = JellyfinClient.__new__(JellyfinClient)
client.base_url = "http://jellyfin.local"
client.api_key = "fake-api-key"
client.user_id = "user-123"
return client
class TestJellyfinAppendToPlaylist:
def test_falls_back_to_create_when_playlist_missing(self):
client = _make_jellyfin_client()
new_tracks = [SimpleNamespace(id='item-100')]
with patch.object(client, 'ensure_connection', return_value=True), \
patch.object(client, 'get_playlist_by_name', return_value=None), \
patch.object(client, 'create_playlist', return_value=True) as mock_create:
result = client.append_to_playlist("Test", new_tracks)
assert result is True
mock_create.assert_called_once_with("Test", new_tracks)
def test_filters_out_already_present_tracks(self):
"""Reporter's exact case for Jellyfin — only new GUIDs go in."""
client = _make_jellyfin_client()
existing_playlist = SimpleNamespace(id='pl-1')
existing_tracks = [
SimpleNamespace(id='aaaaaaaa-bbbb-cccc-dddd-000000000001'),
SimpleNamespace(id='aaaaaaaa-bbbb-cccc-dddd-000000000002'),
]
incoming = [
SimpleNamespace(id='aaaaaaaa-bbbb-cccc-dddd-000000000001'), # present
SimpleNamespace(id='aaaaaaaa-bbbb-cccc-dddd-000000000003'), # NEW
]
captured_post_params = {}
def fake_post(url, params=None, headers=None, timeout=None):
captured_post_params['url'] = url
captured_post_params['ids'] = params['Ids']
return SimpleNamespace(status_code=204, text='')
with patch.object(client, 'ensure_connection', return_value=True), \
patch.object(client, 'get_playlist_by_name', return_value=existing_playlist), \
patch.object(client, 'get_playlist_tracks', return_value=existing_tracks), \
patch.object(client, '_is_valid_guid', return_value=True), \
patch('core.jellyfin_client.requests.post', side_effect=fake_post):
result = client.append_to_playlist("Test", incoming)
assert result is True
# Only the NEW track id should have been POSTed
assert captured_post_params['ids'] == 'aaaaaaaa-bbbb-cccc-dddd-000000000003'
def test_short_circuits_when_no_new_tracks(self):
client = _make_jellyfin_client()
existing_playlist = SimpleNamespace(id='pl-1')
existing_tracks = [SimpleNamespace(id='guid-1')]
with patch.object(client, 'ensure_connection', return_value=True), \
patch.object(client, 'get_playlist_by_name', return_value=existing_playlist), \
patch.object(client, 'get_playlist_tracks', return_value=existing_tracks), \
patch.object(client, '_is_valid_guid', return_value=True), \
patch('core.jellyfin_client.requests.post') as mock_post:
result = client.append_to_playlist("Test", [SimpleNamespace(id='guid-1')])
assert result is True
mock_post.assert_not_called()
def test_returns_false_on_post_error(self):
client = _make_jellyfin_client()
existing_playlist = SimpleNamespace(id='pl-1')
with patch.object(client, 'ensure_connection', return_value=True), \
patch.object(client, 'get_playlist_by_name', return_value=existing_playlist), \
patch.object(client, 'get_playlist_tracks', return_value=[]), \
patch.object(client, '_is_valid_guid', return_value=True), \
patch('core.jellyfin_client.requests.post',
return_value=SimpleNamespace(status_code=500, text='server error')):
result = client.append_to_playlist("Test", [SimpleNamespace(id='new-guid')])
assert result is False
# ---------------------------------------------------------------------------
# Navidrome append_to_playlist
# ---------------------------------------------------------------------------
from core.navidrome_client import NavidromeClient
def _make_navidrome_client():
client = NavidromeClient.__new__(NavidromeClient)
client.base_url = "http://navidrome.local"
client.username = "user"
client.password = "pass"
return client
class TestNavidromeAppendToPlaylist:
def test_falls_back_to_create_when_playlist_missing(self):
client = _make_navidrome_client()
with patch.object(client, 'ensure_connection', return_value=True), \
patch.object(client, 'get_playlists_by_name', return_value=[]), \
patch.object(client, 'create_playlist', return_value=True) as mock_create:
result = client.append_to_playlist("Test", [SimpleNamespace(id='song-1')])
assert result is True
mock_create.assert_called_once()
def test_filters_out_already_present_tracks_and_calls_subsonic(self):
client = _make_navidrome_client()
existing_playlists = [SimpleNamespace(id='pl-1', title='Test')]
existing_tracks = [SimpleNamespace(id='100'), SimpleNamespace(id='101')]
incoming = [
SimpleNamespace(id='100'), # present
SimpleNamespace(id='102'), # NEW
SimpleNamespace(id='103'), # NEW
]
captured = {}
def fake_make_request(endpoint, params=None):
captured['endpoint'] = endpoint
captured['params'] = params
return {'status': 'ok'}
with patch.object(client, 'ensure_connection', return_value=True), \
patch.object(client, 'get_playlists_by_name', return_value=existing_playlists), \
patch.object(client, 'get_playlist_tracks', return_value=existing_tracks), \
patch.object(client, '_make_request', side_effect=fake_make_request):
result = client.append_to_playlist("Test", incoming)
assert result is True
assert captured['endpoint'] == 'updatePlaylist'
assert captured['params']['playlistId'] == 'pl-1'
# Only NEW song IDs in songIdToAdd, not already-present ones
assert sorted(captured['params']['songIdToAdd']) == ['102', '103']
def test_short_circuits_when_no_new_tracks(self):
client = _make_navidrome_client()
existing_playlists = [SimpleNamespace(id='pl-1', title='Test')]
existing_tracks = [SimpleNamespace(id='100')]
with patch.object(client, 'ensure_connection', return_value=True), \
patch.object(client, 'get_playlists_by_name', return_value=existing_playlists), \
patch.object(client, 'get_playlist_tracks', return_value=existing_tracks), \
patch.object(client, '_make_request') as mock_req:
result = client.append_to_playlist("Test", [SimpleNamespace(id='100')])
assert result is True
mock_req.assert_not_called()
def test_falls_back_when_subsonic_returns_failed(self):
client = _make_navidrome_client()
existing_playlists = [SimpleNamespace(id='pl-1', title='Test')]
with patch.object(client, 'ensure_connection', return_value=True), \
patch.object(client, 'get_playlists_by_name', return_value=existing_playlists), \
patch.object(client, 'get_playlist_tracks', return_value=[]), \
patch.object(client, '_make_request', return_value=None):
# _make_request returns None when Subsonic returns 'failed' status
result = client.append_to_playlist("Test", [SimpleNamespace(id='new-1')])
assert result is False
# ---------------------------------------------------------------------------
# Contract pinning — append_to_playlist is in KNOWN_PER_SERVER_METHODS
# ---------------------------------------------------------------------------
def test_append_to_playlist_listed_in_contract():
"""If a future refactor drops `append_to_playlist` from the
contract's KNOWN_PER_SERVER_METHODS list, the conformance test
won't catch it (those are advisory-only). This test is the
explicit pin that the method is part of the recognized
per-server playlist surface."""
from core.media_server.contract import KNOWN_PER_SERVER_METHODS
assert 'append_to_playlist' in KNOWN_PER_SERVER_METHODS
def test_each_client_implements_append_to_playlist():
"""Pin: Plex / Jellyfin / Navidrome all have the method (at the
class level — instance state isn't required for this check).
SoulSync standalone is intentionally excluded — it has no
playlist methods at all per the contract notes."""
assert hasattr(PlexClient, 'append_to_playlist')
assert hasattr(JellyfinClient, 'append_to_playlist')
assert hasattr(NavidromeClient, 'append_to_playlist')

254
tests/test_soulseek_search_throttle.py Normal file
View file

@ -0,0 +1,254 @@
"""Pin `compute_search_wait_seconds` — the pure scheduler behind the
slskd search throttle.
Reddit report (YeloMelo95, Bell Canada): ISP anti-abuse cuts the user's
WAN connection after a burst of slskd searches. The pre-fix throttle
was hardcoded to 35 searches per 220s sliding window, which allowed all
35 in rapid succession and only blocked once the cap was hit. That's
fine for soulseek-side bans but doesn't smooth bursts at the ISP layer.
Fix lifts the cap + window to config and adds a new `min_delay_seconds`
knob. The pure helper takes the throttle inputs and returns how long to
sleep — easy to test independently of asyncio.sleep / the singleton
client / wall-clock time.
"""
from __future__ import annotations
import pytest
from core.soulseek_client import compute_search_wait_seconds
# ---------------------------------------------------------------------------
# Defaults / no-throttle path
# ---------------------------------------------------------------------------
class TestNoThrottleNeeded:
def test_empty_state_returns_zero(self):
"""First search ever → no timestamps, no last-search → no wait."""
assert compute_search_wait_seconds(
timestamps=[],
last_search_at=0.0,
now=100.0,
max_per_window=35,
window_seconds=220,
min_delay_seconds=0,
) == 0.0
def test_below_window_cap_returns_zero(self):
"""When timestamps haven't filled the window cap and min-delay
is disabled, no wait. Preserves prior behavior for existing
users who don't tune the new knob."""
assert compute_search_wait_seconds(
timestamps=[10.0, 20.0, 30.0],
last_search_at=30.0,
now=100.0,
max_per_window=35,
window_seconds=220,
min_delay_seconds=0,
) == 0.0
def test_min_delay_zero_is_disabled(self):
"""Explicit zero (the default) means no min-delay enforcement
even when the last search was a millisecond ago. Confirms
backwards compat — existing users see no new wait."""
assert compute_search_wait_seconds(
timestamps=[],
last_search_at=99.99,
now=100.0,
max_per_window=35,
window_seconds=220,
min_delay_seconds=0,
) == 0.0
# ---------------------------------------------------------------------------
# Sliding-window cap (legacy behavior preserved)
# ---------------------------------------------------------------------------
class TestSlidingWindowCap:
def test_window_full_waits_for_oldest_to_age_out(self):
"""35 timestamps in window → wait until oldest ages out.
Same semantics as the pre-fix hardcoded behavior."""
timestamps = [10.0 + i for i in range(35)] # 10..44
# now = 50, window = 220, oldest = 10 → ages out at 230 → wait 180
wait = compute_search_wait_seconds(
timestamps=timestamps,
last_search_at=44.0,
now=50.0,
max_per_window=35,
window_seconds=220,
min_delay_seconds=0,
)
assert wait == pytest.approx(180.0, abs=1e-9)
def test_window_full_but_oldest_already_aged_out_returns_zero(self):
"""If now is past oldest+window, the negative is clamped to 0
(the caller is expected to prune timestamps before passing —
this is just defense-in-depth)."""
wait = compute_search_wait_seconds(
timestamps=[10.0] * 35,
last_search_at=10.0,
now=400.0,
max_per_window=35,
window_seconds=220,
min_delay_seconds=0,
)
assert wait == 0.0
def test_custom_max_per_window_honored(self):
"""User dials max down to 10 (paranoia mode for ISP anti-abuse).
Cap kicks in at 10, not 35."""
timestamps = [10.0 + i for i in range(10)]
wait = compute_search_wait_seconds(
timestamps=timestamps,
last_search_at=19.0,
now=20.0,
max_per_window=10,
window_seconds=60,
min_delay_seconds=0,
)
# oldest = 10, ages out at 70, now = 20 → wait 50
assert wait == pytest.approx(50.0, abs=1e-9)
def test_max_per_window_zero_disables_window_cap(self):
"""Defensive: max=0 means no cap (don't divide by zero, don't
block forever). Min-delay still applies if set."""
wait = compute_search_wait_seconds(
timestamps=[10.0] * 100,
last_search_at=50.0,
now=51.0,
max_per_window=0,
window_seconds=220,
min_delay_seconds=0,
)
assert wait == 0.0
# ---------------------------------------------------------------------------
# Min-delay between searches (the new knob — Bell Canada fix)
# ---------------------------------------------------------------------------
class TestMinDelayBetweenSearches:
def test_recent_last_search_blocks_for_remaining_delay(self):
"""User sets min_delay=5s. Last search 2s ago → wait 3s.
Smooths the burst pattern that trips Bell's anti-abuse even
when the sliding window isn't full."""
wait = compute_search_wait_seconds(
timestamps=[100.0, 102.0],
last_search_at=102.0,
now=104.0,
max_per_window=35,
window_seconds=220,
min_delay_seconds=5,
)
assert wait == pytest.approx(3.0, abs=1e-9)
def test_min_delay_already_elapsed_returns_zero(self):
"""Last search 10s ago, min-delay 5s → already cleared, no wait."""
wait = compute_search_wait_seconds(
timestamps=[100.0],
last_search_at=100.0,
now=110.0,
max_per_window=35,
window_seconds=220,
min_delay_seconds=5,
)
assert wait == 0.0
def test_min_delay_skipped_on_very_first_search(self):
"""`last_search_at == 0` means there's never been a search.
Don't gate the very first one — that would force an arbitrary
startup delay for no reason."""
wait = compute_search_wait_seconds(
timestamps=[],
last_search_at=0.0,
now=100.0,
max_per_window=35,
window_seconds=220,
min_delay_seconds=10,
)
assert wait == 0.0
# ---------------------------------------------------------------------------
# Both gates active — max wins
# ---------------------------------------------------------------------------
class TestMaxOfBothGates:
def test_returns_window_wait_when_window_wait_is_larger(self):
"""Window says wait 100s, min-delay says wait 5s → return 100s."""
timestamps = [0.0 + i for i in range(35)] # 0..34
# now = 5, window = 220, oldest = 0 → ages out at 220 → wait 215
# min_delay = 5, last = 4, now = 5 → wait 4
wait = compute_search_wait_seconds(
timestamps=timestamps,
last_search_at=4.0,
now=5.0,
max_per_window=35,
window_seconds=220,
min_delay_seconds=5,
)
assert wait == pytest.approx(215.0, abs=1e-9)
def test_returns_min_delay_wait_when_min_delay_is_larger(self):
"""Window not full → window wait = 0. Min-delay 30s, last 5s
ago → wait 25s. Min-delay drives it."""
wait = compute_search_wait_seconds(
timestamps=[100.0, 105.0],
last_search_at=105.0,
now=110.0,
max_per_window=35,
window_seconds=220,
min_delay_seconds=30,
)
assert wait == pytest.approx(25.0, abs=1e-9)
def test_both_zero_returns_zero(self):
"""Window not full + min-delay clear → zero. Sanity."""
wait = compute_search_wait_seconds(
timestamps=[100.0],
last_search_at=50.0,
now=200.0,
max_per_window=35,
window_seconds=220,
min_delay_seconds=10,
)
assert wait == 0.0
# ---------------------------------------------------------------------------
# Defensive — input shape variations
# ---------------------------------------------------------------------------
class TestDefensive:
def test_negative_min_delay_treated_as_disabled(self):
"""Defensive: a negative min-delay (somehow) shouldn't return
a negative wait or trigger weird behavior. Treat as disabled."""
wait = compute_search_wait_seconds(
timestamps=[],
last_search_at=99.0,
now=100.0,
max_per_window=35,
window_seconds=220,
min_delay_seconds=-5,
)
assert wait == 0.0
def test_returns_float(self):
"""Caller passes to asyncio.sleep which wants a float. Pin shape."""
wait = compute_search_wait_seconds(
timestamps=[],
last_search_at=0.0,
now=100.0,
max_per_window=35,
window_seconds=220,
min_delay_seconds=0,
)
assert isinstance(wait, float)

276
tests/test_tidal_album_tracks.py Normal file
View file

@ -0,0 +1,276 @@
"""Pin Tidal `get_album_tracks` — fetches every track on an album
with full artist + name + duration metadata hydrated.
Discord report: clicking 'Download All' on the Your Albums section
showed "Queuing..." but never actually queued any Tidal-only albums.
Root cause: `/api/discover/album/<source>/<album_id>` had no `tidal`
branch and tidal_client had no `get_album_tracks` method — the
frontend's trySources fell back to spotify/deezer which returned
None for Tidal-only IDs.
This test suite covers the new tidal_client method:
- Cursor-paginated walk of `/v2/albums/{id}/relationships/items`
- Track meta (trackNumber + volumeNumber for multi-disc)
- Batch hydration via `_get_tracks_batch` for artist/album names
- Sort by (disc_number, track_number) so the modal renders in
album order across multi-disc releases
- Empty / error paths return [] without raising
"""
from __future__ import annotations
from types import SimpleNamespace
from unittest.mock import MagicMock, patch
import pytest
from core.tidal_client import Track, TidalClient
def _make_client():
client = TidalClient.__new__(TidalClient)
client.access_token = "fake-token"
client.token_expires_at = 9_999_999_999
client.base_url = "https://openapi.tidal.com/v2"
client.alt_base_url = "https://api.tidal.com/v1"
client.session = MagicMock()
return client
class _FakeResp:
def __init__(self, status_code=200, json_body=None, text=""):
self.status_code = status_code
self._body = json_body if json_body is not None else {}
self.text = text or str(self._body)
def json(self):
return self._body
# ---------------------------------------------------------------------------
# Single-page album (12 tracks, single disc)
# ---------------------------------------------------------------------------
_SINGLE_PAGE = {
'data': [
{'id': '1001', 'type': 'tracks', 'meta': {'volumeNumber': 1, 'trackNumber': 1}},
{'id': '1002', 'type': 'tracks', 'meta': {'volumeNumber': 1, 'trackNumber': 2}},
{'id': '1003', 'type': 'tracks', 'meta': {'volumeNumber': 1, 'trackNumber': 3}},
],
'links': {}, # no `next` — single-page album
}
class TestSinglePageAlbum:
def test_walks_page_and_hydrates(self):
"""Happy path: 3-track album, single page, single disc.
IDs enumerated → batch hydrated → returned in album order."""
client = _make_client()
client.session.get = MagicMock(return_value=_FakeResp(200, _SINGLE_PAGE))
client._get_tracks_batch = MagicMock(return_value=[
Track(id='1001', name='Track One', artists=['Artist'], duration_ms=180000),
Track(id='1002', name='Track Two', artists=['Artist'], duration_ms=200000),
Track(id='1003', name='Track Three', artists=['Artist'], duration_ms=220000),
])
with patch('core.tidal_client.time.sleep'):
tracks = client.get_album_tracks('album-1')
assert [t.id for t in tracks] == ['1001', '1002', '1003']
assert [t.track_number for t in tracks] == [1, 2, 3]
# Single disc → all volumeNumber=1
assert all(t.disc_number == 1 for t in tracks)
def test_no_token_returns_empty_without_request(self):
"""Auth precheck failure short-circuits."""
client = _make_client()
client.session.get = MagicMock()
with patch.object(client, '_ensure_valid_token', return_value=False):
assert client.get_album_tracks('album-1') == []
client.session.get.assert_not_called()
def test_http_error_returns_empty(self):
client = _make_client()
client.session.get = MagicMock(return_value=_FakeResp(404, text='not found'))
with patch('core.tidal_client.time.sleep'):
assert client.get_album_tracks('album-1') == []
def test_429_raises_for_rate_limit_decorator(self):
"""The `rate_limited` decorator looks for '429' in the exception
message to trigger retry/backoff. Don't swallow rate-limit
responses — propagate so the decorator can handle them."""
client = _make_client()
client.session.get = MagicMock(return_value=_FakeResp(429, text='rate limited'))
with patch('core.tidal_client.time.sleep'):
with pytest.raises(Exception, match='429'):
client.get_album_tracks('album-1')
def test_skips_non_track_data_entries(self):
"""Forward-compat: schema additions might surface non-track
types alongside tracks — only collect entries with type='tracks'."""
client = _make_client()
client.session.get = MagicMock(return_value=_FakeResp(200, {
'data': [
{'id': '1', 'type': 'tracks', 'meta': {'trackNumber': 1, 'volumeNumber': 1}},
{'id': '99', 'type': 'videos', 'meta': {'trackNumber': 99}},
],
'links': {},
}))
client._get_tracks_batch = MagicMock(return_value=[
Track(id='1', name='Track', artists=['A'], duration_ms=100),
])
with patch('core.tidal_client.time.sleep'):
tracks = client.get_album_tracks('album-1')
assert len(tracks) == 1
assert tracks[0].id == '1'
# ---------------------------------------------------------------------------
# Multi-disc album — sort order matters
# ---------------------------------------------------------------------------
class TestMultiDiscAlbum:
def test_sorts_by_disc_then_track(self):
"""Reporter's albums could be multi-disc compilations. After
batch hydration the tracks may not be in album order
(filter[id] endpoint doesn't guarantee preservation). Verify
the final list is sorted by (disc, track) so the download
modal renders disc 1 → 2 in track order each."""
client = _make_client()
# Page returns IDs in scrambled order intentionally
client.session.get = MagicMock(return_value=_FakeResp(200, {
'data': [
{'id': 'd1t2', 'type': 'tracks', 'meta': {'volumeNumber': 1, 'trackNumber': 2}},
{'id': 'd2t1', 'type': 'tracks', 'meta': {'volumeNumber': 2, 'trackNumber': 1}},
{'id': 'd1t1', 'type': 'tracks', 'meta': {'volumeNumber': 1, 'trackNumber': 1}},
{'id': 'd2t2', 'type': 'tracks', 'meta': {'volumeNumber': 2, 'trackNumber': 2}},
],
'links': {},
}))
client._get_tracks_batch = MagicMock(return_value=[
# Batch endpoint may not preserve order — return scrambled too
Track(id='d2t1', name='D2T1', artists=['A'], duration_ms=100),
Track(id='d1t1', name='D1T1', artists=['A'], duration_ms=100),
Track(id='d2t2', name='D2T2', artists=['A'], duration_ms=100),
Track(id='d1t2', name='D1T2', artists=['A'], duration_ms=100),
])
with patch('core.tidal_client.time.sleep'):
tracks = client.get_album_tracks('album-1')
# Expect: disc 1 first (tracks 1,2), then disc 2 (tracks 1,2)
assert [t.id for t in tracks] == ['d1t1', 'd1t2', 'd2t1', 'd2t2']
assert [(t.disc_number, t.track_number) for t in tracks] == [
(1, 1), (1, 2), (2, 1), (2, 2),
]
# ---------------------------------------------------------------------------
# Multi-page album — cursor walk
# ---------------------------------------------------------------------------
class TestMultiPageAlbum:
def test_follows_cursor_chain(self):
"""Big album (>20 tracks) — cursor chain must be walked.
First page returns links.next, second page returns no next."""
client = _make_client()
page1 = {
'data': [
{'id': '1', 'type': 'tracks', 'meta': {'trackNumber': 1, 'volumeNumber': 1}},
{'id': '2', 'type': 'tracks', 'meta': {'trackNumber': 2, 'volumeNumber': 1}},
],
'links': {'next': '/albums/x/relationships/items?cursor=ABC'},
}
page2 = {
'data': [
{'id': '3', 'type': 'tracks', 'meta': {'trackNumber': 3, 'volumeNumber': 1}},
],
'links': {},
}
responses = iter([_FakeResp(200, page1), _FakeResp(200, page2)])
client.session.get = MagicMock(side_effect=lambda *a, **kw: next(responses))
client._get_tracks_batch = MagicMock(return_value=[
Track(id='1', name='T1', artists=['A'], duration_ms=100),
Track(id='2', name='T2', artists=['A'], duration_ms=100),
Track(id='3', name='T3', artists=['A'], duration_ms=100),
])
with patch('core.tidal_client.time.sleep'):
tracks = client.get_album_tracks('album-1')
assert [t.id for t in tracks] == ['1', '2', '3']
# Two page requests must have happened
assert client.session.get.call_count == 2
def test_limit_short_circuits_at_page_boundary(self):
"""`limit` arg caps the walk early — useful for callers that
only want a preview, not the full tracklist."""
client = _make_client()
page1 = {
'data': [
{'id': '1', 'type': 'tracks', 'meta': {'trackNumber': 1, 'volumeNumber': 1}},
{'id': '2', 'type': 'tracks', 'meta': {'trackNumber': 2, 'volumeNumber': 1}},
],
'links': {'next': '/albums/x/relationships/items?cursor=ABC'},
}
client.session.get = MagicMock(return_value=_FakeResp(200, page1))
client._get_tracks_batch = MagicMock(return_value=[
Track(id='1', name='T1', artists=['A'], duration_ms=100),
])
with patch('core.tidal_client.time.sleep'):
tracks = client.get_album_tracks('album-1', limit=1)
# Only one page fetched even though links.next was set
assert client.session.get.call_count == 1
assert len(tracks) == 1
# ---------------------------------------------------------------------------
# Batch hydration robustness
# ---------------------------------------------------------------------------
class TestHydrationRobustness:
def test_hydration_exception_returns_partial_results(self):
"""If one batch fails to hydrate, other batches still return.
Defensive against transient Tidal errors mid-walk on big albums."""
client = _make_client()
# Big single-page album → 21 IDs split into two batches (20 + 1)
big_page = {
'data': [
{'id': str(i), 'type': 'tracks', 'meta': {'trackNumber': i, 'volumeNumber': 1}}
for i in range(1, 22)
],
'links': {},
}
client.session.get = MagicMock(return_value=_FakeResp(200, big_page))
# First batch succeeds, second raises
def batch_side_effect(batch_ids):
if len(batch_ids) == 1: # The trailing batch
raise RuntimeError("transient")
return [
Track(id=tid, name=f'T{tid}', artists=['A'], duration_ms=100)
for tid in batch_ids
]
client._get_tracks_batch = MagicMock(side_effect=batch_side_effect)
with patch('core.tidal_client.time.sleep'):
tracks = client.get_album_tracks('album-1')
# 20 from the first batch — second batch failed but didn't crash
assert len(tracks) == 20
def test_no_track_ids_returns_empty_without_hydrating(self):
"""Empty album → no batch call (no point hydrating zero IDs)."""
client = _make_client()
client.session.get = MagicMock(return_value=_FakeResp(200, {'data': [], 'links': {}}))
client._get_tracks_batch = MagicMock()
with patch('core.tidal_client.time.sleep'):
tracks = client.get_album_tracks('album-1')
assert tracks == []
client._get_tracks_batch.assert_not_called()

442
tests/test_tidal_favorite_albums_artists.py Normal file
View file

@ -0,0 +1,442 @@
"""Pin Tidal favorite albums + artists fetch via V2 user-collection
endpoints.
Discord report: Discover → Your Albums section showed nothing for
Tidal users regardless of how many albums they'd favorited. Audit
found `get_favorite_albums` (and `get_favorite_artists`) called the
deprecated `/v2/favorites?filter[type]=ALBUMS|ARTISTS` endpoint
which returns 404 for personal favorites — that endpoint is scoped
to collections the third-party app created itself, not the user's
app-level favorites. The V1 fallback (`/v1/users/<id>/favorites/...`)
returns 403 for modern OAuth tokens because they carry
`collection.read` instead of the legacy `r_usr` scope.
Fix: rewire to the same V2 user-collection cursor-paginated
endpoints we shipped for tracks (issue #502):
- `/v2/userCollectionAlbums/me/relationships/items`
- `/v2/userCollectionArtists/me/relationships/items`
Plus per-resource batch hydration via `/v2/{albums|artists}` with
extended-include semantics (`include=artists,coverArt` for albums,
`include=profileArt` for artists) so artist names + image URLs come
back in a single request per batch instead of N+1 lookups.
These tests pin:
- Cursor walkers dispatch correct path + type to the generic
`_iter_collection_resource_ids` helper
- Batch hydrators parse JSON:API `data[]` + `included[]` into the
legacy return shape that `database.upsert_liked_album` /
`upsert_liked_artist` consume — preserves byte-identical wiring
in `web_server.py`'s discover aggregator
- Image URL resolution picks largest variant from artwork files[]
- Artist-name resolution falls through to '' when relationships
are missing (so the upsert path doesn't trip on None)
- Empty-input + HTTP-error paths return [] without raising
"""
from __future__ import annotations
from types import SimpleNamespace
from unittest.mock import MagicMock, patch
import pytest
from core.tidal_client import TidalClient
def _make_client():
"""Bare TidalClient with auth state primed — no real connection.
Mirrors the helper in test_tidal_collection_tracks.py."""
client = TidalClient.__new__(TidalClient)
client.access_token = "fake-token"
client.token_expires_at = 9_999_999_999
client.base_url = "https://openapi.tidal.com/v2"
client.alt_base_url = "https://api.tidal.com/v1"
client.session = MagicMock()
return client
class _FakeResp:
def __init__(self, status_code=200, json_body=None, text=""):
self.status_code = status_code
self._body = json_body if json_body is not None else {}
self.text = text or str(self._body)
def json(self):
return self._body
# ---------------------------------------------------------------------------
# Cursor-walker dispatch
# ---------------------------------------------------------------------------
class TestCollectionWalkerDispatch:
def test_album_iter_passes_album_path_and_type(self):
"""`_iter_collection_album_ids` must dispatch to the generic
walker with the albums path + 'albums' expected_type. If the
wrapper drifts (e.g. typoed path) the IDs come back empty."""
client = _make_client()
with patch.object(client, '_iter_collection_resource_ids',
return_value=['111', '222']) as mock_walk:
ids = client._iter_collection_album_ids(max_ids=50)
mock_walk.assert_called_once_with(
'userCollectionAlbums/me/relationships/items', 'albums', 50,
)
assert ids == ['111', '222']
def test_artist_iter_passes_artist_path_and_type(self):
client = _make_client()
with patch.object(client, '_iter_collection_resource_ids',
return_value=['17275']) as mock_walk:
ids = client._iter_collection_artist_ids()
mock_walk.assert_called_once_with(
'userCollectionArtists/me/relationships/items', 'artists', None,
)
assert ids == ['17275']
# ---------------------------------------------------------------------------
# Helper: included map + relationship resolution
# ---------------------------------------------------------------------------
class TestIncludedMaps:
def test_build_included_maps_groups_by_type(self):
included = [
{'id': 'a1', 'type': 'artists', 'attributes': {'name': 'Foo'}},
{'id': 'art1', 'type': 'artworks', 'attributes': {'files': []}},
{'id': 'a2', 'type': 'artists', 'attributes': {'name': 'Bar'}},
{'id': 'unknown1', 'type': 'something_else'},
{'type': 'artworks'}, # missing id — should be skipped
]
artists, artworks = TidalClient._build_included_maps(included)
assert set(artists.keys()) == {'a1', 'a2'}
assert set(artworks.keys()) == {'art1'}
assert artists['a1']['attributes']['name'] == 'Foo'
def test_first_artist_name_resolves_from_map(self):
artists_map = {'a1': {'attributes': {'name': 'Eminem'}}}
rels = {'artists': {'data': [{'id': 'a1', 'type': 'artists'}]}}
assert TidalClient._first_artist_name(rels, artists_map) == 'Eminem'
def test_first_artist_name_empty_when_no_refs(self):
"""Defensive: relationships block missing or empty → '' so
upsert path doesn't trip on None."""
assert TidalClient._first_artist_name({}, {}) == ''
assert TidalClient._first_artist_name(
{'artists': {'data': []}}, {}
) == ''
def test_first_artist_name_empty_when_unknown_id(self):
"""Artist ref points at an ID not in included map — fall
through to '' rather than crash."""
rels = {'artists': {'data': [{'id': 'missing'}]}}
artists_map = {'other': {'attributes': {'name': 'X'}}}
assert TidalClient._first_artist_name(rels, artists_map) == ''
def test_first_artwork_url_picks_first_file(self):
"""Tidal returns artwork files largest-first. Picking files[0]
gets the highest-resolution variant (typically 1280×1280)."""
artworks_map = {
'art1': {'attributes': {'files': [
{'href': 'https://big.jpg', 'meta': {'width': 1280}},
{'href': 'https://small.jpg', 'meta': {'width': 320}},
]}}
}
rel = {'data': [{'id': 'art1', 'type': 'artworks'}]}
url = TidalClient._first_artwork_url(rel, artworks_map)
assert url == 'https://big.jpg'
def test_first_artwork_url_none_when_no_relationship(self):
assert TidalClient._first_artwork_url({}, {}) is None
assert TidalClient._first_artwork_url({'data': []}, {}) is None
def test_first_artwork_url_none_when_no_files(self):
"""Defensive: artwork resource exists but has no files array.
Return None rather than IndexError."""
artworks_map = {'art1': {'attributes': {'files': []}}}
rel = {'data': [{'id': 'art1'}]}
assert TidalClient._first_artwork_url(rel, artworks_map) is None
# ---------------------------------------------------------------------------
# Batch hydration — albums
# ---------------------------------------------------------------------------
_ALBUM_BATCH_RESPONSE = {
'data': [
{
'id': '141121273',
'type': 'albums',
'attributes': {
'title': 'Mr. Morale & The Big Steppers',
'releaseDate': '2022-05-13',
'numberOfItems': 18,
},
'relationships': {
'artists': {'data': [{'id': '5034248', 'type': 'artists'}]},
'coverArt': {'data': [{'id': 'cover-uuid', 'type': 'artworks'}]},
},
},
{
'id': '999',
'type': 'albums',
'attributes': {'title': 'Album Without Artist or Cover'},
'relationships': {},
},
],
'included': [
{
'id': '5034248', 'type': 'artists',
'attributes': {'name': 'Kendrick Lamar'},
},
{
'id': 'cover-uuid', 'type': 'artworks',
'attributes': {'files': [
{'href': 'https://resources.tidal.com/images/cover/1280x1280.jpg'},
]},
},
],
}
class TestGetAlbumsBatch:
def test_parses_full_album_response(self):
client = _make_client()
client.session.get = MagicMock(
return_value=_FakeResp(200, _ALBUM_BATCH_RESPONSE)
)
results = client._get_albums_batch(['141121273', '999'])
assert len(results) == 2
# First album — full attributes resolved from included
first = results[0]
assert first['tidal_id'] == '141121273'
assert first['album_name'] == 'Mr. Morale & The Big Steppers'
assert first['artist_name'] == 'Kendrick Lamar'
assert first['image_url'] == 'https://resources.tidal.com/images/cover/1280x1280.jpg'
assert first['release_date'] == '2022-05-13'
assert first['total_tracks'] == 18
# Second album — missing relationships fall through to defaults
second = results[1]
assert second['album_name'] == 'Album Without Artist or Cover'
assert second['artist_name'] == ''
assert second['image_url'] is None
assert second['release_date'] == ''
assert second['total_tracks'] == 0
def test_empty_input_returns_empty_without_request(self):
client = _make_client()
client.session.get = MagicMock()
results = client._get_albums_batch([])
assert results == []
client.session.get.assert_not_called()
def test_http_error_returns_empty(self):
client = _make_client()
client.session.get = MagicMock(
return_value=_FakeResp(500, text='server error')
)
results = client._get_albums_batch(['111'])
assert results == []
def test_skips_data_entries_with_wrong_type(self):
"""Forward-compat: response shape might surface non-album
resources alongside the request — only collect entries whose
type is 'albums'."""
client = _make_client()
client.session.get = MagicMock(return_value=_FakeResp(200, {
'data': [
{'id': '1', 'type': 'albums', 'attributes': {'title': 'A'}, 'relationships': {}},
{'id': '2', 'type': 'tracks', 'attributes': {'title': 'Skip Me'}},
],
'included': [],
}))
results = client._get_albums_batch(['1', '2'])
assert len(results) == 1
assert results[0]['album_name'] == 'A'
def test_filter_id_param_is_comma_joined(self):
"""The Tidal API expects `filter[id]=a,b,c` — verify our
param construction. Drift here would break batching against
production silently."""
client = _make_client()
captured_params = {}
def fake_get(url, params=None, headers=None, timeout=None):
captured_params.update(params or {})
return _FakeResp(200, {'data': [], 'included': []})
client.session.get = MagicMock(side_effect=fake_get)
client._get_albums_batch(['111', '222', '333'])
assert captured_params['filter[id]'] == '111,222,333'
assert captured_params['include'] == 'artists,coverArt'
# ---------------------------------------------------------------------------
# Batch hydration — artists
# ---------------------------------------------------------------------------
_ARTIST_BATCH_RESPONSE = {
'data': [
{
'id': '17275',
'type': 'artists',
'attributes': {'name': 'Eminem'},
'relationships': {
'profileArt': {'data': [{'id': 'profile-uuid', 'type': 'artworks'}]},
},
},
],
'included': [
{
'id': 'profile-uuid', 'type': 'artworks',
'attributes': {'files': [
{'href': 'https://resources.tidal.com/images/profile/750x750.jpg'},
]},
},
],
}
class TestGetArtistsBatch:
def test_parses_full_artist_response(self):
client = _make_client()
client.session.get = MagicMock(
return_value=_FakeResp(200, _ARTIST_BATCH_RESPONSE)
)
results = client._get_artists_batch(['17275'])
assert len(results) == 1
assert results[0]['tidal_id'] == '17275'
assert results[0]['name'] == 'Eminem'
assert results[0]['image_url'] == 'https://resources.tidal.com/images/profile/750x750.jpg'
def test_empty_input_returns_empty_without_request(self):
client = _make_client()
client.session.get = MagicMock()
assert client._get_artists_batch([]) == []
client.session.get.assert_not_called()
def test_http_error_returns_empty(self):
client = _make_client()
client.session.get = MagicMock(
return_value=_FakeResp(404, text='not found')
)
assert client._get_artists_batch(['17275']) == []
def test_filter_id_and_include_params(self):
client = _make_client()
captured = {}
def fake_get(url, params=None, headers=None, timeout=None):
captured.update(params or {})
return _FakeResp(200, {'data': [], 'included': []})
client.session.get = MagicMock(side_effect=fake_get)
client._get_artists_batch(['17275', '721'])
assert captured['filter[id]'] == '17275,721'
assert captured['include'] == 'profileArt'
# ---------------------------------------------------------------------------
# Public methods — orchestrator behavior
# ---------------------------------------------------------------------------
class TestGetFavoriteAlbums:
def test_walks_then_batches_then_returns(self):
"""End-to-end: iter returns IDs, batch hydrates them, result
is the concatenation. Backward-compatible shape preserved
for `database.upsert_liked_album` callers."""
client = _make_client()
with patch.object(client, '_iter_collection_album_ids',
return_value=['1', '2', '3']) as mock_iter, \
patch.object(client, '_get_albums_batch',
return_value=[
{'tidal_id': '1', 'album_name': 'A',
'artist_name': 'X', 'image_url': 'u',
'release_date': '2020', 'total_tracks': 10},
{'tidal_id': '2', 'album_name': 'B',
'artist_name': 'Y', 'image_url': None,
'release_date': '', 'total_tracks': 0},
]) as mock_batch:
results = client.get_favorite_albums(limit=100)
mock_iter.assert_called_once_with(max_ids=100)
# Single batch call since 3 IDs fit in one BATCH_SIZE chunk (20)
assert mock_batch.call_count == 1
assert len(results) == 2
assert results[0]['tidal_id'] == '1'
# Verify shape compatibility with upsert_liked_album kwargs
expected_keys = {'tidal_id', 'album_name', 'artist_name',
'image_url', 'release_date', 'total_tracks'}
assert set(results[0].keys()) == expected_keys
def test_no_ids_returns_empty_without_batch(self):
client = _make_client()
with patch.object(client, '_iter_collection_album_ids', return_value=[]), \
patch.object(client, '_get_albums_batch') as mock_batch:
assert client.get_favorite_albums() == []
mock_batch.assert_not_called()
def test_chunks_into_batch_size(self):
"""41 IDs at BATCH_SIZE 20 → three batches of 20/20/1.
Tidal's filter[id] cap is the per-request limit; orchestrator
must respect it."""
client = _make_client()
ids = [str(i) for i in range(41)]
captured_batches = []
def fake_batch(batch):
captured_batches.append(list(batch))
return [{'tidal_id': b, 'album_name': f'A{b}', 'artist_name': '',
'image_url': None, 'release_date': '', 'total_tracks': 0}
for b in batch]
with patch.object(client, '_iter_collection_album_ids', return_value=ids), \
patch.object(client, '_get_albums_batch', side_effect=fake_batch):
results = client.get_favorite_albums()
assert len(results) == 41
assert [len(b) for b in captured_batches] == [20, 20, 1]
class TestGetFavoriteArtists:
def test_walks_then_batches(self):
client = _make_client()
with patch.object(client, '_iter_collection_artist_ids',
return_value=['17275']) as mock_iter, \
patch.object(client, '_get_artists_batch',
return_value=[{'tidal_id': '17275', 'name': 'Eminem',
'image_url': 'https://eminem.jpg'}]) as mock_batch:
results = client.get_favorite_artists(limit=200)
mock_iter.assert_called_once_with(max_ids=200)
mock_batch.assert_called_once()
assert len(results) == 1
assert results[0]['name'] == 'Eminem'
# Backward-compat shape — exactly the keys the prior
# implementation returned
assert set(results[0].keys()) == {'tidal_id', 'name', 'image_url'}
def test_no_ids_returns_empty(self):
client = _make_client()
with patch.object(client, '_iter_collection_artist_ids', return_value=[]), \
patch.object(client, '_get_artists_batch') as mock_batch:
assert client.get_favorite_artists() == []
mock_batch.assert_not_called()
def test_swallows_iter_exception_returns_empty(self):
"""Defensive: if the cursor walker blows up mid-page, the
public method should return [] (no partial corruption of the
liked-artists table)."""
client = _make_client()
with patch.object(client, '_iter_collection_artist_ids',
side_effect=RuntimeError('boom')):
assert client.get_favorite_artists() == []

192
web_server.py
View file

@ -40,7 +40,7 @@ logger = setup_logging(_log_level, _log_path)
# App version — single source of truth for backup metadata, system-info, update check, etc.
# Semver: MAJOR.MINOR.PATCH. Bump at each dev→main release.
_SOULSYNC_BASE_VERSION = "2.5.0"
_SOULSYNC_BASE_VERSION = "2.5.1"
def _build_version_string():
"""Append short commit hash to version when available (e.g. 2.35+abc1234)."""
@ -9270,14 +9270,37 @@ def download_discography(artist_id):
from database.music_database import MusicDatabase
from core.metadata.album_tracks import get_artist_album_tracks
from core.metadata.discography_filters import (
content_type_skip_reason,
load_global_content_filter_settings,
track_already_owned,
track_artist_matches,
)
db = MusicDatabase()
profile_id = get_current_profile_id()
# Honor the same content-type filters the watchlist scanner uses
# (issue #559). One read at the top — settings don't change
# mid-stream and the four bool reads aren't worth re-running per
# track.
content_settings = load_global_content_filter_settings(config_manager)
# Library-ownership check uses the active media server so the
# match is scoped to the same source whose tracks the user can
# actually see in their library. None falls through to a
# cross-server search inside check_track_exists.
active_server = None
try:
active_server = config_manager.get_active_media_server()
except Exception as e:
logger.debug("active media server lookup failed: %s", e)
total_added = 0
total_skipped = 0
total_skipped_artist = 0
total_skipped_filter = 0
total_skipped_owned = 0
def generate_ndjson():
nonlocal total_added, total_skipped
nonlocal total_added, total_skipped, total_skipped_artist, total_skipped_filter, total_skipped_owned
for entry in album_entries:
album_id = entry['id']
@ -9325,6 +9348,9 @@ def download_discography(artist_id):
added = 0
skipped = 0
skipped_artist = 0
skipped_filter = 0
skipped_owned = 0
for track in tracks:
track_name = track.get('name', '')
@ -9333,6 +9359,33 @@ def download_discography(artist_id):
track_artists = track.get('artists', []) or album_artists
track_id = track.get('id', '')
# Issue #559: drop tracks where the requested
# artist isn't in the track's artists list
# (cross-artist compilation / appears_on
# contamination). Keeps features.
if not track_artist_matches(track_artists, hint_artist):
skipped_artist += 1
continue
# Issue #559: honor watchlist global content-type
# filters (live / remix / acoustic / instrumental)
# for one-off discography downloads too — same
# contract as the discography backfill repair job.
skip_reason = content_type_skip_reason(track_name, album_name, content_settings)
if skip_reason:
skipped_filter += 1
continue
# Skowl (Discord): clicking Download Discography
# twice re-queued every track because add_to_wishlist
# only dedups against the wishlist, not the library.
# Same library-ownership check the discography
# backfill repair job uses. Format-agnostic so
# Blasphemy mode (FLAC→MP3) doesn't false-miss.
if track_already_owned(db, track_name, hint_artist, album_name, active_server):
skipped_owned += 1
continue
spotify_track_data = {
'id': track_id,
'name': track_name,
@ -9379,8 +9432,14 @@ def download_discography(artist_id):
total_added += added
total_skipped += skipped
total_skipped_artist += skipped_artist
total_skipped_filter += skipped_filter
total_skipped_owned += skipped_owned
logger.warning(
f"[Discography] {album_name} ({resolved_source}): {added} added, {skipped} skipped"
f"[Discography] {album_name} ({resolved_source}): {added} added, "
f"{skipped} skipped (wishlist), {skipped_artist} skipped (artist mismatch), "
f"{skipped_filter} skipped (content filter), "
f"{skipped_owned} skipped (already in library)"
)
yield json.dumps({
"album_id": album_id,
@ -9388,6 +9447,9 @@ def download_discography(artist_id):
"status": "done",
"tracks_added": added,
"tracks_skipped": skipped,
"tracks_skipped_artist": skipped_artist,
"tracks_skipped_filter": skipped_filter,
"tracks_skipped_owned": skipped_owned,
"tracks_total": len(tracks),
"source": resolved_source,
}) + '\n'
@ -9402,12 +9464,17 @@ def download_discography(artist_id):
logger.warning(
f"[Discography] Complete for {artist_name}: {total_added} tracks added, "
f"{total_skipped} skipped across {len(album_entries)} albums"
f"{total_skipped} skipped (wishlist), {total_skipped_artist} skipped (artist mismatch), "
f"{total_skipped_filter} skipped (content filter), "
f"{total_skipped_owned} skipped (already in library) across {len(album_entries)} albums"
)
yield json.dumps({
"status": "complete",
"total_added": total_added,
"total_skipped": total_skipped,
"total_skipped_artist": total_skipped_artist,
"total_skipped_filter": total_skipped_filter,
"total_skipped_owned": total_skipped_owned,
"total_albums": len(album_entries),
}) + '\n'
@ -20130,6 +20197,81 @@ def get_discover_album(source, album_id):
'source': fallback_source,
})
elif source == 'tidal':
# Tidal albums from Your Albums (sourced via the V2 user-
# collection endpoint). Two-call resolution: get_album for
# metadata, get_album_tracks for the cursor-paginated
# tracklist. `get_album_tracks` returns `Track` objects
# with `track_number` / `disc_number` annotated so the
# download modal renders in album order across multi-disc
# releases. Serialise to the same shape Spotify/Deezer
# return so the frontend track-mapping stays uniform.
if not tidal_client or not tidal_client.is_authenticated():
return jsonify({"error": "Tidal not authenticated"}), 401
album_meta = tidal_client.get_album(album_id)
tidal_tracks = tidal_client.get_album_tracks(album_id)
if not album_meta and not tidal_tracks:
return jsonify({"error": "Tidal album not found"}), 404
album_name = (album_meta or {}).get('title') or request.args.get('name', '')
release_date = (album_meta or {}).get('releaseDate', '')
total_tracks = (album_meta or {}).get('numberOfItems') or len(tidal_tracks)
album_artist_name = request.args.get('artist', '')
# Build cover image URL from the album metadata. Tidal
# exposes cover art via the `coverArt` relationship which
# `get_album` doesn't fetch (it's a one-shot attributes
# call). Best-effort: request it inline.
cover_url = ''
try:
cover_resp = tidal_client.session.get(
f"{tidal_client.base_url}/albums/{album_id}",
params={'countryCode': 'US', 'include': 'coverArt'},
headers={'accept': 'application/vnd.api+json'},
timeout=10,
)
if cover_resp.status_code == 200:
payload = cover_resp.json()
_, artworks = tidal_client._build_included_maps(payload.get('included', []))
cover_rel = (payload.get('data') or {}).get('relationships', {}).get('coverArt', {})
cover_url = tidal_client._first_artwork_url(cover_rel, artworks) or ''
except Exception as e:
logger.debug(f"Tidal cover-art resolve failed for album {album_id}: {e}")
tracks_out = []
for t in tidal_tracks:
tracks_out.append({
'id': t.id,
'name': t.name,
'artists': [{'name': a} for a in (t.artists or [])],
'duration_ms': t.duration_ms,
'track_number': getattr(t, 'track_number', 0),
'disc_number': getattr(t, 'disc_number', 1),
})
# Album-level artist name preference: explicit ?artist=
# query (passed by frontend with the saved-album row) wins
# over guessing from the first track. The saved-album row
# already resolved the canonical artist via the V2
# collection endpoint.
if not album_artist_name and tidal_tracks:
first_artists = tidal_tracks[0].artists or []
album_artist_name = first_artists[0] if first_artists else ''
return jsonify({
'id': album_id,
'name': album_name or 'Unknown Album',
'artists': [{'name': album_artist_name}] if album_artist_name else [],
'release_date': release_date,
'total_tracks': total_tracks,
'album_type': 'album',
'images': [{'url': cover_url}] if cover_url else [],
'tracks': tracks_out,
'source': 'tidal',
})
elif source == 'discogs':
# Discogs release detail. release_id comes from the Your
# Albums Discogs source. Tracklist needs normalizing —
@ -23836,10 +23978,11 @@ def _build_sync_deps():
)
def _run_sync_task(playlist_id, playlist_name, tracks_json, automation_id=None, profile_id=1, playlist_image_url=''):
def _run_sync_task(playlist_id, playlist_name, tracks_json, automation_id=None, profile_id=1, playlist_image_url='', sync_mode='replace'):
return _discovery_sync.run_sync_task(
playlist_id, playlist_name, tracks_json, automation_id, profile_id, playlist_image_url,
_build_sync_deps(),
sync_mode=sync_mode,
)
@ -23855,14 +23998,22 @@ def start_playlist_sync():
playlist_name = data.get('playlist_name')
tracks_json = data.get('tracks') # Pass the full track list
playlist_image_url = data.get('image_url', '')
# 'replace' (default) deletes the server playlist and recreates it from
# the source. 'append' preserves user-added tracks already on the server
# playlist — only adds tracks that aren't there yet. Per-server clients
# implement append via native add APIs (Plex addItems, Jellyfin POST
# /Playlists/<id>/Items, Navidrome updatePlaylist?songIdToAdd=...).
sync_mode = data.get('sync_mode', 'replace')
if sync_mode not in ('replace', 'append'):
sync_mode = 'replace'
if not all([playlist_id, playlist_name, tracks_json]):
return jsonify({"success": False, "error": "Missing playlist_id, name, or tracks."}), 400
# Add activity for sync start
add_activity_item("", "Spotify Sync Started", f"'{playlist_name}' - {len(tracks_json)} tracks", "Now")
add_activity_item("", "Spotify Sync Started", f"'{playlist_name}' - {len(tracks_json)} tracks ({sync_mode})", "Now")
logger.info(f"Starting playlist sync for '{playlist_name}' with {len(tracks_json)} tracks")
logger.info(f"Starting playlist sync for '{playlist_name}' with {len(tracks_json)} tracks (mode: {sync_mode})")
logger.debug(f"Request parsed at {time.strftime('%H:%M:%S')} (took {(time.time()-request_start_time)*1000:.1f}ms)")
with sync_lock:
@ -23875,7 +24026,7 @@ def start_playlist_sync():
# Submit the task to the thread pool (capture profile_id while still in request context)
_sync_profile_id = get_current_profile_id()
thread_submit_time = time.time()
future = sync_executor.submit(_run_sync_task, playlist_id, playlist_name, tracks_json, None, _sync_profile_id, playlist_image_url)
future = sync_executor.submit(_run_sync_task, playlist_id, playlist_name, tracks_json, None, _sync_profile_id, playlist_image_url, sync_mode)
active_sync_workers[playlist_id] = future
thread_submit_duration = (time.time() - thread_submit_time) * 1000
logger.info(f"⏱️ [TIMING] Thread submitted at {time.strftime('%H:%M:%S')} (took {thread_submit_duration:.1f}ms)")
@ -34541,14 +34692,35 @@ def auto_import_approve_all():
@app.route('/api/auto-import/clear-completed', methods=['POST'])
def auto_import_clear_completed():
"""Remove completed/imported items from history."""
"""Remove completed/imported items from history.
`processing` rows are included so zombie entries (server restarted
mid-import → `_record_in_progress` row never got finalized) get
swept. Live in-flight imports are protected by intersecting against
`_snapshot_active()` — anything currently registered in the worker's
`_active_imports` map keeps its row. `pending_review` is left out so
user still has to approve/reject those explicitly.
"""
if not auto_import_worker:
return jsonify({"success": False, "error": "Auto-import not available"}), 500
try:
active_hashes = {e['folder_hash'] for e in auto_import_worker._snapshot_active()}
db = get_database()
with db._get_connection() as conn:
cursor = conn.cursor()
cursor.execute("DELETE FROM auto_import_history WHERE status IN ('completed', 'approved', 'failed', 'needs_identification', 'rejected')")
base_sql = (
"DELETE FROM auto_import_history "
"WHERE status IN ('completed', 'approved', 'failed', "
"'needs_identification', 'rejected', 'processing')"
)
if active_hashes:
placeholders = ','.join('?' * len(active_hashes))
cursor.execute(
f"{base_sql} AND folder_hash NOT IN ({placeholders})",
tuple(active_hashes),
)
else:
cursor.execute(base_sql)
count = cursor.rowcount
conn.commit()
return jsonify({"success": True, "count": count})

10
webui/index.html
View file

@ -4329,6 +4329,16 @@
<small class="settings-hint">Extra time to wait
for late results (5-60 seconds)</small>
</div>
<div class="form-group">
<label>Minimum Delay Between Searches (seconds):</label>
<input type="number" id="soulseek-search-min-delay-seconds" placeholder="0"
min="0" max="60" value="0">
<small class="settings-hint">Forces a gap between
consecutive searches. Smooths burst patterns
that trip ISP anti-abuse (e.g. Bell Canada
cuts the WAN after rapid peer-connection
spikes). 0 disables.</small>
</div>
<div class="form-group">
<label>Minimum Peer Upload Speed:</label>
<select id="soulseek-min-peer-speed">

339
webui/static/discover.js
View file

@ -1085,6 +1085,7 @@ async function openYourAlbumDownload(index) {
const trySources = [];
if (album.spotify_album_id) trySources.push(['spotify', album.spotify_album_id]);
if (album.deezer_album_id) trySources.push(['deezer', album.deezer_album_id]);
if (album.tidal_album_id) trySources.push(['tidal', album.tidal_album_id]);
if (discogsId) trySources.push(['discogs', discogsId]);
for (const [src, id] of trySources) {
@ -1283,6 +1284,13 @@ async function _yaaSourcesSave() {
}
async function downloadMissingYourAlbums() {
// Opens the same selectable-grid modal pattern used by Download
// Discography on the library page. User picks which missing albums
// they want, clicks Add to Wishlist, each album's tracks get
// resolved + added to the wishlist for the existing auto-download
// processor to pick up. Replaces the prior per-album direct-download
// loop which was silently failing — actual downloads should go
// through the wishlist queue, not bypass it.
try {
const resp = await fetch('/api/discover/your-albums?page=1&per_page=1000&status=missing');
const data = await resp.json();
@ -1291,50 +1299,297 @@ async function downloadMissingYourAlbums() {
return;
}
const missing = data.albums.filter(a => !a.in_library);
if (missing.length === 0) { showToast('All albums are already in your library!', 'success'); return; }
if (!confirm(`Download ${missing.length} missing album${missing.length > 1 ? 's' : ''} from your saved albums?`)) return;
showToast(`Starting download for ${missing.length} albums...`, 'info');
for (let i = 0; i < missing.length; i++) {
const album = missing[i];
try {
showToast(`Queuing ${i + 1}/${missing.length}: ${album.album_name}`, 'info');
const nameParams = new URLSearchParams({ name: album.album_name || '', artist: album.artist_name || '' });
let albumData = null;
if (album.spotify_album_id) {
const r = await fetch(`/api/discover/album/spotify/${album.spotify_album_id}?${nameParams}`);
if (r.ok) albumData = await r.json();
}
if (!albumData && album.deezer_album_id) {
const r = await fetch(`/api/discover/album/deezer/${album.deezer_album_id}?${nameParams}`);
if (r.ok) albumData = await r.json();
}
if (!albumData || !albumData.tracks || albumData.tracks.length === 0) continue;
const tracks = albumData.tracks.map(track => {
let artists = track.artists || albumData.artists || [{ name: album.artist_name }];
if (Array.isArray(artists)) artists = artists.map(a => a.name || a);
return {
id: track.id, name: track.name, artists,
album: {
id: albumData.id, name: albumData.name, album_type: albumData.album_type || 'album',
total_tracks: albumData.total_tracks || 0, release_date: albumData.release_date || '',
images: albumData.images || []
},
duration_ms: track.duration_ms || 0, track_number: track.track_number || 0
};
});
const virtualId = `your_albums_${album.spotify_album_id || album.deezer_album_id || i}`;
await openDownloadMissingModalForYouTube(virtualId, albumData.name, tracks,
{ name: album.artist_name, source: albumData.source || 'spotify' },
{
id: albumData.id, name: albumData.name, album_type: albumData.album_type || 'album',
total_tracks: albumData.total_tracks || 0, release_date: albumData.release_date || '',
images: albumData.images || []
}
);
} catch (err) { console.error(`Error queuing ${album.album_name}:`, err); }
if (missing.length === 0) {
showToast('All albums are already in your library!', 'success');
return;
}
_openYourAlbumsBatchModal(missing);
} catch (e) {
console.error('Error downloading missing your albums:', e);
console.error('Error loading missing your albums:', e);
showToast(`Error: ${e.message}`, 'error');
}
}
// Map a Your Albums row to the single best source-id the
// /api/artist/<id>/download-discography endpoint can resolve. Each row
// in the missing list typically only has one populated source-id (the
// service it was saved on), so this is just a priority pick.
function _yourAlbumsPickSource(album) {
if (album.spotify_album_id) return { id: String(album.spotify_album_id), source: 'spotify' };
if (album.deezer_album_id) return { id: String(album.deezer_album_id), source: 'deezer' };
if (album.tidal_album_id) return { id: String(album.tidal_album_id), source: 'tidal' };
const discogsId = album.discogs_release_id || album.discogs_id;
if (discogsId) return { id: String(discogsId), source: 'discogs' };
return null;
}
function _openYourAlbumsBatchModal(missingAlbums) {
// Reuses the .discog-modal styling from the library Download
// Discography flow — same checkboxes, same Select All / Deselect
// All semantics, same footer. Single difference: each card carries
// its own artist+source (multi-artist) instead of all being one
// artist's discography.
const existing = document.getElementById('your-albums-batch-modal-overlay');
if (existing) existing.remove();
// Stash the source-id picks on the cards so the submit handler
// can build the per-album payload without re-mapping the array.
const rows = missingAlbums
.map((a, i) => ({ ...a, _src: _yourAlbumsPickSource(a), _index: i }))
.filter(a => a._src); // Skip albums with no usable source-id
if (rows.length === 0) {
showToast('No missing albums have a usable source ID to resolve', 'warning');
return;
}
const overlay = document.createElement('div');
overlay.className = 'discog-modal-overlay';
overlay.id = 'your-albums-batch-modal-overlay';
overlay.innerHTML = `
<div class="discog-modal">
<div class="discog-modal-hero">
<div class="discog-modal-hero-overlay"></div>
<div class="discog-modal-hero-content">
<h2 class="discog-modal-title">Add Missing Albums to Wishlist</h2>
<p class="discog-modal-artist">${rows.length} albums missing from your library</p>
</div>
<button class="discog-modal-close" onclick="_closeYourAlbumsBatchModal()">&times;</button>
</div>
<div class="discog-filter-bar">
<div class="discog-filters"></div>
<div class="discog-select-actions">
<button class="discog-select-btn" onclick="_yourAlbumsBatchSelectAll(true)">Select All</button>
<button class="discog-select-btn" onclick="_yourAlbumsBatchSelectAll(false)">Deselect All</button>
</div>
</div>
<div class="discog-grid" id="your-albums-batch-grid">
${rows.map((r, i) => _renderYourAlbumsBatchCard(r, i)).join('')}
</div>
<div class="discog-progress" id="your-albums-batch-progress" style="display:none;"></div>
<div class="discog-footer" id="your-albums-batch-footer">
<div class="discog-footer-info" id="your-albums-batch-footer-info"></div>
<div class="discog-footer-actions">
<button class="discog-cancel-btn" onclick="_closeYourAlbumsBatchModal()">Cancel</button>
<button class="discog-submit-btn" id="your-albums-batch-submit-btn">
<span class="discog-submit-icon">⬇</span>
<span id="your-albums-batch-submit-text">Add to Wishlist</span>
</button>
</div>
</div>
</div>
`;
document.body.appendChild(overlay);
// Stash row data on the overlay for the submit handler — keeps the
// multi-artist source info available without re-fetching.
overlay._yourAlbumsRows = rows;
requestAnimationFrame(() => overlay.classList.add('visible'));
_updateYourAlbumsBatchFooterCount();
document.getElementById('your-albums-batch-submit-btn')?.addEventListener('click', (e) => {
e.stopPropagation();
_startYourAlbumsBatchAddToWishlist();
});
}
function _renderYourAlbumsBatchCard(row, index) {
const albumName = row.album_name || '';
const artistName = row.artist_name || '';
const year = row.release_date ? row.release_date.substring(0, 4) : '';
const tracks = row.total_tracks || 0;
const img = row.image_url || '';
const src = row._src?.source || '';
return `
<label class="discog-card" data-type="album" style="animation-delay:${index * 0.03}s">
<input type="checkbox" class="your-albums-batch-cb"
data-row-index="${row._index}" data-tracks="${tracks}" checked
onchange="_updateYourAlbumsBatchFooterCount()">
<div class="discog-card-art">
${img ? `<img src="${escapeHtml(img)}" alt="" loading="lazy">` : '<div class="discog-card-art-placeholder">🎵</div>'}
</div>
<div class="discog-card-info">
<div class="discog-card-title">${escapeHtml(albumName)}</div>
<div class="discog-card-meta">${escapeHtml(artistName)}${year ? ' · ' + year : ''}${tracks ? ' · ' + tracks + ' tracks' : ''}${src ? ' · ' + src : ''}</div>
</div>
<div class="discog-card-check"></div>
</label>
`;
}
function _yourAlbumsBatchSelectAll(select) {
document.querySelectorAll('.your-albums-batch-cb').forEach(cb => {
if (cb.closest('.discog-card').style.display !== 'none') cb.checked = select;
});
_updateYourAlbumsBatchFooterCount();
}
function _updateYourAlbumsBatchFooterCount() {
const checked = document.querySelectorAll('.your-albums-batch-cb:checked');
let releases = 0, tracks = 0;
checked.forEach(cb => {
if (cb.closest('.discog-card').style.display !== 'none') {
releases++;
tracks += parseInt(cb.dataset.tracks) || 0;
}
});
const info = document.getElementById('your-albums-batch-footer-info');
const btn = document.getElementById('your-albums-batch-submit-text');
if (info) info.textContent = `${releases} album${releases !== 1 ? 's' : ''}${tracks ? ' · ' + tracks + ' tracks' : ''}`;
if (btn) btn.textContent = releases > 0 ? `Add ${releases} to Wishlist` : 'Select albums';
const submitBtn = document.getElementById('your-albums-batch-submit-btn');
if (submitBtn) submitBtn.disabled = releases === 0;
}
function _closeYourAlbumsBatchModal() {
const overlay = document.getElementById('your-albums-batch-modal-overlay');
if (overlay) {
overlay.classList.remove('visible');
setTimeout(() => overlay.remove(), 200);
}
}
async function _startYourAlbumsBatchAddToWishlist() {
const overlay = document.getElementById('your-albums-batch-modal-overlay');
if (!overlay) return;
const rows = overlay._yourAlbumsRows || [];
// Collect selected row indices from the checked checkboxes.
const selectedRowIndices = [];
document.querySelectorAll('.your-albums-batch-cb:checked').forEach(cb => {
if (cb.closest('.discog-card').style.display !== 'none') {
selectedRowIndices.push(parseInt(cb.dataset.rowIndex));
}
});
const selected = rows.filter(r => selectedRowIndices.includes(r._index));
if (selected.length === 0) return;
// Switch to progress view.
const grid = document.getElementById('your-albums-batch-grid');
const progress = document.getElementById('your-albums-batch-progress');
const footer = document.getElementById('your-albums-batch-footer');
const filterBar = overlay.querySelector('.discog-filter-bar');
if (grid) grid.style.display = 'none';
if (filterBar) filterBar.style.display = 'none';
if (progress) {
progress.style.display = '';
progress.innerHTML = '';
}
selected.forEach(row => {
const item = document.createElement('div');
item.className = 'discog-progress-item active';
item.id = `your-albums-batch-prog-${row._src.source}-${row._src.id}`;
item.innerHTML = `
<div class="discog-prog-art">${row.image_url ? `<img src="${escapeHtml(row.image_url)}">` : '🎵'}</div>
<div class="discog-prog-info">
<div class="discog-prog-title">${escapeHtml(row.album_name || '')}</div>
<div class="discog-prog-status">Waiting...</div>
</div>
<div class="discog-prog-icon"><div class="discog-spinner"></div></div>
`;
progress.appendChild(item);
});
const submitBtn = document.getElementById('your-albums-batch-submit-btn');
if (submitBtn) submitBtn.style.display = 'none';
if (footer) {
const info = document.getElementById('your-albums-batch-footer-info');
if (info) info.textContent = 'Processing... this may take a moment';
}
// Build per-album payload matching the discography endpoint contract.
// URL artist_id is functionally unused by the endpoint when per-album
// metadata is supplied — backend resolves each album through its own
// `source` + `artist_name`. Placeholder 'your-albums' makes the route
// match without picking an arbitrary library artist.
const albumsPayload = selected.map(r => ({
id: r._src.id,
name: r.album_name || '',
artist_name: r.artist_name || '',
source: r._src.source,
}));
try {
const response = await fetch(`/api/artist/your-albums/download-discography`, {
method: 'POST',
headers: { 'Content-Type': 'application/json' },
body: JSON.stringify({
albums: albumsPayload,
artist_name: 'Your Albums',
source: null,
}),
});
const reader = response.body.getReader();
const decoder = new TextDecoder();
let buffer = '';
let totalAdded = 0, totalSkipped = 0;
while (true) {
const { done, value } = await reader.read();
if (done) break;
buffer += decoder.decode(value, { stream: true });
const lines = buffer.split('\n');
buffer = lines.pop();
for (const line of lines) {
if (!line.trim()) continue;
try {
const data = JSON.parse(line);
if (data.status === 'complete') {
totalAdded = data.total_added || 0;
totalSkipped = data.total_skipped || 0;
} else if (data.album_id) {
// Find the matching progress card — match by composite source-id
// pair since the same album_id could appear across sources.
const matching = selected.find(s => s._src.id === String(data.album_id));
if (matching) {
const item = document.getElementById(`your-albums-batch-prog-${matching._src.source}-${matching._src.id}`);
if (item) {
const status = item.querySelector('.discog-prog-status');
const icon = item.querySelector('.discog-prog-icon');
if (data.status === 'done') {
if (status) status.textContent = `${data.tracks_added || 0} added · ${data.tracks_skipped || 0} skipped`;
if (icon) icon.innerHTML = '✓';
item.classList.add('done');
item.classList.remove('active');
} else if (data.status === 'error') {
if (status) status.textContent = `Error: ${data.message || 'unknown'}`;
if (icon) icon.innerHTML = '✗';
item.classList.add('error');
item.classList.remove('active');
}
}
}
}
} catch (parseErr) {
console.debug('your-albums batch ndjson parse:', parseErr);
}
}
}
if (footer) {
const info = document.getElementById('your-albums-batch-footer-info');
if (info) info.textContent = `${totalAdded} tracks added to wishlist · ${totalSkipped} skipped`;
}
if (submitBtn) {
submitBtn.style.display = '';
submitBtn.disabled = true;
const txt = document.getElementById('your-albums-batch-submit-text');
if (txt) txt.textContent = 'Done';
}
showToast(`${totalAdded} tracks added to wishlist`, totalAdded > 0 ? 'success' : 'info');
} catch (e) {
console.error('Error adding your albums to wishlist:', e);
showToast(`Error: ${e.message}`, 'error');
}
}

19
webui/static/downloads.js
View file

@ -4098,9 +4098,21 @@ async function cancelTrackDownload(playlistId, trackIndex) {
}
// Find and REPLACE the old startPlaylistSyncFromModal function
async function startPlaylistSync(playlistId) {
async function startPlaylistSync(playlistId, syncModeOverride = null) {
const startTime = Date.now();
console.log(`🚀 [${new Date().toTimeString().split(' ')[0]}] Starting sync for playlist: ${playlistId}`);
// Sync mode: prefer explicit override (e.g. from automation/discover code paths
// that don't render the modal selector), else read the per-playlist <select>
// rendered next to the Sync button, else default 'replace' to preserve
// historical behavior for any caller that hasn't been updated yet.
let syncMode = syncModeOverride;
if (!syncMode) {
const modeSelect = document.getElementById(`sync-mode-${playlistId}`);
syncMode = (modeSelect && modeSelect.value) || 'replace';
}
if (syncMode !== 'replace' && syncMode !== 'append') {
syncMode = 'replace';
}
console.log(`🚀 [${new Date().toTimeString().split(' ')[0]}] Starting sync for playlist: ${playlistId} (mode: ${syncMode})`);
const playlist = spotifyPlaylists.find(p => p.id === playlistId);
if (!playlist) {
console.error(`❌ Could not find playlist data for ID: ${playlistId}`);
@ -4160,7 +4172,8 @@ async function startPlaylistSync(playlistId) {
playlist_id: playlist.id,
playlist_name: playlist.name,
tracks: tracks, // Send the full track list
image_url: playlist.image_url || ''
image_url: playlist.image_url || '',
sync_mode: syncMode
})
});

20
webui/static/helper.js
View file

@ -3413,6 +3413,24 @@ function closeHelperSearch() {
// projects that span multiple commits before shipping. Strip the flag at
// release time and add a real `date:` line at the top of the version block.
const WHATS_NEW = {
'2.5.1': [
// --- May 12, 2026 — 2.5.1 release ---
{ date: 'May 12, 2026 — 2.5.1 release' },
{ title: 'Soulseek: Min Delay Between Searches (Fixes ISP Anti-Abuse Trips)', desc: 'reddit report (yelomelo95, bell canada): isp anti-abuse cuts the wan after a burst of slskd searches. soulsync\'s sliding-window cap (35 searches per 220s) prevented soulseek-side bans but allowed all 35 in rapid succession — which is exactly the connection-burst pattern that trips isp throttling. new knob on settings → connections → soulseek: minimum delay between searches (default 0 = disabled, preserves prior behavior). set it to 5-10 seconds if your isp throttles peer-connection spikes. throttle math lifted to a pure `compute_search_wait_seconds` helper so the gate logic is testable independent of asyncio.sleep + the singleton client. 15 new tests pin: defaults / no-throttle, sliding-window cap (legacy), min-delay (the new burst-smoother), max-of-both gates, defensive paths.', page: 'tools' },
{ title: 'Help & Docs: Copy Debug Info Now Reports The Right Music Source + Lists All Services', desc: 'the music_source field always rendered as "unknown" because the code read `_status_cache.get(\'spotify\', {})` — but the cache only has \'media_server\' and \'soulseek\' keys, so the lookup always fell through. same silent miss for spotify_connected and spotify_rate_limited. fix routes those reads through the canonical accessors: `get_primary_source()` for music source (which already accounts for the spotify→deezer auth fallback), `get_spotify_status()` for connection + rate-limit state. also added hydrabase_connected (was missing entirely), youtube_available (always true — yt-dlp + url-based, no auth), hifi_instance_count (separate from connection because each instance is its own endpoint with its own auth), and an always_available_metadata_sources list (deezer / itunes / musicbrainz — public apis, no auth) so the dump reflects the full metadata surface. while in there: removed a local `from core.metadata.status import get_spotify_status` re-import that was making python 3.12 treat the name as a function-scoped local, breaking the new lambda above it (NameError on free variable). 11 new tests at the endpoint boundary pin music_source, spotify_*, hydrabase_*, youtube_available, always_available_metadata_sources, hifi_instance_count, and the defensive paths when each lookup raises.', page: 'tools' },
{ title: 'Download Discography: Skips Tracks Already In Your Library', desc: 'discord report (skowl): clicking download discography on the same artist twice re-queued every track instead of skipping the half already on disk. trace: the endpoint added each track via `add_to_wishlist`, which dedups against the wishlist itself but never checks the library — once a downloaded track leaves the wishlist the next click re-inserts it. fix: same library-ownership check the discography backfill repair job already runs (`db.check_track_exists` at confidence ≥ 0.7). format-agnostic — name + artist + album, no extension comparison — so blasphemy mode (flac → mp3 with original deleted) doesn\'t false-miss. exception during the check returns "not owned" so a transient db hiccup doesn\'t silently nuke the discography fetch (a redundant wishlist add is cheap, a missed track isn\'t). per-album response carries a new `tracks_skipped_owned` counter alongside the artist / content / wishlist skips. 10 new tests at the helper boundary.', page: 'discover' },
{ title: 'Download Discography: No More Cross-Artist Tracks Or Unwanted Remixes', desc: 'issue #559: download discography pulled in tracks from compilations / appears-on albums where the artist was only featured on one or two tracks — every other track on those albums got added too. also ignored your watchlist "include remixes / live / acoustic / instrumental" settings, so one-off discography downloads kept stuffing your wishlist with remix ladders. fix: per-track filter at the endpoint. drops tracks where the requested artist isn\'t named in the track\'s artists list (keeps features, drops unrelated compilation entries). honors `watchlist.global_include_*` settings the same way the discography backfill repair job already does. per-album response carries new skip counts so the ui can show how much got filtered. 21 new tests at the helper boundary.', page: 'discover' },
{ title: 'Album Completeness: "Could Not Determine Album Folder" Error Now Tells You What To Fix', desc: 'github issue #558 (gabistek, navidrome on docker / arch host): clicking auto-fill or fix selected on the album completeness findings page returned a flat "could not determine album folder from existing tracks" error with no diagnostic. trace: the path resolver in `core/library/path_resolver.py` probes transfer + download + `library.music_paths` config + plex api library locations to map db-recorded paths to actual files on disk. for plex users the api auto-discovers the mount paths (per #476). navidrome\'s subsonic api doesn\'t expose filesystem paths at all (only folder names via `getMusicFolders`), and navidrome\'s native rest api on top of that doesn\'t expose them either — there is no api signal we can probe. so for navidrome users in docker, if the path navidrome reports (`/music/artist/album/track.flac`) doesn\'t exist as-is in the soulsync container view AND the user hasn\'t manually configured settings → library → music paths, the resolver returns none and the fix workflow bailed silently. fix: lifted the resolver into a diagnostic-aware variant (`resolve_library_file_path_with_diagnostic` returning a `(resolved, ResolveAttempt)` tuple) that records what was tried — raw-path-existed, base-dirs-probed, whether config_manager / plex_client were wired up. repair_worker uses the diagnostic to render a multi-part error: names the active media server, shows one sample db-recorded path the album\'s tracks have, lists every base directory the resolver actually probed, and points at settings → library → music paths as the actionable fix. user can now read the error and know exactly what to mount or configure. no auto-probing of common docker conventions — too speculative, could resolve to wrong dirs on the suffix-walk if conventional paths happen to contain a partial collision. backwards compatible: legacy `resolve_library_file_path` kept as a thin wrapper that drops the attempt, every existing call site unchanged. 12 new tests pin: tuple shape, raw-path short-circuit attempt fields, base-dirs listed even on walk failure, had-flags reflect caller inputs, error renders active server name + sample path + base dirs, distinguishes empty-base-dirs vs tried-and-failed cases, settings hint always present, defensive against none attempt + missing sample + missing config_manager.', page: 'tools' },
{ title: 'Import History: Clear History Button Now Clears Stuck "Processing" Rows', desc: 'noticed on the import page: clear history left zombie rows behind that all showed "⧗ processing" status from 2-9 days ago. trace: `_record_in_progress` inserts a `status=\'processing\'` row up-front so the ui can render the in-flight import while it runs, then `_finalize_result` updates it to `completed`/`failed` when the import finishes. when the server is restarted mid-import (or the worker crashes), the row never gets finalized — stays at `processing` forever. the clear-history endpoint\'s sql `DELETE ... WHERE status IN (\'completed\', \'approved\', \'failed\', \'needs_identification\', \'rejected\')` didn\'t include `processing`, so those zombies survived every click. fix: add `processing` to the delete list, but guard against nuking actually-live imports by intersecting against `_snapshot_active()` — any folder hash currently registered in the worker\'s in-memory `_active_imports` map is excluded from the delete. `pending_review` deliberately left out so user still has to approve/reject those explicitly. one endpoint touched (`/api/auto-import/clear-completed` in web_server.py). no worker changes. zombie-row pile gets swept on next click, new imports still record + update normally.', page: 'import' },
{ title: 'Auto-Import: Falls Through To Other Metadata Sources When Primary Has No Match', desc: 'discord report (mushy): 16 bandcamp indie albums sat in staging because auto-import couldn\'t identify them. manual search at the bottom of the import music tab found the same albums fine — they just weren\'t on the user\'s primary metadata source (spotify) but existed on tidal/deezer. trace: `_search_metadata_source` in `core/auto_import_worker.py` only queried `get_primary_source()` — single source, no fallback. meanwhile `search_import_albums` (the manual search bar at the bottom of the tab) already iterated the full `get_source_priority(get_primary_source())` chain and broke on first source with results. asymmetric behavior — manual search worked, auto-import didn\'t, same album. fix: lift auto-import to use the same source-chain pattern. try primary first; if it returns nothing OR scores below the 0.4 threshold, fall through to next source in priority order. first source that produces a strong-enough match wins. result dict carries the `source` that actually matched (not the primary name), so downstream `_match_tracks` calls the right client to fetch the album\'s tracklist. defensive per-source try/except so a rate-limited or auth-failed source doesn\'t abort the chain. unconfigured sources (client=None) silently skipped. scoring math lifted to pure helper `_score_album_search_result` so weight tweaks (album 50% / artist 20% / track-count 30%) are pinned at the function boundary independent of the orchestrator. weight constants exposed at module level (`_ALBUM_NAME_WEIGHT`, `_ARTIST_NAME_WEIGHT`, `_TRACK_COUNT_WEIGHT`) — greppable, bumpable in one place. 9 integration tests + 18 scoring-helper tests. integration tests pin: primary-success path unchanged (no fallback fires, only primary client called), primary-empty falls through to next source, primary-weak-score falls through, first fallback success stops the chain (no wasted api calls on remaining sources), all-sources-fail returns None, per-source exception contained, unconfigured-source skipped gracefully, result `source` field reflects winning source, `identification_confidence` from winning source. backwards compatible — single-source users see no change (chain just has one entry).', page: 'import' },
{ title: 'Multi-Artist Tag Settings Now Actually Work (artist_separator + feat_in_title + write_multi_artist)', desc: 'three settings on settings → metadata → tags were partially or completely unimplemented. (1) `write_multi_artist` only worked because of a never-populated `_artists_list` field — `core/metadata/source.py` built `metadata["artist"]` as a hardcoded ", "-joined string but never assigned `metadata["_artists_list"]`, so `core/metadata/enrichment.py:114` always saw an empty list and silently no-op\'d the multi-value tag write. (2) `artist_separator` (default ", ") was referenced in the UI + settings.js save path but ZERO python code read the value — every multi-artist track ended up with hardcoded ", " regardless of what the user picked. (3) `feat_in_title` (when true: pull featured artists into the title as " (feat. X, Y)" and leave only primary in the ARTIST tag — picard convention) had no implementation at all. fix in source.py: populate `_artists_list` from the search response\'s artists array, then build the ARTIST string per the user\'s settings — primary-only when feat_in_title is on (with featured names appended to title; double-append guarded for source titles that already include "feat."), else joined with the configured separator. fix in enrichment.py id3 path: writing TPE1 twice (single-string then list) was overwriting the configured separator. now keeps TPE1 as the display string and writes a separate `TXXX:Artists` frame for the multi-value list (picard convention). vorbis path was already correct (separate "artist" + "artists" keys). deezer-specific upgrade path: deezer\'s `/search` endpoint only returns the primary artist — full contributors live on `/track/<id>`. when source==deezer AND the search response had a single artist AND a track_id is available, enrichment now fetches the per-track endpoint and upgrades the artists list before tag-write. one extra API call per affected deezer track (skipped when search already returned multiple). spotify, tidal, itunes search responses already include all artists so they\'re unaffected. 29 new tests pin: `_artists_list` populated for multi/single/no-artist cases, separator drives ARTIST string (default + custom), single-artist case unaffected by either setting, feat_in_title pulls featured to title + leaves primary in ARTIST, feat_in_title no-op for single artist, double-append guard recognizes 9 source-title variants ("(feat. X)", "(Feat. X)", "(FEAT X)", "(feat X)", "(Featuring X)", "[feat. X]", "ft. X", "(ft X)", "FT. X"), word-boundary regex doesn\'t false-match substrings ("Aftermath" still gets the append), combined-settings precedence (feat_in_title wins over separator for ARTIST string but `_artists_list` carries everyone for the multi-value tag), deezer upgrade fires only when search returned single artist + track_id available, no upgrade for non-deezer sources, upgrade failure falls through to search-result list, no false-positive when /track/<id> confirms single artist.', page: 'settings' },
{ title: 'AudioDB Enrichment: Track Worker No Longer Stuck In Infinite Retry Loop', desc: 'github issue #553: audiodb track enrichment "stuck" — constant requests, no progress, only error log was a 10s read-timeout from `lookup_track_by_id` repeating against the same track. trace: when an entity already has `audiodb_id` populated (from manual match or earlier scan) but `audiodb_match_status` is NULL, the worker tries a direct ID lookup. if it fails (returns None on timeout — audiodb\'s `track.php` endpoint is slow, 10s timeouts common), the prior code logged "preserving manual match" and returned WITHOUT marking status. row stayed NULL → queue picked it up next tick → tried direct lookup → timed out → returned → infinite loop. fix: (1) when direct lookup fails (None or exception), mark `audiodb_match_status="error"` so the queue\'s NULL-status filter stops re-picking the row on every tick. preserves the existing `audiodb_id` (no fallback to name-search guess that would overwrite a manual match). (2) extended the retry-after-cutoff queue priorities (4/5/6) to include `\'error\'` rows alongside `\'not_found\'` — same `retry_days=30` window. transient audiodb outages still recover automatically; permanently-broken IDs eventually get re-attempted once a month. only triggered for entities in the inconsistent state of `audiodb_id` set + `match_status` NULL — happy path and already-matched/already-not-found rows unchanged. 5 new tests pin: lookup-returns-none marks error (no infinite loop), lookup-raises-exception marks error, lookup-success preserves happy path, error-row-past-cutoff gets re-picked, error-row-within-cutoff stays skipped.', page: 'tools' },
{ title: 'Docker: Container No Longer Restart-Loops On Bind-Mounted Staging Folder', desc: 'after pulling latest, the container refused to start. logs showed `mkdir: cannot create directory \'/app/Staging\': Permission denied`. cause traced back to the 2026-05-08 image-bloat fix (commit 70e1750) which changed the Dockerfile from `chown -R /app` to a scoped chown on specific subdirs (the recursive chown was duplicating the whole /app tree into a new layer and ballooning image size). side effect: `/app` itself went from soulsync:soulsync to root:root (Docker WORKDIR default), AND `/app/Staging` was left out of both the Dockerfile mkdir + chown list and only created at runtime by the entrypoint script. on rootless Docker / Podman where in-container "root" maps to a host UID, the entrypoint mkdir on `/app/Staging` could fail with EACCES depending on the bind-mount path\'s host ownership — `set -e` then aborted the script and the container restart-looped. fix: (1) Dockerfile now pre-bakes `/app/Staging` into the image alongside the other runtime mount points (mkdir + scoped chown) so the entrypoint mkdir is a guaranteed no-op even when bind-mount perms are weird. (2) entrypoint mkdir + chown both have `|| true` now so any future bind-mount permission quirk surfaces as a log line, not a restart loop. (3) new writability audit at the end of entrypoint setup — `gosu soulsync test -w` on every bind-mountable dir, logs a loud warning with the exact `chown` command to run on the host if perms mismatch the configured PUID/PGID. catches the underlying bind-mount perm issue that the restart-loop fix would otherwise mask (container starts, but auto-import / downloads write into unwritable dirs and fail silently). zero behavior change for users whose containers were already starting fine; defensive against the rootless/podman config that broke after the image-bloat refactor.', page: 'tools' },
{ title: 'Your Albums: Download Missing Now Opens Selectable Modal + Tidal Resolution', desc: 'two-part fix to the your albums "download missing" flow on discover. (1) replaced the broken per-album direct-download loop with a selectable-grid modal mirroring the library page\'s download discography flow. clicking the download button now opens a checkbox grid showing every missing album (cover, title, artist, year, track count, source) with select all / deselect all controls. user picks what they actually want, hits "add to wishlist", each album\'s tracks get resolved + queued through the existing wishlist auto-download processor. matches the discography flow\'s per-album ndjson progress stream so users see ✓/✗ per album as it processes. previous loop fired direct downloads via `openDownloadMissingModalForYouTube` which the user reported as silently failing — "queuing 2/2" toast with no actual transfer activity. wishlist is the right destination for batch missing-album adds since it already handles retry, source fallback, dedup, and rate limiting. (2) added tidal source resolution. backend `/api/discover/album/<source>/<album_id>` got a new `tidal` source branch that calls a NEW `tidal_client.get_album_tracks(album_id)` method — two-phase fetch (cursor-walk `/v2/albums/<id>/relationships/items?include=items` for track refs + position metadata, batch-hydrate via existing `_get_tracks_batch` for artist/album names). track refs carry `meta.trackNumber` + `meta.volumeNumber` so multi-disc compilations render in album order. inline `?include=coverArt` lookup pulls the album cover too. single-album click flow (`openYourAlbumDownload`) gets `tidal_album_id` added to `trySources`. virtual-id generation includes tidal_album_id for stable identifiers. backend reuses the existing `/api/artist/<id>/download-discography` endpoint — its url artist_id param is functionally unused (per-album payload carries everything), so the modal posts with placeholder `your-albums` and gets multi-artist resolution for free. 10 new tests pin the tidal album-tracks method: single-page walk + hydration, multi-page cursor chain, multi-disc sort order, limit short-circuit, no-token short-circuit, http error returns empty, 429 propagates to rate_limited decorator, forward-compat type filter, partial-batch failure containment, empty-album short-circuit.', page: 'discover' },
{ title: 'AcoustID Scanner: File-Tag Fallback For Legacy Compilation Tracks', desc: 'follow-up to the compilation-album scanner fix. previous patch made the scanner read `tracks.track_artist` (per-track artist column) via COALESCE so compilation tracks would compare against the right value. but tracks downloaded BEFORE that column existed have track_artist=NULL — COALESCE falls back to album artist (the curator) and we\'re back to the wrong-comparison case. fix: explicit 3-tier resolution in `_scan_file` — (1) `tracks.track_artist` from DB if populated → trust it (respects manual edits from the enhanced library view), (2) audio file\'s ARTIST tag via mutagen if present → use it (tidal/spotify/deezer all write the per-track artist into the file at download time, so it\'s ground truth even when DB is stale), (3) album artist → final fallback for files without proper ARTIST tags AND no DB track_artist. file open is essentially free since acoustid is opening it for fingerprinting anyway. critical guard: when DB track_artist is populated (curated value), it always wins over file tag — protects users who edited DB but didn\'t re-tag the file from getting false-positive flags. closes the legacy-data gap without requiring a one-time DB backfill or a re-download. 5 new tests pin: file-tag-resolves-skowl-case (legacy NULL track_artist → file tag wins → no flag), tag-missing-falls-back-to-album-artist (preserves existing genuine-mismatch contract), mutagen-exception-swallowed (debug log, fall-through), tag-matches-DB no behavioral change, and the false-positive guard (DB populated → trumps stale file tag).', page: 'tools' },
{ title: 'Tidal Favorite Albums + Artists Now Show Up On Discover', desc: 'discover → your albums (and your artists) was returning nothing for tidal users regardless of how many albums/artists they\'d favorited. cause: `get_favorite_albums` and `get_favorite_artists` were calling the deprecated `/v2/favorites?filter[type]=ALBUMS|ARTISTS` endpoint, which returns 404 for personal favorites — that endpoint is scoped to collections the third-party app created itself, not the user\'s app-level favorites. the V1 fallback was also dead because modern OAuth tokens carry `collection.read` instead of the legacy `r_usr` scope V1 requires (returns 403). same root cause as the favorited tracks fix from #502. fix: rewire to the working V2 user-collection endpoints — `/v2/userCollectionAlbums/me/relationships/items` and `/v2/userCollectionArtists/me/relationships/items` — using the same cursor-paginated pattern shipped for tracks. ID enumeration lifted into a generic `_iter_collection_resource_ids(path, expected_type, max_ids)` helper so tracks/albums/artists all share one walker (~80 lines deduped). batch hydration via `/v2/{albums|artists}?filter[id]=...&include=...` with extended JSON:API include semantics — single request returns 20 albums + their artists + cover artworks all in `included[]`, parsed via two static helpers (`_first_artist_name`, `_first_artwork_url`) that map relationship refs to the included map. cover/profile images pick `files[0]` (largest variant Tidal returns, typically 1280×1280). public methods preserve the prior return shape so the discover aggregator in web_server.py stays byte-identical. 24 new tests pin: cursor-walker dispatch (correct path + type), included-map building, artist + artwork relationship resolution (full + missing + unknown-id), batch hydration parse for albums + artists, empty-input + HTTP-error short-circuits, BATCH_SIZE chunking (41 IDs → 20/20/1), end-to-end orchestrator behavior.', page: 'discover' },
{ title: 'Server Playlist Sync: Append Mode (Stop Overwriting User-Added Tracks)', desc: 'discord report (cjfc, 2026-04-26): syncing a spotify playlist to your server overwrote anything you\'d manually added to the server-side playlist. now there\'s a per-sync mode picker next to the Sync button on the playlist details modal: "Replace" (default, current behavior — delete + recreate) or "Append only" (preserve existing, only add tracks not already there). useful when the source platform caps playlist size (spotify 100-track limit) and you\'re manually building beyond it on the server. each server client (plex / jellyfin / navidrome) gets a new `append_to_playlist(name, tracks)` method that uses the server\'s native append api — plex `addItems`, jellyfin `POST /Playlists/<id>/Items`, navidrome subsonic `updatePlaylist?songIdToAdd=...`. no delete-recreate, no backup playlist created in append mode (preserves playlist creation date + metadata + non-soulsync-managed tracks). dedup-by-id ensures we never add a track that\'s already on the playlist (matched by ratingKey for plex, jellyfin guid id for jellyfin, song id for navidrome — server-native identity, not fuzzy title+artist match). falls back to `create_playlist` when the playlist doesn\'t exist yet (first sync). sync_service dispatches via the new mode flag through /api/sync/start; soulsync standalone has no playlist methods at all so the dispatch falls back to update_playlist with a warning log when append is requested against it. 15 new tests pin: missing playlist → create delegation, dedup filtering (existing ids skipped), short-circuit on no-new-tracks (no api call), failure paths return False without raising, contract listing for each server client.', page: 'sync' },
],
'2.5.0': [
// --- May 10, 2026 — 2.5.0 release ---
{ date: 'May 10, 2026 — 2.5.0 release' },
@ -4311,7 +4329,7 @@ function _getLatestWhatsNewVersion() {
const versions = Object.keys(WHATS_NEW)
.filter(v => _compareVersions(v, buildVer) <= 0)
.sort((a, b) => _compareVersions(b, a));
return versions[0] || '2.5.0';
return versions[0] || '2.5.1';
}
function openWhatsNew() {

2
webui/static/settings.js
View file

@ -856,6 +856,7 @@ async function loadSettingsData() {
document.getElementById('soulseek-api-key').value = settings.soulseek?.api_key || '';
document.getElementById('soulseek-search-timeout').value = settings.soulseek?.search_timeout || 60;
document.getElementById('soulseek-search-timeout-buffer').value = settings.soulseek?.search_timeout_buffer || 15;
document.getElementById('soulseek-search-min-delay-seconds').value = settings.soulseek?.search_min_delay_seconds ?? 0;
document.getElementById('soulseek-min-peer-speed').value = settings.soulseek?.min_peer_upload_speed || 0;
document.getElementById('soulseek-max-peer-queue').value = settings.soulseek?.max_peer_queue || 0;
document.getElementById('soulseek-download-timeout').value = Math.round((settings.soulseek?.download_timeout || 600) / 60);
@ -2605,6 +2606,7 @@ async function saveSettings(quiet = false) {
transfer_path: document.getElementById('transfer-path').value,
search_timeout: parseInt(document.getElementById('soulseek-search-timeout').value) || 60,
search_timeout_buffer: parseInt(document.getElementById('soulseek-search-timeout-buffer').value) || 15,
search_min_delay_seconds: parseInt(document.getElementById('soulseek-search-min-delay-seconds').value) || 0,
min_peer_upload_speed: parseInt(document.getElementById('soulseek-min-peer-speed').value) || 0,
max_peer_queue: parseInt(document.getElementById('soulseek-max-peer-queue').value) || 0,
download_timeout: (parseInt(document.getElementById('soulseek-download-timeout').value) || 10) * 60,

18
webui/static/style.css
View file

@ -15537,6 +15537,24 @@ body.helper-mode-active #dashboard-activity-feed:hover {
box-shadow: 0 6px 20px rgba(var(--accent-rgb), 0.4);
}
/* Sync mode picker — Replace vs Append, sits left of the Sync button.
Sized to match the modal-btn height so it doesn't break footer alignment. */
.playlist-modal-sync-mode {
background: #2a2a2a;
color: #ffffff;
border: 1px solid #555555;
border-radius: 6px;
padding: 8px 10px;
font-size: 13px;
cursor: pointer;
margin-right: 4px;
}
.playlist-modal-sync-mode:hover {
border-color: #777777;
background: #333333;
}
.playlist-modal-btn-tertiary {
background: #404040;
color: #ffffff;

4
webui/static/sync-services.js
View file

@ -1683,6 +1683,10 @@ function showDeezerArlPlaylistDetailsModal(playlist, originalDeezerPlaylistId) {
<button class="playlist-modal-btn playlist-modal-btn-tertiary" onclick="closeDeezerArlPlaylistDetailsModal(); openDownloadMissingModal('${playlistId}')">
${hasCompletedProcess ? '📊 View Download Results' : '📥 Download Missing Tracks'}
</button>
<select id="sync-mode-${playlistId}" class="playlist-modal-sync-mode" title="Replace overwrites the server playlist; Append only adds new tracks (preserves user-added)" ${_isSoulsyncStandalone ? 'style="display:none"' : ''}>
<option value="replace" selected>Replace</option>
<option value="append">Append only</option>
</select>
<button id="sync-btn-${playlistId}" class="playlist-modal-btn playlist-modal-btn-primary" onclick="startPlaylistSync('${playlistId}')" ${isSyncing ? 'disabled' : ''} ${_isSoulsyncStandalone ? 'style="display:none"' : ''}>${isSyncing ? '⏳ Syncing...' : 'Sync Playlist'}</button>
</div>
</div>

4
webui/static/sync-spotify.js
View file

@ -1935,6 +1935,10 @@ function showPlaylistDetailsModal(playlist) {
? '📊 View Download Results'
: '📥 Download Missing Tracks'}
</button>
<select id="sync-mode-${playlist.id}" class="playlist-modal-sync-mode" title="Replace overwrites the server playlist; Append only adds new tracks (preserves user-added)" ${_isSoulsyncStandalone ? 'style="display:none"' : ''}>
<option value="replace" selected>Replace</option>
<option value="append">Append only</option>
</select>
<button id="sync-btn-${playlist.id}" class="playlist-modal-btn playlist-modal-btn-primary" onclick="startPlaylistSync('${playlist.id}')" ${isSyncing ? 'disabled' : ''} ${_isSoulsyncStandalone ? 'style="display:none"' : ''}>${isSyncing ? '⏳ Syncing...' : 'Sync Playlist'}</button>
</div>
</div>