feat: Library endpoints and search filters for MD catalog editing

[JacksonMeade]

Summary

Backend support for Music Director catalog editing and richer catalog search on the Card Catalog.

• Adds PATCH /library/:id album update with validation (artist in genre, label resolution, etc.).
• Adds artist discovery helpers: in-genre artist search, peek artist number, and related service methods used by the add/edit forms.
• Extends library query/search with multi-value genre and format filters, rotation bin tags, and a missing-album filter; aligns cascade and SQL paths with the new params.
• Wires routes under existing catalog:read / catalog:write permission checks; integration tests cover update, artist search, and query filter behavior.

Pairs with the dj-site feat/md-interface PR for end-to-end MD catalog maintenance.

Test plan

• PATCH /library/:id updates album metadata for authorized callers; rejects invalid artist/genre pairings
• Artist search and peek-code endpoints return expected shapes for form autocomplete
• /library/query honors genres, formats, rotation_bins, and missing filters
• Run integration suite (library.spec.js, library-query.spec.js)

Made with Cursor

[jakebromberg]

Found 15 issues, ranked most-severe first. The new PATCH /library/:id is the riskiest surface — it uses PATCH-route semantics but the handler unconditionally rewrites every column from the body, so several optional fields can be silently wiped on a partial edit.

Blocking

1. genres filter ORs multiple genre names test will fail in CI — tests/integration/library-query.spec.js:95
CI loads only dev_env/seed_db.sql (LOAD_CLONE_FIXTURE=false); the seed has 11 library rows across genre_ids 11/6/15 only — no Jazz. /library/query?genre=Jazz returns total=0, so expect(jazzOnly.body.results.length).toBeGreaterThan(0) fails before the combined-filter assertions run. Either seed a Jazz row or pick two genres that the seed already covers (e.g. Rock + Hiphop).

2. parseEnumQueryList crashes with TypeError on repeated query keys — apps/backend/services/library-search.service.ts:448
Express 5's default 'simple' parser (Node querystring.parse) yields a string[] for repeated keys. ?genres=Rock&genres=Jazz gives req.query.genres = ['Rock','Jazz']; inside parseEnumQueryList the array passes the truthy if (!value) guard, then value.split(',') throws value.split is not a function → 500. Same path for ?genre=, ?format=, ?formats=, ?rotation_bins=. Normalize each rest-arg via Array.isArray(x) ? x : [x] or coerce upstream.

3. searchArtistsInGenre q.trim() crashes on repeated q — apps/backend/controllers/library.controller.ts:284
/library/artists/search?genre_id=11&q=Bu&q=lt → req.query.q is ['Bu','lt'] → (req.query.q ?? '').trim() throws TypeError → 500. The integer-checked params coerce arrays to NaN and 400 cleanly; q is the only explosive parameter.

4. Cascade in-memory filter omits the missing constraint — apps/backend/services/library-search.service.ts:178
SQL primary path applies missing via EXISTS. When primary returns 0 rows and passesCascadeGate(q) is true, runCascade filters on on_streaming/genres/formats/rotation_bins but not missing. TaggedLibraryViewEntry has no date_lost/date_found to inspect, so cascade results from CTA/LML leak through to the librarian-facing 'missing' view as if they were missing. The new rotation_bins AND missing both apply test only asserts rotation_bin, masking this. Easiest fix: skip the cascade when params.missing === true.

High — silent data loss / corruption on PATCH

5. updateAlbumInDB SETs every column unconditionally — apps/backend/services/library.service.ts:1272
The handler defaults omitted optional fields (disc_quantity → 1, alternate_artist_name → null) and updateAlbumInDB writes them unconditionally. A partial PATCH that fixes only a typo in album_title will overwrite a 2-disc album to disc_quantity=1 and wipe a curator-set alternate_artist_name (a load-bearing V/A filing-display surface — see project_librarian_va_invariant). Either only SET fields the body explicitly provided, or use PUT semantics and require every optional field.

6. Empty label: '' wipes existing label_id — apps/backend/controllers/library.controller.ts:527
Missing-params guard only rejects === undefined, so label: '' slides past. if (label_id === undefined && body.label) is false (empty string is falsy), so createLabel is skipped, label_id stays undefined, and updateAlbumInDB writes label_id: undefined which drizzle maps to NULL. The FK to a long-stable label is destroyed without any signal.

7. updateAlbum doesn't regenerate code_number when artist_id changes — apps/backend/services/library.service.ts:1272
No UNIQUE(artist_id, code_number) exists. addAlbum calls generateAlbumCodeNumber(artist_id) to assign the per-artist sequence; updateAlbum carries the old number over. Re-attributing an album to an artist that already has that code_number produces two rows with the same (code_letters, code_artist_number, code_number) — librarian V/A invariant violation, silently filed.

