Skip to content

feat(study): add review streak tracking and home indicator#172

Open
DevamPatel22 wants to merge 4 commits into
mainfrom
feat/streaks
Open

feat(study): add review streak tracking and home indicator#172
DevamPatel22 wants to merge 4 commits into
mainfrom
feat/streaks

Conversation

@DevamPatel22
Copy link
Copy Markdown
Collaborator

@DevamPatel22 DevamPatel22 commented Apr 6, 2026

Adds review streak tracking from completed study sessions and shows a live streak indicator on the home screen while keeping streak stats updated for current, longest, and last completed progress.

@DevamPatel22 DevamPatel22 requested a review from Pairadux as a code owner April 6, 2026 20:43
Copy link
Copy Markdown
Owner

@Pairadux Pairadux left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review

Looks good overall — nice work wiring up session summary persistence and the streak UI. Tests are solid with good edge case coverage, and the asOf parameter for testability is a nice touch.

Stale Streak Data

reviewStreakProvider is a plain FutureProvider that caches until explicitly invalidated. If the app stays open past midnight the streak won't refresh. Either switch to FutureProvider.autoDispose or invalidate on app lifecycle resume via WidgetsBindingObserver.didChangeAppLifecycleState.

getStreak Performance

getStreak() loads every distinct date into Dart and computes in memory. Fine for now, but the current streak could be a backwards SQL scan from today/yesterday and longest streak could use a window function. Worth a follow-up.

@DevamPatel22 DevamPatel22 self-assigned this Apr 19, 2026
Copy link
Copy Markdown
Owner

@Pairadux Pairadux left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both prior comments addressed thoroughly — autoDispose + day/resume refresh providers, and SQL-based streak computation with gap-and-islands. Tests cover all the right edges.

Left two nitpicks below as suggestions you can apply directly. The flame animation can stay as-is.

Comment on lines +205 to +212
SELECT COUNT(*) AS current_streak
FROM streak
WHERE EXISTS (
SELECT 1
FROM ${DatabaseConstants.tableReviewSessionSummary}
WHERE ${DatabaseConstants.colDate} = streak.day
AND ${DatabaseConstants.colTotalReviews} > 0
)
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor: the outer WHERE EXISTS here never filters anything. Every row in the streak recursive CTE is added because the prior day's row exists, and the base case is the anchor day, which is guaranteed to exist by the has_today/has_yesterday check above. Safe to drop:

Suggested change
SELECT COUNT(*) AS current_streak
FROM streak
WHERE EXISTS (
SELECT 1
FROM ${DatabaseConstants.tableReviewSessionSummary}
WHERE ${DatabaseConstants.colDate} = streak.day
AND ${DatabaseConstants.colTotalReviews} > 0
)
SELECT COUNT(*) AS current_streak
FROM streak

Comment thread lib/features/study/presentation/screens/review_stats_screen.dart
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants