Repository navigation
🤖 perf: keep the Markdown stack in the workspace page chunk - #6428
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. |
|
Dogfood screenshots (head e40285a, Flow 1: workspace -> right-sidebar Memory tab -> Desktop (1440 px): Phone (390 px, sidebar full-screen): Flow 2: fresh page load straight to Before the click, the page had loaded only Desktop (1440 px): Phone (390 px): |
|
@codex review |
|
Codex Review: Didn't find any major issues. Already looking forward to the next diff. 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. |
|
Perf owner: ready to merge at Lighthouse A/B on the dedicated VM (
Mobile performance score medians stay at 59 on both arms, and desktop at 92 on both. The predicted change was about −80 ms mobile and −20 ms desktop (#6334 measured +79 to +119 ms mobile). Other gates (lane, one build pair): workspace-page JS 1,411,252 → 1,391,173 B brotli (one page chunk again), first-load −148 B, Settings beyond first load −199,950 B, React Compiler 23/24, The 3 Pixel snapshots are not a required check. They come from the Memory editor's preview, which now loads lazily. Local Storybook shows base and head the same. Generated with |




Summary
Keeps the Markdown stack in the workspace page's chunk again. The seeded workspace page loads 20,079 B less brotli JS (one page chunk instead of two), opening Settings loads 199,950 B less, and first-load JS is unchanged (−148 B). Refs #6334.
Background
Since #6334,
MemoryFileEditorimportsMarkdownFilePreviewstatically, so Settings (SettingsPage -> MemorySection -> MemoryBrowser -> MemoryFileEditor) needs the Markdown stack. Rollup groups modules by the lazy entries that need them. With two lazy importers (the workspace page and Settings), it moved the stack out of the page chunk into a shared chunk. The raw JS the seeded page loads did not change, but two files compress worse than one: the 1.83 MB page chunk becameAIView-*.js+mermaid-LQXJBOYN-*.js. The Lighthouse mobile load is bandwidth-bound; same-session A/Bs put #6334 at +79 to +119 ms simulated LCP.Implementation
MemoryFileEditorlazy-loads the preview through the AIView entry module:React.lazy(() => import("@/browser/components/AIView/AIView")), insideLazyFeature.AIView.tsxre-exportsMarkdownFilePreview. Every importer of the stack then goes through one entry, so Rollup keeps one chunk.markdownStack.importGraph.test.ts) walks the static import graph ofApp.tsx,SettingsPage,ProjectPage,ScratchPageandAnalyticsDashboard, and fails if any reachesMarkdownRenderer/MarkdownFilePreview. It fails on main with the 🤖 feat: render Markdown memory files and AGENTS.md #6334 chain. Mutation caught: re-adding a staticMarkdownFilePreviewimport toMemoryFileEditorfails it.AppEarlyPageImport.test.tsxmocks the AIView module, and Bun keeps that mock for later suites despite itsafterAllrestore. The mock now returns the realMarkdownFilePreview, or the MemoryTab preview tests fail when both files run in one process.Options measured (renderer builds of 79e5be2, brotli bytes)
React.lazy(import(MarkdownFilePreview))manualChunksfor AIView + MarkdownRendererA fails because the lazy preview is one more entry that shares the stack. B fails because Rollup 4 pulls every static dependency of a manual chunk's modules (react…) into it, and
mainthen imports it.Accepted cost: cold preview in Settings
In browser mode, a Markdown preview in Settings opened before any workspace visit fetches the page chunk on click (~567 KB br, 8 files). This is accepted: opening Settings gets ~200 KB br lighter for everyone, desktop preloads AIView at idle (
appShellRef), and on a workspace route the chunk is already loaded. While it loads, the preview box (min height kept) is empty; a failed load showsLazyFeature's Reload message.Validation
Gates on one clean build pair, merge base 2ce183d vs head (each
make build-rendererunder the shared measurement lock):AIView+mermaid-LQXJBOYN)mermaid-LQXJBOYNonly)make check-react-compilermake static-checkmake crap-checkxum server(env -i, telemetry off, 127.0.0.1, temp XUM_ROOT, --no-auth)Gate e flows (screenshots in a comment below): (1) workspace -> right-sidebar Memory tab -> Markdown memory file -> Edit -> Preview; (2) fresh page load straight to
/settings/memory-> open the file in Preview. In flow 2,performanceresources showedSettingsPage+MemoryBrowseronly, andmermaid-LQXJBOYN-*.jsarrived after opening the file.Lighthouse VM A/B: not run here; the perf owner runs it.
Risks
Low. Only the Memory editor's preview changes: it now mounts behind Suspense. Worst case is an empty preview box while the chunk loads, or a Reload message if the chunk fails to load.
Generated with
xum• Model:anthropic:claude-opus-5-5• Thinking:high• Cost:$3.44