fix(flow): elect the topmost element under a recorded tap

[latekvo]

Symptom

QA on Bluesky, physical Android phone:

flow-add-step sometimes records the wrong element and does not warn about it. When another, WIDER element lies underneath a visible clickable element in the view tree (e.g. feed content stretching under the bottom navigation bar), flow-add-step grabs that element underneath instead of the actual touch target. It does so with no warning whatsoever. At flow-execute the step then hits a completely different element - and still gets a PASS.

Root cause

Two facts that are each fine alone and wrong together.

Frames are not clipped by what is drawn over them. A uiautomator dump reports a view at its laid-out bounds regardless of occlusion, and the flow tree keeps that. Measured on a Pixel 3a API 34 emulator (1080x2220): a bottom tab bar is [0,1934][1080,2220] while the scroll container behind it runs to [0,242][1080,1934] and, on a list that reaches the screen bottom, to [0,245][1080,2220]. Feed rows under the bar are really there, at their real bounds.

nodeAtPoint ranked by area. It walked the whole tree and kept the smallest visible frame containing the point:

if (best === undefined || frameArea(node.frame) < frameArea(best.frame)) best = node;

A tab button is 216x220 px = 47 520 px². A feed row's text leaf measured on the same device is 636x50 px = 31 800 px² - wider than the button, yet smaller in area, which is exactly the shape the report describes. So the buried text leaf won, deriveSelector turned it into tap: { text: … }, and replay resolved a feed row on the other side of the screen.

Paint order was ignored entirely.

What changed

nodeAtPoint is now a hit test. Among the visible candidates containing the point, paintsOver decides pairwise, on geometry alone:

• Nesting - a frame contained in the other's wins, whichever order the tree lists them in. This preserves the old intent ("a button over its container"), and on the flattened flow trees it is also what keeps a label leaf beating the testID container flush around it.
• Stacking - frames that nest neither way are separate branches drawn over one another, so the later-listed one is on top and takes the touch.
• Frames equal within WITHIN_EPS put the same point under the finger, so the incumbent stands (unchanged from the old strict <).

The election is a single left-to-right reduction over the candidates in tree order, so "later" means "later in the tree" without reconstructing ancestry - which matters, because nodeAtPoint's only production caller feeds it a flat tree. Every flow adapter (flow-android-tree, flow-ios-tree, flow-chromium-tree, flow-vega-tree) runs flattenHoisting, which emits children: [] leaves under one synthetic root in post-order - descendants precede their container. Verified against a real dump: adaptFullAndroidHierarchyToDescribeResult on the emulator's Clock screen yields flat leaves: 51 | nested? false. A recursive "descend children in reverse" walk would have been a no-op there, and a plain "last containing node wins" would have elected the outermost container every time.

Cross-platform: deliberately not source-aware

Both inputs the rule uses survive every source, which is why there is no per-platform branch:

• Nesting is geometric, so it holds identically for a flattened flow tree and a nested describe tree.
• Sibling order is back-to-front where the recorder actually runs: iOS reads subviews, Android the view-child order uiautomator walks, and both paint earlier siblings first. On Chromium it is CSS paint order for everything the cascade does not reorder.

What is left is genuine ambiguity, not a tie-break away from being solved: a z-index-reordered pair on Chromium, or Vega's undocumented page-source ordering. Those are reported, not guessed - see below. The rationale lives in nodeAtPoint's doc comment.

The missing warning

nodesStackedAtPoint returns the visible candidates that cover the point while nesting neither way with the elected element - i.e. something genuinely overlaps the touch. Containers of the pick are excluded (they are not contenders), and so are equal-framed nodes (same tap point either way). captureTapSelector folds that into the existing warning channel:

Step added to "x" flow — recorded the topmost element under the tap, id="app:id/tab_feeds",
but text="Reply from @alice", id="app:id/feed_row" also cover that point;
confirm the step targets the element you tapped

Warnings now compose (joinWarnings): a capture can be both from a fallback tree source and over a contested point, and dropping either would understate the caveat. Empty for the ordinary tap, so quiet recordings stay quiet.

selectorToFrame - the divergence, resolved in prose