8. Empty album_title: '' persists silently — apps/backend/controllers/library.controller.ts:498
Same === undefined-only guard. ''.trim() === '' and the NOT NULL is satisfied by empty string. addAlbum has the same hole; updateAlbum exposes it on long-stable rows. Either reject empty trimmed strings at the controller, or CHECK (album_title <> '') at the schema.

Medium

9. missing=false is a silent no-op — apps/backend/controllers/library.controller.ts:645 vs services L342
Controller parses to missing: false and forwards; buildFilterClause only branches on missing === true. A UI toggle that round-trips both states will silently disable filtering when set to false. Either reject missing=false as 400 or implement the inverse NOT-EXISTS arm.

10. updateAlbum creates an orphan label row on the 404 path — apps/backend/controllers/library.controller.ts:524
labelsService.createLabel(body.label) runs before updateAlbumInDB. If the album doesn't exist (or another row is concurrently deleted), the label persists. Enumerating album IDs against this endpoint bulk-pollutes the labels table. Move the album existence/lookup before the label upsert.

11. createLabel un-trimmed vs library.label trimmed — labels duplication — apps/backend/controllers/library.controller.ts:527
createLabel(' Drag City ') inserts a padded labels.label_name; library.label then writes 'Drag City'. A later submission with the trimmed form misses the ON CONFLICT and creates a second labels row. addAlbum is internally consistent (both writes un-trimmed); only updateAlbum desyncs. Trim before createLabel.

12. updateAlbum doesn't refresh streaming / artwork / canonical_entity — apps/backend/controllers/library.controller.ts:493
addAlbum (L93–135) fires checkStreamingAvailability, lookupMetadata, and the canonical-entity resolver. updateAlbum doesn't. When a librarian fixes an album's artist or title, library.on_streaming / library.artwork_url / library_identity / canonical_entity_id stay bound to the OLD identity. metadata_attempt_at-driven drain jobs don't repair this because the row's attempt-at is already set. Either invalidate the enrichment columns on metadata-affecting fields or fire the same allSettled enrichment.

13. disc_quantity lacks type/range validation — apps/backend/controllers/library.controller.ts:534
Math.max(1, 'abc') = NaN, Math.max(1, 1.5) = 1.5, Math.max(1, 40000) = 40000 — all reach PG smallint and fail with invalid input syntax / value out of range → 500. Number.isInteger + range check at the controller, mirroring genre_id.

14. searchArtistsInGenre ILIKE prefix doesn't escape % / _ — apps/backend/services/library.service.ts:1149
q='%a' (2 chars passes the length check) builds pattern '%a%' and returns any artist whose name contains 'a'. q='__' → '__%' matches any 2+ char artist. The autocomplete shorts its prefix-search contract; future planner shortcuts on trigram will be bypassed. Same shape exists in searchLabels (pre-existing). Escape %, _, \ in the bound prefix.

15. rotation_bins filter inherits row duplication from library_artist_view — apps/backend/services/library-search.service.ts:341
The rotation table comment (shared/database/src/schema.ts:534) explicitly permits "multiple active rows per (album_id, rotation_bin)" (re-bins, re-promotes). The view LEFT JOINs rotation without distinctOn, so an album with two unkilled rotation rows appears twice. Pre-existing in the view, newly surfaced by the inArray(rotation_bin) filter and the COUNT(*) total. distinctOn(library_artist_view.id) on the projection or a subquery that picks one rotation row per album.

Also worth a quick look

• Client-supplied body.label_id isn't validated against the labels table — a stale/guessed integer bypasses createLabel and surfaces as PG 23503 → 500.
• body.label_id: null (vs undefined) plus a non-empty body.label skips createLabel AND writes label_id=NULL with non-empty library.label text — desync.
• searchArtistsInGenre returns { artists: [] } for any positive genre_id regardless of whether the genre actually exists — silent empty results for stale dropdown IDs.
• recordActivity's new SELECT-then-INSERT adds a DB round-trip to every authenticated request (including all proxy_route.use(trackActivity) endpoints). The previous FK violation also surfaced anonymous/orphaned user IDs to Sentry; the silent no-op masks that signal. A catch-on-23503 would preserve both the latency floor and the visibility.
• No integration tests cover PATCH /library/:id at all — every error branch and the response shape ship unverified.
• api.yaml doesn't declare genres/formats/missing/rotation_bins, PATCH /library/{id}, or GET /library/artists/search — codegen drift across dj-site / iOS / Android.

[JacksonMeade]

Thanks for the thorough review — all 15 issues plus the quick-look items are addressed in d05b81f. Point-by-point:

Blocking

1. Jazz test seed — Fixed. Seeded a Jazz row (Miles Davis / Kind of Blue, genre_id 7) in dev_env/seed_db.sql so the genres-OR test has data under LOAD_CLONE_FIXTURE=false.

