Repository navigation
Conversation
Refs #5971. Adds --budget tests to scripts/perf/firstLoadJs.test.ts: growth at or under the recorded values passes, brotli growth over 2% fails, raw growth over 100 KiB fails, a forbidden module fails even under budget, and an unusable budget file exits 2. They fail on the script before the budget mode exists.
Refs #5971. firstLoadJs.ts --budget fails when first-load brotli grows more than 2% or raw more than 100 KiB over scripts/perf/firstLoadBudget.json (recorded from a clean build of main ea19a44). The must-stay-lazy list gains the seven lazy right-sidebar panels. make check-first-load-js runs it in Smoke / Server right after that job's make build, so CI adds no build.
|
Dogfood output for head
@@ -30,6 +30,7 @@ import {
} from "lucide-react";
import { ErrorBoundary } from "@/browser/components/ErrorBoundary/ErrorBoundary";
import { LazyFeature } from "@/browser/components/LazyFeature/LazyFeature";
+import { OutputTab } from "@/browser/components/OutputTab/OutputTab";
import { StatsContainer } from "@/browser/features/RightSidebar/StatsContainer";
import { ReviewPanel } from "@/browser/features/RightSidebar/CodeReview/ReviewPanel";
import type { GoalCreateIntent } from "@/browser/features/RightSidebar/GoalTab";
@@ -74,9 +75,7 @@ function lazyPanel<P extends object>(name: string, load: () => Promise<React.Com
const InstructionsTab = lazyPanel("Instructions", () =>
import("@/browser/components/InstructionsTab/InstructionsTab").then((m) => m.InstructionsTab)
);
-const OutputTab = lazyPanel("Output", () =>
- import("@/browser/components/OutputTab/OutputTab").then((m) => m.OutputTab)
-);
+
const DesktopPanel = lazyPanel("Desktop", () =>
import("@/browser/features/desktop/DesktopPanel").then((m) => m.DesktopPanel)
);3. PR7 base 08390cd (all panels eager): 8 forbidden hits, and both budgets fail4. Main with the PR1–PR6 product changes reverted: 104 forbidden hits (6 entries), and both budgets fail |
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. |
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c265159054
ℹ️ 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".
🛡️ 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. |
index.html defines the first-load roots, so an index.html-only PR must run Smoke / Server and its first-load budget check.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4974e5e8ee
ℹ️ 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".
🛡️ 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. |
Codex round 2: the totals skipped index.html, so its inline boot script could grow without failing the budget. index.html now counts as one measured entry (not scanned for forbidden sources). Budget re-recorded from the same clean build of main ea19a44: +9,341 bytes.
|
@codex review |
🛡️ 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: 3fe3d4b06e
ℹ️ 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".
Codex round 3: htmlRoots() follows only module scripts, so a classic <script src> added to index.html was not measured. It now exits 2. index.html has none today, so the budget is unchanged.
|
Pausing at the three-round Codex cap. Codex round 3 on I have a minimal fix ready locally ( |
|
Perf owner: I approve one fourth Codex round on this PR as an explicit exception, and it is the last one. The user-level cap is 6, and the 3-round cap was my own lane limit. Rounds 1-3 each found one new case in what the check counts. The round-3 fix rejects other script forms (exit 2) instead of counting them, so the check does not grow further. If round 4 leaves any finding, this PR stops unmerged, and I decide between a reasoned won't-fix and parking. No round 5 will happen. |
|
@codex review |
🛡️ 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: 20c6ed5b4f
ℹ️ 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".
| if (isScript && !isRoot && href != null) { | ||
| fail(`index.html loads classic script ${href}, which first-load totals cannot measure`); | ||
| } | ||
| if (!isRoot || href == null) continue; // inline boot scripts, stylesheets, icons |
There was a problem hiding this comment.
When index.html contains an inline module such as <script type="module">import "./large.js"</script>, isRoot is true but href is null, so this validation does not reject it and the next line skips it. Counting index.html adds only the small import statement; the imported file's bytes and forbidden sources never enter the graph, allowing substantial first-load JavaScript growth to pass CI. Parse inline modules for static imports or reject inline module scripts.
Useful? React with 👍 / 👎.
|
Stopping here: Codex round 4 (the final approved round) on |
|
Perf owner: parked. Do not merge. Head Why: Codex rounds 1-4 each found one new case of what the first-load check counts (the index.html path filter, index.html bytes, classic external scripts, and now imports from an inline module script). That pattern does not converge. A won't-fix for round 4 is not justified, because a budget check must catch future entry changes, and "main has no inline module today" does not show the gap is harmless. A fifth round is not allowed under the final-round rule I set. A restart needs:
The recorded budget values and the CI step time (about 0.35 s) stay valid as evidence for the restart. |
This branch has not been deployed
1 participant
Summary
CI now fails a PR that grows first-load JS over a recorded budget, or that puts a must-stay-lazy module back on the first load.
make check-first-load-jsruns in Smoke / Server right after that job's existingmake build, so CI does no extra build. Refs #5971 (T3 PR8).Background
T3 PR1–PR7 cut first-load JS for browser mode, and each added a forbid entry for the module it made lazy. Nothing in CI ran that check or watched the total bytes. This PR does both.
Implementation
scripts/perf/firstLoadJs.ts --budget <file>compares the first-load totals withscripts/perf/firstLoadBudget.json. It exits 1 when brotli bytes grow more than 2% overbrBytes, or raw bytes more than 100 KiB overrawBytes. A smaller first load never fails, so a PR that shrinks it needs no update. The failure message names the new value, the limit, the budget key and the budget file. It also says how to fix: the perf owner updates the recorded value in the PR that causes the growth. The budget file has the same rule in itscommentfield.FIRST_LOAD_FORBIDDEN_SOURCES) now also covers the seven lazy right-sidebar panels from 🤖 perf: lazy-load the right-sidebar panels #6081, next to DesktopPanel.TimelinePanelandArtifactsPanelare not on the list: they stay eager becauseTimelineDialogandArtifactsDialogimport them statically.index.htmlplus the module JS it loads (its module scripts, modulepreloads and their static imports). The check refuses any other script form: an external<script src>that is nottype="module"exits 2 with a message naming it (Codex round 3). index.html has none today.index.htmlcounts as one entry (Codex round 2), so growth of its inline boot script or markup is measured. It is not scanned for forbidden sources.make buildof mainea19a44101, Bun 1.3.12, under the host measurement lock):rawBytes5,134,092 andbrBytes1,219,205. The limits are 5,236,492 raw and 1,243,589 br.index.htmlis added to thechangesjob'sconfigpaths filter (Codex round 1). It defines the first-load roots, so an index.html-only PR must run Smoke / Server and this check. Such PRs now also run the other test jobs..PHONYchange only appendscheck-first-load-jsto the end of its existing line.Validation
srcchanges of PR1–PR6 reverted (🤖 perf: lazy-load the Lottie loading animation #6013 onlyWorkspaceShell.tsx, because later PRs reuse itsLazyFeature) exits 1. It has hits for all six earlier entries:lottie-web1,ghostty-web1,mermaid15,ProvidersSection1,@shikijs/langs14,recharts72.08390cd9b6exits 1. It has exactly one hit for each of the 8 panel entries, and each hit is that panel's own.tsxfile.--budgetitself. Mutation M5 shows it still catches lax validation. All 6 mutations fail at least one test: never over,>=instead of>, 200 KiB raw slack, 5% br slack, lax budget validation, and a budget result that hides a forbidden module.make check-first-load-json main passes. A one-line scratch change that importsOutputTabstatically intabRegistry.tsxfails with exit 1 on the forbidden module, while its bytes stay within budget. The two scratch builds above fail on both budgets as well.c265159054,make check-first-load-jsran from 20:02:32.552 to 20:02:32.901 (about 0.35 s) and passed on CI's own build: 1,209,802 br and 5,124,749 raw bytes. Those differ from the recorded values by 62 and 2 bytes.make static-checkpasses (23/24 hot components compile (1 known skipped)).Follow-ups
TimelinePanelandArtifactsPanelstay eager throughTimelineDialogandArtifactsDialog, so they are not on the must-stay-lazy list yet. They are part of the F1/Phase 2 decisions on perf: xum server first load downloads and parses about 9.8 MB of JavaScript #5971, and this PR does not change them.Risks
Low, and CI-only. The script runs only in CI and from
make. Bundle bytes change slightly between Bun or Vite versions, so a toolchain bump can move the totals. The 2% / 100 KiB slack covers small moves. A larger move fails with a message that says which value to update.Generated with
xum• Model:anthropic:claude-opus-5-5• Thinking:high• Cost:$181.42