iphone: coordinate-space handling for tap/swipe + screenshot metadata (#1317)

## Why

`iphone.tap`/`swipe` take **points**; `iphone.screenshot` returns
**pixels** (points × display scale, 3× on this device). Reading a
coordinate off a screenshot and passing it to `tap` lands `scale`× too
far — often off-screen entirely. Hit this live driving a real iPhone:
taps read off screenshots silently missed.

## What

- `tap(x, y, *, space="points")` and `swipe(..., space="points")` accept
`"points"` (default, back-compat), `"pixels"` (screenshot coords), or
`"fraction"` (0..1, robust to image rescaling). Conversion factored into
a pure, unit-tested `_to_points`.
- `screen()` and `window_size()` helpers expose geometry (points,
pixels, scale).
- `screenshot()` stamps `img.info` with `scale` / `point_size` /
`pixel_size` so an image is self-describing.
- UI actions (`tap`/`swipe`/`press`/`type_text`/`home`/`tap_element`)
raise a one-line `IphoneError` naming `wda_start` when WDA is down,
instead of a deep urllib stack.
- Docstrings (the `api()`/`help()` source of truth) steer toward
`tap_element(label=...)` and `space="fraction"` over hand-converted
pixels.

## Verification

Local: `nix build .#mcp.tests.iphoneTests` → 18 passed;
`.#mcp.tests.iphoneBundled` → ok; `nix run .#lint` clean.

Live experiment (real iPhone, scale 3, 402×874 pt), 8 on-screen targets:

| arm | hit rate |
|---|---|
| baseline (pixels-as-points) | **0/8** |
| `space="pixels"` | **8/8** |
| `space="fraction"` | **8/8** |

End-to-end: tapping a dock icon's pixel center as points did **not**
launch the app (reproduces the bug); `space="pixels"` launched it.
`tap_element(label=...)` and `screenshot().info` stamping confirmed
live.

<!-- Macroscope's pull request summary starts here -->
<!-- Macroscope will only edit the content between these invisible
markers, and the markers themselves will not be visible in the GitHub
rendered markdown. -->
<!-- If you delete either of the start / end markers from your PR's
description, Macroscope will append its summary at the bottom of the
description. -->
> [!NOTE]
> ### Add coordinate-space handling for `tap`/`swipe` and screenshot
metadata to iPhone module
> - `tap` and `swipe` now accept float coordinates and a `space`
parameter (`'points'`, `'pixels'`, or `'fraction'`), resolving
coordinates via new helpers `_to_points` and `_resolve_points` in
[__init__.py](https://github.com/indexable-inc/index/pull/1317/files#diff-f92c2ac3b6f1cee30bc3c534269f16357522a2d23c4c9e1e984a8f32030227b7).
> - New `screen` and `window_size` async helpers expose screen geometry
(points, pixels, scale) by querying WDA.
> - `screenshot` now populates `Image.info` with `scale`, `pixel_size`,
and `point_size` when captured via WDA.
> - All UI actions (`tap`, `swipe`, `press`, `type_text`, `home`,
`tap_element`) now call `_require_wda()` and raise `IphoneError`
immediately if WDA is unreachable.
> - Cached display scale (`_wda_scale`) is reset on `wda_start` and
`wda_stop` to prevent stale values across sessions or devices.
> - Behavioral Change: `tap` and `swipe` signatures now accept `float`
instead of `int`; passing an unknown space or out-of-range fraction
raises `IphoneError`.
>
> <!-- Macroscope's review summary starts here -->
>
> <sup><a href="https://app.macroscope.com">Macroscope</a> summarized
1720daa.</sup>
> <!-- Macroscope's review summary ends here -->
>
<!-- macroscope-ui-refresh -->
<!-- Macroscope's pull request summary ends here -->

Author: andrewgazelka

packages/mcp/src/iphone/iphone/__init__.py

@@ -29,7 +29,9 @@
   full Xcode + an Apple ID in Xcode) and started (``wda_start(sudo=True)``);
   ``source()`` returns the live accessibility tree (with rects) as a polars
   frame, which is ground truth for where to tap even when a GPU/Metal view
-  screenshots black.
+  screenshots black. Taps take POINTS; screenshots come back in PIXELS (``scale``×
+  bigger). Prefer ``tap_element(label=...)`` or ``tap(px, py, space="pixels")`` /
+  ``space="fraction"`` over hand-converting (see ``screen()``).
 
 Run ``iphone.doctor()`` to see exactly which prerequisite is missing.
 
@@ -76,6 +78,7 @@
     "kill",
     "launch",
     "press",
+    "screen",
     "screenshot",
     "simulate_location",
     "source",
@@ -92,6 +95,7 @@
     "wda_start",
     "wda_status",
     "wda_stop",
+    "window_size",
 ]
 
 _TUNNELD_HOST = "127.0.0.1"
@@ -242,6 +246,11 @@ async def screenshot(udid: str | None = None) -> Image.Image:
     running, captures via WDA instead: while an XCUITest session holds the
     display, the DVT screenshot returns a black frame, so WDA is the reliable
     source during automation.
+
+    The image is in PIXELS, which is ``scale``× the point grid taps use. When WDA
+    is up the returned image's ``.info`` carries ``scale``, ``point_size`` and
+    ``pixel_size`` so coordinates can be reconciled; to tap a spot you see here,
+    pass ``tap(px, py, space="pixels")`` rather than the raw numbers.
     """
     import base64
     import io
@@ -254,7 +263,12 @@ async def screenshot(udid: str | None = None) -> Image.Image:
         value = await _wda("GET", "/screenshot")
         png = base64.b64decode(str(value))
         with Image.open(io.BytesIO(png)) as img:
-            return img.copy()
+            shot = img.copy()
+        scale = await _screen_scale()
+        shot.info["scale"] = scale
+        shot.info["pixel_size"] = shot.size
+        shot.info["point_size"] = (round(shot.width / scale), round(shot.height / scale))
+        return shot
 
     target = await _resolve(udid)
     with tempfile.TemporaryDirectory() as tmp:
@@ -542,6 +556,9 @@ async def clear_location(udid: str | None = None) -> None:
 _wda_xcuitest_proc: asyncio.subprocess.Process | None = None
 _wda_forward_proc: asyncio.subprocess.Process | None = None
 _wda_session: str | None = None
+# Display scale (pixels per point) of the bound device. Fixed per device, so
+# cache it after the first lookup; cleared whenever a fresh runner is bound.
+_wda_scale: float | None = None
 # The device the running WDA runner/forward target, so wda_start does not silently
 # report "already running" for a different udid than the caller asked for.
 _wda_device: str | None = None
@@ -676,8 +693,111 @@ def walk(node: object) -> None:
     return pl.DataFrame(rows, schema=schema)
 
 
-async def tap(x: int, y: int) -> None:
-    """Tap at screen coordinates (points) on the active WDA device, via W3C actions."""
+async def _require_wda() -> None:
+    """Raise a one-line, actionable error when WDA is not reachable.
+
+    UI actions otherwise fail deep inside a urllib stack on the first HTTP call,
+    which reads as a crash rather than a missing precondition.
+    """
+    if not await _wda_up():
+        raise IphoneError(
+            "WebDriverAgent is not reachable; run iphone.wda_start(sudo=True) "
+            "first (one-time install via iphone.wda_build_install())."
+        )
+
+
+# Coordinate spaces a caller may supply. Taps/swipes/element rects are in POINTS
+# (what WDA wants); screenshots come back in PIXELS (points × display scale); a
+# FRACTION is a 0..1 share of the screen, invariant to how an image was rendered.
+_COORD_SPACES = ("points", "pixels", "fraction")
+
+
+def _to_points(
+    x: float,
+    y: float,
+    space: str,
+    scale: float,
+    size: tuple[int, int],
+) -> tuple[int, int]:
+    """Convert an (x, y) in ``space`` to integer device points.
+
+    Pure (no I/O) so it is unit-testable: ``scale`` is pixels-per-point and
+    ``size`` is the screen's (width, height) in points.
+    """
+    if space == "points":
+        return round(x), round(y)
+    if space == "pixels":
+        return round(x / scale), round(y / scale)
+    if space == "fraction":
+        if not (0.0 <= x <= 1.0 and 0.0 <= y <= 1.0):
+            raise IphoneError(f"fraction coordinates must be in 0..1, got ({x}, {y})")
+        return round(x * size[0]), round(y * size[1])
+    raise IphoneError(f"unknown coordinate space {space!r}; use one of {_COORD_SPACES}")
+
+
+async def _screen_scale() -> float:
+    """Display scale (pixels per point) of the bound device, cached."""
+    global _wda_scale
+    if _wda_scale is None:
+        info = await _wda_in_session("GET", "/wda/screen")
+        scale = info.get("scale") if isinstance(info, dict) else None
+        if not isinstance(scale, (int, float)) or scale <= 0:
+            raise IphoneError(f"WDA reported no usable display scale: {info!r}")
+        _wda_scale = float(scale)
+    return _wda_scale
+
+
+async def window_size() -> tuple[int, int]:
+    """Screen size in points (width, height) for the active orientation."""
+    value = await _wda_in_session("GET", "/window/size")
+    if not isinstance(value, dict) or "width" not in value or "height" not in value:
+        raise IphoneError(f"WDA window size returned no dimensions: {value!r}")
+    return int(value["width"]), int(value["height"])
+
+
+async def screen() -> dict[str, object]:
+    """Screen geometry: points (w, h), pixels (w, h), and the pixels/point scale.
+
+    Use this to reconcile a screenshot (pixels) with taps (points): a control at
+    pixel (px, py) in a screenshot is at point (px/scale, py/scale). Prefer
+    ``tap_element`` or ``tap(..., space="fraction")`` so you never do the math by
+    hand.
+    """
+    pts = await window_size()
+    scale = await _screen_scale()
+    return {
+        "points": pts,
+        "pixels": (round(pts[0] * scale), round(pts[1] * scale)),
+        "scale": scale,
+    }
+
+
+async def _resolve_points(x: float, y: float, space: str) -> tuple[int, int]:
+    """Turn caller coordinates in ``space`` into device points (fetching geometry
+    only when the space actually needs it)."""
+    if space == "points":
+        return _to_points(x, y, space, 1.0, (0, 0))
+    scale = await _screen_scale() if space == "pixels" else 1.0
+    size = await window_size() if space == "fraction" else (0, 0)
+    return _to_points(x, y, space, scale, size)
+
+
+async def tap(x: float, y: float, *, space: str = "points") -> None:
+    """Tap the screen via WDA's W3C actions.
+
+    ``space`` selects the coordinate system of ``x``/``y``:
+    ``"points"`` (default, what WDA and ``source()`` rects use), ``"pixels"``
+    (as read off a ``screenshot()``), or ``"fraction"`` (0..1 share of the
+    screen, robust to image rescaling).
+
+    Prefer ``tap_element(label=...)`` when the control has an accessibility
+    label, or ``space="fraction"`` when eyeballing a screenshot. Never feed pixel
+    coordinates read off a screenshot as default (points): a screenshot is
+    ``scale``× larger than the point grid (see ``screen()``), so they overshoot
+    and land off-screen.
+    """
+    await _require_wda()
+    px, py = await _resolve_points(x, y, space)
     await _wda_in_session(
         "POST",
         "/actions",
@@ -688,7 +808,7 @@ async def tap(x: int, y: int) -> None:
                     "id": "finger1",
                     "parameters": {"pointerType": "touch"},
                     "actions": [
-                        {"type": "pointerMove", "duration": 0, "x": x, "y": y},
+                        {"type": "pointerMove", "duration": 0, "x": px, "y": py},
                         {"type": "pointerDown", "button": 0},
                         {"type": "pause", "duration": 90},
                         {"type": "pointerUp", "button": 0},
@@ -700,7 +820,13 @@ async def tap(x: int, y: int) -> None:
 
 
 async def tap_element(*, name: str | None = None, label: str | None = None) -> None:
-    """Find an element by accessibility name or label and tap its center."""
+    """Find an element by accessibility name or label and tap its center.
+
+    The most reliable way to tap: rects come straight from the accessibility tree
+    in points, so there is no coordinate-space guesswork. Prefer this over raw
+    ``tap`` whenever the target has a name/label.
+    """
+    await _require_wda()
     frame = await source()
     import polars as pl
 
@@ -720,14 +846,22 @@ async def tap_element(*, name: str | None = None, label: str | None = None) -> N
 
 
 async def swipe(
-    start_x: int,
-    start_y: int,
-    end_x: int,
-    end_y: int,
+    start_x: float,
+    start_y: float,
+    end_x: float,
+    end_y: float,
     *,
     duration: float = 0.3,
+    space: str = "points",
 ) -> None:
-    """Swipe between two screen coordinates (points) via the W3C Actions API."""
+    """Swipe between two screen coordinates via the W3C Actions API.
+
+    ``space`` is the coordinate system of both endpoints — ``"points"`` (default),
+    ``"pixels"`` (screenshot coordinates), or ``"fraction"`` (0..1) — see ``tap``.
+    """
+    await _require_wda()
+    sx, sy = await _resolve_points(start_x, start_y, space)
+    ex, ey = await _resolve_points(end_x, end_y, space)
     await _wda_in_session(
         "POST",
         "/actions",
@@ -738,9 +872,9 @@ async def swipe(
                     "id": "finger1",
                     "parameters": {"pointerType": "touch"},
                     "actions": [
-                        {"type": "pointerMove", "duration": 0, "x": start_x, "y": start_y},
+                        {"type": "pointerMove", "duration": 0, "x": sx, "y": sy},
                         {"type": "pointerDown", "button": 0},
-                        {"type": "pointerMove", "duration": int(duration * 1000), "x": end_x, "y": end_y},
+                        {"type": "pointerMove", "duration": int(duration * 1000), "x": ex, "y": ey},
                         {"type": "pointerUp", "button": 0},
                     ],
                 }
@@ -751,16 +885,19 @@ async def swipe(
 
 async def type_text(text: str) -> None:
     """Type into the focused field (tap a field first) via WDA."""
+    await _require_wda()
     await _wda_in_session("POST", "/wda/keys", {"value": list(text)})
 
 
 async def press(button: str) -> None:
     """Press a hardware button via WDA (home / volumeUp / volumeDown)."""
+    await _require_wda()
     await _wda_in_session("POST", "/wda/pressButton", {"name": button})
 
 
 async def home() -> None:
     """Go to the home screen via WDA (the home-button gesture)."""
+    await _require_wda()
     await _wda_in_session("POST", "/wda/pressButton", {"name": "home"})
 
 
@@ -912,7 +1049,7 @@ async def wda_start(*, sudo: bool = False, udid: str | None = None) -> str:
     the developer tunnel, ``sudo=True`` (same root-daemon opt-in as
     ``start_tunneld``). Returns once WDA answers, with a session ready for taps.
     """
-    global _wda_xcuitest_proc, _wda_forward_proc, _wda_session, _wda_device
+    global _wda_xcuitest_proc, _wda_forward_proc, _wda_session, _wda_device, _wda_scale
 
     target = await _resolve(udid)
     if await _wda_up():
@@ -935,7 +1072,9 @@ async def wda_start(*, sudo: bool = False, udid: str | None = None) -> str:
         )
     if _wda_xcuitest_proc is None or _wda_xcuitest_proc.returncode is not None:
         # A fresh runner means any cached session id is dead; force a new one.
+        # The scale belongs to the device being (re)bound, so re-read it too.
         _wda_session = None
+        _wda_scale = None
         _wda_device = target
         _wda_xcuitest_proc = await asyncio.create_subprocess_exec(
             *_pmd3_argv(),
@@ -983,10 +1122,14 @@ async def _terminate(proc: asyncio.subprocess.Process | None) -> None:
 
 async def wda_stop() -> None:
     """Stop the WDA runner and port-forward started by this module."""
-    global _wda_xcuitest_proc, _wda_forward_proc, _wda_session, _wda_device
+    global _wda_xcuitest_proc, _wda_forward_proc, _wda_session, _wda_device, _wda_scale
 
     _wda_session = None
     _wda_device = None
+    # Drop the cached scale too: a later wda_start(udid=None) against an
+    # externally-reachable WDA takes the "already running" early return and never
+    # re-reads it, so a stale scale would mis-convert taps on a different device.
+    _wda_scale = None
     await _terminate(_wda_forward_proc)
     await _terminate(_wda_xcuitest_proc)
     _wda_forward_proc = None

packages/mcp/tests/test_iphone.py

@@ -86,6 +86,74 @@ def test_tap_is_coordinate_based() -> None:
     # tap moved from a (broken) WDA selector to W3C coordinate taps.
     params = list(inspect.signature(iphone.tap).parameters)
     assert params[:2] == ["x", "y"], params
+    # The coordinate space is opt-in and defaults to points (back-compat).
+    assert inspect.signature(iphone.tap).parameters["space"].default == "points"
+
+
+def test_to_points_identity_for_points() -> None:
+    # Points pass through unchanged (only rounded to ints).
+    assert iphone._to_points(10.4, 20.6, "points", 3.0, (402, 874)) == (10, 21)
+
+
+def test_to_points_pixels_divides_by_scale() -> None:
+    # A screenshot pixel (1206, 2622) on a scale-3 device is point (402, 874):
+    # exactly the bug we hit (pixels read as points landed 3× too far).
+    assert iphone._to_points(1206, 2622, "pixels", 3.0, (402, 874)) == (402, 874)
+
+
+def test_to_points_fraction_scales_to_size() -> None:
+    assert iphone._to_points(0.5, 0.5, "fraction", 3.0, (402, 874)) == (201, 437)
+    assert iphone._to_points(0.0, 1.0, "fraction", 3.0, (402, 874)) == (0, 874)
+
+
+def test_to_points_pixels_roundtrip_over_grid() -> None:
+    # center(points) -> pixels(×scale) -> back must recover the point, for a
+    # spread of real-looking coordinates and scales.
+    for scale in (2.0, 3.0):
+        for px in range(0, 402, 37):
+            for py in range(0, 874, 53):
+                bx, by = px * scale, py * scale
+                assert iphone._to_points(bx, by, "pixels", scale, (402, 874)) == (px, py)
+
+
+def test_to_points_fraction_out_of_range_raises() -> None:
+    with pytest.raises(iphone.IphoneError, match=r"0\.\.1"):
+        iphone._to_points(1.5, 0.5, "fraction", 3.0, (402, 874))
+
+
+def test_to_points_unknown_space_raises() -> None:
+    with pytest.raises(iphone.IphoneError, match="coordinate space"):
+        iphone._to_points(1, 2, "inches", 3.0, (402, 874))
+
+
+def test_wda_stop_clears_cached_scale(monkeypatch: pytest.MonkeyPatch) -> None:
+    # The display scale is cached per device; wda_stop must drop it so a later
+    # wda_start against a different-scale device cannot reuse a stale value.
+    monkeypatch.setattr(iphone, "_wda_scale", 3.0)
+    monkeypatch.setattr(iphone, "_wda_session", "sid")
+    monkeypatch.setattr(iphone, "_wda_device", "udid")
+    asyncio.run(iphone.wda_stop())
+    assert iphone._wda_scale is None
+    assert iphone._wda_session is None
+    assert iphone._wda_device is None
+
+
+def test_ui_actions_require_wda(monkeypatch: pytest.MonkeyPatch) -> None:
+    # When WDA is down, UI actions must raise a one-line precondition error
+    # (naming wda_start) rather than failing deep in a urllib stack.
+    async def _down() -> bool:
+        return False
+
+    monkeypatch.setattr(iphone, "_wda_up", _down)
+    for call in (
+        iphone.tap(1, 2),
+        iphone.swipe(1, 2, 3, 4),
+        iphone.press("home"),
+        iphone.type_text("x"),
+        iphone.home(),
+    ):
+        with pytest.raises(iphone.IphoneError, match="wda_start"):
+            asyncio.run(call)
 
 
 def test_wda_down_raises_cleanly(monkeypatch: pytest.MonkeyPatch) -> None: