diff --git a/core/watchlist/auto_scan.py b/core/watchlist/auto_scan.py index 759d43f7..e26ab998 100644 --- a/core/watchlist/auto_scan.py +++ b/core/watchlist/auto_scan.py @@ -185,7 +185,11 @@ def process_watchlist_scan_automatically(automation_id=None, profile_id=None, de 'results': [], 'summary': {}, 'error': None, - 'cancel_requested': False + 'cancel_requested': False, + # #933: stamp these so this scan lands in the History modal too — + # the scanner fills scan_track_events; persist_scan_run reads both. + 'scan_run_id': datetime.now().strftime('%Y%m%d-%H%M%S'), + 'scan_track_events': [], } scan_results = [] @@ -294,6 +298,17 @@ def process_watchlist_scan_automatically(automation_id=None, profile_id=None, de total_added_to_wishlist = deps.watchlist_scan_state.get('summary', {}).get('tracks_added_to_wishlist', 0) logger.warning("Automatic watchlist scan cancelled — skipping post-scan steps") + # #933: record this run in the History ledger — same helper the manual + # scan uses, so scheduled scans show up alongside manual ones. + try: + from core.watchlist.scan_history import persist_scan_run + persist_scan_run( + database, deps.watchlist_scan_state, + profile_id=profile_id, was_cancelled=was_cancelled, + ) + except Exception as _hist_err: + logger.error(f"Failed to persist watchlist scan run: {_hist_err}") + # Post-scan steps — skip if cancelled if not was_cancelled: # Populate discovery pool from similar artists (per-profile) diff --git a/core/watchlist/scan_history.py b/core/watchlist/scan_history.py new file mode 100644 index 00000000..a87011ca --- /dev/null +++ b/core/watchlist/scan_history.py @@ -0,0 +1,51 @@ +"""Persist a finished watchlist scan to the History ledger (#831 / #933). + +Both the manual scan (``web_server.start_watchlist_scan``) and the automatic +scan (``core.watchlist.auto_scan.process_watchlist_scan_automatically``) finish +with the same ``watchlist_scan_state`` shape, but only the manual path used to +record a history row — so scheduled/nightly scans never showed up in the +History modal (#933). This single helper is the shared seam: both paths call it, +so they can't drift apart again. + +Pure except for the one ``database.save_watchlist_scan_run`` call — the field +extraction is unit-testable with a fake database. +""" + +from __future__ import annotations + +from datetime import datetime +from typing import Any, Dict, Optional + + +def _iso(value: Any) -> Optional[str]: + """ISO-format a datetime; pass through an already-stringified timestamp.""" + if value is None: + return None + return value.isoformat() if hasattr(value, 'isoformat') else str(value) + + +def persist_scan_run(database: Any, state: Dict[str, Any], *, + profile_id: Any, was_cancelled: bool) -> bool: + """Record one watchlist scan run + its track ledger from ``state``. + + Reads the counts/timestamps/ledger off the live ``watchlist_scan_state`` the + scanner just finished writing, and writes a single history row. ``run_id`` + comes from ``state['scan_run_id']`` (both paths stamp it); a timestamp + fallback keeps it from ever colliding if that's somehow missing. Returns the + DB call's truthiness; callers wrap in their own try/except so a history-write + failure never breaks the scan. + """ + summary = state.get('summary') or {} + run_id = state.get('scan_run_id') or datetime.now().strftime('%Y%m%d-%H%M%S') + return database.save_watchlist_scan_run( + run_id=run_id, + profile_id=profile_id if profile_id else 1, + status='cancelled' if was_cancelled else 'completed', + started_at=_iso(state.get('started_at')), + completed_at=_iso(state.get('completed_at')) or datetime.now().isoformat(), + total_artists=summary.get('total_artists', state.get('total_artists', 0)), + artists_scanned=summary.get('successful_scans', 0), + tracks_found=state.get('tracks_found_this_scan', 0), + tracks_added=state.get('tracks_added_this_scan', 0), + track_events=state.get('scan_track_events') or [], + ) diff --git a/tests/test_watchlist_scan_history.py b/tests/test_watchlist_scan_history.py index 3b6f4924..7281e230 100644 --- a/tests/test_watchlist_scan_history.py +++ b/tests/test_watchlist_scan_history.py @@ -47,6 +47,62 @@ def test_resave_is_idempotent_on_run_id(db): assert len(runs) == 1 and runs[0]['tracks_added'] == 11 +# ── persist_scan_run: the shared seam both scan paths use (#933) ── + +from datetime import datetime + +from core.watchlist.scan_history import persist_scan_run + + +def _state(**over): + """A watchlist_scan_state as the scanner leaves it when a scan finishes.""" + s = { + 'scan_run_id': 'auto-run-1', + 'started_at': datetime(2026, 6, 26, 2, 0, 0), + 'completed_at': datetime(2026, 6, 26, 2, 4, 0), + 'total_artists': 40, + 'tracks_found_this_scan': 7, + 'tracks_added_this_scan': 3, + 'scan_track_events': _events(n_added=3, n_skipped=1), + 'summary': {'total_artists': 40, 'successful_scans': 40, + 'new_tracks_found': 7, 'tracks_added_to_wishlist': 3}, + } + s.update(over) + return s + + +def test_persist_scan_run_records_a_history_row(db): + # the #933 fix: an automatic (all-profiles → profile_id=None) scan must land in History. + assert persist_scan_run(db, _state(), profile_id=None, was_cancelled=False) is True + runs = db.get_watchlist_scan_runs() + assert len(runs) == 1 + r = runs[0] + assert (r['run_id'], r['status'], r['tracks_found'], r['tracks_added']) == \ + ('auto-run-1', 'completed', 7, 3) + assert r['profile_id'] == 1 # None coerced to a concrete profile, never NULL + # the per-run ledger came through too + assert [e['status'] for e in db.get_watchlist_scan_run_events('auto-run-1')] == \ + ['added', 'added', 'added', 'skipped'] + + +def test_persist_scan_run_cancelled_status(db): + persist_scan_run(db, _state(scan_run_id='c1'), profile_id=2, was_cancelled=True) + assert db.get_watchlist_scan_runs()[0]['status'] == 'cancelled' + + +def test_persist_scan_run_accepts_datetime_or_iso_string(db): + # state timestamps may be datetime (auto path) or already-iso strings — both must persist. + persist_scan_run(db, _state(scan_run_id='dt', completed_at='2026-06-26T02:09:00'), + profile_id=1, was_cancelled=False) + assert db.get_watchlist_scan_runs()[0]['run_id'] == 'dt' + + +def test_persist_scan_run_tolerates_sparse_state(db): + # a bare/early-finished state must not raise — history-write must never break a scan. + assert persist_scan_run(db, {'scan_run_id': 'sparse'}, profile_id=None, was_cancelled=False) + assert db.get_watchlist_scan_runs()[0]['tracks_added'] == 0 + + def test_prune_keeps_most_recent(db): for i in range(1, 8): db.save_watchlist_scan_run( diff --git a/tests/watchlist/test_auto_scan.py b/tests/watchlist/test_auto_scan.py index 4ad70258..6ac83413 100644 --- a/tests/watchlist/test_auto_scan.py +++ b/tests/watchlist/test_auto_scan.py @@ -81,6 +81,11 @@ class _FakeDB: self._watchlist_artists = watchlist_artists or [] self._lb_profiles = lb_profiles or [] self.database_path = '/tmp/test.db' + self.scan_runs_saved = [] + + def save_watchlist_scan_run(self, **kwargs): + self.scan_runs_saved.append(kwargs) + return True def get_all_profiles(self): return self._profiles @@ -237,6 +242,39 @@ def test_successful_scan_runs_post_steps(patched_modules): assert deps._state_ref[0]['summary']['new_tracks_found'] == 2 +def test_successful_scan_records_history_run(patched_modules): + """#933: the automatic scan must persist a History row (it used to skip this, + so only manual scans appeared in History).""" + scanner, db = patched_modules + deps = _build_deps() + + autosc.process_watchlist_scan_automatically(automation_id='a1', deps=deps) + + assert len(db.scan_runs_saved) == 1 # exactly one row, no double-record + saved = db.scan_runs_saved[0] + assert saved['status'] == 'completed' + assert saved['artists_scanned'] == 1 # one successful _ScanResult + assert saved['run_id'] # a run id was stamped + + +def test_cancelled_scan_still_records_history_run(patched_modules, monkeypatch): + """A cancelled scan is recorded too, with status='cancelled'.""" + scanner, db = patched_modules + deps = _build_deps() + # Make the scan observe a cancel request mid-run. + orig = scanner.scan_watchlist_artists + def _cancel_during(artists, *, scan_state, progress_callback, cancel_check): + scan_state['cancel_requested'] = True + return orig(artists, scan_state=scan_state, progress_callback=progress_callback, + cancel_check=cancel_check) + monkeypatch.setattr(scanner, 'scan_watchlist_artists', _cancel_during) + + autosc.process_watchlist_scan_automatically(automation_id='a1', deps=deps) + + assert len(db.scan_runs_saved) == 1 + assert db.scan_runs_saved[0]['status'] == 'cancelled' + + def test_completion_emits_automation_event(patched_modules): """Successful scan emits 'watchlist_scan_completed' on automation_engine.""" scanner, _ = patched_modules diff --git a/web_server.py b/web_server.py index 19f9bc52..09614540 100644 --- a/web_server.py +++ b/web_server.py @@ -28223,22 +28223,10 @@ def start_watchlist_scan(): # #831 round 2: persist this run + its track ledger so the # Watchlist History modal can show what every past scan did. try: - _state = watchlist_scan_state - get_database().save_watchlist_scan_run( - run_id=_state.get('scan_run_id') or datetime.now().strftime('%Y%m%d-%H%M%S'), - profile_id=scan_profile_id, - status='cancelled' if was_cancelled else 'completed', - started_at=(_state.get('started_at').isoformat() - if _state.get('started_at') else None), - completed_at=(_state.get('completed_at') or datetime.now()).isoformat() - if not isinstance(_state.get('completed_at'), str) - else _state.get('completed_at'), - total_artists=(_state.get('summary') or {}).get('total_artists', - _state.get('total_artists', 0)), - artists_scanned=(_state.get('summary') or {}).get('successful_scans', 0), - tracks_found=_state.get('tracks_found_this_scan', 0), - tracks_added=_state.get('tracks_added_this_scan', 0), - track_events=_state.get('scan_track_events') or [], + from core.watchlist.scan_history import persist_scan_run + persist_scan_run( + get_database(), watchlist_scan_state, + profile_id=scan_profile_id, was_cancelled=was_cancelled, ) except Exception as _hist_err: logger.error(f"Failed to persist watchlist scan run: {_hist_err}")