feat(explore): carry multi-artist credits in the catalog

A track credited to more than one artist has exactly one navigable artist in this app and the rest are punctuation. `primaryArtist()` string-parses the credit, strips a " feat. " clause and discards the guest; it deliberately does not split on "&", "with" or "," because those live inside real artist names. Measured on a real 26,069-file library plus an 80+80 MusicBrainz sample: 13% of recordings are multi-artist upstream, while only 0.86% of files carry any structured multi-artist tag — mp3 carries zero files with multiple MUSICBRAINZ_ARTISTID across 19,840. Of 1,286 files saying "feat.", 90% have nothing structured behind it, and a sample of 80 such files was multi-artist in MB 80 times out of 80. CLAUDE.md justified plan 013's removal of the credit tables with "3 credits of 2,823 listed more than one artist". That measured our own *writer* — cachedLinkArtist was called once per credit, so a collaboration could never have been recorded. Dropping the join table was still right on cost; the evidence for "multi-artist is rare" was not. A credit is ordered parts and the credit string is derived from them, so join phrases are assembly instructions, not disassembly ones. Nothing here reconstructs a credit by searching a name inside a credit string: the stored text may come from tags while the parts come from the catalog, and those disagree for ~1 in 3 multi-artist credits. Where it comes from, after two dead ends: the canonical dump CI already streams has no join phrases and no as-credited names, and the JSON dumps cover 153,691 recordings of ~35M with *zero* overlap against a real library. So mbdump.tar.bz2 — 7.1 GB, ~13.7 min in pure-Go bzip2, whose members are alphabetical, which is what lets one pass resolve an entity's credit without buffering 35M recordings. - artist_credit_part / artist_credit_ref, multi-artist credits only: a single-artist credit is already explore_index's own artist_name. - Column layouts verified against the real 20260815 export; ErrDumpShape makes a wrong guess a failed build, not a wrong catalog. - The pass runs on every mode, not just a build. The job picks its mode from the index's own state, and a complete import means "refresh", which never enters the importer — so credits could otherwise only arrive via a rebuild that re-downloads ~205 GB. It reports whether it populated anything, which is what flips `changed` and republishes. - The importer asks whether an artifact carries the tables, on the writer where `core` is attached, so the artifact already published still imports. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AfVYUVExXsx1nSWrXN8mAh
2026-08-17 08:25:36 -04:00
parent 66182f82cd
commit b3737d30af
19 changed files with 2278 additions and 2 deletions
@@ -3,6 +3,7 @@ package explore
 import (
 	"context"
 	"database/sql"
+	"encoding/hex"
 	"os"
 	"path/filepath"
 	"strings"
@@ -597,3 +598,153 @@ func TestImportCoreArtifactReadsTotalsWhenPresent(t *testing.T) {
 		t.Errorf("TotalTracks = %d, want 0 (the catalog does not say)", old.TotalTracks)
 	}
 }
