diff --git a/backend/app/routers/questions.py b/backend/app/routers/questions.py index 4e61079..fe2cca5 100644 --- a/backend/app/routers/questions.py +++ b/backend/app/routers/questions.py @@ -528,12 +528,15 @@ def count_builder_questions( difficulty: Literal["easy", "medium", "hard"] | None = Query(None), article_ids: str | None = Query(None, description="Comma-separated article IDs (OR filter)"), tag_ids: str | None = Query(None, description="Comma-separated tag IDs (AND filter)"), + system_ids: str | None = Query(None, description="Comma-separated organ system IDs (OR filter)"), db: Session = Depends(get_db), current_user: User = Depends(get_current_user), ): 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()] - return {"count": filtered_bank_query(db, current_user, category_ids, state, is_shared, difficulty, ids, tag_list).count()} + 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, is_shared, + difficulty, ids, tag_list, systems).count()} @router.post("/builder") diff --git a/backend/app/routers/study_tools.py b/backend/app/routers/study_tools.py index 1661022..e6c7de9 100644 --- a/backend/app/routers/study_tools.py +++ b/backend/app/routers/study_tools.py @@ -5,7 +5,8 @@ from typing import Literal from fastapi import APIRouter, Depends, HTTPException, Query from pydantic import BaseModel, Field, HttpUrl, field_validator -from sqlalchemy import func, or_ +from sqlalchemy import func, inspect, or_ +from sqlalchemy import text as sa_text from sqlalchemy.orm import Session from app.models.exam import Exam @@ -239,7 +240,7 @@ def _category_rollup(categories): @router.get("/recommendations") def study_recommendations( - group: Literal["systems", "subtopics"] = "systems", + group: Literal["articles", "disciplines", "systems"] = "disciplines", limit: int = Query(20, ge=1, le=60), db: Session = Depends(get_db), user: User = Depends(get_current_user), @@ -291,58 +292,130 @@ def study_recommendations( rolled.update(ancestry.get(cid, [cid])) return rolled + # An article is reached through the category it is filed under. + articles: dict[int, Article] = {} + for article in db.query(Article).filter(Article.status == "published", + Article.category_id.isnot(None)).all(): + articles.setdefault(article.category_id, article) + + # A question reaches an organ system through the symptom keyword it carries, + # which is filed under one. No question is tagged with a system directly. + # Raw SQL because question_tags has no ORM model; the tags router is the + # same, and one spelling of this table is better than two. Only read when + # the systems grouping is asked for — and only when the table is there, + # since it is not part of the ORM metadata a fresh database is built from. + system_names: dict[int, str] = {} + systems_of_question: dict[int, set[int]] = defaultdict(set) + if group == "systems" and inspect(db.get_bind()).has_table("question_tags"): + for tag_id, name in db.execute(sa_text( + "SELECT id, name FROM question_tags WHERE type = 'system' AND parent_id IS NULL")).all(): + system_names[tag_id] = name + if system_names: + for question_id, parent in db.execute(sa_text(''' + SELECT l.question_id, t.parent_id + FROM question_tag_links l + JOIN question_tags t ON t.id = l.tag_id + WHERE t.parent_id IS NOT NULL + ''')).all(): + if parent in system_names: + systems_of_question[question_id].add(parent) + + # Three ways to ask the same question of the same answers: which reading to + # go back to, which discipline is weak, which organ system is weak. The + # scoring below does not care which; only the membership differs. + def groups_for(question_id, primary) -> set[int]: + if group == "systems": + return systems_of_question.get(question_id, set()) + rolled = categories_for(question_id, primary) + if group == "articles": + return {articles[cid].id for cid in rolled if cid in articles} + return rolled & top_level + answered: dict[int, int] = defaultdict(int) correct: dict[int, int] = defaultdict(int) seen_questions: dict[int, set[int]] = defaultdict(set) total_answers = len(answered_rows) total_correct = sum(1 for _, is_correct, _ in answered_rows if is_correct) for question_id, is_correct, primary in answered_rows: - for cid in categories_for(question_id, primary): - answered[cid] += 1 - seen_questions[cid].add(question_id) + for key in groups_for(question_id, primary): + answered[key] += 1 + seen_questions[key].add(question_id) if is_correct: - correct[cid] += 1 + correct[key] += 1 - # ── How much bank material each category holds ───────────────── + # ── How much bank material each group holds ──────────────────── available: dict[int, int] = defaultdict(int) bank_total = 0 + grouped_total = 0 for question_id, primary in db.query(Question.id, Question.question_category_id).filter( shareable_question_predicate(), *([exam_filter] if exam_filter is not None else [])).all(): bank_total += 1 - for cid in categories_for(question_id, primary): - available[cid] += 1 - - articles = {} - for article in db.query(Article).filter(Article.status == "published", - Article.category_id.isnot(None)).all(): - articles.setdefault(article.category_id, article) + keys = groups_for(question_id, primary) + if keys: + grouped_total += 1 + for key in keys: + available[key] += 1 overall_accuracy = (total_correct / total_answers) if total_answers else 0.0 unlocked = total_answers >= READINESS_UNLOCK_ANSWERS - scope = top_level if group == "systems" else {cat.id for cat in categories if cat.parent_id is not None} + # What each row is called, and what it links on to. + article_by_id = {a.id: a for a in articles.values()} + category_of_article = {a.id: cid for cid, a in articles.items()} + + def describe(key): + if group == "articles": + article = article_by_id.get(key) + category_id = category_of_article.get(key) + return { + "name": article.title if article else "Untitled", + "parent_name": names.get(parents.get(category_id)) if category_id else None, + "category_id": category_id, + "article_id": key, + "article_title": article.title if article else None, + "system_id": None, + } + if group == "systems": + return { + "name": system_names.get(key, "Unfiled"), + "parent_name": None, + "category_id": None, + "article_id": None, + "article_title": None, + "system_id": key, + } + article = articles.get(key) + return { + "name": names.get(key, "Uncategorized"), + "parent_name": names.get(parents.get(key)), + "category_id": key, + "article_id": article.id if article else None, + "article_title": article.title if article else None, + "system_id": None, + } + rows = [] - for cid in scope: - seen = len(seen_questions.get(cid, ())) - pool = available.get(cid, 0) + for key in set(available) | set(seen_questions): + seen = len(seen_questions.get(key, ())) + pool = available.get(key, 0) if pool == 0 and seen == 0: continue - n = answered.get(cid, 0) - c = correct.get(cid, 0) + n = answered.get(key, 0) + c = correct.get(key, 0) accuracy = round(100 * c / n, 1) if n else None readiness = None if unlocked and n: shrunk = (c + READINESS_PRIOR_ANSWERS * overall_accuracy) / (n + READINESS_PRIOR_ANSWERS) readiness = round(100 * shrunk, 1) - relevance = round(100 * pool / bank_total, 1) if bank_total else 0.0 + # Relevance is measured against the material this grouping can see, not + # the whole bank: half the bank carries no system tag, and dividing by + # the whole bank would make every system look half as relevant as it is. + denominator = grouped_total if group == "systems" else bank_total + relevance = round(100 * pool / denominator, 1) if denominator else 0.0 coverage = round(100 * seen / pool, 1) if pool else 0.0 - article = articles.get(cid) rows.append({ - "category_id": cid, - "name": names.get(cid, "Uncategorized"), - "parent_id": parents.get(cid), - "parent_name": names.get(parents.get(cid)), + "key": key, "answered": n, "correct": c, "seen_questions": seen, @@ -352,8 +425,7 @@ def study_recommendations( "readiness": readiness, "relevance": relevance, "status": "no_data" if not n else "focus" if (readiness if readiness is not None else accuracy) < 70 else "proficient", - "article_id": article.id if article else None, - "article_title": article.title if article else None, + **describe(key), }) # Priority: weak-and-relevant first, then untouched material by relevance. @@ -364,12 +436,9 @@ def study_recommendations( unseen = 1 - (row["coverage"] / 100) row["priority"] = round(max(gap, 0.0) * (row["relevance"] / 100) + 0.25 * unseen * (row["relevance"] / 100), 5) rows.sort(key=lambda row: (-row["priority"], -row["relevance"], row["name"])) - focus_ids = {row["category_id"] for row in rows[:3] if row["answered"]} + focus_keys = {row["key"] for row in rows[:3] if row["answered"]} for row in rows: - if row["category_id"] in focus_ids: - row["is_focus_area"] = True - else: - row["is_focus_area"] = False + row["is_focus_area"] = row["key"] in focus_keys return { "group": group, @@ -380,6 +449,8 @@ def study_recommendations( "total_answered": total_answers, "unique_questions_seen": len({row[0] for row in answered_rows}), "bank_total": bank_total, + # For systems: how much of the bank this grouping can actually see. + "grouped_total": grouped_total, "overall_accuracy": round(100 * overall_accuracy, 1) if total_answers else None, "focus_areas": rows[:limit], "basis": ( diff --git a/backend/app/services/quiz_builder.py b/backend/app/services/quiz_builder.py index 12e0ec3..ff1a625 100644 --- a/backend/app/services/quiz_builder.py +++ b/backend/app/services/quiz_builder.py @@ -86,7 +86,7 @@ def bank_query(db, user): return db.query(Question).filter(bank_question_predicate(user)) -def filtered_bank_query(db, user, category_ids=(), state="all", shared=False, difficulty=None, article_ids=(), tag_ids=()): +def filtered_bank_query(db, user, category_ids=(), state="all", shared=False, difficulty=None, article_ids=(), tag_ids=(), system_ids=()): query = bank_query(db, user) if difficulty: query = query.filter(Question.difficulty == difficulty) @@ -107,6 +107,21 @@ def filtered_bank_query(db, user, category_ids=(), state="all", shared=False, di query = query.filter(Question.id.in_(matching)) else: query = query.filter(Question.id.is_(None)) # No questions match all tags. + if system_ids: + from sqlalchemy import inspect as sa_inspect + from sqlalchemy import text as sa_text + if not sa_inspect(db.get_bind()).has_table("question_tags"): + return query.filter(Question.id.is_(None)) + # A question is never tagged with an organ system directly — it carries + # a symptom keyword that is filed under one. So this matches ANY tag + # beneath the system, unlike tag_ids above, which is an AND across tags. + matching = list(db.execute(sa_text(""" + SELECT DISTINCT l.question_id + FROM question_tag_links l + JOIN question_tags t ON t.id = l.tag_id + WHERE t.parent_id = ANY(:system_ids) OR t.id = ANY(:system_ids) + """), {"system_ids": list(dict.fromkeys(system_ids))}).scalars()) + query = query.filter(Question.id.in_(matching)) if matching else query.filter(Question.id.is_(None)) if category_ids: ids = category_descendants(db.query(QuestionCategory).all(), category_ids) query = query.filter(or_( @@ -166,6 +181,9 @@ class GenerateTestRequest(TestOptions): algorithm: Literal["random", "adaptive"] = "random" article_ids: list[int] = Field(default_factory=list) tag_ids: list[int] = Field(default_factory=list) + #: Organ systems. Matched as "any tag beneath this system", where + #: tag_ids is "every one of these tags". + system_ids: list[int] = Field(default_factory=list) explicit_ids: list[int] = Field(default_factory=list) @@ -243,7 +261,8 @@ def generate_test(db, user, data): if len(ids) < data.count: raise HTTPException(400, f"Only {len(ids)} questions available; requested {data.count}") return create_saved_test(db, user, data, ids) - query = filtered_bank_query(db, user, data.category_ids, data.state, data.is_shared, data.difficulty, data.article_ids, data.tag_ids) + query = filtered_bank_query(db, user, data.category_ids, data.state, data.is_shared, + data.difficulty, data.article_ids, data.tag_ids, data.system_ids) if data.explicit_ids: query = query.filter(Question.id.in_(list(dict.fromkeys(data.explicit_ids)))) ids = [row[0] for row in query.with_entities(Question.id).all()] diff --git a/backend/tests/test_quiz_sessions.py b/backend/tests/test_quiz_sessions.py index cdfce33..ff33af4 100644 --- a/backend/tests/test_quiz_sessions.py +++ b/backend/tests/test_quiz_sessions.py @@ -286,17 +286,36 @@ class RecommendationTests(unittest.TestCase): self.assertEqual(response.status_code, 200, response.text) return response.json() - def test_relevance_and_coverage_roll_up_to_the_parent_system(self): + def test_relevance_and_coverage_roll_up_to_the_discipline(self): data = self.recommend() + self.assertEqual(data["group"], "disciplines") rows = {row["name"]: row for row in data["focus_areas"]} # Cardiology owns 4 of 6 bank questions through its child category. self.assertEqual(rows["Cardiology"]["available"], 4) self.assertEqual(rows["Cardiology"]["relevance"], 66.7) self.assertEqual(rows["Neurology"]["available"], 2) self.assertEqual(data["bank_total"], 6) - # Systems grouping shows top-level categories only. + # Disciplines are the top of the tree; a condition under one is not a row. self.assertNotIn("Kawasaki disease", rows) - self.assertIn("Kawasaki disease", {r["name"] for r in self.recommend(group="subtopics")["focus_areas"]}) + + def test_the_same_answers_group_three_ways(self): + # Articles: the reading to go back to, reached through its category. + articles = self.recommend(group="articles")["focus_areas"] + self.assertEqual([row["name"] for row in articles], ["Kawasaki disease"]) + row = articles[0] + self.assertEqual((row["article_id"], row["category_id"]), (7, 1)) + self.assertEqual(row["available"], 4) + + # Systems: no tag table on a fresh database, so there is nothing to + # show — and that is reported as no rows, not as an error. + systems = self.recommend(group="systems") + self.assertEqual(systems["group"], "systems") + self.assertEqual(systems["focus_areas"], []) + + def test_an_unknown_grouping_is_refused_rather_than_guessed(self): + self.assertEqual( + self.client.get("/study-tools/recommendations", params={"group": "subtopics"}).status_code, + 422) def test_readiness_stays_locked_until_enough_answers(self): self.answer(1, False) diff --git a/frontend/src/pages/AnalysisPage.css b/frontend/src/pages/AnalysisPage.css index a59a14e..d430e7f 100644 --- a/frontend/src/pages/AnalysisPage.css +++ b/frontend/src/pages/AnalysisPage.css @@ -111,3 +111,10 @@ .an-exam { color: var(--primary); text-decoration: none; border-bottom: 2px solid currentColor; } .an-exam:hover { opacity: .8; } + +/* How much of the bank a grouping can actually see. */ +.an-coverage-note { + margin: 0 0 12px; padding: 9px 12px; font-size: 0.8rem; line-height: 1.55; + color: var(--text-muted); background: var(--bg); + border: 1px solid var(--border); border-radius: 8px; +} diff --git a/frontend/src/pages/AnalysisPage.jsx b/frontend/src/pages/AnalysisPage.jsx index ddf4deb..eeaa0aa 100644 --- a/frontend/src/pages/AnalysisPage.jsx +++ b/frontend/src/pages/AnalysisPage.jsx @@ -7,7 +7,7 @@ import './AnalysisPage.css' const STATUS_LABEL = { focus: 'Focus area', proficient: 'Proficient', no_data: 'No data yet' } -function FocusRow({ row, onStartCategory }) { +function FocusRow({ row, onPractise }) { const [open, setOpen] = useState(false) const coverageLabel = `${row.seen_questions}/${row.available}` const accuracyLabel = row.answered ? `${row.accuracy}% (${row.correct} of ${row.answered})` : 'Not attempted' @@ -52,7 +52,10 @@ function FocusRow({ row, onStartCategory }) { Read {row.article_title} )} - @@ -64,7 +67,7 @@ function FocusRow({ row, onStartCategory }) { export default function AnalysisPage() { const [tab, setTab] = useState('recommendations') - const [group, setGroup] = useState('systems') + const [group, setGroup] = useState('articles') const [data, setData] = useState(null) const [loading, setLoading] = useState(true) const [error, setError] = useState('') @@ -84,7 +87,12 @@ export default function AnalysisPage() { const startAdaptive = () => navigate(`/study/new?adaptive=1&count=${count}`) - const startCategory = (categoryId) => navigate(`/study/new?category=${categoryId}&count=${count}`) + const startPractice = (row) => { + const param = row.system_id ? `system=${row.system_id}` + : row.article_id && group === 'articles' ? `article=${row.article_id}` + : `category=${row.category_id}` + navigate(`/study/new?${param}&count=${count}`) + } return ( @@ -161,12 +169,24 @@ export default function AnalysisPage() { {tab === 'recommendations' ? ( <> -
- {[['systems', 'Systems'], ['subtopics', 'Subtopics']].map(([value, label]) => ( - + {/* The same answers asked three ways: which reading to go back + to, which discipline is weak, which organ system is weak. */} +
+ {[['articles', 'Articles'], ['systems', 'Systems'], ['disciplines', 'Disciplines']].map(([value, label]) => ( + ))}
+ {/* Only about half the bank carries an organ-system tag. Saying so + is better than showing a partial picture as if it were whole. */} + {group === 'systems' && data.grouped_total < data.bank_total && ( +

+ Systems covers {data.grouped_total} of {data.bank_total} questions — + the rest are not filed under an organ system yet. +

+ )} + {data.focus_areas.length === 0 ? (
No categorised questions yet — once questions are filed under a system, your focus areas appear here.
) : ( @@ -180,7 +200,7 @@ export default function AnalysisPage() {
{data.focus_areas.map(row => ( - + ))}
diff --git a/frontend/src/pages/AnalysisPage.test.jsx b/frontend/src/pages/AnalysisPage.test.jsx index 1548194..e697b76 100644 --- a/frontend/src/pages/AnalysisPage.test.jsx +++ b/frontend/src/pages/AnalysisPage.test.jsx @@ -16,8 +16,8 @@ const area = (over = {}) => ({ }) const payload = (over = {}) => ({ - group: 'systems', unlocked: true, answers_needed: 0, total_answered: 60, - unique_questions_seen: 42, bank_total: 330, overall_accuracy: 64.0, + group: 'articles', unlocked: true, answers_needed: 0, total_answered: 60, + unique_questions_seen: 42, bank_total: 330, grouped_total: 330, overall_accuracy: 64.0, focus_areas: [area()], basis: 'Readiness shrinks each category toward your overall accuracy.', ...over, }) @@ -56,12 +56,26 @@ it('explains that readiness is locked until enough answers exist', async () => { expect(screen.getByText(/Answer 12 more questions/)).toBeInTheDocument() }) -it('reloads focus areas when the grouping changes', async () => { +it('asks the same answers three ways: articles, systems, disciplines', async () => { render() await screen.findByText('Cardiology') - await userEvent.click(screen.getByRole('button', { name: 'Subtopics' })) + // Articles first: the reading to go back to is the most actionable answer. + expect(api.get).toHaveBeenCalledWith('/study-tools/recommendations', { params: { group: 'articles' } }) + + await userEvent.click(screen.getByRole('tab', { name: 'Systems' })) await waitFor(() => expect(api.get).toHaveBeenLastCalledWith('/study-tools/recommendations', - { params: { group: 'subtopics' } })) + { params: { group: 'systems' } })) + + await userEvent.click(screen.getByRole('tab', { name: 'Disciplines' })) + await waitFor(() => expect(api.get).toHaveBeenLastCalledWith('/study-tools/recommendations', + { params: { group: 'disciplines' } })) +}) + +it('says how much of the bank the systems grouping can actually see', async () => { + api.get.mockResolvedValue({ data: payload({ group: 'systems', bank_total: 2948, grouped_total: 1502 }) }) + render() + await userEvent.click(await screen.findByRole('tab', { name: 'Systems' })) + expect(await screen.findByText(/Systems covers 1502 of 2948 questions/)).toBeInTheDocument() }) it('switches to the performance tab', async () => { diff --git a/frontend/src/pages/CustomQuizPage.jsx b/frontend/src/pages/CustomQuizPage.jsx index 6c59d65..3e80392 100644 --- a/frontend/src/pages/CustomQuizPage.jsx +++ b/frontend/src/pages/CustomQuizPage.jsx @@ -42,7 +42,14 @@ export default function CustomQuizPage() { const [submitting, setSubmitting] = useState(false) const [refresh, setRefresh] = useState(0) - const [articleIds, setArticleIds] = useState([]) + // Arriving from a knowledge-profile row: practise that article, or + // everything filed under that organ system. A system is not a checkbox in + // the picker — it is an axis over the symptom tags — so it is carried as an + // opaque filter rather than shown as a selection. + const [articleIds, setArticleIds] = useState( + () => [...new Set(searchParams.getAll('article').map(Number).filter(id => Number.isSafeInteger(id) && id > 0))]) + const [systemIds] = useState( + () => [...new Set(searchParams.getAll('system').map(Number).filter(id => Number.isSafeInteger(id) && id > 0))]) const [tagIds, setTagIds] = useState([]) const [articles, setArticles] = useState([]) const [tags, setTags] = useState({ subjects: [], keywords: [] }) @@ -52,7 +59,7 @@ export default function CustomQuizPage() { const [globalSearch, setGlobalSearch] = useState('') const [moreOpen, setMoreOpen] = useState(false) - const filterKey = JSON.stringify([categoryIds, state, shared, difficulty, articleIds, tagIds, refresh]) + const filterKey = JSON.stringify([categoryIds, state, shared, difficulty, articleIds, tagIds, systemIds, refresh]) useEffect(() => { let active = true @@ -76,6 +83,7 @@ export default function CustomQuizPage() { categoryIds.forEach(id => params.append('category_ids', id)) articleIds.forEach(id => params.append('article_ids', id)) tagIds.forEach(id => params.append('tag_ids', id)) + if (systemIds.length) params.append('system_ids', systemIds.join(',')) api.get('/questions/builder/count', { params }).then(r => { if (active) { setAvailable(r.data.count); setCountKey(filterKey) } }).catch(() => { if (active) setCountError('Could not load available count. Try refreshing.') }) @@ -105,7 +113,7 @@ export default function CustomQuizPage() { title: title.trim(), category_ids: categoryIds, state, count: Number(count), expected_count: available, mode, time_limit_minutes: mode === 'timed' && time ? Number(time) : null, is_shared: shared, difficulty: difficulty || null, algorithm: adaptive ? 'adaptive' : 'random', - article_ids: articleIds, tag_ids: tagIds, explicit_ids: explicitIds, + article_ids: articleIds, tag_ids: tagIds, system_ids: systemIds, explicit_ids: explicitIds, }) navigate(`/study/${result.data.id}`) } catch (err) { diff --git a/frontend/src/pages/CustomQuizPage.test.jsx b/frontend/src/pages/CustomQuizPage.test.jsx index b0742aa..a5034e6 100644 --- a/frontend/src/pages/CustomQuizPage.test.jsx +++ b/frontend/src/pages/CustomQuizPage.test.jsx @@ -65,7 +65,7 @@ describe('CustomQuizPage', () => { expect(api.post).toHaveBeenCalledWith('/questions/builder', { title: `Custom test from ${new Date().toLocaleDateString('en-US', { month: 'short', day: 'numeric' })}, ${new Date().toLocaleTimeString('en-US', { hour: 'numeric', hour12: true })}`, category_ids: [1, 2], state: 'unused', count: 10, mode: 'timed', time_limit_minutes: 15, expected_count: 30, is_shared: true, - difficulty: null, algorithm: 'random', article_ids: [], tag_ids: [], explicit_ids: [], + difficulty: null, algorithm: 'random', article_ids: [], tag_ids: [], system_ids: [], explicit_ids: [], }) await screen.findByRole('heading', { name: 'Saved test' }) })