fix(quiz): repair the adaptive context loop — restore UNIQUE, stop swallowing, consume the full digest (#529) by AndresL230 · Pull Request #548 · SaplingLearn/Sapling

AndresL230 · 2026-08-12T21:43:14Z

Closes#529. Workstream B of the pre-revamp quiz repair batch (epic #537) — the highest-value fix in the batch: the difference between "adaptive" being a label and being true.

Forensics (verified live)

0001 created quiz_context with UNIQUE (user_id, concept_node_id); 0025_study_integrity.sql:108-114 recreated the table without it (dropped 2026-06-23, 72443e96).
services/quiz_context_service.py:37 upserts with on_conflict="user_id,concept_node_id" → PostgREST 42P10 on every write since; routes/quiz.py swallowed it (except Exception: pass).
Staging measured 2026-08-12: 0 rows, 0 duplicate pairs — the very first write already failed, so nothing ever accumulated. pg_constraint confirmed only the PK + two FKs.

What

B1 — migration 20260812210033 restores the UNIQUE (named quiz_context_user_concept_key), dedup-guarded (keep-newest, RAISE NOTICE reports removals) and idempotent. Applying it is backward-compatible; run db.migrate before/with the deploy.

B2 — the upsert's on_conflict target now has a matching constraint; the pairing is pinned by test.

B3 — the post-submit background write is loud on failure: logger.exception with attempt id + request id, a quiz.context_write_failed analytics event (category="error", added to the pinned EVENT_TAXONOMY) so it lands in admin analytics like error.4xx, and a re-raise under config.IS_LOCAL so this regression class fails CI instead of going quiet for months. Deliberate carve-out: the E2E seam's UnregisteredHandlerError (quiz_context stays unregistered in function mode by design) is a single WARNING with no traceback — the logscan oracle treats tracebacks as findings, and that fail-fast is the seam working.

B4 — the read side now consumes the wholeQuizContext shape. _coerce_summary previously returned notes alone and silently dropped weak_areas / common_mistakes / questions_seen_summary; its list fallback looked for common_errors, a key QuizContext never writes. A tool-level round-trip test covers ciphertext row → decrypt → digest in QuizHistory.summary.

B5 — real-DB integration tests (tests/integration/test_quiz_context_repair_db.py, #397 seam: app writes, raw psycopg reads): UNIQUE present with exactly the upsert's columns, double-upsert keeps one row, raw context_json is ciphertext, app read round-trips. No MagicMock on this path — the hermetic suite's mocks are precisely why this bug survived ~7.5 weeks.

Verification

Hermetic suite: 1926 passed (7 new tests, written first, watched fail).
Full local cycle under the stack lock: Playwright 43 passed → oracles exit 0 → integration lane 42 passed / 0 skipped (counts read, not exit codes) → teardown.
ruff check clean.
Staging migration applied ahead of merge (constraint restore is backward-compatible).

🤖 Generated with Claude Code

…allowing, consume the full digest (#529) Workstream B of the pre-revamp quiz repair batch (epic #537): B1 — migration 20260812210033 restores UNIQUE (user_id, concept_node_id) on quiz_context (dropped by 0025's table recreate), dedup-guarded and idempotent. Staging/prod measured 0 rows + 0 dupes — the very first upsert already 42P10'd, so nothing ever accumulated. B2 — save_quiz_context's on_conflict target now has a matching constraint again; pinned by test. B3 — the post-submit background write is loud on failure: ERROR log with attempt id + request id, a quiz.context_write_failed analytics event (added to the pinned EVENT_TAXONOMY), and a re-raise under config.IS_LOCAL so this regression class fails CI. The E2E seam's UnregisteredHandlerError stays a single WARNING with no traceback — quiz_context is deliberately unregistered in function mode and the logscan oracle treats tracebacks as findings. B4 — the read side now consumes the WHOLE QuizContext shape: _coerce_summary previously returned `notes` alone and dropped weak_areas / common_mistakes / questions_seen_summary (its list fallback also looked for `common_errors`, a key QuizContext never writes). Tool-level round-trip test covers ciphertext row → decrypt → digest in QuizHistory.summary. B5 — real-DB integration tests (tests/integration/ test_quiz_context_repair_db.py): constraint present, double-upsert keeps one row, raw column is ciphertext, app read round-trips. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

cloudflare-workers-and-pages · 2026-08-12T21:43:21Z

Deploying with Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status	Name	Latest Commit	Preview URL	Updated (UTC)
✅ Deployment successful! View logs	frontend-staging	`bdc9552`	Commit Preview URL Branch Preview URL	Aug 13 2026, 01:32 AM

supabase · 2026-08-12T21:43:22Z

This pull request has been ignored for the connected project ybgqdonkoqftwrmweuyv because there are no changes detected in supabase directory. You can change this behaviour in Project Integrations Settings ↗︎.

Preview Branches by Supabase.
Learn more about Supabase Branching ↗︎.

coderabbitai · 2026-08-12T21:43:25Z

Warning

Review limit reached

@AndresL230, you've reached your PR review limit, so we couldn't start this review.

Next review available in:94 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 @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 28afc089-32e5-401b-8776-c582420f41df

📥 Commits

Reviewing files that changed from the base of the PR and between 6081ec5 and bdc9552.

📒 Files selected for processing (10)

backend/agents/tools/quiz_history.py
backend/db/migrations/20260812210033_restore_quiz_context_unique.sql
backend/routes/admin_analytics.py
backend/routes/quiz.py
backend/services/events_service.py
backend/services/quiz_context_service.py
backend/tests/integration/test_quiz_context_repair_db.py
backend/tests/test_admin_analytics_routes.py
backend/tests/test_event_capture_seams.py
backend/tests/test_quiz_context_repair.py

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

…_difficulty, stable upsert id, error-category admin feed Review findings (xhigh, 4 confirmed): - _coerce_summary guards list-shaped keys with isinstance(list) so legacy rows holding a string/dict there no longer explode into per-character bullets in the agent's prompt digest. - recommended_difficulty is surfaced in the digest ("Recommended next difficulty: …") — previously computed, encrypted, persisted, and never consumed anywhere. - save_quiz_context no longer sends a client-generated id: with merge-duplicates live again, the payload id rewrote the row's PRIMARY KEY on every refresh. Fresh inserts use the column's DB default; integration test now pins id stability across refreshes. - /api/admin/analytics/errors filters by category=error instead of the error.* name prefix, so quiz.context_write_failed (and #482's rag.* events) actually appear in the feed the B3 comments promised — non-HTTP rows null their payload fields, which ErrorEvent already models as Optional. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

…e events (#544) (#552) * feat(quiz): rate limit, daily spend guard, generation timeout, failure events (#544) Workstream F of the pre-revamp quiz repair batch (epic #537): F1 — generate is no longer an unbounded LLM call behind a button: a per-user sliding-window limit (8 per 5 minutes, sized for a human comparing difficulties) returns 429 QUIZ_RATE_LIMITED with Retry-After, and a daily per-user LLM spend ceiling ($2, read off the llm_usage ledger agents/usage.py already writes) returns 429 QUIZ_DAILY_LIMIT_REACHED before the model runs. Both guards sit AFTER the ownership check, so probing a stranger's concept can't consume their quota, and the spend check fails OPEN — a usage-table blip must not deny every student. F2 — the whole generation (agent run + tools + the E2 top-up) is bounded by QUIZ_GENERATION_TIMEOUT_SEC and maps to its own QUIZ_GENERATION_TIMEOUT code, so the client can say "that took too long" instead of the generic failure. F3 — quiz.generation_failed events (category=error, with a reason: timeout / agent_guardrail / agent_error) join the pinned taxonomy, so a 502 the student saw is a 502 an admin can count in the errors feed #548 widened. A throttled student is deliberately NOT an error event. F4 — deep-link scoping tests: a foreign concept 404s before the agent runs, and a student's OWN concept from a past semester still generates (scoping is by ownership, not active semester — pinned so a future "scope to active semester" change can't silently break revision). Also: the process-global rate-limit state now resets between tests via a conftest autouse fixture, same reasoning as the lru_cache reset — without it one test's burst throttles every later test hitting the same route. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(quiz): address #552 review — each guardrail was undercutting itself All three F guards failed at what they were added to do, each confirmed by execution in review: - The daily spend cap summed an UNPAGED llm_usage select. PostgREST caps a response at max_rows (1000) and answers 206 — a 2xx — so the sum plateaued and the ceiling could never trip for the runaway user it targets. It pages now (with an early exit once the cap is crossed), and db/connection.py::select gained the `offset` it needed. - Wrapping the whole generation in asyncio.wait_for raised CancelledError — a BaseException — straight past #543's serve-what-we-have handler, so a timed-out top-up threw away a valid partial quiz and returned 502. Each agent run is bounded individually now, so a top-up timeout is an ordinary TimeoutError the existing handler degrades from; a completed 3-question generation is served instead of discarded. - The rate-limit slot was claimed before generation and never refunded, so eight backend 502s locked a student out for five minutes with a message saying they'd generated too many quizzes — having received none. Every failure path now refunds the slot (services/request_limits.py::refund_rate_limit). Also from the review: - The timeout branch caught the builtin OSError-family TimeoutError as well, relabelling transport socket timeouts as wall-clock timeouts. It catches only asyncio.TimeoutError now. - The spend-cap comment claimed cross-feature enforcement it doesn't provide; it now says what's actually true (the spend measured is cross-feature, the ceiling is enforced on quiz generation only). - main.py forwards exc.headers as of this branch, which falsified the comments in routes/extract.py and routes/gradescope.py explaining why they couldn't send Retry-After. Both now send it. Known and accepted: a cancelled agent run never reaches record_agent_usage, so a timed-out generation's tokens don't land in llm_usage. Capturing usage from a cancelled pydantic-ai run isn't available at this seam; the per-run timeout narrows the window considerably versus cancelling the whole request. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>

7681c48) The audit behind the #537 addendum, written read-only in a worktree that has since been cleaned up — recovered and committed so it survives and so the YAML can be diffed as the product evolves. Its two code-only findings are tracked: the #529 42P10 (since fixed and additionally live-DB verified in #548) and the misconceptions offering-id mismatch (#553, which requires a live-DB check before any change). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Three findings, all about the schema-version half rather than the profile. **The version was LLM-controlled.** `schema_version` was a field on the digest agent's output_type, and `submit_quiz` persists `model_dump()` verbatim. Worse, this PR's own prompt feeds the previous digest back in under "update your notes" — so from now on the model would SEE `schema_version: 2` and be invited to update it. A model that helpfully bumped it to 3 would trip the reader's unknown-shape warning on every later read for that (user, concept), forever, with no real drift; one that lowered it would kill the guard just as quietly. The field is now off the agent schema entirely (so it costs no decoding budget either) and `save_quiz_context` stamps it server-side, which also covers every future writer. **The guard couldn't catch the drift it cited.** A version comparison only fires for a writer NEWER than the reader — a mixed-deploy window. #548 was a key RENAME at the same version, and catching that with a version number requires remembering to bump it in the same commit as the rename, which is precisely the discipline that failed the first time. The version now claims only what it can prove, and the rename case is caught by its OUTCOME: a row that is present, stamped with a version this reader understands, and yet yields nothing readable. A real digest cannot be all three at once — the agent always writes at least `questions_seen_summary` or `notes` — so that combination means the keys moved. Logged with the actual key set. **The docstring understated the blast radius.** It claimed this runs in the post-submit BackgroundTask. It does not: `submit_quiz` builds the whole prompt string synchronously and hands only the finished string to `add_task`. So an escaping exception 500s the submit AFTER the atomic `completed_at` claim, the mastery write and the score update have landed — the student sees a failed submit for a quiz that scored, and the retry 409s with the score never returned. The guard was right; the comment beside it invited a future reader to delete it as cheap background-task failure. The review also found a live instance of this same drift class in the OTHER reader of `context_json`: `course_context_service` harvests `effective_explanations`, a key no agent has ever written, so that column has persisted an empty array for every offering since it existed. Out of scope here, filed as #572. Hermetic 2184 passed / 9 skipped, ruff clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

) (#571) * feat(quiz): mine answers_json into the digest as a mistake profile (#554) `quiz_attempts.answers_json` has recorded which distractor a student picked, per question, on every submit since the column existed — and nothing has ever read it back. What the post-submit digest agent actually received was `results`, which carries LABELS: "question 3, picked B, the answer was C". That is not something a model can turn into a misconception; it can only guess one. The option TEXT — the thing that makes a wrong answer mean something — was one join away in `questions_json` the whole time. `services/quiz_distractors.py` does that join and hands the digest the wrong answers in words: the stem, the concept, what the student chose, and what was correct. Deliberately dumb — no model call, no I/O, pure data — and it never raises, because it runs in the post-submit BackgroundTask after the attempt is already graded and written, where a crash would cost the student the digest for a quiz they had already finished. Four cases it deliberately does NOT report, each of which would teach the digest something false: * correct answers — a mistake profile made of right answers spends tokens to report the absence of a problem; * unanswered questions — skipped is not wrong, and recording a blank as a distractor choice invents a misconception out of silence; * items with no correct option (#129's shape, which grade as wrong for everyone) — "the correct answer was <nothing>" is not a fact about the student; * results whose question is missing from the attempt. Digest schema version, the other half of the issue: `DIGEST_SCHEMA_VERSION` is stamped into `context_json` by the model's default rather than by the LLM (a field the model must remember to set is a field that goes missing), and the reader warns when it meets a version it doesn't understand. The drift this exists to catch is #548's: the coercer looked for `common_errors` while the agent wrote `common_mistakes`, and the symptom was an empty digest — indistinguishable from a new student. Tests pin the wiring as well as the logic, because `str.replace` on a renamed placeholder is a silent no-op: the profile would be computed, serialized, and dropped on the floor with nothing to show for it. Hermetic 2181 passed / 9 skipped, ruff clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(quiz): #554 review — the version was the model's to change Three findings, all about the schema-version half rather than the profile. **The version was LLM-controlled.** `schema_version` was a field on the digest agent's output_type, and `submit_quiz` persists `model_dump()` verbatim. Worse, this PR's own prompt feeds the previous digest back in under "update your notes" — so from now on the model would SEE `schema_version: 2` and be invited to update it. A model that helpfully bumped it to 3 would trip the reader's unknown-shape warning on every later read for that (user, concept), forever, with no real drift; one that lowered it would kill the guard just as quietly. The field is now off the agent schema entirely (so it costs no decoding budget either) and `save_quiz_context` stamps it server-side, which also covers every future writer. **The guard couldn't catch the drift it cited.** A version comparison only fires for a writer NEWER than the reader — a mixed-deploy window. #548 was a key RENAME at the same version, and catching that with a version number requires remembering to bump it in the same commit as the rename, which is precisely the discipline that failed the first time. The version now claims only what it can prove, and the rename case is caught by its OUTCOME: a row that is present, stamped with a version this reader understands, and yet yields nothing readable. A real digest cannot be all three at once — the agent always writes at least `questions_seen_summary` or `notes` — so that combination means the keys moved. Logged with the actual key set. **The docstring understated the blast radius.** It claimed this runs in the post-submit BackgroundTask. It does not: `submit_quiz` builds the whole prompt string synchronously and hands only the finished string to `add_task`. So an escaping exception 500s the submit AFTER the atomic `completed_at` claim, the mastery write and the score update have landed — the student sees a failed submit for a quiz that scored, and the retry 409s with the score never returned. The guard was right; the comment beside it invited a future reader to delete it as cheap background-task failure. The review also found a live instance of this same drift class in the OTHER reader of `context_json`: `course_context_service` harvests `effective_explanations`, a key no agent has ever written, so that column has persisted an empty array for every offering since it existed. Out of scope here, filed as #572. Hermetic 2184 passed / 9 skipped, ruff clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(quiz): the real-DB round-trip sees the stamped version too `test_save_quiz_context_upserts_one_row_and_encrypts` asserts the payload round-trips exactly, so server-stamping `schema_version` in `save_quiz_context` changed what comes back. The hermetic twin was updated with the fix; this is its real-DB counterpart, and only the integration lane could see it — which is the whole argument for that lane. Integration 56 passed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>

AndresL230 · 2026-08-12T21:43:14Z

Closes#529. Workstream B of the pre-revamp quiz repair batch (epic #537) — the highest-value fix in the batch: the difference between "adaptive" being a label and being true.

Forensics (verified live)

0001 created quiz_context with UNIQUE (user_id, concept_node_id); 0025_study_integrity.sql:108-114 recreated the table without it (dropped 2026-06-23, 72443e96).
services/quiz_context_service.py:37 upserts with on_conflict="user_id,concept_node_id" → PostgREST 42P10 on every write since; routes/quiz.py swallowed it (except Exception: pass).
Staging measured 2026-08-12: 0 rows, 0 duplicate pairs — the very first write already failed, so nothing ever accumulated. pg_constraint confirmed only the PK + two FKs.

What

B1 — migration 20260812210033 restores the UNIQUE (named quiz_context_user_concept_key), dedup-guarded (keep-newest, RAISE NOTICE reports removals) and idempotent. Applying it is backward-compatible; run db.migrate before/with the deploy.

B2 — the upsert's on_conflict target now has a matching constraint; the pairing is pinned by test.

B3 — the post-submit background write is loud on failure: logger.exception with attempt id + request id, a quiz.context_write_failed analytics event (category="error", added to the pinned EVENT_TAXONOMY) so it lands in admin analytics like error.4xx, and a re-raise under config.IS_LOCAL so this regression class fails CI instead of going quiet for months. Deliberate carve-out: the E2E seam's UnregisteredHandlerError (quiz_context stays unregistered in function mode by design) is a single WARNING with no traceback — the logscan oracle treats tracebacks as findings, and that fail-fast is the seam working.

B4 — the read side now consumes the wholeQuizContext shape. _coerce_summary previously returned notes alone and silently dropped weak_areas / common_mistakes / questions_seen_summary; its list fallback looked for common_errors, a key QuizContext never writes. A tool-level round-trip test covers ciphertext row → decrypt → digest in QuizHistory.summary.

B5 — real-DB integration tests (tests/integration/test_quiz_context_repair_db.py, #397 seam: app writes, raw psycopg reads): UNIQUE present with exactly the upsert's columns, double-upsert keeps one row, raw context_json is ciphertext, app read round-trips. No MagicMock on this path — the hermetic suite's mocks are precisely why this bug survived ~7.5 weeks.

Verification

Hermetic suite: 1926 passed (7 new tests, written first, watched fail).
Full local cycle under the stack lock: Playwright 43 passed → oracles exit 0 → integration lane 42 passed / 0 skipped (counts read, not exit codes) → teardown.
ruff check clean.
Staging migration applied ahead of merge (constraint restore is backward-compatible).

🤖 Generated with Claude Code

…allowing, consume the full digest (#529) Workstream B of the pre-revamp quiz repair batch (epic #537): B1 — migration 20260812210033 restores UNIQUE (user_id, concept_node_id) on quiz_context (dropped by 0025's table recreate), dedup-guarded and idempotent. Staging/prod measured 0 rows + 0 dupes — the very first upsert already 42P10'd, so nothing ever accumulated. B2 — save_quiz_context's on_conflict target now has a matching constraint again; pinned by test. B3 — the post-submit background write is loud on failure: ERROR log with attempt id + request id, a quiz.context_write_failed analytics event (added to the pinned EVENT_TAXONOMY), and a re-raise under config.IS_LOCAL so this regression class fails CI. The E2E seam's UnregisteredHandlerError stays a single WARNING with no traceback — quiz_context is deliberately unregistered in function mode and the logscan oracle treats tracebacks as findings. B4 — the read side now consumes the WHOLE QuizContext shape: _coerce_summary previously returned `notes` alone and dropped weak_areas / common_mistakes / questions_seen_summary (its list fallback also looked for `common_errors`, a key QuizContext never writes). Tool-level round-trip test covers ciphertext row → decrypt → digest in QuizHistory.summary. B5 — real-DB integration tests (tests/integration/ test_quiz_context_repair_db.py): constraint present, double-upsert keeps one row, raw column is ciphertext, app read round-trips. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

cloudflare-workers-and-pages · 2026-08-12T21:43:21Z

Deploying with Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status	Name	Latest Commit	Preview URL	Updated (UTC)
✅ Deployment successful! View logs	frontend-staging	`bdc9552`	Commit Preview URL Branch Preview URL	Aug 13 2026, 01:32 AM

supabase · 2026-08-12T21:43:22Z

This pull request has been ignored for the connected project ybgqdonkoqftwrmweuyv because there are no changes detected in supabase directory. You can change this behaviour in Project Integrations Settings ↗︎.

Preview Branches by Supabase.
Learn more about Supabase Branching ↗︎.

coderabbitai · 2026-08-12T21:43:25Z

Warning

Review limit reached

@AndresL230, you've reached your PR review limit, so we couldn't start this review.

Next review available in:94 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 @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 28afc089-32e5-401b-8776-c582420f41df

📥 Commits

Reviewing files that changed from the base of the PR and between 6081ec5 and bdc9552.

📒 Files selected for processing (10)

backend/agents/tools/quiz_history.py
backend/db/migrations/20260812210033_restore_quiz_context_unique.sql
backend/routes/admin_analytics.py
backend/routes/quiz.py
backend/services/events_service.py
backend/services/quiz_context_service.py
backend/tests/integration/test_quiz_context_repair_db.py
backend/tests/test_admin_analytics_routes.py
backend/tests/test_event_capture_seams.py
backend/tests/test_quiz_context_repair.py

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

…_difficulty, stable upsert id, error-category admin feed Review findings (xhigh, 4 confirmed): - _coerce_summary guards list-shaped keys with isinstance(list) so legacy rows holding a string/dict there no longer explode into per-character bullets in the agent's prompt digest. - recommended_difficulty is surfaced in the digest ("Recommended next difficulty: …") — previously computed, encrypted, persisted, and never consumed anywhere. - save_quiz_context no longer sends a client-generated id: with merge-duplicates live again, the payload id rewrote the row's PRIMARY KEY on every refresh. Fresh inserts use the column's DB default; integration test now pins id stability across refreshes. - /api/admin/analytics/errors filters by category=error instead of the error.* name prefix, so quiz.context_write_failed (and #482's rag.* events) actually appear in the feed the B3 comments promised — non-HTTP rows null their payload fields, which ErrorEvent already models as Optional. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

…e events (#544) (#552) * feat(quiz): rate limit, daily spend guard, generation timeout, failure events (#544) Workstream F of the pre-revamp quiz repair batch (epic #537): F1 — generate is no longer an unbounded LLM call behind a button: a per-user sliding-window limit (8 per 5 minutes, sized for a human comparing difficulties) returns 429 QUIZ_RATE_LIMITED with Retry-After, and a daily per-user LLM spend ceiling ($2, read off the llm_usage ledger agents/usage.py already writes) returns 429 QUIZ_DAILY_LIMIT_REACHED before the model runs. Both guards sit AFTER the ownership check, so probing a stranger's concept can't consume their quota, and the spend check fails OPEN — a usage-table blip must not deny every student. F2 — the whole generation (agent run + tools + the E2 top-up) is bounded by QUIZ_GENERATION_TIMEOUT_SEC and maps to its own QUIZ_GENERATION_TIMEOUT code, so the client can say "that took too long" instead of the generic failure. F3 — quiz.generation_failed events (category=error, with a reason: timeout / agent_guardrail / agent_error) join the pinned taxonomy, so a 502 the student saw is a 502 an admin can count in the errors feed #548 widened. A throttled student is deliberately NOT an error event. F4 — deep-link scoping tests: a foreign concept 404s before the agent runs, and a student's OWN concept from a past semester still generates (scoping is by ownership, not active semester — pinned so a future "scope to active semester" change can't silently break revision). Also: the process-global rate-limit state now resets between tests via a conftest autouse fixture, same reasoning as the lru_cache reset — without it one test's burst throttles every later test hitting the same route. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(quiz): address #552 review — each guardrail was undercutting itself All three F guards failed at what they were added to do, each confirmed by execution in review: - The daily spend cap summed an UNPAGED llm_usage select. PostgREST caps a response at max_rows (1000) and answers 206 — a 2xx — so the sum plateaued and the ceiling could never trip for the runaway user it targets. It pages now (with an early exit once the cap is crossed), and db/connection.py::select gained the `offset` it needed. - Wrapping the whole generation in asyncio.wait_for raised CancelledError — a BaseException — straight past #543's serve-what-we-have handler, so a timed-out top-up threw away a valid partial quiz and returned 502. Each agent run is bounded individually now, so a top-up timeout is an ordinary TimeoutError the existing handler degrades from; a completed 3-question generation is served instead of discarded. - The rate-limit slot was claimed before generation and never refunded, so eight backend 502s locked a student out for five minutes with a message saying they'd generated too many quizzes — having received none. Every failure path now refunds the slot (services/request_limits.py::refund_rate_limit). Also from the review: - The timeout branch caught the builtin OSError-family TimeoutError as well, relabelling transport socket timeouts as wall-clock timeouts. It catches only asyncio.TimeoutError now. - The spend-cap comment claimed cross-feature enforcement it doesn't provide; it now says what's actually true (the spend measured is cross-feature, the ceiling is enforced on quiz generation only). - main.py forwards exc.headers as of this branch, which falsified the comments in routes/extract.py and routes/gradescope.py explaining why they couldn't send Retry-After. Both now send it. Known and accepted: a cancelled agent run never reaches record_agent_usage, so a timed-out generation's tokens don't land in llm_usage. Capturing usage from a cancelled pydantic-ai run isn't available at this seam; the per-run timeout narrows the window considerably versus cancelling the whole request. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>

7681c48) The audit behind the #537 addendum, written read-only in a worktree that has since been cleaned up — recovered and committed so it survives and so the YAML can be diffed as the product evolves. Its two code-only findings are tracked: the #529 42P10 (since fixed and additionally live-DB verified in #548) and the misconceptions offering-id mismatch (#553, which requires a live-DB check before any change). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Three findings, all about the schema-version half rather than the profile. **The version was LLM-controlled.** `schema_version` was a field on the digest agent's output_type, and `submit_quiz` persists `model_dump()` verbatim. Worse, this PR's own prompt feeds the previous digest back in under "update your notes" — so from now on the model would SEE `schema_version: 2` and be invited to update it. A model that helpfully bumped it to 3 would trip the reader's unknown-shape warning on every later read for that (user, concept), forever, with no real drift; one that lowered it would kill the guard just as quietly. The field is now off the agent schema entirely (so it costs no decoding budget either) and `save_quiz_context` stamps it server-side, which also covers every future writer. **The guard couldn't catch the drift it cited.** A version comparison only fires for a writer NEWER than the reader — a mixed-deploy window. #548 was a key RENAME at the same version, and catching that with a version number requires remembering to bump it in the same commit as the rename, which is precisely the discipline that failed the first time. The version now claims only what it can prove, and the rename case is caught by its OUTCOME: a row that is present, stamped with a version this reader understands, and yet yields nothing readable. A real digest cannot be all three at once — the agent always writes at least `questions_seen_summary` or `notes` — so that combination means the keys moved. Logged with the actual key set. **The docstring understated the blast radius.** It claimed this runs in the post-submit BackgroundTask. It does not: `submit_quiz` builds the whole prompt string synchronously and hands only the finished string to `add_task`. So an escaping exception 500s the submit AFTER the atomic `completed_at` claim, the mastery write and the score update have landed — the student sees a failed submit for a quiz that scored, and the retry 409s with the score never returned. The guard was right; the comment beside it invited a future reader to delete it as cheap background-task failure. The review also found a live instance of this same drift class in the OTHER reader of `context_json`: `course_context_service` harvests `effective_explanations`, a key no agent has ever written, so that column has persisted an empty array for every offering since it existed. Out of scope here, filed as #572. Hermetic 2184 passed / 9 skipped, ruff clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

) (#571) * feat(quiz): mine answers_json into the digest as a mistake profile (#554) `quiz_attempts.answers_json` has recorded which distractor a student picked, per question, on every submit since the column existed — and nothing has ever read it back. What the post-submit digest agent actually received was `results`, which carries LABELS: "question 3, picked B, the answer was C". That is not something a model can turn into a misconception; it can only guess one. The option TEXT — the thing that makes a wrong answer mean something — was one join away in `questions_json` the whole time. `services/quiz_distractors.py` does that join and hands the digest the wrong answers in words: the stem, the concept, what the student chose, and what was correct. Deliberately dumb — no model call, no I/O, pure data — and it never raises, because it runs in the post-submit BackgroundTask after the attempt is already graded and written, where a crash would cost the student the digest for a quiz they had already finished. Four cases it deliberately does NOT report, each of which would teach the digest something false: * correct answers — a mistake profile made of right answers spends tokens to report the absence of a problem; * unanswered questions — skipped is not wrong, and recording a blank as a distractor choice invents a misconception out of silence; * items with no correct option (#129's shape, which grade as wrong for everyone) — "the correct answer was <nothing>" is not a fact about the student; * results whose question is missing from the attempt. Digest schema version, the other half of the issue: `DIGEST_SCHEMA_VERSION` is stamped into `context_json` by the model's default rather than by the LLM (a field the model must remember to set is a field that goes missing), and the reader warns when it meets a version it doesn't understand. The drift this exists to catch is #548's: the coercer looked for `common_errors` while the agent wrote `common_mistakes`, and the symptom was an empty digest — indistinguishable from a new student. Tests pin the wiring as well as the logic, because `str.replace` on a renamed placeholder is a silent no-op: the profile would be computed, serialized, and dropped on the floor with nothing to show for it. Hermetic 2181 passed / 9 skipped, ruff clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(quiz): #554 review — the version was the model's to change Three findings, all about the schema-version half rather than the profile. **The version was LLM-controlled.** `schema_version` was a field on the digest agent's output_type, and `submit_quiz` persists `model_dump()` verbatim. Worse, this PR's own prompt feeds the previous digest back in under "update your notes" — so from now on the model would SEE `schema_version: 2` and be invited to update it. A model that helpfully bumped it to 3 would trip the reader's unknown-shape warning on every later read for that (user, concept), forever, with no real drift; one that lowered it would kill the guard just as quietly. The field is now off the agent schema entirely (so it costs no decoding budget either) and `save_quiz_context` stamps it server-side, which also covers every future writer. **The guard couldn't catch the drift it cited.** A version comparison only fires for a writer NEWER than the reader — a mixed-deploy window. #548 was a key RENAME at the same version, and catching that with a version number requires remembering to bump it in the same commit as the rename, which is precisely the discipline that failed the first time. The version now claims only what it can prove, and the rename case is caught by its OUTCOME: a row that is present, stamped with a version this reader understands, and yet yields nothing readable. A real digest cannot be all three at once — the agent always writes at least `questions_seen_summary` or `notes` — so that combination means the keys moved. Logged with the actual key set. **The docstring understated the blast radius.** It claimed this runs in the post-submit BackgroundTask. It does not: `submit_quiz` builds the whole prompt string synchronously and hands only the finished string to `add_task`. So an escaping exception 500s the submit AFTER the atomic `completed_at` claim, the mastery write and the score update have landed — the student sees a failed submit for a quiz that scored, and the retry 409s with the score never returned. The guard was right; the comment beside it invited a future reader to delete it as cheap background-task failure. The review also found a live instance of this same drift class in the OTHER reader of `context_json`: `course_context_service` harvests `effective_explanations`, a key no agent has ever written, so that column has persisted an empty array for every offering since it existed. Out of scope here, filed as #572. Hermetic 2184 passed / 9 skipped, ruff clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(quiz): the real-DB round-trip sees the stamped version too `test_save_quiz_context_upserts_one_row_and_encrypts` asserts the payload round-trips exactly, so server-stamping `schema_version` in `save_quiz_context` changed what comes back. The hermetic twin was updated with the fix; this is its real-DB counterpart, and only the integration lane could see it — which is the whole argument for that lane. Integration 56 passed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>

AndresL230 · 2026-08-12T21:43:14Z

Closes#529. Workstream B of the pre-revamp quiz repair batch (epic #537) — the highest-value fix in the batch: the difference between "adaptive" being a label and being true.

Forensics (verified live)

0001 created quiz_context with UNIQUE (user_id, concept_node_id); 0025_study_integrity.sql:108-114 recreated the table without it (dropped 2026-06-23, 72443e96).
services/quiz_context_service.py:37 upserts with on_conflict="user_id,concept_node_id" → PostgREST 42P10 on every write since; routes/quiz.py swallowed it (except Exception: pass).
Staging measured 2026-08-12: 0 rows, 0 duplicate pairs — the very first write already failed, so nothing ever accumulated. pg_constraint confirmed only the PK + two FKs.

What

B1 — migration 20260812210033 restores the UNIQUE (named quiz_context_user_concept_key), dedup-guarded (keep-newest, RAISE NOTICE reports removals) and idempotent. Applying it is backward-compatible; run db.migrate before/with the deploy.

B2 — the upsert's on_conflict target now has a matching constraint; the pairing is pinned by test.

B3 — the post-submit background write is loud on failure: logger.exception with attempt id + request id, a quiz.context_write_failed analytics event (category="error", added to the pinned EVENT_TAXONOMY) so it lands in admin analytics like error.4xx, and a re-raise under config.IS_LOCAL so this regression class fails CI instead of going quiet for months. Deliberate carve-out: the E2E seam's UnregisteredHandlerError (quiz_context stays unregistered in function mode by design) is a single WARNING with no traceback — the logscan oracle treats tracebacks as findings, and that fail-fast is the seam working.

B4 — the read side now consumes the wholeQuizContext shape. _coerce_summary previously returned notes alone and silently dropped weak_areas / common_mistakes / questions_seen_summary; its list fallback looked for common_errors, a key QuizContext never writes. A tool-level round-trip test covers ciphertext row → decrypt → digest in QuizHistory.summary.

B5 — real-DB integration tests (tests/integration/test_quiz_context_repair_db.py, #397 seam: app writes, raw psycopg reads): UNIQUE present with exactly the upsert's columns, double-upsert keeps one row, raw context_json is ciphertext, app read round-trips. No MagicMock on this path — the hermetic suite's mocks are precisely why this bug survived ~7.5 weeks.

Verification

Hermetic suite: 1926 passed (7 new tests, written first, watched fail).
Full local cycle under the stack lock: Playwright 43 passed → oracles exit 0 → integration lane 42 passed / 0 skipped (counts read, not exit codes) → teardown.
ruff check clean.
Staging migration applied ahead of merge (constraint restore is backward-compatible).

🤖 Generated with Claude Code

…allowing, consume the full digest (#529) Workstream B of the pre-revamp quiz repair batch (epic #537): B1 — migration 20260812210033 restores UNIQUE (user_id, concept_node_id) on quiz_context (dropped by 0025's table recreate), dedup-guarded and idempotent. Staging/prod measured 0 rows + 0 dupes — the very first upsert already 42P10'd, so nothing ever accumulated. B2 — save_quiz_context's on_conflict target now has a matching constraint again; pinned by test. B3 — the post-submit background write is loud on failure: ERROR log with attempt id + request id, a quiz.context_write_failed analytics event (added to the pinned EVENT_TAXONOMY), and a re-raise under config.IS_LOCAL so this regression class fails CI. The E2E seam's UnregisteredHandlerError stays a single WARNING with no traceback — quiz_context is deliberately unregistered in function mode and the logscan oracle treats tracebacks as findings. B4 — the read side now consumes the WHOLE QuizContext shape: _coerce_summary previously returned `notes` alone and dropped weak_areas / common_mistakes / questions_seen_summary (its list fallback also looked for `common_errors`, a key QuizContext never writes). Tool-level round-trip test covers ciphertext row → decrypt → digest in QuizHistory.summary. B5 — real-DB integration tests (tests/integration/ test_quiz_context_repair_db.py): constraint present, double-upsert keeps one row, raw column is ciphertext, app read round-trips. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

cloudflare-workers-and-pages · 2026-08-12T21:43:21Z

Deploying with Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status	Name	Latest Commit	Preview URL	Updated (UTC)
✅ Deployment successful! View logs	frontend-staging	`bdc9552`	Commit Preview URL Branch Preview URL	Aug 13 2026, 01:32 AM

supabase · 2026-08-12T21:43:22Z

This pull request has been ignored for the connected project ybgqdonkoqftwrmweuyv because there are no changes detected in supabase directory. You can change this behaviour in Project Integrations Settings ↗︎.

Preview Branches by Supabase.
Learn more about Supabase Branching ↗︎.

coderabbitai · 2026-08-12T21:43:25Z

Warning

Review limit reached

@AndresL230, you've reached your PR review limit, so we couldn't start this review.

Next review available in:94 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 @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 28afc089-32e5-401b-8776-c582420f41df

📥 Commits

Reviewing files that changed from the base of the PR and between 6081ec5 and bdc9552.

📒 Files selected for processing (10)

backend/agents/tools/quiz_history.py
backend/db/migrations/20260812210033_restore_quiz_context_unique.sql
backend/routes/admin_analytics.py
backend/routes/quiz.py
backend/services/events_service.py
backend/services/quiz_context_service.py
backend/tests/integration/test_quiz_context_repair_db.py
backend/tests/test_admin_analytics_routes.py
backend/tests/test_event_capture_seams.py
backend/tests/test_quiz_context_repair.py

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

…_difficulty, stable upsert id, error-category admin feed Review findings (xhigh, 4 confirmed): - _coerce_summary guards list-shaped keys with isinstance(list) so legacy rows holding a string/dict there no longer explode into per-character bullets in the agent's prompt digest. - recommended_difficulty is surfaced in the digest ("Recommended next difficulty: …") — previously computed, encrypted, persisted, and never consumed anywhere. - save_quiz_context no longer sends a client-generated id: with merge-duplicates live again, the payload id rewrote the row's PRIMARY KEY on every refresh. Fresh inserts use the column's DB default; integration test now pins id stability across refreshes. - /api/admin/analytics/errors filters by category=error instead of the error.* name prefix, so quiz.context_write_failed (and #482's rag.* events) actually appear in the feed the B3 comments promised — non-HTTP rows null their payload fields, which ErrorEvent already models as Optional. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

…e events (#544) (#552) * feat(quiz): rate limit, daily spend guard, generation timeout, failure events (#544) Workstream F of the pre-revamp quiz repair batch (epic #537): F1 — generate is no longer an unbounded LLM call behind a button: a per-user sliding-window limit (8 per 5 minutes, sized for a human comparing difficulties) returns 429 QUIZ_RATE_LIMITED with Retry-After, and a daily per-user LLM spend ceiling ($2, read off the llm_usage ledger agents/usage.py already writes) returns 429 QUIZ_DAILY_LIMIT_REACHED before the model runs. Both guards sit AFTER the ownership check, so probing a stranger's concept can't consume their quota, and the spend check fails OPEN — a usage-table blip must not deny every student. F2 — the whole generation (agent run + tools + the E2 top-up) is bounded by QUIZ_GENERATION_TIMEOUT_SEC and maps to its own QUIZ_GENERATION_TIMEOUT code, so the client can say "that took too long" instead of the generic failure. F3 — quiz.generation_failed events (category=error, with a reason: timeout / agent_guardrail / agent_error) join the pinned taxonomy, so a 502 the student saw is a 502 an admin can count in the errors feed #548 widened. A throttled student is deliberately NOT an error event. F4 — deep-link scoping tests: a foreign concept 404s before the agent runs, and a student's OWN concept from a past semester still generates (scoping is by ownership, not active semester — pinned so a future "scope to active semester" change can't silently break revision). Also: the process-global rate-limit state now resets between tests via a conftest autouse fixture, same reasoning as the lru_cache reset — without it one test's burst throttles every later test hitting the same route. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(quiz): address #552 review — each guardrail was undercutting itself All three F guards failed at what they were added to do, each confirmed by execution in review: - The daily spend cap summed an UNPAGED llm_usage select. PostgREST caps a response at max_rows (1000) and answers 206 — a 2xx — so the sum plateaued and the ceiling could never trip for the runaway user it targets. It pages now (with an early exit once the cap is crossed), and db/connection.py::select gained the `offset` it needed. - Wrapping the whole generation in asyncio.wait_for raised CancelledError — a BaseException — straight past #543's serve-what-we-have handler, so a timed-out top-up threw away a valid partial quiz and returned 502. Each agent run is bounded individually now, so a top-up timeout is an ordinary TimeoutError the existing handler degrades from; a completed 3-question generation is served instead of discarded. - The rate-limit slot was claimed before generation and never refunded, so eight backend 502s locked a student out for five minutes with a message saying they'd generated too many quizzes — having received none. Every failure path now refunds the slot (services/request_limits.py::refund_rate_limit). Also from the review: - The timeout branch caught the builtin OSError-family TimeoutError as well, relabelling transport socket timeouts as wall-clock timeouts. It catches only asyncio.TimeoutError now. - The spend-cap comment claimed cross-feature enforcement it doesn't provide; it now says what's actually true (the spend measured is cross-feature, the ceiling is enforced on quiz generation only). - main.py forwards exc.headers as of this branch, which falsified the comments in routes/extract.py and routes/gradescope.py explaining why they couldn't send Retry-After. Both now send it. Known and accepted: a cancelled agent run never reaches record_agent_usage, so a timed-out generation's tokens don't land in llm_usage. Capturing usage from a cancelled pydantic-ai run isn't available at this seam; the per-run timeout narrows the window considerably versus cancelling the whole request. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>

7681c48) The audit behind the #537 addendum, written read-only in a worktree that has since been cleaned up — recovered and committed so it survives and so the YAML can be diffed as the product evolves. Its two code-only findings are tracked: the #529 42P10 (since fixed and additionally live-DB verified in #548) and the misconceptions offering-id mismatch (#553, which requires a live-DB check before any change). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Three findings, all about the schema-version half rather than the profile. **The version was LLM-controlled.** `schema_version` was a field on the digest agent's output_type, and `submit_quiz` persists `model_dump()` verbatim. Worse, this PR's own prompt feeds the previous digest back in under "update your notes" — so from now on the model would SEE `schema_version: 2` and be invited to update it. A model that helpfully bumped it to 3 would trip the reader's unknown-shape warning on every later read for that (user, concept), forever, with no real drift; one that lowered it would kill the guard just as quietly. The field is now off the agent schema entirely (so it costs no decoding budget either) and `save_quiz_context` stamps it server-side, which also covers every future writer. **The guard couldn't catch the drift it cited.** A version comparison only fires for a writer NEWER than the reader — a mixed-deploy window. #548 was a key RENAME at the same version, and catching that with a version number requires remembering to bump it in the same commit as the rename, which is precisely the discipline that failed the first time. The version now claims only what it can prove, and the rename case is caught by its OUTCOME: a row that is present, stamped with a version this reader understands, and yet yields nothing readable. A real digest cannot be all three at once — the agent always writes at least `questions_seen_summary` or `notes` — so that combination means the keys moved. Logged with the actual key set. **The docstring understated the blast radius.** It claimed this runs in the post-submit BackgroundTask. It does not: `submit_quiz` builds the whole prompt string synchronously and hands only the finished string to `add_task`. So an escaping exception 500s the submit AFTER the atomic `completed_at` claim, the mastery write and the score update have landed — the student sees a failed submit for a quiz that scored, and the retry 409s with the score never returned. The guard was right; the comment beside it invited a future reader to delete it as cheap background-task failure. The review also found a live instance of this same drift class in the OTHER reader of `context_json`: `course_context_service` harvests `effective_explanations`, a key no agent has ever written, so that column has persisted an empty array for every offering since it existed. Out of scope here, filed as #572. Hermetic 2184 passed / 9 skipped, ruff clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

) (#571) * feat(quiz): mine answers_json into the digest as a mistake profile (#554) `quiz_attempts.answers_json` has recorded which distractor a student picked, per question, on every submit since the column existed — and nothing has ever read it back. What the post-submit digest agent actually received was `results`, which carries LABELS: "question 3, picked B, the answer was C". That is not something a model can turn into a misconception; it can only guess one. The option TEXT — the thing that makes a wrong answer mean something — was one join away in `questions_json` the whole time. `services/quiz_distractors.py` does that join and hands the digest the wrong answers in words: the stem, the concept, what the student chose, and what was correct. Deliberately dumb — no model call, no I/O, pure data — and it never raises, because it runs in the post-submit BackgroundTask after the attempt is already graded and written, where a crash would cost the student the digest for a quiz they had already finished. Four cases it deliberately does NOT report, each of which would teach the digest something false: * correct answers — a mistake profile made of right answers spends tokens to report the absence of a problem; * unanswered questions — skipped is not wrong, and recording a blank as a distractor choice invents a misconception out of silence; * items with no correct option (#129's shape, which grade as wrong for everyone) — "the correct answer was <nothing>" is not a fact about the student; * results whose question is missing from the attempt. Digest schema version, the other half of the issue: `DIGEST_SCHEMA_VERSION` is stamped into `context_json` by the model's default rather than by the LLM (a field the model must remember to set is a field that goes missing), and the reader warns when it meets a version it doesn't understand. The drift this exists to catch is #548's: the coercer looked for `common_errors` while the agent wrote `common_mistakes`, and the symptom was an empty digest — indistinguishable from a new student. Tests pin the wiring as well as the logic, because `str.replace` on a renamed placeholder is a silent no-op: the profile would be computed, serialized, and dropped on the floor with nothing to show for it. Hermetic 2181 passed / 9 skipped, ruff clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(quiz): #554 review — the version was the model's to change Three findings, all about the schema-version half rather than the profile. **The version was LLM-controlled.** `schema_version` was a field on the digest agent's output_type, and `submit_quiz` persists `model_dump()` verbatim. Worse, this PR's own prompt feeds the previous digest back in under "update your notes" — so from now on the model would SEE `schema_version: 2` and be invited to update it. A model that helpfully bumped it to 3 would trip the reader's unknown-shape warning on every later read for that (user, concept), forever, with no real drift; one that lowered it would kill the guard just as quietly. The field is now off the agent schema entirely (so it costs no decoding budget either) and `save_quiz_context` stamps it server-side, which also covers every future writer. **The guard couldn't catch the drift it cited.** A version comparison only fires for a writer NEWER than the reader — a mixed-deploy window. #548 was a key RENAME at the same version, and catching that with a version number requires remembering to bump it in the same commit as the rename, which is precisely the discipline that failed the first time. The version now claims only what it can prove, and the rename case is caught by its OUTCOME: a row that is present, stamped with a version this reader understands, and yet yields nothing readable. A real digest cannot be all three at once — the agent always writes at least `questions_seen_summary` or `notes` — so that combination means the keys moved. Logged with the actual key set. **The docstring understated the blast radius.** It claimed this runs in the post-submit BackgroundTask. It does not: `submit_quiz` builds the whole prompt string synchronously and hands only the finished string to `add_task`. So an escaping exception 500s the submit AFTER the atomic `completed_at` claim, the mastery write and the score update have landed — the student sees a failed submit for a quiz that scored, and the retry 409s with the score never returned. The guard was right; the comment beside it invited a future reader to delete it as cheap background-task failure. The review also found a live instance of this same drift class in the OTHER reader of `context_json`: `course_context_service` harvests `effective_explanations`, a key no agent has ever written, so that column has persisted an empty array for every offering since it existed. Out of scope here, filed as #572. Hermetic 2184 passed / 9 skipped, ruff clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(quiz): the real-DB round-trip sees the stamped version too `test_save_quiz_context_upserts_one_row_and_encrypts` asserts the payload round-trips exactly, so server-stamping `schema_version` in `save_quiz_context` changed what comes back. The hermetic twin was updated with the fix; this is its real-DB counterpart, and only the integration lane could see it — which is the whole argument for that lane. Integration 56 passed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>

AndresL230 · 2026-08-12T21:43:14Z

Closes#529. Workstream B of the pre-revamp quiz repair batch (epic #537) — the highest-value fix in the batch: the difference between "adaptive" being a label and being true.

Forensics (verified live)

0001 created quiz_context with UNIQUE (user_id, concept_node_id); 0025_study_integrity.sql:108-114 recreated the table without it (dropped 2026-06-23, 72443e96).
services/quiz_context_service.py:37 upserts with on_conflict="user_id,concept_node_id" → PostgREST 42P10 on every write since; routes/quiz.py swallowed it (except Exception: pass).
Staging measured 2026-08-12: 0 rows, 0 duplicate pairs — the very first write already failed, so nothing ever accumulated. pg_constraint confirmed only the PK + two FKs.

What

B1 — migration 20260812210033 restores the UNIQUE (named quiz_context_user_concept_key), dedup-guarded (keep-newest, RAISE NOTICE reports removals) and idempotent. Applying it is backward-compatible; run db.migrate before/with the deploy.

B2 — the upsert's on_conflict target now has a matching constraint; the pairing is pinned by test.

B3 — the post-submit background write is loud on failure: logger.exception with attempt id + request id, a quiz.context_write_failed analytics event (category="error", added to the pinned EVENT_TAXONOMY) so it lands in admin analytics like error.4xx, and a re-raise under config.IS_LOCAL so this regression class fails CI instead of going quiet for months. Deliberate carve-out: the E2E seam's UnregisteredHandlerError (quiz_context stays unregistered in function mode by design) is a single WARNING with no traceback — the logscan oracle treats tracebacks as findings, and that fail-fast is the seam working.

B4 — the read side now consumes the wholeQuizContext shape. _coerce_summary previously returned notes alone and silently dropped weak_areas / common_mistakes / questions_seen_summary; its list fallback looked for common_errors, a key QuizContext never writes. A tool-level round-trip test covers ciphertext row → decrypt → digest in QuizHistory.summary.

B5 — real-DB integration tests (tests/integration/test_quiz_context_repair_db.py, #397 seam: app writes, raw psycopg reads): UNIQUE present with exactly the upsert's columns, double-upsert keeps one row, raw context_json is ciphertext, app read round-trips. No MagicMock on this path — the hermetic suite's mocks are precisely why this bug survived ~7.5 weeks.

Verification

Hermetic suite: 1926 passed (7 new tests, written first, watched fail).
Full local cycle under the stack lock: Playwright 43 passed → oracles exit 0 → integration lane 42 passed / 0 skipped (counts read, not exit codes) → teardown.
ruff check clean.
Staging migration applied ahead of merge (constraint restore is backward-compatible).

🤖 Generated with Claude Code

…allowing, consume the full digest (#529) Workstream B of the pre-revamp quiz repair batch (epic #537): B1 — migration 20260812210033 restores UNIQUE (user_id, concept_node_id) on quiz_context (dropped by 0025's table recreate), dedup-guarded and idempotent. Staging/prod measured 0 rows + 0 dupes — the very first upsert already 42P10'd, so nothing ever accumulated. B2 — save_quiz_context's on_conflict target now has a matching constraint again; pinned by test. B3 — the post-submit background write is loud on failure: ERROR log with attempt id + request id, a quiz.context_write_failed analytics event (added to the pinned EVENT_TAXONOMY), and a re-raise under config.IS_LOCAL so this regression class fails CI. The E2E seam's UnregisteredHandlerError stays a single WARNING with no traceback — quiz_context is deliberately unregistered in function mode and the logscan oracle treats tracebacks as findings. B4 — the read side now consumes the WHOLE QuizContext shape: _coerce_summary previously returned `notes` alone and dropped weak_areas / common_mistakes / questions_seen_summary (its list fallback also looked for `common_errors`, a key QuizContext never writes). Tool-level round-trip test covers ciphertext row → decrypt → digest in QuizHistory.summary. B5 — real-DB integration tests (tests/integration/ test_quiz_context_repair_db.py): constraint present, double-upsert keeps one row, raw column is ciphertext, app read round-trips. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

cloudflare-workers-and-pages · 2026-08-12T21:43:21Z

Deploying with Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status	Name	Latest Commit	Preview URL	Updated (UTC)
✅ Deployment successful! View logs	frontend-staging	`bdc9552`	Commit Preview URL Branch Preview URL	Aug 13 2026, 01:32 AM

supabase · 2026-08-12T21:43:22Z

This pull request has been ignored for the connected project ybgqdonkoqftwrmweuyv because there are no changes detected in supabase directory. You can change this behaviour in Project Integrations Settings ↗︎.

Preview Branches by Supabase.
Learn more about Supabase Branching ↗︎.

coderabbitai · 2026-08-12T21:43:25Z

Warning

Review limit reached

@AndresL230, you've reached your PR review limit, so we couldn't start this review.

Next review available in:94 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 @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 28afc089-32e5-401b-8776-c582420f41df

📥 Commits

Reviewing files that changed from the base of the PR and between 6081ec5 and bdc9552.

📒 Files selected for processing (10)

backend/agents/tools/quiz_history.py
backend/db/migrations/20260812210033_restore_quiz_context_unique.sql
backend/routes/admin_analytics.py
backend/routes/quiz.py
backend/services/events_service.py
backend/services/quiz_context_service.py
backend/tests/integration/test_quiz_context_repair_db.py
backend/tests/test_admin_analytics_routes.py
backend/tests/test_event_capture_seams.py
backend/tests/test_quiz_context_repair.py

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

…_difficulty, stable upsert id, error-category admin feed Review findings (xhigh, 4 confirmed): - _coerce_summary guards list-shaped keys with isinstance(list) so legacy rows holding a string/dict there no longer explode into per-character bullets in the agent's prompt digest. - recommended_difficulty is surfaced in the digest ("Recommended next difficulty: …") — previously computed, encrypted, persisted, and never consumed anywhere. - save_quiz_context no longer sends a client-generated id: with merge-duplicates live again, the payload id rewrote the row's PRIMARY KEY on every refresh. Fresh inserts use the column's DB default; integration test now pins id stability across refreshes. - /api/admin/analytics/errors filters by category=error instead of the error.* name prefix, so quiz.context_write_failed (and #482's rag.* events) actually appear in the feed the B3 comments promised — non-HTTP rows null their payload fields, which ErrorEvent already models as Optional. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

…e events (#544) (#552) * feat(quiz): rate limit, daily spend guard, generation timeout, failure events (#544) Workstream F of the pre-revamp quiz repair batch (epic #537): F1 — generate is no longer an unbounded LLM call behind a button: a per-user sliding-window limit (8 per 5 minutes, sized for a human comparing difficulties) returns 429 QUIZ_RATE_LIMITED with Retry-After, and a daily per-user LLM spend ceiling ($2, read off the llm_usage ledger agents/usage.py already writes) returns 429 QUIZ_DAILY_LIMIT_REACHED before the model runs. Both guards sit AFTER the ownership check, so probing a stranger's concept can't consume their quota, and the spend check fails OPEN — a usage-table blip must not deny every student. F2 — the whole generation (agent run + tools + the E2 top-up) is bounded by QUIZ_GENERATION_TIMEOUT_SEC and maps to its own QUIZ_GENERATION_TIMEOUT code, so the client can say "that took too long" instead of the generic failure. F3 — quiz.generation_failed events (category=error, with a reason: timeout / agent_guardrail / agent_error) join the pinned taxonomy, so a 502 the student saw is a 502 an admin can count in the errors feed #548 widened. A throttled student is deliberately NOT an error event. F4 — deep-link scoping tests: a foreign concept 404s before the agent runs, and a student's OWN concept from a past semester still generates (scoping is by ownership, not active semester — pinned so a future "scope to active semester" change can't silently break revision). Also: the process-global rate-limit state now resets between tests via a conftest autouse fixture, same reasoning as the lru_cache reset — without it one test's burst throttles every later test hitting the same route. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(quiz): address #552 review — each guardrail was undercutting itself All three F guards failed at what they were added to do, each confirmed by execution in review: - The daily spend cap summed an UNPAGED llm_usage select. PostgREST caps a response at max_rows (1000) and answers 206 — a 2xx — so the sum plateaued and the ceiling could never trip for the runaway user it targets. It pages now (with an early exit once the cap is crossed), and db/connection.py::select gained the `offset` it needed. - Wrapping the whole generation in asyncio.wait_for raised CancelledError — a BaseException — straight past #543's serve-what-we-have handler, so a timed-out top-up threw away a valid partial quiz and returned 502. Each agent run is bounded individually now, so a top-up timeout is an ordinary TimeoutError the existing handler degrades from; a completed 3-question generation is served instead of discarded. - The rate-limit slot was claimed before generation and never refunded, so eight backend 502s locked a student out for five minutes with a message saying they'd generated too many quizzes — having received none. Every failure path now refunds the slot (services/request_limits.py::refund_rate_limit). Also from the review: - The timeout branch caught the builtin OSError-family TimeoutError as well, relabelling transport socket timeouts as wall-clock timeouts. It catches only asyncio.TimeoutError now. - The spend-cap comment claimed cross-feature enforcement it doesn't provide; it now says what's actually true (the spend measured is cross-feature, the ceiling is enforced on quiz generation only). - main.py forwards exc.headers as of this branch, which falsified the comments in routes/extract.py and routes/gradescope.py explaining why they couldn't send Retry-After. Both now send it. Known and accepted: a cancelled agent run never reaches record_agent_usage, so a timed-out generation's tokens don't land in llm_usage. Capturing usage from a cancelled pydantic-ai run isn't available at this seam; the per-run timeout narrows the window considerably versus cancelling the whole request. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>

7681c48) The audit behind the #537 addendum, written read-only in a worktree that has since been cleaned up — recovered and committed so it survives and so the YAML can be diffed as the product evolves. Its two code-only findings are tracked: the #529 42P10 (since fixed and additionally live-DB verified in #548) and the misconceptions offering-id mismatch (#553, which requires a live-DB check before any change). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Three findings, all about the schema-version half rather than the profile. **The version was LLM-controlled.** `schema_version` was a field on the digest agent's output_type, and `submit_quiz` persists `model_dump()` verbatim. Worse, this PR's own prompt feeds the previous digest back in under "update your notes" — so from now on the model would SEE `schema_version: 2` and be invited to update it. A model that helpfully bumped it to 3 would trip the reader's unknown-shape warning on every later read for that (user, concept), forever, with no real drift; one that lowered it would kill the guard just as quietly. The field is now off the agent schema entirely (so it costs no decoding budget either) and `save_quiz_context` stamps it server-side, which also covers every future writer. **The guard couldn't catch the drift it cited.** A version comparison only fires for a writer NEWER than the reader — a mixed-deploy window. #548 was a key RENAME at the same version, and catching that with a version number requires remembering to bump it in the same commit as the rename, which is precisely the discipline that failed the first time. The version now claims only what it can prove, and the rename case is caught by its OUTCOME: a row that is present, stamped with a version this reader understands, and yet yields nothing readable. A real digest cannot be all three at once — the agent always writes at least `questions_seen_summary` or `notes` — so that combination means the keys moved. Logged with the actual key set. **The docstring understated the blast radius.** It claimed this runs in the post-submit BackgroundTask. It does not: `submit_quiz` builds the whole prompt string synchronously and hands only the finished string to `add_task`. So an escaping exception 500s the submit AFTER the atomic `completed_at` claim, the mastery write and the score update have landed — the student sees a failed submit for a quiz that scored, and the retry 409s with the score never returned. The guard was right; the comment beside it invited a future reader to delete it as cheap background-task failure. The review also found a live instance of this same drift class in the OTHER reader of `context_json`: `course_context_service` harvests `effective_explanations`, a key no agent has ever written, so that column has persisted an empty array for every offering since it existed. Out of scope here, filed as #572. Hermetic 2184 passed / 9 skipped, ruff clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

) (#571) * feat(quiz): mine answers_json into the digest as a mistake profile (#554) `quiz_attempts.answers_json` has recorded which distractor a student picked, per question, on every submit since the column existed — and nothing has ever read it back. What the post-submit digest agent actually received was `results`, which carries LABELS: "question 3, picked B, the answer was C". That is not something a model can turn into a misconception; it can only guess one. The option TEXT — the thing that makes a wrong answer mean something — was one join away in `questions_json` the whole time. `services/quiz_distractors.py` does that join and hands the digest the wrong answers in words: the stem, the concept, what the student chose, and what was correct. Deliberately dumb — no model call, no I/O, pure data — and it never raises, because it runs in the post-submit BackgroundTask after the attempt is already graded and written, where a crash would cost the student the digest for a quiz they had already finished. Four cases it deliberately does NOT report, each of which would teach the digest something false: * correct answers — a mistake profile made of right answers spends tokens to report the absence of a problem; * unanswered questions — skipped is not wrong, and recording a blank as a distractor choice invents a misconception out of silence; * items with no correct option (#129's shape, which grade as wrong for everyone) — "the correct answer was <nothing>" is not a fact about the student; * results whose question is missing from the attempt. Digest schema version, the other half of the issue: `DIGEST_SCHEMA_VERSION` is stamped into `context_json` by the model's default rather than by the LLM (a field the model must remember to set is a field that goes missing), and the reader warns when it meets a version it doesn't understand. The drift this exists to catch is #548's: the coercer looked for `common_errors` while the agent wrote `common_mistakes`, and the symptom was an empty digest — indistinguishable from a new student. Tests pin the wiring as well as the logic, because `str.replace` on a renamed placeholder is a silent no-op: the profile would be computed, serialized, and dropped on the floor with nothing to show for it. Hermetic 2181 passed / 9 skipped, ruff clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(quiz): #554 review — the version was the model's to change Three findings, all about the schema-version half rather than the profile. **The version was LLM-controlled.** `schema_version` was a field on the digest agent's output_type, and `submit_quiz` persists `model_dump()` verbatim. Worse, this PR's own prompt feeds the previous digest back in under "update your notes" — so from now on the model would SEE `schema_version: 2` and be invited to update it. A model that helpfully bumped it to 3 would trip the reader's unknown-shape warning on every later read for that (user, concept), forever, with no real drift; one that lowered it would kill the guard just as quietly. The field is now off the agent schema entirely (so it costs no decoding budget either) and `save_quiz_context` stamps it server-side, which also covers every future writer. **The guard couldn't catch the drift it cited.** A version comparison only fires for a writer NEWER than the reader — a mixed-deploy window. #548 was a key RENAME at the same version, and catching that with a version number requires remembering to bump it in the same commit as the rename, which is precisely the discipline that failed the first time. The version now claims only what it can prove, and the rename case is caught by its OUTCOME: a row that is present, stamped with a version this reader understands, and yet yields nothing readable. A real digest cannot be all three at once — the agent always writes at least `questions_seen_summary` or `notes` — so that combination means the keys moved. Logged with the actual key set. **The docstring understated the blast radius.** It claimed this runs in the post-submit BackgroundTask. It does not: `submit_quiz` builds the whole prompt string synchronously and hands only the finished string to `add_task`. So an escaping exception 500s the submit AFTER the atomic `completed_at` claim, the mastery write and the score update have landed — the student sees a failed submit for a quiz that scored, and the retry 409s with the score never returned. The guard was right; the comment beside it invited a future reader to delete it as cheap background-task failure. The review also found a live instance of this same drift class in the OTHER reader of `context_json`: `course_context_service` harvests `effective_explanations`, a key no agent has ever written, so that column has persisted an empty array for every offering since it existed. Out of scope here, filed as #572. Hermetic 2184 passed / 9 skipped, ruff clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(quiz): the real-DB round-trip sees the stamped version too `test_save_quiz_context_upserts_one_row_and_encrypts` asserts the payload round-trips exactly, so server-stamping `schema_version` in `save_quiz_context` changed what comes back. The hermetic twin was updated with the fix; this is its real-DB counterpart, and only the integration lane could see it — which is the whole argument for that lane. Integration 56 passed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>

AndresL230 · 2026-08-12T21:43:14Z

Closes#529. Workstream B of the pre-revamp quiz repair batch (epic #537) — the highest-value fix in the batch: the difference between "adaptive" being a label and being true.

Forensics (verified live)

0001 created quiz_context with UNIQUE (user_id, concept_node_id); 0025_study_integrity.sql:108-114 recreated the table without it (dropped 2026-06-23, 72443e96).
services/quiz_context_service.py:37 upserts with on_conflict="user_id,concept_node_id" → PostgREST 42P10 on every write since; routes/quiz.py swallowed it (except Exception: pass).
Staging measured 2026-08-12: 0 rows, 0 duplicate pairs — the very first write already failed, so nothing ever accumulated. pg_constraint confirmed only the PK + two FKs.

What

B1 — migration 20260812210033 restores the UNIQUE (named quiz_context_user_concept_key), dedup-guarded (keep-newest, RAISE NOTICE reports removals) and idempotent. Applying it is backward-compatible; run db.migrate before/with the deploy.

B2 — the upsert's on_conflict target now has a matching constraint; the pairing is pinned by test.

B3 — the post-submit background write is loud on failure: logger.exception with attempt id + request id, a quiz.context_write_failed analytics event (category="error", added to the pinned EVENT_TAXONOMY) so it lands in admin analytics like error.4xx, and a re-raise under config.IS_LOCAL so this regression class fails CI instead of going quiet for months. Deliberate carve-out: the E2E seam's UnregisteredHandlerError (quiz_context stays unregistered in function mode by design) is a single WARNING with no traceback — the logscan oracle treats tracebacks as findings, and that fail-fast is the seam working.

B4 — the read side now consumes the wholeQuizContext shape. _coerce_summary previously returned notes alone and silently dropped weak_areas / common_mistakes / questions_seen_summary; its list fallback looked for common_errors, a key QuizContext never writes. A tool-level round-trip test covers ciphertext row → decrypt → digest in QuizHistory.summary.

B5 — real-DB integration tests (tests/integration/test_quiz_context_repair_db.py, #397 seam: app writes, raw psycopg reads): UNIQUE present with exactly the upsert's columns, double-upsert keeps one row, raw context_json is ciphertext, app read round-trips. No MagicMock on this path — the hermetic suite's mocks are precisely why this bug survived ~7.5 weeks.

Verification

Hermetic suite: 1926 passed (7 new tests, written first, watched fail).
Full local cycle under the stack lock: Playwright 43 passed → oracles exit 0 → integration lane 42 passed / 0 skipped (counts read, not exit codes) → teardown.
ruff check clean.
Staging migration applied ahead of merge (constraint restore is backward-compatible).

🤖 Generated with Claude Code

…allowing, consume the full digest (#529) Workstream B of the pre-revamp quiz repair batch (epic #537): B1 — migration 20260812210033 restores UNIQUE (user_id, concept_node_id) on quiz_context (dropped by 0025's table recreate), dedup-guarded and idempotent. Staging/prod measured 0 rows + 0 dupes — the very first upsert already 42P10'd, so nothing ever accumulated. B2 — save_quiz_context's on_conflict target now has a matching constraint again; pinned by test. B3 — the post-submit background write is loud on failure: ERROR log with attempt id + request id, a quiz.context_write_failed analytics event (added to the pinned EVENT_TAXONOMY), and a re-raise under config.IS_LOCAL so this regression class fails CI. The E2E seam's UnregisteredHandlerError stays a single WARNING with no traceback — quiz_context is deliberately unregistered in function mode and the logscan oracle treats tracebacks as findings. B4 — the read side now consumes the WHOLE QuizContext shape: _coerce_summary previously returned `notes` alone and dropped weak_areas / common_mistakes / questions_seen_summary (its list fallback also looked for `common_errors`, a key QuizContext never writes). Tool-level round-trip test covers ciphertext row → decrypt → digest in QuizHistory.summary. B5 — real-DB integration tests (tests/integration/ test_quiz_context_repair_db.py): constraint present, double-upsert keeps one row, raw column is ciphertext, app read round-trips. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

cloudflare-workers-and-pages · 2026-08-12T21:43:21Z

Deploying with Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status	Name	Latest Commit	Preview URL	Updated (UTC)
✅ Deployment successful! View logs	frontend-staging	`bdc9552`	Commit Preview URL Branch Preview URL	Aug 13 2026, 01:32 AM

supabase · 2026-08-12T21:43:22Z

This pull request has been ignored for the connected project ybgqdonkoqftwrmweuyv because there are no changes detected in supabase directory. You can change this behaviour in Project Integrations Settings ↗︎.

Preview Branches by Supabase.
Learn more about Supabase Branching ↗︎.

coderabbitai · 2026-08-12T21:43:25Z

Warning

Review limit reached

@AndresL230, you've reached your PR review limit, so we couldn't start this review.

Next review available in:94 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 @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 28afc089-32e5-401b-8776-c582420f41df

📥 Commits

Reviewing files that changed from the base of the PR and between 6081ec5 and bdc9552.

📒 Files selected for processing (10)

backend/agents/tools/quiz_history.py
backend/db/migrations/20260812210033_restore_quiz_context_unique.sql
backend/routes/admin_analytics.py
backend/routes/quiz.py
backend/services/events_service.py
backend/services/quiz_context_service.py
backend/tests/integration/test_quiz_context_repair_db.py
backend/tests/test_admin_analytics_routes.py
backend/tests/test_event_capture_seams.py
backend/tests/test_quiz_context_repair.py

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

…_difficulty, stable upsert id, error-category admin feed Review findings (xhigh, 4 confirmed): - _coerce_summary guards list-shaped keys with isinstance(list) so legacy rows holding a string/dict there no longer explode into per-character bullets in the agent's prompt digest. - recommended_difficulty is surfaced in the digest ("Recommended next difficulty: …") — previously computed, encrypted, persisted, and never consumed anywhere. - save_quiz_context no longer sends a client-generated id: with merge-duplicates live again, the payload id rewrote the row's PRIMARY KEY on every refresh. Fresh inserts use the column's DB default; integration test now pins id stability across refreshes. - /api/admin/analytics/errors filters by category=error instead of the error.* name prefix, so quiz.context_write_failed (and #482's rag.* events) actually appear in the feed the B3 comments promised — non-HTTP rows null their payload fields, which ErrorEvent already models as Optional. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

…e events (#544) (#552) * feat(quiz): rate limit, daily spend guard, generation timeout, failure events (#544) Workstream F of the pre-revamp quiz repair batch (epic #537): F1 — generate is no longer an unbounded LLM call behind a button: a per-user sliding-window limit (8 per 5 minutes, sized for a human comparing difficulties) returns 429 QUIZ_RATE_LIMITED with Retry-After, and a daily per-user LLM spend ceiling ($2, read off the llm_usage ledger agents/usage.py already writes) returns 429 QUIZ_DAILY_LIMIT_REACHED before the model runs. Both guards sit AFTER the ownership check, so probing a stranger's concept can't consume their quota, and the spend check fails OPEN — a usage-table blip must not deny every student. F2 — the whole generation (agent run + tools + the E2 top-up) is bounded by QUIZ_GENERATION_TIMEOUT_SEC and maps to its own QUIZ_GENERATION_TIMEOUT code, so the client can say "that took too long" instead of the generic failure. F3 — quiz.generation_failed events (category=error, with a reason: timeout / agent_guardrail / agent_error) join the pinned taxonomy, so a 502 the student saw is a 502 an admin can count in the errors feed #548 widened. A throttled student is deliberately NOT an error event. F4 — deep-link scoping tests: a foreign concept 404s before the agent runs, and a student's OWN concept from a past semester still generates (scoping is by ownership, not active semester — pinned so a future "scope to active semester" change can't silently break revision). Also: the process-global rate-limit state now resets between tests via a conftest autouse fixture, same reasoning as the lru_cache reset — without it one test's burst throttles every later test hitting the same route. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(quiz): address #552 review — each guardrail was undercutting itself All three F guards failed at what they were added to do, each confirmed by execution in review: - The daily spend cap summed an UNPAGED llm_usage select. PostgREST caps a response at max_rows (1000) and answers 206 — a 2xx — so the sum plateaued and the ceiling could never trip for the runaway user it targets. It pages now (with an early exit once the cap is crossed), and db/connection.py::select gained the `offset` it needed. - Wrapping the whole generation in asyncio.wait_for raised CancelledError — a BaseException — straight past #543's serve-what-we-have handler, so a timed-out top-up threw away a valid partial quiz and returned 502. Each agent run is bounded individually now, so a top-up timeout is an ordinary TimeoutError the existing handler degrades from; a completed 3-question generation is served instead of discarded. - The rate-limit slot was claimed before generation and never refunded, so eight backend 502s locked a student out for five minutes with a message saying they'd generated too many quizzes — having received none. Every failure path now refunds the slot (services/request_limits.py::refund_rate_limit). Also from the review: - The timeout branch caught the builtin OSError-family TimeoutError as well, relabelling transport socket timeouts as wall-clock timeouts. It catches only asyncio.TimeoutError now. - The spend-cap comment claimed cross-feature enforcement it doesn't provide; it now says what's actually true (the spend measured is cross-feature, the ceiling is enforced on quiz generation only). - main.py forwards exc.headers as of this branch, which falsified the comments in routes/extract.py and routes/gradescope.py explaining why they couldn't send Retry-After. Both now send it. Known and accepted: a cancelled agent run never reaches record_agent_usage, so a timed-out generation's tokens don't land in llm_usage. Capturing usage from a cancelled pydantic-ai run isn't available at this seam; the per-run timeout narrows the window considerably versus cancelling the whole request. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>

7681c48) The audit behind the #537 addendum, written read-only in a worktree that has since been cleaned up — recovered and committed so it survives and so the YAML can be diffed as the product evolves. Its two code-only findings are tracked: the #529 42P10 (since fixed and additionally live-DB verified in #548) and the misconceptions offering-id mismatch (#553, which requires a live-DB check before any change). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Three findings, all about the schema-version half rather than the profile. **The version was LLM-controlled.** `schema_version` was a field on the digest agent's output_type, and `submit_quiz` persists `model_dump()` verbatim. Worse, this PR's own prompt feeds the previous digest back in under "update your notes" — so from now on the model would SEE `schema_version: 2` and be invited to update it. A model that helpfully bumped it to 3 would trip the reader's unknown-shape warning on every later read for that (user, concept), forever, with no real drift; one that lowered it would kill the guard just as quietly. The field is now off the agent schema entirely (so it costs no decoding budget either) and `save_quiz_context` stamps it server-side, which also covers every future writer. **The guard couldn't catch the drift it cited.** A version comparison only fires for a writer NEWER than the reader — a mixed-deploy window. #548 was a key RENAME at the same version, and catching that with a version number requires remembering to bump it in the same commit as the rename, which is precisely the discipline that failed the first time. The version now claims only what it can prove, and the rename case is caught by its OUTCOME: a row that is present, stamped with a version this reader understands, and yet yields nothing readable. A real digest cannot be all three at once — the agent always writes at least `questions_seen_summary` or `notes` — so that combination means the keys moved. Logged with the actual key set. **The docstring understated the blast radius.** It claimed this runs in the post-submit BackgroundTask. It does not: `submit_quiz` builds the whole prompt string synchronously and hands only the finished string to `add_task`. So an escaping exception 500s the submit AFTER the atomic `completed_at` claim, the mastery write and the score update have landed — the student sees a failed submit for a quiz that scored, and the retry 409s with the score never returned. The guard was right; the comment beside it invited a future reader to delete it as cheap background-task failure. The review also found a live instance of this same drift class in the OTHER reader of `context_json`: `course_context_service` harvests `effective_explanations`, a key no agent has ever written, so that column has persisted an empty array for every offering since it existed. Out of scope here, filed as #572. Hermetic 2184 passed / 9 skipped, ruff clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

) (#571) * feat(quiz): mine answers_json into the digest as a mistake profile (#554) `quiz_attempts.answers_json` has recorded which distractor a student picked, per question, on every submit since the column existed — and nothing has ever read it back. What the post-submit digest agent actually received was `results`, which carries LABELS: "question 3, picked B, the answer was C". That is not something a model can turn into a misconception; it can only guess one. The option TEXT — the thing that makes a wrong answer mean something — was one join away in `questions_json` the whole time. `services/quiz_distractors.py` does that join and hands the digest the wrong answers in words: the stem, the concept, what the student chose, and what was correct. Deliberately dumb — no model call, no I/O, pure data — and it never raises, because it runs in the post-submit BackgroundTask after the attempt is already graded and written, where a crash would cost the student the digest for a quiz they had already finished. Four cases it deliberately does NOT report, each of which would teach the digest something false: * correct answers — a mistake profile made of right answers spends tokens to report the absence of a problem; * unanswered questions — skipped is not wrong, and recording a blank as a distractor choice invents a misconception out of silence; * items with no correct option (#129's shape, which grade as wrong for everyone) — "the correct answer was <nothing>" is not a fact about the student; * results whose question is missing from the attempt. Digest schema version, the other half of the issue: `DIGEST_SCHEMA_VERSION` is stamped into `context_json` by the model's default rather than by the LLM (a field the model must remember to set is a field that goes missing), and the reader warns when it meets a version it doesn't understand. The drift this exists to catch is #548's: the coercer looked for `common_errors` while the agent wrote `common_mistakes`, and the symptom was an empty digest — indistinguishable from a new student. Tests pin the wiring as well as the logic, because `str.replace` on a renamed placeholder is a silent no-op: the profile would be computed, serialized, and dropped on the floor with nothing to show for it. Hermetic 2181 passed / 9 skipped, ruff clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(quiz): #554 review — the version was the model's to change Three findings, all about the schema-version half rather than the profile. **The version was LLM-controlled.** `schema_version` was a field on the digest agent's output_type, and `submit_quiz` persists `model_dump()` verbatim. Worse, this PR's own prompt feeds the previous digest back in under "update your notes" — so from now on the model would SEE `schema_version: 2` and be invited to update it. A model that helpfully bumped it to 3 would trip the reader's unknown-shape warning on every later read for that (user, concept), forever, with no real drift; one that lowered it would kill the guard just as quietly. The field is now off the agent schema entirely (so it costs no decoding budget either) and `save_quiz_context` stamps it server-side, which also covers every future writer. **The guard couldn't catch the drift it cited.** A version comparison only fires for a writer NEWER than the reader — a mixed-deploy window. #548 was a key RENAME at the same version, and catching that with a version number requires remembering to bump it in the same commit as the rename, which is precisely the discipline that failed the first time. The version now claims only what it can prove, and the rename case is caught by its OUTCOME: a row that is present, stamped with a version this reader understands, and yet yields nothing readable. A real digest cannot be all three at once — the agent always writes at least `questions_seen_summary` or `notes` — so that combination means the keys moved. Logged with the actual key set. **The docstring understated the blast radius.** It claimed this runs in the post-submit BackgroundTask. It does not: `submit_quiz` builds the whole prompt string synchronously and hands only the finished string to `add_task`. So an escaping exception 500s the submit AFTER the atomic `completed_at` claim, the mastery write and the score update have landed — the student sees a failed submit for a quiz that scored, and the retry 409s with the score never returned. The guard was right; the comment beside it invited a future reader to delete it as cheap background-task failure. The review also found a live instance of this same drift class in the OTHER reader of `context_json`: `course_context_service` harvests `effective_explanations`, a key no agent has ever written, so that column has persisted an empty array for every offering since it existed. Out of scope here, filed as #572. Hermetic 2184 passed / 9 skipped, ruff clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(quiz): the real-DB round-trip sees the stamped version too `test_save_quiz_context_upserts_one_row_and_encrypts` asserts the payload round-trips exactly, so server-stamping `schema_version` in `save_quiz_context` changed what comes back. The hermetic twin was updated with the fix; this is its real-DB counterpart, and only the integration lane could see it — which is the whole argument for that lane. Integration 56 passed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>

AndresL230 · 2026-08-12T21:43:14Z

Closes#529. Workstream B of the pre-revamp quiz repair batch (epic #537) — the highest-value fix in the batch: the difference between "adaptive" being a label and being true.

Forensics (verified live)

0001 created quiz_context with UNIQUE (user_id, concept_node_id); 0025_study_integrity.sql:108-114 recreated the table without it (dropped 2026-06-23, 72443e96).
services/quiz_context_service.py:37 upserts with on_conflict="user_id,concept_node_id" → PostgREST 42P10 on every write since; routes/quiz.py swallowed it (except Exception: pass).
Staging measured 2026-08-12: 0 rows, 0 duplicate pairs — the very first write already failed, so nothing ever accumulated. pg_constraint confirmed only the PK + two FKs.

What

B1 — migration 20260812210033 restores the UNIQUE (named quiz_context_user_concept_key), dedup-guarded (keep-newest, RAISE NOTICE reports removals) and idempotent. Applying it is backward-compatible; run db.migrate before/with the deploy.

B2 — the upsert's on_conflict target now has a matching constraint; the pairing is pinned by test.

B3 — the post-submit background write is loud on failure: logger.exception with attempt id + request id, a quiz.context_write_failed analytics event (category="error", added to the pinned EVENT_TAXONOMY) so it lands in admin analytics like error.4xx, and a re-raise under config.IS_LOCAL so this regression class fails CI instead of going quiet for months. Deliberate carve-out: the E2E seam's UnregisteredHandlerError (quiz_context stays unregistered in function mode by design) is a single WARNING with no traceback — the logscan oracle treats tracebacks as findings, and that fail-fast is the seam working.

B4 — the read side now consumes the wholeQuizContext shape. _coerce_summary previously returned notes alone and silently dropped weak_areas / common_mistakes / questions_seen_summary; its list fallback looked for common_errors, a key QuizContext never writes. A tool-level round-trip test covers ciphertext row → decrypt → digest in QuizHistory.summary.

B5 — real-DB integration tests (tests/integration/test_quiz_context_repair_db.py, #397 seam: app writes, raw psycopg reads): UNIQUE present with exactly the upsert's columns, double-upsert keeps one row, raw context_json is ciphertext, app read round-trips. No MagicMock on this path — the hermetic suite's mocks are precisely why this bug survived ~7.5 weeks.

Verification

Hermetic suite: 1926 passed (7 new tests, written first, watched fail).
Full local cycle under the stack lock: Playwright 43 passed → oracles exit 0 → integration lane 42 passed / 0 skipped (counts read, not exit codes) → teardown.
ruff check clean.
Staging migration applied ahead of merge (constraint restore is backward-compatible).

🤖 Generated with Claude Code

…allowing, consume the full digest (#529) Workstream B of the pre-revamp quiz repair batch (epic #537): B1 — migration 20260812210033 restores UNIQUE (user_id, concept_node_id) on quiz_context (dropped by 0025's table recreate), dedup-guarded and idempotent. Staging/prod measured 0 rows + 0 dupes — the very first upsert already 42P10'd, so nothing ever accumulated. B2 — save_quiz_context's on_conflict target now has a matching constraint again; pinned by test. B3 — the post-submit background write is loud on failure: ERROR log with attempt id + request id, a quiz.context_write_failed analytics event (added to the pinned EVENT_TAXONOMY), and a re-raise under config.IS_LOCAL so this regression class fails CI. The E2E seam's UnregisteredHandlerError stays a single WARNING with no traceback — quiz_context is deliberately unregistered in function mode and the logscan oracle treats tracebacks as findings. B4 — the read side now consumes the WHOLE QuizContext shape: _coerce_summary previously returned `notes` alone and dropped weak_areas / common_mistakes / questions_seen_summary (its list fallback also looked for `common_errors`, a key QuizContext never writes). Tool-level round-trip test covers ciphertext row → decrypt → digest in QuizHistory.summary. B5 — real-DB integration tests (tests/integration/ test_quiz_context_repair_db.py): constraint present, double-upsert keeps one row, raw column is ciphertext, app read round-trips. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

cloudflare-workers-and-pages · 2026-08-12T21:43:21Z

Deploying with Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status	Name	Latest Commit	Preview URL	Updated (UTC)
✅ Deployment successful! View logs	frontend-staging	`bdc9552`	Commit Preview URL Branch Preview URL	Aug 13 2026, 01:32 AM

supabase · 2026-08-12T21:43:22Z

This pull request has been ignored for the connected project ybgqdonkoqftwrmweuyv because there are no changes detected in supabase directory. You can change this behaviour in Project Integrations Settings ↗︎.

Preview Branches by Supabase.
Learn more about Supabase Branching ↗︎.

coderabbitai · 2026-08-12T21:43:25Z

Warning

Review limit reached

@AndresL230, you've reached your PR review limit, so we couldn't start this review.

Next review available in:94 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 @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 28afc089-32e5-401b-8776-c582420f41df

📥 Commits

Reviewing files that changed from the base of the PR and between 6081ec5 and bdc9552.

📒 Files selected for processing (10)

backend/agents/tools/quiz_history.py
backend/db/migrations/20260812210033_restore_quiz_context_unique.sql
backend/routes/admin_analytics.py
backend/routes/quiz.py
backend/services/events_service.py
backend/services/quiz_context_service.py
backend/tests/integration/test_quiz_context_repair_db.py
backend/tests/test_admin_analytics_routes.py
backend/tests/test_event_capture_seams.py
backend/tests/test_quiz_context_repair.py

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

…_difficulty, stable upsert id, error-category admin feed Review findings (xhigh, 4 confirmed): - _coerce_summary guards list-shaped keys with isinstance(list) so legacy rows holding a string/dict there no longer explode into per-character bullets in the agent's prompt digest. - recommended_difficulty is surfaced in the digest ("Recommended next difficulty: …") — previously computed, encrypted, persisted, and never consumed anywhere. - save_quiz_context no longer sends a client-generated id: with merge-duplicates live again, the payload id rewrote the row's PRIMARY KEY on every refresh. Fresh inserts use the column's DB default; integration test now pins id stability across refreshes. - /api/admin/analytics/errors filters by category=error instead of the error.* name prefix, so quiz.context_write_failed (and #482's rag.* events) actually appear in the feed the B3 comments promised — non-HTTP rows null their payload fields, which ErrorEvent already models as Optional. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

…e events (#544) (#552) * feat(quiz): rate limit, daily spend guard, generation timeout, failure events (#544) Workstream F of the pre-revamp quiz repair batch (epic #537): F1 — generate is no longer an unbounded LLM call behind a button: a per-user sliding-window limit (8 per 5 minutes, sized for a human comparing difficulties) returns 429 QUIZ_RATE_LIMITED with Retry-After, and a daily per-user LLM spend ceiling ($2, read off the llm_usage ledger agents/usage.py already writes) returns 429 QUIZ_DAILY_LIMIT_REACHED before the model runs. Both guards sit AFTER the ownership check, so probing a stranger's concept can't consume their quota, and the spend check fails OPEN — a usage-table blip must not deny every student. F2 — the whole generation (agent run + tools + the E2 top-up) is bounded by QUIZ_GENERATION_TIMEOUT_SEC and maps to its own QUIZ_GENERATION_TIMEOUT code, so the client can say "that took too long" instead of the generic failure. F3 — quiz.generation_failed events (category=error, with a reason: timeout / agent_guardrail / agent_error) join the pinned taxonomy, so a 502 the student saw is a 502 an admin can count in the errors feed #548 widened. A throttled student is deliberately NOT an error event. F4 — deep-link scoping tests: a foreign concept 404s before the agent runs, and a student's OWN concept from a past semester still generates (scoping is by ownership, not active semester — pinned so a future "scope to active semester" change can't silently break revision). Also: the process-global rate-limit state now resets between tests via a conftest autouse fixture, same reasoning as the lru_cache reset — without it one test's burst throttles every later test hitting the same route. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(quiz): address #552 review — each guardrail was undercutting itself All three F guards failed at what they were added to do, each confirmed by execution in review: - The daily spend cap summed an UNPAGED llm_usage select. PostgREST caps a response at max_rows (1000) and answers 206 — a 2xx — so the sum plateaued and the ceiling could never trip for the runaway user it targets. It pages now (with an early exit once the cap is crossed), and db/connection.py::select gained the `offset` it needed. - Wrapping the whole generation in asyncio.wait_for raised CancelledError — a BaseException — straight past #543's serve-what-we-have handler, so a timed-out top-up threw away a valid partial quiz and returned 502. Each agent run is bounded individually now, so a top-up timeout is an ordinary TimeoutError the existing handler degrades from; a completed 3-question generation is served instead of discarded. - The rate-limit slot was claimed before generation and never refunded, so eight backend 502s locked a student out for five minutes with a message saying they'd generated too many quizzes — having received none. Every failure path now refunds the slot (services/request_limits.py::refund_rate_limit). Also from the review: - The timeout branch caught the builtin OSError-family TimeoutError as well, relabelling transport socket timeouts as wall-clock timeouts. It catches only asyncio.TimeoutError now. - The spend-cap comment claimed cross-feature enforcement it doesn't provide; it now says what's actually true (the spend measured is cross-feature, the ceiling is enforced on quiz generation only). - main.py forwards exc.headers as of this branch, which falsified the comments in routes/extract.py and routes/gradescope.py explaining why they couldn't send Retry-After. Both now send it. Known and accepted: a cancelled agent run never reaches record_agent_usage, so a timed-out generation's tokens don't land in llm_usage. Capturing usage from a cancelled pydantic-ai run isn't available at this seam; the per-run timeout narrows the window considerably versus cancelling the whole request. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>

7681c48) The audit behind the #537 addendum, written read-only in a worktree that has since been cleaned up — recovered and committed so it survives and so the YAML can be diffed as the product evolves. Its two code-only findings are tracked: the #529 42P10 (since fixed and additionally live-DB verified in #548) and the misconceptions offering-id mismatch (#553, which requires a live-DB check before any change). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Three findings, all about the schema-version half rather than the profile. **The version was LLM-controlled.** `schema_version` was a field on the digest agent's output_type, and `submit_quiz` persists `model_dump()` verbatim. Worse, this PR's own prompt feeds the previous digest back in under "update your notes" — so from now on the model would SEE `schema_version: 2` and be invited to update it. A model that helpfully bumped it to 3 would trip the reader's unknown-shape warning on every later read for that (user, concept), forever, with no real drift; one that lowered it would kill the guard just as quietly. The field is now off the agent schema entirely (so it costs no decoding budget either) and `save_quiz_context` stamps it server-side, which also covers every future writer. **The guard couldn't catch the drift it cited.** A version comparison only fires for a writer NEWER than the reader — a mixed-deploy window. #548 was a key RENAME at the same version, and catching that with a version number requires remembering to bump it in the same commit as the rename, which is precisely the discipline that failed the first time. The version now claims only what it can prove, and the rename case is caught by its OUTCOME: a row that is present, stamped with a version this reader understands, and yet yields nothing readable. A real digest cannot be all three at once — the agent always writes at least `questions_seen_summary` or `notes` — so that combination means the keys moved. Logged with the actual key set. **The docstring understated the blast radius.** It claimed this runs in the post-submit BackgroundTask. It does not: `submit_quiz` builds the whole prompt string synchronously and hands only the finished string to `add_task`. So an escaping exception 500s the submit AFTER the atomic `completed_at` claim, the mastery write and the score update have landed — the student sees a failed submit for a quiz that scored, and the retry 409s with the score never returned. The guard was right; the comment beside it invited a future reader to delete it as cheap background-task failure. The review also found a live instance of this same drift class in the OTHER reader of `context_json`: `course_context_service` harvests `effective_explanations`, a key no agent has ever written, so that column has persisted an empty array for every offering since it existed. Out of scope here, filed as #572. Hermetic 2184 passed / 9 skipped, ruff clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

) (#571) * feat(quiz): mine answers_json into the digest as a mistake profile (#554) `quiz_attempts.answers_json` has recorded which distractor a student picked, per question, on every submit since the column existed — and nothing has ever read it back. What the post-submit digest agent actually received was `results`, which carries LABELS: "question 3, picked B, the answer was C". That is not something a model can turn into a misconception; it can only guess one. The option TEXT — the thing that makes a wrong answer mean something — was one join away in `questions_json` the whole time. `services/quiz_distractors.py` does that join and hands the digest the wrong answers in words: the stem, the concept, what the student chose, and what was correct. Deliberately dumb — no model call, no I/O, pure data — and it never raises, because it runs in the post-submit BackgroundTask after the attempt is already graded and written, where a crash would cost the student the digest for a quiz they had already finished. Four cases it deliberately does NOT report, each of which would teach the digest something false: * correct answers — a mistake profile made of right answers spends tokens to report the absence of a problem; * unanswered questions — skipped is not wrong, and recording a blank as a distractor choice invents a misconception out of silence; * items with no correct option (#129's shape, which grade as wrong for everyone) — "the correct answer was <nothing>" is not a fact about the student; * results whose question is missing from the attempt. Digest schema version, the other half of the issue: `DIGEST_SCHEMA_VERSION` is stamped into `context_json` by the model's default rather than by the LLM (a field the model must remember to set is a field that goes missing), and the reader warns when it meets a version it doesn't understand. The drift this exists to catch is #548's: the coercer looked for `common_errors` while the agent wrote `common_mistakes`, and the symptom was an empty digest — indistinguishable from a new student. Tests pin the wiring as well as the logic, because `str.replace` on a renamed placeholder is a silent no-op: the profile would be computed, serialized, and dropped on the floor with nothing to show for it. Hermetic 2181 passed / 9 skipped, ruff clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(quiz): #554 review — the version was the model's to change Three findings, all about the schema-version half rather than the profile. **The version was LLM-controlled.** `schema_version` was a field on the digest agent's output_type, and `submit_quiz` persists `model_dump()` verbatim. Worse, this PR's own prompt feeds the previous digest back in under "update your notes" — so from now on the model would SEE `schema_version: 2` and be invited to update it. A model that helpfully bumped it to 3 would trip the reader's unknown-shape warning on every later read for that (user, concept), forever, with no real drift; one that lowered it would kill the guard just as quietly. The field is now off the agent schema entirely (so it costs no decoding budget either) and `save_quiz_context` stamps it server-side, which also covers every future writer. **The guard couldn't catch the drift it cited.** A version comparison only fires for a writer NEWER than the reader — a mixed-deploy window. #548 was a key RENAME at the same version, and catching that with a version number requires remembering to bump it in the same commit as the rename, which is precisely the discipline that failed the first time. The version now claims only what it can prove, and the rename case is caught by its OUTCOME: a row that is present, stamped with a version this reader understands, and yet yields nothing readable. A real digest cannot be all three at once — the agent always writes at least `questions_seen_summary` or `notes` — so that combination means the keys moved. Logged with the actual key set. **The docstring understated the blast radius.** It claimed this runs in the post-submit BackgroundTask. It does not: `submit_quiz` builds the whole prompt string synchronously and hands only the finished string to `add_task`. So an escaping exception 500s the submit AFTER the atomic `completed_at` claim, the mastery write and the score update have landed — the student sees a failed submit for a quiz that scored, and the retry 409s with the score never returned. The guard was right; the comment beside it invited a future reader to delete it as cheap background-task failure. The review also found a live instance of this same drift class in the OTHER reader of `context_json`: `course_context_service` harvests `effective_explanations`, a key no agent has ever written, so that column has persisted an empty array for every offering since it existed. Out of scope here, filed as #572. Hermetic 2184 passed / 9 skipped, ruff clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(quiz): the real-DB round-trip sees the stamped version too `test_save_quiz_context_upserts_one_row_and_encrypts` asserts the payload round-trips exactly, so server-stamping `schema_version` in `save_quiz_context` changed what comes back. The hermetic twin was updated with the fix; this is its real-DB counterpart, and only the integration lane could see it — which is the whole argument for that lane. Integration 56 passed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>

AndresL230 · 2026-08-12T21:43:14Z

Closes#529. Workstream B of the pre-revamp quiz repair batch (epic #537) — the highest-value fix in the batch: the difference between "adaptive" being a label and being true.

Forensics (verified live)

0001 created quiz_context with UNIQUE (user_id, concept_node_id); 0025_study_integrity.sql:108-114 recreated the table without it (dropped 2026-06-23, 72443e96).
services/quiz_context_service.py:37 upserts with on_conflict="user_id,concept_node_id" → PostgREST 42P10 on every write since; routes/quiz.py swallowed it (except Exception: pass).
Staging measured 2026-08-12: 0 rows, 0 duplicate pairs — the very first write already failed, so nothing ever accumulated. pg_constraint confirmed only the PK + two FKs.

What

B1 — migration 20260812210033 restores the UNIQUE (named quiz_context_user_concept_key), dedup-guarded (keep-newest, RAISE NOTICE reports removals) and idempotent. Applying it is backward-compatible; run db.migrate before/with the deploy.

B2 — the upsert's on_conflict target now has a matching constraint; the pairing is pinned by test.

B3 — the post-submit background write is loud on failure: logger.exception with attempt id + request id, a quiz.context_write_failed analytics event (category="error", added to the pinned EVENT_TAXONOMY) so it lands in admin analytics like error.4xx, and a re-raise under config.IS_LOCAL so this regression class fails CI instead of going quiet for months. Deliberate carve-out: the E2E seam's UnregisteredHandlerError (quiz_context stays unregistered in function mode by design) is a single WARNING with no traceback — the logscan oracle treats tracebacks as findings, and that fail-fast is the seam working.

B4 — the read side now consumes the wholeQuizContext shape. _coerce_summary previously returned notes alone and silently dropped weak_areas / common_mistakes / questions_seen_summary; its list fallback looked for common_errors, a key QuizContext never writes. A tool-level round-trip test covers ciphertext row → decrypt → digest in QuizHistory.summary.

B5 — real-DB integration tests (tests/integration/test_quiz_context_repair_db.py, #397 seam: app writes, raw psycopg reads): UNIQUE present with exactly the upsert's columns, double-upsert keeps one row, raw context_json is ciphertext, app read round-trips. No MagicMock on this path — the hermetic suite's mocks are precisely why this bug survived ~7.5 weeks.

Verification

Hermetic suite: 1926 passed (7 new tests, written first, watched fail).
Full local cycle under the stack lock: Playwright 43 passed → oracles exit 0 → integration lane 42 passed / 0 skipped (counts read, not exit codes) → teardown.
ruff check clean.
Staging migration applied ahead of merge (constraint restore is backward-compatible).

🤖 Generated with Claude Code

…allowing, consume the full digest (#529) Workstream B of the pre-revamp quiz repair batch (epic #537): B1 — migration 20260812210033 restores UNIQUE (user_id, concept_node_id) on quiz_context (dropped by 0025's table recreate), dedup-guarded and idempotent. Staging/prod measured 0 rows + 0 dupes — the very first upsert already 42P10'd, so nothing ever accumulated. B2 — save_quiz_context's on_conflict target now has a matching constraint again; pinned by test. B3 — the post-submit background write is loud on failure: ERROR log with attempt id + request id, a quiz.context_write_failed analytics event (added to the pinned EVENT_TAXONOMY), and a re-raise under config.IS_LOCAL so this regression class fails CI. The E2E seam's UnregisteredHandlerError stays a single WARNING with no traceback — quiz_context is deliberately unregistered in function mode and the logscan oracle treats tracebacks as findings. B4 — the read side now consumes the WHOLE QuizContext shape: _coerce_summary previously returned `notes` alone and dropped weak_areas / common_mistakes / questions_seen_summary (its list fallback also looked for `common_errors`, a key QuizContext never writes). Tool-level round-trip test covers ciphertext row → decrypt → digest in QuizHistory.summary. B5 — real-DB integration tests (tests/integration/ test_quiz_context_repair_db.py): constraint present, double-upsert keeps one row, raw column is ciphertext, app read round-trips. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

cloudflare-workers-and-pages · 2026-08-12T21:43:21Z

Deploying with Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status	Name	Latest Commit	Preview URL	Updated (UTC)
✅ Deployment successful! View logs	frontend-staging	`bdc9552`	Commit Preview URL Branch Preview URL	Aug 13 2026, 01:32 AM

supabase · 2026-08-12T21:43:22Z

This pull request has been ignored for the connected project ybgqdonkoqftwrmweuyv because there are no changes detected in supabase directory. You can change this behaviour in Project Integrations Settings ↗︎.

Preview Branches by Supabase.
Learn more about Supabase Branching ↗︎.

coderabbitai · 2026-08-12T21:43:25Z

Warning

Review limit reached

@AndresL230, you've reached your PR review limit, so we couldn't start this review.

Next review available in:94 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 @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 28afc089-32e5-401b-8776-c582420f41df

📥 Commits

Reviewing files that changed from the base of the PR and between 6081ec5 and bdc9552.

📒 Files selected for processing (10)

backend/agents/tools/quiz_history.py
backend/db/migrations/20260812210033_restore_quiz_context_unique.sql
backend/routes/admin_analytics.py
backend/routes/quiz.py
backend/services/events_service.py
backend/services/quiz_context_service.py
backend/tests/integration/test_quiz_context_repair_db.py
backend/tests/test_admin_analytics_routes.py
backend/tests/test_event_capture_seams.py
backend/tests/test_quiz_context_repair.py

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

…_difficulty, stable upsert id, error-category admin feed Review findings (xhigh, 4 confirmed): - _coerce_summary guards list-shaped keys with isinstance(list) so legacy rows holding a string/dict there no longer explode into per-character bullets in the agent's prompt digest. - recommended_difficulty is surfaced in the digest ("Recommended next difficulty: …") — previously computed, encrypted, persisted, and never consumed anywhere. - save_quiz_context no longer sends a client-generated id: with merge-duplicates live again, the payload id rewrote the row's PRIMARY KEY on every refresh. Fresh inserts use the column's DB default; integration test now pins id stability across refreshes. - /api/admin/analytics/errors filters by category=error instead of the error.* name prefix, so quiz.context_write_failed (and #482's rag.* events) actually appear in the feed the B3 comments promised — non-HTTP rows null their payload fields, which ErrorEvent already models as Optional. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

…e events (#544) (#552) * feat(quiz): rate limit, daily spend guard, generation timeout, failure events (#544) Workstream F of the pre-revamp quiz repair batch (epic #537): F1 — generate is no longer an unbounded LLM call behind a button: a per-user sliding-window limit (8 per 5 minutes, sized for a human comparing difficulties) returns 429 QUIZ_RATE_LIMITED with Retry-After, and a daily per-user LLM spend ceiling ($2, read off the llm_usage ledger agents/usage.py already writes) returns 429 QUIZ_DAILY_LIMIT_REACHED before the model runs. Both guards sit AFTER the ownership check, so probing a stranger's concept can't consume their quota, and the spend check fails OPEN — a usage-table blip must not deny every student. F2 — the whole generation (agent run + tools + the E2 top-up) is bounded by QUIZ_GENERATION_TIMEOUT_SEC and maps to its own QUIZ_GENERATION_TIMEOUT code, so the client can say "that took too long" instead of the generic failure. F3 — quiz.generation_failed events (category=error, with a reason: timeout / agent_guardrail / agent_error) join the pinned taxonomy, so a 502 the student saw is a 502 an admin can count in the errors feed #548 widened. A throttled student is deliberately NOT an error event. F4 — deep-link scoping tests: a foreign concept 404s before the agent runs, and a student's OWN concept from a past semester still generates (scoping is by ownership, not active semester — pinned so a future "scope to active semester" change can't silently break revision). Also: the process-global rate-limit state now resets between tests via a conftest autouse fixture, same reasoning as the lru_cache reset — without it one test's burst throttles every later test hitting the same route. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(quiz): address #552 review — each guardrail was undercutting itself All three F guards failed at what they were added to do, each confirmed by execution in review: - The daily spend cap summed an UNPAGED llm_usage select. PostgREST caps a response at max_rows (1000) and answers 206 — a 2xx — so the sum plateaued and the ceiling could never trip for the runaway user it targets. It pages now (with an early exit once the cap is crossed), and db/connection.py::select gained the `offset` it needed. - Wrapping the whole generation in asyncio.wait_for raised CancelledError — a BaseException — straight past #543's serve-what-we-have handler, so a timed-out top-up threw away a valid partial quiz and returned 502. Each agent run is bounded individually now, so a top-up timeout is an ordinary TimeoutError the existing handler degrades from; a completed 3-question generation is served instead of discarded. - The rate-limit slot was claimed before generation and never refunded, so eight backend 502s locked a student out for five minutes with a message saying they'd generated too many quizzes — having received none. Every failure path now refunds the slot (services/request_limits.py::refund_rate_limit). Also from the review: - The timeout branch caught the builtin OSError-family TimeoutError as well, relabelling transport socket timeouts as wall-clock timeouts. It catches only asyncio.TimeoutError now. - The spend-cap comment claimed cross-feature enforcement it doesn't provide; it now says what's actually true (the spend measured is cross-feature, the ceiling is enforced on quiz generation only). - main.py forwards exc.headers as of this branch, which falsified the comments in routes/extract.py and routes/gradescope.py explaining why they couldn't send Retry-After. Both now send it. Known and accepted: a cancelled agent run never reaches record_agent_usage, so a timed-out generation's tokens don't land in llm_usage. Capturing usage from a cancelled pydantic-ai run isn't available at this seam; the per-run timeout narrows the window considerably versus cancelling the whole request. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>

7681c48) The audit behind the #537 addendum, written read-only in a worktree that has since been cleaned up — recovered and committed so it survives and so the YAML can be diffed as the product evolves. Its two code-only findings are tracked: the #529 42P10 (since fixed and additionally live-DB verified in #548) and the misconceptions offering-id mismatch (#553, which requires a live-DB check before any change). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Three findings, all about the schema-version half rather than the profile. **The version was LLM-controlled.** `schema_version` was a field on the digest agent's output_type, and `submit_quiz` persists `model_dump()` verbatim. Worse, this PR's own prompt feeds the previous digest back in under "update your notes" — so from now on the model would SEE `schema_version: 2` and be invited to update it. A model that helpfully bumped it to 3 would trip the reader's unknown-shape warning on every later read for that (user, concept), forever, with no real drift; one that lowered it would kill the guard just as quietly. The field is now off the agent schema entirely (so it costs no decoding budget either) and `save_quiz_context` stamps it server-side, which also covers every future writer. **The guard couldn't catch the drift it cited.** A version comparison only fires for a writer NEWER than the reader — a mixed-deploy window. #548 was a key RENAME at the same version, and catching that with a version number requires remembering to bump it in the same commit as the rename, which is precisely the discipline that failed the first time. The version now claims only what it can prove, and the rename case is caught by its OUTCOME: a row that is present, stamped with a version this reader understands, and yet yields nothing readable. A real digest cannot be all three at once — the agent always writes at least `questions_seen_summary` or `notes` — so that combination means the keys moved. Logged with the actual key set. **The docstring understated the blast radius.** It claimed this runs in the post-submit BackgroundTask. It does not: `submit_quiz` builds the whole prompt string synchronously and hands only the finished string to `add_task`. So an escaping exception 500s the submit AFTER the atomic `completed_at` claim, the mastery write and the score update have landed — the student sees a failed submit for a quiz that scored, and the retry 409s with the score never returned. The guard was right; the comment beside it invited a future reader to delete it as cheap background-task failure. The review also found a live instance of this same drift class in the OTHER reader of `context_json`: `course_context_service` harvests `effective_explanations`, a key no agent has ever written, so that column has persisted an empty array for every offering since it existed. Out of scope here, filed as #572. Hermetic 2184 passed / 9 skipped, ruff clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

) (#571) * feat(quiz): mine answers_json into the digest as a mistake profile (#554) `quiz_attempts.answers_json` has recorded which distractor a student picked, per question, on every submit since the column existed — and nothing has ever read it back. What the post-submit digest agent actually received was `results`, which carries LABELS: "question 3, picked B, the answer was C". That is not something a model can turn into a misconception; it can only guess one. The option TEXT — the thing that makes a wrong answer mean something — was one join away in `questions_json` the whole time. `services/quiz_distractors.py` does that join and hands the digest the wrong answers in words: the stem, the concept, what the student chose, and what was correct. Deliberately dumb — no model call, no I/O, pure data — and it never raises, because it runs in the post-submit BackgroundTask after the attempt is already graded and written, where a crash would cost the student the digest for a quiz they had already finished. Four cases it deliberately does NOT report, each of which would teach the digest something false: * correct answers — a mistake profile made of right answers spends tokens to report the absence of a problem; * unanswered questions — skipped is not wrong, and recording a blank as a distractor choice invents a misconception out of silence; * items with no correct option (#129's shape, which grade as wrong for everyone) — "the correct answer was <nothing>" is not a fact about the student; * results whose question is missing from the attempt. Digest schema version, the other half of the issue: `DIGEST_SCHEMA_VERSION` is stamped into `context_json` by the model's default rather than by the LLM (a field the model must remember to set is a field that goes missing), and the reader warns when it meets a version it doesn't understand. The drift this exists to catch is #548's: the coercer looked for `common_errors` while the agent wrote `common_mistakes`, and the symptom was an empty digest — indistinguishable from a new student. Tests pin the wiring as well as the logic, because `str.replace` on a renamed placeholder is a silent no-op: the profile would be computed, serialized, and dropped on the floor with nothing to show for it. Hermetic 2181 passed / 9 skipped, ruff clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(quiz): #554 review — the version was the model's to change Three findings, all about the schema-version half rather than the profile. **The version was LLM-controlled.** `schema_version` was a field on the digest agent's output_type, and `submit_quiz` persists `model_dump()` verbatim. Worse, this PR's own prompt feeds the previous digest back in under "update your notes" — so from now on the model would SEE `schema_version: 2` and be invited to update it. A model that helpfully bumped it to 3 would trip the reader's unknown-shape warning on every later read for that (user, concept), forever, with no real drift; one that lowered it would kill the guard just as quietly. The field is now off the agent schema entirely (so it costs no decoding budget either) and `save_quiz_context` stamps it server-side, which also covers every future writer. **The guard couldn't catch the drift it cited.** A version comparison only fires for a writer NEWER than the reader — a mixed-deploy window. #548 was a key RENAME at the same version, and catching that with a version number requires remembering to bump it in the same commit as the rename, which is precisely the discipline that failed the first time. The version now claims only what it can prove, and the rename case is caught by its OUTCOME: a row that is present, stamped with a version this reader understands, and yet yields nothing readable. A real digest cannot be all three at once — the agent always writes at least `questions_seen_summary` or `notes` — so that combination means the keys moved. Logged with the actual key set. **The docstring understated the blast radius.** It claimed this runs in the post-submit BackgroundTask. It does not: `submit_quiz` builds the whole prompt string synchronously and hands only the finished string to `add_task`. So an escaping exception 500s the submit AFTER the atomic `completed_at` claim, the mastery write and the score update have landed — the student sees a failed submit for a quiz that scored, and the retry 409s with the score never returned. The guard was right; the comment beside it invited a future reader to delete it as cheap background-task failure. The review also found a live instance of this same drift class in the OTHER reader of `context_json`: `course_context_service` harvests `effective_explanations`, a key no agent has ever written, so that column has persisted an empty array for every offering since it existed. Out of scope here, filed as #572. Hermetic 2184 passed / 9 skipped, ruff clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(quiz): the real-DB round-trip sees the stamped version too `test_save_quiz_context_upserts_one_row_and_encrypts` asserts the payload round-trips exactly, so server-stamping `schema_version` in `save_quiz_context` changed what comes back. The hermetic twin was updated with the fix; this is its real-DB counterpart, and only the integration lane could see it — which is the whole argument for that lane. Integration 56 passed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>

AndresL230 · 2026-08-12T21:43:14Z

Closes#529. Workstream B of the pre-revamp quiz repair batch (epic #537) — the highest-value fix in the batch: the difference between "adaptive" being a label and being true.

Forensics (verified live)

0001 created quiz_context with UNIQUE (user_id, concept_node_id); 0025_study_integrity.sql:108-114 recreated the table without it (dropped 2026-06-23, 72443e96).
services/quiz_context_service.py:37 upserts with on_conflict="user_id,concept_node_id" → PostgREST 42P10 on every write since; routes/quiz.py swallowed it (except Exception: pass).
Staging measured 2026-08-12: 0 rows, 0 duplicate pairs — the very first write already failed, so nothing ever accumulated. pg_constraint confirmed only the PK + two FKs.

What

B1 — migration 20260812210033 restores the UNIQUE (named quiz_context_user_concept_key), dedup-guarded (keep-newest, RAISE NOTICE reports removals) and idempotent. Applying it is backward-compatible; run db.migrate before/with the deploy.

B2 — the upsert's on_conflict target now has a matching constraint; the pairing is pinned by test.

B3 — the post-submit background write is loud on failure: logger.exception with attempt id + request id, a quiz.context_write_failed analytics event (category="error", added to the pinned EVENT_TAXONOMY) so it lands in admin analytics like error.4xx, and a re-raise under config.IS_LOCAL so this regression class fails CI instead of going quiet for months. Deliberate carve-out: the E2E seam's UnregisteredHandlerError (quiz_context stays unregistered in function mode by design) is a single WARNING with no traceback — the logscan oracle treats tracebacks as findings, and that fail-fast is the seam working.

B4 — the read side now consumes the wholeQuizContext shape. _coerce_summary previously returned notes alone and silently dropped weak_areas / common_mistakes / questions_seen_summary; its list fallback looked for common_errors, a key QuizContext never writes. A tool-level round-trip test covers ciphertext row → decrypt → digest in QuizHistory.summary.

B5 — real-DB integration tests (tests/integration/test_quiz_context_repair_db.py, #397 seam: app writes, raw psycopg reads): UNIQUE present with exactly the upsert's columns, double-upsert keeps one row, raw context_json is ciphertext, app read round-trips. No MagicMock on this path — the hermetic suite's mocks are precisely why this bug survived ~7.5 weeks.

Verification

Hermetic suite: 1926 passed (7 new tests, written first, watched fail).
Full local cycle under the stack lock: Playwright 43 passed → oracles exit 0 → integration lane 42 passed / 0 skipped (counts read, not exit codes) → teardown.
ruff check clean.
Staging migration applied ahead of merge (constraint restore is backward-compatible).

🤖 Generated with Claude Code

…allowing, consume the full digest (#529) Workstream B of the pre-revamp quiz repair batch (epic #537): B1 — migration 20260812210033 restores UNIQUE (user_id, concept_node_id) on quiz_context (dropped by 0025's table recreate), dedup-guarded and idempotent. Staging/prod measured 0 rows + 0 dupes — the very first upsert already 42P10'd, so nothing ever accumulated. B2 — save_quiz_context's on_conflict target now has a matching constraint again; pinned by test. B3 — the post-submit background write is loud on failure: ERROR log with attempt id + request id, a quiz.context_write_failed analytics event (added to the pinned EVENT_TAXONOMY), and a re-raise under config.IS_LOCAL so this regression class fails CI. The E2E seam's UnregisteredHandlerError stays a single WARNING with no traceback — quiz_context is deliberately unregistered in function mode and the logscan oracle treats tracebacks as findings. B4 — the read side now consumes the WHOLE QuizContext shape: _coerce_summary previously returned `notes` alone and dropped weak_areas / common_mistakes / questions_seen_summary (its list fallback also looked for `common_errors`, a key QuizContext never writes). Tool-level round-trip test covers ciphertext row → decrypt → digest in QuizHistory.summary. B5 — real-DB integration tests (tests/integration/ test_quiz_context_repair_db.py): constraint present, double-upsert keeps one row, raw column is ciphertext, app read round-trips. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

cloudflare-workers-and-pages · 2026-08-12T21:43:21Z

Deploying with Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status	Name	Latest Commit	Preview URL	Updated (UTC)
✅ Deployment successful! View logs	frontend-staging	`bdc9552`	Commit Preview URL Branch Preview URL	Aug 13 2026, 01:32 AM

supabase · 2026-08-12T21:43:22Z

This pull request has been ignored for the connected project ybgqdonkoqftwrmweuyv because there are no changes detected in supabase directory. You can change this behaviour in Project Integrations Settings ↗︎.

Preview Branches by Supabase.
Learn more about Supabase Branching ↗︎.

coderabbitai · 2026-08-12T21:43:25Z

Warning

Review limit reached

@AndresL230, you've reached your PR review limit, so we couldn't start this review.

Next review available in:94 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 @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 28afc089-32e5-401b-8776-c582420f41df

📥 Commits

Reviewing files that changed from the base of the PR and between 6081ec5 and bdc9552.

📒 Files selected for processing (10)

backend/agents/tools/quiz_history.py
backend/db/migrations/20260812210033_restore_quiz_context_unique.sql
backend/routes/admin_analytics.py
backend/routes/quiz.py
backend/services/events_service.py
backend/services/quiz_context_service.py
backend/tests/integration/test_quiz_context_repair_db.py
backend/tests/test_admin_analytics_routes.py
backend/tests/test_event_capture_seams.py
backend/tests/test_quiz_context_repair.py

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

…_difficulty, stable upsert id, error-category admin feed Review findings (xhigh, 4 confirmed): - _coerce_summary guards list-shaped keys with isinstance(list) so legacy rows holding a string/dict there no longer explode into per-character bullets in the agent's prompt digest. - recommended_difficulty is surfaced in the digest ("Recommended next difficulty: …") — previously computed, encrypted, persisted, and never consumed anywhere. - save_quiz_context no longer sends a client-generated id: with merge-duplicates live again, the payload id rewrote the row's PRIMARY KEY on every refresh. Fresh inserts use the column's DB default; integration test now pins id stability across refreshes. - /api/admin/analytics/errors filters by category=error instead of the error.* name prefix, so quiz.context_write_failed (and #482's rag.* events) actually appear in the feed the B3 comments promised — non-HTTP rows null their payload fields, which ErrorEvent already models as Optional. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

…e events (#544) (#552) * feat(quiz): rate limit, daily spend guard, generation timeout, failure events (#544) Workstream F of the pre-revamp quiz repair batch (epic #537): F1 — generate is no longer an unbounded LLM call behind a button: a per-user sliding-window limit (8 per 5 minutes, sized for a human comparing difficulties) returns 429 QUIZ_RATE_LIMITED with Retry-After, and a daily per-user LLM spend ceiling ($2, read off the llm_usage ledger agents/usage.py already writes) returns 429 QUIZ_DAILY_LIMIT_REACHED before the model runs. Both guards sit AFTER the ownership check, so probing a stranger's concept can't consume their quota, and the spend check fails OPEN — a usage-table blip must not deny every student. F2 — the whole generation (agent run + tools + the E2 top-up) is bounded by QUIZ_GENERATION_TIMEOUT_SEC and maps to its own QUIZ_GENERATION_TIMEOUT code, so the client can say "that took too long" instead of the generic failure. F3 — quiz.generation_failed events (category=error, with a reason: timeout / agent_guardrail / agent_error) join the pinned taxonomy, so a 502 the student saw is a 502 an admin can count in the errors feed #548 widened. A throttled student is deliberately NOT an error event. F4 — deep-link scoping tests: a foreign concept 404s before the agent runs, and a student's OWN concept from a past semester still generates (scoping is by ownership, not active semester — pinned so a future "scope to active semester" change can't silently break revision). Also: the process-global rate-limit state now resets between tests via a conftest autouse fixture, same reasoning as the lru_cache reset — without it one test's burst throttles every later test hitting the same route. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(quiz): address #552 review — each guardrail was undercutting itself All three F guards failed at what they were added to do, each confirmed by execution in review: - The daily spend cap summed an UNPAGED llm_usage select. PostgREST caps a response at max_rows (1000) and answers 206 — a 2xx — so the sum plateaued and the ceiling could never trip for the runaway user it targets. It pages now (with an early exit once the cap is crossed), and db/connection.py::select gained the `offset` it needed. - Wrapping the whole generation in asyncio.wait_for raised CancelledError — a BaseException — straight past #543's serve-what-we-have handler, so a timed-out top-up threw away a valid partial quiz and returned 502. Each agent run is bounded individually now, so a top-up timeout is an ordinary TimeoutError the existing handler degrades from; a completed 3-question generation is served instead of discarded. - The rate-limit slot was claimed before generation and never refunded, so eight backend 502s locked a student out for five minutes with a message saying they'd generated too many quizzes — having received none. Every failure path now refunds the slot (services/request_limits.py::refund_rate_limit). Also from the review: - The timeout branch caught the builtin OSError-family TimeoutError as well, relabelling transport socket timeouts as wall-clock timeouts. It catches only asyncio.TimeoutError now. - The spend-cap comment claimed cross-feature enforcement it doesn't provide; it now says what's actually true (the spend measured is cross-feature, the ceiling is enforced on quiz generation only). - main.py forwards exc.headers as of this branch, which falsified the comments in routes/extract.py and routes/gradescope.py explaining why they couldn't send Retry-After. Both now send it. Known and accepted: a cancelled agent run never reaches record_agent_usage, so a timed-out generation's tokens don't land in llm_usage. Capturing usage from a cancelled pydantic-ai run isn't available at this seam; the per-run timeout narrows the window considerably versus cancelling the whole request. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>

7681c48) The audit behind the #537 addendum, written read-only in a worktree that has since been cleaned up — recovered and committed so it survives and so the YAML can be diffed as the product evolves. Its two code-only findings are tracked: the #529 42P10 (since fixed and additionally live-DB verified in #548) and the misconceptions offering-id mismatch (#553, which requires a live-DB check before any change). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Three findings, all about the schema-version half rather than the profile. **The version was LLM-controlled.** `schema_version` was a field on the digest agent's output_type, and `submit_quiz` persists `model_dump()` verbatim. Worse, this PR's own prompt feeds the previous digest back in under "update your notes" — so from now on the model would SEE `schema_version: 2` and be invited to update it. A model that helpfully bumped it to 3 would trip the reader's unknown-shape warning on every later read for that (user, concept), forever, with no real drift; one that lowered it would kill the guard just as quietly. The field is now off the agent schema entirely (so it costs no decoding budget either) and `save_quiz_context` stamps it server-side, which also covers every future writer. **The guard couldn't catch the drift it cited.** A version comparison only fires for a writer NEWER than the reader — a mixed-deploy window. #548 was a key RENAME at the same version, and catching that with a version number requires remembering to bump it in the same commit as the rename, which is precisely the discipline that failed the first time. The version now claims only what it can prove, and the rename case is caught by its OUTCOME: a row that is present, stamped with a version this reader understands, and yet yields nothing readable. A real digest cannot be all three at once — the agent always writes at least `questions_seen_summary` or `notes` — so that combination means the keys moved. Logged with the actual key set. **The docstring understated the blast radius.** It claimed this runs in the post-submit BackgroundTask. It does not: `submit_quiz` builds the whole prompt string synchronously and hands only the finished string to `add_task`. So an escaping exception 500s the submit AFTER the atomic `completed_at` claim, the mastery write and the score update have landed — the student sees a failed submit for a quiz that scored, and the retry 409s with the score never returned. The guard was right; the comment beside it invited a future reader to delete it as cheap background-task failure. The review also found a live instance of this same drift class in the OTHER reader of `context_json`: `course_context_service` harvests `effective_explanations`, a key no agent has ever written, so that column has persisted an empty array for every offering since it existed. Out of scope here, filed as #572. Hermetic 2184 passed / 9 skipped, ruff clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

) (#571) * feat(quiz): mine answers_json into the digest as a mistake profile (#554) `quiz_attempts.answers_json` has recorded which distractor a student picked, per question, on every submit since the column existed — and nothing has ever read it back. What the post-submit digest agent actually received was `results`, which carries LABELS: "question 3, picked B, the answer was C". That is not something a model can turn into a misconception; it can only guess one. The option TEXT — the thing that makes a wrong answer mean something — was one join away in `questions_json` the whole time. `services/quiz_distractors.py` does that join and hands the digest the wrong answers in words: the stem, the concept, what the student chose, and what was correct. Deliberately dumb — no model call, no I/O, pure data — and it never raises, because it runs in the post-submit BackgroundTask after the attempt is already graded and written, where a crash would cost the student the digest for a quiz they had already finished. Four cases it deliberately does NOT report, each of which would teach the digest something false: * correct answers — a mistake profile made of right answers spends tokens to report the absence of a problem; * unanswered questions — skipped is not wrong, and recording a blank as a distractor choice invents a misconception out of silence; * items with no correct option (#129's shape, which grade as wrong for everyone) — "the correct answer was <nothing>" is not a fact about the student; * results whose question is missing from the attempt. Digest schema version, the other half of the issue: `DIGEST_SCHEMA_VERSION` is stamped into `context_json` by the model's default rather than by the LLM (a field the model must remember to set is a field that goes missing), and the reader warns when it meets a version it doesn't understand. The drift this exists to catch is #548's: the coercer looked for `common_errors` while the agent wrote `common_mistakes`, and the symptom was an empty digest — indistinguishable from a new student. Tests pin the wiring as well as the logic, because `str.replace` on a renamed placeholder is a silent no-op: the profile would be computed, serialized, and dropped on the floor with nothing to show for it. Hermetic 2181 passed / 9 skipped, ruff clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(quiz): #554 review — the version was the model's to change Three findings, all about the schema-version half rather than the profile. **The version was LLM-controlled.** `schema_version` was a field on the digest agent's output_type, and `submit_quiz` persists `model_dump()` verbatim. Worse, this PR's own prompt feeds the previous digest back in under "update your notes" — so from now on the model would SEE `schema_version: 2` and be invited to update it. A model that helpfully bumped it to 3 would trip the reader's unknown-shape warning on every later read for that (user, concept), forever, with no real drift; one that lowered it would kill the guard just as quietly. The field is now off the agent schema entirely (so it costs no decoding budget either) and `save_quiz_context` stamps it server-side, which also covers every future writer. **The guard couldn't catch the drift it cited.** A version comparison only fires for a writer NEWER than the reader — a mixed-deploy window. #548 was a key RENAME at the same version, and catching that with a version number requires remembering to bump it in the same commit as the rename, which is precisely the discipline that failed the first time. The version now claims only what it can prove, and the rename case is caught by its OUTCOME: a row that is present, stamped with a version this reader understands, and yet yields nothing readable. A real digest cannot be all three at once — the agent always writes at least `questions_seen_summary` or `notes` — so that combination means the keys moved. Logged with the actual key set. **The docstring understated the blast radius.** It claimed this runs in the post-submit BackgroundTask. It does not: `submit_quiz` builds the whole prompt string synchronously and hands only the finished string to `add_task`. So an escaping exception 500s the submit AFTER the atomic `completed_at` claim, the mastery write and the score update have landed — the student sees a failed submit for a quiz that scored, and the retry 409s with the score never returned. The guard was right; the comment beside it invited a future reader to delete it as cheap background-task failure. The review also found a live instance of this same drift class in the OTHER reader of `context_json`: `course_context_service` harvests `effective_explanations`, a key no agent has ever written, so that column has persisted an empty array for every offering since it existed. Out of scope here, filed as #572. Hermetic 2184 passed / 9 skipped, ruff clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(quiz): the real-DB round-trip sees the stamped version too `test_save_quiz_context_upserts_one_row_and_encrypts` asserts the payload round-trips exactly, so server-stamping `schema_version` in `save_quiz_context` changed what comes back. The hermetic twin was updated with the fix; this is its real-DB counterpart, and only the integration lane could see it — which is the whole argument for that lane. Integration 56 passed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>

AndresL230 merged commit 5e1a322 into mainAug 13, 2026
5 of 8 checks passed

This was referenced Aug 13, 2026
feat(quiz): rate limit, daily spend guard, generation timeout, failure events (#544) #552
Merged
quiz H2: mine answers_json into the digest (distractor profile) + digest schema version #554
Closed

This was referenced Aug 22, 2026
feat(quiz): mine answers_json into the digest as a mistake profile (#554) #571
Merged
quiz: offering_concept_stats.effective_explanations is a dead read — no agent writes that key #572
Closed

This was referenced Aug 26, 2026
chore(quiz): delete the dead effective_explanations read (#572) #587
Merged
course_context rollup: opt-out purge gap, silent-empty overwrite, unguarded legacy shapes, N+1 reads/writes #595
Open