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.

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
+    // snapshotTree waits out a NavHost cross-fade before it reads, so the runner
-    // never handed a tree holding two routes at once. It belongs here rather
+    // 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.
+    // not: it fetched the hierarchy ~4 more times on every mutating step. The
-    override fun snapshot(): SnapshotSample {
+    // keyboard leg costs nothing either when no IME is standing in the tree,
-        val tree = treeWithoutKeyboard(
+    // which is what lets the Hierarchy RPC serve this too.
-            awaitSettledTree { hierarchy() },
+    override fun snapshotTree(): String = treeWithoutKeyboard(
-            imePackage,
+        awaitSettledTree { hierarchy() },
-            dismiss = { runCatching { dadb.shell("input keyevent 4") } },
+        imePackage,
-            reread = { awaitSettledTree { hierarchy() } },
+        dismiss = { runCatching { dadb.shell("input keyevent 4") } },
-        )
+        reread = { awaitSettledTree { hierarchy() } },
-        return SnapshotSample(tree, screenshot())
+    )
-    }
 
     override fun waitForIdle(durationMillis: Long) {
         // waitForAppToSettle blocks on the View-system animation and maestro's

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()
         }
     }
 

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)