feat(snapshots): Add 40M pixel limit validation for snapshot images#3179
Conversation
Validate image pixel counts before uploading to catch oversized images early, matching the backend's MAX_DIFF_PIXELS limit. Images exceeding 40,000,000 pixels are reported with their dimensions and the command fails before wasting bandwidth on uploads that would be rejected. Refs EME-885 Co-Authored-By: Claude <noreply@anthropic.com>
This comment was marked as resolved.
This comment was marked as resolved.
Cover passing, boundary, single violation, multiple violations, and empty input cases for validate_image_sizes. Co-Authored-By: Claude <noreply@anthropic.com>
NicoHinderling
force-pushed
the
feat/snapshot-pixel-limit
branch
from
e2d79c4 to
3d06806
Compare
NicoHinderling
force-pushed
the
feat/snapshot-pixel-limit
branch
from
1c4e2b5 to
3d06806
Compare
|
actually not going to bother w a changelog since we're actively developing a feature that isn't available yet, so sharing about feature changes for it doesn't really make sense |
NicoHinderling
changed the title
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
NicoHinderling
changed the title
NicoHinderling
force-pushed
the
feat/snapshot-pixel-limit
branch
from
dcc24dd to
3d06806
Compare
NicoHinderling
added
the
skip-changelog
| }) | ||
| } | ||
|
|
||
| fn validate_image_sizes(images: &[ImageInfo]) -> Result<()> { |
There was a problem hiding this comment.
Why not just do this inline on line 161 and return None/error log if the size is larger than 40M?
There was a problem hiding this comment.
The separate validate_image_sizes function is intentional. It collects and reports ALL oversized images in a single error message, so the user sees every violation at once.
Inlining with return None would silently skip oversized images and produce an incomplete snapshot without a hard failure.
We want to fail loudly here rather than let users unknowingly upload partial snapshots.
There was a problem hiding this comment.
Keeping the separate validation function intentionally — it collects ALL violations and reports them in a single error message, so the user sees every oversized image at once rather than discovering them one-by-one. The reviewer's refactoring commit (4ec1b22) already refined this function with the cleaner peekable iterator approach.
- Claude Code
szokeasaurusrex
left a comment
szokeasaurusrex
left a comment
There was a problem hiding this comment.
Lgtm, see comment for how we might improve readability a bit
Co-authored-by: Daniel Szoke <7881302+szokeasaurusrex@users.noreply.github.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
…x test assertion The reviewer's refactoring commit used img.pixels() and .join() but didn't include the ImageInfo::pixels() impl block or the itertools import. Also fixes the test assertion string to match the updated error message wording. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
3 participants
The backend enforces a 40M pixel limit per image during snapshot comparison
(
MAX_DIFF_PIXELS), but oversized images were only rejected at comparisontime, wasting upload bandwidth. The CLI now checks
width * heightagainstthe 40,000,000 pixel threshold before uploading and reports all violations
with their dimensions.
Refs EME-885