improvement(perf): eight verified cuts to workspace cold-load JavaScript - #6996
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
PR SummaryMedium Risk Overview Biggest splits: Barrel / polyfill hygiene: contracts barrel no longer re-exports tools/selectors/v1/demo-requests (module-scope Zod). Streaming The tool-registry boundary baseline is retightened to lock the smaller graphs. Reviewed by Cursor Bugbot for commit 6331d1e. Configure here. |
Greptile SummaryThis PR reduces workspace cold-load JavaScript by moving heavy dependencies and specialized helpers behind direct or dynamic imports while preserving existing rendering and data behavior.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| apps/sim/app/workspace/[workspaceId]/home/hooks/use-chat.ts | Dynamically loads the desktop filesystem executor and now contains failures from both recovery imports and completion reporting. |
| apps/sim/app/workspace/[workspaceId]/logs/components/log-details/components/execution-snapshot/snapshot-boundary.tsx | Adds an error boundary and loading modal that contain rejected snapshot chunks without replacing the logs route. |
| apps/sim/app/workspace/[workspaceId]/logs/components/log-details/log-details.tsx | Lazily loads snapshots and remounts their boundary across execution and open-state transitions. |
| apps/sim/app/workspace/[workspaceId]/logs/logs.tsx | Lazily loads the log-preview snapshot with a contained loading and failure path. |
| apps/sim/lib/table/import-stream.ts | Isolates the Node streaming CSV parser from client-reachable table exports. |
| apps/sim/stores/workflows/prepare-block-state.ts | Moves registry-dependent block-state preparation out of the broadly imported workflow utilities module. |
Reviews (4): Last reviewed commit: "fix(logs): preserve snapshot modal while..." | Re-trigger Greptile
|
@cursor review |
Second round of load-time work, adversarially verified for strict behaviour preservation before implementation. Each item is an import-graph fix — none changes what renders, when it renders, or any data path: - knowledge/[id] imported one modal through the [documentId] components barrel, which also exports the chunk editor and therefore js-tiktoken (~2.5 MB gzip of BPE tables) on a route that never edits chunks. Deep import. - prepareBlockState moved out of stores/workflows/utils.ts into its own module. It is the only function there needing the block registry and the generated tool-outputs artifact (~476 KB gzip), and utils.ts is reached by the persistent shell — so every workspace route paid for a canvas-only helper, including a module-scope JSON.parse of a 5.4 MB string. - ExecutionSnapshot (the frozen-canvas modal) is now React.lazy behind its interaction gates, per the code-splitting procedure in sim-imports.md: deep import, dead barrel re-export deleted, sibling imports in log-details deepened to break the parent->child barrel cycle, local Suspense at both render sites. Takes ~7.6 MB of source off logs hydration. - The api contracts barrel no longer re-exports ./tools, ./selectors, ./v1, or ./demo-requests (~58 KB gzip of Zod schema construction on every route). Zero importers used the barrel path for any of them. - createCsvParser (streaming csv-parse, a Node Transform) moved to a server-only module so its stream polyfill leaves client bundles. Deliberately not re-exported from the lib/table barrel. - jszip is dynamically imported at both remaining static call sites (skill zip extraction, pptx parsing) — both already-async, user-triggered paths, mirroring the existing pattern in workflow import-export. - The desktop local-filesystem tool executor is dynamically imported in use-chat; a chunk-load failure now reports an error completion so the server-side tool call settles instead of hanging. Production build, JS downloaded before the load event, vs the previous release: /home 4.44 -> 3.87 MB /logs 4.44 -> 3.64 MB /knowledge 4.22 -> 3.68 MB /tables 4.17 -> 3.61 MB /files 4.68 -> 4.10 MB /w/[id] 4.80 -> 4.67 MB /home total after idle prefetch: 8.15 -> 5.52 MB The lazy snapshot was exercised end-to-end: its chunk loads when a log detail opens (off the route's cold path, warm before the View Snapshot click) and the modal renders without errors. Boundary baseline retightened.
…e local-fs tool on recovery failure Review round: wrap both lazy ExecutionSnapshot render sites in a small error boundary (Suspense handles the lazy import's pending state, not its rejection — a failed chunk load would have unwound to the route boundary and replaced the logs page over an optional modal; mirrors PreviewErrorBoundary), and contain rejections inside the local-filesystem executor's load-failure recovery so a failed completion report degrades to a log instead of an unhandled rejection.
23331cc to
357d6a2
Compare
|
@cursor review |
|
@cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 6331d1e. Configure here.
Summary
Second round of workspace load-time work. Every item below was proposed by one audit pass and then adversarially re-measured by an independent verification pass before implementation; the bar was strict behaviour preservation — nothing changes what renders, when it first renders, or any data path.
[documentId]/componentsbarrel, which also exports the chunk editor — and with itjs-tiktoken's BPE rank tables. Deep import; the document detail route that genuinely needs exact token counts is untouched.prepareBlockStateleaves the persistent shell (~476 KB gzip off nearly every route). It was the only function instores/workflows/utils.tsneeding the block registry and the generated tool-outputs artifact, andutils.tsis reached by the workspace shell through the workflow list hooks — so every route paid for a canvas-drop-only helper, including a module-scopeJSON.parseof a 5.4 MB string at shell eval. Pure code motion tostores/workflows/prepare-block-state.ts.ExecutionSnapshotis lazy (~7.6 MB of source off logs hydration). Done exactly per the "Code-splitting through barrels" procedure insim-imports.md: deepReact.lazyimport, the dead barrel re-export deleted (nosideEffects: false, so a leftover re-export silently defeats the split),log-details' sibling imports deepened to break the parent→child barrel cycle, and a local<Suspense fallback={null}>at both render sites — both of which are gated on client-only state, so there is zero SSR/hydration exposure. Exercised end-to-end: the chunk loads when a log detail opens and the modal renders cleanly../tools,./selectors,./v1,./demo-requests(~58 KB gzip on every route). Zod schemas are constructed at module scope, soexport *was defeating tree-shaking; per-integration tool contracts and the v1 admin API were shipping to the workspace home page. Zero importers used the barrel path for any of these — verified, and type-check enforces it.csv-parsemoves to a server-only module. Its streaming export is a NodeTransform, pinning stream polyfills into client bundles through thelib/tablebarrel.createCsvParsernow lives inlib/table/import-stream.ts, deliberately not barrel-re-exported; both consumers are server-side import paths. The client CSV dialog (dynamiccsv-parse/sync) is untouched.jszipgoes dynamic at its two remaining static sites (skill zip extraction, PPTX parsing) — both already-async, user-triggered functions, mirroring the existing dynamic-import pattern in workflow import-export.use-chat— with a real failure path: if the chunk fails to load (deploy skew), an error completion is reported so the server-side tool call settles instead of hanging with the dedupe ref already marked handled.Measurements
Production build, JS downloaded before the load event, vs the previous release:
/home/logs/knowledge/tables/files/w/[id]/hometotal incl. idle prefetchThe knowledge document-list route additionally sheds the 2.5 MB gzip tokenizer chunk, and the largest shared chunk shrank from 2.06 to 1.57 MB.
Rejected on correctness grounds (for the record)
The same audit rejected several tempting wins: swapping
<Home key={chatId}>for a narrower key (leaks a frame of the old conversation plus tool-recovery refs), lazy-mounting the editor panel tabs (makes SSR HTML nondeterministic → guaranteed hydration mismatch), per-viewer code-splitting of chat resource tabs (first-paint change on a persisted-tab path), andstaleTimes.dynamic(measured no-op; the segment cache already serves these navigations).Type of Change
Testing
Tested against a production build: all workspace routes render with zero page errors, and the lazy snapshot path was driven end-to-end in a real browser (open log detail → View Snapshot → frozen canvas renders). 1,746 tests pass across every touched area;
type-check,lint, all 32 audits, and the block-registry audit pass; the tool-registry boundary baseline is retightened to lock the win in.Checklist