Skip to content

Commit 5a49465

Browse files
committed
fix: park the flaky retry scenario instead of declaring it a divergence
Run 29510020718 fired the stale-declaration guard on its first outing and caught my own mistake. tap-retry-if-no-change measured tapRetries=0 in run 29504440599 and tapRetries=1 in 29510020718 — same flow, same commit. So it is not vacuous as #1300 originally claimed: it is NON-DETERMINISTIC. The tap sometimes holds the hierarchy signature still and sometimes does not, because the fixture home screen carries live content. That exposes a real limit of the mechanism added in the previous commit: knownDivergence assumes the divergence REPRODUCES. A declared-but-flaky scenario flips between known-divergence (green) and stale-declaration (red) at random — a coin-flip scheduled job, which is worse than no scenario because it teaches people to ignore the differential. So the scenario is parked, not declared. The flow and the tapRetries invariant stay implemented and unit-tested, so the fix PR only re-adds the scenario once the fixture has an inert control. retryIfNoChange therefore has NO device coverage right now — tracked in #1300 and stated plainly rather than disguised by a green run. A test keeps it out of the active set until then. #1300 updated with the corrected diagnosis and both runs' evidence. Active differential: settle-after-tap (declared divergence, #1299), percent-swipe and optional-warned-not-failed (both enforced, pass/pass on real devices).
1 parent 51aed14 commit 5a49465

2 files changed

Lines changed: 28 additions & 29 deletions

File tree

scripts/maestro-conformance/differential/invariants.test.ts

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -124,11 +124,22 @@ test('a trace whose taps record no metric reports no-data rather than passing',
124124
assert.equal(result.status, 'no-data');
125125
});
126126

127-
test('the retry scenario asserts the retry path ran', () => {
127+
// tap-retry-if-no-change is parked (#1300): tapRetries measured 0 then 1 across
128+
// identical runs, so it is flaky rather than divergent. A knownDivergence
129+
// declaration assumes the failure reproduces, so declaring a flaky scenario
130+
// would make the schedule flip red at random. Assert it stays out of the active
131+
// set until it has an inert control — re-adding it without one silently returns
132+
// the coin-flip.
133+
test('the flaky retry scenario is parked, not declared as a divergence', () => {
128134
const retry = DIFFERENTIAL_SCENARIOS.find((s) => s.id === 'tap-retry-if-no-change');
129-
const invariant = retry?.engineInvariants?.[0];
130-
assert.ok(invariant, 'retry scenario must prove a retry happened, not assume it');
131-
assert.equal(invariant?.kind, 'metricAtLeast');
135+
assert.equal(retry, undefined, 'tap-retry-if-no-change must stay parked until #1300 gives it an inert control');
136+
});
137+
138+
// The invariant itself stays implemented and proven, so the fix PR only has to
139+
// re-add the scenario rather than rebuild the detector.
140+
test('the tapRetries invariant remains implemented for when the scenario returns', () => {
141+
assert.equal(evaluateInvariant([stopWithMetrics('tapOn', { tapRetries: 1 })], RETRY_INVARIANT).status, 'held');
142+
assert.equal(evaluateInvariant([stopWithMetrics('tapOn', { tapRetries: 0 })], RETRY_INVARIANT).status, 'violated');
132143
});
133144

134145
// Truncation-vs-rounding is at most 1px and cannot be observed on a device, so

scripts/maestro-conformance/differential/scenarios.ts

Lines changed: 13 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -110,31 +110,19 @@ export const DIFFERENTIAL_SCENARIOS: DifferentialScenario[] = [
110110
expect: 'pass',
111111
divergenceMeans: 'The swipe behaves differently enough on one engine to fail the flow.',
112112
},
113-
{
114-
id: 'tap-retry-if-no-change',
115-
flow: 'differential/flows/tap-retry-if-no-change.yaml',
116-
comparesAcrossEngines:
117-
'A retryTapIfNoChange tap succeeds on both engines. The flow taps a non-interactive title so the screen cannot change and the retry path is forced; the invariant below then proves a retry actually happened.',
118-
expect: 'pass',
119-
// Outcome parity would pass even if retry never ran. tapRetries is recorded
120-
// per step as a delta (replay-plan-execution.ts), so assert the path executed.
121-
engineInvariants: [
122-
{
123-
kind: 'metricAtLeast',
124-
command: 'tapOn',
125-
metric: 'tapRetries',
126-
min: 1,
127-
because:
128-
'a tap on an unchanging screen must re-tap; zero retries means retryIfNoChange never ran and the scenario proved nothing',
129-
},
130-
],
131-
divergenceMeans: 'agent-device fails a tap upstream completes (or vice versa) under retry-if-no-change.',
132-
knownDivergence: {
133-
reason:
134-
'The invariant caught this scenario being vacuous: both engines pass, but tapRetries was 0, so retryIfNoChange never ran. Tapping the fixture app title does not hold the hierarchy signature still (its ScreenTitle carries a dynamic cart badge), so the no-change path is never forced. Needs an inert fixture control — a defect in the scenario, not the engine.',
135-
tracking: 'https://github.com/callstack/agent-device/issues/1300',
136-
},
137-
},
113+
// PARKED: tap-retry-if-no-change (flow kept at differential/flows/, invariant
114+
// kept implemented and unit-tested) — https://github.com/callstack/agent-device/issues/1300
115+
//
116+
// It is NOT declared as a knownDivergence, deliberately. tapRetries measured 0
117+
// in run 29504440599 and 1 in run 29510020718 with no change to the flow or
118+
// commit: the tap sometimes holds the hierarchy signature still and sometimes
119+
// does not, because the fixture home screen has live content. A declaration
120+
// assumes the divergence REPRODUCES; a flaky one would flip between
121+
// known-divergence (green) and stale-declaration (red) at random, which is
122+
// worse than no scenario — a coin-flip job teaches people to ignore the
123+
// differential. So retryIfNoChange has no device coverage until the fixture
124+
// offers an inert control, and that absence is tracked rather than disguised
125+
// as a green run.
138126
{
139127
id: 'optional-warned-not-failed',
140128
flow: 'differential/flows/optional-warned-not-failed.yaml',

0 commit comments

Comments
 (0)