refactor(android): raw is the acquired tree; one presentation for dialog recovery; residues declared (#1832 C3) - #1865
Conversation
|
Size Report
Startup median (7 runs, lower is better):
Top changed chunks:
|
|
[P1] Make dialog recovery consume the presentation’s occlusion result. Please also make the PR scope and migration claim exact. This is 27 files and includes a mechanical shared Exact-head CI is green and the C3/raw projection work plus Pixel evidence otherwise look sound, but this should not be labeled ready until the recovery behavior is fixed and proven. |
…sult Review P1 on #1865: routing blocking-dialog recovery through buildSnapshotState made the occlusion result available but nothing consumed it. containsBlockingDialog scanned every node and findCloseAppButton returned the first text match with a rect, so a stale ANR surface left under the foreground one could still trigger recovery, and a covered "Close app" could be tapped ahead of the visible top button — the disagreement the routing was supposed to remove. Both decisions now filter through isSnapshotNodeInteractionBlocked, the shared predicate over the annotator's structured result. Two regressions cover it, both proven red against an unfiltered selection: a covered Close app preceding a visible one (asserts the visible center is tapped) and a fully covered dialog signal (asserts recovery does not trigger, no tap dispatched).
|
Both P1 points addressed in Recovery now consumes the occlusion result. You were right that routing through Two regressions in
The pre-existing ref-frame test stays green in both states, so the reds are attributable to the filter and not to fixture drift. Note the fixtures had to use a real overlay-kind type ( Scope and step claims corrected. The summary now says Android behavior changes plus one behavior-neutral shared extraction, and names it: 27 files, CHANGELOG gained the recovery behavior change. |
|
Re-reviewed exact Hold readiness for three evidence/status items:
Do not apply the readiness label yet. |
|
All three items addressed — one of them by admitting a gap rather than closing it. 1. File count corrected to 28 in the body (the occlusion regression added in the blocker fix is the 28th). 2. Exact-head CI is complete: 30/30 green on 3. Live ANR pass — partially done, and I want to be exact about which part. What I did drive live on a throwaway Pixel 7 (after
What I could not stage, and why: a genuine ANR dialog. So the dialog-present branch — detection, reachable-button selection, the tap, dismissal/relaunch — remains unit-only evidence, including the covered-first ordering. That is stated explicitly in the body's Validation section rather than implied. Closing it live needs either a rootable Sessions closed, emulator killed, throwaway AVD deleted; |
|
Went back at the live ANR item rather than leaving it as an offer. Result: I got a real ANR, but API 36 will not keep the dialog on screen, so the dialog-present branch stays unit-only — now with much sharper evidence about why. What I did. Built a purpose-made ANR fixture APK — one Activity that sleeps 120s on tap — compiled with this repo's own What that proved:
What the platform refused: with So, stated plainly in the body's Validation section: detection, reachable-button selection, the tap, and dismissal/relaunch remain unit-only evidence, including the covered-first ordering. The fixture recipe is written down for anyone on an image that does surface the dialog (tap once to hang, tap again to trip input dispatch) — with the caveat that while the app is frozen the a11y bridge returns a null root, so detection can only read the tree once the dialog window owns focus. Body also carries the corrected 28-file count and the 30/30 exact-head CI. Fixture uninstalled, session closed, recording stopped, emulator killed, throwaway AVD deleted — your |
|
Re-reviewed exact The new live run materially strengthens evidence for the ANR inspection-failure/fail-open branch, but it does not exercise the changed positive dialog route: API 36 never surfaced a persistent ANR dialog, so detection, reachable Close-app selection/tap, and dismissal/relaunch remain unit-only. Per the device-facing readiness rule, hold |
|
The positive dialog route is now demonstrated live — the readiness item is closed on evidence, not on accepted risk. What made it possible. My earlier conclusion ('API 36 never surfaces the dialog') was wrong in its cause. App-error dialogs are suppressed on these emulator images until The live run (throwaway Pixel 7 / API 36, fixture APK that sleeps 120s on tap, built with this repo's aapt2/d8/apksigner pipeline):
So detection, reachable-button selection, the tap, and dismissal are all exercised against a real system dialog on the changed path. Still unit-only, and I think justifiably: the synthetic covered-first ordering (a stale covered "Close app" preceding the visible one). Staging it live needs two simultaneous Close-app buttons with one occluded, which a real ANR does not produce. Body updated with this evidence and with the |
…log recovery; residues declared (#1832 C3) - C3: the three regular-projection pruners (invisible subtrees, stale application windows, covered same-window surfaces) move out of parseUiHierarchyTree into the projection as a non-mutating classification (collectAndroidHiddenNodes in ui-hierarchy-visibility.ts). --raw presents the acquired tree; interactive ⊆ regular ⊆ raw by construction. Hidden-content hints and the scope root are derived per projection over retained children, which is what the mutating pruners implied. Property-checked identical to main for regular/-i/depth/scope over 12,000 random tree × projection pairs; raw grew on 2,514/3,000 and never shrank. - Android blocking-dialog recovery routes through buildSnapshotState (the one presentation), which moves to src/daemon/snapshot-state.ts below the daemon-server type cycle; importers repointed, its tests mirror the module. - Freshness route signature drops role/selected (Android never carries them). - Residues declared at their sites and in CONTEXT.md; docs + CHANGELOG. - ui-hierarchy.ts split by question: node predicates (ui-hierarchy-node.ts), regular-projection visibility (ui-hierarchy-visibility.ts), scope (ui-hierarchy-scope.ts); 974 → 634 LOC.
…sult Review P1 on #1865: routing blocking-dialog recovery through buildSnapshotState made the occlusion result available but nothing consumed it. containsBlockingDialog scanned every node and findCloseAppButton returned the first text match with a rect, so a stale ANR surface left under the foreground one could still trigger recovery, and a covered "Close app" could be tapped ahead of the visible top button — the disagreement the routing was supposed to remove. Both decisions now filter through isSnapshotNodeInteractionBlocked, the shared predicate over the annotator's structured result. Two regressions cover it, both proven red against an unfiltered selection: a covered Close app preceding a visible one (asserts the visible center is tapped) and a fully covered dialog signal (asserts recovery does not trigger, no tap dispatched). Rebase reconciliation: screenshot-runtime.ts arrived on main (#1878) importing buildSnapshotState from its old home; repointed to src/daemon/snapshot-state.ts with the other importers.
233f92a to
f2d52b1
Compare
|
Rebased onto The rebase crossed 16 commits, two of which touch this seam: #1876 (one capture-input builder / admit-then-bind) and #1878 (screenshot migrated to the request-bound runtime).
That is the general hazard of a move-plus-repoint branch: conflicts catch collisions on lines you touched, never new callers of the old path. Typecheck is the thing that catches those, which is why the extraction is a compile-time move rather than a re-export shim. Still 28 files; [WARN] This project is configured to use 11.17.0 of pnpm. Your current pnpm is v11.21.0
[skip] android-helpers — GitHub-authoritative (jobs: Android / Smoke Tests) [skip] web-smoke — GitHub-authoritative (jobs: Integration Tests, Packaged CLI Node 22.12, Web Platform Smoke) [skip] replay-ios — parked, workflow_dispatch only (Replay Manual / iOS Replay Suite) [skip] replay-ios-device — parked, workflow_dispatch only (Replay Manual / iOS Replay Suite) [skip] replay-macos — GitHub-authoritative (jobs: macOS / Smoke Tests) [skip] replay-linux — GitHub-authoritative (jobs: Linux / Smoke Tests) [skip] replay-android — parked, workflow_dispatch only (Replay Manual / Android Full Emulator Suite) [run] format: pnpm run format:check All matched files use the correct format. [run] lint: pnpm run lint [run] typecheck: pnpm run typecheck [run] layering: pnpm run check:layering [run] fallow: pnpm run check:fallow --base origin/main Parameters: 95% low · 4% medium · 1% high · 0% very high (0-2 / 3-4 / 5-6 / >=7 params) ● High complexity functions (1 shown, 2 total) [run] build: pnpm run build [dedupe] vitest-related — covered by affected LCOV or GitHub CI [run] coverage: pnpm exec vitest related --run --passWithNoTests --maxWorkers=2 --coverage --coverage.reporter=lcov --coverage.thresholds.statements=0 --coverage.thresholds.lines=0 CHANGELOG.md CONTEXT.md android/snapshot-helper/src/main/java/com/callstack/agentdevice/snapshothelper/AccessibilityTreeCapture.java android/snapshot-helper/src/main/java/com/callstack/agentdevice/snapshothelper/AccessibilityTreeXml.java src/tests/test-file-size-ratchet.test.ts src/daemon/tests/android-system-dialog-occlusion.test.ts src/daemon/tests/generic-settle.test.ts src/daemon/tests/snapshot-state.test.ts src/daemon/android-snapshot-freshness.ts src/daemon/android-system-dialog.ts src/daemon/handlers/tests/interaction-ios-tap-outcome-fixtures.ts src/daemon/handlers/tests/interaction-settle.test.ts src/daemon/handlers/tests/interaction-touch-fixtures.ts src/daemon/handlers/tests/interaction.test.ts src/daemon/handlers/tests/snapshot-capture.test.ts src/daemon/handlers/snapshot-capture.ts src/daemon/screenshot-runtime.ts src/daemon/snapshot-presentation/ios/publication-membership.test.ts src/daemon/snapshot-presentation/ios/transitions.test.ts src/daemon/snapshot-state.ts src/platforms/android/tests/snapshot.test.ts src/platforms/android/tests/ui-hierarchy-scope.test.ts src/platforms/android/tests/ui-hierarchy.test.ts src/platforms/android/snapshot-helper-capture.ts src/platforms/android/ui-hierarchy-node.ts src/platforms/android/ui-hierarchy-visibility.ts src/platforms/android/ui-hierarchy.ts website/docs/docs/commands.md RUN v4.1.8 /Users/thymikee/Developer/agent-device/.claude/worktrees/issue-1806-fix-verify-713262 ✓ |interaction-contract| test/integration/interaction-contract/interaction-response-shape.contract.test.ts (10 tests) 12071ms Test Files 267 passed (267) [run] coverage: pnpm run check:coverage-changed --base origin/main Changed-line coverage gate: PASS
Uncovered changed lines:
[dedupe] provider-integration — covered by affected LCOV or GitHub CI [run] command-docs: pnpm run check:command-docs RUN v4.1.8 /Users/thymikee/Developer/agent-device/.claude/worktrees/issue-1806-fix-verify-713262 ✓ |unit-core| src/tests/command-doc-coverage.test.ts (11 tests) 4ms Test Files 1 passed (1) check:affected: all runnable checks passed. green on the rebased head (1869 tests). Validation section updated with the rebase facts, and the older 'rebased onto fda81c5' line replaced rather than left stale. CI is running on |
|
Re-reviewed exact |
thymikee
added
the
ready-for-human
1 participant
Summary
Second slice of #1832: Android behavior changes plus one behavior-neutral shared extraction. 28 files — the Android snapshot family, plus
buildSnapshotStatemoved verbatim out ofhandlers/snapshot-capture.tsintosrc/daemon/snapshot-state.tswith its importers repointed (generic-settle, iOS presentation, interaction fixtures) and its 409 lines of tests moved to mirror it (28th file = the occlusion regression added in review). The moved function is byte-identical to main's; no iOS or shared behavior changes.Covers step 4 (pruners into the regular projection), step 8 (residues), and the system-dialog half of step 3. The other half of step 3 — the Android
RawAXNodefixture schema — is deliberately not here: its only consumer is the cross-runtime conformance suite in step 9, which needs #1797's Swift presentation to exist as a callable target, so landing the format now would pin a fixture shape nothing reads.C3 —
--rawis the acquired tree. The three pruners (nodes Android marks invisible, stale application windows, covered same-window surfaces) ran insideparseUiHierarchyTree, mutating the tree before either projection existed, so--rawwas not the acquired tree and pruned content was unrecoverable. They now run as a non-mutating classification (collectAndroidHiddenNodes) that the regular projection consults;--rawpresents everything the helper serialized (normalization only).interactive ⊆ regular ⊆ rawholds by construction.Mutation had two silent consequences the move exposed, both fixed here:
node.children, which the mutating pruners had already emptied. They now count the children that projection shows — the semantics the old order implied but could not state.Behavior evidence. A property differential ran main's implementation and this one over 3,000 random accessibility trees × 4 projections (regular,
-i,--depth 2,--scope), comparing full node signatures including depth, parent edge,hittableand both hidden-content hints: 12,000/12,000 identical. Raw grew on 2,514 of 3,000 trees and never shrank. The differential found two real divergences during development (the twochildrensemantics above); both are fixed and pinned by unit tests. The harness was scaffolding against a copy of main and is not part of the diff.One presentation for dialog recovery, and it acts on the result (the #1784 pattern, daemon-side):
android-system-dialog.tshand-rolledattachRefs(pruneGroupNodes(...)), skipping normalization and occlusion annotation, so blocking-dialog detection could disagree withsnapshotabout which button is on top. It now callsbuildSnapshotStateand consumes that presentation's occlusion result: bothcontainsBlockingDialogandfindCloseAppButtonfilter throughisSnapshotNodeInteractionBlocked, so a stale ANR surface left under the foreground one no longer triggers recovery, and a covered "Close app" is never tapped ahead of the visible one. (Routing alone left that gap — caught in review.) Two regressions pin it, both proven red against unfiltered selection; the pre-existing ref-frame test stays green.buildSnapshotStatemoves tosrc/daemon/snapshot-state.ts(it needs no session state) so the daemon-server type cycle does not grow — R9/R10 stay at their baselines.Residues declared (step 8), each at its site plus
CONTEXT.md: nochecked/checkable/selected/long-clickableon the wire; the 5000-node cap applied device-side before any scope; API-level cache-reset divergence. And the residue that was actually load-bearing: the Android freshness route signature keyed onroleandselected, fields the Android backend never carries — two constant columns claiming discrimination they never had. Removed.Also:
ui-hierarchy.tssplits by question — node predicates, regular-projection visibility, scope — 974 → 634 LOC, each new module mirrored by its test file.Refs #1832 (migration step 4, step 8, and the system-dialog half of step 3).
Validation
pnpm check:affected --rungreen on the rebased head (1869 tests).--raw142 / regular 41 visible /-i9 on one screen, and raw 267 vs regular 65 after navigating to a pushed screen — every raw-only node isvisible-to-user="false", zero-height, or system chrome, i.e. exactly what the regular projection hides. Re-verified on this head afterpnpm build:androidfor 0.20.10 (Settings: raw 62 / regular 38 /-i16).open→record start→typeon a recording session drivesrecoverAndroidBlockingSystemDialogfor real. Request diagnostics show the helper session capture running inside that request and the command proceeding — the changed call site captures through the shared presentation, finds no blocking dialog, and does not false-positive. A second run with a deliberately stale helper artifact shows the other branch (android_blocking_dialog_inspection_failed→ fail-open,typestill succeeds).dumpsys windowshowsApplication Not Responding: com.example.anrlaband the dialog reads "ANR Lab isn't responding" / "Close app" / "Wait" — the exact text the detector matches. With that dialog on screen,typeon a recording session droverecoverAndroidBlockingSystemDialogthrough the changed path, and the request diagnostics recordandroid_blocking_dialog_recoveredwith the tap it dispatched (x: 540, y: 1205— the Close-app button), after which the dialog is gone and the app is foreground again. Detection → reachable-button selection → tap → dismissal all exercised against a real system dialog.settings put global hide_error_dialogs 0is set explicitly (it readsnullon a fresh AVD). A forcedam crashproduced no dialog either, which is what identified the setting rather than the ANR staging as the blocker. With it set, both crash and ANR dialogs appear.Pixel_7_CIuntouched).android-lifecycleprovider scenario,daemon-entrypoint) while an emulator was booting on the same machine; both pass in isolation in 8.3s and 1.3s.d29dc22(16 commits, including refactor(daemon): one capture-input builder and one admit-then-bind step #1876's capture-input builder and refactor: migrate screenshot to request-bound runtime #1878's screenshot runtime migration). One conflict, inrequest-generic-dispatch.ts: main had removed thebuildSnapshotState/captureSnapshotDataimports this branch was repointing, so main's side won outright. One reconciliation the conflict did not surface:screenshot-runtime.tsarrived on main importingbuildSnapshotStatefrom its old home, and now points atsrc/daemon/snapshot-state.tswith the other importers — caught by typecheck, noted in the commit message. Still 28 files.test-file-size-ratchetdrift was main's own and is fixed there by test(ratchet): pin snapshot-handler.test.ts at main's 2654 lines #1860; this PR only lowers its own file's pin, which shrank by 2 lines.provider-scenarios/settle-observation"press --settle" flakes under load on this machine — 1 of 3 runs passes on both this branch and plainorigin/main(5s fixture timeout), so it is pre-existing, not a regression from this change.Docs:
website/docs/docs/commands.md(what Android--rawnow shows, and the helper's state/cap limits),CONTEXT.md(projection vocabulary + declared residues), CHANGELOG.