feat(agents): add ID field to Artist and Song structs with direct matching

Add ID field to Artist and Song structs in the agents package. When resolving
similar artists and top songs, the provider now uses a three-phase lookup:
1. Direct ID match (if agent returns internal Navidrome IDs)
2. MBID exact match (if MusicBrainz ID is available)
3. Fuzzy name/title match (existing behavior)

This enables agents to return more precise matches when they have access to
internal database IDs, while maintaining backward compatibility with
name-based matching.
This commit is contained in:
Deluan
2026-01-11 17:06:25 -05:00
parent 5c3568f758
commit 55966ba5ec
5 changed files with 325 additions and 25 deletions
+173 -24
View File
@@ -426,17 +426,21 @@ func (e *provider) getMatchingTopSongs(ctx context.Context, agent agents.ArtistT
return nil, fmt.Errorf("failed to get top songs for artist %s: %w", artistName, err)
}
idMatches, err := e.loadTracksByID(ctx, songs)
if err != nil {
return nil, fmt.Errorf("failed to load tracks by ID: %w", err)
}
mbidMatches, err := e.loadTracksByMBID(ctx, songs)
if err != nil {
return nil, fmt.Errorf("failed to load tracks by MBID: %w", err)
}
titleMatches, err := e.loadTracksByTitle(ctx, songs, artist, mbidMatches)
titleMatches, err := e.loadTracksByTitle(ctx, songs, artist, idMatches, mbidMatches)
if err != nil {
return nil, fmt.Errorf("failed to load tracks by title: %w", err)
}
log.Trace(ctx, "Top Songs loaded", "name", artistName, "numSongs", len(songs), "numMBIDMatches", len(mbidMatches), "numTitleMatches", len(titleMatches))
mfs := e.selectTopSongs(songs, mbidMatches, titleMatches, count)
log.Trace(ctx, "Top Songs loaded", "name", artistName, "numSongs", len(songs), "numIDMatches", len(idMatches), "numMBIDMatches", len(mbidMatches), "numTitleMatches", len(titleMatches))
mfs := e.selectTopSongs(songs, idMatches, mbidMatches, titleMatches, count)
if len(mfs) == 0 {
log.Debug(ctx, "No matching top songs found", "name", artistName)
@@ -477,9 +481,41 @@ func (e *provider) loadTracksByMBID(ctx context.Context, songs []agents.Song) (m
return matches, nil
}
func (e *provider) loadTracksByTitle(ctx context.Context, songs []agents.Song, artist *auxArtist, mbidMatches map[string]model.MediaFile) (map[string]model.MediaFile, error) {
func (e *provider) loadTracksByID(ctx context.Context, songs []agents.Song) (map[string]model.MediaFile, error) {
var ids []string
for _, s := range songs {
if s.ID != "" {
ids = append(ids, s.ID)
}
}
matches := map[string]model.MediaFile{}
if len(ids) == 0 {
return matches, nil
}
res, err := e.ds.MediaFile(ctx).GetAll(model.QueryOptions{
Filters: squirrel.And{
squirrel.Eq{"id": ids},
squirrel.Eq{"missing": false},
},
})
if err != nil {
return matches, err
}
for _, mf := range res {
if _, ok := matches[mf.ID]; !ok {
matches[mf.ID] = mf
}
}
return matches, nil
}
func (e *provider) loadTracksByTitle(ctx context.Context, songs []agents.Song, artist *auxArtist, idMatches, mbidMatches map[string]model.MediaFile) (map[string]model.MediaFile, error) {
titleMap := map[string]string{}
for _, s := range songs {
// Skip if already matched by ID or MBID
if s.ID != "" && idMatches[s.ID].ID != "" {
continue
}
if s.MBID != "" && mbidMatches[s.MBID].ID != "" {
continue
}
@@ -518,18 +554,27 @@ func (e *provider) loadTracksByTitle(ctx context.Context, songs []agents.Song, a
return matches, nil
}
func (e *provider) selectTopSongs(songs []agents.Song, byMBID, byTitle map[string]model.MediaFile, count int) model.MediaFiles {
func (e *provider) selectTopSongs(songs []agents.Song, byID, byMBID, byTitle map[string]model.MediaFile, count int) model.MediaFiles {
var mfs model.MediaFiles
for _, t := range songs {
if len(mfs) == count {
break
}
// Try ID match first
if t.ID != "" {
if mf, ok := byID[t.ID]; ok {
mfs = append(mfs, mf)
continue
}
}
// Try MBID match second
if t.MBID != "" {
if mf, ok := byMBID[t.MBID]; ok {
mfs = append(mfs, mf)
continue
}
}
// Fall back to title match
if mf, ok := byTitle[str.SanitizeFieldForSorting(t.Name)]; ok {
mfs = append(mfs, mf)
}
@@ -593,36 +638,51 @@ func (e *provider) mapSimilarArtists(ctx context.Context, similar []agents.Artis
var result model.Artists
var notPresent []string
artistNames := slice.Map(similar, func(artist agents.Artist) string { return artist.Name })
// Query all artists at once
clauses := slice.Map(artistNames, func(name string) squirrel.Sqlizer {
return squirrel.Like{"artist.name": name}
})
artists, err := e.ds.Artist(ctx).GetAll(model.QueryOptions{
Filters: squirrel.Or(clauses),
})
// Load artists by ID (highest priority)
idMatches, err := e.loadArtistsByID(ctx, similar)
if err != nil {
return nil, err
}
// Create a map for quick lookup
artistMap := make(map[string]model.Artist)
for _, artist := range artists {
artistMap[artist.Name] = artist
// Load artists by MBID (second priority)
mbidMatches, err := e.loadArtistsByMBID(ctx, similar, idMatches)
if err != nil {
return nil, err
}
// Load artists by name (lowest priority, fallback)
nameMatches, err := e.loadArtistsByName(ctx, similar, idMatches, mbidMatches)
if err != nil {
return nil, err
}
count := 0
// Process the similar artists
// Process the similar artists using priority: ID → MBID → Name
for _, s := range similar {
if artist, found := artistMap[s.Name]; found {
if count >= limit {
break
}
// Try ID match first
if s.ID != "" {
if artist, found := idMatches[s.ID]; found {
result = append(result, artist)
count++
continue
}
}
// Try MBID match second
if s.MBID != "" {
if artist, found := mbidMatches[s.MBID]; found {
result = append(result, artist)
count++
continue
}
}
// Fall back to name match
if artist, found := nameMatches[s.Name]; found {
result = append(result, artist)
count++
if count >= limit {
break
}
} else {
notPresent = append(notPresent, s.Name)
}
@@ -645,6 +705,95 @@ func (e *provider) mapSimilarArtists(ctx context.Context, similar []agents.Artis
return result, nil
}
func (e *provider) loadArtistsByID(ctx context.Context, similar []agents.Artist) (map[string]model.Artist, error) {
var ids []string
for _, s := range similar {
if s.ID != "" {
ids = append(ids, s.ID)
}
}
matches := map[string]model.Artist{}
if len(ids) == 0 {
return matches, nil
}
res, err := e.ds.Artist(ctx).GetAll(model.QueryOptions{
Filters: squirrel.Eq{"artist.id": ids},
})
if err != nil {
return matches, err
}
for _, a := range res {
if _, ok := matches[a.ID]; !ok {
matches[a.ID] = a
}
}
return matches, nil
}
func (e *provider) loadArtistsByMBID(ctx context.Context, similar []agents.Artist, idMatches map[string]model.Artist) (map[string]model.Artist, error) {
var mbids []string
for _, s := range similar {
// Skip if already matched by ID
if s.ID != "" && idMatches[s.ID].ID != "" {
continue
}
if s.MBID != "" {
mbids = append(mbids, s.MBID)
}
}
matches := map[string]model.Artist{}
if len(mbids) == 0 {
return matches, nil
}
res, err := e.ds.Artist(ctx).GetAll(model.QueryOptions{
Filters: squirrel.Eq{"mbz_artist_id": mbids},
})
if err != nil {
return matches, err
}
for _, a := range res {
if id := a.MbzArtistID; id != "" {
if _, ok := matches[id]; !ok {
matches[id] = a
}
}
}
return matches, nil
}
func (e *provider) loadArtistsByName(ctx context.Context, similar []agents.Artist, idMatches map[string]model.Artist, mbidMatches map[string]model.Artist) (map[string]model.Artist, error) {
var names []string
for _, s := range similar {
// Skip if already matched by ID or MBID
if s.ID != "" && idMatches[s.ID].ID != "" {
continue
}
if s.MBID != "" && mbidMatches[s.MBID].ID != "" {
continue
}
names = append(names, s.Name)
}
matches := map[string]model.Artist{}
if len(names) == 0 {
return matches, nil
}
clauses := slice.Map(names, func(name string) squirrel.Sqlizer {
return squirrel.Like{"artist.name": name}
})
res, err := e.ds.Artist(ctx).GetAll(model.QueryOptions{
Filters: squirrel.Or(clauses),
})
if err != nil {
return matches, err
}
for _, a := range res {
if _, ok := matches[a.Name]; !ok {
matches[a.Name] = a
}
}
return matches, nil
}
func (e *provider) findArtistByName(ctx context.Context, artistName string) (*auxArtist, error) {
artists, err := e.ds.Artist(ctx).GetAll(model.QueryOptions{
Filters: squirrel.Like{"artist.name": artistName},
+10 -1
View File
@@ -4,6 +4,7 @@ import (
"context"
"errors"
"github.com/Masterminds/squirrel"
"github.com/navidrome/navidrome/core/agents"
. "github.com/navidrome/navidrome/core/external"
"github.com/navidrome/navidrome/model"
@@ -67,8 +68,16 @@ var _ = Describe("Provider - ArtistRadio", func() {
mockAgent.On("GetSimilarArtists", mock.Anything, "artist-1", "Artist One", "", 15).
Return(similarAgentsResp, nil).Once()
// Mock the three-phase artist lookup: ID (skipped - no IDs), MBID, then Name
// MBID lookup returns empty (no match)
artistRepo.On("GetAll", mock.MatchedBy(func(opt model.QueryOptions) bool {
return opt.Max == 0 && opt.Filters != nil
_, ok := opt.Filters.(squirrel.Eq)
return opt.Max == 0 && ok
})).Return(model.Artists{}, nil).Once()
// Name lookup returns the similar artist
artistRepo.On("GetAll", mock.MatchedBy(func(opt model.QueryOptions) bool {
_, ok := opt.Filters.(squirrel.Or)
return opt.Max == 0 && ok
})).Return(model.Artists{similarArtist}, nil).Once()
mockAgent.On("GetArtistTopSongs", mock.Anything, "artist-1", "Artist One", "", mock.Anything).
+56
View File
@@ -271,4 +271,60 @@ var _ = Describe("Provider - TopSongs", func() {
ag.AssertExpectations(GinkgoT())
mediaFileRepo.AssertExpectations(GinkgoT())
})
It("matches songs by ID first when agent provides IDs", func() {
// Mock finding the artist
artist1 := model.Artist{ID: "artist-1", Name: "Artist One", MbzArtistID: "mbid-artist-1"}
artistRepo.On("GetAll", mock.AnythingOfType("model.QueryOptions")).Return(model.Artists{artist1}, nil).Once()
// Mock agent response with IDs provided (highest priority matching)
// Note: Songs have no MBID to ensure only ID matching is used
agentSongs := []agents.Song{
{ID: "song-1", Name: "Song One"},
{ID: "song-2", Name: "Song Two"},
}
ag.On("GetArtistTopSongs", ctx, "artist-1", "Artist One", "mbid-artist-1", 2).Return(agentSongs, nil).Once()
// Mock ID lookup (first query - should match both songs directly)
song1 := model.MediaFile{ID: "song-1", Title: "Song One", ArtistID: "artist-1"}
song2 := model.MediaFile{ID: "song-2", Title: "Song Two", ArtistID: "artist-1"}
mediaFileRepo.On("GetAll", mock.AnythingOfType("model.QueryOptions")).Return(model.MediaFiles{song1, song2}, nil).Once()
songs, err := p.TopSongs(ctx, "Artist One", 2)
Expect(err).ToNot(HaveOccurred())
Expect(songs).To(HaveLen(2))
Expect(songs[0].ID).To(Equal("song-1"))
Expect(songs[1].ID).To(Equal("song-2"))
artistRepo.AssertExpectations(GinkgoT())
ag.AssertExpectations(GinkgoT())
mediaFileRepo.AssertExpectations(GinkgoT())
})
It("falls back to MBID when ID is not found", func() {
// Mock finding the artist
artist1 := model.Artist{ID: "artist-1", Name: "Artist One", MbzArtistID: "mbid-artist-1"}
artistRepo.On("GetAll", mock.AnythingOfType("model.QueryOptions")).Return(model.Artists{artist1}, nil).Once()
// Mock agent response with ID that won't be found, but MBID that will
agentSongs := []agents.Song{
{ID: "non-existent-id", Name: "Song One", MBID: "mbid-song-1"},
}
ag.On("GetArtistTopSongs", ctx, "artist-1", "Artist One", "mbid-artist-1", 1).Return(agentSongs, nil).Once()
// Mock ID lookup - returns empty (ID not found)
mediaFileRepo.On("GetAll", mock.AnythingOfType("model.QueryOptions")).Return(model.MediaFiles{}, nil).Once()
// Mock MBID lookup - finds the song
song1 := model.MediaFile{ID: "song-1", Title: "Song One", ArtistID: "artist-1", MbzRecordingID: "mbid-song-1"}
mediaFileRepo.On("GetAll", mock.AnythingOfType("model.QueryOptions")).Return(model.MediaFiles{song1}, nil).Once()
songs, err := p.TopSongs(ctx, "Artist One", 1)
Expect(err).ToNot(HaveOccurred())
Expect(songs).To(HaveLen(1))
Expect(songs[0].ID).To(Equal("song-1"))
artistRepo.AssertExpectations(GinkgoT())
ag.AssertExpectations(GinkgoT())
mediaFileRepo.AssertExpectations(GinkgoT())
})
})
+84
View File
@@ -226,4 +226,88 @@ var _ = Describe("Provider - UpdateArtistInfo", func() {
Expect(updatedArtist.ID).To(Equal("ar-agent-fail"))
ag.AssertExpectations(GinkgoT())
})
It("matches similar artists by ID first when agent provides IDs", func() {
originalArtist := &model.Artist{
ID: "ar-id-match",
Name: "ID Match Artist",
}
similarByID := model.Artist{ID: "ar-similar-by-id", Name: "Similar By ID", MbzArtistID: "mbid-similar"}
mockArtistRepo.SetData(model.Artists{*originalArtist, similarByID})
// Agent returns similar artist with ID (highest priority matching)
rawSimilar := []agents.Artist{
{ID: "ar-similar-by-id", Name: "Different Name", MBID: "different-mbid"},
}
ag.On("GetArtistMBID", ctx, "ar-id-match", "ID Match Artist").Return("", nil).Once()
ag.On("GetArtistImages", ctx, "ar-id-match", "ID Match Artist", mock.Anything).Return(nil, nil).Maybe()
ag.On("GetArtistBiography", ctx, "ar-id-match", "ID Match Artist", mock.Anything).Return("", nil).Maybe()
ag.On("GetArtistURL", ctx, "ar-id-match", "ID Match Artist", mock.Anything).Return("", nil).Maybe()
ag.On("GetSimilarArtists", ctx, "ar-id-match", "ID Match Artist", mock.Anything, 100).Return(rawSimilar, nil).Once()
updatedArtist, err := p.UpdateArtistInfo(ctx, "ar-id-match", 10, false)
Expect(err).NotTo(HaveOccurred())
Expect(updatedArtist.SimilarArtists).To(HaveLen(1))
// Should match by ID, not by name or MBID
Expect(updatedArtist.SimilarArtists[0].ID).To(Equal("ar-similar-by-id"))
Expect(updatedArtist.SimilarArtists[0].Name).To(Equal("Similar By ID"))
})
It("matches similar artists by MBID when ID is empty", func() {
originalArtist := &model.Artist{
ID: "ar-mbid-match",
Name: "MBID Match Artist",
}
similarByMBID := model.Artist{ID: "ar-similar-by-mbid", Name: "Similar By MBID", MbzArtistID: "mbid-similar"}
mockArtistRepo.SetData(model.Artists{*originalArtist, similarByMBID})
// Agent returns similar artist with only MBID (no ID)
rawSimilar := []agents.Artist{
{Name: "Different Name", MBID: "mbid-similar"},
}
ag.On("GetArtistMBID", ctx, "ar-mbid-match", "MBID Match Artist").Return("", nil).Once()
ag.On("GetArtistImages", ctx, "ar-mbid-match", "MBID Match Artist", mock.Anything).Return(nil, nil).Maybe()
ag.On("GetArtistBiography", ctx, "ar-mbid-match", "MBID Match Artist", mock.Anything).Return("", nil).Maybe()
ag.On("GetArtistURL", ctx, "ar-mbid-match", "MBID Match Artist", mock.Anything).Return("", nil).Maybe()
ag.On("GetSimilarArtists", ctx, "ar-mbid-match", "MBID Match Artist", mock.Anything, 100).Return(rawSimilar, nil).Once()
updatedArtist, err := p.UpdateArtistInfo(ctx, "ar-mbid-match", 10, false)
Expect(err).NotTo(HaveOccurred())
Expect(updatedArtist.SimilarArtists).To(HaveLen(1))
// Should match by MBID since ID was empty
Expect(updatedArtist.SimilarArtists[0].ID).To(Equal("ar-similar-by-mbid"))
Expect(updatedArtist.SimilarArtists[0].Name).To(Equal("Similar By MBID"))
})
It("falls back to name matching when ID and MBID don't match", func() {
originalArtist := &model.Artist{
ID: "ar-name-match",
Name: "Name Match Artist",
}
similarByName := model.Artist{ID: "ar-similar-by-name", Name: "Similar By Name"}
mockArtistRepo.SetData(model.Artists{*originalArtist, similarByName})
// Agent returns similar artist with non-matching ID and MBID
rawSimilar := []agents.Artist{
{ID: "non-existent-id", Name: "Similar By Name", MBID: "non-existent-mbid"},
}
ag.On("GetArtistMBID", ctx, "ar-name-match", "Name Match Artist").Return("", nil).Once()
ag.On("GetArtistImages", ctx, "ar-name-match", "Name Match Artist", mock.Anything).Return(nil, nil).Maybe()
ag.On("GetArtistBiography", ctx, "ar-name-match", "Name Match Artist", mock.Anything).Return("", nil).Maybe()
ag.On("GetArtistURL", ctx, "ar-name-match", "Name Match Artist", mock.Anything).Return("", nil).Maybe()
ag.On("GetSimilarArtists", ctx, "ar-name-match", "Name Match Artist", mock.Anything, 100).Return(rawSimilar, nil).Once()
updatedArtist, err := p.UpdateArtistInfo(ctx, "ar-name-match", 10, false)
Expect(err).NotTo(HaveOccurred())
Expect(updatedArtist.SimilarArtists).To(HaveLen(1))
// Should fall back to name matching since ID and MBID didn't match
Expect(updatedArtist.SimilarArtists[0].ID).To(Equal("ar-similar-by-name"))
Expect(updatedArtist.SimilarArtists[0].Name).To(Equal("Similar By Name"))
})
})