feat: golden tap-point parity table for TS and the iOS runner (ADR 0011)#1086
Merged
Conversation
ADR 0011 Layer 2: the offscreen tap-point rule now has exactly one home per side of the wire, proven equivalent by a shared golden fixture table. - contracts/fixtures/tap-point-policy.json: 12 cases including this week's real bug shapes (closed drawer fully left, 0.07pt edge-grazing container) plus edge-inclusive and empty-frame fail-open semantics. - Swift: pure TapPointPolicy.isAllowed(elementFrame:windowFrame:) extracted; the ELEMENT_OFFSCREEN guard (onScreenWindowFrame stays the frame getter) and hasTappableFrame (Maestro fallback) both delegate the decision to it. Gated XCTest asserts every table case (runs in the existing AGENT_DEVICE_RUNNER_UNIT_TESTS surface CI already compiles). - TS: isTapPointInsideViewport extracted from isNodeVisibleOnScreen (no duplicated math); vitest parity test asserts the same table. - Registry: parityTable wired on direct-ios-selector/offscreen and maestro-non-hittable-fallback/offscreen.
Size Report
Startup median (7 runs, lower is better):
Top changed chunks:
|
Member
Author
|
Reviewed by running. This is the parity mechanism working exactly as the ADR intended:
LGTM. Merge-order note: registry/pin conflicts with #1083 are expected whichever lands second — combined-removal resolution as usual. |
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implements the golden-fixture-table step of ADR 0011 Layer 2 for the
offscreenguarantee: the tap-point rule now has exactly one implementation per side of the wire, and a shared JSON table proves they agree.The rule (contracted, both sides)
Scope note: the table proves the geometric rule only. The TS
isNodeVisibleOnScreenlayers scrollable-ancestor (effective viewport) logic on top; that part is TS-only and out of scope for the table.What changed
contracts/fixtures/tap-point-policy.json— 12 cases, including this week's real bug shapes: the Bluesky closed drawer fully left of the viewport (x=-321.6, w=321.33→ tapped(-161, 265)reported as success, fix: honor wait budgets, preserve the daemon on polling timeouts, refuse off-screen selector taps #1075) and the edge-grazing container (w=321.67, 0.07pt overlap, center off-screen → still refused), plus edge-inclusive boundary pins, empty-frame fail-open, non-zero-origin window frames.TapPointPolicy.isAllowed(elementFrame:windowFrame:)(RunnerTapPointPolicy.swift, noXCUIElement). TheELEMENT_OFFSCREENguard inRunnerTests+CommandExecution.swiftnow calls it;onScreenWindowFramestays as the frame getter. A gated XCTest (AGENT_DEVICE_RUNNER_UNIT_TESTS) loads the JSON via#filePathand asserts every case.isTapPointInsideViewport(rect, viewport)extracted fromisNodeVisibleOnScreen(src/snapshot/mobile-snapshot-semantics.ts), used by both the production rule and the new parity testsrc/snapshot/__tests__/tap-point-policy-parity.test.ts. No duplicated math.parityTablewired ondirect-ios-selector/offscreen(via now points at the policy) and onmaestro-non-hittable-fallback/offscreen.hasTappableFrame: unified
hasTappableFrameused the same geometric decision (center-in-frame, empty app frame → allowed), so it now delegates toTapPointPolicytoo. Its two path-specific choices stay outside the policy and are documented at the call site: empty element frames are refused (can't coordinate-tap a frameless element), andapp.frameis deliberately the frame source (Maestro taps resolved bounds against the union frame).Deliberate micro-semantics (called out for review)
containsPointrule; previouslyCGRect.containswas max-edge-exclusive. Only differs when a center sits exactly onmaxX/maxY— pinned by a table case so it can't drift silently.centerOfRect'sMath.round, matching Swift'smidX/midYexactly. Only differs within 0.5pt of an edge. (The tap dispatch still rounds; the guard evaluates geometry.)Running the Swift test
CI's "Swift Runner Unit Compile" job compiles the unit-test surface (
AGENT_DEVICE_XCUITEST_INCLUDE_UNIT_TESTS=1 pnpm build:xcuitest:macos) but does not execute tests — unchanged. To run locally (iOS simulator; the macOS UITest runner needs a UI session and hangs headless):Verification
format:check,typecheck,lint,fallow audit --base origin/mainall green;vitest run src/snapshot src/contracts(18 tests), semantics/daemon suites (980 tests),vitest run test/integrationgreen except the known flaky-under-contention doctor timeout, which passes in isolation.origin/mainafter feat: preflight native-ref fast path with shared interaction guards (ADR 0011) #1082 (native-ref preflight) landed; gates re-run green post-rebase.