Skip to content

perf(ios): admit tap text-input probes from fresh target evidence - #3881

Open
thymikee wants to merge 6 commits into
mainfrom
perf/ios-tap-text-probe-admission
Open

thymikee wants to merge 6 commits into
mainfrom
perf/ios-tap-text-probe-admission

Conversation

@thymikee

@thymikee thymikee commented Oct 21, 2026 •

Copy link
Copy Markdown
Member

Summary

Skip the iOS tap text-input probe only with fresh, same-target capture geometry proving that no input contains the dispatch point. Clear the remembered typing witness on skip. Contracts carry facts; Apple runtime forwards them; Swift admits them. Collect raw input geometry before presentation can hide editable nodes. Unknown geometry and coordinate-only taps retain probing. 24 files; scope includes capture-private geometry to preserve ADR 0011 and #2438/#2554 routing.

Validation

Base: c8b944ca0116d577941fb47ba82c84504f9b3075. Head: b91a12d556230d5963e8b67c1b89963b41c44b11.

  • pnpm check:affected --run --head b91a12d556: exit 0, exactCommit=true; receipt .tmp/validation/run-ZIax2n/receipt.json; 8,205 tests passed. Format, selection check, TypeScript/iOS builds passed.
  • Focused native tests: head 4 passed; equivalent base regression 1 failed/2 passed at the intended skip assertion. Geometry mutation: control 5 passed; mutant 1 failed/4 passed. Independent review: no remaining findings.
  • Bluesky 1.136.0/iOS 27.0, three runs each, milliseconds (runner / CLI wall): Home ref base 533,552,545 / 831,792,794; head 416,424,420 / 596,595,679. Search selector base 564,572,576 / 934,992,1277; head 414,425,420 / 776,746,757. Median runner improvements: 23.1%/26.5%. All taps landed. Text-field tap → bare type verified; coordinate-only probe retained.

CI owns cross-platform native/integration/coverage obligations. Review capture completeness, ref freshness, system-target matching, and conservative fallback admission.

Implemented by a Codex worker; opened and reviewed by the coordinating Claude session.

🤖 Generated with Claude Code

View guided diff Turn on auto-fix

@github-actions

github-actions Bot commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

Size Report

Metric Base Current Diff
Installed (including dependencies) 5.52 MB 5.53 MB +3.8 kB
Package (unpacked) 5.52 MB 5.53 MB +3.8 kB
Package (download) 1.67 MB 1.68 MB +1.2 kB

Startup median (7 runs, lower is better):

Scenario Base Current Diff
CLI --version 38.6 ms 41.5 ms +3.0 ms
CLI --help 79.7 ms 79.6 ms -0.1 ms

thymikee and others added 2 commits October 21, 2026 21:42
The macOS host lane requires every runner request field to have a
production request in runner-requests.json; the new
coordinateTapTextInputEvidence field had none.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@thymikee

Copy link
Copy Markdown
Member Author

Addressed the review in 309c058486278a2df67c7f22f742722f1d41963b.

  • P2-1 fixed: admission calls isCoordinateTextInputCandidate(enabled: true, frame: rect, point: point), sharing the probe's 2 pt tolerance. The golden table now checks 1 pt outside (probe) and 3 pt outside (skip).
  • P2-2 fixed: a retained ref tree supplies evidence only while it is the session's latest snapshot by identity and is not an outdated observation. Fresh captures consumed by the request remain eligible. Added route tests for a newer read-only observation with an input at the old tap point and for the same observation marked outdated.
  • P3-1 addressed: retained the early witness clear and documented its throw-path protection.
  • P3-2 fixed: removed test-only default values; callers supply all admission arguments.
  • P3-3 fixed: source is diagnostics only; Swift does not treat the daemon's tag as verification.
  • P3-4 fixed: skip markers include reason=evidence or reason=penalized-xctest-channel and the diagnostic source.
  • P3-5 fixed: finite zero-size input frames are omitted without discarding other geometry. Missing, negative, and non-finite frames still prevent exclusion.
  • P3-6 fixed: wrapper fixture children now retain their actual parent and depth.
  • P3-7 fixed: removed the impossible same-target bridge/system-surface fixture and replaced the other row with a producible app capture followed by a changed dispatch target.
  • P3-8 fixed: acquisition geometry owns completeness; removed duplicate daemon checks.

Regression evidence against original PR head b91a12d556230d5963e8b67c1b89963b41c44b11, using only the new test inputs in /private/tmp/pr3881-regression:

  • TypeScript: 4 failed / 34 passed without the fixes; 38 passed with them. Failures: a newer read-only observation keeps probing when the ref frame predates an input at the tap point; an outdated observation keeps probing even when it still owns the active refs; both an empty input frame preserves the remaining geometry cases (0/40, 40/0).
  • Swift: 2 failed without the fixes (testCoordinateTapTextInputAdmissionMatchesGoldenTable, specifically point-1pt-outside-input; testCoordinateTapTextInputEvidenceSourceIsDiagnosticOnly). Fixed runner: all 6 focused admission/witness tests passed, none skipped, including testFreshCoordinateTapEvidenceSkipsProbeAndClearsTypingWitness, testCoordinateTapEvidenceContainingInputPreservesBareTypingWitness, testHealthyCoordinateTapPreservesBareTypingWitness, and the penalized-channel test.
  • Required-argument compilation, nested fixture inputs, producible target-change coverage, retained witness clearing, and acquisition completeness coverage also passed. Cosmetic/comment/ownership changes do not each have a fabricated behavioral regression test.

WebView device evidence: dedicated iPhone 17 Pro simulator 7A872BE0-AA92-4BDE-B493-9B6DDE1B0E16, iOS 27.0, current JS repacked into the Release fixture. On the WebView lab, snapshot -i disclosed Simulator AX snapshot unavailable (circuit-disabled); used XCTest for this app generation. Tapped Email address @e25, issued bare type probe-webview-3881@example.test, and get text returned that exact value. Typing route: synthesized-first-responder. The tap's runner segment enumerated TextField/SecureTextField/SearchField/TextView and had zero PROBE_SKIPPED markers. This runner-produced capture carries no acquisition geometry, so exclusion evidence was absent and probing remained enabled. This proves this WKWebView fixture; it does not independently prove every remote service or SFSafariViewController.

Independent read-only follow-up review of b91a12d556..309c058486: no findings.

Validation: pnpm check:affected --run --head 309c058486278a2df67c7f22f742722f1d41963b exited 0, status=passed, exactCommit=true, checkoutChanged=false. Receipt: .tmp/validation/run-ev3fCx/receipt.json. 996 test files / 8,214 tests passed. Format, lint, typecheck, layering, Fallow, build, XCTest selection, and packaged-runner Swift passed. pnpm check:xctest-selection also passed separately. The unit-enabled iOS runner was built in this worktree's .tmp/ios-runner-derived and the focused tests ran with xcodebuild test-without-building on the owned simulator. Coverage, other native/platform/device lanes remain CI obligations.

