Skip to content

feat(android): carry effective snapshot geometry - #1968

Open
thymikee wants to merge 5 commits into
mainfrom
fix/android-snapshot-engine-contracts
Open

feat(android): carry effective snapshot geometry#1968
thymikee wants to merge 5 commits into
mainfrom
fix/android-snapshot-engine-contracts

Conversation

@thymikee

@thymikee thymikee commented Aug 22, 2026

Copy link
Copy Markdown
Member

Summary

Carry Android helper geometry through a typed raw/effective model, then apply the owning window and cumulative scroll clips before regular snapshot nodes can become hittable.

Traversal/presentation and inclusion policy have focused owners with topology-mirrored tests. Raw output remains acquisition-only, and drawing-order membership is unchanged. Both parser and builder import Android system-chrome semantics from the narrow contract owner.

This completes #1832 C4. C1 remains evidence-gated.

Validation

  • The geometry property was observed red without the effective-geometry guard: fast-check shrank it to a zero-area rect that remained hittable.
  • Two-window regression proves descendants inherit their owning window and disjoint children collapse to zero-area effective geometry.
  • pnpm check:affected --run on 4940d432d: all runnable checks passed; 270 related files / 1,819 tests.
  • The exact prior head ran 7,739/7,740 coverage tests successfully; its sole owner-action failure was the wide contracts facade. The focused closure test now passes with the narrow owner, and fresh Coverage is running.
  • Live full-stack Android Settings evidence used android-helper 0.20.10, captured two windows with the nested scroll hierarchy intact, and completed in 142 ms. The verification session was closed.

Package impact on the prior exact head was +1.6 kB unpacked (+896 B tarball).

@github-actions

github-actions Bot commented Aug 22, 2026

Copy link
Copy Markdown

Size Report

Metric Base Current Diff
JS raw 2.37 MB 2.37 MB +1.2 kB
JS gzip 794.8 kB 795.2 kB +405 B
npm tarball 916.5 kB 917.0 kB +455 B
npm unpacked 3.18 MB 3.18 MB +1.2 kB

npm unpacked components

Component Base Current Diff
JS / dist source 2.51 MB 2.52 MB +1.2 kB
Apple runner source/project 564.2 kB 564.2 kB 0 B
macOS helper source 54.5 kB 54.5 kB 0 B
Android helper artifacts 0 B 0 B 0 B
Other package files 44.5 kB 44.5 kB 0 B

Startup median (7 runs, lower is better):

Scenario Base Current Diff
CLI --version 26.8 ms 26.3 ms -0.4 ms
CLI --help 78.7 ms 78.4 ms -0.3 ms

Top changed chunks:

Chunk Raw diff Gzip diff
dist/src/snapshot.js +1.2 kB +405 B

Top changed packed files

Packed file Base Current Diff
dist/src/snapshot.js 26.2 kB 27.4 kB +1.2 kB

@thymikee

Copy link
Copy Markdown
Member Author

Not ready at ed0bc6513 despite green CI. window-bounds exists on Android window roots, but descendants resolve geometry against node.windowRect ?? state.viewport, where state.viewport is the largest window anywhere in the capture. In dialog/IME/overlay captures, descendants of a smaller window can therefore be clipped against the main app window and retain nonzero regular rects/hittability outside their owning window. Carry/inherit the current root window rect through the walk and add a two-window regression with different bounds.

Android Smoke exercises the real helper→presentation route, but it does not assert partial clipping or multi-window effective geometry; changed-path live evidence is still missing. Size and the remaining structure are otherwise healthy.

@thymikee

Copy link
Copy Markdown
Member Author

Addressed in 7bb9d33c7.

  • regular traversal now carries the nearest window-bounds through descendants instead of falling back to the largest window
  • disjoint clipping now produces a fully zero-area rect, matching the Swift presentation contract
  • added a two-window regression with a smaller dialog child outside its owning window

The regression was observed red before the fix and focused Android/UI-hierarchy, lint, typecheck, format, and the final affected gate are now green locally (1,814 related tests).

