feat: add display name mappings for Essentials tab nodes#9072
Merged
christian-byrne merged 7 commits intomainfrom Feb 22, 2026
Merged
feat: add display name mappings for Essentials tab nodes#9072christian-byrne merged 7 commits intomainfrom
christian-byrne merged 7 commits intomainfrom
Conversation
- Add essentials_category field to zComfyNodeDef schema and ComfyNodeDefImpl - Create static display name mappings for Essentials tab nodes - Integrate display name resolution into NodeLibrarySidebarTab - Support exact name matching for regular nodes and prefix matching for blueprints Amp-Thread-ID: https://ampcode.com/threads/T-019c8326-5356-7604-b9cd-34a6d08e157d
dosubot
bot
added
the
size:L
|
Related Documentation Checked 0 published document(s) in 1 knowledge base(s). No updates required. How did I do? Any feedback? |
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a centralized resolver for "essentials" node display names and updates sidebar components and an essential node card to prefer resolved labels; includes a new locale block and unit tests covering exact names, blueprint prefixes, and unmapped cases. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/constants/essentialsDisplayNames.ts`:
- Around line 9-101: Replace hard-coded English strings used for UI display with
vue-i18n keys: move every user-facing value in EXACT_NAME_MAP and
BLUEPRINT_PREFIX_MAP into locale keys in src/locales/en/main.json, then change
resolveBlueprintDisplayName and resolveEssentialsDisplayName to return t('...')
(or call the i18n instance) for the matching key instead of the literal string;
ensure keys are unique and descriptive (e.g., essentials.loadImage,
essentials.image.inpaint) and update any callers of resolveEssentialsDisplayName
to receive localized strings.
Contributor
Author
|
Do not merge, tweaking some things |
christian-byrne
commented
Feb 22, 2026
- Localize all display name strings via vue-i18n (essentials.* keys) - Replace CLIPTextEncode with PrimitiveStringMultiline (per ComfyUI PR #12553) - Replace deprecated ImageBatch with BatchImagesNode - Fix 'image compare' node name to 'ImageCompare' - Remove ImageBlur (should use subgraph blueprint) - Replace GetVideoComponents with 'Video Slice' (correct node_id) - Add 3D API node alternatives (Meshy, Tripo providers) - Add KlingLipSyncTextToVideoNode alternative - Remove emoji from code comments - Add browser test for essentials display names Amp-Thread-ID: https://ampcode.com/threads/T-019c8340-4da2-723b-a09f-83895c5bbda5
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
browser_tests/tests/sidebar/essentialsDisplayNames.spec.ts (1)
9-12: Consider validating response status for clearer failure diagnostics.If
/object_infofails or returns non-JSON, the test will fail with a confusing JSON parsing error rather than a clear HTTP status error.💡 Optional improvement for clearer failures
const objectInfo = await comfyPage.page.evaluate(async () => { const response = await fetch('/object_info') + if (!response.ok) { + throw new Error(`/object_info failed with status ${response.status}`) + } return response.json() })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@browser_tests/tests/sidebar/essentialsDisplayNames.spec.ts` around lines 9 - 12, The fetch call inside comfyPage.page.evaluate that assigns objectInfo should validate the HTTP response before parsing JSON: check response.ok (or response.status) and, if not ok, throw or return a clear error containing response.status and response.statusText so the test fails with an explicit HTTP error rather than a JSON parse error; update the evaluate callback that calls fetch('/object_info') to perform this check and surface the status in the thrown/returned message so failures reference objectInfo fetching and the HTTP status.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@browser_tests/tests/sidebar/essentialsDisplayNames.spec.ts`:
- Around line 9-12: The fetch call inside comfyPage.page.evaluate that assigns
objectInfo should validate the HTTP response before parsing JSON: check
response.ok (or response.status) and, if not ok, throw or return a clear error
containing response.status and response.statusText so the test fails with an
explicit HTTP error rather than a JSON parse error; update the evaluate callback
that calls fetch('/object_info') to perform this check and surface the status in
the thrown/returned message so failures reference objectInfo fetching and the
HTTP status.
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
browser_tests/tests/sidebar/essentialsDisplayNames.spec.ts (1)
6-33: Deduplicate/object_infofetch logic with a helper.
The fetch + error handling is repeated in two tests; a small helper keeps the logic consistent and easier to update.♻️ Proposed refactor
-import { expect } from '@playwright/test' +import { expect } from '@playwright/test' +import type { Page } from '@playwright/test' import { comfyPageFixture as test } from '../../fixtures/ComfyPage' +async function fetchObjectInfo(page: Page) { + return page.evaluate(async () => { + const response = await fetch('/object_info') + if (!response.ok) { + throw new Error(`/object_info failed with status ${response.status}`) + } + return response.json() + }) +} + test.describe('Essentials display names', () => { test('backend serves essentials_category on nodes via object_info', async ({ comfyPage }) => { - const objectInfo = await comfyPage.page.evaluate(async () => { - const response = await fetch('/object_info') - if (!response.ok) { - throw new Error(`/object_info failed with status ${response.status}`) - } - return response.json() - }) + const objectInfo = await fetchObjectInfo(comfyPage.page) @@ test('mapped nodes exist in object_info', async ({ comfyPage }) => { - const objectInfo = await comfyPage.page.evaluate(async () => { - const response = await fetch('/object_info') - if (!response.ok) { - throw new Error(`/object_info failed with status ${response.status}`) - } - return response.json() - }) + const objectInfo = await fetchObjectInfo(comfyPage.page)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@browser_tests/tests/sidebar/essentialsDisplayNames.spec.ts` around lines 6 - 33, Extract the repeated fetch + error handling into a shared helper (e.g., getObjectInfo or fetchObjectInfo) and use it in both tests ('backend serves essentials_category on nodes via object_info' and 'mapped nodes exist in object_info') to avoid duplication; the helper should call comfyPage.page.evaluate, fetch('/object_info'), throw an Error when response.ok is false, and return response.json(), then replace the inline comfyPage.page.evaluate blocks in both tests with a call to this helper.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@browser_tests/tests/sidebar/essentialsDisplayNames.spec.ts`:
- Around line 6-33: Extract the repeated fetch + error handling into a shared
helper (e.g., getObjectInfo or fetchObjectInfo) and use it in both tests
('backend serves essentials_category on nodes via object_info' and 'mapped nodes
exist in object_info') to avoid duplication; the helper should call
comfyPage.page.evaluate, fetch('/object_info'), throw an Error when response.ok
is false, and return response.json(), then replace the inline
comfyPage.page.evaluate blocks in both tests with a call to this helper.
…ame-mappings-for-essentials-tab Amp-Thread-ID: https://ampcode.com/threads/T-019c835a-3490-7018-809e-a734bd0ddc67 # Conflicts: # src/schemas/nodeDefSchema.ts # src/stores/nodeDefStore.ts
🎨 Storybook: ✅ Built — View Storybook |
🎭 Playwright: ✅ 533 passed, 0 failed · 1 flaky📊 Browser Reports
|
📦 Bundle: 4.37 MB gzip 🔴 +1.2 kBDetailsSummary
Category Glance App Entry Points — 21.6 kB (baseline 21.6 kB) • ⚪ 0 BMain entry bundles and manifests
Status: 1 added / 1 removed Graph Workspace — 951 kB (baseline 951 kB) • ⚪ 0 BGraph editor runtime, canvas, workflow orchestration
Status: 1 added / 1 removed Views & Navigation — 68.8 kB (baseline 68.8 kB) • ⚪ 0 BTop-level views, pages, and routed surfaces
Status: 9 added / 9 removed Panels & Settings — 436 kB (baseline 436 kB) • ⚪ 0 BConfiguration panels, inspectors, and settings screens
Status: 10 added / 10 removed User & Accounts — 16 kB (baseline 16 kB) • ⚪ 0 BAuthentication, profile, and account management bundles
Status: 5 added / 5 removed Editors & Dialogs — 738 B (baseline 738 B) • ⚪ 0 BModals, dialogs, drawers, and in-app editors
Status: 1 added / 1 removed UI Components — 47 kB (baseline 47 kB) • ⚪ 0 BReusable component library chunks
Status: 5 added / 5 removed Data & Services — 2.51 MB (baseline 2.51 MB) • 🔴 +2.6 kBStores, services, APIs, and repositories
Status: 13 added / 13 removed Utilities & Hooks — 58.3 kB (baseline 58.3 kB) • ⚪ 0 BHelpers, composables, and utility bundles
Status: 13 added / 13 removed Vendor & Third-Party — 8.83 MB (baseline 8.83 MB) • ⚪ 0 BExternal libraries and shared vendor chunks
Other — 7.6 MB (baseline 7.6 MB) • 🔴 +1.21 kBBundles that do not match a named category
Status: 51 added / 51 removed |
- Add resolveEssentialsDisplayName to NodeLibrarySidebarTabV2's fillNodeInfo - Update EssentialNodeCard to use node.label (set by fillNodeInfo) - Remove E2E tests (CI backend lacks essentials_category, V1 fixture incompatible with V2 default) Amp-Thread-ID: https://ampcode.com/threads/T-019c835a-3490-7018-809e-a734bd0ddc67
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/sidebar/tabs/NodeLibrarySidebarTabV2.vue`:
- Line 115: The essentials display-name mapping is being applied globally
because fillNodeInfo is used by all tabs; restrict it to the Essentials tab by
adding an explicit flag (e.g., isEssentialsTab) to fillNodeInfo's signature and
call sites in NodeLibrarySidebarTabV2 so mapping via
resolveEssentialsDisplayName only runs when isEssentialsTab is true; also ensure
fillNodeInfo safely falls back if node.data is missing (use a default label or
node.name) to avoid runtime errors when resolveEssentialsDisplayName expects
node.data (apply the same guarded change where similar logic exists around the
lines referencing resolveEssentialsDisplayName).
benceruleanlu
approved these changes
Feb 22, 2026
christian-byrne
deleted the
toolkit/add-display-name-mappings-for-essentials-tab
branch
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
3 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
Add frontend-only display name mappings for nodes shown in the Essentials tab, plus parse the new
essentials_categoryfield from the backend.Changes
src/constants/essentialsDisplayNames.tswith a static mapping of node names to user-friendly display names (e.g.CLIPTextEncode→ "Text",ImageScale→ "Resize Image"). Regular nodes use exact name matching; blueprint nodes use prefix matching since their filenames include model-specific suffixes. Integrated intoNodeLibrarySidebarTab.vue'srenderedRootcomputed for leaf node labels with fallback todisplay_name. Addedessentials_category(z.string().optional()) to the node def schema andComfyNodeDefImplto parse the field already sent by the backend (PR #12357).Review Focus
Display names are resolved only in the Essentials tab tree view (
NodeLibrarySidebarTab.vue), not globally, to avoid side effects on search, bookmarks, or other views. Blueprint prefix matching is ordered longest-first so more specific prefixes (e.g.image_inpainting_) match before shorter ones (e.g.image_edit).┆Issue is synchronized with this Notion page by Unito