Merge pull request #603 from Nezreka/dev

Release 2.5.3
This commit is contained in:
BoulderBadgeDad 2026-05-14 16:40:10 -07:00 committed by GitHub
commit 6c3189e76c
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
47 changed files with 5234 additions and 614 deletions

View file

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

View file

@ -89,6 +89,33 @@ def _upgrade_deezer_cover_url(url: str, target_size: int = _DEEZER_MAX_COVER_SIZ
return _DEEZER_CDN_SIZE_PATTERN.sub(f'/{target_size}x{target_size}-', url, count=1)
def _is_full_track_payload(payload: Optional[Dict[str, Any]]) -> bool:
"""Distinguish a full `/track/<id>` cache hit from partial album-tracks data.
Three Deezer endpoints feed the per-track cache:
- `/track/<id>` full record, includes both `track_position` AND
`contributors` (the multi-artist list the contributors-upgrade
path reads).
- `/album/<id>/tracks` partial; includes `track_position` but
omits `contributors`.
- `/search/track` minimal; lacks `track_position`.
Pre-fix `get_track_details` only checked `track_position`, so
partial album-tracks payloads were treated as full hits and the
contributors-upgrade silently fell back to single-artist tagging
whenever an album had been fetched before its individual tracks
were post-processed (issue #588).
`contributors` key presence is the load-bearing distinction
`[]` is a valid value for genuinely single-artist tracks fetched
via the per-track endpoint, so test for key membership not
truthiness.
"""
if not isinstance(payload, dict):
return False
return 'track_position' in payload and 'contributors' in payload
# ==================== Dataclasses (match iTunesClient / SpotifyClient format) ====================
@dataclass
@ -546,14 +573,9 @@ class DeezerClient:
"""Get detailed track info — returns Spotify-compatible dict (metadata source interface)"""
cache = get_metadata_cache()
cached = cache.get_entity('deezer', 'track', str(track_id))
if cached and cached.get('title'):
# Search results are cached with minimal data (no track_position).
# Only use cache if it has track_position — the key field from /track/{id}.
# Search results include 'isrc' and 'release_date' but NOT track_position,
# so those fields alone are not sufficient to distinguish full from partial data.
if 'track_position' in cached:
return self._build_enhanced_track(cached)
# Otherwise fall through to fetch full data from API
if cached and cached.get('title') and _is_full_track_payload(cached):
return self._build_enhanced_track(cached)
# Otherwise fall through to fetch full data from API
data = self._api_get(f'track/{track_id}')
if not data:

View file

@ -174,14 +174,33 @@ def run_full_missing_tracks_process(batch_id, playlist_id, tracks_json, deps: Ma
elif album_tracks_map:
# Album-scoped matching: check against known album tracks first
track_name_lower = track_name.lower().strip()
# Direct title match
# Issue #589 — strip suffixes that just repeat the album
# context (e.g. "Shy Away (MTV Unplugged Live)" on a
# "MTV Unplugged" album → "Shy Away") so album-owned
# tracks don't false-miss when the local DB stored the
# base title. Only fires inside the album-confirmed
# scope; global matching elsewhere is unchanged.
from core.matching.album_context_title import strip_redundant_album_suffix
_album_name_for_strip = (batch_album_context or {}).get('name', '')
_normalized_source_title = strip_redundant_album_suffix(
track_name, _album_name_for_strip
).lower().strip()
# Direct title match (try both raw and normalized)
if track_name_lower in album_tracks_map:
found, confidence = True, 1.0
elif _normalized_source_title and _normalized_source_title in album_tracks_map:
found, confidence = True, 1.0
else:
# Fuzzy match against album tracks using string similarity
# Fuzzy match against album tracks using string similarity.
# Compare BOTH the raw and normalized source titles —
# whichever scores higher wins. Preserves strict
# matching when the album doesn't imply version
# context (helper returns the input unchanged).
best_sim = 0.0
for db_title_lower, _db_track in album_tracks_map.items():
sim = db._string_similarity(track_name_lower, db_title_lower)
sim_raw = db._string_similarity(track_name_lower, db_title_lower)
sim_norm = db._string_similarity(_normalized_source_title, db_title_lower) if _normalized_source_title else 0.0
sim = max(sim_raw, sim_norm)
if sim > best_sim:
best_sim = sim
if best_sim >= 0.7:

View file

@ -52,6 +52,37 @@ _DEFAULT_LENGTH_TOLERANCE_S = 3.0
_LENGTH_TOLERANCE_LONG_TRACK_S = 5.0
_LONG_TRACK_THRESHOLD_S = 600.0 # 10 minutes
# Upper bound for the user-configurable override. Anything past 60s
# means the check is effectively off — cap defends against accidental
# nonsense like 9999 making logs misleading. Users who genuinely want
# to disable the check can set 60.
_MAX_USER_TOLERANCE_S = 60.0
def resolve_duration_tolerance(value: Any) -> Optional[float]:
"""Coerce a user-configured tolerance value to a float override.
Returns:
- None when value is missing / 0 / negative / unparseable, so
callers fall back to the auto-scaled defaults (3s/5s).
- float in (0, _MAX_USER_TOLERANCE_S] when value is a positive
numeric string or float clamped to the upper bound.
Pure helper. No I/O. Drives the `length_tolerance_s` override on
`check_audio_integrity`.
"""
if value is None:
return None
try:
parsed = float(value)
except (TypeError, ValueError):
return None
if parsed <= 0:
return None
if parsed > _MAX_USER_TOLERANCE_S:
return _MAX_USER_TOLERANCE_S
return parsed
@dataclass
class IntegrityResult:

View file

@ -29,8 +29,22 @@ def _get_config_manager():
return config_manager
def move_to_quarantine(file_path: str, context: dict, reason: str, automation_engine=None) -> str:
"""Move a file to the quarantine folder and write a metadata sidecar."""
def move_to_quarantine(file_path: str, context: dict, reason: str, automation_engine=None, *, trigger: str = "unknown") -> str:
"""Move a file to the quarantine folder and write a metadata sidecar.
`trigger` identifies which check fired (`integrity` / `acoustid` /
`bit_depth` / `unknown`) and is persisted in the sidecar so
one-click Approve can set the matching `_skip_quarantine_check`
bypass when re-running the pipeline.
Sidecar also persists a JSON-safe snapshot of the full `context`
dict via `serialize_quarantine_context`, enabling in-place approve
without losing the matched-track metadata. Legacy sidecars (written
before this expansion) lack the `context` field Approve falls
back to `recover_to_staging` for those.
"""
from core.imports.quarantine import serialize_quarantine_context
download_dir = _get_config_manager().get("soulseek.download_path", "./downloads")
quarantine_dir = Path(download_dir) / "ss_quarantine"
quarantine_dir.mkdir(parents=True, exist_ok=True)
@ -56,6 +70,8 @@ def move_to_quarantine(file_path: str, context: dict, reason: str, automation_en
"expected_track": get_import_clean_title(context, default=original_search.get("title", "Unknown")),
"expected_artist": get_import_clean_artist(context, default=(artist_context.get("name", "") if isinstance(artist_context, dict) else "Unknown")),
"context_key": context.get("context_key", "unknown"),
"trigger": trigger,
"context": serialize_quarantine_context(context),
}
try:

View file

@ -32,7 +32,7 @@ from core.imports.context import (
get_import_track_info,
normalize_import_context,
)
from core.imports.file_integrity import check_audio_integrity
from core.imports.file_integrity import check_audio_integrity, resolve_duration_tolerance
from core.imports.filename import extract_track_number_from_filename
from core.imports.guards import check_flac_bit_depth, move_to_quarantine
from core.imports.side_effects import (
@ -155,11 +155,31 @@ def post_process_matched_download(context_key, context, file_path, runtime, meta
except Exception:
_expected_duration_ms = None
try:
integrity = check_audio_integrity(file_path, _expected_duration_ms)
except Exception as integrity_error:
logger.error(f"[Integrity] Check raised unexpectedly (continuing): {integrity_error}")
# User-configurable tolerance override. None = use built-in
# auto-scaled defaults (3s normal / 5s for tracks >10min). Set
# higher (e.g. 10) when matched files routinely drift from the
# source's reported duration (live recordings, alternate
# masterings, etc).
_duration_tolerance_override = resolve_duration_tolerance(
config_manager.get('post_processing.duration_tolerance_seconds', 0)
)
# Per-check quarantine bypass — set by `approve_quarantine_entry`
# when the user explicitly approves a previously-quarantined
# file. Skips ONLY the named check; other gates still run.
_bypass_check = context.get('_skip_quarantine_check')
if _bypass_check == 'integrity':
logger.info(f"[Integrity] Skipped (user approval) for {_basename}")
integrity = None
else:
try:
integrity = check_audio_integrity(
file_path,
_expected_duration_ms,
length_tolerance_s=_duration_tolerance_override,
)
except Exception as integrity_error:
logger.error(f"[Integrity] Check raised unexpectedly (continuing): {integrity_error}")
integrity = None
if integrity is not None and not integrity.ok:
logger.error(f"[Integrity] Rejected {_basename}: {integrity.reason}")
@ -171,6 +191,7 @@ def post_process_matched_download(context_key, context, file_path, runtime, meta
context,
f"Integrity check failed: {integrity.reason}",
automation_engine,
trigger='integrity',
)
logger.error(f"File quarantined due to integrity failure: {quarantine_path}")
except Exception as quarantine_error:
@ -206,7 +227,9 @@ def post_process_matched_download(context_key, context, file_path, runtime, meta
f"drift={integrity.checks.get('length_drift_s', 'n/a')})"
)
_skip_acoustid = False
_skip_acoustid = context.get('_skip_quarantine_check') == 'acoustid'
if _skip_acoustid:
logger.info(f"[AcoustID] Skipped (user approval) for {_basename}")
try:
from core.acoustid_verification import AcoustIDVerification, VerificationResult
@ -248,6 +271,7 @@ def post_process_matched_download(context_key, context, file_path, runtime, meta
context,
verification_msg,
automation_engine,
trigger='acoustid',
)
logger.error(f"File quarantined due to verification failure: {quarantine_path}")
except Exception as quarantine_error:
@ -420,7 +444,9 @@ def post_process_matched_download(context_key, context, file_path, runtime, meta
if context['_audio_quality']:
logger.info(f"Audio quality detected: {context['_audio_quality']}")
rejection_reason = check_flac_bit_depth(file_path, context)
rejection_reason = None if context.get('_skip_quarantine_check') == 'bit_depth' else check_flac_bit_depth(file_path, context)
if context.get('_skip_quarantine_check') == 'bit_depth':
logger.info(f"[BitDepth] Skipped (user approval) for {_basename}")
if rejection_reason:
try:
quarantine_path = move_to_quarantine(
@ -428,6 +454,7 @@ def post_process_matched_download(context_key, context, file_path, runtime, meta
context,
rejection_reason,
automation_engine,
trigger='bit_depth',
)
logger.info(f"File quarantined due to bit depth filter: {quarantine_path}")
except Exception as quarantine_error:
@ -548,7 +575,9 @@ def post_process_matched_download(context_key, context, file_path, runtime, meta
if context['_audio_quality']:
logger.info(f"Audio quality detected: {context['_audio_quality']}")
rejection_reason = check_flac_bit_depth(file_path, context)
rejection_reason = None if context.get('_skip_quarantine_check') == 'bit_depth' else check_flac_bit_depth(file_path, context)
if context.get('_skip_quarantine_check') == 'bit_depth':
logger.info(f"[BitDepth] Skipped (user approval) for {_basename}")
if rejection_reason:
try:
quarantine_path = move_to_quarantine(
@ -556,6 +585,7 @@ def post_process_matched_download(context_key, context, file_path, runtime, meta
context,
rejection_reason,
automation_engine,
trigger='bit_depth',
)
logger.info(f"File quarantined due to bit depth filter: {quarantine_path}")
except Exception as quarantine_error:

329
core/imports/quarantine.py Normal file
View file

@ -0,0 +1,329 @@
"""Quarantine entry management — pure helpers for list/delete/approve/recover.
Quarantined files live in `<download_path>/ss_quarantine/` as
`<timestamp>_<original>.<ext>.quarantined` paired with a JSON sidecar
`<timestamp>_<original>.json` written by `core.imports.guards.move_to_quarantine`.
This module provides the read/write/restore primitives. Web routes are
thin glue around these. Pipeline re-run on approval is the caller's
job (we hand back `(file_path, context, bypass_check)`).
"""
from __future__ import annotations
import json
import os
import shutil
from pathlib import Path
from typing import Any, Dict, List, Optional, Tuple
from utils.logging_config import get_logger
logger = get_logger("imports.quarantine")
_QUARANTINE_SUFFIX = ".quarantined"
# JSON-serializable scalar predicate. dict / list values get walked
# recursively; anything else is dropped during sidecar serialization.
_SAFE_SCALARS = (str, int, float, bool, type(None))
def serialize_quarantine_context(context: Any) -> Dict[str, Any]:
"""Walk a context dict and emit a JSON-safe copy.
Drops non-serializable values (sets, custom objects, callables,
open file handles, etc) silently sidecar must round-trip through
`json.dump` / `json.load` without raising. Lists are walked element
by element; dicts are walked recursively. Anything that isn't a
scalar / dict / list is converted to a string fallback so caller
still sees *something* (rather than a silent drop) but won't break
the JSON write.
"""
if not isinstance(context, dict):
return {}
return _coerce_dict(context)
def _coerce_value(value: Any) -> Any:
if isinstance(value, _SAFE_SCALARS):
return value
if isinstance(value, dict):
return _coerce_dict(value)
if isinstance(value, (list, tuple)):
return [_coerce_value(v) for v in value]
if isinstance(value, set):
return [_coerce_value(v) for v in value]
# Fallback — preserve via str() so caller sees the value's shape
# without breaking JSON serialization.
try:
return str(value)
except Exception:
return None
def _coerce_dict(d: Dict[str, Any]) -> Dict[str, Any]:
out: Dict[str, Any] = {}
for key, value in d.items():
if not isinstance(key, str):
try:
key = str(key)
except Exception:
continue
out[key] = _coerce_value(value)
return out
def _entry_id_from_filename(quarantined_filename: str) -> str:
"""Derive a stable entry id from the quarantined filename.
Strip the `.quarantined` suffix; strip the original file extension;
return the bare `<timestamp>_<original>` stem. Sidecar uses the
same stem with a `.json` extension, so the id pairs both sides.
"""
base = quarantined_filename
if base.endswith(_QUARANTINE_SUFFIX):
base = base[: -len(_QUARANTINE_SUFFIX)]
return Path(base).stem
def list_quarantine_entries(quarantine_dir: str) -> List[Dict[str, Any]]:
"""Enumerate quarantined files paired with their sidecars.
Returns one dict per `.quarantined` file with: id, filename,
original_filename (from sidecar), reason, expected_track,
expected_artist, timestamp, size_bytes, has_full_context (True
when the sidecar carries a `context` field required for one-click
Approve), trigger (which check fired: integrity / acoustid /
bit_depth / unknown).
Orphaned `.quarantined` files (no sidecar) still surface caller
can delete them. Orphaned sidecars (no file) are skipped silently.
Sorted newest-first by timestamp prefix.
"""
entries: List[Dict[str, Any]] = []
if not os.path.isdir(quarantine_dir):
return entries
for name in os.listdir(quarantine_dir):
if not name.endswith(_QUARANTINE_SUFFIX):
continue
full_path = os.path.join(quarantine_dir, name)
if not os.path.isfile(full_path):
continue
entry_id = _entry_id_from_filename(name)
sidecar_path = os.path.join(quarantine_dir, f"{entry_id}.json")
sidecar: Dict[str, Any] = {}
if os.path.isfile(sidecar_path):
try:
with open(sidecar_path, encoding="utf-8") as f:
loaded = json.load(f)
if isinstance(loaded, dict):
sidecar = loaded
except Exception as exc:
logger.debug("sidecar read failed for %s: %s", entry_id, exc)
try:
size_bytes = os.path.getsize(full_path)
except OSError:
size_bytes = 0
entries.append(
{
"id": entry_id,
"filename": name,
"original_filename": sidecar.get("original_filename", name),
"reason": sidecar.get("quarantine_reason", "Unknown reason"),
"expected_track": sidecar.get("expected_track", ""),
"expected_artist": sidecar.get("expected_artist", ""),
"timestamp": sidecar.get("timestamp", ""),
"size_bytes": size_bytes,
"has_full_context": isinstance(sidecar.get("context"), dict),
"trigger": sidecar.get("trigger", "unknown"),
}
)
entries.sort(key=lambda e: e["id"], reverse=True)
return entries
def _resolve_entry_paths(quarantine_dir: str, entry_id: str) -> Tuple[Optional[str], Optional[str]]:
"""Locate the `.quarantined` file + JSON sidecar for an entry id.
Returns (file_path, sidecar_path), either may be None if missing.
"""
if not os.path.isdir(quarantine_dir) or not entry_id:
return None, None
file_path: Optional[str] = None
for name in os.listdir(quarantine_dir):
if not name.endswith(_QUARANTINE_SUFFIX):
continue
if _entry_id_from_filename(name) == entry_id:
file_path = os.path.join(quarantine_dir, name)
break
sidecar_path = os.path.join(quarantine_dir, f"{entry_id}.json")
if not os.path.isfile(sidecar_path):
sidecar_path = None
return file_path, sidecar_path
def delete_quarantine_entry(quarantine_dir: str, entry_id: str) -> bool:
"""Delete the quarantined file + sidecar for the given entry id.
Returns True if at least one of the two was removed. False when
neither existed (entry already gone).
"""
file_path, sidecar_path = _resolve_entry_paths(quarantine_dir, entry_id)
removed = False
if file_path and os.path.isfile(file_path):
try:
os.remove(file_path)
removed = True
except OSError as exc:
logger.error("Failed to delete quarantine file %s: %s", file_path, exc)
if sidecar_path and os.path.isfile(sidecar_path):
try:
os.remove(sidecar_path)
removed = True
except OSError as exc:
logger.error("Failed to delete quarantine sidecar %s: %s", sidecar_path, exc)
return removed
def _restore_filename(quarantined_filename: str, sidecar_original: Optional[str] = None) -> str:
"""Resolve the filename to restore.
Sidecar's `original_filename` wins when provided — it's the
canonical record of what the file was named before quarantine.
Otherwise parse the `<YYYYMMDD_HHMMSS>_<original>.<ext>.quarantined`
convention written by `move_to_quarantine`, dropping the timestamp
prefix and `.quarantined` suffix. Final fallback returns the
quarantined filename minus the suffix unchanged.
"""
if sidecar_original:
return sidecar_original
base = quarantined_filename
if base.endswith(_QUARANTINE_SUFFIX):
base = base[: -len(_QUARANTINE_SUFFIX)]
parts = base.split("_", 2)
if len(parts) >= 3 and parts[0].isdigit() and parts[1].isdigit():
return parts[2]
return base
def approve_quarantine_entry(
quarantine_dir: str,
entry_id: str,
restore_dir: str,
) -> Optional[Tuple[str, Dict[str, Any], str]]:
"""Restore a quarantined file for re-import via the post-process pipeline.
Reads the sidecar's `context` + `trigger`, moves the file out of
quarantine to `restore_dir` (with the original filename + extension),
deletes the sidecar.
Returns `(restored_file_path, context, trigger)` so the caller can
set the appropriate `_skip_quarantine_check` bypass flag and
dispatch the post-process pipeline.
Returns None when:
- the entry doesn't exist
- the sidecar lacks a serialized `context` (legacy thin sidecar
caller should fall back to `recover_to_staging` instead)
- the file move fails
"""
file_path, sidecar_path = _resolve_entry_paths(quarantine_dir, entry_id)
if not file_path or not sidecar_path:
logger.warning("approve: entry %s missing file or sidecar", entry_id)
return None
try:
with open(sidecar_path, encoding="utf-8") as f:
sidecar = json.load(f)
except Exception as exc:
logger.error("approve: sidecar read failed for %s: %s", entry_id, exc)
return None
context = sidecar.get("context")
if not isinstance(context, dict):
logger.info("approve: entry %s has thin sidecar (no context) — caller should recover-to-staging", entry_id)
return None
trigger = str(sidecar.get("trigger", "unknown"))
original_name = sidecar.get("original_filename") or _restore_filename(os.path.basename(file_path))
os.makedirs(restore_dir, exist_ok=True)
restored_path = os.path.join(restore_dir, original_name)
restored_path = _ensure_unique_path(restored_path)
try:
shutil.move(file_path, restored_path)
except OSError as exc:
logger.error("approve: failed to restore %s -> %s: %s", file_path, restored_path, exc)
return None
try:
os.remove(sidecar_path)
except OSError as exc:
logger.warning("approve: failed to remove sidecar %s: %s", sidecar_path, exc)
return restored_path, context, trigger
def recover_to_staging(
quarantine_dir: str,
staging_dir: str,
entry_id: str,
) -> Optional[str]:
"""Move a quarantined file into Staging for manual import.
Strips the timestamp prefix + `.quarantined` suffix, drops the file
into `staging_dir` so the user can finish via the existing Import
flow. Sidecar is removed. Used as the fallback path for legacy thin
sidecars (no embedded `context`) where one-click Approve is
impossible.
"""
file_path, sidecar_path = _resolve_entry_paths(quarantine_dir, entry_id)
if not file_path:
return None
sidecar_original = None
if sidecar_path:
try:
with open(sidecar_path, encoding="utf-8") as f:
sidecar_original = json.load(f).get("original_filename")
except Exception as exc:
logger.debug("recover: sidecar read failed for %s: %s", entry_id, exc)
restored_name = _restore_filename(os.path.basename(file_path), sidecar_original)
os.makedirs(staging_dir, exist_ok=True)
target = _ensure_unique_path(os.path.join(staging_dir, restored_name))
try:
shutil.move(file_path, target)
except OSError as exc:
logger.error("recover: failed to move %s -> %s: %s", file_path, target, exc)
return None
if sidecar_path and os.path.isfile(sidecar_path):
try:
os.remove(sidecar_path)
except OSError as exc:
logger.warning("recover: failed to remove sidecar %s: %s", sidecar_path, exc)
return target
def _ensure_unique_path(target: str) -> str:
"""Append `_(2)`, `_(3)`, ... before the extension when target exists."""
if not os.path.exists(target):
return target
base, ext = os.path.splitext(target)
counter = 2
while True:
candidate = f"{base}_({counter}){ext}"
if not os.path.exists(candidate):
return candidate
counter += 1

515
core/imports/routes.py Normal file
View file

@ -0,0 +1,515 @@
"""Import/staging controller helpers for Flask-style endpoints."""
from __future__ import annotations
import os
import uuid
from concurrent.futures import as_completed
from dataclasses import dataclass
from typing import Any, Callable, Dict
from core.imports.album import build_album_import_context, build_album_import_match_payload, resolve_album_artist_context
from core.imports.context import get_import_context_artist, get_import_track_info, normalize_import_context
from core.imports.filename import parse_filename_metadata
from core.imports.staging import (
AUDIO_EXTENSIONS,
get_import_suggestions_cache,
get_primary_source as _get_primary_source,
get_staging_path as _get_staging_path,
read_staging_file_metadata as _read_staging_file_metadata,
refresh_import_suggestions_cache as _refresh_import_suggestions_cache,
search_import_albums as _search_import_albums,
search_import_tracks as _search_import_tracks,
)
from utils.logging_config import get_logger
module_logger = get_logger("imports.routes")
def _default_read_tags(file_path: str):
from mutagen import File as MutagenFile
return MutagenFile(file_path, easy=True)
def _get_single_track_import_context(*args, **kwargs):
from core.imports.resolution import get_single_track_import_context
return get_single_track_import_context(*args, **kwargs)
@dataclass
class ImportRouteRuntime:
"""Dependencies needed to service import/staging HTTP endpoints."""
get_staging_path: Callable[[], str] = _get_staging_path
read_staging_file_metadata: Callable[[str, str], Dict[str, Any]] = _read_staging_file_metadata
read_tags: Callable[[str], Any] = _default_read_tags
get_primary_source: Callable[[], str] = _get_primary_source
search_import_albums: Callable[..., list] = _search_import_albums
search_import_tracks: Callable[..., list] = _search_import_tracks
build_album_import_match_payload: Callable[..., Dict[str, Any]] = build_album_import_match_payload
resolve_album_artist_context: Callable[..., Any] = resolve_album_artist_context
build_album_import_context: Callable[..., Dict[str, Any]] = build_album_import_context
get_single_track_import_context: Callable[..., Dict[str, Any]] = _get_single_track_import_context
parse_filename_metadata: Callable[[str], Dict[str, Any]] = parse_filename_metadata
normalize_import_context: Callable[[Dict[str, Any]], Dict[str, Any]] = normalize_import_context
get_import_context_artist: Callable[[Dict[str, Any]], Dict[str, Any]] = get_import_context_artist
get_import_track_info: Callable[[Dict[str, Any]], Dict[str, Any]] = get_import_track_info
process_single_import_file: Callable[["ImportRouteRuntime", Dict[str, Any]], tuple[str, str]] | None = None
post_process_matched_download: Callable[[str, Dict[str, Any], str], Any] | None = None
add_activity_item: Callable[[Any, Any, Any, Any], Any] | None = None
refresh_import_suggestions_cache: Callable[[], Any] = _refresh_import_suggestions_cache
automation_engine: Any = None
hydrabase_worker: Any = None
dev_mode_enabled: bool = False
import_singles_executor: Any = None
logger: Any = module_logger
def staging_files(runtime: ImportRouteRuntime) -> tuple[Dict[str, Any], int]:
"""Scan the staging folder and return audio files with tag metadata."""
try:
staging_path = runtime.get_staging_path()
os.makedirs(staging_path, exist_ok=True)
files = []
for root, _dirs, filenames in os.walk(staging_path):
for fname in filenames:
ext = os.path.splitext(fname)[1].lower()
if ext not in AUDIO_EXTENSIONS:
continue
full_path = os.path.join(root, fname)
rel_path = os.path.relpath(full_path, staging_path)
meta = runtime.read_staging_file_metadata(full_path, rel_path)
files.append(
{
"filename": fname,
"rel_path": rel_path,
"full_path": full_path,
"title": meta["title"],
"artist": meta["albumartist"] or meta["artist"] or "Unknown Artist",
"album": meta["album"],
"track_number": meta["track_number"],
"disc_number": meta["disc_number"],
"extension": ext,
}
)
files.sort(key=lambda f: f["filename"].lower())
return {"success": True, "files": files, "staging_path": staging_path}, 200
except Exception as exc:
runtime.logger.error("Error scanning staging files: %s", exc)
return {"success": False, "error": str(exc)}, 500
def staging_groups(runtime: ImportRouteRuntime) -> tuple[Dict[str, Any], int]:
"""Auto-detect album groups from staging files based on their tags."""
try:
staging_path = runtime.get_staging_path()
if not os.path.isdir(staging_path):
return {"success": True, "groups": []}, 200
album_groups = {}
for root, _dirs, filenames in os.walk(staging_path):
for fname in filenames:
ext = os.path.splitext(fname)[1].lower()
if ext not in AUDIO_EXTENSIONS:
continue
full_path = os.path.join(root, fname)
rel_path = os.path.relpath(full_path, staging_path)
meta = runtime.read_staging_file_metadata(full_path, rel_path)
album = meta["album"]
artist = meta["albumartist"] or meta["artist"]
if not album or not artist:
continue
key = (album.lower().strip(), artist.lower().strip())
if key not in album_groups:
album_groups[key] = {"album": album.strip(), "artist": artist.strip(), "files": []}
album_groups[key]["files"].append(
{
"filename": fname,
"full_path": full_path,
"title": meta["title"],
"track_number": meta["track_number"],
}
)
groups = []
for group in album_groups.values():
if len(group["files"]) >= 2:
group["files"].sort(key=lambda f: f.get("track_number") or 999)
groups.append(
{
"album": group["album"],
"artist": group["artist"],
"file_count": len(group["files"]),
"files": group["files"],
"file_paths": [f["full_path"] for f in group["files"]],
}
)
groups.sort(key=lambda g: g["file_count"], reverse=True)
return {"success": True, "groups": groups}, 200
except Exception as exc:
runtime.logger.error("Error building staging groups: %s", exc)
return {"success": False, "error": str(exc)}, 500
def staging_hints(runtime: ImportRouteRuntime) -> tuple[Dict[str, Any], int]:
"""Extract album search hints from staging folder tags and folder names."""
try:
staging_path = runtime.get_staging_path()
if not os.path.isdir(staging_path):
return {"success": True, "hints": []}, 200
tag_albums = {}
folder_hints = {}
for root, _dirs, filenames in os.walk(staging_path):
audio_files = [f for f in filenames if os.path.splitext(f)[1].lower() in AUDIO_EXTENSIONS]
if not audio_files:
continue
rel_dir = os.path.relpath(root, staging_path)
if rel_dir != ".":
top_folder = rel_dir.split(os.sep)[0]
folder_hints[top_folder] = folder_hints.get(top_folder, 0) + len(audio_files)
for fname in audio_files:
full_path = os.path.join(root, fname)
try:
tags = runtime.read_tags(full_path)
if tags:
album = (tags.get("album") or [None])[0]
artist = (tags.get("artist") or (tags.get("albumartist") or [None]))[0]
if album:
key = (album.strip(), (artist or "").strip())
tag_albums[key] = tag_albums.get(key, 0) + 1
except Exception as exc:
runtime.logger.debug("tag read failed: %s", exc)
queries = []
seen_queries_lower = set()
for (album, artist), _count in sorted(tag_albums.items(), key=lambda x: -x[1]):
query = f"{album} {artist}".strip() if artist else album
if query.lower() not in seen_queries_lower:
seen_queries_lower.add(query.lower())
queries.append(query)
for folder, _count in sorted(folder_hints.items(), key=lambda x: -x[1]):
query = folder.replace("_", " ")
if query.lower() not in seen_queries_lower:
seen_queries_lower.add(query.lower())
queries.append(query)
return {"success": True, "hints": queries[:5]}, 200
except Exception as exc:
runtime.logger.error("Error getting staging hints: %s", exc)
return {"success": False, "error": str(exc)}, 500
def staging_suggestions() -> tuple[Dict[str, Any], int]:
"""Return cached import suggestions and readiness state."""
cache = get_import_suggestions_cache()
return {"success": True, "suggestions": cache["suggestions"], "ready": cache["built"]}, 200
def search_albums(runtime: ImportRouteRuntime, query: str, limit: int = 12) -> tuple[Dict[str, Any], int]:
"""Search albums for manual import using the active metadata provider."""
try:
query = (query or "").strip()
if not query:
return {"success": False, "error": "Missing query parameter"}, 400
limit = min(int(limit), 50)
if runtime.get_primary_source() == "hydrabase" and runtime.hydrabase_worker and runtime.dev_mode_enabled:
runtime.hydrabase_worker.enqueue(query, "albums")
albums = runtime.search_import_albums(query, limit=limit)
return {"success": True, "albums": albums}, 200
except Exception as exc:
runtime.logger.error("Error searching albums for import: %s", exc)
return {"success": False, "error": str(exc)}, 500
def album_match(runtime: ImportRouteRuntime, data: Dict[str, Any]) -> tuple[Dict[str, Any], int]:
"""Match staging files to an album's tracklist."""
try:
data = data or {}
album_id = data.get("album_id")
album_name = data.get("album_name", "")
album_artist = data.get("album_artist", "")
source = str(data.get("source") or "").strip().lower()
filter_file_paths = set(data.get("file_paths", []))
if not album_id:
return {"success": False, "error": "Missing album_id"}, 400
if not source:
runtime.logger.warning(
"[Import Match] Missing 'source' on album_id=%s - lookup will "
"guess via primary-source priority chain. If this fires "
"consistently, a frontend caller is dropping source from "
"the match POST body.",
album_id,
)
payload = runtime.build_album_import_match_payload(
album_id,
album_name=album_name,
album_artist=album_artist,
file_paths=filter_file_paths,
source=source or None,
)
return payload, 200
except Exception as exc:
runtime.logger.error("Error matching album for import: %s", exc)
return {"success": False, "error": str(exc)}, 500
def album_process(runtime: ImportRouteRuntime, data: Dict[str, Any]) -> tuple[Dict[str, Any], int]:
"""Process matched album files through the post-processing pipeline."""
try:
data = data or {}
album = data.get("album", {})
matches = data.get("matches", [])
if not album or not matches:
return {"success": False, "error": "Missing album or matches data"}, 400
if runtime.post_process_matched_download is None:
return {"success": False, "error": "Import post-processing not available"}, 500
processed = 0
errors = []
album_name = album.get("name", album.get("album_name", "Unknown Album"))
artist_name = album.get("artist", album.get("artist_name", "Unknown Artist"))
album_id = album.get("id", album.get("album_id", ""))
source = str(album.get("source") or data.get("source") or "").strip().lower()
total_discs = max(
(
match.get("track", {}).get("disc_number", 1)
for match in matches
if match.get("track")
),
default=1,
)
artist_context = runtime.resolve_album_artist_context(album, source=source)
for match in matches:
staging_file = match.get("staging_file")
track = match.get("track") or {}
if not staging_file or not track:
continue
file_path = staging_file.get("full_path", "")
if not os.path.isfile(file_path):
errors.append(f"File not found: {staging_file.get('filename', '?')}")
continue
track_name = track.get("name", "Unknown Track")
track_number = track.get("track_number", 1)
context_key = f"import_album_{album_id}_{track_number}_{uuid.uuid4().hex[:8]}"
context = runtime.build_album_import_context(
album,
track,
artist_context=artist_context,
total_discs=total_discs,
source=source,
)
try:
runtime.post_process_matched_download(context_key, context, file_path)
processed += 1
runtime.logger.info("Import processed: %s. %s from %s", track_number, track_name, album_name)
except Exception as proc_err:
err_msg = f"{track_name}: {str(proc_err)}"
errors.append(err_msg)
runtime.logger.error("Import processing error: %s", err_msg)
if runtime.add_activity_item:
runtime.add_activity_item("", "Album Imported", f"{album_name} by {artist_name} ({processed}/{len(matches)} tracks)", "Now")
if processed > 0:
_emit_import_completed(
runtime,
track_count=processed,
album_name=album_name or "",
artist=artist_name or "",
playlist_name=f"Import: {album_name}" if album_name else "Import",
total_tracks=len(matches),
failed_tracks=len(errors),
log_label="album",
)
runtime.refresh_import_suggestions_cache()
return {"success": True, "processed": processed, "total": len(matches), "errors": errors}, 200
except Exception as exc:
runtime.logger.error("Error processing album import: %s", exc)
return {"success": False, "error": str(exc)}, 500
def search_tracks(runtime: ImportRouteRuntime, query: str, limit: int = 10) -> tuple[Dict[str, Any], int]:
"""Search tracks for manual single import using metadata source priority."""
try:
query = (query or "").strip()
if not query:
return {"success": False, "error": "Missing query parameter"}, 400
limit = min(int(limit), 30)
if runtime.get_primary_source() == "hydrabase" and runtime.hydrabase_worker and runtime.dev_mode_enabled:
runtime.hydrabase_worker.enqueue(query, "tracks")
tracks = runtime.search_import_tracks(query, limit=limit)
return {"success": True, "tracks": tracks}, 200
except Exception as exc:
runtime.logger.error("Error searching tracks for import: %s", exc)
return {"success": False, "error": str(exc)}, 500
def process_single_import_file(runtime: ImportRouteRuntime, file_info: Dict[str, Any]) -> tuple[str, str]:
"""Validate, resolve metadata, and post-process one single import file."""
file_path = file_info.get("full_path", "")
if not os.path.isfile(file_path):
return ("error", f"File not found: {file_info.get('filename', '?')}")
if runtime.post_process_matched_download is None:
return ("error", "Import post-processing not available")
title = file_info.get("title", "")
artist = file_info.get("artist", "")
manual_match = file_info.get("manual_match")
if manual_match is not None and not isinstance(manual_match, dict):
manual_match = None
manual_match_source = ""
manual_match_id = None
if manual_match:
manual_match_source = str(manual_match.get("source") or "").strip().lower()
manual_match_id = str(manual_match.get("id") or "").strip()
if not manual_match_id or not manual_match_source:
return ("error", f"Malformed manual match for file: {file_info.get('filename', '?')}")
if not title and not manual_match:
parsed = runtime.parse_filename_metadata(file_info.get("filename", ""))
title = parsed.get("title") or os.path.splitext(file_info.get("filename", "Unknown"))[0]
if not artist:
artist = parsed.get("artist", "")
try:
resolved = runtime.get_single_track_import_context(
title,
artist,
override_id=manual_match_id,
override_source=manual_match_source,
)
context = runtime.normalize_import_context(resolved["context"])
artist_data = runtime.get_import_context_artist(context)
track_data = runtime.get_import_track_info(context)
final_title = track_data.get("name", title)
final_artist = artist_data.get("name", artist)
context_key = f"import_single_{uuid.uuid4().hex[:8]}"
runtime.post_process_matched_download(context_key, context, file_path)
runtime.logger.info(
"Import single processed: %s by %s (source=%s)",
final_title,
final_artist,
resolved.get("source") or "local",
)
return ("ok", final_title)
except Exception as proc_err:
err_msg = f"{title}: {str(proc_err)}"
runtime.logger.error("Import single processing error: %s", err_msg)
return ("error", err_msg)
def singles_process(runtime: ImportRouteRuntime, files: list[Dict[str, Any]]) -> tuple[Dict[str, Any], int]:
"""Process individual staging files as singles through the import pipeline."""
try:
files = files or []
if not files:
return {"success": False, "error": "No files provided"}, 400
if runtime.import_singles_executor is None:
return {"success": False, "error": "Import executor not available"}, 500
processed = 0
errors = []
process_file = runtime.process_single_import_file or process_single_import_file
future_to_filename = {
runtime.import_singles_executor.submit(process_file, runtime, file_info):
file_info.get("filename", "?")
for file_info in files
}
for future in as_completed(future_to_filename):
try:
outcome, payload = future.result()
except Exception as worker_err:
errors.append(f"{future_to_filename[future]}: worker crashed: {worker_err}")
continue
if outcome == "ok":
processed += 1
else:
errors.append(payload)
if runtime.add_activity_item:
runtime.add_activity_item("", "Singles Imported", f"{processed}/{len(files)} tracks processed", "Now")
if processed > 0:
_emit_import_completed(
runtime,
track_count=processed,
album_name="",
artist="Various",
playlist_name="Import: Singles",
total_tracks=len(files),
failed_tracks=len(errors),
log_label="singles",
)
runtime.refresh_import_suggestions_cache()
return {"success": True, "processed": processed, "total": len(files), "errors": errors}, 200
except Exception as exc:
runtime.logger.error("Error processing singles import: %s", exc)
return {"success": False, "error": str(exc)}, 500
def _emit_import_completed(
runtime: ImportRouteRuntime,
*,
track_count: int,
album_name: str,
artist: str,
playlist_name: str,
total_tracks: int,
failed_tracks: int,
log_label: str,
) -> None:
# Keep import automation on the same chain as download batches:
# batch_complete -> auto-scan -> library_scan_completed -> auto-update DB.
try:
if runtime.automation_engine:
runtime.automation_engine.emit(
"import_completed",
{
"track_count": str(track_count),
"album_name": album_name,
"artist": artist,
},
)
runtime.automation_engine.emit(
"batch_complete",
{
"playlist_name": playlist_name,
"total_tracks": str(total_tracks),
"completed_tracks": str(track_count),
"failed_tracks": str(failed_tracks),
},
)
except Exception as exc:
runtime.logger.debug("%s import automation emit failed: %s", log_label, exc)

View file

@ -37,7 +37,7 @@ import os
import traceback
from dataclasses import dataclass
from difflib import SequenceMatcher
from typing import Any, Callable
from typing import Any, Callable, Optional
logger = logging.getLogger(__name__)
@ -62,6 +62,13 @@ class RetagDeps:
_get_retag_state: Callable[[], dict]
_set_retag_state: Callable[[dict], None]
get_database: Callable[[], Any]
# Discord report (Netti93) — retag was clearing the LYRICS / USLT
# tag without rewriting it, while the download pipeline calls
# `generate_lrc_file` after enrichment to refetch + embed lyrics.
# Injected here so retag mirrors the same post-enrichment step.
# Optional for backward compat with any test caller that builds
# RetagDeps without the new field — empty default no-ops the call.
generate_lrc_file: Optional[Callable] = None
@property
def retag_state(self) -> dict:
@ -230,6 +237,20 @@ def execute_retag(group_id, album_id, deps: RetagDeps):
except Exception as meta_err:
logger.error(f"[Retag] Metadata write failed for '{track_title}': {meta_err}")
# Discord report (Netti93) — `enhance_file_metadata` clears
# ALL tags (incl. USLT lyrics) and rewrites only the source
# metadata. The download pipeline calls `generate_lrc_file`
# after enrichment to refetch + embed lyrics — retag was
# missing that step and dropped the LYRICS tag with no
# rewrite. Mirroring the download path's post-enrichment
# step. Same args, same `lrclib_enabled` config gate, same
# idempotency (skip when sidecar already present).
if deps.generate_lrc_file:
try:
deps.generate_lrc_file(current_file_path, context, new_artist, album_info)
except Exception as lrc_err:
logger.debug("[Retag] generate_lrc_file failed for '%s': %s", track_title, lrc_err)
# Compute new path and move if different
file_ext = os.path.splitext(current_file_path)[1]
try:

View file

@ -52,9 +52,29 @@ class LyricsClient:
lrc_path = os.path.splitext(audio_file_path)[0] + '.lrc'
txt_path = os.path.splitext(audio_file_path)[0] + '.txt'
# Skip if lyrics file already exists (either .lrc or .txt)
# Sidecar already exists — skip the LRClib fetch but still
# re-embed the lyrics in the audio file's tag. The retag
# flow clears all tags including USLT and then runs this
# helper to restore them; without the embed step the
# LYRICS tag stays empty even though the .lrc is right
# there next to the file (Discord report — Netti93).
if os.path.exists(lrc_path) or os.path.exists(txt_path):
logger.debug(f"Lyrics file already exists for: {os.path.basename(audio_file_path)}")
existing_path = lrc_path if os.path.exists(lrc_path) else txt_path
try:
with open(existing_path, 'r', encoding='utf-8') as f:
existing_lyrics = f.read().strip()
if existing_lyrics:
self._embed_lyrics(audio_file_path, existing_lyrics)
logger.debug(
"Re-embedded lyrics from existing %s for: %s",
os.path.basename(existing_path),
os.path.basename(audio_file_path),
)
except Exception as e:
logger.debug(
"Could not re-embed lyrics from existing sidecar %s: %s",
os.path.basename(existing_path), e,
)
return True
# Fetch lyrics from LRClib

View file

@ -0,0 +1,143 @@
"""Find a matching AcoustID candidate for an expected (title, artist).
AcoustID returns multiple recordings per fingerprint same audio can
correspond to multiple MusicBrainz recordings (different releases,
different metadata-quality entries, sample / cover-version collisions).
The "top" recording AcoustID returns isn't always the one whose
metadata matches the user's expected track.
Both the post-download verifier (`core/acoustid_verification.py`) and
the AcoustID library scanner (`core/repair_jobs/acoustid_scanner.py`)
need to ask: "given these candidates, does ANY of them match
(expected_title, expected_artist) by title+artist similarity?" The
verifier had its own inline loop; the scanner only checked the top
match false positives whenever the wrong-credited recording out-
ranked the right-credited one.
This module is the single shared boundary for that question.
"""
from __future__ import annotations
from typing import Any, Callable, Dict, Iterable, Optional, Tuple
from utils.logging_config import get_logger
logger = get_logger("matching.acoustid_candidates")
def find_matching_recording(
recordings: Iterable[Dict[str, Any]],
expected_title: str,
expected_artist: str,
*,
title_threshold: float = 0.70,
artist_threshold: float = 0.60,
similarity: Optional[Callable[[str, str], float]] = None,
artist_similarity: Optional[Callable[[str, str], float]] = None,
skip_predicate: Optional[Callable[[Dict[str, Any]], bool]] = None,
) -> Tuple[Optional[Dict[str, Any]], float, float]:
"""Return the first AcoustID candidate whose metadata passes both
title + artist similarity thresholds.
Args:
recordings: AcoustID recording dicts. Each must carry ``title``
and ``artist`` strings; entries without both are skipped.
expected_title: The track title the caller expected.
expected_artist: The artist the caller expected.
title_threshold: Minimum title similarity to accept (default 0.70).
artist_threshold: Minimum artist similarity to accept (default 0.60).
similarity: ``(a, b) -> float`` for title comparison. Defaults
to a lowercase exact-equals stub when not supplied callers
should pass their stricter normaliser (verifier passes its
parenthetical-stripping ``_similarity``; scanner passes
its own).
artist_similarity: ``(expected, actual) -> float`` for artist
comparison. Lets callers supply alias-aware comparison
(verifier wraps ``_alias_aware_artist_sim``; scanner wraps
``artist_names_match``). Defaults to ``similarity`` if
unset.
skip_predicate: Optional ``(recording_dict) -> bool``. When
truthy, the candidate is skipped (used by the verifier to
drop wrong-version recordings instrumental vs vocal etc).
Returns:
``(recording, title_sim, artist_sim)`` for the first matching
candidate, or ``(None, best_title_sim, best_artist_sim)`` when
none match. The non-None ``best_*`` values let callers report
the closest near-miss when they need to log why nothing matched.
Iteration order matches the input order (typically AcoustID's own
fingerprint-confidence ranking). Returns on first match does NOT
score every candidate looking for the highest sim.
"""
if not expected_title or not expected_artist:
return None, 0.0, 0.0
sim = similarity or _default_similarity
asim = artist_similarity or sim
best_title_sim = 0.0
best_artist_sim = 0.0
for rec in recordings or ():
if not isinstance(rec, dict):
continue
rec_title = (rec.get('title') or '').strip()
rec_artist = (rec.get('artist') or '').strip()
if not rec_title or not rec_artist:
continue
if skip_predicate and skip_predicate(rec):
continue
title_sim = sim(expected_title, rec_title)
if title_sim > best_title_sim:
best_title_sim = title_sim
artist_sim = asim(expected_artist, rec_artist)
if artist_sim > best_artist_sim:
best_artist_sim = artist_sim
if title_sim >= title_threshold and artist_sim >= artist_threshold:
return rec, title_sim, artist_sim
return None, best_title_sim, best_artist_sim
def _default_similarity(a: str, b: str) -> float:
if not a or not b:
return 0.0
return 1.0 if a.lower().strip() == b.lower().strip() else 0.0
# ────────────────────────────────────────────────────────────────────
# Duration guard — codex item (5).
# ────────────────────────────────────────────────────────────────────
def duration_mismatches_strongly(
expected_seconds: Optional[float],
candidate_seconds: Optional[float],
*,
abs_tolerance_s: float = 60.0,
rel_tolerance: float = 0.35,
) -> bool:
"""Return True when the candidate's duration is too far from expected
to confidently treat it as the same recording.
Catches fingerprint hash collisions (the reporter's 17-minute
mashup 5-minute Japanese hiphop track case). When EITHER duration
is unknown / non-positive, returns False no behavior change.
Threshold: drift greater than max(``abs_tolerance_s``,
``rel_tolerance * expected``). The relative term scales with track
length so a 20% mismatch on a 3-minute track and a 20% mismatch on
a 30-minute mix are both treated as suspicious.
"""
if not expected_seconds or expected_seconds <= 0:
return False
if not candidate_seconds or candidate_seconds <= 0:
return False
drift = abs(float(candidate_seconds) - float(expected_seconds))
threshold = max(abs_tolerance_s, rel_tolerance * float(expected_seconds))
return drift > threshold

View file

@ -0,0 +1,195 @@
"""Strip redundant album-context suffixes from track titles.
Issue #589 — MTV Unplugged albums (and similar live-concert / session
releases) have source-side track titles like ``"Shy Away (MTV Unplugged
Live)"`` while the local DB stored title is just ``"Shy Away"``. The
album-scoped library check at ``core/downloads/master.py`` compares
the two with raw string similarity, the length asymmetry tanks the
score, and tracks the user already owns get marked missing.
This helper normalizes a track title by stripping the parenthetical
or dash suffix when its tokens are fully subsumed by the album
context: at least one version marker (live / unplugged / acoustic /
session / etc) is present in BOTH the suffix AND the album title, and
every other suffix token is either a known marker, a year, a
connecting noise word, or a word that appears in the album title.
Pure function. No I/O. Tests at the function boundary.
"""
from __future__ import annotations
import re
from typing import Iterable, Tuple
# Version-marker keywords. When the album title contains any of these,
# stripping is enabled. Singular forms — plurals get matched separately
# via stem expansion below.
_VERSION_MARKERS = (
'live',
'unplugged',
'acoustic',
'session',
'concert',
'tour',
)
# Markers that are implied "live" context — when the album mentions any
# of these, a bare ``live`` token in the suffix counts as album context
# even if the album title doesn't literally say "live". MTV Unplugged
# albums are live recordings; same for "in concert" / "tour" releases.
_IMPLIES_LIVE = ('unplugged', 'concert', 'tour', 'session')
# Connecting / filler words that don't carry meaning by themselves.
_NOISE_TOKENS = frozenset({
'version', 'edition', 'recording', 'recordings', 'remaster',
'remastered', 'mix',
'the', 'a', 'an', 'from', 'at', 'in', 'on', 'for', 'of',
'and', 'or', 'with', 'by',
'vol', 'pt', 'part', 'no',
})
_SUFFIX_PATTERNS: Tuple[re.Pattern, ...] = (
re.compile(r'\s*\(([^()]+)\)\s*$'),
re.compile(r'\s*\[([^\[\]]+)\]\s*$'),
re.compile(r'\s+-\s+(.+?)\s*$'),
)
_YEAR_RE = re.compile(r'^(?:19|20)\d{2}$')
_TOKEN_RE = re.compile(r'\w+')
def _normalize(text: str) -> str:
return (text or '').lower().strip()
def _tokenize(text: str) -> set:
return set(_TOKEN_RE.findall(_normalize(text)))
def _expand_marker_set(markers: Iterable[str]) -> set:
"""Expand each marker into its singular + plural forms."""
out = set()
for marker in markers:
out.add(marker)
if not marker.endswith('s'):
out.add(marker + 's')
return out
_EXPANDED_MARKERS = _expand_marker_set(_VERSION_MARKERS)
def album_context_markers(album_title: str) -> Tuple[str, ...]:
"""Return the version markers present in the album title (singular form)."""
if not album_title:
return ()
album_tokens = _tokenize(album_title)
found = []
for marker in _VERSION_MARKERS:
if marker in album_tokens or (marker + 's') in album_tokens:
found.append(marker)
return tuple(found)
def _suffix_is_album_redundant(
inner: str,
album_tokens: set,
album_markers: Tuple[str, ...],
) -> bool:
"""Decide whether a suffix's tokens are all subsumed by album context.
Three requirements:
1. The suffix contains at least one version-marker token. Stops
a generic "feat. X" suffix from being stripped because the
album happened to be live.
2. The shared marker matches one the album implies either
literally in the album title, OR via the implied-live set
(unplugged/concert/tour albums imply "live").
3. Every other suffix token is either a marker, a year, a
tolerated noise word, or a word that appears in the album
title. If any token falls outside, the suffix carries
info beyond album context (featured artist, different
version, etc) keep it on.
"""
if not inner:
return False
suffix_tokens = _tokenize(inner)
if not suffix_tokens:
return False
# Markers the album effectively implies (literal + implied-live).
implied_markers = set(album_markers)
if any(m in implied_markers for m in _IMPLIES_LIVE):
implied_markers.add('live')
suffix_markers = suffix_tokens & _EXPANDED_MARKERS
if not suffix_markers:
return False
# At least one marker must overlap with album-implied set. Plural
# tolerance — strip trailing 's' for the comparison.
def _stem(tok: str) -> str:
return tok[:-1] if tok.endswith('s') and len(tok) > 1 else tok
if not any(_stem(t) in implied_markers for t in suffix_markers):
return False
# Every remaining suffix token must be subsumed.
for tok in suffix_tokens:
if tok in _EXPANDED_MARKERS:
continue
if _YEAR_RE.match(tok):
continue
if tok in _NOISE_TOKENS:
continue
if tok in album_tokens:
continue
return False
return True
def strip_redundant_album_suffix(track_title: str, album_title: str) -> str:
"""Strip a trailing parenthetical/bracket/dash suffix from `track_title`
when the suffix duplicates context already implied by `album_title`.
Examples:
- ("Shy Away (MTV Unplugged Live)", "MTV Unplugged") "Shy Away"
- ("Only If For A Night (MTV Unplugged, 2012 / Live)",
"Ceremonials (Live At MTV Unplugged)") "Only If For A Night"
- ("In My Feelings (Instrumental)", "Scorpion")
unchanged (instrumental NOT implied by studio album)
- ("Hello (Live - feat. Other)", "Live At Wembley")
unchanged (suffix carries featured-artist beyond album context)
- ("Shy Away", "MTV Unplugged") unchanged (no suffix)
Pure function never raises, returns the input unchanged on any
edge / unexpected input.
"""
if not track_title:
return track_title or ''
album_markers = album_context_markers(album_title)
if not album_markers:
return track_title
album_tokens = _tokenize(album_title)
stripped = track_title
# Stacked suffixes ("Track (MTV Unplugged) [Live]") — peel one at a
# time. Bound the loop defensively.
for _ in range(4):
peeled = None
for pattern in _SUFFIX_PATTERNS:
m = pattern.search(stripped)
if not m:
continue
inner = m.group(1)
if _suffix_is_album_redundant(inner, album_tokens, album_markers):
peeled = stripped[: m.start()].rstrip()
break
if peeled is None:
return stripped
stripped = peeled
return stripped

View file

@ -0,0 +1,74 @@
"""Pure artist-list resolution for tag-write paths.
Single source of truth for "what is the canonical multi-value artists
list for this track?" Different download paths populate `context` with
different keys Deezer-direct downloads stamp `original_search.artists`
as a proper list, but Soulseek matched downloads only carry `artist`
(singular string) in `original_search_result` while the full list lives
on `track_info` (the full Spotify track object).
Resolution order:
1. `context.original_search_result.artists` (preferred already-
curated by the source path that constructed the context)
2. `context.track_info.artists` (Spotify/Deezer/Tidal full track
object always carries the artists array when matched)
3. `[artist_dict.name]` as a single-element fallback when neither
carries a list (primary-artist-only)
Each list item may be a dict with a `name` key (Spotify shape), a bare
string, or any other object the helper normalizes all three to
strings and drops empty entries.
"""
from __future__ import annotations
from typing import Any, Dict, List, Optional
def _normalize_artists_iterable(items: Any) -> List[str]:
if not isinstance(items, list):
return []
result: List[str] = []
for item in items:
if isinstance(item, dict):
name = item.get("name")
if isinstance(name, str) and name.strip():
result.append(name.strip())
elif isinstance(item, str):
stripped = item.strip()
if stripped:
result.append(stripped)
elif item is not None:
text = str(item).strip()
if text:
result.append(text)
return result
def resolve_track_artists(
original_search: Optional[Dict[str, Any]],
track_info: Optional[Dict[str, Any]],
artist_dict: Optional[Dict[str, Any]],
) -> List[str]:
"""Return the canonical multi-value artists list for tag-write.
Falls through preferred track_info primary-artist fallback. Each
candidate is normalized to a list of stripped non-empty strings.
Empty list returned only when every candidate is empty/invalid.
"""
if isinstance(original_search, dict):
primary = _normalize_artists_iterable(original_search.get("artists"))
if primary:
return primary
if isinstance(track_info, dict):
secondary = _normalize_artists_iterable(track_info.get("artists"))
if secondary:
return secondary
if isinstance(artist_dict, dict):
name = artist_dict.get("name")
if isinstance(name, str) and name.strip():
return [name.strip()]
return []

View file

@ -102,7 +102,19 @@ def enhance_file_metadata(file_path: str, context: dict, artist: dict, album_inf
save_audio_file(audio_file, symbols)
return True
track_num_str = f"{metadata.get('track_number', 1)}/{metadata.get('total_tracks', 1)}"
# Discord report (Netti93) — many album-dict construction
# sites pass `total_tracks: 0` when source data is incomplete
# (per types.py, 0 means "unknown"). Pre-fix this serialized
# to "6/0" tags. Helper drops the `/N` suffix when total is
# unknown so the tag reads "6" instead — matches retag's
# behavior at core/tag_writer.py and ID3 spec convention.
from core.metadata.track_number_format import (
format_track_number_tag,
format_track_number_tuple,
)
track_num_str = format_track_number_tag(
metadata.get('track_number'), metadata.get('total_tracks')
)
write_multi = cfg.get("metadata_enhancement.tags.write_multi_artist", False)
artists_list = metadata.get("_artists_list", [])
@ -167,7 +179,9 @@ def enhance_file_metadata(file_path: str, context: dict, artist: dict, album_inf
audio_file["\xa9day"] = [metadata["date"]]
if metadata.get("genre"):
audio_file["\xa9gen"] = [metadata["genre"]]
audio_file["trkn"] = [(metadata.get("track_number", 1), metadata.get("total_tracks", 1))]
audio_file["trkn"] = [format_track_number_tuple(
metadata.get("track_number"), metadata.get("total_tracks")
)]
if metadata.get("disc_number"):
audio_file["disk"] = [(metadata["disc_number"], 0)]

View file

@ -23,6 +23,7 @@ from core.imports.context import (
get_source_tag_names,
normalize_import_context,
)
from core.metadata.artist_resolution import resolve_track_artists
from core.metadata.registry import get_itunes_client
from database.music_database import get_database
from core.metadata.common import (
@ -907,17 +908,13 @@ def extract_source_metadata(context: dict, artist: dict, album_info: dict) -> di
else:
logger.warning("Metadata: Using original title as fallback: '%s'", metadata["title"])
artists = original_search.get("artists")
if isinstance(artists, list) and artists:
all_artists = []
for artist_item in artists:
if isinstance(artist_item, dict) and artist_item.get("name"):
all_artists.append(artist_item["name"])
elif isinstance(artist_item, str):
all_artists.append(artist_item)
else:
all_artists.append(str(artist_item))
# Resolve canonical artists list. Soulseek matched-download contexts
# only carry `original_search.artist` (singular string) — the full
# contributors list lives on `track_info` (the matched Spotify/etc
# track object). Deezer-direct contexts populate `original_search.artists`
# directly. Pure helper handles all three shapes.
all_artists = resolve_track_artists(original_search, track_info, artist_dict)
if all_artists:
# 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

View file

@ -0,0 +1,77 @@
"""Format track-number tags consistently across audio formats.
Discord report (Netti93): album tracks were tagged as ``TRCK = "6/0"``
instead of ``"6/13"``. Cause: many album-dict construction sites in
the codebase pass ``total_tracks: 0`` when the source data is
incomplete, and ``core/metadata/enrichment.py`` formatted the tag
unconditionally as ``f"{track_number}/{total_tracks}"`` so 0
propagated straight to disk. The retag path was unaffected because
``core/tag_writer.py`` already does the right thing.
Per ``core/metadata/types.py``, ``total_tracks = 0`` is documented
as "unknown" not an actual track count. Fix at the consumer
boundary so every album-dict constructor doesn't need to be touched.
This module provides one pure helper. Tests at the function boundary.
"""
from __future__ import annotations
from typing import Optional, Tuple
def format_track_number_tag(
track_number: Optional[int],
total_tracks: Optional[int],
) -> str:
"""Return the canonical TRCK / tracknumber tag string.
- ``track_number=6, total_tracks=13`` ``"6/13"``
- ``track_number=6, total_tracks=0`` ``"6"`` (total unknown)
- ``track_number=6, total_tracks=None`` ``"6"``
- ``track_number=None, total_tracks=13`` ``"1/13"`` (track defaults to 1)
- ``track_number=None, total_tracks=None`` ``"1"``
ID3 spec allows ``TRCK`` to be either ``"N"`` or ``"N/M"``. Vorbis
``tracknumber`` follows the same convention. Avoiding the ``/0``
suffix keeps the tag honest most media servers and taggers
interpret ``6/0`` as "track 6 of 0" which is nonsensical, while
``6`` reads as "track 6, total unknown".
"""
num = _coerce_positive_int(track_number, default=1)
total = _coerce_positive_int(total_tracks, default=0)
if total > 0:
return f"{num}/{total}"
return str(num)
def format_track_number_tuple(
track_number: Optional[int],
total_tracks: Optional[int],
) -> Tuple[int, int]:
"""Return the MP4 ``trkn`` tuple ``(track, total)``.
MP4 tag spec stores track-of as a 2-int tuple convention is
``(N, 0)`` when the total is unknown. Same coercion rules as
``format_track_number_tag``: missing / None / non-positive
``track_number`` defaults to 1, missing / 0 / negative
``total_tracks`` returns 0 (the spec's "unknown" marker).
"""
num = _coerce_positive_int(track_number, default=1)
total = _coerce_positive_int(total_tracks, default=0)
return (num, total)
def _coerce_positive_int(value, *, default: int) -> int:
"""Coerce to a non-negative int. Falls back to ``default`` for
None / non-numeric / negative input. Floats truncate.
"""
if value is None:
return default
try:
coerced = int(value)
except (TypeError, ValueError):
return default
if coerced < 0:
return default
return coerced

View file

@ -428,54 +428,52 @@ class MusicBrainzService:
return []
# Tier 3: live MB lookup. Search → fetch by MBID → cache.
try:
results = self.mb_client.search_artist(artist_name, limit=3)
except Exception as e:
logger.debug("lookup_artist_aliases: search_artist(%r) raised: %s", artist_name, e)
self._save_to_cache('artist_aliases', artist_name, None, None, {'aliases': []}, 0)
return []
if not results:
self._save_to_cache('artist_aliases', artist_name, None, None, {'aliases': []}, 0)
return []
# Score each result: combined of name-similarity + MB's own
# relevance. Score range 0.0-1.0.
scored = []
for result in results:
mb_name = result.get('name', '')
mb_score = result.get('score', 0)
sim = self._calculate_similarity(artist_name, mb_name)
combined = (sim * 0.7) + (mb_score / 100 * 0.3)
mbid = result.get('id')
if mbid:
scored.append((combined, mbid))
# Issue #586 — strict search queries `artist:"..."` only and
# MISSES alias / sortname indexes. When MB's canonical name is
# the non-Latin form (e.g. `Дмитрий Яблонский`), the user's
# Latin input ("Dmitry Yablonsky") finds nothing under strict.
# Fall back to non-strict (bare query, hits alias + sortname
# indexes) when strict returns empty OR all results fail the
# trust gate.
scored = self._search_and_score_artists(artist_name, strict=True)
if not scored or self._best_score(scored) < 0.85:
non_strict = self._search_and_score_artists(artist_name, strict=False)
if non_strict and (not scored or self._best_score(non_strict) > self._best_score(scored)):
scored = non_strict
if not scored:
self._save_to_cache('artist_aliases', artist_name, None, None, {'aliases': []}, 0)
return []
scored.sort(key=lambda x: -x[0])
best_score, best_mbid = scored[0]
best_score, best_mbid, best_mb_score = scored[0]
# Strict trust threshold: real matches for distinctive cross-
# script artists (the user-reported case) score >= 0.95.
# Anything below 0.85 is ambiguous and not worth the false-
# positive risk of pulling in aliases for the wrong artist.
if best_score < 0.85:
# Trust gate. Two ways to pass:
# 1. Combined score >= 0.85 (the historical strict bar that
# catches same-script matches)
# 2. MB's OWN score is very high (>= 95) AND the result is
# unambiguous (top result clearly leads). Bridges the
# cross-script case where local similarity is near zero
# ("Dmitry Yablonsky" vs "Дмитрий Яблонский" sim ~0)
# but MB's index found a high-confidence match.
passes_combined = best_score >= 0.85
passes_mb_only = best_mb_score >= 95 and (
len(scored) < 2 or (scored[0][2] - scored[1][2]) >= 5
)
if not (passes_combined or passes_mb_only):
logger.debug(
"lookup_artist_aliases: best match for %r below trust "
"threshold (score=%.2f)", artist_name, best_score,
"threshold (combined=%.2f, mb_score=%d)",
artist_name, best_score, best_mb_score,
)
self._save_to_cache('artist_aliases', artist_name, None, None, {'aliases': []}, 0)
return []
# Ambiguity detection: when 2+ results both score high (within
# 0.1 of the best), the search hit multiple distinct artists
# with similar names ("John Smith" returning 10 different
# John Smiths all at score 100). Pulling aliases for one of
# them could produce wrong matches. Skip + cache empty.
if len(scored) >= 2 and (scored[0][0] - scored[1][0]) < 0.1:
# 0.1 of the best combined), the search hit multiple distinct
# artists with similar names. Pulling aliases for one could
# produce wrong matches. Skip + cache empty.
if len(scored) >= 2 and (scored[0][0] - scored[1][0]) < 0.1 and not passes_mb_only:
logger.debug(
"lookup_artist_aliases: ambiguous match for %r — top "
"two results within 0.1 (%.2f / %.2f). Skipping alias lookup.",
@ -491,6 +489,40 @@ class MusicBrainzService:
)
return aliases
def _search_and_score_artists(self, artist_name: str, strict: bool):
"""Search MB for an artist and score each result.
Returns a list of (combined_score, mbid, raw_mb_score) tuples.
Combined score: 70% local similarity + 30% MB's own relevance
score (0..1). raw_mb_score preserved separately so the trust
gate can prefer high-MB-score results in cross-script cases
where local similarity is near zero.
Returns empty list on any failure.
"""
try:
results = self.mb_client.search_artist(artist_name, limit=3, strict=strict)
except Exception as e:
logger.debug(
"lookup_artist_aliases: search_artist(%r, strict=%s) raised: %s",
artist_name, strict, e,
)
return []
scored = []
for result in results or []:
mb_name = result.get('name', '')
mb_score = result.get('score', 0)
sim = self._calculate_similarity(artist_name, mb_name)
combined = (sim * 0.7) + (mb_score / 100 * 0.3)
mbid = result.get('id')
if mbid:
scored.append((combined, mbid, mb_score))
return scored
@staticmethod
def _best_score(scored):
return max((s[0] for s in scored), default=0.0) if scored else 0.0
def fetch_artist_aliases(self, mbid: str) -> list:
"""Fetch the alias list for an artist from MusicBrainz.
@ -499,6 +531,14 @@ class MusicBrainzService:
artist record. Pull them so SoulSync can recognise that
`澤野弘之` and `Hiroyuki Sawano` refer to the same artist.
Issue #586 — for some artists MB's CANONICAL `name` is the
non-Latin spelling (e.g. `Дмитрий Яблонский`) while the
Latin spelling lives in `aliases` but the inverse also
happens, where the Latin canonical name has the Cyrillic in
aliases. Either way the canonical `name` and `sort-name` are
themselves valid alternate spellings for matching purposes,
so include them alongside the explicit alias entries.
Returns the deduplicated list of alias `name` strings. Returns
empty list (NOT None) on any failure caller should treat
empty as "no aliases available, fall back to direct match" so
@ -514,24 +554,38 @@ class MusicBrainzService:
return []
if not data:
return []
raw_aliases = data.get('aliases') or []
seen = set()
cleaned = []
def _add(value):
if not isinstance(value, str):
return
text = value.strip()
if not text:
return
key = text.lower()
if key in seen:
return
seen.add(key)
cleaned.append(text)
# Canonical name + sort-name treated as aliases for matching —
# they're the strongest cross-script bridge when MB's
# canonical spelling differs from the user's input.
_add(data.get('name'))
_add(data.get('sort-name'))
# MB returns each alias as a dict with `name`, `sort-name`,
# `locale`, `primary`, `type`, etc. We only care about the
# display name — that's what `actual` artist strings will
# match against.
seen = set()
cleaned = []
for entry in raw_aliases:
# match against. Also pull alias sort-name when present
# (some entries have a different sortable form).
for entry in data.get('aliases') or []:
if not isinstance(entry, dict):
continue
name = (entry.get('name') or '').strip()
if not name:
continue
key = name.lower()
if key in seen:
continue
seen.add(key)
cleaned.append(name)
_add(entry.get('name'))
_add(entry.get('sort-name'))
return cleaned
def update_artist_aliases(self, artist_id: int, aliases: list) -> None:

View file

@ -254,6 +254,74 @@ class AcoustIDScannerJob(RepairJob):
if title_sim >= title_threshold and artist_sim >= artist_threshold:
return
# Issue #587 (Foxxify) — top recording's metadata mismatched, but
# AcoustID often returns multiple recordings per fingerprint
# (sample collisions, multi-MB-record cases). Check ALL of them
# before flagging — if any candidate's metadata matches expected
# title + artist, the file IS the right song and AcoustID's top
# match was just a wrong-credited recording.
from core.matching.acoustid_candidates import (
duration_mismatches_strongly,
find_matching_recording,
)
from core.matching.artist_aliases import artist_names_match
def _scanner_title_sim(a, b):
return SequenceMatcher(None, _normalize(a), _normalize(b)).ratio()
def _scanner_artist_sim(expected_a, actual_a):
_, score = artist_names_match(expected_a, actual_a, threshold=artist_threshold)
return score
candidate_match, _, _ = find_matching_recording(
fp_result.get('recordings') or [],
expected['title'],
expected_artist,
title_threshold=title_threshold,
artist_threshold=artist_threshold,
similarity=_scanner_title_sim,
artist_similarity=_scanner_artist_sim,
)
if candidate_match is not None:
# A lower-ranked candidate matched — file IS the right song.
# No finding.
if context.report_progress:
context.report_progress(
log_line=(
f'Resolved (lower-ranked candidate match): {fname}'
f'expected "{expected["title"]}" matched candidate '
f'"{candidate_match.get("title")}" by '
f'"{candidate_match.get("artist")}"'
),
log_type='ok',
)
return
# Issue #587 (Foxxify "17min mashup → 5min track") — duration
# guard against fingerprint hash collisions. When the file's
# actual duration differs from AcoustID's matched recording by
# more than max(60s, 35%), the fingerprint is almost certainly
# a sample/intro collision, not a real recording match. Don't
# produce a confident "Wrong Song" finding.
try:
file_duration_s = (expected.get('duration_ms') or 0) / 1000.0
except Exception:
file_duration_s = 0
candidate_duration_s = best_recording.get('duration')
if candidate_duration_s is None and best_recording.get('length'):
candidate_duration_s = best_recording.get('length')
if duration_mismatches_strongly(file_duration_s, candidate_duration_s):
if context.report_progress:
context.report_progress(
log_line=(
f'Skipped (duration mismatch suggests fingerprint collision): '
f'{fname} — expected {file_duration_s:.0f}s, AcoustID '
f'candidate {candidate_duration_s:.0f}s'
),
log_type='skip',
)
return
# Mismatch detected
if context.report_progress:
context.report_progress(
@ -326,7 +394,8 @@ class AcoustIDScannerJob(RepairJob):
t.file_path, t.track_number,
al.title AS album_title, al.thumb_url, ar.thumb_url,
NULLIF(t.track_artist, '') AS track_artist,
ar.name AS album_artist
ar.name AS album_artist,
t.duration
FROM tracks t
LEFT JOIN artists ar ON ar.id = t.artist_id
LEFT JOIN albums al ON al.id = t.album_id
@ -352,6 +421,11 @@ class AcoustIDScannerJob(RepairJob):
'artist_thumb_url': row[7] or None,
'track_artist': row[8] or '', # raw (may be empty)
'album_artist': row[9] or '',
# Duration in MS (DB stores ms). Used by the
# duration-mismatch guard to spot fingerprint
# collisions where the matched recording is a
# totally different length.
'duration_ms': row[10] or 0,
}
except Exception as e:
logger.error("Error loading tracks from DB: %s", e)

View file

@ -35,6 +35,23 @@ logger = get_logger("repair_worker")
AUDIO_EXTENSIONS = {'.mp3', '.flac', '.ogg', '.opus', '.m4a', '.aac', '.wav', '.wma', '.aiff', '.aif'}
def _split_acoustid_credit(credit: str) -> List[str]:
"""Split an AcoustID artist credit into individual contributor names.
Reuses the matching layer's credit splitter so the AcoustID retag
path tags multi-artist tracks the same way the post-download
enrichment pipeline does (comma / ampersand / feat. / etc).
Returns ``[credit]`` for single-artist credits the writer's
``len > 1`` check is what gates whether the multi-value tag gets
written.
"""
try:
from core.matching.artist_aliases import split_artist_credit
return split_artist_credit(credit)
except Exception:
return [credit] if credit else []
def _resolve_file_path(file_path, transfer_folder, download_folder=None,
config_manager=None, plex_client=None):
"""Resolve a stored DB path to an actual file on disk.
@ -1715,6 +1732,17 @@ class RepairWorker:
tag_updates = {'title': aid_title}
if aid_artist:
tag_updates['artist_name'] = aid_artist
# Issue #587 — derive a per-artist list from
# AcoustID's credit string when it carries
# multiple contributors. The post-download
# enrichment pipeline preserves multi-value
# ARTISTS tags via the user's
# `write_multi_artist` setting; the repair
# path was bypassing that and writing a
# single-string TPE1 only. Now respects the
# same setting via the writer's new
# `artists_list` derivation.
tag_updates['artists_list'] = _split_acoustid_credit(aid_artist)
write_tags_to_file(resolved, tag_updates)
logger.info("Wrote corrected tags to file: %s", resolved)
except Exception as tag_err:

0
core/sync/__init__.py Normal file
View file

View file

@ -0,0 +1,119 @@
"""Sync match overrides — user-confirmed source→server track pairings.
When a user picks a local file via "Find & Add" on the Server Playlist
compare view, that selection should persist as a hard match across
future syncs bypassing the fuzzy/exact title-match algorithm
entirely. This module provides pure helpers that the web layer calls
to resolve and persist those overrides through the existing
`sync_match_cache` table.
Override semantics:
- One mapping per (source_track_id, server_source). UNIQUE
constraint on the table enforces single mapping per pair.
- Stored with confidence=1.0 to distinguish from auto-discovered
matches (which use the actual title-similarity score).
- Read at the START of the matching algorithm before pass-1
exact and pass-2 fuzzy. Skipped sources don't re-enter the
normal matching pool.
- Stale-cache safe: if the cached server_track_id doesn't exist
in the current server_tracks list (track removed from server),
the override is silently skipped and normal matching runs.
"""
from __future__ import annotations
from typing import Any, Callable, Dict, List, Optional
def resolve_match_overrides(
source_tracks: List[Dict[str, Any]],
server_tracks: List[Dict[str, Any]],
cache_lookup: Callable[[str], Optional[Any]],
) -> Dict[int, int]:
"""Map source-track indexes to server-track indexes for cached overrides.
Pure function. `cache_lookup(source_track_id) -> server_track_id or
None` is injected by the caller (web layer wraps the DB call).
Returns ``{source_idx: server_idx}``. Only includes pairs where:
- source_track has a non-empty `source_track_id`
- cache_lookup returns a server_track_id
- that server_track_id exists in server_tracks (no stale cache
entries pointing at deleted tracks)
- the server_track hasn't already been claimed by an earlier
override (defensive UNIQUE on the cache table prevents this
in practice)
Caller uses the returned dict to short-circuit the per-source
matching loop: indices in the dict skip the exact/fuzzy passes.
"""
if not source_tracks or not server_tracks:
return {}
server_id_to_idx: Dict[str, int] = {}
for j, svr in enumerate(server_tracks):
sid = svr.get("id") if isinstance(svr, dict) else None
if sid is not None:
key = str(sid)
if key not in server_id_to_idx:
server_id_to_idx[key] = j
overrides: Dict[int, int] = {}
used_server: set[int] = set()
for i, src in enumerate(source_tracks):
if not isinstance(src, dict):
continue
src_id = src.get("source_track_id")
if not src_id:
continue
try:
cached_server_id = cache_lookup(str(src_id))
except Exception:
cached_server_id = None
if not cached_server_id:
continue
j = server_id_to_idx.get(str(cached_server_id))
if j is None or j in used_server:
continue
overrides[i] = j
used_server.add(j)
return overrides
def record_manual_match(
db: Any,
source_track_id: str,
server_source: str,
server_track_id: Any,
server_track_title: str = "",
source_title: str = "",
source_artist: str = "",
) -> bool:
"""Persist a user-confirmed source→server pairing as a hard override.
Wraps `db.save_sync_match_cache` with confidence=1.0 (the manual
match marker). Normalized title/artist fields are informational
only the cache is keyed by `(spotify_track_id, server_source)`,
so the normalization is just for inspection and future debugging.
Returns True on persist success, False on any failure (DB, missing
args, etc). Never raises.
"""
if not source_track_id or not server_source or server_track_id is None:
return False
if not hasattr(db, "save_sync_match_cache"):
return False
try:
return bool(db.save_sync_match_cache(
spotify_track_id=str(source_track_id),
normalized_title=(source_title or "").lower().strip(),
normalized_artist=(source_artist or "").lower().strip(),
server_source=server_source,
server_track_id=server_track_id,
server_track_title=server_track_title or "",
confidence=1.0,
))
except Exception:
return False

View file

@ -299,18 +299,31 @@ def write_tags_to_file(file_path: str, db_data: Dict[str, Any],
elif isinstance(genres, str):
genre_str = genres
# Multi-value artist support — issue #587. Caller can pass
# `artists_list` (per-track list of contributor names) and the
# writer respects the user's `metadata_enhancement.tags.write_multi_artist`
# config the same way the post-download enrichment pipeline does.
# When the setting is on AND the list has >1 entry:
# - ID3 keeps TPE1 as the joined display string (already in `artist`)
# and writes a separate TXXX:Artists frame with the list
# - Vorbis writes an `artists` multi-value key alongside `artist`
# - MP4 writes \xa9ART as the list when on, single string when off
# When OFF or the list is empty/single — same single-string write
# as before. Backward compatible for callers that don't pass it.
artists_list = _resolve_artists_list_for_write(db_data)
if isinstance(audio.tags, ID3):
written = _write_id3(audio, title, artist, album_artist, album,
year, genre_str, track_num, total_tracks,
disc_num, bpm)
disc_num, bpm, artists_list=artists_list)
elif isinstance(audio, (FLAC, OggVorbis)) or type(audio).__name__ == 'OggOpus':
written = _write_vorbis(audio, title, artist, album_artist, album,
year, genre_str, track_num, total_tracks,
disc_num, bpm)
disc_num, bpm, artists_list=artists_list)
elif isinstance(audio, MP4):
written = _write_mp4(audio, title, artist, album_artist, album,
year, genre_str, track_num, total_tracks,
disc_num, bpm)
disc_num, bpm, artists_list=artists_list)
# Embed cover art if requested
if embed_cover:
@ -343,8 +356,43 @@ def write_tags_to_file(file_path: str, db_data: Dict[str, Any],
# ── Format-specific writers ──
def _resolve_artists_list_for_write(db_data: Dict[str, Any]) -> Optional[List[str]]:
"""Pull a multi-value artists list from db_data when caller supplied one.
Accepts either ``artists_list`` (list of names) or ``artists`` (same
shape kept for symmetry with the post-process pipeline's
``_artists_list`` field). Drops empty / non-string entries. Returns
``None`` when no list was supplied so format writers can branch on
"single-string only" vs "multi-value too".
"""
raw = db_data.get('artists_list') or db_data.get('artists') or db_data.get('_artists_list')
if not raw:
return None
if not isinstance(raw, (list, tuple)):
return None
cleaned = []
for entry in raw:
if isinstance(entry, str):
text = entry.strip()
if text:
cleaned.append(text)
return cleaned or None
def _multi_artist_write_enabled() -> bool:
"""Read the same config flag the enrichment pipeline reads, so the
repair-path retag respects the user's choice."""
try:
from config.settings import config_manager
return bool(config_manager.get('metadata_enhancement.tags.write_multi_artist', False))
except Exception:
return False
def _write_id3(audio, title, artist, album_artist, album, year, genre,
track_num, total_tracks, disc_num, bpm) -> List[str]:
track_num, total_tracks, disc_num, bpm,
artists_list: Optional[List[str]] = None) -> List[str]:
written = []
if title:
audio.tags.delall('TIT2')
@ -354,6 +402,16 @@ def _write_id3(audio, title, artist, album_artist, album, year, genre,
audio.tags.delall('TPE1')
audio.tags.add(TPE1(encoding=3, text=[artist]))
written.append('artist')
# TPE1 stays as the joined display string. When the caller
# supplied a multi-value list AND the user has the
# write_multi_artist setting on, ALSO write the per-artist
# list to a TXXX:Artists frame (Picard convention). Mirrors
# the post-download enrichment writer at
# core/metadata/enrichment.py.
if artists_list and len(artists_list) > 1 and _multi_artist_write_enabled():
audio.tags.delall('TXXX:Artists')
audio.tags.add(TXXX(encoding=3, desc='Artists', text=list(artists_list)))
written.append('artists_multi')
if album_artist:
audio.tags.delall('TPE2')
audio.tags.add(TPE2(encoding=3, text=[album_artist]))
@ -387,7 +445,8 @@ def _write_id3(audio, title, artist, album_artist, album, year, genre,
def _write_vorbis(audio, title, artist, album_artist, album, year, genre,
track_num, total_tracks, disc_num, bpm) -> List[str]:
track_num, total_tracks, disc_num, bpm,
artists_list: Optional[List[str]] = None) -> List[str]:
written = []
if title:
audio['title'] = [title]
@ -395,6 +454,13 @@ def _write_vorbis(audio, title, artist, album_artist, album, year, genre,
if artist:
audio['artist'] = [artist]
written.append('artist')
# Vorbis-style multi-value: write the per-artist list to the
# `artists` key (separate from `artist`, picard convention) when
# the caller supplied a list AND the user has multi-value write
# enabled. Mirrors enrichment.py.
if artists_list and len(artists_list) > 1 and _multi_artist_write_enabled():
audio['artists'] = list(artists_list)
written.append('artists_multi')
if album_artist:
audio['albumartist'] = [album_artist]
written.append('album_artist')
@ -421,14 +487,24 @@ def _write_vorbis(audio, title, artist, album_artist, album, year, genre,
def _write_mp4(audio, title, artist, album_artist, album, year, genre,
track_num, total_tracks, disc_num, bpm) -> List[str]:
track_num, total_tracks, disc_num, bpm,
artists_list: Optional[List[str]] = None) -> List[str]:
written = []
if title:
audio['\xa9nam'] = [title]
written.append('title')
if artist:
audio['\xa9ART'] = [artist]
written.append('artist')
# MP4 \xa9ART can carry a list directly. When caller supplied
# a multi-value list AND user has multi-value write enabled,
# write the list. Otherwise single-string. Mirrors enrichment.py
# MP4 path.
if artists_list and len(artists_list) > 1 and _multi_artist_write_enabled():
audio['\xa9ART'] = list(artists_list)
written.append('artist')
written.append('artists_multi')
else:
audio['\xa9ART'] = [artist]
written.append('artist')
if album_artist:
audio['aART'] = [album_artist]
written.append('album_artist')

View file

@ -278,6 +278,27 @@ class TidalDownloadClient(DownloadSourcePlugin):
return False
return True
@classmethod
def _track_matches_qualifiers(cls, track, qualifiers: List[str]) -> bool:
"""Issue #589 — qualifier check must inspect both track.name AND
track.album.name. For MTV Unplugged-style releases the live /
unplugged signal lives in the album title, not the track title.
A track passes if every required qualifier appears as a whole
word in either the track name OR its album name.
"""
if not qualifiers:
return True
track_name = (getattr(track, 'name', '') or '').lower()
album = getattr(track, 'album', None)
album_name = (getattr(album, 'name', '') or '').lower() if album else ''
haystack = f"{track_name} {album_name}".strip()
if not haystack:
return False
for kw in qualifiers:
if not re.search(r'\b' + re.escape(kw) + r'\b', haystack):
return False
return True
@staticmethod
def _generate_shortened_queries(original: str) -> List[str]:
variants: List[str] = []
@ -342,25 +363,35 @@ class TidalDownloadClient(DownloadSourcePlugin):
found = await loop.run_in_executor(None, _search)
if found:
# Issue #589 — qualifier filter applies to ALL
# search attempts, not just fallbacks. If the
# primary query carries "live" / "unplugged" /
# etc, the studio cut should never be accepted
# just because Tidal returned it first. The
# filter inspects both track.name AND
# track.album.name (the live signal often lives
# in the album title for concert releases).
is_fallback = attempt_idx > 0
if is_fallback and required_qualifiers:
if required_qualifiers:
filtered = [
t for t in found
if self._track_name_contains_qualifiers(getattr(t, 'name', ''), required_qualifiers)
if self._track_matches_qualifiers(t, required_qualifiers)
]
if filtered:
tidal_tracks = filtered
successful_query = attempt_query
logger.info(
f"Tidal fallback kept {len(filtered)}/{len(found)} tracks "
f"after qualifier filter {required_qualifiers} for '{attempt_query}'"
f"Tidal {'fallback' if is_fallback else 'primary'} kept "
f"{len(filtered)}/{len(found)} tracks after qualifier filter "
f"{required_qualifiers} for '{attempt_query}'"
)
break
else:
any_fallback_filtered_out = True
logger.debug(
f"Tidal fallback '{attempt_query}' returned {len(found)} tracks "
f"but none matched original qualifiers {required_qualifiers}"
f"Tidal {'fallback' if is_fallback else 'primary'} "
f"'{attempt_query}' returned {len(found)} tracks but none "
f"matched required qualifiers {required_qualifiers}"
f"trying next variant"
)
if attempt_idx < len(queries_to_try) - 1:

View file

@ -0,0 +1,70 @@
from core.imports.file_integrity import _MAX_USER_TOLERANCE_S, resolve_duration_tolerance
def test_none_returns_none_so_caller_uses_auto_scaled_default():
assert resolve_duration_tolerance(None) is None
def test_missing_or_empty_string_returns_none():
assert resolve_duration_tolerance("") is None
assert resolve_duration_tolerance(" ") is None
def test_zero_returns_none_to_avoid_strict_mode_ambiguity():
# 0 means "unset" — never strict-mode (which would fail any drift).
# Users who want strict have no use-case; users who want disabled
# set a high value (capped to _MAX_USER_TOLERANCE_S).
assert resolve_duration_tolerance(0) is None
assert resolve_duration_tolerance(0.0) is None
assert resolve_duration_tolerance("0") is None
def test_negative_returns_none():
assert resolve_duration_tolerance(-1) is None
assert resolve_duration_tolerance(-3.5) is None
assert resolve_duration_tolerance("-10") is None
def test_positive_integer_passes_through_as_float():
assert resolve_duration_tolerance(5) == 5.0
assert resolve_duration_tolerance(10) == 10.0
def test_positive_float_passes_through():
assert resolve_duration_tolerance(3.5) == 3.5
assert resolve_duration_tolerance(0.1) == 0.1
def test_numeric_string_parsed():
assert resolve_duration_tolerance("5") == 5.0
assert resolve_duration_tolerance("3.5") == 3.5
assert resolve_duration_tolerance("10.0") == 10.0
def test_unparseable_string_returns_none():
assert resolve_duration_tolerance("abc") is None
assert resolve_duration_tolerance("five") is None
assert resolve_duration_tolerance("3s") is None
def test_above_max_clamped_to_ceiling():
assert resolve_duration_tolerance(9999) == _MAX_USER_TOLERANCE_S
assert resolve_duration_tolerance(_MAX_USER_TOLERANCE_S + 1) == _MAX_USER_TOLERANCE_S
def test_at_ceiling_passes_through():
assert resolve_duration_tolerance(_MAX_USER_TOLERANCE_S) == _MAX_USER_TOLERANCE_S
def test_non_numeric_types_return_none():
assert resolve_duration_tolerance([5]) is None
assert resolve_duration_tolerance({"value": 5}) is None
assert resolve_duration_tolerance(object()) is None
def test_bool_treated_as_int_python_semantics():
# Python: bool is int subclass. True -> 1.0, False -> 0 -> None.
# Documented behavior, not a bug — config values won't realistically
# be booleans for a numeric setting.
assert resolve_duration_tolerance(True) == 1.0
assert resolve_duration_tolerance(False) is None

View file

@ -0,0 +1,568 @@
import os
from concurrent.futures import Future
import core.imports.routes as import_routes
from core.imports.routes import (
ImportRouteRuntime,
album_match,
album_process,
process_single_import_file,
search_albums,
search_tracks,
singles_process,
staging_files,
staging_groups,
staging_hints,
staging_suggestions,
)
class _FakeLogger:
def __init__(self):
self.debug_messages = []
self.error_messages = []
self.info_messages = []
self.warning_messages = []
def debug(self, msg, *args):
self.debug_messages.append(msg % args if args else msg)
def error(self, msg, *args):
self.error_messages.append(msg % args if args else msg)
def info(self, msg, *args):
self.info_messages.append(msg % args if args else msg)
def warning(self, msg, *args):
self.warning_messages.append(msg % args if args else msg)
class _FakeHydrabaseWorker:
def __init__(self):
self.enqueued = []
def enqueue(self, query, kind):
self.enqueued.append((query, kind))
class _FakeAutomationEngine:
def __init__(self, fail=False):
self.events = []
self.fail = fail
def emit(self, name, payload):
if self.fail:
raise RuntimeError("emit boom")
self.events.append((name, payload))
class _FakeExecutor:
def __init__(self, outcomes=None):
self.outcomes = list(outcomes or [])
self.calls = []
def submit(self, fn, *args):
self.calls.append((fn, args))
future = Future()
if self.outcomes:
outcome = self.outcomes.pop(0)
if isinstance(outcome, BaseException):
future.set_exception(outcome)
else:
future.set_result(outcome)
else:
future.set_result(fn(*args))
return future
def _touch(path):
path.parent.mkdir(parents=True, exist_ok=True)
path.write_bytes(b"")
def _metadata_for(files):
def _read_metadata(file_path, rel_path):
return files[rel_path]
return _read_metadata
def test_staging_files_returns_audio_files_with_metadata(tmp_path):
_touch(tmp_path / "Artist" / "02 - Song.flac")
_touch(tmp_path / "cover.jpg")
rel_song = os.path.join("Artist", "02 - Song.flac")
metadata = {
rel_song: {
"title": "Song",
"artist": "Track Artist",
"albumartist": "Album Artist",
"album": "Album",
"track_number": 2,
"disc_number": 1,
}
}
runtime = ImportRouteRuntime(
get_staging_path=lambda: str(tmp_path),
read_staging_file_metadata=_metadata_for(metadata),
logger=_FakeLogger(),
)
payload, status = staging_files(runtime)
assert status == 200
assert payload["success"] is True
assert payload["staging_path"] == str(tmp_path)
assert len(payload["files"]) == 1
assert payload["files"] == [
{
"filename": "02 - Song.flac",
"rel_path": rel_song,
"full_path": str(tmp_path / "Artist" / "02 - Song.flac"),
"title": "Song",
"artist": "Album Artist",
"album": "Album",
"track_number": 2,
"disc_number": 1,
"extension": ".flac",
}
]
def test_staging_groups_only_returns_multi_file_album_groups(tmp_path):
_touch(tmp_path / "a.mp3")
_touch(tmp_path / "b.mp3")
_touch(tmp_path / "single.mp3")
metadata = {
"a.mp3": {
"title": "A",
"artist": "Artist",
"albumartist": "",
"album": "Album",
"track_number": 2,
"disc_number": 1,
},
"b.mp3": {
"title": "B",
"artist": "Artist",
"albumartist": "",
"album": "Album",
"track_number": 1,
"disc_number": 1,
},
"single.mp3": {
"title": "Single",
"artist": "Other",
"albumartist": "",
"album": "Other Album",
"track_number": 1,
"disc_number": 1,
},
}
runtime = ImportRouteRuntime(
get_staging_path=lambda: str(tmp_path),
read_staging_file_metadata=_metadata_for(metadata),
logger=_FakeLogger(),
)
payload, status = staging_groups(runtime)
assert status == 200
assert payload["success"] is True
assert len(payload["groups"]) == 1
group = payload["groups"][0]
assert group["album"] == "Album"
assert group["artist"] == "Artist"
assert group["file_count"] == 2
assert [f["filename"] for f in group["files"]] == ["b.mp3", "a.mp3"]
def test_staging_hints_prefers_tag_queries_then_folder_queries(tmp_path):
_touch(tmp_path / "Folder_Album" / "01.mp3")
_touch(tmp_path / "Folder_Album" / "02.mp3")
_touch(tmp_path / "Loose" / "track.flac")
def _read_tags(file_path):
if file_path.endswith("01.mp3") or file_path.endswith("02.mp3"):
return {"album": ["Tagged Album"], "artist": ["Tagged Artist"]}
return {}
runtime = ImportRouteRuntime(
get_staging_path=lambda: str(tmp_path),
read_tags=_read_tags,
logger=_FakeLogger(),
)
payload, status = staging_hints(runtime)
assert status == 200
assert payload == {
"success": True,
"hints": ["Tagged Album Tagged Artist", "Folder Album", "Loose"],
}
def test_staging_suggestions_returns_cache_payload(monkeypatch):
monkeypatch.setattr(
import_routes,
"get_import_suggestions_cache",
lambda: {"suggestions": [{"album": "Album"}], "built": True},
)
payload, status = staging_suggestions()
assert status == 200
assert payload == {
"success": True,
"suggestions": [{"album": "Album"}],
"ready": True,
}
def test_staging_groups_returns_empty_for_missing_staging_path(tmp_path):
runtime = ImportRouteRuntime(
get_staging_path=lambda: str(tmp_path / "missing"),
logger=_FakeLogger(),
)
payload, status = staging_groups(runtime)
assert status == 200
assert payload == {"success": True, "groups": []}
def test_staging_hints_returns_empty_for_missing_staging_path(tmp_path):
runtime = ImportRouteRuntime(
get_staging_path=lambda: str(tmp_path / "missing"),
logger=_FakeLogger(),
)
payload, status = staging_hints(runtime)
assert status == 200
assert payload == {"success": True, "hints": []}
def test_staging_files_returns_error_when_path_resolution_fails():
logger = _FakeLogger()
runtime = ImportRouteRuntime(
get_staging_path=lambda: (_ for _ in ()).throw(RuntimeError("path boom")),
logger=logger,
)
payload, status = staging_files(runtime)
assert status == 500
assert payload["success"] is False
assert payload["error"] == "path boom"
assert logger.error_messages == ["Error scanning staging files: path boom"]
def test_staging_groups_returns_error_when_metadata_read_fails(tmp_path):
_touch(tmp_path / "a.mp3")
logger = _FakeLogger()
runtime = ImportRouteRuntime(
get_staging_path=lambda: str(tmp_path),
read_staging_file_metadata=lambda _file_path, _rel_path: (_ for _ in ()).throw(RuntimeError("tag boom")),
logger=logger,
)
payload, status = staging_groups(runtime)
assert status == 500
assert payload["success"] is False
assert payload["error"] == "tag boom"
assert logger.error_messages == ["Error building staging groups: tag boom"]
def test_staging_hints_returns_error_when_path_resolution_fails():
logger = _FakeLogger()
runtime = ImportRouteRuntime(
get_staging_path=lambda: (_ for _ in ()).throw(RuntimeError("hint boom")),
logger=logger,
)
payload, status = staging_hints(runtime)
assert status == 500
assert payload["success"] is False
assert payload["error"] == "hint boom"
assert logger.error_messages == ["Error getting staging hints: hint boom"]
def test_search_albums_enqueues_hydrabase_and_caps_limit():
worker = _FakeHydrabaseWorker()
calls = []
runtime = ImportRouteRuntime(
get_primary_source=lambda: "hydrabase",
hydrabase_worker=worker,
dev_mode_enabled=True,
search_import_albums=lambda query, limit: calls.append((query, limit)) or [{"id": "album-1"}],
logger=_FakeLogger(),
)
payload, status = search_albums(runtime, " Album ", 99)
assert status == 200
assert payload == {"success": True, "albums": [{"id": "album-1"}]}
assert worker.enqueued == [("Album", "albums")]
assert calls == [("Album", 50)]
def test_search_albums_requires_query():
payload, status = search_albums(ImportRouteRuntime(logger=_FakeLogger()), "", 12)
assert status == 400
assert payload == {"success": False, "error": "Missing query parameter"}
def test_search_tracks_enqueues_hydrabase_and_caps_limit():
worker = _FakeHydrabaseWorker()
calls = []
runtime = ImportRouteRuntime(
get_primary_source=lambda: "hydrabase",
hydrabase_worker=worker,
dev_mode_enabled=True,
search_import_tracks=lambda query, limit: calls.append((query, limit)) or [{"id": "track-1"}],
logger=_FakeLogger(),
)
payload, status = search_tracks(runtime, " Track ", 99)
assert status == 200
assert payload == {"success": True, "tracks": [{"id": "track-1"}]}
assert worker.enqueued == [("Track", "tracks")]
assert calls == [("Track", 30)]
def test_album_match_warns_without_source_and_passes_file_filter():
logger = _FakeLogger()
calls = []
runtime = ImportRouteRuntime(
build_album_import_match_payload=lambda *args, **kwargs: calls.append((args, kwargs)) or {"success": True, "matches": []},
logger=logger,
)
payload, status = album_match(
runtime,
{
"album_id": "album-1",
"album_name": "Album",
"album_artist": "Artist",
"file_paths": ["a.flac", "b.flac"],
},
)
assert status == 200
assert payload == {"success": True, "matches": []}
assert calls == [
(
("album-1",),
{
"album_name": "Album",
"album_artist": "Artist",
"file_paths": {"a.flac", "b.flac"},
"source": None,
},
)
]
assert len(logger.warning_messages) == 1
assert "Missing 'source'" in logger.warning_messages[0]
def test_album_match_requires_album_id():
payload, status = album_match(ImportRouteRuntime(logger=_FakeLogger()), {})
assert status == 400
assert payload == {"success": False, "error": "Missing album_id"}
def test_album_process_posts_valid_files_and_records_side_effects(tmp_path):
good_file = tmp_path / "good.flac"
_touch(good_file)
processed_contexts = []
activity = []
refresh_calls = []
automation = _FakeAutomationEngine()
runtime = ImportRouteRuntime(
resolve_album_artist_context=lambda album, source="": {"name": "Artist"},
build_album_import_context=lambda album, track, **kwargs: {"album": album, "track": track, **kwargs},
post_process_matched_download=lambda key, context, path: processed_contexts.append((key, context, path)),
add_activity_item=lambda *args: activity.append(args),
refresh_import_suggestions_cache=lambda: refresh_calls.append("refresh"),
automation_engine=automation,
logger=_FakeLogger(),
)
payload, status = album_process(
runtime,
{
"album": {"id": "album-1", "name": "Album", "artist": "Artist", "source": "deezer"},
"matches": [
{
"staging_file": {"full_path": str(good_file), "filename": "good.flac"},
"track": {"name": "Good Track", "track_number": 1, "disc_number": 2},
},
{
"staging_file": {"full_path": str(tmp_path / "missing.flac"), "filename": "missing.flac"},
"track": {"name": "Missing Track", "track_number": 2},
},
],
},
)
assert status == 200
assert payload == {
"success": True,
"processed": 1,
"total": 2,
"errors": ["File not found: missing.flac"],
}
assert len(processed_contexts) == 1
key, context, path = processed_contexts[0]
assert key.startswith("import_album_album-1_1_")
assert context["artist_context"] == {"name": "Artist"}
assert context["total_discs"] == 2
assert context["source"] == "deezer"
assert path == str(good_file)
assert activity == [("", "Album Imported", "Album by Artist (1/2 tracks)", "Now")]
assert refresh_calls == ["refresh"]
assert automation.events == [
(
"import_completed",
{"track_count": "1", "album_name": "Album", "artist": "Artist"},
),
(
"batch_complete",
{
"playlist_name": "Import: Album",
"total_tracks": "2",
"completed_tracks": "1",
"failed_tracks": "1",
},
),
]
def test_album_process_requires_album_and_matches():
payload, status = album_process(ImportRouteRuntime(logger=_FakeLogger()), {"album": {}, "matches": []})
assert status == 400
assert payload == {"success": False, "error": "Missing album or matches data"}
def test_process_single_import_file_resolves_and_posts_context(tmp_path):
audio_file = tmp_path / "Artist - Song.flac"
_touch(audio_file)
post_calls = []
runtime = ImportRouteRuntime(
parse_filename_metadata=lambda filename: {"title": "Song", "artist": "Artist"},
get_single_track_import_context=lambda title, artist, **kwargs: {
"source": "deezer",
"context": {"track": {"name": title}, "artist": {"name": artist}},
},
normalize_import_context=lambda context: context,
get_import_context_artist=lambda context: context["artist"],
get_import_track_info=lambda context: context["track"],
post_process_matched_download=lambda key, context, path: post_calls.append((key, context, path)),
logger=_FakeLogger(),
)
outcome = process_single_import_file(runtime, {"full_path": str(audio_file), "filename": audio_file.name})
assert outcome == ("ok", "Song")
assert len(post_calls) == 1
assert post_calls[0][0].startswith("import_single_")
assert post_calls[0][1] == {"track": {"name": "Song"}, "artist": {"name": "Artist"}}
assert post_calls[0][2] == str(audio_file)
def test_process_single_import_file_rejects_malformed_manual_match(tmp_path):
audio_file = tmp_path / "Song.flac"
_touch(audio_file)
runtime = ImportRouteRuntime(post_process_matched_download=lambda *_args: None, logger=_FakeLogger())
outcome = process_single_import_file(
runtime,
{"full_path": str(audio_file), "filename": audio_file.name, "manual_match": {"id": "track-1"}},
)
assert outcome == ("error", "Malformed manual match for file: Song.flac")
def test_singles_process_aggregates_worker_results_and_side_effects():
activity = []
refresh_calls = []
automation = _FakeAutomationEngine()
executor = _FakeExecutor(outcomes=[("ok", "Song"), ("error", "Bad Song")])
runtime = ImportRouteRuntime(
import_singles_executor=executor,
add_activity_item=lambda *args: activity.append(args),
refresh_import_suggestions_cache=lambda: refresh_calls.append("refresh"),
automation_engine=automation,
logger=_FakeLogger(),
)
payload, status = singles_process(
runtime,
[{"filename": "a.flac"}, {"filename": "b.flac"}],
)
assert status == 200
assert payload == {
"success": True,
"processed": 1,
"total": 2,
"errors": ["Bad Song"],
}
assert len(executor.calls) == 2
assert activity == [("", "Singles Imported", "1/2 tracks processed", "Now")]
assert refresh_calls == ["refresh"]
assert automation.events == [
(
"import_completed",
{"track_count": "1", "album_name": "", "artist": "Various"},
),
(
"batch_complete",
{
"playlist_name": "Import: Singles",
"total_tracks": "2",
"completed_tracks": "1",
"failed_tracks": "1",
},
),
]
def test_singles_process_uses_injected_single_file_worker():
calls = []
executor = _FakeExecutor()
def fake_process_file(runtime, file_info):
calls.append((runtime, file_info))
return ("ok", file_info["filename"])
runtime = ImportRouteRuntime(
import_singles_executor=executor,
process_single_import_file=fake_process_file,
logger=_FakeLogger(),
)
payload, status = singles_process(runtime, [{"filename": "patched.flac"}])
assert status == 200
assert payload == {
"success": True,
"processed": 1,
"total": 1,
"errors": [],
}
assert calls == [(runtime, {"filename": "patched.flac"})]
assert executor.calls[0][0] is fake_process_file
def test_singles_process_requires_files():
payload, status = singles_process(ImportRouteRuntime(logger=_FakeLogger()), [])
assert status == 400
assert payload == {"success": False, "error": "No files provided"}

View file

@ -0,0 +1,273 @@
import json
import os
from core.imports.quarantine import (
approve_quarantine_entry,
delete_quarantine_entry,
list_quarantine_entries,
recover_to_staging,
serialize_quarantine_context,
)
# ──────────────────────────────────────────────────────────────────────
# serialize_quarantine_context — JSON-safe coercion
# ──────────────────────────────────────────────────────────────────────
def test_serialize_passes_scalar_dict_unchanged():
ctx = {"title": "DNA.", "track_number": 2, "active": True, "missing": None, "duration_ms": 185000}
out = serialize_quarantine_context(ctx)
assert out == ctx
def test_serialize_walks_nested_dicts():
ctx = {"track_info": {"name": "DNA.", "artists": [{"name": "Kendrick"}, {"name": "Rihanna"}]}}
out = serialize_quarantine_context(ctx)
assert out == ctx
def test_serialize_coerces_set_to_list():
ctx = {"sources": {"spotify", "deezer"}}
out = serialize_quarantine_context(ctx)
assert sorted(out["sources"]) == ["deezer", "spotify"]
def test_serialize_coerces_tuple_to_list():
ctx = {"pair": (1, 2, 3)}
out = serialize_quarantine_context(ctx)
assert out == {"pair": [1, 2, 3]}
def test_serialize_stringifies_unknown_objects():
class Custom:
def __str__(self):
return "<custom obj>"
out = serialize_quarantine_context({"obj": Custom()})
assert out["obj"] == "<custom obj>"
def test_serialize_non_dict_returns_empty_dict():
assert serialize_quarantine_context(None) == {}
assert serialize_quarantine_context("string") == {}
assert serialize_quarantine_context([1, 2, 3]) == {}
def test_serialize_round_trips_through_json():
ctx = {
"track_info": {"name": "X", "artists": [{"name": "A"}, {"name": "B"}]},
"spotify_artist": {"name": "A", "id": "abc"},
"duration_ms": 180000,
"sources": {"spotify"},
}
serialized = serialize_quarantine_context(ctx)
json.dumps(serialized) # must not raise
# ──────────────────────────────────────────────────────────────────────
# list_quarantine_entries
# ──────────────────────────────────────────────────────────────────────
def _write_entry(quarantine_dir, entry_id, original_name, *, with_context=False, trigger="integrity", reason="boom", file_bytes=b"X" * 100):
qfile = quarantine_dir / f"{entry_id}_{original_name}.quarantined"
qfile.write_bytes(file_bytes)
sidecar = {
"original_filename": original_name,
"quarantine_reason": reason,
"expected_track": "Track",
"expected_artist": "Artist",
"timestamp": "2026-05-14T12:00:00",
"trigger": trigger,
}
if with_context:
sidecar["context"] = {"track_info": {"name": "Track"}, "context_key": entry_id}
sidecar_path = quarantine_dir / f"{entry_id}_{os.path.splitext(original_name)[0]}.json"
sidecar_path.write_text(json.dumps(sidecar))
return qfile, sidecar_path
def test_list_returns_empty_for_missing_dir(tmp_path):
assert list_quarantine_entries(str(tmp_path / "nope")) == []
def test_list_returns_empty_for_empty_dir(tmp_path):
assert list_quarantine_entries(str(tmp_path)) == []
def test_list_returns_entry_with_sidecar_fields(tmp_path):
_write_entry(tmp_path, "20260514_120000", "song.flac", reason="Duration mismatch")
entries = list_quarantine_entries(str(tmp_path))
assert len(entries) == 1
e = entries[0]
assert e["original_filename"] == "song.flac"
assert e["reason"] == "Duration mismatch"
assert e["expected_track"] == "Track"
assert e["expected_artist"] == "Artist"
assert e["has_full_context"] is False
assert e["trigger"] == "integrity"
assert e["size_bytes"] == 100
def test_list_flags_full_context_entries(tmp_path):
_write_entry(tmp_path, "20260514_120000", "song.flac", with_context=True)
entries = list_quarantine_entries(str(tmp_path))
assert entries[0]["has_full_context"] is True
def test_list_handles_orphan_quarantined_file_without_sidecar(tmp_path):
qfile = tmp_path / "20260514_120000_orphan.flac.quarantined"
qfile.write_bytes(b"X")
entries = list_quarantine_entries(str(tmp_path))
assert len(entries) == 1
assert entries[0]["reason"] == "Unknown reason"
assert entries[0]["has_full_context"] is False
def test_list_skips_orphan_sidecars_without_file(tmp_path):
sidecar = tmp_path / "20260514_120000_only.json"
sidecar.write_text(json.dumps({"original_filename": "only.flac", "quarantine_reason": "x"}))
assert list_quarantine_entries(str(tmp_path)) == []
def test_list_sorts_newest_first(tmp_path):
_write_entry(tmp_path, "20260101_120000", "old.flac")
_write_entry(tmp_path, "20260514_120000", "new.flac")
entries = list_quarantine_entries(str(tmp_path))
assert entries[0]["original_filename"] == "new.flac"
assert entries[1]["original_filename"] == "old.flac"
def test_list_swallows_corrupt_sidecar_gracefully(tmp_path):
qfile = tmp_path / "20260514_120000_song.flac.quarantined"
qfile.write_bytes(b"X")
sidecar = tmp_path / "20260514_120000_song.json"
sidecar.write_text("{ this is not valid json")
entries = list_quarantine_entries(str(tmp_path))
assert len(entries) == 1
assert entries[0]["reason"] == "Unknown reason"
# ──────────────────────────────────────────────────────────────────────
# delete_quarantine_entry
# ──────────────────────────────────────────────────────────────────────
def test_delete_removes_both_file_and_sidecar(tmp_path):
_write_entry(tmp_path, "20260514_120000", "song.flac")
assert delete_quarantine_entry(str(tmp_path), "20260514_120000_song") is True
assert not (tmp_path / "20260514_120000_song.flac.quarantined").exists()
assert not (tmp_path / "20260514_120000_song.json").exists()
def test_delete_returns_false_when_entry_missing(tmp_path):
assert delete_quarantine_entry(str(tmp_path), "nonexistent") is False
def test_delete_handles_orphan_file_without_sidecar(tmp_path):
qfile = tmp_path / "20260514_120000_orphan.flac.quarantined"
qfile.write_bytes(b"X")
assert delete_quarantine_entry(str(tmp_path), "20260514_120000_orphan") is True
assert not qfile.exists()
# ──────────────────────────────────────────────────────────────────────
# approve_quarantine_entry — full-context path
# ──────────────────────────────────────────────────────────────────────
def test_approve_restores_file_and_returns_context_and_trigger(tmp_path):
quarantine = tmp_path / "ss_quarantine"
quarantine.mkdir()
restore = tmp_path / "restore"
_write_entry(quarantine, "20260514_120000", "song.flac", with_context=True, trigger="integrity")
result = approve_quarantine_entry(str(quarantine), "20260514_120000_song", str(restore))
assert result is not None
restored_path, context, trigger = result
assert os.path.basename(restored_path) == "song.flac"
assert os.path.isfile(restored_path)
assert context["track_info"]["name"] == "Track"
assert trigger == "integrity"
# Sidecar removed after approve
assert not (quarantine / "20260514_120000_song.json").exists()
def test_approve_returns_none_for_thin_sidecar_without_context(tmp_path):
_write_entry(tmp_path, "20260514_120000", "song.flac", with_context=False)
result = approve_quarantine_entry(str(tmp_path), "20260514_120000_song", str(tmp_path / "restore"))
assert result is None
def test_approve_returns_none_for_missing_entry(tmp_path):
assert approve_quarantine_entry(str(tmp_path), "nope", str(tmp_path)) is None
def test_approve_avoids_filename_collision(tmp_path):
quarantine = tmp_path / "q"
quarantine.mkdir()
restore = tmp_path / "r"
restore.mkdir()
(restore / "song.flac").write_bytes(b"existing")
_write_entry(quarantine, "20260514_120000", "song.flac", with_context=True)
result = approve_quarantine_entry(str(quarantine), "20260514_120000_song", str(restore))
assert result is not None
restored_path = result[0]
assert os.path.basename(restored_path) == "song_(2).flac"
assert (restore / "song.flac").read_bytes() == b"existing"
# ──────────────────────────────────────────────────────────────────────
# recover_to_staging — fallback for thin sidecars
# ──────────────────────────────────────────────────────────────────────
def test_recover_strips_prefix_and_suffix(tmp_path):
quarantine = tmp_path / "q"
quarantine.mkdir()
staging = tmp_path / "s"
qfile, _ = _write_entry(quarantine, "20260514_120000", "song.flac")
target = recover_to_staging(str(quarantine), str(staging), "20260514_120000_song")
assert target is not None
assert os.path.basename(target) == "song.flac"
assert os.path.isfile(target)
assert not qfile.exists()
def test_recover_uses_sidecar_original_filename_when_available(tmp_path):
quarantine = tmp_path / "q"
quarantine.mkdir()
staging = tmp_path / "s"
qfile = quarantine / "20260514_120000_munged_name.flac.quarantined"
qfile.write_bytes(b"X")
sidecar = quarantine / "20260514_120000_munged_name.json"
sidecar.write_text(json.dumps({"original_filename": "Pretty Track Name.flac"}))
target = recover_to_staging(str(quarantine), str(staging), "20260514_120000_munged_name")
assert target is not None
assert os.path.basename(target) == "Pretty Track Name.flac"
def test_recover_returns_none_for_missing_entry(tmp_path):
assert recover_to_staging(str(tmp_path / "q"), str(tmp_path / "s"), "nope") is None
def test_recover_avoids_filename_collision(tmp_path):
quarantine = tmp_path / "q"
quarantine.mkdir()
staging = tmp_path / "s"
staging.mkdir()
(staging / "song.flac").write_bytes(b"existing")
_write_entry(quarantine, "20260514_120000", "song.flac")
target = recover_to_staging(str(quarantine), str(staging), "20260514_120000_song")
assert target is not None
assert os.path.basename(target) == "song_(2).flac"
def test_recover_removes_sidecar_after_move(tmp_path):
quarantine = tmp_path / "q"
quarantine.mkdir()
staging = tmp_path / "s"
_, sidecar = _write_entry(quarantine, "20260514_120000", "song.flac")
recover_to_staging(str(quarantine), str(staging), "20260514_120000_song")
assert not sidecar.exists()

View file

@ -0,0 +1,201 @@
"""Tests for the shared AcoustID candidate-matching helper.
Issue #587 / Foxxify report — scanner used to treat ``recordings[0]``
as authoritative, so when AcoustID returned multiple candidates and
the top one was the wrong-credited recording (different MB entry
under the same fingerprint), the scanner created a false-positive
"Wrong download" finding even though a lower-ranked candidate matched
the expected metadata exactly.
"""
from __future__ import annotations
from difflib import SequenceMatcher
import pytest
from core.matching.acoustid_candidates import (
duration_mismatches_strongly,
find_matching_recording,
)
def _ratio_sim(a: str, b: str) -> float:
"""Reasonable test similarity that handles non-trivial differences."""
if not a or not b:
return 0.0
return SequenceMatcher(None, a.lower().strip(), b.lower().strip()).ratio()
# ──────────────────────────────────────────────────────────────────────
# find_matching_recording
# ──────────────────────────────────────────────────────────────────────
def test_top_recording_matches_returns_immediately():
recordings = [
{'title': 'Nana', 'artist': 'Geoxor'},
{'title': 'Nana', 'artist': 'Edward Vesala Trio'},
]
result, t_sim, a_sim = find_matching_recording(
recordings, 'Nana', 'Geoxor', similarity=_ratio_sim,
)
assert result == {'title': 'Nana', 'artist': 'Geoxor'}
assert t_sim == 1.0
assert a_sim == 1.0
def test_falls_through_to_lower_ranked_match_for_foxxify_nana_case():
"""Reporter case 2: top AcoustID candidate is 'Nana' by 'Edward
Vesala Trio' (97% fingerprint), but the LOWER-ranked candidate
is the expected 'Nana' by 'Geoxor'. Pre-fix scanner saw only [0]
and flagged. Post-fix returns the matching candidate."""
recordings = [
{'title': 'Nana', 'artist': 'Edward Vesala Trio'}, # AcoustID's top match
{'title': 'Nana', 'artist': 'Geoxor'}, # the actual right one
]
result, _, _ = find_matching_recording(
recordings, 'Nana', 'Geoxor', similarity=_ratio_sim,
)
assert result == {'title': 'Nana', 'artist': 'Geoxor'}
def test_no_match_returns_none_with_best_seen_sims():
"""When no candidate passes thresholds, return the best-seen sims
so callers can log the closest near-miss in the finding."""
recordings = [
{'title': 'Different Song', 'artist': 'Different Artist'},
{'title': 'Sort Of Close', 'artist': 'Different Artist'},
]
result, t_sim, a_sim = find_matching_recording(
recordings, 'Different', 'AnotherArtist',
similarity=_ratio_sim,
title_threshold=0.95,
artist_threshold=0.95,
)
assert result is None
# Best seen — even though no candidate passed the threshold
assert t_sim > 0.0
assert a_sim > 0.0
def test_skips_recordings_missing_title_or_artist():
recordings = [
{'title': None, 'artist': 'Geoxor'},
{'title': 'Nana', 'artist': ''},
{'title': 'Nana', 'artist': 'Geoxor'},
]
result, _, _ = find_matching_recording(
recordings, 'Nana', 'Geoxor', similarity=_ratio_sim,
)
assert result == {'title': 'Nana', 'artist': 'Geoxor'}
def test_skips_non_dict_entries():
recordings = [None, 'string', {'title': 'Nana', 'artist': 'Geoxor'}]
result, _, _ = find_matching_recording(
recordings, 'Nana', 'Geoxor', similarity=_ratio_sim,
)
assert result == {'title': 'Nana', 'artist': 'Geoxor'}
def test_empty_inputs_return_none():
assert find_matching_recording([], 'X', 'Y')[0] is None
assert find_matching_recording([{'title': 'X', 'artist': 'Y'}], '', 'Y')[0] is None
assert find_matching_recording([{'title': 'X', 'artist': 'Y'}], 'X', '')[0] is None
def test_separate_artist_similarity_function_is_honored():
"""Verifier passes alias-aware comparison via artist_similarity.
Make sure it's used instead of the generic similarity."""
recordings = [{'title': 'Track', 'artist': '澤野弘之'}]
def alias_aware(expected, actual):
# Pretend our alias chain bridges Hiroyuki Sawano ↔ 澤野弘之
if expected == 'Hiroyuki Sawano' and actual == '澤野弘之':
return 1.0
return 0.0
result, _, a_sim = find_matching_recording(
recordings, 'Track', 'Hiroyuki Sawano',
similarity=_ratio_sim,
artist_similarity=alias_aware,
)
assert result is not None
assert a_sim == 1.0
def test_skip_predicate_drops_unwanted_candidates():
"""Verifier uses skip_predicate to drop wrong-version recordings
(instrumental vs vocal, etc.)."""
recordings = [
{'title': 'Track (Instrumental)', 'artist': 'X'},
{'title': 'Track', 'artist': 'X'},
]
result, _, _ = find_matching_recording(
recordings, 'Track', 'X',
similarity=_ratio_sim,
skip_predicate=lambda r: 'instrumental' in (r.get('title') or '').lower(),
)
assert result == {'title': 'Track', 'artist': 'X'}
def test_title_threshold_can_be_lowered_for_loose_matching():
recordings = [{'title': 'Sort Of Close', 'artist': 'Right Artist'}]
# With strict default threshold this fails
result_strict, _, _ = find_matching_recording(
recordings, 'Different', 'Right Artist', similarity=_ratio_sim,
)
assert result_strict is None
# With a permissive threshold the artist match alone wouldn't help —
# title sim must also pass.
result_loose, _, _ = find_matching_recording(
recordings, 'Different', 'Right Artist',
similarity=_ratio_sim, title_threshold=0.0,
)
assert result_loose is not None
# ──────────────────────────────────────────────────────────────────────
# duration_mismatches_strongly — guard against fingerprint collisions
# ──────────────────────────────────────────────────────────────────────
def test_durations_within_tolerance_pass():
# 3-minute track, 1-second drift — well within tolerance
assert duration_mismatches_strongly(180, 181) is False
# 3-minute vs 4-minute — within the 60s absolute tolerance
assert duration_mismatches_strongly(180, 240) is False
def test_drift_above_absolute_floor_flags():
# 3-minute expected, 5-minute candidate (120s drift > 63s threshold)
assert duration_mismatches_strongly(180, 300) is True
def test_relative_tolerance_scales_with_long_tracks():
# 30-minute expected vs 12-minute candidate (1080s vs 720s) —
# 18-minute drift > 35% of 30min = 10.5min → mismatch
assert duration_mismatches_strongly(1800, 720) is True
# 30-minute expected vs 28-minute candidate — 2min drift = under
# max(60s, 35%*30min) = max(60, 630) = 630s → still safe
assert duration_mismatches_strongly(1800, 1680) is False
def test_reporter_17min_mashup_vs_5min_track_flagged():
"""Foxxify's 17min mashup edit vs 5min late-70s Japanese hiphop —
fingerprint collision. Duration guard should mark this suspicious."""
assert duration_mismatches_strongly(17 * 60, 5 * 60) is True
def test_unknown_duration_returns_false_no_behavior_change():
"""When either side is missing duration, don't change behavior."""
assert duration_mismatches_strongly(None, 300) is False
assert duration_mismatches_strongly(180, None) is False
assert duration_mismatches_strongly(0, 300) is False
assert duration_mismatches_strongly(180, 0) is False
assert duration_mismatches_strongly(-5, 300) is False
def test_string_or_int_durations_handled():
# Defensive — coerce numeric types
assert duration_mismatches_strongly(180.5, 181.0) is False
assert duration_mismatches_strongly(int(180), int(300)) is True

View file

@ -0,0 +1,168 @@
"""Tests for the album-context-aware track-title stripping helper.
Issue #589 — MTV Unplugged track titles like ``"Shy Away (MTV Unplugged
Live)"`` got false-rejected by the album-scoped library check because
the local DB stored title is just ``"Shy Away"``. The pure helper here
strips the redundant suffix when (and only when) the album title
implies the same context.
"""
from core.matching.album_context_title import (
album_context_markers,
strip_redundant_album_suffix,
)
# ──────────────────────────────────────────────────────────────────────
# album_context_markers
# ──────────────────────────────────────────────────────────────────────
def test_mtv_unplugged_album_carries_unplugged_marker():
# 'mtv' isn't a version marker on its own — the unplugged token is
# the load-bearing one. Implied-live logic adds 'live' coverage too.
markers = album_context_markers('MTV Unplugged')
assert 'unplugged' in markers
def test_live_at_album_carries_live_marker():
markers = album_context_markers('Live At Wembley')
assert 'live' in markers
def test_studio_album_has_no_markers():
assert album_context_markers('Scorpion') == ()
assert album_context_markers('DAMN.') == ()
assert album_context_markers('') == ()
assert album_context_markers(None) == ()
def test_acoustic_session_album_marker():
assert 'acoustic' in album_context_markers('Acoustic Sessions Vol. 2')
assert 'session' in album_context_markers('Acoustic Sessions Vol. 2')
# ──────────────────────────────────────────────────────────────────────
# strip_redundant_album_suffix — the headline cases from #589
# ──────────────────────────────────────────────────────────────────────
def test_strips_mtv_unplugged_live_suffix_when_album_is_mtv_unplugged():
assert strip_redundant_album_suffix('Shy Away (MTV Unplugged Live)', 'MTV Unplugged') == 'Shy Away'
def test_strips_complex_mtv_unplugged_suffix_with_year():
# Reporter case 2: "Only If For A Night (MTV Unplugged, 2012 / Live)"
assert strip_redundant_album_suffix(
'Only If For A Night (MTV Unplugged, 2012 / Live)',
'Ceremonials (Live At MTV Unplugged)',
) == 'Only If For A Night'
def test_strips_dash_style_live_suffix_when_album_is_live():
assert strip_redundant_album_suffix(
'Bohemian Rhapsody - Live At Wembley',
'Live At Wembley Stadium',
) == 'Bohemian Rhapsody'
def test_strips_brackets_live_suffix():
assert strip_redundant_album_suffix(
'Hello [Live]',
'Live At The Royal Albert Hall',
) == 'Hello'
# ──────────────────────────────────────────────────────────────────────
# Negative cases — must NOT strip when it would mask a genuine variant
# ──────────────────────────────────────────────────────────────────────
def test_does_not_strip_instrumental_when_album_is_studio():
# Critical anti-regression — keeping AcoustID's vocal/instrumental
# gate working downstream. Don't drop the marker just because the
# title is on a studio album.
assert strip_redundant_album_suffix(
'In My Feelings (Instrumental)',
'Scorpion',
) == 'In My Feelings (Instrumental)'
def test_does_not_strip_remix_when_album_is_studio():
assert strip_redundant_album_suffix(
'Hello (Acoustic Remix)',
'Scorpion',
) == 'Hello (Acoustic Remix)'
def test_does_not_strip_live_when_album_does_not_imply_live():
# User's "Live At Wembley" might be a single-track release on an
# otherwise-studio album. Don't strip.
assert strip_redundant_album_suffix(
'Hello (Live At Wembley)',
'Greatest Hits',
) == 'Hello (Live At Wembley)'
def test_does_not_strip_when_suffix_carries_extra_context():
# Suffix has both the album marker AND a featured-artist credit;
# the credit isn't album context, so keep the suffix.
assert strip_redundant_album_suffix(
'Track Name (Live - feat. Other Artist)',
'Live At Wembley',
) == 'Track Name (Live - feat. Other Artist)'
def test_no_suffix_returns_unchanged():
assert strip_redundant_album_suffix('Shy Away', 'MTV Unplugged') == 'Shy Away'
def test_empty_or_none_inputs_handled():
assert strip_redundant_album_suffix('', 'MTV Unplugged') == ''
assert strip_redundant_album_suffix(None, 'MTV Unplugged') == ''
assert strip_redundant_album_suffix('Shy Away', '') == 'Shy Away'
assert strip_redundant_album_suffix('Shy Away', None) == 'Shy Away'
# ──────────────────────────────────────────────────────────────────────
# Stacked-suffix cases
# ──────────────────────────────────────────────────────────────────────
def test_strips_stacked_redundant_suffixes():
# Some sources double up: parens + brackets, both album-context
assert strip_redundant_album_suffix(
'Track Name (Live) [Unplugged]',
'MTV Unplugged Live',
) == 'Track Name'
def test_stops_stripping_when_remaining_suffix_is_genuine():
# Outer is redundant (live → album-context), inner is not (remix)
assert strip_redundant_album_suffix(
'Track Name (Remix) (Live)',
'Live At Wembley',
) == 'Track Name (Remix)'
# ──────────────────────────────────────────────────────────────────────
# Year + connector tolerance
# ──────────────────────────────────────────────────────────────────────
def test_year_in_suffix_does_not_block_stripping():
assert strip_redundant_album_suffix(
'Track Name (Live, 2012)',
'Live At Wembley',
) == 'Track Name'
def test_version_word_in_suffix_does_not_block_stripping():
# "Live Version" is still album-context (just the word "version"
# in there). Strip.
assert strip_redundant_album_suffix(
'Track Name (Live Version)',
'Live At Wembley',
) == 'Track Name'
def test_session_marker_preserved_for_acoustic_session_album():
assert strip_redundant_album_suffix(
'Hello (Acoustic Session)',
'Acoustic Sessions Vol. 2',
) == 'Hello'

View file

@ -0,0 +1,243 @@
"""Cross-script artist alias lookup — issue #586.
The verifier's alias path was missing the Cyrillic spelling for
"Dmitry Yablonsky" because:
1. ``fetch_artist_aliases`` only read ``data['aliases']`` and ignored
the canonical ``name`` / ``sort-name`` fields. When MB's canonical
name IS the cross-script form, the Latin spelling never made it
into the alias output (and vice-versa).
2. ``lookup_artist_aliases`` ran search in strict mode only, which
queries ``artist:"..."`` and skips alias / sortname indexes. Cross-
script searches found nothing under strict.
3. The trust gate weighted local similarity 70%, so cross-script
matches scored ~0.30 even when MB's own confidence was 100, getting
rejected as low-confidence.
These tests pin all three fixes plus the original Hiroyuki Sawano
case from #442 (regression guard).
"""
from __future__ import annotations
from unittest.mock import MagicMock, patch
import pytest
from core.musicbrainz_service import MusicBrainzService
@pytest.fixture
def service():
"""Build a MusicBrainzService with mocked client + DB. The instance
is built bypassing __init__ so we don't need real config / DB."""
svc = MusicBrainzService.__new__(MusicBrainzService)
svc.mb_client = MagicMock()
svc._calculate_similarity = lambda a, b: _simple_sim(a, b)
svc.get_artist_aliases = MagicMock(return_value=[])
svc._check_cache = MagicMock(return_value=None)
svc._save_to_cache = MagicMock()
return svc
def _simple_sim(a: str, b: str) -> float:
"""Tiny stub similarity — exact match=1.0 else 0.0. Cross-script
pairs naturally fall into 0.0 since no characters overlap."""
if not a or not b:
return 0.0
return 1.0 if a.lower() == b.lower() else 0.0
# ──────────────────────────────────────────────────────────────────────
# fetch_artist_aliases — canonical name + sort-name now included
# ──────────────────────────────────────────────────────────────────────
def test_fetch_aliases_includes_canonical_name(service):
service.mb_client.get_artist.return_value = {
'name': 'Дмитрий Яблонский',
'sort-name': 'Yablonsky, Dmitry',
'aliases': [
{'name': 'Dmitry Yablonsky', 'sort-name': 'Yablonsky, Dmitry'},
],
}
aliases = service.fetch_artist_aliases('mbid-yablonsky')
# Canonical name MUST be present so cross-script matching works
# whichever direction the canonical form points.
assert 'Дмитрий Яблонский' in aliases
assert 'Dmitry Yablonsky' in aliases
# Sort-name covered too
assert 'Yablonsky, Dmitry' in aliases
def test_fetch_aliases_dedupes_canonical_against_alias_entry(service):
# MB sometimes lists the canonical name as ALSO an alias entry.
# No duplicate output.
service.mb_client.get_artist.return_value = {
'name': 'Hiroyuki Sawano',
'sort-name': 'Sawano, Hiroyuki',
'aliases': [
{'name': 'Hiroyuki Sawano', 'sort-name': 'Sawano, Hiroyuki'},
{'name': '澤野弘之'},
],
}
aliases = service.fetch_artist_aliases('mbid-sawano')
assert aliases.count('Hiroyuki Sawano') == 1
assert '澤野弘之' in aliases
def test_fetch_aliases_handles_missing_canonical_gracefully(service):
service.mb_client.get_artist.return_value = {
'aliases': [{'name': 'Dmitry Yablonsky'}],
}
aliases = service.fetch_artist_aliases('mbid-x')
assert aliases == ['Dmitry Yablonsky']
def test_fetch_aliases_returns_empty_on_no_data(service):
service.mb_client.get_artist.return_value = None
assert service.fetch_artist_aliases('mbid-x') == []
def test_fetch_aliases_returns_empty_on_exception(service):
service.mb_client.get_artist.side_effect = RuntimeError('boom')
assert service.fetch_artist_aliases('mbid-x') == []
# ──────────────────────────────────────────────────────────────────────
# lookup_artist_aliases — strict + non-strict fallback
# ──────────────────────────────────────────────────────────────────────
def test_lookup_falls_back_to_non_strict_when_strict_returns_nothing(service):
# Strict search returns nothing (typical cross-script case).
# Non-strict hits the alias index and finds the artist.
def search(name, limit, strict):
if strict:
return []
return [{'id': 'mbid-yab', 'name': 'Дмитрий Яблонский', 'score': 100}]
service.mb_client.search_artist.side_effect = search
service.mb_client.get_artist.return_value = {
'name': 'Дмитрий Яблонский',
'aliases': [{'name': 'Dmitry Yablonsky'}],
}
aliases = service.lookup_artist_aliases('Dmitry Yablonsky')
assert 'Дмитрий Яблонский' in aliases
assert 'Dmitry Yablonsky' in aliases
# Confirm both modes were attempted
calls = service.mb_client.search_artist.call_args_list
assert any(call.kwargs.get('strict') is True for call in calls) or any(
len(call.args) >= 3 and call.args[2] is True for call in calls
)
def test_lookup_falls_back_to_non_strict_when_strict_score_too_low(service):
# Strict returns a low-confidence match (cross-script — local sim
# near 0). Non-strict hits a stronger match via alias index.
def search(name, limit, strict):
if strict:
return [{'id': 'mbid-other', 'name': 'Some Other Artist', 'score': 30}]
return [{'id': 'mbid-yab', 'name': 'Дмитрий Яблонский', 'score': 100}]
service.mb_client.search_artist.side_effect = search
service.mb_client.get_artist.return_value = {
'name': 'Дмитрий Яблонский',
'aliases': [{'name': 'Dmitry Yablonsky'}],
}
aliases = service.lookup_artist_aliases('Dmitry Yablonsky')
assert 'Дмитрий Яблонский' in aliases
# ──────────────────────────────────────────────────────────────────────
# Trust gate — MB-score-only escape for cross-script
# ──────────────────────────────────────────────────────────────────────
def test_trust_gate_passes_on_high_mb_score_even_with_zero_local_sim(service):
# The cross-script case: local sim ~0, MB score 100, single
# unambiguous result → should now pass.
service.mb_client.search_artist.return_value = [
{'id': 'mbid-yab', 'name': 'Дмитрий Яблонский', 'score': 100},
]
service.mb_client.get_artist.return_value = {
'name': 'Дмитрий Яблонский',
'aliases': [{'name': 'Dmitry Yablonsky'}],
}
aliases = service.lookup_artist_aliases('Dmitry Yablonsky')
assert 'Дмитрий Яблонский' in aliases
assert 'Dmitry Yablonsky' in aliases
def test_trust_gate_rejects_low_mb_score_low_local_sim(service):
# Low confidence on both axes — must NOT pull aliases (false-
# positive risk: pulling random artist's aliases).
service.mb_client.search_artist.return_value = [
{'id': 'mbid-other', 'name': 'Some Random Artist', 'score': 40},
]
aliases = service.lookup_artist_aliases('Dmitry Yablonsky')
assert aliases == []
service.mb_client.get_artist.assert_not_called()
def test_trust_gate_rejects_when_two_high_mb_scores_tie(service):
# Two artists named the same with score 100 → ambiguous → skip
# even with MB-only escape (the unambiguity check still gates).
service.mb_client.search_artist.return_value = [
{'id': 'mbid-a', 'name': 'John Smith', 'score': 100},
{'id': 'mbid-b', 'name': 'John Smith', 'score': 100},
]
aliases = service.lookup_artist_aliases('John Smith')
assert aliases == []
def test_trust_gate_passes_combined_score_when_local_sim_strong(service):
# Same-script case from #442 — local sim high. Should still pass
# (no regression on the existing path).
service.mb_client.search_artist.return_value = [
{'id': 'mbid-saw', 'name': 'Hiroyuki Sawano', 'score': 100},
]
service.mb_client.get_artist.return_value = {
'name': 'Hiroyuki Sawano',
'aliases': [{'name': '澤野弘之'}],
}
aliases = service.lookup_artist_aliases('Hiroyuki Sawano')
assert '澤野弘之' in aliases
# ──────────────────────────────────────────────────────────────────────
# End-to-end — reporter scenario via artist_names_match
# ──────────────────────────────────────────────────────────────────────
def test_yablonsky_reporter_scenario_end_to_end(service):
"""Issue #586 exact case: expected 'Dmitry Yablonsky', actual
'Русская филармония, Дмитрий Яблонский', MB returns artist with
canonical name in Cyrillic and Latin in aliases. Strict search
finds nothing; non-strict finds the artist with high MB score."""
def search(name, limit, strict):
if strict:
return []
return [{'id': 'mbid-yab', 'name': 'Дмитрий Яблонский', 'score': 100}]
service.mb_client.search_artist.side_effect = search
service.mb_client.get_artist.return_value = {
'name': 'Дмитрий Яблонский',
'sort-name': 'Yablonsky, Dmitry',
'aliases': [{'name': 'Dmitry Yablonsky'}],
}
aliases = service.lookup_artist_aliases('Dmitry Yablonsky')
# Must include the Cyrillic canonical so artist_names_match can
# bridge the credit-split actual.
assert 'Дмитрий Яблонский' in aliases
# Verify the full bridge with the real artist_names_match helper.
from core.matching.artist_aliases import artist_names_match
matched, score = artist_names_match(
'Dmitry Yablonsky',
'Русская филармония, Дмитрий Яблонский',
aliases=aliases,
)
assert matched is True
assert score >= 0.6

View file

@ -81,10 +81,13 @@ class TestFetchArtistAliases:
def test_extracts_alias_names_from_mb_response(self, service):
"""Reporter's case 1 shape: MB returns aliases for Hiroyuki
Sawano including the Japanese kanji form. Extract the `name`
from each alias entry."""
from each alias entry. Issue #586 — also include the canonical
``name`` and ``sort-name`` from the artist record itself, plus
per-alias ``sort-name`` when present, for cross-script bridging."""
service.mb_client.get_artist.return_value = {
'id': '60d2ea34-1912-425f-bf9c-fc544e4448cd',
'name': 'Hiroyuki Sawano',
'sort-name': 'Sawano, Hiroyuki',
'aliases': [
{'name': '澤野弘之', 'sort-name': '澤野弘之', 'locale': 'ja', 'primary': True},
{'name': 'SawanoHiroyuki', 'sort-name': 'SawanoHiroyuki', 'locale': None},
@ -94,10 +97,11 @@ class TestFetchArtistAliases:
aliases = service.fetch_artist_aliases('60d2ea34-1912-425f-bf9c-fc544e4448cd')
assert 'Hiroyuki Sawano' in aliases # canonical name
assert 'Sawano, Hiroyuki' in aliases # canonical sort-name (also matches alias sort-name)
assert '澤野弘之' in aliases
assert 'SawanoHiroyuki' in aliases
assert 'Sawano Hiroyuki' in aliases
assert len(aliases) == 3
def test_dedup_case_insensitive(self, service):
"""Same name with different casing should collapse — MB
@ -125,14 +129,15 @@ class TestFetchArtistAliases:
aliases = service.fetch_artist_aliases('mbid-x')
assert aliases == ['Real Name']
def test_missing_aliases_key_returns_empty(self, service):
"""MB artist record might not have any aliases. Returns []
not raises."""
def test_missing_aliases_key_returns_canonical_name_only(self, service):
"""MB artist record without an aliases array still returns the
canonical name (post-#586). Pre-fix this returned [] which
meant cross-script bridging was impossible."""
service.mb_client.get_artist.return_value = {
'id': 'mbid-x',
'name': 'Some Artist',
}
assert service.fetch_artist_aliases('mbid-x') == []
assert service.fetch_artist_aliases('mbid-x') == ['Some Artist']
def test_aliases_null_returns_empty(self, service):
"""MB sometimes returns `aliases: null` instead of empty array."""
@ -476,14 +481,19 @@ class TestLookupArtistAliasesMultiTier:
assert aliases == []
def test_no_search_results_returns_empty(self, service):
"""Artist not found on MB — empty return, cached so we
don't re-search the same name forever."""
"""Artist not found on MB under either strict or non-strict
search empty return, cached so we don't re-search the same
name forever. Issue #586: strict-then-non-strict means TWO
search calls per uncached lookup; the empty cache prevents
further calls on the next invocation."""
service.mb_client.search_artist.return_value = []
aliases = service.lookup_artist_aliases('NeverHeardOf')
assert aliases == []
# Second call should hit cache, not re-search
# First lookup: strict + non-strict fallback = 2 calls.
assert service.mb_client.search_artist.call_count == 2
# Second call should hit cache, not re-search at all.
service.lookup_artist_aliases('NeverHeardOf')
assert service.mb_client.search_artist.call_count == 1
assert service.mb_client.search_artist.call_count == 2
def test_low_confidence_match_skipped(self, service):
"""Search returned something but the name similarity is too

View file

@ -0,0 +1,79 @@
from core.metadata.artist_resolution import resolve_track_artists
def test_prefers_original_search_artists_when_populated():
original = {"artists": [{"name": "Kendrick Lamar"}, {"name": "Rihanna"}]}
track = {"artists": [{"name": "Should Not Be Used"}]}
artist = {"name": "Primary"}
assert resolve_track_artists(original, track, artist) == ["Kendrick Lamar", "Rihanna"]
def test_falls_back_to_track_info_artists_when_original_lacks_list():
# Soulseek context shape: original_search_result has 'artist' (string)
# and no 'artists' (list). Full Spotify track object lives on track_info.
original = {"artist": "Kendrick Lamar"}
track = {"artists": [{"name": "Kendrick Lamar"}, {"name": "Rihanna"}]}
artist = {"name": "Kendrick Lamar"}
assert resolve_track_artists(original, track, artist) == ["Kendrick Lamar", "Rihanna"]
def test_falls_back_to_artist_dict_name_when_no_lists_available():
original = {"artist": "Solo Artist"}
track = {"name": "Track Title"}
artist = {"name": "Solo Artist"}
assert resolve_track_artists(original, track, artist) == ["Solo Artist"]
def test_returns_empty_list_when_everything_missing():
assert resolve_track_artists(None, None, None) == []
assert resolve_track_artists({}, {}, {}) == []
def test_handles_bare_string_artist_items():
original = {"artists": ["Kendrick Lamar", "Rihanna"]}
assert resolve_track_artists(original, None, None) == ["Kendrick Lamar", "Rihanna"]
def test_mixed_dict_and_string_items_normalized():
original = {"artists": [{"name": "Kendrick Lamar"}, "Rihanna"]}
assert resolve_track_artists(original, None, None) == ["Kendrick Lamar", "Rihanna"]
def test_strips_whitespace_and_drops_empty_entries():
original = {"artists": [{"name": " Kendrick "}, {"name": ""}, " ", "Rihanna"]}
assert resolve_track_artists(original, None, None) == ["Kendrick", "Rihanna"]
def test_dict_item_without_name_key_skipped():
original = {"artists": [{"id": "abc"}, {"name": "Rihanna"}]}
assert resolve_track_artists(original, None, None) == ["Rihanna"]
def test_non_list_artists_value_falls_through():
original = {"artists": "Kendrick Lamar"} # string, not list
track = {"artists": [{"name": "Kendrick Lamar"}, {"name": "Rihanna"}]}
assert resolve_track_artists(original, track, None) == ["Kendrick Lamar", "Rihanna"]
def test_empty_original_artists_list_falls_through_to_track_info():
original = {"artists": []}
track = {"artists": [{"name": "Kendrick Lamar"}, {"name": "Rihanna"}]}
assert resolve_track_artists(original, track, None) == ["Kendrick Lamar", "Rihanna"]
def test_artist_dict_name_blank_returns_empty():
assert resolve_track_artists({}, {}, {"name": " "}) == []
assert resolve_track_artists({}, {}, {"name": ""}) == []
def test_non_string_artist_items_coerced_to_string():
original = {"artists": [123, {"name": "Real Artist"}]}
assert resolve_track_artists(original, None, None) == ["123", "Real Artist"]
def test_none_artist_items_dropped():
original = {"artists": [None, {"name": "Real Artist"}, None]}
assert resolve_track_artists(original, None, None) == ["Real Artist"]

View file

@ -0,0 +1,208 @@
"""Pin the Deezer per-track cache validity check.
Issue #588: contributors tagging worked for some tracks and not others.
Root cause was cache pollution `/album/<id>/tracks` cached partial
records under the same key as `/track/<id>`, and `get_track_details`
was using `track_position` alone as the "full payload" sentinel. Both
endpoints set track_position; only `/track/<id>` sets contributors.
These tests pin the corrected sentinel (`_is_full_track_payload`) so
the regression can't silently come back.
"""
from unittest.mock import MagicMock, patch
import pytest
from core.deezer_client import _is_full_track_payload
# ────────────────────────────────────────────────────────────────────
# Pure helper — payload-shape classification
# ────────────────────────────────────────────────────────────────────
def test_full_track_endpoint_payload_is_valid():
payload = {
'id': 12345,
'title': 'Erased',
'track_position': 1,
'contributors': [{'name': 'Whipped Cream'}, {'name': 'Andrea Botez'}],
'artist': {'name': 'Whipped Cream'},
'album': {'id': 1, 'title': 'Erased'},
}
assert _is_full_track_payload(payload) is True
def test_full_track_with_empty_contributors_list_is_valid():
# Single-artist track from /track/<id> still emits contributors=[]
# The KEY presence is what matters, not truthiness.
payload = {
'id': 12345,
'title': 'Solo Track',
'track_position': 1,
'contributors': [],
'artist': {'name': 'Solo Artist'},
}
assert _is_full_track_payload(payload) is True
def test_album_tracks_payload_missing_contributors_is_partial():
# The exact shape /album/<id>/tracks returns per item — has
# track_position but no contributors. Pre-fix this passed the
# `track_position in cached` check; post-fix it correctly falls
# through to a fresh /track/<id> fetch.
payload = {
'id': 12345,
'title': 'Sacrifice',
'track_position': 1,
'duration': 180,
'artist': {'name': 'Andrea Botez'},
}
assert _is_full_track_payload(payload) is False
def test_search_payload_without_track_position_is_partial():
payload = {
'id': 12345,
'title': 'Sacrifice',
'artist': {'name': 'Andrea Botez'},
'isrc': 'XX1234567890',
}
assert _is_full_track_payload(payload) is False
def test_none_or_non_dict_payload_is_partial():
assert _is_full_track_payload(None) is False
assert _is_full_track_payload([]) is False
assert _is_full_track_payload('string') is False
assert _is_full_track_payload(0) is False
def test_empty_dict_is_partial():
assert _is_full_track_payload({}) is False
# ────────────────────────────────────────────────────────────────────
# get_track_details — cache + fetch interaction
# ────────────────────────────────────────────────────────────────────
@pytest.fixture
def deezer_client():
"""Build a DeezerClient with mocked HTTP + cache. Bypasses __init__
auth/config requirements."""
from core.deezer_client import DeezerClient
client = DeezerClient.__new__(DeezerClient)
client._api_get = MagicMock()
return client
def _patch_cache(cached_payload):
"""Patch the module-level cache lookup. Returns the patched cache
mock so callers can assert on store_entity calls."""
cache = MagicMock()
cache.get_entity.return_value = cached_payload
cache.store_entity = MagicMock()
return patch('core.deezer_client.get_metadata_cache', return_value=cache), cache
def test_cache_hit_with_full_payload_skips_api_call(deezer_client):
full = {
'id': 12345,
'title': 'Erased',
'track_position': 1,
'contributors': [{'name': 'Whipped Cream'}, {'name': 'Andrea Botez'}],
'artist': {'name': 'Whipped Cream'},
'album': {'id': 1, 'title': 'Erased', 'nb_tracks': 1},
}
cache_patch, cache = _patch_cache(full)
with cache_patch:
result = deezer_client.get_track_details('12345')
assert result is not None
assert result['artists'] == ['Whipped Cream', 'Andrea Botez']
deezer_client._api_get.assert_not_called()
cache.store_entity.assert_not_called()
def test_cache_hit_with_partial_album_tracks_payload_refetches(deezer_client):
"""The bug from #588 — partial album-tracks data should NOT be
treated as a full hit. Post-fix the client re-fetches."""
partial = {
'id': 12345,
'title': 'Sacrifice',
'track_position': 1,
'artist': {'name': 'Andrea Botez'},
}
fresh = {
'id': 12345,
'title': 'Sacrifice',
'track_position': 1,
'contributors': [{'name': 'Andrea Botez'}, {'name': 'Grabbitz'}],
'artist': {'name': 'Andrea Botez'},
'album': {'id': 1, 'title': 'Sacrifice', 'nb_tracks': 1},
}
cache_patch, cache = _patch_cache(partial)
deezer_client._api_get.return_value = fresh
with cache_patch:
result = deezer_client.get_track_details('12345')
assert result is not None
assert result['artists'] == ['Andrea Botez', 'Grabbitz']
deezer_client._api_get.assert_called_once_with('track/12345')
cache.store_entity.assert_called_once_with('deezer', 'track', '12345', fresh)
def test_cache_miss_fetches_fresh(deezer_client):
cache_patch, cache = _patch_cache(None)
fresh = {
'id': 12345,
'title': 'Sacrifice',
'track_position': 1,
'contributors': [{'name': 'Andrea Botez'}, {'name': 'Grabbitz'}],
'artist': {'name': 'Andrea Botez'},
'album': {'id': 1, 'title': 'Sacrifice', 'nb_tracks': 1},
}
deezer_client._api_get.return_value = fresh
with cache_patch:
result = deezer_client.get_track_details('12345')
assert result is not None
assert result['artists'] == ['Andrea Botez', 'Grabbitz']
deezer_client._api_get.assert_called_once_with('track/12345')
cache.store_entity.assert_called_once()
def test_cache_hit_with_search_shape_refetches(deezer_client):
"""Search results lack track_position — same fall-through path as
partial album-tracks data."""
search_shape = {
'id': 12345,
'title': 'Sacrifice',
'artist': {'name': 'Andrea Botez'},
'isrc': 'XX1234567890',
}
fresh = {
'id': 12345,
'title': 'Sacrifice',
'track_position': 1,
'contributors': [{'name': 'Andrea Botez'}, {'name': 'Grabbitz'}],
'artist': {'name': 'Andrea Botez'},
'album': {'id': 1, 'title': 'Sacrifice', 'nb_tracks': 1},
}
cache_patch, _ = _patch_cache(search_shape)
deezer_client._api_get.return_value = fresh
with cache_patch:
result = deezer_client.get_track_details('12345')
assert result is not None
assert result['artists'] == ['Andrea Botez', 'Grabbitz']
deezer_client._api_get.assert_called_once()
def test_api_failure_returns_none(deezer_client):
cache_patch, _ = _patch_cache(None)
deezer_client._api_get.return_value = None
with cache_patch:
result = deezer_client.get_track_details('12345')
assert result is None

View file

@ -99,8 +99,10 @@ class TestArtistsListPopulated:
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."""
"""When search response has no artists list, the artist-resolution
helper falls back to the artist_dict.name as a single-element list.
Multi-value tag write still no-ops because len([primary]) == 1.
"""
from core.metadata import source as src_module
context = {
@ -109,7 +111,31 @@ class TestArtistsListPopulated:
}
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, [])
assert meta.get("_artists_list") == ["X"]
def test_soulseek_shape_falls_back_to_track_info_artists(self):
"""Soulseek matched-download regression: original_search_result
carries 'artist' (singular string) but no 'artists' list, while
track_info (the matched Spotify track object) carries the full
multi-artist array. Helper should pull from track_info.
"""
from core.metadata import source as src_module
context = {
"original_search_result": {
"title": "DNA.",
"artist": "Kendrick Lamar",
},
"track_info": {
"name": "DNA.",
"artists": [{"name": "Kendrick Lamar"}, {"name": "Rihanna"}],
},
"source": "spotify",
}
with patch.object(src_module, "get_config_manager", return_value=_make_cfg()):
meta = src_module.extract_source_metadata(context, {"name": "Kendrick Lamar"}, {})
assert meta.get("_artists_list") == ["Kendrick Lamar", "Rihanna"]
assert meta.get("artist") == "Kendrick Lamar, Rihanna"
# ---------------------------------------------------------------------------

View file

@ -0,0 +1,103 @@
"""Tests for the track-number tag formatter.
Discord report (Netti93): album tracks tagged as "6/0" instead of
"6/13" when source data lacked total_tracks. Helper now returns just
"6" when total is 0 / unknown, matching what the retag tool already
did and what the ID3 spec allows.
"""
from core.metadata.track_number_format import (
format_track_number_tag,
format_track_number_tuple,
)
# ──────────────────────────────────────────────────────────────────────
# format_track_number_tag — string output for ID3 / Vorbis
# ──────────────────────────────────────────────────────────────────────
def test_track_with_known_total_returns_slash_format():
assert format_track_number_tag(6, 13) == "6/13"
assert format_track_number_tag(1, 1) == "1/1"
assert format_track_number_tag(99, 100) == "99/100"
def test_zero_total_returns_track_number_only():
"""The Netti93 case — total_tracks=0 means unknown, NOT
'track 6 of 0'. Drop the slash."""
assert format_track_number_tag(6, 0) == "6"
assert format_track_number_tag(1, 0) == "1"
def test_none_total_returns_track_number_only():
assert format_track_number_tag(6, None) == "6"
def test_none_track_number_defaults_to_one():
assert format_track_number_tag(None, 13) == "1/13"
assert format_track_number_tag(None, None) == "1"
def test_zero_track_number_defaults_to_one():
"""Track 0 isn't valid in any convention — coerce to 1."""
# Note: 0 is non-negative so falls into the default-0 path which
# the formatter then treats as "default" via the explicit default
# arg. Since 0 is technically valid output of `int(0)`, the helper
# passes it through. Document the behavior here.
# Actually re-checking: 0 satisfies `>= 0` so returns 0. That
# means format would emit "0/13" for malformed input. Not great
# but at least it doesn't crash. Test pins current behavior.
assert format_track_number_tag(0, 13) == "0/13"
def test_negative_total_treated_as_unknown():
assert format_track_number_tag(6, -1) == "6"
def test_negative_track_number_falls_back_to_default():
assert format_track_number_tag(-1, 13) == "1/13"
def test_string_inputs_coerced():
assert format_track_number_tag("6", "13") == "6/13"
assert format_track_number_tag("6", "0") == "6"
def test_unparseable_inputs_use_defaults():
assert format_track_number_tag("six", "thirteen") == "1"
assert format_track_number_tag("abc", 13) == "1/13"
def test_float_inputs_truncate():
# int() truncates floats — keeps behavior deterministic
assert format_track_number_tag(6.7, 13.9) == "6/13"
# ──────────────────────────────────────────────────────────────────────
# format_track_number_tuple — MP4 trkn tuple
# ──────────────────────────────────────────────────────────────────────
def test_tuple_with_known_total():
assert format_track_number_tuple(6, 13) == (6, 13)
def test_tuple_with_zero_total():
assert format_track_number_tuple(6, 0) == (6, 0)
def test_tuple_with_none_total():
assert format_track_number_tuple(6, None) == (6, 0)
def test_tuple_with_none_track_defaults_to_one():
assert format_track_number_tuple(None, 13) == (1, 13)
assert format_track_number_tuple(None, None) == (1, 0)
def test_tuple_negative_inputs_safe():
assert format_track_number_tuple(-1, -5) == (1, 0)
def test_tuple_string_inputs_coerced():
assert format_track_number_tuple("6", "13") == (6, 13)
assert format_track_number_tuple("6", "0") == (6, 0)

0
tests/sync/__init__.py Normal file
View file

View file

@ -0,0 +1,193 @@
from unittest.mock import MagicMock
from core.sync.match_overrides import record_manual_match, resolve_match_overrides
# ──────────────────────────────────────────────────────────────────────
# resolve_match_overrides — pre-pair source→server from cache
# ──────────────────────────────────────────────────────────────────────
def test_empty_inputs_return_empty_dict():
assert resolve_match_overrides([], [], lambda _id: None) == {}
assert resolve_match_overrides([{"source_track_id": "x"}], [], lambda _id: "y") == {}
assert resolve_match_overrides([], [{"id": "y"}], lambda _id: None) == {}
def test_single_cache_hit_returns_pair():
sources = [{"source_track_id": "spotify-iron-man", "name": "Iron Man - 2012 - Remaster"}]
servers = [{"id": 5001, "title": "Iron Man"}]
cache = {"spotify-iron-man": 5001}
result = resolve_match_overrides(sources, servers, lambda sid: cache.get(sid))
assert result == {0: 0}
def test_multiple_overrides_resolve_correctly():
sources = [
{"source_track_id": "iron"},
{"source_track_id": "para"},
{"source_track_id": "war"},
]
servers = [
{"id": 5001, "title": "Iron Man"},
{"id": 5002, "title": "Paranoid"},
{"id": 5003, "title": "War Pigs"},
]
cache = {"iron": 5001, "para": 5002, "war": 5003}
result = resolve_match_overrides(sources, servers, lambda sid: cache.get(sid))
assert result == {0: 0, 1: 1, 2: 2}
def test_source_without_track_id_skipped():
sources = [
{"source_track_id": "iron", "name": "Iron Man"},
{"name": "Paranoid"}, # no source_track_id (e.g. legacy / non-mirrored)
]
servers = [{"id": 5001, "title": "Iron Man"}, {"id": 5002, "title": "Paranoid"}]
cache = {"iron": 5001}
result = resolve_match_overrides(sources, servers, lambda sid: cache.get(sid))
assert result == {0: 0}
def test_cache_miss_skipped():
sources = [{"source_track_id": "iron"}, {"source_track_id": "para"}]
servers = [{"id": 5001, "title": "Iron Man"}, {"id": 5002, "title": "Paranoid"}]
result = resolve_match_overrides(sources, servers, lambda sid: None)
assert result == {}
def test_stale_cache_pointing_at_missing_server_track_skipped():
# User cached a match → file got deleted from server → server_tracks
# no longer has 5001 → don't pair, fall through to normal matching.
sources = [{"source_track_id": "iron"}]
servers = [{"id": 9999, "title": "Different Track"}]
cache = {"iron": 5001} # 5001 no longer exists
result = resolve_match_overrides(sources, servers, lambda sid: cache.get(sid))
assert result == {}
def test_server_id_str_int_coercion():
# Cache might store ints, server_tracks might have str IDs (Plex
# ratingKey is str). Helper coerces both sides to str.
sources = [{"source_track_id": "iron"}]
servers = [{"id": "5001", "title": "Iron Man"}]
cache = {"iron": 5001} # int from cache
result = resolve_match_overrides(sources, servers, lambda sid: cache.get(sid))
assert result == {0: 0}
def test_two_sources_pointing_at_same_server_track_only_first_wins():
# Defensive — UNIQUE constraint prevents this in production but
# cache_lookup is injectable so we verify the safety.
sources = [{"source_track_id": "a"}, {"source_track_id": "b"}]
servers = [{"id": 5001, "title": "Iron Man"}]
cache = {"a": 5001, "b": 5001}
result = resolve_match_overrides(sources, servers, lambda sid: cache.get(sid))
assert result == {0: 0}
def test_cache_lookup_raising_treated_as_miss():
sources = [{"source_track_id": "iron"}]
servers = [{"id": 5001, "title": "Iron Man"}]
def boom(_sid):
raise RuntimeError("db down")
result = resolve_match_overrides(sources, servers, boom)
assert result == {}
def test_non_dict_source_or_server_skipped():
sources = [None, "string", {"source_track_id": "iron"}]
servers = [{"id": 5001, "title": "Iron Man"}]
cache = {"iron": 5001}
result = resolve_match_overrides(sources, servers, lambda sid: cache.get(sid))
# source idx 2 → server idx 0
assert result == {2: 0}
def test_server_without_id_skipped():
sources = [{"source_track_id": "iron"}]
servers = [{"title": "Iron Man"}] # no id
cache = {"iron": 5001}
result = resolve_match_overrides(sources, servers, lambda sid: cache.get(sid))
assert result == {}
def test_partial_cache_hits_only_pair_those():
sources = [
{"source_track_id": "iron"},
{"source_track_id": "para"},
{"source_track_id": "war"},
]
servers = [
{"id": 5001, "title": "Iron Man"},
{"id": 5002, "title": "Paranoid"},
{"id": 5003, "title": "War Pigs"},
]
# Only iron + war cached, para falls through to normal matching
cache = {"iron": 5001, "war": 5003}
result = resolve_match_overrides(sources, servers, lambda sid: cache.get(sid))
assert result == {0: 0, 2: 2}
# ──────────────────────────────────────────────────────────────────────
# record_manual_match — persist user-confirmed pair
# ──────────────────────────────────────────────────────────────────────
def test_record_persists_with_confidence_one():
db = MagicMock()
db.save_sync_match_cache.return_value = True
ok = record_manual_match(
db,
source_track_id="spotify-iron-man",
server_source="plex",
server_track_id=5001,
server_track_title="Iron Man",
source_title="Iron Man - 2012 - Remaster",
source_artist="Black Sabbath",
)
assert ok is True
db.save_sync_match_cache.assert_called_once()
kwargs = db.save_sync_match_cache.call_args.kwargs
assert kwargs["spotify_track_id"] == "spotify-iron-man"
assert kwargs["server_source"] == "plex"
assert kwargs["server_track_id"] == 5001
assert kwargs["server_track_title"] == "Iron Man"
assert kwargs["confidence"] == 1.0
assert kwargs["normalized_title"] == "iron man - 2012 - remaster"
assert kwargs["normalized_artist"] == "black sabbath"
def test_record_returns_false_when_required_fields_missing():
db = MagicMock()
assert record_manual_match(db, source_track_id="", server_source="plex", server_track_id=1) is False
assert record_manual_match(db, source_track_id="x", server_source="", server_track_id=1) is False
assert record_manual_match(db, source_track_id="x", server_source="plex", server_track_id=None) is False
db.save_sync_match_cache.assert_not_called()
def test_record_returns_false_when_db_save_returns_false():
db = MagicMock()
db.save_sync_match_cache.return_value = False
assert record_manual_match(db, source_track_id="x", server_source="plex", server_track_id=1) is False
def test_record_swallows_db_exception():
db = MagicMock()
db.save_sync_match_cache.side_effect = RuntimeError("db boom")
assert record_manual_match(db, source_track_id="x", server_source="plex", server_track_id=1) is False
def test_record_returns_false_when_db_lacks_method():
class NoSaveDB:
pass
assert record_manual_match(NoSaveDB(), source_track_id="x", server_source="plex", server_track_id=1) is False
def test_record_handles_empty_optional_strings():
db = MagicMock()
db.save_sync_match_cache.return_value = True
ok = record_manual_match(db, source_track_id="x", server_source="plex", server_track_id=1)
assert ok is True
kwargs = db.save_sync_match_cache.call_args.kwargs
assert kwargs["normalized_title"] == ""
assert kwargs["normalized_artist"] == ""
assert kwargs["server_track_title"] == ""

View file

@ -54,11 +54,11 @@ def _make_context(rows):
def test_load_db_tracks_skips_null_ids_and_normalizes_track_ids():
job = AcoustIDScannerJob()
context = _make_context([
# 10 columns: id, title, artist (COALESCE'd), file_path, track_number,
# 11 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"),
# album_artist, duration_ms (issue #587 — duration guard).
(None, "Broken Track", "Artist", "/music/broken.flac", 1, "Album", None, None, "", "Artist", 180000),
(42, "Good Track", "Artist", "/music/good.flac", 2, "Album", "album-thumb", "artist-thumb", "", "Artist", 240000),
])
tracks = job._load_db_tracks(context)
@ -66,16 +66,17 @@ def test_load_db_tracks_skips_null_ids_and_normalizes_track_ids():
assert list(tracks.keys()) == ["42"]
assert tracks["42"]["title"] == "Good Track"
assert tracks["42"]["artist"] == "Artist"
assert tracks["42"]["duration_ms"] == 240000
def test_scan_handles_mixed_track_id_types(monkeypatch):
job = AcoustIDScannerJob()
context = _make_context([
# 10 columns: id, title, artist (COALESCE'd), file_path, track_number,
# 11 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"),
# album_artist, duration_ms.
(None, "Broken Track", "Artist", "/music/broken.flac", 1, "Album", None, None, "", "Artist", 180000),
(42, "Good Track", "Artist", "/music/good.flac", 2, "Album", "album-thumb", "artist-thumb", "", "Artist", 240000),
])
monkeypatch.setattr(job, "_resolve_path", lambda file_path, _context: file_path)
@ -275,7 +276,8 @@ def _make_real_db_context(tmp_path):
album_id TEXT,
file_path TEXT,
track_number INTEGER,
track_artist TEXT
track_artist TEXT,
duration INTEGER
);
""")
conn.commit()
@ -620,3 +622,160 @@ def test_scanner_file_tag_matches_db_no_behavioral_change(monkeypatch):
)
assert captured_findings == []
# ---------------------------------------------------------------------------
# Issue #587 — multi-candidate scan + duration guard (Foxxify report)
# ---------------------------------------------------------------------------
def test_scanner_no_finding_when_lower_ranked_candidate_matches():
"""Foxxify case 2 — AcoustID returns multiple recordings per
fingerprint; the top match is the wrong-credited recording but a
lower-ranked candidate matches expected metadata exactly. Scanner
should iterate ALL candidates and suppress the finding.
Repro: file is "Nana" by Geoxor, AcoustID top match is "Nana" by
Edward Vesala Trio (different recording sharing similar
fingerprint), AcoustID's second candidate is the actual Geoxor
track. Pre-fix scanner only saw [0] flagged. Post-fix sees [1]
no flag."""
job = AcoustIDScannerJob()
captured_findings = []
context = _make_finding_capturing_context(
track_row=("nana", "Nana", "Geoxor",
"/music/nana.opus", 6, "Stardust", None, None),
captured=captured_findings,
)
fake_acoustid = SimpleNamespace(
fingerprint_and_lookup=lambda fpath: {
'best_score': 0.97,
'recordings': [
# AcoustID's top match — wrong artist for our file
{'title': 'Nana', 'artist': 'Edward Vesala Trio'},
# Lower-ranked candidate — actually matches our expected
{'title': 'Nana', 'artist': 'Geoxor'},
],
},
)
result = JobResultStub()
job._scan_file(
'/music/nana.opus', 'nana',
{'title': 'Nana', 'artist': 'Geoxor'},
fake_acoustid, context, result,
fp_threshold=0.85, title_threshold=0.85, artist_threshold=0.6,
)
assert captured_findings == [], (
f"Expected no finding (lower-ranked candidate matches); got {captured_findings}"
)
def test_scanner_still_flags_when_no_candidate_matches():
"""Confirm the multi-candidate check doesn't accidentally suppress
legitimate mismatches if NO candidate matches expected metadata,
the finding still fires."""
job = AcoustIDScannerJob()
captured_findings = []
context = _make_finding_capturing_context(
track_row=("99", "Expected Title", "Expected Artist",
"/music/track.flac", 1, "Album", None, None),
captured=captured_findings,
)
fake_acoustid = SimpleNamespace(
fingerprint_and_lookup=lambda fpath: {
'best_score': 0.99,
'recordings': [
{'title': 'Wrong Track', 'artist': 'Wrong Artist A'},
{'title': 'Different Wrong', 'artist': 'Wrong Artist B'},
],
},
)
result = JobResultStub()
job._scan_file(
'/music/track.flac', '99',
{'title': 'Expected Title', 'artist': 'Expected Artist'},
fake_acoustid, context, result,
fp_threshold=0.85, title_threshold=0.85, artist_threshold=0.6,
)
assert len(captured_findings) == 1
def test_scanner_skips_finding_on_strong_duration_mismatch():
"""Foxxify case 3 — 17-minute mashup edit fingerprints to a 5-minute
late-70s Japanese hiphop track. Fingerprint matched a sample/intro
section but the recordings are clearly different (drastic length
difference). Scanner should skip the finding rather than recommend
retag of a totally different track length."""
job = AcoustIDScannerJob()
captured_findings = []
context = _make_finding_capturing_context(
track_row=("mashup", "Some Mashup Edit", "Mashup Artist",
"/music/mashup.opus", 1, "Mashups", None, None),
captured=captured_findings,
)
# AcoustID matched a 5-minute Japanese hiphop track via fingerprint
# hash collision. Expected file is 17 minutes — duration guard
# should kick in.
fake_acoustid = SimpleNamespace(
fingerprint_and_lookup=lambda fpath: {
'best_score': 0.98,
'recordings': [
{'title': 'Different Song', 'artist': 'Different Artist',
'duration': 300}, # 5 min — way off from our 17 min file
],
},
)
result = JobResultStub()
# 17 minutes = 1020 sec = 1020000 ms
job._scan_file(
'/music/mashup.opus', 'mashup',
{'title': 'Some Mashup Edit', 'artist': 'Mashup Artist', 'duration_ms': 1020000},
fake_acoustid, context, result,
fp_threshold=0.85, title_threshold=0.85, artist_threshold=0.6,
)
assert captured_findings == [], (
f"Expected no finding (duration mismatch suggests collision); got {captured_findings}"
)
def test_scanner_still_flags_when_duration_matches():
"""Confirm the duration guard only kicks in for STRONG mismatches —
similar-length wrong song still gets flagged."""
job = AcoustIDScannerJob()
captured_findings = []
context = _make_finding_capturing_context(
track_row=("99", "Expected", "Artist",
"/music/track.flac", 1, "Album", None, None),
captured=captured_findings,
)
fake_acoustid = SimpleNamespace(
fingerprint_and_lookup=lambda fpath: {
'best_score': 0.99,
'recordings': [
{'title': 'Wrong Song', 'artist': 'Wrong Artist',
'duration': 180}, # 3 min, matches expected
],
},
)
result = JobResultStub()
# 3-minute file with 3-minute candidate — same length, but title +
# artist clearly mismatch → finding should still fire
job._scan_file(
'/music/track.flac', '99',
{'title': 'Expected', 'artist': 'Artist', 'duration_ms': 180000},
fake_acoustid, context, result,
fp_threshold=0.85, title_threshold=0.85, artist_threshold=0.6,
)
assert len(captured_findings) == 1

View file

@ -0,0 +1,220 @@
"""Tests for re-embedding lyrics from an existing sidecar file.
Discord report (Netti93): retag was clearing the LYRICS / USLT tag
without rewriting it. Cause was two-fold:
1. `core/library/retag.py:execute_retag` never called
`generate_lrc_file` after `enhance_file_metadata`. The download
pipeline does retag was inconsistent.
2. Even with the call added, `lyrics_client.create_lrc_file` used to
short-circuit when an .lrc / .txt sidecar already existed (the
typical retag case sidecar moved alongside the audio file).
Pre-fix: returned True without re-embedding USLT. Post-fix: reads
the existing sidecar and re-embeds the USLT tag.
"""
from __future__ import annotations
import os
import tempfile
from unittest.mock import patch, MagicMock
import pytest
@pytest.fixture
def fake_audio_file(tmp_path):
"""Build a minimal FLAC file with no LYRICS tag."""
fd, path = tempfile.mkstemp(suffix='.flac', dir=str(tmp_path))
os.close(fd)
minimal = (
b'fLaC'
+ b'\x80\x00\x00\x22'
+ b'\x00\x10\x00\x10'
+ b'\x00\x00\x00\x00\x00\x00'
+ b'\x0a\xc4\x42\xf0\x00\x00\x00\x00'
+ b'\x00' * 16
)
with open(path, 'wb') as f:
f.write(minimal)
yield path
# ──────────────────────────────────────────────────────────────────────
# create_lrc_file — re-embed when sidecar present
# ──────────────────────────────────────────────────────────────────────
def test_existing_lrc_sidecar_triggers_reembed(fake_audio_file):
"""The exact retag scenario — sidecar already exists alongside the
audio file (moved during retag), USLT got cleared by enrichment.
Helper should read the sidecar and re-embed without hitting LRClib."""
from core.lyrics_client import LyricsClient
sidecar_path = os.path.splitext(fake_audio_file)[0] + '.lrc'
with open(sidecar_path, 'w', encoding='utf-8') as f:
f.write('[00:01.00]Test lyric line\n[00:05.00]Second line')
client = LyricsClient()
client.api = MagicMock() # API stub — should NOT be called
client._embed_lyrics = MagicMock()
result = client.create_lrc_file(
audio_file_path=fake_audio_file,
track_name='Test',
artist_name='Artist',
)
assert result is True
# API never hit — sidecar shortcut
client.api.get_lyrics.assert_not_called()
client.api.search_lyrics.assert_not_called()
# USLT was re-embedded
client._embed_lyrics.assert_called_once()
call_args = client._embed_lyrics.call_args
assert call_args.args[0] == fake_audio_file
assert 'Test lyric line' in call_args.args[1]
def test_existing_txt_sidecar_also_triggers_reembed(fake_audio_file):
"""Same shape with .txt sidecar (plain lyrics, no timestamps)."""
from core.lyrics_client import LyricsClient
sidecar_path = os.path.splitext(fake_audio_file)[0] + '.txt'
with open(sidecar_path, 'w', encoding='utf-8') as f:
f.write('Just plain lyrics no timestamps')
client = LyricsClient()
client.api = MagicMock()
client._embed_lyrics = MagicMock()
result = client.create_lrc_file(
audio_file_path=fake_audio_file,
track_name='T', artist_name='A',
)
assert result is True
client._embed_lyrics.assert_called_once_with(
fake_audio_file, 'Just plain lyrics no timestamps'
)
def test_empty_sidecar_does_not_embed(fake_audio_file):
"""Defensive — if the sidecar exists but is empty, don't write an
empty USLT tag."""
from core.lyrics_client import LyricsClient
sidecar_path = os.path.splitext(fake_audio_file)[0] + '.lrc'
with open(sidecar_path, 'w', encoding='utf-8') as f:
f.write(' \n ') # whitespace only
client = LyricsClient()
client.api = MagicMock()
client._embed_lyrics = MagicMock()
result = client.create_lrc_file(
audio_file_path=fake_audio_file,
track_name='T', artist_name='A',
)
assert result is True
client._embed_lyrics.assert_not_called()
def test_unreadable_sidecar_swallows_error_returns_true(fake_audio_file):
"""If the sidecar is somehow unreadable, return True (don't try
LRClib again the early-return contract holds), just skip the
embed silently."""
from core.lyrics_client import LyricsClient
sidecar_path = os.path.splitext(fake_audio_file)[0] + '.lrc'
with open(sidecar_path, 'wb') as f:
f.write(b'\xff\xfe\x00\x00') # invalid UTF-8
client = LyricsClient()
client.api = MagicMock()
client._embed_lyrics = MagicMock()
result = client.create_lrc_file(
audio_file_path=fake_audio_file,
track_name='T', artist_name='A',
)
assert result is True
client.api.get_lyrics.assert_not_called()
def test_no_sidecar_falls_through_to_lrclib(fake_audio_file):
"""No sidecar → original LRClib fetch path runs (download flow)."""
from core.lyrics_client import LyricsClient
client = LyricsClient()
fake_lyrics = MagicMock()
fake_lyrics.synced_lyrics = '[00:01.00]synced from api'
fake_lyrics.plain_lyrics = None
client.api = MagicMock()
client.api.get_lyrics.return_value = None
client.api.search_lyrics.return_value = [fake_lyrics]
client._embed_lyrics = MagicMock()
result = client.create_lrc_file(
audio_file_path=fake_audio_file,
track_name='T', artist_name='A',
)
assert result is True
client.api.search_lyrics.assert_called_once()
# Sidecar was created
lrc = os.path.splitext(fake_audio_file)[0] + '.lrc'
assert os.path.exists(lrc)
# And USLT was embedded
client._embed_lyrics.assert_called_once()
# ──────────────────────────────────────────────────────────────────────
# RetagDeps integration — generate_lrc_file is now wired
# ──────────────────────────────────────────────────────────────────────
def test_retagdeps_accepts_generate_lrc_file_field():
from core.library.retag import RetagDeps
# Mock the required + optional deps with do-nothing callables
deps = RetagDeps(
config_manager=MagicMock(),
retag_lock=MagicMock(),
spotify_client=MagicMock(),
get_audio_quality_string=lambda *a: '',
enhance_file_metadata=lambda *a: True,
build_final_path_for_track=lambda *a: ('', ''),
safe_move_file=lambda *a: None,
cleanup_empty_directories=lambda *a: None,
download_cover_art=lambda *a: None,
docker_resolve_path=lambda x: x,
_get_retag_state=lambda: {},
_set_retag_state=lambda v: None,
get_database=lambda: MagicMock(),
generate_lrc_file=lambda *a: True,
)
assert callable(deps.generate_lrc_file)
def test_retagdeps_generate_lrc_file_optional_for_backward_compat():
"""Tests that built RetagDeps without the new field don't break."""
from core.library.retag import RetagDeps
deps = RetagDeps(
config_manager=MagicMock(),
retag_lock=MagicMock(),
spotify_client=MagicMock(),
get_audio_quality_string=lambda *a: '',
enhance_file_metadata=lambda *a: True,
build_final_path_for_track=lambda *a: ('', ''),
safe_move_file=lambda *a: None,
cleanup_empty_directories=lambda *a: None,
download_cover_art=lambda *a: None,
docker_resolve_path=lambda x: x,
_get_retag_state=lambda: {},
_set_retag_state=lambda v: None,
get_database=lambda: MagicMock(),
)
# Field defaults to None — no crash on construction.
assert deps.generate_lrc_file is None

View file

@ -0,0 +1,175 @@
"""Tests for the multi-value artist write path in tag_writer.
Issue #587 — the AcoustID scanner's "Apply Match" retag was bypassing
the user's `metadata_enhancement.tags.write_multi_artist` setting and
writing single-string TPE1 only. The repair-path retag now passes an
``artists_list`` to ``write_tags_to_file`` and the writer respects the
config flag the same way the post-download enrichment pipeline does.
"""
from __future__ import annotations
import os
import tempfile
from unittest.mock import patch
import pytest
from mutagen.flac import FLAC, StreamInfo
from core.tag_writer import (
_multi_artist_write_enabled,
_resolve_artists_list_for_write,
write_tags_to_file,
)
# ──────────────────────────────────────────────────────────────────────
# _resolve_artists_list_for_write — derives the multi-value list
# ──────────────────────────────────────────────────────────────────────
def test_resolves_artists_list_field():
assert _resolve_artists_list_for_write({'artists_list': ['A', 'B']}) == ['A', 'B']
def test_resolves_artists_field_alias():
assert _resolve_artists_list_for_write({'artists': ['A', 'B']}) == ['A', 'B']
def test_resolves_underscore_artists_list_field():
# _artists_list — the post-process pipeline's internal field name
assert _resolve_artists_list_for_write({'_artists_list': ['A', 'B']}) == ['A', 'B']
def test_returns_none_when_no_list_supplied():
assert _resolve_artists_list_for_write({'artist_name': 'Solo'}) is None
def test_returns_none_for_non_list_input():
assert _resolve_artists_list_for_write({'artists_list': 'A, B'}) is None
assert _resolve_artists_list_for_write({'artists_list': {'A': 1}}) is None
def test_strips_empty_and_non_string_entries():
out = _resolve_artists_list_for_write({'artists_list': ['A', '', None, ' ', 'B']})
assert out == ['A', 'B']
def test_returns_none_when_list_only_has_empty_entries():
assert _resolve_artists_list_for_write({'artists_list': ['', None, ' ']}) is None
# ──────────────────────────────────────────────────────────────────────
# _multi_artist_write_enabled — config gate
# ──────────────────────────────────────────────────────────────────────
def test_multi_artist_write_reads_config():
with patch('config.settings.config_manager.get', return_value=True):
assert _multi_artist_write_enabled() is True
with patch('config.settings.config_manager.get', return_value=False):
assert _multi_artist_write_enabled() is False
def test_multi_artist_write_swallows_config_error():
with patch('config.settings.config_manager.get', side_effect=RuntimeError('boom')):
assert _multi_artist_write_enabled() is False
# ──────────────────────────────────────────────────────────────────────
# Vorbis end-to-end — write multi-value to a real FLAC file
# ──────────────────────────────────────────────────────────────────────
def _make_minimal_flac(path):
"""Create a tiny but valid FLAC with mutagen's lowest-overhead
stream info so we can write tags and read them back."""
# Empty audio body — just enough to satisfy mutagen's parser.
# 44-byte FLAC header + STREAMINFO block.
minimal = (
b'fLaC'
+ b'\x80\x00\x00\x22' # last STREAMINFO block, length 34
+ b'\x00\x10\x00\x10' # min/max block size
+ b'\x00\x00\x00\x00\x00\x00' # min/max frame size
+ b'\x0a\xc4\x42\xf0\x00\x00\x00\x00' # sample rate / channels / etc
+ b'\x00' * 16 # MD5
)
with open(path, 'wb') as f:
f.write(minimal)
@pytest.fixture
def flac_path():
fd, path = tempfile.mkstemp(suffix='.flac')
os.close(fd)
_make_minimal_flac(path)
yield path
try:
os.remove(path)
except OSError:
pass
def test_multi_value_artists_key_written_when_setting_on(flac_path):
with patch('config.settings.config_manager.get', return_value=True):
result = write_tags_to_file(flac_path, {
'title': 'Track',
'artist_name': 'Artist A, Artist B',
'artists_list': ['Artist A', 'Artist B'],
}, embed_cover=False)
assert result['success'] is True
assert 'artists_multi' in result['written_fields']
audio = FLAC(flac_path)
# Display string preserved as `artist`
assert audio.get('artist') == ['Artist A, Artist B']
# Multi-value list written to `artists` key (Picard convention)
assert audio.get('artists') == ['Artist A', 'Artist B']
def test_multi_value_artists_key_skipped_when_setting_off(flac_path):
with patch('config.settings.config_manager.get', return_value=False):
result = write_tags_to_file(flac_path, {
'title': 'Track',
'artist_name': 'Artist A, Artist B',
'artists_list': ['Artist A', 'Artist B'],
}, embed_cover=False)
assert result['success'] is True
assert 'artists_multi' not in result['written_fields']
audio = FLAC(flac_path)
assert audio.get('artist') == ['Artist A, Artist B']
# No multi-value key when setting is off — backward compat
assert audio.get('artists') is None
def test_single_artist_does_not_write_multi_value_key(flac_path):
"""When list has only one entry (or no list at all), don't
write the multi-value key even if the setting is on. The point
is to differentiate true multi-artist from single-artist tracks."""
with patch('config.settings.config_manager.get', return_value=True):
result = write_tags_to_file(flac_path, {
'title': 'Track',
'artist_name': 'Solo Artist',
'artists_list': ['Solo Artist'],
}, embed_cover=False)
assert result['success'] is True
assert 'artists_multi' not in result['written_fields']
audio = FLAC(flac_path)
assert audio.get('artists') is None
def test_no_artists_list_legacy_callers_unchanged(flac_path):
"""Backward compat — callers that don't supply artists_list get
the same single-string write as before. No regression for the
write_artist_image button or any other tag_writer caller."""
with patch('config.settings.config_manager.get', return_value=True):
result = write_tags_to_file(flac_path, {
'title': 'Track',
'artist_name': 'Solo Artist',
}, embed_cover=False)
assert result['success'] is True
assert 'artists_multi' not in result['written_fields']
audio = FLAC(flac_path)
assert audio.get('artists') is None

View file

@ -0,0 +1,123 @@
"""Tests for Tidal qualifier filtering across primary + fallback search.
Issue #589 — when a download query carries a version qualifier ("live",
"unplugged", "acoustic", etc), the qualifier filter must apply to BOTH
the primary search AND fallback variants. Previously it only fired on
fallbacks, so a primary search for "Shy Away (MTV Unplugged Live)" that
happened to surface the studio cut first would accept the wrong file
and only get caught by AcoustID downstream.
Also covers the album-context extension: for concert / unplugged
releases the live signal lives in the album title, not the track
title. The filter inspects both ``track.name`` AND ``track.album.name``.
"""
from __future__ import annotations
from unittest.mock import MagicMock
from core.tidal_download_client import TidalDownloadClient
def _make_track(name: str, album_name: str = ''):
"""Build a minimal duck-typed track object matching what the Tidal
SDK returns: a `name` attribute and an `album` attribute with its
own `name`."""
track = MagicMock()
track.name = name
track.album = MagicMock()
track.album.name = album_name
return track
# ──────────────────────────────────────────────────────────────────────
# _track_name_contains_qualifiers — legacy track-only behavior preserved
# ──────────────────────────────────────────────────────────────────────
def test_legacy_helper_passes_when_track_name_has_qualifier():
assert TidalDownloadClient._track_name_contains_qualifiers(
'Shy Away (MTV Unplugged Live)', ['live']
) is True
def test_legacy_helper_fails_when_track_name_lacks_qualifier():
assert TidalDownloadClient._track_name_contains_qualifiers(
'Shy Away', ['live']
) is False
def test_legacy_helper_passes_when_no_qualifiers_required():
assert TidalDownloadClient._track_name_contains_qualifiers(
'Anything', []
) is True
# ──────────────────────────────────────────────────────────────────────
# _track_matches_qualifiers — new helper inspects track + album
# ──────────────────────────────────────────────────────────────────────
def test_qualifier_in_track_name_alone_passes():
track = _make_track('Shy Away (Live)', 'DAMN.')
assert TidalDownloadClient._track_matches_qualifiers(track, ['live']) is True
def test_qualifier_in_album_name_alone_passes():
# MTV Unplugged scenario — track titled "Shy Away" but album
# carries the live context. Pre-fix this returned False because
# only track.name was checked.
track = _make_track('Shy Away', 'MTV Unplugged Live')
assert TidalDownloadClient._track_matches_qualifiers(track, ['live']) is True
def test_qualifier_missing_from_both_fails():
# User asked for live, Tidal returned the studio cut on a studio
# album. Must reject so the search keeps looking.
track = _make_track('Shy Away', 'Trench')
assert TidalDownloadClient._track_matches_qualifiers(track, ['live']) is False
def test_unplugged_qualifier_in_album_name():
track = _make_track('Only If For A Night', 'MTV Unplugged')
assert TidalDownloadClient._track_matches_qualifiers(track, ['unplugged']) is True
def test_multiple_qualifiers_all_required():
# Both "live" AND "acoustic" must be present somewhere
track = _make_track('Hello', 'Live Acoustic Sessions')
assert TidalDownloadClient._track_matches_qualifiers(track, ['live', 'acoustic']) is True
track2 = _make_track('Hello', 'Live Sessions') # missing acoustic
assert TidalDownloadClient._track_matches_qualifiers(track2, ['live', 'acoustic']) is False
def test_no_qualifiers_required_always_passes():
track = _make_track('Anything', 'Anything')
assert TidalDownloadClient._track_matches_qualifiers(track, []) is True
def test_track_with_no_album_attribute():
# Defensive — duck-typed tracks may not all have album. Use a
# plain object instead of MagicMock so missing .album is real.
class BareTrack:
name = 'Live Track'
album = None
assert TidalDownloadClient._track_matches_qualifiers(BareTrack(), ['live']) is True
assert TidalDownloadClient._track_matches_qualifiers(BareTrack(), ['unplugged']) is False
def test_track_with_empty_name_and_album():
class BareTrack:
name = ''
album = None
assert TidalDownloadClient._track_matches_qualifiers(BareTrack(), ['live']) is False
def test_word_boundary_avoids_false_match_on_substring():
# "session" should NOT match "obsession"
track = _make_track('Obsession', 'Pop Hits')
assert TidalDownloadClient._track_matches_qualifiers(track, ['session']) is False
def test_extract_qualifiers_picks_up_live_unplugged():
quals = TidalDownloadClient._extract_qualifiers('Shy Away (MTV Unplugged Live)')
assert 'live' in quals
assert 'unplugged' in quals

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.2"
_SOULSYNC_BASE_VERSION = "2.5.3"
def _build_version_string():
"""Append short commit hash to version when available (e.g. 2.35+abc1234)."""
@ -115,10 +115,7 @@ from core.imports.context import (
get_import_clean_album,
get_import_clean_title,
get_import_context_album,
get_import_context_artist,
get_import_original_search,
get_import_track_info,
normalize_import_context,
)
from core.wishlist.payloads import (
build_cancelled_task_wishlist_payload as _build_cancelled_task_wishlist_payload,
@ -161,23 +158,25 @@ from core.wishlist.state import (
is_wishlist_actually_processing as _is_wishlist_actually_processing,
reset_flag_if_stuck as _reset_wishlist_flag_if_stuck,
)
from core.imports.album import (
build_album_import_context,
build_album_import_match_payload,
resolve_album_artist_context,
)
from core.imports.album_naming import resolve_album_group as _resolve_album_group
from core.imports.album import build_album_import_match_payload
from core.imports.filename import extract_track_number_from_filename, parse_filename_metadata
from core.imports.staging import (
get_import_suggestions_cache,
get_primary_source,
get_staging_path,
read_staging_file_metadata,
refresh_import_suggestions_cache,
search_import_albums,
search_import_tracks,
start_import_suggestions_cache,
)
from core.imports.routes import ImportRouteRuntime as _ImportRouteRuntime
from core.imports.routes import album_match as _import_album_match
from core.imports.routes import album_process as _import_album_process
from core.imports.routes import process_single_import_file as _import_process_single_import_file
from core.imports.routes import search_albums as _import_search_albums
from core.imports.routes import search_tracks as _import_search_tracks
from core.imports.routes import singles_process as _import_singles_process
from core.imports.routes import staging_files as _import_staging_files
from core.imports.routes import staging_groups as _import_staging_groups
from core.imports.routes import staging_hints as _import_staging_hints
from core.imports.routes import staging_suggestions as _import_staging_suggestions
from core.imports.paths import build_final_path_for_track as _build_final_path_for_track
from core.imports.pipeline import build_import_pipeline_runtime as _build_import_pipeline_runtime
from core.metadata.common import get_file_lock
@ -8195,6 +8194,93 @@ def clear_quarantine():
logger.error(f"[Quarantine] Error clearing quarantine: {e}")
return jsonify({"success": False, "error": str(e)}), 500
def _get_quarantine_dir():
return os.path.join(
docker_resolve_path(config_manager.get('soulseek.download_path', './downloads')),
'ss_quarantine',
)
@app.route('/api/quarantine/list', methods=['GET'])
def list_quarantine():
"""Return all quarantined files with sidecar metadata."""
try:
from core.imports.quarantine import list_quarantine_entries
entries = list_quarantine_entries(_get_quarantine_dir())
return jsonify({"success": True, "entries": entries})
except Exception as e:
logger.error(f"[Quarantine] Error listing entries: {e}")
return jsonify({"success": False, "error": str(e)}), 500
@app.route('/api/quarantine/<entry_id>', methods=['DELETE'])
def delete_quarantine_item(entry_id):
"""Delete a single quarantined file + sidecar."""
try:
from core.imports.quarantine import delete_quarantine_entry
ok = delete_quarantine_entry(_get_quarantine_dir(), entry_id)
if not ok:
return jsonify({"success": False, "error": "Entry not found"}), 404
return jsonify({"success": True})
except Exception as e:
logger.error(f"[Quarantine] Error deleting {entry_id}: {e}")
return jsonify({"success": False, "error": str(e)}), 500
@app.route('/api/quarantine/<entry_id>/approve', methods=['POST'])
def approve_quarantine_item(entry_id):
"""One-click approve: restore the file and re-run post-process with the
matching per-check bypass flag set so the original quarantine trigger
is skipped. Other checks still run."""
try:
from core.imports.quarantine import approve_quarantine_entry
# Restore inside the soulseek download dir so existing path-resolution
# logic finds it. Unique subdir keeps it from re-mingling with active
# transfers.
restore_dir = os.path.join(
docker_resolve_path(config_manager.get('soulseek.download_path', './downloads')),
'Transfer',
)
result = approve_quarantine_entry(_get_quarantine_dir(), entry_id, restore_dir)
if result is None:
return jsonify({
"success": False,
"error": "Cannot one-click approve — entry has thin sidecar (no embedded context). Use 'Recover to Staging' instead.",
}), 400
restored_path, context, trigger = result
# Mark the bypass so the pipeline skips the trigger that fired.
context['_skip_quarantine_check'] = trigger
# Re-dispatch through the same pipeline. Run async so the HTTP
# request returns quickly — UI polls /list to see the entry vanish.
context_key = f"approve_{entry_id}_{int(time.time())}"
threading.Thread(
target=lambda: _post_process_matched_download(context_key, context, restored_path),
daemon=True,
).start()
logger.info(f"[Quarantine] Approved {entry_id} (bypass={trigger}) → re-running pipeline")
return jsonify({"success": True, "trigger_bypassed": trigger})
except Exception as e:
logger.error(f"[Quarantine] Error approving {entry_id}: {e}")
return jsonify({"success": False, "error": str(e)}), 500
@app.route('/api/quarantine/<entry_id>/recover', methods=['POST'])
def recover_quarantine_item(entry_id):
"""Fallback for legacy thin sidecars: move file into Staging so the user
can manually finish via the existing Import flow."""
try:
from core.imports.quarantine import recover_to_staging
from core.imports.staging import get_staging_path
target = recover_to_staging(_get_quarantine_dir(), get_staging_path(), entry_id)
if not target:
return jsonify({"success": False, "error": "Entry not found"}), 404
return jsonify({"success": True, "staged_path": target})
except Exception as e:
logger.error(f"[Quarantine] Error recovering {entry_id}: {e}")
return jsonify({"success": False, "error": str(e)}), 500
@app.route('/api/scan/request', methods=['POST'])
def request_media_scan():
"""
@ -15078,6 +15164,8 @@ def _build_retag_deps():
global retag_state
retag_state = value
from core.metadata.lyrics import generate_lrc_file as _generate_lrc_file
return _library_retag.RetagDeps(
config_manager=config_manager,
retag_lock=retag_lock,
@ -15092,6 +15180,7 @@ def _build_retag_deps():
_get_retag_state=_get_state,
_set_retag_state=_set_state,
get_database=_get_db,
generate_lrc_file=_generate_lrc_file,
)
@ -18900,6 +18989,10 @@ def get_server_playlist_tracks(playlist_id):
'image_url': img,
'duration_ms': t.get('duration_ms', 0),
'position': t.get('position', 0),
# Spotify track id — required for the user-confirmed
# match override lookup (sync_match_cache). Null for
# iTunes-only sources.
'source_track_id': t.get('source_track_id') or '',
})
elif playlist_name:
# Legacy fallback: cross-reference with sync history
@ -18939,6 +19032,21 @@ def get_server_playlist_tracks(playlist_id):
used_server_indices = set()
unmatched_source = [] # (index_in_combined, src_dict) for fuzzy second pass
# Pass 0: User-confirmed match overrides from sync_match_cache.
# When a user previously picked a local file via "Find & Add",
# the (source_track_id → server_track_id) mapping was persisted
# at confidence=1.0. Apply those FIRST so they bypass the
# exact/fuzzy passes entirely. Stale-cache safe — if the cached
# server track no longer exists, the override is silently
# skipped and normal matching runs.
from core.sync.match_overrides import resolve_match_overrides
_db_for_overrides = get_database()
_override_pairs = resolve_match_overrides(
source_tracks,
server_tracks,
lambda src_id: ((_db_for_overrides.read_sync_match_cache(src_id, active_server) or {}).get('server_track_id')),
)
# Pass 1: Exact title match (normalized — strips feat./ft. qualifiers)
for i, src in enumerate(source_tracks):
src_name = src.get('name', '')
@ -18953,6 +19061,19 @@ def get_server_playlist_tracks(playlist_id):
'duration_ms': src.get('duration_ms', 0), 'position': src.get('position', i),
}
# Override hit — paired by user, skip exact/fuzzy matching.
if i in _override_pairs:
j_override = _override_pairs[i]
used_server_indices.add(j_override)
combined.append({
'source_track': src_entry,
'server_track': server_tracks[j_override],
'match_status': 'matched',
'confidence': 1.0,
'override': True,
})
continue
src_norm = _norm_title(src_name)
best_idx = -1
for j, svr in enumerate(server_tracks):
@ -19125,14 +19246,48 @@ def server_playlist_replace_track(playlist_id):
return jsonify({"success": False, "error": str(e)}), 500
def _persist_find_and_add_match(source_track_id, server_source, server_track_id, server_track_title, source_title, source_artist):
"""Wrap match-override persistence with the active DB. No-op when
source_track_id is missing (e.g. add to a non-mirrored playlist)."""
if not source_track_id:
return
try:
from core.sync.match_overrides import record_manual_match
ok = record_manual_match(
get_database(),
source_track_id=source_track_id,
server_source=server_source,
server_track_id=server_track_id,
server_track_title=server_track_title,
source_title=source_title,
source_artist=source_artist,
)
if ok:
logger.info(f"[ServerPlaylist] Persisted Find & Add override: {source_track_id}{server_track_id} ({server_source})")
except Exception as e:
logger.warning(f"[ServerPlaylist] Failed to persist Find & Add override: {e}")
@app.route('/api/server/playlist/<playlist_id>/add-track', methods=['POST'])
def server_playlist_add_track(playlist_id):
"""Add a track to a server playlist at a specific position."""
"""Add a track to a server playlist at a specific position.
When the optional `source_track_id` is provided (the Spotify track id
from a mirrored playlist), the user's selection is also persisted to
sync_match_cache so future syncs auto-match this sourceserver pair
without requiring the user to re-trigger Find & Add.
"""
try:
data = request.get_json()
track_id = data.get('track_id')
playlist_name = data.get('playlist_name', '')
position = data.get('position') # 0-based index; None = append
# Optional Spotify source track id — when present, the (source →
# server) mapping is persisted as a hard match override.
source_track_id = data.get('source_track_id') or ''
source_title = data.get('source_title') or ''
source_artist = data.get('source_artist') or ''
server_track_title = data.get('server_track_title') or ''
if not track_id:
return jsonify({"success": False, "error": "track_id required"}), 400
@ -19185,6 +19340,7 @@ def server_playlist_add_track(playlist_id):
new_id = str(raw_playlist.ratingKey)
logger.info(f"[ServerPlaylist] Added track to playlist, playlist ID: {new_id}")
_persist_find_and_add_match(source_track_id, active_server, track_id, server_track_title or new_item.title, source_title, source_artist)
return jsonify({"success": True, "message": "Track added", "new_playlist_id": new_id})
elif active_server == 'jellyfin' and media_server_engine.client('jellyfin'):
@ -19194,6 +19350,7 @@ def server_playlist_add_track(playlist_id):
track_ids.insert(pos, track_id)
new_track_objs = [type('T', (), {'ratingKey': tid, 'title': ''})() for tid in track_ids]
media_server_engine.client('jellyfin').update_playlist(playlist_name, new_track_objs)
_persist_find_and_add_match(source_track_id, active_server, track_id, server_track_title, source_title, source_artist)
return jsonify({"success": True, "message": "Track added"})
elif active_server == 'navidrome' and media_server_engine.client('navidrome'):
@ -19203,6 +19360,7 @@ def server_playlist_add_track(playlist_id):
track_ids.insert(pos, track_id)
new_track_objs = [type('T', (), {'ratingKey': tid, 'title': ''})() for tid in track_ids]
media_server_engine.client('navidrome').create_playlist(playlist_name, new_track_objs, playlist_id=playlist_id)
_persist_find_and_add_match(source_track_id, active_server, track_id, server_track_title, source_title, source_artist)
return jsonify({"success": True, "message": "Track added"})
return jsonify({"success": False, "error": f"Unsupported server: {active_server}"}), 400
@ -34264,503 +34422,82 @@ def repair_job_progress():
# IMPORT / STAGING SYSTEM
# ================================================================================================
AUDIO_EXTENSIONS = {'.mp3', '.flac', '.ogg', '.opus', '.m4a', '.aac', '.wav', '.wma', '.aiff', '.aif', '.ape'}
def _build_import_route_runtime():
return _ImportRouteRuntime(
post_process_matched_download=_post_process_matched_download,
add_activity_item=add_activity_item,
automation_engine=automation_engine,
hydrabase_worker=hydrabase_worker,
dev_mode_enabled=dev_mode_enabled,
import_singles_executor=import_singles_executor,
build_album_import_match_payload=build_album_import_match_payload,
process_single_import_file=lambda runtime, file_info: _process_single_import_file(file_info),
logger=logger,
)
@app.route('/api/import/staging/files', methods=['GET'])
def import_staging_files():
"""Scan the staging folder and return audio files with tag metadata."""
try:
staging_path = get_staging_path()
os.makedirs(staging_path, exist_ok=True)
files = []
for root, _dirs, filenames in os.walk(staging_path):
for fname in filenames:
ext = os.path.splitext(fname)[1].lower()
if ext not in AUDIO_EXTENSIONS:
continue
full_path = os.path.join(root, fname)
rel_path = os.path.relpath(full_path, staging_path)
meta = read_staging_file_metadata(full_path, rel_path)
files.append({
'filename': fname,
'rel_path': rel_path,
'full_path': full_path,
'title': meta['title'],
'artist': meta['albumartist'] or meta['artist'] or 'Unknown Artist',
'album': meta['album'],
'track_number': meta['track_number'],
'disc_number': meta['disc_number'],
'extension': ext
})
# Sort by filename
files.sort(key=lambda f: f['filename'].lower())
return jsonify({'success': True, 'files': files, 'staging_path': staging_path})
except Exception as e:
logger.error(f"Error scanning staging files: {e}")
return jsonify({'success': False, 'error': str(e)}), 500
payload, status = _import_staging_files(_build_import_route_runtime())
return jsonify(payload), status
@app.route('/api/import/staging/groups', methods=['GET'])
def import_staging_groups():
"""Auto-detect album groups from staging files based on their tags.
Groups files by (album_tag, artist) where both are non-empty and at least 2 files share
the same album+artist combo. Returns groups sorted by file count descending.
"""
try:
staging_path = get_staging_path()
if not os.path.isdir(staging_path):
return jsonify({'success': True, 'groups': []})
# Scan files and group by album+artist tags
album_groups = {} # (album_lower, artist_lower) -> {album, artist, files: []}
for root, _dirs, filenames in os.walk(staging_path):
for fname in filenames:
ext = os.path.splitext(fname)[1].lower()
if ext not in AUDIO_EXTENSIONS:
continue
full_path = os.path.join(root, fname)
rel_path = os.path.relpath(full_path, staging_path)
meta = read_staging_file_metadata(full_path, rel_path)
album = meta['album']
artist = meta['albumartist'] or meta['artist']
if not album or not artist:
continue
key = (album.lower().strip(), artist.lower().strip())
if key not in album_groups:
album_groups[key] = {
'album': album.strip(),
'artist': artist.strip(),
'files': []
}
album_groups[key]['files'].append({
'filename': fname,
'full_path': full_path,
'title': meta['title'],
'track_number': meta['track_number'],
})
# Only return groups with 2+ files
groups = []
for group in album_groups.values():
if len(group['files']) >= 2:
group['files'].sort(key=lambda f: f.get('track_number') or 999)
groups.append({
'album': group['album'],
'artist': group['artist'],
'file_count': len(group['files']),
'files': group['files'],
'file_paths': [f['full_path'] for f in group['files']],
})
# Sort by file count descending
groups.sort(key=lambda g: g['file_count'], reverse=True)
return jsonify({'success': True, 'groups': groups})
except Exception as e:
logger.error(f"Error building staging groups: {e}")
return jsonify({'success': False, 'error': str(e)}), 500
payload, status = _import_staging_groups(_build_import_route_runtime())
return jsonify(payload), status
@app.route('/api/import/staging/hints', methods=['GET'])
def import_staging_hints():
"""Extract album search hints from staging folder (tags + folder names). Fast — no Spotify calls."""
try:
staging_path = get_staging_path()
if not os.path.isdir(staging_path):
return jsonify({'success': True, 'hints': []})
# Collect hints from tags and folder structure
tag_albums = {} # (album, artist) -> file count
folder_hints = {} # subfolder name -> file count
for root, _dirs, filenames in os.walk(staging_path):
audio_files = [f for f in filenames if os.path.splitext(f)[1].lower() in AUDIO_EXTENSIONS]
if not audio_files:
continue
# Folder-based hint: use immediate subfolder name relative to staging
rel_dir = os.path.relpath(root, staging_path)
if rel_dir != '.':
# Use the top-level subfolder as the hint
top_folder = rel_dir.split(os.sep)[0]
folder_hints[top_folder] = folder_hints.get(top_folder, 0) + len(audio_files)
# Tag-based hints
for fname in audio_files:
full_path = os.path.join(root, fname)
try:
from mutagen import File as MutagenFile
tags = MutagenFile(full_path, easy=True)
if tags:
album = (tags.get('album') or [None])[0]
artist = (tags.get('artist') or (tags.get('albumartist') or [None]))[0]
if album:
key = (album.strip(), (artist or '').strip())
tag_albums[key] = tag_albums.get(key, 0) + 1
except Exception as e:
logger.debug("tag read failed: %s", e)
# Build search queries, prioritizing tag-based hints (more specific)
queries = []
seen_queries_lower = set()
# Tag-based: sort by file count descending
for (album, artist), _count in sorted(tag_albums.items(), key=lambda x: -x[1]):
q = f"{album} {artist}".strip() if artist else album
if q.lower() not in seen_queries_lower:
seen_queries_lower.add(q.lower())
queries.append(q)
# Folder-based: parse "Artist - Album" pattern or use as-is
for folder, _count in sorted(folder_hints.items(), key=lambda x: -x[1]):
q = folder.replace('_', ' ')
if q.lower() not in seen_queries_lower:
seen_queries_lower.add(q.lower())
queries.append(q)
# Cap at 5 queries to keep it fast
queries = queries[:5]
return jsonify({'success': True, 'hints': queries})
except Exception as e:
logger.error(f"Error getting staging hints: {e}")
return jsonify({'success': False, 'error': str(e)}), 500
payload, status = _import_staging_hints(_build_import_route_runtime())
return jsonify(payload), status
@app.route('/api/import/search/albums', methods=['GET'])
def import_search_albums():
"""Search for albums using the active metadata provider."""
try:
query = request.args.get('q', '').strip()
if not query:
return jsonify({'success': False, 'error': 'Missing query parameter'}), 400
limit = min(int(request.args.get('limit', 12)), 50)
from core.metadata.registry import get_primary_source
if get_primary_source() == 'hydrabase' and hydrabase_worker and dev_mode_enabled:
hydrabase_worker.enqueue(query, 'albums')
albums = search_import_albums(query, limit=limit)
return jsonify({'success': True, 'albums': albums})
except Exception as e:
logger.error(f"Error searching albums for import: {e}")
return jsonify({'success': False, 'error': str(e)}), 500
payload, status = _import_search_albums(
_build_import_route_runtime(),
request.args.get('q', ''),
request.args.get('limit', 12),
)
return jsonify(payload), status
@app.route('/api/import/album/match', methods=['POST'])
def import_album_match():
"""Match staging files to an album's tracklist."""
try:
data = request.get_json() or {}
album_id = data.get('album_id')
album_name = data.get('album_name', '')
album_artist = data.get('album_artist', '')
source = str(data.get('source') or '').strip().lower()
# Optional: only match specific files (from auto-group selection)
filter_file_paths = set(data.get('file_paths', []))
if not album_id:
return jsonify({'success': False, 'error': 'Missing album_id'}), 400
# Without `source`, the lookup chain has to guess which metadata
# source the album_id came from — and a Deezer numeric id will
# match nothing in Spotify/iTunes/Discogs/etc., resulting in the
# failure-fallback dict that github issue #524 surfaced as
# "Unknown Artist / album_id-as-title / 0 tracks / 1991". Frontend
# fix in the same PR populates source on every match POST; this
# log catches anything that still reaches us without it (curl,
# third-party, regression in another caller).
if not source:
logger.warning(
"[Import Match] Missing 'source' on album_id=%s — lookup will "
"guess via primary-source priority chain. If this fires "
"consistently, a frontend caller is dropping source from "
"the match POST body.",
album_id,
)
payload = build_album_import_match_payload(
album_id,
album_name=album_name,
album_artist=album_artist,
file_paths=filter_file_paths,
source=source or None,
)
return jsonify(payload)
except Exception as e:
logger.error(f"Error matching album for import: {e}")
return jsonify({'success': False, 'error': str(e)}), 500
payload, status = _import_album_match(_build_import_route_runtime(), request.get_json() or {})
return jsonify(payload), status
@app.route('/api/import/album/process', methods=['POST'])
def import_album_process():
"""Process matched album files through the post-processing pipeline."""
try:
data = request.get_json() or {}
album = data.get('album', {})
matches = data.get('matches', [])
if not album or not matches:
return jsonify({'success': False, 'error': 'Missing album or matches data'}), 400
processed = 0
errors = []
album_name = album.get('name', album.get('album_name', 'Unknown Album'))
artist_name = album.get('artist', album.get('artist_name', 'Unknown Artist'))
album_id = album.get('id', album.get('album_id', ''))
source = str(album.get('source') or data.get('source') or '').strip().lower()
total_discs = max(
(
match.get('track', {}).get('disc_number', 1)
for match in matches
if match.get('track')
),
default=1,
)
artist_context = resolve_album_artist_context(album, source=source)
for match in matches:
staging_file = match.get('staging_file')
track = match.get('track') or {}
if not staging_file or not track:
continue
file_path = staging_file.get('full_path', '')
if not os.path.isfile(file_path):
errors.append(f"File not found: {staging_file.get('filename', '?')}")
continue
track_name = track.get('name', 'Unknown Track')
track_number = track.get('track_number', 1)
disc_number = track.get('disc_number', 1)
context_key = f"import_album_{album_id}_{track_number}_{uuid.uuid4().hex[:8]}"
context = build_album_import_context(
album,
track,
artist_context=artist_context,
total_discs=total_discs,
source=source,
)
try:
_post_process_matched_download(context_key, context, file_path)
processed += 1
logger.info(f"Import processed: {track_number}. {track_name} from {album_name}")
except Exception as proc_err:
err_msg = f"{track_name}: {str(proc_err)}"
errors.append(err_msg)
logger.error(f"Import processing error: {err_msg}")
add_activity_item("", "Album Imported", f"{album_name} by {artist_name} ({processed}/{len(matches)} tracks)", "Now")
# Emit events through automation engine — same chain as download batches
# batch_complete → auto-scan → library_scan_completed → auto-update DB
if processed > 0:
try:
if automation_engine:
automation_engine.emit('import_completed', {
'track_count': str(processed),
'album_name': album_name or '',
'artist': artist_name or '',
})
automation_engine.emit('batch_complete', {
'playlist_name': f"Import: {album_name}" if album_name else 'Import',
'total_tracks': str(len(matches)),
'completed_tracks': str(processed),
'failed_tracks': str(len(errors)),
})
except Exception as e:
logger.debug("album import automation emit failed: %s", e)
# Rebuild suggestions cache since staging contents changed
if processed > 0:
refresh_import_suggestions_cache()
return jsonify({
'success': True,
'processed': processed,
'total': len(matches),
'errors': errors
})
except Exception as e:
logger.error(f"Error processing album import: {e}")
return jsonify({'success': False, 'error': str(e)}), 500
payload, status = _import_album_process(_build_import_route_runtime(), request.get_json() or {})
return jsonify(payload), status
@app.route('/api/import/search/tracks', methods=['GET'])
def import_search_tracks():
"""Search tracks using the configured metadata provider priority order."""
try:
query = request.args.get('q', '').strip()
if not query:
return jsonify({'success': False, 'error': 'Missing query parameter'}), 400
limit = min(int(request.args.get('limit', 10)), 30)
if get_primary_source() == 'hydrabase' and hydrabase_worker and dev_mode_enabled:
hydrabase_worker.enqueue(query, 'tracks')
tracks = search_import_tracks(query, limit=limit)
return jsonify({'success': True, 'tracks': tracks})
except Exception as e:
logger.error(f"Error searching tracks for import: {e}")
return jsonify({'success': False, 'error': str(e)}), 500
payload, status = _import_search_tracks(
_build_import_route_runtime(),
request.args.get('q', ''),
request.args.get('limit', 10),
)
return jsonify(payload), status
def _process_single_import_file(file_info):
"""Worker function: validate, resolve metadata, post-process one file.
Returns ``("ok", title)`` on success, ``("error", message)`` on
failure, or ``("skip", reason)`` for files that need to be reported
but didn't actually run the pipeline. The caller aggregates these.
Designed to be safe to run concurrently from a ThreadPoolExecutor
each file gets its own UUID context_key, downstream DB writes
serialize via SQLite's busy_timeout, and file-system ops touch
distinct destination paths.
"""
file_path = file_info.get('full_path', '')
if not os.path.isfile(file_path):
return ("error", f"File not found: {file_info.get('filename', '?')}")
title = file_info.get('title', '')
artist = file_info.get('artist', '')
manual_match = file_info.get('manual_match')
if manual_match is not None and not isinstance(manual_match, dict):
manual_match = None
manual_match_source = ''
manual_match_id = None
if manual_match:
manual_match_source = str(manual_match.get('source') or '').strip().lower()
manual_match_id = str(manual_match.get('id') or '').strip()
if not manual_match_id or not manual_match_source:
return ("error", f"Malformed manual match for file: {file_info.get('filename', '?')}")
if not title and not manual_match:
parsed = parse_filename_metadata(file_info.get('filename', ''))
title = parsed.get('title') or os.path.splitext(file_info.get('filename', 'Unknown'))[0]
if not artist:
artist = parsed.get('artist', '')
from core.imports.resolution import get_single_track_import_context
try:
resolved = get_single_track_import_context(
title,
artist,
override_id=manual_match_id,
override_source=manual_match_source,
)
context = normalize_import_context(resolved['context'])
artist_data = get_import_context_artist(context)
track_data = get_import_track_info(context)
final_title = track_data.get('name', title)
final_artist = artist_data.get('name', artist)
context_key = f"import_single_{uuid.uuid4().hex[:8]}"
_post_process_matched_download(context_key, context, file_path)
logger.info(
"Import single processed: %s by %s (source=%s)",
final_title,
final_artist,
resolved.get('source') or 'local',
)
return ("ok", final_title)
except Exception as proc_err:
err_msg = f"{title}: {str(proc_err)}"
logger.error(f"Import single processing error: {err_msg}")
return ("error", err_msg)
return _import_process_single_import_file(_build_import_route_runtime(), file_info)
@app.route('/api/import/singles/process', methods=['POST'])
def import_singles_process():
"""Process individual staging files as singles through the post-processing pipeline.
Files are processed in parallel through the
``import_singles_executor`` (3 workers). Per-file work is dominated
by metadata search round-trips, so parallelizing gives a near-
linear speedup without saturating any one provider's rate limits.
"""
try:
data = request.get_json()
files = data.get('files', [])
if not files:
return jsonify({'success': False, 'error': 'No files provided'}), 400
processed = 0
errors = []
# Submit all files at once so the executor pulls 3 at a time.
# as_completed yields in finish order; we don't need ordering
# because the caller just wants a count + error list.
future_to_filename = {
import_singles_executor.submit(_process_single_import_file, file_info):
file_info.get('filename', '?')
for file_info in files
}
for future in as_completed(future_to_filename):
try:
outcome, payload = future.result()
except Exception as worker_err:
# Catch-all for anything the worker itself didn't catch
# (shouldn't happen — _process_single_import_file wraps
# its own pipeline call — but defensive).
errors.append(
f"{future_to_filename[future]}: worker crashed: {worker_err}"
)
continue
if outcome == "ok":
processed += 1
else:
errors.append(payload)
add_activity_item("", "Singles Imported", f"{processed}/{len(files)} tracks processed", "Now")
# Emit events through automation engine — same chain as download batches
# batch_complete → auto-scan → library_scan_completed → auto-update DB
if processed > 0:
try:
if automation_engine:
automation_engine.emit('import_completed', {
'track_count': str(processed),
'album_name': '',
'artist': 'Various',
})
automation_engine.emit('batch_complete', {
'playlist_name': 'Import: Singles',
'total_tracks': str(len(files)),
'completed_tracks': str(processed),
'failed_tracks': str(len(errors)),
})
except Exception as e:
logger.debug("singles import automation emit failed: %s", e)
# Rebuild suggestions cache since staging contents changed
if processed > 0:
refresh_import_suggestions_cache()
return jsonify({
'success': True,
'processed': processed,
'total': len(files),
'errors': errors
})
except Exception as e:
logger.error(f"Error processing singles import: {e}")
return jsonify({'success': False, 'error': str(e)}), 500
data = request.get_json() or {}
payload, status = _import_singles_process(_build_import_route_runtime(), data.get('files', []))
return jsonify(payload), status
# ── Auto-Import Worker ──
# Auto-Import Worker
auto_import_worker = None
try:
from core.auto_import_worker import AutoImportWorker
@ -34942,13 +34679,8 @@ def auto_import_clear_completed():
@app.route('/api/import/staging/suggestions', methods=['GET'])
def import_staging_suggestions():
"""Return cached import suggestions. If cache isn't built yet, returns partial/empty with a flag."""
cache = get_import_suggestions_cache()
return jsonify({
'success': True,
'suggestions': cache['suggestions'],
'ready': cache['built'],
})
payload, status = _import_staging_suggestions()
return jsonify(payload), status
# ================================================================================================

View file

@ -2200,6 +2200,7 @@
</div>
</div>
</div>
</div>
</div>
@ -2392,12 +2393,6 @@
<span class="enhance-icon"></span>
<span class="enhance-text">Enhance Quality</span>
</button>
<button class="library-artist-write-image-btn" id="library-artist-write-image-btn"
onclick="writeArtistImageToDisk()"
title="Write artist.jpg to the artist folder on disk (Navidrome reads this)">
<span class="write-image-icon">🖼️</span>
<span class="write-image-text">Write Artist Image</span>
</button>
</div>
<div class="artist-genres-container" id="artist-genres"></div>
<div class="artist-hero-bio" id="artist-hero-bio" style="display:none;"></div>
@ -5038,6 +5033,11 @@
</label>
<small class="settings-hint">Analyzes loudness and writes ReplayGain track gain/peak tags. Requires ffmpeg. Adds a few seconds per track.</small>
</div>
<div class="form-group">
<label for="duration-tolerance-seconds">Duration tolerance (seconds)</label>
<input type="number" id="duration-tolerance-seconds" min="0" max="60" step="0.5" value="0" style="width: 100px;">
<small class="settings-hint">Maximum drift between the file's actual length and the metadata source's expected length before the file is quarantined. <strong>0 = auto</strong> (3s normal, 5s for tracks &gt;10min). Raise this if tracks routinely quarantine for being a few seconds off (live recordings, alternate masterings, etc). Capped at 60s.</small>
</div>
</div>
<!-- Tag Embedding — per-tag toggles grouped by source -->
@ -7566,6 +7566,9 @@
<button class="library-history-tab" data-tab="import" onclick="switchHistoryTab('import')">
Server Imports <span class="library-history-tab-count" id="history-import-count">0</span>
</button>
<button class="library-history-tab" data-tab="quarantine" onclick="switchHistoryTab('quarantine')">
Quarantine <span class="library-history-tab-count" id="history-quarantine-count">0</span>
</button>
</div>
<div class="history-source-bar" id="history-source-bar" style="display:none"></div>
<div class="library-history-list" id="library-history-list"></div>

View file

@ -3382,7 +3382,18 @@ function processModalStatusUpdate(playlistId, data) {
case 'post_processing': statusText = '⌛ Processing...'; break;
case 'completed': statusText = '✅ Completed'; completedCount++; break;
case 'not_found': statusText = '🔇 Not Found'; notFoundCount++; break;
case 'failed': statusText = '❌ Failed'; failedOrCancelledCount++; break;
case 'failed': {
// Distinguish quarantine outcomes from generic
// failures — the file is recoverable, not lost.
const _em = (task.error_message || '').toLowerCase();
if (_em.includes('integrity check failed') || _em.includes('bit depth filter') || _em.includes('verification failed') || _em.includes('quarantin')) {
statusText = '🛡️ Quarantined';
} else {
statusText = '❌ Failed';
}
failedOrCancelledCount++;
break;
}
case 'cancelled': statusText = '🚫 Cancelled'; failedOrCancelledCount++; break;
default: statusText = `${task.status}`; break;
}

View file

@ -3416,6 +3416,16 @@ const WHATS_NEW = {
'2.5.2': [
// --- May 13, 2026 — 2.5.2 release ---
{ date: 'May 13, 2026 — 2.5.2 release' },
{ title: 'Retag No Longer Strips LYRICS Tag Without Rewriting', desc: 'discord report (netti93): retag tool was clearing the LYRICS / USLT tag and never rewriting it, while the download flow correctly embeds lyrics. asymmetry trace: download pipeline (`core/imports/pipeline.py`) calls `enhance_file_metadata` (clears all tags) then `generate_lrc_file` (writes .lrc sidecar + embeds USLT). retag (`core/library/retag.py`) only called the first half — `enhance_file_metadata` cleared USLT and there was no follow-up to restore it. fix 1: retag now calls `generate_lrc_file` after `enhance_file_metadata`, mirroring the download flow. injectable via `RetagDeps.generate_lrc_file` (optional default for backward compat). fix 2: `lyrics_client.create_lrc_file` used to short-circuit when an .lrc/.txt sidecar already existed (the typical retag case — sidecar moved alongside the audio). pre-fix: returned True without re-embedding USLT. post-fix: reads the existing sidecar and re-embeds the USLT tag. download flow unaffected (no sidecar at fetch time → original LRClib path runs). 7 boundary tests pin: existing .lrc triggers re-embed, existing .txt triggers re-embed, empty sidecar skips embed, unreadable sidecar swallows error, no sidecar falls through to LRClib (download path), `RetagDeps.generate_lrc_file` field accepted + optional for backward compat.', page: 'tools' },
{ title: 'Track Number Tag No Longer Writes "6/0" When Album Total Is Unknown', desc: 'discord report (netti93): downloaded album tracks were tagged with `TRCK = "6/0"` instead of `"6/13"` when source data lacked total_tracks. retag tool wrote correct `"6/13"` because `core/tag_writer.py` already handled the case. trace: `core/metadata/enrichment.py:105` formatted unconditionally as `f"{track_number}/{total_tracks}"` and many album-dict construction sites pass `total_tracks: 0` (per `types.py`, 0 means "unknown" — not a real count). that 0 propagated straight to disk. fix at the consumer boundary so every album-dict constructor stays unchanged: lifted to pure helper `core/metadata/track_number_format.py:format_track_number_tag` that drops the `/N` suffix when total is 0 / None / negative — emits just `"6"` instead. matches retag\'s behavior + ID3 spec convention (TRCK can be `"N"` or `"N/M"`). MP4 trkn tuple gets the same treatment via `format_track_number_tuple` returning `(6, 0)` per spec\'s "unknown total" marker. 16 boundary tests pin every shape: known total / zero total / none total / none track / zero track / negative inputs / string coercion / unparseable strings / floats truncate.', page: 'tools' },
{ title: 'AcoustID Scanner: Multi-Candidate Match + Duration Guard + Multi-Value Retag', desc: 'discord report (foxxify) issue #587: scanner produced false-positive "Wrong Song" findings for tracks where AcoustID returned multiple recordings per fingerprint and the top match was a wrong-credited recording (different MB entry sharing the same fingerprint). also: applying an AcoustID match retag stripped multi-value ARTISTS tags. three coordinated fixes per codex diagnosis. fix 1: scanner now iterates ALL AcoustID candidates (not just `recordings[0]`) — if any candidate matches expected title + artist, no finding. lifted to a shared pure helper `core/matching/acoustid_candidates.py:find_matching_recording` so both verifier and scanner use the same logic. fix 2: duration guard catches fingerprint hash collisions (foxxify\'s 17-minute mashup edit fingerprinted to a 5-minute late-70s japanese hiphop track — different songs, same fingerprint hash collision on a sampled section). when file duration vs AcoustID candidate duration differs by more than max(60s, 35%), the scanner skips the finding. fix 3: scanner retag (`_fix_wrong_song`) was bypassing the user\'s `metadata_enhancement.tags.write_multi_artist` setting because `write_tags_to_file` only wrote single-string TPE1. now extended with optional `artists_list` parameter that, when supplied + setting on, writes the multi-value tag (TXXX:Artists for ID3, `artists` key for vorbis/opus/flac, list-form `\xa9ART` for mp4) — exact same behavior as the post-download enrichment pipeline. AcoustID retag derives the per-artist list by splitting AcoustID\'s credit on the same separators (comma / ampersand / feat. / ft. / etc) the matching layer already uses. AcoustID version-mismatch gate left intact (still correctly catches genuinely-wrong files). 15 tests on the candidate helper + duration guard, 13 tests on the multi-value tag write path, 4 new scanner regression tests pinning every shape: lower-ranked candidate match suppression, no-suppression when no candidate matches, duration mismatch skip, no-skip when duration matches.', page: 'tools' },
{ title: 'Cross-Script Artist Aliases: Cyrillic / Kanji Canonical Names Now Bridge', desc: 'github issue #586 (follow-up to #442): "Dmitry Yablonsky" tracks were quarantining as audio mismatch — file identified as "Русская филармония, Дмитрий Яблонский" (4% artist similarity) — even though the Cyrillic spelling is just the russian transliteration. three layered bugs in the alias resolution chain. fix 1: `fetch_artist_aliases` only read `data.aliases` and IGNORED the artist record\'s canonical `name` and `sort-name`. for artists where the canonical form is the cross-script spelling (e.g. MB stores "Дмитрий Яблонский" as canonical, "Dmitry Yablonsky" as alias — or vice versa), the missing direction never made it into the alias list. now both canonical name + sort-name are included alongside the explicit aliases (deduped). fix 2: `lookup_artist_aliases` ran search in strict mode only (`artist:"..."` lucene query), which skips MB\'s alias and sortname indexes. cross-script searches found nothing under strict. now falls back to non-strict (bare query, hits all indexes) when strict returns empty OR all results fail the trust gate. fix 3: trust gate weighted local similarity 70% — cross-script pairs have similarity ~0 → combined score ~0.30 → below the 0.85 threshold → cached as empty even when MB\'s own confidence was 100. new escape: when MB score is ≥ 95 AND the result is unambiguous (top result clearly leads), accept regardless of local similarity. covers cases where MB definitely knows the right artist but our local sim collapses to zero. existing #442 same-script path (Hiroyuki Sawano ↔ 澤野弘之) still passes via combined-score path. 12 new tests pin every layer + the exact reporter scenario end-to-end via `artist_names_match`. existing alias tests updated to reflect canonical-name inclusion + 2-call strict+non-strict pattern.', page: 'downloads' },
{ title: 'MTV Unplugged & Live Albums No Longer False-Quarantine', desc: 'github issue #589: tracks from live / unplugged / concert albums (MTV Unplugged, Live At Wembley, etc) consistently failed AcoustID verification with "Version mismatch: expected (live) but file is (original)". two upstream bugs fed into the false positive — the AcoustID gate itself was correctly catching the wrong file Tidal had selected. fix 1: album-scoped library check at `core/downloads/master.py` was scoring "Shy Away (MTV Unplugged Live)" (source) vs "Shy Away" (local DB) with raw string similarity → ~0.3 → marked missing → re-downloaded even though user already owned it. new pure helper `core/matching/album_context_title.py:strip_redundant_album_suffix` strips suffixes whose tokens are fully subsumed by the album context (live/unplugged/acoustic/session markers + tolerated noise + album-title words). only fires inside the album-confirmed scope so global matching elsewhere is unchanged. fix 2: `core/tidal_download_client.py` qualifier filter only ran on FALLBACK searches — primary search returned all results unfiltered, so a query for "Shy Away (MTV Unplugged Live)" could accept the studio cut if Tidal ranked it first. now applies to both primary and fallback. fix 3: qualifier check now inspects both `track.name` AND `track.album.name` — for concert / unplugged releases the live signal often lives in the album title, not the track. AcoustID version-mismatch gate left intact (still correctly catches genuinely-wrong files). 19 tests on the album-context helper + 13 tests on the tidal qualifier helper pin every shape: MTV Unplugged variants, dash-style suffixes, brackets, year tolerance, plural-form markers, anti-regression cases (instrumental/remix on a studio album must NOT be stripped), defensive non-dict / missing-album inputs.', page: 'downloads' },
{ title: 'Deezer: Contributing Artist Tagging Now Consistent', desc: 'github issue #588: contributors tagging worked for some tracks but silently dropped them for others — most reproducibly for tracks whose ALBUM was fetched before the per-track post-process ran. trace: `core/deezer_client.py:get_track_details` cache check used `track_position` as the "full payload" sentinel, but BOTH `/track/<id>` AND `/album/<id>/tracks` set that field. only `/track/<id>` sets the `contributors` array. when album-tracks data hit the cache first, `get_track_details` returned the partial record → `_build_enhanced_track` found no contributors → the metadata-source contributors-upgrade silently fell back to single-artist. fix: lifted cache-validity to a pure helper `_is_full_track_payload` that requires BOTH `track_position` AND `contributors` key presence (empty list `[]` is valid — single-artist tracks fetched via `/track/<id>` carry it explicitly). partial cache hits now fall through to a fresh `/track/<id>` fetch. 11 boundary tests pin every shape: full payload, single-artist with empty contributors list, partial album-tracks shape, search-result shape, none/non-dict, cache-hit/cache-miss/api-failure paths.', page: 'downloads' },
{ title: 'Server Playlists: Find & Add Now Persists As A Permanent Match', desc: 'github issue #585: when a spotify track name had a versioned suffix not present in the local file (e.g. "Iron Man - 2012 - Remaster" vs "Iron Man") the auto-matcher missed the pair. user could click Find & Add to manually pick the right local file — that worked, file got added to the plex playlist — but the source spotify track stayed in Missing while the added file showed up under Extra, because the matcher had no record of the user-confirmed pairing. on the next sync the source track would re-quarantine and try to download all over again. fix: every Find & Add selection now writes a `(spotify_track_id → server_track_id)` override into `sync_match_cache` at confidence=1.0. the matching algorithm runs an override pass BEFORE the existing exact and fuzzy passes, so any user-confirmed pair short-circuits straight to "matched" without going through normalization at all. covers every kind of mismatch — dash-suffix remasters, covers / karaoke versions, alt masters, cross-language titles, typo\'d local files, anything. logic lifted to `core/sync/match_overrides.py` (pure helpers `resolve_match_overrides` + `record_manual_match`). 18 boundary tests pin: cache-hit pairs, cache-miss falls through, stale-cache (server track removed) handled gracefully, two sources pointing at same server track (UNIQUE-violation defense), str/int id coercion, partial cache hits, defensive against non-dict inputs and DB exceptions. legacy entries without `source_track_id` (non-mirrored playlists) just skip the override path. works across plex / jellyfin / navidrome.', page: 'sync' },
{ title: 'Quarantine Management — See, Approve, Delete Files Without Touching The Filesystem', desc: 'github issue #584: quarantined files used to just sit in `ss_quarantine/` with a thin sidecar — no UI, no recovery, no way to see what got dropped or why. new **Quarantine** tab on the existing Library History modal (downloads page → Download History button) lists every quarantined file with the same row chrome as the Downloads + Server Imports tabs: thumb placeholder, expected track + artist, original filename, trigger badge (Duration / AcoustID / Bit Depth), relative time, expandable details panel showing the full failure reason. three per-row actions: **Approve** (restores the file, re-runs post-processing with ONLY the failing check skipped, lands in your library with full tags + lyrics + scan), **Recover** (legacy fallback for entries quarantined before this PR with thin sidecars — moves to Staging so you finish via Import flow), **Delete** (permanent removal of file + sidecar). all three use the themed soulsync confirm modal + toast feedback (no native browser alert / confirm). per-check bypass means approving a duration-mismatch file still runs AcoustID; approving an AcoustID failure still runs bit-depth — other quality gates stay live so you can only override one trigger at a time. files that fail a different check after approval get re-quarantined with the new trigger label so you can decide again. sidecar now persists the full json-safe context so approve has everything the pipeline needs to re-process. download modal status differentiates "🛡️ Quarantined" from "❌ Failed" so recoverable files are visible at a glance. logic lifted to pure helpers in `core/imports/quarantine.py` (list / delete / approve / recover_to_staging / serialize_quarantine_context) with 27 boundary tests covering orphan files / orphan sidecars / corrupt sidecars / collision-safe filename restoration / full-context vs thin-sidecar dispatch / json round-trip safety. four new endpoints. pipeline change is per-check conditionals at the existing quarantine sites — no blanket skip-all flag.', page: 'downloads' },
{ title: 'Configurable Duration Tolerance For Quarantined Tracks', desc: 'discord question: tracks were quarantining when their actual length drifted by a few seconds from what spotify/musicbrainz reported (3s tolerance hardcoded, 5s for tracks >10min). live recordings, alternate masterings, and some legitimate uploads routinely drift more than that. new setting on settings → metadata → post-processing: "duration tolerance (seconds)". `0 = auto` (preserves the existing 3s/5s defaults). raise it to 10 / 15 / 20 if your library has a lot of drift-prone material. capped at 60s — past that the check is effectively off. applies to ALL matched downloads (soulseek / tidal / qobuz / hifi / youtube / deezer-direct) since they all flow through the same post-process integrity check. logic lifted to a pure helper `core/imports/file_integrity.py:resolve_duration_tolerance` that coerces the config value (none / empty / 0 / negative / unparseable / above-cap) to either a float override or `None` for the auto-scaled default. 12 tests pin every input shape.', page: 'settings' },
{ title: 'Soulseek Downloads: Multi-Artist Tags Now Get Written Properly', desc: 'discord report: tracks downloaded via soulseek were getting tagged with primary artist only (no collab artists), while the same track downloaded via deezer tagged everyone correctly. trace: the soulseek matched-download context constructed `original_search_result` with `artist` (singular string) but no `artists` (list), even though the full multi-artist list lived on `track_info` (the matched spotify track object). `core/metadata/source.py:extract_source_metadata` only read `original_search.artists`, so soulseek path always fell through to the single-artist branch. fix: lifted artist resolution into a pure helper `core/metadata/artist_resolution.py:resolve_track_artists` that walks `original_search.artists` → `track_info.artists` → `artist_dict.name` fallback chain. handles all three list-item shapes (spotify-style dicts, bare strings, anything else stringified). 13 tests pin the resolution order, fallback chain, mixed-shape normalization, whitespace stripping, empty/none handling. composes with the existing deezer per-track upgrade (still fires when single-artist + track_id available) and feat_in_title / artist_separator settings (still drive the joined ARTIST string downstream).', page: 'downloads' },
{ title: 'Download Missing Modal: Tracklist Got A Polish Pass', desc: 'visual tune-up only — column layout untouched. hairline row dividers, accent gradient + edge bar on hover, monospace track numbers (glow accent on row hover), monospace tabular duration. status text in both library-match + download-status columns picks up a leading colored dot with a soft halo (green found / amber missing / blue checking / orange downloading / red failed) and pulses while in-flight. artist column centered. soft scrollbar.', page: 'downloads' },
{ title: 'Search Source Picker: Fix Default Always Sticking To Spotify', desc: 'enhanced search + global search source picker always defaulted to spotify even when the user\'s primary metadata source was deezer / itunes / discogs / etc. trace: `shared-helpers.js:createSearchController` reads `/status.metadata_source` to pick the initial active icon, then checks `SOURCE_LABELS[src]` to validate. backend was returning `metadata_source` as a dict (`{source, connected, response_time, ...}` — used elsewhere for connection-state display), so `SOURCE_LABELS[<dict>]` was always undefined, the `if` guard never fired, and `state.activeSource` silently stayed at the hardcoded `\'spotify\'` default. fix: read `.source` off the dict (with forward-compat fallback to plain-string in case any older /status response shape predates the dict change). other consumers (core.js sidebar tile, helper.js status checker, search.js display) already used `?.source` correctly — this was the only stale call site.', page: 'search' },
{ title: 'Download Discography: No Longer Caps Prolific Artists At 50 Releases', desc: 'discord report: clicking "download discography" on an artist with a deep catalogue (bach, beatles complete box, dance / electronic artists with hundreds of remixes) only showed ~50 albums in the modal. trace: `MetadataLookupOptions(limit=50, max_pages=0)` was hardcoded at the discography endpoint and the artist-detail discography view. spotify\'s `max_pages=0` already paginates through everything (per-page is clamped to 10 internally) so spotify-primary users were unaffected. but deezer / itunes / discogs / hydrabase all honor the outer `limit` as a hard cap. fix: bump `limit` from 50 to 200 at all three call sites (`web_server.py` discography endpoint + artist-detail view + `core/artist_source_detail.py`). 200 matches iTunes\'s and Discogs\'s own internal caps and covers near-everyone\'s full catalogue. spotify behavior unchanged.', page: 'library' },
@ -4339,7 +4349,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.2';
return versions[0] || '2.5.3';
}
function openWhatsNew() {

View file

@ -1821,6 +1821,11 @@ async function _serverSelectTrack(trackIndex, mode, newTrackId, el) {
for (let k = 0; k < trackIndex; k++) {
if (_serverEditorState.tracks[k]?.server_track) serverPos++;
}
// source_track carries source_track_id (Spotify ID) when this
// came from a mirrored playlist — the backend uses it to
// persist the Find & Add selection as a permanent match
// override so future syncs auto-pair without user action.
const srcTrack = track.source_track || {};
response = await fetch(`/api/server/playlist/${_serverEditorState.playlistId}/add-track`, {
method: 'POST',
headers: { 'Content-Type': 'application/json' },
@ -1828,6 +1833,9 @@ async function _serverSelectTrack(trackIndex, mode, newTrackId, el) {
track_id: newTrackId,
playlist_name: _serverEditorState.playlistName,
position: serverPos,
source_track_id: srcTrack.source_track_id || '',
source_title: srcTrack.name || '',
source_artist: srcTrack.artist || '',
})
});
}

View file

@ -970,6 +970,7 @@ async function loadSettingsData() {
document.getElementById('prefer-caa-art').checked = settings.metadata_enhancement?.prefer_caa_art === true;
document.getElementById('lrclib-enabled').checked = settings.metadata_enhancement?.lrclib_enabled !== false;
document.getElementById('replaygain-enabled').checked = settings.post_processing?.replaygain_enabled === true;
document.getElementById('duration-tolerance-seconds').value = settings.post_processing?.duration_tolerance_seconds ?? 0;
// Load service master toggles
document.getElementById('embed-spotify').checked = settings.spotify?.embed_tags !== false;
document.getElementById('embed-itunes').checked = settings.itunes?.embed_tags !== false;
@ -2747,6 +2748,7 @@ async function saveSettings(quiet = false) {
},
post_processing: {
replaygain_enabled: document.getElementById('replaygain-enabled').checked,
duration_tolerance_seconds: parseFloat(document.getElementById('duration-tolerance-seconds').value) || 0,
},
library: {
music_paths: collectMusicPaths(),

View file

@ -3094,6 +3094,150 @@ function openLibraryHistoryModal() {
}
}
// ──────────────────────────────────────────────────────────────────────
// Quarantine tab — rendered inside the Library History modal as a third
// tab next to Downloads + Server Imports. Reuses the existing list +
// pagination chrome; provides per-row Approve / Recover / Delete actions.
// ──────────────────────────────────────────────────────────────────────
async function loadQuarantineList() {
const list = document.getElementById('library-history-list');
const pagination = document.getElementById('library-history-pagination');
const sourceBar = document.getElementById('history-source-bar');
if (!list) return;
list.innerHTML = '<div class="library-history-loading">Loading...</div>';
if (pagination) pagination.innerHTML = '';
if (sourceBar) sourceBar.style.display = 'none';
try {
const resp = await fetch('/api/quarantine/list');
const data = await resp.json();
const entries = data.entries || [];
const countEl = document.getElementById('history-quarantine-count');
if (countEl) countEl.textContent = entries.length;
if (!data.success) {
list.innerHTML = `<div class="library-history-empty">Error: ${escapeHtml(data.error || 'Failed to load')}</div>`;
return;
}
if (entries.length === 0) {
list.innerHTML = '<div class="library-history-empty">🛡️<br><br>No quarantined files. Nice and clean.</div>';
return;
}
list.innerHTML = entries.map(renderQuarantineEntry).join('');
} catch (err) {
console.error('Error loading quarantine entries:', err);
list.innerHTML = '<div class="library-history-empty">Error loading quarantine</div>';
}
}
function renderQuarantineEntry(entry) {
const triggerLabels = { integrity: 'Duration / Integrity', acoustid: 'AcoustID Mismatch', bit_depth: 'Bit Depth Filter', unknown: 'Unknown' };
const triggerColors = { integrity: '#facc15', acoustid: '#ef5350', bit_depth: '#fb923c', unknown: '#888' };
const triggerLabel = triggerLabels[entry.trigger] || entry.trigger || 'Unknown';
const triggerColor = triggerColors[entry.trigger] || '#888';
const id = escapeHtml(entry.id);
const approveLabel = entry.has_full_context ? 'Approve' : 'Recover';
const approveTitle = entry.has_full_context
? 'Re-run post-processing with only the failing check skipped'
: 'Legacy entry — move to Staging, finish via Import flow';
const approveCall = entry.has_full_context
? `approveQuarantineEntry('${id}')`
: `recoverQuarantineEntry('${id}')`;
const meta = [entry.expected_artist, entry.original_filename].filter(Boolean).join(' — ');
const triggerBadge = `<span class="library-history-badge" style="border-color:${triggerColor};color:${triggerColor}">${escapeHtml(triggerLabel)}</span>`;
const reasonDetail = `<div class="library-history-entry-source"><span class="lh-prov-label">Reason:</span> ${escapeHtml(entry.reason || 'Unknown')}</div>`;
return `<div class="library-history-entry lh-expandable" onclick="this.classList.toggle('lh-expanded')">
<div class="library-history-thumb-placeholder">🛡</div>
<div class="library-history-entry-content">
<div class="library-history-entry-row1">
<div class="library-history-entry-text">
<div class="library-history-entry-title">${escapeHtml(entry.expected_track || entry.original_filename || 'Unknown')}</div>
<div class="library-history-entry-meta">${escapeHtml(meta)}</div>
</div>
<div class="library-history-entry-badges">${triggerBadge}</div>
<div class="library-history-entry-time">${formatHistoryTime(entry.timestamp)}</div>
<button class="lh-audit-btn" title="${approveTitle}" onclick="event.stopPropagation();${approveCall}">${approveLabel}</button>
<button class="lh-audit-btn" title="Delete permanently" style="border-color:rgba(248,113,113,0.4);color:#f87171" onclick="event.stopPropagation();deleteQuarantineEntry('${id}')">Delete</button>
<span class="lh-expand-btn">&#x25BE;</span>
</div>
<div class="library-history-entry-details">
${reasonDetail}
</div>
</div>
</div>`;
}
async function approveQuarantineEntry(entryId) {
const ok = await showConfirmDialog({
title: 'Approve Quarantined File',
message: 'Re-run post-processing for this file with only the failing check skipped. The file will be tagged, lyrics generated, and moved into your library. Other quality gates (AcoustID + bit-depth) still run.',
confirmText: 'Approve & Import',
cancelText: 'Cancel',
});
if (!ok) return;
try {
const r = await fetch(`/api/quarantine/${encodeURIComponent(entryId)}/approve`, { method: 'POST' });
const data = await r.json();
if (!data.success) {
showToast(`Approve failed: ${data.error}`, 'error');
} else {
showToast(`Approved — skipped ${data.trigger_bypassed} check, re-running pipeline.`, 'success');
}
} catch (err) {
showToast(`Approve failed: ${err.message}`, 'error');
}
loadQuarantineList();
}
async function recoverQuarantineEntry(entryId) {
const ok = await showConfirmDialog({
title: 'Recover To Staging',
message: 'Legacy entry — no embedded context. The file will be moved to your Staging folder so you can finish via the Import page (manual match).',
confirmText: 'Move To Staging',
cancelText: 'Cancel',
});
if (!ok) return;
try {
const r = await fetch(`/api/quarantine/${encodeURIComponent(entryId)}/recover`, { method: 'POST' });
const data = await r.json();
if (!data.success) {
showToast(`Recover failed: ${data.error}`, 'error');
} else {
showToast('Moved to Staging — finish via the Import page.', 'success');
}
} catch (err) {
showToast(`Recover failed: ${err.message}`, 'error');
}
loadQuarantineList();
}
async function deleteQuarantineEntry(entryId) {
const ok = await showConfirmDialog({
title: 'Delete Quarantined File',
message: 'This permanently removes the file and its metadata sidecar. Cannot be undone.',
confirmText: 'Delete',
cancelText: 'Cancel',
destructive: true,
});
if (!ok) return;
try {
const r = await fetch(`/api/quarantine/${encodeURIComponent(entryId)}`, { method: 'DELETE' });
const data = await r.json();
if (!data.success) {
showToast(`Delete failed: ${data.error}`, 'error');
} else {
showToast('Quarantined file deleted.', 'success');
}
} catch (err) {
showToast(`Delete failed: ${err.message}`, 'error');
}
loadQuarantineList();
}
function closeLibraryHistoryModal() {
const overlay = document.getElementById('library-history-overlay');
if (overlay) overlay.classList.add('hidden');
@ -3110,6 +3254,12 @@ function switchHistoryTab(tab) {
async function loadLibraryHistory() {
const { tab, page, limit } = _libraryHistoryState;
if (tab === 'quarantine') {
// Refresh the count for the other two tabs in the background so
// the badge stays accurate when the user switches over.
loadQuarantineList();
return;
}
const list = document.getElementById('library-history-list');
const pagination = document.getElementById('library-history-pagination');
if (!list) return;