Uh oh!
There was an error while loading. Please reload this page.
feat(quiz): mastery-model seam, honest delivered counts, wire validation, concurrency tests (#543) - #551
Conversation
…ion, concurrency tests (#543) Workstream E of the pre-revamp quiz repair batch (epic #537): E1 — the mastery model is a named seam: services/quiz_config.py holds MASTERY_DELTA_PER_CORRECT/_PER_WRONG plus mastery_after(), with the pedagogy written down. THE NUMBERS DO NOT CHANGE — the #393 journey's +0.09 is byte-identical and pinned by a new test. The options the revamp gets to choose from (length normalization, difficulty weighting, diminishing returns) are written up in docs/quiz-mastery-model.md, including the constraint that any change updates the journey in the same commit. E2 — generation stops silently short-changing quizzes: the response reports requested_count and delivered_count, and losing more than a third of the requested questions to drift triggers ONE bounded top-up run (a retry loop against a drifting model burns tokens without converging). A failed top-up serves what we have; all-dropped still 502s. E3 — wire-format validation at the route boundary: at least two options, no duplicate option text (a student can otherwise pick "the same" answer and be wrong), exactly one correct option, and no duplicate question stems within one attempt. E4 — concurrency tests: double-answer on one index (the UNIQUE arbitrates; the loser re-reads instead of 500ing) and generate-while-generating for one concept (distinct attempt rows). The double-submit claim was already pinned by #464's tests. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This pull request has been ignored for the connected project Preview Branches by Supabase. |
Warning Review limit reached
Next review available in:11 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 (9)
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 |
Deploying with |
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs | frontend-staging | a961f14 | Commit Preview URL Branch Preview URL | Aug 13 2026, 05:48 AM |
…op over-rejecting, surface short quizzes The review found the E2 top-up miskeyed at its core, verified by execution. All ten findings addressed: - The trigger keyed on requested-minus-delivered, conflating "we rejected some" with "the agent returned fewer". The Quiz schema lets a run return any count and the E2E seam always returns 3 against a UI default of 5, so the top-up fired a second full generation on EVERY quiz journey — double tokens and ~5s latency for zero extra questions. It now counts questions actually DROPPED and gates on that. - The top-up prompt said "different from the ones already asked" without saying what they were, so a deterministic model re-emitted the same stems and the dedupe discarded the whole retry. It now lists them. - `wire_questions and ...` made total drift the ONE case that never retried — backwards, since that's the case a retry most obviously clears. Total drift now retries once, then 502s (the old assert_called_once in test_quiz_routes pinned the wrong behaviour and is updated with the reasoning). - The retry reused ORCHESTRATOR_LIMITS, handing it a fresh full budget and doubling the per-request cost backstop. It gets its own smaller TOPUP_LIMITS. - The recovery path logged a traceback on a request that deliberately succeeds — the exact pattern that reds the logscan oracle. Now a warning with the exception type/message. - The duplicate-option check casefolded while grading matches case-SENSITIVELY, so questions whose distractors differ only by case (`list` vs `List` — a real question) were dropped. It now compares the way grading does. - requested_count/delivered_count had no consumer: QuizPanel now warns "We could only build N of M questions" instead of quietly serving a short quiz. - The double-answer test never reached the race path (its fake short-circuited on the pre-read); it now models the real interleaving and asserts the loser actually attempted an insert. Left as-is with reasoning: two of _validate_wire_question's checks are unreachable from today's only caller (the agent schema pins 4 options and the caller builds exactly one correct flag) — they are cheap defence-in-depth for the #537 revamp's new call sites, and the unit tests exercise them directly. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Uh oh!
There was an error while loading. Please reload this page.
Closes#543. Workstream E of the pre-revamp quiz repair batch (epic #537).
What
E1 — the mastery model is a seam, not a magic number.
MASTERY_DELTA_PER_CORRECT/_PER_WRONGandmastery_after()live inservices/quiz_config.pywith the pedagogy written down. The numbers do not change: the#393journey's+0.09is byte-identical and now pinned by a unit test as well.The trade-offs the revamp gets to decide are written up in
docs/quiz-mastery-model.md: length dominates (a 10-question quiz moves mastery 3.3× a 3-question one, purely for having more items), difficulty is free (the agent varies difficulty by mastery and scoring discards that signal), and adaptive mode is unpriced. Options A–E are laid out with the constraint that any change must update the#393journey in the same commit.E2 — generation stops silently short-changing quizzes. The response reports
requested_countanddelivered_count. Losing more than a third of the requested questions to drift triggers one bounded top-up run (a retry loop against a drifting model burns tokens without converging; a slightly short quiz beats a slow one). A failed top-up serves what we have rather than losing it; all-dropped still 502s.E3 — wire-format validation at the route boundary. At least two options, no duplicate option text (a student can otherwise pick "the same" answer and be wrong), exactly one correct option (zero is the #129 free point, two means the grader's first match wins silently), and no duplicate question stems within one attempt.
E4 — concurrency tests. Double-answer on one index (the UNIQUE arbitrates; the loser re-reads instead of 500ing) and generate-while-generating for one concept (distinct attempt rows). Double-submit was already pinned by #464's tests.
Note on the rebase
Rebasing onto the merged #550 surfaced a real shadowing bug: D's post-
apply_graph_updateblock assigned a localmastery_after, which shadows E's importedmastery_after()model function —UnboundLocalErroron every submit. Caught by 24 existing tests, fixed here (the value lives inmastery_score_after).Verification
ruff checkclean. No migration in this workstream.🤖 Generated with Claude Code