From cd46ac397f0800e8c91c695dc9aed4835f0e65b0 Mon Sep 17 00:00:00 2001 From: kami Date: Thu, 30 Jul 2026 23:36:11 +0400 Subject: [PATCH] fix: populate canonical_name and stop writing generated normalized_name Three separate insert paths made a fresh Postgres volume unusable. The live database only works because its volume predates the constraints. - scanner.service.resolveOrCreateArtist inserted only (name), but schema.sql declares canonical_name NOT NULL with no default. Every artist insert failed, and processFile swallows per-file errors, so a scan reported success with 0 tracks and a permanently empty library. - enrichment.service inserted explicitly into artists.normalized_name, which is GENERATED ALWAYS AS (normalize_artist(name)) STORED: "cannot insert a non-DEFAULT value into column" (428C9). All enrichment artist creation failed on a fresh volume. - db.service.createArtist omitted canonical_name, same failure. canonical_name holds the raw tag name, not normalize_artist() output, which truncates on `/` and a standalone `x` ("AC/DC" -> "AC"). That is the convention createLocalArtist already used. The truncation bug in artists.name is pre-existing and deliberately left untouched here. Verified on a scratch postgres:16-alpine with the real schema: the old statement reproduces the NOT NULL violation, the new path yields artists/albums/tracks/track_artists rows. REVIEW-2026-07-30.md finding 1. Co-Authored-By: Claude Opus 5 --- backend/src/services/db.service.ts | 11 ++++++++--- workers/src/enrichment.service.ts | 17 +++++++++++------ workers/src/scanner.service.ts | 12 ++++++++++-- 3 files changed, 29 insertions(+), 11 deletions(-) diff --git a/backend/src/services/db.service.ts b/backend/src/services/db.service.ts index b2f705c..f560f65 100644 --- a/backend/src/services/db.service.ts +++ b/backend/src/services/db.service.ts @@ -1147,9 +1147,14 @@ export class DbService { async createArtist(data: Artist): Promise { const res = await this.pgClient.query( - `INSERT INTO artists (name, mbid, discogs_id, image_path) - VALUES (normalize_artist($1), $2, $3, $4) RETURNING *`, - [data.name, data.mbid, data.discogs_id, data.image_path] + // `canonical_name` is NOT NULL with no default, so omitting it fails on a + // fresh volume (the live DB's column predates the constraint). It holds the + // DISPLAY name: the raw input, not normalize_artist()'s output, which + // truncates on `/` and a standalone `x` ("AC/DC" -> "AC"). Matches the + // convention in workers' scanner.service.resolveOrCreateArtist. + `INSERT INTO artists (name, canonical_name, mbid, discogs_id, image_path) + VALUES (normalize_artist($1), $2, $3, $4, $5) RETURNING *`, + [data.name, data.name?.trim() || data.name, data.mbid, data.discogs_id, data.image_path] ); return res.rows[0]; } diff --git a/workers/src/enrichment.service.ts b/workers/src/enrichment.service.ts index e943b58..8a76d1b 100644 --- a/workers/src/enrichment.service.ts +++ b/workers/src/enrichment.service.ts @@ -330,14 +330,17 @@ export class EnrichmentService { const sortName = mbArtist.sortName || this.generateSortName(canonicalName); const newArtist = await this.pgClient.query( - `INSERT INTO artists (name, canonical_name, sort_name, mbid, normalized_name) - VALUES ($1, $2, $3, $4, $5) + // normalized_name is a GENERATED ALWAYS column in schema.sql + // (normalize_artist(name)); writing to it explicitly errors with + // 428C9 on any database built from schema.sql. Let Postgres derive it. + `INSERT INTO artists (name, canonical_name, sort_name, mbid) + VALUES ($1, $2, $3, $4) ON CONFLICT (mbid) DO UPDATE SET canonical_name = EXCLUDED.canonical_name, sort_name = EXCLUDED.sort_name, name = EXCLUDED.name RETURNING id`, - [rawArtistName, canonicalName, sortName, mbArtist.artistMbid, normalized] + [rawArtistName, canonicalName, sortName, mbArtist.artistMbid] ); const artistId = newArtist.rows[0].id; @@ -371,10 +374,12 @@ export class EnrichmentService { const sortName = this.generateSortName(rawName); const result = await this.pgClient.query( - `INSERT INTO artists (name, canonical_name, sort_name, normalized_name) - VALUES ($1, $2, $3, $4) + // normalized_name is GENERATED ALWAYS (normalize_artist(name)) in + // schema.sql — inserting it explicitly fails with 428C9. Derived by PG. + `INSERT INTO artists (name, canonical_name, sort_name) + VALUES ($1, $2, $3) RETURNING id`, - [rawName, rawName, sortName, normalized] + [rawName, rawName, sortName] ); const artistId = result.rows[0].id; diff --git a/workers/src/scanner.service.ts b/workers/src/scanner.service.ts index 62e0bc8..2e565db 100644 --- a/workers/src/scanner.service.ts +++ b/workers/src/scanner.service.ts @@ -237,9 +237,17 @@ export class ScannerService { return { id: String(found.rows[0].id), name: String(found.rows[0].name) }; } + // `canonical_name` is NOT NULL in schema.sql with no default, so it MUST be + // supplied here — omitting it makes every artist insert fail on a fresh + // volume (and processFile swallows the error, so the scan silently yields an + // empty library). It holds the DISPLAY name: we store the raw tag name, not + // the normalize_artist() output, because that function truncates on `/` and + // a standalone `x` ("AC/DC" -> "AC", "Felix Mendelssohn" -> "Feli"). The + // enrichment path later overwrites canonical_name with the MusicBrainz name; + // until then the raw tag is the most faithful display value we have. const inserted = await this.pgClient.query( - 'INSERT INTO artists (name) VALUES ($1) RETURNING id, name', - [name] + 'INSERT INTO artists (name, canonical_name) VALUES ($1, $2) RETURNING id, name', + [name, rawName.trim() || name] ); return { id: String(inserted.rows[0].id), name: String(inserted.rows[0].name) }; }