fix(website): slugify cloud-node pack ids and mock e2e snapshot#12277
Draft
DrJKL wants to merge 7 commits into
Draft
fix(website): slugify cloud-node pack ids and mock e2e snapshot#12277DrJKL wants to merge 7 commits into
DrJKL wants to merge 7 commits into
Conversation
The website's /cloud/supported-nodes routes use pack ids as URL segments, but pack ids come straight from upstream Python module names which mix PascalCase, snake_case, and kebab-case freely. After a Release: Website snapshot refresh that surfaced packs like ComfyUI-Crystools and basic_data_handling, the Playwright assertions in cloud-nodes.spec.ts broke because the slug regex requires kebab-case and the hardcoded comfyui-impact-pack assertions assumed registry contents would never change. Two coordinated fixes: * slugifyPackId in @comfyorg/object-info-parser normalizes every pack id (lowercase + _\u2192-) at the boundaries where it enters the website: groupNodesByPack for fresh fetches and readSnapshot for the bundled fallback. The raw upstream id is preserved as rawId/registryId so registry enrichment continues to hit https://api.comfy.org/nodes with the exact node_id the API expects. Packs whose raw ids collide on the same slug (24 such pairs exist in the live registry today, e.g. ComfyUI-QwenVL + ComfyUI_QwenVL) are merged deterministically instead of producing duplicate static routes. * WEBSITE_CLOUD_NODES_FIXTURE lets the e2e build read a committed fixture snapshot in place of the bundled one. CI sets it to e2e/fixtures/cloud-nodes.fixture.json so Playwright assertions run against deterministic pack content; future snapshot refreshes can no longer break the test contract by removing the Impact Pack or adding underscored ids. Verification: * 99/99 desktop Playwright tests pass against the fixture-built site * 81/81 website Vitest unit tests pass (including new build/fixture coverage) * 48/48 object-info-parser tests pass (11 new slugifyPackId cases, 4 new groupNodesByPack cases) * astro check: 0 errors
Addresses Oracle review feedback: when two raw upstream ids slugify to the same URL slug (e.g. ComfyUI-QwenVL + ComfyUI_QwenVL both -> comfyui-qwenvl) the previous merge kept only the first rawId and used only that single alias to fetch registry metadata. If that one alias missed but its twin would have resolved, the merged pack lost banner/icon/license info. Now NodePack carries rawIds: string[] holding every raw alias seen for the slug. parseCloudNodes flattens all aliases into a single registry batch and pickRegistryPack walks the alias list in insertion order to find the first non-null hit.
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughNormalize cloud node pack IDs to URL-safe slugs, track raw upstream aliases, merge slug-collided snapshot packs, use an optional on-disk snapshot fixture during website builds (CI e2e), and add tests covering slugification, alias resolution, normalization, and merge semantics. ChangesPack ID slugification, grouping, and registry aliasing
Build-time snapshot override & CI fixture
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (1 error, 1 warning)
✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
🎨 Storybook: ✅ Built — View Storybook |
🎭 Playwright: ✅ 1606 passed, 0 failed · 1 flaky📊 Browser Reports
|
🌐 Website E2ETip All tests passed.
🔗 Website PreviewWebsite Preview: https://comfy-website-preview-pr-12277.vercel.app This commit: https://website-frontend-rgkvdx4yr-comfyui.vercel.app Last updated: 2026-05-15T14:39:39Z for |
📦 Bundle: 5.36 MB gzip 🔴 +197 BDetailsSummary
Category Glance App Entry Points — 26.1 kB (baseline 26.1 kB) • ⚪ 0 BMain entry bundles and manifests
Status: 1 added / 1 removed Graph Workspace — 1.24 MB (baseline 1.24 MB) • ⚪ 0 BGraph editor runtime, canvas, workflow orchestration
Status: 1 added / 1 removed Views & Navigation — 82.9 kB (baseline 82.9 kB) • ⚪ 0 BTop-level views, pages, and routed surfaces
Status: 9 added / 9 removed / 2 unchanged Panels & Settings — 527 kB (baseline 527 kB) • ⚪ 0 BConfiguration panels, inspectors, and settings screens
Status: 10 added / 10 removed / 14 unchanged User & Accounts — 17.8 kB (baseline 17.8 kB) • ⚪ 0 BAuthentication, profile, and account management bundles
Status: 5 added / 5 removed / 2 unchanged Editors & Dialogs — 112 kB (baseline 112 kB) • ⚪ 0 BModals, dialogs, drawers, and in-app editors
Status: 4 added / 4 removed UI Components — 58 kB (baseline 58 kB) • ⚪ 0 BReusable component library chunks
Status: 5 added / 5 removed / 8 unchanged Data & Services — 3.16 MB (baseline 3.16 MB) • ⚪ 0 BStores, services, APIs, and repositories
Status: 13 added / 13 removed / 4 unchanged Utilities & Hooks — 366 kB (baseline 366 kB) • ⚪ 0 BHelpers, composables, and utility bundles
Status: 13 added / 13 removed / 18 unchanged Vendor & Third-Party — 9.94 MB (baseline 9.94 MB) • ⚪ 0 BExternal libraries and shared vendor chunks Status: 16 unchanged Other — 9.16 MB (baseline 9.16 MB) • ⚪ 0 BBundles that do not match a named category
Status: 57 added / 57 removed / 86 unchanged ⚡ Performance Report
No regressions detected. All metrics
Historical variance (last 15 runs)
Trend (last 15 commits on main)
Raw data{
"timestamp": "2026-05-15T14:48:39.557Z",
"gitSha": "412202f553c97c63ef5563dcd4e1a9bb2698f2d3",
"branch": "glary/website-cloud-nodes-mock-and-slugify",
"measurements": [
{
"name": "canvas-idle",
"durationMs": 2063.379999999995,
"styleRecalcs": 8,
"styleRecalcDurationMs": 8.152,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 417.553,
"heapDeltaBytes": 19311556,
"heapUsedBytes": 69430224,
"domNodes": -264,
"jsHeapTotalBytes": 18046976,
"scriptDurationMs": 15.873999999999999,
"eventListeners": -131,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "canvas-idle",
"durationMs": 2013.8499999999908,
"styleRecalcs": 10,
"styleRecalcDurationMs": 10.328000000000001,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 361.002,
"heapDeltaBytes": 24208760,
"heapUsedBytes": 73433200,
"domNodes": 20,
"jsHeapTotalBytes": 14417920,
"scriptDurationMs": 24.034,
"eventListeners": 6,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.670000000000012,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "canvas-mouse-sweep",
"durationMs": 1704.7370000000228,
"styleRecalcs": 71,
"styleRecalcDurationMs": 29.459000000000003,
"layouts": 12,
"layoutDurationMs": 3.138,
"taskDurationMs": 683.932,
"heapDeltaBytes": 18351144,
"heapUsedBytes": 68061972,
"domNodes": 52,
"jsHeapTotalBytes": 15728640,
"scriptDurationMs": 116.041,
"eventListeners": 4,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "canvas-mouse-sweep",
"durationMs": 2020.3669999999647,
"styleRecalcs": 78,
"styleRecalcDurationMs": 37.43299999999999,
"layouts": 12,
"layoutDurationMs": 3.3680000000000003,
"taskDurationMs": 938.7630000000001,
"heapDeltaBytes": 3616064,
"heapUsedBytes": 52064572,
"domNodes": -263,
"jsHeapTotalBytes": 16117760,
"scriptDurationMs": 120.77899999999998,
"eventListeners": -131,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333332,
"p95FrameDurationMs": 16.699999999999818
},
{
"name": "canvas-zoom-sweep",
"durationMs": 1710.470999999984,
"styleRecalcs": 31,
"styleRecalcDurationMs": 18.333000000000002,
"layouts": 6,
"layoutDurationMs": 0.6219999999999999,
"taskDurationMs": 281.03499999999997,
"heapDeltaBytes": 512140,
"heapUsedBytes": 49148772,
"domNodes": 75,
"jsHeapTotalBytes": 14942208,
"scriptDurationMs": 16.881,
"eventListeners": 19,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "canvas-zoom-sweep",
"durationMs": 1736.1339999999927,
"styleRecalcs": 32,
"styleRecalcDurationMs": 15.771999999999998,
"layouts": 6,
"layoutDurationMs": 0.525,
"taskDurationMs": 282.69999999999993,
"heapDeltaBytes": 533016,
"heapUsedBytes": 49129132,
"domNodes": 79,
"jsHeapTotalBytes": 14680064,
"scriptDurationMs": 17.183999999999997,
"eventListeners": 21,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "dom-widget-clipping",
"durationMs": 605.7559999999853,
"styleRecalcs": 12,
"styleRecalcDurationMs": 8.040000000000003,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 365.938,
"heapDeltaBytes": 15303420,
"heapUsedBytes": 64526260,
"domNodes": 19,
"jsHeapTotalBytes": 17039360,
"scriptDurationMs": 60.515,
"eventListeners": 2,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.663333333333338,
"p95FrameDurationMs": 16.700000000000273
},
{
"name": "dom-widget-clipping",
"durationMs": 536.687000000029,
"styleRecalcs": 13,
"styleRecalcDurationMs": 9.662,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 343.418,
"heapDeltaBytes": 9441176,
"heapUsedBytes": 57479164,
"domNodes": 22,
"jsHeapTotalBytes": 15466496,
"scriptDurationMs": 64.91199999999999,
"eventListeners": 2,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.670000000000012,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "large-graph-idle",
"durationMs": 2013.9159999999947,
"styleRecalcs": 9,
"styleRecalcDurationMs": 8.426,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 502.74299999999994,
"heapDeltaBytes": 8118224,
"heapUsedBytes": 66656460,
"domNodes": -262,
"jsHeapTotalBytes": 290816,
"scriptDurationMs": 85.02399999999999,
"eventListeners": -129,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333335,
"p95FrameDurationMs": 16.699999999999818
},
{
"name": "large-graph-idle",
"durationMs": 2026.818999999989,
"styleRecalcs": 8,
"styleRecalcDurationMs": 7.393000000000001,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 500.707,
"heapDeltaBytes": 2892264,
"heapUsedBytes": 61455272,
"domNodes": -264,
"jsHeapTotalBytes": 4485120,
"scriptDurationMs": 84.60099999999998,
"eventListeners": -129,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "large-graph-pan",
"durationMs": 2127.346999999986,
"styleRecalcs": 68,
"styleRecalcDurationMs": 16.773000000000003,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 1064.7540000000001,
"heapDeltaBytes": 60631140,
"heapUsedBytes": 120704380,
"domNodes": -262,
"jsHeapTotalBytes": 35360768,
"scriptDurationMs": 376.206,
"eventListeners": -129,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "large-graph-pan",
"durationMs": 2159.694000000002,
"styleRecalcs": 67,
"styleRecalcDurationMs": 14.226999999999997,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 1072.74,
"heapDeltaBytes": 56495260,
"heapUsedBytes": 117775632,
"domNodes": -252,
"jsHeapTotalBytes": 36700160,
"scriptDurationMs": 380.858,
"eventListeners": -127,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "large-graph-zoom",
"durationMs": 3134.1810000000123,
"styleRecalcs": 66,
"styleRecalcDurationMs": 16.901000000000003,
"layouts": 60,
"layoutDurationMs": 6.988000000000001,
"taskDurationMs": 1275.849,
"heapDeltaBytes": 36702312,
"heapUsedBytes": 104098628,
"domNodes": 14,
"jsHeapTotalBytes": 58339328,
"scriptDurationMs": 475.14099999999996,
"eventListeners": 8,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "large-graph-zoom",
"durationMs": 3184.0459999999666,
"styleRecalcs": 65,
"styleRecalcDurationMs": 16.720000000000002,
"layouts": 60,
"layoutDurationMs": 6.816,
"taskDurationMs": 1340.879,
"heapDeltaBytes": 49686516,
"heapUsedBytes": 111035400,
"domNodes": -269,
"jsHeapTotalBytes": 38039552,
"scriptDurationMs": 488.128,
"eventListeners": -125,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "minimap-idle",
"durationMs": 1993.3689999999729,
"styleRecalcs": 10,
"styleRecalcDurationMs": 8.106000000000002,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 524.548,
"heapDeltaBytes": 45086184,
"heapUsedBytes": 113364416,
"domNodes": 20,
"jsHeapTotalBytes": 47853568,
"scriptDurationMs": 83.231,
"eventListeners": 4,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "minimap-idle",
"durationMs": 2071.8669999999975,
"styleRecalcs": 10,
"styleRecalcDurationMs": 9.707,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 597.62,
"heapDeltaBytes": 45283132,
"heapUsedBytes": 105145920,
"domNodes": -256,
"jsHeapTotalBytes": 32272384,
"scriptDurationMs": 101.06099999999998,
"eventListeners": -129,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333335,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "subgraph-dom-widget-clipping",
"durationMs": 519.9220000000082,
"styleRecalcs": 47,
"styleRecalcDurationMs": 10.865,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 344.003,
"heapDeltaBytes": 9840420,
"heapUsedBytes": 59364264,
"domNodes": 20,
"jsHeapTotalBytes": 15990784,
"scriptDurationMs": 121.54100000000001,
"eventListeners": 8,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000273
},
{
"name": "subgraph-dom-widget-clipping",
"durationMs": 511.4450000000943,
"styleRecalcs": 46,
"styleRecalcDurationMs": 10.273000000000001,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 341.38500000000005,
"heapDeltaBytes": 8883932,
"heapUsedBytes": 57740292,
"domNodes": 18,
"jsHeapTotalBytes": 15990784,
"scriptDurationMs": 120.17399999999999,
"eventListeners": 6,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.663333333333338,
"p95FrameDurationMs": 16.799999999999727
},
{
"name": "subgraph-idle",
"durationMs": 1994.9060000000145,
"styleRecalcs": 8,
"styleRecalcDurationMs": 6.111999999999999,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 357.35999999999996,
"heapDeltaBytes": 38971396,
"heapUsedBytes": 92000372,
"domNodes": 16,
"jsHeapTotalBytes": 45875200,
"scriptDurationMs": 12.604999999999999,
"eventListeners": 4,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "subgraph-idle",
"durationMs": 2016.1330000000248,
"styleRecalcs": 9,
"styleRecalcDurationMs": 7.820999999999998,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 361.282,
"heapDeltaBytes": -5861468,
"heapUsedBytes": 43167536,
"domNodes": 18,
"jsHeapTotalBytes": 15593472,
"scriptDurationMs": 12.683000000000007,
"eventListeners": 4,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.699999999999818
},
{
"name": "subgraph-mouse-sweep",
"durationMs": 1684.759999999983,
"styleRecalcs": 76,
"styleRecalcDurationMs": 34.8,
"layouts": 16,
"layoutDurationMs": 4.14,
"taskDurationMs": 625.5959999999999,
"heapDeltaBytes": 15240996,
"heapUsedBytes": 64646364,
"domNodes": 62,
"jsHeapTotalBytes": 15466496,
"scriptDurationMs": 90.417,
"eventListeners": 6,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "subgraph-mouse-sweep",
"durationMs": 1988.2729999999924,
"styleRecalcs": 87,
"styleRecalcDurationMs": 43.258,
"layouts": 16,
"layoutDurationMs": 4.264,
"taskDurationMs": 882.659,
"heapDeltaBytes": 14426256,
"heapUsedBytes": 63017344,
"domNodes": 71,
"jsHeapTotalBytes": 14680064,
"scriptDurationMs": 91.061,
"eventListeners": 6,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "subgraph-transition-enter",
"durationMs": 1057.7220000000125,
"styleRecalcs": 14,
"styleRecalcDurationMs": 27.92,
"layouts": 5,
"layoutDurationMs": 13.739,
"taskDurationMs": 856.9999999999998,
"heapDeltaBytes": -17104680,
"heapUsedBytes": 138679708,
"domNodes": 12475,
"jsHeapTotalBytes": -11177984,
"scriptDurationMs": 34.579999999999984,
"eventListeners": 1288,
"totalBlockingTimeMs": 158,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.799999999999272
},
{
"name": "viewport-pan-sweep",
"durationMs": 8108.627999999953,
"styleRecalcs": 249,
"styleRecalcDurationMs": 48.300999999999995,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 3422.307,
"heapDeltaBytes": 15558080,
"heapUsedBytes": 74259296,
"domNodes": -260,
"jsHeapTotalBytes": 2854912,
"scriptDurationMs": 1184.485,
"eventListeners": -113,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.80000000000109
},
{
"name": "viewport-pan-sweep",
"durationMs": 8142.048999999929,
"styleRecalcs": 249,
"styleRecalcDurationMs": 49.15,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 3499.708,
"heapDeltaBytes": 13036572,
"heapUsedBytes": 71488996,
"domNodes": -262,
"jsHeapTotalBytes": 2068480,
"scriptDurationMs": 1209.77,
"eventListeners": -113,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333332,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "vue-large-graph-idle",
"durationMs": 11172.401000000036,
"styleRecalcs": 0,
"styleRecalcDurationMs": 0,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 11159.42,
"heapDeltaBytes": -28362708,
"heapUsedBytes": 163383828,
"domNodes": -3310,
"jsHeapTotalBytes": 17534976,
"scriptDurationMs": 549.584,
"eventListeners": -16472,
"totalBlockingTimeMs": 0,
"frameDurationMs": 17.223333333333358,
"p95FrameDurationMs": 16.80000000000291
},
{
"name": "vue-large-graph-idle",
"durationMs": 10725.865999999996,
"styleRecalcs": 0,
"styleRecalcDurationMs": 0,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 10717.364,
"heapDeltaBytes": -33220608,
"heapUsedBytes": 164894228,
"domNodes": -8331,
"jsHeapTotalBytes": 14479360,
"scriptDurationMs": 543.5799999999999,
"eventListeners": -16468,
"totalBlockingTimeMs": 0,
"frameDurationMs": 17.226666666666762,
"p95FrameDurationMs": 16.80000000000291
},
{
"name": "vue-large-graph-pan",
"durationMs": 13762.086999999952,
"styleRecalcs": 67,
"styleRecalcDurationMs": 15.189000000000007,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 13742.988000000001,
"heapDeltaBytes": -39834504,
"heapUsedBytes": 165797828,
"domNodes": -8331,
"jsHeapTotalBytes": -3608576,
"scriptDurationMs": 847.4869999999999,
"eventListeners": -16460,
"totalBlockingTimeMs": 82,
"frameDurationMs": 17.779999999999927,
"p95FrameDurationMs": 16.799999999999272
},
{
"name": "vue-large-graph-pan",
"durationMs": 14006.762999999977,
"styleRecalcs": 66,
"styleRecalcDurationMs": 15.425000000000022,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 13978.495000000003,
"heapDeltaBytes": -35141516,
"heapUsedBytes": 166061076,
"domNodes": -8331,
"jsHeapTotalBytes": -5595136,
"scriptDurationMs": 857.966,
"eventListeners": -16460,
"totalBlockingTimeMs": 0,
"frameDurationMs": 17.776666666666642,
"p95FrameDurationMs": 16.80000000000291
},
{
"name": "workflow-execution",
"durationMs": 466.85999999999694,
"styleRecalcs": 25,
"styleRecalcDurationMs": 23.801,
"layouts": 5,
"layoutDurationMs": 1.5009999999999997,
"taskDurationMs": 133.42900000000003,
"heapDeltaBytes": -16889684,
"heapUsedBytes": 48279156,
"domNodes": 195,
"jsHeapTotalBytes": 5505024,
"scriptDurationMs": 26.497999999999994,
"eventListeners": 71,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333332,
"p95FrameDurationMs": 16.700000000000273
},
{
"name": "workflow-execution",
"durationMs": 118.18500000003951,
"styleRecalcs": 9,
"styleRecalcDurationMs": 15.539000000000001,
"layouts": 6,
"layoutDurationMs": 1.5769999999999997,
"taskDurationMs": 82.98,
"heapDeltaBytes": 3078880,
"heapUsedBytes": 53300268,
"domNodes": 143,
"jsHeapTotalBytes": 262144,
"scriptDurationMs": 17.703,
"eventListeners": 19,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.799999999999727
}
]
} |
![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: 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 `@apps/website/src/utils/cloudNodes.ts`:
- Around line 373-375: When merging slug-collided snapshot packs (inside the
block that checks `if (existing)`), you currently only append `nodes` and drop
any optional metadata from later `pack`s; change it to preserve existing fields
but fill missing/undefined metadata from `pack`. Specifically, after updating
`existing.nodes = [...existing.nodes, ...pack.nodes]`, copy any other fields
from `pack` into `existing` only when `existing[field]` is null/undefined (do
not overwrite existing values and do not touch `nodes`), e.g., loop over
`Object.keys(pack)` and assign missing properties; this ensures optional fields
from later aliases are retained when the first pack lacked them.
🪄 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: CHILL
Plan: Pro
Run ID: 71ab6d22-50c8-4c25-b05f-aa54381b2c64
📒 Files selected for processing (12)
.github/workflows/ci-website-e2e.yamlapps/website/e2e/fixtures/cloud-nodes.fixture.jsonapps/website/src/data/cloud-nodes.snapshot.jsonapps/website/src/utils/cloudNodes.build.test.tsapps/website/src/utils/cloudNodes.build.tsapps/website/src/utils/cloudNodes.test.tsapps/website/src/utils/cloudNodes.tspackages/object-info-parser/src/__tests__/groupNodesByPack.test.tspackages/object-info-parser/src/__tests__/slugifyPackId.test.tspackages/object-info-parser/src/helpers/groupNodesByPack.tspackages/object-info-parser/src/helpers/slugifyPackId.tspackages/object-info-parser/src/index.ts
… slug Addresses CodeRabbit review: when normalizeSnapshotIds merged two snapshot packs whose ids slugified to the same value, only the nodes were combined and every other optional field (registryId, description, repoUrl, publisher, downloads, githubStars, license, ...) was silently dropped from the later alias. If the first row lacked metadata the second had, those fields were lost from the rendered detail page. mergeCollidedPacks now walks every key on the later pack and fills any undefined or null fields on the merged result, never overwriting metadata already present on the first row. The merge stays deterministic (first-wins for filled fields) and the rule covers the full Pack shape instead of an ad-hoc hand-listed subset.
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/website/src/utils/cloudNodes.ts (1)
251-257:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPreserve a raw
registryIdwhen registry enrichment misses.
registryIdnow comes only fromregistryPack?.id, so any pack whose registry lookup returnsnull/empty—or any full registry fetch failure—produces a fresh snapshot with no raw upstream id at all. That breaks the new "sluggedid+ rawregistryId" contract and leaves downstream registry lookups without a stable identifier. Please fall back to the pack's canonical raw alias here, e.g.registryPack?.id ?? pack.rawIds[0], and lock it in with a regression test for the no-hit path.Suggested fix
const packs = grouped.map((pack) => toDomainPack( pack.id, + pack.rawIds[0], pack.displayName, pack.nodes, pickRegistryPack(registryMap, pack.rawIds) ) ) @@ function toDomainPack( packId: string, + fallbackRegistryId: string | undefined, fallbackDisplayName: string, nodes: Array<{ @@ return { id: packId, - registryId: registryPack?.id, + registryId: registryPack?.id ?? fallbackRegistryId, displayName: registryPack?.name?.trim() || fallbackDisplayName || packId,Also applies to: 286-304
🤖 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 `@apps/website/src/utils/cloudNodes.ts` around lines 251 - 257, The pack creation currently uses only registryPack?.id so when pickRegistryPack(registryMap, pack.rawIds) returns null you lose the original raw registry id; change the code that maps grouped packs (the toDomainPack call) to derive a stable registryId variable like const registryPack = pickRegistryPack(registryMap, pack.rawIds); const registryId = registryPack?.id ?? pack.rawIds[0]; and pass registryId into toDomainPack (same change for the other occurrence around the 286-304 block), and add a regression test that simulates a registry miss (pickRegistryPack returning null) asserting the resulting domain pack contains the fallback raw registryId.
🤖 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.
Outside diff comments:
In `@apps/website/src/utils/cloudNodes.ts`:
- Around line 251-257: The pack creation currently uses only registryPack?.id so
when pickRegistryPack(registryMap, pack.rawIds) returns null you lose the
original raw registry id; change the code that maps grouped packs (the
toDomainPack call) to derive a stable registryId variable like const
registryPack = pickRegistryPack(registryMap, pack.rawIds); const registryId =
registryPack?.id ?? pack.rawIds[0]; and pass registryId into toDomainPack (same
change for the other occurrence around the 286-304 block), and add a regression
test that simulates a registry miss (pickRegistryPack returning null) asserting
the resulting domain pack contains the fallback raw registryId.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d3bbc268-8e3e-411d-b77b-803740f190a0
📒 Files selected for processing (2)
apps/website/src/utils/cloudNodes.test.tsapps/website/src/utils/cloudNodes.ts
…isses Addresses CodeRabbit review: registryId was sourced purely from registryPack?.id, so any pack whose registry lookup returned null (or whose entire fetchRegistryPacks call threw) ended up with registryId undefined. That breaks the new 'slugged id + raw registryId' contract and leaves downstream code with no stable upstream identifier. toDomainPack now takes a fallbackRegistryId derived from pack.rawIds[0] and uses it when registryPack is null/undefined, so every Pack always exposes the canonical raw alias even when the registry batch fails. Two regression tests pin the contract: * registry-miss path (fetchRegistryPacks returns empty map) * registry-throw path (fetchRegistryPacks rejects)
|
Addressed the outside-diff finding on
86/86 unit tests pass, 18/18 cloud-nodes e2e pass, |
Contributor
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 `@apps/website/src/utils/cloudNodes.test.ts`:
- Around line 440-586: The fallback-snapshot tests can be affected by
WEBSITE_CLOUD_NODES_FIXTURE; before running the suite that uses withSnapshotDir
and fetchCloudNodesForBuild, save process.env.WEBSITE_CLOUD_NODES_FIXTURE to a
local variable and delete or set it to '' so snapshot reads come from the
provided snapshotUrl, and after the suite restore the original value; add a
beforeAll/afterAll (or beforeEach/afterEach) block in
apps/website/src/utils/cloudNodes.test.ts that captures const originalFixture =
process.env.WEBSITE_CLOUD_NODES_FIXTURE, deletes
process.env.WEBSITE_CLOUD_NODES_FIXTURE (or sets to undefined) in setup, and
resets process.env.WEBSITE_CLOUD_NODES_FIXTURE = originalFixture in teardown so
tests using withSnapshotDir and fetchCloudNodesForBuild are hermetic.
🪄 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: CHILL
Plan: Pro
Run ID: 92461a64-3d38-440b-ae53-527dba551881
📒 Files selected for processing (2)
apps/website/src/utils/cloudNodes.test.tsapps/website/src/utils/cloudNodes.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/website/src/utils/cloudNodes.ts
Defensive hardening per CodeRabbit feedback. WEBSITE_CLOUD_NODES_FIXTURE is currently read only by loadPacksForBuild (cloudNodes.build.ts), not by fetchCloudNodesForBuild, so the tests in cloudNodes.test.ts are not affected today. But clearing the env var in beforeEach and restoring it in afterEach guards against future refactors that might let the override bleed into the fetcher, and matches the pattern already used in cloudNodes.build.test.ts. Also fixes a latent bug in the existing afterEach: previously, process.env.WEBSITE_CLOUD_API_KEY = savedCloudApiKey would set the env var to the literal string 'undefined' when savedCloudApiKey was unset. Now both env vars are conditionally restored (matching the cloudNodes.build test convention).
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/website/src/utils/cloudNodes.test.ts (1)
404-449: ⚡ Quick win“picks the first hit” isn’t fully proven when both aliases resolve.
This case only verifies
null -> hit, so it can’t catch a regression that picks the last non-null entry. Add one assertion path where both aliases return metadata and confirm the first alias wins.Suggested test addition
+ it('prefers the first non-null registry result when multiple aliases resolve', async () => { + fetchRegistryPacksMock.mockResolvedValue( + new Map<string, unknown>([ + [ + 'ComfyUI-QwenVL', + { + id: 'ComfyUI-QwenVL', + name: 'Dash Variant', + repository: 'https://github.com/example/dash-first' + } + ], + [ + 'ComfyUI_QwenVL', + { + id: 'ComfyUI_QwenVL', + name: 'Underscore Variant', + repository: 'https://github.com/example/underscore-second' + } + ] + ]) + ) + + const fetchImpl = vi.fn(async () => + response({ + QwenDash: validNode({ + name: 'QwenDash', + python_module: 'custom_nodes.ComfyUI-QwenVL.nodes' + }), + QwenUnder: validNode({ + name: 'QwenUnder', + python_module: 'custom_nodes.ComfyUI_QwenVL.nodes' + }) + }) + ) + + const outcome = await fetchCloudNodesForBuild({ + apiKey: KEY, + baseUrl: BASE_URL, + fetchImpl: fetchImpl as typeof fetch + }) + + expect(outcome.status).toBe('fresh') + if (outcome.status !== 'fresh') return + expect(outcome.snapshot.packs[0]?.registryId).toBe('ComfyUI-QwenVL') + expect(outcome.snapshot.packs[0]?.repoUrl).toBe( + 'https://github.com/example/dash-first' + ) + })🤖 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 `@apps/website/src/utils/cloudNodes.test.ts` around lines 404 - 449, The test currently only covers a null->hit alias resolution so it doesn't prove the code prefers the first alias when both aliases return metadata; update or add a test case in the same it block (or a new it) that mocks fetchRegistryPacksMock to return non-null metadata for both 'ComfyUI-QwenVL' and 'ComfyUI_QwenVL', call fetchCloudNodesForBuild with the same fetchImpl nodes, and assert that outcome.status === 'fresh' and outcome.snapshot.packs[0].registryId equals the first alias ('ComfyUI-QwenVL' or whichever alias appears first in the array), and that repoUrl/ id correspond to that first-alias metadata; also keep the existing expectation that fetchRegistryPacksMock was called with both aliases to ensure both were queried.
🤖 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 `@apps/website/src/utils/cloudNodes.test.ts`:
- Around line 404-449: The test currently only covers a null->hit alias
resolution so it doesn't prove the code prefers the first alias when both
aliases return metadata; update or add a test case in the same it block (or a
new it) that mocks fetchRegistryPacksMock to return non-null metadata for both
'ComfyUI-QwenVL' and 'ComfyUI_QwenVL', call fetchCloudNodesForBuild with the
same fetchImpl nodes, and assert that outcome.status === 'fresh' and
outcome.snapshot.packs[0].registryId equals the first alias ('ComfyUI-QwenVL' or
whichever alias appears first in the array), and that repoUrl/ id correspond to
that first-alias metadata; also keep the existing expectation that
fetchRegistryPacksMock was called with both aliases to ensure both were queried.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 06d5fa57-3068-46b7-b448-f298cce1ce31
📒 Files selected for processing (1)
apps/website/src/utils/cloudNodes.test.ts
…s resolve CodeRabbit nitpick: the existing alias-collision test mocked ComfyUI-QwenVL -> null + ComfyUI_QwenVL -> hit, so the assertion that the first hit wins was actually proving 'the only non-null hit wins'. A regression that returns the last non-null entry would still pass. New test mocks BOTH aliases to non-null metadata with distinct registryId/repoUrl, and pins registryId === 'ComfyUI-QwenVL' (the first alias in pack.rawIds) so a 'last wins' or 'arbitrary wins' regression fails immediately.
|
Good catch — added in New test mocks both aliases to non-null metadata with distinct expect(outcome.snapshot.packs[0]?.registryId).toBe('ComfyUI-QwenVL')
expect(outcome.snapshot.packs[0]?.repoUrl).toBe('https://github.com/example/dash-first')Locks |
Codecov Report❌ Patch coverage is
@@ Coverage Diff @@
## main #12277 +/- ##
===========================================
- Coverage 74.49% 59.57% -14.92%
===========================================
Files 1523 1412 -111
Lines 92068 71884 -20184
Branches 25502 19025 -6477
===========================================
- Hits 68587 42826 -25761
- Misses 22629 28585 +5956
+ Partials 852 473 -379
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 1016 files with indirect coverage changes 🚀 New features to boost your workflow:
|
Contributor
|
Note Docstrings generation - SUCCESS |
coderabbitai Bot
added a commit
that referenced
this pull request
May 15, 2026
Docstrings generation was requested by @DrJKL. * #12277 (comment) The following files were modified: * `apps/website/src/utils/cloudNodes.build.ts` * `apps/website/src/utils/cloudNodes.ts` * `packages/object-info-parser/src/helpers/groupNodesByPack.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.
PR Created by the Glary-Bot Agent
Summary
/cloud/supported-nodesroutes use pack ids as URL segments, but those ids come straight from upstream Python module names which mix PascalCase, snake_case, and kebab-case freely. The most recentRelease: Websitesnapshot refresh (#12271) surfaced packs likeComfyUI-Crystoolsandbasic_data_handling, and the Playwright assertions incloud-nodes.spec.tsbroke on three independent fronts:[a-z0-9-]+$rejectsbasic_data_handling(underscores) andComfyQR(uppercase)comfyui-impact-packassertions assumed the refreshed snapshot would still contain that pack — it didn'tFaceDetailersearch assumed Impact Pack nodes would always be presentThis PR addresses both the root cause and the test fragility in one change.
Changes
Slugify pack ids at the boundaries —
slugifyPackIdin@comfyorg/object-info-parsernormalizes every pack id (lowercase +_→-) at the two points where ids enter the website:groupNodesByPackfor fresh fetches from the cloud APIreadSnapshotfor the bundled / fallback snapshotThe raw upstream id is preserved as
NodePack.rawIds[]and forwarded tofetchRegistryPacks, so registry enrichment continues to hitapi.comfy.org/nodeswith the exactnode_idthe API expects (uppercase, underscores, all). 24 underscore-vs-hyphen pairs exist in the live registry today (ComfyUI-QwenVL+ComfyUI_QwenVL, etc.); collisions are merged deterministically into a single static route. When two raw ids collide on the same slug, every alias is queried in one registry batch and the first non-null hit wins, so a merged pack never loses metadata that only one alias would have resolved.Mock the snapshot in CI —
WEBSITE_CLOUD_NODES_FIXTURElets the e2e build read a committed fixture in place of the bundled snapshot. CI sets it toe2e/fixtures/cloud-nodes.fixture.json(a deterministic 5-pack fixture designed to satisfy every assertion in the spec). The override only fires when the env var is set; production builds continue to use the live snapshot and thestale-in-productionguard.Verification
cloud-nodestests included)slugifyPackIdcases, +5 newgroupNodesByPackcases)astro check: 0 errorsastro buildwith the fixture env var produces all expected routes (/cloud/supported-nodes/comfyui-impact-pack,/cloud/supported-nodes/comfyui-crystools,/cloud/supported-nodes/alpha-test-pack,/cloud/supported-nodes/rgthree-comfy,/cloud/supported-nodes/was-node-suite-comfyui, plus zh-CN twins)appModeStorefrom fix: preserve app builder inputs through graph reconfiguration #11422, out of scope), 1 addressed in this PR (alias loss on slug collision, fixed byrawIds[]+ multi-alias query inpickRegistryPack)Notes for reviewers
The committed
cloud-nodes.snapshot.jsonis rewritten so the on-disk ids match what the slugifier produces (ComfyUI-Crystools→comfyui-crystools,comfyui_ipadapter_plus→comfyui-ipadapter-plus). This breaks any external link to…/cloud/supported-nodes/ComfyUI-Crystoolsor…/cloud/supported-nodes/comfyui_ipadapter_plus— those URLs were never the documented form, but worth a quick gut-check before merge.PR chore(website): refresh Ashby and cloud nodes snapshots #12271 (the failing auto-refresh) does not need any manual fix once this lands: its broken snapshot data will be regenerated by the next
Release: Websiterun and routed correctly throughslugifyPackId.The
Packshape on disk is unchanged.registryIdcontinues to hold the raw upstream id; onlyidis now guaranteed to be a clean slug.Fixes the website-e2e check failure on chore(website): refresh Ashby and cloud nodes snapshots #12271
┆Issue is synchronized with this Notion page by Unito