Skip to content

Speed up Mobile Checks by overlapping typechecks and tests - #28127

Merged
nwparker merged 4 commits into
mainfrom
nwparker/mobile-checks-speed
Oct 13, 2026
Merged

nwparker merged 4 commits into
mainfrom
nwparker/mobile-checks-speed

Conversation

@nwparker

@nwparker nwparker commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor

ELI5

Mobile Checks waits for both typechecks before starting tests. Run those independent checks together, and remove repeated disk probes and unused device substitutes from recording replays.

What Changed

Both typechecks now run in the background after dependency installation. The job waits for their results after tests, including when a test fails, so compiler failures still fail the job. All existing tests and release checks remain enabled under their existing conditions.

Remove the obsolete featureTips.nativeChatUpgrade.chatModeLabel and chatModeHint keys from five translations. English already retired these unused keys; leaving them in the target catalogs fails the repository localization check.

The exhaustive short-chat snapshot parity cases use an explicit 10-second deadline: a GitHub run reached 5.022s under compiler contention, above the default 5-second deadline. Their enumeration and assertions are retained; the existing longer-chat case already has its own 60-second deadline.

The recording loader resolves each relative import once and builds native substitutes when first needed. Evaluated modules, device state, mutations, and effects still belong to each loader.

Why

The reported job took 3m30s; its typechecks held tests up for 43s. Overlapping independent checks removes that wait without reducing coverage. Loader-local reuse avoids a cross-recording source cache that could hide source edits or leak device state.

Linked Issue

Investigated from Mobile Checks on PR #28062.

Visual Proof

N/A — CI and test tooling only; no visual or interaction changes.

Testing

  • Manually measured the checks locally on macOS: original typechecks followed by the CI test runner took 57.30s; overlapping typechecks and the optimized runner took 39.88s (30.4% faster).
  • All 10,458 mobile tests passed under the existing Node/Bun routing; both production and test typechecks passed.
  • 72 focused workflow and scope tests passed.
  • Added regression tests: 1,000 export reads make zero additional filesystem probes, and unused loaders construct zero substitute tables. Both tests fail against the original loader. Used loaders retain separate clipboard state.
  • Changed-code quality, formatting, and git diff --check passed.
  • Mobile Checks on the final commit passed on ubuntu-24.04-arm: typechecks and tests took 135s versus 163s in the supplied baseline (17.2% faster); the full job took 186s versus 210s (11.4% faster). An earlier passing run took 129s for the checks and 206s overall, including 29s of Ruby setup. Both PR runs also exercised the Ruby release checks that the supplied baseline skipped; setup and cache timing varies.
  • The localization catalog verifier and the broader React Doctor check pass locally.

Review

Reviewed background failure collection and cancellation, dependency installation ordering, loader isolation, fresh-loader source edits, and the shared delivery-ambiguity module. No application execution, persistence, remote protocol, or provider behavior changes.

Agent skill upstream boundary

  • Not applicable: no upstream skill-installer content is copied or translated.

Notes

Cross-platform paths use Node path utilities. Windows and SSH application execution are unchanged. Linux CI will verify the workflow; local measurements are macOS results and are not a prediction of GitHub timing.

Checklist

  • This PR is small and focused.
  • Explained the before/after, mechanism, and choice of approach.
  • Visual proof is N/A with a reason.
  • Self-reviewed correctness, concurrency, isolation, and performance.
  • Cross-platform and remote impact considered.
  • Relevant tests, typechecks, formatting, and quality checks pass locally; CI covers the wider repository checks. Packaging is unchanged.

@coderabbitai

coderabbitai Bot commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 60e8007d-2e6b-4b90-95d3-c68c1c584937
📥 Commits

Reviewing files that changed from the base of the PR and between 52fca074f73486582904aae191c4e0d4c6671a4e and d6041c8.

📒 Files selected for processing (7)
  • mobile/src/session/mobile-native-chat-merged-snapshot-parity.test.ts
  • mobile/src/test-support/rpc-recording/operation-module-loader-work-budget.test.ts
  • src/renderer/src/i18n/locales/es.json
  • src/renderer/src/i18n/locales/fr.json
  • src/renderer/src/i18n/locales/ja.json
  • src/renderer/src/i18n/locales/ko.json
  • src/renderer/src/i18n/locales/zh.json
💤 Files with no reviewable changes (5)
  • src/renderer/src/i18n/locales/ko.json
  • src/renderer/src/i18n/locales/es.json
  • src/renderer/src/i18n/locales/ja.json
  • src/renderer/src/i18n/locales/fr.json
  • src/renderer/src/i18n/locales/zh.json

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 2 remain after this review.

📝 Walkthrough

Walkthrough

The mobile workflow starts production and test typechecks in the background and waits for both after the test step. The operation module loader initializes native substitutes on first import and resolves relative paths once. New tests check workflow scheduling, loader file probes, lazy initialization, and clipboard isolation. The frame-parity test adds a timeout to each case. Five locale files remove chat-mode label and hint entries.

Priority: ⬇️ Low

Merge Risk: ⚪ Minimal · up to d6041

The workflow waits for both typechecks even when tests fail, and the loader, parity-test, and locale changes show no established regression. No actionable merge-blocking risk remains in the reviewed changes.

Pre-merge checks · Passed 4 · Failed 1

Failed Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 4 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks (4 passed)
Check name Status Explanation
Title check Passed The title clearly and concisely describes the primary change: overlapping mobile typechecks and tests to reduce check duration.
Description check Passed The description covers the required sections, explains the before-and-after behavior and rationale, documents testing results, provides visual-proof justification, and addresses compatibility and scop…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@nwparker
nwparker force-pushed the nwparker/mobile-checks-speed branch from 52fca07 to a586834 Compare October 13, 2026 00:46
@nwparker
nwparker merged commit ebfae6a into main Oct 13, 2026
32 checks passed
karlorz added a commit to karlorz/orca that referenced this pull request Oct 13, 2026
* Update README downloads badge

* fix(i18n): drop chat-mode keys removed from en.json in stablyai#28113 (stablyai#28131)

