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).
Symmetric to the post-download reconcile (which handles ADDITIONS): when a
playlist's membership is re-synced (the mirror step — scheduled refresh or the
manual mirror endpoint), rebuild its folder from current membership WITH prune
IF it's organize-by-playlist. So a track that just LEFT the playlist has its
symlink cleaned up the instant membership changes, not only on the next download.
Factored a shared _rebuild_one_from_db (used by the manual 'Rebuild' button and
the mirror hook) + rebuild_mirrored_playlist_if_organized. Gated to organized
playlists, non-fatal at both mirror call sites.
Now the invariant 'folder = the playlist's current owned members' holds on every
change: additions caught at download, removals caught at mirror. 2 new tests
(removed track pruned; non-organized skipped). 985 + 277 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>
The DB-update + deep-scan automation monitor used a hard 2-hour TOTAL cap
(while elapsed < 7200). It tracked progress but only used it to print a stall
warning — the only thing that actually timed out was wall-clock. So a large
library that scans for >2h while progressing fine (reported: 4781 artists) trips
the cap and the automation card flips to 'error: timed out after 2 hours' even
though the scan thread is healthy and still running (the timeout never cancels
it, which is why it keeps progressing in the logs after the 'error').
Time out on STALL, not total runtime:
- 30 min with NO progress -> error ('stalled'); catches a genuinely hung scan.
- 10 min idle -> warning (repeats); unchanged heads-up.
- 24h absolute backstop, purely a runaway-loop guard.
- An actively-progressing scan keeps resetting the idle clock, so it never
times out no matter how many hours the whole library takes.
- Progress is judged on (processed, progress, current_item) so a slow stretch
where the rounded % holds steady (but the artist keeps changing) isn't a
false stall.
The decision is extracted into a pure, testable scan_wait_action(); both the
deep-scan and full-refresh handlers share the monitor loop, so both are fixed.
Tests: tests/test_scan_wait_action.py (9) — headline regression (5h/12h total
but progressing -> 'continue', not timeout), finished/stall-warn/stall-timeout/
abs-cap thresholds, and ordering. 280 automation tests still pass.
Post-incident hardening. A WAL-mode DB corrupted (most likely an interrupted
write during a hard restart), and the backup routine made it unrecoverable:
it (a) never checked integrity, so src.backup() faithfully copied the corrupt
pages into every rolling backup, and (b) pruned oldest-by-mtime, so each new
corrupt backup evicted the last good one. Result: all snapshots poisoned.
New core/db_integrity.py (pure, unit-tested):
- quick_check()/is_healthy(): fast read-only PRAGMA quick_check probe.
- safe_backup(): verifies the SOURCE is healthy BEFORE the Online-Backup copy
and the RESULT after; refuses + discards rather than save a corrupt copy.
- prune_backups(): rotation that NEVER deletes the most-recent verified-healthy
backup, even to honor max_keep — so a run of bad backups can't drop your last
good snapshot.
Wired into BOTH backup paths (the /api/database/backup endpoint and the
auto_backup_database automation handler) — they now refuse on integrity failure
(409 / error status, existing backups untouched) and prune safely.
Tests: tests/test_db_integrity.py (8) using REAL temp DBs incl. a physically
corrupted one — proves refuse-corrupt-source, discard-corrupt-result, and the
exact incident scenario (newest backups corrupt -> the older healthy one is
protected from pruning). Existing maintenance-handler backup test still green
(29 passed). compile + ruff clean.
NOTE: this prevents silent backup poisoning; it does NOT stop the underlying
corruption. Follow-ups still worth doing: WAL-checkpoint on clean shutdown +
a periodic live-DB integrity alert (so corruption is caught on day 1).
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
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.
Three changes folded into one perf+cleanup pass:
1. Indexed fast path for the per-artist pool fetch. The previous
`search_tracks(artist=name)` call hit
`unidecode_lower(artists.name) LIKE ?`, a function-in-WHERE that
can't use `idx_artists_name`. New `MusicDatabase.get_artist_tracks_indexed`
does a two-step lookup: exact-name match (indexed) plus a
case-insensitive fallback, then `tracks WHERE artist_id IN (...)`
via `idx_tracks_artist_id`. Drops per-artist fetch from seconds to
milliseconds for the common case. The sync helper falls back to
the old LIKE-based `search_tracks` only when the indexed lookup
finds nothing, preserving diacritic recall and `tracks.track_artist`
feature-artist matches with zero regression.
2. Public text-normalization helper. Lifted the body of
`MusicDatabase._normalize_for_comparison` into
`core/text/normalize.py:normalize_for_comparison` so callers outside
the database layer (matching engine, sync pool, future import-side
comparisons) don't reach across the module boundary into a
leading-underscore "private" method. The DB method now delegates,
so existing internal call sites stay untouched. Sync's lazy pool
now imports the public helper.
3. Artist-name walker extracted. `_artist_name` at module level in
`services/sync_service.py` replaces two near-identical inline
str-or-dict-or-fallback walkers (one in `sync_playlist`, one in
`_find_track_in_media_server`). Returns `''` for None instead of
the literal string `'None'`.
Plus three small tidies from the same review:
- `_POOL_FETCH_LIMIT = 10000` constant in place of the literal at the
pool-fetch call site.
- Trimmed the verbose docstring + comment block on the pool helper.
- Set-intersection predicate for the trigger-shape reset in
`core/automation/api.py` instead of a two-line `or` chain.
Also removed the duplicate `_get_active_media_client()` call at
sync_service.py:212/214 — pre-existing wart that was sitting in the
same block I was editing.
Tests: 21 new tests across `tests/database/`, `tests/sync/`, and
`tests/text/`, plus updates to the existing pool tests to cover the
new fast/fallback split. Full suite stays green (3953 passing).
The Playlist Auto-Sync schedule board was showing "next in 8h" on every
card regardless of the configured interval. Root cause: backend stores
next_run as a naive UTC string ("2026-05-25 05:00:00") and the new
auto-sync renderer was parsing it with plain `new Date(...)`, which
treats unmarked timestamps as local time. On Pacific time that offsets
the displayed countdown by ~8 hours. Auto-Sync now routes through the
existing `_autoParseUTC` helper that the rest of the Automations page
already uses, so countdowns line up with the wall clock.
A separate correctness fix in the automation update API: when a PUT
changes `trigger_type` or `trigger_config`, the stored `next_run` is
now blanked before the engine reschedules. Previously the scheduler's
restart-survival path would preserve a stale future timestamp from the
prior interval, so dragging a playlist from the 8h column to the 1h
column kept firing at the old 8h mark. Boot-time restart behavior is
unchanged — only user-driven schedule changes reset the clock.
Modal restyle: the Auto-Sync manager's hardcoded sky-blue palette is
replaced with `var(--accent-rgb)` everywhere so the modal honors the
user's chosen accent color. Tinted glow on the modal border, tabbed
header active state, scheduled-playlist chips, scrollbars, and a new
drag-over highlight on columns all follow the accent theme. The
column drag-over state is wired through new ondragleave handling so
the highlight clears reliably when leaving a column.
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.
Extract the all-in-one mirrored playlist lifecycle into core/playlists/pipeline.py so automation becomes a thin adapter. Preserve the existing automation action and behavior while making the pipeline reusable by future direct playlist UI controls.
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.
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.
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.
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
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.
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.
