From 9ea4e22ea016a2ba2d8829035da196b6ef687f7c Mon Sep 17 00:00:00 2001
From: Deluan <deluan@navidrome.org>
Date: Fri, 17 Jul 2026 11:13:59 -0400
Subject: [PATCH] fix(artwork): address PR review bot feedback

- don't record noResult for timed-out/cancelled computations (transient
  failures must not suppress retries for the same artwork version)
- bound the noResult negative cache at 25k entries
- use context.Background for the worker's root context
- add hash:"ignore" to Artist/Playlist blurhash fields for consistency
  with Album (inert today; neither struct is hashed)
---
 core/artwork/blurhash_updater.go | 14 ++++++++++++--
 model/artist.go                  |  4 ++--
 model/playlist.go                |  4 ++--
 3 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/core/artwork/blurhash_updater.go b/core/artwork/blurhash_updater.go
index fee2d388d..9f93b628d 100644
--- a/core/artwork/blurhash_updater.go
+++ b/core/artwork/blurhash_updater.go
@@ -44,7 +44,7 @@ func (u *blurHashUpdater) Enqueue(artID model.ArtworkID, force bool) {
 	u.start.Do(func() {
 		// Playlist artwork readers require a user in the context. Like the cacheWarmer, the worker
 		// lives for the rest of the process; lazy-starting keeps idle Artwork instances goroutine-free.
-		go u.run(request.WithUser(context.TODO(), model.User{IsAdmin: true}))
+		go u.run(request.WithUser(context.Background(), model.User{IsAdmin: true}))
 	})
 	u.mutex.Lock()
 	u.buffer[artID] = u.buffer[artID] || force
@@ -106,7 +106,10 @@ func (u *blurHashUpdater) process(ctx context.Context, artID model.ArtworkID, fo
 	hash, err := u.computeFromArtwork(ctx, artID)
 	if err != nil || hash == "" {
 		log.Trace(ctx, "BlurHash: nothing to persist", "artID", artID, err)
-		u.setNoResult(artID, version)
+		// A timed-out/cancelled attempt is transient — don't suppress future retries for it.
+		if ctx.Err() == nil {
+			u.setNoResult(artID, version)
+		}
 		return
 	}
 	if err := u.persist(ctx, artID, hash, version); err != nil {
@@ -123,9 +126,16 @@ func (u *blurHashUpdater) lastNoResult(artID model.ArtworkID) (time.Time, bool)
 	return t, ok
 }
 
+// maxNoResultEntries bounds the negative cache; entries only accumulate for artwork-less entities,
+// so a wholesale reset just costs those entities one extra verification pass each.
+const maxNoResultEntries = 25_000
+
 func (u *blurHashUpdater) setNoResult(artID model.ArtworkID, version time.Time) {
 	u.mutex.Lock()
 	defer u.mutex.Unlock()
+	if len(u.noResult) >= maxNoResultEntries {
+		clear(u.noResult)
+	}
 	u.noResult[artID] = version
 }
 
diff --git a/model/artist.go b/model/artist.go
index bbb27c3a8..de7cd53be 100644
--- a/model/artist.go
+++ b/model/artist.go
@@ -42,8 +42,8 @@ type Artist struct {
 	CreatedAt *time.Time `structs:"created_at" json:"createdAt,omitempty"`
 	UpdatedAt *time.Time `structs:"updated_at" json:"updatedAt,omitempty"`
 
-	BlurHash          string     `structs:"blur_hash" json:"blurHash,omitempty"`
-	BlurHashUpdatedAt *time.Time `structs:"blur_hash_updated_at" json:"-"`
+	BlurHash          string     `structs:"blur_hash" json:"blurHash,omitempty" hash:"ignore"`
+	BlurHashUpdatedAt *time.Time `structs:"blur_hash_updated_at" json:"-" hash:"ignore"`
 }
 
 type ArtistStats struct {
diff --git a/model/playlist.go b/model/playlist.go
index fde7b2ae4..8970e27e2 100644
--- a/model/playlist.go
+++ b/model/playlist.go
@@ -31,8 +31,8 @@ type Playlist struct {
 	CreatedAt        time.Time      `structs:"created_at" json:"createdAt"`
 	UpdatedAt        time.Time      `structs:"updated_at" json:"updatedAt"`
 
-	BlurHash          string     `structs:"blur_hash" json:"blurHash,omitempty"`
-	BlurHashUpdatedAt *time.Time `structs:"blur_hash_updated_at" json:"-"`
+	BlurHash          string     `structs:"blur_hash" json:"blurHash,omitempty" hash:"ignore"`
+	BlurHashUpdatedAt *time.Time `structs:"blur_hash_updated_at" json:"-" hash:"ignore"`
 
 	// SmartPlaylist attributes
 	Rules       *criteria.Criteria `structs:"rules" json:"rules"`