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>
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>
kami