perf: lazy-load and cache frontend assets - #344
Conversation
📝 WalkthroughWalkthroughThe PR adds frontend bundle measurement and precompression, shared static-file negotiation with HTTP caching, lazy route and markdown loading with recovery, shared syntax highlighting, route preloading, release identity handling, preview worktree cleanup, and expanded tests. ChangesStatic asset delivery
Frontend loading and rendering
Runtime identity and preview worktrees
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Browser
participant BunServer
participant staticFileResponse
participant FrontendAssets
Browser->>BunServer: request asset with Accept-Encoding
BunServer->>staticFileResponse: resolve file and cache policy
staticFileResponse->>FrontendAssets: inspect source and sidecars
FrontendAssets-->>staticFileResponse: return selected representation
staticFileResponse-->>Browser: encoded response or 304
sequenceDiagram
participant User
participant Layout
participant routeModules
participant Router
participant loadLazyModule
User->>Layout: focus or hover route link
Layout->>routeModules: preload route module
User->>Router: navigate to route
Router->>loadLazyModule: load route component
loadLazyModule-->>Router: resolve module or initiate guarded reload
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a1734da7eb
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4add2941c8
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@src/components/features/chat/ChatMessagesList.tsx`:
- Around line 47-53: Update SettledChatMarkdown so onLoadReference.current is
refreshed inside an effect rather than during render, and make the layout effect
depend on text so markdown changes trigger re-measurement. Preserve the existing
onLoad callback invocation while ensuring the latest callback is used.
In `@src/lib/lazyImportRecovery.ts`:
- Around line 87-90: Update the cooldown check in the lazy import reload
recovery logic to require a non-negative elapsed duration before treating
lastReloadAt as within the cooldown, so future timestamps do not suppress
recovery. Add a regression test covering a lastReloadAt later than now and
verify reload recovery proceeds.
In `@src/lib/syntaxHighlighter.tsx`:
- Around line 32-33: Update the GraphQL highlighting path in
PrismSyntaxHighlighter so it uses the requested monokaiSublime theme
consistently with file and chat code blocks instead of always applying okaidia;
remove the unused okaidia import and preserve the existing GraphQL language
registration.
🪄 Autofix (Beta)
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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: f4e8a930-3528-4044-a7c0-bb4bf0e70ef2
📒 Files selected for processing (8)
src/components/features/chat/ChatMessagesList.tsxsrc/components/layout/Layout.tsxsrc/lib/lazyImportRecovery.tssrc/lib/routeModules.tssrc/lib/syntaxHighlighter.tsxsrc/router.tsxsrc/test/componentBehavior.test.tsxsrc/test/lazyImportRecovery.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/router.tsx
📜 Review details
⏰ Context from checks skipped due to timeout. (3)
- GitHub Check: Analyze JavaScript and TypeScript
- GitHub Check: frontend-checks
- GitHub Check: backend-checks
🧰 Additional context used
🪛 React Doctor (0.7.6)
src/components/features/chat/ChatMessagesList.tsx
[error] 49-49: This ref is mutated during render. React can replay or discard render work, so the mutation can leak from UI that never commits.
Move ref writes into an event handler or effect. Render must stay pure because React can replay or discard it. The predictable null-guarded lazy initialization pattern remains supported.
(no-ref-current-in-render)
🔇 Additional comments (8)
src/lib/routeModules.ts (1)
1-53: LGTM!src/lib/lazyImportRecovery.ts (1)
1-86: LGTM!Also applies to: 94-105
src/components/layout/Layout.tsx (1)
28-28: LGTM!Also applies to: 115-116
src/test/lazyImportRecovery.test.ts (1)
1-181: LGTM!src/lib/syntaxHighlighter.tsx (1)
1-5: LGTM!Also applies to: 64-71
src/test/componentBehavior.test.tsx (1)
2618-2618: LGTM!Also applies to: 2689-2689, 2843-2849
src/components/features/chat/ChatMessagesList.tsx (2)
14-26: LGTM!
707-718: LGTM!
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0a2d8765ce
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@backend/src/releaseManifest.ts`:
- Around line 802-805: Restrict the development fallback in the release-loading
flow around isDevelopmentFallback to ENOENT errors raised during manifest lookup
only. Track the current failure phase or isolate manifest loading from artifact
and build-identity verification, so missing artifacts during verification
continue through the non-ready issue-reporting path instead of returning
developmentRuntimeReleaseIdentity with ready: true.
🪄 Autofix (Beta)
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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: fac94042-1f2d-47e4-9ee2-72bf54874351
📒 Files selected for processing (11)
backend/src/releaseManifest.tsbackend/src/services/pullRequestPreviewHost.tsbackend/test/pullRequestPreview.test.tsbackend/test/releaseManifest.test.tsscripts/frontendBuildArtifacts.tssrc/components/features/chat/ChatMessagesList.tsxsrc/lib/lazyImportRecovery.tssrc/lib/syntaxHighlighter.tsxsrc/test/componentBehavior.test.tsxsrc/test/frontendBuildArtifacts.test.tssrc/test/lazyImportRecovery.test.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- src/test/componentBehavior.test.tsx
- src/lib/lazyImportRecovery.ts
- scripts/frontendBuildArtifacts.ts
- src/components/features/chat/ChatMessagesList.tsx
📜 Review details
⏰ Context from checks skipped due to timeout. (3)
- GitHub Check: frontend-checks
- GitHub Check: backend-checks
- GitHub Check: Analyze JavaScript and TypeScript
🧰 Additional context used
🪛 ast-grep (0.44.1)
src/test/frontendBuildArtifacts.test.ts
[warning] 140-140: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.writeFile(path.join(outdir, "assets", "entry.js"), entryContents)
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename-typescript)
🔇 Additional comments (7)
src/lib/syntaxHighlighter.tsx (1)
33-33: LGTM!Also applies to: 66-90, 108-112
src/test/lazyImportRecovery.test.ts (1)
1-17: LGTM!Also applies to: 19-28, 30-40, 42-60, 62-101, 103-127, 129-153, 155-205, 207-227
backend/src/releaseManifest.ts (1)
739-747: LGTM!Also applies to: 754-756
backend/test/releaseManifest.test.ts (1)
583-600: LGTM!backend/src/services/pullRequestPreviewHost.ts (1)
15-15: LGTM!Also applies to: 600-632, 645-649, 717-724, 748-748
backend/test/pullRequestPreview.test.ts (1)
514-519: LGTM!Also applies to: 632-638, 718-721, 822-823, 833-842
src/test/frontendBuildArtifacts.test.ts (1)
138-164: LGTM!
## Summary - resolve the single generated JavaScript output that owns `src/main.tsx` - repair Bun's generated module-script target when HTML-entry builds with splitting and metafile output point at an unrelated shared chunk - fail closed on ambiguous app outputs or module scripts, and measure the initial bundle from the real application graph ## Production incident The deployment of PR #344 (`195e665d`) passed service health checks but served an empty `#root`. Its generated `index.html` loaded a 1.7 kB shared `Switch` chunk instead of the application bootstrap. Production was rolled back successfully to `96f613d2` before this hotfix. ## Verification - `bun run lint:frontend` - `bun test src/test/frontendBuildArtifacts.test.ts` — 6 passed - `bun test` — 499 passed - `bun run build:frontend` - `bun run build:backend` - `bun run release:manifest` — 236 artifacts, both component identities on `2347976e` - clean-build browser smoke rendered the login page with no console errors Docs: not needed — this is an internal build-output correctness fix with no configuration, API, or operator-contract change.
Summary
Behavior and regression coverage
The production build now reports:
The review baseline was approximately 1.78 MB / 533 KB gzip for the eager entry bundle. Route and chat-heavy dependencies now load on demand. Navigation hover/focus preloads the target route to avoid adding avoidable interaction latency.
Static server regression coverage verifies HTML revalidation, immutable hashed caching, Brotli/gzip negotiation (including quality values), ETag and Last-Modified 304 responses, decompression, missing-asset behavior, and rejection of direct sidecar URLs. Bundle helper tests cover static-vs-dynamic graph accounting, valid compressed outputs, fail-closed metadata handling, and budget failures.
Already-open tabs recover from deployment-removed lazy chunks with one guarded hard reload on committed navigation, limited to known dynamic-import loading failures; ordinary module evaluation errors surface without reloading. Speculative hover/focus preload failures remain silent. Repeat navigation failures surface normally instead of looping. When lazy chat markdown commits, it also notifies the sticky-scroll controller so a changed row height is settled correctly.
Managed PR dev now removes only an empty preview path after pruning any stale Git worktree registration, then lets
git worktree addrecreate it. Non-empty or unsafe paths continue to fail closed.Direct source-mode development now uses the existing Git fallback before considering ignored release artifacts. Only a missing manifest can use the non-production fallback; missing or changed declared artifacts remain manifest-invalid. Production and bundled runtimes continue to verify the complete release manifest and embedded build identities strictly.
Visible UI change: syntax-highlighted code uses the Monokai Sublime palette, with the matching Okaidia/Prism palette for GraphQL; application layout and behavior are otherwise unchanged.
Verification
bun run lint:frontendbun run build:frontendbun test— 498 passedbun run lint:backendbun run build:backendbun test— 606 passedbun run release:manifest— 236 artifacts, including 136 compressed representationsRisk checklist
.envfiles, database dumps, or runtime state committedDeployment / operations
Notes for reviewers
Please focus on the initial-graph traversal/budgets, static representation negotiation/conditional request semantics, guarded lazy-import recovery, and the two development-path recovery changes.
Docs: not needed — this changes internal bundling, HTTP delivery, and development recovery behavior without changing user-facing or operator contracts, configuration, or runbooks.