Fix V3 node subclasses inheriting the parent's cached schema values - #16205
Open
L4XB wants to merge 1 commit into
Open
Fix V3 node subclasses inheriting the parent's cached schema values#16205L4XB wants to merge 1 commit into
L4XB wants to merge 1 commit into
Conversation
The RETURN_TYPES, RETURN_NAMES, OUTPUT_NODE etc. sentinels are plain class attributes, so once a parent V3 node was schema'd every subclass saw the populated value and the `is None` guard in GET_SCHEMA never fired. A subclass overriding define_schema to change its outputs kept the parent's RETURN_TYPES and failed output validation at execution time. Reset the cached values on every new subclass in __init_subclass__. Class clones copy the parent's __dict__ on purpose, so attributes the new class already carries are left alone. Fixes Comfy-Org#16132
L4XB
requested review from
Kosinkadink,
alexisrolland,
comfyanonymous,
guill,
kijai and
rattus128
as code owners
September 9, 2026 17:32
Contributor
|
✅ All contributors have signed the CLA. Thank you! This PR is ready to be merged. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📜 Recent review details
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage | Docstring coverage is 14.29% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 7 functions across 2 files. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
✅ Passed checks (4 passed)
| Check name | Status | Explanation |
|---|---|---|
| Linked Issues check | ✅ Passed | The changes address issue #16132 by resetting inherited cached schema attributes for each subclass. The tests cover the parent-child schema case and confirm that class clones retain cached values. |
| Out of Scope Changes check | ✅ Passed | The implementation and tests are directly related to fixing cached schema inheritance and preserving clone behavior. No unrelated changes are identified. |
| Title check | ✅ Passed | The title clearly and concisely describes the main change: preventing V3 node subclasses from inheriting cached schema values. |
| Description check | ✅ Passed | The description directly explains the bug, the cache-reset fix, clone behavior, affected attributes, linked issues, and test coverage. |
- Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
- Create stacked PR
- Commit on current branch
Comment @coderabbitai help to get the list of available commands.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Author
|
I have read and agree to the Contributor License Agreement |
comfy-legal
added a commit
to Comfy-Org/comfy-cla
that referenced
this pull request
Sep 9, 2026
Aidan-Sanders
approved these changes
Sep 10, 2026
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
2 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.
Fixes #16132
Fixes #16026 (same bug, reported for
OUTPUT_IS_LIST)Problem: The
_RETURN_TYPES,_RETURN_NAMES,_OUTPUT_NODE, … sentinels on_ComfyNodeBaseInternalare plain class attributes. Once a parent V3 node has been schema'd (registration does this), every subclass inherits the populated values, so theis Noneguards inGET_SCHEMA()never fire for the subclass. A subclass that overridesdefine_schema()to change its outputs keeps the parent'sRETURN_TYPESwhileSCHEMA(assigned unconditionally) is correct, and output validation inexecution.pythen indexes the stale tuple.Change:
_ComfyNodeBaseInternal.__init_subclass__resets the cached values on every new subclass.shallow_clone_class/copy_class/lock_classbuild the clone namespace from the parent's__dict__on purpose, so attributes the new class already carries in its own__dict__are left untouched and clones keep the cached schema (no extraGET_SCHEMAper execution).Tests:
tests-unit/comfy_api_test/node_schema_cache_test.pycovers the parent/child case from the issue and asserts that a class clone keeps the parent's cache. The first test fails on master and passes with this change.Related: #16058 addresses #16026 by changing every
is Noneguard tocls.__dict__.get(...). This PR takes the smaller route of resetting the caches once per subclass in__init_subclass__, so the guards and the clone behaviour stay as they are. Happy to close either one depending on which approach is preferred.On the design question raised in #16026 ("should this class be prohibited from being inherited?"): with the reset in
__init_subclass__the cache is class-local by construction, so subclassing a V3 node keeps working (the reporter's ownEmptyImage2(EmptyImage1)case relies on it) without any per-property inheritance checks.