Skip to content

Commit 352ad62

Browse files
thymikeethymikee
authored
feat(interaction): name the covering element in covered refusals (#3495)
* feat(interaction): name the covering element in covered refusals The occlusion scan already finds which node draws on top of a covered target but published only the boolean, so a target_covered refusal gave the agent no hint about what to dismiss. Carry a bounded, non-actionable description of the cover (presented role, clipped label, rect) on the annotated node as `coveredBy`, surface it as a typed detail and in the hint of both refusal producers, and compare it in snapshot unchanged-metadata. Backend-policy covers (Android replacement surfaces) report only the boolean and refuse without a guessed culprit. Closes #3488 * fix(kernel): keep the covering-element hint one line for app-controlled labels Collapse whitespace and drop quote characters when rendering a covering node's label into a refusal hint, so an app-set label containing a newline or quote cannot break the hint's one-line shape; the machine detail keeps the label raw. * fix(snapshot): name the covering element only when it is nameable Live evidence from an iPhone 18 Pro (iOS 27.0) running Agent Device Tester with its confirmation alert open: 14 of 38 interactive nodes publish `interactionBlocked: "covered"` and `coveredBy: {kind: "group", label: "Confirm catalog refresh", rect: (41,327 320x248)}`, and `click @ref`, `press` and `find ... focus` all refuse with `Covered by the group "Confirm catalog refresh" at (41, 327 320x248). Dismiss or close that element, ...`. The covering node is the UIAlertController's container, which the tree publishes as the generic `group` kind: the alert's title carried as its accessible label is the only thing that makes it recognizable. Strip that label and the same node would have refused with "Covered by the group at (41, 327 320x248)" — a faceless box nobody would dismiss. So an anonymous `element`/`group`/`other` cover now publishes no cover at all and the refusal keeps its generic recovery, matching what a backend-policy cover already does. A named kind (`dialog`, `sheet`, `tab-bar`) still describes itself without a label. Also single-sources the covered-refusal detail (`coveredInteractionDetails`) so `find ... focus` reports the same typed `reason: target_covered` as press/click/fill instead of omitting it, and documents `coveredBy` next to `interactionBlocked` in the snapshot docs. * docs(snapshot): publish coveredBy only when the cover is nameable Cubic is right that the Android bullet promised the field on every drawing-order cover: the occlusion pass deliberately annotates an anonymous element/group/other cover without it, so the bullet over-claimed. Both places now state the field appears when the cover can be named, and the Android bullet links the field contract instead of restating it. * refactor(selectors): keep the covered hint inside the refusal-detail module CI's eager-closure gate caught the consolidation for real: `@agent-device/selectors/interaction-error` is a vocabulary entry whose import closure is its own module alone, and importing the hint helper out of `kernel/snapshot` made every importer of the reason vocabulary evaluate the whole snapshot presentation eagerly (2 modules where the merge-base had 1, +74 top-level statements). A covered refusal is that rendering's only consumer, so the hint moves here instead of the edge being deferred behind a dynamic import, and `kernel/snapshot` keeps only the published data shape and its label bound. The helper was introduced by this PR and is unreleased (`git tag --contains` finds nothing), so the superseded kernel export is removed rather than kept beside the new home. The hint's four cases move with it into `interaction-error.test.ts`, matching source topology.
1 parent 171ff77 commit 352ad62

14 files changed

Lines changed: 580 additions & 25 deletions

File tree

‎packages/capture-kit/src/snapshot-occlusion.test.ts‎

Lines changed: 145 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,11 @@
11
import { test } from 'vitest';
22
import assert from 'node:assert/strict';
33
import type { RawSnapshotNode } from '@agent-device/kernel/snapshot';
4-
import { annotateCoveredSnapshotNodes } from './snapshot-occlusion.ts';
4+
import { SNAPSHOT_COVERING_LABEL_MAX_LENGTH } from '@agent-device/kernel/snapshot';
5+
import {
6+
annotateCoveredSnapshotNodes,
7+
annotateSnapshotNodesCoveredByPolicy,
8+
} from './snapshot-occlusion.ts';
59

610
// #1478 P5 codec-extraction regression: `annotateCoveredSnapshotNodes` marks
711
// a touch candidate `interactionBlocked: 'covered'` when a later, floating
@@ -93,6 +97,146 @@ test('annotateCoveredSnapshotNodes still marks a touch target covered by a later
9397
assert.equal(result[0]?.hittable, false);
9498
});
9599

100+
test('annotateCoveredSnapshotNodes carries a bounded description of the covering node', () => {
101+
const target: RawSnapshotNode = {
102+
index: 0,
103+
type: 'button',
104+
role: 'button',
105+
hittable: true,
106+
label: 'Save',
107+
rect: { x: 10, y: 10, width: 100, height: 40 },
108+
};
109+
const overlay: RawSnapshotNode = {
110+
index: 1,
111+
type: 'XCUIElementTypeDialog',
112+
label: ' Cookie banner ',
113+
rect: { x: 0, y: 0, width: 200, height: 200 },
114+
};
115+
116+
const result = annotateCoveredSnapshotNodes([target, overlay]);
117+
118+
// The description is the presented role + a trimmed label + the cover's rect, and it carries no
119+
// ref: naming the culprit is diagnosis, never an actionable re-target.
120+
assert.deepEqual(result[0]?.coveredBy, {
121+
kind: 'dialog',
122+
label: 'Cookie banner',
123+
rect: { x: 0, y: 0, width: 200, height: 200 },
124+
});
125+
});
126+
127+
test('annotateCoveredSnapshotNodes clips a covering label to the bounded length', () => {
128+
const target: RawSnapshotNode = {
129+
index: 0,
130+
type: 'button',
131+
role: 'button',
132+
hittable: true,
133+
label: 'Save',
134+
rect: { x: 10, y: 10, width: 100, height: 40 },
135+
};
136+
const overlay: RawSnapshotNode = {
137+
index: 1,
138+
type: 'dialog',
139+
label: 'x'.repeat(SNAPSHOT_COVERING_LABEL_MAX_LENGTH + 40),
140+
rect: { x: 0, y: 0, width: 200, height: 200 },
141+
};
142+
143+
const result = annotateCoveredSnapshotNodes([target, overlay]);
144+
145+
assert.equal(result[0]?.coveredBy?.label?.length, SNAPSHOT_COVERING_LABEL_MAX_LENGTH);
146+
});
147+
148+
// Shapes below are the live iPhone 18 Pro (iOS 27.0) capture of the Agent Device Tester home tab
149+
// with its confirmation alert open. The alert's container publishes as the generic `group` kind and
150+
// is only recognizable through the alert title carried as its accessible label.
151+
const LIVE_HOME_APPLICATION: RawSnapshotNode = {
152+
index: 0,
153+
type: 'Application',
154+
role: 'UIApplication',
155+
label: 'Agent Device Tester',
156+
rect: { x: 0, y: 0, width: 402, height: 874 },
157+
};
158+
159+
const LIVE_COVERED_SWITCH: RawSnapshotNode = {
160+
index: 1,
161+
type: 'Switch',
162+
role: 'UISwitch',
163+
label: 'Lab online',
164+
value: '1',
165+
hittable: true,
166+
parentIndex: 0,
167+
rect: { x: 304.6666774749756, y: 322.66666634877515, width: 63, height: 28 },
168+
};
169+
170+
const LIVE_ALERT_CONTAINER: RawSnapshotNode = {
171+
index: 2,
172+
type: 'Group',
173+
role: '_UIAlertControllerPhoneTVMacView',
174+
label: 'Confirm catalog refresh',
175+
hittable: true,
176+
parentIndex: 0,
177+
rect: { x: 41, y: 327, width: 320, height: 248 },
178+
};
179+
180+
test('a live alert container is named through the label it publishes, not its generic kind', () => {
181+
const result = annotateCoveredSnapshotNodes([
182+
LIVE_HOME_APPLICATION,
183+
LIVE_COVERED_SWITCH,
184+
LIVE_ALERT_CONTAINER,
185+
]);
186+
187+
assert.equal(result[1]?.interactionBlocked, 'covered');
188+
assert.deepEqual(result[1]?.coveredBy, {
189+
kind: 'group',
190+
label: 'Confirm catalog refresh',
191+
rect: { x: 41, y: 327, width: 320, height: 248 },
192+
});
193+
});
194+
195+
test('an anonymous container cover publishes no cover instead of naming a faceless box', () => {
196+
// Same live nodes with the alert title removed: `the group at (41, 327 320x248)` describes a box
197+
// no reader would recognize or dismiss, so the refusal falls back to the generic recovery.
198+
const result = annotateCoveredSnapshotNodes([
199+
LIVE_HOME_APPLICATION,
200+
LIVE_COVERED_SWITCH,
201+
{ ...LIVE_ALERT_CONTAINER, label: undefined },
202+
]);
203+
204+
assert.equal(result[1]?.interactionBlocked, 'covered');
205+
assert.equal(result[1]?.hittable, false);
206+
assert.equal(result[1]?.coveredBy, undefined);
207+
});
208+
209+
test('a named cover kind still describes itself with no label to lean on', () => {
210+
const result = annotateCoveredSnapshotNodes([
211+
LIVE_HOME_APPLICATION,
212+
LIVE_COVERED_SWITCH,
213+
{ ...LIVE_ALERT_CONTAINER, type: 'Sheet', role: undefined, label: undefined },
214+
]);
215+
216+
assert.deepEqual(result[1]?.coveredBy, {
217+
kind: 'sheet',
218+
rect: { x: 41, y: 327, width: 320, height: 248 },
219+
});
220+
});
221+
222+
test('annotateSnapshotNodesCoveredByPolicy refuses without guessing a cover', () => {
223+
// The Android replacement-surface lane reports only the covered boolean; the refusal must not
224+
// invent a culprit it never observed.
225+
const target: RawSnapshotNode = {
226+
index: 0,
227+
type: 'button',
228+
role: 'button',
229+
hittable: true,
230+
label: 'Save',
231+
rect: { x: 10, y: 10, width: 100, height: 40 },
232+
};
233+
234+
const result = annotateSnapshotNodesCoveredByPolicy([target], (node) => node.index === 0);
235+
236+
assert.equal(result[0]?.interactionBlocked, 'covered');
237+
assert.equal(result[0]?.coveredBy, undefined);
238+
});
239+
96240
test('annotateCoveredSnapshotNodes leaves an uncovered touch target unchanged', () => {
97241
const target: RawSnapshotNode = {
98242
index: 0,

‎packages/capture-kit/src/snapshot-occlusion.ts‎

Lines changed: 43 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,9 @@
1-
import type { RawSnapshotNode, Rect } from '@agent-device/kernel/snapshot';
2-
import { centerOfRect } from '@agent-device/kernel/snapshot';
1+
import type { RawSnapshotNode, Rect, SnapshotCoveringNode } from '@agent-device/kernel/snapshot';
2+
import {
3+
centerOfRect,
4+
formatRole,
5+
SNAPSHOT_COVERING_LABEL_MAX_LENGTH,
6+
} from '@agent-device/kernel/snapshot';
37
import { areRectsApproximatelyEqual, normalizeRect } from '@agent-device/kernel/rect-center';
48
import { containsPoint, rectContains } from '@agent-device/kernel/rect';
59
import {
@@ -113,38 +117,69 @@ export function annotateCoveredSnapshotNodes(
113117
),
114118
coverCache: new Map(),
115119
};
116-
const coveredPositions: number[] = [];
120+
const coveredPositions: Array<{ position: number; coveredBy?: SnapshotCoveringNode }> = [];
117121
for (const [position, node] of nodes.entries()) {
118122
if (!isCandidateTouchNode(node)) continue;
119-
if (findCoveringNode(scan, position, node, options)) coveredPositions.push(position);
123+
const coveringNode = findCoveringNode(scan, position, node, options);
124+
if (!coveringNode) continue;
125+
const coveredBy = describeCoveringNode(coveringNode);
126+
coveredPositions.push(coveredBy ? { position, coveredBy } : { position });
120127
}
121128
return annotateCoveredPositions(nodes, coveredPositions);
122129
}
123130

131+
/** Presented roles that name a box rather than a surface a reader would recognize and dismiss. */
132+
const ANONYMOUS_CONTAINER_KINDS = new Set(['element', 'group', 'other']);
133+
134+
/**
135+
* The bounded, non-actionable description of the cover the scan found, or nothing when the cover has
136+
* no name. No ref is ever carried — see {@link SnapshotCoveringNode}.
137+
*
138+
* A live iPhone 18 Pro capture pins the boundary: a `UIAlertController`'s container publishes as the
139+
* generic `group` kind and is nameable only through the alert's title carried as its accessible
140+
* label, so the refusal reads `the group "Confirm catalog refresh" at (41, 327 320x248)` and names
141+
* the surface to dismiss. Strip that label and the same node would say `the group at (...)`, which
142+
* points at a box nobody would recognize; an anonymous container of that shape therefore publishes
143+
* no cover, and the refusal keeps its generic recovery. A named kind (`dialog`, `tab-bar`, `sheet`)
144+
* still describes itself with no label to lean on.
145+
*/
146+
function describeCoveringNode(node: RawSnapshotNode): SnapshotCoveringNode | null {
147+
const label = node.label?.trim();
148+
const kind = formatRole(node.type ?? 'Element');
149+
if (!label && ANONYMOUS_CONTAINER_KINDS.has(kind)) return null;
150+
const rect = positiveRect(node.rect);
151+
return {
152+
kind,
153+
...(label ? { label: label.slice(0, SNAPSHOT_COVERING_LABEL_MAX_LENGTH) } : {}),
154+
...(rect ? { rect } : {}),
155+
};
156+
}
157+
124158
/** Applies backend-owned covered decisions through the same publication annotation contract. */
125159
export function annotateSnapshotNodesCoveredByPolicy(
126160
nodes: RawSnapshotNode[],
127161
isCovered: (node: RawSnapshotNode) => boolean,
128162
): RawSnapshotNode[] {
129-
const coveredPositions: number[] = [];
163+
const coveredPositions: Array<{ position: number; coveredBy?: undefined }> = [];
130164
for (const [position, node] of nodes.entries()) {
131-
if (isCandidateTouchNode(node) && isCovered(node)) coveredPositions.push(position);
165+
if (isCandidateTouchNode(node) && isCovered(node)) coveredPositions.push({ position });
132166
}
133167
return annotateCoveredPositions(nodes, coveredPositions);
134168
}
135169

136170
function annotateCoveredPositions(
137171
nodes: RawSnapshotNode[],
138-
coveredPositions: readonly number[],
172+
coveredPositions: ReadonlyArray<{ position: number; coveredBy?: SnapshotCoveringNode }>,
139173
): RawSnapshotNode[] {
140174
if (coveredPositions.length === 0) return nodes;
141175
const annotated = [...nodes];
142-
for (const position of coveredPositions) {
176+
for (const { position, coveredBy } of coveredPositions) {
143177
const node = nodes[position]!;
144178
annotated[position] = {
145179
...node,
146180
hittable: false,
147181
interactionBlocked: 'covered' as const,
182+
...(coveredBy ? { coveredBy } : {}),
148183
presentationHints: mergeCoveredHint(node.presentationHints),
149184
};
150185
}

‎packages/kernel/src/snapshot.ts‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -246,6 +246,24 @@ export type SnapshotPresentationFlagInput = SnapshotOptionFlagFields<
246246
(typeof SNAPSHOT_PRESENTATION_OPTION_KEYS)[number]
247247
>;
248248

249+
/**
250+
* The bounded, non-actionable description of the element that occludes a covered node. It carries
251+
* only what the occlusion scan observed — presented role, an optional clipped accessible name, and
252+
* the cover's rect — and deliberately no ref: the cover may be a container the snapshot never
253+
* issues refs for, and naming it is diagnosis, not an invitation to target it.
254+
*/
255+
export type SnapshotCoveringNode = {
256+
/** The cover's presented role, from the same table the text presenter reads (`formatRole`). */
257+
kind: string;
258+
/** The cover's accessible name, trimmed and clipped to {@link SNAPSHOT_COVERING_LABEL_MAX_LENGTH}. */
259+
label?: string;
260+
/** The cover's positive rect as the scan measured it. */
261+
rect?: Rect;
262+
};
263+
264+
/** Upper bound for a covering-node label carried in diagnostics; longer names are clipped. */
265+
export const SNAPSHOT_COVERING_LABEL_MAX_LENGTH = 80;
266+
249267
export type RawSnapshotNode = {
250268
index: number;
251269
type?: string;
@@ -297,6 +315,16 @@ export type RawSnapshotNode = {
297315
hiddenContentAbove?: boolean;
298316
hiddenContentBelow?: boolean;
299317
interactionBlocked?: 'covered';
318+
/**
319+
* Bounded description of the element the geometric occlusion scan found drawing on top of this
320+
* node, published together with `interactionBlocked: 'covered'` so a refusal can name the culprit
321+
* instead of only the fact of coverage. Absent when the covered decision came from a
322+
* backend-policy annotation that reports a boolean, and when the cover the scan found is an
323+
* anonymous container no reader could recognize; both refusals keep the generic recovery rather
324+
* than guessing a culprit. This is a description, not an authorization: it carries no ref and is
325+
* never an actionable target.
326+
*/
327+
coveredBy?: SnapshotCoveringNode;
300328
presentationHints?: string[];
301329
/**
302330
* Backend-minted ref for this node, when the capture backend already assigns a

‎packages/selectors/src/interaction-error.test.ts‎

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
1+
import { test } from 'vitest';
2+
import assert from 'node:assert/strict';
3+
import { coveredInteractionDetails } from './interaction-error.ts';
4+
5+
const scanCoveredNode = {
6+
ref: 'e2',
7+
interactionBlocked: 'covered' as const,
8+
coveredBy: {
9+
kind: 'dialog',
10+
label: 'Automation sheet',
11+
rect: { x: 0, y: 330, width: 390, height: 554 },
12+
},
13+
};
14+
15+
const policyCoveredNode = {
16+
ref: 'e7',
17+
interactionBlocked: 'covered' as const,
18+
};
19+
20+
test('coveredInteractionDetails names the cover the scan found and keys the refusal by reason', () => {
21+
assert.deepEqual(coveredInteractionDetails(scanCoveredNode), {
22+
reason: 'target_covered',
23+
hint: 'Covered by the dialog "Automation sheet" at (0, 330 390x554). Dismiss or close that element, scroll the target clear of it, or inspect with snapshot/screenshot before retrying.',
24+
ref: '@e2',
25+
interactionBlocked: 'covered',
26+
coveredBy: scanCoveredNode.coveredBy,
27+
});
28+
});
29+
30+
test('coveredInteractionDetails carries no selector unless the refusal was selector-addressed', () => {
31+
assert.equal(coveredInteractionDetails(scanCoveredNode).selector, undefined);
32+
assert.equal(
33+
coveredInteractionDetails(scanCoveredNode, { selector: 'label=Save draft' }).selector,
34+
'label=Save draft',
35+
);
36+
});
37+
38+
test('coveredInteractionDetails keeps the generic recovery for a policy-covered node', () => {
39+
// A backend-policy cover reports a boolean and names no culprit, so the detail omits `coveredBy`
40+
// rather than guessing one, and the hint stays the generic recovery.
41+
const details = coveredInteractionDetails(policyCoveredNode);
42+
assert.equal('coveredBy' in details, false);
43+
assert.equal(details.interactionBlocked, 'covered');
44+
assert.equal(
45+
details.hint,
46+
'Use a different visible target, scroll it clear of the overlay, or inspect with snapshot/screenshot before retrying.',
47+
);
48+
});
49+
50+
test('coveredInteractionDetails never advertises the cover as an actionable ref', () => {
51+
const details = coveredInteractionDetails(scanCoveredNode);
52+
assert.equal(details.ref, '@e2');
53+
assert.deepEqual(Object.keys(details.coveredBy!), ['kind', 'label', 'rect']);
54+
});
55+
56+
test('the hint rounds cover geometry and stays one line for an app-controlled label', () => {
57+
// The label is app-controlled text, so only the rendering is sanitized — the detail keeps it raw.
58+
const hint = String(
59+
coveredInteractionDetails({
60+
ref: 'e2',
61+
interactionBlocked: 'covered',
62+
coveredBy: {
63+
kind: 'tab-bar',
64+
label: ' Dismiss\n "now" ',
65+
rect: { x: 0.4, y: 760.2, width: 390.5, height: 84 },
66+
},
67+
}).hint,
68+
);
69+
assert.match(
70+
hint,
71+
/^Covered by the tab-bar "Dismiss now" at \(0, 760 391x84\)\. Dismiss or close/,
72+
);
73+
assert.doesNotMatch(hint, /\n/);
74+
});
75+
76+
test('the hint falls back to the bare kind when the label sanitizes to nothing', () => {
77+
const hint = String(
78+
coveredInteractionDetails({
79+
ref: 'e2',
80+
interactionBlocked: 'covered',
81+
coveredBy: { kind: 'tab-bar', label: '""\n', rect: { x: 0, y: 760, width: 390, height: 84 } },
82+
}).hint,
83+
);
84+
assert.match(hint, /^Covered by the tab-bar at \(0, 760 390x84\)/);
85+
});
86+
87+
test('the hint names a cover the capture published without a rect', () => {
88+
const hint = String(
89+
coveredInteractionDetails({
90+
ref: 'e2',
91+
interactionBlocked: 'covered',
92+
coveredBy: { kind: 'sheet' },
93+
}).hint,
94+
);
95+
assert.match(hint, /^Covered by the sheet\. Dismiss or close that element/);
96+
});

0 commit comments

Comments
 (0)