diff --git a/booth/app.py b/booth/app.py index f130d57..ad293b9 100644 --- a/booth/app.py +++ b/booth/app.py @@ -144,6 +144,7 @@ def set_blurred(booth: Path, rel: str, on: bool) -> set[str]: # The link-board logic lives in booth/links.py (stdlib only) so the `booth` CLI # can use it without pulling FastAPI in. Re-exported here because call sites and # tests already reference these names through app. +from booth.thumbs import ensure_thumb from booth.asks import ( # noqa: E402 ANSWER_SUFFIX, ASK_SUFFIX, @@ -706,6 +707,10 @@ def build_gallery(child: Path) -> list[dict]: "section": it.section, # U7. Derived in the resolver (INV-1); this only carries it. "group": it.group, + # Same: the tile's image source. Undefined here rendered as a + # SILENT fallback to the full image — the adapter is exactly + # where a new Item field gets dropped without anything failing. + "thumb": it.thumb, # R2 C1. Same rule: the resolver numbers, this carries. "ordinal": it.ordinal, "caption": it.caption, @@ -727,8 +732,12 @@ def zip_booth(booth: Path) -> bytes: buf = io.BytesIO() with zipfile.ZipFile(buf, "w", zipfile.ZIP_DEFLATED) as zf: for p in sorted(booth.rglob("*")): - if p.is_file() and not p.name.startswith("."): - zf.write(p, p.relative_to(booth).as_posix()) + # Same rule as booth_items: the Booth's dot-namespace is every + # component, not just the leaf. A `.thumbs/` cache would otherwise + # ship inside every download. + rel = p.relative_to(booth) + if p.is_file() and not any(part.startswith(".") for part in rel.parts): + zf.write(p, rel.as_posix()) return buf.getvalue() @@ -1787,7 +1796,10 @@ def create_app( flagged_rels = flagged_targets(marks) # recorded above, before this read: the current item counts as seen seen = read_seen(booth) & set(ring) - film = [{"name": r, "url": by_rel[r].url, "ordinal": by_rel[r].ordinal, + # `thumb` rides along for the filmstrip and the tray. NOT for the + # stage, which is the full-size review and must stay full size. + film = [{"name": r, "url": by_rel[r].url, "thumb": by_rel[r].thumb, + "ordinal": by_rel[r].ordinal, "kind": by_rel[r].kind, "blurred": by_rel[r].blurred, "flagged": r in flagged_rels, "seen": r in seen, "current": r == f} for r in ring] @@ -1839,7 +1851,7 @@ def create_app( ) @app.get("/b/{name}/{filepath:path}") - def booth_file(name: str, filepath: str, dl: int = 0): + def booth_file(name: str, filepath: str, dl: int = 0, thumb: int = 0): booth = resolve_booth(name) try: target = (booth / filepath).resolve() @@ -1852,6 +1864,14 @@ def create_app( # in-page with no easy "save". if dl: return FileResponse(str(target), filename=target.name) + # ?thumb=1 rides THIS route on purpose: the traversal guard above is + # already correct and a second route would be a second place to get it + # wrong. A cache miss, a damaged image or no Pillow all fall through to + # the original, so the tile is never broken — only heavier. + if thumb: + cached = ensure_thumb(booth, target.relative_to(booth).as_posix()) + if cached is not None: + return FileResponse(str(cached), media_type="image/webp") return FileResponse(str(target)) @app.post("/upload") diff --git a/booth/items.py b/booth/items.py index 5ad0a38..323e8ee 100644 --- a/booth/items.py +++ b/booth/items.py @@ -30,6 +30,7 @@ except ImportError: # pragma: no cover _markdown = None from booth.asks import is_answer_file, is_ask_file +from booth.thumbs import wants_thumb # Browser-playable media buckets. Anything else renders as a download link. IMAGE_EXTS = {".png", ".jpg", ".jpeg", ".gif", ".webp", ".avif", ".svg", ".bmp"} @@ -103,6 +104,11 @@ class Item: # and nowhere else (INV-1). APPENDED, never inserted: a mid-dataclass field # is a positional-construction break. ordinal: int + # The tile's image source, or None when the original IS the right source + # (vector, video, a type Pillow cannot open, or an image already tile-sized). + # Derived HERE so no template reasons about `kind` to decide — INV-1, which + # is the caption bug in a new field. + thumb: str | None # R2 C2: which items have been looked at full size. UI state, not judgment — @@ -283,7 +289,12 @@ def booth_items(booth: Path) -> list[Item]: """ by_rel: dict[str, Path] = {} for p in booth.rglob("*"): - if not p.is_file() or p.name.startswith("."): + # ⚠ EVERY path component, not just the filename. `p.name.startswith(".")` + # tested only the leaf, so `.thumbs/a.png` (name `a.png`) sailed through + # as a gallery item — and CLAUDE.md invariant 2 promises a dotfile costs + # nothing in item counts, galleries or zips. That promise was true only + # at the top level until the `.thumbs/` cache made it matter. + if not p.is_file() or any(part.startswith(".") for part in p.relative_to(booth).parts): continue if is_ask_file(p.name) or is_answer_file(p.name): continue @@ -331,6 +342,7 @@ def booth_items(booth: Path) -> list[Item]: # the quote() guard skipped takes no number, so the numbers # stay contiguous over what the operator can see. ordinal=len(items) + 1, + thumb=(quote(rel, safe='/') + '?thumb=1') if wants_thumb(rel) else None, ) ) return items diff --git a/booth/templates/booth.html b/booth/templates/booth.html index 3e08a95..76a6549 100644 --- a/booth/templates/booth.html +++ b/booth/templates/booth.html @@ -386,7 +386,7 @@ {% endif %} {% if it.kind == 'image' %} - {{ it.name }} + {{ it.name }} {% elif it.kind == 'video' %} {# preload="none": a booth of a dozen webms was fetching them all at page load ("metadata" still pulls real ranges per diff --git a/booth/templates/view.html b/booth/templates/view.html index eb74b50..9557cf4 100644 --- a/booth/templates/view.html +++ b/booth/templates/view.html @@ -99,7 +99,7 @@
{% for x in tray %} - {%- if x.kind == 'image' %}{% else %}{{ x.kind }}{% endif -%} + {%- if x.kind == 'image' %}{% else %}{{ x.kind }}{% endif -%} {{ num(x.ordinal) }} {% endfor %}
@@ -124,7 +124,7 @@ {% for x in film %} - {%- if x.kind == 'image' %}{% else %}{{ '♪' if x.kind == 'audio' else '▶' }}{% endif -%} + {%- if x.kind == 'image' %}{% else %}{{ '♪' if x.kind == 'audio' else '▶' }}{% endif -%} {{ num(x.ordinal) }} {% endfor %} diff --git a/booth/thumbs.py b/booth/thumbs.py new file mode 100644 index 0000000..872dfd0 --- /dev/null +++ b/booth/thumbs.py @@ -0,0 +1,99 @@ +"""Derived thumbnails, cached inside the booth. + +MEASURED, not assumed. ROADMAP parked progressive loading on "the largest +gallery is 66 images; at that size a lazy grid is almost certainly fine" — which +counted IMAGES and never weighed BYTES. The live set on 2026-09-23: + + 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 + +A tile renders around 250px wide, so the gallery shipped roughly 16x the pixels +that reach the screen and 77 MB on one page load. 66 is a fine count sitting on +a terrible payload; the operator found it in about a minute of using the Desk. + +The cache lives at `/.thumbs/.webp` — inside the booth on purpose, +so it is swept with the booth and never outlives what it describes. Both +`booth_items` and `zip_booth` skip every dot-prefixed path COMPONENT, which they +did not do until this module needed them to. +""" + +from __future__ import annotations + +import os +from pathlib import Path + +try: # optional: absence degrades to full-size images, never to a broken page + from PIL import Image as _Image +except ImportError: # pragma: no cover + _Image = None + +THUMB_DIR = ".thumbs" +THUMB_MAX = 512 # longest side, px — comfortably above any tile size +THUMB_QUALITY = 78 + +# What Pillow can open from a plain install. SVG is vector (Pillow cannot read +# it, and it is already small); AVIF needs a plugin we do not require. +THUMBABLE = {".png", ".jpg", ".jpeg", ".webp", ".gif", ".bmp"} + + +def thumb_path(booth: Path, rel: str) -> Path: + """Where `rel`'s thumbnail lives. Mirrors the tree so two files with the + same basename in different folders cannot collide.""" + return booth / THUMB_DIR / (rel + ".webp") + + +def wants_thumb(rel: str) -> bool: + """Whether a rel is a candidate at all — extension only, no file read. + + Called from the resolver for every item on every index load, so it must not + touch the disk.""" + return Path(rel).suffix.lower() in THUMBABLE + + +def ensure_thumb(booth: Path, rel: str) -> Path | None: + """The cached thumbnail for `rel`, generating it if needed. None when there + should not be one — Pillow absent, unsupported type, source already small + enough, or anything at all went wrong. + + NEVER RAISES. A thumbnail is an optimisation; a booth page that will not + load is worse than a page that loads slowly, which is the posture every + other read on this path already takes. + + Regenerates when the source is newer than the cache, so editing a file in + place does not leave the old thumbnail behind forever. + """ + if _Image is None or not wants_thumb(rel): + return None + src = booth / rel + out = thumb_path(booth, rel) + try: + s_stat = src.stat() + try: + if out.stat().st_mtime >= s_stat.st_mtime: + return out + except OSError: + pass # no cache yet, or unreadable — fall through and build one + + with _Image.open(src) as im: + # `open` reads the header only, so this is cheap enough to decide on. + if max(im.size) <= THUMB_MAX: + return None # already tile-sized; serving the original is right + im.thumbnail((THUMB_MAX, THUMB_MAX)) + if im.mode not in ("RGB", "RGBA"): + im = im.convert("RGBA" if "A" in im.getbands() else "RGB") + out.parent.mkdir(parents=True, exist_ok=True) + # Atomic, like every other sidecar this service writes: a reader + # polling the cache never sees a half-encoded image. + tmp = out.with_name(out.name + f".{os.getpid()}.tmp") + try: + im.save(tmp, "WEBP", quality=THUMB_QUALITY, method=4) + os.replace(tmp, out) + finally: + try: + tmp.unlink() + except OSError: + pass + return out + except Exception: # noqa: BLE001 — a bad image costs its own tile, never the page + return None diff --git a/pyproject.toml b/pyproject.toml index 7e950d7..be29b11 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -9,6 +9,7 @@ dependencies = [ "jinja2>=3.1", "python-multipart>=0.0.9", "markdown>=3.5", + "pillow>=10.0", ] [project.optional-dependencies] diff --git a/tests/test_items.py b/tests/test_items.py index af26cb9..4cf68f2 100644 --- a/tests/test_items.py +++ b/tests/test_items.py @@ -356,3 +356,28 @@ def test_a_huge_caption_sidecar_is_not_read_whole(tmp_path): cap = {it.rel: it.caption for it in booth_items(b)}["a.png"] assert cap is not None and len(cap) <= CAPTION_MAX + + +def test_a_dot_directory_hides_its_whole_subtree(tmp_path): + """CLAUDE.md invariant 2 claims a dotfile costs nothing in item counts, + galleries or zips. That was only true at the TOP LEVEL: both `booth_items` + and `zip_booth` tested the FILE's name, so `.thumbs/a.png` has `p.name == + "a.png"` and sailed through as a gallery item and a zip entry. + + Pre-existing, found while adding a `.thumbs/` cache. Any path component + starting with a dot is the Booth's own namespace. + + Defeating change: back to `p.name.startswith(".")`.""" + import io + import zipfile + + from booth.app import zip_booth + + b = tmp_path / "b" + (b / ".thumbs").mkdir(parents=True) + (b / "real.png").write_bytes(b"\x89PNG") + (b / ".thumbs" / "real.png").write_bytes(b"\x89PNGthumb") + (b / ".marks.json").write_text("{}") + + assert [i.rel for i in booth_items(b)] == ["real.png"] + assert zipfile.ZipFile(io.BytesIO(zip_booth(b))).namelist() == ["real.png"] diff --git a/tests/test_thumbs.py b/tests/test_thumbs.py new file mode 100644 index 0000000..7c77683 --- /dev/null +++ b/tests/test_thumbs.py @@ -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("") + 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"