fix(sections): harden trending refresher per PR review

- Interleave Trakt movies/shows by rank so the mixed row shows both types
  instead of burying all series past the display limit.
- Treat any Trakt sub-fetch failure as fatal (errors.Join) so a partial
  result never overwrites the last-good snapshot with a media type missing.
- Skip non-title entries (TMDB trending/all returns media_type "person") in
  both ID batching and ordering so they can't match an unrelated library title.
- Guard the refresh task against a nil refresher.
- Tests: person skip, Trakt interleave, Trakt partial-failure preserves
  last-good, snapshot read error propagation.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

internal/sections/fetcher.go

@@ -2432,10 +2432,17 @@ func orderedTrendingContentIDs(entries []trendingDiscoverEntry, movieLookup, ser
 	seen := make(map[string]struct{}, len(entries))
 	out := make([]string, 0, len(entries))
 	for _, e := range entries {
-		lookup := movieLookup
+		// Only movies and series are resolvable; skip anything else (TMDB
+		// trending/all also returns media_type "person").
+		var lookup *catalog.ExternalIDLookup
 		isSeries := e.mediaType == "tv"
-		if isSeries {
+		switch e.mediaType {
+		case "movie":
+			lookup = movieLookup
+		case "tv":
 			lookup = seriesLookup
+		default:
+			continue
 		}
 		if lookup == nil {
 			continue

internal/sections/trending_read_test.go

@@ -2,6 +2,7 @@ package sections
 
 import (
 	"context"
+	"errors"
 	"testing"
 )
 
@@ -50,3 +51,15 @@ func TestLoadTrendingDiscoverContentIDsNotFound(t *testing.T) {
 		t.Fatalf("ids = %v; want nil when no snapshot exists", ids)
 	}
 }
+
+func TestLoadTrendingDiscoverContentIDsPropagatesError(t *testing.T) {
+	boom := errors.New("boom")
+	f := &Fetcher{TrendingSnapshots: fakeSnapshotGetter{err: boom}}
+	ids, err := f.loadTrendingDiscoverContentIDs(context.Background(), "tmdb", "week")
+	if !errors.Is(err, boom) {
+		t.Fatalf("err = %v; want boom", err)
+	}
+	if ids != nil {
+		t.Fatalf("ids = %v; want nil on error", ids)
+	}
+}

internal/sections/trending_refresher.go

@@ -3,6 +3,7 @@ package sections
 import (
 	"context"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"log/slog"
 	"time"
@@ -189,19 +190,18 @@ func (r *TrendingRefresher) fetchEntries(ctx context.Context, source, window str
 		if r.TraktTrending == nil {
 			return nil, nil
 		}
+		// Trakt has no mixed endpoint; fetch movies + shows separately. Treat ANY
+		// failure as fatal so a partial result never overwrites the last-good
+		// snapshot with one media type missing.
 		movies, movieErr := r.TraktTrending.GetCollectionPreset(ctx, "trending", "movie", fetchLimit, "")
 		shows, showErr := r.TraktTrending.GetCollectionPreset(ctx, "trending", "tv", fetchLimit, "")
-		if movieErr != nil && showErr != nil {
-			return nil, fmt.Errorf("trakt trending: %w / %w", movieErr, showErr)
+		if movieErr != nil || showErr != nil {
+			return nil, fmt.Errorf("trakt trending: %w", errors.Join(movieErr, showErr))
 		}
-		out := make([]trendingDiscoverEntry, 0, len(movies)+len(shows))
-		for _, e := range movies {
-			out = append(out, newTrendingEntry(e.TMDBID, e.TVDBID, e.IMDbID, e.MediaType))
-		}
-		for _, e := range shows {
-			out = append(out, newTrendingEntry(e.TMDBID, e.TVDBID, e.IMDbID, e.MediaType))
-		}
-		return out, nil
+		// Interleave by rank so the mixed row actually shows both movies and
+		// series; plain concatenation would bury all series past the display
+		// limit whenever enough movies match the library.
+		return interleaveTraktEntries(movies, shows), nil
 	}
 
 	if r.TMDBTrending == nil {
@@ -218,6 +218,24 @@ func (r *TrendingRefresher) fetchEntries(ctx context.Context, source, window str
 	return out, nil
 }
 
+// interleaveTraktEntries alternates rank-ordered movies and shows so the mixed
+// trending row surfaces both media types. Each input list is already in trending
+// order; alternating preserves that order within each type while mixing them.
+func interleaveTraktEntries(movies, shows []catalog.TraktCollectionEntry) []trendingDiscoverEntry {
+	out := make([]trendingDiscoverEntry, 0, len(movies)+len(shows))
+	for i := 0; i < len(movies) || i < len(shows); i++ {
+		if i < len(movies) {
+			m := movies[i]
+			out = append(out, newTrendingEntry(m.TMDBID, m.TVDBID, m.IMDbID, m.MediaType))
+		}
+		if i < len(shows) {
+			s := shows[i]
+			out = append(out, newTrendingEntry(s.TMDBID, s.TVDBID, s.IMDbID, s.MediaType))
+		}
+	}
+	return out
+}
+
 // resolveIDs matches trending entries to library content IDs via two batched
 // external-ID lookups (movies, series), preserving trending order.
 func (r *TrendingRefresher) resolveIDs(ctx context.Context, entries []trendingDiscoverEntry) ([]string, error) {
@@ -226,9 +244,16 @@ func (r *TrendingRefresher) resolveIDs(ctx context.Context, entries []trendingDi
 	}
 	var movieBatch, seriesBatch catalog.ExternalIDBatch
 	for _, e := range entries {
-		batch := &movieBatch
-		if e.mediaType == "tv" {
+		var batch *catalog.ExternalIDBatch
+		switch e.mediaType {
+		case "movie":
+			batch = &movieBatch
+		case "tv":
 			batch = &seriesBatch
+		default:
+			// Skip non-title entries (TMDB trending/all can return "person") so
+			// they never match an unrelated library title by shared external ID.
+			continue
 		}
 		if e.tmdbID != "" {
 			batch.TMDBIDs = append(batch.TMDBIDs, e.tmdbID)

internal/sections/trending_refresher_test.go

@@ -59,6 +59,18 @@ func (f fakeTMDB) GetCollectionPreset(context.Context, string, string, string, i
 	return f.entries, f.err
 }
 
+type fakeTrakt struct {
+	byMediaType map[string][]catalog.TraktCollectionEntry
+	errByType   map[string]error
+}
+
+func (f fakeTrakt) GetCollectionPreset(_ context.Context, _, mediaType string, _ int, _ string) ([]catalog.TraktCollectionEntry, error) {
+	if err := f.errByType[mediaType]; err != nil {
+		return nil, err
+	}
+	return f.byMediaType[mediaType], nil
+}
+
 type fakeResolver struct {
 	byType map[string]*catalog.ExternalIDLookup
 }
@@ -176,6 +188,90 @@ func TestRefresherEmptyProviderPreservesLastGood(t *testing.T) {
 	}
 }
 
+func TestRefresherSkipsPersonEntries(t *testing.T) {
+	store := newFakeSnapshotStore()
+	r := &TrendingRefresher{
+		Sections:  fakeSectionLister{configs: []json.RawMessage{tmdbConfig(t, "tmdb", "week")}},
+		Snapshots: store,
+		Resolver: fakeResolver{byType: map[string]*catalog.ExternalIDLookup{
+			// "99" is present in the movie lookup to simulate a person ID that
+			// collides with an unrelated library movie's TMDB ID.
+			"movie":  {ByTMDB: map[string]string{"10": "c-movie", "99": "c-person-collision"}, ByIMDb: map[string]string{}, ByTVDB: map[string]string{}},
+			"series": {ByTMDB: map[string]string{"20": "c-series"}, ByIMDb: map[string]string{}, ByTVDB: map[string]string{}},
+		}},
+		TMDBTrending: fakeTMDB{entries: []catalog.TMDBCollectionEntry{
+			{ID: 10, MediaType: "movie"},
+			{ID: 99, MediaType: "person"},
+			{ID: 20, MediaType: "tv"},
+		}},
+		Clock: recipes.FixedClock(time.Date(2026, 5, 29, 12, 0, 0, 0, time.UTC)),
+	}
+
+	if _, err := r.RunOnce(context.Background()); err != nil {
+		t.Fatalf("RunOnce: %v", err)
+	}
+
+	got := store.saved["tmdb|week"].contentIDs
+	want := []string{"c-movie", "c-series"}
+	if len(got) != len(want) || got[0] != want[0] || got[1] != want[1] {
+		t.Fatalf("content IDs = %v; want %v (person entry must be skipped)", got, want)
+	}
+}
+
+func TestRefresherTraktInterleavesMoviesAndShows(t *testing.T) {
+	store := newFakeSnapshotStore()
+	r := &TrendingRefresher{
+		Sections:  fakeSectionLister{configs: []json.RawMessage{tmdbConfig(t, "trakt", "week")}},
+		Snapshots: store,
+		Resolver: fakeResolver{byType: map[string]*catalog.ExternalIDLookup{
+			"movie":  {ByTMDB: map[string]string{"1": "m1", "2": "m2"}, ByIMDb: map[string]string{}, ByTVDB: map[string]string{}},
+			"series": {ByTMDB: map[string]string{"3": "s1"}, ByIMDb: map[string]string{}, ByTVDB: map[string]string{}},
+		}},
+		TraktTrending: fakeTrakt{byMediaType: map[string][]catalog.TraktCollectionEntry{
+			"movie": {{TMDBID: 1, MediaType: "movie"}, {TMDBID: 2, MediaType: "movie"}},
+			"tv":    {{TMDBID: 3, MediaType: "tv"}},
+		}},
+		Clock: recipes.FixedClock(time.Date(2026, 5, 29, 12, 0, 0, 0, time.UTC)),
+	}
+
+	if _, err := r.RunOnce(context.Background()); err != nil {
+		t.Fatalf("RunOnce: %v", err)
+	}
+
+	// Interleaved order: movie[0], show[0], movie[1] => m1, s1, m2. A plain
+	// concat would have buried s1 after all movies.
+	got := store.saved["trakt|week"].contentIDs
+	want := []string{"m1", "s1", "m2"}
+	if len(got) != len(want) || got[0] != want[0] || got[1] != want[1] || got[2] != want[2] {
+		t.Fatalf("content IDs = %v; want %v (interleaved)", got, want)
+	}
+}
+
+func TestRefresherTraktPartialFailurePreservesLastGood(t *testing.T) {
+	store := newFakeSnapshotStore()
+	r := &TrendingRefresher{
+		Sections:  fakeSectionLister{configs: []json.RawMessage{tmdbConfig(t, "trakt", "week")}},
+		Snapshots: store,
+		Resolver:  fakeResolver{},
+		TraktTrending: fakeTrakt{
+			byMediaType: map[string][]catalog.TraktCollectionEntry{"movie": {{TMDBID: 1, MediaType: "movie"}}},
+			errByType:   map[string]error{"tv": errors.New("trakt shows 500")},
+		},
+		Clock: recipes.FixedClock(time.Date(2026, 5, 29, 12, 0, 0, 0, time.UTC)),
+	}
+
+	if _, err := r.RunOnce(context.Background()); err != nil {
+		t.Fatalf("RunOnce: %v", err)
+	}
+
+	if _, ok := store.saved["trakt|week"]; ok {
+		t.Fatal("SaveSuccess must not run when one Trakt sub-fetch fails (would drop a media type)")
+	}
+	if store.attempts["trakt|week"].status != "error" {
+		t.Fatalf("attempt status = %q; want error", store.attempts["trakt|week"].status)
+	}
+}
+
 func TestDistinctTrendingCombosCollapsesTrakt(t *testing.T) {
 	configs := []json.RawMessage{
 		tmdbConfig(t, "trakt", "day"),

internal/taskmanager/tasks/refresh_trending_discover.go

@@ -3,6 +3,7 @@ package tasks
 import (
 	"context"
 	"encoding/json"
+	"errors"
 	"fmt"
 
 	"github.com/Silo-Server/silo-server/internal/taskmanager"
@@ -47,6 +48,10 @@ func (t *RefreshTrendingDiscoverTask) DefaultTriggers() []taskmanager.TriggerCon
 func (t *RefreshTrendingDiscoverTask) Execute(ctx context.Context, progress taskmanager.ProgressReporter) error {
 	progress.Report(0, "Refreshing trending discover")
 
+	if t.refresher == nil {
+		return errors.New("trending discover refresh: refresher not configured")
+	}
+
 	resultData, err := t.refresher.RunOnce(ctx)
 	if err != nil {
 		return fmt.Errorf("trending discover refresh: %w", err)