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') + }) +})