fix(playlists): an Undo puts a row back after its old neighbour, not at a stale index

A raw index recorded at removal drifted once anything changed before the Undo.
Both lists now read the list fresh and re-insert each row after the row that
preceded it (lib/reinsert). A failed watched toggle in a Plex playlist also
puts the row's state back: its local copy was never re-seeded.

frontend/src/components/Playlists.dom.test.tsx

@@ -449,9 +449,10 @@ describe("Playlists — removing a row, and undoing it", () => {
   });
 
   it("puts it back where it stood in the stored order, whatever the view on screen", async () => {
-    vi.spyOn(api, "removeFromPlaylist").mockResolvedValue(undefined as never);
+    const lists = { 1: [vid("A", 0), vid("B", 0), vid("C", 0)] };
+    serverRemoves(lists);
     vi.spyOn(api, "addToPlaylist").mockResolvedValue(undefined as never);
-    mount({ 1: [vid("A", 0), vid("B", 0), vid("C", 0)] });
+    mount(lists);
     await screen.findByRole("button", { name: "card.play — Video C" });
     // Sort the VIEW by title, descending: C, B, A on screen — the stored order is still A, B, C.
     fireEvent.click(screen.getByRole("button", { name: /playlists\.colTitle/ }));
@@ -463,10 +464,25 @@ describe("Playlists — removing a row, and undoing it", () => {
     await waitFor(() => expect(api.addToPlaylist).toHaveBeenCalledWith(1, "A", 0));
   });
 
+  // E28 review C4: the stored index drifts when the list changes before the Undo (a row added at
+  // the top in another tab) — the row goes back after its old neighbour instead.
+  it("puts it back after its neighbour when the list changed before the Undo", async () => {
+    const lists = { 1: [vid("A", 0), vid("B", 0), vid("C", 0)] };
+    serverRemoves(lists);
+    vi.spyOn(api, "addToPlaylist").mockResolvedValue(undefined as never);
+    mount(lists);
+    await rowMenu("Video B", "playlists.removeItem");
+    await waitFor(() => expect(lastToast()?.action).toBeTruthy());
+    lists[1] = [vid("X", 0), ...lists[1]!];
+    act(() => lastToast().action!.onClick());
+    await waitFor(() => expect(api.addToPlaylist).toHaveBeenCalledWith(1, "B", 2));
+  });
+
   it("says so when the Undo fails", async () => {
-    vi.spyOn(api, "removeFromPlaylist").mockResolvedValue(undefined as never);
+    const lists = { 1: [vid("A", 0), vid("B", 0)] };
+    serverRemoves(lists);
     vi.spyOn(api, "addToPlaylist").mockRejectedValue(new TypeError("Failed to fetch"));
-    mount({ 1: [vid("A", 0), vid("B", 0)] });
+    mount(lists);
     await rowMenu("Video A", "playlists.removeItem");
     await waitFor(() => expect(lastToast()?.action).toBeTruthy());
     act(() => lastToast().action!.onClick());

frontend/src/components/Playlists.tsx

@@ -24,6 +24,7 @@ import { notify } from "../lib/notifications";
 import { notifyActionError } from "../lib/actionErrors";
 import { notifyYouTubeActionError } from "../lib/youtubeErrors";
 import { playlistName } from "../lib/playlistName";
+import { reinsertAt } from "../lib/reinsert";
 import { useUndoable } from "../lib/useUndoable";
 import { cycleSort, type SortState } from "../lib/columnSort";
 import { formatRuntime } from "../lib/format";
@@ -476,9 +477,10 @@ export default function Playlists() {
     // so the guard reset the order: the removed row came back and the undo history the rebase had
     // kept went with it. (A failure resyncs below, which puts the row back where it belongs.)
     const before = qc.getQueryData<PlaylistDetail>(qk.playlist(listId));
-    // Where it stood in the STORED order — what the Undo puts it back at (the server's list, so a
+    // The STORED order it stood in — the Undo puts it back after its neighbour there (the server's
-    // view sort on screen does not decide it).
+    // list, so a view sort on screen does not decide it; `reinsertAt`, so a change in between does
-    const at = before?.items.findIndex((v) => v.id === video.id) ?? -1;
+    // not shift it off by the rows added or moved meanwhile).
+    const storedBefore = before?.items.map((v) => v.id) ?? [];
     qc.setQueryData<PlaylistDetail>(qk.playlist(listId), (old) =>
       old ? { ...old, items: old.items.filter((v) => v.id !== video.id) } : old,
     );
@@ -503,10 +505,23 @@ export default function Playlists() {
       action: {
         label: t("common.undo"),
         // ONE request that puts it back at its index (the server renumbers around it): an append
-        // plus a whole-list reorder would land it at the end whenever the second call failed.
+        // plus a whole-list reorder would land it at the end whenever the second call failed. The
+        // index is worked out against the list as it is NOW, fetched fresh.
         onClick: () =>
-          void api
+          void qc
-            .addToPlaylist(listId, video.id, at < 0 ? undefined : at)
+            .fetchQuery({
+              queryKey: qk.playlist(listId),
+              queryFn: () => api.playlist(listId),
+              staleTime: 0,
+            })
+            .then((now) => {
+              const back = reinsertAt(
+                storedBefore,
+                [video.id],
+                now.items.map((v) => v.id),
+              );
+              if (back.keys.length) return api.addToPlaylist(listId, video.id, back.positions[0]);
+            })
             .catch((e: unknown) => notifyActionError(e, t("playlists.undoRemoveFailed")))
             .finally(() => {
               refreshAll(listId);

frontend/src/components/PlexPlaylistView.menu.dom.test.tsx

@@ -152,14 +152,22 @@ describe("PlexPlaylistView · a row's menu", () => {
   it("removes a title with no confirm, and the toast's Undo puts it back at its index", async () => {
     mount([movie("1"), movie("2"), movie("3")]);
     await screen.findByText("Movie 2");
-    vi.spyOn(api, "plexPlaylistRemoveItem").mockResolvedValue({} as never);
+    vi.spyOn(api, "plexPlaylistRemoveItem").mockImplementation(async () => {
+      // The server's side of it: the Undo reads the list fresh, and the row is gone from it.
+      vi.mocked(api.plexPlaylist).mockResolvedValue({
+        id: 5,
+        title: "Films",
+        items: [movie("1"), movie("3")],
+      } as never);
+      return {} as never;
+    });
     const add = vi.spyOn(api, "plexPlaylistAddBulk").mockResolvedValue({} as never);
     await pick(() => fireEvent.contextMenu(rowOf("Movie 2")), "plex.playlist.removeFromPlaylist");
     await waitFor(() => expect(api.plexPlaylistRemoveItem).toHaveBeenCalledWith(5, "2"));
     expect(h.confirm).not.toHaveBeenCalled();
     await waitFor(() => expect(toast()?.message).toBe("plex.playlist.removedToast:Movie 2"));
     act(() => toast()!.action!.onClick());
-    expect(add).toHaveBeenCalledWith(5, ["2"], [1]);
+    await waitFor(() => expect(add).toHaveBeenCalledWith(5, ["2"], [1]));
   });
 
   it("plays and removes a whole show from its group's menu", async () => {
@@ -313,3 +321,24 @@ describe("PlexPlaylistView · the header's '…'", () => {
     );
   });
 });
+
+// E28 review E2: the watched toggle flips the view's LOCAL copy of the rows. On a failure the
+// resync refetches an identical list, react-query keeps the same `data`, and the copy was never
+// re-seeded — the row stayed "watched" on screen while Plex still had it unwatched.
+describe("PlexPlaylistView · a failed watched toggle", () => {
+  it("puts the row's watched state back", async () => {
+    mount([movie("1"), movie("2")]);
+    await screen.findByText("Movie 2");
+    vi.spyOn(api, "plexSetState").mockRejectedValue(new Error("offline"));
+    await pick(() => fireEvent.contextMenu(rowOf("Movie 2")), "plex.markWatched");
+    await waitFor(() =>
+      expect(h.notify).toHaveBeenCalledWith(
+        expect.objectContaining({ level: "error", title: "plex.watchedFailed" }),
+      ),
+    );
+    await act(async () => {});
+    fireEvent.keyDown(document.activeElement ?? document.body, { key: "Escape" });
+    await openMenu(() => fireEvent.contextMenu(rowOf("Movie 2")));
+    expect(menuRows()).toContain("plex.markWatched");
+  });
+});

frontend/src/components/PlexPlaylistView.tsx

@@ -560,6 +560,9 @@ export default function PlexPlaylistView({
       onToggleWatched: (c) =>
         void toggleWatchState(c, t, {
           optimistic: (next) => setStatus(c.id, next),
+          // The flip is on the view's LOCAL copy: a failed write's resync refetches an identical
+          // list, react-query keeps the same `data`, and the copy is never re-seeded from it.
+          revert: () => setStatus(c.id, c.status ?? "new"),
           resync: invalidate,
         }),
     },

frontend/src/components/usePlexPlaylistEdits.dom.test.tsx

@@ -129,9 +129,12 @@ describe("usePlexPlaylistEdits · the removal", () => {
 
   // E28 C1: no confirm — the toast's Undo is the way back, and it puts each row where it stood.
   it("says what went, and its Undo puts the rows back at their own indices in one request", async () => {
-    const { hook } = setup([movie("1"), movie("2"), movie("3"), movie("4")]);
+    const { hook, detail } = setup([movie("1"), movie("2"), movie("3"), movie("4")]);
     await waitFor(() => expect(hook.result.current.items).toHaveLength(4));
-    vi.spyOn(api, "plexPlaylistRemoveBulk").mockResolvedValue({} as never);
+    vi.spyOn(api, "plexPlaylistRemoveBulk").mockImplementation(async () => {
+      detail.mockResolvedValue({ id: 5, title: "F", items: [movie("1"), movie("3")] } as never);
+      return {} as never;
+    });
     const add = vi.spyOn(api, "plexPlaylistAddBulk").mockResolvedValue({} as never);
 
     // A season whose episodes do not sit together: rows 2 and 4.
@@ -140,14 +143,35 @@ describe("usePlexPlaylistEdits · the removal", () => {
     expect(toast.message).toBe("plex.playlist.removedToast");
     expect(toast.action!.label).toBe("common.undo");
     act(() => toast.action!.onClick());
-    expect(add).toHaveBeenCalledTimes(1);
+    await waitFor(() => expect(add).toHaveBeenCalledTimes(1));
-    expect(add).toHaveBeenCalledWith(5, ["4", "2"], [3, 1]);
+    expect(add).toHaveBeenCalledWith(5, ["2", "4"], [1, 3]);
+  });
+
+  // E28 review C4: a raw index drifts when the list changes before the Undo — another tab added a
+  // row at the top — and the row came back one place off. It goes back after its old neighbour.
+  it("puts a row back after its neighbour when the list changed before the Undo", async () => {
+    const { hook, detail } = setup([movie("1"), movie("2"), movie("3")]);
+    await waitFor(() => expect(hook.result.current.items).toHaveLength(3));
+    vi.spyOn(api, "plexPlaylistRemoveItem").mockResolvedValue({} as never);
+    const add = vi.spyOn(api, "plexPlaylistAddBulk").mockResolvedValue({} as never);
+    await act(() => hook.result.current.onRemove(["2"], "Movie 2"));
+    detail.mockResolvedValue({
+      id: 5,
+      title: "F",
+      items: [movie("x"), movie("1"), movie("3")],
+    } as never);
+    const toast = notify.mock.calls.map(([n]) => n).find((n) => n.action)!;
+    act(() => toast.action!.onClick());
+    await waitFor(() => expect(add).toHaveBeenCalledWith(5, ["2"], [2]));
   });
 
   it("says so when the Undo fails", async () => {
-    const { hook } = setup([movie("1"), movie("2")]);
+    const { hook, detail } = setup([movie("1"), movie("2")]);
     await waitFor(() => expect(hook.result.current.items).toHaveLength(2));
-    vi.spyOn(api, "plexPlaylistRemoveItem").mockResolvedValue({} as never);
+    vi.spyOn(api, "plexPlaylistRemoveItem").mockImplementation(async () => {
+      detail.mockResolvedValue({ id: 5, title: "F", items: [movie("2")] } as never);
+      return {} as never;
+    });
     vi.spyOn(api, "plexPlaylistAddBulk").mockRejectedValue(new Error("offline"));
     await act(() => hook.result.current.onRemove(["1"], "Movie 1"));
     const toast = notify.mock.calls.map(([n]) => n).find((n) => n.action)!;

frontend/src/components/usePlexPlaylistEdits.ts

@@ -6,6 +6,7 @@ import { api, type PlexCard } from "../lib/api";
 import { notifyActionError } from "../lib/actionErrors";
 import { notify } from "../lib/notifications";
 import { qk } from "../lib/queryKeys";
+import { reinsertAt } from "../lib/reinsert";
 
 /** A Plex playlist's items and the two writes that edit them in place — a reorder and a removal.
  *  Split from `PlexPlaylistView` so the failure paths are reachable from a test: the reorder is
@@ -81,14 +82,15 @@ export function usePlexPlaylistEdits(playlistId: number) {
   }
 
   /** Take `keys` out — one row, a season or a whole show — with no confirm: the toast's Undo puts
-   *  each row back at the index it held (E28 C1, the same bargain as a YouTube playlist's rows).
+   *  each row back where it stood (E28 C1, the same bargain as a YouTube playlist's rows) — after
-   *  `title` names what went in that toast. */
+   *  its old neighbour in the list as it is at Undo time (`reinsertAt`), since a raw index drifts
+   *  once anything changes in between. `title` names what went in that toast. */
   async function onRemove(keys: string[], title: string) {
     if (busy || !keys.length) return;
     setBusy(true);
-    // Indices over the list as it stands now, which is the server's order: a season's episodes need
+    // The list as it stands now, which is the server's order: a season's episodes need not sit
-    // not sit together (a reorder inside the show interleaves them), so each key keeps its own.
+    // together (a reorder inside the show interleaves them), so each key keeps its own neighbour.
-    const positions = keys.map((k) => items.findIndex((i) => i.id === k));
+    const before = items.map((i) => i.id);
     const set = new Set(keys);
     setItems((prev) => prev.filter((i) => !set.has(i.id)));
     try {
@@ -102,19 +104,27 @@ export function usePlexPlaylistEdits(playlistId: number) {
     } finally {
       setBusy(false);
     }
-    const back = keys.filter((_, i) => positions[i]! >= 0);
     notify({
       level: "success",
       message: t("plex.playlist.removedToast", { title }),
       action: {
         label: t("common.undo"),
         onClick: () =>
-          void api
+          void qc
-            .plexPlaylistAddBulk(
+            .fetchQuery({
-              playlistId,
+              queryKey: qk.plexPlaylist(playlistId),
-              back,
+              queryFn: () => api.plexPlaylist(playlistId),
-              positions.filter((p) => p >= 0),
+              staleTime: 0,
-            )
+            })
+            .then((now) => {
+              const back = reinsertAt(
+                before,
+                keys,
+                now.items.map((i) => i.id),
+              );
+              if (back.keys.length)
+                return api.plexPlaylistAddBulk(playlistId, back.keys, back.positions);
+            })
             .catch((e: unknown) => notifyActionError(e, t("plex.playlist.undoRemoveFailed")))
             .finally(invalidate),
       },

frontend/src/lib/plexWatch.ts

@@ -7,8 +7,10 @@ import { notifyActionError } from "./actionErrors";
 // `resync` (invalidate) — which corrects the optimistic flip to truth either way. Shared by the
 // browse grid and the person page, each of which owns a different cache shape.
 //
-// `revert` is for a cache `resync` must NOT refetch — a random draw (Discover's Surprise rail), which
+// `revert` is for state `resync` cannot correct, called on a failure only to put the card back: a
-// a refetch would reshuffle under the viewer. It is called on a failure only, to put the card back.
+// cache it must NOT refetch — a random draw (Discover's Surprise rail), which a refetch would
+// reshuffle under the viewer — or a component's local copy of the rows (a Plex playlist), which an
+// identical refetch never re-seeds (react-query keeps the same `data`).
 export async function toggleWatchState(
   card: PlexCard,
   t: TFunction,

frontend/src/lib/reinsert.test.ts

@@ -0,0 +1,55 @@
+import { describe, expect, it } from "vitest";
+
+import { reinsertAt } from "./reinsert";
+
+describe("reinsertAt", () => {
+  it("puts a row back where it was when nothing changed", () => {
+    expect(reinsertAt(["a", "b", "c"], ["b"], ["a", "c"])).toEqual({
+      keys: ["b"],
+      positions: [1],
+    });
+  });
+
+  it("follows its neighbour when a row was added above it meanwhile", () => {
+    // "x" landed at the top between the removal and the Undo: index 1 would now sit before "a".
+    expect(reinsertAt(["a", "b", "c"], ["b"], ["x", "a", "c"])).toEqual({
+      keys: ["b"],
+      positions: [2],
+    });
+  });
+
+  it("follows its neighbour through a reorder", () => {
+    expect(reinsertAt(["a", "b", "c"], ["b"], ["c", "a"])).toEqual({ keys: ["b"], positions: [2] });
+  });
+
+  it("walks further back when its neighbour is gone too", () => {
+    expect(reinsertAt(["a", "b", "c", "d"], ["c"], ["b", "d"])).toEqual({
+      keys: ["c"],
+      positions: [1],
+    });
+  });
+
+  it("goes to the start when it was first or no earlier row is left", () => {
+    expect(reinsertAt(["a", "b"], ["a"], ["x", "b"])).toEqual({ keys: ["a"], positions: [0] });
+    expect(reinsertAt(["a", "b", "c"], ["b"], ["c"])).toEqual({ keys: ["b"], positions: [0] });
+  });
+
+  it("keeps scattered and adjacent removed rows in their order, ascending", () => {
+    // A season's episodes interleaved with another's: b and d out, then "x" added at the top.
+    expect(reinsertAt(["a", "b", "c", "d", "e"], ["d", "b"], ["x", "a", "c", "e"])).toEqual({
+      keys: ["b", "d"],
+      positions: [2, 4],
+    });
+    expect(reinsertAt(["a", "b", "c", "d"], ["b", "c"], ["a", "d"])).toEqual({
+      keys: ["b", "c"],
+      positions: [1, 2],
+    });
+  });
+
+  it("skips a row that is already back", () => {
+    expect(reinsertAt(["a", "b", "c"], ["b", "c"], ["a", "c"])).toEqual({
+      keys: ["b"],
+      positions: [1],
+    });
+  });
+});

frontend/src/lib/reinsert.ts

@@ -0,0 +1,39 @@
+/** Where removed rows go back when their Undo comes LATER than the removal.
+ *
+ *  A raw index recorded at removal time drifts: an add or a reorder landing in between (another tab,
+ *  the rail, the player) shifts everything after it, and the row came back one or more places off
+ *  (E28 review C4). So each removed row goes back right after the row that preceded it in the list
+ *  it was removed from — its neighbour, looked up in the list as it stands NOW — or at the start when
+ *  it was first or no earlier neighbour is left. Rows already back in the list are skipped (the
+ *  server's add is idempotent and would leave them where they are).
+ *
+ *  @param before the list's ids when the rows were removed, in stored order
+ *  @param removed the ids taken out
+ *  @param now the list's ids at Undo time, in stored order
+ *  @returns the ids to add back and, paired one to one, the index each lands at — ascending, the
+ *    shape the server's insert takes (each index counts the rows put back before it). */
+export function reinsertAt(
+  before: readonly string[],
+  removed: readonly string[],
+  now: readonly string[],
+): { keys: string[]; positions: number[] } {
+  const out = new Set(removed);
+  const list = [...now];
+  const placed: string[] = [];
+  for (let i = 0; i < before.length; i++) {
+    const id = before[i]!;
+    if (!out.has(id) || now.includes(id)) continue;
+    let at = 0;
+    for (let j = i - 1; j >= 0; j--) {
+      const k = list.indexOf(before[j]!);
+      if (k >= 0) {
+        at = k + 1;
+        break;
+      }
+    }
+    list.splice(at, 0, id);
+    placed.push(id);
+  }
+  const keys = placed.map((id) => ({ id, at: list.indexOf(id) })).sort((a, b) => a.at - b.at);
+  return { keys: keys.map((k) => k.id), positions: keys.map((k) => k.at) };
+}