fix(ios): preserve regular snapshot depth through structural wrappers - #1947
Conversation
Size Report
npm unpacked components
Startup median (7 runs, lower is better):
Top changed chunks:
Top changed packed files
|
|
P1: the new acquisition frontier counts regularPresentedDepth using semantic eligibility before SnapshotVisibilityFold applies visibility and clipping. A labeled or interactive node outside the viewport therefore consumes the requested depth, shouldAcquireChildren stops traversal, and the fold can then remove that node, losing an independently visible descendant that regular presentation would otherwise reparent. The committed wrapper test covers only an in-viewport ineligible wrapper, so it cannot catch this. Drive a fixture with an eligible but clipped parent and a visible independently-projected child; acquisition must continue until the requested visible presented depth is complete, or the frontier must use the same visibility decision as presentation without duplicating that policy. Please also record exact-head live depth evidence and itemize the current +4.6 kB unpacked growth. |
b2d3086 to
9e3686e
Compare
|
P1: the visibility-aware implementation resolves the original clipped-parent logic, but the new regression still does not exercise acquisition. It constructs a fully acquired three-node RawAX tree and manually reenacts traversalDecision, regularPresentedDepth, and shouldAcquireChildren; reverting the recursiveTreeSnapshotAcquisition call-site/state propagation in RunnerTests+Snapshot.swift to the old eligibility-only frontier leaves this test green. Please put the frontier transition behind one pure helper used by the production traversal and drive the clipped-parent fixture through that helper, with recorded red-on-pre-fix evidence, or add an exact-path simulator/live regression proving the child is actually acquired. The test is also absent from the ios.yml only-testing list, while the host lane evaluates platformDefault as plainViewport rather than the iOS cursorProjected policy, so current CI does not cover the changed iOS path. The generic iOS smoke is green but does not exercise clipped-parent snapshot --depth. Please also update the PR body to exact head 9e3686e and 16 files, record exact-head live depth evidence, and itemize the current +7.6 kB npm-unpacked growth with why the shared-fold design is the smallest viable implementation. |
ee334fb to
267f2df
Compare
|
P1: the clipped-parent regression now proves the pure helper, but it still does not exercise recursiveTreeSnapshotAcquisition. Reverting that production loop to the old depth-only frontier (or dropping its parentPresentedDepth / parentTraversal propagation) while leaving regularTraversalTransition intact keeps this test green, because it starts from a fully materialized RawAXNode tree. The exact-head iOS job selected and passed it, but the PR body still says the required public snapshot --depth 1 case is pending. Please add an exact production-path regression, preferably a fixture-screen iOS test that invokes regular snapshot --depth 1, and record pre-fix red or live evidence. It must observe root plus the independently projected child at presented depth 1 with the clipped structural parent absent; raw --depth 1 must still omit the raw depth-2 child. P2: update the PR body size figure from +7.6 kB to the exact-head Size report: +8.4 kB npm unpacked (+8.3 kB Apple runner source/project). The shared-fold rationale is useful; it should accompany the current number. |
|
Re-review at 0bfdf57: the prior production-path P1 is fixed in design. The new public-CLI scenario is non-vacuous: full raw proves the clipped parent/descendant and raw depth >1; regular --depth 1 requires the parent absent and child projected at root depth 1; raw --depth 1 requires the child absent and all nodes bounded to raw depth 1. Reverting the acquisition frontier would fail it. Size evidence is current at +8.4 kB unpacked / +8.3 kB Apple runner with adequate minimality rationale. Readiness remains pending only on evidence: the exact-head iOS smoke is still running, and the PR body still stamps 5f5b1ee... / says live evidence pending rather than current head 0bfdf57.... Once that fixture-backed iOS smoke passes and the body records the exact-head result, this is clean. |
b43967b to
3a9876c
Compare
|
Re-review of exact head
The failed Android smoke is an unrelated |
|
Addressed the re-review blockers in |
thymikee
added
ready-for-human
|
Follow-up on the iOS smoke failure: the exact-head raw payload showed React Native/XCTest flattening the visual wrapper and child into AX siblings, so the ancestry assertion was invalid. I removed that false assumption and simplified the fixture. Focused XCTest remains the owner of the clipped-parent projection invariant; the public live scenario now proves the actual CLI contract: the child is raw-deep, appears at presented depth 1 in a regular snapshot, and is omitted by raw depth 1. Published as 0b0a029; all locally runnable affected checks pass, and exact-head iOS CI is running. |
thymikee
added
the
ready-for-human
|
…equest-bound-migration-803b60 * origin/main: perf(contracts): granularize entry surfaces so hub importers stop evaluating the facade clump (#1969) fix(ios): preserve regular snapshot depth through structural wrappers (#1947) diagnose(1874): instrument the synthesized commit wait and add a dispatchable stall loop (#1941) refactor(tests): replace the test-utils barrel with direct module imports (#1956) fix(ios): enforce regular snapshot clip invariant (#1946) refactor(daemon): lazy-load platform cleanup helpers in session teardown (#1950) perf(typecheck): make the root tsc project incremental (#1957) docs: drop stale apps.ts over-budget warning from AGENTS.md (#1954) refactor: dedupe containsPoint and rectArea into @agent-device/kernel (#1953) # Conflicts: # packages/platform-android/src/runtime.ts # packages/platform-harmonyos/src/runtime.ts # packages/platform-linux/src/runtime.ts # packages/platform-vega/src/runtime.ts # packages/platform-web/src/runtime.ts # packages/provider-limrun/src/app-log-runtime.test.ts # packages/provider-limrun/src/interaction-operations.ts # packages/provider-webdriver/src/platform-runtime.ts # src/core/command-descriptor/registry.ts # src/core/dispatch.ts # src/platforms/apple/plugin.ts
1 participant
Summary
Implements the ADR 0004 regular-snapshot visible-depth frontier for #1797. This PR does not claim to close #1797.
--depthis enforced against the presented tree. A visible raw-deep child can therefore remain insnapshot --depth 1when presentation collapses structural ancestors.SnapshotVisibilityFoldtransition instead of adding arbitrary acquisition depth, node caps, or backend-specific exceptions. Raw projection and backend capability declarations remain unchanged.Validation
0b0a029c90fb6aba31c32009ab4783cf99333d9f, based oncb65d6ca1f18683c7e9cb60e6e454bb974d23154.pnpm check:affected --runpassed every locally runnable check on the published head, including formatting, lint, typecheck, layering, fallow, package verification, 2,806 related tests, XCTest selection, and the gate/fixture models. The focused iOS smoke coverage contract also passed 14/14.+8.4 kB(+8.3 kBApple runner source/project,+111 BJavaScript), npm tarball+1.9 kB, JavaScript gzip+48 B. The shared-fold design is the smallest owning-interface change considered.