Skip to content

fix(scroll): answer with the movement a directional scroll observed - #2771

Merged
thymikee merged 1 commit into
fix/scroll-movement-observationfrom
fix/scroll-observed-movement
Sep 23, 2026
Merged

thymikee merged 1 commit into
fix/scroll-movement-observationfrom
fix/scroll-observed-movement

Conversation

@thymikee

@thymikee thymikee commented Sep 23, 2026 •

Copy link
Copy Markdown
Member

Part of stack #2777. Layers below: #2770 (behavior-neutral readers) and #2776 (the observation module). This layer wires scroll <direction> to it — the only commit here that changes an answer on a device.

The bug (#2714)

scroll down 0.75 reported the travel its gesture plan had computed. Nothing between the gesture and the response looked at the screen, so the distance was a restatement of the request — on every retry, until a later assertion failed for a reason no log explained.

What this layer changes

resolveBoundScrollRuntime's direction arm asks planScrollMovement({device, flags, session}) before the dispatcher runs, then executeDirectionScroll dispatches once, asks #2776 what movement that gesture may claim, and answers:

evidence answer
surface differs movement: 'moved', one capture
unchanged, container still hides content that way, swipe ran inside it scroll_no_progress
unchanged, nothing left to reveal movement: 'at-edge', no distance
unchanged, horizontal movement: 'unchanged'
not comparable movement: 'unobserved'

captureSnapshot is declared conditional on the direction use — not preferred (ADR 0019 §2 reserves that for optimizations, and this observation is what makes the answer honest) and not required (a runtime that cannot read a screen still scrolls, and answers the response it answered before). One wire test per side of that parity.

Callers that already own the read are not charged twice: --settle, and replay's postGestureStabilization: false, decline with a typed reason, as does a non-swipe device.

Review round 2 — what your two findings became

The lineage gate ran only on the quiet path. Fixed, and it was a real defect: pollForSurfaceVerdict classified first, so a baseline and a post-gesture read from different capture lineages could answer moved on a pair this module itself calls incomparable. The gate now runs on every capture, before the changed/unchanged classifier — including the moved path. The deferred loop rebases on a producer change because its question is whether the surface settles; a pre-gesture comparison has no such option, so the claim is withheld. Test: a comparisonKey-drifted baseline whose frames genuinely differ → unobserved in one capture; its pair is a snapshot -i baseline, same expectation. Red/green: deleting the gate turns exactly those two tests red.

The GenericExecutionDispatch seam. Gone, and you were right that it was the wrong seam: resolveGenericRuntimeExecution already had session and req, so the plan is computed there (session + flags, two lines) and handed to the closure as data. runtime-admission.ts and the five unrelated leaf files (back, focus, orientation, system-button, tv-remote) are untouched by this stack now — 30 lines and one shared-type widening deleted. It is also more correct: the flags and device it reads are pre-effect facts, so reading them pre-seam removes the pretense that the execution closure is where they belong.

Two more things fell out of the same pass: the capture! non-null assertion is gone (the capture is bound before it is used), and executeDirectionScroll now dispatches once and returns once instead of three near-copies of scrollOnce(…) → scrollResult(…). The four hand-rolled capture inputs in this file (movement read, edge rest-poll, edge state, --until) collapsed into one scrollCaptureInput(context, scope?), which also makes explicit a decision that was previously only an idiom repeated three times: a scroll read carries the session's app and the request's execution metadata, and deliberately none of the caller's snapshot flags, so snapshot -i on the same request cannot change what a stop condition reads.

Validation

  • Full pnpm check:affected --run → all runnable checks passed on the stack head 882256121 (lint, format, typecheck across all packages + examples, layering, fallow, the vitest and node-test stages). At this layer's head (35d4071af): the scroll wire suites (136 files / 696 tests related to scroll-runtime.ts) and tsc clean, and pnpm check:fallow --base fix/scroll-movement-observation reports no issues.
  • 21 wire tests: refusal and moved keep their paired negatives; moved asserts exactly one capture and every decline asserts zero; the harness runs the real ADR 0014 transition (expireRefFrame) between resolve and execute, so a claim that depends on the wrong moment fails rather than passing by accident.

Live verification — iPhone Duo simulator (iOS 27.1), Agent Device Tester @ /automation

Built from this branch (pnpm build && pnpm clean:daemon), unscoped snapshot first, --verbose for the request ndjson.

probe result
scroll down 0.75 (your control) movement: 'moved' — Scrolled down by 0.75 of the viewport (502px); ndjson scroll_movement_observed {"movement":"moved","attempts":1,"durationMs":359,"requestedPixels":502} (second sample 344 ms). The one-capture cost claim holds on a device.
scroll top → snapshot → scroll up 0.75 movement: 'at-edge' — Scrolled up and no hidden content above was detected; ndjson attempts:2, durationMs:886. The quiet pair forms, no distance is claimed, and a legitimate end-of-content no-op is not refused.
snapshot -i → scroll down 0.75 (your ask) movement: 'unobserved', reason: baseline-presentation-drift. The gate fires on a device, not only in fixtures.
chained scroll down 0.75 (no capture between) movement: 'unobserved', reason: stored-surface-not-current — see below.

The gap, escalated by the live run

The refusal did not fire on the issue's own usage pattern. Repeat scroll down 0.75 without an intervening capture and every call after the first answers unobserved: the first scroll's own gesture is a device side effect, so the stored tree is no longer current when the second one plans. The distance is still reported next to that unobserved, which is the disclosure rather than a lie — but #2714's agent was repeating the command, so this design as shipped answers honestly once and then goes blind exactly where the report needed it.

I had filed this as a known gap; the run says it is the follow-up that has to land, and its shape is now concrete: hand the observation's own post-gesture capture forward as the next scroll's baseline, keyed on the side-effect lineage counter in ref-frame.ts (readSessionRuntimeRevision), so a bare chain keeps a current baseline while any other command invalidates it. Refusing it is what makes the guard correct, so the cache — not a looser freshness rule — is the only right answer. Tracked as the next unit; I did not want to grow this stack by a session-state change while it is under review.

scroll_no_progress now fires on real devices, on both platforms. My earlier note said the refusal was unreachable because /automation honors the synthesized swipe — that was about the list itself, not about what sits on top of it. With the tester's own confirmation sheet open (tap Open automation alert on /automation), the baseline is taken with the sheet up and the gesture lands on the sheet, so the analyzer resolves a container that still reports hidden content below while the contents never shift:

{ "reason": "scroll_no_progress", "direction": "down", "hiddenContentAt": "bottom",
  "containerRect": { "x": 303.7, "y": 244.3, "width": 320, "height": 92.3 },
  "requestedPixels": 502 }

success: false, COMMAND_FAILED, message scroll down moved nothing: the container still reports hidden content below and its contents never shifted, and the hint names the raw drag (swipe x1 y1 x2 y2 started inside the list) because the refusal knows where the gesture ran. The request log shows both reads on one producer (snapshot_capture {"backend":"xctest"} twice, same fallback route), so the pair was comparable rather than lucky.

On Android the same refusal fires on the tester's /catalog after a settled baseline: COMMAND_FAILED — scroll down moved nothing: the container still reports hidden content below and its contents never shifted. So the #2714 loop can no longer repeat itself silently: the first wasted scroll answers moved-grade honesty, and the second one fails the command.

Android: run, with the helper path proven first. pnpm build:android, pnpm clean:daemon, the development build installed via pnpm test-app:android, adb reverse tcp:8081, and snapshot -i --json on the tester's /catalog before any scroll claim:

androidSnapshot: { "backend": "android-helper", "helperVersion": "0.21.12", "helperApiVersion": "2",
                   "helperTransport": "persistent-session", "installReason": "outdated",
                   "captureMode": "interactive-windows", "windowCount": 2, "nodeCount": 53 }

helperVersion equals package.json's 0.21.12 (the stale 0.20.11-dev helper was replaced on the spot), so this is not the UIAutomator fallback. Then snapshot → scroll down 0.75:

scroll_movement_observed {"direction":"down","movement":"moved","attempts":2,"durationMs":441,"requestedPixels":1800}

movement: 'moved' as predicted, but on two reads rather than one: Android's first post-gesture read still showed the pre-gesture surface (a RecyclerView still settling), so the loop polled again and took the change on the second. The one-capture claim is iOS-specific; on Android the honest figure is two reads / ~440 ms. My earlier Can't find service: activity was my own error — I had used the emulator before sys.boot_completed.

What the Android run found is fixed in #2776. /catalog also gave me movement: 'moved' for a scroll that had not scrolled: flipping only a system status-bar icon (dumpsys battery set level) between the baseline and the post-gesture read was enough, because the classifier had been asked about the whole surface and the helper's tree carries the status bar. moved is now confined to the container the swipe ran inside, and that shape answers unobserved / change-outside-container.

🤖 Generated with Apex by Callstack

@thymikee
thymikee added this pull request to stack #2773 September 23, 2026 08:26
@github-actions

github-actions Bot commented Sep 23, 2026 •

Copy link
Copy Markdown

Size Report

Metric Base Current Diff
Installed (including dependencies) 4.79 MB 4.80 MB +5.6 kB
Package (unpacked) 4.79 MB 4.80 MB +5.6 kB
Package (download) 1.43 MB 1.43 MB +1.8 kB

Startup median (7 runs, lower is better):

Scenario Base Current Diff
CLI --version 29.6 ms 30.8 ms +1.2 ms
CLI --help 81.0 ms 86.1 ms +5.1 ms

@thymikee

Copy link
Copy Markdown
Member Author

Reviewed at 8a8bd46. The code is not ready, and the live validation is still missing.

pollForSurfaceVerdict returns movement: 'moved' as soon as classifyBaselineSurfaceEvidence(baseline, observed) says changed (scroll-movement.ts#L199). The comparability check (presentationKey and comparisonKey equality) runs only on the settled/unchanged branch in compareWithBaseline (#L283). So a baseline and an observed tree from different capture lineages, for example after an XCTest-channel or iOS route fallback, can report moved even though the module itself treats that pair as not comparable. The deferred loop in capture-kit rebases on a backend change before it classifies (post-gesture-stability.ts#L129), so the two loops don't apply the same rule. Can every movement claim, including moved and the refusal, require matching keys first, and return unobserved (or rebase) on drift? A test with a lineage-drifted baseline whose frame differs, asserting unobserved, would pin it.

This changes the device-facing directional scroll path on iOS and Android: it adds a post-gesture capture and a new scroll_no_progress failure. The tests only call resolveBoundScrollRuntime directly, so nothing shows the refusal firing on a real inset scroll view or staying quiet on a working list. Could you run the test app's /automation route: snapshot (not -i), then repeat scroll down 0.75 until it fails, and confirm details.reason is scroll_no_progress with a containerRect near (18,178,366,662)? As a control, /catalog with scroll down 0.75 should return movement: 'moved' with scroll_movement_observed attempts:1 in the --debug ndjson. Please also report the movement value of one scroll right after snapshot -i, and one Android run of the moved case.

Is the scope proportional to the problem? The PR adds a module, a new dispatch seam in runtime-admission.ts, a contract change and new daemon behavior, at about +5.3 kB unpacked against the 3 kB threshold. GenericExecutionDispatch now passes the full SessionState and DaemonRequest into every generic leaf, which forced edits in back, focus, orientation, system-button and tv-remote, though scroll is the only consumer (runtime-admission.ts#L80). resolveGenericRuntimeExecution already has req and session; could the frozen baseline be taken there and passed into resolveBoundScrollRuntime, dropping the seam and those five edits? Also, scroll top and scroll --until already read readScrollEdgeState for canScroll, containerRect and the fingerprint. Would comparing that result before and after the gesture, plus the quiet-pair and comparability gates, cover most of what the three reporter families and the 8-value blind-reason union do, and leave one definition of "the scroll moved" instead of two?

CI was still queued or running at review time. Smoke Tests assert live scroll down 0.75 and the test-app scroll scenarios, and Integration and Coverage exercise the scroll runtime, so a failure in those is likely related to this diff.

@thymikee

Copy link
Copy Markdown
Member Author

Reviewed at b4e88b6, as a follow-up to the 8a8bd46 review. The main defect from that review is still there; nothing in b4e88b6 touches it.

pollForSurfaceVerdict still returns kind: 'moved' as soon as classifyBaselineSurfaceEvidence reports 'changed' (https://github.com/callstack/agent-device/blob/b4e88b6/src/daemon/scroll-movement.ts#L199), while the comparability gate in compareWithBaseline (presentationKey at L287, comparisonKey at L291) only runs on the settled branch. A baseline from snapshot -i, or one captured under a different lineage such as an XCTest-channel or iOS route fallback (#1569), won't match the broad post-gesture capture on those keys, and the module's own rule says that pair isn't comparable — yet a directional scroll that never moved can still answer 'moved'. That is the dishonest distance claim #2714 was meant to remove, back in a different shape. Every movement verdict — moved, at-edge, unchanged, and the scroll_no_progress refusal — should only be issued for a baseline/observed pair whose presentationKey and comparisonKey match; check that in pollForSurfaceVerdict before classifying each observed capture, return 'unobserved' (or rebase, as post-gesture-stability.ts does) on drift, and add a test where a drifted baseline has a different frame and asserts 'unobserved'.

The PR changes the device-facing directional scroll path on both iOS and Android, adding a post-gesture capture and the new scroll_no_progress failure, but the tests only exercise resolveBoundScrollRuntime against fixtures, and the PR body says no live run has been done. So nothing yet shows the refusal firing on a real inset scroll view, or staying quiet on a working list, on either platform. On the test app: run iOS /automation snapshot (not -i), then repeat scroll down 0.75 until it fails, and show details.reason === 'scroll_no_progress' with a containerRect near (18,178,366,662); run /catalog scroll down 0.75 and show it returns moved with scroll_movement_observed attempts:1 in the --debug ndjson; report the movement value of one scroll run taken directly after snapshot -i; and do one Android run of the moved case.

Carried over from the 8a8bd46 review, still open: resolveGenericRuntimeExecution already builds a frozen baseline with req and session — could that go straight into resolveBoundScrollRuntime instead of through the GenericExecutionDispatch { session, request, dispatchContext } seam at runtime-admission.ts:82-83, which forced five mechanical edits in back, focus, orientation, system-button and tv-remote? scroll top and --until already build "did the scroll move" from readScrollEdgeState before and after the gesture plus the quiet-pair and comparability gates, and that path could replace most of the 475-line scroll-movement.ts and its 8-value blind-reason union with one definition of movement. What would need to change first: either an answer to that question, or accepting readScrollEdgeState as the single owner of the scroll-movement fact.

The Android smoke job failed at step 47, wait text "Long presses: 1" (live-automation-scenario.ts:152), right after scrollToVisibleSelector (android-emulator-e2e/live-assertions.ts:25-51) issued scroll down 0.25 through executeDirectionScroll, the path this PR changes. The scroll steps themselves succeeded with no scroll_no_progress, since scroll steps don't set allowFailure, and the failing assertion is on a longpress count this PR doesn't touch, so I found no mechanism tying the diff to the miss — but I couldn't read the step-47 snapshot or screenshot artifacts, or a main-branch run of the same job, to rule it out. The other listed jobs were cancelled and give no signal, and since this diff runs through the Integration, Coverage and iOS smoke routes, it needs a fresh full run.

Next step: the comparability gate needs to run before every movement verdict, with a drifted-and-changed test proving it; then the live iOS and Android runs above need to show the numbers they're supposed to show, and CI needs a clean rerun on this commit.

@thymikee
thymikee force-pushed the fix/scroll-observed-movement branch from b4e88b6 to ec02962 Compare September 23, 2026 10:22
@thymikee
thymikee removed this pull request from stack #2773 September 23, 2026 10:23
@thymikee
thymikee changed the base branch from fix/scroll-surface-vocabulary to fix/scroll-movement-observation September 23, 2026 10:23
@thymikee
thymikee added this pull request to stack #2777 September 23, 2026 10:23
@thymikee

Copy link
Copy Markdown
Member Author

Both findings were right; the first was a real bug. Pushed as ec0296227, stack re-laid as #2770 → #2776 → this → #2772 (the module split out so no layer passes the line budget), rebased onto current main.

1. Lineage gate ran only on the quiet path — fixed. pollForSurfaceVerdict classified before it compared keys, so a drifted pair could answer moved. The check (presentationKey, then the comparison key) now runs on every capture, ahead of the classifier, so moved and the refusal both require a comparable pair. Kept the withhold-rather-than-rebase difference from the deferred loop and said why in the module doc: that loop asks whether a surface settles, so adopting a new producer keeps its question answerable; this one compares against a pre-gesture tree, so a drifted pair can never answer either way. Pinned by a capture lineage that changed mid-request withholds the claim, even as the frames differ — the frames differ on purpose, so the test fails if the gate only guards the quiet path. Its pair is a snapshot -i baseline. Deleting the gate turns exactly those two red (verified, then restored).

2. The dispatch seam — deleted, and you were right. resolveGenericRuntimeExecution already had req and session, so planScrollMovement({device, flags, session}) is called there and the plan reaches the closure as data. runtime-admission.ts is untouched now, and so are back, focus, orientation, system-button, tv-remote. It's also more honest: the flags and device the plan reads are pre-effect facts, so there was no reason to read them after the seam. Same pass removed the capture! assertion, collapsed three near-copies of scrollOnce → scrollResult into one dispatch/one return, and pulled the file's four hand-rolled capture inputs into one scrollCaptureInput — which made explicit a rule that was previously just an idiom repeated three times (a scroll read carries app + execution metadata and deliberately no caller snapshot flags). Net: −30 lines and no shared-type widening.

3. Scope / "one definition of moved". The movement claim is the daemon's existing definition — it calls classifyBaselineSurfaceEvidence, the strict both-directions bar and the signature stability from interaction-outcome-policy.ts, the same comparators every other gesture's deferred evidence uses. What readScrollEdgeState answers is a different question (hidden content left), and the signature cannot answer it. I did look at making the container fingerprint the movement signal, and it loses two things: the analyzer must resolve a container in both trees, so on the exact screen in #2714 (a list whose scroller the tree does not name, keyboard-occluded) there is no before/after at all and moved degrades to unobserved — the distance would again rest on the gesture plan, which is the bug; and a container-selection wobble between two captures would read as movement. So: one definition of unchanged, and the edge analyzer kept for is there more content. Happy to be pushed on that if you disagree — it's the one design point here I'd defend as-is. The blind-reason union is 8 diagnostic-only values (4 "not one screen", 4 "this pair can't support this claim"), never matched by callers; collapsible if you'd rather read 4.

4. Live run — done, and it found something. iPhone Duo (iOS 27.1), tester @ /automation, built from this branch. Your control passes: scroll down 0.75 → movement: 'moved', ndjson scroll_movement_observed {"movement":"moved","attempts":1,"durationMs":359,"requestedPixels":502} — one capture. At the top after scroll top + snapshot, scroll up 0.75 → at-edge, attempts:2, durationMs:886, and the message claims no distance. After snapshot -i → unobserved, reason: baseline-presentation-drift.

But scroll_no_progress never fired on the issue's own pattern. Repeat scroll down 0.75 with no capture between and every call after the first is unobserved / stored-surface-not-current: the first scroll's own gesture invalidates the tree the second one plans against. So the refusal — the thing this feature was written for — is unreachable for an agent that just repeats the command, which is exactly what #2714's reporter was doing. It reports honestly (unobserved beside the distance) but it does not catch it. I've escalated that from "known gap" to the named follow-up on the PR body, with the shape I think is right: carry this observation's own post-gesture capture forward as the next scroll's baseline, keyed on the side-effect lineage counter in ref-frame.ts, so a bare chain keeps a current baseline and any other command invalidates it. I held it out of this stack deliberately (it touches session state), but I'd rather you see it as required-than-implied — say the word and I'll land it here instead.

The refusal is additionally unproven on-device in another way: /automation's list honors the synthesized swipe (it kept moving under repeated scrolls), so that screen has no container that reports hidden content while ignoring a gesture. A real one is still owed. Android was not run: ReactNative_API_35 booted but adb shell am start -a android.settings.SETTINGS and the daemon's open both fail there with Can't find service: activity, and per the device-verification doc the run would also need pnpm build:android plus a snapshot -i --json androidSnapshot.backend: android-helper proof first. The exact owed probe is in the body.

On size: the report still reads +5.0 kB unpacked; the seam removal is most of the reduction available, the rest is the module.

@thymikee

Copy link
Copy Markdown
Member Author

Reviewed ec02962. This is still the only layer in the stack that changes a device answer: it adds a post-gesture capture and a new scroll_no_progress failure to the direction path on iOS and Android (scroll-runtime.ts#L130).

The two live runs from the earlier review are still missing. The Android emulator failed with Can't find service: activity, so nobody has shown that the Android helper capture gives a comparable pair against a stored snapshot. And scroll_no_progress has not fired on a real device yet. The iOS moved, at-edge and snapshot -i cases are covered by your report.

For Android: build with pnpm build:android. Run snapshot -i --json and check that androidSnapshot.backend is android-helper. Then run snapshot on tester /catalog and scroll down 0.75 --verbose. The run passes when you see movement: 'moved' and scroll_movement_observed attempts:1. For the refusal: use a container that reports hidden content below but ignores the swipe (the #2714 fixture works). Run snapshot, then scroll down 0.75. Check for details.reason === 'scroll_no_progress' with a containerRect.

One question: a chained scroll down with no capture in between answers unobserved after the first call. So the refusal cannot fire on the repeat pattern from #2714. Is it planned in this stack to keep the post-gesture capture as the next baseline, or is that a follow-up?

CI on ec02962 is cancelled or queued after the force-push, with no failed logs. This diff changes the scroll steps in Smoke Tests, so we still need a finished run on this commit. Next step: the two device runs above, then green CI.

@thymikee
thymikee force-pushed the fix/scroll-observed-movement branch from ec02962 to cc4d6b9 Compare September 23, 2026 12:17
@thymikee
thymikee force-pushed the fix/scroll-observed-movement branch from cc4d6b9 to 7be7922 Compare September 23, 2026 12:21
@thymikee
thymikee force-pushed the fix/scroll-observed-movement branch from 7be7922 to e533710 Compare September 23, 2026 12:25
@thymikee
thymikee force-pushed the fix/scroll-observed-movement branch from e533710 to 55797fc Compare September 23, 2026 12:27
@thymikee

Copy link
Copy Markdown
Member Author

The rename since ec02962 is the only change on this delta; the Android evidence gap from that review is still open. This is a follow-up on the earlier review.

The direction path adds a post-gesture capture and a new scroll_no_progress refusal, on both iOS and Android: https://github.com/callstack/agent-device/blob/55797fc/src/daemon/scroll-runtime.ts#L140. Nobody has shown the Android helper capture gives a comparable pair against a stored snapshot, and nobody has shown the refusal firing on a device. Without that, directional scroll on Android could answer unobserved or refuse wrongly, and it would ship unnoticed. Can you run pnpm build:android, confirm snapshot -i --json reports androidSnapshot.backend == android-helper, then on the tester /catalog route run snapshot followed by scroll down 0.75 --verbose, and show movement:'moved' with scroll_movement_observed attempts:1 in the output? For the refusal, can you run the same on a container that reports hidden content below but ignores the swipe (the #2714 fixture), and show details.reason === 'scroll_no_progress' with a containerRect?

All 16 checks were queued when I looked, and none had failed. Smoke Tests and Integration Tests exercise this route (scroll-runtime.ts, generic-runtime-execution.ts, and the contracts use declaration all changed), so they need a finished green run on 55797fc.

I did not run tests or devices; this review covers only the logical delta from the range-diff. The rebase moved the base from 15ae636 to a26d175, so I did not re-review the base layers from #2770 and #2776. Is the chained-scroll baseline carry-forward that you mentioned as a required follow-up tracked anywhere, since it isn't part of this stack?

The next thing this needs is an Android helper-backed run showing movement:'moved', one on-device run showing the scroll_no_progress refusal, and a finished green run of Smoke and Integration on 55797fc.

@thymikee
thymikee force-pushed the fix/scroll-observed-movement branch from 55797fc to 745a88f Compare September 23, 2026 12:43
@thymikee

Copy link
Copy Markdown
Member Author

Both device runs are done, and your Android question is what surfaced a false moved that #2776 now fixes. On 745a88f59.

Android, with the helper path proven first — pnpm build:android, pnpm clean:daemon, the development build via pnpm test-app:android, adb reverse tcp:8081, then snapshot -i --json on the tester's /catalog before any scroll claim:

androidSnapshot: { "backend": "android-helper", "helperVersion": "0.21.12", "helperApiVersion": "2",
                   "helperTransport": "persistent-session", "installReason": "outdated",
                   "captureMode": "interactive-windows", "windowCount": 2, "nodeCount": 53 }

helperVersion equals package.json's, so this is not the UIAutomator fallback (the stale 0.20.11-dev helper was replaced on the spot). Then snapshot → scroll down 0.75:

scroll_movement_observed {"direction":"down","movement":"moved","attempts":2,"durationMs":441,"requestedPixels":1800}

movement: 'moved' as you predicted, on two reads rather than the one I claimed: Android's first post-gesture read still matched the pre-gesture surface, so the loop polled again and took the change on the second. The one-capture figure is iOS-specific; two reads / ~440 ms is the Android cost. (For the record, my earlier Can't find service: activity was my own mistake — I drove the emulator before sys.boot_completed.)

scroll_no_progress fires on a real device. My earlier "unreachable" note was wrong: I had only tried the list itself, not what sits on top of it. On iOS, with the tester's confirmation sheet open (Open automation alert on /automation), the baseline is taken with the sheet up and the swipe lands on the sheet, so a container still reporting hidden content below never shifts:

{ "reason": "scroll_no_progress", "direction": "down", "hiddenContentAt": "bottom",
  "containerRect": { "x": 303.7, "y": 244.3, "width": 320, "height": 92.3 },
  "requestedPixels": 502 }

success: false / COMMAND_FAILED, message scroll down moved nothing: the container still reports hidden content below and its contents never shifted, hint naming the raw drag because the refusal knows where the gesture ran; the request log shows both reads from one producer (snapshot_capture {"backend":"xctest"} twice), so the pair was comparable rather than lucky. On Android the refusal also fires on /catalog after a settled baseline.

Your second Android probe is what caught a bug. Chasing why /catalog sometimes answered moved over an unmoved list, I found the whole-surface comparison was being satisfied by the system status bar: toggling a battery icon alone (dumpsys battery set level) produced movement: 'moved', attempts: 1. That is your note in #2776 about classifyBaselineSurfaceEvidence not checking the container rect, and it is fixed there — moved is now confined to the resolved container, with change-outside-container as the withheld reason.

On a chained scroll down with no capture between: follow-up, not this stack. The answer is not a looser freshness rule — after the first scroll's gesture, the stored tree genuinely is not the newest observation, so unobserved is correct and refusing it would be wrong. The fix is a cache: hand each scroll's own post-gesture capture forward as the next scroll's baseline, keyed on the side-effect revision in ref-frame.ts (readSessionRuntimeRevision) so any other command invalidates it. That is a session-state change with its own test obligations, and I did not want to grow a stack under review by one. It does mean #2714's exact repeat pattern (scroll down six times, no captures) reports unobserved per call rather than a refusal on the second — the refusal now fires as soon as a capture is in the loop, as the runs above show. If you would rather that gap blocked this PR, say so and I will land the cache first.

CI: 745a88f59 has a full pnpm check:affected --run locally (all runnable checks) and fallow vs fix/scroll-movement-observation reports no issues; waiting on the pushed run for the green Smoke result. The two Smoke failures on ec02962 were not this diff: the same tree passed Smoke on #2772's head (bfffce26a, docs-only above this one), #2776's run passed, and #2770's failure was a runner restart during a snapshot of Apple's own Settings app (iOS runner was already restarted during this request), with unrelated branches failing the same pool in the same window.

🤖 Generated with Apex by Callstack

@thymikee
thymikee force-pushed the fix/scroll-observed-movement branch from 745a88f to 35d4071 Compare September 23, 2026 13:35
@thymikee

Copy link
Copy Markdown
Member Author

Rebased onto main at 19cb1fddd — this layer is now 35d4071af, and the full pnpm check:affected --run passes locally on the new head. Mapping your acceptance list to measurements, plus one re-measurement that only a device could give.

attempts:1 did not reproduce on Android, and the reason is worth having on the record. The mechanism is capture speed, not staleness: the helper's interactive capture returns in ~190 ms, which is faster than the fling lands, so the first post-gesture read still matches the pre-gesture tree and the loop takes the change on the second read. On iOS the runner capture itself costs ~350 ms, so by the time the one read completes the content has already moved — same code, different platform timing. Measured Android cost: attempts:2, durationMs:441, versus iOS attempts:1, durationMs:370. I deliberately did not add a first-read delay to buy the attempts:1 number: it would slow every successful scroll on the fast platform to make a metric on the slow one match a prediction, and durationMs in the diagnostic already shows the honest cost. Say the word if you would rather see the knob anyway.

The refusal was re-verified on devices, on both platforms — details.reason === 'scroll_no_progress' with a containerRect, hiddenContentAt, requestedPixels, and the raw-drag hint (iOS: the tester's confirmation sheet over /automation; Android: /catalog after a settled baseline). The literal #2714 fixture is not runnable here for the reason the issue itself records — that build cannot launch on the 27.x runtimes on this host — so the fixtures above are the closest reproducible "reports hidden content, ignores the swipe" surfaces, and they exercise the same predicate rather than a mocked one.

Your Android note then let me prove the #2776 fix on the machine that produced the bug. Same experiment on the same AVD, before and after confining moved to the container:

build dumpsys battery set level only, list never moved
before (745a88f59) scroll_movement_observed movement=moved attempts:1 durationMs:118
after (d97aba9fb, run twice) scroll_movement_unobserved reason=change-outside-container containerRect:{x:47,y:132,width:986,height:1931}

with the app's own row geometry identical across the gesture in both runs (Tools@209 | … | gesture-lab-card@481 | Gesture lab@524), so the withheld answer is the correct one and not a different false claim. The iOS control on the fixed build still answers movement=moved attempts:1 durationMs:370, so the confinement did not cost the one-capture path.

Everything else on #2770 / #2776 / #2772 is answered in their threads; the chained-scroll baseline cache stays a follow-up as discussed.

🤖 Generated with Apex by Callstack

@thymikee

Copy link
Copy Markdown
Member Author

Reviewed 745a88f. The scroll-container gate from the earlier review (#2771 (comment)) is fixed, and this looks ready for human review on the code.

Not blocking: could #2776 also require a within-container difference for horizontal scrolls? verticalEdgeFor(direction) leaves the container rect undefined for left and right today (src/daemon/scroll-movement.ts#L380), so a horizontal status-bar test would cover it.

The Android and iOS device results are from the author's report on 745a88f. Smoke Tests and Bundle Size were cancelled or queued after the restack. This diff changes executeDirectionScroll, which the live scroll down steps use, so a finished Smoke and Integration run on 745a88f is needed before merge.

The head has since moved to 35d4071, a rebase onto main in which this commit is patch-identical, so the review above still applies.

@thymikee thymikee added the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Sep 23, 2026
@thymikee

Copy link
Copy Markdown
Member Author

CI note, so the red Smoke is not read as this layer's: the failing assertion is test/integration/ios-simulator-e2e/live-snapshot-depth-frontier.ts:126, which requires a regular depth-1 snapshot to disclose the Simulator AX bridge evidence gap. In the failing run the payload reports snapshotDiagnostics.stats.backends: {"xctest":55} with p95 2318ms — the AX bridge never spawned, every capture went through the runner, and the disclosure the assertion demands therefore cannot exist.

That is #2784: packages/platform-apple/src/snapshot-source/host.ts spawns the bridge with a bare xcrun simctl spawn <udid> …, without the scoped simulator set every other simctl call passes. It entered this stack only through today's rebase onto 19cb1fddd (the regression is ff678bb093, #2762, 2026-09-22), which is why the same lane was green on the previous base. #2818 is the fix in flight.

This layer's diff is 19 files and touches neither the bridge nor snapshot warning assembly, and 14 of 15 items in that job passed — including the scroll assertions this layer owns. Rerun triggered; the lane should settle when #2818 lands.

🤖 Generated with Apex by Callstack

@thymikee
thymikee merged commit 4bad784 into main Sep 23, 2026
23 of 35 checks passed
@thymikee
thymikee deleted the fix/scroll-observed-movement branch September 23, 2026 14:19
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