diff --git a/packages/capture-kit/src/snapshot/__tests__/scroll-edge-state-pass-orchestration.test.ts b/packages/capture-kit/src/snapshot/__tests__/scroll-edge-state-pass-orchestration.test.ts index 112da4a2a4..712c3dac8d 100644 --- a/packages/capture-kit/src/snapshot/__tests__/scroll-edge-state-pass-orchestration.test.ts +++ b/packages/capture-kit/src/snapshot/__tests__/scroll-edge-state-pass-orchestration.test.ts @@ -77,6 +77,63 @@ test('an amount-based message names the honored travel when the planner reports ); }); +/** + * #2714. A travel figure describes the swipe that was dispatched, not content that moved, so an + * observation of no change outranks it. `unobserved` is the pair to these: the disclosure travels in + * the `movement` field, and the prose keeps the number the caller asked for. + */ +test('formatScrollEdgeMessage: an observed edge answers without a distance', () => { + assert.equal( + formatScrollEdgeMessage({ direction: 'down', passes: 1, amount: 0.75, movement: 'at-edge' }), + 'Scrolled down and no hidden content below was detected', + ); + assert.equal( + formatScrollEdgeMessage({ direction: 'up', passes: 1, amount: 0.75, movement: 'at-edge' }), + 'Scrolled up and no hidden content above was detected', + ); +}); + +test('formatScrollEdgeMessage: a horizontal scroll never claims a vertical edge', () => { + assert.equal( + formatScrollEdgeMessage({ direction: 'left', passes: 1, amount: 0.75, movement: 'at-edge' }), + 'Scrolled left by 0.75', + ); +}); + +test('formatScrollEdgeMessage: an unchanged surface reports the measurement, not the request', () => { + assert.equal( + formatScrollEdgeMessage({ direction: 'left', passes: 1, pixels: 500, movement: 'unchanged' }), + 'Scrolled left and the visible content did not change', + ); +}); + +test('formatScrollEdgeMessage: an observation outranks the distance it disagrees with', () => { + assert.equal( + formatScrollEdgeMessage({ + direction: 'down', + passes: 1, + amount: 3, + pixels: 5000, + honoredPixels: 640, + movement: 'at-edge', + }), + 'Scrolled down and no hidden content below was detected', + ); +}); + +test('formatScrollEdgeMessage: an unobserved movement keeps the distance the caller asked for', () => { + assert.equal( + formatScrollEdgeMessage({ + direction: 'down', + passes: 1, + amount: 0.75, + honoredPixels: 656, + movement: 'unobserved', + }), + 'Scrolled down by 0.75 of the viewport (656px)', + ); +}); + // --------------------------------------------------------------------------- // captureScrollEdgeState: retry-without-scope on an empty scoped capture // --------------------------------------------------------------------------- diff --git a/packages/capture-kit/src/snapshot/__tests__/scroll-edge-state-read.test.ts b/packages/capture-kit/src/snapshot/__tests__/scroll-edge-state-read.test.ts new file mode 100644 index 0000000000..e51a39a01d --- /dev/null +++ b/packages/capture-kit/src/snapshot/__tests__/scroll-edge-state-read.test.ts @@ -0,0 +1,79 @@ +import { test } from 'vitest'; +import assert from 'node:assert/strict'; +import { captureScrollEdgeState, readScrollEdgeState } from '../scroll-edge-state.ts'; +import { scrollNode, windowRoot } from './scroll-edge-state-fixtures.ts'; + +/** + * The pure door into the edge analyzer. `scroll ` reads an edge decision off a tree it already + * holds to say what its gesture did (#2714), which makes `containerRect` part of the answer: the + * caller has to be able to tell "this content ends here" from "the tree named no container", and + * `canScroll: false` alone cannot tell them apart. + */ + +const CONTAINER = { x: 18, y: 178, width: 366, height: 662 }; + +test('readScrollEdgeState: a resolved container reports the frame the decision was taken on', async () => { + const nodes = [ + windowRoot(), + scrollNode(1, { hiddenContentBelow: true, rect: CONTAINER }), + { + ref: 'e3', + index: 2, + parentIndex: 1, + type: 'StaticText', + label: 'Row', + rect: { x: 24, y: 700, width: 300, height: 40 }, + }, + ]; + + const state = await readScrollEdgeState(nodes, 'bottom'); + + assert.equal(state.canScroll, true); + assert.deepEqual(state.containerRect, CONTAINER); +}); + +test('readScrollEdgeState: a tree with no scroll container reports no frame to check a gesture against', async () => { + const nodes = [ + windowRoot(), + { + ref: 'e2', + index: 1, + parentIndex: 0, + type: 'Button', + label: 'Tap me', + rect: { x: 20, y: 40, width: 100, height: 40 }, + }, + ]; + + const state = await readScrollEdgeState(nodes, 'bottom'); + + assert.equal(state.canScroll, false); + assert.equal(state.containerRect, undefined); +}); + +test('readScrollEdgeState: a zero-area scrollable resolves to no container and therefore no frame', async () => { + const state = await readScrollEdgeState( + [windowRoot(), scrollNode(1, { hiddenContentBelow: true, rect: { ...CONTAINER, height: 0 } })], + 'bottom', + ); + + assert.equal(state.containerRect, undefined); +}); + +/** + * The reader must not become a second source of truth: the captured and pure doors disagreeing about + * one tree would mean an edge loop and a movement claim reading different rules. + */ +test('readScrollEdgeState answers what captureScrollEdgeState answers for the same tree', async () => { + const nodes = [windowRoot(), scrollNode(1, { hiddenContentBelow: true, rect: CONTAINER })]; + + const captured = await captureScrollEdgeState({ + edge: 'bottom', + captureNodes: async () => nodes, + }); + const read = await readScrollEdgeState(nodes, 'bottom'); + + assert.equal(read.canScroll, captured.canScroll); + assert.deepEqual(read.containerRect, captured.containerRect); + assert.equal(read.fingerprint, captured.fingerprint); +}); diff --git a/packages/capture-kit/src/snapshot/scroll-edge-state.ts b/packages/capture-kit/src/snapshot/scroll-edge-state.ts index d9690ad011..7359b2348d 100644 --- a/packages/capture-kit/src/snapshot/scroll-edge-state.ts +++ b/packages/capture-kit/src/snapshot/scroll-edge-state.ts @@ -1,13 +1,31 @@ import { AppError } from '@agent-device/kernel/errors'; +import type { ScrollMovementObservation } from '@agent-device/contracts/scroll-command'; import type { ScrollDirection } from '@agent-device/contracts/scroll-gesture'; -import type { Point, RawSnapshotNode, SnapshotNode } from '@agent-device/kernel/snapshot'; +import type { Point, RawSnapshotNode, Rect, SnapshotNode } from '@agent-device/kernel/snapshot'; export type ScrollEdge = 'top' | 'bottom'; +/** + * The end-of-content analyzer reads vertical edges only, so a horizontal scroll has no end signal + * and is bounded by whatever pass or observation budget its caller owns. + */ +export function verticalEdgeFor(direction: ScrollDirection): ScrollEdge | undefined { + if (direction === 'down') return 'bottom'; + if (direction === 'up') return 'top'; + return undefined; +} + export type ScrollEdgeState = { canScroll: boolean; emptySnapshot: boolean; scope?: string; + /** + * The frame of the container every other field on this record describes, absent when the tree + * named no usable scroll container at all. Without it `canScroll: false` cannot be read as + * "already at this edge": a tree that names no scroller and a scroller with nothing left to + * reveal answer the same way, and only one of them is the end of the content (#2714). + */ + containerRect?: Rect; /** * A cheap signature of what is on screen right now. Two consecutive captures with the same * fingerprint but `canScroll` still true mean the scroll did not move the container — the actuator @@ -55,6 +73,19 @@ export async function captureScrollEdgeState(params: { } } +/** + * Everything one tree can answer about this edge — hidden content left, the container that answer + * was taken on, and the surface signature — with no capture. The one pure door into the analyzer, + * so every reader of an edge decision goes through the same selection rules. + */ +export async function readScrollEdgeState( + nodes: readonly (RawSnapshotNode | SnapshotNode)[], + edge: ScrollEdge, +): Promise { + const { analyzeScrollEdgeState } = await import('./scroll-edge-state/selection.ts'); + return analyzeScrollEdgeState(nodes, edge); +} + /** * Is there hidden content left at this edge? The same question `runScrollEdgePasses` loops on, * exposed for callers with their own stop condition (`scroll --until`) so both read one signal. @@ -63,8 +94,7 @@ export async function canScrollFurtherAtEdge( nodes: readonly (RawSnapshotNode | SnapshotNode)[], edge: ScrollEdge, ): Promise { - const { analyzeScrollEdgeState } = await import('./scroll-edge-state/selection.ts'); - return analyzeScrollEdgeState(nodes, edge).canScroll; + return (await readScrollEdgeState(nodes, edge)).canScroll; } /** @@ -167,6 +197,10 @@ export async function runScrollEdgePasses(params: { * travel that was asked for: one gesture cannot cross more than the viewport axis minus its edge * padding, so a large `amount` saturates. Naming the honored distance is what keeps * `scroll down 3` from reporting a three-viewport scroll it never performed. + * + * `movement` is what the owner measured after the gesture (#2714), and it outranks the requested + * distance: a directional scroll that observed no change cannot answer with a travel figure, since + * that number describes the swipe that was dispatched, not content that moved. */ export function formatScrollEdgeMessage(params: { direction: ScrollDirection; @@ -175,12 +209,25 @@ export function formatScrollEdgeMessage(params: { amount?: number | undefined; pixels?: number | undefined; honoredPixels?: number | undefined; + movement?: ScrollMovementObservation | undefined; }): string { - const { direction, edge, passes, amount, pixels, honoredPixels } = params; + const { direction, edge, passes, amount, pixels, honoredPixels, movement } = params; if (edge && passes === 0) { return `Already at ${edge}; no hidden content ${edge === 'bottom' ? 'below' : 'above'} detected`; } if (edge) return `Scrolled to ${edge} with ${passes} ${direction} passes`; + const verticalEdge = verticalEdgeFor(direction); + if (movement === 'at-edge' && verticalEdge) { + // The same honesty the zero-pass branch above holds: the analyzer reports no hidden content + // DETECTED, which is not a claim that the content provably ends here. A horizontal scroll has + // no edge signal to report, so it falls through to the distance it was asked for. + return `Scrolled ${direction} and no hidden content ${ + verticalEdge === 'bottom' ? 'below' : 'above' + } was detected`; + } + if (movement === 'unchanged') { + return `Scrolled ${direction} and the visible content did not change`; + } if (pixels !== undefined) return `Scrolled ${direction} by ${honoredPixels ?? pixels}px`; if (amount !== undefined) { return honoredPixels === undefined @@ -199,13 +246,37 @@ function buildScrollEdgeNoProgressError(edge: ScrollEdge, passes: number): AppEr reason: 'scroll_edge_no_progress', edge, passes, - hint: - `The scroll is not reaching this container. If a field is focused, dismiss the keyboard first; if it is nested inside another scroller, target it directly with scroll --until . ` + - `Some lists ignore synthesized scrolls — a raw drag moves them: swipe x1 y1 x2 y2 started inside the list.`, + hint: scrollNoProgressHint({ targetDirectly: 'with scroll --until ' }), }, ); } +/** + * What a caller does next when a scroll moved nothing while the container still had content to + * reveal: one hint, because every loop that notices has the same recovery. + * + * `targetDirectly` names the form the failing command has for reaching an inner scroller — `scroll + * --until` already IS that form, so it says only "target it directly". The raw-drag sentence belongs + * only where the command can say where a swipe would land: a tvOS scroll is a remote keypress with no + * coordinates to name, and advice nobody can follow is noise. + */ +export function scrollNoProgressHint( + params: Readonly<{ targetDirectly?: string; rawDrag?: boolean }> = {}, +): string { + const target = + params.targetDirectly === undefined + ? 'target it directly' + : `target it directly ${params.targetDirectly}`; + const reached = + `The scroll is not reaching this container. If a field is focused, dismiss the keyboard first; ` + + `if it is nested inside another scroller, ${target}.`; + if (params.rawDrag === false) return reached; + return ( + reached + + ` Some lists ignore synthesized scrolls — a raw drag moves them: swipe x1 y1 x2 y2 started inside the list.` + ); +} + function buildScrollEdgeVerificationError( edge: ScrollEdge, scope: string | undefined, diff --git a/packages/capture-kit/src/snapshot/scroll-edge-state/selection.ts b/packages/capture-kit/src/snapshot/scroll-edge-state/selection.ts index a2f50658c8..48c9a2571d 100644 --- a/packages/capture-kit/src/snapshot/scroll-edge-state/selection.ts +++ b/packages/capture-kit/src/snapshot/scroll-edge-state/selection.ts @@ -38,6 +38,9 @@ export function analyzeScrollEdgeState( return { canScroll, emptySnapshot: false, + // Every selection path admits only `isUsableRect` containers, so a resolved container always + // has a frame to report and its absence always means no container was resolved. + ...(container.rect ? { containerRect: container.rect } : {}), scope: buildScrollContainerScope(container, nodes), fingerprint: buildSurfaceFingerprint(container, nodes), }; diff --git a/packages/contracts/src/scroll-command.test.ts b/packages/contracts/src/scroll-command.test.ts index ecd96072bc..12e701602e 100644 --- a/packages/contracts/src/scroll-command.test.ts +++ b/packages/contracts/src/scroll-command.test.ts @@ -1,6 +1,6 @@ import assert from 'node:assert/strict'; import { test } from 'vitest'; -import { resolveScrollExecutionOptions } from './scroll-command.ts'; +import { honoredScrollSwipeMidpoint, resolveScrollExecutionOptions } from './scroll-command.ts'; test('resolveScrollExecutionOptions keeps ordinary scrolls controlled', () => { assert.deepEqual(resolveScrollExecutionOptions({ amount: 0.5 }), { @@ -18,3 +18,21 @@ test('resolveScrollExecutionOptions marks edge scrolls inertial without changing releaseBehavior: 'inertial', }); }); + +/** + * The swipe midpoint is what lets a scroll ask whether its own gesture landed inside the container it + * blamed for a no-op, so the pair that pins it is a leaf that reported coordinates and one that did + * not: a tvOS scroll is a remote keypress, and guessing a midpoint there would invent evidence. + */ +test('honoredScrollSwipeMidpoint reports the middle of a swipe the leaf ran', () => { + assert.deepEqual(honoredScrollSwipeMidpoint({ x1: 201, y1: 665, x2: 201, y2: 209 }), { + x: 201, + y: 437, + }); +}); + +test('honoredScrollSwipeMidpoint declines to guess where the owner reported no gesture', () => { + assert.equal(honoredScrollSwipeMidpoint({ button: 'down' }), undefined); + assert.equal(honoredScrollSwipeMidpoint({ x1: 201, y1: 665, x2: 201 }), undefined); + assert.equal(honoredScrollSwipeMidpoint({ x1: 201, y1: 665, x2: 201, y2: 'none' }), undefined); +}); diff --git a/packages/contracts/src/scroll-command.ts b/packages/contracts/src/scroll-command.ts index 59b2146b1a..39de1e80cf 100644 --- a/packages/contracts/src/scroll-command.ts +++ b/packages/contracts/src/scroll-command.ts @@ -9,6 +9,24 @@ export const DEFAULT_IOS_SCROLL_AMOUNT = 0.65; export type ScrollReleaseBehavior = 'controlled' | 'inertial'; +/** + * What a directional scroll actually SAW after its gesture, which is the only evidence that can + * back the distance the same response reports (#2714). + * + * - `'moved'`: the post-gesture surface differs from the pre-gesture one, so content did move. + * - `'at-edge'`: the surface is identical and the resolved container names no hidden content in + * that direction — the scroll was a legitimate no-op at the end of the content. + * - `'unchanged'`: the surface is identical and the direction has no end-of-content signal to read + * (a horizontal scroll: the hidden-content analyzer only covers the vertical axis), so the + * response says what it measured without guessing which of the two it was. + * - `'unobserved'`: nothing comparable was available, so the distance rests on the gesture plan + * alone. Callers that need the effect confirmed ask for a capture or a `--settle` observation. + * + * A directional scroll that measures an unchanged surface WITH hidden content still in that + * direction does not answer at all: it fails with `scroll_no_progress`. + */ +export type ScrollMovementObservation = 'moved' | 'at-edge' | 'unchanged' | 'unobserved'; + export type ScrollDistanceOptions = { amount?: number; pixels?: number; @@ -95,6 +113,29 @@ export function honoredScrollDurationMs( return typeof result?.durationMs === 'number' ? result.durationMs : undefined; } +/** + * Where the leaf's swipe ran, as the midpoint of the coordinates it reported — the same absolute space + * its snapshots use, which is what lets a container rect say whether the gesture landed inside it. + * An owner that reports no coordinates is answered `undefined` rather than guessed at: a tvOS scroll + * is a remote keypress, so there is no midpoint to name. + */ +export function honoredScrollSwipeMidpoint( + result: Record | undefined, +): { x: number; y: number } | undefined { + const x1 = readReportedCoordinate(result?.x1); + const y1 = readReportedCoordinate(result?.y1); + const x2 = readReportedCoordinate(result?.x2); + const y2 = readReportedCoordinate(result?.y2); + if (x1 === undefined || y1 === undefined || x2 === undefined || y2 === undefined) { + return undefined; + } + return { x: (x1 + x2) / 2, y: (y1 + y2) / 2 }; +} + +function readReportedCoordinate(value: unknown): number | undefined { + return typeof value === 'number' && Number.isFinite(value) ? value : undefined; +} + /** * `scroll` — the generic-route result built by `buildDispatchedScrollResult` * (src/core/dispatch-scroll.ts): the resolved direction, the edge-pass @@ -118,6 +159,14 @@ export type ScrollCommandResult = { durationMs?: number; message?: string; settle?: SettleObservation; + /** + * The observation that gated this response's distance claim. See + * {@link ScrollMovementObservation}: `scroll` answers with what it measured after the gesture, + * and only `'moved'` and `'at-edge'` confirm the surface's fate. Absent on the tiers that verify + * per pass instead of per gesture (`scroll top`/`bottom` and `--until`), and on platforms whose + * scroll owner never dispatches a swipe (the Linux wheel). + */ + movement?: ScrollMovementObservation; /** * Set only when an on-screen keyboard made the owner clip the swipe into the band above it * (#2500). Absent means the swipe was not clipped, which is not the same claim as `false`: a diff --git a/src/daemon/scroll-until.ts b/src/daemon/scroll-until.ts index 8b123826b6..300ed28fe7 100644 --- a/src/daemon/scroll-until.ts +++ b/src/daemon/scroll-until.ts @@ -11,8 +11,10 @@ import { SELECTOR_PIPELINE_POLICIES } from '@agent-device/selectors/selector-pip import { canScrollFurtherAtEdge, pushScrollSurfaceSignature, + scrollNoProgressHint, scrollSurfaceFingerprint, scrollSurfaceIsStuck, + verticalEdgeFor, type ScrollEdge, } from '@agent-device/capture-kit/scroll-edge-state'; @@ -252,9 +254,7 @@ function scrollUntilNoProgressError( selector, direction, passes, - hint: - `The scroll is not reaching this container. If a field is focused, dismiss the keyboard first; if it is nested inside another scroller, target it directly. ` + - `Some lists ignore synthesized scrolls — a raw drag moves them: swipe x1 y1 x2 y2 started inside the list.`, + hint: scrollNoProgressHint(), }, ); } @@ -279,13 +279,3 @@ function scrollUntilCaptureError( }, ); } - -/** - * The end-of-content analyzer only reads vertical edges, so a horizontal `--until` is bounded by its - * pass budget alone rather than by a signal that would always report "no room". - */ -function verticalEdgeFor(direction: ScrollDirection): ScrollEdge | undefined { - if (direction === 'down') return 'bottom'; - if (direction === 'up') return 'top'; - return undefined; -}