From 8493be207e8c0141e82336649ac5b8df14d47cc9 Mon Sep 17 00:00:00 2001 From: Broque Thomas <26755000+Nezreka@users.noreply.github.com> Date: Sat, 9 May 2026 19:25:47 -0700 Subject: [PATCH] Auto-import: SoulSync standalone library writes server-quality rows MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit # Background SoulSync standalone is meant to be a full replacement for Plex / Jellyfin / Navidrome — files imported via auto-import (or any other import path) should land in the database with the same field richness a media-server scan would write. They weren't. # Gaps fixed The auto-import worker built a context dict for each track and handed it to `_post_process_matched_download` (the same callback the regular download flow uses). That dict was missing three things downstream needed: 1. **No `source` field anywhere.** `record_soulsync_library_entry` reads `get_import_source(context)` to pick the source-aware ID columns (`spotify_track_id` / `deezer_id` / `itunes_track_id` / etc.) on the artists / albums / tracks rows. With no source, the resolver returned an empty string → `get_library_source_id_columns("")` returned an empty dict → the `UPDATE tracks SET _id = ?` blocks were silently skipped. Result: every auto-imported track landed with NULL on every source-id column. Watchlist scans (which match by stable source IDs to detect "this track is already in library") couldn't recognise these rows and would re-download them on the next pass. 2. **No `_download_username='auto_import'`.** Both `record_library_history_download` and `record_download_provenance` default to "Soulseek" when no `username` is in the context. Every staging-folder import was being labelled as a Soulseek download in library history + provenance — false signal in the UI. 3. **No per-recording IDs (`isrc`, `musicbrainz_recording_id`) on track_info.** The Navidrome scanner already writes `musicbrainz_recording_id` directly to the tracks row when present. Picard-tagged libraries always carry MBID; metadata sources (Spotify via MusicBrainz enrichment, Deezer, etc.) carry ISRC. Auto-import had access to both via the metadata-source response but didn't propagate them — so the soulsync row went in with NULL on both columns. # Changes **`core/auto_import_worker.py` — `_process_matches`:** - Top-level `'source': source` (from `identification['source']`) - `'_download_username': 'auto_import'` - `track_info['isrc']`, `track_info['musicbrainz_recording_id']` — pulled from the per-track payload returned by the metadata source - `track_info['album_id']` — back-reference so source-aware ID resolution works on sources whose API nests album under `track.album.id` rather than `track.album_id` - `spotify_artist['id']` now correctly carries the artist's source ID (was `identification['album_id']`, a copy-paste bug from the original implementation that made artist-id resolution fall back to fuzzy matching) - `spotify_album['artists'][0]['id']` carries artist source ID for the same resolution path **`core/imports/side_effects.py`:** - `record_library_history_download` source_map: add `"auto_import": "Auto-Import"` — tags imported tracks correctly - `record_download_provenance` source_service: add `"auto_import": "auto_import"` — provenance shows real source - `record_soulsync_library_entry` track INSERT: now includes `musicbrainz_recording_id` + `isrc` columns (matches `insert_or_update_media_track`'s shape for Navidrome / Plex / Jellyfin scans). Both default to NULL when not present. # Behavior preserved - Files still land in the same library template path (no path-build change) - Other media-server flows (Plex / Jellyfin / Navidrome users) unaffected — `record_soulsync_library_entry` still gates on `get_active_media_server() == "soulsync"`. Auto-import on those servers continues to drop the file in the library folder + emits `batch_complete` for the scan-trigger automation, same as before. - Direct downloads (search → Download button) unaffected — they already passed `source` + `username` correctly. # Tests added `tests/imports/test_auto_import_context_shape.py` (8 tests, new file): - Worker context carries `source` for every metadata source (parametrised across spotify / deezer / itunes / discogs) - `_download_username='auto_import'` set unconditionally - ISRC + MBID propagate from track payload to track_info when present - ISRC + MBID default to empty string when absent (downstream normalises to NULL at write time) - track_info includes album-id back-reference `tests/imports/test_import_side_effects.py` (4 new tests + 2 schema column adds): - `record_soulsync_library_entry` writes mbid + isrc columns when present in track_info - Deezer source maps to deezer_id column (regression case for source-aware column resolver) - `record_library_history_download` labels `_download_username= 'auto_import'` as "Auto-Import" not "Soulseek" - `record_download_provenance` registers source_service as "auto_import" not "soulseek" # Verification - 8/8 new context-shape tests pass - 6/6 side-effects tests pass (4 new + 2 existing) - 207 imports tests pass - 2359 full suite passes (+12 from baseline 2347, no regressions) - 1 pre-existing flake (`test_watchdog_warns_about_stuck_workers`, passes in isolation, unrelated to this change) - Ruff clean --- core/auto_import_worker.py | 54 +++- core/imports/side_effects.py | 33 ++- .../imports/test_auto_import_context_shape.py | 255 ++++++++++++++++++ tests/imports/test_import_side_effects.py | 187 ++++++++++++- webui/static/helper.js | 1 + 5 files changed, 523 insertions(+), 7 deletions(-) create mode 100644 tests/imports/test_auto_import_context_shape.py diff --git a/core/auto_import_worker.py b/core/auto_import_worker.py index abd53911..3144a994 100644 --- a/core/auto_import_worker.py +++ b/core/auto_import_worker.py @@ -1526,23 +1526,61 @@ class AutoImportWorker: continue try: - # Build context matching the manual import format + # Build context matching the manual import format. + # + # The post-process pipeline (`_post_process_matched_download` + # → `record_soulsync_library_entry`) reads `source` to pick + # the right source-id columns on artists/albums/tracks, + # and reads `_download_username` to label the row in + # library history + provenance. Without these the SoulSync + # standalone library lands the file but leaves + # `spotify_track_id` / `deezer_id` / etc. NULL and tags the + # provenance row as "Soulseek" (the default fallback). + # SoulSync standalone is a full server replacement, so the + # row must carry the same field richness as a Plex/Jellyfin/ + # Navidrome scan would write. context_key = f"auto_import_{candidate.folder_hash}_{track_number}" + # Album-level identifiers from the metadata source response. + # `album_data['id']` is the source-native album id (e.g. + # spotify album id, deezer album id). Identification fed it + # into `identification['album_id']` already; prefer the + # album_data version since it's authoritative when both + # are present. + source_album_id = album_data.get('id') or identification.get('album_id') or '' + # ISRC + MusicBrainz Recording ID — propagated by the + # metadata layer (`_build_album_track_entry`) so files + # tagged with these IDs can match later watchlist scans + # without relying on fuzzy title comparison. + track_isrc = track.get('isrc', '') or '' + track_mbid = track.get('musicbrainz_recording_id', '') or track.get('mbid', '') or '' context = { + # Top-level `source` is the canonical signal that the + # imports pipeline reads via `get_import_source()`. + # `get_library_source_id_columns(source)` then picks + # the right column on artists/albums/tracks for the + # source-aware UPDATE. + 'source': source, + # `_download_username` is read by + # `record_library_history_download` + + # `record_download_provenance` to label the row. + # 'auto_import' maps to "Auto-Import" / "auto_import" + # in those source maps so the UI doesn't show every + # imported file as "Soulseek". + '_download_username': 'auto_import', 'spotify_artist': { - 'id': identification.get('album_id') or 'auto_import', + 'id': identification.get('artist_id') or '', 'name': artist_name, 'genres': [], }, 'spotify_album': { - 'id': album_data.get('id') or identification.get('album_id') or '', + 'id': source_album_id, 'name': album_name, 'release_date': release_date, 'total_tracks': album_data.get('total_tracks', match_result.get('total_tracks', 0)), 'total_discs': total_discs, 'image_url': image_url, 'images': album_data.get('images', [{'url': image_url}] if image_url else []), - 'artists': [{'name': artist_name}], + 'artists': [{'name': artist_name, 'id': identification.get('artist_id') or ''}], 'album_type': album_data.get('album_type', 'album'), }, 'track_info': { @@ -1553,6 +1591,14 @@ class AutoImportWorker: 'duration_ms': track.get('duration_ms', 0), 'artists': track.get('artists', [{'name': artist_name}]), 'uri': track.get('uri', ''), + # Album-id back-reference + per-recording IDs so + # `get_import_source_ids` can resolve them onto + # the right column even when the source's API + # nests them under `album.id` rather than + # `track.album_id`. + 'album_id': source_album_id, + 'isrc': track_isrc, + 'musicbrainz_recording_id': track_mbid, }, 'original_search_result': { 'title': track_name, diff --git a/core/imports/side_effects.py b/core/imports/side_effects.py index 6252abd7..35b3a4c9 100644 --- a/core/imports/side_effects.py +++ b/core/imports/side_effects.py @@ -89,6 +89,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 +167,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 " 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 {} @@ -416,14 +429,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 +463,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") diff --git a/tests/imports/test_auto_import_context_shape.py b/tests/imports/test_auto_import_context_shape.py new file mode 100644 index 00000000..37155f0c --- /dev/null +++ b/tests/imports/test_auto_import_context_shape.py @@ -0,0 +1,255 @@ +"""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" diff --git a/tests/imports/test_import_side_effects.py b/tests/imports/test_import_side_effects.py index 33bfa1e4..92e00706 100644 --- a/tests/imports/test_import_side_effects.py +++ b/tests/imports/test_import_side_effects.py @@ -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,185 @@ 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" + + +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" diff --git a/webui/static/helper.js b/webui/static/helper.js index 5b9107f5..1bf44555 100644 --- a/webui/static/helper.js +++ b/webui/static/helper.js @@ -3416,6 +3416,7 @@ const WHATS_NEW = { '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: 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 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). 14 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, 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//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' },