Skip to content

fix(android): drop the dumpsys scroll-hint probe instead of capping it (#1270)#1314

Merged
thymikee merged 1 commit into
mainfrom
claude/agent-device-issue-1270-c1dc06
Jul 17, 2026
Merged

fix(android): drop the dumpsys scroll-hint probe instead of capping it (#1270)#1314
thymikee merged 1 commit into
mainfrom
claude/agent-device-issue-1270-c1dc06

Conversation

@thymikee

Copy link
Copy Markdown
Member

Closes #1270.

Summary

#1288 rescoped this issue to budget hygiene and fixed the two things it could see: it capped dumpsys activity top at 1.5s and skipped hint derivation in the presence-only wait polls. It left the trigger itself in place — as the diagnosis comment predicted, the probe still fires "whenever a scrollable-TYPED node exists while no node is actually isScrollable()".

That surviving trigger turns out to be pure cost with no possible benefit, so this removes the probe rather than bounding it.

Why the probe can never help

The snapshot helper is the only capture backend (it hard-fails otherwise since #1281/#1217), and it emits can-scroll-forward/can-scroll-backward only when node.isScrollable() (SnapshotInstrumentation.java:555-568). The parser already maps those to hiddenContentAbove/hiddenContentBelow.

So on the only backend we have, absence of the attributes means nothing on screen scrolls — not "scroll state unknown". The probe fires only when the attributes are absent, i.e. only when there is no scrollable content for it to describe. hasAndroidScrollActionAttributes was a stock-uiautomator-era distinction (scroll-hints.ts predates the helper, #343/#427); the fallback it guarded became unreachable-in-any-useful-way when the helper became mandatory.

Live evidence (emulator API 37, 1344x2992 — same environment as the issue's evidence)

Reproduced the surviving trigger on com.callstack.agentdevicelab (a ScrollView whose content fits → no can-scroll-*), n=5 snapshot --interactive per side:

screen before (main, 1.5s cap) after hinted nodes
trigger screen (probe fires) median 2716ms median 1407ms 0 → 0 (identical)
Settings root (probe short-circuits) median 790ms median 675ms 1 → 1, identical (ScrollView below:true)

The probe burned the full 1.5s cap on every capture and returned 0 hints. Uncapped it took ~10.2s and still returned 0 hints — exactly as predicted, because nothing was scrollable.

Mechanism behind the variance: the raw dump shows dumpsys activity top serializing a view dump of every top activity through each app's main thread. One busy app stalls the whole call until Android's own limit:

ACTIVITY com.google.android.deskclock/... 
    Failure while dumping the activity: java.io.IOException: Timeout
*** SERVICE 'activity' DUMP TIMEOUT (10000ms) EXPIRED ***

That explains the 37ms → 5.9s spread in the original evidence: the cost depends on an unrelated app's main thread, and was charged to our wait/get budget.

Settings root (the screen from the original report) already short-circuited on the helper path, consistent with the "fixed-by-side-effect" note — hence the 1/20 bottom-wait residual in the wave-3 run rather than 8/20.

Changes

  1. Remove the probedumpActivityTop, ACTIVITY_TOP_TIMEOUT_MS, and deriveScrollableContentHintsIfNeeded are gone, along with the 1.5s cap comment that justified the workaround. Deletes scroll-hints.ts (385 lines, its only consumer) and the now-orphaned applyHiddenContentHintsToNodes.
  2. Hints are unchanged. Scrollable nodes get their hints from the parsed helper attributes, as they already did on every screen reporting a scroll action. The geometry (deriveMobileSnapshotHiddenContentHints) fallback is untouched and still applies when the helper reports no scrollable node. The full-tree path's hint block was a no-op once the native derivation went, so it's dropped; parser-set hints are unaffected (includeHiddenContentHints: false never removed those).
  3. includeHiddenContentHints plumbing is keptfix(android): bound the scroll-hint dumpsys probe and skip it in wait polling (#1270) #1288's wait-poll skips still apply (now to in-process geometry only), and alert.ts predates that flag.

Tests

  • snapshotAndroid never probes the activity dump for scroll hints (new) — pins Android capture: slow dumpsys activity top scroll-hint call (intermittently ~5s) charged against the wait/get timeout → false 'not found' on unchanged screens #1270 at the surviving trigger (scrollable-typed node, no can-scroll-*), full and interactive paths. Replaces the two probe tests (the 1.5s-cap assertion and its short-circuit twin).
  • Two fixtures corrected. preserves hidden scroll content hints in interactive snapshots and preserves bottomed-out hidden-above hints fed a mid-scrolled ScrollView carrying no can-scroll-* — XML the real helper cannot emit (a scrolled ScrollView always reports scrollable="true" plus its actions). Both now use realistic helper XML and keep their original assertions, so the guarantee they describe is unchanged; only the unreachable input is gone.
  • Verified revert-sensitivity: deleting the canScrollBackward → hiddenContentAbove mapping fails exactly those two tests, so they pin the parser mapping that is now the sole hint source.

Test plan

  • pnpm typecheck, pnpm exec oxlint --deny-warnings, pnpm format:check, pnpm check:layering, pnpm check:production-exports
  • pnpm check:fallow --base origin/main — no issues in changed files
  • src/platforms/android/__tests__/snapshot.test.ts (43/43), selector-read.test.ts (17/17)
  • pnpm check:affected --base origin/main --run — 4218 passed; 7 unrelated failures, all ~5s timeouts in client-metro/runtime-hints/apple index (none reference the touched files) on a host running an emulator. All 134 pass in isolation — the known contention flake.
  • Live A/B on emulator API 37, both the triggering screen and the short-circuiting control (table above)

#1270)

The snapshot helper is the only capture backend and emits `can-scroll-forward`/
`can-scroll-backward` for exactly the nodes Android reports as scrollable. Their
absence therefore means nothing on screen scrolls, not that scroll state is
unknown — so the `dumpsys activity top` probe only ever ran when there was no
scrollable content for it to describe.

#1288 bounded that probe at 1.5s rather than removing it, leaving every capture
of a screen with a scrollable-typed but non-scrollable node (a list short enough
to fit) paying the cap for hints that cannot exist. `dumpsys activity top`
serializes a view dump of every top activity through each app's main thread, so
one busy app stalls the call until Android's own 10s service-dump timeout — the
mechanism behind the 37ms-to-5.9s spread in the issue's evidence.

Hints for genuinely scrollable nodes are unaffected: they come from the parsed
helper attributes, which is where they already came from on every screen that
reports a scroll action.
@github-actions

Copy link
Copy Markdown

Size Report

Metric Base Current Diff
JS raw 1.8 MB 1.8 MB -4.6 kB
JS gzip 568.1 kB 566.6 kB -1.5 kB
npm tarball 682.8 kB 681.4 kB -1.5 kB
npm unpacked 2.4 MB 2.4 MB -4.6 kB

Startup median (7 runs, lower is better):

Scenario Base Current Diff
CLI --version 25.4 ms 25.5 ms +0.1 ms
CLI --help 55.4 ms 56.1 ms +0.7 ms

Top changed chunks:

Chunk Raw diff Gzip diff
dist/src/internal/daemon.js -4.6 kB -1.5 kB

@thymikee

Copy link
Copy Markdown
Member Author

Exact-head review (85e56043e): code-review clean. The remaining #1270 helper-path hazard is removed at the production seam: Android snapshot capture no longer calls dumpsys activity top; helper-native can-scroll-* attributes remain the authoritative scroll-state source, and the existing in-process geometry fallback remains only for interactive presentation when Android reports no scroll actions. The regression exercises the former trigger in both full and interactive capture, corrected fixtures match helper-emittable XML, and no production import/reference can reintroduce the deleted probe. Live API 37 A/B evidence covers both the triggering screen and a short-circuiting control. All 23 checks are green and the branch is mergeable. Ready for human merge.

@thymikee thymikee added the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Jul 17, 2026
@thymikee
thymikee merged commit a68fcdd into main Jul 17, 2026
23 checks passed
@thymikee
thymikee deleted the claude/agent-device-issue-1270-c1dc06 branch July 17, 2026 07:49
@github-actions

Copy link
Copy Markdown
PR Preview Action v1.8.1
Preview removed because the pull request was closed.
2026-07-17 07:49 UTC

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

1 participant