From 35d1a35f77087f0412b644d49df073d9d45bbe4f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Tue, 15 Sep 2026 23:26:09 +0200 Subject: [PATCH] fix(ios): keep an identifier hook the fold cannot prove inert Suppressing a childless structural identifier wrapper deletes its identifier from every canonical view, and the interactive fold did it on a missing fact: the Apple runner declares hittable:false for those wrappers, the Simulator AX bridge declares nothing, and the same testID hook resolved on one producer and vanished on the other. Suppression now needs the node's own declared verdict. Refs #2638 --- .../adr/0004-ios-snapshot-backend-strategy.md | 9 +++ .../src/ios-snapshot-engine/mapping.test.ts | 1 + .../ios-snapshot-engine/noise-structural.ts | 9 ++- .../src/ios-snapshot-engine/noise.test.ts | 33 +++++++++ .../ios-snapshot-engine/presentation.test.ts | 3 + .../snapshot-publication-membership.test.ts | 1 + ...aemon-runtime-port-identifier-hook.test.ts | 73 +++++++++++++++++++ 7 files changed, 128 insertions(+), 1 deletion(-) create mode 100644 src/daemon/adapters/maestro/__tests__/daemon-runtime-port-identifier-hook.test.ts 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, + }); +});