Repository navigation
feat(snapshot): carry a presented-surface fact through capture and settle - #3577
Conversation
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 38 files
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
Size Report
Startup median (7 runs, lower is better):
|
…er's closure Review of #3577 found four problems with how the fact crosses the wire. `readPresentedSurfaceFact` validated against `PRESENTED_SURFACE_STATES`, which made every eager consumer of `kernel/record` evaluate `kernel/snapshot.ts` as well. ADR 0019's loading-shape probe measures exactly that: thirteen carried entries grew by one module and 70 eager statements, which `scripts/__tests__/eager-closure-budgets.test.ts` refuses — the growth is a pre-existing module made eager, so no split explains it and shrinkage elsewhere cannot fund it. The reader now decides from a table it owns, keyed by `PresentedSurface`, so a vocabulary state without a row fails to compile rather than reading as `unknown`. The lookup is own-property, which also closes the inherited-member path an `in` test would have opened: a payload naming `constructor` is not a state. `PRESENTED_SURFACE_STATES` is frozen. `as const` was only a type-level promise on an array the reader trusted payloads against, so any consumer could move the boundary between a state and `unknown` for the whole process. The client normalized the fact on the default response path and returned a digest payload verbatim, which meant the same field was read two ways depending on `--response-level`. Passing a leveled payload through without reshaping it is not a licence to skip the fact's own vocabulary, so the digest path reads it through the same reader; a payload with no key stays byte-identical. Two doc comments claimed a capture "came from read" something. Both now name the producer that did the reading.
There was a problem hiding this comment.
All reported issues were addressed across 6 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
eb13f84 to
6180ffe
Compare
There was a problem hiding this comment.
All reported issues were addressed across 5 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
View guided diff | Turn on auto-fix | Re-trigger cubic
4afd24c to
29aaf3b
Compare
|
The runner's blocking-system-modal snapshot now stamps Not blocking, and you can take or leave these: Could this be simpler? For example: make The code needs the live simulator run above and the find-route baseline mint before merge. The cubic-dev-ai threads are all fixed at 29aaf3b, so please resolve them: #3577 (comment), #3577 (comment), #3577 (comment), #3577 (comment), #3577 (comment), #3577 (comment) and #3577 (comment). Checks are still pending; "Affected decision-kernel mutants" is queued and only reports scores. I reviewed 29aaf3b by reading the diff and did not run any tests, so I have not verified your gate results or the Swift simulator unit-test pass, or that the Swift encoding reaches the daemon unchanged over the live runner transport. I also inferred, not confirmed, that |
518f568 to
3c785c3
Compare
|
Rebased onto Live run. iPhone 17 simulator with the RN lab app, built at 518f568, runner included. Since then 3c785c3 only removed the beside-the-tree backend field (see below), which no production backend wrote, so CLI output is unchanged. The SpringBoard alert is the microphone prompt from Automation lab → Request microphone. Blocking alert, {"success":true,"data":{"presentedSurface":"presented","nodes":[{"type":"Alert","label":"Allow “Agent Device Tester” to access your microphone?"},{"type":"Button","label":"Don’t Allow"},{"type":"Button","label":"Allow"}, …]}}Same alert, {"success":true,"data":{"presentedSurface":"presented","nodeCount":5}}Home, {"success":true,"data":{"nodes":[{"type":"Application","label":"Agent Device Tester"},{"type":"ScrollView"}, …]}}Home with Find-route mint. Your take-or-leave notes:
The cubic threads were already resolved. Rebasing onto |
3c785c3 to
ac8289f
Compare
|
Rebased again onto
#3592's |
|
The PR is ready at ac8289f. The code now fixes what the earlier review at 29aaf3b raised (#3577 (comment)), and I found nothing new that must change. Not blocking, and you can take it or leave it: On the other threads, these cubic-dev-ai threads are fixed at this head and can be resolved: the leveled path now normalizes through No conflicts. CI is still pending, and no job has failed yet. Smoke Tests and the interaction-settle and snapshot-occlusion Mutants jobs exercise the baseline mint, find's handoff and the response views that this PR changes, so they are the checks to wait for before merge. The live simulator excerpts come from runner 518f568, which predates the rebase that moved the stamp into main's split-out |
thymikee
added
the
ready-for-human
… consumers A settle confirmation that has to prove a dismissal (#1831) has nothing to compare against: the capture contract says nothing about whether a modal presentation was over the app, so a consumer can only infer it from tree shape — and a tree shape that happens to hold before a tap is exactly what makes the confirmation arm on a no-op. iOS is where it matters most, because there an overlay is usually the same app process, so it moves neither the surface identity nor the foreground-owner fact. Publish the producer's own reading instead, on the state that is already built from the capture's unfiltered nodes: - `SnapshotState.presentedSurface` takes 'presented' | 'absent' | 'unknown', and `readPresentedSurfaceFact` is the one wire reader. A producer that inspected its whole subject and saw nothing presenting says 'absent'; one whose evidence cannot answer says 'unknown'; only a producer that never classified stays silent. Anything the vocabulary cannot name reads as 'unknown', so an unrecognised value can never become a false absence. - The fact rides the response envelope on every iOS capture route, including a digest, and travels with the nodes wherever a capture crosses a layer: the backend capture and interactor snapshot results, the daemon capture cache, the interactive-retry and deferred-re-observation nodes, the ref-target frame, and the settle baseline. A consumer that adopts an interaction's own capture therefore keeps the fact that capture established rather than re-deriving a weaker one. - Both settle routes mint that baseline from one function, which moves to capture-kit beside the assembly that builds the state it reads: the interaction route lives in `commands/` and the generic route in `daemon/`, and the layering forbids the second from importing the first. So the next fact a baseline owes cannot be carried by one route and dropped by the other. - The runner forwards the reading of the tier that answered. Its accessibility tree carries no window classes, so it states 'presented' from a resolved blocking system modal and never 'absent'; the shared classifier comes next. Consumers are untouched: nothing decides on the fact yet.
…er's closure Review of #3577 found four problems with how the fact crosses the wire. `readPresentedSurfaceFact` validated against `PRESENTED_SURFACE_STATES`, which made every eager consumer of `kernel/record` evaluate `kernel/snapshot.ts` as well. ADR 0019's loading-shape probe measures exactly that: thirteen carried entries grew by one module and 70 eager statements, which `scripts/__tests__/eager-closure-budgets.test.ts` refuses — the growth is a pre-existing module made eager, so no split explains it and shrinkage elsewhere cannot fund it. The reader now decides from a table it owns, keyed by `PresentedSurface`, so a vocabulary state without a row fails to compile rather than reading as `unknown`. The lookup is own-property, which also closes the inherited-member path an `in` test would have opened: a payload naming `constructor` is not a state. `PRESENTED_SURFACE_STATES` is frozen. `as const` was only a type-level promise on an array the reader trusted payloads against, so any consumer could move the boundary between a state and `unknown` for the whole process. The client normalized the fact on the default response path and returned a digest payload verbatim, which meant the same field was read two ways depending on `--response-level`. Passing a leveled payload through without reshaping it is not a licence to skip the fact's own vocabulary, so the digest path reads it through the same reader; a payload with no key stays byte-identical. Two doc comments claimed a capture "came from read" something. Both now name the producer that did the reading.
…es minted PreresolvedInteractionTarget carries one SurfaceScopedNodes minted from find's own capture instead of mirroring nodes, surface and presentation field by field, so the adopting leaf's settle baseline is that value as-is. PresentedSurface becomes a plain union, and the fact travels only inside SnapshotState: the top-level BackendSnapshotResult field had no production writer. Moves the moving-target gate onto the capture-kit baseline mint.
ac8289f to
105fa64
Compare
The moving-target rest tree copied the baseline shape by hand and left out presentedSurface, so the baseline after a rest proof differed from every other one. It now carries the same facts surfaceScopedNodes mints; it stays local because the snapshot-baseline import would grow the eager closure of most daemon entries.
105fa64 to
9b752cf
Compare
|
Thanks. Head is now
I first replaced the body with So the rest tree stays local, carries the same three facts the mint does, and a comment there names the closure reason. Live re-run at the new layer-1 code. I rebuilt the runner at
The cubic threads you listed were already resolved. |
|
1 participant
Summary
Adds the typed presented-surface fact to the iOS capture contract and carries it end to end, so a
consumer of a baseline can ask whether a capture read a modal presentation instead of guessing from
tree shape (#3565). Layer 1 of 3 — the pipe, plus the one positive the runner already had: the
blocking-system-modal snapshot is stamped
presented(live excerpts below). No other capture changes itsanswer until layer 2 lands.
SnapshotState.presentedSurface?: 'presented' | 'absent' | 'unknown'sits beside the otherproducer-stated facts with one wire reader,
readPresentedSurfaceFact(payload): a payload with no ownkey stays silence, a value under the key the reader cannot name is restated
'unknown', so amalformed statement can never read as absence. It travels inside
SnapshotStatethrough the daemon capturecache, the snapshot state minter, the digest response views, the Node client reader, and the
interaction and serialization field lists.
The reader takes the payload rather than the value it holds, because reading a wire fact is a property
lookup on data the producer controls and every caller would otherwise have to remember that twice: the
key has to be own, or a payload that carries no fact walks to
Object.prototype— where anythingsharing the process could park
'absent', the one state that disarms the confirmation #1831 arms. Thevalue-level form is removed rather than left exported beside the safe one; nothing in production read
the bare value and the fact is unreleased, so a call site that cannot express the unsafe read beats a
comment warning callers off it.
PresentedSurfaceis a plain union. The reader validates against a table keyed by it, so avocabulary state without a row fails to compile instead of reading as
unknown(TS2741), and thelookup is own-property, so a published value naming
constructoris not a state.The client normalized the fact on the default response path and returned a digest payload unreshaped,
so the same field was read two ways depending on
--response-level. A leveled payload now goesthrough the same reader; a payload with no key stays byte-identical.
SurfaceScopedNodesbaselines are minted by one function,surfaceScopedNodesin@agent-device/capture-kit/snapshot-baseline: both settle routes, the moving-target gate, and find'shandoff. The one exception is the moving-target rest-proof tree in
daemon/core/deferred-interaction-outcome.ts, which builds the same three facts locally: importing the mint there adds a module to the eager closure of most daemon entries (ADR 0027, measured byeager-closure-budgets). On find's handoff,PreresolvedInteractionTargetnow carriestree: SurfaceScopedNodesminted from find'sown capture instead of mirroring nodes, surface and presentation field by field. Swift gets the vocabulary type
and payload field, with the blocking-system-modal payload stamped
.presented— the one piece ofpositive runner evidence that already exists.
45 files, +715/−69.
Validation
9b752cf69, rebased ontomain:pnpm typecheckclean; vitest over the touched packages and theinteraction, capture and client suites green. The serialized
pnpm check:affected --runresult for eachlayer is posted on the PR.
Live (iPhone 17 simulator, RN lab app, re-run with the runner built at
105fa6477, after the rebase that moved the stamp intosnapshotForBlockingSystemModal;9b752cf69changes only the daemon rest-proof baseline since). The microphone permission prompt over the lab app:{"success":true,"data":{"presentedSurface":"presented","nodes":[{"type":"Alert","label":"Allow “Agent Device Tester” to access your microphone?"}, …]}} {"success":true,"data":{"presentedSurface":"presented","nodeCount":5}} // --level digestHome, the lab after the prompt is dismissed, and Home with
--level digestpublish nopresentedSurfacekey.
New tests fail on revert: the reader's malformed-vs-missing split, that a key present with no value
reads
unknownrather than the silence of no key (the payload helper takes a required argument so atest cannot accidentally build the missing-key shape it meant to test), that a payload with no own key
is silent while
Object.prototype.presentedSurface = 'absent'is installed (both at the reader and atthe client, for
digestandfull; swapping the own-key check for a plain=== undefinedguard failsthree of them), that an inherited
Object.prototypemember naming a state is not one, the digest reader's normalization (which fails when the digest path returns thepayload untouched), the baseline minter, find's handoff carrying the capture's
presentation to the leaf's tree (
find.test.ts), carriage into both the response and the session record(
presented-surface-carriage.test.ts), the client wire reader, and the Swift payload stamp. The two Swiftpayload tests need the unit-test condition to compile at all
(
AGENT_DEVICE_XCUITEST_INCLUDE_UNIT_TESTS=1 pnpm build:xcuitest:ios), which is also how a missingimport AgentDeviceSnapshotPresentationin this layer was caught; the Swift unit tests compile at this head and ran greenon the iPhone 17 simulator at 29aaf3b. The rebase did not touch the Swift side.