Daily audio: engagement uses furthest-reached, not resume pointer#646
Merged
Conversation
is_engaged (which gates whether tomorrow's lesson is pre-generated) read pause_position_seconds — the RESUME pointer, which drops when the learner rewinds or scrubs back. So someone who heard 80% then jumped back to re-hear a sentence and paused there was scored on that lower position and wrongly marked "not engaged", holding the next day's lesson. Add max_position_seconds: a monotonic high-water-mark of the furthest position ever reached. pause_at() grows it; mark_completed() sets it to the full duration. is_engaged now measures against it (falling back to the resume pointer for pre-migration rows). pause_position_seconds keeps its resume role unchanged. Migration adds the column and backfills existing rows from pause_position_seconds (and full duration for completed lessons) so the gate doesn't regress. Response exposes max_position_seconds for the web "listen past halfway to unlock tomorrow" hint. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
ArchLens - No architecturally relevant changes to the existing views |
- update_lesson_state_for_user: coerce position_seconds (max(0, int(...))) so an explicit null/string/negative from the client can't crash pause_at (None > duration → TypeError → 500) or corrupt is_paused / the resume listened_count via a negative value. - migration: drop the always-zero COALESCE(max_position_seconds, 0) term from the backfill GREATEST (column is brand-new, so it's 0 everywhere). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
8 unit tests on DailyAudioLesson.is_engaged / pause_at / mark_completed, including the regression guard for this PR: reach 80%, rewind+pause at 30s, still engaged (the old pause_position-based gate read it as not engaged). Also covers clamping, completion crediting full duration, the legacy-row fallback to pause_position, and the no-duration path. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
is_engaged(which gates whether tomorrow's lesson is pre-generated) readpause_position_seconds— the resume pointer, which moves backwards when the learner rewinds / scrubs back to re-hear something. So a learner who listened to ~80% then jumped back to re-hear a sentence and paused there was scored on that lower position and wrongly marked not engaged, holding the next day's lesson.Fix
max_position_seconds: a monotonic high-water-mark of the furthest position ever reached.pause_at()grows it (max(old, clamped)).mark_completed()sets it to the full duration.is_engagednow measures against the furthest reached (falling back to the resume pointer for pre-migration rows).pause_position_secondskeeps its resume role unchanged.pause_position_seconds(and full duration for completed lessons) so the gate doesn't regress.get_todays_lessonresponse exposesmax_position_seconds(consumed by the web "listen past halfway to unlock tomorrow" hint).Deploy order
tools/migrations/26-05-31--add_max_position_to_daily_audio_lesson.sqlVerification
Logic checked in isolation (rewind-after-80% → engaged ✅; genuine 33% → not engaged ✅; completed → engaged ✅; legacy rows → correct ✅). Not yet run against pytest or a real DB — reviewer should run the audio-lesson tests + apply the migration on staging.
🤖 Generated with Claude Code