Custom nodes example workflows index#9284
Open
lerignoux wants to merge 2 commits intoComfy-Org:mainfrom
Open
Conversation
lerignoux
requested review from
a team,
Myestery,
christian-byrne and
comfyui-wiki
as code owners
dosubot
Bot
added
the
size:L
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds indexed custom template metadata fetching and enrichment, decouples thumbnail index suffixing from sourceModule, detects templates that use two thumbnails ( Changes
Sequence DiagramsequenceDiagram
participant UI as WorkflowTemplateSelectorDialog
participant Store as workflowTemplatesStore
participant Fetch as fetchIndexedCustomTemplates
participant API as ComfyApi
participant Server as /api/workflow_templates/{module}/index(.{locale}).json
UI->>Store: loadWorkflowTemplates()
Store->>Fetch: identify customTemplates needing index
Fetch->>API: getCustomIndexWorkflowTemplates(moduleName, locale)
API->>Server: GET /api/workflow_templates/{module}/index(.{locale}).json
Server-->>API: return JSON or non-JSON / error
API-->>Fetch: indexed data or fallback
Fetch->>Store: populate customNodesIndexedTemplates
Store->>Store: getCustomTemplateInfo() → enrich templates
Store-->>UI: enhancedTemplates (with thumbnailVariant, titles, descriptions)
UI->>UI: usesTwoThumbnails() decides thumbnail indices for display
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🎭 Playwright: ⏳ Running... |
🎨 Storybook:
|
This was referenced Feb 27, 2026
![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: 2
🤖 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/platform/workflow/templates/repositories/workflowTemplatesStore.ts`:
- Line 47: Replace the unnecessary any type on the find callbacks over
indexed.templates: remove "(t: any)" so TypeScript infers the element type from
indexed: WorkflowTemplates (indexed.templates is TemplateInfo[]), or explicitly
annotate with TemplateInfo if you prefer; update both occurrences (the find at
the reference to name and the other at line ~249) to eliminate the any and rely
on TemplateInfo instead.
In `@src/scripts/api.ts`:
- Around line 773-777: Update getCustomIndexWorkflowTemplates to accept the
locale argument and return a single WorkflowTemplates (not an array) and add
try/catch with locale-aware fallbacks like getCoreWorkflowTemplates: change the
signature to getCustomIndexWorkflowTemplates(customNodeName: string, locale:
string): Promise<WorkflowTemplates>, wrap the axios.get call in a try/catch,
attempt the locale-specific path first
(/api/workflow_templates/{customNodeName}/{locale}/index.json or by appending
locale similarly to getCoreWorkflowTemplates), fall back to the generic
/api/workflow_templates/{customNodeName}/index.json on failure, and on total
failure return an empty WorkflowTemplates object (or the same default used by
getCoreWorkflowTemplates) while logging the error.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/components/custom/widget/WorkflowTemplateSelectorDialog.vuesrc/platform/workflow/templates/composables/useTemplateWorkflows.tssrc/platform/workflow/templates/repositories/workflowTemplatesStore.tssrc/scripts/api.ts
This was referenced Feb 27, 2026
lerignoux
force-pushed
the
custom_nodes_example_workflows_index
branch
2 times, most recently
from
cbd1f3e to
4419415
Compare
lerignoux
commented
Feb 27, 2026
| template.sourceModule || 'default' | ||
|
|
||
| const usesTwoThumbnails = (template: TemplateInfo) => | ||
| template.thumbnailVariant === 'compareSlider' || template.thumbnailVariant === 'hoverDissolve' |
Contributor
Author
There was a problem hiding this comment.
Should I test if not null instead in case we have more variant later on ?
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/platform/workflow/templates/repositories/workflowTemplatesStore.ts (2)
246-262: Type mismatch:indexedDatainitialized as{}but used asTemplateInfo | undefined.The variable
indexedDatais initialized as an empty object{}but may be assignedTemplateInfo | undefinedfrom thefind()call. When the find returnsundefined, the optional chaining (indexedData?.title) correctly handles it, but whenindexedDatais{}, expressions likeindexedData?.titlewill returnundefined(correct) whileindexedData?.mediaSubtype || 'jpg'will also work. However, the type inference is loose.Consider explicitly typing and initializing to
undefinedfor clarity:♻️ Suggested type-safe initialization
- let indexedData = {} - if (indexed && Array.isArray(indexed.templates)) { - indexedData = indexed.templates.find((t) => t.name === name) - } + let indexedData: TemplateInfo | undefined + if (indexed && Array.isArray(indexed.templates)) { + indexedData = indexed.templates.find((t) => t.name === name) + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/platform/workflow/templates/repositories/workflowTemplatesStore.ts` around lines 246 - 262, indexedData is initialized to {} but is later treated as TemplateInfo | undefined; change its declaration to be explicitly typed and initialized as undefined (e.g., let indexedData: TemplateInfo | undefined = undefined) and assign the result of indexed.templates.find(...) to it (keep the existing optional chaining uses like indexedData?.title). Update the variable near customNodesIndexedTemplates.value[moduleName] and the EnhancedTemplate construction that references indexedData so TypeScript infers the correct union type instead of a loose object.
501-512: Consider error handling for individual custom node index fetches.If
api.getCustomIndexWorkflowTemplatesfails for one custom node, the entirefetchIndexedCustomTemplatesfunction will throw, potentially leaving the store in an inconsistent state. Consider wrapping the API call in a try/catch to gracefully handle failures for individual nodes while continuing to process others.♻️ Add per-node error handling
async function fetchIndexedCustomTemplates(customTemplatesObj: { [moduleName: string]: string[] }) { const locale = i18n.global.locale.value const customIndexedTemplates: { [moduleName: string]: WorkflowTemplates } = {} const updatedCustomTemplates: { [moduleName: string]: string[] } = { ...customTemplatesObj } for (const [customNodeName, workflowsList] of Object.entries(customTemplatesObj) as [string, string[]][]) { if (workflowsList.includes('index')) { - customIndexedTemplates[customNodeName] = await api.getCustomIndexWorkflowTemplates(customNodeName, locale) - updatedCustomTemplates[customNodeName] = workflowsList.filter(name => name !== 'index') + try { + customIndexedTemplates[customNodeName] = await api.getCustomIndexWorkflowTemplates(customNodeName, locale) + updatedCustomTemplates[customNodeName] = workflowsList.filter(name => name !== 'index') + } catch (error) { + console.error(`Error fetching indexed templates for ${customNodeName}:`, error) + // Keep original workflow list without 'index' on failure + updatedCustomTemplates[customNodeName] = workflowsList.filter(name => name !== 'index') + } } } return { customIndexedTemplates, updatedCustomTemplates } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/platform/workflow/templates/repositories/workflowTemplatesStore.ts` around lines 501 - 512, fetchIndexedCustomTemplates currently lets any failure from api.getCustomIndexWorkflowTemplates abort the whole loop; wrap the per-node API call inside a try/catch within the for-loop (reference fetchIndexedCustomTemplates and api.getCustomIndexWorkflowTemplates) so a single node failure is caught, logged (or recorded), and the loop continues; ensure that when a node's index fetch fails you still set updatedCustomTemplates[customNodeName] (e.g., leave 'index' in or remove based on desired behavior) and only add successful results into customIndexedTemplates before returning the pair of customIndexedTemplates and updatedCustomTemplates.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/platform/workflow/templates/repositories/workflowTemplatesStore.ts`:
- Around line 246-262: indexedData is initialized to {} but is later treated as
TemplateInfo | undefined; change its declaration to be explicitly typed and
initialized as undefined (e.g., let indexedData: TemplateInfo | undefined =
undefined) and assign the result of indexed.templates.find(...) to it (keep the
existing optional chaining uses like indexedData?.title). Update the variable
near customNodesIndexedTemplates.value[moduleName] and the EnhancedTemplate
construction that references indexedData so TypeScript infers the correct union
type instead of a loose object.
- Around line 501-512: fetchIndexedCustomTemplates currently lets any failure
from api.getCustomIndexWorkflowTemplates abort the whole loop; wrap the per-node
API call inside a try/catch within the for-loop (reference
fetchIndexedCustomTemplates and api.getCustomIndexWorkflowTemplates) so a single
node failure is caught, logged (or recorded), and the loop continues; ensure
that when a node's index fetch fails you still set
updatedCustomTemplates[customNodeName] (e.g., leave 'index' in or remove based
on desired behavior) and only add successful results into customIndexedTemplates
before returning the pair of customIndexedTemplates and updatedCustomTemplates.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/components/custom/widget/WorkflowTemplateSelectorDialog.vuesrc/platform/workflow/templates/composables/useTemplateWorkflows.tssrc/platform/workflow/templates/repositories/workflowTemplatesStore.tssrc/scripts/api.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/scripts/api.ts
lerignoux
force-pushed
the
custom_nodes_example_workflows_index
branch
2 times, most recently
from
129f894 to
190d249
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/platform/workflow/templates/repositories/workflowTemplatesStore.ts (1)
554-564:⚠️ Potential issue | 🟠 MajorCustom indexed templates are not re-fetched when locale changes.
The locale watcher at lines 554-564 only calls
fetchCoreTemplates(). However,fetchIndexedCustomTemplates()(lines 505-516) accepts and uses the current locale when fetching custom node indices—passinglocaletoapi.getCustomIndexWorkflowTemplates()(line 511), which supports locale-specific files (e.g.,index.de.json).When users switch languages, custom indexed templates remain in the original locale. Update the locale watcher to also re-fetch custom indexed templates:
watch( () => i18n.global.locale.value, async () => { if (!isLoaded.value) return try { const fetchedCustomTemplates = await api.getWorkflowTemplates() const { customIndexedTemplates, updatedCustomTemplates } = await fetchIndexedCustomTemplates(fetchedCustomTemplates) customNodesIndexedTemplates.value = customIndexedTemplates customTemplates.value = updatedCustomTemplates await fetchCoreTemplates() } catch (error) { console.error('Error reloading templates for new locale:', error) } } )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/platform/workflow/templates/repositories/workflowTemplatesStore.ts` around lines 554 - 564, The locale watcher currently only calls fetchCoreTemplates() and therefore doesn't refresh locale-specific custom indexed templates; update the watcher (the callback watching i18n.global.locale.value) to also call fetchIndexedCustomTemplates(fetchedCustomTemplates) (or the helper that obtains custom indices via api.getCustomIndexWorkflowTemplates) and assign its results to customNodesIndexedTemplates.value and customTemplates.value before (or along with) calling fetchCoreTemplates(), keeping the existing isLoaded check and try/catch error logging around the whole sequence.
🧹 Nitpick comments (2)
src/platform/workflow/templates/repositories/workflowTemplatesStore.ts (1)
505-516: Consider parallelizing index fetches for better performance.The current implementation fetches index files sequentially in a
for...ofloop. If multiple custom nodes have index files, this could be slow. Consider usingPromise.allfor parallel fetching.Proposed refactor
async function fetchIndexedCustomTemplates(customTemplatesObj: { [moduleName: string]: string[] }) { const locale = i18n.global.locale.value const customIndexedTemplates: { [moduleName: string]: WorkflowTemplates } = {} const updatedCustomTemplates: { [moduleName: string]: string[] } = { ...customTemplatesObj } - for (const [customNodeName, workflowsList] of Object.entries(customTemplatesObj) as [string, string[]][]) { - if (workflowsList.includes('index')) { - customIndexedTemplates[customNodeName] = await api.getCustomIndexWorkflowTemplates(customNodeName, locale) - updatedCustomTemplates[customNodeName] = workflowsList.filter(name => name !== 'index') - } - } + const modulesWithIndex = Object.entries(customTemplatesObj) + .filter(([, workflowsList]) => workflowsList.includes('index')) + + const results = await Promise.all( + modulesWithIndex.map(async ([customNodeName, workflowsList]) => { + const indexed = await api.getCustomIndexWorkflowTemplates(customNodeName, locale) + return { customNodeName, indexed, filtered: workflowsList.filter(name => name !== 'index') } + }) + ) + + for (const { customNodeName, indexed, filtered } of results) { + customIndexedTemplates[customNodeName] = indexed + updatedCustomTemplates[customNodeName] = filtered + } return { customIndexedTemplates, updatedCustomTemplates } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/platform/workflow/templates/repositories/workflowTemplatesStore.ts` around lines 505 - 516, The fetchIndexedCustomTemplates function currently awaits api.getCustomIndexWorkflowTemplates sequentially; change it to perform parallel fetches by mapping Object.entries(customTemplatesObj) to an array of promises (calling api.getCustomIndexWorkflowTemplates for entries whose workflowsList includes 'index'), await Promise.all, then assemble customIndexedTemplates and updatedCustomTemplates from the resolved results; keep the same keys and locale (i18n.global.locale.value) and ensure updatedCustomTemplates still filters out 'index' from workflowsList.src/components/custom/widget/WorkflowTemplateSelectorDialog.vue (1)
495-503: Usenullinstead of empty string for the index parameter.Lines 497 and 502 pass an empty string
''as theindexparameter togetTemplateThumbnailUrl. Since the function signature defaults tonulland uses a truthy check (index ?-${index}: ''), passingnullwould be more explicit and better align with the parameter's design intention.const getBaseThumbnailSrc = (template: TemplateInfo) => { const sm = getEffectiveSourceModule(template) return getTemplateThumbnailUrl(template, sm, sm === 'default' || usesTwoThumbnails(template) ? '1' : null) } const getOverlayThumbnailSrc = (template: TemplateInfo) => { const sm = getEffectiveSourceModule(template) return getTemplateThumbnailUrl(template, sm, sm === 'default' || usesTwoThumbnails(template) ? '2' : null) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/custom/widget/WorkflowTemplateSelectorDialog.vue` around lines 495 - 503, Update the two callers of getTemplateThumbnailUrl in getBaseThumbnailSrc and getOverlayThumbnailSrc to pass null instead of an empty string for the index argument when the conditional is false; specifically, keep passing '1' or '2' when (sm === 'default' || usesTwoThumbnails(template)) is true, otherwise pass null so the function's default/truthy check behaves as intended and aligns with its signature.
🤖 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/platform/workflow/templates/repositories/workflowTemplatesStore.ts`:
- Around line 246-257: The variable indexedData lacks a proper type so
TypeScript can't see properties like
title/description/mediaSubtype/thumbnailVariant; change the declaration of
indexedData in the block that reads from customNodesIndexedTemplates (the
variable named indexedData used when building the EnhancedTemplate) to a typed
shape such as Partial<EnhancedTemplate> or an explicit interface ({ title?:
string; description?: string; mediaSubtype?: string; thumbnailVariant?: string
}) and initialize it to undefined or {} as that type so the subsequent
indexed.templates.find(...) result is typed correctly and the optional chaining
accesses compile.
---
Outside diff comments:
In `@src/platform/workflow/templates/repositories/workflowTemplatesStore.ts`:
- Around line 554-564: The locale watcher currently only calls
fetchCoreTemplates() and therefore doesn't refresh locale-specific custom
indexed templates; update the watcher (the callback watching
i18n.global.locale.value) to also call
fetchIndexedCustomTemplates(fetchedCustomTemplates) (or the helper that obtains
custom indices via api.getCustomIndexWorkflowTemplates) and assign its results
to customNodesIndexedTemplates.value and customTemplates.value before (or along
with) calling fetchCoreTemplates(), keeping the existing isLoaded check and
try/catch error logging around the whole sequence.
---
Nitpick comments:
In `@src/components/custom/widget/WorkflowTemplateSelectorDialog.vue`:
- Around line 495-503: Update the two callers of getTemplateThumbnailUrl in
getBaseThumbnailSrc and getOverlayThumbnailSrc to pass null instead of an empty
string for the index argument when the conditional is false; specifically, keep
passing '1' or '2' when (sm === 'default' || usesTwoThumbnails(template)) is
true, otherwise pass null so the function's default/truthy check behaves as
intended and aligns with its signature.
In `@src/platform/workflow/templates/repositories/workflowTemplatesStore.ts`:
- Around line 505-516: The fetchIndexedCustomTemplates function currently awaits
api.getCustomIndexWorkflowTemplates sequentially; change it to perform parallel
fetches by mapping Object.entries(customTemplatesObj) to an array of promises
(calling api.getCustomIndexWorkflowTemplates for entries whose workflowsList
includes 'index'), await Promise.all, then assemble customIndexedTemplates and
updatedCustomTemplates from the resolved results; keep the same keys and locale
(i18n.global.locale.value) and ensure updatedCustomTemplates still filters out
'index' from workflowsList.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/components/custom/widget/WorkflowTemplateSelectorDialog.vuesrc/platform/workflow/templates/composables/useTemplateWorkflows.tssrc/platform/workflow/templates/repositories/workflowTemplatesStore.tssrc/scripts/api.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- src/scripts/api.ts
- src/platform/workflow/templates/composables/useTemplateWorkflows.ts
lerignoux
force-pushed
the
custom_nodes_example_workflows_index
branch
from
190d249 to
1981a0d
Compare
Contributor
Author
|
Hello Sorry to come back on this, @christian-byrne Would this be a more suitable fix for the thumbnail variant in custom nodes than the previous MR ? PS: I see some old code rabbits comments. but It seems they are not relevant anymore, I applied some of them, should I do anything with them ? |
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
1 participant
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
Follow up from previous Merge Request taking into account the blocked status and feedback comment:
Cf #3684
I am allowing custom nodes to add an index.json file with a format similar to the core workflows to defined the workflows description, title, mediaSubtype and thumbnailVariant
Changes
index.jsonReview Focus
Went initially for a lean change, cf #3682
Trying now to make a unification change making custom nodes work like the core workflows
Was helped by AI to understand the project structure and change it.
Screenshots (if applicable)
┆Issue is synchronized with this Notion page by Unito