fix(artwork): clamp the persisted blurhash version to the entity's

The read-side artwork version may over-approximate the served sources (folder
parents for disc layouts), which with omission semantics suppresses a perfectly
valid stored hash. At persist time the hash is fresh for the served bytes by
construction, so the write now loads the entity and clamps blur_hash_updated_at
up to its current ArtworkUpdatedAt: after any serve the DTO accepts the stored
hash, and every omission window closes regardless of read-side precision. The
in-memory state shrinks to a pure decode cache (checksum -> hash); staleness
decisions moved to the row read, which also lets a drifted stored value be
restored from the served bytes.

core/artwork/blurhash_updater.go

@@ -15,14 +15,14 @@ import (
 	"github.com/navidrome/navidrome/log"
 	"github.com/navidrome/navidrome/model"
 	"github.com/navidrome/navidrome/resources"
+	"github.com/navidrome/navidrome/utils"
 )
 
-// blurHashState remembers what was last persisted for an artwork, keyed by a checksum of the served
-// bytes, so repeated serves of the same image skip the decode and the write entirely.
+// blurHashState is a decode cache: the hash last computed for an artwork's served bytes, keyed by
+// their checksum, so repeated serves of the same image skip the decode.
 type blurHashState struct {
-	sum     uint64
-	version time.Time
-	hash    string
+	sum  uint64
+	hash string
 }
 
 // blurHashUpdater keeps stored blurhashes in sync with the bytes actually served. It runs inline in
@@ -55,8 +55,7 @@ func (u *blurHashUpdater) update(ctx context.Context, artID model.ArtworkID, dat
 			log.Error(ctx, "BlurHash: recovered from panic", "artID", artID, "panic", r)
 		}
 	}()
-	// ArtworkID embeds the client token's LastUpdate; without zeroing it the seen key would rotate on
-	// every scan bump, defeating the same-bytes dedup and stranding stale entries forever.
+	// ArtworkID embeds the client token's LastUpdate; zero it so the decode cache keys by identity.
 	artID.LastUpdate = time.Time{}
 	// The response is already written when the tee fires; a client abort must not lose the write.
 	ctx = context.WithoutCancel(ctx)
@@ -65,31 +64,36 @@ func (u *blurHashUpdater) update(ctx context.Context, artID model.ArtworkID, dat
 		return
 	}
 	sum := checksum(data)
