From 69807dbd9c4ebd2b668a67efd49921d9a3654b48 Mon Sep 17 00:00:00 2001 From: AndresL230 <190146319+AndresL230@users.noreply.github.com> Date: Sat, 22 Aug 2026 05:11:05 -0400 Subject: [PATCH 1/2] feat(quiz): weight generation toward an approaching exam (#555) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `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 VALUE is read, stored or prompted on this path. Proximity is a property of the calendar, not of performance; the points columns are encrypted (#521) and this code never selects them. 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 to test that. The exam heuristic is EXTRACTED rather than re-implemented, as the issue asks: `services/exam_proximity.py::is_exam` is now the single definition and `routes/study_guide.py` calls it. Writing a second copy is the failure #557 spent a workstream undoing, one issue earlier in this same workstream. Shape of the answer: * `0` means the exam is TODAY and is deliberately distinct from `None` ("no upcoming exam, or we could not tell"). Collapsing them — or writing `if exam_days_away:` — drops the single most actionable value the feature produces. Pinned by test on both the prompt and the stored row. * Resolved ONCE per generation and then both prompted and stored: computing it twice could report one number to the model and a different one to the analytics row if an exam were entered between the reads. * `min()` over the upcoming exams rather than "first row of a due_date.asc query" — the ordering is asked for, but relying on it makes the answer wrong-and-silent if it is ever dropped or degraded, and the minimum costs nothing to compute here. * Never raises: it runs inline on the generation path, and one optional prompt line is not worth failing a generation over. The prompt line states the deadline and lets the model decide what it implies. It deliberately does NOT say "make it harder" — that would contradict the difficulty the student actually chose, and adaptive mode already owns that decision. When proximity is unknown the line is omitted entirely, rather than spending tokens to say "unknown". Migration `20260822090747` adds a nullable `exam_days_away` to `quiz_attempts`, because the issue's point is to be able to ASK LATER whether deadline-aware quizzes perform differently — a value that only reached a prompt leaves nothing to measure. No DEFAULT: `0` would make every legacy row look like exam day. Applied to staging and verified before this code ships (nullable, no default, 2 existing rows NULL); the write omits the key when unknown, so an environment that takes the code first keeps generating. Hermetic 2200 passed / 9 skipped, ruff clean. Co-Authored-By: Claude Opus 5 --- ...822090747_quiz_attempts_exam_days_away.sql | 29 +++ backend/routes/quiz.py | 37 +++- backend/routes/study_guide.py | 15 +- backend/services/exam_proximity.py | 132 ++++++++++++ backend/tests/test_exam_proximity.py | 203 ++++++++++++++++++ 5 files changed, 405 insertions(+), 11 deletions(-) create mode 100644 backend/db/migrations/20260822090747_quiz_attempts_exam_days_away.sql create mode 100644 backend/services/exam_proximity.py create mode 100644 backend/tests/test_exam_proximity.py 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..ca29b80d 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 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 @@ -806,6 +807,7 @@ async def _quiz_via_agent( use_shared_context: bool, request_id: str, model_pref: str | None = None, + exam_days_away: int | None = None, ) -> list[dict]: """Run quiz_agent and return questions in the legacy wire shape. @@ -861,6 +863,21 @@ async def _quiz_via_agent( " 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. + if exam_days_away is not None: + 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." + ) # Course-material grounding does blocking network I/O (a Gemini # embedding call, bounded at 60s) plus sync Supabase reads. Run it in a @@ -1164,6 +1181,13 @@ async def generate_quiz(body: GenerateQuizBody, request: Request): # quiz.started, which shares this request_id with the llm_usage row. prompt_dimensions.start_capture() + # H3/#555: resolved ONCE here, then both prompted and stored — computing + # it twice could report one number to the model and a different one to the + # analytics row if an exam were entered between the two reads. + exam_days_away = await asyncio.to_thread( + days_until_next_exam, body.user_id, course_id + ) + try: # Each agent run inside is individually bounded by # QUIZ_GENERATION_TIMEOUT_SEC (see _run) — cancelling the whole @@ -1172,6 +1196,7 @@ async def generate_quiz(body: GenerateQuizBody, request: Request): questions = await _quiz_via_agent( user_id=body.user_id, course_id=course_id, + exam_days_away=exam_days_away, concept_node_id=body.concept_node_id, concept_name=concept_name, num_questions=body.num_questions, @@ -1224,13 +1249,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 + table("quiz_attempts").insert(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/services/exam_proximity.py b/backend/services/exam_proximity.py new file mode 100644 index 00000000..8bdcdf6e --- /dev/null +++ b/backend/services/exam_proximity.py @@ -0,0 +1,132 @@ +"""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 +from datetime import date, datetime +from typing import Any + +from db.connection import table +from services.academics import 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. "quiz" is included because a graded in-class quiz is the same +#: kind of deadline pressure, which is the signal this module exists to catch. +_EXAM_KEYWORDS = ("exam", "midterm", "final", "quiz") + + +def is_exam(assignment: dict[str, Any]) -> bool: + """Whether one assignment row reads as an exam. + + THE definition. `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 _today() -> date: + """Seam so tests can pin "now" without freezing the process clock.""" + return datetime.now().date() + + +def _enrollment_ids(offering_ids: list[str], user_id: str) -> list[str]: + rows = table("enrollments").select( + "id", + filters={ + "user_id": f"eq.{user_id}", + "offering_id": f"in.({','.join(offering_ids)})", + }, + ) or [] + return [r["id"] for r in rows if r.get("id")] + + +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(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.""" + 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..6828e7ab --- /dev/null +++ b/backend/tests/test_exam_proximity.py @@ -0,0 +1,203 @@ +"""#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) ─────────────────────────────────────────── + +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 types import SimpleNamespace + from unittest.mock import AsyncMock, MagicMock + from fastapi.testclient import TestClient + + from agents.quiz import Quiz, QuizQuestion + from main import app + + quiz = Quiz(questions=[QuizQuestion( + question="Q?", type="multiple_choice", difficulty="easy", + options=["a", "b", "c", "d"], correct_answer="a", + explanation="x", concept="X", + )]) + 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): + from types import SimpleNamespace + from unittest.mock import AsyncMock + + from agents.quiz import Quiz, QuizQuestion + + run = 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")]))) + msg, attempt = self._generate(3, 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): + from types import SimpleNamespace + from unittest.mock import AsyncMock + + from agents.quiz import Quiz, QuizQuestion + + run = 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")]))) + msg, attempt = self._generate(0, 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.""" + from types import SimpleNamespace + from unittest.mock import AsyncMock + + from agents.quiz import Quiz, QuizQuestion + + run = 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")]))) + msg, attempt = self._generate(None, run) + + assert "next exam" not in msg + assert "exam_days_away" not in attempt From 47d5555da66383f408b34dddee5b9ff5d749e2d1 Mon Sep 17 00:00:00 2001 From: AndresL230 <190146319+AndresL230@users.noreply.github.com> Date: Sat, 22 Aug 2026 05:23:44 -0400 Subject: [PATCH 2/2] =?UTF-8?q?fix(quiz):=20#555=20review=20=E2=80=94=20ei?= =?UTF-8?q?ght=20findings,=20and=20my=20ruff=20claim=20was=20stale?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CI was red: I ran `ruff check` BEFORE adding the wiring tests and reported it clean in the previous commit message. Three violations in the new test file (two dead imports and an unused `Quiz`); the repeated agent-run construction is hoisted into a helper rather than rebuilt per test. **The heuristic was too loose for a decision.** `is_exam` was written for a user-visible picker, where a false positive costs one extra row. Reused verbatim it drove a prompt AND `quiz_attempts.exam_days_away` — so a course with weekly "Quiz 3"/"Quiz 4" rows has an "exam" within a week all semester, and "Final draft - essay 2" is a final. That poisons the exact question the column exists to answer, because the treatment group becomes "any course with weekly quizzes". Added `is_exam_strict` for the decision path: no "quiz" keyword, word-anchored matching, `assignment_type == "exam"` still wins. The picker keeps the loose form, which is right for a list. **No proximity horizon.** A final dated 87 days out put "there is an exam coming, weight toward what an exam tests" on EVERY quiz for the semester — no proximity signal, and it steers week-two practice toward exam questions, which is what the code comment claims to be avoiding. The prompt line is now bounded by `PROMPT_HORIZON_DAYS`; the stored column is deliberately NOT clamped, because the analytics want the real distance. **Four serial round-trips, fully additive, outside the safety net.** The lookup ran before the try block and before the existing gather, so its latency added to every generation and a failure there consumed a rate-limit slot with no refund. It is now the third leg of the gather that already runs grounding and the recently-asked read concurrently — it is best-effort context exactly like those two. `_quiz_via_agent` returns a `GeneratedQuiz` so the value it used for the prompt is the one stored on the attempt, rather than being resolved twice. **The attempt insert could 500 a quiz that already ran.** Omit-when-None does not make a pre-migration environment safe: it protects the no-exam case and breaks precisely the students the feature is for, so it presents as a random partial outage. And it lands after the agent has been billed — quiz lost, no attempt row, no `quiz.generation_failed`, no refund. `_insert_attempt` retries once without the key and re-raises anything else, so a genuine write failure still surfaces. Also: UTC (`_today` was naive local time, a day out from study_guide/calendar on any non-UTC deployment, and off-by-one for the analytics); `datetime` checked before `date` in `_parse_date` (datetime subclasses date, so the obvious order returned it unconverted and the later subtraction raised into the outer catch, silently degrading the lookup); enrollment resolution via `services/academics.py::user_enrollment_ids`, which CLAUDE.md names as its single home and which had already read those rows a line earlier. Hermetic 2207 passed / 9 skipped, ruff clean — verified after the tests, this time. Co-Authored-By: Claude Opus 5 --- backend/routes/quiz.py | 152 +++++++++++++++++-------- backend/scripts/benchmark_quiz.py | 6 +- backend/services/exam_proximity.py | 83 +++++++++++--- backend/tests/test_exam_proximity.py | 159 ++++++++++++++++++++------- 4 files changed, 297 insertions(+), 103 deletions(-) diff --git a/backend/routes/quiz.py b/backend/routes/quiz.py index ca29b80d..74a443e0 100644 --- a/backend/routes/quiz.py +++ b/backend/routes/quiz.py @@ -42,7 +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 days_until_next_exam +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 @@ -796,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, @@ -807,8 +851,7 @@ async def _quiz_via_agent( use_shared_context: bool, request_id: str, model_pref: str | None = None, - exam_days_away: int | 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) @@ -851,33 +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." - ) - # 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. - if exam_days_away is not None: - 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." - ) # Course-material grounding does blocking network I/O (a Gemini # embedding call, bounded at 60s) plus sync Supabase reads. Run it in a @@ -885,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", @@ -917,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, @@ -1095,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(): @@ -1181,22 +1243,14 @@ async def generate_quiz(body: GenerateQuizBody, request: Request): # quiz.started, which shares this request_id with the llm_usage row. prompt_dimensions.start_capture() - # H3/#555: resolved ONCE here, then both prompted and stored — computing - # it twice could report one number to the model and a different one to the - # analytics row if an exam were entered between the two reads. - exam_days_away = await asyncio.to_thread( - days_until_next_exam, body.user_id, course_id - ) - try: # Each agent run inside is individually bounded by # QUIZ_GENERATION_TIMEOUT_SEC (see _run) — cancelling the whole # coroutine here would discard a partial quiz the top-up handler # is designed to serve. - questions = await _quiz_via_agent( + generated = await _quiz_via_agent( user_id=body.user_id, course_id=course_id, - exam_days_away=exam_days_away, concept_node_id=body.concept_node_id, concept_name=concept_name, num_questions=body.num_questions, @@ -1205,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. @@ -1263,7 +1319,7 @@ async def generate_quiz(body: GenerateQuizBody, request: Request): # 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 - table("quiz_attempts").insert(attempt_row) + _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/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 index 8bdcdf6e..53e545b3 100644 --- a/backend/services/exam_proximity.py +++ b/backend/services/exam_proximity.py @@ -19,45 +19,88 @@ from __future__ import annotations import logging -from datetime import date, datetime +import re +from datetime import date, datetime, timezone from typing import Any from db.connection import table -from services.academics import user_offering_ids_for_course +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. "quiz" is included because a graded in-class quiz is the same -#: kind of deadline pressure, which is the signal this module exists to catch. +#: 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. + """Whether one assignment row reads as an exam, LOOSELY. - THE definition. `routes/study_guide.py` calls this; do not re-derive it. + 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.""" - return datetime.now().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]: - rows = table("enrollments").select( - "id", - filters={ - "user_id": f"eq.{user_id}", - "offering_id": f"in.({','.join(offering_ids)})", - }, - ) or [] - return [r["id"] for r in rows if r.get("id")] + """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: @@ -112,7 +155,7 @@ def _resolve(user_id: str, course_id: str) -> int | None: upcoming = [ (due - today).days for row in rows - if is_exam(row) + if is_exam_strict(row) and (due := _parse_date(row.get("due_date"))) is not None and due >= today ] @@ -122,6 +165,12 @@ def _resolve(user_id: str, course_id: str) -> int | 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(): diff --git a/backend/tests/test_exam_proximity.py b/backend/tests/test_exam_proximity.py index 6828e7ab..933044c6 100644 --- a/backend/tests/test_exam_proximity.py +++ b/backend/tests/test_exam_proximity.py @@ -107,24 +107,31 @@ def test_no_course_is_none(self): # ── 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 types import SimpleNamespace - from unittest.mock import AsyncMock, MagicMock + from unittest.mock import MagicMock from fastapi.testclient import TestClient - from agents.quiz import Quiz, QuizQuestion from main import app - quiz = Quiz(questions=[QuizQuestion( - question="Q?", type="multiple_choice", difficulty="easy", - options=["a", "b", "c", "d"], correct_answer="a", - explanation="x", concept="X", - )]) inserted: list = [] def factory(name): @@ -153,31 +160,13 @@ def factory(name): return agent_run.call_args[0][0], attempt def test_a_near_exam_reaches_the_prompt_and_the_attempt_row(self): - from types import SimpleNamespace - from unittest.mock import AsyncMock - - from agents.quiz import Quiz, QuizQuestion - - run = 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")]))) - msg, attempt = self._generate(3, run) + 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): - from types import SimpleNamespace - from unittest.mock import AsyncMock - - from agents.quiz import Quiz, QuizQuestion - - run = 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")]))) - msg, attempt = self._generate(0, run) + 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, @@ -188,16 +177,112 @@ 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.""" - from types import SimpleNamespace - from unittest.mock import AsyncMock + 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"} + ) - from agents.quiz import Quiz, QuizQuestion + def test_a_real_exam_still_counts_both_ways(self): + from services.exam_proximity import is_exam, is_exam_strict - run = 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")]))) - msg, attempt = self._generate(None, run) + 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 - assert "exam_days_away" not in attempt + # 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"})