fix(plugins): key coalescing by request metadata

plugins/lyrics_adapter.go

@@ -2,6 +2,7 @@ package plugins
 
 import (
 	"context"
+	"encoding/json"
 	"fmt"
 	"time"
 
@@ -51,10 +52,18 @@ func (l *LyricsPlugin) GetLyrics(ctx context.Context, mf *model.MediaFile) (mode
 		return nil, err
 	}
 
-	result := l.calls.DoChan(lyricsPluginCallKey(mf), func() (any, error) {
+	req := capabilities.GetLyricsRequest{
+		Track: mediaFileToTrackInfo(l.plugin, mf),
+	}
+	key, err := lyricsPluginCallKey(req)
+	if err != nil {
+		return nil, err
+	}
+
+	result := l.calls.DoChan(key, func() (any, error) {
 		callCtx, cancel := context.WithTimeout(context.WithoutCancel(ctx), lyricsPluginCallTimeout)
 		defer cancel()
-		return l.getLyrics(callCtx, mf)
+		return l.getLyrics(callCtx, mf, req)
 	})
 
 	select {
@@ -72,31 +81,23 @@ func (l *LyricsPlugin) GetLyrics(ctx context.Context, mf *model.MediaFile) (mode
 	}
 }
 
-func lyricsPluginCallKey(mf *model.MediaFile) string {
-	return fmt.Sprintf(
-		"%s\x00%s\x00%d\x00%s\x00%s\x00%s\x00%.3f",
-		mf.ID,
-		mf.Path,
-		mf.UpdatedAt.UnixNano(),
-		mf.Title,
-		mf.Artist,
-		mf.Album,
-		mf.Duration,
-	)
+func lyricsPluginCallKey(req capabilities.GetLyricsRequest) (string, error) {
+	value, err := json.Marshal(req)
+	if err != nil {
+		return "", fmt.Errorf("encode lyrics plugin request key: %w", err)
+	}
+	return string(value), nil
 }
 
 // getLyrics calls the plugin, then content-sniffs each response via
 // model.ParseLyrics (TTML/SRT/YAML/LRC/plain).
-func (l *LyricsPlugin) getLyrics(ctx context.Context, mf *model.MediaFile) (model.LyricList, error) {
+func (l *LyricsPlugin) getLyrics(ctx context.Context, mf *model.MediaFile, req capabilities.GetLyricsRequest) (model.LyricList, error) {
 	select {
 	case l.plugin.lyricsSem <- struct{}{}:
 		defer func() { <-l.plugin.lyricsSem }()
 	case <-ctx.Done():
 		return nil, ctx.Err()
 	}
-	req := capabilities.GetLyricsRequest{
-		Track: mediaFileToTrackInfo(l.plugin, mf),
-	}
 	resp, err := callPluginFunction[capabilities.GetLyricsRequest, capabilities.GetLyricsResponse](
 		ctx, l.plugin, FuncLyricsGetLyrics, req,
 	)

plugins/lyrics_adapter_test.go

@@ -111,6 +111,54 @@ var _ = Describe("LyricsPlugin", Ordered, func() {
 			Expect(calls[0].method).To(Equal(FuncLyricsGetLyrics))
 		})
 
+		It("does not coalesce requests with different plugin metadata", func() {
+			metrics := &mockMetricsRecorder{}
+			manager, _ := createTestManagerWithPluginsAndMetrics(
+				nil,
+				metrics,
+				"test-lyrics"+PackageExtension,
+			)
+
+			p, ok := manager.LoadLyricsProvider("test-lyrics")
+			Expect(ok).To(BeTrue())
+			coalescingProvider := p.(*LyricsPlugin)
+
+			sem := coalescingProvider.plugin.lyricsSem
+			for range cap(sem) {
+				sem <- struct{}{}
+			}
+			DeferCleanup(func() {
+				for len(sem) > 0 {
+					<-sem
+				}
+			})
+
+			start := make(chan struct{})
+			results := make(chan error, 2)
+			tracks := []*model.MediaFile{
+				{ID: "same-id", Title: "Test Song", Artist: "Test Artist", TrackNumber: 1},
+				{ID: "same-id", Title: "Test Song", Artist: "Test Artist", TrackNumber: 2},
+			}
+			for _, track := range tracks {
+				go func() {
+					<-start
+					_, err := coalescingProvider.GetLyrics(GinkgoT().Context(), track)
+					results <- err
+				}()
+			}
+			close(start)
+
+			Consistently(results, "500ms").ShouldNot(Receive())
+			for range cap(sem) {
+				<-sem
+			}
+
+			for range tracks {
+				Eventually(results).Should(Receive(BeNil()))
+			}
+			Expect(metrics.getCalls()).To(HaveLen(2))
+		})
+
 		It("keeps a shared request alive when one caller cancels", func() {
 			metrics := &mockMetricsRecorder{}
 			manager, _ := createTestManagerWithPluginsAndMetrics(