From 1630a3df3c6ff7da0abbc0fad65b2ba21a4d4b03 Mon Sep 17 00:00:00 2001 From: npeter83 Date: Wed, 9 Sep 2026 23:55:51 +0200 Subject: [PATCH 01/52] fix(player): the YT progress checkpoint reaches the cache, so a reopen cannot rewind it The player checkpointed every 5s and the server stored it, but nothing wrote the new position back into react-query: the close path only marked qk.feed() stale with refetchType "none", and nothing refetches the feed until it remounts (staleTime 30s, refetchOnWindowFocus off). So the resume read the position the cache had been FETCHED with, and the next checkpoint then persisted that older value over the newer one. Measured on localdev: played to 830s, closed (server had 830), reopened the same card without a reload -> the row was back to 751s. 79 seconds of progress destroyed by a reopen, and the card's progress bar never moved either. The Plex player already solved this for its detail query, with a comment describing the identical failure; this is the feed-side twin that was missed in that sweep. patchCachedProgress writes the position the SERVER stored (the endpoint clears trivially-early and near-finished checkpoints to 0, so mirroring our own reading would disagree with the row) into every cached feed page and playlist detail, keeping array identity where nothing changed. A session map covers the second half: the launch queue is frozen at mount, so stepping next->prev back to an item played earlier in the same session resumed from that snapshot. Verified in the browser: 811 -> close 816 -> reopen 825, and the card now shows Continue/Restart. --- frontend/src/components/PlayerModal.tsx | 13 +- frontend/src/lib/api/feed.ts | 18 +- .../playerModalHooks.progress.dom.test.tsx | 164 ++++++++++++++++++ frontend/src/lib/playerModalHooks.ts | 20 ++- frontend/src/lib/progressCache.ts | 67 +++++++ 5 files changed, 277 insertions(+), 5 deletions(-) create mode 100644 frontend/src/lib/playerModalHooks.progress.dom.test.tsx create mode 100644 frontend/src/lib/progressCache.ts diff --git a/frontend/src/components/PlayerModal.tsx b/frontend/src/components/PlayerModal.tsx index c9666d11..fbce8674 100644 --- a/frontend/src/components/PlayerModal.tsx +++ b/frontend/src/components/PlayerModal.tsx @@ -129,9 +129,17 @@ export default function PlayerModal({ if (index < 0) index = 0; const active = queue?.[index] ?? video; // the queue item at index, else the opened video (never undefined) const hasQueue = !!queue && queue.length > 1; - // startAt only applies to the item we opened with; advanced items resume their own pos. + // Positions this session has checkpointed (server-confirmed), per video id — see useYtProgress. + // The launch queue is frozen at mount, so its items keep the position they were snapshotted with; + // without this map, stepping next→prev back to an item played earlier in the SAME session would + // resume at that snapshot and then persist it over the newer position. + const sessionPosRef = useRef>(new Map()); + const sessionPos = sessionPosRef.current.get(active.id); + // startAt only applies to the item we opened with; advanced items resume their own pos. A + // position this session already stored wins over both — it is the freshest truth we have. const resumeAt = - startAt != null && active.id === video.id ? startAt : active.position_seconds || 0; + sessionPos ?? + (startAt != null && active.id === video.id ? startAt : active.position_seconds || 0); const mountRef = useRef(null); const playerRef = useRef(null); // Keyboard/wheel controls. We keep focus on the modal card (not the cross-origin player @@ -429,6 +437,7 @@ export default function PlayerModal({ reapplyStored, focusModal, advanceOnEnd, + sessionPosRef, qc, }); diff --git a/frontend/src/lib/api/feed.ts b/frontend/src/lib/api/feed.ts index 8ff19306..0d19b24a 100644 --- a/frontend/src/lib/api/feed.ts +++ b/frontend/src/lib/api/feed.ts @@ -40,6 +40,12 @@ export interface VideoDetail { duration_seconds: number | null; } +/** The stored answer of `POST /api/videos/{id}/progress` — the row as it now stands. */ +export interface StoredProgress { + video_id: string; + position_seconds: number; +} + export interface FeedResponse { items: Video[]; has_more: boolean; @@ -179,8 +185,16 @@ export const feedApi = { ), clearState: (id: string) => req(`/api/videos/${id}/state`, { method: "DELETE" }, { idempotent: true }), - saveProgress: (id: string, positionSeconds: number, durationSeconds: number) => - req( + // Returns the position the server actually STORED — which is not always what we sent: a + // trivially-early or near-finished checkpoint is cleared to 0 (see set_video_progress). Callers + // that mirror the position into a cache must write this answer, never their own reading, so the + // client can never disagree with the row. + saveProgress: ( + id: string, + positionSeconds: number, + durationSeconds: number, + ): Promise => + req( `/api/videos/${id}/progress`, { method: "POST", diff --git a/frontend/src/lib/playerModalHooks.progress.dom.test.tsx b/frontend/src/lib/playerModalHooks.progress.dom.test.tsx new file mode 100644 index 00000000..69745c07 --- /dev/null +++ b/frontend/src/lib/playerModalHooks.progress.dom.test.tsx @@ -0,0 +1,164 @@ +// @vitest-environment jsdom +// +// The YT player's progress checkpoint must reach the CACHE, not just the server. Before this, the +// modal's cleanup only marked `qk.feed()` stale with `refetchType: "none"` — and nothing refetches +// the feed until it remounts (staleTime 30s, refetchOnWindowFocus off), so reopening the same card +// in the same session resumed from the position the cache had been fetched with, and the next +// checkpoint then persisted THAT older value over the newer one. Measured on localdev before the +// fix: played to 830s → closed (server had 830) → reopened → the row was back to 751s. +// +// Its own file on purpose: `loadYouTubeApi` memoises the API promise in a module-level singleton, +// so a test that installs `window.YT` cannot share a module instance with tests that do not. +import { describe, it, expect, vi, afterEach } from "vitest"; +import { renderHook } from "@testing-library/react"; +import { QueryClient } from "@tanstack/react-query"; +import { createRef, type MutableRefObject } from "react"; +import { useYtProgress } from "./playerModalHooks"; +import { patchCachedProgress } from "./progressCache"; +import { qk } from "./queryKeys"; +import { api, type Video } from "./api"; +import type { YTPlayer } from "./youtube"; + +const VID = "EC5oFYy9z9g"; + +const video = (over: Partial