diff --git a/backend/app/routers/tags.py b/backend/app/routers/tags.py index 6321f2f..98d29c1 100644 --- a/backend/app/routers/tags.py +++ b/backend/app/routers/tags.py @@ -1,8 +1,8 @@ -"""Organ systems: the vocabulary, and how many questions reach each one. +"""The vocabularies a question can be filtered by, and how many it holds. -Once this served four vocabularies a model wrote per question — subjects, -diseases, keywords, systems. The first three became the category tree and have -been retired; what is left is the sixteen systems, which a topic points at. +Three of the four a model once wrote per question survive: subjects, diseases +and the sixteen organ systems. Keywords are retired — they were the old route +to a system, which a topic now carries directly. The table is created via raw SQL in setup_pgvector() to avoid race conditions with multiple workers, so there is no ORM class for it. @@ -36,10 +36,14 @@ def get_tags(db: Session = Depends(get_db), current_user: User = Depends(get_cur so the number here is the number the filter will return. It used to come from a symptom keyword the question happened to mention. - Those keywords are retired, and with them the subject and disease - vocabularies that had already become the topic tree. The shape of the - response is kept so nothing that reads it has to be rewritten to find an - empty list where it expected one. + Those keywords are retired — the topic carries the system now, which took + the Systems axis from about half the bank to all of it. + + Subjects and diseases are not retired. They were taken down with the + keywords on the argument that the topic tree says the same thing; it mostly + does, but they are the vocabulary people had learned to filter by, and + "mostly" is not a reason to remove somebody's tools. They are counted here + the way they always were, from the links a question carries directly. """ exam_id = getattr(current_user, "active_exam_id", None) systems = db.execute(text( @@ -85,8 +89,31 @@ def get_tags(db: Session = Depends(get_db), current_user: User = Depends(get_cur if system: counts[system].add(qid) + # The two vocabularies that survived retirement, counted the way they + # always were: a question carries them directly. + vocab: dict[str, list[dict]] = {"subjects": [], "diseases": []} + of_type = {"subject": "subjects", "disease": "diseases"} + rows = db.execute(text(""" + SELECT t.id, t.name, t.type, t.parent_id, + COUNT(DISTINCT tl.question_id) AS count + FROM question_tags t + LEFT JOIN question_tag_links tl ON tl.tag_id = t.id + WHERE t.type IN ('subject', 'disease') + AND (t.exam_id IS NULL OR :exam_id IS NULL OR t.exam_id = :exam_id) + GROUP BY t.id, t.name, t.type, t.parent_id + ORDER BY count DESC, t.name + """), {"exam_id": exam_id}).fetchall() + for row in rows: + if exam_id and row.count == 0: + continue + vocab[of_type[row.type]].append({ + "id": row.id, "name": row.name, "count": row.count, "parent_id": row.parent_id}) + return { - "subjects": [], "diseases": [], "keywords": [], + **vocab, + # Retired: this was the route to an organ system before a topic carried + # one, and nothing reads it now. + "keywords": [], "systems": [{"id": row.id, "name": row.name, "count": len(counts.get(row.id, ())), "parent_id": None} for row in systems], } diff --git a/backend/scripts/restore_subject_disease_tags.py b/backend/scripts/restore_subject_disease_tags.py new file mode 100644 index 0000000..d42e9db --- /dev/null +++ b/backend/scripts/restore_subject_disease_tags.py @@ -0,0 +1,94 @@ +"""Put the discipline and disease vocabularies back. + +`retire_question_tags` took three vocabularies down together: subject, disease +and keyword. Keyword was the one that had to go — it was the old route to an +organ system, which the topic tree now carries properly. Subject and disease +were removed on the argument that the tree already says the same thing, and +that argument was mine to make and not mine to act on alone. + +So this restores those two and leaves keyword retired. Read from a plain +tab-separated extract of the two tables — the shape `pg_dump` writes — so it +works from a nightly dump as well as from a purpose-made file. + + docker compose exec backend python -m scripts.restore_subject_disease_tags \\ + --tags /backups/tags.tsv --links /backups/links.tsv + …same, with --apply + +Idempotent: a tag id that is already present is left alone, and so is a link +that already exists. +""" +import argparse +import sys + +from sqlalchemy import text as sa_text + +from app.database import SessionLocal + +RESTORING = ("subject", "disease") +#: id, name, type, created_at, exam_id, parent_id, sort_order — pg_dump's order. +COLUMNS = ("id", "name", "type", "created_at", "exam_id", "parent_id", "sort_order") + + +def read(path: str) -> list[list[str]]: + with open(path, encoding="utf-8") as handle: + return [line.rstrip("\n").split("\t") for line in handle if line.strip()] + + +def value(raw: str): + """`pg_dump` writes an unquoted \\N for null.""" + return None if raw == "\\N" else raw + + +def main(tags_path: str, links_path: str, apply: bool) -> int: + wanted = [row for row in read(tags_path) if row[2] in RESTORING] + if not wanted: + print(f"! no {' or '.join(RESTORING)} rows in {tags_path}") + return 1 + ids = {row[0] for row in wanted} + links = [row for row in read(links_path) if row[1] in ids] + + db = SessionLocal() + try: + present = {str(r[0]) for r in db.execute(sa_text( + "SELECT id FROM question_tags WHERE type = ANY(:t)"), {"t": list(RESTORING)}).all()} + new_tags = [row for row in wanted if row[0] not in present] + + counts = {kind: sum(1 for row in new_tags if row[2] == kind) for kind in RESTORING} + print(f"in the extract {len(wanted)} tags, {len(links)} links") + print(f"already present {len(wanted) - len(new_tags)}") + for kind, n in counts.items(): + print(f" {kind:<9} {n} to restore") + + if not apply: + print("\ndry run. Pass --apply to write.") + return 0 + + for row in new_tags: + db.execute(sa_text( + "INSERT INTO question_tags (id, name, type, created_at, exam_id, parent_id, sort_order) " + "VALUES (:id, :name, :type, :created_at, :exam_id, :parent_id, :sort_order) " + "ON CONFLICT (id) DO NOTHING"), + {col: value(raw) for col, raw in zip(COLUMNS, row)}) + for question_id, tag_id in links: + db.execute(sa_text( + "INSERT INTO question_tag_links (question_id, tag_id) VALUES (:q, :t) " + "ON CONFLICT DO NOTHING"), {"q": int(question_id), "t": int(tag_id)}) + # The sequence has to clear the ids just written, or the next tag + # created by hand collides with a restored one. + db.execute(sa_text( + "SELECT setval(pg_get_serial_sequence('question_tags', 'id'), " + "GREATEST((SELECT MAX(id) FROM question_tags), 1))")) + db.commit() + print(f"\nrestored {len(new_tags)} tags and {len(links)} links.") + return 0 + finally: + db.close() + + +if __name__ == "__main__": + parser = argparse.ArgumentParser() + parser.add_argument("--tags", required=True) + parser.add_argument("--links", required=True) + parser.add_argument("--apply", action="store_true") + args = parser.parse_args() + sys.exit(main(args.tags, args.links, args.apply)) diff --git a/backend/tests/test_tag_hierarchy.py b/backend/tests/test_tag_hierarchy.py index 0be3641..afa6fc6 100644 --- a/backend/tests/test_tag_hierarchy.py +++ b/backend/tests/test_tag_hierarchy.py @@ -83,12 +83,14 @@ class TagHierarchyTests(unittest.TestCase): self.assertEqual(systems["Nervous System & Special Senses"], 2) self.assertEqual(systems["Respiratory System"], 1) - def test_the_retired_vocabularies_answer_empty_rather_than_missing(self): + def test_keywords_are_retired_and_answer_empty(self): body = self.client.get("/tags").json() - # Subjects, diseases and keywords became the topic tree. The shape of - # the response is kept so nothing reading it has to be rewritten. - for gone in ("subjects", "diseases", "keywords"): - self.assertEqual(body[gone], []) + # Keywords were the old route to an organ system, which a topic now + # carries. Subjects and diseases are still served — they are simply + # not in this fixture. The key stays so nothing reading it breaks. + self.assertEqual(body["keywords"], []) + for kept in ("subjects", "diseases", "systems"): + self.assertIn(kept, body) def test_a_moderator_can_add_a_system(self): made = self.client.post("/tags/", json={"name": "Endocrine System", "type": "system"}) diff --git a/frontend/src/pages/AnalysisPage.css b/frontend/src/pages/AnalysisPage.css index 5b8adda..129f009 100644 --- a/frontend/src/pages/AnalysisPage.css +++ b/frontend/src/pages/AnalysisPage.css @@ -141,7 +141,7 @@ display: grid; grid-template-columns: minmax(0, 1fr) minmax(0, 1fr); gap: 18px; align-items: start; } -.an-perf-top .an-completion { margin: 24px 0 0; } +.an-perf-top > * { margin-top: 24px; } .an-split-card { margin-top: 24px; padding: 16px 18px 18px; border: 1px solid var(--border); border-radius: 12px; background: var(--card-bg); diff --git a/frontend/src/pages/AnalysisPage.jsx b/frontend/src/pages/AnalysisPage.jsx index 6e096f9..ee87bba 100644 --- a/frontend/src/pages/AnalysisPage.jsx +++ b/frontend/src/pages/AnalysisPage.jsx @@ -524,11 +524,14 @@ export default function AnalysisPage() { ) : ( <> - + {/* The chart and the donut read together — the trend beside the + split it is a trend in — and Completion is four plain figures + underneath, which is the order the page is read in. */}
- +
+ )} diff --git a/frontend/src/pages/CustomQuizPage.jsx b/frontend/src/pages/CustomQuizPage.jsx index 584a2cd..c5235b3 100644 --- a/frontend/src/pages/CustomQuizPage.jsx +++ b/frontend/src/pages/CustomQuizPage.jsx @@ -139,11 +139,16 @@ export default function CustomQuizPage() { }, [categories]) const nameById = useMemo(() => Object.fromEntries(categories.map(c => [c.id, c.name])), [categories]) - const tagName = (id) => (tags.systems || []).find(t => t.id === id)?.name - + const subjectTags = tags.subjects || [] + const diseaseTags = tags.diseases || [] const systemTags = tags.systems || [] + const tagName = (id) => [...subjectTags, ...diseaseTags, ...systemTags] + .find(t => t.id === id)?.name + const systems = summarise(categoryIds.map(id => nameById[id]).filter(Boolean)) const organSystems = summarise(tagIds.map(tagName).filter(n => n && systemTags.some(t => t.name === n))) + const disciplines = summarise(tagIds.map(tagName).filter(n => n && subjectTags.some(t => t.name === n))) + const diseases = summarise(tagIds.map(tagName).filter(n => n && diseaseTags.some(t => t.name === n))) const articleSummary = summarise(articleIds.map(id => articles.find(a => a.id === id)?.title).filter(Boolean)) const savedNames = [ ...(state === 'bookmarked' ? ['Bookmarked questions'] : []), @@ -197,9 +202,14 @@ export default function CustomQuizPage() { out.push({ key: `c${cat.id}`, name: cat.name, facet: 'Topics', on: categoryIds.includes(cat.id), toggle: on => toggleCategory(cat.id, on) }) } } - for (const tag of systemTags) { - if (tag.name.toLowerCase().includes(query)) { - out.push({ key: `y${tag.id}`, name: tag.name, facet: 'Systems', on: tagIds.includes(tag.id), toggle: on => toggleTag(tag.id, on) }) + for (const [group, facet, prefix] of [[systemTags, 'Systems', 'y'], + [subjectTags, 'Disciplines', 's'], + [diseaseTags, 'Diseases', 'd']]) { + for (const tag of group) { + if (tag.name.toLowerCase().includes(query)) { + out.push({ key: `${prefix}${tag.id}`, name: tag.name, facet, + on: tagIds.includes(tag.id), toggle: on => toggleTag(tag.id, on) }) + } } } for (const article of articles) { @@ -208,7 +218,8 @@ export default function CustomQuizPage() { } } return out.slice(0, 40) - }, [globalSearch, categories, systemTags, articles, categoryIds, tagIds, articleIds]) + }, [globalSearch, categories, systemTags, subjectTags, diseaseTags, articles, + categoryIds, tagIds, articleIds]) const resetTopics = () => { setCategoryIds([]); setTagIds([]); setArticleIds([]); setPresetIds([]) @@ -266,6 +277,8 @@ export default function CustomQuizPage() { setOpenFacet('organ-systems')} /> setOpenFacet('systems')} /> + setOpenFacet('disciplines')} /> + setOpenFacet('diseases')} /> setOpenFacet('articles')} /> setOpenFacet('saved')} /> @@ -405,11 +418,26 @@ export default function CustomQuizPage() { )} - {/* Disciplines and Symptoms & keywords were two more pickers here, over - the 203 subject tags and 4,281 keyword tags a model had written per - question. Both vocabularies were converted into Topics — 673 of them, - in a tree, which is the same axis said once and said better — and - have been retired. Topics is where a discipline is chosen. */} + {/* Symptoms & keywords used to sit here too, over 4,281 keyword tags. + Those were the old route to an organ system, which a topic now + carries directly, and they are retired. Disciplines and Diseases are + not: the topic tree mostly says the same thing, and "mostly" is not a + reason to take away the vocabulary people filter by. */} + setOpenFacet(null)} + onReset={() => setTagIds(ids => ids.filter(id => !subjectTags.some(t => t.id === id)))} + selected={subjectTags.filter(t => tagIds.includes(t.id)).map(t => ({ id: t.id, label: t.name }))} + onRemove={id => toggleTag(id, false)} + helper="By default, all disciplines are included unless filters are selected."> + {query => checkList(subjectTags, t => tagIds.includes(t.id), (t, on) => toggleTag(t.id, on), query)} + + + setOpenFacet(null)} + onReset={() => setTagIds(ids => ids.filter(id => !diseaseTags.some(t => t.id === id)))} + selected={diseaseTags.filter(t => tagIds.includes(t.id)).map(t => ({ id: t.id, label: t.name }))} + onRemove={id => toggleTag(id, false)} + helper="Named conditions. By default all are included."> + {query => checkList(diseaseTags, t => tagIds.includes(t.id), (t, on) => toggleTag(t.id, on), query)} + setOpenFacet(null)} onReset={() => setArticleIds([])}