feat(gamification): streaks + stats from chat and flashcard use by tpaulshippy · Pull Request #70 · tpaulshippy/bots

tpaulshippy · 2026-09-09T06:18:35Z

Follow-up stack: #49 -> #69 -> this

Base: feature/study-due-reminders (stacked; only the commits below are new).

What changed

Backend: bots/services/stats.py — UTC-day streaks over user chat messages + flashcard reviews (quiet-today keeps yesterday's streak; gaps break current but longest remembers), totals, 7-day buckets. Buckets use explicit UTC ranges after evidence caught __date truncation disagreeing with UTC near midnight (TIME_ZONE). New GET /api/stats.json?profileId= — kid-visible (unlike parent-only activity endpoints); teen sessions locked to their claimed profile; foreign profiles 404
Frontend: dedicated Stats screen (/stats, drawer entry, profile switcher in header) — 🔥 streak, today badge (Chatted / Studied / both), review/chat/message totals with best-streak, 7-day activity bars; refetched on focus/profile switch. Flashcards tab is decks-only again.
Tests: 16 backend (streak math incl. gaps, system/assistant-message exclusion, midnight regression, endpoint auth/scoping incl. teen-delegated claim lock + missing/deleted-claim rejection) + stats API module + Stats screen component tests (streak/totals/badge, start prompt, error state, refetch on switch, stale-response guard, unauthorized redirect) + drawer coverage

Demo

Open Stats from the drawer → streak card (fed by both chats and reviews)
Study a card → Stats updates on next focus (badge flips, totals bump)

Evidence

pr70-stats-walkthrough.mp4

- Backend: stats service (UTC-day streaks over user messages + flashcard reviews, totals, 7-day buckets with explicit UTC ranges so TIME_ZONE truncation can't misplace activity near midnight) + GET /api/stats.json?profileId= (kid-visible; teen sessions locked to their claimed profile; foreign profiles 404) - Frontend: streak card on the flashcards home (flame, today badge, review/chat/message totals, 7-day bars), refetched with decks - Tests: 12 backend (streak math, gaps, system-message exclusion, endpoint auth/scoping) + stats API and stats-card component tests

…tion)

tpaulshippy · 2026-09-13T05:10:05Z

The streaks and stats should be on their own screen, not the flashcards screen.

Owner feedback: the streak card lived on the flashcards home. It now has a dedicated /stats route (drawer entry, teen-visible like the kid-visible stats endpoint) and flashcards.tsx is back to decks only. Tests move from flashcardsStats to stats + drawer coverage.

Copilot

🟡 Changes recommended

Unresolved moderate correctness, error-handling, authorization-coverage, and activity-count issues remain, along with contract and documentation nits.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Adds profile-scoped gamification statistics combining chat and flashcard activity, with UTC streaks, totals, and a standalone Stats screen.

Changes:

Adds backend stats calculations and authenticated API access.
Adds Stats navigation, UI, API client, and refresh behavior.
Adds backend and frontend test coverage.

File summaries

File	Summary
`front/components/ui/IconSymbol.tsx`	Adds the stats icon mapping.
`front/components/NavigationDrawer.tsx`	Adds Stats navigation. Nit (2 votes): Align the Flashcards demo description with the separate `/stats` screen.
`front/components/__tests__/NavigationDrawer-test.tsx`	Tests Stats navigation visibility.
`front/app/stats.tsx`	Implements the Stats screen. Moderate (3 votes): Handle unauthorized errors and prevent stale profile-switch responses from overwriting current stats.
`front/app/_layout.tsx`	Registers the Stats route and header.
`front/app/__tests__/stats-test.tsx`	Tests Stats rendering and profile switching.
`front/app/__tests__/flashcards-test.tsx`	Verifies Stats is separate from Flashcards.
`front/api/stats.ts`	Defines the stats API client. Moderate (1 vote): Do not mask request failures as an empty stats state.
`front/__tests__/api/stats.test.ts`	Tests stats API parsing.
`back/server/urls.py`	Registers the stats endpoint. Nit (3 votes): Regenerate the API schema and add mock-handler coverage.
`back/bots/viewsets/stats_viewset.py`	Handles authentication and profile scoping. Moderate (3 votes): Add delegated-session authorization tests.
`back/bots/tests/test_stats.py`	Tests stats calculations and endpoint behavior.
`back/bots/services/stats.py`	Computes streaks, totals, and activity buckets. Moderate (2 votes): Keep totals and buckets consistent with user-only activity.

Review details

Suppressed comments (1)

front/api/stats.ts:29

request resolves non-2xx and network failures to this null fallback, but the Stats screen renders null as “No stats yet.” A successful empty profile already returns a non-null object with a zero streak, so an outage is misreported as a new user and the failure is hidden. Return a raw response/throw or add an explicit error state so only successful zero-data responses use the empty UI.

  request<ProfileStats | null>(
    `/stats.json?profileId=${profileId}`,
    { method: "GET" },
    null

Files reviewed: 13/17 changed files
Comments generated: 6
Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

+        delegated_profile = delegated_profile_from_auth(request.auth, request.user)
+        if delegated_profile is not None:
+            return delegated_profile
+        if is_teen_delegated(request.auth):
+            return None


 router.register(r'decks', DeckViewSet)
 router.register(r'activity/chats', ActivityChatViewSet, basename='activity-chats')
 router.register(r'activity/summary', ActivitySummaryViewSet, basename='activity-summary')
+router.register(r'stats', StatsViewSet, basename='stats')


  const menuItems: MenuItem[] = [
    { label: "Chats", icon: "bubble.left.fill", path: "/chatHistory" },
    { label: "Flashcards", icon: "square.grid.2x2.fill", path: "/flashcards" },
+    { label: "Stats", icon: "chart.bar.fill", path: "/stats" },


- stats are user-message-only: assistant greetings no longer inflate totals/buckets/chatted_today out of sync with the streak - stats endpoint: teen-delegated claim-lock, missing-claim, and soft-deleted-claim tests - Stats screen: stale profile-switch responses ignored (generation guard), UnauthorizedError routes to login via handleUnauthorized, failures show an error state instead of the empty state - contract checklist: /api/stats/ path in schema.yaml + MSW handler (full regen deferred: it also pulls in unrelated stacked-PR drift)

tpaulshippy · 2026-09-13T15:25:46Z

Owner feedback addressed: streaks/stats moved off the flashcards screen onto a dedicated /stats screen (drawer entry, header profile switcher, teen-visible). Flashcards tab is decks-only again; PR description/demo updated to match.

Copilot round-2 findings also addressed in f82b310:

totals/buckets/chatted_today are user-message-only now (assistant greetings excluded, consistent with the streak)
teen-delegated endpoint tests: sibling-profileId lock, missing claim, soft-deleted claim
Stats screen: stale-response generation guard, UnauthorizedError -> login redirect, failure error-state vs empty-state
contract checklist: /api/stats/ path added to schema.yaml + MSW handler (full regen would also pull in unrelated stacked-PR activity drift, so left for the base PRs)

Copilot

🟡 Changes recommended

Unresolved moderate backend, API-contract, and profile-refresh issues remain.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (2)

front/app/stats.tsx:159

When today is quiet but yesterday was active, the backend deliberately keeps current_streak nonzero. This branch still says "start one", contradicting the streak card and misleading users who should be told to keep the existing streak going.

    : stats.chatted_today
      ? "Chatted today ✓"
      : "Chat or study today to start one!";

front/app/stats.tsx:49

On a profile switch, stats is left populated while the new request is in flight and loading remains false, so the screen can show the previous child's metrics under the new profile header until the response arrives. Clear or associate the displayed stats with the requested profile (and handle the loading state) when starting a profile-change refresh.

  const refresh = useCallback(async () => {
    const requestId = (requestRef.current += 1);
    const isCurrent = () => requestId === requestRef.current;
    setRefreshing(true);
    setLoadError(false);

Files reviewed: 15/19 changed files
Comments generated: 4
Review effort level: Lite

- stats buckets aggregated in 2 queries instead of 14 per request - total_chats counts chats with at least one user message (welcome chat no longer inflates it) - real API contract: Stats/StatsDay serializers + extend_schema on the list action (profileId param + object response), schema.yaml updated with the path and components, MSW stats handler added - Stats screen: per-day accessibility labels, keep-the-streak copy when today is quiet but the streak is alive, previous kid's stats cleared the moment the profile switches

tpaulshippy · 2026-09-13T15:36:22Z

Round-3 Copilot findings addressed in bd26ff1:

14 bucket COUNTs collapsed to 2 aggregated queries per request
total_chats counts only chats with a user message (welcome chat excluded)
real contract: Stats/StatsDay serializers + extend_schema (profileId param + object response); schema.yaml path/components updated, MSW handler added
Stats screen: per-day accessibility labels with weekday + counts, keep-the-streak copy for quiet-today streak holders, previous kid's stats cleared on profile switch

Copilot

🟡 Changes recommended

The Stats screen omits the best-streak value when current and longest streaks are equal.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (1)

front/components/NavigationDrawer.tsx:59

Adding Stats here makes the session-mode comment above inaccurate: teen-delegated sessions now get Chats, Flashcards, and Stats. Please update that comment so it no longer says these sessions only get Chats + Flashcards.

    { label: "Stats", icon: "chart.bar.fill", path: "/stats" },

Files reviewed: 17/21 changed files
Comments generated: 1
Review effort level: Lite

- always show best streak when longest > 0 (was hidden when the current run tied the record) + equality test - drawer comment: teen sessions get Chats + Flashcards + Stats

tpaulshippy · 2026-09-13T15:46:21Z

Round-4 Copilot findings addressed in 430e8b4: best-streak now renders whenever longest > 0 (equality case covered by test); drawer comment updated for the teen-visible Stats entry.

Copilot

🟡 Changes recommended

Two critical issues block backend functionality or CI, and one moderate accessibility issue remains.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (1)

front/app/stats.tsx:333

The subtext here is rendered in fixed #888; on the light theme's #eee background that is only about 3:1 contrast, below the normal-text accessibility threshold, and it also bypasses ThemedText's theme color. Remove the hard-coded color (or apply a theme-aware color such as iconColor) so the error/retry prompt remains readable.

  emptySubtext: {
    fontSize: 14,
    color: "#888",
    marginTop: 8,

Files reviewed: 17/21 changed files
Comments generated: 2
Review effort level: Lite

tpaulshippy · 2026-09-13T15:55:21Z

Round-5 Copilot findings reviewed — both inline items are false positives, with evidence in the thread replies, so no code change this round: (1) timezone.datetime(...) works (django.utils.timezone re-exports the datetime class; REPL + 16 passing tests prove it), (2) Deck is used by the deck fixture and ruff passes. The suppressed #888 note matches the pre-existing flashcards empty-state style app-wide; changing one screen alone would create inconsistency, so leaving it for a themed-text pass across the app.

Copilot

🔵 Needs a closer look

The Stats API test should assert the exact profileId query parameter to protect profile scoping.

Review details

Suppressed comments (1)

front/api/stats.ts:27

The new request's profile scoping is not asserted here: the mock matches any /stats.json URL, so a regression that drops or changes ?profileId=${profileId} would still pass this test while the backend returns the wrong result or a 400. Assert the exact URL (as front/__tests__/api/activity.test.ts does for its profile filter) so profile switching is covered at the API boundary.

    `/stats.json?profileId=${profileId}`,

Files reviewed: 17/21 changed files
Comments generated: 0 new
Review effort level: Lite

tpaulshippy · 2026-09-13T16:02:30Z

Round-6 follow-up (suppressed scoping note) addressed in 22346e4: stats API test now asserts the exact /stats.json?profileId=kid-1 URL, mirroring the activity filter test.

Copilot

🟡 Changes recommended

Ruff will fail the backend lint job because the new datetime imports are unsorted.

Get a fresh assessment by requesting another Copilot review.

Review details

Files reviewed: 17/21 changed files
Comments generated: 1
Review effort level: Lite

Copilot

🟢 Approval recommended

No unresolved issues blocking approval were identified.

Review details

Files reviewed: 17/21 changed files
Comments generated: 0 new
Review effort level: Lite

tpaulshippy added 2 commits September 9, 2026 06:17

chore(evidence): remove PR70 attachments from repo (now on PR descrip…

e19302b

…tion)

tpaulshippy requested a lite review from Copilot September 13, 2026 15:10

Copilot started reviewing on behalf of tpaulshippy September 13, 2026 15:11 View session