* Speed up Mobile Checks by overlapping typechecks and tests (stablyai#28127)

Overlap mobile typechecks with tests and avoid repeated filesystem probes and unused device substitutes in recording replays. Keep state isolated between loaders, give exhaustive snapshot cases an explicit execution deadline, and retire obsolete locale keys that blocked repository validation.

* fix(orcad): identify instances by kernel start ticks, not drifting wall-clock start time (stablyai#28125)

* fix(orcad): identify instances by kernel start ticks, not drifting wall-clock start time

On Linux the start time is derived from /proc/stat btime, which drifts (WSL2, NTP),
so a live orcad was read as a reused PID and its managed stop reported exited
without ever writing the request. The lock and stop request now carry optional
linuxStartTicks + bootId; readers prefer them, and a wall-clock mismatch on Linux
for records without them is unverifiable, never proof of exit.

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

* fix(orcad): match stop receipts without the incarnation an older client drops

A request recovered by an older client loses linuxStartTicks/bootId, which made
the newer host reject its own receipts and decision records as mismatched. Also
exercise the incarnation reader against a real Linux /proc.

---------

Co-authored-by: Claude <noreply@anthropic.com>

* docs(readme): point the Android APK links at 0.0.55 (stablyai#28134)

Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb

* perf(runtime): stop re-sending the worktree list to mobile pollers on every terminal chunk (stablyai#28062)

* perf(runtime): let worktree.ps pollers opt out of per-chunk activity churn

worktree.ps carries `preview` and an exact `lastOutputAt` on every row, both
of which move on each PTY chunk, so the content-addressed snapshot id changed
on every poll while any agent printed and the whole catalog was resent.

Add an optional `coarseActivity: true` param. When set, the host drops
`preview` and floors `lastOutputAt` to 30 s before hashing, so the id holds
while only chunk-level activity moves. Callers without the param get the
byte-identical response they get today; older hosts strip the unknown key.

Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb

* perf(mobile): stop re-sending the worktree list on every terminal chunk

The host-screen snapshot poll now asks worktree.ps for `coarseActivity`, so
the catalog's snapshot id survives terminal output and the 3 s poll gets a
tiny `unchanged` reply instead of the whole catalog. `preview` is optional on
mobile rows because a coarse host leaves it out; old hosts ignore the param
and keep sending it.

Re-records the three worktree.catalog-snapshot goldens; the only semantic
change is the new request param.

Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb

* perf(runtime): coarsen worktree.ps activity for every snapshot poller

Drop the `coarseActivity` param: the only caller that sends
`afterSnapshotId` is the mobile snapshot poll, which never renders
`preview` and uses `lastOutputAt` only to rank rows. Snapshot polls now
always get a blank `preview` and a 30 s-floored `lastOutputAt`, so
already-shipped apps stop re-downloading the catalog on every terminal
chunk with only a desktop update. Callers without `afterSnapshotId`
still get the byte-identical response.

Reverts the mobile param, its scenario steps and the three re-recorded
goldens, and removes the unread `preview` field from mobile row types.

Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb

* fix(runtime): rank coarsened worktree.ps rows so in-bucket swaps keep the snapshot id

Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb

* test: drop preview from row fixtures and keep the runtime cast on one line

Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb

* fix(mobile): load markdown tabs from any desktop split group; center load-error Retry (stablyai#28132)

* fix(mobile): load markdown tabs from any desktop split group and center load-error Retry

The desktop answered markdown.readTab only for tabs in its focused split
group, so a phone opening a markdown tab from another group got
tab_not_found and showed "Couldn't load markdown". Resolve the tab across
all of the worktree's groups.

Load-error states now share one component that centers the message and its
Retry button; the markdown reader's button was pinned to the left.

Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb

* test(renderer): mobile markdown read picks the requested tab across groups

Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb

* refactor(renderer): resolve mobile markdown tab to one file id

Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb

* fix(wsl): run WSL git with Windows Git line-ending settings on checkouts Windows Git wrote (stablyai#27778)

* fix(wsl): run WSL git with Windows Git settings on Windows-authored drive checkouts

* fix(wsl): detect Windows Git checkouts from host autocrlf plus a whole-tree sample

* fix(wsl): keep WSL-created worktrees on Linux Git defaults and never commit CRLF on an unproven checkout

- worktree add / clone write a new checkout whose .git points at /mnt/..., which Windows Git
  cannot open; injecting autocrlf=true there produced CRLF files later read unflagged.
- Writes on a host-autocrlf=true checkout whose sample is unproven run with autocrlf=input,
  so staging normalizes CRLF without ever writing CRLF to disk.
- Writes wait for a stale host answer and re-check checkout identity after joining a sample.

* fix(wsl): compare a joined sample with the checkout identity read after it settles

* fix(runtime): stop a project's terminals before the host forgets the project (stablyai#28024)

* fix(runtime): stop a project's terminals before the host forgets the project

Removing a project on a runtime host (an SSH-managed or paired server) sent repo.rm first; the
host resolves terminal.stop through the project's workspace rows, which repo.rm deletes, so the
stop failed and the shells kept running on the host. Terminals stop first now. The removal toast
also drops the IPC wrapper from its message.

* fix(runtime): only stop host terminals when the host will accept the project removal

Probe with repo.show (same resolver as repo.rm) so a removal the host rejects, such as an
ambiguous project id, no longer kills the shells first. Also poll through terminal_not_writable
in the paired-client e2e.

* test(e2e): read the host shell pid through sh so non-POSIX login shells work

* fix: preserve relay pane claims across managed tab mirrors

* fix(mobile): cancel bridge-post retries when a terminal document stops

Mobile Checks verify failed on an unhandled ReferenceError: window is not
defined inside postToReactNativeWebView's scheduled send() retry while
viewport-frame.test.ts ran. Retries lived outside the document lifecycle, so
a stop left timers that later read a torn-down window.

Bind the default postToHost per scope, clear its timers on stop, no-op posts
after stop, and guard window before reading ReactNativeWebView. Preserve
bounded retry and deliver-when-bridge-appears behavior.

* fix(mobile): use supported own-property check for host overrides

* fix(mobile): align bridge retry teardown with document lifecycle contract

* style(mobile): format bridge retry lifecycle regression test

---------

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Neil <4138956+nwparker@users.noreply.github.com>
Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: Jinwoo Hong <73622457+Jinwoo-H@users.noreply.github.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.

1 participant