Repair the 2026-07-30 review findings #1

2026-07-30T22:03:58+02:00

kami commented

2026-07-30 22:03:58 +02:00

One commit per finding from REVIEW-2026-07-30.md, in the review's "repair now" order.

Commit	Finding
populate `canonical_name`, stop writing generated `normalized_name`	1
repair `decayBeliefs` CTE so belief decay actually runs	3
dedup claims, make the unique constraint `NULLS NOT DISTINCT`	5
allowlist updatable columns (SQL injection via column names)	6
stop the `deleted_permanent` audit row destroying itself	9
inject the admin key so the admin UI stops returning 403	2
track a playback `currentIndex` so prev and repeat-all work	7
serialize the per-host throttle (MusicBrainz rate limiting)	8
give the worker a pg `Pool` and real transactions	4
guard the integrity sweep against a dead mount	secondary
make hard deletion of disliked tracks real	open decision

Plus a package-lock.json refresh and the review doc itself.

Verification

git rebase master --exec typechecked backend + workers at every commit
frontend typecheck clean; 33/33 backend tests pass
three new migrations appended contiguously; master's 10 unaltered and in order

Notes for review

The worker's Pool / integrity / hard-delete trio is type-entangled. So each commit compiles standalone, the Pool commit carries only the constructor type change for integrity.service.ts and cleanup.service.ts; their real fixes land in the next two commits. Consequence: at the Pool commit cleanup.service.ts still issues BEGIN/COMMIT against a Pool — wrong, noted in that commit message, replaced wholesale by the hard-delete commit. No intermediate code exists that isn't in the final tree.

MUZICK_ALLOW_HARD_DELETE is off everywhere — commented out in docker-compose.yml, no enabling default in code, worker's /music bind is the only writable one. Deletion stays dry-run until turned on deliberately. The 37 stuck WARNED rows are untouched, as is the live stack.

The dislike-lifecycle decision went against the review's recommendation at the owner's direction: real unlink() (no trash dir) after a 7-day grace period post-HIDDEN, rather than making HIDDEN terminal.

One commit per finding from `REVIEW-2026-07-30.md`, in the review's "repair now" order. | Commit | Finding | |---|---| | populate `canonical_name`, stop writing generated `normalized_name` | 1 | | repair `decayBeliefs` CTE so belief decay actually runs | 3 | | dedup claims, make the unique constraint `NULLS NOT DISTINCT` | 5 | | allowlist updatable columns (SQL injection via column names) | 6 | | stop the `deleted_permanent` audit row destroying itself | 9 | | inject the admin key so the admin UI stops returning 403 | 2 | | track a playback `currentIndex` so prev and repeat-all work | 7 | | serialize the per-host throttle (MusicBrainz rate limiting) | 8 | | give the worker a pg `Pool` and real transactions | 4 | | guard the integrity sweep against a dead mount | secondary | | make hard deletion of disliked tracks real | open decision | Plus a `package-lock.json` refresh and the review doc itself. ## Verification - `git rebase master --exec` typechecked backend + workers at **every** commit - frontend typecheck clean; 33/33 backend tests pass - three new migrations appended contiguously; master's 10 unaltered and in order ## Notes for review **The worker's Pool / integrity / hard-delete trio is type-entangled.** So each commit compiles standalone, the Pool commit carries only the constructor *type* change for `integrity.service.ts` and `cleanup.service.ts`; their real fixes land in the next two commits. Consequence: at the Pool commit `cleanup.service.ts` still issues `BEGIN`/`COMMIT` against a Pool — wrong, noted in that commit message, replaced wholesale by the hard-delete commit. No intermediate code exists that isn't in the final tree. **`MUZICK_ALLOW_HARD_DELETE` is off everywhere** — commented out in `docker-compose.yml`, no enabling default in code, worker's `/music` bind is the only writable one. Deletion stays dry-run until turned on deliberately. The 37 stuck `WARNED` rows are untouched, as is the live stack. **The dislike-lifecycle decision went against the review's recommendation** at the owner's direction: real `unlink()` (no trash dir) after a 7-day grace period post-HIDDEN, rather than making HIDDEN terminal.

kami added 13 commits 2026-07-30 22:03:58 +02:00

fix: populate canonical_name and stop writing generated normalized_name cd46ac397f

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>

