feat(addToRotation): library_identity lookup + LML resolve + allowlist (BS#1380 PR 4/5)#1420
Merged
Merged
Conversation
4 tasks
jakebromberg
force-pushed
the
feature/bs-1380-3-rotation-etl-drift
branch
from
10adeab to
4e4937a
Compare
jakebromberg
changed the base branch from
feature/bs-1380-3-rotation-etl-drift
to
main
…t (BS#1380 PR 4/5) Fourth in the BS#1380 chain. Chains feature/bs-1380-3-rotation-etl-drift (which carries PR 1's schema). Depends on PR 2 (#1418) for the `resolveIdentity` wrapper; merged in here so this branch typechecks. apps/backend/services/library.service.ts:addToRotation: - Synchronously looks up `library_identity.discogs_release_id` for the requested album_id (library_identity.library_id is PRIMARY KEY per schema.ts:1391-1393). - If non-NULL, awaits `resolveIdentity({kind:'release', source:'discogs_release', external_id})` before INSERT to mint a stable lml_identity_id. - Triple-writes (discogs_release_id, discogs_release_id_source = 'library_identity', lml_identity_id) on the INSERT. - On resolve failure (timeout / 5xx / network), the first two still land; lml_identity_id falls back to NULL. The source-of-the-Discogs-id is library_identity regardless of whether the mint succeeded — kill_date forensics, BS#1029 backfill scoping, and the future discogs_release_id retirement migration all rely on this column being honest. - The daily backfill cron (PR 5) catches up within ~24h. apps/backend/services/library.service.ts:classifyLmlResolveError: - Pure classifier mapping `unknown` → `LmlResolveFallbackReason` (`timeout` | `5xx` | `4xx` | `network` | `other`). Each bucket signals a different operational response (timeout → LML latency, 5xx → LML deploy regression, 4xx → BS-side caller bug, network → infra noise). Sentry counter `lml.resolve.fallback_to_null` carries the result as a `reason` attribute alongside `caller=add_to_rotation`. - Disambiguates LmlClientError statusCode 502 between upstream-5xx (message starts with "LML responded with") and network/fetch-threw (message starts with "LML request failed"). apps/backend/controllers/library.controller.ts:pickAddRotationFields: - Signature-typed allowlist mirroring flowsheet.controller.ts pickUpdateEntryFields (BS#1099). Only {album_id, rotation_bin} pass through. - Server-derived columns (legacy_*, discogs_release_id*, lml_identity_id, tracklist_lookup_attempted_at, kill_date) and tubafrenzy-ETL-only snapshot columns (add_date, artist_name, album_title, record_label) must never be client-supplied through this endpoint — addToRotation derives the LML-handle columns from library_identity + the synchronous resolveIdentity hop, and tubafrenzy is the only legitimate source for the snapshot columns. - Phrased as an allowlist so future column additions to `rotation` are implicitly rejected by typecheck until explicitly added to the signature. Unit tests: - classifyLmlResolveError: every catch-block branch (AbortError → timeout, 504 → timeout, 502 + "LML responded" message → 5xx, 502 + "LML request failed" message → network, 422 → 4xx, 400 → 4xx, 500 → 5xx, ECONNRESET / ENOTFOUND / ECONNREFUSED → network, plain Error → other, non-Error throwables → other). - addToRotation: library_identity hit + resolve success → triple-write lands correctly; library_identity hit + resolve failure → first two fields land, lml_identity_id NULL, Sentry counter fires once with reason=timeout; library_identity miss → no LML call, no triple-write; reads library_identity by library_id PRIMARY KEY. Integration test (tests/integration/library.spec.js): - POST /library/rotation with non-allowlisted fields in the body (forbidden server-derived + ETL-only) verifies the persisted row has those fields server-set / NULL, not client-supplied. Refs #1380
jakebromberg
force-pushed
the
feature/bs-1380-4-add-to-rotation
branch
from
fdc74f9 to
9834cb4
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fourth in a 5-PR chain implementing #1380. Chained on #1419 (PR 3 rotation-etl drift, which chains #1417 / PR 1 schema). Depends on #1418 (PR 2 lml-client wrapper) — merged into this branch so it typechecks; the merge commit disappears once #1418 lands on main and this branch rebases.
Summary
addToRotationtriple-write path (apps/backend/services/library.service.ts):library_identity.discogs_release_idfor the requestedalbum_id(PK lookup).await resolveIdentity({kind:'release', source:'discogs_release', external_id})to mint a stablelml_identity_id.(discogs_release_id, discogs_release_id_source='library_identity', lml_identity_id).lml_identity_idfalls back to NULL. The source-of-the-Discogs-id is stilllibrary_identityregardless of whether the mint succeeded — kill_date forensics, BS#1029 backfill scoping, and the futurediscogs_release_idretirement migration all rely on this column being honest. The daily backfill cron (PR 5) catches up within ~24h.classifyLmlResolveError(pure function in same file):unknown→LmlResolveFallbackReason(timeout|5xx|4xx|network|other)lml.resolve.fallback_to_nullcarriescaller=add_to_rotation+reason=<bucket>LmlClientError.statusCode === 502between upstream-5xx ("LML responded with...") and fetch-threw ("LML request failed...")pickAddRotationFieldsallowlist (apps/backend/controllers/library.controller.ts):pickUpdateEntryFieldsfrom BS#1099. Only{album_id, rotation_bin}pass through.legacy_*,discogs_release_id*,lml_identity_id,tracklist_lookup_attempted_at,kill_date) and ETL-only snapshot columns (add_date,artist_name,album_title,record_label) must never be client-supplied through this endpoint.rotationare implicitly rejected by typecheck until explicitly added.Test plan
classifyLmlResolveErrorcovering every catch-block branchaddToRotationtriple-write success, resolve-failure fallback, library_identity miss, PRIMARY-KEY readtests/integration/library.spec.jsverifies allowlist drops client-supplied non-allowlisted fieldsRefs #1380