diff --git a/docs/adr/0004-ios-snapshot-backend-strategy.md b/docs/adr/0004-ios-snapshot-backend-strategy.md index b20b6d0faf..ec09844f69 100644 --- a/docs/adr/0004-ios-snapshot-backend-strategy.md +++ b/docs/adr/0004-ios-snapshot-backend-strategy.md @@ -164,6 +164,15 @@ nodes belong in the regular presentation, while publication adds refs and emits payload. An optional unscoped quality payload is validated for classification evidence and is never published. +Semantic compaction may move an identifier; it may not un-make one. A structural `Other` wrapper +carrying an identifier and nothing else is suppressed in favour of its content, which is a +delegation: the identifier goes on living in whatever the wrapper stood for. A wrapper with no +content has nothing to delegate to, so suppressing it deletes the identifier from every canonical +view while `is`, `get`, and `click` still resolve it from the same capture. That deletion needs the +node's own declared `hittable: false`, because it is the only verdict in the capture that says the +wrapper is inert; a producer that reports no hittability for any node declares nothing, and an +absent fact is not a negative answer (#2638). + ## Regression Notes PR #639 made XCTest AX serialization failures explicit instead of swallowing them as empty diff --git a/packages/capture-kit/src/ios-snapshot-engine/mapping.test.ts b/packages/capture-kit/src/ios-snapshot-engine/mapping.test.ts index 885f3071c8..f265f274fd 100644 --- a/packages/capture-kit/src/ios-snapshot-engine/mapping.test.ts +++ b/packages/capture-kit/src/ios-snapshot-engine/mapping.test.ts @@ -44,6 +44,7 @@ test('publishes an exact representative for every semantic source index', () => parentIndex: 4, type: 'Other', identifier: 'shared-but-not-a-representative', + hittable: false, rect: { x: 20, y: 240, width: 120, height: 40 }, }, ]; diff --git a/packages/capture-kit/src/ios-snapshot-engine/noise-structural.ts b/packages/capture-kit/src/ios-snapshot-engine/noise-structural.ts index a7ae117622..cafa3847d4 100644 --- a/packages/capture-kit/src/ios-snapshot-engine/noise-structural.ts +++ b/packages/capture-kit/src/ios-snapshot-engine/noise-structural.ts @@ -17,7 +17,14 @@ export function collectIosStructuralIdentifierSuppression( if (!node.identifier?.trim()) { continue; } - context.suppressNode(node, collectSubtreeByParentLinks(node, childrenByParent)); + const content = collectSubtreeByParentLinks(node, childrenByParent); + // Suppression delegates the identifier to the wrapper's content. With no content there is + // nothing to delegate to, and only the node's own declared `hittable: false` says the wrapper + // is inert; a producer that reports no hittability at all declares nothing (#2638). + if (content.length === 0 && node.hittable !== false) { + continue; + } + context.suppressNode(node, content); } } diff --git a/packages/capture-kit/src/ios-snapshot-engine/noise.test.ts b/packages/capture-kit/src/ios-snapshot-engine/noise.test.ts index fa6a3b277f..42c375adfc 100644 --- a/packages/capture-kit/src/ios-snapshot-engine/noise.test.ts +++ b/packages/capture-kit/src/ios-snapshot-engine/noise.test.ts @@ -77,6 +77,39 @@ describe('collectIosStructuralIdentifierSuppression', () => { expect(suppressed.sort((a, b) => a - b)).toEqual([1, 4, 7]); }); + test('keeps a childless identifier wrapper whose capture reported no hittability', () => { + const nodes: RawSnapshotNode[] = [ + { index: 0, type: 'Application', label: 'App' }, + { index: 1, parentIndex: 0, type: 'Other', identifier: 'members.count' }, + ]; + + const unreported: number[] = []; + collectIosStructuralIdentifierSuppression(nodes, { + ...makeRuleContext(nodes), + suppressNode: (source) => unreported.push(source.index), + }); + expect(unreported).toEqual([]); + + const declared: number[] = []; + collectIosStructuralIdentifierSuppression( + [{ ...nodes[0]! }, { ...nodes[1]!, hittable: false }], + { ...makeRuleContext(nodes), suppressNode: (source) => declared.push(source.index) }, + ); + expect(declared).toEqual([1]); + }); + + test('delegates an identifier wrapper to its content even without a hittability verdict', () => { + const { nodes } = makeStructuralTree(1, 2); + const suppressed: number[] = []; + + collectIosStructuralIdentifierSuppression(nodes, { + ...makeRuleContext(nodes), + suppressNode: (source) => suppressed.push(source.index), + }); + + expect(suppressed).toEqual([1]); + }); + test('builds the child index once instead of per structural candidate', () => { const candidateCount = 40; const descendantsPerCandidate = 25; diff --git a/packages/capture-kit/src/ios-snapshot-engine/presentation.test.ts b/packages/capture-kit/src/ios-snapshot-engine/presentation.test.ts index ac6adfe851..0eed84e345 100644 --- a/packages/capture-kit/src/ios-snapshot-engine/presentation.test.ts +++ b/packages/capture-kit/src/ios-snapshot-engine/presentation.test.ts @@ -498,6 +498,7 @@ test('buildSnapshotState keeps React Native warning banner instead of full-scree parentIndex: 1, type: 'Other', identifier: 'SearchRouterPage', + hittable: false, rect: { x: 0, y: 0, width: 402, height: 874 }, }, { @@ -719,6 +720,7 @@ test('buildSnapshotState suppresses structural iOS identifier-only nodes', () => parentIndex: 0, type: 'Other', identifier: 'SearchRouterPage', + hittable: false, rect: { x: 0, y: 0, width: 402, height: 874 }, }, { @@ -735,6 +737,7 @@ test('buildSnapshotState suppresses structural iOS identifier-only nodes', () => parentIndex: 2, type: 'Other', identifier: 'ReportActionAvatars-SingleAvatar', + hittable: false, rect: { x: 20, y: 252, width: 40, height: 40 }, }, ]; diff --git a/src/daemon/__tests__/snapshot-publication-membership.test.ts b/src/daemon/__tests__/snapshot-publication-membership.test.ts index 6d523ee32f..fafc415385 100644 --- a/src/daemon/__tests__/snapshot-publication-membership.test.ts +++ b/src/daemon/__tests__/snapshot-publication-membership.test.ts @@ -46,6 +46,7 @@ const screen: RawSnapshotNode[] = [ parentIndex: 0, type: 'Other', identifier: 'promo-banner', + hittable: false, rect: { x: 110, y: 130, width: 180, height: 44 }, }, // No label, identifier, or value: survival proves the interactive branch, not content. diff --git a/src/daemon/adapters/maestro/__tests__/daemon-runtime-port-identifier-hook.test.ts b/src/daemon/adapters/maestro/__tests__/daemon-runtime-port-identifier-hook.test.ts new file mode 100644 index 0000000000..01d2eeaf21 --- /dev/null +++ b/src/daemon/adapters/maestro/__tests__/daemon-runtime-port-identifier-hook.test.ts @@ -0,0 +1,73 @@ +import { expect, test } from 'vitest'; +import { executeMaestroFlow, inspectMaestroFlow } from '@agent-device/maestro'; +import type { SnapshotNode } from '@agent-device/kernel/snapshot'; +import { noMaestroIncludeSources } from '../../../../__tests__/test-utils/replay-script-source.ts'; +import { createDaemonMaestroRuntimePort } from '../daemon-runtime-port.ts'; +import { makeBaseRequest, makeDependencies, makeSnapshot } from './daemon-runtime-port-fixtures.ts'; + +const ON_SCREEN = { x: 18, y: 62, width: 366, height: 144 }; +const BELOW_VIEWPORT = { x: 18, y: 2000, width: 366, height: 144 }; + +// React Native keeps a `testID` wrapper's text beside it instead of inside it, so the wrapper +// reaches the fold childless with nothing to delegate its identifier to. Whether that is fatal then +// depends on the wrapper's own declared hittability: the Apple runner answers for every node and +// the Simulator AX bridge answers for none, and #2638 is the flow that passed on one producer and +// failed on the other. +function hookCapture( + rect: typeof ON_SCREEN, + hittable: boolean | undefined, +): Array> { + return [ + { index: 0, type: 'Application', rect: { x: 0, y: 0, width: 402, height: 874 } }, + { + index: 1, + parentIndex: 0, + type: 'Other', + identifier: 'members.count', + rect, + ...(hittable === undefined ? {} : { hittable }), + }, + ]; +} + +async function replay( + assertion: 'assertVisible' | 'assertNotVisible', + hittable: boolean | undefined, + rect: typeof ON_SCREEN, +) { + const port = createDaemonMaestroRuntimePort({ + baseReq: makeBaseRequest({ flags: { platform: 'ios', replayBackend: 'maestro' } }), + invoke: async (request) => + request.command === 'snapshot' + ? { ok: true, data: makeSnapshot(hookCapture(rect, hittable)) } + : { ok: true, data: {} }, + dependencies: makeDependencies(), + platform: 'ios', + }); + const flow = inspectMaestroFlow( + ['appId: com.example.app', '---', `- ${assertion}:`, ' id: members.count'].join('\n'), + '/flows/identifier-hook.yaml', + ); + return await executeMaestroFlow(flow, port, { readSource: noMaestroIncludeSources }); +} + +test('resolves an on-screen identifier hook whose capture reported no hittability', async () => { + expect(await replay('assertVisible', undefined, ON_SCREEN)).toMatchObject({ + ok: true, + replayed: 1, + }); +}); + +test('keeps a declared non-hittable identifier hook out of the visible set', async () => { + expect(await replay('assertNotVisible', false, ON_SCREEN)).toMatchObject({ + ok: true, + replayed: 1, + }); +}); + +test('does not report a kept identifier hook as visible past the viewport', async () => { + expect(await replay('assertNotVisible', undefined, BELOW_VIEWPORT)).toMatchObject({ + ok: true, + replayed: 1, + }); +});