Conversation
benceruleanlu
added
the
Update Compiled Requirements
dosubot
bot
added
the
size:M
|
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:
📝 WalkthroughWalkthroughThis pull request updates ComfyUI to version 0.16.0 and synchronizes dependencies across platform-specific requirement files. Multiple workflow template packages and comfy-aimdo are updated to newer versions, while pyopengl-accelerate is removed from all compiled requirements. Changes
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 18bf724f8b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
package.json
Outdated
| "version": "0.15.1", | ||
| "optionalBranch": "" | ||
| "version": "e04d0dbeb8266aa9262b5a4c3934ba4e4a371e37", | ||
| "optionalBranch": "master" |
There was a problem hiding this comment.
Setting config.comfyUI.optionalBranch to "master" makes the new commit SHA in config.comfyUI.version effectively dead code, because both scripts/makeComfy.js and the updated requirements workflow now choose optionalBranch first. In practice, every asset build and requirements refresh will pull the live tip of master instead of e04d0db..., so outputs become non-reproducible and can break as soon as upstream changes (for example, when core-requirements.patch no longer matches). Keep optionalBranch empty (or switch checkout logic to a fixed SHA) to preserve a real pin.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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)
.github/workflows/update_compiled_requirements.yml (1)
103-125:⚠️ Potential issue | 🟠 MajorFallback ref construction is still broken for SHA-based versions.
Lines 124 and 211 always prepend
vwhencomfyui_optional_branchis empty. With SHA versions, that becomes an invalid ref (v<sha>). Normalize a singlecomfyui_refoutput (branch > sha > tag) and use one checkout step per job.Suggested fix pattern (apply in both Windows and macOS jobs)
- COMFYUI_VERSION=$(jq -r '.config.comfyUI.version' package.json) - COMFYUI_OPTIONAL_BRANCH=$(jq -r '.config.comfyUI.optionalBranch // ""' package.json) + COMFYUI_VERSION=$(jq -r '.config.comfyUI.version' package.json) + COMFYUI_OPTIONAL_BRANCH=$(jq -r '.config.comfyUI.optionalBranch // ""' package.json) + if [ -n "$COMFYUI_OPTIONAL_BRANCH" ]; then + COMFYUI_REF="$COMFYUI_OPTIONAL_BRANCH" + elif [[ "$COMFYUI_VERSION" =~ ^[0-9a-f]{40}$ ]]; then + COMFYUI_REF="$COMFYUI_VERSION" + else + COMFYUI_REF="v${COMFYUI_VERSION}" + fi MANAGER_COMMIT=$(jq -r '.config.managerCommit' package.json) { - echo "comfyui_version=${COMFYUI_VERSION}" echo "comfyui_optional_branch=${COMFYUI_OPTIONAL_BRANCH}" + echo "comfyui_ref=${COMFYUI_REF}" echo "manager_commit=${MANAGER_COMMIT}" } >> "$GITHUB_OUTPUT" - - name: Checkout ComfyUI (optional branch) - if: steps.versions.outputs.comfyui_optional_branch != '' - uses: actions/checkout@0ad4b8fadaa221de15dcec353f45205ec38ea70b # v4.1.4 - with: - repository: Comfy-Org/ComfyUI - ref: ${{ steps.versions.outputs.comfyui_optional_branch }} - path: assets/ComfyUI - - - name: Checkout ComfyUI (version tag) - if: steps.versions.outputs.comfyui_optional_branch == '' + - name: Checkout ComfyUI uses: actions/checkout@0ad4b8fadaa221de15dcec353f45205ec38ea70b # v4.1.4 with: repository: Comfy-Org/ComfyUI - ref: v${{ steps.versions.outputs.comfyui_version }} + ref: ${{ steps.versions.outputs.comfyui_ref }} path: assets/ComfyUIAlso applies to: 190-212
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/update_compiled_requirements.yml around lines 103 - 125, The checkout ref construction wrongly prepends "v" to COMFYUI_VERSION when comfyui_optional_branch is empty, breaking SHA refs; create a normalized output (e.g., comfyui_ref) in the versions step that selects: if comfyui_optional_branch != '' then comfyui_optional_branch else if comfyui_version looks like a full SHA then comfyui_version else "v${comfyui_version}", and then replace the conditional two-checkout blocks with a single checkout step that uses steps.versions.outputs.comfyui_ref (and symbols COMFYUI_OPTIONAL_BRANCH, COMFYUI_VERSION / comfyui_version) so both branch, SHA, and tag refs are handled correctly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@package.json`:
- Around line 18-19: The package.json currently contains optionalBranch which
overrides the pinned version SHA and makes builds non-reproducible; remove the
optionalBranch key (or set it to null) from package.json and update the
branch-resolution logic in the makeComfy script so that the "version" SHA takes
precedence over "optionalBranch" (i.e., if version is present use that commit
SHA, otherwise fall back to optionalBranch); ensure the code paths and any
helper such as the branch-resolver use "version" first and only consult
"optionalBranch" when "version" is absent.
---
Outside diff comments:
In @.github/workflows/update_compiled_requirements.yml:
- Around line 103-125: The checkout ref construction wrongly prepends "v" to
COMFYUI_VERSION when comfyui_optional_branch is empty, breaking SHA refs; create
a normalized output (e.g., comfyui_ref) in the versions step that selects: if
comfyui_optional_branch != '' then comfyui_optional_branch else if
comfyui_version looks like a full SHA then comfyui_version else
"v${comfyui_version}", and then replace the conditional two-checkout blocks with
a single checkout step that uses steps.versions.outputs.comfyui_ref (and symbols
COMFYUI_OPTIONAL_BRANCH, COMFYUI_VERSION / comfyui_version) so both branch, SHA,
and tag refs are handled correctly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: c0291f7e-f459-494e-83f5-4978df9180a0
📒 Files selected for processing (3)
.github/workflows/update_compiled_requirements.ymlpackage.jsonscripts/core-requirements.patch
dosubot
bot
added
size:XS
benceruleanlu
changed the title
benceruleanlu
added
Update Compiled Requirements
dosubot
bot
added
the
size:XS
benceruleanlu
added
Update Compiled Requirements
benceruleanlu
removed
the
Update Compiled Requirements
dosubot
bot
added
size:M
|
✅ Compiled requirements have been updated successfully! |
dosubot
bot
added
size:L
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)
assets/requirements/macos.compiled (1)
298-312:⚠️ Potential issue | 🟠 MajorUpgrade torchaudio to 2.10.0 to match torch 2.10.0.
Line 308 pins
torchaudio==2.9.1, but Line 298 istorch==2.10.0. TorchAudio 2.9.1 requires torch 2.9.1 (matching patch version), creating binary/ABI incompatibility. Line 318 correctly hastorchvision==0.25.0(the torch 2.10 series release). Align torchaudio to 2.10.0.Proposed fix
-torchaudio==2.9.1 +torchaudio==2.10.0🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@assets/requirements/macos.compiled` around lines 298 - 312, Update the torchaudio pin to match the torch patch version: change the dependency torchaudio==2.9.1 to torchaudio==2.10.0 so it is ABI-compatible with torch==2.10.0; locate the torchaudio entry in the requirements list (the line that currently reads torchaudio==2.9.1) and replace the version specifier with 2.10.0, keeping any surrounding comments intact.
🧹 Nitpick comments (1)
assets/requirements/windows_cpu.compiled (1)
2-2: Align the lockfile compile input with the runtime's manager requirements path preference.The compiled lockfile command (line 2) uses the legacy path
assets/ComfyUI/custom_nodes/ComfyUI-Manager/requirements.txt, while runtime code (src/virtualEnvironment.ts,scripts/verifyBuild.js,scripts/makeComfy.js) checks for the primary pathComfyUI/manager_requirements.txtfirst and only falls back to the legacy path if absent. This architectural misalignment means the compiled lockfile and runtime may resolve from different sources, creating potential for divergence.The workflow should be updated to compile from the primary path to match the runtime's preference resolution:
Proposed workflow update
- UV_CMD_WINDOWS_CPU: 'uv pip compile assets/ComfyUI/requirements.txt assets/ComfyUI/custom_nodes/ComfyUI-Manager/requirements.txt --emit-index-annotation --emit-index-url --index-strategy unsafe-best-match -o assets/requirements/windows_cpu.compiled --index-url https://pypi.org/simple' - UV_CMD_WINDOWS_NVIDIA: 'uv pip compile assets/ComfyUI/requirements.txt assets/ComfyUI/custom_nodes/ComfyUI-Manager/requirements.txt --emit-index-annotation --emit-index-url --index-strategy unsafe-best-match --override assets/override.txt --index-url https://pypi.org/simple --extra-index-url https://download.pytorch.org/whl/cu129 -o assets/requirements/windows_nvidia.compiled' - UV_CMD_MACOS: 'uv pip compile assets/ComfyUI/requirements.txt assets/ComfyUI/custom_nodes/ComfyUI-Manager/requirements.txt --emit-index-annotation --emit-index-url --index-strategy unsafe-best-match -o assets/requirements/macos.compiled --override assets/override.txt --index-url https://pypi.org/simple' + UV_CMD_WINDOWS_CPU: 'uv pip compile assets/ComfyUI/requirements.txt assets/ComfyUI/manager_requirements.txt --emit-index-annotation --emit-index-url --index-strategy unsafe-best-match -o assets/requirements/windows_cpu.compiled --index-url https://pypi.org/simple' + UV_CMD_WINDOWS_NVIDIA: 'uv pip compile assets/ComfyUI/requirements.txt assets/ComfyUI/manager_requirements.txt --emit-index-annotation --emit-index-url --index-strategy unsafe-best-match --override assets/override.txt --index-url https://pypi.org/simple --extra-index-url https://download.pytorch.org/whl/cu129 -o assets/requirements/windows_nvidia.compiled' + UV_CMD_MACOS: 'uv pip compile assets/ComfyUI/requirements.txt assets/ComfyUI/manager_requirements.txt --emit-index-annotation --emit-index-url --index-strategy unsafe-best-match -o assets/requirements/macos.compiled --override assets/override.txt --index-url https://pypi.org/simple'Additionally, the workflow's checkout step (lines 116-121) could conditionally skip the legacy ComfyUI-Manager clone if the primary path is detected, matching the logic in
scripts/makeComfy.js.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@assets/requirements/windows_cpu.compiled` at line 2, The compiled lockfile command uses the legacy path assets/ComfyUI/custom_nodes/ComfyUI-Manager/requirements.txt; update the uv pip compile invocation to use the runtime-preferred primary path ComfyUI/manager_requirements.txt so the generated lockfile matches the lookup logic in src/virtualEnvironment.ts, scripts/verifyBuild.js and scripts/makeComfy.js. Also adjust the workflow checkout logic that currently clones the legacy ComfyUI-Manager to conditionally skip that clone when ComfyUI/manager_requirements.txt exists (or only clone the legacy repo as a fallback), mirroring the same primary-then-fallback behavior implemented in scripts/makeComfy.js.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@assets/requirements/windows_nvidia.compiled`:
- Around line 2-5: The lockfile currently lists the PyTorch cu129 index ahead of
cu130 ("--extra-index-url https://download.pytorch.org/whl/cu129"), causing
resolution to prefer cu129; update the file so the cu130 index is prioritized by
moving or inserting "--extra-index-url https://download.pytorch.org/whl/cu130"
before any cu129 entries (or replace cu129 occurrences with cu130), and make the
same change for every other cu129 occurrence mentioned (positions 57, 148, 165,
174, 182, 298, 371) to ensure package resolution is consistent with the runtime
cu130 constraints.
---
Outside diff comments:
In `@assets/requirements/macos.compiled`:
- Around line 298-312: Update the torchaudio pin to match the torch patch
version: change the dependency torchaudio==2.9.1 to torchaudio==2.10.0 so it is
ABI-compatible with torch==2.10.0; locate the torchaudio entry in the
requirements list (the line that currently reads torchaudio==2.9.1) and replace
the version specifier with 2.10.0, keeping any surrounding comments intact.
---
Nitpick comments:
In `@assets/requirements/windows_cpu.compiled`:
- Line 2: The compiled lockfile command uses the legacy path
assets/ComfyUI/custom_nodes/ComfyUI-Manager/requirements.txt; update the uv pip
compile invocation to use the runtime-preferred primary path
ComfyUI/manager_requirements.txt so the generated lockfile matches the lookup
logic in src/virtualEnvironment.ts, scripts/verifyBuild.js and
scripts/makeComfy.js. Also adjust the workflow checkout logic that currently
clones the legacy ComfyUI-Manager to conditionally skip that clone when
ComfyUI/manager_requirements.txt exists (or only clone the legacy repo as a
fallback), mirroring the same primary-then-fallback behavior implemented in
scripts/makeComfy.js.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: a2433b05-4ae9-43cb-a970-0f53c53dbcf7
📒 Files selected for processing (3)
assets/requirements/macos.compiledassets/requirements/windows_cpu.compiledassets/requirements/windows_nvidia.compiled
This reverts commit 2f77be6.
dosubot
bot
added
size:M
## Summary - bump desktop app version in `package.json` from `0.8.11` to `0.8.12` Merge after #1633 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Chores** * Version bumped to 0.8.12. <!-- end of auto-generated comment: release notes by coderabbit.ai --> ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-1634-chore-bump-desktop-version-to-0-8-12-31a6d73d36508136a798ff5d4d8f747b) by [Unito](https://www.unito.io)
2 participants
Summary
config.comfyUI.versionto0.16.0(tag-based)config.comfyUI.optionalBranchempty (no SHA or branch pin)scripts/core-requirements.patchforv0.16.0requirements (comfyui-workflow-templates==0.9.7)Testing
yarn formatyarn lintyarn typecheckSummary by CodeRabbit