Skip to content

fix(android): preserve labels across snapshot action boundaries - #4180

Open
andrewfree wants to merge 3 commits into
callstack:mainfrom
andrewfree:fix/android-snapshot-action-labels-upstream
Open

andrewfree wants to merge 3 commits into
callstack:mainfrom
andrewfree:fix/android-snapshot-action-labels-upstream

Conversation

@andrewfree

@andrewfree andrewfree commented Nov 3, 2026 •

Copy link
Copy Markdown

Summary

Android helper snapshots can merge nested Settings category labels into their outer scroll container, leaving Network & internet, Connected devices, and Apps without separate labelled action refs.

Stop promotion at independently hittable or editable descendants. The shared Android predicate now honors explicit editable: true, including custom classes. Passive siblings still label their own row. Four files change, including a captured Settings hierarchy and four helper tests. The fixture retains relevant nesting/geometry; the passive-sibling case guards existing behavior. No public API change or linked issue.

Validation

Head f4d1eb797373146eee25be2d3248133892722547, base cf9153e1973bd17ea034423cf092b2f971fb363e.

  • pnpm check:affected --run --head f4d1eb797: PASS, exact commit, 41 runnable checks; 25 skipped lanes are not passes. Broad unit: 1,916 files, 17,101 passed, one skipped. Provider integration passes.
  • The custom-field regression fails at 265d4ec and passes after correction. Helper plus production formatSnapshotText suites pass 12/12.
  • Tests changed through added custom-class coverage and a stronger assertion that the outer scroll container exists. Original typed-field assertions remain; no timeout changed.
  • Current-head live evidence: matching helper 0.21.37-dev, separate normal and interactive category refs, Connected devices navigation, inspected source/T3 PNGs and verified cleanup.
  • Earlier broad timeout causes remain unproven. Vercel authorization is unrelated. Required upstream lanes remain separate from local validation and merge acceptance.

Codex authored under human direction. Independent Codex review covered the preceding source; the parent addressed subsequent automated findings. Human review remains pending.

@vercel

vercel Bot commented Nov 3, 2026

Copy link
Copy Markdown

Someone is attempting to deploy a commit to the Callstack Team on Vercel.

A member of the Team first needs to authorize it.

@thymikee

thymikee commented Nov 3, 2026

Copy link
Copy Markdown
Member

Thanks for the fix. At 265d4ec the change itself looks right, but there is no live proof yet that the final snapshot text keeps the labels.

The change in structural-noise.ts alters which labels the agent-facing Android snapshot text shows. That is capture output, so the PR needs a live run of the changed route. The only live run in the PR body predates the rebase, and the body says it does not qualify this head. The new unit tests call the helper step directly. They do not run the downstream buildMobileSnapshotPresentation, so they cannot show that the final text still has labelled e10, e13 and e16. Please run agent-device snapshot -i and snapshot -i --json on an Android emulator at 265d4ec (or a descendant with this diff), on Settings home. The JSON must show androidSnapshot.backend: android-helper with helperVersion matching package.json. The text must show separate hittable refs labelled "Network & internet", "Connected devices" and "Apps", with the outer ScrollView ref unlabelled and no merged "Google, Services..." label. Please post the commands, device, SHA and output in the PR.

Not blocking: these can be taken or left. The existing Android presentation tests in packages/command-output/src/output/snapshot-android.test.ts go through formatSnapshotText, the production route, so the Settings case would fit there and could assert the printed @e10/@e13/@e16 lines. The second new test passes without the fix, so the PR body could call it a guard. The 754-line settings-home.fixture.json backs 10 production lines, so a compact builder, or a note on where the capture came from, would be lighter.

The only check that is not passing is the Vercel deploy authorization on this fork PR. It runs no code and the diff does not touch website or deploy config, so it looks unrelated. I could not verify the unit-run timeouts the PR body mentions. I also could not run the tests, so my read of old and new test outcomes is by hand. There are no conflicts. Once the live Settings snapshot is posted, this is ready for human review.

@andrewfree

Copy link
Copy Markdown
Author

Verified the final built CLI at 265d4ece9035ae441549c8b3003a61880cd75af5 on Android 16/API 36, AVD orbitalecho_api36 (emulator-5554). Both pnpm build and pnpm build:android completed. The current source CLI used a fresh private daemon/state directory; this was not the installed v0.21.31 CLI.

Commands below ran from that checkout, with the same explicit target/session/state arguments on each device command:

node bin/agent-device.mjs open settings --relaunch --platform android --serial emulator-5554 --session pr4180-live-formatter --state-dir "$STATE" --json
node bin/agent-device.mjs snapshot --platform android --serial emulator-5554 --session pr4180-live-formatter --state-dir "$STATE"
node bin/agent-device.mjs snapshot -i --platform android --serial emulator-5554 --session pr4180-live-formatter --state-dir "$STATE"
node bin/agent-device.mjs snapshot -i --json --platform android --serial emulator-5554 --session pr4180-live-formatter --state-dir "$STATE"

Actual interactive text excerpt:

@e1 [scroll-area] "com.android.settings:id/settings_homepage_container" [scrollable]
@e4 [scroll-area] "com.android.settings:id/main_content_scrollable_container" [scrollable]
@e7 [group] "Google, Services & preferences"
@e10 [group] "Network & internet, Mobile, Wi‑Fi, hotspot"
@e13 [group] "Connected devices, Bluetooth, pairing"
@e16 [group] "Apps, Assistant, recent apps, default apps"

