feat: the chat can put you into practice, and Settings stops crying wolf
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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TqXevQJhxFrM7jJg82cgZN
This commit is contained in:
Daniel
parent
bc77ba83ae
commit
1c1f327d4a
7 changed files with 294 additions and 28 deletions
|
|
@ -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:
|
||||
|
|
|
|||
|
|
@ -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)):
|
||||
|
|
|
|||
|
|
@ -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]
|
||||
|
|
|
|||
85
backend/tests/test_ai_practice.py
Normal file
85
backend/tests/test_ai_practice.py
Normal file
|
|
@ -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)
|
||||
|
|
@ -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; }
|
||||
|
|
|
|||
|
|
@ -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 (
|
||||
<div className="ai-answer">
|
||||
<ReactMarkdown remarkPlugins={[remarkGfm]}>{numbered}</ReactMarkdown>
|
||||
{/* The chat is for finding out what you do not know; the point of
|
||||
finding out is to go and practise it. */}
|
||||
{onPractise && (
|
||||
<button type="button" className="ai-practise" disabled={practising}
|
||||
onClick={onPractise}>
|
||||
{practising ? 'Building a session…' : '▶ Practise this'}
|
||||
</button>
|
||||
)}
|
||||
{citations.length > 0 && (
|
||||
<ol className="ai-sources">
|
||||
{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() {
|
|||
<div key={message.id} className={`ai-msg is-${message.role}`}>
|
||||
{message.role === 'user'
|
||||
? <p>{message.content}</p>
|
||||
: <Answer content={message.content} citations={message.citations || []} />}
|
||||
: <Answer content={message.content} citations={message.citations || []}
|
||||
practising={practising === message.id}
|
||||
onPractise={() => practise(message.id)} />}
|
||||
</div>
|
||||
))}
|
||||
{sending && (
|
||||
|
|
|
|||
|
|
@ -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()
|
||||
})
|
||||
})
|
||||
|
|
|
|||
Loading…
Reference in a new issue