diff --git a/backend/app/routers/ai_mode.py b/backend/app/routers/ai_mode.py index 5273e2f..e6da9e5 100644 --- a/backend/app/routers/ai_mode.py +++ b/backend/app/routers/ai_mode.py @@ -169,9 +169,21 @@ async def ask(conversation_id: int, data: AskIn, db: Session = Depends(get_db), raise HTTPException(503, "No AI model is configured. Ask an admin to set one up.") sources = ai_mode_service.retrieve(db, current_user, question) + # How close the nearest thing in the library actually is, which decides + # which of the three answers this question gets. The shortlist alone cannot + # tell you: reciprocal-rank fusion throws the distances away and returns an + # order that is never empty, so a question about photosynthesis came back + # with six paediatric sources and an instruction to answer only from them. + similarity = ai_mode_service.closeness(db, question) + mode = ai_mode_service.answer_mode(similarity, sources) + if mode == "open": + # Nothing to cite, so nothing is offered for citation — the shortlist is + # not passed to a model that has just been told the library does not + # cover this. + sources = [] history = [{"role": m.role, "content": m.content} for m in conversation.messages[-HISTORY_TURNS:]] - messages = [{"role": "system", "content": ai_mode_service.build_prompt(sources)}, + messages = [{"role": "system", "content": ai_mode_service.build_prompt(sources, mode)}, *history, {"role": "user", "content": question}] try: diff --git a/backend/app/routers/articles.py b/backend/app/routers/articles.py index 91e3b21..889cd39 100644 --- a/backend/app/routers/articles.py +++ b/backend/app/routers/articles.py @@ -547,6 +547,11 @@ def publish_article( raise HTTPException(404, "Article not found") article.status = "published" if data.published else "draft" db.commit() + # Publication is what decides whether the body is searchable, so it is also + # what has to build or tear down the section index. Nothing else touches + # this article — an article published and never edited again would + # otherwise stay out of search for good. + article_service.reindex(db, article) return _article_json(article) diff --git a/backend/app/services/ai_mode_service.py b/backend/app/services/ai_mode_service.py index cbf74cb..47c1ec5 100644 --- a/backend/app/services/ai_mode_service.py +++ b/backend/app/services/ai_mode_service.py @@ -27,7 +27,7 @@ from app.models.flashcard import Flashcard, FlashcardDeck from app.models.question import Question from app.models.user import User from app.services.quiz_builder import bank_query, exam_scope_predicate -from app.services.search_service import hybrid_ids +from app.services.search_service import hybrid_ids, top_similarity logger = logging.getLogger(__name__) @@ -193,25 +193,98 @@ def sources_block(sources: list[dict]) -> str: return "\n\n".join(lines) -def build_prompt(sources: list[dict]) -> str: +# How close the nearest thing in the library has to be before the answer is +# treated as coming from it. Measured against this corpus with the bodies +# embedded, eight clearly on-topic questions and eight clearly off-topic ones: +# +# off-topic 0.339 – 0.499 (the French revolution … photosynthesis) +# on-topic 0.586 – 0.740 (what causes croup … posterior urethral valves) +# +# The gap between them is where these sit. They are not the retrieval floor and +# must not be: `SEMANTIC_FLOOR` decides what is worth showing in a list, where a +# weak hit costs a reader one glance. Here it decides whether an answer claims +# to come from the library, and a wrong claim costs them their trust in every +# other answer. +# +# Worth re-measuring when the corpus changes size or subject. Anything else is +# tuning by feel against numbers nobody wrote down. +STRONG_MATCH = 0.55 +ADJACENT_MATCH = 0.50 + +ROLE = "You are a study assistant for a pediatrics learning platform.\n" + +CITE = ( + "Cite with the exact marker shown, for example [[article:7]] or " + "[[section:7#abc123]], placed at the end of the sentence it supports. Never " + "write a URL and never cite a marker that is not listed here.\n\n" + "Never reveal the answer to a practice question. You may say what a question " + "is about so the learner can go and attempt it.\n\n" + "Be brief: a few sentences or a short list.\n\n" +) + + +def closeness(db: Session, query: str) -> float | None: + """How close the nearest thing in the library is, or None if unmeasurable.""" + try: + return top_similarity(db, query) + except Exception: + logger.warning("AI Mode could not measure closeness", exc_info=True) + return None + + +def answer_mode(similarity: float | None, sources: list[dict]) -> str: + """Which of the three answers this question gets: sourced, adjacent, or open. + + Decided by a number rather than by asking the model to work out which + situation it is in. Classification written as prose in a prompt is the part + that does not work, and it is also the part that makes the prompt long. + + An unmeasurable closeness — no vector database, encoder down — is not a low + one. Retrieval still found these rows by other means, and discarding them + because the ruler is missing would silently drop every citation on a + deployment where semantic search happens to be unavailable. + """ if not sources: + return "open" + if similarity is None or similarity >= STRONG_MATCH: + return "sourced" + return "adjacent" if similarity >= ADJACENT_MATCH else "open" + + +def build_prompt(sources: list[dict], mode: str = "sourced") -> str: + if mode == "open" or not sources: + # Nothing in the library is close, so the honest answer is to say that + # and then help anyway. Refusing outright was the old behaviour and it + # reads as a broken assistant rather than a careful one; answering as + # though the shortlist supported it would be worse still. return ( - "You are a study assistant for a pediatrics learning platform.\n" - "Nothing in this learner's library matches their question. Say so plainly " - "in one or two sentences and suggest what they might search for instead. " - "Do not answer from your own knowledge, and do not cite anything." + ROLE + + "Nothing in this learner's library covers their question.\n\n" + "Open with one short sentence saying so. Then answer from general " + "knowledge, briefly and plainly. Do not cite anything: there is " + "nothing here to cite, and a marker you invent points nowhere." + ) + if mode == "adjacent": + # Something related, nothing direct. Naming the gap is the point: a + # learner told "here is what is closest" can judge the answer, where one + # handed an adjacent source as though it were the answer cannot. + return ( + ROLE + + "Nothing in this learner's library covers their question directly. " + "The sources below are the closest things in it.\n\n" + "Open with one short sentence saying that, naming what the closest " + "material is about. Then answer using those sources where they help, " + "citing them, and from general knowledge where they do not — saying " + "which is which.\n\n" + + CITE + + f"SOURCES\n\n{sources_block(sources)}" ) return ( - "You are a study assistant for a pediatrics learning platform.\n\n" + ROLE + "\n" "Answer only from the sources below. They are the learner's own library — " "if they do not contain the answer, say so rather than filling the gap from " "your own knowledge, which the learner cannot check against anything.\n\n" - "Cite with the exact marker shown, for example [[article:7]] or " - "[[section:7#abc123]], placed at the end of the sentence it supports. Never " - "write a URL and never cite a marker that is not listed here.\n\n" - "Never reveal the answer to a practice question. You may say what a question " - "is about so the learner can go and attempt it.\n\n" - "Be brief: a few sentences or a short list.\n\n" + + CITE + f"SOURCES\n\n{sources_block(sources)}" ) diff --git a/backend/app/services/article_service.py b/backend/app/services/article_service.py index d714682..7fb7f24 100644 --- a/backend/app/services/article_service.py +++ b/backend/app/services/article_service.py @@ -223,6 +223,17 @@ def rebuild_section_index(db: Session, article: Article) -> int: anything that writes `Article.sections` has to call it; a copy that skipped it left 323 generated articles with no section rows at all. """ + # Only what a reader can reach. A draft is unfinished by definition, and an + # index that carries it puts half-written prose into search results and + # into the shortlist the assistant answers from. Its rows go when it is + # unpublished, and come back when it is published again — this function is + # called on every save, so the index follows the status rather than + # needing to be told about it separately. + if (article.status or "") != "published": + db.query(ArticleSectionIndex).filter( + ArticleSectionIndex.article_id == article.id).delete(synchronize_session=False) + return 0 + pending, keep = [], set() for section in article.sections or []: if not isinstance(section, dict): diff --git a/backend/app/services/search_service.py b/backend/app/services/search_service.py index 035b481..f39b81f 100644 --- a/backend/app/services/search_service.py +++ b/backend/app/services/search_service.py @@ -170,6 +170,44 @@ def _semantic_ranked(db: Session, query_text: str, pool: int, kind: str = "quest return [row.id for row in rows if float(row.similarity) >= SEMANTIC_FLOOR] +def top_similarity(db: Session, query_text: str, + kinds=("article", "article_section")) -> float | None: + """The best cosine similarity any row in these corpora has to the query. + + The one calibrated number retrieval produces. `hybrid_ids` fuses two + rankers by reciprocal rank and throws the distances away, so what comes + back is an order with no sense of scale — and an order is always non-empty + if either ranker matched anything at all. That is why a question about + photosynthesis came back with six paediatric sources and an instruction to + answer only from them. + + `None` means the question could not be asked — no vector database, or the + encoder is down — which is a different thing from "nothing is close" and + must not be collapsed into it. Zero is a real measurement of nothing. + """ + if not _is_postgres(db): + return None + embedding = _query_embedding((query_text or "").strip()) + if not embedding: + return None + literal = "[" + ",".join(str(float(value)) for value in embedding) + "]" + best = 0.0 + for kind in kinds: + if kind not in CORPORA: + continue + table, _ = CORPORA[kind] + row = db.execute(sa_text(f""" + SELECT 1 - (embedding <=> CAST(:vec AS vector)) AS similarity + FROM {table} + WHERE embedding IS NOT NULL + ORDER BY embedding <=> CAST(:vec AS vector) + LIMIT 1 + """), {"vec": literal}).fetchone() + if row is not None: + best = max(best, float(row.similarity)) + return best + + def hybrid_ids(db: Session, query_text: str, kind: str = "question", limit: int = 200) -> tuple[list[int], set[int]]: """Return (ids best-first, ids the semantic ranker contributed), for any corpus. diff --git a/backend/app/tasks/quiz_tasks.py b/backend/app/tasks/quiz_tasks.py index b365a9b..cffbcda 100644 --- a/backend/app/tasks/quiz_tasks.py +++ b/backend/app/tasks/quiz_tasks.py @@ -385,14 +385,35 @@ def retry_missing_embeddings(batch: int = 200) -> dict: embedded_total += 1 except Exception: logger.warning("Retry embedding failed for %s %s", kind, row.id, exc_info=True) - if embedded_total: + # And the other direction. A section row survives its article being + # deleted or unpublished only if something removes it, and nothing did: + # the index is rebuilt when an article is *saved*, so an article that is + # never saved again keeps its rows forever. Half-written prose then + # stays in search results and in the shortlist the assistant answers + # from, long after a reader could open the page it came from. + dropped = _drop_unreachable_sections(db) + if embedded_total or dropped: db.commit() - logger.info("Backfilled %s embeddings (%s pending)", embedded_total, pending_total) - return {"pending": pending_total, "embedded": embedded_total, "model": active} + logger.info("Backfilled %s embeddings (%s pending), dropped %s stale section rows", + embedded_total, pending_total, dropped) + return {"pending": pending_total, "embedded": embedded_total, + "dropped": dropped, "model": active} finally: db.close() +def _drop_unreachable_sections(db) -> int: + """Remove index rows whose article is gone or no longer published.""" + from sqlalchemy import select + + from app.models.article import Article, ArticleSectionIndex + + reachable = select(Article.id).where(Article.status == "published") + return (db.query(ArticleSectionIndex) + .filter(~ArticleSectionIndex.article_id.in_(reachable)) + .delete(synchronize_session=False)) + + @celery_app.task(name="regenerate_embeddings", bind=True) def regenerate_embeddings(self, job_id: str, user_id: int, stale_only: bool = True): """Re-embed questions with the current model. diff --git a/backend/tests/test_ai_mode.py b/backend/tests/test_ai_mode.py index 77a5abc..7647e5e 100644 --- a/backend/tests/test_ai_mode.py +++ b/backend/tests/test_ai_mode.py @@ -60,14 +60,42 @@ class CitationContractTests(unittest.TestCase): _, citations = ai_mode_service.enforce_citations("Here [[section:7#abc]].", self.sources()) self.assertEqual(citations[0]["section_id"], "abc") - def test_with_no_sources_the_model_is_told_to_say_so(self): - prompt = ai_mode_service.build_prompt([]) - self.assertIn("Do not answer from your own knowledge", prompt) + def test_with_nothing_close_it_says_so_and_then_helps(self): + """Refusing outright reads as a broken assistant, not a careful one. + + The old behaviour was to say nothing matched and stop. It is honest to + name the gap; it is not honest to pretend an unrelated shortlist + supports the answer, and it is not useful to withhold one entirely. + """ + prompt = ai_mode_service.build_prompt([], "open") + self.assertIn("Nothing in this learner's library covers their question", prompt) + self.assertIn("answer from general knowledge", prompt) + self.assertIn("Do not cite anything", prompt) # And nothing it writes can be cited anyway. reply, citations = ai_mode_service.enforce_citations("Anything [[article:1]].", []) self.assertEqual(citations, []) self.assertEqual(reply, "Anything.") + def test_something_adjacent_is_named_as_adjacent(self): + # `sources()` carries only what citation enforcement needs; a prompt + # also prints the text of each source. + with_text = [{**s, "text": "Body"} for s in self.sources()] + prompt = ai_mode_service.build_prompt(with_text, "adjacent") + self.assertIn("closest things", prompt) + self.assertIn("[[section:7#abc]]", prompt) # still citable + + def test_the_three_states_are_chosen_by_the_number_not_the_model(self): + sources = self.sources() + self.assertEqual(ai_mode_service.answer_mode(0.72, sources), "sourced") + self.assertEqual(ai_mode_service.answer_mode(0.52, sources), "adjacent") + # 0.49 is where "discuss love" and "photosynthesis" land against this + # corpus, alongside "tell me a joke" — noise, not adjacency. + self.assertEqual(ai_mode_service.answer_mode(0.49, sources), "open") + self.assertEqual(ai_mode_service.answer_mode(0.90, []), "open") + # Unmeasurable is not low: retrieval found these by other means, and + # dropping every citation because the ruler is missing would be worse. + self.assertEqual(ai_mode_service.answer_mode(None, sources), "sourced") + class _AiModeBase(unittest.TestCase): """Fixtures shared by the route and retrieval cases; holds no tests itself.""" diff --git a/backend/tests/test_article_search_index.py b/backend/tests/test_article_search_index.py index d17767e..814d805 100644 --- a/backend/tests/test_article_search_index.py +++ b/backend/tests/test_article_search_index.py @@ -180,10 +180,17 @@ class SectionIndexInStepTests(unittest.TestCase): return {"title": "Laryngomalacia", "slug": "laryngomalacia", "summary": "Inspiratory stridor", "sections": sections, **overrides} + def publish(self, article_id, published=True): + """Only a published article is indexed, and creating one does not publish it.""" + return self.client.post(f"/articles/{article_id}/publish", json={"published": published}) + def test_index_follows_create_edit_and_delete_of_a_section(self): first = {"id": "a" * 32, "slug": "definition", "title": "Definition", "content": "Dynamic collapse"} second = {"id": "b" * 32, "slug": "treatment", "title": "Treatment", "content": "Supraglottoplasty"} article = self.client.post('/articles/', json=self.payload([first, second])).json() + # A draft is not indexed, so nothing exists until it is published. + self.assertEqual(self.rows(article['id']), {}) + self.publish(article['id']) self.assertEqual(set(self.rows(article['id'])), {"a" * 32, "b" * 32}) self.assertEqual(self.rows(article['id'])["b" * 32][1], "Supraglottoplasty") @@ -204,7 +211,7 @@ class SectionIndexInStepTests(unittest.TestCase): article = self.client.post('/articles/', json=self.payload( sections, title="Tinea capitis", slug="tinea-capitis", summary="Scalp ringworm")).json() - self.client.post(f"/articles/{article['id']}/publish", json={'published': True}) + self.publish(article['id']) found = self.client.get('/articles/', params={'q': 'griseofulvin'}).json() self.assertEqual([a['id'] for a in found], [article['id']]) @@ -212,10 +219,28 @@ class SectionIndexInStepTests(unittest.TestCase): def test_an_orphaned_index_row_cannot_resurrect_a_deleted_article(self): sections = [{"id": "a" * 32, "slug": "treatment", "title": "Treatment", "content": "Griseofulvin"}] article = self.client.post('/articles/', json=self.payload(sections)).json() + self.publish(article['id']) self.client.patch(f"/articles/{article['id']}", json=self.payload([])) self.assertEqual(self.rows(article['id']), {}) self.assertEqual(self.client.get('/articles/', params={'q': 'griseofulvin'}).json(), []) + def test_a_draft_is_not_searchable_and_unpublishing_takes_it_back_out(self): + sections = [{"id": "a" * 32, "slug": "treatment", "title": "Treatment", + "content": "Oral griseofulvin for six to eight weeks"}] + article = self.client.post('/articles/', json=self.payload( + sections, title="Tinea capitis", slug="tinea-capitis")).json() + # Unfinished prose has no business in a search result, or in the + # shortlist the assistant answers from. + self.assertEqual(self.rows(article['id']), {}) + + self.publish(article['id']) + self.assertEqual(len(self.rows(article['id'])), 1) + + self.publish(article['id'], published=False) + self.assertEqual(self.rows(article['id']), {}, + "unpublishing left the body searchable") + if __name__ == '__main__': unittest.main() +