Repository navigation
🤖 perf: remove the static first-run view when the wizard is not first, and cache a viewed wizard (T3 C step 2 PR 1b) - #6379
Merged
Conversation
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
github-merge-queue
Bot
removed this pull request from the merge queue due to a conflict with the base branch
Oct 14, 2026
…, and cache a viewed wizard (T3 C step 2 PR 1b) SplashScreenProvider removes #xum-pre once its queue is known and the wizard is not first (viewed, read failed, splashes off or no API, another splash first). It sets the new onboardingWizardViewed paint cache (registered as cache) when the viewed list holds the wizard and after dismiss() marks it viewed, so the early script can skip its API calls. Inert without #xum-pre.
ThomasK33
force-pushed
the
perf/t3-c2-pr1b-queue
branch
from
October 14, 2026 03:47
3c0a4d3 to
1d9bc8c
Compare
Member
Author
|
@codex review Rebased onto main after #6378's squash merge. Head |
|
Codex Review: Didn't find any major issues. Breezy! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
🛡️ Codex Security Review · Automatically triggeredSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
yermakoffivan
pushed a commit
to yermakoffivan/mux
that referenced
this pull request
Oct 14, 2026
…an error or a crash (T3 C step 2 PR 1a) (coder#6378) ## 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 after `load`. Nothing inserts `#xum-pre` yet, so nothing changes for users. **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 - `src/common/constants/firstRunView.ts`: `FIRST_RUN_VIEW_ID = "xum-pre"`, the id `scripts/perf/pageTimingProbe.ts` already treats as a static view. - `SplashScreenProvider.tsx`: `removeFirstRunView()`, shared by every removal path. - `AppLoader.tsx`: `FirstRunViewConnectionGuard` removes the view on `auth_required`, `error` and `reconnecting` (a lost connection must not leave the static view over the reconnect UI). It keeps the view while `connecting`, `connected` and `degraded`. - `main.tsx`: - A boundary around `AppLoader` removes the view on an uncaught render error, then renders the empty root such an error leaves today and reports the error with `window.reportError`. - The service worker registers at once when `document.readyState === "complete"`. C step 2 starts the app JS after the static view paints, which can be after `load`, and then the `load` listener never fires. ## Validation - First commit (`ba5667bf0f`) is test only and green on main: the entry registers the service worker on `load`. - New tests (jest, `tests/ui/onboarding/`), each failing under the named mutation: | Test | Mutation that fails it | |---|---| | registers at once when the entry runs after load | E1: keep only the `load` listener | | an uncaught render error removes the view and leaves the root empty | E2: no `removeFirstRunView()` in `componentDidCatch`. E3: swallow the error instead of `reportError` | | connection guard, 6 statuses | G1: `auth_required` not blocked. G2: `reconnecting` not blocked. G3: remove on every status | - `make 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`_ <!-- 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
SplashScreenProviderremoves 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-preyet.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
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"likeuiTheme):cacheViewedWizard()sets it when the loaded viewed list contains the wizard id, and afterdismiss()marks the wizard viewed.reconnectingstatus, where 🤖 perf: never leave the static first-run view over the token prompt, an error or a crash (T3 C step 2 PR 1a) #6378's guard removes the static view, not the queue.Validation
tests/ui/onboarding/providerHandoff.test.ts)make crap-checkpasses: the effect and the cache write are a named hook and a function with two callers, soSplashScreenProviderkeeps its CC.storage.test.ts,persistedStateBudget.test.ts,legacyLocalStorageCleanup.test.ts) pass with the new key.Risks
Low. Without
#xum-prethe 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