Skip to content

refactor(ios): delete the unused snapshot plan interface - #2392

Merged
thymikee merged 1 commit into
mainfrom
claude/prune-ios-snapshot-plan
Sep 8, 2026
Merged

refactor(ios): delete the unused snapshot plan interface#2392
thymikee merged 1 commit into
mainfrom
claude/prune-ios-snapshot-plan

Conversation

@thymikee

@thymikee thymikee commented Sep 8, 2026

Copy link
Copy Markdown
Member

Context: #2199, and the follow-up comment that found these two items by following #2383's failure mode — a capability declared for a producer that never consults it.

1. The plan half of the engine was production-dead

Evidence that each deleted symbol had no live consumer, re-verified on be622491f6:

symbol every reference outside its own module
planIosSnapshot ios-snapshot-engine/engine.ts:33 (assigns it into the engine object) and ios-snapshot-planning.test.ts
createIosSnapshotEngine the barrel re-export ios-snapshot-engine/index.ts:1 and engine.test.ts:344
engine.plan(...) nothing — grep -rn "\.plan(" src packages test returns no hits
IosSnapshotPlan only its own declaration, planIosSnapshot's return type, and the plan member of IosSnapshotEngine
IosSnapshotAcquisitionNarrowing only IosSnapshotPlan.narrowing

Production reaches presentation through publishIosSnapshot / presentIosSnapshot directly. Neither fallow gate saw it: createIosSnapshotEngine is reachable from a package subpath entry, which is what the "only entry surfaces re-export" rule exists to prevent. No ADR pins it — ADR 0004's "capture plan" is an ordered set of capture backends under a shared wall-clock budget, a different concept from IosSnapshotPlan.

Deleting the plan also takes the last reader of scopeCompleteness, interactiveQueryCompleteness and viewportEvidence (only planIosSnapshot ever read them) and of the not-applicable depth-support variant (only the apple-runner row and planIosSnapshot's presented branch ever built one), so those go with it. IosSnapshotCompleteness had no remaining referent afterwards.

I did not delete IosSnapshotEngine itself — it is the closed-surface guard for "presentation happens exactly once" (#2188 invariant 2), pinned by packages/contracts/src/ios-snapshot.test.ts. It now has one member, and engine.test.ts pins publishIosSnapshot to it so the contract describes the export production actually calls rather than a shape only the deleted factory satisfied.

2. The seam for the capability table, and why drift becomes unrepresentable

IOS_SNAPSHOT_PRODUCER_CAPABILITIES was Record<IosSnapshotProducer, …> — all four producers — but its residue-shaping fields were consumed only by planIosSnapshot (dead) and by createIosSnapshotAcquisition, whose parameter is IosProviderAcquisitionProducer. It had already drifted: it declared simulator-ax-bridge with hittabilityEvidence: 'available' while packages/platform-apple/src/snapshot-source/adapter.ts:228 emits { kind: 'unavailable-fact', fact: 'hittability' } unconditionally. The residue is the truth; the declaration was wrong.

Two tables now, each keyed on exactly the producers a consumer asks about:

  • IOS_PROVIDER_ACQUISITION_CAPABILITY_VALUESRecord<IosProviderAcquisitionProducer, IosProviderAcquisitionCapabilities>, module-private. apple-runner and simulator-ax-bridge have no row, so hittabilityEvidence for them does not merely become correct, it stops being expressible: adding one back is a compile error, not a wrong value. The drifted field is deleted rather than fixed, which is the point — fixing the value would leave the channel open. This table is also no longer exported, so a future reader cannot re-acquire it through the subpath the way the barrel kept createIosSnapshotEngine alive.
  • IOS_SNAPSHOT_TRUNCATION_EVIDENCERecord<IosSnapshotProducer, IosSnapshotEvidenceAvailability>, read through iosSnapshotTruncationEvidence(producer). This is the one fact all four producers owe an answer to, so it is a table of its own instead of a column of the provider capabilities: every producer states it exactly once, in the only place that asks. deriveIosSnapshotAcquisitionResidue reads it from here too, so there is no second declaration to drift against.

The producer.stage === 'acquired' guard in the residue derivation went with the narrowing — every row in a provider-keyed table is acquired by construction, so the guard was a tautology.

3. Proof that snapshotTruncationForResult is unchanged

apple-runner and simulator-ax-bridge were truncationEvidence: 'available' before and are 'available' now, and that is correct: IosRunnerPayloadFacts.truncated is a required boolean, and the bridge adapter rejects an envelope whose truncated is not a boolean (adapter.ts:178) and turns a true into a truncated residue. Two tests pin it:

  • packages/capture-kit/src/ios-snapshot-planning.test.tsiosSnapshotTruncationEvidence returns the expected value for each of the four producers; the source table is satisfies Record<IosSnapshotProducer, …>, so a new producer cannot be added without declaring one.
  • src/commands/capture/runtime/snapshot.test.ts — drives the real capture.snapshot command with an absent truncated for each of the four producers and asserts the observable result: false for apple-runner and simulator-ax-bridge, undefined for appium-source and limrun-ios-tree. This extends the single pre-existing Appium case to the whole producer axis.

No observable CLI behavior changes. The provider (Appium/Limrun) path keeps the table it legitimately uses, and its derived residue is unchanged — the residue test now covers both provider producers instead of Appium alone.

Gates

gate result
pnpm typecheck pass
pnpm lint pass
pnpm format applied, clean
pnpm check:layering pass — 193/193
pnpm check:fallow pass — no issues in 12 changed files
pnpm check:production-exports pass (exit 0); the informational unused-export count drops 65 → 64, the one removal being createIosSnapshotEngine
pnpm check:affected --run 3 pre-existing failures, unrelated (see below)

No test/wire-compat change was needed: the daemon RPC wire surface digests nothing under ios-snapshot.ts, and no removed declaration is listed in surface.ts.

The three check:affected failures

pnpm check:affected --run reports 3 failed / 2645 passed. All three are daemon dead-owner
reconciliation and runner-prewarm tests, none of which touch iOS snapshot presentation, the
capability table, or truncation:

  • src/daemon/__tests__/request-router-open.test.ts :: open reconciles a foreign dead owner through that owner state dir
  • src/__tests__/daemon-entrypoint.test.ts :: startup sweep settles a foreign dead owner without touching a live same-named session
  • src/daemon/session-lifecycle/internal/__tests__/session-open-url-prewarm.test.ts :: prepare ios-runner starts the XCTest runner on an explicit iOS selector

They reproduce on unmodified be622491f6: running those three files on the base commit fails
5 tests, versus 3 on this branch. Two consecutive runs of the full gate on this branch also
produced different failure sets (5 then 3), which is the machine's signature rather than a
regression — this box was at load average 60-95 with swap nearly full throughout.

`planIosSnapshot`, `IosSnapshotPlan`, the `plan` member of `IosSnapshotEngine`
and `createIosSnapshotEngine` had no production caller: production reaches
presentation through `publishIosSnapshot` / `presentIosSnapshot` directly, and
the barrel re-export was all that kept the factory alive for fallow.

Deleting the plan takes the last reader of most of
`IOS_SNAPSHOT_PRODUCER_CAPABILITIES` with it. The table was typed over all four
producers while only the two provider producers ever consumed its
residue-shaping fields, and it had already drifted: it declared
`simulator-ax-bridge` with `hittabilityEvidence: 'available'` while the bridge
adapter emits `unavailable-fact: hittability` on every capture. Rather than
correct the value, the table is now keyed on `IosProviderAcquisitionProducer`,
so a producer that builds its own facts cannot declare one at all.

Truncation is the one capability the runner and the bridge still need answered,
so it moves to a table of its own over all four producers, read through
`iosSnapshotTruncationEvidence`. Both keep `'available'`, which is what the
adapter and the runner payload actually prove.
@github-actions

github-actions Bot commented Sep 8, 2026

Copy link
Copy Markdown

Size Report

Metric Base Current Diff
Installed (including dependencies) 4.51 MB 4.51 MB -484 B
Package (unpacked) 4.51 MB 4.51 MB -484 B
Package (download) 1.34 MB 1.34 MB -92 B

Startup median (7 runs, lower is better):

Scenario Base Current Diff
CLI --version 26.8 ms 27.4 ms +0.5 ms
CLI --help 77.3 ms 76.9 ms -0.4 ms

@thymikee

thymikee commented Sep 8, 2026

Copy link
Copy Markdown
Member Author

Reviewed and independently verified. Three things I checked rather than took on trust:

The 3 reported failures are contention, not regression — settled properly. Comparing failure counts across branches (5 on main vs 3 here) is a weak signal, so I re-ran all three named files serially on this branch instead:

src/daemon/__tests__/request-router-open.test.ts                          14 passed
src/__tests__/daemon-entrypoint.test.ts                                    6 passed
src/daemon/session-lifecycle/internal/__tests__/session-open-url-prewarm.test.ts  16 passed
Test Files 3 passed (3) · Tests 36 passed (36)

Including all three named tests. The box was still at load 52 with swap nearly full; --maxWorkers=1 is what makes it deterministic. Nothing here is a regression.

The truncation split is behavior-preserving, by value. The new IOS_SNAPSHOT_TRUNCATION_EVIDENCE reproduces the old table exactly — apple-runner and simulator-ax-bridge 'available' (both overrode the default before), appium-source and limrun-ios-tree 'unavailable' (both inherited it from ACQUIRED_PRODUCER_CAPABILITY_DEFAULTS). So snapshotTruncationForResult cannot move, which the new per-producer runtime test pins observably.

The drift channel is actually closed. IOS_PROVIDER_ACQUISITION_CAPABILITY_VALUES now has rows only for appium-source and limrun-ios-tree; simulator-ax-bridge has none, so the hittabilityEvidence: 'available' that contradicted the adapter's live residue is deleted rather than corrected, and satisfies Record<IosProviderAcquisitionProducer, …> makes putting it back a compile error. Making the table module-private also shuts the barrel-reachability hole that hid createIosSnapshotEngine from fallow in the first place — that is the part that stops this recurring, and it is a better outcome than the fix I asked for.

Keeping IosSnapshotEngine single-member is the right call: it is live coverage for #2188 invariant 2 in contracts/src/ios-snapshot.test.ts, and pinning publishIosSnapshot to it means the contract now describes what production actually calls instead of a shape only the deleted factory satisfied.

−135 lines net. No behavior change, no allowlist, no compatibility re-export.

@thymikee

thymikee commented Sep 8, 2026

Copy link
Copy Markdown
Member Author

Ready for human review at 3b6bbd2. The deleted plan and factory have no production callers, and the narrowed capability table preserves provider residue and all four truncation answers through the real snapshot command. No code findings; all required checks pass.

@thymikee thymikee added the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Sep 8, 2026
@thymikee
thymikee merged commit a6cf1b1 into main Sep 8, 2026
18 checks passed
@thymikee
thymikee deleted the claude/prune-ios-snapshot-plan branch September 8, 2026 10:41
@github-actions

github-actions Bot commented Sep 8, 2026

Copy link
Copy Markdown
PR Preview Action v1.8.1
Preview removed because the pull request was closed.
2026-09-08 10:41 UTC

thymikee added a commit that referenced this pull request Sep 8, 2026
…cope-contracts

* origin/main: (25 commits)
  fix(ios): avoid duplicate alert routing queries (#2398)
  docs(adr): record the #2278 coupling audit and re-trace entry-to-platform hops (#2355)
  chore(gates): classify #2278 daemon-platform-runtime edges and ratchet handler session authority (#2354)
  test(capture): shrink the 3x crop fixture to avoid coverage timeouts (#2399)
  perf(ios): settle the first interaction on a deadline, not a fixed sleep (#2395)
  feat: support standalone Maestro clearState command (#2366)
  refactor(capture-kit): complete ADR 0019 end state — relocate snapshot and recording zones (#2385)
  refactor(ios): delete the unused snapshot plan interface (#2392)
  fix(conformance): isolate post-tap settling from app launch (#2390)
  test(remote): assert the proxied snapshot's whole wire conversation (#2387)
  fix(daemon): stop an attested tenant from downgrading its own isolation (#2386)
  refactor(ios): prune converged snapshot paths (#2383)
  refactor(move): move the batch runner and batch policy into @agent-device/command-registry (#2388)
  fix(remote): let a plain-session client read its own failure record (#2382)
  refactor(move): move replay divergence vocabulary into @agent-device/ad-replay (#2384)
  refactor(cli): take the CLI's vocabulary off runtime barrels (#2379)
  perf(cli): keep scroll vocabulary off gesture runtime (#2378)
  chore: drop the stale planned-operations closure approval (#2373)
  fix(gates): stop an eager-closure approval from turning main red on merge (#2375)
  fix(ios): honor the startup budget through a cold Simulator boot (#2325)
  ...
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