fix: show backend-submitted node display names in English locale - #14541
Merged
Conversation
The English locale bundle (src/locales/en/nodeDefs.json) previously shadowed the display_name submitted by the backend in /object_info, so renaming a core node had no visible effect until the frontend regenerated its locale files. Merge the backend display names into the en locale messages when node defs are fetched, so every nodeDefs.<name>.display_name lookup (node titles, search, context menus) resolves to the live backend value. Other locales are unaffected: missing keys there already fall back to the backend string, and existing translations still apply. Co-authored-by: Amp <amp@ampcode.com> Amp-Thread-ID: https://ampcode.com/threads/T-019fbe31-690b-76f0-a579-d34bb7adb9e2
dosubot
Bot
added
the
size:M
📄 Knowledge reviewDosu skipped reviewing this PR because your organization has used its
|
🎭 Playwright: ✅ 1779 passed, 0 failed · 2 flaky📊 Browser Reports
🎨 Storybook: ✅ Built — View Storybook📦 Bundle: 8.23 MB gzip 🔴 +1.19 kBDetailsSummary
Category Glance App Entry Points — 3.64 kB (baseline 3.64 kB) • ⚪ 0 BMain entry bundles and manifests
Status: 1 added / 1 removed Graph Workspace — 1.31 MB (baseline 1.31 MB) • 🟢 -51 BGraph editor runtime, canvas, workflow orchestration
Status: 1 added / 1 removed / 1 unchanged Views & Navigation — 111 kB (baseline 111 kB) • ⚪ 0 BTop-level views, pages, and routed surfaces
Status: 12 added / 12 removed / 4 unchanged Panels & Settings — 551 kB (baseline 551 kB) • ⚪ 0 BConfiguration panels, inspectors, and settings screens
Status: 11 added / 11 removed / 15 unchanged User & Accounts — 28.7 kB (baseline 28.7 kB) • ⚪ 0 BAuthentication, profile, and account management bundles
Status: 7 added / 7 removed / 3 unchanged Editors & Dialogs — 124 kB (baseline 124 kB) • ⚪ 0 BModals, dialogs, drawers, and in-app editors
Status: 6 added / 6 removed / 1 unchanged UI Components — 70 kB (baseline 70 kB) • ⚪ 0 BReusable component library chunks
Status: 6 added / 6 removed / 9 unchanged Data & Services — 3.46 MB (baseline 3.46 MB) • 🔴 +433 BStores, services, APIs, and repositories
Status: 14 added / 14 removed / 3 unchanged Utilities & Hooks — 386 kB (baseline 386 kB) • 🔴 +65 BHelpers, composables, and utility bundles
Status: 19 added / 19 removed / 17 unchanged Vendor & Third-Party — 15.7 MB (baseline 15.7 MB) • ⚪ 0 BExternal libraries and shared vendor chunks Status: 16 unchanged Other — 12.8 MB (baseline 12.8 MB) • 🔴 +2.91 kBBundles that do not match a named category
Status: 76 added / 76 removed / 205 unchanged ⚡ Performance Report
Show regressions
All metrics
Historical variance (last 15 runs)
Trend (last 15 commits on main)
Raw data{
"timestamp": "2026-08-03T23:59:34.634Z",
"gitSha": "6d5607e4be665d4482762b57fe6c78adbac26352",
"branch": "fix/en-node-display-names-from-backend",
"measurements": [
{
"name": "canvas-idle",
"durationMs": 2092.010000000073,
"styleRecalcs": 10,
"styleRecalcDurationMs": 8.579000000000002,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 485.7389999999999,
"heapDeltaBytes": 5414236,
"heapUsedBytes": 69366036,
"domNodes": 20,
"jsHeapTotalBytes": 24903680,
"scriptDurationMs": 15.787999999999997,
"eventListeners": 4,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "canvas-idle",
"durationMs": 2039.357999999993,
"styleRecalcs": 9,
"styleRecalcDurationMs": 8.175000000000002,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 450.80600000000004,
"heapDeltaBytes": 5559068,
"heapUsedBytes": 69384240,
"domNodes": 18,
"jsHeapTotalBytes": 25165824,
"scriptDurationMs": 15.312999999999999,
"eventListeners": 4,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "canvas-mouse-sweep",
"durationMs": 2051.3939999999593,
"styleRecalcs": 78,
"styleRecalcDurationMs": 47.614999999999995,
"layouts": 12,
"layoutDurationMs": 3.4919999999999995,
"taskDurationMs": 1029.593,
"heapDeltaBytes": -10155620,
"heapUsedBytes": 53611728,
"domNodes": -280,
"jsHeapTotalBytes": 23318528,
"scriptDurationMs": 134.963,
"eventListeners": -148,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "canvas-mouse-sweep",
"durationMs": 1836.1579999999549,
"styleRecalcs": 74,
"styleRecalcDurationMs": 36.049,
"layouts": 12,
"layoutDurationMs": 3.5030000000000006,
"taskDurationMs": 865.9549999999999,
"heapDeltaBytes": -16743912,
"heapUsedBytes": 46960024,
"domNodes": -279,
"jsHeapTotalBytes": 24104960,
"scriptDurationMs": 121.97699999999999,
"eventListeners": -148,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "canvas-zoom-sweep",
"durationMs": 1720.843000000059,
"styleRecalcs": 31,
"styleRecalcDurationMs": 14.902000000000001,
"layouts": 6,
"layoutDurationMs": 0.5,
"taskDurationMs": 363.18,
"heapDeltaBytes": 8183164,
"heapUsedBytes": 71950576,
"domNodes": 77,
"jsHeapTotalBytes": 25165824,
"scriptDurationMs": 17.567999999999998,
"eventListeners": 19,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.799999999999272
},
{
"name": "canvas-zoom-sweep",
"durationMs": 1771.0960000000568,
"styleRecalcs": 32,
"styleRecalcDurationMs": 18.224999999999998,
"layouts": 6,
"layoutDurationMs": 0.6630000000000001,
"taskDurationMs": 386.59200000000004,
"heapDeltaBytes": 8679232,
"heapUsedBytes": 72616676,
"domNodes": 78,
"jsHeapTotalBytes": 24379392,
"scriptDurationMs": 18.669999999999998,
"eventListeners": 19,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333335,
"p95FrameDurationMs": 16.799999999999272
},
{
"name": "dom-widget-clipping",
"durationMs": 649.9679999999444,
"styleRecalcs": 10,
"styleRecalcDurationMs": 10.081,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 411.64399999999995,
"heapDeltaBytes": -10595376,
"heapUsedBytes": 53131608,
"domNodes": 16,
"jsHeapTotalBytes": 25165824,
"scriptDurationMs": 65.71199999999999,
"eventListeners": 2,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "dom-widget-clipping",
"durationMs": 573.5600000000431,
"styleRecalcs": 11,
"styleRecalcDurationMs": 7.9959999999999996,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 368.025,
"heapDeltaBytes": -10908228,
"heapUsedBytes": 52900588,
"domNodes": 18,
"jsHeapTotalBytes": 25952256,
"scriptDurationMs": 58.419,
"eventListeners": 0,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333335,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "large-graph-idle",
"durationMs": 2026.7109999999775,
"styleRecalcs": 9,
"styleRecalcDurationMs": 7.755000000000002,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 665.4789999999999,
"heapDeltaBytes": 7166372,
"heapUsedBytes": 66574244,
"domNodes": -280,
"jsHeapTotalBytes": 2998272,
"scriptDurationMs": 99.99,
"eventListeners": -142,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333335,
"p95FrameDurationMs": 16.699999999999818
},
{
"name": "large-graph-idle",
"durationMs": 2039.1750000000002,
"styleRecalcs": 8,
"styleRecalcDurationMs": 7.3100000000000005,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 667.9380000000001,
"heapDeltaBytes": 6658732,
"heapUsedBytes": 66003648,
"domNodes": -282,
"jsHeapTotalBytes": 3260416,
"scriptDurationMs": 103.855,
"eventListeners": -142,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "large-graph-pan",
"durationMs": 2147.770000000037,
"styleRecalcs": 68,
"styleRecalcDurationMs": 12.723000000000003,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 1308.947,
"heapDeltaBytes": -6531648,
"heapUsedBytes": 55027812,
"domNodes": -283,
"jsHeapTotalBytes": 3784704,
"scriptDurationMs": 455.281,
"eventListeners": -174,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "large-graph-pan",
"durationMs": 2158.681999999999,
"styleRecalcs": 69,
"styleRecalcDurationMs": 14.140000000000004,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 1265.259,
"heapDeltaBytes": 5111272,
"heapUsedBytes": 65617892,
"domNodes": -282,
"jsHeapTotalBytes": 3977216,
"scriptDurationMs": 430.655,
"eventListeners": -144,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "large-graph-zoom",
"durationMs": 3245.5739999999196,
"styleRecalcs": 63,
"styleRecalcDurationMs": 14.962000000000003,
"layouts": 60,
"layoutDurationMs": 7.271,
"taskDurationMs": 1515.176,
"heapDeltaBytes": 23237224,
"heapUsedBytes": 85758292,
"domNodes": 8,
"jsHeapTotalBytes": 7340032,
"scriptDurationMs": 554.856,
"eventListeners": 8,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "large-graph-zoom",
"durationMs": 3180.115999999998,
"styleRecalcs": 64,
"styleRecalcDurationMs": 16.477,
"layouts": 60,
"layoutDurationMs": 6.944000000000001,
"taskDurationMs": 1517.165,
"heapDeltaBytes": -5420016,
"heapUsedBytes": 57430540,
"domNodes": -288,
"jsHeapTotalBytes": 5881856,
"scriptDurationMs": 542.927,
"eventListeners": -148,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.799999999999272
},
{
"name": "minimap-idle",
"durationMs": 2026.2010000000146,
"styleRecalcs": 8,
"styleRecalcDurationMs": 6.915000000000001,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 655.265,
"heapDeltaBytes": 7144256,
"heapUsedBytes": 67897588,
"domNodes": -282,
"jsHeapTotalBytes": 2998272,
"scriptDurationMs": 101.628,
"eventListeners": -144,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66999999999998,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "minimap-idle",
"durationMs": 2038.2749999999987,
"styleRecalcs": 7,
"styleRecalcDurationMs": 6.187999999999999,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 657.1530000000001,
"heapDeltaBytes": 7331724,
"heapUsedBytes": 67970532,
"domNodes": -281,
"jsHeapTotalBytes": 2736128,
"scriptDurationMs": 101.497,
"eventListeners": -142,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333335,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "subgraph-dom-widget-clipping",
"durationMs": 557.5179999999591,
"styleRecalcs": 46,
"styleRecalcDurationMs": 8.946,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 386.36199999999997,
"heapDeltaBytes": -10438956,
"heapUsedBytes": 53508188,
"domNodes": 18,
"jsHeapTotalBytes": 25690112,
"scriptDurationMs": 113.727,
"eventListeners": 6,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "subgraph-dom-widget-clipping",
"durationMs": 589.8489999999583,
"styleRecalcs": 46,
"styleRecalcDurationMs": 9.632000000000001,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 399.67600000000004,
"heapDeltaBytes": -10367844,
"heapUsedBytes": 53616580,
"domNodes": 18,
"jsHeapTotalBytes": 25952256,
"scriptDurationMs": 116.333,
"eventListeners": 6,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333335,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "subgraph-idle",
"durationMs": 2037.5510000000077,
"styleRecalcs": 9,
"styleRecalcDurationMs": 8.056000000000001,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 479.432,
"heapDeltaBytes": -15863572,
"heapUsedBytes": 47919212,
"domNodes": -280,
"jsHeapTotalBytes": 23056384,
"scriptDurationMs": 13.603,
"eventListeners": -148,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333332,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "subgraph-idle",
"durationMs": 2037.5420000000304,
"styleRecalcs": 9,
"styleRecalcDurationMs": 7.767000000000001,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 481.62699999999995,
"heapDeltaBytes": -19001368,
"heapUsedBytes": 45096480,
"domNodes": 18,
"jsHeapTotalBytes": 24104960,
"scriptDurationMs": 13.980999999999998,
"eventListeners": 4,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "subgraph-mouse-sweep",
"durationMs": 1702.3929999999154,
"styleRecalcs": 74,
"styleRecalcDurationMs": 34.996,
"layouts": 16,
"layoutDurationMs": 4.718,
"taskDurationMs": 818.359,
"heapDeltaBytes": -14896892,
"heapUsedBytes": 48957740,
"domNodes": -283,
"jsHeapTotalBytes": 23580672,
"scriptDurationMs": 95.895,
"eventListeners": -148,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "subgraph-mouse-sweep",
"durationMs": 1730.152999999973,
"styleRecalcs": 75,
"styleRecalcDurationMs": 34.31400000000001,
"layouts": 16,
"layoutDurationMs": 3.9689999999999994,
"taskDurationMs": 789.365,
"heapDeltaBytes": -16902344,
"heapUsedBytes": 47046932,
"domNodes": -278,
"jsHeapTotalBytes": 23580672,
"scriptDurationMs": 90.834,
"eventListeners": -148,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.699999999999818
},
{
"name": "subgraph-transition-enter",
"durationMs": 1383.5579999999936,
"styleRecalcs": 18,
"styleRecalcDurationMs": 30.032000000000004,
"layouts": 14,
"layoutDurationMs": 12.659,
"taskDurationMs": 907.561,
"heapDeltaBytes": 31007968,
"heapUsedBytes": 99045872,
"domNodes": 13673,
"jsHeapTotalBytes": 15466496,
"scriptDurationMs": 33.36800000000001,
"eventListeners": 2371,
"totalBlockingTimeMs": 135,
"frameDurationMs": 16.666666666666636,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "viewport-pan-sweep",
"durationMs": 8224.57799999995,
"styleRecalcs": 249,
"styleRecalcDurationMs": 35.569,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 4369.013,
"heapDeltaBytes": 8686336,
"heapUsedBytes": 67842124,
"domNodes": -282,
"jsHeapTotalBytes": 5812224,
"scriptDurationMs": 1366.769,
"eventListeners": -128,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.799999999999272
},
{
"name": "viewport-pan-sweep",
"durationMs": 8295.044999999958,
"styleRecalcs": 250,
"styleRecalcDurationMs": 36.777,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 4570.399,
"heapDeltaBytes": 11430408,
"heapUsedBytes": 70797852,
"domNodes": -280,
"jsHeapTotalBytes": 5287936,
"scriptDurationMs": 1401.1639999999998,
"eventListeners": -128,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333332,
"p95FrameDurationMs": 16.799999999999272
},
{
"name": "vue-large-graph-idle",
"durationMs": 16477.700000000026,
"styleRecalcs": 0,
"styleRecalcDurationMs": 0,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 16462.67,
"heapDeltaBytes": -68347128,
"heapUsedBytes": 153874092,
"domNodes": -8316,
"jsHeapTotalBytes": -9375744,
"scriptDurationMs": 507.87500000000006,
"eventListeners": -16387,
"totalBlockingTimeMs": 0,
"frameDurationMs": 17.219999999999953,
"p95FrameDurationMs": 16.80000000000291
},
{
"name": "vue-large-graph-idle",
"durationMs": 16942.465999999968,
"styleRecalcs": 0,
"styleRecalcDurationMs": 0,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 16924.858,
"heapDeltaBytes": -31910248,
"heapUsedBytes": 166498092,
"domNodes": -8312,
"jsHeapTotalBytes": -14098432,
"scriptDurationMs": 536.756,
"eventListeners": -16387,
"totalBlockingTimeMs": 0,
"frameDurationMs": 17.776666666666642,
"p95FrameDurationMs": 16.799999999999272
},
{
"name": "vue-large-graph-pan",
"durationMs": 20453.34200000002,
"styleRecalcs": 138,
"styleRecalcDurationMs": 15.950999999999993,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 20428.072,
"heapDeltaBytes": -28448676,
"heapUsedBytes": 176705132,
"domNodes": -8312,
"jsHeapTotalBytes": -10231808,
"scriptDurationMs": 849.672,
"eventListeners": -16381,
"totalBlockingTimeMs": 122,
"frameDurationMs": 17.776666666666642,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "vue-large-graph-pan",
"durationMs": 20794.511000000057,
"styleRecalcs": 143,
"styleRecalcDurationMs": 16.60600000000001,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 20761.001,
"heapDeltaBytes": -54225696,
"heapUsedBytes": 168828076,
"domNodes": -8312,
"jsHeapTotalBytes": -14098432,
"scriptDurationMs": 892.233,
"eventListeners": -16381,
"totalBlockingTimeMs": 325,
"frameDurationMs": 18.33666666666674,
"p95FrameDurationMs": 16.799999999999272
},
{
"name": "workflow-execution",
"durationMs": 453.20300000003044,
"styleRecalcs": 13,
"styleRecalcDurationMs": 18.317,
"layouts": 3,
"layoutDurationMs": 0.662,
"taskDurationMs": 108.062,
"heapDeltaBytes": 4996556,
"heapUsedBytes": 68129248,
"domNodes": 130,
"jsHeapTotalBytes": 4980736,
"scriptDurationMs": 9.45,
"eventListeners": 67,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "workflow-execution",
"durationMs": 454.6069999998963,
"styleRecalcs": 13,
"styleRecalcDurationMs": 17.877,
"layouts": 2,
"layoutDurationMs": 0.30900000000000005,
"taskDurationMs": 97.27900000000001,
"heapDeltaBytes": 4762352,
"heapUsedBytes": 67751924,
"domNodes": 119,
"jsHeapTotalBytes": 4718592,
"scriptDurationMs": 9.060000000000002,
"eventListeners": 65,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333332,
"p95FrameDurationMs": 16.700000000000728
}
]
} |
🌐 Website E2ETip All tests passed.
|
Contributor
📝 WalkthroughWalkthroughThe change adds shared Vue i18n escaping, backend node text tracking, normalized node-definition resolution, and updated display-name consumers. Tests cover precedence, locale behavior, validation, fallbacks, special characters, custom translations, and dotted names. ChangesNode-definition localization
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant getNodeDefs
participant setBackendNodeText
participant translateNodeDef
participant resolveNodeDefText
participant DisplayNameConsumer
getNodeDefs->>setBackendNodeText: backend node definitions
getNodeDefs->>translateNodeDef: fetched definitions
translateNodeDef->>resolveNodeDefText: display name and description
resolveNodeDefText-->>translateNodeDef: resolved node text
DisplayNameConsumer->>resolveNodeDefText: node display-name lookup
resolveNodeDefText-->>DisplayNameConsumer: localized or fallback text
Possibly related issues
Possibly related PRs
Suggested labels: Suggested reviewers: Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 inconclusive)
✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Codecov Report❌ Patch coverage is
@@ Coverage Diff @@
## main #14541 +/- ##
==========================================
+ Coverage 78.14% 78.53% +0.39%
==========================================
Files 1768 1778 +10
Lines 105794 103212 -2582
Branches 32899 33063 +164
==========================================
- Hits 82669 81061 -1608
+ Misses 22659 21721 -938
+ Partials 466 430 -36
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 213 files with indirect coverage changes 🚀 New features to boost your workflow:
|
bigcat88
added
the
New Browser Test Expectations
|
Updating Playwright Expectations |
github-actions
Bot
removed
the
New Browser Test Expectations
christian-byrne
left a comment
christian-byrne
left a comment
Contributor
There was a problem hiding this comment.
Review summary
Taking this over with bigcat88's permission, so this review is documentation for other readers rather than a gate. Nine reviewers were run across i18n semantics, correctness, regression risk, completeness, architecture, ecosystem compatibility, API contract, test quality and CI. Every finding below was independently reproduced against b87cd114a5 before posting; suspicions that did not survive verification were dropped.
What is right and is being kept
The diagnosis is correct, and the precedence rules stated in the Review Focus are the right rules. The normalizeI18nKey fix in translateNodeDef is a genuine bug nobody else caught: nodeTitleUtil.ts:26, useContextMenuTranslation.ts:169, litegraphService.ts:213, NodeTooltip.vue and useNodeTooltips.ts all already normalized, and the serializer writes normalized keys, so app.ts was the lone outlier reading keys nobody writes. It is kept, with the migration caveat in the inline comment.
One correction to an argument that could otherwise be made against this PR, since it turned out to be wrong on inspection: re-sharing the escape helper does not violate a boundary established by #13643. That PR's rationale was module cohesion, not layering, and it kept scripts/nodeDefLocaleSerializer.ts importing normalizeI18nKey from the same shared package. The package is also private: true, so nothing external widens.
Findings that could not be anchored inline
nodeCategories is un-normalized, four lines below the line this PR fixes. app.ts:1115 builds st(`nodeCategories.${category}`, category) with no normalizeI18nKey, while nodeDefLocaleSerializer.ts:122 writes the normalized key. Confirmed in the shipped locale: en/main.json contains nodeCategories.sync_so = "sync.so", so the consumer asks for nodeCategories.sync.so, vue-i18n resolves it as a nested path, misses, and emits a missing key warning. Same defect class as the line above it, and normalizeI18nKey is already imported by this PR.
One regenerated snapshot has no display name cause. vue-nodes-paned-with-touch-mobile-chrome-linux.png differs by 640 pixels of 285711, confined to rows 59, 69-73 and 611-622. Row 611-622 is the ckpt_name combo widget value; glyph x positions are identical and only antialias coverage differs. The node title row is byte identical. That is sub-pixel rasterization noise, not behavior. Re-baselining it hides future genuine regressions in that golden, and playwright.config.ts sets no toHaveScreenshot tolerance, so it will flap. The other 7 are legitimate and were each confirmed visually.
The goldens now assert a name real users never see. The CI container ghcr.io/comfy-org/comfyui-ci-container:0.0.21 is hardcoded in four workflow files, and no Dockerfile, version file or lockfile in this repo records which ComfyUI commit it contains. Meanwhile en/nodeDefs.json is regenerated each release against ComfyUI master. So the container is behind what ships: the bundle says Compare Images, the container says Image Compare, and the new goldens encode the container's stale name. routeObjectInfoFromSetupApi in browser_tests/fixtures/utils/objectInfo.ts:107 already does exactly the needed stubbing for tooltips and combo options, with widgetTooltip.spec.ts:29-41 as a working reference, so a setNodeDisplayName mutator would remove the container from the goldens' dependency set entirely.
A release signal disappears. Before this change, a core node rename broke screenshot goldens on the version bump PR: annoying, but loud and correctly attributed. After it, English names are overridden at runtime, so the same rename only surfaces later on whatever container bump chore PR happens to cross it, whose author has no context.
Precedence is encoded by replaying another module's private state. mergeBackendNodeDisplayNames re-merges the entire customNodesI18nData['en'] subtree on every call, including from reloadNodeDefs(), because the two producers have no ordering relationship. The invariant does hold today in every ordering traced. The cost is that a third source would need N-squared replay edges, and additive mergeLocaleMessage means a later /api/i18n fetch that drops an entry has it resurrected by the replay.
Performance on heavy installs. Each getNodeDefs() now deep copies the full backend node set into the deeply reactive en message tree, twice, and every backend node subsequently hits the vue-i18n message compiler where previously only the 823 bundled keys did. On a stock install that is a wash. On a 3000 node type install it is roughly 4x the compile work plus about 6000 proxied property writes per refresh, and it runs on startup, on every Refresh Node Definitions, and on locale change.
Verified non-findings, recorded so they are not re-litigated
Custom node precedence holds in both load orders, including the void loadI18n() race, because GraphCanvas.vue:528 awaits isI18nReady before comfyApp.setup(). mergeLocaleMessage is a true recursive merge, so bundled description/inputs/outputs survive for all 823 core nodes. No prototype pollution via __proto__. escapeI18nMessage does not become public API: the comfyAPI shim covers only src/scripts and src/extensions/core, and the package is private with an unchanged exports map. Frontend only nodes are unaffected. Cloud e2e is immune because cloudBootMocks.ts:44 stubs object_info to {}. pnpm knip and pnpm format:check both exit 0.
Plan
Rather than patch the character set and the guards individually, the fix being implemented keeps English off the message compiler entirely: a resolver that holds raw backend values and applies the precedence table explicitly, consulting customNodesI18nData directly instead of replaying merges. That removes the backslash class rather than extending it, removes the unvalidated third party merge from the boot path, and drops the need to escape backend text at runtime at all. Non-English keeps today's behavior unchanged.
English node display names were shadowed by the bundled en/nodeDefs.json snapshot, so renaming a core node had no effect until the frontend regenerated its locale files. The previous approach escaped backend strings and merged them into the vue-i18n 'en' message tree. That kept the source language on the message compiler, which brought several problems: the escape set was incomplete for vue-i18n 11 (a backslash before an escaped character swallows the brace the escaper emits, so "C:\@home" failed to compile and rendered as literal escape syntax), an unvalidated /api/i18n payload was re-merged from the boot-critical getNodeDefs path where a throw is fatal rather than degrading, and the merge mutated the imported locale JSON module in place with no way to evict stale entries. Resolve instead. Raw /object_info text is kept in a plain map, and resolveNodeDefText applies precedence explicitly: en: custom-node /api/i18n -> backend -> bundled -> node name other: custom-node /api/i18n -> bundled -> backend -> node name English is the source language, so a backend value is data rather than a translation and is returned without being compiled. Non-English keeps today's behaviour, falling back to the live backend value instead of the stale English snapshot. Custom-node translations still compile, because the generated locale pipeline escapes them. Also: - Fix the escape helper for backslash (double it; vue-i18n cannot express a literal backslash via interpolation) and guard non-string input, so build-time locale generation is correct for the same inputs. - Normalize nodeCategories keys, which had the same defect the dotted node-name fix addressed four lines above. - Resolve the legacy nested custom-locale shape as well as the flat normalized key, so hand-written locales that nest dotted node ids keep working. - Tolerate a non-object /object_info response. - Revert a regenerated pan.spec golden whose 640 changed pixels are antialiasing on a widget value, with the node title row byte-identical. Tests cover the precedence table per locale, verbatim rendering of names containing i18n syntax, eviction of dropped backend text, both key shapes, and escape round-trips through the real vue-i18n compiler. Verified by mutation: dropping the English bypass, the legacy key shape, the eviction, or the backslash fix each fails the suite. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
dosubot
Bot
added
size:L
This was referenced Aug 3, 2026
Closed
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/scripts/app.ts`:
- Around line 1122-1123: Validate the `/object_info/` result in the
`getNodeDefs` flow with a type guard that accepts only non-array records before
passing it to `Object.values` and `_.mapValues`; fall back safely for null,
strings, arrays, booleans, and numbers. Update
`src/scripts/app.getNodeDefs.test.ts` lines 103-108 to cover each invalid
response type, while `src/scripts/app.ts` lines 1122-1123 must implement the
narrowing and safe fallback.
- Around line 1105-1112: Update the node text construction around
resolveNodeDefText in getNodeDefs so display_name does not pass def.name as the
backend fallback when display_name is missing; pass only a non-empty backend
value or omit the third argument, allowing bundled/custom text resolution.
Resolve description unconditionally instead of returning an empty string for
missing def.description, and add getNodeDefs tests covering omitted backend
fields with bundled and custom text available.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: aeef71db-ff66-41a7-aa6c-f9df457d795c
⛔ Files ignored due to path filters (7)
browser_tests/tests/imageCompare.spec.ts-snapshots/image-compare-default-50-chromium-linux.pngis excluded by!**/*.pngbrowser_tests/tests/imageCompare.spec.ts-snapshots/image-compare-slider-25-chromium-linux.pngis excluded by!**/*.pngbrowser_tests/tests/imageCompare.spec.ts-snapshots/image-compare-slider-75-chromium-linux.pngis excluded by!**/*.pngbrowser_tests/tests/nodeBadge.spec.ts-snapshots/node-lifecycle-None-classic-chromium-linux.pngis excluded by!**/*.pngbrowser_tests/tests/nodeBadge.spec.ts-snapshots/node-lifecycle-None-vue-chromium-linux.pngis excluded by!**/*.pngbrowser_tests/tests/nodeBadge.spec.ts-snapshots/node-lifecycle-Show-all-classic-chromium-linux.pngis excluded by!**/*.pngbrowser_tests/tests/nodeBadge.spec.ts-snapshots/node-lifecycle-Show-all-vue-chromium-linux.pngis excluded by!**/*.png
📒 Files selected for processing (19)
packages/shared-frontend-utils/src/formatUtil.test.tspackages/shared-frontend-utils/src/formatUtil.tsscripts/nodeDefLocaleSerializer.test.tssrc/components/queue/QueueInlineProgressSummary.vuesrc/components/rightSidePanel/RightSidePanel.vuesrc/components/rightSidePanel/errors/useErrorGroups.tssrc/components/rightSidePanel/parameters/WidgetItem.vuesrc/composables/queue/useJobList.test.tssrc/composables/queue/useJobList.tssrc/composables/useContextMenuTranslation.tssrc/i18n.test.tssrc/i18n.tssrc/platform/missingMedia/components/MissingMediaCard.vuesrc/platform/missingModel/composables/useMissingModelInteractions.tssrc/renderer/extensions/vueNodes/components/NodeHeader.vuesrc/scripts/app.getNodeDefs.test.tssrc/scripts/app.tssrc/stores/workspace/favoritedWidgetsStore.tssrc/utils/nodeTitleUtil.ts
Two defects in the resolver wiring, both reported by CodeRabbit on the
previous commit.
translateNodeDef passed `def.display_name || def.name` as the backend
value, so a def whose display_name the backend omits was handed its own
node name as though the backend had supplied it. resolveNodeDefText then
returned that and never consulted the bundled snapshot, which inverts the
documented precedence. Pass the backend value only when there is one, and
resolve the description the same way rather than short-circuiting to ''
when the backend omits it.
`?? {}` only guards null and undefined. A string response survived it,
Object.values spread it into characters, and translateNodeDef then threw
on def.category.split during node registration. Accept only a non-array
object, and tolerate a def with no category.
Tests use genuinely bundled nodes rather than custom-node entries, since
a custom-node entry short-circuits ahead of the path under test and hides
the defect. Verified by mutation: restoring either short-circuit or the
weak guard fails the suite.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/scripts/app.getNodeDefs.test.ts`:
- Around line 138-141: Update the test around comfyApp.getNodeDefs() to capture
its resolved node definitions and assert that the definition created by nodeDef
with an undefined category has category equal to an empty string, while
retaining the existing no-throw expectation.
In `@src/scripts/app.ts`:
- Around line 1124-1132: In src/scripts/app.ts lines 1124-1132, update the
getNodeDefs response handling to type-guard every entry and its required text
fields before setBackendNodeText and translateNodeDef, discarding malformed
definitions while retaining valid ones. In src/scripts/app.getNodeDefs.test.ts
lines 103-112, add cases where an otherwise valid object response contains null
or invalid field types and verify those entries are ignored without preventing
valid node registration.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: bf88c4b3-fb00-45c3-9614-c480900c127a
📒 Files selected for processing (2)
src/scripts/app.getNodeDefs.test.tssrc/scripts/app.ts
…ation
The response-level guard accepted an object whose individual entries were
malformed, so `{ Broken: null }` still reached translateNodeDef and threw
on `def.name`, and `{ name: 'X', category: 1 }` threw on `category.split`.
A guard that stops at the envelope implies a safety it does not provide.
Validate each entry, keep the well-formed ones, and narrow the category
read to an actual string. A malformed entry is now dropped rather than
aborting node registration for every node.
Also assert the resolved category value for a def with no category, which
the previous test only checked for absence of a throw.
Verified by mutation: dropping the per-entry filter or widening the
category read back to `?? ''` fails the suite.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/scripts/app.ts`:
- Around line 1124-1140: Strengthen the isNodeDef validator in
src/scripts/app.ts:1124-1140 to require the complete ComfyNodeDefV1 structure,
including input, output_name, and python_module, while preserving the existing
missing-category fallback; in src/scripts/app.getNodeDefs.test.ts:146-159, add a
name-only definition fixture and assert getNodeDefs excludes it from the
returned definitions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 5f06b299-73f1-4f37-a004-ca49c63c5599
📒 Files selected for processing (2)
src/scripts/app.getNodeDefs.test.tssrc/scripts/app.ts
christian-byrne
dismissed
coderabbitai[bot]’s stale review
August 4, 2026 00:02
Valid findings from this review were fixed in a86aa28 and deec5f0 (precedence inversion on missing backend text, weak object_info envelope guard, per-entry validation, category assertion). The remaining request, full structural validation of node definitions at the API boundary, is declined with reasoning in the thread: it is not a regression from this PR, and it would reverse the deliberate removal of node-def validation in #1190 and #4038 while risking silently dropping custom nodes whose shape does not match ComfyNodeDefV1 exactly. Dismissing so the PR is not blocked on a scope increase that belongs in its own change.
christian-byrne
approved these changes
Aug 4, 2026
christian-byrne
left a comment
christian-byrne
left a comment
Contributor
There was a problem hiding this comment.
Approving the reworked version.
Landing bigcat88's fix with his permission, on a different mechanism. His diagnosis and the precedence rules in the original Review Focus were correct, and the normalizeI18nKey fix in translateNodeDef was a real bug nobody else had caught: every other nodeDefs.* consumer already normalized, so that one call site was reading keys nothing writes. That is kept.
What changed is where backend text lives. Merging escaped backend strings into the vue-i18n en message tree kept the source language on the message compiler, which is what produced the escaping defect (incomplete for vue-i18n 11 on backslash), put an unvalidated /api/i18n merge on the boot-critical path, and mutated the imported locale JSON with no way to evict. Resolving instead removes all three rather than patching them.
Verified: 53 checks green, 0 failing. Escape behaviour checked against the pinned vue-i18n@11.4.6, and the change is a provable no-op for the existing corpus (0 backslashes across 13 locales, 7905 strings), so a release regeneration is byte-identical. Coverage verified by mutation rather than inspection: dropping the English bypass, the legacy key shape, the eviction, the backslash fix, the per-entry filter, or either precedence short-circuit each fails the suite.
Known gaps are filed rather than hidden: #14629, #14630, #14631.
Contributor
|
@bigcat88 thanks for this, and for letting me take it over. Merged as 91c31b1. Your diagnosis was right and so were the precedence rules you wrote in the Review Focus. The What I changed is where the backend text lives, not what it means. Merging escaped backend strings into the vue-i18n
Resolving instead of merging removes all three rather than patching each. Precedence is now declared explicitly per locale, English returns the backend value uncompiled, and non-English is unchanged except that it falls back to the live backend value rather than the stale English snapshot. One thing your PR flagged that turned out to be half true: the Review Focus said the legacy nested custom-locale shape was unsupported anywhere. Every other consumer had indeed stopped resolving it, but Follow-ups are filed rather than folded in: #14629, #14630, #14631. |
This was referenced Aug 4, 2026
Merged
christian-byrne
added a commit
that referenced
this pull request
Aug 5, 2026
## Summary Pin the node display names that Playwright goldens capture, so screenshots stop tracking whatever ComfyUI build the CI container happens to ship — and make the failure mode legible when a pinned node type goes missing. ## Changes - **What**: - New `setNodeDisplayName(objectInfo, nodeType, displayName)` mutator in `browser_tests/fixtures/utils/objectInfo.ts`, alongside `setStringInputTooltip` / `setComboInputOptions`. - `imageCompare.spec.ts` pins `ImageCompare` -> `Image Compare`; `nodeBadge.spec.ts` pins `ImageBatch` -> `Batch Images`. - Both install the stub by extending the `page` fixture, so the route is live before `comfyPage.setup()` boots the node-definition store. That is the same pattern as `propertiesPanel/errorsTabModeAware.spec.ts` and avoids the extra `reloadAndWaitForApp()` that `widgetTooltip.spec.ts` needs when routing mid-test. - **Pre-existing bug fix:** `routeObjectInfoFromSetupApi` only wrapped the fetch and JSON parse in a `try`. A throwing `customize` callback rejected the route handler, so `route.fulfill` never ran and the request hung — the test then died on an opaque Playwright timeout rather than reporting the cause. The mutators throw precisely when a node type is missing from `/object_info`, which is what a container bump triggers, so shipping the pins without this would turn "node was renamed upstream" into a mystery hang. `customize` is now guarded and fulfills a 500 naming the failure, with the message logged to test output. No baselines were regenerated. The pinned strings are the ones the committed PNGs already encode, so the images are byte-identical to `main`. ## Review Focus Since #14541, English node titles resolve from the backend `/object_info` rather than from a bundled file. That makes every golden containing a node header a function of the CI container image, which is pinned to `ghcr.io/comfy-org/comfyui-ci-container:0.0.21` (ComfyUI v0.19.3) in four workflows: `ci-tests-e2e.yaml:57` and `:115`, `pr-update-playwright-expectations.yaml:84`, `ci-perf-report.yaml:34`. `0.0.22` (ComfyUI v0.30.0) already exists in the registry, and whoever bumps the pin currently gets unrelated node-title screenshot diffs with nothing in-repo explaining them. This makes that bump safe. The chosen strings are not guesses. #14541 itself re-baselined these goldens, so they encode the container's names, and the rename is visible in the diff of that commit: in `image-compare-default-50`, the title descenders move from `p` at x=60 / `g` at x=112 (bundled `Compare Images`) to `g` at x=56 / `p` at x=98 (container `Image Compare`). `Batch Images` is legible directly in `node-lifecycle-*`. No unit test covers the hang fix: `browser_tests/**` is outside vitest's `include` globs (`src/`, `packages/`, `scripts/`), and widening them would sweep the Playwright `.spec.ts` files into `pnpm test:unit`. Exercising the path for real needs a running ComfyUI backend. Still coupled to the container, deliberately out of scope here: the `api-pricing-*` goldens (title `Flux 1.1 [pro] Ultra Image`, from `FluxProUltraImageNode`) and `node-badge-*` (titles from the `nodes/execution_error` workflow). Same fix applies if we want them covered. Fixes #14630 --------- Co-authored-by: Connor Byrne <c.byrne@comfy.org>
christian-byrne
added a commit
that referenced
this pull request
Aug 5, 2026
#14747) ## Summary Widget labels, socket labels, output names, node descriptions and tooltips now resolve from the live `/object_info` response instead of the bundled `en/nodeDefs.json` snapshot, so a backend rename no longer leaves a new node title beside old widget labels and old tooltips. ## Changes - **What**: `resolveNodeDefInputText()` / `resolveNodeDefOutputText()` (`src/i18n.ts`) extend the precedence introduced in #14541 to slot text. They own all `nodeDefs.*.inputs|outputs.*` key construction and apply the same per-locale order: | locale | order | | ------ | ---------------------------------------------------------------------------------------- | | `en` | custom-node `/api/i18n` -> backend `/object_info` -> bundled snapshot -> caller fallback | | other | custom-node `/api/i18n` -> bundled snapshot -> backend -> caller fallback | The existing `resolveNodeDefText` and the new slot resolvers share one core (`resolveNodeDefPath`), which now takes the message reader as a parameter — the only behavioural difference between a name and a tooltip. - **What**: Call sites moved onto the resolvers: - `litegraphService.addInputWidget` — `widget.label` and the widget's socket `localized_name` - `litegraphService.addInputSocket` — socket `localized_name` - `litegraphService.addOutputs` — output `localized_name` (the `dataTypes.*` branch, used when the output name equals its type, is unchanged) - `NodeTooltip.vue` — input slot, output slot and widget tooltips - `useNodeTooltips.ts` — input slot, output slot and widget tooltips - **What**: `useNodeTooltips.getNodeDescription` no longer re-resolves the description against the snapshot. `getNodeDefs()` has already resolved it, so wrapping it in `st()` let the stale bundled value win. - **Breaking**: None. ## Review Focus - **The raw/compiled split is the risk in this PR.** `scripts/nodeDefLocaleSerializer.ts` escapes `name` and leaves `tooltip` verbatim, because tooltips are read through `stRaw`/`tm()` uncompiled — an escaped `{'@'}` would render literally to the user. `slotMessageReader()` is the single place that picks `st` vs `stRaw`, keyed on the field. Nothing new is escaped at runtime, and no backend value is merged into the vue-i18n message tree. - Widget labels previously always came from the snapshot, whose `inputs.*.name` is the raw input name rather than `display_name` — so a backend `display_name` on an input was never shown. It now is. Where the backend sends no `display_name` the snapshot still applies, which is the existing behaviour. - Tests are verified by mutation, not by inspection, because `st()` swallows a vue-i18n `SyntaxError` and falls back to the raw stored message — a fixture with a throwing character passes either way. The `raw / compiled split` fixture uses `50{'%'} {'@'}`, which compiles cleanly, so `name` -> `50% @` and `tooltip` -> `50{'%'} {'@'}` genuinely diverge. Forcing either reader in both directions fails that test; dropping the `en` backend bypass, the custom-node precedence, or the backend fallback each fails a distinct test; passing `undefined` for the backend value at each of the seven call sites fails the test covering it. Fixes #14631 Co-authored-by: Connor Byrne <c.byrne@comfy.org>
This was referenced Aug 5, 2026
christian-byrne
added a commit
that referenced
this pull request
Aug 5, 2026
…4773) ## Summary `ComfyApp.getNodeDefs()` discards `/object_info` entries that are not objects with a string `name`, so a single malformed def cannot abort node registration for every node. It did so silently, which makes a node vanishing from the UI undiagnosable. Warn once per fetch, naming the discarded keys. ## Why this shape This is the decision half of #14644, which asks whether node definitions should be structurally validated at the API boundary and what should happen to ones that fail. Validation was removed deliberately: #1190 disabled it by default and #4038 removed it outright along with its setting. `/object_info` carries definitions from arbitrary custom nodes whose shapes vary in ways `ComfyNodeDefV1` does not fully capture, so tightening the contract risks silently dropping a node the user installed and expects to see. That posture is kept here. What was actually wrong was not the permissiveness, it was the silence. Naming the dropped keys keeps the permissive behaviour while making the data loss visible, and it is the cheapest way to learn whether real payloads ever reach this path. If telemetry later shows they do, #14644 can be decided with evidence rather than speculation. - **Breaking**: none. Same defs are dropped as before; only the logging changes. ## Review Focus Verified by mutation: removing the warning fails the suite. A second test pins that a well-formed response stays silent, so the warning cannot become unconditional noise. Related: #14644, #14541 --------- Co-authored-by: Connor Byrne <c.byrne@comfy.org>
christian-byrne
added a commit
that referenced
this pull request
Aug 5, 2026
## Summary Finishes what #14744 started: the last two node-title goldens that still track the CI container image are now pinned. No baselines regenerated. ## Changes `browser_tests/tests/nodeBadge.spec.ts` already extended the `page` fixture to stub `/object_info` for `ImageBatch`. That map now also carries: | golden | node type | pinned title | | --- | --- | --- | | `api-pricing-{on,off}-{classic,vue}` | `FluxProUltraImageNode` | `Flux 1.1 [pro] Ultra Image` | | `node-badge-{Show-all,Hide-built-in,None}` | `PreviewImage` | `Preview Image` | The route shape, the `try`/`finally` unroute and the pre-boot install are unchanged — only the mutator body generalises to a loop. `nodes/execution_error` also renders `DevToolsErrorRaiseNode` (`Raise Error`), which is **deliberately not pinned**. That name is defined in this repo at `tools/devtools/nodes/errors.py:76` and copied into `custom_nodes` during CI, so it is not a function of the container image. Pinning it would mask a deliberate in-repo rename. ## Review Focus **Motivation.** Since #14541 English node display names resolve from the backend `/object_info`, so every golden containing a node header is a function of `ghcr.io/comfy-org/comfyui-ci-container`. #14682 moves that pin from `0.0.21` (ComfyUI v0.19.3) to `0.0.22` (v0.30.0) — eleven minors. The goal is that after this lands, a node-title golden that moves on that bump means a real structural change, not an upstream rename. **How the strings were established.** Both are read off the committed PNGs, not guessed: - `Flux 1.1 [pro] Ultra Image` is legible in the title band of `api-pricing-on-classic` and `api-pricing-on-vue`, and matches `src/locales/en/nodeDefs.json`. - `Preview Image` is legible in the title band of all three `node-badge-*` goldens, likewise matching the bundled snapshot. Two independent corroborations that these strings hold on both sides of the bump: 1. #14541 re-baselined only `image-compare-*` and `node-lifecycle-*`. It left `api-pricing-*` and `node-badge-*` byte-identical, so at `0.0.21` the container name already equalled the bundled name. 2. Diffing every golden #14682 regenerates against its committed counterpart, the changed pixels in `node-badge-*` sit at x 454–509 / y 69–81 — an `images` output slot appearing on `PreviewImage`. The title band is byte-identical, and `api-pricing-*` is not regenerated at all. So neither title moves at `0.0.22` either. **Sweep.** All 26 goldens #14682 regenerates were diffed old-vs-new and classified. **None move because of a node display name:** | cause | goldens | | --- | --- | | `SaveImage`/`PreviewImage` gained an `images` output | `colorPalette` ×9, `node-badge-*` ×3, `loadWorkflowInMedia` ×4, `save-image-and-webm-preview`, `lod-comparison-high-quality` | | `control after generate` default `randomize` -> `fixed` | `static-primitive-connected` | | 3D render differences | `load3d-*` ×3 | | sub-5px antialiasing | `interaction` ×3, `lod-comparison-low-quality` | So there is no evidence-backed third site. The remaining exposure is the default-workflow titles (`KSampler`, `CLIP Text Encode (Prompt)`, `Load Checkpoint`, `Save Image`, …), which appear across roughly 37 spec files and 257 goldens. Pinning those would touch most of the screenshot suite for a rename that has not happened, so it is left out; the fixture pattern applies unchanged if one ever does. ## Testing `typecheck`, `lint`, `format`, `knip` pass locally. The stub itself needs a running ComfyUI backend, so CI is the verifier — the proof is that the `nodeBadge` and `imageCompare` screenshot tests stay green with the goldens unmodified, which also exercises the route-install ordering. Related: #14630 (closed by #14744; this is the follow-through) Co-authored-by: Connor Byrne <c.byrne@comfy.org>
This was referenced Aug 5, 2026
christian-byrne
added a commit
that referenced
this pull request
Aug 5, 2026
Reverts five PRs that were merged to `main` without independent review, which is not our process. Backing them out so they can go through review properly. Reverted, newest first: - #14775 — resolve display name and description at read time - #14774 — pin remaining container-coupled node titles - #14773 — warn on dropped malformed node defs - #14747 — widget labels, output names and tooltips from the backend - #14744 — decouple node-title goldens from the CI container **Not reverted:** #14541. It was authored by @bigcat88 and carried a genuine approval before merge, so it followed process. The five above were authored and merged under the same account, and GitHub blocks self-approval, so none of them could receive one. Three had no approval of any kind at merge time. `main` is not branch-protected here, so nothing blocked the merges. That is the gap that let this happen. ## State after this revert All five reverts applied without conflict. The tree returns to the post-14541 state: `resolveNodeDefText` and `setBackendNodeText` remain, `translateNodeDef` resolves `display_name`/`description` inline again, and the slot resolvers, getters, dropped-def warning and golden stubbing are all removed. Verified locally: `pnpm typecheck` clean, `pnpm knip` clean, 156 unit tests pass across the four affected suites. ## Follow-up Each reverted change will be re-opened as its own PR from its original branch, which still exists, for normal review. Nothing is lost. --------- Co-authored-by: Connor Byrne <c.byrne@comfy.org>
christian-byrne
added a commit
that referenced
this pull request
Aug 19, 2026
#14797) #14541 made English `display_name` and `description` come from the backend. Widget labels, output names and tooltips were still read from the stale bundled snapshot, so a renamed node showed a new title beside old labels. Adds `resolveNodeDefInputText` / `resolveNodeDefOutputText` and extracts `resolveNodeDefPath`, so the per-locale precedence table now lives in exactly one place and `resolveNodeDefText` delegates to it. Applied in `litegraphService` (widget labels, socket and output `localized_name`), `NodeTooltip.vue`, and `useNodeTooltips.ts`. Also fixes `getNodeDescription`, which wrapped an already-resolved value in `st()` and let the stale bundle win. ## Review focus **Tooltips must never be escaped or compiled.** They are read through `stRaw`/`tm()`, which returns the message uncompiled, so a literal `{'@'}` would render to the user. One function decides it: ```ts return field === 'tooltip' ? stRaw : st ``` Backend values short-circuit before any reader runs, so a tooltip reaches the compiler by no path. No runtime escaping added; `escapeI18nMessage` stays build-time-only. Coverage was verified by mutation rather than inspection, which matters here: `st()` swallows a vue-i18n `SyntaxError` and returns the raw stored message, so a fixture containing a throwing character passes whether or not the code is correct. The raw/compiled fixture (`50{'%'} {'@'}`) compiles cleanly so that escape hatch cannot mask a defect. Forcing the reader to always-`st` or always-`stRaw` each fails the suite. ## Migration guidance (`node.widgets`) `widget.label` now resolves through the node-def text pipeline (custom-node translations, then the live backend value, then the bundled snapshot) instead of taking the node definition's `display_name` verbatim. The resolved string therefore varies by active locale and by what `/object_info` sends. Extensions must use `widget.name` for widget identity. `widget.label` is display text and was never stable across locales. No type or API shape changed. Fixes #14631 --- Previously merged without review and backed out by #14791. Re-submitting for normal review. Rebased onto post-revert `main`; content unchanged. --------- Co-authored-by: Connor Byrne <c.byrne@comfy.org> Co-authored-by: Alexander Brown <drjkl@comfy.org> Co-authored-by: Amp <amp@ampcode.com> Co-authored-by: DrJKL <DrJKL0424@gmail.com>
comfydesigner
pushed a commit
that referenced
this pull request
Aug 20, 2026
#14797) #14541 made English `display_name` and `description` come from the backend. Widget labels, output names and tooltips were still read from the stale bundled snapshot, so a renamed node showed a new title beside old labels. Adds `resolveNodeDefInputText` / `resolveNodeDefOutputText` and extracts `resolveNodeDefPath`, so the per-locale precedence table now lives in exactly one place and `resolveNodeDefText` delegates to it. Applied in `litegraphService` (widget labels, socket and output `localized_name`), `NodeTooltip.vue`, and `useNodeTooltips.ts`. Also fixes `getNodeDescription`, which wrapped an already-resolved value in `st()` and let the stale bundle win. ## Review focus **Tooltips must never be escaped or compiled.** They are read through `stRaw`/`tm()`, which returns the message uncompiled, so a literal `{'@'}` would render to the user. One function decides it: ```ts return field === 'tooltip' ? stRaw : st ``` Backend values short-circuit before any reader runs, so a tooltip reaches the compiler by no path. No runtime escaping added; `escapeI18nMessage` stays build-time-only. Coverage was verified by mutation rather than inspection, which matters here: `st()` swallows a vue-i18n `SyntaxError` and returns the raw stored message, so a fixture containing a throwing character passes whether or not the code is correct. The raw/compiled fixture (`50{'%'} {'@'}`) compiles cleanly so that escape hatch cannot mask a defect. Forcing the reader to always-`st` or always-`stRaw` each fails the suite. ## Migration guidance (`node.widgets`) `widget.label` now resolves through the node-def text pipeline (custom-node translations, then the live backend value, then the bundled snapshot) instead of taking the node definition's `display_name` verbatim. The resolved string therefore varies by active locale and by what `/object_info` sends. Extensions must use `widget.name` for widget identity. `widget.label` is display text and was never stable across locales. No type or API shape changed. Fixes #14631 --- Previously merged without review and backed out by #14791. Re-submitting for normal review. Rebased onto post-revert `main`; content unchanged. --------- Co-authored-by: Connor Byrne <c.byrne@comfy.org> Co-authored-by: Alexander Brown <drjkl@comfy.org> Co-authored-by: Amp <amp@ampcode.com> Co-authored-by: DrJKL <DrJKL0424@gmail.com>
This was referenced Aug 22, 2026
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
English node display names now come from the backend
/object_inforesponse instead of the bundleden/nodeDefs.jsonsnapshot, so core-node renames take effect without waiting for a frontend locale regeneration.Changes
What:
resolveNodeDefText()(src/i18n.ts) resolves node text in an explicit precedence order, replacing the previous approach of merging escaped backend strings into the vue-i18nenmessage tree.en/api/i18n-> backend/object_info-> bundled snapshot -> node name/api/i18n-> bundled snapshot -> backend/object_info-> node nameEnglish is the source language, so a backend value is data rather than a translation and is returned without being compiled. Raw
/object_infotext is held in a plain map (setBackendNodeText), rebuilt per fetch. Non-English behaviour is unchanged except that it now falls back to the live backend value rather than the stale English snapshot. Custom-node translations still go through the compiler, because the generated custom-node locale pipeline escapes them.What: All
nodeDefs.*key construction moves behind that resolver (app.ts,nodeTitleUtil.ts,useContextMenuTranslation.ts), so node titles, search, the node library and context menus resolve identically.Fix:
escapeI18nMessagemishandled backslash. Since vue-i18n 11 the compiler reads\as an escape introducer, so a backslash before an escaped character swallowed the brace the escaper emits:C:\@homefailed to compile as Invalid linked format, andD:\\outputrendered lossily. Backslash is now doubled (vue-i18n cannot express a literal backslash via interpolation, so{'\'}would escape its own closing quote). Non-string input is guarded, matchingnormalizeI18nKeydirectly above it. This also fixes build-time locale generation for the same inputs.Fix:
nodeCategorieskeys are normalized. They had the same defect as the dotted node-name lookup four lines above: the serializer writesnodeCategories.sync_sowhile the consumer asked fornodeCategories.sync.so, which vue-i18n resolves as a nested path.Fix: The legacy nested custom-locale shape resolves again. Some hand-written custom-node locales nest dotted node ids as objects, which vue-i18n resolves by path traversal.
translateNodeDefwas the last consumer that still reached them, so normalizing alone would have regressed them; both key shapes are now tried.Fix: A non-object
/object_inforesponse no longer throws out ofgetNodeDefs().Revert: One regenerated Playwright golden (
vue-nodes-paned-with-touch-mobile) is restored. Its 640 changed pixels of 285711 are antialiasing on ackpt_namewidget value, with the node title row byte-identical, so it carried no signal from this change.Breaking: None.
Review Focus
vue-i18n@11.4.6; the existing corpus contains zero backslashes across all 13 shipped locales and 7905 strings inen/nodeDefs.json, so a release regeneration produces byte-identical output and only previously-broken inputs change.Known gaps, deliberately not in this PR
routeObjectInfoFromSetupApican stub display names to decouple them; not done here because it cannot be verified without a backend. Follow-up issue to come.stRaw/tm()uncompiled.Original report and diagnosis by @bigcat88, who identified the root cause and the correct precedence rules; the
normalizeI18nKeyfix intranslateNodeDefis his and is kept. Reworked with his permission.Linear: FE-1479