Commit graph

8 commits

Author SHA1 Message Date
Broque Thomas
e140da117a Extract automation handlers (4/3 — finish): progress callbacks + scan-completion emitter
Cleans up the four remaining inline callbacks at the bottom of
`web_server._register_automation_handlers` so the function is now
purely deps-construction + register_all + a logger.info line.

Lifted:
- `_progress_init`, `_progress_finish`, `_record_automation_history`,
  and `_on_library_scan_completed` -> core/automation/handlers/progress_callbacks.py

Each is a top-level function that takes deps as a parameter; the
engine sees thin lambdas through `register_progress_callbacks` /
`register_library_scan_completed_emitter` (called from `register_all`).

Two new deps fields:
- `init_automation_progress` (delegates into the live progress tracker)
- `record_progress_history` (delegates into _auto_progress.record_history)

12 new boundary tests in tests/automation/test_progress_callbacks.py
pin every shape:
- progress_init forwards to init_automation_progress
- progress_finish skips when handler manages its own progress
  (prevents double-emit of finished status)
- progress_finish: completed -> finished/Complete/success;
  error -> error/Error/error; msg falls through error -> reason ->
  status -> 'done'
- record_history threads the live db into the recorder
- on_library_scan_completed: no engine = noop, server type taken
  from web_scan_manager._current_server_type, defaults to 'unknown'
- register_library_scan_completed_emitter: no scan manager = noop,
  registered callback emits the right event when invoked

3256 tests pass, no regression.

Final state of `_register_automation_handlers`:
- Was: 1530 lines, 21 nested closures + 4 progress callbacks
- Now: ~50 lines, builds AutomationDeps and calls register_all

web_server.py: 34,220 -> 34,187 lines (-33 net, -1,406 across the
whole branch).
2026-05-15 11:59:32 -07:00
Broque Thomas
017553193f Extract automation handlers (3/3): maintenance + misc, finishing the lift
Final commit of the automation-handler refactor. With this commit
every closure that used to live in
`web_server._register_automation_handlers` is now a top-level
function in `core/automation/handlers/`.

Handlers extracted in this commit:

- start_database_update + deep_scan_library
    -> core/automation/handlers/database_update.py
    Both share the db_update_state monitoring pattern (poll until
    status flips, stall detection emits warning at 10 min, 2-hour
    outer timeout). Lifted into a shared `_run_with_progress` helper
    inside the module so the per-handler bodies stay tiny.

- run_duplicate_cleaner -> core/automation/handlers/duplicate_cleaner.py
- start_quality_scan    -> core/automation/handlers/quality_scanner.py

- clear_quarantine, cleanup_wishlist, update_discovery_pool,
  backup_database, refresh_beatport_cache
    -> core/automation/handlers/maintenance.py
    Grouped because each body is short (~20-50 lines) and they share
    no state — splitting into per-handler files would just add import
    noise.

- clean_search_history, clean_completed_downloads, full_cleanup
    -> core/automation/handlers/download_cleanup.py
    Grouped because all three reach the download orchestrator,
    tasks_lock, and download_batches/download_tasks accessors. The
    full_cleanup multi-step orchestration shares phase-detection
    logic with clean_completed_downloads.

- run_script         -> core/automation/handlers/run_script.py
- search_and_download -> core/automation/handlers/search_and_download.py

`AutomationDeps` grew with the new dependency surface:
- get_db_update_state + db_update_lock + db_update_executor +
  run_db_update_task + run_deep_scan_task
- get_duplicate_cleaner_state + duplicate_cleaner_lock +
  duplicate_cleaner_executor + run_duplicate_cleaner
- get_quality_scanner_state + quality_scanner_lock +
  quality_scanner_executor + run_quality_scanner
- download_orchestrator + run_async + tasks_lock +
  get_download_batches + get_download_tasks +
  sweep_empty_download_directories + get_staging_path
- docker_resolve_path + get_current_profile_id +
  get_watchlist_scanner + get_app + get_beatport_data_cache
- set_db_update_automation_id (writes the legacy global so the live
  DB-update progress callbacks still living in web_server.py keep
  emitting against the active automation card)

`web_server._register_automation_handlers` is now ~50 lines: build
deps once, call register_all. The 667-line block of remaining
closure definitions and engine register calls is gone.

The final orphan was the `_db_update_automation_id` module global —
the DB-update progress callbacks at line ~14080 still read it
directly, so the extracted database_update handler propagates the
automation id through `deps.set_db_update_automation_id` (a closure
in web_server that writes the global). When the legacy callbacks
get extracted in a future PR the setter goes away.

