feat: unify gesture planning and multi-touch execution#1212
Conversation
Size Report
Startup median (7 runs, lower is better):
Top changed chunks:
|
|
d8657ed to
4d7321b
Compare
|
Two blockers remain on
The prior Android geometry/pinch-axis/capability/Android-TV findings are resolved, and the production adapters consume the canonical plan as ADR 0013 requires. No fixer was started. |
4d7321b to
6603d28
Compare
|
I traced the unified production route and found several regressions that are not Maestro-specific: 1. iOS plan execution drops the established swipe profile and synthesis policy
This is observable outside the compat parser: any public coordinate swipe now takes the changed executor. On iOS 26.2, the react-native-pager-view suite passes only 1/9 flows on this head; the first 90% -> 10% swipe reports success but does not produce the expected page transition. The same suite passes 9/9 on Android. The unified plan should retain backend execution profiles: fling/swipe should use the proven swipe timing, pan should use continuous sampled movement, and both should retain the existing synthesized-coordinate context/policy. Two-pointer plans can continue consuming synchronized trajectories. Relevant paths:
2. Explicit coordinate gestures now require accessibility snapshots
It also adds a fresh snapshot before every repeated swipe. On Android this introduces snapshot-helper/gesture-helper churn; on iOS it adds runner work. A platform viewport should come from an AX-independent source such as the screenshot/device frame, and repetitions should reuse the validated plan/frame where the viewport cannot change within the command. Relevant paths:
3. A duration-less public swipe is now unsupported on Linux
Relevant paths:
4. Repeated-swipe safety bounds were removedThe old implementation enforced Relevant path: 5. Legacy rotate velocity compatibility changed incorrectlyHistorical rotate velocity accepted finite non-zero values, including negative/fractional input. The new compatibility parser requires a non-negative integer, rejecting previously valid values while accepting zero, which used to be invalid. Even if velocity is deprecated and ignored by planning, the compatibility boundary should accept the historical domain and report the deprecation. Relevant path: I am keeping the separate Maestro native-wait false-positive in the stacked compat work because that predates and is specific to the compat assertion strategy. |
Architecture review — solution quality + legacy cleanupReviewed the whole unified-gesture path against ADR 0013, focusing on the design rather than line-nits. Short version: the architecture is the right one and the refactor is unusually clean — but there's one structural gap that is the common root of the regressions already flagged, plus a small amount of dead machinery left behind. The design is goodThe
The one structural concern: the plan is geometry-complete but execution-profile-lossyThe plan faithfully captures where the pointers go, but This is exactly the class of regression already documented in the two review comments above (iOS pager-view 1/9, coordinate swipe now requiring AX snapshots, duration-less swipe unsupported on Linux, removed repeat-swipe bounds, swipe response-shape drop, lost recording overlays). They aren't independent bugs — they're all the unified executor discarding behavior the specialized executors used to own. The ADR itself says "fling/swipe should use the proven swipe timing, pan should use continuous sampled movement" — the plan should carry that execution intent to the adapter (it already carries Smoking gun tying this to leftover code: Legacy leftovers
Everything else I spot-checked as intentionally retained (webdriver Bottom lineArchitecturally this is a keeper — merge-worthy direction. The blocker before it ships is making the executor preserve per-intent profiles (fling/swipe vs. pan) on each adapter instead of flattening to one generic path; that single fix, plus restoring the AX-free coordinate path / Linux duration-less swipe / repeat-swipe bounds / response fields already noted, resolves the behavioral regressions. Then delete the Generated by Claude Code |
|
Follow-up: I implemented and live-verified the cross-cutting fixes from the review above. Cherry-pick: Commit: 7142eab45 What it fixes
Validation
The separate iOS native-wait exact-verification change remains only in stacked Maestro PR #1217. |
|
Four blockers remain on clean head
The generic |
|
Re-review of clean head
Three blockers remain:
All 23 checks are green, but the current tests do not exercise the two high-severity paths above. No fixer was started. |
|
Addressed the review follow-ups in
Validation: format, typecheck, lint, Fallow, focused 33 tests, and build passed. The complete unit projects ran 421 files / 3,824 assertions green; the command exit was tripped only by an unrelated screenshot retry test taking 5.27s against the 5s slow-test ceiling, and that test passed in isolation in 449ms. |
|
Re-review at
New in this delta: Low: No fixer was dispatched. |
|
Resolved the three remaining re-review blockers in
Adversarial review found and prompted the response-shape guard above; a follow-up pass found no remaining blocker. Validation:
Live iOS 26.2 (
|
|
Addressed the additional recording-semantics finding in
Validation: format, typecheck, lint, focused recording tests, Fallow, and diff check green. |
|
Found one Apple runner lifecycle bug still present on the latest #1212 head ( Deterministic reproduction on iPhone 16 / iOS 26.2: agent-device test ./.maestro/tests/pager_basic_example.yaml ./.maestro/tests/tab_view_inside_scroll_view_example.yaml --maestro --platform ios --device "iPhone 16" -e APP_ID=com.pagerviewexample --debugBefore the fix, the first flow passed and the second failed after I made the fix atomic in
Validation:
The full pager-view iOS suite still has separate Maestro/test-fixture failures, so this commit deliberately fixes only the independently proven core runner lifecycle issue. |
|
Re-review at 121a15a: the four prior production findings are fixed: Linux duration-less swipe, iOS fast-swipe and fallback implementation, rotate velocity compatibility, and multi-touch recording classification. One merge-readiness gap remains: the iOS fix is not regression-locked through the command path. Current tests exercise the standalone execution selector and response filtering, but reverting the gesture branch to the old sampled executor would still leave them green. Add a test that executes a single-pointer fling or gesture command, forces synthesis failure, and proves the fast-swipe profile plus xctest-coordinate-drag fallback. Equivalent live runner evidence for that exact failure path would also close the evidence gap. No fixer dispatched. |
121a15a to
e762801
Compare
|
Rebased onto current main and addressed the remaining review gap on head |
|
Re-review at e762801 found two remaining gaps:
Not ready yet. No fixer dispatched. |
|
Confirmed and fixed both blockers in |
Summary
This replaces parallel gesture implementations with one compositional architecture shared by CLI, Node.js, and MCP:
gesture-normalization.tsis the explicit public compatibility boundary; aliases normalize once into semantic intent.gesture-plan.tsseparates contact topology from motion and creates complete synchronized trajectories from a fresh active-app viewport.performGesture(plan)primitive and no longer derive their own geometry.GESTURE_TRAJECTORY_OUT_OF_BOUNDSdetails rather than being clamped.ADR 0013 records the architecture and platform-specific cadence/geometry profiles.
Removed legacy paths
.adpositional syntax behind one named replay compatibility codec.Interactormethods withperformGesture(plan)composition.dragSemanticswire route, per-alias runtime wrappers, dead dispatch resolution, unused wire/type aliases, and the ignored Android helperintentfield. The proven iOS fast-swipe pointer profile remains a private runner primitive selected from canonical plan intent.gesturebecame the command identity; pan/fling/pinch/rotate/transform now produce canonical visualization events, and multi-touch centroid travel cannot be mislabeled as a one-finger back swipe.adb input swipe; Drop automatic adb input swipe fallback from Android gesture swipes #690 continues to own removal of the existing one-contact fallback.Public compatibility
pointerCount: 2/--pointer-count 2opts into two fingers..adrecordings keep their positional syntax and command identity.Reconciliation with #1207
Rebased after #1207 merged and retained its intended behavior:
start/endswipes still take precedence over directional presets;Size and tightening
main.GesturePlanis not accidentally exported from the public package declaration surface.Validation
Head
769b69a43is rebased one61edf753and closes the final Linux swipe, iOS execution-profile, response-shape, rotate-velocity, multi-touch recording-semantic, and Apple runner lifecycle findings. An executable iOS XCTest forces private fling synthesis to fail, then proves the structured gesture command performs the real XCTest coordinate-drag fallback with canonical diagnostics. Missing-appBundleIddispatch also exercises the complete four-field target-cache invalidation path. These focused iOS regressions now execute in CI instead of relying on source scanning.Local validation:
Live device evidence
android-helper0.19.3, freshly built;androidSnapshot.backend=android-helperandroid-helper0.19.3gesture-lab-android.adreplay passed earlier on this PR head lineage.gesture-lab.adreplay passed in 31.6s. Expanded matrix covered pointer count, both fling directions, pinch 1.5/0.6, rotation +/-45 degrees, translation-only, zero-translation, and combined positive/negative transforms at 100/2000 ms with rendered metrics and screenshots. Upstream pager basic passed in 17.6s; a duration-less fast-swipe canary passed in 4.98s; landscape-left fast swipe changed observable state from page 0 to page 1.pointerCount: 1and moved Settings to Screen Time; fling returnedpointerCount: 1and moved onward to Apps. Physical single-pointer capability admission is confirmed.Apple runner relaunch canary: sequential
pager_basic_example.yamlandtab_view_inside_scroll_view_example.yamlflows passed 2/2 on iPhone 16 / iOS 26.2 in 45.3s. The secondlaunchAppcompleted in 2.2s without the stale-process 120-second snapshot penalty; all sessions were closed.Representative observed values:
x 0, y 0, scale 1.21, rotate 0; rotate -35:rotate -24.x 15, y 10, scale 1.10, rotate 13; strong negative:x -18, y -12, scale 0.67, rotate -24.scale 1.39; rotate -45:rotate -37.x 20, y -20, scale 1.15, rotate 15; negative long:x -8, y 8, scale 0.90, rotate -10.The Gesture Lab's main transform pan recognizer intentionally requires exactly two pointers. Therefore live one-finger topology is asserted by proving it does not activate the exact-two-pointer recognizer, while the planner/contract suite asserts its one-pointer trajectory. On iOS, a one-finger pan over the main transform target can satisfy the adjacent fling recognizer; this is recorded as a fixture limitation rather than mislabeled as pan-metric evidence.
The current Android expanded matrix was executed manually at the replay-defined coordinates because a test-suite relaunch returned the development client to its launcher/top-scroll state. Every canonical assertion and the additional negative/short/long cases passed individually. Earlier uninterrupted API 36 and API 37 replays remain recorded above; current iOS replay passed uninterrupted.
Limrun live verification is not claimed: the WIP cloud path requires uploading the freshly built helper APK, and that artifact upload was unavailable in the verification environment. The installable-helper route was inspected structurally, but that is not presented as device evidence.
Suggested review order
gesture-normalization.ts.gesture-plan.ts.