fix(android): carry the checked state of a checkable control on the snapshot node by okwasniewski · Pull Request #2913 · callstack/agent-device

okwasniewski · 2026-09-24T14:40:11Z

Summary

The helper never serialized checked or checkable, and the host reads only its XML, so 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 checkable node, with both answers: an unchecked switch reads false, a node that cannot be checked reads nothing, like an older helper. The parser, the Android node, and the published snapshot node carry it to get attrs, the unchanged-snapshot comparison, the selector digest, and snapshot text, which marks [checked] or [unchecked]. One function, stateMarkers, now yields the printed states, the diff key, and the interaction-outcome key, so a toggle tap reads as a changed pair and not as a no-op to retry. A scroll's within-container check matches content by identity, so a flip is not movement.

@e16 [switch] "Wi-Fi switch" [unchecked]
@e22 [radiobutton] "Small" [selected] [checked]

22 files. No HELPER_API_VERSION bump, as with selected (#2515). No is checked predicate or checked= selector key here.

Validation

Tested at dc8f367 (rebased onto 86d719d). pnpm check:affected --run --base 86d719dc3 passed: format, lint, typecheck, layering, fallow, build, 854 test files, 6,687 tests.

Rebuilt helper on an API 36 emulator, React Native app: Switch, CheckBox, and RadioButton carry checked in snapshot --json and get attrs, text nodes do not, and press @switch --settle reports - [unchecked] then + [checked].

Risk: a --settle that read a toggle tap as unchanged now reports the flip; the outcome lane does too.

Copilot

Copilot review overview

🟡 Changes recommended

Add regression coverage for checked-state unchanged detection and selector digests, including true, false, and absent cases.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 2

Open (2)

Add checked-state regression cases to snapshot unchanged tests · New
Add checked cases to selector digest response-view tests · New

What changed in this PR

Adds Android checked state propagation from helper XML through snapshots, attributes, digests, diffs, and rendered output.

Changes:

Serializes and parses checked states for Android checkable controls.
Displays [checked]/[unchecked] and detects toggle changes.
Updates tests, documentation, and changelog.

File	Description
`website/docs/docs/snapshots.md`	Documents Android checked metadata.
`website/docs/docs/commands.md`	Updates helper limitation documentation.
`src/daemon/response-views.ts`	Includes checked state in selector digests.
`src/commands/output/snapshot.test.ts`	Tests checked markers.
`src/commands/capture/runtime/snapshot-unchanged.ts`	Compares checked state for freshness.
`src/__tests__/android-ui-hierarchy.test.ts`	Tests published checked state.
`packages/platform-android/src/ui-hierarchy.ts`	Parses checked XML attributes.
`packages/platform-android/src/ui-hierarchy-node.ts`	Adds checked to Android nodes.
`packages/platform-android/src/ui-hierarchy-builder.ts`	Publishes checked state.
`packages/platform-android/src/__tests__/ui-hierarchy-checked.test.ts`	Tests checked propagation.
`packages/kernel/src/snapshot.ts`	Extends snapshot contracts.
`packages/capture-kit/src/snapshot/snapshot-lines.ts`	Renders checked markers.
`packages/capture-kit/src/snapshot/snapshot-freshness/android.ts`	Documents freshness behavior.
`packages/capture-kit/src/snapshot/snapshot-diff.ts`	Compares checked state markers.
`packages/capture-kit/src/snapshot/__tests__/snapshot-diff-checked.test.ts`	Tests checked diffs.
`CHANGELOG.md`	Records the Android fix.
`android/snapshot-helper/src/main/java/com/callstack/agentdevice/snapshothelper/AccessibilityTreeXml.java`	Serializes checked state from Android controls.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

okwasniewski · 2026-09-24T15:48:28Z

  identifier: true,
  enabled: true,
  selected: true,
+  checked: true,


Added in e5bba1b: { checked: true } and { checked: false } sit in the "detects presentation changes" table, so an absent-to-false flip is a change too.

okwasniewski · 2026-09-24T15:48:30Z

  'identifier',
  'enabled',
  'selected',
+  'checked',


Added in e5bba1b: the attrs digest test covers checked: true, checked: false, and a node without the fact, which stays absent.

thymikee

Reviewed at d2d4d35. stateMarkers is the right place for the rendering/diff state vocabulary. The remaining issue is that the PR now has a third state-folding lane, interaction outcome, and it still uses the pre-checked shape. Copilot already covers the missing tests; this is the behavioral divergence.

thymikee · 2026-09-24T15:22:36Z

+  const markers: string[] = [];
+  if (node.enabled === false) markers.push('disabled');
+  if (node.selected === true) markers.push('selected');
+  if (node.checked !== undefined) markers.push(node.checked ? 'checked' : 'unchecked');


stateMarkers now declares which states a comparison weighs, and this PR applies that answer in snapshot-unchanged.ts and the Android freshness comment. A third state signature still disagrees: interactionSurfaceSemanticKey in src/daemon/interaction-outcome-policy.ts:595 folds enabled, selected, and hittable but not checked. Android is checked's only producer, so a tap whose only effect is a toggle can be changed on the diff lane but unchanged on the interaction-outcome lane, causing retry or delayed-recheck work for a gesture that actually worked. Please either fold checked into that key, or make the exclusion explicit and test it. If including it, also account for discriminatingSurfaceChangedWithinRect in src/daemon/scroll-movement.ts:385, where an unrelated in-container toggle now counts as scroll movement.

Folded in, e5bba1b. interactionSurfaceSemanticKey now spreads stateMarkers(node) for its state segments, so the outcome lane, the unchanged-snapshot comparison, and the diff weigh one list; a checked-only flip classifies changed instead of feeding a no-change retry that would tap the switch back.

For the scroll lane: the flip alone never reaches discriminatingSurfaceChangedWithinRect, since classifyBaselineSurfaceEvidence matches on the flip-tolerant identity and reads it as unchanged. It does reach it beside an unrelated change, such as the status clock ticking, and there a key-matched view read the flipped switch as content moving inside the container. The function now matches entries on identity where one exists, told apart by document order when repeated, and on key otherwise, so a state flip at the same rect is not movement. haveIdenticalDiscriminatingSurfaces keeps its key-matched veto, as its comment asks.

Tests: a checked-only flip is changed on the outcome lane; a flip at the same rect is no movement while a moved row is; the scroll claim is withheld (change-outside-container) for a flipped toggle inside the container beside a clock tick; both checked answers invalidate snapshot reuse and survive the selector digest.

thymikee · 2026-09-24T15:29:27Z

Reviewed at d2d4d35. The change looks correct: checked is written only for checkable nodes, stays absent (not false) for other nodes and older helpers, and reaches get attrs, snapshot text, the unchanged-snapshot comparison, and the selector digest. It is ready for human review.

Not blocking: the two checked cases Copilot suggested for src/commands/capture/runtime/snapshot-unchanged.test.ts and src/daemon/__tests__/response-views.test.ts are still missing. They would guard the toggle-diff and digest behavior against a later refactor.

The on-device run comes from the PR description; I did not repeat it. The Android Smoke Tests job was still running at review time. It builds the helper this PR changes, so a green run there confirms the change on an emulator.

Copilot

Copilot review overview

🟡 Changes recommended

Resolve the moderate interaction-movement issue and correct the changelog file count.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 3

Open (3)

Anonymous switch state changes falsely appear as surface movement · New
Add checked cases to selector digest response-view tests
Add checked-state regression cases to snapshot unchanged tests

Copilot

Copilot review overview

🔵 Needs a closer look

Anonymous toggles need a state-independent fallback identity to avoid incorrect movement detection.

Review effort: Lite
Findings: 3

Open (3)

Anonymous switch state changes falsely appear as surface movement
Add checked cases to selector digest response-view tests
Add checked-state regression cases to snapshot unchanged tests

Copilot

Copilot review overview

🟡 Changes recommended

The checked state is not yet included in the settle signature, and state-derived identity can misclassify toggle changes as movement.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1 · 2

Open (3)

Include checked state in Maestro tap/settle surface signature · New
Add checked cases to selector digest response-view tests
Add checked-state regression cases to snapshot unchanged tests

Resolved since last review (1)

Anonymous switch state changes falsely appear as surface movement

+  /** Checked state of a checkable control (switch, checkbox, radio); absent means not checkable or unavailable. */
+  checked?: boolean;


thymikee · 2026-09-24T16:09:11Z

needs rebase

thymikee · 2026-09-24T16:40:15Z

One finding is still open at dc8f367, so this stays blocking.

maestroSnapshotSignature at https://github.com/callstack/agent-device/blob/dc8f367/packages/maestro/src/daemon-port/daemon-runtime-port-observation.ts#L396 hashes { index, parentIndex, identifier, label, value, enabled, selected, focused, bounds } but not checked. Tapping an Android switch, checkbox, or radio whose only observable effect is the toggle produces the same pre/post signature, so the settle and no-change-retry logic in daemon-runtime-tap.ts reads the tap as a no-op and can retap, flipping the control back to its pre-tap state on any Maestro flow that toggles one of these controls. Please fold checked into the hashed object the same way commit deb1325876 already did for interactionSurfaceSemanticKey, and add a regression that a checked-only flip changes the signature.

The PR currently has merge conflicts with main and needs a rebase before it can land. Smoke Tests, which builds and runs the Android snapshot helper this PR touches, is still queued, so it has not yet produced a result to weigh in. Repo Guards was cancelled rather than failed, and this diff's logic changes fall outside what that gate checks beyond the normal lint and type gates.

I did not run the daemon or Maestro suites locally for this pass; the read is from the diff and existing tests. I also did not do a live device run for commits 2-4, since they are TS decision logic over snapshots already captured on-device in the earlier commit and reviewed clean before.

okwasniewski · 2026-09-24T16:51:20Z

Rebased onto dc9ab86 (#2915 and the CHANGELOG removal in #2922 landed underneath; the conflicts were the adjacent field lines and the changelog entry, which is gone now). Head 4be8eba. Gate on it: check:affected --run --base dc9ab863a, 855 test files, 6,693 tests, fallow and oxfmt clean.

Copilot

Copilot review overview

🟡 Changes recommended

Stabilize checkable-node identity when labels or values change with checked state, and add regression coverage.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1 · 3

Open (4)

Include checked state in Maestro tap/settle surface signature
Use state-independent identity for checkable controls · New
Add checked cases to selector digest response-view tests
Add checked-state regression cases to snapshot unchanged tests

  return {
    key: `${semanticKey}|#${occurrence}`,
    ...(identity ? { identity } : {}),
+    content: interactionSurfaceContent(node, identity),


okwasniewski · 2026-09-24T17:18:13Z

Folded checked into maestroSnapshotSignature in c7236cb (checked: node.checked ?? false, beside selected and focused), with a regression: a switch that only flips checked changes the signature, and the absent-attribute normalization test now lists checked: false. Gate on the head: check:affected --run --base dc9ab863a passed.

Copilot

Copilot review overview

🔵 Needs a closer look

Mutable label/value identity can still make a toggle appear as movement during scroll detection.

Review effort: Lite
Findings: 1 · 3

Open (4)

Include checked state in Maestro tap/settle surface signature
Use state-independent identity for checkable controls
Add checked cases to selector digest response-view tests
Add checked-state regression cases to snapshot unchanged tests

thymikee · 2026-09-24T17:36:51Z

The code looks ready at c7236cb. The comment sniffing described in dc8f367 is gone, and the checked-state fix now carries through the snapshot node as before. Not blocking: the two-line comment above checked: node.checked ?? false in packages/maestro/src/daemon-port/daemon-runtime-port-observation.ts#L411 describes caller control flow rather than the code itself, and the regression test already encodes the invariant, so it can be trimmed or kept as you prefer.

Smoke Tests and Coverage were still in progress at review time, and Smoke Tests exercises the Android snapshot helper (AccessibilityTreeXml.java) this PR touches, so merge should wait for both to finish. I did not run the Maestro or daemon suites locally, and did not do a live on-device Android run for this change, since it's TS logic over a checked field that earlier commits in this branch already captured from a real device XML fixture.

okwasniewski · 2026-09-24T17:42:56Z

Trimmed the comment above checked: node.checked ?? false as suggested; the regression test carries the invariant. Folded into the Maestro commit, head 67b5525, gate green again (check:affected --run --base dc9ab863a).

Copilot

Copilot review overview

🔵 Needs a closer look

Stabilize interaction identity when checked-state changes also alter a control’s label or value.

Review effort: Lite
Findings: 1 · 3

Open (4)

Include checked state in Maestro tap/settle surface signature
Use state-independent identity for checkable controls
Add checked cases to selector digest response-view tests
Add checked-state regression cases to snapshot unchanged tests

…napshot 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.

…rints, 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.

…th 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.

…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.

…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.

okwasniewski · 2026-09-24T17:51:05Z

Rebased onto fa1082d after #2927 landed (one docs-list conflict in snapshots.md, now listing checked beside placeholder). Head 6dae0a1; check:affected --run --base fa1082dd1 passed: 6,766 tests.

Copilot

Copilot review overview

🔵 Needs a closer look

State-dependent labels or values can still make a same-position toggle appear as content movement; fix this and add regression coverage.

Review effort: Lite
Findings: 1 · 3

Open (4)

Include checked state in Maestro tap/settle surface signature
Use state-independent identity for checkable controls
Add checked cases to selector digest response-view tests
Add checked-state regression cases to snapshot unchanged tests

Copilot AI lite review requested due to automatic review settings September 24, 2026 14:40

Copilot started reviewing on behalf of okwasniewski September 24, 2026 14:40 View session