Skip to content

fix(cli): include captured raster formats in contact sheets - #5301

Merged
miguel-heygen merged 4 commits into
heygen-com:mainfrom
user-github-me:fix/capture-contact-sheet-formats
Oct 25, 2026
Merged

miguel-heygen merged 4 commits into
heygen-com:mainfrom
user-github-me:fix/capture-contact-sheet-formats

Conversation

@user-github-me

@user-github-me user-github-me commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor

What

Include AVIF, GIF, HEIC, and TIFF assets in captured-image contact sheets, alongside the existing JPEG, PNG, and WebP formats.

Why

Capture already validates and downloads these raster formats, but its asset-sheet file list excludes them. A page with a normal AVIF feature image captures successfully with one asset, zero drops, and zero warnings, yet produces no assets/contact-sheet.jpg. The generated agent prompt then points at a missing sheet.

Related work

#5295 fixes EXIF orientation while decoding and resizing thumbnails in the same module. This PR changes only the format selection and its tests; the two fixes cover separate behavior. No matching AVIF/contact-sheet issue or other implementation found in current source and PR searches.

How

Extend the asset-sheet raster extension list to include formats already admitted by the capture image validator. Use the existing Sharp thumbnail path; animated GIFs show their first frame. SVGs retain their dedicated sheet.

Test plan

  • Three regressions fail on main (AVIF, animated GIF, TIFF), with five existing/PNG controls passing. All 21 related contact-sheet, capture post-extraction, and image-validation tests pass.
  • Actual source and built CLI captures of a local page containing this pinned public AVIF succeed with one asset, no drops or warnings, and a 1940×353 asset sheet. The thumbnail matches an independent resize (mean pixel error 4.43/255 after JPEG encoding); visually inspected the built sheet.
  • The downloaded 80,743-byte AVIF is unchanged: SHA-256 cb884c82ac7b6d4fa03b1f687e9e20abc346107095473e9c1d422aaf0de14eaf. Format tests also verify original bytes and the first frame of a two-frame GIF.
  • Full workspace build, CLI rebuild/typecheck, root lint, formatting, test reachability, comment checks, and normal commit hooks pass. Fallow reports no issues in changed files.
  • Comments follow CONTRIBUTING.md.

To reproduce, serve a plain HTML page with an <img> referencing the pinned AVIF and run hyperframes capture <url> --skip-vision --max-screenshots 1 --json. On main the image is in assets/fox-outdoors.avif with no asset sheet; with this change its labeled thumbnail appears in the sheet.

Windows validation

The first Windows engine/CLI run passed 4,486 tests and failed only when removing the animated GIF fixture (EBUSY in finally). The test now clears libvips' cached file handles before removing the temporary directory and restores its prior cache limits afterward. The GIF thumbnail and original-byte assertions remain active on Windows; all 21 related tests and CLI typecheck pass locally after the correction. The corrected signed commit 85f67e2b150b0a2bd0e4d441573ee28ce71e2be7 passes the Windows engine/CLI job; the Windows render, core, and Studio-2 jobs also pass.

Review follow-up

Merged current main (89081b919) and removed the duplicate createAssetContactSheet import reported by the merge queue. The merge preserves main's EXIF-orientation checks and this PR's AVIF, animated GIF, TIFF, and PNG coverage.

The uncorrected merge reproduces TypeScript TS2300 at both import lines; the corrected merge passes CLI typecheck and all 30 related contact-sheet, post-extraction, and image-validation tests. The full workspace build, root lint, formatting, normal commit hooks, and both comment checks also pass. Updated GitHub CI remains to run on the published commit.

@jrusso1020 jrusso1020 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Main still limits the asset contact sheet to png/jpg/jpeg/webp, while captureImageExtension in captureImageValidation.ts can save .avif, .gif, .heic and .tiff. The new list matches that validator's output set exactly. I checked the one risky entry, .heic: the validator only assigns an extension after a successful resize(1, 1).raw() decode, so a HEIC that lands in assets/ is one Sharp can decode, and it cannot crash createContactSheet, which has no per-image try/catch. The tests write real AVIF, two-frame GIF and TIFF fixtures, sample the center pixel of the first cell (244,270 lines up with the 4-col, 480px, 26px-label layout), and assert the originals are byte-identical. The libvips cache toggle keeps Windows cleanup working. Need: yes · Size: ok · Reuse: reuses the validator's format set and the existing Sharp path · Simpler: already minimal.

— Rames

@jrusso1020
jrusso1020 added this pull request to the merge queue Oct 24, 2026
@jrusso1020
jrusso1020 removed this pull request from the merge queue due to a manual request Oct 24, 2026
@jrusso1020
jrusso1020 added this pull request to the merge queue Oct 24, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Oct 24, 2026
@jrusso1020

Copy link
Copy Markdown
Collaborator

Heads up, @user-github-me: the merge queue removed this PR. Your #5295 merged first and also edits packages/cli/src/capture/contactSheet.test.ts. With both applied, typecheck fails:

src/capture/contactSheet.test.ts(15,3): error TS2300: Duplicate identifier 'createAssetContactSheet'.

Could you rebase onto the latest main and fix the duplicate import? We'll requeue it after that.

@miguel-heygen miguel-heygen left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the quick follow-up. The merge with main left a single createAssetContactSheet import (contactSheet.test.ts:14-19), so the duplicate identifier that took this out of the merge queue is gone, and the PR's net change is back to the one-line format list in contactSheet.ts:250 plus its tests. CI is green at this head, including the Windows engine/CLI job that runs these tests.

Verdict: APPROVE
Reasoning: The duplicate import that failed the queue is gone, the change is unchanged from the approved version, and the branch merges cleanly with main.

@miguel-heygen
miguel-heygen added this pull request to the merge queue Oct 25, 2026
Merged via the queue into heygen-com:main with commit bb5e0f0 Oct 25, 2026
107 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants