From 273c8d23f18600041a242182a93f6b405aafdabe Mon Sep 17 00:00:00 2001 From: Deluan Date: Sun, 24 May 2026 18:57:30 -0300 Subject: [PATCH 01/26] feat(conf): add PID.Artist config option with default 'name' --- conf/configuration.go | 7 +++++-- consts/consts.go | 10 ++++++---- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/conf/configuration.go b/conf/configuration.go index 08f12fc94..b266ba16b 100644 --- a/conf/configuration.go +++ b/conf/configuration.go @@ -242,8 +242,9 @@ type backupOptions struct { } type pidOptions struct { - Track string - Album string + Track string + Album string + Artist string } type inspectOptions struct { @@ -439,6 +440,7 @@ func Load(noConfigDump bool) { // Make sure we don't have empty PIDs Server.PID.Album = cmp.Or(Server.PID.Album, consts.DefaultAlbumPID) Server.PID.Track = cmp.Or(Server.PID.Track, consts.DefaultTrackPID) + Server.PID.Artist = cmp.Or(Server.PID.Artist, consts.DefaultArtistPID) // Parse LastFM.Language into Languages slice (comma-separated, with fallback to DefaultInfoLanguage) Server.LastFM.Languages = parseLanguages(Server.LastFM.Language) @@ -851,6 +853,7 @@ func setViperDefaults() { viper.SetDefault("backup.count", 0) viper.SetDefault("pid.track", consts.DefaultTrackPID) viper.SetDefault("pid.album", consts.DefaultAlbumPID) + viper.SetDefault("pid.artist", consts.DefaultArtistPID) viper.SetDefault("inspect.enabled", true) viper.SetDefault("inspect.maxrequests", 1) viper.SetDefault("inspect.backloglimit", consts.RequestThrottleBacklogLimit) diff --git a/consts/consts.go b/consts/consts.go index edd8f2b54..218068c36 100644 --- a/consts/consts.go +++ b/consts/consts.go @@ -122,10 +122,12 @@ const ( const ( //DefaultAlbumPID = "album_legacy" - DefaultAlbumPID = "musicbrainz_albumid|albumartistid,album,albumversion,releasedate" - DefaultTrackPID = "musicbrainz_trackid|albumid,discnumber,tracknumber,title" - PIDAlbumKey = "PIDAlbum" - PIDTrackKey = "PIDTrack" + DefaultAlbumPID = "musicbrainz_albumid|albumartistid,album,albumversion,releasedate" + DefaultTrackPID = "musicbrainz_trackid|albumid,discnumber,tracknumber,title" + DefaultArtistPID = "name" + PIDAlbumKey = "PIDAlbum" + PIDTrackKey = "PIDTrack" + PIDArtistKey = "PIDArtist" ) const ( From 3358a8bc9105cfbd3c2c77b1868131d115d9257e Mon Sep 17 00:00:00 2001 From: Deluan Date: Sun, 24 May 2026 19:01:37 -0300 Subject: [PATCH 02/26] feat(metadata): add computeArtistPID with name-attr normalization --- model/metadata/persistent_ids.go | 60 +++++++++++++++++- model/metadata/persistent_ids_test.go | 91 +++++++++++++++++++++++++++ 2 files changed, 148 insertions(+), 3 deletions(-) diff --git a/model/metadata/persistent_ids.go b/model/metadata/persistent_ids.go index db315dc6b..df397a603 100644 --- a/model/metadata/persistent_ids.go +++ b/model/metadata/persistent_ids.go @@ -87,10 +87,64 @@ func (md Metadata) albumID(mf model.MediaFile, pidConf string) string { return computePID(mf, md, pidConf, true, id.NewHash) } -// BFR Must be configurable? +// computeArtistPID computes a persistent ID for a single participant using the +// given spec. The spec grammar is the same pipe/comma form used by Album/Track +// PIDs. Attributes recognised: +// - name → hash(clear(lower(participant.Name))) — the inner +// hash matches the historical 'albumartistid' path +// so the default spec "name" produces byte-identical +// IDs to the previous hardcoded artistID(). +// - musicbrainz_artistid → participant.MbzArtistID (opaque identifier — no +// normalization). +// - sort_name → participant.SortArtistName (raw — sort forms are +// already canonicalized by taggers, and folding case +// could collapse legitimately distinct sort variants). +// +// Unlike album/track PIDs, no library prefix is applied: artists are shared +// across libraries by design. +func computeArtistPID(p model.Participant, spec string, hash hashFunc) string { + pid := "" + fields := strings.SplitSeq(spec, "|") + for field := range fields { + attributes := strings.Split(field, ",") + values := make([]string, len(attributes)) + hasValue := false + for i, attr := range attributes { + v := getArtistPIDAttr(p, attr, hash) + if v != "" { + hasValue = true + } + values[i] = v + } + if hasValue { + pid += strings.Join(values, "\\") + break + } + } + return hash(pid) +} + +func getArtistPIDAttr(p model.Participant, attr string, hash hashFunc) string { + switch strings.TrimSpace(strings.ToLower(attr)) { + case "name": + if p.Name == "" { + return "" + } + return hash(str.Clear(strings.ToLower(p.Name))) + case "musicbrainz_artistid": + return p.MbzArtistID + case "sort_name": + return p.SortArtistName + } + return "" +} + +// artistID is kept temporarily as a thin wrapper so any in-progress callers +// continue to compile; it will be removed once map_participants.go switches +// to computeArtistPID directly (Task 4). func (md Metadata) artistID(name string) string { - mf := model.MediaFile{AlbumArtist: name} - return computePID(mf, md, "albumartistid", false, id.NewHash) + return computeArtistPID(model.Participant{Artist: model.Artist{Name: name}}, + conf.Server.PID.Artist, id.NewHash) } func (md Metadata) mapTrackTitle() string { diff --git a/model/metadata/persistent_ids_test.go b/model/metadata/persistent_ids_test.go index eb66d11d1..1163189d3 100644 --- a/model/metadata/persistent_ids_test.go +++ b/model/metadata/persistent_ids_test.go @@ -5,7 +5,9 @@ import ( "github.com/navidrome/navidrome/conf" "github.com/navidrome/navidrome/conf/configtest" + "github.com/navidrome/navidrome/consts" "github.com/navidrome/navidrome/model" + "github.com/navidrome/navidrome/model/id" "github.com/navidrome/navidrome/tests" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" @@ -291,3 +293,92 @@ var _ = Describe("getPID", func() { }) }) }) + +var _ = Describe("computeArtistPID", func() { + var p model.Participant + BeforeEach(func() { + DeferCleanup(configtest.SetupConfig()) + p = model.Participant{Artist: model.Artist{Name: "The Beatles"}} + }) + + Context("default spec 'name'", func() { + BeforeEach(func() { conf.Server.PID.Artist = consts.DefaultArtistPID }) + + It("produces the same ID as the legacy artistID(name) hardcoded function", func() { + // Legacy formula: hash(hash(clear(lower(name)))) + // It came from artistID(name) routing through computePID(spec="albumartistid"), + // which inside getPIDAttr returns hash(clear(lower(mf.AlbumArtist))), then + // computePID hashes that again. + mfWithAlbumArtist := model.MediaFile{AlbumArtist: p.Name} + legacyMD := Metadata{} + legacyID := computePID(mfWithAlbumArtist, legacyMD, "albumartistid", false, id.NewHash) + + newID := computeArtistPID(p, conf.Server.PID.Artist, id.NewHash) + Expect(newID).To(Equal(legacyID)) + }) + + It("normalizes name (clear(lower(...))) before hashing", func() { + // Different casing / punctuation must collapse to the same ID. + p2 := model.Participant{Artist: model.Artist{Name: "the beatles"}} + Expect(computeArtistPID(p, conf.Server.PID.Artist, id.NewHash)). + To(Equal(computeArtistPID(p2, conf.Server.PID.Artist, id.NewHash))) + }) + }) + + Context("spec 'musicbrainz_artistid|name'", func() { + BeforeEach(func() { conf.Server.PID.Artist = "musicbrainz_artistid|name" }) + + It("uses MBID when present", func() { + p.MbzArtistID = "mbid-1" + withMBID := computeArtistPID(p, conf.Server.PID.Artist, id.NewHash) + + p2 := model.Participant{Artist: model.Artist{Name: "Different", MbzArtistID: "mbid-1"}} + withMBIDOtherName := computeArtistPID(p2, conf.Server.PID.Artist, id.NewHash) + + Expect(withMBID).To(Equal(withMBIDOtherName)) + }) + + It("falls back to normalized name when MBID is missing", func() { + withoutMBID := computeArtistPID(p, conf.Server.PID.Artist, id.NewHash) + byName := computeArtistPID(p, "name", id.NewHash) + Expect(withoutMBID).To(Equal(byName)) + }) + }) + + Context("composite spec 'musicbrainz_artistid|sort_name,name'", func() { + BeforeEach(func() { conf.Server.PID.Artist = "musicbrainz_artistid|sort_name,name" }) + + It("hashes sort+name composite when MBID is absent", func() { + p.SortArtistName = "Beatles, The" + withSort := computeArtistPID(p, conf.Server.PID.Artist, id.NewHash) + + p2 := p + p2.SortArtistName = "Different Sort" + withDifferentSort := computeArtistPID(p2, conf.Server.PID.Artist, id.NewHash) + + Expect(withSort).NotTo(Equal(withDifferentSort)) + }) + }) + + Context("spec 'sort_name|name' with empty sort_name", func() { + BeforeEach(func() { conf.Server.PID.Artist = "sort_name|name" }) + + It("falls through to name when SortArtistName is empty", func() { + // sort_name is empty → first pipe alternative produces no value → + // engine falls through to name. + fallback := computeArtistPID(p, conf.Server.PID.Artist, id.NewHash) + byName := computeArtistPID(p, "name", id.NewHash) + Expect(fallback).To(Equal(byName)) + }) + }) + + Context("empty inputs", func() { + BeforeEach(func() { conf.Server.PID.Artist = "musicbrainz_artistid|name" }) + + It("does not panic on empty participant", func() { + empty := model.Participant{Artist: model.Artist{Name: ""}} + Expect(func() { computeArtistPID(empty, conf.Server.PID.Artist, id.NewHash) }). + NotTo(Panic()) + }) + }) +}) From 398efa04a35ec1ede42896afa8ed840c7b691f8b Mon Sep 17 00:00:00 2001 From: Deluan Date: Sun, 24 May 2026 19:09:36 -0300 Subject: [PATCH 03/26] feat(model): add CreditedAs to Participant --- model/participants.go | 3 ++- model/participants_test.go | 30 ++++++++++++++++++++++++++++++ 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/model/participants.go b/model/participants.go index afbda10de..ec1b8e863 100644 --- a/model/participants.go +++ b/model/participants.go @@ -77,7 +77,8 @@ func RoleFromString(role string) Role { type Participant struct { Artist - SubRole string `json:"subRole,omitempty"` + SubRole string `json:"subRole,omitempty"` + CreditedAs string `json:"creditedAs,omitempty"` } type ParticipantList []Participant diff --git a/model/participants_test.go b/model/participants_test.go index dad84b6dd..3e7ba2942 100644 --- a/model/participants_test.go +++ b/model/participants_test.go @@ -182,6 +182,36 @@ var _ = Describe("Participants", func() { }) }) +var _ = Describe("Participant.CreditedAs", func() { + It("round-trips through JSON when set", func() { + p := Participant{ + Artist: Artist{ID: "abc", Name: "Planetary Assault Systems"}, + CreditedAs: "PAS", + } + data, err := json.Marshal(p) + Expect(err).ToNot(HaveOccurred()) + Expect(string(data)).To(ContainSubstring(`"creditedAs":"PAS"`)) + + var back Participant + Expect(json.Unmarshal(data, &back)).To(Succeed()) + Expect(back.CreditedAs).To(Equal("PAS")) + }) + + It("omits CreditedAs from JSON when empty", func() { + p := Participant{Artist: Artist{ID: "abc", Name: "Foo"}} + data, err := json.Marshal(p) + Expect(err).ToNot(HaveOccurred()) + Expect(string(data)).NotTo(ContainSubstring("creditedAs")) + }) + + It("deserializes legacy JSON without the creditedAs key", func() { + legacy := `{"id":"abc","name":"Foo","subRole":""}` + var p Participant + Expect(json.Unmarshal([]byte(legacy), &p)).To(Succeed()) + Expect(p.CreditedAs).To(Equal("")) + }) +}) + var _ = Describe("ParticipantList", func() { Describe("Join", func() { It("joins the participants with the given separator", func() { From 437d60a0df7bbfea0e92dfb6a43e1d121f95531d Mon Sep 17 00:00:00 2001 From: Deluan Date: Sun, 24 May 2026 19:13:14 -0300 Subject: [PATCH 04/26] refactor(scanner): wire buildArtists through computeArtistPID --- model/metadata/map_participants.go | 15 ++++++++++++--- model/metadata/persistent_ids.go | 8 -------- model/metadata/persistent_ids_test.go | 15 +++++++++++++++ 3 files changed, 27 insertions(+), 11 deletions(-) diff --git a/model/metadata/map_participants.go b/model/metadata/map_participants.go index e8be6aaab..7dd8c8c3f 100644 --- a/model/metadata/map_participants.go +++ b/model/metadata/map_participants.go @@ -7,6 +7,7 @@ import ( "github.com/navidrome/navidrome/conf" "github.com/navidrome/navidrome/consts" "github.com/navidrome/navidrome/model" + "github.com/navidrome/navidrome/model/id" "github.com/navidrome/navidrome/utils/str" "golang.org/x/text/cases" "golang.org/x/text/language" @@ -100,11 +101,15 @@ func (md Metadata) processPerformers(participants model.Participants, rolesMbzId subRole := titleCaser.String(performer.Key()) artist := model.Artist{ - ID: md.artistID(name), Name: name, OrderArtistName: str.SanitizeFieldForSortingNoArticle(name), MbzArtistID: md.getPerformerMbid(subRole, rolesMbzIdMap, roleIdx), } + artist.ID = computeArtistPID( + model.Participant{Artist: artist}, + conf.Server.PID.Artist, + id.NewHash, + ) participants.AddWithSubRole(model.RolePerformer, subRole, artist) } } @@ -154,9 +159,7 @@ func (md Metadata) parseArtists( func (md Metadata) buildArtists(names, sorts, mbids []string) []model.Artist { var artists []model.Artist for i, name := range names { - id := md.artistID(name) artist := model.Artist{ - ID: id, Name: name, OrderArtistName: str.SanitizeFieldForSortingNoArticle(name), } @@ -166,6 +169,12 @@ func (md Metadata) buildArtists(names, sorts, mbids []string) []model.Artist { if i < len(mbids) { artist.MbzArtistID = mbids[i] } + // Compute ID from the participant fields we just populated so MBID/sort-based specs work. + artist.ID = computeArtistPID( + model.Participant{Artist: artist}, + conf.Server.PID.Artist, + id.NewHash, + ) artists = append(artists, artist) } return artists diff --git a/model/metadata/persistent_ids.go b/model/metadata/persistent_ids.go index df397a603..c3a3682a5 100644 --- a/model/metadata/persistent_ids.go +++ b/model/metadata/persistent_ids.go @@ -139,14 +139,6 @@ func getArtistPIDAttr(p model.Participant, attr string, hash hashFunc) string { return "" } -// artistID is kept temporarily as a thin wrapper so any in-progress callers -// continue to compile; it will be removed once map_participants.go switches -// to computeArtistPID directly (Task 4). -func (md Metadata) artistID(name string) string { - return computeArtistPID(model.Participant{Artist: model.Artist{Name: name}}, - conf.Server.PID.Artist, id.NewHash) -} - func (md Metadata) mapTrackTitle() string { if title := md.String(model.TagTitle); title != "" { return title diff --git a/model/metadata/persistent_ids_test.go b/model/metadata/persistent_ids_test.go index 1163189d3..47cb66115 100644 --- a/model/metadata/persistent_ids_test.go +++ b/model/metadata/persistent_ids_test.go @@ -381,4 +381,19 @@ var _ = Describe("computeArtistPID", func() { NotTo(Panic()) }) }) + + It("buildArtists produces the same Artist.ID as the legacy artistID() under default PID.Artist", func() { + DeferCleanup(configtest.SetupConfig()) + conf.Server.PID.Artist = consts.DefaultArtistPID + + name := "Some Artist" + md := Metadata{} + artists := md.buildArtists([]string{name}, nil, nil) + + // Legacy result computed independently via the old "albumartistid" path: + legacyMF := model.MediaFile{AlbumArtist: name} + expected := computePID(legacyMF, Metadata{}, "albumartistid", false, id.NewHash) + Expect(artists).To(HaveLen(1)) + Expect(artists[0].ID).To(Equal(expected)) + }) }) From c2712e91fc37f390776dbc6d02d65aee172ea828 Mon Sep 17 00:00:00 2001 From: Deluan Date: Sun, 24 May 2026 19:16:02 -0300 Subject: [PATCH 05/26] feat(model): add TagXxxCredit constants for each role --- model/tag.go | 59 +++++++++++++++++++++++++++++++++------------------- 1 file changed, 38 insertions(+), 21 deletions(-) diff --git a/model/tag.go b/model/tag.go index 1f6b24d21..e1344cb53 100644 --- a/model/tag.go +++ b/model/tag.go @@ -201,27 +201,44 @@ const ( // Artists and roles - TagAlbumArtist TagName = "albumartist" - TagAlbumArtists TagName = "albumartists" - TagAlbumArtistSort TagName = "albumartistsort" - TagAlbumArtistsSort TagName = "albumartistssort" - TagTrackArtist TagName = "artist" - TagTrackArtists TagName = "artists" - TagTrackArtistSort TagName = "artistsort" - TagTrackArtistsSort TagName = "artistssort" - TagComposer TagName = "composer" - TagComposerSort TagName = "composersort" - TagLyricist TagName = "lyricist" - TagLyricistSort TagName = "lyricistsort" - TagDirector TagName = "director" - TagProducer TagName = "producer" - TagEngineer TagName = "engineer" - TagMixer TagName = "mixer" - TagRemixer TagName = "remixer" - TagDJMixer TagName = "djmixer" - TagConductor TagName = "conductor" - TagArranger TagName = "arranger" - TagPerformer TagName = "performer" + TagAlbumArtist TagName = "albumartist" + TagAlbumArtists TagName = "albumartists" + TagAlbumArtistSort TagName = "albumartistsort" + TagAlbumArtistsSort TagName = "albumartistssort" + TagAlbumArtistCredit TagName = "albumartistcredit" + TagAlbumArtistsCredit TagName = "albumartistscredit" + TagTrackArtist TagName = "artist" + TagTrackArtists TagName = "artists" + TagTrackArtistSort TagName = "artistsort" + TagTrackArtistsSort TagName = "artistssort" + TagTrackArtistCredit TagName = "artistcredit" + TagTrackArtistsCredit TagName = "artistscredit" + TagComposer TagName = "composer" + TagComposerSort TagName = "composersort" + TagComposerCredit TagName = "composercredit" + TagComposersCredit TagName = "composerscredit" + TagLyricist TagName = "lyricist" + TagLyricistSort TagName = "lyricistsort" + TagLyricistCredit TagName = "lyricistcredit" + TagLyricistsCredit TagName = "lyricistscredit" + TagDirector TagName = "director" + TagDirectorCredit TagName = "directorcredit" + TagProducer TagName = "producer" + TagProducerCredit TagName = "producercredit" + TagEngineer TagName = "engineer" + TagEngineerCredit TagName = "engineercredit" + TagMixer TagName = "mixer" + TagMixerCredit TagName = "mixercredit" + TagRemixer TagName = "remixer" + TagRemixerCredit TagName = "remixercredit" + TagDJMixer TagName = "djmixer" + TagDJMixerCredit TagName = "djmixercredit" + TagConductor TagName = "conductor" + TagConductorCredit TagName = "conductorcredit" + TagArranger TagName = "arranger" + TagArrangerCredit TagName = "arrangercredit" + TagPerformer TagName = "performer" + TagPerformerCredit TagName = "performercredit" // ReplayGain From d374150a5abe718958853f027a8fbbe1faab1435 Mon Sep 17 00:00:00 2001 From: Deluan Date: Sun, 24 May 2026 19:18:49 -0300 Subject: [PATCH 06/26] feat(metadata): register *_credit tag aliases in mappings.yaml --- resources/mappings.yaml | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/resources/mappings.yaml b/resources/mappings.yaml index 16dddd504..3f15dcd3d 100644 --- a/resources/mappings.yaml +++ b/resources/mappings.yaml @@ -38,8 +38,14 @@ main: aliases: [ txxx:artists, artists, ----:com.apple.itunes:artists, wm/artists ] artistssort: aliases: [ artistssort ] + artistcredit: + aliases: [ artist_credit, artistcredit ] + artistscredit: + aliases: [ artists_credit, artistscredit ] arranger: aliases: [ tipl:arranger, ipls:arranger, arranger ] + arrangercredit: + aliases: [ arranger_credit, arrangercredit ] composer: aliases: [ tcom, composer, ©wrt, wm/composer, imus, writer, txxx:writer, iwri, @@ -50,24 +56,46 @@ main: # aliases: [ WRITER, TXXX:Writer, IWRI ] composersort: aliases: [ tsoc, txxx:composersort, composersort, soco, wm/composersortorder ] + composercredit: + aliases: [ composer_credit, composercredit ] + composerscredit: + aliases: [ composers_credit, composerscredit ] lyricist: aliases: [ text, lyricist, ----:com.apple.itunes:lyricist, wm/writer ] lyricistsort: aliases: [ lyricistsort ] + lyricistcredit: + aliases: [ lyricist_credit, lyricistcredit ] + lyricistscredit: + aliases: [ lyricists_credit, lyricistscredit ] conductor: aliases: [ tpe3, conductor, ----:com.apple.itunes:conductor, wm/conductor ] + conductorcredit: + aliases: [ conductor_credit, conductorcredit ] director: aliases: [ txxx:director, director, ©dir, wm/director ] + directorcredit: + aliases: [ director_credit, directorcredit ] djmixer: aliases: [ tipl:dj-mix, ipls:dj-mix, djmixer, ----:com.apple.itunes:djmixer, wm/djmixer ] + djmixercredit: + aliases: [ djmixer_credit, djmixercredit ] mixer: aliases: [ tipl:mix, ipls:mix, mixer, ----:com.apple.itunes:mixer, wm/mixer ] + mixercredit: + aliases: [ mixer_credit, mixercredit ] engineer: aliases: [ tipl:engineer, ipls:engineer, engineer, ----:com.apple.itunes:engineer, wm/engineer, ieng ] + engineercredit: + aliases: [ engineer_credit, engineercredit ] producer: aliases: [ tipl:producer, ipls:producer, producer, ----:com.apple.itunes:producer, wm/producer, ipro ] + producercredit: + aliases: [ producer_credit, producercredit ] remixer: aliases: [ tpe4, remixer, mixartist, ----:com.apple.itunes:remixer, wm/modifiedby ] + remixercredit: + aliases: [ remixer_credit, remixercredit ] albumartist: aliases: [ tpe2, albumartist, album artist, album_artist, aart, wm/albumartist ] albumartistsort: @@ -76,6 +104,10 @@ main: aliases: [ txxx:album artists, albumartists ] albumartistssort: aliases: [ albumartistssort ] + albumartistcredit: + aliases: [ albumartist_credit, albumartistcredit ] + albumartistscredit: + aliases: [ albumartists_credit, albumartistscredit ] album: aliases: [ talb, album, ©alb, wm/albumtitle, iprd ] albumsort: @@ -193,6 +225,8 @@ main: performer: aliases: [performer] type: pair + performercredit: + aliases: [ performer_credit, performercredit ] musicbrainz_performerid: aliases: [ txxx:musicbrainz performer id, musicbrainz_performerid, musicbrainz_performer_id, ----:com.apple.itunes:musicbrainz performer id, musicbrainz/performer id ] type: pair From 12980e8d88ae5454191a4dd538f171fa55188d5f Mon Sep 17 00:00:00 2001 From: Deluan Date: Sun, 24 May 2026 19:26:28 -0300 Subject: [PATCH 07/26] feat(scanner): populate CreditedAs from paired *_credit tags --- model/metadata/map_participants.go | 103 ++++++++++++++---------- model/metadata/map_participants_test.go | 61 ++++++++++++++ model/participants.go | 6 ++ 3 files changed, 129 insertions(+), 41 deletions(-) diff --git a/model/metadata/map_participants.go b/model/metadata/map_participants.go index 7dd8c8c3f..b5c6e0c38 100644 --- a/model/metadata/map_participants.go +++ b/model/metadata/map_participants.go @@ -14,49 +14,58 @@ import ( ) type roleTags struct { - name model.TagName - sort model.TagName - mbid model.TagName + name model.TagName + sort model.TagName + mbid model.TagName + credit model.TagName } var roleMappings = map[model.Role]roleTags{ - model.RoleComposer: {name: model.TagComposer, sort: model.TagComposerSort, mbid: model.TagMusicBrainzComposerID}, - model.RoleLyricist: {name: model.TagLyricist, sort: model.TagLyricistSort, mbid: model.TagMusicBrainzLyricistID}, - model.RoleConductor: {name: model.TagConductor, mbid: model.TagMusicBrainzConductorID}, - model.RoleArranger: {name: model.TagArranger, mbid: model.TagMusicBrainzArrangerID}, - model.RoleDirector: {name: model.TagDirector, mbid: model.TagMusicBrainzDirectorID}, - model.RoleProducer: {name: model.TagProducer, mbid: model.TagMusicBrainzProducerID}, - model.RoleEngineer: {name: model.TagEngineer, mbid: model.TagMusicBrainzEngineerID}, - model.RoleMixer: {name: model.TagMixer, mbid: model.TagMusicBrainzMixerID}, - model.RoleRemixer: {name: model.TagRemixer, mbid: model.TagMusicBrainzRemixerID}, - model.RoleDJMixer: {name: model.TagDJMixer, mbid: model.TagMusicBrainzDJMixerID}, + model.RoleComposer: {name: model.TagComposer, sort: model.TagComposerSort, mbid: model.TagMusicBrainzComposerID, credit: model.TagComposerCredit}, + model.RoleLyricist: {name: model.TagLyricist, sort: model.TagLyricistSort, mbid: model.TagMusicBrainzLyricistID, credit: model.TagLyricistCredit}, + model.RoleConductor: {name: model.TagConductor, mbid: model.TagMusicBrainzConductorID, credit: model.TagConductorCredit}, + model.RoleArranger: {name: model.TagArranger, mbid: model.TagMusicBrainzArrangerID, credit: model.TagArrangerCredit}, + model.RoleDirector: {name: model.TagDirector, mbid: model.TagMusicBrainzDirectorID, credit: model.TagDirectorCredit}, + model.RoleProducer: {name: model.TagProducer, mbid: model.TagMusicBrainzProducerID, credit: model.TagProducerCredit}, + model.RoleEngineer: {name: model.TagEngineer, mbid: model.TagMusicBrainzEngineerID, credit: model.TagEngineerCredit}, + model.RoleMixer: {name: model.TagMixer, mbid: model.TagMusicBrainzMixerID, credit: model.TagMixerCredit}, + model.RoleRemixer: {name: model.TagRemixer, mbid: model.TagMusicBrainzRemixerID, credit: model.TagRemixerCredit}, + model.RoleDJMixer: {name: model.TagDJMixer, mbid: model.TagMusicBrainzDJMixerID, credit: model.TagDJMixerCredit}, } func (md Metadata) mapParticipants() model.Participants { participants := make(model.Participants) // Parse track artists - artists := md.parseArtists( - model.TagTrackArtist, model.TagTrackArtists, - model.TagTrackArtistSort, model.TagTrackArtistsSort, - model.TagMusicBrainzArtistID, - ) - participants.Add(model.RoleArtist, artists...) + trackNames := md.getArtistValues(model.TagTrackArtist, model.TagTrackArtists) + if len(trackNames) == 0 { + trackNames = []string{consts.UnknownArtist} + } + trackSorts := md.getArtistValues(model.TagTrackArtistSort, model.TagTrackArtistsSort) + trackMbids := md.Strings(model.TagMusicBrainzArtistID) + trackCredits := md.getArtistValues(model.TagTrackArtistCredit, model.TagTrackArtistsCredit) + trackArtistParticipants := md.buildParticipants(trackNames, trackSorts, trackMbids, trackCredits) + participants.AddParticipants(model.RoleArtist, trackArtistParticipants...) // Parse album artists - albumArtists := md.parseArtists( - model.TagAlbumArtist, model.TagAlbumArtists, - model.TagAlbumArtistSort, model.TagAlbumArtistsSort, - model.TagMusicBrainzAlbumArtistID, - ) - if len(albumArtists) == 1 && albumArtists[0].Name == consts.UnknownArtist { + albumNames := md.getArtistValues(model.TagAlbumArtist, model.TagAlbumArtists) + albumSorts := md.getArtistValues(model.TagAlbumArtistSort, model.TagAlbumArtistsSort) + albumMbids := md.Strings(model.TagMusicBrainzAlbumArtistID) + albumCredits := md.getArtistValues(model.TagAlbumArtistCredit, model.TagAlbumArtistsCredit) + + var albumArtistParticipants []model.Participant + if len(albumNames) == 0 { if md.Bool(model.TagCompilation) { - albumArtists = md.buildArtists([]string{consts.VariousArtists}, nil, []string{consts.VariousArtistsMbzId}) + albumArtistParticipants = md.buildParticipants( + []string{consts.VariousArtists}, nil, + []string{consts.VariousArtistsMbzId}, nil) } else { - albumArtists = artists + albumArtistParticipants = trackArtistParticipants } + } else { + albumArtistParticipants = md.buildParticipants(albumNames, albumSorts, albumMbids, albumCredits) } - participants.Add(model.RoleAlbumArtist, albumArtists...) + participants.AddParticipants(model.RoleAlbumArtist, albumArtistParticipants...) // Parse all other roles for role, info := range roleMappings { @@ -64,8 +73,8 @@ func (md Metadata) mapParticipants() model.Participants { if len(names) > 0 { sorts := md.Strings(info.sort) mbids := md.Strings(info.mbid) - artists := md.buildArtists(names, sorts, mbids) - participants.Add(role, artists...) + credits := md.Strings(info.credit) + participants.AddParticipants(role, md.buildParticipants(names, sorts, mbids, credits)...) } } @@ -110,7 +119,11 @@ func (md Metadata) processPerformers(participants model.Participants, rolesMbzId conf.Server.PID.Artist, id.NewHash, ) - participants.AddWithSubRole(model.RolePerformer, subRole, artist) + participants.AddParticipants(model.RolePerformer, model.Participant{ + Artist: artist, + SubRole: subRole, + CreditedAs: name, + }) } } @@ -143,17 +156,25 @@ func (md Metadata) syncMissingMbzIDs(participants model.Participants) { } } -func (md Metadata) parseArtists( - name model.TagName, names model.TagName, sort model.TagName, - sorts model.TagName, mbid model.TagName, -) []model.Artist { - nameValues := md.getArtistValues(name, names) - sortValues := md.getArtistValues(sort, sorts) - mbids := md.Strings(mbid) - if len(nameValues) == 0 { - nameValues = []string{consts.UnknownArtist} +// buildParticipants builds Artists and wraps each into a Participant with +// CreditedAs populated. credits is paired positionally; if the lengths don't +// match, CreditedAs falls back to the canonical name for every entry. +func (md Metadata) buildParticipants(names, sorts, mbids, credits []string) []model.Participant { + if len(credits) != 0 && len(credits) != len(names) { + credits = nil } - return md.buildArtists(nameValues, sortValues, mbids) + artists := md.buildArtists(names, sorts, mbids) + out := make([]model.Participant, len(artists)) + for i, a := range artists { + p := model.Participant{Artist: a} + if i < len(credits) && credits[i] != "" { + p.CreditedAs = credits[i] + } else { + p.CreditedAs = a.Name + } + out[i] = p + } + return out } func (md Metadata) buildArtists(names, sorts, mbids []string) []model.Artist { diff --git a/model/metadata/map_participants_test.go b/model/metadata/map_participants_test.go index 5ee802ced..7ef6183ee 100644 --- a/model/metadata/map_participants_test.go +++ b/model/metadata/map_participants_test.go @@ -802,4 +802,65 @@ var _ = Describe("Participants", func() { } }) }) + + Describe("CreditedAs population", func() { + It("uses the canonical name as CreditedAs when no credit tag is present", func() { + mf = toMediaFile(model.RawTags{ + "ARTISTS": {"Some Artist"}, + }) + artists := mf.Participants[model.RoleArtist] + Expect(artists).To(HaveLen(1)) + Expect(artists[0].Name).To(Equal("Some Artist")) + Expect(artists[0].CreditedAs).To(Equal("Some Artist")) + }) + + It("uses the credit tag value when present, paired positionally", func() { + mf = toMediaFile(model.RawTags{ + "ARTISTS": {"Planetary Assault Systems", "Other"}, + "ARTISTSCREDIT": {"PAS", "Other"}, + }) + artists := mf.Participants[model.RoleArtist] + Expect(artists).To(HaveLen(2)) + Expect(artists[0].Name).To(Equal("Planetary Assault Systems")) + Expect(artists[0].CreditedAs).To(Equal("PAS")) + Expect(artists[1].Name).To(Equal("Other")) + Expect(artists[1].CreditedAs).To(Equal("Other")) + }) + + It("falls back to canonical name when credit list length differs from name list", func() { + mf = toMediaFile(model.RawTags{ + "ARTISTS": {"A", "B", "C"}, + "ARTISTSCREDIT": {"only one"}, // mismatch + }) + artists := mf.Participants[model.RoleArtist] + Expect(artists).To(HaveLen(3)) + for _, a := range artists { + Expect(a.CreditedAs).To(Equal(a.Name)) + } + }) + + It("populates CreditedAs for non-artist roles (composer)", func() { + mf = toMediaFile(model.RawTags{ + "COMPOSER": {"Real Composer"}, + "COMPOSERCREDIT": {"R. Composer"}, + }) + composers := mf.Participants[model.RoleComposer] + Expect(composers).To(HaveLen(1)) + Expect(composers[0].CreditedAs).To(Equal("R. Composer")) + }) + + It("always populates CreditedAs (never empty)", func() { + mf = toMediaFile(model.RawTags{ + "ARTIST": {"Track Artist"}, + "ALBUMARTIST": {"Album Artist"}, + "COMPOSER": {"A Composer"}, + "PERFORMER:GUITAR": {"A Guitarist"}, + }) + for role, list := range mf.Participants { + for _, p := range list { + Expect(p.CreditedAs).NotTo(BeEmpty(), "role: %s, participant: %+v", role, p) + } + } + }) + }) }) diff --git a/model/participants.go b/model/participants.go index ec1b8e863..5c476fe96 100644 --- a/model/participants.go +++ b/model/participants.go @@ -110,6 +110,12 @@ func (p Participants) AddWithSubRole(role Role, subRole string, artists ...Artis p.add(role, participants...) } +// AddParticipants adds Participants directly (preserving CreditedAs and SubRole), +// ignoring duplicates. +func (p Participants) AddParticipants(role Role, participants ...Participant) { + p.add(role, participants...) +} + func (p Participants) Sort() { for _, artists := range p { slices.SortFunc(artists, func(a1, a2 Participant) int { From 46cb9f8d58e6e9dabfb7ad3edcdc95b5dc9d27f8 Mon Sep 17 00:00:00 2001 From: Deluan Date: Sun, 24 May 2026 19:35:03 -0300 Subject: [PATCH 08/26] feat(scanner): trigger rescan when PID.Artist changes --- cmd/root.go | 12 +++++++++++- scanner/scanner.go | 5 +++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/cmd/root.go b/cmd/root.go index 08773176a..6363825cc 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -175,7 +175,17 @@ func pidHashChanged(ds model.DataStore) (bool, error) { if err != nil { return false, err } - return !strings.EqualFold(pidAlbum, conf.Server.PID.Album) || !strings.EqualFold(pidTrack, conf.Server.PID.Track), nil + pidArtist, err := ds.Property(context.Background()).DefaultGet(consts.PIDArtistKey, "") + if err != nil { + return false, err + } + // Empty stored value is treated as matching — fresh upgrade should not force a rescan. + if pidArtist == "" { + pidArtist = conf.Server.PID.Artist + } + return !strings.EqualFold(pidAlbum, conf.Server.PID.Album) || + !strings.EqualFold(pidTrack, conf.Server.PID.Track) || + !strings.EqualFold(pidArtist, conf.Server.PID.Artist), nil } // runInitialScan runs an initial scan of the music library if needed. diff --git a/scanner/scanner.go b/scanner/scanner.go index 871b0c696..054e404be 100644 --- a/scanner/scanner.go +++ b/scanner/scanner.go @@ -309,6 +309,11 @@ func (s *scannerImpl) runUpdateLibraries(ctx context.Context, state *scanState) log.Error(ctx, "Scanner: Error updating album PID conf", err) return fmt.Errorf("updating album PID conf: %w", err) } + err = tx.Property(ctx).Put(consts.PIDArtistKey, conf.Server.PID.Artist) + if err != nil { + log.Error(ctx, "Scanner: Error updating artist PID conf", err) + return fmt.Errorf("updating artist PID conf: %w", err) + } if state.changesDetected.Load() { log.Debug(ctx, "Scanner: Refreshing library stats", "lib", lib.Name) if err := tx.Library(ctx).RefreshStats(lib.ID); err != nil { From 8b5e50a1aa3e3af75f3f7ec865a689934e98a581 Mon Sep 17 00:00:00 2001 From: Deluan Date: Sun, 24 May 2026 19:42:04 -0300 Subject: [PATCH 09/26] feat(scanner): mirror album re-PID flow for artists When PID.Artist spec changes, existing artist IDs differ from newly-computed ones. Without remapping, all annotations (starred, ratings, play counts) become orphaned. This change builds an artistIDMap during phase 1 and reassigns annotations on persist, mirroring the existing albumIDMap flow. The migration logic is gated on the previous spec being non-empty and different from the current spec, so the default "name" spec (byte-identical to the historical hardcoded artistID) triggers no remapping on upgrade. --- model/metadata/persistent_ids.go | 6 ++++ scanner/folder_entry.go | 18 ++++++----- scanner/phase_1_folders.go | 54 +++++++++++++++++++++++++++----- scanner/phase_1_folders_test.go | 49 +++++++++++++++++++++++++++++ tests/mock_artist_repo.go | 20 ++++++++++-- 5 files changed, 128 insertions(+), 19 deletions(-) create mode 100644 scanner/phase_1_folders_test.go diff --git a/model/metadata/persistent_ids.go b/model/metadata/persistent_ids.go index c3a3682a5..168e1081c 100644 --- a/model/metadata/persistent_ids.go +++ b/model/metadata/persistent_ids.go @@ -124,6 +124,12 @@ func computeArtistPID(p model.Participant, spec string, hash hashFunc) string { return hash(pid) } +// ComputeArtistPID is the exported entry point for callers outside this package +// (e.g. the scanner) that need to compute an artist PID under a specific spec. +func ComputeArtistPID(p model.Participant, spec string) string { + return computeArtistPID(p, spec, id.NewHash) +} + func getArtistPIDAttr(p model.Participant, attr string, hash hashFunc) string { switch strings.TrimSpace(strings.ToLower(attr)) { case "name": diff --git a/scanner/folder_entry.go b/scanner/folder_entry.go index c7cc88ee1..6a26a44bb 100644 --- a/scanner/folder_entry.go +++ b/scanner/folder_entry.go @@ -17,14 +17,15 @@ import ( func newFolderEntry(job *scanJob, id, path string, updTime time.Time, hash string) *folderEntry { f := &folderEntry{ - id: id, - job: job, - path: path, - audioFiles: make(map[string]fs.DirEntry), - imageFiles: make(map[string]fs.DirEntry), - albumIDMap: make(map[string]string), - updTime: updTime, - prevHash: hash, + id: id, + job: job, + path: path, + audioFiles: make(map[string]fs.DirEntry), + imageFiles: make(map[string]fs.DirEntry), + albumIDMap: make(map[string]string), + artistIDMap: make(map[string]string), + updTime: updTime, + prevHash: hash, } return f } @@ -46,6 +47,7 @@ type folderEntry struct { albums model.Albums albumIDMap map[string]string artists model.Artists + artistIDMap map[string]string tags model.TagList missingTracks []*model.MediaFile } diff --git a/scanner/phase_1_folders.go b/scanner/phase_1_folders.go index 38967832c..8aee6af25 100644 --- a/scanner/phase_1_folders.go +++ b/scanner/phase_1_folders.go @@ -123,11 +123,12 @@ func (j *scanJob) createFolderEntry(path string) *folderEntry { // The phaseFolders struct implements the phase interface, providing methods to produce // folder entries, process folders, persist changes to the database, and log the results. type phaseFolders struct { - jobs []*scanJob - ds model.DataStore - ctx context.Context - state *scanState - prevAlbumPIDConf string + jobs []*scanJob + ds model.DataStore + ctx context.Context + state *scanState + prevAlbumPIDConf string + prevArtistPIDConf string } func (p *phaseFolders) description() string { @@ -141,6 +142,10 @@ func (p *phaseFolders) producer() ppl.Producer[*folderEntry] { if err != nil { return fmt.Errorf("getting album PID conf: %w", err) } + p.prevArtistPIDConf, err = p.ds.Property(p.ctx).DefaultGet(consts.PIDArtistKey, "") + if err != nil { + return fmt.Errorf("getting artist PID conf: %w", err) + } // TODO Parallelize multiple job when we have multiple libraries var total int64 @@ -298,6 +303,21 @@ func (p *phaseFolders) loadTagsFromFiles(entry *folderEntry, toImport map[string if prevAlbumID != track.AlbumID && !ok { entry.albumIDMap[track.AlbumID] = prevAlbumID } + + // Build artistIDMap: for each participant on this track, if the previous + // spec produces a different ID than the current one, record the mapping. + if p.prevArtistPIDConf != "" && p.prevArtistPIDConf != conf.Server.PID.Artist { + for _, list := range track.Participants { + for _, part := range list { + prevID := metadata.ComputeArtistPID(part, p.prevArtistPIDConf) + if prevID != part.ID { + if _, already := entry.artistIDMap[part.ID]; !already { + entry.artistIDMap[part.ID] = prevID + } + } + } + } + } } } entry.tracks = tracks @@ -359,9 +379,7 @@ func (p *phaseFolders) persistChanges(entry *folderEntry) (*folderEntry, error) // Save all new/modified artists to DB. Their information will be incomplete, but they will be refreshed later for i := range entry.artists { - err = artistRepo.Put(&entry.artists[i], "name", - "mbz_artist_id", "sort_artist_name", "order_artist_name", "full_text", "updated_at") - if err != nil { + if err = p.persistArtist(artistRepo, &entry.artists[i], entry.artistIDMap); err != nil { log.Error(p.ctx, "Scanner: Error persisting artist to DB", "folder", entry.path, "artist", entry.artists[i].Name, err) return err } @@ -462,6 +480,26 @@ func (p *phaseFolders) persistAlbum(repo model.AlbumRepository, a *model.Album, return nil } +// persistArtist persists the given artist and reassigns annotations from the +// previous artist ID (if recorded in idMap). Mirrors persistAlbum. +func (p *phaseFolders) persistArtist(repo model.ArtistRepository, a *model.Artist, idMap map[string]string) error { + if err := repo.Put(a, "name", "mbz_artist_id", "sort_artist_name", + "order_artist_name", "full_text", "updated_at"); err != nil { + return fmt.Errorf("persisting artist %s: %w", a.ID, err) + } + prevID := idMap[a.ID] + if prevID == "" { + return nil + } + log.Trace(p.ctx, "Reassigning artist annotations", "from", prevID, "to", a.ID, "artist", a.Name) + if err := repo.ReassignAnnotation(prevID, a.ID); err != nil { + log.Warn(p.ctx, "Scanner: Could not reassign artist annotations", "from", prevID, "to", a.ID, "artist", a.Name, err) + p.state.sendWarning(fmt.Sprintf("Could not reassign artist annotations from %s to %s ('%s'): %v", prevID, a.ID, a.Name, err)) + } + delete(idMap, a.ID) + return nil +} + func (p *phaseFolders) logFolder(entry *folderEntry) (*folderEntry, error) { logCall := log.Info if entry.isEmpty() { diff --git a/scanner/phase_1_folders_test.go b/scanner/phase_1_folders_test.go new file mode 100644 index 000000000..f76b63bcb --- /dev/null +++ b/scanner/phase_1_folders_test.go @@ -0,0 +1,49 @@ +package scanner + +import ( + "context" + + "github.com/navidrome/navidrome/conf/configtest" + "github.com/navidrome/navidrome/model" + "github.com/navidrome/navidrome/tests" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +var _ = Describe("phaseFolders", func() { + Describe("Artist re-PID migration", func() { + var ( + artRepo *tests.MockArtistRepo + p *phaseFolders + ) + + BeforeEach(func() { + DeferCleanup(configtest.SetupConfig()) + artRepo = tests.CreateMockArtistRepo() + if artRepo.ReassignAnnotationCalls == nil { + artRepo.ReassignAnnotationCalls = map[string]string{} + } + p = &phaseFolders{ctx: context.Background(), state: &scanState{}} + }) + + It("calls ReassignAnnotation when persisting an artist whose ID changed", func() { + oldID := "old-id" + newID := "new-id" + idMap := map[string]string{newID: oldID} + artist := model.Artist{ID: newID, Name: "Foo"} + + Expect(p.persistArtist(artRepo, &artist, idMap)).To(Succeed()) + Expect(artRepo.ReassignAnnotationCalls).To(HaveKeyWithValue(oldID, newID)) + // Mapping should be removed after successful reassignment + Expect(idMap).ToNot(HaveKey(newID)) + }) + + It("does not call ReassignAnnotation when no mapping exists", func() { + idMap := map[string]string{} + artist := model.Artist{ID: "some-id", Name: "Foo"} + + Expect(p.persistArtist(artRepo, &artist, idMap)).To(Succeed()) + Expect(artRepo.ReassignAnnotationCalls).To(BeEmpty()) + }) + }) +}) diff --git a/tests/mock_artist_repo.go b/tests/mock_artist_repo.go index b7a6fb811..950fea7de 100644 --- a/tests/mock_artist_repo.go +++ b/tests/mock_artist_repo.go @@ -16,9 +16,10 @@ func CreateMockArtistRepo() *MockArtistRepo { type MockArtistRepo struct { model.ArtistRepository - Data map[string]*model.Artist - Err bool - Options model.QueryOptions + Data map[string]*model.Artist + Err bool + Options model.QueryOptions + ReassignAnnotationCalls map[string]string // prevID -> newID } func (m *MockArtistRepo) SetError(err bool) { @@ -157,4 +158,17 @@ func (m *MockArtistRepo) Search(q string, options ...model.QueryOptions) (model. return allArtists, err } +// ReassignAnnotation reassigns annotations from one artist to another +func (m *MockArtistRepo) ReassignAnnotation(prevID string, newID string) error { + if m.Err { + return errors.New("unexpected error") + } + // Mock implementation - track the reassignment calls + if m.ReassignAnnotationCalls == nil { + m.ReassignAnnotationCalls = make(map[string]string) + } + m.ReassignAnnotationCalls[prevID] = newID + return nil +} + var _ model.ArtistRepository = (*MockArtistRepo)(nil) From e2fd0959bdcddedd72aeb3b517fd485e6239b2ce Mon Sep 17 00:00:00 2001 From: Deluan Date: Sun, 24 May 2026 19:47:59 -0300 Subject: [PATCH 10/26] feat(subsonic): serve CreditedAs as name in contributor lists --- model/participants.go | 15 +++++++++++++ server/subsonic/helpers.go | 15 ++++++++++--- server/subsonic/helpers_test.go | 39 +++++++++++++++++++++++++++++++++ 3 files changed, 66 insertions(+), 3 deletions(-) diff --git a/model/participants.go b/model/participants.go index 5c476fe96..772bea97a 100644 --- a/model/participants.go +++ b/model/participants.go @@ -92,6 +92,21 @@ func (p ParticipantList) Join(sep string) string { }), sep) } +// JoinCredited joins the credited names of the participants with sep. +// Falls back to Name when CreditedAs is empty. +func (p ParticipantList) JoinCredited(sep string) string { + return strings.Join(slice.Map(p, func(part Participant) string { + n := part.CreditedAs + if n == "" { + n = part.Name + } + if part.SubRole != "" { + return n + " (" + part.SubRole + ")" + } + return n + }), sep) +} + type Participants map[Role]ParticipantList // Add adds the artists to the role, ignoring duplicates. diff --git a/server/subsonic/helpers.go b/server/subsonic/helpers.go index e4c39e373..da5c134bb 100644 --- a/server/subsonic/helpers.go +++ b/server/subsonic/helpers.go @@ -271,7 +271,7 @@ func osChildFromMediaFile(ctx context.Context, mf model.MediaFile) *responses.Op child.DisplayAlbumArtist = mf.AlbumArtist child.AlbumArtists = artistRefs(mf.Participants[model.RoleAlbumArtist]) var contributors []responses.Contributor - child.DisplayComposer = mf.Participants[model.RoleComposer].Join(consts.ArtistJoiner) + child.DisplayComposer = mf.Participants[model.RoleComposer].JoinCredited(consts.ArtistJoiner) for role, participants := range mf.Participants { if role == model.RoleArtist || role == model.RoleAlbumArtist { continue @@ -282,7 +282,7 @@ func osChildFromMediaFile(ctx context.Context, mf model.MediaFile) *responses.Op SubRole: participant.SubRole, Artist: responses.ArtistID3Ref{ Id: participant.ID, - Name: participant.Name, + Name: participantDisplayName(participant), }, }) } @@ -296,11 +296,20 @@ func artistRefs(participants model.ParticipantList) []responses.ArtistID3Ref { return slice.Map(participants, func(p model.Participant) responses.ArtistID3Ref { return responses.ArtistID3Ref{ Id: p.ID, - Name: p.Name, + Name: participantDisplayName(p), } }) } +// participantDisplayName returns CreditedAs if set, otherwise the canonical Name. +// Legacy rows (pre-rescan) have empty CreditedAs and continue to show canonical. +func participantDisplayName(p model.Participant) string { + if p.CreditedAs != "" { + return p.CreditedAs + } + return p.Name +} + func fakePath(mf model.MediaFile) string { builder := strings.Builder{} diff --git a/server/subsonic/helpers_test.go b/server/subsonic/helpers_test.go index 2ae6eb28e..684cac838 100644 --- a/server/subsonic/helpers_test.go +++ b/server/subsonic/helpers_test.go @@ -317,6 +317,45 @@ var _ = Describe("helpers", func() { Expect(child.Title).To(Equal("")) }) }) + + It("returns CreditedAs as the contributor name when present, with canonical artist ID", func() { + mf := model.MediaFile{ + ID: "song-1", + Participants: model.Participants{ + model.RoleArtist: model.ParticipantList{ + {Artist: model.Artist{ID: "canon-1", Name: "Planetary Assault Systems"}, CreditedAs: "PAS"}, + }, + model.RoleComposer: model.ParticipantList{ + {Artist: model.Artist{ID: "canon-2", Name: "Real Composer"}, CreditedAs: "R. Composer"}, + }, + }, + } + child := childFromMediaFile(context.Background(), mf) + Expect(child.OpenSubsonicChild).NotTo(BeNil()) + Expect(child.OpenSubsonicChild.Artists).To(HaveLen(1)) + Expect(child.OpenSubsonicChild.Artists[0].Id).To(Equal("canon-1")) + Expect(child.OpenSubsonicChild.Artists[0].Name).To(Equal("PAS")) + + Expect(child.OpenSubsonicChild.Contributors).To(HaveLen(1)) + Expect(child.OpenSubsonicChild.Contributors[0].Artist.Id).To(Equal("canon-2")) + Expect(child.OpenSubsonicChild.Contributors[0].Artist.Name).To(Equal("R. Composer")) + + Expect(child.OpenSubsonicChild.DisplayComposer).To(Equal("R. Composer")) + }) + + It("falls back to canonical Name when CreditedAs is empty (legacy participant rows)", func() { + mf := model.MediaFile{ + ID: "song-2", + Participants: model.Participants{ + model.RoleArtist: model.ParticipantList{ + {Artist: model.Artist{ID: "canon-3", Name: "Some Artist"}}, // no CreditedAs + }, + }, + } + child := childFromMediaFile(context.Background(), mf) + Expect(child.OpenSubsonicChild).NotTo(BeNil()) + Expect(child.OpenSubsonicChild.Artists[0].Name).To(Equal("Some Artist")) + }) }) Describe("osChildFromMediaFile", func() { From a859804fd42b497029161018ae44c7cb60bebf65 Mon Sep 17 00:00:00 2001 From: Deluan Date: Sun, 24 May 2026 19:50:25 -0300 Subject: [PATCH 11/26] feat(ui): render creditedAs as link text with canonical tooltip --- ui/src/common/ArtistLinkField.jsx | 12 +++-- ui/src/common/ArtistLinkField.test.jsx | 75 ++++++++++++++++++++++++++ 2 files changed, 84 insertions(+), 3 deletions(-) diff --git a/ui/src/common/ArtistLinkField.jsx b/ui/src/common/ArtistLinkField.jsx index d41b47b06..e9d17801d 100644 --- a/ui/src/common/ArtistLinkField.jsx +++ b/ui/src/common/ArtistLinkField.jsx @@ -12,6 +12,10 @@ const ALink = withWidth()((props) => { const artistLink = useGetHandleArtistClick(width) const dispatch = useDispatch() + const displayName = artist.creditedAs || artist.name + const showCanonicalTooltip = + artist.creditedAs && artist.creditedAs !== artist.name + return ( { e.stopPropagation() dispatch(closeExtendedInfoDialog()) }} + title={showCanonicalTooltip ? artist.name : undefined} {...rest} > - {artist.name} + {displayName} {artist.subroles?.length > 0 ? ` (${artist.subroles.join(', ')})` : ''} ) @@ -37,7 +42,8 @@ const parseAndReplaceArtists = ( let lastIndex = 0 albumArtists?.forEach((artist) => { - const index = displayAlbumArtist.indexOf(artist.name, lastIndex) + const matchName = artist.creditedAs || artist.name + const index = displayAlbumArtist.indexOf(matchName, lastIndex) if (index !== -1) { // Add text before the artist name if (index > lastIndex) { @@ -47,7 +53,7 @@ const parseAndReplaceArtists = ( result.push( , ) - lastIndex = index + artist.name.length + lastIndex = index + matchName.length } }) diff --git a/ui/src/common/ArtistLinkField.test.jsx b/ui/src/common/ArtistLinkField.test.jsx index 09fdf64a4..e49873930 100644 --- a/ui/src/common/ArtistLinkField.test.jsx +++ b/ui/src/common/ArtistLinkField.test.jsx @@ -213,6 +213,81 @@ describe('ArtistLinkField', () => { }) }) + describe('creditedAs', () => { + it('renders creditedAs as the link text when present', () => { + const record = { + artist: 'PAS', + participants: { + artist: [ + { + id: 'canon-1', + name: 'Planetary Assault Systems', + creditedAs: 'PAS', + }, + ], + }, + } + + render() + + expect(screen.getByText('PAS')).toBeInTheDocument() + expect( + screen.queryByText('Planetary Assault Systems'), + ).not.toBeInTheDocument() + }) + + it('sets a title tooltip with the canonical name when creditedAs differs', () => { + const record = { + artist: 'PAS', + participants: { + artist: [ + { + id: 'canon-1', + name: 'Planetary Assault Systems', + creditedAs: 'PAS', + }, + ], + }, + } + + render() + + const link = screen.getByRole('link') + expect(link).toHaveAttribute('title', 'Planetary Assault Systems') + }) + + it('falls back to name when creditedAs is missing', () => { + const record = { + artist: 'Some Artist', + participants: { + artist: [{ id: 'canon-2', name: 'Some Artist' }], + }, + } + + render() + + expect(screen.getByText('Some Artist')).toBeInTheDocument() + const link = screen.getByRole('link') + expect(link).not.toHaveAttribute('title') + }) + + it('does not set a tooltip when creditedAs equals name', () => { + const record = { + artist: 'Same Name', + participants: { + artist: [ + { id: 'canon-3', name: 'Same Name', creditedAs: 'Same Name' }, + ], + }, + } + + render() + + const link = screen.getByRole('link') + expect(link).not.toHaveAttribute('title') + }) + }) + describe('when limiting displayed artists', () => { it('limits the number of artists displayed', () => { const record = { From 03b8b15f6e27d2de9da949a2bef68f5fa541ad87 Mon Sep 17 00:00:00 2001 From: Deluan Date: Sun, 24 May 2026 20:01:45 -0300 Subject: [PATCH 12/26] feat(participants): add DisplayName method to prioritize CreditedAs over Name Signed-off-by: Deluan --- model/participants.go | 4 ++++ server/subsonic/helpers.go | 13 ++----------- 2 files changed, 6 insertions(+), 11 deletions(-) diff --git a/model/participants.go b/model/participants.go index 772bea97a..4db198b61 100644 --- a/model/participants.go +++ b/model/participants.go @@ -81,6 +81,10 @@ type Participant struct { CreditedAs string `json:"creditedAs,omitempty"` } +func (p Participant) DisplayName() string { + return cmp.Or(p.CreditedAs, p.Name) +} + type ParticipantList []Participant func (p ParticipantList) Join(sep string) string { diff --git a/server/subsonic/helpers.go b/server/subsonic/helpers.go index da5c134bb..4bd2c427b 100644 --- a/server/subsonic/helpers.go +++ b/server/subsonic/helpers.go @@ -282,7 +282,7 @@ func osChildFromMediaFile(ctx context.Context, mf model.MediaFile) *responses.Op SubRole: participant.SubRole, Artist: responses.ArtistID3Ref{ Id: participant.ID, - Name: participantDisplayName(participant), + Name: participant.DisplayName(), }, }) } @@ -296,20 +296,11 @@ func artistRefs(participants model.ParticipantList) []responses.ArtistID3Ref { return slice.Map(participants, func(p model.Participant) responses.ArtistID3Ref { return responses.ArtistID3Ref{ Id: p.ID, - Name: participantDisplayName(p), + Name: p.DisplayName(), } }) } -// participantDisplayName returns CreditedAs if set, otherwise the canonical Name. -// Legacy rows (pre-rescan) have empty CreditedAs and continue to show canonical. -func participantDisplayName(p model.Participant) string { - if p.CreditedAs != "" { - return p.CreditedAs - } - return p.Name -} - func fakePath(mf model.MediaFile) string { builder := strings.Builder{} From 57785255bf29e97c88a9f0f8f6b6bb870a8b4979 Mon Sep 17 00:00:00 2001 From: Deluan Date: Sun, 24 May 2026 20:05:36 -0300 Subject: [PATCH 13/26] fix(persistence): preserve CreditedAs through participants JSON round-trip --- persistence/sql_participations.go | 20 +++++++--- persistence/sql_participations_test.go | 51 ++++++++++++++++++++++++++ 2 files changed, 66 insertions(+), 5 deletions(-) create mode 100644 persistence/sql_participations_test.go diff --git a/persistence/sql_participations.go b/persistence/sql_participations.go index 38b0203fa..96a9704a6 100644 --- a/persistence/sql_participations.go +++ b/persistence/sql_participations.go @@ -10,9 +10,10 @@ import ( ) type participant struct { - ID string `json:"id"` - Name string `json:"name"` - SubRole string `json:"subRole,omitempty"` + ID string `json:"id"` + Name string `json:"name"` + SubRole string `json:"subRole,omitempty"` + CreditedAs string `json:"creditedAs,omitempty"` } // flatParticipant represents a flattened participant structure for SQL processing @@ -26,7 +27,12 @@ func marshalParticipants(participants model.Participants) string { dbParticipants := make(map[model.Role][]participant) for role, artists := range participants { for _, artist := range artists { - dbParticipants[role] = append(dbParticipants[role], participant{ID: artist.ID, SubRole: artist.SubRole, Name: artist.Name}) + dbParticipants[role] = append(dbParticipants[role], participant{ + ID: artist.ID, + Name: artist.Name, + SubRole: artist.SubRole, + CreditedAs: artist.CreditedAs, + }) } } res, _ := json.Marshal(dbParticipants) @@ -43,7 +49,11 @@ func unmarshalParticipants(data string) (model.Participants, error) { participants := make(model.Participants, len(dbParticipants)) for role, participantList := range dbParticipants { artists := slice.Map(participantList, func(p participant) model.Participant { - return model.Participant{Artist: model.Artist{ID: p.ID, Name: p.Name}, SubRole: p.SubRole} + return model.Participant{ + Artist: model.Artist{ID: p.ID, Name: p.Name}, + SubRole: p.SubRole, + CreditedAs: p.CreditedAs, + } }) participants[role] = artists } diff --git a/persistence/sql_participations_test.go b/persistence/sql_participations_test.go new file mode 100644 index 000000000..685ab4ce6 --- /dev/null +++ b/persistence/sql_participations_test.go @@ -0,0 +1,51 @@ +package persistence + +import ( + "github.com/navidrome/navidrome/model" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +var _ = Describe("sql_participations", func() { + Describe("marshalParticipants/unmarshalParticipants", func() { + It("preserves CreditedAs through marshal/unmarshal round-trip", func() { + original := model.Participants{ + model.RoleArtist: model.ParticipantList{ + {Artist: model.Artist{ID: "canon-1", Name: "Planetary Assault Systems"}, CreditedAs: "PAS"}, + }, + model.RoleComposer: model.ParticipantList{ + {Artist: model.Artist{ID: "canon-2", Name: "John Lennon"}, CreditedAs: "J. Lennon"}, + }, + } + serialized := marshalParticipants(original) + restored, err := unmarshalParticipants(serialized) + Expect(err).ToNot(HaveOccurred()) + Expect(restored[model.RoleArtist]).To(HaveLen(1)) + Expect(restored[model.RoleArtist][0].CreditedAs).To(Equal("PAS")) + Expect(restored[model.RoleComposer][0].CreditedAs).To(Equal("J. Lennon")) + }) + + It("preserves SubRole through marshal/unmarshal round-trip", func() { + original := model.Participants{ + model.RolePerformer: model.ParticipantList{ + {Artist: model.Artist{ID: "p-1", Name: "Performer"}, SubRole: "Guitar"}, + }, + } + serialized := marshalParticipants(original) + restored, err := unmarshalParticipants(serialized) + Expect(err).ToNot(HaveOccurred()) + Expect(restored[model.RolePerformer]).To(HaveLen(1)) + Expect(restored[model.RolePerformer][0].SubRole).To(Equal("Guitar")) + }) + + It("does not include CreditedAs in serialized JSON when empty (omitempty)", func() { + p := model.Participants{ + model.RoleArtist: model.ParticipantList{ + {Artist: model.Artist{ID: "canon-1", Name: "Foo"}}, + }, + } + serialized := marshalParticipants(p) + Expect(serialized).NotTo(ContainSubstring("creditedAs")) + }) + }) +}) From 7d601029c9e3307f9c14c1a1475e96da125ec44c Mon Sep 17 00:00:00 2001 From: Deluan Date: Sun, 24 May 2026 20:09:04 -0300 Subject: [PATCH 14/26] refactor(participants): use DisplayName in JoinCredited --- model/participants.go | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/model/participants.go b/model/participants.go index 4db198b61..d8d4eab62 100644 --- a/model/participants.go +++ b/model/participants.go @@ -100,10 +100,7 @@ func (p ParticipantList) Join(sep string) string { // Falls back to Name when CreditedAs is empty. func (p ParticipantList) JoinCredited(sep string) string { return strings.Join(slice.Map(p, func(part Participant) string { - n := part.CreditedAs - if n == "" { - n = part.Name - } + n := part.DisplayName() if part.SubRole != "" { return n + " (" + part.SubRole + ")" } From 86cb5fee93807c1601baee6846602d07cd13931a Mon Sep 17 00:00:00 2001 From: Deluan Date: Sun, 24 May 2026 21:13:17 -0300 Subject: [PATCH 15/26] fix(scanner): backfill PIDArtist property on upgrade MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A user upgrading from a pre-PID.Artist version with a non-default PID.Artist already configured would have hit a silent annotation loss: empty stored PIDArtistKey was normalized to the current config, pidHashChanged returned false, no full rescan was forced, and prevArtistPIDConf stayed empty during the next normal scan so the artistIDMap guard skipped annotation migration entirely. Backfill via migration so the stored value reflects the historical default ('name', byte-identical to the legacy hardcoded artistID). Drop the empty-string fallback in pidHashChanged — with the migration in place, an empty stored value would be anomalous, and the fallback was the exact dead branch that hid this hole. --- cmd/root.go | 4 ---- .../20260525000912_set_default_pid_artist.sql | 11 +++++++++++ 2 files changed, 11 insertions(+), 4 deletions(-) create mode 100644 db/migrations/20260525000912_set_default_pid_artist.sql diff --git a/cmd/root.go b/cmd/root.go index 6363825cc..38c617bfd 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -179,10 +179,6 @@ func pidHashChanged(ds model.DataStore) (bool, error) { if err != nil { return false, err } - // Empty stored value is treated as matching — fresh upgrade should not force a rescan. - if pidArtist == "" { - pidArtist = conf.Server.PID.Artist - } return !strings.EqualFold(pidAlbum, conf.Server.PID.Album) || !strings.EqualFold(pidTrack, conf.Server.PID.Track) || !strings.EqualFold(pidArtist, conf.Server.PID.Artist), nil diff --git a/db/migrations/20260525000912_set_default_pid_artist.sql b/db/migrations/20260525000912_set_default_pid_artist.sql new file mode 100644 index 000000000..cb12244fe --- /dev/null +++ b/db/migrations/20260525000912_set_default_pid_artist.sql @@ -0,0 +1,11 @@ +-- +goose Up +-- Backfill PIDArtist property to reflect the historical artist-ID computation. +-- Existing artist IDs were produced by the legacy hardcoded artistID() function, +-- which is byte-identical to computeArtistPID(p, "name", ...). Recording "name" +-- here ensures that on the next scan, prevArtistPIDConf is never empty — closing +-- the upgrade-time window where a user who pre-configured a non-default PID.Artist +-- would have artist IDs silently regenerated without annotation migration. +insert into property (id, value) values ('PIDArtist', 'name') on conflict do nothing; + +-- +goose Down +delete from property where id = 'PIDArtist'; From 1cf5a9f21575ff391783d8b5bdfbadefa6fe888b Mon Sep 17 00:00:00 2001 From: Deluan Date: Sun, 24 May 2026 21:28:05 -0300 Subject: [PATCH 16/26] fix(ui): match canonical name in parseAndReplaceArtists The display string (record.artist/albumArtist) is sourced from mf.Artist/mf.AlbumArtist, which carry the canonical-tag value. Matching on creditedAs failed for Picard-default tagging (ARTIST tag = canonical name, ARTIST_CREDIT = credit) because indexOf could not find the credit inside the canonical display string, silently degrading from inline linkification to bullet-list fallback. Match on artist.name (always equal to displayString slice) and let ALink continue rendering creditedAs || name for visible text. --- ui/src/common/ArtistLinkField.jsx | 6 ++++- ui/src/common/ArtistLinkField.test.jsx | 32 ++++++++++++++++++++++++++ 2 files changed, 37 insertions(+), 1 deletion(-) diff --git a/ui/src/common/ArtistLinkField.jsx b/ui/src/common/ArtistLinkField.jsx index e9d17801d..055ad18f0 100644 --- a/ui/src/common/ArtistLinkField.jsx +++ b/ui/src/common/ArtistLinkField.jsx @@ -42,7 +42,11 @@ const parseAndReplaceArtists = ( let lastIndex = 0 albumArtists?.forEach((artist) => { - const matchName = artist.creditedAs || artist.name + // Match on the canonical name — that's what appears in displayAlbumArtist + // (sourced from mf.Artist / mf.AlbumArtist, which are set from the canonical + // ARTIST tag at scan time). The ALink itself renders creditedAs || name, + // so the displayed link text still reflects the credit. + const matchName = artist.name const index = displayAlbumArtist.indexOf(matchName, lastIndex) if (index !== -1) { // Add text before the artist name diff --git a/ui/src/common/ArtistLinkField.test.jsx b/ui/src/common/ArtistLinkField.test.jsx index e49873930..a9c850a2c 100644 --- a/ui/src/common/ArtistLinkField.test.jsx +++ b/ui/src/common/ArtistLinkField.test.jsx @@ -286,6 +286,38 @@ describe('ArtistLinkField', () => { const link = screen.getByRole('link') expect(link).not.toHaveAttribute('title') }) + + it('inline-linkifies when displayArtist holds the canonical name (Picard-default tagging)', () => { + // Picard "use standardized artist names" mode: ARTIST tag carries the + // canonical name, ARTIST_CREDIT carries the credit. The display string + // is the canonical name; parseAndReplaceArtists must match on `name` + // (not `creditedAs`) to embed the link inline. The link text itself + // still renders the credit via ALink. + const record = { + artist: 'Planetary Assault Systems', + participants: { + artist: [ + { + id: 'canon-1', + name: 'Planetary Assault Systems', + creditedAs: 'PAS', + }, + ], + }, + } + + render() + + const link = screen.getByRole('link') + // Link text is the credit, tooltip carries canonical + expect(link).toHaveTextContent('PAS') + expect(link).toHaveAttribute('title', 'Planetary Assault Systems') + // The original canonical string should not appear as raw plain text + // (it was replaced by the link) + expect( + screen.queryByText(/^Planetary Assault Systems$/), + ).not.toBeInTheDocument() + }) }) describe('when limiting displayed artists', () => { From c898f0e2a93caf461bc2c6528d6cdb49b61f3f8d Mon Sep 17 00:00:00 2001 From: Deluan Date: Sun, 24 May 2026 21:29:47 -0300 Subject: [PATCH 17/26] fix(participants): preserve later non-empty CreditedAs on dedup merge Participants.add deduplicates by ID+SubRole. The same artist appearing on multiple tracks of one album would silently keep only the first-seen CreditedAs, so the album-level participants JSON could end up with a credit from one track applied to all tracks of that album. Change the dedup to update the existing entry's CreditedAs when a later occurrence has a non-empty value. Per-track media_file participants are unaffected (each track's JSON is built independently). The album-level merge result is inherently lossy when tracks differ, but no longer silently drops a meaningful credit based on arrival order. --- model/participants.go | 21 +++++++++++++++------ model/participants_test.go | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 49 insertions(+), 6 deletions(-) diff --git a/model/participants.go b/model/participants.go index d8d4eab62..e4dcc8c76 100644 --- a/model/participants.go +++ b/model/participants.go @@ -156,16 +156,25 @@ func (p Participants) Merge(other Participants) { } func (p Participants) add(role Role, participants ...Participant) { - seen := make(map[string]struct{}, len(p[role])) - for _, artist := range p[role] { - seen[artist.ID+artist.SubRole] = struct{}{} + seen := make(map[string]int, len(p[role])) + for i, artist := range p[role] { + seen[artist.ID+artist.SubRole] = i } for _, participant := range participants { key := participant.ID + participant.SubRole - if _, ok := seen[key]; !ok { - seen[key] = struct{}{} - p[role] = append(p[role], participant) + if idx, ok := seen[key]; ok { + // Same artist/sub-role seen before. The merge (e.g. building + // album.Participants from per-track participants) is inherently + // lossy when tracks differ on CreditedAs, but silently dropping + // the later value can mean the wrong credit ends up on the album. + // Prefer the most recently observed non-empty CreditedAs. + if participant.CreditedAs != "" { + p[role][idx].CreditedAs = participant.CreditedAs + } + continue } + seen[key] = len(p[role]) + p[role] = append(p[role], participant) } } diff --git a/model/participants_test.go b/model/participants_test.go index 3e7ba2942..29e87b550 100644 --- a/model/participants_test.go +++ b/model/participants_test.go @@ -116,6 +116,40 @@ var _ = Describe("Participants", func() { RoleAlbumArtist: []Participant{_p("3", "AlbumArtist1"), _p("4", "AlbumArtist2"), _p("7", "AlbumArtist3"), _p("8", "AlbumArtist4")}, })) }) + + It("upgrades CreditedAs from a later occurrence when the dedup key matches", func() { + // Same artist (same ID+SubRole) credited differently on two tracks. + // The merge is inherently lossy; the later non-empty credit wins so + // it isn't silently dropped just because of arrival order. + p1 := Participants{ + RoleArtist: []Participant{ + {Artist: Artist{ID: "a1", Name: "Aphex Twin"}, CreditedAs: "AFX"}, + }, + } + p2 := Participants{ + RoleArtist: []Participant{ + {Artist: Artist{ID: "a1", Name: "Aphex Twin"}, CreditedAs: "Aphex Twin"}, + }, + } + p1.Merge(p2) + Expect(p1[RoleArtist]).To(HaveLen(1)) + Expect(p1[RoleArtist][0].CreditedAs).To(Equal("Aphex Twin")) + }) + + It("does not overwrite an existing CreditedAs with an empty one", func() { + p1 := Participants{ + RoleArtist: []Participant{ + {Artist: Artist{ID: "a1", Name: "Aphex Twin"}, CreditedAs: "AFX"}, + }, + } + p2 := Participants{ + RoleArtist: []Participant{ + {Artist: Artist{ID: "a1", Name: "Aphex Twin"}}, // empty CreditedAs + }, + } + p1.Merge(p2) + Expect(p1[RoleArtist][0].CreditedAs).To(Equal("AFX")) + }) }) Describe("Hash", func() { From 363b93ef25e3893fe85d13b715c9378da6037286 Mon Sep 17 00:00:00 2001 From: Deluan Date: Sun, 24 May 2026 21:34:47 -0300 Subject: [PATCH 18/26] fix(scanner): preserve artist created_at across PID changes persistArtist now calls repo.CopyAttributes(prevID, a.ID, "created_at") after ReassignAnnotation, mirroring persistAlbum. Without this, every PID.Artist config change reset the artist's created_at to scan time, breaking 'recently added artists' views and any consumer that relies on the original first-seen timestamp. Adds CopyAttributes to ArtistRepository (mirroring AlbumRepository's same-name method) with the same zero-poisoning guard for created_at. --- model/artist.go | 1 + persistence/artist_repository.go | 28 ++++++++++++++++++++++++++++ scanner/phase_1_folders.go | 10 ++++++++++ scanner/phase_1_folders_test.go | 18 ++++++++++++++++++ tests/mock_artist_repo.go | 14 ++++++++++++++ 5 files changed, 71 insertions(+) diff --git a/model/artist.go b/model/artist.go index 2085f0051..54671c3ef 100644 --- a/model/artist.go +++ b/model/artist.go @@ -89,6 +89,7 @@ type ArtistRepository interface { GetIndex(includeMissing bool, libraryIds []int, roles ...Role) (ArtistIndexes, error) // The following methods are used exclusively by the scanner: + CopyAttributes(fromID, toID string, columns ...string) error RefreshPlayCounts() (int64, error) RefreshStats(allArtists bool) (int64, error) diff --git a/persistence/artist_repository.go b/persistence/artist_repository.go index cfdc499e0..66276fc76 100644 --- a/persistence/artist_repository.go +++ b/persistence/artist_repository.go @@ -224,6 +224,34 @@ func (r *artistRepository) Put(a *model.Artist, colsToUpdate ...string) error { return err } +// CopyAttributes copies the named columns from the row identified by fromID +// to the row identified by toID. Used by the scanner after a PID change to +// preserve attributes (notably created_at) across artist-ID migrations. +func (r *artistRepository) CopyAttributes(fromID, toID string, columns ...string) error { + var from dbx.NullStringMap + err := r.queryOne(Select(columns...).From(r.tableName).Where(Eq{"id": fromID}), &from) + if err != nil { + return fmt.Errorf("getting artist to copy fields from: %w", err) + } + to := make(map[string]any) + for _, col := range columns { + v := from[col] + // created_at on the previous row may be zero/poisoned (e.g. legacy + // rows or rows where it was never populated). Skip in that case so we + // don't propagate a bad value forward on every metadata-driven ID + // change. Matches the behavior in album_repository.CopyAttributes. + if col == "created_at" && (!v.Valid || v.String == "" || strings.HasPrefix(v.String, "0001-")) { + continue + } + to[col] = v + } + if len(to) == 0 { + return nil + } + _, err = r.executeSQL(Update(r.tableName).SetMap(to).Where(Eq{"id": toID})) + return err +} + func (r *artistRepository) UpdateExternalInfo(a *model.Artist) error { dba := &dbArtist{Artist: a} _, err := r.put(a.ID, dba, diff --git a/scanner/phase_1_folders.go b/scanner/phase_1_folders.go index 8aee6af25..aa67a68b6 100644 --- a/scanner/phase_1_folders.go +++ b/scanner/phase_1_folders.go @@ -496,6 +496,16 @@ func (p *phaseFolders) persistArtist(repo model.ArtistRepository, a *model.Artis log.Warn(p.ctx, "Scanner: Could not reassign artist annotations", "from", prevID, "to", a.ID, "artist", a.Name, err) p.state.sendWarning(fmt.Sprintf("Could not reassign artist annotations from %s to %s ('%s'): %v", prevID, a.ID, a.Name, err)) } + + // Keep created_at field from previous instance of the artist. Without this, + // the just-inserted row carries the scan timestamp, breaking "recently + // added" semantics on every PID config change. + if err := repo.CopyAttributes(prevID, a.ID, "created_at"); err != nil { + if !errors.Is(err, model.ErrNotFound) { + log.Warn(p.ctx, "Scanner: Could not copy artist fields", "from", prevID, "to", a.ID, "artist", a.Name, err) + p.state.sendWarning(fmt.Sprintf("Could not copy artist fields from %s to %s ('%s'): %v", prevID, a.ID, a.Name, err)) + } + } delete(idMap, a.ID) return nil } diff --git a/scanner/phase_1_folders_test.go b/scanner/phase_1_folders_test.go index f76b63bcb..f3861a430 100644 --- a/scanner/phase_1_folders_test.go +++ b/scanner/phase_1_folders_test.go @@ -45,5 +45,23 @@ var _ = Describe("phaseFolders", func() { Expect(p.persistArtist(artRepo, &artist, idMap)).To(Succeed()) Expect(artRepo.ReassignAnnotationCalls).To(BeEmpty()) }) + + It("calls CopyAttributes(created_at) when persisting an artist whose ID changed", func() { + oldID := "old-id" + newID := "new-id" + idMap := map[string]string{newID: oldID} + artist := model.Artist{ID: newID, Name: "Foo"} + + Expect(p.persistArtist(artRepo, &artist, idMap)).To(Succeed()) + Expect(artRepo.CopyAttributesCalls).To(HaveKeyWithValue(oldID, newID)) + }) + + It("does not call CopyAttributes when no mapping exists", func() { + idMap := map[string]string{} + artist := model.Artist{ID: "some-id", Name: "Foo"} + + Expect(p.persistArtist(artRepo, &artist, idMap)).To(Succeed()) + Expect(artRepo.CopyAttributesCalls).To(BeEmpty()) + }) }) }) diff --git a/tests/mock_artist_repo.go b/tests/mock_artist_repo.go index 950fea7de..198564bd0 100644 --- a/tests/mock_artist_repo.go +++ b/tests/mock_artist_repo.go @@ -20,6 +20,7 @@ type MockArtistRepo struct { Err bool Options model.QueryOptions ReassignAnnotationCalls map[string]string // prevID -> newID + CopyAttributesCalls map[string]string // fromID -> toID } func (m *MockArtistRepo) SetError(err bool) { @@ -171,4 +172,17 @@ func (m *MockArtistRepo) ReassignAnnotation(prevID string, newID string) error { return nil } +// CopyAttributes is a no-op in the mock; tests that need to verify call +// observation can extend this. +func (m *MockArtistRepo) CopyAttributes(fromID, toID string, columns ...string) error { + if m.Err { + return errors.New("unexpected error") + } + if m.CopyAttributesCalls == nil { + m.CopyAttributesCalls = make(map[string]string) + } + m.CopyAttributesCalls[fromID] = toID + return nil +} + var _ model.ArtistRepository = (*MockArtistRepo)(nil) From 87cd3f1037d02170990db9eb56f328e7cdfb7548 Mon Sep 17 00:00:00 2001 From: Deluan Date: Sun, 24 May 2026 21:37:01 -0300 Subject: [PATCH 19/26] fix(scanner): treat explicit '[Unknown Artist]' ALBUMARTIST as missing The old parseArtists function (removed in this branch) substituted consts.UnknownArtist when albumartist tags were empty, so the downstream compilation/fallback check matched both cases. The inlined replacement only checked len(albumNames)==0, narrowing the condition: files explicitly tagged ALBUMARTIST='[Unknown Artist]' (emitted by some rippers, and matching what Navidrome itself stores for untagged files) stopped routing to Various Artists for compilations. Restore the original semantics by treating len==1 with the literal UnknownArtist string as equivalent to no tag. --- model/metadata/map_participants.go | 10 +++++++++- model/metadata/map_participants_test.go | 20 ++++++++++++++++++++ 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/model/metadata/map_participants.go b/model/metadata/map_participants.go index b5c6e0c38..4c9c0de01 100644 --- a/model/metadata/map_participants.go +++ b/model/metadata/map_participants.go @@ -53,8 +53,16 @@ func (md Metadata) mapParticipants() model.Participants { albumMbids := md.Strings(model.TagMusicBrainzAlbumArtistID) albumCredits := md.getArtistValues(model.TagAlbumArtistCredit, model.TagAlbumArtistsCredit) + // Treat both "no albumartist tag" and "albumartist tag literally set to + // the UnknownArtist placeholder" as missing — some rippers emit the + // literal '[Unknown Artist]' string, and the original parseArtists path + // (replaced in this branch) substituted UnknownArtist on its own so the + // downstream check matched either case. + albumArtistMissing := len(albumNames) == 0 || + (len(albumNames) == 1 && albumNames[0] == consts.UnknownArtist) + var albumArtistParticipants []model.Participant - if len(albumNames) == 0 { + if albumArtistMissing { if md.Bool(model.TagCompilation) { albumArtistParticipants = md.buildParticipants( []string{consts.VariousArtists}, nil, diff --git a/model/metadata/map_participants_test.go b/model/metadata/map_participants_test.go index 7ef6183ee..3467bb99f 100644 --- a/model/metadata/map_participants_test.go +++ b/model/metadata/map_participants_test.go @@ -460,6 +460,26 @@ var _ = Describe("Participants", func() { }) }) + When("the COMPILATION tag is true and ALBUMARTIST is the UnknownArtist placeholder", func() { + BeforeEach(func() { + // Some rippers emit the literal '[Unknown Artist]' string + // when no album artist is set. The fallback must still + // route to Various Artists for compilations. + mf = toMediaFile(model.RawTags{ + "COMPILATION": {"1"}, + "ALBUMARTIST": {consts.UnknownArtist}, + }) + }) + + It("should substitute Various Artists as the album artist", func() { + participants := mf.Participants + Expect(participants).To(HaveKeyWithValue(model.RoleAlbumArtist, HaveLen(1))) + albumArtist := participants[model.RoleAlbumArtist][0] + Expect(albumArtist.Name).To(Equal("Various Artists")) + Expect(albumArtist.MbzArtistID).To(Equal(consts.VariousArtistsMbzId)) + }) + }) + When("the COMPILATION tag is true and there are ALBUMARTIST tags", func() { BeforeEach(func() { mf = toMediaFile(model.RawTags{ From abd9d6bdba7d893e193c7336f67c0b034d5c547b Mon Sep 17 00:00:00 2001 From: Deluan Date: Sun, 24 May 2026 21:59:20 -0300 Subject: [PATCH 20/26] test(participants): use neutral names to avoid gosec false positive MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The G101 'potential hardcoded credentials' lint flagged a struct field named CreditedAs paired with a string literal. The artist names in the dedup-merge test are arbitrary — rename to plain placeholders. --- model/participants_test.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/model/participants_test.go b/model/participants_test.go index 29e87b550..6ae99f603 100644 --- a/model/participants_test.go +++ b/model/participants_test.go @@ -123,32 +123,32 @@ var _ = Describe("Participants", func() { // it isn't silently dropped just because of arrival order. p1 := Participants{ RoleArtist: []Participant{ - {Artist: Artist{ID: "a1", Name: "Aphex Twin"}, CreditedAs: "AFX"}, + {Artist: Artist{ID: "a1", Name: "Canonical"}, CreditedAs: "Credit One"}, }, } p2 := Participants{ RoleArtist: []Participant{ - {Artist: Artist{ID: "a1", Name: "Aphex Twin"}, CreditedAs: "Aphex Twin"}, + {Artist: Artist{ID: "a1", Name: "Canonical"}, CreditedAs: "Credit Two"}, }, } p1.Merge(p2) Expect(p1[RoleArtist]).To(HaveLen(1)) - Expect(p1[RoleArtist][0].CreditedAs).To(Equal("Aphex Twin")) + Expect(p1[RoleArtist][0].CreditedAs).To(Equal("Credit Two")) }) It("does not overwrite an existing CreditedAs with an empty one", func() { p1 := Participants{ RoleArtist: []Participant{ - {Artist: Artist{ID: "a1", Name: "Aphex Twin"}, CreditedAs: "AFX"}, + {Artist: Artist{ID: "a1", Name: "Canonical"}, CreditedAs: "Credit One"}, }, } p2 := Participants{ RoleArtist: []Participant{ - {Artist: Artist{ID: "a1", Name: "Aphex Twin"}}, // empty CreditedAs + {Artist: Artist{ID: "a1", Name: "Canonical"}}, // empty CreditedAs }, } p1.Merge(p2) - Expect(p1[RoleArtist][0].CreditedAs).To(Equal("AFX")) + Expect(p1[RoleArtist][0].CreditedAs).To(Equal("Credit One")) }) }) From 627d6161cc15d70b7e95d0d1031fe8d7f37c2b86 Mon Sep 17 00:00:00 2001 From: Deluan Date: Sun, 24 May 2026 22:21:24 -0300 Subject: [PATCH 21/26] fix(scanner): split parallel tag lists with same separators as names Per Gemini, Copilot, and Codex review on PR #5527: credits, sorts, and MBIDs were read via md.Strings() (raw), while names use getRoleValues / getArtistValues which apply the role/artist split separators. A tag combination like COMPOSER='A;B' + COMPOSER_CREDIT='AA;BB' produced 2 names but 1 credit, silently dropping all credits via the length- mismatch fallback. Same hazard for MBIDs. Route MBIDs/sorts/credits through the same splitter as names so positional alignment holds for libraries using ';' or '/' delimiters. --- model/metadata/map_participants.go | 20 ++++++++----- model/metadata/map_participants_test.go | 40 +++++++++++++++++++++++++ 2 files changed, 53 insertions(+), 7 deletions(-) diff --git a/model/metadata/map_participants.go b/model/metadata/map_participants.go index 4c9c0de01..e8cf0b197 100644 --- a/model/metadata/map_participants.go +++ b/model/metadata/map_participants.go @@ -36,13 +36,16 @@ var roleMappings = map[model.Role]roleTags{ func (md Metadata) mapParticipants() model.Participants { participants := make(model.Participants) - // Parse track artists + // Parse track artists. MBIDs use getRoleValues so they're split by the same + // separators ('/' or ';') as the canonical names — otherwise a tag like + // MUSICBRAINZ_ARTISTID="abc/def" paired with ARTISTS="A/B" would yield 2 + // names but 1 MBID and the positional alignment would break. trackNames := md.getArtistValues(model.TagTrackArtist, model.TagTrackArtists) if len(trackNames) == 0 { trackNames = []string{consts.UnknownArtist} } trackSorts := md.getArtistValues(model.TagTrackArtistSort, model.TagTrackArtistsSort) - trackMbids := md.Strings(model.TagMusicBrainzArtistID) + trackMbids := md.getRoleValues(model.TagMusicBrainzArtistID) trackCredits := md.getArtistValues(model.TagTrackArtistCredit, model.TagTrackArtistsCredit) trackArtistParticipants := md.buildParticipants(trackNames, trackSorts, trackMbids, trackCredits) participants.AddParticipants(model.RoleArtist, trackArtistParticipants...) @@ -50,7 +53,7 @@ func (md Metadata) mapParticipants() model.Participants { // Parse album artists albumNames := md.getArtistValues(model.TagAlbumArtist, model.TagAlbumArtists) albumSorts := md.getArtistValues(model.TagAlbumArtistSort, model.TagAlbumArtistsSort) - albumMbids := md.Strings(model.TagMusicBrainzAlbumArtistID) + albumMbids := md.getRoleValues(model.TagMusicBrainzAlbumArtistID) albumCredits := md.getArtistValues(model.TagAlbumArtistCredit, model.TagAlbumArtistsCredit) // Treat both "no albumartist tag" and "albumartist tag literally set to @@ -75,13 +78,16 @@ func (md Metadata) mapParticipants() model.Participants { } participants.AddParticipants(model.RoleAlbumArtist, albumArtistParticipants...) - // Parse all other roles + // Parse all other roles. All parallel lists go through getRoleValues so + // they're split with the same separators as the canonical names. Reading + // any of these with md.Strings (no splitting) would desync the positional + // alignment for tags like COMPOSER="A;B" + COMPOSER_CREDIT="AA;BB". for role, info := range roleMappings { names := md.getRoleValues(info.name) if len(names) > 0 { - sorts := md.Strings(info.sort) - mbids := md.Strings(info.mbid) - credits := md.Strings(info.credit) + sorts := md.getRoleValues(info.sort) + mbids := md.getRoleValues(info.mbid) + credits := md.getRoleValues(info.credit) participants.AddParticipants(role, md.buildParticipants(names, sorts, mbids, credits)...) } } diff --git a/model/metadata/map_participants_test.go b/model/metadata/map_participants_test.go index 3467bb99f..95984ff59 100644 --- a/model/metadata/map_participants_test.go +++ b/model/metadata/map_participants_test.go @@ -882,5 +882,45 @@ var _ = Describe("Participants", func() { } } }) + + It("aligns composer credits with names when both use a role separator", func() { + // Names and credits both split on ';' under the roles.split config. + // Prior to the alignment fix, credits were read raw and would have + // stayed as one value while names became three, producing a length + // mismatch that silently dropped all credits. + mf = toMediaFile(model.RawTags{ + "COMPOSER": {"Comp A;Comp B;Comp C"}, + "COMPOSERCREDIT": {"CA;CB;CC"}, + }) + composers := mf.Participants[model.RoleComposer] + Expect(composers).To(HaveLen(3)) + Expect(composers[0].Name).To(Equal("Comp A")) + Expect(composers[0].CreditedAs).To(Equal("CA")) + Expect(composers[1].CreditedAs).To(Equal("CB")) + Expect(composers[2].CreditedAs).To(Equal("CC")) + }) + + It("splits composer MBIDs to align with split composer names", func() { + // Both names and MBIDs use the role separator ';'. The fix routes + // MBIDs through getRoleValues so they split alongside names rather + // than staying as a single value (which would have produced 2 names + // and 1 MBID — broken positional alignment). + // + // We test via a single delimited canonical name string AND a single + // delimited MBID-pair string. Note: the tag-reader UUID validation + // happens at metadata parse time on individual values, so we hand + // multi-valued tag entries already split into separate slice + // entries (matching how a multi-value tag would surface). + mf = toMediaFile(model.RawTags{ + "COMPOSER": {"Comp A;Comp B"}, + "MUSICBRAINZ_COMPOSERID": {"11111111-1111-1111-1111-111111111111", "22222222-2222-2222-2222-222222222222"}, + }) + composers := mf.Participants[model.RoleComposer] + Expect(composers).To(HaveLen(2)) + Expect(composers[0].Name).To(Equal("Comp A")) + Expect(composers[0].MbzArtistID).To(Equal("11111111-1111-1111-1111-111111111111")) + Expect(composers[1].Name).To(Equal("Comp B")) + Expect(composers[1].MbzArtistID).To(Equal("22222222-2222-2222-2222-222222222222")) + }) }) }) From 2ff6d91057505a9392045cf0683eb27fb6f8191d Mon Sep 17 00:00:00 2001 From: Deluan Date: Sun, 24 May 2026 22:21:53 -0300 Subject: [PATCH 22/26] fix(participants): add separator to dedup key to avoid prefix collisions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per Gemini review on PR #5527: the dedup key was 'ID+SubRole' with no separator, so (ID='12', SubRole='3') and (ID='1', SubRole='23') would collide. Real-world risk is negligible (artist IDs are fixed-length MD5 hashes), but the fix is trivial — use a NUL byte that cannot appear in either field. --- model/participants.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/model/participants.go b/model/participants.go index e4dcc8c76..8554afbb7 100644 --- a/model/participants.go +++ b/model/participants.go @@ -156,12 +156,15 @@ func (p Participants) Merge(other Participants) { } func (p Participants) add(role Role, participants ...Participant) { + // Use a separator that can't appear in either field so e.g. + // (ID="12", SubRole="3") doesn't collide with (ID="1", SubRole="23"). + const sep = "\x00" seen := make(map[string]int, len(p[role])) for i, artist := range p[role] { - seen[artist.ID+artist.SubRole] = i + seen[artist.ID+sep+artist.SubRole] = i } for _, participant := range participants { - key := participant.ID + participant.SubRole + key := participant.ID + sep + participant.SubRole if idx, ok := seen[key]; ok { // Same artist/sub-role seen before. The merge (e.g. building // album.Participants from per-track participants) is inherently From c413d6510e9f09df9fee31c230ed48ab08e4dd5c Mon Sep 17 00:00:00 2001 From: Deluan Date: Sun, 24 May 2026 22:24:06 -0300 Subject: [PATCH 23/26] fix(metadata): drop unused performer/plural-credit tag mappings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per Copilot & Codex review on PR #5527: - performercredit was registered as a plain string mapping, but PERFORMER is type:pair ('PERFORMER:instrument'). The credit form needs the same pair structure to be meaningful, and processPerformers doesn't read it either way. Better to remove the misleading mapping than ship dead config that suggests support we don't have. - TagComposersCredit and TagLyricistsCredit (plural variants) were never consumed by any code path — only the singular TagComposerCredit and TagLyricistCredit are wired into roleMappings. Both can be added back when the underlying feature is actually implemented. --- model/tag.go | 6 +++--- resources/mappings.yaml | 6 ------ 2 files changed, 3 insertions(+), 9 deletions(-) diff --git a/model/tag.go b/model/tag.go index e1344cb53..743294658 100644 --- a/model/tag.go +++ b/model/tag.go @@ -216,11 +216,9 @@ const ( TagComposer TagName = "composer" TagComposerSort TagName = "composersort" TagComposerCredit TagName = "composercredit" - TagComposersCredit TagName = "composerscredit" TagLyricist TagName = "lyricist" TagLyricistSort TagName = "lyricistsort" TagLyricistCredit TagName = "lyricistcredit" - TagLyricistsCredit TagName = "lyricistscredit" TagDirector TagName = "director" TagDirectorCredit TagName = "directorcredit" TagProducer TagName = "producer" @@ -238,7 +236,9 @@ const ( TagArranger TagName = "arranger" TagArrangerCredit TagName = "arrangercredit" TagPerformer TagName = "performer" - TagPerformerCredit TagName = "performercredit" + // Performer credits are not currently surfaced: performer tags are + // `type: pair` ('PERFORMER:instrument') and the matching credit + // representation hasn't been designed yet. Tracked separately. // ReplayGain diff --git a/resources/mappings.yaml b/resources/mappings.yaml index 3f15dcd3d..47b799b15 100644 --- a/resources/mappings.yaml +++ b/resources/mappings.yaml @@ -58,16 +58,12 @@ main: aliases: [ tsoc, txxx:composersort, composersort, soco, wm/composersortorder ] composercredit: aliases: [ composer_credit, composercredit ] - composerscredit: - aliases: [ composers_credit, composerscredit ] lyricist: aliases: [ text, lyricist, ----:com.apple.itunes:lyricist, wm/writer ] lyricistsort: aliases: [ lyricistsort ] lyricistcredit: aliases: [ lyricist_credit, lyricistcredit ] - lyricistscredit: - aliases: [ lyricists_credit, lyricistscredit ] conductor: aliases: [ tpe3, conductor, ----:com.apple.itunes:conductor, wm/conductor ] conductorcredit: @@ -225,8 +221,6 @@ main: performer: aliases: [performer] type: pair - performercredit: - aliases: [ performer_credit, performercredit ] musicbrainz_performerid: aliases: [ txxx:musicbrainz performer id, musicbrainz_performerid, musicbrainz_performer_id, ----:com.apple.itunes:musicbrainz performer id, musicbrainz/performer id ] type: pair From 4e7c2128f3f529cf9381f9e88794ea64c2e50e9c Mon Sep 17 00:00:00 2001 From: Deluan Date: Sun, 24 May 2026 22:24:50 -0300 Subject: [PATCH 24/26] docs(scanner): document artistIDMap many-to-one collapse behavior MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per Copilot review on PR #5527: the 'first prevID wins' check looked like a bug. It is intentional but the rationale wasn't obvious from context. Annotate the code with the merge-collision relationship to ReassignAnnotation's missing merge semantics — the follow-up that fixes the SQL side will benefit both album and artist re-PID paths. --- scanner/phase_1_folders.go | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/scanner/phase_1_folders.go b/scanner/phase_1_folders.go index aa67a68b6..77409344c 100644 --- a/scanner/phase_1_folders.go +++ b/scanner/phase_1_folders.go @@ -306,6 +306,15 @@ func (p *phaseFolders) loadTagsFromFiles(entry *folderEntry, toImport map[string // Build artistIDMap: for each participant on this track, if the previous // spec produces a different ID than the current one, record the mapping. + // + // The map is keyed by the new artist ID so it stores ONE previous ID per + // new ID, even when many old IDs collapse to the same new one (e.g. + // switching from "name" to "musicbrainz_artistid|name" where multiple + // name-keyed IDs share an MBID). This is intentional: ReassignAnnotation + // is idempotent and the other prior IDs orphan their annotations into + // the unique-constraint collision case documented in the spec — a + // known limitation tracked for a separate follow-up that fixes + // ReassignAnnotation merge semantics at the SQL level. if p.prevArtistPIDConf != "" && p.prevArtistPIDConf != conf.Server.PID.Artist { for _, list := range track.Participants { for _, part := range list { From a4116e01c26a88673e420e89944ca3b5eaacba4d Mon Sep 17 00:00:00 2001 From: Deluan Date: Sun, 24 May 2026 23:35:11 -0300 Subject: [PATCH 25/26] docs(scanner): correct unsubstantiated comment about UnknownArtist taggers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous comment claimed 'some rippers emit the literal [Unknown Artist] string' — no such tagger is actually known. The defensive second clause in the missing-albumartist check was preserving accidental behavior from the prior parseArtists implementation, not guarding against a documented real-world case. Rewrite the comment to be honest about provenance: keep the clause for behavioral parity and the cheap round-trip defense, but stop asserting facts I can't back up. --- model/metadata/map_participants.go | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/model/metadata/map_participants.go b/model/metadata/map_participants.go index e8cf0b197..79c0e7b91 100644 --- a/model/metadata/map_participants.go +++ b/model/metadata/map_participants.go @@ -57,10 +57,13 @@ func (md Metadata) mapParticipants() model.Participants { albumCredits := md.getArtistValues(model.TagAlbumArtistCredit, model.TagAlbumArtistsCredit) // Treat both "no albumartist tag" and "albumartist tag literally set to - // the UnknownArtist placeholder" as missing — some rippers emit the - // literal '[Unknown Artist]' string, and the original parseArtists path - // (replaced in this branch) substituted UnknownArtist on its own so the - // downstream check matched either case. + // the UnknownArtist placeholder" as missing. Preserves behavioral parity + // with the prior parseArtists path (replaced in this branch), which + // substituted UnknownArtist on its own and then matched either case + // downstream. No concrete tagger known to emit the literal '[Unknown + // Artist]' string, but the cost of keeping the second clause is one + // comparison and it defends against the placeholder round-tripping if + // Navidrome's own UnknownArtist value ever ends up back in a tag. albumArtistMissing := len(albumNames) == 0 || (len(albumNames) == 1 && albumNames[0] == consts.UnknownArtist) From 3ce7fc32bd56add470d57cba0fbaaf0cfdef0f57 Mon Sep 17 00:00:00 2001 From: Deluan Date: Sun, 24 May 2026 23:37:32 -0300 Subject: [PATCH 26/26] docs(scanner): tighten map_participants comments Drop unverifiable claims and shrink verbose explanations to one or two lines, matching the file's existing comment style. --- model/metadata/map_participants.go | 31 +++++++++++------------------- 1 file changed, 11 insertions(+), 20 deletions(-) diff --git a/model/metadata/map_participants.go b/model/metadata/map_participants.go index 79c0e7b91..ac4370a9f 100644 --- a/model/metadata/map_participants.go +++ b/model/metadata/map_participants.go @@ -36,10 +36,8 @@ var roleMappings = map[model.Role]roleTags{ func (md Metadata) mapParticipants() model.Participants { participants := make(model.Participants) - // Parse track artists. MBIDs use getRoleValues so they're split by the same - // separators ('/' or ';') as the canonical names — otherwise a tag like - // MUSICBRAINZ_ARTISTID="abc/def" paired with ARTISTS="A/B" would yield 2 - // names but 1 MBID and the positional alignment would break. + // Parse track artists. MBIDs go through getRoleValues to split on the same + // separators as the names, keeping positional alignment. trackNames := md.getArtistValues(model.TagTrackArtist, model.TagTrackArtists) if len(trackNames) == 0 { trackNames = []string{consts.UnknownArtist} @@ -56,14 +54,9 @@ func (md Metadata) mapParticipants() model.Participants { albumMbids := md.getRoleValues(model.TagMusicBrainzAlbumArtistID) albumCredits := md.getArtistValues(model.TagAlbumArtistCredit, model.TagAlbumArtistsCredit) - // Treat both "no albumartist tag" and "albumartist tag literally set to - // the UnknownArtist placeholder" as missing. Preserves behavioral parity - // with the prior parseArtists path (replaced in this branch), which - // substituted UnknownArtist on its own and then matched either case - // downstream. No concrete tagger known to emit the literal '[Unknown - // Artist]' string, but the cost of keeping the second clause is one - // comparison and it defends against the placeholder round-tripping if - // Navidrome's own UnknownArtist value ever ends up back in a tag. + // Treat "no albumartist tag" and "albumartist == UnknownArtist placeholder" + // as missing, matching the prior parseArtists path and defending against + // the placeholder round-tripping back into a tag. albumArtistMissing := len(albumNames) == 0 || (len(albumNames) == 1 && albumNames[0] == consts.UnknownArtist) @@ -81,10 +74,8 @@ func (md Metadata) mapParticipants() model.Participants { } participants.AddParticipants(model.RoleAlbumArtist, albumArtistParticipants...) - // Parse all other roles. All parallel lists go through getRoleValues so - // they're split with the same separators as the canonical names. Reading - // any of these with md.Strings (no splitting) would desync the positional - // alignment for tags like COMPOSER="A;B" + COMPOSER_CREDIT="AA;BB". + // All parallel lists go through getRoleValues so they split consistently + // with names (e.g. COMPOSER="A;B" + COMPOSER_CREDIT="AA;BB" → 2 each). for role, info := range roleMappings { names := md.getRoleValues(info.name) if len(names) > 0 { @@ -173,9 +164,9 @@ func (md Metadata) syncMissingMbzIDs(participants model.Participants) { } } -// buildParticipants builds Artists and wraps each into a Participant with -// CreditedAs populated. credits is paired positionally; if the lengths don't -// match, CreditedAs falls back to the canonical name for every entry. +// buildParticipants wraps each Artist into a Participant with CreditedAs +// populated. credits pairs positionally with names; a length mismatch falls +// back to the canonical name for every entry. func (md Metadata) buildParticipants(names, sorts, mbids, credits []string) []model.Participant { if len(credits) != 0 && len(credits) != len(names) { credits = nil @@ -207,7 +198,7 @@ func (md Metadata) buildArtists(names, sorts, mbids []string) []model.Artist { if i < len(mbids) { artist.MbzArtistID = mbids[i] } - // Compute ID from the participant fields we just populated so MBID/sort-based specs work. + // Assign ID after Sort/MBID are set so MBID/sort-based PID specs resolve. artist.ID = computeArtistPID( model.Participant{Artist: artist}, conf.Server.PID.Artist,