From 9a495bd6ab29c8b1a9e98d533c2b6268244f8613 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Tue, 4 Aug 2026 21:23:22 +0200 Subject: [PATCH] fix(daemon): judge post-gesture movement by identity, not by the intersection MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes #1569. The post-gesture baseline check asked "did anything in the intersection of the two captures move". On a scroll that is the wrong question: a scroll REPLACES content rather than sliding shared elements around, so the intersection is whatever sat still. Measured on a live checkout-form `scroll down 0.6` that moved the entire form, the baseline and the post-scroll capture had exactly five identifiers in common and all five were tab-bar icons, which cannot move. What carried the verdict instead was anonymous layout containers matched by ordinal position among other anonymous nodes. That real pair had 8 of them on one side and 10 on the other, and the six "moved" entries reported deltas of -920, +37 and -7 px for a ~500px scroll. Removing them from the same pair flipped the verdict to 'unchanged' — for a scroll that replaced every element on screen. That is the saturation in #1569 finding 1: the oracle was reading capture composition, not the screen. Three changes: - Signature entries carry an `identity` (identifier|label|value|type) alongside the existing `key`. The key keeps folding in hittable/enabled/selected and an occurrence index, which is right for "did two back-to-back captures agree" and wrong across a gesture: scrolling flips `hittable` the moment a node's centre leaves the viewport, evicting exactly the elements whose movement was the evidence. Anonymous nodes get no identity and leave the comparison. - The verdict is set membership. Content leaving AND arriving is replacement, so 'changed'. One-sided difference is scope drift between differently scoped captures, so 'unchanged' — the subset tolerance the old rule needed, kept. Rect movement of a survivor is still 'changed'. - The loop records the baseline's snapshot backend and re-baselines rather than concluding when it changes. Backends do not return comparable views: on one live screen private AX returned 139 nodes including 43 scrolled off-viewport where the tree backend returned 48. A plan fallback or the XCTest-channel penalty can swap backends mid-poll at any time. The distrust budget is untouched — with an oracle that can see movement, a real scroll settles on the first quiet pair instead of running to the cap. Counterfactuals, each failing the pin it belongs to: putting `hittable` back into identity fails the hittability-flip test; admitting anonymous nodes fails the fluctuating-container test; removing the backend guard fails the re-baseline test. --- ...eraction-surface-baseline-evidence.test.ts | 165 ++++++++++++++++++ .../post-gesture-stabilization.test.ts | 138 ++++++++++++++- src/daemon/interaction-outcome-policy.ts | 140 ++++++++++----- src/daemon/post-gesture-stabilization.ts | 40 ++++- src/daemon/types.ts | 68 +++++--- 5 files changed, 476 insertions(+), 75 deletions(-) create mode 100644 src/daemon/__tests__/interaction-surface-baseline-evidence.test.ts diff --git a/src/daemon/__tests__/interaction-surface-baseline-evidence.test.ts b/src/daemon/__tests__/interaction-surface-baseline-evidence.test.ts new file mode 100644 index 000000000..074be2e2e --- /dev/null +++ b/src/daemon/__tests__/interaction-surface-baseline-evidence.test.ts @@ -0,0 +1,165 @@ +import assert from 'node:assert/strict'; +import { test } from 'vitest'; +import type { SnapshotNode } from '@agent-device/kernel/snapshot'; +import { + buildInteractionSurfaceSignature, + classifyBaselineSurfaceEvidence, +} from '../interaction-outcome-policy.ts'; + +// #1569: node shapes below are transcribed from a live iPhone 17 Pro capture of +// examples/test-app's checkout form, before and after `scroll down 0.6`. Two +// properties of that real screen are what these tests exist to hold: +// +// - the scroll REPLACED the content: nothing from the form section survives +// into the post capture, and the only identifiers common to both are the +// tab-bar entries, which by construction never move; +// - the captures disagree about which anonymous layout containers exist (the +// live pair had 8 and 10 of them), so any rule that matches those by +// ordinal position is reading noise. + +const tabBar = (): SnapshotNode[] => + [ + { label: 'Home', type: 'Other', rect: { x: 21, y: 791, width: 360, height: 62 } }, + { label: 'Home', type: 'Button', rect: { x: 25, y: 795, width: 74, height: 54 } }, + { + identifier: 'house.fill', + label: 'home', + type: 'Image', + rect: { x: 45, y: 801, width: 33, height: 28 }, + }, + ] as SnapshotNode[]; + +const anonymousContainers = (count: number, baseY: number): SnapshotNode[] => + Array.from({ length: count }, (_, index) => ({ + type: 'Other', + rect: { x: 0, y: baseY + index * 40, width: 402, height: 120 }, + })) as SnapshotNode[]; + +const formSection = (): SnapshotNode[] => + [ + { label: 'Full name', type: 'StaticText', rect: { x: 34, y: 319, width: 333, height: 17 } }, + { + identifier: 'field-name', + label: 'Full name', + type: 'TextField', + rect: { x: 35, y: 344, width: 333, height: 51 }, + }, + { label: 'Email', type: 'StaticText', rect: { x: 34, y: 410, width: 333, height: 17 } }, + { + identifier: 'field-email', + label: 'Email', + type: 'TextField', + rect: { x: 35, y: 435, width: 333, height: 51 }, + }, + ] as SnapshotNode[]; + +const deliverySection = (): SnapshotNode[] => + [ + { + identifier: 'shipping-pickup', + label: 'Pickup', + type: 'Button', + rect: { x: 126, y: 137, width: 75, height: 38 }, + }, + { + identifier: 'checkbox-agree', + label: 'I confirm the order details', + type: 'Other', + rect: { x: 34, y: 543, width: 333, height: 24 }, + }, + ] as SnapshotNode[]; + +const classify = (before: SnapshotNode[], after: SnapshotNode[]) => + classifyBaselineSurfaceEvidence( + buildInteractionSurfaceSignature(before), + buildInteractionSurfaceSignature(after), + ); + +test('a scroll that replaces the content is changed, though every shared element sat still', () => { + // The live shape of #1569. Shared identifiers: only the tab bar, all at dy=0. + // Asking those whether the screen moved is asking the one part of the surface + // guaranteed not to. + assert.equal( + classify( + [...formSection(), ...anonymousContainers(8, 60), ...tabBar()], + [...deliverySection(), ...anonymousContainers(10, 40), ...tabBar()], + ), + 'changed', + ); +}); + +test('the verdict does not depend on which anonymous containers a capture happened to include', () => { + // The defect: the previous rule matched anonymous nodes by ordinal position, + // so its entire movement evidence came from containers that alias. Strip them + // and it reported 'unchanged' for the very same pair of screens. Every + // composition of the same two screens must agree. + const compositions: [string, SnapshotNode[], SnapshotNode[]][] = [ + ['no containers', [...formSection(), ...tabBar()], [...deliverySection(), ...tabBar()]], + [ + 'matched container counts', + [...formSection(), ...anonymousContainers(8, 60), ...tabBar()], + [...deliverySection(), ...anonymousContainers(8, 60), ...tabBar()], + ], + [ + 'mismatched container counts', + [...formSection(), ...anonymousContainers(8, 60), ...tabBar()], + [...deliverySection(), ...anonymousContainers(10, 40), ...tabBar()], + ], + ]; + + for (const [label, before, after] of compositions) { + assert.equal(classify(before, after), 'changed', label); + } +}); + +test('an inert gesture stays unchanged while the anonymous container count fluctuates', () => { + // The live pair carried 8 anonymous containers on one side and 10 on the + // other for the SAME screen. Admitting them to the comparison makes that + // fluctuation look like content both leaving and arriving — a false 'changed' + // on a screen where nothing happened, which would settle a genuinely stale + // capture on the strength of layout noise. + assert.equal( + classify( + [...formSection(), ...anonymousContainers(8, 60), ...tabBar()], + [...formSection(), ...anonymousContainers(10, 40), ...tabBar()], + ), + 'unchanged', + ); +}); + +test('an inert gesture is unchanged even when volatile hittability flipped', () => { + // `hittable` is viewport-derived, so it flips without the element moving. + // Keying identity on it would drop this element from the comparison entirely. + const before = [ + ...formSection().map((node) => ({ ...node, hittable: true })), + ...tabBar(), + ] as SnapshotNode[]; + const after = [ + ...formSection().map((node) => ({ ...node, hittable: false })), + ...tabBar(), + ] as SnapshotNode[]; + + assert.equal(classify(before, after), 'unchanged'); +}); + +test('a narrower capture of the same screen is scope drift, not movement', () => { + // Baseline from a broad text search, quiet capture interactive-only: the + // second is a strict subset. Nothing arrived, so nothing was replaced. + assert.equal(classify([...formSection(), ...tabBar()], [...formSection()]), 'unchanged'); + assert.equal(classify([...formSection()], [...formSection(), ...tabBar()]), 'unchanged'); +}); + +test('movement of a surviving element is still changed', () => { + const moved = formSection().map((node) => ({ + ...node, + rect: { ...node.rect!, y: node.rect!.y - 120 }, + })) as SnapshotNode[]; + + assert.equal(classify([...formSection(), ...tabBar()], [...moved, ...tabBar()]), 'changed'); +}); + +test('sharing only the tab bar is ambiguous once the content sets are disjoint on one side', () => { + // Chrome alone carries no evidence: it reads identically whatever happened. + assert.equal(classify(tabBar(), tabBar()), 'unchanged'); + assert.equal(classify([...formSection()], [...deliverySection()]), 'ambiguous'); +}); diff --git a/src/daemon/__tests__/post-gesture-stabilization.test.ts b/src/daemon/__tests__/post-gesture-stabilization.test.ts index 2e3ebf59d..5b65f79af 100644 --- a/src/daemon/__tests__/post-gesture-stabilization.test.ts +++ b/src/daemon/__tests__/post-gesture-stabilization.test.ts @@ -158,12 +158,18 @@ test('capturePostGestureStabilizedResult keeps polling past the normal deadline assert.ok(captureCount > 8, `expected sustained polling, saw ${captureCount} captures`); }); -test('a replaced list under fixed chrome accepts stale but never claims no-effect (#1601 P1)', async () => { - // The reviewer's counterexample: a SUCCESSFUL scroll swapped every list - // cell while the tab-bar chrome (discriminating, shared, unmoved) kept the - // subset-tolerant classifier at 'unchanged'. The loop may still accept the - // stale read — but the agent-facing no-effect claim must be vetoed by the - // unmatched discriminating cells on both sides. +test('a replaced list under fixed chrome now settles outright, and still claims no no-effect (#1601 P1, #1569)', async () => { + // The reviewer's counterexample: a SUCCESSFUL scroll swapped every list cell + // while the tab-bar chrome (discriminating, shared, unmoved) kept the + // classifier at 'unchanged'. #1601 could only veto the agent-facing claim and + // had to let the loop accept the stale read — `staleAccepts` was 1 here. + // + // #1569 removed the premise: the classifier compares identified content by + // set membership, so cells leaving AND arriving is 'changed'. The loop now + // trusts the capture instead of polling to the cap, which is what made every + // checkout-form scroll pay the full distrust budget on live hardware. The + // no-effect claim stays absent — now because there is no accept-stale to + // corroborate at all, which is the stronger reason. vi.useFakeTimers(); const session = makeSession('ios'); session.snapshot = chromeWithListSnapshot(['row-1', 'row-2']); @@ -186,6 +192,45 @@ test('a replaced list under fixed chrome accepts stale but never claims no-effec await vi.advanceTimersByTimeAsync(10_000); const { result, staleAccepts } = await resultPromise; + assert.equal(staleAccepts, 0); + assert.equal(result.gestureNoEffect, undefined); +}); + +test('scope drift accepts stale but is vetoed from claiming no-effect (#1601 P1 gate)', async () => { + // #1601's full-surface gate stays load-bearing, and #1569 makes it more so. + // The classifier treats a one-sided difference as scope drift rather than + // movement, so a narrower quiet capture of an unmoved screen still reaches + // accept-stale — and the agent-facing claim must not follow it there, because + // the missing rows are unexamined, not proven absent. + vi.useFakeTimers(); + const session = makeSession('ios'); + session.snapshot = chromeWithListSnapshot(['row-1', 'row-2']); + markPostGestureStabilization(session, 'scroll'); + + // Same screen, but the quiet captures see only the chrome — the shape a + // broad baseline followed by an interactive-only capture produces. + const narrowed = makeSnapshotState( + chromeWithListSnapshot(['row-1', 'row-2']).nodes.filter( + (node) => node.type !== 'Cell', + ) as never, + ); + const capture = vi.fn(async () => narrowed); + + const resultPromise = withDiagnosticsScope({}, async () => { + const result = await capturePostGestureStabilizedResult({ + session, + capture, + readSnapshot: (snapshot) => snapshot, + }); + return { + result, + staleAccepts: countDiagnosticEventsByPhase(['post_gesture_snapshot_stale_accept']), + }; + }); + + await vi.advanceTimersByTimeAsync(10_000); + const { result, staleAccepts } = await resultPromise; + assert.equal(staleAccepts, 1); assert.equal(result.gestureNoEffect, undefined); }); @@ -431,3 +476,84 @@ test('capturePostGestureStabilizedResult trusts immediately (no cap tax) when th assert.equal(staleAccepts, 0); assert.equal(capture.mock.calls.length, 2); }); + +// --- #1569: a backend swap mid-poll is not comparable evidence --- + +test('capturePostGestureStabilizedResult re-baselines instead of concluding when the backend changes (iOS)', async () => { + // The capture plan can fall back, or the XCTest-channel penalty can pre-empt + // it, at any point during the poll. The backends do not agree on which nodes + // exist — on one live checkout screen private AX returned 139 nodes including + // 43 scrolled off-viewport where the tree backend returned 48 — so a quiet + // capture from a different backend says nothing about the gesture. It must + // become the new baseline, never a verdict. + vi.useFakeTimers(); + const session = makeSession('ios'); + session.snapshot = makeSnapshotState(pickupSnapshot(500).nodes, { + snapshotQuality: { state: 'healthy', backend: 'tree' }, + }); + markPostGestureStabilization(session, 'scroll'); + + // Every capture shows the pre-gesture surface, but on the OTHER backend. + const capture = vi.fn(async () => + makeSnapshotState(pickupSnapshot(500).nodes, { + snapshotQuality: { state: 'healthy', backend: 'private-ax' }, + }), + ); + + const resultPromise = withDiagnosticsScope({}, async () => { + await capturePostGestureStabilizedResult({ + session, + capture, + readSnapshot: (snapshot) => snapshot, + }); + return { + rebased: countDiagnosticEventsByPhase(['post_gesture_snapshot_baseline_rebased']), + staleAccepts: countDiagnosticEventsByPhase(['post_gesture_snapshot_stale_accept']), + settled: countDiagnosticEventsByPhase(['post_gesture_snapshot_stabilized']), + }; + }); + + await vi.advanceTimersByTimeAsync(6_000); + const { rebased, staleAccepts, settled } = await resultPromise; + + // Rebased once onto private-ax, then compared same-backend and settled. A + // cross-backend comparison would have produced a verdict from incomparable + // node sets instead. + assert.equal(rebased, 1); + assert.equal(staleAccepts + settled, 1); +}); + +test('capturePostGestureStabilizedResult still distrusts a same-backend baseline match (iOS)', async () => { + // The guard above must not become a blanket escape hatch: when the backend is + // stable, an unchanged surface is still the stale-read signal #1542 added. + vi.useFakeTimers(); + const session = makeSession('ios'); + session.snapshot = makeSnapshotState(pickupSnapshot(500).nodes, { + snapshotQuality: { state: 'healthy', backend: 'tree' }, + }); + markPostGestureStabilization(session, 'scroll'); + + const capture = vi.fn(async () => + makeSnapshotState(pickupSnapshot(500).nodes, { + snapshotQuality: { state: 'healthy', backend: 'tree' }, + }), + ); + + const resultPromise = withDiagnosticsScope({}, async () => { + await capturePostGestureStabilizedResult({ + session, + capture, + readSnapshot: (snapshot) => snapshot, + }); + return { + rebased: countDiagnosticEventsByPhase(['post_gesture_snapshot_baseline_rebased']), + staleAccepts: countDiagnosticEventsByPhase(['post_gesture_snapshot_stale_accept']), + }; + }); + + await vi.advanceTimersByTimeAsync(6_000); + const { rebased, staleAccepts } = await resultPromise; + + assert.equal(rebased, 0); + assert.equal(staleAccepts, 1); +}); diff --git a/src/daemon/interaction-outcome-policy.ts b/src/daemon/interaction-outcome-policy.ts index 95fe31a8f..a1f9518e6 100644 --- a/src/daemon/interaction-outcome-policy.ts +++ b/src/daemon/interaction-outcome-policy.ts @@ -194,61 +194,96 @@ export function areInteractionSurfaceSignaturesStable( } /** - * Subset-tolerant baseline classifier for post-gesture baseline distrust - * (#1542 defect 2), reusing this module's existing three-valued vocabulary - * (`InteractionSurfaceChange`) instead of a bespoke boolean. The pre-gesture - * baseline and the post-gesture quiet capture routinely come from different - * snapshot scopes (e.g. a broad text-search capture vs. an interactive-only - * selector capture), so their signatures can differ in length/membership even - * when the element that matters never moved — whole-array equality would - * report "changed" purely from scope drift and never catch the real - * staleness. + * Baseline classifier for post-gesture baseline distrust (#1542 defect 2), + * reusing this module's three-valued `InteractionSurfaceChange` vocabulary. * - * The evidence rule: only shared entries flagged `discriminating` (i.e. NOT - * the viewport root or keyboard-window chrome — see - * `isNonDiscriminatingSurfaceNode`) count as evidence. + * The rule is set membership, not rect deltas on the intersection, and #1569 + * is why. A scroll does not slide shared elements to new positions — it + * REPLACES the content. Measured on the checkout form, a `scroll down 0.6` + * that moved the whole form left exactly five identifiers in common with its + * baseline, and all five were tab-bar icons that by construction never move. + * Judging such a pair by "did anything in the intersection shift" asks the + * only elements guaranteed to sit still whether anything moved, so the honest + * signal is that the content set itself differs. * - * - `'ambiguous'`: the shared overlap has zero discriminating entries — this - * includes an empty overlap AND an overlap that is only structurally fixed - * chrome (e.g. two signatures sharing nothing but the Application/Window - * root after a successful scroll swapped every real element — the exact - * live shape #1563's review caught: treating that as a match would extend - * every such interaction to the stale-read cap on zero real evidence). - * Ambiguous is NOT a match — insufficient evidence is its own first-class - * outcome, the same way `classifyInteractionSurfaceChange` already treats - * an empty side. - * - `'changed'`: at least one discriminating shared entry moved beyond - * tolerance — real movement occurred. - * - `'unchanged'`: every discriminating shared entry (and there is at least - * one) still matches — this is the actual "stale, matches baseline" signal - * the distrust check exists to catch. + * Only entries that carry an `identity` participate. Anonymous layout nodes + * can be matched solely by ordinal position among other anonymous nodes, and + * the two captures rarely contain the same number of them — on that same real + * pair the previous implementation's entire "movement" evidence was six such + * aliased entries reporting deltas of -920, +37 and -7 px for a ~500px scroll. + * Structural chrome (viewport root, keyboard) is excluded for the older reason + * that its rect is invariant under any gesture. + * + * Set difference alone would mistake scope drift for movement: the baseline and + * the quiet capture are routinely fetched by different callers with different + * snapshot scopes (a broad text search vs. an interactive-only capture), and + * the narrower one is then a strict SUBSET of the broader. Replacement is what + * separates the two — a scroll leaves each side holding content the other + * lacks, while scope drift only ever removes from one side. + * + * - `'changed'`: each side holds identified content the other does not (the + * surface was replaced), or a surviving element moved beyond tolerance. + * - `'unchanged'`: everything both sides can see agrees, in the same places — + * the real "still showing the pre-gesture screen" signal distrust exists to + * catch. A one-sided difference lands here: it is scope, not movement. + * - `'ambiguous'`: a side carries no identified content, or the two share none, + * so there is nothing comparable. Never treated as a match. + * + * Both signatures must come from the same snapshot backend; the caller owns + * that invariant (see `post-gesture-stabilization.ts`). Backends disagree about + * which nodes exist, so a cross-backend pair differs for reasons that have + * nothing to do with the gesture. */ export function classifyBaselineSurfaceEvidence( baseline: InteractionSurfaceSignature, current: InteractionSurfaceSignature, ): InteractionSurfaceChange { - if (baseline.length === 0 || current.length === 0) return 'ambiguous'; - const baselineByKey = new Map(baseline.map((entry) => [entry.key, entry])); - let discriminatingOverlap = 0; - for (const entry of current) { - const baselineEntry = baselineByKey.get(entry.key); - if (!baselineEntry) continue; - // Shared but non-discriminating (viewport root / keyboard chrome): this - // pair carries no evidence either way, so it neither counts toward the - // overlap nor is checked for movement (its rect is invariant by - // definition and comparing it would be pure noise). - if (!entry.discriminating || !baselineEntry.discriminating) continue; - discriminatingOverlap += 1; + const before = identifiedContent(baseline); + const after = identifiedContent(current); + if (before.size === 0 || after.size === 0) return 'ambiguous'; + + let shared = 0; + let droppedFromBaseline = false; + for (const [identity, seen] of before) { + const now = after.get(identity); + if (!now) { + droppedFromBaseline = true; + continue; + } + shared += 1; if ( - Math.abs(baselineEntry.x - entry.x) > RECT_TOLERANCE_PX || - Math.abs(baselineEntry.y - entry.y) > RECT_TOLERANCE_PX || - Math.abs(baselineEntry.width - entry.width) > RECT_TOLERANCE_PX || - Math.abs(baselineEntry.height - entry.height) > RECT_TOLERANCE_PX + Math.abs(seen.entry.x - now.entry.x) > RECT_TOLERANCE_PX || + Math.abs(seen.entry.y - now.entry.y) > RECT_TOLERANCE_PX || + Math.abs(seen.entry.width - now.entry.width) > RECT_TOLERANCE_PX || + Math.abs(seen.entry.height - now.entry.height) > RECT_TOLERANCE_PX ) { return 'changed'; } } - return discriminatingOverlap > 0 ? 'unchanged' : 'ambiguous'; + if (shared === 0) return 'ambiguous'; + const addedSinceBaseline = after.size > shared; + // Content left AND arrived: the surface was replaced, which is exactly what a + // scroll that moved does. Only one of the two is a narrower or broader + // capture of the same screen. + if (droppedFromBaseline && addedSinceBaseline) return 'changed'; + return 'unchanged'; +} + +/** + * Identity-keyed view of a signature: the entries that can be compared across a + * gesture at all. Repeated identities (list rows sharing a label) collapse onto + * their first occurrence, which is the one whose rect is compared — a + * later duplicate carries no identity the first does not. + */ +function identifiedContent( + signature: InteractionSurfaceSignature, +): Map { + const content = new Map(); + for (const entry of signature) { + if (!entry.identity || !entry.discriminating) continue; + if (!content.has(entry.identity)) content.set(entry.identity, { entry }); + } + return content; } /** @@ -311,8 +346,10 @@ function buildInteractionSurfaceEntry( if (!semanticKey) return undefined; const occurrence = occurrenceCounts.get(semanticKey) ?? 0; occurrenceCounts.set(semanticKey, occurrence + 1); + const identity = interactionSurfaceIdentity(node); return { key: `${semanticKey}|#${occurrence}`, + ...(identity ? { identity } : {}), x: Math.round(node.rect.x), y: Math.round(node.rect.y), width: Math.round(node.rect.width), @@ -321,6 +358,23 @@ function buildInteractionSurfaceEntry( }; } +/** + * What the element IS — never where it sits, and never volatile state a gesture + * is expected to change. `interactionSurfaceSemanticKey` deliberately folds in + * `hittable`/`enabled`/`selected` and an occurrence index, which is right for + * "did these two back-to-back captures agree" and wrong for "is this the same + * element as before the gesture": scrolling flips `hittable` the moment a + * node's centre leaves the viewport, so keying on it evicts precisely the + * elements whose movement would have been the evidence (#1569). + */ +function interactionSurfaceIdentity(node: SnapshotNode): string | undefined { + const identity = [node.identifier, node.label, node.value] + .map((value) => (typeof value === 'string' ? value.trim() : '')) + .join('|'); + if (!identity.replaceAll('|', '')) return undefined; + return `${identity}|${node.type ?? ''}`; +} + /** * Structurally fixed elements whose rect is invariant under a scroll/swipe by * construction — sharing only these between a baseline and a later capture is diff --git a/src/daemon/post-gesture-stabilization.ts b/src/daemon/post-gesture-stabilization.ts index 1e757afb2..a740a1ab7 100644 --- a/src/daemon/post-gesture-stabilization.ts +++ b/src/daemon/post-gesture-stabilization.ts @@ -49,7 +49,12 @@ export function markPostGestureStabilization( // pre-capture — the same "last known pre-action snapshot" idiom // `markPendingInteractionOutcome` already relies on). ...(requiresPostGestureBaselineDistrust(session.device) - ? { baselineSignature: buildInteractionSurfaceSignature(session.snapshot?.nodes ?? []) } + ? { + baselineSignature: buildInteractionSurfaceSignature(session.snapshot?.nodes ?? []), + // Recorded so the loop can tell a comparable quiet capture from one + // served by a different backend, which is not comparable at all. + baselineBackend: session.snapshot?.snapshotQuality?.backend, + } : {}), }; } @@ -120,7 +125,11 @@ export function decidePostGestureStabilityVerdict(params: { return elapsedMs < distrustCapMs ? 'distrust' : 'accept-stale'; } -type CapturedSurface = { value: T; signature: InteractionSurfaceSignature }; +type CapturedSurface = { + value: T; + signature: InteractionSurfaceSignature; + backend: string | undefined; +}; async function captureInteractionSurface( capture: () => Promise, @@ -128,7 +137,12 @@ async function captureInteractionSurface( initial?: T, ): Promise> { const value = initial ?? (await capture()); - return { value, signature: buildInteractionSurfaceSignature(readSnapshot(value).nodes) }; + const snapshot = readSnapshot(value); + return { + value, + signature: buildInteractionSurfaceSignature(snapshot.nodes), + backend: snapshot.snapshotQuality?.backend, + }; } function emitPostGestureSettleDiagnostic( @@ -183,6 +197,8 @@ export async function capturePostGestureStabilizedResult(params: { const startedAt = Date.now(); let attempts = 1; let previous = await captureInteractionSurface(capture, readSnapshot, params.initial); + let baselineSignature = pending.baselineSignature; + let baselineBackend = pending.baselineBackend; // Extended past STABILIZATION_DEADLINE_MS only when the distrust verdict // fires below; the ordinary (non-distrust) timeout path is unaffected. let effectiveDeadlineMs = STABILIZATION_DEADLINE_MS; @@ -193,9 +209,25 @@ export async function capturePostGestureStabilizedResult(params: { const current = await captureInteractionSurface(capture, readSnapshot); if (areInteractionSurfaceSignaturesStable(previous.signature, current.signature)) { const elapsedMs = Date.now() - startedAt; + // A capture plan may fall back or be pre-empted by the XCTest-channel + // penalty at any time, so the backend can change mid-poll. Backends do + // not agree on which nodes exist, so this pair says nothing about the + // gesture: adopt it as the baseline and keep going rather than concluding + // from it (#1569). + if (baselineSignature && baselineBackend !== current.backend) { + emitDiagnostic({ + level: 'debug', + phase: 'post_gesture_snapshot_baseline_rebased', + data: { action: pending.action, from: baselineBackend, to: current.backend, attempts }, + }); + baselineSignature = current.signature; + baselineBackend = current.backend; + previous = current; + continue; + } const verdict = decidePostGestureStabilityVerdict({ needsBaselineDistrust, - baselineSignature: pending.baselineSignature, + baselineSignature, quietSignature: current.signature, elapsedMs, distrustCapMs: STABILIZATION_DISTRUST_DEADLINE_MS, diff --git a/src/daemon/types.ts b/src/daemon/types.ts index 294a1b9e5..4c85655c8 100644 --- a/src/daemon/types.ts +++ b/src/daemon/types.ts @@ -196,6 +196,42 @@ export type AndroidSnapshotFreshness = { routeComparable: boolean; }; +/** + * One node's contribution to an interaction-surface signature. Two comparisons + * read this, and they need different things from it, which is why both `key` + * and `identity` exist: + * + * - `key` answers "is this the same node in the same state", including volatile + * viewport-derived state and an occurrence index. Back-to-back captures use + * it to decide the surface went quiet. + * - `identity` answers "is this the same element at all", and is present only + * for nodes that carry one (an identifier, label or value). Comparisons + * across a gesture use it, because a gesture is exactly what changes the + * volatile state `key` folds in. + */ +export type InteractionSurfaceEntry = { + key: string; + /** + * Identity-only key: what the element IS, never where it currently sits or + * whether it is presently hittable. Undefined for anonymous layout nodes, + * which carry no identity and can therefore only be matched by ordinal + * position — an aliasing trap, so they are excluded from cross-gesture + * comparison entirely. + */ + identity?: string; + x: number; + y: number; + width: number; + height: number; + /** + * False for structurally fixed elements (the viewport root, keyboard + * chrome) whose rect is invariant regardless of any gesture — shared + * evidence limited to these is not evidence at all. See + * `classifyBaselineSurfaceEvidence` in interaction-outcome-policy.ts. + */ + discriminating: boolean; +}; + export type PostGestureStabilization = { action: string; /** The gesture's own positionals (e.g. scroll direction) — wording input for @@ -212,20 +248,15 @@ export type PostGestureStabilization = { * not proof the screen settled. Android's persistent helper clears its a11y * cache before every capture (#1254/#1259) and needs no baseline check. */ - baselineSignature?: Array<{ - key: string; - x: number; - y: number; - width: number; - height: number; - /** - * False for structurally fixed elements (the viewport root, keyboard - * chrome) whose rect is invariant regardless of any gesture — shared - * evidence limited to these never counts toward a baseline match. See - * `classifyBaselineSurfaceEvidence` in interaction-outcome-policy.ts. - */ - discriminating: boolean; - }>; + baselineSignature?: InteractionSurfaceEntry[]; + /** + * Snapshot backend that produced `baselineSignature`. Backends do not return + * comparable views of one screen — on the same iOS screen private AX returns + * the scrolled-away content the tree backend prunes — so a quiet capture from + * a different backend can only be re-baselined against, never concluded from + * (#1569). + */ + baselineBackend?: string; }; export type PendingInteractionOutcome = { @@ -235,14 +266,7 @@ export type PendingInteractionOutcome = { flags?: CommandFlags; markedAt: number; attemptsRemaining: number; - preSignature: Array<{ - key: string; - x: number; - y: number; - width: number; - height: number; - discriminating: boolean; - }>; + preSignature: InteractionSurfaceEntry[]; }; type SessionRecordingBase = {