From 734ff194f21e2b6b6d297a86c8130a5b4b8a17dc Mon Sep 17 00:00:00 2001 From: Daniel Date: Sun, 13 Sep 2026 04:26:59 +0200 Subject: [PATCH] fix: a question's figure says what the question says, not what the library says MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The caption under a figure in a session was the image library's own description, and a library description is written to catalogue an image: "an X-ray of a child's pelvis and hips, showing abnormalities in the right hip joint" is a fine catalogue entry and a complete giveaway under a stem about a limping five-year-old. One line in figure_json fell back to it, and every one of the 343 figures in the bank was inheriting one — 338 of them on stems, read before the question was answered. Not one figure had a caption of its own. So a figure carries what the question says about it and nothing else. The caption and the label are set on the question, when the image is attached; most question figures want neither, and now show neither. The catalogue title went with it — nothing rendered it, and it gives the same thing away over the wire. The second route in was the viewer: an image written into a stem as markdown opens a panel that fetches what the library knows and prints it. Inside an attempt it no longer asks. In the library, in an article, in review of the image itself, the description is still the description. Also: no PMID links. A reference list says what was read; it is not a set of doors out of the article. The number is there to look up, as text. And a drawer's own button is a cross. It kept the ☰ that opened it while covering the screen, which reads as a second menu rather than the way out of the one in front of you. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01TqXevQJhxFrM7jJg82cgZN --- backend/app/routers/articles.py | 5 +-- backend/app/services/pubmed.py | 1 - backend/app/services/question_figures.py | 18 ++++++-- backend/tests/test_question_figures.py | 52 +++++++++++++++++++++++ frontend/src/components/ArticleReader.jsx | 9 ++-- frontend/src/components/ImageFigure.jsx | 21 +++++++-- frontend/src/components/RichText.jsx | 5 ++- frontend/src/pages/ArticlesPage.css | 6 +-- frontend/src/pages/QuizPage.jsx | 6 ++- frontend/src/pages/ResultsPage.jsx | 6 ++- 10 files changed, 106 insertions(+), 23 deletions(-) create mode 100644 backend/tests/test_question_figures.py diff --git a/backend/app/routers/articles.py b/backend/app/routers/articles.py index 1381e8d..076693e 100644 --- a/backend/app/routers/articles.py +++ b/backend/app/routers/articles.py @@ -61,7 +61,6 @@ class ArticleReference(BaseModel): journal: str | None = Field(default=None, max_length=150) year: str | None = Field(default=None, max_length=10) pmid: str | None = Field(default=None, max_length=20) - url: str | None = Field(default=None, max_length=300) #: "Barski L, et al. Management of diabetic ketoacidosis. Eur J Intern Med. @@ -88,9 +87,7 @@ def _reference_json(entry) -> dict: pmid = found.group(1) if found else None return { "title": (_PMID_TAIL.sub("", line).strip() or line)[:300], - "author": None, "pages": [], - "pmid": pmid, - "url": f"https://pubmed.ncbi.nlm.nih.gov/{pmid}/" if pmid else None, + "author": None, "pages": [], "pmid": pmid, } diff --git a/backend/app/services/pubmed.py b/backend/app/services/pubmed.py index 9268842..775ff2d 100644 --- a/backend/app/services/pubmed.py +++ b/backend/app/services/pubmed.py @@ -225,5 +225,4 @@ def as_references(results: list[dict]) -> list[dict]: "journal": record["journal"] or None, "year": record["year"] or None, "pmid": record["pmid"], - "url": record["url"], } for record in results] diff --git a/backend/app/services/question_figures.py b/backend/app/services/question_figures.py index 53deb69..d54950d 100644 --- a/backend/app/services/question_figures.py +++ b/backend/app/services/question_figures.py @@ -22,14 +22,24 @@ def figure_json(link: QuestionMedia, asset: MediaAsset) -> dict: "id": link.id, "media_id": link.media_id, "role": link.role, - # The label the prose refers to. Falls back to a number so a figure is - # never nameless, which is what makes "see the figure" ambiguous. # An educator's label, or nothing. "Figure 1" and "Figure from question # #3360" told a learner only that an image was an image, and the second # one told them the internal path it came from as well. "label": link.label or None, - "caption": link.caption or getattr(asset, "caption", None), - "title": getattr(asset, "title", None), + # What this *question* says about the image, and nothing else. + # + # It used to fall back to the library's own description, which is + # written to catalogue an image, not to sit beside a question — so a + # radiograph attached to a stem about a limping child carried + # "abnormalities in the right hip joint" underneath it, and the + # question was answered before it was read. Every one of the 343 + # figures in the bank was inheriting that description; not one had a + # caption of its own. Most question figures want no caption at all, + # and the ones that do want words chosen for the question they are on. + # + # The library description is still the library's, and still shown + # there and in an article. It is not shown in a quiz. + "caption": link.caption or None, "path": getattr(asset, "path", None), "position": link.position, } diff --git a/backend/tests/test_question_figures.py b/backend/tests/test_question_figures.py new file mode 100644 index 0000000..a57ef87 --- /dev/null +++ b/backend/tests/test_question_figures.py @@ -0,0 +1,52 @@ +"""What a question says about its own figures — and what it must not say. + +Run: DATABASE_URL=sqlite:///:memory: PYTHONPATH=backend python -m unittest discover -s backend/tests +""" +import os +os.environ.setdefault("DATABASE_URL", "sqlite:///:memory:") + +import unittest + +from app.models.media import MediaAsset +from app.models.question_media import QuestionMedia +from app.services.question_figures import figure_json + + +class FigureCaptionTests(unittest.TestCase): + """A caption beside a stem is read before the question is answered. + + The library's description of an image is written to catalogue it — "an + X-ray of a child's pelvis and hips, showing abnormalities in the right hip + joint" is a perfectly good catalogue entry and a complete giveaway under a + stem about a limping five-year-old. Every one of the bank's 343 figures was + inheriting one; not one had a caption of its own. + """ + + def setUp(self): + self.asset = MediaAsset(id=5, path="uploads/hip.png", + title="Right SCFE, AP pelvis", + caption="An X-ray of a child's pelvis and hips, " + "showing abnormalities in the right hip joint.") + + def test_the_library_description_never_reaches_a_question(self): + link = QuestionMedia(id=1, question_id=9, media_id=5, role="stem", + label=None, caption=None, position=0) + figure = figure_json(link, self.asset) + self.assertIsNone(figure["caption"]) + self.assertIsNone(figure["label"]) + # Nor by another name: the catalogue title is a giveaway of its own, + # and nothing renders it. + self.assertNotIn("title", figure) + self.assertEqual(figure["path"], "uploads/hip.png") + + def test_what_the_question_itself_says_is_shown(self): + link = QuestionMedia(id=2, question_id=9, media_id=5, role="stem", + label="Figure 1", caption="AP pelvis at presentation.", + position=0) + figure = figure_json(link, self.asset) + self.assertEqual(figure["caption"], "AP pelvis at presentation.") + self.assertEqual(figure["label"], "Figure 1") + + +if __name__ == "__main__": + unittest.main() diff --git a/frontend/src/components/ArticleReader.jsx b/frontend/src/components/ArticleReader.jsx index f8f6cdf..b38d54e 100644 --- a/frontend/src/components/ArticleReader.jsx +++ b/frontend/src/components/ArticleReader.jsx @@ -295,10 +295,11 @@ export default function ArticleReader({ {' · '}{[ref.journal, ref.year].filter(Boolean).join(', ')} )} - {ref.pmid && ( - PMID {ref.pmid} - )} + {/* The number, not a link. A reference list is a list of what + was read, and every entry that leaves the site is an + invitation to leave mid-article — the PMID is enough for + anybody who wants to look it up. */} + {ref.pmid && PMID {ref.pmid}} ))} diff --git a/frontend/src/components/ImageFigure.jsx b/frontend/src/components/ImageFigure.jsx index 48f995f..83ddf57 100644 --- a/frontend/src/components/ImageFigure.jsx +++ b/frontend/src/components/ImageFigure.jsx @@ -30,7 +30,16 @@ import './ImageFigure.css' //: CSS, which cannot see a file extension, and given a width to work from. const isVector = (src) => /\.svg(\?|#|$)/i.test(String(src || '')) -export function ImageViewer({ src, alt = '', attemptId, onClose }) { +export function ImageViewer({ src, alt = '', attemptId, onClose, + // Whether what the *library* knows about the + // image is shown beside it. In a session it is + // not: a catalogue description is written to + // find an image again, not to sit next to a + // question, and "showing abnormalities in the + // right hip joint" answers the stem it + // illustrates. What a question says about its + // own figure is set on the question. + libraryInfo = true }) { const [asset, setAsset] = useState(null) const [marked, setMarked] = useState(false) @@ -64,14 +73,14 @@ export function ImageViewer({ src, alt = '', attemptId, onClose }) { }, [onClose]) useEffect(() => { - if (asset !== null || !src) return undefined + if (!libraryInfo || asset !== null || !src) return undefined let live = true api.get('/media/by-path', { params: { path: src } }) // `false` rather than null: asked and there is nothing, so do not ask again. .then(res => { if (live) setAsset(res.data || false) }) .catch(() => { if (live) setAsset(false) }) return () => { live = false } - }, [asset, src]) + }, [asset, src, libraryInfo]) const label = (alt || '').trim() const title = (asset && asset.title) || label @@ -123,6 +132,9 @@ export function ImageViewer({ src, alt = '', attemptId, onClose }) { } export default function ImageFigure({ src, alt = '', attemptId, className = '', + // Passed straight through; see the + // viewer. + libraryInfo = true, // Whether the alt text is also printed // under the thumbnail. On a card it is // not: the card's own words are the @@ -147,7 +159,8 @@ export default function ImageFigure({ src, alt = '', attemptId, className = '', {open && ( - setOpen(false)} /> + setOpen(false)} /> )} ) diff --git a/frontend/src/components/RichText.jsx b/frontend/src/components/RichText.jsx index 5f0912b..6037c16 100644 --- a/frontend/src/components/RichText.jsx +++ b/frontend/src/components/RichText.jsx @@ -94,7 +94,10 @@ export default function RichText({ // full size on a click. A 2,000px radiograph rendered inline was a wall of // greyscale in the middle of a sentence, and four megabytes to draw it. img: ({ node, src, alt, ...props }) => ( - + // Inside an attempt the library's description of the image is not shown: + // what a question says about its figure is set on the question, and + // `attemptId` is only ever passed by the player and the review. + ), // `{{phrase|tip}}` — a teaching point that opens where the phrase is. span: ({ node, children, ...props }) => ( diff --git a/frontend/src/pages/ArticlesPage.css b/frontend/src/pages/ArticlesPage.css index 06c4924..2b1b873 100644 --- a/frontend/src/pages/ArticlesPage.css +++ b/frontend/src/pages/ArticlesPage.css @@ -376,12 +376,12 @@ .article-ref-title { color: var(--text); font-weight: 600; } .article-ref-pages { font-variant-numeric: tabular-nums; } /* The checkable part of a paper. Set apart from the citation so the eye finds - it, and a link because it goes somewhere. */ + it — plain text, because a reference list is what was read, not a set of + doors out of the article. */ .article-ref-pmid { margin-left: 8px; font-size: .78rem; font-weight: 600; - color: var(--primary); text-decoration: none; white-space: nowrap; + color: var(--text-muted); white-space: nowrap; } -.article-ref-pmid:hover { text-decoration: underline; } @media (max-width: 900px) { /* Four things in a bar this narrow is three too many; the rail and the diff --git a/frontend/src/pages/QuizPage.jsx b/frontend/src/pages/QuizPage.jsx index 95fdbae..fdb7596 100644 --- a/frontend/src/pages/QuizPage.jsx +++ b/frontend/src/pages/QuizPage.jsx @@ -1599,8 +1599,12 @@ const timerStarted = timeLeft !== null
{/* The same burger that opened it closes it: on a phone this drawer is what that button does while a session is open. */} + {/* A cross, because the menu is open. The button that opened + it kept its ☰ while the drawer covered the screen, which + reads as a second menu to open rather than the way out of + the one you are looking at. */} + onClick={() => setNavOpen(false)}>✕
diff --git a/frontend/src/pages/ResultsPage.jsx b/frontend/src/pages/ResultsPage.jsx index 2569b52..dfb5ac2 100644 --- a/frontend/src/pages/ResultsPage.jsx +++ b/frontend/src/pages/ResultsPage.jsx @@ -114,8 +114,12 @@ export default function ResultsPage() {
e.target === e.currentTarget && setNavOpen(false)}>
+ {/* A cross, because the menu is open. The button that opened + it kept its ☰ while the drawer covered the screen, which + reads as a second menu to open rather than the way out of + the one you are looking at. */} + onClick={() => setNavOpen(false)}>✕