Repository navigation
🤖 perf: add the static first-run view's markup as a server constant (T3 C step 2 PR 2) - #6444
Conversation
… step 2 PR 2) The server will paint a static copy of the onboarding wizard's gateway step before the app JS runs (plan section 5.6), and the live wizard replaces it in one commit. This test parses FIRST_RUN_VIEW_HTML the way the page will (a <template>, content.cloneNode), puts it in the body as #xum-pre, renders OnboardingWizardSplash over it, and compares the tags, classes and text of the overlay and dialog with the live ones. It fails while the constant does not exist yet. anthropic:claude-opus-5-5 high 322.92
…T3 C step 2 PR 2) FIRST_RUN_VIEW_HTML in src/node/orpc/firstRunView.ts is a copy of the onboarding wizard's gateway step, the view PR 4 will paint on / before the app JS runs. No response uses it yet. It comes from the prototype's 412 px capture (61 elements): the app shell's <main> is a <div>, Radix runtime artifacts (focus guards, generated ids, aria-hidden, tabindex, pointer-events) and the disabled attributes are gone, the overlay and dialog take the live class strings, and the root is the inert #xum-pre element. A SECURITY AUDIT comment marks it as trusted server text. The parity test now uses window.DOMParser and window.Node: tests/ui/dom.ts does not install them as globals. anthropic:claude-opus-5-5 high 322.92
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. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e812b9eb05
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
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".
… 2 review) Codex P2 on #6444: outside #root, which globals.css sizes to the viewport, the shell's first <div> had auto height, so its nested h-full resolved against nothing and the shell collapsed to its content: the empty-state text sat at the top instead of the centre. The <div> is now h-full of #xum-pre (position:fixed; inset:0). Checked by painting the constant over a built server at 390 and 1440 px: the dialog box matches the live one (20,120,351,604 and 470,188,500,524). anthropic:claude-opus-5-5 high 322.92
|
@codex review Head |
🛡️ 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. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4de5a0bdbb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
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".
…R 2 review) Codex P2 on #6444: #xum-pre is position:fixed, so it is a stacking context at the auto level, and its z-[1500] overlay could not rise above the iOS standalone .ios-status-bar-band (fixed, z-index 1001). The live Radix overlay sits under body at 1500 and covers the band, so the band would change at handoff. #xum-pre now has z-index:1500. anthropic:claude-opus-5-5 high 322.92
|
Review loop paused after round 2 (convergence rule): round 1 had one finding and round 2 had two, all in the static shell around the dialog.
The head Generated with |
|
@codex review Head |
|
Codex Review: Didn't find any major issues. 🚀 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. |
…step 2 PR 3b) scripts/perf/firstRunViewParity.ts serves a built tree on a fresh empty root and opens "/" in a real Chrome at 390, 744, 1200 and 1900 px, in the light and the dark system theme. It holds the app's main chunk, measures the static view's "OSS contributors" paragraph and counts <main>, then releases the chunk and measures the live wizard's first frame after the handoff. It fails a run when the paragraph drifts more than 2 px in x, y, width or height, or the static phase does not have exactly one <main>. It saves full-page screenshots of both phases for PR 4b's shell check (owner decision 10-14). Until PR 4b serves the view, the served "/" has no #xum-pre, and the tool inserts #6444's FIRST_RUN_VIEW_HTML before </body> through a Playwright route. Make target: first-run-view-parity (FIRST_RUN_VIEW_PARITY_ARGS), as cleared by the perf owner. The test's #root assertion now fails with a compact message instead of an element dump. anthropic:claude-opus-5-5 high 337.13
…#6456) ## Summary Adds `scripts/perf/firstRunViewParity.ts` and `make first-run-view-parity`: gate 8's tool for PR 4b. In a real Chrome against a built server, at 390, 744, 1200 and 1900 px and in the light and dark system theme, it checks that the static first-run view's "OSS contributors..." paragraph is within 2 px of the live wizard's in x, y, width and height, and that the static phase has exactly one `<main>`. It saves full-page screenshots of the static phase and of the live first frame for every run, for PR 4b's shell precondition (owner decision 10-14). ## Background Option C step 2 of coder#5971, plan section 5.6, PR 3 (split into 3a, 3b and 3c by the perf owner). coder#6444 added the static markup (`FIRST_RUN_VIEW_HTML`). PR 4b will serve it on `/`, and its gate 8 needs this tool. ## Implementation - `--tree <built tree> [--out <dir>] [--chrome <path>]`. One `xum server --no-auth` on a fresh empty root (the first-run state), one browser context per width and theme. Widths 390 and 744 use phone emulation (touch, DPR 2); 1200 and 1900 use plain desktop. The theme is the system color scheme with empty localStorage, which is what a new user gets. - Each run holds the app's `main-*.js` so `#xum-pre` stays alone on screen. The tool waits for every stylesheet and then for the fonts layout needs, measures the static paragraph and counts `<main>`, and takes the static screenshot. Then it releases the chunk, waits (per animation frame) until `#xum-pre` is gone and the live paragraph exists, measures it, and takes the live screenshot. - **The test page (until PR 4b exists):** the served `/` has no `#xum-pre` yet. A Playwright route on the `/` document then inserts coder#6444's `FIRST_RUN_VIEW_HTML` once before `</body>`, which is outside `#root` (React clears `#root`). `parity.json` records `"injected": true`. When the served `/` already has `id="xum-pre"` (PR 4b), the tool leaves the response alone. - A route-fulfilled document has no local address space, so Chrome's Local Network Access blocked the app's WebSocket to `127.0.0.1`, and the app showed its connection error instead of the wizard. The tool launches Chrome with `--disable-features=LocalNetworkAccessChecks`. - Exit: 0 every run passes, 1 a run fails the rule, 2 bad input, 3 an invalid run. - Makefile: one target plus its `.PHONY` entry, exactly the hunk the perf owner cleared. ## Validation Tests (first commit, `scripts/perf/firstRunViewParity.test.ts`), each with the mutation it catches. I applied each mutation in a scratch edit, saw the named test fail, and restored the file: | Test | Mutations it catches | |---|---| | the test page gets the constant once, outside #root | inserting the markup inside `#root` | | parity allows 2 px of drift in position and size, and exactly one main | checking x/y only; a 3 px tolerance; allowing 0 `<main>` | The tests fail on main because the tool does not exist yet. **One run against main `6090f7691d` (hand-injected test page):** 8 of 8 pass. Static and live boxes are identical at every width and theme: | Width | Paragraph box x, y, width, height (static = live) | `<main>` in static phase | |---|---|---| | 390 | 44.5, 307, 301, 60 | 1 | | 744 | 147, 451.5, 450, 40 | 1 | | 1200 | 375, 305, 450, 40 | 1 | | 1900 | 725, 405, 450, 40 | 1 | The first runs found two tool bugs (the blocked WebSocket, and a measurement before the stylesheet applied at 1900 px dark), fixed in the third commit. After the fix, three full runs passed 8 of 8 each. Screenshots of the final run are in a comment below. At 1200 and 1900 px they show the shell difference behind the dialog that PR 4b must resolve. Gates: `make static-check` exit 0, `make crap-check` PASS (0 violations, 0 warnings). --- _Generated with `xum` • Model: `anthropic:claude-opus-5-5` • Thinking: `high` • Cost: `$337.13`_ <!-- mux-attribution: model=anthropic:claude-opus-5-5 thinking=high costs=337.13 -->
…tant (T3 C step 2 PR 4a) (coder#6486) ## Summary Adds `FIRST_RUN_EARLY_SCRIPT` and `FIRST_RUN_TEMPLATE_ID` to `src/node/orpc/firstRunView.ts`: the inline script that will decide on the served `/` whether the static first-run view shows, and that loads the app after the view paints. Nothing serves it in this PR. PR 4b injects it, together with the template, the module list and the redirect. ## Background Option C step 2 of coder#5971 (plan section 5.6, PR 4a). coder#6444 added the view's markup, and coder#6445 defined the deferred module list (`<script type="application/json" id="xum-pre-modules">`) that this script loads the app from. ## Implementation The script, in order: 1. **Slashless URL:** the redirect script navigates the page away, so the early script does nothing. 2. **Paint-cache key set** (`onboardingWizardViewed` is `"true"`) **or hidden tab:** load the app at once, with no API call and no view. 3. **Otherwise:** POST `api/splashScreens/getViewedSplashScreens` and `api/providers/getConfig` together, with `Authorization: Bearer` from the trimmed `?token=`, or else the stored `mux:auth-token` (as `ManagedAPIProvider` picks it). One `AbortController` aborts both after 1,000 ms, and that is the script's only timer. 4. **Failure, 401 or abort:** load the app with no view, as today. With a 401 the app shows its token prompt. 5. **Show rule:** the wizard is not in the viewed list, no provider has `isConfigured === true`, and `mux-gateway.couponCodeSet` is not `true`. 6. **Show:** clone the view from `<template id="xum-pre-template">` (waiting for `DOMContentLoaded` if the template is not parsed yet), then load the app two `requestAnimationFrame`s later. If the tab turns hidden first, it loads the app at once, and it never inserts the view after the app has loaded. "Load the app" runs once. It validates the list (a missing or malformed list removes the view and logs, because there is no app to load), then adds the list's modulepreload links at once, and the module script (both with `crossorigin`) once the HTML is parsed: a dynamic module runs as soon as it loads, unlike the deferred tag it replaces, and `main.tsx` needs `#root`. The module script's `error` event removes `#xum-pre`. The script has no request or user data in it, and it inserts the view from a template clone, never as HTML. PR 4b must place the module list before the script, because steps 2 and 4 load the app while the `<head>` is still parsing. ## Validation Tests (first commit, `src/node/orpc/firstRunView.test.ts`, 15 tests now) run the shipped script string in happy-dom with a fake `fetch`, timers and frames. I applied each mutation in a scratch edit, saw the named test fail, and restored the file: | Test | Mutations it catches | |---|---| | shows the view, starts both POSTs together, and loads the app two frames later | one frame instead of two; POSTs one after the other; no bearer header; the stored token before `?token=`; an extra `setTimeout(loadApp, 5000)` safety timer | | sends the stored token when the URL has none | no bearer header | | no view, app at once when: the wizard was viewed / a provider is configured / a gateway coupon is set / a POST returns 401 / a POST fails / the POSTs take over 1,000 ms | each check removed (viewed, provider, coupon, `r.ok`); the abort removed; the `catch` removed | | no API call, app at once when: the paint-cache key is set / the tab is hidden | each check removed | | loads the app at once when the tab turns hidden while the view waits for its frames | no `visibilitychange` listener | | removes the view when the app's module script fails to load | no `error` listener | | waits for DOMContentLoaded when the POSTs answer before the template is parsed | no `readyState` wait | | does nothing on a slashless URL | no slashless check | | drops the view when the module list is unusable (round-1 fix) | the unguarded `JSON.parse` of the list | | (the DOMContentLoaded test also runs with `?token=%20` and a stored token) | the stored token not sent when the URL has none; an untrimmed URL token (round-2 fix) | | adds the app's module script only once the HTML is parsed (round-2 fix) | the module script added while `readyState` is `loading` | The tests fail on main because the two exports do not exist yet. Gates on the perf host (files in `/home/coder/perf-runs/t3/c2-pr4a-gates/`): | Gate | Result | |---|---| | `src/node/orpc/firstRunView.test.ts` | 15 pass, 0 fail | | `make check-react-compiler` | 23/24 hot components compile (1 known skipped) | | `make crap-check` | PASS, 0 violations, 0 warnings | | `make static-check` | Formatting and all three `tsgo` projects pass. The full ESLint run was OOM-killed on the shared host (cgroup full). ESLint on the two changed files passes, and CI's Static Checks covers the final head | | `make first-load-js`, normal builds of base `b20d868fcf` and head | 466,417 vs 466,279 B .br (-138 B, version-stamp noise: chunk names differ) | | `make first-load-js`, head built with base's `src/version.ts` pinned | 466,417 B, and the report is byte-identical to base's. Full `dist/` sha256: only `node/orpc/firstRunView.js` and `.js.map` differ | ## Deferred to PR 4b - `ONBOARDING_WIZARD_SPLASH_ID` is a literal in the script (Codex round 1). This PR may touch only `firstRunView.ts` and its test. PR 4b moves the splash id to `src/common/constants/` together with the module-list id (`xum-pre-modules`, a local constant here). - Known limit (Codex round 1): in a split-origin build (`VITE_BACKEND_URL`), the script asks the serving `xum server`, not the configured backend. A wrong "no" keeps today's load. A wrong "yes" shows the view until PR 1's handoff removes it. --- _Generated with `xum` • Model: `anthropic:claude-opus-5-5` • Thinking: `high` • Cost: `$346.60`_ <!-- mux-attribution: model=anthropic:claude-opus-5-5 thinking=high costs=346.60 -->




Summary
Adds
FIRST_RUN_VIEW_HTMLinsrc/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 #5971 (plan section 5.6, PR 2 of 5). PR 1 (#6378, #6379, #6381) made the app hand over from a
#xum-preelement 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
<main>is a<div>. The boot loader's<main>in#rootstays the only landmark.aria-hidden,tabindexandpointer-events.disabledattributes are removed. The live buttons are enabled, anddisabled:opacity-50made the copy look disabled in PR 1's dogfood screenshots.data-[state=open]entry classes, and the new test found that. Those classes do nothing in the copy, because it has nodata-stateattribute.<div id="xum-pre" inert>(the sharedFIRST_RUN_VIEW_ID), so the whole copy takes no focus and no clicks before the handoff.<div>ish-fullof#xum-pre. Outside#root(whichglobals.csssizes to the viewport), its nestedh-fullwould otherwise collapse to the content height (Codex P2, fixed in4de5a0bdbb).#xum-prehasz-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 in1a76083472).template.content.cloneNode(true), never withinnerHTML.tests/ui/onboarding/firstRunViewParity.test.tsparses the constant from a<template>withcontent.cloneNode, the way the page will. It puts the result in the body as#xum-preand rendersOnboardingWizardSplashover it (directly, becauseDISABLE_SPLASH_SCREENSis 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-preand holds no<main>.gap-3togap-2atOnboardingWizardSplash.tsx:1198) fails the test.Validation
Measured on the perf host, base
7b26188e1a(origin/main) against head4de5a0bdbb(build gates rerun after the review fix;make static-checkand React Compiler one812b9eb05, CI covers the final head). Files:/home/coder/perf-runs/t3/c2-pr2-gates/.dist/<file>.brsum)e812b9eb05, raw sizes identical). With a pinnedsrc/version.ts: 465,239 -> 465,239 B (0 B)src/version.ts, fulldist/sha256dist/index.htmlbyte-identicalnode/orpc/firstRunView.jsand.js.map(main-process tsc output, not imported)make static-checkmake check-react-compilermake crap-checktests/ui/onboarding/firstRunViewParity.test.tsgap-3->gap-2on the live step fails itNo 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 pxLeftSidebaron 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