perf(thumbs): the gallery shipped 77 MB to render 250px tiles
The operator found this in about a minute of using the live Desk: "images load
at full resolution instead of calculated thumbnails, which means they load VERY
slowly and are tiny."
MEASURED on the live set:
sindra-corpus-v1 66 images 77.5 MB 1024x1024 each
sindra-sfw-pool 59 images 71.7 MB
sindra 30 images 61.6 MB 2.1 MB average
sindra-bakeoff 40 images 57.2 MB
A tile renders around 250px, so the grid shipped roughly 16x the pixels that
reach the screen.
⚠ OUR PARKING RATIONALE WAS WRONG IN AN INSTRUCTIVE WAY. ROADMAP parked
progressive loading on "the largest gallery is 66 images; at that size a lazy
grid is almost certainly fine", and the parking-lot row said "270 <img
loading=lazy> may be fine". Both count IMAGES. Neither weighs BYTES. We measured
the dimension that was easy to measure rather than the one that determines the
experience, and 66 really is a fine count sitting on a terrible payload.
booth/thumbs.py caches WebP at 512px longest side inside the booth at
`.thumbs/<rel>.webp` — inside on purpose, so a cache can never outlive what it
describes. Pillow is an optional import: absent, every tile falls back to the
original, so the page is heavier and never broken. Generation is lazy, atomic
(temp + os.replace), rebuilt when the source is newer, and NEVER RAISES.
?thumb=1 rides the EXISTING file route rather than growing a new one, because
that route's traversal guard is already correct and a second route is a second
place to get it wrong.
ALSO FIXES A PRE-EXISTING LEAK THE CACHE WOULD HAVE WALKED INTO. booth_items and
zip_booth both tested `p.name.startswith(".")` — the FILE's name — so
`.thumbs/a.png` (name `a.png`) would have rendered as a gallery item and shipped
inside every zip. CLAUDE.md invariant 2 promises a dotfile costs nothing in item
counts, galleries or zips; that was true only at the top level. Both now skip
every dot-prefixed path COMPONENT.
AND THE FILMSTRIP, which is the same defect in a worse place: it shows EVERY
ring item at a few dozen pixels, so full-resolution frames there cost more than
the grid did. The stage is untouched and stays full size, because that is the
full-size review.
Item.thumb is derived in the resolver, not by a template reasoning about `kind`
(INV-1). build_gallery had to carry it too — a missing key there rendered as a
SILENT fallback to the full image, which is exactly where a new Item field gets
dropped with nothing failing.
754 green.
This commit is contained in:
@@ -0,0 +1,163 @@
|
||||
"""Thumbnails — the fix for a gallery that shipped 77 MB to render 250px tiles.
|
||||
|
||||
The operator found this in about a minute of using the live Desk. ROADMAP had
|
||||
parked it on "the largest gallery is 66 images", which counted IMAGES and never
|
||||
weighed BYTES; 66 is a fine count sitting on a terrible payload.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import io
|
||||
import pathlib
|
||||
import sys
|
||||
|
||||
import pytest
|
||||
from fastapi.testclient import TestClient
|
||||
|
||||
sys.path.insert(0, str(pathlib.Path(__file__).parent.parent))
|
||||
|
||||
from booth.app import create_app # noqa: E402
|
||||
from booth.items import booth_items # noqa: E402
|
||||
from booth.thumbs import THUMB_DIR, THUMB_MAX, ensure_thumb, wants_thumb # noqa: E402
|
||||
|
||||
PIL = pytest.importorskip("PIL.Image", reason="Pillow is not installed")
|
||||
|
||||
|
||||
def _img(path: pathlib.Path, w: int, h: int, fmt="PNG"):
|
||||
path.parent.mkdir(parents=True, exist_ok=True)
|
||||
PIL.new("RGB", (w, h), (120, 30, 90)).save(path, fmt)
|
||||
return path
|
||||
|
||||
|
||||
def test_a_big_image_gets_a_much_smaller_thumbnail(tmp_path):
|
||||
"""The whole point, asserted in BYTES rather than in existence — a thumbnail
|
||||
that is not dramatically smaller has not fixed anything."""
|
||||
b = tmp_path / "g"
|
||||
src = _img(b / "big.png", 1024, 1024)
|
||||
out = ensure_thumb(b, "big.png")
|
||||
assert out is not None and out.is_file()
|
||||
assert max(PIL.open(out).size) <= THUMB_MAX
|
||||
assert out.stat().st_size * 4 < src.stat().st_size, (
|
||||
f"thumb {out.stat().st_size}B vs source {src.stat().st_size}B — not worth the cache"
|
||||
)
|
||||
|
||||
|
||||
def test_an_already_small_image_gets_no_thumbnail(tmp_path):
|
||||
"""Serving the original is correct when it is already tile-sized. A cache
|
||||
entry that saves nothing is pure cost.
|
||||
|
||||
Defeating change: generating unconditionally."""
|
||||
b = tmp_path / "g"
|
||||
_img(b / "small.png", 200, 200)
|
||||
assert ensure_thumb(b, "small.png") is None
|
||||
|
||||
|
||||
def test_the_cache_lives_inside_the_booth_and_is_invisible(tmp_path):
|
||||
"""`.thumbs/` is inside the booth so it is swept with it — a cache that
|
||||
outlives what it describes is a leak. And it must not become gallery items
|
||||
or zip entries: both skip every dot-prefixed path COMPONENT, which they did
|
||||
not do until this module needed them to."""
|
||||
import zipfile
|
||||
|
||||
from booth.app import zip_booth
|
||||
|
||||
b = tmp_path / "g"
|
||||
_img(b / "big.png", 1024, 1024)
|
||||
ensure_thumb(b, "big.png")
|
||||
assert (b / THUMB_DIR).is_dir(), "the cache is not inside the booth"
|
||||
assert [i.rel for i in booth_items(b)] == ["big.png"]
|
||||
assert zipfile.ZipFile(io.BytesIO(zip_booth(b))).namelist() == ["big.png"]
|
||||
|
||||
|
||||
def test_a_damaged_image_costs_its_own_tile_not_the_page(tmp_path):
|
||||
"""NEVER RAISES. A thumbnail is an optimisation; a page that will not load
|
||||
is worse than one that loads slowly.
|
||||
|
||||
Defeating change: letting the Pillow exception out."""
|
||||
b = tmp_path / "g"
|
||||
b.mkdir()
|
||||
(b / "lies.png").write_bytes(b"\x89PNG\r\n\x1a\n" + b"not an image at all" * 20)
|
||||
assert ensure_thumb(b, "lies.png") is None
|
||||
|
||||
|
||||
def test_a_stale_thumbnail_is_rebuilt(tmp_path):
|
||||
"""Editing a file in place must not leave the old thumbnail forever."""
|
||||
import os
|
||||
|
||||
b = tmp_path / "g"
|
||||
_img(b / "x.png", 1024, 1024)
|
||||
first = ensure_thumb(b, "x.png")
|
||||
before = first.stat().st_mtime_ns
|
||||
_img(b / "x.png", 900, 900)
|
||||
os.utime(b / "x.png", None)
|
||||
again = ensure_thumb(b, "x.png")
|
||||
assert again.stat().st_mtime_ns != before, "the stale thumbnail survived an edit"
|
||||
|
||||
|
||||
def test_only_thumbable_types_are_candidates():
|
||||
"""SVG is vector and Pillow cannot read it; a video is not an image."""
|
||||
assert wants_thumb("a.png") and wants_thumb("A.JPG") and wants_thumb("a.webp")
|
||||
assert not wants_thumb("a.svg") and not wants_thumb("a.webm") and not wants_thumb("a.txt")
|
||||
|
||||
|
||||
def test_the_item_record_carries_the_thumb_url(tmp_path):
|
||||
"""INV-1: the resolver decides whether an item has a thumbnail. No template
|
||||
appends `?thumb=1` by reasoning about `kind` itself."""
|
||||
b = tmp_path / "g"
|
||||
_img(b / "big.png", 1024, 1024)
|
||||
_img(b / "vec.svg", 10, 10) if False else (b / "vec.svg").write_text("<svg/>")
|
||||
by = {i.rel: i for i in booth_items(b)}
|
||||
assert by["big.png"].thumb == "big.png?thumb=1"
|
||||
assert by["vec.svg"].thumb is None
|
||||
|
||||
|
||||
def test_the_route_serves_the_thumbnail_and_the_original(tmp_path):
|
||||
"""?thumb=1 rides the EXISTING file route, so it inherits that route's
|
||||
traversal guard rather than growing a second one."""
|
||||
b = tmp_path / "g"
|
||||
src = _img(b / "big.png", 1024, 1024)
|
||||
c = TestClient(create_app(tmp_path, ttl_hours=24, start_sweeper=False))
|
||||
|
||||
full = c.get("/b/g/big.png")
|
||||
thumb = c.get("/b/g/big.png?thumb=1")
|
||||
assert full.status_code == thumb.status_code == 200
|
||||
assert len(thumb.content) * 4 < len(full.content), "the route served the full image"
|
||||
assert len(full.content) == src.stat().st_size
|
||||
|
||||
|
||||
def test_the_gallery_tile_requests_the_thumbnail(tmp_path):
|
||||
"""The operator's actual complaint: the grid pulled full-resolution files."""
|
||||
b = tmp_path / "g"
|
||||
for n in ("a.png", "b.png"):
|
||||
_img(b / n, 1024, 1024)
|
||||
c = TestClient(create_app(tmp_path, ttl_hours=24, start_sweeper=False))
|
||||
html = c.get("/b/g/").text
|
||||
assert 'src="a.png?thumb=1"' in html, "the tile still asks for the full image"
|
||||
|
||||
|
||||
def test_a_thumb_request_for_a_traversal_path_is_still_refused(tmp_path):
|
||||
"""The guard is the file route's, and it must not be weakened by the new
|
||||
query parameter."""
|
||||
b = tmp_path / "g"
|
||||
_img(b / "big.png", 1024, 1024)
|
||||
(tmp_path / "secret.txt").write_text("nope")
|
||||
c = TestClient(create_app(tmp_path, ttl_hours=24, start_sweeper=False))
|
||||
assert c.get("/b/g/../secret.txt?thumb=1").status_code in (404, 400)
|
||||
|
||||
|
||||
def test_the_filmstrip_and_tray_use_thumbnails_but_the_stage_does_not(tmp_path):
|
||||
"""The same 77 MB in a different place. The filmstrip shows EVERY ring item
|
||||
at a few dozen pixels, so full-resolution frames there are worse than the
|
||||
grid was — while the stage is the full-size review and must stay full size.
|
||||
|
||||
Defeating change: `x.url` in the filmstrip, or `it.thumb` on the stage."""
|
||||
b = tmp_path / "g"
|
||||
for n in ("a.png", "b.png", "c.png"):
|
||||
_img(b / n, 1024, 1024)
|
||||
c = TestClient(create_app(tmp_path, ttl_hours=24, start_sweeper=False))
|
||||
html = c.get("/b/g/view?f=a.png").text
|
||||
|
||||
assert 'id="vimg" src="a.png"' in html, "the stage must serve the full image"
|
||||
assert 'src="b.png?thumb=1"' in html, "the filmstrip still pulls full images"
|
||||
assert 'src="b.png"' not in html.replace('src="b.png?thumb=1"', ""), \
|
||||
"a full-size frame survived in the strip"
|
||||
Reference in New Issue
Block a user