Commit graph

484 commits

Author SHA1 Message Date
Broque Thomas
9b086c5a65 Add owned_by column for Auto-Sync schedule ownership
The Auto-Sync schedule board was detecting its own automations by
checking `group_name === 'Playlist Auto-Sync' || name.startsWith('Auto-Sync:')`.
That's fragile — renaming the row from the Automations page silently
hands ownership back to the read-only Automation Pipelines tab and the
board stops managing it.

This commit replaces the string convention with an explicit
`automations.owned_by` TEXT column:

- Migration `_add_automation_owned_by_column` adds the column and
  backfills `'auto_sync'` for existing rows that match the legacy
  `group_name`/`name`-prefix pattern, so users running the migration
  don't lose their schedules.
- `database.create_automation` and `database.update_automation` accept
  `owned_by` (the latter via its `allowed` kwarg set).
- `core/automation/api.py` forwards `owned_by` on both POST and PUT.
  Missing field is left as None, preserving today's behavior for every
  caller that doesn't opt in.
- The Auto-Sync schedule board posts `owned_by: 'auto_sync'` and the
  detection helper now prefers that signal, falling back to the legacy
  name/group convention so any hand-rolled rows still show up.

Tests: three new cases in `tests/automation/test_automation_api.py`
covering create-with-owned-by, create-without (defaults to None), and
update set/clear. The fake DB grew the matching kwarg.
2026-05-24 23:40:22 -07:00
Broque Thomas
feb6778af4 Address Cin review: extract helpers, indexed pool fetch, tidy nits
Three changes folded into one perf+cleanup pass:

1. Indexed fast path for the per-artist pool fetch. The previous
   `search_tracks(artist=name)` call hit
   `unidecode_lower(artists.name) LIKE ?`, a function-in-WHERE that
   can't use `idx_artists_name`. New `MusicDatabase.get_artist_tracks_indexed`
   does a two-step lookup: exact-name match (indexed) plus a
   case-insensitive fallback, then `tracks WHERE artist_id IN (...)`
   via `idx_tracks_artist_id`. Drops per-artist fetch from seconds to
   milliseconds for the common case. The sync helper falls back to
   the old LIKE-based `search_tracks` only when the indexed lookup
   finds nothing, preserving diacritic recall and `tracks.track_artist`
   feature-artist matches with zero regression.

2. Public text-normalization helper. Lifted the body of
   `MusicDatabase._normalize_for_comparison` into
   `core/text/normalize.py:normalize_for_comparison` so callers outside
   the database layer (matching engine, sync pool, future import-side
   comparisons) don't reach across the module boundary into a
   leading-underscore "private" method. The DB method now delegates,
   so existing internal call sites stay untouched. Sync's lazy pool
   now imports the public helper.

3. Artist-name walker extracted. `_artist_name` at module level in
   `services/sync_service.py` replaces two near-identical inline
   str-or-dict-or-fallback walkers (one in `sync_playlist`, one in
   `_find_track_in_media_server`). Returns `''` for None instead of
   the literal string `'None'`.

Plus three small tidies from the same review:

- `_POOL_FETCH_LIMIT = 10000` constant in place of the literal at the
  pool-fetch call site.
- Trimmed the verbose docstring + comment block on the pool helper.
- Set-intersection predicate for the trigger-shape reset in
  `core/automation/api.py` instead of a two-line `or` chain.

Also removed the duplicate `_get_active_media_client()` call at
sync_service.py:212/214 — pre-existing wart that was sitting in the
same block I was editing.

Tests: 21 new tests across `tests/database/`, `tests/sync/`, and
`tests/text/`, plus updates to the existing pool tests to cover the
new fast/fallback split. Full suite stays green (3953 passing).
2026-05-24 23:33:55 -07:00
Broque Thomas
687bb0ca2c Add tests for next_run reset and lazy candidate pool
`tests/automation/test_automation_api.py` gains three update_automation
tests covering the schedule-shape reset:
- trigger_config change blanks next_run
- trigger_type change blanks next_run
- non-trigger field (name) leaves next_run alone

`tests/sync/test_sync_candidate_pool.py` is new — nine tests for the
lazy artist track pool in PlaylistSyncService:
- candidate_pool=None disables pooling and skips the DB call
- first lookup for an artist fetches and caches
- second lookup for the same artist reuses the cache (zero DB calls)
- empty result still cached so the next call short-circuits without SQL
- defensive None return coerced to []
- search_tracks exception returns None and does NOT poison the cache
- pool key is normalized so casing variants share a single fetch
- different artists get separate pool entries
- server_source plumbing survives the trip to search_tracks

All assertions go through fakes / MagicMock — no real DB, no
web_server.py import, no AST-parsing.
2026-05-24 22:58:48 -07:00
Broque Thomas
f83c671570 Add direct mirrored playlist pipeline runs
Expose playlist-native run and status endpoints that reuse the shared mirrored playlist pipeline engine while routing progress into playlist UI state.

Add a Run Pipeline action to mirrored playlist cards and modals with live status polling, and make the shared pipeline lock atomic for manual and scheduled callers.
2026-05-24 19:54:04 -07:00
Broque Thomas
547e499121 Expose mirrored playlist source-ref health
Return normalized source_ref metadata from mirrored playlist APIs so the UI no longer has to infer editable refresh links from description fields. Accept Spotify embed URLs during source-ref repair and add coverage for source-ref health reporting.
2026-05-24 19:32:39 -07:00
Broque Thomas
73bd2db547 Harden playlist pipeline source refresh
Centralize mirrored playlist source reference normalization so edited links and IDs are stored consistently. Preserve URL-backed refresh refs, surface missing-source refresh failures, count background sync failures in pipeline summaries, and retry guarded automation skips after a short delay instead of losing a scheduled run. Add focused coverage for source refs, mirrored playlist source updates, refresh failures, and guarded retry behavior.
2026-05-24 19:31:00 -07:00
Broque Thomas
a7ca7ddfad Harden album bundle fallback flow
Delay torrent and usenet album-bundle dispatch until missing-track analysis confirms there is work to do, matching the Soulseek album flow and avoiding release downloads for already-owned albums.

Clear private album-bundle staging state when a release-level source intentionally falls back to per-track mode so workers can use the normal staging/search path instead of an empty private bundle directory.

Verified by user: focused downloads master tests passed, 2 passed.
2026-05-24 16:15:36 -07:00
BoulderBadgeDad
a1222d5a8f
Merge pull request #686 from kettui/feat/react-migration-import
feat(webui): migrate import page to React
2026-05-24 14:16:05 -07:00
Broque Thomas
0bea332aed Preserve album bundle track numbers
Keep album-bundle staging from replacing known per-track album numbers with the filename parser's default when staged files do not expose a real track number. Carry staging tag numbers through the cache, fall back to task metadata for private release staging, and cap hybrid album batches to one worker when Soulseek is first in the source order.
2026-05-24 13:59:44 -07:00
Antti Kettunen
caa6534ee8
feat(import): show MusicBrainz variants
- pass release metadata through album search normalization
- surface release format, country, label, and disambiguation in React import cards
- add coverage for search normalization and import route rendering
2026-05-24 21:29:22 +03:00
Antti Kettunen
56f642aadd
test(import): remove stale frontend pytest
- delete the source-text guard for the old album lookup cache pattern\n- keep the import-page source-routing contract covered by Vitest route tests\n- avoid duplicating frontend behavior checks across pytest and the webui test suite
2026-05-24 21:17:21 +03:00
Broque Thomas
9a0e3b4011 Persist completed downloads in downloads view
Include a capped recent tail of database-backed download history in the unified Downloads page so completed Deezer and other streaming downloads remain visible after runtime tasks are cleaned up or the container restarts. Use persistent download history for the dashboard finished count, keep live tasks authoritative for active rows, avoid showing the local clear-completed action for persisted history rows, and cover history hydration/deduping/capping in status tests.
2026-05-24 10:02:00 -07:00
Broque Thomas
4ca3f70bf3 Show MusicBrainz release variants in import
Expand matched MusicBrainz release groups into concrete releases for specific album searches so import users can choose the correct edition by track count, format, country, and disambiguation. Preserve distinct MusicBrainz release IDs instead of deduping same-title variants, carry release metadata through import matching, and surface those details on album result cards. Add coverage for variant preservation and release-group expansion.
2026-05-24 09:33:19 -07:00
Broque Thomas
7bee424686 Escape dash-leading YouTube search queries
Fix manual YouTube searches for video IDs that begin with a dash by escaping leading '-' before building yt-dlp ytsearch expressions. This preserves normal search terms and already escaped user input while preventing yt-dlp from treating the ID as search syntax.

Add regression coverage for both YouTube download search and video search paths. Fixes #684.
2026-05-24 08:54:47 -07:00
Broque Thomas
f68afe80c8 Update #524 lookup pattern test for consolidated renderer (#681)
The #681 commit (eba7f61e) collapsed the inline duplicate card-renderer
inside importPageSearchAlbum into a single _renderSuggestionCard call.
The structural test for the issue #524 lookup-cache pattern was still
counting inline cache writes and expecting >=2, which started failing
in CI now that the search-results renderer routes through the shared
helper.

Rewritten to assert the actual invariant of the consolidated design:

* _renderSuggestionCard contains exactly one _albumLookup write
* No other inline write exists (a second write means a caller is
  re-implementing the renderer instead of calling the helper — the
  exact duplication the #524 fix consolidated away)

Same regression guard, matches the new architecture.
2026-05-24 01:46:36 -07:00
Broque Thomas
9769d8be19 Fetch all Qobuz favorite tracks for discovery
Remove the implicit 500-track cap from Qobuz Favorite Tracks so the Sync page discovers the same number of tracks shown on the playlist card. Keep an explicit limit parameter for callers that want a capped fetch.

Add tests covering the default full-pagination behavior and explicit limit handling.
2026-05-24 01:17:38 -07:00
Broque Thomas
a34eae1445 Add Qobuz playlist sync to Sync page (#677)
Qobuz joins Tidal and Deezer as a first-class playlist sync source.
New Qobuz tab on the Sync page lists user playlists + a virtual
Favorite Tracks entry, and clicks route through the same discovery →
sync → download pipeline the other services already use.

Backend:
* core/qobuz_client.py — new get_user_playlists, get_playlist,
  get_user_favorite_tracks, get_user_favorite_tracks_count. Returns
  normalized dicts (matches Deezer client shape, not Tidal's
  dataclasses) so the discovery worker can iterate directly without
  duck-typing. Virtual `qobuz-favorites` ID dispatches to favorites
  fetcher inside get_playlist — same trick Tidal uses with
  COLLECTION_PLAYLIST_ID. Both list endpoints paginate against
  Qobuz's 500-cap limit.
* core/discovery/qobuz.py — new worker module. Mirrors
  core/discovery/deezer.py: pause enrichment, iterate tracks,
  hit discovery cache, fall back to _search_spotify_for_tidal_track,
  build wing-it stub on miss, sync results to mirrored playlist.
* web_server.py — adds /api/qobuz/playlists, /playlist/<id>,
  /discovery/start/<id>, /discovery/status/<id>, /discovery/update_match,
  /playlists/states, /state/<id>, /reset/<id>, /delete/<id>,
  /update_phase/<id>, /sync/start/<id>, /sync/status/<id>,
  /sync/cancel/<id>. One-for-one with the Tidal + Deezer endpoint
  sets. Qobuz discovery executor registered for clean shutdown.

Frontend:
* webui/static/sync-services.js — full handler set (loadQobuzPlaylists,
  createQobuzCard, openQobuzDiscoveryModal, startQobuzDiscoveryPolling,
  startQobuzPlaylistSync, startQobuzSyncPolling, cancelQobuzSync,
  startQobuzDownloadMissing, rehydrateQobuzDownloadModal, etc.).
  Reuses the shared YouTube discovery modal via fake `qobuz_<id>`
  urlHash and is_qobuz_playlist flag. Shared switch statements in
  getModalActionButtons / generateTableRowsFromState / Wing It helpers
  in downloads.js gain new isQobuz branches alongside the existing
  per-service ones.
* webui/index.html — new Qobuz tab button + content div, slotted
  between Deezer and Deezer Link.
* webui/static/style.css — new .qobuz-icon for the tab icon.
* webui/static/core.js — qobuzPlaylists / qobuzPlaylistStates /
  qobuzPlaylistsLoaded globals.

Followed the existing per-service pattern verbatim rather than
refactoring the duplicated transformers across Tidal / Deezer /
Spotify-public / YouTube / Mirrored — that refactor is its own follow-up
PR per the "don't break Tidal/Deezer" scope discipline. Adding the 6th
copy of a proven pattern is lower risk than collapsing 5 working
services behind a new abstraction.

Tests:
* tests/test_qobuz_playlists.py — 12 tests covering pagination,
  normalization, favorites virtual-ID routing, artist-name fallback
  chain (performer → album.artist → 'Unknown Artist'), and
  unauthenticated short-circuits.
2026-05-23 23:27:36 -07:00
Broque Thomas
eba7f61e04 Surface metadata source on Import album results (#681)
Import album search silently fell through to the next source in
METADATA_SOURCE_PRIORITY when the configured primary returned zero
matches — intentional behavior shared with the auto-import worker
(see core/auto_import_worker.py:1316). With MusicBrainz selected and
a query MB couldn't resolve, users saw Deezer cards with no indication
their primary was bypassed.

Backend now echoes `primary_source` on /api/import/search/albums,
/api/import/search/tracks, and /api/import/staging/suggestions.
Frontend renders a per-card 'via {source}' badge when the served
source differs from the primary, plus a banner above the grid when
every card came from a fallback source. Fallback semantics unchanged.

Also collapses an inline duplicate of _renderSuggestionCard inside
importPageSearchAlbum into a single shared renderer.

Regression test pins the contract: response carries primary_source +
per-album source when the chain falls back.
2026-05-23 16:22:17 -07:00
Broque Thomas
6c226613bf Add Soulseek album bundle downloads
Route primary Soulseek album downloads through the album-bundle staging flow, reusing preflight-selected folders when available. In hybrid mode, only the first configured source can claim whole-album bundle behavior so later Soulseek fallback keeps the existing per-track/source-reuse path.

Allow Soulseek album bundles to stage completed tracks when some same-source transfers fail or time out, and keep partial bundles from blocking per-track fallback. Add coverage for dispatcher gating, master flow ordering, task-worker staged-miss behavior, and Soulseek bundle polling.
2026-05-23 15:08:21 -07:00
Broque Thomas
5d1f3c1b48 Fix Picard albumartist orphan false positives
Teach the orphan file detector to match tracked files by both track artist and album artist. This prevents Picard-style albumartist/album (year)/track layouts from being reported as orphans when the DB track artist differs from the album artist.

Also check file albumartist tags during fallback matching and add a regression test for the reported Picard folder layout.
2026-05-22 08:43:57 -07:00
Broque Thomas
4179926899 Fix missing album placeholder asset path
Update Import Music album and queue artwork fallbacks to use the shipped /static/placeholder-album.png asset instead of the nonexistent /static/placeholder.png path.

Replace the remaining static UI fallback to the missing placeholder path and add a regression test that fails if static JS references it again.
2026-05-22 08:34:42 -07:00
Broque Thomas
a41eccbe3c Fix Usenet settings reload without restart
Refresh registry-backed download plugins when settings are saved so cached Prowlarr clients pick up new indexer credentials immediately. This preserves active download state by reloading existing plugin instances instead of rebuilding the registry.

Add regression coverage for orchestrator reload fanout and the Usenet plugin's cached ProwlarrClient refresh path.
2026-05-22 08:28:56 -07:00
Broque Thomas
763888e671 Support legacy HiFi track manifests
Add fallback support for public hifi-api instances that expose playback through /track/ instead of /trackManifests/. The capability checker now accepts either manifest shape, and downloads can use direct URLs decoded from the legacy base64 manifest.

Tests cover legacy instance capability detection and download-manifest fallback while preserving the newer trackManifests path.
2026-05-21 18:11:14 -07:00
Broque Thomas
fae13226e5 Check HiFi download capability via manifests
Probe public HiFi instances with the same trackManifests endpoint used by real downloads instead of the legacy /track endpoint. This prevents compatible instances from being falsely labeled search-only in Settings.

Centralize HiFi instance capability checks in HiFiClient and reuse manifest URI parsing with the download path.

Tests cover manifest-based capability detection, no legacy /track probe, and limited instances without a manifest URI.
2026-05-21 18:04:10 -07:00
Broque Thomas
b9af4ef4ef Handle transient SQLite IO during maintenance
Keep full refresh moving when post-clear VACUUM hits a transient disk I/O error, and retry clear_server_data once when the clear step itself sees the same transient SQLite failure.

Retry metadata cache maintenance writes once on transient disk I/O errors so first-attempt cache jobs do not fail when an immediate retry would succeed.

Tests cover best-effort VACUUM, clear retry behavior, and cache maintenance retry behavior.
2026-05-21 17:50:30 -07:00
Broque Thomas
f1d4f78e0e Repair stale media schema during refresh
Ensure upgraded databases have the tracks.file_size and albums.api_track_count columns after all legacy migrations run. Add defensive repair paths for Jellyfin track imports and album track-count caching so stale schemas self-heal instead of dropping full-refresh track imports.

Tests cover legacy schema repair and api_track_count self-repair.
2026-05-21 17:41:54 -07:00
Broque Thomas
8012f41ef7 fix(album-completeness): block cross-artist auto-fill
Reported bug: filling Jamiroquai's "Light Years" single pulled in
Gut's "Light Years" album tracks (different artist, completely
different genre — track titles like "Wound Fuck" and "Eat My Cum"
made the contamination obvious). The Album Completeness auto-fill
was the only file-copying path with a loose 0.50 SequenceMatcher
artist gate, which let unrelated candidates through whenever the
title matched well.

Two-stage defense now sits on the only album-fill code path
(_fix_incomplete_album in core/repair_worker.py):

- Stage 1 — _album_fill_target_artist_allows_track. Pre-search
  gate: before doing any library lookup for a missing track,
  refuse to operate if the missing track's source artist(s)
  don't match the target album's artist. Compilation albums
  (album_artist in {'various artists', 'various', 'soundtrack'})
  bypass the gate so legitimate VA releases still work. Empty
  source-artist metadata also bypasses for backward compat with
  older missing-track records that don't carry per-track artist.
- Stage 2 — _album_fill_artist_names_match. Replaces the old
  0.50 SequenceMatcher with an alias-aware 0.82 threshold that
  uses core.matching.artist_aliases when available (handles
  diacritic variants like Beyoncé/Beyonce and known stage names)
  with a normalized-similarity fallback if the aliases module
  isn't importable. Skipped candidates are logged at debug so a
  later support ticket can show what was rejected and why.

Tests in tests/test_repair_worker_album_fill.py reproduce the
exact reported scenario: target album "Light Years" by Gut +
missing track from a Jamiroquai source → skipped with a logged
warning, no copy attempted, wishlist not poisoned. Second test
covers Stage 2 directly with a wrong-artist library candidate.
Existing test_perform_album_fill_copy_branch still passes.

Note: this fix prevents NEW cross-artist contamination via
Album Completeness. It does not clean up the data anomaly that
made Gut's library entry appear to have a "Light Years" album
in the first place — that's a separate data-quality issue worth
investigating if it recurs.
2026-05-21 16:49:12 -07:00
Broque Thomas
dee200b267 Add torrent usenet PR notes and test updates
Adds the PR description for the torrent/usenet release-staging feature and updates drifted tests for the new plugin registry entries and search exclude_sources signature.

Verified with the focused pytest command covering cancellation and default registry source registration.
2026-05-21 15:28:48 -07:00
Broque Thomas
6c9b43225a Add torrent and usenet release staging support
Adds torrent/usenet as release-oriented download sources with album-bundle staging, live progress reporting, and post-processing that selects the requested audio file from completed releases instead of blindly importing the first file.

Keeps album-bundle behavior gated to single-source torrent/usenet album downloads, excludes release sources from hybrid album per-track searches, and allows hybrid non-album tracks to use release results safely.

Improves staged-release matching for featured/bonus track filenames while preserving version mismatches, records torrent/usenet provenance in library history, and updates service/status UI labels.

Covers the flow with focused lifecycle, status, staging, validation, task worker, post-processing, and import side-effect tests.
2026-05-21 14:22:21 -07:00
Broque Thomas
8b0de9eb76 fix(downloads): harden album bundle staging
Route torrent and Usenet album bundles through private per-batch staging so Auto-Import cannot race public staging or duplicate imports.

Expose album-bundle progress in batch status and render it on the Downloads page while the external client is still downloading.

Tighten release handoff safety by rejecting archive path traversal, ignoring torrent candidates without a usable URL, and skipping Soulseek source reuse for torrent/Usenet batches.

Tests: .venv/bin/python -m pytest tests/downloads/test_downloads_status.py tests/test_album_bundle_dispatch.py tests/downloads/test_downloads_staging.py tests/test_torrent_usenet_plugins.py
2026-05-20 21:39:06 -07:00
Broque Thomas
440c3624f3 refactor(staging): inject batch-field accessor instead of importing runtime_state
Per code review: the album-bundle provenance override added in an
earlier commit reached into ``core.runtime_state.download_batches``
directly from inside the staging matcher. Sibling modules
shouldn't import each other's globals — the existing StagingDeps
pattern is the canonical way to inject everything else this helper
needs.

- core/downloads/staging.py: new optional ``get_batch_field``
  callable on ``StagingDeps`` (defaults to None for backward compat
  with any caller that doesn't know about it yet). The inline
  ``from core.runtime_state import download_batches`` is gone; the
  helper now calls ``deps.get_batch_field(batch_id,
  'album_bundle_source')`` and falls back to 'staging' when None
  is returned. Accessor exceptions are swallowed with a debug log
  so a deleted batch mid-process can't break the staging match.
- web_server.py: ``_build_staging_deps`` injects a small
  ``_staging_get_batch_field`` helper that wraps the tasks_lock +
  download_batches dict access. Centralises the lock semantics in
  one place — the staging module no longer needs to know about
  the lock or the dict.
- tests/test_staging_album_provenance.py: 5 new tests covering the
  full matrix — torrent override applied, usenet override applied,
  no override falls back to 'staging', missing accessor (default
  None) falls back to 'staging', accessor raising falls back to
  'staging'. Each test seeds + cleans a synthetic task in
  runtime_state so the test doesn't bleed state across the suite.
2026-05-20 20:43:35 -07:00
Broque Thomas
ad59bf05a1 refactor(downloads): lift album-bundle gate into its own module
Per code review: ~90 lines of inline gate logic in
``run_full_missing_tracks_process`` was inflating an already-580-
line worker function and was non-testable in isolation. Lifted to
``core/downloads/album_bundle_dispatch.py`` with two entry points:

- ``is_eligible(mode, is_album, album_name, artist_name)`` — pure
  predicate, no side effects, easy to assert against. Splits the
  gate decision from the resolution + run step so tests can pin
  the gate semantics without standing up a plugin.
- ``try_dispatch(...)`` — full flow. Returns True iff the master
  worker should stop (gate fired + failed); False = engaged-and-
  succeeded OR didn't engage, both fall through to per-track.

State access is now decoupled from ``runtime_state``:
- New ``BatchStateAccess`` Protocol with two methods
  (``update_fields``, ``mark_failed``).
- Concrete impl ``_BatchStateAccessImpl`` lives in master.py and
  wraps the tasks_lock + dict ops the original inline code did.
- Injected via parameter so the dispatch module never imports
  ``download_batches`` / ``tasks_lock`` directly.

Same goes for the plugin resolver and config getter — both
injected, so the dispatcher works against any orchestrator /
config implementation (including the in-test fakes).

Behavior unchanged. The master worker call site is now 11 lines
of boilerplate instead of 90 lines of inline conditional. Plugin
contract (``download_album_to_staging`` return dict shape)
unchanged.

- core/downloads/album_bundle_dispatch.py: new module owning the
  gate + execution. ~150 lines including docstrings and the
  Protocol definition.
- core/downloads/master.py: gate call site shrunk to a single
  ``if _album_bundle_dispatch.try_dispatch(...): return``. New
  ``_BatchStateAccessImpl`` class implements the Protocol against
  the existing ``download_batches`` dict + ``tasks_lock`` so the
  dispatcher gets injected access instead of importing them.
- tests/test_album_bundle_dispatch.py: 16 new tests covering the
  pure predicate (album-required, mode allowlist, name validation,
  case insensitivity), the resolver-failure fall-through
  (plugin missing, plugin lacks method, resolver raises), the
  success path returning False so per-track flows, the failure
  path returning True with state.mark_failed called, plugin-raise
  treated as a normal failure, whitespace stripping on names,
  and progress-callback mirroring lifecycle events into batch
  state.
2026-05-20 20:29:50 -07:00
Broque Thomas
670a2db95e refactor(downloads): extract album_bundle shared helpers + atomic copy
Per code review: the album-bundle helpers (release picker + staging
collision suffix) were defined as private symbols in torrent.py and
imported by usenet.py through ``from core.download_plugins.torrent
import _pick_best_album_release, _unique_staging_path``. Sibling
plugins shouldn't reach into each other's private surface — leaky
module boundary, and the underscore prefix says don't import.

Also addressed two latent issues at the same time:

- The Auto-Import sweep race: my plugin copied audio files into
  staging via plain ``shutil.copy2``, which exposes a partial file
  at the audio extension for the duration of the copy. The Auto-
  Import worker filters by audio extension when scanning Staging
  (AUDIO_EXTENSIONS in core/auto_import_worker.py), so a mid-flight
  scan could pick up a truncated file. Fix: copy to a
  ``.tmp.<random>`` sidecar first, then atomically rename via
  ``Path.replace`` (which is ``os.replace`` — atomic on the same
  filesystem). Auto-Import sees the file either at its final name
  or not at all.

- The 6-hour poll timeout was a hard-coded magic constant. Users
  with slow private trackers or large box sets would silently time
  out after 6h. Both the timeout and the poll interval are now
  read from config (``download_source.album_bundle_timeout_seconds``
  / ``..._poll_interval_seconds``) with safe fallback to the
  existing defaults when unset / non-numeric.

- core/download_plugins/album_bundle.py: new module owns the
  shared surface — ``pick_best_album_release`` (with quality_guess
  passed in as a parameter to avoid the circular import that would
  result from this module trying to know about torrent.py's title
  parser), ``unique_staging_path``, ``atomic_copy_to_staging``,
  ``copy_audio_files_atomically``, ``get_poll_interval``,
  ``get_poll_timeout``. Module-level size constants and quality
  weights live here too. Usenet's grabs-as-popularity-proxy is
  built into the picker so both plugins get the right behavior
  without divergent local logic.
- core/download_plugins/torrent.py: drops the local helpers + the
  hard-coded poll constants, imports from album_bundle. Per-track
  download flow still uses module-level ``_POLL_TIMEOUT_SECONDS``
  / ``_POLL_INTERVAL_SECONDS`` aliases (read from config once at
  import time, same as before from a per-track perspective).
- core/download_plugins/usenet.py: drops the imports of the
  torrent.py private helpers; everything goes through album_bundle
  now. Stops the cross-plugin private-import leak that started
  this whole refactor.
- tests/test_album_bundle.py: 23 new tests covering the picker
  heuristic (empty input, singleton drop, FLAC preference, grabs
  fallback for usenet, size-floor / ceiling boundaries), the
  collision-suffix logic, the atomic-copy invariant (concurrent
  scanner thread asserts it never observes a partial audio file
  during five sequential copies), the failure-skip behavior of the
  batch copier, and the config-driven poll cadence including
  garbage-input fallback.
- tests/test_torrent_usenet_plugins.py: existing picker tests
  updated to call the new module-level helpers instead of the
  former torrent.py privates.
2026-05-20 20:26:30 -07:00
Broque Thomas
c990ce079d feat(downloads): album-bundle flow for torrent/usenet single-source mode
Fixes the core architectural mismatch between indexer-based sources
and the per-track search-and-pick contract every other download
plugin satisfies. Prowlarr returns release-level torrents and NZBs;
searching for "Luther (with SZA)" against the GNX album torrent
scores near-zero on track-title similarity. Per-track candidate
validation rejects every result, every track in the batch flips
to not_found. The album-name fallback added in an earlier commit
papers over it for some cases but doesn't fix the fundamental
behavior: the user wanted the whole album.

New album-bundle flow does what the user actually wanted:
1. Gate fires inside core/downloads/master.py BEFORE the per-track
   analysis loop, strictly when the batch has an album context AND
   download_source.mode is 'torrent' or 'usenet' (single-source —
   hybrid stays per-track to preserve fallback to Soulseek / etc.).
2. Plugin's new download_album_to_staging method searches Prowlarr
   ONCE for the album as a whole ('<artist> <album>'), filters to
   the right protocol, runs results through _pick_best_album_release.
3. Picker prefers seeded FLAC over low-seeded MP3, drops single-
   track torrents that snuck in via the 40 MB size floor (single
   tracks are typically ~10 MB), falls back to most-seeded when
   every candidate is below the floor.
4. Picked release goes to the active adapter (qBit / Transmission /
   Deluge for torrent; SAB / NZBGet for usenet). Polls until
   complete with progress mirrored into the batch state so the
   Downloads page can show meaningful status.
5. On completion the existing archive_pipeline walks the save dir
   (extracting archives if any), every audio file gets copied into
   the staging folder via _unique_staging_path so concurrent batches
   don't collide.
6. Gate exits, master worker continues into the normal per-track
   flow. Each track task hits try_staging_match early in the worker
   and finds its file by fuzzy title match — no Prowlarr search
   ever fires per-track, no candidate rejection, files flow through
   the existing post-processing pipeline (tags, AcoustID, library
   import).

Gate is strictly opt-in. Three orthogonal conditions must all hold:
batch_is_album, mode in ('torrent', 'usenet'), and the plugin must
expose download_album_to_staging. Any other source / hybrid mode /
non-album batch flows through the master worker unchanged. The
existing per-track torrent path still works for basic-search
single-track grabs.

- core/download_plugins/torrent.py: download_album_to_staging plus
  _pick_best_album_release and _unique_staging_path helpers (shared
  with the usenet plugin). _poll_album_download mirrors the existing
  poll loop with progress callback emission.
- core/download_plugins/usenet.py: parallel implementation reusing
  the picker + staging helpers. Different state set ('failed' vs
  'error') from the usenet adapter contract.
- core/downloads/master.py: ~90-line gate right after batch context
  loading. Mirrors plugin lifecycle into batch state under
  ``album_bundle_*`` keys so the Downloads page can render progress
  while the torrent/usenet job runs (per-track tasks don't exist
  yet during this phase). Failed bundle download fails the batch
  with a meaningful error; missing plugin / context falls back to
  the per-track flow with a warning.
- tests/test_torrent_usenet_plugins.py: 5 new tests pinning the
  album picker preferences (FLAC over MP3 with comparable size +
  better seeders, size floor drops singles, fallback when all
  small), staging-path collision suffix, and the not-configured
  short-circuit.
2026-05-20 18:48:48 -07:00
Broque Thomas
478fd25dd6 fix(downloads): pre-fill artist/title so search UI doesn't show download URL
Real-world test surfaced the bug — torrent results displayed
'by download?apikey=c15d6f69...&link=...' as the uploader / artist
in the basic search UI. The cause is TrackResult.__post_init__:
when artist is None it runs parse_filename_metadata on the bare
filename, and our filename starts with the indexer's download URL
(needed so download() can recover the URL later). The auto-parser
treats the URL as 'artist' and ships it to the UI.

Fix:
- core/download_plugins/torrent.py: new _parse_release_title()
  splits 'Artist - Title' / 'Artist - Album' out of the release
  title and strips trailing [FLAC] / (2016) tags. Falls back to
  ('', cleaned_title) when no dash is found, and explicitly
  rejects URL-looking strings as an extra defence. The projection
  pre-fills both artist and title on TrackResult, so __post_init__
  skips the auto-parse entirely. When the release title has no
  dash, artist defaults to the indexer name so the UI shows
  'by Indexer' instead of a URL.
- core/download_plugins/usenet.py: imports the new helper and
  applies the same fix.
- tests/test_torrent_usenet_plugins.py: 5 tests for the new
  helper (dash split, trailing-tag stripping, no-dash fallback,
  multiple-dash preservation, URL-prefix rejection). Existing
  projection tests updated to assert artist + title come through
  parsed correctly, plus a new test pinning the indexer-name
  fallback for titles without a dash so the URL-leak regression
  can't return.
2026-05-20 18:06:23 -07:00
Broque Thomas
080b1aa1b4 feat(downloads): wire torrent + usenet as live download sources
The payoff for the previous five commits. Two new download
sources slot into the existing DownloadSourcePlugin contract,
backed by Prowlarr (search) + the torrent or usenet client
adapter (transfer) + archive_pipeline (post-extract walk). They
appear in the Download Source dropdown next to Soulseek / Tidal /
Lidarr / etc. and also participate in hybrid mode.

Pipeline (both plugins, mirror shape):
1. search(query) → ProwlarrClient.search filtered to the right
   protocol, projected into TrackResult / AlbumResult shapes the
   existing search UI already speaks. Filename field encodes the
   indexer's download URL (or magnet URI for torrents) so
   download() can recover it later.
2. download() → decodes URL, hands it to the active adapter
   (qBittorrent / Transmission / Deluge for torrent; SABnzbd /
   NZBGet for usenet), spawns a background poll thread that
   tracks progress + reports the adapter-reported save_path.
3. On 'seeding' / 'completed' → archive_pipeline walks the save
   directory, extracts any archives the downloader didn't
   already unpack, picks the first audio file as the canonical
   file_path. Matches the Lidarr client's single-track-pick
   contract — picking which specific track to import happens in
   post-processing.

- core/download_plugins/torrent.py: TorrentDownloadPlugin +
  module-level helpers (_decode_filename, _guess_quality_from_title,
  _parse_indexer_id_filter, _adapter_state_to_display, _row_to_status).
  Uses get_active_torrent_adapter() so a settings change to the
  client type takes effect without restart.
- core/download_plugins/usenet.py: UsenetDownloadPlugin —
  parallel shape, reuses the torrent module's helpers. Different
  enough states (no seeding, no magnet) to warrant its own class
  but cheap to keep in lockstep.
- core/download_plugins/registry.py: register 'torrent' and
  'usenet' plugins. Per the registry docstring this is the only
  wiring point needed — the orchestrator picks them up
  automatically via the iteration helpers.
- webui/index.html: 'Torrent Only (via Prowlarr)' + 'Usenet Only
  (via Prowlarr)' added to the Download Source dropdown. New
  redirect card (#prowlarr-source-redirect) explains that the
  actual config lives on the Indexers & Downloaders tab —
  shown whenever torrent or usenet is in the active source set.
- webui/static/settings.js: HYBRID_SOURCES gets two new entries
  so hybrid mode can pick them up. updateDownloadSourceUI now
  toggles the redirect card based on active sources.
- tests/test_torrent_usenet_plugins.py: 23 tests covering pure
  helpers (filename encode/decode round-trip incl. magnet URIs,
  quality guesser, state mapping), search projection logic
  (protocol filter, drops without URLs, magnet-preferred-over-URL,
  filename encoding, neutralised soulseek-specific score fields),
  is_configured (both prowlarr + adapter required), finalize
  (picks first audio file, errors on empty dir / missing save_path),
  clear/get_all lifecycle, DownloadSourcePlugin protocol
  conformance, and registry membership.
2026-05-20 17:22:19 -07:00
Broque Thomas
5f126584f9 feat(downloads): add archive_pipeline module for torrent/usenet downloads
Shared helper the upcoming torrent and usenet download plugins
both compose against. Narrow surface — no matching, no tagging,
no library import. Just walks audio files and extracts archives
when needed.

Why a separate module: usenet downloaders (SABnzbd, NZBGet)
already auto-extract by default, and Lidarr's import pipeline
extracts before SoulSync sees the files. The only client that
sometimes leaves an archive behind is a torrent client when the
album was packed as a .rar — most music torrents ship loose but
not all. Centralising the walk + extract logic means both new
plugins can do the same thing, and a future direct-archive source
(zip download from a private site, etc.) plugs in for free.

- core/archive_pipeline.py:
  - AUDIO_EXTENSIONS / ARCHIVE_EXTENSIONS constants (audio set
    matches core/imports/file_ops.py quality_tiers).
  - is_archive(path) handles compound extensions (.tar.gz etc).
  - walk_audio_files(directory) — recursive, case-insensitive.
  - find_archives_in_dir(directory) — top-level only (don't
    surprise-extract sample / proof folders inside a torrent).
  - extract_archive(archive_path, extract_to=None) — handles
    .zip, .tar variants, .rar (optional rarfile dep), .7z
    (optional py7zr dep). Optional deps warn-and-skip if absent.
  - extract_all_in_dir + collect_audio_after_extraction — the
    one-shot helpers the download plugins call after a download
    completes.
  - Path-traversal protection: every archive member's resolved
    path must stay inside the destination — first violator aborts
    the extract without writing anything. Applies to zip, tar,
    and rar.
- tests/test_archive_pipeline.py: 21 tests covering the walker
  (nested dirs, case-insensitive, ignores non-audio), archive
  detection (compound extensions, missing files), zip extraction
  + path-traversal rejection, tar.gz + tar path-traversal,
  multi-archive directory, mixed-loose-and-archived collection.
2026-05-20 17:05:00 -07:00
Broque Thomas
e4ca56b499 test: cover Prowlarr + torrent + usenet adapters
54 mocked unit tests pinning the parse + dispatch behavior of the
new indexer and downloader plumbing. No live services required —
HTTP is mocked at the requests-library boundary, RPC is mocked at
the _rpc_sync helper.

Coverage:
- core/prowlarr_client.py: parse_indexer / parse_result with
  category-shape variants, search query encodes repeated
  ``categories=`` and ``indexerIds=`` keys, check_connection hits
  the right endpoint with the right header.
- core/torrent_clients/qbittorrent.py: login sends the Referer
  CSRF header, login failure surfaces, parse_status normalises
  field names, eta <= 0 becomes None.
- core/torrent_clients/transmission.py: bare host URL is rewritten
  to /transmission/rpc, 409 + X-Transmission-Session-Id is
  renegotiated and the retry carries the new id, torrent-add
  surfaces torrent-duplicate hashes, eta -1 becomes None.
- core/torrent_clients/deluge.py: requires password to be configured,
  magnet vs HTTP URL hit different RPC methods, progress is
  normalised from 0-100 to 0-1.
- core/usenet_clients/sabnzbd.py: parse_timeleft handles HH:MM:SS
  and the MM:SS fallback, queue + history merge into a single
  get_all, addurl vs addfile are dispatched on the input type.
- core/usenet_clients/nzbget.py: requires URL + username + password,
  mb_value prefers the 64-bit size split over the legacy MB field,
  add_nzb base64-encodes raw bytes, GroupFinalDelete vs GroupDelete
  is picked by the delete_files flag, non-numeric job IDs fail fast.
- state mapping tables for all five adapters get explicit assertions
  so future refactors can't silently lose a native state value.

WHATS_NEW entry covers the test addition; no VERSION_MODAL_SECTIONS
entry — internal infrastructure, not user-facing.
2026-05-20 15:43:18 -07:00
Broque Thomas
3375b6c4bd Handle non-JSON Tidal auth responses
Detect JSON decode-like exceptions from Tidal's token endpoint and return a safer, more actionable error message. Adds a _looks_like_json_decode_error helper and special-cases that error in check_device_auth to log the non-JSON response and advise disabling VPN/proxy/network filtering and restarting SoulSync. A test was added to ensure the user-facing message does not leak the raw exception text while still returning an error status. Other errors continue to fall back to the existing behavior.
2026-05-20 14:04:45 -07:00
Broque Thomas
2fc08e199e Enforce duration tolerance for strict sources
Add duration tolerance logic and pre-download rejection for structured sources (tidal, qobuz, hifi, deezer_dl, amazon) when candidate duration deviates beyond allowed tolerance. Introduces helper functions _duration_tolerance_seconds and _duration_mismatch_exceeds_integrity_tolerance and uses resolve_duration_tolerance from core.imports.file_integrity. Log and skip candidates that would fail post-processing integrity checks to avoid wasted downloads. Update tests to include matching engine stub and new cases covering rejection and acceptance based on duration tolerance; also adjust imports and test fixtures.
2026-05-20 11:24:57 -07:00
Broque Thomas
136d665c8a feat(webui): cache artwork images on disk
Add a disk-backed image cache with hashed browser URLs, SQLite metadata, size/type validation, stale fallback, and per-image fetch locking. Route normalized artwork through /api/image-cache while keeping /api/image-proxy as a compatibility shim, and align browser max-age with the image cache TTL. Add focused tests for cache behavior and image URL normalization.
2026-05-20 10:43:47 -07:00
Broque Thomas
6e5ea1d490 fix(downloads): wait for post-processing result
Do not mark a monitored transfer as successful as soon as slskd reports completion. The monitor now only submits the post-processing worker; that worker reports the real success or failure after finding, verifying, and importing the file. If post-processing cannot be scheduled, mark the task failed and release the batch slot. Add a regression test for the premature success path.
2026-05-20 09:39:56 -07:00
Broque Thomas
735dd73865 fix(repair): rewire Unknown Artist Fixer deferred imports (#646)
The "Fix Unknown Artists" repair job crashed on every run with:

    ImportError: cannot import name '_build_path_from_template' from
    'core.repair_jobs.library_reorganize'

Commit ca5c9316 ("Rewrite Library Reorganize job to delegate to per-
album planner") moved the private path-builder + quality-string
helpers out of `core.repair_jobs.library_reorganize` and into the
import pipeline. `unknown_artist_fixer.py:163` still imported them
from the old module — its scan() defers the imports to avoid pulling
web_server's Flask boot into the test harness, so the broken target
only surfaces at runtime when the user actually runs the job. The
tool was completely unrunnable.

Re-wired the deferred imports:

    core.repair_jobs.library_reorganize._build_path_from_template
        -> core.imports.paths.get_file_path_from_template_raw
    core.repair_jobs.library_reorganize._get_audio_quality
        -> core.imports.file_ops.get_audio_quality_string

Both replacements have identical signatures + return shapes (verified
by inspecting library_reorganize's pre-refactor implementations vs
the import-pipeline equivalents):

    get_file_path_from_template_raw(template: str, context: dict)
        -> tuple[folder: str, filename_base: str]
    get_audio_quality_string(file_path: str) -> str

No call-site changes needed beyond the import target.

2 new regression tests in `tests/test_unknown_artist_fixer.py`:

    test_deferred_path_imports_resolve — runs the same import
    statements scan() runs, so the NEXT refactor that moves these
    helpers fails CI rather than reaching the user.

    test_deferred_path_helper_shape_matches_fixer_usage — pins the
    `(folder, filename_base)` 2-tuple contract the fixer's unpack
    relies on. Catches return-shape drift even when the import
    target stays valid.

Audited every consumer of `core.repair_jobs.library_reorganize` —
only one stale import (this file). The test suite covers the only
production caller.

5 fixer tests pass (3 existing + 2 new regression guards).
2026-05-19 22:09:15 -07:00
Broque Thomas
79ad4d885d fix(quarantine): drop already-quarantined sources from candidate picker (#652)
When a file failed AcoustID verification and got quarantined, the next
auto-wishlist cycle would search for the same track, the deterministic
quality picker would re-select the same (uploader, filename) source,
re-download it, and re-quarantine it. Users woke up to hundreds of
duplicate .quarantined entries from a single bad upload — same source
URL repeatedly, byte-for-byte identical files.

Root cause: `SoulseekClient.filter_results_by_quality_preference` ranks
candidates by quality + bitrate density only. Quarantine history wasn't
consulted, so a high-bitrate FLAC upload with a wrong-track AcoustID
fingerprint kept winning the picker against every other candidate.

Fix shape:

- New helper `core/imports/quarantine.py::get_quarantined_source_keys`
  reads every quarantine sidecar's `context.original_search_result`
  and returns the set of `(username, filename)` tuples for O(1)
  membership checks. Sidecars missing the context field (legacy thin
  sidecars written pre-Feb 2026, or orphaned files) and corrupt JSON
  are skipped silently — defensive against transient FS / encoding
  issues.

- `SoulseekClient._drop_quarantined_sources` runs the membership
  filter against incoming TrackResults, drops matches, logs a single
  INFO line with the skip count. Called first inside
  `filter_results_by_quality_preference` so all four callers
  (search-and-download, master worker, validation, orchestrator)
  benefit transparently.

- Approving or deleting a quarantine entry removes its sidecar, so
  the dedup key disappears from the set on the next search — gives
  the user a way to opt back in to a previously-quarantined source
  without restarting the app.

7 helper tests cover: missing dir, empty dir, well-formed sidecars
collected as tuples, legacy sidecars skipped, empty source fields
skipped (so empty-string keys can't accidentally drop unrelated
results), corrupt JSON tolerated, duplicate quarantines collapse.

5 integration tests pin: clean candidates pass, known-bad candidates
drop, missing quarantine dir returns input unchanged, filesystem
errors swallowed (defensive), full `filter_results_by_quality_preference`
runs the dedup BEFORE the quality picker — so a high-quality
quarantined source can't win on bitrate.

692 existing download + import tests still green. Cosmetic surface
of the fix is invisible — same UX as today when no quarantine entries
exist; loop only kicks in once a sidecar has been written.

Out of scope: bulk-select / multi-delete UI for the quarantine tab —
S-Bryce mentioned this as a separate pain point in the issue, but
it's its own UX work, not a one-commit drive-by.
2026-05-19 21:19:50 -07:00
Broque Thomas
987409508b fix(metadata): surface MusicBrainz 'Other' release-groups in discography (#650)
S-Bryce reported that for some artists (Vocaloid producers, JP indie
acts, niche Western indie) the artist detail page was missing whole
release-groups visible on musicbrainz.org. Downloaded tracks from
those release-groups appeared in artist track counts but were not
bound to any visible album / single card — orphan "ghost" tracks the
user couldn't browse to.

Two duplicated bugs fed each other:

1. `core/musicbrainz_search.py` browsed MB release-groups with
   `release_types=['album', 'ep', 'single']`. MB's primary-type
   vocabulary is {Album, Single, EP, Broadcast, Other} — music
   videos, one-off web releases, and broadcast singles use Other.
   Pre-fix the filter dropped them at the API layer.

2. Three sites duplicated the same "raw primary-type → internal
   album_type" mapping with slightly different vocabularies and all
   silently defaulted unknown values (including 'Other') to 'album':

       core/musicbrainz_search.py  `_map_release_type`
       core/metadata/types.py      inline `{single:single, ep:ep}.get(...)`
       core/metadata/cache.py      Deezer-specific record_type guard

Letting Other through the filter without a real mapper would have
placed music videos in the Albums view alongside LPs — visually
misleading.

Fix shape:

- New `core/metadata/release_type.py` — single canonical mapper
  consumed by every provider's raw→Album projection. Knows the full
  MB vocabulary including 'other' and 'broadcast'; routes both into
  the singles bucket since they're functionally single-track
  releases. Compilation secondary-type override preserved (MB's
  canonical Greatest-Hits pattern is `primary=Album,
  secondary=[Compilation]`).

- `core/musicbrainz_search.py` `_map_release_type` becomes a thin
  alias for the new helper so the six internal call sites stay
  intact. API filter gains 'other'.

- `core/metadata/types.py` Album projection drops its inline mini-
  mapper and calls the canonical helper. Now also handles the
  compilation secondary-type override it was previously missing.

- The Deezer-specific cache.py guard stays as-is — Deezer's
  record_type vocabulary is closed (album|single|ep), not affected
  by this issue.

Verified end-to-end against MB for S-Bryce's artist (`46196b9c-affa-
4616-b53b-e967c8bd70e0`, inabakumori): pre-fix returned 22 release-
groups; post-fix returns 27, with the 5 extra all landing in the
Singles section with album_type='single' as intended.

23 new unit tests pin the mapper contract (case-insensitive primary
types, compilation secondary override, Other/Broadcast → single,
unknown → album default preserved, defensive empty/None inputs).
2 new tests in test_musicbrainz_search pin the API filter inclusion
of 'other' and the round-trip into the Singles bucket. All 516
existing metadata tests still green — refactor leaves historical
behaviour for {album, ep, single, compilation} unchanged.
2026-05-19 20:20:28 -07:00
Broque Thomas
54e4ba843f fix(soulseek): suppress connection-error log spam when slskd unreachable (#649)
When slskd_url is configured but the host is unreachable (slskd not
running, wrong port, host.docker.internal not resolving), the frontend's
/api/downloads/status polling fanned out to every download plugin
including Soulseek. soulseek_client._make_request hit a DNS / connect
failure on each poll and logged it at ERROR. Result: one
"Cannot connect to host host.docker.internal:5030" log line every
~2-3 seconds for the entire duration of any download — visible spam
even when the user wasn't using Soulseek at all.

Caught aiohttp.ClientConnectorError explicitly in both _make_request
and _make_direct_request. First failure emits one WARNING with
actionable context (start slskd, or clear soulseek.slskd_url if you
don't use Soulseek). Subsequent failures demote to DEBUG. The
_last_unreachable_logged flag resets on any successful (200/201/204)
response so a later outage warns again — suppression is per-outage,
not per-process-lifetime. Same shape as the existing _last_401_logged
suppression for auth failures.

The architectural gap (status polling fans out to soulseek even when
the user has soulseek disabled in their active download sources) is
intentionally left for a follow-up. The plugin-iteration code lives
in core/download_engine/engine.py and core/download_orchestrator.py;
threading a "skip-when-not-active" gate through every caller is a
bigger refactor than this user-facing log cleanup warrants. The
WARNING-once message tells the user what to do in the meantime.

5 new pinning tests cover the suppression contract: connection error
returns None (not raises), first failure WARNs + sets flag, repeats
stay quiet, successful response resets the flag, _make_direct_request
follows the same pattern, and non-connection exceptions still log at
ERROR so real bugs aren't hidden behind the new suppression.
2026-05-19 19:44:17 -07:00
Broque Thomas
daf9a527d9 feat(fix-popup): include MusicBrainz in the auto-search cascade
The Fix Track Match modal's auto-search was hardcoded to query only
Spotify -> Deezer -> iTunes, ignoring MusicBrainz entirely — even for
users with MB set as their primary metadata source. MB-niche recordings
(canonical entries with diacritics, fringe / non-mainstream tracks that
the commercial catalogues don't carry) had no chance.

Wiring:

- New `MusicBrainzSearchClient.search_tracks_with_artist(track, artist,
  limit)` for surfaces that already have title + artist split. Uses MB's
  bare-query mode (strict=False) — diacritic-folded, alias/sortname
  indexed — same recall rationale as the earlier MBID-paste endpoint.

- New route `GET /api/musicbrainz/search_tracks` mirrors the existing
  /api/{spotify,itunes,deezer}/search_tracks endpoints exactly: accepts
  `track`+`artist` (or legacy `query`) + `limit`, returns
  `{tracks: [{id, name, artists, album, duration_ms, image_url, source}]}`.
  Applies the same `core.metadata.relevance.rerank_tracks` pass Deezer /
  iTunes use, which is critical because MB's free-text scoring weighs
  title-text matches heavily and would otherwise rank cover / tribute
  recordings above the canonical version.

- `_search_tracks_text` gains a `min_score` parameter. The cascade path
  passes 20 (vs the enhanced-search-tab default of 80) so MB recordings
  whose title doesn't literally contain the artist name still enter the
  candidate pool — without that, "Army of Me" + "Bjork" only surfaces
  the HIRS Collective cover (score 100) and drops Björk's canonical
  recording (score 28). The rerank pass then surfaces Björk by artist
  match. Verified against real MB API: pre-fix returned only the cover;
  post-fix top 5 are all Björk.

- Fix popup `allSources` array (wishlist-tools.js) gets MB appended.
  The existing `activeIdx` reorder logic moves MB to the front when
  it's the active primary; otherwise MB sits last (1 req/sec rate
  limit makes it the slowest source).

7 new unit tests on the adapter: bare-query mode is used, missing
artist falls back to None (drops AND-clause), empty inputs short-circuit,
low-score candidates are kept for rerank to handle, default strict +
default min_score behaviour preserved for the existing search-tab path,
client errors are swallowed so the cascade falls through to the next
source.

Discogs intentionally absent — Discogs has no track-level search API
(see core/discogs_client.py:575 — returns []). Adding a Flask endpoint
that always returns empty would be a permanent no-op.
2026-05-19 19:06:41 -07:00
Broque Thomas
97f35de44e test(amazon): update search_albums test for derived-from-tracks behavior
Commit 478bcc5d (`fix(amazon): search albums/artists and track numbers
for t2tunes`) switched `search_albums` to query `types=track` and derive
Album objects from the album metadata on each track hit — Amazon's
album-type query is broken upstream. The matching test was left asserting
the old "filter out track hits → return []" behavior and has been failing
in CI ever since.

Rewritten to assert the current intended behavior: track hits yield
distinct albums by album ASIN, with the artist credit + name preserved.
No code change.
2026-05-19 18:24:27 -07:00
Broque Thomas
036faff8b1 feat(fix-popup): paste MusicBrainz URL/MBID to match directly
Power-user escape hatch on the Discovery Fix Track Match modal — when
fuzzy auto-search ranks the wrong recording among many same-title
versions (10 remasters, live cuts, alt sessions), paste the MusicBrainz
recording URL or bare UUID into the new field and resolve straight to
that record.

Layout:

- Shape adapter `get_recording_flat(mbid)` lives in
  `core/musicbrainz_search.py` next to existing `get_track_details`.
  Returns the flat Fix-popup track shape (artists as `string[]`,
  album as string, single `image_url`) — distinct from the
  Spotify-shaped nested dict `get_track_details` returns.

- New route `GET /api/musicbrainz/recording/<mbid>` is a thin wrapper:
  validates MBID format with an anchored UUID regex, calls the adapter,
  returns 400 / 404 / 200 with no inline shape massaging.

- Frontend `parseMusicBrainzMbid()` lives in `shared-helpers.js` —
  pure URL/UUID parser, reusable from other surfaces (failed-MB cache,
  manual match) without duplication.

- Fix modal HTML gets one new input row + button; existing search row
  and result render pipeline are untouched. New `lookupDiscoveryFixByMbid()`
  fetches the endpoint and feeds the single result through the existing
  `renderDiscoveryFixResults` -> confirm-dialog -> match pipeline, so MB-
  paste matches go through the exact same selection flow as auto-search
  results.

- Enter-key bound on the MBID input via a separate handler ref so its
  lifecycle matches the search-input handlers without conflating the
  two submit targets.

7 unit tests cover the adapter: happy path, empty/None MBID, MB returns
None, recording-without-release (empty album), multi-artist credits,
includes-list contract, and client-error swallow.

Out of scope: the Fix popup's fuzzy cascade is still hardcoded to
spotify/deezer/itunes regardless of which primary source the user has
configured. Adding MB to that cascade (when MB is the active primary)
is a separate concern.
2026-05-19 17:03:19 -07:00
Broque Thomas
43ed30b4d2 fix(musicbrainz): user-facing search recall + album-detail 404
Two bugs surfacing on the Fix popup and enhanced-search MB tab:

1. Strict Lucene phrase queries (`recording:"X" AND artist:"Y"`) killed
   recall on user-facing manual search — diacritics ("Bjork" vs canonical
   "Björk"), bracketed suffixes like "(Live)", and any AND-clause
   mismatch returned zero results. Added `strict: bool = True` param to
   `search_release` / `search_recording`; when False, sends a bare query
   joining title + artist so MB hits alias/sortname indexes with
   diacritic folding. `/api/musicbrainz/search` (Fix popup) and
   `core/library/service_search.py` (service tabs) now pass strict=False.
   Enrichment workers stay on strict mode — precision matters there
   because they auto-accept the top hit above a confidence threshold.

2. Every MB album click was silently 404-ing — `_render_release_as_album`
   passed `cover-art-archive` as an MB `inc` param, but it's not a valid
   include for the /release resource (MB rejects with 400). The CAA flags
   come back on every release response by default, so dropping the bad
   include preserves the image-scope picker logic intact.
2026-05-19 15:38:24 -07:00
Broque Thomas
e0e31079e6 Update test: get_release includes cover-art-archive 2026-05-18 21:20:57 -07:00
Broque Thomas
f3ad65de34 Complete MusicBrainz watchlist source parity
Add MusicBrainz watchlist artist ID storage, badges, linked-provider editing, and per-artist preferred source support.

Backfill watchlist MusicBrainz matches from already-enriched library artists so existing MusicBrainz worker matches appear in watchlist cards and settings.

Extend bulk watchlist add, liked artist matching, artist map source picking, and service status labels to recognize MusicBrainz, with regression tests for watchlist ID persistence and backfill.
2026-05-18 19:19:25 -07:00
Broque Thomas
5bc5fbb662 Add MusicBrainz as a metadata source
Register MusicBrainz as a first-class metadata source alongside Deezer, iTunes, Spotify, Discogs, and Hydrabase. Expose the shared client through metadata services, add the settings option, and expand the MusicBrainz search adapter with source-compatible artist, album, track, and detail methods.

Carry MusicBrainz IDs through similar-artist discovery, recommended artists, artist map serialization, and personalized playlist selection. Update DB migrations and lookup filters so similar_artist_musicbrainz_id is preserved on older schemas and used for source requirements and library exclusion.

Normalize MusicBrainz album adapter output for import context and add regression coverage for registry mapping, typed album conversion, and similar-artist filtering. Verified by user with 120 focused tests passing.
2026-05-18 18:47:13 -07:00
Broque Thomas
aaf312cd34 Honor manual library matches across source labels
Manual matches can be created from sync history as mirrored while wishlist and download flows later see the same track as wishlist or a provider source. Add a shared track-level lookup that falls back from exact source/id to source_track_id and title/artist, then use it for wishlist adds, cleanup, and download analysis so mapped tracks are not re-added or redownloaded.

Add coverage for mirrored-source matches being honored by wishlist cleanup and download batches, including the internal wishlist force-download path.
2026-05-18 16:32:38 -07:00
Broque Thomas
52dcdbe0f7 Harden Amazon worker schema migration
Ensure the Amazon enrichment worker verifies its required columns before querying pending work or progress, preventing upgraded installs from spamming no-such-column errors when amazon_match_status is missing.

Add regression coverage for legacy databases without Amazon enrichment columns.
2026-05-18 15:47:04 -07:00
Broque Thomas
3a4017ea2b feat: artist-detail deep linking — /artist-detail/:source/:id
Artist detail pages previously always pushed /artist-detail to the URL,
so refreshing the page or sharing a link would drop users on a broken
empty page with no artist loaded.

URL format is now /artist-detail/:source/:id (e.g.
/artist-detail/spotify/4tZwfgrHOc3mvqsCAfo4LT or
/artist-detail/library/42). The source segment lets the backend
synthesize a response from the right metadata client without a DB hit.

Changes:

Client routing (legacy shell + TanStack bridge)
- buildArtistDetailPath / _getDeepLinkArtistDetail added to init.js;
  parse both new :source/:id and legacy bare :id formats so old
  bookmarks still work
- navigateToPage passes artistId + artistSource through to the router
  bridge, which builds the dynamic href instead of hardcoding route.path
- resolveShellPageFromPath / resolveLegacyShellPageFromPath use a prefix
  match so /artist-detail/* resolves to artist-detail page-id
- globals.d.ts typed for artistId / artistSource options
- activateLegacyPath and syncActivePageFromLocation (popstate) both
  restore artist from URL using skipRouteChange:true to avoid a
  re-navigation loop back to /artist-detail
- loadInitialData restores artist from URL on page load (router not yet
  mounted at DOMContentLoaded so legacy path runs unconditionally)
- Same-artist guard in navigateToArtistDetail prevents double-fetch
  when the router fires activateLegacyPath after the initial navigation

Server
- artist_source_detail.build_source_only_artist_detail now resolves
  artist name from the source API when none is supplied, so deep-link
  restores with an empty name string still render correctly

Tests
- test_spa_deep_linking: /artist-detail/42 and /artist-detail/spotify/ID
  both serve index.html
- bridge.test.ts: source-aware URL building and library fallback
- route-manifest.test.ts: prefix path resolution
- artist_source_detail: name resolved from source when input is empty
2026-05-18 13:07:54 -07:00
Broque Thomas
e061f12a05 Filter owned artists from discovery recommendations 2026-05-18 11:40:12 -07:00
Broque Thomas
f25433ea57 Harden quarantine approval flows 2026-05-18 08:37:11 -07:00
Broque Thomas
54dbd150cb Preserve full release dates in audio tags 2026-05-17 23:02:41 -07:00
Broque Thomas
025007b97f Tighten artist discography soundtrack matching 2026-05-17 22:51:38 -07:00
Broque Thomas
0345478361 Skip wishlist adds for manual library matches 2026-05-17 20:50:55 -07:00
Broque Thomas
3e7eeb7c9c Honor manual matches in automatic wishlist cleanup 2026-05-17 20:43:04 -07:00
Broque Thomas
42f4aa5eac Add manual library track matching 2026-05-17 20:27:05 -07:00
Broque Thomas
f3d5ef6528 Test missing-track existing file imports
Add service-level coverage for the Enhanced Library I Have This flow: copying an existing source file, writing the target album DB row, preserving source audio, inheriting album identity tags, and migrating older track tables that lack disc_number.
2026-05-17 14:18:17 -07:00
Broque Thomas
076cf9e516 Improve Soulseek album source selection
Add a conservative Soulseek album preflight scorer so album downloads choose a coherent slskd folder before per-track enqueue. The scorer compares album title, artist, year, track count, tracklist coverage, peer quality, and penalizes unexpected deluxe/remix/live-style folders.

Preserve hybrid source priority by only running Soulseek album preflight when Soulseek is the selected source or first in the hybrid order. If Soulseek is only a fallback behind another source, the normal hybrid flow is left alone.

Reuse the richest wishlist album context across tracks in the same album group so release date, artwork, album type, and album artist stay consistent for path generation. Also preserve peer-quality tie breakers when attempting equal-confidence candidates.

Tests cover correct-folder selection over larger wrong editions, Soulseek primary vs fallback hybrid behavior, shared wishlist album context, and peer-quality candidate ordering.
2026-05-17 10:08:43 -07:00
Broque Thomas
121651da2c Add amazon_id column to artists table for full source parity
Schema: ALTER TABLE artists ADD COLUMN amazon_id TEXT with index, added via
_add_amazon_columns migration called after Discogs in _run_migrations.

SOURCE_ID_FIELD: add "amazon" -> "amazon_id" entry. find_library_artist_for_
source now looks up Amazon artists by slug before falling back to name match,
same as every other source. artist_source_detail already stamps artist_info
[source_id_field] = artist_id so the amazon_id is set on source-only payloads.

Tests: add "amazon": "amazon_id" to EXPECTED_SOURCE_ID_FIELD; revert test
assertion back to strict equality (SOURCE_ONLY_ARTIST_SOURCES == SOURCE_ID_
FIELD.keys() holds again now that amazon has a column).
2026-05-16 17:06:54 -07:00
Broque Thomas
265fe5233e Fix Amazon artist detail: library upgrade lookup and artist images
Library upgrade: find_library_artist_for_source returned None immediately for
Amazon because SOURCE_ID_FIELD has no 'amazon' entry (no DB column for Amazon
artist IDs). The name-based fallback was unreachable. Fix: only skip the column
query when column is None, not the whole function — name lookup now runs for
any source when artist_name + active_server are provided.

Artist images: add AmazonClient._get_artist_image_from_albums so the standard
_get_artist_image_from_source path in metadata/artist_image.py can call it as
a fallback (same hook iTunes/Deezer/Discogs expose). Searches by unslugified
artist name, matches primary artist, fetches album cover from album_metadata.

Test: updated test_source_only_set_matches_mapping_keys → _contains_all_mapped_
sources to assert subset (not equality) — SOURCE_ONLY_ARTIST_SOURCES intentionally
includes sources without a DB column that rely on name-only lookup.
2026-05-16 17:01:37 -07:00
Broque Thomas
d944884ab4 Backfill album release_date from stream tags when T2Tunes metadata omits it
T2Tunes albumList entries may not include a release_date field, leaving the
$year path template empty. get_album() now falls back to the first track's
release_date (populated from the FLAC date tag via get_album_tracks) when
album metadata has none. Also try camelCase releaseDate key at all albumList
read sites (Album.from_metadata, get_album, _fetch_album_metas consumers).

1 new test: release_date backfilled from stream date tag when absent from
album metadata. date tag "2024-11-22" added to MEDIA_RESPONSE_FLAC fixture.
2026-05-16 16:32:54 -07:00
Broque Thomas
96a1c8b7b8 Enrich Amazon album track durations via search results
media_from_asin returns no duration data. get_album_tracks now does one
search_raw call using the album name + primary artist from stream tags,
filters hits by albumAsin == requested asin, and builds a duration_map
(track asin → duration_ms). Search failures are swallowed — duration_ms
falls back to 0 so the existing behaviour is preserved on error.

2 new tests: duration populated when search returns matching hit; duration
stays 0 when search endpoint returns an error.
2026-05-16 16:21:12 -07:00
Broque Thomas
51e00d4ebf Fix Amazon Music search quality: images, dedup, explicit stripping, album/artist clicks
- All search_raw calls switched from single-type to types="track,album" — T2Tunes only
  returns results when both types are requested together
- _fetch_album_metas: parallel fetch (up to 5 workers) of album cover art via
  album_metadata(asin) — T2Tunes search results carry no image URLs
- search_tracks: populates image_url, release_date, total_tracks from album meta
- search_artists: strips feat. credits via _primary_artist() so "Artist feat. X" and
  "Artist ft. Y" collapse to one "Artist" entry; uses album cover as artist image
  stand-in (same approach as iTunes — T2Tunes has no artist images)
- search_albums: name-based dedup (display_name + artist key) instead of ASIN-based;
  populates image_url, release_date, total_tracks from album meta (cap 10 ASIN fetches)
- _strip_edition(): strips [Explicit]/(Explicit) from track/album names — explicit is
  the default version; Clean/Edited/Censored labels kept as-is so they stay distinct
- get_album(): applies _strip_edition to name and _primary_artist to artist so
  MusicBrainz preflight matching doesn't fail on "[Explicit]" album names
- get_album_tracks(): populates track_number and disc_number from T2TunesStreamInfo
  instead of hardcoding None — fixes track ordering in multi-track album downloads
- get_artist() / get_artist_albums(): _unslugify() converts slug artist IDs back to
  search names; _primary_artist() in comparison handles feat-annotated results
- SOURCE_ONLY_ARTIST_SOURCES: added "amazon" so artist detail page doesn't 404
- build_source_only_artist_detail: added amazon_client param + dispatch branch
- web_server.py: resolve amazon_client in _build_source_only_artist_detail wrapper;
  add source_override=="amazon" branch in get_spotify_album_tracks endpoint
- 77 tests covering all above paths; all pass
2026-05-16 15:55:15 -07:00
Broque Thomas
14a99f47ab fix(tests): use asyncio.run() instead of get_event_loop() in amazon test helper
get_event_loop() raises RuntimeError on Python 3.11+ Linux when no loop
exists. asyncio.run() creates its own loop per call — no deprecation warning,
works across all supported Python versions.
2026-05-16 12:48:36 -07:00
Broque Thomas
5d8ca70fe5 Add T2Tunes probe unit tests
Tests for tools/t2tunes_probe.py: status endpoint, nested search
response flattening, streamable typo tolerance + decryption key flag,
non-JSON error handling, and HEAD→range-GET fallback for stream probing.
2026-05-16 11:49:03 -07:00
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.
2026-05-16 11:09:04 -07:00
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).
2026-05-16 10:49:11 -07:00
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.
2026-05-16 10:37:29 -07:00
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).
2026-05-16 10:31:17 -07:00
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.
2026-05-16 10:24:13 -07:00
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.
2026-05-16 07:46:41 -07:00
Broque Thomas
115d7ed9c5 Preserve personalized playlist metadata for wishlist 2026-05-15 21:50:32 -07:00
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`.
2026-05-15 21:13:16 -07:00
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.
2026-05-15 20:53:03 -07:00
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.
2026-05-15 19:33:34 -07:00
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.
2026-05-15 18:41:08 -07:00
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.
2026-05-15 17:18:32 -07:00
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.
2026-05-15 17:15:25 -07:00
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.
2026-05-15 17:02:08 -07:00
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.
2026-05-15 16:19:01 -07:00
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.
2026-05-15 12:35:45 -07:00
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).
2026-05-15 11:59:32 -07:00
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.
2026-05-15 11:24:35 -07:00
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).
2026-05-15 10:47:46 -07:00
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.
2026-05-15 10:25:41 -07:00
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.
2026-05-15 08:55:06 -07:00
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.
2026-05-15 07:56:18 -07:00
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.
2026-05-14 15:52:05 -07:00
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.
2026-05-14 15:25:16 -07:00
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.
2026-05-14 14:09:38 -07:00
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.
2026-05-14 13:07:15 -07:00
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.
2026-05-14 12:14:31 -07:00
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.
2026-05-14 11:10:51 -07:00