From 9b902d4cf852d5674d60585301564681984a13aa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Thu, 6 Aug 2026 15:55:57 +0200 Subject: [PATCH] refactor(daemon): consolidate surface-evidence helpers, delete the wording heuristic MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Re-derived onto #1633's split (`post-gesture-stabilization.ts` became `post-gesture-stability.ts` + `deferred-interaction-outcome.ts` + `gesture-no-effect.ts`). None of this had been subsumed by that refactor — it relocated the code and carried every one of these forward untouched. - Three copies of the four-field rect comparison in `interaction-outcome-policy.ts` become one `rectsWithinTolerance`. - `identifiedContent` returns the entry instead of `{ entry }`, dropping the `.entry` indirection at every call site. - `haveIdenticalDiscriminatingSurfaces` records why it keys on `key` while `classifyBaselineSurfaceEvidence` keys on `identity` — opposite choices, once, at the function that makes the stricter one. - `SnapshotCaptureBackend` names the capture-strategy union in kernel/snapshot beside `SnapshotBackend`, and `PostGestureStabilization.baselineBackend` uses it instead of `string`, closing the silent-typo gap on the comparison the field exists for. Deliberately NOT applied to `post-gesture-stability.ts`: #1633 made that module generic over the surface type, and `backend: string` is right for an interface that must not know about iOS capture strategies. - `formatGestureNoEffectWarning` echoes positionals verbatim. The `/^[\d.-]+$/` filter it replaces ate all four coordinates of `swipe ` and emitted a contentless bare "swipe"; the warning names the gesture the agent issued, and `scroll down 1` is what they issued. - `PostGestureStabilization.positionals` is required — its only writer always sets it, so the `?? []` at the read site guarded an impossible state. Tightening it caught six test fixtures building the state directly. Red evidence: restoring the numeric filter fails the wording test ("scroll down 1 produced no visible change"); 95 files / 757 tests green with it deleted. --- packages/kernel/src/snapshot.ts | 11 +++- .../__tests__/direct-ios-selector.test.ts | 4 +- .../post-gesture-stabilization.test.ts | 9 +++- src/daemon/gesture-no-effect.ts | 4 +- .../handlers/__tests__/interaction.test.ts | 4 +- .../__tests__/snapshot-handler.test.ts | 2 + src/daemon/interaction-outcome-policy.ts | 53 +++++++++++-------- src/daemon/types.ts | 10 ++-- 8 files changed, 60 insertions(+), 37 deletions(-) diff --git a/packages/kernel/src/snapshot.ts b/packages/kernel/src/snapshot.ts index 8daefac39..c5caa730b 100644 --- a/packages/kernel/src/snapshot.ts +++ b/packages/kernel/src/snapshot.ts @@ -6,9 +6,18 @@ * snapshot-quality.ts so SnapshotNode can reference it without a cyclic import; * snapshot-quality.ts (the validation logic) re-exports it for existing callers. */ +/** + * Which capture STRATEGY produced a snapshot, within one platform's plan — + * distinct from `SnapshotBackend`, which names the platform channel + * (`xctest`/`android`/…). The iOS plan walks these in order, so one session can + * change strategy mid-sequence, and two strategies do not return comparable + * views of one screen (#1569). + */ +export type SnapshotCaptureBackend = 'tree' | 'queries' | 'private-ax'; + export type SnapshotQualityVerdict = { state: 'healthy' | 'recovered' | 'sparse'; - backend: 'tree' | 'queries' | 'private-ax'; + backend: SnapshotCaptureBackend; reason?: string; // 'deferred' = the penalty circuit breaker pre-selected a non-XCTest backend; nothing new // degraded on THIS capture (no repeated warning, no settle budget reset). diff --git a/src/daemon/__tests__/direct-ios-selector.test.ts b/src/daemon/__tests__/direct-ios-selector.test.ts index 2c7d503ff..2f153a43d 100644 --- a/src/daemon/__tests__/direct-ios-selector.test.ts +++ b/src/daemon/__tests__/direct-ios-selector.test.ts @@ -108,7 +108,7 @@ test('isLocalIosRunnerSession: iOS local sessions are eligible, Android and unde test('isLocalIosRunnerSession: skipPendingPostGestureStabilization:true excludes a pending session (the tap fast path)', () => { const pending = makeSession('ios', { - postGestureStabilization: { action: 'scroll', markedAt: Date.now() }, + postGestureStabilization: { action: 'scroll', positionals: [], markedAt: Date.now() }, }); assert.equal( isLocalIosRunnerSession(pending, { skipPendingPostGestureStabilization: true }), @@ -118,7 +118,7 @@ test('isLocalIosRunnerSession: skipPendingPostGestureStabilization:true excludes test('isLocalIosRunnerSession: skipPendingPostGestureStabilization:false keeps a pending session eligible (the offscreen double-check)', () => { const pending = makeSession('ios', { - postGestureStabilization: { action: 'scroll', markedAt: Date.now() }, + postGestureStabilization: { action: 'scroll', positionals: [], markedAt: Date.now() }, }); assert.equal( isLocalIosRunnerSession(pending, { skipPendingPostGestureStabilization: false }), diff --git a/src/daemon/__tests__/post-gesture-stabilization.test.ts b/src/daemon/__tests__/post-gesture-stabilization.test.ts index 2286dc115..b41ad2e53 100644 --- a/src/daemon/__tests__/post-gesture-stabilization.test.ts +++ b/src/daemon/__tests__/post-gesture-stabilization.test.ts @@ -250,8 +250,10 @@ test('scope drift accepts stale but is vetoed from claiming no-effect (#1601 P1 }); test('formatGestureNoEffectWarning names the gesture and the raw-drag escape hatch', () => { + // Positionals echo verbatim: the warning names the gesture the agent issued, + // and `scroll down 1` is what they issued. const scrollWarning = formatGestureNoEffectWarning('scroll', ['down', '1']); - assert.match(scrollWarning, /scroll down produced no visible change/); + assert.match(scrollWarning, /scroll down 1 produced no visible change/); assert.match(scrollWarning, /swipe x1 y1 x2 y2/); assert.match(scrollWarning, /already at its edge/); @@ -260,6 +262,11 @@ test('formatGestureNoEffectWarning names the gesture and the raw-drag escape hat const bareWarning = formatGestureNoEffectWarning('swipe', []); assert.match(bareWarning, /swipe produced no visible change/); + + // The regression the deleted heuristic caused: every positional of a swipe is + // a coordinate, so "drop anything numeric-looking" left a contentless "swipe". + const swipeWarning = formatGestureNoEffectWarning('swipe', ['10', '20', '30', '40']); + assert.match(swipeWarning, /^swipe 10 20 30 40 produced no visible change/); }); test('capturePostGestureStabilizedResult trusts a quiet signature once content genuinely differs from the baseline (iOS)', async () => { diff --git a/src/daemon/gesture-no-effect.ts b/src/daemon/gesture-no-effect.ts index 820f29352..ea7924ee3 100644 --- a/src/daemon/gesture-no-effect.ts +++ b/src/daemon/gesture-no-effect.ts @@ -8,9 +8,7 @@ import type { SnapshotCaptureAnnotations } from '@agent-device/contracts/capture * raw `swipe` worked where scroll/fling/pan all silently no-opped). */ export function formatGestureNoEffectWarning(action: string, positionals: string[]): string { - const gesture = [action, ...positionals.filter((value) => !/^[\d.-]+$/.test(value))] - .join(' ') - .trim(); + const gesture = [action, ...positionals].join(' ').trim(); return ( `${gesture} produced no visible change: the tree still matches its pre-gesture state. ` + 'Either the container is already at its edge, or it ignores synthesized scrolls — ' + diff --git a/src/daemon/handlers/__tests__/interaction.test.ts b/src/daemon/handlers/__tests__/interaction.test.ts index 8dfcdcbd6..8437ef350 100644 --- a/src/daemon/handlers/__tests__/interaction.test.ts +++ b/src/daemon/handlers/__tests__/interaction.test.ts @@ -849,7 +849,7 @@ test('click simple iOS id selector waits for snapshot path after pending gesture const sessionStore = makeSessionStore(); const sessionName = 'ios-direct-selector-after-swipe'; const session = makeIosSession(sessionName, { appBundleId: 'com.example.app' }); - session.postGestureStabilization = { action: 'swipe', markedAt: Date.now() }; + session.postGestureStabilization = { action: 'swipe', positionals: [], markedAt: Date.now() }; sessionStore.set(sessionName, session); mockDispatch.mockImplementation(async (_device, command, positionals) => { @@ -3107,7 +3107,7 @@ test('is simple iOS selector falls back to snapshot while gesture stabilization const sessionStore = makeSessionStore(); const sessionName = 'is-selected-ios-stabilizing'; const session = makeIosSession(sessionName, { appBundleId: 'com.example.app' }); - session.postGestureStabilization = { action: 'swipe', markedAt: Date.now() }; + session.postGestureStabilization = { action: 'swipe', positionals: [], markedAt: Date.now() }; sessionStore.set(sessionName, session); mockDispatch.mockImplementation(async (_device, command) => { diff --git a/src/daemon/handlers/__tests__/snapshot-handler.test.ts b/src/daemon/handlers/__tests__/snapshot-handler.test.ts index c518e4e06..590e03107 100644 --- a/src/daemon/handlers/__tests__/snapshot-handler.test.ts +++ b/src/daemon/handlers/__tests__/snapshot-handler.test.ts @@ -1286,6 +1286,7 @@ test('captureSnapshot retries pending tap outcome before post-gesture stabilizat }; session.postGestureStabilization = { action: 'click', + positionals: [], markedAt: Date.now(), }; @@ -1361,6 +1362,7 @@ test('captureSnapshot composes post-gesture stabilization with Android freshness }; session.postGestureStabilization = { action: 'click', + positionals: [], markedAt: Date.now(), }; diff --git a/src/daemon/interaction-outcome-policy.ts b/src/daemon/interaction-outcome-policy.ts index 3010a0830..5b6310e16 100644 --- a/src/daemon/interaction-outcome-policy.ts +++ b/src/daemon/interaction-outcome-policy.ts @@ -177,6 +177,23 @@ export function classifyInteractionSurfaceChange( return 'changed'; } +/** + * Shared rect-tolerance comparison for the surface-stability checks in this + * module. Entry rects are already rounded by `buildInteractionSurfaceEntry`, + * so one `RECT_TOLERANCE_PX` band absorbs residual drift consistently. + */ +function rectsWithinTolerance( + a: Pick, + b: Pick, +): boolean { + return ( + Math.abs(a.x - b.x) <= RECT_TOLERANCE_PX && + Math.abs(a.y - b.y) <= RECT_TOLERANCE_PX && + Math.abs(a.width - b.width) <= RECT_TOLERANCE_PX && + Math.abs(a.height - b.height) <= RECT_TOLERANCE_PX + ); +} + export function areInteractionSurfaceSignaturesStable( left: InteractionSurfaceSignature, right: InteractionSurfaceSignature, @@ -186,10 +203,7 @@ export function areInteractionSurfaceSignaturesStable( const a = left[index]; const b = right[index]; if (!a || !b || a.key !== b.key) return false; - if (Math.abs(a.x - b.x) > RECT_TOLERANCE_PX) return false; - if (Math.abs(a.y - b.y) > RECT_TOLERANCE_PX) return false; - if (Math.abs(a.width - b.width) > RECT_TOLERANCE_PX) return false; - if (Math.abs(a.height - b.height) > RECT_TOLERANCE_PX) return false; + if (!rectsWithinTolerance(a, b)) return false; } return true; } @@ -252,14 +266,7 @@ export function classifyBaselineSurfaceEvidence( continue; } shared += 1; - if ( - 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'; - } + if (!rectsWithinTolerance(seen, now)) return 'changed'; } if (shared === 0) return 'ambiguous'; const addedSinceBaseline = after.size > shared; @@ -278,11 +285,11 @@ export function classifyBaselineSurfaceEvidence( */ function identifiedContent( signature: InteractionSurfaceSignature, -): Map { - const content = new Map(); +): 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 }); + if (!content.has(entry.identity)) content.set(entry.identity, entry); } return content; } @@ -300,6 +307,13 @@ function identifiedContent( * tolerance) vetoes that shape: any appeared or vanished real element kills * the claim. Scope drift between baseline and capture vetoes too — silence * is the safe failure mode for a message that steers the agent's next move. + * + * Matches on `key`, not the flip-tolerant `identity` that + * `classifyBaselineSurfaceEvidence` uses — deliberately the opposite choice. + * That oracle must not lose evidence to a volatile-state flip; this runs only + * after it already returned `'unchanged'`, and a veto wants precision over + * recall: any flip makes the keys mismatch and returns `false`, withholding + * the claim rather than falsifying anything. */ export function haveIdenticalDiscriminatingSurfaces( left: InteractionSurfaceSignature, @@ -313,14 +327,7 @@ export function haveIdenticalDiscriminatingSurfaces( for (const entry of leftEntries) { const other = rightByKey.get(entry.key); if (!other) return false; - if ( - Math.abs(entry.x - other.x) > RECT_TOLERANCE_PX || - Math.abs(entry.y - other.y) > RECT_TOLERANCE_PX || - Math.abs(entry.width - other.width) > RECT_TOLERANCE_PX || - Math.abs(entry.height - other.height) > RECT_TOLERANCE_PX - ) { - return false; - } + if (!rectsWithinTolerance(entry, other)) return false; } return true; } diff --git a/src/daemon/types.ts b/src/daemon/types.ts index 7eb041fcb..f436e9ea7 100644 --- a/src/daemon/types.ts +++ b/src/daemon/types.ts @@ -18,7 +18,7 @@ import type { DaemonRequest as WireRequest, } from '@agent-device/kernel/contracts'; import type { DeviceInfo, Platform, PlatformSelector } from '@agent-device/kernel/device'; -import type { Rect, SnapshotState } from '@agent-device/kernel/snapshot'; +import type { Rect, SnapshotState, SnapshotCaptureBackend } from '@agent-device/kernel/snapshot'; import type { ExecBackgroundResult, ExecResult } from '../utils/exec.ts'; // Type-only import; erased at runtime. ref-frame.ts imports SessionState from // here, so this back-edge must stay type-only to avoid a runtime cycle. @@ -239,9 +239,9 @@ export type InteractionSurfaceEntry = { export type PostGestureStabilization = { action: string; - /** The gesture's own positionals (e.g. scroll direction) — wording input for - * the #1600 no-effect warning; never re-dispatched. */ - positionals?: string[]; + /** The gesture's own positionals — wording input for the #1600 no-effect + * warning; never re-dispatched. Always set by the only writer. */ + positionals: string[]; markedAt: number; /** * Pre-gesture interaction-surface signature, captured from the session's @@ -261,7 +261,7 @@ export type PostGestureStabilization = { * a different backend can only be re-baselined against, never concluded from * (#1569). */ - baselineBackend?: string; + baselineBackend?: SnapshotCaptureBackend; }; export type PendingInteractionOutcome = {