-
Notifications
You must be signed in to change notification settings - Fork 440
Fix workflow loading from PNG images with both workflow and parameter… #7154
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…s metadata - Reorder handleFile() to check workflow before parameters - Add validation to prevent JSON parse errors from crashing imports - Fix loadGraphData() to use explicit type validation instead of falsy check - Ensures ComfyUI-generated PNGs with both metadata types load the workflow, not parameters Fixes issue where large workflows (e.g., 634 nodes) were replaced with basic A1111 format when importing PNG files.
📝 WalkthroughWalkthroughRefactors file-handling in Changes
Possibly related issues
Pre-merge checks and finishing touches✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: ASSERTIVE Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (11)**/*.{vue,ts,tsx}📄 CodeRabbit inference engine (.cursorrules)
Files:
**/*.{ts,tsx,js}📄 CodeRabbit inference engine (.cursorrules)
Files:
**/*.{ts,tsx}📄 CodeRabbit inference engine (.cursorrules)
Files:
**/*.{ts,tsx,js,vue}📄 CodeRabbit inference engine (.cursorrules)
Files:
src/**/*.{vue,ts}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
src/**/*.ts📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
**/*.{ts,tsx,js,jsx,vue}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
**/*.{ts,tsx,vue}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
**/*.ts📄 CodeRabbit inference engine (CLAUDE.md)
Files:
src/**/*.{ts,tsx,vue}📄 CodeRabbit inference engine (src/CLAUDE.md)
Files:
src/**/*.{vue,ts,tsx}📄 CodeRabbit inference engine (src/CLAUDE.md)
Files:
🧠 Learnings (4)📚 Learning: 2025-11-24T19:47:56.371ZApplied to files:
📚 Learning: 2025-11-24T19:47:56.371ZApplied to files:
📚 Learning: 2025-11-24T19:47:02.860ZApplied to files:
📚 Learning: 2025-11-24T19:46:52.279ZApplied to files:
🧬 Code graph analysis (1)src/scripts/app.ts (2)
Comment |
🎭 Playwright Test Results⏰ Completed at: 12/04/2025, 04:47:17 PM UTC 📈 Summary
📊 Test Reports by Browser
🎉 Click on the links above to view detailed test results for each browser configuration. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR fixes a critical issue where PNG files containing both workflow and parameters metadata (commonly generated by ComfyUI) would incorrectly load the A1111 format parameters instead of the full workflow, resulting in data loss for large workflows.
Key changes:
- Reordered metadata checks in
handleFile()to prioritize workflow over parameters - Added JSON parsing error handling and validation for workflow objects
- Enhanced type validation in
loadGraphData()to prevent replacing valid falsy values
Comments suppressed due to low confidence (1)
src/scripts/app.ts:1488
- The
prompthandling (line 1485) lacks error handling for JSON parsing, unlike the improvedworkflowhandling above. Ifpromptis a string containing invalid JSON,JSON.parse(prompt)will throw an uncaught error and crash the import process.
Consider adding similar try-catch error handling:
if (prompt) {
let promptObj
try {
promptObj = typeof prompt === 'string' ? JSON.parse(prompt) : prompt
} catch (err) {
console.error('Failed to parse prompt:', err)
this.showErrorOnFileLoad(file)
return
}
this.loadApiJson(promptObj, fileName)
return
} if (prompt) {
const promptObj = typeof prompt === 'string' ? JSON.parse(prompt) : prompt
this.loadApiJson(promptObj, fileName)
return
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
src/scripts/app.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (11)
**/*.{vue,ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{vue,ts,tsx}: Leverage VueUse functions for performance-enhancing utilities
Use vue-i18n in Composition API for any string literals and place new translation entries in src/locales/en/main.json
Files:
src/scripts/app.ts
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursorrules)
Use es-toolkit for utility functions
Files:
src/scripts/app.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
Use TypeScript for type safety
**/*.{ts,tsx}: Never useanytype - use proper TypeScript types
Never useas anytype assertions - fix the underlying type issue
Files:
src/scripts/app.ts
**/*.{ts,tsx,js,vue}
📄 CodeRabbit inference engine (.cursorrules)
Implement proper error handling in components and services
**/*.{ts,tsx,js,vue}: Use 2-space indentation, single quotes, no semicolons, and maintain 80-character line width as configured in.prettierrc
Organize imports by sorting and grouping by plugin, and runpnpm formatbefore committing
Files:
src/scripts/app.ts
src/**/*.{vue,ts}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.{vue,ts}: Leverage VueUse functions for performance-enhancing styles
Implement proper error handling
Use vue-i18n in composition API for any string literals. Place new translation entries in src/locales/en/main.json
Files:
src/scripts/app.ts
src/**/*.ts
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.ts: Use es-toolkit for utility functions
Use TypeScript for type safety
Files:
src/scripts/app.ts
**/*.{ts,tsx,js,jsx,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
Use camelCase for variable and setting names in TypeScript/Vue files
Files:
src/scripts/app.ts
**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,vue}: Useconst settingStore = useSettingStore()andsettingStore.get('Comfy.SomeSetting')to retrieve settings in TypeScript/Vue files
Useawait settingStore.set('Comfy.SomeSetting', newValue)to update settings in TypeScript/Vue files
Check server capabilities usingapi.serverSupportsFeature('feature_name')before using enhanced features
Useapi.getServerFeature('config_name', defaultValue)to retrieve server feature configurationEnforce ESLint rules for Vue + TypeScript including: no floating promises, no unused imports, and i18n raw text restrictions in templates
Files:
src/scripts/app.ts
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.ts: Define dynamic setting defaults using runtime context with functions in settings configuration
UsedefaultsByInstallVersionproperty for gradual feature rollout based on version in settings configuration
Files:
src/scripts/app.ts
src/**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (src/CLAUDE.md)
src/**/*.{ts,tsx,vue}: Sanitize HTML with DOMPurify to prevent XSS attacks
Avoid using @ts-expect-error; use proper TypeScript types instead
Use es-toolkit for utility functions instead of other utility libraries
Implement proper TypeScript types throughout the codebase
Files:
src/scripts/app.ts
src/**/*.{vue,ts,tsx}
📄 CodeRabbit inference engine (src/CLAUDE.md)
Follow Vue 3 composition API style guide
Files:
src/scripts/app.ts
🧬 Code graph analysis (1)
src/scripts/app.ts (2)
src/stores/queueStore.ts (1)
workflow(314-316)src/platform/workflow/validation/schemas/workflowSchema.ts (1)
ComfyWorkflowJSON(460-462)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Agent
- GitHub Check: CodeQL analysis (javascript-typescript)
🔇 Additional comments (2)
src/scripts/app.ts (2)
1441-1469: LGTM! Good fix for workflow priority handling.The changes correctly ensure workflow metadata takes priority over parameters when both are present in PNG files. The error handling with try-catch for JSON.parse and explicit structure validation provides good robustness against malformed metadata.
1471-1482: LGTM!Parameters are now correctly used as a fallback when no workflow exists, preserving backward compatibility with A1111-only PNG files.
Bundle Size ReportSummary
Category Glance Per-category breakdownApp Entry Points — 3.2 MB (baseline 3.2 MB) • 🔴 +489 BMain entry bundles and manifests
Status: 3 added / 3 removed Graph Workspace — 972 kB (baseline 972 kB) • ⚪ 0 BGraph editor runtime, canvas, workflow orchestration
Status: 1 added / 1 removed Views & Navigation — 6.54 kB (baseline 6.54 kB) • ⚪ 0 BTop-level views, pages, and routed surfaces
Status: 1 added / 1 removed Panels & Settings — 298 kB (baseline 298 kB) • ⚪ 0 BConfiguration panels, inspectors, and settings screens
Status: 6 added / 6 removed UI Components — 173 kB (baseline 173 kB) • ⚪ 0 BReusable component library chunks
Status: 6 added / 6 removed Data & Services — 12.5 kB (baseline 12.5 kB) • ⚪ 0 BStores, services, APIs, and repositories
Status: 2 added / 2 removed Utilities & Hooks — 2.94 kB (baseline 2.94 kB) • ⚪ 0 BHelpers, composables, and utility bundles
Status: 1 added / 1 removed Vendor & Third-Party — 8.56 MB (baseline 8.56 MB) • ⚪ 0 BExternal libraries and shared vendor chunks
Other — 3.81 MB (baseline 3.81 MB) • ⚪ 0 BBundles that do not match a named category
Status: 17 added / 17 removed |
Co-authored-by: Alexander Brown <DrJKL0424@gmail.com>
Refactor workflow object handling to allow undefined type and improve validation checks.
Head branch was pushed to by a user without write access
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/scripts/app.ts (1)
1060-1066: Comment incorrectly describes JS falsy values; clarify intent of validationEmpty objects
{}are truthy in JavaScript, so the current comment is misleading. The intent here is to ensuregraphDatais a non-null object (and not a primitive or array), not to preserve “falsy” values.Consider updating the comment to something like:
- // Use explicit validation instead of falsy check to avoid replacing - // valid but falsy values (empty objects, 0, false, etc.) + // Use explicit validation instead of a generic falsy check to ensure + // graphData is a plain object (not null, undefined, a primitive, or an + // array)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
src/scripts/app.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (11)
**/*.{vue,ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{vue,ts,tsx}: Leverage VueUse functions for performance-enhancing utilities
Use vue-i18n in Composition API for any string literals and place new translation entries in src/locales/en/main.json
Files:
src/scripts/app.ts
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursorrules)
Use es-toolkit for utility functions
Files:
src/scripts/app.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
Use TypeScript for type safety
**/*.{ts,tsx}: Never useanytype - use proper TypeScript types
Never useas anytype assertions - fix the underlying type issue
Files:
src/scripts/app.ts
**/*.{ts,tsx,js,vue}
📄 CodeRabbit inference engine (.cursorrules)
Implement proper error handling in components and services
**/*.{ts,tsx,js,vue}: Use 2-space indentation, single quotes, no semicolons, and maintain 80-character line width as configured in.prettierrc
Organize imports by sorting and grouping by plugin, and runpnpm formatbefore committing
Files:
src/scripts/app.ts
src/**/*.{vue,ts}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.{vue,ts}: Leverage VueUse functions for performance-enhancing styles
Implement proper error handling
Use vue-i18n in composition API for any string literals. Place new translation entries in src/locales/en/main.json
Files:
src/scripts/app.ts
src/**/*.ts
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.ts: Use es-toolkit for utility functions
Use TypeScript for type safety
Files:
src/scripts/app.ts
**/*.{ts,tsx,js,jsx,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
Use camelCase for variable and setting names in TypeScript/Vue files
Files:
src/scripts/app.ts
**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,vue}: Useconst settingStore = useSettingStore()andsettingStore.get('Comfy.SomeSetting')to retrieve settings in TypeScript/Vue files
Useawait settingStore.set('Comfy.SomeSetting', newValue)to update settings in TypeScript/Vue files
Check server capabilities usingapi.serverSupportsFeature('feature_name')before using enhanced features
Useapi.getServerFeature('config_name', defaultValue)to retrieve server feature configurationEnforce ESLint rules for Vue + TypeScript including: no floating promises, no unused imports, and i18n raw text restrictions in templates
Files:
src/scripts/app.ts
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.ts: Define dynamic setting defaults using runtime context with functions in settings configuration
UsedefaultsByInstallVersionproperty for gradual feature rollout based on version in settings configuration
Files:
src/scripts/app.ts
src/**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (src/CLAUDE.md)
src/**/*.{ts,tsx,vue}: Sanitize HTML with DOMPurify to prevent XSS attacks
Avoid using @ts-expect-error; use proper TypeScript types instead
Use es-toolkit for utility functions instead of other utility libraries
Implement proper TypeScript types throughout the codebase
Files:
src/scripts/app.ts
src/**/*.{vue,ts,tsx}
📄 CodeRabbit inference engine (src/CLAUDE.md)
Follow Vue 3 composition API style guide
Files:
src/scripts/app.ts
🧠 Learnings (2)
📚 Learning: 2025-11-24T19:47:56.371Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/lib/litegraph/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:56.371Z
Learning: Applies to src/lib/litegraph/**/*.{js,ts,jsx,tsx} : Do not replace `&&=` or `||=` with `=` when there is no reason to do so. If you do find a reason to remove either `&&=` or `||=`, leave a comment explaining why the removal occurred
Applied to files:
src/scripts/app.ts
📚 Learning: 2025-11-24T19:47:56.371Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/lib/litegraph/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:56.371Z
Learning: Applies to src/lib/litegraph/**/*.{js,ts,jsx,tsx} : When writing methods, prefer returning idiomatic JavaScript `undefined` over `null`
Applied to files:
src/scripts/app.ts
🧬 Code graph analysis (1)
src/scripts/app.ts (2)
src/stores/queueStore.ts (1)
workflow(314-316)src/platform/workflow/validation/schemas/workflowSchema.ts (1)
ComfyWorkflowJSON(460-462)
…s metadata - Reorder handleFile() to check workflow before parameters - Add validation to prevent JSON parse errors from crashing imports, fallback to parameters or prompt - Fix loadGraphData() to use explicit type validation instead of falsy check - Ensures ComfyUI-generated PNGs with both metadata types load the workflow, not parameters Fixes issue where large workflows (e.g., 634 nodes) were replaced with basic A1111 format when importing PNG files.
The parameters section should under promptThe API workflow uses prompt to stroge workflow because we don't care about node position. Otherwise, it won't fully fix for API generated. |
#7154) …s metadata - Reorder handleFile() to check workflow before parameters - Add validation to prevent JSON parse errors from crashing imports - Fix loadGraphData() to use explicit type validation instead of falsy check - Ensures ComfyUI-generated PNGs with both metadata types load the workflow, not parameters Fixes issue where large workflows (e.g., 634 nodes) were replaced with basic A1111 format when importing PNG files. ## Summary Fixed workflow loading from PNG images to prioritize workflow metadata over parameters, preventing large workflows from being replaced with basic A1111 format. ## Changes - **What**: Reordered `handleFile()` to check workflow before parameters, added JSON parse error handling and validation, fixed `loadGraphData()` to use explicit type checking instead of falsy check - **Dependencies**: None ## Review Focus The key issue was in `handleFile()` where parameters were checked before workflow, causing ComfyUI-generated PNGs (which contain both workflow and parameters metadata) to incorrectly import as A1111 format. The fix ensures: 1. Workflow is always checked first and validated properly 2. Parameters are only used as a fallback when no workflow exists 3. Invalid/malformed workflow data doesn't crash the import process Additionally, `loadGraphData()` was using a falsy check (`if (!graphData)`) which could incorrectly replace valid but falsy values. Now uses explicit type validation. Tested with real-world PNG containing 634-node workflow (780KB) + parameters (1KB) - now correctly loads the workflow instead of discarding it. <!-- Fixes #ISSUE_NUMBER --> ## Screenshots (if applicable) N/A - Backend logic fix, no UI changes Fixes #6633 ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-7154-Fix-workflow-loading-from-PNG-images-with-both-workflow-and-parameter-2bf6d73d365081ecb7a6c4bf6b6ccd51) by [Unito](https://www.unito.io) --------- Co-authored-by: Alexander Brown <DrJKL0424@gmail.com> Co-authored-by: Alexander Brown <drjkl@comfy.org>
…orkflow and parameter… (#7265) Backport of #7154 to `core/1.33` Automatically created by backport workflow. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-7265-backport-core-1-33-Fix-workflow-loading-from-PNG-images-with-both-workflow-and-paramete-2c46d73d365081e2b6f2dcbec3f2ba04) by [Unito](https://www.unito.io) --------- Co-authored-by: Rvage <skentler@protonmail.com> Co-authored-by: Alexander Brown <DrJKL0424@gmail.com> Co-authored-by: Alexander Brown <drjkl@comfy.org> Co-authored-by: GitHub Action <action@github.com>

…s metadata
Fixes issue where large workflows (e.g., 634 nodes) were replaced with basic A1111 format when importing PNG files.
Summary
Fixed workflow loading from PNG images to prioritize workflow metadata over parameters, preventing large workflows from being replaced with basic A1111 format.
Changes
handleFile()to check workflow before parameters, added JSON parse error handling and validation, fixedloadGraphData()to use explicit type checking instead of falsy checkReview Focus
The key issue was in
handleFile()where parameters were checked before workflow, causing ComfyUI-generated PNGs (which contain both workflow and parameters metadata) to incorrectly import as A1111 format. The fix ensures:Additionally,
loadGraphData()was using a falsy check (if (!graphData)) which could incorrectly replace valid but falsy values. Now uses explicit type validation.Tested with real-world PNG containing 634-node workflow (780KB) + parameters (1KB) - now correctly loads the workflow instead of discarding it.
Screenshots (if applicable)
N/A - Backend logic fix, no UI changes
Fixes #6633
┆Issue is synchronized with this Notion page by Unito