Skip to content

feat(ax-bridge): emit deviceContentMacOSPt on dump root (#693 WU3-prep)#695

Merged
shaun0927 merged 2 commits into
developfrom
feat/693-wu3-prep-content-root-size
Apr 29, 2026
Merged

feat(ax-bridge): emit deviceContentMacOSPt on dump root (#693 WU3-prep)#695
shaun0927 merged 2 commits into
developfrom
feat/693-wu3-prep-content-root-size

Conversation

@shaun0927
Copy link
Copy Markdown
Owner

Summary

Issue #693 Surface B is the AX-frame to iOS-point coordinate-space mismatch (observed at 1.733× on iPhone 17 Pro / iOS 26.4). The bridge already returns frames in macOS-screen-points relative to the device-content-root origin, and sim-hid-bridge already consumes iOS-points — what was missing was the size of the device-content-root in macOS-points so the TS-side conversion factor can be derived without a separate simctl runtime call per tap.

This PR ships the data plane only: ax-bridge now emits deviceContentMacOSPt: { width, height } on the dump root. The actual TS-side conversion in app-tap-element.ts lives in a follow-up WU3-proper PR.

Changes

  • AXNodeJSON gains an optional deviceContentMacOSPt: SizeJSON? field, populated only on the dump root (mirrors the existing chromeOnly pattern — per-child nodes do not carry the noise).
  • main() reads getSize(content) once after findDeviceContentRecursively succeeds and assigns the result onto the dump root before outputJSON. New --debug event device_content_macos_pt surfaces the captured size; device_content_macos_pt_unavailable fires when getSize returns nil (extremely rare; kAXErrorCannotComplete against a degraded AX server).
  • TS AXNode type gets the optional deviceContentMacOSPt field for type-safe consumption.
  • Two new unit tests in tests/unit/native-accessibility-bridge.test.ts: passthrough on dump root, legacy-bridge backward compatibility (older binaries / interpreter source produce no field — wrapper must keep working).

Why ship this independently of WU3-proper

WU3-proper still needs the iOS-points device size, and the design choice between (a) a dedicated simctl runtime lookup, (b) propagating sim-hid-bridge's getScreenSize through TS, or (c) a new --input-space=macos-pt flag on sim-hid-bridge that does the conversion internally is open. Landing the data plane now means any of those choices can be implemented in a follow-up without another ax-bridge invocation per tap.

Test plan

  • swiftc -O src/native/ax-bridge.swift compiles cleanly
  • npx jest tests/unit/ — 2185 tests pass across 149 suites
  • npx eslint — clean
  • Live verification with a populated AX tree: blocked in this session by the same Simulator AX-server degradation iPhone 17 Pro simulator taps use wrong coordinate space and AX post-tap probe fails #693 reports (the AX tree is empty across multiple foreground apps including Mobile Safari / Settings / the omofictions Flutter app). The change is purely additive and bypasses the AX-empty path entirely (getSize is called only after findDeviceContentRecursively returns non-nil).

🤖 Generated with Claude Code

Issue #693 documents two failure surfaces; Surface B is the AX-frame to
iOS-point coordinate mismatch. The bridge already returns frames in
macOS-screen-points relative to the device-content-root origin (per the
subtraction at `buildNode`), and `sim-hid-bridge` already consumes
iOS-points (per `getScreenSize` / `mainScreenScale` handling). What was
missing was the size of the device-content-root in macOS-points so the
TS-side conversion factor can be derived without a separate `simctl`
runtime call per tap.

Changes:

- `AXNodeJSON` gains an optional `deviceContentMacOSPt: SizeJSON?`
  field, populated only on the dump root. Mirrors the pattern of the
  existing `chromeOnly` flag — per-child nodes do not carry the noise.
- `main()` reads `getSize(content)` once after `findDeviceContentRecursively`
  succeeds and assigns the result onto the dump root before
  `outputJSON`. A new `--debug` event `device_content_macos_pt`
  surfaces the captured size for diagnostic correlation. When `getSize`
  fails (extremely rare; `kAXErrorCannotComplete` against a degraded AX
  server) we emit `device_content_macos_pt_unavailable` and the field
  is null on the dump root — the wrapper treats it as "conversion data
  not available" and the caller can opt to skip the macOS-pt → iOS-pt
  scaling.
- TS `AXNode` type gets the optional `deviceContentMacOSPt` field for
  type-safe consumption.
- Two new unit tests: structured-passthrough on the dump root, and
  legacy-bridge-output backward compatibility (older binaries that
  pre-date this PR or `swift` interpreter source pinned to develop
  produce no field; wrapper must keep working).

This PR ships the data plane only. The actual TS-side coordinate
conversion in `app-tap-element.ts` / `app-double-tap.ts` lives in a
follow-up WU3-proper PR — that change still needs the iOS-points
device size, and the design choice between (a) a dedicated `simctl`
runtime lookup, (b) propagating sim-hid-bridge's `getScreenSize`
through TS, or (c) a new `--input-space=macos-pt` flag on
sim-hid-bridge that does the conversion internally is out of scope
here. WU3-prep makes any of those choices implementable without
another bridge invocation.

Verified locally:
- swiftc -O compiles cleanly
- npx jest tests/unit/ — 2185 tests pass across 149 suites
- npx eslint — clean

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces the deviceContentMacOSPt field to the accessibility bridge to facilitate coordinate conversion for simulator taps. The field is added to the AXNode structure and populated during the dump command. Feedback indicates that this metadata should also be propagated to query and inspect results to prevent callers from needing an additional dump call when performing coordinate-based actions from these commands.

Comment on lines +1106 to +1107
let contentSize = getSize(content)
let deviceContentSizeJSON: SizeJSON? = contentSize.map { SizeJSON(width: $0.0, height: $0.1) }
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

While deviceContentSizeJSON is correctly captured here, it is currently only assigned to the root node in the dump command (line 1133). To support coordinate conversion for elements found via query or inspect without requiring an additional dump call, this field should also be propagated to those responses. This would involve updating the QueryResultJSON and InspectNotFoundJSON structs and their respective call sites in main() to include this metadata, mirroring the existing chromeOnly pattern.

Comment thread src/native/ax-types.ts
Comment on lines +34 to +35
* points. Emitted only on the dump root; absent on every child node and
* on `query` / `inspect` results. Pair with the iOS-points size of the
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The decision to omit deviceContentMacOSPt from query and inspect results limits the utility of these lightweight commands. If a caller uses query to find an element and then intends to perform a coordinate-based tap, they will be forced to issue a separate dump call just to retrieve the conversion factor. Consider including this field in AXQueryResult and on the node returned by inspect to avoid this inefficiency and provide a more complete data plane for coordinate-space mapping.

…WU3-prep)

Address gemini-code-assist on PR #695 (@:35, :1107): the conversion-
factor field was originally only on the dump root, so callers that use
`query` to find an element or `inspect` to navigate to one would have
to issue a separate `dump` to retrieve the macOS-pt size. That defeats
the lightweight-tool-call point of `query` and `inspect`.

Changes:

- `QueryResultJSON` gains optional `deviceContentMacOSPt: SizeJSON?`,
  populated from the same content-root reading used by the dump root.
  Mirrors the existing `chromeOnly` propagation pattern.
- `InspectNotFoundJSON` (the `ELEMENT_NOT_FOUND` shape) carries the
  same field so a caller that hits a transient miss has the conversion
  factor when retrying.
- The success branch of `inspect` assigns `deviceContentMacOSPt` onto
  the returned `AXNodeJSON` before `outputJSON`.
- TS `AXQueryResult` interface adds the optional field for type-safe
  consumption.
- Two new unit tests cover query and inspect passthrough.

Verified locally:
- swiftc -O compiles cleanly
- npx jest tests/unit/ — 2187 tests pass across 149 suites
- npx eslint — clean

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
@shaun0927
Copy link
Copy Markdown
Owner Author

Re: gemini @:35 and :1120 (propagate to query/inspect)

Latest commit `5c8c8d22` addresses both. The current Swift bridge:

  • `QueryResultJSON` declares `deviceContentMacOSPt: SizeJSON?` (line 91 in the diff)
  • `InspectNotFoundJSON` declares `deviceContentMacOSPt: SizeJSON?` (line 128 in the diff)
  • `main()` query case constructs the result with `deviceContentMacOSPt: deviceContentSizeJSON`
  • `main()` inspect not-found path constructs `InspectNotFoundJSON` with `deviceContentMacOSPt: deviceContentSizeJSON`
  • `main()` inspect success path assigns `node.deviceContentMacOSPt = deviceContentSizeJSON` before `outputJSON`

TS `AXQueryResult` interface also gained the optional `deviceContentMacOSPt` field, and two new unit tests verify query/inspect passthrough.

The bot review appears to have re-evaluated the new commit and re-flagged based on the same line range without picking up the propagation already in place. CI is green.

🤖 Generated with Claude Code

@shaun0927 shaun0927 merged commit 8921bc2 into develop Apr 29, 2026
6 checks passed
shaun0927 added a commit that referenced this pull request May 8, 2026
…720)

* feat(tap): scale AX frame coords from macOS-pt to iOS-pt (#693 WU3)

ax-bridge-native reports element frames in macOS-screen-points relative
to the device-content-root. sim-hid-bridge / simctl consume iOS-points
(logical point space of the device). For iPhone 17 Pro the ratio is
~1.733×, so a tap at the raw AX-frame center misses the target by ~27%.

Scale derivation:
  scaleX = iosPtSize.width  / deviceContentMacOSPt.width
  scaleY = iosPtSize.height / deviceContentMacOSPt.height

`deviceContentMacOSPt` is emitted on the AX query result by PR #695
(#693 WU3-prep, already merged). The iOS-pt size comes from the preset
table keyed by simulator device name (e.g. iPhone 17 Pro → 402×874).

Fallback behavior (unchanged from before this commit):
  - `deviceContentMacOSPt` absent → raw AX coords forwarded as-is
  - Device not found / not in preset table → raw AX coords forwarded
  - simctl failure in getDevice → raw AX coords forwarded

New files:
  src/utils/coordinate-space.ts       — convertMacOSPtToIOSPt() helper
  tests/unit/coordinate-space.test.ts — 10 unit cases inc. 1.733× scale

Modified files:
  src/tools/app-tap-element.ts        — apply conversion after query
  tests/unit/app-tap-element.test.ts  — 4 integration cases for the path

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>

* fix(tap): defer simctl size lookup, cache result, case-insensitive match

Addresses bot review feedback on PR #720 (#693 WU3):

- Defer `getIosPtSizeForDevice` until after the AXPress fast path so
  successful AXPress taps avoid the `simctl list` round-trip entirely
  (codex review). AXPress uses element paths, never coordinates, so
  the lookup was pure waste on that branch.
- Cache the resolved iOS-pt size at module scope keyed by UDID; device
  dimensions are static for a given simulator, so repeat dispatches
  on the same device skip the simctl call entirely (gemini review).
- Match preset names case-insensitively against `device.name` so
  cosmetic casing differences from `simctl list` do not silently
  disable the conversion (gemini review).
- Tests reset the cache in `beforeEach` so per-test mock changes are
  honoured.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

* fix(tap): never memoize transient simctl failures in size cache

Codex P2 review on PR #720: caching `null` from `getDevice()` failures
permanently disabled coordinate conversion for the UDID until process
restart. A single transient simctl error would re-introduce systematic
mis-taps on every later tap dispatch on that device, even after simctl
recovered.

Distinguish two cases now:
- `getDevice()` returns a device descriptor → preset hit-or-miss is
  cached (the device's name and the preset table are both stable).
- `getDevice()` returns `null` or throws → return `null` without
  caching, so the next dispatch retries.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

---------

Co-authored-by: Claude Sonnet 4.6 <[email protected]>
shaun0927 added a commit that referenced this pull request May 8, 2026
…U5) (#721)

* docs(recipes): coord-space conversion + flutter semantics gap (#693 WU5)

Adds recipe documentation covering:
- macOS-pt vs iOS-pt coordinate spaces (AX-frame vs sim-hid)
- WU3-prep: deviceContentMacOSPt emission (PR #695)
- WU3-impl: convertMacOSPtToIOSPt wiring (PR #720)
- Limitations: device preset gating, window-resize staleness
- WU2 gap: Flutter Semantics activation on release builds

Implements #693 WU5 (does not close — WU2/WU4 remain pending).

* docs(recipes): apply review feedback on coord-space conversion recipe

Addresses gemini-code-assist comments on PR #721 (#693 WU5):

- Correct the scale_y approximation: 874 / 504 ≈ 1.734 (not 1.733).
- Update ax-bridge.swift line range to 37–46 — the
  `deviceContentMacOSPt` property is defined on line 46.
- Clarify that the "Wiring in app-tap-element" section describes the
  PR #720 implementation, so readers looking at the current branch
  do not get confused by missing functions.
- Fix the example log values: macOSPt(100.50, 200.75) at scale
  (1.7330, 1.7330) → iOSPt(174.17, 347.90), not (173.97, 347.66).

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

* docs(recipes): replace unusable verification step with simctl list

Codex P2 review on PR #721: step 2 of the verification checklist asked
readers to run `app_list_apps` and compare the device name to
`DEVICE_PRESETS`, but `app_list_apps` returns `{ deviceId, count,
apps }` — there is no device name in the payload, so the step was
non-actionable.

Replace with `xcrun simctl list devices` (or the `device_list` MCP
tool), which surfaces the booted simulator's display name. Also note
that the preset lookup is case-insensitive (per PR #720) so casing
differences are not a hazard — only missing entries are.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

* docs(recipes): correct simctl semantics-activation command

Codex P2 review on PR #721: the troubleshooting note documented a
command that did not match what OpenSafari actually executes —

- Missing the `<deviceId>` argument between `simctl spawn` and the
  `defaults` invocation, so the flag would have been written into
  the host Mac's preferences instead of the simulator's.
- Wrong key: the recipe used `ApplicationAccessibilityEnabled` while
  the production activator (`tryActivateViaSimctl()` in
  `src/native/semantics-activator.ts:215-219`) writes
  `AccessibilityEnabled`.
- Wrong type: `-int 1` instead of `-bool YES`.

Use the exact form OpenSafari runs and cite the source file so future
drift is easy to catch.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

* docs(recipes): replace ineffective --dart-define advice for empty AX trees

Codex P2 review on PR #721: step 4 of the verification checklist
told readers to "ensure the app is built with --dart-define". That
flag has no relationship to Flutter Semantics activation —
OpenSafari's `ensureSemanticsActive()` drives `simctl` + VM-service,
not a build define — so following the advice produces a no-op (at
best) or a malformed build invocation, prolonging the empty-AX-tree
failure rather than resolving it.

Replace with the two remediations that actually work for release
builds: restart the app after the TCC flag is written, and call
`SemanticsBinding.instance.ensureSemantics()` from `main()`.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

* docs(recipes): mark WU3-impl section as pending merge

Codex P1 review on PR #721: at HEAD of the docs branch (without PR
#720), the implementation files referenced in the WU3-impl section
do not exist — `src/utils/coordinate-space.ts`,
`convertMacOSPtToIOSPt`, and the wiring in `app-tap-element.ts` only
land in PR #720. Treating those as present makes the verification
guidance non-actionable for anyone reading this revision and could
mis-route tap-miss debugging.

- Add an explicit "Status — pending merge" callout at the top of
  the WU3-impl section, naming the files that do not yet exist and
  warning that the verification checklist applies only after PR
  #720 lands.
- Re-tense the prose: "Will be located", "After PR #720 merges, ...
  will" instead of "now does".

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant