diff --git a/packages/capture-kit/src/snapshot/scroll-edge-state.ts b/packages/capture-kit/src/snapshot/scroll-edge-state.ts index 7359b2348d..240c1237f6 100644 --- a/packages/capture-kit/src/snapshot/scroll-edge-state.ts +++ b/packages/capture-kit/src/snapshot/scroll-edge-state.ts @@ -81,9 +81,10 @@ export async function captureScrollEdgeState(params: { export async function readScrollEdgeState( nodes: readonly (RawSnapshotNode | SnapshotNode)[], edge: ScrollEdge, + target: ScrollEdgeTarget = {}, ): Promise { const { analyzeScrollEdgeState } = await import('./scroll-edge-state/selection.ts'); - return analyzeScrollEdgeState(nodes, edge); + return analyzeScrollEdgeState(nodes, edge, target); } /** diff --git a/src/daemon/__tests__/scroll-movement.test.ts b/src/daemon/__tests__/scroll-movement.test.ts index 1097fa2ad3..5756f614d0 100644 --- a/src/daemon/__tests__/scroll-movement.test.ts +++ b/src/daemon/__tests__/scroll-movement.test.ts @@ -177,6 +177,94 @@ test('a surface that no longer holds the pre-gesture content answers moved on th assert.equal(spy.calls(), 1); }); +/** + * At the end of a list iOS rubber-bands past the edge: the first capture lands mid-bounce with every row + * shifted, then the content springs back to exactly the pre-gesture tree (#2884). A baseline that already + * showed the end of the content turns that first read into a question, not a verdict. + */ +test('a bounce past the edge that springs back is at-edge, not moved', async () => { + const { observation, spy } = observe({ + baseline: baselineOf(screen(0, false)), + screens: [screen(-14, false), screen(0, false), screen(0, false)], + }); + + assert.equal(await observation, 'at-edge'); + assert.equal(spy.calls(), 3); +}); + +/** + * The edge question is asked of the scroller under the swipe, through the same selection the edge + * verdict uses: among the containers holding the point, the one that still hides content in that + * direction. An inner list at its end inside an outer list with more below hands the swipe to the + * outer list (that is what the platform does with the gesture), so the rest requirement does not + * engage and the first differing read is the movement it produced. + */ +test('an inner list at its end inside an outer list with hidden content hands the swipe on', async () => { + const outer = { + type: 'ScrollView', + identifier: 'outer', + rect: { x: 0, y: 100, width: 402, height: 760 }, + hiddenContentBelow: true, + } as SnapshotNode; + const inner = (rowOffset: number) => [ + outer, + ...screen(rowOffset, false).map((node) => + node.type === 'ScrollView' ? ({ ...node, identifier: 'inner' } as SnapshotNode) : node, + ), + ]; + const { observation, spy } = observe({ + baseline: baselineOf(inner(0)), + screens: [inner(-300)], + }); + + assert.equal(await observation, 'moved'); + assert.equal(spy.calls(), 1); +}); + +/** With no outer list left to take it, the swipe point resolves the inner list and its edge gates the claim. */ +test('an inner list at its end with no outer list left to scroll is gated on rest', async () => { + const outerAtEnd = { + type: 'ScrollView', + identifier: 'outer', + rect: { x: 0, y: 100, width: 402, height: 760 }, + } as SnapshotNode; + const inner = (rowOffset: number) => [ + outerAtEnd, + ...screen(rowOffset, false).map((node) => + node.type === 'ScrollView' ? ({ ...node, identifier: 'inner' } as SnapshotNode) : node, + ), + ]; + const { observation, spy } = observe({ + baseline: baselineOf(inner(0)), + screens: [inner(-14), inner(0), inner(0)], + }); + + assert.equal(await observation, 'at-edge'); + assert.equal(spy.calls(), 3); +}); + +test('content that changes at the edge and holds still is still moved', async () => { + const { observation, spy } = observe({ + baseline: baselineOf(screen(0, false)), + screens: [screen(-300, false), screen(-300, false)], + }); + + assert.equal(await observation, 'moved'); + // The rest requirement costs exactly the one extra read, and only at the edge. + assert.equal(spy.calls(), 2); +}); + +test('a bounce that never settles at the edge spends the budget and answers unobserved', async () => { + const { observation } = observe({ + baseline: baselineOf(screen(0, false)), + screens: (attempt) => screen(attempt % 2 === 0 ? -14 : 0, false), + budgetMs: 40, + }); + + assert.equal(await observation, 'unobserved'); + assertWithheld('surface-unsettled'); +}); + test('a surface that never shifted while the container still hides content refuses with a typed reason', async () => { const { observation, spy } = observe({ baseline: baselineOf(screen(0)), diff --git a/src/daemon/scroll-movement.ts b/src/daemon/scroll-movement.ts index e0c878d7bb..7bc96f6ca0 100644 --- a/src/daemon/scroll-movement.ts +++ b/src/daemon/scroll-movement.ts @@ -51,6 +51,8 @@ import type { SessionState } from './session-state.ts'; * baseline is answered on the first capture, so a scroll that worked pays nothing extra; only a * surface that looks untouched keeps polling (a mid-flight or stale read is indistinguishable from a * no-op until it either moves or goes quiet), and only an untouched surface at rest is ever reported. + * The one exception is a baseline that already ended in the scrolled direction: there a differing + * first read is an overscroll bounce until it holds still (`baselineEndsInDirection`). * * Nothing is classified before the two trees are established to be two views of one screen, and that * gate runs on every capture rather than only on the quiet path. The deferred loop can adopt a new @@ -68,6 +70,8 @@ export type ScrollSurfaceBaseline = Readonly<{ signature: InteractionSurfaceSignature; presentationKey: string | undefined; comparisonKey: string | undefined; + /** The tree itself: whether the content already ended in the scrolled direction is asked of nodes. */ + nodes: SnapshotState['nodes']; }>; /** Why there is nothing honest to compare this scroll's effect against. */ @@ -134,6 +138,7 @@ export function readScrollSurfaceBaseline( signature, presentationKey: snapshot.presentationKey, comparisonKey: snapshotSurfaceComparisonKey(snapshot), + nodes: snapshot.nodes, }; } @@ -234,7 +239,9 @@ function surfacePairDrift( /** * Polls until an untouched surface proves itself. A surface that differs from the baseline is a - * verdict on the first capture, so a scroll that worked pays for one read. One that looks untouched + * verdict on the first capture, so a scroll that worked pays for one read, unless the baseline + * already ended in the scrolled direction, where the change must hold still first + * (`baselineEndsInDirection`). One that looks untouched * needs a quiet pair, because a gesture still in flight and a gesture that did nothing answer a * single read identically. A surface that never holds still expires as `surface-unsettled` rather * than being called a no-op — and that answer is worth a warning the others are not, since it spent @@ -254,28 +261,91 @@ async function pollForSurfaceVerdict( const deadline = startedAt + (params.budgetMs ?? MOVEMENT_VERDICT_BUDGET_MS); let previous: InteractionSurfaceSignature | undefined; let attempts = 0; + let changeNeedsRest: boolean | undefined; while (true) { const reading = await readOneCapture(baseline, params.capture); attempts += 1; if (reading.kind === 'blind') return { kind: 'blind', reason: reading.reason }; - if (reading.kind === 'changed') - return { kind: 'moved', observed: reading.observed, attempts, startedAt }; - if (surfaceIsAtRest(previous, reading.observed.signature)) { - return { - kind: 'settled', - observed: reading.observed, - evidence: reading.evidence, - attempts, - startedAt, - }; + if (reading.kind === 'changed') { + changeNeedsRest ??= await baselineEndsInDirection(baseline, params.direction, params.swipe); } + const verdict = settledVerdict(reading, { + previous, + changeNeedsRest: changeNeedsRest === true, + attempts, + startedAt, + }); + if (verdict) return verdict; if (Date.now() >= deadline) return budgetExpiredVerdict(params, attempts, startedAt); previous = reading.observed.signature; await sleep(params.pollMs ?? MOVEMENT_POLL_MS); } } +/** + * The verdict one readable capture supports, or nothing yet. A changed surface is movement on sight + * unless the baseline already ended in that direction, where it must hold still first; an unchanged + * surface is settled only as the second of a quiet pair. + */ +function settledVerdict( + reading: Exclude, + poll: { + previous: InteractionSurfaceSignature | undefined; + changeNeedsRest: boolean; + attempts: number; + startedAt: number; + }, +): SurfaceVerdict | undefined { + const atRest = surfaceIsAtRest(poll.previous, reading.observed.signature); + const { attempts, startedAt } = poll; + if (reading.kind === 'changed') { + if (!poll.changeNeedsRest || atRest) { + return { kind: 'moved', observed: reading.observed, attempts, startedAt }; + } + return undefined; + } + if (!atRest) return undefined; + return { + kind: 'settled', + observed: reading.observed, + evidence: reading.evidence, + attempts, + startedAt, + }; +} + +/** + * Whether the pre-gesture tree already showed the end of the content in the scrolled direction. A + * scroll past that edge on iOS rubber-bands: the first capture lands mid-bounce with every row shifted + * by a few points and reads as `changed`, then the content springs back to exactly the baseline, which + * is what the next command's stabilization later observes as a stale-accept (#2884). So a change against + * a baseline that had nothing left to reveal is credited only once the surface holds still and still + * differs; a scroll whose baseline still hid content keeps paying one read. + */ +async function baselineEndsInDirection( + baseline: ScrollSurfaceBaseline, + direction: ScrollDirection, + swipe: ScrollSwipeEvidence, +): Promise { + const edge = verticalEdgeFor(direction); + if (!edge) return false; + // The question is asked of the scroller the swipe ran in, through the selection the edge verdict + // uses: among the containers holding the point, the one that still hides content in that + // direction. An inner list at its end inside an outer list with more below hands the gesture to + // the outer list, so the outer list's edge is what gates the claim. + const state = await readScrollEdgeState(baseline.nodes, edge, { point: swipe.midpoint }); + const ends = state.containerRect !== undefined && !state.canScroll; + if (ends) { + emitDiagnostic({ + level: 'debug', + phase: 'scroll_movement_edge_rest_required', + data: { direction, containerRect: state.containerRect }, + }); + } + return ends; +} + /** * What a single capture says, on its own: nothing at all when the tree could not be read or comes from * another lineage, movement when the surface differs from the baseline, and otherwise the untouched