From ade03a2f8bf5f88d43f5cdc563c34dc736b8f8f5 Mon Sep 17 00:00:00 2001 From: Even Date: Thu, 5 Nov 2026 15:53:21 +0100 Subject: [PATCH 1/5] fix: #64 keep artist credit order --- server/drizzle/0004_album-artist-position.sql | 1 + server/drizzle/meta/0004_snapshot.json | 604 ++++++++++++++++++ server/drizzle/meta/_journal.json | 7 + server/src/db/schema.ts | 3 + server/src/db/seed.ts | 10 +- server/src/loaders/index.ts | 2 +- 6 files changed, 623 insertions(+), 4 deletions(-) create mode 100644 server/drizzle/0004_album-artist-position.sql create mode 100644 server/drizzle/meta/0004_snapshot.json diff --git a/server/drizzle/0004_album-artist-position.sql b/server/drizzle/0004_album-artist-position.sql new file mode 100644 index 0000000..58dc35e --- /dev/null +++ b/server/drizzle/0004_album-artist-position.sql @@ -0,0 +1 @@ +ALTER TABLE "album_artist" ADD COLUMN "position" smallint DEFAULT 0 NOT NULL; \ No newline at end of file diff --git a/server/drizzle/meta/0004_snapshot.json b/server/drizzle/meta/0004_snapshot.json new file mode 100644 index 0000000..29e2ee3 --- /dev/null +++ b/server/drizzle/meta/0004_snapshot.json @@ -0,0 +1,604 @@ +{ + "id": "eae9124a-5b33-46a1-9607-f1c162a1d10a", + "prevId": "82666a74-d81e-4ae3-85d0-a0ff16223746", + "version": "7", + "dialect": "postgresql", + "tables": { + "public.album": { + "name": "album", + "schema": "", + "columns": { + "id": { + "name": "id", + "type": "serial", + "primaryKey": true, + "notNull": true + }, + "title": { + "name": "title", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "release_year": { + "name": "release_year", + "type": "integer", + "primaryKey": false, + "notNull": false + }, + "musicbrainz_release_group_id": { + "name": "musicbrainz_release_group_id", + "type": "text", + "primaryKey": false, + "notNull": false + }, + "search_vector": { + "name": "search_vector", + "type": "tsvector", + "primaryKey": false, + "notNull": false + }, + "average_rating": { + "name": "average_rating", + "type": "double precision", + "primaryKey": false, + "notNull": false + } + }, + "indexes": { + "album_search_vector_idx": { + "name": "album_search_vector_idx", + "columns": [ + { + "expression": "search_vector", + "isExpression": false, + "asc": true, + "nulls": "last" + } + ], + "isUnique": false, + "concurrently": false, + "method": "gin", + "with": {} + } + }, + "foreignKeys": {}, + "compositePrimaryKeys": {}, + "uniqueConstraints": { + "album_musicbrainz_release_group_id_unique": { + "name": "album_musicbrainz_release_group_id_unique", + "nullsNotDistinct": false, + "columns": ["musicbrainz_release_group_id"] + } + }, + "policies": {}, + "checkConstraints": {}, + "isRLSEnabled": false + }, + "public.album_artist": { + "name": "album_artist", + "schema": "", + "columns": { + "album_id": { + "name": "album_id", + "type": "integer", + "primaryKey": false, + "notNull": true + }, + "artist_id": { + "name": "artist_id", + "type": "integer", + "primaryKey": false, + "notNull": true + }, + "position": { + "name": "position", + "type": "smallint", + "primaryKey": false, + "notNull": true, + "default": 0 + } + }, + "indexes": {}, + "foreignKeys": { + "album_artist_album_id_album_id_fk": { + "name": "album_artist_album_id_album_id_fk", + "tableFrom": "album_artist", + "tableTo": "album", + "columnsFrom": ["album_id"], + "columnsTo": ["id"], + "onDelete": "cascade", + "onUpdate": "no action" + }, + "album_artist_artist_id_artist_id_fk": { + "name": "album_artist_artist_id_artist_id_fk", + "tableFrom": "album_artist", + "tableTo": "artist", + "columnsFrom": ["artist_id"], + "columnsTo": ["id"], + "onDelete": "cascade", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": { + "album_artist_album_id_artist_id_pk": { + "name": "album_artist_album_id_artist_id_pk", + "columns": ["album_id", "artist_id"] + } + }, + "uniqueConstraints": {}, + "policies": {}, + "checkConstraints": {}, + "isRLSEnabled": false + }, + "public.album_genre": { + "name": "album_genre", + "schema": "", + "columns": { + "album_id": { + "name": "album_id", + "type": "integer", + "primaryKey": false, + "notNull": true + }, + "genre_id": { + "name": "genre_id", + "type": "integer", + "primaryKey": false, + "notNull": true + } + }, + "indexes": {}, + "foreignKeys": { + "album_genre_album_id_album_id_fk": { + "name": "album_genre_album_id_album_id_fk", + "tableFrom": "album_genre", + "tableTo": "album", + "columnsFrom": ["album_id"], + "columnsTo": ["id"], + "onDelete": "cascade", + "onUpdate": "no action" + }, + "album_genre_genre_id_genre_id_fk": { + "name": "album_genre_genre_id_genre_id_fk", + "tableFrom": "album_genre", + "tableTo": "genre", + "columnsFrom": ["genre_id"], + "columnsTo": ["id"], + "onDelete": "cascade", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": { + "album_genre_album_id_genre_id_pk": { + "name": "album_genre_album_id_genre_id_pk", + "columns": ["album_id", "genre_id"] + } + }, + "uniqueConstraints": {}, + "policies": {}, + "checkConstraints": {}, + "isRLSEnabled": false + }, + "public.app_user": { + "name": "app_user", + "schema": "", + "columns": { + "id": { + "name": "id", + "type": "uuid", + "primaryKey": true, + "notNull": true + }, + "display_name": { + "name": "display_name", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "created_at": { + "name": "created_at", + "type": "timestamp with time zone", + "primaryKey": false, + "notNull": true, + "default": "now()" + } + }, + "indexes": {}, + "foreignKeys": {}, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "policies": {}, + "checkConstraints": {}, + "isRLSEnabled": false + }, + "public.artist": { + "name": "artist", + "schema": "", + "columns": { + "id": { + "name": "id", + "type": "serial", + "primaryKey": true, + "notNull": true + }, + "name": { + "name": "name", + "type": "text", + "primaryKey": false, + "notNull": true + } + }, + "indexes": {}, + "foreignKeys": {}, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "policies": {}, + "checkConstraints": {}, + "isRLSEnabled": false + }, + "public.crate_item": { + "name": "crate_item", + "schema": "", + "columns": { + "id": { + "name": "id", + "type": "serial", + "primaryKey": true, + "notNull": true + }, + "user_id": { + "name": "user_id", + "type": "uuid", + "primaryKey": false, + "notNull": true + }, + "album_id": { + "name": "album_id", + "type": "integer", + "primaryKey": false, + "notNull": true + }, + "added_at": { + "name": "added_at", + "type": "timestamp with time zone", + "primaryKey": false, + "notNull": true, + "default": "now()" + } + }, + "indexes": { + "crate_item_user_album_unique": { + "name": "crate_item_user_album_unique", + "columns": [ + { + "expression": "user_id", + "isExpression": false, + "asc": true, + "nulls": "last" + }, + { + "expression": "album_id", + "isExpression": false, + "asc": true, + "nulls": "last" + } + ], + "isUnique": true, + "concurrently": false, + "method": "btree", + "with": {} + }, + "crate_item_user_added_at_idx": { + "name": "crate_item_user_added_at_idx", + "columns": [ + { + "expression": "user_id", + "isExpression": false, + "asc": true, + "nulls": "last" + }, + { + "expression": "added_at", + "isExpression": false, + "asc": true, + "nulls": "last" + }, + { + "expression": "id", + "isExpression": false, + "asc": true, + "nulls": "last" + } + ], + "isUnique": false, + "concurrently": false, + "method": "btree", + "with": {} + } + }, + "foreignKeys": { + "crate_item_user_id_app_user_id_fk": { + "name": "crate_item_user_id_app_user_id_fk", + "tableFrom": "crate_item", + "tableTo": "app_user", + "columnsFrom": ["user_id"], + "columnsTo": ["id"], + "onDelete": "cascade", + "onUpdate": "no action" + }, + "crate_item_album_id_album_id_fk": { + "name": "crate_item_album_id_album_id_fk", + "tableFrom": "crate_item", + "tableTo": "album", + "columnsFrom": ["album_id"], + "columnsTo": ["id"], + "onDelete": "cascade", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "policies": {}, + "checkConstraints": {}, + "isRLSEnabled": false + }, + "public.genre": { + "name": "genre", + "schema": "", + "columns": { + "id": { + "name": "id", + "type": "serial", + "primaryKey": true, + "notNull": true + }, + "name": { + "name": "name", + "type": "text", + "primaryKey": false, + "notNull": true + } + }, + "indexes": {}, + "foreignKeys": {}, + "compositePrimaryKeys": {}, + "uniqueConstraints": { + "genre_name_unique": { + "name": "genre_name_unique", + "nullsNotDistinct": false, + "columns": ["name"] + } + }, + "policies": {}, + "checkConstraints": {}, + "isRLSEnabled": false + }, + "public.review": { + "name": "review", + "schema": "", + "columns": { + "id": { + "name": "id", + "type": "serial", + "primaryKey": true, + "notNull": true + }, + "user_id": { + "name": "user_id", + "type": "uuid", + "primaryKey": false, + "notNull": true + }, + "album_id": { + "name": "album_id", + "type": "integer", + "primaryKey": false, + "notNull": true + }, + "rating": { + "name": "rating", + "type": "smallint", + "primaryKey": false, + "notNull": true + }, + "comment": { + "name": "comment", + "type": "text", + "primaryKey": false, + "notNull": true, + "default": "''" + }, + "created_at": { + "name": "created_at", + "type": "timestamp with time zone", + "primaryKey": false, + "notNull": true, + "default": "now()" + }, + "updated_at": { + "name": "updated_at", + "type": "timestamp with time zone", + "primaryKey": false, + "notNull": true, + "default": "now()" + } + }, + "indexes": { + "review_user_album_unique": { + "name": "review_user_album_unique", + "columns": [ + { + "expression": "user_id", + "isExpression": false, + "asc": true, + "nulls": "last" + }, + { + "expression": "album_id", + "isExpression": false, + "asc": true, + "nulls": "last" + } + ], + "isUnique": true, + "concurrently": false, + "method": "btree", + "with": {} + }, + "review_album_idx": { + "name": "review_album_idx", + "columns": [ + { + "expression": "album_id", + "isExpression": false, + "asc": true, + "nulls": "last" + } + ], + "isUnique": false, + "concurrently": false, + "method": "btree", + "with": {} + } + }, + "foreignKeys": { + "review_user_id_app_user_id_fk": { + "name": "review_user_id_app_user_id_fk", + "tableFrom": "review", + "tableTo": "app_user", + "columnsFrom": ["user_id"], + "columnsTo": ["id"], + "onDelete": "cascade", + "onUpdate": "no action" + }, + "review_album_id_album_id_fk": { + "name": "review_album_id_album_id_fk", + "tableFrom": "review", + "tableTo": "album", + "columnsFrom": ["album_id"], + "columnsTo": ["id"], + "onDelete": "cascade", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "policies": {}, + "checkConstraints": { + "review_rating_range": { + "name": "review_rating_range", + "value": "\"review\".\"rating\" BETWEEN 1 AND 5" + } + }, + "isRLSEnabled": false + }, + "public.track": { + "name": "track", + "schema": "", + "columns": { + "id": { + "name": "id", + "type": "serial", + "primaryKey": true, + "notNull": true + }, + "album_id": { + "name": "album_id", + "type": "integer", + "primaryKey": false, + "notNull": true + }, + "title": { + "name": "title", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "track_number": { + "name": "track_number", + "type": "integer", + "primaryKey": false, + "notNull": true + }, + "duration_seconds": { + "name": "duration_seconds", + "type": "integer", + "primaryKey": false, + "notNull": false + } + }, + "indexes": { + "track_album_track_number_unique": { + "name": "track_album_track_number_unique", + "columns": [ + { + "expression": "album_id", + "isExpression": false, + "asc": true, + "nulls": "last" + }, + { + "expression": "track_number", + "isExpression": false, + "asc": true, + "nulls": "last" + } + ], + "isUnique": true, + "concurrently": false, + "method": "btree", + "with": {} + }, + "track_album_track_number_idx": { + "name": "track_album_track_number_idx", + "columns": [ + { + "expression": "album_id", + "isExpression": false, + "asc": true, + "nulls": "last" + }, + { + "expression": "track_number", + "isExpression": false, + "asc": true, + "nulls": "last" + } + ], + "isUnique": false, + "concurrently": false, + "method": "btree", + "with": {} + } + }, + "foreignKeys": { + "track_album_id_album_id_fk": { + "name": "track_album_id_album_id_fk", + "tableFrom": "track", + "tableTo": "album", + "columnsFrom": ["album_id"], + "columnsTo": ["id"], + "onDelete": "cascade", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "policies": {}, + "checkConstraints": {}, + "isRLSEnabled": false + } + }, + "enums": {}, + "schemas": {}, + "sequences": {}, + "roles": {}, + "policies": {}, + "views": {}, + "_meta": { + "columns": {}, + "schemas": {}, + "tables": {} + } +} diff --git a/server/drizzle/meta/_journal.json b/server/drizzle/meta/_journal.json index 4eb27ca..23ce9e2 100644 --- a/server/drizzle/meta/_journal.json +++ b/server/drizzle/meta/_journal.json @@ -29,6 +29,13 @@ "when": 1793199189807, "tag": "0003_album-average-rating", "breakpoints": true + }, + { + "idx": 4, + "version": "7", + "when": 1793889343448, + "tag": "0004_album-artist-position", + "breakpoints": true } ] } diff --git a/server/src/db/schema.ts b/server/src/db/schema.ts index b3f30f3..1b8fa8e 100644 --- a/server/src/db/schema.ts +++ b/server/src/db/schema.ts @@ -56,6 +56,9 @@ export const albumArtist = pgTable( artistId: integer('artist_id') .notNull() .references(() => artist.id, { onDelete: 'cascade' }), + // Credit order from the source, so "Bob James & David Sanborn" is not + // shown the other way round because one artist row happened to exist first. + position: smallint('position').notNull().default(0), }, (t) => [primaryKey({ columns: [t.albumId, t.artistId] })], ); diff --git a/server/src/db/seed.ts b/server/src/db/seed.ts index 7983a5f..13ddabb 100644 --- a/server/src/db/seed.ts +++ b/server/src/db/seed.ts @@ -83,9 +83,13 @@ await db.transaction(async (tx) => { .values({ title: a.title, releaseYear: a.year }) .returning({ id: album.id }); const albumId = row!.id; - await tx - .insert(albumArtist) - .values(a.artists.map((n) => ({ albumId, artistId: artistId.get(n)! }))); + await tx.insert(albumArtist).values( + a.artists.map((n, position) => ({ + albumId, + artistId: artistId.get(n)!, + position, + })), + ); await tx .insert(albumGenre) .values(a.genres.map((n) => ({ albumId, genreId: genreId.get(n)! }))); diff --git a/server/src/loaders/index.ts b/server/src/loaders/index.ts index 8e0756c..aecefe7 100644 --- a/server/src/loaders/index.ts +++ b/server/src/loaders/index.ts @@ -40,7 +40,7 @@ export function createLoaders(db: Db, userId: string | null) { .from(albumArtist) .innerJoin(artist, eq(artist.id, albumArtist.artistId)) .where(inArray(albumArtist.albumId, [...albumIds])) - .orderBy(asc(artist.id)); + .orderBy(asc(albumArtist.position), asc(artist.id)); return groupByAlbum(albumIds, rows); }), From ffac90401a01714566d74e27d1c96b406487cac3 Mon Sep 17 00:00:00 2001 From: Even Date: Thu, 5 Nov 2026 15:53:21 +0100 Subject: [PATCH 2/5] fix: #64 ASCII hyphens and replace stale links on reimport --- server/src/db/musicBrainzCatalog.ts | 79 +++++++++++++++++++++++++---- 1 file changed, 69 insertions(+), 10 deletions(-) diff --git a/server/src/db/musicBrainzCatalog.ts b/server/src/db/musicBrainzCatalog.ts index a0196bc..b22d3ad 100644 --- a/server/src/db/musicBrainzCatalog.ts +++ b/server/src/db/musicBrainzCatalog.ts @@ -1,4 +1,4 @@ -import { and, eq, isNull } from 'drizzle-orm'; +import { and, eq, isNull, notInArray, sql } from 'drizzle-orm'; import { album, albumArtist, albumGenre, artist, genre, track } from './schema.js'; import type { Db } from './client.js'; @@ -17,6 +17,15 @@ export type MusicBrainzAlbum = { tracks: MusicBrainzTrack[]; }; +/** + * MusicBrainz writes hyphens as U+2010 ("Esa-Pekka", "Switched-On"). The + * search tokenizer only joins words around an ASCII hyphen, so a search for + * "esa-pekka" found nothing. Store the ASCII hyphen and collapse whitespace. + */ +export function cleanText(value: string): string { + return value.normalize('NFC').replace(/[‐‑]/g, '-').replace(/\s+/g, ' ').trim(); +} + export function parseMusicBrainzCatalog(value: unknown): MusicBrainzAlbum[] { if (!value || typeof value !== 'object' || !('albums' in value)) { throw new Error('MusicBrainz catalog must contain an albums array'); @@ -96,7 +105,7 @@ export function parseMusicBrainzCatalog(value: unknown): MusicBrainzAlbum[] { throw new Error(`Album ${index} track ${trackIndex} has an invalid duration`); } return { - title: item.title, + title: cleanText(item.title), trackNumber: item.trackNumber as number, durationSeconds: item.durationSeconds as number | null, }; @@ -104,10 +113,10 @@ export function parseMusicBrainzCatalog(value: unknown): MusicBrainzAlbum[] { return { releaseGroupId, - title: title.trim(), + title: cleanText(title), releaseYear: releaseYear as number | null, - artists: [...new Set((artists as string[]).map((name) => name.trim()))], - genres: [...new Set((genres as string[]).map((name) => name.trim()))], + artists: [...new Set((artists as string[]).map(cleanText))], + genres: [...new Set((genres as string[]).map(cleanText))], tracks: normalizedTracks, }; }); @@ -116,6 +125,8 @@ export function parseMusicBrainzCatalog(value: unknown): MusicBrainzAlbum[] { export async function importMusicBrainzAlbums(database: Db, records: MusicBrainzAlbum[]) { let inserted = 0; let updated = 0; + const unlinkedArtistIds = new Set(); + const unlinkedGenreIds = new Set(); await database.transaction(async (transaction) => { const findOrCreateArtist = async (name: string): Promise => { @@ -192,23 +203,56 @@ export async function importMusicBrainzAlbums(database: Db, records: MusicBrainz updated += 1; } + // The snapshot is the source of truth for its albums' artists, genres + // and tracks. A reimport replaces links and tracks that the snapshot no + // longer has, instead of leaving the old ones next to the new ones. + // Reviews and crate items hang off the album ID and are not touched. const artistIds: number[] = []; for (const name of record.artists) artistIds.push(await findOrCreateArtist(name)); - for (const artistId of artistIds) { + const removedArtists = await transaction + .delete(albumArtist) + .where( + and( + eq(albumArtist.albumId, albumId), + notInArray(albumArtist.artistId, artistIds), + ), + ) + .returning({ id: albumArtist.artistId }); + for (const { id } of removedArtists) unlinkedArtistIds.add(id); + for (const [position, artistId] of artistIds.entries()) { await transaction .insert(albumArtist) - .values({ albumId, artistId }) - .onConflictDoNothing(); + .values({ albumId, artistId, position }) + .onConflictDoUpdate({ + target: [albumArtist.albumId, albumArtist.artistId], + set: { position }, + }); } - for (const name of record.genres) { - const genreId = await findOrCreateGenre(name); + const genreIds: number[] = []; + for (const name of record.genres) genreIds.push(await findOrCreateGenre(name)); + const removedGenres = await transaction + .delete(albumGenre) + .where( + and(eq(albumGenre.albumId, albumId), notInArray(albumGenre.genreId, genreIds)), + ) + .returning({ id: albumGenre.genreId }); + for (const { id } of removedGenres) unlinkedGenreIds.add(id); + for (const genreId of genreIds) { await transaction .insert(albumGenre) .values({ albumId, genreId }) .onConflictDoNothing(); } + const trackNumbers = record.tracks.map((item) => item.trackNumber); + await transaction + .delete(track) + .where( + trackNumbers.length + ? and(eq(track.albumId, albumId), notInArray(track.trackNumber, trackNumbers)) + : eq(track.albumId, albumId), + ); for (const item of record.tracks) { await transaction .insert(track) @@ -224,6 +268,21 @@ export async function importMusicBrainzAlbums(database: Db, records: MusicBrainz }); } } + + // A renamed artist or genre leaves a row no album uses any more. Only + // rows this import unlinked are candidates, so it never touches others. + if (unlinkedArtistIds.size) { + await transaction.execute(sql` + DELETE FROM artist + WHERE id IN (${sql.join([...unlinkedArtistIds], sql`, `)}) + AND NOT EXISTS (SELECT 1 FROM album_artist WHERE artist_id = artist.id)`); + } + if (unlinkedGenreIds.size) { + await transaction.execute(sql` + DELETE FROM genre + WHERE id IN (${sql.join([...unlinkedGenreIds], sql`, `)}) + AND NOT EXISTS (SELECT 1 FROM album_genre WHERE genre_id = genre.id)`); + } }); return { inserted, updated }; From b9652d3e4e5f9cc1b44ec5cc3ac6db6c20585842 Mon Sep 17 00:00:00 2001 From: Even Date: Thu, 5 Nov 2026 15:53:22 +0100 Subject: [PATCH 3/5] test: #64 catalog text cleanup and reimport sync --- server/src/db/musicBrainzCatalog.test.ts | 102 ++++++++++++++++++++++- 1 file changed, 101 insertions(+), 1 deletion(-) diff --git a/server/src/db/musicBrainzCatalog.test.ts b/server/src/db/musicBrainzCatalog.test.ts index 6981349..c7ddd66 100644 --- a/server/src/db/musicBrainzCatalog.test.ts +++ b/server/src/db/musicBrainzCatalog.test.ts @@ -1,7 +1,7 @@ import { readFile } from 'node:fs/promises'; import { randomUUID } from 'node:crypto'; import { drizzle } from 'drizzle-orm/node-postgres'; -import { eq } from 'drizzle-orm'; +import { asc, eq } from 'drizzle-orm'; import pg from 'pg'; import { afterAll, describe, expect, it } from 'vitest'; import { @@ -11,9 +11,11 @@ import { import { album, albumArtist, + albumGenre, appUser, artist, crateItem, + genre, review, track, } from './schema.js'; @@ -42,6 +44,28 @@ describe('MusicBrainz catalog snapshot', () => { ).toBe(true); }); + it('stores ASCII hyphens and single spaces so searches for them match', () => { + const [parsed] = parseMusicBrainzCatalog({ + albums: [ + { + releaseGroupId: 'd7f4c5cc-3b9b-4e6e-8e4d-4ae6b7020f50', + title: 'Switched‐On Bach', + releaseYear: 1968, + artists: ['Esa‐Pekka Salonen', 'Esa-Pekka Salonen'], + genres: [' Classical '], + tracks: [{ title: 'Air‑on G ', trackNumber: 1, durationSeconds: null }], + }, + ], + }); + expect(parsed).toMatchObject({ + title: 'Switched-On Bach', + artists: ['Esa-Pekka Salonen'], + genres: ['Classical'], + tracks: [{ title: 'Air-on G' }], + }); + expect(JSON.stringify(catalog)).not.toMatch(/[‐‑]/); + }); + it('rejects repeated release groups and malformed catalog records', () => { const record = { releaseGroupId: 'd7f4c5cc-3b9b-4e6e-8e4d-4ae6b7020f50', @@ -151,6 +175,82 @@ describe.skipIf(!process.env.TEST_DATABASE_URL)('MusicBrainz catalog import', () } }); + it('replaces changed artists, genres and tracks on reimport and keeps user data', async () => { + const id = randomUUID(); + const userId = randomUUID(); + const names = ['First', 'Second', 'Old'].map((name) => `${name} artist ${id}`); + const genres = ['New', 'Old'].map((name) => `${name} genre ${id}`); + const before = { + releaseGroupId: id, + title: `Reimport fixture ${id}`, + releaseYear: 2001, + artists: [names[2]!, names[1]!], + genres: [genres[1]!], + tracks: [1, 2, 3].map((trackNumber) => ({ + title: `Old track ${trackNumber}`, + trackNumber, + durationSeconds: 60, + })), + }; + // Second is created first, so it has the lower artist id. + const after = { + ...before, + artists: [names[0]!, names[1]!], + genres: [genres[0]!], + tracks: [{ title: 'New track', trackNumber: 1, durationSeconds: 90 }], + }; + + try { + await importMusicBrainzAlbums(database, [before]); + const [imported] = await database + .select() + .from(album) + .where(eq(album.musicBrainzReleaseGroupId, id)); + await database.insert(appUser).values({ id: userId, displayName: 'Reimport user' }); + await database.insert(review).values({ userId, albumId: imported!.id, rating: 3 }); + await database.insert(crateItem).values({ userId, albumId: imported!.id }); + + await importMusicBrainzAlbums(database, [after]); + + const artists = await database + .select({ name: artist.name }) + .from(albumArtist) + .innerJoin(artist, eq(artist.id, albumArtist.artistId)) + .where(eq(albumArtist.albumId, imported!.id)) + .orderBy(asc(albumArtist.position), asc(artist.id)); + const linkedGenres = await database + .select({ name: genre.name }) + .from(albumGenre) + .innerJoin(genre, eq(genre.id, albumGenre.genreId)) + .where(eq(albumGenre.albumId, imported!.id)); + const tracks = await database + .select({ title: track.title }) + .from(track) + .where(eq(track.albumId, imported!.id)); + expect(artists.map((row) => row.name)).toEqual([names[0], names[1]]); + expect(linkedGenres.map((row) => row.name)).toEqual([genres[0]]); + expect(tracks.map((row) => row.title)).toEqual(['New track']); + // The artist and genre no album uses any more are gone. + expect( + await database.select().from(artist).where(eq(artist.name, names[2]!)), + ).toHaveLength(0); + expect( + await database.select().from(genre).where(eq(genre.name, genres[1]!)), + ).toHaveLength(0); + expect( + await database.select().from(review).where(eq(review.userId, userId)), + ).toHaveLength(1); + expect( + await database.select().from(crateItem).where(eq(crateItem.userId, userId)), + ).toHaveLength(1); + } finally { + await pool.query('DELETE FROM app_user WHERE id = $1', [userId]); + await pool.query('DELETE FROM album WHERE musicbrainz_release_group_id = $1', [id]); + await pool.query('DELETE FROM genre WHERE name = ANY($1)', [genres]); + await pool.query('DELETE FROM artist WHERE name = ANY($1)', [names]); + } + }); + it('keeps same-title albums with different release-group IDs separate', async () => { const id = randomUUID(); const title = `Same title fixture ${id}`; From c5967c19c87ba792d96e0df184e2e792ca8157a3 Mon Sep 17 00:00:00 2001 From: Even Date: Thu, 5 Nov 2026 15:53:22 +0100 Subject: [PATCH 4/5] docs: #64 catalog quality checks and limitations --- README.md | 30 ++++++++++++++++++++++++++++++ docs/AI-USAGE.md | 1 + docs/data-model.md | 5 ++++- 3 files changed, 35 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 71f69a2..e27f941 100644 --- a/README.md +++ b/README.md @@ -135,6 +135,36 @@ reproducibility. See [MusicBrainz Data License](https://musicbrainz.org/doc/Abou and [MusicBrainz API](https://musicbrainz.org/doc/MusicBrainz_API). Album covers remain out of scope for this issue. +### Catalog quality (#64) + +We checked the snapshot and the imported catalog (516 albums with the sample +seed) for duplicates, missing metadata, artist and genre links and track order. +Every sort was paged through the whole result set without gaps or repeats, and +the facet counts match the filtered totals. Fixes from the check: + +- MusicBrainz writes hyphens as U+2010, so "esa-pekka" or "switched-on" found + nothing. The importer now stores ASCII hyphens. +- Artists are shown in credit order ("Bob James, David Sanborn"), not in the + order their rows happened to be created. +- A reimport replaces artists, genres and tracks the snapshot no longer has, + instead of keeping the old ones next to the new ones. Reviews and crate items + are kept. + +Remaining limitations: + +- Only 50 of the 500 albums have a track list, taken from one official release + of the release group. The album page says when there are no tracks. +- Seven albums have no release year. They sort last and have no decade facet. +- Search is not accent-insensitive: "beyonce" does not find "Beyoncé", and + names in other scripts (for example Mulatu Astatke as ሙላቱ አስታጥቄ) only match + as written. +- Artists are matched by name, so two different artists with the same name + would share a row. None were found in the snapshot. +- Classical albums credit the composer as an artist, and compilations use + "Various Artists", as MusicBrainz does. +- Same title and artist is not always a duplicate: Color Humano released three + albums called "Color Humano" (1972-1974), kept as three albums. + ### Scalability check Use a separate test database, not the shared catalog. Apply migrations, run the large seed and start the API. The SQL checks and GraphQL request are committed in the repository. diff --git a/docs/AI-USAGE.md b/docs/AI-USAGE.md index 7cb270d..5ecfa82 100644 --- a/docs/AI-USAGE.md +++ b/docs/AI-USAGE.md @@ -75,6 +75,7 @@ the part that matters most. | 2026-10-28 | evenkkl | Claude | #12, review fixes on #66. Robert asked for an up-to-date branch, a correct README and green CI. Claude merged main, renumbered our migration to `0003` after #63's `0002`, kept both sides of the other conflicts, fixed a duplicate import from the merge, updated the README status sections and added the sort control to the Load more keyboard test. I checked the diff against main contains only our sorting work and these fixes. | | 2026-10-28 | evenkkl | Claude | #13 review fixes on PR #65: merged main (infinite scroll), resolved conflicts, added a Filters button for small screens and reconciled the README. Even settled the open review questions and updated the issue text. | | 2026-10-29 | evenkkl | Claude | #13 second review round on PR #65: merged main (sorting), selected filters stay in the facets, Filters dropdown on all screens, PostgreSQL tests for AND/OR and facet counts, doc wording. Even chose the dropdown layout. | +| 2026-11-05 | evenkkl | Claude | #64 catalog check: scripts that compared the snapshot and the imported catalog against search, sorting, paging and facets; fixes for U+2010 hyphens in search, artist credit order and stale links on reimport; PostgreSQL tests and README limitations. | ## Notes diff --git a/docs/data-model.md b/docs/data-model.md index b9cc70a..7e825c0 100644 --- a/docs/data-model.md +++ b/docs/data-model.md @@ -21,7 +21,10 @@ catalog data. The separate MusicBrainz importer adds real catalog records from the committed snapshot without deleting albums, tracks, reviews or crate items. It matches an existing hand-seeded album by title and primary artist before assigning its MusicBrainz ID, preserving its database ID and user data. The -unique MusicBrainz release-group ID makes repeat imports idempotent. +unique MusicBrainz release-group ID makes repeat imports idempotent. For the +albums in the snapshot, a reimport replaces artist links, genre links and +tracks with the snapshot's, and removes artists and genres it left unused. +`album_artist.position` keeps the credit order (migration `0004`). The local snapshot keeps one record per release group, not per pressing or reissue. It stores normalized genre names and any available track data. The From a2f9e205997bf4e11a5550c49420f9471d2267fc Mon Sep 17 00:00:00 2001 From: Even Date: Thu, 5 Nov 2026 21:58:21 +0100 Subject: [PATCH 5/5] style: #64 format AI usage log --- docs/AI-USAGE.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/AI-USAGE.md b/docs/AI-USAGE.md index 5ecfa82..4612092 100644 --- a/docs/AI-USAGE.md +++ b/docs/AI-USAGE.md @@ -75,7 +75,7 @@ the part that matters most. | 2026-10-28 | evenkkl | Claude | #12, review fixes on #66. Robert asked for an up-to-date branch, a correct README and green CI. Claude merged main, renumbered our migration to `0003` after #63's `0002`, kept both sides of the other conflicts, fixed a duplicate import from the merge, updated the README status sections and added the sort control to the Load more keyboard test. I checked the diff against main contains only our sorting work and these fixes. | | 2026-10-28 | evenkkl | Claude | #13 review fixes on PR #65: merged main (infinite scroll), resolved conflicts, added a Filters button for small screens and reconciled the README. Even settled the open review questions and updated the issue text. | | 2026-10-29 | evenkkl | Claude | #13 second review round on PR #65: merged main (sorting), selected filters stay in the facets, Filters dropdown on all screens, PostgreSQL tests for AND/OR and facet counts, doc wording. Even chose the dropdown layout. | -| 2026-11-05 | evenkkl | Claude | #64 catalog check: scripts that compared the snapshot and the imported catalog against search, sorting, paging and facets; fixes for U+2010 hyphens in search, artist credit order and stale links on reimport; PostgreSQL tests and README limitations. | +| 2026-11-05 | evenkkl | Claude | #64 catalog check: scripts that compared the snapshot and the imported catalog against search, sorting, paging and facets; fixes for U+2010 hyphens in search, artist credit order and stale links on reimport; PostgreSQL tests and README limitations. | ## Notes