Updated Bluesky comparison: same copied Bluesky 1.136.0 app/account, iPhone 17 Pro / iOS 27.0, Xcode 27.1, after login, notification dismissal, and SafariViewService termination on the owned simulator. Compared pre-optimization baseline c8b944ca0116d577941fb47ba82c84504f9b3075 with fixed head 309c058486278a2df67c7f22f742722f1d41963b, three rounds each after local validation finished. Each round: Search by role="button" label="Search", fresh snapshot -i, Home by the emitted @ref, then snapshot verifying Home. Every command succeeded and every navigation landed. Baseline log records its own /private/tmp/pr3881-regression/.tmp/ios-runner-derived xctestrun, so this was a rebuilt baseline runner rather than adoption of the head binary.

  • Home ref runner ms: baseline 1134, 1143, 1137; fixed 756, 622, 622. Medians 1137 → 622 (45.3% lower).
  • Search selector runner ms: baseline 649, 372, 374; fixed 344, 340, 343. Medians 374 → 343 (8.3% lower).
  • Home ref CLI wall ms: baseline 1493, 1417, 1427; fixed 1416, 991, 966. Medians 1427 → 991 (30.5% lower).
  • Search selector CLI wall ms: baseline 3155, 812, 926; fixed 3791, 1053, 831. Medians 926 → 1053 (13.7% higher). This small sample does not establish a wall-time improvement for Search.
  • Fresh Home refs still skip 3/3 (reason=evidence source=ref-frame), with zero text-input query enumerations in all three tap segments. Search selectors also skip 3/3 (source=selector-capture). Baseline skips 0/3 for both routes. Raw samples and per-tap runner segments: .tmp/pr3881/{head,base}-measurements.json and .tmp/pr3881/{head,base}-*-tap-*-runner.log; summary .tmp/pr3881/timing-summary.json. Runner time is command-accepted → successful command-completed; wall time includes the same Device panel launcher/client wrapper for both SHAs. The result varies from the earlier 23%/27% observation, so these are the updated samples, not a claim that those exact percentages persist.

Publication and cleanup: pushed 309c058486278a2df67c7f22f742722f1d41963b by fast-forward to the same remote branch. The branch was already held by another worktree; implementation stayed detached at the fetched head and did not alter that checkout or create a branch. No PR opened or GitHub comment posted. The fixed and baseline sessions were closed, both manually started task daemons were stopped, the Device panel was closed, the dedicated simulator was deleted, and the disposable regression/baseline worktree was removed. No Metro process was needed. Cleanup logs: .tmp/pr3881/{head,base}-close.json, .tmp/pr3881/final-daemon-stop.json, .tmp/pr3881/final-device-status.json, .tmp/pr3881/final-simulators.json. The daemon stop command could not verify the manually started daemon's identity; each was stopped only after checking its recorded PID/start time/executable against the process this task launched. Final cleanup completed.

CI handoff: at original head b91a12d556, Integration Tests passed and Coverage failed (Run coverage, exit 1; available log does not identify a specific failing assertion). This is not claimed as a CI pass or an infrastructure diagnosis. The follow-up head has been pushed and starts a new CI run; its results remain pending. The coordinator should check Coverage and Integration Tests at 309c058486 before declaring merge readiness. Existing Coverage job: https://github.com/callstack/agent-device/actions/runs/39661564416/job/119435606948

Separate CI fix in 4db71daa24: the macOS host lane's testEveryRunnerRequestFieldHasAProductionRequest failed on b91a12d556. The new coordinateTapTextInputEvidence field had no production request in contracts/fixtures/runner-requests.json. I added the capture site ios-simulator.interactions-single-press-tap.text-input-evidence and its fixture entry. Validation:

  • the macOS XCTest passes locally (macosx27.0-arm64);
  • both runner-request Vitest files pass;
  • pnpm check:affected --run --head 4db71daa24 passed.

The earlier Coverage failure on b91a12d556 ended mid-log with no failing test, the same runner-kill signature seen on #3868, which passed on re-run.

@thymikee

Copy link
Copy Markdown
Member Author

I reviewed 4db71da. The fast path can skip a text-input probe that the baseline would run, so this needs a fix before merge.

For a selector tap, the evidence closure in interaction-runtime.ts always attaches the geometry from the consumed capture. When that capture placed the target offscreen, the #1542 rescue in target-visibility-stages.ts swaps in a live rect, and the tap goes to that live point. The live read just showed the capture is wrong for this screen, yet the capture still serves as exclusion evidence. Take a selector aimed at a text field whose captured rect was offscreen, for example after the keyboard shifts content. The captured input rect does not contain the live point, so Swift skips the probe and clears the remembered-tap witness. The baseline would probe, find the field and remember it. After click <selector> on such a field, a bare type loses its tap witness and falls to a different typing route. No test covers rescue plus evidence. The rule should be that geometry may admit a skip only when it comes from the same observation that produced the dispatch point. Target resolution (the resolver in target-visibility-stages that yields the point) should return the point together with its source observation, and return none when the point came from a live rescue rect or any other non-capture source. The tap backend should attach evidence only from that pair. Please add a route test where the rescue moves the point onto a captured-offscreen input, and assert the probe is kept.

Could the selector/ref branching in the evidence closure go away entirely? If the resolver returned {point, observation}, with observation being the SnapshotState the point came from (or undefined for a live rescue rect or a raw coordinate), the tap backend could call buildCoordinateTapTextInputEvidence(observation, point, ...) directly. Ref freshness would then be one check on that observation, and rescue staleness would be handled by construction. The first change needed is that the tap(point) member in the interaction runtime accepts the resolved observation alongside the point, which means a tap signature change in contracts/backend.ts or a resolved-target object.

Not blocking, and fine to take or leave: import Point at the top of packages/daemon/src/core/interaction/internal/types.ts instead of the inline import type, note that the Swift point-equality guard is tautological today because the same command carries both values, and consider making activeAppBundleId a required argument of executeOnMainPrepared, since a caller that forgets it silently keeps probing.

The Bluesky and WebView runs cover a fresh ref and a fresh selector, which is useful. After the fix, one more live iOS simulator run is needed: a selector tap on a text field whose capture reports it offscreen while it is live on screen (the #1542 rescue, for example a field shifted by the keyboard). The runner log should show the text-input query enumeration with no PROBE_SKIPPED marker, and a following bare type should land in that field, confirmed by get text. I did not rerun the TS or Swift regression tests, so whether the new tests fail on the old head rests on your report. I also could not see the .tmp/pr3881 logs. I did not verify on device whether the AX bridge acquisition includes remote-hosted inputs such as SafariViewService or ASWebAuthenticationSession. If it does not, complete-looking geometry could omit them and skip the probe, with the same consequence. I also did not check how often the #1542 rescue fires on text fields in practice, so the mechanism above comes from reading the code, not from a device reproduction.

The Coverage job log stops at 20:11 while building the macOS helper, and it names no failing test or threshold. That matches the runner-kill signature you cite from #3868, but this PR touches routes that Coverage runs, so a real failure is not ruled out. Please re-run Coverage to tell which it is.

Before merge, tie the evidence to the observation the dispatch point came from, add the rescue route test, and re-run Coverage.

thymikee and others added 2 commits October 22, 2026 05:11
The iOS snapshot presenter imported the text-input geometry helpers through
the contracts capture facade, which made it evaluate seven more modules on
import. A narrow @agent-device/contracts/snapshot-private-evidence export
leaves one: the private-evidence module itself.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Approved by the maintainer for #3881: the presenter now evaluates the
capture-private evidence module to attach text-input geometry. Records the
new @agent-device/contracts/snapshot-private-evidence subpath in the
contracts export snapshot.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@thymikee

Copy link
Copy Markdown
Member Author

Coverage was failing for a real reason, not a runner kill. The step's log just never printed it. Locally, pnpm gate unit-ci at 4db71daa24 failed scripts/__tests__/eager-closure-budgets.test.ts: capture-kit/src/snapshot/ios-snapshot-runtime.ts evaluated 46 modules on import, against 39 at the merge-base, because it imported the geometry helpers through the @agent-device/contracts/capture facade.

Fixed in two commits:

  • a533b05d06: a narrow @agent-device/contracts/snapshot-private-evidence export, now imported directly, so the presenter adds only that one module (39 → 40).
  • b74fcebdc1: chore(gates), the maintainer-approved APPROVED_CLOSURE_ADDITIONS row for that module (owner thymikee, perf(ios): admit tap text-input probes from fresh target evidence #3881), plus the regenerated contracts export snapshot.

Validation at b74fcebdc1:

  • pnpm gate unit-ci: 1,967 files / 16,926 tests passed.
  • pnpm check:affected --run --head: passed (exactCommit: true).

Other routes tried and rejected:

  • A separate geometry module: the capture facade and snapshot-state.ts load it widely, which grew 51 entries.
  • A lazy import: the presenter is synchronous.

@thymikee

Copy link
Copy Markdown
Member Author

The Lint failure here comes from main, not from this PR. snapshot-route.test.ts is 1003 lines on main after #3873/#3874/#3877 (cap 1000), and this branch doesn't touch it. The fix is #3890. Once #3890 lands, this PR needs a re-run.

@thymikee

Copy link
Copy Markdown
Member Author

The earlier review of 4db71da is partly resolved at b74fceb: the subpath export and the eager-closure budget row are in, and the unit-ci report from the author is clear. One defect from that review remains, and the live run it asked for has not been supplied.

For a selector tap, the closure still builds evidence from consumedCapture.state. When the #1542 rescue in target-visibility-stages.ts swaps a captured-offscreen rect for a live rect, the dispatch point comes from the live read, but the stale capture still counts as exclusion evidence. Its input rect does not contain the live point, so Swift skips the probe and clears the remembered-tap witness. A click <selector> on a text field that the capture placed offscreen (for example, content shifted by the keyboard) then loses the witness, and a following bare type takes a different route than the baseline. The rule should be: exclusion evidence comes only from the observation that produced the dispatch point. Please have target resolution return {point, observation}, with observation undefined for a live rescue rect or a raw coordinate, and have tap() call buildCoordinateTapTextInputEvidence(observation, point, ...) directly. Please add a route test where the rescue moves the point onto a captured-offscreen input and asserts that the probe is kept.

The changed device path is still unproven on the route where the stale evidence applies. After the fix, please run on an iOS simulator a selector tap on a text field that the capture reports offscreen while it is live on screen, so the rescue fires. runner.log must show the text-input query enumeration and no PROBE_SKIPPED marker. Then a bare type must land in that field, confirmed by get text.

Would returning {point, observation} from the resolver simplify this? The tap backend would build evidence from that pair, the selector/ref branching and the refFrameTree/isOutdatedObservation checks in the closure would collapse into one check on the observation, and a rescue or raw point would get no evidence by construction. This was raised in the earlier review and is still open. It needs either the tap(point) signature in contracts/backend.ts to accept the resolved observation, or a resolved-target object that carries it.

Not blocking: the geometry helpers now have two import spellings, the new subpath and the re-export in facades/capture.ts, and you can keep the subpath as the single spelling for capture-private evidence and drop the facade re-export once its other importers no longer need it, or leave it as is.

The only failing job is Lint, Typecheck & Package, and its sole error is the oxlint max-lines limit at packages/platform-apple/src/snapshot-route.test.ts:1003. This PR does not touch that file, it is already 1003 lines on main, and #3890 owns the fix, so the failure looks unrelated. The earlier Coverage failure overlapped this PR, and the author reports unit-ci passing at b74fceb. I did not rerun pnpm gate unit-ci or the eager-closure budget test, so the 39 to 40 module count is the author's report. I also did not check on a device how often the rescue fires on text fields, so the defect above comes from reading the code. I did not verify whether AX bridge acquisition includes remote-hosted inputs such as SafariViewService or ASWebAuthenticationSession.

Cubic has not reviewed this head, and no inline threads are open. There are no conflicts. Before merge, tie the tap's exclusion evidence to the observation that produced the dispatch point, add the rescue route test, and attach the live rescue run, then re-run CI after #3890 lands.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant