Skip to content

Commit 2bd1533

Browse files
committed
refactor(contracts): one viewport-root predicate for the whole repo
"Is this the Application/Window root" was written nine times: three spellings normalizing `type|role|subrole`, five lowercasing `type` alone, and one comparing the normalized type for EQUALITY. Two of the nine sat in `contracts/snapshot-visibility.ts` itself, disagreeing with each other. Measured before collapsing, using #1592's method — ground the comparison in what each backend ACTUALLY emits, not in fixture strings. Over the 31 names iOS's `elementTypeName` can return, the 18 fully-qualified class names Android emits, and the 24 mapped/raw forms the macOS helper produces, the nine agreed on 71 of 73. The two exceptions are macOS window subroles, and the only spelling that disagreed is maestro's `===`, whose platform union is `android | ios` — so it can never see them. The duplication was textual, not behavioral, which is what made the collapse safe. `isViewportRootNode` reads role and subrole because the macOS helper is the only backend populating them and the only one able to emit a window whose `type` does not say so: `normalizedSnapshotType` returns the raw subrole for a non-standard window, so an `AXWindow` with subrole `AXSystemDialog` or `AXUnknown` reads as neither from `type` alone. Those two shapes are the whole behavioral delta of this change, at the six call sites that were type-only, and they are windows by role. `snapshot-viewport-root.test.ts` pins the predicate over those three emitted vocabularies. Red evidence: reverting the canonical definition to the type-only spelling fails 2 of 5 cells, to the equality spelling 4 of 5. Also drops two kernel re-declarations this made visible: maestro's local `containsPoint` and `rectsOverlap` were character-identical to `@agent-device/kernel/rect`'s `containsPoint` and `isRectVisibleInViewport`, in a file that already imports from that module. And `resolveViewportRect` loses three `as Rect` casts that only existed because `.filter()` cannot narrow `node.rect` — one `flatMap` states the same thing honestly. Deliberately NOT in this change: the three viewport RESOLVERS still diverge, and on Android that is a live defect rather than duplication. Filed separately with the measurement.
1 parent d8b309c commit 2bd1533

10 files changed

Lines changed: 64 additions & 93 deletions

File tree

