From afface4c52aad0d9ec52f11f52d8cff2fb164333 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Wed, 5 Aug 2026 17:52:37 +0200 Subject: [PATCH 1/2] fix(daemon): corroborate the no-effect claim against the rebased baseline MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #1620. `capturePostGestureStabilizedResult` keeps a local baseline that it REBASES when the capture backend changes mid-poll: backends disagree about which nodes exist, so a cross-backend pair says nothing about the gesture and #1569 adopts the new capture as the baseline rather than concluding from it. The verdict is then computed from that comparable pair. `buildAcceptedStabilizedResult` corroborated against `pending.baselineSignature` instead — the ORIGINAL pre-gesture signature, the one the loop had just discarded as incomparable. So on any backend fallback the agent-facing claim re-introduced exactly the comparison #1569 exists to prevent, and `haveIdenticalDiscriminatingSurfaces` demands set equality, which two backends' views of one screen never satisfy. The consequence was that the warning could not fire on any screen whose capture plan falls back — i.e. every hostile screen, which is the only kind it was built for. #1600's motivating case, element-18, was a Bluesky feed; measured live on a seeded Bluesky fixture, the capture falls back to private-ax and truncates at depth 56, and no no-effect gesture ever produced the warning. Fix: corroborate against the loop's live `baselineSignature`. When no rebase happened it is `pending`'s, so the #1601 veto is unchanged for the ordinary case; when a rebase happened it is the comparable same-backend capture, which is the only pair either check should ever read. Red evidence: the new test models what two backends actually return for one screen — private-ax additionally reports a scrolled-away row the tree backend prunes — with every post-gesture capture identical to the others, so the gesture provably moved nothing. Against the previous line it fails ("a proven-inert gesture must still be reported after the capture backend falls back"); with the fix, 21/21. An earlier draft of that test used identical node content for both backends and passed against the unfixed code — vacuous, since the whole defect is that the two views differ. Recorded here because the green run looked identical either way. --- .../post-gesture-stabilization.test.ts | 58 +++++++++++++++++++ src/daemon/post-gesture-stabilization.ts | 15 ++++- 2 files changed, 71 insertions(+), 2 deletions(-) diff --git a/src/daemon/__tests__/post-gesture-stabilization.test.ts b/src/daemon/__tests__/post-gesture-stabilization.test.ts index 5b65f79af..46f0f4453 100644 --- a/src/daemon/__tests__/post-gesture-stabilization.test.ts +++ b/src/daemon/__tests__/post-gesture-stabilization.test.ts @@ -158,6 +158,64 @@ test('capturePostGestureStabilizedResult keeps polling past the normal deadline assert.ok(captureCount > 8, `expected sustained polling, saw ${captureCount} captures`); }); +test('a backend flip mid-poll still yields the no-effect claim (#1620)', async () => { + // The screens this warning exists for are the hostile ones — #1600's + // element-18 was a Bluesky feed — and those are exactly the screens whose + // capture plan falls back mid-sequence. The loop already handles that: on a + // backend change it REBASES (#1569) and keeps polling against a comparable + // pair. The corroboration then read `pending.baselineSignature` instead, i.e. + // the pre-gesture signature from the OTHER backend, so set equality could + // never hold and the claim was vetoed on every fallback. + vi.useFakeTimers(); + const session = makeSession('ios'); + // Pre-gesture baseline captured by the TREE backend. The two backends do not + // agree on which nodes exist — that disagreement is the entire premise of + // #1569 — so private-ax additionally reports a scrolled-away row the tree + // backend prunes. Same screen, different view of it. + session.snapshot = makeSnapshotState(pickupSnapshot(500).nodes, { + snapshotQuality: { state: 'healthy', backend: 'tree' }, + }); + markPostGestureStabilization(session, 'scroll', ['up']); + + // Every post-gesture capture comes from private-ax: the penalty armed during + // the gesture. They are byte-identical to EACH OTHER — the gesture genuinely + // moved nothing — while differing from the tree baseline by that extra row. + const privateAxNodes = [ + ...pickupSnapshot(500).nodes, + { + index: 2, + parentIndex: 0, + type: 'StaticText', + identifier: 'scrolled-away-row', + label: 'Above the fold', + rect: { x: 20, y: -80, width: 200, height: 44 }, + }, + ]; + const capture = vi.fn(async () => + makeSnapshotState(privateAxNodes, { + snapshotQuality: { state: 'recovered', backend: 'private-ax' }, + }), + ); + + const resultPromise = withDiagnosticsScope( + {}, + async () => + await capturePostGestureStabilizedResult({ + session, + capture, + readSnapshot: (snapshot) => snapshot, + }), + ); + await vi.advanceTimersByTimeAsync(10_000); + const result = await resultPromise; + + assert.equal( + result.gestureNoEffect?.action, + 'scroll', + 'a proven-inert gesture must still be reported after the capture backend falls back', + ); +}); + 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 diff --git a/src/daemon/post-gesture-stabilization.ts b/src/daemon/post-gesture-stabilization.ts index a740a1ab7..01c925398 100644 --- a/src/daemon/post-gesture-stabilization.ts +++ b/src/daemon/post-gesture-stabilization.ts @@ -239,7 +239,7 @@ export async function capturePostGestureStabilizedResult(params: { } clearPostGestureStabilization(session); emitPostGestureSettleDiagnostic(verdict, pending.action, attempts, elapsedMs); - return buildAcceptedStabilizedResult(verdict, pending, current); + return buildAcceptedStabilizedResult(verdict, pending, current, baselineSignature); } previous = current; } @@ -280,15 +280,26 @@ export function formatGestureNoEffectWarning(action: string, positionals: string * corroboration (`haveIdenticalDiscriminatingSurfaces`): the verdict alone is * subset-tolerant, and a successful scroll that replaced every list cell * under fixed chrome still reads accept-stale (#1601 review P1). + * + * Corroborates against the loop's CURRENT baseline, not `pending`'s original + * one (#1620). When the capture backend flips mid-poll the loop rebases — + * `pending`'s pre-gesture signature came from a backend that does not agree + * with this one about which nodes exist, so #1569 already ruled it out for the + * verdict. Reading it back here re-introduced exactly that comparison, and set + * equality across two backends never holds: the corroboration was guaranteed to + * veto on any screen whose capture plan fell back — which is every hostile + * screen, i.e. the ones the warning exists for (#1600's element-18 was a + * Bluesky feed). */ function buildAcceptedStabilizedResult( verdict: 'trust' | 'accept-stale', pending: NonNullable, current: CapturedSurface, + baselineSignature: InteractionSurfaceSignature | undefined, ): PostGestureStabilizedResult { const corroborated = verdict === 'accept-stale' && - haveIdenticalDiscriminatingSurfaces(pending.baselineSignature ?? [], current.signature); + haveIdenticalDiscriminatingSurfaces(baselineSignature ?? [], current.signature); if (!corroborated) return { value: current.value }; return { value: current.value, From 408a020c583938f2ae9a1d4177bfd47c71fcc8cc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Wed, 5 Aug 2026 18:31:24 +0200 Subject: [PATCH 2/2] fix(daemon): never rebase an invented baseline MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review P1 on #1622, and a false-positive path the previous commit introduced. `markPostGestureStabilization` records `baselineSignature: []` when the session has no pre-gesture snapshot. `decidePostGestureStabilityVerdict` already guards that correctly (`!baselineSignature?.length` -> 'trust'), but the backend-rebase branch tested plain truthiness, and `[]` is truthy. So the first capture from a different backend replaced "no before-state" with a post-gesture signature — inventing a baseline. That was harmless while corroboration read `pending.baselineSignature` (still `[]`, and `haveIdenticalDiscriminatingSurfaces` returns false on an empty side), but the previous commit pointed corroboration at the loop's live baseline, so the invented one became reachable: the loop could agree with itself and report a gesture inert with nothing to compare against. Rebase only a non-empty baseline, matching the guard the verdict already uses. Red evidence: with no `session.snapshot` and steady private-AX captures, restoring the truthiness test fails the new case ("a no-effect claim needs a real pre-gesture baseline, never one the loop invented for itself"); 22/22 with `?.length`. --- .../post-gesture-stabilization.test.ts | 38 +++++++++++++++++++ src/daemon/post-gesture-stabilization.ts | 8 +++- 2 files changed, 45 insertions(+), 1 deletion(-) diff --git a/src/daemon/__tests__/post-gesture-stabilization.test.ts b/src/daemon/__tests__/post-gesture-stabilization.test.ts index 46f0f4453..f60417ee5 100644 --- a/src/daemon/__tests__/post-gesture-stabilization.test.ts +++ b/src/daemon/__tests__/post-gesture-stabilization.test.ts @@ -216,6 +216,44 @@ test('a backend flip mid-poll still yields the no-effect claim (#1620)', async ( ); }); +test('no pre-gesture snapshot means no no-effect claim, even after a backend flip (#1622 P1)', async () => { + // `markPostGestureStabilization` records an EMPTY baseline when the session + // has no pre-gesture snapshot, and `[]` is truthy. Rebasing it on a backend + // flip would swap "no before-state" for a post-gesture capture, inventing the + // very evidence the claim is supposed to rest on — the loop would then agree + // with itself and report a gesture as inert with nothing to compare against. + vi.useFakeTimers(); + const session = makeSession('ios'); + session.snapshot = undefined; // nothing captured before the gesture + markPostGestureStabilization(session, 'scroll', ['up']); + + // Steady private-AX captures: quiet, self-consistent, and a different backend + // from the (absent) baseline, so the rebase branch is reached. + const capture = vi.fn(async () => + makeSnapshotState(pickupSnapshot(500).nodes, { + snapshotQuality: { state: 'recovered', backend: 'private-ax' }, + }), + ); + + const resultPromise = withDiagnosticsScope( + {}, + async () => + await capturePostGestureStabilizedResult({ + session, + capture, + readSnapshot: (snapshot) => snapshot, + }), + ); + await vi.advanceTimersByTimeAsync(10_000); + const result = await resultPromise; + + assert.equal( + result.gestureNoEffect, + undefined, + 'a no-effect claim needs a real pre-gesture baseline, never one the loop invented for itself', + ); +}); + 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 diff --git a/src/daemon/post-gesture-stabilization.ts b/src/daemon/post-gesture-stabilization.ts index 01c925398..a7bfba445 100644 --- a/src/daemon/post-gesture-stabilization.ts +++ b/src/daemon/post-gesture-stabilization.ts @@ -214,7 +214,13 @@ export async function capturePostGestureStabilizedResult(params: { // 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) { + // `?.length`, not truthiness: `markPostGestureStabilization` records an + // EMPTY signature when there was no pre-gesture snapshot, and `[]` is + // truthy. Rebasing that would replace "no before-state" with a + // post-gesture capture — inventing a baseline the gesture is then judged + // against, which `decidePostGestureStabilityVerdict` (guarding on + // `?.length`) had deliberately refused to do (#1622 review P1). + if (baselineSignature?.length && baselineBackend !== current.backend) { emitDiagnostic({ level: 'debug', phase: 'post_gesture_snapshot_baseline_rebased',