+
+// addArtifactCredits gives an artifact file the credit tables the
+// exporter now writes, so the import path can be exercised against one
+// that has them.
+func addArtifactCredits(t *testing.T, path string) {
+	t.Helper()
+
+	db, err := sql.Open("sqlite", "file:"+path)
+	if err != nil {
+		t.Fatalf("open artifact: %v", err)
+	}
+
+	defer func() { _ = db.Close() }()
+
+	for _, stmt := range []string{
+		`CREATE TABLE artist_credit_part (
+			credit_id     INTEGER NOT NULL,
+			position      INTEGER NOT NULL,
+			artist_mbid   BLOB NOT NULL,
+			credited_name TEXT NOT NULL,
+			join_phrase   TEXT NOT NULL DEFAULT '',
+			PRIMARY KEY (credit_id, position)
+		) WITHOUT ROWID`,
+		`CREATE TABLE artist_credit_ref (
+			mbid      BLOB NOT NULL PRIMARY KEY,
+			credit_id INTEGER NOT NULL
+		) WITHOUT ROWID`,
+	} {
+		if _, err := db.Exec(stmt); err != nil {
+			t.Fatalf("create credit tables: %v", err)
+		}
+	}
+
+	// The packed form the catalog stores.  uuid16/parseUUID live behind
+	// the indexbuild tag, so this file decodes for itself.
+	pack := func(mbid string) []byte {
+		raw, err := hex.DecodeString(strings.ReplaceAll(mbid, "-", ""))
+		if err != nil || len(raw) != 16 {
+			t.Fatalf("fixture MBID %q is not a UUID: %v", mbid, err)
+		}
+
+		return raw
+	}
+
+	a, b, rec := pack(artA), pack(artB), pack(recA)
+
+	for _, part := range [][]any{
+		{7, 0, a, "Artist A", " feat. "},
+		{7, 1, b, "Artist B", ""},
+	} {
+		if _, err := db.Exec(`INSERT INTO artist_credit_part
+			(credit_id, position, artist_mbid, credited_name, join_phrase)
+			VALUES (?, ?, ?, ?, ?)`, part...); err != nil {
+			t.Fatalf("insert part: %v", err)
+		}
+	}
+
+	if _, err := db.Exec(
+		"INSERT INTO artist_credit_ref (mbid, credit_id) VALUES (?, ?)", rec, 7,
+	); err != nil {
+		t.Fatalf("insert ref: %v", err)
+	}
+}
+
+// TestImportCoreArtifactMergesCredits is the positive half of the
+// compatibility pair: an artifact that carries credits delivers them,
+// rendering back to the credit string they decompose.
+func TestImportCoreArtifactMergesCredits(t *testing.T) {
+	db := database.NewTestDB(t)
+	si := NewSearchIndex(db, nil, nil, testLogger())
+
+	path := writeTestArtifact(t, validMeta(), []artifactRow{
+		{"recording", recA, "Song A", "Artist A feat. Artist B", artA, 2000},
+	})
+
+	addArtifactCredits(t, path)
+
+	if err := si.importCoreArtifact(context.Background(), path); err != nil {
+		t.Fatalf("importCoreArtifact: %v", err)
+	}
+
+	rows, err := db.QueryContext(
+		`SELECT p.credited_name, p.join_phrase
+		 FROM artist_credit_ref r
+		 JOIN artist_credit_part p ON p.credit_id = r.credit_id
+		 ORDER BY p.position`,
+	)
+	if err != nil {
+		t.Fatalf("query credits: %v", err)
+	}
+
+	defer func() { _ = rows.Close() }()
+
+	var rendered strings.Builder
+
+	for rows.Next() {
+		var name, join string
+
+		if err := rows.Scan(&name, &join); err != nil {
+			t.Fatalf("scan: %v", err)
+		}
+
+		rendered.WriteString(name)
+		rendered.WriteString(join)
+	}
+
+	if got := rendered.String(); got != "Artist A feat. Artist B" {
+		t.Errorf("rendered credit = %q, want %q", got, "Artist A feat. Artist B")
+	}
+}
+
+// TestImportCoreArtifactWithoutCredits is the regression that matters
+// most here: an artifact published before credits existed cannot be
+// re-cut retroactively, so it must import as a catalog that declines to
+// answer rather than failing outright.  writeTestArtifact deliberately
+// builds one without the tables.
+func TestImportCoreArtifactWithoutCredits(t *testing.T) {
+	db := database.NewTestDB(t)
+	si := NewSearchIndex(db, nil, nil, testLogger())
+
+	path := writeTestArtifact(t, validMeta(), []artifactRow{
+		{"recording", recA, "Song A", "Artist A", artA, 2000},
+	})
+
+	if err := si.importCoreArtifact(context.Background(), path); err != nil {
+		t.Fatalf("an artifact without credit tables must still import: %v", err)
+	}
+
+	var rows int
+	if err := db.QueryRowWriter(
+		"SELECT COUNT(*) FROM explore_index",
+	).Scan(&rows); err != nil {
+		t.Fatalf("count: %v", err)
+	}
+
+	if rows != 1 {
+		t.Errorf("catalog rows = %d, want 1", rows)
+	}
+
+	var refs int
+	if err := db.QueryRowWriter(
+		"SELECT COUNT(*) FROM artist_credit_ref",
+	).Scan(&refs); err != nil {
+		t.Fatalf("count refs: %v", err)
+	}
+
+	if refs != 0 {
+		t.Errorf("credit refs = %d, want 0", refs)
+	}
+}