Skip to content

Commit 7af45ba

Browse files
committed
fix: require full-surface evidence before the no-effect claim (#1601 review P1)
accept-stale alone is subset-tolerant by design: a successful scroll that replaced every list cell under fixed chrome still classifies 'unchanged' on the shared chrome alone, and #1573 has live evidence of that shape. The agent-facing claim now additionally requires every discriminating entry of the quiet capture to match the baseline exactly, in both directions (haveIdenticalDiscriminatingSurfaces): any appeared or vanished real element — including scope drift — vetoes the warning. Silence is the safe failure mode for a message that steers the agent's next move. Red evidence: the new fixed-chrome + replaced-list regression test fails on the previous PR commit (gestureNoEffect wrongly present), passes with the gate.
1 parent b9c1948 commit 7af45ba

4 files changed

Lines changed: 121 additions & 5 deletions

File tree

src/daemon/__tests__/post-gesture-stabilization-fixtures.ts

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,45 @@ export function pickupSnapshotWithExtraText(y = 500) {
6262
};
6363
}
6464

65+
/**
66+
* Fixed tab-bar chrome + a list whose visible cells sit at stable row rects.
67+
* Two captures with DIFFERENT cell ids model a successful scroll that
68+
* replaced every list row while the chrome (a discriminating, shared,
69+
* unmoved entry) stayed put: `classifyBaselineSurfaceEvidence` reads
70+
* 'unchanged' from the shared chrome alone (#1601 review P1) — the exact
71+
* accept-stale false negative that must never surface as an agent-facing
72+
* no-effect claim.
73+
*/
74+
export function chromeWithListSnapshot(cellIds: [string, string]) {
75+
return makeSnapshotState([
76+
{ index: 0, type: 'Application', label: 'App', rect: { x: 0, y: 0, width: 390, height: 844 } },
77+
{
78+
index: 1,
79+
parentIndex: 0,
80+
type: 'Button',
81+
identifier: 'tab-home',
82+
label: 'Home',
83+
rect: { x: 0, y: 800, width: 195, height: 44 },
84+
},
85+
{
86+
index: 2,
87+
parentIndex: 0,
88+
type: 'Cell',
89+
identifier: cellIds[0],
90+
label: cellIds[0],
91+
rect: { x: 0, y: 100, width: 390, height: 60 },
92+
},
93+
{
94+
index: 3,
95+
parentIndex: 0,
96+
type: 'Cell',
97+
identifier: cellIds[1],
98+
label: cellIds[1],
99+
rect: { x: 0, y: 160, width: 390, height: 60 },
100+
},
101+
]);
102+
}
103+
65104
export function applicationRootNode() {
66105
return {
67106
ref: 'e-root',

src/daemon/__tests__/post-gesture-stabilization.test.ts

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import {
99
markPostGestureStabilization,
1010
} from '../post-gesture-stabilization.ts';
1111
import {
12+
chromeWithListSnapshot,
1213
deliverySnapshot,
1314
keyboardWindowNodes,
1415
makeSession,
@@ -157,6 +158,38 @@ test('capturePostGestureStabilizedResult keeps polling past the normal deadline
157158
assert.ok(captureCount > 8, `expected sustained polling, saw ${captureCount} captures`);
158159
});
159160

161+
test('a replaced list under fixed chrome accepts stale but never claims no-effect (#1601 P1)', async () => {
162+
// The reviewer's counterexample: a SUCCESSFUL scroll swapped every list
163+
// cell while the tab-bar chrome (discriminating, shared, unmoved) kept the
164+
// subset-tolerant classifier at 'unchanged'. The loop may still accept the
165+
// stale read — but the agent-facing no-effect claim must be vetoed by the
166+
// unmatched discriminating cells on both sides.
167+
vi.useFakeTimers();
168+
const session = makeSession('ios');
169+
session.snapshot = chromeWithListSnapshot(['row-1', 'row-2']);
170+
markPostGestureStabilization(session, 'scroll');
171+
172+
const capture = vi.fn(async () => chromeWithListSnapshot(['row-3', 'row-4']));
173+
174+
const resultPromise = withDiagnosticsScope({}, async () => {
175+
const result = await capturePostGestureStabilizedResult({
176+
session,
177+
capture,
178+
readSnapshot: (snapshot) => snapshot,
179+
});
180+
return {
181+
result,
182+
staleAccepts: countDiagnosticEventsByPhase(['post_gesture_snapshot_stale_accept']),
183+
};
184+
});
185+
186+
await vi.advanceTimersByTimeAsync(10_000);
187+
const { result, staleAccepts } = await resultPromise;
188+
189+
assert.equal(staleAccepts, 1);
190+
assert.equal(result.gestureNoEffect, undefined);
191+
});
192+
160193
test('formatGestureNoEffectWarning names the gesture and the raw-drag escape hatch', () => {
161194
const scrollWarning = formatGestureNoEffectWarning('scroll', ['down', '1']);
162195
assert.match(scrollWarning, /scroll down produced no visible change/);

src/daemon/interaction-outcome-policy.ts

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -251,6 +251,44 @@ export function classifyBaselineSurfaceEvidence(
251251
return discriminatingOverlap > 0 ? 'unchanged' : 'ambiguous';
252252
}
253253

254+
/**
255+
* Full-surface agreement over DISCRIMINATING entries, in BOTH directions —
256+
* the stronger bar an agent-facing no-effect claim needs (#1601 review P1).
257+
*
258+
* `classifyBaselineSurfaceEvidence` is deliberately subset-tolerant for the
259+
* distrust loop, where a false 'unchanged' only buys extra polling. But a
260+
* fixed-chrome screen whose list cells were fully replaced by a SUCCESSFUL
261+
* scroll classifies 'unchanged' on the shared chrome alone — new cells are
262+
* absent from the baseline and silently ignored. Requiring the discriminating
263+
* entry sets to match exactly (same keys both ways, every rect within
264+
* tolerance) vetoes that shape: any appeared or vanished real element kills
265+
* the claim. Scope drift between baseline and capture vetoes too — silence
266+
* is the safe failure mode for a message that steers the agent's next move.
267+
*/
268+
export function haveIdenticalDiscriminatingSurfaces(
269+
left: InteractionSurfaceSignature,
270+
right: InteractionSurfaceSignature,
271+
): boolean {
272+
const leftEntries = left.filter((entry) => entry.discriminating);
273+
const rightEntries = right.filter((entry) => entry.discriminating);
274+
if (leftEntries.length === 0 || leftEntries.length !== rightEntries.length) return false;
275+
// Keys carry an occurrence ordinal (`|#N`), so a map by key is lossless.
276+
const rightByKey = new Map(rightEntries.map((entry) => [entry.key, entry]));
277+
for (const entry of leftEntries) {
278+
const other = rightByKey.get(entry.key);
279+
if (!other) return false;
280+
if (
281+
Math.abs(entry.x - other.x) > RECT_TOLERANCE_PX ||
282+
Math.abs(entry.y - other.y) > RECT_TOLERANCE_PX ||
283+
Math.abs(entry.width - other.width) > RECT_TOLERANCE_PX ||
284+
Math.abs(entry.height - other.height) > RECT_TOLERANCE_PX
285+
) {
286+
return false;
287+
}
288+
}
289+
return true;
290+
}
291+
254292
function supportsInteractionOutcomePolicy(session: SessionState): boolean {
255293
return isMobilePlatform(session.device);
256294
}

src/daemon/post-gesture-stabilization.ts

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import {
77
areInteractionSurfaceSignaturesStable,
88
buildInteractionSurfaceSignature,
99
classifyBaselineSurfaceEvidence,
10+
haveIdenticalDiscriminatingSurfaces,
1011
type InteractionSurfaceSignature,
1112
} from './interaction-outcome-policy.ts';
1213
import type { SessionState } from './types.ts';
@@ -154,10 +155,14 @@ function emitPostGestureSettleDiagnostic(
154155
export type PostGestureStabilizedResult<T> = {
155156
value: T;
156157
/**
157-
* Present when the loop accepted a stale read: the quiet capture PROVABLY
158-
* still equals the pre-gesture baseline after the distrust cap (#1600).
159-
* Callers surface this to the agent — a diagnostics-only verdict let one
160-
* benchmark run burn 40 calls re-issuing scrolls the daemon knew did nothing.
158+
* Present ONLY when the accept-stale verdict is corroborated by full-surface
159+
* evidence: every discriminating entry of the quiet capture matches the
160+
* pre-gesture baseline exactly, in both directions
161+
* (`haveIdenticalDiscriminatingSurfaces`). The bare verdict is NOT enough —
162+
* it is subset-tolerant by design, and a successful scroll that replaced
163+
* every list cell under fixed chrome still reads accept-stale (#1601 review
164+
* P1). Callers surface this to the agent: a diagnostics-only signal let one
165+
* benchmark run burn 40 calls re-issuing scrolls that moved nothing (#1600).
161166
*/
162167
gestureNoEffect?: { action: string; positionals: string[] };
163168
};
@@ -204,7 +209,8 @@ export async function capturePostGestureStabilizedResult<T>(params: {
204209
emitPostGestureSettleDiagnostic(verdict, pending.action, attempts, elapsedMs);
205210
return {
206211
value: current.value,
207-
...(verdict === 'accept-stale'
212+
...(verdict === 'accept-stale' &&
213+
haveIdenticalDiscriminatingSurfaces(pending.baselineSignature ?? [], current.signature)
208214
? {
209215
gestureNoEffect: {
210216
action: pending.action,

0 commit comments

Comments
 (0)