Uh oh!
There was an error while loading. Please reload this page.
feat(quiz): attempt lifecycle — mastery snapshot, derived status + TTL sweep, resume, paginated history (#542) - #550
Merged
Conversation
…L sweep, resume, paginated history (#542) Workstream D of the pre-revamp quiz repair batch (epic #537): D1 — submit persists mastery_before/mastery_after on the attempt row (migration 20260813013547; plaintext analytics scalars per #521), so a replayed or audited submit can reconstruct what the student saw. D2 — status is DERIVED, never stored: completed_at → completed, abandoned_at → abandoned, else in_progress; an in-progress row past QUIZ_ATTEMPT_ABANDON_TTL_HOURS (24h, documented) reads as abandoned even before the lazy per-user sweep stamps abandoned_at on the read paths (no scheduler needed; conditional-update filters arbitrate). GET /api/quiz/attempts/{id} returns resume state: questions WITHOUT the answer key plus the responses recorded through /answer. D3 — quizzes_completed counts completed attempts only; generate writes the attempt row before the student answers anything, so the unfiltered count let "generate and close the tab" advance quizzes_10. Blast radius (measured on staging 2026-08-12): 1 user, 2 attempts, 0 completed, badge never granted — nobody loses anything; prod recheck noted in the PR. No revocation in any case. D4 — GET /api/quiz/attempts: paginated history (concept, course, score, total, difficulty, mastery delta, dates) — the plaintext scalars kept for exactly this purpose in #521/#527 finally have a reader. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…eep (#542) The mastery snapshot and abandonment sweep are pure DB behaviour: a mocked table() accepts columns the migration never added (#265 drift class), and the conditional-update filters that decide WHICH attempts get swept only mean something against Postgres. Asserts the columns round-trip and that the sweep touches the stale in-progress attempt while leaving a fresh one and a completed one alone. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Deploying with |
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs | frontend-staging | 24a7203 | Commit Preview URL Branch Preview URL | Aug 13 2026, 04:00 AM |
This pull request has been ignored for the connected project Preview Branches by Supabase. |
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (8)
📝 WalkthroughWalkthroughChangesQuiz attempt lifecycle
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant QuizRoutes
participant GraphUpdate
participant SupabaseQuizAttempts
Client->>QuizRoutes: Submit quiz attempt
QuizRoutes->>GraphUpdate: Apply mastery update
GraphUpdate-->>QuizRoutes: Return mastery values
QuizRoutes->>SupabaseQuizAttempts: Persist completion and snapshots
SupabaseQuizAttempts-->>QuizRoutes: Return completed attempt
QuizRoutes-->>Client: Return submission result
Possibly related PRs
Suggested reviewers: ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…bandoned bite, harden history (#542) Review findings (xhigh, 15 confirmed). The serious ones: - The resume endpoint leaked the answer twice over: _strip_answer_key dropped only per-option `correct` and shipped `explanation` (which names the answer in prose), and being a DENYLIST it passed unknown stored shapes straight through — the rich seed's legacy row exposes its answer under `a` while getting an empty options list. It is an ALLOWLIST now (id/question/concept_tested/difficulty + label/text), and an unrecognised stored shape 409s QUIZ_ATTEMPT_NOT_RESUMABLE rather than being projected at all. - "Abandoned" was cosmetic: resume served the full question set and both /answer and /submit accepted swept attempts, paying out mastery, XP and achievements. Both write paths now 409 QUIZ_ATTEMPT_ABANDONED, and resume returns no questions with resumable:false. - quizzes_completed counted claimed-but-never-graded attempts, because completed_at is stamped by the atomic claim BEFORE grading. It now also requires a persisted score. - The TTL keyed on created_at alone, so an attempt being actively answered was swept. Status and sweep both consider the newest recorded answer; the active attempt is exempted from the sweep. - The stored mastery snapshot was submit's local prediction; it now records what apply_graph_update actually wrote (it clamps and resolves by concept name), so history can't show progression the graph refused. - The sweep is a write on a GET: it now sends Prefer: return=minimal (new db/connection.py option) instead of dragging every swept row — encrypted blobs included — back on each history page load. - history: offset clamped at the top (an unbounded value 500s as bigint-out-of-range) and ordered created_at.desc,id.desc so rows sharing a timestamp can't repeat or vanish across pages. - _attempt_status parsed naive timestamps fine and then raised TypeError comparing them to an aware cutoff, past the ValueError guard; a shared _parse_ts assumes UTC for naive values. - Migration DDL is idempotent (IF NOT EXISTS) per the repo rule, plus a partial index for the sweep/history predicate. Test gaps the review found: the history "no question payloads" assertion was vacuous (it now asserts the SELECTED COLUMNS), and neither new GET had ownership coverage — the hermetic lane structurally cannot provide it (require_self is stubbed there), so the IDOR negatives live in the integration lane with real sessions. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Uh oh!
There was an error while loading. Please reload this page.
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 freeto 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.
Closes#542. Workstream D of the pre-revamp quiz repair batch (epic #537). The attempt lifecycle leaked orphans and leaked the answer key; the plaintext scalars kept for analytics in #521/#527 had no reader at all.
What
D1 — mastery snapshot persisted.
mastery_before/mastery_afterland on the attempt row at submit (migration20260813013547, plaintext scalars per the #521 rationale). Without them a replayed or audited submit can't reconstruct what the student saw — the reason the #129 replay guard had to 409 instead of replaying the original 200.D2 — explicit status, derived not stored.
completed_at→completed,abandoned_at→abandoned, elsein_progress; an in-progress row pastQUIZ_ATTEMPT_ABANDON_TTL_HOURS(24h, documented: a quiz is one sitting, but generous enough that stepping away for hours still resumes) reads as abandoned even before it's stamped. A lazy per-user sweep on the read paths stampsabandoned_atvia conditional-update filters — same idiom as the submit claim, no scheduler.GET /api/quiz/attempts/{id}returns resume state: questions without the answer key plus the responses recorded through/answer.D3 — achievements count completed attempts only.
generatewrites the attempt row before the student answers anything, so the unfiltered count let "generate and close the tab" advancequizzes_10.Blast radius (measured on staging, 2026-08-12): 1 user, 2 attempts, 0 completed,
quizzes_10(threshold 10) never granted. Nobody loses a badge; one user's progress number drops 2 → 0. The achievement system is idempotent on(user_id, achievement_id)and grants are never revoked by this change — a recount can only withhold a badge not yet earned, and any already-granted badge stays. Prod recheck before merge is noted below.D4 —
GET /api/quiz/attempts. Paginated history for the signed-in user: concept, course, score, total, difficulty, mastery delta, dates. No question payloads, so no keys.Wire-contract notes
All three endpoints are additive; no existing response shape changes.
submitgains two persisted columns (response unchanged). No client code depends on the new routes yet — they exist for the #537 flow.Verification
ruff checkclean. Migration applied to staging ahead of merge.Prod check before promoting: re-run the D3 blast-radius query against production (
SELECT count(*) FILTER (WHERE completed_at IS NULL) FROM quiz_attempts+ grantedquizzes_completedbadges). Staging is clean; prod was not re-queried after this branch.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes