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