From 1c1f327d4a54f41c5fefe983844f3426d08fe85c Mon Sep 17 00:00:00 2001 From: Daniel Date: Sat, 12 Sep 2026 03:45:51 +0200 Subject: [PATCH] feat: the chat can put you into practice, and Settings stops crying wolf MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit AI Mode could cite an article and link to it; it could not do the other half of the job. POST /ai/conversations/{id}/practice turns an answer into a study session, built from what that answer actually cited: a question it named first, then questions filed under the category of an article it named, then retrieval on the learner's own words. Everything goes through the bank's visibility rules on the way out — a chat is not a route to questions a learner could not otherwise reach. Study mode, never exam: this is reading followed by practice, not a paper. Two false alarms on the Settings page, both visible in a screenshot: The STT test called /model/info on the LiteLLM proxy. Our virtual key is scoped to llm_api_routes and cannot, so a working transcription model reported a red 403. It now falls back to /v1/models, which the key may call, and says plainly that the proxy would not confirm what the model is for — presence, not suitability. And the TTS test raised a 400 carrying an instruction ("use the Preview button"), which the page rendered in red with a ✗. That is not a failure. It answers, and Preview stays the way to hear a voice. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01TqXevQJhxFrM7jJg82cgZN --- backend/app/routers/admin.py | 74 +++++++++++++-------- backend/app/routers/ai_mode.py | 48 ++++++++++++++ backend/app/services/ai_mode_service.py | 56 ++++++++++++++++ backend/tests/test_ai_practice.py | 85 +++++++++++++++++++++++++ frontend/src/pages/AiModePage.css | 11 ++++ frontend/src/pages/AiModePage.jsx | 33 +++++++++- frontend/src/pages/AiModePage.test.jsx | 15 +++++ 7 files changed, 294 insertions(+), 28 deletions(-) create mode 100644 backend/tests/test_ai_practice.py diff --git a/backend/app/routers/admin.py b/backend/app/routers/admin.py index b6c1614..b97c779 100644 --- a/backend/app/routers/admin.py +++ b/backend/app/routers/admin.py @@ -146,6 +146,34 @@ class LiteLLMSearchRequest(BaseModel): mode: str | None = None +def _proxy_models(base: str, key: str | None, mode: str | None = None) -> tuple[list[str], bool]: + """What the proxy will serve, and whether the answer knows about modes. + + `/model/info` carries each model's mode — transcription, chat, speech — and + is the right question to ask. A virtual key scoped to `llm_api_routes` + cannot call it, which is how a working proxy came to report a red 403 on + the settings page. `/v1/models` is on that allowed list and answers with + ids only, so the fallback can say a model is there but not what it is for. + """ + headers = {"Authorization": f"Bearer {key}"} if key else {} + root = base.rstrip("/").removesuffix("/v1") + try: + resp = httpx.get(f"{root}/model/info", headers=headers, timeout=10) + resp.raise_for_status() + rows = resp.json().get("data", []) + return sorted({ + row.get("model_name") for row in rows + if row.get("model_name") + and (mode is None or (row.get("model_info") or {}).get("mode") == mode) + }), True + except httpx.HTTPStatusError as err: + if err.response.status_code not in (401, 403, 404): + raise + resp = httpx.get(f"{root}/v1/models", headers=headers, timeout=10) + resp.raise_for_status() + return sorted(m["id"] for m in resp.json().get("data", [])), False + + @router.post("/litellm/models") def search_litellm_models( data: LiteLLMSearchRequest, @@ -159,20 +187,12 @@ def search_litellm_models( if base: try: - headers = {"Authorization": f"Bearer {key}"} if key else {} - if data.mode: - info_base = base.rstrip("/").removesuffix("/v1") - resp = httpx.get(f"{info_base}/model/info", headers=headers, timeout=10) - resp.raise_for_status() - models = sorted([ - m.get("model_name") for m in resp.json().get("data", []) - if m.get("model_name") and (m.get("model_info") or {}).get("mode") == data.mode - ]) - return {"models": models, "source": info_base, "mode": data.mode} - resp = httpx.get(f"{base}/v1/models", headers=headers, timeout=10) - resp.raise_for_status() - models = sorted([m["id"] for m in resp.json().get("data", [])]) - return {"models": models, "source": base} + models, by_mode = _proxy_models(base, key, data.mode) + return {"models": models, "source": base, + "mode": data.mode if by_mode else None, + # Said rather than implied: an unfiltered list looks like a + # filtered one that found everything. + "filtered": by_mode and bool(data.mode)} except Exception as e: log.warning(f"LiteLLM model search failed: {e}") raise HTTPException(status_code=400, detail=f"Failed to query models API: {e}") @@ -288,7 +308,10 @@ def test_model( raise HTTPException(status_code=404, detail="Model config not found") if model.task == "tts": - raise HTTPException(status_code=400, detail="Use the Preview button to test TTS voices — it plays audio directly.") + # Not a failure. A voice is tested by hearing it, and the page offers + # that; saying so in red taught administrators to distrust a working + # configuration. + return {"message": f"{model.model_id} is a voice — press Preview to hear it."} if model.task == "stt": base = (settings.LITELLM_API_BASE or "").rstrip("/").removesuffix("/v1") @@ -296,16 +319,17 @@ def test_model( if not base: raise HTTPException(status_code=400, detail="LiteLLM API base is not configured") try: - headers = {"Authorization": f"Bearer {key}"} if key else {} - resp = httpx.get(f"{base}/model/info", headers=headers, timeout=10) - resp.raise_for_status() - found = any( - m.get("model_name") == model.model_id and (m.get("model_info") or {}).get("mode") == "audio_transcription" - for m in resp.json().get("data", []) - ) - if not found: - raise HTTPException(status_code=404, detail=f"{model.model_id} was not found as an audio_transcription model in LiteLLM") - return {"message": f"✓ {model.model_id} is available for speech transcription"} + models, by_mode = _proxy_models(base, key, "audio_transcription") + if model.model_id not in models: + raise HTTPException( + status_code=404, + detail=f"{model.model_id} is not served by the proxy" + + (" as a transcription model" if by_mode else "")) + return {"message": f"✓ {model.model_id} is served by the proxy" + ( + " for speech transcription" if by_mode + # The route that says what a model is for is not open to this + # key, so this is presence, not suitability. + else " — the proxy would not say whether it transcribes")} except HTTPException: raise except Exception as e: diff --git a/backend/app/routers/ai_mode.py b/backend/app/routers/ai_mode.py index dd9efa1..e8bbba1 100644 --- a/backend/app/routers/ai_mode.py +++ b/backend/app/routers/ai_mode.py @@ -17,6 +17,7 @@ from app.models.conversation import Conversation, ConversationMessage from app.models.user import User from app.services import ai_mode_service from app.services.ai_service import get_model_for_task +from app.services.quiz_builder import GenerateTestRequest, generate_test from app.utils.auth import check_rate_limit, get_current_user router = APIRouter() @@ -94,6 +95,53 @@ class AskIn(BaseModel): message: str = Field(min_length=1, max_length=2000) +class PracticeIn(BaseModel): + """Which turn to practise. Absent means the latest answer in the thread.""" + message_id: int | None = None + + +@router.post("/conversations/{conversation_id}/practice") +def practice(conversation_id: int, data: PracticeIn, db: Session = Depends(get_db), + current_user: User = Depends(get_current_user)): + """Turn a chat turn into a study session. + + The chat is for finding out what you do not know; the point of finding out + is to go and practise it. This is the step between — it builds a session + from what the answer actually cited, so the questions follow from the + conversation rather than from the topic in general. + + Study mode, never exam: this is reading followed by practice, not a paper. + """ + conversation = _own(db, conversation_id, current_user) + answer = None + if data.message_id: + answer = next((m for m in conversation.messages if m.id == data.message_id), None) + if answer is None: + raise HTTPException(404, "That message is not in this conversation") + else: + answer = next((m for m in reversed(conversation.messages) if m.role == "assistant"), None) + if answer is None: + raise HTTPException(400, "There is nothing to practise in this conversation yet") + + # What was asked, as the learner put it — the turn before the answer. + asked = "" + for message in conversation.messages: + if message.id == answer.id: + break + if message.role == "user": + asked = message.content + + ids = ai_mode_service.practice_ids(db, current_user, answer.citations or [], asked) + if not ids: + raise HTTPException(404, "No questions in your bank match this conversation yet") + + title = (conversation.title or "AI Mode")[:180] + quiz = generate_test(db, current_user, GenerateTestRequest( + title=f"{title} — practice", category_ids=[], state="all", + count=len(ids), mode="learning", explicit_ids=ids)) + return {"quiz_id": quiz["id"], "count": len(ids)} + + @router.post("/conversations/{conversation_id}/messages") async def ask(conversation_id: int, data: AskIn, db: Session = Depends(get_db), current_user: User = Depends(get_current_user)): diff --git a/backend/app/services/ai_mode_service.py b/backend/app/services/ai_mode_service.py index 4bc87e4..7eaa266 100644 --- a/backend/app/services/ai_mode_service.py +++ b/backend/app/services/ai_mode_service.py @@ -244,3 +244,59 @@ def enforce_citations(reply: str, sources: list[dict]) -> tuple[str, list[dict]] "curated": bool(s.get("curated")), } for s in used.values()] return cleaned, citations + + +#: A session assembled from a conversation. Enough to be worth sitting, few +#: enough that it follows from what was just discussed rather than becoming a +#: general exam on the topic. +PRACTICE_MAX = 20 + + +def practice_ids(db: Session, user: User, citations, question: str) -> list[int]: + """Which questions a chat turn should send a learner to practise. + + Three sources, in the order they deserve trust. A question the answer + actually cited is the closest thing to "this is what we were talking + about". An article it cited stands for a topic, so questions filed under + that article's category follow. Retrieval on the learner's own words + catches the rest. + + Everything is put through the bank's own visibility rules on the way out — + a chat is not a way to reach questions a learner could not otherwise see. + """ + from app.models.question_category import QuestionCategoryLink + + cited_questions = [int(c["id"]) for c in (citations or []) if c.get("kind") == "question"] + article_ids = [int(c["id"]) for c in (citations or []) if c.get("kind") == "article"] + + from_articles: list[int] = [] + if article_ids: + categories = [cid for (cid,) in db.query(Article.category_id).filter( + Article.id.in_(article_ids), Article.category_id.isnot(None)).all()] + if categories: + direct = db.query(Question.id).filter( + Question.question_category_id.in_(categories)).limit(PRACTICE_MAX * 3).all() + linked = db.query(QuestionCategoryLink.question_id).filter( + QuestionCategoryLink.category_id.in_(categories)).limit(PRACTICE_MAX * 3).all() + from_articles = [qid for (qid,) in [*direct, *linked]] + + retrieved: list[int] = [] + try: + retrieved, _ = hybrid_ids(db, question or "", "question", limit=PRACTICE_MAX * 2) + except Exception: + logger.warning("Practice retrieval failed", exc_info=True) + + ordered: list[int] = [] + for group in (cited_questions, from_articles, retrieved): + for qid in group: + if qid not in ordered: + ordered.append(qid) + if not ordered: + return [] + + allowed = bank_query(db, user).filter(Question.id.in_(ordered)) + scope = exam_scope_predicate(db, user) + if scope is not None: + allowed = allowed.filter(scope) + visible = {row.id for row in allowed.all()} + return [qid for qid in ordered if qid in visible][:PRACTICE_MAX] diff --git a/backend/tests/test_ai_practice.py b/backend/tests/test_ai_practice.py new file mode 100644 index 0000000..570c6e3 --- /dev/null +++ b/backend/tests/test_ai_practice.py @@ -0,0 +1,85 @@ +"""Turning a chat turn into a session. + +The chat is for finding out what you do not know; the point of finding out is +to go and practise it. What is under test is which questions that lands on, +and that a chat cannot reach a question the learner could not otherwise see. +""" +import unittest +from unittest.mock import patch + +import test_quiz_builder as fixtures +from app.models.conversation import Conversation, ConversationMessage +from app.routers import ai_mode + + +class PracticeFromChatTests(unittest.TestCase): + def setUp(self): + self.bank = fixtures.BuilderTests() + self.bank.setUp() + self.client = self.bank.client + self.db = self.bank.db + self.client.app.include_router(ai_mode.router, prefix='/ai') + + def tearDown(self): + self.bank.tearDown() + + def thread(self, citations, asked='What causes stridor?'): + conversation = Conversation(user_id=1, title='Stridor') + self.db.add(conversation) + self.db.flush() + self.db.add(ConversationMessage(conversation_id=conversation.id, role='user', + content=asked, citations=[])) + answer = ConversationMessage(conversation_id=conversation.id, role='assistant', + content='Croup, mostly.', citations=citations) + self.db.add(answer) + self.db.commit() + return conversation.id, answer.id + + def practise(self, conversation_id, **body): + with patch('app.services.search_service.hybrid_ids', return_value=([], {})): + return self.client.post(f'/ai/conversations/{conversation_id}/practice', json=body) + + def test_a_cited_question_is_the_session(self): + cid, _ = self.thread([{'marker': '[[question:1]]', 'kind': 'question', 'id': 1, + 'title': 'Question #1'}]) + response = self.practise(cid) + self.assertEqual(response.status_code, 200, response.text) + self.assertEqual(response.json()['count'], 1) + quiz = self.client.get(f"/quizzes/{response.json()['quiz_id']}").json() + self.assertEqual([q['id'] for q in quiz['questions']], [1]) + # Study mode, never exam: this is reading followed by practice. + self.assertEqual(quiz['mode'], 'learning') + + def test_a_cited_article_brings_the_questions_filed_under_its_topic(self): + from app.models.article import Article + # The fixture files questions 1 and 2 under categories 1 and 2. + self.db.add(Article(id=5, title='Croup', slug='croup', content='…', + status='published', user_id=3, category_id=2)) + self.db.commit() + cid, _ = self.thread([{'marker': '[[article:5]]', 'kind': 'article', 'id': 5, + 'title': 'Croup'}]) + response = self.practise(cid) + self.assertEqual(response.status_code, 200, response.text) + quiz = self.client.get(f"/quizzes/{response.json()['quiz_id']}").json() + self.assertEqual([q['id'] for q in quiz['questions']], [2]) + + def test_a_chat_cannot_reach_a_question_the_learner_may_not_see(self): + # Question 4 belongs to another learner and is not shared. A citation + # could only name it by mistake; the bank's own rules answer anyway. + cid, _ = self.thread([{'marker': '[[question:4]]', 'kind': 'question', 'id': 4, + 'title': 'Question #4'}]) + self.assertEqual(self.practise(cid).status_code, 404) + + def test_nothing_to_practise_says_so(self): + cid, _ = self.thread([]) + response = self.practise(cid) + self.assertEqual(response.status_code, 404) + self.assertIn('No questions', response.json()['detail']) + + def test_one_turn_can_be_named_and_another_learner_cannot_ask(self): + cid, answer_id = self.thread([{'marker': '[[question:1]]', 'kind': 'question', + 'id': 1, 'title': 'Question #1'}]) + self.assertEqual(self.practise(cid, message_id=answer_id).status_code, 200) + self.assertEqual(self.practise(cid, message_id=99999).status_code, 404) + self.bank.user = self.bank.peer + self.assertEqual(self.practise(cid).status_code, 404) diff --git a/frontend/src/pages/AiModePage.css b/frontend/src/pages/AiModePage.css index 5aa0d00..9e4ac7d 100644 --- a/frontend/src/pages/AiModePage.css +++ b/frontend/src/pages/AiModePage.css @@ -93,3 +93,14 @@ .ai-rail-toggle { display: inline-block; } .ai-msg.is-user { max-width: 88%; } } + +/* Reading, then practice. Quiet next to the answer — an offer, not the point + of the page. */ +.ai-practise { + margin-top: 12px; padding: 7px 13px; font: inherit; font-size: 0.82rem; + font-weight: 600; cursor: pointer; border-radius: 20px; + background: var(--card-bg); color: var(--primary); + border: 1px solid var(--primary); +} +.ai-practise:hover:not(:disabled) { background: var(--option-sel-bg); } +.ai-practise:disabled { opacity: 0.6; cursor: default; } diff --git a/frontend/src/pages/AiModePage.jsx b/frontend/src/pages/AiModePage.jsx index 96df33a..77ea14c 100644 --- a/frontend/src/pages/AiModePage.jsx +++ b/frontend/src/pages/AiModePage.jsx @@ -1,5 +1,5 @@ import { useCallback, useEffect, useRef, useState } from 'react' -import { Link } from 'react-router-dom' +import { Link, useNavigate } from 'react-router-dom' import ReactMarkdown from 'react-markdown' import remarkGfm from 'remark-gfm' import api from '../api/client' @@ -29,7 +29,7 @@ export const citationHref = (citation) => { * that reaches here has a source behind it. Numbering rather than inlining the * title keeps a sentence readable when it rests on three sources. */ -function Answer({ content, citations }) { +function Answer({ content, citations, onPractise, practising }) { const index = new Map(citations.map((c, i) => [c.marker, i + 1])) // The match swallows the space before the marker, so the number replaces it // rather than following it and leaving a double gap. @@ -40,6 +40,14 @@ function Answer({ content, citations }) { return (
{numbered} + {/* The chat is for finding out what you do not know; the point of + finding out is to go and practise it. */} + {onPractise && ( + + )} {citations.length > 0 && (
    {citations.map((citation, i) => ( @@ -74,7 +82,10 @@ export default function AiModePage() { const [loading, setLoading] = useState(true) const [error, setError] = useState('') const [railOpen, setRailOpen] = useState(false) + // Which answer is being turned into a session, if any. + const [practising, setPractising] = useState(null) const endRef = useRef(null) + const navigate = useNavigate() const loadThreads = useCallback(() => api.get('/ai/conversations') .then(res => { setThreads(res.data || []); return res.data || [] }) @@ -117,6 +128,20 @@ export default function AiModePage() { } catch (err) { setError(apiError(err, 'Could not delete that conversation')) } } + /** Sit the questions this answer rests on, rather than reading about them. */ + const practise = async (messageId) => { + setPractising(messageId) + setError('') + try { + const res = await api.post(`/ai/conversations/${activeId}/practice`, { message_id: messageId }) + navigate(`/study/${res.data.quiz_id}?start=1`) + } catch (err) { + setError(apiError(err, 'Could not build a session from this answer')) + } finally { + setPractising(null) + } + } + const send = async (event) => { event?.preventDefault?.() const text = draft.trim() @@ -190,7 +215,9 @@ export default function AiModePage() {
    {message.role === 'user' ?

    {message.content}

    - : } + : practise(message.id)} />}
    ))} {sending && ( diff --git a/frontend/src/pages/AiModePage.test.jsx b/frontend/src/pages/AiModePage.test.jsx index e0d631f..7f23ffd 100644 --- a/frontend/src/pages/AiModePage.test.jsx +++ b/frontend/src/pages/AiModePage.test.jsx @@ -117,4 +117,19 @@ describe('AI Mode', () => { expect(citationHref({ kind: 'card', id: 3 })).toBe('/flashcards') expect(citationHref({ kind: 'question', id: 9 })).toBe('/questions/9') }) + + it('turns an answer into a session, and says so when there is nothing to sit', async () => { + mockApi(threads, [answer]) + api.post.mockResolvedValue({ data: { quiz_id: 91, count: 6 } }) + mount() + await userEvent.click(await screen.findByRole('button', { name: 'Febrile seizures' })) + await userEvent.click(await screen.findByRole('button', { name: '▶ Practise this' })) + await waitFor(() => expect(api.post).toHaveBeenCalledWith( + '/ai/conversations/1/practice', { message_id: 22 })) + + // Nothing in the bank matches: said plainly, not as a broken button. + api.post.mockRejectedValue({ response: { data: { detail: 'No questions in your bank match this conversation yet' } } }) + await userEvent.click(screen.getByRole('button', { name: '▶ Practise this' })) + expect(await screen.findByText(/No questions in your bank match/)).toBeInTheDocument() + }) })