Commit Graph

21 Commits

Author SHA1 Message Date
bvandeusen f7dfeff256 feat(#392): scan run lifecycle events on the SSE bus
Slice 3c — completes the server-side producer set.

Publishes scan.run_started when InsertScanRun succeeds and
scan.run_finished after FinishScanRun completes (success or with an
error_message payload). Per-file progress events are intentionally NOT
emitted — they'd flood the stream during large library scans without
giving the admin scan card anything actionable. The two-beat signal
gives the UI enough to invalidate scanStatusProvider at the right
moments.

Bus access uses a package-level setter on internal/library because
threading bus through RunScan + TryStartScan + Scheduler + all their
callers would touch ~10 sites without changing behavior at the boundaries
that don't publish. Per-process singleton, matches the log.SetDefault
idiom. cmd/minstrel/main.go calls library.SetEventBus(bus) once at
startup; test contexts that never call it skip publishing safely
(publishScanEvent is a no-op when bus is nil).

This completes the server side of #392. Slice 4 wires the Flutter
consumer: live_events_provider.dart (StreamProvider) +
live_events_dispatcher.dart (event-kind → provider invalidation)
+ AppLifecycleState cold-start invalidation.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-12 21:52:06 -04:00
bvandeusen 0fa7dc7982 feat(server): scanner + DeleteTrackFile write library_changes
Wires sync.LogChange into the library mutation sites so /api/library/sync
reflects upserts and deletes.

Architectural pivot: LogChange's signature is now (ctx, dbq.DBTX, ...) so
it works with both *pgxpool.Pool and pgx.Tx. The scanner doesn't run
mutations in explicit transactions, so it pool-binds; delete.go matches.
Tx-bound callers (likes/playlists in subsequent commits) keep atomicity.

Also: sync.FormatUUID centralizes the pgtype.UUID → canonical string
conversion that both the scanner and the sync handler need; library_sync.go
now uses it instead of a local copy.

Best-effort logging on scanner failures (Warn, don't fail the scan): a
LogChange error after a successful upsert is rare and self-healing — the
next scan that touches the entity re-emits the change.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-09 22:39:58 -04:00
bvandeusen 876f994904 fix(server/m7-recurring-scans): early-return in TryStartScan reap branch
revive flagged the if/else where the else branch was a return — invert
the condition and return the non-stale skip path early so the reap
flow drops one indent level.
2026-05-06 23:03:21 -04:00
bvandeusen 4607ce6db4 feat(server/m7-recurring-scans): Scheduler goroutine + boot wiring
Adds ScheduleConfig + NextFire (per-mode logic for off/interval/
daily/weekly) and the Scheduler struct + loop goroutine. The loop
reads scan_schedule on boot and on Refresh signals, computes
next-fire, sleeps via time.Timer, and calls TryStartScan when the
timer fires. Skip-on-conflict logging happens inside tryFire when
TryStartScan reports started=false with a non-stale in-flight row.

Boot wiring: cmd/minstrel/main.go constructs the scheduler after
coverEnricher and calls Start with the server's long-lived ctx.
The scheduler is not yet plumbed into the HTTP handlers (the next
task does that — handlers gain a scheduler field, admin schedule
endpoints land).

Tests cover the NextFire truth table (off / interval / daily today
+ tomorrow / weekly today-before-time + today-after-time + upcoming
day) plus invalid configs and parseHHMM edge cases. Refresh is
verified non-blocking via the buffered-channel + default pattern.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-06 22:20:56 -04:00
bvandeusen 8784e938bd feat(server/m7-recurring-scans): TryStartScan helper + reaper
Extracts the in-flight check + scan-start logic from handleTriggerScan
into a shared library.TryStartScan helper used by both manual and
(future) scheduled paths. Folds in the lazy 1-hour stuck-scan
reaper: in-flight rows older than StuckScanThreshold get
FinishScanRun-ed with error_message="reaped (stale)" and the new
scan proceeds.

handleTriggerScan becomes a thin wrapper preserving the same
external HTTP behavior (202/409/500). Operators no longer need to
manually clear stuck rows after a server crash — the next manual
trigger reaps it.

Tests gated on MINSTREL_TEST_DATABASE_URL: no-in-flight starts;
fresh-in-flight skips with row pointer; stale-in-flight reaps
(verified via finished_at + error_message); DB error propagates.
2026-05-06 22:15:19 -04:00
bvandeusen 3e9e46f190 feat(server/m7-scan-progress): wire stagePublisher into 4 scan stages
Each of the 4 stage workers (library walk, MBID backfill, cover
enrich, artist art enrich) gains a progressCb parameter that
receives a snapshot of the current running state per item processed.
The orchestrator stores the snapshot in a local var that the
publisher's marshal closure reads at flush time.

scanrun.go's 4 stage blocks are rewritten to construct one publisher
per stage, pass a Tick-firing closure to the worker as progressCb,
and call Flush() after the worker returns (routing any persist error
to captureErr — preserves today's diagnostic behavior).

The previous post-stage UpdateScanRun* calls are removed; the
publisher's Flush handles the final write. Cadence: every 500 items
OR every 1s, whichever fires first.

Outside-orchestrator callers (scanner_test.go, enricher_test.go)
pass nil for progressCb — no-op behavior preserved for tests that
don't need progress observability.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-06 21:14:50 -04:00
bvandeusen 7b9ea131a2 feat(server/m7-scan-progress): stagePublisher helper
Persists a stage tally to scan_runs at a bounded cadence — every
flushEvery items OR every flushPeriod elapsed, whichever fires
first. Caller supplies marshal + persist closures so each stage owns
its own tally type. Tick swallows mid-stage persist errors silently
(observability bug, not scan failure); Flush propagates errors so
the orchestrator's captureErr can surface them.

Tests cover: count-based flush, time-based flush, Flush always
persists, Tick swallows errors and keeps firing, Flush propagates
errors, counters reset after each flush.
2026-05-06 21:10:33 -04:00
bvandeusen 00cd28e79b feat(server,web/m7-cover-sources): scan-orchestrator 4th stage
Adds an artist-art-enrich stage after cover-enrich in RunScan. The
stage drains EnrichArtistBatch with a configurable cap and the
operator's data_dir, persists processed/succeeded/failed tallies to
the scan_runs.artist_art_enrich jsonb column added in migration
0018.

The admin /admin Library Scan card grid expands from 3 to 4 columns
with the new "Artist art" card mirroring the cover-enrichment one
(Processed / Succeeded / Failed). The TS ScanStatus type gains an
optional artist_art_enrich field. The Go scanStatusResp gains the
matching ArtistArtEnrich json.RawMessage field.

cmd/minstrel/main.go (and admin scan-trigger handler) wire the new
RunScanConfig fields: ArtistEnrichCap reuses cfg.Library.CoverArtBackfillCap
for now (can be split if separate budgets are useful), DataDir is
cfg.Storage.DataDir.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-06 13:03:36 -04:00
bvandeusen 4481874482 fix(server,web/m7-382): handle duplicate-MBID conflicts during backfill
Forward-fix from running the prior two commits in production. Backfill
on a real library exposed two further issues:

(1) gofmt -s flagged the column-aligned stubMeta methods in
    mbids_test.go. Switched to the gofmt-canonical single-space form.

(2) Many albums failed to heal with SQLSTATE 23505 (unique constraint
    albums_mbid_unique). Cause: the operator's library has duplicate
    album rows in the DB — same MusicBrainz release, split into
    multiple rows by the pre-MBID scanner because of subtle title or
    artist disagreements between files. When backfill now correctly
    extracts the MBID, two rows want to claim the same one and the
    partial unique index rejects the second.

    The conflict is correct DB behavior — we shouldn't have two rows
    with the same MBID — but it's not a write failure to alarm the
    operator about. Detect 23505 specifically:
      - downgrade the log line from Warn to Info
      - track these in a new BackfillMBIDsResult.Duplicates counter
        (separate from Skipped, which retains its "no MBID in tag"
        meaning)
      - leave the duplicate row's mbid NULL; merging duplicates is a
        separate (future) operator workflow

    Same handling threaded through the scanner heal path so a regular
    rescan doesn't generate the noise either.

    JSON tally + admin UI gain a "Duplicates" line so the operator
    can see how many duplicate rows their library carries.

Follow-up scope (separate task): a duplicate-album merge UX that
reparents the duplicate's tracks to the canonical row and deletes
the orphaned album row.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-04 21:45:15 -04:00
bvandeusen cc12ab7e96 fix(server/m7-382): strip NUL bytes from MBIDs before persist
Follow-up to 947f944. With the dhowden/tag/mbz extractor now correctly
matching ID3v2 TXXX frames, mbid backfill started rejecting every row
with "invalid byte sequence for encoding UTF8: 0x00 (SQLSTATE 22021)".

Cause: dhowden's readTextWithDescrFrame (id3v2frames.go:454) — the path
that decodes TXXX text — does NOT strip the trailing/embedded NUL bytes
that ID3v2.4 uses as frame terminator and multi-value separator. Unlike
readTFrame, which does (line 313). So a TXXX:MusicBrainz Album Id of
"abc-123\x00" comes back to us verbatim, and Postgres refuses to store
NULs in text columns.

cleanMBID splits on NUL and returns the first non-empty segment, which
also handles the multi-value case (collaboration artist IDs are
NUL-separated; Minstrel uses the primary for MBCAA, so first wins).

Tests cover the trailing-NUL, multi-value, and all-NUL paths.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-04 21:35:01 -04:00
bvandeusen 947f944fe9 fix(server/m7-382): MBID extraction never matched dhowden/tag's Raw() keys
The hand-rolled extractor in internal/library/mbids.go looked up keys that
dhowden/tag never produces, for every supported audio format:

  Vorbis (FLAC/OGG): dhowden lowercases keys at parse time (vorbis.go:59),
    we looked up MUSICBRAINZ_ALBUMID. Always missed.
  ID3v2 (MP3): dhowden stores TXXX frames under bare TXXX/TXXX_N keys
    with *tag.Comm values; the Picard tag name is on the value's
    Description field, not part of the key. We looked up
    TXXX:MusicBrainz Album Id. Always missed.
  MP4 (M4A): dhowden stores freeform iTunes atoms under their bare
    sub-name. We looked up ----:com.apple.iTunes:MusicBrainz Album Id.
    Always missed.

Net effect: every album in the m7-380 boot backfill reported
processed=N, healed=0, skipped=N. The m7-379 enricher gate then
short-circuited NULL on every album, so MBCAA was never queried.
Symptom: "all albums skipped" during cover enrichment.

The pre-existing unit tests passed because they hand-built Raw() maps
with the intended-but-incorrect keys, never round-tripping through
dhowden's parser.

Replace the hand-rolled extractor with dhowden's own mbz sub-package,
which knows the per-format Raw() conventions and uses lowercase
canonical keys (mbz.Album, mbz.Artist). Update both call sites to pass
the full tag.Metadata instead of just Raw(). Rewrite the test with a
stubMeta implementing tag.Metadata, asserting against the actual
per-format Raw() shape, plus a regression guard that pins "uppercase
Vorbis keys must not match" so the v0 bug can't sneak back.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-04 21:15:51 -04:00
bvandeusen 65a3c4892c feat(server/m7-381): RunScan orchestrator chains library+backfill+enrich
Adds internal/library/scanrun.go with RunScan, which creates a scan_runs
row and sequences file-walk → MBID backfill → cover enrich, persisting
per-stage tallies as jsonb. Extends coverart.EnrichBatch to return
(processed, succeeded, failed, err) so the orchestrator can classify
outcomes. Replaces main.go's two separate boot goroutines with a single
RunScan call; passes nil scanner in the no-startup-scan branch.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-04 20:08:48 -04:00
bvandeusen 34615fffbd feat(server/m7-380): one-shot MBID backfill worker for existing libraries
Adds a boot-time goroutine that walks albums with NULL mbid, re-reads
tags from one track per album via dhowden/tag, and persists album + artist
MBIDs. Healed albums also get their cover_art_source='none' cleared so the
enricher's next batch retries the MBCAA fetch. Caps at 5000 albums per
boot to avoid stalling startup on huge libraries.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-04 19:40:27 -04:00
bvandeusen 6c6c4bc1e4 feat(server/m7-379): heal existing artist+album mbid on rescan 2026-05-04 19:03:12 -04:00
bvandeusen ab8fff834a feat(server/m7-379): extract MusicBrainz IDs from audio tags during scan
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-04 19:01:55 -04:00
bvandeusen e6e3f297d6 fix(dbtest,library): include quarantine tables in ResetDB; clarify DeleteTrackFile docstring
- dbtest.ResetDB.dataTables now truncates lidarr_quarantine + lidarr_quarantine_actions
  alongside the other M2-M4 data tables. Without this, M5b service tests would
  inherit residual state across runs.
- DeleteTrackFile godoc spells out the post-file/pre-DB failure window
  reconciles via the next library scan; the function is retry-safe by design,
  not atomic.
2026-04-30 17:32:08 -04:00
bvandeusen d0523da520 feat(library): DeleteTrackFile (rm file + tracks row, album/artist preserved) 2026-04-30 17:04:21 -04:00
bvandeusen 4a9193bc52 feat(library): extract track duration via ffprobe
Scrubbing/seeking in clients was a no-op because every track shipped
with duration_ms=0. Shell out to ffprobe (already in the image) per
file during scan and record the parsed duration. ffprobe failures are
warned + recorded as 0 so a single bad file doesn't sink the scan.

Tightened the incremental skip to also require duration_ms > 0 so
existing libraries get backfilled on the next rescan instead of
needing a wipe.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
2026-04-20 08:02:37 -04:00
bvandeusen dcd19c0143 fix(library): drop out-of-range years instead of failing the album
Tag-supplied years were being passed straight to Postgres' date column
without validation. Files with corrupt or 5+ digit years (seen in the
wild on a couple of dozen albums) tripped SQLSTATE 22008 and the entire
album upsert failed, dropping every track on those albums from the
library.

Validate the year is within 1..9999 before constructing the date. If
it's outside that window, log a warning naming the album and year, and
insert the album with no release_date — a soft field shouldn't take
down the whole row.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
2026-04-20 00:14:51 -04:00
bvandeusen c58061778c M1/#293: integration test for scanner walk + incremental 2026-04-19 15:17:09 +00:00
bvandeusen fdf28efef0 M1/#293: library scanner (walk + tag-parse + upsert + mtime incremental) 2026-04-19 15:16:48 +00:00