Repository navigation
fix(snapshot): clip oversized system alert content - #3857
Conversation
|
Live verification and regression evidence:
iOS changed routeDevice: iPhone 17e, iOS 27.0, xcrun simctl ui F25C7515-D67E-4838-BEAC-0AD08DB056EC content_size accessibility-extra-extra-extra-large
xcrun simctl privacy F25C7515-D67E-4838-BEAC-0AD08DB056EC reset all com.callstack.clip3801
agent-device open com.callstack.clip3801 --relaunch
agent-device alert wait 10000
agent-device snapshot -i --jsonThe last command fails on the named base with The changed source runner/daemon returns a regular snapshot successfully. agent-device find text 'Ask App Not to Track' click
agent-device wait text 'Tracking status: 2'Both succeed: Android changed routeDevice: Source capture uses All app-driving commands used device_open's exact launcher/config/session/target flags, with an endpoint override to the isolated source daemon. The installed 0.21.25 CLI predates the new public field, so a two-step batch (regular/raw snapshot) preserved the source daemon JSON for metadata inspection. Final CLI JSON/digest serialization is covered by the regression below. Tests and review
Cleanup: both exact owned sessions closed; isolated source/base daemons stopped; global claims empty, hiddenStaleClaims=0. Both panels detached without shutdown. The disposable base checkout was removed. No tags pushed, no merge or release performed. Exact-head gate receipt summary (all runnable checks passed; 1,364 files / 12,199 tests): {
"version": 1,
"status": "passed",
"exitCode": 0,
"refs": {
"base": "origin/main",
"head": "a3893a57bc4165532961bbe25c378285e050240a",
"resolvedBase": "f8c60ca188f62e110bdc1d3e6225b134960886c0",
"resolvedHead": "a3893a57bc4165532961bbe25c378285e050240a",
"mergeBase": "f8c60ca188f62e110bdc1d3e6225b134960886c0"
},
"checkoutChanged": false,
"exactCommit": true,
"results": [
{
"id": "format",
"status": "passed",
"exitCode": 0
},
{
"id": "lint",
"status": "passed",
"exitCode": 0
},
{
"id": "typecheck",
"status": "passed",
"exitCode": 0
},
{
"id": "layering",
"status": "passed",
"exitCode": 0
},
{
"id": "fallow",
"status": "passed",
"exitCode": 0
},
{
"id": "build",
"status": "passed",
"exitCode": 0
},
{
"id": "ios-snapshot-differential",
"status": "skipped",
"reason": "GitHub-authoritative (jobs: Apple / Smoke Tests)"
},
{
"id": "vitest-related",
"status": "passed",
"exitCode": 0
},
{
"id": "unit",
"status": "deduplicated",
"reason": "covered by related tests"
},
{
"id": "coverage",
"status": "skipped",
"reason": "GitHub-authoritative (jobs: Coverage)"
},
{
"id": "provider-integration",
"status": "deduplicated",
"reason": "covered by related tests"
},
{
"id": "swift-runner-ios",
"status": "skipped",
"reason": "GitHub-authoritative (jobs: Apple / Smoke Tests, Conformance Differential / iOS Conformance Differential, XCTest Nightly / iOS Runner Full XCTest Suite)"
},
{
"id": "swift-runner-macos",
"status": "skipped",
"reason": "GitHub-authoritative (jobs: Apple / Smoke Tests)"
},
{
"id": "web-smoke",
"status": "skipped",
"reason": "GitHub-authoritative (jobs: Apple / Smoke Tests, Integration Tests, Lint, Typecheck & Package)"
},
{
"id": "docs-links",
"status": "passed",
"exitCode": 0
},
{
"id": "docs-prose",
"status": "passed",
"exitCode": 0
},
{
"id": "xctest-selection",
"status": "passed",
"exitCode": 0
},
{
"id": "packaged-runner-swift",
"status": "passed",
"exitCode": 0
},
{
"id": "replay-ios",
"status": "skipped",
"reason": "parked, workflow_dispatch only (Replay Manual / iOS Replay Suite)"
},
{
"id": "replay-ios-device",
"status": "skipped",
"reason": "parked, workflow_dispatch only (Replay Manual / iOS Replay Suite)"
},
{
"id": "replay-macos",
"status": "skipped",
"reason": "GitHub-authoritative (jobs: Apple / Smoke Tests)"
},
{
"id": "replay-linux",
"status": "skipped",
"reason": "GitHub-authoritative (jobs: Linux / Smoke Tests)"
},
{
"id": "linux-command-evidence",
"status": "skipped",
"reason": "GitHub-authoritative (jobs: Linux / Smoke Tests)"
},
{
"id": "replay-android",
"status": "skipped",
"reason": "parked, workflow_dispatch only (Replay Manual / Android Full Emulator Suite)"
}
]
} |
|
Size Report
Startup median (7 runs, lower is better):
|
|
Independent smaller-design assessment for A separate Sol reviewer examined the complete range in an isolated checkout and found no actionable smaller-design findings. Exact diff scope accounting:
The reviewer compared these alternatives:
No new dependencies, production modules, or competing presentation owners were introduced. The production growth is justified by the required cross-runtime geometry/disclosure contract. Exact per-file attribution of the package-byte delta was not measured. This was source/design review only; no source changes or additional test/build/device runs were made. Existing validation remains attributed to its recorded SHAs. |
|
Thanks for the PR. At a3893a5 the clipping fix looks right, but the system-alert path now reports a backend it does not use. CI is green: 19 checks, none failing. Two things still need to happen before merge: fix the modal verdict below, and answer the question about the note population. In RunnerTests+SystemModal.swift:98, Could we keep the modal probe routed through the shared presenters, send the notes as that top-level key, and decide once, in the conformance table, whether ordinary viewport clipping produces a note? If it does not, the counters shrink to overflow past an ancestor clip and most snapshots never carry the field. That needs a maintainer decision first: all clipping, or only overflow past a container clip. ADR 0004 should then be amended to match. Not blocking, and you can take or leave these: the counter in SnapshotVisibilityFold.swift:192 also counts ordinary viewport and scroll-anchor clipping, so the maintainers should confirm that is the intended note population (or count only overflow past a non-viewport ancestor clip) in the conformance table, so all three runtimes follow one rule; the help text in src/commands/capture/snapshot.ts:79 and snapshots.md:246 say only regular iOS snapshots clip, while Android regular snapshots now also omit framed nodes with no visible area (ui-hierarchy-builder.ts:290), so both should name both families and the CHANGELOG should get a line for the Android omission; the new I read the code and did not run the tests or a device. I judged regression validity by reading the pre-change code. The live iOS evidence in the PR covers iOS 27.0 only, and the iOS 17.5 case with the 5.5 pt overflow was not re-run. I did not trace how far the false modal verdict reaches in stability and corroboration decisions, so that impact is inferred from the consumers I read. I also did not check whether action-sheet modals place actions outside the resolved root frame, since using the modal root as the derived viewport would drop them. I did not check whether the Android Appium backend shares |
|
Addressed the runner-wire blocker in the maintainer review. Tested candidate:
Fresh independent Sol review round 1 found a real P2: removing the fabricated verdict exposed the Simulator's empty-scope admission guard. The follow-up accepts complete unscoped modal evidence without a verdict; sparse verdicts, absent/empty/truncated evidence, missing modal evidence and unscoped empty results remain rejected. Shared presentation validation still runs. Fresh independent Sol round 2 traced the new admission, actual Swift modal wire, genuine verdict stamping, raw output, legacy detectors, comparison/corroboration, transport/state/client/CLI/digest and bounded parser: no actionable findings. Both reviews were static, read-only reviews and did not independently run tests/devices. Regression evidence
Live changed routeDevice: iPhone 17e / iOS 27.0 / All app commands used device_open's exact launcher, UDID, config and session
These runs cover iOS 27.0 ATT only; iOS 17.5 and action-sheet layouts were not rerun. Android behavior is unchanged by this follow-up and retains its earlier attributed evidence. The first setup attempt failed before capture when a gate rebuild replaced generated bundle chunks used by an already-started daemon. Subsequent setup detected macOS cache metadata from using the both-target build command with one explicit override; the live run above used a stable completed TypeScript build and a separate explicit iOS build. Neither failed setup is counted as product capture evidence. Cleanup: own session closed, own source daemons exited, owned exec completion confirmed, iPhone panel detached without shutdown, no owned claims, hiddenStaleClaims=0. The other worker's live Android claim was preserved. Source CLI Clipping-note population remains a pending maintainer decision. This follow-up preserves the existing viewport/scroll/ancestor clipping population. It does not claim to settle the all-clipping versus ancestor-overflow question or its dependent ADR/conformance update. Exact-commit receipt
{
"version": 1,
"status": "passed",
"exitCode": 0,
"refs": {
"base": "origin/main",
"head": "c8ec2a4b914140e93971c717cb82bef97cffdad9",
"resolvedBase": "bd3619b05153542c5eb94b3e8a3079608cd96f41",
"resolvedHead": "c8ec2a4b914140e93971c717cb82bef97cffdad9",
"mergeBase": "f8c60ca188f62e110bdc1d3e6225b134960886c0"
},
"checkoutChanged": false,
"exactCommit": true,
"results": [
{
"id": "format",
"status": "passed",
"exitCode": 0
},
{
"id": "lint",
"status": "passed",
"exitCode": 0
},
{
"id": "typecheck",
"status": "passed",
"exitCode": 0
},
{
"id": "layering",
"status": "passed",
"exitCode": 0
},
{
"id": "fallow",
"status": "passed",
"exitCode": 0
},
{
"id": "build",
"status": "passed",
"exitCode": 0
},
{
"id": "ios-snapshot-differential",
"status": "skipped",
"reason": "GitHub-authoritative (jobs: Apple / Smoke Tests)"
},
{
"id": "vitest-related",
"status": "passed",
"exitCode": 0
},
{
"id": "unit",
"status": "deduplicated",
"reason": "covered by related tests"
},
{
"id": "coverage",
"status": "skipped",
"reason": "GitHub-authoritative (jobs: Coverage)"
},
{
"id": "provider-integration",
"status": "deduplicated",
"reason": "covered by related tests"
},
{
"id": "swift-runner-ios",
"status": "skipped",
"reason": "GitHub-authoritative (jobs: Apple / Smoke Tests, Conformance Differential / iOS Conformance Differential, XCTest Nightly / iOS Runner Full XCTest Suite)"
},
{
"id": "swift-runner-macos",
"status": "skipped",
"reason": "GitHub-authoritative (jobs: Apple / Smoke Tests)"
},
{
"id": "web-smoke",
"status": "skipped",
"reason": "GitHub-authoritative (jobs: Apple / Smoke Tests, Integration Tests, Lint, Typecheck & Package)"
},
{
"id": "docs-links",
"status": "passed",
"exitCode": 0
},
{
"id": "docs-prose",
"status": "passed",
"exitCode": 0
},
{
"id": "xctest-selection",
"status": "passed",
"exitCode": 0
},
{
"id": "packaged-runner-swift",
"status": "passed",
"exitCode": 0
},
{
"id": "replay-ios",
"status": "skipped",
"reason": "parked, workflow_dispatch only (Replay Manual / iOS Replay Suite)"
},
{
"id": "replay-ios-device",
"status": "skipped",
"reason": "parked, workflow_dispatch only (Replay Manual / iOS Replay Suite)"
},
{
"id": "replay-macos",
"status": "skipped",
"reason": "GitHub-authoritative (jobs: Apple / Smoke Tests)"
},
{
"id": "replay-linux",
"status": "skipped",
"reason": "GitHub-authoritative (jobs: Linux / Smoke Tests)"
},
{
"id": "linux-command-evidence",
"status": "skipped",
"reason": "GitHub-authoritative (jobs: Linux / Smoke Tests)"
},
{
"id": "replay-android",
"status": "skipped",
"reason": "parked, workflow_dispatch only (Replay Manual / Android Full Emulator Suite)"
}
]
}Pushed head: Publication verification: GitHub and |
|
The code at c8ec2a4 looks right, and all 19 checks pass with none failing. There are no conflicts. The earlier review at a3893a5 waited on evidence. That is now met by your quoted iOS 27.0 ATT run against a source daemon at c8ec2a4. I did not run the tests or a device myself. The client launcher in your run was 0.21.25, so the final CLI JSON rendering of Not blocking, and you can take or leave it: Android now uses the same clipping as iOS through ui-hierarchy-builder.ts. It omits framed nodes that fall fully outside the clip, where the base kept them with a 0x0 rect. It also publishes a regular-geometry-clipped note. The help text and the snapshots.md paragraph say only "Regular iOS snapshots", so they could name Android (helper backend) or be platform-neutral. If the Appium Android route does not share ui-hierarchy-builder, please say which backends the sentence covers. I did not trace that route in this delta. One open question is which clipping produces the note. The conformance fixture and ADR 0004 already record that viewport-edge clips do, so one explicit maintainer confirmation would settle it. One item still blocks merge. Android snapshots now omit nodes that fall fully outside the clip, and the only live run is on iOS. Please post one live Android snapshot at this head that shows a clipped node omitted and the regular-geometry-clipped note. |
|
Android live follow-up for the blocking request above: passed at Clean isolated checkout: Reused the installed native Commands: [{"command":"snapshot","input":{}},{"command":"snapshot","input":{"raw":true}},{"command":"snapshot","input":{"interactiveOnly":true}}]Every app-driving command used All three steps returned Regular: {
"nodes": [
{
"label": "Partial action",
"rect": {
"x": 70,
"y": 650,
"width": 330,
"height": 50
},
"visibleToUser": true
}
],
"snapshotQuality": {
"state": "healthy",
"backend": "android-helper"
},
"snapshotQualityNotes": [
{
"code": "regular-geometry-clipped",
"clippedNodeCount": 1,
"droppedNodeCount": 1
}
],
"androidSnapshot": {
"backend": "android-helper",
"helperVersion": "0.21.30-dev",
"helperTransport": "persistent-session"
}
}Raw: {
"nodes": [
{
"label": "Partial action",
"rect": {
"x": 70,
"y": 650,
"width": 330,
"height": 100
},
"visibleToUser": true
},
{
"label": "Outside scroll action",
"rect": {
"x": 70,
"y": 800,
"width": 330,
"height": 100
},
"visibleToUser": true
}
],
"snapshotQuality": {
"state": "healthy",
"backend": "android-helper"
},
"androidSnapshot": {
"backend": "android-helper",
"helperVersion": "0.21.30-dev",
"helperTransport": "persistent-session"
}
}Interactive ( {
"nodes": [
{
"label": "Partial action",
"rect": {
"x": 70,
"y": 650,
"width": 330,
"height": 50
},
"visibleToUser": true
}
],
"snapshotQuality": {
"state": "healthy",
"backend": "android-helper"
},
"snapshotQualityNotes": [
{
"code": "regular-geometry-clipped",
"clippedNodeCount": 1,
"droppedNodeCount": 1
}
],
"androidSnapshot": {
"backend": "android-helper",
"helperVersion": "0.21.30-dev",
"helperTransport": "persistent-session"
}
}Regular and interactive node arrays contain no The installed helper APK SHA-256 exactly matches the newly built c8 artifact: Transport limit: T3's launcher is 0.21.25. Its standalone Source trace for the optional wording question: local Android helper capture calls Cleanup / reservation END: exact owned session closed; source CLI operator |
|
Thanks, this clears the last blocker at c8ec2a4. The Android helper run on Android 15 shows the outside button omitted, the partial button clipped to 50, and the The run went through a 0.21.25 launcher, so the standalone CLI JSON rendering of the note is still covered only by the serialization tests, and the Appium Android route was not run. Neither blocks this PR. |
thymikee
added
the
ready-for-human
1 participant
Summary
Clip regular iOS and Android snapshot geometry to the cumulative visible area, omit fully outside nodes, and publish typed clipping counts independently of capture quality. Raw snapshots retain reported geometry. Refs #3801 and #1973.
The runner wire now carries sibling
snapshotQualityNotes; modal queries invent no verdict/backend. Genuine capture-plan verdicts remain intact. Complete scoped-modal evidence permits an empty projection. Shared annotation parsing bounds notes.45 files across presenters, conformance, metadata, tests, and docs. Scope includes Android omission and end-to-end metadata carry.
Examples:
agent-device snapshot -i --json;agent-device snapshot --raw.Validation
Tested SHA:
c8ec2a4b914140e93971c717cb82bef97cffdad9.Reader regression: base
a3893a572 failed/24 passed; fixed 26 passed. Provider scope regression failed before the follow-up; fixed suite 16 passed. Projection suites: 66 passed. Native wire tests: 4 passed. Fresh independent Sol review: no findings after fixing scoped admission.Live iPhone17e/iOS27: ordinary modal no verdict; clipped modal independent counts; raw geometry retained; genuine app verdict retained; denial action succeeded.
Exact affected receipt and live evidence. CI owns full coverage, provider integration, native/device lanes. Note-population policy remains pending; existing population is unchanged. iOS17.5/action sheets were not rerun.