Skip to content

perf(ios): fuse scroll frame resolution and drag into one runner command#760

Merged
thymikee merged 3 commits into
mainfrom
perf/668-fused-scroll-command
Jun 11, 2026
Merged

perf(ios): fuse scroll frame resolution and drag into one runner command#760
thymikee merged 3 commits into
mainfrom
perf/668-fused-scroll-command

Conversation

@thymikee

Copy link
Copy Markdown
Member

Closes #668

What

Non-tvOS iOS scroll now executes as one runner lifecycle command instead of a read-only interactionFrame request followed by a mutating drag request.

  • New mutating runner command scroll carries the pre-frame inputs (direction, amount?, pixels?).
  • The Swift runner resolves the interaction frame via the existing resolvedTouchReferenceFrame, computes drag endpoints with a line-for-line Swift port of buildScrollGesturePlan (parity test vectors mirrored in both languages), and executes the same non-synthesized drag path scroll used before.
  • The response is the existing drag-shaped gesture payload, so the daemon recomputes planned pixels and feeds normalizeIosScrollResult unchanged — result shape and gesture recording metadata are byte-compatible.
  • tvOS remote-scroll branch is untouched; interactionFrame stays runner-supported for wire compat with older daemons (annotated as no longer sent).

Lifecycle safety

scroll is intentionally omitted from isReadOnlyRunnerCommand, so it gets command-id tracking, single-send (no transport retry), readiness preflight, and journal-based lost-response recovery. The journal retains its small response JSON (~200 bytes), so a lost response is recovered without replaying the gesture.

Validation (iPhone 17 Pro Max simulator, iOS 26.2)

Per-request diagnostics (--debug) for a 6-scroll hot loop in Settings:

runner sends preflights total runner requests per scroll
main 2 (interactionFrame + drag) 2 4
this PR 1 (scroll) 1 2
  • UI scrolls correctly (snapshot hash changes, gesture lands at the same coordinates the old math produced: 956pt frame → travel 574).
  • Result shape unchanged: {direction, x1, y1, x2, y2, referenceWidth, referenceHeight, pixels, message}.
  • swipe / gesture pan smoke-tested after the Swift drag-body extraction.
  • pnpm typecheck, focused unit suites (285 tests), and pnpm build:xcuitest (iOS + macOS) all pass.

@github-actions

github-actions Bot commented Jun 11, 2026

Copy link
Copy Markdown

Size Report

Metric Base Current Diff
JS raw 1.2 MB 1.2 MB -102 B
JS gzip 390.9 kB 390.8 kB -52 B
npm tarball 502.2 kB 504.6 kB +2.4 kB
npm unpacked 1.7 MB 1.7 MB +10.1 kB

Startup median (7 runs, lower is better):

Scenario Base Current Diff
CLI --version 25.6 ms 25.5 ms -0.1 ms
CLI --help 41.3 ms 40.6 ms -0.7 ms

Top changed chunks:

Chunk Raw diff Gzip diff
dist/src/1620.js +103 B +21 B
dist/src/2415.js +9 B +7 B

@thymikee

Copy link
Copy Markdown
Member Author

Note for whichever of this and #763 merges second: #763 adds a readiness-preflight skip allowlist in runner-session.ts keyed on runner command names. The fused scroll command introduced here should be added to PREFLIGHT_SKIP_ELIGIBLE_RUNNER_COMMANDS once both land — otherwise hot scroll loops lose the preflight skip they currently get via drag (there's a code comment in #763 marking this).

Copy link
Copy Markdown
Member Author

Code review

Verdict: looks good — minor issues only. The Swift port of buildScrollGesturePlan is line-for-line equivalent (axis selection, max(1, round(pixels)), default amount 0.6, edge padding, travel clamp, half-travel rounding, all four direction mappings, NaN/Infinity rejection). Lifecycle classification is consistent in all three places (TS isReadOnlyRunnerCommand omission, Swift CommandTraits readOnly .never, journal shouldRetainResponseJson includes .scroll), so lost-response recovery replays the retained payload instead of re-scrolling. tvOS branch is untouched, and the dropped durationMs is genuinely behavior-neutral.

Findings

  1. Minorsrc/core/__tests__/scroll-gesture.test.ts:12 vs RunnerTests+ScrollGesture.swift:88: both headers claim the parity vector sets mirror each other, but the Swift pixels=120 @ 300x600 plan vector has no buildScrollGesturePlan counterpart in the TS suite (it appears only indirectly via the interactor test), so the "mirror" invariant is asymmetric on day one. The vectors also don't exercise tiny frames (where the max(1, …) floors engage and 0.5 rounding matters) or amount > 1 — the math is provably identical in those regimes (all rounded quantities are positive, so JS half-up and Swift half-away-from-zero agree), but the vectors don't pin it.

  2. Minorsrc/platforms/ios/interactions.ts:349-368: the daemon recomputes plan.pixels from the referenceWidth/Height in the drag-shaped response, but those dims come from resolvedDragVisualizationFrame, which re-resolves the touch reference frame after keyboardAvoidingDragPoints may have shifted the endpoints. If keyboard visibility flips between the two resolutions, reported pixels can disagree with actual travel. This exposure existed in the old two-request flow (and the window is now milliseconds), so informational — but the response could carry the planning frame dims explicitly to close it.

  3. Minorsrc/platforms/ios/runner-contract.ts:31 / RunnerTests+Models.swift:14: a still-running runner built from pre-PR sources can't decode the new scroll enum case, so its transport returns a 400 and every non-tvOS scroll fails until rebuild; there's no daemon-side fallback to the old interactionFrame+drag sequence. Mitigated because computeRunnerSourceFingerprint changes with the Swift sources and triggers a rebuild on the next session — this only bites a runner left alive across a package upgrade.

Overall

A careful, well-tested fusion. The only substantive pre-merge ask is tightening the parity vector mirror (finding 1), since the whole two-language invariant rests on it.


Generated by Claude Code

thymikee and others added 2 commits June 11, 2026 09:48
Non-tvOS scroll now sends a single mutating 'scroll' runner command. The
Swift runner resolves the interaction frame and executes the same
non-synthesized drag path, eliminating the separate read-only
interactionFrame request per scroll. The command is lifecycle-journaled
with retained response JSON so lost-response recovery returns the result
without replaying the gesture.

Closes #668
#763 landed the healthy-mutation preflight skip with a note that the
fused scroll command should join the allowlist once it exists. Add
'scroll' to PREFLIGHT_SKIP_ELIGIBLE_RUNNER_COMMANDS, drop the
now-resolved code note, extend the per-family skip tests, and update
the allowlist enumeration in ADR 0005 and the protocol-optimizations
doc.

https://claude.ai/code/session_01VokBZWESTDgcnbYwS4DkJo
@thymikee thymikee force-pushed the perf/668-fused-scroll-command branch from 6205b7e to 9f46b57 Compare June 11, 2026 09:51
Address review on the cross-language parity vectors:
- mirror the Swift pixels-plan vector (down, 120px @ 300x600) in the
  vitest suite so every vector exists in both languages
- add amount > 1 clamp and tiny-frame (2x2) vectors to both suites;
  the tiny frame engages every max(1, ...) floor and the .5 rounding
  cases where JS half-up and Swift half-away-from-zero must agree

https://claude.ai/code/session_01VokBZWESTDgcnbYwS4DkJo

Copy link
Copy Markdown
Member Author

Follow-up on the review above, as of 01a0279:

  • Finding 1 (parity vector asymmetry): addressed. The Swift pixels-plan vector (down, 120px @ 300x600) now has a buildScrollGesturePlan mirror in the vitest suite, and both suites gained two new mirrored vectors: amount > 1 (400x800, amount 2 → travel clamps to 720) and a tiny 2x2 frame that engages every max(1, …) floor plus the .5 rounding cases (halfTravel 0.5 → 1, center 1) where JS half-up and Swift half-away-from-zero must agree. TS suite passes (12 tests); the Swift vectors use the identical values the TS implementation just verified.
  • Finding 2 (recomputed pixels vs keyboard-shifted endpoints): not changed. Pre-existing exposure carried over from the two-request flow, now with a milliseconds-wide window — left as informational. Carrying the planning-frame dims in the response remains a possible future hardening.
  • Finding 3 (stale pre-PR runner can't decode scroll): not changed. Mitigated by the runner source fingerprint forcing a rebuild on the next session; only bites a runner left alive across a package upgrade.

Also note: the branch was rebased onto main and scroll was added to PREFLIGHT_SKIP_ELIGIBLE_RUNNER_COMMANDS (tests + docs updated) per the merge-order note on #763, so a hot scroll loop now benefits from both the fusion and the preflight skip — worth one on-device --debug pass to confirm the combined behavior before merge.


Generated by Claude Code

@thymikee thymikee merged commit 3780ca6 into main Jun 11, 2026
19 checks passed
@thymikee thymikee deleted the perf/668-fused-scroll-command branch June 11, 2026 10:03
@github-actions

Copy link
Copy Markdown
PR Preview Action v1.8.1
Preview removed because the pull request was closed.
2026-06-11 10:04 UTC

thymikee pushed a commit that referenced this pull request Jun 11, 2026
Rebased onto main with #763 (healthy-mutation preflight skip) and #760
(fused scroll). Per the merge-order note, add 'sequence' to
PREFLIGHT_SKIP_ELIGIBLE_RUNNER_COMMANDS so a successful sequence earns
the next hot-command skip instead of always taking the
conservative_command path. Extend the per-family skip tests and the
allowlist enumeration in ADR 0005 and the protocol-optimizations doc.

https://claude.ai/code/session_01VokBZWESTDgcnbYwS4DkJo
thymikee added a commit that referenced this pull request Jun 11, 2026
…eries (#764)

* perf(ios): add lifecycle-safe runner sequence command for hot press series

Adds a narrow 'sequence' runner command that batches an explicit
allowlist of coordinate steps (tap, longPress, drag) into one
lifecycle-tracked request with stop-on-first-failure and small bounded
per-step results. iOS press series with hold/jitter now issue one
sequence request per ~20-step chunk (also budgeted to stay under the
runner's 30s main-thread watchdog) instead of one request per press.
Sequence responses are journaled and retained, so lost-response recovery
returns observed results without replaying the gesture sequence.

Closes #669

* fix: perform every press in direct press series

runDirectPressSeries guarded the awaited interaction itself with ??=,
so presses 2..N were silently skipped once the first result was kept
(affects Android series and doubleTap series; introduced in #512).
The kept-first-result shape is preserved.

* chore: unexport internal sequence chunk budget constant

* perf(ios): make sequence eligible for readiness preflight skip

Rebased onto main with #763 (healthy-mutation preflight skip) and #760
(fused scroll). Per the merge-order note, add 'sequence' to
PREFLIGHT_SKIP_ELIGIBLE_RUNNER_COMMANDS so a successful sequence earns
the next hot-command skip instead of always taking the
conservative_command path. Extend the per-family skip tests and the
allowlist enumeration in ADR 0005 and the protocol-optimizations doc.

https://claude.ai/code/session_01VokBZWESTDgcnbYwS4DkJo

---------

Co-authored-by: Claude <noreply@anthropic.com>
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.

perf(ios): fuse scroll frame resolution and drag into one runner command

2 participants