Its ranking is unchanged: exact fields, then smallest frame, then reading order. Changing it would touch every tap/type/assert and is not what the report is about; more importantly the two functions answer different questions. A hit test knows a point and asks which of the elements covering it the finger reaches, so paint order decides. selectorToFrame ranks the elements a selector matches - most of which do not overlap at all - and asks which one the flow author meant, which is a question about fit, not stacking. That is now said explicitly in its docstring, and the three sites that claimed a single shared "smallest frame wins" doctrine no longer cross-reference nodeAtPoint: its own docstring, the comparePick comment, and the test comment on selectorToFrame prefers the smallest of several exact matches.

Existing test expectations I changed

• nodeAtPoint returns the smallest element under a point -> ... returns the element nested inside a container under a point. The assertion is unchanged and still passes: the fixture's overlapping AXGroup geometrically contains the AXButton, so the nesting rule elects the button just as the area rule did. Only the name and comment changed - they asserted a doctrine the code no longer follows, and the name would have misled the next reader about why the button wins.
• selectorToFrame prefers the smallest of several exact matches - comment only. It said "same philosophy as nodeAtPoint", which is now false; it explains the specificity-vs-stacking split instead.

No existing expectation was weakened or deleted.

Verification

Reproduced first, on the reporter's geometry. The new fixture is built from pixels measured with uiautomator dump --compressed on an allocated Pixel 3a API 34 (bar [0,1934][1080,2154], 216x220 px tabs, an edge-to-edge list at [0,245][1080,2220], 154 px rows, a 636x50 px row text leaf), arranged in the flat post-order shape flattenHoisting emits. With src/ stashed to origin/main and the tests kept:

× nodeAtPoint elects the tab button drawn over feed content it overlaps
    AssertionError: expected undefined to be 'com.example.social:id/tab_feeds'
× nodesStackedAtPoint names the elements only paint order separates
× nodesStackedAtPoint reports nothing for a plain nested pick
× records the tab button over unclipped feed content, with an overlap caveat
    AssertionError: expected [ { kind: 'tap', …(1) } ] to deeply equal [ { kind: 'tap', selector: {…} } ]
  Tests  4 failed | 80 passed

expected undefined is the bug: on origin/main the elected node is the feed text leaf, which carries no identifier.

Mutation checks - each rule pinned independently, restoring in between:

mutation	result
stacking rule killed (return candidateInside)	4 failed
nesting rule inverted (return incumbentInside)	5 failed
tie-break churns (return true)	1 failed (keeps the first of two elements sharing one frame)
fallbackSourceWarning dropped from the join	2 failed
overlapWarning dropped from the join	2 failed

The third came back green on the first attempt - the equal-frames tie-break was unpinned, so I added nodeAtPoint keeps the first of two elements sharing one frame before re-running.

Gates, all from the worktree root: npx tsc --build, npx prettier --check ., npx eslint . --max-warnings 0, npm run typecheck:tests -w @argent/tool-server, npm test -w @argent/tool-server -> 295 files, 3077 passed, 1 skipped. No flakes hit this run.

On a real device (allocated emulator-5554, freed after; worktree tool-server driven over POST /tools/:name), against the real Android full-hierarchy flow tree rather than a fixture:

• flow-start-recording + flow-add-step a gesture-tap on the Clock app's bottom nav at (0.5, 0.9207), run twice - once with this branch's build, once with src/ stashed to origin/main and rebuilt:
  • origin/main -> id="…:id/navigation_bar_item_icon_view" (smallest frame, 4 356 px²)
  • branch -> id="…:id/navigation_bar_item_active_indicator_view" (topmost, 6 160 px²)
  • Both are nested inside the tab item, their centres are the same pixel, and both hit the pre-existing re-resolve guard (the id repeats across all five tabs), so the recorded step is coordinates either way. No regression; it shows the descent landing on the topmost drawn view.
• Fetched the untrimmed hierarchy straight from the com.argent.androiddevtools helper over its forwarded socket and replayed both rules offline through the built adaptFullAndroidHierarchyToDescribeResult - the offline run reproduces the two live picks exactly, which is what validates the harness. It also confirms the flat/post-order shape (51 leaves, nested? false).
• flow-add-step on the Add-city FAB recorded tap: { id: …:id/fab_container } with no caveat (correctly - everything else under the point contains the pick), then flow-finish-recording and flow-execute -> ok: true, passed: 1.

What I could not reproduce end to end: the unclipped-overlap shape itself, on a device. I swept the emulator's stock apps (Clock, Contacts, Photos, Maps, Gmail, Play Store, launcher) and the installed RN/Expo test builds with a script looking for exactly this pattern and found none - native Android layouts put the bar beside the content, not over it, so nothing overlaps. The report's app is React Native with a translucent tab bar over the feed. I patched a local Expo fixture app to tabBarStyle: { position: "absolute" } to produce it for real, but its installed dev build no longer matches its JS (ExpoLinearGradient view manager missing, Reanimated 4.1.6 vs 4.1.7) and redboxed; the file was restored byte-identical. So the fixture geometry is measured on the device and composed into the reported layout rather than captured from one screen - stated as such in the test comment.

Deliberately out of scope

The "still gets a PASS" half is not closed. With the right element recorded the replay resolves the right selector, so the primary defect is gone - but the pass criterion itself is untouched. runTap (flow-actions.ts:707-718) dispatches gesture-tap and returns { ok: true }; execLeafStep (flow-run.ts:1074) maps that straight to status: "pass". Selector resolution alone is the pass criterion: no post-tap tree read, no landed-on-target assertion, no before/after fingerprint diff (treeFingerprint's two call sites are both pre-action), and no occlusion notion anywhere on the replay path - isVisible is area-only. A tap onto an occluding overlay, a disabled control, or a container whose centre sits over a different child still reports PASS; the only guard is an author-written assert/await/snapshot step. Closing it means adding a verification step to the replay path plus a warning field to StepReport (the CLI already renders one - argent-cli/src/flow.ts:26, glyph at :200, counted at :215 - but no server code emits it any more). That is a separate change with its own blast radius.

A candidate whose frame fits entirely inside the tab button is indistinguishable, from frames plus a flattened tree, from the tab's own icon or label leaf: in post-order a real descendant also precedes its container, so both "nested and earlier" cases look identical. The nesting rule therefore elects it, and no caveat fires - warning there would fire on every ordinary tab tap. The reported shape is the wider overlapping element, which the stacking rule does fix. Separating the two needs a non-geometric signal (Android's clickable is the obvious candidate, but it is platform-skewed and rows are often clickable too).

No platform was scoped out, but the residual ambiguity is unevenly distributed: iOS and Android give a real back-to-front sibling order, Chromium gives one only absent z-index, and Vega's is undocumented. The caveat is what covers the last two.

Interaction with open PRs

Checked #589, #581, #574 and merged #569:

• feat(flow): CSS combinator scopes for flow selectors #569 (merged, in this branch's base) is the doctrine this had to reconcile with - it is the source of the three "smallest frame wins" cross-references, all updated here.
• feat(flow): support concurrent flow recordings #574 feat/concurrent-flow-recordings edits flow-add-step.ts heavily but not one line inside captureTapSelector. It does rewrite the message template and the warning assignment in execute, which this PR leaves untouched - so the two are same-file, non-overlapping. Whoever rebases second should keep joinWarnings' output flowing into its new Step added to "${params.name}" template.
• feat: support screen-sourced flow selectors #589 codex/flow-screen-selectors touches ui-tree-match.ts only at fetchTree, ~155 lines below nodeAtPoint. Worth flagging semantically: it adds a screen tree source to fetchFlowTree, while captureTapSelector still captures against the app tree - a hit test and a caveat computed on one tree say nothing about the other. The caveat's wording is about the tapped point, not a guarantee about replay, so it stays true; but the pairing deserves a look when feat: support screen-sourced flow selectors #589 lands.
• feat(flow): clear on the type directive #581 feat/flow-type-clear touches none of these files (and targets feat/keyboard-clear, not main). Its waitForFocus reads selectorToFrame, whose ranking this PR deliberately leaves alone.

[latekvo]

This is the wrong approach. We should enforce lack of ambiguity instead.