Skip to content

Commit 4ada6aa

Browse files
thymikeethymikee
committed
fix(settle): decide the broad-transition confirmation by the transition alone
The arming bar was `requestedQuietMs >= DEFAULT_STABLE_QUIET_MS`, so the confirmation that #1831 added to hold a modal dismissal for 1.5s was a function of the window the caller asked for rather than of the transition in front of the loop. Three consequences, all reported as #3470: - `--settle-quiet 499` behaved differently from 500, with no rule that explains the boundary - a caller passing `settleQuietMs: 150` (tester-army/e2e) got no guard at all, on exactly the dismissal the guard exists for - a shorter window read as "trust a coherent transitional tree sooner", which is not what `--settle-quiet` asks for: it asks for a tighter verdict, not a different set of transitions to distrust Arming now reads the transition only, and a confirmed replacement is held for the caller's own window, floored at 1.5s. The trigger stays exactly as on main: a complete-viewport post-action projection or a tiny presented baseline, plus fewer than half the node identities carried over. The six #1831 cases run at 500ms and 150ms alike. The provider-backed scenario is a real dialog dismissal, so it now holds the floor at its 25ms window too, which is what that wait is worth on a runner round trip: measured 1510ms over 6 captures, against 1511ms over 6 for the same scenario at the default window on main — the same verdict at the same price, no longer window-dependent.
1 parent faf754a commit 4ada6aa

7 files changed

Lines changed: 343 additions & 114 deletions

File tree

‎packages/command-registry/src/flag-definitions-action.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -210,7 +210,8 @@ export const ACTION_FLAG_DEFINITIONS: readonly FlagDefinition[] = [
210210
type: 'int',
211211
min: 0,
212212
usageLabel: '--settle-quiet <ms>',
213-
usageDescription: 'Settle: quiet window the UI must hold to count as settled (default 500ms)',
213+
usageDescription:
214+
'Settle: quiet window the UI must hold to count as settled (default 500ms). On iOS, a broad screen replacement such as a modal dismissal is confirmed for at least 1500ms whatever window is asked for, within the --timeout settle deadline',
214215
projectConfig: true,
215216
recorded: false,
216217
},

‎src/commands/interaction/runtime/settle-confirmation-floor.test.ts‎

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
import assert from 'node:assert/strict';
2+
import { test } from 'vitest';
3+
import { ref } from './selector-read-utils.ts';
4+
import {
5+
buttonSnapshot,
6+
createSettleDevice,
7+
welcomeSnapshot,
8+
} from './__tests__/settle-device-fixtures.ts';
9+
10+
// #3470: the broad-transition confirmation owns the WAIT, not just the verdict, and it used to
11+
// switch itself off whenever the caller asked for a window under the default. On the interaction
12+
// commands' shared settle path that meant a `settleQuietMs: 150` caller (tester-army/e2e) got no
13+
// guard at all while the default got a 1.5 s one. The same dismissed surface now settles at both
14+
// requested windows: the response still reports the window the caller asked for, and the confirmed
15+
// replacement sets the wait. `stable-capture.test.ts` pins that floor at the loop; this pins it on
16+
// the observation the caller reads.
17+
//
18+
// Budgets are injected (fake clock) — no real waiting.
19+
20+
function pressThatDismissesTheWholeTree() {
21+
const before = buttonSnapshot();
22+
let captures = 0;
23+
const device = createSettleDevice({
24+
stored: before,
25+
captureSnapshot: () => {
26+
captures += 1;
27+
// A ref press resolves against the tree the session already holds and captures nothing before
28+
// the action, so every call here is the settle loop's own sample: capture 1 still shows the
29+
// pre-action tree, captures 2+ the tree the dismissal reveals.
30+
return { snapshot: captures === 1 ? before : welcomeSnapshot() };
31+
},
32+
});
33+
return device;
34+
}
35+
36+
for (const requestedQuietMs of [500, 150]) {
37+
test(`a broad replacement on the interaction route holds the floor at ${requestedQuietMs}ms requested`, async () => {
38+
const result = await pressThatDismissesTheWholeTree().interactions.press(ref('@e1'), {
39+
session: 'default',
40+
settle: { quietMs: requestedQuietMs, timeoutMs: 5_000 },
41+
});
42+
43+
const settle = result.settle;
44+
assert.ok(settle);
45+
assert.equal(settle.settled, true);
46+
// The window the caller asked for is what the response reports…
47+
assert.equal(settle.quietMs, requestedQuietMs);
48+
// …while the confirmed replacement sets the wait's floor (#3470).
49+
assert.ok(
50+
settle.waitedMs >= 1_500,
51+
`confirmation did not extend the settle: waitedMs=${settle.waitedMs}`,
52+
);
53+
assert.ok(settle.diff);
54+
});
55+
}

‎src/commands/interaction/runtime/settle.test.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -307,15 +307,15 @@ test('longpress --settle rides the same observation path', async () => {
307307
const result = await device.interactions.longPress(ref('@e1'), {
308308
session: 'default',
309309
durationMs: 400,
310-
settle: { quietMs: 100, timeoutMs: 1_000 },
310+
settle: { quietMs: 100, timeoutMs: 2_000 },
311311
});
312312

313313
assert.equal(result.kind, 'ref');
314314
const settle = result.settle;
315315
assert.ok(settle);
316316
assert.equal(settle.settled, true);
317317
assert.equal(settle.quietMs, 100);
318-
assert.equal(settle.timeoutMs, 1_000);
318+
assert.equal(settle.timeoutMs, 2_000);
319319
assert.ok(settle.diff);
320320
});
321321

‎src/commands/interaction/runtime/stable-capture.test.ts‎

Lines changed: 156 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,149 @@ const BROAD_TRANSITION_PARAMS = {
2222
broadTransitionBaselineNodes: elementThreadsNoticeSnapshot.nodes,
2323
};
2424

25+
// Each #1831 case below runs at BOTH windows. The confirmation used to switch itself off for a
26+
// requested window under the default, so `--settle-quiet 150` — what tester-army/e2e passes — got no
27+
// guard at all while 500 got a 1.5 s one (#3470).
28+
for (const requestedQuietMs of [500, 150]) {
29+
const confirmationFloor = Math.max(requestedQuietMs, 1_500);
30+
31+
test(`settle confirms a broad screen replacement beyond a self-consistent transitional tree (quiet ${requestedQuietMs}ms)`, async () => {
32+
const { runtime } = transitionRuntime();
33+
34+
const outcome = await runStableCaptureLoop(
35+
runtime,
36+
{ session: 'default' },
37+
{ ...BROAD_TRANSITION_PARAMS, quietMs: requestedQuietMs },
38+
);
39+
40+
assert.equal(outcome.settled, true);
41+
assert.equal(
42+
outcome.lastCapture?.snapshot.nodes.some((node) => node.label === 'action file'),
43+
false,
44+
);
45+
assert.ok(
46+
outcome.waitedMs >= confirmationFloor,
47+
`settled transitional tree after ${outcome.waitedMs}ms`,
48+
);
49+
});
50+
51+
test(`settle confirms a broad screen replacement when capture recovers to private AX (quiet ${requestedQuietMs}ms)`, async () => {
52+
const { runtime } = transitionRuntime({ captureBackend: 'private-ax' });
53+
54+
const outcome = await runStableCaptureLoop(
55+
runtime,
56+
{ session: 'default' },
57+
{ ...BROAD_TRANSITION_PARAMS, quietMs: requestedQuietMs },
58+
);
59+
60+
assert.equal(outcome.settled, true);
61+
assert.equal(
62+
outcome.lastCapture?.snapshot.nodes.some((node) => node.label === 'action file'),
63+
false,
64+
);
65+
assert.ok(
66+
outcome.waitedMs >= confirmationFloor,
67+
`settled transitional tree after ${outcome.waitedMs}ms`,
68+
);
69+
});
70+
71+
test(`settle confirms a broad replacement that begins after the first post-action capture (quiet ${requestedQuietMs}ms)`, async () => {
72+
const { runtime } = transitionRuntime({
73+
captureBackend: 'private-ax',
74+
firstCaptureKeepsBaseline: true,
75+
settledAtMs: 1_200,
76+
});
77+
78+
const outcome = await runStableCaptureLoop(
79+
runtime,
80+
{ session: 'default' },
81+
{ ...BROAD_TRANSITION_PARAMS, quietMs: requestedQuietMs },
82+
);
83+
84+
assert.equal(outcome.settled, true);
85+
assert.equal(
86+
outcome.lastCapture?.snapshot.nodes.some((node) => node.label === 'action file'),
87+
false,
88+
);
89+
assert.ok(
90+
outcome.waitedMs >= confirmationFloor,
91+
`settled delayed transition after ${outcome.waitedMs}ms`,
92+
);
93+
});
94+
95+
test(`settle confirms against the pre-action tree when the stored snapshot already advanced (quiet ${requestedQuietMs}ms)`, async () => {
96+
const { runtime } = transitionRuntime({
97+
sessionSnapshot: { ...elementTransientRoomSnapshot, backend: undefined, producer: undefined },
98+
settledAtMs: 1_200,
99+
});
100+
101+
const outcome = await runStableCaptureLoop(
102+
runtime,
103+
{ session: 'default' },
104+
{ ...BROAD_TRANSITION_PARAMS, quietMs: requestedQuietMs },
105+
);
106+
107+
assert.equal(outcome.settled, true);
108+
assert.equal(
109+
outcome.lastCapture?.snapshot.nodes.some((node) => node.label === 'action file'),
110+
false,
111+
);
112+
assert.ok(
113+
outcome.waitedMs >= confirmationFloor,
114+
`settled advanced-session transition after ${outcome.waitedMs}ms`,
115+
);
116+
});
117+
118+
test(`settle confirms an iOS broad replacement when capture omits snapshot backend provenance (quiet ${requestedQuietMs}ms)`, async () => {
119+
const { runtime } = transitionRuntime({
120+
omitSnapshotBackend: true,
121+
settledAtMs: 1_200,
122+
});
123+
124+
const outcome = await runStableCaptureLoop(
125+
runtime,
126+
{ session: 'default' },
127+
{
128+
...BROAD_TRANSITION_PARAMS,
129+
quietMs: requestedQuietMs,
130+
// Presented iOS modals can be complete interaction captures without retaining the
131+
// Application root in their projection.
132+
broadTransitionBaselineNodes: elementThreadsNoticeSnapshot.nodes.slice(1),
133+
},
134+
);
135+
136+
assert.equal(outcome.settled, true);
137+
assert.equal(
138+
outcome.lastCapture?.snapshot.nodes.some((node) => node.label === 'action file'),
139+
false,
140+
);
141+
assert.ok(
142+
outcome.waitedMs >= confirmationFloor,
143+
`settled backend-less transition after ${outcome.waitedMs}ms`,
144+
);
145+
});
146+
147+
test(`settle confirms a tiny modal replacement through a partial post-action projection (quiet ${requestedQuietMs}ms)`, async () => {
148+
const { runtime } = transitionRuntime({ omitViewportRoot: true, settledAtMs: 1_200 });
149+
150+
const outcome = await runStableCaptureLoop(
151+
runtime,
152+
{ session: 'default' },
153+
{ ...BROAD_TRANSITION_PARAMS, quietMs: requestedQuietMs },
154+
);
155+
156+
assert.equal(outcome.settled, true);
157+
assert.equal(
158+
outcome.lastCapture?.snapshot.nodes.some((node) => node.label === 'action file'),
159+
false,
160+
);
161+
assert.ok(
162+
outcome.waitedMs >= confirmationFloor,
163+
`settled partial transitional tree after ${outcome.waitedMs}ms`,
164+
);
165+
});
166+
}
167+
25168
test('regular captures settle from visible semantics while offscreen rows churn', async () => {
26169
let elapsedMs = 0;
27170
const clock = {
@@ -203,8 +346,11 @@ test('settle keeps the default quiet window for a larger partial projection', as
203346
assert.ok(outcome.waitedMs < 800, `partial projection settled after ${outcome.waitedMs}ms`);
204347
});
205348

206-
test('settle honors an explicitly shorter quiet window across a broad replacement', async () => {
207-
const { runtime } = transitionRuntime();
349+
test('settle confirms a broad replacement on a caller window shorter than the confirmation floor', async () => {
350+
// The counterpart of the loop above at the tightest window a caller realistically asks for. The
351+
// 25 ms cadence still applies — the loop keeps sampling at that grid — but the confirmation floor,
352+
// not the window, decides when a broad replacement is trusted (#3470).
353+
const { runtime } = transitionRuntime({ settledAtMs: 1_200 });
208354

209355
const outcome = await runStableCaptureLoop(
210356
runtime,
@@ -213,8 +359,14 @@ test('settle honors an explicitly shorter quiet window across a broad replacemen
213359
);
214360

215361
assert.equal(outcome.settled, true);
216-
assert.ok(outcome.captures <= 3, `short quiet window spent ${outcome.captures} captures`);
217-
assert.ok(outcome.waitedMs < 800, `short quiet window settled after ${outcome.waitedMs}ms`);
362+
assert.equal(
363+
outcome.lastCapture?.snapshot.nodes.some((node) => node.label === 'action file'),
364+
false,
365+
);
366+
assert.ok(
367+
outcome.waitedMs >= 1_500,
368+
`short quiet window switched the confirmation off: settled after ${outcome.waitedMs}ms`,
369+
);
218370
});
219371

220372
test('settle keeps the default quiet window for a backend-less overlapping local mutation', async () => {

‎src/commands/interaction/runtime/stable-capture.ts‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,12 @@ const QUIET_DEADLINE_EPSILON_MS = 2;
4141
// Broad replacements (for example, dismissing a modal into a room) can expose a coherent but
4242
// transitional AX tree for the default quiet window. Confirm those transitions longer without
4343
// adding latency to local mutations whose pre/post trees substantially overlap.
44+
//
45+
// The confirmation is decided by the transition alone: a caller asking for a shorter quiet window
46+
// asked for a tighter verdict, not for a different set of transitions to distrust. It used to arm
47+
// only at `requestedQuietMs >= DEFAULT_STABLE_QUIET_MS`, which made `--settle-quiet 499` behave
48+
// differently from 500 and left `settleQuietMs: 150` callers (tester-army/e2e) with no guard at all
49+
// (#3470). A confirmed replacement is held for the caller's own window, floored at 1.5 s.
4450
const BROAD_TRANSITION_CONFIRMATION_QUIET_MS = 1_500;
4551
export const DEFAULT_STABLE_QUIET_MS = 500;
4652
export const DEFAULT_STABLE_TIMEOUT_MS = 10_000;
@@ -252,7 +258,7 @@ function stableCaptureNextQuietWindowMs(params: {
252258
requestedQuietMs: number;
253259
currentQuietMs: number;
254260
}): number {
255-
if (params.requestedQuietMs < DEFAULT_STABLE_QUIET_MS || !params.baseline) {
261+
if (!params.baseline) {
256262
return params.currentQuietMs;
257263
}
258264
const replacementSignal = stableCaptureReplacementSignal(params.snapshot, params.appBundleId);

‎src/commands/post-action-observation-grammar.ts‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,10 @@ const settleFields = () => ({
3333
settle: booleanField(
3434
'After the action, wait for the UI to go quiet and return the settled diff vs the pre-action tree in the same response. Best-effort; never fails the action.',
3535
),
36-
settleQuietMs: integerField('Settle: quiet window in milliseconds (default 500).', { min: 0 }),
36+
settleQuietMs: integerField(
37+
'Settle: quiet window in milliseconds (default 500). On iOS, a broad screen replacement such as a modal dismissal is confirmed for at least 1500 whatever window is asked for, within the timeoutMs settle deadline.',
38+
{ min: 0 },
39+
),
3740
timeoutMs: integerField('Settle: wait deadline in milliseconds (default 10000).', { min: 1 }),
3841
});
3942

0 commit comments

Comments
 (0)