Skip to content

fix(nodes): ensure each invocation overrides _original_model_fields with own field data #8114

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

Merged
merged 1 commit into from
Jun 20, 2025

Conversation

psychedelicious
Copy link
Collaborator

@psychedelicious psychedelicious commented Jun 19, 2025

Summary

_original_model_fields is a dict ClassVar on BaseInvocation. It stores the original fields for an Invocation, before any modifications are made by the @invocation decorator.

Individual Invocations were intended to override the whole dict, but instead they added their own fields to it.

# baseinvocation.py
cls._original_model_fields[field_name] = OriginalModelField(annotation=annotation, field_info=field_info)

The dict was therefore accidentally shared amongst all invocation classes. Here's what it looks like for IPAdapterInvocation:

Screenshot 2025-06-19 at 12 44 40 pm

Note that a ton of fields unrelated to IP Adapter are in there. It shouldn't have those! Here's what it should look like:

Screenshot 2025-06-19 at 12 44 10 pm

Note that it only includes fields from the IP Adapter node.


This bug has a knock-on effect when two invocations share the same field name. The original model field data would be replaced/clobbered.

For example, both IPAdapterInvocation and FloatToIntegerInvocation have a method field:

class IPAdapterInvocation(BaseInvocation):
    # ...
    method: Literal["full", "style", "composition", "style_strong", "style_precise"] = InputField(
        default="full", description="The method to apply the IP-Adapter"
    )

class FloatToIntegerInvocation(BaseInvocation):
    # ...
    method: Literal["Nearest", "Floor", "Ceiling", "Truncate"] = InputField(
        default="Nearest", description="The method to use for rounding"
    )

We can have this sequence of events:

  • IPAdapterInvocation is registered, storing its field data in the dict.
    • cls._original_model_fields["method"] is set to IPAdapterInvocation.method's field data.
  • FloatToIntegerInvocation is registered, storing its field data in the dict.
    • cls._original_model_fields["method"] is set to FloatToIntegerInvocation.method's field data.

So IPAdapterInvocation._original_model_fields["method"] ends up referring to FloatToIntegerInvocation.method:

image

But it should refer to its own IPAdapterInvocation.method, like this:

image

Anything that consumes _original_model_fields for IPAdapterInvocation would get the float node's method field instead of its own.

Related Issues / Discussions

n/a

QA Instructions

Run in the debugger to observe _original_model_fields. There is no OSS code that consumes it.

Merge Plan

n/a

Checklist

  • The PR has a short but descriptive title, suitable for a changelog
  • Tests added / updated (if applicable)
  • Documentation added / updated (if applicable)
  • Updated What's New copy (if doing a release after this PR)

@github-actions github-actions bot added python PRs that change python files invocations PRs that change invocations labels Jun 19, 2025
@psychedelicious psychedelicious merged commit 0794eb4 into main Jun 20, 2025
12 checks passed
@psychedelicious psychedelicious deleted the psyche/fix/nodes/original-model-fields branch June 20, 2025 05:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
invocations PRs that change invocations python PRs that change python files
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants