44 Commits (dev)

Author	SHA1	Message	Date
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.	3 days ago
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.	3 days ago
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.	5 days ago
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.	1 week ago
Broque Thomas	f25433ea57	Harden quarantine approval flows	1 week ago
Broque Thomas	f4cff78f13	Quarantine management — list, approve, delete, recover ... Closes #584. Quarantined files used to sit in ss_quarantine/ with a thin sidecar — no UI, no recovery, no way to see what got dropped. This adds the management surface the user needs without going to the filesystem. UI: new "Quarantine" button on the downloads page header opens a modal with every quarantined file (filename, expected track/artist, reason, when, size). Three actions per row: - Approve (one-click): restores the file, re-runs the post-process pipeline with ONLY the failing check skipped, lands in the library with full tags + lyrics + scan - Recover (legacy fallback): moves to Staging for thin-sidecar entries that lack the embedded context Approve needs - Delete: permanent removal of file + sidecar Per-check bypass: context['_skip_quarantine_check'] = 'integrity' / 'acoustid' / 'bit_depth'. Skips ONLY the named check — other quality gates stay live. No blanket bypass-all flag. Sidecar expansion: move_to_quarantine now persists the full json-serializable context via serialize_quarantine_context (drops non-JSON-safe values, walks nested dicts/lists/sets, str-coerces unknown objects) plus the trigger name. Existing thin sidecars are detected and routed to Recover instead of Approve. Pure helpers in core/imports/quarantine.py: list_quarantine_entries / delete_quarantine_entry / approve_quarantine_entry / recover_to_staging / serialize_quarantine_context. 27 tests pin every shape: orphan files / orphan sidecars / corrupt sidecars / collision-safe filename restoration / full-context vs thin-sidecar dispatch / json round-trip safety. Four new endpoints in web_server.py — thin glue around the helpers: GET /api/quarantine/list, DELETE /api/quarantine/<id>, POST /api/quarantine/<id>/approve, POST /api/quarantine/<id>/recover. Download modal status differentiates "🛡️ Quarantined" from "❌ Failed" so recoverable files are visible at a glance — checked against the error_message text, no schema change needed. Pipeline changes are three minimal per-check conditionals at the existing quarantine sites in core/imports/pipeline.py. Each move_to_quarantine call now passes its trigger name so the sidecar records which check fired. Full suite: 2992 passed.	2 weeks ago
Broque Thomas	177bd85355	Configurable duration tolerance for downloaded-file integrity check ... Previously hardcoded at 3s (5s for tracks >10min) — files drifting past that got quarantined with no user override. Live recordings, alternate masterings, and some legitimate uploads routinely drift further. New setting `post_processing.duration_tolerance_seconds`. Default 0 means "use auto-scaled defaults" (unchanged behavior for users who don't touch it). Positive value overrides the per-track defaults. Capped at 60s — past that the check is effectively off. Logic lifted to pure helper `resolve_duration_tolerance` in file_integrity.py. Coerces every plausible input (None / empty / zero / negative / unparseable / above-cap / numeric string / float) to either a float override or None for auto. 12 tests pin every shape. Wired into `core/imports/pipeline.py` at the integrity-check call site — runs for ALL matched downloads (Soulseek / Tidal / Qobuz / HiFi / YouTube / Deezer-direct) since they all share that pipeline. Settings UI input under Settings → Metadata → Post-Processing.	2 weeks ago
Broque Thomas	8a11a660af	Extract manual import route handlers ... Move the remaining manual import endpoint logic out of web_server.py and into core.imports.routes behind ImportRouteRuntime. The Flask endpoints now stay as thin compatibility wrappers for album/track search, album match/process, single-file import processing, and batched singles processing. Keep legacy test patch points intact by re-exporting build_album_import_match_payload from web_server and routing singles_process through an injected process_single_import_file callable. This preserves existing route-level monkeypatch behavior while keeping the extracted helper testable. Add focused helper coverage for Hydrabase enqueueing, search limit clamping, album match payload forwarding, album import side effects, single-file worker outcomes, malformed manual matches, and singles aggregation/injected-worker behavior. Verification: py_compile and git diff --check passed locally; bundled-Python smoke covered the extracted helpers. Claude reran the project tests and reported all tests passing.	2 weeks ago
Broque Thomas	d703d33178	Extract import staging route helpers ... Move import staging files/groups/hints/suggestions controller logic out of web_server.py and into core.imports.routes behind an ImportRouteRuntime dependency object. Keep the existing Flask routes as thin compatibility wrappers so the UI endpoint surface stays unchanged. Add focused tests for staging file filtering, album grouping, hint generation, cached suggestions, empty missing staging paths, and error payloads from failed path/metadata reads. Verification: py_compile passed for web_server.py, core/imports/routes.py, and tests/imports/test_import_routes.py. A bundled-Python smoke pass covered the extracted helper behavior; pytest was not available in this Windows shell because the bundled Python lacks pytest and the repo venv is WSL/Linux-only here.	2 weeks ago
Broque Thomas	698ecc99f0	Import history: Clear History button now sweeps stuck 'processing' rows ... Reported: Clear History button on the Import page left zombie rows behind. Every survivor showed "⧗ Processing" status from 2-9 days ago. Trace: `_record_in_progress` inserts a `status='processing'` row up-front so the UI can render the in-flight import while it runs; `_finalize_result` updates it to `completed`/`failed` when the import finishes. When the worker is killed mid-import (server restart, crash), the row never gets finalized — stays at `processing` forever. The clear-history endpoint's SQL `DELETE ... WHERE status IN (...)` listed every terminal status but omitted `processing`, so zombies survived every click. Fix: add `processing` to the delete list, but guard against nuking genuinely-live imports by intersecting against the worker's `_snapshot_active()` map — any folder hash currently registered in `_active_imports` is excluded from the delete via an `AND folder_hash NOT IN (...)` clause. `pending_review` deliberately left out so user still has to approve/reject those explicitly. One endpoint touched (`/api/auto-import/clear-completed` in web_server.py). No worker changes — guard reuses the existing `_snapshot_active()` method that the UI poller already calls. 5 new tests in `tests/imports/test_auto_import_clear_completed_endpoint.py`: - Zombie `processing` rows swept, live `processing` row preserved (folder_hash currently in `_active_imports` survives) - Response count matches actual delete count - Empty active-set branch (unparameterized DELETE) — pinned because an empty SQL `IN ()` would be a syntax error - Worker-unavailable returns 500 (pre-existing guard not regressed) - `pending_review` rows always survive — never auto-swept Full pytest sweep: 2758 passed (one pre-existing flaky timing test on `test_import_singles_parallel.py` failed under full-suite CPU load, passes in isolation in 2.95s — unrelated to this change).	2 weeks ago
Broque Thomas	3af2d34cee	Auto-import: fall through to other metadata sources when primary returns no match ... Discord report: 16 Bandcamp indie albums sat in staging because auto-import couldn't identify them, but the manual search bar at the bottom of the Import Music tab found the same albums fine. Trace: `_search_metadata_source` only queried `get_primary_source()` — single source, no fallback. Meanwhile `search_import_albums` (manual search bar) already iterated `get_source_priority(get_primary_source())` and broke on the first source with results. Asymmetric behavior, same album: manual worked, auto-import didn't. Fix: lift `_search_metadata_source` to use the same source-chain pattern. Try primary first; if it returns nothing OR scores below the 0.4 threshold, fall through to the next source in priority order. First source producing a strong-enough match wins. Result dict carries the `source` that actually matched (not the primary name) so downstream `_match_tracks` calls the right client. Defensive per-source try/except so a rate-limited or auth-failed source doesn't abort the chain. Unconfigured sources (client=None) silently skipped. Cin-shape lift: scoring math extracted to pure `_score_album_search_result` helper so the weight tweaks (album 50% / artist 20% / track-count 30%) are pinned at the function boundary, independent of the orchestrator (per-source iteration, exception containment, threshold check). Weight constants exposed at module level (`_ALBUM_NAME_WEIGHT`, `_ARTIST_NAME_WEIGHT`, `_TRACK_COUNT_WEIGHT`) — greppable, bumpable in one place. Pre-extraction these were magic numbers inline. 27 new tests: - 9 integration tests in `test_auto_import_multi_source_fallback.py`: primary-success path unchanged (no fallback fires, only primary client called), primary-empty falls through, primary-weak-score falls through, first fallback success stops the chain (no wasted API calls on remaining sources), all-sources-fail returns None, per-source exception contained, unconfigured-source skipped, result `source` field reflects winning source, `identification_confidence` from winning source. - 18 helper tests in `test_album_search_scoring.py`: weights sum to 1.0, album weight dominant (invariant pin), perfect-match returns 1.0, per-component contribution (album / artist / track-count), Bandcamp vs streaming track-count mismatch (7-files vs 4-tracks case still scores ~0.87 above threshold), zero-track-count and zero-file guards, huge-mismatch non-negative guard, list-of-strings artist shape, missing `.name` / `.artists` / `None` total_tracks edge cases. Backwards compatible: single-source users see no change — chain just has one entry. Existing test `test_search_metadata_source_extracts_artist_id_from_dict_artist` needed one extra patch line for `get_source_priority`. Full pytest sweep: 2754 passed.	2 weeks ago
Broque Thomas	402d851cac	Deezer search: drop advanced-syntax at endpoint, free-text + rerank wins ... Live-API verification revealed advanced-syntax queries hurt more than they help on this endpoint. Switching the import-modal Deezer search back to free-text + local rerank. # What live testing showed Hit Deezer's public API with both query forms for the issue #534 case (`Dirty White Boy` + `Foreigner`): **Free-text (`q=Dirty White Boy Foreigner`):** - Returns 21 results - Real Foreigner Head Games studio cut at #1 - Live versions at #2-10 - Karaoke / cover variants at #11-15 **Advanced (`q=track:"Dirty White Boy" artist:"Foreigner"`):** - Returns 12 results - "(2008 Remaster)" at #1 — canonical Head Games cut MISSING from top 8 entirely - Live + alt-album versions follow Advanced syntax DOES filter karaoke at the API level (none in the 12-result set vs. 5 at positions 11-15 in free-text), but it has its own ranking bias that surfaces remasters / "Best Of" cuts ahead of the canonical recording. Net regression for the user- facing goal. # Fix 1. Endpoint reverts to free-text query with local rerank applied. 2. Local rerank gains "remaster" / "remastered" / "reissue" patterns under VARIANT_TAG_PATTERNS (soft 0.4× penalty — user may want them but they shouldn't outrank the original). 3. Client kwarg support (`track=` / `artist=` / `album=`) preserved for future opt-in callers (e.g. exact-match flows where API- level filtering matters more than ranking). # Verified end-to-end against live Deezer API Re-ran the exact #534 case through the live API + new rerank. Top 15 results post-rerank: 1. Dirty White Boy — Foreigner — Head Games ← REAL CUT AT TOP 2-10. Various Live versions 11-15. Karaoke / cover / tribute variants ← BURIED Real Foreigner Head Games studio cut at #1, exactly the user's ask. # Tests - `test_relevance.py` — variant tag patterns extended; existing tests still pass (50 tests). - `test_search_match_endpoints.py::test_joins_track_and_artist_into_free_text_query` — replaces `test_passes_track_and_artist_as_kwargs`; verifies endpoint sends free-text join, NOT field-scoped kwargs (the prior test asserted the wrong direction now). - Karaoke-burying assertion at the endpoint still pins the user-visible behaviour. - Client kwarg path tests untouched (still pin advanced-syntax construction for future opt-in callers). # Verification - 75 relevance + endpoint + query tests pass - 2445 full suite passes - Ruff clean - Live Deezer API shows real cut at #1 post-rerank	2 weeks ago
Broque Thomas	1cc37081a6	Fix Deezer search relevance — issue #534 ... # Background User reported (#534) that the import-modal "Search for Match" dialog returned irrelevant results when Deezer was the metadata source. Searching `Dirty White Boy` + `Foreigner` returned 5+ karaoke / "originally performed by" / "in the style of" / "re-recorded" / tribute-band results ranked above the actual Foreigner studio cut from Head Games. User had to scroll past the junk every time, or fall back to iTunes search which is much slower. # Root cause — two layers 1. **Endpoint joined `track + artist` into free-text query.** `/api/deezer/search_tracks` was passing `q=Dirty White Boy Foreigner` to Deezer's `/search/track` API. Deezer fuzzy-matches that string across title / lyrics / artist / album / contributors and orders by global popularity — anything that appears across many compilations outranks the canonical recording. 2. **No local rerank.** None of the search-modal endpoints applied any post-filtering. Deezer's API order shipped straight to the user. # Fix — same architectural shape Cin would build ## Layer 1: field-scoped query at the client boundary `core/deezer_client.py::search_tracks()` now accepts optional `track`, `artist`, `album` kwargs. When provided, builds Deezer's advanced search syntax: `q=track:"X" artist:"Y" album:"Z"`. Massive relevance improvement because each term matches the right field instead of fuzzy-matching everywhere. Backward compat preserved: legacy free-text `query=` callers still work unchanged. Field-scoped path takes precedence when both are provided. Empty input fast-fails without an API call. Embedded double-quotes stripped (Deezer's syntax has no escape mechanism). ## Layer 2: provider-neutral relevance reranker New `core/metadata/relevance.py` module — pure-function rerank over the canonical `Track` dataclass. Composable scoring: - **Cover/karaoke patterns** (multiplier 0.05, effectively buries): matches "karaoke", "originally performed by", "in the style of", "made famous by", "tribute", "vocal version", "backing track", "cover version", "re-recorded", "cover by", etc. across title, album, AND artist fields. Catches the screenshot's exact junk: artist credits like "Pop Music Workshop" / "The Karaoke Channel" / "Foreigner Tribute Band". - **Variant tags** (multiplier 0.4): live / acoustic / demo / instrumental / remix / radio edit / club mix etc. — softer penalty since the user MAY want them. Skipped entirely when the expected_title contains the same tag (so searching "Track (Live)" still ranks Live versions first). - **Exact artist boost** (multiplier 1.5): primary artist exactly matches expected_artist after normalisation. Single strongest signal for "this is the canonical recording". - **Title + artist similarity** via SequenceMatcher (parentheticals + punctuation stripped before comparison). - **Album-type weighting**: album=1.0 > single/ep=0.85 > compilation=0.7. Compilations are more likely tribute / karaoke repackages. Each component is a standalone function so tests pin them individually without standing up the full pipeline. ## Wired at three search-modal endpoints - `/api/deezer/search_tracks` — uses both layers (field-scoped query + rerank). - `/api/itunes/search_tracks` — uses rerank only (iTunes API has no advanced-syntax search, but karaoke / cover variants still leak through and need the local penalty). - `/api/spotify/search_tracks` — already builds field-scoped `track:X artist:Y` query; rerank added as the consistency safety net so all three sources behave the same from the user's perspective. Other Deezer call sites (matching engine, watchlist scanner, auto-import single-track ID) deliberately not touched in this PR — they have their own elaborate scoring pipelines tuned to their specific contexts and aren't surfacing the user-reported issue. Per Cin: "don't refactor beyond what the task requires." # Tests 71 new tests across 3 files: - `tests/metadata/test_relevance.py` (50 tests) — every scoring component pinned individually + the issue #534 screenshot reproduced as a regression test (real Foreigner cut wins after rerank, karaoke variants drop to bottom). - `tests/metadata/test_deezer_search_query.py` (14 tests) — advanced-syntax query construction, field-scoped wiring at the client boundary, free-text path unchanged, kwargs win when ambiguous, limit clamping, cache key consistency. - `tests/imports/test_search_match_endpoints.py` (7 tests) — end-to-end through Flask test client: Deezer endpoint passes kwargs not joined query; karaoke buried at bottom for all three sources; legacy query param still works without rerank. # Verification - 2441 full suite passes (+71 from baseline 2370) - 0 failures (the prior watchdog flake fix held) - Ruff clean across all changed files - JS parses clean (`node -c webui/static/helper.js`) # Architectural standards followed - **Logic at the right boundary.** Query construction lives in the client (every caller benefits from one change). Rerank lives in a neutral module (`core/metadata/relevance.py`) over the canonical `Track` dataclass — works for any source, not Deezer- specific. - **Explicit > implicit.** Every scoring rule has its own named function. Pattern tables are module-level constants tests can introspect. - **Scope discipline.** Audited every Deezer search call site; fixed the user-reported one + the consistent siblings. Did NOT speculatively normalise every Deezer call across the codebase. - **Backward compat.** Free-text `query=` callers untouched. Kwargs added to existing client method signature with safe defaults. - **Tests pin contract at correct boundary.** Pure-function rerank tests don't mock anything; client-query tests stub at `_api_get`; endpoint tests run through the real Flask app.	2 weeks ago
Broque Thomas	abab663eb7	Auto-import: album duration = album total + conservative re-import UPDATE path ... Two pre-existing parity gaps in `record_soulsync_library_entry` that the prior parity commits left untouched. Both close real holes between auto-import writes and what the soulsync_client deep scan would have produced. # Gap 1: Album duration was the first-imported track's duration `record_soulsync_library_entry` is called once per track. The album INSERT only fires for the FIRST track of a new album (subsequent tracks find the album row already exists). The INSERT was passing `duration_ms` — `track_info["duration_ms"]` — as the album's `duration` column. That's the duration of one track, not the album total. Compare to `SoulSyncAlbum.duration` in soulsync_client which is `sum(t.duration for t in self._tracks)`. Fix: - Worker computes `album_total_duration_ms = sum(...)` across every matched track and threads it onto context as `album.duration_ms`. - side_effects reads that value (or falls back to the per-track duration for legacy non-auto-import callers) and writes it as the album row's `duration`. # Gap 2: Re-imports of the same artist/album were insert-only When the SELECT-by-id or SELECT-by-name found an existing soulsync artist or album row, the function skipped completely — no UPDATE path. Meant: artist genres / thumb / source-id reflected ONLY whatever the FIRST imported album supplied, never refreshing as more albums by that artist landed. Ten more imports later, the artist row still held whatever the first random import wrote. Conservative fix: when an existing row matches, run an UPDATE that fills only the columns whose current value is NULL or empty. Never overwrites populated values — protects manual edits + enrichment-worker writes the same way the scanner UPDATE path preserves enrichment columns. Implementation note: the empty-check happens in Python, NOT SQL. Initial pass tried `COALESCE(NULLIF(col, ''), NULLIF(col, 0), ?)` but SQLite's `NULLIF(text_col, 0)` returns the original text value instead of NULL — different types, no coercion. So the SQL-only conditional was unreliable on text columns. New helper does `SELECT cols FROM table WHERE id`, compares each column in Python, and emits UPDATE clauses only for the ones that need filling. Allowlist defense: f-string column names go through `_SOULSYNC_FILLABLE_COLUMNS` validation before interpolation. Misuse adding new columns without an allowlist update fails closed (logger.debug + skip). # Tests added (4) - `test_album_duration_uses_album_total_not_single_track` — album with single-track context carrying explicit `album.duration_ms = 2_500_000` writes 2_500_000 to the album row, not the per-track 200_000 fallback. - `test_re_import_fills_empty_artist_fields` — first import lands artist with empty thumb + empty genres; second import for same artist with thumb + genres present updates the existing row. - `test_re_import_does_not_clobber_populated_artist_fields` — first import writes rich genres + thumb; second import with worse / different metadata leaves the existing row untouched. - `test_re_import_fills_empty_source_id_when_missing` — first import had no source artist ID; second import does — fills the empty `spotify_artist_id` column on the existing row. # Verification - 10/10 side-effects tests pass (including 4 new + 4 from prior parity commit + 2 history/provenance) - 217 imports tests pass (no regression) - 2369 full suite passes (+4 from prior, +22 PR-total from baseline 2347) - 1 pre-existing flake (`test_watchdog_warns_about_stuck_workers`, passes in isolation, unrelated) - Ruff clean	2 weeks ago
Broque Thomas	f628009ab4	Auto-import: aggregate GENRE tags onto artists row + harden ISRC/MBID types ... Cin pre-review followup. Two small parity gaps the prior commits left open: # 1. Genre tags land on the standalone artists row `soulsync_client._scan_transfer` aggregates the GENRE tag across every track in an album and surfaces it on `SoulSyncAlbum.genres` (which the DatabaseUpdateWorker writes to the artists+albums row). Auto-import was hardcoding `'spotify_artist': {'genres': []}` so the imported artists row landed with empty genres — felt hollow compared to a Plex/Jellyfin scan, which both pull genres from their respective APIs. Fix: - `_read_file_tags` now reads the GENRE tag (mutagen easy mode handles MP3/FLAC/M4A consistently; some files carry multiple genres so it's always returned as a list). - `_process_matches` aggregates genres from each matched file's tags into a deduped insertion-order list. Dedup is case-insensitive but preserves original casing — so "Hip-Hop, Rap, Trap" reads naturally in the JSON column instead of "hip-hop, rap, trap". - Worker context's `spotify_artist['genres']` carries the aggregated list, which `record_soulsync_library_entry` already filters via `core.genre_filter.filter_genres` and writes to the artists row. # 2. Defensive str() cast for ISRC + MBID `_build_album_track_entry` already coerces ISRC + MBID to string today (via `str(isrc) if isrc else ''`). But if a future metadata-source client returns int / None for either ID, the worker would propagate the wrong type and side_effects.py's `.strip()` would AttributeError. Cheap insurance: explicit `str()` cast in the worker before assignment to track_info. Future-proofs against client drift. # Tests added (3, in test_auto_import_context_shape.py): - `test_context_aggregates_genres_from_track_tags` — multi-file album with overlapping genre lists produces deduped, insertion- ordered, original-case-preserved result. Stubs `_read_file_tags` with monkeypatch so we don't need real audio. - `test_context_genres_empty_when_no_tags` — files without GENRE tag → empty list. Standalone library write handles gracefully (genres column stays empty / NULL). - `test_context_isrc_mbid_coerced_to_string` — hostile types (int 12345678, None, int 999) coerced to safe strings before reaching track_info. # Verification - 14/14 context-shape tests pass (11 prior + 3 new) - 213 imports tests pass (no regression) - 2365 full suite passes (+3 from prior, +18 PR-total) - 1 pre-existing flake (`test_watchdog_warns_about_stuck_workers`, passes in isolation) - Ruff clean	2 weeks ago
Broque Thomas	ec7da89434	Auto-import: surface artist source-id from metadata search response ... Cin pre-review followup to the standalone library parity commit. The prior commit fixed `spotify_artist['id']` from the wrong copy-paste value (`identification['album_id']`) to read from `identification['artist_id']`, but the identification dict produced by `_search_metadata_source` and `_search_single_track` never set `artist_id` — both extracted artist NAME from the search response and discarded the source ID sitting right next to it. Net effect of the prior commit: artists row source-id stayed NULL, just for a more honest reason than before. Now properly extracted: - `_search_metadata_source` reads `best_result.artists[0]['id']` alongside the artist name and returns it on the identification dict as `artist_id`. - `_search_single_track` does the same for single-track identification. - `_identify_single`'s tag-based-confidence path forwards `result.get('artist_id')` so the artist source-id propagates even when high-confidence local tags override the search result's name. Result: identification dict now carries `artist_id` whenever the metadata source returned an artist with an ID. The worker context already plumbs it onto `spotify_artist['id']` and `spotify_album['artists'][0]['id']`, so the standalone library write finally populates `<source>_artist_id` on the artists row. Tests added (3, in `test_auto_import_context_shape.py`): - `test_context_artist_id_uses_identification_artist_id` — when the identification dict carries `artist_id`, context propagates it onto `spotify_artist['id']` AND `spotify_album['artists'][0]['id']`. Pins that the prior copy- paste bug (artist['id'] = album_id) doesn't return. - `test_context_artist_id_is_empty_when_identification_missing_it` — fallback case (filename-only identification): context gets empty string, NOT album_id. Honest failure mode. - `test_search_metadata_source_extracts_artist_id_from_dict_artist` — black-box test of `_search_metadata_source`: feed it a spotify-shaped result with `artists[0]['id']` and verify identification dict carries it forward. Verification: - 11/11 context-shape tests pass (8 prior + 3 new) - 210 imports tests pass (no regression) - 2362 full suite passes (+3 from prior commit, +15 PR-total) - 1 pre-existing flake (`test_watchdog_warns_about_stuck_workers`, passes in isolation) - Ruff clean	2 weeks ago
Broque Thomas	8493be207e	Auto-import: SoulSync standalone library writes server-quality rows ... # Background SoulSync standalone is meant to be a full replacement for Plex / Jellyfin / Navidrome — files imported via auto-import (or any other import path) should land in the database with the same field richness a media-server scan would write. They weren't. # Gaps fixed The auto-import worker built a context dict for each track and handed it to `_post_process_matched_download` (the same callback the regular download flow uses). That dict was missing three things downstream needed: 1. **No `source` field anywhere.** `record_soulsync_library_entry` reads `get_import_source(context)` to pick the source-aware ID columns (`spotify_track_id` / `deezer_id` / `itunes_track_id` / etc.) on the artists / albums / tracks rows. With no source, the resolver returned an empty string → `get_library_source_id_columns("")` returned an empty dict → the `UPDATE tracks SET <source>_id = ?` blocks were silently skipped. Result: every auto-imported track landed with NULL on every source-id column. Watchlist scans (which match by stable source IDs to detect "this track is already in library") couldn't recognise these rows and would re-download them on the next pass. 2. **No `_download_username='auto_import'`.** Both `record_library_history_download` and `record_download_provenance` default to "Soulseek" when no `username` is in the context. Every staging-folder import was being labelled as a Soulseek download in library history + provenance — false signal in the UI. 3. **No per-recording IDs (`isrc`, `musicbrainz_recording_id`) on track_info.** The Navidrome scanner already writes `musicbrainz_recording_id` directly to the tracks row when present. Picard-tagged libraries always carry MBID; metadata sources (Spotify via MusicBrainz enrichment, Deezer, etc.) carry ISRC. Auto-import had access to both via the metadata-source response but didn't propagate them — so the soulsync row went in with NULL on both columns. # Changes **`core/auto_import_worker.py` — `_process_matches`:** - Top-level `'source': source` (from `identification['source']`) - `'_download_username': 'auto_import'` - `track_info['isrc']`, `track_info['musicbrainz_recording_id']` — pulled from the per-track payload returned by the metadata source - `track_info['album_id']` — back-reference so source-aware ID resolution works on sources whose API nests album under `track.album.id` rather than `track.album_id` - `spotify_artist['id']` now correctly carries the artist's source ID (was `identification['album_id']`, a copy-paste bug from the original implementation that made artist-id resolution fall back to fuzzy matching) - `spotify_album['artists'][0]['id']` carries artist source ID for the same resolution path **`core/imports/side_effects.py`:** - `record_library_history_download` source_map: add `"auto_import": "Auto-Import"` — tags imported tracks correctly - `record_download_provenance` source_service: add `"auto_import": "auto_import"` — provenance shows real source - `record_soulsync_library_entry` track INSERT: now includes `musicbrainz_recording_id` + `isrc` columns (matches `insert_or_update_media_track`'s shape for Navidrome / Plex / Jellyfin scans). Both default to NULL when not present. # Behavior preserved - Files still land in the same library template path (no path-build change) - Other media-server flows (Plex / Jellyfin / Navidrome users) unaffected — `record_soulsync_library_entry` still gates on `get_active_media_server() == "soulsync"`. Auto-import on those servers continues to drop the file in the library folder + emits `batch_complete` for the scan-trigger automation, same as before. - Direct downloads (search → Download button) unaffected — they already passed `source` + `username` correctly. # Tests added `tests/imports/test_auto_import_context_shape.py` (8 tests, new file): - Worker context carries `source` for every metadata source (parametrised across spotify / deezer / itunes / discogs) - `_download_username='auto_import'` set unconditionally - ISRC + MBID propagate from track payload to track_info when present - ISRC + MBID default to empty string when absent (downstream normalises to NULL at write time) - track_info includes album-id back-reference `tests/imports/test_import_side_effects.py` (4 new tests + 2 schema column adds): - `record_soulsync_library_entry` writes mbid + isrc columns when present in track_info - Deezer source maps to deezer_id column (regression case for source-aware column resolver) - `record_library_history_download` labels `_download_username= 'auto_import'` as "Auto-Import" not "Soulseek" - `record_download_provenance` registers source_service as "auto_import" not "soulseek" # Verification - 8/8 new context-shape tests pass - 6/6 side-effects tests pass (4 new + 2 existing) - 207 imports tests pass - 2359 full suite passes (+12 from baseline 2347, no regressions) - 1 pre-existing flake (`test_watchdog_warns_about_stuck_workers`, passes in isolation, unrelated to this change) - Ruff clean	2 weeks ago
Broque Thomas	8a6ee7a2c7	Auto-import: bounded ThreadPoolExecutor + per-candidate UI state isolation ... # Concurrency model Pre-refactor concurrency was emergent + unbounded: - The worker's `_run` thread called `_scan_cycle` every 60s, processing candidates synchronously in a for-loop. - The `/api/auto-import/scan-now` endpoint spawned a fresh `threading.Thread(target=_scan_cycle)` per click — extra parallel scan cycles on top of the timer. - Multiple "Scan Now" clicks during in-flight processing → multiple threads racing on `_processing_paths` / `_folder_snapshots` state, no upper bound on concurrent scanners. - `stop()` didn't wait for in-flight processing — could leave file moves / tag writes / DB inserts mid-flight. Refactor to the pattern Cin uses elsewhere (`missing_download_executor`, `sync_executor`, `import_singles_executor` all use `ThreadPoolExecutor(max_workers=3, thread_name_prefix=...)`): - **One scan thread** — both timer + manual triggers go through `trigger_scan()`, gated by a non-blocking `_scan_lock`. Duplicate triggers no-op instead of stacking parallel scanners. - **Bounded executor** — `ThreadPoolExecutor` (default 3 workers, configurable via `auto_import.max_workers`) runs per-candidate work. Each candidate runs to completion in its own pool thread; up to N candidates run in parallel. - `_scan_and_submit()` is fast — just enumeration + executor submit, returns immediately, doesn't block on per-candidate work. - `_process_one_candidate(candidate)` holds the per-candidate logic identical to the old for-loop body, lifted into a method so the pool can run multiple instances concurrently. - `_submitted_hashes` set + lock dedupes candidates across the timer + manual triggers so a candidate already queued / running doesn't get re-submitted. - `stop()` calls `executor.shutdown(wait=True)` — clean shutdown, no orphaned file ops. # Per-candidate UI state isolation The executor refactor opened two concurrency holes that the old sequential model masked. Both fixed in this commit: 1. **Scalar UI fields stomped across pool workers.** Pre-refactor `_current_folder` / `_current_status` / `_current_track_*` were safe under the sequential model — only one candidate processed at a time, so the fields tracked the in-flight one. With three pool workers writing the same fields, the polling UI saw garbage like "Processing AlbumA, track 7/14: SongFromAlbumB". Replaced with `_active_imports: Dict[hash, _ActiveImport]` keyed on folder_hash, gated by `_active_lock`. Each pool worker owns its own entry. Helpers `_register_active` / `_update_active` / `_unregister_active` / `_snapshot_active` are the only API. 2. **Stats counters not thread-safe.** `self._stats[k] += 1` is read-modify-write — under load, parallel pool workers drop increments. New `_stats_lock` + `_bump_stat()` helper wraps every mutation. `get_status()` reads under the same lock and returns a copy. # Endpoint change `/api/auto-import/scan-now` no longer spawns its own scan thread — calls `auto_import_worker.trigger_scan()` (which routes through the shared lock + executor). Multiple clicks while a scan is in flight no-op deterministically. Endpoint still wraps the call in a daemon thread so the HTTP response returns immediately even if the staging walk is slow. # Backward compat The scalar `_current_folder` / `_current_status` / `_current_track_*` fields are preserved as **read-only properties** that resolve to the FIRST active import. The existing `get_status()` payload still includes those fields populated from the first entry — single-import UIs (and the test fixture) keep working unchanged. New `active_imports` array exposes the full multi-candidate state for parallel-aware UIs. # Behavior preserved - Per-candidate identify / match / process logic byte-identical - Live-progress state preserved (per candidate now) - Stability gate / already-processed dedup preserved - `_record_in_progress` / `_finalize_result` UI rows preserved - Tag-based loose-file grouping unchanged # Behavior changes - Multiple albums process IN PARALLEL up to `max_workers` - "Scan Now" while scan in progress no-ops (was: spawned another) - `stop()` waits for in-flight pool work via `shutdown(wait=True)` - Auto-import card now lists each in-flight album (one line per active import) instead of a single shared progress line # UI `webui/static/stats-automations.js`: - Progress widget reads `active_imports` array, renders one line per in-flight album with per-candidate status / track index - Falls back to the legacy summary line when payload doesn't carry `active_imports` (older backend) - Per-row "live processing" lookup now matches by `folder_hash` through the array instead of by `folder_name` against scalars # Tests added (`tests/imports/test_auto_import_executor.py`) - Pool config: default max_workers=3, configurable via constructor + via `auto_import.max_workers` config, floors at 1 - Scan lock: 5 concurrent `trigger_scan()` calls run only 1 scan while lock held; releases properly so subsequent triggers run - Executor dispatch: 5 candidates → 5 process calls via the pool - Bounded parallelism: max_workers=3 caps at 3 concurrent; max_workers=2 caps at 2 - Cross-trigger dedup: candidate submitted in scan A doesn't get re-submitted by scan B while still in-flight - Graceful shutdown: `stop()` blocks until in-flight pool work finishes - Per-candidate state isolation: 2 parallel workers updating their own candidate state don't interfere — each candidate's track_index / track_name / folder_name reads back exactly as written for that hash - `get_status()` returns coherent `active_imports` array with one entry per in-flight candidate; aggregate top-level `current_status` is 'processing' when any entry is processing - Unregister removes only that candidate, others stay visible - Stats counter thread-safety: 1000 parallel bumps land at 1000 (the read-modify-write race regresses without the lock) - `get_status()` stats snapshot is a copy, not a live reference # Verification - 17 new tests pass (executor + state isolation) - 2347 full suite passes (1 pre-existing flaky test — `test_watchdog_warns_about_stuck_workers` — passes in isolation, unrelated) - Ruff clean	2 weeks ago
Broque Thomas	e11786ee40	Auto-import matching: fix Deezer source classification + bump tolerance ... User report: all 6 staging candidates failing with "Could not match tracks to album tracklist" despite identification correctly resolving each album. 18 properly-tagged Chris Brown F.A.M.E. tracks, 21 properly-tagged Mr. Morale tracks, etc. — every match attempt rejected by the duration sanity gate. Root cause: I had Deezer in `_SECONDS_DURATION_SOURCES`, assuming Deezer's `duration` field was raw seconds (which the API returns). But `DeezerClient.get_album_tracks` already converts seconds → ms INTERNALLY (`'duration_ms': item.get('duration', 0) * 1000`) before the value reaches the matcher. My helper saw `source='deezer'` → multiplied by 1000 again → 255000 ms became 255,000,000 ms (70 hours). Every track-file pair failed the gate by a factor of 1000×. Diagnostic chain that got me there: 1. Added `[Album Matching] No matches: X files, Y tracks, Z duration-rejected, W below threshold` summary log so future "0 matches" reports surface the rejection reason. 2. Fixed the helper's logger from `logging.getLogger(__name__)` (which resolves outside the soulsync handler tree → invisible in app.log) to `get_logger("imports.album_matching")` (under the namespace the file handler watches). 3. Added per-rejection-type diagnostic showing actual file vs track duration values + raw track keys + source. That third diagnostic surfaced `track 'United In Grief' resolved=255000000 (raw duration_ms=255000, raw duration=None, source='deezer')` — making the bug obvious. Fixes: - Moved Deezer from `_SECONDS_DURATION_SOURCES` to `_MS_DURATION_SOURCES`. Comment documents WHY (the client converts before returning) so a future reader doesn't "fix" the classification back the wrong way. - Bumped `DURATION_TOLERANCE_MS` from 3000 → 10000 (3s → 10s) to match Picard ~7s / Beets ~10-15s / Plex ~10s industry baselines. 3s was a defensive copy of the post-download integrity check threshold but that's a different problem (catching truncated downloads, not identifying recordings across remasters/encodings). - `_track_duration_ms` magnitude heuristic kept as fallback for unknown / missing source (mocked test data without `source` field). - Added `Match aborted` warnings at the three earlier silent return points in `_match_tracks` (no client, no album_data, no tracks) so future "Could not match" reports show WHICH step bailed. - Added per-run diagnostic in `match_files_to_tracks` that logs the first duration rejection's actual values — surfaces unit mismatches + drift problems without spamming N×M lines per run. Test changes: - `test_deezer_seconds_duration_converted_to_ms` renamed + rewritten as `test_deezer_already_normalised_to_ms_by_client` to pin the actual contract (matcher receives ms from the Deezer client, takes as-is). - `test_track_duration_source_aware_dispatch` updated — Deezer test case now uses ms input + expects ms output. - New `test_raw_deezer_seconds_falls_back_to_magnitude_heuristic` pins the rare edge case where raw Deezer items WITHOUT `source` reach the matcher (no client conversion path) — heuristic catches it. Verification: - 179 import tests pass after changes - Live test: all 6 user staging candidates now matching at 95-100% confidence - Multi-disc Mr. Morale lands with proper Disc 1 / Disc 2 / Disc 3 folder structure - Picard-tagged libraries hit MBID fast paths (verified earlier) - Tracks process in parallel via the existing scan-now thread spawn (next commit refactors this to a proper bounded executor)	2 weeks ago
Broque Thomas	a478747a89	Auto-import: dedup on folder_hash, not path — fixes silent-skip bug ... User reported nothing happening on a chaotic staging root despite 6 candidates being detected. Logs showed "Processing folder" for 3 of 6 — the other 3 were silently skipped. Root cause: The previous commit (`a9a6168`) introduced loose-file grouping — multiple `FolderCandidate` objects can now share a `path` (each album group at the staging root has the same parent directory but its own audio_files + folder_hash). But two pieces of dedup machinery still keyed on `path`: - `_processing_hashes` (was `_processing_paths`) — runtime set of in-flight candidates. Path-keyed → first sibling marks the path, second + third siblings hit "already in flight" and skip. - `_folder_snapshots` — mtime cache for stability check. Path-keyed → siblings overwrite each other's mtimes, stability check returns unreliable results for whichever sibling lost the write race. Both kept track of an attribute that was previously unique-per-path (one candidate per directory) but my refactor broke that invariant without updating the dedup keys. Net effect: only the first candidate per directory ever got processed in a chaotic-root scenario. Fix: - Renamed `_processing_paths` → `_processing_hashes` set, keyed on `candidate.folder_hash`. Hash is unique per candidate by construction (different audio_files lists hash differently). - `_folder_snapshots` retyped + rekeyed to `folder_hash`. Siblings no longer overwrite each other's mtime tracking. - Both touched in lockstep — comments document why path-keyed dedup breaks for sibling candidates. Test added (`test_sibling_candidates_have_unique_folder_hashes`): verifies 3-album loose root produces 3 candidates with distinct folder_hashes. If a future change breaks the invariant, the test fails before the silent-skip regression ships. Verification: - 178 imports tests pass (8 new this commit + 170 pre-existing this branch) - Ruff clean - Still scoped to import flow	2 weeks ago
Broque Thomas	a9a6168568	Auto-import scanner: group loose files by album + always recurse subfolders ... Two related bugs in `AutoImportWorker._scan_directory` surfaced during real-world testing of the chaotic-staging case (user dropped loose tracks from multiple albums at staging root, alongside intact album subfolders): Bug 1 — Loose files bundled into one fake "album" When loose audio files existed at a level, the scanner built ONE FolderCandidate from all of them regardless of their album tags. On a chaotic staging root with tracks from 3+ different albums, the identifier picked the most-common album tag and the matcher left every other album's tracks unmatched (or mis-attributed via filename + position guessing). Bug 2 — Subfolders silently ignored when root has loose files The scanner only recursed into non-disc subfolders when there were NO loose files at the parent level. So a layout like: Staging/ loose1.flac (processed via the loose-files path) Other Album Folder/ (silently ignored — never scanned) would skip the album subfolders entirely. Common pattern when a user moves a few tracks out of an album folder while leaving the rest of the parent album folder intact, OR when other album folders sit alongside a partially-extracted album. Fix: `_build_loose_file_candidates` (new method) reads each loose file's `album` tag and groups by normalised album name. Each group becomes its own FolderCandidate so a chaotic staging root produces one candidate per album — identifier + matcher run cleanly per album. Untagged loose files become individual single candidates. Disc folders at the same level attach to whichever loose-file group's album tag matches the disc-folder tracks; standalone disc folders (no matching loose group) get their own multi-disc candidate. The scanner now ALSO always recurses into non-disc subdirectories, even when the current level has loose files. So album subfolders sitting beside loose tracks get processed independently in their own recursive scan. Behavior preservation: - Single-album loose-files staging (every file shares one album tag, no parallel disc folders) → one FolderCandidate, identical to pre-fix behavior. Pinned by `test_single_album_loose_files_still_one_candidate`. - Disc-only directory (no loose files, only Disc 1/Disc 2 subdirs) → one multi-disc FolderCandidate, identical to pre-fix. Pinned by `test_disc_only_directory_still_works`. 7 new tests in `tests/imports/test_auto_import_scanner_grouping.py`: - Multiple-album loose root → multiple candidates - Untagged loose files → individual singles - Single-album loose-files regression guard - Subfolders recursed even when root has loose files - Disc folder attaches to matching loose group by album tag - Disc folder with no matching loose group → standalone candidate - Disc-only directory regression guard All write real FLACs via mutagen + exercise `_scan_directory` end-to-end (no mocking the tag reader — proves the production read path works). Verification: - 7 new tests pass - 2328 full suite passes (+7 new), 1 pre-existing flaky timing test unrelated to this PR - Ruff clean - All changes still scoped to import flow — download flow byte- identical	2 weeks ago
Broque Thomas	f2cd95e0f1	Auto-import polish: real-file tag reader test, source-aware duration, pin consolation ... Cin-pass on the MBID/ISRC fast-paths + duration-gate work. Three small but real gaps closed. Gap 1 — Real-file tag reader integration test (tests/imports/test_auto_import_tag_reader_real_files.py, 6 tests): The matcher unit tests use dict fixtures, which prove the algorithm handles the right shapes once tags are read. They DON'T prove the tag reader itself extracts the right values from real files. Mutagen's easy-mode key normalisation (across FLAC / MP3 / M4A) is the exact spot a future mutagen version could silently drift and break the fast paths in production while every unit test stays green. These tests write real FLAC files via mutagen (using the same `_make_minimal_flac` pattern from `test_album_mbid_consistency.py`) and assert `_read_file_tags` extracts: - Picard's `MUSICBRAINZ_TRACKID` (lowercase normalisation in reader) - `ISRC` (uppercase normalisation in reader; matcher strips formatting at compare time) - "track/total" parsing (TRACKNUMBER='5/12' → 5) - Duration via `audio.info.length` from synthesised STREAMINFO - Graceful empty-default return for tagless files - Graceful empty-default return for invalid audio (not a crash) Acknowledged gap (carried forward): MP3 + M4A integration coverage not added — mutagen docs say easy-mode normalisation is identical across all three formats, but only FLAC is pinned here. Followup candidate. Gap 2 — Source-aware duration dispatch (core/imports/album_matching.py, 4 tests in test_album_matching_exact_id.py): The previous `_track_duration_ms` helper used a magnitude heuristic ("anything below 30000 is seconds, convert × 1000") to decide whether a track's duration was in seconds or ms. That worked for typical tracks but had a real edge case: an actual sub-30-second Spotify track (intros, interludes, skits) would be detected as seconds and converted to 8.5 hours, breaking the duration sanity gate. Replaced with deterministic source-aware dispatch: - Spotify / iTunes / Qobuz / HiFi / Hydrabase → ms (canonical) - Deezer / Discogs / MusicBrainz → seconds, × 1000 - Tidal classified as ms (album-tracks endpoint convention; flagged in code comment as needing real-world verification — defensive if wrong) - Magnitude heuristic kept as fallback for unknown / missing source (mocked test data without source field) Tests pin all four paths: confirmed-ms source, confirmed-seconds source, unknown source falls back to heuristic, and the regression case (sub-30s real track on a known-ms source — must not be × 1000-converted). Gap 3 — Cross-disc consolation rationale (tests/imports/test_album_matching_helper.py, 1 test): The `CROSS_DISC_POSITION_WEIGHT = 0.05` magic number had no test proving it was load-bearing. Anyone could have set it to 0 thinking "strict matching is better" without realising it would silently break a real scenario. New test (`test_cross_disc_consolation_is_load_bearing_for_imperfect_titles`) constructs the exact case the consolation exists for: file has the right title spelling but the metadata source returns a slightly- different version (e.g. "Auntie Diaries" file vs "Auntie Diaries (Remix)" track), AND the file's disc tag is wrong while the track number agrees. Title sim ~0.78 × 0.45 = ~0.35 (below MATCH_THRESHOLD 0.4). Without the 5% consolation → file goes unmatched. With it → ~0.40, just clears. The test doesn't justify "why 0.05 specifically" — that's still a tuned knob, not a measured value. But it forces a deliberate decision if someone wants to drop it: failing this test gives them the "you broke imperfect-title cross-disc matching" message explicitly. Verification: - 10 new tests across 3 files, all pass - 35 album-matching tests total now (including pre-existing 17 + 18 fast-path) - Full suite: 2321 passed, 1 pre-existing flaky timing test (`test_watchdog_warns_about_stuck_workers` — passes in isolation, fails only in full-suite runs, unrelated to this PR) - Ruff clean - All changes still scoped to import flow — download flow byte- identical (verified by grep on every changed file)	2 weeks ago
Broque Thomas	3246490800	Auto-import: MBID/ISRC fast paths + duration sanity gate ... Brings the auto-import matcher to picard / beets / roon parity by reaching for the existing AcoustID-grade infrastructure (typed Album foundation, integrity check thresholds) and layering id-based exact matches on top of the fuzzy scorer. Picard-tagged libraries now land every track with full confidence on the first pass. Three layered phases in `core/imports/album_matching.match_files_to_tracks`: 1. **MBID exact match** — file has `musicbrainz_trackid` tag, source returns the same id → instant pair, full confidence, no fuzzy scoring. Picard's primary identifier; per-recording. 2. **ISRC exact match** — file has `isrc` tag, source returns the same id → same fast-path, slightly lower priority than mbid (isrc can be shared across remasters). Both ids normalised before compare (uppercase + strip dashes/spaces for isrc, lowercase for mbid). 3. **Duration sanity gate** — files in the fuzzy phase whose audio length differs from the candidate track's duration by more than `DURATION_TOLERANCE_MS` (3s, matching the post-download integrity check) are rejected before scoring runs. Defends against the cross-disc / cross-release / wrong-edit problem the integrity check used to catch only AFTER the file had already been moved + tagged + db-inserted. Tag reader (`_read_file_tags`) extended: - Reads `isrc` (uppercased, strip / / spaces normalisation deferred to matcher) - Reads `musicbrainz_trackid` as `mbid` (lowercased) - Reads `audio.info.length` and converts to `duration_ms` to match the metadata-source convention Metadata-source layer (`_build_album_track_entry`) extended: - Propagates `isrc` from top-level OR `external_ids.isrc` (spotify shape — would otherwise be stripped before reaching the matcher) - Propagates `musicbrainz_id` from top-level OR `external_ids.mbid` / `external_ids.musicbrainz` - Without this layer, fast paths would silently never fire in production even though unit tests pass — pinned by `test_album_track_entry_propagates_isrc_and_mbid_from_source` 18 new tests in `tests/imports/test_album_matching_exact_id.py`: - Direct: `find_exact_id_matches` with mbid, isrc, isrc normalisation, mbid > isrc priority, spotify-shape `external_ids.isrc`, no-id empty result, file-used-at-most-once - Direct: `duration_sanity_ok` within / outside tolerance, missing durations defer - End-to-end via `match_files_to_tracks`: mbid match short-circuits fuzzy scoring, id-matched files excluded from fuzzy phase, duration gate rejects wrong-disc collisions in fuzzy phase, normal matches pass through the gate, missing durations fall through, deezer seconds-vs-ms conversion, full picard-tagged 10-track album via mbid only - Production-shape: `_build_album_track_entry` propagates isrc + mbid from spotify-shape (`external_ids.isrc`) AND itunes-shape (top- level `isrc`) Verification: - 35 album-matching tests pass total (17 helper + 18 fast-path) - 23 multi-disc tests still pass after the extension (additive) - Full suite: 2311 passed (+18 new), 1 pre-existing flaky timing test failure (`test_watchdog_warns_about_stuck_workers` — passes in isolation, fails only in full-suite runs, unrelated to this PR) - Ruff clean For users: - Picard / Beets / Mp3Tag-tagged libraries (anyone who's organised their music) get instant perfect-confidence matches every time. - Soulseek-tagged downloads (which usually carry isrc when sourced via metadata-aware soulseekers) get the fast path too. - Naively-named files with no useful tags fall through to the improved fuzzy + duration-gated path — same correctness as before for the common case, much harder for the matcher to confidently pair the wrong file. - One step closer to standalone-DB feature parity with plex / jellyfin / navidrome scanners. Acoustid fingerprint fallback (for files with NO useful tags AND no MBID/ISRC) is the next followup PR.	3 weeks ago
Broque Thomas	f9f74ac511	Lift auto-import matching to testable helper + pin contracts ... Cin-pass on the #524 + multi-disc fixes. Pre-merge polish. Lifts: `core/imports/album_matching.py` `AutoImportWorker._match_tracks` was a 100+-line method buried in a 1400-line class. Testing it required monkey-patching `_read_file_tags` + mocking the metadata client just to exercise the matching algorithm. Per Cin's "lift logic out of monolithic classes" pattern (same shape as the album-info builders / discography / quality scanner lifts), moved the dedup + scoring into `core/imports/album_matching.py` as pure functions over already-fetched data. Helper exposes: - Constants for every match weight (TITLE_WEIGHT, ARTIST_WEIGHT, POSITION_WEIGHT, NEAR_POSITION_WEIGHT, CROSS_DISC_POSITION_WEIGHT, ALBUM_WEIGHT, MATCH_THRESHOLD). Magic numbers killed. - `dedupe_files_by_position(audio_files, file_tags, *, quality_rank)` — position-keyed quality dedup. - `score_file_against_track(file_path, file_tags, track, *, target_album, similarity)` — pure per-(file, track) scorer. - `match_files_to_tracks(audio_files, file_tags, tracks, *, target_album, similarity, quality_rank)` — full matching with greedy best-per-track + first-come-first-serve over deduped files. Worker shrinks from 100 lines of inline algorithm to 8 lines that fetch tags + delegate to the helper. Tests added (26 new across 3 files): `tests/imports/test_album_matching_helper.py` (19 tests): - Constants pin: weights sum to 1.0, threshold above position-only - `dedupe_files_by_position`: quality wins, cross-disc preserved, tag-less files passed through, first-wins on equal quality - `score_file_against_track`: perfect-agreement = 1.0, position needs both disc+track, near-position only same-disc, missing artist tags handled, disc field aliases (Spotify/Deezer/iTunes), filename fallback when title tag missing - `match_files_to_tracks`: happy path, file used at-most-once, below-threshold left unmatched - Edge case Cin would flag: tag-less file with strong filename title matches multi-disc album track via title alone (perfect-name scenario works); tag-less file with weak filename title against multi-disc API correctly stays unmatched (the behavior delta from the disc-aware fix — pinned so future readers see it's intentional) `tests/test_import_album_match_endpoint.py` (3 tests): - Backend warning fires when source missing from match POST - No warning fires on the legit path (catches noisy-warning regression) - Endpoint actually forwards source/name/artist to the payload builder (catches "logging the right warning but doing the wrong lookup" regression) `tests/test_import_page_album_lookup_pattern.py` (4 tests): - Source-text guard for the import-page #524 fix in stats-automations.js. Until the file is modularized enough for a behavioral JS test (under the existing tests/static/*.mjs pattern), regex-based assertions pin: the `_albumLookup` field exists, the click handler reads from it, both card renderers populate it before emitting onclick, and the cache stores `source` per entry. Caveat documented in the test module docstring. Verification: - All 26 new tests pass. - Existing multi-disc tests (test_auto_import_multi_disc_matching.py) still pass after the lift — proves the helper is behavior-equivalent to the inline implementation it replaced. - Full suite: 2293 passed, 1 flaky-timing failure (test_library_reorganize_orchestrator.py::test_watchdog_warns_about_stuck_workers — passes in isolation, fails only in full-suite runs, pre-existing, unrelated to this PR). - Ruff clean. Notes for the reviewer: - The frontend stats-automations.js JS test is structural-only. Behavioral JS testing for that file requires modularizing the ~7k-line monolith first — out of scope for this fix. - The cross-disc 5% consolation bonus is a small behavior change for users with weak/missing tag info on multi-disc albums. Pinned explicitly in `test_tagless_file_with_weak_title_unmatched_in_multidisc` so the trade-off is visible: correct multi-disc matching wins over optimistic position-only matching that produced wrong-disc files.	3 weeks ago
Broque Thomas	c03edc3cb4	Auto-import: respect disc_number in dedup + match scoring ... Caught while live-testing the #524 fix with kendrick lamar mr morale & the big steppers (3 discs). User dropped discs 1+2 loose in staging root + disc 3 in its own folder, every file perfectly tagged with disc_number/track_number/title — only 9 tracks ended up in the library, the rest got integrity-rejected and quarantined. Two related bugs in `AutoImportWorker._match_tracks`: 1. **Quality dedup keyed on track_number alone.** The dedup loop kept `seen_track_nums[track_number] = file` and dropped any later file with the same number, treating it as a quality duplicate. On a multi-disc release where every disc has tracks 1..N, that collapses the album to one disc's worth of files BEFORE the matcher runs. User's 18 loose disc-1+disc-2 files reduced to 9 before any title/disc info was even consulted. 2. **Match scoring ignored disc_number.** The 30% track-number bonus fired whenever `ft[track_number] == track_num` regardless of disc. File with tag (disc=2, track=6, "Auntie Diaries", 281s) got the full bonus matching API track (disc=1, track=6, "Rich Interlude", 103s) — wrong file → wrong destination → integrity check correctly rejected and quarantined the file. Same for tracks 7, 8, 9. Fix: - Dedup keys on `(disc_number, track_number)` tuples — multi-disc files with parallel numbering all survive. - Match scoring's 30% bonus only when BOTH disc AND track agree. Cross-disc same-track-number collisions get a small 5% consolation bonus so title similarity has to carry the match (covers cases where tag disc info is missing or wrong). - API track disc_number read from `disc_number` (Spotify) / `disk_number` (Deezer) / `discNumber` (iTunes) defaulting to 1. 4 new pinning tests in `tests/imports/test_auto_import_multi_disc_matching.py`: - 18-file 2-disc regression case (dedup preserves all) - (disc=2, track=6) file matches API (disc=2, track=6) track, not the disc-1 same-numbered track - Single-disc albums still match normally (no regression) - Quality dedup within a single (disc, track) position still picks higher-quality format (.flac over .mp3) Verification: - 2268 full pytest suite passes (+4 new), 1 skipped, 0 failed - Ruff clean Same branch as the #524 fix because both surfaced from the same import session — easier reviewer context if they ship together.	3 weeks ago
Broque Thomas	967c7f7c0a	Migrate album-info builders to typed Album path ... Steps 2+3 of typed metadata migration. Two album-info builders now route through Album.from_<source>_dict() when caller passes a known source: - _build_album_info (album-tracks lookups) - _build_single_import_context_payload (single-track import context) Legacy duck-typing stays as fallback for unknown source, non-dict input, or converter errors. Pure additive — existing callers without source kwarg unchanged.	3 weeks ago
Broque Thomas	2ab460f5c4	Add Library Disk Usage card to System Statistics ... Discord request (Samuel [KC]): show how much disk space the library takes on the Stats page. Implementation piggybacks on the existing deep scan — Plex/Jellyfin/Navidrome all return file size in their track API responses, so we read it during the deep scan and store it on the tracks row. Aggregation is then a single SQL query — no filesystem walk, no extra I/O during the scan, no separate stat job. SoulSync standalone gets size from os.path.getsize at insert time (different code path; the file is local when we write the row). Schema (`database/music_database.py`): - New `file_size INTEGER` column on `tracks`. Migration uses the established `try SELECT, except ALTER TABLE ADD COLUMN` pattern. Idempotent; safe on existing installs. NULL on legacy rows so they don't contribute to totals until next deep scan refreshes. - Added the column to the canonical CREATE TABLE so fresh installs get it without going through the migration path. Track-object plumbing: - `core/jellyfin_client.py` — JellyfinTrack reads MediaSources[0].Size alongside existing Bitrate read. None when 0 / missing. - `core/navidrome_client.py` — NavidromeTrack reads `size` from the Subsonic song object (int coercion + None on parse fail). - `core/soulsync_client.py` — SoulSyncTrack does os.path.getsize (only "server" where size has to come from disk). - Plex needs no client-side change: track.media[0].parts[0].size is read directly inside insert_or_update_media_track. Persistence — TWO separate insert paths: (a) `database/music_database.py:insert_or_update_media_track` — Plex/Jellyfin/Navidrome flows. Reads file_size from Plex's MediaPart OR `track_obj.file_size` wrapper attribute (defensive Plex-attr-not-present check + > 0 type guard). INSERT writes the new column. UPDATE uses COALESCE(?, file_size) so a None from the server on a re-sync (rare Jellyfin Size omission) doesn't blank an existing value. Pinned via test. (b) `core/imports/side_effects.py:record_soulsync_library_entry` — SoulSync standalone flow. Completely separate code path: the standalone deep scan moves files to staging for auto-import rather than calling insert_or_update_media_track. After the auto-import processes them, side_effects writes the tracks row directly. Reads file_size via os.path.getsize(final_path) at insert time (file is local) and includes it in the INSERT column list. SoulSync only does INSERT-if-not-exists (no UPDATE path), so no COALESCE concern. Aggregator (`database/music_database.py:get_library_disk_usage`): - SELECT COALESCE(SUM(file_size), 0), COUNT(file_size), COUNT(*) - COUNT(file_size) for the totals. - Per-format breakdown done in Python via os.path.splitext over (file_path, file_size) rows — sidesteps SQLite's first-vs-last-dot ambiguity for paths like /music/Kendrick/M.A.A.D City/01.flac. - Defensive: skips empty paths, paths without extension, and implausibly long extensions (>6 chars). Returns the full empty-shape dict (NOT a partial / undefined) when the column doesn't exist or queries fail, so the UI's `if (!data.has_data)` branch handles fresh installs cleanly. API + UI: - `core/stats/queries.py` — thin pass-through get_library_disk_usage matching the existing query-helper convention. - `web_server.py` — new /api/stats/library-disk-usage endpoint mirroring the /api/stats/db-storage pattern. - `webui/index.html` — new card in System Statistics above the Database Storage card. - `webui/static/stats-automations.js` — _loadLibraryDiskUsage + _renderLibraryDiskUsage. Empty state: "Run a Deep Scan to populate (X tracks pending)". Partial: "X measured (+Y pending)". Full: total + format bars proportional to the largest format. - `webui/static/style.css` — .stats-disk-* styled to match the Database Storage card. Backward compatibility: - Migration is additive; existing rows get NULL file_size; the empty-shape return from the aggregator means the UI renders cleanly without errors before any deep scan runs. - Old installs upgrading will see "Run a Deep Scan to populate (N tracks pending)". Running their next deep scan fills sizes — the existing scan flow doesn't need any changes, just consumes the new track-wrapper attribute. Tests: - `tests/test_library_disk_usage.py` — 13 cases covering schema migration, NULL defaults on legacy inserts, fresh-install empty shape, summing with mixed NULL/known sizes, per-format breakdown, mixed-case extensions, paths with album-name dots, missing extensions, empty file_path, implausibly long extensions, JellyfinTrack.file_size persistence via insert_or_update_media_track, COALESCE preservation on null re-sync. - `tests/imports/test_import_side_effects.py` — extended the existing record_soulsync_library_entry test to assert track_row['file_size'] == os.path.getsize(final_path), pinning the SoulSync-standalone path. Test fixture's tracks schema also updated to include the file_size column. Verified: full suite 1813 pass (13 new, 1 existing-test extension), ruff clean, smoke test populating + reading the column round-trips correctly. WHATS_NEW entry under '2.4.2' dev cycle.	3 weeks ago
Broque Thomas	42f3026eef	Reject broken downloads before tagging via universal integrity check ... Discord report (fresh.dumbledore [VRN]): slskd sometimes ships broken files (truncated transfers, corrupt FLAC, wrong file substituted on filename match). They flowed through post-processing and only surfaced later — Plex/Jellyfin scan failures, dead-air playback, duplicate detector tripping over the wrong length. By that point the file was already tagged, copied, mirrored to the media server, and recorded in provenance. New module `core/imports/file_integrity.py`: - `check_audio_integrity(path, expected_duration_ms=None) -> IntegrityResult` - Three tiered checks, cheapest to most expensive: 1. File size sanity (catches 0-byte stubs and stub transfers) 2. Mutagen parse (catches header damage, wrong-format-with-right-extension) 3. Duration agreement vs. metadata source's expected length, ±3s tolerance (5s for tracks over 10 minutes — long tracks naturally drift more) - Returns IntegrityResult with `ok`, human-readable `reason`, and per-check `checks` dict for debugging - Never raises; pathological inputs return ok=False with explanation Pipeline integration in `core/imports/pipeline.py:post_process_matched_download`: - Hooks between the existing file-stability wait and AcoustID verification - On failure: quarantine via existing `move_to_quarantine` helper, mark task failed with descriptive error, clear matched-context, fire `on_download_completed(success=False)` so the slot is released for retry - Mirrors the existing AcoustID-failure path so retry behavior stays consistent - Wrapped in try/except so an unexpected failure inside the check itself cannot block downloads — logs and continues This is intentionally tier 1: universal across formats, no external deps. A future tier could verify FLAC STREAMINFO MD5 by decoding audio (needs flac binary or libflac wrapper) — skipped for now since tier 1 catches the dominant Discord-reported cases (truncated, 0-byte, wrong file). Tests: - `tests/imports/test_file_integrity.py` — 14 cases covering all three check tiers, edge cases (zero/negative expected duration, long-track wider tolerance, caller tolerance override), and the mutagen-unavailable degradation path - `tests/imports/test_import_pipeline.py` — two existing tests use 5-byte fixture files that the new check would reject; they monkeypatch the integrity check since they're testing plumbing (notification + metadata_runtime forwarding), not integrity behavior WHATS_NEW entry under '2.4.2' dev cycle.	3 weeks ago
Broque Thomas	783c543c3e	Auto-import: live per-track progress + in-progress history row ... User reported (Mushy / generally) that dropping an album into the staging folder left the auto-import history blank for the entire processing window — sometimes 5+ minutes for a full album. Pre- existing UX gap, not caused by the recent context-builder refactor. Two root causes: 1. ``_record_result`` only fired AFTER ``_process_matches`` returned. For a 14-track album with ~30s/track post-processing, that meant ~7 minutes of zero rows in auto_import_history → nothing for ``/api/auto-import/results`` to return → empty UI. 2. ``_current_status`` only ever transitioned between 'idle' and 'scanning' — never 'processing'. ``get_status()`` had no per- track index/name fields, so the UI had no way to render "Processing track 3/14: Mine" even if it wanted to. Fix: - New ``_record_in_progress`` inserts a status='processing' row up-front (before the per-track loop starts) so the UI sees the import the moment it begins. Returns the row id. - New ``_finalize_result`` updates that same row with the final outcome (completed/failed) when processing finishes. One row per album, not per track — keeps the history list clean. - Both share ``_serialize_match_data`` (extracted from the original ``_record_result``) so the in-progress row carries the same match payload shape the existing review UI already understands. - ``_process_matches`` updates ``_current_track_index``, ``_current_track_total``, and ``_current_track_name`` BEFORE each per-track callback fires, so a polling UI sees consistent "processing N/M: <name>" snapshots. - ``_scan_cycle`` flips ``_current_status`` to 'processing' before the per-album loop, resets it + the per-track fields after. Defensive ``finally`` clears progress even if the inner code path raised. - ``get_status()`` exposes the new fields so the UI's existing /api/auto-import/status polling picks them up. - Frontend (stats-automations.js): renders the new ``current_status='processing'`` state with track index/total/name in the existing progress bar element. New 'processing' status class for styling parity with 'scanning'. 8 regression tests in tests/imports/test_auto_import_live_progress.py: - get_status surfaces the new fields with sane defaults - track_index advances 1, 2, 3 during a 3-track loop - track_total set BEFORE the first callback fires (no '1/0' flicker) - _record_in_progress writes status='processing' with no processed_at - _finalize_result updates the same row to completed + processed_at, no second insert - _finalize_result with failed status leaves processed_at NULL - _finalize_result with row_id=None is a safe no-op - Per-track fields cleared by _scan_cycle's finally block Full pytest 1643 passed; ruff clean.	3 weeks ago
Broque Thomas	486116c34f	Honor lossy_copy.delete_original after successful conversion ... Reported case (CAL): with lossy_copy.enabled=True, lossy_copy.delete_original=True, and codec=mp3, every download left both the original FLAC AND the converted MP3 in the target folder. Users opting into a lossy-only library ended up dual-format on every import. Root cause: ``core/imports/file_ops.py:create_lossy_copy`` reads ``lossy_copy.codec`` and ``lossy_copy.bitrate`` from config but never reads ``lossy_copy.delete_original``. The setting is only consulted by the pre-move source-vanished check at ``core/imports/pipeline.py:651`` (so the pipeline knows to look for a lossy variant when the FLAC has already moved on), but no code path actually deletes the source after conversion. Fix: after ffmpeg returns success and the QUALITY tag is written, check ``lossy_copy.delete_original`` and ``os.remove`` the original when enabled. Belt-and-suspenders: - Same-path guard (``os.path.normpath(out_path) != os.path.normpath(final_path)``) prevents accidentally wiping the just-converted file if a future codec choice somehow resolves out_path to the source path. - ``FileNotFoundError`` is treated as success (concurrent worker / dedup cleanup got there first). - Other ``OSError`` (permission denied, locked file) is logged but doesn't propagate — the conversion already succeeded, the user just has to clean up the original manually. Failure paths skip the delete: - ffmpeg returns non-zero → returns None, original stays - lossy_copy.enabled=False → early return before conversion runs - delete_original=False (default) → original stays 7 regression tests cover honored-when-enabled, kept-when-disabled, default-keep, ffmpeg-failure-path, lossy-disabled-path, racing-delete, and locked-file paths. Full pytest 1563 passed; ruff clean. Note: this PR does NOT address the second bug CAL mentioned (track re-downloaded despite already existing on disk). That symptom is caused by stale album metadata on the user's existing files — the library DB has the track tagged on a different album than the metadata source reports — combined with wishlist.allow_duplicate_tracks defaulting to True. Same class of issue partially addressed in PR fix/watchlist-redownload-and-duplicate-detection but compilation- album drift is the only currently-handled case. Tracking separately.	3 weeks ago
Antti Kettunen	2b3022f6b0	Fix Spotify source ID fallback ... - Prefer real Spotify IDs when importing Spotify contexts - Skip numeric fallback IDs so Deezer values do not leak into spotify_* columns - Add regressions for import context and SoulSync library writes - Keep the route test asserting the Spotify album link	3 weeks ago
Broque Thomas	46d8e15674	Prune slskd dedup orphans after import ... slskd appends "_<19-digit unix-nanosecond timestamp>" to a downloaded filename when the destination already contains a same-named file (concurrent downloads of the same track, partial-file retries after a connection drop, cancelled-then-redownloaded files, the same track surfacing in multiple synced playlists). The file-finder code already recognized the suffix when matching a download to its source — but after the canonical file moved into the library, the leftover "_<timestamp>" siblings sat orphaned in the downloads folder forever. Reported on Discord by Shdjfgatdif. cleanup_slskd_dedup_siblings() runs at the end of each successful import (3 safe_move_file sites in pipeline.py) and prunes any remaining siblings that strip down to the canonical stem with the same extension. Conservative match (>= 18 trailing digits) keeps legitimate filenames like "Track 5" and "Album 1995" untouched. Per- file unlink failures are swallowed so a single locked file doesn't block the rest. 17 regression tests cover the suffix-strip primitive, orphan removal, no-op cases, mismatched extensions, subdirectories, and partial-failure recovery.	4 weeks ago
Broque Thomas	99a38a6201	Route imported singles/EPs through album_path template ... Discord-reported (winecountrygames + fresh.dumbledore): "Import only makes Albums folder no singles or eps". Users with a ${albumtype}s/$albumartist/... album_path template saw an "Albums" folder fill up correctly but never any "Singles" or "EPs" folder. build_import_album_info detected an album using ``total_tracks > 1`` AND ``album_name != track_title``. Spotify singles fail both — total_tracks is 1 and the album is usually named after the song. The result was that staging/auto-import routed singles through single_path, which doesn't honour $albumtype, so the user's per-type folder layout never applied. Now also treats the metadata source's explicit release-type classification ("single", "ep", "compilation") as evidence that this is an album-shaped release, so it routes through album_path and the user's $albumtype substitution runs. The default fallback value "album" is deliberately excluded from this check so single-track downloads with no real metadata behave exactly as before. Adds 10 regression tests covering the reported scenario, EP and compilation explicit types, and three guards: normal multi-track albums still detected, default 'album' type falls through, and empty/unknown types fall through.	4 weeks ago
BoulderBadgeDad	58a4c1905b	Merge pull request #419 from kettui/refactor/metadata-service-split-and-metadata-client-management-optimizations ... Split metadata service logic into separate modules, move client management out of web_server	4 weeks ago
Antti Kettunen	50e1ae3a3f	Move metadata helpers into package modules ... - split metadata lookup logic into core/metadata/* - keep core/metadata_service.py as the legacy barrel - update tests and artist-detail code to patch concrete modules	4 weeks ago
Broque Thomas	d97d105b97	fix: substitute \$cdnum in download paths and skip auto disc folder when template uses it ... User report: multi-disc albums on the latest dev had literal "\$cdnum" in their filenames instead of the expected "CDxx" label, plus a redundant "Disc N" folder on top of the in-filename label. Two bugs in core/imports/paths.py: 1. _replace_template_variables (the substitution helper used by every download path builder) had no handling for \$cdnum or \${cdnum}. The matching helper in web_server.py and core/repair_jobs/library_reorganize.py did the substitution; this one didn't, so production downloads passed the placeholder through unchanged. Added a cdnum_value computation (CD%02d when total_discs > 1, empty otherwise) plus the corresponding bracket_map entry and \$cdnum replace before \$track (matches the ordering in the other path builders). 2. The album-path branch of build_final_path_for_track auto-injected a "Disc N" folder whenever total_discs > 1, suppressed only when the template contained \$disc. Templates using \$cdnum (or \${disc} / \${discnum} / \${cdnum}) got both a "CDxx" label in the filename and the auto folder. Widened the user_controls_disc check to cover all the disc-bearing placeholders. Bonus cleanup along the way: - Folder-part stripping now drops a leading \$cdnum token (mirrors the existing \$disc / \$discnum / \$quality strip — defensive against an empty cdnum landing alone in a folder segment). - Filename cleanup now strips a leading " - " left behind when \$cdnum expands to empty on a single-disc album (mirrors the same regex in library_reorganize.py). - album_template config access switched from the dotted-path key to the nested-dict access pattern used by the rest of the function — handles both production config_manager and the flat _Config used in tests. Tests: 4 new under tests/imports/test_import_paths.py - multi-disc cdnum substitution produces "CD02" - single-disc cdnum collapses to empty - folder-part containing only \$cdnum is dropped - build_final_path_for_track with \$cdnum template produces no auto "Disc N" folder Full suite: 1276 passing (was 1272). Ruff clean.	4 weeks ago
Antti Kettunen	f32fc9d56e	Extract wishlist logic into dedicated package ... - add core/wishlist as the home for wishlist payload, resolution, state, processing, reporting, and selection helpers - move wishlist-specific tests into tests/wishlist alongside the new package layout - keep web_server.py and the import/search callers as thin adapters for now	4 weeks ago
Antti Kettunen	02305096a3	Tighten metadata and import safety ... - Normalize album import track display handling so queue labels and match rows stay consistent - Bound MusicBrainz caches and avoid caching transient lookup failures - Stop swallowing programmer errors in source enrichment helpers - Restore import config test seams without reintroducing lazy imports - Guard task completion calls and fix the Windows path test expectation - Keep file lock tracking from growing without bound	4 weeks ago
Antti Kettunen	9315e74bea	Broaden import and metadata test coverage ... - Cover search_result fallback normalization and ambiguous album detection. - Add staging metadata, multi-disc path, and MusicBrainz enrichment cases. - Move the single-track context test next to the imports code it exercises.	4 weeks ago
Antti Kettunen	4f236baa6d	Fix import normalization and task completion locking ... - Promote legacy _source into source during import normalization. - Keep the normalized import context neutral after stripping aliases. - Avoid re-entering tasks_lock when marking completed download tasks.	4 weeks ago
Antti Kettunen	4c819681a1	Move single-track resolver; fix wishlist cleanup ... - keep single-track import lookup in imports/resolution.py - normalize simple-download search_result data before wishlist matching - run wishlist cleanup for simple-download post-processing - keep source-only artist detail on resolved names and MB short-circuit	4 weeks ago
Antti Kettunen	9b2b6d856f	Split runtime builders into owning modules ... - Move the import pipeline runtime factory into core.imports.pipeline - Move the metadata runtime factory into core.metadata.enrichment - Keep the web server wiring thin and drop the shared glue module - Add contract tests that keep the two runtime bundles separate	4 weeks ago
Antti Kettunen	bdef127dd6	Lift shared runtime state into core ... - Move app-wide task and activity registries out of core/imports - Share one runtime-state module across the web server, API, and import pipeline - Keep import-specific helpers focused on context and post-processing	4 weeks ago
Antti Kettunen	e10df4caf2	Rehome import helpers into core/imports ... - Move import flow modules into a dedicated package - Update app and test imports to the new namespace - Group the import-focused tests under tests/imports	4 weeks ago