Repository navigation
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
6180ffe to
4afd24c
Compare
… 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.
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 |
This branch has not been deployed
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 only: nothing computes a value yet, so no capture changes
its answer 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 the backend result, the daemon capture
cache, 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.
It validates against a table it owns rather than the exported
PRESENTED_SURFACE_STATES. A load-timeedge from
kernel/recordtokernel/snapshotmakes the latter eager for every consumer of the sharedwire reader, which ADR 0019's loading-shape probe holds at the merge-base's module count (ADR 0027's
no-growth rule): thirteen carried entries grew by one module and 70 eager statements. The table is
keyed by
PresentedSurface, so a vocabulary state without a row fails to compile instead of readingas
unknown(TS2741), and the lookup is own-property, so a published value namingconstructorisnot a state. The exported vocabulary is frozen —
as constwas only a type-level promise on the arraythe reader trusted payloads against.
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.
Both settle routes now mint their
SurfaceScopedNodesbaseline from one function,surfaceScopedNodesin@agent-device/capture-kit/snapshot-baseline, replacing two hand-writtenliterals that each had to be edited whenever a baseline fact was added. 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.
38 files, +699/−33.
Validation
29aaf3be2, rebased ontomain(which had moved nine commits, including the vitest 5 upgrade; theonly conflict was two independent test blocks landing at the same spot in
snapshot-state.test.ts, andboth were kept):
pnpm typecheck,pnpm lint,pnpm check:layering(exit 0),pnpm check:production-exports,pnpm formatclean, andscripts/__tests__/eager-closure-budgets.test.ts(842) green — the gate thatrejected the first version of this layer. The serialized
pnpm check:affected --runis running at theseheads; each head's exit code is recorded in the layer-3 body.
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, that the exported vocabulary survives threemutation attempts, the digest reader's normalization (which fails when the digest path returns the
payload untouched), the baseline minter, carriage into both the response and the session record
(
presented-surface-carriage.test.ts, which pins the fact through both backend routes — a tree-sidefield and backend-built state), the client wire reader, and the Swift payload stamp. The two Swift
payload 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; all three Swift unit tests run andpass on the iPhone 17 simulator. No output change in this layer, so no device run: the live check is
in layer 3.