Skip to content

Commit 50f460c

Browse files
authored
refactor(snapshot): establish presentation ownership boundary (#2005)
* refactor(snapshot): establish presentation ownership boundary * docs: keep context glossary within budget * fix(snapshot): address presentation boundary review * test(snapshot): ratchet eager closure budgets
1 parent 4a3ccf6 commit 50f460c

43 files changed

Lines changed: 603 additions & 432 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

CONTEXT.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -196,6 +196,11 @@ alone.
196196
**Presentation options**:
197197
The policy input controlling how one snapshot acquisition becomes a public projection.
198198

199+
**Snapshot presentation facet**:
200+
The host-side owner of the neutral acquisition-to-presentation contract and TypeScript policies.
201+
Platform acquisition supplies raw facts and a fold policy; runner-side Swift presentation remains
202+
separate across the process boundary.
203+
199204
**Capture hint**:
200205
The acquisition-facing view of a snapshot request, derived once from presentation options. It names
201206
the projection a backend must serve, keeps raw traversal depth separate from regular presented depth,

docs/adr/0004-ios-snapshot-backend-strategy.md

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,26 @@ The daemon should make degraded output observable. If an iOS interactive snapsho
6363
application root or another sparse shape, surface a structured quality verdict and warning so
6464
agents know the snapshot is degraded output rather than proof that the screen has no controls.
6565

66+
## Host-side ownership boundary
67+
68+
The shared TypeScript side has one snapshot-presentation facet. The neutral acquisition-to-presented
69+
carrier and clip-fold geometry contract live in `@agent-device/contracts/snapshot-presentation`; the
70+
host-side iOS post-wire policies and shared tree helpers live under `src/snapshot/snapshot-presentation/`.
71+
Platform-specific presentation adapters retain only the policy mechanics that cannot yet cross their
72+
runtime boundary. Daemon assembly owns only the ordering of capture, compaction, occlusion, and ref
73+
publication. It does not own the presentation vocabulary or a second geometry carrier.
74+
75+
Android acquisition remains in its platform module and adapts its raw hierarchy to the shared
76+
carrier. Swift keeps its runner-side `SnapshotPresentation` implementation because it consumes the
77+
capture-plan tier before the process boundary. The contract fixture under
78+
`contracts/fixtures/snapshot-presentation-conformance.json` is the shared proof between those
79+
runtimes; it does not imply that Swift and TypeScript share an implementation.
80+
81+
This is the first ownership slice of the Wave 4 debt tracked by #1983. Freshness recovery,
82+
timeout evidence, and screenshot-overlay policy retain their existing daemon adapters until their
83+
neutral host seams are extracted; new consumers must use the facet rather than add another daemon
84+
presentation path.
85+
6686
## Regression Notes
6787

6888
PR #639 made XCTest AX serialization failures explicit instead of swallowing them as empty

fallow-baselines/health.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -581,7 +581,7 @@
581581
"src/daemon/session-routing.ts:high impact",
582582
"src/daemon/handlers/session-state.ts:complexity",
583583
"src/commands/cli-grammar/common.ts:high impact",
584-
"src/daemon/snapshot-presentation/tree.ts:high impact",
584+
"src/snapshot/snapshot-presentation/tree.ts:high impact",
585585
"src/utils/success-text.ts:high impact",
586586
"src/cli.ts:complexity",
587587
"src/utils/timeouts.ts:high impact",

packages/contracts/package.json

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,9 @@
77
"dependencies": {
88
"@agent-device/kernel": "workspace:*"
99
},
10+
"devDependencies": {
11+
"fast-check": "^4.9.0"
12+
},
1013
"exports": {
1114
"./alert-contract": {
1215
"types": "./src/alert-contract.ts",
@@ -252,6 +255,10 @@
252255
"types": "./src/facades/replay.ts",
253256
"default": "./src/facades/replay.ts"
254257
},
258+
"./react-native-overlay": {
259+
"types": "./src/react-native-overlay.ts",
260+
"default": "./src/react-native-overlay.ts"
261+
},
255262
"./screen-recording-runtime": {
256263
"types": "./src/screen-recording-runtime.ts",
257264
"default": "./src/screen-recording-runtime.ts"
@@ -292,6 +299,10 @@
292299
"types": "./src/facades/snapshot.ts",
293300
"default": "./src/facades/snapshot.ts"
294301
},
302+
"./snapshot-presentation": {
303+
"types": "./src/snapshot-presentation.ts",
304+
"default": "./src/snapshot-presentation.ts"
305+
},
295306
"./snapshot-runtime": {
296307
"types": "./src/snapshot-runtime.ts",
297308
"default": "./src/snapshot-runtime.ts"
Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
import type { RawSnapshotNode } from '@agent-device/kernel/snapshot';
2+
3+
export type ReactNativeOverlayNode = Pick<
4+
RawSnapshotNode,
5+
'index' | 'type' | 'role' | 'subrole' | 'label' | 'value' | 'identifier' | 'rect' | 'hittable'
6+
>;
7+
8+
const COLLAPSED_WARNING_PREFIX_PATTERNS = [
9+
/^!,\s+/,
10+
/^(warn|warning|error):\s+/,
11+
/\b(?:possible\s+)?unhandled (?:promise )?rejection\b/,
12+
] as const;
13+
const COLLAPSED_WARNING_TEXT_MARKERS = [
14+
'open debugger to view warnings',
15+
'getsnapshot should be cached to avoid an infinite loop',
16+
'unique "key" prop',
17+
"unique 'key' prop",
18+
'virtualizedlists should never be nested',
19+
'failed prop type',
20+
] as const;
21+
22+
export function isReactNativeCollapsedWarningWrapperCandidate(
23+
node: ReactNativeOverlayNode,
24+
): boolean {
25+
return (
26+
isReactNativeCollapsedWarningLabel(node.label?.trim()) && isFullScreenOverlayRect(node.rect)
27+
);
28+
}
29+
30+
export function isReactNativeCollapsedWarningWrapperWithVisibleBanner(
31+
node: ReactNativeOverlayNode,
32+
descendants: ReactNativeOverlayNode[],
33+
): boolean {
34+
const nodeLabel = node.label?.trim();
35+
if (!nodeLabel || !isReactNativeCollapsedWarningWrapperCandidate(node)) return false;
36+
return descendants.some(
37+
(descendant) =>
38+
descendant.label?.trim() === nodeLabel && isReactNativeCollapsedWarningBanner(descendant),
39+
);
40+
}
41+
42+
export function isReactNativeCollapsedWarningLabel(rawLabel: string | undefined): boolean {
43+
const label = rawLabel?.trim().toLowerCase();
44+
if (!label) return false;
45+
return (
46+
COLLAPSED_WARNING_TEXT_MARKERS.some((marker) => label.includes(marker)) ||
47+
COLLAPSED_WARNING_PREFIX_PATTERNS.some((pattern) => pattern.test(label))
48+
);
49+
}
50+
51+
export function isReactNativeOverlayDismissLabel(label: string): boolean {
52+
return /^dismiss(?:\s*\([^)]*\))?$/i.test(label);
53+
}
54+
55+
export function isReactNativeOverlayMinimizeLabel(label: string): boolean {
56+
return /^minimi[sz]e(?:\b|\s|\()/i.test(label);
57+
}
58+
59+
export function isReactNativeCollapsedWarningBanner(node: ReactNativeOverlayNode): boolean {
60+
if (!node.rect) return false;
61+
return node.rect.width >= 120 && node.rect.height >= 36 && node.rect.height <= 180;
62+
}
63+
64+
function isFullScreenOverlayRect(rect: RawSnapshotNode['rect']): boolean {
65+
if (!rect) return false;
66+
return rect.x <= 1 && rect.y <= 1 && rect.width >= 300 && rect.height >= 600;
67+
}
Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,98 @@
1+
import { expect, test } from 'vitest';
2+
import type { RawSnapshotNode } from '@agent-device/kernel/snapshot';
3+
import fc from 'fast-check';
4+
import {
5+
createSnapshotPresentationNode,
6+
foldSnapshotRect,
7+
serializeRegularSnapshotPresentationNode,
8+
} from './snapshot-presentation.ts';
9+
10+
const rawNode: RawSnapshotNode = {
11+
index: 7,
12+
type: 'Button',
13+
label: 'Save',
14+
rect: { x: 0, y: 0, width: 100, height: 80 },
15+
hittable: true,
16+
};
17+
18+
const rectArb = fc.record({
19+
x: fc.integer({ min: -500, max: 500 }),
20+
y: fc.integer({ min: -500, max: 500 }),
21+
width: fc.integer({ min: -20, max: 500 }),
22+
height: fc.integer({ min: -20, max: 500 }),
23+
});
24+
25+
const positiveRectArb = fc.record({
26+
x: fc.integer({ min: -500, max: 500 }),
27+
y: fc.integer({ min: -500, max: 500 }),
28+
width: fc.integer({ min: 1, max: 500 }),
29+
height: fc.integer({ min: 1, max: 500 }),
30+
});
31+
32+
test('the shared fold carries both viewport and ancestor clipping into effective geometry', () => {
33+
expect(
34+
foldSnapshotRect(
35+
rawNode.rect,
36+
{ x: 20, y: 10, width: 50, height: 50 },
37+
{ x: 30, y: 20, width: 100, height: 20 },
38+
),
39+
).toEqual({ x: 30, y: 20, width: 40, height: 20 });
40+
});
41+
42+
test('regular serialization publishes effective geometry and fails closed on degenerate clips', () => {
43+
const presented = createSnapshotPresentationNode(rawNode, {
44+
x: 30,
45+
y: 20,
46+
width: 40,
47+
height: 20,
48+
});
49+
expect(serializeRegularSnapshotPresentationNode(presented)).toEqual({
50+
...rawNode,
51+
rect: { x: 30, y: 20, width: 40, height: 20 },
52+
hittable: true,
53+
});
54+
55+
expect(
56+
serializeRegularSnapshotPresentationNode(
57+
createSnapshotPresentationNode(rawNode, { x: 30, y: 20, width: 0, height: 20 }),
58+
),
59+
).toEqual({
60+
...rawNode,
61+
rect: { x: 30, y: 20, width: 0, height: 20 },
62+
hittable: undefined,
63+
});
64+
});
65+
66+
test('property: effective geometry stays inside every positive clip and never upgrades actionability', () => {
67+
fc.assert(
68+
fc.property(rectArb, positiveRectArb, positiveRectArb, (reported, viewport, ancestorClip) => {
69+
const effective = foldSnapshotRect(reported, viewport, ancestorClip);
70+
const raw = { index: 0, depth: 0, rect: reported, hittable: true as const };
71+
const presented = createSnapshotPresentationNode(raw, effective);
72+
const regular = serializeRegularSnapshotPresentationNode(presented);
73+
74+
expect(presented.raw.rect).toEqual(reported);
75+
expect(regular.rect).toEqual(effective);
76+
if (effective && effective.width > 0 && effective.height > 0) {
77+
expect(rectContains(viewport, effective)).toBe(true);
78+
expect(rectContains(ancestorClip, effective)).toBe(true);
79+
expect(regular.hittable).toBe(true);
80+
} else {
81+
expect(regular.hittable).toBeUndefined();
82+
}
83+
}),
84+
{ numRuns: 100 },
85+
);
86+
});
87+
88+
function rectContains(
89+
outer: { x: number; y: number; width: number; height: number },
90+
inner: { x: number; y: number; width: number; height: number },
91+
): boolean {
92+
return (
93+
inner.x >= outer.x &&
94+
inner.y >= outer.y &&
95+
inner.x + inner.width <= outer.x + Math.max(0, outer.width) &&
96+
inner.y + inner.height <= outer.y + Math.max(0, outer.height)
97+
);
98+
}
Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
import type { RawSnapshotNode, Rect } from '@agent-device/kernel/snapshot';
2+
import { isPositiveFiniteRect } from '@agent-device/kernel/rect';
3+
4+
/**
5+
* The typed carrier between acquisition facts and a regular snapshot projection.
6+
*
7+
* `raw.rect` remains the backend-reported frame for raw output and traversal. A regular
8+
* projection must publish `effectiveRect`, after the owning presentation policy has folded its
9+
* viewport and ancestor clips. Keeping both values in one contract prevents an adapter from
10+
* accidentally publishing acquisition geometry as presented geometry (#1797, #1983).
11+
*/
12+
export type SnapshotPresentationNode = {
13+
raw: RawSnapshotNode;
14+
effectiveRect?: Rect;
15+
};
16+
17+
export function createSnapshotPresentationNode(
18+
raw: RawSnapshotNode,
19+
effectiveRect?: Rect,
20+
): SnapshotPresentationNode {
21+
return {
22+
raw,
23+
...(effectiveRect ? { effectiveRect } : {}),
24+
};
25+
}
26+
27+
/**
28+
* Folds a reported frame through the viewport and the nearest effective ancestor clip.
29+
* Empty intersections preserve the reported origin and become zero-area frames, so diagnostics
30+
* retain useful coordinates while regular actionability can fail closed.
31+
*/
32+
export function foldSnapshotRect(
33+
reportedRect: Rect | undefined,
34+
viewport: Rect | undefined,
35+
ancestorClip: Rect | undefined,
36+
): Rect | undefined {
37+
if (!reportedRect) return undefined;
38+
let effective = normalizeRect(reportedRect);
39+
if (viewport) effective = intersectRect(effective, viewport);
40+
if (ancestorClip) effective = intersectRect(effective, ancestorClip);
41+
return effective;
42+
}
43+
44+
/** Serializes one regular node without allowing its reported frame to cross the presentation seam. */
45+
export function serializeRegularSnapshotPresentationNode(
46+
node: SnapshotPresentationNode,
47+
): RawSnapshotNode {
48+
return {
49+
...node.raw,
50+
...(node.effectiveRect ? { rect: node.effectiveRect } : { rect: undefined }),
51+
hittable:
52+
node.raw.hittable === true && isPositiveFiniteRect(node.effectiveRect) ? true : undefined,
53+
};
54+
}
55+
56+
function intersectRect(left: Rect, right: Rect): Rect {
57+
const x = Math.max(left.x, right.x);
58+
const y = Math.max(left.y, right.y);
59+
const maxX = Math.min(left.x + left.width, right.x + right.width);
60+
const maxY = Math.min(left.y + left.height, right.y + right.height);
61+
const hasIntersection = maxX > x && maxY > y;
62+
return {
63+
x: hasIntersection ? x : left.x,
64+
y: hasIntersection ? y : left.y,
65+
width: hasIntersection ? maxX - x : 0,
66+
height: hasIntersection ? maxY - y : 0,
67+
};
68+
}
69+
70+
function normalizeRect(rect: Rect): Rect {
71+
return {
72+
x: rect.x,
73+
y: rect.y,
74+
width: Math.max(0, rect.width),
75+
height: Math.max(0, rect.height),
76+
};
77+
}

pnpm-lock.yaml

Lines changed: 4 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

scripts/__tests__/test-file-size-ratchet.test.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,6 @@ const PINNED_TEST_FILE_LINES: Readonly<Record<string, number>> = Object.freeze({
5454
'src/daemon/handlers/__tests__/session-replay-divergence.test.ts': 1137,
5555
'src/platforms/apple/core/__tests__/apps.test.ts': 1210,
5656
'src/daemon/handlers/__tests__/session-replay-repair-transaction.test.ts': 1208,
57-
'src/daemon/snapshot-presentation/ios/presentation.test.ts': 1201,
5857
'src/daemon/handlers/__tests__/session-replay-target-verification-runtime.test.ts': 1183,
5958
'src/__tests__/client-metro.test.ts': 1105,
6059
'src/__tests__/cli-network.test.ts': 1092,

scripts/check-affected/device-lanes.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ test('a TypeScript-only Apple change selects the iOS and macOS lanes without a S
3232
),
3333
);
3434
}
35-
assert.deepEqual(lanes('src/daemon/snapshot-presentation/ios/action-shelf.ts'), [...IOS]);
35+
assert.deepEqual(lanes('src/snapshot/snapshot-presentation/ios/action-shelf.ts'), [...IOS]);
3636
assert.deepEqual(lanes('test/integration/replays/macos/01-desktop.ad'), ['replay-macos']);
3737
});
3838

0 commit comments

Comments
 (0)