diff --git a/backend/alembic/versions/f0a1b2c3d4e5_question_figures.py b/backend/alembic/versions/f0a1b2c3d4e5_question_figures.py new file mode 100644 index 0000000..324c586 --- /dev/null +++ b/backend/alembic/versions/f0a1b2c3d4e5_question_figures.py @@ -0,0 +1,42 @@ +"""Figures on a question: many of them, labelled, and each a real image record. + +Revision ID: f0a1b2c3d4e5 +Revises: e9f0a1b2c3d4 +""" +import sqlalchemy as sa +from alembic import op +from sqlalchemy import inspect + +revision = "f0a1b2c3d4e5" +down_revision = "e9f0a1b2c3d4" +branch_labels = None +depends_on = None + + +def upgrade(): + # A question could carry exactly one stem image and one explanation image, + # held as a bare path with no title, no legend and no way to refer to it + # from the text. This makes a figure a row: it points at an image in the + # bank, carries the label the prose calls it by, and there can be several. + if "question_media" not in inspect(op.get_bind()).get_table_names(): + op.create_table( + "question_media", + sa.Column("id", sa.Integer, primary_key=True), + sa.Column("question_id", sa.Integer, + sa.ForeignKey("questions.id", ondelete="CASCADE"), nullable=False, index=True), + sa.Column("media_id", sa.Integer, + sa.ForeignKey("media_assets.id", ondelete="CASCADE"), nullable=False, index=True), + # Where it belongs. A figure that illustrates the answer must not + # appear beside the stem, which is the mistake this whole area had. + sa.Column("role", sa.String(20), nullable=False, server_default="stem"), + # What the prose calls it — "Figure 1" — so the text can say + # "shown in Figure 1" and mean something. + sa.Column("label", sa.String(80), nullable=True), + sa.Column("caption", sa.Text, nullable=True), + sa.Column("position", sa.Integer, server_default="0"), + sa.UniqueConstraint("question_id", "media_id", "role", name="uq_question_media"), + ) + + +def downgrade(): + op.drop_table("question_media") diff --git a/backend/app/main.py b/backend/app/main.py index 6b4c317..ad0dfa6 100644 --- a/backend/app/main.py +++ b/backend/app/main.py @@ -168,7 +168,7 @@ def setup_pgvector(): # Import new models so create_all picks them up from app.models import quiz_category, quiz_question_link, question_category, favorite # noqa from app.models import flashcard, course # noqa - from app.models import category_grant, conversation, exam, media, study_plan # noqa + from app.models import category_grant, conversation, exam, media, question_media, study_plan # noqa # Kill stale idle-in-transaction connections from previous killed startups. # They hold DDL locks and cause ALTER TABLE below to hang indefinitely. diff --git a/backend/app/models/question_media.py b/backend/app/models/question_media.py new file mode 100644 index 0000000..418555c --- /dev/null +++ b/backend/app/models/question_media.py @@ -0,0 +1,26 @@ +from sqlalchemy import Column, ForeignKey, Integer, String, Text, UniqueConstraint + +from app.database import Base + + +class QuestionMedia(Base): + """One figure on a question. + + A figure is a row rather than a path so it can be labelled, ordered, and + reused: the same radiograph can illustrate two questions without being + stored twice, and the bank knows where each one is used. + """ + + __tablename__ = "question_media" + __table_args__ = (UniqueConstraint("question_id", "media_id", "role", name="uq_question_media"),) + + id = Column(Integer, primary_key=True, index=True) + question_id = Column(Integer, ForeignKey("questions.id", ondelete="CASCADE"), nullable=False, index=True) + media_id = Column(Integer, ForeignKey("media_assets.id", ondelete="CASCADE"), nullable=False, index=True) + # stem | explanation. A figure that gives the answer away must never sit + # beside the question, which is the mistake this area started with. + role = Column(String(20), nullable=False, default="stem") + # What the prose calls it, so "shown in Figure 1" refers to something. + label = Column(String(80), nullable=True) + caption = Column(Text, nullable=True) + position = Column(Integer, default=0) diff --git a/backend/app/routers/attempts.py b/backend/app/routers/attempts.py index 0c29fb5..95dc2ab 100644 --- a/backend/app/routers/attempts.py +++ b/backend/app/routers/attempts.py @@ -13,7 +13,7 @@ from app.database import get_db from app.models.quiz import Quiz from app.models.question import Question from app.models.question_category import QuestionCategory -from app.services.quiz_builder import category_breadcrumbs +from app.services.quiz_builder import bank_query, category_breadcrumbs from app.models.attempt import QuizAttempt, AttemptAnswer from app.models.pdf_document import PDFDocument from app.models.user import User @@ -580,12 +580,28 @@ def get_dashboard_stats( average_score=round(sum(pcts) / len(pcts), 1), )) + # What a learner is actually working through is questions, not quizzes: how + # many of the bank they have seen, and how many they have got right at least + # once. A count of quizzes says how the material happens to be packaged. + seen, mastered = db.query( + func.count(func.distinct(AttemptAnswer.question_id)), + func.count(func.distinct(case( + (AttemptAnswer.is_correct.is_(True), AttemptAnswer.question_id)))), + ).join(QuizAttempt, QuizAttempt.id == AttemptAnswer.attempt_id).filter( + QuizAttempt.user_id == current_user.id, + ).first() or (0, 0) + + bank_total = bank_query(db, current_user).count() + return DashboardStats( total_documents=total_docs, total_quizzes=total_quizzes, total_attempts=total_attempts, average_score=avg_score, quiz_stats=quiz_stats, + questions_seen=int(seen or 0), + questions_correct=int(mastered or 0), + bank_total=bank_total, ) diff --git a/backend/app/routers/questions.py b/backend/app/routers/questions.py index e97ba71..4e61079 100644 --- a/backend/app/routers/questions.py +++ b/backend/app/routers/questions.py @@ -854,6 +854,7 @@ def get_question_detail( "option_explanations": question.option_explanations, "key_points": question.key_points, "attending_tip": question.attending_tip, + "figures": _figures_for(db, question.id), "difficulty": question.difficulty, "question_category_id": question.question_category_id, "question_category_name": category.name if category else None, @@ -1216,3 +1217,105 @@ def import_qti( "errors": errors[:20], "total_items": len(items), } + + +# ── Figures ─────────────────────────────────────────────────────────────────── + +class FigureIn(BaseModel): + media_id: int + role: Literal["stem", "explanation"] = "stem" + label: str | None = Field(default=None, max_length=80) + caption: str | None = None + + +class FigureUpdate(BaseModel): + label: str | None = Field(default=None, max_length=80) + caption: str | None = None + role: Literal["stem", "explanation"] | None = None + position: int | None = None + + +def _figure_json(link, asset) -> dict: + return { + "id": link.id, "media_id": link.media_id, "role": link.role, + # The label the prose refers to. Falls back to a number so a figure is + # never nameless, which is what makes "see the figure" ambiguous. + "label": link.label or f"Figure {link.position + 1}", + "caption": link.caption or getattr(asset, "caption", None), + "title": getattr(asset, "title", None), + "path": getattr(asset, "path", None), + "position": link.position, + } + + +def _figures_for(db: Session, question_id: int) -> list[dict]: + from app.models.media import MediaAsset + from app.models.question_media import QuestionMedia + + rows = db.query(QuestionMedia, MediaAsset).join( + MediaAsset, MediaAsset.id == QuestionMedia.media_id).filter( + QuestionMedia.question_id == question_id).order_by( + QuestionMedia.role, QuestionMedia.position, QuestionMedia.id).all() + return [_figure_json(link, asset) for link, asset in rows] + + +@router.get("/detail/{question_id}/figures") +def list_figures(question_id: int, db: Session = Depends(get_db), + current_user: User = Depends(get_current_user)): + return _figures_for(db, question_id) + + +@router.post("/detail/{question_id}/figures", status_code=201) +def add_figure(question_id: int, data: FigureIn, db: Session = Depends(get_db), + current_user: User = Depends(require_moderator)): + from app.models.media import MediaAsset + from app.models.question_media import QuestionMedia + + if not db.query(Question.id).filter(Question.id == question_id).first(): + raise HTTPException(404, "Question not found") + if not db.get(MediaAsset, data.media_id): + raise HTTPException(404, "Image not found") + if db.query(QuestionMedia.id).filter_by( + question_id=question_id, media_id=data.media_id, role=data.role).first(): + raise HTTPException(409, "That image is already on this question") + + position = db.query(QuestionMedia).filter_by(question_id=question_id, role=data.role).count() + link = QuestionMedia(question_id=question_id, media_id=data.media_id, role=data.role, + label=data.label, caption=data.caption, position=position) + db.add(link) + db.commit() + return {"figures": _figures_for(db, question_id)} + + +@router.patch("/figures/{figure_id}") +def update_figure(figure_id: int, data: FigureUpdate, db: Session = Depends(get_db), + current_user: User = Depends(require_moderator)): + from app.models.question_media import QuestionMedia + + link = db.get(QuestionMedia, figure_id) + if not link: + raise HTTPException(404, "Figure not found") + for field, value in data.model_dump(exclude_unset=True).items(): + setattr(link, field, value) + db.commit() + return {"figures": _figures_for(db, link.question_id)} + + +@router.delete("/figures/{figure_id}", status_code=204) +def remove_figure(figure_id: int, db: Session = Depends(get_db), + current_user: User = Depends(require_moderator)): + """Take a figure off a question. The image itself stays in the bank.""" + from app.models.question_media import QuestionMedia + + link = db.get(QuestionMedia, figure_id) + if not link: + raise HTTPException(404, "Figure not found") + question_id, role, position = link.question_id, link.role, link.position + db.delete(link) + db.flush() + # Close the gap so labels that fall back to a number stay sequential. + for other in db.query(QuestionMedia).filter( + QuestionMedia.question_id == question_id, QuestionMedia.role == role, + QuestionMedia.position > position).all(): + other.position -= 1 + db.commit() diff --git a/backend/app/schemas/attempt.py b/backend/app/schemas/attempt.py index 96978be..6e5a09f 100644 --- a/backend/app/schemas/attempt.py +++ b/backend/app/schemas/attempt.py @@ -69,3 +69,8 @@ class DashboardStats(BaseModel): total_attempts: int average_score: float quiz_stats: list[QuizStats] = [] + # Question-centred figures. The quiz counts above describe how the material + # happens to be packaged; these describe what the learner has worked through. + questions_seen: int = 0 + questions_correct: int = 0 + bank_total: int = 0 diff --git a/backend/scripts/backfill_question_figures.py b/backend/scripts/backfill_question_figures.py new file mode 100644 index 0000000..433b924 --- /dev/null +++ b/backend/scripts/backfill_question_figures.py @@ -0,0 +1,99 @@ +"""Turn each question's image paths into labelled figure records. + +Every figure is already a row in the image bank; what was missing was the link +that says *this question, this role, this label*. Without it a question could +hold one stem image and one explanation image, neither of which the prose could +refer to by name. + +Titles are improved at the same time. A figure called +`page_339_img_0.png` tells you where it came from and nothing about what it +shows, so the provenance moves into the caption and the tags — where it is still +searchable — and the title becomes something a person can read. + + docker compose exec backend python -m scripts.backfill_question_figures + docker compose exec backend python -m scripts.backfill_question_figures --apply +""" +import re +import sys + +from sqlalchemy import text as sa_text + +from app.database import SessionLocal +from app.models.media import MediaAsset +from app.models.question_media import QuestionMedia + +ROLES = (("image_path", "stem"), ("explanation_image_path", "explanation")) + + +def readable_title(asset: MediaAsset, question_id: int, role: str, category: str | None) -> str: + """A name a person can use, with the filename kept as provenance.""" + where = "Stem figure" if role == "stem" else "Explanation figure" + subject = category or f"question #{question_id}" + return f"{where} — {subject}"[:300] + + +def main(): + apply_changes = "--apply" in sys.argv + db = SessionLocal() + try: + rows = db.execute(sa_text(""" + SELECT q.id, q.image_path, q.explanation_image_path, c.name + FROM questions q + LEFT JOIN question_categories c ON c.id = q.question_category_id + WHERE (q.image_path IS NOT NULL AND q.image_path <> '') + OR (q.explanation_image_path IS NOT NULL AND q.explanation_image_path <> '') + """)).fetchall() + assets = {a.path: a for a in db.query(MediaAsset).all()} + existing = {(link.question_id, link.media_id, link.role) + for link in db.query(QuestionMedia).all()} + + planned, missing = [], [] + for question_id, stem_path, expl_path, category in rows: + for column, role in ROLES: + path = stem_path if column == "image_path" else expl_path + if not path: + continue + asset = assets.get(path) + if asset is None: + missing.append((question_id, path)) + continue + if (question_id, asset.id, role) in existing: + continue + planned.append((question_id, asset, role, category)) + + print(f" questions with a figure : {len(rows)}") + print(f" figure links to create : {len(planned)}") + if missing: + print(f" paths with no image record: {len(missing)} (left alone)") + if not apply_changes: + for question_id, asset, role, category in planned[:6]: + print(f" q#{question_id:<6} {role:<12} {asset.path}") + print(f" title -> {readable_title(asset, question_id, role, category)}") + print("\n Re-run with --apply.") + return 0 + + made = retitled = 0 + for question_id, asset, role, category in planned: + position = db.query(QuestionMedia).filter_by( + question_id=question_id, role=role).count() + db.add(QuestionMedia(question_id=question_id, media_id=asset.id, role=role, + label=f"Figure {position + 1}", position=position)) + made += 1 + # Keep the filename as provenance in the caption; it is the only + # record of which page of which PDF this came from. + if asset.title and re.match(r"^page_\d+_img", asset.title): + asset.caption = (asset.caption or "") + f" (from {asset.path})" + asset.title = readable_title(asset, question_id, role, category) + retitled += 1 + if made % 200 == 0: + db.commit() + db.commit() + print(f"\n figure links created : {made}") + print(f" images retitled : {retitled}") + finally: + db.close() + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/docs/TODO.md b/docs/TODO.md index b69337c..c50bcdb 100644 --- a/docs/TODO.md +++ b/docs/TODO.md @@ -34,6 +34,29 @@ Captured so nothing is lost while the article writing runs. - [ ] **Adaptive session** — questions ordered by what would help most, with an explanation of how it decides. +### Editor and figures +- [x] **Rich editing on the question page** — no new platform needed: Milkdown + is already installed and used for articles, courses and the quick modal. +- [ ] **Milkdown on the stem and the options**, replacing the plain textareas. + Note the stem carries manual-highlight offsets, so check what a WYSIWYG + rewrite does to them before switching. +- [x] **Many figures per question** — `question_media` links a question to any + number of images in the bank, each with a role (stem or explanation), a + label the prose can refer to ("Figure 1") and an order. The 346 existing + single paths were backfilled and retitled; the filename stays in the + caption as provenance. +- [ ] **Explanation figures as labelled thumbnails** that open a preview, so the + prose can say "refer to Figure 2". +- [ ] **Figure management in the question editor** — add, label, caption, + reorder, remove, using the image picker. + +### Dashboard +- [x] **Stats are about questions, not quizzes** — questions seen out of the + bank, answered correctly, average score. +- [x] **The old performance card is gone** — a per-quiz graph needing two + attempts, superseded by the session analysis. +- [ ] **Vary the greeting** rather than one fixed line. + ### Questions I owe an answer to - [x] **What extracted the PDFs?** PyMuPDF (`fitz`) in `pdf_service.py`, with an MD5 skip list for repeated branding images. It pulled every embedded image diff --git a/frontend/src/pages/DashboardPage.jsx b/frontend/src/pages/DashboardPage.jsx index ffcb7c8..44b6998 100644 --- a/frontend/src/pages/DashboardPage.jsx +++ b/frontend/src/pages/DashboardPage.jsx @@ -60,16 +60,21 @@ export default function DashboardPage() { return (
- + {/* The greeting names the page; "continue your study" is the first thing + on it. Below the fold it read as a heading for the wrong section. */}

{greetingText}

+ {stats && (
+ {/* What is being worked through is questions. A count of quizzes + describes how the material happens to be packaged. */} {[ - { value: stats.total_quizzes, label: 'Quizzes' }, - { value: stats.total_attempts, label: 'Attempts' }, - { value: `${stats.average_score}%`, label: 'Avg Score' }, + { value: stats.bank_total ? `${stats.questions_seen}/${stats.bank_total}` : stats.questions_seen, + label: 'Questions seen' }, + { value: stats.questions_correct, label: 'Answered correctly' }, + { value: `${stats.average_score}%`, label: 'Average score' }, ].map(s => (
{s.value}
@@ -85,84 +90,6 @@ export default function DashboardPage() { - {/* Performance graph with dropdown */} - {history.length > 0 && ( -
-
-

Performance

- -
- - {selectedQuiz && ( - <> -
- {selectedQuiz.attempts.length} attempt{selectedQuiz.attempts.length !== 1 ? 's' : ''} - {selectedQuiz.attempts.length > 0 && ( - <> - Latest: = 75 ? '#22c55e' : '#ef4444' - }}>{selectedQuiz.attempts[selectedQuiz.attempts.length - 1].percentage}% - Best: {Math.max(...selectedQuiz.attempts.map(a => a.percentage))}% - - )} - Retake -
- - {selectedQuiz.attempts.length > 0 && selectedQuiz.attempts[selectedQuiz.attempts.length - 1].percentage < 75 && ( -

- ⚠️ Below 75% — a reminder will be sent to review this quiz -

- )} - {/* Attempt rows with delete */} - {selectedQuiz.attempts.length > 0 && ( -
- {[...selectedQuiz.attempts].reverse().map(a => ( -
- {new Date(a.date).toLocaleDateString()} - - {a.percentage}% ({a.score}/{a.total}) - - {confirmAttempt === a.attempt_id ? ( - <> - - - - ) : ( - - )} -
- ))} -
- )} - - )} -
- )}
) } diff --git a/frontend/src/pages/QuizPage.jsx b/frontend/src/pages/QuizPage.jsx index cb2cf74..e9c7dae 100644 --- a/frontend/src/pages/QuizPage.jsx +++ b/frontend/src/pages/QuizPage.jsx @@ -976,6 +976,9 @@ const timerStarted = timeLeft !== null const answeredCount = Object.keys(answers).length const totalCount = questions.length const isLast = currentIdx === totalCount - 1 + // Whether the answer is in. Category and difficulty are hints, so they wait + // for this; in exam mode nothing is revealed until the whole test is over. + const answerRevealed = isStudy && !!answers[current?.id] const quizNavigation = (position = 'bottom') => (