From 46540dfec3598648293b8ceb7e873afab8bb8316 Mon Sep 17 00:00:00 2001 From: Quick <31828688+Quick104@users.noreply.github.com> Date: Tue, 9 Jun 2026 19:44:40 -0400 Subject: [PATCH] fix(progress): track resume points independently of watched state (#117) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(progress): track resume points independently of watched state Re-watching a finished item never re-entered Continue Watching: completion latched completed = TRUE one-way, pinned position_seconds to the duration, and the resume query filtered on completed = FALSE — so a rewatch heartbeat could never surface the item again (and releasing the latch would have erased the watched state clients display). Adopt the Jellyfin invariant instead of guard heuristics: - Completion resets position_seconds to 0 (UpdateProgress, SetProgress, SetProgressAt, SetProgressIfNewer, MarkWatched, MarkProgressBatch), so position_seconds > 0 now means "live resume point". - completed stays a pure one-way watched latch; rewatch heartbeats re-enter Continue Watching through plain GREATEST/MAX while the watched flag and PlayCount survive (matching Plex and Jellyfin master). - ListProgress("in_progress") keys on position_seconds > 0 in both stores; the SQLite store also gains the min-resume floor the Postgres store had. - jellycompat reports Played=true with live PositionTicks during a rewatch (resumePositionTicks no longer zeroes played items) — the DTO shape real Jellyfin emits since jellyfin/jellyfin#15762. - Web mirrors the latch (playbackProgressCache), resumes rewatches at their stored position, and shows progress bars on rewatched episodes. - ABS audiobook surfaces keep today's behavior: finished books report 100% via the completed flag and Continue Listening still excludes them. - Migrations reset legacy completed rows (position pinned to duration) to 0: a Goose migration for Postgres and a user_version-gated one-time fix for the per-user SQLite DBs. Replaces the guard-based approach of #109, whose restart detection (50% fraction + 60s time gap) could never release the latch for immediate rewatches (blocked heartbeats refreshed updated_at, re-arming the gap) and un-watched items on position-0 heartbeats. Co-Authored-By: Claude Fable 5 * fix(progress): address review — migration gate, one-way latch, missed writers/readers Review fixes for the position-based watch-progress model: - The per-user SQLite data fix is now migrateToV11 in the existing versioned runMigrations chain (schemaVersion 11). The previous standalone PRAGMA gate compared against 1, but existing DBs already sit at user_version 10, so the reset never ran for them — and the gate would have rewound the version. Fresh DBs short-circuit to the current version as before. - `completed` is now one-way across every playback/sync writer: SetProgress (the RecordPlaybackStop path — stopping a rewatch below the watched threshold no longer clears the watched state), SetProgressAt, SetProgressIfNewer (both stores), and the history import upsert, which also stops pinning completed imports to position = duration. Mark-unwatched still releases the latch via ClearProgress/ClearProgressBatch. - MarkProgressBatch regains its freshness guard: a delayed batch mark carrying an old timestamp can no longer zero a newer rewatch resume point (the position-reset now rides the original updated_at check). - Catalog read paths align with the new in-progress definition (position_seconds > 0, completed-agnostic): smart-collection in_progress filter, progress sort ratio, episode progress CTE, and both next-up predicates. - jellycompat derives PlayedPercentage and PlaybackPositionTicks from the same clamped position; a played item at rest reports 100 (as the old model did) while a rewatch reports its live fraction. - ABS audiobook UpsertProgress stores position 0 on finish so finished books can't surface as phantom resume entries; re-listens still move position forward from 0 with the latch intact. - The web optimistic cache zeroes the resume point on completion, mirroring the server invariant until the refetch lands. Co-Authored-By: Claude Fable 5 --------- Co-authored-by: Claude Fable 5 --- internal/api/handlers/items.go | 2 +- internal/audiobooks/abs_progress_store.go | 24 +- .../episode_catalog_entries_executor.go | 1 - internal/catalog/nextup_repo.go | 4 +- internal/catalog/nextup_repo_test.go | 2 +- internal/catalog/query_builder.go | 4 +- internal/catalog/query_builder_test.go | 2 +- internal/historyimport/repo.go | 9 +- internal/jellycompat/content_direct.go | 4 +- internal/jellycompat/mapping.go | 48 +-- internal/jellycompat/mapping_userdata_test.go | 59 +++- internal/userdb/migrate.go | 30 +- internal/userdb/progress.go | 47 ++- internal/userdb/progress_rewatch_test.go | 298 ++++++++++++++++++ internal/userstore/pgstore/progress.go | 36 ++- internal/userstore/storetest/suite.go | 9 +- ...4115_reset_completed_progress_position.sql | 20 ++ web/src/components/EpisodeRow.tsx | 4 +- web/src/components/HeroBanner.tsx | 6 +- .../queries/playbackProgressCache.test.ts | 6 +- .../hooks/queries/playbackProgressCache.ts | 43 +-- .../pages/ItemDetail/EpisodeContent.test.tsx | 3 +- .../pages/ItemDetail/MovieContent.test.tsx | 25 +- .../pages/ItemDetail/itemDetailLayout.test.ts | 21 +- web/src/pages/ItemDetail/itemDetailLayout.ts | 9 +- web/src/pages/watchRouteHelpers.ts | 8 +- 26 files changed, 615 insertions(+), 109 deletions(-) create mode 100644 internal/userdb/progress_rewatch_test.go create mode 100644 migrations/sql/20260609224115_reset_completed_progress_position.sql diff --git a/internal/api/handlers/items.go b/internal/api/handlers/items.go index 48025d5f..1198bbf0 100644 --- a/internal/api/handlers/items.go +++ b/internal/api/handlers/items.go @@ -1207,7 +1207,7 @@ func (h *ItemsHandler) getLeafUserData(r *http.Request, contentID string) *catal return &catalog.SeasonUserData{ PositionSeconds: progress.PositionSeconds, DurationSeconds: progress.DurationSeconds, - IsInProgress: !progress.Completed && progress.PositionSeconds > 0, + IsInProgress: progress.PositionSeconds > 0, Played: progress.Completed, LastFileID: progress.LastFileID, LastResolution: progress.LastResolution, diff --git a/internal/audiobooks/abs_progress_store.go b/internal/audiobooks/abs_progress_store.go index b7388478..461d4ee6 100644 --- a/internal/audiobooks/abs_progress_store.go +++ b/internal/audiobooks/abs_progress_store.go @@ -38,9 +38,13 @@ func (s *ABSProgressStore) GetProgress(ctx context.Context, userID, profileID, c var completed bool var progressPct *float64 + // Completed rows store position_seconds = 0 (no resume point), so the + // percentage must come from the completed flag, not the position. dbRow := s.Pool.QueryRow(ctx, ` SELECT media_item_id, position_seconds, duration_seconds, completed, - CASE WHEN duration_seconds > 0 THEN position_seconds / duration_seconds ELSE 0 END AS progress_pct, + CASE WHEN completed THEN 1.0 + WHEN duration_seconds > 0 THEN position_seconds / duration_seconds + ELSE 0 END AS progress_pct, updated_at FROM user_watch_progress WHERE user_id = $1 AND profile_id = $2 AND media_item_id = $3`, @@ -85,7 +89,9 @@ func (s *ABSProgressStore) ListProgressForAudiobooks(ctx context.Context, userID wp.position_seconds, wp.duration_seconds, wp.completed, - CASE WHEN wp.duration_seconds > 0 THEN wp.position_seconds / wp.duration_seconds ELSE 0 END, + CASE WHEN wp.completed THEN 1.0 + WHEN wp.duration_seconds > 0 THEN wp.position_seconds / wp.duration_seconds + ELSE 0 END, wp.updated_at FROM user_watch_progress wp JOIN media_items mi ON mi.content_id = wp.media_item_id @@ -138,17 +144,27 @@ func (s *ABSProgressStore) UpsertProgress(ctx context.Context, row abs.ProgressR if updatedAt.IsZero() { updatedAt = time.Now().UTC() } + // Completed rows hold no resume point (position_seconds = 0) — the same + // invariant as the userstore write paths — so finished books can't leak + // into position-based continue-watching/listening queries. A later + // non-finished report (re-listen) moves position forward from 0 while + // the completed latch stays. + position := row.CurrentSeconds + if row.IsFinished { + position = 0 + } _, err = s.Pool.Exec(ctx, ` INSERT INTO user_watch_progress (user_id, profile_id, media_item_id, position_seconds, duration_seconds, completed, updated_at) VALUES ($1, $2, $3, $4, $5, $6, $7) ON CONFLICT (user_id, profile_id, media_item_id) DO UPDATE SET - position_seconds = GREATEST(user_watch_progress.position_seconds, EXCLUDED.position_seconds), + position_seconds = CASE WHEN EXCLUDED.completed THEN 0 + ELSE GREATEST(user_watch_progress.position_seconds, EXCLUDED.position_seconds) END, duration_seconds = GREATEST(user_watch_progress.duration_seconds, EXCLUDED.duration_seconds), completed = user_watch_progress.completed OR EXCLUDED.completed, updated_at = GREATEST(user_watch_progress.updated_at, EXCLUDED.updated_at)`, uid, row.ProfileID, row.ContentID, - row.CurrentSeconds, row.DurationSeconds, row.IsFinished, updatedAt, + position, row.DurationSeconds, row.IsFinished, updatedAt, ) if err != nil { return fmt.Errorf("abs_progress_store: upsert progress: %w", err) diff --git a/internal/catalog/episode_catalog_entries_executor.go b/internal/catalog/episode_catalog_entries_executor.go index b9a747c2..6eea36ba 100644 --- a/internal/catalog/episode_catalog_entries_executor.go +++ b/internal/catalog/episode_catalog_entries_executor.go @@ -563,7 +563,6 @@ func episodeCatalogUserStateCTE(plan episodeCatalogUserStatePlan) string { AND hhi.media_item_id = uwp.media_item_id WHERE uwp.user_id = $1 AND uwp.profile_id = $2 - AND uwp.completed = FALSE AND uwp.position_seconds > 0%s AND (hhi.media_item_id IS NULL OR uwp.updated_at > hhi.hidden_before) )`, progressRatioGate) diff --git a/internal/catalog/nextup_repo.go b/internal/catalog/nextup_repo.go index 4ccab104..199b2e37 100644 --- a/internal/catalog/nextup_repo.go +++ b/internal/catalog/nextup_repo.go @@ -142,7 +142,7 @@ func buildListNextUpQuery(q NextUpQuery, limit int) (string, []interface{}) { JOIN episodes e_ip ON e_ip.content_id = uwp_ip.media_item_id WHERE uwp_ip.user_id = $1 AND uwp_ip.profile_id = $2 - AND uwp_ip.completed = FALSE + AND uwp_ip.position_seconds > 0 AND e_ip.series_id = ce.series_id AND uwp_ip.updated_at > ce.updated_at ) @@ -257,7 +257,7 @@ func buildListResumableFirstEpisodesQuery(q NextUpQuery, inProgressIDs []string) JOIN media_items si ON si.content_id = e.series_id WHERE uwp.user_id = $1 AND uwp.profile_id = $2 - AND uwp.completed = FALSE + AND uwp.position_seconds > 0 AND uwp.media_item_id = ANY($3)%s%s ORDER BY e.series_id, uwp.updated_at DESC`, seriesFilter, completedSeriesGate) diff --git a/internal/catalog/nextup_repo_test.go b/internal/catalog/nextup_repo_test.go index 8b60a645..eeec3ae2 100644 --- a/internal/catalog/nextup_repo_test.go +++ b/internal/catalog/nextup_repo_test.go @@ -15,7 +15,7 @@ func TestBuildListNextUpQuery_PrefersRecentCompletedOverOlderPartialProgress(t * expectedFragments := []string{ "eligible_series AS (", - "uwp_ip.completed = FALSE", + "uwp_ip.position_seconds > 0", "e_ip.series_id = ce.series_id", "uwp_ip.updated_at > ce.updated_at", "FROM user_history_hidden_items hhi", diff --git a/internal/catalog/query_builder.go b/internal/catalog/query_builder.go index 6c0043b4..6767db87 100644 --- a/internal/catalog/query_builder.go +++ b/internal/catalog/query_builder.go @@ -954,7 +954,6 @@ func (qb *QueryBuilder) buildInProgressClause(rule QueryRule) (string, error) { WHERE uwp.user_id = $%d AND uwp.profile_id = $%d AND uwp.media_item_id = %s.content_id - AND uwp.completed = FALSE AND uwp.position_seconds > 0 AND NOT EXISTS ( SELECT 1 @@ -1347,8 +1346,7 @@ func (qb *QueryBuilder) progressSortPlan() (string, []string, []any) { `LEFT JOIN ( SELECT uwp.media_item_id AS content_id, CASE - WHEN uwp.completed = FALSE - AND uwp.position_seconds > 0 + WHEN uwp.position_seconds > 0 AND COALESCE(uwp.duration_seconds, 0) > 0 AND (hhi.media_item_id IS NULL OR uwp.updated_at > hhi.hidden_before) THEN uwp.position_seconds::double precision / NULLIF(uwp.duration_seconds, 0) diff --git a/internal/catalog/query_builder_test.go b/internal/catalog/query_builder_test.go index e721b0a3..8c692ffc 100644 --- a/internal/catalog/query_builder_test.go +++ b/internal/catalog/query_builder_test.go @@ -344,7 +344,7 @@ func TestBuildSortClause_PersonalizedComputedSorts(t *testing.T) { }{ { field: "progress", - expectJoin: []string{"FROM user_watch_progress uwp", "uwp.completed = FALSE", "uwp.position_seconds::double precision / NULLIF(uwp.duration_seconds, 0)"}, + expectJoin: []string{"FROM user_watch_progress uwp", "uwp.position_seconds > 0", "uwp.position_seconds::double precision / NULLIF(uwp.duration_seconds, 0)"}, expectExpr: []string{"sort_progress.progress_ratio"}, expectArgN: 2, }, diff --git a/internal/historyimport/repo.go b/internal/historyimport/repo.go index 36ce17bd..ad166ea6 100644 --- a/internal/historyimport/repo.go +++ b/internal/historyimport/repo.go @@ -869,8 +869,11 @@ func (r *Repository) UpsertImportedProgress( if positionSeconds < 0 { positionSeconds = 0 } - if completed && durationSeconds > 0 { - positionSeconds = durationSeconds + // Completed rows hold no resume point and `completed` is a one-way watched + // latch — mirrors the userstore write paths so imported rows can't surface + // as phantom Continue Watching entries. + if completed { + positionSeconds = 0 } _, err := r.pool.Exec(ctx, ` INSERT INTO user_watch_progress ( @@ -879,7 +882,7 @@ func (r *Repository) UpsertImportedProgress( ON CONFLICT (user_id, profile_id, media_item_id) DO UPDATE SET position_seconds = excluded.position_seconds, duration_seconds = excluded.duration_seconds, - completed = excluded.completed, + completed = user_watch_progress.completed OR excluded.completed, updated_at = excluded.updated_at`, userID, profileID, mediaItemID, positionSeconds, durationSeconds, completed, updatedAt, ) diff --git a/internal/jellycompat/content_direct.go b/internal/jellycompat/content_direct.go index 90fb3562..f8e0f882 100644 --- a/internal/jellycompat/content_direct.go +++ b/internal/jellycompat/content_direct.go @@ -509,7 +509,7 @@ func (s *directContentService) GetItemDetail(ctx context.Context, session *Sessi PositionSeconds: progress.PositionSeconds, DurationSeconds: progress.DurationSeconds, Played: progress.Completed, - IsInProgress: progress.PositionSeconds > 0 && !progress.Completed, + IsInProgress: progress.PositionSeconds > 0, } } @@ -941,7 +941,7 @@ func seasonUserDataFromProgress(progress userstore.WatchProgress) *catalog.Seaso PositionSeconds: progress.PositionSeconds, DurationSeconds: progress.DurationSeconds, Played: progress.Completed, - IsInProgress: progress.PositionSeconds > 0 && !progress.Completed, + IsInProgress: progress.PositionSeconds > 0, } } diff --git a/internal/jellycompat/mapping.go b/internal/jellycompat/mapping.go index 694cf712..decf1799 100644 --- a/internal/jellycompat/mapping.go +++ b/internal/jellycompat/mapping.go @@ -533,10 +533,9 @@ func userDataDTO(itemID string, data *catalog.SeasonUserData, isFavorite bool, p dto := &itemUserDataDTO{IsFavorite: isFavorite, ItemID: itemID, Key: itemID} if data != nil { - dto.PlaybackPositionTicks = resumePositionTicks(data.PositionSeconds, data.DurationSeconds, data.Played) - if data.DurationSeconds > 0 { - dto.PlayedPercentage = (data.PositionSeconds / data.DurationSeconds) * 100 - } + pos := clampResumeSeconds(data.PositionSeconds, data.DurationSeconds) + dto.PlaybackPositionTicks = secondsToTicks(pos) + dto.PlayedPercentage = playedPercentage(pos, data.DurationSeconds, data.Played) dto.Played = data.Played dto.UnplayedItemCount = data.UnplayedCount if data.Played { @@ -545,10 +544,9 @@ func userDataDTO(itemID string, data *catalog.SeasonUserData, isFavorite bool, p } if progress != nil { - dto.PlaybackPositionTicks = resumePositionTicks(progress.PositionSeconds, progress.DurationSeconds, progress.Completed) - if progress.DurationSeconds > 0 { - dto.PlayedPercentage = (progress.PositionSeconds / progress.DurationSeconds) * 100 - } + pos := clampResumeSeconds(progress.PositionSeconds, progress.DurationSeconds) + dto.PlaybackPositionTicks = secondsToTicks(pos) + dto.PlayedPercentage = playedPercentage(pos, progress.DurationSeconds, progress.Completed) dto.Played = progress.Completed if progress.Completed { dto.PlayCount = 1 @@ -559,6 +557,21 @@ func userDataDTO(itemID string, data *catalog.SeasonUserData, isFavorite bool, p return dto } +// playedPercentage derives PlayedPercentage from the same clamped position +// used for PlaybackPositionTicks so the two fields can never disagree. A +// played item at rest (position 0, no resume point) reports 100 so clients +// rendering progress from the DTO show it fully watched; a rewatch in flight +// reports its live fraction. +func playedPercentage(clampedPos, duration float64, played bool) float64 { + if played && clampedPos == 0 { + return 100 + } + if duration <= 0 { + return 0 + } + return (clampedPos / duration) * 100 +} + func jellyfinItemType(native string) string { switch strings.ToLower(native) { case "movie": @@ -693,22 +706,21 @@ func secondsToTicks(seconds float64) int64 { return int64(seconds * 10_000_000) } -// resumePositionTicks returns the PlaybackPositionTicks value for a UserData -// DTO. When the item is fully watched the position is reported as 0 so clients -// start fresh on the next play; otherwise the position is clamped to the item -// duration so a stale/overflowed stored position cannot drive the player past -// end-of-file (which stalls the HLS transcoder on resume). -func resumePositionTicks(position, duration float64, played bool) int64 { - if played { - return 0 - } +// clampResumeSeconds normalizes a stored resume position for client-facing +// user data. Completed rows store position 0, so fully watched items report 0 +// (clients start fresh) while a rewatch in flight reports its live resume +// point alongside Played=true — matching real Jellyfin. The position is +// clamped to the item duration so a stale/overflowed stored position cannot +// drive the player past end-of-file (which stalls the HLS transcoder on +// resume). +func clampResumeSeconds(position, duration float64) float64 { if duration > 0 && position > duration { position = duration } if position < 0 { position = 0 } - return secondsToTicks(position) + return position } func imageTagsWithSeed(signer *imageTagSigner, seed, imageURL string) map[string]string { diff --git a/internal/jellycompat/mapping_userdata_test.go b/internal/jellycompat/mapping_userdata_test.go index fc60887f..944192f7 100644 --- a/internal/jellycompat/mapping_userdata_test.go +++ b/internal/jellycompat/mapping_userdata_test.go @@ -6,9 +6,10 @@ import ( "github.com/Silo-Server/silo-server/internal/catalog" ) -func TestUserDataDTOPlayedZerosResumePosition(t *testing.T) { +func TestUserDataDTOPlayedReportsZeroPosition(t *testing.T) { + // Watched rows store position 0, so played items naturally report 0 ticks. data := &catalog.SeasonUserData{ - PositionSeconds: 1290.33, + PositionSeconds: 0, DurationSeconds: 1290.0, Played: true, } @@ -19,6 +20,27 @@ func TestUserDataDTOPlayedZerosResumePosition(t *testing.T) { if !dto.Played { t.Fatalf("Played = false, want true") } + if dto.PlayedPercentage != 100 { + t.Fatalf("PlayedPercentage = %v, want 100 for played item at rest", dto.PlayedPercentage) + } +} + +func TestUserDataDTOPlayedRewatchReportsResumePosition(t *testing.T) { + // A rewatch in flight keeps Played=true with a live resume point; clients + // must see both the checkmark and the position (matches Jellyfin). + data := &catalog.SeasonUserData{ + PositionSeconds: 600.0, + DurationSeconds: 1290.0, + Played: true, + } + dto := userDataDTO("item-1", data, false, nil) + want := secondsToTicks(600.0) + if dto.PlaybackPositionTicks != want { + t.Fatalf("PlaybackPositionTicks = %d, want %d for rewatch in flight", dto.PlaybackPositionTicks, want) + } + if !dto.Played { + t.Fatalf("Played = false, want true") + } } func TestUserDataDTOClampsPositionPastDuration(t *testing.T) { @@ -32,6 +54,9 @@ func TestUserDataDTOClampsPositionPastDuration(t *testing.T) { if dto.PlaybackPositionTicks != want { t.Fatalf("PlaybackPositionTicks = %d, want %d (clamped to duration)", dto.PlaybackPositionTicks, want) } + if dto.PlayedPercentage > 100 { + t.Fatalf("PlayedPercentage = %v, want <= 100 (derived from the clamped position)", dto.PlayedPercentage) + } } func TestUserDataDTOPreservesValidPosition(t *testing.T) { @@ -48,9 +73,10 @@ func TestUserDataDTOPreservesValidPosition(t *testing.T) { } func TestUserDataDTOProgressCompletedZeros(t *testing.T) { + // Completed rows store position 0, so watched items report 0 ticks. progress := &upstreamProgress{ MediaItemID: "x", - PositionSeconds: 1290.33, + PositionSeconds: 0, DurationSeconds: 1290.0, Completed: true, } @@ -61,6 +87,33 @@ func TestUserDataDTOProgressCompletedZeros(t *testing.T) { if !dto.Played { t.Fatalf("Played = false, want true") } + if dto.PlayedPercentage != 100 { + t.Fatalf("PlayedPercentage = %v, want 100 for completed item at rest", dto.PlayedPercentage) + } +} + +func TestUserDataDTOProgressRewatchKeepsPlayedAndPosition(t *testing.T) { + progress := &upstreamProgress{ + MediaItemID: "x", + PositionSeconds: 600.0, + DurationSeconds: 1290.0, + Completed: true, + } + dto := userDataDTO("item-4", nil, false, progress) + want := secondsToTicks(600.0) + if dto.PlaybackPositionTicks != want { + t.Fatalf("PlaybackPositionTicks = %d, want %d for rewatch in flight", dto.PlaybackPositionTicks, want) + } + if !dto.Played { + t.Fatalf("Played = false, want true") + } + if dto.PlayCount != 1 { + t.Fatalf("PlayCount = %d, want 1 (watched state survives rewatch)", dto.PlayCount) + } + wantPct := (600.0 / 1290.0) * 100 + if dto.PlayedPercentage != wantPct { + t.Fatalf("PlayedPercentage = %v, want %v (live rewatch fraction)", dto.PlayedPercentage, wantPct) + } } func TestUserDataDTOProgressClampsPosition(t *testing.T) { diff --git a/internal/userdb/migrate.go b/internal/userdb/migrate.go index afaf8fd9..8e949723 100644 --- a/internal/userdb/migrate.go +++ b/internal/userdb/migrate.go @@ -5,7 +5,7 @@ import ( "fmt" ) -const schemaVersion = 10 +const schemaVersion = 11 func runMigrations(db *sql.DB) error { version, err := userVersion(db) @@ -106,9 +106,37 @@ func runMigrations(db *sql.DB) error { } } + if version < 11 { + if err := migrateToV11(tx); err != nil { + return err + } + if _, err := tx.Exec("PRAGMA user_version = 11"); err != nil { + return fmt.Errorf("setting sqlite user_version 11: %w", err) + } + } + return tx.Commit() } +// migrateToV11 resets legacy completed watch_progress rows to +// position_seconds = 0. The watch-progress model keeps `completed` as a +// one-way watched latch with no resume point; the Continue Watching +// predicate is position_seconds > 0, so legacy rows (position pinned to the +// duration) would otherwise surface as phantom resume entries. Running this +// as a versioned migration (rather than a predicate-only data fix) matters: +// re-running the UPDATE on every boot would wipe the resume point of any +// rewatch in flight. +func migrateToV11(tx *sql.Tx) error { + if _, err := tx.Exec(` + UPDATE watch_progress + SET position_seconds = 0 + WHERE completed = 1 + AND position_seconds <> 0`); err != nil { + return fmt.Errorf("resetting completed watch_progress positions: %w", err) + } + return nil +} + func migrateToV10(tx *sql.Tx) error { if columnExists(tx, "watch_history", "watch_identity") { return nil diff --git a/internal/userdb/progress.go b/internal/userdb/progress.go index 4f0096ba..f34d6aec 100644 --- a/internal/userdb/progress.go +++ b/internal/userdb/progress.go @@ -20,12 +20,20 @@ type WatchHistoryEntry = userstore.WatchHistoryEntry // The position is only updated if the new value is greater than the existing one. // The completed flag is set to true when position/duration exceeds the watched threshold. func UpdateProgress(db *sql.DB, profileID, mediaItemID string, position, duration float64, thresholds userstore.ProgressThresholds) error { + if duration > 0 && position > 0 && position/duration < userstore.MinResumeFraction(thresholds.MinResumePct) { + return nil + } now := nowUTC() + // Mirrors the Postgres pgstore UpdateProgress: `completed` is a one-way + // watched latch; position resets to 0 on completion so a rewatch + // heartbeat on a completed row re-enters Continue Watching through plain + // MAX while the watched flag survives. query := ` INSERT INTO watch_progress (profile_id, media_item_id, position_seconds, duration_seconds, completed, updated_at) VALUES (?, ?, ?, ?, ?, ?) ON CONFLICT(profile_id, media_item_id) DO UPDATE SET - position_seconds = MAX(excluded.position_seconds, watch_progress.position_seconds), + position_seconds = CASE WHEN excluded.completed = 1 THEN 0 + ELSE MAX(excluded.position_seconds, watch_progress.position_seconds) END, duration_seconds = excluded.duration_seconds, completed = CASE WHEN excluded.completed = 1 THEN 1 ELSE watch_progress.completed END, @@ -34,7 +42,7 @@ func UpdateProgress(db *sql.DB, profileID, mediaItemID string, position, duratio completed := false if duration > 0 && position/duration > userstore.WatchedFraction(thresholds.WatchedPct) { completed = true - position = duration // match MarkWatched() — reset so no stale resume point + position = 0 // match MarkWatched() — completed rows hold no resume point } _, err := db.Exec(query, profileID, mediaItemID, position, duration, completed, now) if err != nil { @@ -44,13 +52,15 @@ func UpdateProgress(db *sql.DB, profileID, mediaItemID string, position, duratio } // SetProgress bypasses the forward-only guard (for rewatches/explicit seek). -// It unconditionally sets the position to the given value. +// It unconditionally sets the position to the given value. The completed flag +// stays a one-way watched latch: only ClearProgress/ClearProgressBatch (mark +// unwatched) release it. func SetProgress(db *sql.DB, profileID, mediaItemID string, position, duration float64, thresholds userstore.ProgressThresholds) error { now := nowUTC() completed := false if duration > 0 && position/duration > userstore.WatchedFraction(thresholds.WatchedPct) { completed = true - position = duration // match MarkWatched() — reset so no stale resume point + position = 0 // match MarkWatched() — completed rows hold no resume point } query := ` INSERT INTO watch_progress (profile_id, media_item_id, position_seconds, duration_seconds, completed, updated_at) @@ -58,7 +68,7 @@ func SetProgress(db *sql.DB, profileID, mediaItemID string, position, duration f ON CONFLICT(profile_id, media_item_id) DO UPDATE SET position_seconds = excluded.position_seconds, duration_seconds = excluded.duration_seconds, - completed = excluded.completed, + completed = watch_progress.completed OR excluded.completed, updated_at = excluded.updated_at ` _, err := db.Exec(query, profileID, mediaItemID, position, duration, completed, now) @@ -76,8 +86,8 @@ func SetProgressAt(db *sql.DB, profileID, mediaItemID string, position, duration if duration < 0 { duration = 0 } - if completed && duration > 0 { - position = duration + if completed { + position = 0 } if updatedAt.IsZero() { updatedAt = time.Now().UTC() @@ -96,7 +106,7 @@ func SetProgressAt(db *sql.DB, profileID, mediaItemID string, position, duration ON CONFLICT(profile_id, media_item_id) DO UPDATE SET position_seconds = excluded.position_seconds, duration_seconds = excluded.duration_seconds, - completed = excluded.completed, + completed = watch_progress.completed OR excluded.completed, updated_at = excluded.updated_at ` _, err = db.Exec(query, profileID, mediaItemID, position, duration, completed, updatedAtText) @@ -113,8 +123,8 @@ func SetProgressIfNewer(db *sql.DB, profileID, mediaItemID string, position, dur if duration < 0 { duration = 0 } - if completed && duration > 0 { - position = duration + if completed { + position = 0 } if updatedAt.IsZero() { updatedAt = time.Now().UTC() @@ -133,7 +143,7 @@ func SetProgressIfNewer(db *sql.DB, profileID, mediaItemID string, position, dur ON CONFLICT(profile_id, media_item_id) DO UPDATE SET position_seconds = excluded.position_seconds, duration_seconds = excluded.duration_seconds, - completed = excluded.completed, + completed = watch_progress.completed OR excluded.completed, updated_at = excluded.updated_at WHERE excluded.updated_at > watch_progress.updated_at `, profileID, mediaItemID, position, duration, completed, updatedAtText) @@ -156,14 +166,14 @@ func MarkWatched(db *sql.DB, profileID, mediaItemID string, duration float64) er now := nowUTC() query := ` INSERT INTO watch_progress (profile_id, media_item_id, position_seconds, duration_seconds, completed, updated_at) - VALUES (?, ?, ?, ?, 1, ?) + VALUES (?, ?, 0, ?, 1, ?) ON CONFLICT(profile_id, media_item_id) DO UPDATE SET - position_seconds = excluded.position_seconds, + position_seconds = 0, duration_seconds = excluded.duration_seconds, completed = 1, updated_at = excluded.updated_at ` - _, err := db.Exec(query, profileID, mediaItemID, duration, duration, now) + _, err := db.Exec(query, profileID, mediaItemID, duration, now) if err != nil { return fmt.Errorf("marking watched: %w", err) } @@ -207,8 +217,10 @@ func MarkProgressBatch(db *sql.DB, profileID string, mediaItemIDs []string, upda VALUES (?, ?, 0, 0, 1, ?) ON CONFLICT(profile_id, media_item_id) DO UPDATE SET completed = 1, + position_seconds = 0, updated_at = excluded.updated_at - WHERE watch_progress.completed != 1 OR watch_progress.updated_at < excluded.updated_at + WHERE watch_progress.completed != 1 + OR watch_progress.updated_at < excluded.updated_at `, profileID, mediaItemID, updatedAtText); err != nil { return fmt.Errorf("mark progress batch row: %w", err) } @@ -309,11 +321,14 @@ func ListProgress(db *sql.DB, profileID string, status string, limit, offset int switch status { case "in_progress": + // position_seconds > 0 (not completed = 0): completed rows hold + // position 0, so a rewatch of a watched item has completed = 1 with + // a live resume point and belongs in Continue Watching. query = ` SELECT profile_id, media_item_id, position_seconds, duration_seconds, completed, updated_at, last_file_id, last_resolution, last_hdr, last_codec_video, last_edition_key FROM watch_progress - WHERE profile_id = ? AND completed = 0 + WHERE profile_id = ? AND position_seconds > 0 AND NOT EXISTS ( SELECT 1 FROM hidden_history_items hhi diff --git a/internal/userdb/progress_rewatch_test.go b/internal/userdb/progress_rewatch_test.go new file mode 100644 index 00000000..841a3cdb --- /dev/null +++ b/internal/userdb/progress_rewatch_test.go @@ -0,0 +1,298 @@ +package userdb + +import ( + "database/sql" + "testing" + "time" + + "github.com/Silo-Server/silo-server/internal/userstore" +) + +func newRewatchTestDB(t *testing.T) *sql.DB { + t.Helper() + db, err := sql.Open("sqlite3", ":memory:") + if err != nil { + t.Fatalf("open sqlite: %v", err) + } + t.Cleanup(func() { _ = db.Close() }) + if err := InitSchema(db); err != nil { + t.Fatalf("InitSchema: %v", err) + } + return db +} + +func rewatchGetProgress(t *testing.T, db *sql.DB, profileID, itemID string) *WatchProgress { + t.Helper() + wp, err := GetProgress(db, profileID, itemID) + if err != nil { + t.Fatalf("GetProgress: %v", err) + } + if wp == nil { + t.Fatalf("GetProgress(%s) = nil, want row", itemID) + } + return wp +} + +func rewatchInProgressIDs(t *testing.T, db *sql.DB, profileID string) []string { + t.Helper() + entries, err := ListProgress(db, profileID, "in_progress", 50, 0) + if err != nil { + t.Fatalf("ListProgress: %v", err) + } + ids := make([]string, 0, len(entries)) + for _, e := range entries { + ids = append(ids, e.MediaItemID) + } + return ids +} + +// TestProgressRewatchLifecycle pins the watch → complete → rewatch → complete +// cycle of the position-based Continue Watching model: completion resets the +// resume point to 0 and latches completed; a later heartbeat re-enters the +// in-progress set through plain MAX while completed stays true; finishing the +// rewatch resets the resume point again. +func TestProgressRewatchLifecycle(t *testing.T) { + db := newRewatchTestDB(t) + th := userstore.ProgressThresholds{WatchedPct: 90, MinResumePct: 5} + + // First watch in progress. + if err := UpdateProgress(db, "p", "m", 600, 7200, th); err != nil { + t.Fatalf("UpdateProgress(first watch): %v", err) + } + wp := rewatchGetProgress(t, db, "p", "m") + if wp.Completed || wp.PositionSeconds != 600 { + t.Fatalf("first watch: completed=%v pos=%v, want false/600", wp.Completed, wp.PositionSeconds) + } + if ids := rewatchInProgressIDs(t, db, "p"); len(ids) != 1 || ids[0] != "m" { + t.Fatalf("in_progress = %v, want [m]", ids) + } + + // Completion: watched latch set, resume point cleared, leaves Continue Watching. + if err := UpdateProgress(db, "p", "m", 7000, 7200, th); err != nil { + t.Fatalf("UpdateProgress(completion): %v", err) + } + wp = rewatchGetProgress(t, db, "p", "m") + if !wp.Completed || wp.PositionSeconds != 0 { + t.Fatalf("completion: completed=%v pos=%v, want true/0", wp.Completed, wp.PositionSeconds) + } + if ids := rewatchInProgressIDs(t, db, "p"); len(ids) != 0 { + t.Fatalf("in_progress after completion = %v, want empty", ids) + } + + // Rewatch: re-enters Continue Watching, watched state survives. + if err := UpdateProgress(db, "p", "m", 900, 7200, th); err != nil { + t.Fatalf("UpdateProgress(rewatch): %v", err) + } + wp = rewatchGetProgress(t, db, "p", "m") + if !wp.Completed || wp.PositionSeconds != 900 { + t.Fatalf("rewatch: completed=%v pos=%v, want true/900", wp.Completed, wp.PositionSeconds) + } + if ids := rewatchInProgressIDs(t, db, "p"); len(ids) != 1 || ids[0] != "m" { + t.Fatalf("in_progress during rewatch = %v, want [m]", ids) + } + + // Rewatch completes: resume point cleared again, still watched. + if err := UpdateProgress(db, "p", "m", 7100, 7200, th); err != nil { + t.Fatalf("UpdateProgress(rewatch completion): %v", err) + } + wp = rewatchGetProgress(t, db, "p", "m") + if !wp.Completed || wp.PositionSeconds != 0 { + t.Fatalf("rewatch completion: completed=%v pos=%v, want true/0", wp.Completed, wp.PositionSeconds) + } + if ids := rewatchInProgressIDs(t, db, "p"); len(ids) != 0 { + t.Fatalf("in_progress after rewatch completion = %v, want empty", ids) + } +} + +func TestProgressRewatchGuards(t *testing.T) { + th := userstore.ProgressThresholds{WatchedPct: 90, MinResumePct: 5} + + t.Run("zero position heartbeat does not disturb watched item", func(t *testing.T) { + db := newRewatchTestDB(t) + if err := MarkWatched(db, "p", "m", 7200); err != nil { + t.Fatalf("MarkWatched: %v", err) + } + // Opening a watched item and backing out reports position 0. + if err := UpdateProgress(db, "p", "m", 0, 7200, th); err != nil { + t.Fatalf("UpdateProgress(zero): %v", err) + } + wp := rewatchGetProgress(t, db, "p", "m") + if !wp.Completed || wp.PositionSeconds != 0 { + t.Fatalf("got completed=%v pos=%v, want true/0", wp.Completed, wp.PositionSeconds) + } + if ids := rewatchInProgressIDs(t, db, "p"); len(ids) != 0 { + t.Fatalf("in_progress = %v, want empty", ids) + } + }) + + t.Run("below min-resume heartbeat is discarded", func(t *testing.T) { + db := newRewatchTestDB(t) + if err := MarkWatched(db, "p", "m", 7200); err != nil { + t.Fatalf("MarkWatched: %v", err) + } + // 100/7200 = 1.4% < 5% floor: no write, no resume entry. + if err := UpdateProgress(db, "p", "m", 100, 7200, th); err != nil { + t.Fatalf("UpdateProgress(below floor): %v", err) + } + wp := rewatchGetProgress(t, db, "p", "m") + if !wp.Completed || wp.PositionSeconds != 0 { + t.Fatalf("got completed=%v pos=%v, want true/0", wp.Completed, wp.PositionSeconds) + } + }) + + t.Run("mark watched cancels rewatch resume point", func(t *testing.T) { + db := newRewatchTestDB(t) + if err := MarkWatched(db, "p", "m", 7200); err != nil { + t.Fatalf("MarkWatched: %v", err) + } + if err := UpdateProgress(db, "p", "m", 900, 7200, th); err != nil { + t.Fatalf("UpdateProgress(rewatch): %v", err) + } + if err := MarkWatched(db, "p", "m", 7200); err != nil { + t.Fatalf("MarkWatched(again): %v", err) + } + wp := rewatchGetProgress(t, db, "p", "m") + if !wp.Completed || wp.PositionSeconds != 0 { + t.Fatalf("got completed=%v pos=%v, want true/0", wp.Completed, wp.PositionSeconds) + } + }) + + t.Run("mark progress batch clears stale resume points", func(t *testing.T) { + db := newRewatchTestDB(t) + // In-progress row at 40%, then series-level mark played. + if err := UpdateProgress(db, "p", "ep", 2880, 7200, th); err != nil { + t.Fatalf("UpdateProgress: %v", err) + } + if err := MarkProgressBatch(db, "p", []string{"ep"}, time.Time{}); err != nil { + t.Fatalf("MarkProgressBatch: %v", err) + } + wp := rewatchGetProgress(t, db, "p", "ep") + if !wp.Completed || wp.PositionSeconds != 0 { + t.Fatalf("got completed=%v pos=%v, want true/0", wp.Completed, wp.PositionSeconds) + } + if ids := rewatchInProgressIDs(t, db, "p"); len(ids) != 0 { + t.Fatalf("in_progress = %v, want empty", ids) + } + }) + + t.Run("playback stop mid-rewatch keeps the watched latch", func(t *testing.T) { + db := newRewatchTestDB(t) + if err := MarkWatched(db, "p", "m", 7200); err != nil { + t.Fatalf("MarkWatched: %v", err) + } + // RecordPlaybackStop persists the final position via SetProgress with + // completed=false (below the watched threshold). The latch must hold. + if err := SetProgress(db, "p", "m", 2880, 7200, th); err != nil { + t.Fatalf("SetProgress: %v", err) + } + wp := rewatchGetProgress(t, db, "p", "m") + if !wp.Completed || wp.PositionSeconds != 2880 { + t.Fatalf("got completed=%v pos=%v, want true/2880 (latch + resume point)", wp.Completed, wp.PositionSeconds) + } + }) + + t.Run("set progress if newer keeps the watched latch", func(t *testing.T) { + db := newRewatchTestDB(t) + if err := MarkWatched(db, "p", "m", 7200); err != nil { + t.Fatalf("MarkWatched: %v", err) + } + wrote, err := SetProgressIfNewer(db, "p", "m", 1800, 7200, false, time.Now().UTC().Add(time.Hour)) + if err != nil { + t.Fatalf("SetProgressIfNewer: %v", err) + } + if !wrote { + t.Fatalf("SetProgressIfNewer wrote = false, want true") + } + wp := rewatchGetProgress(t, db, "p", "m") + if !wp.Completed || wp.PositionSeconds != 1800 { + t.Fatalf("got completed=%v pos=%v, want true/1800 (latch + resume point)", wp.Completed, wp.PositionSeconds) + } + }) + + t.Run("stale mark batch does not zero a newer rewatch", func(t *testing.T) { + db := newRewatchTestDB(t) + if err := MarkWatched(db, "p", "ep", 7200); err != nil { + t.Fatalf("MarkWatched: %v", err) + } + if err := UpdateProgress(db, "p", "ep", 900, 7200, th); err != nil { + t.Fatalf("UpdateProgress(rewatch): %v", err) + } + // A delayed batch mark carrying an old timestamp must not clobber the + // live rewatch resume point. + if err := MarkProgressBatch(db, "p", []string{"ep"}, time.Now().UTC().Add(-time.Hour)); err != nil { + t.Fatalf("MarkProgressBatch(stale): %v", err) + } + wp := rewatchGetProgress(t, db, "p", "ep") + if !wp.Completed || wp.PositionSeconds != 900 { + t.Fatalf("got completed=%v pos=%v, want true/900 (stale batch ignored)", wp.Completed, wp.PositionSeconds) + } + }) + + t.Run("set progress at completed normalizes position to zero", func(t *testing.T) { + db := newRewatchTestDB(t) + // Import path (trakt/jellyfin) reporting a completed watch with a + // raw end-of-file position. + if err := SetProgressAt(db, "p", "m", 7200, 7200, true, time.Time{}); err != nil { + t.Fatalf("SetProgressAt: %v", err) + } + wp := rewatchGetProgress(t, db, "p", "m") + if !wp.Completed || wp.PositionSeconds != 0 { + t.Fatalf("got completed=%v pos=%v, want true/0", wp.Completed, wp.PositionSeconds) + } + }) +} + +// TestMigrateToV11ResetsCompletedPositions verifies the v11 userdb migration: +// an existing DB (user_version 10) gets its legacy completed rows (position +// pinned to duration) reset to 0, and the version gate prevents re-runs from +// wiping rewatch state. +func TestMigrateToV11ResetsCompletedPositions(t *testing.T) { + db, err := sql.Open("sqlite3", ":memory:") + if err != nil { + t.Fatalf("open sqlite: %v", err) + } + t.Cleanup(func() { _ = db.Close() }) + if err := InitSchema(db); err != nil { + t.Fatalf("InitSchema: %v", err) + } + + // Simulate an existing pre-v11 database: schema already at v10 with a + // legacy completed row written under the old model. + if _, err := db.Exec("PRAGMA user_version = 10"); err != nil { + t.Fatalf("set user_version: %v", err) + } + if _, err := db.Exec(` + INSERT INTO watch_progress (profile_id, media_item_id, position_seconds, duration_seconds, completed, updated_at) + VALUES ('p', 'legacy', 7200, 7200, 1, '2026-01-01T00:00:00Z')`); err != nil { + t.Fatalf("seed legacy row: %v", err) + } + + if err := runMigrations(db); err != nil { + t.Fatalf("runMigrations: %v", err) + } + wp := rewatchGetProgress(t, db, "p", "legacy") + if !wp.Completed || wp.PositionSeconds != 0 { + t.Fatalf("after migration: completed=%v pos=%v, want true/0", wp.Completed, wp.PositionSeconds) + } + version, err := userVersion(db) + if err != nil { + t.Fatalf("userVersion: %v", err) + } + if version != schemaVersion { + t.Fatalf("user_version = %d, want %d", version, schemaVersion) + } + + // A rewatch in flight must survive subsequent runs (the version gate + // blocks re-runs). + th := userstore.ProgressThresholds{WatchedPct: 90, MinResumePct: 5} + if err := UpdateProgress(db, "p", "legacy", 900, 7200, th); err != nil { + t.Fatalf("UpdateProgress(rewatch): %v", err) + } + if err := runMigrations(db); err != nil { + t.Fatalf("runMigrations(again): %v", err) + } + wp = rewatchGetProgress(t, db, "p", "legacy") + if !wp.Completed || wp.PositionSeconds != 900 { + t.Fatalf("after re-run: completed=%v pos=%v, want true/900 (rewatch preserved)", wp.Completed, wp.PositionSeconds) + } +} diff --git a/internal/userstore/pgstore/progress.go b/internal/userstore/pgstore/progress.go index 7556c320..e372836f 100644 --- a/internal/userstore/pgstore/progress.go +++ b/internal/userstore/pgstore/progress.go @@ -58,13 +58,19 @@ func (s *PostgresUserStore) UpdateProgress(ctx context.Context, profileID, media completed := false if duration > 0 && position/duration > userstore.WatchedFraction(thresholds.WatchedPct) { completed = true - position = duration // match MarkWatched() — reset so no stale resume point + position = 0 // match MarkWatched() — completed rows hold no resume point } + // `completed` is a one-way watched latch; position resets to 0 on + // completion so `position_seconds > 0` means "has a resume point". A + // rewatch heartbeat on a completed row therefore re-enters Continue + // Watching through plain GREATEST (stored position is 0) while the + // watched flag survives. _, err := s.pool.Exec(ctx, ` INSERT INTO user_watch_progress (user_id, profile_id, media_item_id, position_seconds, duration_seconds, completed, updated_at) VALUES ($1, $2, $3, $4, $5, $6, $7) ON CONFLICT(user_id, profile_id, media_item_id) DO UPDATE SET - position_seconds = GREATEST(excluded.position_seconds, user_watch_progress.position_seconds), + position_seconds = CASE WHEN excluded.completed THEN 0 + ELSE GREATEST(excluded.position_seconds, user_watch_progress.position_seconds) END, duration_seconds = excluded.duration_seconds, completed = CASE WHEN excluded.completed THEN TRUE ELSE user_watch_progress.completed END, @@ -85,7 +91,7 @@ func (s *PostgresUserStore) SetProgress(ctx context.Context, profileID, mediaIte completed := false if duration > 0 && position/duration > userstore.WatchedFraction(thresholds.WatchedPct) { completed = true - position = duration // match MarkWatched() — reset so no stale resume point + position = 0 // match MarkWatched() — completed rows hold no resume point } _, err := s.pool.Exec(ctx, ` INSERT INTO user_watch_progress (user_id, profile_id, media_item_id, position_seconds, duration_seconds, completed, updated_at) @@ -93,7 +99,7 @@ func (s *PostgresUserStore) SetProgress(ctx context.Context, profileID, mediaIte ON CONFLICT(user_id, profile_id, media_item_id) DO UPDATE SET position_seconds = excluded.position_seconds, duration_seconds = excluded.duration_seconds, - completed = excluded.completed, + completed = user_watch_progress.completed OR excluded.completed, updated_at = excluded.updated_at`, s.userID, profileID, mediaItemID, position, duration, completed, now, ) @@ -110,8 +116,8 @@ func (s *PostgresUserStore) SetProgressAt(ctx context.Context, profileID, mediaI if duration < 0 { duration = 0 } - if completed && duration > 0 { - position = duration + if completed { + position = 0 } if updatedAt.IsZero() { updatedAt = time.Now().UTC() @@ -130,7 +136,7 @@ func (s *PostgresUserStore) SetProgressAt(ctx context.Context, profileID, mediaI ON CONFLICT(user_id, profile_id, media_item_id) DO UPDATE SET position_seconds = excluded.position_seconds, duration_seconds = excluded.duration_seconds, - completed = excluded.completed, + completed = user_watch_progress.completed OR excluded.completed, updated_at = excluded.updated_at`, s.userID, profileID, mediaItemID, position, duration, completed, updatedAt.UTC(), ) @@ -147,8 +153,8 @@ func (s *PostgresUserStore) SetProgressIfNewer(ctx context.Context, profileID, m if duration < 0 { duration = 0 } - if completed && duration > 0 { - position = duration + if completed { + position = 0 } if updatedAt.IsZero() { updatedAt = time.Now().UTC() @@ -186,13 +192,13 @@ func (s *PostgresUserStore) MarkWatched(ctx context.Context, profileID, mediaIte now := nowUTC() _, err := s.pool.Exec(ctx, ` INSERT INTO user_watch_progress (user_id, profile_id, media_item_id, position_seconds, duration_seconds, completed, updated_at) - VALUES ($1, $2, $3, $4, $5, TRUE, $6) + VALUES ($1, $2, $3, 0, $4, TRUE, $5) ON CONFLICT(user_id, profile_id, media_item_id) DO UPDATE SET - position_seconds = excluded.position_seconds, + position_seconds = 0, duration_seconds = excluded.duration_seconds, completed = TRUE, updated_at = excluded.updated_at`, - s.userID, profileID, mediaItemID, duration, duration, now, + s.userID, profileID, mediaItemID, duration, now, ) if err != nil { return fmt.Errorf("marking watched: %w", err) @@ -230,6 +236,7 @@ func (s *PostgresUserStore) MarkProgressBatch(ctx context.Context, profileID str FROM unnest($3::text[]) AS mid ON CONFLICT (user_id, profile_id, media_item_id) DO UPDATE SET completed = TRUE, + position_seconds = 0, updated_at = EXCLUDED.updated_at WHERE user_watch_progress.completed IS DISTINCT FROM TRUE OR user_watch_progress.updated_at < EXCLUDED.updated_at`, @@ -304,11 +311,14 @@ func (s *PostgresUserStore) ListProgress(ctx context.Context, profileID, status switch status { case "in_progress": + // position_seconds > 0 (not completed = FALSE): completed rows hold + // position 0, so a rewatch of a watched item has completed = TRUE with + // a live resume point and belongs in Continue Watching. query = ` SELECT profile_id, media_item_id, position_seconds, duration_seconds, completed, updated_at, last_file_id, last_resolution, last_hdr, last_codec_video, last_edition_key FROM user_watch_progress - WHERE user_id = $1 AND profile_id = $2 AND completed = FALSE + WHERE user_id = $1 AND profile_id = $2 AND position_seconds > 0 AND NOT EXISTS ( SELECT 1 FROM user_history_hidden_items hhi diff --git a/internal/userstore/storetest/suite.go b/internal/userstore/storetest/suite.go index c66bec53..e1bc0363 100644 --- a/internal/userstore/storetest/suite.go +++ b/internal/userstore/storetest/suite.go @@ -247,7 +247,7 @@ func testProgress(t *testing.T, newStore func(t *testing.T) userstore.UserStore) t.Errorf("PositionSeconds after SetProgress backward = %v, want 500", wp.PositionSeconds) } - // Completion flag: >90% + // Completion flag: >90%. Completed rows hold no resume point. if err := store.SetProgress(ctx, "p1", "movie-2", 6500, 7200, noThreshold); err != nil { t.Fatalf("SetProgress(near end): %v", err) } @@ -255,6 +255,9 @@ func testProgress(t *testing.T, newStore func(t *testing.T) userstore.UserStore) if !wp.Completed { t.Error("Expected Completed=true for >90% progress") } + if wp.PositionSeconds != 0 { + t.Errorf("PositionSeconds after completion = %v, want 0", wp.PositionSeconds) + } // Min-resume threshold: below 5% should be silently discarded if err := store.SetProgress(ctx, "p1", "movie-tiny", 10, 7200, noThreshold); err != nil { @@ -486,8 +489,8 @@ func testProgress(t *testing.T, newStore func(t *testing.T) userstore.UserStore) if !wp.Completed { t.Error("MarkWatched should create a completed progress row") } - if wp.PositionSeconds != 5400 || wp.DurationSeconds != 5400 { - t.Errorf("MarkWatched stored position=%v duration=%v, want 5400/5400", wp.PositionSeconds, wp.DurationSeconds) + if wp.PositionSeconds != 0 || wp.DurationSeconds != 5400 { + t.Errorf("MarkWatched stored position=%v duration=%v, want 0/5400", wp.PositionSeconds, wp.DurationSeconds) } if err := store.ClearProgress(ctx, "p1", "movie-3"); err != nil { diff --git a/migrations/sql/20260609224115_reset_completed_progress_position.sql b/migrations/sql/20260609224115_reset_completed_progress_position.sql new file mode 100644 index 00000000..12727a13 --- /dev/null +++ b/migrations/sql/20260609224115_reset_completed_progress_position.sql @@ -0,0 +1,20 @@ +-- +goose Up +-- Watch-progress model change: `completed` is a one-way watched latch and +-- completed rows hold no resume point (position_seconds = 0), mirroring +-- Jellyfin. The Continue Watching predicate becomes position_seconds > 0, so +-- a rewatch of a watched item re-enters the row naturally while keeping its +-- watched state. Legacy completed rows pinned position_seconds to +-- duration_seconds; reset them so they don't surface as phantom resume +-- entries under the new predicate. +UPDATE user_watch_progress +SET position_seconds = 0 +WHERE completed = TRUE + AND position_seconds <> 0; + +-- +goose Down +-- Restore the legacy convention (completed rows pinned to duration) so the +-- old `completed = FALSE` resume predicate doesn't surface watched items. +UPDATE user_watch_progress +SET position_seconds = duration_seconds +WHERE completed = TRUE + AND duration_seconds > 0; diff --git a/web/src/components/EpisodeRow.tsx b/web/src/components/EpisodeRow.tsx index 65b13c02..28839a4e 100644 --- a/web/src/components/EpisodeRow.tsx +++ b/web/src/components/EpisodeRow.tsx @@ -16,7 +16,9 @@ export default function EpisodeRow({ episode, rating, watched, progress }: Episo (() => { const position = episode.user_data?.position_seconds ?? 0; const duration = episode.user_data?.duration_seconds ?? 0; - if (duration <= 0 || position <= 0 || watchedState) { + // Watched episodes store position 0; a nonzero position on a watched + // episode is a rewatch in flight and should show its bar. + if (duration <= 0 || position <= 0) { return undefined; } return Math.max(0, Math.min(100, (position / duration) * 100)); diff --git a/web/src/components/HeroBanner.tsx b/web/src/components/HeroBanner.tsx index 8302ef05..37e6fb73 100644 --- a/web/src/components/HeroBanner.tsx +++ b/web/src/components/HeroBanner.tsx @@ -54,14 +54,14 @@ function heroPlayLabel(item: SectionItem, activeAudiobookPlaying?: boolean | nul if (activeAudiobookPlaying === false) { return "Resume"; } - if (item.user_state?.played) { - return "Listen Again"; - } const position = item.position_seconds ?? 0; const duration = item.duration_seconds ?? 0; if (position > 0 && (duration <= 0 || position < duration)) { return "Resume"; } + if (item.user_state?.played) { + return "Listen Again"; + } return "Listen"; } diff --git a/web/src/hooks/queries/playbackProgressCache.test.ts b/web/src/hooks/queries/playbackProgressCache.test.ts index 90970213..00cc0ae3 100644 --- a/web/src/hooks/queries/playbackProgressCache.test.ts +++ b/web/src/hooks/queries/playbackProgressCache.test.ts @@ -153,7 +153,7 @@ describe("applyPlaybackProgressToCache", () => { ]); }); - it("turns a previously played item back into resume state when exit progress is partial", () => { + it("keeps the watched badge while a rewatch is in progress", () => { const queryClient = new QueryClient(); queryClient.setQueryData(itemKeys.detail("movie-1"), { @@ -172,10 +172,12 @@ describe("applyPlaybackProgressToCache", () => { durationSeconds: 3600, }); + // played is a one-way latch mirroring the server model: the rewatch gets + // a live resume point without clearing watched state. expect( queryClient.getQueryData(itemKeys.detail("movie-1"))?.user_data, ).toMatchObject({ - played: false, + played: true, is_in_progress: true, position_seconds: 300, duration_seconds: 3600, diff --git a/web/src/hooks/queries/playbackProgressCache.ts b/web/src/hooks/queries/playbackProgressCache.ts index b03a44ad..82490588 100644 --- a/web/src/hooks/queries/playbackProgressCache.ts +++ b/web/src/hooks/queries/playbackProgressCache.ts @@ -18,16 +18,19 @@ function mergeLeafProgress( existing: LeafItemUserData | undefined, snapshot: PlaybackProgressSnapshot, ): LeafItemUserData { - const positionSeconds = Math.max(0, snapshot.positionSeconds); + const rawPosition = Math.max(0, snapshot.positionSeconds); const durationSeconds = snapshot.durationSeconds ?? existing?.duration_seconds; - const played = - durationSeconds != null && durationSeconds > 0 - ? positionSeconds >= durationSeconds - : existing?.played === true && positionSeconds === 0; + // Mirror the server model: played is a one-way latch (a rewatch keeps the + // watched badge), completion clears the resume point, and any nonzero + // position is an active resume point. + const completedNow = + durationSeconds != null && durationSeconds > 0 && rawPosition >= durationSeconds; + const played = existing?.played === true || completedNow; + const positionSeconds = completedNow ? 0 : rawPosition; return { played, - is_in_progress: positionSeconds > 0 && !played, + is_in_progress: positionSeconds > 0, position_seconds: positionSeconds, duration_seconds: durationSeconds, last_file_id: snapshot.lastFileId ?? existing?.last_file_id, @@ -91,20 +94,20 @@ export function applyPlaybackProgressToCache( })) { if (!existing) continue; - const nextProgress = existing.progress.map((entry) => - entry.media_item_id === snapshot.contentId - ? { - ...entry, - position_seconds: Math.max(0, snapshot.positionSeconds), - duration_seconds: snapshot.durationSeconds ?? entry.duration_seconds, - completed: - (snapshot.durationSeconds ?? entry.duration_seconds) > 0 && - Math.max(0, snapshot.positionSeconds) >= - (snapshot.durationSeconds ?? entry.duration_seconds), - updated_at: updatedAt, - } - : entry, - ); + const nextProgress = existing.progress.map((entry) => { + if (entry.media_item_id !== snapshot.contentId) return entry; + const rawPosition = Math.max(0, snapshot.positionSeconds); + const duration = snapshot.durationSeconds ?? entry.duration_seconds; + // Mirror the server model: completion latches and clears the resume point. + const completedNow = duration > 0 && rawPosition >= duration; + return { + ...entry, + position_seconds: completedNow ? 0 : rawPosition, + duration_seconds: duration, + completed: entry.completed || completedNow, + updated_at: updatedAt, + }; + }); queryClient.setQueryData(queryKey, { ...existing, diff --git a/web/src/pages/ItemDetail/EpisodeContent.test.tsx b/web/src/pages/ItemDetail/EpisodeContent.test.tsx index 2cafb608..bb2c0d55 100644 --- a/web/src/pages/ItemDetail/EpisodeContent.test.tsx +++ b/web/src/pages/ItemDetail/EpisodeContent.test.tsx @@ -354,8 +354,9 @@ describe("EpisodeContent", () => { { { }); }); + it("offers resume and restart during a rewatch of a completed movie", () => { + renderToStaticMarkup( + + + , + ); + + expect(mocks.capturedActionBarProps.value).toMatchObject({ + playLabel: "Resume", + restartHref: "/watch/movie-1?restart=1", + }); + }); + it("allows marker editing without metadata curation permission", () => { mocks.useAuth.mockReturnValue({ user: { role: "user", permissions: ["marker_edit"], download_allowed: true }, diff --git a/web/src/pages/ItemDetail/itemDetailLayout.test.ts b/web/src/pages/ItemDetail/itemDetailLayout.test.ts index 75501cfd..38179a55 100644 --- a/web/src/pages/ItemDetail/itemDetailLayout.test.ts +++ b/web/src/pages/ItemDetail/itemDetailLayout.test.ts @@ -253,7 +253,26 @@ describe("resolveLeafPrimaryAction", () => { ), ).toEqual({ label: "Play Episode", - progress: 0, + progress: undefined, + }); + }); + + it("offers resume for a rewatch of a played item", () => { + expect( + resolveLeafPrimaryAction( + makeEpisodeItem({ + user_data: { + played: true, + is_in_progress: true, + position_seconds: 600, + duration_seconds: 2400, + }, + }), + "Play Episode", + ), + ).toEqual({ + label: "Resume", + progress: 25, }); }); }); diff --git a/web/src/pages/ItemDetail/itemDetailLayout.ts b/web/src/pages/ItemDetail/itemDetailLayout.ts index 2ad884f6..64c686a5 100644 --- a/web/src/pages/ItemDetail/itemDetailLayout.ts +++ b/web/src/pages/ItemDetail/itemDetailLayout.ts @@ -47,9 +47,12 @@ export function resolveLeafPrimaryAction( const positionSeconds = userData.position_seconds ?? 0; const durationSeconds = userData.duration_seconds ?? 0; const progress = - durationSeconds > 0 ? clampProgress((positionSeconds / durationSeconds) * 100) : undefined; - const isInProgress = - userData.played !== true && ((userData.is_in_progress ?? false) || positionSeconds > 0); + positionSeconds > 0 && durationSeconds > 0 + ? clampProgress((positionSeconds / durationSeconds) * 100) + : undefined; + // Watched items store position 0, so any nonzero position is a live resume + // point — including a rewatch in flight (played stays true). + const isInProgress = (userData.is_in_progress ?? false) || positionSeconds > 0; if (isInProgress) { return { diff --git a/web/src/pages/watchRouteHelpers.ts b/web/src/pages/watchRouteHelpers.ts index e002314b..41be892c 100644 --- a/web/src/pages/watchRouteHelpers.ts +++ b/web/src/pages/watchRouteHelpers.ts @@ -246,11 +246,9 @@ export function buildWatchPageProps({ const autoSkipIntro = currentProfile?.auto_skip_intro ?? false; const autoSkipRecap = currentProfile?.auto_skip_recap ?? false; const autoPlayNextPreview = currentProfile?.auto_play_next_preview ?? false; - const initialPosition = request.restart - ? 0 - : item.user_data?.played === true - ? 0 - : (item.user_data?.position_seconds ?? 0); + // Watched items store position 0, so any nonzero position is a live resume + // point — including a rewatch in flight (played stays true). + const initialPosition = request.restart ? 0 : (item.user_data?.position_seconds ?? 0); const resumeHints: ResumeHints | undefined = item.user_data?.last_file_id != null ||