@thymikee

Copy link
Copy Markdown
Member Author

The owning-window fix is correct and exact-head CI is green, but this is not ready at 7bb9d33c. The change grows src/platforms/android/ui-hierarchy.ts from 645 to 755 lines while adding traversal/window/clip behavior; AGENTS requires extraction before adding behavior past 500 lines. Move that owning traversal/presentation seam into a topology-mirrored sibling module/test. Also refresh the stale PR body/head/stats and record targeted live effective-geometry evidence (the current Android smoke exercises the route but does not assert the claimed multi-window rect).

@thymikee

Copy link
Copy Markdown
Member Author

Addressed the new review request in 52242d7c38bcc292b459b6e7ea24c9db052d68c4.

  • ui-hierarchy.ts is now 292 lines; traversal/presentation is in ui-hierarchy-builder.ts (352 lines) and inclusion policy is in ui-hierarchy-inclusion.ts (116 lines), with a topology-mirrored builder regression.
  • The regression is planted-red against the pre-feat(android): carry effective snapshot geometry #1968 builder: a node outside a smaller owning window retained positive geometry.
  • Final affected gate passed after format: 268 Vitest files / 1,815 tests and all runnable structural, build, and test checks.
  • Changed-path live multi-window evidence is still unavailable, so drawing-order pruning remains in place and C1 stays explicitly evidence-gated.

@thymikee

Copy link
Copy Markdown
Member Author

The effective-geometry carrier/route and concrete multi-window regression are sound, and size is reasonable, but readiness needs four things: (1) rebase the current DIRTY branch, preserving main’s narrow @agent-device/contracts/android-system-chrome owner rather than restoring broad platform-facade imports; (2) run the full authoritative suite—this exact extraction head currently has only CodeQL; (3) provide changed-path live Android multi-window/dialog evidence including helper backend/version and emitted effective rect; and (4) move tests with the extracted owners. ui-hierarchy.test.ts remains an 813-line parser/builder/inclusion aggregation, while presentation/inclusion lack topology-mirrored tests. Split those cases and add a fast-check geometry property proving regular rects stay within window+cumulative clips, raw rects remain unchanged, and non-positive effective geometry is never hittable.

@thymikee
thymikee force-pushed the fix/android-snapshot-engine-contracts branch from 52242d7 to f3a3e26 Compare August 22, 2026 16:45
@thymikee

Copy link
Copy Markdown
Member Author

Addressed on f3a3e26 after rebasing onto main.

  • preserved the narrow android-system-chrome contract owner
  • split presentation/inclusion tests out of the legacy aggregate
  • added a fast-check geometry invariant, observed red against the removed effective-geometry guard
  • exact-head affected gate passed (270 files / 1,819 tests)
  • full-stack live Android helper evidence is now recorded in the PR body (0.20.10, two windows, healthy, 142 ms)

The published branch is mergeable; required CI is running.

@thymikee

Copy link
Copy Markdown
Member Author

Exact-head Coverage has one owner-action failure: src/platforms/android/ui-hierarchy-builder.ts still value-imports the wide @agent-device/contracts/platform facade. The rebase preserved the narrow owner only in ui-hierarchy.ts, not the extracted builder. Replace the builder's type/value imports with one import from @agent-device/contracts/android-system-chrome (AndroidSystemChromeProvenance + isAndroidSystemChromeWindowResourceId), run the focused contracts-entry closure test, and obtain green Coverage on the new head. All other completed checks, including Android Smoke, are green. Do not apply ready-for-human yet.

@thymikee

thymikee commented Aug 23, 2026

Copy link
Copy Markdown
Member Author

Resolved the sole Coverage blocker in 4940d43: the extracted builder now imports both chrome provenance and the classifier from @agent-device/contracts/android-system-chrome; no value import of the wide platform facade remains. The focused closure test passes, and the full affected gate is green (270 files / 1,819 tests). Fresh Coverage is authoritative for readiness.

@thymikee thymikee added the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Aug 23, 2026
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