feat(snapshot): publish the viewport a snapshot's rects are measured in - #3210
Conversation
…in (#3182) Carry a SnapshotViewportSize from each producer through the state, response, serialization, digest view, and client reader, published once at response level and only through the kernel guard that makes a zero-size viewport unrepresentable. Android: the snapshot helper reports the DisplayMetrics extent beside the density it already declares; the host converts the pair to a viewport at the helper-result boundary and keeps only the response fact. Apple: the host publishes the viewport evidence the presentation fold already resolves, with no new runner wire key.
Kernel: the construction guard and the wire re-read refuse zero, non-finite, and the infinite sentinel as absence. Android: both helper transports read the display pair, snapshotAndroid publishes the display beside an empty tree, the publication adapter carries it, and the backend metadata keeps no second copy.
…t, client (#3182) Runtime command publishes what its producer measured and omits what it didn't; the session state carries the box so find-path consumers read the same one; the digest survives the tree collapse; serialization emits it once; the client re-reads it through the kernel guard so a broken payload stays absent.
#3182) The proxied iOS capture now publishes the app-window box the presentation fold resolves; the parity table names it rather than the guard loosening to a size.
…ture test (#3182) parseAndroidSnapshotHelperOutput lives in snapshot-helper-capture.ts, so its viewport claim follows the source topology instead of the legacy aggregation; the capture fixture gains result lines rather than keeping a second output builder.
`--json` consumers now read the box the rects are measured in off the response instead of inferring one from the largest rect on screen. commands.md names each producer's answer, the absence rule, and the separation from content-safe gesture bounds.
…y holds (#3182) presentIosSnapshotAcquisition is the one seam that turns an acquisition into a snapshot result for the simulator AX bridge, an Appium source, and a Limrun tree, and it had the viewport evidence in hand while returning no viewport. That left the default local-iOS producer without the field the issue's first Done-when line asks for. Like the runner fold, it publishes only the box the regular projection measured against: raw validates no box at all.
The published shape carries no origin, so a wire payload with the CGRectInfinite extents and no coordinates used to mint (0, 0) around them and pass the guard, handing the caller the largest number on the wire as the screen every rect is measured in (#2891). Both viewport gates now read the extents themselves. kernel/record keeps its inlined rect rule and points at the other half of that seam.
…d it (#3182) The runner fold published the largest root even on a raw projection, where the engine validates no box at all — reinstating the largest-rect guess #3182 exists to retire under the name of a measured fact, on a payload the regular projection refuses outright. Publication now follows the validated projection, and the absence cases are pinned off the runnerFatal early return.
…question (#3182) The contract and user docs claimed a web producer that does not exist: platform-web hands over a tree and never reads a screen, and neither does Linux. They now name the two surfaces that answer and the absences that are real, including raw, which validates no box. The Node client page gains the field, following how readiness was documented.
…3182) normalizeBackendSnapshot returned a backend-built state verbatim, so a producer that named its own keyboard band or viewport box beside that state lost it on the one path where the backend was the authority. The two Android rejection helpers advertise a capture they can never yield and now say never, and the helper's one-read claim is scoped to what the code guarantees: density and extent from one DisplayMetrics, not a metrics sample taken with the dump. Proves the display read answers the geometry-free-tree case the issue came from.
…ndow (#3182) The re-capture loop #3160 split out of helper capture declared the plain xml+metadata pair, so the display read a usable attempt made was invisible to the caller while it travelled inside the object. Declaring the capture type makes the viewport the loop already carries part of its contract.
…3182) The issue's second Done-when line is a capture of an empty screen that still reports the box. On the provider acquisition seam that is now a test: the fold runs with no nodes and the reported viewport still reaches the result.
|
Size Report
Startup median (7 runs, lower is better):
|
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 41 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
|
The Android viewport reads the wrong display size, so this needs a fix and a live Android run before merge (reviewed at a31e682). The helper reads This PR changes a device-facing producer (new helper metadata keys and host derivation at https://github.com/callstack/agent-device/blob/a31e682/packages/platform-android/src/snapshot-capture.ts#L383) with no live Android run, and it closes #3182. The PR body says Android evidence rides on CI, but no CI e2e reads Could this be simpler? The iOS engine already resolves the validated viewport ( Open threads from the other review that still apply: the rotation window in the metrics read, #3210 (comment), and the plain structural The Android smoke job failed in the no-index Maestro |
…ump (#3182) Resources.getSystem() reports the app display, which excludes a persistent navigation bar, while the tree's getBoundsInScreen rects span the full panel. Read the real extent instead (WindowManager.getMaximumWindowMetrics on API 30+, Display.getRealMetrics below) before and after each dump on both the one-shot and the session path, and publish it only when the two reads agree so a rotation mid-dump omits the viewport instead of pairing one rotation's bounds with the other's dimensions.
A plain structural { width, height } accepted the zero the type claimed was
unrepresentable, and readSnapshotViewportSize stood as a second constructor
beside snapshotViewportSizeFrom. Brand the type with a token only kernel/rect
imports, so a literal with a zero in it stops being assignable, and let the
reader delegate to snapshotViewportSizeFrom so one guard decides what a usable
box is. Test fixtures now mint their viewport through the guard, which is the
mutation that proves the brand bites.
#3182) The engine already resolves the box the regular projection validates against; hand it over on the presentation result and publication instead of making snapshot-presentation.ts and ios-snapshot-runtime.ts each re-derive the rule that regular validates a box and raw does not. Callers now only pass the returned rect through the shared construction guard, so the rule lives with its owner.
…id (#3182) The capture carrier held a pre-derived viewport beside its metadata, so every re-capture hop had to thread the sibling through; a second copy of the display pair also meant two lifetimes for one fact. Carry the raw helper metadata on AndroidUiHierarchyCapture instead and answer the viewport question only in snapshotAndroid, through the shared guard, so both the healthy and the presentation-failed return publish the same box the tree was measured against.
Live Android evidence (3-button navigation) — head
|
…erge (#3182) The state seam added the backend's keyboard and viewport facts to the state without carrying the private clickability evidence over, and that evidence is retained by object identity. Android Maestro taps then read `exact-evidence-not-retained`, skip clickable-first ordering, and tap the document-order duplicate of a requested id — the Android smoke lane's no-index `tapOn` failure, which is this PR's, not a flake. The mutation that makes the new test fail: remove the copy and the evidence read on the published result is undefined.
Summary — head
|
|
The PR is ready at 5a05a3e. The Android display-extent fix, the TS clickability copy and the docs change from the earlier findings (#3210 (comment)) are all in, and nothing from that review remains open. Not blocking, and you can take or leave it: the doc on SnapshotViewportSize at https://github.com/callstack/agent-device/blob/5a05a3e/packages/kernel/src/snapshot.ts#L358 says only modules that import the brand symbol can build the type, but rect.ts mints it with a cast and never imports the symbol, so please reword the doc to say snapshotViewportSizeFrom is the sanctioned mint and casts are the escape hatch, or stop exporting the brand symbol. All 21 checks pass at 5a05a3e, and the earlier Android smoke failure came from this PR's snapshot spread, which the latest change fixes. I saw no conflicts. Nothing else must happen before merge. On the open inline threads: the Android display-extent read (#3210 (comment)), the viewport brand (#3210 (comment)) and the docs note on --raw (#3210 (comment)) are fixed at this head, so please resolve them. The non-positive rect guard (#3210 (comment)) is benign, because the guard and the fold agree and absence is allowed by the contract, so please resolve it too. For the record, the live run was at 8ff568a, not 5a05a3e. I accepted it because the only later change is the TS clickability copy, which does not touch the helper or the viewport path. The legacy API below 30 Display.getRealMetrics branch has no live run, since only API 35 was exercised. I did not run the JVM DisplayExtentTest or the vitest regression locally, and I took the live transcript, CI status and smoke reproduction as you quoted them. |
thymikee
added
the
ready-for-human
1 participant
Summary
A snapshot response now carries a response-level
viewport: { width, height }— the box the node rects are measured in, in the same coordinate space and orientation as the nodes beside it — so consumers scale and clip against the screen they were shown instead of inferring one from the largest rect (Closes #3182).Producers answer per their own surface: the Android helper reads the real display extent behind the tree (
getMaximumWindowMetricson API 30+,getRealMetricsbelow) around each dump and publishes it only when both reads agree; the Apple runner publishes the box the fold validated, which the engine now returns rather than each caller re-deriving; absent means the producer measured nothing, never a zero —snapshotViewportSizeFromis the sole construction path and the type is branded. The daemon state, digest view, serialization, client, and remote-proxy parity set all carry it.45 files: kernel/contracts types, Android helper (Java
DisplayExtent) + capture plumbing, Apple presentation, daemon/client/digest seams, tests per seam, docs.Validation
Tested at
5a05a3ea4:pnpm check:affected --rungreen (1036 files / 8404 tests, plus helper JVM suite, format, lint, typecheck, layering, fallow, build). All CI lanes green on that head: CI, Android, iOS, macOS, Linux, Size.Live evidence through the daemon CLI on an API 35 emulator with 3-button navigation (
#issuecomment-5988412865): portraitviewportequalswm sizeand the PNG, with Back/Home/Recents inside (the old app-display read ended above the nav bar); landscape swap withsessionReused/transport=persistent-session; Android--rawpublishes the viewport while Apple--rawomits it; empty-screen captures still report it on both.The Android smoke failure was this PR's, not a flake: the state-seam spread dropped WeakMap-keyed clickability evidence, so Maestro's clickable-first ordering fell back and tapped an inert duplicate. Fixed at the seam with a regression test that fails without it.