Skip to content

Commit 2db7c42

Browse files
authored
fix(android): carry the checked state of a checkable control on the snapshot node (#2913)
* fix(android): carry the checked state of a checkable control on the snapshot node The snapshot helper never serialized `checked` or `checkable`, and the host reads only the helper's XML, so no later layer could recover it: a switch, checkbox, or radio button looked the same on or off, `get attrs` had no `checked` field, and snapshot text showed the control as plain. The helper now writes `checked` on every node Android reports as checkable, with both answers, so an unchecked switch reads `false`, a node that cannot be checked reads nothing, and so does a helper older than the attribute. The parser, the Android hierarchy node, and the published snapshot node carry it to `get attrs`, the unchanged-snapshot comparison, the selector digest, and the `[checked]` and `[unchecked]` markers in snapshot text. Both answers render because the diff compares the state and a checkable control rendered as plain would hide that it toggles. * fix(daemon): the interaction outcome weighs the states stateMarkers prints, and a scroll's container check matches content by identity Review follow-up. The interaction surface key folded enabled and selected but not checked, so a tap whose only effect was a toggle read as a no-op on the outcome lane while the diff called it a change, and a no-change retry would tap the switch straight back. The key now spreads stateMarkers, so the outcome lane, the unchanged-snapshot comparison, and the diff weigh one list. With checked in the key, a flip inside a scroll container beside an unrelated change elsewhere (the status clock) read as content moving within the container. discriminatingSurfaceChangedWithinRect now matches entries on the flip-tolerant identity where one exists, told apart by document order when repeated, so a state flip at the same rect is not movement. Tests: a checked-only flip is a change on the outcome lane; a flip at the same rect is not movement and a moved row is; the scroll claim is withheld for a flipped toggle inside the container; checked in both answers invalidates snapshot reuse and survives the selector digest. * test(daemon): the capture-retry fixtures spell the interaction key with state markers The two hand-written pre-signatures carried the literal enabled and unselected segments the key no longer writes for a plain node, so the post-tap capture read as changed and the retry never fired. * fix(daemon): a surface entry carries a state-free content key, so an anonymous toggle is not movement Review follow-up. The within-container movement check fell back to `key` for an entry without an identity, and `key` carries the checked state, so an unlabelled switch a swipe brushed still read as content moving. Every entry now carries `content`: the identity where the node has one, else its type and role; the check compares on that alone. * fix(maestro): the snapshot signature weighs checked, so a toggle tap is not a no-op to retap maestroSnapshotSignature hashed label, value, enabled, selected, focused and bounds; a switch, checkbox or radio whose only observable effect is its checked state produced the same signature before and after the tap, so the settle and the no-change retry read the tap as a no-op and could tap it back. checked joins the hashed object the way the interaction outcome key already weighs it.
1 parent 9b08543 commit 2db7c42

25 files changed

Lines changed: 384 additions & 41 deletions

File tree

‎android/snapshot-helper/src/main/java/com/callstack/agentdevice/snapshothelper/AccessibilityTreeXml.java‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,12 @@ static void appendNode(
6969
// unselected control reports selected=false and a helper older than this attribute reports
7070
// nothing at all.
7171
appendAttribute(xml, "selected", Boolean.toString(node.isSelected()));
72+
// Present only on a checkable control, with both answers: an unchecked switch reports
73+
// checked=false, while a node that cannot be checked reports nothing, like a helper older than
74+
// this attribute.
75+
if (node.isCheckable()) {
76+
appendAttribute(xml, "checked", Boolean.toString(node.isChecked()));
77+
}
7278
boolean scrollable = node.isScrollable();
7379
if (scrollable) {
7480
appendAttribute(xml, "scrollable", "true");
@@ -164,9 +170,8 @@ private static boolean isHeading(AccessibilityNodeInfo node) {
164170
return Build.VERSION.SDK_INT >= Build.VERSION_CODES.P && node.isHeading();
165171
}
166172

167-
// Declared residue (agent-device #1832): checked / checkable / long-clickable are not serialized,
168-
// so toggle state is invisible to agents. Adding them is a helper protocol change (new attributes
169-
// + host parser + fields on the wire node), tracked there.
173+
// Declared residue (agent-device #1832): long-clickable is not serialized. Adding it is a helper
174+
// protocol change (new attribute + host parser + field on the wire node).
170175
private static void appendDrawingOrderAttribute(StringBuilder xml, AccessibilityNodeInfo node) {
171176
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.N) {
172177
appendAttribute(xml, "drawing-order", Integer.toString(node.getDrawingOrder()));
Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
import assert from 'node:assert/strict';
2+
import { test } from 'vitest';
3+
import type { SnapshotNode } from '@agent-device/kernel/snapshot';
4+
import { buildSnapshotDiff } from '../snapshot-diff.ts';
5+
6+
function toggle(checked?: boolean): SnapshotNode {
7+
return {
8+
ref: 'e12',
9+
index: 0,
10+
depth: 0,
11+
type: 'android.widget.Switch',
12+
label: 'Wi-Fi switch',
13+
enabled: true,
14+
hittable: true,
15+
...(checked === undefined ? {} : { checked }),
16+
};
17+
}
18+
19+
test('a checked-only flip diffs as changed lines that read differently', () => {
20+
const diff = buildSnapshotDiff([toggle(false)], [toggle(true)]);
21+
const changed = diff.lines.filter((line) => line.kind !== 'unchanged');
22+
23+
assert.equal(diff.summary.unchanged, 0);
24+
assert.equal(changed.length > 0, true);
25+
// The comparable key carries the checked state, so the rendered line has to as well, and both
26+
// answers render: the pair reads `[unchecked]` -> `[checked]`, not two identical lines.
27+
assert.match(changed[0]!.text, /\[unchecked\]/);
28+
assert.match(changed.at(-1)!.text, /\[checked\]/);
29+
});
30+
31+
test.each([
32+
['both unchecked', [toggle(false)], [toggle(false)]],
33+
['both unreported', [toggle()], [toggle()]],
34+
])('a still toggle with %s diffs as unchanged', (_label, previous, current) => {
35+
const diff = buildSnapshotDiff(previous, current);
36+
37+
assert.equal(
38+
diff.lines.every((line) => line.kind === 'unchanged'),
39+
true,
40+
);
41+
});
42+
43+
test('a node that cannot be checked never reads as unchecked', () => {
44+
const diff = buildSnapshotDiff([toggle()], [toggle()]);
45+
assert.equal(
46+
diff.lines.every((line) => !line.text.includes('checked')),
47+
true,
48+
);
49+
});

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

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import {
55
displayLabel,
66
formatRole,
77
formatSnapshotLine,
8+
stateMarkers,
89
} from './snapshot-lines.ts';
910

1011
export type SnapshotDiffResult = {
@@ -27,18 +28,17 @@ type SnapshotComparableLine = {
2728
function snapshotNodeToComparableLine(node: SnapshotNode, depthOverride?: number): string {
2829
const role = formatRole(node.type ?? 'Element');
2930
const textPart = displayLabel(node, role);
30-
const enabledPart = node.enabled === false ? 'disabled' : 'enabled';
31-
const selectedPart = node.selected === true ? 'selected' : 'unselected';
31+
// The states the rendered line prints are the states the key compares, from one function, so a
32+
// fact the diff weighs is always visible in the line it prints.
33+
const statePart = stateMarkers(node).join(',');
3234
const hittablePart = node.hittable === true ? 'hittable' : 'not-hittable';
3335
const depthPart = String(depthOverride ?? node.depth ?? 0);
3436
// The rendered line carries the actions list, so the comparable key has to as
3537
// well: otherwise an action-only change diffs as 'unchanged' while its text
3638
// silently differs from the baseline's. JSON-encoded because the names are
3739
// app-authored and may contain the field separator.
3840
const actionsPart = node.actions ? JSON.stringify(node.actions) : '';
39-
return [depthPart, role, textPart, enabledPart, selectedPart, hittablePart, actionsPart].join(
40-
'|',
41-
);
41+
return [depthPart, role, textPart, statePart, hittablePart, actionsPart].join('|');
4242
}
4343

4444
export function buildSnapshotDiff(

‎packages/capture-kit/src/snapshot/snapshot-freshness/android.ts‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -28,11 +28,12 @@ export function isNavigationSensitiveAction(command: string): boolean {
2828

2929
/**
3030
* Route signature of an Android snapshot, from the fields the Android backend actually carries.
31-
* The helper serializes no `role`, `checked` or `long-clickable` (declared residue,
32-
* #1832), so a signature keying on them would compare constants and claim discrimination it does
33-
* not have. `selected` is left out as a judgement call rather than an inability: a tab-bar tap
34-
* flips it on two nodes, which the 90%-identical threshold below absorbs at every tree size this
35-
* check runs on, so keying on it would only add capture retries.
31+
* The helper serializes no `role` or `long-clickable` (declared residue, #1832), so a signature
32+
* keying on them would compare constants and claim discrimination it does not have. `selected`
33+
* and `checked` are left out as a judgement call rather than an inability: a tab-bar tap flips
34+
* selection on two nodes and a toggle tap flips one checked state, which the 90%-identical
35+
* threshold below absorbs at every tree size this check runs on, so keying on them would only add
36+
* capture retries.
3637
*/
3738
export function buildSnapshotSignatures(nodes: SnapshotState['nodes']): string[] {
3839
return nodes.map((node) =>

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

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -195,6 +195,21 @@ export function formatRole(type: string): string {
195195
return lookupRoleLabel(normalized) || normalized || 'element';
196196
}
197197

198+
/**
199+
* The state markers every rendering path prints, and the states a snapshot diff compares: the diff
200+
* renders its lines without text-surface summarizing, and a fact it weighs has to be visible in the
201+
* line it prints, or a flip reads as a changed pair whose two lines look identical. Both checked
202+
* answers render, since a checkable control shown plain would hide that it toggles; a node that
203+
* cannot be checked carries neither.
204+
*/
205+
export function stateMarkers(node: SnapshotNode): string[] {
206+
const markers: string[] = [];
207+
if (node.enabled === false) markers.push('disabled');
208+
if (node.selected === true) markers.push('selected');
209+
if (node.checked !== undefined) markers.push(node.checked ? 'checked' : 'unchecked');
210+
return markers;
211+
}
212+
198213
function lookupRoleLabel(normalized: string): string | undefined {
199214
return Object.prototype.hasOwnProperty.call(ROLE_LABELS, normalized)
200215
? ROLE_LABELS[normalized]
@@ -238,13 +253,7 @@ function buildLineMetadata(
238253
options: SnapshotLineFormatOptions,
239254
textSurface: { text: string; isLargeSurface: boolean; shouldSummarize: boolean },
240255
): string[] {
241-
const metadata: string[] = [];
242-
if (node.enabled === false) metadata.push('disabled');
243-
// Selection is a state a snapshot diff can report as changed, and the diff renders its lines
244-
// without text-surface summarizing. A fact the diff compares has to be visible in the line it
245-
// prints, or a selection flip reads as a changed pair whose two lines look identical.
246-
if (node.selected === true) metadata.push('selected');
247-
metadata.push(...(node.presentationHints ?? []));
256+
const metadata = [...stateMarkers(node), ...(node.presentationHints ?? [])];
248257
if (!options.summarizeTextSurfaces) {
249258
return uniqueMetadata(metadata);
250259
}

‎packages/kernel/src/snapshot.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -251,6 +251,8 @@ export type RawSnapshotNode = {
251251
rect?: Rect;
252252
enabled?: boolean;
253253
selected?: boolean;
254+
/** Checked state of a checkable control (switch, checkbox, radio); absent means not checkable or unavailable. */
255+
checked?: boolean;
254256
focused?: boolean;
255257
/** Accessibility heading flag an app set on the node; absent means not a heading or unavailable. */
256258
heading?: boolean;

‎packages/maestro/src/daemon-port/__tests__/daemon-runtime-port-observation.test.ts‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -359,12 +359,26 @@ test('normalizes absent attributes like Maestro iOS hierarchy mapping', () => {
359359
enabled: false,
360360
selected: false,
361361
focused: false,
362+
checked: false,
362363
},
363364
]);
364365

365366
expect(maestroSnapshotSignature(first)).toBe(maestroSnapshotSignature(second));
366367
});
367368

369+
test('a checked-only flip changes the snapshot signature', () => {
370+
// An Android switch tapped on: nothing about it moves or renames, only `checked` flips. The
371+
// same signature before and after would read the tap as a no-op and retap it off again.
372+
const off = makeSnapshot([
373+
{ index: 0, type: 'android.widget.Switch', label: 'Wi-Fi', checked: false },
374+
]);
375+
const on = makeSnapshot([
376+
{ index: 0, type: 'android.widget.Switch', label: 'Wi-Fi', checked: true },
377+
]);
378+
379+
expect(maestroSnapshotSignature(off)).not.toBe(maestroSnapshotSignature(on));
380+
});
381+
368382
test('excludes agent-device presentation metadata from Maestro hierarchy signatures', () => {
369383
const first = makeSnapshot([
370384
{

‎packages/maestro/src/daemon-port/daemon-runtime-port-observation.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -408,6 +408,7 @@ export function maestroSnapshotSignature(snapshot: SnapshotState): string {
408408
enabled: node.enabled ?? false,
409409
selected: node.selected ?? false,
410410
focused: node.focused ?? false,
411+
checked: node.checked ?? false,
411412
bounds: maestroSnapshotBounds(node.rect),
412413
})),
413414
),
Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
import { expect, test } from 'vitest';
2+
import { buildUiHierarchySnapshot, parseUiHierarchyTree } from '../ui-hierarchy.ts';
3+
4+
// A settings row: a switch the helper reports as checkable beside a label that is not. The helper
5+
// writes `checked` only on the checkable node, with both answers.
6+
function togglesXml(wifiChecked: boolean): string {
7+
return `<hierarchy>
8+
<node class="android.widget.LinearLayout" resource-id="toggles" bounds="[0,0][400,200]"
9+
window-index="0" window-type="1" window-layer="1" window-active="true" window-focused="true"
10+
window-bounds="[0,0][400,800]" visible-to-user="true" enabled="true">
11+
<node class="android.widget.TextView" resource-id="wifi-label" text="Wi-Fi"
12+
bounds="[0,0][200,100]" enabled="true" visible-to-user="true" />
13+
<node class="android.widget.Switch" resource-id="wifi-switch" content-desc="Wi-Fi switch"
14+
bounds="[200,0][400,100]" clickable="true" enabled="true" visible-to-user="true"
15+
checked="${wifiChecked}" />
16+
<node class="android.widget.RadioButton" resource-id="size-small" text="Small"
17+
bounds="[0,100][400,200]" clickable="true" enabled="true" visible-to-user="true"
18+
checked="false" />
19+
</node>
20+
</hierarchy>`;
21+
}
22+
23+
// A helper older than the `checked` attribute, or a switch it did not report as checkable.
24+
const UNREPORTED_CHECKED_XML =
25+
'<hierarchy><node class="android.widget.Switch" resource-id="legacy-switch" content-desc="Wi-Fi"' +
26+
' bounds="[200,0][400,100]" clickable="true" enabled="true" visible-to-user="true" /></hierarchy>';
27+
28+
function toggleNodes(raw: boolean, interactiveOnly = false, wifiChecked = true) {
29+
const { nodes } = buildUiHierarchySnapshot(
30+
parseUiHierarchyTree(togglesXml(wifiChecked)),
31+
undefined,
32+
{ raw, interactiveOnly },
33+
);
34+
const byId = (identifier: string) => nodes.find((node) => node.identifier === identifier);
35+
return { label: byId('wifi-label'), wifi: byId('wifi-switch'), small: byId('size-small') };
36+
}
37+
38+
test.each([
39+
{ raw: false, interactiveOnly: false },
40+
{ raw: false, interactiveOnly: true },
41+
{ raw: true, interactiveOnly: false },
42+
{ raw: true, interactiveOnly: true },
43+
])(
44+
'checked state reaches snapshot nodes in every projection (raw=$raw, -i=$interactiveOnly)',
45+
({ raw, interactiveOnly }) => {
46+
const { wifi, small } = toggleNodes(raw, interactiveOnly);
47+
expect(wifi?.checked).toBe(true);
48+
expect(small?.checked).toBe(false);
49+
},
50+
);
51+
52+
test.each([true, false])('the switch answers the state the helper observed (%s)', (wifiChecked) => {
53+
expect(toggleNodes(false, false, wifiChecked).wifi?.checked).toBe(wifiChecked);
54+
});
55+
56+
test('attrs answer explicit false where a node that cannot be checked answers nothing', () => {
57+
// Serialized, because that is the answer an agent reads: an unavailable fact drops the key
58+
// while JSON encodes an observed `false`.
59+
const { label, small } = toggleNodes(false);
60+
expect(JSON.parse(JSON.stringify(small)).checked).toBe(false);
61+
expect(JSON.parse(JSON.stringify(label))).not.toHaveProperty('checked');
62+
});
63+
64+
test('an unreported checked state stays unknown instead of becoming false', () => {
65+
const node = buildUiHierarchySnapshot(parseUiHierarchyTree(UNREPORTED_CHECKED_XML), undefined, {
66+
raw: false,
67+
}).nodes.find((node) => node.identifier === 'legacy-switch');
68+
expect(node).toBeDefined();
69+
expect(node?.checked).toBeUndefined();
70+
});

‎packages/platform-android/src/ui-hierarchy-builder.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -356,6 +356,7 @@ function createAndroidRawSnapshotNode(
356356
selected: node.selected,
357357
heading: node.heading,
358358
roleDescription: node.roleDescription,
359+
checked: node.checked,
359360
editable: node.editable,
360361
password: node.password,
361362
hintShowing: node.hintShowing,

0 commit comments

Comments
 (0)