kami/muzick
1
0
Fork 0
Code Issues 1 Pull Requests Actions Packages Projects Releases Wiki Activity

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

New Issue
Open
opened 2026-07-30 22:44:15 +02:00 by kami · 0 comments
kami commented 2026-07-30 22:44:15 +02:00
Owner
Copy Link
Copy Source

Follow-up to #1, deliberately not done there: extracting it is a behavioural refactor rather than a move, so it would have made that PR harder to review.

The files

File Lines
workers/src/enrichment.service.ts 1316
workers/src/integrations/musicbrainz.client.ts 548
frontend/src/pages/Jobs.tsx 514

Why enrichment.service.ts is the hard one

It is a single class holding private pgClient: Queryable, so splitting it means turning methods into free functions that take a Queryable explicitly (as mergeArtistInto in reprocess-artists.service.ts now does), or composing sub-services the class delegates to. Either way call sites and this. references change — not a pure move, so "diff the content byte-identical" no longer works as the safety check.

Seams that look clean:

  • schema — ensureSchema (78-157)
  • artist identity — resolveArtistIdentity, createLocalArtist, generateSortName, addArtistAlias, updateArtistLookupCache (213-448), plus analyzeArtistIdentification / verifyArtistIdentity (1099-1242)
  • images/covers — getArtistImage, updateArtistImage (449-542), refreshArtistImage (873-908), refreshAlbumCover (909-1045)
  • enrichTrack (543-838) — 295 lines on its own, the real monster, and the one that needs decomposing rather than relocating
  • album dedup — dedupAlbums, mergeAlbum (1243-1330)

Constraints when this happens

  • The worker has no tests. npm run typecheck is the only gate, and it will not catch a Queryable being passed where a transaction-scoped client was meant. Consider landing a few tests for resolveArtistIdentity and dedupAlbums before moving them.
  • enrichTrack and resolveArtistIdentity are on the hot enrichment path with concurrency: 10; keep the transaction boundaries exactly where they are (see the Pool commit in #1 for why a shared BEGIN is dangerous here).
  • Do it as several small PRs — one seam each — not one 1300-line move.

Line numbers above are as of bec77f4 and will drift.

Follow-up to #1, deliberately **not** done there: extracting it is a behavioural refactor rather than a move, so it would have made that PR harder to review. ## The files | File | Lines | |---|---| | `workers/src/enrichment.service.ts` | 1316 | | `workers/src/integrations/musicbrainz.client.ts` | 548 | | `frontend/src/pages/Jobs.tsx` | 514 | ## Why `enrichment.service.ts` is the hard one It is a single class holding `private pgClient: Queryable`, so splitting it means turning methods into free functions that take a `Queryable` explicitly (as `mergeArtistInto` in `reprocess-artists.service.ts` now does), or composing sub-services the class delegates to. Either way call sites and `this.` references change — not a pure move, so "diff the content byte-identical" no longer works as the safety check. Seams that look clean: - **schema** — `ensureSchema` (78-157) - **artist identity** — `resolveArtistIdentity`, `createLocalArtist`, `generateSortName`, `addArtistAlias`, `updateArtistLookupCache` (213-448), plus `analyzeArtistIdentification` / `verifyArtistIdentity` (1099-1242) - **images/covers** — `getArtistImage`, `updateArtistImage` (449-542), `refreshArtistImage` (873-908), `refreshAlbumCover` (909-1045) - **`enrichTrack`** (543-838) — 295 lines on its own, the real monster, and the one that needs decomposing rather than relocating - **album dedup** — `dedupAlbums`, `mergeAlbum` (1243-1330) ## Constraints when this happens - The worker has **no tests**. `npm run typecheck` is the only gate, and it will not catch a `Queryable` being passed where a transaction-scoped client was meant. Consider landing a few tests for `resolveArtistIdentity` and `dedupAlbums` *before* moving them. - `enrichTrack` and `resolveArtistIdentity` are on the hot enrichment path with `concurrency: 10`; keep the transaction boundaries exactly where they are (see the `Pool` commit in #1 for why a shared `BEGIN` is dangerous here). - Do it as several small PRs — one seam each — not one 1300-line move. Line numbers above are as of `bec77f4` and will drift.
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
No items
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
kami
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: kami/muzick#2
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?