From 025e5bb4ac06d454ea94acd3c556b5195e7063ec Mon Sep 17 00:00:00 2001 From: Daniel Date: Thu, 10 Sep 2026 17:13:07 +0200 Subject: [PATCH] feat: article CMS, three reading views, and articles written from the library MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Standardises cross-references the way we agreed, and puts a CMS around articles so hundreds of generated drafts are reviewable rather than merely present. Links, made rename-proof `[[7|Febrile seizures]]` resolves by id and displays the text — the id is the part that must not change, the text is what keeps prose readable while you write it. `[[old-slug]]` still resolves and is rewritten to the id form on save, not in a migration: an article nobody has touched is not broken, and rewriting prose no one asked to change is how an editor stops trusting the editor. Every slug an article has ever had is kept, so a rename redirects instead of 404ing, and a save reports markers pointing at nothing — at the moment the person who wrote the link is still looking at it. Three views of one topic The full article to study from, the key points to revise from, the clinical view to act from, with doses. They are views of one article rather than three articles, so the numbers cannot drift apart and a question linked to the topic still means one thing. Each section carries its variant; articles written before this are the long view, unchanged. CMS draft → in review → published, with an author able to submit and only a moderator able to publish. Every save snapshots what was there, restorable, and restoring is itself snapshotted or the way back from a mistaken restore is gone. The editorial queue is work rather than inventory: waiting for review, generated and unread, published without sources, published with nothing to practise, barely written. An empty bucket is drawn as good news, not as an alert. Articles from the clinical library The library index is 1.8M chunks of reference texts embedded with bge-m3 — the same model PedsHub already uses, so our query vectors are directly comparable and nothing had to be re-indexed. Retrieval supplies the facts and the provenance; the model supplies the prose. References are built from the metadata of the passages actually retrieved, never from the model, so a reference cannot be invented — the same property that makes an AI Mode citation trustworthy. A topic with fewer than three grounding passages is skipped rather than written from memory. Everything lands as a draft. Two things worth naming. The generated text is original writing grounded in those books, not extracts from them: their facts are usable, their sentences are their publishers'. And there are two Milvus servers on this host — the collection with the data is the one reached as `milvus`, not the similarly named one on the other stack, which I wired up first and which silently refused. Also fixed along the way: `litellm==1.28.13` has been withdrawn from PyPI, so requirements.txt could no longer be resolved from scratch and the image only built because of a cached layer. Later additions go in their own layer until the pins are refreshed. 182 backend, 223 frontend green. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01TqXevQJhxFrM7jJg82cgZN --- .../versions/b6c7d8e9f0a1_article_cms.py | 90 ++++ backend/app/config.py | 8 + backend/app/models/article.py | 50 +- backend/app/routers/articles.py | 183 +++++++- backend/app/services/article_service.py | 162 +++++++ backend/app/services/article_writer.py | 190 ++++++++ backend/app/services/clinical_library.py | 173 +++++++ backend/scripts/fix_lab_formatting.py | 429 ++++++++++++++++++ backend/scripts/generate_articles.py | 100 ++++ docker-compose.yml | 9 + frontend/src/App.jsx | 2 + frontend/src/components/ArticleLink.css | 25 +- frontend/src/components/ArticleLink.jsx | 82 +++- frontend/src/components/ArticleLink.test.jsx | 104 ++++- frontend/src/components/ArticleReader.jsx | 242 ++++++++++ frontend/src/components/ArticleSplitPane.css | 89 ++++ frontend/src/components/ArticleSplitPane.jsx | 58 +++ frontend/src/components/Navbar.jsx | 3 +- frontend/src/context/SplitViewContext.jsx | 19 + frontend/src/pages/ArticleSplitView.test.jsx | 118 +++++ frontend/src/pages/ArticlesPage.css | 23 + frontend/src/pages/ArticlesPage.jsx | 211 ++------- frontend/src/pages/EditorialPage.css | 61 +++ frontend/src/pages/EditorialPage.jsx | 135 ++++++ frontend/src/pages/EditorialPage.test.jsx | 84 ++++ 25 files changed, 2446 insertions(+), 204 deletions(-) create mode 100644 backend/alembic/versions/b6c7d8e9f0a1_article_cms.py create mode 100644 backend/app/services/article_service.py create mode 100644 backend/app/services/article_writer.py create mode 100644 backend/app/services/clinical_library.py create mode 100644 backend/scripts/fix_lab_formatting.py create mode 100644 backend/scripts/generate_articles.py create mode 100644 frontend/src/components/ArticleReader.jsx create mode 100644 frontend/src/components/ArticleSplitPane.css create mode 100644 frontend/src/components/ArticleSplitPane.jsx create mode 100644 frontend/src/context/SplitViewContext.jsx create mode 100644 frontend/src/pages/ArticleSplitView.test.jsx create mode 100644 frontend/src/pages/EditorialPage.css create mode 100644 frontend/src/pages/EditorialPage.jsx create mode 100644 frontend/src/pages/EditorialPage.test.jsx diff --git a/backend/alembic/versions/b6c7d8e9f0a1_article_cms.py b/backend/alembic/versions/b6c7d8e9f0a1_article_cms.py new file mode 100644 index 0000000..e256aca --- /dev/null +++ b/backend/alembic/versions/b6c7d8e9f0a1_article_cms.py @@ -0,0 +1,90 @@ +"""Article CMS: review workflow, revisions, slug history, references. + +Revision ID: b6c7d8e9f0a1 +Revises: a5b6c7d8e9f0 +""" +import sqlalchemy as sa +from alembic import op +from sqlalchemy import inspect + +revision = "b6c7d8e9f0a1" +down_revision = "a5b6c7d8e9f0" +branch_labels = None +depends_on = None + + +def _has_table(name: str) -> bool: + return name in inspect(op.get_bind()).get_table_names() + + +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(): + # `Base.metadata.create_all()` runs at startup as a fallback for fresh + # deploys, so on a running box it will already have created whatever the + # models describe. This migration is the record of the change and must apply + # cleanly either way, so every step checks first. + # Review sits between draft and published: generated medical writing that + # nobody has read is exactly what must not reach a learner. + for column in ( + sa.Column("submitted_at", sa.DateTime, nullable=True), + sa.Column("reviewed_at", sa.DateTime, nullable=True), + sa.Column("reviewed_by", sa.Integer, + sa.ForeignKey("users.id", ondelete="SET NULL"), nullable=True), + # Where the facts came from. A list of sources, not in-text markers. + sa.Column("references_json", sa.JSON, nullable=True), + # What produced it, so machine-written drafts stay visibly distinct from + # an educator's own writing for as long as they need to be. + sa.Column("generated_by", sa.String(80), nullable=True), + sa.Column("generated_at", sa.DateTime, nullable=True), + ): + if not _has_column("articles", column.name): + op.add_column("articles", column) + + if not _has_table("article_revisions"): + op.create_table( + "article_revisions", + sa.Column("id", sa.Integer, primary_key=True), + sa.Column("article_id", sa.Integer, + sa.ForeignKey("articles.id", ondelete="CASCADE"), nullable=False, index=True), + sa.Column("title", sa.String(300), nullable=False), + sa.Column("summary", sa.Text, nullable=True), + sa.Column("content", sa.Text, nullable=True), + sa.Column("sections", sa.JSON, nullable=False), + sa.Column("references_json", sa.JSON, nullable=True), + sa.Column("status", sa.String(20), nullable=True), + sa.Column("note", sa.String(200), nullable=True), + sa.Column("created_by", sa.Integer, + sa.ForeignKey("users.id", ondelete="SET NULL"), nullable=True), + sa.Column("created_at", sa.DateTime, server_default=sa.func.now()), + ) + + # Every slug an article has ever had, so a rename cannot orphan the + # cross-references pointing at the old one. + if not _has_table("article_slugs"): + op.create_table( + "article_slugs", + sa.Column("id", sa.Integer, primary_key=True), + sa.Column("slug", sa.String(120), nullable=False, unique=True, index=True), + sa.Column("article_id", sa.Integer, + sa.ForeignKey("articles.id", ondelete="CASCADE"), nullable=False, index=True), + sa.Column("created_at", sa.DateTime, server_default=sa.func.now()), + ) + # Seed it with what every article is called today, or the history starts + # empty and the first rename still breaks the links it should have caught. + op.execute(""" + INSERT INTO article_slugs (slug, article_id) + SELECT slug, id FROM articles + ON CONFLICT (slug) DO NOTHING + """) + + +def downgrade(): + op.drop_table("article_slugs") + op.drop_table("article_revisions") + for column in ("generated_at", "generated_by", "references_json", + "reviewed_by", "reviewed_at", "submitted_at"): + if _has_column("articles", column): + op.drop_column("articles", column) diff --git a/backend/app/config.py b/backend/app/config.py index 3b090e5..4b9fd7d 100644 --- a/backend/app/config.py +++ b/backend/app/config.py @@ -71,4 +71,12 @@ class Settings(BaseSettings): LOG_LEVEL: str = "INFO" # DEBUG, INFO, WARNING, ERROR + # Clinical library index (Milvus on the ped-ai stack), read-only. Articles + # are grounded in what it retrieves; nothing here writes to it. + CLINICAL_MILVUS_URI: str = "" + CLINICAL_MILVUS_TOKEN: str = "" + CLINICAL_MILVUS_COLLECTION: str = "mcp_bge_m3_1024" + + settings = Settings() + diff --git a/backend/app/models/article.py b/backend/app/models/article.py index 954a4b9..8011f2b 100644 --- a/backend/app/models/article.py +++ b/backend/app/models/article.py @@ -22,7 +22,16 @@ class Article(Base, Embeddable): category_id = Column(Integer, ForeignKey("question_categories.id", ondelete="SET NULL"), nullable=True) section_id = Column(Integer, ForeignKey("sections.id", ondelete="SET NULL"), nullable=True) # Optional PDF source range. user_id = Column(Integer, ForeignKey("users.id", ondelete="SET NULL"), nullable=True) - status = Column(String, default="draft") # draft | published + status = Column(String, default="draft") # draft | in_review | published + submitted_at = Column(DateTime, nullable=True) + reviewed_at = Column(DateTime, nullable=True) + reviewed_by = Column(Integer, ForeignKey("users.id", ondelete="SET NULL"), nullable=True) + # Sources the article was written from. A list, not in-text markers. + references_json = Column(JSON, nullable=True) + # What wrote it, so a machine-written draft stays visibly one until an + # educator has been through it. + generated_by = Column(String(80), nullable=True) + generated_at = Column(DateTime, nullable=True) created_at = Column(DateTime, default=datetime.utcnow) updated_at = Column(DateTime, default=datetime.utcnow, onupdate=datetime.utcnow) @@ -46,6 +55,45 @@ class QuestionArticleLink(Base): article = relationship("Article", back_populates="links") +class ArticleRevision(Base): + """A snapshot of an article as it was before a save. + + Kept for the same reason question versions are: an editor who breaks + something at 2am needs to get back to what was there, and a diff nobody can + reach is not a safety net. + """ + + __tablename__ = "article_revisions" + + id = Column(Integer, primary_key=True, index=True) + article_id = Column(Integer, ForeignKey("articles.id", ondelete="CASCADE"), nullable=False, index=True) + title = Column(String(300), nullable=False) + summary = Column(Text, nullable=True) + content = Column(Text, nullable=True) + sections = Column(JSON, nullable=False, default=list) + references_json = Column(JSON, nullable=True) + status = Column(String(20), nullable=True) + note = Column(String(200), nullable=True) + created_by = Column(Integer, ForeignKey("users.id", ondelete="SET NULL"), nullable=True) + created_at = Column(DateTime, default=datetime.utcnow) + + +class ArticleSlug(Base): + """Every slug an article has ever had. + + A cross-reference written today must survive a rename tomorrow. Old slugs + redirect rather than 404, so the author who renamed something does not + silently break every article that pointed at it. + """ + + __tablename__ = "article_slugs" + + id = Column(Integer, primary_key=True, index=True) + slug = Column(String(120), unique=True, nullable=False, index=True) + article_id = Column(Integer, ForeignKey("articles.id", ondelete="CASCADE"), nullable=False, index=True) + created_at = Column(DateTime, default=datetime.utcnow) + + class ArticleView(Base): """Last time a learner opened an article — one row per user and article.""" diff --git a/backend/app/routers/articles.py b/backend/app/routers/articles.py index 796def9..6637ade 100644 --- a/backend/app/routers/articles.py +++ b/backend/app/routers/articles.py @@ -8,7 +8,11 @@ from sqlalchemy.orm import Session from app.database import get_db from app.services.search_service import hybrid_ids from app.services import embedding_service -from app.models.article import Article, ArticleSectionIndex, ArticleView, QuestionArticleLink +from app.models.article import ( + Article, ArticleRevision, ArticleSectionIndex, ArticleSlug, ArticleView, + QuestionArticleLink, +) +from app.services import article_service from app.models.flashcard import Flashcard, FlashcardDeck, FlashcardArticleLink from app.models.question import Question from app.models.section import Section @@ -36,6 +40,9 @@ class ArticleSection(BaseModel): # belongs to "Review of systems" rather than standing alongside it. Absent # or null means top level, so every article written before this stays valid. parent_id: str | None = None + # Which of the three readings this section belongs to: the full article, the + # key points, or the clinical view carrying management and doses. + variant: str = "long" class ArticleWrite(BaseModel): @@ -121,6 +128,10 @@ def _validate_sections(sections: list[ArticleSection]): ids.add(section.id) slugs.add(section.slug.strip()) + for section in sections: + if section.variant not in article_service.VARIANTS: + raise HTTPException(400, f"Unknown section variant: {section.variant}") + # Nesting is one level deep and points backwards: a parent has to be a # section of this article that was already listed, which rules out a cycle # and a sub-section that renders before the heading it belongs to. @@ -130,6 +141,11 @@ def _validate_sections(sections: list[ArticleSection]): if section.parent_id is not None: if section.parent_id == section.id or section.parent_id not in seen: raise HTTPException(400, "A sub-section must sit under an earlier section of this article") + parent = next(s for s in sections if s.id == section.parent_id) + # A sub-section belongs to the same reading as its parent, or the + # contents rail would list a child that its view never renders. + if parent.variant != section.variant: + raise HTTPException(400, "A sub-section must be in the same view as the section it sits under") parents.add(section.parent_id) seen.add(section.id) nested = {s.id for s in sections if s.parent_id is not None} @@ -158,6 +174,11 @@ def _article_json(article: Article) -> dict: "section_id": article.section_id, "user_id": article.user_id, "status": article.status, + "references": article.references_json or [], + "variants": article_service.available_variants(article), + "generated_by": article.generated_by, + "reviewed_at": article.reviewed_at, + "submitted_at": article.submitted_at, "created_at": article.created_at, "updated_at": article.updated_at, } @@ -273,6 +294,8 @@ def create_article( user_id=current_user.id, status="draft", ) db.add(article) + db.flush() + article_service.record_slug(db, article) db.commit() db.refresh(article) _reembed(db, article) @@ -336,6 +359,20 @@ def _plain_excerpt(article: Article, limit: int = 260) -> str: return text[:limit].rstrip() + "…" if len(text) > limit else text +@router.get("/by-slug/{slug}") +def article_by_slug(slug: str, db: Session = Depends(get_db), + current_user: User = Depends(get_current_user)): + """Resolve a slug — current or historical — to the article it names.""" + article = article_service.resolve_slug(db, slug) + if not article: + raise HTTPException(404, "Article not found") + if article.status != "published" and not current_user.is_moderator and article.user_id != current_user.id: + raise HTTPException(404, "Article not found") + # `moved` tells the caller to correct its URL rather than keep using the old one. + return {"id": article.id, "slug": article.slug, "title": article.title, + "moved": article.slug != slug.strip().lower()} + + @router.get("/preview/{slug}") def preview_article( slug: str, @@ -347,7 +384,7 @@ def preview_article( Deliberately small: a link preview that fetched whole articles would pull down the library a paragraph at a time as somebody reads. """ - article = db.query(Article).filter(Article.slug == slug.lower()).first() + article = article_service.resolve_slug(db, slug) if not article: raise HTTPException(404, "Article not found") if article.status != "published" and not current_user.is_moderator and article.user_id != current_user.id: @@ -450,9 +487,18 @@ def update_article( _validate_source_section(db, data.section_id) if db.query(Article.id).filter(Article.slug == data.slug, Article.id != article.id).first(): raise HTTPException(400, "Slug is already in use") - article.slug, article.title, article.summary, article.content = data.slug, data.title, data.summary, data.content - article.sections = [section.model_dump() for section in data.sections] + # Keep what it was before it becomes something else. + article_service.snapshot(db, article, current_user.id) + + article.slug, article.title, article.summary = data.slug, data.title, data.summary + article.content = article_service.upgrade_markers(db, data.content) + article.sections = article_service.normalise_sections([ + {**section.model_dump(), + "content": article_service.upgrade_markers(db, section.content)} + for section in data.sections + ]) article.category_id, article.section_id = data.category_id, data.section_id + article_service.record_slug(db, article) # Remediate links whose section was removed; whole-article links survive renames. kept = _section_ids(article) db.query(QuestionArticleLink).filter( @@ -468,7 +514,9 @@ def update_article( db.commit() db.refresh(article) _reembed(db, article) - return _article_json(article) + # Told at the moment of saving, which is the last point the person who wrote + # the link is still looking at it. + return {**_article_json(article), "broken_links": article_service.broken_markers(db, article)} @router.delete("/{article_id}", status_code=204) @@ -622,3 +670,128 @@ def get_article_job(job_id: str, current_user: User = Depends(get_current_user)) if status == "failed": result["error"] = r.get(f"extraction:error:{job_id}") or "Unknown error" return result + + +# ── Editorial: review workflow, revisions, and the queue of work ────────────── + +class StatusIn(BaseModel): + status: str + + +@router.post("/{article_id}/status") +def set_article_status(article_id: int, data: StatusIn, db: Session = Depends(get_db), + current_user: User = Depends(get_current_user)): + """Move an article through draft → in review → published. + + An author may send their own work for review; only a moderator may publish, + because publishing is the point at which nobody checks it again. + """ + article = db.get(Article, article_id) + if not article: + raise HTTPException(404, "Article not found") + is_author = article.user_id == current_user.id + if not (current_user.is_moderator or is_author): + raise HTTPException(403, "Not your article") + if data.status == "published" and not current_user.is_moderator: + raise HTTPException(403, "Only a moderator can publish an article") + try: + article_service.set_status(db, article, data.status, current_user.id) + except ValueError as error: + raise HTTPException(400, str(error)) + db.commit() + return {"id": article.id, "status": article.status} + + +@router.get("/{article_id}/revisions") +def list_revisions(article_id: int, db: Session = Depends(get_db), + current_user: User = Depends(get_current_user)): + article = db.get(Article, article_id) + if not article: + raise HTTPException(404, "Article not found") + if not (current_user.is_moderator or article.user_id == current_user.id): + raise HTTPException(403, "Not your article") + rows = db.query(ArticleRevision).filter( + ArticleRevision.article_id == article_id).order_by(ArticleRevision.id.desc()).limit(30).all() + return [{ + "id": r.id, "title": r.title, "status": r.status, "note": r.note, + "created_at": r.created_at, "created_by": r.created_by, + "section_count": len(r.sections or []), + } for r in rows] + + +@router.get("/{article_id}/revisions/{revision_id}") +def read_revision(article_id: int, revision_id: int, db: Session = Depends(get_db), + current_user: User = Depends(get_current_user)): + revision = db.get(ArticleRevision, revision_id) + if not revision or revision.article_id != article_id: + raise HTTPException(404, "Revision not found") + article = db.get(Article, article_id) + if not (current_user.is_moderator or (article and article.user_id == current_user.id)): + raise HTTPException(403, "Not your article") + return {"id": revision.id, "title": revision.title, "summary": revision.summary, + "content": revision.content, "sections": revision.sections, + "references": revision.references_json or [], "created_at": revision.created_at} + + +@router.post("/{article_id}/revisions/{revision_id}/restore") +def restore_revision(article_id: int, revision_id: int, db: Session = Depends(get_db), + current_user: User = Depends(require_moderator)): + """Put an old version back, keeping the current one as a revision of its own. + + Restoring is itself an edit, so it is snapshotted too — otherwise the way + back from a mistaken restore is gone. + """ + revision = db.get(ArticleRevision, revision_id) + article = db.get(Article, article_id) + if not article or not revision or revision.article_id != article_id: + raise HTTPException(404, "Revision not found") + article_service.snapshot(db, article, current_user.id, note=f"before restoring #{revision.id}") + article.title, article.summary, article.content = revision.title, revision.summary, revision.content + article.sections = article_service.normalise_sections(revision.sections or []) + article.references_json = revision.references_json + db.commit() + db.refresh(article) + _reembed(db, article) + return _article_json(article) + + +@router.get("/editorial/queue") +def editorial_queue(db: Session = Depends(get_db), + current_user: User = Depends(require_moderator)): + """What still needs a person: the work, not a list of everything. + + Each bucket is something an editor can act on today. Counting articles by + status alone would say how many exist, which is not a queue. + """ + articles = db.query(Article).all() + linked = {row[0] for row in db.query(QuestionArticleLink.article_id).distinct().all()} + + def row(article): + return {"id": article.id, "slug": article.slug, "title": article.title, + "status": article.status, "generated_by": article.generated_by, + "updated_at": article.updated_at, + "variants": article_service.available_variants(article)} + + awaiting = [row(a) for a in articles if a.status == "in_review"] + machine_drafts = [row(a) for a in articles + if a.status == "draft" and a.generated_by] + no_references = [row(a) for a in articles + if a.status != "draft" and not (a.references_json or [])] + no_questions = [row(a) for a in articles + if a.status == "published" and a.id not in linked] + thin = [row(a) for a in articles if len(a.sections or []) < 2] + + return { + "counts": { + "total": len(articles), + "draft": sum(1 for a in articles if a.status == "draft"), + "in_review": len(awaiting), + "published": sum(1 for a in articles if a.status == "published"), + }, + # Ordered by what blocks a learner soonest. + "awaiting_review": awaiting[:100], + "machine_drafts": machine_drafts[:100], + "published_without_references": no_references[:100], + "published_without_questions": no_questions[:100], + "thin": thin[:100], + } diff --git a/backend/app/services/article_service.py b/backend/app/services/article_service.py new file mode 100644 index 0000000..764d1f2 --- /dev/null +++ b/backend/app/services/article_service.py @@ -0,0 +1,162 @@ +"""Article editing rules: variants, revisions, slugs and cross-reference markers. + +Three things live here because they are decisions rather than plumbing, and each +one has a failure mode worth naming. + +**Variants.** One article is read three ways — the full text, the key points, and +the clinical view that carries management and doses. They are one article rather +than three because they describe one condition: split into three rows they drift +apart, and a question linked to "Bronchiolitis" would have to pick which of the +three it meant. Each section carries the variant it belongs to. + +**Slugs.** A cross-reference written today has to survive a rename tomorrow, so +every slug an article has ever had is kept and old ones redirect. Without this a +rename silently breaks every article pointing at the old name, and the person who +renamed it never finds out. + +**Markers.** `[[7|Febrile seizures]]` resolves by id and displays the text. The +id is what makes it rename-proof; the text is what makes the prose readable while +you are writing it. +""" +import logging +import re +from datetime import datetime + +from sqlalchemy.orm import Session + +from app.models.article import Article, ArticleRevision, ArticleSlug + +logger = logging.getLogger(__name__) + +VARIANTS = ("long", "key_points", "clinical") +DEFAULT_VARIANT = "long" + +# [[7|Febrile seizures]] — id first, because the id is the part that must not +# change. [[febrile-seizures]] is the older slug form and still resolves. +MARKER_RE = re.compile(r"\[\[(?:(\d+)\|([^\]]+)|([a-z0-9][a-z0-9-]*))\]\]") + +STATUSES = ("draft", "in_review", "published") + + +def normalise_sections(sections: list[dict]) -> list[dict]: + """Stamp every section with a variant, defaulting to the full article. + + Articles written before variants existed have no such field; treating them + as the long version is what keeps them rendering unchanged. + """ + out = [] + for section in sections: + row = dict(section) + if row.get("variant") not in VARIANTS: + row["variant"] = DEFAULT_VARIANT + out.append(row) + return out + + +def sections_for(article: Article, variant: str) -> list[dict]: + """The sections of one view, in order.""" + variant = variant if variant in VARIANTS else DEFAULT_VARIANT + return [s for s in normalise_sections(article.sections or []) + if s.get("variant") == variant] + + +def available_variants(article: Article) -> list[str]: + """Which views this article actually has, so the toggle offers no empty one.""" + present = {s.get("variant") for s in normalise_sections(article.sections or [])} + return [v for v in VARIANTS if v in present] + + +def record_slug(db: Session, article: Article) -> None: + """Remember the article's current slug, so it keeps resolving after a rename.""" + existing = db.query(ArticleSlug).filter(ArticleSlug.slug == article.slug).first() + if existing is None: + db.add(ArticleSlug(slug=article.slug, article_id=article.id)) + elif existing.article_id != article.id: + # Another article now owns this name. The newest claim wins, because the + # alternative is a redirect that sends readers to the wrong condition. + existing.article_id = article.id + + +def resolve_slug(db: Session, slug: str) -> Article | None: + """Find an article by its current slug, or by one it used to have.""" + slug = (slug or "").strip().lower() + if not slug: + return None + article = db.query(Article).filter(Article.slug == slug).first() + if article: + return article + historical = db.query(ArticleSlug).filter(ArticleSlug.slug == slug).first() + return db.get(Article, historical.article_id) if historical else None + + +def snapshot(db: Session, article: Article, user_id: int | None, note: str | None = None) -> None: + """Keep what the article was, before it becomes something else.""" + db.add(ArticleRevision( + article_id=article.id, title=article.title, summary=article.summary, + content=article.content, sections=article.sections or [], + references_json=article.references_json, status=article.status, + note=(note or "")[:200] or None, created_by=user_id, + )) + + +def marker_targets(text: str) -> tuple[set[int], set[str]]: + """The article ids and legacy slugs a piece of prose points at.""" + ids, slugs = set(), set() + for match in MARKER_RE.finditer(text or ""): + if match.group(1): + ids.add(int(match.group(1))) + elif match.group(3): + slugs.add(match.group(3)) + return ids, slugs + + +def broken_markers(db: Session, article: Article) -> list[str]: + """Markers in this article that point at nothing. + + Checked when an article is saved, because that is the last moment the person + who wrote the link is still looking at it. A dead cross-reference found a + week later belongs to nobody. + """ + body = " ".join(filter(None, [ + article.content or "", + *[(s.get("content") or "") for s in (article.sections or [])], + ])) + ids, slugs = marker_targets(body) + broken = [] + if ids: + known = {row[0] for row in db.query(Article.id).filter(Article.id.in_(ids)).all()} + broken.extend(f"[[{missing}|…]]" for missing in sorted(ids - known)) + for slug in sorted(slugs): + if resolve_slug(db, slug) is None: + broken.append(f"[[{slug}]]") + return broken + + +def upgrade_markers(db: Session, text: str) -> str: + """Rewrite legacy slug markers to the id form, keeping the title as the label. + + Done on save rather than in a migration: an article nobody has touched is not + broken, and rewriting prose that no one asked to change is how you lose an + author's trust in the editor. + """ + def replace(match: re.Match) -> str: + if match.group(1): + return match.group(0) + slug = match.group(3) + article = resolve_slug(db, slug) + return f"[[{article.id}|{article.title}]]" if article else match.group(0) + + return MARKER_RE.sub(replace, text or "") + + +def set_status(db: Session, article: Article, status: str, user_id: int | None) -> None: + """Move an article through draft → in_review → published.""" + if status not in STATUSES: + raise ValueError(f"Unknown status: {status}") + now = datetime.utcnow() + if status == "in_review" and article.status != "in_review": + article.submitted_at = now + if status == "published" and article.status != "published": + article.reviewed_at = now + article.reviewed_by = user_id + article.status = status diff --git a/backend/app/services/article_writer.py b/backend/app/services/article_writer.py new file mode 100644 index 0000000..0dbe014 --- /dev/null +++ b/backend/app/services/article_writer.py @@ -0,0 +1,190 @@ +"""Write a topic article from what the clinical library returns. + +The division of labour is deliberate and matches AI Mode: **retrieval supplies +the facts and the provenance; the model supplies the prose.** References are +built from the metadata of the passages that were actually retrieved, never from +the model, so a reference cannot be invented — the same property that makes an +AI Mode citation trustworthy. + +What is generated is an original piece of writing grounded in the library, not +an extract from it. A textbook's sentences belong to its publisher; its facts do +not, and an article that reproduced the sentences would be redistribution +whatever produced the copy. The prompt says this plainly, and the length limits +make a long verbatim passage impossible to hide. + +Three views come out of one call rather than three, because they are three +readings of one topic and generating them separately lets them contradict each +other on the numbers. +""" +import json +import logging +import re +import uuid + +from sqlalchemy.orm import Session + +from app.models.article import Article +from app.services import article_service, clinical_library +from app.services.ai_service import _call_model, get_model_for_task + +logger = logging.getLogger(__name__) + +PASSAGES = 10 +# Long enough to be worth reading, short enough that nobody skims past the point. +LONG_WORDS = 550 +SHELF = "Pediatrics" + + +def slugify(name: str) -> str: + slug = re.sub(r"[^a-z0-9]+", "-", (name or "").lower()).strip("-") + return slug[:120] or "topic" + + +def _prompt(topic: str, passages: list[dict]) -> str: + sources = "\n\n".join( + f"[{i + 1}] {p['source'].get('title', 'source')}" + f"{', p. ' + str(p['source']['page']) if p['source'].get('page') else ''}\n{p['text']}" + for i, p in enumerate(passages) + ) + return f"""You are writing a study article about **{topic}** for a paediatric exam-revision platform. + +Write it from the reference passages below. They are extracts from standard +textbooks and guidelines. Use them for the facts, the numbers and the structure — +then write the article in your own words. Do not reproduce sentences from the +passages: this is an article about the topic, not an extract from a book. + +If the passages do not cover something, leave it out rather than filling the gap +from memory. A gap a reader can see is safer than a confident sentence nobody +can check. + +Produce three views of the same topic: + +1. "long" — the full article, about {LONG_WORDS} words, in 4 to 7 sections. + Typical headings: Definition, Epidemiology, Etiology, Pathophysiology, + Clinical features, Diagnostics, Differential diagnosis, Treatment, + Complications, Prevention. Use only the ones the passages support. +2. "key_points" — one section titled "Key points", 6 to 10 short bullets a + learner could revise from the night before an exam. +3. "clinical" — one or two sections covering management at the bedside: + what to do, in what order, with drug doses and routes where the passages give + them. Include units and per-kilogram dosing exactly as stated. If a dose is + not in the passages, do not state one. + +Return ONLY valid JSON, no markdown fence: + +{{ + "summary": "one or two sentences, no heading", + "long": [{{"title": "Definition", "content": "markdown"}}], + "key_points": [{{"title": "Key points", "content": "- bullet\\n- bullet"}}], + "clinical": [{{"title": "Management", "content": "markdown"}}] +}} + +Markdown may use lists, bold and tables. Do not write citation markers, footnote +numbers or URLs anywhere: the reference list is attached separately. + +REFERENCE PASSAGES + +{sources}""" + + +def _sections(blocks: list[dict], variant: str) -> list[dict]: + """Turn the model's blocks into stable sections of one view.""" + out = [] + for block in blocks or []: + # The model occasionally returns a bare string where a block was asked + # for. Skipping it costs one section; letting it through ends the run. + if not isinstance(block, dict): + continue + title = str(block.get("title") or "").strip() + content = str(block.get("content") or "").strip() + if not title or not content: + continue + out.append({ + # A fresh id per section, because links are made against these and a + # regenerated article must not silently inherit another's links. + "id": uuid.uuid4().hex, + "slug": slugify(title)[:60] or f"section-{len(out) + 1}", + "title": title[:300], + "content": content, + "parent_id": None, + "variant": variant, + }) + return out + + +def _unique_slugs(sections: list[dict]) -> list[dict]: + """Slugs are unique within an article, and three views repeat titles.""" + seen: set[str] = set() + for section in sections: + base = section["slug"] + candidate, n = base, 2 + while candidate in seen: + candidate = f"{base}-{n}" + n += 1 + section["slug"] = candidate + seen.add(candidate) + return sections + + +def write_article(db: Session, topic: str, category_id: int | None = None, + user_id: int | None = None) -> dict: + """Retrieve, write, and store one article as a draft. + + Returns a small report rather than the article, because the caller is a + background run over hundreds of topics and wants to know what happened. + """ + passages = clinical_library.search(topic, limit=PASSAGES, folder_contains=SHELF) + if len(passages) < 3: + # Too little to ground an article. Writing one anyway would produce + # exactly the confident unverifiable prose this is designed to avoid. + return {"topic": topic, "status": "skipped", "reason": "not enough source material", + "passages": len(passages)} + + model_id, api_key = get_model_for_task(db, "extraction") + raw = _call_model(_prompt(topic, passages), model_id, api_key) + text = raw.strip() + if text.startswith("```"): + text = re.sub(r"^```[a-z]*\n?|```$", "", text).strip() + try: + data = json.loads(text) + except ValueError: + logger.warning("Article writer returned unparseable JSON for %s", topic) + return {"topic": topic, "status": "failed", "reason": "model did not return JSON"} + if not isinstance(data, dict): + return {"topic": topic, "status": "failed", "reason": "model returned the wrong shape"} + + sections = _unique_slugs([ + *_sections(data.get("long"), "long"), + *_sections(data.get("key_points"), "key_points"), + *_sections(data.get("clinical"), "clinical"), + ]) + if not sections: + return {"topic": topic, "status": "failed", "reason": "no usable sections"} + + slug = slugify(topic) + existing = db.query(Article).filter(Article.slug == slug).first() + if existing: + return {"topic": topic, "status": "exists", "article_id": existing.id} + + article = Article( + slug=slug, title=topic[:300], + summary=(data.get("summary") or "").strip()[:2000] or None, + content=None, sections=sections, category_id=category_id, user_id=user_id, + # Never published by generation. A person decides that. + status="draft", + # References come from the retrieved metadata, not from the model, so a + # source cannot be invented. + references_json=clinical_library.references_from(passages), + generated_by=f"clinical-library:{model_id}", + ) + from datetime import datetime + + article.generated_at = datetime.utcnow() + db.add(article) + db.flush() + article_service.record_slug(db, article) + db.commit() + db.refresh(article) + return {"topic": topic, "status": "written", "article_id": article.id, + "sections": len(sections), "references": len(article.references_json or []), + "passages": len(passages)} diff --git a/backend/app/services/clinical_library.py b/backend/app/services/clinical_library.py new file mode 100644 index 0000000..79beab6 --- /dev/null +++ b/backend/app/services/clinical_library.py @@ -0,0 +1,173 @@ +"""Read-only retrieval from the clinical library index. + +The index is a Milvus collection of ~1.8M chunks from the reference library — +Nelson, Harrison's, Mandell's and the rest — embedded with bge-m3 at 1024 +dimensions. PedsHub already embeds with the same model through the same LiteLLM +proxy, so a query vector generated here is directly comparable to what is +stored there; no second embedder and no re-indexing. + +Two things this module will not do, deliberately: + +* **It never writes.** The credentials are a query user and the collection + belongs to another system. Articles are grounded in what comes back; nothing + flows the other way. +* **It never hands back long verbatim passages for publication.** What it + returns is source material for someone to write *from*, plus the citation + needed to say where a fact came from. Copying a textbook into an article and + publishing it is redistribution, whatever produced the copy. +""" +import json +import logging +import re +import urllib.parse + +import requests + +from app.config import settings + +logger = logging.getLogger(__name__) + +# Enough context to write a section from, without turning a prompt into a book. +SNIPPET_CHARS = 1200 +DEFAULT_LIMIT = 12 + + +TIMEOUT = 20 + + +def _call(path: str, body: dict) -> dict | None: + """Milvus over its HTTP API rather than gRPC. + + The gRPC client drags in a protobuf and grpcio version surface that fights + with what this image already pins for other services. HTTP has neither + problem and this module only ever reads. + """ + if not settings.CLINICAL_MILVUS_URI: + return None + try: + response = requests.post( + f"{settings.CLINICAL_MILVUS_URI.rstrip('/')}{path}", + headers={"Authorization": f"Bearer {settings.CLINICAL_MILVUS_TOKEN}", + "Content-Type": "application/json"}, + json=body, timeout=TIMEOUT, + ) + except requests.RequestException: + logger.warning("Clinical library unreachable", exc_info=True) + return None + if response.status_code != 200: + logger.warning("Clinical library refused %s: %s %s", path, + response.status_code, response.text[:200]) + return None + payload = response.json() + if payload.get("code") not in (0, None): + logger.warning("Clinical library error on %s: %s", path, str(payload)[:200]) + return None + return payload + + +def available() -> bool: + """Whether the index can be reached at all, for an honest error upstream.""" + payload = _call("/v2/vectordb/collections/list", {}) + return bool(payload) and settings.CLINICAL_MILVUS_COLLECTION in (payload.get("data") or []) + + +def _tidy(value: str | None) -> str: + """File paths in this index are URL-encoded; a citation should not be.""" + if not value: + return "" + return re.sub(r"\s+", " ", urllib.parse.unquote(value)).strip() + + +def _source_of(payload: dict) -> dict: + """The bibliographic details, separated from the prose.""" + title = _tidy(payload.get("title")) or _tidy(payload.get("file_path")).rsplit("/", 1)[-1] + # Library filenames carry the shop they came from; a reference should not. + title = re.sub(r"\s*\((?:z-library|1lib|z-lib)[^)]*\)", "", title, flags=re.I).strip() + title = re.sub(r"\.pdf$", "", title, flags=re.I).strip() + return { + "title": title, + "author": _tidy(payload.get("author")), + "page": payload.get("page_number"), + "specialty": _tidy(payload.get("subcategory")), + } + + +def search(query: str, limit: int = DEFAULT_LIMIT, + folder_contains: str | None = None) -> list[dict]: + """Passages relevant to a topic, each with where it came from. + + `folder_contains` narrows to one shelf of the library — passing "Pediatrics" + is how an article about a childhood condition is grounded in the paediatric + texts rather than in the adult ones that outnumber them. + """ + query = (query or "").strip() + if not query: + return [] + + from app.services.embedding_service import generate_embedding + + vector = generate_embedding(query) + if not vector: + logger.warning("No query embedding; cannot search the clinical library") + return [] + + body = { + "collectionName": settings.CLINICAL_MILVUS_COLLECTION, + "data": [vector], + "annsField": "dense", + "limit": limit, + "outputFields": ["payload", "folder_path", "file_path", "chunk_index"], + } + # Filtering in the query is what keeps the shelf constraint honest: taking + # the top hits and discarding the wrong folder afterwards would silently + # return fewer results the more specific the request was. + if folder_contains: + safe = folder_contains.replace('"', "") + body["filter"] = f'folder_path like "%{safe}%"' + + payload = _call("/v2/vectordb/entities/search", body) + if not payload: + return [] + + results = [] + for hit in (payload.get("data") or []): + raw = hit.get("payload") or {} + if isinstance(raw, str): + try: + raw = json.loads(raw) + except ValueError: + raw = {} + text = _tidy(raw.get("excerpt")) + if not text: + continue + results.append({ + "text": text[:SNIPPET_CHARS], + "score": float(hit.get("distance", 0.0)), + "source": _source_of(raw), + "folder": _tidy(hit.get("folder_path")), + }) + return results + + +def references_from(passages: list[dict], limit: int = 8) -> list[dict]: + """One entry per book, with the pages that were actually used. + + Deduplicated by title because forty chunks of Nelson is one reference, not + forty — a reference list that repeats a book is noise standing where + provenance should be. + """ + by_title: dict[str, dict] = {} + for passage in passages: + source = passage.get("source") or {} + title = source.get("title") + if not title: + continue + entry = by_title.setdefault(title, { + "title": title, "author": source.get("author") or None, "pages": [], + }) + page = source.get("page") + if isinstance(page, int) and page not in entry["pages"]: + entry["pages"].append(page) + for entry in by_title.values(): + entry["pages"] = sorted(entry["pages"])[:6] + return list(by_title.values())[:limit] diff --git a/backend/scripts/fix_lab_formatting.py b/backend/scripts/fix_lab_formatting.py new file mode 100644 index 0000000..eb4a9b8 --- /dev/null +++ b/backend/scripts/fix_lab_formatting.py @@ -0,0 +1,429 @@ +"""Repair OCR-damaged units and turn inline lab panels into markdown tables. + +The PREP PDFs were scanned, so the extracted stems carry two separate injuries. + + 1. Unit corruption. The scanner confuses letter pairs that share a shape — + "m" reads as "rn" or "in", "µ" as "p" or "4" — and superscripts are lost + entirely, so "3.5 × 10⁹/L" arrives as "3.5 x 109/L". The replacements below + were not guessed: every one was found by listing the unit-shaped tokens + that actually occur in the bank and reading the ones that occur once or + twice, which is where the damage hides. Anything whose correction needed a + judgement call — "3,000/mL" that should probably be "/µL", "14.6 x 10A" — + is deliberately absent: a wrong unit is worse than an ugly one. + + 2. A lab panel is a wall of prose. "Sodium, 139 mEq/L (139 mmol/L); Potassium, + …" runs for a paragraph, and the reader has to parse it themselves. A + markdown table separates analyte from value from SI value. + +Every change snapshots the question first through the same helper the edit UI +uses, so anything here can be rolled back from the question's version history. + + docker compose exec backend python -m scripts.fix_lab_formatting + docker compose exec backend python -m scripts.fix_lab_formatting --apply + + --explanations repair units in explanation text as well as stems + --tables also convert lab panels to markdown tables (see below) + --limit N only look at the first N questions, for a quick look + +NOTE ON --tables: the quiz player renders a stem as plain text (QuizPage's +ManualHighlightText slices the raw string so highlight offsets stay valid), so a +markdown table currently shows up as literal pipes there. Table conversion is +therefore opt-in until the stem is rendered as markdown. The unit repair is +safe in every view and runs by default. +""" +import argparse +import re +import sys + +from app.database import SessionLocal +from app.models.question import Question +from app.routers.questions import _snapshot_question + +# The user id recorded against the snapshots this script writes. NULL is +# allowed by the column and is honest: no person made these edits. +SCRIPT_EDITOR_ID = None + +SUPERSCRIPT = {"0": "⁰", "1": "¹", "2": "²", "3": "³", "4": "⁴", + "5": "⁵", "6": "⁶", "7": "⁷", "8": "⁸", "9": "⁹"} + +# Powers of ten that name a real haematology unit: 10³/µL, 10⁶/µL, 10⁹/L, +# 10¹²/L. A stem holding "x 101/µL" or "x 10A" lost a digit rather than a +# superscript, and rewriting it would invent a number, so it is left alone. +REAL_POWERS = {"3", "6", "9", "12"} + + +def _repair_power(match: re.Match) -> str: + """"5.6 x 109/L" -> "5.6 × 10⁹/L", but only for a power that exists.""" + value, exponent = match.group("value"), match.group("exp") + if exponent not in REAL_POWERS: + return match.group(0) + digits = "".join(SUPERSCRIPT[d] for d in exponent) + return f"{value} × 10{digits}{match.group('tail')}" + + +# (pattern, replacement, why) — applied in order, every one idempotent. +# +# Each was verified against the questions it fires on. The narrow ones name a +# single mangled token; the broad ones are anchored so they cannot fire on +# prose (a digit before the unit, a slash after it, or an explicit multiplier). +OCR_REPAIRS = [ + # "m" scanned as the ligature "rn" or as "in". Both appear in chloride and + # sodium lines whose SI twin in the same sentence spells the unit correctly. + (re.compile(r"\bmrnol/L\b"), "mmol/L", "rn read for m"), + (re.compile(r"\binEq/L\b"), "mEq/L", "in read for m"), + (re.compile(r"\bbeats/ruin\b"), "beats/min", "min read as ruin"), + # Units that do not exist. Electrolytes and their SI twins are per litre; + # a per-decilitre spelling is always the scan losing the "m" of "mmol". + (re.compile(r"\bmmol/dL\b"), "mmol/L", "mmol/dL is not a unit"), + (re.compile(r"\bmEq/dL\b"), "mEq/L", "mEq/dL is not a unit"), + (re.compile(r"\bmmo/L\b"), "mmol/L", "dropped l"), + # "q" scanned as "g" — both lines carry an SI twin in mmol/L. + (re.compile(r"\bmEg/L\b"), "mEq/L", "g read for q"), + (re.compile(r"\bmOsm/Kg\b"), "mOsm/kg", "casing"), + # "≤" flattened to "<_" — the underscore is the lower half of the glyph. + (re.compile(r"<_\s*(?=\d)"), "≤", "<_ read for ≤"), + (re.compile(r">_\s*(?=\d)"), "≥", ">_ read for ≥"), + (re.compile(r"\bmMol/L\b"), "mmol/L", "casing"), + (re.compile(r"\bmg/dl\b"), "mg/dL", "casing"), + (re.compile(r"\bg/dl\b"), "g/dL", "casing"), + # µ scanned as p, 4 or lowercase l. Picolitres and "4mol" are never real; + # the counts they carry are ordinary per-microlitre cell counts. + (re.compile(r"(?<=\d)/pL\b"), "/µL", "p read for µ"), + (re.compile(r"\b4mol/L\b"), "µmol/L", "4 read for µ"), + (re.compile(r"(?<=\d)/µl\b"), "/µL", "casing"), + # mm Hg is two tokens; a slash between them is a scanning artefact. + (re.compile(r"\bmm\s*/\s*Hg\b"), "mm Hg", "stray slash"), + (re.compile(r"\bmmHg\b"), "mm Hg", "missing space"), + # Superscripts the scan flattened. Cubic units and BMI first, then powers + # of ten, which need the exponent checked before they can be rewritten. + (re.compile(r"\bkg/m2\b"), "kg/m²", "lost superscript"), + (re.compile(r"\b([µμu])m3\b"), r"µm³", "lost superscript"), + (re.compile(r"\bmm3\b"), "mm³", "lost superscript"), + # MCV is a volume, not a rate: "90/µm³" means "90 µm³". + (re.compile(r"(?<=\d)/µm³"), " µm³", "stray slash"), + (re.compile( + r"(?P\d[\d,]*(?:\.\d+)?)\s*[x×X]\s*10\s*(?P\d{1,2})(?P\s*/)"), + _repair_power, "lost power-of-ten superscript"), +] + + +def repair_units(text: str) -> tuple[str, list[str]]: + """Apply the replacement table, returning the new text and what fired.""" + if not text: + return text or "", [] + fired = [] + for pattern, replacement, reason in OCR_REPAIRS: + new_text, count = pattern.subn(replacement, text) + if new_text != text: + fired.append(f"{reason} ×{count}") + text = new_text + return text, fired + + +# --------------------------------------------------------------------------- +# Lab panel -> markdown table +# --------------------------------------------------------------------------- + +# A value is a number (or a stock qualitative phrase) followed by up to four +# unit-ish tokens and an optional parenthetical. "Unit-ish" is the load-bearing +# part: without it the scanner walks straight out of the panel and swallows the +# sentence that follows the last result. +QUALITATIVE = (r"within normal limits|within the normal range|normal|negative|positive|" + r"pending|trace|not detected|nondetectable|nonreactive|reactive|absent|present") + +# Bare unit words that carry no digit and no slash, so nothing else identifies them. +BARE_UNITS = (r"Hg|fL|fl|mm|cm|kg|mg|g|dL|mL|L|U|IU|sec|s|units?|cells?|mEq|mmol|nmol|pmol|" + r"[µμu]mol|mOsm|ng|pg|mIU|[µμu]IU|kcal|mcg|[µμu]g|[µμu]L|mL|MoM|mg|dl|per|" + r"[µμu]m³|mm³|hpf|HPF|LPF|seconds?|minutes?|hours?|days?|weeks?|months?|years?|to") +_UNIT_TOKEN = (rf"(?:[x×X]|[<>≤≥]|\d[\w,./%°³²⁰-⁹\-]*|" + rf"[A-Za-zµμ³²⁰-⁹]+(?:/[A-Za-zµμ0-9³²⁰-⁹\-]+)+|%|°[CF]|" + rf"(?:{BARE_UNITS}))\.?(?![\w/])") +_CONTINUATION = re.compile(rf"^{_UNIT_TOKEN}$") + +_QUALITATIVE_ONLY = re.compile(rf"^(?:{QUALITATIVE})$", re.I) +_LEAD = rf"(?:[<>≤≥]\s*)?(?:\d[\w,./%°³²⁰-⁹\-]*|{QUALITATIVE})" +# Continuations are separated by a space, never a newline: a line break ends the +# result. Letting \s match it lets the value run into the sentence below the list. +_ITEM = re.compile( + rf"(?[A-Za-z][A-Za-z0-9 '’\-()/%]{{1,60}}?)\s*[,:]\s*" + rf"(?P{_LEAD}(?:[ \t]+{_UNIT_TOKEN}){{0,6}})" + rf"(?P(?:[ \t]*\([^()]{{1,90}}\)){{0,2}})", + re.I) + +# Words that mean the "name" is really a clause, not an analyte. +_NOT_A_NAME = re.compile( + r"\b(?:is|are|was|were|has|have|had|shows?|showed|reveals?|revealed|" + r"includes?|included|and|with|of|the|his|her|he|she|but|which|that|" + r"who|when|following|about|approximately|over|after|before|during|for)\b", re.I) + +# A differential is written the other way round — "29% segmented neutrophils, +# 28% bands" — so reading it as name-then-value pairs each result with the +# NEXT analyte's percentage. Anything sitting directly after a bare percentage +# is part of such a list and is left as prose. +_PERCENT_LABELS_NEXT = re.compile(r"\d\s*%\s*$") + +# A test name never carries a measurement; if it does, the split was wrong. +_VALUE_INSIDE_NAME = re.compile(r"\d\s*(?:%|[A-Za-zµμ]+/[A-Za-zµμ])") + +# What a panel may start after: a lead-in's colon, a separator, a bullet, or a +# line of its own. Anything else means the "panel" is a sentence — a string of +# vital signs, say — and rewriting a sentence as a table breaks its grammar. +_PANEL_OPENERS = (":", ";", "•", "·", "-", "–", "—", "\n") + +def _IS_SI_VALUE(inner: str) -> bool: + """A parenthetical is the SI twin only if it is one number and its unit. + + "(4% segmented, 83% lymphocytes)" also opens with a digit but is a + differential, and filing it under SI units would be a lie about the data. + """ + return (bool(re.match(r"^[<>≤≥]?\s*\d", inner)) + and "," not in inner and len(inner.split()) <= 3) + + +# A reference range is an annotation on the result above it, not a test. +_RANGE_LABEL = re.compile(r"^(?:reference|normal)\s+(?:range|value)s?$", re.I) + +# Enough consecutive results that the block is a panel rather than a passing +# mention of one or two values inside a sentence. +MIN_ITEMS = 4 + +# How much punctuation may sit between two results and still count as the same +# panel: a separator and a space or two, never a whole clause. +MAX_GAP = 4 + + +def _clean_value(value: str) -> str: + """Trim the sentence-ending punctuation that belongs to the prose, not the value.""" + return value.strip().rstrip(",;.").strip() + + +def _accept(match: re.Match) -> tuple[str, str, str] | None: + """(analyte, result, si) if this match is a real lab result, else None.""" + # A "result" that starts inside a bracket is part of the previous result's + # reference range ("(normal, 150 to 350)"), not a new analyte. + if match.start() and match.string[match.start() - 1] in "([": + return None + if _PERCENT_LABELS_NEXT.search(match.string[: match.start()]): + return None + + # The last entry of a written-out list carries the conjunction that joined + # it: "…; and oxygen saturation, 97%". + name = re.sub(r"^and\s+", "", match.group("name").strip(" ,:;-"), flags=re.I) + # A column header the extraction glued on ("Patient Result - Hemoglobin"). + name = name.rpartition(" - ")[2].strip() or name + if len(name) < 2 or _NOT_A_NAME.search(name): + return None + # A name carrying its own value ("Factor II 0.20 U/mL (reference range") + # means the comma we split on belongs to a parenthetical, not to this test. + if _VALUE_INSIDE_NAME.search(name): + return None + + value = match.group("value").strip() + if not _QUALITATIVE_ONLY.match(value): + # The first token opens the value; every later one must look like a unit. + for token in value.split()[1:]: + if not _CONTINUATION.match(token): + return None + + si = "" + for paren in re.findall(r"\([^()]*\)", match.group("paren") or ""): + inner = paren[1:-1].strip() + # A parenthetical that opens with a number is the SI twin; anything + # else ("normal, 150 to 350") is a reference range and stays beside + # the result, where a reader expects to find it. + if not si and _IS_SI_VALUE(inner): + si = inner + else: + value = f"{value} {paren}" + + # A bare percentage that labels the words after it ("Differential count, 29% + # segmented neutrophils") is the same trap from the other side. + if not si and value.rstrip().endswith("%") and \ + re.match(r"^[ \t]+[a-z]", match.string[match.end():]): + return None + return name, _clean_value(value), si + + +def find_panels(text: str) -> list[tuple[int, int, list[tuple[str, str, str]]]]: + """Locate runs of consecutive lab results: (start, end, items).""" + if not text: + return [] + accepted = [] + for match in _ITEM.finditer(text): + item = _accept(match) + if item: + accepted.append((match.start(), match.end(), item)) + + panels, run = [], [] + for entry in accepted: + if run and entry[0] - run[-1][1] > MAX_GAP: + panels.append(run) + run = [] + # Two results for the same analyte mean the run has drifted into prose + # that repeats a word; end the panel rather than emit a confused table. + if run and not _RANGE_LABEL.match(entry[2][0]) and \ + entry[2][0].lower() in {r[2][0].lower() for r in run}: + panels.append(run) + run = [] + run.append(entry) + if run: + panels.append(run) + + out = [] + for panel in panels: + end = panel[-1][1] + # A panel whose next word is lowercase was not parsed to its end — the + # last result trails off into "on arterial blood gas" or a differential. + # Leave the whole block as prose rather than publish a truncated table. + if re.match(r"^[ \t\n]*[.;,•·]*[ \t\n]*[a-z]", text[end:]): + continue + before = text[: panel[0][0]].rstrip(" \t") + if before and not before.endswith(_PANEL_OPENERS): + continue + items = _fold_reference_ranges([e[2] for e in panel]) + if len(items) >= MIN_ITEMS: + out.append((panel[0][0], end, items)) + return out + + +def _fold_reference_ranges(items: list[tuple[str, str, str]]) -> list[tuple[str, str, str]]: + """"…, 125 U/L; reference range, ≤40 U/L" is one result, not two.""" + folded: list[tuple[str, str, str]] = [] + for name, value, si in items: + if folded and _RANGE_LABEL.match(name): + prev_name, prev_value, prev_si = folded[-1] + folded[-1] = (prev_name, f"{prev_value} ({name.lower()}, {value})", prev_si) + continue + folded.append((name, value, si)) + return folded + + +def render_table(items: list[tuple[str, str, str]]) -> str: + """A GFM table; the SI column only exists when something fills it.""" + with_si = any(si for _n, _v, si in items) + header = ["Test", "Result"] + (["SI units"] if with_si else []) + lines = ["| " + " | ".join(header) + " |", + "| " + " | ".join(["---"] * len(header)) + " |"] + for name, value, si in items: + cells = [name, value] + ([si] if with_si else []) + lines.append("| " + " | ".join(c.replace("|", "\\|") for c in cells) + " |") + return "\n".join(lines) + + +def tabulate_panels(text: str) -> tuple[str, int]: + """Replace every confidently-parsed lab panel with a markdown table.""" + if not text or "\n|" in text or text.lstrip().startswith("|"): + return text or "", 0 # already tabulated — keep this idempotent + panels = find_panels(text) + if not panels: + return text, 0 + out, cursor = [], 0 + for start, end, items in panels: + # The lead-in keeps its colon but loses the bullet that was about to + # introduce the first result, and the prose after a table loses the + # separator that used to join it to the last result. + before = text[cursor:start].rstrip().rstrip("•·-–—").rstrip() + out.append(_leading_prose(before) if cursor else before) + out.append("\n\n" + render_table(items) + "\n\n") + cursor = end + out.append(_leading_prose(text[cursor:])) + return "".join(out).strip(), len(panels) + + +def _leading_prose(text: str) -> str: + """Prose that followed a result: drop the punctuation that joined it on.""" + return text.lstrip(" \t\n.;,•·").lstrip() + + +# --------------------------------------------------------------------------- + + +def main() -> int: + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("--apply", action="store_true", help="write the changes") + parser.add_argument("--explanations", action="store_true", + help="repair units in explanations too") + parser.add_argument("--tables", action="store_true", + help="also convert lab panels to markdown tables") + parser.add_argument("--limit", type=int, default=0, help="only scan N questions") + parser.add_argument("--show", type=int, default=6, help="how many examples to print") + args = parser.parse_args() + + db = SessionLocal() + try: + query = db.query(Question).order_by(Question.id) + if args.limit: + query = query.limit(args.limit) + questions = query.all() + + stem_fixes, expl_fixes, tabled = [], [], [] + for question in questions: + new_stem, stem_reasons = repair_units(question.question_text or "") + panels = 0 + if args.tables: + new_stem, panels = tabulate_panels(new_stem) + new_expl, expl_reasons = (question.explanation or ""), [] + if args.explanations: + new_expl, expl_reasons = repair_units(question.explanation or "") + + stem_changed = new_stem != (question.question_text or "") + expl_changed = args.explanations and new_expl != (question.explanation or "") + if not stem_changed and not expl_changed: + continue + + if stem_reasons: + stem_fixes.append((question.id, stem_reasons)) + if expl_reasons: + expl_fixes.append((question.id, expl_reasons)) + if panels: + tabled.append((question.id, panels)) + + if args.apply: + # Snapshot the question as it stands, through the same helper + # the edit endpoint uses, so the version history and the + # restore button work on these edits exactly as on a human one. + _snapshot_question(db, question, SCRIPT_EDITOR_ID) + if stem_changed: + question.question_text = new_stem + if expl_changed: + question.explanation = new_expl + + if args.apply: + db.commit() + + print("APPLIED" if args.apply else "DRY RUN") + print(f" questions scanned : {len(questions)}") + print(f" stems with unit repairs : {len(stem_fixes)}") + if args.explanations: + print(f" explanations repaired : {len(expl_fixes)}") + else: + print(" explanations : skipped (pass --explanations)") + if args.tables: + print(f" stems tabulated : {len(tabled)} " + f"({sum(n for _i, n in tabled)} panels)") + else: + print(" lab tables : skipped (pass --tables)") + + reasons: dict[str, int] = {} + for _qid, fired in stem_fixes + expl_fixes: + for entry in fired: + reason, _, count = entry.rpartition(" ×") + reasons[reason] = reasons.get(reason, 0) + int(count) + print("\n repairs by cause:") + for reason, count in sorted(reasons.items(), key=lambda kv: -kv[1]): + print(f" {count:6d} {reason}") + + for qid, fired in stem_fixes[: args.show]: + print(f" question {qid}: {', '.join(fired)}") + for qid, panels in tabled[: args.show]: + print(f" question {qid}: {panels} lab table(s)") + + if not args.apply: + print("\n Re-run with --apply to write these changes.") + return 0 + finally: + db.close() + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/backend/scripts/generate_articles.py b/backend/scripts/generate_articles.py new file mode 100644 index 0000000..d8839f4 --- /dev/null +++ b/backend/scripts/generate_articles.py @@ -0,0 +1,100 @@ +"""Write a topic article for every condition that has questions. + +Long-running and resumable: an article that already exists is skipped, so the +run can be stopped and restarted without duplicating work or spending a model +call twice on the same topic. + +Everything it writes is a draft. Generated medical writing that nobody has read +must not reach a learner, so publishing stays a decision a person makes on the +editorial queue. + + docker compose exec backend python -m scripts.generate_articles + docker compose exec backend python -m scripts.generate_articles --apply + docker compose exec backend python -m scripts.generate_articles --apply --limit 25 +""" +import argparse +import sys +import time + +from sqlalchemy import text as sa_text + +from app.database import SessionLocal +from app.models.article import Article +from app.services import clinical_library +from app.services.article_writer import slugify, write_article + +# A pause between topics, so a run of hundreds does not monopolise the model +# proxy that the rest of the platform shares. +PAUSE_SECONDS = 1.5 + + +def candidates(db, limit: int | None): + """Leaf categories that actually hold questions, biggest first. + + Leaves because a condition is what an article is about; a discipline is a + shelf. Biggest first so that stopping the run early still leaves the topics + carrying the most questions covered. + """ + rows = db.execute(sa_text(""" + SELECT c.id, c.name, COUNT(q.id) 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 + """)).fetchall() + return rows[:limit] if limit else rows + + +def main(): + parser = argparse.ArgumentParser() + parser.add_argument("--apply", action="store_true", help="actually write articles") + parser.add_argument("--limit", type=int, default=None, help="stop after this many topics") + parser.add_argument("--user-id", type=int, default=None, help="author to record") + args = parser.parse_args() + + if args.apply and not clinical_library.available(): + print(" The clinical library index is unreachable; refusing to write " + "articles with nothing to ground them in.") + return 1 + + db = SessionLocal() + try: + topics = candidates(db, args.limit) + existing = {row[0] for row in db.query(Article.slug).all()} + todo = [(cid, name, uses) for cid, name, uses in topics if slugify(name) not in existing] + + print(f" conditions with questions : {len(topics)}") + print(f" already written : {len(topics) - len(todo)}") + print(f" to write : {len(todo)}") + if not args.apply: + print("\n First 15:") + for _cid, name, uses in todo[:15]: + print(f" {uses:4d} {name}") + print("\n Re-run with --apply to write them. Everything lands as a draft.") + return 0 + + written = skipped = failed = 0 + for index, (category_id, name, uses) in enumerate(todo, start=1): + try: + result = write_article(db, name, category_id=category_id, user_id=args.user_id) + except Exception as error: # one bad topic must not end a run of hundreds + db.rollback() + result = {"status": "failed", "reason": str(error)[:120]} + status = result.get("status") + written += status == "written" + skipped += status in ("skipped", "exists") + failed += status == "failed" + note = result.get("reason", "") + print(f" [{index}/{len(todo)}] {name[:52]:<52} {status:<8} {note[:40]}", flush=True) + time.sleep(PAUSE_SECONDS) + + print(f"\n written : {written}\n skipped : {skipped}\n failed : {failed}") + print(" All drafts. Review them in the editorial queue before publishing.") + finally: + db.close() + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/docker-compose.yml b/docker-compose.yml index b7a462f..0099b88 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -38,6 +38,7 @@ services: networks: - default - danvics_speech + - danvics_milvus depends_on: postgres: condition: service_healthy @@ -57,6 +58,9 @@ services: volumes: - uploads_data:/app/uploads - chroma_data:/app/chroma_data + networks: + - default + - danvics_milvus depends_on: postgres: condition: service_healthy @@ -172,3 +176,8 @@ volumes: networks: danvics_speech: external: true + # The clinical library index. Note there are two Milvus servers on this host: + # this is the one holding mcp_bge_m3_1024, reached as `milvus`. The other, on + # ped-ai-storage_basic, is a different instance with different credentials. + danvics_milvus: + external: true diff --git a/frontend/src/App.jsx b/frontend/src/App.jsx index 803b56b..63e94c8 100644 --- a/frontend/src/App.jsx +++ b/frontend/src/App.jsx @@ -34,6 +34,7 @@ const ArticlesPage = lazy(() => import('./pages/ArticlesPage')) const SearchPage = lazy(() => import('./pages/SearchPage')) const AiModePage = lazy(() => import('./pages/AiModePage')) const MediaPage = lazy(() => import('./pages/MediaPage')) +const EditorialPage = lazy(() => import('./pages/EditorialPage')) const StudyPlansPage = lazy(() => import('./pages/StudyPlansPage')) const StudyPlanPage = lazy(() => import('./pages/StudyPlanPage')) const ArticlePage = lazy(() => import('./pages/ArticlesPage').then(m => ({ default: m.ArticlePage }))) @@ -130,6 +131,7 @@ function AppRoutes() { } /> } /> } /> + } /> } /> } /> diff --git a/frontend/src/components/ArticleLink.css b/frontend/src/components/ArticleLink.css index 574f375..32c9c9d 100644 --- a/frontend/src/components/ArticleLink.css +++ b/frontend/src/components/ArticleLink.css @@ -25,8 +25,9 @@ font-size: 0.84rem; line-height: 1.5; text-align: left; - /* The card is a hint, not a target: it must never sit between the pointer and - the link it describes. */ + /* The card is a hint before it is a target: its body stays transparent to the + pointer so it never sits between the reader and the link it describes, and + only the controls below take clicks. */ pointer-events: none; } .al-card.is-above { top: auto; bottom: calc(100% + 8px); } @@ -34,6 +35,26 @@ .al-card-excerpt { color: var(--text-muted); } .al-card-meta { font-size: 0.75rem; color: var(--text-subtle); } +.al-card-actions { display: flex; gap: 6px; margin-top: 2px; pointer-events: auto; } +.al-card-action { + display: inline-flex; + align-items: center; + gap: 5px; + padding: 4px 9px; + background: none; + border: 1px solid var(--border); + border-radius: 6px; + font: inherit; + font-size: 0.72rem; + line-height: 1.4; + color: var(--text-muted); + text-decoration: none; + white-space: nowrap; + cursor: pointer; +} +.al-card-action:hover { border-color: var(--primary); color: var(--primary); } +.al-card-action:focus-visible { outline: 2px solid var(--primary); outline-offset: 1px; } + /* No hover on touch, so the card never appears there and the link is just a link. */ @media (hover: none) { .al-card { display: none; } diff --git a/frontend/src/components/ArticleLink.jsx b/frontend/src/components/ArticleLink.jsx index d8ee7fd..e40df7f 100644 --- a/frontend/src/components/ArticleLink.jsx +++ b/frontend/src/components/ArticleLink.jsx @@ -1,6 +1,7 @@ import { useCallback, useEffect, useRef, useState } from 'react' import { Link } from 'react-router-dom' import api from '../api/client' +import { useSplitView } from '../context/SplitViewContext' import './ArticleLink.css' // One fetch per article for the life of the page. A reader hovers the same @@ -9,6 +10,9 @@ import './ArticleLink.css' const cache = new Map() const HOVER_DELAY = 350 // Long enough that crossing a link does not summon a card. +// Long enough to cross the gap between the link and the card's controls, and to +// cover the blur-then-focus gap when Tab moves between them. +const HIDE_DELAY = 260 export function fetchPreview(slug) { if (!cache.has(slug)) { @@ -24,7 +28,8 @@ export function fetchPreview(slug) { * * Following a link to find out whether it was worth following is the thing that * breaks a train of thought, so the preview answers that in place: title, a - * couple of sentences, and how much is there. + * couple of sentences, how much is there, and — where the page can hold one — + * the offer to open it beside what you are reading rather than in place of it. * * Touch has no hover, so on a phone this is just a link — a card that appears on * tap would sit between the finger and the thing it was about to open. @@ -38,31 +43,61 @@ export default function ArticleLink({ slug, children, className = '' }) { const [above, setAbove] = useState(false) const timer = useRef(null) const anchor = useRef(null) + const split = useSplitView() + const href = `/articles/s/${slug}` useEffect(() => () => clearTimeout(timer.current), []) - const show = useCallback(() => { - clearTimeout(timer.current) - timer.current = setTimeout(async () => { - const data = await fetchPreview(slug) - if (!data) return - // Flip the card above the link when there is no room beneath it. - const box = anchor.current?.getBoundingClientRect?.() - if (box) setAbove(window.innerHeight - box.bottom < 220) - setPreview(data) - setOpen(true) - }, HOVER_DELAY) + const reveal = useCallback(async () => { + const data = await fetchPreview(slug) + if (!data) return + // Flip the card above the link when there is no room beneath it. + const box = anchor.current?.getBoundingClientRect?.() + if (box) setAbove(window.innerHeight - box.bottom < 220) + setPreview(data) + setOpen(true) }, [slug]) - const hide = useCallback(() => { + const showSoon = useCallback(() => { + clearTimeout(timer.current) + timer.current = setTimeout(reveal, HOVER_DELAY) + }, [reveal]) + + // Focus is a deliberate act, not a pointer passing through, and a keyboard + // reader who tabs on before the delay elapses would never meet the controls. + const showNow = useCallback(() => { + clearTimeout(timer.current) + reveal() + }, [reveal]) + + // Leaving is given a grace period so the pointer can travel the gap from the + // link down onto the card's controls without the card vanishing under it. + const hideSoon = useCallback(() => { + clearTimeout(timer.current) + timer.current = setTimeout(() => setOpen(false), HIDE_DELAY) + }, []) + + const hideNow = useCallback(() => { clearTimeout(timer.current) setOpen(false) }, []) + const followLink = (event) => { + hideNow() + // Inside the second pane a cross-reference stays in that pane: the article + // you started from is on the left and should still be there afterwards. + // Modified and middle clicks are left alone so they still open a tab. + if (!split?.inPane || event.button !== 0) return + if (event.metaKey || event.ctrlKey || event.shiftKey || event.altKey) return + event.preventDefault() + split.open(slug) + } + return ( - - + { if (event.key === 'Escape') hideNow() }}> + {children} {open && preview && ( @@ -73,6 +108,21 @@ export default function ArticleLink({ slug, children, className = '' }) { {preview.section_count} section{preview.section_count === 1 ? '' : 's'} {preview.status !== 'published' && ' · draft'} + + {split && ( + + )} + {/* A real link on the real URL, so middle-click, ctrl-click and + "copy link address" work without this component's help. */} + + New tab + + )} diff --git a/frontend/src/components/ArticleLink.test.jsx b/frontend/src/components/ArticleLink.test.jsx index e38bbdb..11212e7 100644 --- a/frontend/src/components/ArticleLink.test.jsx +++ b/frontend/src/components/ArticleLink.test.jsx @@ -1,8 +1,9 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' -import { render, screen, waitFor } from '@testing-library/react' +import { render, screen, waitFor, within } from '@testing-library/react' import userEvent from '@testing-library/user-event' -import { MemoryRouter } from 'react-router-dom' +import { MemoryRouter, Route, Routes } from 'react-router-dom' import ArticleLink from './ArticleLink' +import { SplitViewProvider } from '../context/SplitViewContext' import api from '../api/client' vi.mock('../api/client', () => ({ default: { get: vi.fn() } })) @@ -16,6 +17,20 @@ const preview = { const mount = (slug = 'febrile-seizures') => render( febrile seizures) +// The link under a page that can hold a second pane, with somewhere to navigate +// to so an ordinary click can be told apart from one the component handled. +const mountBeside = (view, slug) => render( + + + + febrile seizures} /> + Whole article

} /> + + + ) + +const linkNamed = () => screen.getByRole('link', { name: 'febrile seizures' }) + describe('cross-reference previews', () => { beforeEach(() => { vi.clearAllMocks() @@ -33,14 +48,14 @@ describe('cross-reference previews', () => { mount() expect(screen.queryByRole('tooltip')).not.toBeInTheDocument() - await userEvent.hover(screen.getByRole('link')) + await userEvent.hover(linkNamed()) const card = await screen.findByRole('tooltip', {}, { timeout: 2000 }) expect(card).toHaveTextContent('Febrile seizures') expect(card).toHaveTextContent('A seizure with fever') expect(card).toHaveTextContent('4 sections') - await userEvent.unhover(screen.getByRole('link')) - expect(screen.queryByRole('tooltip')).not.toBeInTheDocument() + await userEvent.unhover(linkNamed()) + await waitFor(() => expect(screen.queryByRole('tooltip')).not.toBeInTheDocument()) }) it('fetches a given article once however often it is hovered', async () => { @@ -73,3 +88,82 @@ describe('cross-reference previews', () => { expect(screen.queryByRole('tooltip')).not.toBeInTheDocument() }) }) + +describe('reading a cross-reference without leaving the article', () => { + beforeEach(() => { + vi.clearAllMocks() + vi.useRealTimers() + api.get.mockResolvedValue({ data: preview }) + }) + + it('offers the card two ways out: beside this article, or in a tab', async () => { + const open = vi.fn() + mountBeside({ open, inPane: false }, 'card-controls') + await userEvent.hover(linkNamed()) + const card = await screen.findByRole('tooltip', {}, { timeout: 2000 }) + + expect(within(card).getByRole('button', { name: /split view/i })).toBeInTheDocument() + const newTab = within(card).getByRole('link', { name: /new tab/i }) + expect(newTab).toHaveAttribute('href', '/articles/s/card-controls') + expect(newTab).toHaveAttribute('target', '_blank') + expect(newTab).toHaveAttribute('rel', 'noopener noreferrer') + }) + + it('offers no split where the page has nowhere to put one', async () => { + mount('no-pane-here') + await userEvent.hover(screen.getByRole('link')) + const card = await screen.findByRole('tooltip', {}, { timeout: 2000 }) + expect(within(card).queryByRole('button', { name: /split view/i })).not.toBeInTheDocument() + expect(within(card).getByRole('link', { name: /new tab/i })).toBeInTheDocument() + }) + + it('stays open while the pointer travels from the link to its controls', async () => { + const open = vi.fn() + mountBeside({ open, inPane: false }, 'reachable-card') + await userEvent.hover(linkNamed()) + const card = await screen.findByRole('tooltip', {}, { timeout: 2000 }) + const split = within(card).getByRole('button', { name: /split view/i }) + + // Leaving the link is not leaving the card: the gap between them is the + // route to the controls, not a reason to take them away. + await userEvent.unhover(linkNamed()) + await userEvent.hover(split) + await new Promise(resolve => setTimeout(resolve, 400)) + expect(screen.getByRole('tooltip')).toBeInTheDocument() + + await userEvent.click(split) + expect(open).toHaveBeenCalledWith('reachable-card') + expect(screen.queryByRole('tooltip')).not.toBeInTheDocument() + }) + + it('lets a keyboard reader reach both controls', async () => { + const open = vi.fn() + mountBeside({ open, inPane: false }, 'keyboard-reachable') + await userEvent.tab() + expect(linkNamed()).toHaveFocus() + + // Focus is deliberate, so the card does not make a keyboard reader wait out + // the hover delay before its controls exist to be tabbed onto. + const card = await screen.findByRole('tooltip', {}, { timeout: 2000 }) + await userEvent.tab() + expect(within(card).getByRole('button', { name: /split view/i })).toHaveFocus() + await userEvent.tab() + expect(within(card).getByRole('link', { name: /new tab/i })).toHaveFocus() + }) + + it('leaves an ordinary click alone: the link is still a link', async () => { + const open = vi.fn() + mountBeside({ open, inPane: false }, 'plain-click') + await userEvent.click(linkNamed()) + expect(await screen.findByText('Whole article')).toBeInTheDocument() + expect(open).not.toHaveBeenCalled() + }) + + it('keeps a link followed inside the pane inside the pane', async () => { + const open = vi.fn() + mountBeside({ open, inPane: true }, 'deeper-topic') + await userEvent.click(linkNamed()) + expect(open).toHaveBeenCalledWith('deeper-topic') + expect(screen.queryByText('Whole article')).not.toBeInTheDocument() + }) +}) diff --git a/frontend/src/components/ArticleReader.jsx b/frontend/src/components/ArticleReader.jsx new file mode 100644 index 0000000..ef2301b --- /dev/null +++ b/frontend/src/components/ArticleReader.jsx @@ -0,0 +1,242 @@ +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 +} + +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)} + + ) +} + +/** + * An article as it reads: contents rail, summary, and sections that expand + * where they sit. + * + * One renderer serves both the page and the second pane beside it, so a + * cross-reference opened in split view behaves like the article it came from + * rather than like a cut-down copy of it. + * + * `activeSection` is the caller's, because the page keeps it in the URL for deep + * links and the pane keeps it to itself; everything else — what is open, the + * rail, the mobile drawer — belongs to this reader alone. + */ +// One topic, three readings. The full article is what you study from; the key +// points are what you revise from; the clinical view is what you act from at the +// bedside. They are views of one article rather than three articles, so the +// numbers cannot drift apart and a question linked to the topic still means one +// thing. +const VIEWS = [ + { key: 'long', label: 'Article' }, + { key: 'key_points', label: 'Key points' }, + { key: 'clinical', label: 'Clinical' }, +] + +export default function ArticleReader({ article, activeSection = '', onOpenSection, idPrefix = '', landmark = true, children }) { + // Which sections are open. Everything starts closed: an article is a reference + // you consult, and a wall of prose hides the one heading you came for. + const [openIds, setOpenIds] = useState({}) + const [drawerOpen, setDrawerOpen] = useState(false) + // Collapsing the contents rail hands its width to the prose. + const [railOpen, setRailOpen] = useState(true) + const root = useRef(null) + // A page has one main landmark, so the second reader on it is not one. + const Content = landmark ? 'main' : 'div' + + // Older articles have no variant on their sections; they are the full article. + const everySection = (article.sections || []).map( + sec => ({ ...sec, variant: sec.variant || 'long' })) + 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) + // 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}`) + + // Deep link: open the section (and the one it sits under) and scroll to it. + // Landing on a collapsed heading would look like the link had gone nowhere. + useEffect(() => { + if (!activeSection) return + const parentId = allSections.find(sec => sec.id === activeSection)?.parent_id + setOpenIds(prev => ({ ...prev, [activeSection]: true, ...(parentId ? { [parentId]: true } : {}) })) + sectionElement(activeSection)?.scrollIntoView?.({ block: 'start' }) + }, [activeSection, article]) + + /** Jump to a section in the page and hand it back for the URL. */ + const openSection = (secId) => { + setDrawerOpen(false) + if (secId) { + const parentId = allSections.find(sec => sec.id === secId)?.parent_id + setOpenIds(prev => ({ ...prev, [secId]: true, ...(parentId ? { [parentId]: true } : {}) })) + sectionElement(secId)?.scrollIntoView?.({ behavior: 'smooth', block: 'start' }) + } + onOpenSection?.(secId) + } + + /** Open or close one section without moving the page. */ + const toggleSection = (secId) => setOpenIds(prev => ({ ...prev, [secId]: !prev[secId] })) + + const childSections = allSections.filter(sec => sec.parent_id) + const kidsOf = (secId) => childSections.filter(sec => sec.parent_id === secId) + // References belong at the end whatever order they were written in — a reader + // scrolling for content should not hit the bibliography halfway down. + const isReferences = (sec) => /^references$/i.test(sec.title || '') || sec.slug === 'references' + const topSections = [ + ...allSections.filter(sec => !sec.parent_id && !isReferences(sec)), + ...allSections.filter(sec => !sec.parent_id && isReferences(sec)), + ] + const setAll = (open) => setOpenIds(open + ? Object.fromEntries(allSections.map(sec => [sec.id, true])) + : {}) + const allOpen = allSections.length > 0 && allSections.every(sec => openIds[sec.id]) + + const renderSection = (sec, depth) => { + const isOpen = !!openIds[sec.id] + const kids = kidsOf(sec.id) + const Heading = depth === 0 ? 'h2' : 'h3' + return ( +
+ + + + {isOpen && ( +
+ {sec.content} + {kids.map(kid => renderSection(kid, depth + 1))} +
+ )} +
+ ) + } + + return ( +
+ + + + {article.updated_at && ( +

Last edited {new Date(article.updated_at).toLocaleDateString(undefined, { year: 'numeric', month: 'short', day: 'numeric' })}

+ )} + {article.summary &&

{article.summary}

} + {article.content && {article.content}} + {!allSections.length && (!article.summary && !article.content) && ( +
Content is being prepared by educators.
+ )} + + {/* Headings first, prose on request: the article opens as a contents + page you can scan, and each section expands where it sits rather + than in a modal that loses the thread. */} +
+ {/* Only the views this article actually has: an empty tab is a + promise the article cannot keep. */} + {present.length > 1 && ( +
+ {present.map(option => ( + + ))} +
+ )} + {allSections.length > 0 && ( + + )} +
+
+ {topSections.map(sec => renderSection(sec, 0))} +
+ + {(article.references || []).length > 0 && ( +
+

References

+ {/* Sources for the whole article, not markers in the prose: a + learner checking a claim wants the book and the page, and a + sentence peppered with superscripts is harder to read. */} +
    + {article.references.map((ref, index) => ( +
  1. + {ref.title} + {ref.author && — {ref.author}} + {(ref.pages || []).length > 0 && ( + · p. {ref.pages.join(', ')} + )} +
    2. + ))} +
+
+ )} + + {children} + +
+ ) +} diff --git a/frontend/src/components/ArticleSplitPane.css b/frontend/src/components/ArticleSplitPane.css new file mode 100644 index 0000000..78500d3 --- /dev/null +++ b/frontend/src/components/ArticleSplitPane.css @@ -0,0 +1,89 @@ +/* Split view: a cross-reference read beside the article that pointed at it. */ + +.article-split { min-width: 0; } +.article-split.is-open { + display: grid; + grid-template-columns: minmax(0, 1fr) minmax(0, 1fr); + gap: 16px; + align-items: start; +} +/* Two columns of prose need more room than one. At the reading page's usual + 1080px each pane would be narrower than a table. */ +.article-page.is-split { max-width: min(1560px, 100%); } + +/* Each pane scrolls alone, which is the whole point: following a + cross-reference must not move the article you were reading. */ +.article-split.is-open > .article-split-main, +.article-split.is-open > .article-split-pane { + max-height: calc(100vh - 120px); + overflow-y: auto; +} +/* The rail is sticky inside its own pane now, not inside the window. */ +.article-page.is-split .article-sections { top: 6px; max-height: calc(100vh - 200px); } +.article-page.is-split .article-layout { grid-template-columns: 190px 1fr; gap: 14px; } +.article-page.is-split .article-content { padding: 16px 18px; } + +.article-split-pane { + background: var(--card-bg); + border: 1px solid var(--border); + border-radius: var(--card-radius); +} +.article-split-pane:focus-visible { outline: 2px solid var(--primary); outline-offset: 2px; } + +.asplit-head { + position: sticky; + top: 0; + z-index: 2; + display: flex; + align-items: center; + gap: 10px; + padding: 10px 12px; + background: var(--card-bg); + border-bottom: 1px solid var(--border); + border-radius: var(--card-radius) var(--card-radius) 0 0; +} +.asplit-title { + flex: 1; + min-width: 0; + font-size: 0.95rem; + font-weight: 700; + color: var(--text); + text-decoration: none; + overflow-wrap: anywhere; +} +.asplit-title:hover { color: var(--primary); } +.asplit-close { + flex-shrink: 0; + width: 30px; + height: 30px; + background: none; + border: 1px solid var(--border); + border-radius: 8px; + font: inherit; + color: var(--text-muted); + cursor: pointer; +} +.asplit-close:hover { border-color: var(--primary); color: var(--primary); } +.asplit-body { padding: 4px 12px 16px; } +/* The pane is already a card, so the reader inside it does not draw a second one. */ +.asplit-body .article-content { background: none; border: 0; padding: 8px 0 0; } + +@media (max-width: 820px) { + /* Two panes on a phone are two unreadable columns, and shrinking one to a + strip only invites pinch-zoom. The cross-reference takes the screen + instead, as a sheet over the article — which is still exactly where it was, + at the same scroll position, when the sheet closes. */ + .article-split.is-open { display: block; } + .article-split.is-open > .article-split-main { max-height: none; overflow: visible; } + .article-page.is-split { max-width: 1080px; } + .article-page.is-split .article-layout { grid-template-columns: 1fr; } + .article-split.is-open > .article-split-pane { + position: fixed; + inset: 0; + z-index: 60; + max-height: none; + border: 0; + border-radius: 0; + } + .asplit-head { border-radius: 0; } +} diff --git a/frontend/src/components/ArticleSplitPane.jsx b/frontend/src/components/ArticleSplitPane.jsx new file mode 100644 index 0000000..5c241ef --- /dev/null +++ b/frontend/src/components/ArticleSplitPane.jsx @@ -0,0 +1,58 @@ +import { useEffect, useRef, useState } from 'react' +import { Link } from 'react-router-dom' +import api from '../api/client' +import ArticleReader from './ArticleReader' +import { resolveArticleId } from './ArticleLink' +import './ArticleSplitPane.css' + +/** + * A second article beside the one being read. + * + * The point of split view is that the article you came from stays put, so this + * pane carries only the reading — no comments, no practice, no editing — and + * the full article is one click away in the heading for anything more. + */ +export default function ArticleSplitPane({ slug, onClose }) { + const [article, setArticle] = useState(null) + const [error, setError] = useState('') + const [activeSection, setActiveSection] = useState('') + const pane = useRef(null) + + useEffect(() => { + let live = true + setArticle(null) + setError('') + setActiveSection('') + // The slug's id has already been fetched by the preview card that offered + // the split, so opening the pane costs one request, not two. + resolveArticleId(slug) + .then(id => (id ? api.get(`/articles/${id}`) : Promise.reject(new Error('unresolved')))) + .then(res => { if (live) setArticle(res.data) }) + .catch(() => { if (live) setError('That article could not be opened here.') }) + return () => { live = false } + }, [slug]) + + // Focus follows the reader into the pane: on a narrow screen it covers the + // article, so leaving focus behind would strand a keyboard or screen-reader + // user on prose they can no longer see. + useEffect(() => { pane.current?.focus() }, [slug]) + + return ( + + ) +} diff --git a/frontend/src/components/Navbar.jsx b/frontend/src/components/Navbar.jsx index 9df3b43..33ab937 100644 --- a/frontend/src/components/Navbar.jsx +++ b/frontend/src/components/Navbar.jsx @@ -111,7 +111,8 @@ export default function Navbar({ onSignIn, onRegister }) { { to: '/analysis', label: 'Analysis' }, { to: '/question-bank', label: 'Question Bank' }, ...(canManageQuestions ? [{ to: '/questions/manage', label: 'Manage Qs' }, - { to: '/media', label: 'Images' }] : []), + { to: '/media', label: 'Images' }, + { to: '/editorial', label: 'Editorial' }] : []), { to: '/study-plans', label: 'Study plans' }, { to: '/articles', label: 'Reading' }, { to: '/flashcards', label: 'Cards' }, diff --git a/frontend/src/context/SplitViewContext.jsx b/frontend/src/context/SplitViewContext.jsx new file mode 100644 index 0000000..73b4f2e --- /dev/null +++ b/frontend/src/context/SplitViewContext.jsx @@ -0,0 +1,19 @@ +import { createContext, useContext } from 'react' + +/** + * Where a cross-reference can be opened beside the article you are reading. + * + * A cross-reference sits deep inside rendered Markdown, several components away + * from the page that owns the second pane, so the offer travels by context + * rather than through every renderer in between. No provider means no second + * pane exists — a library listing, a question review — and the link is then + * only a link. + */ +const SplitViewContext = createContext(null) + +export const SplitViewProvider = SplitViewContext.Provider + +/** `{ open(slug), inPane }`, or null where there is nowhere to put a pane. */ +export function useSplitView() { + return useContext(SplitViewContext) +} diff --git a/frontend/src/pages/ArticleSplitView.test.jsx b/frontend/src/pages/ArticleSplitView.test.jsx new file mode 100644 index 0000000..fe20f86 --- /dev/null +++ b/frontend/src/pages/ArticleSplitView.test.jsx @@ -0,0 +1,118 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import { render, screen, waitFor, within } from '@testing-library/react' +import userEvent from '@testing-library/user-event' +import { MemoryRouter, Route, Routes } from 'react-router-dom' +import { ArticlePage } from './ArticlesPage' +import api from '../api/client' + +vi.mock('../api/client', () => ({ default: { get: vi.fn(), post: vi.fn(), patch: vi.fn(), put: vi.fn(), delete: vi.fn() } })) +vi.mock('../context/AuthContext', () => ({ useAuth: () => ({ user: { id: 1, name: 'Learner', role: 'user' } }) })) +vi.mock('../components/RichEditor', () => ({ default: ({ value }) =>
{value}
})) +vi.mock('../utils/uploads', () => ({ markdownImageUrl: (src) => src })) + +const article = { + id: 1, slug: 'febrile-seizures', title: 'Febrile seizures', summary: '', status: 'published', + content: 'Fever alone rarely explains it — see [[meningitis]].', category_breadcrumbs: [], + sections: [{ id: 'a'.repeat(32), slug: 'workup', title: 'Initial workup', content: 'Workup body' }], + user_id: 3, +} +const meningitis = { + id: 7, slug: 'meningitis', title: 'Meningitis', status: 'published', content: 'Meningitis intro, and [[sepsis]].', + sections: [{ id: 'b'.repeat(32), slug: 'signs', title: 'Signs', content: 'Neck stiffness' }], +} +const sepsis = { id: 9, slug: 'sepsis', title: 'Sepsis', status: 'published', content: 'Sepsis intro', sections: [] } + +const previews = { + meningitis: { id: 7, slug: 'meningitis', title: 'Meningitis', excerpt: 'Inflammation of the meninges…', section_count: 1, status: 'published' }, + sepsis: { id: 9, slug: 'sepsis', title: 'Sepsis', excerpt: 'Life-threatening organ dysfunction…', section_count: 0, status: 'published' }, +} + +const mount = () => render( + + } /> + ) + +describe('reading a cross-reference beside the article', () => { + beforeEach(() => { + vi.resetAllMocks() + api.get.mockImplementation(url => { + if (url.startsWith('/articles/preview/')) return Promise.resolve({ data: previews[url.split('/').pop()] }) + if (url === '/articles/1') return Promise.resolve({ data: article }) + if (url === '/articles/7') return Promise.resolve({ data: meningitis }) + if (url === '/articles/9') return Promise.resolve({ data: sepsis }) + return Promise.resolve({ data: [] }) + }) + }) + + const openSplit = async (name) => { + await userEvent.hover(screen.getByRole('link', { name })) + const card = await screen.findByRole('tooltip', {}, { timeout: 2000 }) + await userEvent.click(within(card).getByRole('button', { name: /split view/i })) + } + + it('opens the linked article beside the one that pointed at it', async () => { + mount() + expect(await screen.findByText(/Fever alone rarely explains it/)).toBeInTheDocument() + expect(screen.queryByRole('region', { name: /^Split view/ })).not.toBeInTheDocument() + + await openSplit('meningitis') + await waitFor(() => expect(api.get).toHaveBeenCalledWith('/articles/7')) + const pane = await screen.findByRole('region', { name: 'Split view: Meningitis' }) + expect(within(pane).getByText(/Meningitis intro/)).toBeInTheDocument() + + // The article that sent the reader there is still on the page, unmoved. + expect(screen.getByText(/Fever alone rarely explains it/)).toBeInTheDocument() + expect(document.querySelector('.article-split')).toHaveClass('is-open') + expect(document.querySelector('.article-page')).toHaveClass('is-split') + }) + + it('brings the whole reader with it: contents, and sections that expand', async () => { + mount() + await screen.findByText(/Fever alone rarely explains it/) + await openSplit('meningitis') + const pane = await screen.findByRole('region', { name: 'Split view: Meningitis' }) + + expect(within(pane).queryByText('Neck stiffness')).not.toBeInTheDocument() + // Contents rail and collapsible heading both, the same as the page behind it. + expect(within(pane.querySelector('.article-sections')).getByRole('button', { name: 'Signs' })).toBeInTheDocument() + await userEvent.click(within(pane.querySelector('.asec-list')).getByRole('button', { name: /Signs/ })) + expect(within(pane).getByText('Neck stiffness')).toBeInTheDocument() + }) + + it('closing it gives the page back its single column', async () => { + mount() + await screen.findByText(/Fever alone rarely explains it/) + await openSplit('meningitis') + const pane = await screen.findByRole('region', { name: 'Split view: Meningitis' }) + + await userEvent.click(within(pane).getByRole('button', { name: 'Close split view' })) + expect(screen.queryByRole('region', { name: /^Split view/ })).not.toBeInTheDocument() + expect(document.querySelector('.article-split')).not.toHaveClass('is-open') + expect(document.querySelector('.article-page')).not.toHaveClass('is-split') + expect(screen.getByText(/Fever alone rarely explains it/)).toBeInTheDocument() + }) + + it('follows a link inside the pane in the pane, not into a third column', async () => { + mount() + await screen.findByText(/Fever alone rarely explains it/) + await openSplit('meningitis') + const pane = await screen.findByRole('region', { name: 'Split view: Meningitis' }) + + await userEvent.click(within(pane).getByRole('link', { name: 'sepsis' })) + await waitFor(() => expect(api.get).toHaveBeenCalledWith('/articles/9')) + expect(await screen.findByRole('region', { name: 'Split view: Sepsis' })).toBeInTheDocument() + expect(document.querySelectorAll('.article-split-pane')).toHaveLength(1) + // The article on the left is where it was; only the pane changed. + expect(screen.getByText(/Fever alone rarely explains it/)).toBeInTheDocument() + }) + + it('closes on Escape, the way anything laid over the page should', async () => { + mount() + await screen.findByText(/Fever alone rarely explains it/) + await openSplit('meningitis') + await screen.findByRole('region', { name: 'Split view: Meningitis' }) + + await userEvent.keyboard('{Escape}') + await waitFor(() => expect(screen.queryByRole('region', { name: /^Split view/ })).not.toBeInTheDocument()) + }) +}) diff --git a/frontend/src/pages/ArticlesPage.css b/frontend/src/pages/ArticlesPage.css index 398ab09..32922fe 100644 --- a/frontend/src/pages/ArticlesPage.css +++ b/frontend/src/pages/ArticlesPage.css @@ -199,3 +199,26 @@ .asec-head { font-size: 0.95rem; } .asec-body { font-size: 0.92rem; } } + +/* One topic, three readings. */ +.asec-controls { display: flex; align-items: center; gap: 10px; justify-content: space-between; flex-wrap: wrap; } +.aview-switch { display: inline-flex; gap: 2px; padding: 3px; background: var(--bg); border: 1px solid var(--border); border-radius: 9px; } +.aview { + min-height: 34px; padding: 6px 13px; border: 0; border-radius: 7px; cursor: pointer; + background: none; font: inherit; font-size: 0.83rem; font-weight: 600; color: var(--text-muted); +} +.aview:hover { color: var(--text); } +.aview.is-active { background: var(--card-bg); color: var(--primary); box-shadow: 0 1px 2px rgba(15,23,42,0.08); } + +/* Sources for the whole article, not markers scattered through the prose. */ +.article-references { margin-top: 26px; padding-top: 14px; border-top: 1px solid var(--border); } +.article-references h2 { margin: 0 0 10px; font-size: 0.78rem; font-weight: 700; letter-spacing: 0.06em; text-transform: uppercase; color: var(--text-subtle); } +.article-references ol { margin: 0; padding-left: 20px; display: flex; flex-direction: column; gap: 5px; } +.article-references li { font-size: 0.83rem; line-height: 1.5; color: var(--text-muted); } +.article-ref-title { color: var(--text); font-weight: 600; } +.article-ref-pages { font-variant-numeric: tabular-nums; } + +@media (max-width: 640px) { + .asec-controls { gap: 8px; } + .aview { padding: 6px 10px; font-size: 0.79rem; } +} diff --git a/frontend/src/pages/ArticlesPage.jsx b/frontend/src/pages/ArticlesPage.jsx index f427fbd..0bf8749 100644 --- a/frontend/src/pages/ArticlesPage.jsx +++ b/frontend/src/pages/ArticlesPage.jsx @@ -1,49 +1,19 @@ -import { useState, useEffect, useCallback } from 'react' +import { useState, useEffect, useCallback, useMemo } from 'react' import { Link, useNavigate, useParams, useSearchParams } from 'react-router-dom' -import ReactMarkdown from 'react-markdown' -import remarkGfm from 'remark-gfm' import api from '../api/client' import { useAuth } from '../context/AuthContext' +import { SplitViewProvider } from '../context/SplitViewContext' import RichEditor from '../components/RichEditor' import CategoryColumns from '../components/CategoryColumns' -import ArticleLink, { resolveArticleId } from '../components/ArticleLink' +import { resolveArticleId } from '../components/ArticleLink' +import ArticleReader from '../components/ArticleReader' +import ArticleSplitPane from '../components/ArticleSplitPane' import CommentSection from '../components/CommentSection' import PractiseTopic from '../components/PractiseTopic' -import { markdownImageUrl } from '../utils/uploads' import './ArticlesPage.css' const sectionId = () => Array.from(crypto.getRandomValues(new Uint8Array(16)), b => b.toString(16).padStart(2, '0')).join('') -// [[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 -} - -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)} - - ) -} - function DraftBadge({ status }) { if (status === 'published') return null return Draft @@ -204,12 +174,8 @@ export function ArticlePage() { const [loading, setLoading] = useState(true) const [error, setError] = useState('') const [activeSection, setActiveSection] = useState('') - // Which sections are open. Everything starts closed: an article is a reference - // you consult, and a wall of prose hides the one heading you came for. - const [openIds, setOpenIds] = useState({}) - const [drawerOpen, setDrawerOpen] = useState(false) - // Collapsing the contents rail hands its width to the prose. - const [railOpen, setRailOpen] = useState(true) + // The cross-reference being read beside this article, by slug. + const [splitSlug, setSplitSlug] = useState(null) const [questions, setQuestions] = useState([]) const [cards, setCards] = useState([]) const [editing, setEditing] = useState(searchParams.get('edit') === '1') @@ -253,31 +219,16 @@ export function ArticlePage() { if (id === null && slug) { setError('Article not found'); setLoading(false) } }, [id, slug]) - // Deep link: open the section (and the one it sits under) and scroll to it. - // Landing on a collapsed heading would look like the link had gone nowhere. - useEffect(() => { - if (!activeSection || !article) return - const parentId = (article.sections || []).find(s => s.id === activeSection)?.parent_id - setOpenIds(prev => ({ ...prev, [activeSection]: true, ...(parentId ? { [parentId]: true } : {}) })) - const target = document.getElementById(`section-${activeSection}`) - target?.scrollIntoView?.({ block: 'start' }) - }, [activeSection, article]) - - /** Jump to a section in the page and record it in the URL for deep links. */ + /** Record the section being read in the URL, so a reader can link to it. */ const openSection = (secId) => { setActiveSection(secId) setSearchParams(secId ? { section: secId } : {}) - setDrawerOpen(false) - if (secId) { - const parentId = (article?.sections || []).find(s => s.id === secId)?.parent_id - setOpenIds(prev => ({ ...prev, [secId]: true, ...(parentId ? { [parentId]: true } : {}) })) - const target = document.getElementById(`section-${secId}`) - target?.scrollIntoView?.({ behavior: 'smooth', block: 'start' }) - } } - /** Open or close one section without moving the page. */ - const toggleSection = (secId) => setOpenIds(prev => ({ ...prev, [secId]: !prev[secId] })) + // Both readers open a cross-reference into the same slot, so one followed + // from inside the pane replaces what is there instead of splitting again. + const splitView = useMemo(() => ({ open: setSplitSlug, inPane: false }), []) + const paneView = useMemo(() => ({ open: setSplitSlug, inPane: true }), []) const save = async (publish = null) => { setSaving(true) @@ -338,47 +289,8 @@ export function ArticlePage() { const canEdit = user?.is_moderator || article.user_id === user?.id - const allSections = article.sections || [] - const childSections = allSections.filter(sec => sec.parent_id) - const kidsOf = (secId) => childSections.filter(sec => sec.parent_id === secId) - // References belong at the end whatever order they were written in — a reader - // scrolling for content should not hit the bibliography halfway down. - const isReferences = (sec) => /^references$/i.test(sec.title || '') || sec.slug === 'references' - const topSections = [ - ...allSections.filter(sec => !sec.parent_id && !isReferences(sec)), - ...allSections.filter(sec => !sec.parent_id && isReferences(sec)), - ] - const setAll = (open) => setOpenIds(open - ? Object.fromEntries(allSections.map(sec => [sec.id, true])) - : {}) - const allOpen = allSections.length > 0 && allSections.every(sec => openIds[sec.id]) - - const renderSection = (sec, depth) => { - const isOpen = !!openIds[sec.id] - const kids = kidsOf(sec.id) - const Heading = depth === 0 ? 'h2' : 'h3' - return ( -
- - - - {isOpen && ( -
- {sec.content} - {kids.map(kid => renderSection(kid, depth + 1))} -
- )} -
- ) - } - return ( -
+
) : ( -
- - -
- {article.updated_at && ( -

Last edited {new Date(article.updated_at).toLocaleDateString(undefined, { year: 'numeric', month: 'short', day: 'numeric' })}

- )} - {article.summary &&

{article.summary}

} - {article.content && {article.content}} - {!allSections.length && (!article.summary && !article.content) && ( -
Content is being prepared by educators.
- )} - - {/* Headings first, prose on request: the article opens as a contents - page you can scan, and each section expands where it sits rather - than in a modal that loses the thread. */} - {allSections.length > 0 && ( -
- -
- )} -
- {topSections.map(sec => renderSection(sec, 0))} -
- - - {cards.length > 0 && ( -
-

Related cards

- {cards.map(card => ( -
-
{card.front} → {card.back}
- Study deck +
+
+ + + + {cards.length > 0 && ( +
+

Related cards

+ {cards.map(card => ( +
+
{card.front} → {card.back}
+ Study deck +
+ ))}
- ))} -
- )} - -
+ )} + + + +
+ {splitSlug && ( + + setSplitSlug(null)} /> + + )} )} diff --git a/frontend/src/pages/EditorialPage.css b/frontend/src/pages/EditorialPage.css new file mode 100644 index 0000000..1628b1f --- /dev/null +++ b/frontend/src/pages/EditorialPage.css @@ -0,0 +1,61 @@ +/* The editorial queue: work, not inventory. */ + +.ed-page { max-width: 900px; margin: 0 auto; padding-bottom: 48px; } +.ed-header { display: flex; justify-content: space-between; align-items: flex-end; gap: 12px; flex-wrap: wrap; margin-bottom: 16px; } +.ed-header h1 { margin: 0 0 4px; font-size: 1.35rem; } +.ed-header p { margin: 0; color: var(--text-muted); font-size: 0.87rem; } + +.ed-counts { display: flex; gap: 10px; flex-wrap: wrap; margin-bottom: 18px; } +.ed-count { + flex: 1; min-width: 110px; padding: 12px 14px; background: var(--card-bg); + border: 1px solid var(--border); border-radius: 12px; + display: flex; flex-direction: column; gap: 2px; +} +.ed-count strong { font-size: 1.4rem; font-variant-numeric: tabular-nums; } +.ed-count span { font-size: 0.76rem; color: var(--text-muted); } + +.ed-error { color: var(--wrong-fg); font-size: 0.85rem; } +.ed-empty { background: var(--card-bg); border: 1px solid var(--border); border-radius: 12px; padding: 30px; text-align: center; color: var(--text-muted); } + +.ed-bucket { background: var(--card-bg); border: 1px solid var(--border); border-radius: 12px; margin-bottom: 10px; overflow: hidden; } +.ed-bucket.is-open { border-color: var(--primary); } +.ed-bucket-head { + display: flex; align-items: center; gap: 10px; width: 100%; min-height: 52px; + padding: 13px 16px; background: none; border: 0; cursor: pointer; + font: inherit; text-align: left; color: var(--text); +} +.ed-bucket-head:hover { background: var(--bg); } +.ed-bucket-title { flex: 1; min-width: 0; font-weight: 650; font-size: 0.94rem; } +.ed-bucket-count { + flex-shrink: 0; min-width: 26px; padding: 2px 9px; border-radius: 11px; + background: #fef3c7; color: #92400e; font-size: 0.76rem; font-weight: 700; + text-align: center; font-variant-numeric: tabular-nums; +} +/* An empty queue is good news and should not look like an alert. */ +.ed-bucket-count.is-clear { background: var(--correct-bg); color: var(--correct-fg); } +.ed-chevron { flex-shrink: 0; color: var(--text-muted); transition: transform 0.16s ease; } +.ed-bucket-head[aria-expanded='true'] .ed-chevron { transform: rotate(180deg); } + +.ed-bucket-body { padding: 0 16px 14px; } +.ed-blurb { margin: 0 0 10px; font-size: 0.83rem; color: var(--text-muted); } +.ed-clear { margin: 0; font-size: 0.85rem; color: var(--correct-fg); } + +.ed-list { list-style: none; margin: 0; padding: 0; } +.ed-list li { display: flex; align-items: center; gap: 8px; flex-wrap: wrap; padding: 9px 0; border-top: 1px solid var(--border); } +.ed-title { flex: 1; min-width: 160px; color: var(--text); text-decoration: none; font-size: 0.88rem; font-weight: 600; overflow-wrap: anywhere; } +.ed-title:hover { color: var(--primary); } +.ed-status, .ed-variants, .ed-generated { + flex-shrink: 0; font-size: 0.66rem; font-weight: 700; letter-spacing: 0.04em; + text-transform: uppercase; padding: 1px 8px; border-radius: 10px; + background: var(--bg); color: var(--text-muted); border: 1px solid var(--border); +} +.ed-status.is-published { background: var(--correct-bg); color: var(--correct-fg); border-color: var(--correct-bd); } +.ed-status.is-in_review { background: #fef3c7; color: #92400e; border-color: #fed7aa; } +.ed-generated { background: var(--option-sel-bg); color: var(--primary); border-color: var(--primary); } +.ed-actions { flex-shrink: 0; display: flex; gap: 6px; } + +@media (max-width: 640px) { + .ed-count { min-width: 45%; } + .ed-actions { width: 100%; } + .ed-actions .btn { flex: 1; } +} diff --git a/frontend/src/pages/EditorialPage.jsx b/frontend/src/pages/EditorialPage.jsx new file mode 100644 index 0000000..cf2c648 --- /dev/null +++ b/frontend/src/pages/EditorialPage.jsx @@ -0,0 +1,135 @@ +import { useCallback, useEffect, useState } from 'react' +import { Link } from 'react-router-dom' +import api from '../api/client' +import './EditorialPage.css' + +const apiError = (err, fallback) => { + const detail = err?.response?.data?.detail + if (typeof detail === 'string') return detail + return fallback +} + +/** + * Each bucket is something an editor can act on today. + * + * Counting articles by status would say how many exist, which is not a queue. + * These are ordered by what blocks a learner soonest: work waiting on a person, + * then machine-written drafts nobody has read, then published articles missing + * the things that make them checkable. + */ +const BUCKETS = [ + { key: 'awaiting_review', title: 'Waiting for review', + blurb: 'Someone has finished with these and asked for a second pair of eyes.' }, + { key: 'machine_drafts', title: 'Generated, unread', + blurb: 'Written from the library and never opened by a person. Not visible to learners.' }, + { key: 'published_without_references', title: 'Published without sources', + blurb: 'A claim a learner cannot check is a claim they have to take on faith.' }, + { key: 'published_without_questions', title: 'Published, nothing to practise', + blurb: 'Reading with no linked questions never appears where a learner is revising.' }, + { key: 'thin', title: 'Barely written', + blurb: 'Fewer than two sections — a stub standing where an article should be.' }, +] + +export default function EditorialPage() { + const [data, setData] = useState(null) + const [loading, setLoading] = useState(true) + const [error, setError] = useState('') + const [open, setOpen] = useState('awaiting_review') + const [busy, setBusy] = useState(false) + + const load = useCallback(() => { + setLoading(true) + api.get('/articles/editorial/queue') + .then(res => setData(res.data)) + .catch(err => setError(apiError(err, 'Could not load the editorial queue'))) + .finally(() => setLoading(false)) + }, []) + + useEffect(() => { load() }, [load]) + + const setStatus = async (article, status) => { + setBusy(true); setError('') + try { + await api.post(`/articles/${article.id}/status`, { status }) + load() + } catch (err) { setError(apiError(err, 'Could not change that')) } + finally { setBusy(false) } + } + + if (loading) return
+ if (!data) return
{error || 'Nothing to show.'}
+ + const counts = data.counts || {} + + return ( +
+
+
+

Editorial

+

What still needs a person, rather than a list of everything that exists.

+
+ Library +
+ +
+ {[['total', 'articles'], ['draft', 'drafts'], ['in_review', 'in review'], ['published', 'published']] + .map(([key, label]) => ( +
+ {counts[key] ?? 0} + {label} +
+ ))} +
+ + {error &&

{error}

} + + {BUCKETS.map(bucket => { + const rows = data[bucket.key] || [] + const isOpen = open === bucket.key + return ( +
+ + {isOpen && ( +
+

{bucket.blurb}

+ {rows.length === 0 ? ( +

Nothing here — this queue is clear.

+ ) : ( +
    + {rows.map(article => ( +
  • + {article.title} + {article.status.replace('_', ' ')} + {(article.variants || []).length > 0 && ( + {article.variants.length} views + )} + {article.generated_by && generated} + + {article.status !== 'published' && ( + + )} + {article.status === 'published' && ( + + )} + +
    • + ))} +
+ )} +
+ )} +
+ ) + })} +
+ ) +} diff --git a/frontend/src/pages/EditorialPage.test.jsx b/frontend/src/pages/EditorialPage.test.jsx new file mode 100644 index 0000000..5333862 --- /dev/null +++ b/frontend/src/pages/EditorialPage.test.jsx @@ -0,0 +1,84 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import { render, screen, waitFor, within } from '@testing-library/react' +import userEvent from '@testing-library/user-event' +import { MemoryRouter } from 'react-router-dom' +import EditorialPage from './EditorialPage' +import api from '../api/client' + +vi.mock('../api/client', () => ({ default: { get: vi.fn(), post: vi.fn() } })) + +const queue = { + counts: { total: 548, draft: 500, in_review: 3, published: 45 }, + awaiting_review: [ + { id: 1, slug: 'croup', title: 'Croup', status: 'in_review', generated_by: null, variants: ['long'] }, + ], + machine_drafts: [ + { id: 2, slug: 'intussusception', title: 'Intussusception', status: 'draft', + generated_by: 'clinical-library:claude', variants: ['long', 'key_points', 'clinical'] }, + ], + published_without_references: [], + published_without_questions: [ + { id: 3, slug: 'asthma', title: 'Asthma', status: 'published', generated_by: null, variants: ['long'] }, + ], + thin: [], +} + +const mount = () => render() + +describe('editorial queue', () => { + beforeEach(() => { vi.clearAllMocks(); api.get.mockResolvedValue({ data: queue }) }) + + it('opens on the work waiting for a person', async () => { + mount() + await screen.findByRole('heading', { name: 'Editorial' }) + const waiting = screen.getByRole('button', { name: /Waiting for review/ }) + expect(waiting).toHaveAttribute('aria-expanded', 'true') + expect(screen.getByRole('link', { name: 'Croup' })).toBeInTheDocument() + // The other buckets are counted but not unfolded on top of it. + expect(screen.queryByRole('link', { name: 'Intussusception' })).not.toBeInTheDocument() + }) + + it('shows an empty queue as good news rather than as an alert', async () => { + mount() + await screen.findByRole('heading', { name: 'Editorial' }) + const clear = screen.getByRole('button', { name: /Published without sources/ }) + expect(within(clear).getByText('0')).toHaveClass('is-clear') + + await userEvent.click(clear) + expect(screen.getByText('Nothing here — this queue is clear.')).toBeInTheDocument() + }) + + it('marks what a machine wrote, so it is not mistaken for reviewed writing', async () => { + mount() + await screen.findByRole('heading', { name: 'Editorial' }) + await userEvent.click(screen.getByRole('button', { name: /Generated, unread/ })) + const row = screen.getByRole('link', { name: 'Intussusception' }).closest('li') + expect(within(row).getByText('generated')).toBeInTheDocument() + expect(within(row).getByText('3 views')).toBeInTheDocument() + }) + + it('publishes from the queue and reloads it', async () => { + mount() + await screen.findByRole('heading', { name: 'Editorial' }) + api.post.mockResolvedValue({ data: {} }) + await userEvent.click(screen.getByRole('button', { name: 'Publish Croup' })) + await waitFor(() => expect(api.post).toHaveBeenCalledWith('/articles/1/status', { status: 'published' })) + expect(api.get).toHaveBeenCalledTimes(2) + }) + + it('offers unpublish, not publish, on something already live', async () => { + mount() + await screen.findByRole('heading', { name: 'Editorial' }) + await userEvent.click(screen.getByRole('button', { name: /Published, nothing to practise/ })) + expect(screen.getByRole('button', { name: 'Unpublish Asthma' })).toBeInTheDocument() + expect(screen.queryByRole('button', { name: 'Publish Asthma' })).not.toBeInTheDocument() + }) + + it('surfaces a refusal rather than looking like it worked', async () => { + mount() + await screen.findByRole('heading', { name: 'Editorial' }) + api.post.mockRejectedValue({ response: { data: { detail: 'Only a moderator can publish an article' } } }) + await userEvent.click(screen.getByRole('button', { name: 'Publish Croup' })) + expect(await screen.findByRole('alert')).toHaveTextContent('Only a moderator can publish') + }) +})