Uh oh!
There was an error while loading. Please reload this page.
feat(quiz): rate limit, daily spend guard, generation timeout, failure events (#544) - #552
Conversation
…e 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>
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (12)
📝 WalkthroughWalkthroughQuiz generation now enforces per-user rate and spend limits, bounds agent runs, refunds failed attempts, and records failure events. HTTP exception handling preserves response headers, rate-limit routes expose ChangesQuiz generation controls
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant QuizRoute as quiz.generate route
participant Limits as request_limits
participant Usage as SupabaseTable
participant Agent as quiz agent
participant Events as events_service
Client->>QuizRoute: Submit quiz generation request
QuizRoute->>Limits: Claim per-user rate-limit slot
QuizRoute->>Usage: Read paginated daily LLM usage
Usage-->>QuizRoute: Return usage rows
QuizRoute->>Agent: Run generation with timeout
Agent-->>QuizRoute: Return questions or failure
QuizRoute->>Limits: Refund slot after generation failure
QuizRoute->>Events: Record quiz.generation_failed
QuizRoute-->>Client: Return quiz or structured error response
Possibly related PRs
Suggested reviewers: ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This pull request has been ignored for the connected project Preview Branches by Supabase. |
Deploying with |
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs | frontend-staging | 1485478 | Commit Preview URL Branch Preview URL | Aug 13 2026, 06:18 AM |
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>
Uh oh!
There was an error while loading. Please reload this page.
Closes#544. Workstream F of the pre-revamp quiz repair batch (epic #537) — the last one.
What
F1 — generation is no longer an unbounded LLM call behind a button.
429 QUIZ_RATE_LIMITEDwith aRetry-Afterheader, reusingservices/request_limits.py.llm_usageledgeragents/usage.py::record_agent_usagealready writes) returns429 QUIZ_DAILY_LIMIT_REACHEDbefore the model runs. A default-tier generation costs well under a cent, so this bounds a runaway rather than rationing normal use.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 — explicit timeout. The whole generation (agent run, its tool calls, and E2's top-up) is bounded by
QUIZ_GENERATION_TIMEOUT_SECand maps to its ownQUIZ_GENERATION_TIMEOUTcode, so the client can say "that took too long" rather than the generic failure.F3 — failures reach admin analytics.
quiz.generation_failed(categoryerror, withreason:timeout/agent_guardrail/agent_error) joins the pinned taxonomy, so a 502 the student saw is a 502 an admin can count in the errors feed #548 widened to category-based filtering. A throttled student is deliberately not an error event.F4 — deep-link scoping tests. A foreign concept 404s before the agent runs; 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 without a deliberate decision.
Also
The process-global rate-limit state now resets between tests via a conftest autouse fixture, same reasoning as the existing
_clear_lru_caches. Without it one test's burst throttles every later test hitting the same route — which is exactly what happened when the limit first landed (24 unrelated failures).Verification
ruff checkclean. No migration in this workstream.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes