Skip to content

Denormalize library.artwork_url onto flowsheet on bin-pick#643

Draft
jakebromberg wants to merge 1 commit into
mainfrom
feat/bin-pick-artwork-denorm
Draft

Denormalize library.artwork_url onto flowsheet on bin-pick#643
jakebromberg wants to merge 1 commit into
mainfrom
feat/bin-pick-artwork-denorm

Conversation

@jakebromberg
Copy link
Copy Markdown
Member

Summary

  • The bin-pick branch of addEntry already calls getAlbumFromDB to backfill album info, but did not pull library.artwork_url even though the column has existed since migration 0045. This patch adds it to the SELECT so the freshly-inserted flowsheet row carries artwork at INSERT time.
  • Eliminates the metadata race for bin-picks on every client surface at once: the addEntry HTTP response carries the URL to dj-site (which polls every 60s and won't otherwise see it for up to that long), and the playlist-proxy's synchronous SELECT artwork_url WHERE … on the tubafrenzy created event reads a non-null value.
  • Free-form entries (no album_id) still rely on the asynchronous LML enrichment UPDATE — the second-emit + in-process proxy re-enrich tracked in the rest of Re-emit liveFs SSE event after metadata enrichment UPDATE lands #628 covers that path.

Part of #628 (does not close it; the free-form-entry second-emit fix is the remainder).

Test plan

  • tests/unit/controllers/flowsheet.controller.test.ts: new tests for the populated and null library.artwork_url cases on the bin-pick path. All 36 tests in that file pass.
  • prettier --check on the touched files.
  • After merge, run SELECT count(*) FILTER (WHERE artwork_url IS NOT NULL), count(*) FROM library against prod to confirm population is high enough that this patch does meaningful work in practice. (If population is low, this still doesn't regress — null artwork_url passes through and the LML enrichment path takes over as before.)

The bin-pick branch of addEntry already calls getAlbumFromDB to backfill
album info, but until now did not pull artwork_url even though the column
exists on library since migration 0045. Adding it to the SELECT lets the
freshly-inserted flowsheet row carry artwork at INSERT time, eliminating
the metadata race for bin-picks on every client surface at once: the
addEntry HTTP response carries the URL to dj-site, and the playlist-proxy's
synchronous SELECT-on-tubafrenzy-created reads a non-null value.

Free-form entries (no album_id) still rely on the asynchronous LML
enrichment UPDATE and need the second-emit fix tracked in the rest of #628.

Test coverage added at the controller boundary: asserts the bin-pick
addTrack call carries through both the populated and the null
library.artwork_url cases.
@jakebromberg
Copy link
Copy Markdown
Member Author

Holding this in draft pending the library-side artwork backfill.

Per the precondition comment on #628 (issuecomment-4338014422), the production query reported there shows that library.artwork_url is populated for only 155/64163 rows (0.2%) today.

The denormalization in this PR is correct and harmless, but until the library-side backfill ships, it denormalizes ≈zero rows in practice — the user-visible win is near zero. Filing the backfill as its own follow-up; will rebase + un-draft this once that has run and the population number is materially higher.

In the meantime, surface 2 in #628 (second-emit + in-process proxy re-enrich) is the load-bearing next step and will be picked up in a separate session.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant