mirror of
https://github.com/software-mansion/argent.git
synced 2026-09-14 19:27:14 +08:00
feat/android-atomic-clear
7 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 filip131311
|
c52f2dba9e |
fix(screenshot-diff): say when two screenshots were resized to be compared (#689)
Fixes #617. ## Reproduced three ways **The reported false green**, using an image whose 2× downscale is lossless so no resampling noise can mask it: ``` baseline 800x1600 current 400x800 (exact half) → status: unchanged pixel_mismatch: 0% (size difference never mentioned) ``` **Worse on real screenshots** — a 1152×648 capture vs its aspect-preserving half: ``` → status: changed pixel_mismatch: 0.35% changed_areas: 8 Text changes: Disappeared "General" / Appeared "Remotes and Devices" ``` The content is identical; every region and both OCR "text changes" are resampling artifacts. So the tool doesn't merely stay silent about the size difference — it manufactures a plausible-looking regression report out of noise. **The control shows the asymmetry is the bug** — aspect-*breaking* correctly reports `dimension_mismatch: expected=1,152x648 actual=1,152x400`. Sizes are disclosed exactly when the tool refuses to compare, and never when it silently rescales. ## The fix `PngDiffResult` gains `sizeNormalization` (both original sizes plus the size compared at), populated only when a rescale actually happened — so its presence *is* the signal that the comparison carries resampling uncertainty. The summary prints it directly under the status: ``` - status: resized_no_change - size_normalized: baseline=800x1,600 current=400x800 compared_at=400x800 - the inputs share an aspect ratio but not a resolution, so the larger was downscaled before comparing; any pixel or text differences below may include resampling artifacts, and the diff images are at compared_at rather than full size - re-capture the baseline at the same scale as the current image (screenshot with scale: 1.0) to compare without resampling ``` **`resized_no_change` is checked last**, after the difference checks, so a rescaled comparison that *did* find something is still a plain `changed` — a caveat status must never soften a real regression. There's a test pinning that ordering specifically. **The name matters.** `unchanged_after_resize` contains `"unchanged"`, so the one naive gate this protects — `summary.includes("status: unchanged")` — would still read it green. `resized_no_change` contains neither `"unchanged"` nor `"changed"`. ## What I deliberately did not do **Report `dimension_mismatch` whenever sizes differ** — the issue's primary suggestion. The normalization is deliberate: `screenshot-diff/index.ts:292-294` states that same-aspect normalization *"keeps a scaled capture diff-compatible with a baseline saved at any scale"*. The `screenshot` tool defaults to **0.3** for iOS/Android while the diff's live capture is full-res, and on Android the live side falls back to the default scale when full-res capture fails. Hard-failing would break the case this code exists for. **Expose the internal `normalizeSizes` flag.** `screenshot-diff-tool.test.ts:10-23` is titled *"rejects public tuning options so defaults stay internal"* and asserts exactly this kind of param is refused. And the better remedy is the one the summary now gives — re-capture the baseline at the same scale, which the skill already instructs and which removes the resampling rather than reporting it. Worth reconsidering separately if a user actually wants "green means byte-identical". *(An earlier draft proposed exposing it via `z.coerce.boolean()`. Worth recording why that was dropped beyond the posture conflict: in zod, `z.coerce.boolean("false")` is **`true`** — so a user asking for strict comparison would have silently got normalization anyway, re-creating this exact bug through the fix's own escape hatch.)* **Suppress the OCR noise.** Text analysis runs on each *original file at its own resolution* and only rescales the resulting bounds, so a diff-time filter cannot separate "OCR read a 576px image badly" from a real text change. Suppressing it whenever normalization occurred would silence text analysis on every scaled comparison — a new, worse false green of exactly the class this issue is about. Disclosure is the honest treatment; running OCR on the normalized pair is a separate change to `text-diff`'s contract. ## Tests Three new cases plus three strengthened ones: the false green now discloses and no longer reads `unchanged`; a normalized comparison with a real change still reports `changed` *and* still discloses; equal sizes say nothing; and the retina case still compares rather than hard-failing. Mutation-verified both halves — removing the status fails the #617 regression test; moving its check before the difference checks fails the ordering test. Full tool-server suite green — 3089 passed. No existing assertion needed changing, which is also why this survived: nothing pinned it. ## Notes for the reviewer - The flow snapshot path is untouched and **cannot** reach normalization: full-screen baselines key on the capture's own pixel dimensions (`flow-visual.ts:212`), so a differently-sized capture misses the baseline entirely and fails; `cropOn` diffs already pass `normalizeSizes: false`. - `SummaryStatus` is unexported and nothing switches on it — the status reaches the outside only as text inside `summary`, which the MCP layer passes through verbatim. No skill doc, flow assertion or e2e script matches on a status string. - The diff artifacts were already produced at the normalized size; that was simply never stated. The caveat now says so. |
||
|
Jakub Piasecki
|
e23f0e285e |
feat(flow): snapshot cropOn for single-element visual diffs (#516)
`snapshot` gains an optional `cropOn: <selector>` — diff one element's
region instead of the whole screen:
`- snapshot: { name: cart-total, cropOn: "Order summary" }`
- The selector resolves like a directive target: settled tree,
auto-wait, the standard
not-found failure. Selector-only — no point form (a point has no extent
to crop to).
- The frame resolves **before** the capture, and the **cropped** image
is what gets
compared, stored as the baseline, and reported as the `current`
artifact. The pixel
rect is the normalized frame × capture size; the crop is a pngjs
`bitblt` written to
a swept scratch dir and registered only when a failure returns it.
- The baseline key stays on the **full** capture's WxH (device-class
identity), so the
previously unreachable `dimensionMismatch` guard becomes load-bearing:
an element that
grew or shrank fails with "nothing was compared" plus a
crop-fixed-size-containers
hint. Deliberately strict — a 1px layout drift fails on dimensions, not
content (the
same tradeoff as Playwright element screenshots).
- A tree-source outage while resolving `cropOn` errors the step instead
of degrading to
a full-screen compare against a cropped baseline.
- No tool-schema changes; `snapshotKey` and `--output` behavior ride
along unchanged.
Tests: parse/serialize round-trip + rejection cases, and runSnapshot
coverage with real
PNGs (crop dims, key vs. content, artifact retention/sweep, unresolved
selector, abort,
size-drift message, sub-pixel region).
|
||
|
Jakub Piasecki
|
49991e9fe1 |
feat(screenshot-diff): support full-image comparisons (#515)
## Summary - Add an explicit top-mask policy to the shared PNG differ. - Preserve status-bar masking by default while allowing isolated image regions to compare every row. - Add real-PNG regressions for top-band changes and one-row images. This prepares the differ for cropped comparisons without changing existing callers. ## Testing - Screenshot-diff tests - Tool-server build |
||
|
Ignacy Łątka
|
91277a1b3b |
test(screenshot-diff): pin that the OCR/font pass gets the normalized images (#457)
Follow-up test hardening for the merged #442 (`fix(screenshot-diff): rescale OCR text bounds into the normalized diff space`). Rides on a fresh branch off main. ## Gap `diffPngFiles` (screenshot-diff.ts) hands the OCR/font pass the *normalized* (rescaled) images and each image's decoded->normalized region scale, so the rescaled text bounds and the pixels they crop share one coordinate space. #442 pinned the per-image region scales for a single baseline-larger fixture — but that fixture left the *current*-side assertions trivial: `normalizeToCommonSize` only ever downscales the *larger* image and returns the smaller one untouched, so on the untouched side decoded == normalized and the assertion holds whether production passes the raw or the normalized value. ## Tests - Replaces the #442 region-scale test and the initial normalized-images test with one parametrized `it.each` over **both** scale directions (baseline-larger and current-larger). Each direction makes the downscaled side's `baselineImage`/`currentImage` **and** `baselineRegionScale`/`currentRegionScale` load-bearing, so all four wiring fields are pinned across the two cases. Verified by mutation: reverting any one of them to the raw decoded value / identity scale fails the matching direction (and only that direction), whereas before, reverting `currentImage` to the decoded value left the suite green. - Adds a both-directions `hasPixelDiff` test (no diff -> `false`, a pixel change -> `true`), covering the one remaining load-bearing OCR-call input that had no wiring coverage: hardcoding it (e.g. to `false`, which silently disables text analysis for every real diff) previously left the whole suite green. No production code changed. |
||
|
Ignacy Łątka
|
b8d729bb1f |
fix(screenshot-diff): rescale OCR text bounds into the normalized diff space (#442)
## Problem When two same-aspect screenshots of different resolution are compared (the explicitly supported flow, e.g. a 0.3x saved baseline vs a full-res live capture), `diffPngFiles` calls `normalizeToCommonSize` to downscale both framebuffers to a common size for the pixel diff and sets `imageSize` to those normalized dimensions. The OCR/text pass, however, ran OCR on the original decoded images: baseline OCR bounds came back in baseline-original coordinates and current OCR bounds in current-original coordinates - two different coordinate spaces. `analyzeTextRegions` then compared those bounds directly (boundsDelta / centerDistance / IoU), so identical text at the same logical position was flagged as a half-image `moved` change. The summary compounded it by normalizing the baseline bounds against the smaller normalized `imageSize`, clamping the reported "from" width to 1. A uniform rescale of one framebuffer is not a content change. ### Behavior repro The same "Settings" text rendered at 480x240 vs 240x120 (2:1, same aspect) produced one `kind:"moved"` change with a delta of roughly negative half-image and a "from" normalized width of 1. ## Fix Rescale each image's OCR text bounds (and per-word bounds) into the normalized common coordinate space before the cross-image geometric comparison and the normalized-coordinate reporting. Baseline bounds are multiplied by `(normalizedW / baselineOrigW, normalizedH / baselineOrigH)` and current bounds by `(normalizedW / currentOrigW, normalizedH / currentOrigH)`, so both sets live in the same space the pixel diff already uses. The font-geometry pass is handed the normalized images to match the rescaled bounds, and the top cutoff is derived from the normalized height. The scale factors are 1/1 when the inputs already share dimensions, so same-size comparisons are unchanged. A small pure helper `rescaleTextRegions(regions, scale)` performs the mapping; `analyzeScreenshotTextChanges` accepts optional `baselineRegionScale` / `currentRegionScale` and applies it to the OCR output, and `diffPngFiles` computes the factors from the original vs normalized dimensions. ## Test `packages/tool-server/test/screenshot-diff-ocr-coordinate-space.test.ts` is a pure unit test with no OCR engine dependency - it never shells out to tesseract, python, or PIL. It supplies synthetic OCR regions for "Settings" at the same logical location in the 480x240 baseline space and the 240x120 current space, documents the spurious half-image `moved` change from mismatched spaces, and asserts that after rescaling into the common space the geometry produces no `moved` change and sane normalized coordinates. A pipeline-level case mocks the OCR module and verifies the same through `analyzeScreenshotTextChanges` with per-image scales. |
||
|
Ignacy Łątka
|
01eca45eb4 |
fix(tool-server): compare same-aspect screenshots in screenshot-diff instead of failing on a resolution mismatch (#285)
## Problem
`screenshot-diff` rejects the workflow its own docs call "the common
visual-regression flow" — `baselinePath` + `captureCurrent: true`.
The `screenshot` tool saves PNGs at a default scale of `0.3` (e.g.
`396x860`), while `screenshot-diff`'s live capture is hardcoded to full
resolution `1.0` (e.g. `1320x2868`). Feeding a saved `0.3x` baseline
together with a `1.0x` live capture returns `status: dimension_mismatch`
and refuses to compare — even though both images are uniform scalings of
the same framebuffer and share an identical aspect ratio. The advertised
default workflow is therefore broken out of the box.
## Root cause
`diffPngFiles` (in
`packages/tool-server/src/tools/screenshot-diff/screenshot-diff.ts`)
treated **any** difference in pixel dimensions as a hard failure:
```ts
if (baseline.width !== current.width || baseline.height !== current.height) {
return summarizeResult(buildDimensionMismatchResult({ baseline, current }));
}
```
There was no allowance for two images that depict the same content at
different scales.
## Fix
Before the hard dimension-equality check, normalize the two decoded
images to a common size when their aspect ratios match (within a 1%
tolerance, to absorb integer-rounding of pixel dimensions). The
**larger** image is downscaled to the **smaller** one's exact dimensions
using the existing Lanczos3 resampler — we never upscale, so we never
invent detail. When the aspect ratios genuinely differ, the existing
`dimension_mismatch` hard-fail is preserved unchanged.
- New pngjs-free module
`packages/tool-server/src/tools/screenshot-diff/resize.ts` exports
`normalizeToCommonSize(baseline, current)` (returns same-sized images,
or `null` when aspects differ) and `resizeDecodedPng(src, width,
height)`. The pure Lanczos3 resampling math (which has no `pngjs`
dependency) was moved here; `screenshot-diff.ts` now imports it, and its
`downscalePng` delegates to `resizeDecodedPng`, so there is a single
resampler implementation.
- The OCR / font-geometry pass (`analyzeScreenshotTextChangesSafely` →
`detectFontGeometryChange`) re-reads the original files from disk and
samples in-memory pixels using OCR-derived bounds in those files'
coordinate space. It therefore keeps receiving the **originally
decoded** images (not the normalized copies), and its top cutoff is
derived from the original baseline height. Only the pixel-diff path
operates on the normalized, same-sized images. This keeps the text pass
byte-for-byte consistent with prior behavior.
## Testing
- **`packages/tool-server/test/screenshot-diff-resize.test.ts`** (new,
pngjs-free, runs locally): unit tests for the new pure helpers —
same-aspect different-size pairs normalize to the smaller common size;
uniform color survives the resample; transposed and out-of-tolerance
aspect ratios return `null`; an in-tolerance (~0.1%) aspect difference
still normalizes. **11/11 passing locally.**
- **`packages/tool-server/test/screenshot-diff.test.ts`** (existing):
added an integration case asserting that a same-aspect,
different-resolution baseline/current pair is **not** reported as
`dimension_mismatch` and produces a diff at the smaller common size.
This is the true before/after for the bug.
### CI caveat
`screenshot-diff.ts` imports `pngjs` at module load, so
`screenshot-diff.test.ts` (and the integration case above) can only run
in an environment where `pngjs` is installed — i.e. **CI**. The new
`resize.ts` and `screenshot-diff-resize.test.ts` are intentionally
`pngjs`-free so the core normalization logic is verifiable without that
dependency. Typecheck (`tsc --noEmit -p tsconfig.test.json`) is clean
for the changed files.
|
||
|
Hubert Gancarczyk
|
de0dbbda66 |
feat(tools): implement screenshot-diff tool (#229)
## Summary Adds a new `screenshot-diff` tool for comparing mobile screenshots during UI QA and visual regression checks. The tool can compare two saved PNGs, or compare one saved PNG against one live full-resolution device capture, then returns a compact text summary and generated diff artifacts. ## Context Mobile UI validation often needs more than a single screenshot: small layout, color, typography, copy, clipping, and regression changes are easy to miss manually. This feature gives agents a deterministic before/after comparison path while keeping `screenshot-diff` as supporting evidence. ## What Changed - Registered a new `screenshot-diff` tool in the tool server. - Supports saved PNG comparisons and one-sided live capture via `captureBaseline` or `captureCurrent`. - Adds PNG diffing with fixed internal defaults, changed-region detection, status-bar-band ignoring, full-size diff output, and downscaled context diff output. - Adds optional OCR/text analysis for moved, appeared, disappeared, changed, and restyled text. - Updates MCP content rendering to attach the downscaled context diff image plus the text summary. 1. Capture a stable full-resolution baseline. 2. Perform an interaction or apply a UI change. 3. Compare the current screen against the baseline. 4. Review a summarized diff plus generated artifacts. |