feat(1492): attribution-corrected album_popularity signal (Phase-2 Track 2) - #1501
Merged
Conversation
…ack 2) Master-collapsed catalog popularity table + refresh service. - Migration 0107: album_popularity (logical_album_key PK, plays, linked_plays, freetext_plays, representative_library_id) — fresh empty table. - Refresh service (apps/backend/services/album-popularity-refresh.service.ts), modeled on album-plays-refresh: dedicated single-conn client, own statement_timeout, cronjob_runs last-run, NO library_watermark bump. Two legs — LINKED (pure SQL, keyed on the discogs:-stripped canonical_entity_id with a library:<id> fallback for unresolved rows) and FREE-TEXT (UPSERT that folds flowsheet_freetext_resolution plays onto the linked rows). - Option A: reads the Discogs master id already persisted in library.canonical_entity_id (discogs:master:<id> for ~90% of resolved rows), so no library backfill and no LML dependency on the linked-plays path. - Shared freetextPairKey helper in @wxyc/database so Track 1's writer and Track 2's reader cannot drift on the (norm_artist, norm_album) key; Track 1's job refactored onto it. - Unit (freetext-norm + refresh pure core + lifecycle) and pg integration (master collapse + free-text fold-in) tests. Closes #1492
Schema constraint shape reportdata-shape report errored (exit 0): node:internal/modules/runmain:107 triggerUncaughtException( ^ Error ERRMODULENOTFOUND: Cannot find package 'postgres' imported from /home/runner/work/Backend-Service/Backend-Service/scripts/schema-shape-report.mjs Did you mean to import "postgres/cjs/src/index.js"? at Object.getPackageJ; manual check required |
Review-loop findings on the album_popularity refresh: - The rebuild ran at READ COMMITTED, so the free-text rawPlays SELECT (album_id IS NULL) and the linked INSERT...SELECT (album_id NOT NULL) took separate MVCC snapshots. A concurrent enrichment-worker link committing between them would have the play counted by BOTH legs (a double-count); a library delete would drop it from both (an under-count). Run the transaction at REPEATABLE READ so both legs read one snapshot. The txn writes only album_popularity (single max:1 writer, no concurrent writer), so the stricter level raises no serialization error. - The integration teardown deleted only the four probe keys, but rebuildLinkedLeg rebuilds the whole table from all flowsheet-join-library rows, leaking library:<id> shape-fixture rows past afterAll. Clear the whole derived table instead, so Track 3's catalog-export spec starts clean. - The free-text read filter (entry_type='track' AND album_id IS NULL AND artist/album NOT NULL) was exercised by neither suite. Add an integration test that seeds qualifying and disqualifying rows and asserts each clause.
The service header and the schema doc-comment summarized the signal as collapsing "every pressing" and folding in "the ~43%" of free-text plays — the same overclaim corrected in the CatalogExportRow SSOT (wxyc-shared#198). In reality master-collapse applies only to rows that resolve to a Discogs master (~90% of resolved; release-only and unresolved rows keep their own per-release / library:<id> key), and only the Track-1-resolved subset of the ~43% free-text tail folds in. Prose-only; no behavior change.
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
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.
What
Phase-2 Track 2 of attribution-corrected catalog popularity (#1486): a per-logical-album popularity signal that collapses every pressing of one record under its Discogs master and folds in the ~43% of plays that are free-text/unlinked. Adds the
album_popularitytable (migration 0107) + a refresh service. Self-contained in Backend-Service — no LML calls, no cross-repo dependency.Substrate correction (deviation from the approved plan — please read)
The reviewed plan (
WXYC/wiki/plans/catalog-popularity-phase2.md) and the epic asserted "no master-level key exists;library.canonical_entity_idis release-level," and prescribed a newlibrary.discogs_master_idcolumn + an LML release→master backfill (gated on deploying Track 0 / LML#689 to prod).Checking the actual prod clone (
dev_env/seed-clone.sql, 64,193 library rows) contradicts that premise:canonical_entity_iddiscogs:master:<id>discogs:release:<id>The master collapse key already exists for ~90% of resolved rows — stripping
discogs:offcanonical_entity_idyields exactly the plan'slogical_album_key(master:<id>/release:<id>). So Track 2 reads it directly (Option A, chosen with the issue author): no new column, no backfill, and the LML→prod promotion drops off Track 2's critical path entirely. Two honest caveats are documented in thealbum_popularityschema comment +MEMORY:mapLookupToCanonicalEntity) emitsdiscogs:release:<id>, so coverage won't grow. Filed as a follow-up (the live writer should namespace by master).How it works
apps/backend/services/album-popularity-refresh.service.ts, modeled onalbum-plays-refresh.service.ts(dedicatedmax:1client, ownstatement_timeout,cronjob_runslast-run, nolibrary_watermarkbump per plan decision 4). A plain TABLE, not an MV, because the free-text leg can't be a single SQL SELECT. Rebuilt in one transaction:flowsheettrack rows JOINlibrary, keyed by thediscogs:-strippedcanonical_entity_id, with alibrary:<id>fallback for unresolved rows so a played row's plays are never lost. Sums pressings sharing a master into one row.(artist, album)of unlinked plays, re-keyed viafreetextPairKeyto match Track 1's persisted(norm_artist, norm_album), joined to the resolution's release/master, summed, then UPSERT-folded onto the linked rows (plays = linked_plays + EXCLUDED.freetext_plays).No-drift parity helper
The free-text join only works if Track 2 re-derives the exact
(norm_artist, norm_album)Track 1 wrote. Extracted that key composition into@wxyc/database'sfreetextPairKey(the artist leg's whitespace-collapse thatnormalizeArtistNameomits, +normalizeAlbumTitle) and refactored Track 1's job onto it, so the writer and reader can't diverge. Pinned bytests/unit/database/freetext-norm.test.ts.Tests
freetext-normparity contract; refresh pure core (freetextLogicalKey,aggregateFreetextPlaysagainst the real normalizers); lifecycle/dedicated-client wiring. Track 1'snormalizePairssuite still green post-refactor.pg): two pressings of one master collapse into one row summing both plays;release:/library:fallbacks; free-text fold-in arithmetic; free-text-only key inserts fresh. Fullci:testmockgreen (migration 0107 applies + whole integration suite).Scope / sequencing
api.yamlchange —album_popularityis Backend-internal; the exportpopularityfield is Track 3 (Catalog export popularity field + wiring (Phase-2 Track 3) #1493), which reads this table and is unblocked by this PR.@no-precondition-needed), populated at runtime.Closes #1492