Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions openspec/changes/fix-autoplay-next-lesson-loop/.openspec.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
schema: spec-driven
created: 2026-07-27
38 changes: 38 additions & 0 deletions openspec/changes/fix-autoplay-next-lesson-loop/design.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
## Context

When the "Auto-Play Next Lesson" setting is enabled, completing a video lesson automatically triggers navigation to the next lesson. However, because the app saves the playback progress, navigating back to a completed video lesson causes the player to initialize and seek to the end of the video. Once the seek completes, the player's position changes to the end, which is immediately interpreted as a new completion event. Since the orchestrator's state (including completion flags) is re-initialized on navigation, it immediately fires the `onNext` callback, throwing the user forward again in an inescapable navigation loop.

## Goals / Non-Goals

**Goals:**
- Prevent automatic navigation to the next lesson when navigating back to or reloading a completed video lesson.
- Ensure that completed video lessons start playback from the beginning (`0.0`) when opened, so the user can actually replay them.
- Keep the `onComplete` trigger functional if the user seeks backwards or chooses to replay the video from the beginning.

**Non-Goals:**
- Changing the global autoplay logic for non-video content types.
- Modifying how playback tracking (attempts) are synchronized to the backend.

## Decisions

### 1. Reset initial playback position for completed video lessons
In `VideoLessonViewer` and `VideoLessonDetailScreen`, check if the lesson has been completed (`lesson.progressStatus == LessonProgressStatus.completed`). If it has, set `initialPosition` to `0.0`.

- *Rationale:* If the user has finished a video, returning to it should let them watch it again from the beginning, rather than staring at a black ended screen. This naturally avoids seeking to the end on initialization.

### 2. Introduce an initial completion guard in `CustomVideoPlayer`
Add a `_shouldIgnoreInitialCompletion` boolean flag inside `CustomVideoPlayerState`. During the first build/seek (where `_hasSeekedToInitial` is processed), if the target seek position is within a near-end threshold (defined dynamically as `duration > 2.0 ? 2.0 : duration * 0.5`) of the video duration, set this flag to `true`. In the player's listener, check this flag before firing `widget.onComplete`.

- *Rationale:* This serves as a safety guard. If the initial position is at/near the end of the video, we do not want to trigger `onComplete` immediately on load. The dynamic threshold ensures short clips (≤ 5s) are also guarded without activating too early on their initial playback.

### 3. Reset the guard on user interaction, backward seek, or forward playback progress
If `_shouldIgnoreInitialCompletion` is `true`, reset it to `false` when:
- The current position has advanced forward from the initial seek position by a small margin (e.g., `currentPos > _initialSeekPos + 0.1`).
- The user seeks backwards from the initial seek position (e.g., `currentPos < _initialSeekPos - 0.5`).

- *Rationale:* This ensures that if a user resumes a video near the end and actually starts playing it, the video can still complete naturally and trigger auto-play. It also allows completion if they seek backwards to watch from an earlier point. It only suppresses completion if the video loads and is immediately evaluated at the end without any actual forward progress or user interaction.

## Risks / Trade-offs

- **Risk**: Video playback state might not update the duration immediately when `_hasSeekedToInitial` is evaluated.
- *Mitigation*: We only perform the initial seek and set the ignore flag when `controller.value.duration != Duration.zero` is true.
20 changes: 20 additions & 0 deletions openspec/changes/fix-autoplay-next-lesson-loop/proposal.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
## Why

When the Auto-Play Next Lesson feature is enabled, navigating back to a previously completed video lesson (using the Previous button or other navigation) causes the app to immediately auto-play the next lesson again. This is because the video player seeks to the end (its last watched position), triggering a video completion event which immediately auto-navigates the user forward, creating an inescapable loop.

## What Changes

- Modify completed video lessons to initialize their playback position at `0.0` rather than the end of the video, so that users can replay them from the beginning.
- Introduce an initialization completion guard in `CustomVideoPlayer` that ignores completion events if the video was initialized at or near the end.
- Allow completion events to fire normally if the user actively seeks backward or replays the video from a non-completed position.

## Capabilities

### Modified Capabilities
- `unified-lesson-shell`: Refine "AutoPlay Next Video Evaluation" to specify that initialization or seek-on-load to near-end positions of a video lesson must not trigger automatic navigation.

## Impact

- `packages/courses/lib/widgets/lesson_detail/custom_video_player.dart`
- `packages/courses/lib/widgets/lesson_detail/video_lesson_viewer.dart`
- `packages/courses/lib/screens/video_lesson_detail_screen.dart`
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
## MODIFIED Requirements

### Requirement: AutoPlay Next Video Evaluation
The lesson shell SHALL evaluate whether to automatically navigate to the next lesson when a video completes. The completion event SHALL NOT trigger automatic navigation if the video was initialized or seeked to a near-end position on initial load.

#### Scenario: AutoPlay enabled
- **WHEN** a video lesson completes naturally, and the `autoPlayNext` user setting is enabled
- **THEN** the system SHALL automatically navigate to the next lesson, regardless of its content type.

#### Scenario: AutoPlay disabled
- **WHEN** a video lesson completes and the `autoPlayNext` user setting is disabled
- **THEN** the system SHALL NOT automatically navigate to the next lesson.

#### Scenario: Video loaded at the end with AutoPlay enabled
- **WHEN** a video lesson is opened, and the initial position is at or near the end of the video
- **THEN** the system SHALL NOT trigger a completion event or automatically navigate to the next lesson.
11 changes: 11 additions & 0 deletions openspec/changes/fix-autoplay-next-lesson-loop/tasks.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
## 1. Lesson Viewers initial position reset

- [x] 1.1 In `video_lesson_viewer.dart`, reset `initialPos` to `0.0` if `widget.lesson.progressStatus == LessonProgressStatus.completed`.
- [x] 1.2 In `video_lesson_detail_screen.dart`, reset `initialPos` to `0.0` if `widget.lesson.progressStatus == LessonProgressStatus.completed`.

## 2. CustomVideoPlayer initial completion guard

