fix(core): report the tile offset a clamped upload replaced, and which snapshot it came from - #2373
Open
aryanku-dev wants to merge 1 commit into
Open
fix(core): report the tile offset a clamped upload replaced, and which snapshot it came from#2373aryanku-dev wants to merge 1 commit into
aryanku-dev wants to merge 1 commit into
Conversation
…h snapshot it came from
An App Automate iOS fullpage capture can return a negative `header_height`
for a tile. `PercyConfig.validate` repairs out-of-range values rather than
rejecting them, so the negative became 0 and the snapshot uploaded anyway —
with no top crop, which stitches the device status bar and app nav bar into
the middle of the full-page image. The only signal was:
[percy] Invalid upload options:
[percy] - tiles[1].headerHeight: must be >= 0
which names neither the snapshot nor the value that was thrown away, so a run
warning about several pages gives no way to tell them apart. Diagnosing one
such build (PER-10200) needed the stored tile rows and a hand-rebuilt stitch.
Record `received`/`clampedTo` on clamp errors, set the logging meta before
validating so the warnings are attributed to their snapshot, and report the
substitution:
[percy] - tiles[1].headerHeight: must be >= 0 (received -318, continuing with 0)
The clamp itself is unchanged — this only makes it visible. The bad value
originates device-side and is fixed there.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes the diagnosability half of PER-10200.
Problem
On App Automate iOS full-page capture, the device-side
percyScreenshotexecutor can return a negativeheader_heightfor a tile.header_heightis the number of pixels cropped off the top of a tile before stitching — it removes the fixed device chrome and the band the previous tile already showed.PercyConfig.validaterepairs out-of-range values instead of rejecting them (packages/config/src/validate.js,keyword === 'minimum'→Math.max(data, schema)), andpercy.upload()only warns and continues. So the negative silently became0, the snapshot uploaded, andpercy-apicropped nothing off the tile top — stitching the iOS status bar and the app nav bar into the middle of the full-page screenshot. The customer gets a plausible-looking but wrong baseline rather than a visible failure.The only signal was:
Two things missing there:
options.metawas assigned after the validation block, so these lines carried no meta. A run that warns about three different pages produces three identical anonymous pairs, with no way to tell whichtiles[n]belongs to which page.Diagnosing one such build needed the stored
tilesrows out of the DB plus a hand-rebuilt stitch from the raw GCS tiles. That should have been readable from the CLI output.Change
packages/config/src/validate.js— recordreceived/clampedToon clamp errors. Additive; the other twovalidate()consumers (percy.js:224,snapshot.js:255) read onlypath/messageand are unaffected.packages/core/src/percy.js— set the logging meta before validating so warnings are attributed to their snapshot, and report the substitution.The clamp behaviour itself is deliberately unchanged — this only makes it visible. The bad value originates device-side in the App Automate capture routine and is fixed there; rejecting the upload here would turn a wrong screenshot into a failed build for customers whose only recourse is an upstream fix.
Testing
packages/core/test/percy.test.js— two new specs: the clamp is reported with the received value (using the real PER-10200 tile geometry), and validation warnings carry their snapshot's meta.packages/config/test/index.test.js— existing clamp assertions updated for the new fields.@percy/config: 82/82 pass.@percy/corepercy.test.js: 137/137 pass.yarn lintclean.🤖 Generated with Claude Code