One device holds the audio; the rest watch the same session over an
event stream and act as remotes. Picking a device hands the audio over
at the position the previous one reported, and that device stops.
Also centre the command palette with margins instead of a translate:
animate-rise sets its own transform and dropped the offset on mobile.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Acquisition ran yt-dlp without --embed-metadata, so every download
arrived untagged. The scanner then stored the video id as the title and
"Unknown Artist" as the artist, the vetted-candidate tag check rejected
the mismatch, and all 18 acquired tracks were hidden and retired.
- Pass --embed-metadata so downloads carry real tags.
- Let a scan take fallback title/artist from the candidate, for sources
that still ship untagged files.
- Install Deno alongside yt-dlp: YouTube guards some formats with a JS
challenge yt-dlp must execute, and no other runtime is enabled.
- Dedupe candidates by artist and title. The (source, external_id) key
misses the same song reaching us under two Deezer release ids.
Also carries the in-flight discovery work this builds on: the
Recommendations page replacing Discover, the discovery source service,
and the acquisition spec tests.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The image proxy allowlist covered none of the hosts enrichment writes, so every
album cover answered 403: 305 covers sit on coverartarchive.org (which 302s to
archive.org and on to an ia*.us.archive.org node), 126 on mzstatic, 115 on
dzcdn, 26 on discogs.
Artist images were empty for a different reason — no working source. Fanart
needs a key the worker does not have, TheAudioDB and Discogs 404 on most names,
Wikidata needs an MBID that 638 of 734 artists lack, and Last.fm stopped
serving photos. Deezer needs no auth and its host is already allowlisted.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The director had no idea what hour or season a session started in, so a
22:00 weeknight and a Sunday morning drew from the same pool. The client
now sends localHour, weekday, month and an optional timeZone; the route
validates and bounds all four, the coordinator threads them through, and
the generators use them as scoring signals.
Also exposes GET /library/stats, which the Home page counts read.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
db.service.ts was 2085 lines, of which ~700 were not behaviour at all: the
migration registry, the row-shape interfaces, and the column allowlist. That
makes the file painful to review — the reviewer's note on the MIGRATIONS
array.
Three pure moves into backend/src/db/, which already owns schema.sql:
- migrations.ts (513) — the registry, plus a named Migration type
- types.ts (168) — the row shapes
- updatable-columns.ts (42) — UPDATABLE_COLUMNS + allowedFields
db.service.ts drops to 1378 lines and re-exports ../db/types.js, so existing
`import { Track, ListenerBelief } from '../services/db.service.js'` in the
routes, generators and session-director keeps working untouched.
No behaviour change, and verified as such rather than asserted: the migration
id list and the entire 502-line SQL body diff byte-identical against the
previous commit, backend typecheck is clean and 33/33 tests pass.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The dislike lifecycle promised WARNED -> HIDDEN -> deleted, but nothing ever
removed a file: cleanup.service logged its intent behind
MUZICK_ALLOW_HARD_DELETE and returned, and the two backend delete paths
(hardDeleteTrack, permanentlyDeleteTrack) disagreed about what deletion
meant. The review recommended dropping hard deletion and making HIDDEN
terminal; the owner chose to make deletion real instead.
- cleanup.service performs a true unlink() — no trash directory — for
tracks that have been HIDDEN for a 7-day grace period, then settles the
row. This is the single unlink() call site in the system.
- permanentlyDeleteTrack is the one delete path; hardDeleteTrack is gone.
- a deleted_permanent audit row records what was removed, and
migration 20260730_hard_delete_audit_trail backs it.
MUZICK_ALLOW_HARD_DELETE remains OFF: the docker-compose entry is commented
out, there is no enabling default in code, and the worker's /music bind is
the only writable one. Deletion stays dry-run until the owner opts in
deliberately.
REVIEW-2026-07-30.md open decision: dislike lifecycle.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
db.service inserted the 'deleted_permanent' feedback row and then deleted
the track, but feedback.track_id was ON DELETE CASCADE (verified on the
live DB: confdeltype = 'c'), so the audit row deleted itself. feedback
contains zero deleted_permanent rows.
feedback is an audit log and must outlive its subject: the FK becomes
ON DELETE SET NULL. track_id was already nullable, and nothing in backend/
or workers/ SELECTs from feedback — the only other reference is
mergeTracks()'s UPDATE feedback SET track_id, which re-points to the
survivor — so no caller assumed non-null.
Migration 20260730_feedback_track_id_set_null drops the constraint by
matching confdeltype rather than by name, since the live schema has
drifted. Verified on a scratch PG16: confdeltype flips 'c' -> 'n' and a
deleted_permanent row survives its track's deletion.
Correct under either resolution of the dislike-lifecycle decision, so it
lands independently of it.
REVIEW-2026-07-30.md finding 6 (cascade only).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
updateTrack/updateArtist/updateAlbum built their SET clause from
Object.keys(data) where data is `request.body as any`, interpolating
request-supplied keys straight into SQL as quoted identifiers:
fields.map((f, i) => `"${f}" = $${i + 2}`)
A crafted body key closes the quoted identifier and injects into the SET
list. Mitigated in practice only by the LAN/VPN-only proxy — which supplies
the auth token automatically, so any device on the LAN could reach it from
a browser.
Adds per-table UPDATABLE_COLUMNS plus an allowedFields() helper, applied in
all three methods. The allowlist lives in the service layer rather than the
routes so it covers every caller.
Unknown keys are dropped rather than rejected: the three routes do no error
mapping, so a throw surfaces as a bare 500, and the pre-existing "No fields
to update" error still fires for a payload rejected in its entirety.
Also closes plain mass-assignment. Excluded: path/hash/mtime
(scanner-owned; path is the only link to the read-only bind),
state/quarantined_at/deleted_at (dislike lifecycle and integrity sweep),
play_count/skip_count/dislike_count/last_played_at (learning signal —
forgeable counters poison the engine), and identity/generated columns.
The only callers are the three HTTP PUTs; the frontend's update* service
exports are dead code, so nothing relied on writing an excluded column.
REVIEW-2026-07-30.md finding 5.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
claims declared UNIQUE (..., source, user_id). Every objective claim has
user_id IS NULL, and under default NULLS DISTINCT semantics Postgres treats
those rows as unique, so the ON CONFLICT DO UPDATE / DO NOTHING clauses in
db.service and mb-spine-writer never fired. Re-enrichment inserted a fresh
duplicate every run instead of reinforcing.
Live data: 236 duplicate groups, 2110 excess rows, worst single claim 86
copies, ~15% of 13,910 claims. claim_fusion is SUM(trust * confidence *
recency), so one edge could carry 86x its intended weight — the likely
cause of repetitive recommendations, and almost certainly the root of
d497588 (claim_fusion MV duplicate-key failure).
Migration 20260730_claims_dedup_nulls_not_distinct, two phases in one
transaction. Dedup MUST precede the constraint or adding it fails.
Phase 1 collapses each group into its most recently reinforced row,
carrying forward MAX(last_reinforced_at), MAX(evidence_at) and
MAX(confidence) — reinforcement recency would otherwise be lost by simply
deleting extras. The MAX(...) OVER grp and ROW_NUMBER() OVER ordered
windows are deliberately separate: an ORDER BY inside the window makes the
default frame UNBOUNDED PRECEDING TO CURRENT ROW, which turns MAX() into a
running maximum and would silently keep the wrong confidence.
Phase 2 drops the old constraint by matching its definition rather than its
name, because the live DB has drifted and its autogenerated name is
truncated at 63 characters.
Verified on a scratch PG16 seeded with the old schema plus a 3-row
duplicate group, a distinct-source singleton and a real-user_id row:
UPDATE 3 / DELETE 2, keeper retained the group max of each field from three
different rows, re-run is a no-op, and a subsequent ON CONFLICT DO UPDATE
with user_id = NULL fires correctly.
REVIEW-2026-07-30.md finding 4.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The CTE was `WITH halflives AS (SELECT profile, CASE profile ...)` with no
FROM clause. Postgres rejects it with 42703 (column "profile" does not
exist) on every hourly invocation, so the temporal dimension of the
recommendation engine had never executed once — obsession (14d half-life)
and contextual (7d) never faded.
Rewritten as `WITH halflives(profile, halflife_sec) AS (VALUES ...)`,
half-lives preserved exactly.
One deliberate semantic change: the broken CASE had an `ELSE 30 * 86400`
fallback, so an unrecognised profile would have decayed on a 30-day
half-life. The VALUES join leaves unknown profiles undecayed instead.
Today that is a no-op (only `forgotten`, already excluded by the WHERE),
but a future profile added without a half-life will now conspicuously not
decay rather than quietly decaying at an arbitrary rate.
Verified against a scratch PG16 with one belief per profile aged exactly
one half-life: UPDATE 2, obsession and contextual halved, forgotten and a
fresh longterm untouched. Against the live DB (in a rolled-back
transaction) the fix reports UPDATE 843.
NOTE ON ROLLOUT: the first successful run applies ~23 days of accrued
decay at once, cutting obsession beliefs to ~0.32x. That is correct
behaviour, but recommendations will shift visibly. Expected, not a
regression.
REVIEW-2026-07-30.md finding 3.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Three separate insert paths made a fresh Postgres volume unusable. The live
database only works because its volume predates the constraints.
- scanner.service.resolveOrCreateArtist inserted only (name), but
schema.sql declares canonical_name NOT NULL with no default. Every
artist insert failed, and processFile swallows per-file errors, so a
scan reported success with 0 tracks and a permanently empty library.
- enrichment.service inserted explicitly into artists.normalized_name,
which is GENERATED ALWAYS AS (normalize_artist(name)) STORED:
"cannot insert a non-DEFAULT value into column" (428C9). All
enrichment artist creation failed on a fresh volume.
- db.service.createArtist omitted canonical_name, same failure.
canonical_name holds the raw tag name, not normalize_artist() output,
which truncates on `/` and a standalone `x` ("AC/DC" -> "AC"). That is the
convention createLocalArtist already used. The truncation bug in
artists.name is pre-existing and deliberately left untouched here.
Verified on a scratch postgres:16-alpine with the real schema: the old
statement reproduces the NOT NULL violation, the new path yields
artists/albums/tracks/track_artists rows.
REVIEW-2026-07-30.md finding 1.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The MV SELECTed COALESCE(user_id, zero-uuid) but GROUPed BY raw user_id,
so a global enrichment claim (user_id NULL) and a default-user behavior
claim (user_id = zero-uuid) for the same edge produced two rows that
collide on idx_claim_fusion_pk, breaking REFRESH ... CONCURRENTLY.
Exposed by the #219 listener_behavior same_scene_as/alias_of writes.
New migration rebuilds the MV grouping by the COALESCE'd user_id so the
two fuse into one row.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Per-user SET NX PX lock with Lua CAS release around the
getActivePlan->mutate->setActivePlan span so concurrent prefetch +
feedback requests serialize instead of losing one side's write.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
both generators ran N sequential queries per artist/album.
rewritten with ROW_NUMBER() OVER (PARTITION BY ...) to get per-group
limits in a single round-trip, preserving existing semantics:
- comfortGenerator: up to 2 tracks per artist (was 20 queries → 1)
- deepDiveGenerator: up to 5 tracks per album (was 20 queries → 1)
kami