fix: repair decayBeliefs CTE so belief decay actually runs cc4199c79e

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>

fix: dedup claims and make the unique constraint NULLS NOT DISTINCT 3de1cfb4ca

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>

fix: allowlist updatable columns to close SQL injection via column names 1e59d21dee

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>

fix: stop the deleted_permanent audit row from destroying itself 3ffba3f24b

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>

fix: inject the admin key so the admin UI stops returning 403 755de34501

The entire admin surface of the SPA had been dead since auth landed
(5ed8d9e / 3bc9f2d). nginx.conf.template injected only
`Authorization: Bearer ${MUZICK_API_KEY}` for all of /api, docker-compose
passed only MUZICK_API_KEY to the frontend container, and app.ts requires
token === adminKey for /api/admin/*. The two keys differ, and
services/api.ts sets no headers of its own.

All 10 admin call sites were affected: the Jobs page polled 403s every
3s/5s forever and rendered a blank Overview with no error state, and every
Settings library action (Scan, Reindex, Reprocess artists, Re-enrich,
Duplicates merge) silently failed.

Three changes, each necessary:
  - a `location /api/admin/` block injecting the admin key
  - the Dockerfile envsubst list widened to include MUZICK_ADMIN_KEY,
    without which the new variable substitutes to empty and the header
    becomes a bare "Bearer"
  - MUZICK_ADMIN_KEY passed to the frontend service in docker-compose

nginx selects the longest matching prefix regardless of block order;
verified empirically in a throwaway nginx:stable-alpine running the real
envsubst output against a stub that echoes $http_authorization:

    /api/admin/queue-stats       -> Bearer ADMINKEY456
    /api/admin/duplicates/merge  -> Bearer ADMINKEY456
    /api/tracks                  -> Bearer APIKEY123
    /api/health                  -> Bearer APIKEY123

All 10 call sites use /admin/... under the axios /api baseURL and none
request bare /api/admin without a trailing slash.

Also gives the Jobs page an error state: a banner that names a 401/403 as a
missing or wrong admin key, a Retry button, "Loading queue stats..." in
place of a blank Overview, and refetchInterval returning false once the
query has errored so it stops hammering a failing endpoint.

Deletes frontend/nginx.conf — unreferenced by the Dockerfile (confirmed by
grep) and the insecure variant of the template.

Worth noting and not addressed here: the outer LAN-only proxy already
forges credentials for everything reaching /api, so this key split buys no
real security while having cost the whole admin surface. Collapsing to one
key would be simpler.

REVIEW-2026-07-30.md finding 2.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

fix: track a playback currentIndex so prev and repeat-all work 543031e48c

next() did `queue.slice(idx + 1)`, so the current track was always
queue[0]. prev()'s `idx > 0` guard could therefore never pass after an
auto-advance — Previous did nothing, ever — and repeat: 'all' jumped to
queue[0], which is the track that just finished, looping the last track of
an album instead of restarting it.

Replaced with a currentIndex cursor; the queue is no longer trimmed behind
the playhead. The old slice did serve a purpose — bounding Vibe-prefetch
growth — so that is preserved as a MAX_HISTORY = 50 cap that drops the
oldest entries and re-bases the index, rather than dropped outright.
setQueue/playTrack/setCurrentTrack recompute the cursor, next()/prev() fall
back to findIndex if it drifts, and shuffle now picks by index so the
cursor stays valid.

Consumer audit: NowPlayingPanel and Vibe.tsx already derived position via
findIndex and needed no change. TrackRow.handlePlay did
`setQueue(queue.slice(index))`, which re-broke prev at the point of click
even with the store fixed; it now passes the intact queue.

This commit also includes a pre-existing uncommitted fix from the working
tree (not authored by Claude): the end-of-queue auto-resume loop, which
stops playback at the end of the queue instead of restarting. It is correct
and independent of the cursor bug, and is preserved verbatim here.

REVIEW-2026-07-30.md finding 7.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

fix: serialize the per-host throttle so MusicBrainz rate limiting holds d0ca479d4f

throttle() read lastRequestAt, awaited delay(), then wrote it back — a
TOCTOU race with no mutex or queue, under concurrency: 10. Ten jobs read the
same timestamp, slept the same duration and fired in the same tick, giving
up to ~10 req/s against MusicBrainz's 1 req/s policy and risking an IP
block.

Replaced the lastRequestAt map with a per-host { lastRequestAt, tail }
limiter; each call links onto that host's promise chain, so the
read-sleep-write critical section is serialized and N concurrent callers
space out by minIntervalMs. Chain rejections are swallowed so one failure
cannot poison the queue. Per-host rather than global, so other integrations
are not starved by MusicBrainz.

Measured: 5 concurrent same-host calls at 200ms -> 802ms (previously all in
one tick); 3 distinct hosts at 1000ms -> 0ms, confirming no cross-host
starvation.

musicbrainz.client caught HttpError and returned null at all 7 catch sites,
making a rate-limited MusicBrainz indistinguishable from "no data for your
library" while every job reported success. A shared logMbFailure() now logs
429 (and 503 whose body mentions a rate limit) at error, stating results are
INCOMPLETE. The error model is otherwise unchanged.

REVIEW-2026-07-30.md finding 8.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

fix: give the worker a pg Pool and real transactions ee43995e96

The worker ran every job through a single pg Client while BullMQ was
configured with concurrency: 10. A Client is one connection with one
protocol stream and no queueing: ten concurrent jobs interleave on it, and
any BEGIN/COMMIT is shared by all of them, so an unrelated job's failure can
roll back another's work and a rollback can discard a third's committed
intent.

Switched to a Pool, added a small withTransaction(pool, fn) helper that
takes a dedicated connection per transaction, and threaded a Queryable
interface through the services so they accept either a pool or a pooled
client. Both reprocess_artists merge blocks — the artist merge and the
duplicate-album merge — now run inside withTransaction; previously a failure
partway through left artists merged and their tracks unmoved.

integrity.service and cleanup.service get only the constructor type change
here so this commit compiles; their own fixes follow in the next two
commits. cleanup.service's BEGIN/COMMIT-on-a-Pool is therefore still wrong
at this commit and is replaced wholesale by the hard-delete commit.

REVIEW-2026-07-30.md finding 4 (and the concurrency note in finding 3).

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

fix: guard the integrity sweep against wiping the library on a dead mount 27e8acc592

The sweep stats every track path and marks unreadable files missing, with no
check that /music is mounted. An unmounted or misbehaving bind would fail
every stat and mark the entire library missing in one pass; the ratio is only
recoverable by a full rescan.

Three guards, cheapest first:
  - liveness: probe a sample of existing track paths before doing anything;
    abort if none are readable
  - ratio: abort mid-sweep if the missing fraction crosses a threshold,
    leaving already-marked rows alone rather than rolling back a partial pass
  - progress: keyset pagination over id with the cursor persisted in
    integrity_sweep_state, so a sweep aborted or restarted mid-run resumes
    instead of re-walking from the top and re-marking

The repair-corrupted-metadata script shares the same failure mode and gets
the same abort path.

REVIEW-2026-07-30.md secondary finding: integrity sweep has no mount check.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

feat: make hard deletion of disliked tracks real 963f845733

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>

chore: refresh frontend package-lock.json 512c3fdb90

Regenerated by a clean npm install; no dependency ranges in package.json
were changed.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

docs: add the 2026-07-30 engineering review and CLAUDE.md, drop AUDIT.md

Typecheck / typecheck (backend) (push) Has been cancelled

Details

Typecheck / typecheck (workers) (push) Has been cancelled

Details

Typecheck / typecheck (backend) (pull_request) Has been cancelled

Details

Typecheck / typecheck (workers) (pull_request) Has been cancelled

Details

2ee9116d4d

REVIEW-2026-07-30.md is the source for the preceding commits. AUDIT.md was
its superseded predecessor.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

kami reviewed 2026-07-30 22:21:36 +02:00

backend/src/services/db.service.ts Outdated

						
				@@ -506,1 +552,4 @@

				  },

				  {

				    id: '20260730_claims_dedup_nulls_not_distinct',

				    sql: `

kami commented

2026-07-30 22:21:36 +02:00

might be a good idea to extract this in separate file.

kami commented

2026-07-30 22:34:02 +02:00

Done, two extraction commits pushed.

db.service.ts 2085 -> 1398 — three pure moves into backend/src/db/ (which already owns schema.sql): migrations.ts (513), types.ts (168), updatable-columns.ts (42). db.service.ts re-exports ../db/types.js, so existing import { Track, ListenerBelief } from '../services/db.service.js' in the routes/generators/session-director is untouched. Verified rather than asserted: the migration id list and the whole 502-line SQL body diff byte-identical against the parent commit.

index.ts 515 -> 313 — the reprocess_artists case was 208 lines (40% of the file, and most of what this PR changed there); it moves to reprocess-artists.service.ts. That also collapsed a real duplication: the artist merge and the normalized_name dedup pass ran the same five statements in the same order on different id pairs, so the withTransaction change had to be written twice. Both now call one mergeArtistInto(client, keepId, loserId) taking a Queryable, so the caller owns the transaction and the ON DELETE CASCADE hazard is documented once. Statement sequence diffs identical against the parent.

Gate re-run over all 15 commits: backend + workers typecheck at every commit, frontend typecheck clean, 33/33 tests.

Deliberately not split here: enrichment.service.ts (1316), musicbrainz.client.ts (548), Jobs.tsx (514). enrichment.service.ts has clean seams (identity resolution / images / enrichTrack / album dedup) but it is one class, so splitting it means converting methods to free functions over a Queryable — a real behavioural refactor, not a move, and it would make this PR harder to review, not easier. Better as its own PR.

Done, two extraction commits pushed. **`db.service.ts` 2085 -> 1398** — three pure moves into `backend/src/db/` (which already owns `schema.sql`): `migrations.ts` (513), `types.ts` (168), `updatable-columns.ts` (42). `db.service.ts` re-exports `../db/types.js`, so existing `import { Track, ListenerBelief } from '../services/db.service.js'` in the routes/generators/session-director is untouched. Verified rather than asserted: the migration id list and the whole 502-line SQL body diff byte-identical against the parent commit. **`index.ts` 515 -> 313** — the `reprocess_artists` case was 208 lines (40% of the file, and most of what this PR changed there); it moves to `reprocess-artists.service.ts`. That also collapsed a real duplication: the artist merge and the normalized_name dedup pass ran the same five statements in the same order on different id pairs, so the `withTransaction` change had to be written twice. Both now call one `mergeArtistInto(client, keepId, loserId)` taking a `Queryable`, so the caller owns the transaction and the ON DELETE CASCADE hazard is documented once. Statement sequence diffs identical against the parent. Gate re-run over all 15 commits: backend + workers typecheck at every commit, frontend typecheck clean, 33/33 tests. **Deliberately not split here:** `enrichment.service.ts` (1316), `musicbrainz.client.ts` (548), `Jobs.tsx` (514). `enrichment.service.ts` has clean seams (identity resolution / images / `enrichTrack` / album dedup) but it is one class, so splitting it means converting methods to free functions over a `Queryable` — a real behavioural refactor, not a move, and it would make this PR harder to review, not easier. Better as its own PR.

kami added 2 commits 2026-07-30 22:34:02 +02:00

refactor: split db.service.ts into data, migrations, and behaviour dee2b0ad57

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>

refactor: extract reprocess_artists out of the worker's job switch

Typecheck / typecheck (backend) (push) Has been cancelled

Details

Typecheck / typecheck (workers) (push) Has been cancelled

Details

Typecheck / typecheck (backend) (pull_request) Has been cancelled

Details

Typecheck / typecheck (workers) (pull_request) Has been cancelled

Details

bec77f4297

The reprocess_artists case was 208 of index.ts's 515 lines — 40% of the file
and most of what this PR changed in it, buried inside a switch. index.ts is
now 313 lines and reads as what it is: wiring, the job switch, cron
registration, shutdown.

The move also collapses a real duplication. The artist merge and the
normalized_name dedup pass ran the same five statements in the same order
against different id pairs, so the withTransaction change had to be made
twice, identically. Both now call one mergeArtistInto(client, keepId,
loserId), which takes a Queryable so the caller owns the transaction, and the
ON DELETE CASCADE hazard is documented once instead of twice.

Verified as behaviour-preserving: the statement sequence diffs identical
against the previous commit, and workers typecheck is clean.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

kami referenced this pull request

2026-07-30 22:44:15 +02:00

Split enrichment.service.ts (and the other oversized files) #2

kami merged commit a0c9f42a89 into master

2026-07-30 22:48:15 +02:00

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: kami/muzick#1