diff --git a/backend/db/migrations/20260822090747_quiz_attempts_exam_days_away.sql b/backend/db/migrations/20260822090747_quiz_attempts_exam_days_away.sql new file mode 100644 index 00000000..dbae4777 --- /dev/null +++ b/backend/db/migrations/20260822090747_quiz_attempts_exam_days_away.sql @@ -0,0 +1,29 @@ +-- H3 (#555): how many days away the student's next exam was when this quiz +-- was generated. +-- +-- Stored on the attempt, not merely used in the prompt, because the point of +-- the issue is to be able to ASK LATER whether deadline-aware quizzes perform +-- differently. A value that only ever reached a prompt leaves no way to +-- compare a quiz taken three days before a midterm against one taken in week +-- two — the measurement is the deliverable, not the prompt line. +-- +-- Nullable with no default, and the distinction is load-bearing: +-- NULL = we did not know (no course, no enrollment, no dated exams, or the +-- lookup failed — `days_until_next_exam` degrades to None rather +-- than failing a generation) +-- 0 = the exam is TODAY, which is the most actionable value the feature +-- produces. A DEFAULT 0 would make every legacy row and every +-- unknown look like exam day. +-- +-- Dates only. This column is a day count derived from `assignments.due_date`, +-- which is plaintext and indexed; no grade VALUE is read, stored or prompted +-- anywhere on this path (points columns are encrypted per #521, and the audit +-- flags that the current ToS does not clearly cover feeding grades to a +-- model). +-- +-- Additive and idempotent. Ordering still matters: PostgREST 400s on a column +-- its schema cache doesn't have, and `generate_quiz` includes this key in the +-- attempt INSERT — so this must be applied strictly before the code ships, as +-- with 20260814051517. +ALTER TABLE quiz_attempts + ADD COLUMN IF NOT EXISTS exam_days_away INTEGER; diff --git a/backend/routes/quiz.py b/backend/routes/quiz.py index f703127c..74a443e0 100644 --- a/backend/routes/quiz.py +++ b/backend/routes/quiz.py @@ -42,6 +42,7 @@ from services.encryption import encrypt_json, decrypt_json_column from services.graph_service import apply_graph_update from services.quiz_context_service import get_quiz_context, save_quiz_context +from services.exam_proximity import PROMPT_HORIZON_DAYS, days_until_next_exam from services.quiz_distractors import build_distractor_profile from services.fingerprint import fingerprint from services.quiz_identity import question_hash, normalize_text @@ -795,6 +796,50 @@ def _do_not_repeat_block(recent: list[RecentQuestion]) -> str: ) +def _insert_attempt(attempt_row: dict) -> None: + """Write the attempt, surviving a schema that predates `exam_days_away`. + + The omit-when-None rule alone does NOT make a pre-migration environment + safe, and the failure is nastier than it looks: it strikes exactly the + students the feature is FOR (the ones with a dated upcoming exam), so it + presents as a random partial outage rather than an obvious missing + migration. And it strikes late — the agent has already run and been + billed — so an unhandled 400 here loses the generated quiz, writes no + attempt row, emits no `quiz.generation_failed`, and never refunds the + rate-limit slot. + # + Ordering is still the rule (migration before code, as with + 20260814051517). This is the seatbelt for the window where PostgREST has + not yet reloaded its schema cache, not a licence to deploy first. + """ + try: + table("quiz_attempts").insert(attempt_row) + return + except Exception: + if "exam_days_away" not in attempt_row: + raise + retry = {k: v for k, v in attempt_row.items() if k != "exam_days_away"} + logger.warning( + "quiz: attempt insert failed with exam_days_away present; retrying " + "without it (is 20260822090747 applied?) quiz_id=%s", + attempt_row.get("id"), + ) + table("quiz_attempts").insert(retry) + + +class GeneratedQuiz(NamedTuple): + """What one generation produced. + + `exam_days_away` rides back with the questions rather than being resolved + again by the caller: it is used for the prompt here and stored on the + attempt there, and two lookups could disagree if an exam were entered + between them. + """ + + questions: list[dict] + exam_days_away: int | None = None + + async def _quiz_via_agent( *, user_id: str, @@ -806,7 +851,7 @@ async def _quiz_via_agent( use_shared_context: bool, request_id: str, model_pref: str | None = None, -) -> list[dict]: +) -> GeneratedQuiz: """Run quiz_agent and return questions in the legacy wire shape. The agent's tools (read_concepts_for_user, read_misconceptions_for_course) @@ -849,18 +894,6 @@ async def _quiz_via_agent( difficulty_clause = ( f"Generate {num_questions} {difficulty} questions for the student." ) - routing_msg = ( - f"{difficulty_clause} " - f"The target concept is '{concept_name}' " - f"(concept_node_id={concept_node_id}). Follow the workflow in your " - f"system prompt; pass concept_node_id='{concept_node_id}' to " - f"read_recent_quiz_attempts." - ) - if use_shared_context: - routing_msg += ( - " Also call read_misconceptions_for_course and use those misconceptions " - "as distractors and probes." - ) # Course-material grounding does blocking network I/O (a Gemini # embedding call, bounded at 60s) plus sync Supabase reads. Run it in a @@ -868,23 +901,32 @@ async def _quiz_via_agent( # event loop for every other in-flight request. Matches the # asyncio.to_thread pattern used by the agent read tools. # - # E6's recently-asked read is an independent Supabase read + decrypt, so - # it runs CONCURRENTLY with grounding rather than after it — the two have - # nothing to say to each other and serializing them would add the slower - # one's latency to every generation. + # E6's recently-asked read and H3's exam-proximity lookup are independent + # Supabase reads, so all three run CONCURRENTLY rather than in sequence — + # they have nothing to say to each other, and serializing them would add + # every one of their latencies to every generation. Proximity costs + # several round-trips on its own, so running it before this block made it + # fully additive. # - # return_exceptions=True because BOTH are best-effort context, and a bare - # gather propagates the first failure straight out of generation: an + # return_exceptions=True because ALL THREE are best-effort context, and a + # bare gather propagates the first failure straight out of generation: an # unreadable past attempt would 502 a quiz that needed no history at all. # Each helper already degrades internally; this is the backstop for the # failure they cannot catch (an unexpected raise on the way in or out). - material, recent = await asyncio.gather( + material, recent, exam_days_away = await asyncio.gather( asyncio.to_thread(_course_material, course_id, concept_name), asyncio.to_thread( recent_question_identities, user_id, concept_node_id ), + asyncio.to_thread(days_until_next_exam, user_id, course_id), return_exceptions=True, ) + if isinstance(exam_days_away, BaseException): + logger.warning( + "quiz: exam-proximity lookup failed (%s); generating without it", + type(exam_days_away).__name__, exc_info=exam_days_away, + ) + exam_days_away = None if isinstance(material, BaseException): logger.warning( "quiz: course-material assembly failed (%s); generating ungrounded", @@ -900,6 +942,40 @@ async def _quiz_via_agent( "do-not-repeat list", type(recent).__name__, exc_info=recent, ) recent = [] + + routing_msg = ( + f"{difficulty_clause} " + f"The target concept is '{concept_name}' " + f"(concept_node_id={concept_node_id}). Follow the workflow in your " + f"system prompt; pass concept_node_id='{concept_node_id}' to " + f"read_recent_quiz_attempts." + ) + if use_shared_context: + routing_msg += ( + " Also call read_misconceptions_for_course and use those misconceptions " + "as distractors and probes." + ) + # H3/#555: one line, dates only. Says how near the deadline is and lets + # the model decide what that implies — not "make it harder", which would + # contradict the adaptive difficulty the student actually chose. Omitted + # entirely when unknown: "next exam: unknown" is prompt tokens spent to + # say nothing. + # Bounded by a horizon: a final dated 87 days out would otherwise put + # "there is an exam coming, weight toward what an exam tests" on EVERY + # quiz for the whole semester, which carries no proximity signal and + # steers week-two practice toward exam-style questions — the opposite of + # what this line is for. The stored `exam_days_away` is NOT clamped: the + # analytics want the real distance, including the far ones. + if exam_days_away is not None and exam_days_away <= PROMPT_HORIZON_DAYS: + when = ( + "TODAY" if exam_days_away == 0 + else "tomorrow" if exam_days_away == 1 + else f"in {exam_days_away} days" + ) + routing_msg += ( + f" The student's next exam in this course is {when}. Weight the" + " questions toward what an exam would actually test." + ) _log_rag_uncovered( material, user_id=user_id, @@ -1078,7 +1154,10 @@ def _absorb(quiz: Quiz, model: str) -> None: "quiz_agent produced no valid questions after wire-format validation" ) # Never serve more than asked for (a generous top-up run can overshoot). - return wire_questions[:num_questions] + return GeneratedQuiz( + questions=wire_questions[:num_questions], + exam_days_away=exam_days_away, + ) @router.get("/config") def quiz_config(): @@ -1169,7 +1248,7 @@ async def generate_quiz(body: GenerateQuizBody, request: Request): # 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( + generated = await _quiz_via_agent( user_id=body.user_id, course_id=course_id, concept_node_id=body.concept_node_id, @@ -1180,6 +1259,8 @@ async def generate_quiz(body: GenerateQuizBody, request: Request): request_id=request_id, model_pref=body.model_pref, ) + questions = generated.questions + exam_days_away = generated.exam_days_away except HTTPException: # The 404 for an unknown concept node is raised before the agent call; # never swallow a known HTTP state. @@ -1224,13 +1305,21 @@ async def generate_quiz(body: GenerateQuizBody, request: Request): ) from e quiz_id = str(uuid.uuid4()) - table("quiz_attempts").insert({ + attempt_row = { "id": quiz_id, "user_id": body.user_id, "concept_node_id": body.concept_node_id, "difficulty": body.difficulty, "questions_json": encrypt_json(questions), - }) + } + # H3/#555: recorded so the question "do deadline-aware quizzes perform + # differently?" is answerable later. Omitted when unknown rather than + # written as an explicit null, mirroring apply_graph_update's rule: an + # environment that took this code before the migration keeps generating + # instead of 400ing on a column PostgREST's schema cache doesn't have. + if exam_days_away is not None: + attempt_row["exam_days_away"] = exam_days_away + _insert_attempt(attempt_row) # #117: quiz.started once the attempt row exists. num_questions is the # actual generated count (the agent may return fewer than requested). events_service.log_event( diff --git a/backend/routes/study_guide.py b/backend/routes/study_guide.py index 4a227de4..6ef307ed 100644 --- a/backend/routes/study_guide.py +++ b/backend/routes/study_guide.py @@ -25,6 +25,7 @@ ) from services.graph_service import get_courses as graph_get_courses from services.auth_guard import require_self +from services.exam_proximity import is_exam from services.encryption import ( decrypt_if_present, decrypt_json, @@ -303,15 +304,11 @@ def get_exams( order="due_date.asc", ) or [] - exam_keywords = ["exam", "midterm", "final", "quiz"] - exams = [] - for a in all_assignments: - atype = (a.get("assignment_type") or "").lower() - title = (a.get("title") or "").lower() - if atype == "exam" or any(kw in title for kw in exam_keywords): - exams.append(a) - - return {"exams": exams} + # The heuristic moved to services/exam_proximity.py (#555), which needed + # the same question answered on the quiz path. One definition, two + # callers — a second copy would drift, which is the failure #557 spent a + # workstream undoing. + return {"exams": [a for a in all_assignments if is_exam(a)]} @router.get("/{user_id}/guide") diff --git a/backend/scripts/benchmark_quiz.py b/backend/scripts/benchmark_quiz.py index 61c2bf50..ddd9d4c0 100644 --- a/backend/scripts/benchmark_quiz.py +++ b/backend/scripts/benchmark_quiz.py @@ -147,7 +147,10 @@ async def _generate_quiz_for_async(concept_name: str) -> list[dict]: so this exercises the real production grounding path rather than generating an ungrounded quiz. """ - return await _quiz_via_agent( + # `.questions`: _quiz_via_agent returns a GeneratedQuiz (#555) so the + # exam-proximity value it resolved can reach the attempt row. This bench + # only wants the questions. + generated = await _quiz_via_agent( user_id="quizfix-user-0001", course_id=FIXTURE_COURSE_ID, concept_node_id="quizfix-node-0001", @@ -157,6 +160,7 @@ async def _generate_quiz_for_async(concept_name: str) -> list[dict]: use_shared_context=False, request_id="quizfix-bench", ) + return generated.questions def generate_quiz_for(concept_name: str) -> list[dict]: diff --git a/backend/services/exam_proximity.py b/backend/services/exam_proximity.py new file mode 100644 index 00000000..53e545b3 --- /dev/null +++ b/backend/services/exam_proximity.py @@ -0,0 +1,181 @@ +"""How close is the student's next exam in this course? (#555, H3) + +`assignments.due_date` is plaintext and indexed, and nothing anywhere computed +"exam in N days" — so a quiz taken the night before a midterm was generated +exactly like one taken in week two. This module answers that one question. + +**Dates only.** No grade VALUES enter any prompt from here. The audit flags +that the current ToS and privacy policy don't clearly cover feeding grades to +a model, and this feature does not need them: proximity is a property of the +calendar, not of performance. `points_earned`/`points_possible` are encrypted +anyway (#521) and are neither read nor decrypted here. + +The exam heuristic is EXTRACTED from `routes/study_guide.py` rather than +re-implemented — `is_exam` is now the single definition and the study-guide +picker calls it. A second copy would drift, which is exactly the failure #557 +spent a workstream undoing one issue earlier. +""" + +from __future__ import annotations + +import logging +import re +from datetime import date, datetime, timezone +from typing import Any + +from db.connection import table +from services.academics import user_enrollment_ids, user_offering_ids_for_course + +logger = logging.getLogger(__name__) + +#: Titles that mean "exam" even when `assignment_type` says otherwise — +#: instructors type these into the title far more reliably than they set the +#: type field. +_EXAM_KEYWORDS = ("exam", "midterm", "final", "quiz") + +#: The SAME words, but anchored, for the decision path. See `is_exam_strict`. +_STRICT_EXAM_PATTERN = re.compile( + r"\b(exam|midterm|final(?!\s+draft)|finals)\b", re.IGNORECASE +) + +#: Only say something about an exam that is actually near. Beyond this, the +#: sentence carries no proximity signal — see `days_until_next_exam`. +PROMPT_HORIZON_DAYS = 14 + + +def is_exam(assignment: dict[str, Any]) -> bool: + """Whether one assignment row reads as an exam, LOOSELY. + + THE definition for the study-guide picker, which is a user-visible list + where a false positive costs the student one extra row to look at. + `routes/study_guide.py` calls this; do not re-derive it. + """ + atype = (assignment.get("assignment_type") or "").lower() + title = (assignment.get("title") or "").lower() + return atype == "exam" or any(kw in title for kw in _EXAM_KEYWORDS) + + +def is_exam_strict(assignment: dict[str, Any]) -> bool: + """Whether one row is an exam, for a DECISION rather than a list. + + Deliberately tighter than `is_exam`, because the cost of a false positive + is different here. The picker shows an extra row; this drives a prompt and + writes `quiz_attempts.exam_days_away`, which exists to answer "do + deadline-aware quizzes perform differently?". Loose matching poisons that + question at the source: a course with weekly "Quiz 3"/"Quiz 4" rows has an + "exam" within a week all semester, so the treatment group silently becomes + "any course with weekly quizzes". + + Two changes from the loose form: + * "quiz" is not a keyword. A graded weekly quiz is not the deadline this + feature is about, and `assignment_type == "exam"` still catches one + that genuinely is. + * word-anchored, so "Final draft - essay 2" is no longer a final. + """ + if (assignment.get("assignment_type") or "").lower() == "exam": + return True + return bool(_STRICT_EXAM_PATTERN.search(assignment.get("title") or "")) + + +def _today() -> date: + """Seam so tests can pin "now" without freezing the process clock. + + UTC, matching every other date boundary in the codebase + (`routes/study_guide.py`, `routes/calendar.py`). A naive local date would + put this module a day out from the study-guide filter on any deployment + whose process TZ is not UTC — the same exam "today" in one surface and + "tomorrow" in the other, and `exam_days_away` off by one for the very + analytics the column exists to enable. + """ + return datetime.now(timezone.utc).date() + + +def _enrollment_ids(offering_ids: list[str], user_id: str) -> list[str]: + """Enrollment resolution via `services/academics.py`, which CLAUDE.md + names as its single home — and which has already read this user's + enrollments inside `user_offering_ids_for_course`. Intersecting in memory + costs nothing and keeps the resolution in one place.""" + wanted = set(offering_ids) + return [ + r["id"] + for r in user_enrollment_ids(user_id) + if r.get("id") and r.get("offering_id") in wanted + ] + + +def days_until_next_exam(user_id: str, course_id: str | None) -> int | None: + """Whole days until this student's soonest UPCOMING exam in the course. + + `0` means "today" and is the most actionable value this produces — it is + deliberately distinct from `None`, which means "no upcoming exam, or we + could not tell". Collapsing the two would drop the exact case the feature + exists for. + + Never raises: this runs inline on the quiz generation path, and one + optional prompt line is not worth failing a generation over. + """ + if not course_id: + return None + try: + return _resolve(user_id, course_id) + except Exception: + logger.warning( + "exam proximity lookup failed; generating without it", exc_info=True + ) + return None + + +def _resolve(user_id: str, course_id: str) -> int | None: + offering_ids = user_offering_ids_for_course(user_id, course_id) + if not offering_ids: + return None + # `assignments` is enrollment-keyed — the gradebook table carries no + # user_id/course_id — so course -> the user's offerings -> their + # enrollments, the same path routes/study_guide.py and routes/calendar.py + # take. + enrollment_ids = _enrollment_ids(offering_ids, user_id) + if not enrollment_ids: + return None + + rows = table("assignments").select( + # Titles and types only. Nothing here selects a points column. + "title,assignment_type,due_date", + filters={ + "enrollment_id": f"in.({','.join(enrollment_ids)})", + "due_date": "not.is.null", + }, + order="due_date.asc", + ) or [] + + today = _today() + # `min` rather than "take the first row": the query asks for due_date.asc, + # but relying on that makes the answer wrong-and-silent if the ordering is + # ever dropped from the query or degraded by the transport. The soonest + # exam is computable here for nothing, so compute it. + upcoming = [ + (due - today).days + for row in rows + if is_exam_strict(row) + and (due := _parse_date(row.get("due_date"))) is not None + and due >= today + ] + return min(upcoming) if upcoming else None + + +def _parse_date(raw: Any) -> date | None: + """`due_date` is DATE in 0021 but TEXT in the 0001 baseline, so rows in the + wild can be either. An unparseable one is skipped, never fatal.""" + # datetime BEFORE date: datetime subclasses date, so the obvious order + # returns a datetime unconverted and the later `due - today` raises + # TypeError into the outer catch — degrading the whole lookup to None for + # that student, silently. + if isinstance(raw, datetime): + return raw.date() + if isinstance(raw, date): + return raw + if not isinstance(raw, str) or not raw.strip(): + return None + try: + return date.fromisoformat(raw.strip()[:10]) + except ValueError: + return None diff --git a/backend/tests/test_exam_proximity.py b/backend/tests/test_exam_proximity.py new file mode 100644 index 00000000..933044c6 --- /dev/null +++ b/backend/tests/test_exam_proximity.py @@ -0,0 +1,288 @@ +"""#555 (Workstream H3, epic #537): how close is the next exam? + +`assignments.due_date` is plaintext and indexed, and nothing anywhere computed +"exam in N days" — so a quiz taken the night before a midterm was generated +exactly like one taken in week two. + +DATES ONLY. No grade values enter any prompt: the audit flags that the current +ToS and privacy policy don't clearly cover that, and this feature does not need +it. `points_earned`/`points_possible` are encrypted anyway; nothing here reads +or decrypts them. +""" +from datetime import date, timedelta +from unittest.mock import patch + +from services.exam_proximity import days_until_next_exam, is_exam + + +def _due(days_from_today: int) -> str: + return (date(2026, 8, 22) + timedelta(days=days_from_today)).isoformat() + + +class TestIsExam: + """The heuristic is EXTRACTED from routes/study_guide.py, not re-written — + a second copy would drift, which is #557's whole lesson one workstream + earlier.""" + + def test_assignment_type_exam_counts(self): + assert is_exam({"assignment_type": "Exam", "title": "Week 4 review"}) + + def test_title_keywords_count(self): + for title in ["Midterm 1", "Final Exam", "Pop QUIZ", "unit exam"]: + assert is_exam({"assignment_type": "homework", "title": title}), title + + def test_ordinary_work_does_not(self): + assert not is_exam({"assignment_type": "homework", "title": "Problem set 3"}) + + def test_missing_fields_do_not_raise(self): + assert not is_exam({}) + assert not is_exam({"assignment_type": None, "title": None}) + + +class TestDaysUntilNextExam: + def _run(self, rows, offerings=("off-1",), today=date(2026, 8, 22)): + with ( + patch("services.exam_proximity.user_offering_ids_for_course", + return_value=list(offerings)), + patch("services.exam_proximity._enrollment_ids", return_value=["enr-1"]), + patch("services.exam_proximity.table") as t, + patch("services.exam_proximity._today", return_value=today), + ): + t.return_value.select.return_value = rows + return days_until_next_exam("u1", "course-1") + + def test_returns_days_to_the_soonest_future_exam(self): + rows = [ + {"title": "Final Exam", "assignment_type": "exam", "due_date": _due(30)}, + {"title": "Midterm", "assignment_type": "exam", "due_date": _due(3)}, + ] + assert self._run(rows) == 3 + + def test_ignores_non_exams(self): + rows = [ + {"title": "Problem set", "assignment_type": "homework", "due_date": _due(1)}, + {"title": "Midterm", "assignment_type": "exam", "due_date": _due(9)}, + ] + assert self._run(rows) == 9 + + def test_a_past_exam_is_not_upcoming(self): + rows = [{"title": "Midterm", "assignment_type": "exam", "due_date": _due(-2)}] + assert self._run(rows) is None + + def test_an_exam_today_is_zero_not_none(self): + """Zero is the most actionable value this feature produces; returning + None for it would silently drop the exact case it exists for.""" + rows = [{"title": "Final", "assignment_type": "exam", "due_date": _due(0)}] + assert self._run(rows) == 0 + + def test_no_exams_is_none(self): + assert self._run([]) is None + + def test_unparseable_due_dates_are_skipped_not_fatal(self): + rows = [ + {"title": "Midterm", "assignment_type": "exam", "due_date": "not-a-date"}, + {"title": "Final", "assignment_type": "exam", "due_date": None}, + {"title": "Quiz 2", "assignment_type": "exam", "due_date": _due(5)}, + ] + assert self._run(rows) == 5 + + def test_no_enrollments_short_circuits_without_reading_assignments(self): + with ( + patch("services.exam_proximity.user_offering_ids_for_course", return_value=[]), + patch("services.exam_proximity.table") as t, + ): + assert days_until_next_exam("u1", "course-1") is None + t.assert_not_called() + + def test_never_raises(self): + """It runs on the quiz generation request path. One optional prompt + line is not worth failing a generation over.""" + with patch("services.exam_proximity.user_offering_ids_for_course", + side_effect=RuntimeError("db down")): + assert days_until_next_exam("u1", "course-1") is None + + def test_no_course_is_none(self): + assert days_until_next_exam("u1", None) is None + + +# ── wiring into generation (#555) ─────────────────────────────────────────── + + +def _agent_run(): + """A quiz_agent.run stand-in returning one valid question.""" + from types import SimpleNamespace + from unittest.mock import AsyncMock + + from agents.quiz import Quiz, QuizQuestion + + return AsyncMock(return_value=SimpleNamespace(output=Quiz(questions=[ + QuizQuestion(question="Q?", type="multiple_choice", difficulty="easy", + options=["a", "b", "c", "d"], correct_answer="a", + explanation="x", concept="X")]))) + + +class TestExamProximityReachesGeneration: + """The service being right is half of it; the other half is that its + answer actually reaches the model AND the attempt row. Both are one + keyword away from being silently dropped.""" + + def _generate(self, days, agent_run): + from unittest.mock import MagicMock + from fastapi.testclient import TestClient + + from main import app + + inserted: list = [] + + def factory(name): + m = MagicMock() + m.select.return_value = ( + [{"id": "node1", "user_id": "user_andres", "course_id": "c1", + "concept_name": "Recursion", "mastery_score": 0.5}] + if name == "graph_nodes" else [] + ) + m.insert.side_effect = lambda row, *a, **k: inserted.append((name, row)) or [] + return m + + with ( + patch("routes.quiz.table", side_effect=factory), + patch("routes.quiz.days_until_next_exam", return_value=days), + patch("routes.quiz.recent_question_identities", return_value=[]), + patch("routes.quiz.quiz_agent.run", new=agent_run), + ): + r = TestClient(app).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, r.text + attempt = next(row for name, row in inserted if name == "quiz_attempts") + return agent_run.call_args[0][0], attempt + + def test_a_near_exam_reaches_the_prompt_and_the_attempt_row(self): + msg, attempt = self._generate(3, _agent_run()) + + assert "next exam in this course is in 3 days" in msg + assert attempt["exam_days_away"] == 3 + + def test_an_exam_today_says_TODAY(self): + msg, attempt = self._generate(0, _agent_run()) + + assert "next exam in this course is TODAY" in msg + # 0 must survive to the row: `if exam_days_away:` would drop exam day, + # the single most actionable value the feature produces. + assert attempt["exam_days_away"] == 0 + + def test_unknown_proximity_adds_nothing_and_omits_the_column(self): + """'next exam: unknown' is prompt tokens spent to say nothing, and + omitting the key (rather than sending null) keeps generation working + on an environment that took this code before the migration.""" + msg, attempt = self._generate(None, _agent_run()) + + assert "next exam" not in msg + assert "exam_days_away" not in attempt + + +# ── review findings: strictness, horizon, pre-migration insert ────────────── + + +class TestStrictnessOnTheDecisionPath: + """`is_exam` drives a user-visible LIST, where a false positive costs one + extra row. `is_exam_strict` drives a PROMPT and a stored analytics column, + where a false positive poisons the question the column exists to answer.""" + + def test_a_weekly_quiz_is_not_an_exam_deadline(self): + from services.exam_proximity import is_exam, is_exam_strict + + row = {"assignment_type": "homework", "title": "Quiz 4"} + assert is_exam(row), "the picker still lists it" + assert not is_exam_strict(row), ( + "a course with weekly quizzes would otherwise have an 'exam' " + "within a week all semester, making the treatment group for " + "'do deadline-aware quizzes perform differently' meaningless" + ) + + def test_a_final_draft_is_not_a_final(self): + from services.exam_proximity import is_exam_strict + + assert not is_exam_strict( + {"assignment_type": "homework", "title": "Final draft - essay 2"} + ) + + def test_a_real_exam_still_counts_both_ways(self): + from services.exam_proximity import is_exam, is_exam_strict + + for row in ( + {"assignment_type": "exam", "title": "Week 4"}, + {"assignment_type": "homework", "title": "Midterm 2"}, + {"assignment_type": "homework", "title": "Final Exam"}, + ): + assert is_exam(row) and is_exam_strict(row), row + + +class TestPromptHorizon: + """A final dated 87 days out would otherwise put 'there is an exam coming' + on every quiz for the whole semester — no proximity signal at all, and it + steers week-two practice toward exam-style questions.""" + + def test_a_distant_exam_is_stored_but_not_prompted(self): + from services.exam_proximity import PROMPT_HORIZON_DAYS + + far = PROMPT_HORIZON_DAYS + 30 + msg, attempt = TestExamProximityReachesGeneration()._generate(far, _agent_run()) + + assert "next exam" not in msg + # The COLUMN is not clamped: the analytics want the real distance. + assert attempt["exam_days_away"] == far + + def test_an_exam_on_the_horizon_boundary_is_still_prompted(self): + from services.exam_proximity import PROMPT_HORIZON_DAYS + + msg, _ = TestExamProximityReachesGeneration()._generate( + PROMPT_HORIZON_DAYS, _agent_run() + ) + assert f"in {PROMPT_HORIZON_DAYS} days" in msg + + +def test_the_attempt_insert_survives_a_schema_without_the_column(): + """Pre-migration (or before PostgREST reloads its schema cache) the column + is unknown and the insert 400s — and it strikes exactly the students who + HAVE an upcoming exam, so it looks like a random partial outage. It would + land after the agent already ran and was billed: quiz lost, no attempt + row, no failure event, no rate-limit refund. Retry without the key.""" + from unittest.mock import MagicMock, patch as _patch + + from routes.quiz import _insert_attempt + + calls: list = [] + tbl = MagicMock() + + def _insert(row): + calls.append(row) + if "exam_days_away" in row: + raise RuntimeError("PGRST204: column not found") + return [] + + tbl.insert.side_effect = _insert + with _patch("routes.quiz.table", return_value=tbl): + _insert_attempt({"id": "q1", "user_id": "u1", "exam_days_away": 3}) + + assert len(calls) == 2 + assert "exam_days_away" not in calls[1] + assert calls[1]["id"] == "q1" + + +def test_an_insert_failure_unrelated_to_the_column_still_raises(): + """The retry must not become a blanket swallow — a genuine write failure + has to keep surfacing.""" + from unittest.mock import MagicMock, patch as _patch + + import pytest as _pytest + + from routes.quiz import _insert_attempt + + tbl = MagicMock() + tbl.insert.side_effect = RuntimeError("connection refused") + with _patch("routes.quiz.table", return_value=tbl): + with _pytest.raises(RuntimeError): + _insert_attempt({"id": "q1", "user_id": "u1"})