Skip to content

fix(scroll): keep saturated scroll gestures out of the status bar; gate Android replays from android/emulator (#1781 A1) - #1820

Merged
thymikee merged 2 commits into
mainfrom
claude/unruffled-murdock-bc79aa
Aug 18, 2026
Merged

fix(scroll): keep saturated scroll gestures out of the status bar; gate Android replays from android/emulator (#1781 A1)#1820
thymikee merged 2 commits into
mainfrom
claude/unruffled-murdock-bc79aa

Conversation

@thymikee

@thymikee thymikee commented Aug 18, 2026

Copy link
Copy Markdown
Member

Refs #1781 (A1). Follow-up to #1793 / #1794: makes pnpm gate replay-android pass after the full-tier Android E2E.

Symptom

Replay Nightly run 32107665052 (job 95620294899): the E2E passes 9/9, then the gate reports 4 passed (8), 4 failed05-app-lifecycle, 06-swipe-gestures, fixture/02, fixture/01 all diverge with "A system surface (notification shade…) covers the app".

Root cause (reproduced live; not an E2E leftover)

03-scroll-discovery.ad runs scroll up 3. The scroll planner clamps travel to the viewport minus a 5 % edge band, so on 1080×2400 the swipe touches down at y = 120. CI's avdmanager --device pixel_7 AVD carries the Pixel 7 cutout, whose status bar is 136 px (a plain 1080×2400 skin gives 63 px — which is why a naive local repro doesn't show it). On API 36 the app window is edge-to-edge, so the reported viewport starts at y = 0: the touch-down landed inside the status bar and pulled the notification shade instead of scrolling. The shade then covered every replay until 04's back closed it. Native readdir order on the runner (03, 05, 06, fixture/02, fixture/01, 04, 01, 02) put four files in that window; the last green run (07-30) had 04 right after 03, masking it.

Evidence: CI's 03 screenshots (Settings before/after scroll down 3, shade afterwards); a control run at main head on a cutout AVD in CI order reproduces exactly CI's 4/8 with the same steps.

Fix — in the product, not the lane

Every real Pixel has a cutout (5.7 % of a Pixel 7's height) and an iPhone's Dynamic Island bar is 6.9 %, so any saturated scroll up opened the shade / Notification Center for real agents too. A "dismiss system surfaces" recovery in the replay lane would have hidden that.

  • packages/contracts/src/scroll-gesture.ts: DEFAULT_EDGE_PADDING_FRACTION 0.05 → 0.1, mirrored in the Swift port (RunnerTests+ScrollGesture.swift). Parity vectors updated in both suites plus a Pixel 7 regression vector (1080×2400, amount 3 → touch-down y=240 > 136). Cost: max single-gesture travel 90 % → 80 % of the axis; amounts ≤ 0.8 (default 0.6, scroll top/bottom) are unchanged.

Second contamination the same order exposed

Once the shade was gone, fixture/02-selector-routes-covered-diagnosis.ad — a #1715 reproduction recipe that fails by design at step 9 and leaves the device in landscape — broke the files after it, because the gate enumerated test/integration/replays/android recursively. iOS keeps gate replays in ios/simulator and recipes in ios/fixture; Android now mirrors that: the six Settings replays move to test/integration/replays/android/emulator/, test:replay:android points there, and fixture/ stays E2E-owned (full:fixture-replays already runs fixture/01 by path — this also removes that duplication). android.yml and test/ci/android-workflow-evidence.json follow the path; the replay-compat manifest keeps the historical paths it pins at released tags. #1793's files are untouched.

Verification

  • Live, Pixel 7 geometry / API 36, --retries 0: pnpm gate replay-android 6/6 in both native and CI order; 03 leaves Settings on screen (scroll up 3 now touches down at y=240).
  • contracts vectors + 24 scroll-related test files green; typecheck, lint, format clean; unit suite 6830/6831 — the one failure (hover frameworkTier parity) is pre-existing on main.

Notes for reviewers

  • The Swift constant lands the same way on iOS; ios/simulator/03-scroll-discovery.ad also does scroll up 3 (possibly relevant to the parked iOS lane — not verified here).
  • The scroll parity XCTests are not in the PR -only-testing list; the nightly runs them.
  • Local-repro gotchas (documented in the commit): reproduce the cutout with adb shell cmd overlay enable-exclusive --category com.android.internal.display.cutout.emulation.hole; without the IME helper packaged, Gboard stays up and 04's back only hides the keyboard, leaving the Settings search activity on top of open settings --relaunch — not a CI condition.

Review follow-ups (thymikee)

  • Follow-up issue filed: Gesture viewport should expose content-safe bounds (app window minus system bars) so scroll planning clamps to real geometry #1821 — gesture viewport should expose content-safe bounds (app window minus system bars / safe-area insets) so the planner clamps to real geometry and the 10 % fraction returns to a small generic margin. The comment on the constant frames it as must exceed the tallest known bar (Pixel 7 cutout 5.7 %, Dynamic Island 6.9 %). Suite discovery order (native readdir, which is verified Maestro parity — WorkspaceExecutionPlanner uses Files.walkunsortedFlowFiles) is on the same issue rather than changed here.
  • Perf/scroll benchmark impact: none — scripts/perf uses the default-amount scroll down (0.6) and scroll top/bottom loops the default too; only amount ≥ 0.8 / large explicit pixels lose travel (90 % → 80 % max).
  • Parity vectors → one table: contracts/fixtures/scroll-gesture.json (vectors + both constants), iterated by scroll-gesture.test.ts and RunnerTests+ScrollGesture.swift (same #filePath pattern as RunnerTapPointPolicy.swift). Verified live: vitest 10/10, and the four XCTests executed on an iOS 26.2 simulator with the unit flag on (4/4).
  • test/ci/android-workflow-evidence.json now carries a purpose field saying what it guards.

…te Android replays from android/emulator (#1781 A1)

`pnpm gate replay-android` failed 4/8 whenever it ran after the full-tier Android E2E
(replays-nightly run 32107665052, job 95620294899): 05-app-lifecycle, 06-swipe-gestures and
both fixture replays diverged under "A system surface covers the app". The E2E was not the
cause. Reproduced on a pixel_7 / API 36 AVD with the same cutout geometry CI's
`avdmanager --device pixel_7` produces (status bar 136px, not the 63px of a plain 1080x2400
skin):

- `03-scroll-discovery.ad` runs `scroll up 3`. The scroll planner clamps travel to the viewport
  minus a 5% band, so the touch-down landed at y=120 — inside the 136px status bar — and
  pulled the notification shade instead of scrolling. On API 36 the app window is
  edge-to-edge, so the reported viewport starts at y=0 and includes that bar.
- The shade then covered every replay until `04`'s `back` closed it. Native readdir order on
  the runner (03, 05, 06, fixture/02, fixture/01, 04, 01, 02) put four files in that window;
  the last green run (2026-07-30) had 04 right after 03, so the pull was masked.

Fix in the product, not the lane: DEFAULT_EDGE_PADDING_FRACTION 0.05 -> 0.1 in the TS scroll
planner and its Swift port. Every real Pixel has a cutout (5.7% of a Pixel 7's height) and an
iPhone's Dynamic Island status bar is 6.9%, so any saturated `scroll up` opened the shade /
Notification Center for real agents too. Parity vectors updated in both suites plus a Pixel 7
regression vector (1080x2400, amount 3 -> touch-down y=240 > 136).

Second contamination the same order exposed once the shade was gone:
`fixture/02-selector-routes-covered-diagnosis.ad` is a #1715 reproduction recipe that FAILS
BY DESIGN at step 9 (covered-target refusal) and leaves the device in landscape, yet the
gate enumerated `test/integration/replays/android` recursively. iOS keeps gate replays in
`replays/ios/simulator` and fixture recipes in `replays/ios/fixture`; Android now mirrors that:
the six Settings replays move to `replays/android/emulator`, `test:replay:android` points there,
and `fixture/` stays E2E-owned (`full:fixture-replays` already runs 01 by path). android.yml
and the workflow-evidence fixture follow the path; the replay-compat manifest keeps the
historical paths it pins at released tags.

Verified live (Pixel 7 geometry, API 36, --retries 0): control run at main head in CI order
reproduces exactly CI's 4/8; with the fix, `pnpm gate replay-android` 6/6 in both native and
CI order, and `03` leaves Settings on screen (scroll up 3 now touches down at y=240).
@github-actions

github-actions Bot commented Aug 18, 2026

Copy link
Copy Markdown
PR Preview Action v1.8.1
Preview removed because the pull request was closed.
2026-08-18 12:33 UTC

@github-actions

github-actions Bot commented Aug 18, 2026

Copy link
Copy Markdown

Size Report

Metric Base Current Diff
JS raw 2.27 MB 2.27 MB -26 B
JS gzip 747.7 kB 747.7 kB -3 B
npm tarball 869.5 kB 869.5 kB +75 B
npm unpacked 3.03 MB 3.03 MB +206 B

Startup median (7 runs, lower is better):

Scenario Base Current Diff
CLI --version 29.8 ms 29.1 ms -0.6 ms
CLI --help 70.3 ms 69.5 ms -0.8 ms

Top changed chunks:

Chunk Raw diff Gzip diff
dist/src/sdk-batch-runner.js -25 B -1 B

@thymikee

Copy link
Copy Markdown
Member Author

Code-quality review (thermo-nuclear pass). Verdict: approve with two asks — one in-PR, one as a named follow-up. The diff is small (65 lines), the diagnosis is real and reproduced, and it makes the codebase more canonical (Android replays now mirror ios/{simulator,fixture}). No file-size, spaghetti, or layering regressions. What follows is about the shape of the fix, not its correctness.

1. The constant is a proxy for a fact the platform layer already knows — say so, and name the follow-up

DEFAULT_EDGE_PADDING_FRACTION 0.05 → 0.1 encodes "the status bar is ≤10% of the height" inside a device-agnostic pure planner (packages/contracts). The actual invariant is "a scroll must not touch down inside a system bar", and the thing that knows where the bars are is the viewport reader: GestureViewportReader.read() already iterates getWindows() and only keeps TYPE_APPLICATION — the status/navigation bar windows are right there to subtract; on iOS the runner has safe-area insets. Today the viewport is the raw app window (edge-to-edge → includes the bar), so the planner compensates with a fraction that must be tuned to the tallest known bar. That works, but it's a hidden data-shape assumption in the wrong layer, and it costs every user 10 points of max scroll travel (90%→80% for amount ≥ 1) whether or not their device has a cutout.

I would not block on this — the inset-aware version needs a viewport contract change across the helper, the runner, and the planner, and this PR unblocks a nightly. But please (a) keep the comment framing the 10% as "must exceed the tallest known bar (Pixel 7 cutout 5.7%, Dynamic Island 6.9%)" — it already does, good — and (b) file the follow-up as an issue: gesture viewport should expose content-safe bounds (app window minus system bar windows / safe-area insets) so the planner clamps to real geometry and the fraction returns to a small generic margin. One sentence in the PR body linking it is enough. Also worth one data point before merge: does 80% max travel change any perf/scroll benchmark expectation (scripts/perf, #1366-era scroll landing)? Default 0.6 and top/bottom are unaffected, so likely no — just say so.

2. The TS↔Swift parity vectors are hand-mirrored; the repo already has the canonical pattern for this

This PR had to update the same three vectors in packages/contracts/src/scroll-gesture.test.ts and RunnerTests+ScrollGesture.swift by hand, and add a fourth in both. RunnerTapPointPolicy.swift shows how the repo does cross-language parity: one table in contracts/fixtures/tap-point-policy.json, read by both suites (the Swift side via #filePath). Please move the scroll-gesture vectors (and ideally the two constants, 0.6 / 0.1) into contracts/fixtures/scroll-gesture.json and have both tests iterate it — that deletes the drift class this PR just exercised, and it's ~40 lines. If you'd rather keep this PR to the fix, name it as a follow-up; but it's small enough to do here.

3. Root-cause of the masking: gate enumeration order is native readdir order

The body notes the last green run had 04 right after 03, masking the shade for weeks, and that the runner's readdir order differs from a laptop's. That's the second time this repo has been bitten by non-deterministic suite input order (see docs/agents/testing.md / the earlier readdir note). Moving the by-design-failing recipe out of the gated dir is the right structural fix for that file, but the general fix is one line: sort directory suite inputs in the replay test runner's discovery. Reproducible order across machines is what turns "green on 07-30, red on the runner" into "same result everywhere". Not required for this PR — but it's the change that makes this class of bug unrepresentable, and it belongs on the same issue as A1's residue.

Minor

  • test/ci/android-workflow-evidence.json path update: fine, but a reader won't know why a JSON file pins a workflow path — one comment line in that file (or its test) saying what it guards would help.
  • The Swift comment says "Mirrors DEFAULT_EDGE_PADDING_FRACTION" — once (2) lands, that comment becomes the fixture path instead.

Nothing here changes behaviour further; approve once (1)'s follow-up issue exists and (2) is either done or named.

…one table (#1820 review)

The two suites hand-mirrored the same vectors and #1820 had to edit both by hand — the drift
class the repo already closes for the tap-point rule via contracts/fixtures/tap-point-policy.json.
The scroll vectors (plus both planner constants, pinned behaviourally on a 1000px axis) now live in
contracts/fixtures/scroll-gesture.json; scroll-gesture.test.ts and RunnerTests+ScrollGesture.swift
iterate it. Verified: vitest 10/10; the four XCTests run on an iOS 26.2 simulator with the unit
flag on (Executed 4 tests, 0 failures).

Also: test/ci/android-workflow-evidence.json says what it guards.

Follow-up for content-safe viewport bounds + discovery order: #1821.
@thymikee

Copy link
Copy Markdown
Member Author

Addressed in 9f4670b:

  1. Follow-up filed as Gesture viewport should expose content-safe bounds (app window minus system bars) so scroll planning clamps to real geometry #1821 (content-safe viewport bounds; planner clamps to real geometry, fraction back to a small generic margin). Constant comment already frames 10 % as "must exceed the tallest known bar". Perf/scroll benchmark impact: none — scripts/perf and scroll top/bottom use the default 0.6; only amount ≥ 0.8 loses travel (90 % → 80 % max). Stated in the PR body.
  2. Parity vectors + both constants moved to contracts/fixtures/scroll-gesture.json, iterated by both suites (#filePath reader, same shape as RunnerTapPointPolicy.swift); the Swift comment now points at the table. Verified: vitest 10/10, and the four XCTests executed on an iOS 26.2 sim with the unit flag on (4/4, count checked).
  3. Discovery order: left as is here and put on Gesture viewport should expose content-safe bounds (app window minus system bars) so scroll planning clamps to real geometry #1821 — the "native DFS = Maestro parity" header is accurate (Maestro's WorkspaceExecutionPlanner: Files.walkunsortedFlowFiles, no sort), so sorting is a deliberate parity departure worth deciding on its own.
  4. test/ci/android-workflow-evidence.json carries a purpose field saying what it guards.

🤖 Addressed by Claude Code

@thymikee

Copy link
Copy Markdown
Member Author

Reviewed exact head 9f4670b: clean and merge-ready. The 10% scroll edge band is implemented at the shared TS/Swift planning seams, governed by one parity fixture, and directly validated against the Pixel 7 cutout failure. The replay directory split keeps normal Settings replays in the CI lane while leaving fixture recipes E2E-owned. #1821 appropriately tracks the deeper content-safe-bounds follow-up. All exact-head checks and smoke lanes are green.

@thymikee thymikee added the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Aug 18, 2026
@thymikee
thymikee merged commit f843dc2 into main Aug 18, 2026
47 of 48 checks passed
@thymikee
thymikee deleted the claude/unruffled-murdock-bc79aa branch August 18, 2026 12:31
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