bug: renaming the action_types only changed code — a db already seeded under the
old names kept the stale rows (the seeder only CREATES missing ones, never removes
or renames). so on restart you got BOTH 'Auto-Download Movie Wishlist' (orphaned,
dead action) AND 'Auto-Process Movie Wishlist'.
add _fix_wishlist_processor_rename (runs in ensure_system_automations like the
other _fix_* migrations): deletes the orphaned video_download_movie/episode_wishlist
system rows (clearing is_system first, since delete_automation guards system rows)
and renames the youtube row in place (its action_type was unchanged). idempotent.
tested.
- 'Download X Wishlist' -> 'Process X Wishlist' everywhere (label + action_type):
matches the music side's 'process_wishlist', the already-process youtube action,
and reads right (movie/episode do search+pick+download, not just download).
action_types video_download_movie/episode_wishlist -> video_process_*.
- group the new automations as a two-stage pipeline in the seed list + builder
palette: Stage 1 SCANS (people/channels/playlists) fill the wishlist, Stage 2
PROCESSORS (movie/episode/youtube) drain it. icons/labels/drift test updated.
brand-new this session so no migration needed. 337 automation tests green.
the soulseek counterpart of the youtube drain — finally makes the people/airing
scans pay off. two automations (Auto-Download Movie / Episode Wishlist, hourly):
for each wished+released item, do a bounded blocking slskd search, pick the top
ACCEPTED release per the quality profile, and enqueue it exactly like a manual
grab (same add_video_download shape → the monitor finishes + organises it).
same standard as youtube: processes the WHOLE eligible wishlist (no total cap)
but searches a few at a time (max_concurrent, default 3, via a thread pool). a
busy guard skips the next hourly tick while a drain is still working. movies gate
on status='wanted' (skips monitored); episodes are all-wished. items already
downloading are skipped. quiet skip if the library folder isn't set.
reuses the real infra (build_query/slskd_search/_evaluate_hits/start_download/
download_monitor) — pure pick/select/record seam-tested; db queries added. 377
automation+video tests green.
playlists are followable but had no scan. new 'Scan Watchlist Playlists'
automation, sibling of the channel scan but a different rule: a playlist is a
curated finite set, so MIRROR it — wishlist every long-form video you don't have
plus any later additions (no forward-looking baseline, no last-N net).
playlist-as-show: videos wishlisted under the playlist's title, so the worker
files them as 'Playlist Name / Season YEAR / ... - date - title' (the ytdl-sub
tv_show_name-on-a-playlist convention). reuses all the channel plumbing — same
wishlist rows, same Download YouTube Wishlist drain, quality + org template.
seeded every 6h (Auto-Scan Watchlist Playlists); block + registration + icon +
drift test. dedup reuses wishlisted_video_ids_for_channel (parent_source_id=PL).
seam-tested; 944-test sweep green.
new 'Download YouTube Wishlist' automation: pushes a polite batch of wished
youtube videos into the shared video_downloads queue + spawns the yt-dlp worker
per video. skips in-flight ones (no double-grab); big backlogs drain over
several scheduled runs (batch_size, default 3). needs the youtube library folder
set.
- download_monitor: SKIP source='youtube' rows (owned by their worker thread, no
slskd transfer to match) — surgical, slskd path untouched.
- db.youtube_wishlist_to_download(): flat newest-first list of wished videos with
channel/title/date/thumb for organising.
block + registration + icon/label + drift test. all seam-tested.
two new video-side automation blocks that keep the wishlist fed:
- scan watchlist people: for each followed person, wishlist every un-owned
movie they acted in or directed (back catalog + upcoming). released ->
wanted, upcoming -> monitored (engine skips it til it's out, promotes on
release). grabs rich detail at add time (backdrop/cast/overview/etc +
provenance) into a new video_wishlist.detail_json col. fast re-runs skip
already-wishlisted + only promote.
- scan watchlist channels: for each followed youtube channel, wishlist new
long-form uploads (shorts excluded). forward-looking from follow time +
a last-N safety net (default 10). diffs against wishlisted/downloaded/
dismissed so it never dupes. pair with a 6h schedule trigger. scan-only;
fulfillment engine comes later.
both are pure handlers with injected seams + full seam tests. add_movie_to_
wishlist gains status + detail_json (promote-only upsert). no schema break,
music side untouched.
Unlike the cleanup twins, backup can't share the music handler — it's a different
DB file. Extract the music backup body into _backup_db_at(db_path, ...) (music
behaviour byte-identical, now a thin wrapper over DATABASE_PATH) and add
auto_backup_video_database pointing at VIDEO_DATABASE_PATH (video_library.db).
New video_backup_database action (scope='video' block + registry), owned_by='video'
system automation on the music cadence (every 3 days).
Tests: a REAL backup behaviour test — music backup lands next to music_library.db,
video backup next to video_library.db, no cross-contamination (this is the whole
reason it can't be shared); scope isolation; single video-owned seed; own handler.
Existing music maintenance tests (22) still green — refactor is non-regressing.
EXPECTED_ACTION_NAMES updated.
Same pattern: video_full_cleanup action (scope='video' block + registry), reuses
the shared auto_full_cleanup handler, owned_by='video' system automation on the
music cadence (every 12h). Music copy untouched. Seam tests + EXPECTED_ACTION_NAMES.
Same pattern as phase 2: video_clean_completed_downloads action (scope='video'
block + registry), reuses the shared auto_clean_completed_downloads handler, and
an owned_by='video' system automation on the music cadence (every 5 min). Music
copy untouched. Seam tests for scope isolation, single video-owned seed, and
shared-handler reuse; EXPECTED_ACTION_NAMES updated.
The music side's 'Clean Search History' automation now has a video counterpart so
it appears on the video Automations page too. Distinct action_type
video_clean_search_history (the system seeder keys on action_type, so reusing the
music key would collide), registered to the SAME shared handler so behaviour is
identical, scope='video' block (registry — users can build their own), and an
owned_by='video' system automation on the same 1h cadence. The music action/row
is untouched.
Seam tests: video-scoped only (not on music), music action still music-scoped,
exactly one video-owned system row at the 1h cadence, and it reuses the music
handler. Registration contract (EXPECTED_ACTION_NAMES) updated.
The probe fired the instant a batch finished, but a fresh drop takes ~1-2 min to
appear even with the server's auto-scan ON — so it always missed and we crawled
anyway, defeating the optimization. Now probe_present_libraries POLLS each candidate
over a grace window (probe_grace_minutes, default 2), skipping a library's crawl as
soon as the server reports it has the item, and only crawling what's still missing
when grace expires. The probe target for a media type you DIDN'T just download is an
old item the server already has → confirmed instantly, no wait. grace=0 probes once.
Scanning is expensive and most servers auto-ingest new files, so a full crawl after
every download is usually wasted. Stage 1 now probes per library: take the newest
completed grab of that type from download history and ask the server (cheap targeted
search) whether it already has it. If yes, the server auto-picked it up (and the
earlier ones) → skip that library's crawl + poll entirely. Only libraries the server
is missing get rescanned. Always emits so stage 2 still reads the new items in.
- sources: PlexVideoSource.has_item / JellyfinVideoSource.has_item (match movie by
title+year, episode by show+SxE) + video_server_has_item() — conservative, any
uncertainty → False so we scan.
- handler: per-scope skip decision fed by latest_completed + server_has_item seams;
narrows the scan scope to only the missing libraries; toggle skip_if_present
(default on). Returns scanned/skipped for visibility.
Seam tests: skip-both, scan-only-missing, no-history, toggle-off, probe-error→scan;
Plex has_item match tests.
A fixed debounce can't fit a big library — 8500 movies + 4500 shows scan sequentially
through Plex's queue and can take 10-20 min, so the old 120s wait read the DB before
Plex finished and fresh downloads showed up late. Now Stage 1 (video_scan_server)
fires the rescan then POLLS the server until its scan queue goes idle, then emits the
done event.
- sources: PlexVideoSource.is_scanning (section.refreshing + activity feed, scoped by
media_type) and JellyfinVideoSource.is_scanning (scheduled-task state), plus
video_server_scan_in_progress() returning True/False/None.
- handler: pure wait_for_server_scan(scan_status, sleep, …) — grace, then poll every
interval until idle or a generous cap; falls back to the fixed wait only when the
server can't report status (None). debounce_seconds is now that fallback; new
max_wait_minutes caps the poll.
Seam tests for the poll logic (idle/poll/fallback/cap/lost-status), the handler wiring,
and Plex scan-status detection.
A deep scan is the equivalent of music's full refresh — it READS the server's
current state into video.db and prunes what's gone. It should NOT tell Plex to
rescan its disk. The deep-scan action types were wired to auto_video_scan_library
(nudge Plex + read); point them at the read-only auto_video_update_database in
'deep' mode instead. Update-db phase wording no longer says "new" for a full re-read;
deep-scan block descriptions clarify it's a read, not a disk-scan. Registration test
asserts the deep scans route to the read-only handler and never nudge the server.
The deep-scan action types weren't selectable builder actions, and Scan Video Server
/ Update Video Database had no movie-vs-TV dimension — inconsistent with the rest.
- video_deep_scan_tv / video_deep_scan_movies are now proper builder blocks
(Deep Scan TV/Movie Library), not just system-automation action types.
- video_scan_server + video_update_database gain a media_type ('all'|'movie'|'show')
config + selector, threaded through. The post-download chain carries the scope on
the scan-done event, so a TV-only rescan updates only TV (stage 2 inherits it).
- refresh_video_server_sections / Plex+Jellyfin refresh_sections scope the server
nudge to the chosen library; auto_video_scan_library now nudges only its library.
- shared normalize_media_type() in sources; update_database skips cleanly when the
singleton scanner is busy. Defaults stay 'all' so existing chains are unchanged.
Seam tests for refresh scoping, scan-server scope+event, update-db scope/inherit/skip.
Switch the two deep-scan system automations from a rolling 7-day interval to
weekly_time at 02:00 server-local — TV Mondays, Movies Tuesdays. Different days
means they never overlap, and a fixed wall-clock time doesn't drift with restarts.
Drop initial_delay (the seeder arms timed system triggers). _fix_deep_scan_schedules
migrates the original interval rows to the weekly schedule (the seeder only creates
rows, never updates a drifted trigger); it skips once trigger_type is weekly_time so
a hand-tuned day/time sticks. Idempotent.
Video twin of music's 'Auto-Deep Scan Library', split in two because Movies and TV
are separate libraries — scanning the TV library must not pull in new movies and
vice-versa.
- scanner: add a media_type param ('all'|'movie'|'show', friendly aliases) that
gates the movies vs shows passes (and their pruning), plus an in_progress busy
guard so the singleton scanner can't be stomped by an overlapping run.
- video_scan_library handler: thread media_type through, skip cleanly when the
scanner is busy, and name only the scanned library in the summary.
- two system automations (owned_by=video, weekly deep scan, staggered start delays):
'Auto-Deep Scan Movie Library' + 'Auto-Deep Scan TV Library'. Distinct action
types (video_deep_scan_movies / _tv) because the seeder keys on action_type; both
reuse the one handler, scoped via action_config.
- builder block gains a Library selector (Movies+TV / Movies / TV) so custom scans
can scope too; card label/icon maps cover the video action types.
Seam tests for scanner scope + busy guard, handler scope + skip, registration set.
The job shipped as a 24h 'schedule' because the system-automation seeder only armed
next_run for interval specs — a 'daily_time' spec sat idle and never fired. The
interval fired reliably but drifted with every restart (5min after startup, then
+24h) instead of a fixed wall-clock time, which is worse for 'today's airings' (you
want it queued overnight).
Fix, the robust way:
- Seeder now arms timed system triggers (daily/weekly/monthly) via next_run_at, not
just interval ones. Event-based triggers still return None and are left alone.
- Spec -> daily_time {time:'01:00'} for fresh installs.
- _fix_airing_automation_schedule migrates the existing 24h-interval row to daily
01:00 (the seeder only creates rows, never updates a drifted trigger). Idempotent.
_finish_run already reschedules daily_time to the next 1am, so it stays pinned.
_fix_video_scan_default set its 'done' flag even on runs where it deleted
nothing, so once the flag latched True the standalone 'Scan Video Library'
system automation survived forever (the row the post-download chain replaced).
Drop the flag entirely — get_system_automation_by_action already matches only
the is_system-seeded row, so the cleanup is safe to run every startup and
no-ops once the row is gone.
Two Sonarr-parity features.
1) Per-episode live tracking. "Grab season" was headless (only a button label
changed); episode rows had a status span that was never populated. Now every
episode ROW shows its own live state — Searching → Downloading % → Downloaded
/ Failed — via epTrack() polling /downloads/status?id, matching the inline
movie tracker. Grab season lights all target rows at once; manual + per-source
auto grabs also light their row; reopening the modal resumes tracking in-flight
episodes (resumeEpisodeTracking via /downloads/active + search_ctx match).
Season batch grabs through the same payload as a manual grab (_pickAndGrab →
sendGrab(buildGrabPayload)).
2) Auto-wishlist airing episodes. New daily automation (video_add_airing_episodes):
reads the calendar for episodes airing TODAY for followed shows, skips owned
ones, adds the rest to the wishlist (idempotent). Handler uses injected seams
(calendar read + wishlist write) so it's unit-tested without a DB/server.
Registered + action block + seeded as a daily system automation (01:00),
owned_by=video.
Superseded by the post-download chain (Auto-Scan Video After Downloads → Auto-Update
Video Database After Scan), which keeps the library fresh without a separate scheduled
scan. Two small changes: drop it from SYSTEM_AUTOMATIONS (no longer seeded), and a
one-time flag-guarded cleanup (_fix_video_scan_default, v3) that DELETES the existing
system row — so it actually stops running, not just hidden. The video_scan_library
action/handler/block stay, so a custom scan automation can still be built later.
Engine seed test updated. 66 automation tests pass.
Mirrors the music flow: a finished download batch → refresh the media server → (after
it indexes) pull the new media into the DB — so a downloaded movie/episode shows as
owned without waiting for the 6h scheduled scan.
Two event-based system automations (owned_by='video'):
- 'Auto-Scan Video After Downloads' (video_batch_complete → video_scan_server)
- 'Auto-Update Video Database After Scan' (video_library_scan_completed → video_update_database)
Pieces:
- core/video/download_events.py: a callback registry (core/video can't import the
engine — isolation). The monitor publishes batch-complete; web_server bridges it to
automation_engine.emit('video_batch_complete', …), like music's web_scan_manager.
- download_monitor: fires the batch-complete event once, when the last in-flight
download finishes (none queued/downloading/searching left).
- video_scan_server handler (stage 1): refresh server, wait a debounce for indexing,
then emit 'video_library_scan_completed' (mirrors music's time-based completion).
- video_update_database handler (stage 2): incremental read (newest-first, stop after
25 consecutive known — same as music).
- blocks: 2 video triggers + 2 video actions (scope='video'); registration; seeds.
kettui: seam tests for both handlers (refresh/wait/emit, incremental read, error paths),
the event registry (idempotent register, isolated failures), and the monitor (fires once
on last completion, never while work remains). 298 automation tests + isolation green.
The scheduled 'Scan Video Library' defaulted to a FULL scan (re-reads everything +
prunes) that fired ~5 min after every app start. A recurring scan should be light:
switched the seed to INCREMENTAL (newest-only, no prune) and pushed the first run a
full interval out so it runs on its 6h cadence rather than right after startup.
One-time migration (_fix_video_scan_default, flag-guarded) corrects existing rows that
still carry the old full+startup default, without overriding a deliberate user choice.
The video side gets its OWN automations at music-side parity, kept separate so
nothing on the music side breaks. First twin: Scan Video Library — tells the media
server to rescan the user's SELECTED video sections (movies/TV, never music), then
reads the result into video.db so freshly-downloaded media shows as owned.
Architecture (scope tags + video twins on the shared engine):
- Handler core/automation/handlers/video_scan_library.py — pure function with
injected I/O (server_refresh / run_video_scan); production lazily binds
refresh_video_server_sections() + the video scanner. Owns its own progress.
Lives on the SHARED automation side so it may import core.video (isolation only
forbids core/video & api/video from importing music, not the reverse).
- blocks.py gains a 'scope' tag ('both' generic / 'video' video-only / absent=music)
+ blocks_for_scope(). The music /api/automations/blocks now filters out video
blocks; new isolated /api/video/automations/blocks serves the video palette.
- automation_engine seeds 'Scan Video Library' (owned_by='video', schedule 6h) so
it appears ONLY on the video Automations page; ensure_system_automations now
honours owned_by + action_config. Music page excludes owned_by='video' rows.
kettui: seam-level tests for every handler path (happy/no-server/scan-error/never-
raises/mode), scope filtering (music excludes video, video gets generics, music
parity preserved), seeding (owned_by + mode), registration drift guard. 39 new
tests; full automation suite (288) + isolation guards green.
Phase 2 of the redesign. The tool that judged quality by extension and auto-dumped
matches into the wishlist is gone; quality scanning is now the reviewed
quality_upgrade repair job.
Removed:
- Frontend: Tools-page Quality Scanner card, its JS handlers/poller/socket listener,
help tooltip + tour entry (webui index.html, core.js, helper.js, wishlist-tools.js).
- Backend: /api/quality-scanner/{start,status,stop} endpoints, the in-memory state +
executor + 1s socket broadcast, the QualityScannerDeps/run_quality_scanner shim.
- core/discovery/quality_scanner.py: the auto-acting worker + deps class (the shared
match/normalize helpers stay — the new job imports them).
Rewired:
- Automation 'start_quality_scan' action now triggers the quality_upgrade repair job
via repair_worker.run_job_now() (AutomationDeps gains run_repair_job_now, drops the
4 scanner fields). Action block's vestigial scope field removed (scope lives in the
job's settings now). NOTE: the 'quality_scan_completed' trigger no longer fires (the
repair job doesn't emit it).
- Updated all automation test _build_deps helpers + conftest tool-progress harness;
deleted the obsolete worker test. 528 affected tests pass; 6123 collect cleanly.
QUALITY_TIERS / _get_quality_tier_from_extension kept (used elsewhere).
Background automations had no session, so get_current_profile_id() fell back to
admin (1) — wrong for a non-admin's scheduled job. Now the engine declares the
automation's owner around handler execution via a contextvar
(core/profile_context.py), and get_current_profile_id() consults it only when
there's NO web request. So:
- a real logged-in request always wins (foreground unchanged),
- admin + system automations are profile 1 → resolve to admin exactly as before
(the 8 admin-owned auto-sync pipelines behave identically),
- only non-admin-owned automations gain their correct identity, deep through the
whole call chain (incl. the per-profile client resolvers) — no threading
profile_id through dozens of signatures.
Reset in a finally so a pooled thread can't leak the override to the next job.
Tests: contextvar set/reset/nested; get_current_profile_id honours the override
only outside a request (a real session still wins); and end-to-end — the engine
runs a non-admin automation as profile 4, an admin one as 1, an explicit trigger
profile overrides the owner, and the context resets even when the handler raises.
27 + 4 tests pass.
Part 2 (next): point the sync handlers' source-playlist READ at
get_spotify_client_for_profile so a non-admin's auto-sync pulls THEIR playlist.
carlosjfcasero: "append" sync mode still recreated the playlist (wiping image +
description) on both the sync-page auto-sync and the Playlist Pipeline. Root
cause: _run_sync_task defaulted sync_mode='replace', and every AUTOMATED caller
omits the mode — auto_sync_playlist (mirrored auto-sync + pipeline), the
iTunes-link sync, and Wing It. So those paths always replaced, ignoring the
user's chosen mode entirely. (Manual sync + the per-source discovery path already
passed a mode, which is why it only bit automated runs.)
Fix: when no mode is passed, _run_sync_task resolves the user's configured global
"Playlist sync mode" (normalize_sync_mode(None, playlist_sync.mode)) — the same
thing _submit_sync_task already does — instead of hardcoding 'replace'. The
global default is still 'replace', so users who never changed it are unaffected;
only those who set Append/Reconcile get the corrected behavior.
Tests: normalize_sync_mode(None,'append')→'append' (and 'replace' unchanged);
auto_sync_playlist must not force a mode (no sync_mode kwarg / no 7th positional)
so the resolution can happen. 896 sync/automation/discovery/playlist tests pass.
SoulSync standalone matches library tracks without Plex fetchItem,
reports missing counts correctly, and skips server playlist writes.
Automation re-syncs when the mirror grows; after sync finishes, starts
organize download (organize-by-playlist) or wishlist processing.
UI: Spotify URL playlist-folder controls, organize toggle layout in the
discovery modal, reload organize preference when reopening Download Missing.
Co-authored-by: Cursor <cursoragent@cursor.com>
Persist organize_by_playlist on mirrored playlists and run playlist-folder
downloads from the auto-sync pipeline instead of the global wishlist phase.
Register SoulSync library rows after playlist-folder post-processing, route
failed organize batches to the wishlist correctly, and skip sync-time
unmatched wishlist only when organize download handles retries.
Invalidate stale playlist track caches on refresh (Spotify and Deezer ARL),
re-mirror on refetch, and improve standalone playlist modals (re-analysis,
Open in Mirrored). Add filesystem missing-track detection and tests.
Co-authored-by: Cursor <cursoragent@cursor.com>
Pipeline-driven Auto-Sync runs against any ListenBrainz playlist
(Weekly Jams, Weekly Exploration, Top Discoveries, etc.) would sit
on ``Refreshing: "<name>"`` with no UI updates for 5-7 minutes
before the pipeline progressed. Two real bugs stacked:
1. **Double discovery.** The refresh handler called
``_maybe_discover`` (matching engine, per-track Spotify/iTunes/
Deezer matches) inline for any source returning
``needs_discovery=True`` tracks. Phase 2 of the pipeline then
ran the SAME matching engine via ``run_playlist_discovery_worker``
on the same tracks. The refresh-side run blocked the loop with
zero progress emission; Phase 2's already has the timed
progress-poll pattern. So LB tracks discovered twice, the first
time silently.
Pipeline now sets ``skip_discovery=True`` on its refresh config.
The handler honors the flag and lets Phase 2 handle discovery
end-to-end. Standalone callers (Sync-page tab, registration
action) leave the flag unset so they still get matched_data
on refresh.
2. **No targeted LB refresh.** The LB adapter's ``refresh_playlist``
called ``manager.update_all_playlists()`` — the only refresh
entry-point the manager exposed — which re-pulls every cached
LB playlist's details from the API (~12+ round-trips) even
when only one playlist needed refreshing. Wasteful;
tax-on-everyone for one-playlist work.
Added ``LBManager.refresh_playlist(mbid)`` — reads the cached
playlist_type, fetches just that playlist's details, runs the
normal ``_update_playlist`` upsert path. Defaults type to
``user`` for un-cached mbids so new-playlist discovery still
works. Skips ``_cleanup_old_playlists`` and
``_ensure_rolling_mirrors_from_cache`` (wasted work for a
single-playlist refresh).
Also: killed a silent ``except Exception: pass`` in the LB
adapter's old refresh wrapper that was masking every LB API
failure as a stale-cache hit. Refresh errors now log with full
traceback at warning level and propagate ``None`` so the outer
handler at ``refresh_mirrored.py:104`` counts the error and
surfaces it to the run-history error tally.
Pinned with 12 new unit tests across:
- ``tests/test_listenbrainz_manager.py`` (8): targeted refresh
happy path, unauthenticated guard, empty-mbid guard, upstream
``None`` return, default playlist_type for unknown mbid,
exception propagation, cost guard skipping cleanup, skipped-
when-unchanged signal
- ``tests/test_playlist_sources_adapters.py`` (3): adapter uses
targeted call (not legacy), adapter returns ``None`` on manager
error (not silent swallow), adapter resolves synthetic series
ids before calling the manager
- ``tests/automation/test_handlers_playlist.py`` (1):
skip_discovery flag bypasses ``_maybe_discover`` end-to-end
PR 1 (commit 6ad85e27) shipped the ``next_run_at`` pure function as
foundation plumbing. PR 2 wires the engine through it and adds
``monthly_time`` as a real registered trigger type. After this PR
``core/automation_engine.py`` no longer has its own datetime
arithmetic for daily / weekly schedules — every next-run computation
flows through one function with one set of defensive fallbacks.
Net user-visible change: zero (no UI surface for monthly_time yet —
that's PR 3). New ``monthly_time`` trigger is reachable only via
direct API for now.
**Engine refactor:**
- ``_finish_run`` — collapsed three inline branches (daily_time
arithmetic, weekly_time arithmetic, fallback schedule arithmetic)
into a single ``next_run_at(...)`` call with ``_dt_to_db_str``
normalising the aware-UTC result to the engine's naive-UTC string
convention. Retry-delay short-circuit preserved. Exception
swallowing preserved (logged at debug, writes None next_run).
- ``_setup_daily_time_trigger`` + ``_setup_weekly_time_trigger`` +
new ``_setup_monthly_time_trigger`` — three near-identical methods
collapsed into one ``_setup_timed_trigger`` skeleton. Each public
method is now a one-line dispatch passing trigger_type to the
shared helper with a human-readable label for the debug log.
- Existing ``_next_weekly_occurrence`` deleted — its logic now lives
in ``core/automation/schedule.py:_next_weekly`` (lifted in PR 1).
- New ``_dt_to_db_str(dt)`` module-level helper normalises aware-UTC
→ naive-UTC string. Centralised so a tz mistake here surfaces in
one place. Aware non-UTC datetimes converted to UTC first
(defensive against a future bug that passes the wrong tz).
- New ``_resolve_system_default_tz()`` reads the server's local IANA
tz via ``tzlocal``. Cached at module import (the host's tz doesn't
change while the process runs). Falls back to UTC when ``tzlocal``
is missing — defensive for minimal Docker images.
- New ``self._default_tz`` engine attribute reads from
``automation.default_timezone`` config first, falls back to the
system-detected IANA name. Override path lets users on weird
setups pin a specific tz without touching env vars.
**Convergence fix (intentional behaviour change):**
Old ``_setup_daily_time_trigger`` / ``_setup_weekly_time_trigger``
didn't check the DB for an existing future ``next_run`` — they'd
recompute from scratch on every engine startup, overwriting manual
edits or pending retries. The interval path (``_setup_schedule_trigger``)
already had this check. The new shared ``_setup_timed_trigger``
brings daily / weekly in line: existing-future next_run wins over
freshly-computed delay. Treat this as a correctness fix, not a
breaking change — the old behaviour was an inconsistency, not a
deliberate choice.
**Backward-compat:**
- Existing ``schedule`` / ``daily_time`` / ``weekly_time`` rows
continue to work unchanged. The ``_trigger_handlers`` registry
keeps every historic key.
- Existing rows without an explicit ``tz`` field use
``self._default_tz`` (server-local IANA via ``tzlocal``) —
preserves "every Monday 09:00 server-local" behaviour on
non-UTC servers. Pre-fix the engine used naive
``datetime.now()`` which is also server-local; net effect is
identical wall-clock time, just routed through a tz-aware
pipeline that handles DST correctly (the May 2026 "next in 8h"
bug fix class).
- Engine boots even when ``tzlocal`` is missing — the resolver
falls back to UTC silently. Existing tests would catch a hard
dependency on tzlocal here.
**``tzlocal>=5.0`` added to requirements.txt** alongside
``tzdata>=2024.1`` from PR 1. Both libraries are small and stable;
``tzlocal`` returns a clean IANA name across Windows / Linux /
Docker, sidestepping the platform-specific tz detection mess.
**Tests:** 20 new in ``tests/automation/test_engine_schedule_integration.py``:
- ``_dt_to_db_str`` x3 (aware UTC, aware non-UTC converted to UTC,
naive assumed UTC)
- ``_resolve_system_default_tz`` x2 (returns IANA string, falls back
to UTC without tzlocal)
- ``_finish_run`` dispatch through next_run_at for each trigger type
(schedule, daily_time, weekly_time, monthly_time)
- Retry-delay short-circuits next_run_at
- next_run_at returns None → DB next_run cleared
- next_run_at raises → engine swallows + writes None
- Event triggers skipped (no scheduled next-run)
- ``self._default_tz`` passed through to next_run_at
- monthly_time registered in _trigger_handlers
- All historic trigger types kept registered
- ``_setup_monthly_time_trigger`` arms timer + writes DB
- ``_setup_timed_trigger`` honours existing future DB next_run
- Skip-with-log when next_run_at returns None
- End-to-end no-mock smoke for monthly_time
260 automation suite tests pass; the 240 from PR 1's branch plus 20
new integration tests. Ruff clean.
No WHATS_NEW entry — UI doesn't expose monthly_time yet (PR 3),
and the backward-compat path preserves existing daily/weekly
schedule timing.
Self-review pass on ec4a55c1 — applying the standing kettui-grade
rule (see memory/feedback_always_build_kettui_grade.md). Three issues
that would have surfaced on review:
1. Silent tz fallback to UTC
``_resolve_tz`` returned UTC when the IANA name was unknown — no
log, no warning. User on a host without ``tzdata`` who configures
``America/Los_Angeles`` got schedules running silently at UTC
offset with no way to debug. Now logs WARNING once per unknown
name (deduped via ``_UNKNOWN_TZ_WARNED`` set so a misconfigured
row doesn't spam every poll cycle) and the log line names BOTH
real causes — typo or missing tzdata — so the user can fix from
a single grep.
2. ``weeks`` unit drift from engine
I added ``'weeks': 86400*7`` to ``_INTERVAL_MULTIPLIERS`` but the
engine's existing ``_calc_delay_seconds`` only recognises
minutes/hours/days. Until PR 2 collapses both paths through this
function, any row whose config snuck through with ``unit='weeks'``
would get scheduled by the engine as 1-hour and by this function
as 7-day — drift between two live implementations. Dropped
``weeks`` from the map to match the engine. Added a comment
pinning the map to the engine's contract and a regression test
that asserts ``unit='weeks'`` falls back to the same hours
default the engine produces.
3. DST edge cases unverified
The module docstring claims DST-aware via ``zoneinfo`` but no test
pinned the spring-forward gap (02:30 LA on DST-Sunday doesn't
exist) or fall-back ambiguity (01:30 LA on fall-Sunday happens
twice). Three new tests:
- ``test_dst_spring_forward_lands_after_the_gap`` — pins that the
function doesn't crash + lands on a real instant past ``now``.
- ``test_dst_fall_back_handles_ambiguous_local_time`` — pins
zoneinfo's default-earlier-instant resolution for ambiguous
local times (01:30 PDT vs 01:30 PST → picks PDT).
- ``test_weekly_across_dst_boundary_keeps_local_wall_clock`` —
pins that a "every Sunday at 09:00 LA" schedule keeps the
local wall clock across the boundary even though the UTC
equivalent shifts by an hour. This is the exact bug class
that caused the May 2026 "next in 8h" tz mismatch.
Also loosened ``tzdata==2026.2`` to ``tzdata>=2024.1``. IANA tz data
changes a few times a year for real-world DST policy updates; pinning
to one snapshot would freeze the app's tz knowledge to the build date
and miss future government-mandated rule changes.
41 schedule tests pass (5 new); 240 across the full automation suite.
Ruff clean.
Backend plumbing for upcoming weekly + monthly Auto-Sync schedules.
PR 1 of 4 in the schedule-types feature — see
``memory/project_auto_sync_schedule_types.md`` for the full plan.
Net behaviour change in this PR: zero. The automation engine still
computes next_run via its existing inline ``_calc_delay_seconds`` /
``_next_weekly_occurrence`` helpers; this module is unused until PR 2
wires the engine through. Lands separately so the foundation can sit
on dev for a beat before the engine change.
``core/automation/schedule.py:next_run_at(trigger_type, trigger_config,
now_utc, default_tz)``:
- Pure function. ``now_utc`` injected (tests freeze time without
monkeypatching ``datetime.now``); ``default_tz`` injected (so daily /
weekly / monthly schedules compute against the USER's timezone, not
the server's — the same class of bug that produced the May 2026
"Auto-Sync next in 8h" timezone fix).
- Returns aware-UTC ``datetime`` ready to serialise to the DB
``next_run`` column, or ``None`` for unrecognised / event-based
triggers (callers should not write a next_run for those).
- Naive ``now_utc`` inputs are assumed UTC for defensive symmetry
with the engine's DB-string parser convention.
Trigger types covered:
- ``schedule``: ``{interval: N, unit: 'minutes'|'hours'|'days'|'weeks'}``
— matches engine's existing ``_calc_delay_seconds``. Unknown unit
defaults to hours; zero/negative interval clamps to 1 (preserves
the engine's guard against scheduling for the past); non-numeric
interval falls back to 1.
- ``daily_time``: ``{time: 'HH:MM', tz: '<IANA>'}`` — DST-aware via
``zoneinfo``; ``tz`` falls back to ``default_tz``; unknown IANA
string falls back to UTC; garbage ``time`` falls back to 00:00.
- ``weekly_time``: ``{time, days: ['mon',...], tz}`` — empty / all-
invalid ``days`` list means "every day" (matches engine fallback);
abbreviations case-insensitive; 8-day scan finds the next match.
- ``monthly_time``: ``{time, day_of_month: 1-31, tz}`` — NEW shape.
Day clamped to [1, 31]. Months too short for the target day clamp
to the LAST valid day rather than skipping a month (standard cron
convention; running a day early in February is less surprising
than missing the whole month). 12-iteration loop cap so a
pathological config can't infinite-loop.
Tests (36 cases, all passing):
- Interval: every unit, unknown-unit fallback, zero/negative/garbage
interval clamp, tz field ignored on interval (wall-clock-independent).
- Daily: today-at-future-time runs today, today-at-past-time rolls to
tomorrow, exact-match rolls to tomorrow (no schedule-now-then-schedule-
again-immediately), user-tz vs server-tz, default_tz fallback,
garbage time / unknown tz defensive returns.
- Weekly: same-day-still-future qualifies, same-day-past rolls to next
allowed day, wraps across week boundary, empty days = every day,
garbage abbreviations dropped, case-insensitive, tz across day
boundary (LA Wednesday evening is Thursday UTC).
- Monthly: target day this month, rolls to next month when passed,
Feb 31 → Feb 28 / Feb 29 leap year, day_of_month above 31 / below
1 clamp, Dec → Jan year roll, user-tz pre-midnight edge case.
- Result-shape contract: every returned datetime is aware UTC at
offset zero (engine relies on this when serialising to the
``next_run`` string column).
Added ``tzdata==2026.2`` to requirements.txt. Windows ``zoneinfo`` and
minimal Docker base images ship without the system tz database;
without ``tzdata`` ``ZoneInfo('America/Los_Angeles')`` raises
``ZoneInfoNotFoundError`` and the helper silently falls back to UTC.
No WHATS_NEW entry — no user-visible behaviour change in this PR.
PR 2 (engine wire-through) will land the user-facing changelog entry
when ``monthly_time`` becomes a real schedulable trigger.
Adds ``discover_tracks(tracks) -> List[NormalizedTrack]`` to the
PlaylistSource interface. Sources whose tracks already carry
provider IDs (Spotify, Tidal, Qobuz, YouTube, Deezer, Spotify
public, iTunes link, SoulSync Discovery) inherit a no-op default;
ListenBrainz + Last.fm override to run the matching engine.
This closes the last gap before LB / Last.fm / SoulSync Discovery
can land as Sync-page mirror sources: the refresh handler now
calls ``source.discover_tracks(...)`` whenever a source returns
tracks with ``needs_discovery=True``, so mirrored LB rows arrive
already discovered + ready for the sync pipeline. Previously, LB
playlists ran through a separate state-machine worker tied to the
Discover-page UI, with results stored in ``discovery_cache``
instead of ``mirrored_playlist_tracks.extra_data``.
Changes:
- ``core/playlists/sources/base.py`` — PlaylistSource switches from
Protocol to ABC so a concrete default for ``discover_tracks``
can live on the base class. The four real-work methods stay
``@abstractmethod``; instantiating an adapter that forgets one
fails loudly at construction.
- ``core/discovery/matching.py`` (new) — pure ``match_mb_tracks``
helper that runs Strategy-1-only matching-engine queries against
Spotify (primary) or iTunes (fallback). No state machine, no
discovery-cache writes, no wing-it stub — that richer flow stays
in ``core/discovery/listenbrainz.py`` for the Discover-page UI.
- ``ListenBrainzPlaylistSource`` + ``LastFMPlaylistSource`` take
an optional ``discover_callable`` constructor arg. Last.fm reuses
the LB implementation since the track shape is identical.
- ``bootstrap.build_playlist_source_registry`` accepts a
``discover_callable`` kwarg and wires it into LB + Last.fm
adapters.
- ``web_server.py`` boot constructs the discovery callable from the
existing matching engine + ``_discovery_score_candidates`` +
Spotify / iTunes clients, passes through to the registry.
- ``refresh_mirrored.py`` adds a small ``_maybe_discover`` helper
that calls ``source.discover_tracks(...)`` between fetch and
``to_mirror_track_dict`` projection — only fires when at least
one track has ``needs_discovery=True``, so the normal Spotify /
Tidal / etc. refresh path stays a zero-cost pass-through.
Tests:
- 5 new adapter tests: default no-op pass-through, LB discovery
with mixed matches/misses, LB no-callable fallback, Last.fm
shares the LB implementation, mirror-dict spotify_hint emit.
- 1 new automation test: end-to-end LB refresh with a stub
discover_callable proves the matched_data lands in
``mirror_playlist_tracks.extra_data`` after the registry
refresh + discover hop.
225 tests across adapter + automation suites green.
Phase 1a of the Discover-to-Sync unification. The mirrored-playlist
refresh handler used to branch per-source through a ~190-line
if/elif chain (Spotify, Spotify public, Deezer, Tidal, YouTube).
Each branch hand-built its own ``extra_data`` JSON for the matched-
data block. With every new source we considered for Sync-page mirror
support (ListenBrainz, Last.fm radio, SoulSync Discovery, iTunes
link), that chain would have grown a new elif.
This commit lifts the per-source logic into the existing adapter
layer and collapses the dispatch to a registry lookup:
- ``core/playlists/sources/deezer.py`` — new adapter so the registry
covers every source the refresh handler previously branched on.
- ``core/playlists/sources/bootstrap.py`` — single helper that builds
a populated registry from injected getter callables. Both
``web_server.py`` boot and the automation test fixtures call it,
so the two construction paths can't drift.
- ``core/playlists/sources/base.py`` — ``to_mirror_track_dict``
projection helper centralises the NormalizedTrack → DB-row
conversion (including the discovered/matched_data and
spotify_hint extra_data shapes the downstream sync + wishlist
consumers already expect).
- Spotify adapter now populates ``extra['discovered']`` + an
``extra['matched_data']`` block when fetching via the authed API,
so Spotify mirrors keep landing pre-discovered (matches the
pre-refactor contract pinned by
``test_spotify_refresh_writes_to_db``).
- Spotify-public adapter populates ``extra['spotify_hint']`` so the
discovery worker can skip its search step and jump straight to
enrichment for the known track ID.
- All artist-name fields now project to first-artist-only across
every adapter — matches the pre-refactor mirror_playlist DB shape
(``t.artists[0]``).
``refresh_mirrored.py`` shrinks ~190 → ~80 lines and keeps:
- the file/beatport unrefreshable-source filter,
- URL extraction from ``description`` via ``require_refresh_url``
for spotify_public + youtube,
- the Spotify-public → authed-Spotify fallback when the user is
signed in (handler-level branch, not in any adapter),
- the Tidal-not-authenticated soft-skip log (skip, not error),
- existing-extra_data preservation across refreshes,
- the ``playlist_changed`` automation event emit on track-set delta.
Test scaffolding:
- ``_build_deps`` in ``tests/automation/test_handlers_playlist.py``
now builds a default registry from the passed clients via
``build_playlist_source_registry``, so existing refresh tests
exercise the same path without per-test changes. New tests cover
Tidal-not-authed soft-skip, Deezer refresh writes plain tracks,
YouTube refresh reads URL from description, and Spotify-public
uses authed Spotify when signed in.
- 4 new adapter tests for Deezer projection +
``to_mirror_track_dict`` (minimal track, Spotify matched_data,
Spotify-public spotify_hint).
- ``playlist_source_registry`` field on ``AutomationDeps`` defaults
to ``None`` so the other 5 automation test files (which don't
exercise refresh_mirrored) keep working unchanged.
220 tests across automation + adapter suites green.
The Auto-Sync schedule board was detecting its own automations by
checking `group_name === 'Playlist Auto-Sync' || name.startsWith('Auto-Sync:')`.
That's fragile — renaming the row from the Automations page silently
hands ownership back to the read-only Automation Pipelines tab and the
board stops managing it.
This commit replaces the string convention with an explicit
`automations.owned_by` TEXT column:
- Migration `_add_automation_owned_by_column` adds the column and
backfills `'auto_sync'` for existing rows that match the legacy
`group_name`/`name`-prefix pattern, so users running the migration
don't lose their schedules.
- `database.create_automation` and `database.update_automation` accept
`owned_by` (the latter via its `allowed` kwarg set).
- `core/automation/api.py` forwards `owned_by` on both POST and PUT.
Missing field is left as None, preserving today's behavior for every
caller that doesn't opt in.
- The Auto-Sync schedule board posts `owned_by: 'auto_sync'` and the
detection helper now prefers that signal, falling back to the legacy
name/group convention so any hand-rolled rows still show up.
Tests: three new cases in `tests/automation/test_automation_api.py`
covering create-with-owned-by, create-without (defaults to None), and
update set/clear. The fake DB grew the matching kwarg.
`tests/automation/test_automation_api.py` gains three update_automation
tests covering the schedule-shape reset:
- trigger_config change blanks next_run
- trigger_type change blanks next_run
- non-trigger field (name) leaves next_run alone
`tests/sync/test_sync_candidate_pool.py` is new — nine tests for the
lazy artist track pool in PlaylistSyncService:
- candidate_pool=None disables pooling and skips the DB call
- first lookup for an artist fetches and caches
- second lookup for the same artist reuses the cache (zero DB calls)
- empty result still cached so the next call short-circuits without SQL
- defensive None return coerced to []
- search_tracks exception returns None and does NOT poison the cache
- pool key is normalized so casing variants share a single fetch
- different artists get separate pool entries
- server_source plumbing survives the trip to search_tracks
All assertions go through fakes / MagicMock — no real DB, no
web_server.py import, no AST-parsing.
Expose playlist-native run and status endpoints that reuse the shared mirrored playlist pipeline engine while routing progress into playlist UI state.
Add a Run Pipeline action to mirrored playlist cards and modals with live status polling, and make the shared pipeline lock atomic for manual and scheduled callers.
Centralize mirrored playlist source reference normalization so edited links and IDs are stored consistently. Preserve URL-backed refresh refs, surface missing-source refresh failures, count background sync failures in pipeline summaries, and retry guarded automation skips after a short delay instead of losing a scheduled run. Add focused coverage for source refs, mirrored playlist source updates, refresh failures, and guarded retry behavior.
Reproduced: selecting Fresh Tape (or any kind never generated before)
and running the pipeline silently skipped — UI showed
"No tracks in Fresh Tape — skipping sync" with no clue why.
Root cause: ensure_playlist auto-creates the playlist row on first
access with `track_count=0` and `last_generated_at=NULL`, but
`is_stale=0` by default (the column default — fresh rows aren't
"stale", they're "never generated"). Pipeline only refreshed when
`is_stale=True` OR `refresh_first=True`, so first-run rows fell
through both branches → read the empty snapshot → skip.
Fix: pipeline now also refreshes when `existing.last_generated_at is
None`. Same control flow, one extra condition:
if refresh_first OR is_stale OR last_generated_at is None:
refresh
else:
read existing snapshot
This is the right signal: "has the generator ever run for this row"
is exactly what `last_generated_at` tracks (the column is set in
`_persist_snapshot` after every successful refresh).
Stubs in test_handlers_personalized_pipeline.py updated to expose
`last_generated_at` on their SimpleNamespace returns so the new
attribute read doesn't AttributeError. Fresh stubs get a non-None
timestamp so they're treated as already-generated; the new test
`test_never_generated_snapshot_triggers_first_refresh` pins the
first-run-forces-refresh behavior with `last_generated_at=None`.
Snapshots now track when their source data changes. Watchlist scan
emits stale flags on the playlists whose underlying pool just got
refreshed; the next pipeline run sees the flag and regenerates the
snapshot before syncing, so the server playlist never lags the source.
Schema:
- new `is_stale INTEGER NOT NULL DEFAULT 0` column on
`personalized_playlists`, plus an idempotent ADD COLUMN migration
in `ensure_personalized_schema` for installs created before this PR.
- `PlaylistRecord.is_stale: bool = False` exposed on the dataclass so
callers can branch on freshness without re-querying.
Manager:
- new `mark_kinds_stale(kinds, profile_id=None)` flips the flag in
bulk for a list of kinds (used by upstream data refreshers).
- `_persist_snapshot` clears `is_stale = 0` on successful refresh.
- SELECT statements + `_row_to_record` updated to read the column
(with tuple-form length guard for safety).
Pipeline:
- `_build_payloads_for_kinds` now branches: refresh_first=True OR
`existing.is_stale` -> refresh_playlist, else read existing
snapshot. So the auto-refresh kicks in without needing the user to
toggle the refresh-each-run option.
Watchlist scanner emits stale flags at three sites:
- after `update_discovery_pool_timestamp` -> marks pool-fed kinds
stale: hidden_gems, discovery_shuffle, popular_picks, time_machine,
genre_playlist, daily_mix.
- after release_radar `save_curated_playlist` -> marks `fresh_tape`.
- after discovery_weekly `save_curated_playlist` -> marks `archives`.
All three calls go through a module-level `_mark_personalized_kinds_stale`
helper that builds a PersonalizedPlaylistManager with `deps=None` (only
DB access is needed for the flag update — no generator dispatch). Each
call is wrapped in try/except so a flag failure can never abort the
scan itself.
Tests:
- new `TestStaleFlag` class in `test_personalized_manager.py` (6
tests): default-false, single-kind flip, multi-kind, profile
scoping, refresh-clears, empty-list noop.
- two new pipeline tests pin the auto-refresh dispatch:
`test_stale_snapshot_auto_refreshes_even_without_refresh_first`
and `test_non_stale_snapshot_skips_refresh`.
- existing stub-manager `SimpleNamespace` returns gained
`is_stale=False` so the new attribute read doesn't AttributeError.
Full suite: 3391 pass.
User-facing WHATS_NEW entry added under 2.5.2 (above the prior
pipeline auto-sync entry) describing the auto-refresh behavior.
The action was registered + the block declared, but the automation
builder's per-action config renderer didn't have a case for
`personalized_pipeline` so users only saw the bare card with the
generic delay-minutes input — no way to select which playlists to
sync. This commit adds the multi-select picker.
Backend:
- `core/personalized/api.list_kinds(manager=...)` now optionally
takes a manager and includes the resolved variant list per kind
(calls each spec's variant_resolver(deps) when present). Singleton
kinds get an empty `variants` list. Variant-bearing kinds
(time_machine / genre_playlist / daily_mix / seasonal_mix) get
their full enumerated set.
- `web_server.py` `/api/personalized/kinds` route now passes a built
manager so the variants list lands in the response.
Frontend:
- `webui/static/stats-automations.js` `_renderBlockConfigFields`
gains a `personalized_pipeline` branch that renders a scrollable
multi-select picker:
- Singletons (Hidden Gems, Discovery Shuffle, Popular Picks,
Fresh Tape, The Archives) = one checkbox row per kind
- Variant kinds = a section header + one checkbox row per variant
(e.g. Time Machine: 1960s/1970s/.../2020s; Seasonal: halloween/
christmas/valentines/summer/spring/autumn)
- Pre-checks rows that match the existing `kinds` config on edit
- New `_autoLoadPersonalizedKinds(slotKey)` fetches `/api/personalized/kinds`
(cached after first load), renders the picker DOM, and pre-checks
saved selections via `data-kind` / `data-variant` attributes on
the checkboxes.
- `_renderBuilderCanvas` calls the loader for any `cfg-*-kinds-picker`
it finds in the freshly-rendered slots.
- The save-time `_collectActionConfig` walks the picker's checked
inputs (matched by `data-kind` attribute) and emits
`{kinds: [{kind, variant?}, ...], refresh_first, skip_wishlist}`
in the same shape the handler expects.
Tests:
- `tests/automation/test_automation_blocks.py::_FIELD_TYPES` adds
'personalized_playlist_select' so the block-shape regression test
accepts the new field type. (Test was failing because it whitelists
every field type used across all blocks.)
- 189 automation + personalized API tests pass; full suite intact.
Follow-up to the personalized-playlists standardization PR. New
`personalized_pipeline` automation action syncs selected discover-
page playlists (Hidden Gems / Discovery Shuffle / Time Machine /
Genre / Daily Mix / Fresh Tape / The Archives / Seasonal Mix) to
the active media server + queues missing tracks for download.
Same pattern as the existing mirrored `playlist_pipeline` but two
phases instead of four — no REFRESH (no external source to re-pull)
and no DISCOVER (manager-backed snapshots are already metadata-
matched). Pipeline shape:
SNAPSHOT → SYNC → WISHLIST
Where SNAPSHOT either reads the persisted track list from
`PersonalizedPlaylistManager` (default) or refreshes it first when
`refresh_first=true` (cron use case: regenerate Hidden Gems nightly
and sync the fresh set).
Shared helper extraction:
PHASE 3 (SYNC loop) + PHASE 4 (WISHLIST tail) lifted out of mirrored
`playlist_pipeline` into `core/automation/handlers/_pipeline_shared.py`
as `run_sync_and_wishlist(deps, automation_id, playlists, sync_one_fn,
sync_id_for_fn, ...)`. Both pipelines call it. Mirrored injects
`auto_sync_playlist` as the per-playlist sync function; personalized
injects a thin wrapper that launches `_run_sync_task` directly with
a pre-built tracks_json. Same sync-state polling / progress emission
/ status counting / wishlist trigger logic — 0 duplication.
Files added:
- core/automation/handlers/_pipeline_shared.py
- core/automation/handlers/personalized_pipeline.py
- tests/automation/test_handlers_personalized_pipeline.py
Files changed:
- core/automation/handlers/playlist_pipeline.py: PHASE 3+4 replaced
with shared helper call (~100 lines deleted, 1 helper invocation
added; behavior identical).
- core/automation/deps.py: new `build_personalized_manager` field
(lazy builder so the pipeline gets a fresh PersonalizedPlaylistManager
per run).
- core/automation/handlers/__init__.py + registration.py: register
`personalized_pipeline` action with the shared `pipeline_running`
guard so it can't overlap mirrored.
- core/automation/blocks.py: new `personalized_pipeline` block
declaration with config_fields (kinds multi-select, refresh_first,
skip_wishlist).
- web_server.py: thread `_build_personalized_manager` into
AutomationDeps construction.
- All 5 automation test fixtures: `_build_deps` adds
`build_personalized_manager=lambda: None` stub.
- tests/automation/test_handler_registration.py:
EXPECTED_ACTION_NAMES + EXPECTED_GUARDED_ACTIONS gain
`personalized_pipeline`.
Trigger schema:
{
"_automation_id": "...",
"kinds": [
{"kind": "hidden_gems"},
{"kind": "time_machine", "variant": "1980s"},
{"kind": "seasonal_mix", "variant": "halloween"}
],
"refresh_first": false,
"skip_wishlist": false
}
Tests (14 new, 178 automation total):
- _track_to_sync_shape: basic shape, source ID fallback chain,
no-id returns empty string
- empty config / non-list kinds / empty kinds list all return
error + clear pipeline_running flag
- _build_payloads_for_kinds: skips invalid entries, skips kinds
with no tracks, refresh_first vs ensure dispatch, payload shape
+ sync_id format, manager exception swallowed continues
- _sync_personalized_playlist: launches background thread + returns
status='started'
- happy path: stubbed sync_states drives helper to completion, flag
cleaned up
Full suite: 3383 passed.
Note: the trigger UI block declares config_fields but the frontend
doesn't yet render the `personalized_playlist_select` multi-select
type — usable today via API; polished UI ships in a follow-up
frontend PR.
Per-handler boundary tests pin each handler's body in isolation.
Adding engine-boundary tests that pin the REGISTRATION layer:
- every expected action name registered, no drops, no extras
- guarded actions register a guard, unguarded ones don't
- every registered handler is callable
- every guard returns a bool
- all four progress callbacks registered in the right slots
- progress_init / progress_finish / record_history / on_library_scan_completed
are invocable through the engine's stored callable shape (not just
the bare extracted function)
- finish callback respects _manages_own_progress flag at the engine
boundary too
- library_scan_completed wiring registers a callback on the scan
manager and that callback fires engine.emit when invoked
- every handler returns a `{'status': ...}` dict on a minimal config
trigger -- proves no handler raises into the engine, even when its
guard / short-circuit / error path is the one taken
Uses a minimal _RecordingEngine that captures registrations + a
_RecordingScanMgr that captures completion callbacks. No real
AutomationEngine, no real Flask app, no real DB. The kettui standard
for refactor PRs: don't ship "behavior preserved" claim that's only
validated at the function boundary -- exercise the engine seam too.
EXPECTED_ACTION_NAMES + EXPECTED_GUARDED_ACTIONS frozen sets at the
top: any future drift (rename / drop / add a handler / change which
ones are guarded) fails this test immediately so refactor PRs can't
quietly mutate the registration shape.
13 new tests, 164 automation tests pass total.
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).
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.
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).
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.
The "Clean Search History" automation card kept showing a stale
'DownloadOrchestrator' object has no attribute 'base_url' error
even after the underlying handler bug was fixed in 77d20e9. Root
cause is in the engine, not that handler: AutomationEngine only
captured uncaught exceptions into last_error. Handlers that
report failure by RETURNING {'status': 'error', ...} were treated
as successful from the engine's perspective, so subsequent
gracefully-failing runs never updated the row to reflect the
current state.
Both the timer (run_automation) and event (_handle_event_trigger)
paths now extract the error string from a result whose status is
'error', falling through 'error' -> 'reason' -> 'message' -> a
placeholder so last_error is never None on actual failures
regardless of which key the handler chose. Existing behaviour for
raised exceptions and successful runs is preserved.
Also normalizes _auto_clean_search_history's return key from
'reason' to 'error' so older deployed engines that only check
the canonical key still see the failure.
Adds 7 regression tests covering every result shape the engine
might receive.
