Skip to content

Commit bfbb264

Browse files
committed
fix: drop only status/nav-bar systemui runs from settle output
Second review round on PR #1200: com.android.systemui hosts actionable overlays (volume panel, media/output pickers, some system prompts), so package-level dropping reintroduced dialog blindness for that class. Key the drop on window-runs instead, live-verified on the emulator: the status-bar window carries com.android.systemui:id/status_bar* markers throughout while the VolumeDialog window carries only volume_dialog* ids. A systemui run (contiguous same-package parent chain) drops only when a member matches the status_bar*/navigation_bar* marker prefixes; every other systemui surface is kept with actionable refs. Run grouping never crosses packages, preserving the per-node cross-window guarantee. Also corrects the tracking-issue references (#1178 was an unrelated registry issue; the Android settle-noise issue is #1198) and adds a real-capture VolumeDialog regression test: status bar drops, volume dialog survives in both diff and tail.
1 parent 69d2e46 commit bfbb264

2 files changed

Lines changed: 236 additions & 47 deletions

File tree

‎src/commands/interaction/runtime/settle.test.ts‎

Lines changed: 158 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -687,7 +687,7 @@ test('a keyboard window hosting an app composer field is never window-classified
687687
assert.ok(!diff.lines.some((line) => /\[key\]|shift/.test(line.text)));
688688
});
689689

690-
// #1178: Android settle-diff scope. Shapes below are trimmed from a real
690+
// #1198: Android settle-diff scope. Shapes below are trimmed from a real
691691
// `snapshot --raw --json` capture against the react-navigation playground on
692692
// an Android emulator (July 2026, stock-UIAutomator fallback path) — status
693693
// bar and IME each render as their own whole top-level root, one node per
@@ -707,29 +707,40 @@ function androidStatusBarNodes(startIndex: number, clockLabel = '12:23') {
707707
rect: { x: 0, y: 0, width: 1344, height: 159 },
708708
},
709709
{
710+
// The marker every real status-bar capture carries; the window-run
711+
// drops as a whole because this member matches the status_bar* prefix.
710712
index: root + 1,
711-
depth: 10,
713+
depth: 1,
712714
parentIndex: root,
715+
type: 'android.widget.FrameLayout',
716+
identifier: 'com.android.systemui:id/status_bar_container',
717+
bundleId: ANDROID_SYSTEM_UI_BUNDLE_ID,
718+
rect: { x: 0, y: 0, width: 1344, height: 159 },
719+
},
720+
{
721+
index: root + 2,
722+
depth: 10,
723+
parentIndex: root + 1,
713724
type: 'android.widget.TextView',
714725
identifier: 'com.android.systemui:id/clock',
715726
label: clockLabel,
716727
bundleId: ANDROID_SYSTEM_UI_BUNDLE_ID,
717728
rect: { x: 40, y: 40, width: 80, height: 40 },
718729
},
719730
{
720-
index: root + 2,
731+
index: root + 3,
721732
depth: 11,
722-
parentIndex: root,
733+
parentIndex: root + 1,
723734
type: 'android.widget.FrameLayout',
724735
identifier: 'com.android.systemui:id/mobile_combo',
725736
label: 'T-Mobile, one bar.',
726737
bundleId: ANDROID_SYSTEM_UI_BUNDLE_ID,
727738
rect: { x: 900, y: 40, width: 60, height: 40 },
728739
},
729740
{
730-
index: root + 3,
741+
index: root + 4,
731742
depth: 11,
732-
parentIndex: root,
743+
parentIndex: root + 1,
733744
type: 'android.view.View',
734745
label: 'Battery 100 percent.',
735746
bundleId: ANDROID_SYSTEM_UI_BUNDLE_ID,
@@ -738,6 +749,65 @@ function androidStatusBarNodes(startIndex: number, clockLabel = '12:23') {
738749
];
739750
}
740751

