0.2.73

server/internal/api/admin.go

@@ -60,8 +60,6 @@ func (s *Server) adminRoutes() http.Handler {
 	mux.Handle("POST /admin/api/quiet-time", s.adminAuth(s.handleAdminQuietTime))
 	mux.Handle("POST /admin/api/deployment-alert", s.adminAuth(s.handleAdminDeploymentAlert))
 	mux.Handle("POST /admin/api/update-policy", s.adminAuth(s.handleAdminUpdatePolicy))
-	mux.Handle("GET /admin/api/release-builder", s.adminAuth(s.handleAdminReleaseBuilderStatus))
-	mux.Handle("POST /admin/api/release-builder", s.adminAuth(s.handleAdminReleaseBuilderStart))
 	mux.Handle("POST /admin/api/request-policy", s.adminAuth(s.handleAdminRequestPolicy))
 	mux.Handle("GET /admin/api/media-reports", s.adminAuth(s.handleAdminMediaReports))
 	mux.Handle("POST /admin/api/media-reports/{id}/status", s.adminAuth(s.handleAdminMediaReportStatus))

server/internal/api/genres.go

@@ -67,7 +67,7 @@ func (s *Server) handleBrowseItems(
 	limit := queryInt(r, "limit", genrePageSize, genrePageMax)
 	offset := queryOffset(r, "offset")
 	itemType, ok := genreItemType(r.URL.Query().Get("type"))
-	if !ok || (genre == "" && itemType == "Movie,Series") {
+	if !ok {
 		writeError(w, http.StatusBadRequest, "type must be Movie or Series")
 		return
 	}
@@ -145,8 +145,15 @@ func (s *Server) handleBrowseItems(
 	writeRaw(w, http.StatusOK, body)
 }
 
-// genreItemType keeps the old mixed search as the default for Search, while the Movies
-// and TV Series destinations can ask for a shelf that never crosses media types.
+// genreItemType keeps the mixed shelf as the default, which is what the Search chips and
+// the Genres destination ask for, while the Movies and TV Series destinations name a type
+// and get a shelf that never crosses media types.
+//
+// The unfiltered browse used to refuse the mixed type, on the reasoning that a whole
+// library with no genre and no media type is not a shelf anybody asked for. The Genres
+// destination is exactly that request — its "All genres" entry is the catalogue itself —
+// and refusing it here only made the one entry at the top of that rail the one entry that
+// could not answer.
 func genreItemType(value string) (string, bool) {
 	switch strings.ToLower(strings.TrimSpace(value)) {
 	case "":

server/internal/api/items.go

@@ -46,21 +46,43 @@ func (s *Server) handleItem(w http.ResponseWriter, r *http.Request, sess store.S
 		writeError(w, http.StatusBadRequest, "item id is required")
 		return
 	}
-	// Version the entry when the detail contract grows so older cached payloads cannot
-	// hide newly requested fields such as People or the stored ratings.
-	key := cache.UserKey(sess.EmbyUserID, "item:v6:"+itemID)
-
-	if raw, err := s.cache.Get(ctx, key); err == nil {
+	if raw, err := s.cache.Get(ctx, itemDetailKey(sess.EmbyUserID, itemID)); err == nil {
 		w.Header().Set("X-Memby-Cache", "hit")
 		writeRaw(w, http.StatusOK, raw)
 		return
 	}
 
-	item, err := s.emby.Item(ctx, credentials(sess), itemID, fieldsDetail)
+	item, err := s.detailItem(ctx, sess, itemID)
 	if err != nil {
 		s.writeUpstreamError(ctx, w, err, "could not load the item")
 		return
 	}
+	w.Header().Set("X-Memby-Cache", "miss")
+	writeRaw(w, http.StatusOK, item)
+}
+
+// itemDetailKey is versioned so that when the detail contract grows, older cached payloads
+// cannot hide newly requested fields such as People or the stored ratings.
+func itemDetailKey(userID, itemID string) string {
+	return cache.UserKey(userID, "item:v6:"+itemID)
+}
+
+// detailItem is the full record for one item, decorated and kept.
+//
+// It is shared rather than private to the item route because a Magic press hands back a
+// title the television is about to open a detail page for — and before this, that press
+// paid its own uncached Emby lookup and then the page paid a second one a moment later.
+func (s *Server) detailItem(
+	ctx context.Context, sess store.Session, itemID string,
+) (json.RawMessage, error) {
+	key := itemDetailKey(sess.EmbyUserID, itemID)
+	if raw, err := s.cache.Get(ctx, key); err == nil {
+		return raw, nil
+	}
+	item, err := s.emby.Item(ctx, credentials(sess), itemID, fieldsDetail)
+	if err != nil {
+		return nil, err
+	}
 	// A detail page can then draw its ratings with the rest of the hero rather than
 	// after a second request. Anything not yet stored still arrives on /ratings.
 	decorated := []json.RawMessage{item}
@@ -69,8 +91,7 @@ func (s *Server) handleItem(w http.ResponseWriter, r *http.Request, sess store.S
 	if err := s.cache.Set(ctx, key, item, s.cfg.ItemTTL); err != nil {
 		s.loggerFor(ctx).Warn("item cache write failed", "error", err)
 	}
-	w.Header().Set("X-Memby-Cache", "miss")
-	writeRaw(w, http.StatusOK, item)
+	return item, nil
 }
 
 // handleSeasonFinale verifies an episode against Sonarr's complete season, including

server/internal/api/magic.go

@@ -1,11 +1,14 @@
 package api
 
 import (
+	"context"
 	"encoding/json"
 	"math/rand"
 	"net/http"
 	"strings"
+	"time"
 
+	"github.com/ponzischeme89/memby/server/internal/cache"
 	"github.com/ponzischeme89/memby/server/internal/recommend"
 	"github.com/ponzischeme89/memby/server/internal/store"
 )
@@ -53,12 +56,24 @@ func (s *Server) handleMagic(w http.ResponseWriter, r *http.Request, sess store.
 		_ = json.NewDecoder(r.Body).Decode(&req)
 	}
 
-	selection, ok := s.recommender.MagicPick(ctx, credentials(sess), recommend.MagicOptions{
+	pool, cached := s.magicPool(ctx, sess)
+	selection, ok := recommend.ChooseMagic(pool, recommend.MagicOptions{
 		ExcludeIDs:       req.ExcludeIDs,
 		AvailableMinutes: req.AvailableMinutes,
 		// The one non-deterministic thing about the feature, named in one place.
 		Roll: rand.Float64(),
 	})
+	if !ok && cached {
+		// Everything the kept pool held has already been offered. That is a pool that has
+		// run its course rather than a household with nothing left, so it is rebuilt once
+		// before the button is allowed to say no.
+		pool = s.rebuildMagicPool(ctx, sess)
+		selection, ok = recommend.ChooseMagic(pool, recommend.MagicOptions{
+			ExcludeIDs:       req.ExcludeIDs,
+			AvailableMinutes: req.AvailableMinutes,
+			Roll:             rand.Float64(),
+		})
+	}
 	if !ok {
 		// A household that has run out of unseen library is not an error, and the television
 		// says so quietly rather than showing a failure over somebody's film.
@@ -67,24 +82,54 @@ func (s *Server) handleMagic(w http.ResponseWriter, r *http.Request, sess store.
 		return
 	}
 
-	item, err := s.emby.Item(ctx, credentials(sess), selection.Item.ID, fieldsDetail)
+	item, err := s.detailItem(ctx, sess, selection.ItemID)
 	if err != nil {
 		s.writeUpstreamError(ctx, w, err, "could not load the suggestion")
 		return
 	}
-	decorated := []json.RawMessage{item}
-	s.decorateItemRatings(ctx, decorated)
 
 	s.loggerFor(ctx).Info("magic picked",
-		"item", selection.Item.ID,
-		"title", selection.Item.Name,
+		"item", selection.ItemID,
+		"title", selection.Title,
 		"score", selection.Score,
 		"pool", selection.PoolSize,
 		"signals", strings.Join(selection.Signals, ","),
 	)
 
 	writeJSON(w, http.StatusOK, magicResponse{
-		Item:    decorated[0],
+		Item:    item,
 		Reasons: selection.Reasons,
 	})
 }
+
+// magicPool returns the kept pool, and whether it came from the cache.
+//
+// The press is made with the film paused behind a loading panel, so what happens on it
+// matters: building a pool is two complete reads of this viewer's Emby history plus a
+// catalogue query, and none of that answer changes between one press and the next. Keeping
+// it turns every press after the first into arithmetic over a few dozen numbers.
+func (s *Server) magicPool(ctx context.Context, sess store.Session) ([]recommend.MagicCandidate, bool) {
+	if raw, err := s.cache.Get(ctx, cache.MagicPoolKey(sess.EmbyUserID)); err == nil {
+		var pool []recommend.MagicCandidate
+		if err := json.Unmarshal(raw, &pool); err == nil && len(pool) > 0 {
+			return pool, true
+		}
+	}
+	return s.rebuildMagicPool(ctx, sess), false
+}
+
+func (s *Server) rebuildMagicPool(ctx context.Context, sess store.Session) []recommend.MagicCandidate {
+	pool := s.recommender.MagicPool(ctx, credentials(sess), time.Time{})
+	if len(pool) == 0 {
+		// Deliberately not cached: an empty pool is a household whose library or Emby was
+		// unavailable far more often than it is one with no films, and keeping that answer
+		// would withdraw the button for hours over a moment's trouble.
+		return nil
+	}
+	if raw, err := json.Marshal(pool); err == nil {
+		if err := s.cache.Set(ctx, cache.MagicPoolKey(sess.EmbyUserID), raw, s.cfg.MagicPoolTTL); err != nil {
+			s.loggerFor(ctx).Warn("magic pool cache write failed", "error", err)
+		}
+	}
+	return pool
+}

server/internal/api/release_builder.go

@@ -1,86 +0,0 @@
-package api
-
-import (
-	"bytes"
-	"encoding/json"
-	"io"
-	"net/http"
-	"regexp"
-	"strings"
-	"time"
-)
-
-var releaseBuilderTagPattern = regexp.MustCompile(`^v\d+\.\d+\.\d+$`)
-
-type releaseBuilderRequest struct {
-	Tag       string `json:"tag"`
-	Notes     string `json:"notes"`
-	Mandatory bool   `json:"mandatory"`
-}
-
-func (s *Server) handleAdminReleaseBuilderStatus(w http.ResponseWriter, r *http.Request) {
-	s.relayReleaseBuilder(w, r, http.MethodGet, "/v1/status", nil)
-}
-
-func (s *Server) handleAdminReleaseBuilderStart(w http.ResponseWriter, r *http.Request) {
-	if s.rejectWorkDuringQuietTime(w) {
-		return
-	}
-	r.Body = http.MaxBytesReader(w, r.Body, 16<<10)
-	var request releaseBuilderRequest
-	decoder := json.NewDecoder(r.Body)
-	decoder.DisallowUnknownFields()
-	if err := decoder.Decode(&request); err != nil {
-		writeError(w, http.StatusBadRequest, "invalid release request")
-		return
-	}
-	request.Tag = strings.TrimSpace(request.Tag)
-	request.Notes = strings.TrimSpace(request.Notes)
-	if request.Tag != "" && !releaseBuilderTagPattern.MatchString(request.Tag) {
-		writeError(w, http.StatusBadRequest, "tag must be blank or look like v0.2.64")
-		return
-	}
-	if len(request.Notes) > 4000 {
-		writeError(w, http.StatusBadRequest, "release notes are too long")
-		return
-	}
-	body, err := json.Marshal(request)
-	if err != nil {
-		writeError(w, http.StatusInternalServerError, "could not prepare release request")
-		return
-	}
-	s.relayReleaseBuilder(w, r, http.MethodPost, "/v1/releases", body)
-}
-
-func (s *Server) relayReleaseBuilder(w http.ResponseWriter, incoming *http.Request, method, path string, body []byte) {
-	if s.cfg.ReleaseBuilderURL == "" || s.cfg.ReleasePublishToken == "" {
-		writeError(w, http.StatusServiceUnavailable, "the Docker release builder is not configured")
-		return
-	}
-	request, err := http.NewRequestWithContext(incoming.Context(), method,
-		s.cfg.ReleaseBuilderURL+path, bytes.NewReader(body))
-	if err != nil {
-		writeError(w, http.StatusInternalServerError, "could not prepare builder request")
-		return
-	}
-	request.Header.Set("Authorization", "Bearer "+s.cfg.ReleasePublishToken)
-	if len(body) > 0 {
-		request.Header.Set("Content-Type", "application/json")
-	}
-	client := &http.Client{Timeout: 10 * time.Second}
-	response, err := client.Do(request)
-	if err != nil {
-		writeError(w, http.StatusServiceUnavailable, "the Docker release builder is not available")
-		return
-	}
-	defer response.Body.Close()
-	payload, err := io.ReadAll(io.LimitReader(response.Body, 1<<20))
-	if err != nil {
-		writeError(w, http.StatusBadGateway, "could not read the Docker release builder response")
-		return
-	}
-	w.Header().Set("Content-Type", "application/json")
-	w.Header().Set("Cache-Control", "no-store")
-	w.WriteHeader(response.StatusCode)
-	_, _ = w.Write(payload)
-}

server/internal/api/release_builder_test.go

@@ -1,62 +0,0 @@
-package api
-
-import (
-	"io"
-	"net/http"
-	"net/http/httptest"
-	"strings"
-	"testing"
-
-	"github.com/ponzischeme89/memby/server/internal/config"
-)
-
-func TestAdminReleaseBuilderRelaysWithoutExposingToken(t *testing.T) {
-	var receivedAuth string
-	builder := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		receivedAuth = r.Header.Get("Authorization")
-		if r.Method != http.MethodPost || r.URL.Path != "/v1/releases" {
-			t.Fatalf("builder request = %s %s", r.Method, r.URL.Path)
-		}
-		body, _ := io.ReadAll(r.Body)
-		if !strings.Contains(string(body), `"tag":"v0.2.64"`) || !strings.Contains(string(body), `"mandatory":true`) {
-			t.Fatalf("builder body = %s", body)
-		}
-		w.Header().Set("Content-Type", "application/json")
-		w.WriteHeader(http.StatusAccepted)
-		_, _ = io.WriteString(w, `{"state":"running","logs":[]}`)
-	}))
-	defer builder.Close()
-
-	s := &Server{cfg: config.Config{ReleaseBuilderURL: builder.URL, ReleasePublishToken: "release-secret"}}
-	recorder := httptest.NewRecorder()
-	request := httptest.NewRequest(http.MethodPost, "/admin/api/release-builder",
-		strings.NewReader(`{"tag":"v0.2.64","notes":"Living room polish","mandatory":true}`))
-	s.handleAdminReleaseBuilderStart(recorder, request)
-
-	if recorder.Code != http.StatusAccepted || receivedAuth != "Bearer release-secret" {
-		t.Fatalf("response/auth = %d/%q", recorder.Code, receivedAuth)
-	}
-	if strings.Contains(recorder.Body.String(), "release-secret") {
-		t.Fatal("release token was exposed to the browser")
-	}
-}
-
-func TestAdminReleaseBuilderValidatesTagBeforeRelay(t *testing.T) {
-	s := &Server{cfg: config.Config{ReleaseBuilderURL: "http://builder", ReleasePublishToken: "secret"}}
-	recorder := httptest.NewRecorder()
-	request := httptest.NewRequest(http.MethodPost, "/admin/api/release-builder",
-		strings.NewReader(`{"tag":"latest; rm -rf /"}`))
-	s.handleAdminReleaseBuilderStart(recorder, request)
-	if recorder.Code != http.StatusBadRequest {
-		t.Fatalf("invalid tag status = %d, want 400", recorder.Code)
-	}
-}
-
-func TestAdminReleaseBuilderIsUnavailableWhenUnconfigured(t *testing.T) {
-	s := &Server{}
-	recorder := httptest.NewRecorder()
-	s.handleAdminReleaseBuilderStatus(recorder, httptest.NewRequest(http.MethodGet, "/admin/api/release-builder", nil))
-	if recorder.Code != http.StatusServiceUnavailable {
-		t.Fatalf("unconfigured builder status = %d, want 503", recorder.Code)
-	}
-}

server/internal/cache/cache.go

@@ -88,5 +88,11 @@ func UserKey(userID, view string) string { return fmt.Sprintf("u:%s:%s", userID,
 // invalidation and expire on their own slow-moving daily cadence.
 func RecommendationsKey(userID string) string { return fmt.Sprintf("r:%s:rows:v3", userID) }
 
+// MagicPoolKey sits outside the `u:` namespace for the same reason RecommendationsKey
+// does, and one more besides: a Magic press *is* a playback change, so a pool filed under
+// the user's ordinary views would be invalidated by the very press that read it and every
+// press would pay the full rebuild.
+func MagicPoolKey(userID string) string { return fmt.Sprintf("m:%s:pool:v1", userID) }
+
 // SessionKey caches a token→session lookup, keyed by token hash (never the token).
 func SessionKey(tokenHashHex string) string { return "sess:" + tokenHashHex }

server/internal/config/config.go

@@ -68,6 +68,12 @@ type Config struct {
 	// RecommendTimeout bounds a background rebuild, which fans out further than a
 	// normal request and so needs more headroom than UpstreamTimeout.
 	RecommendTimeout time.Duration
+	// MagicPoolTTL is how long Magic's scored pool stays warm. Shorter than
+	// RecommendTTL, which is a daily rotation nobody is waiting on: this one is
+	// rebuilt in front of somebody standing at a player with the film paused
+	// behind a loading panel, and a household that has just acquired something
+	// should be able to be handed it the same evening.
+	MagicPoolTTL time.Duration
 	// RecommendationWeights is an optional JSON overlay on the weighted defaults.
 	RecommendationWeights string
 	// RemoteConfig is the complete, validated presentation document served to TVs.
@@ -97,9 +103,6 @@ type Config struct {
 	// ReleasePublishToken authorizes the CI-only release upload endpoint. It is separate
 	// from AdminToken so a compromised build runner cannot change maintenance settings.
 	ReleasePublishToken string
-	// ReleaseBuilderURL is the private Compose address of the Android release controller.
-	// It is never given to the browser; the authenticated admin API relays requests to it.
-	ReleaseBuilderURL string
 
 	// SyncInterval is how often the library import runs. Zero disables the schedule.
 	SyncInterval time.Duration
@@ -221,6 +224,7 @@ func Load() (Config, error) {
 		SessionIdleExpiry: duration("MEMBY_SESSION_IDLE_EXPIRY", 90*24*time.Hour),
 		RecommendTTL:      duration("MEMBY_RECOMMEND_TTL", 24*time.Hour),
 		RecommendTimeout:  duration("MEMBY_RECOMMEND_TIMEOUT", 60*time.Second),
+		MagicPoolTTL:      duration("MEMBY_MAGIC_POOL_TTL", 2*time.Hour),
 		RecommendationWeights: strings.TrimSpace(
 			os.Getenv("MEMBY_RECOMMENDATION_WEIGHTS"),
 		),
@@ -231,7 +235,6 @@ func Load() (Config, error) {
 		PublicURL:               strings.TrimRight(strings.TrimSpace(os.Getenv("MEMBY_PUBLIC_URL")), "/"),
 		ReleaseDir:              env("MEMBY_RELEASE_DIR", "/data/releases"),
 		ReleasePublishToken:     releasePublishToken,
-		ReleaseBuilderURL:       strings.TrimRight(strings.TrimSpace(os.Getenv("MEMBY_RELEASE_BUILDER_URL")), "/"),
 		SyncInterval:            duration("MEMBY_SYNC_INTERVAL", time.Hour),
 		SyncTimeout:             duration("MEMBY_SYNC_TIMEOUT", 30*time.Minute),
 		SyncOnStart:             boolean("MEMBY_SYNC_ON_START", false),

server/internal/recommend/magic.go

@@ -30,6 +30,14 @@ const (
 	// nothing genuinely unsuitable can be drawn.
 	MagicPoolLimit = 40
 
+	// MagicPoolReserve is how many scored titles [Engine.MagicPool] keeps, which is
+	// deliberately several times the hat. The pool is built once and drawn from many
+	// times, and every press narrows it further — the film playing now and the last few
+	// this button offered come out, and a viewer who said how long they had re-ranks
+	// what is left. Reserving only the hat's own size would leave a household with
+	// nothing to draw after a handful of presses.
+	MagicPoolReserve = MagicPoolLimit * 3
+
 	// magicUnwatchedBonus is the largest single term, because "something I have not seen"
 	// is most of what somebody means by the button.
 	magicUnwatchedBonus = 1.4
@@ -63,13 +71,47 @@ type MagicOptions struct {
 	// draw is deterministic under test — and so that the *only* non-deterministic thing
 	// about this feature sits in one named parameter.
 	Roll float64
-	// Now is injectable for the same reason.
+	// Now is injectable for the same reason. It reaches the pool rather than the draw —
+	// the only thing it decides is what counts as recently added.
 	Now time.Time
 }
 
+// MagicCandidate is one title already weighed, reduced to what a draw needs and nothing
+// more.
+//
+// It exists because the two halves of this feature have completely different costs. Working
+// out what the viewer likes is two full reads of their Emby history plus a catalogue query;
+// drawing from the result is arithmetic over a few dozen numbers. Separating them is what
+// lets the expensive half be done once and kept, while every press still gets its own
+// genuinely unpredictable answer — the property the button cannot lose. It is JSON-tagged
+// because being cached is the whole point of the separation.
+type MagicCandidate struct {
+	ItemID string `json:"itemId"`
+	// Title is carried so a draw can be logged by name without re-reading the item.
+	Title string `json:"title"`
+	// Score is everything the profile had to say, which is fixed for as long as the pool
+	// is. The request-scoped terms are applied at the draw.
+	Score float64 `json:"score"`
+	// RuntimeMinutes is kept rather than folded into the score because "there is an hour
+	// before bed" is a property of the press, not of the title.
+	RuntimeMinutes int `json:"runtimeMinutes,omitempty"`
+	// Signals is why this title was eligible, in machine-readable slugs. Not shown.
+	Signals []string `json:"signals,omitempty"`
+	// Reasons is viewer-facing wording from the same explanation layer a detail page
+	// uses. It is computed here rather than at the draw because it needs the profile,
+	// which is exactly what the pool exists to avoid rebuilding.
+	Reasons []string `json:"reasons,omitempty"`
+}
+
 // MagicSelection is one drawn title with its evidence.
+//
+// It carries the item's id and name rather than the item itself: a draw may be made from a
+// pool built hours ago, and the caller re-reads the record it is about to hand a television
+// regardless — which is one lookup, against a title somebody is about to watch for two
+// hours.
 type MagicSelection struct {
-	Item Item
+	ItemID string
+	Title  string
 	// Reasons is viewer-facing wording from the same explanation layer a detail page uses.
 	Reasons []string
 	// Signals is why this title was *eligible*, in machine-readable slugs, so the choice
@@ -82,38 +124,70 @@ type MagicSelection struct {
 	PoolSize int
 }
 
-// MagicPick gathers the signals and draws. Errors only when the profile cannot be built at
-// all and the catalogue is empty with it — every lesser failure degrades, on the principle
-// [Engine.RelatedTo] already applies: a button that sometimes does nothing is worse than one
-// that occasionally picks less well.
-func (e *Engine) MagicPick(
+// MagicPool does the expensive half: the taste profile, the candidate query and the
+// weighing. Nothing about it is request-scoped, which is what makes it safe to keep.
+//
+// It never errors. A profile that cannot be built costs the weighting and not the button,
+// on the principle [Engine.RelatedTo] already applies — an empty pool is the one failure,
+// and it means a household with no films rather than a server having trouble.
+func (e *Engine) MagicPool(
 	ctx context.Context,
 	cred emby.Credentials,
-	opts MagicOptions,
-) (MagicSelection, bool) {
-	if opts.Now.IsZero() {
-		opts.Now = e.now()
+	now time.Time,
+) []MagicCandidate {
+	if now.IsZero() {
+		now = e.now()
 	}
 
 	history, favorites, err := e.gatherSignals(ctx, cred)
 	if err != nil {
-		// A profile that cannot be built costs the weighting, not the button. What is left
-		// is an unweighted draw over the catalogue, which is still "put something on".
+		// What is left is an unweighted draw over the catalogue, which is still "put
+		// something on".
 		e.log.Warn("magic signals unavailable; drawing without taste", "error", err)
 	}
 	profile := BuildProfile(history, favorites)
 
 	candidates := e.magicCandidates(ctx, cred, profile)
 	if len(candidates) == 0 {
-		return MagicSelection{}, false
+		return nil
 	}
 
-	selection, ok := ChooseMagic(profile, candidates, opts)
-	if !ok {
-		return MagicSelection{}, false
+	pool := make([]MagicCandidate, 0, len(candidates))
+	byID := make(map[string]Item, len(candidates))
+	for _, candidate := range candidates {
+		if candidate.ID == "" || byID[candidate.ID].ID != "" {
+			continue
+		}
+		byID[candidate.ID] = candidate
+		score, signals := magicScore(profile, candidate, now)
+		pool = append(pool, MagicCandidate{
+			ItemID:         candidate.ID,
+			Title:          candidate.Name,
+			Score:          score,
+			RuntimeMinutes: candidate.RuntimeMinutes(),
+			Signals:        signals,
+		})
 	}
-	selection.Reasons = Why(profile, selection.Item, 2)
-	return selection, true
+	sortMagicPool(pool)
+	if len(pool) > MagicPoolReserve {
+		pool = pool[:MagicPoolReserve]
+	}
+	// Worded only for what survived the reserve: the explanation layer runs per title, and
+	// wording several hundred nobody will ever be offered is work thrown away.
+	for i := range pool {
+		pool[i].Reasons = Why(profile, byID[pool[i].ItemID], 2)
+	}
+	return pool
+}
+
+// MagicPick builds a pool and draws from it in one go — the whole feature for a caller with
+// nowhere to keep the pool, and what the tests exercise.
+func (e *Engine) MagicPick(
+	ctx context.Context,
+	cred emby.Credentials,
+	opts MagicOptions,
+) (MagicSelection, bool) {
+	return ChooseMagic(e.MagicPool(ctx, cred, opts.Now), opts)
 }
 
 // magicCandidates prefers the imported catalogue, which costs Postgres one read rather than
@@ -175,7 +249,7 @@ func onlyMovies(items []Item) []Item {
 // household would get the same film every night, which is the one outcome the button cannot
 // have. Ranking then *drawing from the ranking* keeps merit deciding which titles are in the
 // hat and how many tickets each holds, while leaving the answer genuinely unpredictable.
-func ChooseMagic(profile Profile, candidates []Item, opts MagicOptions) (MagicSelection, bool) {
+func ChooseMagic(candidates []MagicCandidate, opts MagicOptions) (MagicSelection, bool) {
 	excluded := map[string]bool{}
 	for _, id := range opts.ExcludeIDs {
 		if id = strings.TrimSpace(id); id != "" {
@@ -183,32 +257,28 @@ func ChooseMagic(profile Profile, candidates []Item, opts MagicOptions) (MagicSe
 		}
 	}
 
-	type scored struct {
-		item    Item
-		score   float64
-		signals []string
-	}
-	pool := make([]scored, 0, len(candidates))
+	pool := make([]MagicCandidate, 0, len(candidates))
 	seen := map[string]bool{}
 	for _, candidate := range candidates {
-		if candidate.ID == "" || excluded[candidate.ID] || seen[candidate.ID] {
+		if candidate.ItemID == "" || excluded[candidate.ItemID] || seen[candidate.ItemID] {
 			continue
 		}
-		seen[candidate.ID] = true
-		score, signals := magicScore(profile, candidate, opts)
-		pool = append(pool, scored{item: candidate, score: score, signals: signals})
+		seen[candidate.ItemID] = true
+		// Only the terms that belong to this press: everything the profile had to say is
+		// already in the score the pool was built with.
+		if adjustment, signal := magicRuntimeAdjustment(
+			candidate.RuntimeMinutes, opts.AvailableMinutes,
+		); signal != "" {
+			candidate.Score += adjustment
+			candidate.Signals = append(append([]string(nil), candidate.Signals...), signal)
+		}
+		pool = append(pool, candidate)
 	}
 	if len(pool) == 0 {
 		return MagicSelection{}, false
 	}
 
-	sort.SliceStable(pool, func(i, j int) bool {
-		if pool[i].score != pool[j].score {
-			return pool[i].score > pool[j].score
-		}
-		// Ties break by id so the *pool* is reproducible even though the draw is not.
-		return pool[i].item.ID < pool[j].item.ID
-	})
+	sortMagicPool(pool)
 	if len(pool) > MagicPoolLimit {
 		pool = pool[:MagicPoolLimit]
 	}
@@ -226,27 +296,42 @@ func ChooseMagic(profile Profile, candidates []Item, opts MagicOptions) (MagicSe
 	for index, entry := range pool {
 		cumulative += float64(len(pool) - index)
 		if target < cumulative {
-			return MagicSelection{
-				Item:     entry.item,
-				Signals:  entry.signals,
-				Score:    entry.score,
-				PoolSize: len(pool),
-			}, true
+			return magicSelection(entry, len(pool)), true
 		}
 	}
-	last := pool[len(pool)-1]
-	return MagicSelection{
-		Item:     last.item,
-		Signals:  last.signals,
-		Score:    last.score,
-		PoolSize: len(pool),
-	}, true
+	return magicSelection(pool[len(pool)-1], len(pool)), true
 }
 
-// magicScore sums the stated terms and reports which of them fired. The signals are the
-// point of returning two values: a weighting nobody can see the workings of is a weighting
-// nobody can improve.
-func magicScore(profile Profile, item Item, opts MagicOptions) (float64, []string) {
+func magicSelection(entry MagicCandidate, poolSize int) MagicSelection {
+	return MagicSelection{
+		ItemID:   entry.ItemID,
+		Title:    entry.Title,
+		Reasons:  entry.Reasons,
+		Signals:  entry.Signals,
+		Score:    entry.Score,
+		PoolSize: poolSize,
+	}
+}
+
+// sortMagicPool orders by merit, with ties broken by id so the *pool* is reproducible even
+// though the draw from it is not. It is one function because the pool is ordered twice — as
+// it is built and again after a press has adjusted it — and two copies of a comparison is
+// how the two orders come to disagree.
+func sortMagicPool(pool []MagicCandidate) {
+	sort.SliceStable(pool, func(i, j int) bool {
+		if pool[i].Score != pool[j].Score {
+			return pool[i].Score > pool[j].Score
+		}
+		return pool[i].ItemID < pool[j].ItemID
+	})
+}
+
+// magicScore sums the terms that belong to the *title*, and reports which of them fired.
+// The signals are the point of returning two values: a weighting nobody can see the
+// workings of is a weighting nobody can improve.
+//
+// The runtime fit is deliberately not here — see [magicRuntimeAdjustment].
+func magicScore(profile Profile, item Item, now time.Time) (float64, []string) {
 	signals := make([]string, 0, 6)
 	score := profile.Affinity(item)
 	if score > 0 {
@@ -266,28 +351,33 @@ func magicScore(profile Profile, item Item, opts MagicOptions) (float64, []strin
 		signals = append(signals, "favourite")
 	}
 
-	if addedDays, ok := daysSince(item.DateCreated, opts.Now); ok && addedDays <= magicRecentlyAddedDays {
+	if addedDays, ok := daysSince(item.DateCreated, now); ok && addedDays <= magicRecentlyAddedDays {
 		score += magicRecentlyAddedBonus
 		signals = append(signals, "recently_added")
 	}
 
-	if opts.AvailableMinutes > 0 {
-		switch runtime := item.RuntimeMinutes(); {
-		case runtime <= 0:
-			// Nothing recorded is not evidence either way, and refusing to draw it would
-			// quietly delete a slice of the library from the feature.
-		case runtime > opts.AvailableMinutes+magicRuntimeSlackMinutes:
-			score -= magicRuntimeOverPenalty
-			signals = append(signals, "too_long")
-		default:
-			score += magicRuntimeFitBonus
-			signals = append(signals, "fits_time")
-		}
-	}
-
 	return score, signals
 }
 
+// magicRuntimeAdjustment is how "there is an hour before bed" gets a different answer from
+// "it is Saturday afternoon". It is applied at the draw rather than folded into the pool
+// because it belongs to the press: the same pool has to be able to answer both questions.
+//
+// An empty signal means the term did not apply at all, which covers both "no limit was
+// given" and "this title has no runtime recorded" — nothing recorded is not evidence
+// either way, and refusing to draw it would quietly delete a slice of the library from the
+// feature.
+func magicRuntimeAdjustment(runtimeMinutes, availableMinutes int) (float64, string) {
+	switch {
+	case availableMinutes <= 0, runtimeMinutes <= 0:
+		return 0, ""
+	case runtimeMinutes > availableMinutes+magicRuntimeSlackMinutes:
+		return -magicRuntimeOverPenalty, "too_long"
+	default:
+		return magicRuntimeFitBonus, "fits_time"
+	}
+}
+
 // daysSince reads Emby's ISO-8601 DateCreated. A field that is absent or unreadable is not
 // an error: it simply cannot earn the recently-added bonus.
 func daysSince(value string, now time.Time) (int, bool) {

server/internal/recommend/magic_test.go

@@ -0,0 +1,168 @@
+package recommend
+
+import (
+	"encoding/json"
+	"testing"
+)
+
+func magicCandidate(id string, score float64, runtimeMinutes int) MagicCandidate {
+	return MagicCandidate{ItemID: id, Title: id, Score: score, RuntimeMinutes: runtimeMinutes}
+}
+
+// The pool is kept between presses, so it has to survive the round trip that keeping it
+// means. A field that lost its tag would show up as a button that quietly stopped weighing
+// anything rather than as an error.
+func TestMagicCandidateSurvivesBeingKept(t *testing.T) {
+	pool := []MagicCandidate{{
+		ItemID: "1", Title: "A Film", Score: 2.5, RuntimeMinutes: 104,
+		Signals: []string{"unwatched"}, Reasons: []string{"Because you like Drama"},
+	}}
+	raw, err := json.Marshal(pool)
+	if err != nil {
+		t.Fatalf("marshal: %v", err)
+	}
+	var restored []MagicCandidate
+	if err := json.Unmarshal(raw, &restored); err != nil {
+		t.Fatalf("unmarshal: %v", err)
+	}
+	if len(restored) != 1 {
+		t.Fatalf("pool length = %d, want 1", len(restored))
+	}
+	got, want := restored[0], pool[0]
+	if got.ItemID != want.ItemID || got.Title != want.Title || got.Score != want.Score ||
+		got.RuntimeMinutes != want.RuntimeMinutes ||
+		len(got.Signals) != len(want.Signals) || len(got.Reasons) != len(want.Reasons) {
+		t.Fatalf("pool did not survive being kept: %+v", got)
+	}
+}
+
+func TestChooseMagicNeverReturnsAnExcludedTitle(t *testing.T) {
+	pool := []MagicCandidate{
+		magicCandidate("playing-now", 9, 0),
+		magicCandidate("offered-before", 8, 0),
+		magicCandidate("fresh", 1, 0),
+	}
+	for roll := 0.0; roll < 1; roll += 0.01 {
+		selection, ok := ChooseMagic(pool, MagicOptions{
+			ExcludeIDs: []string{"playing-now", " offered-before "},
+			Roll:       roll,
+		})
+		if !ok {
+			t.Fatalf("roll %.2f: expected a pick", roll)
+		}
+		if selection.ItemID != "fresh" {
+			t.Fatalf("roll %.2f: drew an excluded title %q", roll, selection.ItemID)
+		}
+	}
+}
+
+// A household that has been offered everything the pool holds is the one case the button
+// has no answer for, and it must say so rather than repeat itself.
+func TestChooseMagicRefusesWhenEverythingIsExcluded(t *testing.T) {
+	pool := []MagicCandidate{magicCandidate("only", 3, 0)}
+	if _, ok := ChooseMagic(pool, MagicOptions{ExcludeIDs: []string{"only"}, Roll: 0.5}); ok {
+		t.Fatal("expected no pick when the whole pool is excluded")
+	}
+	if _, ok := ChooseMagic(nil, MagicOptions{Roll: 0.5}); ok {
+		t.Fatal("expected no pick from an empty pool")
+	}
+}
+
+// The whole reason for drawing rather than sorting: pressing it twice must be able to give
+// two answers, while merit still decides how many tickets each title holds.
+func TestChooseMagicFavoursMeritWithoutBeingAForegoneConclusion(t *testing.T) {
+	pool := make([]MagicCandidate, 0, 10)
+	for index := 0; index < 10; index++ {
+		pool = append(pool, magicCandidate(string(rune('a'+index)), float64(10-index), 0))
+	}
+	counts := map[string]int{}
+	for roll := 0.0; roll < 1; roll += 0.001 {
+		selection, ok := ChooseMagic(pool, MagicOptions{Roll: roll})
+		if !ok {
+			t.Fatalf("roll %.3f: expected a pick", roll)
+		}
+		counts[selection.ItemID]++
+	}
+	if len(counts) != len(pool) {
+		t.Fatalf("every title should be reachable, got %d of %d", len(counts), len(pool))
+	}
+	if counts["a"] <= counts["j"] {
+		t.Fatalf("the best title should hold the most tickets: %v", counts)
+	}
+}
+
+// The pool is built once and asked more than one question, so the time budget cannot have
+// been folded into it. The same pool has to answer "there is an hour" differently from
+// "it is Saturday afternoon".
+func TestChooseMagicAppliesTheTimeBudgetAtTheDraw(t *testing.T) {
+	pool := []MagicCandidate{
+		magicCandidate("epic", 1.0, 180),
+		magicCandidate("short", 0.6, 85),
+	}
+	unhurried, ok := ChooseMagic(pool, MagicOptions{Roll: 0})
+	if !ok || unhurried.ItemID != "epic" {
+		t.Fatalf("with no limit the better title should lead, got %+v", unhurried)
+	}
+	rushed, ok := ChooseMagic(pool, MagicOptions{AvailableMinutes: 90, Roll: 0})
+	if !ok || rushed.ItemID != "short" {
+		t.Fatalf("with 90 minutes the one that fits should lead, got %+v", rushed)
+	}
+	if !hasSignal(rushed.Signals, "fits_time") {
+		t.Fatalf("the fit should be reported as a signal: %v", rushed.Signals)
+	}
+	// And the pool itself must be unchanged by having been asked, or the second press
+	// would inherit the first press's constraints.
+	if pool[0].Score != 1.0 || len(pool[0].Signals) != 0 {
+		t.Fatalf("the draw mutated the kept pool: %+v", pool[0])
+	}
+}
+
+// Nothing recorded is not evidence either way: refusing those titles would quietly delete
+// a slice of the library from the feature.
+func TestMagicRuntimeAdjustmentStaysSilentWithoutEvidence(t *testing.T) {
+	cases := []struct {
+		name                             string
+		runtimeMinutes, availableMinutes int
+		wantSignal                       string
+	}{
+		{"no limit given", 200, 0, ""},
+		{"no runtime recorded", 0, 60, ""},
+		{"comfortably inside", 85, 90, "fits_time"},
+		{"inside the slack", 95, 90, "fits_time"},
+		{"past the slack", 101, 90, "too_long"},
+	}
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			_, signal := magicRuntimeAdjustment(tc.runtimeMinutes, tc.availableMinutes)
+			if signal != tc.wantSignal {
+				t.Fatalf("signal = %q, want %q", signal, tc.wantSignal)
+			}
+		})
+	}
+}
+
+// Ties break by id so that the pool is reproducible even though the draw from it is not.
+func TestSortMagicPoolIsReproducible(t *testing.T) {
+	pool := []MagicCandidate{
+		magicCandidate("z", 2, 0),
+		magicCandidate("a", 2, 0),
+		magicCandidate("m", 5, 0),
+	}
+	sortMagicPool(pool)
+	got := []string{pool[0].ItemID, pool[1].ItemID, pool[2].ItemID}
+	want := []string{"m", "a", "z"}
+	for i := range want {
+		if got[i] != want[i] {
+			t.Fatalf("order = %v, want %v", got, want)
+		}
+	}
+}
+
+func hasSignal(signals []string, want string) bool {
+	for _, signal := range signals {
+		if signal == want {
+			return true
+		}
+	}
+	return false
+}