Tests:
- tests/automation/test_handlers_maintenance.py adds 21 boundary
  tests covering every newly-extracted handler shape: guard
  short-circuits (already-running returns skipped), deps wiring
  (set_db_update_automation_id called with the right id),
  exception swallow contract, status returns, path-traversal
  blocked in run_script, source-mode skip in clean_search_history,
  active-batch skip in clean_completed_downloads, etc.
- 3244 tests pass (was 3223 — 21 new), no regression.

web_server.py: 35,593 -> 34,220 lines (-1,373 net across 3 commits).
Issue #1 from the extraction punch list is now COMPLETE.
2026-05-15 11:24:35 -07:00
Broque Thomas
cde237c7e7 Extract automation handlers (2/N): playlist lifecycle group
Continues the lift from `web_server._register_automation_handlers`.
This commit extracts the four playlist-lifecycle closures:

- `refresh_mirrored`   -> core/automation/handlers/refresh_mirrored.py
- `sync_playlist`      -> core/automation/handlers/sync_playlist.py
- `discover_playlist`  -> core/automation/handlers/discover_playlist.py
- `playlist_pipeline`  -> core/automation/handlers/playlist_pipeline.py

The pipeline composes refresh + sync + discover, so all four ship
together. The pipeline imports the other three handler modules
directly (cross-handler call) instead of going through the engine,
preserving the "single trigger from the user's perspective" UX.

`AutomationDeps` grew to cover the new dependency surface:
- run_playlist_discovery_worker, run_sync_task, load_sync_status_file
  (pre-existing background-task entry points)
- get_deezer_client, parse_youtube_playlist (per-source clients)
- get_sync_states (live mutable accessor for the sync UI's state dict)

`web_server._register_automation_handlers` now wires those plus the
existing infrastructure into a single `AutomationDeps` and calls
`register_all`. The 669-line block of closure definitions and engine
register calls (lines 959-1627 pre-edit) is gone -- the file shed
743 lines net on this commit.

`tests/automation/test_handlers_playlist.py` adds 17 new boundary
tests:
- discover_playlist: no_id error, specific_id starts worker, all=True
  enumerates, no playlists in db
- refresh_mirrored: error path, source filter (file/beatport excluded),
  Spotify happy path with auto-discovered marker, per-playlist
  exception captured into errors counter
- sync_playlist: no_id, not_found, no_tracks, no-discovered-tracks
  skip, discovered-track happy path, unchanged-since-last-sync skip
- playlist_pipeline: no_playlist clears running flag, no-refreshable
  clears running flag, exception clears running flag

3223 tests pass. web_server.py: 35,593 -> 34,850 lines (743 removed).
2026-05-15 10:47:46 -07:00
Broque Thomas
ea7d5c65bb Extract automation handlers (1/N): infrastructure + 3 simple handlers
Begins the lift of `web_server._register_automation_handlers` (1530
lines, 20 nested closures) into `core/automation/handlers/`. Each
extracted handler is a top-level function that accepts
`(config, deps)` instead of reaching for module-level globals --
makes them unit-testable in isolation.

Infrastructure:
- `core/automation/deps.py`: `AutomationDeps` (dependency-injection
  bundle of clients + callables) and `AutomationState` (mutable flags
  shared across handler invocations, with thread-safe accessors).
- `core/automation/handlers/__init__.py` + `registration.py`: one-stop
  `register_all(deps)` that wires every extracted handler to the
  engine.

First batch of handlers extracted:
- `process_wishlist` -> `core/automation/handlers/process_wishlist.py`
- `scan_watchlist`   -> `core/automation/handlers/scan_watchlist.py`
- `scan_library`     -> `core/automation/handlers/scan_library.py`

`web_server._register_automation_handlers` now builds the deps once
and calls `register_all(deps)` for the extracted batch. Remaining
17 closures still live below; subsequent commits in this branch
finish the lift.

14 boundary tests in `tests/automation/test_handlers_simple.py` pin
every shape: success path, exception swallow contract, fresh-vs-stale
state detection (scan_watchlist's id() trick), guard short-circuits,
state cleanup on exceptions, AutomationState concurrent-safe accessors.
All 101 automation tests pass; no regression.
2026-05-15 10:25:41 -07:00
Broque Thomas
aa54bed818 Surface silent exceptions across remaining modules — ~70 sites
Final sweep. Covers:
- Downloads: candidates / lifecycle / master / monitor / wishlist_failed
- Metadata: source / registry / cache / common / artwork (+ plex_client)
- Imports: pipeline / resolution / file_ops / paths / guards
- Library: path_resolver / retag / duplicate_cleaner
- Stats / playlists / wishlist / discovery / automation / enrichment
- Misc: hydrabase_client, soulsync_client, tag_writer, debug_info,
  api_call_tracker, album_consistency, beatport_unified_scraper,
  reorganize_runner, seasonal_discovery, lidarr_download_client,
  services/sync_service.py, automation_engine, automation/progress

Two `_e` renames in imports/file_ops.py (outer scope binding `e`).
A few finally-block sites in metadata/album_mbid_cache.py,
library/track_identity.py, listening_stats_worker.py, watchlist/
auto_scan.py left silent — same reason as the rest of the sweep
(logger calls during cleanup paths can themselves raise).

Refs #369
2026-05-07 10:28:58 -07:00
Broque Thomas
8219771304 Add module logger + surface silent exceptions in 7 logger-less files — 12 sites
These files had silent `except Exception: pass` blocks but no module
logger. Added `import logging` + `logger = logging.getLogger(__name__)`
at the top of each, then replaced the silent excepts with
`logger.debug(...)`.

- core/replaygain.py — 4 sites (id3 txxx + vorbis + mp4 atom reads)
- core/wishlist/presence.py — 3 sites (wishlist row parsing + queries)
- core/runtime_state.py — 1 site (activity toast emit)
- core/automation/signals.py — 1 site (collect known signals)
- core/download_engine/rate_limit.py — 1 site (plugin rate_limit_policy)
- api/system.py — 1 site (hydrabase status probe)
- api/search.py — 1 site (hydrabase search)

Refs #369
2026-05-07 10:27:04 -07:00
Broque Thomas
a8319156ce Lift /api/automations/blocks static config into core/automation/blocks.py
The endpoint was returning a 200-line literal dict inline. Moved the
three lists (TRIGGERS, ACTIONS, NOTIFICATIONS) to module-level constants
in core/automation/blocks.py. Route shrinks to 7 lines. Data is now
importable for tests + future docs.

Added 8 shape tests so a typo in the dict (missing 'type', wrong
field type, missing options on a select, etc.) gets caught by CI
instead of breaking the builder UI silently.

The `known_signals` field stays computed at request time via
_collect_known_signals(database) since it's dynamic.

No behavior change. Same response shape. 869 tests passing (was 861).
Ruff clean.
2026-04-27 18:31:36 -07:00
Broque Thomas
6cdcf778f3 Lift /api/automations/* into core/automation/
Routes moved to thin parse-args/jsonify handlers; logic now lives in
three focused modules under core/automation/. 436 lines deleted from
web_server.py; 53 added back as wrappers.

Module split:
- core/automation/api.py — CRUD + run + history helpers. Each function
  takes (database, automation_engine, ...) explicitly and returns
  (response_body, http_status). Includes signal cycle detection
  preflight checks for create + update.
- core/automation/progress.py — owns the in-memory progress state dict
  + lock (mirroring the original web_server.py globals as module-level
  shared state so all callers see one view), init/update/history
  helpers, and the WebSocket emit loop.
- core/automation/signals.py — collect_known_signals for the builder
  autocomplete.

Out of scope (deferred):
- _register_automation_handlers — the 23+ action handler closures stay
  in web_server.py because each one is tightly coupled to feature-
  specific implementations (wishlist, watchlist, library scan, etc.).
- Worker functions (_process_wishlist_automatically, etc.) — belong
  with their feature lifts.
- _run_sync_task / _run_playlist_discovery_worker — sync + discovery
  PRs.

Behavior preserved 1:1:
- Same route response shapes + status codes
- Same JSON field hydration (trigger_config, action_config,
  notify_config, last_result, then_actions)
- Same backward-compat: empty then_actions + notify_type set →
  synthesize then_actions from notify_type/notify_config
- Same signal cycle detection behavior on create + update
- Same system-automation protection on delete + duplicate
- Same reschedule/cancel logic on toggle + bulk-toggle + update
- Same progress state shape (status, progress, phase, current_item,
  log capped at 50, started_at/finished_at, action_type)
- Same emit-on-finish socketio push from update_progress
- Same emit loop semantics (1s tick, snapshot active states, reap
  finished after window)

Pre-existing bugs preserved (will fix in follow-up PRs):
- emit_progress_loop uses naive datetime.now() against tz-aware
  started_at/finished_at, so the timeout-zombie check raises
  TypeError → caught → never fires, and the cleanup-after-window
  check raises → caught → state is reaped on FIRST tick regardless
  of the window. Tests document this behavior so the next PR can
  flip them to the corrected expectation.

Tests: 72 new under tests/automation/ (signals 10, progress 24,
api 38). Full suite: 861 passing (was 789). Ruff clean.
2026-04-27 18:05:14 -07:00