From 65b24f63a213f456407c0db814ada4adef20606e Mon Sep 17 00:00:00 2001 From: Deluan Date: Tue, 1 Sep 2026 17:34:00 -0400 Subject: [PATCH] feat(artwork): report which absent states gave up An absent state is now terminal, so the operator needs to know how each one was reached. "Every source answered, there is no image" and "the retry budget ran out" both land in the same row, and only the second is worth a reprocess. artwork status splits the absent total by whether last_failure is set, which recordGiveUp writes only on exhaustion and a later success clears. Counted on absent rows alone. An item that kept its previous art through a failure is not missing artwork, and folding it in would have doubled the artist figure on a test library with a flaky provider. --- cmd/artwork.go | 13 ++++++++--- cmd/artwork_test.go | 8 ++++++- model/artwork.go | 3 +++ persistence/artwork_queue_repository.go | 7 ++++++ persistence/artwork_queue_repository_test.go | 23 ++++++++++++++++++++ tests/mock_artwork_queue_repo.go | 15 +++++++++++++ 6 files changed, 65 insertions(+), 4 deletions(-) diff --git a/cmd/artwork.go b/cmd/artwork.go index 2fb696ca4..72291460c 100644 --- a/cmd/artwork.go +++ b/cmd/artwork.go @@ -149,6 +149,8 @@ type sourceCount struct { type absentCount struct { kind model.Kind count int64 + // afterFailure is the subset that gave up rather than being told "no image". + afterFailure int64 } type statusReport struct { @@ -192,7 +194,11 @@ func collectStatus(ctx context.Context, ds model.DataStore) (statusReport, error rep.sources = append(rep.sources, sourceCount{kind: k, source: s, count: n}) // An absent state is exactly a row with no source, so it needs no second query. if s == "" { - rep.absent = append(rep.absent, absentCount{kind: k, count: n}) + failed, err := q.CountAbsentAfterFailure(k) + if err != nil { + return rep, fmt.Errorf("counting absent %s artwork that gave up: %w", k, err) + } + rep.absent = append(rep.absent, absentCount{kind: k, count: n, afterFailure: failed}) } } } @@ -222,11 +228,12 @@ func formatStatus(rep statusReport) string { } fmt.Fprintln(w, "\nAbsent (resolved, no image found)") - fmt.Fprintln(w, " KIND\tABSENT") + fmt.Fprintln(w, " KIND\tABSENT\tGAVE UP") for _, a := range rep.absent { - fmt.Fprintf(w, " %s\t%d\n", a.kind, a.count) + fmt.Fprintf(w, " %s\t%d\t%d\n", a.kind, a.count, a.afterFailure) } fmt.Fprintln(w, " (never retried on their own; run 'artwork reprocess --source absent')") + fmt.Fprintln(w, " (gave up: the retry budget ran out rather than a source answering; most likely to resolve)") fmt.Fprintln(w, "\nConfig") fmt.Fprintf(w, " State:\t%s\n", configState(rep)) diff --git a/cmd/artwork_test.go b/cmd/artwork_test.go index b24fd5d06..4bee2f203 100644 --- a/cmd/artwork_test.go +++ b/cmd/artwork_test.go @@ -862,7 +862,7 @@ var _ = Describe("formatStatus", func() { {kind: model.KindArtistArtwork, source: "", count: 2}, }, absent: []absentCount{ - {kind: model.KindArtistArtwork, count: 2}, + {kind: model.KindArtistArtwork, count: 2, afterFailure: 1}, }, inputs: []artwork.FingerprintInput{{Name: "Agents", Value: "deezer,lastfm"}}, stored: "abc123", @@ -895,6 +895,12 @@ var _ = Describe("formatStatus", func() { Expect(sources).To(MatchRegexp(`artist\s+absent\s+2`)) }) + It("splits the absent total by how it was reached", func() { + out := block(formatStatus(rep), "Absent (resolved, no image found)") + Expect(out).To(MatchRegexp(`artist\s+2\s+1`), "2 absent, of which 1 gave up") + Expect(formatStatus(rep)).To(ContainSubstring("the retry budget ran out")) + }) + It("says absent states are never retried on their own, and names the command that does", func() { Expect(formatStatus(rep)).To(ContainSubstring("never retried on their own")) Expect(formatStatus(rep)).To(ContainSubstring("artwork reprocess --source absent")) diff --git a/model/artwork.go b/model/artwork.go index d4413ec9c..c6bfadf1d 100644 --- a/model/artwork.go +++ b/model/artwork.go @@ -146,6 +146,9 @@ type ArtworkQueueRepository interface { CountBySource(kind Kind, sources []string) (int64, error) // SourcesInUse lists the distinct sources items of a kind currently resolve from, "" included. SourcesInUse(kind Kind) ([]string, error) + // CountAbsentAfterFailure reports absent states a kind recorded because the retry budget ran + // out, not because every source answered. Those are the ones a reprocess is likely to resolve. + CountAbsentAfterFailure(kind Kind) (int64, error) // EnqueueBySource inserts queue rows for items of a kind whose current source matches. // It does not clear existing artwork state: the current image stays until it is replaced. EnqueueBySource(kind Kind, sources []string, priority int) (int64, error) diff --git a/persistence/artwork_queue_repository.go b/persistence/artwork_queue_repository.go index 3ad368772..2889bddaa 100644 --- a/persistence/artwork_queue_repository.go +++ b/persistence/artwork_queue_repository.go @@ -223,4 +223,11 @@ func (r *artworkQueueRepository) Count() (int64, error) { return res.Count, err } +func (r *artworkQueueRepository) CountAbsentAfterFailure(kind model.Kind) (int64, error) { + var res struct{ Count int64 } + err := r.queryOne(Select("count(*) as count").From(itemArtworkTable). + Where(And{Eq{"item_kind": kind.Prefix(), "hash": ""}, NotEq{"last_failure": ""}}), &res) + return res.Count, err +} + var _ model.ArtworkQueueRepository = (*artworkQueueRepository)(nil) diff --git a/persistence/artwork_queue_repository_test.go b/persistence/artwork_queue_repository_test.go index a4a1ea76f..2b936eca2 100644 --- a/persistence/artwork_queue_repository_test.go +++ b/persistence/artwork_queue_repository_test.go @@ -400,6 +400,29 @@ var _ = Describe("ArtworkQueueRepository", func() { )) }) + It("counts only the absent states that gave up, not those a source answered", func() { + awRepo := NewArtworkRepository(context.Background(), GetDBXBuilder()) + for _, ia := range []model.ItemArtwork{ + {ItemKind: "ar", ItemID: "gaveup", ImageType: model.ImageTypePrimary, LastFailure: "[]"}, + {ItemKind: "ar", ItemID: "toldno", ImageType: model.ImageTypePrimary}, + {ItemKind: "ar", ItemID: "hasart", ImageType: model.ImageTypePrimary, Hash: "hX", LastFailure: "[]"}, + } { + Expect(awRepo.PutItemArtwork(&ia)).To(Succeed()) + } + + Expect(repo.CountAbsentAfterFailure(model.KindArtistArtwork)).To(Equal(int64(1)), + "an item still serving art is not absent, however its last attempt went") + + // A later success rewrites the row, clearing the record. + Expect(awRepo.PutItemArtwork(&model.ItemArtwork{ItemKind: "ar", ItemID: "gaveup", + ImageType: model.ImageTypePrimary, Hash: "hZ"})).To(Succeed()) + Expect(repo.CountAbsentAfterFailure(model.KindArtistArtwork)).To(Equal(int64(0))) + }) + + It("reports a kind that never gave up as zero", func() { + Expect(repo.CountAbsentAfterFailure(model.KindRadioArtwork)).To(Equal(int64(0))) + }) + It("reports an empty queue as no rows", func() { Expect(repo.CountQueued(nil, nil)).To(BeEmpty()) }) diff --git a/tests/mock_artwork_queue_repo.go b/tests/mock_artwork_queue_repo.go index a1b7b3e3d..f0e09d4cf 100644 --- a/tests/mock_artwork_queue_repo.go +++ b/tests/mock_artwork_queue_repo.go @@ -275,6 +275,21 @@ func (m *MockArtworkQueueRepo) CountBySource(kind model.Kind, sources []string) return int64(len(m.matchingSource(kind, sources))), nil } +func (m *MockArtworkQueueRepo) CountAbsentAfterFailure(kind model.Kind) (int64, error) { + m.mu.Lock() + defer m.mu.Unlock() + if m.Err != nil || m.ItemArtworkSource == nil { + return 0, m.Err + } + var n int64 + for _, ia := range m.ItemArtworkSource.ItemData { + if ia.ItemKind == kind.Prefix() && ia.Hash == "" && ia.LastFailure != "" { + n++ + } + } + return n, nil +} + func (m *MockArtworkQueueRepo) SourcesInUse(kind model.Kind) ([]string, error) { m.mu.Lock() defer m.mu.Unlock()