packages/contracts/src/facades/snapshot.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ export {
55
isNodeVisibleInEffectiveViewport,
66
isNodeVisibleOnScreen,
77
isUsefulVisibilityAnchor,
8+
isViewportRootNode,
89
isTapPointInsideViewport,
910
resolveEffectiveViewportRect,
1011
resolveViewportRect,

packages/contracts/src/scroll-gesture.ts

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { AppError } from '@agent-device/kernel/errors';
22
import { defineStringEnum } from './string-enum.ts';
3+
import { isViewportRootNode } from './snapshot-visibility.ts';
34
import type { Rect, SnapshotNode } from '@agent-device/kernel/snapshot';
45

56
// What a caller may ASK for, as opposed to `ScrollDirection` (what the gesture resolves to):
@@ -273,7 +274,7 @@ export function parseScrollDirection(direction: string): ScrollDirection {
273274

274275
function inferViewportRect(nodes: Array<Pick<SnapshotNode, 'type' | 'rect'>>): Rect | undefined {
275276
const candidate = nodes
276-
.filter((node) => isViewportNode(node.type) && isValidRect(node.rect))
277+
.filter((node) => isViewportRootNode(node) && isValidRect(node.rect))
277278
.map((node) => node.rect)
278279
.sort(
279280
(left, right) =>
@@ -290,12 +291,6 @@ function inferViewportRect(nodes: Array<Pick<SnapshotNode, 'type' | 'rect'>>): R
290291
return { x: 0, y: 0, width, height };
291292
}
292293

293-
function isViewportNode(type: string | undefined): boolean {
294-
if (!type) return false;
295-
const normalized = type.toLowerCase();
296-
return normalized.includes('application') || normalized.includes('window');
297-
}
298-
299294
function isValidRect(rect: Rect | undefined): rect is Rect {
300295
return !!rect && rect.width > 0 && rect.height > 0;
301296
}

packages/contracts/src/snapshot-visibility.ts

Lines changed: 36 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -15,37 +15,52 @@ type SnapshotVisibilityNode = Pick<
1515
'rect' | 'index' | 'parentIndex' | 'type' | 'role' | 'subrole'
1616
>;
1717

18+
/**
19+
* The application/window root: the node a target rect is measured against, and
20+
* the node whose own rect is invariant under any gesture.
21+
*
22+
* One definition for the whole repo. It reads `type`, `role` AND `subrole`
23+
* because the macOS helper is the only backend that populates the latter two,
24+
* and it is the only backend that can emit a window whose `type` does not say
25+
* so — `normalizedSnapshotType` returns the raw subrole for a non-standard
26+
* window, so an `AXWindow` with subrole `AXSystemDialog` or `AXUnknown` reads
27+
* as neither from `type` alone while `role` names it exactly.
28+
*
29+
* Substring, not equality: macOS emits unmapped roles with their `AX` prefix
30+
* intact and subroles like `AXFloatingWindow` that are windows by any reading.
31+
* iOS emits a closed set of 31 short names in which only `Application` and
32+
* `Window` contain either word, so substring and equality agree there. Android
33+
* emits fully-qualified Java class names and no root node at all, so no
34+
* spelling of this predicate matches anything on Android — see
35+
* `resolveViewportRect`'s third fallback, which is what Android actually uses.
36+
*/
37+
export function isViewportRootNode(node: Pick<SnapshotNode, 'type' | 'role' | 'subrole'>): boolean {
38+
const kind = [node.type, node.role, node.subrole]
39+
.map((value) => normalizeType(value ?? ''))
40+
.join(' ');
41+
return kind.includes('application') || kind.includes('window');
42+
}
43+
1844
/**
1945
* The root viewport a target rect is measured against: the largest
2046
* Application/Window rect containing the target's center, falling back to the
2147
* largest such rect, then to the largest containing rect of any node.
2248
*/
2349
export function resolveViewportRect(nodes: RawSnapshotNode[], targetRect: Rect): Rect | null {
2450
const targetCenter = centerOfRect(targetRect);
25-
const rectNodes = nodes.filter((node) => hasValidRect(node.rect));
26-
const viewportNodes = rectNodes.filter((node) => {
27-
const type = (node.type ?? '').toLowerCase();
28-
return type.includes('application') || type.includes('window');
29-
});
30-
31-
const containingViewport = pickLargestRect(
32-
viewportNodes
33-
.map((node) => node.rect as Rect)
34-
.filter((rect) => containsPoint(rect, targetCenter.x, targetCenter.y)),
51+
const rects = nodes.flatMap((node) =>
52+
hasValidRect(node.rect) ? [{ node, rect: node.rect }] : [],
3553
);
36-
if (containingViewport) return containingViewport;
37-
38-
const viewportFallback = pickLargestRect(viewportNodes.map((node) => node.rect as Rect));
39-
if (viewportFallback) return viewportFallback;
54+
const viewportRects = rects
55+
.filter((entry) => isViewportRootNode(entry.node))
56+
.map((entry) => entry.rect);
57+
const contains = (rect: Rect) => containsPoint(rect, targetCenter.x, targetCenter.y);
4058

41-
const genericContaining = pickLargestRect(
42-
rectNodes
43-
.map((node) => node.rect as Rect)
44-
.filter((rect) => containsPoint(rect, targetCenter.x, targetCenter.y)),
59+
return (
60+
pickLargestRect(viewportRects.filter(contains)) ??
61+
pickLargestRect(viewportRects) ??
62+
pickLargestRect(rects.map((entry) => entry.rect).filter(contains))
4563
);
46-
if (genericContaining) return genericContaining;
47-
48-
return null;
4964
}
5065

5166
function hasValidRect(rect: Rect | undefined): rect is Rect {

packages/maestro/src/internal/runtime-port-geometry.ts

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import { pointInsideRect } from './shared.ts';
66
import {
77
findNearestScrollableAncestor,
88
isScrollableNodeLike,
9-
normalizeType,
9+
isViewportRootNode,
1010
} from '@agent-device/contracts/snapshot';
1111
import { MAESTRO_COMPATIBILITY_PRESETS } from './compatibility-policy.ts';
1212
import { resolveNumeric } from './engine-flow.ts';
@@ -108,8 +108,7 @@ function findScrollContainer(
108108
function findLargestViewportRect(nodes: SnapshotState['nodes']): Rect | undefined {
109109
return nodes
110110
.filter((node) => {
111-
const type = normalizeType(node.type ?? '');
112-
return isPositiveFiniteRect(node.rect) && (type === 'application' || type === 'window');
111+
return isPositiveFiniteRect(node.rect) && isViewportRootNode(node);
113112
})
114113
.sort(
115114
(left, right) =>

packages/maestro/src/internal/snapshot-policy.ts

Lines changed: 8 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,13 @@ import {
33
findNearestScrollableAncestor,
44
findSnapshotAncestor,
55
isUsefulVisibilityAnchor,
6+
isViewportRootNode,
67
} from '@agent-device/contracts/snapshot';
7-
import { isPositiveFiniteRect } from '@agent-device/kernel/rect';
8+
import {
9+
containsPoint,
10+
isPositiveFiniteRect,
11+
isRectVisibleInViewport,
12+
} from '@agent-device/kernel/rect';
813
import type { Rect, SnapshotNode } from '@agent-device/kernel/snapshot';
914

1015
export function isMaestroNodeVisible(
@@ -47,15 +52,12 @@ function isVisibleInEffectiveViewport(node: SnapshotNode, nodes: SnapshotNode[])
4752
const viewport =
4853
findNearestScrollableAncestor(node, byIndex, (ancestor) => Boolean(ancestor.rect))?.rect ??
4954
resolveRootViewport(nodes, node.rect);
50-
return viewport ? rectsOverlap(node.rect, viewport) : true;
55+
return viewport ? isRectVisibleInViewport(node.rect, viewport) : true;
5156
}
5257

5358
function resolveRootViewport(nodes: SnapshotNode[], target: Rect): Rect | null {
5459
const viewportRects = nodes
55-
.filter((node) => {
56-
const type = (node.type ?? '').toLowerCase();
57-
return node.rect && (type.includes('application') || type.includes('window'));
58-
})
60+
.filter((node) => node.rect && isViewportRootNode(node))
5961
.map((node) => node.rect!)
6062
.sort((left, right) => right.width * right.height - left.width * left.height);
6163
const centerX = target.x + target.width / 2;
@@ -64,14 +66,3 @@ function resolveRootViewport(nodes: SnapshotNode[], target: Rect): Rect | null {
6466
viewportRects.find((rect) => containsPoint(rect, centerX, centerY)) ?? viewportRects[0] ?? null
6567
);
6668
}
67-
68-
function rectsOverlap(left: Rect, right: Rect): boolean {
69-
return (
70-
Math.max(left.x, right.x) <= Math.min(left.x + left.width, right.x + right.width) &&
71-
Math.max(left.y, right.y) <= Math.min(left.y + left.height, right.y + right.height)
72-
);
73-
}
74-
75-
function containsPoint(rect: Rect, x: number, y: number): boolean {
76-
return x >= rect.x && x <= rect.x + rect.width && y >= rect.y && y <= rect.y + rect.height;
77-
}

src/core/interaction-targeting.ts

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import type { Rect, SnapshotNode } from '@agent-device/kernel/snapshot';
22
import { centerOfRect } from '@agent-device/kernel/snapshot';
33
import { containsPoint, pickLargestRect } from '@agent-device/kernel/rect';
4-
import { normalizeType } from '@agent-device/contracts/snapshot';
4+
import { normalizeType, isViewportRootNode } from '@agent-device/contracts/snapshot';
55
import { findNearestHittableAncestor } from '../snapshot/snapshot-processing.ts';
66
import { isSnapshotNodeInteractionBlocked } from '../snapshot/snapshot-occlusion.ts';
77
import {
@@ -158,10 +158,7 @@ function isScrollingContainer(node: SnapshotNode): boolean {
158158
function resolveRootViewportRect(nodes: SnapshotNode[], targetRect: Rect): Rect | null {
159159
const targetCenter = centerOfRect(targetRect);
160160
const viewportRects = nodes
161-
.filter((node) => {
162-
const type = (node.type ?? '').toLowerCase();
163-
return type.includes('application') || type.includes('window');
164-
})
161+
.filter(isViewportRootNode)
165162
.map((node) => normalizeRect(node.rect))
166163
.filter((rect): rect is Rect => rect !== null);
167164
if (viewportRects.length === 0) return null;

src/daemon/handlers/find.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { dispatchCommand } from '../../core/dispatch.ts';
2+
import { isViewportRootNode } from '@agent-device/contracts/snapshot';
23
import {
34
findBestMatchesByLocator,
45
isReadOnlyFindAction,
@@ -371,8 +372,7 @@ function isRootInteractionContainer(
371372
root: SnapshotState['nodes'][number] | undefined,
372373
): boolean {
373374
if (!root?.rect || !node.rect) return false;
374-
const type = node.type?.toLowerCase() ?? '';
375-
if (!type.includes('application') && !type.includes('window')) return false;
375+
if (!isViewportRootNode(node)) return false;
376376
return rectsMatch(node.rect, root.rect);
377377
}
378378

src/daemon/interaction-outcome-policy.ts

Lines changed: 2 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import { isMobilePlatform } from '@agent-device/kernel/device';
33
import type { SnapshotNode, SnapshotState } from '@agent-device/kernel/snapshot';
44
import { collectKeyboardChromeRefs } from '../core/snapshot-chrome.ts';
55
import { emitDiagnostic } from '../utils/diagnostics.ts';
6-
import { normalizeType } from '@agent-device/contracts/snapshot';
6+
import { isViewportRootNode } from '@agent-device/contracts/snapshot';
77
import { contextFromFlags } from './context.ts';
88
import type { SessionState } from './types.ts';
99

@@ -390,22 +390,7 @@ function isNonDiscriminatingSurfaceNode(
390390
node: SnapshotNode,
391391
keyboardChromeRefs: ReadonlySet<string>,
392392
): boolean {
393-
return isViewportRootKind(node) || (node.ref !== undefined && keyboardChromeRefs.has(node.ref));
394-
}
395-
396-
/**
397-
* Minimal local equivalent of `isViewportRoot` in
398-
* `src/snapshot/snapshot-occlusion.ts` (source of truth) — that function is
399-
* module-private and keyed off the broader `RawSnapshotNode` shape used by
400-
* occlusion/viewport resolution, so it is reimplemented here rather than
401-
* exported solely for this caller. Same normalized-kind substring test; keep
402-
* the two in lockstep if the underlying AX vocabulary changes.
403-
*/
404-
function isViewportRootKind(node: Pick<SnapshotNode, 'type' | 'role' | 'subrole'>): boolean {
405-
const normalizedKind = [node.type, node.role, node.subrole]
406-
.map((value) => normalizeType(value ?? ''))
407-
.join(' ');
408-
return normalizedKind.includes('application') || normalizedKind.includes('window');
393+
return isViewportRootNode(node) || (node.ref !== undefined && keyboardChromeRefs.has(node.ref));
409394
}
410395

411396
function interactionSurfaceSemanticKey(node: SnapshotNode): string | undefined {

src/daemon/screenshot-overlay.ts

Lines changed: 5 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import {
99
import type { PNG } from '../utils/png.ts';
1010
import { decodePngAsync, encodePngAsync } from '../utils/png-worker-client.ts';
1111
import { analyzeReactNativeOverlay } from '../core/react-native-overlay.ts';
12-
import { normalizeType } from '@agent-device/contracts/snapshot';
12+
import { isViewportRootNode, normalizeType } from '@agent-device/contracts/snapshot';
1313
import { findNearestAncestor } from '../snapshot/snapshot-processing.ts';
1414
import { resolveAndroidOverlaySourceRect } from './screenshot-overlay-android.ts';
1515
import { hasPositiveRect, rectArea, rectContains } from './screenshot-overlay-rects.ts';
@@ -212,7 +212,7 @@ function isAndroidUnlabeledClickableSource(
212212
node: SnapshotNode,
213213
): boolean {
214214
if (snapshot.backend !== 'android') return false;
215-
if (!node.hittable || !hasPositiveRect(node.rect) || isViewportLikeNode(node)) return false;
215+
if (!node.hittable || !hasPositiveRect(node.rect) || isViewportRootNode(node)) return false;
216216
const normalizedType = normalizeType(node.type ?? '');
217217
if (ANDROID_UNLABELED_CLICKABLE_EXCLUDED_TYPES.some((type) => normalizedType.includes(type))) {
218218
return false;
@@ -318,7 +318,7 @@ function projectRectToScreenshot(
318318
function resolveSnapshotBounds(nodes: SnapshotState['nodes']): Rect | null {
319319
let viewport: Rect | null = null;
320320
for (const node of nodes) {
321-
if (!isViewportLikeNode(node) || !hasPositiveRect(node.rect)) continue;
321+
if (!isViewportRootNode(node) || !hasPositiveRect(node.rect)) continue;
322322
if (!viewport || rectArea(node.rect) > rectArea(viewport)) {
323323
viewport = node.rect;
324324
}
@@ -366,7 +366,7 @@ function hasActionableRole(node: SnapshotNode): boolean {
366366
}
367367

368368
function isOverlayActionableNode(node: SnapshotNode): boolean {
369-
return hasActionableRole(node) && !isViewportLikeNode(node);
369+
return hasActionableRole(node) && !isViewportRootNode(node);
370370
}
371371

372372
function isProxyOverlayNode(node: SnapshotNode): boolean {
@@ -379,15 +379,8 @@ function isProxyOverlayNode(node: SnapshotNode): boolean {
379379
);
380380
}
381381

382-
function isViewportLikeNode(node: Pick<SnapshotNode, 'type' | 'role' | 'subrole'>): boolean {
383-
const roleText = [node.type, node.role, node.subrole]
384-
.map((value) => normalizeType(value ?? ''))
385-
.join(' ');
386-
return roleText.includes('application') || roleText.includes('window');
387-
}
388-
389382
function isUsableOverlayTarget(node: SnapshotNode | null): node is SnapshotNode {
390-
return Boolean(node?.rect && hasPositiveRect(node.rect) && !isViewportLikeNode(node));
383+
return Boolean(node?.rect && hasPositiveRect(node.rect) && !isViewportRootNode(node));
391384
}
392385

393386
function isMeaningfulSignal(value: string | undefined): boolean {

src/snapshot/snapshot-occlusion.ts

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import type { RawSnapshotNode, Rect } from '@agent-device/kernel/snapshot';
22
import { centerOfRect } from '@agent-device/kernel/snapshot';
33
import { areRectsApproximatelyEqual, normalizeRect } from '../utils/rect-center.ts';
44
import { containsPoint } from '@agent-device/kernel/rect';
5-
import { normalizeType } from '@agent-device/contracts/snapshot';
5+
import { normalizeType, isViewportRootNode } from '@agent-device/contracts/snapshot';
66

77
const COVERED_PRESENTATION_HINT = 'covered';
88
const OVERLAY_KIND_FRAGMENTS = [
@@ -220,7 +220,7 @@ function isOverlayLikeNode(
220220
options: SnapshotOcclusionOptions,
221221
): boolean {
222222
if (!positiveRect(node.rect)) return false;
223-
if (isViewportRoot(node)) return false;
223+
if (isViewportRootNode(node)) return false;
224224
if (isFullViewportChromeContainer(node, byIndex)) return false;
225225
// This is a presentation-order heuristic: only known floating UI chrome should cover
226226
// later targets. Generic hittable containers can appear later without being visually on top.
@@ -241,7 +241,7 @@ function isFullViewportChromeContainer(
241241
let current = typeof node.parentIndex === 'number' ? byIndex.get(node.parentIndex) : undefined;
242242
const visited = new Set<number>();
243243
while (current && !visited.has(current.index)) {
244-
if (isViewportRoot(current)) {
244+
if (isViewportRootNode(current)) {
245245
const viewportRect = positiveRect(current.rect);
246246
return Boolean(viewportRect && areRectsApproximatelyEqual(rect, viewportRect));
247247
}
@@ -295,7 +295,7 @@ function isRenderableAdditionalOverlayNode(
295295
return (
296296
options.isAdditionalOverlayNode?.(node) === true &&
297297
positiveRect(node.rect) !== null &&
298-
!isViewportRoot(node)
298+
!isViewportRootNode(node)
299299
);
300300
}
301301

@@ -321,11 +321,6 @@ function normalizeNodeKind(node: Pick<RawSnapshotNode, 'type' | 'role' | 'subrol
321321
return [node.type, node.role, node.subrole].map((value) => normalizeType(value ?? '')).join(' ');
322322
}
323323

324-
function isViewportRoot(node: RawSnapshotNode): boolean {
325-
const normalized = normalizeNodeKind(node);
326-
return normalized.includes('application') || normalized.includes('window');
327-
}
328-
329324
function areRelatedSnapshotNodes(
330325
left: RawSnapshotNode,
331326
right: RawSnapshotNode,

0 commit comments

Comments
 (0)