test: stabilize cloud template filtering e2e#12999
Merged
Merged
Conversation
Contributor
📝 WalkthroughWalkthrough
ChangesTemplate Mock Route and Fixture Update
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 7✅ Passed checks (7 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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: ✅ 1670 passed, 0 failed · 2 flaky📊 Browser Reports
📦 Bundle: 7.45 MB gzip 🟢 -5 BDetailsSummary
Category Glance App Entry Points — 46.7 kB (baseline 46.7 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 — 95.3 kB (baseline 95.3 kB) • ⚪ 0 BTop-level views, pages, and routed surfaces
Status: 9 added / 9 removed / 3 unchanged Panels & Settings — 525 kB (baseline 525 kB) • ⚪ 0 BConfiguration panels, inspectors, and settings screens
Status: 11 added / 11 removed / 15 unchanged User & Accounts — 19.9 kB (baseline 19.9 kB) • ⚪ 0 BAuthentication, profile, and account management bundles
Status: 6 added / 6 removed / 3 unchanged Editors & Dialogs — 112 kB (baseline 112 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 — 268 kB (baseline 268 kB) • ⚪ 0 BStores, services, APIs, and repositories
Status: 13 added / 13 removed / 3 unchanged Utilities & Hooks — 3.32 MB (baseline 3.32 MB) • ⚪ 0 BHelpers, composables, and utility bundles
Status: 14 added / 14 removed / 16 unchanged Vendor & Third-Party — 15.3 MB (baseline 15.3 MB) • ⚪ 0 BExternal libraries and shared vendor chunks Status: 16 unchanged Other — 10.4 MB (baseline 10.4 MB) • ⚪ 0 BBundles that do not match a named category
Status: 62 added / 62 removed / 89 unchanged ⚡ Performance Report
Show regressions
All metrics
Historical variance (last 15 runs)
Trend (last 15 commits on main)
Raw data{
"timestamp": "2026-06-19T15:32:30.780Z",
"gitSha": "5c8b0e06d60ac94684661592c01c77e92251fe10",
"branch": "jaeone/fix-cloud-template-assets-e2e",
"measurements": [
{
"name": "canvas-idle",
"durationMs": 2032.3700000000144,
"styleRecalcs": 10,
"styleRecalcDurationMs": 10.241999999999999,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 497.925,
"heapDeltaBytes": -2079108,
"heapUsedBytes": 56472056,
"domNodes": 20,
"jsHeapTotalBytes": 24641536,
"scriptDurationMs": 29.918,
"eventListeners": 6,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333332,
"p95FrameDurationMs": 16.799999999999272
},
{
"name": "canvas-idle",
"durationMs": 2013.4259999999813,
"styleRecalcs": 9,
"styleRecalcDurationMs": 9.661,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 468.65900000000005,
"heapDeltaBytes": -2406824,
"heapUsedBytes": 56285692,
"domNodes": 18,
"jsHeapTotalBytes": 26214400,
"scriptDurationMs": 27.413999999999998,
"eventListeners": 4,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "canvas-mouse-sweep",
"durationMs": 1940.6490000000076,
"styleRecalcs": 78,
"styleRecalcDurationMs": 46.117,
"layouts": 12,
"layoutDurationMs": 3.633,
"taskDurationMs": 933.474,
"heapDeltaBytes": -7371176,
"heapUsedBytes": 51376808,
"domNodes": 61,
"jsHeapTotalBytes": 26214400,
"scriptDurationMs": 149.54000000000002,
"eventListeners": 6,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "canvas-mouse-sweep",
"durationMs": 1839.3429999999853,
"styleRecalcs": 76,
"styleRecalcDurationMs": 42.198,
"layouts": 12,
"layoutDurationMs": 3.6359999999999997,
"taskDurationMs": 838.844,
"heapDeltaBytes": -7058348,
"heapUsedBytes": 51276276,
"domNodes": 59,
"jsHeapTotalBytes": 26214400,
"scriptDurationMs": 138.393,
"eventListeners": 4,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "canvas-zoom-sweep",
"durationMs": 1726.1780000000044,
"styleRecalcs": 32,
"styleRecalcDurationMs": 19.505000000000003,
"layouts": 6,
"layoutDurationMs": 0.806,
"taskDurationMs": 384.407,
"heapDeltaBytes": 1479704,
"heapUsedBytes": 60195600,
"domNodes": 76,
"jsHeapTotalBytes": 25690112,
"scriptDurationMs": 28.456,
"eventListeners": 19,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "canvas-zoom-sweep",
"durationMs": 1748.6410000000205,
"styleRecalcs": 31,
"styleRecalcDurationMs": 20.255000000000003,
"layouts": 6,
"layoutDurationMs": 0.755,
"taskDurationMs": 388.967,
"heapDeltaBytes": 1724648,
"heapUsedBytes": 60301852,
"domNodes": 78,
"jsHeapTotalBytes": 26738688,
"scriptDurationMs": 31.552999999999997,
"eventListeners": 19,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "dom-widget-clipping",
"durationMs": 587.9560000000197,
"styleRecalcs": 13,
"styleRecalcDurationMs": 10.095,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 382.315,
"heapDeltaBytes": 7611316,
"heapUsedBytes": 66126260,
"domNodes": 22,
"jsHeapTotalBytes": 17301504,
"scriptDurationMs": 65.026,
"eventListeners": 2,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333332,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "dom-widget-clipping",
"durationMs": 606.7669999999907,
"styleRecalcs": 12,
"styleRecalcDurationMs": 9.964999999999998,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 397.093,
"heapDeltaBytes": -14166424,
"heapUsedBytes": 55469956,
"domNodes": 20,
"jsHeapTotalBytes": 23379968,
"scriptDurationMs": 68.134,
"eventListeners": 2,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "large-graph-idle",
"durationMs": 2035.2080000000115,
"styleRecalcs": 10,
"styleRecalcDurationMs": 10.736000000000002,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 696.329,
"heapDeltaBytes": -7598136,
"heapUsedBytes": 62657812,
"domNodes": 20,
"jsHeapTotalBytes": 10891264,
"scriptDurationMs": 128.577,
"eventListeners": 6,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333332,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "large-graph-idle",
"durationMs": 2018.4619999999995,
"styleRecalcs": 10,
"styleRecalcDurationMs": 9.802999999999999,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 653.559,
"heapDeltaBytes": -9016896,
"heapUsedBytes": 62662256,
"domNodes": 20,
"jsHeapTotalBytes": 9580544,
"scriptDurationMs": 113.588,
"eventListeners": 6,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "large-graph-pan",
"durationMs": 2217.1389999999747,
"styleRecalcs": 69,
"styleRecalcDurationMs": 19.738000000000003,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 1301.5159999999998,
"heapDeltaBytes": 9247848,
"heapUsedBytes": 72080920,
"domNodes": -267,
"jsHeapTotalBytes": 6246400,
"scriptDurationMs": 476.342,
"eventListeners": -161,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "large-graph-pan",
"durationMs": 2126.871999999935,
"styleRecalcs": 68,
"styleRecalcDurationMs": 19.432000000000002,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 1221.2969999999998,
"heapDeltaBytes": 10318560,
"heapUsedBytes": 83408868,
"domNodes": 14,
"jsHeapTotalBytes": 9232384,
"scriptDurationMs": 424.736,
"eventListeners": 4,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "large-graph-zoom",
"durationMs": 3198.3109999999897,
"styleRecalcs": 64,
"styleRecalcDurationMs": 19.479,
"layouts": 60,
"layoutDurationMs": 8.331000000000001,
"taskDurationMs": 1471.4910000000002,
"heapDeltaBytes": 15139728,
"heapUsedBytes": 70640884,
"domNodes": 10,
"jsHeapTotalBytes": 5505024,
"scriptDurationMs": 537.63,
"eventListeners": 8,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333332,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "large-graph-zoom",
"durationMs": 3279.390000000035,
"styleRecalcs": 64,
"styleRecalcDurationMs": 20.465,
"layouts": 60,
"layoutDurationMs": 8.572,
"taskDurationMs": 1551.142,
"heapDeltaBytes": 17117608,
"heapUsedBytes": 72779580,
"domNodes": 10,
"jsHeapTotalBytes": 7864320,
"scriptDurationMs": 594.248,
"eventListeners": 8,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "minimap-idle",
"durationMs": 2021.19399999998,
"styleRecalcs": 9,
"styleRecalcDurationMs": 10.276,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 674.6220000000001,
"heapDeltaBytes": 12049040,
"heapUsedBytes": 74111404,
"domNodes": 18,
"jsHeapTotalBytes": 13950976,
"scriptDurationMs": 120.63399999999999,
"eventListeners": 6,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.799999999999272
},
{
"name": "minimap-idle",
"durationMs": 2004.9870000000283,
"styleRecalcs": 9,
"styleRecalcDurationMs": 8.809000000000001,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 634.8130000000001,
"heapDeltaBytes": -9544692,
"heapUsedBytes": 63966604,
"domNodes": 18,
"jsHeapTotalBytes": 7483392,
"scriptDurationMs": 115.427,
"eventListeners": 6,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "subgraph-dom-widget-clipping",
"durationMs": 617.3320000000331,
"styleRecalcs": 48,
"styleRecalcDurationMs": 14.351,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 416.447,
"heapDeltaBytes": 8037408,
"heapUsedBytes": 66823840,
"domNodes": 22,
"jsHeapTotalBytes": 17825792,
"scriptDurationMs": 130.816,
"eventListeners": 8,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.799999999999272
},
{
"name": "subgraph-dom-widget-clipping",
"durationMs": 583.4290000000237,
"styleRecalcs": 47,
"styleRecalcDurationMs": 11.709000000000003,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 388.377,
"heapDeltaBytes": 8002056,
"heapUsedBytes": 66823188,
"domNodes": 20,
"jsHeapTotalBytes": 18874368,
"scriptDurationMs": 121.74100000000001,
"eventListeners": 8,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "subgraph-idle",
"durationMs": 1990.6070000000113,
"styleRecalcs": 11,
"styleRecalcDurationMs": 10.46,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 466.19699999999995,
"heapDeltaBytes": -2680484,
"heapUsedBytes": 56077720,
"domNodes": 22,
"jsHeapTotalBytes": 25165824,
"scriptDurationMs": 24.142,
"eventListeners": 4,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333335,
"p95FrameDurationMs": 16.699999999999818
},
{
"name": "subgraph-idle",
"durationMs": 1990.4109999999946,
"styleRecalcs": 10,
"styleRecalcDurationMs": 9.791,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 451.658,
"heapDeltaBytes": -2246904,
"heapUsedBytes": 56542052,
"domNodes": 20,
"jsHeapTotalBytes": 26214400,
"scriptDurationMs": 24.499999999999996,
"eventListeners": 4,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "subgraph-mouse-sweep",
"durationMs": 1706.0529999999972,
"styleRecalcs": 76,
"styleRecalcDurationMs": 37.95399999999999,
"layouts": 16,
"layoutDurationMs": 4.335,
"taskDurationMs": 744.2869999999999,
"heapDeltaBytes": -10706056,
"heapUsedBytes": 47961368,
"domNodes": 65,
"jsHeapTotalBytes": 26214400,
"scriptDurationMs": 103.623,
"eventListeners": 4,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.699999999999818
},
{
"name": "subgraph-mouse-sweep",
"durationMs": 1690.5380000000605,
"styleRecalcs": 75,
"styleRecalcDurationMs": 39.283,
"layouts": 16,
"layoutDurationMs": 4.399,
"taskDurationMs": 758.1610000000001,
"heapDeltaBytes": -10923136,
"heapUsedBytes": 47851928,
"domNodes": 61,
"jsHeapTotalBytes": 24903680,
"scriptDurationMs": 102.505,
"eventListeners": 4,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "subgraph-transition-enter",
"durationMs": 1400.6299999999783,
"styleRecalcs": 15,
"styleRecalcDurationMs": 29.327000000000005,
"layouts": 4,
"layoutDurationMs": 15.045,
"taskDurationMs": 874.273,
"heapDeltaBytes": 4581560,
"heapUsedBytes": 80088520,
"domNodes": 13833,
"jsHeapTotalBytes": 17039360,
"scriptDurationMs": 43.646,
"eventListeners": 2529,
"totalBlockingTimeMs": 176,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "viewport-pan-sweep",
"durationMs": 8262.75499999997,
"styleRecalcs": 251,
"styleRecalcDurationMs": 60.855000000000004,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 4317.233,
"heapDeltaBytes": -2671332,
"heapUsedBytes": 67485584,
"domNodes": 20,
"jsHeapTotalBytes": 19456000,
"scriptDurationMs": 1373.665,
"eventListeners": 20,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "viewport-pan-sweep",
"durationMs": 8338.943000000087,
"styleRecalcs": 251,
"styleRecalcDurationMs": 67.023,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 4694.521,
"heapDeltaBytes": 7001172,
"heapUsedBytes": 78916968,
"domNodes": 18,
"jsHeapTotalBytes": 23650304,
"scriptDurationMs": 1680.367,
"eventListeners": 22,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333332,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "vue-large-graph-idle",
"durationMs": 13725.139000000014,
"styleRecalcs": 1,
"styleRecalcDurationMs": 1.5899999999999803,
"layouts": 1,
"layoutDurationMs": 3.392999999999993,
"taskDurationMs": 13696.340000000002,
"heapDeltaBytes": -37232852,
"heapUsedBytes": 169761320,
"domNodes": -3307,
"jsHeapTotalBytes": 26447872,
"scriptDurationMs": 687.2640000000001,
"eventListeners": -16467,
"totalBlockingTimeMs": 974,
"frameDurationMs": 17.776666666666763,
"p95FrameDurationMs": 16.799999999999272
},
{
"name": "vue-large-graph-idle",
"durationMs": 13212.455999999975,
"styleRecalcs": 0,
"styleRecalcDurationMs": 0,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 13192.634999999998,
"heapDeltaBytes": -42214548,
"heapUsedBytes": 166234724,
"domNodes": -3304,
"jsHeapTotalBytes": 20418560,
"scriptDurationMs": 663.24,
"eventListeners": -16473,
"totalBlockingTimeMs": 0,
"frameDurationMs": 17.219999999999953,
"p95FrameDurationMs": 16.80000000000291
},
{
"name": "vue-large-graph-pan",
"durationMs": 15512.685000000034,
"styleRecalcs": 77,
"styleRecalcDurationMs": 20.506999999999998,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 15484.577000000001,
"heapDeltaBytes": -36941812,
"heapUsedBytes": 171695152,
"domNodes": -3308,
"jsHeapTotalBytes": 17711104,
"scriptDurationMs": 908.859,
"eventListeners": -16467,
"totalBlockingTimeMs": 5,
"frameDurationMs": 17.776666666666642,
"p95FrameDurationMs": 16.799999999999272
},
{
"name": "vue-large-graph-pan",
"durationMs": 15597.392000000014,
"styleRecalcs": 80,
"styleRecalcDurationMs": 21.10500000000004,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 15570.088000000002,
"heapDeltaBytes": -15366748,
"heapUsedBytes": 178970168,
"domNodes": -3303,
"jsHeapTotalBytes": 17711104,
"scriptDurationMs": 931.357,
"eventListeners": -16470,
"totalBlockingTimeMs": 0,
"frameDurationMs": 17.223333333333237,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "workflow-execution",
"durationMs": 469.0409999999474,
"styleRecalcs": 19,
"styleRecalcDurationMs": 28.171,
"layouts": 6,
"layoutDurationMs": 1.8110000000000002,
"taskDurationMs": 142.75499999999997,
"heapDeltaBytes": 5539712,
"heapUsedBytes": 65281476,
"domNodes": 168,
"jsHeapTotalBytes": 3145728,
"scriptDurationMs": 25.253999999999998,
"eventListeners": 69,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.663333333333338,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "workflow-execution",
"durationMs": 145.13800000008814,
"styleRecalcs": 11,
"styleRecalcDurationMs": 21.714000000000006,
"layouts": 5,
"layoutDurationMs": 1.817,
"taskDurationMs": 107.61900000000001,
"heapDeltaBytes": 3418316,
"heapUsedBytes": 63335304,
"domNodes": 146,
"jsHeapTotalBytes": 1572864,
"scriptDurationMs": 19.504,
"eventListeners": 37,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.663333333333338,
"p95FrameDurationMs": 16.700000000000273
}
]
} |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Codecov Report✅ All modified and coverable lines are covered by tests. @@ Coverage Diff @@
## main #12999 +/- ##
==========================================
- Coverage 76.35% 76.13% -0.22%
==========================================
Files 1573 1574 +1
Lines 88149 95451 +7302
Branches 26621 28756 +2135
==========================================
+ Hits 67305 72672 +5367
- Misses 20190 21991 +1801
- Partials 654 788 +134
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
dosubot
Bot
added
the
size:S
DrJKL
approved these changes
Jun 19, 2026
DrJKL
left a comment
DrJKL
left a comment
Contributor
There was a problem hiding this comment.
Approving. Validated the route interception against the production path: workflowTemplatesStore.loadWorkflowTemplates() → api.getWorkflowTemplates() → fetchApi('/workflow_templates') → apiURL() prepends /api, so the request resolves to <api_base>/api/workflow_templates, which the glob matches on both cloud and local. The empty {} body matches the declared { [name: string]: string[] } shape, and the mock is necessary — switching to createCloudAssetsFixture([]) runs these as @cloud, so without it the real custom templates would leak into the exact-count assertions. Cleanup is correct (handler is pushed to routeHandlers and unrouted in clearMocks).
All inline notes below are non-blocking polish.
| } | ||
|
|
||
| async mockIndex(): Promise<void> { | ||
| const customTemplatesHandler = async (route: Route) => { |
Contributor
There was a problem hiding this comment.
Route interception is correct and necessary here — confirmed it matches <api_base>/api/workflow_templates and that the empty {} keeps the asserted template counts deterministic.
Nit (optional): this custom-templates mock lives inside mockIndex(), which otherwise only concerns the core index. It works because every test calls mockIndex(), but a dedicated mockCustomTemplates() invoked from mock() would read more truthfully.
Collaborator
Author
There was a problem hiding this comment.
Agreed, this is a valid readability cleanup. I kept the custom templates mock colocated with the existing template index mocking in this PR because the change was intentionally scoped to stabilizing the cloud template count E2E, and I wanted to avoid widening it into fixture refactoring while we continue the missing model detection work before .46 becomes stable.
This would be better handled as follow-up cleanup rather than mixed into this stabilization PR.
@coderabbitai please create an issue for this and assign it to me.
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
gh issue create \
--repo "Comfy-Org/ComfyUI_frontend" \
--title "Refactor TemplateHelper: extract mockCustomTemplates() from mockIndex()" \
--assignee "jaeone94" \
--body "## Summary
Currently, \`TemplateHelper.mockIndex()\` in \`browser_tests/fixtures/helpers/TemplateHelper.ts\` contains two responsibilities:
1. Mocking the core template index (\`**/templates/index.json\`)
2. Mocking the custom templates endpoint (\`**/api/workflow_templates\`)
For readability and separation of concerns, the custom-templates mock should be extracted into a dedicated \`mockCustomTemplates()\` method that is invoked from the top-level \`mock()\` method (alongside \`mockIndex()\` and \`mockThumbnails()\`).
## Proposed change
Extract the \`customTemplatesHandler\` block from \`mockIndex()\` into a new \`mockCustomTemplates(): Promise<void>\` method, and call it from \`mock()\`.
## Context
Identified during review of PR `#12999` (comment: https://github.com/Comfy-Org/ComfyUI_frontend/pull/12999#discussion_r3443589794).
The current colocated placement was intentional in that PR to keep the stabilization change scoped — this refactor is deferred as follow-up cleanup."Length of output: 1310
@jaeone94, the follow-up issue has been created and assigned to you: #13016 — Refactor TemplateHelper: extract mockCustomTemplates() from mockIndex(). It includes the full context from this discussion so nothing gets lost.
🧠 Learnings used
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7169
File: src/platform/remote/comfyui/jobs/jobTypes.ts:1-107
Timestamp: 2025-12-09T03:39:54.501Z
Learning: In the ComfyUI_frontend project, Zod is on v3.x. Do not suggest Zod v4 standalone validators (z.uuid, z.ulid, z.cuid2, z.nanoid) until an upgrade to Zod 4 is performed. When reviewing TypeScript files (e.g., src/platform/remote/comfyui/jobs/jobTypes.ts) validate against Zod 3 capabilities and avoid introducing v4-specific features; flag any proposal to upgrade or incorporate v4-only validators and propose staying with compatible 3.x patterns.
Learnt from: christian-byrne
Repo: Comfy-Org/ComfyUI_frontend PR: 7358
File: src/components/dialog/content/signin/SignUpForm.vue:45-54
Timestamp: 2025-12-11T12:25:15.470Z
Learning: This repository uses CI automation to format code (pnpm format). Do not include manual formatting suggestions in code reviews for Comfy-Org/ComfyUI_frontend. If formatting issues are detected, rely on the CI formatter or re-run pnpm format. Focus reviews on correctness, readability, performance, accessibility, and maintainability rather than style formatting.
Learnt from: christian-byrne
Repo: Comfy-Org/ComfyUI_frontend PR: 7416
File: src/stores/imagePreviewStore.ts:5-7
Timestamp: 2025-12-13T11:03:11.264Z
Learning: In the ComfyUI_frontend repository, lint rules require keeping 'import type' statements separate from non-type imports, even if importing from the same module. Do not suggest consolidating them into a single import statement. Ensure type imports remain on their own line (import type { ... } from 'module') and regular imports stay on separate lines.
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7537
File: src/components/ui/button/Button.stories.ts:45-55
Timestamp: 2025-12-17T00:40:09.635Z
Learning: Prefer pure function declarations over function expressions (e.g., use function foo() { ... } instead of const foo = () => { ... }) for pure functions in the repository. Function declarations are more functional-leaning, offer better hoisting clarity, and can improve readability and tooling consistency. Apply this guideline across TypeScript files in Comfy-Org/ComfyUI_frontend, including story and UI component code, except where a function expression is semantically required (e.g., callbacks, higher-order functions with closures).
Learnt from: kaili-yang
Repo: Comfy-Org/ComfyUI_frontend PR: 7805
File: src/composables/useCoreCommands.ts:439-439
Timestamp: 2025-12-30T22:22:33.836Z
Learning: When accessing reactive properties from Pinia stores in TypeScript files, avoid using .value on direct property access (e.g., useStore().isOverlayExpanded). Pinia auto-wraps refs when accessed directly, returning the primitive value. The .value accessor is only needed when destructuring store properties or when using storeToRefs().
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 8248
File: src/platform/assets/schemas/assetSchema.ts:109-118
Timestamp: 2026-01-22T23:44:59.888Z
Learning: Do not export declarations (e.g., schemas, constants, types) unless they are actually used elsewhere in the codebase. Keep the public API surface minimal by exporting only items with concrete usage. This reduces coupling and keeps imports explicit. In src/platform/assets/schemas/assetSchema.ts, ensure only necessary exports are exposed; remove unused exports or switch to non-exported/internal where appropriate.
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 8722
File: src/composables/maskeditor/useBrushDrawing.ts:5-5
Timestamp: 2026-02-08T03:19:12.009Z
Learning: Treat unresolved import warnings caused by pnpm workspace catalog mappings (pnpm-workspace.yaml) and unplugin-typegpu in vite.config.mts as false positives. Do not flag these in PR reviews; they are expected in sandboxes without node_modules and are not indicative of actual missing dependencies in the TS codebase.
Learnt from: christian-byrne
Repo: Comfy-Org/ComfyUI_frontend PR: 8966
File: src/extensions/core/load3d.ts:427-430
Timestamp: 2026-02-19T02:06:23.468Z
Learning: In TypeScript, you can use typeof SomeClass in type annotation positions (e.g., param: typeof LGraphNode) even when SomeClass is imported with import type. This is a type query that only exists at compile time and is erased in runtime, so it is safe to combine with type-only imports. Apply this pattern to TS files broadly when you need a type that references the shape of a class or constructor function without importing the value at runtime.
Learnt from: christian-byrne
Repo: Comfy-Org/ComfyUI_frontend PR: 8966
File: src/extensions/core/uploadAudio.ts:91-94
Timestamp: 2026-02-19T02:06:38.395Z
Learning: In TypeScript files, you can use a type annotation like 'nodeType: typeof MyClass' even if MyClass is imported via 'import type'. Both the type-only import and 'typeof' operate at the type level and are erased at compile time. This pattern is commonly used for constructor types (e.g., 'nodeType: typeof LGraphNode'). Apply this pattern across TypeScript files in the repository (src/**/*.ts) as appropriate, ensuring the imported symbol is a type-only import when possible for clarity and to avoid runtime imports.
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 8992
File: src/lib/litegraph/src/widgets/GradientSliderWidget.ts:18-18
Timestamp: 2026-02-20T21:08:19.814Z
Learning: When drawing with CanvasRenderingContext2D in TypeScript/JavaScript, wrap the drawing logic with ctx.save() at the start and ctx.restore() at the end to preserve and restore the canvas state. Do not manually destructure and restore individual properties (e.g., fillStyle, strokeStyle); rely on save/restore to manage state changes in a scoped manner. This should be applied to all TS files that perform canvas drawing.
Learnt from: dante01yoon
Repo: Comfy-Org/ComfyUI_frontend PR: 9075
File: src/scripts/api.featureFlags.test.ts:237-268
Timestamp: 2026-02-22T04:27:33.379Z
Learning: In Vite/Vitest, import.meta.env.DEV is true for any mode that is not 'production' (i.e., DEV is the opposite of PROD, and can be true in 'test', 'development', etc.). Do not assume DEV implies only 'development' mode. When reviewing code and tests, treat DEV as a non-production flag and verify environment-specific logic accordingly. Reference: https://vite.dev/guide/env-and-mode#modes
Learnt from: christian-byrne
Repo: Comfy-Org/ComfyUI_frontend PR: 9427
File: src/renderer/extensions/vueNodes/widgets/components/form/dropdown/FormDropdownMenuFilter.vue:33-33
Timestamp: 2026-03-06T00:53:28.835Z
Learning: When reviewing code, note that the enforce-canonical-classes (better-tailwindcss) rule may auto-fix Tailwind v3 !class-name syntax by converting it to v4 class-name! syntax. Do not treat these auto-fixed class-name! instances as newly introduced issues; the perceived change is in syntax placement, not in usage or intent. This guidance applies across all .vue and .ts files in the repository.
Learnt from: sonnybox
Repo: Comfy-Org/ComfyUI_frontend PR: 9446
File: src/renderer/extensions/vueNodes/widgets/components/WidgetTextarea.vue:45-45
Timestamp: 2026-03-06T01:55:00.013Z
Learning: Treat wrap-break-word as a valid Tailwind CSS utility for overflow-wrap: break-word in Tailwind v4+ projects. Do not flag this class as invalid in any Vue (.vue) or TypeScript (.ts/.tsx) files within the repository (e.g., Comfy-Org/ComfyUI_frontend) or other Tailwind v4+ projects. When reviewing, verify that the class is used to enable word breaking in long text content and reference the Tailwind docs: https://tailwindcss.com/docs/overflow-wrap.
Learnt from: christian-byrne
Repo: Comfy-Org/ComfyUI_frontend PR: 9554
File: browser_tests/fixtures/ComfyPage.ts:189-193
Timestamp: 2026-03-12T10:03:55.383Z
Learning: In browser_tests, seed localStorage-based feature flags at init time using FeatureFlagHelper.seedFlags() (which uses page.addInitScript) before any navigation. Ensure this happens before comfyPage.setup() is called. The comfyPageFixture currently yields after calling comfyPage.setup(), so do not defer setup() from the fixture or add a pre-setup hook, as changing this would break existing tests.
Learnt from: Yourz
Repo: Comfy-Org/ComfyUI_frontend PR: 10310
File: browser_tests/tests/topbarMenuDismissal.spec.ts:0-0
Timestamp: 2026-03-19T16:16:40.350Z
Learning: In Comfy-Org/ComfyUI_frontend, when reviewing changes limited to adding/revising `:modal="false"` (or similar modal/non-modal behavior props) on reakit-ui/reka-ui primitives such as `ContextMenuRoot` and `DropdownMenuRoot`, do not request Playwright/E2E regression coverage solely for that prop-level change. The dismissal behavior for modal vs non-modal is documented and tested upstream in the reka-ui library; only ask for E2E tests when the change affects ComfyUI_frontend behavior beyond these primitive prop wiring differences (e.g., custom logic, integration behavior, or new user flows not covered upstream).
Learnt from: jaeone94
Repo: Comfy-Org/ComfyUI_frontend PR: 10741
File: browser_tests/fixtures/helpers/boundsUtils.ts:32-45
Timestamp: 2026-03-31T12:17:31.232Z
Learning: In this codebase’s Playwright E2E test helpers under `browser_tests/fixtures/helpers/`, prefer measuring collapsed node dimensions directly from the DOM using `offsetWidth`/`offsetHeight`. Do not access Pinia stores from within `page.evaluate` blocks in E2E helpers (e.g., avoid calling `getNodeCollapsedSize()` or reading from `layoutStore`). This is intentional: E2E helpers should validate the final user-visible result, while Pinia store synchronization correctness is expected to be covered by unit tests.
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 10958
File: browser_tests/tests/bottomPanelLogs.spec.ts:4-4
Timestamp: 2026-04-08T18:09:35.787Z
Learning: In this repo’s E2E browser tests, do not flag `import-x/no-unresolved` for imports using the `e2e/*` path alias. CodeRabbit sandbox reports these as unresolved because it may not run a full `pnpm install`; `e2e/*` is defined in the root `tsconfig.json` and the related `apps/website/tsconfig.json`/workspace dependency resolution only works when dependencies are installed. Apply this only to `browser_tests/**/*.ts` files.
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 10958
File: browser_tests/tests/subgraph/subgraphSearch.spec.ts:3-4
Timestamp: 2026-04-08T18:09:50.771Z
Learning: In Comfy-Org/ComfyUI_frontend browser tests (browser_tests/**/*.ts), do not report `import-x/no-unresolved` for imports using the `e2e/*` alias. The alias is defined in the root tsconfig.json as `"e2e/*": ["./browser_tests/*"]`, and eslint.config.ts includes the correct tsconfig `project` list; lint passes locally and in CI. Treat any `e2e/*` unresolved-import findings in the CodeRabbit sandbox as false positives.
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 10958
File: browser_tests/tests/rightClickMenu.spec.ts:3-5
Timestamp: 2026-04-08T18:10:12.748Z
Learning: In Comfy-Org/ComfyUI_frontend, `import-x/no-unresolved` errors caused by `@/` (src alias) and `e2e/` (browser_tests alias) imports inside `browser_tests/**/*.ts` are false positives in CodeRabbit’s sandbox because the sandbox cannot resolve the root `tsconfig.json` (it extends `astro/tsconfigs/strict`), which breaks the TypeScript import resolver. Since ESLint passes clean locally and in CI, do not treat these specific `@/` and `e2e/` unresolved import findings as real issues, and do not recommend adding `browser_tests/tsconfig.json` (or similar) to ESLint/CodeRabbit resolver project lists for this case.
Learnt from: kaili-yang
Repo: Comfy-Org/ComfyUI_frontend PR: 11531
File: src/composables/maskeditor/brushDrawingUtils.ts:14-14
Timestamp: 2026-04-22T04:06:01.353Z
Learning: In the Comfy-Org/ComfyUI_frontend codebase, `knip` is used to detect unused exports. When reviewing TypeScript/TSX code, avoid recommending or introducing exported types/functions/constants that have no concrete external consumers (e.g., they’re only used within the same module). Only recommend exporting when there is an actual external usage outside the module. If something might be needed externally later, keep it non-exported for now and defer exporting until an external consumer is added.
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 12197
File: src/renderer/extensions/linearMode/AppInput.vue:23-23
Timestamp: 2026-05-14T21:10:45.382Z
Learning: In the ComfyUI_frontend repo, `WidgetEntityId` is a branded template-literal type (e.g., `${UUID}:${NodeId}:${string}`) and should be treated as structurally non-empty when valid—so an empty string is invalid and must be treated as “absent”. When handling optional `entityId?: WidgetEntityId` values (e.g., for Vue props or TS function parameters), guard missing/invalid IDs with a truthy check (e.g., `if (!entityId) return`) rather than `if (entityId === undefined)`. Rationale: `=== undefined` would allow empty-string values through into stores such as `appModeStore.selectedInputs`, creating rows that can’t resolve back to a widget.
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 12197
File: src/core/schemas/parseNodePropertyArray.ts:1-1
Timestamp: 2026-05-18T21:34:41.153Z
Learning: In TypeScript, it is correct to use `import type { z } from 'zod'` when the imported symbol `z` is used exclusively in type-annotation/type-only positions (e.g., `schema: z.ZodType<T[]>`, `z.infer<typeof schema>`, `type X = z.AnyZodObject`, etc.). Because `import type` is fully erased at compile time and requires no runtime value, code reviewers should not flag `import type { z }` as an error (e.g., as an unused value import or incorrect runtime import) in TypeScript/React projects when `z` is only referenced in type positions. If `z` is referenced in a value/runtime position, it should instead be imported with a normal `import { z } from 'zod'`.
Comment on lines
+73
to
+76
| const customTemplates: Record<string, string[]> = {} | ||
| await route.fulfill({ | ||
| status: 200, | ||
| body: JSON.stringify(customTemplates), |
Contributor
There was a problem hiding this comment.
The typed local customTemplates then JSON.stringify(customTemplates) is indirection for an empty object — inline it.
Suggested change
| const customTemplates: Record<string, string[]> = {} | |
| await route.fulfill({ | |
| status: 200, | |
| body: JSON.stringify(customTemplates), | |
| await route.fulfill({ | |
| status: 200, | |
| body: '{}', |
Collaborator
Author
There was a problem hiding this comment.
Agreed, this is a valid nit. The inline body: '{}' form is simpler for this fixed empty response. I left it as-is here because I wanted to avoid another non-functional review round on this already-approved stabilization PR while we continue toward the missing model detection changes before .46 becomes stable.
@coderabbitai please create an issue for this and assign it to me.
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
gh issue create \
--repo "Comfy-Org/ComfyUI_frontend" \
--title "Simplify TemplateHelper.mockIndex(): inline '{}' instead of JSON.stringify(customTemplates)" \
--assignee "jaeone94" \
--body "## Summary
In \`browser_tests/fixtures/helpers/TemplateHelper.ts\`, the \`mockIndex()\` method constructs an unnecessary intermediate variable before serializing it:
\`\`\`ts
const customTemplates: Record<string, string[]> = {}
await route.fulfill({
status: 200,
body: JSON.stringify(customTemplates),
...
})
\`\`\`
Since the response is always a fixed empty object, the typed local variable and \`JSON.stringify\` call are indirection that can be replaced with an inlined string literal:
\`\`\`ts
await route.fulfill({
status: 200,
body: '{}',
...
})
\`\`\`
## Context
This was flagged as a nit during review of PR `#12999` (comment: https://github.com/Comfy-Org/ComfyUI_frontend/pull/12999#discussion_r3443589801). It was deferred from that PR to avoid a non-functional review round on an already-approved stabilization PR.
Requested by `@jaeone94`."Length of output: 1202
@jaeone94, I've created the follow-up issue and assigned it to you: #13014
🐇 (ᵔᴥᵔ)
🧠 Learnings used
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7169
File: src/platform/remote/comfyui/jobs/jobTypes.ts:1-107
Timestamp: 2025-12-09T03:39:54.501Z
Learning: In the ComfyUI_frontend project, Zod is on v3.x. Do not suggest Zod v4 standalone validators (z.uuid, z.ulid, z.cuid2, z.nanoid) until an upgrade to Zod 4 is performed. When reviewing TypeScript files (e.g., src/platform/remote/comfyui/jobs/jobTypes.ts) validate against Zod 3 capabilities and avoid introducing v4-specific features; flag any proposal to upgrade or incorporate v4-only validators and propose staying with compatible 3.x patterns.
Learnt from: christian-byrne
Repo: Comfy-Org/ComfyUI_frontend PR: 7358
File: src/components/dialog/content/signin/SignUpForm.vue:45-54
Timestamp: 2025-12-11T12:25:15.470Z
Learning: This repository uses CI automation to format code (pnpm format). Do not include manual formatting suggestions in code reviews for Comfy-Org/ComfyUI_frontend. If formatting issues are detected, rely on the CI formatter or re-run pnpm format. Focus reviews on correctness, readability, performance, accessibility, and maintainability rather than style formatting.
Learnt from: christian-byrne
Repo: Comfy-Org/ComfyUI_frontend PR: 7416
File: src/stores/imagePreviewStore.ts:5-7
Timestamp: 2025-12-13T11:03:11.264Z
Learning: In the ComfyUI_frontend repository, lint rules require keeping 'import type' statements separate from non-type imports, even if importing from the same module. Do not suggest consolidating them into a single import statement. Ensure type imports remain on their own line (import type { ... } from 'module') and regular imports stay on separate lines.
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7537
File: src/components/ui/button/Button.stories.ts:45-55
Timestamp: 2025-12-17T00:40:09.635Z
Learning: Prefer pure function declarations over function expressions (e.g., use function foo() { ... } instead of const foo = () => { ... }) for pure functions in the repository. Function declarations are more functional-leaning, offer better hoisting clarity, and can improve readability and tooling consistency. Apply this guideline across TypeScript files in Comfy-Org/ComfyUI_frontend, including story and UI component code, except where a function expression is semantically required (e.g., callbacks, higher-order functions with closures).
Learnt from: kaili-yang
Repo: Comfy-Org/ComfyUI_frontend PR: 7805
File: src/composables/useCoreCommands.ts:439-439
Timestamp: 2025-12-30T22:22:33.836Z
Learning: When accessing reactive properties from Pinia stores in TypeScript files, avoid using .value on direct property access (e.g., useStore().isOverlayExpanded). Pinia auto-wraps refs when accessed directly, returning the primitive value. The .value accessor is only needed when destructuring store properties or when using storeToRefs().
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 8248
File: src/platform/assets/schemas/assetSchema.ts:109-118
Timestamp: 2026-01-22T23:44:59.888Z
Learning: Do not export declarations (e.g., schemas, constants, types) unless they are actually used elsewhere in the codebase. Keep the public API surface minimal by exporting only items with concrete usage. This reduces coupling and keeps imports explicit. In src/platform/assets/schemas/assetSchema.ts, ensure only necessary exports are exposed; remove unused exports or switch to non-exported/internal where appropriate.
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 8722
File: src/composables/maskeditor/useBrushDrawing.ts:5-5
Timestamp: 2026-02-08T03:19:12.009Z
Learning: Treat unresolved import warnings caused by pnpm workspace catalog mappings (pnpm-workspace.yaml) and unplugin-typegpu in vite.config.mts as false positives. Do not flag these in PR reviews; they are expected in sandboxes without node_modules and are not indicative of actual missing dependencies in the TS codebase.
Learnt from: christian-byrne
Repo: Comfy-Org/ComfyUI_frontend PR: 8966
File: src/extensions/core/load3d.ts:427-430
Timestamp: 2026-02-19T02:06:23.468Z
Learning: In TypeScript, you can use typeof SomeClass in type annotation positions (e.g., param: typeof LGraphNode) even when SomeClass is imported with import type. This is a type query that only exists at compile time and is erased in runtime, so it is safe to combine with type-only imports. Apply this pattern to TS files broadly when you need a type that references the shape of a class or constructor function without importing the value at runtime.
Learnt from: christian-byrne
Repo: Comfy-Org/ComfyUI_frontend PR: 8966
File: src/extensions/core/uploadAudio.ts:91-94
Timestamp: 2026-02-19T02:06:38.395Z
Learning: In TypeScript files, you can use a type annotation like 'nodeType: typeof MyClass' even if MyClass is imported via 'import type'. Both the type-only import and 'typeof' operate at the type level and are erased at compile time. This pattern is commonly used for constructor types (e.g., 'nodeType: typeof LGraphNode'). Apply this pattern across TypeScript files in the repository (src/**/*.ts) as appropriate, ensuring the imported symbol is a type-only import when possible for clarity and to avoid runtime imports.
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 8992
File: src/lib/litegraph/src/widgets/GradientSliderWidget.ts:18-18
Timestamp: 2026-02-20T21:08:19.814Z
Learning: When drawing with CanvasRenderingContext2D in TypeScript/JavaScript, wrap the drawing logic with ctx.save() at the start and ctx.restore() at the end to preserve and restore the canvas state. Do not manually destructure and restore individual properties (e.g., fillStyle, strokeStyle); rely on save/restore to manage state changes in a scoped manner. This should be applied to all TS files that perform canvas drawing.
Learnt from: dante01yoon
Repo: Comfy-Org/ComfyUI_frontend PR: 9075
File: src/scripts/api.featureFlags.test.ts:237-268
Timestamp: 2026-02-22T04:27:33.379Z
Learning: In Vite/Vitest, import.meta.env.DEV is true for any mode that is not 'production' (i.e., DEV is the opposite of PROD, and can be true in 'test', 'development', etc.). Do not assume DEV implies only 'development' mode. When reviewing code and tests, treat DEV as a non-production flag and verify environment-specific logic accordingly. Reference: https://vite.dev/guide/env-and-mode#modes
Learnt from: christian-byrne
Repo: Comfy-Org/ComfyUI_frontend PR: 9427
File: src/renderer/extensions/vueNodes/widgets/components/form/dropdown/FormDropdownMenuFilter.vue:33-33
Timestamp: 2026-03-06T00:53:28.835Z
Learning: When reviewing code, note that the enforce-canonical-classes (better-tailwindcss) rule may auto-fix Tailwind v3 !class-name syntax by converting it to v4 class-name! syntax. Do not treat these auto-fixed class-name! instances as newly introduced issues; the perceived change is in syntax placement, not in usage or intent. This guidance applies across all .vue and .ts files in the repository.
Learnt from: sonnybox
Repo: Comfy-Org/ComfyUI_frontend PR: 9446
File: src/renderer/extensions/vueNodes/widgets/components/WidgetTextarea.vue:45-45
Timestamp: 2026-03-06T01:55:00.013Z
Learning: Treat wrap-break-word as a valid Tailwind CSS utility for overflow-wrap: break-word in Tailwind v4+ projects. Do not flag this class as invalid in any Vue (.vue) or TypeScript (.ts/.tsx) files within the repository (e.g., Comfy-Org/ComfyUI_frontend) or other Tailwind v4+ projects. When reviewing, verify that the class is used to enable word breaking in long text content and reference the Tailwind docs: https://tailwindcss.com/docs/overflow-wrap.
Learnt from: christian-byrne
Repo: Comfy-Org/ComfyUI_frontend PR: 9554
File: browser_tests/fixtures/ComfyPage.ts:189-193
Timestamp: 2026-03-12T10:03:55.383Z
Learning: In browser_tests, seed localStorage-based feature flags at init time using FeatureFlagHelper.seedFlags() (which uses page.addInitScript) before any navigation. Ensure this happens before comfyPage.setup() is called. The comfyPageFixture currently yields after calling comfyPage.setup(), so do not defer setup() from the fixture or add a pre-setup hook, as changing this would break existing tests.
Learnt from: Yourz
Repo: Comfy-Org/ComfyUI_frontend PR: 10310
File: browser_tests/tests/topbarMenuDismissal.spec.ts:0-0
Timestamp: 2026-03-19T16:16:40.350Z
Learning: In Comfy-Org/ComfyUI_frontend, when reviewing changes limited to adding/revising `:modal="false"` (or similar modal/non-modal behavior props) on reakit-ui/reka-ui primitives such as `ContextMenuRoot` and `DropdownMenuRoot`, do not request Playwright/E2E regression coverage solely for that prop-level change. The dismissal behavior for modal vs non-modal is documented and tested upstream in the reka-ui library; only ask for E2E tests when the change affects ComfyUI_frontend behavior beyond these primitive prop wiring differences (e.g., custom logic, integration behavior, or new user flows not covered upstream).
Learnt from: jaeone94
Repo: Comfy-Org/ComfyUI_frontend PR: 10741
File: browser_tests/fixtures/helpers/boundsUtils.ts:32-45
Timestamp: 2026-03-31T12:17:31.232Z
Learning: In this codebase’s Playwright E2E test helpers under `browser_tests/fixtures/helpers/`, prefer measuring collapsed node dimensions directly from the DOM using `offsetWidth`/`offsetHeight`. Do not access Pinia stores from within `page.evaluate` blocks in E2E helpers (e.g., avoid calling `getNodeCollapsedSize()` or reading from `layoutStore`). This is intentional: E2E helpers should validate the final user-visible result, while Pinia store synchronization correctness is expected to be covered by unit tests.
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 10958
File: browser_tests/tests/bottomPanelLogs.spec.ts:4-4
Timestamp: 2026-04-08T18:09:35.787Z
Learning: In this repo’s E2E browser tests, do not flag `import-x/no-unresolved` for imports using the `e2e/*` path alias. CodeRabbit sandbox reports these as unresolved because it may not run a full `pnpm install`; `e2e/*` is defined in the root `tsconfig.json` and the related `apps/website/tsconfig.json`/workspace dependency resolution only works when dependencies are installed. Apply this only to `browser_tests/**/*.ts` files.
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 10958
File: browser_tests/tests/subgraph/subgraphSearch.spec.ts:3-4
Timestamp: 2026-04-08T18:09:50.771Z
Learning: In Comfy-Org/ComfyUI_frontend browser tests (browser_tests/**/*.ts), do not report `import-x/no-unresolved` for imports using the `e2e/*` alias. The alias is defined in the root tsconfig.json as `"e2e/*": ["./browser_tests/*"]`, and eslint.config.ts includes the correct tsconfig `project` list; lint passes locally and in CI. Treat any `e2e/*` unresolved-import findings in the CodeRabbit sandbox as false positives.
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 10958
File: browser_tests/tests/rightClickMenu.spec.ts:3-5
Timestamp: 2026-04-08T18:10:12.748Z
Learning: In Comfy-Org/ComfyUI_frontend, `import-x/no-unresolved` errors caused by `@/` (src alias) and `e2e/` (browser_tests alias) imports inside `browser_tests/**/*.ts` are false positives in CodeRabbit’s sandbox because the sandbox cannot resolve the root `tsconfig.json` (it extends `astro/tsconfigs/strict`), which breaks the TypeScript import resolver. Since ESLint passes clean locally and in CI, do not treat these specific `@/` and `e2e/` unresolved import findings as real issues, and do not recommend adding `browser_tests/tsconfig.json` (or similar) to ESLint/CodeRabbit resolver project lists for this case.
Learnt from: kaili-yang
Repo: Comfy-Org/ComfyUI_frontend PR: 11531
File: src/composables/maskeditor/brushDrawingUtils.ts:14-14
Timestamp: 2026-04-22T04:06:01.353Z
Learning: In the Comfy-Org/ComfyUI_frontend codebase, `knip` is used to detect unused exports. When reviewing TypeScript/TSX code, avoid recommending or introducing exported types/functions/constants that have no concrete external consumers (e.g., they’re only used within the same module). Only recommend exporting when there is an actual external usage outside the module. If something might be needed externally later, keep it non-exported for now and defer exporting until an external consumer is added.
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 12197
File: src/renderer/extensions/linearMode/AppInput.vue:23-23
Timestamp: 2026-05-14T21:10:45.382Z
Learning: In the ComfyUI_frontend repo, `WidgetEntityId` is a branded template-literal type (e.g., `${UUID}:${NodeId}:${string}`) and should be treated as structurally non-empty when valid—so an empty string is invalid and must be treated as “absent”. When handling optional `entityId?: WidgetEntityId` values (e.g., for Vue props or TS function parameters), guard missing/invalid IDs with a truthy check (e.g., `if (!entityId) return`) rather than `if (entityId === undefined)`. Rationale: `=== undefined` would allow empty-string values through into stores such as `appModeStore.selectedInputs`, creating rows that can’t resolve back to a widget.
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 12197
File: src/core/schemas/parseNodePropertyArray.ts:1-1
Timestamp: 2026-05-18T21:34:41.153Z
Learning: In TypeScript, it is correct to use `import type { z } from 'zod'` when the imported symbol `z` is used exclusively in type-annotation/type-only positions (e.g., `schema: z.ZodType<T[]>`, `z.infer<typeof schema>`, `type X = z.AnyZodObject`, etc.). Because `import type` is fully erased at compile time and requires no runtime value, code reviewers should not flag `import type { z }` as an error (e.g., as an unused value import or incorrect runtime import) in TypeScript/React projects when `z` is only referenced in type positions. If `z` is referenced in a value/runtime position, it should instead be imported with a normal `import { z } from 'zod'`.
| } | ||
| }) | ||
| } | ||
| const customTemplatesPattern = '**/api/workflow_templates' |
Contributor
There was a problem hiding this comment.
Convention nit (non-blocking): sibling fixtures anchor with regex (/\/api\/assets(?:\?.*)?$/, /\/api\/prompt$/). The glob is fine here — it matches the README's **/api/object_info style, has no trailing wildcard so it won't catch the per-template .json fetches, and there are no query strings on this endpoint. Just noting the mixed convention.
Collaborator
Author
There was a problem hiding this comment.
Agreed. The current glob is safe for this endpoint, but the mixed route pattern convention is a valid cleanup concern. Since that is more of a fixture consistency/refactor pass than part of the E2E stabilization itself, I think it is better handled in a follow-up PR.
@coderabbitai please create an issue for this and assign it to me.
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
gh issue create \
--repo "Comfy-Org/ComfyUI_frontend" \
--assignee "jaeone94" \
--title "Refactor: Standardize Playwright route pattern convention in E2E fixture helpers" \
--body "## Summary
Fixture helpers under \`browser_tests/fixtures/helpers/\` currently use a mixed convention for Playwright route patterns:
- Some fixtures (e.g. the assets and prompt handlers) anchor routes with regex patterns such as \`/\/api\/assets(?:\?.*)?$/\` and \`/\/api\/prompt$/\`.
- The \`TemplateHelper.mockIndex()\` handler added in `#12999` uses a glob pattern (\`**/api/workflow_templates\`) consistent with the \`README\`-documented \`**/api/object_info\` style.
Both approaches work correctly, but the inconsistency makes the fixture layer harder to maintain and reason about. A follow-up refactor pass should align all route patterns to a single convention (either regex or glob) across the fixture helpers.
## References
- PR: `#12999`
- Comment: https://github.com/Comfy-Org/ComfyUI_frontend/pull/12999#discussion_r3443589817
- Requested by: `@jaeone94`
"Length of output: 1191
@jaeone94, the follow-up issue has been created and assigned to you: Refactor: Standardize Playwright route pattern convention in E2E fixture helpers → #13015
(ᵔᴥᵔ)~🐇
🧠 Learnings used
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7169
File: src/platform/remote/comfyui/jobs/jobTypes.ts:1-107
Timestamp: 2025-12-09T03:39:54.501Z
Learning: In the ComfyUI_frontend project, Zod is on v3.x. Do not suggest Zod v4 standalone validators (z.uuid, z.ulid, z.cuid2, z.nanoid) until an upgrade to Zod 4 is performed. When reviewing TypeScript files (e.g., src/platform/remote/comfyui/jobs/jobTypes.ts) validate against Zod 3 capabilities and avoid introducing v4-specific features; flag any proposal to upgrade or incorporate v4-only validators and propose staying with compatible 3.x patterns.
Learnt from: christian-byrne
Repo: Comfy-Org/ComfyUI_frontend PR: 7358
File: src/components/dialog/content/signin/SignUpForm.vue:45-54
Timestamp: 2025-12-11T12:25:15.470Z
Learning: This repository uses CI automation to format code (pnpm format). Do not include manual formatting suggestions in code reviews for Comfy-Org/ComfyUI_frontend. If formatting issues are detected, rely on the CI formatter or re-run pnpm format. Focus reviews on correctness, readability, performance, accessibility, and maintainability rather than style formatting.
Learnt from: christian-byrne
Repo: Comfy-Org/ComfyUI_frontend PR: 7416
File: src/stores/imagePreviewStore.ts:5-7
Timestamp: 2025-12-13T11:03:11.264Z
Learning: In the ComfyUI_frontend repository, lint rules require keeping 'import type' statements separate from non-type imports, even if importing from the same module. Do not suggest consolidating them into a single import statement. Ensure type imports remain on their own line (import type { ... } from 'module') and regular imports stay on separate lines.
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7537
File: src/components/ui/button/Button.stories.ts:45-55
Timestamp: 2025-12-17T00:40:09.635Z
Learning: Prefer pure function declarations over function expressions (e.g., use function foo() { ... } instead of const foo = () => { ... }) for pure functions in the repository. Function declarations are more functional-leaning, offer better hoisting clarity, and can improve readability and tooling consistency. Apply this guideline across TypeScript files in Comfy-Org/ComfyUI_frontend, including story and UI component code, except where a function expression is semantically required (e.g., callbacks, higher-order functions with closures).
Learnt from: kaili-yang
Repo: Comfy-Org/ComfyUI_frontend PR: 7805
File: src/composables/useCoreCommands.ts:439-439
Timestamp: 2025-12-30T22:22:33.836Z
Learning: When accessing reactive properties from Pinia stores in TypeScript files, avoid using .value on direct property access (e.g., useStore().isOverlayExpanded). Pinia auto-wraps refs when accessed directly, returning the primitive value. The .value accessor is only needed when destructuring store properties or when using storeToRefs().
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 8248
File: src/platform/assets/schemas/assetSchema.ts:109-118
Timestamp: 2026-01-22T23:44:59.888Z
Learning: Do not export declarations (e.g., schemas, constants, types) unless they are actually used elsewhere in the codebase. Keep the public API surface minimal by exporting only items with concrete usage. This reduces coupling and keeps imports explicit. In src/platform/assets/schemas/assetSchema.ts, ensure only necessary exports are exposed; remove unused exports or switch to non-exported/internal where appropriate.
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 8722
File: src/composables/maskeditor/useBrushDrawing.ts:5-5
Timestamp: 2026-02-08T03:19:12.009Z
Learning: Treat unresolved import warnings caused by pnpm workspace catalog mappings (pnpm-workspace.yaml) and unplugin-typegpu in vite.config.mts as false positives. Do not flag these in PR reviews; they are expected in sandboxes without node_modules and are not indicative of actual missing dependencies in the TS codebase.
Learnt from: christian-byrne
Repo: Comfy-Org/ComfyUI_frontend PR: 8966
File: src/extensions/core/load3d.ts:427-430
Timestamp: 2026-02-19T02:06:23.468Z
Learning: In TypeScript, you can use typeof SomeClass in type annotation positions (e.g., param: typeof LGraphNode) even when SomeClass is imported with import type. This is a type query that only exists at compile time and is erased in runtime, so it is safe to combine with type-only imports. Apply this pattern to TS files broadly when you need a type that references the shape of a class or constructor function without importing the value at runtime.
Learnt from: christian-byrne
Repo: Comfy-Org/ComfyUI_frontend PR: 8966
File: src/extensions/core/uploadAudio.ts:91-94
Timestamp: 2026-02-19T02:06:38.395Z
Learning: In TypeScript files, you can use a type annotation like 'nodeType: typeof MyClass' even if MyClass is imported via 'import type'. Both the type-only import and 'typeof' operate at the type level and are erased at compile time. This pattern is commonly used for constructor types (e.g., 'nodeType: typeof LGraphNode'). Apply this pattern across TypeScript files in the repository (src/**/*.ts) as appropriate, ensuring the imported symbol is a type-only import when possible for clarity and to avoid runtime imports.
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 8992
File: src/lib/litegraph/src/widgets/GradientSliderWidget.ts:18-18
Timestamp: 2026-02-20T21:08:19.814Z
Learning: When drawing with CanvasRenderingContext2D in TypeScript/JavaScript, wrap the drawing logic with ctx.save() at the start and ctx.restore() at the end to preserve and restore the canvas state. Do not manually destructure and restore individual properties (e.g., fillStyle, strokeStyle); rely on save/restore to manage state changes in a scoped manner. This should be applied to all TS files that perform canvas drawing.
Learnt from: dante01yoon
Repo: Comfy-Org/ComfyUI_frontend PR: 9075
File: src/scripts/api.featureFlags.test.ts:237-268
Timestamp: 2026-02-22T04:27:33.379Z
Learning: In Vite/Vitest, import.meta.env.DEV is true for any mode that is not 'production' (i.e., DEV is the opposite of PROD, and can be true in 'test', 'development', etc.). Do not assume DEV implies only 'development' mode. When reviewing code and tests, treat DEV as a non-production flag and verify environment-specific logic accordingly. Reference: https://vite.dev/guide/env-and-mode#modes
Learnt from: christian-byrne
Repo: Comfy-Org/ComfyUI_frontend PR: 9427
File: src/renderer/extensions/vueNodes/widgets/components/form/dropdown/FormDropdownMenuFilter.vue:33-33
Timestamp: 2026-03-06T00:53:28.835Z
Learning: When reviewing code, note that the enforce-canonical-classes (better-tailwindcss) rule may auto-fix Tailwind v3 !class-name syntax by converting it to v4 class-name! syntax. Do not treat these auto-fixed class-name! instances as newly introduced issues; the perceived change is in syntax placement, not in usage or intent. This guidance applies across all .vue and .ts files in the repository.
Learnt from: sonnybox
Repo: Comfy-Org/ComfyUI_frontend PR: 9446
File: src/renderer/extensions/vueNodes/widgets/components/WidgetTextarea.vue:45-45
Timestamp: 2026-03-06T01:55:00.013Z
Learning: Treat wrap-break-word as a valid Tailwind CSS utility for overflow-wrap: break-word in Tailwind v4+ projects. Do not flag this class as invalid in any Vue (.vue) or TypeScript (.ts/.tsx) files within the repository (e.g., Comfy-Org/ComfyUI_frontend) or other Tailwind v4+ projects. When reviewing, verify that the class is used to enable word breaking in long text content and reference the Tailwind docs: https://tailwindcss.com/docs/overflow-wrap.
Learnt from: christian-byrne
Repo: Comfy-Org/ComfyUI_frontend PR: 9554
File: browser_tests/fixtures/ComfyPage.ts:189-193
Timestamp: 2026-03-12T10:03:55.383Z
Learning: In browser_tests, seed localStorage-based feature flags at init time using FeatureFlagHelper.seedFlags() (which uses page.addInitScript) before any navigation. Ensure this happens before comfyPage.setup() is called. The comfyPageFixture currently yields after calling comfyPage.setup(), so do not defer setup() from the fixture or add a pre-setup hook, as changing this would break existing tests.
Learnt from: Yourz
Repo: Comfy-Org/ComfyUI_frontend PR: 10310
File: browser_tests/tests/topbarMenuDismissal.spec.ts:0-0
Timestamp: 2026-03-19T16:16:40.350Z
Learning: In Comfy-Org/ComfyUI_frontend, when reviewing changes limited to adding/revising `:modal="false"` (or similar modal/non-modal behavior props) on reakit-ui/reka-ui primitives such as `ContextMenuRoot` and `DropdownMenuRoot`, do not request Playwright/E2E regression coverage solely for that prop-level change. The dismissal behavior for modal vs non-modal is documented and tested upstream in the reka-ui library; only ask for E2E tests when the change affects ComfyUI_frontend behavior beyond these primitive prop wiring differences (e.g., custom logic, integration behavior, or new user flows not covered upstream).
Learnt from: jaeone94
Repo: Comfy-Org/ComfyUI_frontend PR: 10741
File: browser_tests/fixtures/helpers/boundsUtils.ts:32-45
Timestamp: 2026-03-31T12:17:31.232Z
Learning: In this codebase’s Playwright E2E test helpers under `browser_tests/fixtures/helpers/`, prefer measuring collapsed node dimensions directly from the DOM using `offsetWidth`/`offsetHeight`. Do not access Pinia stores from within `page.evaluate` blocks in E2E helpers (e.g., avoid calling `getNodeCollapsedSize()` or reading from `layoutStore`). This is intentional: E2E helpers should validate the final user-visible result, while Pinia store synchronization correctness is expected to be covered by unit tests.
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 10958
File: browser_tests/tests/bottomPanelLogs.spec.ts:4-4
Timestamp: 2026-04-08T18:09:35.787Z
Learning: In this repo’s E2E browser tests, do not flag `import-x/no-unresolved` for imports using the `e2e/*` path alias. CodeRabbit sandbox reports these as unresolved because it may not run a full `pnpm install`; `e2e/*` is defined in the root `tsconfig.json` and the related `apps/website/tsconfig.json`/workspace dependency resolution only works when dependencies are installed. Apply this only to `browser_tests/**/*.ts` files.
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 10958
File: browser_tests/tests/subgraph/subgraphSearch.spec.ts:3-4
Timestamp: 2026-04-08T18:09:50.771Z
Learning: In Comfy-Org/ComfyUI_frontend browser tests (browser_tests/**/*.ts), do not report `import-x/no-unresolved` for imports using the `e2e/*` alias. The alias is defined in the root tsconfig.json as `"e2e/*": ["./browser_tests/*"]`, and eslint.config.ts includes the correct tsconfig `project` list; lint passes locally and in CI. Treat any `e2e/*` unresolved-import findings in the CodeRabbit sandbox as false positives.
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 10958
File: browser_tests/tests/rightClickMenu.spec.ts:3-5
Timestamp: 2026-04-08T18:10:12.748Z
Learning: In Comfy-Org/ComfyUI_frontend, `import-x/no-unresolved` errors caused by `@/` (src alias) and `e2e/` (browser_tests alias) imports inside `browser_tests/**/*.ts` are false positives in CodeRabbit’s sandbox because the sandbox cannot resolve the root `tsconfig.json` (it extends `astro/tsconfigs/strict`), which breaks the TypeScript import resolver. Since ESLint passes clean locally and in CI, do not treat these specific `@/` and `e2e/` unresolved import findings as real issues, and do not recommend adding `browser_tests/tsconfig.json` (or similar) to ESLint/CodeRabbit resolver project lists for this case.
Learnt from: kaili-yang
Repo: Comfy-Org/ComfyUI_frontend PR: 11531
File: src/composables/maskeditor/brushDrawingUtils.ts:14-14
Timestamp: 2026-04-22T04:06:01.353Z
Learning: In the Comfy-Org/ComfyUI_frontend codebase, `knip` is used to detect unused exports. When reviewing TypeScript/TSX code, avoid recommending or introducing exported types/functions/constants that have no concrete external consumers (e.g., they’re only used within the same module). Only recommend exporting when there is an actual external usage outside the module. If something might be needed externally later, keep it non-exported for now and defer exporting until an external consumer is added.
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 12197
File: src/renderer/extensions/linearMode/AppInput.vue:23-23
Timestamp: 2026-05-14T21:10:45.382Z
Learning: In the ComfyUI_frontend repo, `WidgetEntityId` is a branded template-literal type (e.g., `${UUID}:${NodeId}:${string}`) and should be treated as structurally non-empty when valid—so an empty string is invalid and must be treated as “absent”. When handling optional `entityId?: WidgetEntityId` values (e.g., for Vue props or TS function parameters), guard missing/invalid IDs with a truthy check (e.g., `if (!entityId) return`) rather than `if (entityId === undefined)`. Rationale: `=== undefined` would allow empty-string values through into stores such as `appModeStore.selectedInputs`, creating rows that can’t resolve back to a widget.
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 12197
File: src/core/schemas/parseNodePropertyArray.ts:1-1
Timestamp: 2026-05-18T21:34:41.153Z
Learning: In TypeScript, it is correct to use `import type { z } from 'zod'` when the imported symbol `z` is used exclusively in type-annotation/type-only positions (e.g., `schema: z.ZodType<T[]>`, `z.infer<typeof schema>`, `type X = z.AnyZodObject`, etc.). Because `import type` is fully erased at compile time and requires no runtime value, code reviewers should not flag `import type { z }` as an error (e.g., as an unused value import or incorrect runtime import) in TypeScript/React projects when `z` is only referenced in type positions. If `z` is referenced in a value/runtime position, it should instead be imported with a normal `import { z } from 'zod'`.
Comment on lines
+84
to
+88
| this.routeHandlers.push({ | ||
| pattern: customTemplatesPattern, | ||
| handler: customTemplatesHandler | ||
| }) | ||
| await this.page.route(customTemplatesPattern, customTemplatesHandler) |
Contributor
There was a problem hiding this comment.
All three handlers (customTemplates, index, thumbnails) repeat this routeHandlers.push({pattern, handler}) + page.route(pattern, handler) pair. Extracting a small private helper would DRY it up and prevent the easy mistake of routing without registering for cleanup:
private async registerRoute(
pattern: string,
handler: (route: Route) => Promise<void>
): Promise<void> {
this.routeHandlers.push({ pattern, handler })
await this.page.route(pattern, handler)
}Then each call site collapses to await this.registerRoute(pattern, handler).
Collaborator
Author
There was a problem hiding this comment.
Agreed, this helper extraction would make the route registration safer and clearer. I did not want to broaden this PR from a targeted E2E stabilization into a TemplateHelper refactor, especially since the immediate goal was to stop cloud-only runtime requests from leaking into exact-count assertions.
This is a good follow-up cleanup.
@coderabbitai please create an issue for this and assign it to me.
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
gh issue create \
--repo Comfy-Org/ComfyUI_frontend \
--title "Refactor: Extract registerRoute private helper in TemplateHelper" \
--assignee jaeone94 \
--body "## Summary
In \`browser_tests/fixtures/helpers/TemplateHelper.ts\`, the pattern of pushing to \`routeHandlers\` and calling \`page.route()\` is repeated across all three route registrations (\`customTemplates\`, \`index\`, \`thumbnails\`). Extracting a small private helper would DRY this up and prevent the easy mistake of routing without registering for cleanup.
## Proposed change
\`\`\`ts
private async registerRoute(
pattern: string,
handler: (route: Route) => Promise<void>
): Promise<void> {
this.routeHandlers.push({ pattern, handler })
await this.page.route(pattern, handler)
}
\`\`\`
Each call site (\`mockIndex\`, \`mockThumbnails\`) then collapses to:
\`\`\`ts
await this.registerRoute(pattern, handler)
\`\`\`
## Context
Identified during review of PR `#12999` (comment: https://github.com/Comfy-Org/ComfyUI_frontend/pull/12999#discussion_r3443589821).
The current PR was intentionally scoped to E2E test stabilization; this cleanup is deferred as a follow-up.
## Related
- PR: https://github.com/Comfy-Org/ComfyUI_frontend/pull/12999
- File: \`browser_tests/fixtures/helpers/TemplateHelper.ts\`
"Length of output: 1429
@jaeone94 Done! I've created the follow-up issue and assigned it to you: #13017
(ﾉ◕ヮ◕)ﾉ*:･ﾟ✧
🧠 Learnings used
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7169
File: src/platform/remote/comfyui/jobs/jobTypes.ts:1-107
Timestamp: 2025-12-09T03:39:54.501Z
Learning: In the ComfyUI_frontend project, Zod is on v3.x. Do not suggest Zod v4 standalone validators (z.uuid, z.ulid, z.cuid2, z.nanoid) until an upgrade to Zod 4 is performed. When reviewing TypeScript files (e.g., src/platform/remote/comfyui/jobs/jobTypes.ts) validate against Zod 3 capabilities and avoid introducing v4-specific features; flag any proposal to upgrade or incorporate v4-only validators and propose staying with compatible 3.x patterns.
Learnt from: christian-byrne
Repo: Comfy-Org/ComfyUI_frontend PR: 7358
File: src/components/dialog/content/signin/SignUpForm.vue:45-54
Timestamp: 2025-12-11T12:25:15.470Z
Learning: This repository uses CI automation to format code (pnpm format). Do not include manual formatting suggestions in code reviews for Comfy-Org/ComfyUI_frontend. If formatting issues are detected, rely on the CI formatter or re-run pnpm format. Focus reviews on correctness, readability, performance, accessibility, and maintainability rather than style formatting.
Learnt from: christian-byrne
Repo: Comfy-Org/ComfyUI_frontend PR: 7416
File: src/stores/imagePreviewStore.ts:5-7
Timestamp: 2025-12-13T11:03:11.264Z
Learning: In the ComfyUI_frontend repository, lint rules require keeping 'import type' statements separate from non-type imports, even if importing from the same module. Do not suggest consolidating them into a single import statement. Ensure type imports remain on their own line (import type { ... } from 'module') and regular imports stay on separate lines.
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7537
File: src/components/ui/button/Button.stories.ts:45-55
Timestamp: 2025-12-17T00:40:09.635Z
Learning: Prefer pure function declarations over function expressions (e.g., use function foo() { ... } instead of const foo = () => { ... }) for pure functions in the repository. Function declarations are more functional-leaning, offer better hoisting clarity, and can improve readability and tooling consistency. Apply this guideline across TypeScript files in Comfy-Org/ComfyUI_frontend, including story and UI component code, except where a function expression is semantically required (e.g., callbacks, higher-order functions with closures).
Learnt from: kaili-yang
Repo: Comfy-Org/ComfyUI_frontend PR: 7805
File: src/composables/useCoreCommands.ts:439-439
Timestamp: 2025-12-30T22:22:33.836Z
Learning: When accessing reactive properties from Pinia stores in TypeScript files, avoid using .value on direct property access (e.g., useStore().isOverlayExpanded). Pinia auto-wraps refs when accessed directly, returning the primitive value. The .value accessor is only needed when destructuring store properties or when using storeToRefs().
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 8248
File: src/platform/assets/schemas/assetSchema.ts:109-118
Timestamp: 2026-01-22T23:44:59.888Z
Learning: Do not export declarations (e.g., schemas, constants, types) unless they are actually used elsewhere in the codebase. Keep the public API surface minimal by exporting only items with concrete usage. This reduces coupling and keeps imports explicit. In src/platform/assets/schemas/assetSchema.ts, ensure only necessary exports are exposed; remove unused exports or switch to non-exported/internal where appropriate.
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 8722
File: src/composables/maskeditor/useBrushDrawing.ts:5-5
Timestamp: 2026-02-08T03:19:12.009Z
Learning: Treat unresolved import warnings caused by pnpm workspace catalog mappings (pnpm-workspace.yaml) and unplugin-typegpu in vite.config.mts as false positives. Do not flag these in PR reviews; they are expected in sandboxes without node_modules and are not indicative of actual missing dependencies in the TS codebase.
Learnt from: christian-byrne
Repo: Comfy-Org/ComfyUI_frontend PR: 8966
File: src/extensions/core/load3d.ts:427-430
Timestamp: 2026-02-19T02:06:23.468Z
Learning: In TypeScript, you can use typeof SomeClass in type annotation positions (e.g., param: typeof LGraphNode) even when SomeClass is imported with import type. This is a type query that only exists at compile time and is erased in runtime, so it is safe to combine with type-only imports. Apply this pattern to TS files broadly when you need a type that references the shape of a class or constructor function without importing the value at runtime.
Learnt from: christian-byrne
Repo: Comfy-Org/ComfyUI_frontend PR: 8966
File: src/extensions/core/uploadAudio.ts:91-94
Timestamp: 2026-02-19T02:06:38.395Z
Learning: In TypeScript files, you can use a type annotation like 'nodeType: typeof MyClass' even if MyClass is imported via 'import type'. Both the type-only import and 'typeof' operate at the type level and are erased at compile time. This pattern is commonly used for constructor types (e.g., 'nodeType: typeof LGraphNode'). Apply this pattern across TypeScript files in the repository (src/**/*.ts) as appropriate, ensuring the imported symbol is a type-only import when possible for clarity and to avoid runtime imports.
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 8992
File: src/lib/litegraph/src/widgets/GradientSliderWidget.ts:18-18
Timestamp: 2026-02-20T21:08:19.814Z
Learning: When drawing with CanvasRenderingContext2D in TypeScript/JavaScript, wrap the drawing logic with ctx.save() at the start and ctx.restore() at the end to preserve and restore the canvas state. Do not manually destructure and restore individual properties (e.g., fillStyle, strokeStyle); rely on save/restore to manage state changes in a scoped manner. This should be applied to all TS files that perform canvas drawing.
Learnt from: dante01yoon
Repo: Comfy-Org/ComfyUI_frontend PR: 9075
File: src/scripts/api.featureFlags.test.ts:237-268
Timestamp: 2026-02-22T04:27:33.379Z
Learning: In Vite/Vitest, import.meta.env.DEV is true for any mode that is not 'production' (i.e., DEV is the opposite of PROD, and can be true in 'test', 'development', etc.). Do not assume DEV implies only 'development' mode. When reviewing code and tests, treat DEV as a non-production flag and verify environment-specific logic accordingly. Reference: https://vite.dev/guide/env-and-mode#modes
Learnt from: christian-byrne
Repo: Comfy-Org/ComfyUI_frontend PR: 9427
File: src/renderer/extensions/vueNodes/widgets/components/form/dropdown/FormDropdownMenuFilter.vue:33-33
Timestamp: 2026-03-06T00:53:28.835Z
Learning: When reviewing code, note that the enforce-canonical-classes (better-tailwindcss) rule may auto-fix Tailwind v3 !class-name syntax by converting it to v4 class-name! syntax. Do not treat these auto-fixed class-name! instances as newly introduced issues; the perceived change is in syntax placement, not in usage or intent. This guidance applies across all .vue and .ts files in the repository.
Learnt from: sonnybox
Repo: Comfy-Org/ComfyUI_frontend PR: 9446
File: src/renderer/extensions/vueNodes/widgets/components/WidgetTextarea.vue:45-45
Timestamp: 2026-03-06T01:55:00.013Z
Learning: Treat wrap-break-word as a valid Tailwind CSS utility for overflow-wrap: break-word in Tailwind v4+ projects. Do not flag this class as invalid in any Vue (.vue) or TypeScript (.ts/.tsx) files within the repository (e.g., Comfy-Org/ComfyUI_frontend) or other Tailwind v4+ projects. When reviewing, verify that the class is used to enable word breaking in long text content and reference the Tailwind docs: https://tailwindcss.com/docs/overflow-wrap.
Learnt from: christian-byrne
Repo: Comfy-Org/ComfyUI_frontend PR: 9554
File: browser_tests/fixtures/ComfyPage.ts:189-193
Timestamp: 2026-03-12T10:03:55.383Z
Learning: In browser_tests, seed localStorage-based feature flags at init time using FeatureFlagHelper.seedFlags() (which uses page.addInitScript) before any navigation. Ensure this happens before comfyPage.setup() is called. The comfyPageFixture currently yields after calling comfyPage.setup(), so do not defer setup() from the fixture or add a pre-setup hook, as changing this would break existing tests.
Learnt from: Yourz
Repo: Comfy-Org/ComfyUI_frontend PR: 10310
File: browser_tests/tests/topbarMenuDismissal.spec.ts:0-0
Timestamp: 2026-03-19T16:16:40.350Z
Learning: In Comfy-Org/ComfyUI_frontend, when reviewing changes limited to adding/revising `:modal="false"` (or similar modal/non-modal behavior props) on reakit-ui/reka-ui primitives such as `ContextMenuRoot` and `DropdownMenuRoot`, do not request Playwright/E2E regression coverage solely for that prop-level change. The dismissal behavior for modal vs non-modal is documented and tested upstream in the reka-ui library; only ask for E2E tests when the change affects ComfyUI_frontend behavior beyond these primitive prop wiring differences (e.g., custom logic, integration behavior, or new user flows not covered upstream).
Learnt from: jaeone94
Repo: Comfy-Org/ComfyUI_frontend PR: 10741
File: browser_tests/fixtures/helpers/boundsUtils.ts:32-45
Timestamp: 2026-03-31T12:17:31.232Z
Learning: In this codebase’s Playwright E2E test helpers under `browser_tests/fixtures/helpers/`, prefer measuring collapsed node dimensions directly from the DOM using `offsetWidth`/`offsetHeight`. Do not access Pinia stores from within `page.evaluate` blocks in E2E helpers (e.g., avoid calling `getNodeCollapsedSize()` or reading from `layoutStore`). This is intentional: E2E helpers should validate the final user-visible result, while Pinia store synchronization correctness is expected to be covered by unit tests.
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 10958
File: browser_tests/tests/bottomPanelLogs.spec.ts:4-4
Timestamp: 2026-04-08T18:09:35.787Z
Learning: In this repo’s E2E browser tests, do not flag `import-x/no-unresolved` for imports using the `e2e/*` path alias. CodeRabbit sandbox reports these as unresolved because it may not run a full `pnpm install`; `e2e/*` is defined in the root `tsconfig.json` and the related `apps/website/tsconfig.json`/workspace dependency resolution only works when dependencies are installed. Apply this only to `browser_tests/**/*.ts` files.
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 10958
File: browser_tests/tests/subgraph/subgraphSearch.spec.ts:3-4
Timestamp: 2026-04-08T18:09:50.771Z
Learning: In Comfy-Org/ComfyUI_frontend browser tests (browser_tests/**/*.ts), do not report `import-x/no-unresolved` for imports using the `e2e/*` alias. The alias is defined in the root tsconfig.json as `"e2e/*": ["./browser_tests/*"]`, and eslint.config.ts includes the correct tsconfig `project` list; lint passes locally and in CI. Treat any `e2e/*` unresolved-import findings in the CodeRabbit sandbox as false positives.
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 10958
File: browser_tests/tests/rightClickMenu.spec.ts:3-5
Timestamp: 2026-04-08T18:10:12.748Z
Learning: In Comfy-Org/ComfyUI_frontend, `import-x/no-unresolved` errors caused by `@/` (src alias) and `e2e/` (browser_tests alias) imports inside `browser_tests/**/*.ts` are false positives in CodeRabbit’s sandbox because the sandbox cannot resolve the root `tsconfig.json` (it extends `astro/tsconfigs/strict`), which breaks the TypeScript import resolver. Since ESLint passes clean locally and in CI, do not treat these specific `@/` and `e2e/` unresolved import findings as real issues, and do not recommend adding `browser_tests/tsconfig.json` (or similar) to ESLint/CodeRabbit resolver project lists for this case.
Learnt from: kaili-yang
Repo: Comfy-Org/ComfyUI_frontend PR: 11531
File: src/composables/maskeditor/brushDrawingUtils.ts:14-14
Timestamp: 2026-04-22T04:06:01.353Z
Learning: In the Comfy-Org/ComfyUI_frontend codebase, `knip` is used to detect unused exports. When reviewing TypeScript/TSX code, avoid recommending or introducing exported types/functions/constants that have no concrete external consumers (e.g., they’re only used within the same module). Only recommend exporting when there is an actual external usage outside the module. If something might be needed externally later, keep it non-exported for now and defer exporting until an external consumer is added.
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 12197
File: src/renderer/extensions/linearMode/AppInput.vue:23-23
Timestamp: 2026-05-14T21:10:45.382Z
Learning: In the ComfyUI_frontend repo, `WidgetEntityId` is a branded template-literal type (e.g., `${UUID}:${NodeId}:${string}`) and should be treated as structurally non-empty when valid—so an empty string is invalid and must be treated as “absent”. When handling optional `entityId?: WidgetEntityId` values (e.g., for Vue props or TS function parameters), guard missing/invalid IDs with a truthy check (e.g., `if (!entityId) return`) rather than `if (entityId === undefined)`. Rationale: `=== undefined` would allow empty-string values through into stores such as `appModeStore.selectedInputs`, creating rows that can’t resolve back to a widget.
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 12197
File: src/core/schemas/parseNodePropertyArray.ts:1-1
Timestamp: 2026-05-18T21:34:41.153Z
Learning: In TypeScript, it is correct to use `import type { z } from 'zod'` when the imported symbol `z` is used exclusively in type-annotation/type-only positions (e.g., `schema: z.ZodType<T[]>`, `z.infer<typeof schema>`, `type X = z.AnyZodObject`, etc.). Because `import type` is fully erased at compile time and requires no runtime value, code reviewers should not flag `import type { z }` as an error (e.g., as an unused value import or incorrect runtime import) in TypeScript/React projects when `z` is only referenced in type positions. If `z` is referenced in a value/runtime position, it should instead be imported with a normal `import { z } from 'zod'`.
| import { templateApiFixture } from '@e2e/fixtures/templateApiFixture' | ||
|
|
||
| const test = mergeTests(comfyPageFixture, templateApiFixture) | ||
| const test = mergeTests(createCloudAssetsFixture([]), templateApiFixture) |
Contributor
There was a problem hiding this comment.
This is the line that makes the new **/api/workflow_templates mock necessary: createCloudAssetsFixture([]) runs the suite as @cloud, so the store now actually calls /workflow_templates. Good change — just flagging the coupling for future readers.
Collaborator
Author
There was a problem hiding this comment.
Yes, exactly. Switching this suite to createCloudAssetsFixture([]) is what exposes the cloud runtime path, so the /api/workflow_templates mock is needed to keep the exact-count assertions deterministic. Thanks for calling out that coupling for future readers.
DrJKL
reviewed
Jun 19, 2026
| } | ||
| }) | ||
| } | ||
| const customTemplatesPattern = '**/api/workflow_templates' |
Contributor
There was a problem hiding this comment.
Also, if this is a constant, could you pull it out to module scope and name it something like ROUTE_PATTERN_WORKFLOW_TEMPLATES?
Collaborator
Author
There was a problem hiding this comment.
Agreed. Pulling the workflow templates route pattern to module scope would be a valid cleanup, and it likely makes the most sense together with normalizing the other TemplateHelper route patterns rather than changing only this new one. I kept that refactor out of this stabilization PR for scope control.
@coderabbitai please create an issue for this and assign it to me.
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
gh issue create \
--repo "Comfy-Org/ComfyUI_frontend" \
--title "Refactor: Extract TemplateHelper route patterns to named module-scope constants" \
--assignee "jaeone94" \
--body "## Summary
The route pattern strings inside \`TemplateHelper\` in \`browser_tests/fixtures/helpers/TemplateHelper.ts\` are currently defined as inline \`const\` variables inside method bodies. These should be lifted to module-scope named constants (e.g., \`ROUTE_PATTERN_WORKFLOW_TEMPLATES\`, \`ROUTE_PATTERN_TEMPLATE_INDEX\`, \`ROUTE_PATTERN_TEMPLATE_THUMBNAILS\`) to:
- Improve readability and discoverability
- Enable reuse across tests without duplication
- Make intent explicit through descriptive names
## Scope
Normalize **all three** existing route patterns in \`TemplateHelper\` at the same time:
- \`'**/api/workflow_templates'\`
- \`'**/templates/index.json'\`
- \`'**/templates/**.webp'\`
## Background
Identified during review of PR `#12999` (comment: https://github.com/Comfy-Org/ComfyUI_frontend/pull/12999#discussion_r3443639562). The refactor was intentionally kept out of that stabilization PR for scope control.
Requested by: `@jaeone94`"Length of output: 1283
@jaeone94 Done! The follow-up issue has been created and assigned to you: #13018 — Refactor: Extract TemplateHelper route patterns to named module-scope constants.
It covers normalizing all three existing route patterns in TemplateHelper at once, as you suggested. 🐇
🧠 Learnings used
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7169
File: src/platform/remote/comfyui/jobs/jobTypes.ts:1-107
Timestamp: 2025-12-09T03:39:54.501Z
Learning: In the ComfyUI_frontend project, Zod is on v3.x. Do not suggest Zod v4 standalone validators (z.uuid, z.ulid, z.cuid2, z.nanoid) until an upgrade to Zod 4 is performed. When reviewing TypeScript files (e.g., src/platform/remote/comfyui/jobs/jobTypes.ts) validate against Zod 3 capabilities and avoid introducing v4-specific features; flag any proposal to upgrade or incorporate v4-only validators and propose staying with compatible 3.x patterns.
Learnt from: christian-byrne
Repo: Comfy-Org/ComfyUI_frontend PR: 7358
File: src/components/dialog/content/signin/SignUpForm.vue:45-54
Timestamp: 2025-12-11T12:25:15.470Z
Learning: This repository uses CI automation to format code (pnpm format). Do not include manual formatting suggestions in code reviews for Comfy-Org/ComfyUI_frontend. If formatting issues are detected, rely on the CI formatter or re-run pnpm format. Focus reviews on correctness, readability, performance, accessibility, and maintainability rather than style formatting.
Learnt from: christian-byrne
Repo: Comfy-Org/ComfyUI_frontend PR: 7416
File: src/stores/imagePreviewStore.ts:5-7
Timestamp: 2025-12-13T11:03:11.264Z
Learning: In the ComfyUI_frontend repository, lint rules require keeping 'import type' statements separate from non-type imports, even if importing from the same module. Do not suggest consolidating them into a single import statement. Ensure type imports remain on their own line (import type { ... } from 'module') and regular imports stay on separate lines.
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7537
File: src/components/ui/button/Button.stories.ts:45-55
Timestamp: 2025-12-17T00:40:09.635Z
Learning: Prefer pure function declarations over function expressions (e.g., use function foo() { ... } instead of const foo = () => { ... }) for pure functions in the repository. Function declarations are more functional-leaning, offer better hoisting clarity, and can improve readability and tooling consistency. Apply this guideline across TypeScript files in Comfy-Org/ComfyUI_frontend, including story and UI component code, except where a function expression is semantically required (e.g., callbacks, higher-order functions with closures).
Learnt from: kaili-yang
Repo: Comfy-Org/ComfyUI_frontend PR: 7805
File: src/composables/useCoreCommands.ts:439-439
Timestamp: 2025-12-30T22:22:33.836Z
Learning: When accessing reactive properties from Pinia stores in TypeScript files, avoid using .value on direct property access (e.g., useStore().isOverlayExpanded). Pinia auto-wraps refs when accessed directly, returning the primitive value. The .value accessor is only needed when destructuring store properties or when using storeToRefs().
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 8248
File: src/platform/assets/schemas/assetSchema.ts:109-118
Timestamp: 2026-01-22T23:44:59.888Z
Learning: Do not export declarations (e.g., schemas, constants, types) unless they are actually used elsewhere in the codebase. Keep the public API surface minimal by exporting only items with concrete usage. This reduces coupling and keeps imports explicit. In src/platform/assets/schemas/assetSchema.ts, ensure only necessary exports are exposed; remove unused exports or switch to non-exported/internal where appropriate.
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 8722
File: src/composables/maskeditor/useBrushDrawing.ts:5-5
Timestamp: 2026-02-08T03:19:12.009Z
Learning: Treat unresolved import warnings caused by pnpm workspace catalog mappings (pnpm-workspace.yaml) and unplugin-typegpu in vite.config.mts as false positives. Do not flag these in PR reviews; they are expected in sandboxes without node_modules and are not indicative of actual missing dependencies in the TS codebase.
Learnt from: christian-byrne
Repo: Comfy-Org/ComfyUI_frontend PR: 8966
File: src/extensions/core/load3d.ts:427-430
Timestamp: 2026-02-19T02:06:23.468Z
Learning: In TypeScript, you can use typeof SomeClass in type annotation positions (e.g., param: typeof LGraphNode) even when SomeClass is imported with import type. This is a type query that only exists at compile time and is erased in runtime, so it is safe to combine with type-only imports. Apply this pattern to TS files broadly when you need a type that references the shape of a class or constructor function without importing the value at runtime.
Learnt from: christian-byrne
Repo: Comfy-Org/ComfyUI_frontend PR: 8966
File: src/extensions/core/uploadAudio.ts:91-94
Timestamp: 2026-02-19T02:06:38.395Z
Learning: In TypeScript files, you can use a type annotation like 'nodeType: typeof MyClass' even if MyClass is imported via 'import type'. Both the type-only import and 'typeof' operate at the type level and are erased at compile time. This pattern is commonly used for constructor types (e.g., 'nodeType: typeof LGraphNode'). Apply this pattern across TypeScript files in the repository (src/**/*.ts) as appropriate, ensuring the imported symbol is a type-only import when possible for clarity and to avoid runtime imports.
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 8992
File: src/lib/litegraph/src/widgets/GradientSliderWidget.ts:18-18
Timestamp: 2026-02-20T21:08:19.814Z
Learning: When drawing with CanvasRenderingContext2D in TypeScript/JavaScript, wrap the drawing logic with ctx.save() at the start and ctx.restore() at the end to preserve and restore the canvas state. Do not manually destructure and restore individual properties (e.g., fillStyle, strokeStyle); rely on save/restore to manage state changes in a scoped manner. This should be applied to all TS files that perform canvas drawing.
Learnt from: dante01yoon
Repo: Comfy-Org/ComfyUI_frontend PR: 9075
File: src/scripts/api.featureFlags.test.ts:237-268
Timestamp: 2026-02-22T04:27:33.379Z
Learning: In Vite/Vitest, import.meta.env.DEV is true for any mode that is not 'production' (i.e., DEV is the opposite of PROD, and can be true in 'test', 'development', etc.). Do not assume DEV implies only 'development' mode. When reviewing code and tests, treat DEV as a non-production flag and verify environment-specific logic accordingly. Reference: https://vite.dev/guide/env-and-mode#modes
Learnt from: christian-byrne
Repo: Comfy-Org/ComfyUI_frontend PR: 9427
File: src/renderer/extensions/vueNodes/widgets/components/form/dropdown/FormDropdownMenuFilter.vue:33-33
Timestamp: 2026-03-06T00:53:28.835Z
Learning: When reviewing code, note that the enforce-canonical-classes (better-tailwindcss) rule may auto-fix Tailwind v3 !class-name syntax by converting it to v4 class-name! syntax. Do not treat these auto-fixed class-name! instances as newly introduced issues; the perceived change is in syntax placement, not in usage or intent. This guidance applies across all .vue and .ts files in the repository.
Learnt from: sonnybox
Repo: Comfy-Org/ComfyUI_frontend PR: 9446
File: src/renderer/extensions/vueNodes/widgets/components/WidgetTextarea.vue:45-45
Timestamp: 2026-03-06T01:55:00.013Z
Learning: Treat wrap-break-word as a valid Tailwind CSS utility for overflow-wrap: break-word in Tailwind v4+ projects. Do not flag this class as invalid in any Vue (.vue) or TypeScript (.ts/.tsx) files within the repository (e.g., Comfy-Org/ComfyUI_frontend) or other Tailwind v4+ projects. When reviewing, verify that the class is used to enable word breaking in long text content and reference the Tailwind docs: https://tailwindcss.com/docs/overflow-wrap.
Learnt from: christian-byrne
Repo: Comfy-Org/ComfyUI_frontend PR: 9554
File: browser_tests/fixtures/ComfyPage.ts:189-193
Timestamp: 2026-03-12T10:03:55.383Z
Learning: In browser_tests, seed localStorage-based feature flags at init time using FeatureFlagHelper.seedFlags() (which uses page.addInitScript) before any navigation. Ensure this happens before comfyPage.setup() is called. The comfyPageFixture currently yields after calling comfyPage.setup(), so do not defer setup() from the fixture or add a pre-setup hook, as changing this would break existing tests.
Learnt from: Yourz
Repo: Comfy-Org/ComfyUI_frontend PR: 10310
File: browser_tests/tests/topbarMenuDismissal.spec.ts:0-0
Timestamp: 2026-03-19T16:16:40.350Z
Learning: In Comfy-Org/ComfyUI_frontend, when reviewing changes limited to adding/revising `:modal="false"` (or similar modal/non-modal behavior props) on reakit-ui/reka-ui primitives such as `ContextMenuRoot` and `DropdownMenuRoot`, do not request Playwright/E2E regression coverage solely for that prop-level change. The dismissal behavior for modal vs non-modal is documented and tested upstream in the reka-ui library; only ask for E2E tests when the change affects ComfyUI_frontend behavior beyond these primitive prop wiring differences (e.g., custom logic, integration behavior, or new user flows not covered upstream).
Learnt from: jaeone94
Repo: Comfy-Org/ComfyUI_frontend PR: 10741
File: browser_tests/fixtures/helpers/boundsUtils.ts:32-45
Timestamp: 2026-03-31T12:17:31.232Z
Learning: In this codebase’s Playwright E2E test helpers under `browser_tests/fixtures/helpers/`, prefer measuring collapsed node dimensions directly from the DOM using `offsetWidth`/`offsetHeight`. Do not access Pinia stores from within `page.evaluate` blocks in E2E helpers (e.g., avoid calling `getNodeCollapsedSize()` or reading from `layoutStore`). This is intentional: E2E helpers should validate the final user-visible result, while Pinia store synchronization correctness is expected to be covered by unit tests.
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 10958
File: browser_tests/tests/bottomPanelLogs.spec.ts:4-4
Timestamp: 2026-04-08T18:09:35.787Z
Learning: In this repo’s E2E browser tests, do not flag `import-x/no-unresolved` for imports using the `e2e/*` path alias. CodeRabbit sandbox reports these as unresolved because it may not run a full `pnpm install`; `e2e/*` is defined in the root `tsconfig.json` and the related `apps/website/tsconfig.json`/workspace dependency resolution only works when dependencies are installed. Apply this only to `browser_tests/**/*.ts` files.
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 10958
File: browser_tests/tests/subgraph/subgraphSearch.spec.ts:3-4
Timestamp: 2026-04-08T18:09:50.771Z
Learning: In Comfy-Org/ComfyUI_frontend browser tests (browser_tests/**/*.ts), do not report `import-x/no-unresolved` for imports using the `e2e/*` alias. The alias is defined in the root tsconfig.json as `"e2e/*": ["./browser_tests/*"]`, and eslint.config.ts includes the correct tsconfig `project` list; lint passes locally and in CI. Treat any `e2e/*` unresolved-import findings in the CodeRabbit sandbox as false positives.
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 10958
File: browser_tests/tests/rightClickMenu.spec.ts:3-5
Timestamp: 2026-04-08T18:10:12.748Z
Learning: In Comfy-Org/ComfyUI_frontend, `import-x/no-unresolved` errors caused by `@/` (src alias) and `e2e/` (browser_tests alias) imports inside `browser_tests/**/*.ts` are false positives in CodeRabbit’s sandbox because the sandbox cannot resolve the root `tsconfig.json` (it extends `astro/tsconfigs/strict`), which breaks the TypeScript import resolver. Since ESLint passes clean locally and in CI, do not treat these specific `@/` and `e2e/` unresolved import findings as real issues, and do not recommend adding `browser_tests/tsconfig.json` (or similar) to ESLint/CodeRabbit resolver project lists for this case.
Learnt from: kaili-yang
Repo: Comfy-Org/ComfyUI_frontend PR: 11531
File: src/composables/maskeditor/brushDrawingUtils.ts:14-14
Timestamp: 2026-04-22T04:06:01.353Z
Learning: In the Comfy-Org/ComfyUI_frontend codebase, `knip` is used to detect unused exports. When reviewing TypeScript/TSX code, avoid recommending or introducing exported types/functions/constants that have no concrete external consumers (e.g., they’re only used within the same module). Only recommend exporting when there is an actual external usage outside the module. If something might be needed externally later, keep it non-exported for now and defer exporting until an external consumer is added.
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 12197
File: src/renderer/extensions/linearMode/AppInput.vue:23-23
Timestamp: 2026-05-14T21:10:45.382Z
Learning: In the ComfyUI_frontend repo, `WidgetEntityId` is a branded template-literal type (e.g., `${UUID}:${NodeId}:${string}`) and should be treated as structurally non-empty when valid—so an empty string is invalid and must be treated as “absent”. When handling optional `entityId?: WidgetEntityId` values (e.g., for Vue props or TS function parameters), guard missing/invalid IDs with a truthy check (e.g., `if (!entityId) return`) rather than `if (entityId === undefined)`. Rationale: `=== undefined` would allow empty-string values through into stores such as `appModeStore.selectedInputs`, creating rows that can’t resolve back to a widget.
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 12197
File: src/core/schemas/parseNodePropertyArray.ts:1-1
Timestamp: 2026-05-18T21:34:41.153Z
Learning: In TypeScript, it is correct to use `import type { z } from 'zod'` when the imported symbol `z` is used exclusively in type-annotation/type-only positions (e.g., `schema: z.ZodType<T[]>`, `z.infer<typeof schema>`, `type X = z.AnyZodObject`, etc.). Because `import type` is fully erased at compile time and requires no runtime value, code reviewers should not flag `import type { z }` as an error (e.g., as an unused value import or incorrect runtime import) in TypeScript/React projects when `z` is only referenced in type positions. If `z` is referenced in a value/runtime position, it should instead be imported with a normal `import { z } from 'zod'`.
Contributor
|
Follow-up observation (non-blocking, not specific to this diff): The It would only matter if the page were reused across tests (worker/context-scoped) or if This is an established convention across several helpers ( No change requested here; approval stands. |
This was referenced Jun 19, 2026
Open
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
Stabilizes the cloud template filtering E2E by making the test own both startup asset API responses and the complete template universe it asserts against.
Changes
createCloudAssetsFixture([])fixture so startup/api/assetscalls do not create an unrelated error toast that can intercept the Clear Filters click.TemplateHelper.mockIndex()to also mock/api/workflow_templatesas an empty custom-template map, so tests that configure a core template index do not accidentally include custom templates from the local backend.Review Focus
Please focus on fixture ownership. This spec asserts exact template counts, so
templateApi.mockIndex()should isolate the core template index and the custom workflow-template endpoint together. The asset fixture change is intentionally scoped to this cloud spec and reuses existing infrastructure instead of dismissing arbitrary toasts or forcing clicks.Verification:
pnpm exec oxfmt --check browser_tests/tests/templateFilteringCount.spec.ts browser_tests/fixtures/helpers/TemplateHelper.ts browser_tests/fixtures/helpers/WorkflowHelper.tspnpm exec eslint browser_tests/tests/templateFilteringCount.spec.ts browser_tests/fixtures/helpers/TemplateHelper.ts browser_tests/fixtures/helpers/WorkflowHelper.tspnpm typecheck:browserPLAYWRIGHT_LOCAL=1 PLAYWRIGHT_TEST_URL=http://localhost:8188 PLAYWRIGHT_SETUP_API_URL=http://localhost:8188 pnpm exec playwright test browser_tests/tests/templateFilteringCount.spec.ts --grep "clear filters button resets" --project=cloud --repeat-each=5PLAYWRIGHT_LOCAL=1 PLAYWRIGHT_TEST_URL=http://localhost:8188 PLAYWRIGHT_SETUP_API_URL=http://localhost:8188 pnpm exec playwright test browser_tests/tests/templateFilteringCount.spec.ts --project=cloudScreenshots (if applicable)
N/A