Skip to content

refactor(ios): prune converged snapshot paths - #2383

Open
thymikee wants to merge 1 commit into
mainfrom
prune/ios-converged-snapshot-paths
Open

refactor(ios): prune converged snapshot paths#2383
thymikee wants to merge 1 commit into
mainfrom
prune/ios-converged-snapshot-paths

Conversation

@thymikee

@thymikee thymikee commented Sep 7, 2026

Copy link
Copy Markdown
Member

Executes the pruning audit of #2199 against main at 65ff270. Deletion only: no compatibility re-export, no fallback, no allowlist. Context: #2199, tracker #2188.

What the audit found

shouldPresentLegacyIosInteractiveSnapshot was not dead. It fired for two live cases:

  1. provenance.producer === undefined on the xctest channel (stage falls back to 'acquired', owner falls back to undefined);
  2. simulator-ax-bridge, whose capability table still declared presentationOwner: 'snapshot-state' — a value introduced by 7ee1a5d (refactor(ios): carry provider acquisitions through one presentation owner #2233), which moved appium-source/limrun-ios-tree onto the engine and left the bridge behind, and never revised once routing put the bridge on the engine too.

The bridge is presented by the engine today: packages/platform-apple/src/snapshot-route.ts:124 calls host.snapshot.presentIosAcquisitionsrc/snapshot/ios-snapshot-runtime.ts:45 publishIosSnapshotpackages/capture-kit/src/ios-snapshot-engine/engine.ts:135, which applies buildIosInteractiveSnapshotPresentation for interactiveOnly. The assembly then applied the same rules a second time. That is a live violation of #2188 invariant 2 on main for agent-device snapshot --interactive-only on a local Simulator through the bridge route.

Per-item audit

# Prune-list item Verdict Evidence / owner
1 shouldPresentIosInteractiveSnapshot (renamed shouldPresentLegacyIosInteractiveSnapshot, src/core/snapshot-state.ts:129) delete Both live cases closed: producer is now required on a capture, and presentationOwner is gone. Coverage moved to src/core/__tests__/ios-snapshot-presentation-once.test.ts over all four producers.
2 Direct presentIosInteractiveSnapshot orchestration in buildSnapshotState (:61) delete The engine presents; the assembly is now the identity on an iOS tree. R74 direction 1 forbids the import returning.
3 presentationOwner capability + IosSnapshotPresentationOwner (packages/contracts/src/ios-snapshot.ts:18,89) delete One value remained once the bridge was accounted for. Its only other reader, snapshotTruncationForResult, now reads truncationEvidence — the fact it was standing in for. Producer-for-producer identical: apple-runner false, simulator-ax-bridge false, appium-source undefined, limrun-ios-tree undefined.
4 compactIosInteractiveSnapshot (ios-snapshot-engine/engine.ts:74) delete Byte-identical alias of presentIosInteractiveSnapshot, exported from the same index, zero production callers; the one test re-points at the surviving export. This is the "old semantic-compaction location" the prune list names, in duplicate-entrypoint form.
5 backendScopesAfterWire — the xctest term delete The blocklist is inverted to name the channels that still need the post-wire pass, so iOS leaves post-wire scope planning entirely.
6 backendScopesAfterWire — the rest retain linux-atspi, harmonyos-arkui, web and provenance-free captures hand over an unscoped tree and have no in-projection scope pass; --scope would silently stop working for them. Behaviour is bit-identical to the blocklist it replaces (exact complement over the six channels). Removing it needs those channels to take scope ownership, which is their own work, not a deletion here.
7 Synthesized WebDriver hittability already gone packages/provider-webdriver/src/webdriver-source.ts:187-201: the visibleToUser && enabled && rect synthesis is inside the platform === 'android' arm. The iOS arm (reportedHittabilityFact, :212-217) omits the key entirely when Appium did not report it. The iOS route (webdriver-interactor.ts:308webdriver-ios-snapshot.ts:21) cannot reach the Android arm.
8 Old iOS semantic-compaction locations already gone (implementation) No second implementation of collectIosRowPresentation / …Transition… / …NoiseSuppression / …WebSemantic… / reindexSnapshotNodesWithSuppressedParents exists. src/snapshot/snapshot-presentation/ios/ holds no implementation — four test/fixture files that import the capture-kit engine. See "not relocated" below.
9 Runner-local Swift geometric-presentation copies already gone One shared SwiftPM source of truth, apple/snapshot-presentation/Sources/AgentDeviceSnapshotPresentation/. The runner consumes it as an XCLocalSwiftPackageReference (AgentDeviceRunner.xcodeproj/project.pbxproj:163-166) and calls into it (RunnerTests+Snapshot.swift:166,223,232); no source-level copy. Package.runner.swift is a trimmed manifest swapped in by scripts/package-apple-runner-source.mjs:9-11, not a code copy.
10 Simulator observation dependency on runner readiness retain — STOP Owner: packages/platform-apple/src/snapshot-route.ts:153-170 resolveTargetForObservation. #2329 already removed the wait; the surviving hasLiveRunnerSession call at :167 decides whether a pending discovery keeps spinning or hands to the XCTest fallback. Deleting it makes observation spin when a live runner could answer at once. Covered by snapshot-route.test.ts:240,338-360.
11 Repeated bridge retries after the generation circuit opens retain — STOP Owner: packages/platform-apple/src/snapshot-observability.ts:61-74, re-exported ungated at snapshot-route.ts:63. The circuit (disabledGenerations) lives in the capture closure, so awaitObservable re-polls a disabled generation for the full transition window (~33 source.acquire round trips at OBSERVATION_POLL_MS = 150). Removing the retries means adding a circuit gate to the probe, which changes local open/settle timing — #2198's optimization surface, and a platform contract change this issue's stop conditions forbid. Filed as a follow-up rather than done here.
12 Tests asserting removed producer-name branches delete / re-owned Two snapshot-state.test.ts cases whose subject was the presentation-owner branch are gone, superseded by the per-producer exactly-once suite. snapshot-publication-membership.test.ts and snapshot-presentation-transitions.test.ts reached presentation through the legacy producer === undefined door; both now run the production order — engine presents, assembly publishes.

Deliberately not touched (#2199 "Do not delete")

XCTest tree / query-sweep / private-AX backends, the Apple runner manager/cache/.xctestrun/interaction protocol, Swift geometric presentation, TypeScript iOS semantic compaction, generic scope/normalization/refs/quality/occlusion, Android policy, provider acquisition adapters, proxy routes, leases, auth, RPC compatibility. Physical-iPhone behaviour is unchanged: the 'xctest' literals in packages/platform-apple/src/core/physical-device-control.ts and runner/runner-usbmux*.ts are IosPhysicalDeviceBackend values and AppError detail tags — a different type that happens to share the string, and untouched.

Not relocated, with reason

src/snapshot/snapshot-presentation/ios/ (4 test/fixture files) reads like a stale implementation home but holds only coverage of the capture-kit engine. transitions.fixtures.ts is shared with src/daemon/__tests__/snapshot-presentation-transitions.test.ts, whose first case is a daemon-assembly test — moving the fixture into packages/capture-kit would force a cross-package deep import (R11) or a split of that test. That is a test-ownership refactor, not a prune.

Exactly-once proof

src/core/__tests__/ios-snapshot-presentation-once.test.ts, 12 cases.

Presentation. Two halves, both pinned per producer:

  • at least once — the engine's published tree is a fixed point of presentIosInteractiveSnapshot, for simulator-ax-bridge, appium-source, limrun-ios-tree driven through the real presentIosSnapshotAcquisition path;
  • at most oncebuildSnapshotState returns the same (type, label) sequence it was given, for all four producers including apple-runner.

A positive control (the collapsible fixture is not already a presentation fixed point) asserts the fixture genuinely needs compaction, so "already compacted" cannot pass by the rules having stopped matching.

Scope. Per producer, buildSnapshotState is handed snapshotScope: 'no-such-scope' on an xctest tree. A second post-wire pass would return scopeSnapshotNodes' empty no-match slice (snapshot-desktop-projection.ts:44); the tree comes back whole. Same probe covers backendScopesAfterWire's deleted xctest term.

Unrepresentability. buildSnapshotState takes SnapshotCaptureProvenance (whole pair or nothing), so the producer === undefined case cannot be constructed. Enumerating every xctest snapshot-provenance construction site backs this: src/snapshot/ios-snapshot-runtime.ts:47-49 (producer: acquired.acquisition.producer) and packages/platform-apple/src/interactor.ts:250-251 (producer: 'apple-runner') are the only two, and both always set it — SnapshotResult requires SnapshotProvenance at the interactor boundary. Tightening the assembly's input broke only test fixtures, which is the evidence. SnapshotState keeps its optional producer for the three client-side fallbacks that rebuild a state from a bare BackendSnapshotResult (commands/capture/runtime/snapshot.ts:190, interaction/runtime/resolution.ts:700, selector-read-shared.ts:101) and genuinely do not know it; those never reach the assembly.

Planted red

Exactly-once tests, against the restored double presentation

Restoring the deleted branch in buildSnapshotState:

 ❯ src/core/__tests__/ios-snapshot-presentation-once.test.ts (12 tests | 2 failed)
   × the daemon assembly never presents an iOS tree (simulator-ax-bridge) 4ms
   × the daemon assembly never scopes an iOS tree a second time (simulator-ax-bridge) 1ms

 FAIL  src/core/__tests__/ios-snapshot-presentation-once.test.ts > the daemon assembly never presents an iOS tree (simulator-ax-bridge)
AssertionError: expected [ [ 'Application', 'App' ], …(2) ] to deeply equal [ [ 'Application', 'App' ], …(4) ]
-   [ "Button", "General" ],
-   [ "StaticText", "General" ],

Note the composed-path case still passed under the regression: on that fixture a second pass over an already-presented tree is idempotent. The invariant that catches it is the stronger one — the assembly presents at all — which is why it is asserted directly rather than by output comparison alone.

R74, against all three forbidden directions planted in the real tree

node scripts/layering/check.ts:

  [R74 snapshot-assembly-presentation-neutrality] 5 violation(s):
::error file=packages/platform-apple/src/snapshot-source/adapter.ts,line=256,title=Layering drift (R74 snapshot-assembly-presentation-neutrality)::a producer adapter must not import iOS presentation (packages/capture-kit/src/ios-snapshot-engine/index.ts); producers report acquisition facts and the engine presents them
::error file=src/core/snapshot-state.ts,line=151,title=Layering drift (R74 snapshot-assembly-presentation-neutrality)::the daemon snapshot assembly must not import iOS presentation (packages/capture-kit/src/ios-snapshot-engine/index.ts); an iOS capture is presented once, by the engine, before it reaches the assembly
::error file=src/core/snapshot-state.ts,line=152,title=Layering drift (R74 snapshot-assembly-presentation-neutrality)::the daemon snapshot assembly must not import iOS presentation (packages/capture-kit/src/ios-snapshot-acquisition.ts); an iOS capture is presented once, by the engine, before it reaches the assembly
::error file=src/core/snapshot-state.ts,line=154,title=Layering drift (R74 snapshot-assembly-presentation-neutrality)::the daemon snapshot assembly must not branch on the iOS channel or producer name ('xctest'); producer differences enter through typed capabilities, read by the engine
::error file=src/core/snapshot-state.ts,line=154,title=Layering drift (R74 snapshot-assembly-presentation-neutrality)::the daemon snapshot assembly must not branch on the iOS channel or producer name ('simulator-ax-bridge'); producer differences enter through typed capabilities, read by the engine

scripts/layering/snapshot-assembly-presentation-policy.test.ts plants each direction independently and adds a fail-closed case: if the assembly moves out from under SNAPSHOT_ASSEMBLY_FILES, the rule reports rather than passing over an empty file set.

No allowlist was added. The one path that could have needed one — backendScopesAfterWire — was fixed at the declaration instead, by naming the channels that still need post-wire scope so the iOS name disappears from the assembly entirely.

Ownership coverage, and what was already enforced

#2199 requirement Rule
producer adapters cannot import presentation behavior R73 for packages/provider-*; R74 (new) for packages/platform-apple/src/snapshot-source/**, the Simulator bridge adapter R73 did not reach
contracts cannot own algorithms R18 contracts-implementation-authority already bans non-type statements in packages/contracts/src/ios-snapshot.ts; not duplicated
root orchestration cannot reconstruct engine policy R74 (new), directions 1 and 2
reject backend-name scope/presentation branching and direct iOS presenter invocation outside the engine R74 (new), direction 2 (literals) plus direction 1 (imports); the engine's own call topology stays R72's

Gates

Gate Result
pnpm typecheck pass
pnpm lint pass
pnpm format / format:check pass (oxfmt)
pnpm check:layering pass — 193 policy tests, guard OK over 1667 source files
pnpm check:fallow pass — no issues in 24 changed files
pnpm check:production-exports pass (exit 0)
pnpm check:daemon-wire-compat pass — protocol unchanged; no declaration in test/wire-compat/surface.ts was removed or changed, so no ledger edit
pnpm check:di-seams pass — 7 approved seams, no unapproved
pnpm check:gate-manifest pass — 52 checks across 27 lanes
test-file-size ratchet pass
pnpm check:affected --run pass — all runnable checks passed (751 files / 5788 tests)

No fallow baseline was regenerated and none needed moving: nothing deleted here is path-keyed in fallow-baselines/ or fallow-production-exports.json. Native, provider, coverage and full macOS/Simulator lanes remain GitHub-authoritative.

Not done here, and why

The two STOP items in rows 10 and 11 are the only prune-list entries left standing. Both are Simulator observation concerns owned by packages/platform-apple/src/snapshot-route.ts and snapshot-observability.ts, and closing either means adding behaviour (a circuit gate on the probe) rather than deleting a converged path — #2198's surface, and a change to local open/settle timing that #2199's stop conditions put out of bounds.

The daemon snapshot assembly no longer presents. `shouldPresentLegacyIosInteractiveSnapshot`
fired whenever an xctest capture arrived without a producer, or with a producer whose capability
table still named `snapshot-state` as its presentation owner — which `simulator-ax-bridge`
still did after routing moved it onto the engine, so a bridge capture with `--interactive-only`
ran the iOS semantic presentation twice (#2188 invariant 2).

Rather than deleting a runtime guard and hoping, `buildSnapshotState` now takes
`SnapshotCaptureProvenance`: a capture either knows nothing about its origin or carries the whole
pair, so the producer-less branch does not compile. Requiring the pair broke only test fixtures,
which is the proof that production never omitted it.

`presentationOwner` had one value left once the bridge was accounted for, so the capability and
its type are gone; the truncation verdict that read it now reads `truncationEvidence`, which is
the fact it was standing in for and matches it producer for producer. Post-wire scope planning
names the channels that still need the pass instead of excluding the ones that do not, which takes
iOS out of it. `compactIosInteractiveSnapshot` was a byte-identical alias of
`presentIosInteractiveSnapshot` with no production caller.

R74 holds it: the assembly and the Simulator bridge producer adapter may not import iOS
presentation, and the assembly may not name the iOS channel or a producer.
@github-actions

github-actions Bot commented Sep 7, 2026

Copy link
Copy Markdown

Size Report

Metric Base Current Diff
Installed (including dependencies) 4.51 MB 4.51 MB -454 B
Package (unpacked) 4.51 MB 4.51 MB -454 B
Package (download) 1.34 MB 1.34 MB +30 B

Startup median (7 runs, lower is better):

Scenario Base Current Diff
CLI --version 24.9 ms 26.0 ms +1.1 ms
CLI --help 69.1 ms 70.5 ms +1.4 ms

@thymikee

thymikee commented Sep 7, 2026

Copy link
Copy Markdown
Member Author

No code findings at 730c33a: presentation stays in the engine, and the new regressions cover the removed assembly pass. All checks now pass, including the live iOS fixture run with snapshot -i. Before marking this ready, please confirm from the run evidence that the interactive snapshot used simulator-ax-bridge rather than XCTest fallback; that is the producer whose extra presentation pass changed.

The final size/conformance/provider/proxy release evidence remains tracked in #2199, so this pruning PR should not close that issue by itself.

@thymikee

thymikee commented Sep 8, 2026

Copy link
Copy Markdown
Member Author

Confirmed — and worth saying first that the CI evidence could not answer this. The iOS smoke lane records no producer anywhere in its log (I searched the full run for simulator-ax-bridge, producer and bridge markers; the only "producer" hits are the unrelated fixture producer wait policy step). The producer is internal and is not in the published snapshot payload, so a green smoke run alone does not distinguish bridge from fallback. So I ran it live rather than inferring it.

Live run on this PR's head

dist built from this branch's head, dedicated Simulator ad-bench-2198 (iOS 26.2, UDID 93FC0A0B…), CI fixture app com.callstack.agentdevicelab, isolated state dir:

open com.callstack.agentdevicelab --relaunch
snapshot -i        # ×2

Both captures returned 26 nodes, exit 0.

Direct evidence it was the bridge, not XCTest fallback — the XCTest runner's own log for that session, after two snapshot -i captures:

AGENT_DEVICE_RUNNER_LISTENER_READY   1
AGENT_DEVICE_RUNNER_WAITING          1
... snapshot commands: 0

The runner was started and ready (--relaunch prewarms it, and the startup markers are there), and it served zero snapshot commands. The captures did not touch it. If the route had fallen back, the runner would have served them.

Corroborating fingerprint. Both captures emitted:

iOS snapshot acquisition does not provide hittability evidence; regular snapshots omit unverified hittability while raw snapshots preserve supplied facts.

That warning comes from the unavailable-fact: hittability residue, which on a local Simulator only the bridge adapter emits — unconditionally, at packages/platform-apple/src/snapshot-source/adapter.ts:228, in the module whose producer constant is simulator-ax-bridge (:19). The XCTest path (apple-runner) declares hittabilityEvidence: 'available' and does not emit it.

So the interactive snapshot ran through simulator-ax-bridge — the producer whose extra presentation pass this PR removes.

Independently of the live run, the routing is also pinned at unit level in snapshot-route.test.ts: a healthy bridge resolves producer: 'simulator-ax-bridge' with fallback not called.

On #2199

Agreed, and it is already the case mechanically: this PR has no closing keyword and GitHub reports no linked closing issue for it, so merging it will not close #2199. The final size/conformance/provider/proxy release evidence stays tracked there — #2189's package-size baseline for that comparison is 71fb2483f (packed 982,960 B, clean-installed 3,347,347 B / 436 files, bundled gzip 835,279 B / 330 files), and note the PR Size Report bot measures installed including dependencies, so it is not the comparable number.

I am separately diffing snapshot -i output between main and this head on the same device, to establish whether the removed double presentation changed user-visible output or was idempotent on real trees. That determines whether #2199's "update docs only if observable behavior changed" applies. Result follows here.

@thymikee

thymikee commented Sep 8, 2026

Copy link
Copy Markdown
Member Author

main vs this head, same device, same producer: byte-identical

The removed double presentation was idempotent on a real tree — no user-visible output change. So #2199's "update CLI help/user docs only if observable behavior changed" does not apply, and this lands as an invariant fix plus removed wasted work rather than a behavior change.

dist built from main (527a56a6e7) and from this head, same Simulator ad-bench-2198, same fixture app, isolated state dirs:

Leg Producer Nodes vs head
main, attempt 1 XCTest fallback 33 not comparable
main, attempts 2-4 simulator-ax-bridge 26 byte-identical ×3
this head simulator-ax-bridge 26

The first attempt is worth reporting rather than discarding

My first main capture came back with 33 nodes, and the naive reading — "the fix changes output by 7 nodes" — would have been wrong. It carried:

Simulator AX snapshot unavailable (application-server-unavailable); used XCTest for this app generation.

It had fallen back. Re-running until the bridge served it gave 26 nodes, byte-identical to this head, three times running.

Two things follow. First, the fallback is genuinely reachable right after a relaunch, which is exactly why asking to confirm the producer was the right call — a green run proves nothing about which path executed. Second, the two producers' output is visibly different (33 vs 26 nodes, different structure), so the producer fingerprint discriminates cleanly rather than being a subtle distinction.

Scope of the claim

Idempotent on this tree. I am not claiming iOS interactive compaction is idempotent in general — action-shelf retention and row collapse are the rules most likely to differ on a second pass, and this screen may simply not exercise them. That is why the guard belongs where this PR puts it: ios-snapshot-presentation-once.test.ts pins exactly-once per producer regardless of whether a second pass would have been visible, and R74 stops the assembly from acquiring a presentation pass again.

So the honest summary of the defect: on main, a bridge-served snapshot -i ran iOS semantic compaction twice; on this screen the second pass changed nothing observable, but it was wasted work and a standing violation of #2188 invariant 2, and nothing guaranteed it would stay invisible.

Raw outputs and the fallback transcript are in my scratchpad; happy to attach them to evidence/ios-snapshot alongside the #2199 final corpus if you want them as durable evidence.

@thymikee

thymikee commented Sep 8, 2026

Copy link
Copy Markdown
Member Author

The reported live captures distinguish the Simulator AX bridge from XCTest fallback and resolve the remaining producer-evidence gap at 730c33a. No remaining code findings; ready for human review, with checks green. The broader release evidence remains on #2199.

@thymikee thymikee added the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Sep 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-for-human Valid work that needs human implementation, judgment, or maintainer merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant