Skip to content

test(core): the glide keeps advancing through a blocked main thread - #10229

Merged
cixzhang merged 10 commits into
meta:mainfrom
vjeux:uikit/f1-glide-busy-proof
Nov 6, 2026
Merged

cixzhang merged 10 commits into
meta:mainfrom
vjeux:uikit/f1-glide-busy-proof

Conversation

@vjeux

@vjeux vjeux commented Nov 6, 2026 •

Copy link
Copy Markdown
Contributor

This pull request's own commits: vjeux@3e3009c364 and vjeux@5d50028a14 (the proof), vjeux@cb9783ba97 and vjeux@000ab1957f (the record row), and vjeux@904df990a8 (each frame decoded once). The glide it judges plays the theme's curve tokens, on main since #10236.

A new browser proof shows that the animated-layout glide keeps advancing while the page's main thread is blocked, as spec:AST-094 FR5 requires. It covers the glide's adopters on main: the content after a Collapsible, after a TreeList branch and after a Table row. This is a test-only change.

Maintainer impact

FR5 says a motion keeps advancing through a main-thread block. The springs, BottomSheet's settle and other motions have a busy-main-thread proof. The animated-layout glide had none, so a change that moved it onto the main thread would pass every test.

Intended invariant

While the page blocks its main thread for 700 ms in the middle of a glide, the gliding content shows a new position on every frame where the harness's compositor control moves.

Change

  • animatedLayoutBusy.interaction.browser.spec.ts: each case opens a story, marks the follower, starts the harness's compositor control and a screencast, and clicks the trigger.
  • The page blocks itself for 700 ms from a task queued in the first frame after the click, so the glide is under way when the block starts.
  • The harness's busyWindowProof judges the follower's frames against the control's. The judged part of the block ends where the glide's own keyframes move the follower by less than one cast step per frame (1 px in Chromium's cast). After that point, an easing tail moves less than the cast can show. No threshold is widened.
  • The glide's length is set to 1000 ms, so the screencast has about 20 frames to judge in each case. This changes nothing the proof judges. It is set through both --duration-medium-min and --duration-medium, so the proof holds before and after fix(core): the layout glide takes --duration-medium #10261 moves the glide to --duration-medium. Once fix(core): the layout glide takes --duration-medium #10261 lands, the --duration-medium-min line goes.
  • Each frame is decoded once, for the marker and the control together (the harness's new trackMarkers), and only the frames the proof reads are decoded: up to BUSY_PROOF_LOOKAROUND_MS (250 ms, now a named harness constant) past the judged part. Two decodes of every frame ran past the test's 60 s budget on a loaded machine.
  • spec:AST-094: its Verification gains an FR5 row for this proof, and verified_by links the file, so pnpm specs:for on it finds the claim it verifies. The row says that WebKit rests on an iOS Simulator spot-check (38 of 39 frames), not an automated run: an evidence gap. This record edit needs the spec owners' stamp.
  • WebKit is skipped, with the reason in the file. Linux WebKit starts the glide on its compositor 30 to 100 ms late, so too little of the curve is left to judge. It also runs a table row's translate on the main thread. Apple's WebKit does neither (Evidence).

Evidence

  • Failure without the property: a not-for-landing mutant pauses every animated-layout animation and steps it from requestAnimationFrame (vjeux@26a9b1b588). All 3 cases fail 2 of 2. The follower moved on 0 of the 21 to 23 frames the control moved, and stalled 347 to 402 ms against the control's 18 to 23 ms.

  • Success: Chromium 15 of 15 (5 repeats). Also 9 of 9 on a 2-core pin with 1 worker (3 repeats), the repository's busy-proof setup. Each case judges about 20 frames, and the follower moves on every one the control moves.

  • Apple's WebKit, measured by hand on the iOS 26.2 Simulator with the same 700 ms block, a compositor control and a 60 fps capture:

    • The Table row moved on 38 of the 39 frames inside the block.
    • The Collapsible follower moved on 39 of 40.
    • The one repeated frame in each repeated the control too: a capture hiccup.
  • The decode change keeps the verdict exact. A not-for-landing arm computes both paths on the same frames (vjeux@172ea7615e), and the proofs were identical in 29 of 29 runs, 14 on the host and 15 in a container held to 4 CPUs with 8 busy loops beside the browser. Per case, the median decode count fell from 136 to 41 frames on the host, and from 102 to 26 in the container. The median decode time fell from 1.28 s to 0.52 s on the host, and from 3.6 s to 1.6 s in the container, where its longest fell from 20.2 s to 3.9 s.

  • Product/runtime behavior verified unchanged: test-only.

  • Next adopter, as a follow-up: AppShell's rail collapse. In the phone story, a marker in the content stays visible for about 170 ms of the glide, then the collapsing rail covers it. The follow-up needs a marker spot that stays clear. Resizable's close and spring-back follow once their pull request lands.

Scope

  • No intended public API, product behavior, visual direction, or policy change.
  • Product changes discovered during the work were removed or split.
  • Public text and artifacts contain no internal Meta context.

Writing rubric results

  • R1 narration: none; the new FR5 row states what the proof checks and its WebKit evidence gap, in the present tense.
  • R2 research: none in the record.
  • R3 evidence: the row names the fact it verifies (the glide moving through a block), no pull request or commit.
  • R4 rejected alternatives: none touched.
  • R5 change references: none added.
  • R6 open questions: none touched.

Testing

  • animatedLayoutBusy.interaction.browser.spec.ts: Chromium 15 of 15 (5 repeats), 9 of 9 on a 2-core pin with 1 worker; WebKit skipped by design.
  • The requestAnimationFrame mutant: 6 of 6 red.
  • The harness's own units: 173 of 173.
  • CI=true lint and Prettier on the changed files; pnpm check:knowledge.

vjeux added 5 commits November 5, 2026 19:23
The animated-layout glide, the in-place swap and the edge reveal played
the settle spring, about 735 ms, so a theme's `character` never reached
them, and on a mouse they kept moving after the motion around them had
landed. They now play the theme's move curve, `--ease-move`, over
`--duration-medium-min`, read where the area is through the shared
theme-timing reader. The tokens carry the theme's `character` and
`tempo`, so nothing scales them twice. A zero duration lands the change
at once. Reduce Motion is unchanged: a cross-fade under a coarse
pointer, an immediate change otherwise.

A running glide is now read as the browser draws it, a straight line
between two of its keyframes, so a new glide starts exactly where the
old one shows. The curve bends more than the spring did where a
retarget lands, and reading the true curve there left a jump of up to
about 0.3 px on an 80 px move.

A disclosure's indicator takes the same curve and duration as CSS
values. The motion's kind is now named for what it is, a glide, in
AppShell and SideNav's rail too, which share it. The record's line that
the glide still plays the spring goes, since FR4 now holds.

The glides of AppShell's content area, Collapsible, Table's row
expansion and TreeList change feel: 225 ms in the Neutral theme and
310 ms with no theme, instead of about 735 ms.
The reader took core's declared timing values from the token file,
which declares its variables through StyleX. Importing it ran
`stylex.defineVars`: DOM work under the test runtime, and an error in a
plain Node process. Since the glide reads the theme's timing, the
animated-layout module and every factory that imports it took that on.

The reader now keeps a typed copy of the timing tokens' declared
values, which a unit test holds equal to the token file's, and imports
only the token names, as types. A server test imports the reader and
the animated-layout module in plain Node.
A theme's move easing can be a linear() or steps() function: the motion
scale's easing option writes one. The theme-timing reader read only a
cubic Bézier, so such a theme gave no curve and the change landed at
once. The reader now plays any other easing as the browser plays it: a
paused animation with that easing gives its eased progress. A jump in
such an easing carries no speed, so a retarget at a jump inherits none.
The segmented control's selection reads its timing the same way, so it
plays such an easing too.
The animated-layout glide runs on the compositor (AST-094 FR5), but no
proof read it while the main thread is blocked. This one blocks the page
for 700 ms across a glide and judges the screencast against the
harness's compositor control: the content after a Collapsible, after a
TreeList branch and after a Table row keeps moving on the frames the
control moves.

The judgment reads the block while the glide's own keyframes move the
follower by a cast step or more per frame; an easing tail moves under
what the cast can show. Chromium only: Linux WebKit starts the glide on
its compositor 30 to 100 ms late, while Apple's WebKit (the iOS 26.2
Simulator) moves on every frame from the block's start.
FR5 holds that motion runs on the compositor and keeps advancing through
a main-thread block. Its verification gains a row for the
animated-layout glide's busy-main-thread proof, and the record links the
proof's file, so a lookup on that file finds the claim it verifies.
@vercel

vercel Bot commented Nov 6, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
astryx Ready Ready Preview Nov 6, 2026 7:29pm UTC

Request Review

@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Meta Open Source bot. label Nov 6, 2026
@astracat-bot

astracat-bot Bot commented Nov 6, 2026 •

Copy link
Copy Markdown
Contributor

AI review status for this pull request.

Review status Updated
🟢 Reviewed (for maintainers only) Nov 6, 2026, 7:42 PM UTC

@github-actions github-actions Bot added the needs:spec-owner-review Current knowledge records await owner approval label Nov 6, 2026
@github-actions

github-actions Bot commented Nov 6, 2026 •

Copy link
Copy Markdown
Contributor

PR Analysis Report

🔎 Preview · 83344c2

Storybook · Sandbox · built from this PR's head

Package size: measured; main has no measurement of the merge base to compare yet.

API changes: cli +14 · ~5 · −0 (against the merge base's committed report)

Committed API report behind this build: cli 19 rows, core 24 rows; run pnpm api:report.

19 API changes (advisory)
Package Where Signature Usage
+ cli ./api · Types · type SearchResultEntry deprecated?: ComponentDeprecation | undefined import type {SearchResultEntry} from '@astryxdesign/cli/api'; const x: SearchResultEntry = {deprecated: …}
+ cli ./authoring · Types · interface ComponentDeprecation interface ComponentDeprecation import type {ComponentDeprecation} from '@astryxdesign/cli/authoring'
+ cli ./authoring · Types · interface ComponentIconSlotDoc interface ComponentIconSlotDoc import type {ComponentIconSlotDoc} from '@astryxdesign/cli/authoring'
+ cli ./authoring · Types · interface ComponentIconSlotStateDoc interface ComponentIconSlotStateDoc import type {ComponentIconSlotStateDoc} from '@astryxdesign/cli/authoring'
+ cli ./authoring · Types · interface MultiComponentDoc deprecated?: ComponentDeprecation import type {MultiComponentDoc} from '@astryxdesign/cli/authoring'; const x: MultiComponentDoc = {deprecated: …}
+ cli ./authoring · Types · interface SingleComponentDoc deprecated?: ComponentDeprecation import type {SingleComponentDoc} from '@astryxdesign/cli/authoring'; const x: SingleComponentDoc = {deprecated: …}
+ cli ./doc · Types · interface ComponentBaseDoc deprecated?: ComponentDeprecation import type {ComponentBaseDoc} from '@astryxdesign/cli/doc'; const x: ComponentBaseDoc = {deprecated: …}
+ cli ./doc · Types · interface SubComponentDoc deprecated?: ComponentDeprecation import type {SubComponentDoc} from '@astryxdesign/cli/doc'; const x: SubComponentDoc = {deprecated: …}
+ cli ./doc · Also exported from an earlier entry point · ComponentDeprecation ComponentDeprecation (./authoring) import {ComponentDeprecation} from '@astryxdesign/cli/doc'
+ cli ./doc · Also exported from an earlier entry point · ComponentIconSlotDoc ComponentIconSlotDoc (./authoring) import {ComponentIconSlotDoc} from '@astryxdesign/cli/doc'
+ cli ./doc · Also exported from an earlier entry point · ComponentIconSlotStateDoc ComponentIconSlotStateDoc (./authoring) import {ComponentIconSlotStateDoc} from '@astryxdesign/cli/doc'
+ cli ./template · Also exported from an earlier entry point · ComponentDeprecation ComponentDeprecation (./authoring) import {ComponentDeprecation} from '@astryxdesign/cli/template'
+ cli ./template · Also exported from an earlier entry point · ComponentIconSlotDoc ComponentIconSlotDoc (./authoring) import {ComponentIconSlotDoc} from '@astryxdesign/cli/template'
+ cli ./template · Also exported from an earlier entry point · ComponentIconSlotStateDoc ComponentIconSlotStateDoc (./authoring) import {ComponentIconSlotStateDoc} from '@astryxdesign/cli/template'
~ cli ./api · Functions · function search function search: (query: string, options?: { cwd?: string | undefined; type?: "template" | "component" | "hook" | "theme" | "doc" | undefined; limit?: number | undefined }) => Promise<SearchResponse> → function search: (query: string, options?: { cwd?: string | undefined; type?: "template" | "component" | "doc" | "theme" | "hook" | undefined; limit?: number | undefined }) => Promise<SearchResponse> import {search} from '@astryxdesign/cli/api'; search(…)
~ cli ./authoring · Types · interface MultiComponentDoc theming?: { container?: boolean; targets: ComponentThemingTarget[]; vars?: ComponentThemingVar[]; derived?: ComponentThemingDerivedVar[] } → theming?: { container?: boolean; targets: ComponentThemingTarget[]; vars?: ComponentThemingVar[]; derived?: ComponentThemingDerivedVar[]; componentIcons?: ComponentIconSlotDoc[] } import type {MultiComponentDoc} from '@astryxdesign/cli/authoring'; const x: MultiComponentDoc = {theming: …}
~ cli ./authoring · Types · interface SingleComponentDoc theming?: { container?: boolean; targets: ComponentThemingTarget[]; vars?: ComponentThemingVar[]; derived?: ComponentThemingDerivedVar[] } → theming?: { container?: boolean; targets: ComponentThemingTarget[]; vars?: ComponentThemingVar[]; derived?: ComponentThemingDerivedVar[]; componentIcons?: ComponentIconSlotDoc[] } import type {SingleComponentDoc} from '@astryxdesign/cli/authoring'; const x: SingleComponentDoc = {theming: …}
~ cli ./doc · Types · interface ComponentBaseDoc theming?: { container?: boolean; targets: ComponentThemingTarget[]; vars?: ComponentThemingVar[]; derived?: ComponentThemingDerivedVar[] } → theming?: { container?: boolean; targets: ComponentThemingTarget[]; vars?: ComponentThemingVar[]; derived?: ComponentThemingDerivedVar[]; componentIcons?: ComponentIconSlotDoc[] } import type {ComponentBaseDoc} from '@astryxdesign/cli/doc'; const x: ComponentBaseDoc = {theming: …}
~ cli ./doc · Types · interface SubComponentDoc theming?: { container?: boolean; targets: ComponentThemingTarget[]; vars?: ComponentThemingVar[]; derived?: ComponentThemingDerivedVar[] } → theming?: { container?: boolean; targets: ComponentThemingTarget[]; vars?: ComponentThemingVar[]; derived?: ComponentThemingDerivedVar[]; componentIcons?: ComponentIconSlotDoc[] } import type {SubComponentDoc} from '@astryxdesign/cli/doc'; const x: SubComponentDoc = {theming: …}

Not run on this PR: add ci:run-visual, ci:run-a11y or ci:run-rtl to run them.


Generated by PR Enrichment workflow | View full report

The row's proof runs in Chromium. WebKit rests on a spot-check on the
iOS Simulator, not an automated run, and the row says so, so FR5 does
not read as fully covered.
@cixzhang

cixzhang commented Nov 6, 2026

Copy link
Copy Markdown
Contributor

/approve-spec 000ab19

@github-actions github-actions Bot removed the needs:spec-owner-review Current knowledge records await owner approval label Nov 6, 2026

@astracat-bot astracat-bot 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.

Changes requested: one item stands between this pull request and approval. The busy-main-thread proof itself holds up: it passes on this head and fails when the glide is stepped from requestAnimationFrame. The spec:AST-094 row has an engineering owner's /approve-spec for this head.

  • Desktop behavior change needs its evidence here: this diff also carries #10094's glide retiming for mouse and trackpad users (packages/core/src/motion/animatedLayout.ts:137-172, about 735 ms becoming 225 ms), but the description calls the change test-only, so please cite the spec:AST-105 FR4 line that main violates and add the desktop before and after, or put the same Stack: #10094 → #10229 line in both descriptions, or land #10094 first so this diff carries only the proof and the record row.

CI's test jobs are skipped while the pull request is a draft.

[Automated review]

The page reads the glides into an array declared with no type, which
the type check rejects as an implicit any. It now holds GlideRead
entries, each keyframe pair typed as a pair.
@cixzhang

cixzhang commented Nov 6, 2026

Copy link
Copy Markdown
Contributor

/approve-spec 5d50028

@astracat-bot astracat-bot 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.

Changes requested: two items block this pull request, and both come from this branch sitting on an earlier #10094 commit.

  • Desktop behavior change needs an owner decision: the diff retimes the glide for mouse and trackpad users (packages/core/src/motion/animatedLayout.ts:137-172, about 735 ms becoming 225 ms), so gate it to touch input or cite the decision.
  • Miss: CI's WebKit interaction routing test fails at this head, because animatedLayout.ts now brings packages/core/src/theme/tokenReferences.ts into the WebKit proofs' bundle and the pr-interaction-webkit scope in .github/workflows/ci.yml leaves that file out.

The busy-main-thread proof itself holds up: it passes on this head and fails when the glide is stepped from requestAnimationFrame. The Stack: #10094 → #10229 line does not hold yet, because this head lacks #10094's last commit, so this pull request was reviewed on its own. Restacking onto main once #10236 lands leaves only the proof and the record row in this diff and clears both items.

[Automated review]

@github-actions github-actions Bot added the needs:spec-owner-review Current knowledge records await owner approval label Nov 6, 2026
@cixzhang

cixzhang commented Nov 6, 2026

Copy link
Copy Markdown
Contributor

/approve-spec d5b2531

@github-actions github-actions Bot removed the needs:spec-owner-review Current knowledge records await owner approval label Nov 6, 2026

@astracat-bot astracat-bot 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.

Approved pending CI: CI has not run on this head yet. It runs once the pull request is marked ready for review and a maintainer adds the ci:run label.

The busy-main-thread proof holds up: it passes on this head in Chromium and fails when the glide is stepped from requestAnimationFrame. The spec:AST-094 FR5 row has an engineering owner's /approve-spec for this head, and both earlier items left the diff with the restack onto main.

[Automated review]

The busy proof decoded every frame of its screencast twice, once for
the marker and once for the control. On a loaded machine that decoding
ran past the test's 60 s budget. The harness's trackMarkers finds
several colors in one decode of each frame, and the proof decodes only
the frames busyWindowProof reads, up to BUSY_PROOF_LOOKAROUND_MS past
the judged part. The verdict is unchanged. The glide's length is set
through both short medium durations, whichever the module plays.

@astracat-bot astracat-bot 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.

Approved pending CI: CI has not run on this head yet. It runs once the pull request is marked ready for review and a maintainer adds the ci:run label.

The busy-main-thread proof still holds after the decode change: it passes in Chromium, fails when the glide is stepped from requestAnimationFrame, and the old and new decode paths give the same verdict on the same frames. The spec:AST-094 FR5 glide row is unchanged since an engineering owner approved its content.

[Automated review]

@cixzhang

cixzhang commented Nov 6, 2026

Copy link
Copy Markdown
Contributor

/approve-spec 904df99

@vjeux
vjeux marked this pull request as ready for review November 6, 2026 12:08
@cixzhang cixzhang added the ci:run A maintainer lets this outside contributor's pull request run CI (AST-030 FR17) label Nov 6, 2026
@cixzhang
cixzhang marked this pull request as draft November 6, 2026 12:24
@cixzhang
cixzhang marked this pull request as ready for review November 6, 2026 12:24
@github-actions github-actions Bot added the needs:spec-owner-review Current knowledge records await owner approval label Nov 6, 2026
@cixzhang

cixzhang commented Nov 6, 2026

Copy link
Copy Markdown
Contributor

/approve-spec 83344c2

@github-actions github-actions Bot removed the needs:spec-owner-review Current knowledge records await owner approval label Nov 6, 2026

@astracat-bot astracat-bot 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.

Approved. CI passes on this head.

The busy-main-thread proof passes in Chromium and fails when the glide is stepped from requestAnimationFrame, and the spec:AST-094 FR5 glide row has an engineering owner's approval for this head. Two small non-blocking cleanups are left as inline comments: the placement of BUSY_PROOF_LOOKAROUND_MS between busyWindowProof and its JSDoc, and the --duration-medium-min override, which nothing reads now that #10261 is in this branch.

[Automated review]

Comment on lines +869 to +874
/**
* How far around its judged part a busy proof reads frames, ms: the rest
* before it and the frame cadence on both sides. A proof that decodes only
* part of a cast keeps the frames this far after the window.
*/
export const BUSY_PROOF_LOOKAROUND_MS = 250;

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.

Miss (non-blocking): this constant now sits between busyWindowProof's JSDoc (lines 773–868) and the function at line 876. The constant carries the verdict's documentation and busyWindowProof has none. Moving the constant and its comment above line 773, after motionStepPx, keeps both documented.

.first()
.waitFor({state: 'attached'});
await page.addStyleTag({
content: `* { --duration-medium-min: ${GLIDE_MS}ms !important; --duration-medium: ${GLIDE_MS}ms !important; }`,

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.

Regression (non-blocking): #10261 is now in this branch through the merge from main, so the glide reads --duration-medium (packages/core/src/motion/animatedLayout.ts:147), and nothing in these stories reads --duration-medium-min. As the description plans, the --duration-medium-min half of this rule and the "whichever the module plays" note on GLIDE_MS can go.

@cixzhang
cixzhang marked this pull request as draft November 6, 2026 20:42
@cixzhang
cixzhang marked this pull request as ready for review November 6, 2026 20:42
@cixzhang
cixzhang merged commit b07a832 into meta:main Nov 6, 2026
133 of 134 checks passed
@vjeux
vjeux deleted the uikit/f1-glide-busy-proof branch November 6, 2026 21:21

This branch was successfully deployed

1 active deployment
Preview — 83344c21 Deployed Nov 6, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci:run A maintainer lets this outside contributor's pull request run CI (AST-030 FR17) CLA Signed This label is managed by the Meta Open Source bot.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants