diff --git a/backend/alembic/versions/k1b2c3d4e5f6_article_topic_claims.py b/backend/alembic/versions/k1b2c3d4e5f6_article_topic_claims.py new file mode 100644 index 0000000..5466d0a --- /dev/null +++ b/backend/alembic/versions/k1b2c3d4e5f6_article_topic_claims.py @@ -0,0 +1,50 @@ +"""An article can claim a category, and the claim keeps its links up to date + +`question_article_links` stays the only thing anything reads. This table holds +the *reason* some of those rows exist: "the Cardiology article covers the +Cardiology questions" is a standing statement, not a snapshot of who was filed +where in September. + +Rows are materialised from a claim when it is staked, and again for a question +the moment it is filed into a claimed category. One source of truth to read, +one place that writes it. + +Revision ID: k1b2c3d4e5f6 +Revises: j0a1b2c3d4e5 +""" +import sqlalchemy as sa +from alembic import op + +revision = "k1b2c3d4e5f6" +down_revision = "j0a1b2c3d4e5" +branch_labels = None +depends_on = None + + +def upgrade() -> None: + inspector = sa.inspect(op.get_bind()) + # `Base.metadata.create_all()` runs at startup, so a fresh deploy may have + # built this already. + if "article_topic_claims" in inspector.get_table_names(): + return + op.create_table( + "article_topic_claims", + sa.Column("id", sa.Integer(), primary_key=True), + sa.Column("article_id", sa.Integer(), + sa.ForeignKey("articles.id", ondelete="CASCADE"), nullable=False), + sa.Column("category_id", sa.Integer(), + sa.ForeignKey("question_categories.id", ondelete="CASCADE"), nullable=False), + sa.Column("section_id", sa.String(length=64), nullable=True), + sa.Column("include_subtopics", sa.Boolean(), nullable=False, server_default=sa.true()), + sa.Column("user_id", sa.Integer(), + sa.ForeignKey("users.id", ondelete="SET NULL"), nullable=True), + sa.Column("created_at", sa.DateTime(), nullable=True), + sa.UniqueConstraint("article_id", "category_id", "section_id", + name="uq_article_topic_claim"), + ) + op.create_index("ix_article_topic_claims_article_id", "article_topic_claims", ["article_id"]) + op.create_index("ix_article_topic_claims_category_id", "article_topic_claims", ["category_id"]) + + +def downgrade() -> None: + op.drop_table("article_topic_claims") diff --git a/backend/app/models/__init__.py b/backend/app/models/__init__.py index 54134ba..3b254a8 100644 --- a/backend/app/models/__init__.py +++ b/backend/app/models/__init__.py @@ -8,7 +8,7 @@ from app.models.ai_model_config import AIModelConfig from app.models.favorite import Favorite from app.models.user_note import UserNote from app.models.lab_reference import LabReference, LabReferenceCardLink -from app.models.article import Article, QuestionArticleLink +from app.models.article import Article, ArticleTopicClaim, QuestionArticleLink from app.models.flashcard import FlashcardDeck, Flashcard, FlashcardDeckRating, FlashcardQuestionLink, FlashcardArticleLink from app.models.question_category import QuestionCategory, QuestionCategoryLink from app.models.collection import UserCollection, UserCollectionQuestion @@ -27,6 +27,7 @@ __all__ = [ "LabReference", "LabReferenceCardLink", "Article", + "ArticleTopicClaim", "QuestionArticleLink", "FlashcardDeck", "Flashcard", diff --git a/backend/app/models/article.py b/backend/app/models/article.py index 30891c3..ac7778e 100644 --- a/backend/app/models/article.py +++ b/backend/app/models/article.py @@ -1,6 +1,7 @@ from datetime import datetime -from sqlalchemy import Column, Integer, String, Text, JSON, DateTime, ForeignKey, UniqueConstraint +from sqlalchemy import (Boolean, Column, Integer, String, Text, JSON, DateTime, + ForeignKey, UniqueConstraint) from sqlalchemy.orm import relationship from app.database import Base @@ -63,6 +64,42 @@ class QuestionArticleLink(Base): article = relationship("Article", back_populates="links") +class ArticleTopicClaim(Base): + """An article claims a category: every question filed there links to it. + + The alternative was a one-off copy, and it was wrong for the thing people + actually do here. "The Cardiology article covers the Cardiology questions" + is a *standing* statement, not a snapshot of who was in the room in + September — and a copy quietly stops being true the first time somebody + files a new question, with nothing on any screen to say so. + + So this table holds the claim, and `question_article_links` is still the + only thing anything reads. The rows are *materialised* from the claim: + made when it is staked, and made again for a question the moment it is + filed into the category. One source of truth to read, one place that + writes it. + + A link made this way is an ordinary row and can be removed one at a time — + but re-syncing puts it back, which is the honest consequence of a standing + claim. Dropping the claim is how you stop it. + """ + + __tablename__ = "article_topic_claims" + __table_args__ = (UniqueConstraint("article_id", "category_id", "section_id", + name="uq_article_topic_claim"),) + + id = Column(Integer, primary_key=True, index=True) + article_id = Column(Integer, ForeignKey("articles.id", ondelete="CASCADE"), nullable=False, index=True) + category_id = Column(Integer, ForeignKey("question_categories.id", ondelete="CASCADE"), nullable=False, index=True) + #: Where the links land: a section of the article, or the whole of it. + section_id = Column(String(64), nullable=True) + #: Whether the claim reaches the category's subtopics, which is what an + #: educator usually means by the name of a discipline. + include_subtopics = Column(Boolean, nullable=False, default=True) + user_id = Column(Integer, ForeignKey("users.id", ondelete="SET NULL"), nullable=True) + created_at = Column(DateTime, default=datetime.utcnow) + + class ArticleRevision(Base): """A snapshot of an article as it was before a save. diff --git a/backend/app/routers/articles.py b/backend/app/routers/articles.py index cf29f4b..aaa1375 100644 --- a/backend/app/routers/articles.py +++ b/backend/app/routers/articles.py @@ -9,10 +9,10 @@ from sqlalchemy.orm import Session from app.database import get_db from app.services.search_service import article_ids_with_sections from app.models.article import ( - Article, ArticleRevision, ArticleSlug, ArticleView, + Article, ArticleTopicClaim, ArticleRevision, ArticleSlug, ArticleView, QuestionArticleLink, ) -from app.services import article_service +from app.services import article_service, topic_claims from app.models.flashcard import Flashcard, FlashcardDeck, FlashcardArticleLink from app.models.question import Question from app.models.section import Section @@ -748,30 +748,14 @@ def link_question( return {"linked": True} -#: How many questions one action may attach. A top-level discipline holds -#: hundreds, and "link Cardiology" typed by mistake should be a refusal with a -#: number in it rather than nine hundred rows to undo one at a time. -MAX_BULK_LINKS = 300 +class _ClaimLike: + """Enough of a claim to price one, before it exists.""" - -def _questions_in_category(db, user, category_id: int, include_subtopics: bool): - """Every question filed under a category, primary or additional. - - Both, because a question's primary category is one fact about it and the - extra ones are others — a question filed under Cardiology and also tagged - Emergency Medicine is in both, and an educator linking either expects it. - """ - if not db.query(QuestionCategory.id).filter(QuestionCategory.id == category_id).first(): - raise HTTPException(404, "Category not found") - ids = {category_id} - if include_subtopics: - ids = category_descendants(db.query(QuestionCategory).all(), ids) - return db.query(Question.id).filter( - bank_question_predicate(user), - or_(Question.question_category_id.in_(ids), - Question.id.in_(select(QuestionCategoryLink.question_id).where( - QuestionCategoryLink.category_id.in_(ids)))), - ).order_by(Question.id) + def __init__(self, article_id, category_id, section_id, include_subtopics): + self.article_id = article_id + self.category_id = category_id + self.section_id = section_id + self.include_subtopics = include_subtopics @router.get("/{article_id}/links/from-category") @@ -783,21 +767,45 @@ def preview_category_link( db: Session = Depends(get_db), current_user: User = Depends(require_moderator), ): - """How many questions linking this category would add, before it is done. + """How many questions claiming this topic would add, before it is claimed. - Asked separately so the button can carry the number. "Link 43 questions" - is a decision; "Link this category" is a guess, and the difference matters - when the category turns out to be the whole of Cardiology. + Asked separately so the button can carry the number. "Link 43 questions" is + a decision; "Link this topic" is a guess, and the difference matters when + the topic turns out to be the whole of Cardiology. """ article = db.get(Article, article_id) if not article: raise HTTPException(404, "Article not found") - found = {row[0] for row in _questions_in_category(db, current_user, category_id, include_subtopics)} - already = {row[0] for row in db.query(QuestionArticleLink.question_id).filter( - QuestionArticleLink.article_id == article_id, - QuestionArticleLink.section_id == section_id).all()} - return {"total": len(found), "already_linked": len(found & already), - "would_link": len(found - already), "limit": MAX_BULK_LINKS} + if not db.query(QuestionCategory.id).filter(QuestionCategory.id == category_id).first(): + raise HTTPException(404, "Category not found") + return topic_claims.preview( + db, _ClaimLike(article_id, category_id, section_id, include_subtopics)) + + +@router.get("/{article_id}/claims") +def list_claims(article_id: int, db: Session = Depends(get_db), + current_user: User = Depends(require_moderator)): + """The topics this article stands on, and how many questions each brings.""" + rows = db.query(ArticleTopicClaim).filter( + ArticleTopicClaim.article_id == article_id).order_by(ArticleTopicClaim.id).all() + names = {row.id: row.name for row in db.query(QuestionCategory).all()} + sections = {section["id"]: section.get("title") + for section in ((db.get(Article, article_id) or Article()).sections or []) + if isinstance(section, dict) and section.get("id")} + out = [] + for claim in rows: + covered = topic_claims.question_ids_in( + db, topic_claims.categories_under(db, claim.category_id, claim.include_subtopics)) + out.append({ + "id": claim.id, + "category_id": claim.category_id, + "category_name": names.get(claim.category_id, "A deleted topic"), + "section_id": claim.section_id, + "section_title": sections.get(claim.section_id) if claim.section_id else None, + "include_subtopics": claim.include_subtopics, + "questions": len(covered), + }) + return out @router.post("/{article_id}/links/from-category") @@ -807,43 +815,77 @@ def link_category( db: Session = Depends(get_db), current_user: User = Depends(require_moderator), ): - """Link every question in a category to this article, in one go. + """Claim a topic: every question in it links to this article, now and later. - Ordinary link rows, not a rule: the article does not "claim" the category, - it gains the questions that are in it now. A rule would be a second answer - to "which questions belong to this article" — one that the counts, the - QBank button, the mirror panel on each question and the AI Mode boost would - each have to learn to ask — and it would silently attach tomorrow's - questions to an article nobody has looked at since. A snapshot is what an - educator can see, check and unpick one row at a time. + Not a one-off copy. "The Cardiology article covers the Cardiology + questions" is a standing statement about the material, and a copy of it + stops being true the first time somebody files a new question — silently, + with nothing on any screen to say so. + + What is stored is the claim; what everything *reads* is still + `question_article_links`, whose rows this makes. See + `services/topic_claims.py` for why it is built that way round. """ article = db.get(Article, article_id) if not article: raise HTTPException(404, "Article not found") if data.section_id is not None and data.section_id not in _section_ids(article): raise HTTPException(400, "Section not found in this article") + if not db.query(QuestionCategory.id).filter( + QuestionCategory.id == data.category_id).first(): + raise HTTPException(404, "Category not found") - found = [row[0] for row in _questions_in_category(db, current_user, data.category_id, data.include_subtopics)] - already = {row[0] for row in db.query(QuestionArticleLink.question_id).filter( - QuestionArticleLink.article_id == article_id, - QuestionArticleLink.section_id == data.section_id).all()} - pending = [question_id for question_id in found if question_id not in already] - - if not pending: - return {"linked": 0, "skipped": len(found), "total": len(found)} - if len(pending) > MAX_BULK_LINKS: + priced = topic_claims.preview(db, _ClaimLike( + article_id, data.category_id, data.section_id, data.include_subtopics)) + if priced["would_link"] > topic_claims.MAX_LINKS_PER_CLAIM: raise HTTPException( 400, - f"That category holds {len(pending)} unlinked questions, over the " - f"{MAX_BULK_LINKS} one action may attach. Link a subtopic instead.") + f"That topic holds {priced['would_link']} unlinked questions, over the " + f"{topic_claims.MAX_LINKS_PER_CLAIM} one claim may attach at once. " + "Claim a subtopic instead.") - db.bulk_save_objects([ - QuestionArticleLink(question_id=question_id, article_id=article_id, - section_id=data.section_id, user_id=current_user.id) - for question_id in pending - ]) + claim = db.query(ArticleTopicClaim).filter_by( + article_id=article_id, category_id=data.category_id, + section_id=data.section_id).first() + if claim is None: + claim = ArticleTopicClaim( + article_id=article_id, category_id=data.category_id, + section_id=data.section_id, include_subtopics=data.include_subtopics, + user_id=current_user.id) + db.add(claim) + else: + claim.include_subtopics = data.include_subtopics + db.commit() + db.refresh(claim) + + linked = topic_claims.apply(db, claim) + return {"linked": linked, "skipped": priced["total"] - linked, + "total": priced["total"], "claim_id": claim.id} + + +@router.delete("/{article_id}/claims/{claim_id}", status_code=204) +def drop_claim(article_id: int, claim_id: int, keep_links: bool = Query(True), + db: Session = Depends(get_db), + current_user: User = Depends(require_moderator)): + """Stop claiming a topic. + + The links it made stay by default, because they are ordinary links and + somebody may have curated them since. `keep_links=false` removes the ones + this claim would make — the "undo the whole thing" case. + """ + claim = db.query(ArticleTopicClaim).filter_by(id=claim_id, article_id=article_id).first() + if claim is None: + raise HTTPException(404, "Claim not found") + if not keep_links: + covered = topic_claims.question_ids_in( + db, topic_claims.categories_under(db, claim.category_id, claim.include_subtopics)) + if covered: + db.query(QuestionArticleLink).filter( + QuestionArticleLink.article_id == article_id, + QuestionArticleLink.section_id == claim.section_id, + QuestionArticleLink.question_id.in_(covered)).delete(synchronize_session=False) + db.delete(claim) db.commit() - return {"linked": len(pending), "skipped": len(found) - len(pending), "total": len(found)} @router.delete("/{article_id}/links/{question_id}", status_code=204) diff --git a/backend/app/routers/questions.py b/backend/app/routers/questions.py index 8739f41..6c000ba 100644 --- a/backend/app/routers/questions.py +++ b/backend/app/routers/questions.py @@ -26,7 +26,7 @@ from app.models.quiz import Quiz from app.models.user import User from app.models.favorite import Favorite from app.models.folder import QuestionFolderQuestion -from app.services import article_service, file_intake, vision_service +from app.services import article_service, file_intake, topic_claims, vision_service from app.services.ai_service import get_configured_model from app.services.search_service import hybrid_ids, hybrid_question_ids, rerank_ids from app.services.question_figures import figures_for as _figures_for @@ -246,7 +246,11 @@ def set_question_category( raise HTTPException(status_code=404, detail="Category not found") question.question_category_id = category_id db.commit() - return {"question_id": question_id, "question_category_id": category_id} + # An article standing on this topic gains the question now, rather than + # when somebody next remembers to press the link button. + linked = topic_claims.link_new_question(db, question) + return {"question_id": question_id, "question_category_id": category_id, + "articles_linked": linked} @router.get("/bank/ids") @@ -541,6 +545,9 @@ def create_question_manually( db.commit() db.refresh(question) + # Any article standing on this question's topic gains it immediately. + topic_claims.link_new_question(db, question) + # Generate embedding in background try: from app.services import embedding_service @@ -610,8 +617,20 @@ def count_builder_questions( ids = [int(part) for part in (article_ids or "").split(",") if part.strip().isdigit()] tag_list = [int(part) for part in (tag_ids or "").split(",") if part.strip().isdigit()] systems = [int(part) for part in (system_ids or "").split(",") if part.strip().isdigit()] - return {"count": filtered_bank_query(db, current_user, category_ids, state, - difficulty, ids, tag_list, systems).count()} + scoped = filtered_bank_query(db, current_user, category_ids, state, + difficulty, ids, tag_list, systems) + # How many of the *other* filters' matches carry each difficulty, so the + # facet can say "Hard — none" instead of offering a choice that empties the + # bank. Counted without the difficulty filter applied, because a facet that + # counts only what it has already selected always reads zero for the rest. + by_difficulty = dict(filtered_bank_query(db, current_user, category_ids, state, + None, ids, tag_list, systems) + .with_entities(Question.difficulty, func.count(Question.id)) + .group_by(Question.difficulty).all()) + return {"count": scoped.count(), + "difficulties": {level: by_difficulty.get(level, 0) + for level in ("easy", "medium", "hard")}, + "unrated": by_difficulty.get(None, 0)} @router.post("/builder") @@ -816,7 +835,14 @@ def bulk_set_question_category( {"question_category_id": data.category_id}, synchronize_session=False ) db.commit() - return {"updated": updated, "question_category_id": data.category_id} + # Filing forty questions into a topic an article stands on links all forty, + # for the same reason filing one does. + linked = 0 + if data.category_id is not None: + for question in db.query(Question).filter(Question.id.in_(data.question_ids)).all(): + linked += topic_claims.link_new_question(db, question) + return {"updated": updated, "question_category_id": data.category_id, + "articles_linked": linked} class BulkQuestionAction(BaseModel): diff --git a/backend/app/services/topic_claims.py b/backend/app/services/topic_claims.py new file mode 100644 index 0000000..de6e860 --- /dev/null +++ b/backend/app/services/topic_claims.py @@ -0,0 +1,170 @@ +"""An article's standing claim over a category, and the links it produces. + +The question this answers, which came up the moment topic-linking existed: +*if the Cardiology article covers the Cardiology questions, does it cover the +one filed there tomorrow?* + +It does. "This article covers this topic" is a standing statement about the +material, not a snapshot of who happened to be filed where on the afternoon +somebody pressed the button — and a copy stops being true silently, with +nothing on any screen to say so. + +The shape that gets both halves right: + +* `question_article_links` stays the **only** table anything reads. Every + count, the QBank button, the mirror panel on each question, the AI Mode + curated boost — none of them learn a second question to ask. +* `article_topic_claims` records **why** some of those rows exist, and is the + one place that writes them: when the claim is staked, when a question is + filed into the category, and on a sweep that catches whatever the other two + missed. + +A link made this way is an ordinary row and can be deleted one at a time. A +sweep will put it back, which is the honest consequence of a standing claim — +dropping the claim is how you stop it, and that is a sentence the interface can +say out loud. +""" +import logging + +from sqlalchemy import or_, select +from sqlalchemy.orm import Session + +from app.models.article import Article, ArticleTopicClaim, QuestionArticleLink +from app.models.question import Question +from app.models.question_category import QuestionCategory, QuestionCategoryLink +from app.services.quiz_builder import category_descendants, general_question_predicate + +logger = logging.getLogger(__name__) + +#: How many links one claim may create in a single pass. A top-level discipline +#: holds hundreds; a runaway claim over the root of the tree should stop and be +#: reported rather than attach the whole bank to one article. +MAX_LINKS_PER_CLAIM = 300 + + +def categories_under(db: Session, category_id: int, include_subtopics: bool) -> set[int]: + if not include_subtopics: + return {category_id} + return category_descendants(db.query(QuestionCategory).all(), {category_id}) + + +def question_ids_in(db: Session, category_ids: set[int]) -> set[int]: + """Every question filed under these categories, primary or additional. + + Both, because a question's primary category is one fact about it and its + extra ones are others: a question filed under Cardiology and also tagged + Emergency Medicine is in both, and an educator claiming either expects it. + """ + rows = db.query(Question.id).filter( + general_question_predicate(), + or_(Question.question_category_id.in_(category_ids), + Question.id.in_(select(QuestionCategoryLink.question_id).where( + QuestionCategoryLink.category_id.in_(category_ids)))), + ).all() + return {row[0] for row in rows} + + +def apply(db: Session, claim: ArticleTopicClaim, *, limit: int = MAX_LINKS_PER_CLAIM) -> int: + """Make the links this claim implies. Returns how many were new.""" + wanted = question_ids_in(db, categories_under(db, claim.category_id, claim.include_subtopics)) + if not wanted: + return 0 + existing = {row[0] for row in db.query(QuestionArticleLink.question_id).filter( + QuestionArticleLink.article_id == claim.article_id, + QuestionArticleLink.section_id == claim.section_id, + QuestionArticleLink.question_id.in_(wanted)).all()} + pending = sorted(wanted - existing)[:limit] + if not pending: + return 0 + db.bulk_save_objects([ + QuestionArticleLink(question_id=question_id, article_id=claim.article_id, + section_id=claim.section_id, user_id=claim.user_id) + for question_id in pending + ]) + db.commit() + return len(pending) + + +def preview(db: Session, claim_like) -> dict: + """What staking this claim would do, before it is staked.""" + wanted = question_ids_in( + db, categories_under(db, claim_like.category_id, claim_like.include_subtopics)) + existing = {row[0] for row in db.query(QuestionArticleLink.question_id).filter( + QuestionArticleLink.article_id == claim_like.article_id, + QuestionArticleLink.section_id == claim_like.section_id).all()} if wanted else set() + return {"total": len(wanted), "already_linked": len(wanted & existing), + "would_link": len(wanted - existing), "limit": MAX_LINKS_PER_CLAIM} + + +def claims_for_category(db: Session, category_ids) -> list[ArticleTopicClaim]: + """Claims that reach any of these categories, ancestors included. + + A claim on Cardiology with subtopics covers a question filed under + Cardiology › Arrhythmias, so the lookup has to walk *up* from the question's + category, not down from the claim. + """ + if not category_ids: + return [] + parents = {row.id: row.parent_id for row in db.query(QuestionCategory).all()} + reach = set() + for category_id in category_ids: + node = category_id + seen = set() + while node is not None and node not in seen: + seen.add(node) + reach.add(node) + node = parents.get(node) + direct = set(category_ids) + rows = db.query(ArticleTopicClaim).filter(ArticleTopicClaim.category_id.in_(reach)).all() + # A claim without subtopics only covers its own category. + return [claim for claim in rows + if claim.include_subtopics or claim.category_id in direct] + + +def link_new_question(db: Session, question: Question) -> int: + """Link a question to every article whose claim now covers it. + + Called wherever a question's filing changes. Best-effort by design: a + failure here must not stop somebody saving a question, and the sweep will + catch whatever this missed. + """ + try: + categories = {question.question_category_id} if question.question_category_id else set() + categories |= {row[0] for row in db.query(QuestionCategoryLink.category_id).filter( + QuestionCategoryLink.question_id == question.id).all()} + claims = claims_for_category(db, categories) + made = 0 + for claim in claims: + exists = db.query(QuestionArticleLink.id).filter_by( + question_id=question.id, article_id=claim.article_id, + section_id=claim.section_id).first() + if exists: + continue + db.add(QuestionArticleLink(question_id=question.id, article_id=claim.article_id, + section_id=claim.section_id, user_id=claim.user_id)) + made += 1 + if made: + db.commit() + return made + except Exception: + db.rollback() + logger.warning("Could not apply topic claims to question %s", question.id, exc_info=True) + return 0 + + +def sweep(db: Session) -> dict: + """Re-apply every claim. The safety net, not the mechanism. + + Runs periodically because the two live paths — staking a claim, filing a + question — can both be bypassed: a bulk category change written in SQL, an + import, a restored article. A claim that is true should stay true without + anybody remembering to press anything. + """ + made = 0 + claims = db.query(ArticleTopicClaim).all() + alive = {row[0] for row in db.query(Article.id).filter(Article.deleted_at.is_(None)).all()} + for claim in claims: + if claim.article_id not in alive: + continue + made += apply(db, claim) + return {"claims": len(claims), "links_added": made} diff --git a/backend/app/tasks/__init__.py b/backend/app/tasks/__init__.py index 19e95cc..dc83350 100644 --- a/backend/app/tasks/__init__.py +++ b/backend/app/tasks/__init__.py @@ -25,5 +25,12 @@ celery_app.conf.beat_schedule = { "task": "retry_missing_embeddings", "schedule": 900.0, # every 15 minutes }, + # An article's claim over a topic is applied when it is staked and when a + # question is filed; this catches whatever bypassed both. Also normally + # finds nothing. + "apply-topic-claims": { + "task": "apply_topic_claims", + "schedule": 1800.0, # every 30 minutes + }, } celery_app.conf.timezone = "UTC" diff --git a/backend/app/tasks/quiz_tasks.py b/backend/app/tasks/quiz_tasks.py index 0e96f86..96e6205 100644 --- a/backend/app/tasks/quiz_tasks.py +++ b/backend/app/tasks/quiz_tasks.py @@ -351,6 +351,29 @@ def extract_quiz( db.close() +@celery_app.task(name="apply_topic_claims") +def apply_topic_claims() -> dict: + """Re-apply every article's topic claim. The safety net, not the mechanism. + + A claim is applied when it is staked and again whenever a question is filed + into the category, which covers the two ways it normally changes. Both can + be bypassed — a bulk update written in SQL, an import, an article restored + from the trash — and a claim that is true should stay true without anybody + remembering to press anything. Normally finds nothing. + """ + db = SessionLocal() + try: + from app.services import topic_claims + + result = topic_claims.sweep(db) + if result["links_added"]: + logger.info("Topic claims: %s links added across %s claims", + result["links_added"], result["claims"]) + return result + finally: + db.close() + + @celery_app.task(name="retry_missing_embeddings") def retry_missing_embeddings(batch: int = 200) -> dict: """Backfill questions that have no usable vector. @@ -585,6 +608,16 @@ def generate_flashcard_deck(self, job_id: str, section_id: int, user_id: int, #: Written into the prompt because a model that is not told about them writes #: one article and the other two tabs stay empty — which is what happened to #: every generated article until now. +#: Room for the whole thing. An article with a long view, a high-yield view and +#: a clinical one runs well past four thousand tokens, and a reply that stops +#: mid-string is not a JSON document. +ARTICLE_MAX_TOKENS = 16000 + + +class ArticleDraftError(RuntimeError): + """A refusal with a sentence the educator can act on.""" + + ARTICLE_DRAFT_PROMPT = """You write educational articles for a pediatric medical learning platform. Topic: {topic} {instructions} @@ -634,15 +667,32 @@ def generate_article_draft(self, job_id: str, user_id: int, topic: str, instructions=f"Refine this existing draft: {existing.title}\n{instructions}" if existing else instructions or "", existing=existing_block, ) + # 4,000 was the cap, and it is what broke this: the prompt asks for a + # full article *plus* a high-yield view plus a clinical one, the reply + # ran past the ceiling, and `json.loads` failed on a string the model + # never got to close — reported to the educator as "the model may need + # an 'article' configuration", which was nothing to do with it. raw = chat( model=ai_model_id, messages=[{"role": "user", "content": prompt}], - max_tokens=4000, temperature=0.4, api_key=ai_api_key).strip() + max_tokens=ARTICLE_MAX_TOKENS, temperature=0.4, api_key=ai_api_key).strip() if raw.startswith("```"): raw = raw.split("\n", 1)[1] if "\n" in raw else raw[3:] if raw.endswith("```"): raw = raw[:-3] raw = raw.strip() - data = json.loads(raw) + try: + data = json.loads(raw) + except json.JSONDecodeError as broken: + # A truncated reply is a different problem from a badly configured + # one, and telling an educator to check their model settings when + # the model worked fine is how an afternoon is wasted. + truncated = not raw.rstrip().endswith("}") + raise ArticleDraftError( + "The model's reply was cut off before it finished the article. " + "Try a narrower topic, or ask for fewer sections in the instructions." + if truncated else + f"The model did not return usable JSON ({broken})." + ) from broken title = str(data.get("title", topic)).strip()[:300] slug = re.sub(r"[^a-z0-9]+", "-", str(data.get("slug", topic)).strip().lower()).strip("-")[:120] or "topic" sections = [] @@ -684,11 +734,16 @@ def generate_article_draft(self, job_id: str, user_id: int, topic: str, article_service.reindex(db, article) r.set(f"extraction:status:{job_id}", "completed", ex=EXPIRE_SECONDS) _push_step(r, job_id, "done", f"Draft saved: {title}") + except ArticleDraftError as refusal: + logger.warning("Article draft job %s: %s", job_id, refusal) + r.set(f"extraction:status:{job_id}", "failed", ex=EXPIRE_SECONDS) + r.set(f"extraction:error:{job_id}", str(refusal)[:300], ex=EXPIRE_SECONDS) + _push_step(r, job_id, "error", str(refusal)[:300]) except Exception as exc: logger.warning("Article draft job %s failed: %s", job_id, exc) r.set(f"extraction:status:{job_id}", "failed", ex=EXPIRE_SECONDS) r.set(f"extraction:error:{job_id}", str(exc)[:300], ex=EXPIRE_SECONDS) - _push_step(r, job_id, "error", "Drafting failed; the model may need an 'article' configuration.") + _push_step(r, job_id, "error", f"Drafting failed: {str(exc)[:200]}") finally: db.close() diff --git a/backend/tests/test_articles_cards.py b/backend/tests/test_articles_cards.py index 4181650..b7bb614 100644 --- a/backend/tests/test_articles_cards.py +++ b/backend/tests/test_articles_cards.py @@ -101,6 +101,34 @@ class ArticlesCardsTests(unittest.TestCase): json={'category_id': 2, 'section_id': section_id}).json() self.assertEqual(deeper['linked'], preview['total']) + # The claim is standing, not a copy. A question filed into the topic + # afterwards is linked without anybody pressing anything again, which + # is the whole difference between "covers this topic" and "covered it + # in September". + from app.models.question import Question + newcomer = Question(question_category_id=2, user_id=3, question_text="Filed later", + question_type="mcq", options=["yes", "no"], correct_answer="yes") + self.bank.db.add(newcomer) + self.bank.db.commit() + # Two links, because by now there are two claims on this topic — one on + # the whole article and one on its first section — and a claim means + # what it says wherever it points. + self.assertEqual(self.client.post('/questions/bulk-category', json={ + 'question_ids': [newcomer.id], 'category_id': 2}).json()['articles_linked'], 2) + linked_now = self.client.get(f"/articles/{article_id}/questions").json() + self.assertIn(newcomer.id, [row['question_id'] for row in linked_now]) + + # And the claim is visible, with a way to stop. + claims = self.client.get(f"/articles/{article_id}/claims").json() + self.assertEqual([c['category_id'] for c in claims], [2, 2]) + self.assertTrue(all(c['include_subtopics'] for c in claims)) + whole_article = next(c for c in claims if c['section_id'] is None) + self.client.delete(f"/articles/{article_id}/claims/{whole_article['id']}", + params={'keep_links': 'false'}) + left = self.client.get(f"/articles/{article_id}/questions").json() + # The section-scoped claim's links survive; the whole-article ones went. + self.assertTrue(all(row['section_id'] for row in left)) + self.assertEqual(self.client.post(f"/articles/{article_id}/links/from-category", json={'category_id': 999}).status_code, 404) self.assertEqual(self.client.post(f"/articles/{article_id}/links/from-category", diff --git a/frontend/src/components/ArticleQuestions.jsx b/frontend/src/components/ArticleQuestions.jsx index b0ff82d..d3b649a 100644 --- a/frontend/src/components/ArticleQuestions.jsx +++ b/frontend/src/components/ArticleQuestions.jsx @@ -38,11 +38,16 @@ export default function ArticleQuestions({ articleId, sections }) { //: What linking it would actually do, asked before it is done — so the //: button carries the number rather than the educator guessing at it. const [preview, setPreview] = useState(null) + //: The topics this article stands on. A claim is standing, not a copy: a + //: question filed into one of these tomorrow is linked tomorrow. + const [claims, setClaims] = useState([]) const load = useCallback(() => { if (!articleId) return api.get(`/articles/${articleId}/questions`) .then(res => setLinks(res.data || [])).catch(() => setLinks([])) + api.get(`/articles/${articleId}/claims`) + .then(res => setClaims(res.data || [])).catch(() => setClaims([])) }, [articleId]) useEffect(() => { load() }, [load]) @@ -97,14 +102,27 @@ export default function ArticleQuestions({ articleId, sections }) { }) const { linked, skipped } = res.data setNote(linked - ? `Linked ${linked} question${linked === 1 ? '' : 's'}${skipped ? `, ${skipped} already here` : ''}.` - : 'Every question in that topic is already linked here.') + ? `Linked ${linked} question${linked === 1 ? '' : 's'}${skipped ? `, ${skipped} already here` : ''}. Anything filed there later is linked automatically.` + : 'Every question in that topic is already linked. The topic is now claimed, so new ones follow automatically.') setCategoryId(''); setPreview(null) load() } catch (err) { setError(apiError(err, 'Could not link that topic')) } finally { setBusy(false) } } + const dropClaim = async (claim, keepLinks) => { + setBusy(true); setError(''); setNote('') + try { + await api.delete(`/articles/${articleId}/claims/${claim.id}`, + { params: { keep_links: keepLinks } }) + setNote(keepLinks + ? `Stopped following ${claim.category_name}. The links it made are still here.` + : `Stopped following ${claim.category_name} and removed its links.`) + load() + } catch (err) { setError(apiError(err, 'Could not drop that topic')) } + finally { setBusy(false) } + } + const unlink = async (row) => { setBusy(true); setError(''); setNote('') try { @@ -148,6 +166,32 @@ export default function ArticleQuestions({ articleId, sections }) { disabled={busy} search={search} onPick={row => { setError(''); setNote(''); setSectionId(''); setChosen(row) }} /> )} + {claims.length > 0 && ( +
+ Every question in these is linked, including ones filed there later. +
+- Adds the questions in that topic as ordinary links, which you can remove one - at a time. Questions filed there later are not added. + The article follows the topic: every question in it is linked + now, and anything filed there later is linked too. Individual links can still + be removed by hand, and you can stop following a topic at any time.