From b7a4eaff8da139cc04443bab17640ad6c4171ada Mon Sep 17 00:00:00 2001
From: npeter83 <npeter83+claude@gmail.com>
Date: Sun, 23 Aug 2026 01:39:29 +0200
Subject: [PATCH] fix(plex): resolve deep-review findings on the filter-bar
 overhaul
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Self-review of the branch diff (feature/plex-filter-bar-overhaul) against main:

- Surprise rail query key omitted `lang` while the backend /surprise now filters
  against the language-specific genre column → a HU/EN switch under an active genre
  filter left the rail stale. Key it by scope+lang+filters, matching plexDiscover.
- Chip-removing a collection left its entry in `collectionTitles` (hashed into the
  plex query keys), splitting the cache from the pristine no-collection state and
  growing the titles map unboundedly. Prune the title on remove, like CollectionPicker.
- The saved-views default-mirror effect gained `cfg` in its deps (re-memoized on every
  live filter edit) → a synchronous localStorage write per keystroke. Depend on the
  module-stable mirror fn instead.
- Collections-browse search input had no accessible name (placeholder only). Add
  aria-label + type="search".
- Extract the tile hue-hash + gradient duplicated between PlexDiscover and
  plexBrowseLists into a shared lib/hueTile so the two surfaces can't drift.
- Document migration 0073 downgrade() as lossy and de-dup non-feed rows before
  recreating the narrow (user_id, name) constraint (verified by the drift test).
- Add unit tests for coercePlexFilters (legacy single-collection migration) and
  plexFiltersSignature (order-independence, title-ignoring).

Gate: tsc/lint/knip/prettier clean, 410 vitest; backend ruff + migration drift +
saved-views tests green.
---
 .../versions/0073_saved_view_source.py        |  5 ++
 frontend/src/components/PlexDiscover.tsx      | 19 ++----
 .../src/components/SavedViewsProvider.tsx     |  9 ++-
 frontend/src/components/plexBrowseLists.tsx   | 14 ++--
 .../src/components/usePlexActiveFilters.ts    | 10 ++-
 frontend/src/lib/api/plex.dom.test.ts         | 66 +++++++++++++++++++
 frontend/src/lib/hueTile.ts                   | 13 ++++
 frontend/src/lib/queryKeys.ts                 |  3 +-
 8 files changed, 110 insertions(+), 29 deletions(-)
 create mode 100644 frontend/src/lib/api/plex.dom.test.ts
 create mode 100644 frontend/src/lib/hueTile.ts

diff --git a/backend/alembic/versions/0073_saved_view_source.py b/backend/alembic/versions/0073_saved_view_source.py
index c2aa8843..6b2a9061 100644
--- a/backend/alembic/versions/0073_saved_view_source.py
+++ b/backend/alembic/versions/0073_saved_view_source.py
@@ -29,6 +29,11 @@ def upgrade() -> None:
 
 
 def downgrade() -> None:
+    # LOSSY / effectively forward-only: upgrade lets a user hold a feed view and a plex view of the same
+    # name, so re-creating the narrow (user_id, name) constraint can raise IntegrityError on such a pair.
+    # De-dup by dropping the non-feed rows first (they can't exist in the pre-0073 schema anyway) so the
+    # narrow constraint re-applies cleanly for the drift test's downgrade→re-upgrade cycle.
+    op.execute("DELETE FROM saved_views WHERE source <> 'feed'")
     op.drop_constraint("uq_user_source_saved_view", "saved_views", type_="unique")
     op.create_unique_constraint("uq_user_saved_view", "saved_views", ["user_id", "name"])
     op.drop_column("saved_views", "source")
diff --git a/frontend/src/components/PlexDiscover.tsx b/frontend/src/components/PlexDiscover.tsx
index 4137cddf..e1c1d6a1 100644
--- a/frontend/src/components/PlexDiscover.tsx
+++ b/frontend/src/components/PlexDiscover.tsx
@@ -4,6 +4,7 @@ import { useQuery, useQueryClient } from "@tanstack/react-query";
 import { RotateCcw } from "lucide-react";
 import { qk } from "../lib/queryKeys";
 import { api, type PlexCard, type PlexDiscover as PlexDiscoverData } from "../lib/api";
+import { hueOf, hueTileGradient } from "../lib/hueTile";
 import { plexLang } from "../i18n";
 import { toggleWatchState } from "../lib/plexWatch";
 import { usePlex } from "./PlexProvider";
@@ -18,14 +19,6 @@ import { LoadingState, StateMessage } from "./QueryState";
 // info / watched-toggle handlers come from the shell (PlexBrowse) so a rail card behaves exactly like
 // a grid card; the watched-toggle optimistically flips THIS surface's cache too.
 
-// A stable, pleasant hue per genre string (no art to collage yet — colored tiles stand in). Hashing
-// keeps a genre the same color across renders/sessions without storing anything.
-function genreHue(name: string): number {
-  let h = 0;
-  for (let i = 0; i < name.length; i++) h = (h * 31 + name.charCodeAt(i)) % 360;
-  return h;
-}
-
 function CardRail({
   title,
   ariaLabel,
@@ -99,7 +92,7 @@ export default function PlexDiscover({
   });
   // Surprise-me is its own query so the ↻ button can redraw it without refetching the whole surface.
   const surpriseQ = useQuery({
-    queryKey: qk.plexSurprise(scope, filters),
+    queryKey: qk.plexSurprise(scope, lang, filters),
     queryFn: () => api.plexSurprise(scope, undefined, filters, lang),
     staleTime: 0,
   });
@@ -124,7 +117,7 @@ export default function PlexDiscover({
             : old,
         );
         qc.setQueryData<{ scope: string; items: PlexCard[] }>(
-          qk.plexSurprise(scope, filters),
+          qk.plexSurprise(scope, lang, filters),
           (old) => (old ? { ...old, items: flip(old.items) } : old),
         );
       },
@@ -207,16 +200,14 @@ export default function PlexDiscover({
           <h2 className="mb-3 text-sm font-semibold">{t("plex.discover.byGenre")}</h2>
           <div className="grid grid-cols-2 gap-3 sm:grid-cols-3 md:grid-cols-4 lg:grid-cols-6">
             {d.genres.map((g) => {
-              const hue = genreHue(g.value);
+              const hue = hueOf(g.value);
               return (
                 <button
                   key={g.value}
                   type="button"
                   onClick={() => onGenre(g.value)}
                   className="glass-hover relative flex h-20 items-end overflow-hidden rounded-xl p-3 text-left transition"
-                  style={{
-                    background: `linear-gradient(135deg, hsl(${hue} 55% 32%), hsl(${(hue + 40) % 360} 60% 22%))`,
-                  }}
+                  style={{ background: hueTileGradient(hue) }}
                 >
                   <span className="text-sm font-semibold text-white drop-shadow">{g.value}</span>
                   <span className="absolute right-2 top-2 text-2xs text-white/70">{g.count}</span>
diff --git a/frontend/src/components/SavedViewsProvider.tsx b/frontend/src/components/SavedViewsProvider.tsx
index 58a1f3ae..c02db1a1 100644
--- a/frontend/src/components/SavedViewsProvider.tsx
+++ b/frontend/src/components/SavedViewsProvider.tsx
@@ -140,10 +140,13 @@ export function SavedViewsProvider({ children }: { children: ReactNode }) {
     setModalOpen(false);
   }, [source]);
 
-  // Feed only: keep the default-view mirror (read on load) in sync with the server state.
+  // Feed only: keep the default-view mirror (read on load) in sync with the server state. Depend on the
+  // mirror fn (a module-stable ref, or undefined for plex) NOT the whole `cfg` object — cfg re-memoizes
+  // on every live filter edit, which would fire this synchronous localStorage write far beyond its intent.
+  const syncMirror = cfg.syncDefaultMirror;
   useEffect(() => {
-    if (viewsQuery.data) cfg.syncDefaultMirror?.(viewsQuery.data);
-  }, [viewsQuery.data, cfg]);
+    if (viewsQuery.data) syncMirror?.(viewsQuery.data);
+  }, [viewsQuery.data, syncMirror]);
 
   const currentKey = cfg.keyOf(cfg.filters);
   const activeView = useMemo(
diff --git a/frontend/src/components/plexBrowseLists.tsx b/frontend/src/components/plexBrowseLists.tsx
index 437fd421..91e69634 100644
--- a/frontend/src/components/plexBrowseLists.tsx
+++ b/frontend/src/components/plexBrowseLists.tsx
@@ -4,6 +4,7 @@ import { useQuery } from "@tanstack/react-query";
 import { Layers, ListMusic } from "lucide-react";
 import { qk } from "../lib/queryKeys";
 import { api } from "../lib/api";
+import { hueOf, hueTileGradient } from "../lib/hueTile";
 import { useDebounced } from "../lib/useDebounced";
 import { LoadingState, StateMessage } from "./QueryState";
 
@@ -13,13 +14,6 @@ import { LoadingState, StateMessage } from "./QueryState";
 // union), so this is a pure front-end surface. Tiles are coloured by a title hash (no per-item art to
 // fetch/proxy), matching the Discover genre tiles.
 
-// A stable, pleasant hue per title string — same idea as PlexDiscover's genre tiles.
-function hueOf(name: string): number {
-  let h = 0;
-  for (let i = 0; i < name.length; i++) h = (h * 31 + name.charCodeAt(i)) % 360;
-  return h;
-}
-
 function Tile({
   title,
   count,
@@ -38,9 +32,7 @@ function Tile({
       onClick={onClick}
       title={title}
       className="glass-hover group relative flex h-28 flex-col justify-end overflow-hidden rounded-xl p-3 text-left transition"
-      style={{
-        background: `linear-gradient(135deg, hsl(${hue} 55% 32%), hsl(${(hue + 40) % 360} 60% 22%))`,
-      }}
+      style={{ background: hueTileGradient(hue) }}
     >
       <span className="absolute right-2 top-2 text-white/70">{icon}</span>
       <span className="line-clamp-2 text-sm font-semibold text-white drop-shadow">{title}</span>
@@ -98,9 +90,11 @@ export function PlexCollectionsBrowse({ onPick }: { onPick: (id: string, title:
   return (
     <div>
       <input
+        type="search"
         value={search}
         onChange={(e) => setSearch(e.target.value)}
         placeholder={t("plex.filter.collectionSearch")}
+        aria-label={t("plex.filter.collectionSearch")}
         className="glass-card mb-4 w-full max-w-sm rounded-lg px-3 py-1.5 text-sm focus:outline-hidden focus:ring-2 focus:ring-accent/40"
       />
       {q.isError && !q.data ? (
diff --git a/frontend/src/components/usePlexActiveFilters.ts b/frontend/src/components/usePlexActiveFilters.ts
index a0d5e026..82e116a7 100644
--- a/frontend/src/components/usePlexActiveFilters.ts
+++ b/frontend/src/components/usePlexActiveFilters.ts
@@ -121,7 +121,15 @@ export function buildPlexActiveFilters(
     out.push({
       key: `collection:${c}`,
       label: filters.collectionTitles?.[c] ?? t("plex.filter.collection"),
-      remove: () => patch({ collections: filters.collections.filter((x) => x !== c) }),
+      // Prune the title too — collectionTitles is hashed into the plex query keys, so a stale entry
+      // splits the cache from the logically-identical no-collection state (mirrors CollectionPicker).
+      remove: () => {
+        const { [c]: _drop, ...restTitles } = filters.collectionTitles ?? {};
+        patch({
+          collections: filters.collections.filter((x) => x !== c),
+          collectionTitles: restTitles,
+        });
+      },
     });
   }
   if (show !== "all") {
diff --git a/frontend/src/lib/api/plex.dom.test.ts b/frontend/src/lib/api/plex.dom.test.ts
new file mode 100644
index 00000000..5bd6d79b
--- /dev/null
+++ b/frontend/src/lib/api/plex.dom.test.ts
@@ -0,0 +1,66 @@
+// @vitest-environment jsdom
+import { describe, it, expect } from "vitest";
+import {
+  coercePlexFilters,
+  plexFiltersSignature,
+  EMPTY_PLEX_FILTERS,
+  type PlexFilters,
+} from "./plex";
+
+describe("coercePlexFilters", () => {
+  it("migrates the legacy single collection/collectionTitle into the multi-select shape", () => {
+    const out = coercePlexFilters({ collection: "A", collectionTitle: "Marvel" });
+    expect(out.collections).toEqual(["A"]);
+    expect(out.collectionTitles).toEqual({ A: "Marvel" });
+  });
+
+  it("migrates a legacy collection with no title", () => {
+    const out = coercePlexFilters({ collection: "A" });
+    expect(out.collections).toEqual(["A"]);
+    expect(out.collectionTitles).toBeUndefined();
+  });
+
+  it("leaves an already-multi collections blob untouched (guard on !collections.length)", () => {
+    const out = coercePlexFilters({ collections: ["X", "Y"], collection: "A" });
+    expect(out.collections).toEqual(["X", "Y"]);
+  });
+
+  it("fills every default for an empty blob", () => {
+    expect(coercePlexFilters({})).toEqual(EMPTY_PLEX_FILTERS);
+    expect(coercePlexFilters({}).collections).toEqual([]);
+  });
+
+  it("drops the legacy sortDir field", () => {
+    const out = coercePlexFilters({ sortDir: "desc", genres: ["Drama"] });
+    expect((out as unknown as Record<string, unknown>).sortDir).toBeUndefined();
+    expect(out.genres).toEqual(["Drama"]);
+  });
+});
+
+describe("plexFiltersSignature", () => {
+  const base = (): PlexFilters => ({
+    ...EMPTY_PLEX_FILTERS,
+    genres: ["Drama"],
+    collections: ["A"],
+  });
+
+  it("is independent of key order for equivalent filters", () => {
+    // Same field values, keys assigned in a different literal order — appendPlexFilters emits by field,
+    // not by object key order, so the signatures must match.
+    const a: PlexFilters = { ...EMPTY_PLEX_FILTERS, genres: ["Drama"], collections: ["A"] };
+    const b: PlexFilters = { ...EMPTY_PLEX_FILTERS, collections: ["A"], genres: ["Drama"] };
+    expect(plexFiltersSignature(a)).toBe(plexFiltersSignature(b));
+  });
+
+  it("ignores display-only collectionTitles (not part of the wire params)", () => {
+    const a = base();
+    const b: PlexFilters = { ...base(), collectionTitles: { A: "Marvel" } };
+    expect(plexFiltersSignature(a)).toBe(plexFiltersSignature(b));
+  });
+
+  it("differs when a real filter differs", () => {
+    const a = base();
+    const b: PlexFilters = { ...base(), genres: ["Comedy"] };
+    expect(plexFiltersSignature(a)).not.toBe(plexFiltersSignature(b));
+  });
+});
diff --git a/frontend/src/lib/hueTile.ts b/frontend/src/lib/hueTile.ts
new file mode 100644
index 00000000..2bd14dee
--- /dev/null
+++ b/frontend/src/lib/hueTile.ts
@@ -0,0 +1,13 @@
+// A stable, pleasant hue per string (genre/playlist/collection title) plus the matching tile gradient.
+// Shared by the Discover genre tiles (PlexDiscover) and the header Playlist/Collection browse tiles
+// (plexBrowseLists) so the two surfaces that are meant to look identical can't drift. Hashing keeps a
+// name the same color across renders/sessions without storing anything.
+export function hueOf(name: string): number {
+  let h = 0;
+  for (let i = 0; i < name.length; i++) h = (h * 31 + name.charCodeAt(i)) % 360;
+  return h;
+}
+
+export function hueTileGradient(hue: number): string {
+  return `linear-gradient(135deg, hsl(${hue} 55% 32%), hsl(${(hue + 40) % 360} 60% 22%))`;
+}
diff --git a/frontend/src/lib/queryKeys.ts b/frontend/src/lib/queryKeys.ts
index 5c4065d4..356d5910 100644
--- a/frontend/src/lib/queryKeys.ts
+++ b/frontend/src/lib/queryKeys.ts
@@ -99,7 +99,8 @@ export const qk = {
   // both for a prefix invalidation after a watched-toggle. Surprise is its own key so the ↻ reshuffle
   // refetches it alone.
   plexDiscover: (...rest: unknown[]) => ["plex-discover", ...rest],
-  // Variadic like plexDiscover so it can key by scope + the active filters; `plexSurprise()` (no args)
+  // Variadic like plexDiscover so it can key by scope + lang + the active filters (lang because a genre
+  // filter narrows against the language-specific genre column, like the grid); `plexSurprise()` (no args)
   // stays a prefix for the reshuffle/watched-toggle invalidations.
   plexSurprise: (...rest: unknown[]) => ["plex-surprise", ...rest],
   plexSuggest: (...rest: unknown[]) => ["plex-suggest", ...rest],