0.2.59 - Settings save fixes

2026-08-12 15:39:08 +12:00
parent 613f203cf9
commit 4fb68f4bbc
14 changed files with 227 additions and 25 deletions
@@ -1,3 +1,9 @@
+## 0.2.59 — 2026-08-12
+- Fixed: Moving up through the on-screen keyboard on the Requests page no longer jumps straight to the tabs — only the top row of letters leaves for them.
+- Improved: Requests now waits for five letters, and for a short pause in typing, before looking anything up.
+- Fixed: The featured cards at the top of Home no longer lead with a film you have already watched. Series are unchanged, so a new season of something you are up to date with still features.
+- Fixed: Saving a setting could fail repeatedly with a server error.
+
 ## 0.2.58 — 2026-08-12
 - Added: Improved Requests option to the user selector menu.

@@ -42,7 +42,7 @@ val projectNoticeText =

 // A release workflow can derive the app version from its Git tag without editing the
 // source tree. Local builds keep using the checked-in default.
-val defaultVersionName = "0.2.58"
+val defaultVersionName = "0.2.59"
 val membyVersionName: String =
    (project.findProperty("memby.versionName") as String?)
        ?.trim()
@@ -132,6 +132,27 @@ private const val LABEL_LIBRARY = "FROM YOUR LIBRARY"
 /** The `kind` of the server-composed hero row. It is consumed here, never drawn as a row. */
 internal const val SERVER_HERO_ROW_KIND = "hero"

+/**
+ * Whether a card is something to lead the launcher with tonight.
+ *
+ * A film somebody has already finished is not: the hero exists to be pressed, and the
+ * answer to "watch this" cannot be a title they watched last week. This holds however the
+ * card was chosen, and regardless of the *Hide films you have seen* preference — that one
+ * is about shelves, and a shelf of a hundred cards can afford to carry one somebody has
+ * seen where a row of four cannot.
+ *
+ * **A series is deliberately left alone.** Being marked watched there means somebody is up
+ * to date, which is exactly who a season premiere is news for — the whole reason the
+ * gateway can lead with a returning show.
+ *
+ * The gateway applies the same rule when it composes the row, so this is the second half of
+ * one decision rather than a disagreement with it: what it catches is the launcher a
+ * television draws from its cache before the first refresh lands, and a gateway older than
+ * the rule.
+ */
+internal fun heroWorthLeadingWith(item: BaseItem): Boolean =
+    !(item.isMovie && item.userData?.played == true)
+
 /**
 * The hero the gateway composed, if it sent one.
 *
@@ -151,6 +172,7 @@ internal fun serverHeroPicks(rows: List<HomeRow>): List<HomeHeroPick> =
        .filter { it.kind == SERVER_HERO_ROW_KIND }
        .flatMap { it.items.asSequence() }
        .filter { it.membyPlayable }
+        .filter(::heroWorthLeadingWith)
        .distinctBy(BaseItem::id)
        .take(4)
        .map { item ->
@@ -191,17 +213,20 @@ internal fun selectHomeHeroMovies(
        return words.any(label::contains)
    }

+    // Unwatched throughout: see heroWorthLeadingWith. It is applied to each list rather
+    // than to the rows so the rotation still counts in cards that can actually lead.
+    fun List<HomeBrowseRow>.unwatchedMovies() =
+        flatMap(HomeBrowseRow::items).filter(BaseItem::isMovie).filter(::heroWorthLeadingWith)
+
    val newReleases = rows
        .filter { it.matches("latest", "recent", "new release", "just added") }
-        .flatMap(HomeBrowseRow::items)
-        .filter(BaseItem::isMovie)
+        .unwatchedMovies()
        .rotatedBy(day)
    val popular = rows
        .filter { it.matches("popular", "trending", "recommended", "top pick") }
-        .flatMap(HomeBrowseRow::items)
-        .filter(BaseItem::isMovie)
+        .unwatchedMovies()
        .rotatedBy(day)
-    val everyMovie = rows.flatMap(HomeBrowseRow::items).filter(BaseItem::isMovie).rotatedBy(day)
+    val everyMovie = rows.unwatchedMovies().rotatedBy(day)

    fun List<BaseItem>.labelled(label: String) = map { HomeHeroPick(it, label) }

@@ -459,11 +459,12 @@ private fun DiscoverPane(
            QueryLine(state)
            Spacer(Modifier.height(12.dp))
            Box(
-                Modifier
-                    // The keyboard is the pane's entry point and its top-left control, so
-                    // Down from the tabs lands on it and Up from it goes back.
-                    .focusRequester(paneFocusRequester)
-                    .focusProperties { up = tabsFocusRequester },
+                // The keyboard is the pane's entry point, so Down from the tabs lands on it.
+                // Only the *entry* is declared here: an `up` hung off this Box would be
+                // inherited by every key underneath it, which is what made Up anywhere in
+                // the letters leave for the tab strip instead of moving one row up. The
+                // escape belongs to the top row alone, and the keyboard states it per key.
+                Modifier.focusRequester(paneFocusRequester),
            ) {
                TvKeyboard(
                    navigationFocusRequester = navigationFocusRequester,
@@ -476,6 +477,7 @@ private fun DiscoverPane(
                    onCharacter = onAppendToQuery,
                    onBackspace = onBackspace,
                    onClear = onClearQuery,
+                    upTarget = tabsFocusRequester,
                )
            }
        }
@@ -267,16 +267,24 @@ class RequestsViewModel(private val repository: EmbyRepository) : ViewModel() {
    fun dismissNotice() = _state.update { it.copy(notice = null) }

    companion object {
-        const val DEBOUNCE_MS = 300L
+        /**
+         * Longer than the Search tab's, because the thing at the other end is different.
+         * Typing on a remote is around half a second a letter, so a 300 ms window let every
+         * keystroke past the threshold through as its own live Radarr and Sonarr query.
+         */
+        const val DEBOUNCE_MS = 700L

        /**
-         * Three characters, not the Search tab's two.
+         * Five characters, not the Search tab's two.
         *
-         * Every keystroke here reaches Radarr and Sonarr rather than a local index, and a
-         * two-letter prefix returns a hundred films nobody meant while costing two live
-         * provider queries. The gateway independently refuses under two.
+         * Every keystroke here reaches Radarr and Sonarr rather than a local index, so a
+         * viewer typing a title on a remote spends a live provider query per letter — and
+         * the early ones are worthless: a three-letter prefix returns a hundred films
+         * nobody meant. Five is roughly where a prefix starts naming something, so the
+         * lookups begin when they can answer rather than as soon as they are legal. The
+         * gateway independently refuses under two.
         */
