Skip to content

fix(settle): arm the dismissal confirmation on the producer's presented-surface fact - #3570

Open
thymikee wants to merge 2 commits into
mainfrom
fix/3470-presented-surface-arming
Open

thymikee wants to merge 2 commits into
mainfrom
fix/3470-presented-surface-arming

Conversation

@thymikee

@thymikee thymikee commented Oct 13, 2026 •

Copy link
Copy Markdown
Member

Closes #3470. Builds on #3522 (merged).

Summary

The iOS 1.5 s dismissal confirmation (#1831) now reads the producer's presented-surface fact (#3565):

  • absent rules it out: nothing was presented, so a tab switch or push keeps the plain window.
  • presented arms it, even on a rootless first capture.
  • unknown, or no fact, keeps the parent's arming, so the fact can only narrow the guard.

A stored tree outdated by a later side effect drops its fact, so an absent read taken before a sheet opened cannot disarm that sheet's dismissal. Help text updated. 10 files.

Validation

927d2ce57, iPhone 17 Pro sim, RN lab Release build (the dev client's floating button makes every screen unknown, per #3578).

Route, 4 reps Baseline waitedMs Late diff snapshot -i
(a) selector press close presented, 4 nodes 1694–1887 empty
(b) snapshot, then @ref close presented, 6 nodes 1824–1857 empty
(c) back --settle, Contacts New Contact sheet presented, 70 nodes 1784–1801 empty
(d) same sheet, press x y close presented, 70 nodes 2524–2865 empty
(e) selector tab switch absent 627–778 empty

Tab switch, same device and route: | base 40d868233 1829–1872 ms | head 627–778 ms |.

A full-screen modal reads unknown, so it keeps the confirmation. Each fact branch has a unit or daemon case that fails under mutation.

pnpm check:affected --run --base origin/main at fea0bb95a (rebased on main after #3522 merged): all runnable checks passed, 535 test files, 4,060 tests, Fallow clean on 10 files, under the host lock. The device runs above were taken at 927d2ce57. Since then the only change is help text, and the rebase onto #3522's squash is identical in content.

🤖 Generated with Claude Code

@github-actions

github-actions Bot commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor

Size Report

Metric Base Current Diff
Installed (including dependencies) 5.25 MB 5.25 MB +533 B
Package (unpacked) 5.25 MB 5.25 MB +533 B
Package (download) 1.58 MB 1.58 MB +207 B

Startup median (7 runs, lower is better):

Scenario Base Current Diff
CLI --version 41.6 ms 42.9 ms +1.3 ms
CLI --help 119.8 ms 118.1 ms -1.6 ms

@thymikee

thymikee commented Oct 13, 2026 •

Copy link
Copy Markdown
Member Author

Gate — fix/3470-presented-surface-arming @ 76bbe2475

Supersedes the earlier gate comment on this PR, taken at 183801de7 before the stack's parents merged and this layer was restacked onto #3522's main-based head.

Local, held under the machine-wide lock, started 2026-10-13T10:59:53Z, 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 fix/3470-settle-quiet-per-capture-backend  exit=0

All runnable checks passed: format:check, lint, typecheck, check:layering, check:fallow --base fix/3470-settle-quiet-per-capture-backend (No issues in 4 changed files), build, and vitest related over the 4 changed files — 296 files / 2,050 tests (unit-core, interaction-contract, provider-integration).

This layer's 4 files are byte-identical to a3dc159c8, the head every measurement in the body was taken on (git diff a3dc159c8 HEAD -- <those 4 files> is empty); the restacks moved only the parent.

Still a draft: the rule it demonstrates keys on how the baseline was captured, and the durable version reads #3565's producer fact. Do not merge.

@thymikee

Copy link
Copy Markdown
Member Author

I found two problems at 183801d, so this PR should not merge as it stands. CI is still pending: Smoke Tests is queued, and since this change alters iOS settle arming, a later failure there could not be ruled unrelated without its log.

The arming rule in showsPresentedSurface (stable-capture.ts#L455) depends on the baseline's node count (5 or fewer) or an Alert root. A baseline from a full-tree capture breaks this. Take a plain snapshot followed by press @ref --settle, or back --settle over a stored full tree. The same pageSheet has 6 nodes there, so broadTransitionBaseline returns null and the 1.5s dismissal confirmation never arms. A sheet with more than 5 controls fails the same way on every route. Your own device runs show it: 531/524/522 ms against the parent's ~1850 ms, and a broad 24+/25- back arrival also goes unarmed. So on 2 of 3 measured iOS routes, and on any sheet with more than 5 controls, --settle can report settled:true on a coherent tree captured mid-dismissal. That removes the #1831 guard on production routes. The rule the code must satisfy is that arming is identical for every baseline flavour (interactive, full, depth-limited, scoped) of one screen. Only a producer fact computed before projection can meet that. Please rebase onto #3565 and arm unless baseline.presentedSurface === 'absent', so 'unknown' keeps #3512's arming. Then delete showsPresentedSurface, isBlockingSystemAlertRoot and the count bound used as an arming rule.

The tests do not catch this. The daemon-route test in settle-observation.test.ts#L21 used a 3-node Continue/Cancel dialog and asserted waitedMs >= 1_500. Now the nodes are padded with SHARED_SCREEN_NODES and the assertion became captures in 2..3, so no provider-scenario test checks the confirmation through the daemon. The new unit tests feed broadTransitionBaselineNodes straight into runStableCaptureLoop, which skips resolveSettleBaseline, the step that picks the interactive or full flavour. Nothing in the suite can see the flavour regression. Please keep one daemon-route dismissal case asserting waitedMs >= 1_500. Please also add a parity case that dismisses the same presented surface from an interactive baseline and from a full snapshot baseline, and asserts both arm. It should fail at this head and pass once #3565's fact is read.

Could this layer skip the node-count classifier entirely? If #3565's typed presentedSurface lands first (computed before projection, carried through resolveSettleBaseline, refFrameSnapshot and planGenericSettleObservation, with golden parity under contracts/fixtures/), this change shrinks to if (baseline.presentedSurface === 'absent') return null; in broadTransitionBaseline, plus passing the baseline SnapshotState instead of bare nodes into runStableCaptureLoop. That would replace about 200 production lines with about 10, and the navigation table tests carry over with presentedSurface:'absent' fixtures. Is there a reason the count-based rule is needed beyond that?

I read the fixtures against the parent's arm but did not run the navigation table or mutation check. The device numbers are yours and I did not reproduce them. They support the finding rather than clear it. Before merge, #3565 must land, then this layer needs the rewrite above and the interactive-versus-full parity test.

@thymikee
thymikee force-pushed the fix/3470-presented-surface-arming branch from 183801d to 3a25bb2 Compare October 13, 2026 10:41
@thymikee
thymikee force-pushed the fix/3470-presented-surface-arming branch from 3a25bb2 to 76bbe24 Compare October 13, 2026 11:05
@thymikee

Copy link
Copy Markdown
Member Author

The rebase onto main at 76bbe24 leaves the two problems from the earlier review (183801d) in place, so this still needs changes before it can merge. The arming rule is still a node count, which gives different answers for different views of the same screen.

In stable-capture.ts, showsPresentedSurface arms only when the baseline has 5 or fewer nodes or an Alert root. The same pageSheet has 4 nodes in an interactive baseline but 6 in a full one (snapshot then press @ref --settle, or back --settle over a stored full tree). A sheet with 6 or more controls fails on every route. In those cases broadTransitionBaseline returns null and the 1.5 s dismissal confirmation never arms, so iOS --settle can return settled:true on a coherent tree captured mid-dismissal. That removes the #1831 guard on production routes, and the 52x ms timings in the PR table against the parent's ~1850 ms show it. The rule the code must satisfy is that arming is identical for every baseline flavour of one screen (interactive, full, depth-limited, scoped). Only a producer fact computed before projection can guarantee that. Once #3565 lands, could you rebase onto it and arm unless baseline.presentedSurface === 'absent', so that 'unknown' keeps #3512's arming? Then showsPresentedSurface, isBlockingSystemAlertRoot and the count bound can go.

In settle-observation.test.ts, the daemon-route dismissal case no longer asserts waitedMs >= 1500. It now asserts 2..3 captures over padded SHARED_SCREEN_NODES. The new unit tests feed nodes straight into runStableCaptureLoop and skip resolveSettleBaseline, which is the step that picks the baseline flavour, so nothing can see the regression above and it passes CI. Please keep one daemon-route dismissal case that asserts waitedMs >= 1500. Please also add a parity case that dismisses the same presented surface from an interactive baseline and from a full snapshot baseline, and asserts both arm. It should fail at this head and pass once #3565's fact is read.

For live evidence, after the rebase please run at least 4 reps each on an iPhone simulator with the RN test app. The routes are (a) a selector press of close, (b) a plain snapshot then a @ref press of close, (c) back --settle, and (d) a sheet with 6 or more controls. Each must report settle.waitedMs >= 1500, and an empty diff snapshot -i taken 2 s later. Please also include one selector tab switch that stays near 800 ms. The current table shows the state before this change on the count rule.

I read the code only. I did not run the suites or reproduce the device timings. #3565 is not merged yet, so I could not check how the rewrite will consume the fact. All CI checks were still queued or running when I looked, and the Integration Tests, Coverage and iOS Smoke Tests jobs overlap this change, so I can't rule out a failure there yet. Before merge, #3565 must land, then the arming rewrite, the two tests and the device runs above.

@thymikee

Copy link
Copy Markdown
Member Author

Confirming the disposition, taking the two test asks now, and answering your closing question. Head since: e43dd7abc (one commit, on top of the layer you reviewed; the rule and fixtures it demonstrates are byte-identical to what you read — git diff 183801d..HEAD -- stable-capture.ts stable-capture.fixtures.ts stable-capture.test.ts is empty for the rule and its fixtures, so your reading of the mechanism still applies exactly).

Stays a draft, and gets rewritten, not repaired. When #3565 lands, this layer becomes if (baseline.presentedSurface === 'absent') return null; in broadTransitionBaseline, with the baseline SnapshotState threaded through instead of bare nodes, and showsPresentedSurface, isBlockingSystemAlertRoot and the TINY_STABLE_TREE_NODE_COUNT arming bound deleted. presentedSurface: 'unknown' keeps #3512's arming, so a producer that has not learned to answer degrades to today's behaviour and never to no confirmation. The navigation-table cases carry over on presentedSurface: 'absent' fixtures.

The rebase you asked for is not possible yet: #3565 is still an open issue — no PR, no branch on the remote — so there is nothing to rebase onto and no presentedSurface field to read. That is the whole reason for the draft. I have not started a speculative version of the fact; the producer-side shape belongs to #3565.

Answer to "is the count-based rule needed beyond that?" — no. I kept it only because the fact does not exist yet, and the two heuristics that could substitute for it both fail the way the count bound does. Measured on the same screens: main's viewport-root arm is useless for full-tree baselines because every iOS tree has an Application root at 0,0,402x874, presented or not, so restoring it re-arms the tab-switch and push cases (the #3470 cost, relocated to whichever route took a plain snapshot first, not fixed); the chrome-classifier heuristic was rejected because it regresses modals that carry their own nav bar. A size bound and a root-type test are both read-after-projection proxies, which is exactly the property your finding attacks. Nothing argues for keeping the count.

Your line estimate understates how small the change is, which only helps the case: the rule in production code is 19 non-doc lines (broadTransitionBaseline, showsPresentedSurface, isBlockingSystemAlertRoot), not ~200. This layer's production delta is +46/−20 in stable-capture.ts; the ~200 you saw is mostly the new fixtures, which per your own note carry over.

Both test asks are in, and I measured them rather than writing them blind. Reproduced through the generic back --settle route on the injected clock, one pageSheet dismissed from the two flavours: interactive baseline (4 nodes) confirms for 1,502 ms; full-tree baseline of the same sheet (6 nodes) for 27 ms, settled: true either way.

  1. One daemon-route dismissal asserting waitedMs >= 1_500 — restored by deleting the padding rather than adding a parallel case. You are right that SHARED_SCREEN_NODES is what hid this, and the mechanism is worth stating exactly: padding the dialog to 6 nodes made the transition non-broad (4 of 6 node identities survive, a 0.67 ratio against the 0.5 bound; unpadded it is 1 of 3, i.e. 0.33), which is why the assertion had to become captures in 2..3 — the confirmation was not merely unasserted, it genuinely stopped arming. With the padding reverted, all four provider-scenario cases pass and the flagship case asserts the floor again; settle-observation.test.ts now matches its parent exactly, so it is out of this PR's diff entirely. That padding was mine, added to keep the fix(settle): iOS broad-transition confirmation fires on tab switches and pushes and is coupled to the default quiet window #3470 navigation cost story visible in that file, and it silently traded away the fix: derive iOS transition snapshots from visible presentation #1831 check — the review caught something the suite could not.
  2. An interactive-versus-full parity case that fails here and passes once the fact is read — added in generic-settle.test.ts, both flavours in one case, both asserted to confirm.

One deviation to flag, since it inverts your instruction. You asked for a case that fails at this head; as a plain test() it would have made #3570's CI permanently red, and a red lane on a draft blocks nothing except the next person's read of "is this layer green?". I marked it test.fails instead, so the lane is green now (reported as 1 expected fail) and vitest raises "expected to fail but passed" the moment #3565 makes arming flavour-independent — that failure is the signal to drop the marker, and the assertion is the rewrite's acceptance test. If you would rather have the honest red, say so and I will drop the marker in one line; I did not want to spend the shared macOS runner pool deciding that for you. Gate stays exit 0 either way (296 files / 2,050 tests + 1 expected fail, Fallow clean on 4 files, under the machine-wide lock at 11:58:40Z).

Evidence this will carry when it leaves draft — 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 press of close, (c) back --settle, (d) a sheet with ≥6 controls, each reporting settle.waitedMs >= 1500 and an empty diff snapshot -i taken 2 s later, plus one selector tab switch staying near 800 ms — the #3470 cost this layer exists to remove, asserted rather than assumed — and the two route regressions (settleAfterInteraction with a full-tree selector-resolution baseline, settleObservation over a stored full tree) both asserting waitedMs >= 1500. (d) matters specifically: it is the flavour the count bound loses today, so it must be measured under the producer fact, not inferred from (b). The parity case above is the unit-level stand-in for exactly those two route regressions; the device rows are what close it. Nothing in the current body's table is that evidence — those numbers are the before state on the count rule, and the body now says so.

And noted on your own verification boundary: the device rows here are mine and unreproduced, the navigation table and mutation check were read not run. The parity reproduction above is a unit run on the injected clock, not a device run — it establishes the arming divergence, not the wall-clock behaviour on a real dismissal.

@thymikee
thymikee force-pushed the fix/3470-presented-surface-arming branch from e43dd7a to 06a4c5b Compare October 13, 2026 13:32
@thymikee

Copy link
Copy Markdown
Member Author

The red Coverage lane at e43dd7abc was mine, and it was not the test.fails marker — recording it so the two are not conflated.

What actually failed was scripts/__tests__/test-file-size-ratchet.test.ts:

src/daemon/__tests__/generic-settle.test.ts is 1035 lines, 801 at the merge-base:
a test file over the tripwire may not grow; split it along the source module it
mirrors (docs/agents/testing.md).

The requested parity case is a good test in the wrong file. That suite was already 801 lines and the 1,000-line tripwire lets an over-tripwire test file hold its size, not grow, so adding 134 lines to it was never allowed. The same run reports 1 expected fail among 14,754 tests and no other failure, which also answers the question the deviation left open: test.fails behaves identically in CI, so the marker is not what reddened the lane.

Fixed the way the finding names rather than by shrinking the assertions: the three raw trees the case needs (the sheet as an interactive capture projects it, the same screen as a full read, and the app screen a dismissal lands on) now live in src/daemon/__tests__/support/presented-sheet-trees.ts, and both the case and the back --settle helper that drives it stay in the suite that owns the route. generic-settle.test.ts is 963 lines, the ratchet is green, and the measured numbers are unchanged — the interactive baseline still confirms for 1,502 ms and the full-tree baseline for 27 ms, settled: true either way. Local: 296 files, 2,050 tests + 1 expected fail, exit 0.

Re-pushed as the layer's single commit 06a4c5b86 (e43dd7abc...06a4c5b86, tests only — the rule and the stable-capture fixtures are byte-identical, so every device number in the description still describes this code). The draft stays draft until #3565, and the test.fails deviation and its offer stand as described above.

@thymikee

Copy link
Copy Markdown
Member Author

The new commit 06a4c5b only adds tests, so the arming defect from the earlier review (76bbe24) is still there, and the required simulator runs have not been taken. CI is pending: both Smoke Tests jobs are queued with no log yet, and since this PR changes iOS --settle arming, a later smoke failure could not be ruled unrelated. I did not run the generic-settle suite, did not reproduce the device timings, and could not check the rewrite against #3565/#3577 because they are not merged. The timings and the 1,502 ms / 27 ms split in the parity case are your numbers. The ratchet's "801 at merge-base" also disagrees with the 902 lines I see at 84f170e, and I did not run the ratchet script. No conflicts.

The main problem is in stable-capture.ts. showsPresentedSurface arms only when the baseline has 5 or fewer nodes or an Alert root. The same pageSheet projects 4 nodes interactive and 6 full, so snapshot then press @ref --settle, back --settle over a stored full tree, and any sheet with 6+ controls never arm the 1.5 s confirmation. In those cases iOS --settle can report settled:true on a coherent tree captured mid-dismissal, which removes the #1831 guard on 2 of the 3 measured routes. Your own table shows about 520 ms against about 1850 ms. The rule should be that arming is identical for every baseline flavour of one screen (interactive, full, depth-limited, scoped), and only a fact computed before projection can meet that. Once #3565/#3577 land, please arm unless baseline.presentedSurface === 'absent' ('unknown' keeps the #3512 arming), pass the baseline SnapshotState into runStableCaptureLoop, and delete showsPresentedSurface, isBlockingSystemAlertRoot and the count-bound arming. You have already agreed to this, so the next step is that #3565's presentedSurface fact lands and the arming is rewritten to read it.

This change also needs device proof on the rewritten rule, because the tables in the body show the state before it. Please run on an iPhone simulator with the RN test app, at least 4 reps each of: (a) selector press of close, (b) plain snapshot then @ref press of close, (c) back --settle with the sheet actually dismissed (use a device or sheet where back dismisses it), and (d) a sheet with 6+ controls. Each run needs settle.waitedMs >= 1500 and an empty diff snapshot -i taken 2 s later. Please add one selector tab switch that stays near 800 ms, so we know the 1.5 s floor does not leak into tab switches.

Not blocking: the test.fails case at generic-settle.test.ts:596 passes on any throw, so a regression of the interactive assertion (checked first) or a setup error would stay green, and it covers only the generic stored-tree route, not the baseline flavour choice in resolveSettleBaseline; you could assert the interactive baseline in a plain test, keep only the full-baseline assertion under test.fails, and drop the marker in the rewrite, but take or leave this as you like.

@thymikee
thymikee force-pushed the fix/3470-presented-surface-arming branch from 06a4c5b to d2d7d7b Compare October 14, 2026 12:45
@thymikee

Copy link
Copy Markdown
Member Author

The 06a4c5b findings are not fixed in d2d7d7b. The arming rule in stable-capture.ts is unchanged, and one blocking defect and one missing validation remain.

showsPresentedSurface still infers a presented surface from the baseline's projected node count (5 or fewer) or an Alert root (stable-capture.ts#L458). The same pageSheet projects 4 nodes from an interactive capture and 6 from a full one. So snapshot then press @ref --settle, back --settle over a stored full tree, and any sheet with 6 or more controls never arm the 1.5 s confirmation. They can return settled:true on a coherent mid-dismissal tree. The PR's own test.fails case at generic-settle.test.ts:598 measures this as 1,502 ms against 27 ms. On 2 of the 3 measured dismissal routes, iOS --settle loses the #1831 guard. The rule the code must satisfy is that arming is identical for every baseline flavour of one screen (interactive, full, depth-limited, scoped, stored ref frame), and every baseline that resolveSettleBaseline can return must go through one fact computed before projection. Once #3565/#3577 land, could you pass the baseline SnapshotState into runStableCaptureLoop and decide arming once in the constant half of broadTransitionConfirmation? Rule it out only when presentedSurface === 'absent', and let 'unknown' keep the #3512 arming. Then delete showsPresentedSurface, isBlockingSystemAlertRoot and the count-bound arming, and drop the test.fails marker.

This change alters device-facing iOS --settle arming, and no simulator run exists on the rewritten rule. The tables in the PR body predate it. After the rewrite, please run on an iPhone simulator with the RN test app, at least 4 reps of each case: (a) selector press of close; (b) plain snapshot then @ref press of close; (c) back --settle with the sheet actually dismissed; (d) a sheet with 6 or more controls. Each run should show settle.waitedMs of at least 1500 and an empty diff snapshot -i taken 2 s later. Please add one selector tab switch whose waitedMs stays near 800 ms, so we know tab switches stay off the 1.5 s floor.

I did not run the tests or reproduce the device timings, so the 1,502 ms and 27 ms figures are yours. I also could not check the rewrite against #3565/#3577, because presentedSurface is on neither main nor this head. Smoke Tests and Compatibility & Provenance were still running when I looked. Smoke Tests can exercise iOS --settle, so a later failure there would not be unrelated to this diff. Compatibility & Provenance has no overlap with this change. Before merge, #3565/#3577 must land presentedSurface, the arming must read it, and the simulator runs must be attached.

@thymikee
thymikee added this pull request to stack #3615 October 14, 2026 15:17
@thymikee
thymikee force-pushed the fix/3470-presented-surface-arming branch 4 times, most recently from 1a9743c to d037f58 Compare October 14, 2026 17:04
@thymikee
thymikee force-pushed the fix/3470-presented-surface-arming branch from d037f58 to 927d2ce Compare October 14, 2026 17:15
@thymikee thymikee changed the title DRAFT fix(settle): arm the confirmation on the dismissed surface, not on screen replacement fix(settle): arm the dismissal confirmation on the producer's presented-surface fact Oct 14, 2026
@thymikee

Copy link
Copy Markdown
Member Author

Rewritten on #3565's fact, out of draft — 927d2ce57

#3577–#3579 merged, so this layer now does what the last three reviews asked.

The rule. runStableCaptureLoop takes the baseline as SurfaceScopedNodes, so the fact arrives with its nodes. In the constant half of broadTransitionConfirmation, a baseline stamped absent rules the confirmation out for the run. presented admits a rootless first capture, as a tiny baseline already did. unknown, or no fact, keeps #3522's arming unchanged. showsPresentedSurface, isBlockingSystemAlertRoot, and the count bound used as an arming rule are deleted.

Tests. The test.fails marker is gone. The parity case dismisses one sheet from its interactive (4-node) and full (6-node) baselines, both stamped presented, into rootless captures, and asserts both hold ≥1500 ms. It fails on #3522's rule (27 ms). New cases carry the fact through every route: selector, @ref, and generic back each stamped absent, and each fails if the fact is ignored. The daemon-route dismissal in settle-observation.test.ts still asserts waitedMs >= 1_500, unchanged from main.

A finding from self-review, fixed. Take snapshot (home, absent), then a press that opens a sheet without --settle, then back --settle. The generic route would have trusted the stale absent and dropped the confirmation. A stored tree that a side effect outdated now loses its fact. So does the point-press fallback, since that layer cannot see staleness. Both cases are pinned.

Live evidence. The table in the body shows 4 reps each of (a)–(e) at this head, all armed, with an empty late diff 2 s after each response. The tab switch takes 627–778 ms here against 1829–1872 ms on #3522's build. Two route notes:

  • (c) and (d) ran on Contacts' New Contact sheet. The lab sheet has one control, and back is unavailable on it ("in-app back control is not available"; back --system leaves it up).
  • (d) used a coordinate press, because selector and @ref presses of that sheet's close are refused as covered. That refusal is an unrelated actionability guard.

I also checked whether a full-screen modal could be stamped absent. The lab's fullScreenModal route reads unknown, so it keeps the confirmation.

Stack. #3522 was 19 commits behind main and lacked #3577. I rebased its own commits onto main unchanged; its worker handed it to this thread.

🤖 Generated with Claude Code

@thymikee
thymikee marked this pull request as ready for review October 14, 2026 17:32

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

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

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

Comment thread packages/command-registry/src/flag-definitions-action.ts
Base automatically changed from fix/3470-settle-quiet-per-capture-backend to main October 14, 2026 18:12
@thymikee
thymikee force-pushed the fix/3470-presented-surface-arming branch from 453221d to 93b8220 Compare October 14, 2026 18:12
thymikee and others added 2 commits October 14, 2026 20:35
…ed-surface fact

A tab switch or a push replaces the screen broadly, so main's arming charged ordinary navigation the
1.5 s confirmation floor (#3470). The settle baseline now carries the presentedSurface fact its
producer stamped before projection (#3565), and the confirmation reads it:

- `absent` rules the confirmation out for the run: nothing presented, no dismissal to confirm.
- `presented` admits a rootless first capture after the dismissal, as a tiny baseline already did.
- `unknown` or no fact keeps the parent's arming, so the fact only ever narrows the guard.

Because the fact is stamped before projection, interactive and full-tree baselines of one sheet arm
alike, which the node-count draft of this layer could not do.

Closes #3470

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@thymikee
thymikee force-pushed the fix/3470-presented-surface-arming branch from 93b8220 to fea0bb9 Compare October 14, 2026 18:35
@thymikee

Copy link
Copy Markdown
Member Author

This PR is ready. At fea0bb9, both findings from the earlier review (#3570 (comment)) are fixed. CI is green: 19 checks, none failing, including the smoke lanes that exercise iOS --settle. No conflicts. Nothing else stands in the way of a maintainer merge.

Not blocking: sessionBaseline in settle.ts strips the presented-surface fact unconditionally, while generic-settle.ts:97 checks isOutdatedObservation. If createRuntimeSessionRecord dropped the fact when the observation is outdated, both routes would read one daemon-owned answer and both strips could go. You can take this or leave it.

The Cubic thread on the help text (#3570 (comment)) is fixed at this head, so you can resolve it.

I did not reproduce the simulator timings in the PR body. They were taken at 927d2ce, and the patch differs from fea0bb9 only in help text, so they apply to this head. I did not run the tests. I judged the regression from reading the old arming path. I did not re-audit the producer's "absent" fact across baseline types, since it is merged upstream and this PR trusts it. Baselines that read "unknown" still use the count-based arm, as the earlier review accepted.

@thymikee thymikee added the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Oct 14, 2026

This branch has not been deployed

No deployments
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.

fix(settle): iOS broad-transition confirmation fires on tab switches and pushes and is coupled to the default quiet window

1 participant