Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions backend/db/connection.py
Original file line numberDiff line numberDiff line change
Expand Up@@ -34,14 +34,21 @@ def select(
filters: Optional[dict] = None,
order: Optional[str] = None,
limit: Optional[int] = None,
offset: Optional[int] = None,
) -> list:
"""Read rows. Pass `limit`/`offset` to page — PostgREST caps a
response at `max_rows` (1000) and answers 206 Partial Content,
which is a 2xx, so an unpaged read over that many rows truncates
silently."""
params: dict = {"select": columns}
if filters:
params.update(filters)
if order:
params["order"] = order
if limit:
params["limit"] = str(limit)
if offset is not None:
params["offset"] = str(offset)
r = _client.get(self.url, params=params)
r.raise_for_status()
return r.json()
Expand Down
8 changes: 7 additions & 1 deletion backend/main.py
Original file line numberDiff line numberDiff line change
Expand Up@@ -201,10 +201,16 @@ async def http_exception_handler(request: Request, exc: StarletteHTTPException):
code=getattr(exc, "code", None),
machine_detail=getattr(exc, "machine_detail", None),
)
# Preserve headers the raise site set (e.g. Retry-After on a 429) —
# dropping them would strip the only machine-readable part of a
# throttling response.
headers = dict(getattr(exc, "headers", None) or {})
if rid:
headers["X-Request-ID"] = rid
return JSONResponse(
status_code=exc.status_code,
content=content,
headers={"X-Request-ID": rid} if rid else {},
headers=headers,
)


Expand Down
7 changes: 4 additions & 3 deletions backend/routes/extract.py
Original file line numberDiff line numberDiff line change
Expand Up@@ -30,12 +30,13 @@ def _enforce_ocr_limits(request: Request) -> str:
user_id = get_session_user_id(request)
retry = check_rate_limit(f"ocr:{user_id}", limit=_OCR_RATE_LIMIT, window_sec=_OCR_RATE_WINDOW)
if retry is not None:
# NB: the app's global HTTPException handler (main.py) doesn't forward
# exc.headers, so the retry budget is conveyed in the detail string
# rather than a Retry-After header.
# main.py's HTTPException handler forwards exc.headers as of #544, so
# the budget rides in a real Retry-After. It stays in the detail
# string too — clients that only surface the message keep working.
raise HTTPException(
status_code=429,
detail=f"Too many OCR requests. Retry in {retry}s.",
headers={"Retry-After": str(retry)},
)
return user_id

Expand Down
6 changes: 4 additions & 2 deletions backend/routes/gradescope.py
Original file line numberDiff line numberDiff line change
Expand Up@@ -202,11 +202,13 @@ def _enforce_gs_rate_limit(user_id: str, action: str, *, limit: int, window_sec:
f"gradescope:{action}:{user_id}", limit=limit, window_sec=window_sec
)
if retry is not None:
# main.py's HTTPException handler drops exc.headers, so the retry budget
# rides in the detail string rather than a Retry-After header.
# main.py's HTTPException handler forwards exc.headers as of #544, so
# the budget rides in a real Retry-After. It stays in the detail
# string too — clients that only surface the message keep working.
raise HTTPException(
status_code=429,
detail=f"Too many Gradescope {action} requests. Retry in {retry}s.",
headers={"Retry-After": str(retry)},
)


Expand Down
151 changes: 149 additions & 2 deletions backend/routes/quiz.py
Original file line numberDiff line numberDiff line change
Expand Up@@ -25,12 +25,17 @@
from services.quiz_config import (
CONCRETE_DIFFICULTIES,
QUIZ_ATTEMPT_ABANDON_TTL_HOURS,
QUIZ_DAILY_SPEND_CAP_USD,
QUIZ_GENERATE_RATE_LIMIT,
QUIZ_GENERATE_RATE_WINDOW_SEC,
QUIZ_GENERATION_TIMEOUT_SEC,
QUIZ_TOPUP_DROP_RATIO,
QUIZ_TOPUP_MAX_RETRIES,
REQUESTED_DIFFICULTIES,
mastery_after,
quiz_config_payload,
)
from services.request_limits import check_rate_limit, refund_rate_limit
from services.quiz_errors import QuizAPIError, QuizErrorCode
from services.profiles import get_display_name
from services.encryption import encrypt_json, decrypt_json_column
Expand DownExpand Up@@ -87,6 +92,80 @@ def _load_prompt(name: str) -> str:
_MAX_HISTORY_OFFSET = 1_000_000


# supabase/config.toml sets PostgREST's max_rows = 1000, and an over-cap
# response is 206 Partial Content — a 2xx, so raise_for_status never fires
# and the truncation is silent. Same constant and same reasoning as
# achievement_service._daily_totals; page to completion or the sum is a lie.
_USAGE_PAGE = 1000


def _daily_spend_exceeded(user_id: str) -> bool:
"""True if this user is past the daily LLM spend ceiling (#544 F1).

Reads the llm_usage ledger agents/usage.py already writes, PAGED: an
unpaged read stops at max_rows, so a heavy user's sum plateaus below
the cap and the guard never trips for exactly the runaway it targets.
Stops early once the ceiling is crossed — the common case is a couple
of rows, and a user past the cap doesn't need an exact total.

Fails OPEN on any error: this is a cost control, not a correctness
gate, and denying every student because a usage read blipped is worse
than the spend it would save.
"""
try:
since = (datetime.now(timezone.utc) - timedelta(days=1)).isoformat()
spent = 0.0
offset = 0
while True:
rows = table("llm_usage").select(
"cost_usd",
filters={"user_id": f"eq.{user_id}", "created_at": f"gte.{since}"},
limit=_USAGE_PAGE,
offset=offset,
) or []
spent += sum(float(r.get("cost_usd") or 0.0) for r in rows)
if spent >= QUIZ_DAILY_SPEND_CAP_USD:
return True
if len(rows) < _USAGE_PAGE:
return False
offset += _USAGE_PAGE
except Exception:
logger.exception("quiz: daily spend check failed user=%s; allowing", user_id)
return False


def _refund_generate_slot(user_id: str) -> None:
"""Hand back the rate-limit slot a failed generation consumed (#544 F1).

The slot is claimed BEFORE the model runs (so a burst can't get past
the gate concurrently), which means a backend failure would otherwise
spend the student's quota: eight 502s in two minutes would lock them
out for five with a message saying they'd generated too many quizzes,
having received none. A failure the student didn't cause shouldn't
cost them anything, and the 502 explicitly invites a retry.
"""
try:
refund_rate_limit(f"quiz_generate:{user_id}")
except Exception:
logger.exception("quiz: rate-limit refund failed user=%s", user_id)


def _log_generation_failed(body, request_id: str | None, reason: str) -> None:
"""#544 F3: make a 502 the student saw a 502 an admin can count."""
events_service.log_event(
"quiz.generation_failed",
category="error",
user_id=body.user_id,
request_id=request_id,
payload={
"concept_node_id": body.concept_node_id,
"difficulty": body.difficulty,
"num_questions": body.num_questions,
"reason": reason,
},
)


def _abandon_cutoff() -> datetime:
return datetime.now(timezone.utc) - timedelta(
hours=QUIZ_ATTEMPT_ABANDON_TTL_HOURS
Expand DownExpand Up@@ -510,8 +589,17 @@ async def _quiz_via_agent(
run_kwargs["model"] = model_override

async def _run(message: str, limits) -> Quiz:
# #544 F2: bound EACH agent run rather than the whole function.
# Wrapping the outer coroutine cancelled it mid-flight, and
# CancelledError is a BaseException — it flew straight past the
# top-up's serve-what-we-have handler and threw away questions the
# student had already paid for. Timing out one run raises an
# ordinary TimeoutError the existing handlers can reason about.
result = record_agent_usage(
await quiz_agent.run(message, usage_limits=limits, **run_kwargs),
await asyncio.wait_for(
quiz_agent.run(message, usage_limits=limits, **run_kwargs),
timeout=QUIZ_GENERATION_TIMEOUT_SEC,
),
feature="quiz", task="quiz", user_id=deps.user_id,
)
return result.output
Expand DownExpand Up@@ -582,7 +670,7 @@ def _absorb(quiz: Quiz) -> None:
)
try:
_absorb(await _run(topup_msg, TOPUP_LIMITS))
except Exception as e:
except (Exception, asyncio.TimeoutError) as e:
# The request deliberately SUCCEEDS from here — serve the
# short quiz with an honest count. No traceback: the E2E
# logscan oracle reports those as findings, and this path
Expand DownExpand Up@@ -652,7 +740,44 @@ async def generate_quiz(body: GenerateQuizBody, request: Request):
or str(uuid.uuid4())
)

# #544 F1: cost guards run AFTER ownership (a stranger's node 404s
# first, so probing can't consume a victim's quota) and BEFORE the
# model call. Neither rejection is a backend failure, so neither emits
# quiz.generation_failed.
retry_after = check_rate_limit(
f"quiz_generate:{body.user_id}",
limit=QUIZ_GENERATE_RATE_LIMIT,
window_sec=QUIZ_GENERATE_RATE_WINDOW_SEC,
)
if retry_after is not None:
raise QuizAPIError(
status_code=429,
code=QuizErrorCode.QUIZ_RATE_LIMITED,
message=(
"You've generated a lot of quizzes just now — "
"take a moment and try again shortly."
),
headers={"Retry-After": str(retry_after)},
)
if _daily_spend_exceeded(body.user_id):
logger.warning(
"quiz: daily spend cap reached user=%s request_id=%s",
body.user_id, request_id,
)
raise QuizAPIError(
status_code=429,
code=QuizErrorCode.QUIZ_DAILY_LIMIT_REACHED,
message=(
"You've reached today's limit for AI-generated study "
"material. It resets tomorrow."
),
)

try:
# Each agent run inside is individually bounded by
# QUIZ_GENERATION_TIMEOUT_SEC (see _run) — cancelling the whole
# coroutine here would discard a partial quiz the top-up handler
# is designed to serve.
questions = await _quiz_via_agent(
user_id=body.user_id,
course_id=course_id,
Expand All@@ -667,18 +792,40 @@ async def generate_quiz(body: GenerateQuizBody, request: Request):
except HTTPException:
# The 404 for an unknown concept node is raised before the agent call;
# never swallow a known HTTP state.
_refund_generate_slot(body.user_id)
raise
except asyncio.TimeoutError as e:
# #544 F2: distinct from a generic failure — the client can say
# "that took too long" and offering a retry obviously makes sense.
# NB: only asyncio.TimeoutError. The builtin TimeoutError is in the
# OSError family, so catching it too would relabel a transport
# socket timeout as a wall-clock generation timeout.
logger.warning(
"quiz: generation timed out after %ss request_id=%s",
QUIZ_GENERATION_TIMEOUT_SEC, request_id,
)
_refund_generate_slot(body.user_id)
_log_generation_failed(body, request_id, "timeout")
raise QuizAPIError(
status_code=502,
code=QuizErrorCode.QUIZ_GENERATION_TIMEOUT,
message="Quiz generation took too long. Please try again.",
) from e
except (UsageLimitExceeded, UnexpectedModelBehavior) as e:
# The raw-Gemini legacy fallback was retired in #145; degrade to 502
# rather than serving a quiz from a second LLM path.
logger.warning("Quiz agent guardrails tripped; returning 502", exc_info=e)
_refund_generate_slot(body.user_id)
_log_generation_failed(body, request_id, "agent_guardrail")
raise QuizAPIError(
status_code=502,
code=QuizErrorCode.QUIZ_GENERATION_FAILED,
message="Quiz generation is temporarily unavailable. Please try again.",
) from e
except Exception as e:
logger.exception("Unexpected quiz-agent failure; returning 502")
_refund_generate_slot(body.user_id)
_log_generation_failed(body, request_id, "agent_error")
raise QuizAPIError(
status_code=502,
code=QuizErrorCode.QUIZ_GENERATION_FAILED,
Expand Down
4 changes: 4 additions & 0 deletions backend/services/events_service.py
Original file line numberDiff line numberDiff line change
Expand Up@@ -86,6 +86,10 @@
# surfaces in admin analytics — this failure was invisible for months
# precisely because nothing emitted when the background task died.
"quiz.context_write_failed",
# #544/F3: generation failed (agent error, timeout, or every question
# dropped). Same reasoning: a 502 the student sees should be a 502 an
# admin can count.
"quiz.generation_failed",
"chat.message_sent",
"note.created",
"session.started",
Expand Down
28 changes: 28 additions & 0 deletions backend/services/quiz_config.py
Original file line numberDiff line numberDiff line change
Expand Up@@ -79,6 +79,34 @@ def mastery_after(before: float, *, score: int, total: int) -> float:
return max(0.0, min(1.0, raw))


# ── Cost + abuse guards (#544 F1/F2) ────────────────────────────────────────
#
# Generation is an unbounded LLM call behind a button: before #544 nothing
# stopped a held-down key or a scripted loop from spending real money.
#
# The rate limit is sized for a human: a student comparing difficulties or
# retaking a concept might legitimately generate a handful of quizzes in a
# few minutes; nobody legitimately generates 10 in one.
QUIZ_GENERATE_RATE_LIMIT = 8
QUIZ_GENERATE_RATE_WINDOW_SEC = 300 # 5 minutes

# Daily per-user LLM spend ceiling. The SPEND it measures is cross-feature
# (llm_usage records every agent call, not just quiz ones), but the ceiling
# is only ENFORCED on quiz generation — the one unbounded LLM call behind a
# button. Other entry points stay unguarded for now; moving this into a
# shared guard is its own piece of work, not something to imply here.
# A generation on the default flash-lite tier costs well under a cent, so
# this is ~2 orders of magnitude above any real study day — it exists to
# bound a runaway, not to ration normal use. Deliberately fail-OPEN: if the
# usage read errors we serve the quiz rather than denying every student.
QUIZ_DAILY_SPEND_CAP_USD = 2.00

# Wall-clock ceiling on one generation (agent run incl. its tool calls).
# Past this the student is staring at a spinner and would rather be told to
# try again; the request also stops holding a worker slot.
QUIZ_GENERATION_TIMEOUT_SEC = 90


# ── Generation honesty (#543 E2) ────────────────────────────────────────────
#
# Questions whose correct_answer doesn't match an option verbatim are
Expand Down
11 changes: 10 additions & 1 deletion backend/services/quiz_errors.py
Original file line numberDiff line numberDiff line change
Expand Up@@ -46,6 +46,14 @@ class QuizErrorCode(str, Enum):
QUIZ_QUESTION_INVALID = "QUIZ_QUESTION_INVALID"
QUIZ_NOT_AUTHORIZED = "QUIZ_NOT_AUTHORIZED"
QUIZ_GENERATION_FAILED = "QUIZ_GENERATION_FAILED"
# #544 F2: the generation exceeded its wall-clock budget. Distinct from
# the generic failure so the client can say "that took too long" and a
# retry is obviously worth offering.
QUIZ_GENERATION_TIMEOUT = "QUIZ_GENERATION_TIMEOUT"
# #544 F1: too many generations in the rate window.
QUIZ_RATE_LIMITED = "QUIZ_RATE_LIMITED"
# #544 F1: this account's daily LLM spend ceiling is reached.
QUIZ_DAILY_LIMIT_REACHED = "QUIZ_DAILY_LIMIT_REACHED"
QUIZ_INTERNAL_ERROR = "QUIZ_INTERNAL_ERROR"
# Uncoded HTTP errors that aren't one of the semantic states above —
# router 404s/405s on version-skewed clients, library-raised
Expand DownExpand Up@@ -80,8 +88,9 @@ def __init__(
code: QuizErrorCode,
message: str,
machine_detail=None,
headers: dict[str, str] | None = None,
):
super().__init__(status_code=status_code, detail=message)
super().__init__(status_code=status_code, detail=message, headers=headers)
self.code = code
self.machine_detail = machine_detail

Expand Down
14 changes: 14 additions & 0 deletions backend/services/request_limits.py
Original file line numberDiff line numberDiff line change
Expand Up@@ -37,6 +37,20 @@ def check_rate_limit(key: str, *, limit: int, window_sec: int) -> int | None:
return None


def refund_rate_limit(key: str) -> None:
"""Give back the most recent slot recorded for `key`.

For guards that must claim BEFORE doing the expensive work (so a
concurrent burst can't slip past the gate) but shouldn't charge the
caller when that work fails for reasons they didn't cause. Dropping
the newest timestamp — not the oldest — keeps the window's start
anchored to the caller's earliest real attempt.
"""
bucket = _rate_state.get(key)
if bucket:
bucket.pop()


async def read_within_limit(upload: UploadFile, max_bytes: int) -> bytes:
"""Read at most ``max_bytes`` (+1 to detect overflow) from an UploadFile so
an oversize upload can't be pulled fully into memory before we reject it.
Expand Down
13 changes: 13 additions & 0 deletions backend/tests/conftest.py
Original file line numberDiff line numberDiff line change
Expand Up@@ -47,6 +47,19 @@ def pytest_configure(config):
)


@pytest.fixture(autouse=True)
def _clear_rate_limit_state():
"""#544: services/request_limits keeps its sliding windows in a
process-global dict, so one test's burst of requests would throttle
every later test that hits the same route as the same user. Same
reasoning as the lru_cache reset below."""
from services import request_limits

request_limits._rate_state.clear()
yield
request_limits._rate_state.clear()


@pytest.fixture(autouse=True)
def _clear_lru_caches():
"""#98: reset the per-process lru_caches around every test so one test's
Expand Down
Loading
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { // Add copy buttons to all
 blocks
(function() {
function addCopyButtons() {
document.querySelectorAll('pre code').forEach(function(codeBlock) {
if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;
codeBlock.parentElement.setAttribute('data-copy-added', 'true');
var btn = document.createElement('button');
btn.textContent = 'Copy';
btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';
btn.onmouseover = function() { this.style.opacity = '1'; };
btn.onmouseout = function() { this.style.opacity = '0.7'; };
btn.onclick = function() {
navigator.clipboard.writeText(codeBlock.textContent).then(function() {
btn.textContent = 'Copied!';
setTimeout(function() { btn.textContent = 'Copy'; }, 1500);
});
};
codeBlock.parentElement.style.position = 'relative';
codeBlock.parentElement.appendChild(btn);
});
}
addCopyButtons();
// Re-run on dynamic content
var observer = new MutationObserver(addCopyButtons);
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
feat(quiz): rate limit, daily spend guard, generation timeout, failure events (#544) by AndresL230 · Pull Request #552 · SaplingLearn/Sapling · GitHub
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions backend/db/connection.py
Original file line numberDiff line numberDiff line change
Expand Up@@ -34,14 +34,21 @@ def select(
filters: Optional[dict] = None,
order: Optional[str] = None,
limit: Optional[int] = None,
offset: Optional[int] = None,
) -> list:
"""Read rows. Pass `limit`/`offset` to page — PostgREST caps a
response at `max_rows` (1000) and answers 206 Partial Content,
which is a 2xx, so an unpaged read over that many rows truncates
silently."""
params: dict = {"select": columns}
if filters:
params.update(filters)
if order:
params["order"] = order
if limit:
params["limit"] = str(limit)
if offset is not None:
params["offset"] = str(offset)
r = _client.get(self.url, params=params)
r.raise_for_status()
return r.json()
Expand Down
8 changes: 7 additions & 1 deletion backend/main.py
Original file line numberDiff line numberDiff line change
Expand Up@@ -201,10 +201,16 @@ async def http_exception_handler(request: Request, exc: StarletteHTTPException):
code=getattr(exc, "code", None),
machine_detail=getattr(exc, "machine_detail", None),
)
# Preserve headers the raise site set (e.g. Retry-After on a 429) —
# dropping them would strip the only machine-readable part of a
# throttling response.
headers = dict(getattr(exc, "headers", None) or {})
if rid:
headers["X-Request-ID"] = rid
return JSONResponse(
status_code=exc.status_code,
content=content,
headers={"X-Request-ID": rid} if rid else {},
headers=headers,
)


Expand Down
7 changes: 4 additions & 3 deletions backend/routes/extract.py
Original file line numberDiff line numberDiff line change
Expand Up@@ -30,12 +30,13 @@ def _enforce_ocr_limits(request: Request) -> str:
user_id = get_session_user_id(request)
retry = check_rate_limit(f"ocr:{user_id}", limit=_OCR_RATE_LIMIT, window_sec=_OCR_RATE_WINDOW)
if retry is not None:
# NB: the app's global HTTPException handler (main.py) doesn't forward
# exc.headers, so the retry budget is conveyed in the detail string
# rather than a Retry-After header.
# main.py's HTTPException handler forwards exc.headers as of #544, so
# the budget rides in a real Retry-After. It stays in the detail
# string too — clients that only surface the message keep working.
raise HTTPException(
status_code=429,
detail=f"Too many OCR requests. Retry in {retry}s.",
headers={"Retry-After": str(retry)},
)
return user_id

Expand Down
6 changes: 4 additions & 2 deletions backend/routes/gradescope.py
Original file line numberDiff line numberDiff line change
Expand Up@@ -202,11 +202,13 @@ def _enforce_gs_rate_limit(user_id: str, action: str, *, limit: int, window_sec:
f"gradescope:{action}:{user_id}", limit=limit, window_sec=window_sec
)
if retry is not None:
# main.py's HTTPException handler drops exc.headers, so the retry budget
# rides in the detail string rather than a Retry-After header.
# main.py's HTTPException handler forwards exc.headers as of #544, so
# the budget rides in a real Retry-After. It stays in the detail
# string too — clients that only surface the message keep working.
raise HTTPException(
status_code=429,
detail=f"Too many Gradescope {action} requests. Retry in {retry}s.",
headers={"Retry-After": str(retry)},
)


Expand Down
151 changes: 149 additions & 2 deletions backend/routes/quiz.py
Original file line numberDiff line numberDiff line change
Expand Up@@ -25,12 +25,17 @@
from services.quiz_config import (
CONCRETE_DIFFICULTIES,
QUIZ_ATTEMPT_ABANDON_TTL_HOURS,
QUIZ_DAILY_SPEND_CAP_USD,
QUIZ_GENERATE_RATE_LIMIT,
QUIZ_GENERATE_RATE_WINDOW_SEC,
QUIZ_GENERATION_TIMEOUT_SEC,
QUIZ_TOPUP_DROP_RATIO,
QUIZ_TOPUP_MAX_RETRIES,
REQUESTED_DIFFICULTIES,
mastery_after,
quiz_config_payload,
)
from services.request_limits import check_rate_limit, refund_rate_limit
from services.quiz_errors import QuizAPIError, QuizErrorCode
from services.profiles import get_display_name
from services.encryption import encrypt_json, decrypt_json_column
Expand DownExpand Up@@ -87,6 +92,80 @@ def _load_prompt(name: str) -> str:
_MAX_HISTORY_OFFSET = 1_000_000


# supabase/config.toml sets PostgREST's max_rows = 1000, and an over-cap
# response is 206 Partial Content — a 2xx, so raise_for_status never fires
# and the truncation is silent. Same constant and same reasoning as
# achievement_service._daily_totals; page to completion or the sum is a lie.
_USAGE_PAGE = 1000


def _daily_spend_exceeded(user_id: str) -> bool:
"""True if this user is past the daily LLM spend ceiling (#544 F1).

Reads the llm_usage ledger agents/usage.py already writes, PAGED: an
unpaged read stops at max_rows, so a heavy user's sum plateaus below
the cap and the guard never trips for exactly the runaway it targets.
Stops early once the ceiling is crossed — the common case is a couple
of rows, and a user past the cap doesn't need an exact total.

Fails OPEN on any error: this is a cost control, not a correctness
gate, and denying every student because a usage read blipped is worse
than the spend it would save.
"""
try:
since = (datetime.now(timezone.utc) - timedelta(days=1)).isoformat()
spent = 0.0
offset = 0
while True:
rows = table("llm_usage").select(
"cost_usd",
filters={"user_id": f"eq.{user_id}", "created_at": f"gte.{since}"},
limit=_USAGE_PAGE,
offset=offset,
) or []
spent += sum(float(r.get("cost_usd") or 0.0) for r in rows)
if spent >= QUIZ_DAILY_SPEND_CAP_USD:
return True
if len(rows) < _USAGE_PAGE:
return False
offset += _USAGE_PAGE
except Exception:
logger.exception("quiz: daily spend check failed user=%s; allowing", user_id)
return False


def _refund_generate_slot(user_id: str) -> None:
"""Hand back the rate-limit slot a failed generation consumed (#544 F1).

The slot is claimed BEFORE the model runs (so a burst can't get past
the gate concurrently), which means a backend failure would otherwise
spend the student's quota: eight 502s in two minutes would lock them
out for five with a message saying they'd generated too many quizzes,
having received none. A failure the student didn't cause shouldn't
cost them anything, and the 502 explicitly invites a retry.
"""
try:
refund_rate_limit(f"quiz_generate:{user_id}")
except Exception:
logger.exception("quiz: rate-limit refund failed user=%s", user_id)


def _log_generation_failed(body, request_id: str | None, reason: str) -> None:
"""#544 F3: make a 502 the student saw a 502 an admin can count."""
events_service.log_event(
"quiz.generation_failed",
category="error",
user_id=body.user_id,
request_id=request_id,
payload={
"concept_node_id": body.concept_node_id,
"difficulty": body.difficulty,
"num_questions": body.num_questions,
"reason": reason,
},
)


def _abandon_cutoff() -> datetime:
return datetime.now(timezone.utc) - timedelta(
hours=QUIZ_ATTEMPT_ABANDON_TTL_HOURS
Expand DownExpand Up@@ -510,8 +589,17 @@ async def _quiz_via_agent(
run_kwargs["model"] = model_override

async def _run(message: str, limits) -> Quiz:
# #544 F2: bound EACH agent run rather than the whole function.
# Wrapping the outer coroutine cancelled it mid-flight, and
# CancelledError is a BaseException — it flew straight past the
# top-up's serve-what-we-have handler and threw away questions the
# student had already paid for. Timing out one run raises an
# ordinary TimeoutError the existing handlers can reason about.
result = record_agent_usage(
await quiz_agent.run(message, usage_limits=limits, **run_kwargs),
await asyncio.wait_for(
quiz_agent.run(message, usage_limits=limits, **run_kwargs),
timeout=QUIZ_GENERATION_TIMEOUT_SEC,
),
feature="quiz", task="quiz", user_id=deps.user_id,
)
return result.output
Expand DownExpand Up@@ -582,7 +670,7 @@ def _absorb(quiz: Quiz) -> None:
)
try:
_absorb(await _run(topup_msg, TOPUP_LIMITS))
except Exception as e:
except (Exception, asyncio.TimeoutError) as e:
# The request deliberately SUCCEEDS from here — serve the
# short quiz with an honest count. No traceback: the E2E
# logscan oracle reports those as findings, and this path
Expand DownExpand Up@@ -652,7 +740,44 @@ async def generate_quiz(body: GenerateQuizBody, request: Request):
or str(uuid.uuid4())
)

# #544 F1: cost guards run AFTER ownership (a stranger's node 404s
# first, so probing can't consume a victim's quota) and BEFORE the
# model call. Neither rejection is a backend failure, so neither emits
# quiz.generation_failed.
retry_after = check_rate_limit(
f"quiz_generate:{body.user_id}",
limit=QUIZ_GENERATE_RATE_LIMIT,
window_sec=QUIZ_GENERATE_RATE_WINDOW_SEC,
)
if retry_after is not None:
raise QuizAPIError(
status_code=429,
code=QuizErrorCode.QUIZ_RATE_LIMITED,
message=(
"You've generated a lot of quizzes just now — "
"take a moment and try again shortly."
),
headers={"Retry-After": str(retry_after)},
)
if _daily_spend_exceeded(body.user_id):
logger.warning(
"quiz: daily spend cap reached user=%s request_id=%s",
body.user_id, request_id,
)
raise QuizAPIError(
status_code=429,
code=QuizErrorCode.QUIZ_DAILY_LIMIT_REACHED,
message=(
"You've reached today's limit for AI-generated study "
"material. It resets tomorrow."
),
)

try:
# Each agent run inside is individually bounded by
# QUIZ_GENERATION_TIMEOUT_SEC (see _run) — cancelling the whole
# coroutine here would discard a partial quiz the top-up handler
# is designed to serve.
questions = await _quiz_via_agent(
user_id=body.user_id,
course_id=course_id,
Expand All@@ -667,18 +792,40 @@ async def generate_quiz(body: GenerateQuizBody, request: Request):
except HTTPException:
# The 404 for an unknown concept node is raised before the agent call;
# never swallow a known HTTP state.
_refund_generate_slot(body.user_id)
raise
except asyncio.TimeoutError as e:
# #544 F2: distinct from a generic failure — the client can say
# "that took too long" and offering a retry obviously makes sense.
# NB: only asyncio.TimeoutError. The builtin TimeoutError is in the
# OSError family, so catching it too would relabel a transport
# socket timeout as a wall-clock generation timeout.
logger.warning(
"quiz: generation timed out after %ss request_id=%s",
QUIZ_GENERATION_TIMEOUT_SEC, request_id,
)
_refund_generate_slot(body.user_id)
_log_generation_failed(body, request_id, "timeout")
raise QuizAPIError(
status_code=502,
code=QuizErrorCode.QUIZ_GENERATION_TIMEOUT,
message="Quiz generation took too long. Please try again.",
) from e
except (UsageLimitExceeded, UnexpectedModelBehavior) as e:
# The raw-Gemini legacy fallback was retired in #145; degrade to 502
# rather than serving a quiz from a second LLM path.
logger.warning("Quiz agent guardrails tripped; returning 502", exc_info=e)
_refund_generate_slot(body.user_id)
_log_generation_failed(body, request_id, "agent_guardrail")
raise QuizAPIError(
status_code=502,
code=QuizErrorCode.QUIZ_GENERATION_FAILED,
message="Quiz generation is temporarily unavailable. Please try again.",
) from e
except Exception as e:
logger.exception("Unexpected quiz-agent failure; returning 502")
_refund_generate_slot(body.user_id)
_log_generation_failed(body, request_id, "agent_error")
raise QuizAPIError(
status_code=502,
code=QuizErrorCode.QUIZ_GENERATION_FAILED,
Expand Down
4 changes: 4 additions & 0 deletions backend/services/events_service.py
Original file line numberDiff line numberDiff line change
Expand Up@@ -86,6 +86,10 @@
# surfaces in admin analytics — this failure was invisible for months
# precisely because nothing emitted when the background task died.
"quiz.context_write_failed",
# #544/F3: generation failed (agent error, timeout, or every question
# dropped). Same reasoning: a 502 the student sees should be a 502 an
# admin can count.
"quiz.generation_failed",
"chat.message_sent",
"note.created",
"session.started",
Expand Down
28 changes: 28 additions & 0 deletions backend/services/quiz_config.py
Original file line numberDiff line numberDiff line change
Expand Up@@ -79,6 +79,34 @@ def mastery_after(before: float, *, score: int, total: int) -> float:
return max(0.0, min(1.0, raw))


# ── Cost + abuse guards (#544 F1/F2) ────────────────────────────────────────
#
# Generation is an unbounded LLM call behind a button: before #544 nothing
# stopped a held-down key or a scripted loop from spending real money.
#
# The rate limit is sized for a human: a student comparing difficulties or
# retaking a concept might legitimately generate a handful of quizzes in a
# few minutes; nobody legitimately generates 10 in one.
QUIZ_GENERATE_RATE_LIMIT = 8
QUIZ_GENERATE_RATE_WINDOW_SEC = 300 # 5 minutes

# Daily per-user LLM spend ceiling. The SPEND it measures is cross-feature
# (llm_usage records every agent call, not just quiz ones), but the ceiling
# is only ENFORCED on quiz generation — the one unbounded LLM call behind a
# button. Other entry points stay unguarded for now; moving this into a
# shared guard is its own piece of work, not something to imply here.
# A generation on the default flash-lite tier costs well under a cent, so
# this is ~2 orders of magnitude above any real study day — it exists to
# bound a runaway, not to ration normal use. Deliberately fail-OPEN: if the
# usage read errors we serve the quiz rather than denying every student.
QUIZ_DAILY_SPEND_CAP_USD = 2.00

# Wall-clock ceiling on one generation (agent run incl. its tool calls).
# Past this the student is staring at a spinner and would rather be told to
# try again; the request also stops holding a worker slot.
QUIZ_GENERATION_TIMEOUT_SEC = 90


# ── Generation honesty (#543 E2) ────────────────────────────────────────────
#
# Questions whose correct_answer doesn't match an option verbatim are
Expand Down
11 changes: 10 additions & 1 deletion backend/services/quiz_errors.py
Original file line numberDiff line numberDiff line change
Expand Up@@ -46,6 +46,14 @@ class QuizErrorCode(str, Enum):
QUIZ_QUESTION_INVALID = "QUIZ_QUESTION_INVALID"
QUIZ_NOT_AUTHORIZED = "QUIZ_NOT_AUTHORIZED"
QUIZ_GENERATION_FAILED = "QUIZ_GENERATION_FAILED"
# #544 F2: the generation exceeded its wall-clock budget. Distinct from
# the generic failure so the client can say "that took too long" and a
# retry is obviously worth offering.
QUIZ_GENERATION_TIMEOUT = "QUIZ_GENERATION_TIMEOUT"
# #544 F1: too many generations in the rate window.
QUIZ_RATE_LIMITED = "QUIZ_RATE_LIMITED"
# #544 F1: this account's daily LLM spend ceiling is reached.
QUIZ_DAILY_LIMIT_REACHED = "QUIZ_DAILY_LIMIT_REACHED"
QUIZ_INTERNAL_ERROR = "QUIZ_INTERNAL_ERROR"
# Uncoded HTTP errors that aren't one of the semantic states above —
# router 404s/405s on version-skewed clients, library-raised
Expand DownExpand Up@@ -80,8 +88,9 @@ def __init__(
code: QuizErrorCode,
message: str,
machine_detail=None,
headers: dict[str, str] | None = None,
):
super().__init__(status_code=status_code, detail=message)
super().__init__(status_code=status_code, detail=message, headers=headers)
self.code = code
self.machine_detail = machine_detail

Expand Down
14 changes: 14 additions & 0 deletions backend/services/request_limits.py
Original file line numberDiff line numberDiff line change
Expand Up@@ -37,6 +37,20 @@ def check_rate_limit(key: str, *, limit: int, window_sec: int) -> int | None:
return None


def refund_rate_limit(key: str) -> None:
"""Give back the most recent slot recorded for `key`.

For guards that must claim BEFORE doing the expensive work (so a
concurrent burst can't slip past the gate) but shouldn't charge the
caller when that work fails for reasons they didn't cause. Dropping
the newest timestamp — not the oldest — keeps the window's start
anchored to the caller's earliest real attempt.
"""
bucket = _rate_state.get(key)
if bucket:
bucket.pop()


async def read_within_limit(upload: UploadFile, max_bytes: int) -> bytes:
"""Read at most ``max_bytes`` (+1 to detect overflow) from an UploadFile so
an oversize upload can't be pulled fully into memory before we reject it.
Expand Down
13 changes: 13 additions & 0 deletions backend/tests/conftest.py
Original file line numberDiff line numberDiff line change
Expand Up@@ -47,6 +47,19 @@ def pytest_configure(config):
)


@pytest.fixture(autouse=True)
def _clear_rate_limit_state():
"""#544: services/request_limits keeps its sliding windows in a
process-global dict, so one test's burst of requests would throttle
every later test that hits the same route as the same user. Same
reasoning as the lru_cache reset below."""
from services import request_limits

request_limits._rate_state.clear()
yield
request_limits._rate_state.clear()


@pytest.fixture(autouse=True)
def _clear_lru_caches():
"""#98: reset the per-process lru_caches around every test so one test's
Expand Down
Loading
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { // Force GitHub README to respect dark mode (function() { var style = document.createElement('style'); style.textContent = ' .markdown-body { color-scheme: dark light; } .markdown-body pre { background: #161b22 !important; } .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; } .markdown-body table th, .markdown-body table td { border-color: #30363d !important; } .markdown-body img { background: #0d1117; } .markdown-body blockquote { border-left-color: #8b949e; } .markdown-body hr { border-color: #30363d; } '; document.head.appendChild(style); })(); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' feat(quiz): rate limit, daily spend guard, generation timeout, failure events (#544) by AndresL230 · Pull Request #552 · SaplingLearn/Sapling · GitHub
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions backend/db/connection.py
Original file line numberDiff line numberDiff line change
Expand Up@@ -34,14 +34,21 @@ def select(
filters: Optional[dict] = None,
order: Optional[str] = None,
limit: Optional[int] = None,
offset: Optional[int] = None,
) -> list:
"""Read rows. Pass `limit`/`offset` to page — PostgREST caps a
response at `max_rows` (1000) and answers 206 Partial Content,
which is a 2xx, so an unpaged read over that many rows truncates
silently."""
params: dict = {"select": columns}
if filters:
params.update(filters)
if order:
params["order"] = order
if limit:
params["limit"] = str(limit)
if offset is not None:
params["offset"] = str(offset)
r = _client.get(self.url, params=params)
r.raise_for_status()
return r.json()
Expand Down
8 changes: 7 additions & 1 deletion backend/main.py
Original file line numberDiff line numberDiff line change
Expand Up@@ -201,10 +201,16 @@ async def http_exception_handler(request: Request, exc: StarletteHTTPException):
code=getattr(exc, "code", None),
machine_detail=getattr(exc, "machine_detail", None),
)
# Preserve headers the raise site set (e.g. Retry-After on a 429) —
# dropping them would strip the only machine-readable part of a
# throttling response.
headers = dict(getattr(exc, "headers", None) or {})
if rid:
headers["X-Request-ID"] = rid
return JSONResponse(
status_code=exc.status_code,
content=content,
headers={"X-Request-ID": rid} if rid else {},
headers=headers,
)


Expand Down
7 changes: 4 additions & 3 deletions backend/routes/extract.py
Original file line numberDiff line numberDiff line change
Expand Up@@ -30,12 +30,13 @@ def _enforce_ocr_limits(request: Request) -> str:
user_id = get_session_user_id(request)
retry = check_rate_limit(f"ocr:{user_id}", limit=_OCR_RATE_LIMIT, window_sec=_OCR_RATE_WINDOW)
if retry is not None:
# NB: the app's global HTTPException handler (main.py) doesn't forward
# exc.headers, so the retry budget is conveyed in the detail string
# rather than a Retry-After header.
# main.py's HTTPException handler forwards exc.headers as of #544, so
# the budget rides in a real Retry-After. It stays in the detail
# string too — clients that only surface the message keep working.
raise HTTPException(
status_code=429,
detail=f"Too many OCR requests. Retry in {retry}s.",
headers={"Retry-After": str(retry)},
)
return user_id

Expand Down
6 changes: 4 additions & 2 deletions backend/routes/gradescope.py
Original file line numberDiff line numberDiff line change
Expand Up@@ -202,11 +202,13 @@ def _enforce_gs_rate_limit(user_id: str, action: str, *, limit: int, window_sec:
f"gradescope:{action}:{user_id}", limit=limit, window_sec=window_sec
)
if retry is not None:
# main.py's HTTPException handler drops exc.headers, so the retry budget
# rides in the detail string rather than a Retry-After header.
# main.py's HTTPException handler forwards exc.headers as of #544, so
# the budget rides in a real Retry-After. It stays in the detail
# string too — clients that only surface the message keep working.
raise HTTPException(
status_code=429,
detail=f"Too many Gradescope {action} requests. Retry in {retry}s.",
headers={"Retry-After": str(retry)},
)


Expand Down
151 changes: 149 additions & 2 deletions backend/routes/quiz.py
Original file line numberDiff line numberDiff line change
Expand Up@@ -25,12 +25,17 @@
from services.quiz_config import (
CONCRETE_DIFFICULTIES,
QUIZ_ATTEMPT_ABANDON_TTL_HOURS,
QUIZ_DAILY_SPEND_CAP_USD,
QUIZ_GENERATE_RATE_LIMIT,
QUIZ_GENERATE_RATE_WINDOW_SEC,
QUIZ_GENERATION_TIMEOUT_SEC,
QUIZ_TOPUP_DROP_RATIO,
QUIZ_TOPUP_MAX_RETRIES,
REQUESTED_DIFFICULTIES,
mastery_after,
quiz_config_payload,
)
from services.request_limits import check_rate_limit, refund_rate_limit
from services.quiz_errors import QuizAPIError, QuizErrorCode
from services.profiles import get_display_name
from services.encryption import encrypt_json, decrypt_json_column
Expand DownExpand Up@@ -87,6 +92,80 @@ def _load_prompt(name: str) -> str:
_MAX_HISTORY_OFFSET = 1_000_000


# supabase/config.toml sets PostgREST's max_rows = 1000, and an over-cap
# response is 206 Partial Content — a 2xx, so raise_for_status never fires
# and the truncation is silent. Same constant and same reasoning as
# achievement_service._daily_totals; page to completion or the sum is a lie.
_USAGE_PAGE = 1000


def _daily_spend_exceeded(user_id: str) -> bool:
"""True if this user is past the daily LLM spend ceiling (#544 F1).

Reads the llm_usage ledger agents/usage.py already writes, PAGED: an
unpaged read stops at max_rows, so a heavy user's sum plateaus below
the cap and the guard never trips for exactly the runaway it targets.
Stops early once the ceiling is crossed — the common case is a couple
of rows, and a user past the cap doesn't need an exact total.

Fails OPEN on any error: this is a cost control, not a correctness
gate, and denying every student because a usage read blipped is worse
than the spend it would save.
"""
try:
since = (datetime.now(timezone.utc) - timedelta(days=1)).isoformat()
spent = 0.0
offset = 0
while True:
rows = table("llm_usage").select(
"cost_usd",
filters={"user_id": f"eq.{user_id}", "created_at": f"gte.{since}"},
limit=_USAGE_PAGE,
offset=offset,
) or []
spent += sum(float(r.get("cost_usd") or 0.0) for r in rows)
if spent >= QUIZ_DAILY_SPEND_CAP_USD:
return True
if len(rows) < _USAGE_PAGE:
return False
offset += _USAGE_PAGE
except Exception:
logger.exception("quiz: daily spend check failed user=%s; allowing", user_id)
return False


def _refund_generate_slot(user_id: str) -> None:
"""Hand back the rate-limit slot a failed generation consumed (#544 F1).

The slot is claimed BEFORE the model runs (so a burst can't get past
the gate concurrently), which means a backend failure would otherwise
spend the student's quota: eight 502s in two minutes would lock them
out for five with a message saying they'd generated too many quizzes,
having received none. A failure the student didn't cause shouldn't
cost them anything, and the 502 explicitly invites a retry.
"""
try:
refund_rate_limit(f"quiz_generate:{user_id}")
except Exception:
logger.exception("quiz: rate-limit refund failed user=%s", user_id)


def _log_generation_failed(body, request_id: str | None, reason: str) -> None:
"""#544 F3: make a 502 the student saw a 502 an admin can count."""
events_service.log_event(
"quiz.generation_failed",
category="error",
user_id=body.user_id,
request_id=request_id,
payload={
"concept_node_id": body.concept_node_id,
"difficulty": body.difficulty,
"num_questions": body.num_questions,
"reason": reason,
},
)


def _abandon_cutoff() -> datetime:
return datetime.now(timezone.utc) - timedelta(
hours=QUIZ_ATTEMPT_ABANDON_TTL_HOURS
Expand DownExpand Up@@ -510,8 +589,17 @@ async def _quiz_via_agent(
run_kwargs["model"] = model_override

async def _run(message: str, limits) -> Quiz:
# #544 F2: bound EACH agent run rather than the whole function.
# Wrapping the outer coroutine cancelled it mid-flight, and
# CancelledError is a BaseException — it flew straight past the
# top-up's serve-what-we-have handler and threw away questions the
# student had already paid for. Timing out one run raises an
# ordinary TimeoutError the existing handlers can reason about.
result = record_agent_usage(
await quiz_agent.run(message, usage_limits=limits, **run_kwargs),
await asyncio.wait_for(
quiz_agent.run(message, usage_limits=limits, **run_kwargs),
timeout=QUIZ_GENERATION_TIMEOUT_SEC,
),
feature="quiz", task="quiz", user_id=deps.user_id,
)
return result.output
Expand DownExpand Up@@ -582,7 +670,7 @@ def _absorb(quiz: Quiz) -> None:
)
try:
_absorb(await _run(topup_msg, TOPUP_LIMITS))
except Exception as e:
except (Exception, asyncio.TimeoutError) as e:
# The request deliberately SUCCEEDS from here — serve the
# short quiz with an honest count. No traceback: the E2E
# logscan oracle reports those as findings, and this path
Expand DownExpand Up@@ -652,7 +740,44 @@ async def generate_quiz(body: GenerateQuizBody, request: Request):
or str(uuid.uuid4())
)

# #544 F1: cost guards run AFTER ownership (a stranger's node 404s
# first, so probing can't consume a victim's quota) and BEFORE the
# model call. Neither rejection is a backend failure, so neither emits
# quiz.generation_failed.
retry_after = check_rate_limit(
f"quiz_generate:{body.user_id}",
limit=QUIZ_GENERATE_RATE_LIMIT,
window_sec=QUIZ_GENERATE_RATE_WINDOW_SEC,
)
if retry_after is not None:
raise QuizAPIError(
status_code=429,
code=QuizErrorCode.QUIZ_RATE_LIMITED,
message=(
"You've generated a lot of quizzes just now — "
"take a moment and try again shortly."
),
headers={"Retry-After": str(retry_after)},
)
if _daily_spend_exceeded(body.user_id):
logger.warning(
"quiz: daily spend cap reached user=%s request_id=%s",
body.user_id, request_id,
)
raise QuizAPIError(
status_code=429,
code=QuizErrorCode.QUIZ_DAILY_LIMIT_REACHED,
message=(
"You've reached today's limit for AI-generated study "
"material. It resets tomorrow."
),
)

try:
# Each agent run inside is individually bounded by
# QUIZ_GENERATION_TIMEOUT_SEC (see _run) — cancelling the whole
# coroutine here would discard a partial quiz the top-up handler
# is designed to serve.
questions = await _quiz_via_agent(
user_id=body.user_id,
course_id=course_id,
Expand All@@ -667,18 +792,40 @@ async def generate_quiz(body: GenerateQuizBody, request: Request):
except HTTPException:
# The 404 for an unknown concept node is raised before the agent call;
# never swallow a known HTTP state.
_refund_generate_slot(body.user_id)
raise
except asyncio.TimeoutError as e:
# #544 F2: distinct from a generic failure — the client can say
# "that took too long" and offering a retry obviously makes sense.
# NB: only asyncio.TimeoutError. The builtin TimeoutError is in the
# OSError family, so catching it too would relabel a transport
# socket timeout as a wall-clock generation timeout.
logger.warning(
"quiz: generation timed out after %ss request_id=%s",
QUIZ_GENERATION_TIMEOUT_SEC, request_id,
)
_refund_generate_slot(body.user_id)
_log_generation_failed(body, request_id, "timeout")
raise QuizAPIError(
status_code=502,
code=QuizErrorCode.QUIZ_GENERATION_TIMEOUT,
message="Quiz generation took too long. Please try again.",
) from e
except (UsageLimitExceeded, UnexpectedModelBehavior) as e:
# The raw-Gemini legacy fallback was retired in #145; degrade to 502
# rather than serving a quiz from a second LLM path.
logger.warning("Quiz agent guardrails tripped; returning 502", exc_info=e)
_refund_generate_slot(body.user_id)
_log_generation_failed(body, request_id, "agent_guardrail")
raise QuizAPIError(
status_code=502,
code=QuizErrorCode.QUIZ_GENERATION_FAILED,
message="Quiz generation is temporarily unavailable. Please try again.",
) from e
except Exception as e:
logger.exception("Unexpected quiz-agent failure; returning 502")
_refund_generate_slot(body.user_id)
_log_generation_failed(body, request_id, "agent_error")
raise QuizAPIError(
status_code=502,
code=QuizErrorCode.QUIZ_GENERATION_FAILED,
Expand Down
4 changes: 4 additions & 0 deletions backend/services/events_service.py
Original file line numberDiff line numberDiff line change
Expand Up@@ -86,6 +86,10 @@
# surfaces in admin analytics — this failure was invisible for months
# precisely because nothing emitted when the background task died.
"quiz.context_write_failed",
# #544/F3: generation failed (agent error, timeout, or every question
# dropped). Same reasoning: a 502 the student sees should be a 502 an
# admin can count.
"quiz.generation_failed",
"chat.message_sent",
"note.created",
"session.started",
Expand Down
28 changes: 28 additions & 0 deletions backend/services/quiz_config.py
Original file line numberDiff line numberDiff line change
Expand Up@@ -79,6 +79,34 @@ def mastery_after(before: float, *, score: int, total: int) -> float:
return max(0.0, min(1.0, raw))


# ── Cost + abuse guards (#544 F1/F2) ────────────────────────────────────────
#
# Generation is an unbounded LLM call behind a button: before #544 nothing
# stopped a held-down key or a scripted loop from spending real money.
#
# The rate limit is sized for a human: a student comparing difficulties or
# retaking a concept might legitimately generate a handful of quizzes in a
# few minutes; nobody legitimately generates 10 in one.
QUIZ_GENERATE_RATE_LIMIT = 8
QUIZ_GENERATE_RATE_WINDOW_SEC = 300 # 5 minutes

# Daily per-user LLM spend ceiling. The SPEND it measures is cross-feature
# (llm_usage records every agent call, not just quiz ones), but the ceiling
# is only ENFORCED on quiz generation — the one unbounded LLM call behind a
# button. Other entry points stay unguarded for now; moving this into a
# shared guard is its own piece of work, not something to imply here.
# A generation on the default flash-lite tier costs well under a cent, so
# this is ~2 orders of magnitude above any real study day — it exists to
# bound a runaway, not to ration normal use. Deliberately fail-OPEN: if the
# usage read errors we serve the quiz rather than denying every student.
QUIZ_DAILY_SPEND_CAP_USD = 2.00

# Wall-clock ceiling on one generation (agent run incl. its tool calls).
# Past this the student is staring at a spinner and would rather be told to
# try again; the request also stops holding a worker slot.
QUIZ_GENERATION_TIMEOUT_SEC = 90


# ── Generation honesty (#543 E2) ────────────────────────────────────────────
#
# Questions whose correct_answer doesn't match an option verbatim are
Expand Down
11 changes: 10 additions & 1 deletion backend/services/quiz_errors.py
Original file line numberDiff line numberDiff line change
Expand Up@@ -46,6 +46,14 @@ class QuizErrorCode(str, Enum):
QUIZ_QUESTION_INVALID = "QUIZ_QUESTION_INVALID"
QUIZ_NOT_AUTHORIZED = "QUIZ_NOT_AUTHORIZED"
QUIZ_GENERATION_FAILED = "QUIZ_GENERATION_FAILED"
# #544 F2: the generation exceeded its wall-clock budget. Distinct from
# the generic failure so the client can say "that took too long" and a
# retry is obviously worth offering.
QUIZ_GENERATION_TIMEOUT = "QUIZ_GENERATION_TIMEOUT"
# #544 F1: too many generations in the rate window.
QUIZ_RATE_LIMITED = "QUIZ_RATE_LIMITED"
# #544 F1: this account's daily LLM spend ceiling is reached.
QUIZ_DAILY_LIMIT_REACHED = "QUIZ_DAILY_LIMIT_REACHED"
QUIZ_INTERNAL_ERROR = "QUIZ_INTERNAL_ERROR"
# Uncoded HTTP errors that aren't one of the semantic states above —
# router 404s/405s on version-skewed clients, library-raised
Expand DownExpand Up@@ -80,8 +88,9 @@ def __init__(
code: QuizErrorCode,
message: str,
machine_detail=None,
headers: dict[str, str] | None = None,
):
super().__init__(status_code=status_code, detail=message)
super().__init__(status_code=status_code, detail=message, headers=headers)
self.code = code
self.machine_detail = machine_detail

Expand Down
14 changes: 14 additions & 0 deletions backend/services/request_limits.py
Original file line numberDiff line numberDiff line change
Expand Up@@ -37,6 +37,20 @@ def check_rate_limit(key: str, *, limit: int, window_sec: int) -> int | None:
return None


def refund_rate_limit(key: str) -> None:
"""Give back the most recent slot recorded for `key`.

For guards that must claim BEFORE doing the expensive work (so a
concurrent burst can't slip past the gate) but shouldn't charge the
caller when that work fails for reasons they didn't cause. Dropping
the newest timestamp — not the oldest — keeps the window's start
anchored to the caller's earliest real attempt.
"""
bucket = _rate_state.get(key)
if bucket:
bucket.pop()


async def read_within_limit(upload: UploadFile, max_bytes: int) -> bytes:
"""Read at most ``max_bytes`` (+1 to detect overflow) from an UploadFile so
an oversize upload can't be pulled fully into memory before we reject it.
Expand Down
13 changes: 13 additions & 0 deletions backend/tests/conftest.py
Original file line numberDiff line numberDiff line change
Expand Up@@ -47,6 +47,19 @@ def pytest_configure(config):
)


@pytest.fixture(autouse=True)
def _clear_rate_limit_state():
"""#544: services/request_limits keeps its sliding windows in a
process-global dict, so one test's burst of requests would throttle
every later test that hits the same route as the same user. Same
reasoning as the lru_cache reset below."""
from services import request_limits

request_limits._rate_state.clear()
yield
request_limits._rate_state.clear()


@pytest.fixture(autouse=True)
def _clear_lru_caches():
"""#98: reset the per-process lru_caches around every test so one test's
Expand Down
Loading
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { // Highlight search terms from Google/DuckDuckGo/Bing referrer (function() { var ref = document.referrer; var terms = []; if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) { var url = new URL(ref); var q = url.searchParams.get('q') || url.searchParams.get('p'); if (q) { terms = q.split(/\s+/).filter(function(t) { return t.length > 2; }); } } if (terms.length === 0) return; var style = document.createElement('style'); style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }'; document.head.appendChild(style); function highlight(node) { if (node.nodeType === 3) { // text node var text = node.textContent; var found = false; terms.forEach(function(term) { var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\]\\]/g, '\\') + ')', 'gi'); if (regex.test(text)) { found = true; var frag = document.createDocumentFragment(); var parts = text.split(regex); parts.forEach(function(part, i) { if (i % 2 === 0) { frag.appendChild(document.createTextNode(part)); } else { var span = document.createElement('span'); span.className = 'userscript-highlight'; span.textContent = part; frag.appendChild(span); } }); node.parentNode.replaceChild(frag, node); } }); } else if (node.nodeType === 1 && node.childNodes) { // element var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT']; if (!skipTags.includes(node.tagName)) { Array.from(node.childNodes).forEach(highlight); } } } highlight(document.body); // Re-highlight on dynamic content var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1 || node.nodeType === 3) highlight(node); }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' feat(quiz): rate limit, daily spend guard, generation timeout, failure events (#544) by AndresL230 · Pull Request #552 · SaplingLearn/Sapling · GitHub
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions backend/db/connection.py
Original file line numberDiff line numberDiff line change
Expand Up@@ -34,14 +34,21 @@ def select(
filters: Optional[dict] = None,
order: Optional[str] = None,
limit: Optional[int] = None,
offset: Optional[int] = None,
) -> list:
"""Read rows. Pass `limit`/`offset` to page — PostgREST caps a
response at `max_rows` (1000) and answers 206 Partial Content,
which is a 2xx, so an unpaged read over that many rows truncates
silently."""
params: dict = {"select": columns}
if filters:
params.update(filters)
if order:
params["order"] = order
if limit:
params["limit"] = str(limit)
if offset is not None:
params["offset"] = str(offset)
r = _client.get(self.url, params=params)
r.raise_for_status()
return r.json()
Expand Down
8 changes: 7 additions & 1 deletion backend/main.py
Original file line numberDiff line numberDiff line change
Expand Up@@ -201,10 +201,16 @@ async def http_exception_handler(request: Request, exc: StarletteHTTPException):
code=getattr(exc, "code", None),
machine_detail=getattr(exc, "machine_detail", None),
)
# Preserve headers the raise site set (e.g. Retry-After on a 429) —
# dropping them would strip the only machine-readable part of a
# throttling response.
headers = dict(getattr(exc, "headers", None) or {})
if rid:
headers["X-Request-ID"] = rid
return JSONResponse(
status_code=exc.status_code,
content=content,
headers={"X-Request-ID": rid} if rid else {},
headers=headers,
)


Expand Down
7 changes: 4 additions & 3 deletions backend/routes/extract.py
Original file line numberDiff line numberDiff line change
Expand Up@@ -30,12 +30,13 @@ def _enforce_ocr_limits(request: Request) -> str:
user_id = get_session_user_id(request)
retry = check_rate_limit(f"ocr:{user_id}", limit=_OCR_RATE_LIMIT, window_sec=_OCR_RATE_WINDOW)
if retry is not None:
# NB: the app's global HTTPException handler (main.py) doesn't forward
# exc.headers, so the retry budget is conveyed in the detail string
# rather than a Retry-After header.
# main.py's HTTPException handler forwards exc.headers as of #544, so
# the budget rides in a real Retry-After. It stays in the detail
# string too — clients that only surface the message keep working.
raise HTTPException(
status_code=429,
detail=f"Too many OCR requests. Retry in {retry}s.",
headers={"Retry-After": str(retry)},
)
return user_id

Expand Down
6 changes: 4 additions & 2 deletions backend/routes/gradescope.py
Original file line numberDiff line numberDiff line change
Expand Up@@ -202,11 +202,13 @@ def _enforce_gs_rate_limit(user_id: str, action: str, *, limit: int, window_sec:
f"gradescope:{action}:{user_id}", limit=limit, window_sec=window_sec
)
if retry is not None:
# main.py's HTTPException handler drops exc.headers, so the retry budget
# rides in the detail string rather than a Retry-After header.
# main.py's HTTPException handler forwards exc.headers as of #544, so
# the budget rides in a real Retry-After. It stays in the detail
# string too — clients that only surface the message keep working.
raise HTTPException(
status_code=429,
detail=f"Too many Gradescope {action} requests. Retry in {retry}s.",
headers={"Retry-After": str(retry)},
)


Expand Down
151 changes: 149 additions & 2 deletions backend/routes/quiz.py
Original file line numberDiff line numberDiff line change
Expand Up@@ -25,12 +25,17 @@
from services.quiz_config import (
CONCRETE_DIFFICULTIES,
QUIZ_ATTEMPT_ABANDON_TTL_HOURS,
QUIZ_DAILY_SPEND_CAP_USD,
QUIZ_GENERATE_RATE_LIMIT,
QUIZ_GENERATE_RATE_WINDOW_SEC,
QUIZ_GENERATION_TIMEOUT_SEC,
QUIZ_TOPUP_DROP_RATIO,
QUIZ_TOPUP_MAX_RETRIES,
REQUESTED_DIFFICULTIES,
mastery_after,
quiz_config_payload,
)
from services.request_limits import check_rate_limit, refund_rate_limit
from services.quiz_errors import QuizAPIError, QuizErrorCode
from services.profiles import get_display_name
from services.encryption import encrypt_json, decrypt_json_column
Expand DownExpand Up@@ -87,6 +92,80 @@ def _load_prompt(name: str) -> str:
_MAX_HISTORY_OFFSET = 1_000_000


# supabase/config.toml sets PostgREST's max_rows = 1000, and an over-cap
# response is 206 Partial Content — a 2xx, so raise_for_status never fires
# and the truncation is silent. Same constant and same reasoning as
# achievement_service._daily_totals; page to completion or the sum is a lie.
_USAGE_PAGE = 1000


def _daily_spend_exceeded(user_id: str) -> bool:
"""True if this user is past the daily LLM spend ceiling (#544 F1).

Reads the llm_usage ledger agents/usage.py already writes, PAGED: an
unpaged read stops at max_rows, so a heavy user's sum plateaus below
the cap and the guard never trips for exactly the runaway it targets.
Stops early once the ceiling is crossed — the common case is a couple
of rows, and a user past the cap doesn't need an exact total.

Fails OPEN on any error: this is a cost control, not a correctness
gate, and denying every student because a usage read blipped is worse
than the spend it would save.
"""
try:
since = (datetime.now(timezone.utc) - timedelta(days=1)).isoformat()
spent = 0.0
offset = 0
while True:
rows = table("llm_usage").select(
"cost_usd",
filters={"user_id": f"eq.{user_id}", "created_at": f"gte.{since}"},
limit=_USAGE_PAGE,
offset=offset,
) or []
spent += sum(float(r.get("cost_usd") or 0.0) for r in rows)
if spent >= QUIZ_DAILY_SPEND_CAP_USD:
return True
if len(rows) < _USAGE_PAGE:
return False
offset += _USAGE_PAGE
except Exception:
logger.exception("quiz: daily spend check failed user=%s; allowing", user_id)
return False


def _refund_generate_slot(user_id: str) -> None:
"""Hand back the rate-limit slot a failed generation consumed (#544 F1).

The slot is claimed BEFORE the model runs (so a burst can't get past
the gate concurrently), which means a backend failure would otherwise
spend the student's quota: eight 502s in two minutes would lock them
out for five with a message saying they'd generated too many quizzes,
having received none. A failure the student didn't cause shouldn't
cost them anything, and the 502 explicitly invites a retry.
"""
try:
refund_rate_limit(f"quiz_generate:{user_id}")
except Exception:
logger.exception("quiz: rate-limit refund failed user=%s", user_id)


def _log_generation_failed(body, request_id: str | None, reason: str) -> None:
"""#544 F3: make a 502 the student saw a 502 an admin can count."""
events_service.log_event(
"quiz.generation_failed",
category="error",
user_id=body.user_id,
request_id=request_id,
payload={
"concept_node_id": body.concept_node_id,
"difficulty": body.difficulty,
"num_questions": body.num_questions,
"reason": reason,
},
)


def _abandon_cutoff() -> datetime:
return datetime.now(timezone.utc) - timedelta(
hours=QUIZ_ATTEMPT_ABANDON_TTL_HOURS
Expand DownExpand Up@@ -510,8 +589,17 @@ async def _quiz_via_agent(
run_kwargs["model"] = model_override

async def _run(message: str, limits) -> Quiz:
# #544 F2: bound EACH agent run rather than the whole function.
# Wrapping the outer coroutine cancelled it mid-flight, and
# CancelledError is a BaseException — it flew straight past the
# top-up's serve-what-we-have handler and threw away questions the
# student had already paid for. Timing out one run raises an
# ordinary TimeoutError the existing handlers can reason about.
result = record_agent_usage(
await quiz_agent.run(message, usage_limits=limits, **run_kwargs),
await asyncio.wait_for(
quiz_agent.run(message, usage_limits=limits, **run_kwargs),
timeout=QUIZ_GENERATION_TIMEOUT_SEC,
),
feature="quiz", task="quiz", user_id=deps.user_id,
)
return result.output
Expand DownExpand Up@@ -582,7 +670,7 @@ def _absorb(quiz: Quiz) -> None:
)
try:
_absorb(await _run(topup_msg, TOPUP_LIMITS))
except Exception as e:
except (Exception, asyncio.TimeoutError) as e:
# The request deliberately SUCCEEDS from here — serve the
# short quiz with an honest count. No traceback: the E2E
# logscan oracle reports those as findings, and this path
Expand DownExpand Up@@ -652,7 +740,44 @@ async def generate_quiz(body: GenerateQuizBody, request: Request):
or str(uuid.uuid4())
)

# #544 F1: cost guards run AFTER ownership (a stranger's node 404s
# first, so probing can't consume a victim's quota) and BEFORE the
# model call. Neither rejection is a backend failure, so neither emits
# quiz.generation_failed.
retry_after = check_rate_limit(
f"quiz_generate:{body.user_id}",
limit=QUIZ_GENERATE_RATE_LIMIT,
window_sec=QUIZ_GENERATE_RATE_WINDOW_SEC,
)
if retry_after is not None:
raise QuizAPIError(
status_code=429,
code=QuizErrorCode.QUIZ_RATE_LIMITED,
message=(
"You've generated a lot of quizzes just now — "
"take a moment and try again shortly."
),
headers={"Retry-After": str(retry_after)},
)
if _daily_spend_exceeded(body.user_id):
logger.warning(
"quiz: daily spend cap reached user=%s request_id=%s",
body.user_id, request_id,
)
raise QuizAPIError(
status_code=429,
code=QuizErrorCode.QUIZ_DAILY_LIMIT_REACHED,
message=(
"You've reached today's limit for AI-generated study "
"material. It resets tomorrow."
),
)

try:
# Each agent run inside is individually bounded by
# QUIZ_GENERATION_TIMEOUT_SEC (see _run) — cancelling the whole
# coroutine here would discard a partial quiz the top-up handler
# is designed to serve.
questions = await _quiz_via_agent(
user_id=body.user_id,
course_id=course_id,
Expand All@@ -667,18 +792,40 @@ async def generate_quiz(body: GenerateQuizBody, request: Request):
except HTTPException:
# The 404 for an unknown concept node is raised before the agent call;
# never swallow a known HTTP state.
_refund_generate_slot(body.user_id)
raise
except asyncio.TimeoutError as e:
# #544 F2: distinct from a generic failure — the client can say
# "that took too long" and offering a retry obviously makes sense.
# NB: only asyncio.TimeoutError. The builtin TimeoutError is in the
# OSError family, so catching it too would relabel a transport
# socket timeout as a wall-clock generation timeout.
logger.warning(
"quiz: generation timed out after %ss request_id=%s",
QUIZ_GENERATION_TIMEOUT_SEC, request_id,
)
_refund_generate_slot(body.user_id)
_log_generation_failed(body, request_id, "timeout")
raise QuizAPIError(
status_code=502,
code=QuizErrorCode.QUIZ_GENERATION_TIMEOUT,
message="Quiz generation took too long. Please try again.",
) from e
except (UsageLimitExceeded, UnexpectedModelBehavior) as e:
# The raw-Gemini legacy fallback was retired in #145; degrade to 502
# rather than serving a quiz from a second LLM path.
logger.warning("Quiz agent guardrails tripped; returning 502", exc_info=e)
_refund_generate_slot(body.user_id)
_log_generation_failed(body, request_id, "agent_guardrail")
raise QuizAPIError(
status_code=502,
code=QuizErrorCode.QUIZ_GENERATION_FAILED,
message="Quiz generation is temporarily unavailable. Please try again.",
) from e
except Exception as e:
logger.exception("Unexpected quiz-agent failure; returning 502")
_refund_generate_slot(body.user_id)
_log_generation_failed(body, request_id, "agent_error")
raise QuizAPIError(
status_code=502,
code=QuizErrorCode.QUIZ_GENERATION_FAILED,
Expand Down
4 changes: 4 additions & 0 deletions backend/services/events_service.py
Original file line numberDiff line numberDiff line change
Expand Up@@ -86,6 +86,10 @@
# surfaces in admin analytics — this failure was invisible for months
# precisely because nothing emitted when the background task died.
"quiz.context_write_failed",
# #544/F3: generation failed (agent error, timeout, or every question
# dropped). Same reasoning: a 502 the student sees should be a 502 an
# admin can count.
"quiz.generation_failed",
"chat.message_sent",
"note.created",
"session.started",
Expand Down
28 changes: 28 additions & 0 deletions backend/services/quiz_config.py
Original file line numberDiff line numberDiff line change
Expand Up@@ -79,6 +79,34 @@ def mastery_after(before: float, *, score: int, total: int) -> float:
return max(0.0, min(1.0, raw))


# ── Cost + abuse guards (#544 F1/F2) ────────────────────────────────────────
#
# Generation is an unbounded LLM call behind a button: before #544 nothing
# stopped a held-down key or a scripted loop from spending real money.
#
# The rate limit is sized for a human: a student comparing difficulties or
# retaking a concept might legitimately generate a handful of quizzes in a
# few minutes; nobody legitimately generates 10 in one.
QUIZ_GENERATE_RATE_LIMIT = 8
QUIZ_GENERATE_RATE_WINDOW_SEC = 300 # 5 minutes

# Daily per-user LLM spend ceiling. The SPEND it measures is cross-feature
# (llm_usage records every agent call, not just quiz ones), but the ceiling
# is only ENFORCED on quiz generation — the one unbounded LLM call behind a
# button. Other entry points stay unguarded for now; moving this into a
# shared guard is its own piece of work, not something to imply here.
# A generation on the default flash-lite tier costs well under a cent, so
# this is ~2 orders of magnitude above any real study day — it exists to
# bound a runaway, not to ration normal use. Deliberately fail-OPEN: if the
# usage read errors we serve the quiz rather than denying every student.
QUIZ_DAILY_SPEND_CAP_USD = 2.00

# Wall-clock ceiling on one generation (agent run incl. its tool calls).
# Past this the student is staring at a spinner and would rather be told to
# try again; the request also stops holding a worker slot.
QUIZ_GENERATION_TIMEOUT_SEC = 90


# ── Generation honesty (#543 E2) ────────────────────────────────────────────
#
# Questions whose correct_answer doesn't match an option verbatim are
Expand Down
11 changes: 10 additions & 1 deletion backend/services/quiz_errors.py
Original file line numberDiff line numberDiff line change
Expand Up@@ -46,6 +46,14 @@ class QuizErrorCode(str, Enum):
QUIZ_QUESTION_INVALID = "QUIZ_QUESTION_INVALID"
QUIZ_NOT_AUTHORIZED = "QUIZ_NOT_AUTHORIZED"
QUIZ_GENERATION_FAILED = "QUIZ_GENERATION_FAILED"
# #544 F2: the generation exceeded its wall-clock budget. Distinct from
# the generic failure so the client can say "that took too long" and a
# retry is obviously worth offering.
QUIZ_GENERATION_TIMEOUT = "QUIZ_GENERATION_TIMEOUT"
# #544 F1: too many generations in the rate window.
QUIZ_RATE_LIMITED = "QUIZ_RATE_LIMITED"
# #544 F1: this account's daily LLM spend ceiling is reached.
QUIZ_DAILY_LIMIT_REACHED = "QUIZ_DAILY_LIMIT_REACHED"
QUIZ_INTERNAL_ERROR = "QUIZ_INTERNAL_ERROR"
# Uncoded HTTP errors that aren't one of the semantic states above —
# router 404s/405s on version-skewed clients, library-raised
Expand DownExpand Up@@ -80,8 +88,9 @@ def __init__(
code: QuizErrorCode,
message: str,
machine_detail=None,
headers: dict[str, str] | None = None,
):
super().__init__(status_code=status_code, detail=message)
super().__init__(status_code=status_code, detail=message, headers=headers)
self.code = code
self.machine_detail = machine_detail

Expand Down
14 changes: 14 additions & 0 deletions backend/services/request_limits.py
Original file line numberDiff line numberDiff line change
Expand Up@@ -37,6 +37,20 @@ def check_rate_limit(key: str, *, limit: int, window_sec: int) -> int | None:
return None


def refund_rate_limit(key: str) -> None:
"""Give back the most recent slot recorded for `key`.

For guards that must claim BEFORE doing the expensive work (so a
concurrent burst can't slip past the gate) but shouldn't charge the
caller when that work fails for reasons they didn't cause. Dropping
the newest timestamp — not the oldest — keeps the window's start
anchored to the caller's earliest real attempt.
"""
bucket = _rate_state.get(key)
if bucket:
bucket.pop()


async def read_within_limit(upload: UploadFile, max_bytes: int) -> bytes:
"""Read at most ``max_bytes`` (+1 to detect overflow) from an UploadFile so
an oversize upload can't be pulled fully into memory before we reject it.
Expand Down
13 changes: 13 additions & 0 deletions backend/tests/conftest.py
Original file line numberDiff line numberDiff line change
Expand Up@@ -47,6 +47,19 @@ def pytest_configure(config):
)


@pytest.fixture(autouse=True)
def _clear_rate_limit_state():
"""#544: services/request_limits keeps its sliding windows in a
process-global dict, so one test's burst of requests would throttle
every later test that hits the same route as the same user. Same
reasoning as the lru_cache reset below."""
from services import request_limits

request_limits._rate_state.clear()
yield
request_limits._rate_state.clear()


@pytest.fixture(autouse=True)
def _clear_lru_caches():
"""#98: reset the per-process lru_caches around every test so one test's
Expand Down
Loading
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { // Strip utm_, fbclid, gclid, etc. from all links on page (function() { var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content', 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid', 'ref', 'ref_src', 'source', 'medium', 'campaign']; function cleanUrl(url) { try { var u = new URL(url, window.location.origin); var changed = false; trackingParams.forEach(function(p) { if (u.searchParams.has(p)) { u.searchParams.delete(p); changed = true; } }); return changed ? u.toString() : url; } catch (e) { return url; } } function cleanLinks() { document.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } cleanLinks(); var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1) { if (node.tagName === 'A') cleanLinks(); node.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + ' feat(quiz): rate limit, daily spend guard, generation timeout, failure events (#544) by AndresL230 · Pull Request #552 · SaplingLearn/Sapling · GitHub
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions backend/db/connection.py
Original file line numberDiff line numberDiff line change
Expand Up@@ -34,14 +34,21 @@ def select(
filters: Optional[dict] = None,
order: Optional[str] = None,
limit: Optional[int] = None,
offset: Optional[int] = None,
) -> list:
"""Read rows. Pass `limit`/`offset` to page — PostgREST caps a
response at `max_rows` (1000) and answers 206 Partial Content,
which is a 2xx, so an unpaged read over that many rows truncates
silently."""
params: dict = {"select": columns}
if filters:
params.update(filters)
if order:
params["order"] = order
if limit:
params["limit"] = str(limit)
if offset is not None:
params["offset"] = str(offset)
r = _client.get(self.url, params=params)
r.raise_for_status()
return r.json()
Expand Down
8 changes: 7 additions & 1 deletion backend/main.py
Original file line numberDiff line numberDiff line change
Expand Up@@ -201,10 +201,16 @@ async def http_exception_handler(request: Request, exc: StarletteHTTPException):
code=getattr(exc, "code", None),
machine_detail=getattr(exc, "machine_detail", None),
)
# Preserve headers the raise site set (e.g. Retry-After on a 429) —
# dropping them would strip the only machine-readable part of a
# throttling response.
headers = dict(getattr(exc, "headers", None) or {})
if rid:
headers["X-Request-ID"] = rid
return JSONResponse(
status_code=exc.status_code,
content=content,
headers={"X-Request-ID": rid} if rid else {},
headers=headers,
)


Expand Down
7 changes: 4 additions & 3 deletions backend/routes/extract.py
Original file line numberDiff line numberDiff line change
Expand Up@@ -30,12 +30,13 @@ def _enforce_ocr_limits(request: Request) -> str:
user_id = get_session_user_id(request)
retry = check_rate_limit(f"ocr:{user_id}", limit=_OCR_RATE_LIMIT, window_sec=_OCR_RATE_WINDOW)
if retry is not None:
# NB: the app's global HTTPException handler (main.py) doesn't forward
# exc.headers, so the retry budget is conveyed in the detail string
# rather than a Retry-After header.
# main.py's HTTPException handler forwards exc.headers as of #544, so
# the budget rides in a real Retry-After. It stays in the detail
# string too — clients that only surface the message keep working.
raise HTTPException(
status_code=429,
detail=f"Too many OCR requests. Retry in {retry}s.",
headers={"Retry-After": str(retry)},
)
return user_id

Expand Down
6 changes: 4 additions & 2 deletions backend/routes/gradescope.py
Original file line numberDiff line numberDiff line change
Expand Up@@ -202,11 +202,13 @@ def _enforce_gs_rate_limit(user_id: str, action: str, *, limit: int, window_sec:
f"gradescope:{action}:{user_id}", limit=limit, window_sec=window_sec
)
if retry is not None:
# main.py's HTTPException handler drops exc.headers, so the retry budget
# rides in the detail string rather than a Retry-After header.
# main.py's HTTPException handler forwards exc.headers as of #544, so
# the budget rides in a real Retry-After. It stays in the detail
# string too — clients that only surface the message keep working.
raise HTTPException(
status_code=429,
detail=f"Too many Gradescope {action} requests. Retry in {retry}s.",
headers={"Retry-After": str(retry)},
)


Expand Down
151 changes: 149 additions & 2 deletions backend/routes/quiz.py
Original file line numberDiff line numberDiff line change
Expand Up@@ -25,12 +25,17 @@
from services.quiz_config import (
CONCRETE_DIFFICULTIES,
QUIZ_ATTEMPT_ABANDON_TTL_HOURS,
QUIZ_DAILY_SPEND_CAP_USD,
QUIZ_GENERATE_RATE_LIMIT,
QUIZ_GENERATE_RATE_WINDOW_SEC,
QUIZ_GENERATION_TIMEOUT_SEC,
QUIZ_TOPUP_DROP_RATIO,
QUIZ_TOPUP_MAX_RETRIES,
REQUESTED_DIFFICULTIES,
mastery_after,
quiz_config_payload,
)
from services.request_limits import check_rate_limit, refund_rate_limit
from services.quiz_errors import QuizAPIError, QuizErrorCode
from services.profiles import get_display_name
from services.encryption import encrypt_json, decrypt_json_column
Expand DownExpand Up@@ -87,6 +92,80 @@ def _load_prompt(name: str) -> str:
_MAX_HISTORY_OFFSET = 1_000_000


# supabase/config.toml sets PostgREST's max_rows = 1000, and an over-cap
# response is 206 Partial Content — a 2xx, so raise_for_status never fires
# and the truncation is silent. Same constant and same reasoning as
# achievement_service._daily_totals; page to completion or the sum is a lie.
_USAGE_PAGE = 1000


def _daily_spend_exceeded(user_id: str) -> bool:
"""True if this user is past the daily LLM spend ceiling (#544 F1).

Reads the llm_usage ledger agents/usage.py already writes, PAGED: an
unpaged read stops at max_rows, so a heavy user's sum plateaus below
the cap and the guard never trips for exactly the runaway it targets.
Stops early once the ceiling is crossed — the common case is a couple
of rows, and a user past the cap doesn't need an exact total.

Fails OPEN on any error: this is a cost control, not a correctness
gate, and denying every student because a usage read blipped is worse
than the spend it would save.
"""
try:
since = (datetime.now(timezone.utc) - timedelta(days=1)).isoformat()
spent = 0.0
offset = 0
while True:
rows = table("llm_usage").select(
"cost_usd",
filters={"user_id": f"eq.{user_id}", "created_at": f"gte.{since}"},
limit=_USAGE_PAGE,
offset=offset,
) or []
spent += sum(float(r.get("cost_usd") or 0.0) for r in rows)
if spent >= QUIZ_DAILY_SPEND_CAP_USD:
return True
if len(rows) < _USAGE_PAGE:
return False
offset += _USAGE_PAGE
except Exception:
logger.exception("quiz: daily spend check failed user=%s; allowing", user_id)
return False


def _refund_generate_slot(user_id: str) -> None:
"""Hand back the rate-limit slot a failed generation consumed (#544 F1).

The slot is claimed BEFORE the model runs (so a burst can't get past
the gate concurrently), which means a backend failure would otherwise
spend the student's quota: eight 502s in two minutes would lock them
out for five with a message saying they'd generated too many quizzes,
having received none. A failure the student didn't cause shouldn't
cost them anything, and the 502 explicitly invites a retry.
"""
try:
refund_rate_limit(f"quiz_generate:{user_id}")
except Exception:
logger.exception("quiz: rate-limit refund failed user=%s", user_id)


def _log_generation_failed(body, request_id: str | None, reason: str) -> None:
"""#544 F3: make a 502 the student saw a 502 an admin can count."""
events_service.log_event(
"quiz.generation_failed",
category="error",
user_id=body.user_id,
request_id=request_id,
payload={
"concept_node_id": body.concept_node_id,
"difficulty": body.difficulty,
"num_questions": body.num_questions,
"reason": reason,
},
)


def _abandon_cutoff() -> datetime:
return datetime.now(timezone.utc) - timedelta(
hours=QUIZ_ATTEMPT_ABANDON_TTL_HOURS
Expand DownExpand Up@@ -510,8 +589,17 @@ async def _quiz_via_agent(
run_kwargs["model"] = model_override

async def _run(message: str, limits) -> Quiz:
# #544 F2: bound EACH agent run rather than the whole function.
# Wrapping the outer coroutine cancelled it mid-flight, and
# CancelledError is a BaseException — it flew straight past the
# top-up's serve-what-we-have handler and threw away questions the
# student had already paid for. Timing out one run raises an
# ordinary TimeoutError the existing handlers can reason about.
result = record_agent_usage(
await quiz_agent.run(message, usage_limits=limits, **run_kwargs),
await asyncio.wait_for(
quiz_agent.run(message, usage_limits=limits, **run_kwargs),
timeout=QUIZ_GENERATION_TIMEOUT_SEC,
),
feature="quiz", task="quiz", user_id=deps.user_id,
)
return result.output
Expand DownExpand Up@@ -582,7 +670,7 @@ def _absorb(quiz: Quiz) -> None:
)
try:
_absorb(await _run(topup_msg, TOPUP_LIMITS))
except Exception as e:
except (Exception, asyncio.TimeoutError) as e:
# The request deliberately SUCCEEDS from here — serve the
# short quiz with an honest count. No traceback: the E2E
# logscan oracle reports those as findings, and this path
Expand DownExpand Up@@ -652,7 +740,44 @@ async def generate_quiz(body: GenerateQuizBody, request: Request):
or str(uuid.uuid4())
)

# #544 F1: cost guards run AFTER ownership (a stranger's node 404s
# first, so probing can't consume a victim's quota) and BEFORE the
# model call. Neither rejection is a backend failure, so neither emits
# quiz.generation_failed.
retry_after = check_rate_limit(
f"quiz_generate:{body.user_id}",
limit=QUIZ_GENERATE_RATE_LIMIT,
window_sec=QUIZ_GENERATE_RATE_WINDOW_SEC,
)
if retry_after is not None:
raise QuizAPIError(
status_code=429,
code=QuizErrorCode.QUIZ_RATE_LIMITED,
message=(
"You've generated a lot of quizzes just now — "
"take a moment and try again shortly."
),
headers={"Retry-After": str(retry_after)},
)
if _daily_spend_exceeded(body.user_id):
logger.warning(
"quiz: daily spend cap reached user=%s request_id=%s",
body.user_id, request_id,
)
raise QuizAPIError(
status_code=429,
code=QuizErrorCode.QUIZ_DAILY_LIMIT_REACHED,
message=(
"You've reached today's limit for AI-generated study "
"material. It resets tomorrow."
),
)

try:
# Each agent run inside is individually bounded by
# QUIZ_GENERATION_TIMEOUT_SEC (see _run) — cancelling the whole
# coroutine here would discard a partial quiz the top-up handler
# is designed to serve.
questions = await _quiz_via_agent(
user_id=body.user_id,
course_id=course_id,
Expand All@@ -667,18 +792,40 @@ async def generate_quiz(body: GenerateQuizBody, request: Request):
except HTTPException:
# The 404 for an unknown concept node is raised before the agent call;
# never swallow a known HTTP state.
_refund_generate_slot(body.user_id)
raise
except asyncio.TimeoutError as e:
# #544 F2: distinct from a generic failure — the client can say
# "that took too long" and offering a retry obviously makes sense.
# NB: only asyncio.TimeoutError. The builtin TimeoutError is in the
# OSError family, so catching it too would relabel a transport
# socket timeout as a wall-clock generation timeout.
logger.warning(
"quiz: generation timed out after %ss request_id=%s",
QUIZ_GENERATION_TIMEOUT_SEC, request_id,
)
_refund_generate_slot(body.user_id)
_log_generation_failed(body, request_id, "timeout")
raise QuizAPIError(
status_code=502,
code=QuizErrorCode.QUIZ_GENERATION_TIMEOUT,
message="Quiz generation took too long. Please try again.",
) from e
except (UsageLimitExceeded, UnexpectedModelBehavior) as e:
# The raw-Gemini legacy fallback was retired in #145; degrade to 502
# rather than serving a quiz from a second LLM path.
logger.warning("Quiz agent guardrails tripped; returning 502", exc_info=e)
_refund_generate_slot(body.user_id)
_log_generation_failed(body, request_id, "agent_guardrail")
raise QuizAPIError(
status_code=502,
code=QuizErrorCode.QUIZ_GENERATION_FAILED,
message="Quiz generation is temporarily unavailable. Please try again.",
) from e
except Exception as e:
logger.exception("Unexpected quiz-agent failure; returning 502")
_refund_generate_slot(body.user_id)
_log_generation_failed(body, request_id, "agent_error")
raise QuizAPIError(
status_code=502,
code=QuizErrorCode.QUIZ_GENERATION_FAILED,
Expand Down
4 changes: 4 additions & 0 deletions backend/services/events_service.py
Original file line numberDiff line numberDiff line change
Expand Up@@ -86,6 +86,10 @@
# surfaces in admin analytics — this failure was invisible for months
# precisely because nothing emitted when the background task died.
"quiz.context_write_failed",
# #544/F3: generation failed (agent error, timeout, or every question
# dropped). Same reasoning: a 502 the student sees should be a 502 an
# admin can count.
"quiz.generation_failed",
"chat.message_sent",
"note.created",
"session.started",
Expand Down
28 changes: 28 additions & 0 deletions backend/services/quiz_config.py
Original file line numberDiff line numberDiff line change
Expand Up@@ -79,6 +79,34 @@ def mastery_after(before: float, *, score: int, total: int) -> float:
return max(0.0, min(1.0, raw))


# ── Cost + abuse guards (#544 F1/F2) ────────────────────────────────────────
#
# Generation is an unbounded LLM call behind a button: before #544 nothing
# stopped a held-down key or a scripted loop from spending real money.
#
# The rate limit is sized for a human: a student comparing difficulties or
# retaking a concept might legitimately generate a handful of quizzes in a
# few minutes; nobody legitimately generates 10 in one.
QUIZ_GENERATE_RATE_LIMIT = 8
QUIZ_GENERATE_RATE_WINDOW_SEC = 300 # 5 minutes

# Daily per-user LLM spend ceiling. The SPEND it measures is cross-feature
# (llm_usage records every agent call, not just quiz ones), but the ceiling
# is only ENFORCED on quiz generation — the one unbounded LLM call behind a
# button. Other entry points stay unguarded for now; moving this into a
# shared guard is its own piece of work, not something to imply here.
# A generation on the default flash-lite tier costs well under a cent, so
# this is ~2 orders of magnitude above any real study day — it exists to
# bound a runaway, not to ration normal use. Deliberately fail-OPEN: if the
# usage read errors we serve the quiz rather than denying every student.
QUIZ_DAILY_SPEND_CAP_USD = 2.00

# Wall-clock ceiling on one generation (agent run incl. its tool calls).
# Past this the student is staring at a spinner and would rather be told to
# try again; the request also stops holding a worker slot.
QUIZ_GENERATION_TIMEOUT_SEC = 90


# ── Generation honesty (#543 E2) ────────────────────────────────────────────
#
# Questions whose correct_answer doesn't match an option verbatim are
Expand Down
11 changes: 10 additions & 1 deletion backend/services/quiz_errors.py
Original file line numberDiff line numberDiff line change
Expand Up@@ -46,6 +46,14 @@ class QuizErrorCode(str, Enum):
QUIZ_QUESTION_INVALID = "QUIZ_QUESTION_INVALID"
QUIZ_NOT_AUTHORIZED = "QUIZ_NOT_AUTHORIZED"
QUIZ_GENERATION_FAILED = "QUIZ_GENERATION_FAILED"
# #544 F2: the generation exceeded its wall-clock budget. Distinct from
# the generic failure so the client can say "that took too long" and a
# retry is obviously worth offering.
QUIZ_GENERATION_TIMEOUT = "QUIZ_GENERATION_TIMEOUT"
# #544 F1: too many generations in the rate window.
QUIZ_RATE_LIMITED = "QUIZ_RATE_LIMITED"
# #544 F1: this account's daily LLM spend ceiling is reached.
QUIZ_DAILY_LIMIT_REACHED = "QUIZ_DAILY_LIMIT_REACHED"
QUIZ_INTERNAL_ERROR = "QUIZ_INTERNAL_ERROR"
# Uncoded HTTP errors that aren't one of the semantic states above —
# router 404s/405s on version-skewed clients, library-raised
Expand DownExpand Up@@ -80,8 +88,9 @@ def __init__(
code: QuizErrorCode,
message: str,
machine_detail=None,
headers: dict[str, str] | None = None,
):
super().__init__(status_code=status_code, detail=message)
super().__init__(status_code=status_code, detail=message, headers=headers)
self.code = code
self.machine_detail = machine_detail

Expand Down
14 changes: 14 additions & 0 deletions backend/services/request_limits.py
Original file line numberDiff line numberDiff line change
Expand Up@@ -37,6 +37,20 @@ def check_rate_limit(key: str, *, limit: int, window_sec: int) -> int | None:
return None


def refund_rate_limit(key: str) -> None:
"""Give back the most recent slot recorded for `key`.

For guards that must claim BEFORE doing the expensive work (so a
concurrent burst can't slip past the gate) but shouldn't charge the
caller when that work fails for reasons they didn't cause. Dropping
the newest timestamp — not the oldest — keeps the window's start
anchored to the caller's earliest real attempt.
"""
bucket = _rate_state.get(key)
if bucket:
bucket.pop()


async def read_within_limit(upload: UploadFile, max_bytes: int) -> bytes:
"""Read at most ``max_bytes`` (+1 to detect overflow) from an UploadFile so
an oversize upload can't be pulled fully into memory before we reject it.
Expand Down
13 changes: 13 additions & 0 deletions backend/tests/conftest.py
Original file line numberDiff line numberDiff line change
Expand Up@@ -47,6 +47,19 @@ def pytest_configure(config):
)


@pytest.fixture(autouse=True)
def _clear_rate_limit_state():
"""#544: services/request_limits keeps its sliding windows in a
process-global dict, so one test's burst of requests would throttle
every later test that hits the same route as the same user. Same
reasoning as the lru_cache reset below."""
from services import request_limits

request_limits._rate_state.clear()
yield
request_limits._rate_state.clear()


@pytest.fixture(autouse=True)
def _clear_lru_caches():
"""#98: reset the per-process lru_caches around every test so one test's
Expand Down
Loading
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { // Auto-enable theater mode on YouTube (function() { function tryTheater() { var btn = document.querySelector('button[aria-label="Theater mode"], ytd-player #player button[title="Theater mode"]'); if (btn && !btn.classList.contains('activated')) { btn.click(); } } // Try immediately tryTheater(); // Try after navigation (SPA) var lastUrl = location.href; setInterval(function() { if (location.href !== lastUrl) { lastUrl = location.href; setTimeout(tryTheater, 500); } }, 1000); // Also try on player load var observer = new MutationObserver(tryTheater); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' feat(quiz): rate limit, daily spend guard, generation timeout, failure events (#544) by AndresL230 · Pull Request #552 · SaplingLearn/Sapling · GitHub
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions backend/db/connection.py
Original file line numberDiff line numberDiff line change
Expand Up@@ -34,14 +34,21 @@ def select(
filters: Optional[dict] = None,
order: Optional[str] = None,
limit: Optional[int] = None,
offset: Optional[int] = None,
) -> list:
"""Read rows. Pass `limit`/`offset` to page — PostgREST caps a
response at `max_rows` (1000) and answers 206 Partial Content,
which is a 2xx, so an unpaged read over that many rows truncates
silently."""
params: dict = {"select": columns}
if filters:
params.update(filters)
if order:
params["order"] = order
if limit:
params["limit"] = str(limit)
if offset is not None:
params["offset"] = str(offset)
r = _client.get(self.url, params=params)
r.raise_for_status()
return r.json()
Expand Down
8 changes: 7 additions & 1 deletion backend/main.py
Original file line numberDiff line numberDiff line change
Expand Up@@ -201,10 +201,16 @@ async def http_exception_handler(request: Request, exc: StarletteHTTPException):
code=getattr(exc, "code", None),
machine_detail=getattr(exc, "machine_detail", None),
)
# Preserve headers the raise site set (e.g. Retry-After on a 429) —
# dropping them would strip the only machine-readable part of a
# throttling response.
headers = dict(getattr(exc, "headers", None) or {})
if rid:
headers["X-Request-ID"] = rid
return JSONResponse(
status_code=exc.status_code,
content=content,
headers={"X-Request-ID": rid} if rid else {},
headers=headers,
)


Expand Down
7 changes: 4 additions & 3 deletions backend/routes/extract.py
Original file line numberDiff line numberDiff line change
Expand Up@@ -30,12 +30,13 @@ def _enforce_ocr_limits(request: Request) -> str:
user_id = get_session_user_id(request)
retry = check_rate_limit(f"ocr:{user_id}", limit=_OCR_RATE_LIMIT, window_sec=_OCR_RATE_WINDOW)
if retry is not None:
# NB: the app's global HTTPException handler (main.py) doesn't forward
# exc.headers, so the retry budget is conveyed in the detail string
# rather than a Retry-After header.
# main.py's HTTPException handler forwards exc.headers as of #544, so
# the budget rides in a real Retry-After. It stays in the detail
# string too — clients that only surface the message keep working.
raise HTTPException(
status_code=429,
detail=f"Too many OCR requests. Retry in {retry}s.",
headers={"Retry-After": str(retry)},
)
return user_id

Expand Down
6 changes: 4 additions & 2 deletions backend/routes/gradescope.py
Original file line numberDiff line numberDiff line change
Expand Up@@ -202,11 +202,13 @@ def _enforce_gs_rate_limit(user_id: str, action: str, *, limit: int, window_sec:
f"gradescope:{action}:{user_id}", limit=limit, window_sec=window_sec
)
if retry is not None:
# main.py's HTTPException handler drops exc.headers, so the retry budget
# rides in the detail string rather than a Retry-After header.
# main.py's HTTPException handler forwards exc.headers as of #544, so
# the budget rides in a real Retry-After. It stays in the detail
# string too — clients that only surface the message keep working.
raise HTTPException(
status_code=429,
detail=f"Too many Gradescope {action} requests. Retry in {retry}s.",
headers={"Retry-After": str(retry)},
)


Expand Down
151 changes: 149 additions & 2 deletions backend/routes/quiz.py
Original file line numberDiff line numberDiff line change
Expand Up@@ -25,12 +25,17 @@
from services.quiz_config import (
CONCRETE_DIFFICULTIES,
QUIZ_ATTEMPT_ABANDON_TTL_HOURS,
QUIZ_DAILY_SPEND_CAP_USD,
QUIZ_GENERATE_RATE_LIMIT,
QUIZ_GENERATE_RATE_WINDOW_SEC,
QUIZ_GENERATION_TIMEOUT_SEC,
QUIZ_TOPUP_DROP_RATIO,
QUIZ_TOPUP_MAX_RETRIES,
REQUESTED_DIFFICULTIES,
mastery_after,
quiz_config_payload,
)
from services.request_limits import check_rate_limit, refund_rate_limit
from services.quiz_errors import QuizAPIError, QuizErrorCode
from services.profiles import get_display_name
from services.encryption import encrypt_json, decrypt_json_column
Expand DownExpand Up@@ -87,6 +92,80 @@ def _load_prompt(name: str) -> str:
_MAX_HISTORY_OFFSET = 1_000_000


# supabase/config.toml sets PostgREST's max_rows = 1000, and an over-cap
# response is 206 Partial Content — a 2xx, so raise_for_status never fires
# and the truncation is silent. Same constant and same reasoning as
# achievement_service._daily_totals; page to completion or the sum is a lie.
_USAGE_PAGE = 1000


def _daily_spend_exceeded(user_id: str) -> bool:
"""True if this user is past the daily LLM spend ceiling (#544 F1).

Reads the llm_usage ledger agents/usage.py already writes, PAGED: an
unpaged read stops at max_rows, so a heavy user's sum plateaus below
the cap and the guard never trips for exactly the runaway it targets.
Stops early once the ceiling is crossed — the common case is a couple
of rows, and a user past the cap doesn't need an exact total.

Fails OPEN on any error: this is a cost control, not a correctness
gate, and denying every student because a usage read blipped is worse
than the spend it would save.
"""
try:
since = (datetime.now(timezone.utc) - timedelta(days=1)).isoformat()
spent = 0.0
offset = 0
while True:
rows = table("llm_usage").select(
"cost_usd",
filters={"user_id": f"eq.{user_id}", "created_at": f"gte.{since}"},
limit=_USAGE_PAGE,
offset=offset,
) or []
spent += sum(float(r.get("cost_usd") or 0.0) for r in rows)
if spent >= QUIZ_DAILY_SPEND_CAP_USD:
return True
if len(rows) < _USAGE_PAGE:
return False
offset += _USAGE_PAGE
except Exception:
logger.exception("quiz: daily spend check failed user=%s; allowing", user_id)
return False


def _refund_generate_slot(user_id: str) -> None:
"""Hand back the rate-limit slot a failed generation consumed (#544 F1).

The slot is claimed BEFORE the model runs (so a burst can't get past
the gate concurrently), which means a backend failure would otherwise
spend the student's quota: eight 502s in two minutes would lock them
out for five with a message saying they'd generated too many quizzes,
having received none. A failure the student didn't cause shouldn't
cost them anything, and the 502 explicitly invites a retry.
"""
try:
refund_rate_limit(f"quiz_generate:{user_id}")
except Exception:
logger.exception("quiz: rate-limit refund failed user=%s", user_id)


def _log_generation_failed(body, request_id: str | None, reason: str) -> None:
"""#544 F3: make a 502 the student saw a 502 an admin can count."""
events_service.log_event(
"quiz.generation_failed",
category="error",
user_id=body.user_id,
request_id=request_id,
payload={
"concept_node_id": body.concept_node_id,
"difficulty": body.difficulty,
"num_questions": body.num_questions,
"reason": reason,
},
)


def _abandon_cutoff() -> datetime:
return datetime.now(timezone.utc) - timedelta(
hours=QUIZ_ATTEMPT_ABANDON_TTL_HOURS
Expand DownExpand Up@@ -510,8 +589,17 @@ async def _quiz_via_agent(
run_kwargs["model"] = model_override

async def _run(message: str, limits) -> Quiz:
# #544 F2: bound EACH agent run rather than the whole function.
# Wrapping the outer coroutine cancelled it mid-flight, and
# CancelledError is a BaseException — it flew straight past the
# top-up's serve-what-we-have handler and threw away questions the
# student had already paid for. Timing out one run raises an
# ordinary TimeoutError the existing handlers can reason about.
result = record_agent_usage(
await quiz_agent.run(message, usage_limits=limits, **run_kwargs),
await asyncio.wait_for(
quiz_agent.run(message, usage_limits=limits, **run_kwargs),
timeout=QUIZ_GENERATION_TIMEOUT_SEC,
),
feature="quiz", task="quiz", user_id=deps.user_id,
)
return result.output
Expand DownExpand Up@@ -582,7 +670,7 @@ def _absorb(quiz: Quiz) -> None:
)
try:
_absorb(await _run(topup_msg, TOPUP_LIMITS))
except Exception as e:
except (Exception, asyncio.TimeoutError) as e:
# The request deliberately SUCCEEDS from here — serve the
# short quiz with an honest count. No traceback: the E2E
# logscan oracle reports those as findings, and this path
Expand DownExpand Up@@ -652,7 +740,44 @@ async def generate_quiz(body: GenerateQuizBody, request: Request):
or str(uuid.uuid4())
)

# #544 F1: cost guards run AFTER ownership (a stranger's node 404s
# first, so probing can't consume a victim's quota) and BEFORE the
# model call. Neither rejection is a backend failure, so neither emits
# quiz.generation_failed.
retry_after = check_rate_limit(
f"quiz_generate:{body.user_id}",
limit=QUIZ_GENERATE_RATE_LIMIT,
window_sec=QUIZ_GENERATE_RATE_WINDOW_SEC,
)
if retry_after is not None:
raise QuizAPIError(
status_code=429,
code=QuizErrorCode.QUIZ_RATE_LIMITED,
message=(
"You've generated a lot of quizzes just now — "
"take a moment and try again shortly."
),
headers={"Retry-After": str(retry_after)},
)
if _daily_spend_exceeded(body.user_id):
logger.warning(
"quiz: daily spend cap reached user=%s request_id=%s",
body.user_id, request_id,
)
raise QuizAPIError(
status_code=429,
code=QuizErrorCode.QUIZ_DAILY_LIMIT_REACHED,
message=(
"You've reached today's limit for AI-generated study "
"material. It resets tomorrow."
),
)

try:
# Each agent run inside is individually bounded by
# QUIZ_GENERATION_TIMEOUT_SEC (see _run) — cancelling the whole
# coroutine here would discard a partial quiz the top-up handler
# is designed to serve.
questions = await _quiz_via_agent(
user_id=body.user_id,
course_id=course_id,
Expand All@@ -667,18 +792,40 @@ async def generate_quiz(body: GenerateQuizBody, request: Request):
except HTTPException:
# The 404 for an unknown concept node is raised before the agent call;
# never swallow a known HTTP state.
_refund_generate_slot(body.user_id)
raise
except asyncio.TimeoutError as e:
# #544 F2: distinct from a generic failure — the client can say
# "that took too long" and offering a retry obviously makes sense.
# NB: only asyncio.TimeoutError. The builtin TimeoutError is in the
# OSError family, so catching it too would relabel a transport
# socket timeout as a wall-clock generation timeout.
logger.warning(
"quiz: generation timed out after %ss request_id=%s",
QUIZ_GENERATION_TIMEOUT_SEC, request_id,
)
_refund_generate_slot(body.user_id)
_log_generation_failed(body, request_id, "timeout")
raise QuizAPIError(
status_code=502,
code=QuizErrorCode.QUIZ_GENERATION_TIMEOUT,
message="Quiz generation took too long. Please try again.",
) from e
except (UsageLimitExceeded, UnexpectedModelBehavior) as e:
# The raw-Gemini legacy fallback was retired in #145; degrade to 502
# rather than serving a quiz from a second LLM path.
logger.warning("Quiz agent guardrails tripped; returning 502", exc_info=e)
_refund_generate_slot(body.user_id)
_log_generation_failed(body, request_id, "agent_guardrail")
raise QuizAPIError(
status_code=502,
code=QuizErrorCode.QUIZ_GENERATION_FAILED,
message="Quiz generation is temporarily unavailable. Please try again.",
) from e
except Exception as e:
logger.exception("Unexpected quiz-agent failure; returning 502")
_refund_generate_slot(body.user_id)
_log_generation_failed(body, request_id, "agent_error")
raise QuizAPIError(
status_code=502,
code=QuizErrorCode.QUIZ_GENERATION_FAILED,
Expand Down
4 changes: 4 additions & 0 deletions backend/services/events_service.py
Original file line numberDiff line numberDiff line change
Expand Up@@ -86,6 +86,10 @@
# surfaces in admin analytics — this failure was invisible for months
# precisely because nothing emitted when the background task died.
"quiz.context_write_failed",
# #544/F3: generation failed (agent error, timeout, or every question
# dropped). Same reasoning: a 502 the student sees should be a 502 an
# admin can count.
"quiz.generation_failed",
"chat.message_sent",
"note.created",
"session.started",
Expand Down
28 changes: 28 additions & 0 deletions backend/services/quiz_config.py
Original file line numberDiff line numberDiff line change
Expand Up@@ -79,6 +79,34 @@ def mastery_after(before: float, *, score: int, total: int) -> float:
return max(0.0, min(1.0, raw))


# ── Cost + abuse guards (#544 F1/F2) ────────────────────────────────────────
#
# Generation is an unbounded LLM call behind a button: before #544 nothing
# stopped a held-down key or a scripted loop from spending real money.
#
# The rate limit is sized for a human: a student comparing difficulties or
# retaking a concept might legitimately generate a handful of quizzes in a
# few minutes; nobody legitimately generates 10 in one.
QUIZ_GENERATE_RATE_LIMIT = 8
QUIZ_GENERATE_RATE_WINDOW_SEC = 300 # 5 minutes

# Daily per-user LLM spend ceiling. The SPEND it measures is cross-feature
# (llm_usage records every agent call, not just quiz ones), but the ceiling
# is only ENFORCED on quiz generation — the one unbounded LLM call behind a
# button. Other entry points stay unguarded for now; moving this into a
# shared guard is its own piece of work, not something to imply here.
# A generation on the default flash-lite tier costs well under a cent, so
# this is ~2 orders of magnitude above any real study day — it exists to
# bound a runaway, not to ration normal use. Deliberately fail-OPEN: if the
# usage read errors we serve the quiz rather than denying every student.
QUIZ_DAILY_SPEND_CAP_USD = 2.00

# Wall-clock ceiling on one generation (agent run incl. its tool calls).
# Past this the student is staring at a spinner and would rather be told to
# try again; the request also stops holding a worker slot.
QUIZ_GENERATION_TIMEOUT_SEC = 90


# ── Generation honesty (#543 E2) ────────────────────────────────────────────
#
# Questions whose correct_answer doesn't match an option verbatim are
Expand Down
11 changes: 10 additions & 1 deletion backend/services/quiz_errors.py
Original file line numberDiff line numberDiff line change
Expand Up@@ -46,6 +46,14 @@ class QuizErrorCode(str, Enum):
QUIZ_QUESTION_INVALID = "QUIZ_QUESTION_INVALID"
QUIZ_NOT_AUTHORIZED = "QUIZ_NOT_AUTHORIZED"
QUIZ_GENERATION_FAILED = "QUIZ_GENERATION_FAILED"
# #544 F2: the generation exceeded its wall-clock budget. Distinct from
# the generic failure so the client can say "that took too long" and a
# retry is obviously worth offering.
QUIZ_GENERATION_TIMEOUT = "QUIZ_GENERATION_TIMEOUT"
# #544 F1: too many generations in the rate window.
QUIZ_RATE_LIMITED = "QUIZ_RATE_LIMITED"
# #544 F1: this account's daily LLM spend ceiling is reached.
QUIZ_DAILY_LIMIT_REACHED = "QUIZ_DAILY_LIMIT_REACHED"
QUIZ_INTERNAL_ERROR = "QUIZ_INTERNAL_ERROR"
# Uncoded HTTP errors that aren't one of the semantic states above —
# router 404s/405s on version-skewed clients, library-raised
Expand DownExpand Up@@ -80,8 +88,9 @@ def __init__(
code: QuizErrorCode,
message: str,
machine_detail=None,
headers: dict[str, str] | None = None,
):
super().__init__(status_code=status_code, detail=message)
super().__init__(status_code=status_code, detail=message, headers=headers)
self.code = code
self.machine_detail = machine_detail

Expand Down
14 changes: 14 additions & 0 deletions backend/services/request_limits.py
Original file line numberDiff line numberDiff line change
Expand Up@@ -37,6 +37,20 @@ def check_rate_limit(key: str, *, limit: int, window_sec: int) -> int | None:
return None


def refund_rate_limit(key: str) -> None:
"""Give back the most recent slot recorded for `key`.

For guards that must claim BEFORE doing the expensive work (so a
concurrent burst can't slip past the gate) but shouldn't charge the
caller when that work fails for reasons they didn't cause. Dropping
the newest timestamp — not the oldest — keeps the window's start
anchored to the caller's earliest real attempt.
"""
bucket = _rate_state.get(key)
if bucket:
bucket.pop()


async def read_within_limit(upload: UploadFile, max_bytes: int) -> bytes:
"""Read at most ``max_bytes`` (+1 to detect overflow) from an UploadFile so
an oversize upload can't be pulled fully into memory before we reject it.
Expand Down
13 changes: 13 additions & 0 deletions backend/tests/conftest.py
Original file line numberDiff line numberDiff line change
Expand Up@@ -47,6 +47,19 @@ def pytest_configure(config):
)


@pytest.fixture(autouse=True)
def _clear_rate_limit_state():
"""#544: services/request_limits keeps its sliding windows in a
process-global dict, so one test's burst of requests would throttle
every later test that hits the same route as the same user. Same
reasoning as the lru_cache reset below."""
from services import request_limits

request_limits._rate_state.clear()
yield
request_limits._rate_state.clear()


@pytest.fixture(autouse=True)
def _clear_lru_caches():
"""#98: reset the per-process lru_caches around every test so one test's
Expand Down
Loading
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { // Remove or un-stick sticky/fixed headers that block content (function() { function unstick() { document.querySelectorAll('header, nav, [role="banner"], .header, .navbar, .sticky, .fixed-top, [style*="position: fixed"], [style*="position:sticky"]').forEach(function(el) { if (el.style.position === 'fixed' || el.style.position === 'sticky' || getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') { el.style.position = 'static'; el.style.top = 'auto'; el.style.zIndex = 'auto'; } }); } unstick(); var observer = new MutationObserver(unstick); observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] }); })(); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' feat(quiz): rate limit, daily spend guard, generation timeout, failure events (#544) by AndresL230 · Pull Request #552 · SaplingLearn/Sapling · GitHub
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions backend/db/connection.py
Original file line numberDiff line numberDiff line change
Expand Up@@ -34,14 +34,21 @@ def select(
filters: Optional[dict] = None,
order: Optional[str] = None,
limit: Optional[int] = None,
offset: Optional[int] = None,
) -> list:
"""Read rows. Pass `limit`/`offset` to page — PostgREST caps a
response at `max_rows` (1000) and answers 206 Partial Content,
which is a 2xx, so an unpaged read over that many rows truncates
silently."""
params: dict = {"select": columns}
if filters:
params.update(filters)
if order:
params["order"] = order
if limit:
params["limit"] = str(limit)
if offset is not None:
params["offset"] = str(offset)
r = _client.get(self.url, params=params)
r.raise_for_status()
return r.json()
Expand Down
8 changes: 7 additions & 1 deletion backend/main.py
Original file line numberDiff line numberDiff line change
Expand Up@@ -201,10 +201,16 @@ async def http_exception_handler(request: Request, exc: StarletteHTTPException):
code=getattr(exc, "code", None),
machine_detail=getattr(exc, "machine_detail", None),
)
# Preserve headers the raise site set (e.g. Retry-After on a 429) —
# dropping them would strip the only machine-readable part of a
# throttling response.
headers = dict(getattr(exc, "headers", None) or {})
if rid:
headers["X-Request-ID"] = rid
return JSONResponse(
status_code=exc.status_code,
content=content,
headers={"X-Request-ID": rid} if rid else {},
headers=headers,
)


Expand Down
7 changes: 4 additions & 3 deletions backend/routes/extract.py
Original file line numberDiff line numberDiff line change
Expand Up@@ -30,12 +30,13 @@ def _enforce_ocr_limits(request: Request) -> str:
user_id = get_session_user_id(request)
retry = check_rate_limit(f"ocr:{user_id}", limit=_OCR_RATE_LIMIT, window_sec=_OCR_RATE_WINDOW)
if retry is not None:
# NB: the app's global HTTPException handler (main.py) doesn't forward
# exc.headers, so the retry budget is conveyed in the detail string
# rather than a Retry-After header.
# main.py's HTTPException handler forwards exc.headers as of #544, so
# the budget rides in a real Retry-After. It stays in the detail
# string too — clients that only surface the message keep working.
raise HTTPException(
status_code=429,
detail=f"Too many OCR requests. Retry in {retry}s.",
headers={"Retry-After": str(retry)},
)
return user_id

Expand Down
6 changes: 4 additions & 2 deletions backend/routes/gradescope.py
Original file line numberDiff line numberDiff line change
Expand Up@@ -202,11 +202,13 @@ def _enforce_gs_rate_limit(user_id: str, action: str, *, limit: int, window_sec:
f"gradescope:{action}:{user_id}", limit=limit, window_sec=window_sec
)
if retry is not None:
# main.py's HTTPException handler drops exc.headers, so the retry budget
# rides in the detail string rather than a Retry-After header.
# main.py's HTTPException handler forwards exc.headers as of #544, so
# the budget rides in a real Retry-After. It stays in the detail
# string too — clients that only surface the message keep working.
raise HTTPException(
status_code=429,
detail=f"Too many Gradescope {action} requests. Retry in {retry}s.",
headers={"Retry-After": str(retry)},
)


Expand Down
151 changes: 149 additions & 2 deletions backend/routes/quiz.py
Original file line numberDiff line numberDiff line change
Expand Up@@ -25,12 +25,17 @@
from services.quiz_config import (
CONCRETE_DIFFICULTIES,
QUIZ_ATTEMPT_ABANDON_TTL_HOURS,
QUIZ_DAILY_SPEND_CAP_USD,
QUIZ_GENERATE_RATE_LIMIT,
QUIZ_GENERATE_RATE_WINDOW_SEC,
QUIZ_GENERATION_TIMEOUT_SEC,
QUIZ_TOPUP_DROP_RATIO,
QUIZ_TOPUP_MAX_RETRIES,
REQUESTED_DIFFICULTIES,
mastery_after,
quiz_config_payload,
)
from services.request_limits import check_rate_limit, refund_rate_limit
from services.quiz_errors import QuizAPIError, QuizErrorCode
from services.profiles import get_display_name
from services.encryption import encrypt_json, decrypt_json_column
Expand DownExpand Up@@ -87,6 +92,80 @@ def _load_prompt(name: str) -> str:
_MAX_HISTORY_OFFSET = 1_000_000


# supabase/config.toml sets PostgREST's max_rows = 1000, and an over-cap
# response is 206 Partial Content — a 2xx, so raise_for_status never fires
# and the truncation is silent. Same constant and same reasoning as
# achievement_service._daily_totals; page to completion or the sum is a lie.
_USAGE_PAGE = 1000


def _daily_spend_exceeded(user_id: str) -> bool:
"""True if this user is past the daily LLM spend ceiling (#544 F1).

Reads the llm_usage ledger agents/usage.py already writes, PAGED: an
unpaged read stops at max_rows, so a heavy user's sum plateaus below
the cap and the guard never trips for exactly the runaway it targets.
Stops early once the ceiling is crossed — the common case is a couple
of rows, and a user past the cap doesn't need an exact total.

Fails OPEN on any error: this is a cost control, not a correctness
gate, and denying every student because a usage read blipped is worse
than the spend it would save.
"""
try:
since = (datetime.now(timezone.utc) - timedelta(days=1)).isoformat()
spent = 0.0
offset = 0
while True:
rows = table("llm_usage").select(
"cost_usd",
filters={"user_id": f"eq.{user_id}", "created_at": f"gte.{since}"},
limit=_USAGE_PAGE,
offset=offset,
) or []
spent += sum(float(r.get("cost_usd") or 0.0) for r in rows)
if spent >= QUIZ_DAILY_SPEND_CAP_USD:
return True
if len(rows) < _USAGE_PAGE:
return False
offset += _USAGE_PAGE
except Exception:
logger.exception("quiz: daily spend check failed user=%s; allowing", user_id)
return False


def _refund_generate_slot(user_id: str) -> None:
"""Hand back the rate-limit slot a failed generation consumed (#544 F1).

The slot is claimed BEFORE the model runs (so a burst can't get past
the gate concurrently), which means a backend failure would otherwise
spend the student's quota: eight 502s in two minutes would lock them
out for five with a message saying they'd generated too many quizzes,
having received none. A failure the student didn't cause shouldn't
cost them anything, and the 502 explicitly invites a retry.
"""
try:
refund_rate_limit(f"quiz_generate:{user_id}")
except Exception:
logger.exception("quiz: rate-limit refund failed user=%s", user_id)


def _log_generation_failed(body, request_id: str | None, reason: str) -> None:
"""#544 F3: make a 502 the student saw a 502 an admin can count."""
events_service.log_event(
"quiz.generation_failed",
category="error",
user_id=body.user_id,
request_id=request_id,
payload={
"concept_node_id": body.concept_node_id,
"difficulty": body.difficulty,
"num_questions": body.num_questions,
"reason": reason,
},
)


def _abandon_cutoff() -> datetime:
return datetime.now(timezone.utc) - timedelta(
hours=QUIZ_ATTEMPT_ABANDON_TTL_HOURS
Expand DownExpand Up@@ -510,8 +589,17 @@ async def _quiz_via_agent(
run_kwargs["model"] = model_override

async def _run(message: str, limits) -> Quiz:
# #544 F2: bound EACH agent run rather than the whole function.
# Wrapping the outer coroutine cancelled it mid-flight, and
# CancelledError is a BaseException — it flew straight past the
# top-up's serve-what-we-have handler and threw away questions the
# student had already paid for. Timing out one run raises an
# ordinary TimeoutError the existing handlers can reason about.
result = record_agent_usage(
await quiz_agent.run(message, usage_limits=limits, **run_kwargs),
await asyncio.wait_for(
quiz_agent.run(message, usage_limits=limits, **run_kwargs),
timeout=QUIZ_GENERATION_TIMEOUT_SEC,
),
feature="quiz", task="quiz", user_id=deps.user_id,
)
return result.output
Expand DownExpand Up@@ -582,7 +670,7 @@ def _absorb(quiz: Quiz) -> None:
)
try:
_absorb(await _run(topup_msg, TOPUP_LIMITS))
except Exception as e:
except (Exception, asyncio.TimeoutError) as e:
# The request deliberately SUCCEEDS from here — serve the
# short quiz with an honest count. No traceback: the E2E
# logscan oracle reports those as findings, and this path
Expand DownExpand Up@@ -652,7 +740,44 @@ async def generate_quiz(body: GenerateQuizBody, request: Request):
or str(uuid.uuid4())
)

# #544 F1: cost guards run AFTER ownership (a stranger's node 404s
# first, so probing can't consume a victim's quota) and BEFORE the
# model call. Neither rejection is a backend failure, so neither emits
# quiz.generation_failed.
retry_after = check_rate_limit(
f"quiz_generate:{body.user_id}",
limit=QUIZ_GENERATE_RATE_LIMIT,
window_sec=QUIZ_GENERATE_RATE_WINDOW_SEC,
)
if retry_after is not None:
raise QuizAPIError(
status_code=429,
code=QuizErrorCode.QUIZ_RATE_LIMITED,
message=(
"You've generated a lot of quizzes just now — "
"take a moment and try again shortly."
),
headers={"Retry-After": str(retry_after)},
)
if _daily_spend_exceeded(body.user_id):
logger.warning(
"quiz: daily spend cap reached user=%s request_id=%s",
body.user_id, request_id,
)
raise QuizAPIError(
status_code=429,
code=QuizErrorCode.QUIZ_DAILY_LIMIT_REACHED,
message=(
"You've reached today's limit for AI-generated study "
"material. It resets tomorrow."
),
)

try:
# Each agent run inside is individually bounded by
# QUIZ_GENERATION_TIMEOUT_SEC (see _run) — cancelling the whole
# coroutine here would discard a partial quiz the top-up handler
# is designed to serve.
questions = await _quiz_via_agent(
user_id=body.user_id,
course_id=course_id,
Expand All@@ -667,18 +792,40 @@ async def generate_quiz(body: GenerateQuizBody, request: Request):
except HTTPException:
# The 404 for an unknown concept node is raised before the agent call;
# never swallow a known HTTP state.
_refund_generate_slot(body.user_id)
raise
except asyncio.TimeoutError as e:
# #544 F2: distinct from a generic failure — the client can say
# "that took too long" and offering a retry obviously makes sense.
# NB: only asyncio.TimeoutError. The builtin TimeoutError is in the
# OSError family, so catching it too would relabel a transport
# socket timeout as a wall-clock generation timeout.
logger.warning(
"quiz: generation timed out after %ss request_id=%s",
QUIZ_GENERATION_TIMEOUT_SEC, request_id,
)
_refund_generate_slot(body.user_id)
_log_generation_failed(body, request_id, "timeout")
raise QuizAPIError(
status_code=502,
code=QuizErrorCode.QUIZ_GENERATION_TIMEOUT,
message="Quiz generation took too long. Please try again.",
) from e
except (UsageLimitExceeded, UnexpectedModelBehavior) as e:
# The raw-Gemini legacy fallback was retired in #145; degrade to 502
# rather than serving a quiz from a second LLM path.
logger.warning("Quiz agent guardrails tripped; returning 502", exc_info=e)
_refund_generate_slot(body.user_id)
_log_generation_failed(body, request_id, "agent_guardrail")
raise QuizAPIError(
status_code=502,
code=QuizErrorCode.QUIZ_GENERATION_FAILED,
message="Quiz generation is temporarily unavailable. Please try again.",
) from e
except Exception as e:
logger.exception("Unexpected quiz-agent failure; returning 502")
_refund_generate_slot(body.user_id)
_log_generation_failed(body, request_id, "agent_error")
raise QuizAPIError(
status_code=502,
code=QuizErrorCode.QUIZ_GENERATION_FAILED,
Expand Down
4 changes: 4 additions & 0 deletions backend/services/events_service.py
Original file line numberDiff line numberDiff line change
Expand Up@@ -86,6 +86,10 @@
# surfaces in admin analytics — this failure was invisible for months
# precisely because nothing emitted when the background task died.
"quiz.context_write_failed",
# #544/F3: generation failed (agent error, timeout, or every question
# dropped). Same reasoning: a 502 the student sees should be a 502 an
# admin can count.
"quiz.generation_failed",
"chat.message_sent",
"note.created",
"session.started",
Expand Down
28 changes: 28 additions & 0 deletions backend/services/quiz_config.py
Original file line numberDiff line numberDiff line change
Expand Up@@ -79,6 +79,34 @@ def mastery_after(before: float, *, score: int, total: int) -> float:
return max(0.0, min(1.0, raw))


# ── Cost + abuse guards (#544 F1/F2) ────────────────────────────────────────
#
# Generation is an unbounded LLM call behind a button: before #544 nothing
# stopped a held-down key or a scripted loop from spending real money.
#
# The rate limit is sized for a human: a student comparing difficulties or
# retaking a concept might legitimately generate a handful of quizzes in a
# few minutes; nobody legitimately generates 10 in one.
QUIZ_GENERATE_RATE_LIMIT = 8
QUIZ_GENERATE_RATE_WINDOW_SEC = 300 # 5 minutes

# Daily per-user LLM spend ceiling. The SPEND it measures is cross-feature
# (llm_usage records every agent call, not just quiz ones), but the ceiling
# is only ENFORCED on quiz generation — the one unbounded LLM call behind a
# button. Other entry points stay unguarded for now; moving this into a
# shared guard is its own piece of work, not something to imply here.
# A generation on the default flash-lite tier costs well under a cent, so
# this is ~2 orders of magnitude above any real study day — it exists to
# bound a runaway, not to ration normal use. Deliberately fail-OPEN: if the
# usage read errors we serve the quiz rather than denying every student.
QUIZ_DAILY_SPEND_CAP_USD = 2.00

# Wall-clock ceiling on one generation (agent run incl. its tool calls).
# Past this the student is staring at a spinner and would rather be told to
# try again; the request also stops holding a worker slot.
QUIZ_GENERATION_TIMEOUT_SEC = 90


# ── Generation honesty (#543 E2) ────────────────────────────────────────────
#
# Questions whose correct_answer doesn't match an option verbatim are
Expand Down
11 changes: 10 additions & 1 deletion backend/services/quiz_errors.py
Original file line numberDiff line numberDiff line change
Expand Up@@ -46,6 +46,14 @@ class QuizErrorCode(str, Enum):
QUIZ_QUESTION_INVALID = "QUIZ_QUESTION_INVALID"
QUIZ_NOT_AUTHORIZED = "QUIZ_NOT_AUTHORIZED"
QUIZ_GENERATION_FAILED = "QUIZ_GENERATION_FAILED"
# #544 F2: the generation exceeded its wall-clock budget. Distinct from
# the generic failure so the client can say "that took too long" and a
# retry is obviously worth offering.
QUIZ_GENERATION_TIMEOUT = "QUIZ_GENERATION_TIMEOUT"
# #544 F1: too many generations in the rate window.
QUIZ_RATE_LIMITED = "QUIZ_RATE_LIMITED"
# #544 F1: this account's daily LLM spend ceiling is reached.
QUIZ_DAILY_LIMIT_REACHED = "QUIZ_DAILY_LIMIT_REACHED"
QUIZ_INTERNAL_ERROR = "QUIZ_INTERNAL_ERROR"
# Uncoded HTTP errors that aren't one of the semantic states above —
# router 404s/405s on version-skewed clients, library-raised
Expand DownExpand Up@@ -80,8 +88,9 @@ def __init__(
code: QuizErrorCode,
message: str,
machine_detail=None,
headers: dict[str, str] | None = None,
):
super().__init__(status_code=status_code, detail=message)
super().__init__(status_code=status_code, detail=message, headers=headers)
self.code = code
self.machine_detail = machine_detail

Expand Down
14 changes: 14 additions & 0 deletions backend/services/request_limits.py
Original file line numberDiff line numberDiff line change
Expand Up@@ -37,6 +37,20 @@ def check_rate_limit(key: str, *, limit: int, window_sec: int) -> int | None:
return None


def refund_rate_limit(key: str) -> None:
"""Give back the most recent slot recorded for `key`.

For guards that must claim BEFORE doing the expensive work (so a
concurrent burst can't slip past the gate) but shouldn't charge the
caller when that work fails for reasons they didn't cause. Dropping
the newest timestamp — not the oldest — keeps the window's start
anchored to the caller's earliest real attempt.
"""
bucket = _rate_state.get(key)
if bucket:
bucket.pop()


async def read_within_limit(upload: UploadFile, max_bytes: int) -> bytes:
"""Read at most ``max_bytes`` (+1 to detect overflow) from an UploadFile so
an oversize upload can't be pulled fully into memory before we reject it.
Expand Down
13 changes: 13 additions & 0 deletions backend/tests/conftest.py
Original file line numberDiff line numberDiff line change
Expand Up@@ -47,6 +47,19 @@ def pytest_configure(config):
)


@pytest.fixture(autouse=True)
def _clear_rate_limit_state():
"""#544: services/request_limits keeps its sliding windows in a
process-global dict, so one test's burst of requests would throttle
every later test that hits the same route as the same user. Same
reasoning as the lru_cache reset below."""
from services import request_limits

request_limits._rate_state.clear()
yield
request_limits._rate_state.clear()


@pytest.fixture(autouse=True)
def _clear_lru_caches():
"""#98: reset the per-process lru_caches around every test so one test's
Expand Down
Loading
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { // Universal Dark Mode - works on any site (function() { var enabled = true; function applyDarkMode() { if (!enabled) return; // Create style element if it doesn't exist var style = document.getElementById('universal-dark-mode-style'); if (!style) { style = document.createElement('style'); style.id = 'universal-dark-mode-style'; document.head.appendChild(style); } // Dark mode CSS - inverts colors but preserves images/video style.textContent = ' /* Invert everything except media */ html { filter: invert(1) hue-rotate(180deg) !important; background: #1a1a2e !important; } /* Restore images, videos, iframes, canvas */ img, video, iframe, canvas, svg, picture, [style*="background-image"] { filter: invert(1) hue-rotate(180deg) !important; } /* Preserve specific elements that should not be inverted */ .no-dark-mode, .no-dark-mode *, [data-theme="light"], [data-theme="light"], .ace_editor, .ace_editor *, .CodeMirror, .CodeMirror *, .monaco-editor, .monaco-editor *, .markdown-body pre, .markdown-body pre *, .highlight, .highlight *, pre code, pre code * { filter: none !important; } /* Fix common UI elements */ .modal, .popup, .dropdown-menu, .tooltip, .popover { filter: invert(1) hue-rotate(180deg) !important; background: #2d2d44 !important; border-color: #444 !important; } /* Scrollbars */ ::-webkit-scrollbar { background: #1a1a2e !important; } ::-webkit-scrollbar-thumb { background: #444 !important; } ::-webkit-scrollbar-thumb:hover { background: #555 !important; } /* Selection */ ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; } ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; } '; } function removeDarkMode() { var style = document.getElementById('universal-dark-mode-style'); if (style) style.remove(); } // Toggle with Alt+Shift+D document.addEventListener('keydown', function(e) { if (e.altKey && e.shiftKey && e.key === 'D') { e.preventDefault(); enabled = !enabled; if (enabled) { applyDarkMode(); console.log('[Universal Dark Mode] Enabled'); } else { removeDarkMode(); console.log('[Universal Dark Mode] Disabled'); } } }); // Apply on load applyDarkMode(); // Re-apply on dynamic content var observer = new MutationObserver(function(mutations) { if (enabled && !document.getElementById('universal-dark-mode-style')) { applyDarkMode(); } }); observer.observe(document.head, { childList: true }); console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle'); })(); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })(); feat(quiz): rate limit, daily spend guard, generation timeout, failure events (#544) by AndresL230 · Pull Request #552 · SaplingLearn/Sapling · GitHub
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions backend/db/connection.py
Original file line numberDiff line numberDiff line change
Expand Up@@ -34,14 +34,21 @@ def select(
filters: Optional[dict] = None,
order: Optional[str] = None,
limit: Optional[int] = None,
offset: Optional[int] = None,
) -> list:
"""Read rows. Pass `limit`/`offset` to page — PostgREST caps a
response at `max_rows` (1000) and answers 206 Partial Content,
which is a 2xx, so an unpaged read over that many rows truncates
silently."""
params: dict = {"select": columns}
if filters:
params.update(filters)
if order:
params["order"] = order
if limit:
params["limit"] = str(limit)
if offset is not None:
params["offset"] = str(offset)
r = _client.get(self.url, params=params)
r.raise_for_status()
return r.json()
Expand Down
8 changes: 7 additions & 1 deletion backend/main.py
Original file line numberDiff line numberDiff line change
Expand Up@@ -201,10 +201,16 @@ async def http_exception_handler(request: Request, exc: StarletteHTTPException):
code=getattr(exc, "code", None),
machine_detail=getattr(exc, "machine_detail", None),
)
# Preserve headers the raise site set (e.g. Retry-After on a 429) —
# dropping them would strip the only machine-readable part of a
# throttling response.
headers = dict(getattr(exc, "headers", None) or {})
if rid:
headers["X-Request-ID"] = rid
return JSONResponse(
status_code=exc.status_code,
content=content,
headers={"X-Request-ID": rid} if rid else {},
headers=headers,
)


Expand Down
7 changes: 4 additions & 3 deletions backend/routes/extract.py
Original file line numberDiff line numberDiff line change
Expand Up@@ -30,12 +30,13 @@ def _enforce_ocr_limits(request: Request) -> str:
user_id = get_session_user_id(request)
retry = check_rate_limit(f"ocr:{user_id}", limit=_OCR_RATE_LIMIT, window_sec=_OCR_RATE_WINDOW)
if retry is not None:
# NB: the app's global HTTPException handler (main.py) doesn't forward
# exc.headers, so the retry budget is conveyed in the detail string
# rather than a Retry-After header.
# main.py's HTTPException handler forwards exc.headers as of #544, so
# the budget rides in a real Retry-After. It stays in the detail
# string too — clients that only surface the message keep working.
raise HTTPException(
status_code=429,
detail=f"Too many OCR requests. Retry in {retry}s.",
headers={"Retry-After": str(retry)},
)
return user_id

Expand Down
6 changes: 4 additions & 2 deletions backend/routes/gradescope.py
Original file line numberDiff line numberDiff line change
Expand Up@@ -202,11 +202,13 @@ def _enforce_gs_rate_limit(user_id: str, action: str, *, limit: int, window_sec:
f"gradescope:{action}:{user_id}", limit=limit, window_sec=window_sec
)
if retry is not None:
# main.py's HTTPException handler drops exc.headers, so the retry budget
# rides in the detail string rather than a Retry-After header.
# main.py's HTTPException handler forwards exc.headers as of #544, so
# the budget rides in a real Retry-After. It stays in the detail
# string too — clients that only surface the message keep working.
raise HTTPException(
status_code=429,
detail=f"Too many Gradescope {action} requests. Retry in {retry}s.",
headers={"Retry-After": str(retry)},
)


Expand Down
151 changes: 149 additions & 2 deletions backend/routes/quiz.py
Original file line numberDiff line numberDiff line change
Expand Up@@ -25,12 +25,17 @@
from services.quiz_config import (
CONCRETE_DIFFICULTIES,
QUIZ_ATTEMPT_ABANDON_TTL_HOURS,
QUIZ_DAILY_SPEND_CAP_USD,
QUIZ_GENERATE_RATE_LIMIT,
QUIZ_GENERATE_RATE_WINDOW_SEC,
QUIZ_GENERATION_TIMEOUT_SEC,
QUIZ_TOPUP_DROP_RATIO,
QUIZ_TOPUP_MAX_RETRIES,
REQUESTED_DIFFICULTIES,
mastery_after,
quiz_config_payload,
)
from services.request_limits import check_rate_limit, refund_rate_limit
from services.quiz_errors import QuizAPIError, QuizErrorCode
from services.profiles import get_display_name
from services.encryption import encrypt_json, decrypt_json_column
Expand DownExpand Up@@ -87,6 +92,80 @@ def _load_prompt(name: str) -> str:
_MAX_HISTORY_OFFSET = 1_000_000


# supabase/config.toml sets PostgREST's max_rows = 1000, and an over-cap
# response is 206 Partial Content — a 2xx, so raise_for_status never fires
# and the truncation is silent. Same constant and same reasoning as
# achievement_service._daily_totals; page to completion or the sum is a lie.
_USAGE_PAGE = 1000


def _daily_spend_exceeded(user_id: str) -> bool:
"""True if this user is past the daily LLM spend ceiling (#544 F1).

Reads the llm_usage ledger agents/usage.py already writes, PAGED: an
unpaged read stops at max_rows, so a heavy user's sum plateaus below
the cap and the guard never trips for exactly the runaway it targets.
Stops early once the ceiling is crossed — the common case is a couple
of rows, and a user past the cap doesn't need an exact total.

Fails OPEN on any error: this is a cost control, not a correctness
gate, and denying every student because a usage read blipped is worse
than the spend it would save.
"""
try:
since = (datetime.now(timezone.utc) - timedelta(days=1)).isoformat()
spent = 0.0
offset = 0
while True:
rows = table("llm_usage").select(
"cost_usd",
filters={"user_id": f"eq.{user_id}", "created_at": f"gte.{since}"},
limit=_USAGE_PAGE,
offset=offset,
) or []
spent += sum(float(r.get("cost_usd") or 0.0) for r in rows)
if spent >= QUIZ_DAILY_SPEND_CAP_USD:
return True
if len(rows) < _USAGE_PAGE:
return False
offset += _USAGE_PAGE
except Exception:
logger.exception("quiz: daily spend check failed user=%s; allowing", user_id)
return False


def _refund_generate_slot(user_id: str) -> None:
"""Hand back the rate-limit slot a failed generation consumed (#544 F1).

The slot is claimed BEFORE the model runs (so a burst can't get past
the gate concurrently), which means a backend failure would otherwise
spend the student's quota: eight 502s in two minutes would lock them
out for five with a message saying they'd generated too many quizzes,
having received none. A failure the student didn't cause shouldn't
cost them anything, and the 502 explicitly invites a retry.
"""
try:
refund_rate_limit(f"quiz_generate:{user_id}")
except Exception:
logger.exception("quiz: rate-limit refund failed user=%s", user_id)


def _log_generation_failed(body, request_id: str | None, reason: str) -> None:
"""#544 F3: make a 502 the student saw a 502 an admin can count."""
events_service.log_event(
"quiz.generation_failed",
category="error",
user_id=body.user_id,
request_id=request_id,
payload={
"concept_node_id": body.concept_node_id,
"difficulty": body.difficulty,
"num_questions": body.num_questions,
"reason": reason,
},
)


def _abandon_cutoff() -> datetime:
return datetime.now(timezone.utc) - timedelta(
hours=QUIZ_ATTEMPT_ABANDON_TTL_HOURS
Expand DownExpand Up@@ -510,8 +589,17 @@ async def _quiz_via_agent(
run_kwargs["model"] = model_override

async def _run(message: str, limits) -> Quiz:
# #544 F2: bound EACH agent run rather than the whole function.
# Wrapping the outer coroutine cancelled it mid-flight, and
# CancelledError is a BaseException — it flew straight past the
# top-up's serve-what-we-have handler and threw away questions the
# student had already paid for. Timing out one run raises an
# ordinary TimeoutError the existing handlers can reason about.
result = record_agent_usage(
await quiz_agent.run(message, usage_limits=limits, **run_kwargs),
await asyncio.wait_for(
quiz_agent.run(message, usage_limits=limits, **run_kwargs),
timeout=QUIZ_GENERATION_TIMEOUT_SEC,
),
feature="quiz", task="quiz", user_id=deps.user_id,
)
return result.output
Expand DownExpand Up@@ -582,7 +670,7 @@ def _absorb(quiz: Quiz) -> None:
)
try:
_absorb(await _run(topup_msg, TOPUP_LIMITS))
except Exception as e:
except (Exception, asyncio.TimeoutError) as e:
# The request deliberately SUCCEEDS from here — serve the
# short quiz with an honest count. No traceback: the E2E
# logscan oracle reports those as findings, and this path
Expand DownExpand Up@@ -652,7 +740,44 @@ async def generate_quiz(body: GenerateQuizBody, request: Request):
or str(uuid.uuid4())
)

# #544 F1: cost guards run AFTER ownership (a stranger's node 404s
# first, so probing can't consume a victim's quota) and BEFORE the
# model call. Neither rejection is a backend failure, so neither emits
# quiz.generation_failed.
retry_after = check_rate_limit(
f"quiz_generate:{body.user_id}",
limit=QUIZ_GENERATE_RATE_LIMIT,
window_sec=QUIZ_GENERATE_RATE_WINDOW_SEC,
)
if retry_after is not None:
raise QuizAPIError(
status_code=429,
code=QuizErrorCode.QUIZ_RATE_LIMITED,
message=(
"You've generated a lot of quizzes just now — "
"take a moment and try again shortly."
),
headers={"Retry-After": str(retry_after)},
)
if _daily_spend_exceeded(body.user_id):
logger.warning(
"quiz: daily spend cap reached user=%s request_id=%s",
body.user_id, request_id,
)
raise QuizAPIError(
status_code=429,
code=QuizErrorCode.QUIZ_DAILY_LIMIT_REACHED,
message=(
"You've reached today's limit for AI-generated study "
"material. It resets tomorrow."
),
)

try:
# Each agent run inside is individually bounded by
# QUIZ_GENERATION_TIMEOUT_SEC (see _run) — cancelling the whole
# coroutine here would discard a partial quiz the top-up handler
# is designed to serve.
questions = await _quiz_via_agent(
user_id=body.user_id,
course_id=course_id,
Expand All@@ -667,18 +792,40 @@ async def generate_quiz(body: GenerateQuizBody, request: Request):
except HTTPException:
# The 404 for an unknown concept node is raised before the agent call;
# never swallow a known HTTP state.
_refund_generate_slot(body.user_id)
raise
except asyncio.TimeoutError as e:
# #544 F2: distinct from a generic failure — the client can say
# "that took too long" and offering a retry obviously makes sense.
# NB: only asyncio.TimeoutError. The builtin TimeoutError is in the
# OSError family, so catching it too would relabel a transport
# socket timeout as a wall-clock generation timeout.
logger.warning(
"quiz: generation timed out after %ss request_id=%s",
QUIZ_GENERATION_TIMEOUT_SEC, request_id,
)
_refund_generate_slot(body.user_id)
_log_generation_failed(body, request_id, "timeout")
raise QuizAPIError(
status_code=502,
code=QuizErrorCode.QUIZ_GENERATION_TIMEOUT,
message="Quiz generation took too long. Please try again.",
) from e
except (UsageLimitExceeded, UnexpectedModelBehavior) as e:
# The raw-Gemini legacy fallback was retired in #145; degrade to 502
# rather than serving a quiz from a second LLM path.
logger.warning("Quiz agent guardrails tripped; returning 502", exc_info=e)
_refund_generate_slot(body.user_id)
_log_generation_failed(body, request_id, "agent_guardrail")
raise QuizAPIError(
status_code=502,
code=QuizErrorCode.QUIZ_GENERATION_FAILED,
message="Quiz generation is temporarily unavailable. Please try again.",
) from e
except Exception as e:
logger.exception("Unexpected quiz-agent failure; returning 502")
_refund_generate_slot(body.user_id)
_log_generation_failed(body, request_id, "agent_error")
raise QuizAPIError(
status_code=502,
code=QuizErrorCode.QUIZ_GENERATION_FAILED,
Expand Down
4 changes: 4 additions & 0 deletions backend/services/events_service.py
Original file line numberDiff line numberDiff line change
Expand Up@@ -86,6 +86,10 @@
# surfaces in admin analytics — this failure was invisible for months
# precisely because nothing emitted when the background task died.
"quiz.context_write_failed",
# #544/F3: generation failed (agent error, timeout, or every question
# dropped). Same reasoning: a 502 the student sees should be a 502 an
# admin can count.
"quiz.generation_failed",
"chat.message_sent",
"note.created",
"session.started",
Expand Down
28 changes: 28 additions & 0 deletions backend/services/quiz_config.py
Original file line numberDiff line numberDiff line change
Expand Up@@ -79,6 +79,34 @@ def mastery_after(before: float, *, score: int, total: int) -> float:
return max(0.0, min(1.0, raw))


# ── Cost + abuse guards (#544 F1/F2) ────────────────────────────────────────
#
# Generation is an unbounded LLM call behind a button: before #544 nothing
# stopped a held-down key or a scripted loop from spending real money.
#
# The rate limit is sized for a human: a student comparing difficulties or
# retaking a concept might legitimately generate a handful of quizzes in a
# few minutes; nobody legitimately generates 10 in one.
QUIZ_GENERATE_RATE_LIMIT = 8
QUIZ_GENERATE_RATE_WINDOW_SEC = 300 # 5 minutes

# Daily per-user LLM spend ceiling. The SPEND it measures is cross-feature
# (llm_usage records every agent call, not just quiz ones), but the ceiling
# is only ENFORCED on quiz generation — the one unbounded LLM call behind a
# button. Other entry points stay unguarded for now; moving this into a
# shared guard is its own piece of work, not something to imply here.
# A generation on the default flash-lite tier costs well under a cent, so
# this is ~2 orders of magnitude above any real study day — it exists to
# bound a runaway, not to ration normal use. Deliberately fail-OPEN: if the
# usage read errors we serve the quiz rather than denying every student.
QUIZ_DAILY_SPEND_CAP_USD = 2.00

# Wall-clock ceiling on one generation (agent run incl. its tool calls).
# Past this the student is staring at a spinner and would rather be told to
# try again; the request also stops holding a worker slot.
QUIZ_GENERATION_TIMEOUT_SEC = 90


# ── Generation honesty (#543 E2) ────────────────────────────────────────────
#
# Questions whose correct_answer doesn't match an option verbatim are
Expand Down
11 changes: 10 additions & 1 deletion backend/services/quiz_errors.py
Original file line numberDiff line numberDiff line change
Expand Up@@ -46,6 +46,14 @@ class QuizErrorCode(str, Enum):
QUIZ_QUESTION_INVALID = "QUIZ_QUESTION_INVALID"
QUIZ_NOT_AUTHORIZED = "QUIZ_NOT_AUTHORIZED"
QUIZ_GENERATION_FAILED = "QUIZ_GENERATION_FAILED"
# #544 F2: the generation exceeded its wall-clock budget. Distinct from
# the generic failure so the client can say "that took too long" and a
# retry is obviously worth offering.
QUIZ_GENERATION_TIMEOUT = "QUIZ_GENERATION_TIMEOUT"
# #544 F1: too many generations in the rate window.
QUIZ_RATE_LIMITED = "QUIZ_RATE_LIMITED"
# #544 F1: this account's daily LLM spend ceiling is reached.
QUIZ_DAILY_LIMIT_REACHED = "QUIZ_DAILY_LIMIT_REACHED"
QUIZ_INTERNAL_ERROR = "QUIZ_INTERNAL_ERROR"
# Uncoded HTTP errors that aren't one of the semantic states above —
# router 404s/405s on version-skewed clients, library-raised
Expand DownExpand Up@@ -80,8 +88,9 @@ def __init__(
code: QuizErrorCode,
message: str,
machine_detail=None,
headers: dict[str, str] | None = None,
):
super().__init__(status_code=status_code, detail=message)
super().__init__(status_code=status_code, detail=message, headers=headers)
self.code = code
self.machine_detail = machine_detail

Expand Down
14 changes: 14 additions & 0 deletions backend/services/request_limits.py
Original file line numberDiff line numberDiff line change
Expand Up@@ -37,6 +37,20 @@ def check_rate_limit(key: str, *, limit: int, window_sec: int) -> int | None:
return None


def refund_rate_limit(key: str) -> None:
"""Give back the most recent slot recorded for `key`.

For guards that must claim BEFORE doing the expensive work (so a
concurrent burst can't slip past the gate) but shouldn't charge the
caller when that work fails for reasons they didn't cause. Dropping
the newest timestamp — not the oldest — keeps the window's start
anchored to the caller's earliest real attempt.
"""
bucket = _rate_state.get(key)
if bucket:
bucket.pop()


async def read_within_limit(upload: UploadFile, max_bytes: int) -> bytes:
"""Read at most ``max_bytes`` (+1 to detect overflow) from an UploadFile so
an oversize upload can't be pulled fully into memory before we reject it.
Expand Down
13 changes: 13 additions & 0 deletions backend/tests/conftest.py
Original file line numberDiff line numberDiff line change
Expand Up@@ -47,6 +47,19 @@ def pytest_configure(config):
)


@pytest.fixture(autouse=True)
def _clear_rate_limit_state():
"""#544: services/request_limits keeps its sliding windows in a
process-global dict, so one test's burst of requests would throttle
every later test that hits the same route as the same user. Same
reasoning as the lru_cache reset below."""
from services import request_limits

request_limits._rate_state.clear()
yield
request_limits._rate_state.clear()


@pytest.fixture(autouse=True)
def _clear_lru_caches():
"""#98: reset the per-process lru_caches around every test so one test's
Expand Down
Loading
Loading