perf(studio): code-split the entry chunk (1,641 kB -> 326 kB) - #1100
Merged
Merged
Conversation
Contributor
|
marcusds
marked this pull request as ready for review
August 5, 2026 19:16
steramae-nvidia
approved these changes
Aug 5, 2026
Contributor
📝 WalkthroughWalkthroughChangesRuntime loading changes
Sequence Diagram(s)sequenceDiagram
participant ChatTrigger
participant CopilotTopBarChat
participant CopilotChatThread
participant ChatThreadErrorBoundary
ChatTrigger->>CopilotTopBarChat: Hover trigger
CopilotTopBarChat->>CopilotChatThread: Preload module
ChatTrigger->>CopilotTopBarChat: Open popout
CopilotTopBarChat->>CopilotChatThread: Render lazily
CopilotChatThread-->>ChatThreadErrorBoundary: Return content or throw error
ChatThreadErrorBoundary->>CopilotTopBarChat: Invoke retry callback
CopilotTopBarChat->>CopilotChatThread: Recreate lazy component
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
web/packages/common/src/components/FileContentPreview/index.tsx (1)
15-15: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winImport
FCas a type.
FCis used only in theFileContentPreviewtype annotation. Keep it out of the runtime React import.Proposed change
-import { FC, lazy, Suspense, useEffect, useMemo, useState } from 'react'; +import { lazy, Suspense, useEffect, useMemo, useState } from 'react'; +import type { FC } from 'react';As per coding guidelines, use
import typefor type-only imports.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/packages/common/src/components/FileContentPreview/index.tsx` at line 15, Update the React imports in FileContentPreview to import FC with a type-only import while keeping lazy, Suspense, useEffect, useMemo, and useState in the runtime import.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@web/packages/studio/src/main.tsx`:
- Line 12: Update the startup flow around telemetryReady so failures from the
telemetry import or module initialization are caught before Promise.all can
reject. Allow the application to mount without telemetry when it is optional, or
render the established explicit startup error when telemetry is required, while
preserving normal mounting on successful initialization.
In
`@web/packages/studio/src/routes/agents/ClaudeCodeChatRoute/ClaudeCodeTopBarChat.tsx`:
- Around line 29-33: Update preloadChatThread to locally catch and handle
importChatThread() failures instead of discarding the rejected promise. Also
ensure ClaudeCodeChatThread’s lazy import rejection is handled outside Suspense,
either with an appropriate error boundary or a retry/error state when the user
opens the chat.
---
Nitpick comments:
In `@web/packages/common/src/components/FileContentPreview/index.tsx`:
- Line 15: Update the React imports in FileContentPreview to import FC with a
type-only import while keeping lazy, Suspense, useEffect, useMemo, and useState
in the runtime import.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: c38f00e7-8e74-4b28-a703-46af85596228
📒 Files selected for processing (6)
web/packages/common/src/components/CodeEditor/constants.tsweb/packages/common/src/components/CodeEditor/linters/yaml.tsweb/packages/common/src/components/FileContentPreview/FileContentPreview.test.tsxweb/packages/common/src/components/FileContentPreview/index.tsxweb/packages/studio/src/main.tsxweb/packages/studio/src/routes/agents/ClaudeCodeChatRoute/ClaudeCodeTopBarChat.tsx
aray12
approved these changes
Aug 5, 2026
The Studio entry chunk had grown to 1,641 kB (gzip 535 kB) because the
app shell statically imported subtrees that pulled in heavy libraries on
every route.
GlobalNav statically imported ClaudeCodeTopBarChat, which reached
ClaudeCodeChatThread and through it assistant-ui, the remark/micromark
markdown stack, DataView + table-core + dnd-kit + date-fns, and — via
AgentBlockingInput -> DatasetFileSelect -> FileContentPreview ->
CodeEditor — all of CodeMirror, the lezer grammars, yaml and papaparse.
None of it is needed until the copilot pop-out is opened.
- ClaudeCodeTopBarChat: lazy() the chat thread, gated on a hasOpened
latch so the chunk loads on first open and stays mounted afterwards.
The trigger button and its thinking/unread badges stay synchronous.
- FileContentPreview: lazy() the CodeEditor behind a Suspense spinner.
- CodeEditor/constants: import BasicSetupOptions as a type, so importing
ContentType alone no longer drags in @uiw/react-codemirror.
- CodeEditor yaml linter: await import('yaml') inside the lint source.
- main.tsx: start the telemetry import without awaiting it, then await
it alongside the theme stylesheet before rendering. OpenTelemetry
still patches fetch/XHR before the first request, but the OTel SDK no
longer sits in the entry chunk.
Entry chunk is now 326 kB (gzip 76 kB), an 80% reduction.
Signed-off-by: mschwab <mschwab@nvidia.com>
Follow-up from a pass with the Vercel React best-practices rules. - bundle-preload: warm the chat chunk on hover of the top-bar trigger so the first open is instant instead of waiting on a ~400 kB fetch. Uses onMouseEnter, not onPointerEnter/onFocus — KUI's PopoverTrigger spreads `...props` after its own handlers, so either of those replaces the trigger's and breaks opening the pop-out. - rendering-hoist-jsx: hoist the Suspense fallback element to module scope instead of rebuilding it on every render. Signed-off-by: mschwab <mschwab@nvidia.com>
Signed-off-by: mschwab <mschwab@nvidia.com>
Address review on the entry-chunk code split: - main.tsx: telemetry is optional, so catch its dynamic import. Without a handler a failed telemetry chunk rejects the Promise.all and React never mounts, leaving a blank page. - ClaudeCodeTopBarChat: catch the hover preload rejection, and wrap the lazy chat thread in an error boundary. Nothing above GlobalNav catches, so a failed chunk fetch unwound to the root and blanked all of Studio. Retry builds a fresh lazy component since React caches a rejected import. - FileContentPreview: import FC as a type. Signed-off-by: mschwab <mschwab@nvidia.com>
marcusds
enabled auto-merge
August 6, 2026 20:58
marcusds
force-pushed
the
studio-entry-bundle-split/mschwab
branch
from
August 6, 2026 21:51
b03a629 to
08431ff
Compare
Contributor
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
web/packages/studio/src/routes/agents/CopilotChatRoute/CopilotTopBarChat.tsx (1)
27-33: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTest rejected lazy-import recovery through the popout.
The boundary tests throw from
Boomafter module loading. They do not exerciselazy,preloadChatThread, orretryChatThread. Add aCopilotTopBarChattest that rejects the first chat-thread import and resolves a fresh import after Try Again.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/packages/studio/src/routes/agents/CopilotChatRoute/CopilotTopBarChat.tsx` around lines 27 - 33, Add a CopilotTopBarChat test covering rejected lazy-import recovery through the popout: mock importChatThread so its first call rejects, render the popout, trigger Try Again via retryChatThread, and verify a fresh import resolves and the chat thread renders. Exercise createChatThread and preloadChatThread rather than only testing the Boom boundary.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In
`@web/packages/studio/src/routes/agents/CopilotChatRoute/CopilotTopBarChat.tsx`:
- Around line 27-33: Add a CopilotTopBarChat test covering rejected lazy-import
recovery through the popout: mock importChatThread so its first call rejects,
render the popout, trigger Try Again via retryChatThread, and verify a fresh
import resolves and the chat thread renders. Exercise createChatThread and
preloadChatThread rather than only testing the Boom boundary.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: b761b2e8-278b-4c22-8ffe-cdb9edfe5b39
📒 Files selected for processing (8)
web/packages/common/src/components/CodeEditor/constants.tsweb/packages/common/src/components/CodeEditor/linters/yaml.tsweb/packages/common/src/components/FileContentPreview/FileContentPreview.test.tsxweb/packages/common/src/components/FileContentPreview/index.tsxweb/packages/studio/src/main.tsxweb/packages/studio/src/routes/agents/CopilotChatRoute/ChatThreadErrorBoundary.test.tsxweb/packages/studio/src/routes/agents/CopilotChatRoute/ChatThreadErrorBoundary.tsxweb/packages/studio/src/routes/agents/CopilotChatRoute/CopilotTopBarChat.tsx
🚧 Files skipped from review as they are similar to previous changes (5)
- web/packages/common/src/components/CodeEditor/constants.ts
- web/packages/studio/src/main.tsx
- web/packages/common/src/components/FileContentPreview/FileContentPreview.test.tsx
- web/packages/common/src/components/FileContentPreview/index.tsx
- web/packages/common/src/components/CodeEditor/linters/yaml.ts
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
3 participants
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
Studio's entry chunk had grown to 1,641 kB (gzip 535 kB). This splits it down to 326 kB (gzip 76 kB) — an 80% reduction in raw bytes, 86% gzipped.
index-*.jsWhy it was big
Two static imports in the app shell — which renders on every route — were pulling the entire NeMo Copilot chat surface into the entry chunk:
ClaudeCodeChatRoutewas alreadylazy(), but that made no difference — everything it needed had already been hoisted into the entry chunk by the nav's static import. The route chunk was 2.2 kB.Separately,
main.tsximported@studio/telemetry/telemetryas a top-level side effect, putting the OpenTelemetry SDK +zone.js(~162 kB) on the critical path.Changes
ClaudeCodeTopBarChat.tsx—lazy()the chat thread, gated on ahasOpenedlatch so the chunk fetches on first pop-out open and stays mounted after (matching the previous always-mounted behaviour). The trigger button and its thinking/unread badges stay synchronous. Warmed on hover so the first open is instant.FileContentPreview—lazy()theCodeEditorbehind aSuspensespinner. Moves 602 kB off the boot path.CodeEditor/constants.ts—import type { BasicSetupOptions }. It was a value import, so importingContentTypealone still dragged in@uiw/react-codemirror; without this the split above wouldn't hold.CodeEditor/linters/yaml.ts—await import('yaml')inside the lint source (linteraccepts an asyncLintSource). Splits 103 kB out of the editor chunk into its own.main.tsx— telemetry moved from a top-level side-effect import to a fired-not-awaitedimport(), awaited inPromise.allalongsidewaitForThemeStylesheet()beforeroot.render. OTel still patchesfetch/XHR before the first app request; the 140 kB now downloads in parallel instead of inflating the entry chunk.Resulting chunk layout
Measured load impact
Built
mainat the branch point and this PR head, served eachdistfrom a local static server, loaded cold-cache in headless Chromium. Median of 9 runs after a discarded warm-up; backend requests aborted so both variants see identical (zero) API latency.Desktop, no throttling
Throttled — 4× CPU, 1.6 Mbps / 150 ms RTT
The 1,449 kB drop in critical-path JS is the whole story. Script execution barely moves, which is expected — the deferred code was never executing at boot, it was being downloaded and parsed.
Caveat: the absolute throttled numbers are inflated — the measurement server is HTTP/1.1, so every request pays the full 150 ms RTT. The relative delta is the trustworthy part. Note that the uncompressed bytes may not be an artifact: see next steps, I could not find compression on the Studio static mount either.
Netting out the shared vendor bundles (2,379 kB, byte-identical in both builds), Studio's own critical-path JS goes 1,973 kB → 558 kB (−72%).
Recommended next steps
After this PR the critical path is 24 resources / 3,488 kB uncompressed, broken down as:
vendor/foundations.jsindex.cssindex.js(entry)vendor/react-router.jsvendor/react-dom.jsoidc-client-ts.jsOrdered by measured impact:
1. Serve Studio's assets compressed. I could not find any compression on the static mount — no
GZipMiddleware(or equivalent) inservices/studio/src/nmp/studio/service.py, which mountsSPAStaticFiles(a plain StarletteStaticFilessubclass), and no gzip/brotli configuration ink8s/helm. If that's accurate for deployed environments too, the whole 3,488 kB critical path is going over the wire uncompressed. The chunks gzip at roughly 4:1 (the entry chunk is 326 kB → 76 kB), so this is on the order of a ~2.5 MB reduction for a few lines of middleware — a bigger win than this entire PR, and much cheaper. Worth confirming against whatever ingress fronts a real deployment before acting.2.
vendor/foundations.js— 1,996 kB, 57% of what's left. The vendor shim invite.config.tsis a blanketexport * from '@nvidia/foundations-react-core'built withcodeSplitting: false, so the entire design system ships regardless of how much of it Studio actually renders, and nothing tree-shakes. The blanket re-export exists for a good reason — the bundle has to satisfy the union of what Studio and every runtime-loaded plugin imports, and there must be exactly one instance. But that union could be generated from the components actually imported acrosspackages/studio,packages/commonandplugins/*/webrather than assumed to be everything. Biggest single lever left.3.
index.css— 480 kB, render-blocking. Emitted by the Tailwind build. Worth checking whether thecontentglobs are over-broad (the postcss config points at all ofpackages/) and whether KUI's full stylesheet is being inlined alongside Studio's own utilities.4. The generated SDK barrel — 142 kB, 44% of the remaining entry chunk.
packages/sdk/generated/platform/api.tsis a single orval-generated barrel pulled in eagerly. Per-tag entry points would let each route import only the operations it uses.5.
oidc-client-ts— 108 kB,modulepreloaded inindex.html. Eager at boot. Worth checking whether it's genuinely needed before first paint or only on the auth/silent-renew path.6.
ClaudeCodeChatProvider— ~90 kB, mostly@assistant-ui/core. Still eager inPageLayout; see the note below on why it wasn't done here.7. Add a size budget to CI.
build.chunkSizeWarningLimitonly warns and is easy to ignore. An assertion on the entry chunk's gzip size would stop this regressing back to 535 kB the next time something is imported into the app shell — which is exactly how it got there.Notes
PopoverTriggerspreads...propsafter its ownonPointerEnter, so passingonPointerEnter(oronFocus) to aPopovertrigger silently replaces the trigger's handler and breaks opening the pop-out. The hover preload usesonMouseEnterfor that reason — there are unit tests covering it.ClaudeCodeChatProvideris still eager inPageLayout(~90 kB, mostly@assistant-ui/core). Every no-remount way to defer it requires makingClaudeCodeChatContextValuenullable —useClaudeCodeChatContextcurrently throws on null. That's a design change, not a perf tweak, so it's left out.Testing
tsc --noEmitclean@nemo/common: 1379/1379 pass (fourFileContentPreviewassertions moved tofindByTestIdfor the now-suspended editor)nemo-studio-ui: full suite green exceptSafeSynthesizerNewRoute, which times out at 10 s under full-suite contention and passes on its own — unrelated to these filestelemetry-*.jsloads, and neitherCodeEditor-*.jsnorClaudeCodeChatThread-*.jsis requested at bootSummary by CodeRabbit
Performance
Bug Fixes