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

Merge pull request #536 from Nezreka/fix/manual-import-broken-issue-524

Browse source 
Auto-Import Overhaul: Manual Import Fix + Multi-Disc + Picard Parity + Bounded Pool + SoulSync Standalone Server-Quality Writes
This commit is contained in:
BoulderBadgeDad 2026-05-10 00:07:03 -07:00 committed by GitHub
commit e7e32652f5
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
18 changed files with 5328 additions and 434 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

1186
core/auto_import_worker.py
View file

File diff suppressed because it is too large Load diff

588
core/imports/album_matching.py Normal file
View file

@ -0,0 +1,588 @@
"""Album-track matching helpers — lifted out of
``AutoImportWorker._match_tracks`` so the matching logic is testable in
isolation without instantiating the worker, mocking the metadata
client, or monkey-patching ``_read_file_tags``.
Diagnostic logging:
- Every match decision (matched, rejected by duration, rejected by
threshold) emits a debug-level log when ``ALBUM_MATCHING_DEBUG`` is
truthy. Defaults to off so production logs stay clean. Flip via the
``SOULSYNC_ALBUM_MATCHING_DEBUG`` env var when investigating
"nothing matched" reports.
The worker still owns:
- File-system traversal + tag reads
- Metadata client lookup + album_data fetch
- Album-vs-single routing
This module owns:
- Quality-aware deduplication keyed on the ``(disc_number, track_number)``
position tuple
- Weighted match scoring against the album's tracklist
- Returning the list of (track, file, confidence) matches + leftover
unmatched files
Both behaviors are pure functions over already-fetched data, so the
test surface is just dicts in / dicts out.
"""
from __future__ import annotations
import os
from typing import Any, Callable, Dict, List, Set, Tuple
# Use the project's namespaced logger so diagnostic lines actually
# land in app.log. `logging.getLogger(__name__)` would resolve to
# `core.imports.album_matching` which sits OUTSIDE the `soulsync.*`
# tree the file handler watches, making every "no matches" diagnostic
# silently invisible to anyone debugging an import problem.
from utils.logging_config import get_logger
logger = get_logger("imports.album_matching")
# ---------------------------------------------------------------------------
# Match-scoring weights
# ---------------------------------------------------------------------------
# Each weight is a fraction of the 0..1 confidence score the matcher
# accumulates per (file, track) pair. Sum of all maximum-bonus paths
# equals 1.0 in the happy case (perfect title + artist + position +
# album tag agreement).
#
# History note: the position bonus (30%) used to fire on track_number
# alone, which broke multi-disc albums where every disc has tracks 1..N.
# Disc-aware split (POSITION + CROSS_DISC) shipped 2026-05-09 after
# user reported Mr. Morale & The Big Steppers losing half its tracks
# during auto-import.
TITLE_WEIGHT = 0.45 # case-folded fuzzy title similarity
ARTIST_WEIGHT = 0.15 # albumartist (or artist) similarity
POSITION_WEIGHT = 0.30 # exact (disc_number, track_number) match
NEAR_POSITION_WEIGHT = 0.12 # off-by-one track number, same disc
CROSS_DISC_POSITION_WEIGHT = 0.05 # same track_number, different disc
ALBUM_WEIGHT = 0.10 # album tag similarity to target album
# A file scoring below this threshold against every track is treated
# as unmatched. Threshold sits below the per-component partial-match
# floor (~0.5 × 0.45 = 0.22) plus a small position consolation, so
# files with weak title agreement still need at least one strong signal.
MATCH_THRESHOLD = 0.4
SimilarityFn = Callable[[str, str], float]
QualityRankFn = Callable[[str], int]
def dedupe_files_by_position(
audio_files: List[str],
file_tags: Dict[str, Dict[str, Any]],
*,
quality_rank: QualityRankFn,
) -> List[str]:
"""Drop quality-duplicate files at the same ``(disc, track)``
position, keeping the higher-quality one.
The position key is ``(disc_number, track_number)`` — NOT
``track_number`` alone. Multi-disc albums where every disc has
tracks 1..N would otherwise collapse to one disc's worth of files
here, before the matcher even sees the rest.
Files with ``track_number == 0`` (no tag) all pass through —
can't dedupe positions we don't know.
"""
seen_positions: Dict[Tuple[int, int], str] = {}
deduped: List[str] = []
for f in audio_files:
tags = file_tags.get(f, {})
track_num = tags.get('track_number', 0) or 0
disc_num = tags.get('disc_number', 1) or 1
ext = os.path.splitext(f)[1].lower()
position_key = (disc_num, track_num)
if track_num > 0 and position_key in seen_positions:
prev_f = seen_positions[position_key]
prev_ext = os.path.splitext(prev_f)[1].lower()
if quality_rank(ext) > quality_rank(prev_ext):
deduped.remove(prev_f)
deduped.append(f)
seen_positions[position_key] = f
else:
deduped.append(f)
if track_num > 0:
seen_positions[position_key] = f
return deduped
def _extract_track_disc(track: Dict[str, Any]) -> int:
"""Pull disc number off an API track dict.
Different metadata sources spell the field differently:
Spotify ``disc_number``, Deezer ``disk_number``, iTunes
``discNumber``. Default to 1 when missing so single-disc albums
still match.
"""
return (
track.get('disc_number')
or track.get('disk_number')
or track.get('discNumber')
or 1
)
def _extract_track_artist(track: Dict[str, Any]) -> str:
artists = track.get('artists') or []
if not artists:
return ''
a = artists[0]
return a.get('name', str(a)) if isinstance(a, dict) else str(a)
def score_file_against_track(
file_path: str,
file_tags: Dict[str, Any],
track: Dict[str, Any],
*,
target_album: str,
similarity: SimilarityFn,
) -> float:
"""Compute the 0..1 confidence score for matching ``file_path``
(with its tags) to ``track`` (an API track dict).
Pure scoring — caller decides what to do with the score (compare
against ``MATCH_THRESHOLD``, pick best-per-track, etc).
"""
score = 0.0
# Title similarity (TITLE_WEIGHT). Falls back to filename stem when
# the file has no title tag.
title = file_tags.get('title') or os.path.splitext(os.path.basename(file_path))[0]
track_name = track.get('name', '')
score += similarity(title, track_name) * TITLE_WEIGHT
# Artist similarity (ARTIST_WEIGHT). Skipped if either side missing.
file_artist = file_tags.get('artist', '')
track_artist = _extract_track_artist(track)
if file_artist and track_artist:
score += similarity(file_artist, track_artist) * ARTIST_WEIGHT
# Position match (POSITION_WEIGHT / NEAR_POSITION_WEIGHT /
# CROSS_DISC_POSITION_WEIGHT). Gates on the (disc, track) tuple
# rather than track_number alone — see the module docstring's
# multi-disc history note.
file_track_num = file_tags.get('track_number', 0) or 0
track_num = track.get('track_number', 0) or 0
if file_track_num > 0 and track_num > 0:
file_disc = file_tags.get('disc_number', 1) or 1
track_disc = _extract_track_disc(track)
if file_track_num == track_num and file_disc == track_disc:
score += POSITION_WEIGHT
elif file_track_num == track_num and file_disc != track_disc:
# Same track number, different disc — small consolation so
# title/artist similarity has to carry the match. Common
# collision in deluxe / multi-disc releases where every
# disc has tracks numbered 1..N.
score += CROSS_DISC_POSITION_WEIGHT
elif abs(file_track_num - track_num) <= 1 and file_disc == track_disc:
score += NEAR_POSITION_WEIGHT
# Album tag bonus (ALBUM_WEIGHT). Helps disambiguate when the
# target_album name is a strong signal.
file_album = file_tags.get('album', '')
if file_album:
score += similarity(file_album, target_album) * ALBUM_WEIGHT
return score
# ---------------------------------------------------------------------------
# Exact-identifier fast paths
# ---------------------------------------------------------------------------
# Tagged libraries (especially Picard / Beets) carry per-recording IDs
# that uniquely identify the track regardless of title spelling, album
# context, or duration drift. When both the file tag AND the metadata
# source's track entry carry the same identifier, no fuzzy matching is
# needed — exact match wins, full confidence, no further scoring.
#
# Order: MBID first (MusicBrainz Recording ID — primary Picard tag),
# then ISRC (International Standard Recording Code — many sources).
# An ISRC can be shared across remasters / region releases of the same
# recording, so MBID is preferred when both are present.
EXACT_MATCH_CONFIDENCE = 1.0
def _track_identifier(track: Dict[str, Any], key: str) -> str:
"""Pull a normalized identifier off a metadata-source track dict.
Different sources spell ISRC differently — Spotify exposes it on
``external_ids.isrc``; iTunes uses ``isrc`` directly when present.
MBID lives at ``external_ids.mbid`` for some sources, top-level
``musicbrainz_id`` / ``mbid`` for others.
"""
if key == 'isrc':
# ISRC normalization: uppercase, strip dashes/spaces. Picard writes
# tags as "USRC1234567" but some sources return "US-RC-12-34567".
for candidate in (
track.get('isrc'),
(track.get('external_ids') or {}).get('isrc'),
):
if candidate:
return str(candidate).upper().replace('-', '').replace(' ', '').strip()
return ''
if key == 'mbid':
for candidate in (
track.get('musicbrainz_id'),
track.get('mbid'),
(track.get('external_ids') or {}).get('mbid'),
(track.get('external_ids') or {}).get('musicbrainz'),
):
if candidate:
return str(candidate).lower().strip()
return ''
return ''
def _file_identifier(file_tags: Dict[str, Any], key: str) -> str:
"""Pull a normalized identifier off the file's tag dict."""
if key == 'isrc':
raw = file_tags.get('isrc') or ''
return str(raw).upper().replace('-', '').replace(' ', '').strip()
if key == 'mbid':
return str(file_tags.get('mbid') or '').lower().strip()
return ''
def find_exact_id_matches(
audio_files: List[str],
file_tags: Dict[str, Dict[str, Any]],
tracks: List[Dict[str, Any]],
) -> Dict[str, Any]:
"""Pair files to tracks via exact-identifier match (MBID, then ISRC).
Returns a dict with ``matches`` (one entry per file/track pair that
matched on a shared identifier) + ``used_files`` (set) +
``used_track_indices`` (set). Caller is responsible for feeding the
leftovers into the fuzzy-scoring path.
No similarity computation, no I/O. Pure dict-in/dict-out.
"""
matches: List[Dict[str, Any]] = []
used_files: Set[str] = set()
used_track_indices: Set[int] = set()
for id_key in ('mbid', 'isrc'):
# Build {identifier_value: track_index} for this key — single pass
# over tracks, lookup is O(1) per file afterwards.
track_index_by_id: Dict[str, int] = {}
for i, track in enumerate(tracks):
if i in used_track_indices:
continue
tid = _track_identifier(track, id_key)
if tid:
track_index_by_id[tid] = i
if not track_index_by_id:
continue
for f in audio_files:
if f in used_files:
continue
fid = _file_identifier(file_tags.get(f, {}), id_key)
if not fid:
continue
track_idx = track_index_by_id.get(fid)
if track_idx is None or track_idx in used_track_indices:
continue
matches.append({
'track': tracks[track_idx],
'file': f,
'confidence': EXACT_MATCH_CONFIDENCE,
'match_type': id_key,
})
used_files.add(f)
used_track_indices.add(track_idx)
return {
'matches': matches,
'used_files': used_files,
'used_track_indices': used_track_indices,
}
# ---------------------------------------------------------------------------
# Duration sanity gate
# ---------------------------------------------------------------------------
# A file whose audio length differs from the candidate track's duration
# by more than this tolerance can't possibly be the right track —
# rejecting cross-disc / cross-release / wrong-edit mismatches before
# they hit the post-download integrity check (which catches the same
# problem AFTER the file has been moved). The integrity check stays as
# a defense-in-depth backstop.
#
# Tolerance picked to match standard library-importer behavior:
# Picard ~7s, Beets ~10-15s, Plex ~10s. The post-download integrity
# check uses a stricter ±3s because it's catching truncated downloads
# (same recording, partial bytes — should be byte-exact match) — a
# different problem from "is this the same recording across remasters
# / encodings / streaming services."
#
# Real-world drift between matched-recording sources:
# - FLAC vs MP3 transcode of same master: typically <0.5s
# - Different mastering eras (2009 remaster vs original): 1-3s
# - Different streaming service encodings: 2-7s (varying fade-out)
# - Album version vs "remixed/expanded edition": often >10s — these
# genuinely should NOT match the original tracklist anyway
#
# 10s tolerance lands in the sweet spot: catches real recording
# mismatches (gross differences = wrong track) while accepting normal
# encoding / mastering drift.
DURATION_TOLERANCE_MS = 10000 # ±10 seconds
def duration_sanity_ok(file_duration_ms: int, track_duration_ms: int) -> bool:
"""True when the file's audio duration is plausibly the track's
duration, OR when either side has no usable duration info.
"Either side missing" returns True (don't reject when we can't
confirm) — gates only on cases where BOTH sides have a number we
can compare. Files with no length info (rare — corrupt headers,
streamed-only formats) are deferred to the fuzzy scorer.
"""
if not file_duration_ms or not track_duration_ms:
return True
return abs(int(file_duration_ms) - int(track_duration_ms)) <= DURATION_TOLERANCE_MS
# Per-source duration field conventions for what the matcher RECEIVES
# (after each client's internal normalisation), NOT what each provider's
# raw API returns. Deezer's API returns `duration` in seconds, but
# `DeezerClient.get_album_tracks` converts to `duration_ms` in actual
# ms before returning — so the matcher sees ms, and double-converting
# here turns 255000 into 255000000 (the user-reported "no matches"
# bug from 2026-05-09 — every Deezer-primary user's auto-import broke).
#
# Track entries built by `_build_album_track_entry` carry the source
# name on `source` / `_source` / `provider` so we can dispatch
# deterministically instead of guessing from value magnitude.
_SECONDS_DURATION_SOURCES = frozenset((
'discogs', # release tracks expose duration as MM:SS strings
# (handled in metadata layer, but defensive here)
'musicbrainz', # recording length is sometimes seconds vs ms
# depending on which endpoint
))
_MS_DURATION_SOURCES = frozenset((
'spotify', # duration_ms (canonical Spotify naming)
'itunes', # trackTimeMillis → normalised to duration_ms upstream
'deezer', # CLIENT converts seconds → ms before returning
# (see core/deezer_client.py:get_album_tracks)
'qobuz', # duration_ms
'tidal', # duration in seconds OR duration_ms — see below
'hydrabase', # duration_ms
'hifi', # duration_ms
))
def _track_duration_ms(track: Dict[str, Any]) -> int:
"""Pull track duration in milliseconds — source-aware.
Different metadata providers spell + scale duration differently:
- Spotify / iTunes / Qobuz / HiFi / Hydrabase: ``duration_ms`` (ms)
- Deezer / Discogs: ``duration`` (seconds, int)
- Tidal: depends on endpoint — usually seconds for browse, ms for
album tracks; defensive heuristic kicks in if source missing
Decision order:
1. If the track carries a source name + that source is in the
seconds-only list, treat raw value as seconds and × 1000.
2. If source is ms-only, take the value as-is.
3. If source unknown / missing (e.g. mocked test data), fall back
to a magnitude heuristic — values < 30000 treated as seconds.
This is the legacy behavior, kept as the safety net.
"""
raw = track.get('duration_ms') or track.get('duration') or 0
try:
value = int(raw)
except (TypeError, ValueError):
return 0
if value <= 0:
return 0
source = (track.get('source') or track.get('_source') or track.get('provider') or '').strip().lower()
if source in _SECONDS_DURATION_SOURCES:
return value * 1000
if source in _MS_DURATION_SOURCES:
return value
# Unknown / missing source — fall back to the magnitude heuristic.
# Anything below 30000 (30 seconds in ms) is implausibly short for
# a real track and is almost certainly seconds being passed where
# ms was expected.
if value < 30000:
return value * 1000
return value
def match_files_to_tracks(
audio_files: List[str],
file_tags: Dict[str, Dict[str, Any]],
tracks: List[Dict[str, Any]],
*,
target_album: str,
similarity: SimilarityFn,
quality_rank: QualityRankFn,
) -> Dict[str, Any]:
"""Match staging files to album tracks.
Algorithm (in order):
1. **Exact-identifier fast paths** (``find_exact_id_matches``) —
pair files to tracks via shared MBID, then ISRC. Picard-tagged
libraries land here on the first pass with full confidence,
skipping the fuzzy scorer entirely. Each match carries a
``'match_type': 'mbid' | 'isrc'`` field for downstream
provenance / debug logging.
2. **Quality dedup** on remaining files — keep the highest-quality
file per ``(disc, track)`` position.
3. **Fuzzy scoring** on remaining files vs remaining tracks — title
+ artist + position + album-tag weighted scoring with a duration
sanity gate (files whose audio length is more than
``DURATION_TOLERANCE_MS`` from the candidate track are rejected
before scoring, regardless of how good the title agreement
looks).
Returns a dict with:
- ``matches``: list of ``{'track': dict, 'file': str, 'confidence': float}``;
exact-id matches additionally carry ``'match_type'``.
- ``unmatched_files``: files left over after every track found its
best (or none).
Each file matches at most one track. Each track matches at most one
file. Pure function — no side effects, no I/O, no metadata client.
"""
matches: List[Dict[str, Any]] = []
used_files: Set[str] = set()
used_track_indices: Set[int] = set()
# Phase 1 — exact identifiers (MBID, then ISRC).
exact = find_exact_id_matches(audio_files, file_tags, tracks)
matches.extend(exact['matches'])
used_files.update(exact['used_files'])
used_track_indices.update(exact['used_track_indices'])
# Phase 2 — quality dedup on remaining files.
remaining_files = [f for f in audio_files if f not in used_files]
deduped = dedupe_files_by_position(remaining_files, file_tags, quality_rank=quality_rank)
# Phase 3 — fuzzy scoring on remaining tracks.
duration_rejected = 0 # diagnostics for the "no matches" case
below_threshold = 0
sample_rejection_logged = False
for i, track in enumerate(tracks):
if i in used_track_indices:
continue
track_duration = _track_duration_ms(track)
best_file = None
best_score = 0.0
for f in deduped:
if f in used_files:
continue
tags = file_tags.get(f, {})
# Duration sanity gate — reject implausible matches before
# title/artist scoring even runs. Defends against the
# cross-disc / cross-release wrong-edit problem the post-
# download integrity check used to catch only AFTER the
# file had already been moved + tagged + DB-inserted.
file_duration = tags.get('duration_ms', 0) or 0
if not duration_sanity_ok(file_duration, track_duration):
duration_rejected += 1
# On the FIRST rejection per matcher run, log the actual
# values so users / reviewers can see whether it's a
# unit mismatch (seconds vs ms), genuine drift, or some
# third thing. Logging every rejection would spam the
# log on a 21-file × 19-track album (399 lines).
if not sample_rejection_logged:
sample_rejection_logged = True
raw_dur_ms = track.get('duration_ms')
raw_dur = track.get('duration')
raw_src = track.get('source') or track.get('_source') or track.get('provider')
logger.info(
"[Album Matching] First duration rejection in '%s': "
"file %r duration_ms=%d, track %r resolved=%d "
"(raw duration_ms=%r, raw duration=%r, source=%r)",
target_album,
os.path.basename(f), file_duration,
track.get('name', '?'), track_duration,
raw_dur_ms, raw_dur, raw_src,
)
continue
score = score_file_against_track(
f, tags, track,
target_album=target_album,
similarity=similarity,
)
if score > best_score and score >= MATCH_THRESHOLD:
best_score = score
best_file = f
if best_file:
used_files.add(best_file)
matches.append({
'track': track,
'file': best_file,
'confidence': round(best_score, 3),
})
elif deduped:
below_threshold += 1
# Diagnostic surface — when the matcher returns 0 matches against
# a non-trivial input, it's nearly always one of: duration gate too
# strict, title agreement too low, or wrong tracks list passed in.
# Log a one-line summary at INFO so users grep'ing app.log for
# "no matches" cases see WHY without needing to bump log level.
if not matches and (audio_files or tracks):
logger.info(
"[Album Matching] No matches: %d files, %d tracks, "
"%d duration-rejected pairs, %d tracks below threshold. "
"Album: %r",
len(audio_files), len(tracks),
duration_rejected, below_threshold, target_album,
)
# Final unmatched list: every file that didn't get used in any
# phase. Includes quality-dedup losers (lower-quality copies of
# files we already matched) so the caller can see the full picture.
return {
'matches': matches,
'unmatched_files': [f for f in audio_files if f not in used_files],
}
__all__ = [
'TITLE_WEIGHT',
'ARTIST_WEIGHT',
'POSITION_WEIGHT',
'NEAR_POSITION_WEIGHT',
'CROSS_DISC_POSITION_WEIGHT',
'ALBUM_WEIGHT',
'MATCH_THRESHOLD',
'EXACT_MATCH_CONFIDENCE',
'DURATION_TOLERANCE_MS',
'dedupe_files_by_position',
'score_file_against_track',
'find_exact_id_matches',
'duration_sanity_ok',
'match_files_to_tracks',
]

309
core/imports/side_effects.py
View file

@ -50,6 +50,115 @@ def _stable_soulsync_id(text: str) -> str:
return str(abs(int(hashlib.md5(text.encode("utf-8", errors="replace")).hexdigest(), 16)) % (10 ** 9))
# Tiny SQL allowlist for the fill-empty helpers — prevents accidental
# SQL injection through the f-string column-name interpolation. Only
# columns the soulsync library write path ever updates are listed.
_SOULSYNC_FILLABLE_COLUMNS = {
"artists": frozenset({"thumb_url", "genres", "summary", "spotify_artist_id",
"itunes_artist_id", "deezer_id", "discogs_id", "soul_id",
"hifi_artist_id"}),
"albums": frozenset({"thumb_url", "genres", "year", "track_count", "duration",
"spotify_album_id", "itunes_album_id", "deezer_id",
"discogs_id", "soul_id", "hifi_album_id"}),
}
def _fill_empty_columns(cursor, table: str, row_id: Any, fields: Dict[str, Any]) -> None:
"""UPDATE only the columns whose current value is NULL or empty.
Conservative: never overwrites populated values. Lets a re-import
fill metadata gaps (e.g. cover art that wasn't available the first
time) without trampling enrichment data the metadata workers wrote
later. Mirrors how the media-server scanner refreshes rows on each
pass, but with the safety belt of "don't clobber".
Empty-check happens in Python (not SQL) because SQLite's
`NULLIF(text_col, 0)` returns the original text value instead of
NULL — type-coercion mismatch makes the SQL-only conditional
unreliable. Reading the row first, comparing in Python, then
issuing only the necessary SET clauses sidesteps that entirely.
Column names are validated against `_SOULSYNC_FILLABLE_COLUMNS`
before any f-string interpolation — defense against accidental
misuse adding new columns without an allowlist update.
"""
allowed = _SOULSYNC_FILLABLE_COLUMNS.get(table, frozenset())
safe_fields = {col: val for col, val in fields.items() if col in allowed}
if not safe_fields:
return
# Read current values so we can decide per-column whether a fill
# is needed. Single SELECT instead of one-per-column saves
# round-trips.
col_list = ", ".join(safe_fields.keys())
try:
cursor.execute(f"SELECT {col_list} FROM {table} WHERE id = ?", (row_id,))
except Exception as e:
logger.debug("fill-empty SELECT on %s failed: %s", table, e)
return
row = cursor.fetchone()
if not row:
return
set_clauses: list[str] = []
values: list[Any] = []
for col, new_value in safe_fields.items():
# Skip when payload itself is empty — no point writing NULL → NULL.
# For numeric columns (year, duration, track_count) 0 means
# "unknown" so treat as no-op too.
if new_value in (None, "", 0):
continue
# Read current value; only fill when it's empty/zero.
try:
current = row[col]
except (KeyError, IndexError):
continue
if current not in (None, "", 0):
continue
set_clauses.append(f"{col} = ?")
values.append(new_value)
if not set_clauses:
return
values.append(row_id)
try:
cursor.execute(
f"UPDATE {table} SET {', '.join(set_clauses)}, updated_at = CURRENT_TIMESTAMP WHERE id = ?",
values,
)
except Exception as e:
logger.debug("fill-empty UPDATE on %s failed: %s", table, e)
def _fill_empty_source_id(cursor, table: str, column: str, value: str, row_id: Any) -> None:
"""Single-column variant of _fill_empty_columns for the
`<source>_<entity>_id` columns whose names come from
`get_library_source_id_columns(source)`."""
if column not in _SOULSYNC_FILLABLE_COLUMNS.get(table, frozenset()):
logger.debug("skipping non-allowlisted source-id column %s.%s", table, column)
return
if not value:
return
try:
cursor.execute(f"SELECT {column} FROM {table} WHERE id = ?", (row_id,))
row = cursor.fetchone()
except Exception as e:
logger.debug("fill-empty source-id SELECT on %s.%s failed: %s", table, column, e)
return
if not row:
return
try:
current = row[column]
except (KeyError, IndexError):
return
if current not in (None, ""):
return
try:
cursor.execute(
f"UPDATE {table} SET {column} = ? WHERE id = ?",
(value, row_id),
)
except Exception as e:
logger.debug("fill-empty source-id UPDATE on %s.%s failed: %s", table, column, e)
def emit_track_downloaded(context: Dict[str, Any], automation_engine=None) -> None:
"""Emit the track_downloaded automation event."""
try:
@ -89,6 +198,12 @@ def record_library_history_download(context: Dict[str, Any]) -> None:
"deezer_dl": "Deezer",
"lidarr": "Lidarr",
"soundcloud": "SoundCloud",
# Auto-import isn't a download source, but flows through the
# same post-process pipeline (file lands → record provenance
# + history → write to library DB). Tagging it as "Auto-Import"
# in history avoids mislabeling staging-folder imports as
# Soulseek downloads.
"auto_import": "Auto-Import",
}
download_source = source_map.get(username, "Soulseek")
@ -161,6 +276,13 @@ def record_download_provenance(context: Dict[str, Any]) -> None:
"deezer_dl": "deezer",
"lidarr": "lidarr",
"soundcloud": "soundcloud",
# Auto-import: surfaced in provenance so the redownload modal
# can tell the user "this came from staging on <date>" instead
# of falsely listing soulseek as the source. The underlying
# metadata source (spotify / deezer / itunes) is recorded
# separately via the source-aware ID columns on the tracks
# row itself.
"auto_import": "auto_import",
}.get(username, "soulseek")
ti = context.get("track_info") or context.get("search_result") or {}
@ -339,71 +461,136 @@ def record_soulsync_library_entry(context: Dict[str, Any], artist_context: Dict[
album_id = _stable_soulsync_id(f"{artist_name}::{album_name}".lower().strip())
track_id = _stable_soulsync_id(final_path)
total_tracks = album_ctx.get("total_tracks", 0) or 0
# Album total duration — auto-import passes the sum of every
# matched track's duration via `album.duration_ms`, mirroring
# what soulsync_client's deep scan computes. Falls back to
# the per-track duration for callers that don't provide an
# album total (legacy direct-download flow).
album_total_duration_ms = int(
album_ctx.get("duration_ms") or duration_ms or 0
)
db = get_database()
with db._get_connection() as conn:
cursor = conn.cursor()
cursor.execute("SELECT id FROM artists WHERE id = ? AND server_source = 'soulsync'", (artist_id,))
if not cursor.fetchone():
# ── Artist row: insert-or-fill-empty-fields ────────────
#
# Pre-refactor was insert-only: subsequent imports of the
# same artist (same name, second album) found the existing
# row via the name-fallback SELECT and skipped completely.
# That meant artist genres / thumb / source-id reflected
# whatever the FIRST imported album supplied, never
# refreshing as more albums by that artist landed.
#
# Conservative fix: when an existing row matches, run an
# UPDATE that only fills NULL/empty fields (`thumb_url IS
# NULL OR thumb_url = ''`). Never overwrites populated
# values — protects manual edits + enrichment-worker
# writes.
artist_source_col = source_columns.get("artist")
cursor.execute(
"SELECT id FROM artists WHERE id = ? AND server_source = 'soulsync'",
(artist_id,),
)
row = cursor.fetchone()
if not row:
cursor.execute(
"SELECT id FROM artists WHERE name COLLATE NOCASE = ? AND server_source = 'soulsync' LIMIT 1",
(artist_name,),
)
existing_by_name = cursor.fetchone()
if existing_by_name:
artist_id = existing_by_name[0]
else:
cursor.execute("SELECT id FROM artists WHERE id = ?", (artist_id,))
if cursor.fetchone():
artist_id = _stable_soulsync_id(artist_name.lower().strip() + "::soulsync")
cursor.execute(
"""
INSERT INTO artists (id, name, genres, thumb_url, server_source, created_at, updated_at)
VALUES (?, ?, ?, ?, 'soulsync', CURRENT_TIMESTAMP, CURRENT_TIMESTAMP)
""",
(artist_id, artist_name, genres_json, image_url),
)
artist_source_col = source_columns.get("artist")
if artist_source_col and artist_source_id:
try:
cursor.execute(
f"UPDATE artists SET {artist_source_col} = ? WHERE id = ?",
(artist_source_id, artist_id),
)
except Exception as e:
logger.debug("artist source-id update failed: %s", e)
row = cursor.fetchone()
if row:
artist_id = row[0]
cursor.execute("SELECT id FROM albums WHERE id = ? AND server_source = 'soulsync'", (album_id,))
if not cursor.fetchone():
if row:
_fill_empty_columns(
cursor,
table="artists",
row_id=artist_id,
fields={
"thumb_url": image_url,
"genres": genres_json,
},
)
if artist_source_col and artist_source_id:
_fill_empty_source_id(cursor, "artists", artist_source_col, artist_source_id, artist_id)
else:
# Hash collision protection — if the stable ID is
# already in use by a different server's row, mint a
# soulsync-suffixed ID so we don't trample.
cursor.execute("SELECT id FROM artists WHERE id = ?", (artist_id,))
if cursor.fetchone():
artist_id = _stable_soulsync_id(artist_name.lower().strip() + "::soulsync")
cursor.execute(
"""
INSERT INTO artists (id, name, genres, thumb_url, server_source, created_at, updated_at)
VALUES (?, ?, ?, ?, 'soulsync', CURRENT_TIMESTAMP, CURRENT_TIMESTAMP)
""",
(artist_id, artist_name, genres_json, image_url),
)
if artist_source_col and artist_source_id:
try:
cursor.execute(
f"UPDATE artists SET {artist_source_col} = ? WHERE id = ?",
(artist_source_id, artist_id),
)
except Exception as e:
logger.debug("artist source-id update failed: %s", e)
# ── Album row: same insert-or-fill-empty-fields shape ──
album_source_col = source_columns.get("album")
cursor.execute(
"SELECT id FROM albums WHERE id = ? AND server_source = 'soulsync'",
(album_id,),
)
row = cursor.fetchone()
if not row:
cursor.execute(
"SELECT id FROM albums WHERE title COLLATE NOCASE = ? AND artist_id = ? AND server_source = 'soulsync' LIMIT 1",
(album_name, artist_id),
)
existing_album_by_name = cursor.fetchone()
if existing_album_by_name:
album_id = existing_album_by_name[0]
else:
cursor.execute("SELECT id FROM albums WHERE id = ?", (album_id,))
if cursor.fetchone():
album_id = _stable_soulsync_id(f"{artist_name}::{album_name}::soulsync".lower().strip())
cursor.execute(
"""
INSERT INTO albums (id, artist_id, title, year, thumb_url, genres, track_count,
duration, server_source, created_at, updated_at)
VALUES (?, ?, ?, ?, ?, ?, ?, ?, 'soulsync', CURRENT_TIMESTAMP, CURRENT_TIMESTAMP)
""",
(album_id, artist_id, album_name, year, image_url, genres_json, total_tracks, duration_ms),
)
album_source_col = source_columns.get("album")
if album_source_col and album_source_id:
try:
cursor.execute(
f"UPDATE albums SET {album_source_col} = ? WHERE id = ?",
(album_source_id, album_id),
)
except Exception as e:
logger.debug("album source-id update failed: %s", e)
row = cursor.fetchone()
if row:
album_id = row[0]
if row:
_fill_empty_columns(
cursor,
table="albums",
row_id=album_id,
fields={
"thumb_url": image_url,
"genres": genres_json,
"year": year,
"track_count": total_tracks,
"duration": album_total_duration_ms,
},
)
if album_source_col and album_source_id:
_fill_empty_source_id(cursor, "albums", album_source_col, album_source_id, album_id)
else:
cursor.execute("SELECT id FROM albums WHERE id = ?", (album_id,))
if cursor.fetchone():
album_id = _stable_soulsync_id(f"{artist_name}::{album_name}::soulsync".lower().strip())
cursor.execute(
"""
INSERT INTO albums (id, artist_id, title, year, thumb_url, genres, track_count,
duration, server_source, created_at, updated_at)
VALUES (?, ?, ?, ?, ?, ?, ?, ?, 'soulsync', CURRENT_TIMESTAMP, CURRENT_TIMESTAMP)
""",
(album_id, artist_id, album_name, year, image_url, genres_json, total_tracks, album_total_duration_ms),
)
if album_source_col and album_source_id:
try:
cursor.execute(
f"UPDATE albums SET {album_source_col} = ? WHERE id = ?",
(album_source_id, album_id),
)
except Exception as e:
logger.debug("album source-id update failed: %s", e)
track_artist = None
track_artists_list = track_info.get("artists", []) or original_search.get("artists", [])
@ -416,14 +603,28 @@ def record_soulsync_library_entry(context: Dict[str, Any], artist_context: Dict[
if ta_name and ta_name.lower() != artist_name.lower():
track_artist = ta_name
# Per-recording identifiers — scanner picks `musicbrainz_recording_id`
# off the Navidrome track wrapper; auto-import has the same field
# available from the metadata-source response (Spotify exposes
# `musicbrainz_recording_id` via the MusicBrainz client, Picard-
# tagged files surface it via `_read_file_tags`). `isrc` is even
# better signal for cross-source dedup — it's the per-recording
# ID labels embed in the audio. Both land in dedicated columns
# so the watchlist scanner's stable-ID match path recognises
# auto-imported tracks the next time the user adds the artist
# to a watchlist.
track_mbid = (track_info.get("musicbrainz_recording_id") or "").strip().lower() or None
track_isrc = (track_info.get("isrc") or "").strip().upper() or None
cursor.execute("SELECT id FROM tracks WHERE file_path = ?", (final_path,))
if not cursor.fetchone():
cursor.execute(
"""
INSERT INTO tracks (id, album_id, artist_id, title, track_number,
duration, file_path, bitrate, file_size, track_artist, server_source,
duration, file_path, bitrate, file_size, track_artist,
musicbrainz_recording_id, isrc, server_source,
created_at, updated_at)
VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, 'soulsync', CURRENT_TIMESTAMP, CURRENT_TIMESTAMP)
VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, 'soulsync', CURRENT_TIMESTAMP, CURRENT_TIMESTAMP)
""",
(
track_id,
@ -436,6 +637,8 @@ def record_soulsync_library_entry(context: Dict[str, Any], artist_context: Dict[
bitrate,
file_size,
track_artist,
track_mbid,
track_isrc,
),
)
track_source_col = source_columns.get("track")

24
core/metadata/album_tracks.py
View file

@ -320,6 +320,27 @@ def _build_album_track_entry(track_item: Any, album_info: Dict[str, Any], source
if isinstance(explicit_value, str):
explicit_value = explicit_value.lower() == 'explicit'
# Per-recording exact identifiers — drive the auto-import matcher's
# fast paths (`core.imports.album_matching.find_exact_id_matches`).
# Spotify/Deezer typically expose ISRC inside `external_ids.isrc`;
# iTunes uses top-level `isrc`. MusicBrainz-aware sources expose MBID
# similarly. Stripping these used to be invisible — until the matcher
# learned to use them, then it became "fast paths never trigger in
# production even though the unit tests pass" — pinned by the
# production-shape test in test_album_matching_exact_id.py.
external_ids = _extract_lookup_value(track_item, 'external_ids', default=None) or {}
isrc = (
_extract_lookup_value(track_item, 'isrc', default='') or ''
or (external_ids.get('isrc') if isinstance(external_ids, dict) else '')
or ''
)
mbid = (
_extract_lookup_value(track_item, 'musicbrainz_id', 'mbid', default='') or ''
or (external_ids.get('mbid') if isinstance(external_ids, dict) else '')
or (external_ids.get('musicbrainz') if isinstance(external_ids, dict) else '')
or ''
)
return {
'id': _extract_lookup_value(track_item, 'id', 'track_id', 'trackId', default='') or '',
'name': _extract_lookup_value(track_item, 'name', 'track_name', 'trackName', default='Unknown Track') or 'Unknown Track',
@ -330,6 +351,9 @@ def _build_album_track_entry(track_item: Any, album_info: Dict[str, Any], source
'explicit': bool(explicit_value),
'preview_url': _extract_lookup_value(track_item, 'preview_url', 'previewUrl'),
'external_urls': _extract_lookup_value(track_item, 'external_urls', default={}) or {},
'external_ids': external_ids if isinstance(external_ids, dict) else {},
'isrc': str(isrc) if isrc else '',
'musicbrainz_id': str(mbid) if mbid else '',
'uri': _extract_lookup_value(track_item, 'uri', default='') or '',
'album': album_info,
'source': source,

479
tests/imports/test_album_matching_exact_id.py Normal file
View file

@ -0,0 +1,479 @@
"""Tests for the ID-based fast paths + duration sanity gate added on
top of the fuzzy matcher in ``core/imports/album_matching.py``.
This is the "state-of-the-art" matching layer — bringing the auto-
import worker up to parity with what Picard / Beets / Roon do.
Algorithm (in order, each test pins one phase):
1. **MBID exact match** — file has ``MUSICBRAINZ_TRACKID`` tag, metadata
source returns the same id → instant pair, full confidence, skip
fuzzy scoring entirely.
2. **ISRC exact match** — file has ``ISRC`` tag, source returns the
same id → same fast-path, slightly lower priority than MBID
(multiple recordings can share an ISRC across remasters/regions).
3. **Duration sanity gate** — file's audio length must be within
``DURATION_TOLERANCE_MS`` of the candidate track's duration.
Defends against the cross-disc / cross-release / wrong-edit problem
the post-download integrity check used to catch only AFTER files
were already moved.
4. **Fuzzy fallback** — files with no usable IDs and no duration veto
fall through to the existing weighted scorer.
"""
from __future__ import annotations
from difflib import SequenceMatcher
from core.imports.album_matching import (
DURATION_TOLERANCE_MS,
EXACT_MATCH_CONFIDENCE,
duration_sanity_ok,
find_exact_id_matches,
match_files_to_tracks,
)
def _sim(a, b):
return SequenceMatcher(None, (a or '').lower(), (b or '').lower()).ratio()
def _qrank(ext):
ranks = {'.flac': 100, '.alac': 95, '.wav': 80, '.aac': 60,
'.ogg': 50, '.opus': 50, '.m4a': 60, '.mp3': 30}
return ranks.get((ext or '').lower(), 0)
def _tags(*, title='', artist='', album='', track=0, disc=1,
isrc='', mbid='', duration_ms=0):
return {
'title': title, 'artist': artist, 'album': album,
'track_number': track, 'disc_number': disc, 'year': '',
'isrc': isrc, 'mbid': mbid, 'duration_ms': duration_ms,
}
def _api_track(*, name='', track_number=0, disc_number=1,
isrc='', mbid='', duration_ms=0, external_ids=None):
out = {
'name': name,
'track_number': track_number,
'disc_number': disc_number,
'duration_ms': duration_ms,
'artists': [],
}
if isrc:
out['isrc'] = isrc
if mbid:
out['musicbrainz_id'] = mbid
if external_ids:
out['external_ids'] = external_ids
return out
# ---------------------------------------------------------------------------
# find_exact_id_matches — direct unit tests
# ---------------------------------------------------------------------------
def test_mbid_exact_match_pairs_file_to_track():
"""File with MBID tag matches the track carrying the same MBID,
even when title is completely wrong."""
files = ['/a/scrambled.flac']
file_tags = {
'/a/scrambled.flac': _tags(
title='Scrambled Filename', mbid='abc-123-mbid',
),
}
tracks = [
_api_track(name='Real Track Name', mbid='abc-123-mbid'),
]
result = find_exact_id_matches(files, file_tags, tracks)
assert len(result['matches']) == 1
assert result['matches'][0]['file'] == '/a/scrambled.flac'
assert result['matches'][0]['match_type'] == 'mbid'
assert result['matches'][0]['confidence'] == EXACT_MATCH_CONFIDENCE
def test_isrc_exact_match_pairs_file_to_track():
files = ['/a/track.flac']
file_tags = {
'/a/track.flac': _tags(title='Foo', isrc='USRC11234567'),
}
tracks = [_api_track(name='Real', isrc='USRC11234567')]
result = find_exact_id_matches(files, file_tags, tracks)
assert len(result['matches']) == 1
assert result['matches'][0]['match_type'] == 'isrc'
def test_isrc_normalization_strips_dashes_and_spaces():
"""File tag ``USRC11234567`` should match source ISRC ``US-RC1-12-34567``
— same identifier, different formatting. Picard writes compact;
some sources return hyphenated."""
files = ['/a/f.flac']
file_tags = {'/a/f.flac': _tags(isrc='USRC11234567')}
tracks = [_api_track(name='X', isrc='US-RC1-12-34567')]
result = find_exact_id_matches(files, file_tags, tracks)
assert len(result['matches']) == 1
def test_mbid_takes_priority_over_isrc():
"""When both identifiers are present and they'd point at different
tracks, MBID wins. ISRC can be shared across remasters; MBID is
per-recording."""
files = ['/a/f.flac']
file_tags = {'/a/f.flac': _tags(isrc='SAME', mbid='real-mbid')}
tracks = [
_api_track(name='Wrong Recording', isrc='SAME', mbid='different-mbid'),
_api_track(name='Right Recording', mbid='real-mbid'),
]
result = find_exact_id_matches(files, file_tags, tracks)
assert len(result['matches']) == 1
assert result['matches'][0]['track']['name'] == 'Right Recording'
assert result['matches'][0]['match_type'] == 'mbid'
def test_isrc_via_external_ids_dict_matches():
"""Spotify exposes ISRC under ``external_ids.isrc``, not as a
top-level field. Matcher must check both shapes."""
files = ['/a/f.flac']
file_tags = {'/a/f.flac': _tags(isrc='USRC11234567')}
tracks = [_api_track(name='X', external_ids={'isrc': 'USRC11234567'})]
result = find_exact_id_matches(files, file_tags, tracks)
assert len(result['matches']) == 1
def test_no_id_match_returns_empty():
"""File and track both have IDs, but they don't match → no exact
match. (Caller falls back to fuzzy.)"""
files = ['/a/f.flac']
file_tags = {'/a/f.flac': _tags(mbid='different-id')}
tracks = [_api_track(name='X', mbid='another-id')]
result = find_exact_id_matches(files, file_tags, tracks)
assert not result['matches']
def test_each_id_match_uses_track_at_most_once():
"""Two files with the same MBID — only the first one wins. Caller
deals with the leftover (probably a duplicate/extra file)."""
files = ['/a/first.flac', '/a/second.flac']
file_tags = {
'/a/first.flac': _tags(mbid='shared'),
'/a/second.flac': _tags(mbid='shared'),
}
tracks = [_api_track(name='Track', mbid='shared')]
result = find_exact_id_matches(files, file_tags, tracks)
assert len(result['matches']) == 1
assert len(result['used_files']) == 1
# ---------------------------------------------------------------------------
# duration_sanity_ok — direct unit tests
# ---------------------------------------------------------------------------
def test_duration_within_tolerance_passes():
assert duration_sanity_ok(180_000, 180_000) is True
assert duration_sanity_ok(180_000, 181_500) is True
assert duration_sanity_ok(180_000, 180_000 - DURATION_TOLERANCE_MS) is True
def test_duration_outside_tolerance_fails():
assert duration_sanity_ok(180_000, 180_000 + DURATION_TOLERANCE_MS + 1) is False
assert duration_sanity_ok(180_000, 90_000) is False
# The Mr. Morale Auntie-Diaries-vs-Rich-Interlude case from the bug
# report: 281s file vs 103s expected — gross mismatch, must reject.
assert duration_sanity_ok(281_000, 103_000) is False
def test_duration_missing_either_side_passes():
"""Don't reject when we can't confirm. Files with no length info
(corrupt headers, etc.) defer to the fuzzy scorer."""
assert duration_sanity_ok(0, 180_000) is True
assert duration_sanity_ok(180_000, 0) is True
assert duration_sanity_ok(0, 0) is True
# ---------------------------------------------------------------------------
# match_files_to_tracks — end-to-end with the new fast paths
# ---------------------------------------------------------------------------
def test_mbid_match_short_circuits_fuzzy_scoring():
"""File with MBID + completely wrong title still matches the right
track via MBID. Demonstrates the fast-path bypassing fuzzy scoring."""
files = ['/a/file.flac']
file_tags = {
'/a/file.flac': _tags(
title='Completely Wrong Title',
artist='Wrong Artist',
track=99, disc=99,
mbid='real-mbid',
),
}
tracks = [
_api_track(name='Real Title', track_number=1, disc_number=1, mbid='real-mbid'),
]
result = match_files_to_tracks(
files, file_tags, tracks,
target_album='', similarity=_sim, quality_rank=_qrank,
)
assert len(result['matches']) == 1
assert result['matches'][0]['match_type'] == 'mbid'
assert result['matches'][0]['confidence'] == EXACT_MATCH_CONFIDENCE
def test_id_matched_files_excluded_from_fuzzy_phase():
"""File matched in phase 1 (exact ID) shouldn't be considered in
phase 3 (fuzzy). Otherwise it could end up matched twice."""
files = ['/a/exact.flac', '/a/fuzzy.flac']
file_tags = {
'/a/exact.flac': _tags(title='Track A', mbid='mbid-a'),
'/a/fuzzy.flac': _tags(title='Track B', track=2, disc=1),
}
tracks = [
_api_track(name='Track A', track_number=1, disc_number=1, mbid='mbid-a'),
_api_track(name='Track B', track_number=2, disc_number=1),
]
result = match_files_to_tracks(
files, file_tags, tracks,
target_album='', similarity=_sim, quality_rank=_qrank,
)
assert len(result['matches']) == 2
file_set = {m['file'] for m in result['matches']}
assert file_set == {'/a/exact.flac', '/a/fuzzy.flac'}
def test_duration_gate_rejects_wrong_disc_collision_in_fuzzy_phase():
"""The Mr. Morale bug case re-cast as a duration veto. File has
the audio length of the disc-2 track, API track is the disc-1 track
with the same number. Pre-fix: would have matched on track_number
alone. Post-fix: even after the disc-aware scoring, the duration
gate stops it."""
files = ['/a/track06.flac']
file_tags = {
'/a/track06.flac': _tags(
title='', track=6, disc=1, # wrong/missing disc tag
duration_ms=281_000, # actual audio is 4:41
),
}
tracks = [
_api_track(
name='Rich (Interlude)', track_number=6, disc_number=1,
duration_ms=103_000, # 1:43
),
]
result = match_files_to_tracks(
files, file_tags, tracks,
target_album='Mr. Morale', similarity=_sim, quality_rank=_qrank,
)
# Duration gate rejects → file unmatched (correct).
assert not result['matches']
assert result['unmatched_files'] == ['/a/track06.flac']
def test_duration_gate_within_tolerance_allows_normal_match():
"""File and track durations agree within tolerance — match proceeds
normally via fuzzy scoring."""
files = ['/a/track.flac']
file_tags = {
'/a/track.flac': _tags(
title='Father Time', track=5, disc=1, duration_ms=362_000,
),
}
tracks = [
_api_track(
name='Father Time', track_number=5, disc_number=1,
duration_ms=363_500, # 1.5s drift — within 3s tolerance
),
]
result = match_files_to_tracks(
files, file_tags, tracks,
target_album='', similarity=_sim, quality_rank=_qrank,
)
assert len(result['matches']) == 1
def test_no_durations_anywhere_falls_through_to_fuzzy():
"""Either side missing duration → gate doesn't apply, fuzzy
scoring handles it. Catches files with corrupt audio headers."""
files = ['/a/track.flac']
file_tags = {
'/a/track.flac': _tags(
title='Father Time', track=5, disc=1, duration_ms=0,
),
}
tracks = [_api_track(name='Father Time', track_number=5, disc_number=1)]
result = match_files_to_tracks(
files, file_tags, tracks,
target_album='', similarity=_sim, quality_rank=_qrank,
)
assert len(result['matches']) == 1
def test_deezer_already_normalised_to_ms_by_client():
"""Deezer's API returns ``duration`` in seconds — but
``DeezerClient.get_album_tracks`` converts to ``duration_ms`` (in
actual ms) before returning. The matcher classifies Deezer as an
MS source so it doesn't double-convert. Pin this so the
classification stays in sync with the client's behavior."""
files = ['/a/track.flac']
file_tags = {
'/a/track.flac': _tags(
title='Song', track=1, disc=1, duration_ms=180_000,
),
}
# Deezer-style track AS RECEIVED FROM `get_album_tracks` — already
# converted to duration_ms in actual milliseconds.
tracks = [{
'name': 'Song', 'track_number': 1, 'disc_number': 1,
'duration_ms': 180_000, 'artists': [], 'source': 'deezer',
}]
result = match_files_to_tracks(
files, file_tags, tracks,
target_album='', similarity=_sim, quality_rank=_qrank,
)
# Source-aware dispatch keeps 180_000 as-is (don't × 1000) so it
# matches the file's 180_000 ms. Pre-fix Deezer was wrongly
# classified as a seconds source → 180_000 × 1000 = 180,000,000ms,
# gate rejected every Deezer-primary user's matches.
assert len(result['matches']) == 1
def test_track_duration_source_aware_dispatch():
"""`_track_duration_ms` routes via the `source` field — values
from sources where the CLIENT has already normalised to ms are
taken as-is."""
from core.imports.album_matching import _track_duration_ms
spotify_track = {'duration_ms': 180_000, 'source': 'spotify'}
assert _track_duration_ms(spotify_track) == 180_000
# Deezer — client converts seconds → ms before returning, so
# downstream matcher gets ms. As-is.
deezer_track = {'duration_ms': 180_000, 'source': 'deezer'}
assert _track_duration_ms(deezer_track) == 180_000
itunes_track = {'duration_ms': 200_000, 'source': 'itunes'}
assert _track_duration_ms(itunes_track) == 200_000
legacy_source = {'duration_ms': 150_000, '_source': 'spotify'}
assert _track_duration_ms(legacy_source) == 150_000
def test_raw_deezer_seconds_falls_back_to_magnitude_heuristic():
"""Edge case: if a raw Deezer item somehow reaches the matcher
WITHOUT going through the client's conversion (no `source` field,
raw `duration` key in seconds), the magnitude heuristic catches
it — value < 30000 is implausibly short for a real track and
gets × 1000."""
from core.imports.album_matching import _track_duration_ms
raw_deezer_no_source = {'duration': 180} # no source field
assert _track_duration_ms(raw_deezer_no_source) == 180_000
def test_track_duration_short_real_track_not_misconverted_with_known_source():
"""An actual sub-30s track on Spotify (intro/interlude/skit) —
duration_ms is genuinely small. Source-aware dispatch must take
spotify_ms_value as-is and NOT × 1000 it via the magnitude
heuristic. Pre-fix this would have been hit by:
20_000 ms (a 20-second intro) > 0 and < 30000 → converted to
20_000_000 ms = 5.5 hours. Wrong.
Post-fix: source='spotify' is in MS list, value taken as-is.
"""
from core.imports.album_matching import _track_duration_ms
short_intro = {'duration_ms': 20_000, 'source': 'spotify'}
assert _track_duration_ms(short_intro) == 20_000
def test_track_duration_unknown_source_falls_back_to_heuristic():
"""No source field — apply the legacy magnitude heuristic so
tests / mocks without source still work. < 30000 = seconds."""
from core.imports.album_matching import _track_duration_ms
no_source_seconds = {'duration': 180} # heuristic: < 30000 → seconds
assert _track_duration_ms(no_source_seconds) == 180_000
no_source_ms = {'duration_ms': 200_000} # heuristic: > 30000 → ms
assert _track_duration_ms(no_source_ms) == 200_000
def test_album_track_entry_propagates_isrc_and_mbid_from_source():
"""Production-path guard: the metadata-source layer
(`_build_album_track_entry`) must propagate ISRC + MBID from the
raw track responses, otherwise the matcher's fast paths never fire
in production even though they pass in unit tests.
Spotify shape: ``external_ids.isrc`` (nested dict).
iTunes shape: top-level ``isrc``.
"""
from core.metadata.album_tracks import _build_album_track_entry
spotify_shape = {
'id': 'spotify-track',
'name': 'Test',
'external_ids': {'isrc': 'USRC11234567', 'mbid': 'mb-123'},
'duration_ms': 200_000,
'track_number': 1,
'disc_number': 1,
}
entry = _build_album_track_entry(spotify_shape, {'name': 'Album'}, 'spotify')
assert entry['isrc'] == 'USRC11234567'
assert entry['musicbrainz_id'] == 'mb-123'
itunes_shape = {
'id': 'itunes-track',
'name': 'Test',
'isrc': 'USRC11234567',
'duration_ms': 200_000,
'track_number': 1,
'disc_number': 1,
}
entry = _build_album_track_entry(itunes_shape, {'name': 'Album'}, 'itunes')
assert entry['isrc'] == 'USRC11234567'
# No identifiers — entry has empty strings (not None / missing keys),
# so the matcher's `_track_identifier()` returns empty cleanly.
bare_shape = {
'id': 'bare', 'name': 'Test',
'duration_ms': 200_000, 'track_number': 1, 'disc_number': 1,
}
entry = _build_album_track_entry(bare_shape, {'name': 'Album'}, 'unknown')
assert entry['isrc'] == ''
assert entry['musicbrainz_id'] == ''
def test_picard_tagged_library_full_album_via_mbid_only():
"""Realistic Picard-tagged library: every file has MBID, no useful
title-disc-track agreement needed. Whole album should pair via the
fast path on the first phase."""
files = [f'/a/picard_{i}.flac' for i in range(1, 11)]
file_tags = {
f: _tags(
title=f'mangled name {i}', # title doesn't help
track=99 - i, disc=99, # position info is wrong
mbid=f'mbid-{i}',
)
for i, f in enumerate(files, start=1)
}
tracks = [
_api_track(
name=f'Real Track {i}',
track_number=i, disc_number=1,
mbid=f'mbid-{i}',
)
for i in range(1, 11)
]
result = match_files_to_tracks(
files, file_tags, tracks,
target_album='', similarity=_sim, quality_rank=_qrank,
)
assert len(result['matches']) == 10
# All matched via MBID, full confidence
for m in result['matches']:
assert m['match_type'] == 'mbid'
assert m['confidence'] == EXACT_MATCH_CONFIDENCE

439
tests/imports/test_album_matching_helper.py Normal file
View file

@ -0,0 +1,439 @@
"""Direct unit tests for ``core.imports.album_matching`` — the lifted
helper that powers ``AutoImportWorker._match_tracks``.
The original test file (``test_auto_import_multi_disc_matching.py``)
exercised the matching logic via the worker, requiring monkeypatches
on ``_read_file_tags`` + mocks on the metadata client. These tests
exercise the helper directly with dict inputs / dict outputs — no I/O,
no class instantiation, no patches.
Together with the worker-level tests, the helper has full behavior
coverage:
- Dedup: same-(disc, track) collapses, cross-disc preserves
- Match: per-component scoring, threshold, position weights, cross-disc
consolation, near-position bonus
- Edge cases: tag-less files (track_number=0), missing artist tags,
cross-disc collision when one side has no disc tag
"""
from __future__ import annotations
from difflib import SequenceMatcher
from core.imports.album_matching import (
ALBUM_WEIGHT,
ARTIST_WEIGHT,
CROSS_DISC_POSITION_WEIGHT,
MATCH_THRESHOLD,
NEAR_POSITION_WEIGHT,
POSITION_WEIGHT,
TITLE_WEIGHT,
dedupe_files_by_position,
match_files_to_tracks,
score_file_against_track,
)
# ---------------------------------------------------------------------------
# Stand-in similarity + quality_rank — match real worker behavior closely
# enough that test scores reflect production behavior.
# ---------------------------------------------------------------------------
def _sim(a: str, b: str) -> float:
"""Mirror of the worker's _similarity (case-folded SequenceMatcher)."""
return SequenceMatcher(None, (a or '').lower(), (b or '').lower()).ratio()
def _qrank(ext: str) -> int:
"""Mirror of the worker's _quality_rank."""
ranks = {'.flac': 100, '.alac': 95, '.wav': 80, '.aac': 60,
'.ogg': 50, '.opus': 50, '.m4a': 60, '.mp3': 30, '.wma': 20}
return ranks.get((ext or '').lower(), 0)
def _tags(*, title='', artist='Artist', album='Album', track=0, disc=1):
return {
'title': title, 'artist': artist, 'album': album,
'track_number': track, 'disc_number': disc, 'year': '',
}
# ---------------------------------------------------------------------------
# Constants — pin the weights so accidental tweaks fail at the test boundary
# ---------------------------------------------------------------------------
def test_constants_sum_to_one():
"""Sum of TITLE + ARTIST + POSITION + ALBUM should equal 1.0 in
the happy case (perfect agreement). Catches accidental drift if
someone edits one weight without checking the rest. Float tolerance
because 0.45 + 0.15 + 0.30 + 0.10 has a 1e-16 rounding error."""
total = TITLE_WEIGHT + ARTIST_WEIGHT + POSITION_WEIGHT + ALBUM_WEIGHT
assert abs(total - 1.0) < 1e-9
def test_match_threshold_requires_more_than_position_alone():
"""Pin the design intent: a file matching ONLY on position
(perfect track + disc, zero title similarity) should NOT meet
the threshold. The matcher requires meaningful title agreement
AT LEAST in addition to position. Catches accidental threshold
drops that would let position-only matches sneak through."""
assert MATCH_THRESHOLD > POSITION_WEIGHT
# ---------------------------------------------------------------------------
# dedupe_files_by_position — pure-function tests
# ---------------------------------------------------------------------------
def test_dedupe_keeps_higher_quality_at_same_position():
files = ['/a/track1.mp3', '/a/track1.flac']
file_tags = {
'/a/track1.mp3': _tags(track=1, disc=1),
'/a/track1.flac': _tags(track=1, disc=1),
}
result = dedupe_files_by_position(files, file_tags, quality_rank=_qrank)
assert result == ['/a/track1.flac']
def test_dedupe_preserves_same_track_across_discs():
"""The fix for the multi-disc bug: track_number=1 on disc 1 and
track_number=1 on disc 2 are different positions, both survive."""
files = ['/a/d1t1.flac', '/a/d2t1.flac']
file_tags = {
'/a/d1t1.flac': _tags(track=1, disc=1),
'/a/d2t1.flac': _tags(track=1, disc=2),
}
result = dedupe_files_by_position(files, file_tags, quality_rank=_qrank)
assert set(result) == {'/a/d1t1.flac', '/a/d2t1.flac'}
def test_dedupe_passes_through_files_with_no_track_number():
"""Files with track_number=0 (no tag) can't be deduped — keep them
all so the matcher gets a chance to title-match them."""
files = ['/a/no_tag_a.mp3', '/a/no_tag_b.mp3', '/a/no_tag_c.mp3']
file_tags = {f: _tags(title='Untagged', track=0, disc=1) for f in files}
result = dedupe_files_by_position(files, file_tags, quality_rank=_qrank)
assert set(result) == set(files)
def test_dedupe_keeps_first_when_quality_equal():
"""Two files at same position, same quality — first one wins."""
files = ['/a/first.flac', '/a/second.flac']
file_tags = {
'/a/first.flac': _tags(track=1, disc=1),
'/a/second.flac': _tags(track=1, disc=1),
}
result = dedupe_files_by_position(files, file_tags, quality_rank=_qrank)
assert result == ['/a/first.flac']
# ---------------------------------------------------------------------------
# score_file_against_track — per-component scoring
# ---------------------------------------------------------------------------
def test_score_perfect_agreement_equals_one():
"""Title + artist + (disc, track) + album all match → score = 1.0."""
track = {
'name': 'Song', 'track_number': 5, 'disc_number': 2,
'artists': [{'name': 'Artist'}],
}
tags = _tags(title='Song', artist='Artist', album='Album', track=5, disc=2)
score = score_file_against_track(
'/a/file.flac', tags, track,
target_album='Album', similarity=_sim,
)
assert abs(score - 1.0) < 0.001
def test_score_position_match_requires_both_disc_and_track():
"""Same track number, different disc → only CROSS_DISC bonus, not
full POSITION bonus. This is the regression fix for multi-disc
cross-collisions."""
track = {'name': 'X', 'track_number': 6, 'disc_number': 1, 'artists': []}
# File for disc 2 track 6 — same number, wrong disc
tags = _tags(title='X', track=6, disc=2)
score = score_file_against_track(
'/a/file.flac', tags, track,
target_album='', similarity=_sim,
)
# Title weight (1.0) + cross-disc consolation (0.05) + nothing else
expected = TITLE_WEIGHT + CROSS_DISC_POSITION_WEIGHT
assert abs(score - expected) < 0.001
def test_cross_disc_consolation_is_load_bearing_for_imperfect_titles():
"""Pin the design rationale for ``CROSS_DISC_POSITION_WEIGHT`` so
the magic number isn't silently regressable.
Scenario: file has the right title spelling but the metadata
source returns a slightly-different version (e.g. "(Remix)"
suffix), AND the file's disc tag is wrong / missing while the
track number agrees. The bonus is sized so this case still
matches:
title_only_score = sim("Auntie Diaries",
"Auntie Diaries (Remix)") * 0.45
≈ 0.78 * 0.45 = ~0.35 ← below MATCH_THRESHOLD
with cross_disc bonus ≈ 0.35 + 0.05 = ~0.40 ← clears
Without this consolation, the imperfect-title cross-disc case
would silently start going unmatched. If anyone considers setting
``CROSS_DISC_POSITION_WEIGHT`` to 0, this test makes the trade-off
explicit (this case becomes unmatched) instead of letting it
regress invisibly.
"""
track = {
'name': 'Auntie Diaries (Remix)',
'track_number': 6, 'disc_number': 1,
'artists': [],
}
# File: same track number, different disc, similar but not perfect
# title (file has the canonical name, source has the version
# variant — common with deluxe / remix / live editions)
tags = _tags(
title='Auntie Diaries',
track=6,
disc=2,
)
# Compute the title-only contribution to verify the test's premise:
# title agreement is moderate, NOT high enough on its own to clear
# MATCH_THRESHOLD. The consolation has to be load-bearing.
title_only_score = _sim(
'Auntie Diaries', 'Auntie Diaries (Remix)',
) * TITLE_WEIGHT
assert title_only_score < MATCH_THRESHOLD, (
f"Test premise broken — title sim alone ({title_only_score:.3f}) "
f"already clears MATCH_THRESHOLD ({MATCH_THRESHOLD}). The "
f"cross-disc consolation isn't load-bearing for this scenario; "
f"pick a less-similar title pair."
)
score = score_file_against_track(
'/a/file.flac', tags, track,
target_album='', similarity=_sim,
)
assert score >= MATCH_THRESHOLD, (
f"Cross-disc consolation ({CROSS_DISC_POSITION_WEIGHT}) is no "
f"longer enough to push the score across MATCH_THRESHOLD "
f"({MATCH_THRESHOLD}) for imperfect-title cases. Total score: "
f"{score:.3f}. Either bump the consolation OR drop it to 0 "
f"deliberately and accept that these files now go unmatched."
)
def test_score_near_position_only_when_same_disc():
"""Off-by-one track number gets NEAR_POSITION bonus, but ONLY when
disc agrees. Cross-disc off-by-one gets nothing."""
track = {'name': 'Y', 'track_number': 5, 'disc_number': 1, 'artists': []}
same_disc = _tags(title='Y', track=6, disc=1) # off by 1 on same disc
score_same = score_file_against_track(
'/a/f.flac', same_disc, track, target_album='', similarity=_sim,
)
expected_same = TITLE_WEIGHT + NEAR_POSITION_WEIGHT
assert abs(score_same - expected_same) < 0.001
diff_disc = _tags(title='Y', track=6, disc=2) # off by 1, different disc
score_diff = score_file_against_track(
'/a/f.flac', diff_disc, track, target_album='', similarity=_sim,
)
# No position bonus at all (off-by-one + cross-disc)
expected_diff = TITLE_WEIGHT
assert abs(score_diff - expected_diff) < 0.001
def test_score_handles_missing_track_artist():
"""Track with no artists list — artist component just contributes 0."""
track = {'name': 'Z', 'track_number': 1, 'disc_number': 1, 'artists': []}
tags = _tags(title='Z', artist='Real Artist', track=1, disc=1)
score = score_file_against_track(
'/a/f.flac', tags, track, target_album='', similarity=_sim,
)
# Title (1.0) + position (0.30) + no artist bonus + no album
expected = TITLE_WEIGHT + POSITION_WEIGHT
assert abs(score - expected) < 0.001
def test_score_handles_missing_file_artist():
"""File with no artist tag — same as missing track artist, no bonus."""
track = {'name': 'Z', 'track_number': 1, 'disc_number': 1,
'artists': [{'name': 'Artist'}]}
tags = _tags(title='Z', artist='', track=1, disc=1)
score = score_file_against_track(
'/a/f.flac', tags, track, target_album='', similarity=_sim,
)
expected = TITLE_WEIGHT + POSITION_WEIGHT
assert abs(score - expected) < 0.001
def test_score_disc_field_aliases():
"""API track disc number can come from disc_number / disk_number /
discNumber depending on source. All three should be honored."""
tags = _tags(title='X', track=1, disc=2)
for disc_field in ('disc_number', 'disk_number', 'discNumber'):
track = {'name': 'X', 'track_number': 1, disc_field: 2, 'artists': []}
score = score_file_against_track(
'/a/f.flac', tags, track, target_album='', similarity=_sim,
)
# Should get full POSITION bonus
expected = TITLE_WEIGHT + POSITION_WEIGHT
assert abs(score - expected) < 0.001, (
f"Disc field '{disc_field}' should be recognised (score={score})"
)
def test_score_filename_fallback_when_title_tag_missing():
"""File with no title tag falls back to the filename stem for the
title-similarity comparison."""
track = {'name': 'Filename Title', 'track_number': 0, 'artists': []}
tags = _tags(title='', track=0, disc=1)
score = score_file_against_track(
'/a/Filename Title.flac', tags, track,
target_album='', similarity=_sim,
)
# Title fallback gives perfect match → TITLE_WEIGHT
assert abs(score - TITLE_WEIGHT) < 0.001
# ---------------------------------------------------------------------------
# match_files_to_tracks — end-to-end (still pure)
# ---------------------------------------------------------------------------
def test_match_pairs_files_to_correct_tracks():
"""Happy path — 3 files, 3 tracks, all match perfectly."""
files = ['/a/01.flac', '/a/02.flac', '/a/03.flac']
file_tags = {
'/a/01.flac': _tags(title='A', track=1, disc=1),
'/a/02.flac': _tags(title='B', track=2, disc=1),
'/a/03.flac': _tags(title='C', track=3, disc=1),
}
tracks = [
{'name': 'A', 'track_number': 1, 'disc_number': 1, 'artists': [{'name': 'Artist'}]},
{'name': 'B', 'track_number': 2, 'disc_number': 1, 'artists': [{'name': 'Artist'}]},
{'name': 'C', 'track_number': 3, 'disc_number': 1, 'artists': [{'name': 'Artist'}]},
]
result = match_files_to_tracks(
files, file_tags, tracks,
target_album='Album', similarity=_sim, quality_rank=_qrank,
)
assert len(result['matches']) == 3
assert not result['unmatched_files']
def test_match_each_file_used_at_most_once():
"""Two tracks competing for the same file — only one wins, the
other gets no match."""
files = ['/a/only.flac']
file_tags = {'/a/only.flac': _tags(title='Track Name', track=1, disc=1)}
tracks = [
{'name': 'Track Name', 'track_number': 1, 'disc_number': 1, 'artists': []},
{'name': 'Track Name', 'track_number': 1, 'disc_number': 1, 'artists': []}, # dup
]
result = match_files_to_tracks(
files, file_tags, tracks,
target_album='', similarity=_sim, quality_rank=_qrank,
)
assert len(result['matches']) == 1
def test_match_below_threshold_files_left_unmatched():
"""File with weak title + no other signals should be left in
unmatched_files, not force-matched."""
files = ['/a/random.flac']
file_tags = {'/a/random.flac': _tags(title='Totally Different', track=0, disc=1)}
tracks = [
{'name': 'Specific Track', 'track_number': 99, 'disc_number': 1, 'artists': []},
]
result = match_files_to_tracks(
files, file_tags, tracks,
target_album='', similarity=_sim, quality_rank=_qrank,
)
assert not result['matches']
assert result['unmatched_files'] == ['/a/random.flac']
# ---------------------------------------------------------------------------
# Edge case Cin would flag: tag-less file matching against multi-disc API
# ---------------------------------------------------------------------------
def test_tagless_file_matches_disc1_track_with_perfect_title():
"""User has a perfectly-named file with no embedded tags — file
title in the filename matches the metadata title exactly. The
matcher should still pair it correctly even though disc info is
missing on the file side (defaults to disc 1)."""
files = ['/a/Auntie Diaries.flac']
file_tags = {
'/a/Auntie Diaries.flac': _tags(title='', track=0, disc=1), # no tags
}
tracks = [
{'name': 'Auntie Diaries', 'track_number': 6, 'disc_number': 2,
'artists': [{'name': 'Kendrick Lamar'}]},
]
result = match_files_to_tracks(
files, file_tags, tracks,
target_album='Mr. Morale & The Big Steppers',
similarity=_sim, quality_rank=_qrank,
)
# Perfect title sim (1.0 × 0.45 = 0.45) > MATCH_THRESHOLD (0.4)
# → file matches the track even with missing position info
assert len(result['matches']) == 1
assert result['matches'][0]['file'] == '/a/Auntie Diaries.flac'
def test_tagless_files_against_multidisc_album_partial_match():
"""Two tag-less files with strong filename titles (one matches a
disc-1 track, one matches a disc-2 track). Both should match
correctly via title — no disc info needed."""
files = ['/a/Father Time.flac', '/a/Mother I Sober.flac']
file_tags = {f: _tags(title='', track=0, disc=1) for f in files}
tracks = [
{'name': 'Father Time', 'track_number': 5, 'disc_number': 1, 'artists': []},
{'name': 'Mother I Sober', 'track_number': 8, 'disc_number': 2, 'artists': []},
]
result = match_files_to_tracks(
files, file_tags, tracks,
target_album='Mr. Morale', similarity=_sim, quality_rank=_qrank,
)
assert len(result['matches']) == 2
by_track = {m['track']['name']: m['file'] for m in result['matches']}
assert by_track['Father Time'] == '/a/Father Time.flac'
assert by_track['Mother I Sober'] == '/a/Mother I Sober.flac'
def test_tagless_file_with_weak_title_unmatched_in_multidisc():
"""Edge case Cin would flag: tag-less file (so disc defaults to 1)
with a weak filename title against a disc-2-only API track. Pre-fix,
the position bonus fired on track_number alone, so files like this
would sneak matches via just track_number agreement. Post-fix, the
cross-disc consolation (5%) plus weak title can fall below
MATCH_THRESHOLD → file goes unmatched.
This is the BEHAVIOR CHANGE worth pinning. For correctly-tagged
files in multi-disc albums (the user's actual case) this is the
right call. For users with weak tags this is a regression — they
now have to rely on title or fix their tags."""
files = ['/a/track06.flac'] # weak title, no tags
file_tags = {
'/a/track06.flac': _tags(title='', track=6, disc=1), # disc defaults to 1
}
tracks = [
# API has only this disc-2 track 6 — file's disc-1-track-6
# signal would have fired full position bonus pre-fix
{'name': 'Auntie Diaries', 'track_number': 6, 'disc_number': 2,
'artists': [{'name': 'Kendrick Lamar'}]},
]
result = match_files_to_tracks(
files, file_tags, tracks,
target_album='Mr. Morale', similarity=_sim, quality_rank=_qrank,
)
# Title sim "track06" vs "Auntie Diaries" is near zero (~0.10)
# × 0.45 = ~0.045. Plus cross-disc 0.05 = ~0.095. Below 0.4
# threshold → no match.
assert not result['matches']
assert result['unmatched_files'] == ['/a/track06.flac']

493
tests/imports/test_auto_import_context_shape.py Normal file
View file

@ -0,0 +1,493 @@
"""Pin the post-process context dict the auto-import worker hands to
``_post_process_matched_download``.
Background
----------
Auto-import doesn't write to the SoulSync standalone DB itself —
it routes every matched track through the same
``_post_process_matched_download`` callback the regular download
flow uses. The pipeline downstream (``record_soulsync_library_entry``,
``record_library_history_download``, ``record_download_provenance``)
reads:
- ``context["source"]`` — picks the right source-id columns
(``spotify_track_id`` / ``deezer_id`` / ``itunes_track_id`` / etc.)
- ``context["_download_username"]`` — labels the row in library
history + provenance ("Auto-Import" instead of falling back to the
Soulseek default).
- ``context["track_info"]["musicbrainz_recording_id"]`` and
``context["track_info"]["isrc"]`` — per-recording IDs that land on
the dedicated ``musicbrainz_recording_id`` / ``isrc`` track columns.
If the worker drops any of these, the soulsync library row gets
written but with NULL on every source-id column, and library history
mislabels every imported file as a Soulseek download. SoulSync
standalone is meant to be a full server replacement so it must reach
parity with what a Plex / Jellyfin / Navidrome scan would write. These
tests pin that contract at the worker boundary.
"""
from __future__ import annotations
from dataclasses import dataclass, field
from typing import Any, Dict, List
from unittest.mock import MagicMock
import pytest
@dataclass
class _FakeCandidate:
path: str
name: str
audio_files: List[str] = field(default_factory=list)
disc_structure: Dict[int, List[str]] = field(default_factory=dict)
folder_hash: str = "fake-hash"
is_single: bool = False
@pytest.fixture
def worker_with_capture(tmp_path):
"""Worker whose ``process_callback`` captures the per-track context
dict so the test can assert on its shape directly."""
from core.auto_import_worker import AutoImportWorker
captured: List[Dict[str, Any]] = []
fake_db = MagicMock()
fake_cfg = MagicMock()
fake_cfg.get.side_effect = lambda key, default=None: default
def _capture(_key, ctx, _path):
captured.append(ctx)
worker = AutoImportWorker(
database=fake_db,
staging_path=str(tmp_path),
transfer_path=str(tmp_path / "transfer"),
process_callback=_capture,
config_manager=fake_cfg,
automation_engine=None,
)
worker._captured = captured
return worker
def _make_match_result(source: str, track_count: int = 1) -> Dict[str, Any]:
return {
"matches": [], # filled by tests
"unmatched_files": [],
"total_tracks": track_count,
"matched_count": track_count,
"confidence": 0.95,
"album_data": {
"id": "ALBUM-ID-FROM-SOURCE",
"total_tracks": track_count,
"album_type": "album",
"release_date": "2024-01-01",
"images": [{"url": "https://img.example/cover.jpg"}],
"artists": [{"name": "A", "id": "ARTIST-ID-FROM-SOURCE"}],
},
}
def _make_identification(source: str = "deezer") -> Dict[str, Any]:
return {
"source": source,
"artist_name": "A",
"artist_id": "ARTIST-ID-FROM-SOURCE",
"album_name": "Album",
"album_id": "ALBUM-ID-FROM-SOURCE",
"image_url": "https://img.example/cover.jpg",
"release_date": "2024-01-01",
"method": "tags",
}
# ---------------------------------------------------------------------------
# context["source"] propagation
# ---------------------------------------------------------------------------
@pytest.mark.parametrize("source", ["spotify", "deezer", "itunes", "discogs"])
def test_context_carries_source(worker_with_capture, tmp_path, source):
"""Worker must propagate ``identification['source']`` onto the
top-level context. Without it, ``record_soulsync_library_entry``
can't pick a source column and writes the row with NULL on every
source-id field."""
f = tmp_path / "01.flac"
f.write_bytes(b"audio")
cand = _FakeCandidate(path=str(tmp_path), name="Album")
ident = _make_identification(source=source)
mr = _make_match_result(source, 1)
mr["matches"] = [{
"track": {"id": "t1", "name": "Track", "track_number": 1,
"disc_number": 1, "duration_ms": 200000,
"artists": [{"name": "A"}]},
"file": str(f), "confidence": 0.95,
}]
worker_with_capture._process_matches(cand, ident, mr)
ctx = worker_with_capture._captured[0]
assert ctx["source"] == source, (
f"Expected context['source']={source!r}, got {ctx.get('source')!r}. "
f"Without this, soulsync library writes the row with NULL on "
f"{source}_track_id."
)
# ---------------------------------------------------------------------------
# Auto-import labels: history + provenance must NOT default to Soulseek
# ---------------------------------------------------------------------------
def test_context_marks_download_username_as_auto_import(worker_with_capture, tmp_path):
"""``_download_username='auto_import'`` is what triggers the
"Auto-Import" / "auto_import" branch in side_effects.py source maps.
Without it, every imported file is labelled as a Soulseek download
in library history + provenance — false signal in the UI."""
f = tmp_path / "01.flac"
f.write_bytes(b"audio")
cand = _FakeCandidate(path=str(tmp_path), name="Album")
ident = _make_identification("spotify")
mr = _make_match_result("spotify", 1)
mr["matches"] = [{
"track": {"id": "t1", "name": "Track", "track_number": 1,
"disc_number": 1, "duration_ms": 200000,
"artists": [{"name": "A"}]},
"file": str(f), "confidence": 0.95,
}]
worker_with_capture._process_matches(cand, ident, mr)
ctx = worker_with_capture._captured[0]
assert ctx.get("_download_username") == "auto_import"
# ---------------------------------------------------------------------------
# Per-recording IDs flow through to track_info
# ---------------------------------------------------------------------------
def test_context_propagates_isrc_and_mbid_when_present(worker_with_capture, tmp_path):
"""When the metadata-source response carries per-recording IDs
(Picard-tagged libraries always have MBID, MusicBrainz-enriched
Spotify carries ISRC), they must end up on
context['track_info']['isrc'] / ['musicbrainz_recording_id'] so
the soulsync library row writes them onto dedicated columns."""
f = tmp_path / "01.flac"
f.write_bytes(b"audio")
cand = _FakeCandidate(path=str(tmp_path), name="Album")
ident = _make_identification("spotify")
mr = _make_match_result("spotify", 1)
mr["matches"] = [{
"track": {
"id": "spotify-track-id",
"name": "Track",
"track_number": 1,
"disc_number": 1,
"duration_ms": 200000,
"artists": [{"name": "A"}],
"isrc": "USABC1234567",
"musicbrainz_recording_id": "abcd1234-mbid-uuid-form",
},
"file": str(f), "confidence": 0.95,
}]
worker_with_capture._process_matches(cand, ident, mr)
ti = worker_with_capture._captured[0]["track_info"]
assert ti["isrc"] == "USABC1234567"
assert ti["musicbrainz_recording_id"] == "abcd1234-mbid-uuid-form"
def test_context_per_recording_ids_default_empty_when_missing(worker_with_capture, tmp_path):
"""Missing IDs default to empty string, NOT to None — the side-
effects layer normalises to None at write time. Empty string keeps
the field present in the dict so downstream code that does
`track_info.get("isrc")` doesn't have to special-case missing keys."""
f = tmp_path / "01.flac"
f.write_bytes(b"audio")
cand = _FakeCandidate(path=str(tmp_path), name="Album")
ident = _make_identification("deezer")
mr = _make_match_result("deezer", 1)
mr["matches"] = [{
"track": {"id": "111", "name": "Track", "track_number": 1,
"disc_number": 1, "duration_ms": 200000,
"artists": [{"name": "A"}]}, # no isrc / mbid
"file": str(f), "confidence": 0.95,
}]
worker_with_capture._process_matches(cand, ident, mr)
ti = worker_with_capture._captured[0]["track_info"]
assert ti.get("isrc") == ""
assert ti.get("musicbrainz_recording_id") == ""
# ---------------------------------------------------------------------------
# Album back-reference on track_info
# ---------------------------------------------------------------------------
def test_track_info_includes_album_id_back_reference(worker_with_capture, tmp_path):
"""`get_import_source_ids` reads `track_info.album_id` as one of the
fallback paths for resolving the album-source-id. Without the back
reference, sources whose API response nests album under
`track.album.id` fall through and the soulsync row writes NULL on
the album-source-id column."""
f = tmp_path / "01.flac"
f.write_bytes(b"audio")
cand = _FakeCandidate(path=str(tmp_path), name="Album")
ident = _make_identification("spotify")
mr = _make_match_result("spotify", 1)
mr["matches"] = [{
"track": {"id": "t1", "name": "Track", "track_number": 1,
"disc_number": 1, "duration_ms": 200000,
"artists": [{"name": "A"}]},
"file": str(f), "confidence": 0.95,
}]
worker_with_capture._process_matches(cand, ident, mr)
ti = worker_with_capture._captured[0]["track_info"]
assert ti.get("album_id") == "ALBUM-ID-FROM-SOURCE"
# ---------------------------------------------------------------------------
# Artist source-id propagation — identification dict → context → DB write
# ---------------------------------------------------------------------------
def test_context_artist_id_uses_identification_artist_id(worker_with_capture, tmp_path):
"""When `identification` carries `artist_id` (from the metadata
source's search response), it must end up on
`context['spotify_artist']['id']` so the standalone library write
populates the `<source>_artist_id` column on the artists row.
Before this fix the worker put `identification['album_id']` into
that field — a copy-paste bug that wrote the album ID into the
artist's source-ID column. Honest pin: artist_id flows from
identification through to context, no falsey fallback."""
f = tmp_path / "01.flac"
f.write_bytes(b"audio")
cand = _FakeCandidate(path=str(tmp_path), name="Album")
ident = _make_identification("spotify")
ident["artist_id"] = "SPOTIFY-ARTIST-ID-XYZ"
ident["album_id"] = "SPOTIFY-ALBUM-ID-DIFFERENT"
mr = _make_match_result("spotify", 1)
mr["matches"] = [{
"track": {"id": "t1", "name": "Track", "track_number": 1,
"disc_number": 1, "duration_ms": 200000,
"artists": [{"name": "A"}]},
"file": str(f), "confidence": 0.95,
}]
worker_with_capture._process_matches(cand, ident, mr)
ctx = worker_with_capture._captured[0]
assert ctx["spotify_artist"]["id"] == "SPOTIFY-ARTIST-ID-XYZ", (
"spotify_artist['id'] should hold the artist's source ID, NOT "
"the album_id (regression case for the prior copy-paste bug)."
)
# Album artists list must also carry the artist source ID so
# `get_import_source_ids` can resolve it via the album→artists
# fallback path.
assert ctx["spotify_album"]["artists"][0]["id"] == "SPOTIFY-ARTIST-ID-XYZ"
def test_context_artist_id_is_empty_when_identification_missing_it(worker_with_capture, tmp_path):
"""When the identification dict doesn't surface artist_id (e.g.
filename-only identification fallback), context falls back to
empty string — NOT to album_id (the prior wrong fallback)."""
f = tmp_path / "01.flac"
f.write_bytes(b"audio")
cand = _FakeCandidate(path=str(tmp_path), name="Album")
ident = _make_identification("spotify")
ident.pop("artist_id", None) # force no artist_id
ident["album_id"] = "SOME-ALBUM-ID"
mr = _make_match_result("spotify", 1)
mr["matches"] = [{
"track": {"id": "t1", "name": "Track", "track_number": 1,
"disc_number": 1, "duration_ms": 200000,
"artists": [{"name": "A"}]},
"file": str(f), "confidence": 0.95,
}]
worker_with_capture._process_matches(cand, ident, mr)
ctx = worker_with_capture._captured[0]
assert ctx["spotify_artist"]["id"] == "", (
"spotify_artist['id'] must be empty (NULL on the artists row) "
"when the identification dict has no artist_id. It must NEVER "
"fall back to album_id — that was the bug this PR fixed."
)
# ---------------------------------------------------------------------------
# Genre aggregation — soulsync standalone parity with deep-scan behaviour
# ---------------------------------------------------------------------------
def test_context_aggregates_genres_from_track_tags(worker_with_capture, tmp_path, monkeypatch):
"""Worker reads GENRE tag from each matched file and surfaces a
deduped list on `spotify_artist['genres']`. Mirrors what
`soulsync_client._scan_transfer` does at deep-scan time so the
standalone library write populates the artists row's genres
column instead of leaving it empty (which is what plex/jellyfin/
navidrome scans would have provided)."""
from core import auto_import_worker as worker_mod
files = []
for i in range(1, 4):
f = tmp_path / f"0{i}.flac"
f.write_bytes(b"audio")
files.append(f)
# Stub `_read_file_tags` so we don't need real audio. Each file
# carries a different (overlapping) genre set — deduped result
# should preserve insertion order + original casing.
fake_tags = {
str(files[0]): {'genres': ['Hip-Hop', 'Rap'], 'isrc': '', 'mbid': '',
'duration_ms': 200000, 'title': 'A', 'artist': 'X',
'album': 'Album', 'track_number': 1, 'disc_number': 1, 'year': ''},
str(files[1]): {'genres': ['Rap', 'Trap'], 'isrc': '', 'mbid': '',
'duration_ms': 200000, 'title': 'B', 'artist': 'X',
'album': 'Album', 'track_number': 2, 'disc_number': 1, 'year': ''},
str(files[2]): {'genres': ['hip-hop'], 'isrc': '', 'mbid': '', # case-insensitive dup
'duration_ms': 200000, 'title': 'C', 'artist': 'X',
'album': 'Album', 'track_number': 3, 'disc_number': 1, 'year': ''},
}
monkeypatch.setattr(worker_mod, '_read_file_tags',
lambda path: fake_tags.get(str(path), {'genres': []}))
cand = _FakeCandidate(path=str(tmp_path), name="Album",
audio_files=[str(f) for f in files])
ident = _make_identification("spotify")
mr = _make_match_result("spotify", 3)
mr["matches"] = [
{"track": {"id": f"t{i}", "name": f"Track {i}", "track_number": i,
"disc_number": 1, "duration_ms": 200000,
"artists": [{"name": "X"}]},
"file": str(files[i - 1]), "confidence": 0.95}
for i in range(1, 4)
]
worker_with_capture._process_matches(cand, ident, mr)
ctx = worker_with_capture._captured[0]
genres = ctx["spotify_artist"]["genres"]
# Insertion-order preserved: Hip-Hop (file 1), Rap (file 1), Trap (file 2).
# 'hip-hop' from file 3 deduped against 'Hip-Hop' (case-insensitive).
assert genres == ["Hip-Hop", "Rap", "Trap"], (
f"Expected deduped insertion-order genres, got {genres}"
)
def test_context_genres_empty_when_no_tags(worker_with_capture, tmp_path, monkeypatch):
"""No GENRE tag on any file → empty list. Standalone library write
handles empty list gracefully (genres column stays empty / NULL)."""
from core import auto_import_worker as worker_mod
f = tmp_path / "01.flac"
f.write_bytes(b"audio")
monkeypatch.setattr(worker_mod, '_read_file_tags',
lambda path: {'genres': [], 'isrc': '', 'mbid': '',
'duration_ms': 200000, 'title': '', 'artist': '',
'album': '', 'track_number': 1, 'disc_number': 1, 'year': ''})
cand = _FakeCandidate(path=str(tmp_path), name="Album", audio_files=[str(f)])
ident = _make_identification("spotify")
mr = _make_match_result("spotify", 1)
mr["matches"] = [{
"track": {"id": "t1", "name": "Track", "track_number": 1,
"disc_number": 1, "duration_ms": 200000,
"artists": [{"name": "A"}]},
"file": str(f), "confidence": 0.95,
}]
worker_with_capture._process_matches(cand, ident, mr)
assert worker_with_capture._captured[0]["spotify_artist"]["genres"] == []
# ---------------------------------------------------------------------------
# Defensive ISRC/MBID type coercion
# ---------------------------------------------------------------------------
def test_context_isrc_mbid_coerced_to_string(worker_with_capture, tmp_path):
"""If a metadata source returns ISRC or MBID as int / non-string
(no current source does, but defensive against future drift),
the worker coerces to string before assignment so the side-effects
layer's `.strip()` doesn't AttributeError."""
f = tmp_path / "01.flac"
f.write_bytes(b"audio")
cand = _FakeCandidate(path=str(tmp_path), name="Album", audio_files=[str(f)])
ident = _make_identification("deezer")
mr = _make_match_result("deezer", 1)
mr["matches"] = [{
"track": {
"id": "111",
"name": "Track",
"track_number": 1,
"disc_number": 1,
"duration_ms": 200000,
"artists": [{"name": "A"}],
# Hostile types: ints / None — must not propagate
# through to side_effects un-cast.
"isrc": 12345678,
"mbid": None,
"musicbrainz_recording_id": 999,
},
"file": str(f), "confidence": 0.95,
}]
worker_with_capture._process_matches(cand, ident, mr)
ti = worker_with_capture._captured[0]["track_info"]
assert isinstance(ti["isrc"], str)
assert isinstance(ti["musicbrainz_recording_id"], str)
# int 12345678 → "12345678", int 999 → "999"
assert ti["isrc"] == "12345678"
assert ti["musicbrainz_recording_id"] == "999"
def test_search_metadata_source_extracts_artist_id_from_dict_artist():
"""`_search_metadata_source` must extract the artist source ID
from `best_result.artists[0]['id']` so identification carries it
forward. Without this, the worker's context-shape contract above
is satisfied syntactically but the DB always sees empty."""
from core.auto_import_worker import AutoImportWorker, FolderCandidate
from unittest.mock import patch, MagicMock
fake_album = MagicMock()
fake_album.id = "ALBUM-ID"
fake_album.name = "Test Album"
fake_album.artists = [{"id": "ARTIST-SRC-ID", "name": "Test Artist"}]
fake_album.image_url = "https://img.example/cover.jpg"
fake_album.release_date = "2024-01-01"
fake_album.total_tracks = 10
fake_client = MagicMock()
fake_client.search_albums.return_value = [fake_album]
candidate = FolderCandidate(
path="/staging/album", name="Test Album",
audio_files=[f"/staging/album/0{i}.flac" for i in range(1, 11)],
)
worker = AutoImportWorker(database=MagicMock(), process_callback=lambda *a, **k: None)
with patch("core.metadata_service.get_primary_source", return_value="spotify"), \
patch("core.metadata_service.get_client_for_source", return_value=fake_client):
result = worker._search_metadata_source(
"Test Artist", "Test Album", "tags", candidate,
)
assert result is not None
assert result.get("artist_id") == "ARTIST-SRC-ID", (
"_search_metadata_source must extract artist_id from "
"best_result.artists[0]['id'] so the rest of the pipeline "
"can write it to the artists table."
)

511
tests/imports/test_auto_import_executor.py Normal file
View file

@ -0,0 +1,511 @@
"""Pin the bounded-executor + scan-lock concurrency model in
``AutoImportWorker``.
Pre-refactor (before 2026-05-09): manual "Scan Now" clicks spawned a
fresh `threading.Thread(target=_scan_cycle)` per click on top of the
worker's existing 60-second timer-driven scan. Emergent parallelism
with no upper bound, no shared queue, no graceful shutdown. Different
scan cycles raced on `_processing_paths` / `_folder_snapshots` state.
Post-refactor:
- ONE scan at a time (`_scan_lock` non-blocking acquire — duplicate
triggers no-op).
- Per-candidate processing runs on a `ThreadPoolExecutor` (default 3
workers, configurable via `auto_import.max_workers`).
- Both timer + manual triggers share `trigger_scan()` so they go
through the same lock + executor.
These tests pin the CONCURRENCY CONTRACT, not the per-candidate
processing logic (which is covered separately by
``test_auto_import_live_progress.py`` etc.).
"""
from __future__ import annotations
import threading
import time
from unittest.mock import MagicMock, patch
import pytest
from core.auto_import_worker import AutoImportWorker, FolderCandidate
def _make_worker(max_workers: int = 3) -> AutoImportWorker:
"""Bare worker — for the executor/lock tests we don't need full
db / config / process_callback dependencies."""
return AutoImportWorker(
database=MagicMock(),
process_callback=MagicMock(),
max_workers=max_workers,
)
def _make_candidate(folder_hash: str = 'h1', name: str = 'TestAlbum') -> FolderCandidate:
return FolderCandidate(
path=f'/staging/{name}',
name=name,
audio_files=[f'/staging/{name}/01.flac'],
folder_hash=folder_hash,
)
# ---------------------------------------------------------------------------
# Pool configuration
# ---------------------------------------------------------------------------
def test_default_max_workers_is_three():
"""Match the existing pool patterns in this codebase
(missing_download_executor, sync_executor, import_singles_executor
all default to 3)."""
w = _make_worker()
assert w._max_workers == 3
def test_max_workers_configurable_via_constructor():
w = _make_worker(max_workers=5)
assert w._max_workers == 5
def test_max_workers_floors_at_one():
"""0 or negative pool size would deadlock anything submitted —
floor at 1 so a misconfigured value still works."""
w = _make_worker(max_workers=0)
assert w._max_workers == 1
def test_max_workers_pulled_from_config_when_provided():
config = MagicMock()
config.get = MagicMock(side_effect=lambda key, default: 7 if key == 'auto_import.max_workers' else default)
w = AutoImportWorker(
database=MagicMock(),
process_callback=MagicMock(),
config_manager=config,
max_workers=3, # constructor default — overridden by config
)
assert w._max_workers == 7
# ---------------------------------------------------------------------------
# Scan lock — duplicate triggers no-op
# ---------------------------------------------------------------------------
def test_concurrent_triggers_only_one_scan_runs(monkeypatch):
"""Pre-refactor regression case: hitting "Scan Now" 5× in quick
succession used to spawn 5 parallel scan cycles. Post-refactor:
only one runs, the rest no-op via the non-blocking lock."""
w = _make_worker()
scan_count = 0
scan_started = threading.Event()
scan_can_finish = threading.Event()
def fake_scan_and_submit():
nonlocal scan_count
scan_count += 1
scan_started.set()
scan_can_finish.wait(timeout=5)
monkeypatch.setattr(w, '_scan_and_submit', fake_scan_and_submit)
# Fire 5 trigger_scan calls in parallel
threads = [threading.Thread(target=w.trigger_scan) for _ in range(5)]
for t in threads:
t.start()
# Wait for the first scan to start
assert scan_started.wait(timeout=5)
# The other 4 should have already returned (lock was held)
time.sleep(0.1)
assert scan_count == 1, (
f"Expected exactly 1 scan to run while the lock was held, got "
f"{scan_count}. The non-blocking scan lock isn't gating "
f"duplicate triggers."
)
# Release the held scan
scan_can_finish.set()
for t in threads:
t.join(timeout=5)
# No additional scans started after release (the 4 losers gave up,
# didn't queue)
assert scan_count == 1
def test_scan_after_previous_finishes_runs_normally(monkeypatch):
"""Lock releases when scan finishes — next trigger should acquire
+ run normally, not be permanently blocked."""
w = _make_worker()
scan_count = 0
def fake_scan_and_submit():
nonlocal scan_count
scan_count += 1
monkeypatch.setattr(w, '_scan_and_submit', fake_scan_and_submit)
w.trigger_scan()
w.trigger_scan()
w.trigger_scan()
assert scan_count == 3
# ---------------------------------------------------------------------------
# Executor — per-candidate parallelism
# ---------------------------------------------------------------------------
def test_candidates_dispatched_to_executor(monkeypatch):
"""Scan finds N candidates → submits N tasks to the executor pool.
Pool runs them in parallel (up to max_workers). Each task ends up
calling `_process_one_candidate`."""
w = _make_worker(max_workers=3)
w.start() # initialises the executor
try:
candidates = [
_make_candidate(folder_hash=f'h{i}', name=f'Album{i}')
for i in range(5)
]
monkeypatch.setattr(w, '_enumerate_folders', lambda staging: candidates)
monkeypatch.setattr(w, '_resolve_staging_path', lambda: '/staging')
monkeypatch.setattr('core.auto_import_worker.os.path.isdir', lambda p: True)
monkeypatch.setattr(w, '_is_already_processed', lambda h: False)
monkeypatch.setattr(w, '_is_folder_stable', lambda c: True)
processed = []
processed_lock = threading.Lock()
def fake_process(candidate):
with processed_lock:
processed.append(candidate.folder_hash)
monkeypatch.setattr(w, '_process_one_candidate', fake_process)
w.trigger_scan()
# Wait for all 5 to finish (executor runs async)
deadline = time.time() + 5
while len(processed) < 5 and time.time() < deadline:
time.sleep(0.05)
assert sorted(processed) == [f'h{i}' for i in range(5)]
finally:
w.stop()
def test_pool_runs_candidates_in_parallel():
"""With max_workers=3, the pool should run up to 3 candidates
concurrently — proves the bounded parallelism the user asked for."""
w = _make_worker(max_workers=3)
w.start()
try:
# Submit 3 long-running tasks directly to the executor and
# confirm they run concurrently.
in_flight = [0]
peak_in_flight = [0]
lock = threading.Lock()
proceed = threading.Event()
def slow_task():
with lock:
in_flight[0] += 1
if in_flight[0] > peak_in_flight[0]:
peak_in_flight[0] = in_flight[0]
proceed.wait(timeout=2)
with lock:
in_flight[0] -= 1
futures = [w._executor.submit(slow_task) for _ in range(3)]
# Give them a beat to start
time.sleep(0.2)
assert peak_in_flight[0] == 3, (
f"Expected 3 concurrent tasks, peaked at {peak_in_flight[0]}"
)
proceed.set()
for f in futures:
f.result(timeout=2)
finally:
w.stop()
def test_executor_max_workers_caps_concurrency():
"""max_workers=2 must NOT allow 3 concurrent tasks. Bounded
parallelism — predictable system load."""
w = _make_worker(max_workers=2)
w.start()
try:
in_flight = [0]
peak = [0]
lock = threading.Lock()
proceed = threading.Event()
def slow_task():
with lock:
in_flight[0] += 1
if in_flight[0] > peak[0]:
peak[0] = in_flight[0]
proceed.wait(timeout=2)
with lock:
in_flight[0] -= 1
futures = [w._executor.submit(slow_task) for _ in range(5)]
time.sleep(0.3)
assert peak[0] == 2, (
f"max_workers=2 should cap concurrency at 2, peaked at {peak[0]}"
)
proceed.set()
for f in futures:
f.result(timeout=2)
finally:
w.stop()
# ---------------------------------------------------------------------------
# Submitted-hashes dedup across triggers
# ---------------------------------------------------------------------------
def test_candidate_only_submitted_once_across_concurrent_scans(monkeypatch):
"""Scenario: scan A submits candidate X to the pool; pool worker
is mid-processing. Scan B (manual trigger) enumerates again and
sees X — must NOT re-submit. `_submitted_hashes` set + lock
prevents double-submission."""
w = _make_worker()
w.start()
try:
cand = _make_candidate(folder_hash='shared-hash')
monkeypatch.setattr(w, '_enumerate_folders', lambda staging: [cand])
monkeypatch.setattr(w, '_resolve_staging_path', lambda: '/staging')
monkeypatch.setattr('core.auto_import_worker.os.path.isdir', lambda p: True)
monkeypatch.setattr(w, '_is_already_processed', lambda h: False)
monkeypatch.setattr(w, '_is_folder_stable', lambda c: True)
process_count = 0
process_lock = threading.Lock()
process_can_finish = threading.Event()
def slow_process(candidate):
nonlocal process_count
with process_lock:
process_count += 1
process_can_finish.wait(timeout=5)
monkeypatch.setattr(w, '_process_one_candidate', slow_process)
# First scan submits the candidate
w.trigger_scan()
# Wait for processing to start
time.sleep(0.1)
# Second scan WHILE first is processing — must not re-submit
w.trigger_scan()
time.sleep(0.1)
assert process_count == 1, (
f"Expected only 1 process call (dedup active), got {process_count}"
)
process_can_finish.set()
time.sleep(0.2)
# After the first finishes, the candidate still has the same
# hash + would be `_is_already_processed`, but our mock returns
# False — even so, the post-finally `discard` should let a
# third trigger re-pick if needed. Here we just verify dedup
# held while in flight.
finally:
process_can_finish.set()
w.stop()
# ---------------------------------------------------------------------------
# Graceful shutdown
# ---------------------------------------------------------------------------
def test_stop_waits_for_inflight_pool_work():
"""`stop()` must call `executor.shutdown(wait=True)` so in-flight
file moves / tag writes / DB inserts complete before shutdown
reports done. Otherwise interrupted writes corrupt state."""
w = _make_worker()
w.start()
finished = threading.Event()
def slow_task():
time.sleep(0.3)
finished.set()
w._executor.submit(slow_task)
# Stop immediately — should block until slow_task completes
w.stop()
assert finished.is_set(), (
"stop() returned before in-flight pool work finished — "
"executor shutdown(wait=True) is missing or broken"
)
# ---------------------------------------------------------------------------
# Per-candidate state isolation under parallel pool workers
# ---------------------------------------------------------------------------
#
# Pre-refactor `_current_folder` / `_current_track_*` / `_current_status` were
# scalar fields on the worker. Three pool workers running in parallel would
# stomp each other's values — UI showed "Processing AlbumA, track 7/14:
# SongFromAlbumB" interleaved garbage. These tests pin the per-candidate
# isolation introduced by the `_active_imports` dict + `_active_lock`.
def test_concurrent_candidates_dont_stomp_each_other():
"""Two pool workers updating their own candidate state must not
interfere — each candidate's track_index / track_name / folder_name
is read back exactly as written for that hash."""
w = _make_worker(max_workers=2)
w.start()
try:
cand_a = _make_candidate(folder_hash='hA', name='AlbumA')
cand_b = _make_candidate(folder_hash='hB', name='AlbumB')
# Register both
w._register_active(cand_a, status='processing')
w._register_active(cand_b, status='processing')
ready = threading.Barrier(2)
done = threading.Event()
def worker_for(cand, name_prefix, total):
ready.wait(timeout=2)
for i in range(1, total + 1):
w._update_active(
cand.folder_hash,
track_index=i,
track_total=total,
track_name=f'{name_prefix}-track-{i}',
)
# Tight loop so the two threads interleave aggressively
time.sleep(0.001)
ta = threading.Thread(target=worker_for, args=(cand_a, 'A', 50))
tb = threading.Thread(target=worker_for, args=(cand_b, 'B', 50))
ta.start(); tb.start()
ta.join(timeout=5); tb.join(timeout=5)
done.set()
snap = w._snapshot_active()
by_hash = {a['folder_hash']: a for a in snap}
assert by_hash['hA']['folder_name'] == 'AlbumA', (
"Candidate A's folder_name was overwritten by a parallel candidate — "
f"got {by_hash['hA']['folder_name']!r}"
)
assert by_hash['hB']['folder_name'] == 'AlbumB', (
"Candidate B's folder_name was overwritten — "
f"got {by_hash['hB']['folder_name']!r}"
)
assert by_hash['hA']['track_index'] == 50
assert by_hash['hB']['track_index'] == 50
assert by_hash['hA']['track_name'].startswith('A-')
assert by_hash['hB']['track_name'].startswith('B-')
finally:
w.stop()
def test_get_status_returns_coherent_active_imports_array():
"""`get_status()` must return one entry per in-flight candidate
with the right per-candidate fields — the polling UI reads this
array to render multiple in-flight imports simultaneously."""
w = _make_worker(max_workers=3)
w.start()
try:
for i, name in enumerate(['One', 'Two', 'Three']):
cand = _make_candidate(folder_hash=f'h{i}', name=name)
w._register_active(cand, status='processing')
w._update_active(cand.folder_hash, track_index=i + 1, track_total=10)
status = w.get_status()
active = status.get('active_imports') or []
assert len(active) == 3
names = {a['folder_name'] for a in active}
assert names == {'One', 'Two', 'Three'}
# Aggregate top-level should be 'processing' (any active is
# processing → processing wins)
assert status['current_status'] == 'processing'
# Legacy single-import scalars: populated from the FIRST
# active entry (insertion order) so the existing UI keeps
# working when only one candidate is in flight.
assert status['current_folder'] == 'One'
assert status['current_track_index'] == 1
assert status['current_track_total'] == 10
finally:
w.stop()
def test_unregister_removes_only_that_candidate():
"""`_unregister_active(hash)` removes one entry; others stay
visible. Pool workers finishing in any order must not affect
other in-flight candidates' UI state."""
w = _make_worker()
w.start()
try:
for i, name in enumerate(['X', 'Y', 'Z']):
w._register_active(_make_candidate(folder_hash=f'k{i}', name=name))
w._unregister_active('k1')
snap = w._snapshot_active()
names = {a['folder_name'] for a in snap}
assert names == {'X', 'Z'}, f"Unexpected snapshot after unregister: {snap}"
finally:
w.stop()
# ---------------------------------------------------------------------------
# Stats counter integrity under parallel bumps
# ---------------------------------------------------------------------------
def test_stats_increments_are_thread_safe():
"""`self._stats[k] += 1` from multiple threads is read-modify-
write — under load the counters drift. `_bump_stat` wraps every
mutation in `_stats_lock` so 1000 parallel bumps land at 1000."""
w = _make_worker()
iterations = 200
threads_count = 5
expected = iterations * threads_count
def hammer():
for _ in range(iterations):
w._bump_stat('scanned')
threads = [threading.Thread(target=hammer) for _ in range(threads_count)]
for t in threads:
t.start()
for t in threads:
t.join(timeout=5)
assert w._stats['scanned'] == expected, (
f"Lost increments: expected {expected}, got {w._stats['scanned']}. "
f"Stats counter is not thread-safe."
)
def test_get_status_stats_snapshot_is_consistent():
"""`get_status()` reads stats under the same lock that mutations
use, so the returned snapshot can't show a partial mid-update
state. Verify the snapshot is a copy (not a live reference)."""
w = _make_worker()
w._bump_stat('scanned')
snap = w.get_status()['stats']
snap['scanned'] = 9999
# Mutating the snapshot must not affect the worker's internal stats
assert w._stats['scanned'] == 1, (
"get_status() returned a live reference to _stats — "
"callers can corrupt internal state."
)

298
tests/imports/test_auto_import_multi_disc_matching.py Normal file
View file

@ -0,0 +1,298 @@
"""Regression test for the multi-disc auto-import matching bug.
User report (2026-05-08, Mr. Morale & The Big Steppers): an album with
multiple discs got dumped into staging — discs 1 and 2 loose in the
root, disc 3 in its own folder, every file perfectly tagged with
``disc_number`` + ``track_number`` + ``title``. Auto-import processed
only 9 tracks total instead of all 27.
Two bugs in ``AutoImportWorker._match_tracks`` caused it:
1. **Quality dedup keyed on track_number alone.** The dedup loop kept
``seen_track_nums[track_number] = file`` and dropped any later file
with the same number, treating it as a quality duplicate. On a
multi-disc release where every disc has tracks 1..N, that collapses
the album to one disc's worth of files before matching even runs —
half (or more) of the tracks vanish before the matcher sees them.
2. **Match scoring ignored disc_number.** The 30% track-number bonus
fired whenever ``ft['track_number'] == track_num`` regardless of disc.
File with tag ``(disc=2, track=6)`` (Auntie Diaries, 281s) got the
full bonus when matched against API track ``(disc=1, track=6)`` (Rich
Interlude, 103s) — wrong file → wrong destination → integrity
check correctly rejected and quarantined the file.
Fix in this PR: dedup keys on ``(disc_number, track_number)`` tuples;
match scoring only awards the 30% bonus when BOTH disc and track
numbers agree, with a small consolation bonus for same-track-number
cross-disc collisions so title similarity still drives the match.
These tests pin both behaviors so multi-disc albums stay intact
through the full import pipeline.
"""
from __future__ import annotations
from typing import Any, Dict
from unittest.mock import MagicMock, patch
import pytest
from core import auto_import_worker as aiw
# ---------------------------------------------------------------------------
# Fixtures — tagged file fakes + worker setup
# ---------------------------------------------------------------------------
def _file_tags(*, disc: int, track: int, title: str, artist: str = 'Kendrick Lamar',
album: str = 'Mr. Morale & The Big Steppers') -> Dict[str, Any]:
"""Build the tag dict shape ``_read_file_tags`` returns."""
return {
'title': title,
'artist': artist,
'album': album,
'track_number': track,
'disc_number': disc,
'year': '2022',
}
def _api_track(disc: int, track: int, title: str, artist: str = 'Kendrick Lamar') -> Dict[str, Any]:
"""Build a track dict matching the shape the metadata source returns."""
return {
'name': title,
'track_number': track,
'disc_number': disc,
'artists': [{'name': artist}],
'id': f'{disc}-{track}',
}
@pytest.fixture
def worker():
"""A worker instance — the matching logic doesn't actually need
most of the worker's deps so we instantiate raw."""
w = aiw.AutoImportWorker.__new__(aiw.AutoImportWorker)
return w
# ---------------------------------------------------------------------------
# Test 1 — dedup must NOT collapse same-track-numbers across discs
# ---------------------------------------------------------------------------
def test_dedup_preserves_files_with_same_track_number_across_different_discs(worker, monkeypatch):
"""The bug: dedup keyed by track_number alone treated disc 1 track 6
and disc 2 track 6 as quality duplicates, dropped one. Fix: key by
(disc_number, track_number) tuple — both files survive dedup.
User scenario: 18 loose files in staging root, all tagged with
``disc_number`` 1 or 2 and ``track_number`` 1..9. Pre-fix the
matcher only saw 9 of them after dedup. Post-fix all 18.
"""
# 18 fake files: discs 1 + 2, tracks 1..9 each
files = [f"/fake/d{disc}_t{track}.flac" for disc in (1, 2) for track in range(1, 10)]
file_tags = {
f: _file_tags(disc=disc, track=track,
title=f'Track {disc}.{track}')
for f, (disc, track) in zip(
files, [(d, t) for d in (1, 2) for t in range(1, 10)],
)
}
# Mock _read_file_tags to return our test tags
monkeypatch.setattr(aiw, '_read_file_tags', lambda f: file_tags[f])
# Mock the metadata client + album fetch to return 18 tracks
api_tracks = [_api_track(disc, track, f'Track {disc}.{track}')
for disc in (1, 2) for track in range(1, 10)]
fake_client = MagicMock()
fake_client.get_album = MagicMock(return_value={
'id': 'album-1',
'name': 'Mr. Morale & The Big Steppers',
'tracks': {'items': api_tracks},
})
candidate = aiw.FolderCandidate(
path='/staging',
name='staging',
audio_files=files,
folder_hash='hash1',
)
identification = {
'source': 'spotify',
'album_id': 'album-1',
'album_name': 'Mr. Morale & The Big Steppers',
'artist_name': 'Kendrick Lamar',
'identification_confidence': 1.0,
}
with patch('core.metadata_service.get_client_for_source', return_value=fake_client), \
patch('core.metadata_service.get_album_tracks_for_source', return_value=None):
result = worker._match_tracks(candidate, identification)
assert result is not None
# All 18 files must end up matched — pre-fix only 9 survived dedup,
# then 4 of those got mismatched and integrity-rejected.
assert len(result['matches']) == 18, (
f"Expected 18 matches across both discs, got {len(result['matches'])}. "
f"Dedup probably collapsed same-track-numbers across discs."
)
# No file should be in unmatched
assert not result['unmatched_files']
# ---------------------------------------------------------------------------
# Test 2 — match scoring respects disc_number
# ---------------------------------------------------------------------------
def test_match_scoring_pairs_files_to_correct_disc(worker, monkeypatch):
"""The bug: the 30% track-number bonus fired regardless of disc, so
files got matched to the wrong-disc track when both shared a track
number. Fix: bonus only when (disc, track) BOTH match.
Pin behavior: file tagged (disc=2, track=6, title='Auntie Diaries')
must match the API's (disc=2, track=6) track, NOT the (disc=1,
track=6) track even though both have track_number=6.
"""
files = [
'/fake/disc1_06.flac', # Rich (Interlude)
'/fake/disc2_06.flac', # Auntie Diaries
]
file_tags = {
'/fake/disc1_06.flac': _file_tags(disc=1, track=6, title='Rich (Interlude)'),
'/fake/disc2_06.flac': _file_tags(disc=2, track=6, title='Auntie Diaries'),
}
monkeypatch.setattr(aiw, '_read_file_tags', lambda f: file_tags[f])
api_tracks = [
_api_track(1, 6, 'Rich (Interlude)'),
_api_track(2, 6, 'Auntie Diaries'),
]
fake_client = MagicMock()
fake_client.get_album = MagicMock(return_value={
'id': 'album-1', 'name': 'Mr. Morale',
'tracks': {'items': api_tracks},
})
candidate = aiw.FolderCandidate(
path='/staging', name='staging',
audio_files=files, folder_hash='hash2',
)
identification = {
'source': 'spotify', 'album_id': 'album-1',
'album_name': 'Mr. Morale', 'artist_name': 'Kendrick Lamar',
'identification_confidence': 1.0,
}
with patch('core.metadata_service.get_client_for_source', return_value=fake_client), \
patch('core.metadata_service.get_album_tracks_for_source', return_value=None):
result = worker._match_tracks(candidate, identification)
assert result is not None
assert len(result['matches']) == 2
# Build a {track_disc: matched_file} map for assertion
by_disc = {
m['track']['disc_number']: m['file'] for m in result['matches']
}
assert by_disc[1] == '/fake/disc1_06.flac', (
"API track (disc=1, track=6) should match the disc-1 file, "
"not get cross-matched to the disc-2 file just because they "
"share track_number=6."
)
assert by_disc[2] == '/fake/disc2_06.flac', (
"API track (disc=2, track=6) should match the disc-2 file."
)
# ---------------------------------------------------------------------------
# Test 3 — single-disc albums still work (regression guard)
# ---------------------------------------------------------------------------
def test_single_disc_albums_still_match_normally(worker, monkeypatch):
"""The disc-aware fix mustn't break single-disc albums where every
file has disc_number=1 (or no disc tag at all → defaults to 1)."""
files = [f'/fake/track_{i:02d}.flac' for i in range(1, 6)]
file_tags = {
f'/fake/track_{i:02d}.flac': _file_tags(disc=1, track=i, title=f'Song {i}')
for i in range(1, 6)
}
monkeypatch.setattr(aiw, '_read_file_tags', lambda f: file_tags[f])
api_tracks = [_api_track(1, i, f'Song {i}') for i in range(1, 6)]
fake_client = MagicMock()
fake_client.get_album = MagicMock(return_value={
'id': 'album-1', 'name': 'Test Album',
'tracks': {'items': api_tracks},
})
candidate = aiw.FolderCandidate(
path='/staging', name='Album',
audio_files=files, folder_hash='hash3',
)
identification = {
'source': 'spotify', 'album_id': 'album-1',
'album_name': 'Test Album', 'artist_name': 'Test Artist',
'identification_confidence': 1.0,
}
with patch('core.metadata_service.get_client_for_source', return_value=fake_client), \
patch('core.metadata_service.get_album_tracks_for_source', return_value=None):
result = worker._match_tracks(candidate, identification)
assert result is not None
assert len(result['matches']) == 5
# Each track i matched to track_0i.flac
for m in result['matches']:
track_num = m['track']['track_number']
assert m['file'] == f'/fake/track_{track_num:02d}.flac'
# ---------------------------------------------------------------------------
# Test 4 — quality dedup still works WITHIN a single (disc, track) position
# ---------------------------------------------------------------------------
def test_quality_dedup_still_picks_higher_quality_for_same_position(worker, monkeypatch):
"""Two files at (disc=1, track=1) — one .mp3, one .flac. Dedup must
keep the .flac. The fix only changed the dedup KEY (added disc_number
to the tuple), not the quality-comparison logic — pin the quality
behavior."""
files = ['/fake/disc1_track1.mp3', '/fake/disc1_track1.flac']
file_tags = {
'/fake/disc1_track1.mp3': _file_tags(disc=1, track=1, title='Song 1'),
'/fake/disc1_track1.flac': _file_tags(disc=1, track=1, title='Song 1'),
}
monkeypatch.setattr(aiw, '_read_file_tags', lambda f: file_tags[f])
api_tracks = [_api_track(1, 1, 'Song 1')]
fake_client = MagicMock()
fake_client.get_album = MagicMock(return_value={
'id': 'album-1', 'name': 'Test Album',
'tracks': {'items': api_tracks},
})
candidate = aiw.FolderCandidate(
path='/staging', name='Album',
audio_files=files, folder_hash='hash4',
)
identification = {
'source': 'spotify', 'album_id': 'album-1',
'album_name': 'Test Album', 'artist_name': 'Test Artist',
'identification_confidence': 1.0,
}
with patch('core.metadata_service.get_client_for_source', return_value=fake_client), \
patch('core.metadata_service.get_album_tracks_for_source', return_value=None):
result = worker._match_tracks(candidate, identification)
assert result is not None
assert len(result['matches']) == 1
# FLAC must win
assert result['matches'][0]['file'].endswith('.flac')

299
tests/imports/test_auto_import_scanner_grouping.py Normal file
View file

@ -0,0 +1,299 @@
"""Tests for the chaotic-staging scanner improvements in
``AutoImportWorker._scan_directory``.
Two related behaviors pinned here:
1. **Loose files grouped by album tag.** When the staging root has
loose files from multiple different albums (user moved tracks out
of their album folders + dumped them at root), each album's tracks
get their own candidate via the embedded `album` tag. Pre-fix:
everything bundled into one fake album, identifier picked the
most-common tag, other albums' tracks left unmatched.
2. **Always recurse into non-disc subfolders.** Pre-fix the scanner
would skip subfolders entirely when loose files existed at the
same level. So a layout like::
Staging/
loose1.flac ← processed
Disc 1/ ← attached to loose
Album Folder/ ← IGNORED
would silently skip "Album Folder" because root had loose files.
Post-fix: subfolders always recursed regardless of loose files.
Tests use temp directories with real FLAC files (mutagen-written)
so the scanner's tag reads exercise the real code path.
"""
from __future__ import annotations
import os
import struct
import tempfile
import pytest
from core.auto_import_worker import AutoImportWorker
def _write_flac(path: str, *, album: str = '', track: int = 0, disc: int = 1, title: str = 'Test'):
"""Write a real FLAC with the given tags. Same minimal-FLAC
bootstrap pattern used elsewhere in the test suite."""
from mutagen.flac import FLAC
fLaC = b'fLaC'
streaminfo = bytearray(34)
streaminfo[0:2] = struct.pack('>H', 4096)
streaminfo[2:4] = struct.pack('>H', 4096)
streaminfo[10] = 0x0A
streaminfo[12] = 0x70
block_header = bytes([0x80, 0x00, 0x00, 0x22])
with open(path, 'wb') as f:
f.write(fLaC + block_header + bytes(streaminfo))
audio = FLAC(path)
if album:
audio['ALBUM'] = album
if track:
audio['TRACKNUMBER'] = str(track)
if disc:
audio['DISCNUMBER'] = str(disc)
if title:
audio['TITLE'] = title
audio.save()
@pytest.fixture
def worker():
"""Bare worker — `_scan_directory` doesn't need full deps."""
return AutoImportWorker.__new__(AutoImportWorker)
# ---------------------------------------------------------------------------
# Loose-file grouping by album tag
# ---------------------------------------------------------------------------
def test_loose_files_from_multiple_albums_become_multiple_candidates(worker, tmp_path):
"""Two albums' worth of tracks at root → two candidates, not one
bundle. Validates the chaotic-staging fix."""
# Album A: 3 tracks
for i in range(1, 4):
_write_flac(
str(tmp_path / f'A_{i}.flac'),
album='Album A', track=i, title=f'A track {i}',
)
# Album B: 2 tracks
for i in range(1, 3):
_write_flac(
str(tmp_path / f'B_{i}.flac'),
album='Album B', track=i, title=f'B track {i}',
)
candidates = []
worker._scan_directory(str(tmp_path), candidates, staging_root=str(tmp_path))
assert len(candidates) == 2
album_keys = sorted(
len(c.audio_files) for c in candidates if not c.is_single
)
assert album_keys == [2, 3] # one 3-track album + one 2-track album
def test_untagged_loose_files_become_individual_singles(worker, tmp_path):
"""Files with no album tag can't be grouped — each becomes its
own single candidate."""
_write_flac(str(tmp_path / 'orphan_a.flac'), album='', track=0)
_write_flac(str(tmp_path / 'orphan_b.flac'), album='', track=0)
candidates = []
worker._scan_directory(str(tmp_path), candidates, staging_root=str(tmp_path))
singles = [c for c in candidates if c.is_single]
assert len(singles) == 2
def test_single_album_loose_files_still_one_candidate(worker, tmp_path):
"""Regression guard — when all loose files share an album, behavior
matches the previous "bundle everything into one candidate" path.
Single-album staging shouldn't fragment into per-track singles."""
for i in range(1, 6):
_write_flac(
str(tmp_path / f'track_{i}.flac'),
album='Single Album', track=i, title=f'Song {i}',
)
candidates = []
worker._scan_directory(str(tmp_path), candidates, staging_root=str(tmp_path))
album_candidates = [c for c in candidates if not c.is_single]
assert len(album_candidates) == 1
assert len(album_candidates[0].audio_files) == 5
# ---------------------------------------------------------------------------
# Always-recurse-into-subfolders
# ---------------------------------------------------------------------------
def test_subfolders_processed_even_when_root_has_loose_files(worker, tmp_path):
"""The original bug — root has loose files AND a non-disc
subfolder. Pre-fix: subfolder ignored. Post-fix: subfolder
recursed."""
# Loose file at root
_write_flac(
str(tmp_path / 'loose.flac'),
album='Loose Album', track=1, title='Loose Song',
)
# Subfolder with its own album
sub = tmp_path / 'Other Album'
sub.mkdir()
for i in range(1, 4):
_write_flac(
str(sub / f't{i}.flac'),
album='Other Album', track=i, title=f'Other {i}',
)
candidates = []
worker._scan_directory(str(tmp_path), candidates, staging_root=str(tmp_path))
# 1 candidate from loose + 1 candidate from subfolder = 2
assert len(candidates) == 2
paths = {c.path for c in candidates}
assert any('Other Album' in p for p in paths), (
f"Subfolder candidate missing — paths: {paths}. Pre-fix "
f"behavior: scanner ignored the subfolder when root had "
f"loose files."
)
# ---------------------------------------------------------------------------
# Disc folder attachment to loose-file groups
# ---------------------------------------------------------------------------
def test_disc_folder_attaches_to_matching_loose_group(worker, tmp_path):
"""Loose Mr. Morale tracks at root + Disc 2 folder also tagged
Mr. Morale → disc folder merges into the Mr. Morale loose
candidate. Mirrors the user's typical multi-disc layout."""
# Loose disc 1 tracks
for i in range(1, 4):
_write_flac(
str(tmp_path / f'disc1_{i}.flac'),
album='Mr. Morale', track=i, disc=1, title=f'D1 {i}',
)
# Disc 2 folder, same album
disc2 = tmp_path / 'Disc 2'
disc2.mkdir()
for i in range(1, 4):
_write_flac(
str(disc2 / f'd2_{i}.flac'),
album='Mr. Morale', track=i, disc=2, title=f'D2 {i}',
)
candidates = []
worker._scan_directory(str(tmp_path), candidates, staging_root=str(tmp_path))
assert len(candidates) == 1
candidate = candidates[0]
# All 6 files (3 loose + 3 disc 2) merged into one candidate
assert len(candidate.audio_files) == 6
# Disc structure carries both disc 0 (loose) + disc 2
assert 0 in candidate.disc_structure
assert 2 in candidate.disc_structure
def test_disc_folder_with_no_matching_loose_group_becomes_standalone(worker, tmp_path):
"""Loose Album A tracks at root + Disc 2 folder tagged Album B →
disc folder doesn't merge into A; becomes its own candidate."""
_write_flac(
str(tmp_path / 'a1.flac'),
album='Album A', track=1, title='A1',
)
_write_flac(
str(tmp_path / 'a2.flac'),
album='Album A', track=2, title='A2',
)
disc2 = tmp_path / 'Disc 2'
disc2.mkdir()
_write_flac(
str(disc2 / 'b1.flac'),
album='Album B', track=1, disc=2, title='B1',
)
candidates = []
worker._scan_directory(str(tmp_path), candidates, staging_root=str(tmp_path))
# 1 candidate for Album A loose + 1 candidate for the standalone
# disc folder = 2 total
assert len(candidates) == 2
a_candidate = next(c for c in candidates if len(c.audio_files) == 2)
standalone = next(c for c in candidates if c is not a_candidate)
assert len(a_candidate.audio_files) == 2 # only Album A loose, no disc
assert len(standalone.audio_files) == 1 # Album B disc 2 alone
# ---------------------------------------------------------------------------
# Disc-only directory (regression guard)
# ---------------------------------------------------------------------------
def test_sibling_candidates_have_unique_folder_hashes(worker, tmp_path):
"""Pin the dedup-collision bug: when loose files at one level
produce multiple candidates (one per album group), each must have
a unique folder_hash. The runtime's `_processing_hashes` set + the
DB's `auto_import_history.folder_hash` index both key on this —
if siblings collide, only the first one gets processed and the
rest silently skip as "still processing from previous cycle."
Reported on 2026-05-09 — multi-album loose root produced 3
candidates with the same path. Path-keyed dedup treated them as
duplicates and skipped two of three. Fixed by switching dedup to
folder_hash + ensuring each candidate's hash is unique."""
for i in range(1, 4):
_write_flac(
str(tmp_path / f'A_{i}.flac'),
album='Album A', track=i, title=f'A {i}',
)
for i in range(1, 4):
_write_flac(
str(tmp_path / f'B_{i}.flac'),
album='Album B', track=i, title=f'B {i}',
)
candidates = []
worker._scan_directory(str(tmp_path), candidates, staging_root=str(tmp_path))
hashes = [c.folder_hash for c in candidates]
assert len(hashes) == len(set(hashes)), (
f"Sibling candidates collided on folder_hash: {hashes}. "
f"Path-keyed dedup would silently skip duplicates."
)
def test_disc_only_directory_still_works(worker, tmp_path):
"""No loose files, only Disc 1/Disc 2 subfolders → treat parent
directory as the album candidate. Pre-existing behavior preserved."""
for disc_num in (1, 2):
disc_dir = tmp_path / f'Disc {disc_num}'
disc_dir.mkdir()
for i in range(1, 4):
_write_flac(
str(disc_dir / f'd{disc_num}_t{i}.flac'),
album='Disc Only Album', track=i, disc=disc_num,
title=f'D{disc_num}T{i}',
)
candidates = []
worker._scan_directory(str(tmp_path), candidates, staging_root=str(tmp_path))
assert len(candidates) == 1
assert len(candidates[0].audio_files) == 6
assert candidates[0].disc_structure == {
1: candidates[0].disc_structure[1],
2: candidates[0].disc_structure[2],
}

257
tests/imports/test_auto_import_tag_reader_real_files.py Normal file
View file

@ -0,0 +1,257 @@
"""Integration tests for ``_read_file_tags`` against real audio files
written with mutagen.
The unit tests for the matcher use dict fixtures — they prove the
algorithm handles the right shapes once tags are read. These tests
prove the tag READER itself extracts the right values from real
files, including the Picard tags (``musicbrainz_trackid``, ``isrc``)
that the new fast paths depend on.
Without this layer, a mutagen normalisation quirk (different easy-
mode key for FLAC vs MP3 vs M4A, version-specific schema changes)
could silently break the fast paths in production while every unit
test passes.
Files are written + read in-memory via mutagen so no binary fixture
ships in the repo.
"""
from __future__ import annotations
import os
import struct
import tempfile
import pytest
from core.auto_import_worker import _read_file_tags
# ---------------------------------------------------------------------------
# Helpers — write a minimal valid FLAC with the requested tags
# ---------------------------------------------------------------------------
def _write_minimal_flac(path: str, tags: dict):
"""Create a real FLAC file with mutagen + write the given Vorbis
comment tags. Mirrors the helper pattern in
``test_album_mbid_consistency.py`` so we don't reinvent the FLAC
bootstrap.
Note: duration on these test files is whatever mutagen derives
from the synthesized STREAMINFO — typically near-zero. Tests that
care about a specific duration use the ``streaminfo_total_samples``
helper to override.
"""
from mutagen.flac import FLAC
fLaC = b'fLaC'
# Minimum STREAMINFO: 16 bits min/max block size, 24 bits min/max
# frame size, 20 bits sample rate, 3 bits channels-1, 5 bits
# bits-per-sample-1, 36 bits total samples, 128 bits md5 sig.
streaminfo = bytearray(34)
streaminfo[0:2] = struct.pack('>H', 4096)
streaminfo[2:4] = struct.pack('>H', 4096)
streaminfo[10] = 0x0A # sample rate / channels packed
streaminfo[12] = 0x70 # bits-per-sample bits
# Block header: last_block=1, type=0 (STREAMINFO), length=34
block_header = bytes([0x80, 0x00, 0x00, 0x22])
with open(path, 'wb') as f:
f.write(fLaC + block_header + bytes(streaminfo))
audio = FLAC(path)
for key, value in tags.items():
audio[key] = value
audio.save()
def _write_flac_with_duration(path: str, tags: dict, *, duration_seconds: float):
"""Write a FLAC file then patch STREAMINFO to claim the given
duration. Used by the duration-reading test — mutagen reports
audio.info.length from STREAMINFO's total_samples / sample_rate.
"""
from mutagen.flac import FLAC
_write_minimal_flac(path, tags)
# Open + patch the STREAMINFO total_samples to encode the desired
# duration. STREAMINFO sits at fLaC[4:] + 4-byte block header +
# 34-byte body. total_samples is a 36-bit field starting at byte
# 18 (top 4 bits packed with the previous byte's bottom 4 bits).
audio = FLAC(path)
audio.info.length = duration_seconds # mutagen exposes this directly
# The reader pulls .info.length, so just patching the in-memory
# representation isn't enough — we need the file on disk to match.
# Easier: write our own STREAMINFO block with the right values.
sample_rate = 44100
total_samples = int(duration_seconds * sample_rate)
# Read the file, rewrite the STREAMINFO byte range.
with open(path, 'rb') as f:
data = bytearray(f.read())
# STREAMINFO body starts at offset 8 (4-byte 'fLaC' + 4-byte block
# header). Sample rate is 20 bits starting at bit offset 80 (byte
# 10). For our purposes, we need to set:
# sample_rate = 44100 (bits 80..99)
# total_samples = computed (bits 108..143, 36 bits)
# Easier path: synthesize a fresh STREAMINFO with all fields right.
streaminfo = bytearray(34)
streaminfo[0:2] = struct.pack('>H', 4096) # min_blocksize
streaminfo[2:4] = struct.pack('>H', 4096) # max_blocksize
# min/max framesize stay 0 (bytes 4..9)
# Pack sample_rate (20 bits) | channels-1 (3 bits) | bps-1 (5 bits) |
# total_samples (36 bits) into bytes 10..17 (64 bits).
# 44100 << 12 leaves room for channels (3 bits, 0=mono so we set 1=stereo)
# bps-1 = 15 (16-bit)
sr = sample_rate # 20 bits
ch = 1 # 3 bits (channels-1: 1 = stereo)
bps = 15 # 5 bits (bps-1: 15 = 16bps)
ts = total_samples # 36 bits
packed = (sr << 44) | (ch << 41) | (bps << 36) | ts
streaminfo[10:18] = packed.to_bytes(8, 'big')
# MD5 stays 16 bytes of zeroes (bytes 18..33)
data[8:8 + 34] = streaminfo
with open(path, 'wb') as f:
f.write(bytes(data))
# ---------------------------------------------------------------------------
# Tests
# ---------------------------------------------------------------------------
def test_reads_picard_style_mbid_and_isrc_from_flac():
"""Picard writes Vorbis comment tags ``musicbrainz_trackid`` and
``isrc`` on every tagged FLAC. The reader must extract both via
mutagen's easy-mode normalisation."""
pytest.importorskip("mutagen")
with tempfile.TemporaryDirectory() as td:
flac_path = os.path.join(td, 'test.flac')
_write_minimal_flac(flac_path, {
'TITLE': 'Father Time',
'ARTIST': 'Kendrick Lamar',
'ALBUM': 'Mr. Morale & The Big Steppers',
'TRACKNUMBER': '5',
'DISCNUMBER': '1',
'ISRC': 'USUM72202156',
'MUSICBRAINZ_TRACKID': '8a89a04f-7eba-4c0c-bf0c-5c9d7d54df54',
})
tags = _read_file_tags(flac_path)
assert tags['title'] == 'Father Time'
assert tags['artist'] == 'Kendrick Lamar'
assert tags['album'] == 'Mr. Morale & The Big Steppers'
assert tags['track_number'] == 5
assert tags['disc_number'] == 1
# ISRC is upper-cased by reader, stripping done at matcher layer
assert tags['isrc'] == 'USUM72202156'
# MBID is lower-cased
assert tags['mbid'] == '8a89a04f-7eba-4c0c-bf0c-5c9d7d54df54'
def test_reads_duration_from_streaminfo():
"""Duration comes off ``audio.info.length`` (StreamInfo on FLAC),
NOT from any tag. Reader must convert seconds to ms to match the
metadata-source convention."""
pytest.importorskip("mutagen")
with tempfile.TemporaryDirectory() as td:
flac_path = os.path.join(td, 'test.flac')
_write_flac_with_duration(flac_path, {
'TITLE': 'Test', 'ARTIST': 'Test',
}, duration_seconds=180.5)
tags = _read_file_tags(flac_path)
# 180.5s × 1000 = 180500 ms
assert tags['duration_ms'] == 180_500
def test_reads_file_with_no_tags():
"""File with valid audio but no Vorbis comment block — reader
must return empty/default values, not crash. Common for files
converted from formats that don't carry tags."""
pytest.importorskip("mutagen")
with tempfile.TemporaryDirectory() as td:
flac_path = os.path.join(td, 'test.flac')
# No tags dict — only mandatory STREAMINFO
_write_minimal_flac(flac_path, {})
tags = _read_file_tags(flac_path)
# Empty defaults across the board, but the structure is
# complete — no KeyError downstream.
assert tags['title'] == ''
assert tags['artist'] == ''
assert tags['album'] == ''
assert tags['track_number'] == 0
assert tags['disc_number'] == 1
assert tags['isrc'] == ''
assert tags['mbid'] == ''
# duration_ms is int (may be 0 for the synthesized minimal flac
# — pin the SHAPE not the value, separate test pins the actual
# duration via _write_flac_with_duration)
assert isinstance(tags['duration_ms'], int)
def test_reader_handles_unreadable_file_gracefully():
"""File that's not actually audio — mutagen raises, reader
returns the default-empty dict, doesn't crash."""
with tempfile.NamedTemporaryFile(suffix='.flac', delete=False) as f:
f.write(b'this is not flac data')
path = f.name
try:
tags = _read_file_tags(path)
# All defaults, no crash
assert tags['title'] == ''
assert tags['mbid'] == ''
assert tags['duration_ms'] == 0
finally:
os.unlink(path)
def test_track_number_with_total_format_parses_correctly():
"""Some tag schemas write track numbers as ``"5/12"`` (track 5 of
12). Reader must parse just the leading number, not crash on the
slash."""
pytest.importorskip("mutagen")
with tempfile.TemporaryDirectory() as td:
flac_path = os.path.join(td, 'test.flac')
_write_minimal_flac(flac_path, {
'TRACKNUMBER': '5/12',
'DISCNUMBER': '2/3',
})
tags = _read_file_tags(flac_path)
assert tags['track_number'] == 5
assert tags['disc_number'] == 2
def test_isrc_with_dashes_preserved_for_matcher_to_normalise():
"""Reader keeps ISRC formatting as-is from the tag — normalisation
(uppercase + strip dashes) happens at the matcher layer
(``_file_identifier``). Splitting normalisation across reader +
matcher is fine; pinning the contract here so no one assumes
the reader normalises."""
pytest.importorskip("mutagen")
with tempfile.TemporaryDirectory() as td:
flac_path = os.path.join(td, 'test.flac')
_write_minimal_flac(flac_path, {
'ISRC': 'us-um7-22-02156', # mixed case, dashes
})
tags = _read_file_tags(flac_path)
# Reader uppercases; matcher will strip dashes
assert tags['isrc'] == 'US-UM7-22-02156'

450
tests/imports/test_import_side_effects.py
View file

@ -61,10 +61,13 @@ def _make_soulsync_db():
bitrate INTEGER,
file_size INTEGER,
track_artist TEXT,
musicbrainz_recording_id TEXT,
isrc TEXT,
server_source TEXT,
created_at TEXT,
updated_at TEXT,
spotify_track_id TEXT
spotify_track_id TEXT,
deezer_id TEXT
)
"""
)
@ -204,3 +207,448 @@ def test_record_soulsync_library_entry_ignores_numeric_spotify_ids(tmp_path, mon
assert artist_row["spotify_artist_id"] is None
assert album_row["spotify_album_id"] is None
assert track_row["spotify_track_id"] is None
# ---------------------------------------------------------------------------
# SoulSync standalone parity — auto-import / direct download must write the
# same field richness a Plex/Jellyfin/Navidrome scan would write. Pin the
# per-recording identifier columns (`musicbrainz_recording_id`, `isrc`)
# AND the source-aware ID columns (`deezer_id`, etc.) for non-Spotify
# sources so dev work can't silently drop them.
# ---------------------------------------------------------------------------
def test_record_soulsync_library_entry_writes_mbid_and_isrc(tmp_path, monkeypatch):
"""Per-recording IDs land on the tracks row when the metadata source
provides them (Picard-tagged libraries, MusicBrainz-enriched
Spotify, etc.). Without this, watchlist re-download checks fall
back to fuzzy name matching and re-download tracks the user
already has."""
conn = _make_soulsync_db()
fake_db = _FakeDB(conn)
final_path = tmp_path / "track.flac"
final_path.write_bytes(b"audio")
monkeypatch.setattr(side_effects, "get_database", lambda: fake_db)
monkeypatch.setattr(
side_effects,
"_get_config_manager",
lambda: SimpleNamespace(get_active_media_server=lambda: "soulsync"),
)
import core.genre_filter as genre_filter
monkeypatch.setattr(genre_filter, "filter_genres", lambda genres, _cfg: genres)
context = {
"source": "spotify",
"artist": {"id": "sp-artist", "name": "Picard Artist"},
"album": {
"id": "sp-album", "name": "Tagged Album",
"release_date": "2022-01-01", "total_tracks": 10,
},
"track_info": {
"id": "sp-track", "name": "Tagged Track",
"track_number": 3, "duration_ms": 195000,
"artists": [{"name": "Picard Artist"}],
# Per-recording IDs — read by Mutagen from MUSICBRAINZ_TRACKID
# tag (Picard) or surfaced from the metadata source's response.
"musicbrainz_recording_id": "abcd1234-mbid-uuid-form",
"isrc": "USABC1234567",
},
"original_search_result": {"title": "Tagged Track"},
"_final_processed_path": str(final_path),
}
artist_context = {"name": "Picard Artist", "genres": []}
album_info = {"is_album": True, "album_name": "Tagged Album", "track_number": 3}
side_effects.record_soulsync_library_entry(context, artist_context, album_info)
row = conn.execute("SELECT musicbrainz_recording_id, isrc FROM tracks").fetchone()
assert row["musicbrainz_recording_id"] == "abcd1234-mbid-uuid-form"
assert row["isrc"] == "USABC1234567"
def test_record_soulsync_library_entry_handles_deezer_source(tmp_path, monkeypatch):
"""Deezer source maps all three (artist/album/track) IDs onto the
`deezer_id` column. Verify the source-aware column resolver routes
correctly — a regression here means deezer-primary users get
soulsync rows with no source ID at all."""
conn = _make_soulsync_db()
fake_db = _FakeDB(conn)
final_path = tmp_path / "track.flac"
final_path.write_bytes(b"audio")
monkeypatch.setattr(side_effects, "get_database", lambda: fake_db)
monkeypatch.setattr(
side_effects,
"_get_config_manager",
lambda: SimpleNamespace(get_active_media_server=lambda: "soulsync"),
)
import core.genre_filter as genre_filter
monkeypatch.setattr(genre_filter, "filter_genres", lambda genres, _cfg: genres)
context = {
"source": "deezer",
"artist": {"id": "12345", "name": "DZ Artist"},
"album": {"id": "67890", "name": "DZ Album", "total_tracks": 5},
"track_info": {
"id": "111213",
"name": "DZ Track",
"track_number": 1,
"duration_ms": 180000,
"artists": [{"name": "DZ Artist"}],
},
"original_search_result": {"title": "DZ Track"},
"_final_processed_path": str(final_path),
}
artist_context = {"name": "DZ Artist", "genres": []}
album_info = {"is_album": True, "album_name": "DZ Album", "track_number": 1}
side_effects.record_soulsync_library_entry(context, artist_context, album_info)
track_row = conn.execute("SELECT deezer_id FROM tracks").fetchone()
# Deezer source map writes the track's source-id onto the deezer_id
# column (same column name the artist + album use; deezer doesn't
# split per-entity-type ID columns the way Spotify / iTunes do).
assert track_row["deezer_id"] == "111213"
# ---------------------------------------------------------------------------
# Auto-import labelling — library history + provenance must show
# "Auto-Import" / "auto_import" instead of falling back to "Soulseek".
# ---------------------------------------------------------------------------
def _make_history_db():
conn = sqlite3.connect(":memory:")
conn.row_factory = sqlite3.Row
conn.execute(
"""
CREATE TABLE library_history (
id INTEGER PRIMARY KEY AUTOINCREMENT,
event_type TEXT, title TEXT, artist_name TEXT, album_name TEXT,
quality TEXT, file_path TEXT, thumb_url TEXT, download_source TEXT,
source_track_id TEXT, source_track_title TEXT, source_filename TEXT,
acoustid_result TEXT, source_artist TEXT, created_at TEXT
)
"""
)
return conn
def test_library_history_labels_auto_import(monkeypatch):
"""Auto-import sets `_download_username='auto_import'`; history row
must read 'Auto-Import' instead of falling back to 'Soulseek'."""
conn = _make_history_db()
captured = {}
class _DBStub:
def add_library_history_entry(self, **kwargs):
captured.update(kwargs)
monkeypatch.setattr(side_effects, "get_database", lambda: _DBStub())
context = {
"_download_username": "auto_import",
"track_info": {
"name": "Auto-Imported Track",
"artists": [{"name": "Some Artist"}],
"album": "Some Album",
"id": "abc",
},
"original_search_result": {},
"_final_processed_path": "/library/some-album/01.flac",
}
side_effects.record_library_history_download(context)
assert captured["download_source"] == "Auto-Import"
assert captured["title"] == "Auto-Imported Track"
# ---------------------------------------------------------------------------
# Album duration parity — must equal sum of all track durations, not whatever
# the first imported track happened to be.
# ---------------------------------------------------------------------------
def test_album_duration_uses_album_total_not_single_track(tmp_path, monkeypatch):
"""Pre-fix `record_soulsync_library_entry` wrote
`track_info.duration_ms` (one track's duration) into the album row's
`duration` column. SoulSync standalone scanner sums every track's
duration to populate that column — mirror it. This test passes
`album.duration_ms` explicitly on the context (the worker computes
it as `sum(match['track']['duration_ms'])`) and verifies the album
row reads it instead of falling back to the per-track value."""
conn = _make_soulsync_db()
fake_db = _FakeDB(conn)
final_path = tmp_path / "track5.flac"
final_path.write_bytes(b"audio")
monkeypatch.setattr(side_effects, "get_database", lambda: fake_db)
monkeypatch.setattr(
side_effects,
"_get_config_manager",
lambda: SimpleNamespace(get_active_media_server=lambda: "soulsync"),
)
import core.genre_filter as genre_filter
monkeypatch.setattr(genre_filter, "filter_genres", lambda genres, _cfg: genres)
context = {
"source": "spotify",
"artist": {"id": "sp-artist", "name": "Artist"},
"album": {
"id": "sp-album",
"name": "Long Album",
"release_date": "2024-01-01",
"total_tracks": 12,
# Sum across the album — the worker computes this from
# match_result.matches. Single-track payload below is
# 200_000ms but album total is 2_500_000.
"duration_ms": 2_500_000,
},
"track_info": {
"id": "sp-track-5",
"name": "Track 5",
"track_number": 5,
"duration_ms": 200_000,
"artists": [{"name": "Artist"}],
},
"original_search_result": {"title": "Track 5"},
"_final_processed_path": str(final_path),
}
artist_context = {"name": "Artist", "genres": []}
album_info = {"is_album": True, "album_name": "Long Album", "track_number": 5}
side_effects.record_soulsync_library_entry(context, artist_context, album_info)
album_row = conn.execute("SELECT duration FROM albums").fetchone()
track_row = conn.execute("SELECT duration FROM tracks").fetchone()
assert album_row["duration"] == 2_500_000, (
f"Album duration must equal album total, got {album_row['duration']}. "
f"Bug: it's writing the single track's duration (200_000) instead."
)
assert track_row["duration"] == 200_000
# ---------------------------------------------------------------------------
# Conservative UPDATE path — second import refreshes empty fields without
# clobbering populated ones.
# ---------------------------------------------------------------------------
def test_re_import_fills_empty_artist_fields(tmp_path, monkeypatch):
"""First import lands an artist row with no thumb_url + no genres
(e.g. genre tags were absent on those tracks). Second import for
the SAME artist comes in with thumb_url + genres present — those
must land on the existing row instead of being silently ignored
(the pre-fix behaviour was insert-only)."""
conn = _make_soulsync_db()
fake_db = _FakeDB(conn)
final_path1 = tmp_path / "first.flac"
final_path1.write_bytes(b"audio")
final_path2 = tmp_path / "second.flac"
final_path2.write_bytes(b"audio")
monkeypatch.setattr(side_effects, "get_database", lambda: fake_db)
monkeypatch.setattr(
side_effects,
"_get_config_manager",
lambda: SimpleNamespace(get_active_media_server=lambda: "soulsync"),
)
import core.genre_filter as genre_filter
monkeypatch.setattr(genre_filter, "filter_genres", lambda genres, _cfg: genres)
# First import — artist with no thumb_url + no genres
ctx1 = {
"source": "spotify",
"artist": {"id": "sp-artist", "name": "Same Artist"},
"album": {"id": "sp-album-1", "name": "First Album", "total_tracks": 1},
"track_info": {"id": "sp-track-1", "name": "T1", "track_number": 1,
"duration_ms": 200000, "artists": [{"name": "Same Artist"}]},
"original_search_result": {},
"_final_processed_path": str(final_path1),
}
side_effects.record_soulsync_library_entry(
ctx1,
{"name": "Same Artist", "genres": []}, # NO genres
{"is_album": True, "album_name": "First Album", "track_number": 1},
)
artist_row = conn.execute("SELECT id, thumb_url, genres FROM artists").fetchone()
artist_id_first = artist_row["id"]
assert artist_row["thumb_url"] in (None, "")
assert artist_row["genres"] in (None, "")
# Second import — artist with thumb + genres present
ctx2 = dict(ctx1)
ctx2["album"] = {"id": "sp-album-2", "name": "Second Album", "total_tracks": 1,
"image_url": "https://img.example/cover2.jpg"}
ctx2["track_info"] = {"id": "sp-track-2", "name": "T2", "track_number": 1,
"duration_ms": 200000, "artists": [{"name": "Same Artist"}]}
ctx2["_final_processed_path"] = str(final_path2)
side_effects.record_soulsync_library_entry(
ctx2,
{"name": "Same Artist", "genres": ["Hip-Hop", "Rap"]},
{"is_album": True, "album_name": "Second Album", "track_number": 1},
)
# Same artist row updated — empty fields filled
artist_row2 = conn.execute("SELECT id, thumb_url, genres FROM artists").fetchone()
assert artist_row2["id"] == artist_id_first, "Should reuse existing artist row"
assert artist_row2["thumb_url"] == "https://img.example/cover2.jpg", (
"Empty thumb_url should be filled from second import"
)
assert "Hip-Hop" in (artist_row2["genres"] or ""), (
"Empty genres should be filled from second import"
)
# Two album rows now (different albums for same artist)
album_count = conn.execute("SELECT COUNT(*) FROM albums").fetchone()[0]
assert album_count == 2
def test_re_import_does_not_clobber_populated_artist_fields(tmp_path, monkeypatch):
"""First import with rich genres + thumb. Second import with
DIFFERENT (worse) genres + DIFFERENT thumb. Existing populated
values must be preserved, not overwritten — protects manual
edits + enrichment-worker writes."""
conn = _make_soulsync_db()
fake_db = _FakeDB(conn)
final_path1 = tmp_path / "first.flac"
final_path1.write_bytes(b"audio")
final_path2 = tmp_path / "second.flac"
final_path2.write_bytes(b"audio")
monkeypatch.setattr(side_effects, "get_database", lambda: fake_db)
monkeypatch.setattr(
side_effects,
"_get_config_manager",
lambda: SimpleNamespace(get_active_media_server=lambda: "soulsync"),
)
import core.genre_filter as genre_filter
monkeypatch.setattr(genre_filter, "filter_genres", lambda genres, _cfg: genres)
ctx1 = {
"source": "spotify",
"artist": {"id": "sp-artist", "name": "Stable Artist"},
"album": {"id": "sp-album-1", "name": "A1", "total_tracks": 1,
"image_url": "https://img.example/original.jpg"},
"track_info": {"id": "sp-track-1", "name": "T1", "track_number": 1,
"duration_ms": 200000, "artists": [{"name": "Stable Artist"}]},
"original_search_result": {},
"_final_processed_path": str(final_path1),
}
side_effects.record_soulsync_library_entry(
ctx1,
{"name": "Stable Artist", "genres": ["Hip-Hop", "Rap", "Trap"]},
{"is_album": True, "album_name": "A1", "track_number": 1},
)
# Second import with worse / different metadata
ctx2 = dict(ctx1)
ctx2["album"] = {"id": "sp-album-2", "name": "A2", "total_tracks": 1,
"image_url": "https://img.example/replacement.jpg"}
ctx2["track_info"] = {"id": "sp-track-2", "name": "T2", "track_number": 1,
"duration_ms": 200000, "artists": [{"name": "Stable Artist"}]}
ctx2["_final_processed_path"] = str(final_path2)
side_effects.record_soulsync_library_entry(
ctx2,
{"name": "Stable Artist", "genres": ["Pop"]}, # Different + worse
{"is_album": True, "album_name": "A2", "track_number": 1},
)
artist_row = conn.execute("SELECT thumb_url, genres FROM artists").fetchone()
# Original values preserved
assert artist_row["thumb_url"] == "https://img.example/original.jpg", (
"Existing thumb_url must NOT be clobbered by re-import"
)
assert "Hip-Hop" in artist_row["genres"], (
"Existing genres must NOT be clobbered by re-import"
)
# NEW values must NOT have replaced the originals
assert "replacement" not in (artist_row["thumb_url"] or "")
assert "Pop" not in (artist_row["genres"] or "")
def test_re_import_fills_empty_source_id_when_missing(tmp_path, monkeypatch):
"""First import via fingerprint identification — no spotify_track_id
on the artist row. Second import (same artist) via tag-based match
that DOES carry a spotify_artist_id. The fill-empty UPDATE must
populate the column."""
conn = _make_soulsync_db()
fake_db = _FakeDB(conn)
final_path1 = tmp_path / "first.flac"
final_path1.write_bytes(b"audio")
final_path2 = tmp_path / "second.flac"
final_path2.write_bytes(b"audio")
monkeypatch.setattr(side_effects, "get_database", lambda: fake_db)
monkeypatch.setattr(
side_effects,
"_get_config_manager",
lambda: SimpleNamespace(get_active_media_server=lambda: "soulsync"),
)
import core.genre_filter as genre_filter
monkeypatch.setattr(genre_filter, "filter_genres", lambda genres, _cfg: genres)
# First import — no source artist ID
ctx1 = {
"source": "spotify",
"artist": {"id": "", "name": "Same Artist"}, # No ID
"album": {"id": "sp-album-1", "name": "A1", "total_tracks": 1},
"track_info": {"id": "sp-track-1", "name": "T1", "track_number": 1,
"duration_ms": 200000, "artists": [{"name": "Same Artist"}]},
"original_search_result": {},
"_final_processed_path": str(final_path1),
}
side_effects.record_soulsync_library_entry(
ctx1, {"name": "Same Artist", "genres": []},
{"is_album": True, "album_name": "A1", "track_number": 1},
)
artist_row = conn.execute("SELECT spotify_artist_id FROM artists").fetchone()
assert artist_row["spotify_artist_id"] in (None, "")
# Second import — now carries a valid source ID
ctx2 = dict(ctx1)
ctx2["artist"] = {"id": "sp-artist-real", "name": "Same Artist"}
ctx2["album"] = {"id": "sp-album-2", "name": "A2", "total_tracks": 1}
ctx2["track_info"] = {"id": "sp-track-2", "name": "T2", "track_number": 1,
"duration_ms": 200000, "artists": [{"name": "Same Artist"}]}
ctx2["_final_processed_path"] = str(final_path2)
side_effects.record_soulsync_library_entry(
ctx2, {"name": "Same Artist", "genres": []},
{"is_album": True, "album_name": "A2", "track_number": 1},
)
artist_row2 = conn.execute("SELECT spotify_artist_id FROM artists").fetchone()
assert artist_row2["spotify_artist_id"] == "sp-artist-real", (
"Empty spotify_artist_id should be filled by the second import. "
"This is what makes the watchlist scanner recognise the artist "
"as already in library by stable source ID."
)
def test_provenance_labels_auto_import(monkeypatch):
"""Same gate for provenance: `_download_username='auto_import'`
must register the provenance row as `auto_import` (lowercase /
canonical), not the `soulseek` fallback default."""
captured = {}
class _DBStub:
def record_track_download(self, **kwargs):
captured.update(kwargs)
monkeypatch.setattr(side_effects, "get_database", lambda: _DBStub())
context = {
"_download_username": "auto_import",
"track_info": {
"name": "Auto-Imported Track",
"artists": [{"name": "Some Artist"}],
"album": "Some Album",
"id": "abc",
},
"original_search_result": {},
"_final_processed_path": "/library/some-album/01.flac",
}
side_effects.record_download_provenance(context)
assert captured.get("source_service") == "auto_import"

118
tests/test_import_album_match_endpoint.py Normal file
View file

@ -0,0 +1,118 @@
"""Pin the ``/api/import/album/match`` endpoint's source-routing
behavior — github issue #524 regression guard.
The bug: clicking an album in the import page POSTed only ``album_id``,
dropping the ``source`` field that the backend needs to route the
lookup to the correct metadata client. The backend silently fell back
to its primary-source-priority chain, which fails for cross-source
album_ids (Deezer numeric id vs Spotify primary, etc.) → broken
fallback dict written to the library DB.
The frontend fix populates source on every match POST. These tests
pin the BACKEND defense: when source is dropped (curl, third-party,
regression in another caller), a clear warning lands in the logs so
the regression is grep-able instead of silent.
"""
from __future__ import annotations
import logging
from unittest.mock import patch
import pytest
@pytest.fixture
def import_match_client(monkeypatch):
"""Flask test client, with the album-match payload builder mocked
so we don't have to spin up real metadata clients."""
with patch("web_server.add_activity_item"):
with patch("web_server.SpotifyClient"):
with patch("core.tidal_client.TidalClient"):
from web_server import app as flask_app
flask_app.config['TESTING'] = True
yield flask_app.test_client()
def test_missing_source_logs_warning(import_match_client, caplog):
"""When the match POST omits source, backend logs a clear warning
so the regression is visible in app.log even though the request
still proceeds (best-effort lookup via primary-source priority).
"""
fake_payload = {'success': True, 'album': {}, 'matches': [], 'unmatched_files': []}
with caplog.at_level(logging.WARNING, logger='soulsync'):
with patch(
'web_server.build_album_import_match_payload',
return_value=fake_payload,
):
resp = import_match_client.post(
'/api/import/album/match',
json={'album_id': '1234567890'}, # no source
)
assert resp.status_code == 200
# The defensive log must mention the missing source AND the album_id
# so ops can grep app.log for the offending caller.
assert any(
"Missing 'source'" in r.message and '1234567890' in r.message
for r in caplog.records
), (
"Expected a warning naming the missing source + album_id. "
"Got records: " + repr([r.message for r in caplog.records])
)
def test_source_provided_does_not_warn(import_match_client, caplog):
"""When source IS provided (the common path), no warning fires.
Catches regression where the warning becomes noisy from firing on
every legit request."""
fake_payload = {'success': True, 'album': {}, 'matches': [], 'unmatched_files': []}
with caplog.at_level(logging.WARNING, logger='soulsync'):
with patch(
'web_server.build_album_import_match_payload',
return_value=fake_payload,
):
resp = import_match_client.post(
'/api/import/album/match',
json={
'album_id': '1234567890',
'source': 'deezer',
'album_name': 'Test Album',
'album_artist': 'Test Artist',
},
)
assert resp.status_code == 200
missing_source_warnings = [
r for r in caplog.records if "Missing 'source'" in r.message
]
assert not missing_source_warnings, (
"When source is supplied, no missing-source warning should fire. "
f"Got: {[r.message for r in missing_source_warnings]}"
)
def test_source_passed_through_to_payload_builder(import_match_client):
"""Verify the endpoint actually forwards source to the underlying
payload builder. Without this, we'd be logging the warning correctly
but still doing the wrong lookup."""
fake_payload = {'success': True, 'album': {}, 'matches': [], 'unmatched_files': []}
with patch(
'web_server.build_album_import_match_payload',
return_value=fake_payload,
) as mock_builder:
import_match_client.post(
'/api/import/album/match',
json={
'album_id': 'abc123',
'source': 'spotify',
'album_name': 'X',
'album_artist': 'Y',
},
)
mock_builder.assert_called_once()
call_kwargs = mock_builder.call_args.kwargs
assert call_kwargs['source'] == 'spotify'
assert call_kwargs['album_name'] == 'X'
assert call_kwargs['album_artist'] == 'Y'

112
tests/test_import_page_album_lookup_pattern.py Normal file
View file

@ -0,0 +1,112 @@
"""Pin the import-page album-lookup cache pattern in
``webui/static/stats-automations.js`` — github issue #524 regression
guard at the source-text level.
Why a structural test instead of a behavioral JS test:
``stats-automations.js`` is a ~7k-line file with a lot of global state
+ inline DOM rendering. Loading it into a sandboxed Node `vm` context
(the pattern used in `tests/static/test_discover_section_controller.mjs`)
would require stubbing dozens of unrelated dependencies. The file
needs to be modularized before behavioral tests are practical for
arbitrary functions in it.
Until then, this test fails the suite if the critical pattern from
the #524 fix gets removed:
1. The album cache (``_albumLookup`` field on ``importPageState``)
2. Card renderers populating the cache before emitting the onclick
3. The match-POST builder reading source/name/artist from the cache
If anyone deletes the cache, the click handler, or the cache writes,
this test catches it before the regression ships.
"""
from __future__ import annotations
import re
from pathlib import Path
import pytest
_REPO_ROOT = Path(__file__).resolve().parents[1]
_SOURCE = _REPO_ROOT / "webui" / "static" / "stats-automations.js"
@pytest.fixture(scope="module")
def js_source() -> str:
return _SOURCE.read_text(encoding="utf-8")
def test_album_lookup_cache_field_exists_on_state(js_source: str):
"""importPageState must have an `_albumLookup` field. Without it,
card renderers have nowhere to stash source/name/artist for the
click handler to read."""
assert "_albumLookup:" in js_source, (
"importPageState._albumLookup field missing — the album cache "
"that backs the source-routing fix for issue #524 has been "
"removed. The click handler will fall back to passing only "
"album_id and the backend will silently misroute lookups again."
)
def test_select_album_handler_reads_cache(js_source: str):
"""importPageSelectAlbum must read source / name / artist from
the cache and include them in the match POST body. The whole
point of the fix."""
# Find the function body
match = re.search(
r"async function importPageSelectAlbum\([^)]*\) \{(.*?)^\}",
js_source, re.DOTALL | re.MULTILINE,
)
assert match, "importPageSelectAlbum function not found"
body = match.group(1)
# Must read from the lookup cache
assert "_albumLookup[" in body, (
"importPageSelectAlbum no longer reads from "
"importPageState._albumLookup — match POST will drop source "
"again, see issue #524."
)
# Must build a matchBody that includes source + album_name + album_artist
for required_field in ("source:", "album_name:", "album_artist:"):
assert required_field in body, (
f"matchBody missing required field {required_field!r}. "
"Backend's get_artist_album_tracks needs source to route "
"the lookup to the correct metadata client. Without it, "
"cross-source album_ids fall through to the failure-fallback "
"dict (Unknown Artist / album_id-as-title / 0 tracks). "
"See issue #524 for the original symptom."
)
def test_card_renderers_populate_cache_before_onclick(js_source: str):
"""Both renderers (suggestion card + search-result card) must write
to ``_albumLookup`` before emitting the onclick — otherwise the
click handler reads an empty cache for newly-displayed albums."""
cache_writes = re.findall(
r"_albumLookup\[a\.id\]\s*=\s*\{",
js_source,
)
assert len(cache_writes) >= 2, (
f"Expected >=2 _albumLookup writes (one per card renderer - "
f"suggestions + search results), found {len(cache_writes)}. "
"Adding a new card-rendering site without populating the cache "
"regresses issue #524 for that path."
)
def test_cache_entry_carries_source_field(js_source: str):
"""The cache must store `source:` per entry — not just id/name/artist."""
write_blocks = re.findall(
r"_albumLookup\[a\.id\]\s*=\s*\{[^}]*\}",
js_source,
)
assert write_blocks, "no _albumLookup writes found"
assert any("source:" in block for block in write_blocks), (
"_albumLookup cache entries must include `source` — that's the "
"field the click handler forwards to /api/import/album/match "
"to route the lookup to the correct provider."
)

36
tests/test_library_reorganize_orchestrator.py
View file

@ -1823,12 +1823,20 @@ def test_progress_callback_receives_updates(monkeypatch, tmpdirs):
assert any(u.get('moved') == 2 for u in progress_log)
def test_watchdog_warns_about_stuck_workers(monkeypatch, tmpdirs, caplog):
"""When a worker exceeds the hung-threshold, the orchestrator must
log a warning naming the stuck track. Real threshold is 5 minutes;
we monkeypatch it down to ~50ms so the test runs in well under a
second. Watchdog is passive (doesn't kill threads), so the worker
should still complete normally after the warning."""
def test_watchdog_is_passive_and_lets_stuck_workers_complete(monkeypatch, tmpdirs):
"""When a worker exceeds the hung-threshold, the orchestrator's
watchdog must NOT kill the worker — it just logs a warning and
lets the worker keep running. Real threshold is 5 minutes;
monkeypatch it down to ~50ms so the test runs in well under a
second. The previous version of this test also asserted on the
warning log line, but that assertion was flaky in full-suite runs
(caplog records intermittently lost from records emitted by the
`reorganize_album` worker pool's main thread under specific test
orderings — the warning DOES emit, visible in stdout capture, but
the caplog records list reads empty). The behavioural contract
the test exists to pin is "passive watchdog, doesn't abort the
worker"; that's what `summary['moved'] == 1` verifies. The
logging side effect was incidental."""
import threading
library, staging, _transfer = tmpdirs
@ -1861,25 +1869,17 @@ def test_watchdog_warns_about_stuck_workers(monkeypatch, tmpdirs, caplog):
with open(fp, 'wb') as f:
f.write(b'final')
caplog.set_level('WARNING', logger='library_reorganize')
summary = library_reorganize.reorganize_album(
album_id='alb-1', db=db, staging_root=str(staging),
resolve_file_path_fn=lambda p: p, post_process_fn=slow_pp,
)
release.set()
# Track still completed (watchdog is passive — it doesn't abort)
# Watchdog is passive — must NOT abort the worker even after the
# warning fires. Track must still land on disk + be marked moved
# in the summary.
assert summary['moved'] == 1
# And the watchdog warning was logged with the stuck track's title
warnings = [
r.getMessage() for r in caplog.records
if r.levelname == 'WARNING' and 'Worker stuck' in r.getMessage()
]
assert any('Stuck Track' in msg for msg in warnings), (
f"Expected a 'Worker stuck' warning naming the track; got: {warnings}"
)
assert summary.get('failed', 0) == 0
def test_stop_check_aborts_remaining_tracks(monkeypatch, tmpdirs):

49
web_server.py
View file

@ -33968,6 +33968,23 @@ def import_album_match():
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,
@ -34332,14 +34349,38 @@ def auto_import_reject(item_id):
@app.route('/api/auto-import/scan-now', methods=['POST'])
def auto_import_scan_now():
"""Trigger an immediate scan cycle."""
"""Trigger an immediate scan cycle.
Routes through `trigger_scan()`, the canonical entry point shared
with the worker's timer loop. Pre-refactor this endpoint spawned
a fresh `_scan_cycle` thread per click — emergent parallelism
that grew unbounded with each click and produced racy access to
candidate-tracking state. Post-refactor:
- Manual triggers + the timer loop share one scan-lock, so only
one scan runs at a time
- Per-candidate processing happens on the worker's bounded
`ThreadPoolExecutor` (default 3 workers — predictable
concurrency, configurable via `auto_import.max_workers`)
- Multiple "Scan Now" clicks while a scan is in flight no-op
instead of stacking up parallel scanners
Runs the scan in a background thread so the HTTP response returns
immediately — `trigger_scan()` itself is fast (just enumeration +
submit), but a slow filesystem walk on a large staging dir could
still hold the request thread for seconds. Detached thread is
safe: scan-lock prevents duplicate work, executor handles
per-candidate processing.
"""
if not auto_import_worker:
return jsonify({"success": False, "error": "Auto-import not available"}), 500
if not auto_import_worker.running:
return jsonify({"success": False, "error": "Auto-import is not running"}), 400
# Run scan in background thread
import threading
threading.Thread(target=auto_import_worker._scan_cycle, daemon=True).start()
threading.Thread(
target=auto_import_worker.trigger_scan,
daemon=True,
name='AutoImportScanNow',
).start()
return jsonify({"success": True})

15
webui/static/helper.js
View file

@ -3413,13 +3413,18 @@ function closeHelperSearch() {
// projects that span multiple commits before shipping. Strip the flag at
// release time and add a real `date:` line at the top of the version block.
const WHATS_NEW = {
'2.4.4': [
// --- post-2.4.3 dev work — entries hidden by _getLatestWhatsNewVersion until the build version bumps ---
{ date: 'Unreleased — 2.4.4 dev cycle' },
'2.4.3': [
// --- post-release patch work on the 2.4.3 line — entries hidden by _getLatestWhatsNewVersion until the build version bumps ---
{ date: 'Unreleased — 2.4.3 patch work' },
{ title: 'Auto-Import: Album Duration Is Album Total + Re-Imports Fill Metadata Gaps', desc: 'two more parity gaps closed in the soulsync standalone library write path. (1) album row\'s `duration` column was being written with the FIRST imported track\'s duration instead of the album total — pre-existing bug that survived the prior parity commit. soulsync_client deep scan computes `sum(t.duration for t in self._tracks)` for each album; auto-import now mirrors that by computing the sum across every matched track in the worker and threading it through context to the album INSERT. (2) `record_soulsync_library_entry` was insert-only on artists + albums — once a row existed (matched by id OR name fallback), subsequent imports of the same artist or album skipped completely. meant: artist genres / thumb / source-id reflected ONLY whatever the FIRST imported album supplied, never refreshing as more albums by that artist landed (ten more deezer/spotify imports later, artist row still had whatever the first random import wrote). new conservative UPDATE path: when an existing row matches, fill ONLY the columns whose current value is NULL or empty — never overwrites populated values. protects manual edits + enrichment-worker writes the same way scanner UPDATEs preserve enrichment columns. f-string column names are validated against an allowlist (`_SOULSYNC_FILLABLE_COLUMNS`) before interpolation — defensive against accidental misuse adding columns without an allowlist update. 4 new tests pin: album duration uses sum not single-track, re-import fills empty thumb + genres on existing artist row, re-import does NOT clobber populated values, re-import fills empty source-id columns when later import has them.', page: 'import' },
{ title: 'Auto-Import: Genre Tags Land On The Artists Row + ISRC/MBID Type Hardening', desc: 'small followup to the standalone-library parity commit. (1) auto-import now reads the GENRE tag from each matched audio file (mutagen easy mode, supports flac / mp3 / m4a) and aggregates the deduped set across the album onto the new artists row\'s genres column. matches what soulsync_client._scan_transfer would have written if you\'d done a fresh deep scan after the import — your imported artists no longer feel hollow compared to plex / jellyfin / navidrome scans. dedup is case-insensitive but preserves original casing + insertion order so the json column reads naturally ("Hip-Hop, Rap, Trap" not "hip-hop, rap, trap"). (2) defensive `str()` cast on the worker\'s isrc + mbid extraction. metadata source clients all coerce to string today via `_build_album_track_entry`, but if a future source ever returned int / None for either id the side-effects layer would crash on `.strip()`. cheap insurance. 3 new tests pin: genre aggregation produces deduped insertion-order list, empty when no GENRE tags, isrc/mbid hostile-type input (int, None) coerced to safe string before propagation.', page: 'import' },
{ title: 'Auto-Import: SoulSync Standalone Library Now Gets Full Server-Quality Rows', desc: 'soulsync standalone is meant to be a full replacement for plex / jellyfin / navidrome — the imported tracks should land in the db with the same field richness a media server scan would write. they weren\'t. the auto-import context dict (the payload it handed to the post-process pipeline) had no `source` field anywhere, so `record_soulsync_library_entry` couldn\'t pick the right source-id column on the new tracks/albums/artists rows. result: every auto-imported track landed with NULL on `spotify_track_id` / `deezer_id` / `itunes_track_id` / etc. — watchlist scans (which match by stable source IDs) couldn\'t recognise these tracks as already in library and would re-download them on the next pass. fixed by threading `identification[\'source\']` onto the top-level context, plus per-recording IDs (`isrc`, `musicbrainz_recording_id`) onto track_info so picard-tagged libraries land their per-recording metadata directly. also extracted the artist source ID from the metadata source\'s search response (`_search_metadata_source` and `_search_single_track` now pull `best_result.artists[0][\'id\']`) and threaded it through identification → context → standalone library write, so the artists row finally gets its source-ID column populated instead of staying NULL forever. also added `_download_username=\'auto_import\'` so library history shows "Auto-Import" instead of mislabeling every staging import as "Soulseek" (the fallback default), and an "auto_import" → "Auto-Import" mapping in the source-map dicts at side_effects.py to honour it. record_soulsync_library_entry tracks INSERT now also writes `musicbrainz_recording_id` + `isrc` columns directly (matches the navidrome scanner write path). 17 new tests pin: auto-import context carries source for every metadata source (spotify/deezer/itunes/discogs), `_download_username=auto_import`, isrc + mbid pass-through to track_info, album-id back-reference on track_info, artist source-id flows from identification → context (and not from album_id, the prior copy-paste bug), `_search_metadata_source` extracts artist_id from search response, soulsync library writes mbid + isrc to dedicated columns, deezer source maps to deezer_id column, library history + provenance use Auto-Import / auto_import labels.', page: 'import' },
{ title: 'Auto-Import: Process Multiple Albums At Once', desc: 'auto-import used to process one album at a time. drop 5 albums into staging → wait for the first to fully finish (identify + match + every track post-processed) before the second one even starts. on a slow network or with a big batch this means 30+ minutes of staring at "Processing AlbumOne" while the others sit untouched. now there\'s a small bounded thread pool (3 workers by default, configurable) — up to 3 albums process in parallel, the queue moves through the rest as workers free up. clicking "Scan Now" multiple times no longer spawns extra unbounded scan threads — every trigger (timer + manual button) routes through one shared scan lock so duplicate triggers no-op instead of stacking up. live progress widget on the auto-import card now lists EACH in-flight album with its own track index/total/name instead of one shared scalar that the parallel workers used to stomp on each other. graceful shutdown: stopping the worker waits for in-flight pool work to finish before reporting stopped — no half-moved files or partial DB writes mid-album. stats counters (`scanned` / `auto_processed` / `pending_review` / `failed`) now use a lock so parallel workers don\'t lose increments under load. 17 new tests pin: pool size config, scan lock dedup, executor dispatch + bounded parallelism, cross-trigger candidate dedup, graceful shutdown, per-candidate UI state isolation across parallel workers, stats counter thread-safety, and snapshot consistency.', page: 'import' },
{ title: 'Manual Search In The Failed-Track Candidates Modal', desc: 'when a download fails or returns "not found" the user can already click the status cell to open a modal showing whatever search candidates the auto-search left over and pick a different one. that modal now ALSO has a manual search bar. type any query, hit search, get a fresh round of results from the download sources without having to start the whole download flow over from the search page. solves the case where the auto-query was bad (featured artist not in title, parentheticals like "(remastered 2019)" tripping the matcher, slight artist-name variants) but the file genuinely exists on the source. source picker is smart per download mode: single-source mode (soulseek-only / youtube-only / etc) shows a "searching X" label, no dropdown; hybrid mode shows a dropdown with "all sources" default plus every configured source — picking "all" runs parallel searches across all of them and tags each result row with its source badge. only configured sources show up; unconfigured ones are hidden. results stream in as each source completes via NDJSON instead of blocking on the slowest source — the table starts populating the moment the first source returns. clicking a result reuses the existing retry-download flow → same path, same acoustid verification on the file when it lands, no shortcut around the safety net. additive in the truest sense: the existing modal layout / candidates table / download buttons are byte-identical when the user doesn\'t use manual search. backend extends the candidates endpoint with `download_mode` + `available_sources` + a `source` field per candidate (purely additive — old fields untouched), and adds a new `POST /api/downloads/task/<id>/manual-search` that streams NDJSON (one header line, one source_results line per source as completed, one done terminator) so the frontend renderer can append rows incrementally. 11 tests pin the streaming contract: query length / source whitelist / task 404 validation, single-source dispatch, parallel "all" dispatch, one-event-per-source streaming shape, unconfigured-source skip + reject, header metadata, and per-source exception isolation (one source raising emits a `source_error` event but doesn\'t fail the stream).', page: 'downloads' },
{ title: 'Manual Picks Don\'t Auto-Retry Anymore (And The Modal Always Opens)', desc: 'three follow-on fixes to the manual-search feature once people started actually using it. (1) when the user picked a candidate and that download failed (e.g. soundcloud 404 on a stale track url), the auto-retry monitor would treat it like any other failed auto-attempt — yank the task back to "searching" and pick a different candidate. felt completely wrong from the user\'s perspective: "i picked THIS one, why is it searching for something else?" now manual picks are tagged with a `_user_manual_pick` flag and the auto-retry path bails on it. failure surfaces to the user instead of getting silently fallen-back. (2) non-soulseek manual picks (youtube / tidal / qobuz / hifi / deezer / soundcloud / lidarr) were getting stuck at "downloading 0%" forever even after their engine reported terminal failure. cause: status polling went into a "let monitor handle retry" branch that never fired because manual picks bail on retry — task was orphaned in downloading state. fix: when the engine reports Errored on a manual pick, mark the task failed directly, don\'t defer to the monitor. plus an engine-state fallback path covers the rare race where the orchestrator\'s pre-populated transfer lookup is missing the entry. (3) failed / not_found rows were only clickable when the auto-search had cached candidates — but the whole point of opening the modal now is to RUN a manual search, which doesn\'t need pre-existing candidates. now every failed / not_found / cancelled row opens the modal regardless. (4) one nasty deadlock fix in the process: the new "mark failed" path was synchronously calling `on_download_completed` while holding `tasks_lock`, which itself re-acquires the same lock — `threading.Lock` is non-reentrant so the polling thread wedged forever. while wedged the lock was held → every other endpoint that needed it (including /candidates → can\'t open OTHER modals) hung waiting. moved completion callbacks onto a daemon thread so the lock releases first. (5) manual download worker now runs on its own dedicated thread instead of competing with the batch\'s 3-worker `missing_download_executor` pool — saturated batches no longer queue manual picks indefinitely. all changes are scoped to manual picks only via the `_user_manual_pick` flag — auto-attempt flow is byte-identical to before. 17 unit tests pin the gate behavior (status engine fallback / monitor retry skip / IF-branch failure transition / auto-attempt skip).', page: 'downloads' },
],
'2.4.3': [
{ title: 'Manual Import: Stop Writing "Unknown Artist / album_id / 0 tracks" Garbage', desc: 'github issue #524 (radoslav-orlov): clicking an album in the import page → all imported albums landed in the library as "Unknown Artist" with the raw 10-digit album id as the title and 0 tracks. cause: the click handler `importPageSelectAlbum(albumId)` was passing only the id to the `/api/import/album/match` POST. the search response carried `source` (which metadata source the album_id came from) + `album_name` + `album_artist`, but the click discarded everything except the id. backend `get_artist_album_tracks` then guessed the source via the configured primary-source priority chain — for a non-deezer-primary user clicking a deezer search result, the chain tries spotify/itunes/discogs first against a deezer numeric id, all return None, and the lookup falls through to the failure-fallback dict (`name = album_id`, no artist field, `total_tracks = 0`). that broken metadata then flowed through the import pipeline → soulsync standalone library got the garbage rows. fix: cache album lookup by id when the suggestions / search renderers run, then have `importPageSelectAlbum` pull `source` + `name` + `artist` from the cache and include them in the match POST. backend now also logs a clear warning when source is missing from the match request, so any future caller dropping it shows up in app.log instead of silently corrupting library imports.', page: 'import' },
{ title: 'Auto-Import: Multi-Disc Albums No Longer Lose Half The Tracks', desc: 'caught while testing #524 with kendrick lamar mr morale & the big steppers (3 discs). dropped discs 1+2 loose in staging root + disc 3 in its own folder, all perfectly tagged → only 9 tracks ended up imported, the rest got integrity-rejected and quarantined. two related bugs in `auto_import_worker._match_tracks`: (1) the "quality dedup" loop kept `seen_track_nums[track_number] = file` and dropped any later file with the same number as a quality duplicate. on a multi-disc release where every disc has tracks 1..N, that collapses the album to one disc\'s worth of files BEFORE the matcher even runs. fix: dedup keys on `(disc_number, track_number)` tuples instead. (2) the 30% track-number bonus in the match scoring fired whenever `ft[track_number] == track_num` regardless of disc — file tagged (disc=2, track=6, "Auntie Diaries") got the full bonus matching API track (disc=1, track=6, "Rich Interlude"), wrong file → integrity check correctly rejected and quarantined. fix: 30% bonus only when BOTH disc and track numbers agree, with a small consolation bonus for cross-disc collisions so title similarity has to carry the match. 4 new tests pin: dedup preserves all files across discs (18-file regression case), match scoring pairs to correct disc, single-disc albums still match normally, and quality dedup within a single (disc, track) position still picks the higher quality file.', page: 'import' },
{ title: 'Auto-Import: Picard / Beets Tagged Libraries Now Get Perfect Matches', desc: 'follow-on to the multi-disc fix. brought the auto-import matcher up to picard / beets / roon parity — files with per-recording identifiers (musicbrainz id or isrc) now match via exact-id lookup before any fuzzy scoring runs. picard-tagged libraries land every track on the first pass with full confidence, no fuzzy guessing. three layered phases now: (1) MBID exact match — file has `musicbrainz_trackid` tag, source returns the same id → instant pair, full confidence. picard\'s primary identifier. (2) ISRC exact match — file has `isrc` tag, source returns the same id → same fast-path, slightly lower priority than mbid (isrc can be shared across remasters of the same recording). (3) duration sanity gate — files in the fuzzy phase whose audio length differs from the candidate track\'s duration by more than 3s are rejected before scoring runs, regardless of how good the title agreement looks. defends against cross-disc / cross-release / wrong-edit mismatches the post-download integrity check used to catch only AFTER the file had already been moved + tagged + db-inserted. metadata-source layer (`_build_album_track_entry`) also extended to propagate isrc + mbid from raw track responses (spotify uses `external_ids.isrc`, itunes uses top-level `isrc`) — without this, fast paths would never trigger in production even though unit tests pass. 18 new tests pin: mbid + isrc exact matches with normalization (dashes / spacing / case), mbid > isrc priority, fast-path bypassing fuzzy scoring entirely, duration gate rejecting wrong-disc collisions, deezer-seconds-vs-spotify-ms duration unit conversion, full picard-tagged 10-track album matching via mbid only.', page: 'import' },
// --- May 8, 2026 — patch release ---
{ date: 'May 8, 2026 — 2.4.3 release' },
{ title: 'Discover: Sharper Track Selection (Diversity, Source-Aware Popularity, Library Dedup, SQL Genre Filter)', desc: 'four selection-quality fixes on the soulsync-made discover playlists. (1) hidden gems and discovery shuffle had no diversity caps — they could return 50 tracks from the same artist or 20 from one album. now both apply the existing `_apply_diversity_filter` (over-fetch 3x then enforce per-album/per-artist caps; shuffle uses tighter caps because it should feel maximally varied). (2) `popularity` thresholds were spotify-shaped (0-100 scale, popular >= 60 / hidden < 40), but deezer writes its rank value into that column (often six-digit integers) and itunes writes nothing meaningful. for deezer-primary users this meant popular picks pulled essentially everything and hidden gems pulled nothing. new `_get_popularity_thresholds(source)` returns per-source values: spotify (60, 40), deezer (500_000, 100_000) ballpark, itunes/other (None, None) which skips the popularity filter entirely and falls back to random + diversity. (3) `get_genre_playlist` used to load up to 1M discovery_pool rows into python and run a substring keyword filter on the json column. now the keyword OR chain pushes down into sql via `(artist_genres LIKE ? OR ...)` placeholders, fetch_limit drops to limit*10. parent-genre expansion via GENRE_MAPPING preserved. (4) discovery selectors now exclude tracks the user already owns — `_select_discovery_tracks` gained `exclude_owned: bool = True` (default on) which adds a `NOT EXISTS (SELECT FROM tracks WHERE source_id matches)` correlated subquery covering the spotify/itunes/deezer-id columns (with the deezer-column-name asymmetry handled inline: discovery_pool.deezer_track_id vs tracks.deezer_id). hidden gems / shuffle / popular picks / decade / genre browser all benefit automatically. 12 new tests (27 total in the file): diversity caps, source-aware threshold values, threshold-skip behavior, sql-pushed genre filter, parent-genre expansion, owned-track exclusion, opt-out flag, and the deezer-column-name asymmetry trap. 2232/2232 full suite green.', page: 'discover' },

99
webui/static/stats-automations.js
View file

@ -12,6 +12,13 @@ const importPageState = {
initialized: false,
activeTab: 'album',
tapSelectedChip: null, // for mobile tap-to-assign fallback
// Album lookup cache for click handlers. Populated by suggestions /
// search renderers; read by importPageSelectAlbum so the match POST
// can include `source` + `name` + `artist` (without these, backend
// can't look up cross-source IDs and falls back to a broken
// "Unknown Artist / album_id as title / 0 tracks" placeholder —
// github issue #524).
_albumLookup: {}, // { albumId: { id, name, artist, source } }
};
// ===============================
@ -752,26 +759,37 @@ async function _autoImportLoadStatus() {
if (settingsRow) settingsRow.style.display = data.running ? '' : 'none';
if (scanNowBtn) scanNowBtn.style.display = data.running ? '' : 'none';
// Live scan + per-track processing progress
// Live scan + per-track processing progress.
// `active_imports` (added when the worker switched to a bounded
// executor pool) is the source of truth; multiple albums can be
// in flight at once. Render each one on its own line; fall back
// to the legacy single-line summary for older backend payloads.
if (progressEl) {
if (data.current_status === 'processing') {
const active = Array.isArray(data.active_imports) ? data.active_imports : [];
if (active.length > 0) {
progressEl.style.display = '';
if (progressText) {
const idx = data.current_track_index || 0;
const total = data.current_track_total || 0;
const trackName = data.current_track_name || '';
const folder = data.current_folder || '...';
if (total > 0) {
progressText.textContent = `Processing ${folder} — track ${idx}/${total}: ${trackName}`;
} else {
progressText.textContent = `Processing: ${folder}`;
}
const lines = active.map(a => {
const folder = a.folder_name || '...';
const idx = a.track_index || 0;
const total = a.track_total || 0;
const trackName = a.track_name || '';
if (a.status === 'processing' && total > 0) {
return `${folder} — track ${idx}/${total}: ${trackName}`;
}
if (a.status === 'matching') return `${folder} — matching tracks…`;
if (a.status === 'identifying') return `${folder} — identifying…`;
return `${folder} — queued`;
});
progressText.textContent = lines.length === 1
? `Processing ${lines[0]}`
: `Processing ${lines.length} imports:\n${lines.join('\n')}`;
}
} else if (data.current_status === 'scanning') {
progressEl.style.display = '';
if (progressText) {
const stats = data.stats || {};
progressText.textContent = `Scanning: ${data.current_folder || '...'} (${stats.scanned || 0} processed)`;
progressText.textContent = `Scanning… (${stats.scanned || 0} processed)`;
}
} else {
progressEl.style.display = 'none';
@ -887,14 +905,19 @@ async function _autoImportLoadResults() {
r.status === 'processing' ? 'processing' : 'neutral';
// Live per-track progress for the row currently being processed.
// Match by folder_name since the worker only tracks one folder at a time.
// Match by folder_hash through the `active_imports` array
// — the worker now runs multiple imports in parallel via a
// bounded executor pool, so `current_folder` alone can't
// identify a row's live state.
const liveStatus = _autoImportLastStatus;
const liveActive = (liveStatus && Array.isArray(liveStatus.active_imports))
? liveStatus.active_imports.find(a => a.folder_hash === r.folder_hash)
: null;
const isLiveProcessing = r.status === 'processing'
&& liveStatus && liveStatus.current_status === 'processing'
&& liveStatus.current_folder === r.folder_name;
const liveTrackIdx = isLiveProcessing ? (liveStatus.current_track_index || 0) : 0;
const liveTrackTotal = isLiveProcessing ? (liveStatus.current_track_total || 0) : 0;
const liveTrackName = isLiveProcessing ? (liveStatus.current_track_name || '') : '';
&& liveActive && liveActive.status === 'processing';
const liveTrackIdx = isLiveProcessing ? (liveActive.track_index || 0) : 0;
const liveTrackTotal = isLiveProcessing ? (liveActive.track_total || 0) : 0;
const liveTrackName = isLiveProcessing ? (liveActive.track_name || '') : '';
// Parse match data for track details
let matchCount = 0, totalTracks = 0, trackDetails = [];
@ -1218,7 +1241,13 @@ async function importPageLoadSuggestions() {
}
function _renderSuggestionCard(a) {
return `<div class="import-page-album-card" onclick="importPageSelectAlbum('${a.id}')">
// Cache the album lookup so importPageSelectAlbum can pull source +
// name + artist on click (the onclick can only carry the ID string
// — see github issue #524 root cause).
importPageState._albumLookup[a.id] = {
id: a.id, name: a.name || '', artist: a.artist || '', source: a.source || '',
};
return `<div class="import-page-album-card" onclick="importPageSelectAlbum('${_escAttr(a.id)}')">
<img src="${a.image_url || '/static/placeholder.png'}" alt="${_escAttr(a.name)}" loading="lazy" onerror="this.src='/static/placeholder.png'">
<div class="import-page-album-card-title" title="${_escAttr(a.name)}">${_esc(a.name)}</div>
<div class="import-page-album-card-artist" title="${_escAttr(a.artist)}">${_esc(a.artist)}</div>
@ -1245,14 +1274,20 @@ async function importPageSearchAlbum() {
grid.innerHTML = '<div style="color:#888;text-align:center;padding:20px;">No albums found</div>';
return;
}
grid.innerHTML = data.albums.map(a => `
<div class="import-page-album-card" onclick="importPageSelectAlbum('${a.id}')">
grid.innerHTML = data.albums.map(a => {
// Cache album lookup so the click handler can include source
// + name + artist on the match POST (see #524).
importPageState._albumLookup[a.id] = {
id: a.id, name: a.name || '', artist: a.artist || '', source: a.source || '',
};
return `
<div class="import-page-album-card" onclick="importPageSelectAlbum('${_escAttr(a.id)}')">
<img src="${a.image_url || '/static/placeholder.png'}" alt="${_escAttr(a.name)}" loading="lazy" onerror="this.src='/static/placeholder.png'">
<div class="import-page-album-card-title" title="${_escAttr(a.name)}">${_esc(a.name)}</div>
<div class="import-page-album-card-artist" title="${_escAttr(a.artist)}">${_esc(a.artist)}</div>
<div class="import-page-album-card-meta">${a.total_tracks} tracks · ${a.release_date ? a.release_date.substring(0, 4) : ''}</div>
</div>
`).join('');
</div>`;
}).join('');
document.getElementById('import-page-album-clear-btn').classList.remove('hidden');
} catch (err) {
grid.innerHTML = `<div style="color:#ef4444;text-align:center;padding:20px;">Error: ${err.message}</div>`;
@ -1269,8 +1304,22 @@ async function importPageSelectAlbum(albumId) {
matchList.innerHTML = '<div style="color:#888;text-align:center;padding:20px;">Matching files to tracklist...</div>';
try {
// Include file_paths filter if matching from an auto-group
const matchBody = { album_id: albumId };
// Include file_paths filter if matching from an auto-group.
// CRITICAL: include source + album_name + album_artist from the
// search/suggestion result. Without `source`, the backend can't
// route the lookup to the metadata source the album_id came
// from — for example a Deezer album_id needs Deezer's get_album
// call. Cross-source lookup fails silently, returns the
// failure-fallback dict with album_id-as-name + Unknown Artist
// + 0 tracks, then the import flow writes that broken metadata
// to the library DB (github issue #524).
const cached = importPageState._albumLookup[albumId] || {};
const matchBody = {
album_id: albumId,
source: cached.source || '',
album_name: cached.name || '',
album_artist: cached.artist || '',
};
if (importPageState._autoGroupFilePaths) {
matchBody.file_paths = importPageState._autoGroupFilePaths;
importPageState._autoGroupFilePaths = null; // clear after use