From 3bbeb6191733d19e04580b8bdff5698c68abf21b Mon Sep 17 00:00:00 2001 From: Ahmad Al-Faqih Date: Thu, 10 Sep 2026 15:12:41 +0300 Subject: [PATCH] fix(snapshot): compare unchanged presentations by value (#2442) * fix(snapshot): compare unchanged presentations by value * test(android): verify unchanged snapshot output live --------- Co-authored-by: PLASMA-FR <173463847+PLASMA-FR@users.noreply.github.com> --- CHANGELOG.md | 2 + .../runtime/snapshot-unchanged.test.ts | 138 ++++++++++++++++- .../capture/runtime/snapshot-unchanged.ts | 109 ++++++++++---- src/commands/capture/runtime/snapshot.test.ts | 35 +++++ src/commands/capture/snapshot.ts | 2 +- .../live-automation-scenario.ts | 8 + .../live-snapshot-compaction.ts | 142 ++++++++++++++++++ website/docs/docs/snapshots.md | 1 + 8 files changed, 403 insertions(+), 34 deletions(-) create mode 100644 test/integration/android-emulator-e2e/live-snapshot-compaction.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 7f0ed9ab4..77cf6cc33 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,8 @@ backend acquired, so every backend serves the request; the private AX declaration is now `regular-depth=presentation-cut` and an acquisition that stopped short of the cut keeps disclosing that through `truncated`/`effectiveDepth` as it does unscoped. +- Fixed: repeated unfiltered Android snapshots stay compact when identical element bounds arrive + with a different property order. Changes to the bounds still re-emit the tree. - Added: `replay export` supports flows that switch apps and return, preserving each `open ` target as an explicit Maestro `launchApp.appId`. - Added: `replay export` converts recorded `home` actions to Maestro `pressKey: Home`, allowing diff --git a/src/commands/capture/runtime/snapshot-unchanged.test.ts b/src/commands/capture/runtime/snapshot-unchanged.test.ts index 6c6fb507b..c0ef723b7 100644 --- a/src/commands/capture/runtime/snapshot-unchanged.test.ts +++ b/src/commands/capture/runtime/snapshot-unchanged.test.ts @@ -1,9 +1,18 @@ import { expect, test } from 'vitest'; +import fc from 'fast-check'; import { buildUnchangedSnapshotMetadata, ensureSnapshotPresentationKey, } from './snapshot-unchanged.ts'; -import type { SnapshotState, SnapshotStateProvenance } from '@agent-device/kernel/snapshot'; +import type { + SnapshotNode, + SnapshotState, + SnapshotStateProvenance, +} from '@agent-device/kernel/snapshot'; +import { + distinctRectPairArb, + PROPERTY_RUNS, +} from '@agent-device/selectors/snapshot-geometry-fixtures'; function snapshot( label: string, @@ -52,6 +61,133 @@ test('unchanged metadata detects visible label changes', () => { ).toBeUndefined(); }); +test.each>([ + { index: 1 }, + { depth: 1 }, + { parentIndex: 1 }, + { type: 'TextField' }, + { role: 'button' }, + { subrole: 'AXCloseButton' }, + { value: 'Draft' }, + { identifier: 'create' }, + { enabled: false }, + { selected: true }, + { focused: true }, + { hittable: false }, + { bundleId: 'com.example.app' }, + { appName: 'Example' }, + { windowTitle: 'Compose' }, + { surface: 'app' }, + { hiddenContentAbove: true }, + { hiddenContentBelow: true }, + { interactionBlocked: 'covered' }, +])('unchanged metadata detects presentation changes: %j', (change) => { + const previous = snapshot('Create'); + const current = snapshot('Create', { nodes: [{ ...previous.nodes[0]!, ...change }] }); + + expect(buildUnchangedSnapshotMetadata({ previous, current, options: {} })).toBeUndefined(); +}); + +test.each>([ + { editable: true }, + { password: true }, + { hintShowing: true }, + { selectionStart: 2 }, + { selectionEnd: 3 }, + { visibleToUser: true }, + { inheritsLabel: true }, + { inheritsIdentifier: true }, +])('unchanged metadata ignores non-presentation fields: %j', (change) => { + const previous = snapshot('Create'); + const current = snapshot('Create', { nodes: [{ ...previous.nodes[0]!, ...change }] }); + + expect(buildUnchangedSnapshotMetadata({ previous, current, options: {} })).toMatchObject({ + nodeCount: 1, + }); +}); + +test('unchanged metadata detects node count, order and truncation changes', () => { + const first = snapshot('Create').nodes[0]!; + const second = { ...first, index: 1, ref: 'e2', label: 'Cancel' }; + const previous = snapshot('Create', { nodes: [first, second], truncated: false }); + + for (const current of [ + { ...previous, nodes: [first] }, + { ...previous, nodes: [second, first] }, + { ...previous, truncated: true }, + ]) { + expect(buildUnchangedSnapshotMetadata({ previous, current, options: {} })).toBeUndefined(); + } +}); + +test('unchanged metadata compares rectangle values independently of property order', () => { + fc.assert( + fc.property(distinctRectPairArb, ({ ancestor, target }) => { + const previous = snapshot('Create'); + previous.nodes[0]!.rect = ancestor; + const current = snapshot('Create', { + nodes: [ + { + ...previous.nodes[0]!, + rect: { + height: ancestor.height, + width: ancestor.width, + y: ancestor.y, + x: ancestor.x, + }, + }, + ], + }); + + expect(buildUnchangedSnapshotMetadata({ previous, current, options: {} })).toMatchObject({ + nodeCount: 1, + }); + + current.nodes[0]!.rect = target; + expect(buildUnchangedSnapshotMetadata({ previous, current, options: {} })).toBeUndefined(); + + current.nodes[0]!.rect = undefined; + expect(buildUnchangedSnapshotMetadata({ previous, current, options: {} })).toBeUndefined(); + expect( + buildUnchangedSnapshotMetadata({ previous: current, current: previous, options: {} }), + ).toBeUndefined(); + }), + { numRuns: PROPERTY_RUNS }, + ); +}); + +test('unchanged metadata compares action and presentation hint arrays by value', () => { + const previous = snapshot('Create'); + previous.nodes[0]!.actions = ['Reply', 'Share']; + previous.nodes[0]!.presentationHints = ['offscreen']; + const current = snapshot('Create', { + nodes: [ + { + ...previous.nodes[0]!, + actions: ['Reply', 'Share'], + presentationHints: ['offscreen'], + }, + ], + }); + + expect(buildUnchangedSnapshotMetadata({ previous, current, options: {} })).toMatchObject({ + nodeCount: 1, + }); + + current.nodes[0]!.actions = ['Share', 'Reply']; + expect(buildUnchangedSnapshotMetadata({ previous, current, options: {} })).toBeUndefined(); + + current.nodes[0]!.actions = ['Reply']; + expect(buildUnchangedSnapshotMetadata({ previous, current, options: {} })).toBeUndefined(); + + current.nodes[0]!.actions = ['Reply', 'Share']; + current.nodes[0]!.presentationHints = []; + expect(buildUnchangedSnapshotMetadata({ previous, current, options: {} })).toBeUndefined(); + + current.nodes[0]!.presentationHints = undefined; + expect(buildUnchangedSnapshotMetadata({ previous, current, options: {} })).toBeUndefined(); +}); + test('unchanged metadata requires comparison-safe snapshots', () => { expect( buildUnchangedSnapshotMetadata({ diff --git a/src/commands/capture/runtime/snapshot-unchanged.ts b/src/commands/capture/runtime/snapshot-unchanged.ts index a3d450a34..dab0fe96a 100644 --- a/src/commands/capture/runtime/snapshot-unchanged.ts +++ b/src/commands/capture/runtime/snapshot-unchanged.ts @@ -1,6 +1,7 @@ import type { SnapshotCommandOptions } from '../../runtime-types.ts'; import { buildSnapshotPresentationKey, + type Rect, type SnapshotNode, type SnapshotState, type SnapshotUnchanged, @@ -69,41 +70,85 @@ function areSnapshotPresentationsEquivalent( current: SnapshotState, ): boolean { if (previous.truncated !== current.truncated) return false; - // TODO: replace stringify with a field-by-field comparison or stable presentation hash. return ( - JSON.stringify(buildComparableSnapshotPresentation(previous.nodes)) === - JSON.stringify(buildComparableSnapshotPresentation(current.nodes)) + previous.nodes.length === current.nodes.length && + previous.nodes.every((node, index) => areSnapshotNodesEquivalent(node, current.nodes[index]!)) ); } -function buildComparableSnapshotPresentation( - nodes: readonly SnapshotNode[], -): ComparableSnapshotNode[] { - return nodes.map((node) => ({ - index: node.index, - depth: node.depth, - parentIndex: node.parentIndex, - type: node.type, - role: node.role, - subrole: node.subrole, - label: node.label, - value: node.value, - identifier: node.identifier, - enabled: node.enabled, - selected: node.selected, - focused: node.focused, - hittable: node.hittable, - rect: node.rect, - bundleId: node.bundleId, - appName: node.appName, - windowTitle: node.windowTitle, - surface: node.surface, - hiddenContentAbove: node.hiddenContentAbove, - hiddenContentBelow: node.hiddenContentBelow, - interactionBlocked: node.interactionBlocked, - presentationHints: node.presentationHints, - actions: node.actions, - })); +// Native text-entry/visibility facts are not rendered; refs and process ids are volatile. +// Inheritance markers are output-only: this comparison runs before label deduplication. +type ComparableSnapshotNode = Omit< + SnapshotNode, + | 'ref' + | 'pid' + | 'editable' + | 'password' + | 'hintShowing' + | 'selectionStart' + | 'selectionEnd' + | 'visibleToUser' + | 'inheritsLabel' + | 'inheritsIdentifier' +>; + +type ScalarPresentationField = Exclude< + keyof ComparableSnapshotNode, + 'rect' | 'presentationHints' | 'actions' +>; + +const PRESENTATION_SCALAR_FIELDS = { + index: true, + depth: true, + parentIndex: true, + type: true, + role: true, + subrole: true, + label: true, + value: true, + identifier: true, + enabled: true, + selected: true, + focused: true, + hittable: true, + bundleId: true, + appName: true, + windowTitle: true, + surface: true, + hiddenContentAbove: true, + hiddenContentBelow: true, + interactionBlocked: true, +} satisfies Record; + +const PRESENTATION_SCALAR_KEYS = Object.keys( + PRESENTATION_SCALAR_FIELDS, +) as ScalarPresentationField[]; + +function areSnapshotNodesEquivalent(previous: SnapshotNode, current: SnapshotNode): boolean { + return ( + PRESENTATION_SCALAR_KEYS.every((field) => previous[field] === current[field]) && + areRectsEquivalent(previous.rect, current.rect) && + areStringArraysEquivalent(previous.presentationHints, current.presentationHints) && + areStringArraysEquivalent(previous.actions, current.actions) + ); } -type ComparableSnapshotNode = Omit; +function areRectsEquivalent(previous: Rect | undefined, current: Rect | undefined): boolean { + if (!previous || !current) return previous === current; + return ( + previous.x === current.x && + previous.y === current.y && + previous.width === current.width && + previous.height === current.height + ); +} + +function areStringArraysEquivalent( + previous: readonly string[] | undefined, + current: readonly string[] | undefined, +): boolean { + if (!previous || !current) return previous === current; + return ( + previous.length === current.length && previous.every((value, index) => value === current[index]) + ); +} diff --git a/src/commands/capture/runtime/snapshot.test.ts b/src/commands/capture/runtime/snapshot.test.ts index a8c193646..194abfe41 100644 --- a/src/commands/capture/runtime/snapshot.test.ts +++ b/src/commands/capture/runtime/snapshot.test.ts @@ -82,6 +82,41 @@ test('runtime snapshot upgrades an absent truncation flag only for producers tha } }); +test('runtime snapshot recognizes equivalent bounds and still updates its baseline', async () => { + let stored: Parameters[0] | undefined; + let rect = { x: 10, y: 20, width: 100, height: 40 }; + const device = createAgentDevice({ + backend: createSnapshotBackend(() => ({ + snapshot: makeSnapshotState([{ index: 0, depth: 0, type: 'Button', label: 'Save', rect }], { + comparisonSafe: true, + }), + })), + artifacts: createLocalArtifactAdapter(), + sessions: { + get: () => stored, + set: (record) => { + stored = record; + }, + }, + policy: localCommandPolicy(), + }); + + await device.capture.snapshot({ session: 'default' }); + rect = { height: 40, width: 100, y: 20, x: 10 }; + const repeated = await device.capture.snapshot({ session: 'default' }); + + assert.equal(repeated.unchanged?.nodeCount, 1); + assert.equal(stored?.snapshot?.nodes, repeated.nodes); + + const forced = await device.capture.snapshot({ session: 'default', forceFull: true }); + assert.equal(forced.unchanged, undefined); + + rect = { ...rect, x: 11 }; + const moved = await device.capture.snapshot({ session: 'default' }); + assert.equal(moved.unchanged, undefined); + assert.equal(stored?.snapshot?.nodes[0]?.rect?.x, 11); +}); + test('runtime snapshot uses the Appium sparse-tree disclosure for Appium acquisition', async () => { const device = createSnapshotOnlyDevice({ snapshot: { diff --git a/src/commands/capture/snapshot.ts b/src/commands/capture/snapshot.ts index 3e13e49bc..4cd05d8f6 100644 --- a/src/commands/capture/snapshot.ts +++ b/src/commands/capture/snapshot.ts @@ -76,7 +76,7 @@ export const snapshotCommandFacet = defineCommandFacet({ name: SNAPSHOT_COMMAND_NAME, text: { summary: 'Capture or diff the accessibility tree', - cliDetail: `For iOS raw-coordinate fallback after a no-op ref press, inspect rects with snapshot -i --json, press the rect center, then verify with diff snapshot -i or snapshot --diff. iOS backend capability contract: ${snapshotBackendCapabilityHelp}.`, + cliDetail: `Repeated equivalent unfiltered Android snapshots return a compact unchanged acknowledgement. Use --force-full to re-emit the tree; --json and --raw retain full output. For iOS raw-coordinate fallback after a no-op ref press, inspect rects with snapshot -i --json, press the rect center, then verify with diff snapshot -i or snapshot --diff. iOS backend capability contract: ${snapshotBackendCapabilityHelp}.`, }, metadata: snapshotCommandMetadata, run: (client, input) => client.capture.snapshot(input), diff --git a/test/integration/android-emulator-e2e/live-automation-scenario.ts b/test/integration/android-emulator-e2e/live-automation-scenario.ts index 358e1630c..8d9b86ff8 100644 --- a/test/integration/android-emulator-e2e/live-automation-scenario.ts +++ b/test/integration/android-emulator-e2e/live-automation-scenario.ts @@ -15,6 +15,7 @@ import { scrollToVisibleSelector, } from './live-assertions.ts'; import { type LiveContext, runStep, verifyBehavior, verifyCommand } from './live-harness.ts'; +import { assertHumanSnapshotCompaction } from './live-snapshot-compaction.ts'; const C = PUBLIC_COMMANDS; @@ -52,6 +53,13 @@ export async function assertAutomationSystem(context: LiveContext): Promise { + const evidence: { command: string; result: ExecResult }[] = []; + try { + const baseline = await runSnapshot(context, ['--force-full'], evidence); + assertFullSnapshot(baseline.stdout ?? '', 'forced baseline'); + + const { baseline: stableBaseline, unchanged } = await readUntilCompact( + context, + baseline, + evidence, + ); + assert.doesNotMatch( + unchanged.stdout ?? '', + /@e\d+/, + 'compact output must not re-emit element refs', + ); + assert.ok( + Buffer.byteLength(unchanged.stdout ?? '') < Buffer.byteLength(stableBaseline.stdout ?? ''), + 'compact output should be smaller than the full tree', + ); + + const forced = await runSnapshot(context, ['--force-full'], evidence); + assertFullSnapshot(forced.stdout ?? '', 'forced repeat'); + + const back = await runCmd( + 'adb', + ['-s', context.serial, 'shell', 'input', 'keyevent', 'KEYCODE_BACK'], + { + allowFailure: true, + env: context.env, + timeoutMs: CLI_TIMEOUT_MS, + }, + ); + evidence.push({ command: 'adb shell input keyevent KEYCODE_BACK', result: back }); + assert.equal(back.exitCode, 0, `external Android Back failed: ${back.stderr ?? ''}`); + + const changed = await readUntilChangedRoute(context, evidence); + + console.log( + `Android human snapshot compaction passed: full=${Buffer.byteLength(stableBaseline.stdout ?? '')}B, compact=${Buffer.byteLength(unchanged.stdout ?? '')}B, forced=${Buffer.byteLength(forced.stdout ?? '')}B, changed=${Buffer.byteLength(changed.stdout ?? '')}B`, + ); + } finally { + fs.writeFileSync( + path.join(context.artifactDir, 'snapshot-compaction.txt'), + evidence + .map(({ command, result }) => + [`$ ${command}`, result.stdout ?? '', result.stderr ?? ''].filter(Boolean).join('\n'), + ) + .join('\n\n'), + ); + } +} + +async function readUntilCompact( + context: LiveContext, + initialBaseline: ExecResult, + evidence: { command: string; result: ExecResult }[], +): Promise<{ baseline: ExecResult; unchanged: ExecResult }> { + let baseline = initialBaseline; + for (let attempt = 1; attempt <= MAX_SNAPSHOT_ATTEMPTS; attempt += 1) { + const current = await runSnapshot(context, [], evidence); + if (/^Snapshot unchanged since previous read /m.test(current.stdout ?? '')) { + return { baseline, unchanged: current }; + } + assertFullSnapshot(current.stdout ?? '', `comparison attempt ${attempt.toString()}`); + baseline = current; + } + assert.fail(`no identical Android snapshot compacted after ${MAX_SNAPSHOT_ATTEMPTS} attempts`); +} + +async function readUntilChangedRoute( + context: LiveContext, + evidence: { command: string; result: ExecResult }[], +): Promise { + for (let attempt = 1; attempt <= MAX_SNAPSHOT_ATTEMPTS; attempt += 1) { + const current = await runSnapshot(context, [], evidence); + if (/Open automation lab/.test(current.stdout ?? '')) { + assertFullSnapshot(current.stdout ?? '', 'changed route'); + return current; + } + if (attempt < MAX_SNAPSHOT_ATTEMPTS) { + await new Promise((resolve) => setTimeout(resolve, UI_TRANSITION_SETTLE_MS)); + } + } + assert.fail( + `the externally changed Android route was not visible after ${MAX_SNAPSHOT_ATTEMPTS} attempts`, + ); +} + +async function runSnapshot( + context: LiveContext, + extraArgs: string[], + evidence: { command: string; result: ExecResult }[], +): Promise { + const args = [ + 'bin/agent-device.mjs', + 'snapshot', + ...extraArgs, + '--platform', + 'android', + '--serial', + context.serial, + '--session', + context.session, + '--daemon-server-mode', + 'dual', + ]; + const result = await runCmd(process.execPath, args, { + allowFailure: true, + env: context.env, + timeoutMs: CLI_TIMEOUT_MS, + }); + evidence.push({ command: `agent-device ${args.slice(1).join(' ')}`, result }); + assert.equal(result.exitCode, 0, `human snapshot failed: ${result.stderr ?? ''}`); + return result; +} + +function assertFullSnapshot(output: string, description: string): void { + assert.match( + output, + /^Snapshot: \d+(?: visible)? nodes?(?: \(\d+ total\))?(?: \(truncated\))?$/m, + `${description} should print a tree header`, + ); + assert.match(output, /@e\d+/, `${description} should print element refs`); + assert.doesNotMatch(output, /snapshot unchanged/i, `${description} must not compact`); +} diff --git a/website/docs/docs/snapshots.md b/website/docs/docs/snapshots.md index bd38276f9..c2e162c51 100644 --- a/website/docs/docs/snapshots.md +++ b/website/docs/docs/snapshots.md @@ -48,6 +48,7 @@ agent-device snapshot --diff # Alias for the same diff operation - iOS and Android share the same mobile snapshot contract: visible-first output, actionable-now refs, and hidden list content communicated via discovery hints. - Default to `snapshot -i` for agent loops. +- Repeated unfiltered Android snapshots with unchanged presented content and bounds return a compact acknowledgement. `-i`, `-d`, `-s`, `--json`, and `--raw` retain full output. Use `--force-full` to re-emit the tree explicitly. - Default snapshot text is an agent-facing, token-efficient view for planning and targeting actions. It is visible-first and may collapse helper/accessibility noise; use `--raw` or `--json` when you need the full provider tree. - Off-screen interactive content is collapsed into discovery summaries such as `[off-screen below] 3 interactive items: "Privacy", "Battery", "About"`. - If a target only appears in an off-screen summary, use `scroll ` and re-snapshot until the target becomes visible.