Commit graph

5 commits

Author SHA1 Message Date
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
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
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