[FIX] completion_rate 100% 초과 방지 및 진행률 계산 개선#111
Conversation
There was a problem hiding this comment.
Pull request overview
This PR addresses a bug where completion_rate could exceed 100% by introducing a new database field unique_watched_seconds to track the unique (non-repeated) watch time for progress calculations, while preserving watched_seconds for analytics purposes. The PR adds proper capping logic to ensure completion rates never exceed 100%.
Key Changes:
- Added
unique_watched_secondsfield to theProgressmodel with an Alembic migration - Updated progress calculation logic to use
unique_watched_secondsinstead ofwatched_seconds - Applied
min(completion_rate, 100.0)capping throughout the codebase
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 7 comments.
| File | Description |
|---|---|
app/models/progress.py |
Added unique_watched_seconds field to track unique watch time for progress calculations |
app/services/enrollment_service.py |
Updated all progress calculation functions to use unique_watched_seconds and apply 100% capping |
alembic/versions/a3a28d52a24f_add_unique_watched_seconds_to_progress.py |
Database migration to add the new field with default values |
Critical Issues Found:
- Logic Bug: The
unique_watched_secondscalculation (= last_position) will cause the value to decrease when users rewind videos, which defeats its purpose. It should usemax(existing_value, last_position)to only track forward progress. - Migration Data Issue: Existing records are migrated with
unique_watched_seconds = watched_seconds, which may still allow >100% completion rates until users update their progress. - Documentation Mismatch: Comments suggest a calculation is happening, but the implementation is just a direct assignment that doesn't properly handle the "unique" aspect.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # unique_watched_seconds 계산 (진행률 계산용) | ||
| # last_position을 기준으로 유니크 시청 시간 계산 | ||
| unique_watched_seconds = last_position |
There was a problem hiding this comment.
Critical Logic Bug: The unique_watched_seconds calculation is incorrect. Setting it equal to last_position means that if a user watches a video from the beginning multiple times, the unique watched seconds won't update properly.
For example:
- User watches 0-100 seconds → last_position=100, unique_watched_seconds=100 ✓
- User rewinds and watches 0-150 seconds → last_position=150, unique_watched_seconds=150 ✓
- User rewinds to 50 and watches to 120 → last_position=120, unique_watched_seconds=120 ✗ (should still be 150)
The current implementation only tracks the furthest position reached (last_position), which already serves that purpose. unique_watched_seconds should be calculated differently (e.g., tracking unique segments watched) or should be set to max(unique_watched_seconds, last_position) when updating existing records to preserve the maximum progress.
…ogress.py Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…on tracking Co-authored-by: b1351b1 <223806567+b1351b1@users.noreply.github.com>
Clarify unique_watched_seconds tracks maximum position, not unique watch time
…ched_seconds Co-authored-by: b1351b1 <223806567+b1351b1@users.noreply.github.com>
[WIP] Address feedback on completion rate improvements
|
@copilot open a new pull request to apply changes based on the comments in this thread |
[WIP] Fix completion rate exceeding 100% and improve calculation
개요
변경사항
1. DB 스키마 변경
Progress모델에unique_watched_seconds필드 추가watched_seconds: 총 시청 시간 (반복 재생 포함 누적) - 분석용unique_watched_seconds: 유니크 시청 시간 - 진행률 계산용2. 진행률 계산 로직 개선
completion_rate계산 시unique_watched_seconds사용completion_rate를min(completion_rate, 100.0)으로 100% 이하로 제한last_position을duration_seconds이하로 제한3. 수정된 파일
app/models/progress.py: unique_watched_seconds 필드 추가app/services/enrollment_service.py: 진행률 계산 로직 전면 개선alembic/versions/a3a28d52a24f_add_unique_watched_seconds_to_progress.py:마이그레이션 파일
4. 기대 효과
활용도 향상
관련 이슈
**Closes #105