Uh oh!
There was an error while loading. Please reload this page.
feat(quiz): adaptive difficulty, /api/quiz/config, stable error envelope (#540) - #547
Conversation
…ope (#540) Workstream A of the pre-revamp quiz repair batch (epic #537): A1 — 'adaptive' is a real difficulty. The route accepts it, the agent picks the per-question mix (ADAPTIVE MODE prompt rules), the attempt row records the request (CHECK extended by migration), and the response reports requested_difficulty + resolved_difficulty (mode, ties break harder) so the client can say what was actually chosen. A2 — GET /api/quiz/config is the single source of truth for selector values; the Pydantic model reads the same constants. Cap decision is measured, not guessed: scripts/bench_quiz_question_cap.py shows 15- and 20-question schemas are rejected outright by gemini-2.5-flash-lite ("too many states for serving"), so 10 is a hard ceiling. QuizPanel now builds its selects from the endpoint (static fallback mirrors it; the dead "15 questions" option is gone). A3 — every quiz-route 4xx/5xx returns {error: {code, message, detail?, request_id}} with codes in services/quiz_errors.py::QuizErrorCode; the legacy top-level detail key is kept so the current client still works. 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:19 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 (11)
📝 WalkthroughWalkthroughThe PR adds adaptive quiz difficulty, centralizes quiz configuration, updates question-count validation and persistence constraints, introduces structured quiz errors, and makes frontend selectors load backend-defined options with static fallbacks. ChangesAdaptive quiz configuration
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related issues
Possibly related PRs
Sequence Diagram(s)sequenceDiagram
participant QuizPanel
participant QuizRoute
participant QuizAgent
participant Database
QuizPanel->>QuizRoute: Request adaptive quiz generation
QuizRoute->>Database: Read mastery and recent attempts
QuizRoute->>QuizAgent: Generate questions with adaptive instructions
QuizAgent-->>QuizRoute: Return concrete question difficulties
QuizRoute->>Database: Store quiz attempt
QuizRoute-->>QuizPanel: Return requested and resolved difficulty
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 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 |
Deploying with |
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs | frontend-staging | 1bd3512 | Commit Preview URL Branch Preview URL | Aug 12 2026, 09:41 PM |
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/db/e2e_checks/quiz.py (1)
35-44: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winExercise the adaptive persistence path.
Line 42 still sends
"easy". This E2E flow does not verify that a successful adaptive request stores"adaptive"inquiz_attempts.difficulty. Send an adaptive request and, when generation succeeds, assert the created attempt row hasdifficulty == "adaptive"before submit.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/db/e2e_checks/quiz.py` around lines 35 - 44, Update the generate request in the E2E flow to use difficulty "adaptive" instead of "easy". After successful generation, query the created quiz_attempts row and assert its difficulty is "adaptive" before submitting the quiz.
🧹 Nitpick comments (1)
frontend/src/components/QuizPanel.tsx (1)
129-137: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winReconcile selected values with loaded quiz configuration.
If the server removes
"5"or"medium", the selectors render no selected option butstart()still sends those stale values. This defeats the server-defined selector contract.
frontend/src/components/QuizPanel.tsx#L129-L137: after accepting a non-empty config payload, use functional state setters to retain a selected value only when it remains allowed; otherwise select the first allowed value.frontend/src/components/QuizPanel.test.tsx#L45-L47: add a successful config mock that excludes"5"and"medium", then assert quiz generation uses the reconciled values.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/components/QuizPanel.tsx` around lines 129 - 137, The quiz configuration load must reconcile selected count and difficulty values with the server’s allowed options. In frontend/src/components/QuizPanel.tsx lines 129-137, after accepting a non-empty payload, use functional setters to preserve each current selection only when allowed, otherwise choose the first allowed value. In frontend/src/components/QuizPanel.test.tsx lines 45-47, add a successful config mock excluding "5" and "medium", then assert quiz generation uses the reconciled selections.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@backend/agents/quiz.py`:
- Around line 116-124: Update the fixed-difficulty rules immediately following
the adaptive-mode instructions in the quiz prompt so they apply only when the
user provides a concrete difficulty request. Prevent adaptive mode from
inheriting rules that shift from or honor a requested difficulty; preserve
adaptive’s mastery-based selection and medium baseline when no history exists.
In `@backend/db/migrations/20260812204809_quiz_attempts_adaptive_difficulty.sql`:
- Around line 9-14: Update the quiz_attempts_difficulty_check replacement in
this migration to add the constraint as NOT VALID, avoiding validation during
the migration transaction. Create a subsequent timestamped migration that
validates quiz_attempts_difficulty_check with VALIDATE CONSTRAINT.
In `@backend/services/quiz_errors.py`:
- Around line 44-51: Update the status-resolution logic using _STATUS_FALLBACK
so unlisted 4xx statuses, including 400, return QUIZ_VALIDATION_ERROR, while
unlisted 5xx statuses continue returning QUIZ_INTERNAL_ERROR. Preserve the
explicit mappings for listed statuses.
In `@backend/tests/test_quiz_preflight_a.py`:
- Around line 14-19: Update the tests in this module to remove the global
TestClient setup and route-internal patching, and use the shared Supabase and
Gemini fixtures provided by tests/conftest.py. Adjust affected test functions to
receive and use those fixtures while preserving their existing assertions and
behavior.
---
Outside diff comments:
In `@backend/db/e2e_checks/quiz.py`:
- Around line 35-44: Update the generate request in the E2E flow to use
difficulty "adaptive" instead of "easy". After successful generation, query the
created quiz_attempts row and assert its difficulty is "adaptive" before
submitting the quiz.
---
Nitpick comments:
In `@frontend/src/components/QuizPanel.tsx`:
- Around line 129-137: The quiz configuration load must reconcile selected count
and difficulty values with the server’s allowed options. In
frontend/src/components/QuizPanel.tsx lines 129-137, after accepting a non-empty
payload, use functional setters to preserve each current selection only when
allowed, otherwise choose the first allowed value. In
frontend/src/components/QuizPanel.test.tsx lines 45-47, add a successful config
mock excluding "5" and "medium", then assert quiz generation uses the reconciled
selections.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: ee118b9c-e231-4709-a0b2-ba45685d6e26
📒 Files selected for processing (13)
backend/agents/quiz.pybackend/db/e2e_checks/quiz.pybackend/db/migrations/20260812204809_quiz_attempts_adaptive_difficulty.sqlbackend/main.pybackend/models/__init__.pybackend/routes/quiz.pybackend/scripts/bench_quiz_question_cap.pybackend/services/quiz_config.pybackend/services/quiz_errors.pybackend/tests/test_quiz_preflight_a.pyfrontend/src/components/QuizPanel.test.tsxfrontend/src/components/QuizPanel.tsxfrontend/src/lib/api.ts
| "ADAPTIVE MODE (#540): when the user message says the quiz is in " | ||
| "adaptive mode, there is no user-requested difficulty — you choose " | ||
| "each question's difficulty yourself. Base the mix on mastery and " | ||
| "`recent_attempts.accuracy`: struggling/low-accuracy concepts get " | ||
| "easy-leaning questions, strong/high-accuracy ones get hard-leaning " | ||
| "questions; with no history at all, center the mix on medium. The " | ||
| "±1-step limits below do not apply in adaptive mode, but every " | ||
| "question still carries a concrete easy|medium|hard difficulty — " | ||
| "'adaptive' is never a per-question value.\n\n" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Scope the fixed-difficulty rules to non-adaptive requests.
Adaptive mode says there is no requested difficulty and defaults to medium without history. The following rules still instruct the agent to shift from, or honor, a requested difficulty. This gives adaptive generation an undefined baseline.
Limit the rules at lines 125-136 to concrete requests, or define an explicit adaptive baseline for each rule.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/agents/quiz.py` around lines 116 - 124, Update the fixed-difficulty
rules immediately following the adaptive-mode instructions in the quiz prompt so
they apply only when the user provides a concrete difficulty request. Prevent
adaptive mode from inheriting rules that shift from or honor a requested
difficulty; preserve adaptive’s mastery-based selection and medium baseline when
no history exists.
| ALTER TABLE quiz_attempts | ||
| DROP CONSTRAINT IF EXISTS quiz_attempts_difficulty_check; | ||
| ALTER TABLE quiz_attempts | ||
| ADD CONSTRAINT quiz_attempts_difficulty_check | ||
| CHECK (difficulty IN ('easy', 'medium', 'hard', 'adaptive')); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bashset -euo pipefail
migrator="$(fd -a '^migrate\.py$' backend/db | head -n1)"test -n "$migrator"
ast-grep outline "$migrator" --items all
rg -n -C 5 'BEGIN|COMMIT|transaction|execute|migrations'"$migrator"Repository: SaplingLearn/Sapling
Length of output: 7968
🏁 Script executed:
#!/bin/bashset -euo pipefail
migration="$(fd -a '20260812204809_quiz_attempts_adaptive_difficulty\.sql$' backend/db/migrations | head -n1)"test -n "$migration"
cat -n "$migration"printf'\nConstraint migration patterns:\n'
rg -n -C 4 'ADD CONSTRAINT|DROP CONSTRAINT|NOT VALID|VALIDATE CONSTRAINT' backend/db/migrationsRepository: SaplingLearn/Sapling
Length of output: 19373
🏁 Script executed:
#!/bin/bashset -euo pipefail
python3 - <<'PY'from pathlib import Pathimport rerunner = Path("backend/db/migrate.py").read_text()migration = Path("backend/db/migrations/20260812204809_quiz_attempts_adaptive_difficulty.sql").read_text()apply = re.search( r"def apply_migration\(.*?^\s*def run\(", runner, re.MULTILINE | re.DOTALL,)assert apply, "apply_migration block not found"block = apply.group(0)checks = { "executes migration SQL": "cur.execute(path.read_text" in block, "records ledger before commit": block.find("INSERT INTO schema_migrations") < block.find("conn.commit()"), "commits once per migration": block.count("conn.commit()") == 1, "current constraint is not NOT VALID": "NOT VALID" not in migration.upper(),}for name, result in checks.items(): print(f"{name}: {result}")assert all(checks.values())PYRepository: SaplingLearn/Sapling
Length of output: 297
🌐 Web query:
PostgreSQL documentation ALTER TABLE ADD CHECK NOT VALID lock level VALIDATE CONSTRAINT
💡 Result:
In PostgreSQL, the VALIDATE CONSTRAINT command acquires a SHARE UPDATE EXCLUSIVE lock on the table being altered [1][2]. This lock level is specifically designed to allow concurrent reads, updates, inserts, and deletes, as the validation process only needs to check pre-existing rows [1][2]. Because new rows inserted or updated after the constraint was initially added (using NOT VALID) are already being enforced by the constraint, they do not need to be scanned during the validation phase [1][2]. If the constraint being validated is a foreign key, an additional ROW SHARE lock is required on the referenced table [1][2]. When using ALTER TABLE ADD CONSTRAINT with the NOT VALID option, the command itself requires a SHARE ROW EXCLUSIVE lock (for check constraints) [3][4], which is less restrictive than the ACCESS EXCLUSIVE lock required for standard constraint addition without NOT VALID [1][2]. This allows the initial constraint definition to proceed without blocking concurrent data modifications [1][2].
Citations:
- 1: https://www.postgresql.org/docs/18/sql-altertable.html
- 2: https://www.postgresql.org/docs/19/sql-altertable.html
- 3: https://www.postgresql.org/message-id/CAFBsxsH4%3DTT7zv3R4d7-JKrLymG6Ow8GaiwBvVcVsQG8YwJf0g%40mail.gmail.com
- 4: https://www.postgresql.org/message-id/CANbhV-Ep3p0vnEfhDTro7Z2WfbOLab__tZnC%2BpYhT1qgoT7O%3DA%40mail.gmail.com
Avoid full-table blocking validation in this migration.
backend/db/migrate.py commits each migration after executing its SQL and ledger insert. The current ADD CONSTRAINT therefore holds an ACCESS EXCLUSIVE lock on quiz_attempts through the validation scan. Add the replacement constraint as NOT VALID, then validate it in a later timestamped migration with VALIDATE CONSTRAINT.
🧰 Tools
🪛 Squawk (2.61.0)
[warning] 13-14: By default new constraints require a table scan and block writes to the table while that scan occurs. Use NOT VALID with a later VALIDATE CONSTRAINT call.
(constraint-missing-not-valid)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/db/migrations/20260812204809_quiz_attempts_adaptive_difficulty.sql`
around lines 9 - 14, Update the quiz_attempts_difficulty_check replacement in
this migration to add the constraint as NOT VALID, avoiding validation during
the migration transaction. Create a subsequent timestamped migration that
validates quiz_attempts_difficulty_check with VALIDATE CONSTRAINT.
Source: Linters/SAST tools
| _STATUS_FALLBACK: dict[int, QuizErrorCode] = { | ||
| 401: QuizErrorCode.QUIZ_NOT_AUTHORIZED, | ||
| 403: QuizErrorCode.QUIZ_NOT_AUTHORIZED, | ||
| 404: QuizErrorCode.QUIZ_ATTEMPT_NOT_FOUND, | ||
| 409: QuizErrorCode.QUIZ_ATTEMPT_ALREADY_COMPLETED, | ||
| 422: QuizErrorCode.QUIZ_VALIDATION_ERROR, | ||
| 502: QuizErrorCode.QUIZ_GENERATION_FAILED, | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Map unlisted 4xx statuses to a client-error code.
A plain quiz-route HTTPException(status_code=400) resolves to QUIZ_INTERNAL_ERROR. This conflicts with the stable error-code contract because the response reports a client error as an internal failure.
Use QUIZ_VALIDATION_ERROR as the fallback for unlisted 4xx statuses. Keep QUIZ_INTERNAL_ERROR as the fallback for 5xx statuses.
Proposed fix
- resolved_code = code or _STATUS_FALLBACK.get(- status_code,- QuizErrorCode.QUIZ_INTERNAL_ERROR,- )+ resolved_code = code or _STATUS_FALLBACK.get(+ status_code,+ (+ QuizErrorCode.QUIZ_VALIDATION_ERROR+ if 400 <= status_code < 500+ else QuizErrorCode.QUIZ_INTERNAL_ERROR+ ),+ )Also applies to: 87-90
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/services/quiz_errors.py` around lines 44 - 51, Update the
status-resolution logic using _STATUS_FALLBACK so unlisted 4xx statuses,
including 400, return QUIZ_VALIDATION_ERROR, while unlisted 5xx statuses
continue returning QUIZ_INTERNAL_ERROR. Preserve the explicit mappings for
listed statuses.
| from fastapi.testclient import TestClient | ||
| from main import app | ||
| from agents.quiz import Quiz, QuizQuestion | ||
| client = TestClient(app) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Use the shared backend fixtures.
This module creates its own global TestClient and patches route internals. Use the shared Supabase and Gemini fixtures from tests/conftest.py instead. This keeps backend test setup consistent and prevents fixture behavior from being bypassed.
As per coding guidelines, backend/tests/**/*.py must “use the shared Supabase and Gemini fixtures from tests/conftest.py.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/tests/test_quiz_preflight_a.py` around lines 14 - 19, Update the
tests in this module to remove the global TestClient setup and route-internal
patching, and use the shared Supabase and Gemini fixtures provided by
tests/conftest.py. Adjust affected test functions to receive and use those
fixtures while preserving their existing assertions and behavior.
Source: Coding guidelines
… semantics, config-driven coverage Review findings (xhigh, 15 defects): - 422s on num_questions only get QUIZ_COUNT_OUT_OF_RANGE for actual bounds violations; type errors stay QUIZ_VALIDATION_ERROR. - _STATUS_FALLBACK narrowed to auth/validation; uncoded 404/405s get the new generic QUIZ_HTTP_ERROR instead of impersonating domain states, and a non-QuizErrorCode `code` attr can no longer crash the handler. - The envelope-vs-legacy branch now lives once in quiz_errors.error_content; main.py's three handlers all call it. - The system prompt defines stored difficulty='adaptive' history rows (judge by accuracy) so the quiz-history tool doesn't feed the stepping rules an undefined token. - The adaptive migration drops the old CHECK by introspection (name-robust) and documents migrate-before-deploy ordering. - bench_quiz_question_cap exits 1 when the baseline cap measured nothing. - QUIZ_QUESTION_TYPES uses the schema token 'multiple_choice', not 'mcq'. - _DIFFICULTY_RANK derived from CONCRETE_DIFFICULTIES. - QuizPanel: humanizeError in both catches (the envelope's sentence now actually reaches the student) and an "Adaptive · <resolved>" chip in the active phase; vitest covers the config-driven select path both ways; a promoted #540 journey pins config-mirroring selects + the adaptive wire round trip in a real browser. Skipped (documented): moving difficulty validation into a Pydantic Literal — the 400 + QUIZ_DIFFICULTY_INVALID contract is published in #540 and changing it to a 422 buys no client anything today; revisit in #537. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…urney Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Uh oh!
There was an error while loading. Please reload this page.
Closes#540. Workstream A of the pre-revamp quiz repair batch (epic #537) — the P0 that ships standalone.
What
A1 —
adaptiveis a real difficulty. The route accepts it (REQUESTED_DIFFICULTIES), the agent picks the per-question mix under new ADAPTIVE MODE prompt rules (no ±1 clamp; every question still emits a concrete easy|medium|hard), and the attempt row stores the requested value —quiz_attempts_difficulty_checkextended by migration20260812204809. The response now carriesrequested_difficulty+resolved_difficulty(mode of per-question difficulties, ties break harder) so the client can tell the student what was actually chosen. Both fields are additive; the old client ignores them.A2 — one source of truth for selector options.
GET /api/quiz/configservesnum_questions {min,max,options},difficulties,question_typesfromservices/quiz_config.py;GenerateQuizBodyreads the same constants (pinned by a sync test).QuizPanelbuilds its selects from the endpoint with a static fallback mirroring it — the dead "15 questions" option is gone.Cap decision (measured, not guessed):
scripts/bench_quiz_question_cap.py, 2026-08-12, gemini-2.5-flash-lite:10 is a hard ceiling for a single structured call on the quiz model tier, not a preference. Raising it = model-tier change or batched generation = #537 revamp scope.
A3 — stable error envelope. Every quiz-route 4xx/5xx returns
{ error: { code, message, detail?, request_id } }; codes live in one enum (services/quiz_errors.py::QuizErrorCode); scoped strictly to/api/quiz/*via the main.py handlers, so no other route changes shape.Wire-contract notes (for the current client)
requested_difficulty,resolved_difficulty).detailandrequest_idkeys are kept alongside the newerrorobject —frontend/src/lib/api.tsreadsdata?.detailand keeps working unchanged.QuizPanel.tsxselector lists are now config-driven; defaults (5 / medium) unchanged, so the test(e2e): journey — quiz answer → mastery update (UI + DB) #393 journey is untouched.Verification
tests/test_quiz_preflight_a.py, written first, watched fail).make e2e-up→ Playwright 42 passed (incl. test(e2e): journey — quiz answer → mastery update (UI + DB) #393 quiz journey + QuizPanel strands the user on a blank screen when a quiz generates zero questions #184 zero-question guard) →e2e_oraclesexit 0 →make e2e-down.ruff checkclean,tsc --noEmitclean, QuizPanel vitest 3/3.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes