Skip to content

Commit 4bee59e

Browse files
fix(quiz): close 2 remaining review issues from PR #71
1. All-drift cascade test (TestQuizAgentFallback) New `test_falls_back_to_legacy_when_all_questions_drift` pins the path the 3 contract tests don't cover directly: agent returns a schema-valid Quiz where every question's correct_answer doesn't appear in its options → _quiz_via_agent's wire-format filter drops all of them → raises RuntimeError → bare-Exception catch in generate_quiz routes to _legacy_generate_quiz. Asserts the legacy gemini path actually runs and the legacy fallback question is what reaches the client. 2. Drift warning no longer leaks student content to local logs _agent_question_to_wire's drift warning was using %r to dump the raw correct_answer, options, and concept text. Logfire's egress scrubber (PR #67) handled remote ingestion, but Railway's local stdout still saw the unredacted strings. Now we log: n_options=4, canonical_len=18, fp=<sha256[:12]> The fingerprint is stable across recurrences of the same drift, so we still get correlation; the actual content stays out of stdout. Hashlib import hoisted to module scope. Pre-existing transient: tests/test_ocr_pipeline.py::test_gemini_parse that flickered red in the previous review run cleared on re-run (skipped in isolation, passing in full suite). Confirmed transient live-Gemini hiccup, not caused by this branch. Tests - tests/test_quiz_routes.py: 23/23 (the previous "24" was a miscount; net +1 from the new cascade test). - Full backend suite: 443 passed, 3 pre-existing live-Supabase failures unchanged. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 0bcc664 commit 4bee59e

2 files changed

Lines changed: 67 additions & 5 deletions

File tree

backend/routes/quiz.py

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
1-
import logging
2-
import uuid
1+
import hashlib
32
import json
3+
import logging
44
import os
5+
import uuid
56
from datetime import datetime
67

78
from fastapi import APIRouter, BackgroundTasks, HTTPException, Request
@@ -79,10 +80,21 @@ def _agent_question_to_wire(q: QuizQuestion, qid: int) -> dict | None:
7980
# Generation drift: agent's correct_answer doesn't match any
8081
# option verbatim. Surface in logs (Logfire span carries the
8182
# question_id correlation) and drop. Caller filters None.
83+
#
84+
# Don't log the raw text — student-content concept names and
85+
# quiz answers don't belong in stdout/Railway logs. The
86+
# fingerprint is stable enough to correlate with the same
87+
# generation drift if it recurs (sha256 of "<canonical>|<options>");
88+
# the full content is still in Logfire spans (where the scrubber
89+
# from PR #67 controls egress).
90+
canonical_only = q.correct_answer.strip()
91+
fp = hashlib.sha256(
92+
f"{canonical_only}|{'|'.join(q.options)}".encode("utf-8")
93+
).hexdigest()[:12]
8294
logger.warning(
83-
"quiz: dropping question %d — correct_answer %r not found "
84-
"in options %r (concept=%s)",
85-
qid, q.correct_answer, q.options, q.concept,
95+
"quiz: dropping question id=%d — correct_answer not found in "
96+
"options (n_options=%d, canonical_len=%d, fp=%s)",
97+
qid, len(q.options), len(canonical_only), fp,
8698
)
8799
return None
88100
return {

backend/tests/test_quiz_routes.py

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -492,6 +492,56 @@ def test_falls_back_to_legacy_on_unexpected_exception(self):
492492
gemini_mock.assert_called_once()
493493
assert r.json()["questions"][0]["question"] == "Legacy fallback question?"
494494

495+
def test_falls_back_to_legacy_when_all_questions_drift(self):
496+
"""Cascade: agent succeeds but every question fails wire-format
497+
validation → _quiz_via_agent raises RuntimeError →
498+
bare-Exception catch in generate_quiz routes to legacy.
499+
500+
This pins the path the 3 contract tests don't directly exercise
501+
(they test _agent_question_to_wire in isolation; this exercises
502+
the full route under the all-drift condition).
503+
"""
504+
# Build a Quiz where every question's correct_answer doesn't
505+
# appear in its options — schema-valid, but the wire-format
506+
# check drops every one.
507+
drift_quiz = Quiz(questions=[
508+
QuizQuestion(
509+
question=f"Q{i}?",
510+
type="multiple_choice",
511+
difficulty="easy",
512+
options=["a", "b", "c", "d"],
513+
correct_answer="MISMATCH", # not in options
514+
explanation="x",
515+
concept="X",
516+
)
517+
for i in range(3)
518+
])
519+
520+
get_graph_p, get_ctx_p, gemini_p = self._patch_legacy_dependencies()
521+
with (
522+
patch("routes.quiz.table", side_effect=_generate_table_factory()),
523+
patch(
524+
"routes.quiz.quiz_agent.run",
525+
new=AsyncMock(return_value=SimpleNamespace(output=drift_quiz)),
526+
),
527+
get_graph_p,
528+
get_ctx_p,
529+
gemini_p as gemini_mock,
530+
):
531+
r = client.post("/api/quiz/generate", json={
532+
"user_id": "user_andres",
533+
"concept_node_id": "node1",
534+
"num_questions": 3,
535+
"difficulty": "easy",
536+
"use_shared_context": False,
537+
})
538+
539+
assert r.status_code == 200
540+
# Legacy path fired (every question dropped → RuntimeError →
541+
# caught → _legacy_generate_quiz called).
542+
gemini_mock.assert_called_once()
543+
assert r.json()["questions"][0]["question"] == "Legacy fallback question?"
544+
495545

496546
# ── Wire-format contract: pinned by tests so silent drift can't recur ───────
497547

0 commit comments

Comments
 (0)