CORE-329 feat: wire up Save 3D (Advanced) node family#13330
Open
jtydhr88 wants to merge 1 commit into
Open
Conversation
dosubot
Bot
added
the
size:L
Contributor
📝 WalkthroughWalkthroughThis PR adds Save3DAdvanced, SaveGaussianSplat, and SavePointCloud support, routes their 3D outputs to the output folder, and updates extension registration, node matching, and serialization tests. ChangesSave3D/output-folder support
Estimated code review effort: 3 (Moderate) | ~25 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 6✅ Passed checks (6 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
🎭 Playwright: ✅ 1693 passed, 0 failed · 1 flaky📊 Browser Reports
🎨 Storybook: ✅ Built — View Storybook📦 Bundle: 7.77 MB gzip 🔴 +355 BDetailsSummary
Category Glance App Entry Points — 47.4 kB (baseline 47.4 kB) • ⚪ 0 BMain entry bundles and manifests
Status: 1 added / 1 removed Graph Workspace — 1.25 MB (baseline 1.25 MB) • ⚪ 0 BGraph editor runtime, canvas, workflow orchestration
Status: 1 added / 1 removed Views & Navigation — 97.7 kB (baseline 97.7 kB) • ⚪ 0 BTop-level views, pages, and routed surfaces
Status: 9 added / 9 removed / 3 unchanged Panels & Settings — 546 kB (baseline 546 kB) • ⚪ 0 BConfiguration panels, inspectors, and settings screens
Status: 11 added / 11 removed / 16 unchanged User & Accounts — 26.9 kB (baseline 26.9 kB) • ⚪ 0 BAuthentication, profile, and account management bundles
Status: 7 added / 7 removed / 3 unchanged Editors & Dialogs — 117 kB (baseline 117 kB) • ⚪ 0 BModals, dialogs, drawers, and in-app editors
Status: 4 added / 4 removed / 1 unchanged UI Components — 57.2 kB (baseline 57.2 kB) • ⚪ 0 BReusable component library chunks
Status: 5 added / 5 removed / 8 unchanged Data & Services — 270 kB (baseline 270 kB) • ⚪ 0 BStores, services, APIs, and repositories
Status: 13 added / 13 removed / 3 unchanged Utilities & Hooks — 3.37 MB (baseline 3.37 MB) • ⚪ 0 BHelpers, composables, and utility bundles
Status: 16 added / 16 removed / 17 unchanged Vendor & Third-Party — 15.3 MB (baseline 15.3 MB) • ⚪ 0 BExternal libraries and shared vendor chunks Status: 16 unchanged Other — 11.7 MB (baseline 11.7 MB) • 🔴 +872 BBundles that do not match a named category
Status: 67 added / 67 removed / 98 unchanged ⚡ Performance Report
Show regressions
All metrics
Historical variance (last 15 runs)
Trend (last 15 commits on main)
Raw data{
"timestamp": "2026-07-01T12:36:55.481Z",
"gitSha": "5307215a4f74f64722a168a94fedb67f150982be",
"branch": "CORE-329-save-3d-advanced",
"measurements": [
{
"name": "canvas-idle",
"durationMs": 2134.619999999984,
"styleRecalcs": 9,
"styleRecalcDurationMs": 15.498999999999999,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 486.751,
"heapDeltaBytes": -7050336,
"heapUsedBytes": 62274772,
"domNodes": -281,
"jsHeapTotalBytes": 19034112,
"scriptDurationMs": 14.781999999999998,
"eventListeners": -197,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "canvas-idle",
"durationMs": 2055.9389999999667,
"styleRecalcs": 11,
"styleRecalcDurationMs": 10.839999999999998,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 389.02500000000003,
"heapDeltaBytes": -5299868,
"heapUsedBytes": 48467628,
"domNodes": 22,
"jsHeapTotalBytes": 24641536,
"scriptDurationMs": 20.756000000000004,
"eventListeners": 4,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.670000000000012,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "canvas-mouse-sweep",
"durationMs": 1830.124000000012,
"styleRecalcs": 73,
"styleRecalcDurationMs": 35.285,
"layouts": 12,
"layoutDurationMs": 4.779999999999999,
"taskDurationMs": 768.33,
"heapDeltaBytes": 19144984,
"heapUsedBytes": 71178388,
"domNodes": -282,
"jsHeapTotalBytes": 15069184,
"scriptDurationMs": 114.78999999999999,
"eventListeners": -199,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66999999999998,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "canvas-mouse-sweep",
"durationMs": 1825.0289999999723,
"styleRecalcs": 73,
"styleRecalcDurationMs": 43.921,
"layouts": 12,
"layoutDurationMs": 3.574,
"taskDurationMs": 867.205,
"heapDeltaBytes": -12820264,
"heapUsedBytes": 56635616,
"domNodes": -243,
"jsHeapTotalBytes": 19558400,
"scriptDurationMs": 129.95800000000003,
"eventListeners": -199,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.699999999999818
},
{
"name": "canvas-zoom-sweep",
"durationMs": 1752.4740000000065,
"styleRecalcs": 30,
"styleRecalcDurationMs": 16.339,
"layouts": 6,
"layoutDurationMs": 0.508,
"taskDurationMs": 342.2579999999999,
"heapDeltaBytes": 5626480,
"heapUsedBytes": 57542808,
"domNodes": -242,
"jsHeapTotalBytes": 2224128,
"scriptDurationMs": 18.543000000000003,
"eventListeners": -186,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "canvas-zoom-sweep",
"durationMs": 1747.2729999999501,
"styleRecalcs": 32,
"styleRecalcDurationMs": 18.15,
"layouts": 6,
"layoutDurationMs": 0.755,
"taskDurationMs": 348.032,
"heapDeltaBytes": -3573604,
"heapUsedBytes": 65767700,
"domNodes": -224,
"jsHeapTotalBytes": 12480512,
"scriptDurationMs": 18.166,
"eventListeners": -184,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "dom-widget-clipping",
"durationMs": 580.0570000000107,
"styleRecalcs": 12,
"styleRecalcDurationMs": 8.674000000000001,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 372.321,
"heapDeltaBytes": -5126236,
"heapUsedBytes": 47106852,
"domNodes": -272,
"jsHeapTotalBytes": 389120,
"scriptDurationMs": 53.39999999999999,
"eventListeners": -203,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333332,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "dom-widget-clipping",
"durationMs": 597.7669999999762,
"styleRecalcs": 11,
"styleRecalcDurationMs": 8.516000000000002,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 391.713,
"heapDeltaBytes": -23473584,
"heapUsedBytes": 45948300,
"domNodes": -281,
"jsHeapTotalBytes": 6713344,
"scriptDurationMs": 57.839,
"eventListeners": -203,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.699999999999818
},
{
"name": "large-graph-idle",
"durationMs": 1998.962000000006,
"styleRecalcs": 9,
"styleRecalcDurationMs": 8.241999999999999,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 513.2699999999999,
"heapDeltaBytes": -9746524,
"heapUsedBytes": 64369248,
"domNodes": 18,
"jsHeapTotalBytes": 11153408,
"scriptDurationMs": 96.33500000000001,
"eventListeners": 4,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "large-graph-idle",
"durationMs": 2040.879000000018,
"styleRecalcs": 8,
"styleRecalcDurationMs": 6.737,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 598.5899999999999,
"heapDeltaBytes": -25338496,
"heapUsedBytes": 59708404,
"domNodes": -269,
"jsHeapTotalBytes": 1658880,
"scriptDurationMs": 110.509,
"eventListeners": -197,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "large-graph-pan",
"durationMs": 2123.654999999985,
"styleRecalcs": 69,
"styleRecalcDurationMs": 17.615000000000002,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 1157.6979999999999,
"heapDeltaBytes": 19768676,
"heapUsedBytes": 81225412,
"domNodes": -276,
"jsHeapTotalBytes": 696320,
"scriptDurationMs": 442.29,
"eventListeners": -197,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333335,
"p95FrameDurationMs": 16.799999999999272
},
{
"name": "large-graph-pan",
"durationMs": 2145.857000000092,
"styleRecalcs": 69,
"styleRecalcDurationMs": 18.308999999999994,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 1216.577,
"heapDeltaBytes": -11787748,
"heapUsedBytes": 57124380,
"domNodes": -274,
"jsHeapTotalBytes": -1310720,
"scriptDurationMs": 466.34700000000004,
"eventListeners": -197,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.670000000000012,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "large-graph-zoom",
"durationMs": 3106.0979999999745,
"styleRecalcs": 67,
"styleRecalcDurationMs": 20.172000000000004,
"layouts": 60,
"layoutDurationMs": 7.513000000000001,
"taskDurationMs": 1279.7580000000003,
"heapDeltaBytes": 13821136,
"heapUsedBytes": 68867720,
"domNodes": 16,
"jsHeapTotalBytes": 6291456,
"scriptDurationMs": 486.791,
"eventListeners": 8,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.670000000000012,
"p95FrameDurationMs": 16.699999999999818
},
{
"name": "large-graph-zoom",
"durationMs": 3163.248000000067,
"styleRecalcs": 64,
"styleRecalcDurationMs": 17.136,
"layouts": 60,
"layoutDurationMs": 7.928000000000001,
"taskDurationMs": 1384.382,
"heapDeltaBytes": 13660840,
"heapUsedBytes": 66988684,
"domNodes": -280,
"jsHeapTotalBytes": 0,
"scriptDurationMs": 517.9469999999999,
"eventListeners": -195,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333332,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "minimap-idle",
"durationMs": 2038.3919999999875,
"styleRecalcs": 9,
"styleRecalcDurationMs": 8.243,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 468.618,
"heapDeltaBytes": 10828788,
"heapUsedBytes": 65778896,
"domNodes": 18,
"jsHeapTotalBytes": 5242880,
"scriptDurationMs": 86.358,
"eventListeners": 6,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "minimap-idle",
"durationMs": 2022.7989999999636,
"styleRecalcs": 7,
"styleRecalcDurationMs": 7.050999999999998,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 502.25300000000004,
"heapDeltaBytes": -3335296,
"heapUsedBytes": 59123124,
"domNodes": -275,
"jsHeapTotalBytes": -294912,
"scriptDurationMs": 88.81699999999998,
"eventListeners": -197,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "subgraph-dom-widget-clipping",
"durationMs": 578.8000000000011,
"styleRecalcs": 46,
"styleRecalcDurationMs": 14.009,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 383.481,
"heapDeltaBytes": -22002608,
"heapUsedBytes": 47501660,
"domNodes": -278,
"jsHeapTotalBytes": 5664768,
"scriptDurationMs": 121.018,
"eventListeners": -197,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "subgraph-dom-widget-clipping",
"durationMs": 564.7450000000163,
"styleRecalcs": 46,
"styleRecalcDurationMs": 10.479000000000001,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 375.087,
"heapDeltaBytes": 7754192,
"heapUsedBytes": 66679196,
"domNodes": 18,
"jsHeapTotalBytes": 19922944,
"scriptDurationMs": 128.349,
"eventListeners": 6,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "subgraph-idle",
"durationMs": 2053.092000000049,
"styleRecalcs": 12,
"styleRecalcDurationMs": 10.786,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 386.75,
"heapDeltaBytes": -7916228,
"heapUsedBytes": 61631412,
"domNodes": -273,
"jsHeapTotalBytes": 18509824,
"scriptDurationMs": 12.961,
"eventListeners": -199,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "subgraph-idle",
"durationMs": 2032.5840000000426,
"styleRecalcs": 10,
"styleRecalcDurationMs": 10.139,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 422.986,
"heapDeltaBytes": 12334404,
"heapUsedBytes": 64575532,
"domNodes": -269,
"jsHeapTotalBytes": 17166336,
"scriptDurationMs": 19.498999999999995,
"eventListeners": -199,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333335,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "subgraph-mouse-sweep",
"durationMs": 1708.5289999999986,
"styleRecalcs": 77,
"styleRecalcDurationMs": 38.112,
"layouts": 16,
"layoutDurationMs": 4.292,
"taskDurationMs": 703.442,
"heapDeltaBytes": -17249412,
"heapUsedBytes": 52475432,
"domNodes": -235,
"jsHeapTotalBytes": 20082688,
"scriptDurationMs": 91.486,
"eventListeners": -199,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "subgraph-mouse-sweep",
"durationMs": 1697.0639999999548,
"styleRecalcs": 76,
"styleRecalcDurationMs": 38.217,
"layouts": 16,
"layoutDurationMs": 4.134,
"taskDurationMs": 707.707,
"heapDeltaBytes": -6529752,
"heapUsedBytes": 59184860,
"domNodes": 64,
"jsHeapTotalBytes": 20480000,
"scriptDurationMs": 95.97,
"eventListeners": 4,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "subgraph-transition-enter",
"durationMs": 1002.4700000000166,
"styleRecalcs": 17,
"styleRecalcDurationMs": 27.851999999999993,
"layouts": 4,
"layoutDurationMs": 13.561999999999998,
"taskDurationMs": 753.0540000000001,
"heapDeltaBytes": 4557540,
"heapUsedBytes": 81096796,
"domNodes": 13833,
"jsHeapTotalBytes": 18350080,
"scriptDurationMs": 29.53,
"eventListeners": 2533,
"totalBlockingTimeMs": 163,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "viewport-pan-sweep",
"durationMs": 8191.720999999972,
"styleRecalcs": 253,
"styleRecalcDurationMs": 55.556,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 3846.976,
"heapDeltaBytes": 8403988,
"heapUsedBytes": 75976712,
"domNodes": -265,
"jsHeapTotalBytes": 5185536,
"scriptDurationMs": 1384.498,
"eventListeners": -183,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "viewport-pan-sweep",
"durationMs": 8191.220000000044,
"styleRecalcs": 251,
"styleRecalcDurationMs": 54.949,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 3747.946,
"heapDeltaBytes": 837588,
"heapUsedBytes": 68224004,
"domNodes": -272,
"jsHeapTotalBytes": 729088,
"scriptDurationMs": 1243.031,
"eventListeners": -183,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333332,
"p95FrameDurationMs": 16.80000000000109
},
{
"name": "vue-large-graph-idle",
"durationMs": 12175.882000000001,
"styleRecalcs": 0,
"styleRecalcDurationMs": 0,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 12160.404999999999,
"heapDeltaBytes": -23865040,
"heapUsedBytes": 172079116,
"domNodes": -3302,
"jsHeapTotalBytes": 21204992,
"scriptDurationMs": 493.522,
"eventListeners": -16374,
"totalBlockingTimeMs": 0,
"frameDurationMs": 17.223333333333358,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "vue-large-graph-idle",
"durationMs": 12291.801999999961,
"styleRecalcs": 0,
"styleRecalcDurationMs": 0,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 12277.699000000002,
"heapDeltaBytes": -59511464,
"heapUsedBytes": 167900628,
"domNodes": -3300,
"jsHeapTotalBytes": -8126464,
"scriptDurationMs": 535.29,
"eventListeners": -16380,
"totalBlockingTimeMs": 0,
"frameDurationMs": 17.223333333333358,
"p95FrameDurationMs": 16.80000000000291
},
{
"name": "vue-large-graph-pan",
"durationMs": 14441.835999999967,
"styleRecalcs": 66,
"styleRecalcDurationMs": 16.868000000000023,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 14419.890000000003,
"heapDeltaBytes": -39496912,
"heapUsedBytes": 156301768,
"domNodes": -3302,
"jsHeapTotalBytes": 18583552,
"scriptDurationMs": 812.43,
"eventListeners": -16372,
"totalBlockingTimeMs": 0,
"frameDurationMs": 17.223333333333237,
"p95FrameDurationMs": 16.799999999999272
},
{
"name": "vue-large-graph-pan",
"durationMs": 14130.325999999968,
"styleRecalcs": 66,
"styleRecalcDurationMs": 17.230999999999995,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 14098.892,
"heapDeltaBytes": -52157496,
"heapUsedBytes": 154735456,
"domNodes": -3300,
"jsHeapTotalBytes": -524288,
"scriptDurationMs": 803.685,
"eventListeners": -16366,
"totalBlockingTimeMs": 0,
"frameDurationMs": 17.223333333333358,
"p95FrameDurationMs": 16.799999999999272
},
{
"name": "workflow-execution",
"durationMs": 541.1900000000287,
"styleRecalcs": 21,
"styleRecalcDurationMs": 27.843999999999998,
"layouts": 5,
"layoutDurationMs": 1.534,
"taskDurationMs": 229.516,
"heapDeltaBytes": -21773684,
"heapUsedBytes": 48912344,
"domNodes": -155,
"jsHeapTotalBytes": 5402624,
"scriptDurationMs": 20.416,
"eventListeners": -144,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.670000000000012,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "workflow-execution",
"durationMs": 175.87600000001657,
"styleRecalcs": 12,
"styleRecalcDurationMs": 24.985999999999994,
"layouts": 4,
"layoutDurationMs": 2.2430000000000003,
"taskDurationMs": 140.614,
"heapDeltaBytes": -4028880,
"heapUsedBytes": 49523880,
"domNodes": -153,
"jsHeapTotalBytes": -921600,
"scriptDurationMs": 17.072000000000003,
"eventListeners": -166,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333332,
"p95FrameDurationMs": 16.700000000000728
}
]
} |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Codecov Report❌ Patch coverage is
@@ Coverage Diff @@
## main #13330 +/- ##
==========================================
+ Coverage 77.55% 77.58% +0.03%
==========================================
Files 1633 1639 +6
Lines 97390 97469 +79
Branches 32870 33558 +688
==========================================
+ Hits 75528 75621 +93
+ Misses 21160 21152 -8
+ Partials 702 696 -6
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 29 files with indirect coverage changes 🚀 New features to boost your workflow:
|
Member
|
@jtydhr88 I think we need to add the save nodes to |
jtydhr88
force-pushed
the
CORE-329-save-3d-advanced
branch
from
2c4513c to
9b649d9
Compare
Collaborator
Author
fixed |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/extensions/core/load3d.ts`:
- Around line 716-747: `useLoad3d(node)` is being called twice in the same setup
path, which creates duplicate watchers and reactive overhead. Refactor this
section to call `useLoad3d(node)` once, destructure both `onLoad3dReady` and
`waitForLoad3d` from that single result, and then use those methods for the
existing camera-restore and load-wait logic. Follow the same single-call pattern
used in the sibling `load3dPreviewExtensions` implementation so the `load3d`
lifecycle setup stays lean.
In `@src/extensions/core/load3dPreviewExtensions.test.ts`:
- Around line 159-195: The new save-extension test only covers the onExecuted
flow, so add coverage for the nodeCreated restore path in the save variants. In
load3dPreviewExtensions.test.ts, exercise SaveGaussianSplat and SavePointCloud
with a persisted “Last Time Model File” on the node and verify nodeCreated
routes through onLoad3dReady to configureForSaveMesh using the output folder
(not temp), matching the Save3DAdvanced restore-path pattern in load3d.test.ts.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 32d45b1a-df91-4d4e-8cb5-a296b67a9f51
📒 Files selected for processing (8)
src/extensions/core/load3d.test.tssrc/extensions/core/load3d.tssrc/extensions/core/load3d/nodeTypes.tssrc/extensions/core/load3dLazy.test.tssrc/extensions/core/load3dPreviewExtensions.test.tssrc/extensions/core/load3dPreviewExtensions.tssrc/extensions/core/saveImageExtraOutput.test.tssrc/extensions/core/saveImageExtraOutput.ts
Comment on lines
+716
to
+747
| useLoad3d(node).onLoad3dReady((load3d) => { | ||
| const lastTimeModelFile = node.properties['Last Time Model File'] | ||
| if (!lastTimeModelFile) return | ||
|
|
||
| const config = new Load3DConfiguration(load3d, node.properties) | ||
| config.configureForSaveMesh('temp', lastTimeModelFile as string, { | ||
| silentOnNotFound: true | ||
| const config = new Load3DConfiguration(load3d, node.properties) | ||
| config.configureForSaveMesh(loadFolder, lastTimeModelFile as string, { | ||
| silentOnNotFound: true | ||
| }) | ||
|
|
||
| const cameraConfig = node.properties['Camera Config'] as | ||
| | CameraConfig | ||
| | undefined | ||
| const cameraState = cameraConfig?.state | ||
| if (!cameraState) return | ||
|
|
||
| const targetGeneration = load3d.currentLoadGeneration | ||
| void load3d | ||
| .whenLoadIdle() | ||
| .then(() => { | ||
| if (load3d.currentLoadGeneration !== targetGeneration) return | ||
| load3d.setCameraState(cameraState) | ||
| load3d.forceRender() | ||
| }) | ||
| .catch((error) => { | ||
| console.error( | ||
| `Failed to restore camera state for ${comfyClass}:`, | ||
| error | ||
| ) | ||
| }) | ||
| }) | ||
|
|
||
| const cameraConfig = node.properties['Camera Config'] as | ||
| | CameraConfig | ||
| | undefined | ||
| const cameraState = cameraConfig?.state | ||
| if (!cameraState) return | ||
| useLoad3d(node).waitForLoad3d((load3d) => { |
Contributor
There was a problem hiding this comment.
🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win
Avoid calling useLoad3d(node) twice — destructure once instead.
useLoad3d(node) is invoked separately for onLoad3dReady (Line 716) and waitForLoad3d (Line 747). Since the composable sets up several top-level watch() calls on every invocation, calling it twice for the same node doubles the reactive overhead (refs/watchers) outside any component lifecycle. The sibling implementation in load3dPreviewExtensions.ts (Line 117) destructures both functions from a single call — this factory should follow the same pattern.
As per coding guidelines, "Keep component and composable surface area small; avoid unnecessary refs, computed values, and watchers when simpler props or direct values suffice."
♻️ Proposed fix
- useLoad3d(node).onLoad3dReady((load3d) => {
+ const { onLoad3dReady, waitForLoad3d } = useLoad3d(node)
+
+ onLoad3dReady((load3d) => {
const lastTimeModelFile = node.properties['Last Time Model File']
if (!lastTimeModelFile) return
...
})
- useLoad3d(node).waitForLoad3d((load3d) => {
+ waitForLoad3d((load3d) => {
const sceneWidget = node.widgets?.find(📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| useLoad3d(node).onLoad3dReady((load3d) => { | |
| const lastTimeModelFile = node.properties['Last Time Model File'] | |
| if (!lastTimeModelFile) return | |
| const config = new Load3DConfiguration(load3d, node.properties) | |
| config.configureForSaveMesh('temp', lastTimeModelFile as string, { | |
| silentOnNotFound: true | |
| const config = new Load3DConfiguration(load3d, node.properties) | |
| config.configureForSaveMesh(loadFolder, lastTimeModelFile as string, { | |
| silentOnNotFound: true | |
| }) | |
| const cameraConfig = node.properties['Camera Config'] as | |
| | CameraConfig | |
| | undefined | |
| const cameraState = cameraConfig?.state | |
| if (!cameraState) return | |
| const targetGeneration = load3d.currentLoadGeneration | |
| void load3d | |
| .whenLoadIdle() | |
| .then(() => { | |
| if (load3d.currentLoadGeneration !== targetGeneration) return | |
| load3d.setCameraState(cameraState) | |
| load3d.forceRender() | |
| }) | |
| .catch((error) => { | |
| console.error( | |
| `Failed to restore camera state for ${comfyClass}:`, | |
| error | |
| ) | |
| }) | |
| }) | |
| const cameraConfig = node.properties['Camera Config'] as | |
| | CameraConfig | |
| | undefined | |
| const cameraState = cameraConfig?.state | |
| if (!cameraState) return | |
| useLoad3d(node).waitForLoad3d((load3d) => { | |
| const { onLoad3dReady, waitForLoad3d } = useLoad3d(node) | |
| onLoad3dReady((load3d) => { | |
| const lastTimeModelFile = node.properties['Last Time Model File'] | |
| if (!lastTimeModelFile) return | |
| const config = new Load3DConfiguration(load3d, node.properties) | |
| config.configureForSaveMesh(loadFolder, lastTimeModelFile as string, { | |
| silentOnNotFound: true | |
| }) | |
| const cameraConfig = node.properties['Camera Config'] as | |
| | CameraConfig | |
| | undefined | |
| const cameraState = cameraConfig?.state | |
| if (!cameraState) return | |
| const targetGeneration = load3d.currentLoadGeneration | |
| void load3d | |
| .whenLoadIdle() | |
| .then(() => { | |
| if (load3d.currentLoadGeneration !== targetGeneration) return | |
| load3d.setCameraState(cameraState) | |
| load3d.forceRender() | |
| }) | |
| .catch((error) => { | |
| console.error( | |
| `Failed to restore camera state for ${comfyClass}:`, | |
| error | |
| ) | |
| }) | |
| }) | |
| waitForLoad3d((load3d) => { |
🤖 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 `@src/extensions/core/load3d.ts` around lines 716 - 747, `useLoad3d(node)` is
being called twice in the same setup path, which creates duplicate watchers and
reactive overhead. Refactor this section to call `useLoad3d(node)` once,
destructure both `onLoad3dReady` and `waitForLoad3d` from that single result,
and then use those methods for the existing camera-restore and load-wait logic.
Follow the same single-call pattern used in the sibling
`load3dPreviewExtensions` implementation so the `load3d` lifecycle setup stays
lean.
Source: Coding guidelines
Comment on lines
+159
to
+195
| it('registers preview and save extensions on import', async () => { | ||
| const { splatExt, pointCloudExt, saveSplatExt, savePointCloudExt } = | ||
| await loadExtensionsFresh() | ||
|
|
||
| expect(registerExtensionMock).toHaveBeenCalledTimes(2) | ||
| expect(registerExtensionMock).toHaveBeenCalledTimes(4) | ||
| expect(splatExt.name).toBe('Comfy.PreviewGaussianSplat') | ||
| expect(pointCloudExt.name).toBe('Comfy.PreviewPointCloud') | ||
| expect(saveSplatExt.name).toBe('Comfy.SaveGaussianSplat') | ||
| expect(savePointCloudExt.name).toBe('Comfy.SavePointCloud') | ||
| }) | ||
|
|
||
| it('save extensions load the saved file from the output folder, not temp', async () => { | ||
| const { saveSplatExt, savePointCloudExt } = await loadExtensionsFresh() | ||
| const load3d = makeLoad3dMock() | ||
| waitForLoad3dMock.mockImplementation((cb: (l: FakeLoad3d) => void) => | ||
| cb(load3d) | ||
| ) | ||
|
|
||
| const splatNode = makePreviewNode({ comfyClass: 'SaveGaussianSplat' }) | ||
| await saveSplatExt.nodeCreated(splatNode) | ||
| splatNode.onExecuted!({ result: ['3d/ComfyUI_00001_.ply'] }) | ||
|
|
||
| expect(configureForSaveMeshMock).toHaveBeenLastCalledWith( | ||
| 'output', | ||
| '3d/ComfyUI_00001_.ply', | ||
| expect.objectContaining({ silentOnNotFound: true }) | ||
| ) | ||
|
|
||
| const pcNode = makePreviewNode({ comfyClass: 'SavePointCloud' }) | ||
| await savePointCloudExt.nodeCreated(pcNode) | ||
| pcNode.onExecuted!({ result: ['3d/ComfyUI_00002_.ply'] }) | ||
|
|
||
| expect(configureForSaveMeshMock).toHaveBeenLastCalledWith( | ||
| 'output', | ||
| '3d/ComfyUI_00002_.ply', | ||
| expect.objectContaining({ silentOnNotFound: true }) | ||
| ) |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
Add coverage for the nodeCreated restore path on save variants.
The added test only exercises the onExecuted path. The sibling nodeCreated→onLoad3dReady restore path (using persisted Last Time Model File) that reads from loadFolder is untested here, unlike the equivalent test added for Save3DAdvanced in load3d.test.ts.
✅ Suggested additional test
it('restores persisted models from the output folder on nodeCreated, not temp', async () => {
const { saveSplatExt } = await loadExtensionsFresh()
const node = makePreviewNode({
comfyClass: 'SaveGaussianSplat',
properties: { 'Last Time Model File': '3d/ComfyUI_00001_.ply' }
})
await saveSplatExt.nodeCreated(node)
expect(configureForSaveMeshMock).toHaveBeenCalledWith(
'output',
'3d/ComfyUI_00001_.ply',
expect.objectContaining({ silentOnNotFound: true })
)
})🤖 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 `@src/extensions/core/load3dPreviewExtensions.test.ts` around lines 159 - 195,
The new save-extension test only covers the onExecuted flow, so add coverage for
the nodeCreated restore path in the save variants. In
load3dPreviewExtensions.test.ts, exercise SaveGaussianSplat and SavePointCloud
with a persisted “Last Time Model File” on the node and verify nodeCreated
routes through onLoad3dReady to configureForSaveMesh using the output folder
(not temp), matching the Save3DAdvanced restore-path pattern in load3d.test.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
2 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
Register the save-side advanced nodes in the Load3D viewer infrastructure: Save3DAdvanced reuses the mesh advanced extension, while SaveGaussianSplat and SavePointCloud reuse the splat/point cloud preview extensions.
Parameterize both extension factories with a loadFolder so save nodes load the persisted file from the output folder instead of temp, and add the node types to the lazy-load and viewport-state sets.
BE change Comfy-Org/ComfyUI#14701
Screenshots (if applicable)
Save 3D (Advanced)
Save Splat
