Skip to content

fix(diff): include maximum RGB distance at threshold one - #2508

Merged
thymikee merged 1 commit into
callstack:mainfrom
PLASMA-FR:fix/screenshot-diff-threshold-a-20260911-umvmlw58
Sep 12, 2026
Merged

fix(diff): include maximum RGB distance at threshold one#2508
thymikee merged 1 commit into
callstack:mainfrom
PLASMA-FR:fix/screenshot-diff-threshold-a-20260911-umvmlw58

Conversation

@PLASMA-FR

Copy link
Copy Markdown
Contributor

Summary

agent-device diff screenshot --baseline black.png white.png --threshold 1 reported 100% different pixels instead of a match. The maximum RGB distance rounded one floating-point step below the per-pixel calculation.

Use the same square-root calculation for the maximum. This completes the existing Euclidean comparison and saved-image workflow from #214 and #404; thresholds below 1 still detect maximum contrasts. Five files change: the calculation, regression tests, command help, and documentation.

Validation

Tested cc92eb97d6c07314b87641aa19ff10ae516def59 with Node 24.13.1 and pnpm 11.17.0 on Linux:

  • Six new regression cases fail without the fix.
  • Focused screenshot, runtime, CLI, PNG-worker and startup suites: 82 tests pass.
  • pnpm build and pnpm check:command-docs pass (12 documentation tests).
  • Fourteen built-CLI text/JSON cases pass, including the nearest threshold below 1, dimension mismatches, and stale artifact cleanup.
  • pnpm check:affected --run: all runnable checks pass, including 3,062 tests across 414 files.

CI pending. Saved-image comparisons require no device; no local native run.

@thymikee

Copy link
Copy Markdown
Member

No code findings at cc92eb9. The maximum RGB distance now follows the same rounding as the pixel comparison, and the regressions cover threshold 1 and the boundary immediately below it. Current-head checks pass and there are no conflicts; ready for human review and merge.

@thymikee thymikee added the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Sep 12, 2026
@thymikee
thymikee merged commit 5643107 into callstack:main Sep 12, 2026
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-for-human Valid work that needs human implementation, judgment, or maintainer merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants