Skip to content

fix(settle): arm the iOS broad-transition confirmation at any requested quiet window - #3512

Merged
thymikee merged 2 commits into
fix/3469-ios-keyboard-chrome-interactivefrom
fix/3470-broad-transition-scope
Oct 13, 2026
Merged

thymikee merged 2 commits into
fix/3469-ios-keyboard-chrome-interactivefrom
fix/3470-broad-transition-scope

Conversation

@thymikee

@thymikee thymikee commented Oct 12, 2026 •

Copy link
Copy Markdown
Member

Part of #3470. The narrowing — arming on which transitions, rather than at which window — is not in this PR: it is the draft that sits on top of this stack and is blocked by #3565. See "Not in this PR" at the bottom.

Stacks on #3482 (fix/3469-ios-keyboard-chrome-interactive) — it builds on the chrome-free comparison that PR adds. Retarget to main once #3482 lands.

What changed

The confirmation #1831 added for a broad screen replacement armed on two things, and only one of them describes the transition the rule exists for. The second was requestedQuietMs >= DEFAULT_STABLE_QUIET_MS, which made the guard a function of the window the caller asked for:

  • --settle-quiet 499 behaved differently from 500, with no rule that explains the boundary;
  • a caller passing settleQuietMs: 150 (tester-army/e2e) got no guard at all, on exactly the dismissal the guard exists for;
  • a shorter window silently meant "trust a coherent transitional tree sooner", which is not what --settle-quiet asks for. It asks for a tighter verdict, not for a different set of transitions to distrust.

Arming now reads the transition only, and a confirmed replacement is held for the caller's own window, floored at 1.5 s. The trigger itself is unchanged from main: a complete-viewport post-action projection or a tiny presented baseline, plus fewer than half the node identities carried over. No narrowing, no new heuristic — this PR only removes the window coupling, so it can land and be reasoned about on its own.

One consequence stated up front, because it is the point of the split: at the default window a tab switch or a push already pays the 1.5 s floor on main, and still does here. What this layer adds is that a caller asking for a shorter window now pays it too (measured below). That is the correct reading of --settle-quiet, and it is also what makes removing the cost in the right places urgent — which is the successor layer's job, not this one's.

Verification

#1831's six loop-level cases now run at both 500 ms and 150 ms, and a caller-route case pins the same floor on the observation a caller reads (settle-confirmation-floor.test.ts): the response reports the window the caller asked for, while the confirmed replacement sets the wait. Restoring the requestedQuietMs >= DEFAULT_STABLE_QUIET_MS bar fails exactly the short-window cases and nothing else, so the guard cannot silently switch itself off at a shorter window again.

Cadence follows the window in force (second commit). Without it the extended wait was sampled on the caller's tight grid: at --settle-quiet 25 that is 109 capture round trips to reach a verdict the default grid reaches in 10 — a confirmation costing 11x the reads it needs, on the tightest windows where settle is asked for at all. Mutation-checked: removing the recompute fails the new capture-count assertion (109 vs 10) and nothing else in the file.

Live A/B (iPhone 18 Pro simulator, BDA9B772, RN test app, purpose-specific session)

Sheet = Settings → Open automation lab → Open automation sheet (pageSheet). late is diff snapshot -i after settle returned; late=0 means settle answered on the tree the UI kept.

route baseline it carries waitedMs @ --settle-quiet 150 confirms?
press 'label="Close automation sheet"' --settle interactive, 4 nodes 1855 / 1890 / 1862 yes, late=0
snapshot then press @ref --settle full tree, 6 nodes 1854 / 1991 / 1856 yes, late=0
press <tab> --settle (control, no dismissal) full tree 1987 / 1848 / 1951 armed, as at the default window on main

The generic-route case (back --system --settle, whose baseline is the session's stored full tree) is the third dismissal route and the one the reviewer's table marks as losing coverage under the narrowing. It could not be exercised as a dismissal on this device — back --system does not dismiss this pageSheet on the iPhone 18 Pro, verified across three independent runs (the sheet is still up afterwards). It was exercised over a transition back can perform there, with a stored full-tree baseline and a genuinely broad arrival (the settle diff reports 24 additions / 25 removals / 3 unchanged): 1840 ms over 5 captures and 1879 ms over 5, i.e. armed, against 506–583 ms over 2 captures on the narrowing head for the same arrival. Those four numbers were taken on the stack's second-layer build, which carries this layer's arming rule verbatim.

And the control row is not free — it is the cost the successor layer removes, measured so the reviewer can see exactly what this layer costs a short-window caller on ordinary navigation.

The provider-backed scenario is itself a dialog dismissal, so it now holds the floor at its 25 ms window too: 1510 ms over 6 captures, against 1511 ms over 6 for the same scenario at the default window on the base — same verdict, same price, no longer window-dependent. Its capture-count budget becomes a floor plus an explicit waitedMs >= 1_500, and the test gains the parallel-scenario timeout its 1.5 s wait needs (that scenario already timed out at base under this machine's load, and the body it sits in reindents as a result — ~210 of this PR's lines are that formatter-forced reindent).

Gates

pnpm format, pnpm lint, tsc --noEmit clean. vitest --project unit-core: 13,373 passed / 1 skipped (a first full run tripped one deadline-sensitive snapshot-source/lifecycle.test.ts case under host load; it passes in isolation and on the clean re-run above, and this layer touches no capture-source code). interaction-contract (68) and provider-integration green; two interaction-contract failures seen mid-branch were confirmed present at the base commit and pass at this head. pnpm check:affected --run --base origin/fix/3469-ios-keyboard-chrome-interactive at bee9e1f34: all runnable checks passed (exit 0) — 506 related test files / 3,962 tests, Fallow clean on the 7 changed files — held under the machine-wide lock. Counts in the gate comment on this PR.

Not in this PR

The narrowing — arming on "the baseline shows a surface presented over app content" instead of on any broad screen replacement — is split out. Its node-count rule keys on how the baseline was captured (the same sheet projects 4 nodes interactively and 6 in a full tree), so it loses dismissal confirmation on 2 of 3 routes; that is measured in #3512 (comment) and the durable rule is a producer-stamped presentedSurface fact, which is #3565. The narrowing now sits on top of this stack as draft #3570, blocked by #3565, so this decoupling can land without waiting for it — and so no head of this branch ever ships a showsPresentedSurface arm.

@github-actions

github-actions Bot commented Oct 12, 2026 •

Copy link
Copy Markdown
Contributor

Size Report

Metric Base Current Diff
Installed (including dependencies) 5.17 MB 5.17 MB +335 B
Package (unpacked) 5.17 MB 5.17 MB +335 B
Package (download) 1.55 MB 1.55 MB +137 B

Startup median (7 runs, lower is better):

Scenario Base Current Diff
CLI --version 24.4 ms 24.5 ms +0.1 ms
CLI --help 68.1 ms 66.4 ms -1.7 ms

@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 8 files

Reply with feedback, questions, or to request a fix.

View guided diff | Turn on auto-fix | Re-trigger cubic

Comment thread src/commands/post-action-observation-grammar.ts Outdated
Comment thread src/commands/interaction/runtime/settle.test.ts
Comment thread src/commands/interaction/runtime/stable-capture.ts Outdated
Comment thread src/commands/interaction/runtime/stable-capture.fixtures.ts Outdated
Comment thread src/commands/interaction/runtime/stable-capture.ts Outdated
@thymikee
thymikee added this pull request to stack #3513 October 12, 2026 15:24
thymikee added a commit that referenced this pull request Oct 12, 2026
…ts help

Review on #3512:
- the null answer is a real answer: '??=' reran the chrome classification on every poll for
  the ordinary full-screen baseline, which is the case the whole point is to keep cheap
- the 1500ms floor is iOS XCTest-only and bounded by the settle deadline, so neither the CLI
  flag nor the MCP schema field may state it as universal
- the comment said a short-window caller is held to 'its own window rather than the fixed
  floor', which is the opposite of what Math.max(requested, 1500) does
- the navigation fixtures are compact projections, not the 24-34 nodes a live screen reports;
  the comment claimed otherwise
- the longpress case budgets for the confirmation without asserting it, so it could not catch
  the baseline failing to reach the loop
@thymikee

Copy link
Copy Markdown
Member Author

I found a problem in b8d7540. The narrowing can disarm the dismissal confirmation on some routes, so this is not ready to merge yet.

In stable-capture.ts:299, showsPresentedSurface treats a baseline with 5 nodes or fewer as proof of a presented surface. That count depends on how the baseline was captured, not on the screen. Three routes hand it a full tree: press 'label="Close"' --settle (selector capture), press @ref --settle after a plain snapshot, and the generic back/scroll route. A full tree of a sheet has more than 5 nodes, so dismissedPresentedSurface returns null and the #1831 confirmation never arms. A form sheet with more than 5 interactive nodes has the same problem. On those routes, iOS settle can again accept the transitional tree within 500 ms of a modal dismissal. That gives a wrong settled diff and refs issued against a tree the UI then replaces. The rule should be that the presented-surface verdict does not depend on how the baseline was captured. Every baseline source that resolveSettleBaseline and planGenericSettleObservation can return must give the same verdict for the same screen. Please add a regression through settleAfterInteraction with a selector target whose resolution capture is the full tree of a sheet, and assert waitedMs >= 1500. I could not run iOS captures, so this comes from reading the capture routing.

Could the classification be made well defined for every baseline? One option is to have the iOS capture producer set a typed presented-surface fact on SnapshotState, next to iosSystemSurfaceBundleId, so stable-capture reads one boolean. A smaller step is to classify only an interactive projection of the baseline, using the flavor its presentationKey already records. Either way, the Alert-root special case could go once the fact covers it. That means passing SurfaceScopedNodes or the baseline SnapshotState into runStableCaptureLoop instead of bare nodes.

The live A/B does not say how steps were targeted. The "dismiss sheet still confirmed" row proves the guard only for the route used, likely @ref from snapshot -i. Please run on the RN test app (iPhone sim, default window), 4 or more runs each. First, press 'label="<sheet close>"' --settle on the presented sheet. Second, snapshot without -i, then press @<close ref> --settle. Both must show settle.waitedMs >= 1500 and an empty diff snapshot -i 2 s later. Also show a selector-targeted tab switch staying near 800 ms.

Coverage fails in test-file-size-ratchet.test.ts. settle.test.ts is 2122 lines at head against 2120 at the merge base, and those are the two comment lines this PR adds at lines 310-311. Please remove them. Smoke Tests are still running, so I have no result for them yet.

The open inline threads still apply. These two P2 threads still stand: help text states the 1500 ms floor without iOS or the timeout cap and settle test should assert waitedMs. Three P3 threads also still apply. No open thread needs to be resolved as not applicable.

A newer head, d969e79, arrived after this review. Its delta looks like it addresses both P2 threads and changes the settle.test.ts lines behind the size check, so please confirm that check at the new head. It does not change showsPresentedSurface, so the main finding and the live-run request above still apply.

@thymikee

Copy link
Copy Markdown
Member Author

The presented-surface check in this PR is too narrow, so the #1831 regression can come back for some sheet dismissals. I reviewed d969e79 by reading the code. I did not run iOS captures.

showsPresentedSurface returns true only when the baseline has 5 nodes or fewer, or an Alert root. That count describes how the baseline was captured, not what is on screen. Three sources can pass a larger tree of a presented sheet: the generic back/scroll route uses session.snapshot as stored (generic-settle.ts:84, a full tree after a plain snapshot), the @ref route uses refFrameSnapshot (settle.ts:240), and the selector route uses the resolution capture (resolution.ts:113). A form sheet with 6 or more interactive nodes is rejected the same way. Then dismissedPresentedSurface returns null and the #1831 confirmation never arms. The merge-base code armed in all these cases through the viewport root in the post-action capture, and your own comment at 15:38 names this against b8d7540. Lines 296-309 are unchanged in d969e79. After a modal dismissal on iOS, settle can accept the transitional tree inside 500 ms again. That gives a wrong settled diff and refs issued against a tree the UI then replaces. The rule to satisfy: a screen gets the same presented-surface verdict whichever source produced its baseline, meaning every SurfaceScopedNodes that resolveSettleBaseline and planGenericSettleObservation can return. Please have the iOS capture producer stamp a typed presented-surface fact on SnapshotState, next to iosSystemSurfaceBundleId, and pass the baseline into runStableCaptureLoop instead of bare nodes. Then delete the count bound and the Alert-root special case. Please add regressions through settleAfterInteraction (a selector target whose resolution capture is a full sheet tree) and through settleObservation (a stored full tree on the generic route), each asserting waitedMs >= 1500.

The live A/B has one "dismiss sheet" row, and it does not say how it was targeted. Its pageSheet projects to 4 nodes, so it can only exercise the 5-node arm. The original #1831 case (Element) was not re-run. Nothing yet shows that the narrowed trigger still covers the other dismissal routes, or a sheet with more than 5 controls. On the iPhone simulator with the RN test app and the default window, please run each of these at least 4 times: (a) press 'label="<close>"' --settle, (b) a plain snapshot (no -i) then press @<close> --settle, (c) back --settle, and (d) any route on a sheet with 6 or more interactive controls. Please report settle.waitedMs >= 1500 for each, plus a diff snapshot -i 2 s later that shows no late diff.

Could the iOS snapshot producer, which already knows the presentation it captured (presentationKey, window or modal scope), stamp a presentedSurface boolean on SnapshotState, with runStableCaptureLoop reading that one fact from the baseline? That would remove the count bound, the Alert-root case and the fixture reshaping. A smaller first step could classify only baselines whose presentationKey marks an interactive projection, and keep the old viewport-root arm for full-tree baselines so no route loses #1831 coverage. Either way, settle.ts:137 and generic-settle.ts need to pass the baseline SnapshotState or SurfaceScopedNodes rather than bare nodes, and SnapshotState needs the new field.

Not blocking, and fine to take or leave: the waitedMs assertion in d969e79 grows settle.test.ts from 2120 to 2127 lines, so move that case into stable-capture.test.ts or a split file that mirrors its source; and the arm also fires the other way, since any baseline with 5 nodes or fewer (a sparse real screen, a truncated capture, the 1-node longpress fixture) pays the 1.5 s floor, which the typed fact above removes.

The Coverage check fails on the size ratchet for settle.test.ts, and those 7 added lines come from this PR. Smoke Tests were still running when I looked, so I cannot attribute them yet. Before merge, the presented-surface check needs to give the same answer for every baseline source, with the two regressions and the live runs above.

The five cubic-dev-ai threads (the post-action-observation-grammar claim scope, the settle.test.ts waitedMs assertion, the stable-capture.ts floor and === undefined fixes, and the fixtures comment) are fixed at this head, so you can resolve them: #3512 (comment), #3512 (comment), #3512 (comment), #3512 (comment), #3512 (comment).

@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 2 files (changes from recent commits).

Reply with feedback, questions, or to request a fix.

View guided diff | Turn on auto-fix | Re-trigger cubic

Comment thread src/commands/interaction/runtime/settle-dismissal-confirmation.test.ts Outdated
@thymikee

Copy link
Copy Markdown
Member Author

The latest push (a565d42) only moves a test, so the problem I raised on d969e79 still stands. The new test and the PR's live A/B cover the narrow case. The fix does not yet cover every route that dismisses a modal.

The main problem is in stable-capture.ts. showsPresentedSurface decides from the baseline's node count (at most TINY_STABLE_TREE_NODE_COUNT) or an Alert root. That count depends on which source produced the baseline, not on what is on screen. The generic back and scroll route passes the stored session.snapshot (generic-settle.ts:85). The @ref route passes refFrameSnapshot. The selector route passes the resolution capture. Any of these can be a full tree of a presented sheet, and a form sheet can have 6 or more interactive nodes. Then dismissedPresentedSurface (line 251) returns null and the #1831 confirmation does not arm. On iOS, settle can then accept the transitional tree right after a modal dismissal. The diff would be wrong, and refs would point at a tree the UI then replaces.

The rule I would like the code to satisfy: a screen gets the same presented-surface answer whatever source produced its baseline. That must hold for every SurfaceScopedNodes that resolveSettleBaseline and planGenericSettleObservation can return. The iOS capture producer owns that fact, so it should stamp a typed presentedSurface on SnapshotState, next to iosSystemSurfaceBundleId. Then pass the baseline SnapshotState or SurfaceScopedNodes into runStableCaptureLoop instead of bare nodes (settle.ts:137), and delete the count bound and the Alert-root arm. Please add regressions through settleAfterInteraction with a selector whose resolution capture is a full sheet tree, and through settleObservation with a stored full tree. Each should assert waitedMs >= 1500.

Could the simpler route be better? Stamping one boolean on SnapshotState would delete the node-count bound, the Alert-root arm and the fixture reshaping. If that feels too large, a smaller first step could classify only baselines whose presentationKey marks an interactive projection, and keep the old viewport-root arm for full-tree baselines so no route loses #1831 coverage. I did not see an answer to this in the latest push.

The live runs I asked for are still missing (stable-capture.ts:218). The one 'dismiss sheet' row is a 4-node pageSheet with no route stated, so it only exercises the small-tree arm. Please run on the iPhone simulator with the RN test app at the default window, 4 or more runs each, for these cases. (a) press 'label="<close>"' --settle. (b) A plain snapshot (no -i), then press @<close> --settle. (c) back --settle. (d) Any route on a sheet with 6 or more interactive controls. For each, report settle.waitedMs >= 1500 and an empty diff snapshot -i taken 2 s later. Please also show a selector-targeted tab switch staying near 800 ms.

I did not run the new test, so I cannot confirm the claim that it fails on the old loop at waitedMs=304/802. I did not run iOS captures either. The routing finding comes from reading resolveSettleBaseline, generic-settle.ts:85 and the node-count rule.

The cubic-dev-ai P3 thread still applies: the comment at settle-dismissal-confirmation.test.ts:26 calls capture 1 selector resolution, but the press targets @e1, so capture 1 is the first sample of the settle loop (#3512 (comment)).

The checks were still queued or running when I looked, so I cannot say yet whether CI passes. This push touches only unit tests under src/commands/interaction/runtime, so Coverage and Lint & Format are the ones to watch. A failure in Smoke or Integration would likely be related, since they run the settle path.

Before merge, the presented-surface answer must be the same for every baseline source through a typed SnapshotState fact. The PR also needs the route-level regressions and the four live dismissal runs.

@thymikee

Copy link
Copy Markdown
Member Author

Confirmed on device, and the numbers are worse than the reading suggested — the same dismissal loses the confirmation on two of the three routes.

iPhone 17 sim (0E0FCBDC), four reps per route, sheet = Settings → Open automation lab → Open automation sheet. The settle response's own settle.quietMs / waitedMs is the signal; late is diff snapshot -i taken after settle returned:

route baseline it carries waitedMs confirms?
press 'label="Close automation sheet"' --settle interactive projection, 4 nodes 1833 / 1842 / 1917 / 2137 yes, late=0
snapshot then press @e6 --settle full tree, 6 nodes 745 / 784 / 793 / 1096 no
back --system --settle over a stored full tree full tree, 6 nodes 732 / 737 / 755 / 763 no
press 'label="Catalog"' --settle (control) — ~700 correctly not armed

The sheet is pageSheet, and the full flavor carries the same surface as the interactive one plus a depth-2 duplicate of each label at the same rect: 4 → 6 nodes. The bound is 5, so the verdict flips on the flavor alone. Route (b) needs nothing exotic — one extra label child in the app's own sheet flips it.

P0 for the fix shape. I then measured what the merge-base arm keys on, on the same screens, both flavors:

screen -i full-viewport roots full-tree roots
sheet 1 1
home / catalog / settings 1 1
lab screen (no sheet) 1 1

Every iOS tree has an Application root at 0,0,402x874, presented or not, interactive or full. So hasCompleteViewportProjection says nothing about presentation — restoring it for full-tree baselines re-arms the tab-switch and push cases exactly as at the merge base (waitedMs 1831 there), it just moves the cost onto the routes that take a plain snapshot first. That is the #3470 complaint, relocated rather than fixed.

So I agree the durable rule is yours: the capture producer stamps a typed presented-surface fact on SnapshotState beside iosSystemSurfaceBundleId, runStableCaptureLoop takes the baseline rather than bare nodes, and both the count bound and the Alert-root special case go. I have not started it because it lands in the capture producers, not the settle loop: the interactive trees on a simulator come from producer simulator-ax-bridge (apple/snapshot-bridge/*.m, which today reports no window or modality at all — grep -c window SnapshotBridgeCapture.m = 0), and the runner has its own presentation code, so the fact has to be stamped by both native producers plus the wire type and the cross-language golden table under contracts/fixtures/. That is its own unit of work and its own PR, and #3512 and #3522 should not be merged until it exists — I'll mark #3512 as not ready rather than paper over it.

Two things I need before I take that on: confirmation that the scope should move into the capture producers here rather than a separate issue, and whether you want #3512 to hold the interim flavor-gated rule in the meantime (interactive-projection baselines keep today's behaviour; full-tree baselines fall back to the merge-base arm) or stay as it is and land the producer fact first. I won't add the interim arm without your call, since by the table above it buys route (b)/(c) coverage by re-charging tab switches on those routes.

@thymikee

Copy link
Copy Markdown
Member Author

Adding a third option, with measurements, because it changes the shape of the decision.

There is a host-side signal that already satisfies the invariant in the review — "the presented-surface verdict does not depend on how the baseline was captured". It is the app's own navigation chrome, not the node count. iPhone 17 sim, two reps, TabBar|NavigationBar|Toolbar|Tab node count per capture:

screen -i capture full capture
home n=30, chrome 2 (TabBar+Toolbar) n=48, chrome 2 (TabBar+Toolbar)
settings n=24, chrome 2 (TabBar+Toolbar) n=42, chrome 2 (TabBar+Toolbar)
lab (pushed, no sheet) n=28, chrome 2 (NavigationBar+Toolbar) n=54, chrome 2 (NavigationBar+Toolbar)
automation sheet presented n=4, chrome 0 n=6, chrome 0

Identical verdict per screen in both flavors, which is exactly what the node count failed at (4 vs 6 straddling the bound of 5). It also fixes both broken routes, because the full tree of the sheet is chrome-free too, and it cannot re-arm tab switches: their baseline carries the tab bar, so it never arms regardless of flavor. No native change, no wire field, no golden table.

Two failure classes I have not solved and would be shipping:

  • A surface that carries its own nav bar (an RN stack inside a modal, a form sheet with a header bar) presents chrome of its own, so it reads as an ordinary screen and the confirmation is missed. Same shape of miss as today's count rule, different population of apps.
  • An app with no navigation chrome at all (single-screen, no tab or nav bar) has a chrome-free baseline for every action, so any action that broadly replaces the tree arms the floor — a wizard or step flow that swaps most of its content per press pays 1.5 s on every step. The broad-replacement test still has to agree, so toggling one label is unaffected, but the whole class of chrome-less apps is exposed where fix(settle): iOS broad-transition confirmation fires on tab switches and pushes and is coupled to the default quiet window #3470 does not expose them today.

So the choice is:

  1. Chrome arm now — closes the live regression on routes (b)/(c) in one function of stable-capture.ts, ships the two failure classes above, and is superseded by the producer fact whenever that lands.
  2. Producer fact (the review's preference) — correct for both classes, because the producer knows what it presented. Needs stamping in both native producers (apple/snapshot-bridge/*.m, which today carries no window or modality fact at all, and the Swift runner), plus the wire type and the cross-language golden table, plus runner device verification.

My read is that 1 is only defensible as a stepping stone you intend to delete, and its second failure class is the objection — if the over-arm matters, #3470 stays open and 2 is the only answer. Say which and I'll implement it; I'm not adding an arm that can charge every press in a chrome-less app without your call.

@thymikee
thymikee force-pushed the fix/3470-broad-transition-scope branch from 70e3e8f to 356ebec Compare October 12, 2026 21:20
@thymikee

Copy link
Copy Markdown
Member Author

Thanks for the update. The code at 356ebec still has the problem from the earlier review (#3512 (comment)). The fixes for the test comments, the claim scope, the waitedMs assertion and the ??= re-evaluation are in, and the six cubic-dev-ai threads on them are fixed at this head and can be resolved: #3512 (comment), #3512 (comment), #3512 (comment), #3512 (comment), #3512 (comment), #3512 (comment). The delta from the last review only changes a test comment. Smoke Tests is still running. It covers the interaction and settle path that this diff touches, so a failure there would likely be related. I did not run the iOS simulator or the unit tests. The device numbers are yours.

The first problem is unchanged and your device run now confirms it. In stable-capture.ts, showsPresentedSurface decides from the baseline node count or an Alert root. A full-tree baseline of the same pageSheet has 6 nodes. That baseline is the stored session.snapshot on the generic back route, or refFrameSnapshot on @ref after a plain snapshot. So dismissedPresentedSurface (line 251) returns null and the #1831 confirmation does not arm. After a modal dismissal on iOS through routes (b) and (c), settle returns in about 730-1100 ms on a transitional tree. The diff is wrong, and the refs it issues point at a tree the UI then replaces, so #1831 regresses on those routes. The rule: the presented-surface verdict must reflect what the capture producer presented, and it must be the same for every SurfaceScopedNodes that resolveSettleBaseline and planGenericSettleObservation can return. The producer owns that fact. Please stamp a typed presentedSurface on SnapshotState (wire type, both native producers, and the contracts/fixtures golden table). Then pass the baseline SnapshotState into runStableCaptureLoop and delete the count bound and the Alert arm. The chrome arm does not meet this rule. By your own analysis it misses modals that carry their own nav bar, such as RN modal stacks and header form sheets. It also re-charges every broad replacement in chrome-less apps, which moves #3470 to another population instead of fixing it. Either land the producer fact first and stack this PR on it, or hold the PR.

The second problem is missing evidence for the changed iOS settle path in stable-capture.ts. Routes (b) and (c) are measured and fail, and route (d), a sheet with 6 or more controls, is not reported. The route-level regressions are also missing: settleAfterInteraction with a full-tree selector resolution capture, and settleObservation with a stored full tree, both asserting waitedMs >= 1500. Under the current rule both would fail. Once the producer fact exists, please run on an iPhone simulator with the RN test app, at least 4 reps each of (a) a selector press of close, (b) a plain snapshot then @ref close, (c) back --settle, and (d) a sheet with 6 or more controls. Each run must report settle.waitedMs >= 1500 and an empty diff snapshot -i 2 s later. Add one selector tab switch that stays near 800 ms.

Is there a smaller correct design? I did not find one. The chrome arm is smaller, but it trades one inferred rule for another, with the two failure classes you named, and I judged it only from your description and 2-rep table because there is no code for it at head. With a producer-stamped SnapshotState.presentedSurface, the settle-loop part of this PR shrinks to a single field read. That needs a producer-fact PR first, covering the snapshot wire type, the simulator-ax-bridge (.m) and Swift runner producers, the golden table, and runner device verification. This PR then stacks on it, or #3470 stays open.

Before merge, the maintainer needs to choose between landing the producer-owned fact and stacking this PR on it, or holding the PR. Smoke Tests also needs to finish green.

…on alone

The arming bar was `requestedQuietMs >= DEFAULT_STABLE_QUIET_MS`, so the confirmation that #1831
added to hold a modal dismissal for 1.5s was a function of the window the caller asked for rather
than of the transition in front of the loop. Three consequences, all reported as #3470:

- `--settle-quiet 499` behaved differently from 500, with no rule that explains the boundary
- a caller passing `settleQuietMs: 150` (tester-army/e2e) got no guard at all, on exactly the
  dismissal the guard exists for
- a shorter window read as "trust a coherent transitional tree sooner", which is not what
  `--settle-quiet` asks for: it asks for a tighter verdict, not a different set of transitions to
  distrust

Arming now reads the transition only, and a confirmed replacement is held for the caller's own
window, floored at 1.5s. The trigger stays exactly as on main: a complete-viewport post-action
projection or a tiny presented baseline, plus fewer than half the node identities carried over.

The six #1831 cases run at 500ms and 150ms alike. The provider-backed scenario is a real dialog
dismissal, so it now holds the floor at its 25ms window too, which is what that wait is worth on a
runner round trip: measured 1510ms over 6 captures, against 1511ms over 6 for the same scenario at
the default window on main — the same verdict at the same price, no longer window-dependent.
@thymikee
thymikee force-pushed the fix/3470-broad-transition-scope branch from 962b838 to 6e22190 Compare October 13, 2026 08:01
Poll cadence derived from the requested window once, before the loop ran, so the 1.5s confirmation
this branch extends that window to was spent sampling at the caller's grid. At `--settle-quiet 25`
that is 109 capture round trips to reach a verdict the default grid reaches in 10 — a confirmation
whose cost is 11x the reads it needs, on the tightest windows where settle is asked for at all.

Cadence now follows the window in force. Both directions hold: a tight window still gets a grid that
can measure it, and an extended wait gets the default grid rather than the caller's. Pinned by the
capture count in the 25ms case: 10 here, 109 with the recompute removed (mutation-checked).
@thymikee
thymikee force-pushed the fix/3470-broad-transition-scope branch from 6e22190 to bee9e1f Compare October 13, 2026 09:06
@thymikee thymikee changed the title fix(settle): scope the broad-transition confirmation to presented-surface dismissal fix(settle): arm the iOS broad-transition confirmation at any requested quiet window Oct 13, 2026
@thymikee

thymikee commented Oct 13, 2026 •

Copy link
Copy Markdown
Member Author

Re: #3512 (comment) — the split you asked for is now the shape of the stack, and this PR no longer contains the arm you rejected.

What changed here. This head (bee9e1f34, 7 files / 482 lines gross) is only the decoupling: the 1.5 s confirmation arms on the transition alone, at any requested quiet window. showsPresentedSurface is not in this branch at any commit — the count bound, the Alert arm, and dismissedPresentedSurface were all lifted out with the narrowing. The trigger stays byte-for-byte main's: complete-viewport post-action projection or a tiny presented baseline, plus fewer than half the node identities carried over. So the #1831 coverage cannot regress on routes (b), (c) or (d) at this head: every broad replacement still gets the floor, which is why the reviewer table's routes read as they do at this head: (a) 1855 / 1890 / 1862, (b) 1854 / 1991 / 1856, (c) 1840 and 1879 over 5 captures — all armed, none of them the 730–1100 ms the narrowing produced.

Where the narrowing went. #3570 — draft, base fix/3470-settle-quiet-per-capture-backend, Blocked by #3565. It carries your route table, the 4-vs-6-node flip, the iPhone 18 Pro A/B at the head (selector press 1736/1708/1714 armed; @ref after snapshot 531/524/522 lost), the generic-route A/B, and the note that back --system does not dismiss this pageSheet on that device so your route (c) dismissal numbers are yours and I am carrying them, not confirming them. Its docblock on showsPresentedSurface says outright that the rule reads a property of the baseline capture, not of the screen, and that the durable rule is the producer's fact.

On the producer fact. Agreed it is the durable rule, and the draft states the contract you set: runStableCaptureLoop takes the baseline SnapshotState rather than bare nodes, both producers stamp presentedSurface, wire type plus contracts/fixtures/ golden table, and 'unknown' keeps the parent's arming — so the fact can only narrow the guard, never switch it off. TINY_STABLE_TREE_NODE_COUNT as an arming bound and the Alert arm both go with showsPresentedSurface. That producer work is #3565's scope (owner: the capture side, runner verification included), so this stack does not attempt it.

On the route-level evidence you asked for. The 4-rep × (a)(b)(c)(d) + tab-switch run is written as a requirement on #3470, and it belongs to the layer that changes which transitions arm — i.e. the draft, once #3565 lands. What this PR owes is evidence for the change it actually makes, which is that arming no longer depends on the window: the (a) and (b) rows above are --settle-quiet 150, a window at which main arms nothing, both late=0; the provider scenario holds the floor at its own 25 ms window (1510 ms / 6 captures, against 1511 / 6 at the default window on the base); and the tab-switch control row (1987/1848/1951) is the cost that draft removes, measured rather than asserted. I did not run (d) — a sheet with 6+ controls — because at this head the arm is main's and node count cannot disarm it; if you want it anyway as a guard against the rebase, say so and I'll run it.

Gate for this head: pnpm check:affected --run --base origin/fix/3469-ios-keyboard-chrome-interactive → all runnable checks passed (exit 0), under the machine-wide lock; counts in the gate comment. The Compatibility & Provenance Fallow job is green at bee9e1f34: the one finding this layer actually caused (runStableCaptureLoop crossing the complexity threshold on the cadence recompute) is fixed by recomputing the cadence unconditionally — the window is a monotonic max, so the guard carried no behavior — and the 1.5 s cadence case still pins 10 captures vs 109. The job's 729-file audit scope is a merge-ref artifact of stacking on a branch whose own PR targets main; everything it lists there is inherited from main, not from these 7 files, and nothing was suppressed.

@thymikee

Copy link
Copy Markdown
Member Author

Gate — fix/3470-broad-transition-scope @ bee9e1f3406e9fa7a2f5d5ed5d71355105b92877

Local, held under the machine-wide lock, started 2026-10-13T09:15:01Z, finished 09:18:29Z, against a clean tree.

pnpm install --frozen-lockfile                      exit=0
pnpm build                                          exit=0
pnpm format                                         exit=0  (tree clean afterwards)
pnpm check:affected --run \
  --base origin/fix/3469-ios-keyboard-chrome-interactive   exit=0

check:affected selected the affected set (7 checks, no fail-open) and reported all runnable checks passed:

check result
format:check pass
lint pass
typecheck (tsc -b) pass
check:layering pass
check:fallow --base origin/fix/3469-ios-keyboard-chrome-interactive pass — No issues in 7 changed files (22.23 s)
build pass
vitest related (7 changed files) pass — 506 files / 3,962 tests

Full vitest --project unit-core separately: 13,372 passed / 1 skipped, and one load-sensitive failure unrelated to this diff — packages/platform-apple/src/snapshot-source/lifecycle.test.ts "request cancellation after dispatch reaps the exact helper before recovery" tripped its target-identity-deadline inside the 13k-test run and passes in isolation (16/16). This layer touches no capture-source code.

CI at this head: all jobs green except the two intentionally-skipped lanes (cubic · AI code reviewer, matrix.name) — including Compatibility & Provenance.

Fallow at the previous head — fixed at the owner, not suppressed. The one gated finding this layer caused was its own: the cadence recompute pushed runStableCaptureLoop to cyclomatic 16 / cognitive 17, past the threshold. The previousQuietMs guard was redundant — requiredQuietMs is a monotonic max, so the branch could only re-derive the same number — and removing it puts the function back under the threshold while the cadence case still pins 10 captures vs 109 at --settle-quiet 25. No baseline entry, no suppression comment.

Two details from the Compatibility & Provenance log, so nobody re-chases them. The job's scope reads 729 changed files vs faf754aa6 rather than this layer's 7, because GitHub's merge ref for a stack whose base branch is itself an open PR against main (59193e095 = merge(faf754aa6 → main@6087be3a)) already carries main's content on its first parent. Fallow's gate then excluded 17 inherited findings on that basis — all 14 dead-code entries (8 unused exports in src/daemon/handlers/__tests__/session-test-harness.ts: mockPrewarmIosRunnerSession, mockNotifyIosRunnerAppRelaunched, mockPrepareIosRunner, mockStopIosRunner, mockDismissMacOsAlert +3; and 6 unused root devDependencies: @types/json-schema, ipaddr.js, pngjs, tar-stream, undici, yauzl) plus the 3 clone groups — every one in a file this layer never touches, and the mock-export set reproduces when the same rule is run directly at origin/main. They are still printed under ● headings, which is what made them look like this PR's.

@thymikee

Copy link
Copy Markdown
Member Author

This looks right at bee9e1f. The concerns from 356ebec are fixed. The help text now limits the 1.5 s confirmation to iOS and names the timeout deadline. The stable-capture comment, the tab/push fixtures and the extra presented-surface re-evaluation are cleaned up.

One decision is left before merge: the merge order. With this change, a caller with a short window (for example --settle-quiet 150) pays the 1.5 s floor on every tab switch and push. Should this land together with #3570, or do we accept that cost until #3570 lands?

Not blocking: the MCP/Node text at post-action-observation-grammar.ts#L37 should say "1500ms", like the CLI text does.

CI is green at bee9e1f (19 checks, none failing), and there are no conflicts. I did not run a simulator or the tests, so the live timings at --settle-quiet 150 are your report. Did the press rows run at bee9e1f or at 1f97dfc? The back-route rows ran on the #3570 build.

The six cubic-dev-ai threads are fixed at this head and can be resolved. Five are fixed directly: r4249620115, r4249620160, r4249620174, r4249620186 and r4250614159. r4249620152 is covered because settle-confirmation-floor.test.ts asserts the 1.5 s floor on the same shared settle route.

@thymikee thymikee added the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Oct 13, 2026
@thymikee
thymikee merged commit 4952ad4 into main Oct 13, 2026
19 checks passed
@github-actions

Copy link
Copy Markdown
Contributor
PR Preview Action v1.8.1
Preview removed because the pull request was closed.
2026-10-13 10:47 UTC

@thymikee

Copy link
Copy Markdown
Member Author

Answering both, on the head you reviewed (bee9e1f34, since merged).

The merge order — the decision, and what actually happened. #3482 and #3512 landed at 10:18:56/57Z, so the question is now retrospective; recording the judgement for #3470's trail. The added cost is confined to callers who pass a sub-500 ms window — in practice the settleQuietMs: 150 population — and that is precisely the population that, on main, got no confirmation on a dismissal at all. The layer trades a latency saving they were never entitled to (the flag asks for a tighter verdict, not for a different set of transitions to distrust) for the #1831 guard being on where it silently was not. Accepting the cost until the narrowing lands is the right call, and the narrowing has a defined route: #3565's producer fact, then #3570 reads it with 'unknown' degrading to this arming. If the cost ever needs removing faster than #3565 can land, the fix is that fact, not a second heuristic — the chrome arm and the node-count arm both failed for reasons already on this thread.

1500 → 1500ms — taken, and it moved. The MCP/Node string is owned by post-action-observation-grammar.ts, which #3522 already rewrites on that very line (it is adding the 300 ms clause), so the unit is fixed there rather than in a separate diff: #3522. Nothing pins the old wording — at least 1500 occurs in exactly the two description sources, the CLI's flag-definitions-action.ts already read 1500ms, and the schema/parser/command-registry suites (47 files, 309 tests) pass with the change.

On the provenance of the live rows — you read it correctly, and here is the precise answer. The press rows (a)/(b) and the tab-switch control were run on the working tree that became 6e2219075 — the 1f97dfc arming rule plus the cadence commit — so not on bee9e1f34 itself: that head's identity comes from the amend that landed at 08:57Z, after the last request in the session state dir. The back-route rows are the #3570 build, as you say. What I can claim is the distance between the two: from the reviewed head to the shipped head, this layer's tree changes only by the previousQuietMs guard around the cadence recompute (git diff 6e2219075 bee9e1f34 = 3 insertions / 2 deletions, both in stable-capture.ts), and that guard could not alter waitedMs — requiredQuietMs is a monotonic max, so the recompute re-derives the same pollMs either way, which is why removing it was the fix for the complexity finding rather than a behavior change. Cadence has its own assertion (10 captures vs 109 at --settle-quiet 25), and the dismissal cases all assert waitedMs >= 1_500, so if the guard had carried behavior the unit layer would have caught it. Say the word if you want the (a)/(b)/(d) rows re-run on a head-identical build and I'll schedule the device time; I did not want to claim numbers against a SHA they were not taken on.

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.

1 participant