From eb76c96e9416c64a490cc795de68e2a1beaafb92 Mon Sep 17 00:00:00 2001 From: Duncan Tourolle Date: Sat, 25 Jul 2026 09:20:56 +0200 Subject: [PATCH] feat(player): skipping an episode marks it watched, not paused Skipping to the next episode left a mid-episode resume point behind, so the skipped episode reappeared in Continue Watching with a partial progress bar. Skipping means "done with this one", not "stopped here". - reportSkippedEpisode marks the outgoing episode played instead of reporting a stop position, and arms a one-shot suppression consumed by the player's stop handler, so VideoPlayer's post-navigation unmount stop report can't overwrite the 100% progress with the partial one. - Continue Watching drops resume entries superseded by Next Up: an in-progress episode whose series has a next-up entry strictly later in series order (season, then episode) is hidden from the Home and TV rows. Movies, series without a next-up entry, and items with unknown or mixed ordering are always kept. Adds UR-059, DR-088, DR-089. TRACES: UR-059 | DR-088, DR-089 --- docs/requirements.md | 3 + src/lib/services/skipReporting.test.ts | 88 +++++++++++++ src/lib/services/skipReporting.ts | 64 +++++++++ src/lib/stores/continueWatchingFilter.test.ts | 124 ++++++++++++++++++ src/lib/stores/continueWatchingFilter.ts | 75 +++++++++++ src/lib/stores/home.ts | 9 +- src/lib/stores/tv.ts | 10 +- src/routes/player/[id]/+page.svelte | 15 ++- 8 files changed, 383 insertions(+), 5 deletions(-) create mode 100644 src/lib/services/skipReporting.test.ts create mode 100644 src/lib/services/skipReporting.ts create mode 100644 src/lib/stores/continueWatchingFilter.test.ts create mode 100644 src/lib/stores/continueWatchingFilter.ts diff --git a/docs/requirements.md b/docs/requirements.md index 98c03c88..258b8272 100644 --- a/docs/requirements.md +++ b/docs/requirements.md @@ -69,6 +69,7 @@ For a narrative overview of the system design, see | UR-056 | See how much disk each downloaded item/album/series consumes, in familiar rounded units shown on the card and detail page, with a device total on the Downloaded surface and a reclaim amount stated at the point of removal (see [ux-flows.md §7.3.1](ux-flows.md)) | Medium | Done | | UR-057 | Settings apply the instant a control is changed — no "Save" button and no save/dirty state — so leaving the page never loses a change; sliders show a live readout while dragging but persist on release (see [ux-flows.md §8.1](ux-flows.md)) | Medium | Done | | UR-058 | On the home screen, a tap on a media card opens the item (movie/episode detail page, or the series Episode Focus View for episodes) rather than starting playback; a long-press starts "play now" after a confirm; an episode detail/focus page links back to its parent series and season (see [ux-flows.md §5B.5](ux-flows.md) and [§5B.1](ux-flows.md)) | Medium | Done | +| UR-059 | Skipping to the next episode records the episode left behind as **fully watched** rather than saving a mid-episode resume point — skipping means "done with this one", not "stopped here" — and Continue Watching hides episodes the viewer has already moved past (a partial position behind that series' next-up episode), so the row only ever offers genuinely unfinished media | Medium | Done | --- @@ -239,6 +240,8 @@ Internal architecture, components, and application logic. | DR-085 | Per-item on-disk size: stat downloaded files, aggregate to album/season/series subtotals and a device total, format in consistent rounded human units; surface size on cards and detail pages, the device total on the Downloaded surface, and a reclaim figure in the remove confirmation | Downloads | UR-056 | Done | | DR-086 | Settings page persists each control on change via per-group writers (`playerSetAudioSettings` / `playerSetVideoSettings` / `updateCacheConfig`) rather than a batch Save action; slider controls persist on `change` (pointer release) not each `input` tick; no Save button, `saving`, or `saveMessage` state | Settings | UR-057 | Done | | DR-087 | `MediaCard` gains an `onLongPress` prop with pointer-based long-press detection (~500 ms hold, cancelled on >10 px move so carousel scroll is unaffected, trailing click suppressed); home carousels wire tap→detail/focus routing and long-press→confirm→player; episode taps route to `/library/?episode=`; the bare-episode detail page links to its parent series/season | UI | UR-058 | Done | +| DR-088 | Skip-to-next-episode marks the outgoing episode played (`markAsPlayed`) instead of reporting a stop position, and arms a one-shot suppression consumed by the player's stop handler so `VideoPlayer`'s post-navigation unmount stop report cannot overwrite the 100% progress with the partial position | UI | UR-059 | Done | +| DR-089 | Continue Watching suppresses resume entries superseded by Next Up: an in-progress episode whose series has a next-up entry strictly later in series order (season, then episode) is dropped from the Home and TV rows; movies, series without a next-up entry, and items with unknown/mixed episode ordering are always kept | UI | UR-059 | Done | --- diff --git a/src/lib/services/skipReporting.test.ts b/src/lib/services/skipReporting.test.ts new file mode 100644 index 00000000..d757f25f --- /dev/null +++ b/src/lib/services/skipReporting.test.ts @@ -0,0 +1,88 @@ +/** + * Skip-to-next-episode reporting tests. + * + * Regression: pressing "skip to next episode" left the outgoing episode with a + * mid-episode resume position, so it showed a partial progress bar and offered + * to resume. A manual skip means the user is done with that episode — it must + * be recorded as fully watched. + * + * TRACES: UR-059, UR-025 | DR-088 + */ + +import { describe, it, expect, beforeEach, vi } from "vitest"; + +const markAsPlayed = vi.fn(async (_itemId: string) => undefined); +const reportPlaybackStopped = vi.fn( + async (_itemId: string, _positionSeconds: number) => undefined +); + +vi.mock("./playbackReporting", () => ({ + markAsPlayed: (itemId: string) => markAsPlayed(itemId), + reportPlaybackStopped: (itemId: string, positionSeconds: number) => + reportPlaybackStopped(itemId, positionSeconds), +})); + +import { + shouldSuppressStopReport, + markSkipped, + reportSkippedEpisode, + resetSkipState, +} from "./skipReporting"; + +describe("skip reporting", () => { + beforeEach(() => { + vi.clearAllMocks(); + resetSkipState(); + }); + + describe("reportSkippedEpisode", () => { + it("marks the skipped episode as fully played", async () => { + await reportSkippedEpisode("ep-1"); + + expect(markAsPlayed).toHaveBeenCalledWith("ep-1"); + }); + + it("does not stamp the mid-episode position as a resume point", async () => { + await reportSkippedEpisode("ep-1"); + + expect(reportPlaybackStopped).not.toHaveBeenCalled(); + }); + + it("ignores a null item id", async () => { + await reportSkippedEpisode(null); + + expect(markAsPlayed).not.toHaveBeenCalled(); + }); + }); + + describe("shouldSuppressStopReport", () => { + it("suppresses the unmount stop report for the skipped episode", async () => { + await reportSkippedEpisode("ep-1"); + + // VideoPlayer.onDestroy fires after navigation with the mid-episode time. + expect(shouldSuppressStopReport("ep-1")).toBe(true); + }); + + it("only suppresses the episode that was actually skipped", async () => { + await reportSkippedEpisode("ep-1"); + + expect(shouldSuppressStopReport("ep-2")).toBe(false); + }); + + it("suppresses only once, so a later real stop still reports", async () => { + await reportSkippedEpisode("ep-1"); + + expect(shouldSuppressStopReport("ep-1")).toBe(true); + expect(shouldSuppressStopReport("ep-1")).toBe(false); + }); + + it("does not suppress when nothing was skipped", () => { + expect(shouldSuppressStopReport("ep-1")).toBe(false); + }); + + it("does not suppress a null item id", () => { + markSkipped("ep-1"); + expect(shouldSuppressStopReport(null)).toBe(false); + }); + }); +}); diff --git a/src/lib/services/skipReporting.ts b/src/lib/services/skipReporting.ts new file mode 100644 index 00000000..927344e8 --- /dev/null +++ b/src/lib/services/skipReporting.ts @@ -0,0 +1,64 @@ +// Skip-to-next-episode reporting. +// +// Skipping an episode is a "done with it" signal, not a "stopped here" one: +// the user is moving on because they've already seen it. So a manual skip +// records the outgoing episode as fully played rather than saving the +// mid-episode position as a resume point. +// +// The suppression handshake exists because VideoPlayer.onDestroy fires its +// final reportStop *after* the skip navigation, with the mid-episode time. If +// that landed, it would overwrite the just-written 100% progress and the +// episode would look partially watched again. markSkipped() arms a one-shot +// suppression that the stop handler consumes. +// +// TRACES: UR-059, UR-025 | DR-088 +import { markAsPlayed, reportPlaybackStopped } from "./playbackReporting"; + +/** Item id whose next stop report should be dropped, if any. */ +let suppressedItemId: string | null = null; + +/** + * Arm suppression of the next stop report for `itemId`. + * + * Exported separately from `reportSkippedEpisode` so callers that already + * handled their own reporting can still silence the unmount stop. + */ +export function markSkipped(itemId: string | null): void { + if (!itemId) return; + suppressedItemId = itemId; +} + +/** + * Should the pending stop report for `itemId` be dropped? + * + * One-shot: consumes the armed suppression, so a later genuine stop on the + * same episode still reports its position normally. + */ +export function shouldSuppressStopReport(itemId: string | null): boolean { + if (!itemId) return false; + if (suppressedItemId !== itemId) return false; + suppressedItemId = null; + return true; +} + +/** + * Record a manually skipped episode as fully watched. + * + * Deliberately does NOT call `reportPlaybackStopped` — that would write the + * partial position we are trying to avoid. + */ +export async function reportSkippedEpisode(itemId: string | null): Promise { + if (!itemId) return; + + markSkipped(itemId); + await markAsPlayed(itemId); +} + +/** Test hook: clear armed suppression between cases. */ +export function resetSkipState(): void { + suppressedItemId = null; +} + +// Re-exported so the module owns the full skip story; callers that need the +// normal stop path keep importing it from playbackReporting directly. +export { reportPlaybackStopped }; diff --git a/src/lib/stores/continueWatchingFilter.test.ts b/src/lib/stores/continueWatchingFilter.test.ts new file mode 100644 index 00000000..c628f674 --- /dev/null +++ b/src/lib/stores/continueWatchingFilter.test.ts @@ -0,0 +1,124 @@ +/** + * Continue Watching stale-entry suppression tests. + * + * A partially-watched episode should drop off Continue Watching once the user + * has moved past it — i.e. when Next Up for that series points at a *later* + * episode. Otherwise skipping an episode leaves it lingering as a resume + * suggestion behind the episode the user is actually on. + * + * TRACES: UR-059 | DR-089 + */ + +import { describe, it, expect } from "vitest"; +import type { MediaItem } from "$lib/api/types"; +import { filterSupersededResumeItems } from "./continueWatchingFilter"; + +function episode( + id: string, + seriesId: string, + season: number | undefined, + index: number | undefined +): MediaItem { + return { + id, + name: `Episode ${index}`, + kind: "episode", + seriesId, + parentIndexNumber: season, + indexNumber: index, + } as MediaItem; +} + +function movie(id: string): MediaItem { + return { id, name: "A Movie", kind: "movie" } as MediaItem; +} + +describe("filterSupersededResumeItems", () => { + it("drops a partially-watched episode when next up is later in the same season", () => { + const resume = [episode("s1e2", "series-a", 1, 2)]; + const nextUp = [episode("s1e5", "series-a", 1, 5)]; + + const result = filterSupersededResumeItems(resume, nextUp); + + expect(result).toEqual([]); + }); + + it("drops it when next up is in a later season", () => { + const resume = [episode("s1e9", "series-a", 1, 9)]; + const nextUp = [episode("s2e1", "series-a", 2, 1)]; + + expect(filterSupersededResumeItems(resume, nextUp)).toEqual([]); + }); + + it("keeps the episode the user is actually mid-way through", () => { + const resume = [episode("s1e4", "series-a", 1, 4)]; + const nextUp = [episode("s1e4", "series-a", 1, 4)]; + + expect(filterSupersededResumeItems(resume, nextUp)).toHaveLength(1); + }); + + it("keeps an episode ahead of next up (user jumped forward)", () => { + const resume = [episode("s1e7", "series-a", 1, 7)]; + const nextUp = [episode("s1e3", "series-a", 1, 3)]; + + expect(filterSupersededResumeItems(resume, nextUp)).toHaveLength(1); + }); + + it("only compares within the same series", () => { + const resume = [episode("a-s1e2", "series-a", 1, 2)]; + const nextUp = [episode("b-s1e9", "series-b", 1, 9)]; + + expect(filterSupersededResumeItems(resume, nextUp)).toHaveLength(1); + }); + + it("never suppresses movies", () => { + const resume = [movie("movie-1")]; + const nextUp = [episode("s1e5", "series-a", 1, 5)]; + + expect(filterSupersededResumeItems(resume, nextUp)).toHaveLength(1); + }); + + it("keeps items when ordering is unknown on either side", () => { + const resume = [episode("s1e2", "series-a", undefined, undefined)]; + const nextUp = [episode("s1e5", "series-a", 1, 5)]; + + expect(filterSupersededResumeItems(resume, nextUp)).toHaveLength(1); + }); + + it("treats a missing season number as season 1 only when both sides agree", () => { + // Flat series (no season folders): episode numbers alone must still order. + const resume = [episode("e2", "series-a", undefined, 2)]; + const nextUp = [episode("e6", "series-a", undefined, 6)]; + + expect(filterSupersededResumeItems(resume, nextUp)).toEqual([]); + }); + + it("is a no-op when next up is empty", () => { + const resume = [episode("s1e2", "series-a", 1, 2)]; + + expect(filterSupersededResumeItems(resume, [])).toHaveLength(1); + }); + + it("preserves the original order of surviving items", () => { + const resume = [ + episode("a-s1e2", "series-a", 1, 2), + episode("b-s1e1", "series-b", 1, 1), + episode("c-s1e3", "series-c", 1, 3), + ]; + const nextUp = [episode("b-s1e4", "series-b", 1, 4)]; + + const result = filterSupersededResumeItems(resume, nextUp); + + expect(result.map(i => i.id)).toEqual(["a-s1e2", "c-s1e3"]); + }); + + it("uses the furthest-ahead next-up entry for a series", () => { + const resume = [episode("s1e2", "series-a", 1, 2)]; + const nextUp = [ + episode("s1e1", "series-a", 1, 1), + episode("s1e8", "series-a", 1, 8), + ]; + + expect(filterSupersededResumeItems(resume, nextUp)).toEqual([]); + }); +}); diff --git a/src/lib/stores/continueWatchingFilter.ts b/src/lib/stores/continueWatchingFilter.ts new file mode 100644 index 00000000..1b9ee0fb --- /dev/null +++ b/src/lib/stores/continueWatchingFilter.ts @@ -0,0 +1,75 @@ +// Continue Watching stale-entry suppression. +// +// Continue Watching is built from raw resume positions, so an episode the user +// has moved past keeps showing up as a resume suggestion — most visibly after +// skipping an episode, which leaves a partial position behind. Next Up already +// tells us where the user actually is in each series, so an in-progress episode +// that sits *behind* its series' Next Up entry is stale and gets suppressed. +// +// This is presentation-layer de-duplication over two lists the frontend already +// holds — no Jellyfin taxonomy involved, so it stays in `src/`. +// +// TRACES: UR-059 | DR-089 +import type { MediaItem } from "$lib/api/types"; + +/** + * Position of an episode within its series, as (season, episode). + * + * Returns null when the episode number is unknown — without it there is no + * defensible ordering and we must not suppress anything. A missing *season* + * number is normal for flat series (no season folders), so it is only usable + * when both sides are equally season-less; callers compare via `isAheadOf`. + */ +function episodeOrder(item: MediaItem): { season: number | null; index: number } | null { + if (item.indexNumber == null) return null; + return { season: item.parentIndexNumber ?? null, index: item.indexNumber }; +} + +/** Is `a` strictly later in series order than `b`? */ +function isAheadOf(a: MediaItem, b: MediaItem): boolean { + const oa = episodeOrder(a); + const ob = episodeOrder(b); + if (!oa || !ob) return false; + + // Mixed season-numbering (one side foldered, the other flat) is not safely + // comparable — leave the entry alone rather than hide something wrongly. + if ((oa.season == null) !== (ob.season == null)) return false; + + if (oa.season != null && ob.season != null && oa.season !== ob.season) { + return oa.season > ob.season; + } + return oa.index > ob.index; +} + +/** + * Drop resume entries the user has already moved past. + * + * An episode is suppressed when its series has a Next Up entry strictly later + * in series order. Movies, items without a series, and anything whose ordering + * is unknown are always kept — suppression must never hide something the user + * genuinely still wants to resume. + */ +export function filterSupersededResumeItems( + resumeItems: MediaItem[], + nextUpItems: MediaItem[] +): MediaItem[] { + if (nextUpItems.length === 0) return resumeItems; + + // Furthest-ahead Next Up entry per series: Next Up can carry more than one + // entry for a series, and the latest is the true watch frontier. + const frontier = new Map(); + for (const item of nextUpItems) { + if (!item.seriesId) continue; + const current = frontier.get(item.seriesId); + if (!current || isAheadOf(item, current)) { + frontier.set(item.seriesId, item); + } + } + + return resumeItems.filter(item => { + if (item.kind !== "episode" || !item.seriesId) return true; + const ahead = frontier.get(item.seriesId); + if (!ahead) return true; + return !isAheadOf(ahead, item); + }); +} diff --git a/src/lib/stores/home.ts b/src/lib/stores/home.ts index f67a1375..c7dd9da7 100644 --- a/src/lib/stores/home.ts +++ b/src/lib/stores/home.ts @@ -1,8 +1,9 @@ // Home screen data store - featured items, continue watching, recently added -// TRACES: UR-023, UR-024, UR-034 | DR-026, DR-027, DR-038, DR-039 +// TRACES: UR-023, UR-024, UR-034, UR-059 | DR-026, DR-027, DR-038, DR-039, DR-089 import { writable, derived } from "svelte/store"; import type { MediaItem } from "$lib/api/types"; import { auth } from "./auth"; +import { filterSupersededResumeItems } from "./continueWatchingFilter"; interface HomeState { heroItems: MediaItem[]; @@ -50,8 +51,12 @@ function createHomeStore() { const valueOr = (i: number, fallback: T): T => settled[i].status === "fulfilled" ? (settled[i] as PromiseFulfilledResult).value : fallback; - const resume = valueOr(0, [] as typeof initialState.resumeItems); + const rawResume = valueOr(0, [] as typeof initialState.resumeItems); const nextUp = valueOr(1, [] as typeof initialState.nextUpItems); + // Drop episodes the user has already moved past (their series' Next Up + // points further ahead) so Continue Watching isn't cluttered with stale + // partial positions left behind by skipping. + const resume = filterSupersededResumeItems(rawResume, nextUp); const latest = valueOr(2, [] as typeof initialState.latestItems); const recentAudio = valueOr(3, [] as typeof initialState.recentlyPlayedAudio); const resumeMovies = valueOr(4, [] as typeof initialState.resumeMovies); diff --git a/src/lib/stores/tv.ts b/src/lib/stores/tv.ts index 8f58bdef..ade69a8a 100644 --- a/src/lib/stores/tv.ts +++ b/src/lib/stores/tv.ts @@ -1,10 +1,11 @@ // TV library landing page data store. // Powers the focused TV landing: hero + horizontal sliders. -// TRACES: UR-007, UR-023, UR-034 | DR-007, DR-038, DR-039 +// TRACES: UR-007, UR-023, UR-034, UR-059 | DR-007, DR-038, DR-039, DR-089 import { writable, derived } from "svelte/store"; import type { MediaItem } from "$lib/api/types"; import { auth } from "./auth"; import { buildHeroMix } from "$lib/utils/heroMix"; +import { filterSupersededResumeItems } from "./continueWatchingFilter"; /** A single "by genre" row: the genre name plus the series in it. */ export interface GenreRow { @@ -81,7 +82,12 @@ function createTvStore() { // Resume items are already video-only from the server, but keep episodes // (and the occasional movie that lives in a mixed library) defensively. - const continueWatching = resume.filter(i => i.kind === "episode" || i.kind === "movie"); + // Then drop episodes the user has moved past — a stale partial position + // behind the series' Next Up entry isn't something to continue. + const continueWatching = filterSupersededResumeItems( + resume.filter(i => i.kind === "episode" || i.kind === "movie"), + nextUp + ); // Mix the hero: in-progress episodes first (most personal), then next-up, // recent additions, and random series from across the library. diff --git a/src/routes/player/[id]/+page.svelte b/src/routes/player/[id]/+page.svelte index 709731c4..64a6a1fd 100644 --- a/src/routes/player/[id]/+page.svelte +++ b/src/routes/player/[id]/+page.svelte @@ -20,6 +20,7 @@ reportPlaybackProgress, reportPlaybackStopped, } from "$lib/services/playbackReporting"; + import { reportSkippedEpisode, shouldSuppressStopReport } from "$lib/services/skipReporting"; import { cleanup as cleanupNextEpisode } from "$lib/services/nextEpisodeService"; import * as html5Adapter from "$lib/player/html5Adapter"; @@ -536,7 +537,11 @@ function handleReportStop(positionSeconds: number, reportId?: string) { const id = reportId ?? itemId; - if (id) { + // A skipped episode was already recorded as fully watched. Its unmount stop + // report arrives after the skip navigation carrying the mid-episode + // position; letting it through would undo that and restore the partial + // progress bar. + if (id && !shouldSuppressStopReport(id)) { reportPlaybackStopped(id, positionSeconds); } // Intentionally do NOT emit a "stopped" player state here. This runs on both @@ -592,6 +597,14 @@ function handleSkipToNextEpisode() { if (nextEpisode) { + // Skipping means "I'm done with this one" — record the outgoing episode as + // fully watched rather than leaving a mid-episode resume point behind. This + // also arms suppression of the VideoPlayer's unmount stop report, which + // would otherwise fire after navigation and overwrite the 100% progress + // with the partial position (see skipReporting.ts). + const skippedId = currentMedia?.id ?? itemId ?? null; + void reportSkippedEpisode(skippedId); + // Use replaceState so "close/back" returns to the library, not the previous episode. // restart=true so advancing to the next episode always starts from the beginning, // even if it was previously started or watched.