fix(feed): an override yields only to a refetch that set out after its write was answered

Send order let a refetch read before an unanswered mark committed and drop it. A reset's position 0 is written again after a refetch that set out before the server took the reset. The player-session refetch also waits for the player's closing checkpoint, which the player now tracks (progressFlushes); refetched first, the list put the older position back.

frontend/src/components/Feed.overrides.dom.test.tsx

@@ -29,6 +29,7 @@ vi.mock("./VirtualFeed", () => ({
         <li key={v.id}>
           <span>{`${v.id}:${v.status}`}</span>
           <button onClick={() => onState(v.id, "watched")}>watch {v.id}</button>
+          <button onClick={() => onState(v.id, "hidden")}>hide {v.id}</button>
         </li>
       ))}
       <button onClick={() => void fetchNextPage({})}>more</button>
@@ -87,10 +88,11 @@ const video = (id: string) =>
 const page = (ids: string[], next: string | null = null) =>
   ({ items: ids.map(video), next_cursor: next }) as never;
 
-function mount(feed: ReturnType<typeof vi.fn>) {
+function mount(feed: ReturnType<typeof vi.fn>, setState?: () => Promise<unknown>) {
   vi.spyOn(api, "feed").mockImplementation(feed as never);
   vi.spyOn(api, "feedSummary").mockResolvedValue({ count: 1, facets: {} } as never);
-  vi.spyOn(api, "setState").mockResolvedValue({ ok: true } as never);
+  if (setState) vi.spyOn(api, "setState").mockImplementation(setState as never);
+  else vi.spyOn(api, "setState").mockResolvedValue({ ok: true } as never);
   const qc = new QueryClient({ defaultOptions: { queries: { retry: false } } });
   qc.setQueryData(qk.me(), { id: 1, can_write: true, prefs: {} });
   const ui = (filters: FeedFilters) => (
@@ -149,6 +151,44 @@ describe("Feed — optimistic overrides", () => {
     expect(screen.getByText("v1:watched")).toBeTruthy();
   });
 
+  it("keep a mark sent before the refetch set out but not answered by then", async () => {
+    // The server may read the list before it commits the mark: that answer is older than the mark.
+    let answer: (v: unknown) => void = () => {};
+    let commit: (v: unknown) => void = () => {};
+    const feed = vi
+      .fn()
+      .mockResolvedValueOnce(page(["v1"]))
+      .mockImplementationOnce(() => new Promise((r) => (answer = r)));
+    const { qc } = mount(feed, () => new Promise((r) => (commit = r)));
+    fireEvent.click(await screen.findByText("watch v1"));
+    await screen.findByText("v1:watched");
+    act(() => void qc.invalidateQueries({ queryKey: qk.feed() }));
+    await waitFor(() => expect(feed).toHaveBeenCalledTimes(2));
+    await settle();
+    await act(async () => answer(page(["v1"])));
+    await settle();
+    expect(screen.getByText("v1:watched")).toBeTruthy();
+    await act(async () => commit({ ok: true }));
+  });
+
+  it("are taken back by a failed write only while no later write stands", async () => {
+    // Marked watched, then hidden before the first answer came back: the refused mark must not take
+    // the hide with it and put the card back in the view.
+    const writes: { resolve: (v: unknown) => void; reject: (e: unknown) => void }[] = [];
+    mount(
+      vi.fn().mockResolvedValue(page(["v1", "v2"])),
+      () => new Promise((resolve, reject) => writes.push({ resolve, reject })),
+    );
+    fireEvent.click(await screen.findByText("watch v1"));
+    fireEvent.click(await screen.findByText("hide v1"));
+    await waitFor(() => expect(screen.queryByText(/^v1:/)).toBeNull());
+    await act(async () => writes[0]?.reject(new Error("refused")));
+    await settle();
+    expect(screen.queryByText(/^v1:/)).toBeNull();
+    await act(async () => writes[1]?.reject(new Error("refused")));
+    await waitFor(() => expect(screen.getByText("v1:new")).toBeTruthy());
+  });
+
   it("survive an append of the next page", async () => {
     let answer: (v: unknown) => void = () => {};
     const feed = vi

frontend/src/components/Feed.playerSession.dom.test.tsx

@@ -40,10 +40,14 @@ vi.mock("./VirtualFeed", () => ({
   ),
 }));
 // The player's progress hook marks a clip closed near its end watched from its unmount cleanup:
-// `watchOnUnmount` stands in for that.
-const player = vi.hoisted(() => ({ watchOnUnmount: false }));
+// `watchOnUnmount` stands in for that, and `flush` for its closing checkpoint, which it tracks.
+const player = vi.hoisted(() => ({
+  watchOnUnmount: false,
+  flush: null as Promise<unknown> | null,
+}));
 vi.mock("./PlayerModal", async () => {
   const { useEffect } = await import("react");
+  const { trackProgressFlush } = await import("../lib/progressCache");
   return {
     default: function Player({
       video,
@@ -57,6 +61,7 @@ vi.mock("./PlayerModal", async () => {
       useEffect(
         () => () => {
           if (player.watchOnUnmount) onState(video.id, "watched");
+          if (player.flush) trackProgressFlush(player.flush);
         },
         [video.id, onState],
       );
@@ -91,6 +96,7 @@ afterEach(() => {
   cleanup();
   vi.restoreAllMocks();
   player.watchOnUnmount = false;
+  player.flush = null;
 });
 
 const FILTERS: FeedFilters = {
@@ -203,6 +209,22 @@ describe("Feed — a video marked watched in the player", () => {
     await waitFor(() => expect(feed.mock.calls.length).toBeGreaterThan(before));
   });
 
+  it("waits for the player's closing checkpoint before it refetches", async () => {
+    // Refetched before that write landed, the list read the older position and, landing after the
+    // player's patch, put it back: a reopen resumed early.
+    const { feed } = mount();
+    let land: (v: unknown) => void = () => {};
+    player.flush = new Promise((r) => (land = r));
+    fireEvent.click(await screen.findByText("open v1"));
+    fireEvent.click(await screen.findByText("mark"));
+    const before = feed.mock.calls.length;
+    fireEvent.click(screen.getByText("close"));
+    await act(() => new Promise((r) => setTimeout(r, 30)));
+    expect(feed.mock.calls.length).toBe(before);
+    await act(async () => land(undefined));
+    await waitFor(() => expect(feed.mock.calls.length).toBeGreaterThan(before));
+  });
+
   it("does not refetch on a close with nothing changed", async () => {
     const { feed } = mount();
     fireEvent.click(await screen.findByText("open v1"));

frontend/src/components/Feed.reset.dom.test.tsx

@@ -122,6 +122,23 @@ describe("Feed — resetting a half-watched video", () => {
     await waitFor(() => expect(screen.getByText("v1@40")).toBeTruthy());
   });
 
+  it("keeps the position at 0 through a refetch that set out before the reset", async () => {
+    // That refetch read the row the reset deletes: its answer put the old position back under a
+    // card the reset had made new, and a reopen resumed there.
+    let answer: (v: unknown) => void = () => {};
+    const qc = mount(() => new Promise(() => {}));
+    await screen.findByText("v1@95");
+    vi.mocked(api.feed).mockImplementationOnce(() => new Promise((r) => (answer = r)) as never);
+    act(() => void qc.invalidateQueries({ queryKey: qk.feed() }));
+    await waitFor(() => expect(api.feed).toHaveBeenCalledTimes(2));
+    await act(() => new Promise((r) => setTimeout(r, 20)));
+    fireEvent.click(screen.getByText("reset v1"));
+    await screen.findByText("v1@0");
+    await act(async () => answer({ items: [halfWatched], next_cursor: null }));
+    await act(() => new Promise((r) => setTimeout(r, 20)));
+    expect(screen.getByText("v1@0")).toBeTruthy();
+  });
+
   it("puts the position back when the server refuses the reset", async () => {
     mount(() => Promise.reject(new Error("down")));
     fireEvent.click(await screen.findByText("reset v1"));

frontend/src/components/Feed.tsx

@@ -27,7 +27,7 @@ import i18n from "../i18n";
 import { notifyActionError } from "../lib/actionErrors";
 import { notify, resolveVideo } from "../lib/notifications";
 import { useDebounced } from "../lib/useDebounced";
-import { patchCachedProgress } from "../lib/progressCache";
+import { patchCachedProgress, progressFlushes } from "../lib/progressCache";
 import {
   buildSort,
   feedSortOptions,
@@ -103,13 +103,14 @@ function matchesView(status: VideoStatus, show: string): boolean {
   }
 }
 
-// An optimistic value and its place in this feed's write order. A refetch that lands is the server's
-// word only on what was written BEFORE it set out: a mark made while it was in flight is newer than
-// its answer, and dropping it put a card marked watched back into an Unwatched view.
+// An optimistic value and the write that made it. A refetch that lands is the server's word only on
+// the writes it ANSWERED before that refetch set out: a mark made while it was in flight is newer
+// than its answer, and so is one sent just before it whose answer had not come back (the server may
+// read the list first). Dropping either put a card marked watched back into an Unwatched view.
 type Stamped<T> = { value: T; seq: number };
 type Overrides<T> = Record<string, Stamped<T>>;
-const writtenAfter = <T,>(o: Overrides<T>, since: number): Overrides<T> =>
-  Object.fromEntries(Object.entries(o).filter(([, s]) => s.seq > since));
+const unconfirmed = <T,>(o: Overrides<T>, answered: ReadonlySet<number>): Overrides<T> =>
+  Object.fromEntries(Object.entries(o).filter(([, s]) => !answered.has(s.seq)));
 // A failed write takes back only its OWN value: a later write for the same video stands.
 const withoutWrite = <T,>(o: Overrides<T>, id: string, seq: number): Overrides<T> => {
   if (o[id]?.seq !== seq) return o;
@@ -164,9 +165,14 @@ export default function Feed({
     : SORT_KEYS;
   const [overrides, setOverrides] = useState<Overrides<VideoStatus>>({});
   const [savedOverrides, setSavedOverrides] = useState<Overrides<boolean>>({});
-  // The write order the stamps above count in, and where it stood when the running refetch set out.
+  // The write order the stamps above count in, the writes the server has answered, and which of
+  // them it had answered when the running refetch set out.
   const writeSeq = useRef(0);
-  const refetchFrom = useRef(0);
+  const answered = useRef(new Set<number>());
+  const answeredAtRefetch = useRef<ReadonlySet<number>>(new Set());
+  // A reset's position 0 lives in the cached lists (see onResetState), where a refetch that set out
+  // before the server took the reset puts the old position back: written again on its landing.
+  const resets = useRef(new Map<string, number>());
   // The open player: which video and where to start (null = resume from saved position).
   const [activeVideo, setActiveVideo] = useState<{ video: Video; startAt: number | null } | null>(
     null,
@@ -184,6 +190,8 @@ export default function Feed({
   });
   const openVideo = useCallback((video: Video, startAt: number | null = null) => {
     playerSession.current.open = true;
+    // Its player writes the next position: a reset no longer speaks for it.
+    resets.current.delete(video.id);
     setActiveVideo({ video, startAt });
   }, []);
   // Stable identity, like Playlists' `closePlayer`: PlayerModal's keydown effect
@@ -193,7 +201,9 @@ export default function Feed({
   const closePlayer = useCallback(() => setActiveVideo(null), []);
   // The session ends once the player has UNMOUNTED, not on the close click: its progress hook's
   // cleanup marks a clip closed near its end watched, a write that lands after the click. React runs
-  // every unmount cleanup before any effect's setup, so here that write is already in the session.
+  // every unmount cleanup before any effect's setup, so here that write is already in the session,
+  // and so is the player's closing checkpoint (`progressFlushes`): refetched before it lands, the
+  // list read the older position and, landing after the player's patch, put it back.
   const playerOpen = activeVideo !== null;
   useEffect(() => {
     const session = playerSession.current;
@@ -201,7 +211,7 @@ export default function Feed({
     session.open = false;
     if (!session.changed) return;
     session.changed = false;
-    void Promise.allSettled([...session.pending]).then(() => {
+    void Promise.allSettled([...session.pending, ...progressFlushes()]).then(() => {
       void qc.invalidateQueries({ queryKey: qk.feed() });
       void qc.invalidateQueries({ queryKey: qk.feedSummary() });
     });
@@ -289,20 +299,26 @@ export default function Feed({
   // page 1 rows are unchanged, so a just-hidden card would flash back), and not on a local write into
   // the cache: the player's closing position patch bumped `dataUpdatedAt` too, cleared the overrides,
   // and put a card marked watched back into an Unwatched view as "new", with nothing to refetch it.
-  // And only the overrides written before it set out (`Stamped`).
+  // And only the overrides whose write it had answered when it set out (`Stamped`).
   const refetching = query.isFetching && !query.isFetchingNextPage;
   const wasRefetching = useRef(false);
   useEffect(() => {
     const started = !wasRefetching.current && refetching;
     const landed = wasRefetching.current && !refetching && query.status === "success";
     wasRefetching.current = refetching;
-    if (started) refetchFrom.current = writeSeq.current;
-    if (landed) {
-      const since = refetchFrom.current;
-      setOverrides((o) => writtenAfter(o, since));
-      setSavedOverrides((o) => writtenAfter(o, since));
+    if (started) answeredAtRefetch.current = new Set(answered.current);
+    if (!landed) return;
+    const known = answeredAtRefetch.current;
+    setOverrides((o) => unconfirmed(o, known));
+    setSavedOverrides((o) => unconfirmed(o, known));
+    for (const seq of known) answered.current.delete(seq);
+    const zero = new Map<string, number>();
+    for (const [id, seq] of resets.current) {
+      if (known.has(seq)) resets.current.delete(id);
+      else zero.set(id, 0);
     }
-  }, [refetching, query.status]);
+    patchCachedProgress(qc, zero);
+  }, [refetching, query.status, qc]);
 
   // One shared query with the sidebar (same key + filters -> React Query dedupes to a single request):
   // the feed reads `.count`, the sidebar reads `.facets` (PERF-13, replaces feedCount + facets).
@@ -346,6 +362,7 @@ export default function Feed({
           // already show the correct state (a hidden/watched card drops out immediately), so refetching
           // every loaded infinite-query page here was pure waste — 10 pages x rapid marks = a refetch
           // storm (PERF-2). The next filter change / remount refetches fresh from the server.
+          answered.current.add(seq);
           qc.invalidateQueries({ queryKey: qk.feed(), refetchType: "none" });
           if (status === "hidden") {
             const v = loadedRef.current.find((x) => x.id === id);
@@ -408,13 +425,19 @@ export default function Feed({
       // checkpoint wrote over the progress just made.
       const before = loadedRef.current.find((v) => v.id === id)?.position_seconds;
       patchCachedProgress(qc, new Map([[id, 0]]));
+      resets.current.set(id, seq);
       api
         .clearState(id)
         // Stale-mark only (see setState above): the optimistic override already reset the card.
-        .then(() => qc.invalidateQueries({ queryKey: qk.feed(), refetchType: "none" }))
+        .then(() => {
+          answered.current.add(seq);
+          qc.invalidateQueries({ queryKey: qk.feed(), refetchType: "none" });
+        })
         .catch((e: unknown) => {
           notifyActionError(e, i18n.t("errors.writeFailed.videoReset"));
           setOverrides((o) => withoutWrite(o, id, seq));
+          if (resets.current.get(id) !== seq) return;
+          resets.current.delete(id);
           if (before !== undefined) patchCachedProgress(qc, new Map([[id, before]]));
         });
     },
@@ -427,6 +450,7 @@ export default function Feed({
       setSavedOverrides((o) => ({ ...o, [id]: { value: saved, seq } }));
       (saved ? api.watchLaterAdd(id) : api.watchLaterRemove(id))
         .then(() => {
+          answered.current.add(seq);
           qc.invalidateQueries({ queryKey: qk.playlists() });
           qc.invalidateQueries({ queryKey: qk.playlist() });
         })

frontend/src/lib/playerModalHooks.progress.dom.test.tsx

@@ -14,7 +14,7 @@ import { renderHook } from "@testing-library/react";
 import { QueryClient } from "@tanstack/react-query";
 import { createRef, type RefObject } from "react";
 import { useYtProgress } from "./playerModalHooks";
-import { patchCachedProgress } from "./progressCache";
+import { patchCachedProgress, progressFlushes } from "./progressCache";
 import { qk } from "./queryKeys";
 import { api, type Video } from "./api";
 import type { YTPlayer } from "./youtube";
@@ -113,6 +113,27 @@ describe("useYtProgress — the checkpoint reaches the cache", () => {
     expect(cachedPos(qc)).toBe(830);
   });
 
+  it("hands its closing checkpoint out until it has landed and been patched in", async () => {
+    // A list refetched on the player's close waits for it: read before it landed, the list held the
+    // older position and, landing after the patch, put it back.
+    const qc = new QueryClient();
+    seedFeed(qc, video());
+    atPosition(830, 3118);
+    let land: (v: unknown) => void = () => {};
+    vi.spyOn(api, "saveProgress").mockReturnValue(new Promise((r) => (land = r)) as never);
+
+    const { unmount } = mountPlayer(qc, new Map());
+    await settle();
+    unmount();
+    const flushes = progressFlushes();
+    expect(flushes).toHaveLength(1);
+    land({ video_id: VID, position_seconds: 830 });
+    await flushes[0];
+    expect(cachedPos(qc)).toBe(830);
+    await settle();
+    expect(progressFlushes()).toHaveLength(0);
+  });
+
   it("mirrors what the SERVER stored, not what the player read", async () => {
     const qc = new QueryClient();
     seedFeed(qc, video({ position_seconds: 748 }));

frontend/src/lib/playerModalHooks.ts

@@ -19,7 +19,7 @@ import {
 import { useBackToExit } from "./history";
 import { api, type Video } from "./api";
 import { qk } from "./queryKeys";
-import { patchCachedProgress } from "./progressCache";
+import { patchCachedProgress, trackProgressFlush } from "./progressCache";
 import type { YTNamespace, YTPlayer, YTPlayerEvent } from "./youtube";
 import { logYouTubeApiFailed, logYouTubeError, logYouTubeState } from "./mediaLog";
 import type { AutoMode, LoopMode } from "./playerModes";
@@ -527,15 +527,18 @@ export function useYtProgress({
       window.removeEventListener("pagehide", onHide);
       // Final flush, then mark the feed + playlists stale so resume bars reflect this session on the next
       // natural refetch — WITHOUT refetching every loaded feed page on each player close (PERF-2).
-      void Promise.resolve(persist()).finally(() => {
-        // Write this session's stored positions INTO the cached lists first. Marking them stale is
-        // not enough on its own: nothing refetches the feed until it remounts (staleTime 30s,
-        // refetchOnWindowFocus off), so a reopen used to resume from the position the cache was
-        // fetched with — and the next checkpoint then persisted that older value over this one.
-        patchCachedProgress(qc, sessionPos);
-        qc.invalidateQueries({ queryKey: qk.feed(), refetchType: "none" });
-        qc.invalidateQueries({ queryKey: qk.playlist(), refetchType: "none" });
-      });
+      // Tracked, so a list refetch on the player's close waits for it (`progressFlushes`).
+      trackProgressFlush(
+        Promise.resolve(persist()).finally(() => {
+          // Write this session's stored positions INTO the cached lists first. Marking them stale is
+          // not enough on its own: nothing refetches the feed until it remounts (staleTime 30s,
+          // refetchOnWindowFocus off), so a reopen used to resume from the position the cache was
+          // fetched with — and the next checkpoint then persisted that older value over this one.
+          patchCachedProgress(qc, sessionPos);
+          qc.invalidateQueries({ queryKey: qk.feed(), refetchType: "none" });
+          qc.invalidateQueries({ queryKey: qk.playlist(), refetchType: "none" });
+        }),
+      );
       const p = playerRef.current;
       if (p && typeof p.destroy === "function") {
         try {

frontend/src/lib/progressCache.ts

@@ -25,6 +25,19 @@ const isItemList = (v: unknown): v is ItemList =>
  * read: the endpoint clears trivially-early and near-finished checkpoints to 0, and mirroring our
  * own reading would re-introduce that disagreement on the client.
  */
+// The player's closing flushes still on their way: its last checkpoint and the patch above. A list
+// refetched before one lands read the older checkpoint and, landing after the patch, put it back.
+const flushes = new Set<Promise<unknown>>();
+
+/** Track a closing flush until it settles (`progressFlushes` hands out the ones still running). */
+export function trackProgressFlush(flush: Promise<unknown>): void {
+  flushes.add(flush);
+  const settled = () => void flushes.delete(flush);
+  flush.then(settled, settled);
+}
+
+export const progressFlushes = (): Promise<unknown>[] => [...flushes];
+
 export function patchCachedProgress(qc: QueryClient, positions: Map<string, number>): void {
   if (positions.size === 0) return;