Skip to content

Inc 1: the loop closes on the documented path (C1, C2, C4, C5, C7, C9, C11, C15, L3, E1) - #14

Merged
fluxxion82 merged 10 commits into
mainfrom
fix/inc-1-loop-closes-from-output
Sep 3, 2026
Merged

fluxxion82 merged 10 commits into
mainfrom
fix/inc-1-loop-closes-from-output

Conversation

@fluxxion82

@fluxxion82 fluxxion82 commented Sep 3, 2026

Copy link
Copy Markdown
Owner

Inc 1's "see the screen -> act on it" half, plus round 1 (coordinator) and round 2 (Codex xhigh). Rebased onto 0c40d27#12's recordings, #13's quoting and #15's exit codes are all in, and app_launcher.launch() keeps both sides of the #13 merge: every argument quoted for the device shell and -W.

Suite: 1287 passed / 22 xfailed at the branch point → 1613 passed, 1 skipped, 0 xfailed now (the last xfails were #15's to delete, not mine). Black and Ruff clean. All three required checks pass, including the real-AVD lane — which is the only place the live rectangle check and the focus wait can be exercised. Every mutation transcript below was run on the pushed tree, restored with cp from a snapshot, never with git checkout.


Round 2 (Codex) and round 1 (coordinator)

INC1-02 = O1 — am start -W, from the recording instead of from memory

#12 recorded both halves, and both carry a trap:

  • success (am_start_wait_settings): Activity: names the component that actually came up, which for an alias is not the one requested — asking for com.android.settings/.Settings reports com.android.settings/.homepage.SettingsHomepageActivity. A launch therefore cannot be confirmed by comparing those strings. It is confirmed by Status: ok; the resolved component is information.
  • failure (am_start_wait_missing): no Status: line at allStarting: Intent {...}, Error type 3, Error: Activity class {...} does not exist., all on stdout with stderr empty (stderr_bytes: 0), exit 1.

The parser's old "no Status: means inert success" is exactly the failure shape, so it is gone: success requires status == "ok", and am_start_error() reads the Error: line off stdout. Both the exit-1 path and the bad-report path build the same message through _launch_failure, which names the remedy (cmd package resolve-activity --brief). The docstring claiming no ground truth is gone; four tests now read the two recordings, and _ok()-with-empty-stdout no longer stands in for a launch anywhere.

launch() returns (success, message, report) — the resolved activity has to reach the caller as data, not as prose, for INC1-10.

$ cp .../scripts/app_launcher.py $SNAP/app_launcher.py.snapshot
$ python3 /tmp/m3.py
MUTATED: no Status line is treated as success again
$ uvx ... pytest tests/test_app_launcher.py -k no_status_line -q
E  AssertionError: a launch with no `Status: ok` was reported as one
E  assert True is False
1 failed, 52 deselected
$ cp $SNAP/app_launcher.py.snapshot .../scripts/app_launcher.py

INC1-04 — ownership is structural, and a passive label is refused

The name test on the owner silently un-fixed C1 for every control named by a resource id: search_action_bar carries an id, so it recovers no caption, so it did not "answer to" Search settings and the tap went back to the passive TextView inside it. Ownership now asks only where the caption sits: nearest interactive ancestor, else the row-adjacent interactive sibling — next before previous, documented, because a caption follows its control in both recorded Compose layouts.

What makes that safe is the other end: a name-only search that resolves to nothing operable is refused — Not actionable: "X" is a passive label with no control — and exits non-zero rather than tapping a no-op.

$ cp .../scripts/navigator.py $SNAP/navigator.py.snapshot
$ python3 /tmp/m4.py
MUTATED: an owner must carry a name of its own again
$ uvx ... pytest tests/test_navigator.py -k caption_inside_a_control -q
E  AssertionError: fuzzy=True: Element(type='TextView', text='Search settings', ...
E    resource_id='com.android.settings:id/search_action_bar_title', bounds=(200, 638, 553, 709), ...)
E  assert (200, 638, 553, 709) == (42, 605, 1038, 742)
1 failed, 15 deselected
$ cp $SNAP/navigator.py.snapshot .../scripts/navigator.py

test_navigator_scroll.py's coordinate test is reverted from "any enclosing ancestor" to the exact row rectangle [0,1899][1080,2130], and a new test holds that literal to the recorded dump so it cannot rot.

INC1-08 — the bare id, in both scripts

Brief and Codex agreed and I was wrong: com.android.settings:id/search_action_bar now prints as search_action_bar in both scripts. --find-text accepts either form, matched whole (never as a substring — ids embed the package). --find-id is unchanged. Compose test tags were already bare, so this is the only choice under which both toolkits name things alike.

The loop spec's three inventory entries and its independent resolver move to the bare form. That is a deliberate spec change, marked as one in the file — not an adaptation of the expectation to the code: the two scripts printed different strings for one control and one of them had to give. test_navigator_scroll.py:541 is back to title.

INC1-01 — fullmatch, and eligibility means a rectangle

parse_bounds used re.match, so [1,2][3,4]junk parsed as (1,2,3,4) — a confident answer to a value nobody wrote. And is_interactive waved through a node whose bounds were missing or unreadable, which is "operable" asserted about something whose position is unknown.

$ python3 /tmp/m1.py
MUTATED: parse_bounds uses re.match again
$ uvx ... pytest tests/test_hierarchy.py -k trailing -q
E  AssertionError: trailing text -- re.match would take the numbers in front
E  assert (33, 754, 159, 880) is None
1 failed, 27 deselected

$ python3 /tmp/m2.py
MUTATED: is_interactive waves through unreadable bounds again
$ uvx ... pytest tests/test_hierarchy.py -k not_operable -q
E  AssertionError: assert not True
E   +  where True = hierarchy.is_interactive(_recorded_control(..., '[33,754]'))
1 failed, 2 passed, 25 deselected

Knock-on: an unreadable-bounds node can no longer be reached by name at all, so the C5 refusal test is re-derived onto the --find-id route (the route that still gets an agent there) using the recorded search_action_bar with its bounds truncated.

INC1-03 — a deadline that bounds the slow part

The clock now starts before the first capture (which is inside the search), is re-checked after the settle, and is passed into every dump — divided by CAPTURE_ATTEMPTS, so the retries cannot run past it either. The progress line prints when the scroll happens, not after the dump that follows it. And ANDROID_EMU_MAX_SCROLLS goes through the same validator as the flag, so the env default can no longer walk past the ceiling the flag enforces (it warns and clamps rather than crashing at import).

INC1-05 — one eligibility rule in the audit too

accessibility_audit computed is_interactive(node) for check 1 and still used clickable and enabled for the touch-target and resource-id checks. Now it is computed once and used by all three. The visible consequence: missing_resource_id on the Compose screen moves 6 → 7, the new one being the scrolling list at [32,1164][1048,1637], which is driven by scrollable and carries no clickable — a control the audit could not see while the rest of the skill could.

INC1-06 — the bounds cases are derived, not invented

test_hierarchy.py's parse_bounds cases were literals. Each is now the recorded Compose CheckBox with only its bounds attribute replaced (empty / one corner / trailing junk / wrong brackets / negative), with the reason stated: API 35 clips every rectangle to the display, so an unreadable one cannot be recorded on demand.

INC1-07 — one failure contract under --json

{"error": ...} on stdout with a non-zero exit for every navigator failure: not-found, no-usable-bounds, passive-label, a failed tap, and the argparse usage error C2 added (exit 2 — correct for usage — but the payload is now printed instead of nothing). A _JsonAwareParser reads --json from argv, because a usage error happens before there are parsed arguments. app_launcher's launch failure goes through #13's _report_failure unchanged.

INC1-09 — the guard says "one", not "none here"

The bounds guard enumerated zero sites outside common/hierarchy.py and checked only that the file existed — satisfied by a skill with no parser at all, or by two grammars in the shared file. It now enumerates the shared module positively: exactly one site, common/hierarchy.py (<module>) [regex] signed.

$ python3 /tmp/m5.py
MUTATED: the shared grammar is gone from common/hierarchy.py
$ uvx ... pytest tests/test_shape_guards.py -k shared_module -q
E  AssertionError: the guard no longer reports ['common/hierarchy.py (<module>) [regex] signed'].
E    Everything it did report, with the line it is on now: [].
1 failed, 28 deselected

INC1-10 = O2 — the live lane checks geometry, and waits for the right screen

The e2e no longer verifies the tap by navigator's own Tapped: line. It captures the screen the way the skill does (common.hierarchy.capture_hierarchy), resolves the target's rectangle with _expected_rect imported from test_agent_loop_from_output (not re-implemented, so the live lane and the mocked one cannot disagree about "the right control"), and takes the tap point from navigator's new --json field tapped_at: [x, y], documented in SKILL.md.

The focus wait no longer accepts "any activity in the package": --launch --json reports the activity am start -W resolved, and the fixture polls until the focused component equals that.


Round 4 — Codex's verification pass

All twelve earlier items came back RESOLVED. Four new ones, all P1, all small.

VER-01 — open_url was the last E1-shaped site in scripts/

It ran a bare am start and answered "Opened URL: …" unless the word "Error" appeared in stdout. An intent that resolves to nothing — a typo in the scheme, an app that does not declare the filter — was reported as a link the agent had followed. It now passes -W, reads its verdict through the same parse_am_start, requires Status: ok, and reports the device's own Error: line with a remedy.

The failure test uses the recorded am_start_wait_missing. Its docstring states plainly what is derived: that capture came from am start -W -n <component>, not from -a VIEW -d <url>, and what the two share is the shape the parser keys on — an Error: line and no Status: line. Every byte is the device's; only the command that produced them differs. The success control uses the recorded report, and test_device_shell_quoting's open_url case is repointed at it too, since an empty stdout is no longer a launch.

$ cp .../scripts/app_launcher.py $SCRATCH/app_launcher.v4.snapshot
$ python3 -   # verdict back to "no Error substring seen"
MUTATED: open_url infers success from the absence of an error again
$ uvx ... pytest tests/test_app_launcher.py -k deep_link -q
E  AssertionError: a link that opened nothing was reported as opened
E  assert True is False
1 failed, 1 passed, 53 deselected
$ cp $SCRATCH/app_launcher.v4.snapshot .../scripts/app_launcher.py

VER-02 — the live-scroll helper could still run against the wrong screen

It discarded the results of force-stop and am start -W, then fell out of the focus loop without asserting anything. Both are checked now, and the loop's else raises with the focused activity in the message. A test that runs navigator against the wrong screen and then blames navigator for what it finds is worse than one that does not run.

VER-03 — a force-stop nobody read is not a guarantee

The e2e's snapshot/ANR mitigation ignored --terminate's result, so a failed force-stop permitted exactly the stale state it was added to prevent. It now asserts exit 0 and the JSON success — hence --json on that call, and the Quick-Start baseline gains the flag.

VER-04 — the component comparison is backed by recordings

Both halves now come from recorded output, read with the production parsers:

  • equality: dumpsys_window_focus carries one activity spelled two ways — mCurrentFocus fully qualified, mFocusedApp with the leading dot. That is the entire phenomenon in two lines of real output, and parse_focused_activity reads each. (The pair is the launcher's activity rather than Settings, which is what that recording actually contains.)
  • inequality: the resolved activity from am_start_wait_settings via parse_am_start, against the requested component read out of that fixture's own manifest command — so neither side is retyped. If expansion collapsed the alias difference, the launch wait would accept any screen in the package.

The inline Compose pair is gone.


Round 3 — an owner must be tappable, not merely interactive

The trade I flagged at the end of round 2, and it was a real defect. _owning_control took the first ancestor whose .interactive was true, and is_interactive counts scrollable. A ScrollView or RecyclerView encloses nearly the whole screen, so for a caption in a list whose rows are not themselves clickable, the container was the first interactive ancestor — the tap went to its centre and the message still named the caption.

Measured on the recorded Settings screen with one row's clickable cleared:

MUTANT resolves 'Battery' to: ScrollView main_content_scrollable_container (0, 784, 1080, 2361) -> tap at (540, 1572)

(540, 1572) is the middle of the screen, reported as Tapped: ... "Battery" — C2's defect wearing different clothes.

common/hierarchy.py now answers two questions instead of conflating them, with one implementation behind both (_operable), so the enabled check and the rectangle check cannot drift apart:

  • is_interactive — unchanged, still counts scrollable. This is eligibility: what gets enumerated, counted and listed. A scroll container belongs there; an agent acts on it by scrolling, and it stays findable by its own name.
  • is_actionable — clickable / long-clickable / checkable. _owning_control requires this of an owner, in both the ancestor walk and the sibling rule.

A scrollable-only ancestor is passed over rather than accepted, so the walk continues upward, then tries the row, and failing both the existing refusal fires: exit non-zero, {"error": ...} under --json, naming the label.

Mutation — drop the action-property condition:

$ cp .../scripts/navigator.py $SCRATCH/navigator.r3.snapshot
$ python3 -   # owner need only be .interactive again, in both places
MUTATED: an owner need only be interactive again (scrollable counts)
$ uvx ... pytest tests/test_navigator.py -k owned_only_by_a_scroll -q
E  AssertionError: a caption owned by nothing tappable reported success
E  assert 0 != 0  +  where 0 = SystemExit(0).code
1 failed, 19 deselected
$ cp $SCRATCH/navigator.r3.snapshot .../scripts/navigator.py

Four tests, all derived from recordings per the fixture policy: the Battery caption resolving to its clickable row on the unmodified dump; the same caption refused once that row's clickable is cleared (no input tap issued, JSON error shape, "passive label"); the container still enumerable and still findable by its own name; and the Compose Checkbox/Switch/logo sibling cases restated, since ownership is exactly what would break them.


What the emulator lane found (three pushes, three real defects)

The lane is a required check and it earned it. None of the three was visible to the mocked suite.

1. The live scroll helper had the same launch race E1 fixed in the e2e. It force-stopped Settings, ran a bare am start, and dumped immediately — so navigator answered "nothing on it scrolls, so there is no content below" about the launcher. Now am start -W plus a bounded poll on dumpsys window through the shared parse_focused_activity. (The same push also fixed a live test still reading payload["message"] from a failed lookup, which INC1-07 renamed to error.)

2. A saved snapshot brings back an ANR dialog. The e2e mapped two controls — "Close app" and "Wait" — with the fixture app focused. That is an ANR dialog, and the failure message said so rather than blaming R11, which is exactly what E1 asked of it. The lane boots from a snapshot saved at the end of the previous run, so the fixture now force-stops the app before launching it.

3. One component, two spellings. am start -W echoes the class as passed (com.example.composefixture/.DefaultActivity); dumpsys window prints it fully qualified (.../com.example.composefixture.DefaultActivity). My "focused == reported" comparison called a correctly launched activity the wrong screen. Both sides are expanded before comparison now, with a test holding that to the exact pair the lane produced and to the alias pair from the recording — the alias must still compare unequal, which is the whole reason the wait is against what am start reported. The emulator mark moved from the module to the two device-backed tests so this one runs in the mocked check.

4. The e2e used a fixture it never requested. capture_hierarchy(compose_device) with compose_device absent from the signature: pytest's fixture definition object reached adb, and build_command reported 'FixtureFunctionDefinition' object has no attribute 'startswith'. Deselected without -m emulator, so only the lane could see it.


Round 0 — the original Inc 1 work

C2 — an action with no target

--tap/--enter-text with no --find-* and no --tap-at matched every enabled node; the first was the <hierarchy> root, so input tap 0 0 was issued and the exit status was 0. Now a parser.error before anything reaches the device, and _flatten_tree no longer emits the root.

$ python3 /tmp/m7.py
MUTATED: the criterion check is deleted
$ uvx ... pytest tests/test_agent_loop_from_output.py -k no_criterion -q
E  AssertionError: navigator --tap with no --find-* reported success (exit 0) and said: 'Tapped: FrameLayout "Unnamed" at (540, 1212)'
E  AssertionError: navigator --enter-text x with no --find-* reported success (exit 0) and said: 'Entered text in: FrameLayout "Unnamed"'

C1 — a printed name is findable, and resolves to the control it names

_find_in matched text + content_desc only, so recovered_label — the caption both --list and the screen report print — was not searchable: five of seven Compose names and ten of eleven Settings names returned "Not found". Where a name did match it matched the caption, whose centre is outside the control.

$ python3 /tmp/m6.py
MUTATED: _find_in matches text + content_desc only
$ uvx ... pytest tests/test_agent_loop_from_output.py -q
E  AssertionError: a name the screen report printed produced 0 taps, not one.
E      name:     'Order #4821 2 items Ships tomorrow'
E      control:  (32, 353, 1048, 584)
E      exit:     1
E      said:     "Not found: text='Order #4821 2 items Ships tomorrow' (searched 1 screen; ...)"
15 failed, 12 passed

(Before INC1-08 the same mutation also reproduced 'Remember me'(302, 816), the caption TextView, against the CheckBox at (33, 754, 159, 880).)

Seed switch: the case inventory is read off the default text report, not --json — JSON always exposed Control names, which is how a loop test passes with C4 unfixed.

C4 — the default report names what it counts

One line per interactive bucket, capped per bucket at BUTTONS_PREVIEW with , ... (N total); --hints seeded from the same buckets and printed as the navigator command to run next. The Compose report went from three lines naming nothing to naming all seven controls.

C7 + C5 — one bounds grammar, one eligibility rule

common/hierarchy.py owns parse_bounds, is_interactive, node_attributes and now bare_resource_id; navigator, screen_mapper and accessibility_audit consume them. An unparseable value is None, never (0, 0, 0, 0), whose centre is a real tappable pixel.

$ python3 /tmp/m8.py
MUTATED: Element.center falls back to (0,0,0,0)
$ uvx ... pytest tests/test_navigator.py -k will_not_parse -q
E  AssertionError: acting on an element of unknown position reported success
E  assert 0 != 0  +  where 0 = SystemExit(0).code

Bounds guard enumeration — before:

accessibility_audit.py  _parse_bounds  function        :70
accessibility_audit.py  _parse_bounds  regex, signed   :72
navigator.py            _parse_bounds  function        :290
navigator.py            _parse_bounds  regex, unsigned :301
screen_mapper.py        _bounds        function        :268
screen_mapper.py        _bounds        regex, unsigned :270

after: zero outside common/hierarchy.py, exactly one inside it (INC1-09), and test_bounds_are_parsed_in_one_place has no marker.

Correction to the brief's premise, from #12: uiautomator on API 35 clips every rectangle to the display; eight recipes for an off-screen node all came back clipped, so no recorded dump has a negative bound. The signed grammar is kept as precaution, and the comments claiming otherwise are corrected.

L3, C9, C11, C15

  • L3missing_content_description keys off is_interactive and "nothing in this node or its subtree names it". Marker deleted, test otherwise unchanged.
  • C9screen_mapper asks device_utils.get_current_activity; its own second parser is deleted.
  • C11capture_hierarchy(display=...) deleted: no caller, no recording.
  • C15 — see INC1-03.

E1 — the launch waits, and the lane stops mis-diagnosing

--launch passes am start -W, bounded by run_adb's timeout. The e2e fixture polls dumpsys window through the shared parse_focused_activity, and the mapping assertion reports the focused activity — so "the wrong screen was in front" is never again reported as "R11 has regressed". test_agent_task_e2e.py walks every Quick Start command literally, seeding step 3 from step 2's default text output.


Markers deleted (24 cases, 6 declarations)

C1 ×15, C2 ×4, C4 ×2, QS ×1, L3 ×1, bounds guard ×1. No assertion was loosened to make one pass.

Tests changed beyond a marker deletion, and why

Test Change Why
test_agent_loop_from_output.py inventory + _captions three resource-id names to the bare form INC1-08: one name per control; deliberate spec change, marked in the file
test_agent_loop_from_output.py inventory source seeded from the default report, not --json the plan's own step 1; JSON hid C4
test_navigator_scroll.py coordinate test exact row rectangle + a test pinning that literal to the recording INC1-04: "encloses" also accepts the RecyclerView and the window
test_navigator_scroll.py --find-id asserts the bare title INC1-08
test_navigator_scroll.py VISIBLE lookup asserts the control that answers to the name, not the caption node C1 resolves a caption to its control; the old assertion pinned the defect
test_screen_mapper.py preview-cap tests per-bucket cap on the dialer's 17-name Control bucket C4 caps per bucket, not per report
test_accessibility_audit.py missing_resource_id 6 → 7 INC1-05: the scrolling list was invisible to clickable and enabled
test_accessibility_audit.py _parse_bounds unit tests replaced by one on a finding's payload shape the function is deleted (bounds guard to zero)
test_app_launcher.py launch/restart tests fed the recorded am start -W report INC1-02: they proved a launch from empty output
test_hierarchy.py bounds cases derived from the recorded CheckBox INC1-06
test_quick_start_contract.py E2E_BASELINE 5 → 8 entries, --json on two the e2e grew steps; updated in the same commit
test_navigator_scroll.py live helper am start -W + focus poll the lane's own launch race (finding 1 below)
test_navigator_scroll.py live absent-text reads payload["error"] INC1-07's failure contract
test_agent_task_e2e.py marks emulator per test, not per module so the component comparison runs in the mocked check
test_navigator.py (round 3) four tests on tappable ownership the scroll-container defect above
test_device_shell_quoting.py open_url fed the recorded am start -W report VER-01: empty stdout is no longer a launch
test_app_launcher.py (round 4) two deep-link tests VER-01
test_agent_task_e2e.py component test rebuilt from two recordings VER-04

Still open, and named as debt

  • am start -W on an already-foreground activity (Warning: Activity not started..., Status: ok, TotalTime: 0) is described in Inc 0: recordings on two API levels and a real JUnit XML, plus T5 and T14 #12's manifest but not recorded as a file; nothing in this PR depends on distinguishing it, and the launcher treats it as the success it reports.
  • No adb, emulator or -m emulator run at any point from this worktree; the attached phone was never touched. Everything above is the mocked suite against recorded fixtures.

@fluxxion82
fluxxion82 force-pushed the fix/inc-1-loop-closes-from-output branch from 5394a49 to 0fa438f Compare September 3, 2026 08:56
The agent's documented path is screen_mapper -> a name from its report ->
navigator --find-text <name> --tap. Every step of that path was broken in a way
the suite could not see, because the suite exercised a different path.

C2 -- an action needs a target. `--tap`/`--enter-text` with no `--find-*` and no
`--tap-at` matched every enabled node, and the first of those was the
`<hierarchy>` root: `input tap 0 0` was issued, the exit status was 0, and the
message named a real on-screen element that had never been touched. It is now a
usage error, refused before anything reaches the device, and `_flatten_tree` no
longer emits the root at all -- it carries no class, which is the same gate
screen_mapper has always applied.

C1 -- a printed name is findable, and resolves to the control it names.
`_find_in` matched `text + content_desc` only, so `recovered_label` -- the
caption both `--list` and the screen report print -- was not searchable: five of
the seven Compose names and ten of the eleven Settings names came back "Not
found". Where a name did match, it matched the caption, whose centre is outside
the control: "Remember me" tapped (302, 816) while the CheckBox is
[33,754][159,880]. Matching now covers the recovered caption, and a match on a
node that is not itself operable resolves to the control that owns it -- the
nearest interactive ancestor, else the row-adjacent interactive sibling with
overlapping vertical bounds, since a Compose Checkbox's caption is its sibling
and not its ancestor. The owner must answer to the same name, so a tap on a
passive label is never promoted to the scroll container that encloses it.
Resolution applies to a search by name alone: `--find-id`/`--find-type` name a
node, and its owner carries neither that id nor that class.

Resource ids are now carried and printed in full (`pkg:id/name`), which is what
screen_mapper prints, so the two scripts report one name per control; `--find-id`
still matches on any part of it, and `--find-text` matches an id only in full.

C4 -- the default report names what it counts. Every interactive bucket is
printed with its control names, capped per bucket at BUTTONS_PREVIEW; `--hints`
is seeded from the same buckets and emits the navigator invocation to run next.
Quick Start step 2 passes neither `--verbose` nor `--json`, so a report that said
"7 interactive" and named none of them left step 3 with nothing to type.

C7 + C5 -- one bounds grammar and one eligibility rule, in common/hierarchy.py.
`parse_bounds` is signed, so a partially off-screen view parses instead of
collapsing to `(0, 0, 0, 0)`, and returns None on garbage; None refuses the
action with a message naming the remedy, rather than tapping the corner.
`is_interactive` is the single answer to "can an agent operate this", consumed by
navigator, screen_mapper and accessibility_audit.

L3 -- `missing_content_description` keys off `is_interactive` and "nothing in
this node or its subtree names it", not off a class-name whitelist that could
not fire on a Compose screen at all.

C9 -- screen_mapper asks device_utils.get_current_activity for the focused
activity; its own second parser, with a different grammar, is deleted.

C15 -- `--scroll-to-find` is bounded in wall-clock time
(ANDROID_EMU_SCROLL_SEARCH_DEADLINE, default 120s) as well as in scrolls,
`--max-scrolls` is capped at 50 by an argparse type, and the per-scroll progress
line goes to stderr on every run rather than only under `--verbose`.

C11 -- `capture_hierarchy(display=...)` is deleted: no caller ever passed it and
no recording ever proved it worked.

The C1, C2 and C4 xfail markers in tests/test_agent_loop_from_output.py are
deleted, and its case inventory is now seeded from the DEFAULT text report
rather than from `--json`.
A lane failure on PR #8 reported "R11 has regressed" from a test the PR did not
touch: the Compose screen showed 2 interactive elements instead of 7. The cause
was timing, not the mapper. `app_launcher --launch` returned as soon as
`am start` dispatched the intent, and `screen_mapper` ran immediately, so on a
slow hosted emulator the dump captured the launcher or a splash -- and the
assertion named a defect the evidence did not support.

`--launch` now passes `-W`, so `am start` returns when the activity has been
displayed, bounded by run_adb's existing timeout. `Status:`, `Activity:` and
`TotalTime:` are read from its report; a Status other than `ok` is a failure
naming the component, and the success message says which activity came up and
how long it took. `parse_am_start` is written to be inert on output it does not
recognise -- no profile has recorded `am start -W` yet, and a parser that
insists on a shape nobody has measured is the bug class this repo exists to
avoid -- so an unrecognised report leaves the previous behaviour exactly as it
was. Recording it is pending.

The e2e fixture then polls `dumpsys window` (through the shared
`parse_focused_activity`, bounded at 30s) until the fixture app is focused, and
the mapping assertion reports the focused activity so "the wrong screen was in
front" is never again reported as "the mapper went blind".

The same test now walks every Quick Start command literally, in order: the bare
`screen_mapper.py`, then `navigator --find-text <a name that report printed>
--tap`, then `--find-type EditText --enter-text`, then the bare
`accessibility_audit.py`. Step 3's target is parsed out of step 2's DEFAULT text
output rather than out of `--json`, because that is the path SKILL.md documents.
The audit's floor is a verdict without a traceback and an exit of 0 or 1, since
exit 1 is its documented CI gate for "there are criticals".

With that, the Quick-Start-equals-e2e guard is green and its xfail(strict=True)
marker is deleted; the e2e baseline it pins is updated in this commit.
…ed case

The recording unit measured what this skill had only assumed: uiautomator on
API 35 clips every node's rectangle to the display. Eight recipes aimed at
producing an off-screen node -- a half-row swipe, a mid-fling dump, a
half-pulled shade, the task switcher mid-animation -- returned min_left=0,
max_right=1080, max_bottom=2424 every time. There is no recorded dump on this
API level in which a bound is negative.

So the comment saying "a partially off-screen view reports a negative left or
top" is replaced by what is actually known. The signed grammar stays, as
precaution rather than as observation: accessibility_audit's grammar was already
signed, older API levels are not known to clip, and it costs nothing. The half
of C5 that matters is unchanged and is the half that was measured -- an
unparseable value yields None, and None refuses the action instead of becoming
`(0, 0, 0, 0)`, whose centre is a real tappable pixel.

That refusal is now pinned by a test built from the recorded Compose dump with
ONE attribute changed -- the CheckBox's `bounds`, truncated -- rather than from
an invented screen, and its docstring says why a recording cannot supply the
case. Mutating `Element.center` back to a `(0, 0, 0, 0)` fallback turns it red.

Also: `type_text` names the limitation when the text it failed to type is not
ASCII. `input text 'héllo'` throws a NullPointerException and exits 255 with
nothing typed (measured on API 35); the cause is not visible in what adb hands
back, so the message says it.
…s gone

Rebased onto 8bc7c79, which brought the bounds guard (#10) and the repointed
parser tests (#11). This finishes the three findings those two made checkable.

The bounds guard now enumerates ZERO sites outside common/hierarchy.py. The last
one was `accessibility_audit._parse_bounds`, which no longer held a grammar --
it shaped the parsed rectangle into the dict a finding carries -- but a function
of that name is where a fourth grammar grows back, so the shaping is inlined at
the single call site that needs it and the function is deleted. Its two unit
tests are replaced by one that asserts what this file still owns: a finding's
rectangle comes back as named ints, read from a recorded screen.

KNOWN_BOUNDS_SITES goes from six entries to none, with the six recorded in a
comment; `test_bounds_are_parsed_in_one_place` loses its xfail(strict=True). The
detector is still proven by its two synthetic self-tests, which matters more
now that it has no live violation to point at. Its docstring also drops the
claim that a partially scrolled-off view reports a negative bound -- API 35
clips every rectangle to the display -- and keeps the part that is provable from
the grammars themselves.

`parse_bounds` and `is_interactive` get their tests in test_hierarchy.py, beside
the implementation rather than once per consumer: every bounds value on a
recorded screen parses, an unreadable one is None and not the corner, a Compose
`android.view.View` is eligible while the Settings dump's collapsed
`[0,2401][1080,2361]` row is not, the dict shape answers the same as the
element, and `focusable` alone is not enough. The two cases no recording can
supply -- a disabled control and a negative coordinate -- are derived by
changing one attribute of a recorded node, with the reason in the docstring.

L3's `test_unlabelled_clickable_nodes_are_critical` loses its xfail unchanged:
the audit now finds criticals on the Compose screen because the check keys off
`is_interactive` and "no label in this node or its subtree".

screen_mapper's two preview-cap tests are updated for C4's per-bucket cap: the
dialer's 17-name `Control` bucket is what truncates now, and the two-name
`ImageButton` bucket is what proves the cap is per bucket rather than per report.
…e bare

Rebased onto 016bfea (#12's recordings and #13's quoting). app_launcher's
`launch()` keeps both sides of the merge: every argument quoted for the device
shell, and `-W` to wait for the activity.

E1 / INC1-02 -- `am start -W` is no longer parsed from memory. #12 recorded
`am_start_wait_settings` and `am_start_wait_missing`, and both carry a trap the
first version fell into. On success `Activity:` names the component that
actually came up, which for an alias is NOT the one requested
(`.Settings` -> `.homepage.SettingsHomepageActivity`), so a launch can never be
confirmed by comparing those strings: it is confirmed by `Status: ok` and the
resolved component is information. On failure there is no `Status:` line at all
-- `Error type 3` and `Error: Activity class {...} does not exist.` on stdout,
stderr empty, exit 1 -- so "no status" is the failure shape, where the inert
parser had read it as success. `launch()` returns the parsed report as its third
value and `--launch --json` reports the resolved activity, because the caller
that has to check which screen is in front needs that name as data.

INC1-04 -- ownership is structural: nearest interactive ancestor, else the
row-adjacent interactive sibling, next before previous. The owner no longer has
to answer to the caption's name, a requirement that silently un-fixed C1 for
every control named by a resource id -- `search_action_bar` recovers no caption,
so "Search settings" resolved back to the passive TextView inside it. What makes
the structural rule safe is the other end: a name that resolves to nothing
operable is refused with "passive label with no control", never tapped.

INC1-08 -- one name for one control, and it is the BARE resource id.
`com.android.settings:id/search_action_bar` prints as `search_action_bar` in
both scripts; `--find-text` accepts either form, matched whole; `--find-id` is
unchanged. Compose test tags were already bare, so this is the only choice under
which both toolkits name things alike. The loop spec's static inventory and its
independent resolver are updated to the bare form -- deliberately, as a spec
change, with the reason in the file.

INC1-01 -- `parse_bounds` uses `fullmatch`, so `[1,2][3,4]junk` is not a
rectangle; `is_interactive` requires bounds that parse with positive area,
because "operable" cannot mean "we do not know where it is".

INC1-03 -- the scroll deadline starts before the first capture, is re-checked
after the settle, and is passed into every dump (divided by the retry count, so
the retries cannot run past it). The progress line prints when the scroll
happens rather than after the dump that follows it. `ANDROID_EMU_MAX_SCROLLS`
goes through the same validator as `--max-scrolls`, so the env default can no
longer walk past the ceiling the flag enforces.

INC1-05 -- accessibility_audit computes `is_interactive(node)` once and every
check about a control uses it. `clickable and enabled` was a second eligibility
rule: it missed the scrolling list on the Compose screen, which is why the
resource-id finding count moves from six to seven.

INC1-07 -- one failure contract in navigator: `{"error": ...}` on stdout with a
non-zero exit for every failure under `--json`, including the argparse usage
error that C2 added (exit 2, but the payload is printed).

INC1-06 / INC1-09 -- every bounds case in test_hierarchy is now derived from the
recorded CheckBox by changing only its `bounds`, and the bounds guard
enumerates the shared module positively: exactly one grammar inside
common/hierarchy.py, zero outside. "None outside" alone is satisfied by a skill
with no parser at all.

INC1-10 -- the live e2e checks the tap against the hierarchy, not against
navigator's own account of it: the screen is captured the way the skill captures
it, the target's rectangle comes from the independent resolver in
test_agent_loop_from_output, and the coordinates come from navigator's new
`tapped_at`. The focus wait compares against the activity `am start -W`
reported, not the one that was requested.
… can bring back an ANR

Three failures on the emulator lane, all attributable and none of them the
mapper.

`test_live_scroll_search_reaches_an_item_below_the_fold` and its sibling
force-stopped Settings and ran a bare `am start`, then dumped the screen
immediately -- E1 in the one place it had not been fixed. The dump caught the
launcher and navigator reported "nothing on it scrolls, so there is no content
below", which is a confident statement about the wrong screen. The helper now
uses `am start -W` and polls `dumpsys window` through the shared
`parse_focused_activity` until Settings is focused.

`test_live_absent_text_reports_the_search_rather_than_a_bare_miss` read
`payload["message"]` from a failed lookup. Under INC1-07 every failure answers
`{"error": ...}`, so it reads `error` now; the search detail still rides
alongside it, which is what makes "not found" actionable.

The e2e saw two controls on the Compose screen -- "Close app" and "Wait", which
is an ANR dialog, not a blind mapper. The failure message said so, which is what
E1 asked of it: it printed the report and the focused activity instead of
blaming R11. The lane boots from a saved snapshot, so the state the app was left
in at the end of a run comes back with it; the fixture now force-stops the app
before launching, so the run starts from a screen the test is entitled to make
claims about.
The live lane rejected a correctly launched activity. `am start -W` echoes the
class as it was passed -- `com.example.composefixture/.DefaultActivity` -- and
`dumpsys window` prints it fully qualified,
`com.example.composefixture/com.example.composefixture.DefaultActivity`. Both are
the same component; comparing the strings as written called the right screen the
wrong one.

A leading dot is Android's shorthand for "in this package", so both sides are
expanded before they are compared. The alias case the recording found is
untouched by that: an alias resolves to a genuinely different class, and still
does not compare equal to the name that was requested -- which is the whole
reason the wait is against what `am start` REPORTED.

The comparison has a test of its own, holding it to the exact pair the lane
produced and to the alias pair from the recording. The `emulator` mark moves
from the module to the two device-backed tests so that this one runs in the
mocked check, where a mistake in the fixture's only piece of judgement is cheap
to find -- it was not cheap this time.
`capture_hierarchy(compose_device)` in the test body, with `compose_device` not
in the test's signature: what reached adb was pytest's fixture definition
object, and `build_command` reported `'FixtureFunctionDefinition' object has no
attribute 'startswith'`.

Only the device lane could see it -- the test is deselected without `-m
emulator` -- which is the argument for the mark being per test rather than per
module, and for the lane being a required check.
@fluxxion82
fluxxion82 force-pushed the fix/inc-1-loop-closes-from-output branch from db21cad to 8f918fb Compare September 3, 2026 09:18
The trade I flagged at the end of round 2, confirmed by the coordinator reading
`_owning_control`: it returned the first ancestor whose `.interactive` was true,
and `is_interactive` counts `scrollable`. A ScrollView or RecyclerView encloses
nearly the whole screen, so for a passive caption in a list whose rows are not
themselves clickable, the container WAS the first interactive ancestor. The tap
went to its centre -- measured on the recorded Settings screen with one row's
`clickable` cleared: (540, 1572), the middle of the screen -- and the message
still said `Tapped: ... "Battery"`. A success naming a control that was never
touched, which is C2's defect wearing different clothes.

`common/hierarchy.py` now answers two questions instead of conflating them, with
one implementation behind both so the enabled check and the rectangle check
cannot drift apart:

- `is_interactive` (unchanged, still counts `scrollable`) is ELIGIBILITY -- what
  gets enumerated, counted and listed. A scroll container belongs there: an
  agent acts on it by scrolling, and it stays findable by its own name.
- `is_actionable` is "can be operated BY A TAP": clickable, long-clickable or
  checkable. `_owning_control` requires this of an owner, in both the ancestor
  walk and the sibling rule.

A scrollable-only ancestor is passed over rather than accepted, so the search
continues upward, then tries the row, and failing both the existing refusal
fires -- exit non-zero, `{"error": ...}` under `--json`, naming the label.

Four tests, all derived from recordings: the Battery caption resolving to its
clickable row on the unmodified dump; the same caption refused once that row's
`clickable` is cleared (no `input tap` issued, JSON error shape); the container
still enumerable and still findable by its own name; and the Compose
Checkbox/Switch/logo sibling cases restated, since ownership is exactly what
would break them.
… results

VER-01 -- `app_launcher.open_url` ran a bare `am start` and answered "Opened
URL" unless the word "Error" appeared in stdout. That is the E1 shape, and it
was the only instance left in scripts/: an intent that resolves to nothing --
a typo in the scheme, an app that does not declare the filter -- was reported
as a link the agent had followed. It now passes `-W`, reads the verdict through
the same `parse_am_start`, requires `Status: ok`, and reports the `Error:` line
with the remedy when there is one. Its failure test uses the recorded
`am_start_wait_missing`; the docstring says plainly that the recording came
from `-n <component>` rather than `-a VIEW -d <url>` and that what is shared is
the shape both produce -- an `Error:` line and no `Status:` line -- rather than
pretending the capture is of the command under test. The success control uses
the recorded `am start -W` report, and `test_device_shell_quoting`'s open_url
case is repointed at it too, since an empty stdout is no longer a launch.

VER-02 -- the live-scroll helper discarded the results of force-stop and
`am start -W`, then fell out of the focus loop without asserting anything. Both
are checked now, and the loop's `else` raises with the focused activity in the
message: a test that runs navigator against the wrong screen and then blames
navigator for what it finds is worse than one that does not run.

VER-03 -- the e2e's ANR mitigation discarded `--terminate`'s result, so a failed
force-stop permitted exactly the stale-snapshot state the terminate was added to
prevent. It now asserts exit 0 and the JSON success, which is why the call
carries `--json` and the Quick-Start baseline gains that flag.

VER-04 -- the component-normalisation test fed hand-typed spellings to
`_expand_component`. Both halves come from recordings now, read with the
production parsers: `dumpsys_window_focus` carries one activity spelled two ways
(`mCurrentFocus` fully qualified, `mFocusedApp` with the leading dot), which is
the whole phenomenon in two lines of real output; and the alias case reads the
resolved activity from `am_start_wait_settings` against the requested component
taken from that fixture's own manifest command, so neither side is retyped. The
inline Compose pair is gone.
@fluxxion82
fluxxion82 merged commit 701ed7d into main Sep 3, 2026
3 checks passed
fluxxion82 added a commit that referenced this pull request Sep 3, 2026
* release: v0.7.0

The agent loop is now verified from the skill's own printed output: a test
reads what screen_mapper prints and feeds each printed name back into
navigator, so "implemented but unreachable" fails instead of passing.

Merged for this release:

#8  Inc -1: emulator_shutdown --serial could power off an attached phone, and
    two required checks a docs-only PR cannot satisfy.
#9  Inc 0: the red agent-loop spec and the Quick-Start-equals-e2e guard.
#10 Inc 0: three AST shape guards (quoting, emulator console, bounds), the
    23-mode runtime exit-code sweep, and a call-count floor against a blind
    detector.
#11 Inc 0: both fixture-policy ratchets, so the detector sees hoisted
    constants and hierarchy consumers it used to miss.
#12 Inc 0: recordings on two API levels, a real Gradle JUnit XML, the
    am start -W pair, and the display-override tests.
#13 Inc 1: quote every value that crosses the device shell (all 19 sites), and
    stop reporting a failed lookup as an empty one.
#14 Inc 1: the loop closes on the documented path -- every name screen_mapper
    prints is findable and taps inside the control it names, a bare --tap is
    refused, and --launch waits for the activity.
#15 Inc 1/2: exit codes that mean something, a missing SDK tool that says so
    rather than returning an empty list, the IME guard, and one emulator
    console every adb emu call goes through.
#16 Inc 2: the two destructive lifecycle paths stop guessing -- one tri-state
    "which AVD is this serial" probe for every caller -- and the READMEs stop
    deferring.

Version bumped in the four manifests (SKILL.md frontmatter and its Scripts
heading, plugin.json, marketplace.json, pyproject.toml). SKILL.md's
Status & Roadmap now states what v0.7.0 verifies and the gaps that remain,
and stale test prose describing xfail markers that no longer exist is
corrected (no assertions changed).

* docs(SKILL.md): two overclaims in the v0.7.0 status text

Both found in review, and both are the defect class this release exists to
correct: a status line asserting more than the tests establish.

"Every failing mode exits non-zero and prints {"error": ...} under --json"
welded two claims together, and only the first is verified. The runtime sweep
asserts exit status, never the payload shape. Probing all nineteen --json
modes the sweep drives, under its own fake toolchain: every one exits 1, but
only app_launcher, screen_mapper, anr_watcher, app_state_capture, device_list
and the emulator_* lifecycle scripts print a bare {"error": ...}. snapshot,
sms and container print success and error together; privacy_manager prints
{"success": false, "message": ...}; and navigator, log_monitor, crash_triage
and status_bar refuse at device resolution with prose on stderr and no JSON at
all. The bullet now states the exit-status guarantee, says the sweep does not
assert the shape, and names both sides. "The JSON failure shape is not
uniform" is added under Known gaps.

"checked against bounds resolved independently of the hierarchy" inverted what
the test does. The rectangle is resolved FROM the hierarchy, by the test's own
parser reading the same dump -- independently of navigator, not of the
hierarchy. That is the whole point of the check, so the sentence now says
navigator is checked against the screen rather than against itself.

No script changed. 1652 passed, 1 skipped; Black, Ruff and the plugin
manifest validation are clean.

* docs: narrow three more status claims, and guard the Scripts heading

Review round 2. No script changed.

R1. "A missing SDK tool says so" was true of the five scripts that route
through common/sdk_tools.py and false of emulator_create, which still returns
[] at six sites when avdmanager or sdkmanager is missing or fails, so
--list-devices and --list-images print nothing and exit 0. The bullet now names
the five, notes that device_list keeps a missing avdmanager as a warning
because it only decorates AVDs the emulator already listed, and emulator_create
is a Known gap.

R3. "names every control on the screen" was false: the default report slices
each bucket to BUTTONS_PREVIEW (15) and appends "... (N total)". Now "names up
to 15 controls per bucket in the default report (the full inventory is in
--json)".

R4. tests/test_packaging_contract.py only read SKILL.md's frontmatter, and so
do validate-version.yml and release.yml, so the "## Scripts (vX.Y.Z)" heading
could drift a whole release behind with every guard green. It is now a fifth
parametrized case in the agreement test. Mutation, against a cp snapshot rather
than git: heading set to v0.6.0 fails with "SKILL.md `## Scripts` heading says
0.6.0, pyproject.toml says 0.7.0"; restored from the snapshot, 5 passed.

R2. Confirmed the previous round's wording stands, and extended the Known-gaps
line to name both remaining groups as measured: the {"success": false,
"message": ...} scripts (with snapshot, sms and container emitting both keys),
and navigator, log_monitor, crash_triage and status_bar, which refuse an
unreachable device before any JSON is built and print prose on stderr.

R6. SKILL.md said a known defect "is pinned with xfail(strict=True)"; none is.
Stated as the policy, plus "None is pinned in v0.7.0".

R7. accessibility_audit.py:10-12 now says outright that contrast is not
claimed, so the gap keeps the fact and drops the "despite its own description"
rationale.

R5, stale prose, docstrings and comments only, zero assertions touched:
test_exit_code_sweep.py's header said device_list "still answers" an empty
success (fixed in v0.7.0; the shape survives in emulator_create, which the
sweep does not cover) and its table comment said "the six marked ones" (eight
were marked, none is today); test_quick_start_contract.py described C1 and L3
in the present tense; test_shape_guards.py claimed 47 device-shell calls where
the detector reports 45 -- corrected, and labelled an observation rather than a
contract, since only the floors are asserted -- and two enumeration docstrings
still described the sites they held before L7 and C5/C7 emptied them.

1653 passed, 1 skipped; Black, Ruff and the plugin manifest validation clean.
@fluxxion82
fluxxion82 deleted the fix/inc-1-loop-closes-from-output branch September 3, 2026 17:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant