Skip to content

Conversation

@coderfromthenorth93
Copy link
Contributor

No description provided.

@christian-byrne
Copy link
Contributor

LGTM

@christian-byrne
Copy link
Contributor

Testing Existing Behavior ✔️

  1. Added fork and checked out master
  2. Installed 3 custom nodes (ComfyUI-Manager, comfyui-impact-pack, claude-code-comfyui-nodes)
  3. Started ComfyUI
  4. Observed project config parsing prcoeeds without error
Total VRAM 11975 MB, total RAM 64121 MB
pytorch version: 2.6.0+cu124
Set vram state to: NORMAL_VRAM
Device: cuda:0 NVIDIA GeForce RTX 4070 : cudaMallocAsync
Using pytorch attention
Python version: 3.12.3 (main, Feb  4 2025, 14:48:35) [GCC 13.3.0]
ComfyUI version: 0.3.40
ComfyUI frontend version: 1.21.7
[Prompt Server] web root: /home/c_byrne/projects/comfy-testing-environment/ComfyUI/.venv/lib/python3.12/site-packages/comfyui_frontend_package/static
### Loading: ComfyUI-Impact-Pack (V8.16)
project_data: {'name': 'comfyui-impact-pack', 'description': 'This node pack offers various detector nodes and detailer nodes that allow you to configure a workflow that automatically enhances facial details. And provide iterative upscaler.', 'version': '8.16', 'license': {'file': 'LICENSE.txt'}, 'dependencies': ['segment-anything', 'scikit-image', 'piexif', 'transformers', 'opencv-python-headless', 'GitPython', 'scipy>=1.11.4'], 'urls': {'Repository': 'https://github.com/ltdrdata/ComfyUI-Impact-Pack'}}
comfy_data: {'PublisherId': 'drltdata', 'DisplayName': 'ComfyUI Impact Pack', 'Icon': ''}
[Impact Pack] Wildcards loading done.
### Loading: ComfyUI-Manager (V3.32.8)
[ComfyUI-Manager] network_mode: public
### ComfyUI Revision: 3569 [a52c665f] *DETACHED | Released on '2025-06-16'
project_data: {'name': 'comfyui-manager', 'description': 'ComfyUI-Manager provides features to install and manage custom nodes for ComfyUI, as well as various functionalities to assist with ComfyUI.', 'version': '3.32.8', 'license': {'file': 'LICENSE.txt'}, 'dependencies': ['GitPython', 'PyGithub', 'matrix-client==0.4.0', 'transformers', 'huggingface-hub>0.20', 'typer', 'rich', 'typing-extensions', 'toml', 'uv', 'chardet'], 'urls': {'Repository': 'https://github.com/ltdrdata/ComfyUI-Manager'}}
comfy_data: {'PublisherId': 'drltdata', 'DisplayName': 'ComfyUI-Manager', 'Icon': ''}

@christian-byrne
Copy link
Contributor

christian-byrne commented Jun 16, 2025

Testing New Behavior ❌

  1. Edit the pyproject.toml of one of my custom nodes. Just copy-pasted the example from https://docs.comfy.org/registry/specifications, which includes the newly added fields
  2. Started up ComfyUI

Not all supported fields seem to have been parsed:

project_data: {'name': 'super-resolution-node', 'version': '1.0.0', 'description': 'Enhance image quality using advanced super resolution techniques', 'license': {'file': 'LICENSE'}, 'requires-python': '>=3.8', 'dependencies': ['comfyui-frontend-package<=1.21.6'], 'classifiers': ['Operating System :: OS Independent'], 'dynamic': ['dependencies'], 'urls': {'Repository': 'https://github.com/username/super-resolution-node', 'Documentation': 'https://github.com/username/super-resolution-node/wiki', 'Bug Tracker': 'https://github.com/username/super-resolution-node/issues'}}
comfy_data: {'PublisherId': 'image-wizard', 'DisplayName': 'Super Resolution Node', 'Icon': 'https://raw.githubusercontent.com/username/super-resolution-node/main/icon.png', 'Banner': 'https://raw.githubusercontent.com/username/super-resolution-node/main/banner.png', 'requires-comfyui': '>=1.0.0'}

Copy link
Contributor

@christian-byrne christian-byrne left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PR adds four new fields to ProjectConfig in comfy_config/types.py:

  • supported_os
  • supported_accelerators
  • supported_comfyui_version
  • supported_comfyui_frontend_version

However, these fields aren't being populated because the PR is missing the necessary data transformation logic in config_parser.py.

@coderfromthenorth93 Do we need to add the same parsing logic that was added in Comfy-Org/comfy-cli#279?

Copy link
Contributor

@christian-byrne christian-byrne left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Comprehensive Testing Completed

I've thoroughly tested the PR implementation with full coverage of all new metadata fields and edge cases.

✅ Basic Functionality Test

Created comprehensive test node with all new fields:

classifiers = [
    "Operating System :: OS Independent",
    "Operating System :: Microsoft :: Windows", 
    "Operating System :: POSIX :: Linux",
    "Operating System :: MacOS",
    "Environment :: GPU :: NVIDIA CUDA",
    "Environment :: GPU :: AMD ROCm",
    "Environment :: GPU :: Apple Metal",
]
dependencies = ["comfyui-frontend-package>=1.21.0"]

[tool.comfy]
requires-comfyui = ">=0.3.0"

Results: All fields parsed correctly ✅

  • OS classifiers: ['OS Independent', 'Microsoft :: Windows', 'POSIX :: Linux', 'MacOS']
  • Accelerator classifiers: ['GPU :: NVIDIA CUDA', 'GPU :: AMD ROCm', 'GPU :: Apple Metal']
  • ComfyUI version: >=0.3.0
  • Frontend version: >=1.21.0

✅ Edge Cases & Error Handling (6/6 passed)

  1. Invalid OS classifiers: Returns empty list when any classifier is invalid ✅
  2. Invalid accelerator classifiers: Returns empty list when any classifier is invalid ✅
  3. No classifiers: Returns empty lists for both fields ✅
  4. No frontend dependency: Returns empty string ✅
  5. No ComfyUI version: Returns empty string ✅
  6. Mixed valid/invalid classifiers: Returns empty list (all-or-nothing validation) ✅

Implementation Status

PR #8507 implementation is working correctly - all new metadata fields are properly parsed, validated, and populated according to the specifications from comfy-cli PR #279 and docs.comfy.org registry specifications.

@christian-byrne christian-byrne added Good PR This PR looks good to go, it needs comfy's final review. Core-Important labels Jun 18, 2025
@comfyanonymous comfyanonymous merged commit 5b12b55 into Comfy-Org:master Jun 18, 2025
9 checks passed
adlerfaulkner pushed a commit to LucaLabsInc/ComfyUI that referenced this pull request Oct 16, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Core Core team dependency Core-Important Good PR This PR looks good to go, it needs comfy's final review.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants