From 179e51d143aa79a8d087b4b080fed08b81d9c232 Mon Sep 17 00:00:00 2001 From: Daniel Date: Tue, 8 Sep 2026 19:29:21 +0200 Subject: [PATCH] feat: per-option explanations Questions support an explanation per option, edited in the question dialog and shown in study feedback. Keys must match current options. Migration j3e4f5a6b708. 60 backend and 95 frontend tests pass. --- .../j3e4f5a6b708_option_explanations.py | 19 ++++++ backend/app/models/question.py | 1 + backend/app/routers/questions.py | 10 +++ backend/app/routers/quizzes.py | 6 +- backend/app/schemas/quiz.py | 1 + backend/app/utils/quiz_questions.py | 15 ++++ backend/tests/test_option_explanations.py | 68 +++++++++++++++++++ frontend/src/pages/QuestionBankPage.jsx | 50 +++++++++++--- frontend/src/pages/QuestionBankPage.test.jsx | 19 ++++++ frontend/src/pages/QuizPage.jsx | 3 + frontend/src/pages/QuizPage.test.jsx | 17 +++++ frontend/src/pages/QuizPlayer.css | 1 + 12 files changed, 199 insertions(+), 11 deletions(-) create mode 100644 backend/alembic/versions/j3e4f5a6b708_option_explanations.py create mode 100644 backend/tests/test_option_explanations.py diff --git a/backend/alembic/versions/j3e4f5a6b708_option_explanations.py b/backend/alembic/versions/j3e4f5a6b708_option_explanations.py new file mode 100644 index 0000000..9dc0bbe --- /dev/null +++ b/backend/alembic/versions/j3e4f5a6b708_option_explanations.py @@ -0,0 +1,19 @@ +"""Per-option explanations. + +Revision ID: j3e4f5a6b708 +Revises: i2d3e4f5a607 +""" +from alembic import op + +revision = "j3e4f5a6b708" +down_revision = "i2d3e4f5a607" +branch_labels = None +depends_on = None + + +def upgrade(): + op.execute("ALTER TABLE questions ADD COLUMN IF NOT EXISTS option_explanations JSON") + + +def downgrade(): + op.execute("ALTER TABLE questions DROP COLUMN IF EXISTS option_explanations") diff --git a/backend/app/models/question.py b/backend/app/models/question.py index b63e639..2b5bfd9 100644 --- a/backend/app/models/question.py +++ b/backend/app/models/question.py @@ -24,6 +24,7 @@ class Question(Base): page_reference = Column(Integer, nullable=True) image_path = Column(String, nullable=True) explanation_image_path = Column(String, nullable=True) + option_explanations = Column(JSON, nullable=True) # {option_text: explanation} user_id = Column(Integer, ForeignKey("users.id", ondelete="SET NULL"), nullable=True) is_shared = Column(Integer, default=1) # 1 = visible in bank, 0 = private (only owner sees it) embedding = deferred(Column(Vector(1024), nullable=True)) # semantic search vector — deferred: not loaded in standard queries diff --git a/backend/app/routers/questions.py b/backend/app/routers/questions.py index 52afc17..b60931d 100644 --- a/backend/app/routers/questions.py +++ b/backend/app/routers/questions.py @@ -17,6 +17,7 @@ from sqlalchemy.orm import Session from app.config import settings from app.database import get_db from app.utils.upload_access import validate_image_attachments, stored_upload_path +from app.utils.quiz_questions import validate_option_explanations from app.models.question import Question from app.models.question_category import QuestionCategory, QuestionCategoryLink from app.models.quiz import Quiz @@ -68,6 +69,7 @@ class QuestionEdit(BaseModel): explanation: str | None = None question_category_id: int | None = None additional_category_ids: list[int] | None = None # Full set of extra (non-primary) categories. + option_explanations: dict | None = None image_path: str | None = None explanation_image_path: str | None = None @@ -87,6 +89,10 @@ def edit_question( if question.user_id != current_user.id and not is_mod: raise HTTPException(status_code=403, detail="Not authorized to edit this question") values = validate_image_attachments(db, current_user, data.model_dump(exclude_unset=True)) + explanations = values.pop("option_explanations", None) + if explanations is not None: + values["option_explanations"] = validate_option_explanations( + values.get("options", question.options), explanations) extra_ids = values.pop("additional_category_ids", None) if extra_ids is not None: extra_ids = list(dict.fromkeys(extra_ids)) @@ -357,6 +363,7 @@ def get_question_bank( "explanation": qu.explanation, "image_path": qu.image_path, "explanation_image_path": qu.explanation_image_path, + "option_explanations": qu.option_explanations, "user_id": qu.user_id, "is_shared": qu.is_shared if qu.is_shared is not None else 1, }) @@ -384,6 +391,7 @@ class ManualQuestionCreate(BaseModel): correct_answer: str explanation: str | None = None question_category_id: int | None = None + option_explanations: dict | None = None image_path: str | None = None explanation_image_path: str | None = None @@ -406,6 +414,7 @@ def create_question_manually( raise HTTPException(status_code=400, detail="Correct answer must be one of the options") images = validate_image_attachments(db, current_user, data.model_dump()) + option_explanations = validate_option_explanations(data.options, data.option_explanations) question = Question( question_text=q_text, question_type=data.question_type, @@ -415,6 +424,7 @@ def create_question_manually( question_category_id=data.question_category_id, image_path=images["image_path"], explanation_image_path=images["explanation_image_path"], + option_explanations=option_explanations, user_id=current_user.id, is_shared=1, ) diff --git a/backend/app/routers/quizzes.py b/backend/app/routers/quizzes.py index 430c25e..9a029dc 100644 --- a/backend/app/routers/quizzes.py +++ b/backend/app/routers/quizzes.py @@ -5,6 +5,7 @@ from sqlalchemy import cast, String, or_, and_, func from sqlalchemy.orm import Session from app.utils.upload_access import validate_image_attachments +from app.utils.quiz_questions import validate_option_explanations from app.database import get_db from app.models.quiz import Quiz from app.models.question import Question as QuestionModel @@ -452,8 +453,11 @@ def update_question( if not question: raise HTTPException(status_code=404, detail="Question not found") - allowed = {"question_text", "options", "correct_answer", "explanation", "question_type", "image_path", "explanation_image_path"} + allowed = {"question_text", "options", "correct_answer", "explanation", "question_type", "image_path", "explanation_image_path", "option_explanations"} data = validate_image_attachments(db, current_user, data) + if "option_explanations" in data: + data["option_explanations"] = validate_option_explanations( + data.get("options", question.options), data["option_explanations"]) for key, value in data.items(): if key in allowed: setattr(question, key, value) diff --git a/backend/app/schemas/quiz.py b/backend/app/schemas/quiz.py index 3d7af0e..70c490c 100644 --- a/backend/app/schemas/quiz.py +++ b/backend/app/schemas/quiz.py @@ -42,6 +42,7 @@ class QuestionWithAnswer(QuestionResponse): explanation: str | None explanation_image_path: str | None = None page_reference: int | None + option_explanations: dict | None = None class QuizResponse(BaseModel): diff --git a/backend/app/utils/quiz_questions.py b/backend/app/utils/quiz_questions.py index f2d01ac..f493084 100644 --- a/backend/app/utils/quiz_questions.py +++ b/backend/app/utils/quiz_questions.py @@ -6,6 +6,21 @@ from app.models.question import Question from app.models.quiz_question_link import QuizQuestionLink +def validate_option_explanations(options, explanations): + """Per-option explanations must be a {option_text: explanation} map within options.""" + if explanations is None: + return None + if not isinstance(explanations, dict): + raise HTTPException(400, "Option explanations must be an object keyed by option text") + allowed = set(options or []) + for key, value in explanations.items(): + if not isinstance(key, str) or key not in allowed: + raise HTTPException(400, "Option explanation keys must match existing options") + if not isinstance(value, str) or len(value) > 2000: + raise HTTPException(400, "Option explanations are strings up to 2000 characters") + return {key: value for key, value in explanations.items() if value.strip()} + + def get_quiz_questions(db: Session, quiz_id: int) -> list[Question]: """Fetch questions for a quiz in position order via junction table.""" links = ( diff --git a/backend/tests/test_option_explanations.py b/backend/tests/test_option_explanations.py new file mode 100644 index 0000000..431b22e --- /dev/null +++ b/backend/tests/test_option_explanations.py @@ -0,0 +1,68 @@ +"""Per-option explanation validation and serialization.""" +import unittest + +import test_quiz_builder as fixtures +from app.models.question import Question +from app.routers import questions, quizzes + + +class OptionExplanationTests(unittest.TestCase): + def setUp(self): + self.bank = fixtures.BuilderTests() + self.bank.setUp() + self.client = self.bank.client + + def tearDown(self): + self.bank.tearDown() + + def create_payload(self, **overrides): + payload = {"question_text": "Explained", "question_type": "mcq", + "options": ["yes", "no"], "correct_answer": "yes", + "option_explanations": {"yes": "Right because", "no": "Wrong because"}} + payload.update(overrides) + return payload + + def test_create_validates_and_serializes_explanations(self): + self.bank.user = self.bank.owner + created = self.client.post('/questions/create', json=self.create_payload()) + self.assertIn(created.status_code, (200, 201), created.text) + question_id = created.json()['id'] + bank = self.client.get('/questions/bank', params={'q': 'Explained'}).json() + row = next(r for r in bank['questions'] if r['id'] == question_id) + self.assertEqual(row['option_explanations'], {"yes": "Right because", "no": "Wrong because"}) + for bad in ( + {"option_explanations": {"nope": "x"}}, + {"option_explanations": "not-a-dict"}, + {"option_explanations": {"yes": "x" * 2001}}, + ): + response = self.client.post('/questions/create', json=self.create_payload(**bad)) + self.assertIn(response.status_code, (400, 422), bad) + + def test_edit_requires_keys_to_match_current_options(self): + self.bank.user = self.bank.owner + response = self.client.patch('/questions/3', json={"option_explanations": {"yes": "Correct path", "no": "Incorrect path"}}) + self.assertEqual(response.status_code, 200, response.text) + self.assertEqual(self.bank.db.get(Question, 3).option_explanations, {"yes": "Correct path", "no": "Incorrect path"}) + # Changing options without updating keys is rejected atomically. + response = self.client.patch('/questions/3', json={"options": ["a", "b"], "correct_answer": "a", + "option_explanations": {"yes": "stale"}}) + self.assertEqual(response.status_code, 400, response.text) + self.assertEqual(self.bank.db.get(Question, 3).options, ["yes", "no"]) + + def test_quiz_reveal_and_editor_route_carry_explanations(self): + self.bank.user = self.bank.mod + self.client.patch('/questions/1', json={"option_explanations": {"yes": "Preferred response", "no": "Distractor"}}) + self.bank.user = self.bank.owner + attempt = self.client.post('/attempts/start?quiz_id=1&mode=study').json()['id'] + detail = self.client.get(f'/quizzes/1?attempt_id={attempt}').json() + self.assertEqual(detail['questions'][0]['option_explanations'], {"yes": "Preferred response", "no": "Distractor"}) + self.bank.user = self.bank.mod + response = self.client.patch('/quizzes/1/questions/1', json={"option_explanations": {"nope": "bad"}}) + self.assertEqual(response.status_code, 400, response.text) + response = self.client.patch('/quizzes/1/questions/1', json={"option_explanations": {"no": "Updated distractor"}}) + self.assertEqual(response.status_code, 200, response.text) + self.assertEqual(self.bank.db.get(Question, 1).option_explanations, {"no": "Updated distractor"}) + + +if __name__ == '__main__': + unittest.main() diff --git a/frontend/src/pages/QuestionBankPage.jsx b/frontend/src/pages/QuestionBankPage.jsx index 8743f06..19eb0a8 100644 --- a/frontend/src/pages/QuestionBankPage.jsx +++ b/frontend/src/pages/QuestionBankPage.jsx @@ -230,6 +230,7 @@ function QuestionEditModal({ question, categories, onSaved, onClose }) { explanation: question.explanation || '', question_category_id: question.question_category_id || '', extraCategoryIds: (question.category_ids || []).filter(id => id !== (question.question_category_id || null)), + option_explanations: { ...(question.option_explanations || {}) }, }) const [saving, setSaving] = useState(false) const [error, setError] = useState('') @@ -237,8 +238,25 @@ function QuestionEditModal({ question, categories, onSaved, onClose }) { const setOption = (i, val) => { const updated = [...form.options] const wasCorrect = form.options[i] === form.correct_answer + const oldValue = updated[i] updated[i] = val - setForm(f => ({ ...f, options: updated, correct_answer: wasCorrect ? val : f.correct_answer })) + setForm(f => { + const explanations = { ...f.option_explanations } + if (oldValue in explanations) { + explanations[val] = explanations[oldValue] + delete explanations[oldValue] + } + return { ...f, options: updated, correct_answer: wasCorrect ? val : f.correct_answer, option_explanations: explanations } + }) + } + + const setOptionExplanation = (option, value) => { + setForm(f => { + const explanations = { ...f.option_explanations } + if (value.trim()) explanations[option] = value + else delete explanations[option] + return { ...f, option_explanations: explanations } + }) } const toggleExtra = (categoryId) => { @@ -261,6 +279,7 @@ function QuestionEditModal({ question, categories, onSaved, onClose }) { ...form, question_category_id: primary, additional_category_ids: form.extraCategoryIds.filter(id => id !== primary), + option_explanations: Object.keys(form.option_explanations).length ? form.option_explanations : null, } const res = await api.patch(`/questions/${question.id}`, payload) onSaved({ ...question, ...res.data, @@ -292,15 +311,26 @@ function QuestionEditModal({ question, categories, onSaved, onClose }) {
{form.options.map((opt, i) => ( -
- setForm(f => ({ ...f, correct_answer: opt }))} - style={{ width: 'auto', accentColor: 'var(--primary)' }} /> - {LETTERS[i]} - setOption(i, e.target.value)} - style={{ flex: 1, padding: '7px 12px', border: `1.5px solid ${form.correct_answer === opt ? 'var(--correct-bd)' : 'var(--border)'}`, borderRadius: 6, fontSize: '0.875rem', background: form.correct_answer === opt ? 'var(--correct-bg)' : 'var(--input-bg)', color: 'var(--text)', fontFamily: 'inherit' }} /> +
+
+ setForm(f => ({ ...f, correct_answer: opt }))} + style={{ width: 'auto', accentColor: 'var(--primary)' }} /> + {LETTERS[i]} + setOption(i, e.target.value)} + style={{ flex: 1, padding: '7px 12px', border: `1.5px solid ${form.correct_answer === opt ? 'var(--correct-bd)' : 'var(--border)'}`, borderRadius: 6, fontSize: '0.875rem', background: form.correct_answer === opt ? 'var(--correct-bg)' : 'var(--input-bg)', color: 'var(--text)', fontFamily: 'inherit' }} /> +
+
+ + Explain this option{form.option_explanations[opt] ? ' ✓' : ''} + +