Skip to content

fix: eliminate deferred caret and e2e timing races - #249

Merged
MerciHanrim merged 1 commit into
mainfrom
fix/release-e2e-timing-races
Sep 21, 2026
Merged

MerciHanrim merged 1 commit into
mainfrom
fix/release-e2e-timing-races

Conversation

@MerciHanrim

Copy link
Copy Markdown
Owner

Product impact

The Register expression field (RegisterExprField) restored the caret on a bare
requestAnimationFrame after an autocomplete pick. A frame is not a bounded delay: with the main
thread busy — a loaded machine, a heavy graph, modest hardware — the pick's frame was measured
landing 0.9–4.8 s after the click. Until it lands the caret is wherever the DOM left it, and when
it finally runs it replays a caret position captured at click time, whatever the field contains by
then.

So a user who picks Wallet from the @ list and starts typing gets the rest of their input pushed
to the old position. Holding the frame and releasing it after the Nth keystroke makes this exact:

frame released after input value stored expr
keystroke 1 "@wallet+ 10 " "@wallet+ 10 "
keystroke 2 "@wallet 10 +" "@wallet "
keystroke 3 "@wallet10 + " "@wallet "
keystroke 4 "@wallet0 + 1" "@wallet0 + 1"
keystroke 5 "@wallet + 10" "@wallet + 10"

Rows 2 and 3 are worse than reordered text: the field shows one expression while the model stores
"@wallet ", a silent divergence between what the user sees and what is committed. A user who has
already stopped typing gets the other half of the defect — the caret silently jumps back mid-string,
so their next character lands in the middle of the expression.

The §RXA8 arm-and-click canvas insert had the same defect class, with
requestAnimationFrame(refocus) and setTimeout(refocus, 0): a user who moved to another
control after the insert had focus pulled back out from under them.

The fix

One caret path, consumed by the commit:

  • caretReqRef holds { gen, caret }; caretGenRef is a monotonic generation.
  • requestCaret(caret) bumps the generation, stores the request, and mirrors the generation into
    state.
  • a useLayoutEffect on [draft, caretGen] consumes it: it drops any request that is not the newest
    generation, clears the ref first, then calls focus() and setSelectionRange() exactly once.
  • cancelCaret() runs in the input's onChange, so a real keystroke invalidates a request made
    before it. A stale caret can no longer be replayed over what the user typed.

A layout effect rather than a passive one: it runs inside the commit that carries the new value, so
there is no window at all between the value landing and the caret being placed. Clearing the ref
before applying it also makes the effect safe under StrictMode's re-invocation.

Removed: the bare requestAnimationFrame in pick(), and the
requestAnimationFrame + setTimeout(0) double refocus in the §RXA8 consume effect. applyOp()
(§RXA8b) moved from the old single-purpose pendingCaretRef onto the same call.

Identical-string picks. Choosing the row that is already written produces the same text, so
draft does not change. The generation is what makes the effect run anyway, which keeps the caret
behaviour the previous animation frame provided.

The two e2e files

Both were waiting for a weaker state than they asserted, and are brought onto the same rule: wait for
exactly the state the assertions need, then assert the sample that satisfied it.

  • e2e/edge-routing.spec.ts — the poll exited as soon as the token was on the path in travel, which
    is already true at the first instant of travel while the token still sits on the source; only then
    did it assert the token was mid-path. The poll now tests the full predicate and the assertions run
    on the sample it accepted.
  • e2e/data-import-guide.spec.tsUse this example focuses the new card on the next animation
    frame, and the same frame scrolls the dialog body to reach it, so a scrollTop = 0 reset applied
    before that frame was undone and the quick start left the dialog's clip. The test now waits for the
    card's name input to be genuinely focused, then resets the scroll, then polls the containment
    itself through a new clippedInsideDialog() helper that keeps the boxes that passed. The error
    scene uses the same helper, which also removes a latent bug: it was comparing against the
    quick-start scene's dialog box, measured before the issue list grew the dialog.

red-first

Both new behavioural tests were run against the unmodified product first.

test on main with the fix
§RXA9.3b — release the held frame after the user has typed FAIL — caret {start: 7, end: 7}, expected {12, 12} pass
§RXA8.10 — insert, then the user moves to another field FAILexpect(locator).toBeFocused() received inactive pass

A first draft of §RXA9.3b froze animation frames and checked the caret; it passed on the old code
too, because React's own value write already leaves the caret at 7. Holding the frame and releasing
it after the typing is what separates the two implementations.

The data-import-guide race was pinned the same way, with a throwaway spec that delayed every frame
by 400 ms: the old sequence reproduced toggle.y = -86 against dlg.y = 90, the exact number the
suite failed with; the new sequence held at 144. All throwaway specs were deleted.

New regression coverage (4 tests)

  • §RXA9.3 (updated) — the pick is not finished when the value lands; the test waits for focus and
    caret 7/7 before typing.
  • §RXA9.3b (new) — nothing the pick defers may touch the caret after the user has typed: frames
    are held, the pick settles, the user types, the frames are released, and the caret, the value, the
    stored expr, the AST and the loop-revision/2 digest must all still match hand-typing.
  • §RXA9.3c (new) — picking the row that is already written puts the caret back even though the
    text did not change, and the next characters land there.
  • §RXA8.10 (new) — the canvas insert refocuses the input inside the commit, and after the user
    moves to another field nothing pulls focus back; every animation frame and zero-delay macrotask
    scheduled in between is held and then released before the assertion.

No fixed sleeps, no raised timeouts, no fill() substituted for real typing — the tests still
exercise the real typing path and the AST/digest equality.

Verification, including the runs that failed

Three full suite runs were made before this change. Reporting them as they were:

run result failure
1 1049 passed / 1 failed / 7 skipped register-expr-authoring.spec.ts:97Expected "@wallet + 10" / Received "@wallet+ 10 "
2 1049 passed / 1 failed / 7 skipped data-import-guide.spec.ts:393expect(b.y).toBeGreaterThanOrEqual(dlg.y), expected >= 90, received -86
3 1049 passed / 1 failed / 7 skipped register-expr-authoring.spec.ts:97 again, the same signature as run 1

None of those three was a pass. The register-expr-authoring failure appeared in two of the three,
which is what took it out of "known flake" and into this change.

Final undisturbed run, after the fix:

1053 passed / 0 failed / 7 skipped   (22.0 min, 1060 discovered)

Other gates: npx tsc -b, oxlint (0 errors, no new warnings), check:icons, check:mobile-query,
check:i18n, check:i18n-surface, check:template-labels, check:snapshot-policy, vitest
(131 files / 1991 tests), and a targeted regression of every file that touches the expression field
or the two stabilised specs (88/88).

Scope

  • 5 files: 1 product, 4 e2e.
  • 0 baseline images changed; the snapshot policy is unchanged at 52 baselines / 33 snap()
    calls
    .
  • No product documentation change: §RXA8/§RXA9 specify where the caret ends up, not which scheduler
    puts it there, and the observable contract is unchanged — it is now also kept when a frame is late.
  • No file-format, schema or migration change. No i18n strings added, removed or edited.

🤖 Generated with Claude Code

The Register expression field restored the caret on a bare animation frame
after an autocomplete pick. A frame is not a bounded delay — with the main
thread busy it was measured landing 0.9-4.8 s after the click — so the stale
caret could arrive after the user had started typing and push every later
character to the old position. `@wallet` followed by ` + 10` came out as
`@wallet+ 10 `, and two of the interleavings left the stored expression
disagreeing with the one on screen.

Every programmatic caret move now goes through one request that the commit
consumes: a generation-stamped ref plus a layout effect, so the caret is placed
inside the commit that carries the new value. The bare rAF in `pick()` and the
`requestAnimationFrame` + `setTimeout(0)` pair in the §RXA8 canvas insert are
gone; the generation keeps the caret working when an insert reproduces the
existing text, and a real keystroke cancels any pending request.

Two e2e files were waiting for a weaker state than they asserted and are
brought onto the same rule — wait for exactly the state the assertions need,
then assert the sample that satisfied it.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@cloudflare-workers-and-pages

Copy link
Copy Markdown

Deploying cozy-loop-studio with  Cloudflare Pages  Cloudflare Pages

Latest commit: bb14b0d
Status: ✅  Deploy successful!
Preview URL: https://dd4b2e44.cozy-loop-studio.pages.dev
Branch Preview URL: https://fix-release-e2e-timing-races.cozy-loop-studio.pages.dev

View logs

@MerciHanrim
MerciHanrim merged commit 0447f7e into main Sep 21, 2026
9 checks passed
@MerciHanrim
MerciHanrim deleted the fix/release-e2e-timing-races branch September 21, 2026 03:48
MerciHanrim added a commit that referenced this pull request Sep 21, 2026
package.json / package-lock.json -> 0.12.0; CHANGELOG `## Unreleased` ->
`## v0.12.0 — 2026-09-21` covering the whole v0.11.0..ed8ffd8 range (17
commits, 17 merged PRs: #236 #237 #238 #239 #240 #241 #242 #243 #244 #245 #246
#247 #248 #249 #250 #251 #252) as Added / Changed / Fixed / Internal; README:
the Visual diagram editor bullet gains a frames clause and `Latest — v0.12.0`
replaces the v0.11.0 block.

No product code and no baseline. The save format, schema and wire shape are
untouched across the range, so there is no migration and no breaking change;
the only version-dependent stored value is the informational `meta.tool`
string.

Verified on this branch: tsc -b clean, oxlint 0 errors, unit 131 files / 1,991
tests, the six check:* gates ok (snapshot policy 52 baselines / 33 snap()),
web + portable + PWA builds with 0.12.0 stamped in all three artefacts
(meta.tool, desktop About, mobile sheet stamp, toolbar title), check:no-pwa and
check:pwa-closure ok, and one undisturbed local full e2e: 1,081 discovered /
1,074 passed / 0 failed / 7 skipped, 0 baselines changed.

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant