fix(selectors): collapse an unverified-hittability wrapper chain to its control - #2482
Conversation
…ts control A SwiftUI toolbar wrapper and its control share one identifier, and regular iOS snapshots omit hittability evidence. findPreferredActionableDescendant requires verified hittability, and the wrapper's rect differs by under a point per edge, so press/fill saw two distinct actionable elements for one control and refused with AMBIGUOUS_MATCH. Resolve the deepest semantic touch target when every candidate lacks hittability evidence and all rects agree within sub-pixel slack. Candidates carrying any hittability fact keep the existing rules.
|
The new fallback also accepts two real nested controls. With a Cell and child Button sharing a selector, no hittability fields, and rectangles within 1 pt, the old rules keep their distinct semantic targets ambiguous; resolveUnverifiedWrapperControl now picks the child because it checks only that the deepest candidate is semantic. That can silently press the wrong control. Restrict this fallback to non-actionable wrappers around one control and add a negative regression for a semantic ancestor, alongside the existing Other/Button success case. |
…wrappers Review follow-up on callstack#2482. The unverified-hittability collapse accepted any ancestry chain whose rects agreed within a point, so a cell and the button inside it (both actionable, no hittability evidence) collapsed to the descendant: a silent wrong-control press where the previous rules refused as ambiguous. The fallback now requires every candidate above the control to be a non-actionable wrapper, and a negative regression covers the semantic-ancestor case next to the captured Other/Button success case. Gate: pnpm check:affected --run - 304 files / 2011 tests, all runnable checks passed.
|
Addressed in The fallback now requires every candidate above the control to be a non-actionable wrapper ( New regression: Gate: |
|
Red evidence for that regression, on the previous commit: with the |
|
The nested-control guard at 8f1e50b addresses the earlier finding, and the negative regression targets it correctly. One scope claim still needs correction: this helper is called by action-target resolution, not selector wait, so the reported wait improvement cannot be attributed to this change. Please narrow the claim to press and leave the native wait case explicitly open, or add the missing wait-path change and evidence. GitHub currently reports no checks for this head. |
|
Corrected - the scope claim is narrowed, and the earlier attribution is withdrawn.
Press-path device evidence is unchanged and reproducible on the same screen state: 0.21.0 refuses with |
thymikee
added
the
ready-for-human
|
Thanks! Please fix coverage |
|
The Coverage failure at 8f1e50b is related to this change: the new static import of interaction-targeting-wrapper-chain adds one eagerly evaluated module to three selector entry points. Restore the import budget at the shared owning module and rerun the gate; this is not a coverage-percentage failure. The behavior review and press evidence remain clean, so ready-for-human stays applied, but this gate still blocks merge. |
The extracted module grew the eager closure of three budgeted entries by one module each -- interaction-targeting.ts 13 -> 14, selector-pipeline.ts 25 -> 26, absence-observation-resolution.ts likewise -- and the eager-closure gate ratchets that closure against the merge-base: an entry surface that drags more of the repo onto the import path is a loading-shape regression whatever the reason. The rule has exactly one consumer, so it now lives beside the classification that asks it and is no longer an exported surface. Its tests move to the owning module's test file and exercise `classifyActionableTouchCandidates`, the boundary the command actually calls. Each of the four refusals fails when its own guard is mutated: the hittability condition, the 1 pt slack, the non-actionable-wrapper condition, and the semantic-control condition.
|
Pushed So the rule now lives in
One housekeeping item: the "Tests" bullet still names |
|
The move at 6839c9e preserves the helper and its guards while removing the extra eager import. Coverage now passes, and the moved tests exercise the production classifier. No code findings; ready for human review, with the earlier press evidence still applicable. Please correct the Tests section to name interaction-targeting.test.ts; the standalone wrapper-chain test file was removed. The iOS smoke job has since failed with RUNNER_BUSY at the WebView page wait, matching the main-lane signature and appearing unrelated to this helper move. |
A control reported through its own accessibility wrapper answers a selector twice, and a regular iOS snapshot omits unverified hittability, so the ladder that relates a wrapper to its control cannot fire. #2482 collapsed that chain for mutating resolution only: `press` tapped the toolbar button while `is visible` and `get attrs` reported "Selector did not match" and `screenshot --crop-on` refused the same screen as two nodes. Export the collapse beside the classification that asks for it and apply it where a read row's answer was a refusal. Rows that resolve before any refusal are untouched, and a candidate set the rule does not recognize as one control - a cell and the button inside it, or matches in distinct subtrees - still refuses. Replay verifies a recorded target by resolving its recorded selector again under the same row's refusal rules, so a step whose screen had not changed verified as an identity mismatch on its first replay. Verification names the collapsed control too, which is the node dispatch acted on and the node the recorded identity carries.
#2501) A control reported through its own accessibility wrapper answers a selector twice, and a regular iOS snapshot omits unverified hittability, so the ladder that relates a wrapper to its control cannot fire. #2482 collapsed that chain for mutating resolution only: `press` tapped the toolbar button while `is visible` and `get attrs` reported "Selector did not match" and `screenshot --crop-on` refused the same screen as two nodes. Export the collapse beside the classification that asks for it and apply it where a read row's answer was a refusal. Rows that resolve before any refusal are untouched, and a candidate set the rule does not recognize as one control - a cell and the button inside it, or matches in distinct subtrees - still refuses. Replay verifies a recorded target by resolving its recorded selector again under the same row's refusal rules, so a step whose screen had not changed verified as an identity mismatch on its first replay. Verification names the collapsed control too, which is the node dispatch acted on and the node the recorded identity carries.
2 participants
Collapse an unverified-hittability wrapper chain to its control
Fixes #2480
What changes
A SwiftUI toolbar wrapper (
Other) and its control (Button) share one accessibility identifier. Regular iOS snapshots omit hittability evidence, and the two rects differ by under a point per edge, sopress 'id="scoring_home_button"'fails with:findPreferredActionableDescendantrequires verified hittability, which regular iOS snapshots never provide, so the documented wrapper collapse cannot fire for the most common SwiftUI toolbar shape.New module
packages/selectors/src/interaction-targeting-wrapper-chain.tsresolves the deepest semantic touch target when every candidate lacks hittability evidence, all rects agree within 1 pt of slack, and every candidate above the control is a non-actionable wrapper. A candidate carrying any hittability fact keeps the existing rules; a chain of two real controls (a cell and the button inside it) keeps the ambiguity refusal.Scope of this change
classifyActionableTouchCandidatesis reached only from the action-target resolution (src/commands/interaction/runtime/selector-action-resolution.ts, called byresolution.ts). This change therefore coverspress/click/fill/longpress, theistarget verification, and the post-action settle observation.waitdoes not run through that path:src/daemon/wait-runtime.tsbuilds its own selector runtime, and its multi-match refusal is produced there. Thewaitcase is not fixed by this change and stays open (see "Runner side" below). An earlier version of this description attributed awaitimprovement to this change; that attribution was wrong, and the device-lane rows below are press-only.Tests
interaction-targeting-wrapper-chain.test.ts(new): the collapse, the slack boundary, the "a hittability fact still wins" case, andrefuses a chain of two real controls that share one rect(a semantic ancestor with a child button, no hittability, rects 0.5 pt apart).interaction-targeting.test.ts,selector-action-resolution.test.ts: the pair resolves to the button.Selector matched 2 distinct actionable elements: id="scoring_home_button". Removing only the nested-control guard fails the new negative case while the captured Other/Button success case still passes (1 failed | 4 passed).Device lane (iPhone 15 simulator, iOS 26.5, Xcode 26.6; 0.21.0 as the control)
Same screen state in both runs:
scoring_home_button->Other [20,63,36,36]+Button [21,63,35,36].press 'id="scoring_home_button"' --settleAMBIGUOUS_MATCHTapped id="scoring_home_button" (38, 81)wait 'id="new_game_button"' 8000(consequence: the press left the scoring screen)Gates
pnpm check:affected --run: 304 files / 2011 tests, all runnable checks passed.Runner side (not in this PR)
The
waitrefusal is produced by the selector runtime thatwaitbuilds for itself, and on iOS that is the native XCTest runner (apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+CommandExecution.swift:1683andRunnerTests+Interaction.swift:238), which does not use the shared structural-equivalence rules. I can prepare that patch; it needs a simulator lane run. Which shape do you prefer - delegate to the shared rules, or mirror them?