Follow-up from the review of #278, raised there and deferred out of that PR by agreement.
The contract with no coverage
#278 introduced an invariant: every <img> the hook rewrites points at a file on_post_build
actually emits. Nothing checks it. scripts/check_links.py collects hrefs only where
tag == "a", so <img src> is never validated — if the two halves disagree, all five CI checks
stay green and the figure 404s in production.
A way they can already disagree
on_files accepts any depth (src_uri.startswith("images/")), but on_post_build globs
non-recursively (images.glob("*.svg")). A themed SVG under src/images/<subdir>/ therefore has
its pages rewritten to variants that are never generated.
src/images is flat today, so this is latent rather than live. rglob closes it, and also covers
_themed being keyed by bare stem, which would otherwise collide across subdirectories.
Suggested fix
Teach check_links.py to collect img src alongside a href. That looks like a few lines and
makes the whole mechanism self-verifying — the generator and the rewriter can no longer drift
apart without CI noticing. Switch the glob to rglob in the same pass.
Smaller items from the same review
- Dead fragment handling.
stem = Path(path.split("#")[0].split("?")[0]).stem strips # and
?, but the next line tests path.lower().endswith(".svg") on the unstripped path, so
foo.svg#view never rewrites. The two lines disagree about whether fragments are supported —
pick one.
IMG_SRC matches double-quoted src only. Everything in the tree is double-quoted, so this
is latent, but it fails open and silently.- The build-time-split note landed in
pipeline.svg alone. Someone editing dj-platform.svg
will not see it. The module docstring already covers it, so dropping the note reads better than
copying it into the other 17 files.
- Both variants are fetched.
display:none does not suppress the request, so a themed figure
downloads twice. Inherent to the approach and small for SVG — recorded, not a defect to fix.
Follow-up from the review of #278, raised there and deferred out of that PR by agreement.
The contract with no coverage
#278 introduced an invariant: every
<img>the hook rewrites points at a fileon_post_buildactually emits. Nothing checks it.
scripts/check_links.pycollects hrefs only wheretag == "a", so<img src>is never validated — if the two halves disagree, all five CI checksstay green and the figure 404s in production.
A way they can already disagree
on_filesaccepts any depth (src_uri.startswith("images/")), buton_post_buildglobsnon-recursively (
images.glob("*.svg")). A themed SVG undersrc/images/<subdir>/therefore hasits pages rewritten to variants that are never generated.
src/imagesis flat today, so this is latent rather than live.rglobcloses it, and also covers_themedbeing keyed by bare stem, which would otherwise collide across subdirectories.Suggested fix
Teach
check_links.pyto collectimg srcalongsidea href. That looks like a few lines andmakes the whole mechanism self-verifying — the generator and the rewriter can no longer drift
apart without CI noticing. Switch the glob to
rglobin the same pass.Smaller items from the same review
stem = Path(path.split("#")[0].split("?")[0]).stemstrips#and?, but the next line testspath.lower().endswith(".svg")on the unstripped path, sofoo.svg#viewnever rewrites. The two lines disagree about whether fragments are supported —pick one.
IMG_SRCmatches double-quotedsrconly. Everything in the tree is double-quoted, so thisis latent, but it fails open and silently.
pipeline.svgalone. Someone editingdj-platform.svgwill not see it. The module docstring already covers it, so dropping the note reads better than
copying it into the other 17 files.
display:nonedoes not suppress the request, so a themed figuredownloads twice. Inherent to the approach and small for SVG — recorded, not a defect to fix.