fix(jellycompat): aggregate series watch state from episode progress (#70)

A series never has a user_watch_progress row of its own, so series items always rendered Played:false with no UnplayedItemCount — every show looked unwatched in compat clients' library views, and the IsPlayed browse filter treated all series as unplayed. Roll episode progress up to series rows in the browse/search/detail paths, mirroring applySeasonUserData and the native API's rollup semantics (WatchedCount/UnplayedCount/InProgressCount). Progress lookups are chunked at 500 ids (per-user stores may be SQLite-backed) and the rollup is capped at 250 series per page so max-limit browses of series-only libraries stay bounded.
2026-06-07 11:42:44 -04:00
parent 826f0d0b9a
commit 3353456d46
3 changed files with 261 additions and 0 deletions
@@ -61,6 +61,7 @@ type seasonListSource interface {
 type episodeListSource interface {
 	ListBySeason(ctx context.Context, seriesID string, seasonNum int) ([]*models.Episode, error)
 	ListBySeriesGroupedBySeason(ctx context.Context, seriesID string) (map[int][]*models.Episode, error)
+	ListBySeriesIDs(ctx context.Context, seriesIDs []string) (map[string][]*models.Episode, error)
 }

 // directContentService implements ContentService by calling catalog repos directly.
@@ -265,6 +266,18 @@ func (s *directContentService) BrowseItems(ctx context.Context, session *Session
 	if len(collected) > requestedLimit {
 		collected = collected[:requestedLimit]
 	}
+
+	// The handler's resolveUserStateForContentIDs overlay only covers items
+	// with their own progress rows, which a series never has — aggregate
+	// series watch state here so library views carry Played/UnplayedItemCount.
+	// The is_played path already did this via enrichListItemsUserData.
+	// That same absence is what keeps this safe from the handler overlay:
+	// userDataDTO only overrides item-level UserData when a direct progress
+	// row exists, and one never does for a series id.
+	if isPlayedFilter == "" {
+		s.enrichSeriesUserData(ctx, session, collected)
+	}
+
 	if totalKnown {
 		hasMore = totalFromCatalog > requestedOffset+requestedLimit
 	}
@@ -347,6 +360,17 @@ func (s *directContentService) GetItemDetail(ctx context.Context, session *Sessi
 					IsInProgress:    progress.PositionSeconds > 0 && !progress.Completed,
 				}
 			}
+
+			// A series never has a progress row of its own, so roll watch
+			// state up from its episodes (mirrors applySeasonUserData) to
+			// give clients Played/UnplayedItemCount at the series level.
+			if result.UserData == nil && strings.EqualFold(result.Type, "series") && s.episodeRepo != nil {
+				if episodesBySeries, epErr := s.episodeRepo.ListBySeriesIDs(ctx, []string{contentID}); epErr == nil {
+					episodes := episodesBySeries[contentID]
+					progressMap := chunkedProgressByMediaItems(ctx, store, session.ProfileID, modelEpisodeContentIDs(episodes))
+					result.UserData = seriesUserDataFromEpisodes(episodes, progressMap)
+				}
+			}
 		}
 	}

@@ -545,6 +569,128 @@ func (s *directContentService) enrichListItemsUserData(ctx context.Context, sess
 			items[i].UserData = seasonUserDataFromProgress(progress)
 		}
 	}
