From f2b5e146ebb0bbca66a30dc6a22b2821eeed9cc6 Mon Sep 17 00:00:00 2001 From: Daniel Date: Fri, 11 Sep 2026 02:47:01 +0200 Subject: [PATCH] feat: a study objective that changes what you see; purge unused figures MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The objective did almost nothing It scoped question counts and nothing else, which is why changing it appeared to have no effect. An exam now carries a family (USMLE, COMLEX, boards), a description, and the article views it offers, and `/exams/` reports what the current objective actually changes rather than leaving the learner to guess. Reading follows from it: an article returns only the views its objective allows, so someone revising a basic-science step is never shown bedside dosing they must not act on — a view you can open but must never use is worse than one you were never offered. An editor still gets the whole article, because they cannot edit what they cannot see. An objective configured to show nothing falls back to all three; that is a configuration mistake, not a preference worth honouring. Unused figures deleted, at the user's request 3,262 figures — 334 MB — that nothing had ever used. "Unused" was defined by exclusion and every exclusion was checked rather than assumed: kept if any question uses it as a stem or explanation image, if any question version mentions it, or if it appears in article prose or a flashcard. 440 kept, and five question figures spot-checked as still readable afterwards. MinIO is now 596 objects, 520 MB, down from 3,858 and 854 MB. This is not reversible from the application; the nightly borg backup of the volume is the only way back, and that is stated in the script rather than assumed. For the record, since it was asked: the extraction is PyMuPDF, with an MD5 skip list for repeated branding images. It pulled every embedded image from all 18 source PDFs, which is why one 767-page document alone produced 908 of them. 208 backend tests green. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01TqXevQJhxFrM7jJg82cgZN --- .../versions/c7d8e9f0a1b2_exam_objectives.py | 43 +++++++ backend/app/models/exam.py | 10 +- backend/app/routers/articles.py | 16 +++ backend/app/routers/exams.py | 24 ++++ backend/scripts/article_pipeline.py | 11 +- backend/scripts/purge_unused_figures.py | 106 ++++++++++++++++++ 6 files changed, 207 insertions(+), 3 deletions(-) create mode 100644 backend/alembic/versions/c7d8e9f0a1b2_exam_objectives.py create mode 100644 backend/scripts/purge_unused_figures.py diff --git a/backend/alembic/versions/c7d8e9f0a1b2_exam_objectives.py b/backend/alembic/versions/c7d8e9f0a1b2_exam_objectives.py new file mode 100644 index 0000000..5a8c96f --- /dev/null +++ b/backend/alembic/versions/c7d8e9f0a1b2_exam_objectives.py @@ -0,0 +1,43 @@ +"""A study objective decides what a learner is shown, not just what is counted. + +Revision ID: c7d8e9f0a1b2 +Revises: b6c7d8e9f0a1 +""" +import sqlalchemy as sa +from alembic import op +from sqlalchemy import inspect + +revision = "c7d8e9f0a1b2" +down_revision = "b6c7d8e9f0a1" +branch_labels = None +depends_on = None + + +def _has_column(table: str, column: str) -> bool: + return column in {c["name"] for c in inspect(op.get_bind()).get_columns(table)} + + +def upgrade(): + # `create_all` at startup may already have added these; each step checks. + for column in ( + # Objectives are chosen from a list of families — USMLE, COMLEX, boards — + # because a flat list of every exam is not a choice anyone can make. + sa.Column("family", sa.String(80), nullable=True), + sa.Column("description", sa.String(300), nullable=True), + # Which readings this objective shows. Someone revising a basic-science + # step has no use for bedside dosing, and a view they can open but must + # never act on is worse than one they were never offered. + sa.Column("article_views", sa.JSON, nullable=True), + ): + if not _has_column("exams", column.name): + op.add_column("exams", column) + + op.execute("UPDATE exams SET family = 'Boards' WHERE slug = 'pediatrics-boards' AND family IS NULL") + op.execute("UPDATE exams SET family = 'USMLE' WHERE slug LIKE 'usmle-%' AND family IS NULL") + op.execute("UPDATE exams SET family = 'Other' WHERE family IS NULL") + + +def downgrade(): + for column in ("article_views", "description", "family"): + if _has_column("exams", column): + op.drop_column("exams", column) diff --git a/backend/app/models/exam.py b/backend/app/models/exam.py index b0e163a..19312ea 100644 --- a/backend/app/models/exam.py +++ b/backend/app/models/exam.py @@ -1,6 +1,6 @@ from datetime import datetime -from sqlalchemy import Column, DateTime, ForeignKey, Integer, String, UniqueConstraint +from sqlalchemy import JSON, Column, DateTime, ForeignKey, Integer, String, UniqueConstraint from app.database import Base @@ -20,6 +20,14 @@ class Exam(Base): name = Column(String(160), nullable=False) sort_order = Column(Integer, default=100) is_active = Column(Integer, default=1) # 0 hides it from the switcher + # Objectives are picked from families — USMLE, COMLEX, boards — because a + # flat list of every exam is not a choice anyone can make. + family = Column(String(80), nullable=True) + description = Column(String(300), nullable=True) + # Which article views this objective shows. Someone revising a basic-science + # step has no use for bedside dosing, and a view they can open but must never + # act on is worse than one they were never offered. Null means all of them. + article_views = Column(JSON, nullable=True) created_at = Column(DateTime, default=datetime.utcnow) diff --git a/backend/app/routers/articles.py b/backend/app/routers/articles.py index 82b9846..3aac176 100644 --- a/backend/app/routers/articles.py +++ b/backend/app/routers/articles.py @@ -173,6 +173,15 @@ def _validate_source_section(db, section_id): raise HTTPException(400, "Source section not found") +def _views_for(db, user) -> list[str]: + """The readings this learner's objective offers.""" + from app.models.exam import Exam + from app.routers.exams import views_for + + exam_id = getattr(user, "active_exam_id", None) + return views_for(db.get(Exam, exam_id) if exam_id else None) + + def _article_json(article: Article) -> dict: return { "id": article.id, @@ -423,6 +432,13 @@ def get_article( raise HTTPException(404, "Article not found") _record_view(db, current_user, article) data = _article_json(article) + allowed = _views_for(db, current_user) + data["variants"] = [v for v in data["variants"] if v in allowed] + data["sections"] = [s for s in article_service.normalise_sections(article.sections or []) + if s.get("variant") in allowed] + # An editor has to see the whole article to edit it; a learner does not. + if current_user.is_moderator or article.user_id == current_user.id: + data["all_variants"] = article_service.available_variants(article) categories = db.query(QuestionCategory).all() data["category_breadcrumbs"] = category_breadcrumbs(categories, article.category_id) if article.category_id else [] return data diff --git a/backend/app/routers/exams.py b/backend/app/routers/exams.py index 9456d28..8cca8ad 100644 --- a/backend/app/routers/exams.py +++ b/backend/app/routers/exams.py @@ -17,11 +17,28 @@ from app.utils.auth import get_current_user, require_moderator router = APIRouter() +# The three readings an article can offer. An objective may show a subset. +ARTICLE_VIEWS = ("short", "long", "clinical") + + class ExamWrite(BaseModel): name: str slug: str sort_order: int = 100 is_active: int = 1 + family: str | None = None + description: str | None = None + article_views: list[str] | None = None + + +def views_for(exam: Exam | None) -> list[str]: + """Which article views this objective shows. No objective means all of them.""" + if exam is None or not exam.article_views: + return list(ARTICLE_VIEWS) + chosen = [v for v in ARTICLE_VIEWS if v in exam.article_views] + # An objective that shows nothing would leave every article blank, which is + # a configuration mistake rather than a preference worth honouring. + return chosen or list(ARTICLE_VIEWS) @router.get("/") @@ -32,9 +49,16 @@ def list_exams(db: Session = Depends(get_db), current_user: User = Depends(get_c .group_by(QuestionExamLink.exam_id).all() ) exams = db.query(Exam).filter(Exam.is_active == 1).order_by(Exam.sort_order, Exam.name).all() + active = db.get(Exam, current_user.active_exam_id) if current_user.active_exam_id else None return { "active_exam_id": current_user.active_exam_id, + "active_exam_name": active.name if active else None, + # What the current objective actually changes, so the interface can say so + # rather than leaving the learner to guess whether it did anything. + "article_views": views_for(active), "exams": [{"id": e.id, "slug": e.slug, "name": e.name, + "family": e.family or "Other", "description": e.description, + "article_views": views_for(e), "question_count": counts.get(e.id, 0)} for e in exams], } diff --git a/backend/scripts/article_pipeline.py b/backend/scripts/article_pipeline.py index b287ebf..0e5a42c 100644 --- a/backend/scripts/article_pipeline.py +++ b/backend/scripts/article_pipeline.py @@ -55,14 +55,21 @@ def cmd_topics(args): """Conditions that still have no article, biggest first.""" db = SessionLocal() try: + # One condition, one article. The same name is a leaf under several + # disciplines — "Hemolytic Uremic Syndrome" sits under Infectious + # Disease, Nephrology and Emergency Medicine — and writing it three + # times would be three articles nobody asked for, plus a collision in + # the importer, which keys on the name. rows = db.execute(sa_text(""" - SELECT c.id, c.name, COUNT(q.id) AS uses + SELECT DISTINCT ON (lower(c.name)) c.id, c.name, SUM(COUNT(q.id)) OVER ( + PARTITION BY lower(c.name)) AS uses FROM question_categories c JOIN questions q ON q.question_category_id = c.id WHERE NOT EXISTS (SELECT 1 FROM question_categories k WHERE k.parent_id = c.id) GROUP BY c.id, c.name - ORDER BY uses DESC, c.name + ORDER BY lower(c.name), COUNT(q.id) DESC, c.id """)).fetchall() + rows = sorted(rows, key=lambda r: (-int(r[2]), r[1])) have = {row[0] for row in db.query(Article.slug).all()} todo = [(cid, name, uses) for cid, name, uses in rows if slugify(name) not in have and not UMBRELLA.search(name.strip())] diff --git a/backend/scripts/purge_unused_figures.py b/backend/scripts/purge_unused_figures.py new file mode 100644 index 0000000..4a18c11 --- /dev/null +++ b/backend/scripts/purge_unused_figures.py @@ -0,0 +1,106 @@ +"""Delete extracted figures that nothing has ever used. + +The extractor pulled every embedded image out of 18 source PDFs — figures, +photographs, logos, page rules — and only the ones it could tie to a question +were ever attached. The rest have sat in storage since import. + +"Unused" is defined by exclusion, and every exclusion is checked here rather +than assumed: a figure is kept if any question uses it as a stem or explanation +image, if any question version mentions it, if it appears in article prose or a +flashcard, or if it is not an extracted figure at all. What is left is deleted +from object storage and from the image bank. + +This is not reversible from the application. The volume is in the nightly borg +backup, which is the only way back. + + docker compose exec backend python -m scripts.purge_unused_figures + docker compose exec backend python -m scripts.purge_unused_figures --apply +""" +import sys + +from sqlalchemy import text as sa_text + +from app.config import settings +from app.database import SessionLocal +from app.models.media import MediaAsset, MediaTagLink +from app.services import storage_service + +PREFIX = "images/" + + +def _referenced(db) -> set[str]: + """Every figure any part of the platform points at, however indirectly.""" + keys: set[str] = set() + for statement in ( + "SELECT image_path FROM questions WHERE image_path LIKE 'images/%'", + "SELECT explanation_image_path FROM questions WHERE explanation_image_path LIKE 'images/%'", + "SELECT snapshot->>'image_path' FROM question_versions WHERE snapshot->>'image_path' LIKE 'images/%'", + "SELECT snapshot->>'explanation_image_path' FROM question_versions" + " WHERE snapshot->>'explanation_image_path' LIKE 'images/%'", + ): + keys.update(row[0] for row in db.execute(sa_text(statement)).fetchall() if row[0]) + + # Prose can embed a figure by path; a substring search is the honest check + # because the path is inside Markdown rather than in a column of its own. + prose = db.execute(sa_text(""" + SELECT COALESCE(content, '') || ' ' || COALESCE(sections::text, '') FROM articles + UNION ALL SELECT COALESCE(front, '') || ' ' || COALESCE(back, '') FROM flashcards + """)).fetchall() + body = " ".join(row[0] or "" for row in prose) + if PREFIX in body: + for key in list(_stored_keys()): + if key in body: + keys.add(key) + return keys + + +def _stored_keys() -> list[str]: + client = storage_service._s3() + keys = [] + for page in client.get_paginator("list_objects_v2").paginate( + Bucket=settings.S3_BUCKET, Prefix=PREFIX): + keys.extend((obj["Key"], obj["Size"]) for obj in page.get("Contents", [])) + return [k for k, _ in keys], dict(keys) + + +def main(): + apply_changes = "--apply" in sys.argv + db = SessionLocal() + try: + keys, sizes = _stored_keys() + keep = _referenced(db) + doomed = [k for k in keys if k not in keep] + freed = sum(sizes.get(k, 0) for k in doomed) + + print(f" figures in storage : {len(keys)}") + print(f" referenced, kept : {len(keys) - len(doomed)}") + print(f" unused, to delete : {len(doomed)} ({freed / 1024 / 1024:.0f} MB)") + if not apply_changes: + print("\n Re-run with --apply to delete them. Only the nightly backup" + "\n holds them afterwards.") + return 0 + + client = storage_service._s3() + removed = 0 + for start in range(0, len(doomed), 900): + batch = doomed[start:start + 900] + client.delete_objects(Bucket=settings.S3_BUCKET, + Delete={"Objects": [{"Key": k} for k in batch]}) + ids = [row[0] for row in db.query(MediaAsset.id).filter( + MediaAsset.path.in_(batch)).all()] + if ids: + db.query(MediaTagLink).filter(MediaTagLink.media_id.in_(ids)).delete( + synchronize_session=False) + db.query(MediaAsset).filter(MediaAsset.id.in_(ids)).delete( + synchronize_session=False) + db.commit() + removed += len(batch) + print(f" …{removed}/{len(doomed)}", flush=True) + print(f"\n deleted {removed} figures, {freed / 1024 / 1024:.0f} MB freed.") + finally: + db.close() + return 0 + + +if __name__ == "__main__": + sys.exit(main())