Uh oh!
There was an error while loading. Please reload this page.
feat(quiz): encrypt quiz performance JSON at rest (#521) - #527
Merged
Conversation
Warning Review limit reached
Next review available in:55 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (14)
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 |
This pull request has been ignored for the connected project Preview Branches by Supabase. |
Deploying with |
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs | frontend-staging | 1bc62c2 | Commit Preview URL Branch Preview URL | Aug 06 2026, 01:16 AM |
AndresL230
commented
Aug 6, 2026
CollaboratorAuthor
Code reviewFound 1 issue:
Sapling/backend/services/quiz_context_service.py Lines 12 to 15 in 8ab0866 🤖 Generated with Claude Code - If this code review was useful, please react with 👍. Otherwise, react with 👎. |
AndresL230force-pushed
the
feat/522-c-521-quiz
branch
from
August 6, 2026 01:12
8ab0866 to
1bc62c2CompareAndresL230force-pushed
the
feat/522-b-520-feedback
branch
from
August 6, 2026 01:21
3052b3a to
1e36394CompareAndresL230force-pushed
the
feat/522-c-521-quiz
branch
from
August 6, 2026 01:22
1bc62c2 to
b09c0e8CompareCo-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
#521) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…rypt; boundary tests (#521) get_quiz_context ran unguarded post-commit in submit_quiz — a corrupt context_json 500ed a request whose mastery/score writes already landed, and every retry then hit the completed_at 409 with no recovery path. Wrap the decrypt in try/except (mirrors documents.py's degrade convention) and log + return None instead of raising. Also reorders submit_quiz so require_self(user_id, request) runs before decrypt_json_column(questions_json) — auth before decrypt work. Adds tests/test_quiz_context_service.py (FakeTable pattern) covering the ciphertext upsert (on_conflict="user_id,concept_node_id" asserted), ciphertext + legacy-plaintext reads, and the corrupt-string guard (verified to fail via stash/run/pop when the guard is reverted). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
AndresL230force-pushed
the
feat/522-c-521-quiz
branch
from
August 6, 2026 01:25
b09c0e8 to
3e78b3eCompareUh oh!
There was an error while loading. Please reload this page.
This was referenced Aug 6, 2026
AndresL230 added a commit
that referenced
this pull request
Aug 13, 2026
…L sweep, resume, paginated history (#542) (#550) * feat(quiz): attempt lifecycle — mastery snapshot, derived status + TTL 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> * test(quiz): real-DB coverage for the attempt lifecycle columns and sweep (#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> * fix(quiz): address #550 review — close the resume answer leak, make abandoned 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> --------- Co-authored-by: Claude Fable 5 <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 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#521.
quiz_attempts.questions_json/answers_json+quiz_context.context_jsonvia encrypt_json, matching sessions.summary_jsonStacked on #520's PR.
🤖 Generated with Claude Code