fix: keep Preview as Text rendering when the output payload has no text - #14073
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
Included review availability: 0 reviews are currently available. Based on recent review activity, included reviews refill at 3 per hour. 📝 WalkthroughWalkthroughThe preview update API now normalizes nullable, array, numeric, and structured execution output values. Unit and browser tests cover rendering, empty output, null filtering, recovery, and named preview widgets. ChangesText preview normalization
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: 🔵 Low · up to The change prevents Preview as Text from crashing or rendering blank for null, missing, mixed, and numeric payload values, with broad regression coverage. It is mergeable with owner awareness that an explicit empty-array test is not shown in the supplied current-head evidence. Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 inconclusive)
✅ Passed checks (6 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
🎨 Storybook: ✅ Built — View Storybook🎭 Playwright: ✅ 1825 passed, 0 failed · 5 flaky📊 Browser Reports
📦 Bundle: 8.86 MB gzip 🔴 +152 BDetailsSummary
Category Glance App Entry Points — 3.71 kB (baseline 3.71 kB) • ⚪ 0 BMain entry bundles and manifests
Status: 1 added / 1 removed Graph Workspace — 1.37 MB (baseline 1.37 MB) • ⚪ 0 BGraph editor runtime, canvas, workflow orchestration
Status: 2 added / 2 removed / 1 unchanged Views & Navigation — 124 kB (baseline 124 kB) • ⚪ 0 BTop-level views, pages, and routed surfaces
Status: 13 added / 13 removed / 4 unchanged Panels & Settings — 566 kB (baseline 566 kB) • ⚪ 0 BConfiguration panels, inspectors, and settings screens
Status: 10 added / 10 removed / 16 unchanged User & Accounts — 27.7 kB (baseline 27.7 kB) • ⚪ 0 BAuthentication, profile, and account management bundles
Status: 6 added / 6 removed / 5 unchanged Editors & Dialogs — 125 kB (baseline 125 kB) • ⚪ 0 BModals, dialogs, drawers, and in-app editors
Status: 7 added / 7 removed / 1 unchanged UI Components — 67.1 kB (baseline 67.1 kB) • ⚪ 0 BReusable component library chunks
Status: 6 added / 6 removed / 8 unchanged Data & Services — 3.52 MB (baseline 3.52 MB) • ⚪ 0 BStores, services, APIs, and repositories
Status: 14 added / 14 removed / 3 unchanged Utilities & Hooks — 550 kB (baseline 550 kB) • ⚪ 0 BHelpers, composables, and utility bundles
Status: 18 added / 18 removed / 20 unchanged Vendor & Third-Party — 16.8 MB (baseline 16.8 MB) • ⚪ 0 BExternal libraries and shared vendor chunks Status: 18 unchanged Other — 14.2 MB (baseline 14.2 MB) • 🔴 +116 BBundles that do not match a named category
Status: 68 added / 68 removed / 217 unchanged ⚡ Performance Report
Show regressions
All metrics
Historical variance (last 15 runs)
Trend (last 15 commits on main)
Raw data{
"timestamp": "2026-08-18T21:21:27.181Z",
"gitSha": "829a33f6b2fd9b05b5f77a12e50900fee583892a",
"branch": "fix/fe-685-previewany-null-safety",
"measurements": [
{
"name": "canvas-idle",
"durationMs": 2138.420999999994,
"styleRecalcs": 7,
"styleRecalcDurationMs": 7.184999999999999,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 608.1229999999999,
"heapDeltaBytes": -25932,
"heapUsedBytes": 60289520,
"domNodes": -283,
"jsHeapTotalBytes": 4972544,
"scriptDurationMs": 8.292000000000002,
"eventListeners": -151,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "canvas-idle",
"durationMs": 2068.0609999999433,
"styleRecalcs": 8,
"styleRecalcDurationMs": 7.778999999999999,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 522.028,
"heapDeltaBytes": -2828568,
"heapUsedBytes": 58227996,
"domNodes": -283,
"jsHeapTotalBytes": 5496832,
"scriptDurationMs": 8.249999999999998,
"eventListeners": -149,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.699999999999818
},
{
"name": "canvas-mouse-sweep",
"durationMs": 1881.732999999997,
"styleRecalcs": 73,
"styleRecalcDurationMs": 39.003,
"layouts": 12,
"layoutDurationMs": 3.513,
"taskDurationMs": 879.9840000000002,
"heapDeltaBytes": 368360,
"heapUsedBytes": 60903920,
"domNodes": -282,
"jsHeapTotalBytes": 4448256,
"scriptDurationMs": 110.773,
"eventListeners": -149,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333335,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "canvas-mouse-sweep",
"durationMs": 1835.472999999979,
"styleRecalcs": 71,
"styleRecalcDurationMs": 34.421,
"layouts": 12,
"layoutDurationMs": 3.258,
"taskDurationMs": 864.178,
"heapDeltaBytes": 13512424,
"heapUsedBytes": 73961880,
"domNodes": -283,
"jsHeapTotalBytes": 4972544,
"scriptDurationMs": 105.54100000000001,
"eventListeners": -183,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333332,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "canvas-zoom-sweep",
"durationMs": 1730.2360000000476,
"styleRecalcs": 31,
"styleRecalcDurationMs": 16.605999999999998,
"layouts": 6,
"layoutDurationMs": 0.6880000000000001,
"taskDurationMs": 354.105,
"heapDeltaBytes": 2792528,
"heapUsedBytes": 63155748,
"domNodes": 76,
"jsHeapTotalBytes": 4980736,
"scriptDurationMs": 9.883999999999999,
"eventListeners": 19,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.699999999999818
},
{
"name": "canvas-zoom-sweep",
"durationMs": 1719.920000000002,
"styleRecalcs": 32,
"styleRecalcDurationMs": 17.747,
"layouts": 6,
"layoutDurationMs": 0.631,
"taskDurationMs": 357.544,
"heapDeltaBytes": 2821484,
"heapUsedBytes": 63246296,
"domNodes": 78,
"jsHeapTotalBytes": 5242880,
"scriptDurationMs": 10.109,
"eventListeners": 19,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.799999999999272
},
{
"name": "dom-widget-clipping",
"durationMs": 563.603999999998,
"styleRecalcs": 10,
"styleRecalcDurationMs": 6.753999999999998,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 347.96000000000004,
"heapDeltaBytes": 10751144,
"heapUsedBytes": 71179712,
"domNodes": 16,
"jsHeapTotalBytes": 4980736,
"scriptDurationMs": 52.470000000000006,
"eventListeners": 2,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333332,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "dom-widget-clipping",
"durationMs": 579.845999999975,
"styleRecalcs": 11,
"styleRecalcDurationMs": 6.976,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 359.17299999999994,
"heapDeltaBytes": 10403244,
"heapUsedBytes": 70787032,
"domNodes": 18,
"jsHeapTotalBytes": 4980736,
"scriptDurationMs": 52.242999999999995,
"eventListeners": 2,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "large-graph-idle",
"durationMs": 2065.27200000005,
"styleRecalcs": 8,
"styleRecalcDurationMs": 6.909000000000002,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 606.6279999999999,
"heapDeltaBytes": 2846276,
"heapUsedBytes": 76812860,
"domNodes": -262,
"jsHeapTotalBytes": -2363392,
"scriptDurationMs": 15.592999999999996,
"eventListeners": -147,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "large-graph-idle",
"durationMs": 2039.6779999999808,
"styleRecalcs": 9,
"styleRecalcDurationMs": 8.745999999999997,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 621.4730000000001,
"heapDeltaBytes": 2341356,
"heapUsedBytes": 76667660,
"domNodes": -261,
"jsHeapTotalBytes": -2101248,
"scriptDurationMs": 16.507,
"eventListeners": -151,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "large-graph-pan",
"durationMs": 2154.075999999975,
"styleRecalcs": 68,
"styleRecalcDurationMs": 15.572,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 1167.97,
"heapDeltaBytes": 10888732,
"heapUsedBytes": 84878652,
"domNodes": -283,
"jsHeapTotalBytes": 4939776,
"scriptDurationMs": 315.11400000000003,
"eventListeners": -179,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "large-graph-pan",
"durationMs": 2080.4709999999886,
"styleRecalcs": 65,
"styleRecalcDurationMs": 10.979,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 1110.1080000000002,
"heapDeltaBytes": 17381972,
"heapUsedBytes": 78685024,
"domNodes": -273,
"jsHeapTotalBytes": 225280,
"scriptDurationMs": 305.521,
"eventListeners": 4,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "large-graph-zoom",
"durationMs": 3128.0849999999987,
"styleRecalcs": 64,
"styleRecalcDurationMs": 13.806,
"layouts": 60,
"layoutDurationMs": 7.938999999999998,
"taskDurationMs": 1342.072,
"heapDeltaBytes": 15494424,
"heapUsedBytes": 77227732,
"domNodes": -272,
"jsHeapTotalBytes": 0,
"scriptDurationMs": 381.532,
"eventListeners": -147,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.799999999999272
},
{
"name": "large-graph-zoom",
"durationMs": 3171.9799999999623,
"styleRecalcs": 63,
"styleRecalcDurationMs": 14.051000000000004,
"layouts": 60,
"layoutDurationMs": 8.393,
"taskDurationMs": 1379.313,
"heapDeltaBytes": 14744760,
"heapUsedBytes": 77017656,
"domNodes": -273,
"jsHeapTotalBytes": 0,
"scriptDurationMs": 398.106,
"eventListeners": 8,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333335,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "legacy-node-drag",
"durationMs": 2424.443999999994,
"styleRecalcs": 45,
"styleRecalcDurationMs": 12.215,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 1508.4520000000002,
"heapDeltaBytes": -18928936,
"heapUsedBytes": 63801920,
"domNodes": -248,
"jsHeapTotalBytes": 6619136,
"scriptDurationMs": 481.92799999999994,
"eventListeners": 33,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "legacy-node-drag",
"durationMs": 2393.389999999954,
"styleRecalcs": 46,
"styleRecalcDurationMs": 17.326999999999998,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 1480.804,
"heapDeltaBytes": -19632804,
"heapUsedBytes": 63001252,
"domNodes": -255,
"jsHeapTotalBytes": 8716288,
"scriptDurationMs": 477.643,
"eventListeners": 33,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333335,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "minimap-idle",
"durationMs": 2052.1100000000274,
"styleRecalcs": 8,
"styleRecalcDurationMs": 9.413000000000002,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 606.4100000000001,
"heapDeltaBytes": -4229720,
"heapUsedBytes": 76409024,
"domNodes": -267,
"jsHeapTotalBytes": 3473408,
"scriptDurationMs": 15.096000000000002,
"eventListeners": -147,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "minimap-idle",
"durationMs": 2049.4900000001053,
"styleRecalcs": 7,
"styleRecalcDurationMs": 6.228999999999998,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 602.603,
"heapDeltaBytes": -3891692,
"heapUsedBytes": 76912412,
"domNodes": -268,
"jsHeapTotalBytes": 3473408,
"scriptDurationMs": 22.362999999999996,
"eventListeners": -147,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "subgraph-dom-widget-clipping",
"durationMs": 577.7659999999969,
"styleRecalcs": 46,
"styleRecalcDurationMs": 9.605999999999998,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 382.549,
"heapDeltaBytes": 11408232,
"heapUsedBytes": 72107512,
"domNodes": 18,
"jsHeapTotalBytes": 5242880,
"scriptDurationMs": 117.33000000000001,
"eventListeners": 8,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333332,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "subgraph-dom-widget-clipping",
"durationMs": 576.4279999999644,
"styleRecalcs": 47,
"styleRecalcDurationMs": 11.202,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 387.718,
"heapDeltaBytes": 11878012,
"heapUsedBytes": 72692528,
"domNodes": 20,
"jsHeapTotalBytes": 6029312,
"scriptDurationMs": 121.03100000000002,
"eventListeners": 8,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333335,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "subgraph-idle",
"durationMs": 2013.2950000000278,
"styleRecalcs": 9,
"styleRecalcDurationMs": 8.000000000000002,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 490.62899999999996,
"heapDeltaBytes": 10960776,
"heapUsedBytes": 71817720,
"domNodes": -280,
"jsHeapTotalBytes": 4710400,
"scriptDurationMs": 6.8249999999999975,
"eventListeners": -183,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.699999999999818
},
{
"name": "subgraph-idle",
"durationMs": 2008.3419999999705,
"styleRecalcs": 9,
"styleRecalcDurationMs": 8.690999999999999,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 503.668,
"heapDeltaBytes": -340804,
"heapUsedBytes": 60395356,
"domNodes": -280,
"jsHeapTotalBytes": 3923968,
"scriptDurationMs": 7.845999999999999,
"eventListeners": -149,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "subgraph-mouse-sweep",
"durationMs": 1727.6640000000043,
"styleRecalcs": 75,
"styleRecalcDurationMs": 36.263,
"layouts": 16,
"layoutDurationMs": 4.782,
"taskDurationMs": 801.0649999999999,
"heapDeltaBytes": -2777732,
"heapUsedBytes": 57946160,
"domNodes": -280,
"jsHeapTotalBytes": 5234688,
"scriptDurationMs": 86.316,
"eventListeners": -151,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "subgraph-mouse-sweep",
"durationMs": 1707.763,
"styleRecalcs": 75,
"styleRecalcDurationMs": 35.768,
"layouts": 16,
"layoutDurationMs": 4.5760000000000005,
"taskDurationMs": 801.046,
"heapDeltaBytes": -945756,
"heapUsedBytes": 59634236,
"domNodes": -281,
"jsHeapTotalBytes": 5234688,
"scriptDurationMs": 85.32799999999999,
"eventListeners": -151,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.670000000000012,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "subgraph-transition-enter",
"durationMs": 1401.2749999999983,
"styleRecalcs": 18,
"styleRecalcDurationMs": 30.908999999999992,
"layouts": 13,
"layoutDurationMs": 13.124000000000002,
"taskDurationMs": 923.8679999999999,
"heapDeltaBytes": 23749776,
"heapUsedBytes": 99614372,
"domNodes": 13673,
"jsHeapTotalBytes": 16515072,
"scriptDurationMs": 17.33300000000001,
"eventListeners": 2375,
"totalBlockingTimeMs": 138,
"frameDurationMs": 16.666666666666636,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "viewport-pan-sweep",
"durationMs": 8309.55300000005,
"styleRecalcs": 250,
"styleRecalcDurationMs": 42.29900000000001,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 4125.395,
"heapDeltaBytes": 9205904,
"heapUsedBytes": 83391336,
"domNodes": -263,
"jsHeapTotalBytes": -827392,
"scriptDurationMs": 993.2750000000001,
"eventListeners": -131,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.799999999999272
},
{
"name": "viewport-pan-sweep",
"durationMs": 8233.022000000004,
"styleRecalcs": 250,
"styleRecalcDurationMs": 43.059,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 4111.445,
"heapDeltaBytes": 8299672,
"heapUsedBytes": 82270892,
"domNodes": -235,
"jsHeapTotalBytes": -303104,
"scriptDurationMs": 983.125,
"eventListeners": -131,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.799999999999272
},
{
"name": "vue-large-graph-idle",
"durationMs": 18716.62900000001,
"styleRecalcs": 0,
"styleRecalcDurationMs": 0,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 17702.883,
"heapDeltaBytes": -40687340,
"heapUsedBytes": 178754780,
"domNodes": -8312,
"jsHeapTotalBytes": -5443584,
"scriptDurationMs": 131.93500000000003,
"eventListeners": -16389,
"totalBlockingTimeMs": 0,
"frameDurationMs": 17.776666666666763,
"p95FrameDurationMs": 16.799999999999272
},
{
"name": "vue-large-graph-idle",
"durationMs": 18526.796999999988,
"styleRecalcs": 0,
"styleRecalcDurationMs": 0,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 17484.322,
"heapDeltaBytes": -42226212,
"heapUsedBytes": 177799996,
"domNodes": -8312,
"jsHeapTotalBytes": -7245824,
"scriptDurationMs": 115.775,
"eventListeners": -16387,
"totalBlockingTimeMs": 0,
"frameDurationMs": 17.77333333333336,
"p95FrameDurationMs": 16.80000000000291
},
{
"name": "vue-large-graph-pan",
"durationMs": 22632.810000000005,
"styleRecalcs": 172,
"styleRecalcDurationMs": 22.08899999999997,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 22022.874000000003,
"heapDeltaBytes": -21631780,
"heapUsedBytes": 183941512,
"domNodes": -8316,
"jsHeapTotalBytes": -14168064,
"scriptDurationMs": 427.62199999999996,
"eventListeners": -16379,
"totalBlockingTimeMs": 48,
"frameDurationMs": 18.330000000000048,
"p95FrameDurationMs": 16.799999999999272
},
{
"name": "vue-large-graph-pan",
"durationMs": 22278.65499999996,
"styleRecalcs": 173,
"styleRecalcDurationMs": 21.496000000000016,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 21700.760000000002,
"heapDeltaBytes": -22873916,
"heapUsedBytes": 183983004,
"domNodes": -8312,
"jsHeapTotalBytes": -13381632,
"scriptDurationMs": 509.21000000000004,
"eventListeners": -16385,
"totalBlockingTimeMs": 215,
"frameDurationMs": 18.333333333333332,
"p95FrameDurationMs": 16.799999999999272
},
{
"name": "workflow-execution",
"durationMs": 462.1309999999994,
"styleRecalcs": 13,
"styleRecalcDurationMs": 19.217,
"layouts": 2,
"layoutDurationMs": 0.446,
"taskDurationMs": 103.877,
"heapDeltaBytes": 4950660,
"heapUsedBytes": 65494356,
"domNodes": 124,
"jsHeapTotalBytes": 262144,
"scriptDurationMs": 7.866000000000001,
"eventListeners": 99,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.699999999999818
},
{
"name": "workflow-execution",
"durationMs": 113.62099999996644,
"styleRecalcs": 8,
"styleRecalcDurationMs": 15.691,
"layouts": 3,
"layoutDurationMs": 1.226,
"taskDurationMs": 71.51299999999999,
"heapDeltaBytes": 2891276,
"heapUsedBytes": 63548132,
"domNodes": 127,
"jsHeapTotalBytes": 0,
"scriptDurationMs": 3.8909999999999987,
"eventListeners": 25,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333335,
"p95FrameDurationMs": 16.700000000000728
}
]
} |
Codecov Report✅ All modified and coverable lines are covered by tests. @@ Coverage Diff @@
## main #14073 +/- ##
==========================================
+ Coverage 78.99% 81.49% +2.49%
==========================================
Files 2209 1882 -327
Lines 114923 110428 -4495
Branches 35275 33792 -1483
==========================================
- Hits 90788 89994 -794
+ Misses 23665 20056 -3609
+ Partials 470 378 -92
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 350 files with indirect coverage changes 🚀 New features to boost your workflow:
|
jaeone94
left a comment
There was a problem hiding this comment.
Reviewed the diff, then re-ran the new suite against the pre-fix implementation (git checkout 335adad^ -- src/extensions/core/textPreviewWidgets.ts, then vitest) to see which tests actually discriminate. Numbers below come from that run.
The production change looks right to me. Normalizing in this consumer is proportionate — output.text has exactly one reader in the tree — and the parameter widening is contravariant, so nothing breaks for callers of the window.comfyAPI.textPreviewWidgets shim. I also want to explicitly agree with the Review Focus note: shallow String() over JSON.stringify is the right call, and I am not asking for object handling that nobody has reported.
My substantive concern is the test suite. 11 of the 15 new unit tests, and 100% of the new e2e spec, pass against the code this commit replaces. The four that fail — null message, undefined message, [null, null], ['first', null, 'second'] — carry the entire regression story. Details inline, along with a question about what the numeric payload actually looked like, since that determines whether one of the claimed failure modes existed at all.
Out of scope and not this PR's job, but worth a ticket: src/scripts/api.ts:843 casts unvalidated WebSocket JSON with JSON.parse(event.data) as ApiMessageUnion, while zExecutedWsMessage declares output: zOutputs and is never parsed on that path. Every consumer downstream of that cast is exposed to the same class of payload this PR is defending against.
| function toPreviewText(text: unknown): string { | ||
| if (typeof text === 'string') return text | ||
| if (text == null) return '' | ||
| if (Array.isArray(text)) | ||
| return text | ||
| .filter((part) => part != null) | ||
| .map((part) => (typeof part === 'string' ? part : String(part))) | ||
| .join('\n\n') | ||
| return String(text) | ||
| } |
There was a problem hiding this comment.
Suggestion: two of the four branches are no-ops, and dropping them makes the contract easier to read.
Array.prototype.join already calls String() on every non-nullish element, so the .map(...) is identity once .filter(...) has run. And String(s) === s for strings, so the typeof text === 'string' fast path returns exactly what the trailing return String(text) would.
function toPreviewText(text: unknown): string {
if (text == null) return ''
if (Array.isArray(text)) return text.filter((part) => part != null).join('\n\n')
return String(text)
}I diffed both versions over 19 inputs — nullish, [], [null], ['a', null, 'b'], [30, 23.976], bare scalars, objects, nested arrays — with no divergence. The .filter is the load-bearing piece; deleting it fails two tests.
Side benefit: return String(text) becomes the path strings take, so it stops being the line Codecov flags as uncovered.
There was a problem hiding this comment.
✅ Applied in 731afa2. Dropped the typeof text === 'string' fast path and the .map() - both were no-ops as you showed. Three branches, identical behavior, and return String(text) is now the string path so the Codecov flag is gone too.
| export function updateTextPreviewWidgets( | ||
| node: LGraphNode, | ||
| message: { text?: string | string[] } | ||
| message: { text?: string | string[] } | null | undefined |
There was a problem hiding this comment.
Suggestion: this is a hand-written spelling of a type that already has a name, and the nullable form has precedent for exactly this wire data — src/stores/resultItemParsing.ts:32 and src/renderer/extensions/linearMode/flattenNodeOutput.ts:7 both type it as NodeExecutionOutput | null | undefined.
import type { NodeExecutionOutput } from '@/schemas/apiSchema'
export function updateTextPreviewWidgets(
node: LGraphNode,
message: NodeExecutionOutput | null | undefined
) {Both call sites already pass NodeExecutionOutput (previewAny.ts:33 via the onExecuted augmentation, previewAny.ts:39 via onNodeOutputsUpdated), so nothing at the call sites changes. I applied it locally: pnpm typecheck is clean and the 18 tests still pass.
The as unknown as casts in the test file survive that change, and I think that is worth noticing rather than working around — zOutputs.text is declared string | string[] while the premise of this PR is that it can be neither. The casts are the schema and reality disagreeing out loud.
There was a problem hiding this comment.
✅ Applied in 231dea6. Switched to NodeExecutionOutput | null | undefined - matches the pattern in resultItemParsing.ts and flattenNodeOutput.ts as you noted. The casts in the test file survive the change as expected.
| it.each([ | ||
| ['compact JSON', '{"name":"Comfy","emoji":"🌟"}'], | ||
| [ | ||
| 'pretty JSON', | ||
| '{\n "name": "Comfy",\n "arr": [\n 1,\n 2\n ]\n}' | ||
| ], | ||
| ['markdown-fenced JSON', '```json\n{"name":"Comfy"}\n```'], | ||
| ['JSON array', '[{"a": 1}, {"b": 2}]'], | ||
| ['non-ASCII text', '你好,世界。'], | ||
| ['prompt with trailing space', '"A red car" is a great prompt. '], | ||
| ['prompt ending in a quoted period', "ending in 'best quality.'"] | ||
| ])('renders %s verbatim', (_label, text) => { | ||
| updateTextPreviewWidgets(node, { text }) | ||
| expect(node.widgets[0].value).toBe(text) | ||
| }) |
There was a problem hiding this comment.
Issue: these seven rows all exercise one branch — if (typeof text === 'string') return text. The module does no parsing, trimming or escaping, so compact JSON, pretty JSON, fenced JSON, a JSON array, non-ASCII, a trailing space and a quoted period are seven spellings of the same identity assertion, and writes a plain string message as-is at line 99 already covers it. All seven stay green against the pre-fix implementation.
They also imply a guarantee this module cannot give. The payloads listed can only be mangled on the render side — renderMarkdownToHtml in WidgetTextPreview.vue — where WidgetTextPreview.test.ts already has a working harness. Fenced JSON through markdown rendering is a real test; fenced JSON through x => x is not.
Suggest dropping the block, keeping at most one row, and spending the budget on the branch nothing currently reaches (see my question on the numeric case above).
There was a problem hiding this comment.
✅ Applied in 6a10a9d. Dropped the 7-row block and replaced it with a single test for the bare scalar { text: 23.976 } - the only shape that genuinely reaches return String(text) and the one that was actually blank before the fix.
| it('renders non-string values rather than blanking the node', () => { | ||
| updateTextPreviewWidgets(node, { text: [30.0, 23.976] } as unknown as { | ||
| text: string[] | ||
| }) | ||
|
|
||
| expect(node.widgets[0].value).toBe('30\n\n23.976') | ||
| }) |
There was a problem hiding this comment.
Question: what did the payload actually look like when the numeric case was reported?
[30.0, 23.976].join('\n\n') is '30\n\n23.976' on the pre-fix implementation too, so this test passes against the code the commit replaces — for arrays, the unguarded join produced the right answer. The shape that genuinely broke is a bare non-array value:
{"text": 23.976}— pre-fix this assigned a number topreview.value, andsetValue(line 46 of the source file) coerces non-strings to'', so the node went blank. That matches the reported symptom, andreturn String(text)is what fixes it.{"text": [23.976]}— rendered correctly before and after.
If Get Video Components sent the first shape, a test for it would be the only one in this file that reaches return String(text):
it('stringifies a bare non-string payload', () => {
updateTextPreviewWidgets(node, { text: 23.976 } as unknown as { text: string })
expect(node.widgets[0].value).toBe('23.976')
})I ran that against the pre-fix implementation and it fails with expected 23.976 to be '23.976'. If it was the second shape, the non-string failure mode may be worth dropping from the commit message instead.
Either way, one caveat about what this layer can prove: beforeEach pushes a plain { name: 'preview_text', options: {}, value: '' } object, so preview.value = ... is a bare property write and the setValue coercion — the actual blanking mechanism — is never in the assertion path. The unit tests can demonstrate stringification, but not "rather than blanking the node".
There was a problem hiding this comment.
Good analysis. The bare non-string scalar shape ({ text: 23.976 }) is what the fix targets - added a unit test for it in 6a10a9d and an e2e step in 91c26ea that sends it unwrapped. Your point about setValue coercion not being in the assertion path is fair - the unit test proves stringification happens, but the e2e step is the one that exercises the actual blanking prevention end to end.
| for (const [label, text] of payloads) { | ||
| await test.step(label, async () => { | ||
| execution.executed('', id, { text: [text] }) | ||
| await expect(preview).toHaveValue(text) | ||
| }) | ||
| } | ||
|
|
||
| await test.step('null text does not wedge the widget', async () => { | ||
| // The shape the Cloud backend produced when it misclassified the text | ||
| // as a filename and dropped it from the payload (BE-3601). | ||
| execution.executed('', id, { text: [null] }) | ||
| await expect(preview).toHaveValue('') | ||
|
|
||
| execution.executed('', id, { text: ['recovered'] }) | ||
| await expect(preview).toHaveValue('recovered') | ||
| }) |
There was a problem hiding this comment.
Issue: every frame this spec sends yields the same assertion result against the pre-fix implementation, so it cannot detect the bug in its own commit.
- The six payload rows are strings inside arrays;
text.join('\n\n')returned them unchanged before the fix. { text: [null] }—[null].join('\n\n')was already'', so nothing was ever wedged by that shape, and therecoveredfollow-up passed before too.{}—message.text ?? ''was already''.
The failure that needed fixing is message itself being nullish, and this spec structurally cannot send it: ExecutionHelper.executed() types output as Record<string, unknown>, and app.ts:803 forwards detail.output unmodified.
Two ways to give the spec teeth, both covering ground the unit tests cannot reach:
- widen
ExecutionHelper.executed'soutputtounknownand drive a null-outputexecutedframe, asserting the widget renders empty and still updates on the next frame; - send
{ text: 23.976 }unquoted and unwrapped, which is the only way to exercise the realsetValuestring guard end to end.
Either way the six-row loop could collapse to one representative payload — those characters are already covered at the unit layer, and six browser round-trips is a lot for one identity path.
There was a problem hiding this comment.
Good catch. Added a dedicated step in 91c26ea that sends { text: 23.976 } as a bare scalar - this is the shape that would have been blank before the fix and is the only one the loop couldn't exercise. The 6-row loop is also trimmed to 5 representative strings. The ExecutionHelper.executed output-widening approach would be stronger for null-message coverage but that's a bigger change; happy to do it in a followup if you think it's worth it.
| ['markdown-fenced JSON', '```json\n{"name":"Comfy"}\n```'], | ||
| ['non-ASCII text', '你好,世界。'], | ||
| ['prompt with a trailing space', '"A red car" is a great prompt. '], | ||
| ['numeric output from Get Video Components', '23.976'] |
There was a problem hiding this comment.
Nit: the label says numeric output, but the payload is the string '23.976' wrapped in an array, which makes this row indistinguishable from the five above it. Sending 23.976 unquoted and unwrapped is what would exercise the new String() path — otherwise the label overclaims.
There was a problem hiding this comment.
✅ Fixed in 91c26ea. Moved the numeric case to a separate step that sends 23.976 as a bare unwrapped scalar rather than a string in an array.
| await comfyPage.menu.topbar.newWorkflowButton.click() | ||
| await comfyPage.searchBoxV2.addNode('Preview as Text') | ||
| const node = await comfyPage.vueNodes.getFixtureByTitle('Preview as Text') | ||
| const preview = node.root.locator('textarea') |
There was a problem hiding this comment.
Nit: raw CSS element selector, and this is the second copy in the file (line 58 has the first). WidgetTextPreview.vue sets :aria-label="widget.name", and VueNodeHelpers.getWidgetByName(nodeTitle, widgetName) already resolves it by label:
const preview = comfyPage.vueNodes.getWidgetByName('Preview as Text', 'preview_text')There was a problem hiding this comment.
✅ Fixed in 82b60d5. Both occurrences replaced with comfyPage.vueNodes.getWidgetByName('Preview as Text', 'preview_text'). The node variable was kept in the workflow-restore test since it's still used for the visibility assertions, and dropped entirely from the payloads test where it had no other purpose.
Array.prototype.join already calls String() on non-nullish elements, so the .map() was identity after .filter(). The typeof string fast path returned the same result as the trailing String(text), so it was a no-op too. Four branches collapse to three with identical behavior. Side benefit: return String(text) is now the path strings take, fixing the Codecov uncovered-line flag. Addresses review feedback: #14073 (comment)
Uses the canonical type from apiSchema instead of a hand-written inline shape. Both call sites (previewAny.ts onExecuted and onNodeOutputsUpdated) already pass NodeExecutionOutput, so nothing at the call sites changes. Matches the pattern in resultItemParsing.ts:32 and flattenNodeOutput.ts:7. Addresses review feedback: #14073 (comment)
The 7-row it.each block all exercised the typeof-string identity path,
which was already covered by 'writes a plain string message as-is'.
The bare scalar {text: 23.976} is the only input that reaches
return String(text) and the only shape that was actually broken before
the fix (setValue coerces non-strings to '', blanking the widget).
Addresses review feedback:
#14073 (comment)
The previous row sent '23.976' as a string inside an array, which was
indistinguishable from the string payloads above it. A bare number
{text: 23.976} is the shape that exercises return String(text) and the
one that was blank before the fix.
Addresses review feedback:
#14073 (comment)
WidgetTextPreview.vue sets :aria-label="widget.name", so the widget can be located by label instead of by element type. Removes the duplicate raw CSS selector at both occurrences and uses the established helper. Addresses review feedback: #14073 (comment)
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/extensions/core/textPreviewWidgets.test.ts`:
- Around line 138-143: Remove the no-throw-only test case for
updateTextPreviewWidgets when the node lacks a preview widget, since it asserts
no observable behavior beyond an existing safe no-op guard.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 2920c147-e89c-4d64-aaf6-0325cb28e78c
📒 Files selected for processing (3)
browser_tests/tests/previewAsText.spec.tssrc/extensions/core/textPreviewWidgets.test.tssrc/extensions/core/textPreviewWidgets.ts
|
CodeRabbit's point seems valid. |
Array.prototype.join already calls String() on non-nullish elements, so the .map() was identity after .filter(). The typeof string fast path returned the same result as the trailing String(text), so it was a no-op too. Four branches collapse to three with identical behavior. Side benefit: return String(text) is now the path strings take, fixing the Codecov uncovered-line flag. Addresses review feedback: #14073 (comment)
Uses the canonical type from apiSchema instead of a hand-written inline shape. Both call sites (previewAny.ts onExecuted and onNodeOutputsUpdated) already pass NodeExecutionOutput, so nothing at the call sites changes. Matches the pattern in resultItemParsing.ts:32 and flattenNodeOutput.ts:7. Addresses review feedback: #14073 (comment)
The 7-row it.each block all exercised the typeof-string identity path,
which was already covered by 'writes a plain string message as-is'.
The bare scalar {text: 23.976} is the only input that reaches
return String(text) and the only shape that was actually broken before
the fix (setValue coerces non-strings to '', blanking the widget).
Addresses review feedback:
#14073 (comment)
The previous row sent '23.976' as a string inside an array, which was
indistinguishable from the string payloads above it. A bare number
{text: 23.976} is the shape that exercises return String(text) and the
one that was blank before the fix.
Addresses review feedback:
#14073 (comment)
WidgetTextPreview.vue sets :aria-label="widget.name", so the widget can be located by label instead of by element type. Removes the duplicate raw CSS selector at both occurrences and uses the established helper. Addresses review feedback: #14073 (comment)
|
|
||
| for (const [label, text] of payloads) { | ||
| await test.step(label, async () => { | ||
| execution.executed('', id, { text: [text] }) |
There was a problem hiding this comment.
The repeated empty string as the first argument smells to me.
| expect(node.widgets[0].value).toBe('hello') | ||
| }) | ||
|
|
||
| it.each([ |
There was a problem hiding this comment.
Shouldn't the linter require it.for instead?
| it('renders non-string entries rather than blanking the node', () => { | ||
| updateTextPreviewWidgets(node, { | ||
| text: ['first', 23.976, null, 'second'] | ||
| } as unknown as { text: string[] }) |
| }) | ||
|
|
||
| it('stringifies a bare non-string scalar payload', () => { | ||
| updateTextPreviewWidgets(node, { text: 23.976 } as unknown as { |
updateTextPreviewWidgets read `message.text` off an unvalidated WebSocket
payload. Three failure modes, all reported against the Preview as Text node:
- `message` null/undefined threw `Cannot read properties of null (reading
'text')`, which aborted the update and left the node showing stale or empty
content.
- `{"text": [null]}` joined to an empty string, rendering the node blank. Cloud
produces exactly this shape when it misclassifies a text output as a filename,
fails to upload it, and drops it from the payload.
- `{"text": ["a", null]}` rendered a leading blank separator, and non-string
entries (e.g. an FPS number) went through an unguarded join.
Normalize the payload before assigning: nullish becomes an empty string, null
entries are filtered out of arrays, and non-string values are stringified rather
than blanking the node.
This is the frontend half of the "Preview as Text is blank" reports. The
upstream cause of the nulled payloads is fixed separately in the cloud repo;
this change makes the node degrade to empty instead of throwing, and render
correctly as soon as real text arrives.
Tests: unit coverage for every payload shape above plus the exact strings users
reported blank (LLM JSON in four shapes, non-ASCII, trailing-space prompts,
numeric outputs), and an e2e spec driving those payloads through the WebSocket
that also asserts the widget recovers after a null payload.
- Fixes FE-685
Array.prototype.join already calls String() on non-nullish elements, so the .map() was identity after .filter(). The typeof string fast path returned the same result as the trailing String(text), so it was a no-op too. Four branches collapse to three with identical behavior. Side benefit: return String(text) is now the path strings take, fixing the Codecov uncovered-line flag. Addresses review feedback: #14073 (comment)
Uses the canonical type from apiSchema instead of a hand-written inline shape. Both call sites (previewAny.ts onExecuted and onNodeOutputsUpdated) already pass NodeExecutionOutput, so nothing at the call sites changes. Matches the pattern in resultItemParsing.ts:32 and flattenNodeOutput.ts:7. Addresses review feedback: #14073 (comment)
The 7-row it.each block all exercised the typeof-string identity path,
which was already covered by 'writes a plain string message as-is'.
The bare scalar {text: 23.976} is the only input that reaches
return String(text) and the only shape that was actually broken before
the fix (setValue coerces non-strings to '', blanking the widget).
Addresses review feedback:
#14073 (comment)
The previous row sent '23.976' as a string inside an array, which was
indistinguishable from the string payloads above it. A bare number
{text: 23.976} is the shape that exercises return String(text) and the
one that was blank before the fix.
Addresses review feedback:
#14073 (comment)
WidgetTextPreview.vue sets :aria-label="widget.name", so the widget can be located by label instead of by element type. Removes the duplicate raw CSS selector at both occurrences and uses the established helper. Addresses review feedback: #14073 (comment)
ff23ae3
a8f47a3 to
ff23ae3
Compare
ff23ae3 to
7b0ab98
Compare
`AGENTS.md` says "Never mention Claude/AI in commits", but nothing enforces it here. ComfyUI core already has this check; this ports it. It matters more in this repo than in core: `main` is squash-merge only, and squash does not strip trailers from the composed message, so a trailer on any commit lands in `main`'s history. This is not hypothetical — #14073 carried `Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>` on commit `4afc68a7c3` through a full green CI run. It was caught by hand and stripped. Verified against that exact commit range, red-green: - `947e35b..ff23ae3` (before the fix) → exit 1, reports `4afc68a7c3: Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>` - `947e35b..7b0ab98` (after) → exit 0, "No AI agent Co-authored-by trailers found" Both trees are byte-identical, so the trailer is the only variable. Two changes from core's copy to match this repo: `actions/checkout@v7` (`.pinact.yaml` exempts checkout only at `v7`, so core's `@v4` would fail `Validate Action SHA Pins`), and the `ci-*.yaml` filename convention. Added `permissions: contents: read`. The script covers 12 agent vendors by trailer email plus generic name patterns, so it is not Claude-specific. --------- Co-authored-by: DrJKL <DrJKL0424@gmail.com>
Summary
updateTextPreviewWidgetsreadmessage.textoff an unvalidated WebSocket payload, so a null message threw and a nulled/absent text array rendered the Preview as Text node blank — the frontend half of the long-running "Preview as Text shows nothing" reports.Changes
nullentries are filtered out of arrays, and non-string values are stringified instead of blanking the node.Three failure modes this fixes, all reported against the node:
null/undefinedmessageTypeError: Cannot read properties of null (reading 'text'), update aborts, stale or empty content{"text": [null]}{"text": ["a", null]}a{"text": [23.976]}23.976Review Focus
The
{"text": [null]}shape is not hypothetical — Cloud produces exactly that when inference misclassifies a text output as a filename, fails to upload it, and drops it from the payload. That upstream cause is fixed separately inComfy-Org/cloud#5502; this change is the defensive half, so the node degrades to empty instead of throwing and renders correctly as soon as real text arrives. The two are independently mergeable.Non-string stringification is deliberately shallow (
String(part)) — it preserves today's behavior for objects rather than speculating about a JSON shape the backend does not currently send.Tests
src/extensions/core/textPreviewWidgets.test.ts): every payload shape above, plus the exact strings users reported blank — LLM JSON in four shapes (compact, pretty-printed, markdown-fenced, array of objects), non-ASCII text, trailing-space prompts, prompts ending in a quoted period, numeric outputs — and the no-preview-widget case. 18 tests pass.browser_tests/tests/previewAsText.spec.ts): drives those payloads through the WebSocket and asserts the rendered textarea, including that the widget recovers after anullpayload and after an output with notextkey.Verified:
pnpm typecheck,pnpm typecheck:browser,pnpm lint(0 errors),pnpm format,pnpm knip,pnpm vitest run. The new e2e spec collects underplaywright --listbut was not executed locally (needs a running backend).🤖 Generated with Claude Code