fix: a question's figure says what the question says, not what the library says
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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TqXevQJhxFrM7jJg82cgZN
This commit is contained in:
parent
dbfdff90a1
commit
734ff194f2
10 changed files with 106 additions and 23 deletions
|
|
@ -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,
|
||||
}
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -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]
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
}
|
||||
|
|
|
|||
52
backend/tests/test_question_figures.py
Normal file
52
backend/tests/test_question_figures.py
Normal file
|
|
@ -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()
|
||||
|
|
@ -295,10 +295,11 @@ export default function ArticleReader({
|
|||
{' · '}{[ref.journal, ref.year].filter(Boolean).join(', ')}
|
||||
</span>
|
||||
)}
|
||||
{ref.pmid && (
|
||||
<a className="article-ref-pmid" href={ref.url || `https://pubmed.ncbi.nlm.nih.gov/${ref.pmid}/`}
|
||||
target="_blank" rel="noopener noreferrer">PMID {ref.pmid}</a>
|
||||
)}
|
||||
{/* 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 && <span className="article-ref-pmid">PMID {ref.pmid}</span>}
|
||||
</li>
|
||||
))}
|
||||
</ol>
|
||||
|
|
|
|||
|
|
@ -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 = '',
|
|||
</span>
|
||||
|
||||
{open && (
|
||||
<ImageViewer src={src} alt={alt} attemptId={attemptId} onClose={() => setOpen(false)} />
|
||||
<ImageViewer src={src} alt={alt} attemptId={attemptId}
|
||||
libraryInfo={libraryInfo} onClose={() => setOpen(false)} />
|
||||
)}
|
||||
</>
|
||||
)
|
||||
|
|
|
|||
|
|
@ -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 }) => (
|
||||
<ImageFigure src={src} alt={alt} attemptId={attemptId} />
|
||||
// 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.
|
||||
<ImageFigure src={src} alt={alt} attemptId={attemptId} libraryInfo={!attemptId} />
|
||||
),
|
||||
// `{{phrase|tip}}` — a teaching point that opens where the phrase is.
|
||||
span: ({ node, children, ...props }) => (
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -1599,8 +1599,12 @@ const timerStarted = timeLeft !== null
|
|||
<div className="quiz-drawer-head">
|
||||
{/* 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. */}
|
||||
<button type="button" className="quiz-drawer-close" aria-label="Close"
|
||||
onClick={() => setNavOpen(false)}>☰</button>
|
||||
onClick={() => setNavOpen(false)}>✕</button>
|
||||
<div className="quiz-drawer-tabs" role="tablist">
|
||||
<button type="button" role="tab" aria-selected={drawerTab === 'menu'}
|
||||
onClick={() => setDrawerTab('menu')}>Main menu</button>
|
||||
|
|
|
|||
|
|
@ -114,8 +114,12 @@ export default function ResultsPage() {
|
|||
<div className="quiz-drawer" onClick={e => e.target === e.currentTarget && setNavOpen(false)}>
|
||||
<div className="quiz-drawer-panel" role="dialog" aria-modal="true" aria-label="Answer review">
|
||||
<div className="quiz-drawer-head">
|
||||
{/* 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. */}
|
||||
<button type="button" className="quiz-drawer-close" aria-label="Close"
|
||||
onClick={() => setNavOpen(false)}>☰</button>
|
||||
onClick={() => setNavOpen(false)}>✕</button>
|
||||
<div className="quiz-drawer-tabs" role="tablist">
|
||||
<button type="button" role="tab" aria-selected="true">Questions</button>
|
||||
</div>
|
||||
|
|
|
|||
Loading…
Reference in a new issue