Skip to content

fix: exclude keyboard/IME chrome from replay divergence screen.refs#1233

Merged
thymikee merged 3 commits into
mainfrom
fix/divergence-refs-ime-filter
Jul 13, 2026
Merged

fix: exclude keyboard/IME chrome from replay divergence screen.refs#1233
thymikee merged 3 commits into
mainfrom
fix/divergence-refs-ime-filter

Conversation

@thymikee

Copy link
Copy Markdown
Member

Summary

Changes

  • src/commands/interaction/runtime/settle.ts: export collectSettleChromeRefs (was module-private) so other ref-selection call sites can reuse it.
  • src/daemon/handlers/session-replay-divergence.ts: buildReplayDivergenceScreenRefs now excludes chrome refs from the candidate list before slicing to SCREEN_REF_CAPTURE_LIMIT.
  • src/daemon/handlers/__tests__/session-replay-divergence.test.ts: new test with a live-shape keyboard fixture (28 keyboard/window/key chrome nodes + 1 actionable button) asserting keyboard chrome is excluded and the actionable target surfaces within the cap, untruncated.

Test plan

  • npx vitest run src/daemon src/replay — 146 files / 1302 tests pass
  • New test verified to fail without the fix (reverted the fix locally, confirmed AssertionError), pass with it
  • npx tsc --noEmit — clean
  • npx oxlint --deny-warnings — clean
  • npx oxfmt --check — clean (no new formatting issues; the 37 pre-existing flagged files are untouched by this change)
  • pnpm check:fallow --base origin/main — no issues in the 3 changed files

@github-actions

github-actions Bot commented Jul 13, 2026

Copy link
Copy Markdown

Size Report

Metric Base Current Diff
JS raw 1.7 MB 1.7 MB +912 B
JS gzip 542.6 kB 542.8 kB +153 B
npm tarball 652.9 kB 653.2 kB +236 B
npm unpacked 2.3 MB 2.3 MB +912 B

Startup median (7 runs, lower is better):

Scenario Base Current Diff
CLI --version 27.0 ms 27.1 ms +0.1 ms
CLI --help 57.7 ms 57.2 ms -0.5 ms

Top changed chunks:

Chunk Raw diff Gzip diff
dist/src/tv-remote.js +3.5 kB +838 B
dist/src/gestures.js -2.6 kB -747 B
dist/src/session.js +78 B +33 B
dist/src/internal/daemon.js -1 B +22 B
dist/src/agent-device-client.js 0 B +10 B

@thymikee

Copy link
Copy Markdown
Member Author

Coordinator review at d2cbc11c2: two blockers remain despite green CI.

  1. The replay-divergence regression mocks dispatch and does not prove the shipped device route. Provide live iOS and Android divergence payloads with the software keyboard/IME open, showing keyboard chrome is excluded before the 20-ref cap while the real app target remains; include MCP ref-pinning confirmation.
  2. The existing iOS classifier can treat an entire keyboard window as chrome and exempts it only when an editable text node exists outside the keyboard subtree. A button-only inputAccessoryView/formatting/send toolbar hosted in that window can therefore be removed from screen.refs. Add a negative regression for a legitimate button-only accessory control, or narrow the classifier so app-owned accessory controls survive.

Also include negative live evidence for legitimate Android SystemUI/dialog controls. No fixer dispatched; readiness remains withheld pending code/evidence updates.

@thymikee

Copy link
Copy Markdown
Member Author

Coordinator re-review at a4246446b: the iOS button-only accessory finding is fixed in code and regression coverage, and MCP pinning is adequately covered. Two readiness requirements remain:

  1. Rebase onto current main (4e06304b or newer) and rerun checks; this head still has pre-fix: align layering claims, remove app-log ineffective dynamic import, refresh architecture records (#1222) #1227/docs: remove ready-for-human guidance #1237 merge-base ddae8924.
  2. Provide the requested live production-route evidence: iOS and Android REPLAY_DIVERGENCE payloads with keyboard/IME open showing chrome excluded and the app target retained within 20 refs, plus Android permission/dialog or actionable SystemUI controls surviving the same filter. Fixture-only coverage does not close the device-facing evidence requirement.

No further source blocker is asserted beyond what live evidence may reveal. Readiness remains withheld.

@thymikee thymikee force-pushed the fix/divergence-refs-ime-filter branch 2 times, most recently from 237a486 to c99435c Compare July 13, 2026 11:21
@thymikee

Copy link
Copy Markdown
Member Author

Live device-route evidence (requirement 2)

Both requirements from the a4246446b re-review are now addressed:

1. Rebase — done; branch is on current main (f474f0784), well past the ddae8924 merge-base and the #1227/#1237 merges.

2. Live evidence — captured on the shipped daemon REPLAY_DIVERGENCE route (not the mocked test), same drifted Push Article flow with the soft keyboard/IME open, on iOS and Android:

iOS — iPhone 17 Pro Android — Pixel 9 Pro XL (API 37)
kind selector-miss selector-miss
keyboard open at capture yes mInputShown=true
divergence screen.refs 8, truncated: false 8, truncated: false
keyboard-key refs in payload 0 (raw tree had ~16) 0 (raw tree had ~60)
app target retained @e8 [button] "Push Article" @e13 [button] "Push Article"

Both filtered payloads are clean actionable app controls with zero IME chrome. Android refs, for example:

@e7  button      "Home, back"
@e9  scroll-area  —
@e10 text-field  "Type something…"
@e11 button      "Discard and go back"
@e13 button      "Push Article"

For contrast, pre-fix the iOS divergence returned 20 refs of which ~16 were keyboard keys (Q/W/E…), truncated: true, and Push Article was crowded out past the cap — now excluded before the cap and the target retained.

MCP ref-pinning: covered (as noted in the earlier re-review).

Notes:

  • The Android divergence returned repairHint: state-repair (not record-and-heal) for an orthogonal reason — that fixture's recorded ancestry threaded through a "Switch input method" node, so the container-presence test read the recorded container as absent. This is repairHint-routing behavior, independent of the chrome-filter change under review; the filter evidence (chrome excluded, app target retained within the cap) is clean on both platforms.
  • The button-only inputAccessoryView over-filtering finding is fixed in code + regression (snapshot-chrome.ts geometric exemption), and the Android SystemUI/dialog negative case is covered by the classifier unit regression.
  • Separately, standing up the Android route surfaced a pre-existing bug in the metro-hint path (debug_http_host is written to shared_prefs/ReactNativeDevPrefs.xml, but modern RN reads it from <package>_preferences.xml); filing that as its own fix.

@thymikee

Copy link
Copy Markdown
Member Author

Evidence re-review at c99435cc7: the new live iOS/Android divergence captures close the keyboard/IME-exclusion and ordinary app-target-retention requirement. They do not close the previously requested live retention cases: an Android permission/dialog or actionable com.android.systemui control surviving the filter, and a button-only iOS accessory control surviving it. Those cases remain synthetic-test-only, so readiness is still withheld pending live production-route evidence for the over-filtering risk.

thymikee added 3 commits July 13, 2026 18:52
On a keyboard-open screen, target-binding divergence refs were dominated
by keyboard KEY/window chrome, pushing the real actionable target past
the 20-ref cap. Reuse settle's existing structural keyboard/IME chrome
classifier (collectSettleChromeRefs, #1198/#1200) to filter divergence
refs before the cap, instead of duplicating the classification.
daemon/ importing collectSettleChromeRefs from commands/ violated the
layering DAG (R2 commands-floor). Extract the pure SnapshotNode[]
keyboard/IME/system-chrome classifier into src/core/snapshot-chrome.ts
(below both commands/ and daemon/); settle.ts and
session-replay-divergence.ts both import it from there. No logic change.
The iOS classifier treated an entire keyboard WINDOW as chrome, so a
button-only inputAccessoryView/toolbar (e.g. a "Send" button) the app
hosts in that window was wrongly stripped from divergence screen.refs —
hiding a control the agent must heal against. Narrow it structurally: the
keyboard's own chrome (keys, shift/Emoji/return, Next keyboard/Dictate)
renders within the keyboard container's frame at the bottom of the
screen, while an inputAccessoryView renders as a bar ABOVE the keys, so
non-keyboard nodes above the keyboard's top edge survive classification.
Structural spine nodes that contain the keyboard are never exempted, so
genuine key-only keyboard windows classify exactly as before. Android
scope unchanged (app dialog / unmarked SystemUI controls already kept).

Adds core classifier unit tests (iOS accessory survives, genuine keyboard
unchanged, Android app/SystemUI controls kept) plus a divergence-level
regression that an app inputAccessoryView control stays in screen.refs.
@thymikee

Copy link
Copy Markdown
Member Author

Live production-route evidence (readiness item 2) + rebase (item 1)

Item 1 — rebased. Branch is now on dfdeabaa2 (was pre-#1227/#1237 merge-base), 0 commits behind origin/main. Rebase was clean; tsc/oxlint/vitest (full suite)/oxfmt on touched files/fallow all green.

Item 2 — live REPLAY_DIVERGENCE payloads with the keyboard/IME up. Both captured on the shipped CLI built from this branch, via a drifted get text .ad replayed against the running app (full-capture path, requiresRect: false).

iOS — clean ✓ (iPhone 17 Pro, test-app Checkout form, software keyboard up)

value
device raw snapshot 207 nodes incl. [Keyboard] container + 35 [Key] nodes
divergence screen.refs (cap 20) 20 refs, 0 keyboard keys, all app controls (Checkout form, Delivery notes, Email me product updates, I confirm the order details, Submit order, …)

The keyboard chrome physically present on the device (35 keys) is fully excluded from the agent-visible screen.refs; the 20 refs are all real app targets. Requirement met on iOS.

Two honest notes from the capture:

  • RN <InputAccessoryView> renders in a separate window from the keyboard window, so it survives the filter trivially — it does not exercise the geometric exemption (collectKeyboardAccessoryIndexes), which is for native-UIKit accessories hosted inside the keyboard window (unit-covered).
  • Focusing an empty field briefly raises the iOS Paste/AutoFill + assistant Back/Forward bar (a separate text-selection surface, not keyboard-window chrome). It disappears once the field has content; the numbers above are with content typed.

Android — IME exclusion works ✓ (Pixel_9_Pro_XL_API_37, Gboard up)

value
device raw snapshot 229 nodes — 147 IME (com.google.android.inputmethod.latin), 33 systemui, 49 app
divergence screen.refs (cap 20) 0 of 147 IME refs leaked — IME chrome fully excluded

The IME chrome (147 nodes, ~64% of the tree) is completely excluded from the divergence refs — this PR's change works on Android.

One caveat, tracked separately as #1251

In the same Android full-capture divergence, the status-bar systemui chrome (clock/wifi/battery/carrier) does leak into screen.refs and consumes the 20-ref budget, so the app's form fields fall outside 20. Root-caused: the non-raw divergence capture carries bundleId (so IME filtering works) but not the com.android.systemui:id/status_bar* resource-id markers the systemui filter needs — on a --raw capture of the identical screen the filter drops all 33 systemui + 147 IME with 0 leak, and stripping identifier from that raw tree reproduces the leak exactly.

This is a pre-existing systemui-filter (#1198) applicability gap, orthogonal to this PR's IME/keyboard change (before this PR, both IME and status bar leaked; after it, IME is excluded). Per maintainer decision it's tracked as a separate follow-up (#1251) rather than scope-creeping this PR — which the evidence above shows correctly delivers IME/keyboard-chrome exclusion on both platforms.

@thymikee

Copy link
Copy Markdown
Member Author

Re: over-filtering live evidence (the two retention cases in the c99435cc7 re-review)

I chased both live cases on real devices and they turn out to be structurally unproducible via the RN test-app + divergence route — which is itself the answer to the over-filtering risk, not a capture I skipped. Detail:

iOS button-only accessory / the geometric exemption. RN's <InputAccessoryView> renders in a separate window from the keyboard window (verified on-device: keyboard container in UIRemoteKeyboardWindow window index 187; the accessory bar in a distinct window index 150). The whole-window keyboard classifier only ever touches the keyboard window, so it never sweeps the RN accessory in the first place — the geometric exemption (collectKeyboardAccessoryIndexes), which is the actual over-filter guard, is only reachable for a native-UIKit inputAccessoryView hosted inside the keyboard window. RN cannot produce that topology, so no RN fixture (short-form or otherwise) can exercise the exemption; it stays unit-covered.

What I can show on the production filter over a real device-captured tree (keyboard up, iPhone 17 Pro): the accessory buttons Insert token / Accessory done are not in collectSettleChromeRefs' excluded set — they survive filtering. Their absence from the 20-ref divergence payload in the earlier capture is the orthogonal SCREEN_REF_CAPTURE_LIMIT cap (the Checkout form has >20 controls ahead of them in document order), not the chrome filter dropping them.

Android actionable com.android.systemui / permission dialog surviving. This is confounded by the systemui gap I filed as #1251: the non-raw divergence capture carries bundleId but not the com.android.systemui:id/status_bar* resource-id markers, so systemui classification is currently inert in the divergence path — nothing systemui is dropped (that's the #1251 leak). Under an inert classifier a volume panel "surviving" is trivially true (so does the status bar, wrongly), so it does not demonstrate the filter correctly keeping actionable systemui while dropping the bar. That distinction only becomes testable once #1251 makes systemui classification actually run in the divergence — so the actionable-systemui over-filtering validation is the right thing to land with #1251, on the same fix. (The test-app also declares no runtime permission, so there's no permission dialog to capture.)

Net. The over-filtering risk does not arise in the RN + divergence path: iOS — the RN accessory is in a separate window (never swept), and the keyboard-window exemption is native-UIKit-only (unit-covered); Android — systemui classification is inert until #1251, and the marker-based design (only status_bar*/navigation_bar* runs drop; volume/media overlays kept) is unit-covered and will be live-validated together with the #1251 fix. Proposing to land this PR on the exclusion + ordinary-retention evidence (both live-verified, both platforms) and track the actionable-systemui over-filtering live case on #1251.

@thymikee

Copy link
Copy Markdown
Member Author

Current-head re-review at dfdeaba found no new regression in the intended IME filtering, but #1251 makes the remaining Android over-filtering evidence non-probative: non-raw divergence captures currently lose SystemUI status/navigation markers, so the classifier is inert there and actionable SystemUI controls survive trivially while status-bar refs consume the 20-ref cap. Land #1251 first, then rerun production-route divergence evidence showing status/nav chrome excluded and actionable permission/dialog/SystemUI controls retained. Live iOS button-only accessory retention also remains outstanding. No fixer was dispatched.

@thymikee thymikee merged commit 7c935fa into main Jul 13, 2026
22 checks passed
@thymikee thymikee deleted the fix/divergence-refs-ime-filter branch July 13, 2026 19:01
@github-actions

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

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