Skip to content

fix(ios): isolate snapshot acquisition timing from presentation - #1948

Merged
thymikee merged 3 commits into
mainfrom
fix/ios-snapshot-phase-timing
Aug 21, 2026
Merged

fix(ios): isolate snapshot acquisition timing from presentation#1948
thymikee merged 3 commits into
mainfrom
fix/ios-snapshot-phase-timing

Conversation

@thymikee

@thymikee thymikee commented Aug 21, 2026

Copy link
Copy Markdown
Member

Summary

Implements the acquisition/presentation timing-isolation slice of #1797.

  • Measures backend acquisition and runner-side presentation separately within each capture-plan attempt.
  • Restricts XCTest slow/timeout penalty decisions to qualifying acquisition time and failures, so slow presentation cannot arm the breaker.
  • Reports acquisition and presentation timing once at response level through the existing snapshot-quality diagnostics contract.
  • Preserves total command deadlines, fallback ordering, clip and quality-reason semantics, visible-depth behavior, backend differential lanes, and the file-private presentation construction boundary from PR fix(ios): make snapshot presentation construction private #1937.
  • The response-level timing fixture assigns its SnapshotCaptureTiming to SnapshotBackendCapture; a direct-presentation default test confirms timing remains omitted when no capture plan stamps it.
  • The final iOS reruns exposed an existing automation-channel race in the hidden-keyboard text canary: commit verification resolved XCUI state every 20 ms while the synthesized text event was still in flight. The owning wait now observes at a dedicated 200 ms cadence, preserving the 3 s deadline while allowing the target to consume the event.
  • Scope expanded from five to seven files for that CI-owned text-entry fix; there is no snapshot/text fallback or duplicated execution path.
  • Exact local size comparison is +3.4 kB npm unpacked: +3.0 kB Apple runner source/project and +426 B JS/dist. The focused RunnerTests+SnapshotTiming.swift module adds +3.1 kB, RunnerTests+SnapshotCapturePlan.swift removes 382 B, and the text-entry cadence fix adds 272 B across its two owning files.
  • The timing module is intentional: the capture-plan file is already over 1,000 lines, so keeping the typed phase carrier, injectable clock seam, penalty policy, and deterministic timing tests in a focused module avoids further coupling to plan orchestration. A smaller parameter-only seam would duplicate date arithmetic or lose the typed failure phase needed by the breaker.

This PR addresses only the timing-isolation slice and does not close all of #1797.

Validation

  • Exact base head: d2f2dae798153a489623ac0e65d50f6c83f2dec3 (origin/main).
  • Exact PR head: b0122094552a9661af97b56a58dc9df37e9f57ad.
  • pnpm check:affected --run passed: 5,304 related tests plus format, lint, typecheck, layering, Fallow, build, and XCTest selection.
  • iPhone 17 Pro simulator, iOS 26.2: the unchanged hidden-keyboard canary failed in the pre-fix five-iteration stress run; it passed 5/5 after the cadence fix, with every 17-character commit completing in 0.56–0.57 s.
  • The complete 67-test iOS PR XCTest selection passed on the rebased head, including snapshot timing/presentation and the hidden-keyboard canary.
  • Local package comparison: +234 B JS raw, +54 B JS gzip, +996 B npm tarball, +3.4 kB npm unpacked.

@github-actions

github-actions Bot commented Aug 21, 2026

Copy link
Copy Markdown

Size Report

Metric Base Current Diff
JS raw 2.36 MB 2.36 MB +234 B
JS gzip 791.5 kB 791.6 kB +54 B
npm tarball 910.7 kB 911.7 kB +996 B
npm unpacked 3.15 MB 3.16 MB +3.4 kB

npm unpacked components

Component Base Current Diff
JS / dist source 2.51 MB 2.51 MB +426 B
Apple runner source/project 544.4 kB 547.4 kB +3.0 kB
macOS helper source 54.5 kB 54.5 kB 0 B
Android helper artifacts 0 B 0 B 0 B
Other package files 44.4 kB 44.4 kB 0 B

Startup median (7 runs, lower is better):

Scenario Base Current Diff
CLI --version 27.6 ms 26.0 ms -1.6 ms
CLI --help 82.1 ms 77.8 ms -4.3 ms

Top changed chunks: no changes in the largest emitted chunks.

Top changed packed files

Packed file Base Current Diff
dist/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+SnapshotTiming.swift 0 B 3.1 kB +3.1 kB
dist/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+SnapshotCapturePlan.swift 31.0 kB 30.6 kB -382 B
dist/src/verdict.js 1.5 kB 1.8 kB +234 B
dist/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+SynthesizedTextEntry.swift 10.5 kB 10.7 kB +207 B
dist/src/sdk-contracts.d.ts 16.0 kB 16.2 kB +192 B
dist/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+TextEntry.swift 19.9 kB 20.0 kB +65 B

@thymikee

Copy link
Copy Markdown
Member Author

P1: testSnapshotQualityCarriesPhaseTimingAtResponseLevel creates a local timing value but constructs SnapshotBackendCapture without assigning timing, then expects stampedSnapshotPayload to contain that timing. The capture therefore carries nil and the new host XCTest should fail once this exact head reaches that lane. Set the capture timing in the fixture so the test actually pins response-level propagation, and include a negative/default case if direct presentation is meant to omit it. The current Size result is +3.1 kB unpacked, so the body also needs exact growth attribution and why a smaller interface was rejected.

@thymikee

Copy link
Copy Markdown
Member Author

Re-reviewed exact head 5b70fa34. The prior timing-fixture blocker is resolved: the positive fixture now assigns timing to SnapshotBackendCapture, the direct-presentation negative case pins omission, and Host XCTests pass. I traced the shipped route from capture-plan execution through separate phase measurement, acquisition-only breaker policy, response stamping, and verdict parsing; no further code findings. The +3.1 kB unpacked growth is itemized, and the focused timing module is justified against further growth in the 1,099-line plan file. All completed gates are green and the branch is mergeable. Code review is clean; merge readiness still awaits the exact-head iOS Smoke/live simulator lane, which is currently queued.

@thymikee thymikee added the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Aug 21, 2026
@thymikee
thymikee force-pushed the fix/ios-snapshot-phase-timing branch from 5b70fa3 to b012209 Compare August 21, 2026 18:44
@thymikee

Copy link
Copy Markdown
Member Author

Fixed and rebased at b012209455. The repeated iOS failure was the hidden-keyboard text canary starving its own in-flight synthesized event with XCUI resolution every 20 ms; snapshot timing tests were not failing. Commit observation now has a dedicated 200 ms cadence while retaining the 3 s refusal deadline. The unchanged canary was red in a five-iteration pre-fix stress run, then passed 5/5 after the fix (full 17-character commits in 0.56–0.57 s). The complete 67-test iOS PR XCTest selection and pnpm check:affected --run (5,304 related tests plus all runnable gates) pass on the rebased head. PR body now records the seven-file scope expansion and exact +3.4 kB unpacked attribution. CI has been triggered on the new head; no further code blocker is known.

@thymikee

Copy link
Copy Markdown
Member Author

Exact-head follow-up: all required checks are now green, including the rerun Coverage lane and iOS Smoke. The runner-timing path and synthesized-text canary were validated on this head, the branch is clean/mergeable, and the +3.4 kB unpacked growth is itemized in the PR body. No remaining findings; merge-ready.

@thymikee
thymikee merged commit 3e584d1 into main Aug 21, 2026
42 of 43 checks passed
@thymikee
thymikee deleted the fix/ios-snapshot-phase-timing branch August 21, 2026 20:10
@github-actions

Copy link
Copy Markdown
PR Preview Action v1.8.1
Preview removed because the pull request was closed.
2026-08-21 20:11 UTC

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