752+
// Real systemui VolumeDialog window captured live alongside the status bar
753+
// (volume keyevent held open + `snapshot --raw --json`, android-helper
754+
// multi-window backend): same package as the status bar but only
755+
// volume_dialog* ids — no status/nav-bar marker in the run, so it must
756+
// survive with actionable refs.
757+
function androidVolumeDialogNodes(startIndex: number) {
758+
const root = startIndex;
759+
return [
760+
{
761+
index: root,
762+
depth: 0,
763+
type: 'android.widget.FrameLayout',
764+
bundleId: ANDROID_SYSTEM_UI_BUNDLE_ID,
765+
rect: { x: 816, y: 159, width: 528, height: 2833 },
766+
},
767+
{
768+
index: root + 1,
769+
depth: 2,
770+
parentIndex: root,
771+
type: 'android.widget.FrameLayout',
772+
identifier: 'com.android.systemui:id/volume_dialog',
773+
bundleId: ANDROID_SYSTEM_UI_BUNDLE_ID,
774+
rect: { x: 1100, y: 1000, width: 220, height: 1100 },
775+
},
776+
{
777+
index: root + 2,
778+
depth: 3,
779+
parentIndex: root + 1,
780+
type: 'android.widget.ImageButton',
781+
identifier: 'com.android.systemui:id/volume_dialog_settings',
782+
label: 'Sound settings',
783+
bundleId: ANDROID_SYSTEM_UI_BUNDLE_ID,
784+
rect: { x: 1140, y: 1950, width: 140, height: 140 },
785+
hittable: true,
786+
},
787+
{
788+
index: root + 3,
789+
depth: 3,
790+
parentIndex: root + 1,
791+
type: 'android.widget.SeekBar',
792+
identifier: 'com.android.systemui:id/volume_dialog_slider',
793+
label: 'Media',
794+
bundleId: ANDROID_SYSTEM_UI_BUNDLE_ID,
795+
rect: { x: 1140, y: 1100, width: 140, height: 800 },
796+
hittable: true,
797+
},
798+
{
799+
index: root + 4,
800+
depth: 3,
801+
parentIndex: root + 1,
802+
type: 'android.widget.ImageButton',
803+
label: 'Ring, tap to change ringer mode',
804+
bundleId: ANDROID_SYSTEM_UI_BUNDLE_ID,
805+
rect: { x: 1140, y: 950, width: 140, height: 140 },
806+
hittable: true,
807+
},
808+
];
809+
}
810+
741811
function androidImeNodes(startIndex: number) {
742812
const root = startIndex;
743813
return [
@@ -847,7 +917,7 @@ function androidAppNodes() {
847917
];
848918
}
849919

850-
test('Android status bar and IME chrome never spend the settled diff budget (#1178)', async () => {
920+
test('Android status bar and IME chrome never spend the settled diff budget (#1198)', async () => {
851921
// The status bar clock ticks and the keyboard is summoned between captures
852922
// — exactly the noise the July 2026 Android settle benchmark flagged. The
853923
// systemui status bar disappears entirely (both sides) and the IME
@@ -886,7 +956,7 @@ test('Android status bar and IME chrome never spend the settled diff budget (#11
886956
assert.equal(diff.summary.removals, 0);
887957
});
888958

889-
test('Android IME-only settle changes do not suppress the settle tail, and chrome never populates it (#1178)', async () => {
959+
test('Android IME-only settle changes do not suppress the settle tail, and chrome never populates it (#1198)', async () => {
890960
// Mirrors the iOS "keyboard-only changes... do not suppress" case above:
891961
// the only tree change is the keyboard summoning (plus unrelated status
892962
// bar churn), so the tail must still surface the real screen buttons —
@@ -922,7 +992,7 @@ test('Android IME-only settle changes do not suppress the settle tail, and chrom
922992
);
923993
});
924994

925-
test('an Android IME-looking root hosting app-owned content is never collapsed as chrome (#1178)', async () => {
995+
test('an Android IME-looking root hosting app-owned content is never collapsed as chrome (#1198)', async () => {
926996
// Per-node classification: the app-owned "Send" under an IME root survives;
927997
// the sibling "q" key is dropped for its own foreign package, not by
928998
// subtree collapse. Indexes start above androidAppNodes()'s 0-2.
@@ -984,7 +1054,7 @@ test('an Android IME-looking root hosting app-owned content is never collapsed a
9841054
assert.ok(!diff.lines.some((line) => /"q"/.test(line.text)));
9851055
});
9861056

987-
test('a real capture with a cross-window parentIndex artifact never loses app content to IME collapse (#1178)', () => {
1057+
test('a real capture with a cross-window parentIndex artifact never loses app content to IME collapse (#1198)', () => {
9881058
// Real `snapshot -i --json` capture (RN playground, emulator, Gboard up):
9891059
// interactive-only pruning reparented the app's "Tab View, back" (index 6)
9901060
// onto IME toolbar button index 5 — a subtree walk from the IME root would
@@ -1153,7 +1223,7 @@ test('a real capture with a cross-window parentIndex artifact never loses app co
11531223

11541224
// Real Android Sharesheet (`am start -a android.intent.action.SEND`,
11551225
// ResolverActivity) captured live via `snapshot --raw --json` on the same
1156-
// emulator/session as the other #1178 fixtures: 41 nodes, every one owned by
1226+
// emulator/session as the other #1198 fixtures: 41 nodes, every one owned by
11571227
// package `android` — the shape shared by permission prompts, the package
11581228
// installer, and chooser/resolver sheets. Indexes offset by +100 to stay
11591229
// clear of androidAppNodes().
@@ -1527,7 +1597,7 @@ function androidSharesheetNodes() {
15271597
];
15281598
}
15291599

1530-
test('a system dialog (real Sharesheet capture) stays fully visible in the settled diff (#1178)', async () => {
1600+
test('a system dialog (real Sharesheet capture) stays fully visible in the settled diff (#1198)', async () => {
15311601
// PR #1200 review blocker: the initial fix dropped EVERY foreign package,
15321602
// so a blocking system dialog appearing mid-action produced an empty diff
15331603
// and a tail pointing at now-covered app buttons. Keep-unknown-foreign is
@@ -1628,7 +1698,7 @@ function androidSharesheetInteractiveNodes() {
16281698
];
16291699
}
16301700

1631-
test("a system dialog's buttons are settle-tail candidates, never chrome (#1178)", () => {
1701+
test("a system dialog's buttons are settle-tail candidates, never chrome (#1198)", () => {
16321702
const settledNodes = makeSnapshotState([
16331703
...androidAppNodes(),
16341704
...androidSharesheetInteractiveNodes(),
@@ -2033,7 +2103,7 @@ test('buildSettleTailEntries drops the keyboard container and its chrome descend
20332103
assert.deepEqual(result.tail, [{ ref: 'e1', role: 'button', label: 'Send' }]);
20342104
});
20352105

2036-
test('buildSettleTailEntries drops Android IME chrome and persistent system chrome (#1178)', () => {
2106+
test('buildSettleTailEntries drops Android IME chrome and status-bar chrome (#1198)', () => {
20372107
const settledNodes = makeSnapshotState([
20382108
{
20392109
index: 0,
@@ -2045,24 +2115,33 @@ test('buildSettleTailEntries drops Android IME chrome and persistent system chro
20452115
hittable: true,
20462116
},
20472117
{
2118+
// Status-bar marker: this systemui run drops whole.
20482119
index: 1,
20492120
depth: 0,
2121+
type: 'android.widget.FrameLayout',
2122+
identifier: 'com.android.systemui:id/status_bar',
2123+
bundleId: 'com.android.systemui',
2124+
},
2125+
{
2126+
index: 2,
2127+
depth: 1,
2128+
parentIndex: 1,
20502129
type: 'android.widget.TextView',
20512130
identifier: 'com.android.systemui:id/clock',
20522131
label: '12:23',
20532132
bundleId: 'com.android.systemui',
20542133
},
20552134
{
2056-
index: 2,
2135+
index: 3,
20572136
depth: 0,
20582137
type: 'android.widget.FrameLayout',
20592138
bundleId: 'com.google.android.inputmethod.latin',
20602139
hittable: true,
20612140
},
20622141
{
2063-
index: 3,
2142+
index: 4,
20642143
depth: 1,
2065-
parentIndex: 2,
2144+
parentIndex: 3,
20662145
type: 'android.widget.FrameLayout',
20672146
label: 'Delete',
20682147
bundleId: 'com.google.android.inputmethod.latin',
@@ -2075,10 +2154,10 @@ test('buildSettleTailEntries drops Android IME chrome and persistent system chro
20752154
assert.deepEqual(result.tail, [{ ref: 'e1', role: 'button', label: 'Send' }]);
20762155
});
20772156

2078-
test('buildSettleTailEntries keeps unknown-foreign packages and drops only systemui, with or without appBundleId (#1178)', () => {
2157+
test('buildSettleTailEntries keeps unknown-foreign packages and drops only marked systemui chrome, with or without appBundleId (#1198)', () => {
20792158
// Keep-unknown-foreign default: a system dialog's buttons (package
2080-
// `android`) stay tail candidates; only the persistent status-bar package
2081-
// is dropped, and that does not depend on knowing the session's app.
2159+
// `android`) stay tail candidates; only the marked status/nav-bar
2160+
// window-run drops, and that does not depend on knowing the session's app.
20822161
const settledNodes = makeSnapshotState([
20832162
{
20842163
index: 0,
@@ -2092,14 +2171,23 @@ test('buildSettleTailEntries keeps unknown-foreign packages and drops only syste
20922171
{
20932172
index: 1,
20942173
depth: 0,
2174+
type: 'android.widget.FrameLayout',
2175+
identifier: 'com.android.systemui:id/status_bar_container',
2176+
bundleId: 'com.android.systemui',
2177+
rect: { x: 0, y: 0, width: 1344, height: 159 },
2178+
},
2179+
{
2180+
index: 2,
2181+
depth: 1,
2182+
parentIndex: 1,
20952183
type: 'android.widget.TextView',
20962184
identifier: 'com.android.systemui:id/clock',
20972185
label: '12:23',
20982186
bundleId: 'com.android.systemui',
20992187
rect: { x: 40, y: 40, width: 80, height: 40 },
21002188
},
21012189
{
2102-
index: 2,
2190+
index: 3,
21032191
depth: 0,
21042192
type: 'android.widget.Button',
21052193
label: 'Just once',
@@ -2118,3 +2206,52 @@ test('buildSettleTailEntries keeps unknown-foreign packages and drops only syste
21182206
);
21192207
}
21202208
});
2209+
2210+
test('a systemui volume dialog survives the settled diff and tail while the status bar drops (#1198)', async () => {
2211+
// PR #1200 second review round: systemui is not all chrome — it hosts
2212+
// actionable overlays (volume panel, media pickers). Only window-runs
2213+
// carrying a status/nav-bar marker drop; the VolumeDialog run (real
2214+
// capture, same package, volume_dialog* ids only) must stay actionable.
2215+
const before = makeSnapshotState([...androidAppNodes(), ...androidStatusBarNodes(10, '12:23')]);
2216+
const settledTree = makeSnapshotState([
2217+
...androidAppNodes(),
2218+
...androidStatusBarNodes(10, '12:24'),
2219+
...androidVolumeDialogNodes(30),
2220+
]);
2221+
let captures = 0;
2222+
const device = createSettleDevice({
2223+
stored: before,
2224+
appBundleId: ANDROID_APP_BUNDLE_ID,
2225+
captureSnapshot: () => {
2226+
captures += 1;
2227+
return { snapshot: captures === 1 ? before : settledTree };
2228+
},
2229+
});
2230+
2231+
const result = await device.interactions.press(selector('label="Discard and go back"'), {
2232+
session: 'default',
2233+
settle: {},
2234+
});
2235+
2236+
const diff = result.settle?.diff;
2237+
assert.ok(diff);
2238+
const added = diff.lines.filter((line) => line.kind === 'added');
2239+
const texts = added.map((line) => line.text).join('\n');
2240+
// The volume dialog registers as change, with actionable refs.
2241+
assert.match(texts, /Sound settings/);
2242+
assert.match(texts, /Media/);
2243+
assert.ok(
2244+
added.some((line) => /Sound settings/.test(line.text) && line.ref),
2245+
'volume dialog controls must carry refs',
2246+
);
2247+
// Status-bar churn (clock tick) still never appears.
2248+
assert.ok(!/12:2[34]/.test(diff.lines.map((line) => line.text).join('\n')));
2249+
2250+
// Tail-candidate check on the same shape: dialog controls are candidates,
2251+
// status-bar nodes are not.
2252+
const labels = (
2253+
buildSettleTailEntries(settledTree.nodes, new Set(), ANDROID_APP_BUNDLE_ID).tail ?? []
2254+
).map((entry) => entry.label);
2255+
assert.ok(labels.includes('Sound settings'), `tail must list dialog controls, got: ${labels}`);
2256+
assert.ok(!labels.includes('12:24'), 'status bar must stay excluded');
2257+
});

0 commit comments

Comments
 (0)