JSON reports androidSnapshot.backend: "android-helper", helperVersion: "0.21.37-dev", helperApiVersion: "2", and installReason: "current"; the helper version matches package.json. JSON refs e10, e13, and e16 are each hittable: true. Outer scroll refs e1 and e4 have no label; their printed fallback names are resource identifiers. Google labels only its own e7 row, with no categories merged into either scroll container. Normal text also preserves separate category refs, which have different IDs because references are scoped to each snapshot mode.

click @e13 opened Connected devices. Source CLI PNGs and a separate T3 Device screenshot were saved and visually inspected. The private session closed, its daemon stopped gracefully with no orphaned claims, and the emulator was shut down. Managed helper compatibility was rechecked at 0.21.31 before shutdown.

The final project-runner verifier passed. Its initial attempt incorrectly equated refs across snapshot modes and supplied target flags to daemon stop; those verifier mistakes were corrected and the failed attempt retained. Product assertions and deadlines were not weakened. The fresh full affected gate also passed all 41 runnable checks at this SHA, with 25 explicitly skipped lanes. The body now calls the passive-sibling test a guard and explains the captured fixture's nesting and geometry.

Verification: OpenAI Codex, parent gpt-6-astra; independent source review gpt-6.1-sol. This supplies the requested live evidence; human review and required upstream checks remain separate from merge acceptance.

@andrewfree
andrewfree marked this pull request as ready for review November 3, 2026 04:27

@cubic-dev-ai cubic-dev-ai Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 3 files

Reply to a comment to ask cubic a question or push back. It learns from your replies.

View guided diff | Re-trigger cubic

@thymikee

thymikee commented Nov 3, 2026

Copy link
Copy Markdown
Member

Thanks, this live run covers what the review asked for at 265d4ec. The helper reports android-helper at the package version, Network & internet, Connected devices and Apps each get their own hittable labelled ref, the outer scroll areas carry no label, and Google labels only its own row. The code looks good, and the only failing check is the Vercel fork authorization, which does not run this code.

@thymikee thymikee added the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Nov 3, 2026
@andrewfree

Copy link
Copy Markdown
Author

Current head f4d1eb797373146eee25be2d3248133892722547 passes the complete local affected gate and a fresh Android 16/API 36 run through the built source CLI.

pnpm check:affected --run --head f4d1eb797 returned exit 0 with exactCommit: true, 41 passed checks and 25 explicitly skipped lanes. The broad unit stage reported 1,916 files, 17,101 passed tests and one skipped test. Skipped lanes and required upstream CI remain separate from this result.

The explicit-editable regression fails on preceding head 265d4ece9035ae441549c8b3003a61880cd75af5 with outer label Profile, Nested value. The correction preserves Profile, Name, and Nested value separately. Focused helper and production formatter suites pass 12/12. Tests changed by adding a custom editable class and requiring the outer Settings container to exist. The original typed-field assertions and deadlines remain.

For live verification, a repository test invoked node bin/agent-device.mjs from this head with an isolated state directory and device session. The final gate rebuilt JavaScript. Native helper inputs were unchanged from the prior successful build, and the actual capture reported:

{"backend":"android-helper","helperVersion":"0.21.37-dev","helperApiVersion":"2","helperTransport":"persistent-session","rootPresent":true,"helperTruncated":false}

After open settings --relaunch --no-test-ime, both snapshot --force-full and snapshot -i --force-full preserved the category labels. Interactive output included:

@e10 [group] "Network & internet, Mobile, Wi‑Fi, hotspot"
@e13 [group] "Connected devices, Bluetooth, pairing"
@e16 [group] "Apps, Assistant, recent apps, default apps"

JSON confirmed separate hittable refs. Scroll containers remained present without stolen category labels. A fresh click @e13 opened Connected devices. I inspected both source PNGs and the actual T3 Device screenshot. All 12 CLI commands exited 0, and the live test passed. Ref values are scoped to each snapshot mode.

The private session closed and daemon stop --clean reported clean: true with no pending releases. The managed helper was restored and verified as 0.21.31. The emulator was shut down, and T3 reported no open devices. This is simulator evidence, not physical-device qualification.

Verified by OpenAI gpt-6-astra through Codex. The preceding source also has independent gpt-6.1-sol review. Vercel's fork authorization failure remains unrelated.

@thymikee

thymikee commented Nov 3, 2026

Copy link
Copy Markdown
Member

The code looks good at f4d1eb7. The two earlier threads are fixed, and I found no new problems in this change. The explicit editable fact is honored in the shared predicate, and the editable:true case is covered by the parameterized test. The e4 test now asserts the node exists before it checks the label.

Both cubic-dev-ai threads are fixed and already resolved: the editable fact in the shared predicate (#4180 (comment)) and the defined-node assertion (#4180 (comment)).

I did not run the tests or a device. I traced the pre-change code by hand to confirm the regression is real. The live Settings run is your report, and those nodes have editable:false, so only the unit test covers the new editable:true branch. That is enough for a one-clause change to a shared predicate.

The only check that is not passing is Vercel fork authorization. It does not run repository code, and this diff does not touch the website, so it looks unrelated. No conflicts are known.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-for-human Valid work that needs human implementation, judgment, or maintainer merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants