From 1f77d421c7e6066e376492fb421f4ea3d67a73e3 Mon Sep 17 00:00:00 2001 From: Daniel Date: Fri, 11 Sep 2026 03:44:44 +0200 Subject: [PATCH] feat: cross-link the articles, and strip what the analysis page replaced MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The linking was the gap The marker system was built weeks ago — resolves by id, survives a rename, shows a preview on hover — and not one of 333 articles used it. Every article was written in isolation, so a piece on croup named stridor and epiglottitis and offered no way to reach either. `scripts/link_articles.py` reads what is written and links it: 3,718 cross-references across 307 articles, by id, so a later rename cannot break them. Conservative on purpose, because a wrong link is worse than a missing one: only the first mention in a section, whole words, longest title first so "Otitis media with effusion" beats "Otitis media", never inside an existing link, marker, heading, code span or table, and never an article to itself. That exposed a second thing: the reading view had its own Markdown pipeline with its own cross-reference regex, and it only understood the old slug form. It would have printed every one of those 3,718 links as literal brackets. Article prose now goes through the same renderer as the rest of the site. Short and Clinical looked empty Both are usually a single section, and everything starts collapsed, so the tab showed one heading over blank space. A view of one section is not a contents page; it opens. Removed Quiz reminders — emailed nudges to retake anything under 75%, with a scheduler that existed solely to send them: the model, the service, the scheduler, the email, the table. Article comments. The dashboard's in-progress list and its stat cards, both of which the analysis page now answers better. One mistake worth recording: the first pass at removing the reminder cleanup used a regex that took 109 lines with it, including an unrelated endpoint. The test suite caught it (`/attempts/quiz/{id}/in-progress` returning 404 instead of 403), and the file was restored and edited by exact match instead. 208 backend, 249 frontend green. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01TqXevQJhxFrM7jJg82cgZN --- backend/alembic/env.py | 2 +- .../versions/a1b2c3d4e5f6_drop_reminders.py | 35 ++++++ backend/app/main.py | 10 +- backend/app/models/__init__.py | 2 - backend/app/models/quiz.py | 1 - backend/app/models/reminder.py | 23 ---- backend/app/models/user.py | 1 - backend/app/routers/attempts.py | 14 +-- backend/app/routers/media.py | 59 +++++++-- backend/app/services/email_service.py | 22 ---- backend/app/services/reminder_service.py | 61 --------- backend/app/utils/scheduler.py | 98 --------------- backend/scripts/link_articles.py | 116 ++++++++++++++++++ backend/scripts/retitle_figures.py | 22 ++++ docs/TODO.md | 22 +++- frontend/src/components/ArticleReader.jsx | 51 +++----- frontend/src/components/ImagePicker.css | 9 ++ frontend/src/components/ImagePicker.jsx | 33 ++++- frontend/src/components/InProgressQuizzes.jsx | 50 -------- frontend/src/components/MediaTile.jsx | 19 +++ frontend/src/pages/ArticleSplitView.test.jsx | 6 +- frontend/src/pages/ArticlesPage.jsx | 2 - frontend/src/pages/ArticlesPage.test.jsx | 8 +- frontend/src/pages/DashboardPage.jsx | 22 ---- frontend/src/pages/MediaPage.css | 2 + frontend/src/pages/MediaPage.jsx | 29 +++-- frontend/src/pages/MediaPage.test.jsx | 16 ++- 27 files changed, 364 insertions(+), 371 deletions(-) create mode 100644 backend/alembic/versions/a1b2c3d4e5f6_drop_reminders.py delete mode 100644 backend/app/models/reminder.py delete mode 100644 backend/app/services/reminder_service.py delete mode 100644 backend/app/utils/scheduler.py create mode 100644 backend/scripts/link_articles.py delete mode 100644 frontend/src/components/InProgressQuizzes.jsx create mode 100644 frontend/src/components/MediaTile.jsx diff --git a/backend/alembic/env.py b/backend/alembic/env.py index 1289767..171c346 100644 --- a/backend/alembic/env.py +++ b/backend/alembic/env.py @@ -21,7 +21,7 @@ if _db_url: if config.config_file_name is not None: fileConfig(config.config_file_name) -from app.models import User, PDFDocument, Section, Quiz, Question, QuizAttempt, AttemptAnswer, ReminderSchedule, UserNote # noqa +from app.models import User, PDFDocument, Section, Quiz, Question, QuizAttempt, AttemptAnswer, UserNote # noqa from app.database import Base target_metadata = Base.metadata diff --git a/backend/alembic/versions/a1b2c3d4e5f6_drop_reminders.py b/backend/alembic/versions/a1b2c3d4e5f6_drop_reminders.py new file mode 100644 index 0000000..6d80e58 --- /dev/null +++ b/backend/alembic/versions/a1b2c3d4e5f6_drop_reminders.py @@ -0,0 +1,35 @@ +"""Remove quiz reminders. + +Revision ID: a1b2c3d4e5f6 +Revises: f0a1b2c3d4e5 +""" +import sqlalchemy as sa +from alembic import op +from sqlalchemy import inspect + +revision = "a1b2c3d4e5f6" +down_revision = "f0a1b2c3d4e5" +branch_labels = None +depends_on = None + + +def upgrade(): + # Emailed nudges to retake a quiz below 75%. The platform is about sessions + # and topics now, and a scheduler existed solely to send these. + if "reminder_schedules" in inspect(op.get_bind()).get_table_names(): + op.drop_table("reminder_schedules") + + +def downgrade(): + op.create_table( + "reminder_schedules", + sa.Column("id", sa.Integer, primary_key=True), + sa.Column("user_id", sa.Integer, sa.ForeignKey("users.id", ondelete="CASCADE"), nullable=False), + sa.Column("quiz_id", sa.Integer, sa.ForeignKey("quizzes.id", ondelete="CASCADE"), nullable=False), + sa.Column("next_reminder_at", sa.DateTime, nullable=False), + sa.Column("interval_days", sa.Integer, server_default="1"), + sa.Column("performance_score", sa.Float, server_default="0"), + sa.Column("is_active", sa.Boolean, server_default=sa.true()), + sa.Column("created_at", sa.DateTime, server_default=sa.func.now()), + sa.Column("updated_at", sa.DateTime, server_default=sa.func.now()), + ) diff --git a/backend/app/main.py b/backend/app/main.py index ad0dfa6..241492d 100644 --- a/backend/app/main.py +++ b/backend/app/main.py @@ -13,7 +13,6 @@ from app.database import engine, Base, SessionLocal from app.routers import auth, documents, quizzes, attempts, admin, tts, nextcloud, categories, questions, question_categories, favorites, teach, contact, tags, flashcards, courses, mobile, mynote, exams from app.routers import study_tools, uploads, articles, comments, share, collections, study_plans, media, search, ai_mode from app.utils.auth import get_password_hash -from app.utils.scheduler import start_scheduler, stop_scheduler def seed_admin(): @@ -574,15 +573,10 @@ async def lifespan(app: FastAPI): os.makedirs(settings.CHROMA_PERSIST_DIR, exist_ok=True) seed_admin() seed_default_models() - # Scheduler and backfill must only run in one worker to avoid duplicate jobs / race conditions - is_primary = _acquire_singleton_lock() - if is_primary: + # The backfill must run in one worker only, or several race each other. + if _acquire_singleton_lock(): backfill_embeddings() - start_scheduler() yield - # Shutdown - if is_primary: - stop_scheduler() app = FastAPI( diff --git a/backend/app/models/__init__.py b/backend/app/models/__init__.py index 4eba7fc..ce0a2b9 100644 --- a/backend/app/models/__init__.py +++ b/backend/app/models/__init__.py @@ -4,7 +4,6 @@ from app.models.section import Section from app.models.quiz import Quiz from app.models.question import Question from app.models.attempt import QuizAttempt, AttemptAnswer -from app.models.reminder import ReminderSchedule from app.models.ai_model_config import AIModelConfig from app.models.favorite import Favorite from app.models.user_note import UserNote @@ -23,7 +22,6 @@ __all__ = [ "Question", "QuizAttempt", "AttemptAnswer", - "ReminderSchedule", "AIModelConfig", "Favorite", "UserNote", diff --git a/backend/app/models/quiz.py b/backend/app/models/quiz.py index 1748e19..d191c0c 100644 --- a/backend/app/models/quiz.py +++ b/backend/app/models/quiz.py @@ -43,4 +43,3 @@ class Quiz(Base): viewonly=True, # mutations handled explicitly via QuizQuestionLink ) attempts = relationship("QuizAttempt", back_populates="quiz", cascade="all, delete-orphan") - reminders = relationship("ReminderSchedule", cascade="all, delete-orphan", foreign_keys="ReminderSchedule.quiz_id") diff --git a/backend/app/models/reminder.py b/backend/app/models/reminder.py deleted file mode 100644 index 90fd648..0000000 --- a/backend/app/models/reminder.py +++ /dev/null @@ -1,23 +0,0 @@ -from datetime import datetime - -from sqlalchemy import Column, Integer, Float, Boolean, DateTime, ForeignKey -from sqlalchemy.orm import relationship - -from app.database import Base - - -class ReminderSchedule(Base): - __tablename__ = "reminder_schedules" - - id = Column(Integer, primary_key=True, index=True) - user_id = Column(Integer, ForeignKey("users.id", ondelete="CASCADE"), nullable=False) - quiz_id = Column(Integer, ForeignKey("quizzes.id", ondelete="CASCADE"), nullable=False) - next_reminder_at = Column(DateTime, nullable=False) - interval_days = Column(Integer, default=1) - performance_score = Column(Float, default=0.0) - is_active = Column(Boolean, default=True) - created_at = Column(DateTime, default=datetime.utcnow) - updated_at = Column(DateTime, default=datetime.utcnow, onupdate=datetime.utcnow) - - user = relationship("User", back_populates="reminders") - quiz = relationship("Quiz", back_populates="reminders") diff --git a/backend/app/models/user.py b/backend/app/models/user.py index e088ad0..2b7099e 100644 --- a/backend/app/models/user.py +++ b/backend/app/models/user.py @@ -25,7 +25,6 @@ class User(Base): documents = relationship("PDFDocument", back_populates="user") quizzes = relationship("Quiz", back_populates="user") attempts = relationship("QuizAttempt", back_populates="user") - reminders = relationship("ReminderSchedule", back_populates="user") favorites = relationship("Favorite", back_populates="user", cascade="all, delete-orphan") note = relationship("UserNote", back_populates="user", cascade="all, delete-orphan", uselist=False) diff --git a/backend/app/routers/attempts.py b/backend/app/routers/attempts.py index 95dc2ab..b1d7663 100644 --- a/backend/app/routers/attempts.py +++ b/backend/app/routers/attempts.py @@ -176,13 +176,6 @@ def submit_attempt( percentage = (score / attempt.total_questions * 100) if attempt.total_questions > 0 else 0 - # Update reminder schedule (skip course quizzes) - if not is_course_quiz: - try: - from app.services.reminder_service import update_reminder_schedule - update_reminder_schedule(db, current_user.id, attempt.quiz_id, percentage) - except Exception: - logger.warning("Failed to update reminder schedule for quiz %d", attempt.quiz_id, exc_info=True) return AttemptDetail( id=attempt.id, @@ -401,7 +394,7 @@ def delete_attempt( db: Session = Depends(get_db), current_user: User = Depends(get_current_user), ): - """Delete own attempt + answers + reminders for that quiz. Cannot delete course quiz attempts.""" + """Delete own attempt and its answers. Cannot delete course quiz attempts.""" attempt = db.query(QuizAttempt).filter( QuizAttempt.id == attempt_id, QuizAttempt.user_id == current_user.id, @@ -416,11 +409,6 @@ def delete_attempt( quiz_id = attempt.quiz_id # Delete all attempts for this quiz by this user (wipe history) - from app.models.reminder import ReminderSchedule - db.query(ReminderSchedule).filter( - ReminderSchedule.user_id == current_user.id, - ReminderSchedule.quiz_id == quiz_id, - ).delete() db.delete(attempt) db.commit() diff --git a/backend/app/routers/media.py b/backend/app/routers/media.py index d2fe3a6..7fe3aef 100644 --- a/backend/app/routers/media.py +++ b/backend/app/routers/media.py @@ -16,6 +16,7 @@ from app.database import get_db from app.models.media import MediaAsset, MediaLibrary, MediaLibraryGrant, MediaTagLink from app.models.user import User from app.services import embedding_service, storage_service +from app.models.question_media import QuestionMedia from app.services.search_service import hybrid_ids from app.utils.auth import get_current_user, require_moderator @@ -23,7 +24,28 @@ router = APIRouter() log = logging.getLogger(__name__) MAX_IMAGE_BYTES = 12 * 1024 * 1024 -ALLOWED_TYPES = {"image/png", "image/jpeg", "image/gif", "image/webp", "image/svg+xml"} +# A murmur is thirty seconds of audio and a bedside clip is a few megabytes, so +# they get their own ceiling rather than being squeezed under the image one. +MAX_MEDIA_BYTES = 60 * 1024 * 1024 +IMAGE_TYPES = {"image/png", "image/jpeg", "image/gif", "image/webp", "image/svg+xml"} +# Heart sounds are the reason this exists: a murmur cannot be shown as a picture. +AUDIO_TYPES = {"audio/mpeg", "audio/mp3", "audio/wav", "audio/x-wav", + "audio/ogg", "audio/webm", "audio/mp4", "audio/aac"} +VIDEO_TYPES = {"video/mp4", "video/webm", "video/ogg", "video/quicktime"} +ALLOWED_TYPES = IMAGE_TYPES | AUDIO_TYPES | VIDEO_TYPES + + +def kind_for(content_type: str) -> str: + """Which of the three a file is. The player differs for each.""" + if content_type in AUDIO_TYPES: + return "audio" + if content_type in VIDEO_TYPES: + return "video" + return "image" + + +def limit_for(kind: str) -> int: + return MAX_IMAGE_BYTES if kind == "image" else MAX_MEDIA_BYTES def readable_libraries(db: Session, user: User) -> set[int] | None: @@ -159,7 +181,14 @@ def list_media( total = query.count() assets = query.order_by(MediaAsset.id.desc()).offset(offset).limit(limit).all() tags = _tags_for(db, [a.id for a in assets]) - return {"total": total, "images": [_asset_json(a, tags.get(a.id, [])) for a in assets]} + # How many questions point at each one. Renaming and moving are safe because + # the link is the id; deleting is the one act that cannot be undone, so the + # count travels with the row and is shown before anyone presses delete. + used = dict(db.query(QuestionMedia.media_id, func.count(QuestionMedia.id)).filter( + QuestionMedia.media_id.in_([a.id for a in assets] or [0]) + ).group_by(QuestionMedia.media_id).all()) if assets else {} + return {"total": total, "images": [ + {**_asset_json(a, tags.get(a.id, [])), "used_by": used.get(a.id, 0)} for a in assets]} @router.post("/upload", status_code=201) @@ -180,10 +209,13 @@ def upload_media( raise HTTPException(400, "Choose one of your libraries for this image") if file.content_type not in ALLOWED_TYPES: - raise HTTPException(400, "Upload a PNG, JPEG, GIF, WebP or SVG image") - data = file.file.read(MAX_IMAGE_BYTES + 1) - if len(data) > MAX_IMAGE_BYTES: - raise HTTPException(413, f"Keep images under {MAX_IMAGE_BYTES // (1024 * 1024)} MB") + raise HTTPException(400, "Upload an image (PNG, JPEG, GIF, WebP, SVG), " + "audio (MP3, WAV, OGG, M4A) or video (MP4, WebM, MOV)") + kind = kind_for(file.content_type) + ceiling = limit_for(kind) + data = file.file.read(ceiling + 1) + if len(data) > ceiling: + raise HTTPException(413, f"Keep {kind} under {ceiling // (1024 * 1024)} MB") if not data: raise HTTPException(400, "That file is empty") @@ -193,7 +225,7 @@ def upload_media( asset = MediaAsset( path=key, title=title or file.filename, caption=caption, alt_text=alt_text, - kind="image", library_id=library_id, user_id=current_user.id, + kind=kind, library_id=library_id, user_id=current_user.id, storage="s3" if storage_service.using_s3() else "local", byte_size=len(data), ) db.add(asset) @@ -259,8 +291,19 @@ def update_media(media_id: int, data: MediaUpdate, db: Session = Depends(get_db) @router.delete("/{media_id}", status_code=204) -def delete_media(media_id: int, db: Session = Depends(get_db), +def delete_media(media_id: int, force: bool = False, db: Session = Depends(get_db), current_user: User = Depends(require_moderator)): + """Delete an image. Refused while a question still uses it, unless forced. + + A question refers to a figure by id, so renaming it or moving it between + libraries never breaks anything. Deleting does, and silently — the cascade + would take the link with it and the question would simply stop having a + picture. So the count has to be faced first. + """ + used = db.query(QuestionMedia).filter(QuestionMedia.media_id == media_id).count() + if used and not force: + raise HTTPException(409, f"{used} question{'s' if used > 1 else ''} still use this. " + "Detach it there first, or delete it anyway.") asset = db.get(MediaAsset, media_id) if not asset: raise HTTPException(404, "Image not found") diff --git a/backend/app/services/email_service.py b/backend/app/services/email_service.py index 3ba6d6e..02447a5 100644 --- a/backend/app/services/email_service.py +++ b/backend/app/services/email_service.py @@ -150,25 +150,3 @@ We received a request to reset your password. Click below to choose a new one. await _send(to_email, subject, _wrap(subject, md)) -async def send_reminder_email(email: str, user_name: str, quiz_title: str, score: float, next_date: str): - subject = f"Time to review: {quiz_title}" - note = "Great work — keep the streak going! 🌟" if score >= 75 else "A bit more practice will get you there. 📚" - md = f"""# Time to review - -Hi **{user_name}**, - -Your spaced repetition schedule says it's time to revisit **{quiz_title}**. - -## Last score - -**{score:.0f}%** — {note} - -Reviewing at spaced intervals is one of the most effective ways to retain medical knowledge long-term. Each session strengthens the memory trace. - -[button:Take Quiz Now]({settings.APP_URL}/quizzes) - ---- - -You're receiving this because you have active quiz reminders. To stop, disable reminders in your account settings. -""" - await _send(email, subject, _wrap(subject, md)) diff --git a/backend/app/services/reminder_service.py b/backend/app/services/reminder_service.py deleted file mode 100644 index 977eac9..0000000 --- a/backend/app/services/reminder_service.py +++ /dev/null @@ -1,61 +0,0 @@ -from datetime import datetime, timedelta - -from sqlalchemy.orm import Session - -from app.models.reminder import ReminderSchedule - -# SM-2 simplified intervals in days -INTERVALS = [1, 3, 7, 14, 30] - - -def update_reminder_schedule( - db: Session, - user_id: int, - quiz_id: int, - score_percentage: float, -): - """Update or create a reminder schedule based on quiz performance.""" - reminder = db.query(ReminderSchedule).filter( - ReminderSchedule.user_id == user_id, - ReminderSchedule.quiz_id == quiz_id, - ).first() - - if not reminder: - reminder = ReminderSchedule( - user_id=user_id, - quiz_id=quiz_id, - performance_score=score_percentage, - interval_days=INTERVALS[0], - next_reminder_at=datetime.utcnow() + timedelta(days=INTERVALS[0]), - is_active=True, - ) - db.add(reminder) - else: - reminder.performance_score = score_percentage - reminder.is_active = True - - # Find current interval index - current_idx = 0 - for i, interval in enumerate(INTERVALS): - if reminder.interval_days <= interval: - current_idx = i - break - - if score_percentage < 75: - # Below threshold: reset to shortest interval - new_idx = 0 - elif score_percentage < 90: - # Decent performance: advance one step - new_idx = min(current_idx + 1, len(INTERVALS) - 1) - else: - # Excellent performance: advance or deactivate - if current_idx >= len(INTERVALS) - 1: - reminder.is_active = False - db.commit() - return - new_idx = min(current_idx + 2, len(INTERVALS) - 1) - - reminder.interval_days = INTERVALS[new_idx] - reminder.next_reminder_at = datetime.utcnow() + timedelta(days=INTERVALS[new_idx]) - - db.commit() diff --git a/backend/app/utils/scheduler.py b/backend/app/utils/scheduler.py deleted file mode 100644 index 688e941..0000000 --- a/backend/app/utils/scheduler.py +++ /dev/null @@ -1,98 +0,0 @@ -import asyncio -import logging -from datetime import datetime - -from apscheduler.schedulers.background import BackgroundScheduler - -from app.database import SessionLocal -from app.models.reminder import ReminderSchedule -from app.models.user import User -from app.models.quiz import Quiz -from app.models.attempt import QuizAttempt - -logger = logging.getLogger(__name__) - -scheduler = BackgroundScheduler() - - -def check_and_send_reminders(): - """Check for due reminders and send emails.""" - db = SessionLocal() - try: - due_reminders = db.query(ReminderSchedule).filter( - ReminderSchedule.is_active == True, - ReminderSchedule.next_reminder_at <= datetime.utcnow(), - ).all() - - if not due_reminders: - return - - logger.info(f"Found {len(due_reminders)} due reminders") - - for reminder in due_reminders: - user = db.query(User).filter(User.id == reminder.user_id).first() - quiz = db.query(Quiz).filter(Quiz.id == reminder.quiz_id).first() - - if not user or not quiz or quiz.deleted_at or quiz.course_id: - reminder.is_active = False - continue - - # Deactivate if the user has no completed attempts left for this quiz - # (e.g., they deleted their attempts — spaced repetition no longer applies). - has_completed_attempt = db.query(QuizAttempt.id).filter( - QuizAttempt.user_id == user.id, - QuizAttempt.quiz_id == quiz.id, - QuizAttempt.completed_at.isnot(None), - ).first() is not None - if not has_completed_attempt: - reminder.is_active = False - continue - - # Respect the user's opt-out preference (canonical source: Postgres). - if user.reminders_disabled: - continue - - # Send email asynchronously - try: - from app.services.email_service import send_reminder_email - loop = asyncio.new_event_loop() - loop.run_until_complete( - send_reminder_email( - email=user.email, - user_name=user.name, - quiz_title=quiz.title, - score=reminder.performance_score, - next_date=reminder.next_reminder_at.strftime("%Y-%m-%d"), - ) - ) - loop.close() - except Exception as e: - logger.error(f"Failed to send reminder {reminder.id}: {e}") - - # Schedule next reminder - from datetime import timedelta - reminder.next_reminder_at = datetime.utcnow() + timedelta(days=reminder.interval_days) - - db.commit() - except Exception as e: - logger.exception(f"Scheduler error: {e}") - finally: - db.close() - - -def start_scheduler(): - """Start the APScheduler with daily reminder check.""" - scheduler.add_job( - check_and_send_reminders, - "interval", - hours=24, - id="reminder_check", - replace_existing=True, - ) - scheduler.start() - logger.info("Scheduler started — reminder check runs every 24 hours") - - -def stop_scheduler(): - if scheduler.running: - scheduler.shutdown() diff --git a/backend/scripts/link_articles.py b/backend/scripts/link_articles.py new file mode 100644 index 0000000..19b25fa --- /dev/null +++ b/backend/scripts/link_articles.py @@ -0,0 +1,116 @@ +"""Cross-reference the articles to each other. + +The marker system exists — `[[7|Febrile seizures]]` resolves by id, survives a +rename, and shows a preview on hover — and not one article used it. Every article +was written in isolation, so a piece on croup mentions stridor and epiglottitis +and offers no way to reach either. + +This reads what is already written and links it: where an article's prose names +another article, the first mention becomes a link. By id, so a later rename +cannot break it. + +Deliberately conservative, because a wrong link is worse than a missing one: + + * only the first mention in a section, so prose is not peppered with the same + link five times; + * whole words only, case-insensitively, longest title first — "Otitis media + with effusion" wins over "Otitis media" where both would match; + * never inside an existing link, a marker, a heading, a code span or a table; + * never an article linking to itself; + * titles under four characters are skipped, since short ones collide. + + docker compose exec backend python -m scripts.link_articles + docker compose exec backend python -m scripts.link_articles --apply +""" +import re +import sys + +from app.database import SessionLocal +from app.models.article import Article + +MIN_TITLE = 4 +# Only the body of a section, never a heading, a link, a marker or code. +SKIP = re.compile(r"(\[\[[^\]]*\]\]|\[[^\]]*\]\([^)]*\)|`[^`]*`|^\s{0,3}#{1,6}.*$|^\s*\|.*$)", re.M) + + +def link_text(text: str, titles: list[tuple[str, int, str]], self_id: int) -> tuple[str, int]: + """Link the first mention of each other article. Returns (text, count).""" + if not text: + return text, 0 + + # Carve out everything that must not be touched, link the rest, put it back. + holes: list[str] = [] + + def stash(match: re.Match) -> str: + holes.append(match.group(0)) + return f"\x00{len(holes) - 1}\x00" + + working = SKIP.sub(stash, text) + + linked = 0 + for lowered, article_id, display in titles: + if article_id == self_id: + continue + pattern = re.compile(rf"(?= MIN_TITLE), + key=lambda row: -len(row[0]), + ) + print(f" articles: {len(articles)} linkable titles: {len(titles)}") + + touched = links = 0 + for article in articles: + sections = article.sections or [] + changed = False + for section in sections: + body, count = link_text(section.get("content"), titles, article.id) + if count: + section["content"] = body + links += count + changed = True + summary, count = link_text(article.summary, titles, article.id) + if count: + article.summary = summary + links += count + changed = True + if changed: + touched += 1 + if apply_changes: + from sqlalchemy.orm.attributes import flag_modified + + article.sections = sections + flag_modified(article, "sections") + + print(f" articles gaining links: {touched}") + print(f" links added : {links}") + if not apply_changes: + print("\n Re-run with --apply.") + return 0 + db.commit() + print("\n Linked by id, so renaming an article cannot break them.") + finally: + db.close() + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/backend/scripts/retitle_figures.py b/backend/scripts/retitle_figures.py index d90ef5c..9174bed 100644 --- a/backend/scripts/retitle_figures.py +++ b/backend/scripts/retitle_figures.py @@ -75,7 +75,29 @@ def main(): for asset, _old, new in changed: asset.title = new[:300] + db.flush() + + # A legend is what a reader sees in an explanation, so twelve figures all + # called "Pediatric Surgery" is twelve legends that say nothing. Only the + # repeats are numbered; a subject used once stays clean. + from collections import defaultdict + + by_title = defaultdict(list) + for asset in db.query(MediaAsset).order_by(MediaAsset.id).all(): + by_title[asset.title].append(asset) + questions = dict(db.execute(sa_text( + "SELECT media_id, MIN(question_id) FROM question_media GROUP BY media_id")).fetchall()) + numbered = 0 + for title, assets in by_title.items(): + if len(assets) < 2: + continue + for asset in assets: + question_id = questions.get(asset.id) + asset.title = (f"{title} · Q{question_id}" if question_id + else f"{title} · #{asset.id}")[:300] + numbered += 1 db.commit() + print(f" disambiguated: {numbered} repeated subjects") print(f"\n retitled: {len(changed)} (no link changed; the id is the link)") finally: db.close() diff --git a/docs/TODO.md b/docs/TODO.md index 06c9f23..386a9a5 100644 --- a/docs/TODO.md +++ b/docs/TODO.md @@ -36,11 +36,31 @@ 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. -### Settings +### Settings and access - [ ] **Revamp the settings page** — called out as the worst screen in the app. Work out what belongs there at all before restyling it: account, theme, exam objective, voice, integrations, data. Several of those have drifted in because there was nowhere else to put them. +- [ ] **Access as a tree** — grant a person articles, an image library, a + question folder, or a category branch, at any depth, with "everything" as + one option rather than a checkbox per row. Three grant tables already + exist (`category_grants`, `media_library_grants`, question folders); they + need one way in rather than three screens, and inheritance down the tree + so granting a branch grants what is under it. + +### Media +- [x] **Audio and video** — a murmur cannot be shown as a picture. Uploads take + MP3/WAV/OGG/M4A and MP4/WebM/MOV alongside images, with their own 60 MB + ceiling, and each renders as what it is. +- [x] **Editing opens over the grid**, not inside a cell that stretches its + column and shoves the neighbours out of line. +- [x] **Every figure named by its subject**, with the question number appended + only where a subject repeats — a legend a reader sees in an explanation + should not be twelve identical words. +- [x] **Deleting a figure in use is refused** unless forced, with the count + shown. Renaming and moving are safe because the link is the id. +- [ ] **Library filter in the picker** is in; a library *rail* in the picker + would be better on a wide screen. ### Editor and figures - [x] **Rich editing on the question page** — no new platform needed: Milkdown diff --git a/frontend/src/components/ArticleReader.jsx b/frontend/src/components/ArticleReader.jsx index 0dabe50..02ac6b2 100644 --- a/frontend/src/components/ArticleReader.jsx +++ b/frontend/src/components/ArticleReader.jsx @@ -1,37 +1,14 @@ import { useEffect, useRef, useState } from 'react' -import ReactMarkdown from 'react-markdown' -import remarkGfm from 'remark-gfm' -import ArticleLink from './ArticleLink' -import { markdownImageUrl } from '../utils/uploads' - -// [[febrile-seizures]] and [[Febrile seizures|febrile-seizures]] are how an -// educator writes a cross-reference without having to know an article's numeric -// id, which changes nothing for them and everything for a link that has to last. -const WIKI_LINK = /\[\[([^\]|]+?)(?:\|([a-z0-9-]+))?\]\]/g -const expandWikiLinks = (text) => (text || '').replace(WIKI_LINK, (_m, label, slug) => - `[${label.trim()}](/articles/${(slug || label).trim().toLowerCase()})`) - -const internalSlug = (href) => { - const match = /^\/articles\/(?:s\/)?([a-z0-9-]+)\/?$/.exec(href || '') - return match ? match[1] : null -} - +import RichText from './RichText' +/** + * Article prose goes through the same renderer as everything else. + * + * This was a second Markdown pipeline with its own cross-reference regex, and it + * only understood the old slug form — so `[[292|Febrile seizures]]`, which is + * what the linker writes, printed as literal brackets. + */ export function Markdown({ children, attemptId }) { - // Educator content renders as Markdown only; raw HTML is escaped, not executed. - return ( - , - a: ({ node, href, children: kids, ...props }) => { - const slug = internalSlug(href) - // A link into the library stays in the app and shows what it leads to; - // only links off the site get a new tab. - if (slug) return {kids} - return {kids} - }, - }}> - {expandWikiLinks(children)} - - ) + return } /** @@ -76,6 +53,16 @@ export default function ArticleReader({ article, activeSection = '', onOpenSecti const present = VIEWS.filter(v => everySection.some(sec => sec.variant === v.key)) const [view, setView] = useState(present[0]?.key || 'long') const allSections = everySection.filter(sec => sec.variant === view) + + // One section is not a contents page. Short and Clinical are usually a single + // section each, so collapsing them by default showed a heading and a blank + // space — which read as the view having no content at all. + useEffect(() => { + const tops = allSections.filter(sec => !sec.parent_id) + if (tops.length === 1) { + setOpenIds(Object.fromEntries(allSections.map(sec => [sec.id, true]))) + } + }, [view, article]) // Element ids are prefixed because two readers can share a page, and a // duplicate id would point the pane's contents at the article behind it. const sectionElement = (secId) => root.current?.querySelector(`#section-${idPrefix}${secId}`) diff --git a/frontend/src/components/ImagePicker.css b/frontend/src/components/ImagePicker.css index b086a63..bcb552e 100644 --- a/frontend/src/components/ImagePicker.css +++ b/frontend/src/components/ImagePicker.css @@ -53,3 +53,12 @@ .ip-panel { max-height: 92vh; border-radius: 14px 14px 0 0; } .ip-grid { grid-template-columns: repeat(auto-fill, minmax(140px, 1fr)); } } + +.ip-library { + min-height: 40px; padding: 8px 11px; cursor: pointer; + border: 1px solid var(--border); border-radius: 9px; + background: var(--input-bg); color: var(--text); font: inherit; font-size: 0.85rem; +} +/* Audio has no thumbnail to speak of; it gets the row rather than a square. */ +.ip-thumb audio, .media-thumb audio { width: 100%; } +.ip-thumb video, .media-thumb video { max-width: 100%; max-height: 100%; } diff --git a/frontend/src/components/ImagePicker.jsx b/frontend/src/components/ImagePicker.jsx index 4d0f485..6068d7e 100644 --- a/frontend/src/components/ImagePicker.jsx +++ b/frontend/src/components/ImagePicker.jsx @@ -1,6 +1,6 @@ import { useCallback, useEffect, useRef, useState } from 'react' import api from '../api/client' -import { uploadUrl } from '../utils/uploads' +import MediaTile from './MediaTile' import './ImagePicker.css' /** @@ -16,6 +16,10 @@ import './ImagePicker.css' export default function ImagePicker({ open, onPick, onClose, title = 'Choose an image' }) { const [query, setQuery] = useState('') const [images, setImages] = useState([]) + const [libraries, setLibraries] = useState([]) + // Narrowing by library is how you find a figure when the bank holds hundreds: + // "the radiology one" is a far better first cut than a search term. + const [library, setLibrary] = useState('all') const [loading, setLoading] = useState(false) const [error, setError] = useState('') const [busy, setBusy] = useState(false) @@ -23,11 +27,14 @@ export default function ImagePicker({ open, onPick, onClose, title = 'Choose an const load = useCallback(() => { setLoading(true) - api.get('/media/', { params: query.trim() ? { q: query.trim(), limit: 40 } : { limit: 40 } }) + const params = { limit: 40 } + if (query.trim()) params.q = query.trim() + if (library !== 'all') params.library_id = library + api.get('/media/', { params }) .then(res => setImages(res.data?.images || [])) .catch(() => setError('Could not load the image bank')) .finally(() => setLoading(false)) - }, [query]) + }, [query, library]) useEffect(() => { if (!open) return @@ -35,6 +42,11 @@ export default function ImagePicker({ open, onPick, onClose, title = 'Choose an return () => clearTimeout(timer) }, [open, load, query]) + useEffect(() => { + if (!open || libraries.length) return + api.get('/media/libraries').then(res => setLibraries(res.data || [])).catch(() => setLibraries([])) + }, [open, libraries.length]) + useEffect(() => { if (!open) return const onKey = (event) => { if (event.key === 'Escape') onClose() } @@ -73,8 +85,17 @@ export default function ImagePicker({ open, onPick, onClose, title = 'Choose an
setQuery(e.target.value)} - placeholder="Search by what the image shows" aria-label="Search images" /> - + placeholder="Search by what it shows" aria-label="Search images" /> + + {libraries.length > 0 && ( + + )}
@@ -90,7 +111,7 @@ export default function ImagePicker({ open, onPick, onClose, title = 'Choose an
  • - deleteAttempt(a.attempt_id)} - /> - - - ))} - - - ) -} diff --git a/frontend/src/components/MediaTile.jsx b/frontend/src/components/MediaTile.jsx new file mode 100644 index 0000000..a30563d --- /dev/null +++ b/frontend/src/components/MediaTile.jsx @@ -0,0 +1,19 @@ +import { uploadUrl } from '../utils/uploads' + +/** + * One item from the bank, drawn as whatever it is. + * + * Heart sounds are the reason this exists: an audio file rendered as an + * is a broken icon, and a video rendered as one is the same. The kind decides + * the element; everything else about the tile stays identical. + */ +export default function MediaTile({ item, className = '', controls = true }) { + const src = uploadUrl(item.path) + if (item.kind === 'audio') { + return
  • - {image.alt_text + {/* The id is what you type into a question, so it has to be readable without opening anything. */} #{image.id} @@ -209,8 +208,18 @@ export default function MediaPage() { {confirmDelete === image.id && (
    - Delete image #{image.id}? Questions using it will lose their picture. - + {/* Renaming and moving are safe — a question refers to a + figure by id. Deleting is the one act that breaks the + link, so the count is faced before the button. */} + + {image.used_by > 0 + ? `${image.used_by} question${image.used_by > 1 ? 's' : ''} use this. Deleting takes the figure off ${image.used_by > 1 ? 'them' : 'it'}.` + : 'Nothing uses this. Delete it?'} + +
    )} @@ -236,7 +245,7 @@ export default function MediaPage() {
    - {image.alt_text +
    diff --git a/frontend/src/pages/MediaPage.test.jsx b/frontend/src/pages/MediaPage.test.jsx index b993a87..69a2931 100644 --- a/frontend/src/pages/MediaPage.test.jsx +++ b/frontend/src/pages/MediaPage.test.jsx @@ -74,14 +74,24 @@ describe('image bank', () => { await waitFor(() => expect(api.get).toHaveBeenCalledWith('/media/', { params: { library_id: 1 } })) }) - it('warns what a deletion costs before doing it', async () => { + it('counts what a deletion costs before doing it', async () => { + mockApi([{ ...images[0], used_by: 3 }, { ...images[1], used_by: 0 }]) mount() await screen.findByText('#11') api.delete.mockResolvedValue({}) + + // Renaming and moving are safe — a question refers to a figure by id. + // Deleting is the one act that breaks the link, so the count is faced first. await userEvent.click(screen.getByRole('button', { name: 'Delete image 11' })) - expect(screen.getByRole('alert')).toHaveTextContent('Questions using it will lose their picture') + expect(screen.getByRole('alert')).toHaveTextContent('3 questions use this') + await userEvent.click(screen.getByRole('button', { name: 'Delete anyway' })) + await waitFor(() => expect(api.delete).toHaveBeenCalledWith('/media/11', { params: { force: true } })) + + // Nothing pointing at it needs no forcing. + await userEvent.click(screen.getByRole('button', { name: 'Delete image 12' })) + expect(screen.getByRole('alert')).toHaveTextContent('Nothing uses this') await userEvent.click(screen.getByRole('button', { name: 'Delete' })) - await waitFor(() => expect(api.delete).toHaveBeenCalledWith('/media/11')) + await waitFor(() => expect(api.delete).toHaveBeenCalledWith('/media/12', { params: {} })) }) it('keeps library creation and deletion to moderators', async () => {