From 86da6ed8ee119082340415ae80edbeee758817ea Mon Sep 17 00:00:00 2001
From: PJ <ipriyanshujain@gmail.com>
Date: Sun, 16 Aug 2026 00:57:36 +0530
Subject: [PATCH 1/8] fix(runner): a bounded hold puts the swallow back one
 step later

the hold carries one action; letting the runner act again while the verifier
is still skipped overwrites it, so the carried action reaches no spec. hold
for as long as the verifier is skipped, and settle on a held step so the
reread pair is not tighter than the window the detector was measured over.
---
 internal/runner/composition_reread_test.go | 131 ++++++++++++++++++---
 internal/runner/runner.go                  |  36 +++---
 2 files changed, 130 insertions(+), 37 deletions(-)

diff --git a/internal/runner/composition_reread_test.go b/internal/runner/composition_reread_test.go
index 5847d8e..f052582 100644
--- a/internal/runner/composition_reread_test.go
+++ b/internal/runner/composition_reread_test.go
@@ -133,22 +133,25 @@ func TestRunner_AStepWhoseTreeChangedBetweenReadsIsNotVerified(t *testing.T) {
 	})
 }
 
-// submitsOnTapDriver commits a transaction on every tap, shows the running
-// total in the tree, and grows a row under the hierarchy read that follows the
-// paired Snapshot: on one chosen step, or on every one of them.
+// submitsOnTapDriver commits commitsPerTap transactions on every tap, shows the
+// running total in the tree, and grows a row under the hierarchy read that
+// follows the paired Snapshot: on one chosen step, on the run of steps from
+// composingRead through composingThrough, or on every one of them.
 type submitsOnTapDriver struct {
 	*mockdriver.Driver
-	composingRead int64
-	everyRead     bool
-	reads         atomic.Int64
-	committed     atomic.Int64
+	commitsPerTap    int64
+	composingRead    int64
+	composingThrough int64
+	everyRead        bool
+	reads            atomic.Int64
+	committed        atomic.Int64
 }
 
 func (d *submitsOnTapDriver) Tap(context.Context, int, int) error       { return d.commit() }
 func (d *submitsOnTapDriver) TapSelector(context.Context, string) error { return d.commit() }
 
 func (d *submitsOnTapDriver) commit() error {
-	d.committed.Add(1)
+	d.committed.Add(d.commitsPerTap)
 	return nil
 }
 
@@ -157,7 +160,10 @@ func (d *submitsOnTapDriver) Snapshot(context.Context) (string, driver.Image, er
 }
 
 func (d *submitsOnTapDriver) Hierarchy(context.Context) (string, error) {
-	if read := d.reads.Add(1); d.everyRead || read == d.composingRead {
+	read := d.reads.Add(1)
+	composing := d.everyRead || read == d.composingRead ||
+		(read >= d.composingRead && read <= d.composingThrough)
+	if composing {
 		return fmt.Sprintf(homeWithTxnCountComposing, d.committed.Load()), nil
 	}
 	return fmt.Sprintf(homeWithTxnCount, d.committed.Load()), nil
@@ -188,7 +194,11 @@ func TestRunner_ASkippedStepDoesNotSwallowTheActionBeforeIt(t *testing.T) {
 	run := func(t *testing.T, composingRead int64) (Summary, int64) {
 		t.Helper()
 		state := newHarnessWithSpec(t, spec)
-		device := &submitsOnTapDriver{Driver: state.mock, composingRead: composingRead}
+		device := &submitsOnTapDriver{
+			Driver:        state.mock,
+			commitsPerTap: 1,
+			composingRead: composingRead,
+		}
 
 		ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
 		defer cancel()
@@ -243,13 +253,92 @@ func TestRunner_ASkippedStepDoesNotSwallowTheActionBeforeIt(t *testing.T) {
 	})
 }
 
-// Holding an action back is bounded. A screen that changes shape under every
-// pair of reads (a live list, a spinner mounting and unmounting) would
-// otherwise take the whole run: nothing verified, nothing tapped, and a green
-// summary at the end of it.
-func TestRunner_AScreenThatNeverSettlesDoesNotStallTheRun(t *testing.T) {
+// One skipped step is held; a run of them has to be held too. A bound that lets
+// the runner act again while the verifier is still being skipped puts back the
+// exact swallow the hold exists to prevent, only later: the action drawn on the
+// step past the bound overwrites the one the hold was carrying, and the carried
+// action is never reported to any spec.
+//
+// The screen composes on steps 3 through 5 of 6 and the device commits one
+// transaction per tap throughout, so the property has a clean pair to judge
+// (step 2 to step 6) and nothing in between it can be told about except the
+// action step 2 applied.
+func TestRunner_ARunOfSkippedStepsReportsEveryActionItApplied(t *testing.T) {
+	spec := specWithFolioPredicates(t)
+
+	run := func(t *testing.T, commitsPerTap int64) (Summary, int64) {
+		t.Helper()
+		state := newHarnessWithSpec(t, spec)
+		device := &submitsOnTapDriver{
+			Driver:           state.mock,
+			commitsPerTap:    commitsPerTap,
+			composingRead:    3,
+			composingThrough: 5,
+		}
+
+		ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
+		defer cancel()
+		summary, err := Run(ctx, Options{
+			Duration:    time.Hour,
+			IdleTimeout: 20 * time.Millisecond,
+			MaxSteps:    6,
+			Driver:      device,
+			Verifier:    state.verifier,
+			TraceWriter: state.writer,
+		})
+		if err != nil {
+			t.Fatalf("Run: %v", err)
+		}
+		if summary.Steps != 6 {
+			t.Fatalf("steps = %d, want 6", summary.Steps)
+		}
+		if summary.SkippedVerification != 3 {
+			t.Fatalf("the run skipped %d step(s), want 3; the reread never fired across "+
+				"the run this test is about", summary.SkippedVerification)
+		}
+		return summary, device.committed.Load()
+	}
+
+	t.Run("no submit is lost to the run of skipped steps", func(t *testing.T) {
+		summary, committed := run(t, 1)
+		if committed == 0 {
+			t.Fatal("the device committed nothing; a runner that never acts passes this " +
+				"test without meaning anything")
+		}
+		if len(summary.Violations) != 0 {
+			t.Errorf("the counting property convicted a healthy app: %v\n"+
+				"one transaction per submit rose, and a submit went unreported because "+
+				"the runner acted on a step the verifier skipped", summary.Violations)
+		}
+		if committed != 3 {
+			t.Errorf("the device committed %d transaction(s), want 3: one per verified "+
+				"step (1, 2 and 6) and none from a step nothing would judge", committed)
+		}
+	})
+
+	// The control. Without it a green above proves nothing: a property handed
+	// no comparable pair is silently vacuous and reports the same empty list.
+	t.Run("two transactions per tap still convicts across the same run", func(t *testing.T) {
+		summary, _ := run(t, 2)
+		if len(summary.Violations) == 0 {
+			t.Fatal("the counting property missed a double submit; the skipped steps left " +
+				"it with nothing to judge, so the case above proves nothing")
+		}
+		if got := summary.Violations[0].Properties[0]; got != "submitCommitsOneTransactionPerAction" {
+			t.Errorf("violated %v, want submitCommitsOneTransactionPerAction",
+				summary.Violations[0].Properties)
+		}
+	})
+}
+
+// A screen that changes shape under every pair of reads (a live list, a spinner
+// mounting and unmounting) costs the run its actions: an action applied onto it
+// would be the one the next verified step never hears about, and there is no
+// next verified step. What the run must not do is come back green off that,
+// which is what the "judged by nothing" count and the run's outcome are for.
+func TestRunner_AScreenThatNeverSettlesActsOnNothingAndSaysSo(t *testing.T) {
 	state := newHarnessWithSpec(t, specWithFolioPredicates(t))
-	device := &submitsOnTapDriver{Driver: state.mock, everyRead: true}
+	device := &submitsOnTapDriver{Driver: state.mock, commitsPerTap: 1, everyRead: true}
 
 	ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
 	defer cancel()
@@ -264,13 +353,17 @@ func TestRunner_AScreenThatNeverSettlesDoesNotStallTheRun(t *testing.T) {
 	if err != nil {
 		t.Fatalf("Run: %v", err)
 	}
+	if summary.Steps != 5 {
+		t.Fatalf("steps = %d, want 5; the run stalled instead of finishing its budget",
+			summary.Steps)
+	}
 	if summary.SkippedVerification != 5 {
 		t.Fatalf("the run verified some step of a screen that never settled: skipped %d of 5",
 			summary.SkippedVerification)
 	}
-	if device.committed.Load() == 0 {
-		t.Error("the fuzzer never acted across 5 steps; a screen that keeps moving must " +
-			"cost the run a step or two, not all of them")
+	if got := device.committed.Load(); got != 0 {
+		t.Errorf("the fuzzer applied %d action(s) onto a screen no property would judge; "+
+			"each one is an action no spec will ever be told about", got)
 	}
 }
 
diff --git a/internal/runner/runner.go b/internal/runner/runner.go
index a7aac31..36f8ef8 100644
--- a/internal/runner/runner.go
+++ b/internal/runner/runner.go
@@ -100,7 +100,6 @@ func Run(ctx context.Context, options Options) (Summary, error) {
 	deadline := summary.StartTime.Add(options.Duration)
 	stepIndex := 0
 	consecutiveApplyFailures := 0
-	heldSteps := 0
 	var lastAction *verifier.Action
 	var lastLogTime time.Time
 	for time.Now().Before(deadline) {
@@ -287,16 +286,16 @@ func Run(ctx context.Context, options Options) (Summary, error) {
 		// their effects would then see an effect whose cause the runner
 		// swallowed. See TestRunner_ASkippedStepDoesNotSwallowTheActionBeforeIt.
 		//
-		// Bounded, because a screen that never settles must not stall the whole
-		// run: past the bound the runner acts anyway, which is where it was
-		// before this held anything back.
-		held := skippedVerification && heldSteps < maxHeldSteps
+		// Unbounded, because lastAction holds exactly one action: any bound that
+		// let the runner act again while the verifier was still being skipped
+		// would overwrite the action the hold was carrying, and that is the same
+		// swallow arriving one step later. A screen that keeps moving therefore
+		// costs the run its actions rather than its soundness, and a run that
+		// verified nothing says so in its outcome (internal/testrun).
+		held := skippedVerification
 		if held {
-			heldSteps++
 			logger.Warn("screen still moving; holding this step's action back",
-				"step", stepIndex, "held", heldSteps)
-		} else {
-			heldSteps = 0
+				"step", stepIndex)
 		}
 
 		var nextAction verifier.Action
@@ -401,7 +400,16 @@ func Run(ctx context.Context, options Options) (Summary, error) {
 		// concurrent fetches observe a stable post-action state. A transient
 		// apply error means nothing landed, so the idle poll has nothing to
 		// settle and may itself hang on the same device condition.
-		if nextErr == nil && !applySkipped && nextAction.Kind != verifier.ActionKindWait {
+		//
+		// A held step settles too, and it is the only case here that waits with
+		// nothing applied. The reread that held it takes its two reads a round
+		// trip apart, which is a tighter window than the one the detector was
+		// measured over (an action and a settle); looping straight back into it
+		// would compare two reads of a composing screen closer together still,
+		// so the screen that most needs to settle is the one given least room.
+		applied := nextErr == nil && !applySkipped &&
+			nextAction.Kind != verifier.ActionKindWait
+		if held || applied {
 			idleCtx, idleCancel := context.WithTimeout(ctx, options.IdleTimeout)
 			idleErr := options.Driver.WaitForIdle(idleCtx, options.IdleTimeout)
 			if idleErr != nil && idleCtx.Err() == nil {
@@ -1316,14 +1324,6 @@ func encodeResiduals(residuals map[string]ltl.Formula) (map[string]json.RawMessa
 // would be spent doing nothing.
 const maxConsecutiveApplyFailures = 3
 
-// maxHeldSteps bounds how many steps in a row the runner will decline to act on
-// because their screen was still moving. It is a livelock bound, not a settle
-// budget: a screen that changes shape under every pair of reads (a live list, a
-// spinner that mounts and unmounts) would otherwise take the whole run without
-// the fuzzer ever touching it. Two is what the measured cases need, which came
-// one step at a time and never twice in a row.
-const maxHeldSteps = 2
-
 // isWDADrop reports that the sidecar could not restart the iOS XCTest
 // runner: the channel is gone for good and the run must abort. Transient
 // drops are classified by the sidecar itself (it reconnects and surfaces

From 0a7afbabeceaad5d7522693f03a7bc11eb55e7a1 Mon Sep 17 00:00:00 2001
From: PJ <ipriyanshujain@gmail.com>
Date: Sun, 16 Aug 2026 00:59:13 +0530
Subject: [PATCH 2/8] test(runner): pin what the two reads are compared on

structuralShape excluding text and bounds is the decision separating this
feature from a run that verifies nothing, and only prose held it. adding
either field back now turns a case red.
---
 internal/runner/composition_reread_test.go | 117 +++++++++++++++++++++
 internal/runner/runner.go                  |   3 +
 2 files changed, 120 insertions(+)

diff --git a/internal/runner/composition_reread_test.go b/internal/runner/composition_reread_test.go
index f052582..5bb3721 100644
--- a/internal/runner/composition_reread_test.go
+++ b/internal/runner/composition_reread_test.go
@@ -367,6 +367,123 @@ func TestRunner_AScreenThatNeverSettlesActsOnNothingAndSaysSo(t *testing.T) {
 	}
 }
 
+// homeWithTicker is one settled route holding a total that ticks and a button
+// whose measured bounds shift under it. The nodes, their ids and their classes
+// are the same in every rendering of it.
+const homeWithTicker = `{"attributes":{"resource-id":"HomeScreen","class":"android.view.View"},"children":[
+	{"attributes":{"resource-id":"Total","class":"android.widget.TextView","text":"%s"},"children":[]},
+	{"attributes":{"resource-id":"TxnSubmit","class":"android.widget.Button","bounds":"%s"},"children":[]}
+]}`
+
+// The same route with a node in it that was not there a read ago.
+const homeWithTickerAndRow = `{"attributes":{"resource-id":"HomeScreen","class":"android.view.View"},"children":[
+	{"attributes":{"resource-id":"Total","class":"android.widget.TextView","text":"120.00"},"children":[]},
+	{"attributes":{"resource-id":"TxnSubmit","class":"android.widget.Button","bounds":"[40,80,240,160]"},"children":[]},
+	{"attributes":{"resource-id":"TxnRowLate","class":"android.view.View"},"children":[]}
+]}`
+
+// rereadsDriver answers the paired Snapshot with one fixed tree and the
+// hierarchy read that follows with another, so a test can say exactly what
+// moved between the two reads the detector compares.
+type rereadsDriver struct {
+	*mockdriver.Driver
+	snapshotTree string
+	rereadTree   string
+}
+
+func (d *rereadsDriver) Snapshot(context.Context) (string, driver.Image, error) {
+	return d.snapshotTree, driver.Image{}, nil
+}
+
+func (d *rereadsDriver) Hierarchy(context.Context) (string, error) {
+	return d.rereadTree, nil
+}
+
+// What the two reads are compared ON is the whole feature. Comparing the values
+// in the tree instead of the nodes in it would fire on every step of a screen
+// with a total on it or a measure pass in flight, and a runner that skips every
+// step verifies nothing while reporting no violations: green and vacuous, which
+// is a worse answer than the composition the comparison set out to catch.
+//
+// The always-false property is the witness: it fires on the first step that
+// reaches the verifier, so its presence and its step index say whether the step
+// was judged at all.
+func TestRunner_OnlyAChangeOfShapeCostsAStepItsVerdict(t *testing.T) {
+	settled := fmt.Sprintf(homeWithTicker, "120.00", "[40,80,240,160]")
+	cases := []struct {
+		name     string
+		reread   string
+		verified bool
+	}{
+		{
+			name:     "a total that ticked between the two reads",
+			reread:   fmt.Sprintf(homeWithTicker, "121.00", "[40,80,240,160]"),
+			verified: true,
+		},
+		{
+			name:     "a measure pass that moved the button",
+			reread:   fmt.Sprintf(homeWithTicker, "120.00", "[40,84,240,164]"),
+			verified: true,
+		},
+		{
+			name:     "a node that was not in the tree a read ago",
+			reread:   homeWithTickerAndRow,
+			verified: false,
+		},
+	}
+
+	for _, testCase := range cases {
+		t.Run(testCase.name, func(t *testing.T) {
+			state := newHarnessWithSpec(t, violationSpec)
+			device := &rereadsDriver{
+				Driver:       state.mock,
+				snapshotTree: settled,
+				rereadTree:   testCase.reread,
+			}
+
+			ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
+			defer cancel()
+			summary, err := Run(ctx, Options{
+				Duration:    time.Hour,
+				IdleTimeout: 20 * time.Millisecond,
+				MaxSteps:    3,
+				Driver:      device,
+				Verifier:    state.verifier,
+				TraceWriter: state.writer,
+			})
+			if err != nil {
+				t.Fatalf("Run: %v", err)
+			}
+			if summary.Steps != 3 {
+				t.Fatalf("steps = %d, want 3", summary.Steps)
+			}
+
+			if !testCase.verified {
+				if summary.SkippedVerification != 3 {
+					t.Errorf("the run judged %d of 3 steps whose tree grew a node between "+
+						"the two reads; a screen still composing must reach no property",
+						3-summary.SkippedVerification)
+				}
+				if len(summary.Violations) != 0 {
+					t.Errorf("a skipped step reached the verifier anyway: %v",
+						summary.Violations)
+				}
+				return
+			}
+			if summary.SkippedVerification != 0 {
+				t.Fatalf("the run judged nothing: %d of 3 steps were skipped over a tree "+
+					"whose nodes never changed", summary.SkippedVerification)
+			}
+			if len(summary.Violations) != 1 {
+				t.Fatalf("violations = %v, want exactly one", summary.Violations)
+			}
+			if got := summary.Violations[0].StepIndex; got != 1 {
+				t.Errorf("the property first judged step %d, want 1", got)
+			}
+		})
+	}
+}
+
 type traceLine struct {
 	Step       int      `json:"step"`
 	Violations []string `json:"violations"`
diff --git a/internal/runner/runner.go b/internal/runner/runner.go
index 36f8ef8..53fa151 100644
--- a/internal/runner/runner.go
+++ b/internal/runner/runner.go
@@ -1130,6 +1130,9 @@ func changedOnReread(
 // worse than the composition it set out to catch. The trade is measured rather
 // than assumed: over 100 folio steps on an API 35 emulator, text moved under
 // an unchanged shape on 1 step, and the shape itself moved on 1 other.
+//
+// TestRunner_OnlyAChangeOfShapeCostsAStepItsVerdict is what holds the line:
+// adding either field back to the shape turns one of its cases red.
 func structuralShape(tree *hierarchy.Tree) string {
 	var shape strings.Builder
 	for _, element := range tree.Elements {

From 8e37be2afdef00c12c3317c25b5948733e13548e Mon Sep 17 00:00:00 2001
From: PJ <ipriyanshujain@gmail.com>
Date: Sun, 16 Aug 2026 01:01:03 +0530
Subject: [PATCH 3/8] test(runner): pin both guard writes to what the spec
 reads

deleting lastAction.Relaunched or lastAction.Applied left the whole suite
green, so the only producer of the two fields every spec-side guard reads
had nothing holding it. both now assert the value out of the trace.
---
 internal/runner/composition_reread_test.go    |  6 +-
 .../foreground_guard_last_action_test.go      | 76 +++++++++++++++++++
 2 files changed, 80 insertions(+), 2 deletions(-)

diff --git a/internal/runner/composition_reread_test.go b/internal/runner/composition_reread_test.go
index 5bb3721..942e602 100644
--- a/internal/runner/composition_reread_test.go
+++ b/internal/runner/composition_reread_test.go
@@ -14,6 +14,7 @@ import (
 
 	"github.com/priyanshujain/sanderling/internal/driver"
 	mockdriver "github.com/priyanshujain/sanderling/internal/driver/mock"
+	"github.com/priyanshujain/sanderling/internal/trace"
 )
 
 // homeWithRows is one settled route whose list holds rows. A row arriving
@@ -485,8 +486,9 @@ func TestRunner_OnlyAChangeOfShapeCostsAStepItsVerdict(t *testing.T) {
 }
 
 type traceLine struct {
-	Step       int      `json:"step"`
-	Violations []string `json:"violations"`
+	Step             int                              `json:"step"`
+	Violations       []string                         `json:"violations"`
+	ExtractorChanges map[string]trace.ExtractorChange `json:"extractor_changes"`
 }
 
 func traceSteps(t *testing.T, directory string) []traceLine {
diff --git a/internal/runner/foreground_guard_last_action_test.go b/internal/runner/foreground_guard_last_action_test.go
index 8932f1b..6a5a265 100644
--- a/internal/runner/foreground_guard_last_action_test.go
+++ b/internal/runner/foreground_guard_last_action_test.go
@@ -285,3 +285,79 @@ func TestRunner_AnOverlayDoesNotConvictTheSubmitCountingProperty(t *testing.T) {
 		}
 	})
 }
+
+// reportedActionSpec puts what the runner told the spec about the last action
+// into an extractor, so a test can read it out of the trace. `applied` and
+// `relaunched` have no other producer: the runner's two guard writes are the
+// only thing that ever sets them, and every spec-side guard built on them (see
+// acrossRelaunch and confirmedApplied in the folio predicates) reads nothing
+// else. A regression in either write leaves those guards permanently off with
+// no property anywhere able to notice.
+const reportedActionSpec = `
+import { actions, always, extract, Tap } from "@sanderling/spec";
+const reportedAction = extract("reportedAction", state => {
+  const last = state.lastAction;
+  if (last == null) return "none";
+  const dispatch = last.applied === true ? "applied" : "unconfirmed";
+  const process = last.relaunched === true ? "relaunched" : "same-process";
+  return dispatch + "/" + process;
+});
+globalThis.properties = {
+  theGuardTheRunnerRanReachesTheSpec: always(
+    () => reportedAction.current !== "applied/same-process",
+  ),
+};
+globalThis.actions = actions(() => [Tap({ on: "id:TxnSubmit" })]);
+`
+
+// runReportingTheGuard drives two steps against a device whose submit tap trips
+// one of the foreground guards, and hands back what the spec read off
+// state.lastAction on the step the guard fired.
+func runReportingTheGuard(t *testing.T, device committingDevice, state *harness) string {
+	t.Helper()
+	if violations := runTwoSubmitSteps(t, state, device, 1); len(violations) != 0 {
+		t.Errorf("the spec was told the action ran untouched by any guard: %v", violations)
+	}
+	steps := traceSteps(t, state.writer.Directory())
+	if len(steps) != 2 {
+		t.Fatalf("trace holds %d step(s), want 2", len(steps))
+	}
+	change, ok := steps[1].ExtractorChanges["reportedAction"]
+	if !ok {
+		t.Fatalf("step 2 recorded no reading of the reported action: %+v", steps[1])
+	}
+	return string(change.Curr)
+}
+
+func TestRunner_TheSpecIsToldTheAppWasRelaunchedUnderTheAction(t *testing.T) {
+	state := newHarnessWithSpec(t, reportedActionSpec)
+	device := &leavesForegroundAfterSubmitDriver{Driver: state.mock, commitsPerTap: 1}
+
+	reported := runReportingTheGuard(t, device, state)
+
+	if countMockActions(state, mockdriver.ActionLaunch, "") == 0 {
+		t.Fatal("the app was never relaunched, so the write this test is about never ran")
+	}
+	if reported != `"applied/relaunched"` {
+		t.Errorf("the spec read %s off state.lastAction, want \"applied/relaunched\"; "+
+			"a property relaxed across a relaunch cannot fire on a run that never "+
+			"tells it one happened", reported)
+	}
+}
+
+func TestRunner_TheSpecIsToldAnObscuredActionWasNotConfirmed(t *testing.T) {
+	state := newHarnessWithSpec(t, reportedActionSpec)
+	device := &obscuredAfterSubmitDriver{Driver: state.mock, commitsPerTap: 1}
+
+	reported := runReportingTheGuard(t, device, state)
+
+	if countMockActions(state, mockdriver.ActionPressKey, "back") == 0 {
+		t.Fatal("the overlay was never dismissed, so the write this test is about never ran")
+	}
+	if reported != `"unconfirmed/same-process"` {
+		t.Errorf("the spec read %s off state.lastAction, want "+
+			"\"unconfirmed/same-process\"; a system window held the focused window, "+
+			"so whether the app received the tap is exactly what nobody can say",
+			reported)
+	}
+}

From 33c585de7be4016cceabac47a55ab06eb291df8e Mon Sep 17 00:00:00 2001
From: PJ <ipriyanshujain@gmail.com>
Date: Sun, 16 Aug 2026 01:02:29 +0530
Subject: [PATCH 4/8] fix(testrun): a run that judged nothing is not a green
 run

every step skipped means no property ever evaluated, so no violations is the
absence of a verdict rather than a clean one. the hold makes that reachable
now, so the run says it instead of exiting 0.
---
 internal/testrun/testrun.go      | 24 ++++++++++++++++++++++++
 internal/testrun/testrun_test.go | 25 +++++++++++++++++++++++++
 2 files changed, 49 insertions(+)

diff --git a/internal/testrun/testrun.go b/internal/testrun/testrun.go
index 893e5ce..7a46a94 100644
--- a/internal/testrun/testrun.go
+++ b/internal/testrun/testrun.go
@@ -242,7 +242,15 @@ func Execute(ctx context.Context, options Options, stdout io.Writer) error {
 // --exit-on-violation a run that found violations is still a successful run
 // (the summary reports them), which is the behaviour every existing caller
 // depends on.
+//
+// A run none of whose steps reached the verifier fails whatever the flags say,
+// because it holds no verdict to report. The threshold is every step and not a
+// fraction of them: a screen that composes now and then costs a healthy android
+// run a step or two, and a check that fired on those would be red on every run.
 func runOutcome(options Options, summary runner.Summary) error {
+	if summary.Steps > 0 && summary.SkippedVerification == summary.Steps {
+		return VacuousRunError{Steps: summary.Steps}
+	}
 	if options.ExitOnViolation && len(summary.Violations) > 0 {
 		return ViolationsError{Count: len(summary.Violations)}
 	}
@@ -261,6 +269,22 @@ func (e ViolationsError) Error() string {
 	return fmt.Sprintf("%d violation record(s)", e.Count)
 }
 
+// VacuousRunError reports a run in which no step reached the verifier, so no
+// property ever judged anything. It is not a clean run and it is not a found
+// bug: it is a run that produced no evidence either way, and the absence of
+// violations in it says nothing about the app. It stays untyped to the CLI's
+// violation path on purpose, so it exits 1 as a broken run rather than 2.
+type VacuousRunError struct {
+	Steps int
+}
+
+func (e VacuousRunError) Error() string {
+	return fmt.Sprintf(
+		"%d step(s) ran and none of them reached the verifier: the screen was "+
+			"still moving every time it was read, so no property judged this run",
+		e.Steps)
+}
+
 // bundleInputs holds the pre-driver assembly: alias map, seed, esbuild defines,
 // and the resolved spec-API/goja-runtime paths the bundler consumes.
 type bundleInputs struct {
diff --git a/internal/testrun/testrun_test.go b/internal/testrun/testrun_test.go
index fd6303e..52c77c1 100644
--- a/internal/testrun/testrun_test.go
+++ b/internal/testrun/testrun_test.go
@@ -266,6 +266,31 @@ func TestRunOutcome_ReportsViolationsOnlyUnderTheFlag(t *testing.T) {
 	}
 }
 
+// A step the verifier skipped was judged by nothing, so a run whose every step
+// was skipped holds no verdict at all: "no violations" there is the absence of
+// an answer rather than a clean one. Reporting it as a successful run is the
+// green and vacuous outcome structuralShape's own design notes call worse than
+// the composition it catches, and the runner's hold is what makes a fully
+// skipped run reachable.
+func TestRunOutcome_ARunThatJudgedNothingIsNotASuccess(t *testing.T) {
+	nothingJudged := runner.Summary{Steps: 6, SkippedVerification: 6}
+	err := runOutcome(Options{}, nothingJudged)
+	var vacuous VacuousRunError
+	if !errors.As(err, &vacuous) {
+		t.Fatalf("a run that judged none of its 6 steps came back %v, want a VacuousRunError", err)
+	}
+	if vacuous.Steps != 6 {
+		t.Errorf("steps: got %d, want 6", vacuous.Steps)
+	}
+
+	// A screen that composes now and then costs a run steps, not its verdict. A
+	// check that fired here would turn every healthy android run red.
+	mostlyJudged := runner.Summary{Steps: 6, SkippedVerification: 5}
+	if err := runOutcome(Options{}, mostlyJudged); err != nil {
+		t.Errorf("a run that judged one of its 6 steps must succeed, got %v", err)
+	}
+}
+
 // wedgedLaunchDriver never returns from Launch, standing in for a driver whose
 // device-side session is stuck.
 type wedgedLaunchDriver struct {

From 1afe9c3a7d892cad802ccd54de738f8d72031a96 Mon Sep 17 00:00:00 2001
From: PJ <ipriyanshujain@gmail.com>
Date: Sun, 16 Aug 2026 01:20:28 +0530
Subject: [PATCH 5/8] fix(sidecar): the hierarchy rpc serves the tree the
 snapshot reads

the runner compares the two per step, but snapshot settles and closes a
keyboard while hierarchy was a bare contentDescriptor. measured on emulator
-5556 (api 34) with an ime open: 489 nodes against the snapshot's 134. both
now come off snapshotTree under the same lock; the reread still costs ~75ms
when no keyboard is up.
---
 .../dev/sanderling/sidecar/DriverBackend.kt   | 35 ++++++++++++-------
 .../dev/sanderling/sidecar/DriverService.kt   |  8 ++++-
 .../sanderling/sidecar/DriverServiceTest.kt   | 12 +++++--
 3 files changed, 39 insertions(+), 16 deletions(-)

diff --git a/sidecar/src/main/kotlin/dev/sanderling/sidecar/DriverBackend.kt b/sidecar/src/main/kotlin/dev/sanderling/sidecar/DriverBackend.kt
index 3d69e2f..ded9614 100644
--- a/sidecar/src/main/kotlin/dev/sanderling/sidecar/DriverBackend.kt
+++ b/sidecar/src/main/kotlin/dev/sanderling/sidecar/DriverBackend.kt
@@ -30,11 +30,21 @@ interface DriverBackend {
     fun healthy(): Boolean
     fun metrics(bundleId: String): MetricsSample
 
+    // snapshotTree is the tree a snapshot reads, without the screenshot. It is
+    // what the Hierarchy RPC serves, so the runner's two reads of a step come
+    // off one pipeline: a backend that waits out a transition or closes a
+    // keyboard before reading has to do the same on both, or the two trees
+    // differ over what the backend did between them rather than over what the
+    // app did. On this device an IME standing open is a 489-node tree against
+    // the snapshot's 134.
+    fun snapshotTree(): String = hierarchy()
+
     // snapshot captures hierarchy then screenshot back-to-back. The service
     // layer holds a mutex around the call so concurrent callers observe a
     // serialized pair from the same on-device frame. Backends may override
     // to fuse the two reads more tightly when their native API allows.
-    fun snapshot(): SnapshotSample = SnapshotSample(hierarchy(), screenshot())
+    fun snapshot(): SnapshotSample =
+        SnapshotSample(snapshotTree(), screenshot())
 
     // close releases device-side resources on shutdown. The iOS backend must
     // stop its XCTest runner here: an orphaned runner session auto-restarts
@@ -1250,8 +1260,8 @@ class MaestroDriverBackend(private val serial: String?) : DriverBackend {
     override fun recentLogs(sinceUnixMillis: Long, minLevel: String) =
         readLogcat(serial, sinceUnixMillis, minLevel)
 
-    // snapshot waits out a NavHost cross-fade before it reads, so the runner is
-    // never handed a tree holding two routes at once. It belongs here rather
+    // snapshotTree waits out a NavHost cross-fade before it reads, so the runner
+    // is never handed a tree holding two routes at once. It belongs here rather
     // than in waitForIdle: the runner gives waitForIdle a one-second deadline
     // and abandons the RPC when it expires, which is not enough room for a
     // 700ms fade that began before the settle did, and a wait that outlives the
@@ -1262,16 +1272,15 @@ class MaestroDriverBackend(private val serial: String?) : DriverBackend {
     // The predicate costs nothing on a settled frame: the read it needs is the
     // read the snapshot was going to do anyway. That is what makes this
     // affordable, where the structural poll that used to run in waitForIdle was
-    // not: it fetched the hierarchy ~4 more times on every mutating step.
-    override fun snapshot(): SnapshotSample {
-        val tree = treeWithoutKeyboard(
-            awaitSettledTree { hierarchy() },
-            imePackage,
-            dismiss = { runCatching { dadb.shell("input keyevent 4") } },
-            reread = { awaitSettledTree { hierarchy() } },
-        )
-        return SnapshotSample(tree, screenshot())
-    }
+    // not: it fetched the hierarchy ~4 more times on every mutating step. The
+    // keyboard leg costs nothing either when no IME is standing in the tree,
+    // which is what lets the Hierarchy RPC serve this too.
+    override fun snapshotTree(): String = treeWithoutKeyboard(
+        awaitSettledTree { hierarchy() },
+        imePackage,
+        dismiss = { runCatching { dadb.shell("input keyevent 4") } },
+        reread = { awaitSettledTree { hierarchy() } },
+    )
 
     override fun waitForIdle(durationMillis: Long) {
         // waitForAppToSettle blocks on the View-system animation and maestro's
diff --git a/sidecar/src/main/kotlin/dev/sanderling/sidecar/DriverService.kt b/sidecar/src/main/kotlin/dev/sanderling/sidecar/DriverService.kt
index 647655b..ea3b137 100644
--- a/sidecar/src/main/kotlin/dev/sanderling/sidecar/DriverService.kt
+++ b/sidecar/src/main/kotlin/dev/sanderling/sidecar/DriverService.kt
@@ -170,12 +170,18 @@ class DriverService(
         }
     }
 
+    // The runner reads this a second time per step to see whether the screen
+    // changed while it was looking, so it has to describe the same thing the
+    // snapshot's tree describes: same settle, same keyboard handling, same
+    // lock. Served off the bare backend read, the pair differed over what the
+    // backend did between them rather than over what the app did.
     override fun hierarchy(
         request: Empty,
         responseObserver: StreamObserver<HierarchyJSON>,
     ) {
         runRpc(responseObserver) {
-            HierarchyJSON.newBuilder().setJson(backend.hierarchy()).build()
+            val tree = synchronized(snapshotLock) { backend.snapshotTree() }
+            HierarchyJSON.newBuilder().setJson(tree).build()
         }
     }
 
diff --git a/sidecar/src/test/kotlin/dev/sanderling/sidecar/DriverServiceTest.kt b/sidecar/src/test/kotlin/dev/sanderling/sidecar/DriverServiceTest.kt
index 0187cca..29cd6a9 100644
--- a/sidecar/src/test/kotlin/dev/sanderling/sidecar/DriverServiceTest.kt
+++ b/sidecar/src/test/kotlin/dev/sanderling/sidecar/DriverServiceTest.kt
@@ -333,9 +333,17 @@ class DriverServiceTest {
         assertEquals(3, image.png.size())
     }
 
-    @Test fun hierarchyReturnsBackendJson() {
+    // The runner reads the hierarchy a second time per step to see whether the
+    // screen changed while it was looking, so this has to answer with the tree
+    // the snapshot's read produces: same settle, same keyboard handling. Served
+    // off the bare backend read, the pair differs over what the backend did
+    // between the two reads rather than over what the app did. Measured on an
+    // API 34 emulator with an IME standing open, that is a 489-node tree
+    // against the snapshot's 134.
+    @Test fun hierarchyServesTheTreeTheSnapshotReads() {
         val backend = object : DriverBackend by StubDriverBackend("android") {
-            override fun hierarchy(): String = "{\"x\":1}"
+            override fun hierarchy(): String = "{\"bare\":1}"
+            override fun snapshotTree(): String = "{\"x\":1}"
         }
         val client = newClient(backend)
 

From 127234b58cff12e63ede821d781c35d17628f299 Mon Sep 17 00:00:00 2001
From: PJ <ipriyanshujain@gmail.com>
Date: Sun, 16 Aug 2026 01:20:33 +0530
Subject: [PATCH 6/8] docs(runner): say what makes the two reads comparable

the reread's comment claimed the round trip was the only interval between
them; what it left out is that the two rpcs have to read the same way, which
the repo's own android backend did not do.
---
 internal/runner/foreground_guard_last_action_test.go | 6 ++++--
 internal/runner/runner.go                            | 7 +++++++
 2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/internal/runner/foreground_guard_last_action_test.go b/internal/runner/foreground_guard_last_action_test.go
index 6a5a265..426f672 100644
--- a/internal/runner/foreground_guard_last_action_test.go
+++ b/internal/runner/foreground_guard_last_action_test.go
@@ -74,8 +74,10 @@ func (d *leavesForegroundAfterSubmitDriver) Snapshot(context.Context) (string, d
 	return fmt.Sprintf(homeWithTxnCount, d.committed.Load()), driver.Image{}, nil
 }
 
-// No device answers Snapshot and Hierarchy off different trees, and the runner
-// reads both per step, so this one answers them off the same commit count.
+// The runner reads both per step and compares them, so a device that answered
+// them off different trees would make every step of this test transitional and
+// judged by nothing. The sidecar serves both off one read path (snapshotTree)
+// for the same reason; this one answers them off the same commit count.
 func (d *leavesForegroundAfterSubmitDriver) Hierarchy(context.Context) (string, error) {
 	return fmt.Sprintf(homeWithTxnCount, d.committed.Load()), nil
 }
diff --git a/internal/runner/runner.go b/internal/runner/runner.go
index 53fa151..dfde026 100644
--- a/internal/runner/runner.go
+++ b/internal/runner/runner.go
@@ -1079,6 +1079,13 @@ retryLoop:
 // filling in. Two reads a read apart are the cheapest thing that can see it
 // happening: the round trip IS the interval, so there is no sleep here.
 //
+// The comparison only means anything because the Hierarchy RPC serves the tree
+// the snapshot's own read produces (see snapshotTree in the sidecar). Off the
+// bare device read it does not: with an IME standing open, the snapshot answers
+// with 134 nodes and the bare read with 489, and the pair then differs over
+// whether the sidecar closed a keyboard between them rather than over anything
+// the app did.
+//
 // Waiting for the change to stop was measured on an API 34 device and refused:
 // a 750ms-quiet poll capped at 2s cost a median 1434ms against 76ms for one
 // read, hit its cap on every frame it fired for, and still handed back a frame

From 0d588ff22fe660fb0c3ec6a829d15056941d1226 Mon Sep 17 00:00:00 2001
From: PJ <ipriyanshujain@gmail.com>
Date: Sun, 16 Aug 2026 01:21:30 +0530
Subject: [PATCH 7/8] refactor(runner): name the settle predicate for what it
 means

---
 internal/runner/runner.go | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/internal/runner/runner.go b/internal/runner/runner.go
index dfde026..478907c 100644
--- a/internal/runner/runner.go
+++ b/internal/runner/runner.go
@@ -407,9 +407,9 @@ func Run(ctx context.Context, options Options) (Summary, error) {
 		// measured over (an action and a settle); looping straight back into it
 		// would compare two reads of a composing screen closer together still,
 		// so the screen that most needs to settle is the one given least room.
-		applied := nextErr == nil && !applySkipped &&
+		mutated := nextErr == nil && !applySkipped &&
 			nextAction.Kind != verifier.ActionKindWait
-		if held || applied {
+		if held || mutated {
 			idleCtx, idleCancel := context.WithTimeout(ctx, options.IdleTimeout)
 			idleErr := options.Driver.WaitForIdle(idleCtx, options.IdleTimeout)
 			if idleErr != nil && idleCtx.Err() == nil {

From 92d70ad9b8caf750c3b50b7ede824abfa3cf2866 Mon Sep 17 00:00:00 2001
From: PJ <ipriyanshujain@gmail.com>
Date: Sun, 16 Aug 2026 01:23:14 +0530
Subject: [PATCH 8/8] docs(sidecar): name the device the node counts came off

---
 .../src/main/kotlin/dev/sanderling/sidecar/DriverBackend.kt   | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/sidecar/src/main/kotlin/dev/sanderling/sidecar/DriverBackend.kt b/sidecar/src/main/kotlin/dev/sanderling/sidecar/DriverBackend.kt
index ded9614..b6d61c8 100644
--- a/sidecar/src/main/kotlin/dev/sanderling/sidecar/DriverBackend.kt
+++ b/sidecar/src/main/kotlin/dev/sanderling/sidecar/DriverBackend.kt
@@ -35,8 +35,8 @@ interface DriverBackend {
     // off one pipeline: a backend that waits out a transition or closes a
     // keyboard before reading has to do the same on both, or the two trees
     // differ over what the backend did between them rather than over what the
-    // app did. On this device an IME standing open is a 489-node tree against
-    // the snapshot's 134.
+    // app did. Measured on an API 34 emulator, an IME standing open is a
+    // 489-node bare read against the snapshot's 134.
     fun snapshotTree(): String = hierarchy()
 
     // snapshot captures hierarchy then screenshot back-to-back. The service