Skip to content

ref(snapshots): Make diffing logic more strict + better handle different sized images when comparing#112632

Merged
NicoHinderling merged 1 commit intomasterfrom
04-09-ref_snapshots_make_diffing_logic_more_strict_better_handle_different_sized_images_when_comparing
Apr 10, 2026
Merged

ref(snapshots): Make diffing logic more strict + better handle different sized images when comparing#112632
NicoHinderling merged 1 commit intomasterfrom
04-09-ref_snapshots_make_diffing_logic_more_strict_better_handle_different_sized_images_when_comparing

Conversation

@NicoHinderling
Copy link
Copy Markdown
Contributor

@NicoHinderling NicoHinderling commented Apr 9, 2026

odiff only compares the overlapping region of differently-sized images — pixels outside the overlap are silently ignored. This meant dimension changes like width (200→300px) or height (2688→3676px) could report 0% diff.

Fix: pad both images to the max dimensions with transparent pixels before passing to odiff. This normalizes the canvas so odiff always compares same-sized images and correctly counts the non-overlapping area as changed pixels.

Also restores DIFF_THRESHOLD to 0.01 (was temporarily set to 0 for testing).

Copy link
Copy Markdown
Contributor Author

NicoHinderling commented Apr 9, 2026

@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Apr 9, 2026
@NicoHinderling NicoHinderling marked this pull request as ready for review April 9, 2026 21:59
@NicoHinderling NicoHinderling requested a review from a team as a code owner April 9, 2026 21:59
Copy link
Copy Markdown
Contributor

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 001b87e. Configure here.

Comment thread src/sentry/preprod/snapshots/image_diff/compare.py Outdated
Base automatically changed from 04-09-ref_snapshots_two_small_ui_tweaks to master April 9, 2026 22:06
@NicoHinderling NicoHinderling force-pushed the 04-09-ref_snapshots_make_diffing_logic_more_strict_better_handle_different_sized_images_when_comparing branch from 001b87e to ca3a049 Compare April 9, 2026 22:07
@NicoHinderling
Copy link
Copy Markdown
Contributor Author

NicoHinderling commented Apr 9, 2026

@rbro112 FWIW i tested with 0.001 and the results were pretty much the same as 0.01 but with slightly louder (and not necessarily more helpful) feedback on the diff. I think 0.01 is good compromise for now based off my testing

CleanShot 2026-04-09 at 15 22 28@2x

maybe we can try 0 properly once https://github.com/dmtrKovalenko/odiff 's v4.3.3 is actually released, but id rather do that separately

logger = logging.getLogger(__name__)

DIFF_THRESHOLD = 0
DIFF_THRESHOLD = 0.01
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Would highly recommend a comment documenting why this is .01 as I could see someone coming along and having no context getting confused

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Added a comment explaining the threshold value.

@NicoHinderling NicoHinderling force-pushed the 04-09-ref_snapshots_make_diffing_logic_more_strict_better_handle_different_sized_images_when_comparing branch from 96dfb80 to 199b40c Compare April 10, 2026 21:19
@NicoHinderling NicoHinderling merged commit f6e153a into master Apr 10, 2026
57 checks passed
@NicoHinderling NicoHinderling deleted the 04-09-ref_snapshots_make_diffing_logic_more_strict_better_handle_different_sized_images_when_comparing branch April 10, 2026 21:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Scope: Backend Automatically applied to PRs that change backend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants