refactor(artwork): export the trace types directly

ChainTrace and TraceStep were unexported types re-exported through aliases,
which existed only so the CLI had a name to refer to them by. The types are
public API — Resolver.Steps returns []TraceStep and the CLI constructs a
ChainTrace — so name them that way and drop the indirection.

Encapsulation is unchanged: add, mu and steps stay unexported, so only this
package can write a step.
This commit is contained in:
Deluan 2026-08-14 15:07:29 -04:00
commit 8e1ed55ca6
6 changed files with 57 additions and 60 deletions

View file

@ -47,13 +47,13 @@ func fetchArtistImage(ctx context.Context, ag *agents.Agents, gate gateFunc, ar
// Synthetic artists would otherwise get an unrelated agent result assigned to them.
switch ar.ID {
case consts.UnknownArtistID, consts.VariousArtistsID:
traceFrom(ctx).add(traceStep{Candidate: ExternalCandidate, Outcome: OutcomeSkipped, Detail: "synthetic artist"})
traceFrom(ctx).add(TraceStep{Candidate: ExternalCandidate, Outcome: OutcomeSkipped, Detail: "synthetic artist"})
return nil, "", false
}
name := externalName(ar.Name)
imageAgents := ag.ArtistImageAgents()
if len(imageAgents) == 0 {
traceFrom(ctx).add(traceStep{Candidate: ExternalCandidate, Outcome: OutcomeSkipped,
traceFrom(ctx).add(TraceStep{Candidate: ExternalCandidate, Outcome: OutcomeSkipped,
Detail: "no enabled agent provides artist images"})
return nil, "", false
}
@ -85,7 +85,7 @@ func fetchAlbumImage(ctx context.Context, ag *agents.Agents, gate gateFunc, al m
name, artist := externalName(al.Name), externalName(al.AlbumArtist)
imageAgents := ag.AlbumImageAgents()
if len(imageAgents) == 0 {
traceFrom(ctx).add(traceStep{Candidate: ExternalCandidate, Outcome: OutcomeSkipped,
traceFrom(ctx).add(TraceStep{Candidate: ExternalCandidate, Outcome: OutcomeSkipped,
Detail: "no enabled agent provides album images"})
return nil, "", false
}

View file

@ -175,12 +175,12 @@ var _ = Describe("agent images", func() {
It("records a skipped external candidate when no agent provides artist images", func() {
ag := imageAgents()
t := &chainTrace{}
t := &ChainTrace{}
r, _, extErr := fetchArtistImage(withTrace(ctx, t), ag, passthroughGate, model.Artist{ID: "ar1"})
Expect(r).To(BeNil())
Expect(extErr).To(BeFalse())
Expect(t.Steps()).To(Equal([]traceStep{{Candidate: "external", Outcome: OutcomeSkipped,
Expect(t.Steps()).To(Equal([]TraceStep{{Candidate: "external", Outcome: OutcomeSkipped,
Detail: "no enabled agent provides artist images"}}),
"a configured external token must never be silently absent from the chain")
})
@ -188,7 +188,7 @@ var _ = Describe("agent images", func() {
It("records a skipped external candidate for synthetic artists", func() {
a := &fakeImageAgent{name: "agentA", imgs: []agents.ExternalImage{img("/a", 100)}}
ag := imageAgents(a)
t := &chainTrace{}
t := &ChainTrace{}
_, _, _ = fetchArtistImage(withTrace(ctx, t), ag, passthroughGate,
model.Artist{ID: consts.VariousArtistsID, Name: "Various Artists"})
@ -257,12 +257,12 @@ var _ = Describe("agent images", func() {
It("records a skipped external candidate when no agent provides album images", func() {
ag := imageAgents()
t := &chainTrace{}
t := &ChainTrace{}
r, _, extErr := fetchAlbumImage(withTrace(ctx, t), ag, passthroughGate, model.Album{Name: "Album"})
Expect(r).To(BeNil())
Expect(extErr).To(BeFalse())
Expect(t.Steps()).To(Equal([]traceStep{{Candidate: "external", Outcome: OutcomeSkipped,
Expect(t.Steps()).To(Equal([]TraceStep{{Candidate: "external", Outcome: OutcomeSkipped,
Detail: "no enabled agent provides album images"}}),
"a configured external token must never be silently absent from the chain")
})

View file

@ -387,9 +387,6 @@ func (s *service) parseArtworkID(ctx context.Context, id string) (model.ArtworkI
return model.ArtworkID{}, model.ErrNotFound
}
type ChainTrace = chainTrace
type TraceStep = traceStep
// Resolver is the CLI's read-only view of resolution: it walks the priority chain, records
// the walk and reports the winning source, without ever writing artwork state.
type Resolver struct {

View file

@ -36,7 +36,7 @@ type resolution struct {
// transient external failure still retries; localErr is dropped, as the scanner re-lists changes.
type chainState struct {
extErr, localErr bool
trace *chainTrace // nil unless the CLI asked for a trace
trace *ChainTrace // nil unless the CLI asked for a trace
}
// try stamps the accumulated external failure onto a hit, and records the miss otherwise.
@ -56,7 +56,7 @@ func (c *chainState) try(candidate string, res resolution, ok bool) (resolution,
}
func (c *chainState) record(candidate string, out Outcome, detail string) {
c.trace.add(traceStep{Candidate: candidate, Outcome: out, Detail: detail})
c.trace.add(TraceStep{Candidate: candidate, Outcome: out, Detail: detail})
}
// exhausted is the outcome when no source in the chain yielded an image.

View file

@ -28,21 +28,21 @@ const (
ExternalPrefix = ExternalCandidate + ":"
)
// traceStep is one candidate the priority chain considered.
type traceStep struct {
// TraceStep is one candidate the priority chain considered.
type TraceStep struct {
Candidate string
Outcome Outcome
Detail string
}
// chainTrace collects the walk of a single resolution. The artwork worker never attaches
// ChainTrace collects the walk of a single resolution. The artwork worker never attaches
// one; only the CLI does, so resolution stays allocation-free in the hot path.
type chainTrace struct {
type ChainTrace struct {
mu sync.Mutex
steps []traceStep
steps []TraceStep
}
func (t *chainTrace) add(step traceStep) {
func (t *ChainTrace) add(step TraceStep) {
if t == nil {
return
}
@ -51,7 +51,7 @@ func (t *chainTrace) add(step traceStep) {
t.steps = append(t.steps, step)
}
func (t *chainTrace) Steps() []traceStep {
func (t *ChainTrace) Steps() []TraceStep {
if t == nil {
return nil
}
@ -62,29 +62,29 @@ func (t *chainTrace) Steps() []traceStep {
type traceCtxKey struct{}
func withTrace(ctx context.Context, t *chainTrace) context.Context {
func withTrace(ctx context.Context, t *ChainTrace) context.Context {
return context.WithValue(ctx, traceCtxKey{}, t)
}
func traceFrom(ctx context.Context) *chainTrace {
t, _ := ctx.Value(traceCtxKey{}).(*chainTrace)
func traceFrom(ctx context.Context) *ChainTrace {
t, _ := ctx.Value(traceCtxKey{}).(*ChainTrace)
return t
}
var errOfflineSkipped = errors.New("artwork: external lookup skipped (offline)")
// tracingGate records each external agent's outcome without changing what the gate returns.
func tracingGate(t *chainTrace, inner gateFunc) gateFunc {
func tracingGate(t *ChainTrace, inner gateFunc) gateFunc {
return func(name string, f func() (io.ReadCloser, string, error)) (io.ReadCloser, string, error) {
r, path, err := inner(name, f)
candidate := ExternalPrefix + name
switch {
case r != nil:
t.add(traceStep{Candidate: candidate, Outcome: OutcomeHit, Detail: path})
t.add(TraceStep{Candidate: candidate, Outcome: OutcomeHit, Detail: path})
case isTransientExternal(err):
t.add(traceStep{Candidate: candidate, Outcome: OutcomeError, Detail: err.Error()})
t.add(TraceStep{Candidate: candidate, Outcome: OutcomeError, Detail: err.Error()})
default:
t.add(traceStep{Candidate: candidate, Outcome: OutcomeMiss})
t.add(TraceStep{Candidate: candidate, Outcome: OutcomeMiss})
}
return r, path, err
}
@ -92,9 +92,9 @@ func tracingGate(t *chainTrace, inner gateFunc) gateFunc {
// offlineGate reports which agents would be asked without asking them, so a diagnostic
// command cannot add load to a provider that is already rate-limiting us.
func offlineGate(t *chainTrace) gateFunc {
func offlineGate(t *ChainTrace) gateFunc {
return func(name string, _ func() (io.ReadCloser, string, error)) (io.ReadCloser, string, error) {
t.add(traceStep{Candidate: ExternalPrefix + name, Outcome: OutcomeWouldTry})
t.add(TraceStep{Candidate: ExternalPrefix + name, Outcome: OutcomeWouldTry})
return nil, "", errOfflineSkipped
}
}

View file

@ -37,13 +37,13 @@ var _ = Describe("chainTrace", func() {
})
It("collects steps in order", func() {
t := &chainTrace{}
t := &ChainTrace{}
ctx := withTrace(context.Background(), t)
traceFrom(ctx).add(traceStep{Candidate: "cover.*", Outcome: OutcomeMiss})
traceFrom(ctx).add(traceStep{Candidate: "embedded", Outcome: OutcomeHit, Detail: "/music/a.flac"})
traceFrom(ctx).add(TraceStep{Candidate: "cover.*", Outcome: OutcomeMiss})
traceFrom(ctx).add(TraceStep{Candidate: "embedded", Outcome: OutcomeHit, Detail: "/music/a.flac"})
Expect(t.Steps()).To(Equal([]traceStep{
Expect(t.Steps()).To(Equal([]TraceStep{
{Candidate: "cover.*", Outcome: OutcomeMiss},
{Candidate: "embedded", Outcome: OutcomeHit, Detail: "/music/a.flac"},
}))
@ -54,18 +54,18 @@ var _ = Describe("chainTrace", func() {
})
It("does not panic when the trace is nil", func() {
var t *chainTrace
Expect(func() { t.add(traceStep{Candidate: "cover.*", Outcome: OutcomeMiss}) }).ToNot(Panic())
var t *ChainTrace
Expect(func() { t.add(TraceStep{Candidate: "cover.*", Outcome: OutcomeMiss}) }).ToNot(Panic())
Expect(t.Steps()).To(BeEmpty(), "a nil trace collects nothing, so reading it must be as safe as writing it")
})
It("is safe to use concurrently", func() {
t := &chainTrace{}
t := &ChainTrace{}
done := make(chan struct{})
for range 10 {
go func() {
defer GinkgoRecover()
t.add(traceStep{Candidate: "x", Outcome: OutcomeMiss})
t.add(TraceStep{Candidate: "x", Outcome: OutcomeMiss})
done <- struct{}{}
}()
}
@ -78,17 +78,17 @@ var _ = Describe("chainTrace", func() {
var _ = Describe("chainState tracing", func() {
It("records a miss when the candidate was absent", func() {
t := &chainTrace{}
t := &ChainTrace{}
c := chainState{trace: t}
_, ok := c.try("cover.*", resolution{}, false)
Expect(ok).To(BeFalse())
Expect(t.Steps()).To(Equal([]traceStep{{Candidate: "cover.*", Outcome: OutcomeMiss}}))
Expect(t.Steps()).To(Equal([]TraceStep{{Candidate: "cover.*", Outcome: OutcomeMiss}}))
})
It("records unreadable when the candidate existed but could not be read", func() {
t := &chainTrace{}
t := &ChainTrace{}
c := chainState{trace: t}
_, ok := c.try("cover.*", resolution{localError: true}, false)
@ -100,14 +100,14 @@ var _ = Describe("chainState tracing", func() {
})
It("records a hit with the backing path", func() {
t := &chainTrace{}
t := &ChainTrace{}
c := chainState{trace: t}
res, ok := c.try("embedded", resolution{reader: nil, source: "embedded", sourcePath: "/music/a.flac"}, true)
Expect(ok).To(BeTrue())
Expect(res.source).To(Equal("embedded"))
Expect(t.Steps()).To(Equal([]traceStep{
Expect(t.Steps()).To(Equal([]TraceStep{
{Candidate: "embedded", Outcome: OutcomeHit, Detail: "/music/a.flac"},
}))
})
@ -126,26 +126,26 @@ var _ = Describe("external gate tracing", func() {
boom := func() (io.ReadCloser, string, error) { return nil, "", errors.New("returned status 429") }
It("records a hit with the image path", func() {
t := &chainTrace{}
t := &ChainTrace{}
g := tracingGate(t, passthroughGate)
r, _, err := g("deezer", hit)
Expect(err).ToNot(HaveOccurred())
Expect(r).ToNot(BeNil())
Expect(t.Steps()).To(Equal([]traceStep{
Expect(t.Steps()).To(Equal([]TraceStep{
{Candidate: "external:deezer", Outcome: OutcomeHit, Detail: "http://img"},
}))
})
It("records a miss for a not-found", func() {
t := &chainTrace{}
t := &ChainTrace{}
_, _, _ = tracingGate(t, passthroughGate)("deezer", miss)
Expect(t.Steps()[0].Outcome).To(Equal(OutcomeMiss))
})
It("records a miss for a model not-found", func() {
t := &chainTrace{}
t := &ChainTrace{}
notFound := func() (io.ReadCloser, string, error) { return nil, "", model.ErrNotFound }
_, _, _ = tracingGate(t, passthroughGate)("deezer", notFound)
Expect(t.Steps()[0].Outcome).To(Equal(OutcomeMiss),
@ -153,14 +153,14 @@ var _ = Describe("external gate tracing", func() {
})
It("records an error with its reason", func() {
t := &chainTrace{}
t := &ChainTrace{}
_, _, _ = tracingGate(t, passthroughGate)("apple-music", boom)
Expect(t.Steps()[0].Outcome).To(Equal(OutcomeError))
Expect(t.Steps()[0].Detail).To(ContainSubstring("429"))
})
It("never calls the agent in offline mode", func() {
t := &chainTrace{}
t := &ChainTrace{}
called := false
counting := func() (io.ReadCloser, string, error) {
called = true
@ -171,7 +171,7 @@ var _ = Describe("external gate tracing", func() {
Expect(called).To(BeFalse(), "offline mode must not perform external requests")
Expect(err).To(MatchError(errOfflineSkipped))
Expect(t.Steps()).To(Equal([]traceStep{
Expect(t.Steps()).To(Equal([]TraceStep{
{Candidate: "external:deezer", Outcome: OutcomeWouldTry},
}))
})
@ -185,7 +185,7 @@ var _ = Describe("resolveAlbum tracing", func() {
folderRepo *fakeFolderRepo
ffm *tests.MockFFmpeg
ag *agents.Agents
t *chainTrace
t *ChainTrace
)
BeforeEach(func() {
@ -204,7 +204,7 @@ var _ = Describe("resolveAlbum tracing", func() {
}
ffm = tests.NewMockFFmpeg("")
ag = agents.GetAgents(&tests.MockDataStore{}, nil)
t = &chainTrace{}
t = &ChainTrace{}
ctx = withTrace(context.Background(), t)
})
@ -218,7 +218,7 @@ var _ = Describe("resolveAlbum tracing", func() {
Expect(res.reader).ToNot(BeNil())
defer res.reader.Close()
Expect(t.Steps()).To(HaveLen(2), "a configured pattern must appear even when the chain never evaluated it")
Expect(t.Steps()[0]).To(Equal(traceStep{
Expect(t.Steps()[0]).To(Equal(TraceStep{
Candidate: "cover.jpg", Outcome: OutcomeSkipped, Detail: "no images in album folder",
}))
Expect(t.Steps()[1].Candidate).To(Equal("embedded"))
@ -238,7 +238,7 @@ var _ = Describe("resolveAlbum tracing", func() {
Expect(err).ToNot(HaveOccurred())
Expect(res.reader).ToNot(BeNil())
defer res.reader.Close()
Expect(t.Steps()[0]).To(Equal(traceStep{Candidate: "cover.jpg", Outcome: OutcomeMiss}),
Expect(t.Steps()[0]).To(Equal(TraceStep{Candidate: "cover.jpg", Outcome: OutcomeMiss}),
"the folder was searched and held no cover.jpg, which is not the same as never looking")
})
@ -248,7 +248,7 @@ var _ = Describe("resolveAlbum tracing", func() {
_, err := newResolver(ds, ag, ffm, nil).resolve(ctx, model.ArtworkQueueItem{ItemKind: "al", ItemID: "al2"})
Expect(err).ToNot(HaveOccurred())
Expect(t.Steps()).To(Equal([]traceStep{
Expect(t.Steps()).To(Equal([]TraceStep{
{Candidate: "cover.jpg", Outcome: OutcomeSkipped, Detail: "no images in album folder"},
}))
})
@ -263,7 +263,7 @@ var _ = Describe("resolveArtist tracing", func() {
folderRepo *fakeFolderRepo
ffm *tests.MockFFmpeg
ag *agents.Agents
t *chainTrace
t *ChainTrace
repoRoot string
)
@ -294,7 +294,7 @@ var _ = Describe("resolveArtist tracing", func() {
}
ffm = tests.NewMockFFmpeg("")
ag = agents.GetAgents(&tests.MockDataStore{}, nil)
t = &chainTrace{}
t = &ChainTrace{}
ctx = withTrace(context.Background(), t)
})
@ -306,7 +306,7 @@ var _ = Describe("resolveArtist tracing", func() {
Expect(err).ToNot(HaveOccurred())
Expect(res.reader).ToNot(BeNil())
defer res.reader.Close()
Expect(t.Steps()).To(Equal([]traceStep{{Candidate: "upload", Outcome: OutcomeHit, Detail: path}}))
Expect(t.Steps()).To(Equal([]TraceStep{{Candidate: "upload", Outcome: OutcomeHit, Detail: path}}))
})
It("records an upload miss before walking the chain", func() {
@ -314,7 +314,7 @@ var _ = Describe("resolveArtist tracing", func() {
_, err := newResolver(ds, ag, ffm, nil).resolve(ctx, model.ArtworkQueueItem{ItemKind: "ar", ItemID: "ar2"})
Expect(err).ToNot(HaveOccurred())
Expect(t.Steps()[0]).To(Equal(traceStep{Candidate: "upload", Outcome: OutcomeMiss}))
Expect(t.Steps()[0]).To(Equal(TraceStep{Candidate: "upload", Outcome: OutcomeMiss}))
})
It("labels each step with the configured priority token", func() {
@ -342,7 +342,7 @@ var _ = Describe("resolveArtist tracing", func() {
_, err := newResolver(ds, ag, ffm, nil).resolve(ctx, model.ArtworkQueueItem{ItemKind: "ar", ItemID: "ar5"})
Expect(err).ToNot(HaveOccurred())
Expect(t.Steps()).To(Equal([]traceStep{
Expect(t.Steps()).To(Equal([]TraceStep{
{Candidate: "upload", Outcome: OutcomeMiss},
{Candidate: "album/artist.*", Outcome: OutcomeSkipped, Detail: "artist has no albums"},
}), "a configured pattern that was never evaluated must still appear, and say why")
@ -355,7 +355,7 @@ var _ = Describe("resolveArtist tracing", func() {
_, err := newResolver(ds, ag, ffm, nil).resolve(ctx, model.ArtworkQueueItem{ItemKind: "ar", ItemID: "ar6"})
Expect(err).ToNot(HaveOccurred())
Expect(t.Steps()).To(Equal([]traceStep{
Expect(t.Steps()).To(Equal([]TraceStep{
{Candidate: "upload", Outcome: OutcomeMiss},
{Candidate: "artist.*", Outcome: OutcomeSkipped, Detail: "no artist folder"},
}))