Skip to content

Commit e04e85b

Browse files
committed
fix(is): answer the review pass on #3340
- Eager-closure budget: the daemon entry gained one static edge for the new failure-door module. The gate names its own fix -- give the code a home in a module the closure already evaluates -- so the builders move into selector-read-shared.ts (the strict reads' existing shared-failure home) and the standalone module is deleted. No baseline touched, no lazy import bolted onto an error path. - Contract fields are now written AFTER caller details, so no caller spread order can clobber them; both strict rows prove they report the matched alternative as details.selector for the same capture. - The ambiguity hint no longer echoes the selector into a single-quoted find command (label="It's here" broke the outer quoting). - The text-echo collapse gains the reportage clause: every non-reporter candidate must be the accessibility element the platform reports for the reporter (isReportedAccessibilityElement). Same label + same frame with an authored (view-backed) descendant stays ambiguous -- the closest-negative fixture pins exactly that. The offset-rect fixture gains the live RN roles so the rect is the only differing fact. - The 5-candidate cap moves to kernel/errors as ELEMENT_MATCH_CANDIDATE_LIMIT beside the ElementMatchCandidateDetails type the surfaces read; all three producers (acting refusal, find refusal, read door) consume it. The shared- shape comment now names what is actually shared. - Help/docs: 'Uniqueness-based reads' replaces internal 'nominating reads'; ref guidance scoped to ref-taking commands (is rejects refs); the RN pair bullet qualified as the observed iOS accessibility shape.
1 parent 810b923 commit e04e85b

13 files changed

Lines changed: 239 additions & 104 deletions

‎packages/kernel/src/errors.ts‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,14 @@ export type NormalizedError = {
161161
details?: ErrorWireDetails;
162162
};
163163

164+
/**
165+
* How many candidate lines an `AMBIGUOUS_MATCH` producer puts on the wire.
166+
* Owned beside the detail type every surface reads (`readErrorCandidateViews`
167+
* computes the "+N more" marker from `matches - candidates.length`), so the
168+
* producers' caps cannot drift from each other or from the renderers.
169+
*/
170+
export const ELEMENT_MATCH_CANDIDATE_LIMIT = 5;
171+
164172
export type ElementMatchCandidateDetails = {
165173
candidates: string[];
166174
matches: number;

‎packages/selectors/src/interaction-targeting.fixtures.ts‎

Lines changed: 50 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -365,14 +365,17 @@ export const RN_TEXT_ECHO_DISTINCT_SUBTREE_NODES: RawSnapshotNode[] = [
365365
/**
366366
* The other closest negative: one ancestry chain whose descendant repeats the
367367
* ancestor's label at a DIFFERENT rect — two runs of the same words, which is two
368-
* elements the caller still has to choose between.
368+
* elements the caller still has to choose between. Roles mirror the live RN pair
369+
* so the rect is the ONLY fact that differs from the collapsing positive.
369370
*/
370371
export const RN_TEXT_ECHO_OFFSET_RECT_NODES: RawSnapshotNode[] = [
371372
{
372373
index: 0,
373374
depth: 1,
374375
parentIndex: 2,
375376
type: 'XCUIElementTypeStaticText',
377+
role: 'RCTParagraphComponentView',
378+
subrole: 'UIView',
376379
label: 'Catalog scroll: top',
377380
rect: { x: 18, y: 168, width: 350, height: 17 },
378381
enabled: true,
@@ -383,6 +386,8 @@ export const RN_TEXT_ECHO_OFFSET_RECT_NODES: RawSnapshotNode[] = [
383386
depth: 2,
384387
parentIndex: 0,
385388
type: 'XCUIElementTypeStaticText',
389+
role: 'RCTAccessibilityElement',
390+
subrole: 'UIAccessibilityElement',
386391
label: 'Catalog scroll: top',
387392
rect: { x: 18, y: 420, width: 350, height: 17 },
388393
enabled: true,
@@ -397,3 +402,47 @@ export const RN_TEXT_ECHO_OFFSET_RECT_NODES: RawSnapshotNode[] = [
397402
hittable: true,
398403
},
399404
];
405+
406+
/**
407+
* The reportage negative: one ancestry chain with the identical label at the
408+
* identical rect — the shape geometry cannot distinguish — where the descendant
409+
* is an AUTHORED element (a nested `<Text>` or a `<View>` carrying the same
410+
* accessibilityLabel, view-backed role/subrole), not the accessibility element
411+
* the platform reports for the reporter. Same frame, same label, two authored
412+
* elements: the collapse rule's `isReportedAccessibilityElement` clause keeps
413+
* this ambiguous.
414+
*/
415+
export const RN_TEXT_ECHO_AUTHORED_CHILD_NODES: RawSnapshotNode[] = [
416+
{
417+
index: 0,
418+
depth: 1,
419+
parentIndex: 2,
420+
type: 'XCUIElementTypeStaticText',
421+
role: 'RCTParagraphComponentView',
422+
subrole: 'UIView',
423+
label: 'Catalog scroll: top',
424+
rect: { x: 18, y: 168, width: 350, height: 17 },
425+
enabled: true,
426+
hittable: true,
427+
},
428+
{
429+
index: 1,
430+
depth: 2,
431+
parentIndex: 0,
432+
type: 'RCTParagraphComponentView',
433+
role: 'RCTParagraphComponentView',
434+
subrole: 'UIView',
435+
label: 'Catalog scroll: top',
436+
rect: { x: 18, y: 168, width: 350, height: 17 },
437+
enabled: true,
438+
hittable: true,
439+
},
440+
{
441+
index: 2,
442+
depth: 0,
443+
type: 'XCUIElementTypeApplication',
444+
rect: { x: 0, y: 0, width: 386, height: 678 },
445+
enabled: true,
446+
hittable: true,
447+
},
448+
];

‎packages/selectors/src/interaction-targeting.ts‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,23 @@ function resolveUnverifiedWrapperControlWithIndex(
145145
: null;
146146
}
147147

148+
/**
149+
* The mirror half of the RN pair is not a view the app authored: it is the
150+
* synthetic element the platform reports for the reporter's accessibility
151+
* subtree, and it carries that reportage in its role/subrole
152+
* (`RCTAccessibilityElement` / `UIAccessibilityElement`, measured live via
153+
* `snapshot --raw`). Requiring it on every non-reporter candidate is what
154+
* distinguishes "one element the platform reported twice" from "two authored
155+
* elements that happen to share a label and a frame" — geometry cannot tell
156+
* those apart, only the reportage can. An authored child (a nested `<Text>`
157+
* styled to the same label at the same place, a `<View>` carrying the same
158+
* accessibilityLabel) has view-backed role/subrole and keeps the refusal.
159+
*/
160+
function isReportedAccessibilityElement(node: SnapshotNode): boolean {
161+
const roles = [node.type, node.role, node.subrole].map((value) => normalizeType(value ?? ''));
162+
return roles.some((role) => role.includes('accessibilityelement'));
163+
}
164+
148165
/**
149166
* The React Native text shape, captured live from the fixture Catalog screen: a
150167
* `RCTParagraphComponentView` reporting the accessibility label (and the app's
@@ -166,6 +183,10 @@ function resolveUnverifiedWrapperControlWithIndex(
166183
* - identical non-empty labels and rects agreeing within wrapper slack — a nested
167184
* `<Text>` that repeats a word at its own position is a second run of text, not
168185
* a mirror, and a distinct rect proves it;
186+
* - every non-reporter candidate is a reported accessibility element (above) —
187+
* same label AND same frame is exactly the case where geometry cannot
188+
* distinguish a mirror from a second authored element, so the reportage is
189+
* required and an authored same-frame child stays ambiguous;
169190
* - no candidate is a semantic touch target — a button labelled like its own static
170191
* text is two roles the caller still has to choose between (that shape is the
171192
* wrapper rule above's job, through the hittability door it keeps);
@@ -189,6 +210,7 @@ function resolveTextEchoReporterWithIndex(
189210
if (!reporterRect) return null;
190211
const mirrorsOneReporter = candidates.every(
191212
(candidate) =>
213+
(candidate === reporter || isReportedAccessibilityElement(candidate)) &&
192214
candidate.label?.trim() === reporterLabel &&
193215
agreesWithinWrapperSlack(normalizeRect(candidate.rect), reporterRect),
194216
);

‎packages/selectors/src/selector-pipeline.test.ts‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import { SELECTOR_RESOLUTION_POLICIES } from '@agent-device/selectors';
55
import { makeSnapshotState } from './snapshot-geometry.fixtures.ts';
66
import {
77
ELEMENT14_DISTINCT_SUBTREE_NODES,
8+
RN_TEXT_ECHO_AUTHORED_CHILD_NODES,
89
RN_TEXT_ECHO_DISTINCT_SUBTREE_NODES,
910
RN_TEXT_ECHO_NODES,
1011
RN_TEXT_ECHO_OFFSET_RECT_NODES,
@@ -355,8 +356,8 @@ test('the uniqueness rows collapse React Native text reported twice', async () =
355356

356357
/**
357358
* Each negative changes exactly ONE structural fact of the pair above while every
358-
* label and rect still matches, so a rule reading the description rather than the
359-
* structure would answer and these tests would notice.
359+
* label and rect still matches (or the ancestry breaks), so a rule reading the
360+
* description rather than the structure would answer and these tests would notice.
360361
*/
361362
test('the uniqueness rows refuse a text pair that is not one reporter and its mirror', async () => {
362363
const cases = [
@@ -370,6 +371,14 @@ test('the uniqueness rows refuse a text pair that is not one reporter and its mi
370371
nodesOf(RN_TEXT_ECHO_OFFSET_RECT_NODES),
371372
RN_TEXT_ECHO_SELECTOR,
372373
],
374+
[
375+
// Same label AND same frame — the case geometry cannot decide. The
376+
// descendant is authored (view-backed roles), not the platform's
377+
// reported accessibility element, so the pair stays two elements.
378+
'same label at the same frame with an authored descendant',
379+
nodesOf(RN_TEXT_ECHO_AUTHORED_CHILD_NODES),
380+
RN_TEXT_ECHO_SELECTOR,
381+
],
373382
] as const;
374383
for (const [name, nodes, selector] of cases) {
375384
for (const row of ['readUnique', 'cropTarget'] as const) {

‎src/commands/interaction/runtime/__tests__/selector-read-policy.test.ts‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,39 @@ test('is reports the ambiguity it refuses as an ambiguity, not an absence (readU
8282
assert.deepEqual(details?.candidates, ['@e2 [button] "Save"', '@e3 [button] "Save"']);
8383
});
8484

85+
/**
86+
* Both rows share one door, so `details.selector` must name the MATCHED
87+
* alternative for both, never the caller's authored expression. `is` passes
88+
* its authored selector among its details; a spread order that let caller
89+
* details win would make `is` report the whole expression while `get attrs`
90+
* reported the alternative — same door, two shapes (#2870 review).
91+
*/
92+
test('both strict rows report the matched alternative as details.selector, not the authored expression', async () => {
93+
const AUTHORED = 'label="Nowhere" || label="Save"';
94+
const device = createSelectorDevice(ambiguousSelectorReadSnapshot());
95+
96+
const errors = await Promise.all([
97+
device.selectors.is({ session: 'default', predicate: 'visible', selector: AUTHORED }).then(
98+
() => null,
99+
(error: unknown) => error as AppError,
100+
),
101+
device.selectors.getAttrs(selector(AUTHORED), { session: 'default' }).then(
102+
() => null,
103+
(error: unknown) => error as AppError,
104+
),
105+
]);
106+
107+
for (const error of errors) {
108+
assert.ok(error instanceof AppError);
109+
assert.equal(error.code, 'AMBIGUOUS_MATCH');
110+
assert.equal(
111+
(error.details as { selector?: string } | undefined)?.selector,
112+
'label="Save"',
113+
'details.selector names the alternative that matched twice',
114+
);
115+
}
116+
});
117+
85118
/**
86119
* The closest negative to the pair above: a selector that matches NOTHING still
87120
* reports proof of absence. The two failures carry different typed reasons and

‎src/commands/interaction/runtime/selector-action-resolution.ts‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { AppError } from '@agent-device/kernel/errors';
1+
import { AppError, ELEMENT_MATCH_CANDIDATE_LIMIT } from '@agent-device/kernel/errors';
22
import type { Platform, PublicPlatform } from '@agent-device/kernel/device';
33
import type { SnapshotNode } from '@agent-device/kernel/snapshot';
44
import type { SelectorResolution } from '@agent-device/selectors';
@@ -7,8 +7,6 @@ import { listSelectorPipelineMatches } from '@agent-device/selectors/selector-pi
77
import type { ActingPipelinePolicy } from '@agent-device/selectors/selector-pipeline-policy';
88
import { formatSnapshotLine } from '@agent-device/capture-kit/snapshot-lines';
99

10-
const AMBIGUOUS_ACTION_CANDIDATE_LIMIT = 5;
11-
1210
/**
1311
* How an acting row narrows its candidate set: wrapper duplicates may collapse
1412
* through structural/actionable equivalence; matches in distinct branches are
@@ -51,7 +49,7 @@ export function resolveActionSelector(
5149
selector: list.selector,
5250
matches: classification.candidates.length,
5351
candidates: classification.candidates
54-
.slice(0, AMBIGUOUS_ACTION_CANDIDATE_LIMIT)
52+
.slice(0, ELEMENT_MATCH_CANDIDATE_LIMIT)
5553
.map((candidate) => formatSnapshotLine(candidate, 0, false)),
5654
},
5755
);

‎src/commands/interaction/runtime/selector-is.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import {
2222
type CapturedSnapshot,
2323
type SelectorSnapshotOptions,
2424
captureSelectorSnapshot,
25+
observationReadFailure,
2526
} from './selector-read-shared.ts';
2627
import { deriveSelectorCapturePolicy } from './selector-capture-policy.ts';
2728
import { absenceCaptureOptionRefusal } from '@agent-device/selectors/absence-observation';
@@ -30,7 +31,6 @@ import {
3031
absenceUnreadableError,
3132
} from '@agent-device/selectors/absence-observation-errors';
3233
import { resolveAbsenceObservation } from '@agent-device/selectors/absence-observation-resolution';
33-
import { observationReadFailure } from './selector-observation-failure.ts';
3434

3535
export type IsCommandOptions = CommandContext &
3636
SelectorSnapshotOptions & {

‎src/commands/interaction/runtime/selector-observation-failure.ts‎

Lines changed: 0 additions & 81 deletions
This file was deleted.

0 commit comments

Comments
 (0)