fix(e4): address code-review findings — usePref flush, ?pl guard, dedup (E4 review)

Applies the /code-review high findings on the E4 branch:

- usePref: flush the pending debounced server write on unmount instead of
  clearing it, so the last change isn't lost (and can't later be overwritten by
  the stale server value via the adopt effect). [correctness]
- PlaylistsProvider.readSelectedId: accept ?pl only when it's a positive integer
  — "?pl=" gave Number("")===0 (finite) and "?pl=1.5" a fractional id, both of
  which selected a non-existent playlist. [correctness]
- usePref: extract mergePrefsCache() so useSavePref and usePref.set share the
  one me-cache merge shape instead of duplicating it. [reuse]
- New lib/useUrlMirror: the shared replaceState(urlWithParams(...)) mirror
  effect, so FeedFiltersProvider and PlaylistsProvider stop hand-rolling the
  identical block (keyed on the serialized delta so it only fires on change).
  [reuse]
- plexCards: swapBadgeReveal() helper for the repeated
  reveal-idle/group-hover:opacity-0 swap-badge ternary. [simplification]

Gate green (tsc, knip, lint 0-err, 247 vitest). Self-E2E (localdev): feed filter
mirrors to ?show and clears on leaving the feed / re-mirrors on return via
useUrlMirror; ?pl= (empty) and ?pl=abc no longer select a bogus playlist (fall
back to the stored selection); Plex reveal-idle badges still display:none on
touch after the helper refactor; 0 console errors.

frontend/src/components/FeedFiltersProvider.tsx

@@ -10,12 +10,8 @@ import {
 import { qk } from "../lib/queryKeys";
 import { useQuery } from "@tanstack/react-query";
 import { api, getActiveAccount, type FeedFilters } from "../lib/api";
-import {
-  filterParamUpdates,
-  hasFilterParams,
-  paramsToFilters,
-  urlWithParams,
-} from "../lib/urlState";
+import { filterParamUpdates, hasFilterParams, paramsToFilters } from "../lib/urlState";
+import { useUrlMirror } from "../lib/useUrlMirror";
 import { accountKey, LS, readJSON, readMerged, writeJSON } from "../lib/storage";
 import { useNavigation } from "./NavigationProvider";
 
@@ -114,19 +110,13 @@ export function FeedFiltersProvider({ children }: { children: ReactNode }) {
     if (key) writeJSON(key, next);
   }, []);
 
-  // Mirror the feed filters into the address bar so every feed view is bookmarkable (replaceState — a
-  // filter tweak is not a new history entry). Gated on page===feed: on other pages the filter keys are
-  // cleared from the URL (localStorage still holds them; they re-mirror on return to the feed). The
-  // channel detail page keeps its own isolated filters, so those never reach here. urlWithParams
-  // preserves the nav params (?page/?chan/?v/?pl) the other providers own.
+  // Mirror the feed filters into the address bar so every feed view is bookmarkable. Gated on
+  // page===feed: on other pages the filter keys are cleared from the URL (localStorage still holds
+  // them; they re-mirror on return to the feed). The channel detail page keeps its own isolated
+  // filters, so those never reach here. useUrlMirror preserves the nav params (?page/?chan/?v/?pl) the
+  // other providers own.
   const { page } = useNavigation();
-  useEffect(() => {
-    window.history.replaceState(
-      window.history.state,
-      "",
-      urlWithParams(filterParamUpdates(page === "feed" ? filters : null)),
-    );
-  }, [filters, page]);
+  useUrlMirror(filterParamUpdates(page === "feed" ? filters : null));
 
   // Load THIS account's own filters when the account first resolves or changes (login / switch /
   // add). Filters are per-account localStorage state, so another account's last view (incl. its

frontend/src/components/PlaylistsProvider.tsx

@@ -1,15 +1,7 @@
-import {
-  createContext,
-  useCallback,
-  useContext,
-  useEffect,
-  useMemo,
-  useState,
-  type ReactNode,
-} from "react";
+import { createContext, useCallback, useContext, useMemo, useState, type ReactNode } from "react";
 import { getAccountRaw, LS, setAccountRaw } from "../lib/storage";
 import { useAccountRehydrate } from "../lib/useAccountRehydrate";
-import { urlWithParams } from "../lib/urlState";
+import { useUrlMirror } from "../lib/useUrlMirror";
 import { useNavigation } from "./NavigationProvider";
 
 // Owns the Playlists module's shared local state — the rail's name filter and the selected playlist
@@ -47,9 +39,12 @@ export function usePlaylistsActions(): PlaylistsActions {
 }
 
 function readSelectedId(): number | null {
-  // A ?pl bookmark wins over the stored selection so a pasted link opens that playlist.
+  // A ?pl bookmark wins over the stored selection so a pasted link opens that playlist. Accept only a
+  // positive integer id — a bare "?pl=" gives Number("")===0 (finite!) and "?pl=1.5" a fractional id,
+  // both of which would select a non-existent playlist.
   const fromUrl = new URLSearchParams(window.location.search).get("pl");
-  if (fromUrl != null && Number.isFinite(Number(fromUrl))) return Number(fromUrl);
+  const n = fromUrl ? Number(fromUrl) : NaN;
+  if (Number.isInteger(n) && n > 0) return n;
   const s = getAccountRaw(LS.playlist);
   return s ? Number(s) : null;
 }
@@ -73,16 +68,10 @@ export function PlaylistsProvider({ children }: { children: ReactNode }) {
   );
 
   // Mirror the selected playlist to ?pl on the Playlists page so it's bookmarkable; clear it elsewhere
-  // (the localStorage copy still restores it on return). replaceState — selecting isn't a new history
-  // entry — and urlWithParams keeps ?page and the other nav params intact.
+  // (the localStorage copy still restores it on return). useUrlMirror keeps ?page and the other nav
+  // params intact.
   const { page } = useNavigation();
-  useEffect(() => {
-    window.history.replaceState(
-      window.history.state,
-      "",
-      urlWithParams({ pl: page === "playlists" && selectedId != null ? String(selectedId) : null }),
-    );
-  }, [page, selectedId]);
+  useUrlMirror({ pl: page === "playlists" && selectedId != null ? String(selectedId) : null });
 
   const state = useMemo<PlaylistsState>(() => ({ search, selectedId }), [search, selectedId]);
   const actions = useMemo<PlaylistsActions>(

frontend/src/components/plexCards.tsx

@@ -16,6 +16,14 @@ const PLAYABLE_TINT: Record<string, string> = {
   transcode: "bg-danger",
 };
 
+// The reveal class for a status pill (watched / in-progress) that shares a corner with a hover-reveal
+// control (the watched-toggle). When that control is present it takes over on touch, so the pill yields
+// via `.reveal-idle` (hidden on touch, faded on hover); when there's no such control the pill is the
+// sole indicator and just fades on hover. (The single-episode card, where the pill is itself the
+// full-size watched marker with no fade, keeps its own inline rule.)
+const swapBadgeReveal = (hasControl: boolean): string =>
+  hasControl ? "reveal-idle" : "group-hover:opacity-0";
+
 export function PlexPosterCard({
   card,
   onOpen,
@@ -105,9 +113,9 @@ export function PlexPosterCard({
             where the toggle is always shown); a watched show keeps the badge as its sole marker. */}
         {card.status === "watched" && (
           <span
-            className={`pointer-events-none absolute top-1.5 right-1.5 text-3xs bg-black/70 text-white px-1.5 py-0.5 rounded-sm ${
-              isPlayable ? "reveal-idle" : "group-hover:opacity-0"
-            }`}
+            className={`pointer-events-none absolute top-1.5 right-1.5 text-3xs bg-black/70 text-white px-1.5 py-0.5 rounded-sm ${swapBadgeReveal(
+              isPlayable,
+            )}`}
           >
             {t("plex.watched")}
           </span>
@@ -229,18 +237,18 @@ export function SeasonCard({
         )}
         {watched && (
           <span
-            className={`pointer-events-none absolute top-1.5 right-1.5 text-3xs bg-black/70 text-white px-1.5 py-0.5 rounded-sm ${
-              onToggleWatched ? "reveal-idle" : "group-hover:opacity-0"
-            }`}
+            className={`pointer-events-none absolute top-1.5 right-1.5 text-3xs bg-black/70 text-white px-1.5 py-0.5 rounded-sm ${swapBadgeReveal(
+              !!onToggleWatched,
+            )}`}
           >
             {t("plex.watched")}
           </span>
         )}
         {se.status === "in_progress" && (
           <span
-            className={`pointer-events-none absolute top-1.5 right-1.5 text-3xs bg-black/75 text-accent px-1.5 py-0.5 rounded-sm ${
-              onToggleWatched ? "reveal-idle" : "group-hover:opacity-0"
-            }`}
+            className={`pointer-events-none absolute top-1.5 right-1.5 text-3xs bg-black/75 text-accent px-1.5 py-0.5 rounded-sm ${swapBadgeReveal(
+              !!onToggleWatched,
+            )}`}
           >
             {t("plex.inProgress")}
           </span>

frontend/src/lib/usePref.ts

@@ -1,5 +1,5 @@
 import { useCallback, useEffect, useRef, useState } from "react";
-import { useQuery, useQueryClient } from "@tanstack/react-query";
+import { useQuery, useQueryClient, type QueryClient } from "@tanstack/react-query";
 import { qk } from "./queryKeys";
 import { api } from "./api";
 import type { Me } from "./api/account";
@@ -37,6 +37,16 @@ function readServerPref(me: Me | null | undefined, key: string): unknown {
   return me?.preferences?.[key];
 }
 
+// Optimistically merge a preferences patch into the `me` cache so readers see it without a refetch —
+// the shape mirrors the server's PUT /api/me/preferences merge. Shared by useSavePref and usePref.set
+// (their sole difference is WHEN the server write fires — immediately vs debounced), so the cache-merge
+// shape lives in ONE place.
+function mergePrefsCache(qc: QueryClient, patch: Record<string, unknown>): void {
+  qc.setQueryData<Me | null>(qk.me(), (m) =>
+    m ? { ...m, preferences: { ...m.preferences, ...patch } } : m,
+  );
+}
+
 /** The raw preference persister: save an arbitrary partial preferences patch to the server and merge
  *  it into the `me` cache optimistically (so readers see it without a refetch). For dynamic or
  *  multi-key prefs that don't map to a single stateful usePref call — e.g. a variable set of toggles
@@ -46,9 +56,7 @@ export function useSavePref(): (patch: Record<string, unknown>) => void {
   return useCallback(
     (patch: Record<string, unknown>) => {
       api.savePrefs(patch).catch(() => {});
-      qc.setQueryData<Me | null>(qk.me(), (m) =>
-        m ? { ...m, preferences: { ...m.preferences, ...patch } } : m,
-      );
+      mergePrefsCache(qc, patch);
     },
     [qc],
   );
@@ -87,7 +95,18 @@ export function usePref<T>(
   const [value, setValue] = useState<T>(seed);
 
   const saveTimer = useRef<number | undefined>(undefined);
-  useEffect(() => () => window.clearTimeout(saveTimer.current), []);
+  // The pending debounced server write, if any — flushed (NOT dropped) on unmount so the last change
+  // isn't lost. Dropping it would leave the server stale while local/cache/localStorage hold the new
+  // value, and a later adopt could then overwrite the newer local value with the stale server one.
+  const pendingSave = useRef<(() => void) | null>(null);
+  useEffect(
+    () => () => {
+      window.clearTimeout(saveTimer.current);
+      pendingSave.current?.();
+      pendingSave.current = null;
+    },
+    [],
+  );
 
   const set = useCallback(
     (next: T) => {
@@ -95,19 +114,20 @@ export function usePref<T>(
       if (localKey) writeAccount(localKey, next);
       // Keep the `me` cache in step so other readers of this key (and this hook's own adopt effect)
       // see the new value without a refetch. Merges into preferences, mirroring the server PUT.
-      qc.setQueryData<Me | null>(qk.me(), (m) =>
-        m ? { ...m, preferences: { ...m.preferences, [serverKey]: next } } : m,
-      );
       const payload = { [serverKey]: next };
+      mergePrefsCache(qc, payload);
       // savePrefs is fire-and-forget: api.req() already surfaces a failure toast, and the local state +
       // cache already reflect the change (idempotent PUT retries a transient blip).
+      const flush = () => api.savePrefs(payload).catch(() => {});
       if (debounceMs) {
         window.clearTimeout(saveTimer.current);
+        pendingSave.current = flush;
         saveTimer.current = window.setTimeout(() => {
-          api.savePrefs(payload).catch(() => {});
+          pendingSave.current = null;
+          flush();
         }, debounceMs);
       } else {
-        api.savePrefs(payload).catch(() => {});
+        flush();
       }
     },
     [qc, serverKey, localKey, debounceMs],

frontend/src/lib/useUrlMirror.ts

@@ -0,0 +1,17 @@
+import { useEffect } from "react";
+import { urlWithParams } from "./urlState";
+
+// Mirror a set of nav params onto the CURRENT history entry (replaceState — mirroring is not a new
+// entry) whenever they change. Each provider owns its own key(s) and passes the delta it wants; a null
+// value clears that key, and urlWithParams preserves every other param (so providers don't clobber each
+// other). Extracted from the identical replaceState/urlWithParams blocks FeedFiltersProvider and
+// PlaylistsProvider used to hand-roll, so the mirroring contract lives in one place.
+export function useUrlMirror(updates: Record<string, string | null>): void {
+  // The caller typically passes an inline object literal (new identity every render); key on its
+  // serialized value so the effect only fires when a param actually changes, not every render.
+  const dep = JSON.stringify(updates);
+  useEffect(() => {
+    window.history.replaceState(window.history.state, "", urlWithParams(updates));
+    // eslint-disable-next-line react-hooks/exhaustive-deps
+  }, [dep]);
+}