Image rendition slots, thumbnail seam rewiring + responsive srcset - #6028
Conversation
ImageDef ships a declared-screenshot roster: a thumb slot (170x250, the CardsGrid tile box, useAsThumbnail) plus rendition-640/rendition-1280 capture-only slots, all keyed on file content so metadata edits skip the re-decode. Capture components live in file-formats/image-captures: the thumb cover-crops like the fitted stage (letterbox-aware), renditions contain at scale-down on a transparent background. The file view model derives thumbnailUrl from the useAsThumbnail entry of meta.screenshots via the new screenshotsMeta getter on CardDef/FileDef (reserved like screenshotURLs, enforced by the eslint rule), replacing the never-produced thumbnailImage/thumbnailMetadata reads and the bespoke sourceHash staleness — file-content keying makes staleness a non-event, so the stale checks in the preview stage and fitted shell are gone with it. ImagePreview assembles srcset/sizes from the captured renditions plus the original as the largest candidate, skipping SVG (nothing to save), GIF (a rendition is a still of frame one), fitted (the stage already prefers the thumb), and sources smaller than the smallest rendition. Part of CS-12492. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Preview deploymentsHost Test Results 1 files ±0 1 suites ±0 2h 2m 44s ⏱️ - 3m 53s Results for commit 9ac314f. ± Comparison against earlier commit f908340. Realm Server Test Results 1 files ±0 202 suites ±0 1h 3m 20s ⏱️ - 8m 40s Results for commit 9ac314f. ± Comparison against earlier commit f908340. |
…nents-file' into cs-12492-image-thumbnails-rendition-slots-view-model-rewiring
…nents-file' into cs-12492-image-thumbnails-rendition-slots-view-model-rewiring
…nents-file' into cs-12492-image-thumbnails-rendition-slots-view-model-rewiring
The image family's declared-screenshot module rides card-api's universal dependency graph, so the realm-indexing tests' exact reference lists carry it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…nents-file' into cs-12492-image-thumbnails-rendition-slots-view-model-rewiring
…nents-file' into cs-12492-image-thumbnails-rendition-slots-view-model-rewiring
lukemelia
left a comment
There was a problem hiding this comment.
[Claude Code 🤖] This review went after what the consumers actually render once captures exist — srcset candidate fidelity across aspect ratios and animated formats, the roster's per-family cost profile, and capture determinism. It did not re-review the capture machinery beneath this branch, which has its own review.
Bottom line: no blocking issues, but one fidelity regression is worth fixing before this leaves draft — srcset needs an aspect gate (thread on the srcset getter in image-preview.gts).
- srcset shrinks portrait and square images, badly — gate by aspect. See the first comment on the
srcsetgetter. - Animated WebP/AVIF lose their animation when a rendition is selected, and no extracted signal exists to detect them — exclude those content types until the extractor can tell. See the second comment on the
srcsetgetter. - SVG pays two captures nothing can consume — move the rendition slots to
RasterImageDef. See the comment onIMAGE_FAMILY_SCREENSHOTSin image-captures.gts. - Capture pixels shouldn't track theme tokens — see the comment on the thumb capture's matte.
Adjacent, out of scope: sizes='100vw' overselects candidates for embedded stages that occupy a fraction of the viewport — the comment already calls this the honest ceiling, and it only costs bytes, never fidelity. If it shows up in transfer sizes later, sizes: auto (which requires loading='lazy') is the escape hatch to evaluate.
| // (their declared width is their physical width) but the descriptor | ||
| // multiplies it anyway so a future dsf change can't silently skew the | ||
| // browser's density math. | ||
| get srcset(): string | undefined { |
There was a problem hiding this comment.
[Claude Code 🤖] srcset makes portrait and square images render dramatically smaller in the reading formats — gate it by aspect ratio. A rendition artifact is the 4:3 capture box with the image contained inside, so its intrinsic aspect is 4:3 regardless of the image's. Under scale-down the browser sizes the canvas, and the content occupies only its unbanded sub-rectangle: a 3:4 portrait in a portrait-shaped stage displays at ~56% of the linear size the original would (contain-in-canvas, then canvas-contained-in-frame). Landscape images at or above 4:3 render pixel-identical whenever the frame isn't wider than the image, and lose height proportionally in wider frames (a 16:9 image gives up ~25% of its height to baked bands there).
Ask: skip srcset when this.model.aspectRatio is below 4/3 (the catastrophic class), and note the residual wide-frame height loss beside the existing tradeoff comment — or tighten the gate to a tolerance band around 4:3 if that residual matters. The margins-are-transparent reasoning in the comment is true but only covers visibility, not the size math.
Regression introduced by this feature; the gate is a few lines — worth fixing in this PR.
| // multiplies it anyway so a future dsf change can't silently skew the | ||
| // browser's density math. | ||
| get srcset(): string | undefined { | ||
| if (this.isSvg || this.model.previewKind === 'gif') { |
There was a problem hiding this comment.
[Claude Code 🤖] The animation gate covers GIF only, but WebP and AVIF can be animated too — a selected rendition silently freezes them in embedded/isolated, and no extracted signal exists to detect animation (none of the image meta extractors record a frame count or animated flag; verified across packages/base/*-meta-extractor.ts).
Ask: exclude image/webp and image/avif from srcset until the extractor records an animated flag — de-animating content is a correctness break, while oversized bytes for the static majority is only the perf win deferred. The extractor flag plus re-widening the gate is a concrete follow-up worth filing.
Gap introduced by this feature; non-blocking, small change.
| export const IMAGE_FAMILY_SCREENSHOTS: Record<string, ScreenshotSpec> = { | ||
| thumb: { | ||
| render: ImageThumbCapture, | ||
| width: 170, | ||
| height: 250, | ||
| keyBy: 'file-content', | ||
| useAsThumbnail: true, | ||
| type: 'webp', | ||
| }, | ||
| 'rendition-640': { |
There was a problem hiding this comment.
[Claude Code 🤖] SVG pays two captures per content change that nothing can ever consume: srcset excludes vectors, and the fitted cell uses thumb — so both rendition-* slots are pure prerender cost for the whole SVG family.
Ask: declare the rendition slots on RasterImageDef (image-file-def.gts) and leave only thumb here. SvgDef extends ImageDef directly while every raster leaf extends RasterImageDef, so class placement excludes SVG cleanly — and placement is the only lever, since getScreenshots validates every merged entry and has no removal-by-subclass mechanism. Residual: GifDef is a raster, so GIFs keep two unconsumed captures under the current srcset gate; that resolves itself if renditions become consumable for stills, or GIF-family placement can be revisited then.
Efficiency regression introduced by this declaration; non-blocking, cheap fix.
| } | ||
| .thumb-capture[data-image-fit='contain'] { | ||
| object-fit: contain; | ||
| background: var(--fd-paper, #f7f7f5); |
There was a problem hiding this comment.
[Claude Code 🤖] Capture pixels shouldn't track a theme token. The letterbox matte reads var(--fd-paper, #f7f7f5), so a future theme-default change silently changes what new captures bake — and because recapture is content-keyed, old and new captures then carry different mattes side by side in the same grid until each file's next content change.
Ask: hardcode the matte in capture-only components (the fixed hex is the deterministic choice here; the semantic-token convention is for live-rendered component CSS, where theming is the point).
Non-blocking, one-line change.
What this does
Stacked on #6027 (file-row declared-screenshot capture).
The image family becomes the first FileDef consumer of declared screenshots:
ImageDef(file-formats/image-captures): athumbcapture at the recommended thumbnail box (170×250, the CardsGrid tile,useAsThumbnail) plusrendition-640/rendition-1280capture-only slots at deviceScaleFactor 1 (declared width = physical width, thewdescriptor srcset needs). AllkeyBy: 'file-content', webp. The thumb cover-crops with the shared letterbox exemption (vectors and extreme proportions contain on the paper matte); renditions contain atscale-downon a transparent background so their letterbox margins are invisible wherever they're drawn.screenshotsMetagetter on CardDef/FileDef —meta.screenshotsentries verbatim (dimensions, thumbnail flag): the dimensional companion toscreenshotURLsthat srcset assembly and the view model read. Reserved likescreenshotURLs(card-api decorator + the eslint rule).thumbnailUrlnow derives from theuseAsThumbnailentry ofmeta.screenshots; the never-producedthumbnailImage/thumbnailMetadatareads and the bespoke sourceHash staleness are gone — a file-content-keyed capture recaptures when the bytes change, so there is no stale state to model. The preview stage and fitted shell drop their stale checks; the isolated inspector drops the thumbnail-metadata rows.ImagePreviewassemblessrcset/sizesfrom the captured renditions plus the original as the largest candidate — skipped for SVG (scales crisply, nothing to save), GIF (a rendition is a still of frame one), fitted (the stage already prefers the thumb capture), and sources no larger than the smallest rendition.Costs worth a reviewer's eye
Every image file now runs three captures on its first prerender pass (thumb + two renditions); unchanged bytes carry forward on subsequent passes. GIF tiles show the static thumb capture rather than the animated source (reading formats keep the animation) — a deliberate trade for grid render cost.
Test plan
Unit | file-formats): thumbnail derivation fromscreenshotsMeta(flagged entry wins, unflagged captures don't masquerade, absence reads as no thumbnail). 25/25 pass locally.sizeshint, and the GIF/vector opt-outs. 12/12 pass locally.screenshotsMeta. 131/131 pass.🤖 Generated with Claude Code