+
+	s.enrichSeriesListUserData(ctx, session, store, items)
+}
+
+// enrichSeriesUserData is enrichSeriesListUserData with store acquisition,
+// for callers that haven't already resolved the user store.
+func (s *directContentService) enrichSeriesUserData(ctx context.Context, session *Session, items []upstreamListItem) {
+	if s.storeProvider == nil || len(items) == 0 {
+		return
+	}
+	store, err := s.userStore(ctx, session)
+	if err != nil {
+		return
+	}
+	s.enrichSeriesListUserData(ctx, session, store, items)
+}
+
+// enrichSeriesListUserData fills aggregated user data for series rows in a
+// list response. A series never has a progress row of its own, so Played and
+// UnplayedItemCount are rolled up from episode progress, mirroring
+// applySeasonUserData semantics.
+func (s *directContentService) enrichSeriesListUserData(ctx context.Context, session *Session, store userstore.UserStore, items []upstreamListItem) {
+	if s.episodeRepo == nil {
+		return
+	}
+	seriesIDs := make([]string, 0, len(items))
+	for i := range items {
+		if items[i].UserData == nil && items[i].ContentID != "" && strings.EqualFold(items[i].Type, "series") {
+			seriesIDs = append(seriesIDs, items[i].ContentID)
+		}
+	}
+	if len(seriesIDs) == 0 {
+		return
+	}
+	// Cap the rollup so a max-limit browse (compatBrowseMaxLimit) of a
+	// series-only library can't materialize hundreds of thousands of episode
+	// rows in one request. Series past the cap keep a nil UserData — the same
+	// shape they had before series aggregation existed. Real clients page at
+	// 20-100 items, well under the cap.
+	const maxSeriesUserDataRollups = 250
+	if len(seriesIDs) > maxSeriesUserDataRollups {
+		seriesIDs = seriesIDs[:maxSeriesUserDataRollups]
+	}
+	episodesBySeries, err := s.episodeRepo.ListBySeriesIDs(ctx, seriesIDs)
+	if err != nil {
+		return
+	}
+	episodeIDs := make([]string, 0, 256)
+	for _, episodes := range episodesBySeries {
+		episodeIDs = append(episodeIDs, modelEpisodeContentIDs(episodes)...)
+	}
+	progressMap := chunkedProgressByMediaItems(ctx, store, session.ProfileID, episodeIDs)
+	for i := range items {
+		if items[i].UserData != nil || !strings.EqualFold(items[i].Type, "series") {
+			continue
+		}
+		if episodes, ok := episodesBySeries[items[i].ContentID]; ok {
+			items[i].UserData = seriesUserDataFromEpisodes(episodes, progressMap)
+		}
+	}
+}
+
+// seriesUserDataFromEpisodes computes WatchedCount/UnplayedCount/
+// InProgressCount/Played for a whole series from a pre-fetched progressMap.
+// Pure function — no I/O. Counting semantics match the native API's series
+// rollup (all episodes including specials; in-progress = started but not
+// completed).
+func seriesUserDataFromEpisodes(episodes []*models.Episode, progressMap map[string]userstore.WatchProgress) *catalog.SeasonUserData {
+	watched := 0
+	unplayed := 0
+	inProgress := 0
+	for _, ep := range episodes {
+		if ep == nil {
+			continue
+		}
+		progress, ok := progressMap[ep.ContentID]
+		if ok && progress.Completed {
+			watched++
+			continue
+		}
+		if ok && progress.PositionSeconds > 0 {
+			inProgress++
+		}
+		unplayed++
+	}
+	return &catalog.SeasonUserData{
+		WatchedCount:    watched,
+		UnplayedCount:   unplayed,
+		InProgressCount: inProgress,
+		Played:          unplayed == 0 && len(episodes) > 0,
+	}
+}
+
+// modelEpisodeContentIDs returns the non-empty content ids of the given episodes.
+func modelEpisodeContentIDs(episodes []*models.Episode) []string {
+	ids := make([]string, 0, len(episodes))
+	for _, ep := range episodes {
+		if ep != nil && ep.ContentID != "" {
+			ids = append(ids, ep.ContentID)
+		}
+	}
+	return ids
+}
+
+// chunkedProgressByMediaItems batches ListProgressByMediaItems calls when a
+// series list expands to thousands of episode ids: per-user stores may be
+// SQLite-backed (999 bind-variable default), and chunking also keeps
+// Postgres IN-lists at a sane size. Returns an empty map (never nil); chunks
+// that error are skipped.
+func chunkedProgressByMediaItems(ctx context.Context, store userstore.UserStore, profileID string, mediaItemIDs []string) map[string]userstore.WatchProgress {
+	const chunkSize = 500
+	result := make(map[string]userstore.WatchProgress, len(mediaItemIDs))
+	for start := 0; start < len(mediaItemIDs); start += chunkSize {
+		chunk, err := store.ListProgressByMediaItems(ctx, profileID, mediaItemIDs[start:min(start+chunkSize, len(mediaItemIDs))])
+		if err != nil {
+			continue
+		}
+		for id, progress := range chunk {
+			result[id] = progress
+		}
+	}
+	return result
 }

 // enrichSeasonUserData adds aggregated user data for a season. Used by
@@ -65,6 +65,10 @@ func (c *countingEpisodeListSource) ListBySeriesGroupedBySeason(_ context.Contex
 	return out, nil
 }

