Repository navigation
🤖 perf: never leave the static first-run view over the token prompt, an error or a crash (T3 C step 2 PR 1a) - #6378
Merged
Merged
Conversation
…tep 2 PR 1a) Test only, green on main: the entry registers the service worker on the window load event.
…an error or a crash (T3 C step 2 PR 1a) Adds the shared #xum-pre id and removeFirstRunView. AppLoader removes the view on auth_required, error and reconnecting. A boundary around AppLoader removes it on an uncaught render error, then renders the empty root such an error leaves today and reports the error. The entry registers the service worker at once when it runs after load. Inert without #xum-pre.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
ThomasK33
added this pull request to stack #6382
October 13, 2026 20:04
ThomasK33
changed the title
yermakoffivan
pushed a commit
to yermakoffivan/mux
that referenced
this pull request
Oct 14, 2026
…, and cache a viewed wizard (T3 C step 2 PR 1b) (coder#6379) ## Summary `SplashScreenProvider` removes the static first-run view (`#xum-pre`) once its splash queue is known and the onboarding wizard is not first. It also records a new paint cache, so the server's early script (a later PR) can skip its API calls for a browser that has seen the wizard marked viewed. Nothing inserts `#xum-pre` yet. **Stack** (T3 coder#5971, Option C step 2, PR 1 split at the 300-line limit into 3 layers, each inert without `#xum-pre`): 1. coder#6378: entry and connection guard (this stack's bottom) 2. coder#6379: splash queue and paint cache 3. coder#6381: wizard handoff Plan: section 5.6 "C step 2: the static first-run view on `/` (P1)" of the T3 Phase 3 plan. Nothing in this stack inserts `#xum-pre`: a later PR (4b) makes `xum server` paint it for `/`. Until then every change here is a no-op, and the gates (Lighthouse, page timing, first-load bytes) run once on the stack's top against the merge base; their table is on coder#6381. ## Implementation - `useFirstRunViewQueueGuard(loaded, wizardIsFirst)` removes the view in a layout effect when the queue is loaded and the wizard is not first: already viewed, the viewed list could not be read, splash screens off or no API, or another splash comes first. - `onboardingWizardViewed` (`storage.ts`, registered as `"cache"` like `uiTheme`): `cacheViewedWizard()` sets it when the loaded viewed list contains the wizard id, and after `dismiss()` marks the wizard viewed. - coder#6351 (open) changes the same file to keep a shown splash on a lost connection. That rule stands: a lost connection is the `reconnecting` status, where coder#6378's guard removes the static view, not the queue. ## Validation | Test (jest, `tests/ui/onboarding/providerHandoff.test.ts`) | Mutation that fails it | |---|---| | keeps the static view while the wizard is first | Q2: remove before the queue is loaded | | removes it when viewed / read failed / another splash first / no API | Q1: no queue guard | | caches a viewed wizard on load and on dismiss | Q3: no load-time write. Q4: no dismiss write | - `make crap-check` passes: the effect and the cache write are a named hook and a function with two callers, so `SplashScreenProvider` keeps its CC. - The storage registry tests (`storage.test.ts`, `persistedStateBudget.test.ts`, `legacyLocalStorageCleanup.test.ts`) pass with the new key. ## Risks Low. Without `#xum-pre` the only change is one new 4-byte localStorage value per browser (`true`), written when the wizard is viewed. --- _Generated with `xum` • Model: `anthropic:claude-opus-5-5` • Thinking: `high` • Cost: `$289.12`_ <!-- mux-attribution: model=anthropic:claude-opus-5-5 thinking=high costs=289.12 -->
yermakoffivan
pushed a commit
to yermakoffivan/mux
that referenced
this pull request
Oct 14, 2026
… in one commit (T3 C step 2 PR 1c) (coder#6381) ## Summary The live onboarding wizard takes over from the static first-run view (`#xum-pre`) without a flash. Under the view it renders nothing while it loads, so its "Checking your provider configuration…" step never paints over the view. Its first real commit removes the view in a layout effect and opens the dialog, overlay and step without entry animations, because the user already sees that frame. The step animations return when the user moves on. Without `#xum-pre`, the wizard renders exactly as today. **Stack** (T3 coder#5971, Option C step 2, PR 1 split at the 300-line limit into 3 layers, each inert without `#xum-pre`): 1. coder#6378: entry and connection guard (this stack's bottom) 2. coder#6379: splash queue and paint cache 3. coder#6381: wizard handoff Plan: section 5.6 "C step 2: the static first-run view on `/` (P1)" of the T3 Phase 3 plan. Nothing in this stack inserts `#xum-pre`: a later PR (4b) makes `xum server` paint it for `/`. Until then every change here is a no-op, and the gates (Lighthouse, page timing, first-load bytes) run once on the stack's top against the merge base; their table is on coder#6381. ## Implementation - `OnboardingWizardSplash.tsx`: - `useFirstRunViewHandoff(isLoading)` reads `#xum-pre` once at mount. It hides the wizard while it is loading under the view and removes the view in a layout effect once loading ends. The dialog and overlay skip their entry animation for the whole mount: they stay open, so dropping the override later would replay it (Codex round 1, fixed in `2702e7ec65`). The step skips its entry animation until the user moves on. - A wizard that starts past the gateway step (a provider configured or a coupon set after the early script read the config) removes the view too. - `stepEntryClassName()` picks the step's animation class. - `Dialog.tsx` and `SplashScreen.tsx`: a `skipEntryAnimation` prop sets `animation: none` on the dialog content and overlay. ## Validation - First commit (`5c70d0ece6`) is test only and green on main: without `#xum-pre` the wizard shows its loading step, then the gateway step, with the dialog, overlay and step entry animations (plan decision Q1: "the dialog renders the same classes and entry animation as today"). - New tests (jest, `tests/ui/onboarding/wizardHandoff.test.ts`), each failing under the named mutation: | Test | Mutation that fails it | |---|---| | never paints its loading step, and no commit shows the gateway step and the view together | W1: render the loading step under the view | | ... opens without entry animations: the step's return on Next, the dialog's and overlay's stay off for the mount | W3: dialog animates. W4: step animates. W5: step skip kept after Next. D1: overlay animates. W7: skip without a view. C1: the dialog follows the step's flag (the Codex round 1 finding) | | removes the view when it opens past the gateway step | W6: remove only on the gateway step | - Equivalent mutation W2 (`useLayoutEffect` → `useEffect` for the removal): no test can tell them apart here. Radix mounts the dialog's portal content in a second synchronous commit (its own layout effect), and React flushes pending passive effects before that commit. So both versions remove the view and show the dialog in the same task, before any paint. The layout effect stays, because it does not depend on Radix's portal timing. - `make crap-check` passes. `OnboardingWizardSplash` keeps CC 125 (a touched-hotspot warning remains). `make check-react-compiler`: 23/24. ## Gates (whole PR 1, stack top against the merge base) Measured on the perf host, base `1c66fd6811` (merge base) against head `320e63abe2`. Every number comes from the gate's `summary.md` in `/home/coder/perf-runs/t3/c2-pr1-gates/<folder>/`. | # | Gate | Result | Numbers | Folder | |---|---|---|---|---| | 1 | First-load JS (`dist/<file>.br` sum) | Budget passes. Source-list rule not met literally | 462,219 -> 462,757 B = **+538 B** (budget 1,024). Per layer: 1a 462,494 (+275), 1b 462,490 (+271), 1c 462,757 (+538). Source-map delta: PR 1's modules +1,927 raw B; 12 other sources -19 to +36 B each, net +28 B (A/A base vs base: 0 sources) | `first-load-js/` | | 2 | React Compiler + static-check per layer | Pass | 23/24 on `8786197d9d`, `3c0a4d302f`, `320e63abe2`. CI Static Checks and Required success on all three heads | `static/` | | 3 | Lighthouse P1, 6+6 | Pass | Mobile perf median 75.5 -> 77.0 (+1.5), desktop 99.0 -> 99.0 (+0.0). Quiet runs only: mobile +4.5 (2 base runs), desktop +0.0 | `lighthouse-p1/` | | 4 | Lighthouse P2 LCP, 6+6 | Not called a pass | Mobile 9,517 -> 9,537 ms (+20), desktop 1,826 -> 1,840 ms (+15, 4 head runs, not 6). <= 473 ms: yes. Base is over the D3 bound on both presets, so the rule is head <= base: not met on all runs. Quiet runs only: mobile -2 ms, desktop +16 ms | `lighthouse-p2/` | | 5 | Page timing (A/A, then A/B) | Inconclusive (rerun 2026-10-14) | A/A P1 qualified: MDI UCS 0.040 s, IR 0.136 s (`aa-p1-2`, after a failed first block at UCS 0.472 s). A/A P2 failed after one rerun: MDI UCS 0.256 s (limit 0.25), IR 0.075 s (`aa-p2-2`; earlier `aa-p2-1` invalid IR timeout, `aa-p2-1-r` UCS 1.352 s, IR 5.449 s). No gated A/B ran. Quiet waits 3,073 s | `page-timing/` | | 6 | Dogfood (plan 6.2, owner scope) | Pass | `#xum-pre` handoff in 4 runs (no-auth, token x 390, 1440): 0 frames with the loading step under the static view, 0 frames with both, 0 empty frames, same dialog rect before and after, inline animation none. Without `#xum-pre`: unchanged. Screenshots: [comment](coder#6381 (comment)) | `dogfood/` | ## Risks Low to medium. Without `#xum-pre`, the change is two hook calls and one class helper in the wizard, plus an unused-by-default prop on the shared Dialog. The pin tests above cover that path. --- _Generated with `xum` • Model: `anthropic:claude-opus-5-5` • Thinking: `high` • Cost: `$289.12`_ <!-- mux-attribution: model=anthropic:claude-opus-5-5 thinking=high costs=289.12 -->
yermakoffivan
pushed a commit
to yermakoffivan/mux
that referenced
this pull request
Oct 14, 2026
…T3 C step 2 PR 2) (coder#6444) ## Summary Adds `FIRST_RUN_VIEW_HTML` in `src/node/orpc/firstRunView.ts`: the static markup of the onboarding wizard's gateway step ("Xum Gateway (evaluation credits)"). A later PR paints it on `/` before the app JS runs. Nothing imports or serves it in this PR, so no response and no bundle changes. A new happy-dom test fails when the constant drifts from the live wizard. ## Background Option C step 2 of coder#5971 (plan section 5.6, PR 2 of 5). PR 1 (coder#6378, coder#6379, coder#6381) made the app hand over from a `#xum-pre` element to the live wizard in one commit. This PR adds the markup that element will hold. PR 4a adds the early script and PR 4b injects both into the `/` response. ## Implementation - The markup comes from the prototype's capture of the live first-run page at 412 px (61 elements), with these edits: - The app shell's `<main>` is a `<div>`. The boot loader's `<main>` in `#root` stays the only landmark. - Radix runtime artifacts are removed: focus guards, generated ids, `aria-hidden`, `tabindex` and `pointer-events`. - The `disabled` attributes are removed. The live buttons are enabled, and `disabled:opacity-50` made the copy look disabled in PR 1's dogfood screenshots. - The overlay and the dialog use the live class strings. The capture had lost their `data-[state=open]` entry classes, and the new test found that. Those classes do nothing in the copy, because it has no `data-state` attribute. - The root is `<div id="xum-pre" inert>` (the shared `FIRST_RUN_VIEW_ID`), so the whole copy takes no focus and no clicks before the handoff. - The shell's root `<div>` is `h-full` of `#xum-pre`. Outside `#root` (which `globals.css` sizes to the viewport), its nested `h-full` would otherwise collapse to the content height (Codex P2, fixed in `4de5a0bdbb`). - `#xum-pre` has `z-index:1500`, the live overlay's layer. As a fixed element it is a stacking context, so without it the iOS standalone `.ios-status-bar-band` (`z-index: 1001`) would paint over the copy (Codex P2, fixed in `1a76083472`). - SECURITY AUDIT comment at the constant: it is trusted server text, nothing user-controlled goes into it, and PR 4 inserts it with `template.content.cloneNode(true)`, never with `innerHTML`. - `tests/ui/onboarding/firstRunViewParity.test.ts` parses the constant from a `<template>` with `content.cloneNode`, the way the page will. It puts the result in the body as `#xum-pre` and renders `OnboardingWizardSplash` over it (directly, because `DISABLE_SPLASH_SCREENS` is true in test mode). PR 1's handoff then removes the copy. The test compares the tags, classes and own text of every overlay and dialog element with the live ones. It also checks that the root is the inert `#xum-pre` and holds no `<main>`. - Mutation it catches: changing one class on the live step (`gap-3` to `gap-2` at `OnboardingWizardSplash.tsx:1198`) fails the test. - The app shell behind the dialog is not part of the wizard component, so this test does not cover it. PR 3's box-parity tool checks the paragraph's position in a real browser. ## Validation Measured on the perf host, base `7b26188e1a` (origin/main) against head `4de5a0bdbb` (build gates rerun after the review fix; `make static-check` and React Compiler on `e812b9eb05`, CI covers the final head). Files: `/home/coder/perf-runs/t3/c2-pr2-gates/`. | Gate | Result | Numbers | |---|---|---| | First-load JS, normal build pair (exact `dist/<file>.br` sum) | Version-stamp noise only | 465,239 -> 464,905 B (-334 B; -332 B on `e812b9eb05`, raw sizes identical). With a pinned `src/version.ts`: 465,239 -> 465,239 B (0 B) | | Control build, pinned `src/version.ts`, full `dist/` sha256 | Renderer and `dist/index.html` byte-identical | Only difference: added `node/orpc/firstRunView.js` and `.js.map` (main-process tsc output, not imported) | | `make static-check` | Pass | exit 0 | | `make check-react-compiler` | Pass | 23/24 | | `make crap-check` | Pass | 0 violations, 0 warnings | | `tests/ui/onboarding/firstRunViewParity.test.ts` | Pass | 2/2. Mutation `gap-3` -> `gap-2` on the live step fails it | No Lighthouse and no page timing: this PR changes no served response. ## Deferred to PR 4b (blocking precondition, owner decision 10-14) At desktop widths the static shell behind the dialog is the 412 px capture's mobile layout ("Open sidebar" header), while the live first paint shows the default expanded 288 px `LeftSidebar` (Codex round 2). Nothing serves this constant until PR 4b. **PR 4b may not merge until the static shell behind the dialog matches the live first paint at 390, 744, 1200 and 1900 px** (the default expanded 288 px `LeftSidebar` on desktop widths, or a documented neutral background that the live first frame also shows). That check joins PR 4b's parity gate. ## Risks Low. The constant has no importer, so no runtime path changes. --- _Generated with `xum` • Model: `anthropic:claude-opus-5-5` • Thinking: `high` • Cost: `$322.92`_ <!-- mux-attribution: model=anthropic:claude-opus-5-5 thinking=high costs=322.92 -->
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The app removes the static first-run view (
#xum-pre) whenever it will show something other than the live onboarding wizard: the token prompt, a startup error, the reconnect screen, or an uncaught render error. The entry also registers the service worker when it runs afterload. Nothing inserts#xum-preyet, so nothing changes for users.Stack (T3 #5971, Option C step 2, PR 1 split at the 300-line limit into 3 layers, each inert without
#xum-pre):Plan: section 5.6 "C step 2: the static first-run view on
/(P1)" of the T3 Phase 3 plan. Nothing in this stack inserts#xum-pre: a later PR (4b) makesxum serverpaint it for/. Until then every change here is a no-op, and the gates (Lighthouse, page timing, first-load bytes) run once on the stack's top against the merge base; their table is on #6381.Implementation
src/common/constants/firstRunView.ts:FIRST_RUN_VIEW_ID = "xum-pre", the idscripts/perf/pageTimingProbe.tsalready treats as a static view.SplashScreenProvider.tsx:removeFirstRunView(), shared by every removal path.AppLoader.tsx:FirstRunViewConnectionGuardremoves the view onauth_required,errorandreconnecting(a lost connection must not leave the static view over the reconnect UI). It keeps the view whileconnecting,connectedanddegraded.main.tsx:AppLoaderremoves the view on an uncaught render error, then renders the empty root such an error leaves today and reports the error withwindow.reportError.document.readyState === "complete". C step 2 starts the app JS after the static view paints, which can be afterload, and then theloadlistener never fires.Validation
ba5667bf0f) is test only and green on main: the entry registers the service worker onload.tests/ui/onboarding/), each failing under the named mutation:loadlistenerremoveFirstRunView()incomponentDidCatch. E3: swallow the error instead ofreportErrorauth_requirednot blocked. G2:reconnectingnot blocked. G3: remove on every statusmake check-react-compiler: 23/24.make crap-check: pass. Typecheck (3 configs) passes. ESLint on the changed files passes. The full-repo ESLint run was OOM-killed twice on the shared host, so CI's Static Checks job is the full lint for this PR.Risks
Low. Every change is a no-op without
#xum-pre. The new boundary changes only how an uncaught render error ends: the root is empty either way, and the error is still reported as uncaught.Generated with
xum• Model:anthropic:claude-opus-5-5• Thinking:high• Cost:$289.12