diff --git a/backend/agents/_providers.py b/backend/agents/_providers.py index c05916ec..5eb5d99e 100644 --- a/backend/agents/_providers.py +++ b/backend/agents/_providers.py @@ -7,6 +7,7 @@ SAPLING_MODEL_SUMMARY=gemini-2.5-flash-lite SAPLING_MODEL_CONCEPTS=gemini-2.5-flash SAPLING_MODEL_SYLLABUS=gemini-2.5-flash + SAPLING_MODEL_QUIZ=gemini-2.5-flash-lite Defaults are tuned per task: cheaper models for simpler classifications, flagship Flash for tasks where output quality drives downstream UX. @@ -23,7 +24,7 @@ from config import GEMINI_API_KEY -AgentTask = Literal["classifier", "summary", "concepts", "syllabus"] +AgentTask = Literal["classifier", "summary", "concepts", "syllabus", "quiz"] # Defaults are conservative. Bumping a model up costs more; the env var @@ -33,6 +34,10 @@ "summary": "gemini-2.5-flash-lite", "concepts": "gemini-2.5-flash", "syllabus": "gemini-2.5-flash", + # Quiz generation defaults to lite: it's a single-shot non-streaming + # call where the agent pulls structured graph data via tools, so the + # bulk of the value is in tool wiring, not raw model strength. + "quiz": "gemini-2.5-flash-lite", } diff --git a/backend/agents/quiz.py b/backend/agents/quiz.py new file mode 100644 index 00000000..5b8fe495 --- /dev/null +++ b/backend/agents/quiz.py @@ -0,0 +1,111 @@ +"""Quiz-generation agent. + +Replaces routes/quiz.py:82's call_gemini_json + manual prompt-string +augmentation. The agent has tools to pull weak concepts + class +misconceptions on demand instead of pre-stuffing them into the prompt. + +Per ADR 0003 convention 4: keep the output schema compact. Gemini's +structured-output API rejects rich nested schemas with too many states +for serving — Quiz is a flat top-level model with a list of +QuizQuestion items, no further nesting. +""" + +from __future__ import annotations + +import hashlib +from typing import Literal + +from pydantic import BaseModel, Field +from pydantic_ai import Agent + +from agents._providers import model_for +from agents.deps import SaplingDeps +from agents.tools.graph_read import ( + read_concepts_for_user_tool, + read_misconceptions_for_course_tool, +) + + +# Difficulty + question type are Literals so Gemini's enum constraint +# applies and downstream UI can branch on stable strings. +# +# `QuizQuestionType` is intentionally MCQ-only today. The frontend +# `submitQuiz` flow grades by `q["options"][i].correct` lookup — there's +# no UI for free-text answers, no fuzzy-match grading, no LLM-judged +# scoring. Generating short-answer questions through this path would +# emit unrenderable, ungradable items. Keep the type narrow until real +# short-answer support exists; revisit when that lands. +QuizDifficulty = Literal["easy", "medium", "hard"] +QuizQuestionType = Literal["multiple_choice"] + + +class QuizQuestion(BaseModel): + """A single multiple-choice quiz question. Kept small so the parent + Quiz schema doesn't trip Gemini's structured-output complexity limit.""" + + question: str = Field(max_length=600) + type: QuizQuestionType + difficulty: QuizDifficulty + # 3-6 options. The agent is required to populate this for every + # question and to make `correct_answer` match exactly one of them. + options: list[str] = Field(min_length=3, max_length=6) + # The option text the agent considers correct. Must appear verbatim + # in `options`; the route validates this and drops questions that + # violate the contract rather than silently mis-marking them. + correct_answer: str = Field(max_length=400) + explanation: str = Field(max_length=600) + # Concept the question is testing — must be one of the user's known + # concept_names per the prompt. Used by the route to award mastery + # on a correct answer. + concept: str = Field(max_length=120) + + +class Quiz(BaseModel): + """The agent's structured output.""" + + questions: list[QuizQuestion] = Field(min_length=1, max_length=20) + + +_SYSTEM_PROMPT = ( + "You generate adaptive multiple-choice quizzes for a student. Each " + "question must target a specific concept the student has weak " + "mastery on, OR address a class-level misconception you've seen.\n\n" + "Workflow:\n" + "1. Call `read_concepts_for_user` to see the student's mastery per " + " concept for this course (returned sorted by mastery ASC — " + " weakest first).\n" + "2. Call `read_misconceptions_for_course` to see anonymized class " + " misconceptions. Use these to phrase distractors and to write " + " a question that probes the misconception.\n" + "3. Compose `Quiz.questions` so the WEAKEST concepts get the most " + " questions, AND each item's `concept` field exactly matches a " + " concept_name returned by tool 1.\n\n" + "Per-question rules (multiple-choice only — the type field is " + "constrained to 'multiple_choice'):\n" + "- 4 options, exactly one correct. The text in `correct_answer` " + " MUST appear verbatim in `options` — character-for-character. " + " Questions that violate this are dropped at the route layer.\n" + "- Distractors should reflect plausible misconceptions, not random " + " noise. Use the read_misconceptions_for_course return value.\n" + "- explanation: 1-3 sentences explaining WHY the correct answer " + " is correct — used in the post-quiz review screen.\n" + "- difficulty: align with the student's mastery on the concept; " + " weakest concepts get easy/medium, strongest get hard.\n\n" + "Honor the requested num_questions and difficulty distribution " + "in the user message. Don't invent concepts the student doesn't " + "have." +) +_PROMPT_HASH = hashlib.sha256(_SYSTEM_PROMPT.encode("utf-8")).hexdigest()[:12] + + +quiz_agent = Agent[SaplingDeps, Quiz]( + model=model_for("quiz"), + deps_type=SaplingDeps, + output_type=Quiz, + system_prompt=_SYSTEM_PROMPT, + metadata={"prompt_version": _PROMPT_HASH, "agent": "quiz"}, + tools=[ + read_concepts_for_user_tool, + read_misconceptions_for_course_tool, + ], +) diff --git a/backend/agents/tools/graph_read.py b/backend/agents/tools/graph_read.py new file mode 100644 index 00000000..51b2e9ed --- /dev/null +++ b/backend/agents/tools/graph_read.py @@ -0,0 +1,163 @@ +"""Read-side graph tools for Pydantic AI agents. + +Per ADR 0004: the agent needs to pull mastery state + course-level +misconceptions when planning a quiz. These are the tool surfaces. +The pure-async functions are callable directly from routes; +the *_tool wrappers register on a Pydantic AI Agent. +""" + +from __future__ import annotations + +import asyncio +import logging +from typing import Any + +from pydantic import BaseModel, Field +from pydantic_ai import RunContext + +from agents.deps import SaplingDeps +from db.connection import table + +logger = logging.getLogger(__name__) + + +# ── read_concepts_for_user ──────────────────────────────────────────────── + + +class ConceptMastery(BaseModel): + """Per-concept mastery state for the user in a course.""" + + concept_name: str + mastery: float = Field(ge=0.0, le=1.0) + last_reviewed_at: str | None = None + + +async def read_concepts_for_user( + user_id: str, + course_id: str | None, +) -> list[ConceptMastery]: + """Return the user's concept mastery for a course (or globally if + course_id is None). Sorted by mastery ASC so the weakest concepts + appear first — quiz_agent uses this ordering to focus on weak areas. + + Pure async, callable from routes. Wraps the underlying sync + Supabase read in asyncio.to_thread so it doesn't block the loop. + + NOTE: The underlying `graph_nodes` table stores the mastery value as + `mastery_score` and the timestamp as `last_studied_at`. We map them + here to the agent-facing names (`mastery`, `last_reviewed_at`) so the + tool contract stays stable even if storage column names change. + """ + + def _fetch() -> list[dict[str, Any]]: + filters = {"user_id": f"eq.{user_id}"} + if course_id: + filters["course_id"] = f"eq.{course_id}" + try: + return ( + table("graph_nodes").select( + "concept_name,mastery_score,last_studied_at", + filters=filters, + order="mastery_score.asc", + ) + or [] + ) + except Exception: + logger.exception( + "read_concepts_for_user failed for user=%s course=%s", + user_id, + course_id, + ) + return [] + + rows = await asyncio.to_thread(_fetch) + return [ + ConceptMastery( + concept_name=r.get("concept_name") or "", + mastery=float(r.get("mastery_score") or 0.0), + last_reviewed_at=r.get("last_studied_at"), + ) + for r in rows + if r.get("concept_name") + ] + + +async def read_concepts_for_user_tool( + ctx: RunContext[SaplingDeps], +) -> list[ConceptMastery]: + """Pydantic AI tool wrapper. Reads from ctx.deps.""" + return await read_concepts_for_user(ctx.deps.user_id, ctx.deps.course_id) + + +# ── read_misconceptions_for_course ──────────────────────────────────────── + + +class Misconception(BaseModel): + """A class-level misconception observed across student sessions.""" + + text: str + related_concept: str | None = None + + +async def read_misconceptions_for_course( + course_id: str | None, +) -> list[Misconception]: + """Return aggregated misconception strings for a course. Anonymized + (sourced from class-wide patterns, not from any single student). + Returns [] when course_id is None or the underlying table is empty. + + Source: `course_concept_stats` rows for the course. Each row + represents one concept and carries a `common_misconceptions` array + (populated by the hash-gated aggregation in + `services/course_context_service.py`). We flatten each array entry + into its own Misconception, tagging `related_concept` with the + concept name so the agent can route distractors per-concept. + + The spec referenced a hypothetical `misconceptions` table — that + table does not exist in this schema. `course_concept_stats` is the + real source of class-wide misconception strings, so we read that. + The tool contract (returning Misconception[]) is unchanged. + """ + if not course_id: + return [] + + def _fetch() -> list[dict[str, Any]]: + try: + return ( + table("course_concept_stats").select( + "concept_name,common_misconceptions", + filters={"course_id": f"eq.{course_id}"}, + order="updated_at.desc", + limit=20, + ) + or [] + ) + except Exception: + logger.exception( + "read_misconceptions_for_course failed for course=%s", + course_id, + ) + return [] + + rows = await asyncio.to_thread(_fetch) + out: list[Misconception] = [] + seen: set[str] = set() + for r in rows: + concept = r.get("concept_name") or None + for m in r.get("common_misconceptions") or []: + text = (m or "").strip() if isinstance(m, str) else "" + if not text: + continue + key = text.lower() + if key in seen: + continue + seen.add(key) + out.append(Misconception(text=text, related_concept=concept)) + return out + + +async def read_misconceptions_for_course_tool( + ctx: RunContext[SaplingDeps], +) -> list[Misconception]: + """Pydantic AI tool wrapper. Reads from ctx.deps.""" + return await read_misconceptions_for_course(ctx.deps.course_id) diff --git a/backend/models/__init__.py b/backend/models/__init__.py index 229af218..b57fe91b 100644 --- a/backend/models/__init__.py +++ b/backend/models/__init__.py @@ -44,6 +44,12 @@ class GenerateQuizBody(BaseModel): num_questions: int = 5 difficulty: str = "medium" use_shared_context: bool = True + # Mirrors the Learn-route fast/smart toggle so quiz generation has + # the same per-request model-quality choice as chat tutoring. + # "fast" → gemini-2.5-flash, "smart" → gemini-2.5-pro. None falls + # through to whatever SAPLING_MODEL_QUIZ resolves to (default + # gemini-2.5-flash-lite per ADR 0008). + model_pref: Optional[Literal["fast", "smart"]] = None class AnswerItem(BaseModel): diff --git a/backend/routes/quiz.py b/backend/routes/quiz.py index c0327dc8..2aeeb5e6 100644 --- a/backend/routes/quiz.py +++ b/backend/routes/quiz.py @@ -1,18 +1,27 @@ -import uuid import json +import logging import os +import uuid from datetime import datetime from fastapi import APIRouter, BackgroundTasks, HTTPException, Request +from pydantic_ai.exceptions import UsageLimitExceeded, UnexpectedModelBehavior + +from agents.quiz import quiz_agent, Quiz, QuizQuestion +from agents.deps import SaplingDeps from config import get_mastery_tier from db.connection import table from models import GenerateQuizBody, SubmitQuizBody from services.auth_guard import require_self from services.encryption import decrypt_if_present -from services.gemini_service import MODEL_LITE, call_gemini_json +from services.gemini_service import MODEL_DEFAULT, MODEL_LITE, MODEL_SMART, call_gemini_json from services.graph_service import get_graph, update_streak from services.quiz_context_service import get_quiz_context, save_quiz_context +from services.fingerprint import fingerprint +from services.request_context import current_request_id + +logger = logging.getLogger(__name__) router = APIRouter() @@ -24,10 +33,186 @@ def _load_prompt(name: str) -> str: return f.read() -@router.post("/generate") -def generate_quiz(body: GenerateQuizBody, request: Request): - require_self(body.user_id, request) - node_rows = table("graph_nodes").select("*", filters={"id": f"eq.{body.concept_node_id}"}) +# ── Wire-format helpers (legacy + agent paths share this shape) ────────────── +# +# `submit_quiz` expects each question dict to look like: +# { +# "id": int, +# "question": str, +# "options": [{"label": "A"|"B"|..., "text": str, "correct": bool}, ...], +# "explanation": str, +# "concept_tested": str, +# "difficulty": "easy"|"medium"|"hard", +# } +# This is the format the original quiz_generation.txt prompt produced. +# The agent's QuizQuestion has a flatter shape — we map it back here so the +# stored `questions_json` and the response payload don't change. Frontend +# `submitQuiz`/`scoreQuiz` flows are unaffected. + +_OPTION_LABELS = ["A", "B", "C", "D", "E", "F"] + + +def _agent_question_to_wire(q: QuizQuestion, qid: int) -> dict | None: + """Map an agent QuizQuestion to the legacy wire-format dict, or + return None if the question violates the contract. + + The agent must produce `correct_answer` as one of the strings in + `q.options` verbatim. If that invariant is broken (LLM drift), we + DROP the question rather than silently mark an arbitrary option + correct — emitting an unverifiable question to the user is worse + than a slightly shorter quiz. + + Returning None lets the caller filter questions out cleanly. + """ + options: list[dict] = [] + matched = False + canonical = q.correct_answer.strip() + for i, text in enumerate(q.options[: len(_OPTION_LABELS)]): + is_correct = (not matched) and (text.strip() == canonical) + if is_correct: + matched = True + options.append({ + "label": _OPTION_LABELS[i], + "text": text, + "correct": is_correct, + }) + if not matched: + # Generation drift: agent's correct_answer doesn't match any + # option verbatim. Surface in logs (Logfire span carries the + # question_id correlation) and drop. Caller filters None. + # + # Don't log the raw text — student-content concept names and + # quiz answers don't belong in stdout/Railway logs. The + # fingerprint is stable enough to correlate with the same + # generation drift if it recurs; the full content is still in + # Logfire spans (where the scrubber from PR #67 controls egress). + # services.fingerprint.fingerprint joins parts with the ASCII + # unit-separator (\x1f), so option text containing pipes or + # other punctuation can't accidentally collide. + canonical_only = q.correct_answer.strip() + fp = fingerprint(canonical_only, q.options) + logger.warning( + "quiz: dropping question id=%d — correct_answer not found in " + "options (n_options=%d, canonical_len=%d, fp=%s)", + qid, len(q.options), len(canonical_only), fp, + ) + return None + return { + "id": qid, + "question": q.question, + "options": options, + "explanation": q.explanation, + "concept_tested": q.concept, + "difficulty": q.difficulty, + } + + +# Per-request model override map. Mirrors the chat tutor's +# fast/smart toggle so quiz body's `model_pref` resolves to the same +# model strings as Learn. None falls through to the agent's +# task-default model from agents/_providers.py::model_for("quiz"). +_PREF_MODEL_NAMES: dict[str, str] = { + "fast": "gemini-2.5-flash", + "smart": "gemini-2.5-pro", +} + + +def _resolve_model_pref(model_pref: str | None): + """Build a GoogleModel override for the per-request fast/smart + preference, or return None to use the agent's default. + + `google_model` is imported lazily so that constructing a + GoogleProvider (which reads GEMINI_API_KEY at call time) only + happens when an override is actually requested — not at module + import. agents.quiz is already in this route's import graph, so + this isn't about import-path isolation; it's about deferring the + one runtime side-effect (the provider build) to the request that + needs it. + """ + if not model_pref: + return None + name = _PREF_MODEL_NAMES.get(model_pref) + if not name: + return None + from agents._providers import google_model + return google_model(name) + + +async def _quiz_via_agent( + *, + user_id: str, + course_id: str | None, + concept_node_id: str, + concept_name: str, + num_questions: int, + difficulty: str, + use_shared_context: bool, + request_id: str, + model_pref: str | None = None, +) -> list[dict]: + """Run quiz_agent and return questions in the legacy wire shape. + + The agent's tools (read_concepts_for_user, read_misconceptions_for_course) + pull weak-area + class misconception data themselves, replacing the + manual prompt-string augmentation that used to live in generate_quiz. + + `model_pref` ("fast" or "smart") overrides the agent's default model + on this single run. Anything else (None, unknown string) falls + through to model_for("quiz") at agent-construction time. + """ + deps = SaplingDeps( + user_id=user_id, + course_id=course_id, + supabase=None, + request_id=request_id, + ) + user_message = ( + f"Generate {num_questions} {difficulty} questions for the student. " + f"The target concept is '{concept_name}' (concept_node_id={concept_node_id}). " + f"Call read_concepts_for_user to find the student's weakest concepts in this course " + f"and bias the question mix toward those." + ) + if use_shared_context: + user_message += ( + " Also call read_misconceptions_for_course and use those misconceptions " + "as distractors and probes." + ) + + model_override = _resolve_model_pref(model_pref) + run_kwargs: dict = {"deps": deps} + if model_override is not None: + run_kwargs["model"] = model_override + result = await quiz_agent.run(user_message, **run_kwargs) + quiz: Quiz = result.output + # Filter out questions where the agent's correct_answer didn't match + # any option verbatim — _agent_question_to_wire returns None for those. + # Re-number the survivors so question IDs stay 1-based and contiguous. + wire_questions: list[dict] = [] + for q in quiz.questions: + mapped = _agent_question_to_wire(q, len(wire_questions) + 1) + if mapped is not None: + wire_questions.append(mapped) + if not wire_questions: + # All questions dropped — degrade to legacy rather than serve + # an empty quiz. Raise a sentinel that generate_quiz catches + # and routes to the legacy fallback. + raise RuntimeError( + "quiz_agent produced no valid questions after wire-format validation" + ) + return wire_questions + + +async def _legacy_generate_quiz(body: GenerateQuizBody, request: Request) -> list[dict]: + """The pre-agent quiz generation pipeline, kept as a fallback per ADR-0001. + + Verbatim copy of the original generate_quiz body: prompt-template assembly, + course-context augmentation, and a single call_gemini_json. + Returns the raw `questions` list — the route handler is responsible for + persisting it and shaping the HTTP response. + """ + node_rows = table("graph_nodes").select( + "*", filters={"id": f"eq.{body.concept_node_id}"} + ) if not node_rows: raise HTTPException(status_code=404, detail="Concept node not found") node = node_rows[0] @@ -84,12 +269,74 @@ def generate_quiz(body: GenerateQuizBody, request: Request): ) prompt += "\n\n" + "\n\n".join(addendum_parts) + # Honor the same fast/smart toggle the agent path uses. The mapping + # mirrors _PREF_MODEL_NAMES exactly, so a user choosing "fast" gets + # the same upgraded model whether the agent succeeded or fell back + # here. Without this, "fast" was silently a no-op on the legacy + # path (still MODEL_LITE) — the kind of inconsistency that surfaces + # six months later as "why does Fast sometimes feel slower?" + if body.model_pref == "smart": + legacy_model = MODEL_SMART # gemini-2.5-pro + elif body.model_pref == "fast": + legacy_model = MODEL_DEFAULT # gemini-2.5-flash, matches _PREF_MODEL_NAMES + else: + legacy_model = MODEL_LITE # gemini-2.5-flash-lite, agent default per ADR 0008 try: - result = call_gemini_json(prompt, model=MODEL_LITE) + result = call_gemini_json(prompt, model=legacy_model) except Exception as e: raise HTTPException(status_code=502, detail=f"Gemini error: {e}") - questions = result.get("questions", []) + return result.get("questions", []) + + +@router.post("/generate") +async def generate_quiz(body: GenerateQuizBody, request: Request): + require_self(body.user_id, request) + node_rows = table("graph_nodes").select( + "*", filters={"id": f"eq.{body.concept_node_id}"} + ) + if not node_rows: + raise HTTPException(status_code=404, detail="Concept node not found") + node = node_rows[0] + course_id = node.get("course_id") or None + concept_name = node.get("concept_name") or "" + + # Unify with the middleware-stamped request ID so agent traces and any + # downstream error payloads share the same correlation key. + request_id = ( + getattr(request.state, "request_id", None) + or current_request_id() + or str(uuid.uuid4()) + ) + + try: + questions = await _quiz_via_agent( + user_id=body.user_id, + course_id=course_id, + concept_node_id=body.concept_node_id, + concept_name=concept_name, + num_questions=body.num_questions, + difficulty=body.difficulty, + use_shared_context=body.use_shared_context, + request_id=request_id, + model_pref=body.model_pref, + ) + except (UsageLimitExceeded, UnexpectedModelBehavior) as e: + logger.warning( + "Quiz agent guardrails tripped; falling back to legacy", + exc_info=e, + ) + questions = await _legacy_generate_quiz(body, request) + except HTTPException: + # Legacy path raises HTTPException for known states (404/502); never + # treat those as a reason to fall back. Re-raise. + raise + except Exception: + logger.exception( + "Unexpected quiz-agent failure; falling back to legacy" + ) + questions = await _legacy_generate_quiz(body, request) + quiz_id = str(uuid.uuid4()) table("quiz_attempts").insert({ "id": quiz_id, diff --git a/backend/services/fingerprint.py b/backend/services/fingerprint.py new file mode 100644 index 00000000..874a261c --- /dev/null +++ b/backend/services/fingerprint.py @@ -0,0 +1,54 @@ +"""Stable, short content fingerprints for log correlation. + +Use case: we want to log a value (a prompt body, a quiz answer, a span +attribute) without putting the actual content in stdout. A truncated +SHA-256 is short enough for greppability and stable enough that the +same input fingerprints to the same string across runs and machines. + +This is correlation-only — NOT a security boundary. Truncation makes +collisions theoretically possible (~2^48 inputs at 12 hex chars) but +the use sites only need "if I see fp=abc123def456 again, that's the +same drift." Adversarial collisions don't matter. +""" + +from __future__ import annotations + +import hashlib + +# ASCII unit-separator. Won't appear in any user-typed text (concept +# names, quiz options, prompt bodies), so joining a list with this +# avoids the ambiguity that `|` introduces when items contain pipes. +SEPARATOR = "\x1f" + + +def fingerprint(*parts: str | int | float | bool | None, length: int = 12) -> str: + """Return a short, stable fingerprint of `parts` joined by SEPARATOR. + + Each part is str()'d. None becomes the empty string. Lists/tuples + are flattened with the same separator (one level deep). Output is + `length` lowercase hex chars (default 12 = 48 bits). + + >>> fingerprint("hello", "world") # noqa + '...' # 12 hex chars + >>> fingerprint("hello", "world") == fingerprint("hello", "world") + True + """ + flat: list[str] = [] + for p in parts: + if isinstance(p, (list, tuple)): + flat.append(SEPARATOR.join("" if x is None else str(x) for x in p)) + else: + flat.append("" if p is None else str(p)) + body = SEPARATOR.join(flat) + digest = hashlib.sha256(body.encode("utf-8", errors="replace")).hexdigest() + return digest[:length] + + +def fingerprint_text(value: str, length: int = 16) -> str: + """Single-string convenience used by the Logfire scrubber. Default + length is 16 chars (64 bits) for that callsite's redaction-trace + use; quiz drift logs use the 12-char form via `fingerprint(...)`. + """ + return hashlib.sha256( + value.encode("utf-8", errors="replace"), + ).hexdigest()[:length] diff --git a/backend/services/logfire_scrubber.py b/backend/services/logfire_scrubber.py index 7e727f35..e30897e3 100644 --- a/backend/services/logfire_scrubber.py +++ b/backend/services/logfire_scrubber.py @@ -23,12 +23,13 @@ from __future__ import annotations -import hashlib import re from typing import Any from logfire import ScrubMatch +from services.fingerprint import fingerprint_text + # Path components that indicate the value contains user-supplied prompt # text or model output. These are matched against any segment of the @@ -76,8 +77,13 @@ def _fingerprint(value: str) -> str: - """Short, stable hash for matching a redacted body to a re-uploaded copy.""" - return hashlib.sha256(value.encode("utf-8", errors="replace")).hexdigest()[:16] + """Short, stable hash for matching a redacted body to a re-uploaded copy. + + Delegates to services.fingerprint.fingerprint_text so the hashing + convention is consistent across log-correlation sites (this scrubber + + routes/quiz.py drift warnings). + """ + return fingerprint_text(value, length=16) def _path_is_risky(path: tuple[Any, ...] | str) -> bool: diff --git a/backend/tests/evals/quiz_generation.py b/backend/tests/evals/quiz_generation.py new file mode 100644 index 00000000..b302c30e --- /dev/null +++ b/backend/tests/evals/quiz_generation.py @@ -0,0 +1,276 @@ +"""pydantic-evals cases for the quiz_agent. + +Run via tests.evals._replay (record/replay/live): + cd backend + SAPLING_EVAL_MODE=record python tests/evals/quiz_generation.py + SAPLING_EVAL_MODE=replay python tests/evals/quiz_generation.py +""" + +from __future__ import annotations + +import sys +from dataclasses import dataclass +from pathlib import Path + +# `python tests/evals/quiz_generation.py` from backend/ — import path setup. +sys.path.insert(0, str(Path(__file__).resolve().parents[2])) +sys.path.insert(0, str(Path(__file__).parent)) # for _replay sibling import + +from pydantic_evals import Case, Dataset +from pydantic_evals.evaluators import Evaluator, EvaluatorContext + +from agents.quiz import quiz_agent, Quiz, QuizQuestion +from _replay import run_with_cassette, cli_main + + +# ── Evaluators ────────────────────────────────────────────────────────────── + +@dataclass +class QuestionCountEvaluator(Evaluator[str, Quiz]): + """Exact match on the requested number of questions (in metadata).""" + + def evaluate(self, ctx: EvaluatorContext[str, Quiz]) -> float: + expected = (ctx.metadata or {}).get("expected_count") + if expected is None: + return 1.0 # no constraint + return 1.0 if len(ctx.output.questions) == expected else 0.0 + + +@dataclass +class DifficultyMixEvaluator(Evaluator[str, Quiz]): + """All questions land at the requested difficulty (single-difficulty + cases only). Cases with `expected_difficulty=None` skip this check.""" + + def evaluate(self, ctx: EvaluatorContext[str, Quiz]) -> float: + target = (ctx.metadata or {}).get("expected_difficulty") + if target is None: + return 1.0 + matches = sum(1 for q in ctx.output.questions if q.difficulty == target) + total = max(1, len(ctx.output.questions)) + return matches / total + + +@dataclass +class TypeMixEvaluator(Evaluator[str, Quiz]): + """All questions are the requested type (single-type cases only).""" + + def evaluate(self, ctx: EvaluatorContext[str, Quiz]) -> float: + target = (ctx.metadata or {}).get("expected_type") + if target is None: + return 1.0 + matches = sum(1 for q in ctx.output.questions if q.type == target) + total = max(1, len(ctx.output.questions)) + return matches / total + + +@dataclass +class MultipleChoiceShapeEvaluator(Evaluator[str, Quiz]): + """Every multiple_choice question has 3-6 options AND the + correct_answer appears verbatim in those options.""" + + def evaluate(self, ctx: EvaluatorContext[str, Quiz]) -> float: + mc = [q for q in ctx.output.questions if q.type == "multiple_choice"] + if not mc: + return 1.0 + ok = sum( + 1 for q in mc + if 3 <= len(q.options) <= 6 and q.correct_answer in q.options + ) + return ok / len(mc) + + +@dataclass +class ConceptCoverageEvaluator(Evaluator[str, Quiz]): + """Every question's `concept` field is one of the concepts the + case's prompt named (declared in metadata).""" + + def evaluate(self, ctx: EvaluatorContext[str, Quiz]) -> float: + allowed = set((ctx.metadata or {}).get("concepts", [])) + if not allowed: + return 1.0 + ok = sum(1 for q in ctx.output.questions if q.concept in allowed) + return ok / max(1, len(ctx.output.questions)) + + +# ── Cases ─────────────────────────────────────────────────────────────────── + +# Inputs are deliberately small — Gemini reads the prompt + tool results +# (mocked via cassette in CI), so keeping inputs short keeps cassette +# diffs readable. + +CASES: list[Case[str, Quiz]] = [ + # ── Easy × MCQ ───────────────────────────────────────────────────────── + Case( + name="easy_mcq_intro_calculus", + inputs=( + "Course: MATH 101. Generate 3 easy multiple-choice questions " + "covering Limits, Derivatives, and Continuity. The student " + "is new to the material; favor definitional questions." + ), + metadata={ + "expected_count": 3, + "expected_difficulty": "easy", + "expected_type": "multiple_choice", + "concepts": ["Limits", "Derivatives", "Continuity"], + }, + ), + Case( + name="easy_mcq_basic_biology", + inputs=( + "Course: BIO 100. Generate 4 easy multiple-choice questions " + "covering Cell Membrane, Photosynthesis, Mitosis, Diffusion." + ), + metadata={ + "expected_count": 4, + "expected_difficulty": "easy", + "expected_type": "multiple_choice", + "concepts": ["Cell Membrane", "Photosynthesis", "Mitosis", "Diffusion"], + }, + ), + + # ── Medium × MCQ ─────────────────────────────────────────────────────── + Case( + name="medium_mcq_data_structures", + inputs=( + "Course: CS 201. Generate 3 medium multiple-choice questions " + "covering Hash Tables, Binary Search Trees, and Big-O Analysis. " + "Use distractors that reflect common student misconceptions." + ), + metadata={ + "expected_count": 3, + "expected_difficulty": "medium", + "expected_type": "multiple_choice", + "concepts": ["Hash Tables", "Binary Search Trees", "Big-O Analysis"], + }, + ), + Case( + name="medium_mcq_organic_chem", + inputs=( + "Course: CHEM 230. Generate 3 medium multiple-choice questions " + "covering Stereochemistry, SN1 vs SN2, and Resonance Structures." + ), + metadata={ + "expected_count": 3, + "expected_difficulty": "medium", + "expected_type": "multiple_choice", + "concepts": ["Stereochemistry", "SN1 vs SN2", "Resonance Structures"], + }, + ), + + # ── Hard × MCQ ───────────────────────────────────────────────────────── + Case( + name="hard_mcq_real_analysis", + inputs=( + "Course: MATH 411. Generate 2 hard multiple-choice questions " + "covering Cauchy Sequences and Uniform Convergence. Expect " + "questions that combine multiple definitions." + ), + metadata={ + "expected_count": 2, + "expected_difficulty": "hard", + "expected_type": "multiple_choice", + "concepts": ["Cauchy Sequences", "Uniform Convergence"], + }, + ), + + # NOTE: ADR 0005 originally specified 3 difficulty × 2 question type + # (MCQ + short answer). Short answer was dropped during refactor #2 + # because the frontend grade path has no UI for free-text answers + # and `submit_quiz` grades by option-label lookup. The 3 cases below + # replace the original short-answer cases with additional MCQ + # coverage at the same difficulty levels — total stays at 8. + # Revisit when real short-answer grading exists (LLM-judged or + # fuzzy-match). + + # ── Easy × MCQ (extra coverage) ─────────────────────────────────────── + Case( + name="easy_mcq_physics_definitions", + inputs=( + "Course: PHYS 101. Generate 3 easy multiple-choice questions " + "covering Newton's First Law, Kinetic Energy, and Momentum. " + "Each question should test a definitional understanding." + ), + metadata={ + "expected_count": 3, + "expected_difficulty": "easy", + "expected_type": "multiple_choice", + "concepts": ["Newton's First Law", "Kinetic Energy", "Momentum"], + }, + ), + + # ── Medium × MCQ (extra coverage — econ) ─────────────────────────────── + Case( + name="medium_mcq_econ", + inputs=( + "Course: ECON 201. Generate 3 medium multiple-choice questions " + "covering Marginal Cost, Price Elasticity of Demand, and " + "Comparative Advantage. Use distractors that reflect common " + "confusions between these concepts." + ), + metadata={ + "expected_count": 3, + "expected_difficulty": "medium", + "expected_type": "multiple_choice", + "concepts": ["Marginal Cost", "Price Elasticity of Demand", "Comparative Advantage"], + }, + ), + + # ── Hard × MCQ (extra coverage — theory of computation) ──────────────── + Case( + name="hard_mcq_theory_of_computation", + inputs=( + "Course: MATH 320. Generate 2 hard multiple-choice questions " + "covering the Pumping Lemma and Decidability. Distractors " + "should target the most common misapplications of these " + "results." + ), + metadata={ + "expected_count": 2, + "expected_difficulty": "hard", + "expected_type": "multiple_choice", + "concepts": ["Pumping Lemma", "Decidability"], + }, + ), +] + +assert len(CASES) == 8, f"Expected 8 cases per ADR 0005, got {len(CASES)}" +# All cases are MCQ today — short_answer dropped pending frontend support. +assert all( + c.metadata and c.metadata.get("expected_type") == "multiple_choice" + for c in CASES +), "All quiz_agent eval cases must be multiple_choice (see refactor #2 fixes)" + + +# ── Adapter (replay layer) ────────────────────────────────────────────────── + +# Lookup map for case_name from input (input strings are unique). +_INPUT_TO_NAME: dict[str, str] = {c.inputs: c.name for c in CASES} + + +async def _run(case_input: str) -> Quiz: + case_name = _INPUT_TO_NAME.get(case_input, "unknown") + return await run_with_cassette( + dataset="quiz_generation", + case_name=case_name, + agent=quiz_agent, + case_input=case_input, + output_model=Quiz, + ) + + +def make_dataset() -> Dataset[str, Quiz]: + return Dataset( + name="quiz_generation", + cases=CASES, + evaluators=[ + QuestionCountEvaluator(), + DifficultyMixEvaluator(), + TypeMixEvaluator(), + MultipleChoiceShapeEvaluator(), + ConceptCoverageEvaluator(), + ], + ) + + +if __name__ == "__main__": + cli_main(make_dataset, _run) diff --git a/backend/tests/test_documents_routes.py b/backend/tests/test_documents_routes.py index d1672fc2..73e084c1 100644 --- a/backend/tests/test_documents_routes.py +++ b/backend/tests/test_documents_routes.py @@ -231,12 +231,21 @@ def test_rejects_unsupported_extension(self): assert r.status_code == 400 assert "Unsupported file type" in r.json()["detail"] - def test_rejects_file_over_15mb(self): - big = b"x" * (15 * 1024 * 1024 + 1) - with _mock_validate_user(), patch("routes.documents.extract_text_from_file", return_value=""): - r = _make_upload(content=big) + def test_rejects_file_over_max_size(self): + # Cap was raised from 15 MB → 100 MB in commit 9912a25. Don't + # allocate 100MB here — it makes the test fragile under full-suite + # memory pressure. Instead, monkeypatch MAX_FILE_SIZE to 1 KB and + # pin the rejection-message contract. + with ( + _mock_validate_user(), + patch("routes.documents.MAX_FILE_SIZE", 1024), + patch("routes.documents.extract_text_from_file", return_value=""), + ): + r = _make_upload(content=b"x" * 2048) assert r.status_code == 400 - assert "15 MB" in r.json()["detail"] + # The error message names the actual cap (100 MB) — verifies the + # route's hardcoded copy in the detail string didn't drift. + assert "100 MB" in r.json()["detail"] def test_accepts_pdf_by_extension(self): ai_result = { diff --git a/backend/tests/test_fingerprint.py b/backend/tests/test_fingerprint.py new file mode 100644 index 00000000..0c42f960 --- /dev/null +++ b/backend/tests/test_fingerprint.py @@ -0,0 +1,76 @@ +"""Unit tests for services.fingerprint. + +Covers: deterministic hashing, the unit-separator delimiter (so +ambiguous-pipe collisions are impossible), list/tuple flattening, +None handling, custom length. +""" + +from __future__ import annotations + +from services.fingerprint import SEPARATOR, fingerprint, fingerprint_text + + +def test_deterministic_same_inputs_same_fingerprint(): + a = fingerprint("hello", "world") + b = fingerprint("hello", "world") + assert a == b + assert len(a) == 12 + + +def test_default_length_12_hex_chars(): + fp = fingerprint("anything") + assert len(fp) == 12 + assert all(c in "0123456789abcdef" for c in fp) + + +def test_custom_length(): + fp = fingerprint("anything", length=20) + assert len(fp) == 20 + + +def test_unit_separator_avoids_pipe_ambiguity(): + """If options contain `|`, naive `'|'.join(...)` would collapse + distinct option lists into the same string. The unit-separator + keeps them distinct.""" + a = fingerprint("answer", ["a|b", "c"]) + b = fingerprint("answer", ["a", "b|c"]) + assert a != b, ( + "Fingerprint must distinguish [a|b, c] from [a, b|c]; " + "the unit-separator is supposed to prevent this collision." + ) + + +def test_list_arg_flattens_with_separator(): + """A list argument is one part, joined with SEPARATOR internally.""" + fp_list = fingerprint("x", ["a", "b", "c"]) + # Equivalent direct call: hash("x" + sep + "a" + sep + "b" + sep + "c") + direct = fingerprint_text("x" + SEPARATOR + "a" + SEPARATOR + "b" + SEPARATOR + "c", length=12) + assert fp_list == direct + + +def test_none_becomes_empty_string(): + """None parts collapse to empty so the fingerprint stays stable + across Optional fields without special-casing each call site.""" + fp_none = fingerprint("a", None, "b") + fp_empty = fingerprint("a", "", "b") + assert fp_none == fp_empty + + +def test_int_and_bool_coerced_to_string(): + fp_int = fingerprint(42) + fp_str = fingerprint("42") + assert fp_int == fp_str + fp_true = fingerprint(True) + fp_str_true = fingerprint("True") + assert fp_true == fp_str_true + + +def test_fingerprint_text_default_length_16(): + fp = fingerprint_text("hello") + assert len(fp) == 16 + + +def test_separator_is_unit_separator(): + """Sanity check that the constant is what we documented.""" + assert SEPARATOR == "\x1f" + assert ord(SEPARATOR) == 31 diff --git a/backend/tests/test_graph_read_tools.py b/backend/tests/test_graph_read_tools.py new file mode 100644 index 00000000..add98542 --- /dev/null +++ b/backend/tests/test_graph_read_tools.py @@ -0,0 +1,147 @@ +""" +Unit tests for backend/agents/tools/graph_read.py + +Tests cover: + - read_concepts_for_user: ordering, missing-concept-name drop, error fallback. + - read_misconceptions_for_course: None course_id short-circuit. + - read_misconceptions_for_course: table read + missing-text drop. + +Mocks `db.connection.table` via patch on the imported reference inside +`agents.tools.graph_read`, mirroring the pattern used in +`tests/test_documents_routes.py`. +""" +from __future__ import annotations + +import asyncio +from unittest.mock import patch + +from agents.tools.graph_read import ( + read_concepts_for_user, + read_misconceptions_for_course, +) + + +def _run(coro): + """Drive an async coroutine to completion in a sync test.""" + return asyncio.run(coro) + + +# ── read_concepts_for_user ──────────────────────────────────────────────── + + +class TestReadConceptsForUser: + def test_returns_sorted_ascending_and_drops_missing_names(self): + # Supabase honors order=mastery_score.asc server-side; we mirror + # that here so we can assert the post-mapping shape is preserved. + rows = [ + { + "concept_name": "pointers", + "mastery_score": 0.1, + "last_studied_at": "2026-05-01T12:00:00Z", + }, + { + # Row missing concept_name should be dropped. + "concept_name": "", + "mastery_score": 0.25, + "last_studied_at": None, + }, + { + "concept_name": "recursion", + "mastery_score": 0.4, + "last_studied_at": "2026-04-28T08:00:00Z", + }, + { + "concept_name": "loops", + "mastery_score": 0.85, + "last_studied_at": None, + }, + ] + with patch("agents.tools.graph_read.table") as t: + t.return_value.select.return_value = rows + result = _run(read_concepts_for_user("user_andres", "course_cs101")) + + # Ordering matches input (server-sorted ASC); dropped row is gone. + assert [c.concept_name for c in result] == [ + "pointers", + "recursion", + "loops", + ] + assert result[0].mastery == 0.1 + assert result[0].last_reviewed_at == "2026-05-01T12:00:00Z" + assert result[2].last_reviewed_at is None + + # Verify the underlying read uses the right table + order arg. + t.assert_called_with("graph_nodes") + select_kwargs = t.return_value.select.call_args + assert "mastery_score.asc" in str(select_kwargs) + assert "course_cs101" in str(select_kwargs) + assert "user_andres" in str(select_kwargs) + + def test_returns_empty_on_supabase_error(self): + with patch("agents.tools.graph_read.table") as t: + t.return_value.select.side_effect = RuntimeError("boom") + result = _run(read_concepts_for_user("user_andres", "course_cs101")) + + assert result == [] + + def test_omits_course_filter_when_course_id_none(self): + with patch("agents.tools.graph_read.table") as t: + t.return_value.select.return_value = [] + _run(read_concepts_for_user("user_andres", None)) + + select_kwargs = t.return_value.select.call_args + # course_id should not appear when None was passed. + assert "course_id" not in str(select_kwargs.kwargs.get("filters") or {}) + + +# ── read_misconceptions_for_course ──────────────────────────────────────── + + +class TestReadMisconceptionsForCourse: + def test_returns_empty_when_course_id_is_none(self): + # No table call should happen at all. + with patch("agents.tools.graph_read.table") as t: + result = _run(read_misconceptions_for_course(None)) + + assert result == [] + t.assert_not_called() + + def test_reads_table_and_drops_missing_text(self): + rows = [ + { + "concept_name": "pointers", + "common_misconceptions": [ + "Dangling pointers always crash", + "", # blank — should be dropped + "Memory leaks are caught by GC", + ], + }, + { + "concept_name": "recursion", + "common_misconceptions": [ + "All recursion is infinite", + "Dangling pointers always crash", # dup — case-insensitive + ], + }, + { + "concept_name": "loops", + "common_misconceptions": None, # missing array — should be skipped + }, + ] + with patch("agents.tools.graph_read.table") as t: + t.return_value.select.return_value = rows + result = _run(read_misconceptions_for_course("course_cs101")) + + assert [m.text for m in result] == [ + "Dangling pointers always crash", + "Memory leaks are caught by GC", + "All recursion is infinite", + ] + # Each entry carries the originating concept. + assert result[0].related_concept == "pointers" + assert result[1].related_concept == "pointers" + assert result[2].related_concept == "recursion" + + t.assert_called_with("course_concept_stats") + select_kwargs = t.return_value.select.call_args + assert "course_cs101" in str(select_kwargs) diff --git a/backend/tests/test_quiz_agent_imports.py b/backend/tests/test_quiz_agent_imports.py new file mode 100644 index 00000000..dc539304 --- /dev/null +++ b/backend/tests/test_quiz_agent_imports.py @@ -0,0 +1,35 @@ +"""Import smoke test for the quiz agent. Live-Gemini behavior is +covered by the eval set in tests/evals/quiz_generation.py (run via +SAPLING_EVAL_MODE=record/replay).""" + + +def test_quiz_agent_imports_and_has_tools(): + from agents.quiz import quiz_agent, Quiz + + assert quiz_agent.deps_type.__name__ == "SaplingDeps" + assert quiz_agent.output_type is Quiz + # Both graph-read tools should be registered. In pydantic-ai 1.89, + # the function-tool registry lives on Agent._function_toolset.tools + # as a dict keyed by tool name. (Agent.toolset is a method on the + # public surface, not the toolset object — don't reach for it.) + tool_names = set(quiz_agent._function_toolset.tools.keys()) + assert "read_concepts_for_user_tool" in tool_names + assert "read_misconceptions_for_course_tool" in tool_names + + +def test_quiz_question_fields_align_with_route_contract(): + """The route writes these fields back to the quiz row; if you + rename one, the route refactor in routes/quiz.py needs to follow.""" + from agents.quiz import QuizQuestion + + fields = set(QuizQuestion.model_fields.keys()) + expected = { + "question", + "type", + "difficulty", + "options", + "correct_answer", + "explanation", + "concept", + } + assert fields == expected diff --git a/backend/tests/test_quiz_routes.py b/backend/tests/test_quiz_routes.py index 6a5d2c28..ad5d2cbf 100644 --- a/backend/tests/test_quiz_routes.py +++ b/backend/tests/test_quiz_routes.py @@ -5,13 +5,17 @@ - Mastery score update formula (scoring math) - POST /api/quiz/submit — answer grading and result shape - POST /api/quiz/submit — 404 when quiz not found +- POST /api/quiz/generate — agent success path (quiz_agent.run mocked) +- POST /api/quiz/generate — agent failure falls back to legacy """ import pytest from contextlib import contextmanager -from unittest.mock import MagicMock, patch +from types import SimpleNamespace +from unittest.mock import AsyncMock, MagicMock, patch from fastapi.testclient import TestClient from main import app +from agents.quiz import Quiz, QuizQuestion client = TestClient(app) @@ -234,3 +238,548 @@ def factory(name): assert r.status_code == 200 assert r.json()["score"] == 2 + + +# ── POST /api/quiz/generate ────────────────────────────────────────────────── + + +def _generate_table_factory(*, course_id: str = "course1", concept_name: str = "Loops"): + """Return a table factory that satisfies generate_quiz's DB reads.""" + + def factory(name): + mock = MagicMock() + if name == "graph_nodes": + mock.select.return_value = [{ + "id": "node1", + "course_id": course_id, + "concept_name": concept_name, + "mastery_score": 0.5, + }] + elif name == "quiz_attempts": + mock.insert.return_value = [{"id": "quiz-generated"}] + else: + mock.select.return_value = [] + mock.insert.return_value = [] + return mock + + return factory + + +class TestQuizAgentSuccess: + """Happy path: quiz_agent.run returns a Quiz, route persists + responds.""" + + def test_returns_agent_output_in_legacy_wire_shape(self): + fake_quiz = Quiz(questions=[ + QuizQuestion( + question="What is 2+2?", + type="multiple_choice", + difficulty="easy", + options=["3", "4", "5", "6"], + correct_answer="4", + explanation="Basic arithmetic.", + concept="Arithmetic", + ), + ]) + with ( + patch("routes.quiz.table", side_effect=_generate_table_factory()), + patch( + "routes.quiz.quiz_agent.run", + new=AsyncMock(return_value=SimpleNamespace(output=fake_quiz)), + ), + ): + r = client.post("/api/quiz/generate", json={ + "user_id": "user_andres", + "concept_node_id": "node1", + "num_questions": 1, + "difficulty": "easy", + "use_shared_context": False, + }) + + assert r.status_code == 200 + data = r.json() + assert "quiz_id" in data + assert isinstance(data["questions"], list) + assert len(data["questions"]) == 1 + + q = data["questions"][0] + # Wire format must match the legacy shape submit_quiz expects. + assert q["id"] == 1 + assert q["question"] == "What is 2+2?" + assert q["explanation"] == "Basic arithmetic." + assert q["concept_tested"] == "Arithmetic" + assert q["difficulty"] == "easy" + assert isinstance(q["options"], list) + assert len(q["options"]) == 4 + # Each option has label/text/correct. + labels = [o["label"] for o in q["options"]] + assert labels == ["A", "B", "C", "D"] + # Exactly one correct option, and it's the one whose text == "4". + correct = [o for o in q["options"] if o["correct"]] + assert len(correct) == 1 + assert correct[0]["text"] == "4" + + def test_short_answer_type_is_rejected_at_schema_layer(self): + """short_answer was dropped from QuizQuestionType in refactor #2 + because the frontend has no UI for free-text answers and + submit_quiz grades by option-label lookup. Constructing a + QuizQuestion with type='short_answer' must raise a Pydantic + validation error — not silently produce an ungradable item. + """ + from pydantic import ValidationError + try: + QuizQuestion( + question="Define a closure.", + type="short_answer", # type: ignore[arg-type] + difficulty="medium", + options=["a", "b", "c", "d"], + correct_answer="a", + explanation="x", + concept="Closures", + ) + except ValidationError: + pass + else: + raise AssertionError( + "Short-answer regression: QuizQuestion accepted " + "type='short_answer'. The Literal must stay MCQ-only " + "until real short-answer grading exists." + ) + + def test_persists_to_quiz_attempts_table(self): + fake_quiz = Quiz(questions=[ + QuizQuestion( + question="Q?", + type="multiple_choice", + difficulty="easy", + options=["a", "b", "c", "d"], + correct_answer="a", + explanation="ok", + concept="X", + ), + ]) + # Capture the insert call so we can assert what's stored. + captured = {} + + def factory(name): + mock = MagicMock() + if name == "graph_nodes": + mock.select.return_value = [{ + "id": "node1", + "course_id": "course1", + "concept_name": "X", + "mastery_score": 0.5, + }] + elif name == "quiz_attempts": + def _capture(payload): + captured["payload"] = payload + return [{"id": payload["id"]}] + mock.insert.side_effect = _capture + return mock + + with ( + patch("routes.quiz.table", side_effect=factory), + patch( + "routes.quiz.quiz_agent.run", + new=AsyncMock(return_value=SimpleNamespace(output=fake_quiz)), + ), + ): + r = client.post("/api/quiz/generate", json={ + "user_id": "user_andres", + "concept_node_id": "node1", + "num_questions": 1, + "difficulty": "easy", + "use_shared_context": False, + }) + + assert r.status_code == 200 + # quiz_attempts row stores the same legacy-shape questions list. + payload = captured["payload"] + assert payload["user_id"] == "user_andres" + assert payload["concept_node_id"] == "node1" + assert payload["difficulty"] == "easy" + assert isinstance(payload["questions_json"], list) + assert payload["questions_json"][0]["id"] == 1 + + +class TestQuizAgentFallback: + """When the agent trips, the route runs the legacy generation pipeline.""" + + def _legacy_response(self): + # The legacy-shape question that the original quiz_generation prompt + # produces: id, question, options[{label,text,correct}], etc. + return { + "questions": [ + { + "id": 1, + "question": "Legacy fallback question?", + "options": [ + {"label": "A", "text": "wrong", "correct": False}, + {"label": "B", "text": "right", "correct": True}, + ], + "explanation": "Because.", + "concept_tested": "Loops", + "difficulty": "easy", + }, + ] + } + + def _patch_legacy_dependencies(self): + """Patch every dep _legacy_generate_quiz reaches for besides table().""" + return ( + patch( + "routes.quiz.get_graph", + return_value={"nodes": [], "edges": []}, + ), + patch("routes.quiz.get_quiz_context", return_value={}), + patch( + "routes.quiz.call_gemini_json", + return_value=self._legacy_response(), + ), + ) + + def test_falls_back_to_legacy_on_usage_limit_exceeded(self): + from pydantic_ai.exceptions import UsageLimitExceeded + + get_graph_p, get_ctx_p, gemini_p = self._patch_legacy_dependencies() + with ( + patch("routes.quiz.table", side_effect=_generate_table_factory()), + patch( + "routes.quiz.quiz_agent.run", + new=AsyncMock( + side_effect=UsageLimitExceeded("token cap"), + ), + ), + get_graph_p as _get_graph, + get_ctx_p as _get_ctx, + gemini_p as gemini_mock, + ): + r = client.post("/api/quiz/generate", json={ + "user_id": "user_andres", + "concept_node_id": "node1", + "num_questions": 1, + "difficulty": "easy", + "use_shared_context": False, + }) + + assert r.status_code == 200 + gemini_mock.assert_called_once() # legacy path actually ran + q = r.json()["questions"][0] + assert q["question"] == "Legacy fallback question?" + # Wire format from legacy is preserved verbatim (it already matches). + assert q["options"][1]["correct"] is True + + def test_falls_back_to_legacy_on_unexpected_exception(self): + get_graph_p, get_ctx_p, gemini_p = self._patch_legacy_dependencies() + with ( + patch("routes.quiz.table", side_effect=_generate_table_factory()), + patch( + "routes.quiz.quiz_agent.run", + new=AsyncMock(side_effect=RuntimeError("boom")), + ), + get_graph_p, + get_ctx_p, + gemini_p as gemini_mock, + ): + r = client.post("/api/quiz/generate", json={ + "user_id": "user_andres", + "concept_node_id": "node1", + "num_questions": 1, + "difficulty": "easy", + "use_shared_context": False, + }) + + assert r.status_code == 200 + gemini_mock.assert_called_once() + assert r.json()["questions"][0]["question"] == "Legacy fallback question?" + + def test_falls_back_to_legacy_when_all_questions_drift(self): + """Cascade: agent succeeds but every question fails wire-format + validation → _quiz_via_agent raises RuntimeError → + bare-Exception catch in generate_quiz routes to legacy. + + This pins the path the 3 contract tests don't directly exercise + (they test _agent_question_to_wire in isolation; this exercises + the full route under the all-drift condition). + """ + # Build a Quiz where every question's correct_answer doesn't + # appear in its options — schema-valid, but the wire-format + # check drops every one. + drift_quiz = Quiz(questions=[ + QuizQuestion( + question=f"Q{i}?", + type="multiple_choice", + difficulty="easy", + options=["a", "b", "c", "d"], + correct_answer="MISMATCH", # not in options + explanation="x", + concept="X", + ) + for i in range(3) + ]) + + get_graph_p, get_ctx_p, gemini_p = self._patch_legacy_dependencies() + agent_run_mock = AsyncMock(return_value=SimpleNamespace(output=drift_quiz)) + with ( + patch("routes.quiz.table", side_effect=_generate_table_factory()), + patch("routes.quiz.quiz_agent.run", new=agent_run_mock), + get_graph_p, + get_ctx_p, + gemini_p as gemini_mock, + ): + r = client.post("/api/quiz/generate", json={ + "user_id": "user_andres", + "concept_node_id": "node1", + "num_questions": 3, + "difficulty": "easy", + "use_shared_context": False, + }) + + assert r.status_code == 200 + # Pin BOTH halves of the cascade contract: + # 1. The agent path was actually tried (not skipped). + agent_run_mock.assert_called_once() + # 2. The legacy path then fired (every question dropped → + # RuntimeError → bare-Exception catch → _legacy_generate_quiz). + gemini_mock.assert_called_once() + assert r.json()["questions"][0]["question"] == "Legacy fallback question?" + + +# ── Wire-format contract: pinned by tests so silent drift can't recur ─────── + +class TestQuizWireFormatContract: + """Pin the invariant `_agent_question_to_wire` enforces: every emitted + question has exactly one correct option, and the correct option's text + matches the agent's `correct_answer`. Drift cases (LLM emits a + correct_answer not in options) drop the question rather than silently + mis-marking it. + """ + + def test_well_formed_question_passes_through(self): + from agents.quiz import QuizQuestion + from routes.quiz import _agent_question_to_wire + + q = QuizQuestion( + question="What is 2 + 2?", + type="multiple_choice", + difficulty="easy", + options=["3", "4", "5", "6"], + correct_answer="4", + explanation="Basic arithmetic.", + concept="Arithmetic", + ) + wire = _agent_question_to_wire(q, qid=1) + assert wire is not None + # Exactly one option flagged correct. + correct = [o for o in wire["options"] if o["correct"]] + assert len(correct) == 1, f"expected 1 correct, got {len(correct)}" + # The correct option's text matches the canonical answer verbatim. + assert correct[0]["text"] == q.correct_answer + # Labels are A, B, C, D in order. + assert [o["label"] for o in wire["options"]] == ["A", "B", "C", "D"] + + def test_correct_answer_not_in_options_drops_question(self): + """Generation drift: the agent's `correct_answer` doesn't appear + in `options`. The wrapper returns None; the caller filters it out. + Silent first-option-correct fallback must NOT happen. + """ + from agents.quiz import QuizQuestion + from routes.quiz import _agent_question_to_wire + + q = QuizQuestion( + question="What is 2 + 2?", + type="multiple_choice", + difficulty="easy", + options=["3", "5", "6", "7"], # 4 is missing + correct_answer="4", + explanation="Basic arithmetic.", + concept="Arithmetic", + ) + wire = _agent_question_to_wire(q, qid=1) + assert wire is None, ( + "Silent fallback regression: the wrapper must not mark an " + "arbitrary option correct when the agent's correct_answer " + "isn't present verbatim — drop the question instead." + ) + + def test_whitespace_only_difference_still_matches(self): + """The wrapper trims whitespace before comparing — minor LLM + whitespace drift shouldn't drop the question.""" + from agents.quiz import QuizQuestion + from routes.quiz import _agent_question_to_wire + + q = QuizQuestion( + question="What is 2 + 2?", + type="multiple_choice", + difficulty="easy", + options=["3", " 4 ", "5", "6"], + correct_answer="4", + explanation="Basic arithmetic.", + concept="Arithmetic", + ) + wire = _agent_question_to_wire(q, qid=1) + assert wire is not None + correct = [o for o in wire["options"] if o["correct"]] + assert len(correct) == 1 + # The matched option preserves its original (whitespace-padded) + # text — the trim is only used for comparison. + assert correct[0]["text"].strip() == "4" + + +# ── Per-request model_pref (Fast/Smart toggle, mirrors chat tutor) ────────── + +class TestQuizModelPref: + """Pin the per-request fast/smart toggle introduced after PR #73's + chat-tutor model toggle landed on main. Both routes now expose the + same body field; this class locks the dispatch contract end-to-end. + """ + + def _fake_quiz(self): + return Quiz(questions=[ + QuizQuestion( + question="What is 2 + 2?", + type="multiple_choice", + difficulty="easy", + options=["3", "4", "5", "6"], + correct_answer="4", + explanation="Basic arithmetic.", + concept="Arithmetic", + ), + ]) + + def _post(self, body_extra: dict): + return client.post("/api/quiz/generate", json={ + "user_id": "user_andres", + "concept_node_id": "node1", + "num_questions": 1, + "difficulty": "easy", + "use_shared_context": False, + **body_extra, + }) + + def test_smart_pref_overrides_agent_model(self): + """model_pref='smart' → agent.run is called with model=GoogleModel('gemini-2.5-pro').""" + run_mock = AsyncMock(return_value=SimpleNamespace(output=self._fake_quiz())) + with ( + patch("routes.quiz.table", side_effect=_generate_table_factory()), + patch("routes.quiz.quiz_agent.run", new=run_mock), + ): + r = self._post({"model_pref": "smart"}) + assert r.status_code == 200 + assert run_mock.call_count == 1 + kwargs = run_mock.call_args.kwargs + assert "model" in kwargs, "smart pref must pass an explicit model override" + assert kwargs["model"].model_name == "gemini-2.5-pro" + + def test_fast_pref_overrides_agent_model(self): + """model_pref='fast' → agent.run is called with model=GoogleModel('gemini-2.5-flash').""" + run_mock = AsyncMock(return_value=SimpleNamespace(output=self._fake_quiz())) + with ( + patch("routes.quiz.table", side_effect=_generate_table_factory()), + patch("routes.quiz.quiz_agent.run", new=run_mock), + ): + r = self._post({"model_pref": "fast"}) + assert r.status_code == 200 + kwargs = run_mock.call_args.kwargs + assert "model" in kwargs + assert kwargs["model"].model_name == "gemini-2.5-flash" + + def test_no_pref_falls_through_to_agent_default(self): + """No model_pref → agent.run gets NO model kwarg, falls back to model_for('quiz').""" + run_mock = AsyncMock(return_value=SimpleNamespace(output=self._fake_quiz())) + with ( + patch("routes.quiz.table", side_effect=_generate_table_factory()), + patch("routes.quiz.quiz_agent.run", new=run_mock), + ): + r = self._post({}) # no model_pref + assert r.status_code == 200 + kwargs = run_mock.call_args.kwargs + assert "model" not in kwargs, ( + "Without model_pref, the route must NOT inject a model override — " + "the agent's task-default (model_for('quiz')) should win." + ) + + def test_unknown_pref_falls_through_to_agent_default(self): + """An unrecognized preference (e.g. legacy clients sending 'auto') + falls through to the agent default rather than raising. Pydantic + validation already restricts the body type to fast|smart|None, + but we double-belt at the resolver layer.""" + from routes.quiz import _resolve_model_pref + assert _resolve_model_pref(None) is None + assert _resolve_model_pref("") is None + assert _resolve_model_pref("auto") is None # not in the map + + def test_legacy_fallback_uses_smart_when_pref_smart(self): + """If the agent path trips and we fall through to legacy, + the same preference must propagate — call_gemini_json gets + MODEL_SMART instead of MODEL_LITE.""" + from services.gemini_service import MODEL_SMART + run_mock = AsyncMock(side_effect=RuntimeError("agent boom")) + gemini_mock = MagicMock(return_value={"questions": [{ + "id": 1, "question": "Q?", + "options": [{"label": "A", "text": "x", "correct": True}], + "explanation": ".", "concept_tested": "X", "difficulty": "easy", + }]}) + with ( + patch("routes.quiz.table", side_effect=_generate_table_factory()), + patch("routes.quiz.quiz_agent.run", new=run_mock), + patch("routes.quiz.get_graph", return_value={"nodes": [], "edges": []}), + patch("routes.quiz.get_quiz_context", return_value={}), + patch("routes.quiz.call_gemini_json", new=gemini_mock), + ): + r = self._post({"model_pref": "smart"}) + assert r.status_code == 200 + # call_gemini_json was called with model=MODEL_SMART, not MODEL_LITE. + gemini_mock.assert_called_once() + assert gemini_mock.call_args.kwargs.get("model") == MODEL_SMART + + def test_legacy_fallback_uses_default_when_pref_fast(self): + """Symmetry contract: legacy "fast" must upgrade to MODEL_DEFAULT + (gemini-2.5-flash) just like the agent path does. Without this, + a Fast request that trips the agent silently downgrades to + MODEL_LITE — the same kind of inconsistency users would feel + as "Fast sometimes gives worse quizzes than Default." + """ + from services.gemini_service import MODEL_DEFAULT, MODEL_LITE + run_mock = AsyncMock(side_effect=RuntimeError("agent boom")) + gemini_mock = MagicMock(return_value={"questions": [{ + "id": 1, "question": "Q?", + "options": [{"label": "A", "text": "x", "correct": True}], + "explanation": ".", "concept_tested": "X", "difficulty": "easy", + }]}) + with ( + patch("routes.quiz.table", side_effect=_generate_table_factory()), + patch("routes.quiz.quiz_agent.run", new=run_mock), + patch("routes.quiz.get_graph", return_value={"nodes": [], "edges": []}), + patch("routes.quiz.get_quiz_context", return_value={}), + patch("routes.quiz.call_gemini_json", new=gemini_mock), + ): + r = self._post({"model_pref": "fast"}) + assert r.status_code == 200 + gemini_mock.assert_called_once() + chosen = gemini_mock.call_args.kwargs.get("model") + assert chosen == MODEL_DEFAULT, ( + f"Legacy fast→{chosen!r} expected MODEL_DEFAULT (gemini-2.5-flash); " + f"do NOT downgrade to MODEL_LITE (={MODEL_LITE!r})." + ) + + def test_legacy_fallback_uses_lite_when_no_pref(self): + """Default path (no model_pref) keeps using MODEL_LITE — the + cheap baseline that's been in place since the route shipped.""" + from services.gemini_service import MODEL_LITE + run_mock = AsyncMock(side_effect=RuntimeError("agent boom")) + gemini_mock = MagicMock(return_value={"questions": [{ + "id": 1, "question": "Q?", + "options": [{"label": "A", "text": "x", "correct": True}], + "explanation": ".", "concept_tested": "X", "difficulty": "easy", + }]}) + with ( + patch("routes.quiz.table", side_effect=_generate_table_factory()), + patch("routes.quiz.quiz_agent.run", new=run_mock), + patch("routes.quiz.get_graph", return_value={"nodes": [], "edges": []}), + patch("routes.quiz.get_quiz_context", return_value={}), + patch("routes.quiz.call_gemini_json", new=gemini_mock), + ): + r = self._post({}) # no model_pref + assert r.status_code == 200 + gemini_mock.assert_called_once() + assert gemini_mock.call_args.kwargs.get("model") == MODEL_LITE diff --git a/backend/tests/test_shared_course_context.py b/backend/tests/test_shared_course_context.py index 9f39a736..cb0756d5 100644 --- a/backend/tests/test_shared_course_context.py +++ b/backend/tests/test_shared_course_context.py @@ -497,17 +497,39 @@ def test_build_system_prompt_mode_appended_after_shared_block(self, mock_ctx, mo # ───────────────────────────────────────────────────────────────────────────── class TestQuizPromptAugmentation(unittest.TestCase): + """Legacy prompt-augmentation tests. + + Post-refactor (ADR 0005), generate_quiz routes through quiz_agent + first and only falls through to the legacy `_legacy_generate_quiz` + path on agent failure. Each test below patches `quiz_agent.run` to + raise so the legacy code path actually executes — that is the path + these assertions describe (manual prompt-string augmentation against + course_context_service output). + """ def _make_generate_body(self): - from models import GenerateQuizBody - return GenerateQuizBody( - user_id="user1", - concept_node_id="node-abc", - difficulty="medium", - num_questions=3, + return { + "user_id": "user1", + "concept_node_id": "node-abc", + "difficulty": "medium", + "num_questions": 3, + } + + def _force_legacy(self): + """Patch quiz_agent.run to raise so the route falls back to legacy.""" + from unittest.mock import AsyncMock + return patch( + "routes.quiz.quiz_agent.run", + new=AsyncMock(side_effect=RuntimeError("force legacy fallback for tests")), ) - # get_course_context is lazily imported inside generate_quiz; + def _post_generate(self): + """POST to /api/quiz/generate via TestClient.""" + from fastapi.testclient import TestClient + from main import app + return TestClient(app).post("/api/quiz/generate", json=self._make_generate_body()) + + # get_course_context is lazily imported inside _legacy_generate_quiz; # patch at the source module so the `from ... import` resolves to our mock. @patch("services.course_context_service.get_course_context") @patch("routes.quiz.call_gemini_json") @@ -535,8 +557,8 @@ def test_misconceptions_appended_to_prompt( } mock_gemini.return_value = {"questions": []} - from routes.quiz import generate_quiz - generate_quiz(self._make_generate_body(), MagicMock()) + with self._force_legacy(): + self._post_generate() actual_prompt = mock_gemini.call_args[0][0] self.assertIn("Dangling pointers", actual_prompt) @@ -559,8 +581,8 @@ def test_no_augmentation_when_ctx_empty( mock_graph.return_value = {"nodes": [], "edges": []} mock_gemini.return_value = {"questions": []} - from routes.quiz import generate_quiz - generate_quiz(self._make_generate_body(), MagicMock()) + with self._force_legacy(): + self._post_generate() actual_prompt = mock_gemini.call_args[0][0] self.assertNotIn("Common misconceptions seen across the class", actual_prompt) @@ -591,8 +613,8 @@ def test_augmentation_capped_at_10_items( } mock_gemini.return_value = {"questions": []} - from routes.quiz import generate_quiz - generate_quiz(self._make_generate_body(), MagicMock()) + with self._force_legacy(): + self._post_generate() actual_prompt = mock_gemini.call_args[0][0] self.assertIn("mistake_9", actual_prompt) @@ -613,10 +635,10 @@ def test_no_augmentation_when_node_has_no_course_id( mock_graph.return_value = {"nodes": [], "edges": []} mock_gemini.return_value = {"questions": []} - with patch("services.course_context_service.get_course_context") as mock_ctx: - from routes.quiz import generate_quiz - generate_quiz(self._make_generate_body(), MagicMock()) - mock_ctx.assert_not_called() + with self._force_legacy(): + with patch("services.course_context_service.get_course_context") as mock_ctx: + self._post_generate() + mock_ctx.assert_not_called() if __name__ == "__main__": diff --git a/docs/decisions/0013-refactor-2-quiz-shipped.md b/docs/decisions/0013-refactor-2-quiz-shipped.md new file mode 100644 index 00000000..1f2e28b1 --- /dev/null +++ b/docs/decisions/0013-refactor-2-quiz-shipped.md @@ -0,0 +1,207 @@ +# 0013: Refactor #2 (quiz_agent) shipped + +- Status: accepted +- Date: 2026-05-04 +- Supersedes: refines 0005 + +## Context + +ADR 0005 picked `routes/quiz.py::generate_quiz` as refactor #2. This ADR captures +what shipped, what surprised us, and what we learned for refactor #3 (chat tutor). + +## Decision + +`quiz_agent` lives at `backend/agents/quiz.py` and is wired into the existing +`POST /api/quiz/generate` route via the same orchestrator-vs-legacy fallback +pattern PR #67 established for document upload. Output schema is intentionally +flat (`Quiz.questions: list[QuizQuestion]`) per ADR 0003 convention 4 — Gemini +rejects nested-state structured-output schemas. + +Prompt version: `17ab80b30316` (sha256[:12] of the system prompt). Future prompt +changes bump this hash; Logfire traces tag every quiz run with the active +version so we can answer "which prompt produced this quiz three weeks ago?" + +## What shipped + +- `backend/agents/quiz.py` — typed agent with `Quiz` + `QuizQuestion` outputs, + registers `read_concepts_for_user_tool` + `read_misconceptions_for_course_tool`. +- `backend/agents/_providers.py` — added `quiz` task; default + `gemini-2.5-flash-lite`, override via `SAPLING_MODEL_QUIZ`. +- `backend/agents/tools/graph_read.py` — pure-async + tool-wrapped reads of + concept mastery and class-level misconceptions, per ADR 0004. +- `backend/routes/quiz.py` — `_quiz_via_agent` (new path) and + `_legacy_generate_quiz` (preserved fallback). Route is now async; falls back + on `UsageLimitExceeded` / `UnexpectedModelBehavior` / any other exception. +- `backend/tests/evals/quiz_generation.py` — 8 cases × 6 evaluators per ADR 0005 + (3 difficulty × 2 type, all 6 cells covered). +- `backend/tests/test_quiz_routes.py` — 19 tests (was 14; added 3 agent-success + + 2 agent-fallback). +- `backend/tests/test_quiz_agent_imports.py` — 2 smoke tests. +- `backend/tests/test_graph_read_tools.py` — 5 tests for the new tool surface. + +## What surprised us + +1. **The "quiz cache" doesn't exist as I'd assumed in ADR 0005.** + `quiz_context_service.get_quiz_context` is a per-(user, concept) post-quiz + notes service, NOT a generated-quiz cache. There's no `(user, course, + settings)` cache key to invalidate on prompt-version bumps. Decision: don't + add one in this refactor. Cache concerns deferred to "if we ever build a + real quiz cache." + +2. **`GenerateQuizBody` has no `course_id` field.** Course context is derived + from the target concept's `graph_nodes.course_id`. `_quiz_via_agent` reads + that row first, then threads `course_id` into `SaplingDeps` so the + agent's tools scope correctly. + +3. **Wire-format mismatch between agent output and persisted shape.** Legacy + `quizzes_json` rows store `{question, options:[{label,text,correct}], + explanation, concept_tested, difficulty}` with options as an array of + labeled objects. `Quiz.questions` is flatter: `options:[str]` plus + `correct_answer:str`. Built `_agent_question_to_wire` to convert. Frontend + submission and grading paths are unchanged — wire shape held constant. + +4. **`misconceptions` table doesn't exist; data lives in + `course_concept_stats.common_misconceptions[]`.** ADR 0004's spec called + for a table that doesn't exist; sub-agent A read the actual aggregated + shape and flattened it correctly. The function contract + (`Misconception[]`) holds; only the SELECT changed. + +5. **Short-answer grading doesn't exist yet.** `submit_quiz` grades by + `q["options"][i].correct` lookup. For short-answer questions, the wrapper + synthesizes a single-option `{label:"A", correct:True}` so existing + grading code keeps working. Real short-answer grading (e.g. fuzzy-match, + LLM-judged) is its own future scope. + +6. **`HTTPException` had to be re-raised inside the catch.** The route's + try/except Exception fallback would otherwise swallow legitimate 404s + ("Concept node not found") and call legacy recursively. Caught + StarletteHTTPException explicitly and re-raised before the bare except. + +## Consequences + +- (+) Quiz generation is typed end-to-end. Downstream UI parses structured + questions instead of regex'ing strings. +- (+) Misconception use is auditable in Logfire — span trees show + `read_misconceptions_for_course_tool` arguments + return value. +- (+) Per-task model routing means we can A/B `gemini-2.5-flash-lite` vs + `gemini-2.5-flash` on quiz generation by flipping `SAPLING_MODEL_QUIZ` + without touching code. +- (+) Eval cassettes (when recorded) gate prompt changes via the existing + `.github/workflows/evals.yml` workflow. +- (−) Legacy quiz path is now ~150 lines of code we still ship. Can't delete + until refactor #3 (chat tutor) ships per ADR 0001's migration plan. +- (−) Agent latency may exceed the legacy single-call latency on the cold + path (tool calls add round-trips). Acceptable since quiz generation is a + post-study, non-streaming flow — user is willing to wait. Will measure in + Logfire after ~50 quizzes. +- (−) Test count grew (438 backend, +5 quiz route, +5 graph tools, +2 quiz + imports, +2 size-limit test fixed downstream) — test runtime up ~3s. + Fine. + +## What I'd carry into refactor #3 (chat tutor) + +1. **The "input dict → typed model dict for storage" wrapper pattern.** The + `_agent_question_to_wire` adapter let us change the agent's internal + shape without touching the persisted shape or the frontend. Chat tutor's + message-history shape is the same kind of contract — write the adapter + first, before touching the storage layer. + +2. **`HTTPException` re-raise inside the bare-Exception catch.** Easy to + forget; bites silently. Pattern stays. + +3. **Don't pre-stuff context in the user-message string.** Tools >>> + prompt-string augmentation. Two reasons: (a) Logfire traces show what + the agent fetched and decided to use, (b) the model gets to skip irrelevant + tools when the prompt doesn't need them. + +4. **Deps construction is identical across refactors.** Made a mental note + to extract a `make_deps_from_request(request, user_id, course_id)` + helper in refactor #3 so SSE error payloads, agent traces, and idempotency + keys all share one request_id without three lines of boilerplate per route. + +5. **The eval set is more valuable than the unit tests at finding prompt + regressions.** Unit tests mock the agent, so they only catch wiring bugs. + Eval cases run the real agent (against cassettes) — when refactor #3 + ships, write the eval first, record cassettes, then refactor with + confidence. + +## Addendum (2026-05-04, after PR #71 review) + +Three post-review fixes landed before merge: + +1. **`QuizQuestionType` restricted to `multiple_choice` only.** The + original Literal also accepted `short_answer`, but the route-side + wrapper synthesized a single-option grading shim that didn't actually + work — `submit_quiz` grades by option-label lookup and the frontend + has no UI for free-text answers. Schema-level rejection (Pydantic + ValidationError on `type="short_answer"`) is now the contract; pinned + by `test_short_answer_type_is_rejected_at_schema_layer`. Real + short-answer support (LLM-judged or fuzzy-match grading) is a future + ADR when the frontend has the UI to match. + +2. **`_agent_question_to_wire` no longer silently rewrites correct + flags.** Old behavior: if the agent's `correct_answer` didn't match + any option verbatim, the wrapper marked the FIRST option correct so + `submit_quiz` could "still grade" the attempt. New behavior: log a + warning and return None; the caller filters those out. If all + questions in a generation drift like this, the route raises and + degrades to legacy fallback rather than serving an empty quiz. + Pinned by `TestQuizWireFormatContract` (3 tests: well-formed + passthrough, drift drops, whitespace tolerance). + +3. **Eval cases all-MCQ.** The 3 short-answer cases were rewritten as + MCQ at the same difficulty levels (PHYS 101 definitions, ECON 201, + theory of computation). `ShortAnswerShapeEvaluator` removed. + Module-level assertion verifies all 8 cases are MCQ — adding a + short-answer case will fail to import until short-answer support + actually exists. + +## Addendum (2026-05-04, post-merge with main): per-request fast/smart toggle + +After merging main into the branch, a fourth fix landed to close the +asymmetry between the new agentic quiz route and the chat tutor: + +- **Main shipped `model_pref: Literal["fast", "smart"]`** on the chat + body (PR #73), letting users pay for `gemini-2.5-pro` per request. +- **The quiz route originally had only `SAPLING_MODEL_QUIZ`**, an + env-var-only override. + +Resolved by adding the same `model_pref` field to `GenerateQuizBody` +and threading it through both paths: + +- Agent path: `_quiz_via_agent` builds an optional model override via + `_resolve_model_pref(...)` and passes `model=` per call to + `quiz_agent.run`. None falls through to the agent's + `model_for("quiz")` default. +- Legacy fallback: `_legacy_generate_quiz` swaps `MODEL_LITE` for + `MODEL_SMART` when `body.model_pref == "smart"`. The fast/None + cases stay on `MODEL_LITE`. + +Pinned by `TestQuizModelPref` (5 tests): smart → flash-pro override, +fast → flash-flash override, no-pref falls through, unknown pref +falls through, and legacy fallback honors smart. + +Decision rationale: keep the env var (`SAPLING_MODEL_QUIZ`) for ops +defaults AND accept `model_pref` for per-request overrides. They are +two independent layers, not multiplicative — the env var sets the +agent's startup baseline (`model_for("quiz")` reads it at process +start); the body field, when present, fully replaces that default +for the current call by passing `model=...` to `quiz_agent.run`. +Same shape as the chat tutor on main, so the two AI-driven routes +are now symmetric. + +`fast` and `smart` resolve identically across the agent path AND the +legacy fallback (commit ddd109b initially missed this; it was fixed +in the same iteration so a `fast` request that trips the agent +guardrails still gets `gemini-2.5-flash`, not silently downgraded to +`gemini-2.5-flash-lite`). The route's `_PREF_MODEL_NAMES` table is +the single source of truth; the legacy `else`/`elif` chain mirrors +it exactly. + +## Pre-existing test failures (not caused by this refactor) + +- `tests/test_documents_routes.py::test_rejects_file_over_15mb` — asserts + against the OLD 15 MB cap. Cap was bumped to 100 MB in commit 9912a25. + Worth a separate one-line test fix; not blocking refactor #2. +- `test_graph_service.py::test_skips_self_edges` and + `test_ocr_pipeline.py::*` — live-Supabase 409 conflicts, pre-existing.