Commit graph

10 commits

Author SHA1 Message Date
dev
1770cae04f fix(quarantine): use name-based group key across batches/sources
Source-specific IDs (Spotify id, Qobuz id, URI) break sibling detection
when the same song is imported from different playlists or sources, since
each batch produces a different id: key. Siblings from a second batch were
never recognised and stayed in quarantine after approving the first.

Drop id:/uri: fallbacks; keep only ISRC (truly universal) and nm:artist|track
(stable across all sources). This correctly groups all quarantine entries for
the same intended target regardless of which batch or source produced them.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-06-24 12:48:04 +02:00
dev
b761229a00 merge: pull upstream/main (2.7.4) into feature/best-quality-search-mode
- Keep our v3 ranked-targets quality system (filter_and_rank, QualityTarget)
  in soulseek_client.py, settings.js, database presets, and index.html
- Take upstream removal of standalone quality-scanner code:
  QualityScannerDeps + run_quality_scanner moved to repair job
  (core/repair_jobs/quality_upgrade_scanner.py)
- Take upstream AAC-tier addition in database/music_database.py default profile
- Take upstream removal of /api/quality-scanner/* routes from web_server.py
- Remove test_discovery_quality_scanner.py (deleted upstream)
- 47 upstream commits absorbed (2.7.3 + 2.7.4 including re-identify flow,
  dead-folder cleanup, track-number prefix strip, and more)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-06-19 16:54:51 +02:00
BoulderBadgeDad
46be97b195 #876: group quarantine alternatives by target track-id + auto-clear siblings on approve
Multiple failed source attempts at one song each land in quarantine as
separate entries. Group them by the *intended* target (sidecar context
track_info isrc -> id -> uri, falling back to normalized artist|title for
legacy thin sidecars) — an exact relationship across siblings, since the
bad files' own tags differ but the target track is constant.

- core: quarantine_group_key() + find_quarantine_siblings() seams; list
  entries now carry group_key.
- approve endpoint: remove_siblings flag auto-deletes the other attempts
  once one is accepted (captured BEFORE approve restores the file out of
  quarantine, or the id lookup would resolve nothing). Scoped to the
  quarantine manager; download-modal chooser + version-mismatch fallback
  pass no flag and are unaffected.
- UI: multi-member groups render as a collapsible parent row (album art +
  'N alternatives'); singletons unchanged. Toast reports removed count.
- 11 tests incl. ordering regression for capture-before-approve.
2026-06-15 22:12:06 -07:00
dev
cf8f562e61 feat(quarantine): show real audio quality in the review UI + path-resolve diag
- list_quarantine_entries now surfaces the probed quality (context._audio_quality,
  recorded before the gates) so each quarantine row shows what the file actually
  is when deciding to approve/delete. Rendered as a quality chip in the review UI.
- _resolve_library_file_path logs the searched base dirs once when it can't
  resolve a path, so a remaining mount/path mismatch is diagnosable.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-06-14 23:53:52 +02:00
dev
37ea6604c7 Fix import artist override and verification review 2026-06-11 01:28:31 +02:00
BoulderBadgeDad
3060678f29 Quarantine: manage a quarantined file from the download modal (Listen / Accept / Search)
Clicking a quarantined track's status used to open the generic search modal,
identical to a plain failure — no way to review or recover the file. It now
opens a chooser:
- Listen: streams the file in-app via a new /api/quarantine/<id>/stream
  endpoint (range-supported; the real audio Content-Type is recovered from the
  sidecar since the on-disk file ends in .quarantined).
- Accept & Import: existing /approve (restore + re-import, gates bypassed).
- Search for a different result: the existing candidates modal (old behavior).

Non-quarantine failures (not_found / failed / cancelled) are unchanged — a
single click listener routes by dataset set at render time, so a task that
fails then later quarantines can't end up double-bound.

Also fixes the Accept failure on Windows: the Listen stream holds an open file
handle, so the subsequent restore move hit WinError 32 ('file in use') and the
endpoint mislabeled it 'thin sidecar'. Accept now releases the audio handle
before approving, and approve/recover moves retry briefly on transient OS locks
(_move_with_retry). Accept also auto-falls-back to Recover-to-Staging for
genuinely thin/orphaned sidecars.

Tests: stream-info resolution (sidecar + filename-fallback + missing), and
_move_with_retry success/give-up.
2026-05-31 15:41:04 -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
f25433ea57 Harden quarantine approval flows 2026-05-18 08:37:11 -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
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.
2026-05-14 08:06:19 -07:00