2. parseEnumQueryList TypeError on repeated keys — Fixed. The signature now accepts (string | string[] | undefined)[] and flattens arrays before the .split(','), so ?genres=Rock&genres=Jazz merges instead of 500ing. Covers genre/genres/format/formats/rotation_bins.

3. Repeated q TypeError — Fixed. Both searchArtistsInGenre and searchLibraryQueryEndpoint reject a non-string q with 400 before .trim().

4. Cascade leaks into the missing view — Fixed via the suggested skip: the cascade is bypassed whenever the missing filter is present (either value), since TaggedLibraryViewEntry carries no date_lost/date_found to filter on in-memory. Regression test asserts q=Bioluminescence&missing=true returns 0 rows.

PATCH /library/:id (5–8, 10, 11, 13)

We went with true partial PATCH semantics: UpdateAlbumRequest is fully optional (≥1 field required, else 400), and updateAlbumInDB only SETs keys present in the body. Specifically:

5. Unconditional column rewrite — Fixed. A title-typo PATCH no longer resets disc_quantity to 1 or wipes alternate_artist_name/label_id. Integration test pins exactly this scenario.

6. label: '' wipes label_id — Fixed. Empty trimmed label is rejected with 400; clearing the linkage is now explicit via label_id: null (which clears both label_id and label text, keeping them in sync).

7. code_number on re-attribution — We chose collision-only regeneration: the album keeps its code_number unless the new artist already owns it, in which case generateAlbumCodeNumber assigns the next one. This preserves filing stability for the common case while guaranteeing the (code_letters, code_artist_number, code_number) invariant. Tests cover both the collision and no-collision paths.

8. Empty album_title — Fixed at the controller for both addAlbum and updateAlbum (reject empty trimmed string with 400). We skipped the CHECK constraint for now to keep this PR migration-free; happy to add it as a follow-up if you'd prefer the schema-level guarantee.

10. Orphan label on the 404 path — Fixed. The album row is resolved first; all validation and the label upsert happen only after the 404 check. Test asserts no labels row is created when PATCHing a nonexistent id.

11. Un-trimmed createLabel desync — Fixed. label is trimmed before the upsert, so padded re-submissions hit the same labels row (test asserts a single row after padded + trimmed submissions).

13. disc_quantity validation — Fixed. Number.isInteger + 1–99 range at the controller, 400 otherwise (NaN/float/out-of-smallint all rejected).

12. Stale enrichment on identity change — We did both halves: when artist_id, album_title, or alternate_artist_name actually change, the UPDATE nulls on_streaming/artwork_url/canonical_entity_* (so a failed lookup leaves the row visibly unenriched rather than bound to the wrong identity), then the same Promise.allSettled pipeline addAlbum uses re-fires (streaming + artwork + fire-and-forget canonical entity), gated on isLmlConfigured().

Medium

9. missing=false no-op — Implemented the inverse NOT-EXISTS arm. Intended semantics, for the record: missing unspecified returns both missing and non-missing albums; true and false each filter to their partition. Regression test exercises all three states across a mark-missing/mark-found round trip.

14. ILIKE prefix escaping — Fixed in searchArtistsInGenre (%, _, \ escaped in the bound prefix; test asserts q='%u' matches nothing). We deliberately left the pre-existing same-shape issue in searchLabels untouched to keep this PR scoped — flagging it for a follow-up.

15. Rotation-driven row duplication — Fixed. /library/query now projects through SELECT DISTINCT ON (id) (deterministic bin pick) with the caller's sort applied over the deduped set, and total uses COUNT(DISTINCT id). Test adds two active rotation rows to one album and asserts it appears once in both the plain and rotation_bins-filtered queries.

Quick-look items

• Unvalidated body.label_id — now validated against the labels table; stale/guessed ids return 400 instead of a 23503 → 500.
• label_id: null + non-empty label desync — rejected with 400.
• searchArtistsInGenre silent empty results — unknown genre_id now returns 404.
• recordActivity SELECT-then-INSERT — dropped the pre-check entirely. The insert's FK violation propagates to trackActivity's existing fire-and-forget catch, which logs and reports to Sentry — restoring both the latency floor and the orphaned-user signal without failing the request.
• No PATCH integration tests — added tests/integration/library-update.spec.js covering partial-update preservation, empty title/label rejection, label trim dedupe, orphan-label 404 path, label_id validation/clearing, disc_quantity validation, genre-membership rejection, and the code_number collision/no-collision moves; plus a regression block in library-query.spec.js for repeated keys, missing partitioning, cascade skip, and rotation dedupe.
• OpenAPI drift — app.yaml now declares the /library/query filter params (genres, formats, missing, rotation_bins, plus the rest), PATCH /library/{id}, PATCH /library/{id}/missing|found, and GET /library/artists/search.