+func (c *countingEpisodeListSource) ListBySeriesIDs(_ context.Context, _ []string) (map[string][]*models.Episode, error) {
+	return map[string][]*models.Episode{}, nil
+}
+
 // TestListSeasons_UsesBatchEpisodeFetch verifies ListSeasons makes exactly
 // one ListBySeriesGroupedBySeason call (replacing N per-season ListBySeason
 // calls) and exactly one ListProgressByMediaItems call (replacing N
@@ -0,0 +1,111 @@
+package jellycompat
+
+import (
+	"testing"
+
+	"github.com/Silo-Server/silo-server/internal/models"
+	"github.com/Silo-Server/silo-server/internal/userstore"
+)
+
+func ep(id string) *models.Episode {
+	return &models.Episode{ContentID: id}
+}
+
+// TestSeriesUserDataFromEpisodes covers the series watch-state rollup: a
+// series has no progress row of its own, so Played/UnplayedCount/
+// InProgressCount are aggregated from per-episode progress.
+func TestSeriesUserDataFromEpisodes(t *testing.T) {
+	cases := []struct {
+		name           string
+		episodes       []*models.Episode
+		progress       map[string]userstore.WatchProgress
+		wantWatched    int
+		wantUnplayed   int
+		wantInProgress int
+		wantPlayed     bool
+	}{
+		{
+			name:     "no episodes",
+			episodes: nil,
+			progress: map[string]userstore.WatchProgress{},
+			// Played must be false for an empty series — unplayed==0 alone
+			// must not flip it.
+			wantPlayed: false,
+		},
+		{
+			name:     "all watched",
+			episodes: []*models.Episode{ep("a"), ep("b")},
+			progress: map[string]userstore.WatchProgress{
+				"a": {Completed: true},
+				"b": {Completed: true},
+			},
+			wantWatched: 2,
+			wantPlayed:  true,
+		},
+		{
+			name:     "mixed watched, in-progress, untouched",
+			episodes: []*models.Episode{ep("a"), ep("b"), ep("c")},
+			progress: map[string]userstore.WatchProgress{
+				"a": {Completed: true},
+				"b": {PositionSeconds: 120},
+			},
+			wantWatched:    1,
+			wantUnplayed:   2,
+			wantInProgress: 1,
+			wantPlayed:     false,
+		},
+		{
+			name:         "no progress at all",
+			episodes:     []*models.Episode{ep("a")},
+			progress:     map[string]userstore.WatchProgress{},
+			wantUnplayed: 1,
+			wantPlayed:   false,
+		},
+		{
+			name:     "nil episodes skipped",
+			episodes: []*models.Episode{nil, ep("a"), nil},
+			progress: map[string]userstore.WatchProgress{
+				"a": {Completed: true},
+			},
+			wantWatched: 1,
+			wantPlayed:  true,
+		},
+		{
+			name:     "zero-position progress row is not in-progress",
+			episodes: []*models.Episode{ep("a")},
+			progress: map[string]userstore.WatchProgress{
+				"a": {PositionSeconds: 0},
+			},
+			wantUnplayed:   1,
+			wantInProgress: 0,
+			wantPlayed:     false,
+		},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			got := seriesUserDataFromEpisodes(tc.episodes, tc.progress)
+			if got.WatchedCount != tc.wantWatched {
+				t.Errorf("WatchedCount = %d, want %d", got.WatchedCount, tc.wantWatched)
+			}
+			if got.UnplayedCount != tc.wantUnplayed {
+				t.Errorf("UnplayedCount = %d, want %d", got.UnplayedCount, tc.wantUnplayed)
+			}
+			if got.InProgressCount != tc.wantInProgress {
+				t.Errorf("InProgressCount = %d, want %d", got.InProgressCount, tc.wantInProgress)
+			}
+			if got.Played != tc.wantPlayed {
+				t.Errorf("Played = %v, want %v", got.Played, tc.wantPlayed)
+			}
+		})
+	}
+}
+
+// TestModelEpisodeContentIDs verifies nil episodes and empty content ids are
+// dropped — these ids feed SQL IN-lists.
+func TestModelEpisodeContentIDs(t *testing.T) {
+	got := modelEpisodeContentIDs([]*models.Episode{nil, ep(""), ep("a"), ep("b")})
+	if len(got) != 2 || got[0] != "a" || got[1] != "b" {
+		t.Errorf("modelEpisodeContentIDs = %v, want [a b]", got)
+	}
+}