-        const val MIN_QUERY_LENGTH = 3
+        const val MIN_QUERY_LENGTH = 5
    }
 }

@@ -511,6 +511,14 @@ internal fun TvKeyboard(
    onCharacter: (String) -> Unit,
    onBackspace: () -> Unit,
    onClear: () -> Unit,
+    /**
+     * Where Up goes from the **top row of letters**, for a host that has something above the
+     * keyboard — the Requests page's tab strip. It is stated per key rather than declared on
+     * a parent, because `focusProperties` is inherited by every descendant: hung off a Box
+     * around the keyboard it applies to all thirty-nine keys, so Up anywhere in the letters
+     * leaves for the tabs instead of moving one row up.
+     */
+    upTarget: FocusRequester? = null,
 ) {
    Column(verticalArrangement = Arrangement.spacedBy(8.dp)) {
        KeyboardRows.forEachIndexed { rowIndex, row ->
@@ -532,6 +540,7 @@ internal fun TvKeyboard(
                                if (columnIndex == KEYBOARD_COLUMNS - 1) {
                                    right = if (hasResultsTarget) resultsEntry else FocusRequester.Cancel
                                }
+                                if (rowIndex == 0) upTarget?.let { up = it }
                            }
                            .then(
                                if (index == 0) Modifier.focusRequester(keyboardEntry) else Modifier,
@@ -5,6 +5,7 @@ import com.ponzischeme89.memby.data.localEpochDay
 import com.ponzischeme89.memby.data.millisUntilNextLocalDay
 import com.ponzischeme89.memby.data.model.BaseItem
 import com.ponzischeme89.memby.data.model.HomeRow
+import com.ponzischeme89.memby.data.model.UserItemData
 import org.junit.Assert.assertEquals
 import org.junit.Assert.assertTrue
 import org.junit.Test
@@ -259,6 +260,58 @@ class HomeMovieHeroTest {
        }
    }

+    // --- Watched films -------------------------------------------------------------
+
+    /**
+     * The hero is four cards answering "watch this tonight". A film already finished is
+     * not an answer to that, however new it is or however well it was reviewed.
+     */
+    @Test
+    fun `a film already watched never leads the launcher`() {
+        val rows = listOf(
+            HomeBrowseRow(
+                id = "latest-movies",
+                title = "Recently Added Movies",
+                items = listOf(
+                    movie("seen", watched = true),
+                    movie("partway", positionTicks = 6_000_000_000L),
+                    movie("unseen"),
+                ),
+                kind = MediaRowKind.MOVIES,
+                emptyMessage = "",
+            ),
+        )
+
+        assertEquals(
+            listOf("partway", "unseen"),
+            selectHomeHeroMovies(rows).map { it.item.id },
+        )
+    }
+
+    /**
+     * The gateway applies the same rule, so this is the launcher a television draws from
+     * its cache before the first refresh — and any gateway older than the rule.
+     */
+    @Test
+    fun `a watched film is dropped from the row the gateway sent`() {
+        val row = heroRow(
+            movie("seen", watched = true).copy(membyHeroLabel = "NEW RELEASE"),
+            movie("unseen").copy(membyHeroLabel = "NEW RELEASE"),
+        )
+
+        assertEquals(listOf("unseen"), serverHeroPicks(listOf(row)).map { it.item.id })
+    }
+
+    /** A show marked watched is somebody up to date — exactly who a new season is for. */
+    @Test
+    fun `a watched series still leads`() {
+        val row = heroRow(
+            movie("show", watched = true).copy(type = "Series", membyHeroLabel = "NEW SEASON"),
+        )
+
+        assertEquals(listOf("show"), serverHeroPicks(listOf(row)).map { it.item.id })
+    }
+
    // --- Midnight ----------------------------------------------------------------

    /** Local, not UTC: the day must turn over at the viewer's midnight. */
@@ -308,6 +361,17 @@ class HomeMovieHeroTest {
        const val DAY_MS = 24L * HOUR_MS
    }

+    private fun movie(
+        id: String,
+        watched: Boolean = false,
+        positionTicks: Long = 0L,
+    ) = BaseItem(
+        id = id,
+        name = id,
+        type = "Movie",
+        userData = UserItemData(played = watched, playbackPositionTicks = positionTicks),
+    )
+
    private fun heroItem(id: String, label: String?, reason: String?) = BaseItem(
        id = id,
        name = id,
@@ -95,9 +95,10 @@ class RequestPresentationTest {
    fun `the lookup threshold is stricter than the library search's`() {
        // Every keystroke here reaches Radarr and Sonarr rather than a local index.
        assertFalse(shouldLookup("du"))
-        assertTrue(shouldLookup("dun"))
+        assertFalse(shouldLookup("dune"))
+        assertTrue(shouldLookup("dunes"))
        assertFalse(shouldLookup("  a  "))
-        assertTrue(shouldLookup("  dune  "))
+        assertTrue(shouldLookup("  dune part two  "))
    }

    @Test
@@ -414,6 +414,10 @@ func clientLogValue(value string) string {
 // visible regardless of path.
 func requestLogLevel(path string, status int) slog.Level {
 	switch {
+	// A request nobody is waiting for any more is not a failure of anything. It is only
+	// ever answered this way deliberately, so it never hides a fault.
+	case status == statusClientClosedRequest:
+		return slog.LevelDebug
 	case status >= http.StatusInternalServerError:
 		return slog.LevelError
 	case status >= http.StatusBadRequest:
@@ -557,6 +561,11 @@ func writeRaw(w http.ResponseWriter, status int, body []byte) {
 	_, _ = w.Write(body)
 }

+// statusClientClosedRequest is nginx's 499. Go has no constant for it because it is not
+// in the RFC — it exists to say "this was not answered, and that is nobody's fault",
+// which is a distinction a log is read for and a 5xx destroys.
+const statusClientClosedRequest = 499
+
 func writeError(w http.ResponseWriter, status int, message string) {
 	writeJSON(w, status, map[string]string{"error": message})
 }
@@ -566,6 +575,17 @@ func writeError(w http.ResponseWriter, status int, message string) {
 func (s *Server) writeUpstreamError(
 	ctx context.Context, w http.ResponseWriter, err error, message string,
 ) {
+	// The television having navigated on is not a fault, and it is the ordinary case here:
+	// artwork loaders abandon requests as cards leave the screen, and a detail page warmed
+	// on focus is cancelled the moment the D-pad moves. Reported as 502 it filled the
+	// operator's log with errors describing a launcher working exactly as designed, and
+	// buried the ones that meant something. Nobody is left to read the answer, so it goes
+	// out as 499 — nginx's "client closed request" — and is recorded at DEBUG.
+	if clientGaveUp(ctx, err) {
+		s.loggerFor(ctx).Debug("abandoned before the answer", "detail", message, "error", err)
+		writeError(w, statusClientClosedRequest, "the request was abandoned")
+		return
+	}
 	var apiErr *emby.APIError
 	if errors.As(err, &apiErr) {
 		switch {
@@ -452,6 +452,10 @@ type heroItemFacts struct {
 	Type     string
 	Premiere time.Time
 	Playable bool
+
+	// Watched is Emby's own answer for this viewer. The rows are fetched per person and
+	// carry their user data, so it costs nothing to read.
+	Watched bool
 }

 func heroFactsOf(raw json.RawMessage) (heroItemFacts, bool) {
@@ -462,6 +466,9 @@ func heroFactsOf(raw json.RawMessage) (heroItemFacts, bool) {
 		PremiereDate string `json:"PremiereDate"`
 		Source       string `json:"MembySource"`
 		Playable     *bool  `json:"MembyPlayable"`
+		UserData     struct {
+			Played bool `json:"Played"`
+		} `json:"UserData"`
 	}
 	if json.Unmarshal(raw, &payload) != nil || strings.TrimSpace(payload.ID) == "" {
 		return heroItemFacts{}, false
@@ -474,6 +481,7 @@ func heroFactsOf(raw json.RawMessage) (heroItemFacts, bool) {
 		// Anything from Emby carries neither field, and is.
 		Playable: strings.TrimSpace(payload.Source) == "" &&
 			(payload.Playable == nil || *payload.Playable),
+		Watched: payload.UserData.Played,
 	}
 	if parsed, err := parseEmbyDate(payload.PremiereDate); err == nil {
 		facts.Premiere = parsed
@@ -837,6 +845,18 @@ func heroMovieCandidates(rows []recommend.Row) ([]heroCandidate, map[string]hero
 				!strings.EqualFold(fact.Type, "Movie") {
 				continue
 			}
+			// A film somebody has already seen is not something to lead the launcher
+			// with: the hero exists to be pressed, and the answer to "watch this
+			// tonight" cannot be a film that finished last week. It is only ever the
+			// whole title here — a film halfway through belongs to Continue Watching,
+			// which is not a candidate row at all.
+			//
+			// Series are deliberately not filtered this way. A show marked watched is
+			// one somebody is up to date with, which is exactly who a season premiere
+			// is news for, and premieres come from Sonarr rather than from here.
+			if fact.Watched {
+				continue
+			}
 			seen[fact.ID] = true
 			facts[fact.ID] = fact
 			rating, rated := heroRatingOf(raw)
@@ -357,6 +357,27 @@ func TestHeroMovieCandidatesSkipUnpressableRows(t *testing.T) {
 	}
 }

+// A film somebody has finished is not an answer to "watch this tonight", however new or
+// well reviewed it is. A series is left alone: watched there means up to date, which is
+// exactly who a returning season is news for.
+func TestHeroMovieCandidatesSkipWatchedFilms(t *testing.T) {
+	watched, _ := json.Marshal(map[string]any{
+		"Id": "seen", "Name": "Seen", "Type": "Movie",
+		"UserData": map[string]any{"Played": true},
+	})
+	partway, _ := json.Marshal(map[string]any{
+		"Id": "partway", "Name": "Partway", "Type": "Movie",
+		"UserData": map[string]any{"Played": false, "PlaybackPositionTicks": 6_000_000_000},
+	})
+	rows := []recommend.Row{{ID: "latest", Kind: "latest", Items: []json.RawMessage{
+		watched, partway, heroItem("unseen", "Unseen", "Movie"),
+	}}}
+	candidates, _ := heroMovieCandidates(rows)
+	if ids := strings.Join(heroIDs(candidates), ","); ids != "partway,unseen" {
+		t.Fatalf("expected the unfinished and unseen films only, got %s", ids)
+	}
+}
+
 // Emby writes dates in more than one shape, and one it will not parse is unknown rather
 // than fatal.
 func TestParseEmbyDate(t *testing.T) {
@@ -110,6 +110,10 @@ func (s *Server) handleRadarrImage(w http.ResponseWriter, r *http.Request, itemI
 			writeError(w, http.StatusNotFound, "image not found")
 			return
 		}
+		if clientGaveUp(r.Context(), err) {
+			writeError(w, statusClientClosedRequest, "the request was abandoned")
+			return
+		}
 		s.log.Warn("radarr image failed", "movie_id", movieID, "type", coverType, "error", err)
 		writeError(w, http.StatusBadGateway, "could not load the image")
 		return
@@ -154,6 +158,10 @@ func (s *Server) handleSonarrImage(w http.ResponseWriter, r *http.Request, itemI
 			writeError(w, http.StatusNotFound, "image not found")
 			return
 		}
+		if clientGaveUp(r.Context(), err) {
+			writeError(w, statusClientClosedRequest, "the request was abandoned")
+			return
+		}
 		s.log.Warn("sonarr image failed", "series_id", seriesID, "type", coverType, "error", err)
 		writeError(w, http.StatusBadGateway, "could not load the image")
 		return
@@ -192,7 +200,17 @@ func copyImage(
 }

 func expectedClientDisconnect(r *http.Request, err error) bool {
-	if r.Context().Err() != nil ||
+	return clientGaveUp(r.Context(), err)
+}
+
+// clientGaveUp reports whether a failure is the television having walked away rather than
+// anything being wrong here.
+//
+// A cancellation is the *only* thing it treats as such. A deadline is our own patience
+// running out, which is a real failure with a real cause; conflating the two would hide
+// exactly the timeouts worth seeing.
+func clientGaveUp(ctx context.Context, err error) bool {
+	if errors.Is(ctx.Err(), context.Canceled) ||
 		errors.Is(err, context.Canceled) ||
 		errors.Is(err, net.ErrClosed) ||
 		errors.Is(err, syscall.EPIPE) ||
@@ -1 +1 @@
-0.1.34
+0.1.35
@@ -171,10 +171,18 @@ func (s *Store) SetUserPreferences(
 	// Pruned here rather than on a schedule: this is the only writer, so it is the only
 	// place the table can grow, and the acks of a revision nobody can see any more are
 	// dead weight with it.
+	//
+	// The cutoff is worked out in Go rather than as `$2 - $3` in the statement. Postgres
+	// has to infer a type for every placeholder, and two of them either side of an
+	// operator give it nothing to infer from — it answers `operator is not unique:
+	// unknown - unknown`, which failed the whole transaction and returned 500 to every
+	// television trying to save a setting. Arithmetic on two parameters belongs on this
+	// side of the wire.
+	pruneBelow := next.Revision - preferenceHistoryLimit
 	if _, err := tx.Exec(ctx, `
 		DELETE FROM user_preference_revisions
-		WHERE emby_user_id = $1 AND revision <= $2 - $3`,
-		userID, next.Revision, preferenceHistoryLimit,
+		WHERE emby_user_id = $1 AND revision <= $2`,
+		userID, pruneBelow,
 	); err != nil {
 		return UserPreferences{}, fmt.Errorf("store: prune preference history: %w", err)
 	}
@@ -183,13 +191,13 @@ func (s *Store) SetUserPreferences(
 	// described as "on revision 12" rather than as one that has never checked in.
 	if _, err := tx.Exec(ctx, `
 		DELETE FROM user_preference_acks stale
-		WHERE stale.emby_user_id = $1 AND stale.revision <= $2 - $3
+		WHERE stale.emby_user_id = $1 AND stale.revision <= $2
 		  AND EXISTS (
 			SELECT 1 FROM user_preference_acks newer
 			WHERE newer.emby_user_id = stale.emby_user_id
 			  AND newer.device_id = stale.device_id
 			  AND newer.revision > stale.revision)`,
-		userID, next.Revision, preferenceHistoryLimit,
+		userID, pruneBelow,
 	); err != nil {
 		return UserPreferences{}, fmt.Errorf("store: prune preference acks: %w", err)
 	}
@@ -1 +1 @@
 .1.34
 .1.35