- [x] 2.1 In `custom_video_player.dart`, define the `_shouldIgnoreInitialCompletion` flag in `CustomVideoPlayerState`.
- [x] 2.2 In `custom_video_player.dart`, store `_initialSeekPos` and set `_shouldIgnoreInitialCompletion = true` if `targetSeek` is within the dynamic threshold `duration > 2.0 ? 2.0 : duration * 0.5` of the end.
- [x] 2.3 In `custom_video_player.dart` controller listener, reset `_shouldIgnoreInitialCompletion` to `false` when `currentPos > _initialSeekPos + 0.1` or `currentPos < _initialSeekPos - 0.5`.
- [x] 2.4 In `custom_video_player.dart` controller listener, guard `widget.onComplete?.call()` to only run when `!_shouldIgnoreInitialCompletion`.
8 changes: 6 additions & 2 deletions openspec/specs/unified-lesson-shell/spec.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,17 @@ The bookmark icon in the lesson details header SHALL render in an active (filled
- **THEN** the header bookmark icon SHALL be displayed as a filled icon.

### Requirement: AutoPlay Next Video Evaluation
The lesson shell SHALL evaluate whether to automatically navigate to the next lesson when a video completes.
The lesson shell SHALL evaluate whether to automatically navigate to the next lesson when a video completes. The completion event SHALL NOT trigger automatic navigation if the video was initialized or seeked to a near-end position on initial load.

#### Scenario: AutoPlay enabled
- **WHEN** a video lesson completes and the `autoPlayNext` user setting is enabled
- **WHEN** a video lesson completes naturally, and the `autoPlayNext` user setting is enabled
- **THEN** the system SHALL automatically navigate to the next lesson, regardless of its content type.

#### Scenario: AutoPlay disabled
- **WHEN** a video lesson completes and the `autoPlayNext` user setting is disabled
- **THEN** the system SHALL NOT automatically navigate to the next lesson.

#### Scenario: Video loaded at the end with AutoPlay enabled
- **WHEN** a video lesson is opened, and the initial position is at or near the end of the video
- **THEN** the system SHALL NOT trigger a completion event or automatically navigate to the next lesson.

7 changes: 5 additions & 2 deletions packages/courses/lib/screens/video_lesson_detail_screen.dart
Original file line number Diff line number Diff line change
Expand Up @@ -147,8 +147,11 @@ class _VideoLessonDetailScreenState
@override
Widget build(BuildContext context) {
final design = Design.of(context);
final initialPos =
double.tryParse(widget.lesson.lastWatchedDuration ?? '0') ?? 0.0;
final isCompleted =
widget.lesson.progressStatus == LessonProgressStatus.completed;
final initialPos = isCompleted
? 0.0
: (double.tryParse(widget.lesson.lastWatchedDuration ?? '0') ?? 0.0);

final padding = MediaQuery.of(context).padding;
final l10n = L10n.of(context);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -192,28 +192,50 @@ class CustomVideoPlayerState extends ConsumerState<CustomVideoPlayer> {

bool _hasSeekedToInitial = false;
double? _pendingSeekPosition;
bool _shouldIgnoreInitialCompletion = false;
double _initialSeekPos = 0.0;

void _onPlayerCreated(TestpressPlayerController controller) {
_controller = controller;

controller.addListener(() {
final isPlaying = controller.value.isPlaying;
final currentPos = controller.value.position.inMilliseconds / 1000.0;
final duration = controller.value.duration.inMilliseconds / 1000.0;

// Ensure we only seek once the video is loaded (duration > 0)
final targetSeek = _pendingSeekPosition ?? widget.initialPosition;
final needsInitialSeek = targetSeek > 0 && !_hasSeekedToInitial;
if (needsInitialSeek) {
if (!_hasSeekedToInitial) {
if (controller.value.duration != Duration.zero) {
controller.seek(Duration(milliseconds: (targetSeek * 1000).toInt()));
_lastPosition = targetSeek;
_currentIntervalStart = targetSeek;
final targetSeek = _pendingSeekPosition ?? widget.initialPosition;
if (targetSeek > 0) {
controller
.seek(Duration(milliseconds: (targetSeek * 1000).toInt()));
_lastPosition = targetSeek;
_currentIntervalStart = targetSeek;
_initialSeekPos = targetSeek;
// Guard: If the initial position is close to the end, ignore the completion trigger
final nearEndThreshold = duration > 2.0 ? 2.0 : (duration * 0.5);
if (targetSeek >= duration - nearEndThreshold) {
_shouldIgnoreInitialCompletion = true;
}
} else {
_initialSeekPos = 0.0;
}
_hasSeekedToInitial = true;
_pendingSeekPosition = null;
}
return;
}

// Reset the ignore flag if the user seeks backwards or plays forward past the initial seek position
if (_shouldIgnoreInitialCompletion) {
final isProgressingForward = currentPos > _initialSeekPos + 0.1;
final isSeekingBackward = currentPos < _initialSeekPos - 0.5;
if (isProgressingForward || isSeekingBackward) {
_shouldIgnoreInitialCompletion = false;
}
}

// Detect seek (position jumped by more than 1.5s or went backwards)
final isSeeking = (currentPos - _lastPosition).abs() > 1.5;
if (isSeeking) {
Expand All @@ -235,7 +257,8 @@ class CustomVideoPlayerState extends ConsumerState<CustomVideoPlayer> {
}

// Check for completion
if (controller.value.position >= controller.value.duration &&
if (!_shouldIgnoreInitialCompletion &&
controller.value.position >= controller.value.duration &&
controller.value.duration != Duration.zero) {
widget.onComplete?.call();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -176,8 +176,11 @@ class _VideoLessonViewerState extends State<VideoLessonViewer>
}

Widget _buildVideoSection(DesignConfig design) {
final initialPos =
double.tryParse(widget.lesson.lastWatchedDuration ?? '0') ?? 0.0;
final isCompleted =
widget.lesson.progressStatus == LessonProgressStatus.completed;
final initialPos = isCompleted
? 0.0
: (double.tryParse(widget.lesson.lastWatchedDuration ?? '0') ?? 0.0);

return CustomVideoPlayer(
key: _videoPlayerKey,
Expand Down