-	u.mutex.Lock()
-	prev, ok := u.seen[artID]
-	u.mutex.Unlock()
-	if ok && prev.hash != "" && prev.sum == sum {
-		if !version.After(prev.version) {
+	hash := u.cachedHash(artID, sum)
+	if hash == "" {
+		img, _, err := image.Decode(bytes.NewReader(data))
+		if err != nil {
+			// Undecodable served bytes are not proof of change; leave the stored hash intact.
+			log.Trace(ctx, "BlurHash: served bytes not decodable, keeping stored hash", "artID", artID, err)
+			return
+		}
+		b := img.Bounds()
+		x, y := blurhash.Components(b.Dx(), b.Dy())
+		if hash, err = blurhash.Encode(img, x, y); err != nil || hash == "" {
 			return
 		}
-		// Same bytes under a newer artwork version: re-persist so blur_hash_updated_at keeps pace with
-		// the entity version, or the DTO staleness gate would emit the fake after any routine scan.
-		u.persistAndRemember(ctx, artID, prev.hash, sum, version)
-		return
 	}
-	img, _, err := image.Decode(bytes.NewReader(data))
+	stored, storedAt, entityVersion, err := u.loadState(ctx, artID)
 	if err != nil {
-		// Undecodable served bytes are not proof of change; leave the stored hash intact.
-		log.Trace(ctx, "BlurHash: served bytes not decodable, keeping stored hash", "artID", artID, err)
 		return
 	}
-	b := img.Bounds()
-	x, y := blurhash.Components(b.Dx(), b.Dy())
-	hash, err := blurhash.Encode(img, x, y)
-	if err != nil || hash == "" {
+	if stored == hash && storedAt != nil && !storedAt.Before(entityVersion) {
+		u.remember(artID, blurHashState{sum: sum, hash: hash})
 		return
 	}
-	u.persistAndRemember(ctx, artID, hash, sum, version)
+	// Clamp the persisted version up to the entity's: the hash is fresh for what is served right now,
+	// so the DTO accepts it after any serve, however much the read-side version over-approximates.
+	target := capAtNow(utils.TimeNewest(version, entityVersion))
+	if err := u.persist(ctx, artID, hash, target); err != nil {
+		log.Warn(ctx, "BlurHash: error persisting", "artID", artID, err)
+		return
+	}
+	u.remember(artID, blurHashState{sum: sum, hash: hash})
 }
 
 // clearIfStored clears the persisted hash after a placeholder was served (a cold map costs one row
@@ -107,7 +111,7 @@ func (u *blurHashUpdater) clearIfStored(ctx context.Context, artID model.Artwork
 		return
 	}
 	if !ok {
-		stored, err := u.loadStoredHash(ctx, artID)
+		stored, _, _, err := u.loadState(ctx, artID)
 		if err != nil {
 			return
 		}
@@ -123,12 +127,14 @@ func (u *blurHashUpdater) clearIfStored(ctx context.Context, artID model.Artwork
 	u.remember(artID, blurHashState{})
 }
 
-func (u *blurHashUpdater) persistAndRemember(ctx context.Context, artID model.ArtworkID, hash string, sum uint64, version time.Time) {
-	if err := u.persist(ctx, artID, hash, version); err != nil {
-		log.Warn(ctx, "BlurHash: error persisting", "artID", artID, err)
-		return
+// cachedHash returns the previously computed hash when the served bytes are unchanged.
+func (u *blurHashUpdater) cachedHash(artID model.ArtworkID, sum uint64) string {
+	u.mutex.Lock()
+	defer u.mutex.Unlock()
+	if prev, ok := u.seen[artID]; ok && prev.hash != "" && prev.sum == sum {
+		return prev.hash
 	}
-	u.remember(artID, blurHashState{sum: sum, version: version, hash: hash})
+	return ""
 }
 
 func (u *blurHashUpdater) remember(artID model.ArtworkID, s blurHashState) {
@@ -143,28 +149,28 @@ func checksum(data []byte) uint64 {
 	return h.Sum64()
 }
 
-func (u *blurHashUpdater) loadStoredHash(ctx context.Context, artID model.ArtworkID) (string, error) {
+func (u *blurHashUpdater) loadState(ctx context.Context, artID model.ArtworkID) (string, *time.Time, time.Time, error) {
 	switch artID.Kind {
 	case model.KindAlbumArtwork:
 		al, err := u.ds.Album(ctx).Get(artID.ID)
 		if err != nil {
-			return "", err
+			return "", nil, time.Time{}, err
 		}
-		return al.BlurHash, nil
+		return al.BlurHash, al.BlurHashUpdatedAt, al.ArtworkUpdatedAt(), nil
 	case model.KindArtistArtwork:
 		ar, err := u.ds.Artist(ctx).Get(artID.ID)
 		if err != nil {
-			return "", err
+			return "", nil, time.Time{}, err
 		}
-		return ar.BlurHash, nil
+		return ar.BlurHash, ar.BlurHashUpdatedAt, ar.ArtworkUpdatedAt(), nil
 	case model.KindPlaylistArtwork:
 		pl, err := u.ds.Playlist(ctx).Get(artID.ID)
 		if err != nil {
-			return "", err
+			return "", nil, time.Time{}, err
 		}
-		return pl.BlurHash, nil
+		return pl.BlurHash, pl.BlurHashUpdatedAt, pl.ArtworkUpdatedAt(), nil
 	}
-	return "", model.ErrNotFound
+	return "", nil, time.Time{}, model.ErrNotFound
 }
 
 // isPlaceholder byte-compares against the embedded placeholder assets: placeholder artwork must never

core/artwork/blurhash_updater_internal_test.go

@@ -79,30 +79,42 @@ var _ = Describe("blurHashUpdater", func() {
 		Expect(stored("al-1").BlurHash).To(Equal("KEEP"))
 	})
 
-	It("skips the write when the same bytes are served again under the same version", func() {
+	It("does not rewrite when the stored hash is current for the entity version", func() {
 		id := album(model.Album{ID: "al-1", UpdatedAt: version})
 		data := realPNGBytes("dedup")
 		u.update(GinkgoT().Context(), id, data, version)
-		// Tamper with the stored value: a second identical serve must not touch the row.
-		Expect(repo.UpdateBlurHash("al-1", "TAMPERED", version)).To(Succeed())
-		u.update(GinkgoT().Context(), id, data, version)
-		Expect(stored("al-1").BlurHash).To(Equal("TAMPERED"))
+		first := stored("al-1")
+		// A later serve of the same bytes (newer tee version, unchanged entity) must not move the row.
+		u.update(GinkgoT().Context(), id, data, version.Add(time.Hour))
+		Expect(stored("al-1").BlurHashUpdatedAt).To(HaveValue(Equal(*first.BlurHashUpdatedAt)))
 	})
 
-	It("re-persists the same hash when the artwork version advances", func() {
-		// A scan can bump the entity version without changing the cover; blur_hash_updated_at must
-		// follow, or the DTO's staleness gate would emit the fake hash forever after.
+	It("clamps the persisted version up to the entity's artwork version", func() {
+		// The read-side version may over-approximate (folder parents); after a serve the hash is fresh
+		// by construction, so the write clamps up and the DTO accepts it — omission windows close.
 		id := album(model.Album{ID: "al-1", UpdatedAt: version})
-		data := realPNGBytes("same-bytes")
+		data := realPNGBytes("clamp")
 		u.update(GinkgoT().Context(), id, data, version)
 		first := stored("al-1")
+
 		newer := version.Add(time.Hour)
-		u.update(GinkgoT().Context(), id, data, newer)
+		album(model.Album{ID: "al-1", UpdatedAt: newer, BlurHash: first.BlurHash, BlurHashUpdatedAt: first.BlurHashUpdatedAt})
+		u.update(GinkgoT().Context(), id, data, version) // same bytes, old tee version
 		second := stored("al-1")
 		Expect(second.BlurHash).To(Equal(first.BlurHash))
 		Expect(second.BlurHashUpdatedAt).To(HaveValue(Equal(newer)))
 	})
 
+	It("restores the stored hash when it drifts from the served bytes", func() {
+		id := album(model.Album{ID: "al-1", UpdatedAt: version})
+		data := realPNGBytes("truth")
+		u.update(GinkgoT().Context(), id, data, version)
+		truth := stored("al-1").BlurHash
+		Expect(repo.UpdateBlurHash("al-1", "DRIFTED", version)).To(Succeed())
+		u.update(GinkgoT().Context(), id, data, version)
+		Expect(stored("al-1").BlurHash).To(Equal(truth))
+	})
+
 	It("does not write when a placeholder is served and nothing was ever stored", func() {
 		id := album(model.Album{ID: "al-1", UpdatedAt: version})
 		u.update(GinkgoT().Context(), id, placeholderImages()[0], version)
@@ -116,17 +128,13 @@ var _ = Describe("blurHashUpdater", func() {
 		Expect(u.seen).To(BeEmpty())
 	})
 
-	It("dedups across artwork ids that differ only in their embedded timestamp", func() {
-		// Client coverArt tokens embed a LastUpdate; the seen key must ignore it, or every scan bump
-		// would defeat the dedup and re-decode identical bytes.
+	It("keys the decode cache by identity, ignoring the artwork id's embedded timestamp", func() {
 		id := album(model.Album{ID: "al-1", UpdatedAt: version})
 		data := realPNGBytes("dedup")
 		u.update(GinkgoT().Context(), id, data, version)
-		Expect(repo.UpdateBlurHash("al-1", "TAMPERED", version)).To(Succeed())
 		bumped := id
 		bumped.LastUpdate = version.Add(time.Hour)
 		u.update(GinkgoT().Context(), bumped, data, version)
-		Expect(stored("al-1").BlurHash).To(Equal("TAMPERED"))
 		Expect(u.seen).To(HaveLen(1))
 	})
 })