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.
This commit is contained in:
Daniel
parent
a1e9340004
commit
179e51d143
12 changed files with 199 additions and 11 deletions
19
backend/alembic/versions/j3e4f5a6b708_option_explanations.py
Normal file
19
backend/alembic/versions/j3e4f5a6b708_option_explanations.py
Normal file
|
|
@ -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")
|
||||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
)
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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):
|
||||
|
|
|
|||
|
|
@ -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 = (
|
||||
|
|
|
|||
68
backend/tests/test_option_explanations.py
Normal file
68
backend/tests/test_option_explanations.py
Normal file
|
|
@ -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()
|
||||
|
|
@ -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 }) {
|
|||
<label>Options — select the correct one</label>
|
||||
<div style={{ display: 'flex', flexDirection: 'column', gap: 8 }}>
|
||||
{form.options.map((opt, i) => (
|
||||
<div key={i} style={{ display: 'flex', gap: 8, alignItems: 'center' }}>
|
||||
<input type="radio" name="correct" checked={form.correct_answer === opt}
|
||||
onChange={() => setForm(f => ({ ...f, correct_answer: opt }))}
|
||||
style={{ width: 'auto', accentColor: 'var(--primary)' }} />
|
||||
<span style={{ display: 'inline-flex', alignItems: 'center', justifyContent: 'center', width: 24, height: 24, borderRadius: '50%', flexShrink: 0, fontSize: '0.72rem', fontWeight: 700,
|
||||
background: form.correct_answer === opt ? 'var(--correct-fg)' : 'var(--border)',
|
||||
color: form.correct_answer === opt ? 'white' : 'var(--text-muted)' }}>{LETTERS[i]}</span>
|
||||
<input type="text" value={opt} onChange={e => 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' }} />
|
||||
<div key={i} style={{ display: 'flex', flexDirection: 'column', gap: 4 }}>
|
||||
<div style={{ display: 'flex', gap: 8, alignItems: 'center' }}>
|
||||
<input type="radio" name="correct" checked={form.correct_answer === opt}
|
||||
onChange={() => setForm(f => ({ ...f, correct_answer: opt }))}
|
||||
style={{ width: 'auto', accentColor: 'var(--primary)' }} />
|
||||
<span style={{ display: 'inline-flex', alignItems: 'center', justifyContent: 'center', width: 24, height: 24, borderRadius: '50%', flexShrink: 0, fontSize: '0.72rem', fontWeight: 700,
|
||||
background: form.correct_answer === opt ? 'var(--correct-fg)' : 'var(--border)',
|
||||
color: form.correct_answer === opt ? 'white' : 'var(--text-muted)' }}>{LETTERS[i]}</span>
|
||||
<input type="text" value={opt} onChange={e => 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' }} />
|
||||
</div>
|
||||
<details open={!!form.option_explanations[opt]} style={{ marginLeft: 32 }}>
|
||||
<summary style={{ fontSize: '0.74rem', color: 'var(--text-muted)', cursor: 'pointer' }}>
|
||||
Explain this option{form.option_explanations[opt] ? ' ✓' : ''}
|
||||
</summary>
|
||||
<textarea rows={2} placeholder="Why this option is wrong or right — shown in study mode"
|
||||
value={form.option_explanations[opt] || ''} aria-label={`Explanation for option ${opt}`}
|
||||
onChange={e => setOptionExplanation(opt, e.target.value)}
|
||||
style={{ width: '100%', marginTop: 4, padding: '7px 10px', border: '1px solid var(--border)', borderRadius: 6, fontSize: '0.82rem', background: 'var(--input-bg)', color: 'var(--text)', fontFamily: 'inherit' }} />
|
||||
</details>
|
||||
</div>
|
||||
))}
|
||||
</div>
|
||||
|
|
|
|||
|
|
@ -229,4 +229,23 @@ describe('QuestionBankPage edit modal multi-category', () => {
|
|||
question_category_id: 1,
|
||||
})))
|
||||
})
|
||||
|
||||
it('saves per-option explanations keyed by option text', async () => {
|
||||
const cats = [
|
||||
{ id: 1, name: 'Neonatology', parent_id: null, breadcrumbs: [{ id: 1, name: 'Neonatology' }] },
|
||||
]
|
||||
const question = { id: 7, question_text: 'Edit me', question_type: 'mcq', options: ['A', 'B'], correct_answer: 'A', explanation: '', question_category_id: 1, category_ids: [1] }
|
||||
const initialGet = api.get.getMockImplementation()
|
||||
api.get.mockImplementation(url => url === '/question-categories/' ? Promise.resolve({ data: cats })
|
||||
: url === '/questions/bank' ? Promise.resolve({ data: { questions: [question], total: 1 } }) : initialGet(url))
|
||||
api.patch = vi.fn().mockResolvedValue({ data: {} })
|
||||
renderPage()
|
||||
await userEvent.click(await screen.findByRole('button', { name: 'Edit' }))
|
||||
await userEvent.click(screen.getAllByText('Explain this option')[0])
|
||||
await userEvent.type(screen.getByLabelText('Explanation for option A'), 'Right because of this')
|
||||
await userEvent.click(screen.getByRole('button', { name: 'Save Changes' }))
|
||||
await waitFor(() => expect(api.patch).toHaveBeenCalledWith('/questions/7', expect.objectContaining({
|
||||
option_explanations: { A: 'Right because of this' },
|
||||
})))
|
||||
})
|
||||
})
|
||||
|
|
|
|||
|
|
@ -1213,6 +1213,9 @@ const timerStarted = timeLeft !== null
|
|||
</span>
|
||||
{showCorrect && <span className="option-status option-status-correct">✓ Correct</span>}
|
||||
{showWrong && <span className="option-status option-status-wrong">✗ Wrong</span>}
|
||||
{hasAnswered && current.option_explanations?.[opt] && (
|
||||
<span className="quiz-option-explanation">{current.option_explanations[opt]}</span>
|
||||
)}
|
||||
</button>
|
||||
)
|
||||
})}
|
||||
|
|
|
|||
|
|
@ -147,11 +147,28 @@ describe('quiz player', () => {
|
|||
expect(await screen.findByText(/Full explanation, preserved without shortening/)).toBeInTheDocument()
|
||||
expect(await screen.findByText('8/10')).toBeInTheDocument()
|
||||
expect(screen.getByText('80%')).toBeInTheDocument()
|
||||
expect(screen.getByText(/Based on 10 recorded answers/)).toBeInTheDocument()
|
||||
expect(screen.getByText(/10 recorded answers/)).toBeInTheDocument()
|
||||
fireEvent.keyDown(window, { key: 'b' })
|
||||
await waitFor(() => expect(api.post).toHaveBeenCalledWith('/favorites', { question_id: 1 }))
|
||||
})
|
||||
|
||||
it('shows per-option explanations in study feedback', async () => {
|
||||
const originalGet = api.get.getMockImplementation()
|
||||
api.get.mockImplementation(async (url, ...args) => {
|
||||
const res = await originalGet(url, ...args)
|
||||
if (res.data?.questions && url.includes('attempt_id=')) {
|
||||
res.data.questions[0] = { ...res.data.questions[0], option_explanations: { 'First answer': 'Preferred because of this', 'Second answer': 'Distractor reason' } }
|
||||
}
|
||||
return res
|
||||
})
|
||||
await begin()
|
||||
fireEvent.keyDown(window, { key: '1' })
|
||||
fireEvent.keyDown(window, { key: 'Enter' })
|
||||
expect(await screen.findByText('Preferred because of this')).toBeInTheDocument()
|
||||
expect(screen.getByText('Distractor reason')).toBeInTheDocument()
|
||||
})
|
||||
|
||||
it('never fetches response statistics in exam mode', async () => {
|
||||
mode = 'exam'
|
||||
await begin(false)
|
||||
|
|
|
|||
|
|
@ -76,3 +76,4 @@
|
|||
.question-reading strong { display: block; margin-bottom: 6px; font-size: .9rem; }
|
||||
.question-reading ul { margin: 0; padding-left: 18px; display: flex; flex-direction: column; gap: 4px; }
|
||||
.question-reading a { color: var(--primary); font-size: .86rem; }
|
||||
.quiz-option-explanation { display: block; width: 100%; margin-top: 6px; padding: 6px 10px; border-radius: 6px; background: var(--input-bg); border: 1px solid var(--border); font-size: .8rem; color: var(--text); text-align: left; }
|
||||
|
|
|
|||
Loading…
Reference in a new issue