SoulSync

Commit Graph

Author	SHA1	Message	Date
Broque Thomas	ff27effdae	Amazon download client: write final size==transferred before returning file path The download monitor blocks post-processing with a bytes-incomplete guard: if size > 0 and transferred < size: continue _stream_to_file throttles engine updates to every 0.5s. The last tick before the file finishes typically leaves transferred slightly below the Content-Length size in the engine record. Other streaming clients (YouTube, Tidal, HiFi, etc.) use their own download threads and don't track bytes at all, so size stays 0 and the guard is always skipped. Amazon was the only client hitting it. Fix: just before returning the file path from _download_sync, write a final engine record update setting size == transferred == out_path.stat().st_size (the decrypted output size). The bytes-incomplete guard then sees transferred == size and falls through to trigger post-processing normally.	2 weeks ago
Broque Thomas	b4403ed393	Amazon download client: fix engine API calls in status methods `get_all_downloads` was calling `engine.get_all_records()` — a method that doesn't exist on DownloadEngine. Same story for `cancel_record` and `clear_completed`. The engine exposes `iter_records_for_source`, `get_record`, `update_record`, and `remove_record` — matching what every other streaming client (Deezer, HiFi, Qobuz, SoundCloud, Tidal, YouTube) already uses. With `get_all_downloads` silently returning `[]` on every call (the missing method raised, `except Exception: return []` swallowed it), the download monitor never saw Amazon records as complete — tasks stayed stuck at 0% even after the file had fully downloaded. Changes: - `get_all_downloads` → `iter_records_for_source('amazon')` - `get_download_status` → `get_record('amazon', id)`, no try/except - `cancel_download` → `get_record` check + `update_record` (Cancelled) + optional `remove_record` — same pattern as deezer/hifi/etc - `clear_all_completed_downloads` → iterate + `remove_record` for terminal states; returns True on no-engine (nothing to clear = success) - `_record_to_status` drops the `download_id` argument; reads `rec['id']` instead (worker stores `'id'` in every record — `iter_records_for_source` returns the full record dict) Tests updated to match: `iter_records_for_source` mock replaces `get_all_records`, cancel test verifies `update_record`+`remove_record`, clear test verifies only terminal-state records are removed, graceful-error test replaced with no-records boundary test (exception propagation is handled at the engine aggregator layer, not per-plugin).	2 weeks ago
Broque Thomas	ebda0b8613	fix(amazon): _record_to_status read 'filename' not 'original_filename' The engine worker stores the encoded filename under the key 'filename' (see worker.py dispatch). _record_to_status was reading 'original_filename', which always returns "" — so every DownloadStatus emitted by get_all_downloads/get_download_status had an empty filename string. The download monitor builds lookup keys as _make_context_key(download.username, download.filename). With filename="" the key was always "amazon::" which never matched the task's "amazon::B0B1234\|\|Artist - Title" key. Monitor never detected Amazon download completions, so tasks sat stuck at Downloading 0% forever even though the files had actually downloaded. Also fixes tests that had the same wrong key.	2 weeks ago
Broque Thomas	9fb63ff86d	fix(amazon): add set_engine/set_shutdown_check so _engine gets wired AmazonDownloadClient was missing set_engine() and set_shutdown_check(). The download engine auto-wires plugins by calling set_engine(self) at registration time if the method exists (engine.py:136). Without it, _engine stayed None forever, causing every download() call to raise RuntimeError("_engine is not set") — silently failing and marking all tracks not found. All other streaming clients (Deezer, Qobuz, Tidal, HiFi, SoundCloud) expose set_engine(); Amazon now matches the pattern. Tests added: set_engine wires _engine, set_shutdown_check wires callback, set_engine unblocks download dispatch (the exact live failure mode).	2 weeks ago
Broque Thomas	791e3630ff	fix(amazon): wire amazon into all streaming-source guards `validation.py` had amazon absent from `_streaming_sources`, causing Amazon TrackResult objects (bitrate=None, size=0) to fall through to the Soulseek P2P code path and get rejected by `filter_results_by_quality_preference`. Every album track was marked not found. Fix: add 'amazon' to every streaming-source guard tuple/set that was previously missing it: - core/downloads/validation.py — primary bug fix (quality-filter bypass) - core/downloads/status.py — _STREAMING_SOURCE_NAMES frozenset - core/downloads/task_worker.py — hybrid fallback client map - core/imports/side_effects.py — \|\| filename→stream-id extraction - web_server.py — is_streaming_source, transfer list display, candidate source label, _try_source_reuse, _store_batch_source - tests/test_download_plugin_conformance.py — registry count + parametrize Also updates the 2.5.3 What's New entry to drop the stale "not yet wired" disclaimer.	2 weeks ago
Broque Thomas	85984d4174	Amazon Music provider: metadata client + download source (T2Tunes) core/amazon_client.py — T2Tunes-backed metadata client following the DeezerClient/iTunesClient contract. Exposes search_tracks, search_artists, search_albums, get_track_details, get_album, get_album_tracks, get_artist, get_artist_albums, get_track_features. T2TunesStreamInfo dataclass captures the hex decryption key returned by the proxy (CENC/AES-128). Handles the "stremeable" API typo. 0.5 s rate-limit guard + api_call_tracker. core/amazon_download_client.py — DownloadSourcePlugin backed by the above client. Codec waterfall: FLAC → Opus → EAC3. Downloads the encrypted MP4 container, decrypts with ffmpeg -decryption_key, yields the native audio file (.flac / .opus / .eac3). Not yet wired into the app source registry — validated in isolation only; see tests/tools/. tools/t2tunes_probe.py + tools/t2tunes_media_plan.py — standalone CLI tools used for live API exploration during development. tests/tools/test_amazon_client.py — 72 unit tests (all mocked). tests/tools/test_amazon_download_client.py — 52 unit tests (all mocked). 124 tests pass.	2 weeks ago
Broque Thomas	115d7ed9c5	Preserve personalized playlist metadata for wishlist	2 weeks ago
Broque Thomas	d861a40277	Personalized pipeline: refresh snapshot on first-run too Reproduced: selecting Fresh Tape (or any kind never generated before) and running the pipeline silently skipped — UI showed "No tracks in Fresh Tape — skipping sync" with no clue why. Root cause: ensure_playlist auto-creates the playlist row on first access with `track_count=0` and `last_generated_at=NULL`, but `is_stale=0` by default (the column default — fresh rows aren't "stale", they're "never generated"). Pipeline only refreshed when `is_stale=True` OR `refresh_first=True`, so first-run rows fell through both branches → read the empty snapshot → skip. Fix: pipeline now also refreshes when `existing.last_generated_at is None`. Same control flow, one extra condition: if refresh_first OR is_stale OR last_generated_at is None: refresh else: read existing snapshot This is the right signal: "has the generator ever run for this row" is exactly what `last_generated_at` tracks (the column is set in `_persist_snapshot` after every successful refresh). Stubs in test_handlers_personalized_pipeline.py updated to expose `last_generated_at` on their SimpleNamespace returns so the new attribute read doesn't AttributeError. Fresh stubs get a non-None timestamp so they're treated as already-generated; the new test `test_never_generated_snapshot_triggers_first_refresh` pins the first-run-forces-refresh behavior with `last_generated_at=None`.	2 weeks ago
Broque Thomas	877d0e7d81	Personalized pipeline: auto-refresh stale snapshots after watchlist scan Snapshots now track when their source data changes. Watchlist scan emits stale flags on the playlists whose underlying pool just got refreshed; the next pipeline run sees the flag and regenerates the snapshot before syncing, so the server playlist never lags the source. Schema: - new `is_stale INTEGER NOT NULL DEFAULT 0` column on `personalized_playlists`, plus an idempotent ADD COLUMN migration in `ensure_personalized_schema` for installs created before this PR. - `PlaylistRecord.is_stale: bool = False` exposed on the dataclass so callers can branch on freshness without re-querying. Manager: - new `mark_kinds_stale(kinds, profile_id=None)` flips the flag in bulk for a list of kinds (used by upstream data refreshers). - `_persist_snapshot` clears `is_stale = 0` on successful refresh. - SELECT statements + `_row_to_record` updated to read the column (with tuple-form length guard for safety). Pipeline: - `_build_payloads_for_kinds` now branches: refresh_first=True OR `existing.is_stale` -> refresh_playlist, else read existing snapshot. So the auto-refresh kicks in without needing the user to toggle the refresh-each-run option. Watchlist scanner emits stale flags at three sites: - after `update_discovery_pool_timestamp` -> marks pool-fed kinds stale: hidden_gems, discovery_shuffle, popular_picks, time_machine, genre_playlist, daily_mix. - after release_radar `save_curated_playlist` -> marks `fresh_tape`. - after discovery_weekly `save_curated_playlist` -> marks `archives`. All three calls go through a module-level `_mark_personalized_kinds_stale` helper that builds a PersonalizedPlaylistManager with `deps=None` (only DB access is needed for the flag update — no generator dispatch). Each call is wrapped in try/except so a flag failure can never abort the scan itself. Tests: - new `TestStaleFlag` class in `test_personalized_manager.py` (6 tests): default-false, single-kind flip, multi-kind, profile scoping, refresh-clears, empty-list noop. - two new pipeline tests pin the auto-refresh dispatch: `test_stale_snapshot_auto_refreshes_even_without_refresh_first` and `test_non_stale_snapshot_skips_refresh`. - existing stub-manager `SimpleNamespace` returns gained `is_stale=False` so the new attribute read doesn't AttributeError. Full suite: 3391 pass. User-facing WHATS_NEW entry added under 2.5.2 (above the prior pipeline auto-sync entry) describing the auto-refresh behavior.	2 weeks ago
Broque Thomas	e1f0810df5	Personalized pipeline: UI multi-select picker for kinds + variants The action was registered + the block declared, but the automation builder's per-action config renderer didn't have a case for `personalized_pipeline` so users only saw the bare card with the generic delay-minutes input — no way to select which playlists to sync. This commit adds the multi-select picker. Backend: - `core/personalized/api.list_kinds(manager=...)` now optionally takes a manager and includes the resolved variant list per kind (calls each spec's variant_resolver(deps) when present). Singleton kinds get an empty `variants` list. Variant-bearing kinds (time_machine / genre_playlist / daily_mix / seasonal_mix) get their full enumerated set. - `web_server.py` `/api/personalized/kinds` route now passes a built manager so the variants list lands in the response. Frontend: - `webui/static/stats-automations.js` `_renderBlockConfigFields` gains a `personalized_pipeline` branch that renders a scrollable multi-select picker: - Singletons (Hidden Gems, Discovery Shuffle, Popular Picks, Fresh Tape, The Archives) = one checkbox row per kind - Variant kinds = a section header + one checkbox row per variant (e.g. Time Machine: 1960s/1970s/.../2020s; Seasonal: halloween/ christmas/valentines/summer/spring/autumn) - Pre-checks rows that match the existing `kinds` config on edit - New `_autoLoadPersonalizedKinds(slotKey)` fetches `/api/personalized/kinds` (cached after first load), renders the picker DOM, and pre-checks saved selections via `data-kind` / `data-variant` attributes on the checkboxes. - `_renderBuilderCanvas` calls the loader for any `cfg-*-kinds-picker` it finds in the freshly-rendered slots. - The save-time `_collectActionConfig` walks the picker's checked inputs (matched by `data-kind` attribute) and emits `{kinds: [{kind, variant?}, ...], refresh_first, skip_wishlist}` in the same shape the handler expects. Tests: - `tests/automation/test_automation_blocks.py::_FIELD_TYPES` adds 'personalized_playlist_select' so the block-shape regression test accepts the new field type. (Test was failing because it whitelists every field type used across all blocks.) - 189 automation + personalized API tests pass; full suite intact.	2 weeks ago
Broque Thomas	cc44254bf9	Personalized playlist pipeline: auto-sync discover-page playlists Follow-up to the personalized-playlists standardization PR. New `personalized_pipeline` automation action syncs selected discover- page playlists (Hidden Gems / Discovery Shuffle / Time Machine / Genre / Daily Mix / Fresh Tape / The Archives / Seasonal Mix) to the active media server + queues missing tracks for download. Same pattern as the existing mirrored `playlist_pipeline` but two phases instead of four — no REFRESH (no external source to re-pull) and no DISCOVER (manager-backed snapshots are already metadata- matched). Pipeline shape: SNAPSHOT → SYNC → WISHLIST Where SNAPSHOT either reads the persisted track list from `PersonalizedPlaylistManager` (default) or refreshes it first when `refresh_first=true` (cron use case: regenerate Hidden Gems nightly and sync the fresh set). Shared helper extraction: PHASE 3 (SYNC loop) + PHASE 4 (WISHLIST tail) lifted out of mirrored `playlist_pipeline` into `core/automation/handlers/_pipeline_shared.py` as `run_sync_and_wishlist(deps, automation_id, playlists, sync_one_fn, sync_id_for_fn, ...)`. Both pipelines call it. Mirrored injects `auto_sync_playlist` as the per-playlist sync function; personalized injects a thin wrapper that launches `_run_sync_task` directly with a pre-built tracks_json. Same sync-state polling / progress emission / status counting / wishlist trigger logic — 0 duplication. Files added: - core/automation/handlers/_pipeline_shared.py - core/automation/handlers/personalized_pipeline.py - tests/automation/test_handlers_personalized_pipeline.py Files changed: - core/automation/handlers/playlist_pipeline.py: PHASE 3+4 replaced with shared helper call (~100 lines deleted, 1 helper invocation added; behavior identical). - core/automation/deps.py: new `build_personalized_manager` field (lazy builder so the pipeline gets a fresh PersonalizedPlaylistManager per run). - core/automation/handlers/__init__.py + registration.py: register `personalized_pipeline` action with the shared `pipeline_running` guard so it can't overlap mirrored. - core/automation/blocks.py: new `personalized_pipeline` block declaration with config_fields (kinds multi-select, refresh_first, skip_wishlist). - web_server.py: thread `_build_personalized_manager` into AutomationDeps construction. - All 5 automation test fixtures: `_build_deps` adds `build_personalized_manager=lambda: None` stub. - tests/automation/test_handler_registration.py: EXPECTED_ACTION_NAMES + EXPECTED_GUARDED_ACTIONS gain `personalized_pipeline`. Trigger schema: { "_automation_id": "...", "kinds": [ {"kind": "hidden_gems"}, {"kind": "time_machine", "variant": "1980s"}, {"kind": "seasonal_mix", "variant": "halloween"} ], "refresh_first": false, "skip_wishlist": false } Tests (14 new, 178 automation total): - _track_to_sync_shape: basic shape, source ID fallback chain, no-id returns empty string - empty config / non-list kinds / empty kinds list all return error + clear pipeline_running flag - _build_payloads_for_kinds: skips invalid entries, skips kinds with no tracks, refresh_first vs ensure dispatch, payload shape + sync_id format, manager exception swallowed continues - _sync_personalized_playlist: launches background thread + returns status='started' - happy path: stubbed sync_states drives helper to completion, flag cleaned up Full suite: 3383 passed. Note: the trigger UI block declares config_fields but the frontend doesn't yet render the `personalized_playlist_select` multi-select type — usable today via API; polished UI ships in a follow-up frontend PR.	2 weeks ago
Broque Thomas	cc0828e9ff	Personalized playlists (4/N): staleness post-filter (exclude_recent_days) Adds the first quality feature on top of the manager: when `config.exclude_recent_days > 0`, the manager drops any track from the generator's output whose primary id was served by this kind for this profile in the last N days. Lives at the manager layer, not in each generator, so: - generators stay focused on selection logic - staleness behavior stays uniform across every kind - enabling/disabling per playlist is just a config patch Implementation: - New `PersonalizedPlaylistManager._apply_quality_filters` runs after generator returns, before `_persist_snapshot`. - Reads recent ids via existing `recent_track_ids` accessor. - Tracks without a primary id pass through unchanged (nothing to dedupe on -- happens for sourceless tracks during edge cases). - Returns a new list (never mutates input). Default `exclude_recent_days = 0` preserves pre-overhaul behavior. Per-playlist override via `PUT /api/personalized/playlist/<kind>/config` with `{"exclude_recent_days": N}`. Recommended values: - Discovery Shuffle: 1-3 days (high churn desired) - Hidden Gems: 7-14 days (avoid same gems weekly) - Time Machine / Genre: 30+ days (slow rotation OK, stable view preferred) 4 new boundary tests: - Zero days = no filter (default behavior preserved) - Positive days drops tracks served in window - Filter preserves new tracks alongside dropped ones - Tracks without primary id pass through unchanged 3369 tests pass total. Note: listening-history cross-ref + seeded shuffle are deferred to a future PR. Each requires deeper integration -- listening history needs a play-events table the discovery pool can query against; seeded shuffle needs the legacy generators to accept a seed param without breaking their existing diversity / popularity logic.	2 weeks ago
Broque Thomas	9f383acbfb	Personalized playlists (3/N): standardized API endpoints Wraps the manager + generator dispatch behind one HTTP surface so the UI can drop the patchwork `/api/discover/personalized/*` calls in favor of a single REST shape. Legacy endpoints stay alive for backward compat during the UI migration window. New endpoints: - GET /api/personalized/kinds — list every registered kind + metadata - GET /api/personalized/playlists — list every persisted playlist for the active profile - GET /api/personalized/playlist/<kind> — fetch singleton + tracks - GET /api/personalized/playlist/<kind>/<variant> — fetch variant + tracks - POST /api/personalized/playlist/<kind>/refresh — regenerate singleton - POST /api/personalized/playlist/<kind>/<variant>/refresh — regenerate variant - PUT /api/personalized/playlist/<kind>/config — patch singleton config - PUT /api/personalized/playlist/<kind>/<variant>/config — patch variant config Per-call manager construction wires the deps each generator needs: - database (MusicDatabase singleton) - service (PersonalizedPlaylistsService for legacy generator calls) - seasonal_service (SeasonalDiscoveryService for seasonal_mix) - get_current_profile_id (active profile accessor) - get_active_discovery_source (source dispatcher) API handlers themselves live as pure functions in `core/personalized/api.py` so they're testable without Flask. The Flask layer in `web_server.py` is a thin parse-body / call-handler / jsonify wrapper. 11 new boundary tests (122 personalized total): - list_kinds enumerates registry, exposes default config + tags - list_playlists returns empty list when none exist, serializes PlaylistRecord shape correctly - get_playlist_with_tracks auto-creates on first access, returns persisted tracks, raises ValueError on unknown kind - refresh_playlist runs generator and returns track snapshot, forwards config_overrides to the generator - update_config patches stored config 3365 tests pass total. Manager construction triggers generator registration via `from core.personalized import generators` import side-effect.	2 weeks ago
Broque Thomas	53284ee7c8	Personalized playlists (2/N): all 8 generators wired through manager Adds the per-kind generator modules and registers them with the PlaylistKindRegistry so the manager's `refresh_playlist` can dispatch to any of them. Generators (each in its own module under `core/personalized/generators/`): Singletons (variant=''): - hidden_gems -> wraps service.get_hidden_gems - discovery_shuffle -> wraps service.get_discovery_shuffle - popular_picks -> wraps service.get_popular_picks Variant-bearing kinds: - time_machine -> variant = decade label ('1980s', '1990s', ...). Variant resolver returns 7 standard decades. Generator parses '1980s' -> 1980 + delegates to service.get_decade_playlist. - genre_playlist -> variant = URL-safe genre key ('electronic_dance', 'hip_hop_rap', ...). Resolver normalizes parent-genre keys from service.GENRE_MAPPING; free-form keywords pass through to service.get_genre_playlist. - daily_mix -> variant = top-genre rank ('1' / '2' / '3' / '4'). Generator looks up user's Nth-ranked library genre and returns discovery picks within it. Library half (was a stub returning []) is intentionally dropped: tracks table has no source IDs, so library rows can't sync. Fixed the stub to return [] cleanly without the misleading log warning. - fresh_tape -> Spotify Release Radar. Reads curated track IDs from discovery_curated_playlists (tries 'release_radar_<source>' first, falls back to 'release_radar') and hydrates against the discovery pool. - archives -> Spotify Discover Weekly. Same hydration path as fresh_tape but uses 'discovery_weekly'. - seasonal_mix -> variant = season key ('halloween' / 'christmas' / 'valentines' / 'summer' / 'spring' / 'autumn'). Reads curated IDs via SeasonalDiscoveryService then hydrates from seasonal_tracks (which carries full track_data_json). Each module: - Defines `generate(deps, variant, config) -> List[Track]`. - Defines `SPEC = PlaylistKindSpec(...)` and registers it on import (idempotent — re-import safe via `if registry.get(...) is None`). - For variant-bearing kinds, also defines `variant_resolver(deps)`. Shared helpers in `_common.py`: - `get_service(deps)` pulls the legacy `PersonalizedPlaylistsService` instance (deps.service or deps['service']). - `coerce_tracks(rows)` runs each dict through `Track.from_dict`, tolerates None / non-list inputs. Tests (50 new, total 85 across personalized subsystem): - Singletons: registration + display name + dispatch + limit forwarding + empty/None tolerance + missing-deps error + dict-form deps acceptance (16 tests). - Variants: variant_resolver listing + label parsing + invalid variant errors + parent-key normalization + free-form passthrough (13 tests). - Curated/hybrid: daily_mix rank-to-genre resolution + rank-out-of- range empty + invalid-variant error; fresh_tape & archives hydration order + missing-id skip + source-specific-then-fallback key dispatch + limit + missing-database-dep error; seasonal_mix curated-id hydration order + missing-id skip + JSON round-trip + empty-curated empty + limit + missing-service error (21 tests). 3304+ tests pass. No regression on existing 62 personalized tests.	2 weeks ago
Broque Thomas	79224ed294	Personalized playlists (1/N): unified storage + manager foundation Begins the standardization of the personalized-playlist subsystem. Pre-existing state was a patchwork: Group A (Fresh Tape / Archives / Seasonal Mix) lived in `discovery_curated_playlists` and `curated_seasonal_playlists` with inconsistent shapes; Group B (Hidden Gems / Discovery Shuffle / Time Machine / Popular Picks / Genre / Daily Mixes) was computed on-demand by `PersonalizedPlaylistsService` with no persistence -- every call reran the generator with `ORDER BY RANDOM()` so results rotated. Post-overhaul (this PR) every personalized playlist lands in one unified storage layer with stable identity, persistent track lists, explicit refresh, and per-playlist user-tweakable config. Foundation in this commit (no behavior change yet): - `database/personalized_schema.py`: 3 tables created idempotently at app startup (wired into `MusicDatabase._initialize_database`). - `personalized_playlists`: one row per (profile, kind, variant) with config_json, track_count, last_generated_at, last_synced_at, last_generation_source, last_generation_error. Variant '' (empty string) for singletons; non-empty for time_machine / seasonal_mix / genre_playlist / daily_mix. - `personalized_playlist_tracks`: current snapshot per playlist. Atomically replaced on refresh. - `personalized_track_history`: append-only log powering the `exclude_recent_days` config knob. - `core/personalized/types.py`: `Track`, `PlaylistConfig`, `PlaylistRecord` dataclasses. `PlaylistConfig.merged()` for partial-update PATCH semantics; `Track.from_dict()` accepts the legacy generator output shape unchanged. - `core/personalized/specs.py`: `PlaylistKindSpec` (kind, name_template, default_config, generator, variant_resolver) and a module-level registry. Generators register at import time; manager dispatches by kind. - `core/personalized/manager.py`: `PersonalizedPlaylistManager` -- the only thing that touches the new tables. Owns: - ensure_playlist (auto-create row from kind defaults) - get_playlist / list_playlists - refresh_playlist (atomic snapshot replace; generator exception preserves previous good snapshot + records error on row) - get_playlist_tracks - update_config (deep-merge with stored config, including extra dict) - recent_track_ids (staleness lookup for generators) 35 boundary tests in `tests/test_personalized_manager.py` pin every shape: config round-trip / merge semantics / extra deep-merge / defaults; Track.from_dict tolerance + primary_id fallback chain; registry dedup / display_name with+without variant; manager ensure_playlist auto-create + idempotency, variant separation, required-variant enforcement, unknown-kind error; refresh persists + replaces atomically + survives generator exception with previous snapshot intact + records source from first track + round-trips nested track_data_json; update_config patch semantics; list_playlists profile scoping; staleness history scoped to (profile, kind, days). 3304 tests pass total. Generators ship in subsequent commits on this branch -- each kind migrated one at a time with its own per-kind boundary tests.	2 weeks ago
Broque Thomas	d3768610d7	Extract automation handlers (kettui-bar): engine-boundary tests Per-handler boundary tests pin each handler's body in isolation. Adding engine-boundary tests that pin the REGISTRATION layer: - every expected action name registered, no drops, no extras - guarded actions register a guard, unguarded ones don't - every registered handler is callable - every guard returns a bool - all four progress callbacks registered in the right slots - progress_init / progress_finish / record_history / on_library_scan_completed are invocable through the engine's stored callable shape (not just the bare extracted function) - finish callback respects _manages_own_progress flag at the engine boundary too - library_scan_completed wiring registers a callback on the scan manager and that callback fires engine.emit when invoked - every handler returns a `{'status': ...}` dict on a minimal config trigger -- proves no handler raises into the engine, even when its guard / short-circuit / error path is the one taken Uses a minimal _RecordingEngine that captures registrations + a _RecordingScanMgr that captures completion callbacks. No real AutomationEngine, no real Flask app, no real DB. The kettui standard for refactor PRs: don't ship "behavior preserved" claim that's only validated at the function boundary -- exercise the engine seam too. EXPECTED_ACTION_NAMES + EXPECTED_GUARDED_ACTIONS frozen sets at the top: any future drift (rename / drop / add a handler / change which ones are guarded) fails this test immediately so refactor PRs can't quietly mutate the registration shape. 13 new tests, 164 automation tests pass total.	2 weeks ago
Broque Thomas	e140da117a	Extract automation handlers (4/3 — finish): progress callbacks + scan-completion emitter Cleans up the four remaining inline callbacks at the bottom of `web_server._register_automation_handlers` so the function is now purely deps-construction + register_all + a logger.info line. Lifted: - `_progress_init`, `_progress_finish`, `_record_automation_history`, and `_on_library_scan_completed` -> core/automation/handlers/progress_callbacks.py Each is a top-level function that takes deps as a parameter; the engine sees thin lambdas through `register_progress_callbacks` / `register_library_scan_completed_emitter` (called from `register_all`). Two new deps fields: - `init_automation_progress` (delegates into the live progress tracker) - `record_progress_history` (delegates into _auto_progress.record_history) 12 new boundary tests in tests/automation/test_progress_callbacks.py pin every shape: - progress_init forwards to init_automation_progress - progress_finish skips when handler manages its own progress (prevents double-emit of finished status) - progress_finish: completed -> finished/Complete/success; error -> error/Error/error; msg falls through error -> reason -> status -> 'done' - record_history threads the live db into the recorder - on_library_scan_completed: no engine = noop, server type taken from web_scan_manager._current_server_type, defaults to 'unknown' - register_library_scan_completed_emitter: no scan manager = noop, registered callback emits the right event when invoked 3256 tests pass, no regression. Final state of `_register_automation_handlers`: - Was: 1530 lines, 21 nested closures + 4 progress callbacks - Now: ~50 lines, builds AutomationDeps and calls register_all web_server.py: 34,220 -> 34,187 lines (-33 net, -1,406 across the whole branch).	2 weeks ago
Broque Thomas	017553193f	Extract automation handlers (3/3): maintenance + misc, finishing the lift Final commit of the automation-handler refactor. With this commit every closure that used to live in `web_server._register_automation_handlers` is now a top-level function in `core/automation/handlers/`. Handlers extracted in this commit: - start_database_update + deep_scan_library -> core/automation/handlers/database_update.py Both share the db_update_state monitoring pattern (poll until status flips, stall detection emits warning at 10 min, 2-hour outer timeout). Lifted into a shared `_run_with_progress` helper inside the module so the per-handler bodies stay tiny. - run_duplicate_cleaner -> core/automation/handlers/duplicate_cleaner.py - start_quality_scan -> core/automation/handlers/quality_scanner.py - clear_quarantine, cleanup_wishlist, update_discovery_pool, backup_database, refresh_beatport_cache -> core/automation/handlers/maintenance.py Grouped because each body is short (~20-50 lines) and they share no state — splitting into per-handler files would just add import noise. - clean_search_history, clean_completed_downloads, full_cleanup -> core/automation/handlers/download_cleanup.py Grouped because all three reach the download orchestrator, tasks_lock, and download_batches/download_tasks accessors. The full_cleanup multi-step orchestration shares phase-detection logic with clean_completed_downloads. - run_script -> core/automation/handlers/run_script.py - search_and_download -> core/automation/handlers/search_and_download.py `AutomationDeps` grew with the new dependency surface: - get_db_update_state + db_update_lock + db_update_executor + run_db_update_task + run_deep_scan_task - get_duplicate_cleaner_state + duplicate_cleaner_lock + duplicate_cleaner_executor + run_duplicate_cleaner - get_quality_scanner_state + quality_scanner_lock + quality_scanner_executor + run_quality_scanner - download_orchestrator + run_async + tasks_lock + get_download_batches + get_download_tasks + sweep_empty_download_directories + get_staging_path - docker_resolve_path + get_current_profile_id + get_watchlist_scanner + get_app + get_beatport_data_cache - set_db_update_automation_id (writes the legacy global so the live DB-update progress callbacks still living in web_server.py keep emitting against the active automation card) `web_server._register_automation_handlers` is now ~50 lines: build deps once, call register_all. The 667-line block of remaining closure definitions and engine register calls is gone. The final orphan was the `_db_update_automation_id` module global — the DB-update progress callbacks at line ~14080 still read it directly, so the extracted database_update handler propagates the automation id through `deps.set_db_update_automation_id` (a closure in web_server that writes the global). When the legacy callbacks get extracted in a future PR the setter goes away. Tests: - tests/automation/test_handlers_maintenance.py adds 21 boundary tests covering every newly-extracted handler shape: guard short-circuits (already-running returns skipped), deps wiring (set_db_update_automation_id called with the right id), exception swallow contract, status returns, path-traversal blocked in run_script, source-mode skip in clean_search_history, active-batch skip in clean_completed_downloads, etc. - 3244 tests pass (was 3223 — 21 new), no regression. web_server.py: 35,593 -> 34,220 lines (-1,373 net across 3 commits). Issue #1 from the extraction punch list is now COMPLETE.	2 weeks ago
Broque Thomas	cde237c7e7	Extract automation handlers (2/N): playlist lifecycle group Continues the lift from `web_server._register_automation_handlers`. This commit extracts the four playlist-lifecycle closures: - `refresh_mirrored` -> core/automation/handlers/refresh_mirrored.py - `sync_playlist` -> core/automation/handlers/sync_playlist.py - `discover_playlist` -> core/automation/handlers/discover_playlist.py - `playlist_pipeline` -> core/automation/handlers/playlist_pipeline.py The pipeline composes refresh + sync + discover, so all four ship together. The pipeline imports the other three handler modules directly (cross-handler call) instead of going through the engine, preserving the "single trigger from the user's perspective" UX. `AutomationDeps` grew to cover the new dependency surface: - run_playlist_discovery_worker, run_sync_task, load_sync_status_file (pre-existing background-task entry points) - get_deezer_client, parse_youtube_playlist (per-source clients) - get_sync_states (live mutable accessor for the sync UI's state dict) `web_server._register_automation_handlers` now wires those plus the existing infrastructure into a single `AutomationDeps` and calls `register_all`. The 669-line block of closure definitions and engine register calls (lines 959-1627 pre-edit) is gone -- the file shed 743 lines net on this commit. `tests/automation/test_handlers_playlist.py` adds 17 new boundary tests: - discover_playlist: no_id error, specific_id starts worker, all=True enumerates, no playlists in db - refresh_mirrored: error path, source filter (file/beatport excluded), Spotify happy path with auto-discovered marker, per-playlist exception captured into errors counter - sync_playlist: no_id, not_found, no_tracks, no-discovered-tracks skip, discovered-track happy path, unchanged-since-last-sync skip - playlist_pipeline: no_playlist clears running flag, no-refreshable clears running flag, exception clears running flag 3223 tests pass. web_server.py: 35,593 -> 34,850 lines (743 removed).	2 weeks ago
Broque Thomas	ea7d5c65bb	Extract automation handlers (1/N): infrastructure + 3 simple handlers Begins the lift of `web_server._register_automation_handlers` (1530 lines, 20 nested closures) into `core/automation/handlers/`. Each extracted handler is a top-level function that accepts `(config, deps)` instead of reaching for module-level globals -- makes them unit-testable in isolation. Infrastructure: - `core/automation/deps.py`: `AutomationDeps` (dependency-injection bundle of clients + callables) and `AutomationState` (mutable flags shared across handler invocations, with thread-safe accessors). - `core/automation/handlers/__init__.py` + `registration.py`: one-stop `register_all(deps)` that wires every extracted handler to the engine. First batch of handlers extracted: - `process_wishlist` -> `core/automation/handlers/process_wishlist.py` - `scan_watchlist` -> `core/automation/handlers/scan_watchlist.py` - `scan_library` -> `core/automation/handlers/scan_library.py` `web_server._register_automation_handlers` now builds the deps once and calls `register_all(deps)` for the extracted batch. Remaining 17 closures still live below; subsequent commits in this branch finish the lift. 14 boundary tests in `tests/automation/test_handlers_simple.py` pin every shape: success path, exception swallow contract, fresh-vs-stale state detection (scan_watchlist's id() trick), guard short-circuits, state cleanup on exceptions, AutomationState concurrent-safe accessors. All 101 automation tests pass; no regression.	2 weeks ago
Broque Thomas	b42cafa150	AcoustID + quarantine modal: three bug fixes (closes #607 , closes #608 ) Issue #607 (AfonsoG6) -- two AcoustID problems: 1. Live recordings false-quarantining as "Version mismatch: expected '... (Live at Venue)' (live) but file is '...' (original)" because MusicBrainz often stores the recording entity with a bare title -- the venue / live annotation lives on the release entity, not the recording. The audio fingerprint correctly identifies the live recording, but the title-text comparison flagged it as wrong. New pure helper `core/matching/version_mismatch.py:is_acceptable_version_mismatch` accepts the mismatch only when: - One-sided AND involves 'live': exactly one side is 'live' and the other is bare 'original'. Two-sided mismatches stay strict. - Fingerprint score >= 0.85 (stricter than the existing 0.80 minimum -- escape valve only fires when AcoustID is more confident than its own threshold). - Bare title similarity >= 0.70. - Artist similarity >= 0.60. Other version markers (instrumental, remix, acoustic, demo, etc) stay strict -- those have distinct fingerprints AND MB always annotates them in the recording title. The existing test_acoustid_version_mismatch.py suite passes unchanged. 2. Audio-mismatch failure message reported "identified as '' by '' (artist=100%)" when AcoustID returned multiple recordings -- prior code mixed `recordings[0]`'s strings (which can be empty) with `best_rec`'s scores. Now uses `matched_title` / `matched_artist` consistently in both the high-confidence-skip path and the final fail message. Issue #608 (AfonsoG6) -- quarantine modal: 3. Approve / Delete buttons silently no-op'd when the filename contained an apostrophe -- the unescaped quote broke the inline JS in the onclick handler. Now wraps the id via `escapeHtml(JSON.stringify(id))`, which round-trips quotes / backslashes / unicode / newlines safely through the HTML attribute to JS string boundary. 4. Bonus UX: quarantine entry expanded view now shows source uploader (username) and original soulseek filename when the sidecar carries that context -- helps trace which uploader the bad file came from. Backend exposes `source_username` + `source_filename` fields from `sidecar.context.original_search_result`. Degrades to '' on legacy thin sidecars. Tests: - 23 new boundary tests in tests/matching/test_version_mismatch.py pin every shape: equal versions trivial, one-sided live both directions, threshold floors (each just below default -> reject), two-sided strict, non-live one-sided strict (covers exact test_instrumental_returned_for_vocal_request_fails scenario), custom-threshold overrides. - 4 existing test_acoustid_version_mismatch.py tests pass unchanged. - 507 AcoustID / matching / imports tests pass.	2 weeks ago
Broque Thomas	b05ba5d498	Reorganize: optional embedded-tag mode (closes #592 ) Adds an opt-in alternative metadata source for reorganize. The existing API path (query Spotify / iTunes / Deezer / Discogs / Hydrabase for the canonical tracklist) stays the default and is unchanged. The new tag mode reads each file's embedded tags as the source of truth instead -- useful for well-enriched libraries where API drift can produce inconsistent renames, and avoids API calls entirely. - New pure helper `core/library/reorganize_tag_source.py` adapts the output of `read_embedded_tags` (the same mutagen path the audit- trail modal uses) to the `api_album` / `api_track` shapes that `_build_post_process_context` already consumes. Handles ID3-style "5/12" track + disc shapes, multi-value Artists tags, year normalization across 5 date formats, releasetype canonical tokens, multi-artist string splits across 9 separators. - `plan_album_reorganize` accepts `metadata_source: 'api' \| 'tags'` (default 'api') and `resolve_file_path_fn`. Tag mode branches into a new `_plan_from_tags` that reads each track's file and produces per-item `api_album` + `api_track` instead of a shared one. - `_run_post_process_for_track` accepts a per-item `api_album` override so each file's own album metadata flows through post- process (not a single shared dict). - `total_discs` in tag mode honors the `totaldiscs` tag and the trailing `/N` of an ID3 `discnumber = "1/2"`. Partial-album reorganize still routes into the correct `Disc N/` subfolder when the tag knows the total even if not all discs are present locally. - Bare `discnumber = "1"` no longer poisons `total_discs` -- it carries no total signal. - `reorganize_album` surfaces a tag-mode-specific error when no files are readable, instead of the API-mode "run enrichment first" message which would mislead in tag mode. - `QueueItem.metadata_source` field, `enqueue` / `enqueue_many` pass-through, runner injects `item.metadata_source` into `reorganize_album`. - `web_server.py` endpoints accept `mode` body param. Falls back to the `library.reorganize_metadata_source` config setting, then to 'api'. Strict allowlist (api / tags) -- anything else falls back. - Frontend: per-album modal + reorganize-all modal both grow a new "Metadata Mode" dropdown above the source picker. Tag mode hides the source picker (irrelevant). Choice persisted in localStorage. Both preview + execute fetches send `mode` in body. Tests: - 49 boundary tests on the pure helper pin every shape: ID3 "5/12", multi-artist split, year normalization, releasetype validation, total_discs precedence, defensive paths. - 6 planner-level integration tests pin the wiring: tag-mode with good tags, partial-disc with totaldiscs tag, file missing, some-match-some-fail, defensive resolve_file_path_fn=None, API-mode regression guard. - All 3171 tests pass; 52 existing reorganize tests unchanged.	2 weeks ago
Broque Thomas	2f284efa57	Retag now re-embeds LYRICS tag instead of leaving it empty Discord report (netti93). The download flow runs `enhance_file_metadata` (clears all tags) then `generate_lrc_file` (writes .lrc sidecar AND embeds USLT). The retag flow only ran the first half — `enhance_file_metadata` cleared USLT and there was no follow-up to restore it. Two coordinated fixes (no new setting per kettui scope discipline — user described it as "might even be an idea," consistency was the load-bearing ask). Fix 1 — retag calls generate_lrc_file after enhance `core/library/retag.py:execute_retag` now invokes `deps.generate_lrc_file` right after the `enhance_file_metadata` call, mirroring the download pipeline. New `generate_lrc_file` field on `RetagDeps`, defaults to None for backward compat with any test caller that builds RetagDeps without it. Web_server's `_build_retag_deps()` factory wires in the real `core.metadata.lyrics.generate_lrc_file`. Placement matters — runs BEFORE `safe_move_file` so the helper sees the audio file at its current path with its existing sidecar (which retag hasn't moved yet). After the embed, the audio file gets moved with USLT now present; the sidecar move step that follows is unaffected. Fix 2 — create_lrc_file re-embeds from existing sidecar `core/lyrics_client.py:create_lrc_file` used to early-return True when an .lrc / .txt sidecar already existed (skipping the LRClib fetch). For the retag case the sidecar is already there, so the shortcut hit and USLT was never re-written. Now the helper reads the existing sidecar and calls `_embed_lyrics` with its content before returning. Empty / unreadable sidecars short-circuit silently — defensive, no crash. Download flow unaffected because no sidecar exists at fetch time. 7 boundary tests pin: existing .lrc triggers re-embed, existing .txt triggers re-embed, empty sidecar skips embed, unreadable sidecar swallows error, no sidecar falls through to LRClib (download path regression guard), RetagDeps.generate_lrc_file field accepted, field optional for backward compat. Full suite: 3120 passed.	2 weeks ago
Broque Thomas	30f017d1f0	Stop writing TRCK as "6/0" when album total_tracks is unknown Discord report (netti93): downloaded album tracks were tagged with TRCK = "6/0" instead of "6/13" when source data was incomplete. The retag tool wrote correct "6/13" because core/tag_writer.py already handled the case. Trace: core/metadata/enrichment.py:105 formatted unconditionally as f"{track_number}/{total_tracks}" and many album-dict construction sites pass total_tracks: 0 (per types.py, 0 means "unknown" — not a real count). That 0 propagated straight to disk. Fix at the consumer boundary so every album-dict constructor stays unchanged. Lifted to pure helper core/metadata/track_number_format.py:format_track_number_tag that drops the /N suffix when total is 0 / None / negative — emits just "6" instead. Matches retag's behavior + ID3 spec convention (TRCK can be "N" or "N/M"). MP4 trkn tuple gets the same treatment via format_track_number_tuple returning (6, 0) per spec's "unknown total" marker. Wired into all three format-write sites in enrichment.py: ID3 (TRCK), Vorbis (tracknumber), MP4 (trkn). When source data has correct total_tracks (album downloads via the metadata-source pipeline, retag flow), behavior unchanged — still writes "6/13". 16 boundary tests pin every shape: known total / zero total / none total / none track / zero track / negative inputs / string coercion / unparseable strings / floats truncate. Full suite: 3113 passed.	2 weeks ago
Broque Thomas	9cc09118bf	AcoustID scanner: multi-candidate match + duration guard + multi-value retag Closes #587. Three coordinated fixes per codex's diagnosis. AcoustID verification gate left intact — these fixes target the upstream scanner false-positive surface plus a separate retag-path gap. Bug 1 — scanner used recordings[0] as authoritative `core/repair_jobs/acoustid_scanner.py:_scan_file` only checked the top fingerprint match's metadata. AcoustID often returns multiple recordings per fingerprint (sample collisions, multi-MB-record cases) and the wrong-credited recording can outrank the right- credited one. Foxxify case 2 (Nana / Nana): top match credited the wrong artist while a lower-ranked candidate matched the user's expected metadata exactly. Lifted the verifier's all-candidates check to a shared pure helper `core/matching/acoustid_candidates.py:find_matching_recording`. Both verifier and scanner can now ask "given these candidates, does ANY of them match expected (title, artist)?" with the same contract. Scanner suppresses the finding when any candidate matches. Bug 2 — no duration check guards against fingerprint hash collisions Foxxify case 3: 17-minute mashup edit fingerprinted to a 5-minute late-70s Japanese hiphop track (different songs, fingerprint hash collision on a sampled section). Scanner had no signal to detect this and would have recommended retagging the 17-min file as the 5-min track. `duration_mismatches_strongly` in the same helper module flags drifts beyond max(60s, 35%). Scanner now skips findings when the candidate's duration disagrees strongly with the file's expected duration. Loaded duration via the existing tracks SQL (added `t.duration` to the SELECT). Returns False when either side is unknown — no behavior change for older rows without duration data. Bug 3 — scanner retag bypassed multi-value ARTISTS tag setting `core/repair_worker.py:_fix_wrong_song` called `write_tags_to_file` with single-string artist updates. The writer only wrote TPE1 (single string) and never read the user's `metadata_enhancement.tags.write_multi_artist` config. Multi-value ARTISTS tags got stripped on every retag, contradicting the post-download enrichment pipeline's behavior. Per codex's pick (option B over routing through enhance_file_metadata), extended `write_tags_to_file` with an optional `artists_list` parameter. Each format-specific writer respects the config flag the same way enrichment.py does: - ID3: TPE1 stays as joined display string + TXXX:Artists multi-value - Vorbis/Opus/FLAC: `artist` display string + `artists` multi-value key - MP4: \xa9ART as list when on, single string when off Scanner retag derives the per-artist list by splitting AcoustID's credit through the existing `split_artist_credit` helper (same separators the matching layer already uses). Backward compatible: callers that don't pass `artists_list` get the exact same single-string write as before. No regression for the write_artist_image button or any other tag_writer caller. 15 tests on the candidate helper + duration guard. 13 tests on the tag_writer multi-value path (write/skip/single/ no-list cases for FLAC + the config-gate helper). 4 new scanner regression tests pinning lower-ranked candidate suppression, no-suppression when no candidate matches, duration mismatch skip, no-skip when duration matches. Existing scanner tests updated for the new 11-column SQL select (added duration column to fake schema + test row tuples). Full suite: 3097 passed. Ruff clean.	2 weeks ago
Broque Thomas	0aa18b0180	Cross-script artist aliases: include canonical name + non-strict fallback Closes #586. Follow-up to #442 — Cyrillic / kanji canonical names weren't bridging cross-script comparisons. Reporter case: "Dmitry Yablonsky" tracks quarantined as audio mismatch with file identified as "Русская филармония, Дмитрий Яблонский" (4% artist sim) even though the Cyrillic spelling is just the Russian transliteration. Codex diagnosed three layered bugs in the alias resolution chain. This fixes all three. Bug 1 — fetch_artist_aliases ignores canonical name + sort-name `core/musicbrainz_service.py:fetch_artist_aliases` only read `data['aliases']`. For artists where MB's canonical `name` IS the cross-script form (and the Latin spelling lives only in aliases — or vice versa), the missing direction never made it into the returned list. Fix: include both `data['name']` and `data['sort-name']` alongside the explicit alias entries (deduped, also pulls each alias entry's sort-name when present). Bug 2 — lookup_artist_aliases ran search in strict mode only Strict mode queries `artist:"..."` only and skips MB's alias and sortname indexes. Cross-script searches found nothing under strict because the user's Latin input never matches a Cyrillic canonical name in the artist index. Fix: lifted the search-and-score logic to a private helper `_search_and_score_artists(name, strict=)` and fall back to non-strict when strict returns empty OR all results fail the trust gate. Non-strict (bare query) hits all indexes. Bug 3 — trust gate weighted local similarity 70% Combined score = local_sim * 0.7 + mb_score/100 * 0.3. Cross-script pairs have local sim ~0 → combined ~0.30 → below the 0.85 threshold → cached as empty even when MB's own confidence was 100. Fix: added an MB-only escape — when MB score is >= 95 AND the result is unambiguous (top result's MB score leads the runner-up by >= 5), accept regardless of local similarity. The existing combined-score path stays intact for same-script matches (#442 Hiroyuki Sawano case still passes via that path). 12 new tests pin every layer: - fetch_artist_aliases canonical-name inclusion + dedup against alias entries + missing-canonical handling + exception path - strict-then-non-strict fallback (empty-strict + low-strict-score) - trust gate MB-only escape + low-confidence rejection + ambiguity rejection (two artists same MB score) + same-script regression - end-to-end reporter scenario with the real `artist_names_match` helper proving the bridge works for "Русская филармония, Дмитрий Яблонский" vs expected "Dmitry Yablonsky" Existing alias tests in `test_artist_alias_service.py` updated to reflect: canonical name now appears in `fetch_artist_aliases` output, lookup makes 2 search calls (strict + non-strict fallback) on first cache miss instead of 1. Full suite: 3065 passed.	2 weeks ago
Broque Thomas	e7ecaca3fd	Fix MTV Unplugged & live-album false-quarantine pipeline Closes #589. Tracks from MTV Unplugged / Live At / unplugged albums consistently failed AcoustID verification with "Version mismatch: expected (live) but file is (original)". Two upstream bugs fed into the false positive — the AcoustID gate itself was correctly catching the wrong file Tidal had selected. Codex diagnosed all three layers, this fixes the two upstream causes and leaves the verifier alone. Bug 1 — album-scoped library check false-misses owned albums `core/downloads/master.py:184` scored "Shy Away (MTV Unplugged Live)" (source title from playlist) vs "Shy Away" (local DB stored title) with raw string similarity. Massive length asymmetry → ~0.3 → below the 0.7 threshold → marked missing. Combined with the `allow_duplicates and batch_is_album` short-circuit that disables the global fallback for album downloads, the user's already-owned album re-triggered every track for download. Explains the screenshot showing "0 found / 7 missing" on an album the user manually placed. New pure helper `core/matching/album_context_title.py:strip_redundant_album_suffix` strips trailing parenthetical / bracket / dash suffixes whose tokens are fully subsumed by the album context — at least one version marker (live / unplugged / acoustic / session / concert / tour) overlapping with the album, and every other token is either a known marker, a year, a tolerated noise word, or a word from the album title. Album-context-implied "live" added when the album mentions unplugged / concert / tour / session. Wired into the album-confirmed scope ONLY (not global matching). Compares both raw and normalized source titles per album track and takes the max similarity, so the helper returning the input unchanged (when album doesn't imply version context) preserves the pre-fix behavior. Bug 2 — Tidal qualifier filter only ran on fallback searches `core/tidal_download_client.py:345` set `is_fallback = attempt_idx > 0` and only filtered when `is_fallback and required_qualifiers`. Primary search returned all results unfiltered, so a query for "Shy Away (MTV Unplugged Live)" could accept the studio cut if Tidal happened to rank it first. Now the qualifier filter applies to BOTH primary and fallback search attempts — log message updated to indicate which path triggered. Bug 3 — qualifier check ignored album.name The legacy `_track_name_contains_qualifiers` only inspected the track name. For concert / unplugged releases the live signal typically lives in the album title, not the track title. New `_track_matches_qualifiers` accepts a track object and inspects both `track.name` AND `track.album.name`. Legacy helper preserved to keep its existing test contract. AcoustID version-mismatch gate at core/acoustid_verification.py left intact — it correctly catches genuinely-wrong files that slip through upstream filters. The In My Feelings (Instrumental) test that pins this behavior continues to pass. 19 tests on the album-context helper covering MTV Unplugged variants, dash/parens/brackets suffix shapes, year tolerance, plural-form markers, the implied-live set, anti-regression cases (instrumental/remix on a studio album must NOT be stripped), empty/none defensive paths. 13 tests on the Tidal qualifier helper covering legacy track-name-only behavior preserved, qualifier in track name alone, qualifier in album name alone (the MTV Unplugged scenario), multi-qualifier requirements, no-qualifiers always passes, defensive against missing track.album, word-boundary avoiding substring false-matches, _extract_qualifiers picking up live + unplugged from the user's exact reporter query. Full suite: 3053 passed.	2 weeks ago
Broque Thomas	c9d4b02a02	Fix Deezer contributors tagging silently dropping for cache-polluted tracks Closes #588. Contributing-artist tagging worked for some tracks but silently dropped them for others — most reproducibly when the album had been fetched before the per-track post-process ran. Trace: get_track_details cache check used `track_position in cached` as the "full payload" sentinel. Both `/track/<id>` AND `/album/<id>/tracks` set track_position. Only `/track/<id>` sets the `contributors` array. When album-tracks data hit the cache first, get_track_details returned the partial record → _build_enhanced_track found no contributors → metadata-source contributors-upgrade silently fell back to single-artist. Reporter's case (Andrea Botez - Sacrifice): the album fetch logged "Retrieved 4 tracks for album 673558211" before the post-process, which cached all 4 tracks as partial records. The contributors- upgrade then hit the partial cache and the upgrade log line never fired because len(upgraded) was never > 1. Lifted cache-validity to a pure helper `_is_full_track_payload` that requires BOTH `track_position` AND `contributors` key presence. Empty list `[]` is valid — single-artist tracks fetched via `/track/<id>` carry it explicitly. Partial cache hits fall through to a fresh `/track/<id>` fetch, which writes the full payload back to cache. 11 boundary tests pin every shape: full payload, single-artist with empty contributors list, partial album-tracks shape, search-result shape, none/non-dict, and the cache-hit/cache-miss/api-failure paths on get_track_details (including the exact reporter-scenario regression). Full suite: 3021 passed.	2 weeks ago
Broque Thomas	083355ec8c	Persist Find & Add selections as permanent server-playlist match overrides Closes #585. When a Spotify source track had a versioned suffix not present in the local file ("Iron Man - 2012 - Remaster" vs "Iron Man"), the auto-matcher missed the pair. User could click Find & Add to pick the right local file — that worked, file got added to the Plex playlist — but the source track stayed in Missing while the added file appeared in Extra, because the matcher kept no record of the user-confirmed pairing. On the next sync the source track re-tried to download. Fix: every Find & Add selection now writes a (spotify_track_id → server_track_id) override into sync_match_cache at confidence=1.0. The matching algorithm runs an override pass BEFORE the existing exact and fuzzy passes, so any user-confirmed pair short-circuits straight to "matched" without going through title normalization. Covers every mismatch class — dash-suffix remasters, covers / karaoke, alt masters, cross-language titles, typo'd local files. - core/sync/match_overrides.py (new) — pure helpers resolve_match_overrides + record_manual_match. 18 boundary tests pin: cache hits, cache misses falling through to normal matching, stale-cache (server track removed) handled gracefully, str/int id coercion, partial cache hits, defensive against non-dict inputs and DB exceptions. - web_server.py — get_server_playlist_tracks runs the override pre-pass before exact/fuzzy matching. server_playlist_add_track accepts source_track_id + source_title + source_artist and persists the override after every successful add (Plex / Jellyfin / Navidrome). source_track_id added to source_tracks payload so the frontend has it. - webui/static/pages-extra.js — _serverSelectTrack sends source_track_id + source_title + source_artist when adding a track from a mirrored playlist context. - Sync match cache schema unchanged — already had UNIQUE (spotify_track_id, server_source) which fits the override semantics perfectly. Manual overrides distinguished from auto-discovered matches by confidence=1.0. Full suite: 3010 passed.	2 weeks ago
Broque Thomas	f4cff78f13	Quarantine management — list, approve, delete, recover Closes #584. Quarantined files used to sit in ss_quarantine/ with a thin sidecar — no UI, no recovery, no way to see what got dropped. This adds the management surface the user needs without going to the filesystem. UI: new "Quarantine" button on the downloads page header opens a modal with every quarantined file (filename, expected track/artist, reason, when, size). Three actions per row: - Approve (one-click): restores the file, re-runs the post-process pipeline with ONLY the failing check skipped, lands in the library with full tags + lyrics + scan - Recover (legacy fallback): moves to Staging for thin-sidecar entries that lack the embedded context Approve needs - Delete: permanent removal of file + sidecar Per-check bypass: context['_skip_quarantine_check'] = 'integrity' / 'acoustid' / 'bit_depth'. Skips ONLY the named check — other quality gates stay live. No blanket bypass-all flag. Sidecar expansion: move_to_quarantine now persists the full json-serializable context via serialize_quarantine_context (drops non-JSON-safe values, walks nested dicts/lists/sets, str-coerces unknown objects) plus the trigger name. Existing thin sidecars are detected and routed to Recover instead of Approve. Pure helpers in core/imports/quarantine.py: list_quarantine_entries / delete_quarantine_entry / approve_quarantine_entry / recover_to_staging / serialize_quarantine_context. 27 tests pin every shape: orphan files / orphan sidecars / corrupt sidecars / collision-safe filename restoration / full-context vs thin-sidecar dispatch / json round-trip safety. Four new endpoints in web_server.py — thin glue around the helpers: GET /api/quarantine/list, DELETE /api/quarantine/<id>, POST /api/quarantine/<id>/approve, POST /api/quarantine/<id>/recover. Download modal status differentiates "🛡️ Quarantined" from "❌ Failed" so recoverable files are visible at a glance — checked against the error_message text, no schema change needed. Pipeline changes are three minimal per-check conditionals at the existing quarantine sites in core/imports/pipeline.py. Each move_to_quarantine call now passes its trigger name so the sidecar records which check fired. Full suite: 2992 passed.	2 weeks ago
Broque Thomas	177bd85355	Configurable duration tolerance for downloaded-file integrity check Previously hardcoded at 3s (5s for tracks >10min) — files drifting past that got quarantined with no user override. Live recordings, alternate masterings, and some legitimate uploads routinely drift further. New setting `post_processing.duration_tolerance_seconds`. Default 0 means "use auto-scaled defaults" (unchanged behavior for users who don't touch it). Positive value overrides the per-track defaults. Capped at 60s — past that the check is effectively off. Logic lifted to pure helper `resolve_duration_tolerance` in file_integrity.py. Coerces every plausible input (None / empty / zero / negative / unparseable / above-cap / numeric string / float) to either a float override or None for auto. 12 tests pin every shape. Wired into `core/imports/pipeline.py` at the integrity-check call site — runs for ALL matched downloads (Soulseek / Tidal / Qobuz / HiFi / YouTube / Deezer-direct) since they all share that pipeline. Settings UI input under Settings → Metadata → Post-Processing.	2 weeks ago
Broque Thomas	0769fcd5cc	Fix Soulseek downloads losing collab artist tags Soulseek matched-download contexts populate `original_search_result` with `artist` (singular string) and no `artists` list — the full multi-artist array lives on `track_info` (the matched Spotify track object). `extract_source_metadata` only read `original_search.artists`, so the Soulseek path always fell through to the single-artist branch and TPE1 ended up with the primary artist only. Deezer-direct downloads were unaffected because their context populates `original_search.artists` as a proper list. Lifted artist resolution into a pure helper `core/metadata/artist_resolution.py:resolve_track_artists` that walks `original_search.artists` → `track_info.artists` → `artist_dict.name` fallback chain. Normalizes mixed list-item shapes (Spotify-style dicts, bare strings, anything else stringified) and drops empty entries. 13 new tests pin the resolution order, fallback chain, mixed-shape normalization, whitespace stripping, and empty/none handling. The existing `_artists_list` no-fall-through test in `test_multi_artist_tag_settings.py` was updated to reflect the new contract (always populated; multi-value write still gated on `len > 1`) plus a new regression test for the Soulseek shape. Composes with the existing Deezer per-track upgrade (still fires when single-artist + track_id available) and feat_in_title / artist_separator settings (still drive the joined ARTIST string downstream).	2 weeks ago
Broque Thomas	8a11a660af	Extract manual import route handlers Move the remaining manual import endpoint logic out of web_server.py and into core.imports.routes behind ImportRouteRuntime. The Flask endpoints now stay as thin compatibility wrappers for album/track search, album match/process, single-file import processing, and batched singles processing. Keep legacy test patch points intact by re-exporting build_album_import_match_payload from web_server and routing singles_process through an injected process_single_import_file callable. This preserves existing route-level monkeypatch behavior while keeping the extracted helper testable. Add focused helper coverage for Hydrabase enqueueing, search limit clamping, album match payload forwarding, album import side effects, single-file worker outcomes, malformed manual matches, and singles aggregation/injected-worker behavior. Verification: py_compile and git diff --check passed locally; bundled-Python smoke covered the extracted helpers. Claude reran the project tests and reported all tests passing.	2 weeks ago
Broque Thomas	d703d33178	Extract import staging route helpers Move import staging files/groups/hints/suggestions controller logic out of web_server.py and into core.imports.routes behind an ImportRouteRuntime dependency object. Keep the existing Flask routes as thin compatibility wrappers so the UI endpoint surface stays unchanged. Add focused tests for staging file filtering, album grouping, hint generation, cached suggestions, empty missing staging paths, and error payloads from failed path/metadata reads. Verification: py_compile passed for web_server.py, core/imports/routes.py, and tests/imports/test_import_routes.py. A bundled-Python smoke pass covered the extracted helper behavior; pytest was not available in this Windows shell because the bundled Python lacks pytest and the repo venv is WSL/Linux-only here.	2 weeks ago
Antti Kettunen	32bf52cc18	Extract WebUI asset helpers - move Vite manifest handling and SPA route rules into core/webui - keep web_server.py focused on Flask route wiring - add tests for asset rendering and manifest reload behavior - keep image URL normalization coverage alongside the metadata helpers	2 weeks ago
Broque Thomas	fdda64963f	Drop platform-biased trailing-backslash test for derive_artist_folder POSIX os.path.dirname doesn't treat '\' as separator, so the assertion 'Drake' in result fails on Linux CI even though the function's rstrip removes the trailing backslash correctly. The forward-slash test already covers the trim contract.	2 weeks ago
Broque Thomas	89246a7304	Write artist.jpg to artist folder so Navidrome shows real photos Closes #572 (rhwc). Navidrome has no API for setting an artist image — it reads `artist.jpg` (or `folder.jpg`) from the artist folder during library scans. SoulSync's `update_artist_poster` for Navidrome was a no-op, so users only ever saw album-art-derived thumbnails as the artist photo. - new "Write Artist Image" button on artist detail page - POST /api/artist/<id>/write-image-to-disk derives the artist folder from any track's resolved file_path (reuses _resolve_library_file_path so docker mount translation + library.music_paths probes from #558 apply), fetches the photo from the configured metadata source priority chain, downloads with content-type validation, writes atomically via `<filename>.tmp + os.replace` - when active server is Navidrome, triggers a library scan immediately so the file is picked up - respects existing artist.jpg (frontend prompts before overwriting) so user-supplied photos aren't clobbered - works for plex / jellyfin too as a fallback layer — both servers also read artist.jpg from disk 26 tests pin the pure helpers in core/library/artist_image.py: folder derivation (trailing sep / empty / non-string), URL picking (missing attr / whitespace / non-string), download (non-image content-type / 404 / timeout / empty body), atomic write (replace / temp-cleanup-on-failure / overwrite guard / missing folder).	2 weeks ago
Broque Thomas	6ce185491d	Add per-download Audit Trail modal to Library History - new "Audit" button on each download row in the library history modal opens a second modal visualizing the download lifecycle as an interactive horizontal stepper (request → source → match → verify → process → place) with click-to-expand detail cards - hero header with album art + track title + meta line + status pills (source / quality / acoustid result) - three tabs: Lifecycle / Tags / Lyrics - Tags tab reads the audio file live via mutagen at audit-open time via new GET /api/library/history/<id>/file-tags endpoint; file is the single source of truth so background enrichment writes (audiodb / lastfm / genius / replaygain / lyrics fetch) show up too. flat key/value rows stacked vertically (label-above- value) so long MBIDs / URLs / joined genre lists wrap cleanly. source IDs grouped per-service into 2-col sub-card grid. - Lyrics tab renders the full transcript with dimmed timecodes. - post-processing step infers observable changes from source-vs- final state (format conversion, file rename via tag template, folder template). - "Download History" button also added to the Downloads page batch panel header so it's reachable outside the dashboard. - mobile responsive: tabs + stepper scroll horizontally, modal goes full-screen, hero stacks below 480px. 19 helper tests pin the mutagen reader: id3 (TIT2/TPE1/TALB + TXXX + USLT + APIC), vorbis (FLAC dict + _id/_url passthrough), file metadata (format / bitrate / duration), defensive paths (empty / missing file / mutagen returns None / mutagen raises), stringify edge cases (list / tuple / int / frame-with-text / whitespace).	2 weeks ago
Broque Thomas	46206b3240	Pin type='track' / type='artist' collision case for album-type normalizer	2 weeks ago
Broque Thomas	5eae24b8bb	Fix $albumtype defaulting to album for non-Spotify sources - legacy duck-typed builder only checked the `album_type` key; deezer uses `record_type`, tidal uses `type` (uppercase), some flattened musicbrainz shapes use `primary-type` — all defaulted to album, so EPs and singles ended up filed under Album/ in user templates that reference $albumtype - widen lookup to album_type / record_type / type / primary-type and route through new pure `_normalize_album_type` helper that case-folds + validates against the canonical token set (album / single / ep / compilation), unknown → album - typed-converter path (spotify / deezer / itunes / discogs / mb / hydrabase / qobuz) unchanged — those were already correct Discord report (CAL).	2 weeks ago
Broque Thomas	b9feed1a67	Add min delay between slskd searches (Bell Canada anti-abuse fix) - new soulseek.search_min_delay_seconds knob forces a gap between consecutive searches; smooths the burst pattern that trips ISP anti-abuse (Reddit report: Bell Canada cuts the WAN after rapid peer-connection spikes) even when the existing 35/220 sliding-window cap isn't hit - throttle math lifted to a pure compute_search_wait_seconds helper so the gate logic is testable independent of asyncio.sleep + the singleton client - new field on settings → connections → soulseek; default 0 = disabled so existing users see no change 15 helper-boundary tests pin defaults / no-throttle, sliding-window cap (legacy), min-delay (the new burst-smoother), max-of-both gates, and defensive paths.	2 weeks ago
Broque Thomas	6233860d66	Fix Copy Debug Info music_source + surface missing services - music_source / spotify_connected / spotify_rate_limited were reading a non-existent 'spotify' key on _status_cache and silently falling through to the missing-value default (always 'unknown' / False). Routed through the canonical accessors get_primary_source + get_spotify_status now. - added hydrabase_connected, youtube_available, hifi_instance_count, and always_available_metadata_sources so the debug dump reflects the full service surface - removed a local re-import of get_spotify_status that was making python 3.12 treat the name as function-scoped, breaking the new lambda above it (NameError on free variable) — module-level import already exists 11 endpoint-level tests pin music_source / spotify_* / hydrabase_* / youtube_available / always_available_metadata_sources / hifi_instance_count and the defensive fall-through paths when each lookup raises.	2 weeks ago
Broque Thomas	4892baf8d4	Skip already-owned tracks during download discography - new track_already_owned helper wraps db.check_track_exists at the same confidence threshold the discography backfill repair job uses (0.7) — name+artist+album, format-agnostic so blasphemy-mode libraries (flac → mp3 + delete original) match correctly - endpoint runs the check after the artist + content-type filters and before add_to_wishlist, so a second discography click on the same artist no longer re-queues every track that already downloaded - per-album response carries a new tracks_skipped_owned counter alongside the existing artist/content/wishlist skip categories Discord report (Skowl).	2 weeks ago
Broque Thomas	d4ad5bf57f	Filter cross-artist + content-type tracks during download discography - drop tracks where the requested artist isn't named in track.artists (keeps features, drops compilation / appears_on contamination) - honor watchlist.global_include_live/remixes/acoustic/instrumentals the same way the discography backfill repair job already does - surface per-album skip counts in the ndjson stream (artist mismatch + content filter) so the ui can show what was filtered Closes #559.	2 weeks ago
Broque Thomas	56ae10693b	Album Completeness: surface diagnostic when resolver can't find album folder GitHub issue #558: clicking Auto-Fill / Fix Selected on the Album Completeness findings page returned a flat "Could not determine album folder from existing tracks" error with no diagnostic. Reporter is on Navidrome on Docker — the path resolver in `core/library/path_resolver.py` couldn't find any of the album's tracks on disk because Navidrome's Subsonic API doesn't expose filesystem library paths the way Plex's API does (probed in #476). Default settings → `library.music_paths` empty → no base directories to probe → silent None. User had no signal about what to configure. Not a regression of #476 — that fix targeted Plex auto-discovery and worked correctly for it. Navidrome was never covered because the protocol gives the resolver nothing to probe. Fix scoped to the diagnostic surface, not auto-magic discovery: - Added `resolve_library_file_path_with_diagnostic` returning `(resolved, ResolveAttempt)`. ResolveAttempt records what the resolver tried — `raw_path_existed`, `base_dirs_tried`, `had_config_manager`, `had_plex_client`. Pure data, no rendering opinions. - Legacy `resolve_library_file_path` becomes a thin wrapper that drops the attempt; every existing call site is unchanged. - `RepairWorker._fix_incomplete_album` now uses the diagnostic helper and renders a multi-part error via `_build_unresolvable_album_folder_error`: names the active media server, shows one sample DB-recorded path, lists every base directory the resolver actually probed, and points the user at Settings → Library → Music Paths as the actionable fix. - Distinguishes empty-base-dirs vs tried-and-failed cases so the user knows whether to add a mount or fix the existing one. - No auto-probing of common Docker conventions (`/music`, `/media`, etc). Speculative — could resolve to wrong dirs on the suffix-walk if a conventional path happens to contain a partial collision. User stays in control. 12 new tests: - 7 in `tests/library/test_path_resolver.py`: tuple-shape contract, raw-path-existed short-circuit, base-dirs listed even on walk failure, had-flags reflect caller inputs, no-base-dirs returns None with empty attempt, legacy `resolve_library_file_path` delegates correctly across happy / suffix-walk / failure paths. - 8 in `tests/test_repair_worker_unresolvable_folder_error.py`: active server name in error, sample DB path verbatim, base dirs listed, empty-base-dirs phrased differently, Settings hint always present, defensive against None attempt / missing sample / missing config_manager. Full pytest sweep: 2774 passed.	2 weeks ago
Broque Thomas	698ecc99f0	Import history: Clear History button now sweeps stuck 'processing' rows Reported: Clear History button on the Import page left zombie rows behind. Every survivor showed "⧗ Processing" status from 2-9 days ago. Trace: `_record_in_progress` inserts a `status='processing'` row up-front so the UI can render the in-flight import while it runs; `_finalize_result` updates it to `completed`/`failed` when the import finishes. When the worker is killed mid-import (server restart, crash), the row never gets finalized — stays at `processing` forever. The clear-history endpoint's SQL `DELETE ... WHERE status IN (...)` listed every terminal status but omitted `processing`, so zombies survived every click. Fix: add `processing` to the delete list, but guard against nuking genuinely-live imports by intersecting against the worker's `_snapshot_active()` map — any folder hash currently registered in `_active_imports` is excluded from the delete via an `AND folder_hash NOT IN (...)` clause. `pending_review` deliberately left out so user still has to approve/reject those explicitly. One endpoint touched (`/api/auto-import/clear-completed` in web_server.py). No worker changes — guard reuses the existing `_snapshot_active()` method that the UI poller already calls. 5 new tests in `tests/imports/test_auto_import_clear_completed_endpoint.py`: - Zombie `processing` rows swept, live `processing` row preserved (folder_hash currently in `_active_imports` survives) - Response count matches actual delete count - Empty active-set branch (unparameterized DELETE) — pinned because an empty SQL `IN ()` would be a syntax error - Worker-unavailable returns 500 (pre-existing guard not regressed) - `pending_review` rows always survive — never auto-swept Full pytest sweep: 2758 passed (one pre-existing flaky timing test on `test_import_singles_parallel.py` failed under full-suite CPU load, passes in isolation in 2.95s — unrelated to this change).	2 weeks ago
Broque Thomas	3af2d34cee	Auto-import: fall through to other metadata sources when primary returns no match Discord report: 16 Bandcamp indie albums sat in staging because auto-import couldn't identify them, but the manual search bar at the bottom of the Import Music tab found the same albums fine. Trace: `_search_metadata_source` only queried `get_primary_source()` — single source, no fallback. Meanwhile `search_import_albums` (manual search bar) already iterated `get_source_priority(get_primary_source())` and broke on the first source with results. Asymmetric behavior, same album: manual worked, auto-import didn't. Fix: lift `_search_metadata_source` to use the same source-chain pattern. Try primary first; if it returns nothing OR scores below the 0.4 threshold, fall through to the next source in priority order. First source producing a strong-enough match wins. Result dict carries the `source` that actually matched (not the primary name) so downstream `_match_tracks` calls the right client. Defensive per-source try/except so a rate-limited or auth-failed source doesn't abort the chain. Unconfigured sources (client=None) silently skipped. Cin-shape lift: scoring math extracted to pure `_score_album_search_result` helper so the weight tweaks (album 50% / artist 20% / track-count 30%) are pinned at the function boundary, independent of the orchestrator (per-source iteration, exception containment, threshold check). Weight constants exposed at module level (`_ALBUM_NAME_WEIGHT`, `_ARTIST_NAME_WEIGHT`, `_TRACK_COUNT_WEIGHT`) — greppable, bumpable in one place. Pre-extraction these were magic numbers inline. 27 new tests: - 9 integration tests in `test_auto_import_multi_source_fallback.py`: primary-success path unchanged (no fallback fires, only primary client called), primary-empty falls through, primary-weak-score falls through, first fallback success stops the chain (no wasted API calls on remaining sources), all-sources-fail returns None, per-source exception contained, unconfigured-source skipped, result `source` field reflects winning source, `identification_confidence` from winning source. - 18 helper tests in `test_album_search_scoring.py`: weights sum to 1.0, album weight dominant (invariant pin), perfect-match returns 1.0, per-component contribution (album / artist / track-count), Bandcamp vs streaming track-count mismatch (7-files vs 4-tracks case still scores ~0.87 above threshold), zero-track-count and zero-file guards, huge-mismatch non-negative guard, list-of-strings artist shape, missing `.name` / `.artists` / `None` total_tracks edge cases. Backwards compatible: single-source users see no change — chain just has one entry. Existing test `test_search_metadata_source_extracts_artist_id_from_dict_artist` needed one extra patch line for `get_source_priority`. Full pytest sweep: 2754 passed.	2 weeks ago
Broque Thomas	d5de724f9b	Multi-artist Deezer upgrade + double-append guard hardening Two follow-ups to the multi-artist tag settings PR: 1. Deezer contributors upgrade — closes the "known limitation" flagged in the prior commit. Deezer's `/search` endpoint only returns the primary artist for each track; the full contributors array (feat., remix collaborators, producers credited as artists) lives on `/track/<id>` and gets parsed by `_build_enhanced_track`. Without the upgrade Deezer-sourced tracks never got multi-artist tags even with the right settings on. Fix in `core/metadata/source.py`: when source==deezer AND the search response had a single artist AND a track_id is available, fetch full track details via `get_deezer_client().get_track_details` and replace `all_artists` with the upgraded list. - One extra API call per affected Deezer track - Skipped when search already returned multiple (no-op fast path) - Skipped for non-Deezer sources (Spotify/Tidal/iTunes search responses already include all artists) - Skipped when no track_id is available - Defensive try/except: on /track/<id> failure (network error, deezer client unavailable), fall through to the search-result list — never lose the data we already had 2. Double-append guard hardened with a word-boundary regex. Prior commit checked for `"feat." not in title.lower() and "(ft." not in title.lower()` — too narrow. Source platforms produce wildly different feat-marker conventions: "(feat. X)", "(Feat X)", "(FEAT X)", "(Featuring X)", "[feat. X]", "ft. X" (no parens), "FT. X", etc. Any of these as the SOURCE title would cause a double-append: `"Track (Feat X) (feat. Y)"`. Replaced with `re.search(r'\b(?:feat\|feat\.\|featuring\|ft\|ft\.)\b', title, IGNORECASE)`. Word-boundary regex catches every common variant. Substring matches like "Aftermath" containing `ft` correctly fall through to the append path (pinned by a regression test). 16 new tests (29 total in the file): - 9 parametrized variants of the double-append guard - 1 substring guard ("Aftermath") - 6 Deezer upgrade scenarios (fires when expected, doesn't fire for non-Deezer / multi-artist search / no track_id, defensive fall-through on failure, no false-positive when /track/<id> confirms single artist) Full pytest 2727 passed.	2 weeks ago
Broque Thomas	c11a5b7eab	Multi-artist tag settings: implement artist_separator + feat_in_title + populate _artists_list Three settings on Settings → Metadata → Tags were partially or completely unimplemented. Reporter (Netti93) traced each one. (1) `write_multi_artist` only "worked" because of a never-populated `_artists_list` field. `core/metadata/source.py` built `metadata["artist"]` as a hardcoded ", "-joined string but never assigned `metadata["_artists_list"]`. `core/metadata/enrichment.py` line 107 reads that field and gates the multi-value tag write on `len(_artists_list) > 1` — always saw an empty list, silently no-op'd the write. (2) `artist_separator` (default ", ") was referenced in the UI + settings.js save path but ZERO Python code read the value. Every multi-artist track ended up with hardcoded ", " regardless of what the user picked. (3) `feat_in_title` (when true: pull featured artists into the title as " (feat. X, Y)" and leave only primary in the ARTIST tag — Picard convention) had no implementation at all. Fix in source.py: * Populate `_artists_list` from the search response's artists array * Read `feat_in_title` and `artist_separator` configs * When `feat_in_title=True` and >1 artist: ARTIST = primary only, append "(feat. X, Y)" to title with double-append guard * Else: ARTIST = artists joined with `artist_separator` * Single-artist case unaffected by either setting Double-append guard uses a word-boundary regex catching all common "feat" variants source platforms produce — `feat`, `feat.`, `featuring`, `ft`, `ft.` — case-insensitive. Substring matches (e.g. "Aftermath" containing "ft") correctly fall through to the append path. Fix in enrichment.py ID3 branch: * TPE1 stays as the display string (with separator or primary-only per the user's settings) * Multi-value list goes to a separate `TXXX:Artists` frame (Picard convention) when `write_multi_artist` is on * Pre-fix the ID3 path wrote TPE1 twice — single-string then list — and the second `add` overwrote the first, clobbering both the configured separator AND the feat_in_title semantics. Vorbis path was already correct (separate "artist" + "artists" keys). Known limitation (flagged in WHATS_NEW): Deezer's `/search` endpoint only returns the primary artist. The full contributors array lives on `/track/<id>`. Enrichment uses search-result data so Deezer- sourced tracks may still get only the primary artist until a follow- up commit wires the per-track contributors fetch into the enrichment flow. Spotify, Tidal, and iTunes search responses include all artists so they work now. 23 new tests in `tests/metadata/test_multi_artist_tag_settings.py`: * `_artists_list` populated for multi/single/no-artist cases * `artist_separator` drives ARTIST string (default ", " + custom ";" + custom "; " + " & ") * Single-artist case unaffected by either setting * `feat_in_title=True` pulls featured to title, leaves primary in ARTIST * `feat_in_title` no-op for single artist * Double-append guard recognizes 9 source-title variants ("(feat. X)", "(Feat. X)", "(FEAT X)", "(feat X)", "(Featuring X)", "[feat. X]", "ft. X", "(ft X)", "FT. X") * Substring guard test pins "Aftermath" doesn't false-positive * Combined-settings precedence: feat_in_title wins ARTIST string but `_artists_list` carries everyone for multi-value tag Full pytest 2711 passed.	2 weeks ago
Broque Thomas	fc573a5f19	AudioDB worker: stop infinite loop on direct-ID lookup failure (#553 ) Track enrichment was stuck in a constant retry loop. Logs showed nothing but `Read timed out. (read timeout=10)` from `lookup_track_by_id` repeating against the same track ID. AudioDB itself was being hammered nonstop with no progress. Cause: when an entity already has `audiodb_id` populated (from a manual match or earlier scan) but `audiodb_match_status` is still NULL — an inconsistent state some import paths can leave behind — the worker tries a direct ID lookup. If that lookup fails (returns None on timeout, which AudioDB's `track.php` endpoint hits frequently because it's slow), the prior code logged "preserving manual match" and returned WITHOUT marking status. Row stayed NULL → queue's NULL-status filter picked it up next tick → tried direct lookup → timed out → returned → infinite loop. The "preserve manual match" intent was correct: don't fall through to the name-search path because that could overwrite a manually-set `audiodb_id` with a wrong guess. Bug was the missing `_mark_status` call before the early return. Fix: * `_process_item` direct-lookup-failure branch now calls `_mark_status(item_type, item_id, 'error')` before returning. The existing `audiodb_id` is preserved (column not touched). Queue's NULL-status filter no longer re-picks the row. * `_get_next_item` retry-cutoff queue priorities (4/5/6) extended from `audiodb_match_status = 'not_found'` to `audiodb_match_status IN ('not_found', 'error')`. Same `retry_days` window. Transient AudioDB outages still recover automatically; permanently-broken IDs eventually get re-attempted once a month rather than staying errored forever. 5 new tests in `tests/test_audiodb_worker_stuck_track.py` use a real SQLite DB (not mocks) so the SQL queries are actually exercised: - lookup-returns-None marks status='error' (no infinite loop) - lookup-raises-exception marks status='error' (defensive) - lookup-success preserves the existing match-success path - error-status row past retry-cutoff gets picked up again - error-status row within cutoff stays skipped (loop prevention works) Only triggers for entities in the inconsistent `audiodb_id` set + `match_status` NULL state. Happy path and already-matched / already-not-found rows unchanged. Full pytest 2698 passed. Closes #553.	2 weeks ago

1 2 3 4 5 ...

362 Commits (ff27effdae3b2b25f650cd9920cbb7604ff3f6c7)